Re: [PATCH v2 2/2] i2c: Support for Netlogic XLP9XX/5XX I2C controller.
Hello,
On Wed, Mar 04, 2015 at 12:25:33PM +0530, Jayachandran C wrote:
> From: Subhendu Sekhar Behera
>
> Add an I2C bus driver i2c-xlp9xx.c to support the I2C block in the
> XLP9xx/XLP5xx MIPS SoC. Update Kconfig and Makefile to add the
> CONFIG_I2C_XLP9XX option.
>
> Also add document Documentation/devicetree/bindings/i2c/i2c-xlp9xx.txt
> for DT compatible string "netlogic,xlp9xx-i2c".
>
> Signed-off-by: Subhendu Sekhar Behera
> Signed-off-by: Jayachandran C
> ---
> .../devicetree/bindings/i2c/i2c-xlp9xx.txt | 22 +
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile| 1 +
> drivers/i2c/busses/i2c-xlp9xx.c| 446
> +
> 4 files changed, 479 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-xlp9xx.txt
> create mode 100644 drivers/i2c/busses/i2c-xlp9xx.c
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-xlp9xx.txt
> b/Documentation/devicetree/bindings/i2c/i2c-xlp9xx.txt
> new file mode 100644
> index ..f23ae87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-xlp9xx.txt
> @@ -0,0 +1,22 @@
> +Device tree configuration for the I2C controller on the XLP9xx/5xx SoC
> +
> +Required properties:
> +- compatible : should be "netlogic,xlp9xx-i2c"
> +- reg : bus address start and address range size of device
> +- interrupts : interrupt number
> +
> +Optional properties:
> +- clock-frequency : frequency of bus clock in Hz
> +Defaults to 100 KHz when the property is not specified
> +
> +Example:
> +
> +i2c0: i2c@113100 {
> + compatible = "netlogic,xlp9xx-i2c";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0 0x113100 0x100>;
> + clock-frequency = <40>;
> + interrupts = <30>;
> + interrupt-parent = <&pic>;
> +};
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 22da9c2..dde4648 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -898,6 +898,16 @@ config I2C_XLR
> This driver can also be built as a module. If so, the module
> will be called i2c-xlr.
>
> +config I2C_XLP9XX
> + tristate "XLP9XX I2C support"
> + depends on CPU_XLP || COMPILE_TEST
> + help
> + This driver enables support for the on-chip I2C interface of
> + the Broadcom XLP9xx/XLP5xx MIPS processors.
> +
> + This driver can also be built as a module. If so, the module will
> + be called i2c-xlp9xx.
> +
> config I2C_RCAR
> tristate "Renesas R-Car I2C Controller"
> depends on ARCH_SHMOBILE || COMPILE_TEST
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 3638feb..f8e0f0e 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -87,6 +87,7 @@ obj-$(CONFIG_I2C_WMT) += i2c-wmt.o
> obj-$(CONFIG_I2C_OCTEON) += i2c-octeon.o
> obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o
> obj-$(CONFIG_I2C_XLR)+= i2c-xlr.o
> +obj-$(CONFIG_I2C_XLP9XX) += i2c-xlp9xx.o
> obj-$(CONFIG_I2C_RCAR) += i2c-rcar.o
>
> # External I2C/SMBus adapter drivers
> diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
> new file mode 100644
> index ..6886add
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-xlp9xx.c
> @@ -0,0 +1,446 @@
> +/*
> + * Copyright (c) 2003-2015 Broadcom Corporation
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
It would be nice to point here a link to the device's documentation.
> +
> +#include
> +#include
> +#include
Do you need hardirq.h?
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> [...]
> +static void xlp9xx_i2c_mask_irq(struct xlp9xx_i2c_dev *priv, u32 mask)
> +{
> + mask = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_INTEN) & ~mask;
s/ / /
For code clarity I'd spend another variable name and make this read:
u32 inten = xlp9xx_read_i2c_reg(...);
inten &= ~mask;
xlp9xx_write_i2c_reg(..., inten);
(If you prefer, combine the two last commands.)
> +static void xlp9xx_i2c_set_rx_fifo_thres(struct xlp9xx_i2c_dev *priv,
> + u32 th)
> +{
> + xlp9xx_write_i2c_reg(priv, XLP9XX_I2C_MFIFOCTRL,
> + (th << XLP9XX_I2C_MFIFOCTRL_HITH_SHIFT));
Usually a single tab is not considered to be "placed substantially to
the right" (see Documentation/CodingStyle) for continuation lines. Usual
are either two tabs or alignment to the opening (. Whichever you choose
use it consitently in the whole file.
> +static void xlp9xx_i2c_drain_rx_fifo(struct xlp9xx_i2c_dev *priv)
> +{
> + u32 len, i;
> + u8 *buf = priv->msg_buf;
> +
> + len = xlp9xx_read_
Re: [PATCH v2 2/2] i2c: Support for Netlogic XLP9XX/5XX I2C controller.
Hi Jayachandran,
On 3/3/2015 10:55 PM, Jayachandran C wrote:
> From: Subhendu Sekhar Behera
>
> Add an I2C bus driver i2c-xlp9xx.c to support the I2C block in the
> XLP9xx/XLP5xx MIPS SoC. Update Kconfig and Makefile to add the
> CONFIG_I2C_XLP9XX option.
>
> Also add document Documentation/devicetree/bindings/i2c/i2c-xlp9xx.txt
> for DT compatible string "netlogic,xlp9xx-i2c".
>
> Signed-off-by: Subhendu Sekhar Behera
> Signed-off-by: Jayachandran C
> ---
> .../devicetree/bindings/i2c/i2c-xlp9xx.txt | 22 +
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile| 1 +
> drivers/i2c/busses/i2c-xlp9xx.c| 446
> +
> 4 files changed, 479 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-xlp9xx.txt
> create mode 100644 drivers/i2c/busses/i2c-xlp9xx.c
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-xlp9xx.txt
> b/Documentation/devicetree/bindings/i2c/i2c-xlp9xx.txt
> new file mode 100644
> index ..f23ae87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-xlp9xx.txt
> @@ -0,0 +1,22 @@
> +Device tree configuration for the I2C controller on the XLP9xx/5xx SoC
> +
> +Required properties:
> +- compatible : should be "netlogic,xlp9xx-i2c"
> +- reg : bus address start and address range size of device
> +- interrupts : interrupt number
> +
> +Optional properties:
> +- clock-frequency : frequency of bus clock in Hz
> +Defaults to 100 KHz when the property is not specified
> +
> +Example:
> +
> +i2c0: i2c@113100 {
> + compatible = "netlogic,xlp9xx-i2c";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0 0x113100 0x100>;
> + clock-frequency = <40>;
> + interrupts = <30>;
> + interrupt-parent = <&pic>;
> +};
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 22da9c2..dde4648 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -898,6 +898,16 @@ config I2C_XLR
> This driver can also be built as a module. If so, the module
> will be called i2c-xlr.
>
> +config I2C_XLP9XX
> + tristate "XLP9XX I2C support"
> + depends on CPU_XLP || COMPILE_TEST
> + help
> + This driver enables support for the on-chip I2C interface of
> + the Broadcom XLP9xx/XLP5xx MIPS processors.
> +
> + This driver can also be built as a module. If so, the module will
> + be called i2c-xlp9xx.
> +
> config I2C_RCAR
> tristate "Renesas R-Car I2C Controller"
> depends on ARCH_SHMOBILE || COMPILE_TEST
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 3638feb..f8e0f0e 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -87,6 +87,7 @@ obj-$(CONFIG_I2C_WMT) += i2c-wmt.o
> obj-$(CONFIG_I2C_OCTEON) += i2c-octeon.o
> obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o
> obj-$(CONFIG_I2C_XLR)+= i2c-xlr.o
> +obj-$(CONFIG_I2C_XLP9XX) += i2c-xlp9xx.o
> obj-$(CONFIG_I2C_RCAR) += i2c-rcar.o
>
> # External I2C/SMBus adapter drivers
> diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
> new file mode 100644
> index ..6886add
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-xlp9xx.c
> @@ -0,0 +1,446 @@
> +/*
> + * Copyright (c) 2003-2015 Broadcom Corporation
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define XLP9XX_I2C_DIV 0x0
> +#define XLP9XX_I2C_CTRL 0x1
> +#define XLP9XX_I2C_CMD 0x2
> +#define XLP9XX_I2C_STATUS0x3
> +#define XLP9XX_I2C_MTXFIFO 0x4
> +#define XLP9XX_I2C_MRXFIFO 0x5
> +#define XLP9XX_I2C_MFIFOCTRL 0x6
> +#define XLP9XX_I2C_STXFIFO 0x7
> +#define XLP9XX_I2C_SRXFIFO 0x8
> +#define XLP9XX_I2C_SFIFOCTRL 0x9
> +#define XLP9XX_I2C_SLAVEADDR 0xA
> +#define XLP9XX_I2C_OWNADDR 0xB
> +#define XLP9XX_I2C_FIFOWCNT 0xC
> +#define XLP9XX_I2C_INTEN 0xD
> +#define XLP9XX_I2C_INTST 0xE
> +#define XLP9XX_I2C_WAITCNT 0xF
> +#define XLP9XX_I2C_TIMEOUT 0X10
> +#define XLP9XX_I2C_GENCALLADDR 0x11
> +
> +#define XLP9XX_I2C_CTRL_MCTLEN_SHIFT 16
> +#define XLP9XX_I2C_MFIFOCTRL_HITH_SHIFT 8
> +#define XLP9XX_I2C_MFIFOCTRL_LOTH_SHIFT 0
> +#define XLP9XX_I2C_SLAVEADDR_ADDR_SHIFT 1
> +
> +#define XLP9XX_I2C_CMD_START BIT(7)
> +#define XLP9XX_I2C_CMD_STOP BIT(6)
