Re: [PATCH v7 3/8] i2c: i2c-cortina: added CAxxxx I2C support

[Heiko Schocher]

Hello Alex,

Am 14.05.2020 um 22:11 schrieb Alex Nemirovsky:

From: Arthur Li <arthur...@cortina-access.com>

Add I2C controller support for Cortina Access CAxxxx SoCs

Signed-off-by: Arthur Li <arthur...@cortina-access.com>
Signed-off-by: Alex Nemirovsky <alex.nemirov...@cortina-access.com>
CC: Heiko Schocher <h...@denx.de>

CA_I2C: DT binding for I2C controller

DT binding document for Cortina I2C driver

---

Changes in v7:
- Added additional description info in I2C KConfig

Changes in v6:
- Add I2C DT binding document

Changes in v5: None
Changes in v4:
- Utilize standard I2C macros from <i2c.h>
- Return ETIMEDOUT in funcs that can timeout
- Return i2c_xfer_init() result to caller of i2c_read() if it
        fails within i2c_read() execution
- Fix misc. style guide conformance issues
- Use printf() to report i2c_xfer() runtime errors
        instead of debug()

Changes in v3: None
Changes in v2: None

  MAINTAINERS                                  |   4 +
  doc/device-tree-bindings/i2c/i2c-cortina.txt |  18 ++
  drivers/i2c/Kconfig                          |   8 +
  drivers/i2c/Makefile                         |   1 +
  drivers/i2c/i2c-cortina.c                    | 346 +++++++++++++++++++++++++++
  drivers/i2c/i2c-cortina.h                    |  84 +++++++
  6 files changed, 461 insertions(+)
  create mode 100644 doc/device-tree-bindings/i2c/i2c-cortina.txt
  create mode 100644 drivers/i2c/i2c-cortina.c
  create mode 100644 drivers/i2c/i2c-cortina.h

Reviewed-by: Heiko Schocher <h...@denx.de>

Nitpick only ...

[...]

diff --git a/drivers/i2c/i2c-cortina.c b/drivers/i2c/i2c-cortina.c
new file mode 100644
index 0000000..08b812a
--- /dev/null
+++ b/drivers/i2c/i2c-cortina.c
@@ -0,0 +1,346 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2020
+ * Arthur Li, Cortina Access, arthur...@cortina-access.com.
+ */
+
+#include <common.h>
+#include <i2c.h>
+#include <asm/io.h>
+#include <dm.h>
+#include <mapmem.h>
+#include "i2c-cortina.h"
+
+static void set_speed(struct i2c_regs *regs, int i2c_spd)
+{
+       union ca_biw_cfg i2c_cfg;
+
+       i2c_cfg.wrd = readl(&regs->i2c_cfg);
+       i2c_cfg.bf.core_en = 0;
+       writel(i2c_cfg.wrd, &regs->i2c_cfg);
+
+       switch (i2c_spd) {
+       case IC_SPEED_MODE_FAST_PLUS:
+               i2c_cfg.bf.prer = CORTINA_PER_IO_FREQ /
+                                 (5 * I2C_SPEED_FAST_PLUS_RATE) - 1;
+               break;
+
+       case IC_SPEED_MODE_STANDARD:
+               i2c_cfg.bf.prer = CORTINA_PER_IO_FREQ /
+                                 (5 * I2C_SPEED_STANDARD_RATE) - 1;
+               break;
+
+       case IC_SPEED_MODE_FAST:
+       default:
+               i2c_cfg.bf.prer = CORTINA_PER_IO_FREQ /
+                                 (5 * I2C_SPEED_FAST_RATE) - 1;
+               break;
+       }
+
+       i2c_cfg.bf.core_en = 1;
+       writel(i2c_cfg.wrd, &regs->i2c_cfg);
+}
+
+static int ca_i2c_set_bus_speed(struct udevice *bus, unsigned int speed)
+{
+       struct ca_i2c *priv = dev_get_priv(bus);
+       int i2c_spd;
+
+       if (speed >= I2C_SPEED_FAST_PLUS_RATE) {
+               i2c_spd = IC_SPEED_MODE_FAST_PLUS;
+               priv->speed = I2C_SPEED_FAST_PLUS_RATE;
+       } else if (speed >= I2C_SPEED_FAST_RATE) {
+               i2c_spd = IC_SPEED_MODE_FAST;
+               priv->speed = I2C_SPEED_FAST_RATE;
+       } else {
+               i2c_spd = IC_SPEED_MODE_STANDARD;
+               priv->speed = I2C_SPEED_STANDARD_RATE;
+       }

you could set only one vairable in the if paths (and drop {)
and set the other variable later ...
[...]

bye,
Heiko
--
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: h...@denx.de