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)
Ray Jui (4):
i2c: iProc: define Broadcom iProc I2C binding
i2c: iproc: Add Broadcom iProc I2C Driver
ARM: mach-bcm: Enable I2C support for iProc
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 +
arch/arm/mach-bcm/Kconfig | 1 +
drivers/i2c/busses/Kconfig | 9 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-bcm-iproc.c | 503 ++++++++++++++++++++
6 files changed, 571 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]>
---
.../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
Enable I2C driver support for Broadcom iProc family of SoCs by
selecting I2C_BCM_IPROC
Signed-off-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
---
arch/arm/mach-bcm/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index aaeec78..86ee90b 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -19,6 +19,7 @@ config ARCH_BCM_IPROC
select ARCH_REQUIRE_GPIOLIB
select ARM_AMBA
select PINCTRL
+ select I2C_BCM_IPROC
help
This enables support for systems based on Broadcom IPROC architected SoCs.
The IPROC complex contains one or more ARM CPUs along with common
--
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]>
---
drivers/i2c/busses/Kconfig | 9 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-bcm-iproc.c | 503 ++++++++++++++++++++++++++++++++++++
3 files changed, 513 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-bcm-iproc.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index c1351d9..8a2eb7e 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -372,6 +372,15 @@ 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
+ 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 5e6c822..216e7be 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..0e6e603
--- /dev/null
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -0,0 +1,503 @@
+/*
+ * 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/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/clk.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 TIME_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;
+
+ void __iomem *base;
+ struct i2c_msg *msg;
+
+ struct i2c_adapter adapter;
+
+ struct completion 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 *dev = data;
+ u32 status = readl(dev->base + IS_OFFSET);
+
+ status &= ISR_MASK;
+
+ if (!status)
+ return IRQ_NONE;
+
+ writel(status, dev->base + IS_OFFSET);
+ complete_all(&dev->done);
+
+ return IRQ_HANDLED;
+}
+
+static int __wait_for_bus_idle(struct bcm_iproc_i2c_dev *dev)
+{
+ unsigned long timeout = jiffies + msecs_to_jiffies(I2C_TIMEOUT_MESC);
+
+ while (readl(dev->base + M_CMD_OFFSET) &
+ (1 << M_CMD_START_BUSY_SHIFT)) {
+ if (time_after(jiffies, timeout)) {
+ dev_err(dev->device, "wait for bus idle timeout\n");
+ return -ETIMEDOUT;
+ }
+ }
+
+ return 0;
+}
+
+static int bcm_iproc_i2c_format_addr(struct bcm_iproc_i2c_dev *dev,
+ struct i2c_msg *msg, u8 *addr)
+{
+
+ if (msg->flags & I2C_M_TEN) {
+ dev_err(dev->device, "no support for 10-bit address\n");
+ return -EINVAL;
+ }
+
+ *addr = (msg->addr << 1) & 0xfe;
+
+ if (msg->flags & I2C_M_RD)
+ *addr |= 1;
+
+ return 0;
+}
+
+static int bcm_iproc_i2c_check_status(struct bcm_iproc_i2c_dev *dev)
+{
+ u32 val;
+
+ val = readl(dev->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_err(dev->device, "lost bus arbitration\n");
+ return -EREMOTEIO;
+
+ case M_CMD_STATUS_NACK_ADDR:
+ dev_err(dev->device, "NAK addr:0x%02x\n", dev->msg->addr);
+ return -EREMOTEIO;
+
+ case M_CMD_STATUS_NACK_DATA:
+ dev_err(dev->device, "NAK data\n");
+ return -EREMOTEIO;
+
+ case M_CMD_STATUS_TIMEOUT:
+ dev_err(dev->device, "bus timeout\n");
+ return -ETIMEDOUT;
+
+ default:
+ dev_err(dev->device, "unknown error code=%d\n", val);
+ return -EREMOTEIO;
+ }
+
+ return -EREMOTEIO;
+}
+
+static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *dev,
+ struct i2c_msg *msg)
+{
+ int ret, i;
+ u8 addr;
+ u32 val;
+ unsigned long time_left = msecs_to_jiffies(I2C_TIMEOUT_MESC);
+
+ if (msg->len < 1 || msg->len > M_TX_RX_FIFO_SIZE - 1) {
+ dev_err(dev->device,
+ "supported data length is 1 - %u bytes\n",
+ M_TX_RX_FIFO_SIZE - 1);
+ return -EINVAL;
+ }
+
+ dev->msg = msg;
+ ret = __wait_for_bus_idle(dev);
+ if (ret)
+ return ret;
+
+ ret = bcm_iproc_i2c_format_addr(dev, msg, &addr);
+ if (ret)
+ return ret;
+
+ /* load slave address into the TX FIFO */
+ writel(addr, dev->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, dev->base + M_TX_OFFSET);
+ }
+ }
+
+ /* mark as incomplete before starting the transaction */
+ reinit_completion(&dev->done);
+
+ /*
+ * Enable the "start busy" interrupt, which will be triggered after
+ * the transaction is done
+ */
+ writel(1 << IE_M_START_BUSY_SHIFT, dev->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, dev->base + M_CMD_OFFSET);
+
+ time_left = wait_for_completion_timeout(&dev->done, time_left);
+
+ /* disable all interrupts */
+ writel(0, dev->base + IE_OFFSET);
+
+ if (!time_left) {
+ dev_err(dev->device, "transaction times out\n");
+
+ /* flush FIFOs */
+ val = (1 << M_FIFO_RX_FLUSH_SHIFT) |
+ (1 << M_FIFO_TX_FLUSH_SHIFT);
+ writel(val, dev->base + M_FIFO_CTRL_OFFSET);
+ return -EREMOTEIO;
+ }
+
+ ret = bcm_iproc_i2c_check_status(dev);
+ if (ret) {
+ /* flush both TX/RX FIFOs */
+ val = (1 << M_FIFO_RX_FLUSH_SHIFT) |
+ (1 << M_FIFO_TX_FLUSH_SHIFT);
+ writel(val, dev->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(dev->base + M_RX_OFFSET) >>
+ M_RX_DATA_SHIFT) & M_RX_DATA_MASK;
+ }
+ }
+
+ dev_dbg(dev->device, "xfer %c, addr=0x%02x, len=%d\n",
+ (msg->flags & I2C_M_RD) ? 'R' : 'W', msg->addr,
+ msg->len);
+ dev_dbg(dev->device, "**** data start ****\n");
+ for (i = 0; i < msg->len; i++)
+ dev_dbg(dev->device, "0x%02x ", msg->buf[i]);
+ dev_dbg(dev->device, "**** data end ****\n");
+
+ return 0;
+}
+
+static int bcm_iproc_i2c_xfer(struct i2c_adapter *adapter,
+ struct i2c_msg msgs[], int num)
+{
+ struct bcm_iproc_i2c_dev *dev = i2c_get_adapdata(adapter);
+ int ret, i;
+
+ /* go through all messages */
+ for (i = 0; i < num; i++) {
+ ret = bcm_iproc_i2c_xfer_single_msg(dev, &msgs[i]);
+ if (ret) {
+ dev_err(dev->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 *dev)
+{
+ unsigned int bus_speed, speed_bit;
+ u32 val;
+ int ret = of_property_read_u32(dev->device->of_node, "clock-frequency",
+ &bus_speed);
+ if (ret < 0) {
+ dev_err(dev->device, "missing clock-frequency property\n");
+ return -ENODEV;
+ }
+
+ switch (bus_speed) {
+ case 100000:
+ speed_bit = 0;
+ break;
+ case 400000:
+ speed_bit = 1;
+ break;
+ default:
+ dev_err(dev->device, "%d Hz bus speed not supported\n",
+ bus_speed);
+ dev_err(dev->device, "valid speeds are 100khz and 400khz\n");
+ return -EINVAL;
+ }
+
+ val = readl(dev->base + TIM_CFG_OFFSET);
+ val &= ~(1 << TIME_CFG_MODE_400_SHIFT);
+ val |= speed_bit << TIME_CFG_MODE_400_SHIFT;
+ writel(val, dev->base + TIM_CFG_OFFSET);
+
+ dev_info(dev->device, "bus set to %u Hz\n", bus_speed);
+
+ return 0;
+}
+
+static int bcm_iproc_i2c_init(struct bcm_iproc_i2c_dev *dev)
+{
+ u32 val;
+
+ /* put controller in reset */
+ val = readl(dev->base + CFG_OFFSET);
+ val |= 1 << CFG_RESET_SHIFT;
+ val &= ~(1 << CFG_EN_SHIFT);
+ writel(val, dev->base + CFG_OFFSET);
+
+ /* wait 100 usec per spec */
+ udelay(100);
+
+ /* bring controller out of reset */
+ val = readl(dev->base + CFG_OFFSET);
+ val &= ~(1 << CFG_RESET_SHIFT);
+ writel(val, dev->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, dev->base + M_FIFO_CTRL_OFFSET);
+
+ /* disable all interrupts */
+ val = 0;
+ writel(val, dev->base + IE_OFFSET);
+
+ /* clear all pending interrupts */
+ val = readl(dev->base + IS_OFFSET);
+ writel(val, dev->base + IS_OFFSET);
+
+ return 0;
+}
+
+static void bcm_iproc_i2c_enable(struct bcm_iproc_i2c_dev *dev)
+{
+ u32 val;
+
+ val = readl(dev->base + CFG_OFFSET);
+ val |= 1 << CFG_EN_SHIFT;
+ writel(val, dev->base + CFG_OFFSET);
+}
+
+static void bcm_iproc_i2c_disable(struct bcm_iproc_i2c_dev *dev)
+{
+ u32 val;
+
+ val = readl(dev->base + CFG_OFFSET);
+ val &= ~(1 << CFG_EN_SHIFT);
+ writel(val, dev->base + CFG_OFFSET);
+}
+
+static int bcm_iproc_i2c_probe(struct platform_device *pdev)
+{
+ int irq, ret = 0;
+ struct bcm_iproc_i2c_dev *dev;
+ struct i2c_adapter *adap;
+ struct resource *res;
+
+ dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, dev);
+ dev->device = &pdev->dev;
+ init_completion(&dev->done);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;
+ dev->base = devm_ioremap_resource(dev->device, res);
+ if (IS_ERR(dev->base))
+ return -ENOMEM;
+
+ ret = bcm_iproc_i2c_init(dev);
+ if (ret)
+ return ret;
+
+ ret = bcm_iproc_i2c_cfg_speed(dev);
+ if (ret)
+ return ret;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(dev->device, "no irq resource\n");
+ return irq;
+ }
+
+ ret = devm_request_irq(&pdev->dev, irq, bcm_iproc_i2c_isr,
+ IRQF_SHARED, pdev->name, dev);
+ if (ret) {
+ dev_err(dev->device, "unable to request irq %i\n", irq);
+ return ret;
+ }
+
+ bcm_iproc_i2c_enable(dev);
+
+ adap = &dev->adapter;
+ i2c_set_adapdata(adap, dev);
+ 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(dev->device, "failed to add adapter\n");
+ return ret;
+ }
+
+ dev_info(dev->device, "device registered successfully\n");
+
+ return 0;
+}
+
+static int bcm_iproc_i2c_remove(struct platform_device *pdev)
+{
+ struct bcm_iproc_i2c_dev *dev = platform_get_drvdata(pdev);
+
+ i2c_del_adapter(&dev->adapter);
+ bcm_iproc_i2c_disable(dev);
+
+ return 0;
+}
+
+static const struct of_device_id bcm_iproc_i2c_of_match[] = {
+ {.compatible = "brcm,iproc-i2c",},
+ {},
+};
+MODULE_DEVICE_TABLE(of, bcm_iproc_i2c_of_match);
+
+static struct platform_driver bcm_iproc_i2c_driver = {
+ .driver = {
+ .name = "bcm-iproc-i2c",
+ .owner = THIS_MODULE,
+ .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]>
---
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..f7d6c1d 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,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,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,
On Wednesday 10 December 2014 06:24 AM, Ray Jui wrote:
> 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]>
> ---
> .../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
> +
All the properties defined with two lines of statements.
Why cant they be with single line statement, like:
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
....
--
Thanks and Regards,
Varka Bhadram.
On Wednesday 10 December 2014 06:24 AM, 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)
>
> Signed-off-by: Ray Jui <[email protected]>
> Reviewed-by: Scott Branden <[email protected]>
> ---
> drivers/i2c/busses/Kconfig | 9 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-bcm-iproc.c | 503 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 513 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-bcm-iproc.c
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index c1351d9..8a2eb7e 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -372,6 +372,15 @@ 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
> + 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 5e6c822..216e7be 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..0e6e603
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> @@ -0,0 +1,503 @@
> +/*
> + * 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/device.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/sched.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.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 TIME_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;
> +
> + void __iomem *base;
> + struct i2c_msg *msg;
> +
> + struct i2c_adapter adapter;
> +
> + struct completion 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 *dev = data;
> + u32 status = readl(dev->base + IS_OFFSET);
> +
> + status &= ISR_MASK;
> +
> + if (!status)
> + return IRQ_NONE;
> +
> + writel(status, dev->base + IS_OFFSET);
> + complete_all(&dev->done);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int __wait_for_bus_idle(struct bcm_iproc_i2c_dev *dev)
> +{
> + unsigned long timeout = jiffies + msecs_to_jiffies(I2C_TIMEOUT_MESC);
> +
> + while (readl(dev->base + M_CMD_OFFSET) &
> + (1 << M_CMD_START_BUSY_SHIFT)) {
> + if (time_after(jiffies, timeout)) {
> + dev_err(dev->device, "wait for bus idle timeout\n");
> + return -ETIMEDOUT;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int bcm_iproc_i2c_format_addr(struct bcm_iproc_i2c_dev *dev,
> + struct i2c_msg *msg, u8 *addr)
> +{
Match open parenthesis..
static int bcm_iproc_i2c_format_addr(struct bcm_iproc_i2c_dev *dev,
struct i2c_msg *msg, u8 *addr)
> +
> + if (msg->flags & I2C_M_TEN) {
> + dev_err(dev->device, "no support for 10-bit address\n");
> + return -EINVAL;
> + }
> +
> + *addr = (msg->addr << 1) & 0xfe;
> +
> + if (msg->flags & I2C_M_RD)
> + *addr |= 1;
> +
> + return 0;
> +}
> +
> +static int bcm_iproc_i2c_check_status(struct bcm_iproc_i2c_dev *dev)
> +{
> + u32 val;
> +
> + val = readl(dev->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_err(dev->device, "lost bus arbitration\n");
> + return -EREMOTEIO;
> +
> + case M_CMD_STATUS_NACK_ADDR:
> + dev_err(dev->device, "NAK addr:0x%02x\n", dev->msg->addr);
> + return -EREMOTEIO;
> +
> + case M_CMD_STATUS_NACK_DATA:
> + dev_err(dev->device, "NAK data\n");
> + return -EREMOTEIO;
> +
> + case M_CMD_STATUS_TIMEOUT:
> + dev_err(dev->device, "bus timeout\n");
> + return -ETIMEDOUT;
> +
> + default:
> + dev_err(dev->device, "unknown error code=%d\n", val);
> + return -EREMOTEIO;
> + }
> +
> + return -EREMOTEIO;
> +}
> +
> +static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *dev,
> + struct i2c_msg *msg)
> +{
dto...
> + int ret, i;
> + u8 addr;
> + u32 val;
> + unsigned long time_left = msecs_to_jiffies(I2C_TIMEOUT_MESC);
> +
> + if (msg->len < 1 || msg->len > M_TX_RX_FIFO_SIZE - 1) {
> + dev_err(dev->device,
> + "supported data length is 1 - %u bytes\n",
> + M_TX_RX_FIFO_SIZE - 1);
> + return -EINVAL;
> + }
> +
> + dev->msg = msg;
> + ret = __wait_for_bus_idle(dev);
> + if (ret)
> + return ret;
> +
> + ret = bcm_iproc_i2c_format_addr(dev, msg, &addr);
> + if (ret)
> + return ret;
> +
> + /* load slave address into the TX FIFO */
> + writel(addr, dev->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, dev->base + M_TX_OFFSET);
> + }
> + }
> +
> + /* mark as incomplete before starting the transaction */
> + reinit_completion(&dev->done);
> +
> + /*
> + * Enable the "start busy" interrupt, which will be triggered after
> + * the transaction is done
> + */
> + writel(1 << IE_M_START_BUSY_SHIFT, dev->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, dev->base + M_CMD_OFFSET);
> +
> + time_left = wait_for_completion_timeout(&dev->done, time_left);
> +
> + /* disable all interrupts */
> + writel(0, dev->base + IE_OFFSET);
> +
> + if (!time_left) {
> + dev_err(dev->device, "transaction times out\n");
> +
> + /* flush FIFOs */
> + val = (1 << M_FIFO_RX_FLUSH_SHIFT) |
> + (1 << M_FIFO_TX_FLUSH_SHIFT);
> + writel(val, dev->base + M_FIFO_CTRL_OFFSET);
> + return -EREMOTEIO;
> + }
> +
> + ret = bcm_iproc_i2c_check_status(dev);
> + if (ret) {
> + /* flush both TX/RX FIFOs */
> + val = (1 << M_FIFO_RX_FLUSH_SHIFT) |
> + (1 << M_FIFO_TX_FLUSH_SHIFT);
> + writel(val, dev->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(dev->base + M_RX_OFFSET) >>
> + M_RX_DATA_SHIFT) & M_RX_DATA_MASK;
> + }
> + }
> +
> + dev_dbg(dev->device, "xfer %c, addr=0x%02x, len=%d\n",
> + (msg->flags & I2C_M_RD) ? 'R' : 'W', msg->addr,
> + msg->len);
> + dev_dbg(dev->device, "**** data start ****\n");
> + for (i = 0; i < msg->len; i++)
> + dev_dbg(dev->device, "0x%02x ", msg->buf[i]);
> + dev_dbg(dev->device, "**** data end ****\n");
> +
> + return 0;
> +}
> +
> +static int bcm_iproc_i2c_xfer(struct i2c_adapter *adapter,
> + struct i2c_msg msgs[], int num)
> +{
> + struct bcm_iproc_i2c_dev *dev = i2c_get_adapdata(adapter);
> + int ret, i;
> +
> + /* go through all messages */
> + for (i = 0; i < num; i++) {
> + ret = bcm_iproc_i2c_xfer_single_msg(dev, &msgs[i]);
> + if (ret) {
> + dev_err(dev->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 *dev)
> +{
> + unsigned int bus_speed, speed_bit;
> + u32 val;
> + int ret = of_property_read_u32(dev->device->of_node, "clock-frequency",
> + &bus_speed);
> + if (ret < 0) {
> + dev_err(dev->device, "missing clock-frequency property\n");
> + return -ENODEV;
> + }
> +
> + switch (bus_speed) {
> + case 100000:
> + speed_bit = 0;
> + break;
> + case 400000:
> + speed_bit = 1;
> + break;
> + default:
> + dev_err(dev->device, "%d Hz bus speed not supported\n",
> + bus_speed);
> + dev_err(dev->device, "valid speeds are 100khz and 400khz\n");
> + return -EINVAL;
> + }
> +
> + val = readl(dev->base + TIM_CFG_OFFSET);
> + val &= ~(1 << TIME_CFG_MODE_400_SHIFT);
> + val |= speed_bit << TIME_CFG_MODE_400_SHIFT;
> + writel(val, dev->base + TIM_CFG_OFFSET);
> +
> + dev_info(dev->device, "bus set to %u Hz\n", bus_speed);
> +
> + return 0;
> +}
> +
> +static int bcm_iproc_i2c_init(struct bcm_iproc_i2c_dev *dev)
> +{
> + u32 val;
> +
> + /* put controller in reset */
> + val = readl(dev->base + CFG_OFFSET);
> + val |= 1 << CFG_RESET_SHIFT;
> + val &= ~(1 << CFG_EN_SHIFT);
> + writel(val, dev->base + CFG_OFFSET);
> +
> + /* wait 100 usec per spec */
> + udelay(100);
> +
> + /* bring controller out of reset */
> + val = readl(dev->base + CFG_OFFSET);
> + val &= ~(1 << CFG_RESET_SHIFT);
> + writel(val, dev->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, dev->base + M_FIFO_CTRL_OFFSET);
> +
> + /* disable all interrupts */
> + val = 0;
> + writel(val, dev->base + IE_OFFSET);
> +
> + /* clear all pending interrupts */
> + val = readl(dev->base + IS_OFFSET);
> + writel(val, dev->base + IS_OFFSET);
> +
> + return 0;
> +}
> +
> +static void bcm_iproc_i2c_enable(struct bcm_iproc_i2c_dev *dev)
> +{
> + u32 val;
> +
> + val = readl(dev->base + CFG_OFFSET);
> + val |= 1 << CFG_EN_SHIFT;
> + writel(val, dev->base + CFG_OFFSET);
> +}
> +
> +static void bcm_iproc_i2c_disable(struct bcm_iproc_i2c_dev *dev)
> +{
> + u32 val;
> +
> + val = readl(dev->base + CFG_OFFSET);
> + val &= ~(1 << CFG_EN_SHIFT);
> + writel(val, dev->base + CFG_OFFSET);
> +}
> +
> +static int bcm_iproc_i2c_probe(struct platform_device *pdev)
> +{
> + int irq, ret = 0;
> + struct bcm_iproc_i2c_dev *dev;
> + struct i2c_adapter *adap;
> + struct resource *res;
> +
> + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
> + if (!dev)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, dev);
> + dev->device = &pdev->dev;
> + init_completion(&dev->done);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENODEV;
We can remove this resource check. This checking will happen with devm_ioremap_resource()
> + dev->base = devm_ioremap_resource(dev->device, res);
> + if (IS_ERR(dev->base))
> + return -ENOMEM;
> +
> + ret = bcm_iproc_i2c_init(dev);
> + if (ret)
> + return ret;
> +
> + ret = bcm_iproc_i2c_cfg_speed(dev);
> + if (ret)
> + return ret;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(dev->device, "no irq resource\n");
> + return irq;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, irq, bcm_iproc_i2c_isr,
> + IRQF_SHARED, pdev->name, dev);
> + if (ret) {
> + dev_err(dev->device, "unable to request irq %i\n", irq);
> + return ret;
> + }
> +
> + bcm_iproc_i2c_enable(dev);
> +
> + adap = &dev->adapter;
> + i2c_set_adapdata(adap, dev);
> + 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(dev->device, "failed to add adapter\n");
> + return ret;
> + }
> +
> + dev_info(dev->device, "device registered successfully\n");
> +
> + return 0;
> +}
> +
> +static int bcm_iproc_i2c_remove(struct platform_device *pdev)
> +{
> + struct bcm_iproc_i2c_dev *dev = platform_get_drvdata(pdev);
> +
> + i2c_del_adapter(&dev->adapter);
> + bcm_iproc_i2c_disable(dev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id bcm_iproc_i2c_of_match[] = {
> + {.compatible = "brcm,iproc-i2c",},
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, bcm_iproc_i2c_of_match);
> +
> +static struct platform_driver bcm_iproc_i2c_driver = {
> + .driver = {
> + .name = "bcm-iproc-i2c",
> + .owner = THIS_MODULE,
No need to update this field. Its updated by module_platform_driver().
> + .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");
--
Thanks and Regards,
Varka Bhadram.
On 12/9/2014 5:27 PM, Varka Bhadram wrote:
> Hi,
>
> On Wednesday 10 December 2014 06:24 AM, Ray Jui wrote:
>> 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]>
>> ---
>> .../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
>> +
>
> All the properties defined with two lines of statements.
>
> Why cant they be with single line statement, like:
>
> 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
>
> ....
>
>
I thought making them two lines are more readable (and obviously that's
very subjective, :)). But more importantly, it matches the format of
other Broadcom iProc/Cygnus devicetree binding documents that are
currently in progress of upstreaming.
On 12/9/2014 5:33 PM, Varka Bhadram wrote:
>
> On Wednesday 10 December 2014 06:24 AM, 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)
>>
>> Signed-off-by: Ray Jui <[email protected]>
>> Reviewed-by: Scott Branden <[email protected]>
>> ---
>> drivers/i2c/busses/Kconfig | 9 +
>> drivers/i2c/busses/Makefile | 1 +
>> drivers/i2c/busses/i2c-bcm-iproc.c | 503
>> ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 513 insertions(+)
>> create mode 100644 drivers/i2c/busses/i2c-bcm-iproc.c
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index c1351d9..8a2eb7e 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -372,6 +372,15 @@ 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
>> + 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 5e6c822..216e7be 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..0e6e603
>> --- /dev/null
>> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
>> @@ -0,0 +1,503 @@
>> +/*
>> + * 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/device.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/sched.h>
>> +#include <linux/i2c.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/clk.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 TIME_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;
>> +
>> + void __iomem *base;
>> + struct i2c_msg *msg;
>> +
>> + struct i2c_adapter adapter;
>> +
>> + struct completion 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 *dev = data;
>> + u32 status = readl(dev->base + IS_OFFSET);
>> +
>> + status &= ISR_MASK;
>> +
>> + if (!status)
>> + return IRQ_NONE;
>> +
>> + writel(status, dev->base + IS_OFFSET);
>> + complete_all(&dev->done);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int __wait_for_bus_idle(struct bcm_iproc_i2c_dev *dev)
>> +{
>> + unsigned long timeout = jiffies +
>> msecs_to_jiffies(I2C_TIMEOUT_MESC);
>> +
>> + while (readl(dev->base + M_CMD_OFFSET) &
>> + (1 << M_CMD_START_BUSY_SHIFT)) {
>> + if (time_after(jiffies, timeout)) {
>> + dev_err(dev->device, "wait for bus idle timeout\n");
>> + return -ETIMEDOUT;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int bcm_iproc_i2c_format_addr(struct bcm_iproc_i2c_dev *dev,
>> + struct i2c_msg *msg, u8 *addr)
>> +{
>
> Match open parenthesis..
>
> static int bcm_iproc_i2c_format_addr(struct bcm_iproc_i2c_dev *dev,
> struct i2c_msg *msg, u8 *addr)
>
>
Okay I can make this change.
>> +
>> + if (msg->flags & I2C_M_TEN) {
>> + dev_err(dev->device, "no support for 10-bit address\n");
>> + return -EINVAL;
>> + }
>> +
>> + *addr = (msg->addr << 1) & 0xfe;
>> +
>> + if (msg->flags & I2C_M_RD)
>> + *addr |= 1;
>> +
>> + return 0;
>> +}
>> +
>> +static int bcm_iproc_i2c_check_status(struct bcm_iproc_i2c_dev *dev)
>> +{
>> + u32 val;
>> +
>> + val = readl(dev->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_err(dev->device, "lost bus arbitration\n");
>> + return -EREMOTEIO;
>> +
>> + case M_CMD_STATUS_NACK_ADDR:
>> + dev_err(dev->device, "NAK addr:0x%02x\n", dev->msg->addr);
>> + return -EREMOTEIO;
>> +
>> + case M_CMD_STATUS_NACK_DATA:
>> + dev_err(dev->device, "NAK data\n");
>> + return -EREMOTEIO;
>> +
>> + case M_CMD_STATUS_TIMEOUT:
>> + dev_err(dev->device, "bus timeout\n");
>> + return -ETIMEDOUT;
>> +
>> + default:
>> + dev_err(dev->device, "unknown error code=%d\n", val);
>> + return -EREMOTEIO;
>> + }
>> +
>> + return -EREMOTEIO;
>> +}
>> +
>> +static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *dev,
>> + struct i2c_msg *msg)
>> +{
>
> dto...
>
One more indent? Sure.
>> + int ret, i;
>> + u8 addr;
>> + u32 val;
>> + unsigned long time_left = msecs_to_jiffies(I2C_TIMEOUT_MESC);
>> +
>> + if (msg->len < 1 || msg->len > M_TX_RX_FIFO_SIZE - 1) {
>> + dev_err(dev->device,
>> + "supported data length is 1 - %u bytes\n",
>> + M_TX_RX_FIFO_SIZE - 1);
>> + return -EINVAL;
>> + }
>> +
>> + dev->msg = msg;
>> + ret = __wait_for_bus_idle(dev);
>> + if (ret)
>> + return ret;
>> +
>> + ret = bcm_iproc_i2c_format_addr(dev, msg, &addr);
>> + if (ret)
>> + return ret;
>> +
>> + /* load slave address into the TX FIFO */
>> + writel(addr, dev->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, dev->base + M_TX_OFFSET);
>> + }
>> + }
>> +
>> + /* mark as incomplete before starting the transaction */
>> + reinit_completion(&dev->done);
>> +
>> + /*
>> + * Enable the "start busy" interrupt, which will be triggered after
>> + * the transaction is done
>> + */
>> + writel(1 << IE_M_START_BUSY_SHIFT, dev->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, dev->base + M_CMD_OFFSET);
>> +
>> + time_left = wait_for_completion_timeout(&dev->done, time_left);
>> +
>> + /* disable all interrupts */
>> + writel(0, dev->base + IE_OFFSET);
>> +
>> + if (!time_left) {
>> + dev_err(dev->device, "transaction times out\n");
>> +
>> + /* flush FIFOs */
>> + val = (1 << M_FIFO_RX_FLUSH_SHIFT) |
>> + (1 << M_FIFO_TX_FLUSH_SHIFT);
>> + writel(val, dev->base + M_FIFO_CTRL_OFFSET);
>> + return -EREMOTEIO;
>> + }
>> +
>> + ret = bcm_iproc_i2c_check_status(dev);
>> + if (ret) {
>> + /* flush both TX/RX FIFOs */
>> + val = (1 << M_FIFO_RX_FLUSH_SHIFT) |
>> + (1 << M_FIFO_TX_FLUSH_SHIFT);
>> + writel(val, dev->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(dev->base + M_RX_OFFSET) >>
>> + M_RX_DATA_SHIFT) & M_RX_DATA_MASK;
>> + }
>> + }
>> +
>> + dev_dbg(dev->device, "xfer %c, addr=0x%02x, len=%d\n",
>> + (msg->flags & I2C_M_RD) ? 'R' : 'W', msg->addr,
>> + msg->len);
>> + dev_dbg(dev->device, "**** data start ****\n");
>> + for (i = 0; i < msg->len; i++)
>> + dev_dbg(dev->device, "0x%02x ", msg->buf[i]);
>> + dev_dbg(dev->device, "**** data end ****\n");
>> +
>> + return 0;
>> +}
>> +
>> +static int bcm_iproc_i2c_xfer(struct i2c_adapter *adapter,
>> + struct i2c_msg msgs[], int num)
>> +{
>> + struct bcm_iproc_i2c_dev *dev = i2c_get_adapdata(adapter);
>> + int ret, i;
>> +
>> + /* go through all messages */
>> + for (i = 0; i < num; i++) {
>> + ret = bcm_iproc_i2c_xfer_single_msg(dev, &msgs[i]);
>> + if (ret) {
>> + dev_err(dev->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 *dev)
>> +{
>> + unsigned int bus_speed, speed_bit;
>> + u32 val;
>> + int ret = of_property_read_u32(dev->device->of_node,
>> "clock-frequency",
>> + &bus_speed);
>> + if (ret < 0) {
>> + dev_err(dev->device, "missing clock-frequency property\n");
>> + return -ENODEV;
>> + }
>> +
>> + switch (bus_speed) {
>> + case 100000:
>> + speed_bit = 0;
>> + break;
>> + case 400000:
>> + speed_bit = 1;
>> + break;
>> + default:
>> + dev_err(dev->device, "%d Hz bus speed not supported\n",
>> + bus_speed);
>> + dev_err(dev->device, "valid speeds are 100khz and 400khz\n");
>> + return -EINVAL;
>> + }
>> +
>> + val = readl(dev->base + TIM_CFG_OFFSET);
>> + val &= ~(1 << TIME_CFG_MODE_400_SHIFT);
>> + val |= speed_bit << TIME_CFG_MODE_400_SHIFT;
>> + writel(val, dev->base + TIM_CFG_OFFSET);
>> +
>> + dev_info(dev->device, "bus set to %u Hz\n", bus_speed);
>> +
>> + return 0;
>> +}
>> +
>> +static int bcm_iproc_i2c_init(struct bcm_iproc_i2c_dev *dev)
>> +{
>> + u32 val;
>> +
>> + /* put controller in reset */
>> + val = readl(dev->base + CFG_OFFSET);
>> + val |= 1 << CFG_RESET_SHIFT;
>> + val &= ~(1 << CFG_EN_SHIFT);
>> + writel(val, dev->base + CFG_OFFSET);
>> +
>> + /* wait 100 usec per spec */
>> + udelay(100);
>> +
>> + /* bring controller out of reset */
>> + val = readl(dev->base + CFG_OFFSET);
>> + val &= ~(1 << CFG_RESET_SHIFT);
>> + writel(val, dev->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, dev->base + M_FIFO_CTRL_OFFSET);
>> +
>> + /* disable all interrupts */
>> + val = 0;
>> + writel(val, dev->base + IE_OFFSET);
>> +
>> + /* clear all pending interrupts */
>> + val = readl(dev->base + IS_OFFSET);
>> + writel(val, dev->base + IS_OFFSET);
>> +
>> + return 0;
>> +}
>> +
>> +static void bcm_iproc_i2c_enable(struct bcm_iproc_i2c_dev *dev)
>> +{
>> + u32 val;
>> +
>> + val = readl(dev->base + CFG_OFFSET);
>> + val |= 1 << CFG_EN_SHIFT;
>> + writel(val, dev->base + CFG_OFFSET);
>> +}
>> +
>> +static void bcm_iproc_i2c_disable(struct bcm_iproc_i2c_dev *dev)
>> +{
>> + u32 val;
>> +
>> + val = readl(dev->base + CFG_OFFSET);
>> + val &= ~(1 << CFG_EN_SHIFT);
>> + writel(val, dev->base + CFG_OFFSET);
>> +}
>> +
>> +static int bcm_iproc_i2c_probe(struct platform_device *pdev)
>> +{
>> + int irq, ret = 0;
>> + struct bcm_iproc_i2c_dev *dev;
>> + struct i2c_adapter *adap;
>> + struct resource *res;
>> +
>> + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
>> + if (!dev)
>> + return -ENOMEM;
>> +
>> + platform_set_drvdata(pdev, dev);
>> + dev->device = &pdev->dev;
>> + init_completion(&dev->done);
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!res)
>> + return -ENODEV;
>
> We can remove this resource check. This checking will happen with
> devm_ioremap_resource()
>
Don't you need to obtain a valid resource and pass it into
devm_ioremap_resource? Without 'res' being assigned a valid resource,
devm_ioremap_resource will reject with "invalid resource".
>> + dev->base = devm_ioremap_resource(dev->device, res);
>> + if (IS_ERR(dev->base))
>> + return -ENOMEM;
>> +
>> + ret = bcm_iproc_i2c_init(dev);
>> + if (ret)
>> + return ret;
>> +
>> + ret = bcm_iproc_i2c_cfg_speed(dev);
>> + if (ret)
>> + return ret;
>> +
>> + irq = platform_get_irq(pdev, 0);
>> + if (irq < 0) {
>> + dev_err(dev->device, "no irq resource\n");
>> + return irq;
>> + }
>> +
>> + ret = devm_request_irq(&pdev->dev, irq, bcm_iproc_i2c_isr,
>> + IRQF_SHARED, pdev->name, dev);
>> + if (ret) {
>> + dev_err(dev->device, "unable to request irq %i\n", irq);
>> + return ret;
>> + }
>> +
>> + bcm_iproc_i2c_enable(dev);
>> +
>> + adap = &dev->adapter;
>> + i2c_set_adapdata(adap, dev);
>> + 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(dev->device, "failed to add adapter\n");
>> + return ret;
>> + }
>> +
>> + dev_info(dev->device, "device registered successfully\n");
>> +
>> + return 0;
>> +}
>> +
>> +static int bcm_iproc_i2c_remove(struct platform_device *pdev)
>> +{
>> + struct bcm_iproc_i2c_dev *dev = platform_get_drvdata(pdev);
>> +
>> + i2c_del_adapter(&dev->adapter);
>> + bcm_iproc_i2c_disable(dev);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id bcm_iproc_i2c_of_match[] = {
>> + {.compatible = "brcm,iproc-i2c",},
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, bcm_iproc_i2c_of_match);
>> +
>> +static struct platform_driver bcm_iproc_i2c_driver = {
>> + .driver = {
>> + .name = "bcm-iproc-i2c",
>> + .owner = THIS_MODULE,
>
> No need to update this field. Its updated by module_platform_driver().
>
Okay will get rid of .owner = THIS_MODULES,
>> + .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");
>
On 12/9/2014 7:12 PM, Varka Bhadram wrote:
>
>
> On Wed, Dec 10, 2014 at 7:05 AM, Ray Jui <[email protected]
> <mailto:[email protected]>> wrote:
>
>
>
> On 12/9/2014 5:27 PM, Varka Bhadram wrote:
>
> Hi,
>
> On Wednesday 10 December 2014 06:24 AM, Ray Jui wrote:
>
> Document the I2C device tree binding for Broadcom iProc
> family of
> SoCs
>
> Signed-off-by: Ray Jui <[email protected]
> <mailto:[email protected]>>
> Reviewed-by: Scott Branden <[email protected]
> <mailto:[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
> +
>
>
> All the properties defined with two lines of statements.
>
> Why cant they be with single line statement, like:
>
> 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
>
> ....
>
>
> I thought making them two lines are more readable (and obviously
> that's very subjective, :)). But more importantly, it matches the
> format of other Broadcom iProc/Cygnus devicetree binding documents
> that are currently in progress of upstreaming.
>
>
> But max of the bindings over the kernel follows single line statements.
>
> --
> Thanks and Regards,
> Varka Bhadram.
Is it a requirement for these property descriptions to be one line? If
not, I prefer to stick with the way it is now. Thanks.
On 12/9/2014 7:28 PM, Varka Bhadram wrote:
>
>
> On Wed, Dec 10, 2014 at 8:51 AM, Varka Bhadram <[email protected]
> <mailto:[email protected]>> wrote:
>
> On Wed, Dec 10, 2014 at 7:11 AM, Ray Jui <[email protected]
> <mailto:[email protected]>> wrote:
>
>
>
> On 12/9/2014 5:33 PM, Varka Bhadram wrote:
>
>
> On Wednesday 10 December 2014 06:24 AM, 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)
>
> Signed-off-by: Ray Jui <[email protected]
> <mailto:[email protected]>>
> Reviewed-by: Scott Branden <[email protected]
> <mailto:[email protected]>>
> ---
> drivers/i2c/busses/Kconfig | 9 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-bcm-iproc.c | 503
> ++++++++++++++++++++++++++++++++++++
> 3 files changed, 513 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-bcm-iproc.c
>
> (...)
>
> +static int bcm_iproc_i2c_probe(struct platform_device
> *pdev)
> +{
> + int irq, ret = 0;
> + struct bcm_iproc_i2c_dev *dev;
> + struct i2c_adapter *adap;
> + struct resource *res;
> +
> + dev = devm_kzalloc(&pdev->dev, sizeof(*dev),
> GFP_KERNEL);
> + if (!dev)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, dev);
> + dev->device = &pdev->dev;
> + init_completion(&dev->done);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENODEV;
>
>
> We can remove this resource check. This checking will happen
> with
> devm_ioremap_resource()
>
> Don't you need to obtain a valid resource and pass it into
> devm_ioremap_resource? Without 'res' being assigned a valid
> resource, devm_ioremap_resource will reject with "invalid resource".
>
> platform_get_resource() will return a resource, checking on this
> resource is happening at
> http://lxr.free-electrons.com/source/lib/devres.c#L115. So no need
> to check it explicitly.
>
> If you check here it will be duplication of check with resource. Two
> times we are checking on
> the resource. No point of doing like that.
>
> Thanks.
Sorry I misunderstood what you meant. Okay I'll get rid of if (!res)
check there. Thanks.
>
>
> See this: http://lxr.free-electrons.com/source/lib/devres.c#L102
>
> --
> Thanks and Regards,
> Varka Bhadram.