This patchset contains the initial I2C support for Broadcom iProc family of
SoCs.
The iProc I2C controller has separate internal TX and RX FIFOs, each has a
size of 64 bytes. The iProc I2C controller supports two bus speeds including
standard mode (100 kHz) and fast mode (400 kHz)
Changes from v7:
- Remove redundant 10-bit address check in the driver
- Fix the driver that accidentally emits 1-byte of data with zero content in
the case of SMBUS quick command
- Improve debugging prints in the driver
- Other minor improvements
Changes from v6:
- Get rid of unnecessary atomic variable usage in the driver
- Improve the "waiting for transaction to complete" logic further by making
sure there's no pending/ongoing interrupt by the time when flag
'xfer_is_done' is checked
- After disabling interrupt with 'writel', add 'readl' to the same register
to flush the bus to ensure the write has gone through
Changes from v5:
- Improve the "waiting for transaction to be complete" logic to take care of
the corner case when an interrupt fires after wait_for_completion_timeout
times out
- Improve the logic to disable I2C interrupt in the remove function. Make it
more generic so it works for both dedicated and shared interrupt
Changes from v4:
- Remove redundant header file includes
- Change the logic that waits for the host controller to be idle to
simply return -EBUSY
- Use proper print level and error codes in the driver
- Allow zero length message in the driver to support I2C_SMBUS_QUICK
- Change back to use devm_request_irq. Disable interrupt in the remove
function so there's no outstanding I2C interrupt when the driver is
being removed from the framework
- Other minor miscellaneous improvements and fixes
Changes from v3:
- Add config dependency to COMPILE_TEST to allow the driver to be build tested
by other platforms
- Improve CPU utilization efficiency in the loop of waiting for bus to idle
- Add more comment in the driver to clarify the way how the "start busy"
interrupt is triggered from the I2C controller
- Fix inconsistent coding style and format
- Improve the bus speed validation logic in the driver
- Add code to free the interrupt line in driver's remove function. Also
change to use non-devm API to request the interrupt line
- Other miscellaneous improvements and fixes
Changes from v2:
- Have the I2C driver default to y so it does not need to be selected from
ARCH_BCM_IPROC. This also helps to get rid of one patch. The driver still
depends on ARCH_BCM_IPROC
- Get rid of redundant check on resource returned by platform_get_resource
Changes from v1:
- Fix function argument parenthesis
- Get rid of redundant driver owner field
Ray Jui (3):
i2c: iProc: define Broadcom iProc I2C binding
i2c: iproc: Add Broadcom iProc I2C Driver
ARM: dts: add I2C device nodes for Broadcom Cygnus
.../devicetree/bindings/i2c/brcm,iproc-i2c.txt | 37 ++
arch/arm/boot/dts/bcm-cygnus.dtsi | 20 +
drivers/i2c/busses/Kconfig | 10 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-bcm-iproc.c | 494 ++++++++++++++++++++
5 files changed, 562 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/brcm,iproc-i2c.txt
create mode 100644 drivers/i2c/busses/i2c-bcm-iproc.c
--
1.7.9.5
Document the I2C device tree binding for Broadcom iProc family of
SoCs
Signed-off-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
Reviewed-by: Kevin Cernekee <[email protected]>
---
.../devicetree/bindings/i2c/brcm,iproc-i2c.txt | 37 ++++++++++++++++++++
1 file changed, 37 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/brcm,iproc-i2c.txt
diff --git a/Documentation/devicetree/bindings/i2c/brcm,iproc-i2c.txt b/Documentation/devicetree/bindings/i2c/brcm,iproc-i2c.txt
new file mode 100644
index 0000000..81f982c
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/brcm,iproc-i2c.txt
@@ -0,0 +1,37 @@
+Broadcom iProc I2C controller
+
+Required properties:
+
+- compatible:
+ Must be "brcm,iproc-i2c"
+
+- reg:
+ Define the base and range of the I/O address space that contain the iProc
+ I2C controller registers
+
+- interrupts:
+ Should contain the I2C interrupt
+
+- clock-frequency:
+ This is the I2C bus clock. Need to be either 100000 or 400000
+
+- #address-cells:
+ Always 1 (for I2C addresses)
+
+- #size-cells:
+ Always 0
+
+Example:
+ i2c0: i2c@18008000 {
+ compatible = "brcm,iproc-i2c";
+ reg = <0x18008000 0x100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupts = <GIC_SPI 85 IRQ_TYPE_NONE>;
+ clock-frequency = <100000>;
+
+ codec: wm8750@1a {
+ compatible = "wlf,wm8750";
+ reg = <0x1a>;
+ };
+ };
--
1.7.9.5
Add initial support to the Broadcom iProc I2C controller found in the
iProc family of SoCs.
The iProc I2C controller has separate internal TX and RX FIFOs, each has
a size of 64 bytes. The iProc I2C controller supports two bus speeds
including standard mode (100kHz) and fast mode (400kHz)
Signed-off-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
Reviewed-by: Kevin Cernekee <[email protected]>
---
drivers/i2c/busses/Kconfig | 10 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-bcm-iproc.c | 494 ++++++++++++++++++++++++++++++++++++
3 files changed, 505 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-bcm-iproc.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index ab838d9..3d08731 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -372,6 +372,16 @@ config I2C_BCM2835
This support is also available as a module. If so, the module
will be called i2c-bcm2835.
+config I2C_BCM_IPROC
+ tristate "Broadcom iProc I2C controller"
+ depends on ARCH_BCM_IPROC || COMPILE_TEST
+ default ARCH_BCM_IPROC
+ help
+ If you say yes to this option, support will be included for the
+ Broadcom iProc I2C controller.
+
+ If you don't know what to do here, say N.
+
config I2C_BCM_KONA
tristate "BCM Kona I2C adapter"
depends on ARCH_BCM_MOBILE
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 56388f6..d93b509 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_I2C_AT91) += i2c-at91.o
obj-$(CONFIG_I2C_AU1550) += i2c-au1550.o
obj-$(CONFIG_I2C_AXXIA) += i2c-axxia.o
obj-$(CONFIG_I2C_BCM2835) += i2c-bcm2835.o
+obj-$(CONFIG_I2C_BCM_IPROC) += i2c-bcm-iproc.o
obj-$(CONFIG_I2C_BLACKFIN_TWI) += i2c-bfin-twi.o
obj-$(CONFIG_I2C_CADENCE) += i2c-cadence.o
obj-$(CONFIG_I2C_CBUS_GPIO) += i2c-cbus-gpio.o
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
new file mode 100644
index 0000000..5d0a03f
--- /dev/null
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -0,0 +1,494 @@
+/*
+ * Copyright (C) 2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+
+#define CFG_OFFSET 0x00
+#define CFG_RESET_SHIFT 31
+#define CFG_EN_SHIFT 30
+#define CFG_M_RETRY_CNT_SHIFT 16
+#define CFG_M_RETRY_CNT_MASK 0x0f
+
+#define TIM_CFG_OFFSET 0x04
+#define TIM_CFG_MODE_400_SHIFT 31
+
+#define M_FIFO_CTRL_OFFSET 0x0c
+#define M_FIFO_RX_FLUSH_SHIFT 31
+#define M_FIFO_TX_FLUSH_SHIFT 30
+#define M_FIFO_RX_CNT_SHIFT 16
+#define M_FIFO_RX_CNT_MASK 0x7f
+#define M_FIFO_RX_THLD_SHIFT 8
+#define M_FIFO_RX_THLD_MASK 0x3f
+
+#define M_CMD_OFFSET 0x30
+#define M_CMD_START_BUSY_SHIFT 31
+#define M_CMD_STATUS_SHIFT 25
+#define M_CMD_STATUS_MASK 0x07
+#define M_CMD_STATUS_SUCCESS 0x0
+#define M_CMD_STATUS_LOST_ARB 0x1
+#define M_CMD_STATUS_NACK_ADDR 0x2
+#define M_CMD_STATUS_NACK_DATA 0x3
+#define M_CMD_STATUS_TIMEOUT 0x4
+#define M_CMD_PROTOCOL_SHIFT 9
+#define M_CMD_PROTOCOL_MASK 0xf
+#define M_CMD_PROTOCOL_BLK_WR 0x7
+#define M_CMD_PROTOCOL_BLK_RD 0x8
+#define M_CMD_PEC_SHIFT 8
+#define M_CMD_RD_CNT_SHIFT 0
+#define M_CMD_RD_CNT_MASK 0xff
+
+#define IE_OFFSET 0x38
+#define IE_M_RX_FIFO_FULL_SHIFT 31
+#define IE_M_RX_THLD_SHIFT 30
+#define IE_M_START_BUSY_SHIFT 28
+
+#define IS_OFFSET 0x3c
+#define IS_M_RX_FIFO_FULL_SHIFT 31
+#define IS_M_RX_THLD_SHIFT 30
+#define IS_M_START_BUSY_SHIFT 28
+
+#define M_TX_OFFSET 0x40
+#define M_TX_WR_STATUS_SHIFT 31
+#define M_TX_DATA_SHIFT 0
+#define M_TX_DATA_MASK 0xff
+
+#define M_RX_OFFSET 0x44
+#define M_RX_STATUS_SHIFT 30
+#define M_RX_STATUS_MASK 0x03
+#define M_RX_PEC_ERR_SHIFT 29
+#define M_RX_DATA_SHIFT 0
+#define M_RX_DATA_MASK 0xff
+
+#define I2C_TIMEOUT_MESC 100
+#define M_TX_RX_FIFO_SIZE 64
+
+enum bus_speed_index {
+ I2C_SPD_100K = 0,
+ I2C_SPD_400K,
+};
+
+struct bcm_iproc_i2c_dev {
+ struct device *device;
+ int irq;
+
+ void __iomem *base;
+
+ struct i2c_adapter adapter;
+
+ struct completion done;
+ int xfer_is_done;
+};
+
+/*
+ * Can be expanded in the future if more interrupt status bits are utilized
+ */
+#define ISR_MASK (1 << IS_M_START_BUSY_SHIFT)
+
+static irqreturn_t bcm_iproc_i2c_isr(int irq, void *data)
+{
+ struct bcm_iproc_i2c_dev *iproc_i2c = data;
+ u32 status = readl(iproc_i2c->base + IS_OFFSET);
+
+ status &= ISR_MASK;
+
+ if (!status)
+ return IRQ_NONE;
+
+ writel(status, iproc_i2c->base + IS_OFFSET);
+ iproc_i2c->xfer_is_done = 1;
+ complete_all(&iproc_i2c->done);
+
+ return IRQ_HANDLED;
+}
+
+static bool bcm_iproc_i2c_bus_busy(struct bcm_iproc_i2c_dev *iproc_i2c)
+{
+ if (readl(iproc_i2c->base + M_CMD_OFFSET) &
+ (1 << M_CMD_START_BUSY_SHIFT))
+ return true;
+ else
+ return false;
+}
+
+static int bcm_iproc_i2c_format_addr(struct bcm_iproc_i2c_dev *iproc_i2c,
+ struct i2c_msg *msg, u8 *addr)
+{
+ *addr = msg->addr << 1;
+
+ if (msg->flags & I2C_M_RD)
+ *addr |= 1;
+
+ return 0;
+}
+
+static int bcm_iproc_i2c_check_status(struct bcm_iproc_i2c_dev *iproc_i2c,
+ struct i2c_msg *msg)
+{
+ u32 val;
+
+ val = readl(iproc_i2c->base + M_CMD_OFFSET);
+ val = (val >> M_CMD_STATUS_SHIFT) & M_CMD_STATUS_MASK;
+
+ switch (val) {
+ case M_CMD_STATUS_SUCCESS:
+ return 0;
+
+ case M_CMD_STATUS_LOST_ARB:
+ dev_dbg(iproc_i2c->device, "lost bus arbitration\n");
+ return -EAGAIN;
+
+ case M_CMD_STATUS_NACK_ADDR:
+ dev_dbg(iproc_i2c->device, "NAK addr:0x%02x\n", msg->addr);
+ return -ENXIO;
+
+ case M_CMD_STATUS_NACK_DATA:
+ dev_dbg(iproc_i2c->device, "NAK data\n");
+ return -ENXIO;
+
+ case M_CMD_STATUS_TIMEOUT:
+ dev_dbg(iproc_i2c->device, "bus timeout\n");
+ return -ETIMEDOUT;
+
+ default:
+ dev_dbg(iproc_i2c->device, "unknown error code=%d\n", val);
+ return -EIO;
+ }
+}
+
+static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
+ struct i2c_msg *msg)
+{
+ int ret, i;
+ u8 addr;
+ u32 val;
+ unsigned long time_left = msecs_to_jiffies(I2C_TIMEOUT_MESC);
+
+ /* need to reserve one byte in the FIFO for the slave address */
+ if (msg->len > M_TX_RX_FIFO_SIZE - 1) {
+ dev_err(iproc_i2c->device,
+ "only support data length up to %u bytes\n",
+ M_TX_RX_FIFO_SIZE - 1);
+ return -EINVAL;
+ }
+
+ if (bcm_iproc_i2c_bus_busy(iproc_i2c)) {
+ dev_warn(iproc_i2c->device, "bus is busy\n");
+ return -EBUSY;
+ }
+
+ ret = bcm_iproc_i2c_format_addr(iproc_i2c, msg, &addr);
+ if (ret)
+ return ret;
+
+ /* load slave address into the TX FIFO */
+ writel(addr, iproc_i2c->base + M_TX_OFFSET);
+
+ /* for a write transaction, load data into the TX FIFO */
+ if (!(msg->flags & I2C_M_RD)) {
+ for (i = 0; i < msg->len; i++) {
+ val = msg->buf[i];
+
+ /* mark the last byte */
+ if (i == msg->len - 1)
+ val |= 1 << M_TX_WR_STATUS_SHIFT;
+
+ writel(val, iproc_i2c->base + M_TX_OFFSET);
+ }
+ }
+
+ /* mark as incomplete before starting the transaction */
+ reinit_completion(&iproc_i2c->done);
+ iproc_i2c->xfer_is_done = 0;
+
+ /*
+ * Enable the "start busy" interrupt, which will be triggered after the
+ * transaction is done, i.e., the internal start_busy bit, transitions
+ * from 1 to 0.
+ */
+ writel(1 << IE_M_START_BUSY_SHIFT, iproc_i2c->base + IE_OFFSET);
+
+ /*
+ * Now we can activate the transfer. For a read operation, specify the
+ * number of bytes to read
+ */
+ val = 1 << M_CMD_START_BUSY_SHIFT;
+ if (msg->flags & I2C_M_RD) {
+ val |= (M_CMD_PROTOCOL_BLK_RD << M_CMD_PROTOCOL_SHIFT) |
+ (msg->len << M_CMD_RD_CNT_SHIFT);
+ } else {
+ val |= (M_CMD_PROTOCOL_BLK_WR << M_CMD_PROTOCOL_SHIFT);
+ }
+ writel(val, iproc_i2c->base + M_CMD_OFFSET);
+
+ time_left = wait_for_completion_timeout(&iproc_i2c->done, time_left);
+
+ /* disable all interrupts */
+ writel(0, iproc_i2c->base + IE_OFFSET);
+ /* read it back to flush the write */
+ readl(iproc_i2c->base + IE_OFFSET);
+
+ /* make sure the interrupt handler isn't running */
+ synchronize_irq(iproc_i2c->irq);
+
+ if (!time_left && !iproc_i2c->xfer_is_done) {
+ dev_err(iproc_i2c->device, "transaction timed out\n");
+
+ /* flush FIFOs */
+ val = (1 << M_FIFO_RX_FLUSH_SHIFT) |
+ (1 << M_FIFO_TX_FLUSH_SHIFT);
+ writel(val, iproc_i2c->base + M_FIFO_CTRL_OFFSET);
+ return -ETIMEDOUT;
+ }
+
+ ret = bcm_iproc_i2c_check_status(iproc_i2c, msg);
+ if (ret) {
+ /* flush both TX/RX FIFOs */
+ val = (1 << M_FIFO_RX_FLUSH_SHIFT) |
+ (1 << M_FIFO_TX_FLUSH_SHIFT);
+ writel(val, iproc_i2c->base + M_FIFO_CTRL_OFFSET);
+ return ret;
+ }
+
+ /*
+ * For a read operation, we now need to load the data from FIFO
+ * into the memory buffer
+ */
+ if (msg->flags & I2C_M_RD) {
+ for (i = 0; i < msg->len; i++) {
+ msg->buf[i] = (readl(iproc_i2c->base + M_RX_OFFSET) >>
+ M_RX_DATA_SHIFT) & M_RX_DATA_MASK;
+ }
+ }
+
+ dev_dbg(iproc_i2c->device, "xfer %c, addr=0x%02x, len=%d\n",
+ (msg->flags & I2C_M_RD) ? 'R' : 'W', msg->addr,
+ msg->len);
+ dev_dbg(iproc_i2c->device, "*** data: %*ph\n", msg->len, msg->buf);
+
+ return 0;
+}
+
+static int bcm_iproc_i2c_xfer(struct i2c_adapter *adapter,
+ struct i2c_msg msgs[], int num)
+{
+ struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adapter);
+ int ret, i;
+
+ /* go through all messages */
+ for (i = 0; i < num; i++) {
+ ret = bcm_iproc_i2c_xfer_single_msg(iproc_i2c, &msgs[i]);
+ if (ret) {
+ dev_dbg(iproc_i2c->device, "xfer failed\n");
+ return ret;
+ }
+ }
+
+ return num;
+}
+
+static uint32_t bcm_iproc_i2c_functionality(struct i2c_adapter *adap)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_algorithm bcm_iproc_algo = {
+ .master_xfer = bcm_iproc_i2c_xfer,
+ .functionality = bcm_iproc_i2c_functionality,
+};
+
+static int bcm_iproc_i2c_cfg_speed(struct bcm_iproc_i2c_dev *iproc_i2c)
+{
+ unsigned int bus_speed, speed_bit;
+ u32 val;
+ int ret = of_property_read_u32(iproc_i2c->device->of_node,
+ "clock-frequency", &bus_speed);
+ if (ret < 0) {
+ dev_info(iproc_i2c->device,
+ "unable to interpret clock-frequency DT property\n");
+ bus_speed = 100000;
+ }
+
+ if (bus_speed < 100000) {
+ dev_err(iproc_i2c->device, "%d Hz bus speed not supported\n",
+ bus_speed);
+ dev_err(iproc_i2c->device,
+ "valid speeds are 100khz and 400khz\n");
+ return -EINVAL;
+ } else if (bus_speed < 400000) {
+ bus_speed = 100000;
+ speed_bit = 0;
+ } else {
+ bus_speed = 400000;
+ speed_bit = 1;
+ }
+
+ val = readl(iproc_i2c->base + TIM_CFG_OFFSET);
+ val &= ~(1 << TIM_CFG_MODE_400_SHIFT);
+ val |= speed_bit << TIM_CFG_MODE_400_SHIFT;
+ writel(val, iproc_i2c->base + TIM_CFG_OFFSET);
+
+ dev_info(iproc_i2c->device, "bus set to %u Hz\n", bus_speed);
+
+ return 0;
+}
+
+static int bcm_iproc_i2c_init(struct bcm_iproc_i2c_dev *iproc_i2c)
+{
+ u32 val;
+
+ /* put controller in reset */
+ val = readl(iproc_i2c->base + CFG_OFFSET);
+ val |= 1 << CFG_RESET_SHIFT;
+ val &= ~(1 << CFG_EN_SHIFT);
+ writel(val, iproc_i2c->base + CFG_OFFSET);
+
+ /* wait 100 usec per spec */
+ udelay(100);
+
+ /* bring controller out of reset */
+ val &= ~(1 << CFG_RESET_SHIFT);
+ writel(val, iproc_i2c->base + CFG_OFFSET);
+
+ /* flush TX/RX FIFOs and set RX FIFO threshold to zero */
+ val = (1 << M_FIFO_RX_FLUSH_SHIFT) | (1 << M_FIFO_TX_FLUSH_SHIFT);
+ writel(val, iproc_i2c->base + M_FIFO_CTRL_OFFSET);
+
+ /* disable all interrupts */
+ writel(0, iproc_i2c->base + IE_OFFSET);
+
+ /* clear all pending interrupts */
+ writel(0xffffffff, iproc_i2c->base + IS_OFFSET);
+
+ return 0;
+}
+
+static void bcm_iproc_i2c_enable(struct bcm_iproc_i2c_dev *iproc_i2c)
+{
+ u32 val;
+
+ val = readl(iproc_i2c->base + CFG_OFFSET);
+ val |= 1 << CFG_EN_SHIFT;
+ writel(val, iproc_i2c->base + CFG_OFFSET);
+}
+
+static void bcm_iproc_i2c_disable(struct bcm_iproc_i2c_dev *iproc_i2c)
+{
+ u32 val;
+
+ val = readl(iproc_i2c->base + CFG_OFFSET);
+ val &= ~(1 << CFG_EN_SHIFT);
+ writel(val, iproc_i2c->base + CFG_OFFSET);
+}
+
+static int bcm_iproc_i2c_probe(struct platform_device *pdev)
+{
+ int irq, ret = 0;
+ struct bcm_iproc_i2c_dev *iproc_i2c;
+ struct i2c_adapter *adap;
+ struct resource *res;
+
+ iproc_i2c = devm_kzalloc(&pdev->dev, sizeof(*iproc_i2c),
+ GFP_KERNEL);
+ if (!iproc_i2c)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, iproc_i2c);
+ iproc_i2c->device = &pdev->dev;
+ init_completion(&iproc_i2c->done);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ iproc_i2c->base = devm_ioremap_resource(iproc_i2c->device, res);
+ if (IS_ERR(iproc_i2c->base))
+ return PTR_ERR(iproc_i2c->base);
+
+ ret = bcm_iproc_i2c_init(iproc_i2c);
+ if (ret)
+ return ret;
+
+ ret = bcm_iproc_i2c_cfg_speed(iproc_i2c);
+ if (ret)
+ return ret;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq <= 0) {
+ dev_err(iproc_i2c->device, "no irq resource\n");
+ return irq;
+ }
+ iproc_i2c->irq = irq;
+
+ ret = devm_request_irq(iproc_i2c->device, irq, bcm_iproc_i2c_isr, 0,
+ pdev->name, iproc_i2c);
+ if (ret < 0) {
+ dev_err(iproc_i2c->device, "unable to request irq %i\n", irq);
+ return ret;
+ }
+
+ bcm_iproc_i2c_enable(iproc_i2c);
+
+ adap = &iproc_i2c->adapter;
+ i2c_set_adapdata(adap, iproc_i2c);
+ strlcpy(adap->name, "Broadcom iProc I2C adapter", sizeof(adap->name));
+ adap->algo = &bcm_iproc_algo;
+ adap->dev.parent = &pdev->dev;
+ adap->dev.of_node = pdev->dev.of_node;
+
+ ret = i2c_add_adapter(adap);
+ if (ret) {
+ dev_err(iproc_i2c->device, "failed to add adapter\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int bcm_iproc_i2c_remove(struct platform_device *pdev)
+{
+ struct bcm_iproc_i2c_dev *iproc_i2c = platform_get_drvdata(pdev);
+
+ /* make sure there's no pending interrupt when we remove the adapter */
+ writel(0, iproc_i2c->base + IE_OFFSET);
+ readl(iproc_i2c->base + IE_OFFSET);
+ synchronize_irq(iproc_i2c->irq);
+
+ i2c_del_adapter(&iproc_i2c->adapter);
+ bcm_iproc_i2c_disable(iproc_i2c);
+
+ return 0;
+}
+
+static const struct of_device_id bcm_iproc_i2c_of_match[] = {
+ { .compatible = "brcm,iproc-i2c" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, bcm_iproc_i2c_of_match);
+
+static struct platform_driver bcm_iproc_i2c_driver = {
+ .driver = {
+ .name = "bcm-iproc-i2c",
+ .of_match_table = bcm_iproc_i2c_of_match,
+ },
+ .probe = bcm_iproc_i2c_probe,
+ .remove = bcm_iproc_i2c_remove,
+};
+module_platform_driver(bcm_iproc_i2c_driver);
+
+MODULE_AUTHOR("Ray Jui <[email protected]>");
+MODULE_DESCRIPTION("Broadcom iProc I2C Driver");
+MODULE_LICENSE("GPL v2");
--
1.7.9.5
Add I2C device nodes and its properties in bcm-cygnus.dtsi but keep
them disabled there. Individual I2C devices can be enabled in board
specific dts file when I2C slave devices are enabled in the future
Signed-off-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
Reviewed-by: Kevin Cernekee <[email protected]>
---
arch/arm/boot/dts/bcm-cygnus.dtsi | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi b/arch/arm/boot/dts/bcm-cygnus.dtsi
index 5126f9e..ff5fb6a 100644
--- a/arch/arm/boot/dts/bcm-cygnus.dtsi
+++ b/arch/arm/boot/dts/bcm-cygnus.dtsi
@@ -70,6 +70,26 @@
};
};
+ i2c0: i2c@18008000 {
+ compatible = "brcm,cygnus-iproc-i2c", "brcm,iproc-i2c";
+ reg = <0x18008000 0x100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupts = <GIC_SPI 85 IRQ_TYPE_NONE>;
+ clock-frequency = <100000>;
+ status = "disabled";
+ };
+
+ i2c1: i2c@1800b000 {
+ compatible = "brcm,cygnus-iproc-i2c", "brcm,iproc-i2c";
+ reg = <0x1800b000 0x100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupts = <GIC_SPI 86 IRQ_TYPE_NONE>;
+ clock-frequency = <100000>;
+ status = "disabled";
+ };
+
uart0: serial@18020000 {
compatible = "snps,dw-apb-uart";
reg = <0x18020000 0x100>;
--
1.7.9.5
Hi Ray,
On Fri, Feb 06, 2015 at 05:28:26PM -0800, Ray Jui wrote:
> Add initial support to the Broadcom iProc I2C controller found in the
> iProc family of SoCs.
>
> The iProc I2C controller has separate internal TX and RX FIFOs, each has
> a size of 64 bytes. The iProc I2C controller supports two bus speeds
> including standard mode (100kHz) and fast mode (400kHz)
Mostly looking good.
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
Please sort the includes.
> +static bool bcm_iproc_i2c_bus_busy(struct bcm_iproc_i2c_dev *iproc_i2c)
> +{
> + if (readl(iproc_i2c->base + M_CMD_OFFSET) &
> + (1 << M_CMD_START_BUSY_SHIFT))
> + return true;
> + else
> + return false;
> +}
Minor: return !!(readl(...))? You decide.
> +
> +static int bcm_iproc_i2c_format_addr(struct bcm_iproc_i2c_dev *iproc_i2c,
> + struct i2c_msg *msg, u8 *addr)
> +{
> + *addr = msg->addr << 1;
> +
> + if (msg->flags & I2C_M_RD)
> + *addr |= 1;
> +
> + return 0;
> +}
I'd suggest a oneliner.
*addr = msg->addr << 1 | (msg->flags & I2C_M_RD ? 1 : 0)
Or use !! like above.
Don't do an extra function for that. It is only used once and it also
doesn't need to be int since it can't fail anyhow.
(Note to self: I should make a macro for that in i2c.h)
> + /* need to reserve one byte in the FIFO for the slave address */
> + if (msg->len > M_TX_RX_FIFO_SIZE - 1) {
> + dev_err(iproc_i2c->device,
> + "only support data length up to %u bytes\n",
> + M_TX_RX_FIFO_SIZE - 1);
> + return -EINVAL;
-EOPNOTSUPP
Is it really a HW limitation? Could the driver later be extended to
continue filling the FIFO if a certain threshold is reached?
> + dev_dbg(iproc_i2c->device, "xfer %c, addr=0x%02x, len=%d\n",
> + (msg->flags & I2C_M_RD) ? 'R' : 'W', msg->addr,
> + msg->len);
> + dev_dbg(iproc_i2c->device, "*** data: %*ph\n", msg->len, msg->buf);
Not really needed. We have tracing for that.
> + if (bus_speed < 100000) {
> + dev_err(iproc_i2c->device, "%d Hz bus speed not supported\n",
> + bus_speed);
> + dev_err(iproc_i2c->device,
> + "valid speeds are 100khz and 400khz\n");
> + return -EINVAL;
> + } else if (bus_speed < 400000) {
> + bus_speed = 100000;
> + speed_bit = 0;
> + } else {
> + bus_speed = 400000;
> + speed_bit = 1;
> + }
> +
> + val = readl(iproc_i2c->base + TIM_CFG_OFFSET);
> + val &= ~(1 << TIM_CFG_MODE_400_SHIFT);
> + val |= speed_bit << TIM_CFG_MODE_400_SHIFT;
val |= (bus_speed == 400000) ...
and skip speed_bit? You decide.
> +static void bcm_iproc_i2c_enable(struct bcm_iproc_i2c_dev *iproc_i2c)
> +{
> + u32 val;
> +
> + val = readl(iproc_i2c->base + CFG_OFFSET);
> + val |= 1 << CFG_EN_SHIFT;
> + writel(val, iproc_i2c->base + CFG_OFFSET);
> +}
> +
> +static void bcm_iproc_i2c_disable(struct bcm_iproc_i2c_dev *iproc_i2c)
> +{
> + u32 val;
> +
> + val = readl(iproc_i2c->base + CFG_OFFSET);
> + val &= ~(1 << CFG_EN_SHIFT);
> + writel(val, iproc_i2c->base + CFG_OFFSET);
> +}
Extra functions? They are self explaining and only used once. You
decide.
Rest looks fine, thanks!
On 2/7/2015 9:50 AM, Wolfram Sang wrote:
> Hi Ray,
>
> On Fri, Feb 06, 2015 at 05:28:26PM -0800, Ray Jui wrote:
>> Add initial support to the Broadcom iProc I2C controller found in the
>> iProc family of SoCs.
>>
>> The iProc I2C controller has separate internal TX and RX FIFOs, each has
>> a size of 64 bytes. The iProc I2C controller supports two bus speeds
>> including standard mode (100kHz) and fast mode (400kHz)
>
> Mostly looking good.
>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/i2c.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/io.h>
>> +#include <linux/slab.h>
>> +#include <linux/delay.h>
>
> Please sort the includes.
>
Will do.
>> +static bool bcm_iproc_i2c_bus_busy(struct bcm_iproc_i2c_dev *iproc_i2c)
>> +{
>> + if (readl(iproc_i2c->base + M_CMD_OFFSET) &
>> + (1 << M_CMD_START_BUSY_SHIFT))
>> + return true;
>> + else
>> + return false;
>> +}
>
> Minor: return !!(readl(...))? You decide.
>
Okay will do that. Will also remove this function since now it becomes
one line and is used only once.
>> +
>> +static int bcm_iproc_i2c_format_addr(struct bcm_iproc_i2c_dev *iproc_i2c,
>> + struct i2c_msg *msg, u8 *addr)
>> +{
>> + *addr = msg->addr << 1;
>> +
>> + if (msg->flags & I2C_M_RD)
>> + *addr |= 1;
>> +
>> + return 0;
>> +}
>
> I'd suggest a oneliner.
>
> *addr = msg->addr << 1 | (msg->flags & I2C_M_RD ? 1 : 0)
>
> Or use !! like above.
>
> Don't do an extra function for that. It is only used once and it also
> doesn't need to be int since it can't fail anyhow.
>
> (Note to self: I should make a macro for that in i2c.h)
>
Yes will change. Thanks.
>> + /* need to reserve one byte in the FIFO for the slave address */
>> + if (msg->len > M_TX_RX_FIFO_SIZE - 1) {
>> + dev_err(iproc_i2c->device,
>> + "only support data length up to %u bytes\n",
>> + M_TX_RX_FIFO_SIZE - 1);
>> + return -EINVAL;
>
> -EOPNOTSUPP
>
> Is it really a HW limitation? Could the driver later be extended to
> continue filling the FIFO if a certain threshold is reached?
>
Will return -EOPNOTSUPP. This really depends on whether or not we expect
one sequence of START + SLV ADDR + DATA + STOP per i2c message. I can
later extend the driver to refill/re-drain the FIFO for data size >= 64
bytes, if one sequence of SATRT...STOP per message is not a requirement.
>> + dev_dbg(iproc_i2c->device, "xfer %c, addr=0x%02x, len=%d\n",
>> + (msg->flags & I2C_M_RD) ? 'R' : 'W', msg->addr,
>> + msg->len);
>> + dev_dbg(iproc_i2c->device, "*** data: %*ph\n", msg->len, msg->buf);
>
> Not really needed. We have tracing for that.
>
Will remove.
>> + if (bus_speed < 100000) {
>> + dev_err(iproc_i2c->device, "%d Hz bus speed not supported\n",
>> + bus_speed);
>> + dev_err(iproc_i2c->device,
>> + "valid speeds are 100khz and 400khz\n");
>> + return -EINVAL;
>> + } else if (bus_speed < 400000) {
>> + bus_speed = 100000;
>> + speed_bit = 0;
>> + } else {
>> + bus_speed = 400000;
>> + speed_bit = 1;
>> + }
>> +
>> + val = readl(iproc_i2c->base + TIM_CFG_OFFSET);
>> + val &= ~(1 << TIM_CFG_MODE_400_SHIFT);
>> + val |= speed_bit << TIM_CFG_MODE_400_SHIFT;
>
> val |= (bus_speed == 400000) ...
>
> and skip speed_bit? You decide.
>
Okay, I'll get rid of speed_bit.
>> +static void bcm_iproc_i2c_enable(struct bcm_iproc_i2c_dev *iproc_i2c)
>> +{
>> + u32 val;
>> +
>> + val = readl(iproc_i2c->base + CFG_OFFSET);
>> + val |= 1 << CFG_EN_SHIFT;
>> + writel(val, iproc_i2c->base + CFG_OFFSET);
>> +}
>> +
>> +static void bcm_iproc_i2c_disable(struct bcm_iproc_i2c_dev *iproc_i2c)
>> +{
>> + u32 val;
>> +
>> + val = readl(iproc_i2c->base + CFG_OFFSET);
>> + val &= ~(1 << CFG_EN_SHIFT);
>> + writel(val, iproc_i2c->base + CFG_OFFSET);
>> +}
>
> Extra functions? They are self explaining and only used once. You
> decide.
In fact I'll keep the function, since it will likely be needed later
when we add suspend/resume support to the driver. But I'll combine the
two functions and make it a single function called
bcm_iproc_i2c_enable_disable.
>
> Rest looks fine, thanks!
>
Thanks for the review!
Ray
> > Is it really a HW limitation? Could the driver later be extended to
> > continue filling the FIFO if a certain threshold is reached?
> >
>
> Will return -EOPNOTSUPP. This really depends on whether or not we expect
> one sequence of START + SLV ADDR + DATA + STOP per i2c message. I can
> later extend the driver to refill/re-drain the FIFO for data size >= 64
> bytes, if one sequence of SATRT...STOP per message is not a requirement.
It is important to have the terminology clear here: One transfer can
consist of multiple messages. The transfer uses START/STOP at the
beginning/end, the messages within the transfer only REPEATED_START.
On 2/8/2015 3:03 AM, Wolfram Sang wrote:
>
>>> Is it really a HW limitation? Could the driver later be extended to
>>> continue filling the FIFO if a certain threshold is reached?
>>>
>>
>> Will return -EOPNOTSUPP. This really depends on whether or not we expect
>> one sequence of START + SLV ADDR + DATA + STOP per i2c message. I can
>> later extend the driver to refill/re-drain the FIFO for data size >= 64
>> bytes, if one sequence of SATRT...STOP per message is not a requirement.
>
> It is important to have the terminology clear here: One transfer can
> consist of multiple messages. The transfer uses START/STOP at the
> beginning/end, the messages within the transfer only REPEATED_START.
>
Okay. Let me check with our ASIC engineer to see if there's a way to get
the driver extended to support the case when data size is larger than
the FIFO size. From my understanding based on the data sheet I have, I
don't think that can be done with this controller. But if the ASIC
engineers tells me the opposite, I'll add it as a separate patch later.
Thanks,
Ray
> Okay. Let me check with our ASIC engineer to see if there's a way to get
> the driver extended to support the case when data size is larger than
> the FIFO size. From my understanding based on the data sheet I have, I
> don't think that can be done with this controller. But if the ASIC
> engineers tells me the opposite, I'll add it as a separate patch later.
Perfect, thanks!