On June 28, 2018 8:10:41 PM GMT+02:00, Manivannan Sadhasivam 
<manivannan.sadhasi...@linaro.org> wrote:
>Add Actions Semi OWL family S900 I2C driver.
>
>Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasi...@linaro.org>
>---
> drivers/i2c/busses/Kconfig   |   7 +
> drivers/i2c/busses/Makefile  |   1 +
> drivers/i2c/busses/i2c-owl.c | 471 +++++++++++++++++++++++++++++++++++
> 3 files changed, 479 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-owl.c
>
>diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>index 4f8df2ec87b1..2062da17e33b 100644
>--- a/drivers/i2c/busses/Kconfig
>+++ b/drivers/i2c/busses/Kconfig
>@@ -762,6 +762,13 @@ config I2C_OMAP
>         Like OMAP1510/1610/1710/5912 and OMAP242x.
>         For details see http://www.ti.com/omap.
> 
>+config I2C_OWL
>+      tristate "OWL I2C Controller"
>+      depends on ARCH_ACTIONS || COMPILE_TEST
>+      help
>+        Say Y here if you want to use the I2C bus controller on
>+        the Actions Semi OWL SoCs.
>+
> config I2C_PASEMI
>       tristate "PA Semi SMBus interface"
>       depends on PPC_PASEMI && PCI
>diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
>index 5a869144a0c5..b71618f77880 100644
>--- a/drivers/i2c/busses/Makefile
>+++ b/drivers/i2c/busses/Makefile
>@@ -76,6 +76,7 @@ obj-$(CONFIG_I2C_MXS)                += i2c-mxs.o
> obj-$(CONFIG_I2C_NOMADIK)     += i2c-nomadik.o
> obj-$(CONFIG_I2C_OCORES)      += i2c-ocores.o
> obj-$(CONFIG_I2C_OMAP)                += i2c-omap.o
>+obj-$(CONFIG_I2C_OWL)         += i2c-owl.o
> obj-$(CONFIG_I2C_PASEMI)      += i2c-pasemi.o
> obj-$(CONFIG_I2C_PCA_PLATFORM)        += i2c-pca-platform.o
> obj-$(CONFIG_I2C_PMCMSP)      += i2c-pmcmsp.o
>diff --git a/drivers/i2c/busses/i2c-owl.c
>b/drivers/i2c/busses/i2c-owl.c
>new file mode 100644
>index 000000000000..12320fca6755
>--- /dev/null
>+++ b/drivers/i2c/busses/i2c-owl.c
>@@ -0,0 +1,471 @@
>+// SPDX-License-Identifier: GPL-2.0+
>+/*
>+ * OWL SoC's I2C driver
>+ *
>+ * Copyright (c) 2014 Actions Semi Inc.
>+ * Author: David Liu <liu...@actions-semi.com>
>+ *
>+ * Copyright (c) 2018 Linaro Ltd.
>+ * Author: Manivannan Sadhasivam <manivannan.sadhasi...@linaro.org>
>+ */
>+
>+#include <linux/clk.h>
>+#include <linux/delay.h>
>+#include <linux/i2c.h>
>+#include <linux/interrupt.h>
>+#include <linux/module.h>
>+#include <linux/of_device.h>
>+#include <linux/io.h>

I would have kept this list fully sorted.

>+
>+/* I2C registers */
>+#define OWL_I2C_REG_CTL               0x0000
>+#define OWL_I2C_REG_CLKDIV    0x0004
>+#define OWL_I2C_REG_STAT      0x0008
>+#define OWL_I2C_REG_ADDR      0x000C
>+#define OWL_I2C_REG_TXDAT     0x0010
>+#define OWL_I2C_REG_RXDAT     0x0014
>+#define OWL_I2C_REG_CMD               0x0018
>+#define OWL_I2C_REG_FIFOCTL   0x001C
>+#define OWL_I2C_REG_FIFOSTAT  0x0020
>+#define OWL_I2C_REG_DATCNT    0x0024
>+#define OWL_I2C_REG_RCNT      0x0028
>+
>+/* I2Cx_CTL Bit Mask */
>+#define OWL_I2C_CTL_RB                BIT(1)
>+#define OWL_I2C_CTL_GBCC(x)   (((x) & 0x3) << 2)
>+#define       OWL_I2C_CTL_GBCC_NONE   OWL_I2C_CTL_GBCC(0)
>+#define       OWL_I2C_CTL_GBCC_START  OWL_I2C_CTL_GBCC(1)
>+#define       OWL_I2C_CTL_GBCC_STOP   OWL_I2C_CTL_GBCC(2)
>+#define       OWL_I2C_CTL_GBCC_RSTART OWL_I2C_CTL_GBCC(3)
>+#define OWL_I2C_CTL_IRQE      BIT(5)
>+#define OWL_I2C_CTL_EN                BIT(7)
>+#define OWL_I2C_CTL_AE                BIT(8)
>+#define OWL_I2C_CTL_SHSM      BIT(10)
>+
>+#define OWL_I2C_DIV_FACTOR(x) ((x) & 0xff)
>+
>+/* I2Cx_STAT Bit Mask */
>+#define OWL_I2C_STAT_RACK     BIT(0)
>+#define OWL_I2C_STAT_BEB      BIT(1)
>+#define OWL_I2C_STAT_IRQP     BIT(2)
>+#define OWL_I2C_STAT_LAB      BIT(3)
>+#define OWL_I2C_STAT_STPD     BIT(4)
>+#define OWL_I2C_STAT_STAD     BIT(5)
>+#define OWL_I2C_STAT_BBB      BIT(6)
>+#define OWL_I2C_STAT_TCB      BIT(7)
>+#define OWL_I2C_STAT_LBST     BIT(8)
>+#define OWL_I2C_STAT_SAMB     BIT(9)
>+#define OWL_I2C_STAT_SRGC     BIT(10)
>+
>+/* I2Cx_CMD Bit Mask */
>+#define OWL_I2C_CMD_SBE               BIT(0)
>+#define OWL_I2C_CMD_RBE               BIT(4)
>+#define OWL_I2C_CMD_DE                BIT(8)
>+#define OWL_I2C_CMD_NS                BIT(9)
>+#define OWL_I2C_CMD_SE                BIT(10)
>+#define OWL_I2C_CMD_MSS               BIT(11)
>+#define OWL_I2C_CMD_WRS               BIT(12)
>+#define OWL_I2C_CMD_SECL      BIT(15)
>+
>+#define OWL_I2C_CMD_AS(x)     (((x) & 0x7) << 1)
>+#define OWL_I2C_CMD_SAS(x)    (((x) & 0x7) << 5)
>+
>+/* I2Cx_FIFOCTL Bit Mask */
>+#define OWL_I2C_FIFOCTL_NIB   BIT(0)
>+#define OWL_I2C_FIFOCTL_RFR   BIT(1)
>+#define OWL_I2C_FIFOCTL_TFR   BIT(2)
>+
>+/* I2Cc_FIFOSTAT Bit Mask */
>+#define OWL_I2C_FIFOSTAT_RNB  BIT(1)
>+#define OWL_I2C_FIFOSTAT_RFE  BIT(2)
>+#define OWL_I2C_FIFOSTAT_TFF  BIT(5)
>+#define OWL_I2C_FIFOSTAT_TFD  GENMASK(23, 16)
>+#define OWL_I2C_FIFOSTAT_RFD  GENMASK(15, 8)
>+
>+/* I2C bus timeout */
>+#define OWL_I2C_TIMEOUT               msecs_to_jiffies(4 * 1000)
>+
>+#define OWL_I2C_MAX_RETRIES   50
>+
>+#define OWL_I2C_DEFAULT_SPEED 100000
>+#define OWL_I2C_MAX_SPEED     400000
>+
>+struct owl_i2c_dev {
>+      struct i2c_adapter      adap;
>+      struct i2c_msg          *msg;
>+      struct completion       msg_complete;
>+      struct clk              *clk;
>+      void __iomem            *base;
>+      unsigned long           clk_rate;
>+      u32                     bus_freq;
>+      u32                     msg_ptr;
>+};
>+
>+static void owl_i2c_update_reg(void __iomem *base, unsigned int val,
>bool state)

Is base really a good name here? I would go with reg.

>+{
>+      unsigned int regval;
>+
>+      regval = readl(base);
>+
>+      if (state)
>+              regval |= val;
>+      else
>+              regval &= ~val;
>+
>+      writel(regval, base);
>+}
>+
>+static int owl_i2c_reset(struct owl_i2c_dev *i2c_dev)
>+{
>+      unsigned int val, timeout = 0;
>+
>+      owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_CTL,
>+                              OWL_I2C_CTL_EN, false);
>+      mdelay(1);
>+      owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_CTL,
>+                              OWL_I2C_CTL_EN, true);
>+
>+      /* Reset FIFO */
>+      owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_FIFOCTL,
>+                              OWL_I2C_FIFOCTL_RFR | OWL_I2C_FIFOCTL_TFR,
>+                              true);
>+
>+      /* Wait 50ms for FIFO reset complete */
>+      do {
>+              val = readl(i2c_dev->base + OWL_I2C_REG_FIFOCTL);
>+              if (!(val & (OWL_I2C_FIFOCTL_RFR | OWL_I2C_FIFOCTL_TFR)))
>+                      break;
>+              mdelay(1);
>+      } while (timeout++ < OWL_I2C_MAX_RETRIES);
>+
>+      if (timeout > OWL_I2C_MAX_RETRIES) {
>+              dev_err(&i2c_dev->adap.dev, "FIFO reset timeout");
>+              return -ETIMEDOUT;
>+      }
>+
>+      /* Clear status registers */
>+      writel(0, i2c_dev->base + OWL_I2C_REG_STAT);
>+
>+      return 0;
>+}

This ..._reset function does a number of unlocked read-modify-write cycles, and 
it is called from an interrupt. Is that safe? What if a spurious interrupt 
happen?

>+
>+static int owl_i2c_set_freq(struct owl_i2c_dev *i2c_dev)
>+{
>+      unsigned int val;
>+
>+      val = (i2c_dev->clk_rate + i2c_dev->bus_freq * 16 - 1) /
>+                              (i2c_dev->bus_freq * 16);
>+
>+      /* Set clock divider factor */
>+      writel(OWL_I2C_DIV_FACTOR(val), i2c_dev->base + OWL_I2C_REG_CLKDIV);
>+
>+      return 0;
>+}
>+
>+static int owl_i2c_hw_init(struct owl_i2c_dev *i2c_dev)
>+{
>+      int ret;
>+
>+      /* Reset I2C controller */
>+      ret = owl_i2c_reset(i2c_dev);
>+      if (ret)
>+              return ret;
>+
>+      /* Set bus frequency */
>+      return owl_i2c_set_freq(i2c_dev);
>+}
>+
>+static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
>+{
>+      struct owl_i2c_dev *i2c_dev = _dev;
>+      struct i2c_msg *msg = i2c_dev->msg;
>+      unsigned int stat, fifostat;
>+
>+      fifostat = readl(i2c_dev->base + OWL_I2C_REG_FIFOSTAT);
>+      if (fifostat & OWL_I2C_FIFOSTAT_RNB) {
>+              dev_dbg(&i2c_dev->adap.dev, "received NACK from device");
>+              owl_i2c_reset(i2c_dev);
>+              goto stop;
>+      }
>+
>+      stat = readl(i2c_dev->base + OWL_I2C_REG_STAT);
>+      if (stat & OWL_I2C_STAT_BEB) {
>+              dev_dbg(&i2c_dev->adap.dev, "bus error");
>+              owl_i2c_reset(i2c_dev);
>+              goto stop;
>+      }
>+
>+      /* Handle FIFO read */
>+      if (msg->flags & I2C_M_RD) {
>+              while ((readl(i2c_dev->base + OWL_I2C_REG_FIFOSTAT) &
>+                              OWL_I2C_FIFOSTAT_RFE) &&
>+                              (i2c_dev->msg_ptr < msg->len)) {
>+                      msg->buf[i2c_dev->msg_ptr++] = readl(i2c_dev->base +
>+                                                      OWL_I2C_REG_RXDAT);
>+              }
>+      } else {
>+              /* Handle the remaining bytes which were not sent */
>+              while (!(readl(i2c_dev->base + OWL_I2C_REG_FIFOSTAT) &
>+                              OWL_I2C_FIFOSTAT_TFF) &&
>+                              i2c_dev->msg_ptr < msg->len) {
>+                      writel(msg->buf[i2c_dev->msg_ptr++], i2c_dev->base +
>+                                                      OWL_I2C_REG_TXDAT);
>+              }
>+      }
>+
>+stop:
>+      /* Clear pending interrupts */
>+      owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_STAT,
>+                              OWL_I2C_STAT_IRQP, true);
>+
>+      complete_all(&i2c_dev->msg_complete);
>+
>+      return IRQ_HANDLED;
>+}
>+
>+static u32 owl_i2c_func(struct i2c_adapter *adap)
>+{
>+      return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
>+}
>+
>+static int owl_i2c_check_bus_busy(struct i2c_adapter *adap)
>+{
>+      struct owl_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
>+      unsigned long timeout;
>+      unsigned int val;
>+
>+      /* Check for Arbitration lost */
>+      val = readl(i2c_dev->base + OWL_I2C_REG_STAT);
>+      if (val & OWL_I2C_STAT_LAB) {
>+              val &= ~OWL_I2C_STAT_LAB;
>+              writel(val, i2c_dev->base + OWL_I2C_REG_STAT);
>+              return -EAGAIN;
>+      }
>+
>+      /* Check for Bus busy */
>+      timeout = jiffies + OWL_I2C_TIMEOUT;
>+      while (readl(i2c_dev->base + OWL_I2C_REG_STAT) & OWL_I2C_STAT_BBB) {
>+              if (time_after(jiffies, timeout)) {
>+                      dev_err(&adap->dev, "Bus busy timeout");
>+                      return -ETIMEDOUT;
>+              }
>+      }
>+
>+      return 0;
>+}
>+
>+static int owl_i2c_master_xfer(struct i2c_adapter *adap, struct
>i2c_msg *msgs,
>+                              int num)
>+{
>+      struct owl_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
>+      struct i2c_msg *msg;
>+      unsigned long time_left;
>+      unsigned int i2c_cmd;
>+      unsigned int addr;
>+      int ret = 0, idx;
>+
>+      ret = owl_i2c_hw_init(i2c_dev);
>+      if (ret)
>+              return ret;
>+
>+      ret = owl_i2c_check_bus_busy(adap);
>+      if (ret)
>+              return ret;
>+
>+      reinit_completion(&i2c_dev->msg_complete);
>+
>+      /* Enable I2C controller interrupt */
>+      owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_CTL,
>+                              OWL_I2C_CTL_IRQE, true);
>+
>+      /*
>+       * Select: FIFO enable, Master mode, Stop enable, Data count enable,
>+       * Send start bit
>+       */
>+      i2c_cmd = (OWL_I2C_CMD_SECL | OWL_I2C_CMD_MSS | OWL_I2C_CMD_SE
>+                      | OWL_I2C_CMD_NS | OWL_I2C_CMD_DE | OWL_I2C_CMD_SBE);
>+
>+      /* Handle repeated start condition */
>+      if (num > 1) {
>+              /* Set internal address length and enable repeated start */
>+              i2c_cmd |= (OWL_I2C_CMD_AS(msgs[0].len + 1)
>+                              | OWL_I2C_CMD_SAS(1) | OWL_I2C_CMD_RBE);
>+
>+              /* Write slave address */
>+              addr = i2c_8bit_addr_from_msg(&msgs[0]);
>+              writel(addr, i2c_dev->base + OWL_I2C_REG_TXDAT);
>+
>+              /* Write internal register address */
>+              for (idx = 0; idx < msgs[0].len; idx++)
>+                      writel(msgs[0].buf[idx], i2c_dev->base +
>+                                              OWL_I2C_REG_TXDAT);
>+
>+              msg = &msgs[1];
>+      } else {
>+              /* Set address length */
>+              i2c_cmd |= OWL_I2C_CMD_AS(1);
>+              msg = &msgs[0];
>+      }
>+
>+      i2c_dev->msg = msg;
>+      i2c_dev->msg_ptr = 0;
>+
>+      /* Set data count for the message */
>+      writel(msg->len, i2c_dev->base + OWL_I2C_REG_DATCNT);
>+
>+      addr = i2c_8bit_addr_from_msg(msg);
>+      writel(addr, i2c_dev->base + OWL_I2C_REG_TXDAT);
>+
>+      if (!(msg->flags & I2C_M_RD)) {
>+              /* Write data to FIFO */
>+              for (idx = 0; idx < msg->len; idx++) {
>+                      /* Check for FIFO full */
>+                      if (readl(i2c_dev->base + OWL_I2C_REG_FIFOSTAT)
>+                                      & OWL_I2C_FIFOSTAT_TFF)

You should cleanup here, and return some error. I think? Or, why do you break 
out of the loop early?

>+                              break;
>+
>+                      writel(msg->buf[idx],
>+                                      i2c_dev->base + OWL_I2C_REG_TXDAT);
>+              }
>+
>+              i2c_dev->msg_ptr = idx;
>+      }
>+
>+      /* Ignore the NACK if needed */
>+      if (msg->flags & I2C_M_IGNORE_NAK)
>+              owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_FIFOCTL,
>+                              OWL_I2C_FIFOCTL_NIB, true);
>+      else
>+              owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_FIFOCTL,
>+                              OWL_I2C_FIFOCTL_NIB, false);
>+
>+      /* Start the transfer */
>+      writel(i2c_cmd, i2c_dev->base + OWL_I2C_REG_CMD);
>+
>+      time_left = wait_for_completion_timeout(&i2c_dev->msg_complete,
>+                                              adap->timeout);
>+      if (time_left == 0) {
>+              dev_err(&adap->dev, "Transaction timed out");
>+              /* Send stop condition and release the bus */
>+              owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_CTL,
>+                      OWL_I2C_CTL_GBCC_STOP | OWL_I2C_CTL_RB, true);
>+              ret = -ETIMEDOUT;
>+      }
>+
>+      /* Disable I2C controller */
>+      owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_CTL,
>+                              OWL_I2C_CTL_EN, false);
>+
>+      return i2c_dev->msg_ptr ? num : i2c_dev->msg_ptr;

This is wrong. Should probably be

   return ret ?: num;

Cheers,
Peter

>+}
>+
>+static const struct i2c_algorithm owl_i2c_algorithm = {
>+      .master_xfer    = owl_i2c_master_xfer,
>+      .functionality  = owl_i2c_func
>+};
>+
>+static const struct i2c_adapter_quirks owl_i2c_quirks = {
>+      .flags          = I2C_AQ_COMB | I2C_AQ_COMB_WRITE_FIRST,
>+      .max_read_len   = 240,
>+      .max_write_len  = 240,
>+      .max_comb_1st_msg_len = 6,
>+      .max_comb_2nd_msg_len = 240
>+};
>+
>+static int owl_i2c_probe(struct platform_device *pdev)
>+{
>+      struct device *dev = &pdev->dev;
>+      struct owl_i2c_dev *i2c_dev;
>+      struct resource *res;
>+      int ret, irq;
>+
>+      i2c_dev = devm_kzalloc(dev, sizeof(*i2c_dev), GFP_KERNEL);
>+      if (!i2c_dev)
>+              return -ENOMEM;
>+
>+      res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>+      i2c_dev->base = devm_ioremap_resource(dev, res);
>+      if (IS_ERR(i2c_dev->base))
>+              return PTR_ERR(i2c_dev->base);
>+
>+      irq = platform_get_irq(pdev, 0);
>+      if (irq < 0) {
>+              dev_err(dev, "failed to get IRQ number\n");
>+              return irq;
>+      }
>+
>+      if (of_property_read_u32(dev->of_node, "clock-frequency",
>+                                      &i2c_dev->bus_freq))
>+              i2c_dev->bus_freq = OWL_I2C_DEFAULT_SPEED;
>+
>+      /* We support only frequencies of 100k and 400k for now */
>+      if (i2c_dev->bus_freq != OWL_I2C_DEFAULT_SPEED &&
>+                      i2c_dev->bus_freq > OWL_I2C_MAX_SPEED) {
>+              dev_err(dev, "invalid clock-frequency %d\n", i2c_dev->bus_freq);
>+              return -EINVAL;
>+      }
>+
>+      i2c_dev->clk = devm_clk_get(dev, NULL);
>+      if (IS_ERR(i2c_dev->clk)) {
>+              dev_err(dev, "failed to get clock\n");
>+              return PTR_ERR(i2c_dev->clk);
>+      }
>+
>+      ret = clk_prepare_enable(i2c_dev->clk);
>+      if (ret)
>+              return ret;
>+
>+      i2c_dev->clk_rate = clk_get_rate(i2c_dev->clk);
>+      if (!i2c_dev->clk_rate) {
>+              dev_err(dev, "input clock rate should not be zero\n");
>+              ret = -EINVAL;
>+              goto disable_clk;
>+      }
>+
>+      init_completion(&i2c_dev->msg_complete);
>+      i2c_dev->adap.owner = THIS_MODULE;
>+      i2c_dev->adap.algo = &owl_i2c_algorithm;
>+      i2c_dev->adap.timeout = OWL_I2C_TIMEOUT;
>+      i2c_dev->adap.quirks = &owl_i2c_quirks;
>+      i2c_dev->adap.dev.parent = dev;
>+      i2c_dev->adap.dev.of_node = dev->of_node;
>+      snprintf(i2c_dev->adap.name, sizeof(i2c_dev->adap.name),
>+                      "%s", "OWL I2C adapter");
>+      i2c_set_adapdata(&i2c_dev->adap, i2c_dev);
>+
>+      platform_set_drvdata(pdev, i2c_dev);
>+
>+      ret = devm_request_irq(dev, irq, owl_i2c_interrupt, 0, pdev->name,
>+                              i2c_dev);
>+      if (ret) {
>+              dev_err(dev, "failed to request irq %d\n", irq);
>+              goto disable_clk;
>+      }
>+
>+      return i2c_add_adapter(&i2c_dev->adap);
>+
>+disable_clk:
>+      clk_disable_unprepare(i2c_dev->clk);
>+
>+      return ret;
>+}
>+
>+static const struct of_device_id owl_i2c_of_match[] = {
>+      {.compatible = "actions,s900-i2c"},
>+      { /* sentinel */ }
>+};
>+MODULE_DEVICE_TABLE(of, owl_i2c_of_match);
>+
>+static struct platform_driver owl_i2c_driver = {
>+      .probe          = owl_i2c_probe,
>+      .driver         = {
>+              .name   = "owl-i2c",
>+              .of_match_table = of_match_ptr(owl_i2c_of_match),
>+      },
>+};
>+module_platform_driver(owl_i2c_driver);
>+
>+MODULE_AUTHOR("David Liu <liu...@actions-semi.com>");
>+MODULE_AUTHOR("Manivannan Sadhasivam
><manivannan.sadhasi...@linaro.org>");
>+MODULE_DESCRIPTION("Actions Semi OWL SoCs I2C driver");
>+MODULE_LICENSE("GPL");

Reply via email to