* RE: [Patch V3] i2c: imx: add low power i2c bus driver
2016-08-17 7:59 [Patch V3] i2c: imx: add low power i2c bus driver Gao Pan
@ 2016-08-26 7:37 ` Pan Gao
2016-10-24 5:51 ` Pandy Gao
2016-09-09 1:49 ` Pandy Gao
2016-10-24 23:15 ` Vladimir Zapolskiy
2 siblings, 1 reply; 6+ messages in thread
From: Pan Gao @ 2016-08-26 7:37 UTC (permalink / raw)
To: wsa, u.kleine-koenig, cmo; +Cc: linux-i2c, Frank Li, Fugang Duan
Ping...
> -----Original Message-----
> From: Pan Gao
> Sent: Wednesday, August 17, 2016 4:00 PM
> To: wsa@the-dreams.de; u.kleine-koenig@pengutronix.de; cmo@melexis.com
> Cc: linux-i2c@vger.kernel.org; Frank Li <frank.li@nxp.com>; Fugang Duan
> <fugang.duan@nxp.com>; Pan Gao <pandy.gao@nxp.com>
> Subject: [Patch V3] i2c: imx: add low power i2c bus driver
>
> This patch adds low power i2c bus driver to support new i.MX products which use
> low power i2c instead of the old imx i2c.
>
> The low power i2c can continue operating in stop mode when an appropriate
> clock is available. It is also designed for low CPU overhead with DMA offloading of
> FIFO register accesses.
>
> Signed-off-by: Gao Pan <pandy.gao@nxp.com>
> Reviewed-by: Fugang Duan <B38611@freescale.com>
> ---
> V2:
> -stop i2c transfer under the wrong condition -add timeout check in while()
> domain
>
> V3:
> -fix typo inside commit message and the driver.
>
> .../devicetree/bindings/i2c/i2c-imx-lpi2c.txt | 25 +
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-imx-lpi2c.c | 667 +++++++++++++++++++++
> 4 files changed, 703 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> b/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> new file mode 100644
> index 0000000..1f10cbf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> @@ -0,0 +1,25 @@
> +* Freescale Low Power Inter IC (LPI2C) for i.MX
> +
> +Required properties:
> +- compatible :
> + - "fsl,imx8dv-lpi2c" for LPI2C compatible with the one integrated on
> +i.MX8DV soc
> + - "fsl,imx7ulp-lpi2c" for LPI2C compatible with the one integrated on
> +i.MX7ULP soc
> +- reg : Should contain LPI2C registers location and length
> +- interrupts : Should contain LPI2C interrupt
> +- clocks : Should contain LPI2C clock specifier
> +- power-domains : should contain LPI2C power domain
> +
> +Optional properties:
> +- clock-frequency : Constains desired LPI2C bus clock frequency in Hz.
> + The absence of the property indicates the default frequency 100 kHz.
> +
> +Examples:
> +
> +i2c1: i2c@5e110000 { /* LPI2C on i.MX8DV */
> + compatible = "fsl,imx8dv-lpi2c";
> + reg = <0x0 0x5e110000 0x0 0x4000>;
> + interrupts = <0 88 4>;
> + clocks = <&clk IMX8DV_I2C1_CLK>;
> + clock-names = "per";
> + power-domains = <&pd_lsio_i2c1>;
> +};
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index
> efa3d9b..1fc7a10 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -596,6 +596,16 @@ config I2C_IMX
> This driver can also be built as a module. If so, the module
> will be called i2c-imx.
>
> +config I2C_IMX_LPI2C
> + tristate "IMX Low Power I2C interface"
> + depends on ARCH_MXC || COMPILE_TEST
> + help
> + Say Y here if you want to use the Low Power IIC bus controller
> + on the Freescale i.MX processors.
> +
> + This driver can also be built as a module. If so, the module
> + will be called i2c-imx-lpi2c.
> +
> config I2C_IOP3XX
> tristate "Intel IOPx3xx and IXP4xx on-chip I2C interface"
> depends on ARCH_IOP32X || ARCH_IOP33X || ARCH_IXP4XX ||
> ARCH_IOP13XX diff --git a/drivers/i2c/busses/Makefile
> b/drivers/i2c/busses/Makefile index 37f2819..cc93457 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_I2C_HIX5HD2) += i2c-hix5hd2.o
> obj-$(CONFIG_I2C_IBM_IIC) += i2c-ibm_iic.o
> obj-$(CONFIG_I2C_IMG) += i2c-img-scb.o
> obj-$(CONFIG_I2C_IMX) += i2c-imx.o
> +obj-$(CONFIG_I2C_IMX_LPI2C) += i2c-imx-lpi2c.o
> obj-$(CONFIG_I2C_IOP3XX) += i2c-iop3xx.o
> obj-$(CONFIG_I2C_JZ4780) += i2c-jz4780.o
> obj-$(CONFIG_I2C_KEMPLD) += i2c-kempld.o
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> new file mode 100644
> index 0000000..308ecf5
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -0,0 +1,667 @@
> +/*
> + * This is i.MX low power i2c controller driver.
> + *
> + * Copyright 2016 Freescale Semiconductor, Inc.
> + *
> + * 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; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +
> +#define DRIVER_NAME "imx-lpi2c"
> +
> +#define LPI2C_PARAM 0x04 /* i2c RX/TX FIFO size */
> +#define LPI2C_MCR 0x10 /* i2c contrl register */
> +#define LPI2C_MSR 0x14 /* i2c status register */
> +#define LPI2C_MIER 0x18 /* i2c interrupt enable */
> +#define LPI2C_MCFGR0 0x20 /* i2c master configuration */
> +#define LPI2C_MCFGR1 0x24 /* i2c master configuration */
> +#define LPI2C_MCFGR2 0x28 /* i2c master configuration */
> +#define LPI2C_MCFGR3 0x2C /* i2c master configuration */
> +#define LPI2C_MCCR0 0x48 /* i2c master clk configuration */
> +#define LPI2C_MCCR1 0x50 /* i2c master clk configuration */
> +#define LPI2C_MFCR 0x58 /* i2c master FIFO control */
> +#define LPI2C_MFSR 0x5C /* i2c master FIFO status */
> +#define LPI2C_MTDR 0x60 /* i2c master TX data register */
> +#define LPI2C_MRDR 0x70 /* i2c master RX data register */
> +
> +/* i2c command */
> +#define TRAN_DATA 0X00
> +#define RECV_DATA 0X01
> +#define GEN_STOP 0X02
> +#define RECV_DISCARD 0X03
> +#define GEN_START 0X04
> +#define START_NACK 0X05
> +#define START_HIGH 0X06
> +#define START_HIGH_NACK 0X07
> +
> +#define MCR_MEN (1 << 0)
> +#define MCR_RST (1 << 1)
> +#define MCR_DOZEN (1 << 2)
> +#define MCR_DBGEN (1 << 3)
> +#define MCR_RTF (1 << 8)
> +#define MCR_RRF (1 << 9)
> +#define MSR_TDF (1 << 0)
> +#define MSR_RDF (1 << 1)
> +#define MSR_SDF (1 << 9)
> +#define MSR_NDF (1 << 10)
> +#define MSR_ALF (1 << 11)
> +#define MSR_MBF (1 << 24)
> +#define MSR_BBF (1 << 25)
> +#define MIER_TDIE (1 << 0)
> +#define MIER_RDIE (1 << 1)
> +#define MIER_SDIE (1 << 9)
> +#define MIER_NDIE (1 << 10)
> +#define MCFGR1_AUTOSTOP (1 << 8)
> +#define MCFGR1_IGNACK (1 << 9)
> +#define MRDR_RXEMPTY (1 << 14)
> +
> +#define I2C_CLK_RATIO 2
> +#define CHUNK_DATA 256
> +
> +#define LPI2C_RX_FIFOSIZE 4
> +#define LPI2C_TX_FIFOSIZE 4
> +
> +#define LPI2C_DEFAULT_RATE 100000
> +#define STARDARD_MAX_BITRATE 400000
> +#define FAST_MAX_BITRATE 1000000
> +#define FAST_PLUS_MAX_BITRATE 3400000
> +#define HIGHSPEED_MAX_BITRATE 5000000
> +
> +
> +enum lpi2c_imx_mode {
> + STANDARD, /* 100+Kbps */
> + FAST, /* 400+Kbps */
> + FAST_PLUS, /* 1.0+Mbps */
> + ULTRA_FAST, /* 5.0+Mbps */
> + HS, /* 3.4+Mbps */
> +};
> +
> +enum lpi2c_imx_pincfg {
> + TWO_PIN_OD, /* 2-pin open drain mode */
> + TWO_PIN_OO, /* 2-pin output only mode (utra-fast mode) */
> + TWO_PIN_PP, /* 2-pin push-pull mode */
> + FOUR_PIN_PP, /* 4-pin push-pull mode */
> + TWO_PIN_OD_SS, /* 2-pin open drain mode with separate slave */
> + TWO_PIN_OO_SS, /* 2-pin output only mode with separate slave */
> + TWO_PIN_PP_SS, /* 2-pin push-pull mode with separate slave */
> + FOUR_PIN_PP_IO, /* 4-pin push-pull mode (inverted output) */
> +};
> +
> +struct lpi2c_imx_clkcfg {
> + u8 prescale;
> + u8 filtscl;
> + u8 filtsda;
> + u8 sethold;
> + u8 clklo;
> + u8 clkhi;
> + u8 datavd;
> +};
> +
> +struct lpi2c_imx_struct {
> + struct i2c_adapter adapter;
> + struct clk *per_clk;
> + void __iomem *base;
> + __u8 *rx_buf;
> + __u8 *tx_buf;
> + struct completion complete;
> + unsigned int msglen;
> + unsigned int delivered;
> + unsigned int block_data;
> + unsigned int bitrate;
> + enum lpi2c_imx_mode mode;
> +};
> +
> +static void lpi2c_imx_intctrl(
> + struct lpi2c_imx_struct *lpi2c_imx, unsigned int enable) {
> + writel(enable, lpi2c_imx->base + LPI2C_MIER); }
> +
> +static int lpi2c_imx_bus_busy(struct lpi2c_imx_struct *lpi2c_imx) {
> + unsigned long orig_jiffies = jiffies;
> + unsigned int temp;
> +
> + while (1) {
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> +
> + /* check for arbitration lost, clear if set */
> + if (temp & MSR_ALF) {
> + writel(temp, lpi2c_imx->base + LPI2C_MSR);
> + return -EAGAIN;
> + }
> +
> + if ((temp & MSR_BBF) && (temp & MSR_MBF))
> + break;
> +
> + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "bus not work\n");
> + return -ETIMEDOUT;
> + }
> + schedule();
> + }
> +
> + return 0;
> +}
> +
> +static void lpi2c_imx_set_mode(struct lpi2c_imx_struct *lpi2c_imx) {
> + enum lpi2c_imx_mode mode;
> + unsigned int bitrate = lpi2c_imx->bitrate;
> +
> + if (bitrate < STARDARD_MAX_BITRATE)
> + mode = STANDARD;
> + else if (bitrate < FAST_MAX_BITRATE)
> + mode = FAST;
> + else if (bitrate < FAST_PLUS_MAX_BITRATE)
> + mode = FAST_PLUS;
> + else if (bitrate < HIGHSPEED_MAX_BITRATE)
> + mode = HS;
> + else
> + mode = ULTRA_FAST;
> +
> + lpi2c_imx->mode = mode;
> +}
> +
> +static int lpi2c_imx_start(struct lpi2c_imx_struct *lpi2c_imx,
> + struct i2c_msg *msgs)
> +{
> + u8 read;
> + unsigned int temp;
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> + temp |= MCR_RRF | MCR_RTF;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> + writel(0x7f00, lpi2c_imx->base + LPI2C_MSR);
> +
> + read = msgs->flags & I2C_M_RD;
> + temp = (msgs->addr << 1 | read) | (GEN_START << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> +
> + return lpi2c_imx_bus_busy(lpi2c_imx);
> +}
> +
> +static void lpi2c_imx_stop(struct lpi2c_imx_struct *lpi2c_imx) {
> + unsigned int temp;
> + unsigned long orig_jiffies = jiffies;
> +
> + writel(GEN_STOP << 8, lpi2c_imx->base + LPI2C_MTDR);
> + do {
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> + if (temp & MSR_SDF)
> + break;
> +
> + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "stop timeout\n");
> + break;
> + }
> + schedule();
> +
> + } while (1);
> +}
> +
> +
> +/* CLKLO = I2C_CLK_RATIO * CLKHI, SETHOLD = CLKHI, DATAVD = CLKHI/2 */
> +static int lpi2c_imx_config(struct lpi2c_imx_struct *lpi2c_imx) {
> + unsigned int temp;
> + unsigned int per_clk_rate;
> + unsigned int prescale, clk_high, clk_low, clk_cycle;
> + enum lpi2c_imx_pincfg pincfg;
> + struct lpi2c_imx_clkcfg clkcfg;
> +
> + lpi2c_imx_set_mode(lpi2c_imx);
> + per_clk_rate = clk_get_rate(lpi2c_imx->per_clk);
> +
> + if (lpi2c_imx->mode == HS || lpi2c_imx->mode == ULTRA_FAST)
> + clkcfg.filtscl = clkcfg.filtsda = 0;
> + else
> + clkcfg.filtscl = clkcfg.filtsda = 2;
> +
> + for (prescale = 0; prescale <= 7; prescale++) {
> + clk_cycle = per_clk_rate / ((1 << prescale) * lpi2c_imx->bitrate)
> + - 3 - (clkcfg.filtscl >> 1);
> + clk_high = (clk_cycle + I2C_CLK_RATIO) / (I2C_CLK_RATIO + 1);
> + clk_low = clk_cycle - clk_high;
> + if (clk_low < 64)
> + break;
> + }
> +
> + if (prescale > 7)
> + return -EINVAL;
> +
> + clkcfg.prescale = prescale;
> + clkcfg.sethold = clk_high;
> + clkcfg.clklo = clk_low;
> + clkcfg.clkhi = clk_high;
> + clkcfg.datavd = clk_high >> 1;
> +
> + /* set MCFGR1: PINCFG, PRESCALE, IGNACK */
> + if (lpi2c_imx->mode == ULTRA_FAST)
> + pincfg = TWO_PIN_OO;
> + else
> + pincfg = TWO_PIN_OD;
> + temp = clkcfg.prescale | pincfg << 24;
> +
> + if (lpi2c_imx->mode == ULTRA_FAST)
> + temp |= MCFGR1_IGNACK;
> +
> + writel(temp, lpi2c_imx->base + LPI2C_MCFGR1);
> +
> + /* set MCFGR2: FILTSDA, FILTSCL */
> + temp = (clkcfg.filtscl << 16) | (clkcfg.filtsda << 24);
> + writel(temp, lpi2c_imx->base + LPI2C_MCFGR2);
> +
> +
> + /* set MCCR: DATAVD, SETHOLD, CLKHI, CLKLO */
> + temp = clkcfg.datavd << 24 | clkcfg.sethold << 16 |
> + clkcfg.clkhi << 8 | clkcfg.clklo;
> +
> + if (lpi2c_imx->mode == HS)
> + writel(temp, lpi2c_imx->base + LPI2C_MCCR1);
> + else
> + writel(temp, lpi2c_imx->base + LPI2C_MCCR0);
> +
> + return 0;
> +}
> +
> +static int lpi2c_imx_master_enable(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + int ret;
> + unsigned int temp;
> +
> + ret = clk_prepare_enable(lpi2c_imx->per_clk);
> + if (ret)
> + return ret;
> +
> + temp = MCR_RST;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> + writel(0, lpi2c_imx->base + LPI2C_MCR);
> +
> + ret = lpi2c_imx_config(lpi2c_imx);
> + if (ret)
> + return ret;
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> + temp |= MCR_MEN;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> +
> + return 0;
> +}
> +
> +static int lpi2c_imx_master_disable(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + unsigned int temp = 0;
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> + temp &= ~MCR_MEN;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> +
> + clk_disable_unprepare(lpi2c_imx->per_clk);
> +
> + return 0;
> +}
> +
> +static int lpi2c_imx_msg_complete(struct lpi2c_imx_struct *lpi2c_imx) {
> + unsigned int timeout;
> +
> + timeout = wait_for_completion_timeout(&lpi2c_imx->complete, HZ);
> +
> + return timeout ? 0 : -ETIMEDOUT;
> +}
> +
> +static int lpi2c_imx_txfifo_empty(struct lpi2c_imx_struct *lpi2c_imx) {
> + u32 txcnt;
> + unsigned long orig_jiffies = jiffies;
> +
> + do {
> + txcnt = readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff;
> +
> + if (readl(lpi2c_imx->base + LPI2C_MSR) & MSR_NDF) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "NDF detected\n");
> + return -EIO;
> + }
> +
> + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "txfifo empty
> timeout\n");
> + return -ETIMEDOUT;
> + }
> + schedule();
> +
> + } while (txcnt);
> +
> + return 0;
> +}
> +
> +static void lpi2c_imx_set_tx_watermark(struct lpi2c_imx_struct
> +*lpi2c_imx) {
> + unsigned int temp;
> +
> + temp = LPI2C_TX_FIFOSIZE >> 1;
> + writel(temp, lpi2c_imx->base + LPI2C_MFCR); }
> +
> +static void lpi2c_imx_set_rx_watermark(struct lpi2c_imx_struct
> +*lpi2c_imx) {
> + unsigned int temp, remaining;
> +
> + remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
> +
> + if (remaining > (LPI2C_RX_FIFOSIZE >> 1))
> + temp = LPI2C_RX_FIFOSIZE >> 1;
> + else
> + temp = 0;
> +
> + writel(temp << 16, lpi2c_imx->base + LPI2C_MFCR); }
> +
> +static void lpi2c_imx_write_txfifo(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + unsigned int data, txcnt;
> +
> + txcnt = readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff;
> + while (txcnt < LPI2C_TX_FIFOSIZE) {
> + if (lpi2c_imx->delivered == lpi2c_imx->msglen)
> + break;
> + data = lpi2c_imx->tx_buf[lpi2c_imx->delivered++];
> + writel(data, lpi2c_imx->base + LPI2C_MTDR);
> + txcnt++;
> + }
> +
> + if (lpi2c_imx->delivered < lpi2c_imx->msglen)
> + lpi2c_imx_intctrl(lpi2c_imx, MIER_TDIE | MIER_NDIE);
> + else
> + complete(&lpi2c_imx->complete);
> +}
> +
> +static void lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx) {
> + unsigned int temp, data;
> + unsigned int blocklen, remaining;
> +
> + do {
> + data = readl(lpi2c_imx->base + LPI2C_MRDR);
> + if (data & MRDR_RXEMPTY)
> + break;
> + lpi2c_imx->rx_buf[lpi2c_imx->delivered++] = data & 0xff;
> + } while (1);
> +
> + /*
> + * First byte is the length of remaining packet in the SMBus block
> + * data read. Add it to msgs->len.
> + */
> + if (lpi2c_imx->block_data) {
> + blocklen = lpi2c_imx->rx_buf[0];
> + lpi2c_imx->msglen += blocklen;
> + }
> +
> + remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
> + /* not finished, still waiting for rx data */
> + if (remaining) {
> + lpi2c_imx_set_rx_watermark(lpi2c_imx);
> + /* multiple receive commands */
> + if (lpi2c_imx->block_data) {
> + lpi2c_imx->block_data = 0;
> + temp = remaining;
> + temp |= (RECV_DATA << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> + } else if (!(lpi2c_imx->delivered & 0xff)) {
> + temp = remaining > CHUNK_DATA ?
> + CHUNK_DATA - 1 : (remaining - 1);
> + temp |= (RECV_DATA << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> + }
> +
> + lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE);
> + } else
> + complete(&lpi2c_imx->complete);
> +}
> +
> +static void lpi2c_imx_write(struct lpi2c_imx_struct *lpi2c_imx,
> + struct i2c_msg *msgs)
> +{
> + lpi2c_imx->tx_buf = msgs->buf;
> + lpi2c_imx_set_tx_watermark(lpi2c_imx);
> + lpi2c_imx_write_txfifo(lpi2c_imx);
> +}
> +
> +static void lpi2c_imx_read(struct lpi2c_imx_struct *lpi2c_imx,
> + struct i2c_msg *msgs)
> +{
> + unsigned int temp;
> +
> + lpi2c_imx->rx_buf = msgs->buf;
> + lpi2c_imx->block_data = msgs->flags & I2C_M_RECV_LEN;
> +
> + lpi2c_imx_set_rx_watermark(lpi2c_imx);
> + temp = msgs->len > CHUNK_DATA ? CHUNK_DATA - 1 : msgs->len - 1;
> + temp |= (RECV_DATA << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> +
> + lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE | MIER_NDIE); }
> +
> +static int lpi2c_imx_xfer(struct i2c_adapter *adapter,
> + struct i2c_msg *msgs, int num)
> +{
> + int i, result;
> + unsigned int temp;
> + struct lpi2c_imx_struct *lpi2c_imx = i2c_get_adapdata(adapter);
> +
> + result = lpi2c_imx_master_enable(lpi2c_imx);
> + if (result)
> + return result;
> +
> + for (i = 0; i < num; i++) {
> + result = lpi2c_imx_start(lpi2c_imx, &msgs[i]);
> + if (result)
> + goto disable;
> +
> + /* quick smbus */
> + if (num == 1 && msgs[0].len == 0)
> + goto stop;
> +
> + lpi2c_imx->delivered = 0;
> + lpi2c_imx->msglen = msgs[i].len;
> + init_completion(&lpi2c_imx->complete);
> +
> + if (msgs[i].flags & I2C_M_RD)
> + lpi2c_imx_read(lpi2c_imx, &msgs[i]);
> + else
> + lpi2c_imx_write(lpi2c_imx, &msgs[i]);
> +
> + result = lpi2c_imx_msg_complete(lpi2c_imx);
> + if (result)
> + goto stop;
> +
> + if (!(msgs[i].flags & I2C_M_RD)) {
> + result = lpi2c_imx_txfifo_empty(lpi2c_imx);
> + if (result)
> + goto stop;
> + }
> + }
> +
> +stop:
> + lpi2c_imx_stop(lpi2c_imx);
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> + if ((temp & MSR_NDF) && !result)
> + result = -EIO;
> +
> +disable:
> + lpi2c_imx_master_disable(lpi2c_imx);
> +
> + dev_dbg(&lpi2c_imx->adapter.dev, "<%s> exit with: %s: %d\n", __func__,
> + (result < 0) ? "error" : "success msg",
> + (result < 0) ? result : num);
> +
> + return (result < 0) ? result : num;
> +}
> +
> +static irqreturn_t lpi2c_imx_isr(int irq, void *dev_id) {
> + unsigned int temp;
> + struct lpi2c_imx_struct *lpi2c_imx = dev_id;
> +
> + lpi2c_imx_intctrl(lpi2c_imx, 0);
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> +
> + if (temp & MSR_RDF) {
> + lpi2c_imx_read_rxfifo(lpi2c_imx);
> + return IRQ_HANDLED;
> + }
> +
> + if (temp & MSR_TDF) {
> + lpi2c_imx_write_txfifo(lpi2c_imx);
> + return IRQ_HANDLED;
> + }
> +
> + complete(&lpi2c_imx->complete);
> + return IRQ_HANDLED;
> +}
> +
> +static u32 lpi2c_imx_func(struct i2c_adapter *adapter) {
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL
> + | I2C_FUNC_SMBUS_READ_BLOCK_DATA;
> +}
> +
> +static struct i2c_algorithm lpi2c_imx_algo = {
> + .master_xfer = lpi2c_imx_xfer,
> + .functionality = lpi2c_imx_func,
> +};
> +
> +static const struct of_device_id lpi2c_imx_of_match[] = {
> + { .compatible = "fsl,imx8dv-lpi2c" },
> + { .compatible = "fsl,imx7ulp-lpi2c" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, lpi2c_imx_of_match)
> +
> +static int lpi2c_imx_probe(struct platform_device *pdev) {
> + int irq, ret;
> + void __iomem *base;
> + struct resource *res;
> + struct lpi2c_imx_struct *lpi2c_imx;
> +
> + lpi2c_imx = devm_kzalloc(&pdev->dev,
> + sizeof(*lpi2c_imx), GFP_KERNEL);
> + if (!lpi2c_imx)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "can't get irq number\n");
> + return irq;
> + }
> +
> + lpi2c_imx->adapter.owner = THIS_MODULE;
> + lpi2c_imx->adapter.algo = &lpi2c_imx_algo;
> + lpi2c_imx->adapter.dev.parent = &pdev->dev;
> + lpi2c_imx->adapter.nr = pdev->id;
> + lpi2c_imx->base = base;
> + lpi2c_imx->adapter.dev.of_node = pdev->dev.of_node;
> + strlcpy(lpi2c_imx->adapter.name, pdev->name,
> + sizeof(lpi2c_imx->adapter.name));
> +
> + lpi2c_imx->per_clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(lpi2c_imx->per_clk)) {
> + dev_err(&pdev->dev, "can't get I2C peripheral clock\n");
> + return PTR_ERR(lpi2c_imx->per_clk);
> + }
> +
> + ret = of_property_read_u32(pdev->dev.of_node,
> + "clock-frequency", &lpi2c_imx->bitrate);
> + if (ret)
> + lpi2c_imx->bitrate = LPI2C_DEFAULT_RATE;
> +
> + ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr, 0,
> + pdev->name, lpi2c_imx);
> + if (ret) {
> + dev_err(&pdev->dev, "can't claim irq %d\n", irq);
> + goto ret;
> + }
> +
> + i2c_set_adapdata(&lpi2c_imx->adapter, lpi2c_imx);
> + platform_set_drvdata(pdev, lpi2c_imx);
> +
> + ret = i2c_add_numbered_adapter(&lpi2c_imx->adapter);
> + if (ret) {
> + dev_err(&pdev->dev, "registration failed\n");
> + goto ret;
> + }
> +
> + dev_info(&lpi2c_imx->adapter.dev, "LPI2C adapter registered\n");
> +
> +ret:
> + return ret;
> +}
> +
> +static int lpi2c_imx_remove(struct platform_device *pdev) {
> + struct lpi2c_imx_struct *lpi2c_imx = platform_get_drvdata(pdev);
> +
> + i2c_del_adapter(&lpi2c_imx->adapter);
> +
> + return 0;
> +}
> +
> +static struct platform_driver lpi2c_imx_driver = {
> + .probe = lpi2c_imx_probe,
> + .remove = lpi2c_imx_remove,
> + .driver = {
> + .name = DRIVER_NAME,
> + .of_match_table = lpi2c_imx_of_match,
> + },
> +};
> +
> +static int __init i2c_adap_imx_init(void) {
> + return platform_driver_register(&lpi2c_imx_driver);
> +}
> +module_init(i2c_adap_imx_init);
> +
> +static void __exit i2c_adap_imx_exit(void) {
> + platform_driver_unregister(&lpi2c_imx_driver);
> +}
> +module_exit(i2c_adap_imx_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Gao Pan <pandy.gao@nxp.com>");
> MODULE_DESCRIPTION("I2C
> +adapter driver for LPI2C bus"); MODULE_ALIAS("platform:" DRIVER_NAME);
> --
> 1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [Patch V3] i2c: imx: add low power i2c bus driver
2016-08-26 7:37 ` Pan Gao
@ 2016-10-24 5:51 ` Pandy Gao
0 siblings, 0 replies; 6+ messages in thread
From: Pandy Gao @ 2016-10-24 5:51 UTC (permalink / raw)
To: Pandy Gao, wsa, u.kleine-koenig, cmo; +Cc: linux-i2c, Frank Li, Andy Duan
Ping...
> -----Original Message-----
> From: linux-i2c-owner@vger.kernel.org [mailto:linux-i2c-
> owner@vger.kernel.org] On Behalf Of Pan Gao
> Sent: Friday, August 26, 2016 3:37 PM
> To: wsa@the-dreams.de; u.kleine-koenig@pengutronix.de; cmo@melexis.com
> Cc: linux-i2c@vger.kernel.org; Frank Li <frank.li@nxp.com>; Fugang Duan
> <fugang.duan@nxp.com>
> Subject: RE: [Patch V3] i2c: imx: add low power i2c bus driver
>
> Ping...
>
> > -----Original Message-----
> > From: Pan Gao
> > Sent: Wednesday, August 17, 2016 4:00 PM
> > To: wsa@the-dreams.de; u.kleine-koenig@pengutronix.de;
> cmo@melexis.com
> > Cc: linux-i2c@vger.kernel.org; Frank Li <frank.li@nxp.com>; Fugang
> > Duan <fugang.duan@nxp.com>; Pan Gao <pandy.gao@nxp.com>
> > Subject: [Patch V3] i2c: imx: add low power i2c bus driver
> >
> > This patch adds low power i2c bus driver to support new i.MX products
> > which use low power i2c instead of the old imx i2c.
> >
> > The low power i2c can continue operating in stop mode when an
> > appropriate clock is available. It is also designed for low CPU
> > overhead with DMA offloading of FIFO register accesses.
> >
> > Signed-off-by: Gao Pan <pandy.gao@nxp.com>
> > Reviewed-by: Fugang Duan <B38611@freescale.com>
> > ---
> > V2:
> > -stop i2c transfer under the wrong condition -add timeout check in
> > while() domain
> >
> > V3:
> > -fix typo inside commit message and the driver.
> >
> > .../devicetree/bindings/i2c/i2c-imx-lpi2c.txt | 25 +
> > drivers/i2c/busses/Kconfig | 10 +
> > drivers/i2c/busses/Makefile | 1 +
> > drivers/i2c/busses/i2c-imx-lpi2c.c | 667 +++++++++++++++++++++
> > 4 files changed, 703 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> > b/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> > new file mode 100644
> > index 0000000..1f10cbf
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> > @@ -0,0 +1,25 @@
> > +* Freescale Low Power Inter IC (LPI2C) for i.MX
> > +
> > +Required properties:
> > +- compatible :
> > + - "fsl,imx8dv-lpi2c" for LPI2C compatible with the one integrated
> > +on i.MX8DV soc
> > + - "fsl,imx7ulp-lpi2c" for LPI2C compatible with the one integrated
> > +on i.MX7ULP soc
> > +- reg : Should contain LPI2C registers location and length
> > +- interrupts : Should contain LPI2C interrupt
> > +- clocks : Should contain LPI2C clock specifier
> > +- power-domains : should contain LPI2C power domain
> > +
> > +Optional properties:
> > +- clock-frequency : Constains desired LPI2C bus clock frequency in Hz.
> > + The absence of the property indicates the default frequency 100 kHz.
> > +
> > +Examples:
> > +
> > +i2c1: i2c@5e110000 { /* LPI2C on i.MX8DV */
> > + compatible = "fsl,imx8dv-lpi2c";
> > + reg = <0x0 0x5e110000 0x0 0x4000>;
> > + interrupts = <0 88 4>;
> > + clocks = <&clk IMX8DV_I2C1_CLK>;
> > + clock-names = "per";
> > + power-domains = <&pd_lsio_i2c1>;
> > +};
> > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > index
> > efa3d9b..1fc7a10 100644
> > --- a/drivers/i2c/busses/Kconfig
> > +++ b/drivers/i2c/busses/Kconfig
> > @@ -596,6 +596,16 @@ config I2C_IMX
> > This driver can also be built as a module. If so, the module
> > will be called i2c-imx.
> >
> > +config I2C_IMX_LPI2C
> > + tristate "IMX Low Power I2C interface"
> > + depends on ARCH_MXC || COMPILE_TEST
> > + help
> > + Say Y here if you want to use the Low Power IIC bus controller
> > + on the Freescale i.MX processors.
> > +
> > + This driver can also be built as a module. If so, the module
> > + will be called i2c-imx-lpi2c.
> > +
> > config I2C_IOP3XX
> > tristate "Intel IOPx3xx and IXP4xx on-chip I2C interface"
> > depends on ARCH_IOP32X || ARCH_IOP33X || ARCH_IXP4XX ||
> ARCH_IOP13XX
> > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> > index 37f2819..cc93457 100644
> > --- a/drivers/i2c/busses/Makefile
> > +++ b/drivers/i2c/busses/Makefile
> > @@ -56,6 +56,7 @@ obj-$(CONFIG_I2C_HIX5HD2) += i2c-hix5hd2.o
> > obj-$(CONFIG_I2C_IBM_IIC) += i2c-ibm_iic.o
> > obj-$(CONFIG_I2C_IMG) += i2c-img-scb.o
> > obj-$(CONFIG_I2C_IMX) += i2c-imx.o
> > +obj-$(CONFIG_I2C_IMX_LPI2C) += i2c-imx-lpi2c.o
> > obj-$(CONFIG_I2C_IOP3XX) += i2c-iop3xx.o
> > obj-$(CONFIG_I2C_JZ4780) += i2c-jz4780.o
> > obj-$(CONFIG_I2C_KEMPLD) += i2c-kempld.o
> > diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c
> > b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > new file mode 100644
> > index 0000000..308ecf5
> > --- /dev/null
> > +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > @@ -0,0 +1,667 @@
> > +/*
> > + * This is i.MX low power i2c controller driver.
> > + *
> > + * Copyright 2016 Freescale Semiconductor, Inc.
> > + *
> > + * 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; either version 2
> > + * of the License, or (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/completion.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/errno.h>
> > +#include <linux/i2c.h>
> > +#include <linux/init.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/sched.h>
> > +#include <linux/slab.h>
> > +
> > +#define DRIVER_NAME "imx-lpi2c"
> > +
> > +#define LPI2C_PARAM 0x04 /* i2c RX/TX FIFO size */
> > +#define LPI2C_MCR 0x10 /* i2c contrl register */
> > +#define LPI2C_MSR 0x14 /* i2c status register */
> > +#define LPI2C_MIER 0x18 /* i2c interrupt enable */
> > +#define LPI2C_MCFGR0 0x20 /* i2c master configuration */
> > +#define LPI2C_MCFGR1 0x24 /* i2c master configuration */
> > +#define LPI2C_MCFGR2 0x28 /* i2c master configuration */
> > +#define LPI2C_MCFGR3 0x2C /* i2c master configuration */
> > +#define LPI2C_MCCR0 0x48 /* i2c master clk configuration */
> > +#define LPI2C_MCCR1 0x50 /* i2c master clk configuration */
> > +#define LPI2C_MFCR 0x58 /* i2c master FIFO control */
> > +#define LPI2C_MFSR 0x5C /* i2c master FIFO status */
> > +#define LPI2C_MTDR 0x60 /* i2c master TX data register */
> > +#define LPI2C_MRDR 0x70 /* i2c master RX data register */
> > +
> > +/* i2c command */
> > +#define TRAN_DATA 0X00
> > +#define RECV_DATA 0X01
> > +#define GEN_STOP 0X02
> > +#define RECV_DISCARD 0X03
> > +#define GEN_START 0X04
> > +#define START_NACK 0X05
> > +#define START_HIGH 0X06
> > +#define START_HIGH_NACK 0X07
> > +
> > +#define MCR_MEN (1 << 0)
> > +#define MCR_RST (1 << 1)
> > +#define MCR_DOZEN (1 << 2)
> > +#define MCR_DBGEN (1 << 3)
> > +#define MCR_RTF (1 << 8)
> > +#define MCR_RRF (1 << 9)
> > +#define MSR_TDF (1 << 0)
> > +#define MSR_RDF (1 << 1)
> > +#define MSR_SDF (1 << 9)
> > +#define MSR_NDF (1 << 10)
> > +#define MSR_ALF (1 << 11)
> > +#define MSR_MBF (1 << 24)
> > +#define MSR_BBF (1 << 25)
> > +#define MIER_TDIE (1 << 0)
> > +#define MIER_RDIE (1 << 1)
> > +#define MIER_SDIE (1 << 9)
> > +#define MIER_NDIE (1 << 10)
> > +#define MCFGR1_AUTOSTOP (1 << 8)
> > +#define MCFGR1_IGNACK (1 << 9)
> > +#define MRDR_RXEMPTY (1 << 14)
> > +
> > +#define I2C_CLK_RATIO 2
> > +#define CHUNK_DATA 256
> > +
> > +#define LPI2C_RX_FIFOSIZE 4
> > +#define LPI2C_TX_FIFOSIZE 4
> > +
> > +#define LPI2C_DEFAULT_RATE 100000
> > +#define STARDARD_MAX_BITRATE 400000
> > +#define FAST_MAX_BITRATE 1000000
> > +#define FAST_PLUS_MAX_BITRATE 3400000
> > +#define HIGHSPEED_MAX_BITRATE 5000000
> > +
> > +
> > +enum lpi2c_imx_mode {
> > + STANDARD, /* 100+Kbps */
> > + FAST, /* 400+Kbps */
> > + FAST_PLUS, /* 1.0+Mbps */
> > + ULTRA_FAST, /* 5.0+Mbps */
> > + HS, /* 3.4+Mbps */
> > +};
> > +
> > +enum lpi2c_imx_pincfg {
> > + TWO_PIN_OD, /* 2-pin open drain mode */
> > + TWO_PIN_OO, /* 2-pin output only mode (utra-fast mode) */
> > + TWO_PIN_PP, /* 2-pin push-pull mode */
> > + FOUR_PIN_PP, /* 4-pin push-pull mode */
> > + TWO_PIN_OD_SS, /* 2-pin open drain mode with separate slave
> */
> > + TWO_PIN_OO_SS, /* 2-pin output only mode with separate slave
> */
> > + TWO_PIN_PP_SS, /* 2-pin push-pull mode with separate slave */
> > + FOUR_PIN_PP_IO, /* 4-pin push-pull mode (inverted output) */
> > +};
> > +
> > +struct lpi2c_imx_clkcfg {
> > + u8 prescale;
> > + u8 filtscl;
> > + u8 filtsda;
> > + u8 sethold;
> > + u8 clklo;
> > + u8 clkhi;
> > + u8 datavd;
> > +};
> > +
> > +struct lpi2c_imx_struct {
> > + struct i2c_adapter adapter;
> > + struct clk *per_clk;
> > + void __iomem *base;
> > + __u8 *rx_buf;
> > + __u8 *tx_buf;
> > + struct completion complete;
> > + unsigned int msglen;
> > + unsigned int delivered;
> > + unsigned int block_data;
> > + unsigned int bitrate;
> > + enum lpi2c_imx_mode mode;
> > +};
> > +
> > +static void lpi2c_imx_intctrl(
> > + struct lpi2c_imx_struct *lpi2c_imx, unsigned int enable) {
> > + writel(enable, lpi2c_imx->base + LPI2C_MIER); }
> > +
> > +static int lpi2c_imx_bus_busy(struct lpi2c_imx_struct *lpi2c_imx) {
> > + unsigned long orig_jiffies = jiffies;
> > + unsigned int temp;
> > +
> > + while (1) {
> > + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> > +
> > + /* check for arbitration lost, clear if set */
> > + if (temp & MSR_ALF) {
> > + writel(temp, lpi2c_imx->base + LPI2C_MSR);
> > + return -EAGAIN;
> > + }
> > +
> > + if ((temp & MSR_BBF) && (temp & MSR_MBF))
> > + break;
> > +
> > + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> > + dev_dbg(&lpi2c_imx->adapter.dev, "bus not work\n");
> > + return -ETIMEDOUT;
> > + }
> > + schedule();
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void lpi2c_imx_set_mode(struct lpi2c_imx_struct *lpi2c_imx) {
> > + enum lpi2c_imx_mode mode;
> > + unsigned int bitrate = lpi2c_imx->bitrate;
> > +
> > + if (bitrate < STARDARD_MAX_BITRATE)
> > + mode = STANDARD;
> > + else if (bitrate < FAST_MAX_BITRATE)
> > + mode = FAST;
> > + else if (bitrate < FAST_PLUS_MAX_BITRATE)
> > + mode = FAST_PLUS;
> > + else if (bitrate < HIGHSPEED_MAX_BITRATE)
> > + mode = HS;
> > + else
> > + mode = ULTRA_FAST;
> > +
> > + lpi2c_imx->mode = mode;
> > +}
> > +
> > +static int lpi2c_imx_start(struct lpi2c_imx_struct *lpi2c_imx,
> > + struct i2c_msg *msgs)
> > +{
> > + u8 read;
> > + unsigned int temp;
> > +
> > + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> > + temp |= MCR_RRF | MCR_RTF;
> > + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> > + writel(0x7f00, lpi2c_imx->base + LPI2C_MSR);
> > +
> > + read = msgs->flags & I2C_M_RD;
> > + temp = (msgs->addr << 1 | read) | (GEN_START << 8);
> > + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> > +
> > + return lpi2c_imx_bus_busy(lpi2c_imx); }
> > +
> > +static void lpi2c_imx_stop(struct lpi2c_imx_struct *lpi2c_imx) {
> > + unsigned int temp;
> > + unsigned long orig_jiffies = jiffies;
> > +
> > + writel(GEN_STOP << 8, lpi2c_imx->base + LPI2C_MTDR);
> > + do {
> > + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> > + if (temp & MSR_SDF)
> > + break;
> > +
> > + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> > + dev_dbg(&lpi2c_imx->adapter.dev, "stop timeout\n");
> > + break;
> > + }
> > + schedule();
> > +
> > + } while (1);
> > +}
> > +
> > +
> > +/* CLKLO = I2C_CLK_RATIO * CLKHI, SETHOLD = CLKHI, DATAVD = CLKHI/2
> > +*/ static int lpi2c_imx_config(struct lpi2c_imx_struct *lpi2c_imx) {
> > + unsigned int temp;
> > + unsigned int per_clk_rate;
> > + unsigned int prescale, clk_high, clk_low, clk_cycle;
> > + enum lpi2c_imx_pincfg pincfg;
> > + struct lpi2c_imx_clkcfg clkcfg;
> > +
> > + lpi2c_imx_set_mode(lpi2c_imx);
> > + per_clk_rate = clk_get_rate(lpi2c_imx->per_clk);
> > +
> > + if (lpi2c_imx->mode == HS || lpi2c_imx->mode == ULTRA_FAST)
> > + clkcfg.filtscl = clkcfg.filtsda = 0;
> > + else
> > + clkcfg.filtscl = clkcfg.filtsda = 2;
> > +
> > + for (prescale = 0; prescale <= 7; prescale++) {
> > + clk_cycle = per_clk_rate / ((1 << prescale) * lpi2c_imx->bitrate)
> > + - 3 - (clkcfg.filtscl >> 1);
> > + clk_high = (clk_cycle + I2C_CLK_RATIO) / (I2C_CLK_RATIO + 1);
> > + clk_low = clk_cycle - clk_high;
> > + if (clk_low < 64)
> > + break;
> > + }
> > +
> > + if (prescale > 7)
> > + return -EINVAL;
> > +
> > + clkcfg.prescale = prescale;
> > + clkcfg.sethold = clk_high;
> > + clkcfg.clklo = clk_low;
> > + clkcfg.clkhi = clk_high;
> > + clkcfg.datavd = clk_high >> 1;
> > +
> > + /* set MCFGR1: PINCFG, PRESCALE, IGNACK */
> > + if (lpi2c_imx->mode == ULTRA_FAST)
> > + pincfg = TWO_PIN_OO;
> > + else
> > + pincfg = TWO_PIN_OD;
> > + temp = clkcfg.prescale | pincfg << 24;
> > +
> > + if (lpi2c_imx->mode == ULTRA_FAST)
> > + temp |= MCFGR1_IGNACK;
> > +
> > + writel(temp, lpi2c_imx->base + LPI2C_MCFGR1);
> > +
> > + /* set MCFGR2: FILTSDA, FILTSCL */
> > + temp = (clkcfg.filtscl << 16) | (clkcfg.filtsda << 24);
> > + writel(temp, lpi2c_imx->base + LPI2C_MCFGR2);
> > +
> > +
> > + /* set MCCR: DATAVD, SETHOLD, CLKHI, CLKLO */
> > + temp = clkcfg.datavd << 24 | clkcfg.sethold << 16 |
> > + clkcfg.clkhi << 8 | clkcfg.clklo;
> > +
> > + if (lpi2c_imx->mode == HS)
> > + writel(temp, lpi2c_imx->base + LPI2C_MCCR1);
> > + else
> > + writel(temp, lpi2c_imx->base + LPI2C_MCCR0);
> > +
> > + return 0;
> > +}
> > +
> > +static int lpi2c_imx_master_enable(struct lpi2c_imx_struct
> > +*lpi2c_imx) {
> > + int ret;
> > + unsigned int temp;
> > +
> > + ret = clk_prepare_enable(lpi2c_imx->per_clk);
> > + if (ret)
> > + return ret;
> > +
> > + temp = MCR_RST;
> > + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> > + writel(0, lpi2c_imx->base + LPI2C_MCR);
> > +
> > + ret = lpi2c_imx_config(lpi2c_imx);
> > + if (ret)
> > + return ret;
> > +
> > + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> > + temp |= MCR_MEN;
> > + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> > +
> > + return 0;
> > +}
> > +
> > +static int lpi2c_imx_master_disable(struct lpi2c_imx_struct
> > +*lpi2c_imx) {
> > + unsigned int temp = 0;
> > +
> > + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> > + temp &= ~MCR_MEN;
> > + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> > +
> > + clk_disable_unprepare(lpi2c_imx->per_clk);
> > +
> > + return 0;
> > +}
> > +
> > +static int lpi2c_imx_msg_complete(struct lpi2c_imx_struct *lpi2c_imx) {
> > + unsigned int timeout;
> > +
> > + timeout = wait_for_completion_timeout(&lpi2c_imx->complete, HZ);
> > +
> > + return timeout ? 0 : -ETIMEDOUT;
> > +}
> > +
> > +static int lpi2c_imx_txfifo_empty(struct lpi2c_imx_struct *lpi2c_imx) {
> > + u32 txcnt;
> > + unsigned long orig_jiffies = jiffies;
> > +
> > + do {
> > + txcnt = readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff;
> > +
> > + if (readl(lpi2c_imx->base + LPI2C_MSR) & MSR_NDF) {
> > + dev_dbg(&lpi2c_imx->adapter.dev, "NDF detected\n");
> > + return -EIO;
> > + }
> > +
> > + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> > + dev_dbg(&lpi2c_imx->adapter.dev, "txfifo empty
> > timeout\n");
> > + return -ETIMEDOUT;
> > + }
> > + schedule();
> > +
> > + } while (txcnt);
> > +
> > + return 0;
> > +}
> > +
> > +static void lpi2c_imx_set_tx_watermark(struct lpi2c_imx_struct
> > +*lpi2c_imx) {
> > + unsigned int temp;
> > +
> > + temp = LPI2C_TX_FIFOSIZE >> 1;
> > + writel(temp, lpi2c_imx->base + LPI2C_MFCR); }
> > +
> > +static void lpi2c_imx_set_rx_watermark(struct lpi2c_imx_struct
> > +*lpi2c_imx) {
> > + unsigned int temp, remaining;
> > +
> > + remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
> > +
> > + if (remaining > (LPI2C_RX_FIFOSIZE >> 1))
> > + temp = LPI2C_RX_FIFOSIZE >> 1;
> > + else
> > + temp = 0;
> > +
> > + writel(temp << 16, lpi2c_imx->base + LPI2C_MFCR); }
> > +
> > +static void lpi2c_imx_write_txfifo(struct lpi2c_imx_struct
> > +*lpi2c_imx) {
> > + unsigned int data, txcnt;
> > +
> > + txcnt = readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff;
> > + while (txcnt < LPI2C_TX_FIFOSIZE) {
> > + if (lpi2c_imx->delivered == lpi2c_imx->msglen)
> > + break;
> > + data = lpi2c_imx->tx_buf[lpi2c_imx->delivered++];
> > + writel(data, lpi2c_imx->base + LPI2C_MTDR);
> > + txcnt++;
> > + }
> > +
> > + if (lpi2c_imx->delivered < lpi2c_imx->msglen)
> > + lpi2c_imx_intctrl(lpi2c_imx, MIER_TDIE | MIER_NDIE);
> > + else
> > + complete(&lpi2c_imx->complete);
> > +}
> > +
> > +static void lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx) {
> > + unsigned int temp, data;
> > + unsigned int blocklen, remaining;
> > +
> > + do {
> > + data = readl(lpi2c_imx->base + LPI2C_MRDR);
> > + if (data & MRDR_RXEMPTY)
> > + break;
> > + lpi2c_imx->rx_buf[lpi2c_imx->delivered++] = data & 0xff;
> > + } while (1);
> > +
> > + /*
> > + * First byte is the length of remaining packet in the SMBus block
> > + * data read. Add it to msgs->len.
> > + */
> > + if (lpi2c_imx->block_data) {
> > + blocklen = lpi2c_imx->rx_buf[0];
> > + lpi2c_imx->msglen += blocklen;
> > + }
> > +
> > + remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
> > + /* not finished, still waiting for rx data */
> > + if (remaining) {
> > + lpi2c_imx_set_rx_watermark(lpi2c_imx);
> > + /* multiple receive commands */
> > + if (lpi2c_imx->block_data) {
> > + lpi2c_imx->block_data = 0;
> > + temp = remaining;
> > + temp |= (RECV_DATA << 8);
> > + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> > + } else if (!(lpi2c_imx->delivered & 0xff)) {
> > + temp = remaining > CHUNK_DATA ?
> > + CHUNK_DATA - 1 : (remaining - 1);
> > + temp |= (RECV_DATA << 8);
> > + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> > + }
> > +
> > + lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE);
> > + } else
> > + complete(&lpi2c_imx->complete);
> > +}
> > +
> > +static void lpi2c_imx_write(struct lpi2c_imx_struct *lpi2c_imx,
> > + struct i2c_msg *msgs)
> > +{
> > + lpi2c_imx->tx_buf = msgs->buf;
> > + lpi2c_imx_set_tx_watermark(lpi2c_imx);
> > + lpi2c_imx_write_txfifo(lpi2c_imx);
> > +}
> > +
> > +static void lpi2c_imx_read(struct lpi2c_imx_struct *lpi2c_imx,
> > + struct i2c_msg *msgs)
> > +{
> > + unsigned int temp;
> > +
> > + lpi2c_imx->rx_buf = msgs->buf;
> > + lpi2c_imx->block_data = msgs->flags & I2C_M_RECV_LEN;
> > +
> > + lpi2c_imx_set_rx_watermark(lpi2c_imx);
> > + temp = msgs->len > CHUNK_DATA ? CHUNK_DATA - 1 : msgs->len - 1;
> > + temp |= (RECV_DATA << 8);
> > + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> > +
> > + lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE | MIER_NDIE); }
> > +
> > +static int lpi2c_imx_xfer(struct i2c_adapter *adapter,
> > + struct i2c_msg *msgs, int num)
> > +{
> > + int i, result;
> > + unsigned int temp;
> > + struct lpi2c_imx_struct *lpi2c_imx = i2c_get_adapdata(adapter);
> > +
> > + result = lpi2c_imx_master_enable(lpi2c_imx);
> > + if (result)
> > + return result;
> > +
> > + for (i = 0; i < num; i++) {
> > + result = lpi2c_imx_start(lpi2c_imx, &msgs[i]);
> > + if (result)
> > + goto disable;
> > +
> > + /* quick smbus */
> > + if (num == 1 && msgs[0].len == 0)
> > + goto stop;
> > +
> > + lpi2c_imx->delivered = 0;
> > + lpi2c_imx->msglen = msgs[i].len;
> > + init_completion(&lpi2c_imx->complete);
> > +
> > + if (msgs[i].flags & I2C_M_RD)
> > + lpi2c_imx_read(lpi2c_imx, &msgs[i]);
> > + else
> > + lpi2c_imx_write(lpi2c_imx, &msgs[i]);
> > +
> > + result = lpi2c_imx_msg_complete(lpi2c_imx);
> > + if (result)
> > + goto stop;
> > +
> > + if (!(msgs[i].flags & I2C_M_RD)) {
> > + result = lpi2c_imx_txfifo_empty(lpi2c_imx);
> > + if (result)
> > + goto stop;
> > + }
> > + }
> > +
> > +stop:
> > + lpi2c_imx_stop(lpi2c_imx);
> > +
> > + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> > + if ((temp & MSR_NDF) && !result)
> > + result = -EIO;
> > +
> > +disable:
> > + lpi2c_imx_master_disable(lpi2c_imx);
> > +
> > + dev_dbg(&lpi2c_imx->adapter.dev, "<%s> exit with: %s: %d\n",
> __func__,
> > + (result < 0) ? "error" : "success msg",
> > + (result < 0) ? result : num);
> > +
> > + return (result < 0) ? result : num;
> > +}
> > +
> > +static irqreturn_t lpi2c_imx_isr(int irq, void *dev_id) {
> > + unsigned int temp;
> > + struct lpi2c_imx_struct *lpi2c_imx = dev_id;
> > +
> > + lpi2c_imx_intctrl(lpi2c_imx, 0);
> > + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> > +
> > + if (temp & MSR_RDF) {
> > + lpi2c_imx_read_rxfifo(lpi2c_imx);
> > + return IRQ_HANDLED;
> > + }
> > +
> > + if (temp & MSR_TDF) {
> > + lpi2c_imx_write_txfifo(lpi2c_imx);
> > + return IRQ_HANDLED;
> > + }
> > +
> > + complete(&lpi2c_imx->complete);
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static u32 lpi2c_imx_func(struct i2c_adapter *adapter) {
> > + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL
> > + | I2C_FUNC_SMBUS_READ_BLOCK_DATA;
> > +}
> > +
> > +static struct i2c_algorithm lpi2c_imx_algo = {
> > + .master_xfer = lpi2c_imx_xfer,
> > + .functionality = lpi2c_imx_func,
> > +};
> > +
> > +static const struct of_device_id lpi2c_imx_of_match[] = {
> > + { .compatible = "fsl,imx8dv-lpi2c" },
> > + { .compatible = "fsl,imx7ulp-lpi2c" },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, lpi2c_imx_of_match)
> > +
> > +static int lpi2c_imx_probe(struct platform_device *pdev) {
> > + int irq, ret;
> > + void __iomem *base;
> > + struct resource *res;
> > + struct lpi2c_imx_struct *lpi2c_imx;
> > +
> > + lpi2c_imx = devm_kzalloc(&pdev->dev,
> > + sizeof(*lpi2c_imx), GFP_KERNEL);
> > + if (!lpi2c_imx)
> > + return -ENOMEM;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + base = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(base))
> > + return PTR_ERR(base);
> > +
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0) {
> > + dev_err(&pdev->dev, "can't get irq number\n");
> > + return irq;
> > + }
> > +
> > + lpi2c_imx->adapter.owner = THIS_MODULE;
> > + lpi2c_imx->adapter.algo = &lpi2c_imx_algo;
> > + lpi2c_imx->adapter.dev.parent = &pdev->dev;
> > + lpi2c_imx->adapter.nr = pdev->id;
> > + lpi2c_imx->base = base;
> > + lpi2c_imx->adapter.dev.of_node = pdev->dev.of_node;
> > + strlcpy(lpi2c_imx->adapter.name, pdev->name,
> > + sizeof(lpi2c_imx->adapter.name));
> > +
> > + lpi2c_imx->per_clk = devm_clk_get(&pdev->dev, NULL);
> > + if (IS_ERR(lpi2c_imx->per_clk)) {
> > + dev_err(&pdev->dev, "can't get I2C peripheral clock\n");
> > + return PTR_ERR(lpi2c_imx->per_clk);
> > + }
> > +
> > + ret = of_property_read_u32(pdev->dev.of_node,
> > + "clock-frequency", &lpi2c_imx->bitrate);
> > + if (ret)
> > + lpi2c_imx->bitrate = LPI2C_DEFAULT_RATE;
> > +
> > + ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr, 0,
> > + pdev->name, lpi2c_imx);
> > + if (ret) {
> > + dev_err(&pdev->dev, "can't claim irq %d\n", irq);
> > + goto ret;
> > + }
> > +
> > + i2c_set_adapdata(&lpi2c_imx->adapter, lpi2c_imx);
> > + platform_set_drvdata(pdev, lpi2c_imx);
> > +
> > + ret = i2c_add_numbered_adapter(&lpi2c_imx->adapter);
> > + if (ret) {
> > + dev_err(&pdev->dev, "registration failed\n");
> > + goto ret;
> > + }
> > +
> > + dev_info(&lpi2c_imx->adapter.dev, "LPI2C adapter registered\n");
> > +
> > +ret:
> > + return ret;
> > +}
> > +
> > +static int lpi2c_imx_remove(struct platform_device *pdev) {
> > + struct lpi2c_imx_struct *lpi2c_imx = platform_get_drvdata(pdev);
> > +
> > + i2c_del_adapter(&lpi2c_imx->adapter);
> > +
> > + return 0;
> > +}
> > +
> > +static struct platform_driver lpi2c_imx_driver = {
> > + .probe = lpi2c_imx_probe,
> > + .remove = lpi2c_imx_remove,
> > + .driver = {
> > + .name = DRIVER_NAME,
> > + .of_match_table = lpi2c_imx_of_match,
> > + },
> > +};
> > +
> > +static int __init i2c_adap_imx_init(void) {
> > + return platform_driver_register(&lpi2c_imx_driver);
> > +}
> > +module_init(i2c_adap_imx_init);
> > +
> > +static void __exit i2c_adap_imx_exit(void) {
> > + platform_driver_unregister(&lpi2c_imx_driver);
> > +}
> > +module_exit(i2c_adap_imx_exit);
> > +
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_AUTHOR("Gao Pan <pandy.gao@nxp.com>");
> > MODULE_DESCRIPTION("I2C
> > +adapter driver for LPI2C bus"); MODULE_ALIAS("platform:"
> > +DRIVER_NAME);
> > --
> > 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body
> of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [Patch V3] i2c: imx: add low power i2c bus driver
2016-08-17 7:59 [Patch V3] i2c: imx: add low power i2c bus driver Gao Pan
2016-08-26 7:37 ` Pan Gao
@ 2016-09-09 1:49 ` Pandy Gao
2016-10-24 23:15 ` Vladimir Zapolskiy
2 siblings, 0 replies; 6+ messages in thread
From: Pandy Gao @ 2016-09-09 1:49 UTC (permalink / raw)
To: wsa, u.kleine-koenig, cmo; +Cc: linux-i2c, Frank Li, Andy Duan
Ping...
> -----Original Message-----
> From: Pan Gao
> Sent: Wednesday, August 17, 2016 4:00 PM
> To: wsa@the-dreams.de; u.kleine-koenig@pengutronix.de; cmo@melexis.com
> Cc: linux-i2c@vger.kernel.org; Frank Li <frank.li@nxp.com>; Fugang Duan
> <fugang.duan@nxp.com>; Pan Gao <pandy.gao@nxp.com>
> Subject: [Patch V3] i2c: imx: add low power i2c bus driver
>
> This patch adds low power i2c bus driver to support new i.MX products which use
> low power i2c instead of the old imx i2c.
>
> The low power i2c can continue operating in stop mode when an appropriate
> clock is available. It is also designed for low CPU overhead with DMA offloading of
> FIFO register accesses.
>
> Signed-off-by: Gao Pan <pandy.gao@nxp.com>
> Reviewed-by: Fugang Duan <B38611@freescale.com>
> ---
> V2:
> -stop i2c transfer under the wrong condition -add timeout check in while()
> domain
>
> V3:
> -fix typo inside commit message and the driver.
>
> .../devicetree/bindings/i2c/i2c-imx-lpi2c.txt | 25 +
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-imx-lpi2c.c | 667 +++++++++++++++++++++
> 4 files changed, 703 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> b/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> new file mode 100644
> index 0000000..1f10cbf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> @@ -0,0 +1,25 @@
> +* Freescale Low Power Inter IC (LPI2C) for i.MX
> +
> +Required properties:
> +- compatible :
> + - "fsl,imx8dv-lpi2c" for LPI2C compatible with the one integrated on
> +i.MX8DV soc
> + - "fsl,imx7ulp-lpi2c" for LPI2C compatible with the one integrated on
> +i.MX7ULP soc
> +- reg : Should contain LPI2C registers location and length
> +- interrupts : Should contain LPI2C interrupt
> +- clocks : Should contain LPI2C clock specifier
> +- power-domains : should contain LPI2C power domain
> +
> +Optional properties:
> +- clock-frequency : Constains desired LPI2C bus clock frequency in Hz.
> + The absence of the property indicates the default frequency 100 kHz.
> +
> +Examples:
> +
> +i2c1: i2c@5e110000 { /* LPI2C on i.MX8DV */
> + compatible = "fsl,imx8dv-lpi2c";
> + reg = <0x0 0x5e110000 0x0 0x4000>;
> + interrupts = <0 88 4>;
> + clocks = <&clk IMX8DV_I2C1_CLK>;
> + clock-names = "per";
> + power-domains = <&pd_lsio_i2c1>;
> +};
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index
> efa3d9b..1fc7a10 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -596,6 +596,16 @@ config I2C_IMX
> This driver can also be built as a module. If so, the module
> will be called i2c-imx.
>
> +config I2C_IMX_LPI2C
> + tristate "IMX Low Power I2C interface"
> + depends on ARCH_MXC || COMPILE_TEST
> + help
> + Say Y here if you want to use the Low Power IIC bus controller
> + on the Freescale i.MX processors.
> +
> + This driver can also be built as a module. If so, the module
> + will be called i2c-imx-lpi2c.
> +
> config I2C_IOP3XX
> tristate "Intel IOPx3xx and IXP4xx on-chip I2C interface"
> depends on ARCH_IOP32X || ARCH_IOP33X || ARCH_IXP4XX ||
> ARCH_IOP13XX diff --git a/drivers/i2c/busses/Makefile
> b/drivers/i2c/busses/Makefile index 37f2819..cc93457 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_I2C_HIX5HD2) += i2c-hix5hd2.o
> obj-$(CONFIG_I2C_IBM_IIC) += i2c-ibm_iic.o
> obj-$(CONFIG_I2C_IMG) += i2c-img-scb.o
> obj-$(CONFIG_I2C_IMX) += i2c-imx.o
> +obj-$(CONFIG_I2C_IMX_LPI2C) += i2c-imx-lpi2c.o
> obj-$(CONFIG_I2C_IOP3XX) += i2c-iop3xx.o
> obj-$(CONFIG_I2C_JZ4780) += i2c-jz4780.o
> obj-$(CONFIG_I2C_KEMPLD) += i2c-kempld.o
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> new file mode 100644
> index 0000000..308ecf5
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -0,0 +1,667 @@
> +/*
> + * This is i.MX low power i2c controller driver.
> + *
> + * Copyright 2016 Freescale Semiconductor, Inc.
> + *
> + * 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; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +
> +#define DRIVER_NAME "imx-lpi2c"
> +
> +#define LPI2C_PARAM 0x04 /* i2c RX/TX FIFO size */
> +#define LPI2C_MCR 0x10 /* i2c contrl register */
> +#define LPI2C_MSR 0x14 /* i2c status register */
> +#define LPI2C_MIER 0x18 /* i2c interrupt enable */
> +#define LPI2C_MCFGR0 0x20 /* i2c master configuration */
> +#define LPI2C_MCFGR1 0x24 /* i2c master configuration */
> +#define LPI2C_MCFGR2 0x28 /* i2c master configuration */
> +#define LPI2C_MCFGR3 0x2C /* i2c master configuration */
> +#define LPI2C_MCCR0 0x48 /* i2c master clk configuration */
> +#define LPI2C_MCCR1 0x50 /* i2c master clk configuration */
> +#define LPI2C_MFCR 0x58 /* i2c master FIFO control */
> +#define LPI2C_MFSR 0x5C /* i2c master FIFO status */
> +#define LPI2C_MTDR 0x60 /* i2c master TX data register */
> +#define LPI2C_MRDR 0x70 /* i2c master RX data register */
> +
> +/* i2c command */
> +#define TRAN_DATA 0X00
> +#define RECV_DATA 0X01
> +#define GEN_STOP 0X02
> +#define RECV_DISCARD 0X03
> +#define GEN_START 0X04
> +#define START_NACK 0X05
> +#define START_HIGH 0X06
> +#define START_HIGH_NACK 0X07
> +
> +#define MCR_MEN (1 << 0)
> +#define MCR_RST (1 << 1)
> +#define MCR_DOZEN (1 << 2)
> +#define MCR_DBGEN (1 << 3)
> +#define MCR_RTF (1 << 8)
> +#define MCR_RRF (1 << 9)
> +#define MSR_TDF (1 << 0)
> +#define MSR_RDF (1 << 1)
> +#define MSR_SDF (1 << 9)
> +#define MSR_NDF (1 << 10)
> +#define MSR_ALF (1 << 11)
> +#define MSR_MBF (1 << 24)
> +#define MSR_BBF (1 << 25)
> +#define MIER_TDIE (1 << 0)
> +#define MIER_RDIE (1 << 1)
> +#define MIER_SDIE (1 << 9)
> +#define MIER_NDIE (1 << 10)
> +#define MCFGR1_AUTOSTOP (1 << 8)
> +#define MCFGR1_IGNACK (1 << 9)
> +#define MRDR_RXEMPTY (1 << 14)
> +
> +#define I2C_CLK_RATIO 2
> +#define CHUNK_DATA 256
> +
> +#define LPI2C_RX_FIFOSIZE 4
> +#define LPI2C_TX_FIFOSIZE 4
> +
> +#define LPI2C_DEFAULT_RATE 100000
> +#define STARDARD_MAX_BITRATE 400000
> +#define FAST_MAX_BITRATE 1000000
> +#define FAST_PLUS_MAX_BITRATE 3400000
> +#define HIGHSPEED_MAX_BITRATE 5000000
> +
> +
> +enum lpi2c_imx_mode {
> + STANDARD, /* 100+Kbps */
> + FAST, /* 400+Kbps */
> + FAST_PLUS, /* 1.0+Mbps */
> + ULTRA_FAST, /* 5.0+Mbps */
> + HS, /* 3.4+Mbps */
> +};
> +
> +enum lpi2c_imx_pincfg {
> + TWO_PIN_OD, /* 2-pin open drain mode */
> + TWO_PIN_OO, /* 2-pin output only mode (utra-fast mode) */
> + TWO_PIN_PP, /* 2-pin push-pull mode */
> + FOUR_PIN_PP, /* 4-pin push-pull mode */
> + TWO_PIN_OD_SS, /* 2-pin open drain mode with separate slave */
> + TWO_PIN_OO_SS, /* 2-pin output only mode with separate slave */
> + TWO_PIN_PP_SS, /* 2-pin push-pull mode with separate slave */
> + FOUR_PIN_PP_IO, /* 4-pin push-pull mode (inverted output) */
> +};
> +
> +struct lpi2c_imx_clkcfg {
> + u8 prescale;
> + u8 filtscl;
> + u8 filtsda;
> + u8 sethold;
> + u8 clklo;
> + u8 clkhi;
> + u8 datavd;
> +};
> +
> +struct lpi2c_imx_struct {
> + struct i2c_adapter adapter;
> + struct clk *per_clk;
> + void __iomem *base;
> + __u8 *rx_buf;
> + __u8 *tx_buf;
> + struct completion complete;
> + unsigned int msglen;
> + unsigned int delivered;
> + unsigned int block_data;
> + unsigned int bitrate;
> + enum lpi2c_imx_mode mode;
> +};
> +
> +static void lpi2c_imx_intctrl(
> + struct lpi2c_imx_struct *lpi2c_imx, unsigned int enable) {
> + writel(enable, lpi2c_imx->base + LPI2C_MIER); }
> +
> +static int lpi2c_imx_bus_busy(struct lpi2c_imx_struct *lpi2c_imx) {
> + unsigned long orig_jiffies = jiffies;
> + unsigned int temp;
> +
> + while (1) {
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> +
> + /* check for arbitration lost, clear if set */
> + if (temp & MSR_ALF) {
> + writel(temp, lpi2c_imx->base + LPI2C_MSR);
> + return -EAGAIN;
> + }
> +
> + if ((temp & MSR_BBF) && (temp & MSR_MBF))
> + break;
> +
> + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "bus not work\n");
> + return -ETIMEDOUT;
> + }
> + schedule();
> + }
> +
> + return 0;
> +}
> +
> +static void lpi2c_imx_set_mode(struct lpi2c_imx_struct *lpi2c_imx) {
> + enum lpi2c_imx_mode mode;
> + unsigned int bitrate = lpi2c_imx->bitrate;
> +
> + if (bitrate < STARDARD_MAX_BITRATE)
> + mode = STANDARD;
> + else if (bitrate < FAST_MAX_BITRATE)
> + mode = FAST;
> + else if (bitrate < FAST_PLUS_MAX_BITRATE)
> + mode = FAST_PLUS;
> + else if (bitrate < HIGHSPEED_MAX_BITRATE)
> + mode = HS;
> + else
> + mode = ULTRA_FAST;
> +
> + lpi2c_imx->mode = mode;
> +}
> +
> +static int lpi2c_imx_start(struct lpi2c_imx_struct *lpi2c_imx,
> + struct i2c_msg *msgs)
> +{
> + u8 read;
> + unsigned int temp;
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> + temp |= MCR_RRF | MCR_RTF;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> + writel(0x7f00, lpi2c_imx->base + LPI2C_MSR);
> +
> + read = msgs->flags & I2C_M_RD;
> + temp = (msgs->addr << 1 | read) | (GEN_START << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> +
> + return lpi2c_imx_bus_busy(lpi2c_imx);
> +}
> +
> +static void lpi2c_imx_stop(struct lpi2c_imx_struct *lpi2c_imx) {
> + unsigned int temp;
> + unsigned long orig_jiffies = jiffies;
> +
> + writel(GEN_STOP << 8, lpi2c_imx->base + LPI2C_MTDR);
> + do {
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> + if (temp & MSR_SDF)
> + break;
> +
> + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "stop timeout\n");
> + break;
> + }
> + schedule();
> +
> + } while (1);
> +}
> +
> +
> +/* CLKLO = I2C_CLK_RATIO * CLKHI, SETHOLD = CLKHI, DATAVD = CLKHI/2 */
> +static int lpi2c_imx_config(struct lpi2c_imx_struct *lpi2c_imx) {
> + unsigned int temp;
> + unsigned int per_clk_rate;
> + unsigned int prescale, clk_high, clk_low, clk_cycle;
> + enum lpi2c_imx_pincfg pincfg;
> + struct lpi2c_imx_clkcfg clkcfg;
> +
> + lpi2c_imx_set_mode(lpi2c_imx);
> + per_clk_rate = clk_get_rate(lpi2c_imx->per_clk);
> +
> + if (lpi2c_imx->mode == HS || lpi2c_imx->mode == ULTRA_FAST)
> + clkcfg.filtscl = clkcfg.filtsda = 0;
> + else
> + clkcfg.filtscl = clkcfg.filtsda = 2;
> +
> + for (prescale = 0; prescale <= 7; prescale++) {
> + clk_cycle = per_clk_rate / ((1 << prescale) * lpi2c_imx->bitrate)
> + - 3 - (clkcfg.filtscl >> 1);
> + clk_high = (clk_cycle + I2C_CLK_RATIO) / (I2C_CLK_RATIO + 1);
> + clk_low = clk_cycle - clk_high;
> + if (clk_low < 64)
> + break;
> + }
> +
> + if (prescale > 7)
> + return -EINVAL;
> +
> + clkcfg.prescale = prescale;
> + clkcfg.sethold = clk_high;
> + clkcfg.clklo = clk_low;
> + clkcfg.clkhi = clk_high;
> + clkcfg.datavd = clk_high >> 1;
> +
> + /* set MCFGR1: PINCFG, PRESCALE, IGNACK */
> + if (lpi2c_imx->mode == ULTRA_FAST)
> + pincfg = TWO_PIN_OO;
> + else
> + pincfg = TWO_PIN_OD;
> + temp = clkcfg.prescale | pincfg << 24;
> +
> + if (lpi2c_imx->mode == ULTRA_FAST)
> + temp |= MCFGR1_IGNACK;
> +
> + writel(temp, lpi2c_imx->base + LPI2C_MCFGR1);
> +
> + /* set MCFGR2: FILTSDA, FILTSCL */
> + temp = (clkcfg.filtscl << 16) | (clkcfg.filtsda << 24);
> + writel(temp, lpi2c_imx->base + LPI2C_MCFGR2);
> +
> +
> + /* set MCCR: DATAVD, SETHOLD, CLKHI, CLKLO */
> + temp = clkcfg.datavd << 24 | clkcfg.sethold << 16 |
> + clkcfg.clkhi << 8 | clkcfg.clklo;
> +
> + if (lpi2c_imx->mode == HS)
> + writel(temp, lpi2c_imx->base + LPI2C_MCCR1);
> + else
> + writel(temp, lpi2c_imx->base + LPI2C_MCCR0);
> +
> + return 0;
> +}
> +
> +static int lpi2c_imx_master_enable(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + int ret;
> + unsigned int temp;
> +
> + ret = clk_prepare_enable(lpi2c_imx->per_clk);
> + if (ret)
> + return ret;
> +
> + temp = MCR_RST;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> + writel(0, lpi2c_imx->base + LPI2C_MCR);
> +
> + ret = lpi2c_imx_config(lpi2c_imx);
> + if (ret)
> + return ret;
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> + temp |= MCR_MEN;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> +
> + return 0;
> +}
> +
> +static int lpi2c_imx_master_disable(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + unsigned int temp = 0;
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> + temp &= ~MCR_MEN;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> +
> + clk_disable_unprepare(lpi2c_imx->per_clk);
> +
> + return 0;
> +}
> +
> +static int lpi2c_imx_msg_complete(struct lpi2c_imx_struct *lpi2c_imx) {
> + unsigned int timeout;
> +
> + timeout = wait_for_completion_timeout(&lpi2c_imx->complete, HZ);
> +
> + return timeout ? 0 : -ETIMEDOUT;
> +}
> +
> +static int lpi2c_imx_txfifo_empty(struct lpi2c_imx_struct *lpi2c_imx) {
> + u32 txcnt;
> + unsigned long orig_jiffies = jiffies;
> +
> + do {
> + txcnt = readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff;
> +
> + if (readl(lpi2c_imx->base + LPI2C_MSR) & MSR_NDF) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "NDF detected\n");
> + return -EIO;
> + }
> +
> + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "txfifo empty
> timeout\n");
> + return -ETIMEDOUT;
> + }
> + schedule();
> +
> + } while (txcnt);
> +
> + return 0;
> +}
> +
> +static void lpi2c_imx_set_tx_watermark(struct lpi2c_imx_struct
> +*lpi2c_imx) {
> + unsigned int temp;
> +
> + temp = LPI2C_TX_FIFOSIZE >> 1;
> + writel(temp, lpi2c_imx->base + LPI2C_MFCR); }
> +
> +static void lpi2c_imx_set_rx_watermark(struct lpi2c_imx_struct
> +*lpi2c_imx) {
> + unsigned int temp, remaining;
> +
> + remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
> +
> + if (remaining > (LPI2C_RX_FIFOSIZE >> 1))
> + temp = LPI2C_RX_FIFOSIZE >> 1;
> + else
> + temp = 0;
> +
> + writel(temp << 16, lpi2c_imx->base + LPI2C_MFCR); }
> +
> +static void lpi2c_imx_write_txfifo(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + unsigned int data, txcnt;
> +
> + txcnt = readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff;
> + while (txcnt < LPI2C_TX_FIFOSIZE) {
> + if (lpi2c_imx->delivered == lpi2c_imx->msglen)
> + break;
> + data = lpi2c_imx->tx_buf[lpi2c_imx->delivered++];
> + writel(data, lpi2c_imx->base + LPI2C_MTDR);
> + txcnt++;
> + }
> +
> + if (lpi2c_imx->delivered < lpi2c_imx->msglen)
> + lpi2c_imx_intctrl(lpi2c_imx, MIER_TDIE | MIER_NDIE);
> + else
> + complete(&lpi2c_imx->complete);
> +}
> +
> +static void lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx) {
> + unsigned int temp, data;
> + unsigned int blocklen, remaining;
> +
> + do {
> + data = readl(lpi2c_imx->base + LPI2C_MRDR);
> + if (data & MRDR_RXEMPTY)
> + break;
> + lpi2c_imx->rx_buf[lpi2c_imx->delivered++] = data & 0xff;
> + } while (1);
> +
> + /*
> + * First byte is the length of remaining packet in the SMBus block
> + * data read. Add it to msgs->len.
> + */
> + if (lpi2c_imx->block_data) {
> + blocklen = lpi2c_imx->rx_buf[0];
> + lpi2c_imx->msglen += blocklen;
> + }
> +
> + remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
> + /* not finished, still waiting for rx data */
> + if (remaining) {
> + lpi2c_imx_set_rx_watermark(lpi2c_imx);
> + /* multiple receive commands */
> + if (lpi2c_imx->block_data) {
> + lpi2c_imx->block_data = 0;
> + temp = remaining;
> + temp |= (RECV_DATA << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> + } else if (!(lpi2c_imx->delivered & 0xff)) {
> + temp = remaining > CHUNK_DATA ?
> + CHUNK_DATA - 1 : (remaining - 1);
> + temp |= (RECV_DATA << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> + }
> +
> + lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE);
> + } else
> + complete(&lpi2c_imx->complete);
> +}
> +
> +static void lpi2c_imx_write(struct lpi2c_imx_struct *lpi2c_imx,
> + struct i2c_msg *msgs)
> +{
> + lpi2c_imx->tx_buf = msgs->buf;
> + lpi2c_imx_set_tx_watermark(lpi2c_imx);
> + lpi2c_imx_write_txfifo(lpi2c_imx);
> +}
> +
> +static void lpi2c_imx_read(struct lpi2c_imx_struct *lpi2c_imx,
> + struct i2c_msg *msgs)
> +{
> + unsigned int temp;
> +
> + lpi2c_imx->rx_buf = msgs->buf;
> + lpi2c_imx->block_data = msgs->flags & I2C_M_RECV_LEN;
> +
> + lpi2c_imx_set_rx_watermark(lpi2c_imx);
> + temp = msgs->len > CHUNK_DATA ? CHUNK_DATA - 1 : msgs->len - 1;
> + temp |= (RECV_DATA << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> +
> + lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE | MIER_NDIE); }
> +
> +static int lpi2c_imx_xfer(struct i2c_adapter *adapter,
> + struct i2c_msg *msgs, int num)
> +{
> + int i, result;
> + unsigned int temp;
> + struct lpi2c_imx_struct *lpi2c_imx = i2c_get_adapdata(adapter);
> +
> + result = lpi2c_imx_master_enable(lpi2c_imx);
> + if (result)
> + return result;
> +
> + for (i = 0; i < num; i++) {
> + result = lpi2c_imx_start(lpi2c_imx, &msgs[i]);
> + if (result)
> + goto disable;
> +
> + /* quick smbus */
> + if (num == 1 && msgs[0].len == 0)
> + goto stop;
> +
> + lpi2c_imx->delivered = 0;
> + lpi2c_imx->msglen = msgs[i].len;
> + init_completion(&lpi2c_imx->complete);
> +
> + if (msgs[i].flags & I2C_M_RD)
> + lpi2c_imx_read(lpi2c_imx, &msgs[i]);
> + else
> + lpi2c_imx_write(lpi2c_imx, &msgs[i]);
> +
> + result = lpi2c_imx_msg_complete(lpi2c_imx);
> + if (result)
> + goto stop;
> +
> + if (!(msgs[i].flags & I2C_M_RD)) {
> + result = lpi2c_imx_txfifo_empty(lpi2c_imx);
> + if (result)
> + goto stop;
> + }
> + }
> +
> +stop:
> + lpi2c_imx_stop(lpi2c_imx);
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> + if ((temp & MSR_NDF) && !result)
> + result = -EIO;
> +
> +disable:
> + lpi2c_imx_master_disable(lpi2c_imx);
> +
> + dev_dbg(&lpi2c_imx->adapter.dev, "<%s> exit with: %s: %d\n", __func__,
> + (result < 0) ? "error" : "success msg",
> + (result < 0) ? result : num);
> +
> + return (result < 0) ? result : num;
> +}
> +
> +static irqreturn_t lpi2c_imx_isr(int irq, void *dev_id) {
> + unsigned int temp;
> + struct lpi2c_imx_struct *lpi2c_imx = dev_id;
> +
> + lpi2c_imx_intctrl(lpi2c_imx, 0);
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> +
> + if (temp & MSR_RDF) {
> + lpi2c_imx_read_rxfifo(lpi2c_imx);
> + return IRQ_HANDLED;
> + }
> +
> + if (temp & MSR_TDF) {
> + lpi2c_imx_write_txfifo(lpi2c_imx);
> + return IRQ_HANDLED;
> + }
> +
> + complete(&lpi2c_imx->complete);
> + return IRQ_HANDLED;
> +}
> +
> +static u32 lpi2c_imx_func(struct i2c_adapter *adapter) {
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL
> + | I2C_FUNC_SMBUS_READ_BLOCK_DATA;
> +}
> +
> +static struct i2c_algorithm lpi2c_imx_algo = {
> + .master_xfer = lpi2c_imx_xfer,
> + .functionality = lpi2c_imx_func,
> +};
> +
> +static const struct of_device_id lpi2c_imx_of_match[] = {
> + { .compatible = "fsl,imx8dv-lpi2c" },
> + { .compatible = "fsl,imx7ulp-lpi2c" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, lpi2c_imx_of_match)
> +
> +static int lpi2c_imx_probe(struct platform_device *pdev) {
> + int irq, ret;
> + void __iomem *base;
> + struct resource *res;
> + struct lpi2c_imx_struct *lpi2c_imx;
> +
> + lpi2c_imx = devm_kzalloc(&pdev->dev,
> + sizeof(*lpi2c_imx), GFP_KERNEL);
> + if (!lpi2c_imx)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "can't get irq number\n");
> + return irq;
> + }
> +
> + lpi2c_imx->adapter.owner = THIS_MODULE;
> + lpi2c_imx->adapter.algo = &lpi2c_imx_algo;
> + lpi2c_imx->adapter.dev.parent = &pdev->dev;
> + lpi2c_imx->adapter.nr = pdev->id;
> + lpi2c_imx->base = base;
> + lpi2c_imx->adapter.dev.of_node = pdev->dev.of_node;
> + strlcpy(lpi2c_imx->adapter.name, pdev->name,
> + sizeof(lpi2c_imx->adapter.name));
> +
> + lpi2c_imx->per_clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(lpi2c_imx->per_clk)) {
> + dev_err(&pdev->dev, "can't get I2C peripheral clock\n");
> + return PTR_ERR(lpi2c_imx->per_clk);
> + }
> +
> + ret = of_property_read_u32(pdev->dev.of_node,
> + "clock-frequency", &lpi2c_imx->bitrate);
> + if (ret)
> + lpi2c_imx->bitrate = LPI2C_DEFAULT_RATE;
> +
> + ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr, 0,
> + pdev->name, lpi2c_imx);
> + if (ret) {
> + dev_err(&pdev->dev, "can't claim irq %d\n", irq);
> + goto ret;
> + }
> +
> + i2c_set_adapdata(&lpi2c_imx->adapter, lpi2c_imx);
> + platform_set_drvdata(pdev, lpi2c_imx);
> +
> + ret = i2c_add_numbered_adapter(&lpi2c_imx->adapter);
> + if (ret) {
> + dev_err(&pdev->dev, "registration failed\n");
> + goto ret;
> + }
> +
> + dev_info(&lpi2c_imx->adapter.dev, "LPI2C adapter registered\n");
> +
> +ret:
> + return ret;
> +}
> +
> +static int lpi2c_imx_remove(struct platform_device *pdev) {
> + struct lpi2c_imx_struct *lpi2c_imx = platform_get_drvdata(pdev);
> +
> + i2c_del_adapter(&lpi2c_imx->adapter);
> +
> + return 0;
> +}
> +
> +static struct platform_driver lpi2c_imx_driver = {
> + .probe = lpi2c_imx_probe,
> + .remove = lpi2c_imx_remove,
> + .driver = {
> + .name = DRIVER_NAME,
> + .of_match_table = lpi2c_imx_of_match,
> + },
> +};
> +
> +static int __init i2c_adap_imx_init(void) {
> + return platform_driver_register(&lpi2c_imx_driver);
> +}
> +module_init(i2c_adap_imx_init);
> +
> +static void __exit i2c_adap_imx_exit(void) {
> + platform_driver_unregister(&lpi2c_imx_driver);
> +}
> +module_exit(i2c_adap_imx_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Gao Pan <pandy.gao@nxp.com>");
> MODULE_DESCRIPTION("I2C
> +adapter driver for LPI2C bus"); MODULE_ALIAS("platform:" DRIVER_NAME);
> --
> 1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch V3] i2c: imx: add low power i2c bus driver
2016-08-17 7:59 [Patch V3] i2c: imx: add low power i2c bus driver Gao Pan
2016-08-26 7:37 ` Pan Gao
2016-09-09 1:49 ` Pandy Gao
@ 2016-10-24 23:15 ` Vladimir Zapolskiy
2016-11-14 9:21 ` Pandy Gao
2 siblings, 1 reply; 6+ messages in thread
From: Vladimir Zapolskiy @ 2016-10-24 23:15 UTC (permalink / raw)
To: Gao Pan, wsa, u.kleine-koenig, cmo; +Cc: linux-i2c, frank.li, fugang.duan
Hello Pandy,
On 17.08.2016 10:59, Gao Pan wrote:
> This patch adds low power i2c bus driver to support new i.MX
> products which use low power i2c instead of the old imx i2c.
>
> The low power i2c can continue operating in stop mode when
> an appropriate clock is available. It is also designed for
> low CPU overhead with DMA offloading of FIFO register accesses.
>
> Signed-off-by: Gao Pan <pandy.gao@nxp.com>
> Reviewed-by: Fugang Duan <B38611@freescale.com>
> ---
> V2:
> -stop i2c transfer under the wrong condition
> -add timeout check in while() domain
>
> V3:
> -fix typo inside commit message and the driver.
>
> .../devicetree/bindings/i2c/i2c-imx-lpi2c.txt | 25 +
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-imx-lpi2c.c | 667 +++++++++++++++++++++
> 4 files changed, 703 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt b/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> new file mode 100644
> index 0000000..1f10cbf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> @@ -0,0 +1,25 @@
> +* Freescale Low Power Inter IC (LPI2C) for i.MX
> +
> +Required properties:
> +- compatible :
> + - "fsl,imx8dv-lpi2c" for LPI2C compatible with the one integrated on i.MX8DV soc
> + - "fsl,imx7ulp-lpi2c" for LPI2C compatible with the one integrated on i.MX7ULP soc
> +- reg : Should contain LPI2C registers location and length
> +- interrupts : Should contain LPI2C interrupt
> +- clocks : Should contain LPI2C clock specifier
> +- power-domains : should contain LPI2C power domain
> +
> +Optional properties:
> +- clock-frequency : Constains desired LPI2C bus clock frequency in Hz.
typo, what is "constains"? Contains, constrains?
> + The absence of the property indicates the default frequency 100 kHz.
> +
> +Examples:
> +
> +i2c1: i2c@5e110000 { /* LPI2C on i.MX8DV */
> + compatible = "fsl,imx8dv-lpi2c";
> + reg = <0x0 0x5e110000 0x0 0x4000>;
> + interrupts = <0 88 4>;
> + clocks = <&clk IMX8DV_I2C1_CLK>;
> + clock-names = "per";
> + power-domains = <&pd_lsio_i2c1>;
> +};
For this part please send the change to devicetree mailing list and get
Rob Herring's ack. You may split it into a separate patch.
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index efa3d9b..1fc7a10 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -596,6 +596,16 @@ config I2C_IMX
> This driver can also be built as a module. If so, the module
> will be called i2c-imx.
>
> +config I2C_IMX_LPI2C
> + tristate "IMX Low Power I2C interface"
> + depends on ARCH_MXC || COMPILE_TEST
> + help
> + Say Y here if you want to use the Low Power IIC bus controller
> + on the Freescale i.MX processors.
> +
> + This driver can also be built as a module. If so, the module
> + will be called i2c-imx-lpi2c.
> +
> config I2C_IOP3XX
> tristate "Intel IOPx3xx and IXP4xx on-chip I2C interface"
> depends on ARCH_IOP32X || ARCH_IOP33X || ARCH_IXP4XX || ARCH_IOP13XX
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 37f2819..cc93457 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_I2C_HIX5HD2) += i2c-hix5hd2.o
> obj-$(CONFIG_I2C_IBM_IIC) += i2c-ibm_iic.o
> obj-$(CONFIG_I2C_IMG) += i2c-img-scb.o
> obj-$(CONFIG_I2C_IMX) += i2c-imx.o
> +obj-$(CONFIG_I2C_IMX_LPI2C) += i2c-imx-lpi2c.o
> obj-$(CONFIG_I2C_IOP3XX) += i2c-iop3xx.o
> obj-$(CONFIG_I2C_JZ4780) += i2c-jz4780.o
> obj-$(CONFIG_I2C_KEMPLD) += i2c-kempld.o
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> new file mode 100644
> index 0000000..308ecf5
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -0,0 +1,667 @@
> +/*
> + * This is i.MX low power i2c controller driver.
> + *
> + * Copyright 2016 Freescale Semiconductor, Inc.
> + *
> + * 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; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +
> +#define DRIVER_NAME "imx-lpi2c"
> +
> +#define LPI2C_PARAM 0x04 /* i2c RX/TX FIFO size */
> +#define LPI2C_MCR 0x10 /* i2c contrl register */
> +#define LPI2C_MSR 0x14 /* i2c status register */
> +#define LPI2C_MIER 0x18 /* i2c interrupt enable */
> +#define LPI2C_MCFGR0 0x20 /* i2c master configuration */
> +#define LPI2C_MCFGR1 0x24 /* i2c master configuration */
> +#define LPI2C_MCFGR2 0x28 /* i2c master configuration */
> +#define LPI2C_MCFGR3 0x2C /* i2c master configuration */
> +#define LPI2C_MCCR0 0x48 /* i2c master clk configuration */
> +#define LPI2C_MCCR1 0x50 /* i2c master clk configuration */
> +#define LPI2C_MFCR 0x58 /* i2c master FIFO control */
> +#define LPI2C_MFSR 0x5C /* i2c master FIFO status */
> +#define LPI2C_MTDR 0x60 /* i2c master TX data register */
> +#define LPI2C_MRDR 0x70 /* i2c master RX data register */
> +
> +/* i2c command */
> +#define TRAN_DATA 0X00
> +#define RECV_DATA 0X01
> +#define GEN_STOP 0X02
> +#define RECV_DISCARD 0X03
> +#define GEN_START 0X04
> +#define START_NACK 0X05
> +#define START_HIGH 0X06
> +#define START_HIGH_NACK 0X07
> +
> +#define MCR_MEN (1 << 0)
> +#define MCR_RST (1 << 1)
> +#define MCR_DOZEN (1 << 2)
> +#define MCR_DBGEN (1 << 3)
> +#define MCR_RTF (1 << 8)
> +#define MCR_RRF (1 << 9)
> +#define MSR_TDF (1 << 0)
> +#define MSR_RDF (1 << 1)
> +#define MSR_SDF (1 << 9)
> +#define MSR_NDF (1 << 10)
> +#define MSR_ALF (1 << 11)
> +#define MSR_MBF (1 << 24)
> +#define MSR_BBF (1 << 25)
> +#define MIER_TDIE (1 << 0)
> +#define MIER_RDIE (1 << 1)
> +#define MIER_SDIE (1 << 9)
> +#define MIER_NDIE (1 << 10)
> +#define MCFGR1_AUTOSTOP (1 << 8)
> +#define MCFGR1_IGNACK (1 << 9)
> +#define MRDR_RXEMPTY (1 << 14)
Please use BIT() helper above.
Also please don't use tab symbol between #define and a token.
> +
> +#define I2C_CLK_RATIO 2
> +#define CHUNK_DATA 256
> +
> +#define LPI2C_RX_FIFOSIZE 4
> +#define LPI2C_TX_FIFOSIZE 4
> +
> +#define LPI2C_DEFAULT_RATE 100000
> +#define STARDARD_MAX_BITRATE 400000
> +#define FAST_MAX_BITRATE 1000000
> +#define FAST_PLUS_MAX_BITRATE 3400000
> +#define HIGHSPEED_MAX_BITRATE 5000000
Please don't use tab symbol right after #define
> +
> +
Double empty line, this kind of problem is reported by
checkpatch --strict, please pay attention to all of them:
total: 0 errors, 2 warnings, 33 checks, 715 lines checked
> +enum lpi2c_imx_mode {
> + STANDARD, /* 100+Kbps */
> + FAST, /* 400+Kbps */
> + FAST_PLUS, /* 1.0+Mbps */
> + ULTRA_FAST, /* 5.0+Mbps */
> + HS, /* 3.4+Mbps */
Any reason why the list is not sorted by bus speed?
> +};
> +
> +enum lpi2c_imx_pincfg {
> + TWO_PIN_OD, /* 2-pin open drain mode */
> + TWO_PIN_OO, /* 2-pin output only mode (utra-fast mode) */
> + TWO_PIN_PP, /* 2-pin push-pull mode */
> + FOUR_PIN_PP, /* 4-pin push-pull mode */
> + TWO_PIN_OD_SS, /* 2-pin open drain mode with separate slave */
Unused.
> + TWO_PIN_OO_SS, /* 2-pin output only mode with separate slave */
Unused.
> + TWO_PIN_PP_SS, /* 2-pin push-pull mode with separate slave */
Unused.
> + FOUR_PIN_PP_IO, /* 4-pin push-pull mode (inverted output) */
Unused.
> +};
> +
> +struct lpi2c_imx_clkcfg {
> + u8 prescale;
> + u8 filtscl;
> + u8 filtsda;
> + u8 sethold;
> + u8 clklo;
> + u8 clkhi;
> + u8 datavd;
> +};
> +
> +struct lpi2c_imx_struct {
> + struct i2c_adapter adapter;
> + struct clk *per_clk;
> + void __iomem *base;
> + __u8 *rx_buf;
> + __u8 *tx_buf;
> + struct completion complete;
> + unsigned int msglen;
> + unsigned int delivered;
> + unsigned int block_data;
> + unsigned int bitrate;
> + enum lpi2c_imx_mode mode;
> +};
> +
> +static void lpi2c_imx_intctrl(
> + struct lpi2c_imx_struct *lpi2c_imx, unsigned int enable)
Indentation issue.
> +{
> + writel(enable, lpi2c_imx->base + LPI2C_MIER);
> +}
> +
> +static int lpi2c_imx_bus_busy(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + unsigned long orig_jiffies = jiffies;
> + unsigned int temp;
> +
> + while (1) {
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> +
> + /* check for arbitration lost, clear if set */
> + if (temp & MSR_ALF) {
> + writel(temp, lpi2c_imx->base + LPI2C_MSR);
> + return -EAGAIN;
> + }
> +
> + if ((temp & MSR_BBF) && (temp & MSR_MBF))
if (temp & (MSR_BBF | MSR_MBF))
> + break;
> +
> + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "bus not work\n");
> + return -ETIMEDOUT;
> + }
> + schedule();
> + }
> +
> + return 0;
> +}
> +
> +static void lpi2c_imx_set_mode(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + enum lpi2c_imx_mode mode;
> + unsigned int bitrate = lpi2c_imx->bitrate;
unsigned int bitrate = lpi2c_imx->bitrate;
enum lpi2c_imx_mode mode;
If possible please use "reverse christmas tree" order while declaring
local variables, this applies to some other functions below as well.
I see that you mainly use "christmas tree" order, but this style
isn't commonly used in the Linux kernel sources.
> +
> + if (bitrate < STARDARD_MAX_BITRATE)
> + mode = STANDARD;
> + else if (bitrate < FAST_MAX_BITRATE)
> + mode = FAST;
> + else if (bitrate < FAST_PLUS_MAX_BITRATE)
> + mode = FAST_PLUS;
> + else if (bitrate < HIGHSPEED_MAX_BITRATE)
> + mode = HS;
> + else
> + mode = ULTRA_FAST;
> +
> + lpi2c_imx->mode = mode;
> +}
> +
> +static int lpi2c_imx_start(struct lpi2c_imx_struct *lpi2c_imx,
> + struct i2c_msg *msgs)
> +{
> + u8 read;
> + unsigned int temp;
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> + temp |= MCR_RRF | MCR_RTF;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> + writel(0x7f00, lpi2c_imx->base + LPI2C_MSR);
> +
> + read = msgs->flags & I2C_M_RD;
> + temp = (msgs->addr << 1 | read) | (GEN_START << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> +
> + return lpi2c_imx_bus_busy(lpi2c_imx);
> +}
> +
> +static void lpi2c_imx_stop(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + unsigned int temp;
> + unsigned long orig_jiffies = jiffies;
> +
> + writel(GEN_STOP << 8, lpi2c_imx->base + LPI2C_MTDR);
Add an empty line here.
> + do {
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> + if (temp & MSR_SDF)
> + break;
> +
> + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "stop timeout\n");
> + break;
> + }
> + schedule();
> +
> + } while (1);
> +}
> +
> +
> +/* CLKLO = I2C_CLK_RATIO * CLKHI, SETHOLD = CLKHI, DATAVD = CLKHI/2 */
> +static int lpi2c_imx_config(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + unsigned int temp;
> + unsigned int per_clk_rate;
> + unsigned int prescale, clk_high, clk_low, clk_cycle;
> + enum lpi2c_imx_pincfg pincfg;
> + struct lpi2c_imx_clkcfg clkcfg;
> +
> + lpi2c_imx_set_mode(lpi2c_imx);
> + per_clk_rate = clk_get_rate(lpi2c_imx->per_clk);
> +
> + if (lpi2c_imx->mode == HS || lpi2c_imx->mode == ULTRA_FAST)
if (lpi2c_imx->mode > FAST_PLUS)
> + clkcfg.filtscl = clkcfg.filtsda = 0;
> + else
> + clkcfg.filtscl = clkcfg.filtsda = 2;
> +
Multiple assignments on a single line are not welcome, in this
case one variable "filt" assigned to 0 or 2 should be enough.
Why do you need struct lpi2c_imx_clkcfg in general?
clkcfg.filtscl, clkcfg.filtsda etc. are all used locally inside
this function only, it should be sufficient to replace "clkcfg"
with a number of local variables.
> + for (prescale = 0; prescale <= 7; prescale++) {
> + clk_cycle = per_clk_rate / ((1 << prescale) * lpi2c_imx->bitrate)
> + - 3 - (clkcfg.filtscl >> 1);
> + clk_high = (clk_cycle + I2C_CLK_RATIO) / (I2C_CLK_RATIO + 1);
> + clk_low = clk_cycle - clk_high;
> + if (clk_low < 64)
> + break;
> + }
> +
> + if (prescale > 7)
> + return -EINVAL;
> +
> + clkcfg.prescale = prescale;
> + clkcfg.sethold = clk_high;
> + clkcfg.clklo = clk_low;
> + clkcfg.clkhi = clk_high;
> + clkcfg.datavd = clk_high >> 1;
Useless duplication of variables, see a note above.
> +
> + /* set MCFGR1: PINCFG, PRESCALE, IGNACK */
> + if (lpi2c_imx->mode == ULTRA_FAST)
> + pincfg = TWO_PIN_OO;
> + else
> + pincfg = TWO_PIN_OD;
> + temp = clkcfg.prescale | pincfg << 24;
> +
> + if (lpi2c_imx->mode == ULTRA_FAST)
> + temp |= MCFGR1_IGNACK;
> +
> + writel(temp, lpi2c_imx->base + LPI2C_MCFGR1);
> +
> + /* set MCFGR2: FILTSDA, FILTSCL */
> + temp = (clkcfg.filtscl << 16) | (clkcfg.filtsda << 24);
> + writel(temp, lpi2c_imx->base + LPI2C_MCFGR2);
> +
> +
> + /* set MCCR: DATAVD, SETHOLD, CLKHI, CLKLO */
> + temp = clkcfg.datavd << 24 | clkcfg.sethold << 16 |
> + clkcfg.clkhi << 8 | clkcfg.clklo;
> +
> + if (lpi2c_imx->mode == HS)
> + writel(temp, lpi2c_imx->base + LPI2C_MCCR1);
> + else
> + writel(temp, lpi2c_imx->base + LPI2C_MCCR0);
> +
> + return 0;
> +}
> +
> +static int lpi2c_imx_master_enable(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + int ret;
> + unsigned int temp;
> +
> + ret = clk_prepare_enable(lpi2c_imx->per_clk);
You can do clk_prepare() in probe function and clk_unprepare() in remove
function to avoid potential sleeping in runtime, then here you just
do clk_enable()/clk_disable().
> + if (ret)
> + return ret;
> +
> + temp = MCR_RST;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> + writel(0, lpi2c_imx->base + LPI2C_MCR);
> +
> + ret = lpi2c_imx_config(lpi2c_imx);
> + if (ret)
> + return ret;
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> + temp |= MCR_MEN;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> +
> + return 0;
> +}
> +
> +static int lpi2c_imx_master_disable(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + unsigned int temp = 0;
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> + temp &= ~MCR_MEN;
> + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> +
> + clk_disable_unprepare(lpi2c_imx->per_clk);
See a note above.
> +
> + return 0;
> +}
> +
> +static int lpi2c_imx_msg_complete(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + unsigned int timeout;
> +
> + timeout = wait_for_completion_timeout(&lpi2c_imx->complete, HZ);
> +
> + return timeout ? 0 : -ETIMEDOUT;
> +}
> +
> +static int lpi2c_imx_txfifo_empty(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + u32 txcnt;
> + unsigned long orig_jiffies = jiffies;
> +
> + do {
> + txcnt = readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff;
> +
> + if (readl(lpi2c_imx->base + LPI2C_MSR) & MSR_NDF) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "NDF detected\n");
> + return -EIO;
> + }
> +
> + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "txfifo empty timeout\n");
> + return -ETIMEDOUT;
> + }
> + schedule();
> +
> + } while (txcnt);
> +
> + return 0;
> +}
> +
> +static void lpi2c_imx_set_tx_watermark(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + unsigned int temp;
> +
> + temp = LPI2C_TX_FIFOSIZE >> 1;
> + writel(temp, lpi2c_imx->base + LPI2C_MFCR);
writel(LPI2C_TX_FIFOSIZE >> 1, lpi2c_imx->base + LPI2C_MFCR);
> +}
> +
> +static void lpi2c_imx_set_rx_watermark(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + unsigned int temp, remaining;
> +
> + remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
> +
> + if (remaining > (LPI2C_RX_FIFOSIZE >> 1))
> + temp = LPI2C_RX_FIFOSIZE >> 1;
> + else
> + temp = 0;
> +
> + writel(temp << 16, lpi2c_imx->base + LPI2C_MFCR);
> +}
> +
> +static void lpi2c_imx_write_txfifo(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + unsigned int data, txcnt;
> +
> + txcnt = readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff;
Add an empty line here.
> + while (txcnt < LPI2C_TX_FIFOSIZE) {
> + if (lpi2c_imx->delivered == lpi2c_imx->msglen)
> + break;
Add an empty line here.
> + data = lpi2c_imx->tx_buf[lpi2c_imx->delivered++];
> + writel(data, lpi2c_imx->base + LPI2C_MTDR);
> + txcnt++;
> + }
> +
> + if (lpi2c_imx->delivered < lpi2c_imx->msglen)
> + lpi2c_imx_intctrl(lpi2c_imx, MIER_TDIE | MIER_NDIE);
> + else
> + complete(&lpi2c_imx->complete);
> +}
> +
> +static void lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx)
> +{
> + unsigned int temp, data;
> + unsigned int blocklen, remaining;
> +
> + do {
> + data = readl(lpi2c_imx->base + LPI2C_MRDR);
> + if (data & MRDR_RXEMPTY)
> + break;
Add an empty line here.
> + lpi2c_imx->rx_buf[lpi2c_imx->delivered++] = data & 0xff;
> + } while (1);
> +
> + /*
> + * First byte is the length of remaining packet in the SMBus block
> + * data read. Add it to msgs->len.
> + */
> + if (lpi2c_imx->block_data) {
> + blocklen = lpi2c_imx->rx_buf[0];
> + lpi2c_imx->msglen += blocklen;
> + }
> +
> + remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
> + /* not finished, still waiting for rx data */
Please move the comment under if (remaining) condition.
> + if (remaining) {
> + lpi2c_imx_set_rx_watermark(lpi2c_imx);
> + /* multiple receive commands */
> + if (lpi2c_imx->block_data) {
> + lpi2c_imx->block_data = 0;
> + temp = remaining;
> + temp |= (RECV_DATA << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> + } else if (!(lpi2c_imx->delivered & 0xff)) {
> + temp = remaining > CHUNK_DATA ?
> + CHUNK_DATA - 1 : (remaining - 1);
> + temp |= (RECV_DATA << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> + }
> +
> + lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE);
> + } else
> + complete(&lpi2c_imx->complete);
Start it from
if (!remaining) {
complete(&lpi2c_imx->complete);
return;
}
/* not finished, still waiting for rx data */
....
Then you get less indentations. Generally please use more return points
instead of if-if-if constructions.
> +}
> +
> +static void lpi2c_imx_write(struct lpi2c_imx_struct *lpi2c_imx,
> + struct i2c_msg *msgs)
> +{
> + lpi2c_imx->tx_buf = msgs->buf;
> + lpi2c_imx_set_tx_watermark(lpi2c_imx);
> + lpi2c_imx_write_txfifo(lpi2c_imx);
> +}
> +
> +static void lpi2c_imx_read(struct lpi2c_imx_struct *lpi2c_imx,
> + struct i2c_msg *msgs)
> +{
> + unsigned int temp;
> +
> + lpi2c_imx->rx_buf = msgs->buf;
> + lpi2c_imx->block_data = msgs->flags & I2C_M_RECV_LEN;
> +
> + lpi2c_imx_set_rx_watermark(lpi2c_imx);
> + temp = msgs->len > CHUNK_DATA ? CHUNK_DATA - 1 : msgs->len - 1;
> + temp |= (RECV_DATA << 8);
> + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> +
> + lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE | MIER_NDIE);
> +}
> +
> +static int lpi2c_imx_xfer(struct i2c_adapter *adapter,
> + struct i2c_msg *msgs, int num)
> +{
> + int i, result;
> + unsigned int temp;
> + struct lpi2c_imx_struct *lpi2c_imx = i2c_get_adapdata(adapter);
> +
> + result = lpi2c_imx_master_enable(lpi2c_imx);
> + if (result)
> + return result;
> +
> + for (i = 0; i < num; i++) {
> + result = lpi2c_imx_start(lpi2c_imx, &msgs[i]);
> + if (result)
> + goto disable;
> +
> + /* quick smbus */
> + if (num == 1 && msgs[0].len == 0)
> + goto stop;
> +
> + lpi2c_imx->delivered = 0;
> + lpi2c_imx->msglen = msgs[i].len;
> + init_completion(&lpi2c_imx->complete);
> +
> + if (msgs[i].flags & I2C_M_RD)
> + lpi2c_imx_read(lpi2c_imx, &msgs[i]);
> + else
> + lpi2c_imx_write(lpi2c_imx, &msgs[i]);
> +
> + result = lpi2c_imx_msg_complete(lpi2c_imx);
> + if (result)
> + goto stop;
> +
> + if (!(msgs[i].flags & I2C_M_RD)) {
> + result = lpi2c_imx_txfifo_empty(lpi2c_imx);
> + if (result)
> + goto stop;
> + }
> + }
> +
> +stop:
> + lpi2c_imx_stop(lpi2c_imx);
> +
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> + if ((temp & MSR_NDF) && !result)
> + result = -EIO;
Zero-length transactions are not supported, right?
> +
> +disable:
> + lpi2c_imx_master_disable(lpi2c_imx);
> +
> + dev_dbg(&lpi2c_imx->adapter.dev, "<%s> exit with: %s: %d\n", __func__,
> + (result < 0) ? "error" : "success msg",
> + (result < 0) ? result : num);
> +
> + return (result < 0) ? result : num;
> +}
> +
> +static irqreturn_t lpi2c_imx_isr(int irq, void *dev_id)
> +{
> + unsigned int temp;
> + struct lpi2c_imx_struct *lpi2c_imx = dev_id;
> +
> + lpi2c_imx_intctrl(lpi2c_imx, 0);
> + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> +
> + if (temp & MSR_RDF) {
> + lpi2c_imx_read_rxfifo(lpi2c_imx);
> + return IRQ_HANDLED;
> + }
> +
> + if (temp & MSR_TDF) {
> + lpi2c_imx_write_txfifo(lpi2c_imx);
> + return IRQ_HANDLED;
> + }
> +
> + complete(&lpi2c_imx->complete);
Add an empty line here.
> + return IRQ_HANDLED;
> +}
> +
> +static u32 lpi2c_imx_func(struct i2c_adapter *adapter)
> +{
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL
> + | I2C_FUNC_SMBUS_READ_BLOCK_DATA;
checkpatch does not complain? I expect it should be
return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
I2C_FUNC_SMBUS_READ_BLOCK_DATA;
> +}
> +
> +static struct i2c_algorithm lpi2c_imx_algo = {
> + .master_xfer = lpi2c_imx_xfer,
> + .functionality = lpi2c_imx_func,
> +};
> +
> +static const struct of_device_id lpi2c_imx_of_match[] = {
> + { .compatible = "fsl,imx8dv-lpi2c" },
> + { .compatible = "fsl,imx7ulp-lpi2c" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, lpi2c_imx_of_match)
> +
> +static int lpi2c_imx_probe(struct platform_device *pdev)
> +{
> + int irq, ret;
> + void __iomem *base;
> + struct resource *res;
> + struct lpi2c_imx_struct *lpi2c_imx;
> +
> + lpi2c_imx = devm_kzalloc(&pdev->dev,
> + sizeof(*lpi2c_imx), GFP_KERNEL);
> + if (!lpi2c_imx)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "can't get irq number\n");
> + return irq;
> + }
> +
> + lpi2c_imx->adapter.owner = THIS_MODULE;
> + lpi2c_imx->adapter.algo = &lpi2c_imx_algo;
> + lpi2c_imx->adapter.dev.parent = &pdev->dev;
> + lpi2c_imx->adapter.nr = pdev->id;
Do you really need it? Please consider to use i2c_add_adapter().
> + lpi2c_imx->base = base;
For sake of consistency please initialize lpi2c_imx->adapter fields
in a row.
You don't need this local 'base' variable, use lpi2c_imx->base instead.
> + lpi2c_imx->adapter.dev.of_node = pdev->dev.of_node;
> + strlcpy(lpi2c_imx->adapter.name, pdev->name,
> + sizeof(lpi2c_imx->adapter.name));
> +
> + lpi2c_imx->per_clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(lpi2c_imx->per_clk)) {
> + dev_err(&pdev->dev, "can't get I2C peripheral clock\n");
> + return PTR_ERR(lpi2c_imx->per_clk);
> + }
> +
> + ret = of_property_read_u32(pdev->dev.of_node,
> + "clock-frequency", &lpi2c_imx->bitrate);
> + if (ret)
> + lpi2c_imx->bitrate = LPI2C_DEFAULT_RATE;
> +
> + ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr, 0,
> + pdev->name, lpi2c_imx);
> + if (ret) {
> + dev_err(&pdev->dev, "can't claim irq %d\n", irq);
> + goto ret;
Just return ret;
> + }
> +
> + i2c_set_adapdata(&lpi2c_imx->adapter, lpi2c_imx);
> + platform_set_drvdata(pdev, lpi2c_imx);
> +
> + ret = i2c_add_numbered_adapter(&lpi2c_imx->adapter);
> + if (ret) {
> + dev_err(&pdev->dev, "registration failed\n");
> + goto ret;
Just return ret;
> + }
> +
> + dev_info(&lpi2c_imx->adapter.dev, "LPI2C adapter registered\n");
> +
> +ret:
> + return ret;
return 0;
> +}
> +
> +static int lpi2c_imx_remove(struct platform_device *pdev)
> +{
> + struct lpi2c_imx_struct *lpi2c_imx = platform_get_drvdata(pdev);
> +
> + i2c_del_adapter(&lpi2c_imx->adapter);
> +
> + return 0;
> +}
> +
> +static struct platform_driver lpi2c_imx_driver = {
> + .probe = lpi2c_imx_probe,
> + .remove = lpi2c_imx_remove,
> + .driver = {
> + .name = DRIVER_NAME,
> + .of_match_table = lpi2c_imx_of_match,
> + },
> +};
> +
> +static int __init i2c_adap_imx_init(void)
> +{
> + return platform_driver_register(&lpi2c_imx_driver);
> +}
> +module_init(i2c_adap_imx_init);
> +
> +static void __exit i2c_adap_imx_exit(void)
> +{
> + platform_driver_unregister(&lpi2c_imx_driver);
> +}
> +module_exit(i2c_adap_imx_exit);
> +
Please use module_platform_driver(lpi2c_imx_driver);
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Gao Pan <pandy.gao@nxp.com>");
> +MODULE_DESCRIPTION("I2C adapter driver for LPI2C bus");
> +MODULE_ALIAS("platform:" DRIVER_NAME);
>
Are you sure that the driver needs a platform alias here?
--
With best wishes,
Vladimir
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [Patch V3] i2c: imx: add low power i2c bus driver
2016-10-24 23:15 ` Vladimir Zapolskiy
@ 2016-11-14 9:21 ` Pandy Gao
0 siblings, 0 replies; 6+ messages in thread
From: Pandy Gao @ 2016-11-14 9:21 UTC (permalink / raw)
To: Vladimir Zapolskiy, wsa, u.kleine-koenig, cmo
Cc: linux-i2c, Frank Li, Andy Duan
From: Vladimir Zapolskiy <mailto:vz@mleia.com> Sent: Tuesday, October 25, 2016 7:15 AM
> To: Pandy Gao <pandy.gao@nxp.com>; wsa@the-dreams.de; u.kleine-
> koenig@pengutronix.de; cmo@melexis.com
> Cc: linux-i2c@vger.kernel.org; Frank Li <frank.li@nxp.com>; Andy Duan
> <fugang.duan@nxp.com>
> Subject: Re: [Patch V3] i2c: imx: add low power i2c bus driver
>
> Hello Pandy,
>
> On 17.08.2016 10:59, Gao Pan wrote:
> > This patch adds low power i2c bus driver to support new i.MX products
> > which use low power i2c instead of the old imx i2c.
> >
> > The low power i2c can continue operating in stop mode when an
> > appropriate clock is available. It is also designed for low CPU
> > overhead with DMA offloading of FIFO register accesses.
> >
> > Signed-off-by: Gao Pan <pandy.gao@nxp.com>
> > Reviewed-by: Fugang Duan <B38611@freescale.com>
> > ---
> > V2:
> > -stop i2c transfer under the wrong condition -add timeout check in
> > while() domain
> >
> > V3:
> > -fix typo inside commit message and the driver.
> >
> > .../devicetree/bindings/i2c/i2c-imx-lpi2c.txt | 25 +
> > drivers/i2c/busses/Kconfig | 10 +
> > drivers/i2c/busses/Makefile | 1 +
> > drivers/i2c/busses/i2c-imx-lpi2c.c | 667 +++++++++++++++++++++
> > 4 files changed, 703 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> > b/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> > new file mode 100644
> > index 0000000..1f10cbf
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
> > @@ -0,0 +1,25 @@
> > +* Freescale Low Power Inter IC (LPI2C) for i.MX
> > +
> > +Required properties:
> > +- compatible :
> > + - "fsl,imx8dv-lpi2c" for LPI2C compatible with the one integrated
> > +on i.MX8DV soc
> > + - "fsl,imx7ulp-lpi2c" for LPI2C compatible with the one integrated
> > +on i.MX7ULP soc
> > +- reg : Should contain LPI2C registers location and length
> > +- interrupts : Should contain LPI2C interrupt
> > +- clocks : Should contain LPI2C clock specifier
> > +- power-domains : should contain LPI2C power domain
> > +
> > +Optional properties:
> > +- clock-frequency : Constains desired LPI2C bus clock frequency in Hz.
>
> typo, what is "constains"? Contains, constrains?
It should be "contains", Thanks!
> > + The absence of the property indicates the default frequency 100 kHz.
> > +
> > +Examples:
> > +
> > +i2c1: i2c@5e110000 { /* LPI2C on i.MX8DV */
> > + compatible = "fsl,imx8dv-lpi2c";
> > + reg = <0x0 0x5e110000 0x0 0x4000>;
> > + interrupts = <0 88 4>;
> > + clocks = <&clk IMX8DV_I2C1_CLK>;
> > + clock-names = "per";
> > + power-domains = <&pd_lsio_i2c1>;
> > +};
>
> For this part please send the change to devicetree mailing list and get Rob
> Herring's ack. You may split it into a separate patch.
Thanks, it is better to split these part from i2c driver.
> > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > index efa3d9b..1fc7a10 100644
> > --- a/drivers/i2c/busses/Kconfig
> > +++ b/drivers/i2c/busses/Kconfig
> > @@ -596,6 +596,16 @@ config I2C_IMX
> > This driver can also be built as a module. If so, the module
> > will be called i2c-imx.
> >
> > +config I2C_IMX_LPI2C
> > + tristate "IMX Low Power I2C interface"
> > + depends on ARCH_MXC || COMPILE_TEST
> > + help
> > + Say Y here if you want to use the Low Power IIC bus controller
> > + on the Freescale i.MX processors.
> > +
> > + This driver can also be built as a module. If so, the module
> > + will be called i2c-imx-lpi2c.
> > +
> > config I2C_IOP3XX
> > tristate "Intel IOPx3xx and IXP4xx on-chip I2C interface"
> > depends on ARCH_IOP32X || ARCH_IOP33X || ARCH_IXP4XX ||
> ARCH_IOP13XX
> > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> > index 37f2819..cc93457 100644
> > --- a/drivers/i2c/busses/Makefile
> > +++ b/drivers/i2c/busses/Makefile
> > @@ -56,6 +56,7 @@ obj-$(CONFIG_I2C_HIX5HD2) += i2c-hix5hd2.o
> > obj-$(CONFIG_I2C_IBM_IIC) += i2c-ibm_iic.o
> > obj-$(CONFIG_I2C_IMG) += i2c-img-scb.o
> > obj-$(CONFIG_I2C_IMX) += i2c-imx.o
> > +obj-$(CONFIG_I2C_IMX_LPI2C) += i2c-imx-lpi2c.o
> > obj-$(CONFIG_I2C_IOP3XX) += i2c-iop3xx.o
> > obj-$(CONFIG_I2C_JZ4780) += i2c-jz4780.o
> > obj-$(CONFIG_I2C_KEMPLD) += i2c-kempld.o
> > diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c
> > b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > new file mode 100644
> > index 0000000..308ecf5
> > --- /dev/null
> > +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > @@ -0,0 +1,667 @@
> > +/*
> > + * This is i.MX low power i2c controller driver.
> > + *
> > + * Copyright 2016 Freescale Semiconductor, Inc.
> > + *
> > + * 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; either version 2
> > + * of the License, or (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/completion.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/errno.h>
> > +#include <linux/i2c.h>
> > +#include <linux/init.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/sched.h>
> > +#include <linux/slab.h>
> > +
> > +#define DRIVER_NAME "imx-lpi2c"
> > +
> > +#define LPI2C_PARAM 0x04 /* i2c RX/TX FIFO size */
> > +#define LPI2C_MCR 0x10 /* i2c contrl register */
> > +#define LPI2C_MSR 0x14 /* i2c status register */
> > +#define LPI2C_MIER 0x18 /* i2c interrupt enable */
> > +#define LPI2C_MCFGR0 0x20 /* i2c master configuration */
> > +#define LPI2C_MCFGR1 0x24 /* i2c master configuration */
> > +#define LPI2C_MCFGR2 0x28 /* i2c master configuration */
> > +#define LPI2C_MCFGR3 0x2C /* i2c master configuration */
> > +#define LPI2C_MCCR0 0x48 /* i2c master clk configuration */
> > +#define LPI2C_MCCR1 0x50 /* i2c master clk configuration */
> > +#define LPI2C_MFCR 0x58 /* i2c master FIFO control */
> > +#define LPI2C_MFSR 0x5C /* i2c master FIFO status */
> > +#define LPI2C_MTDR 0x60 /* i2c master TX data register */
> > +#define LPI2C_MRDR 0x70 /* i2c master RX data register */
> > +
> > +/* i2c command */
> > +#define TRAN_DATA 0X00
> > +#define RECV_DATA 0X01
> > +#define GEN_STOP 0X02
> > +#define RECV_DISCARD 0X03
> > +#define GEN_START 0X04
> > +#define START_NACK 0X05
> > +#define START_HIGH 0X06
> > +#define START_HIGH_NACK 0X07
> > +
> > +#define MCR_MEN (1 << 0)
> > +#define MCR_RST (1 << 1)
> > +#define MCR_DOZEN (1 << 2)
> > +#define MCR_DBGEN (1 << 3)
> > +#define MCR_RTF (1 << 8)
> > +#define MCR_RRF (1 << 9)
> > +#define MSR_TDF (1 << 0)
> > +#define MSR_RDF (1 << 1)
> > +#define MSR_SDF (1 << 9)
> > +#define MSR_NDF (1 << 10)
> > +#define MSR_ALF (1 << 11)
> > +#define MSR_MBF (1 << 24)
> > +#define MSR_BBF (1 << 25)
> > +#define MIER_TDIE (1 << 0)
> > +#define MIER_RDIE (1 << 1)
> > +#define MIER_SDIE (1 << 9)
> > +#define MIER_NDIE (1 << 10)
> > +#define MCFGR1_AUTOSTOP (1 << 8)
> > +#define MCFGR1_IGNACK (1 << 9)
> > +#define MRDR_RXEMPTY (1 << 14)
>
> Please use BIT() helper above.
Thanks, will change to BIT() in next version.
> Also please don't use tab symbol between #define and a token.
Got it, thanks!
> > +
> > +#define I2C_CLK_RATIO 2
> > +#define CHUNK_DATA 256
> > +
> > +#define LPI2C_RX_FIFOSIZE 4
> > +#define LPI2C_TX_FIFOSIZE 4
> > +
> > +#define LPI2C_DEFAULT_RATE 100000
> > +#define STARDARD_MAX_BITRATE 400000
> > +#define FAST_MAX_BITRATE 1000000
> > +#define FAST_PLUS_MAX_BITRATE 3400000
> > +#define HIGHSPEED_MAX_BITRATE 5000000
>
> Please don't use tab symbol right after #define
Thanks, will change it in next version.
> > +
> > +
>
> Double empty line, this kind of problem is reported by checkpatch --strict,
> please pay attention to all of them:
Thanks, I didn't use "--strict" option for checkpatch, so I missed this problem. Will change it in next version.
> total: 0 errors, 2 warnings, 33 checks, 715 lines checked
>
> > +enum lpi2c_imx_mode {
> > + STANDARD, /* 100+Kbps */
> > + FAST, /* 400+Kbps */
> > + FAST_PLUS, /* 1.0+Mbps */
> > + ULTRA_FAST, /* 5.0+Mbps */
> > + HS, /* 3.4+Mbps */
>
> Any reason why the list is not sorted by bus speed?
"HS" use different config with " ULTRA_FAST" and " FAST_PLUS", so I thought this order may be better. Will sort it by bus speed in next version. Thanks!
> > +};
> > +
> > +enum lpi2c_imx_pincfg {
> > + TWO_PIN_OD, /* 2-pin open drain mode */
> > + TWO_PIN_OO, /* 2-pin output only mode (utra-fast mode) */
> > + TWO_PIN_PP, /* 2-pin push-pull mode */
> > + FOUR_PIN_PP, /* 4-pin push-pull mode */
> > + TWO_PIN_OD_SS, /* 2-pin open drain mode with separate slave
> */
>
> Unused.
Will remove them in next version. Thanks!
> > + TWO_PIN_OO_SS, /* 2-pin output only mode with separate slave
> */
>
> Unused.
Thanks!
> > + TWO_PIN_PP_SS, /* 2-pin push-pull mode with separate slave */
>
> Unused.
Thanks!
> > + FOUR_PIN_PP_IO, /* 4-pin push-pull mode (inverted output) */
>
> Unused.
Thanks!
> > +};
> > +
> > +struct lpi2c_imx_clkcfg {
> > + u8 prescale;
> > + u8 filtscl;
> > + u8 filtsda;
> > + u8 sethold;
> > + u8 clklo;
> > + u8 clkhi;
> > + u8 datavd;
> > +};
> > +
> > +struct lpi2c_imx_struct {
> > + struct i2c_adapter adapter;
> > + struct clk *per_clk;
> > + void __iomem *base;
> > + __u8 *rx_buf;
> > + __u8 *tx_buf;
> > + struct completion complete;
> > + unsigned int msglen;
> > + unsigned int delivered;
> > + unsigned int block_data;
> > + unsigned int bitrate;
> > + enum lpi2c_imx_mode mode;
> > +};
> > +
> > +static void lpi2c_imx_intctrl(
> > + struct lpi2c_imx_struct *lpi2c_imx, unsigned int enable)
>
> Indentation issue.
Thanks!
> > +{
> > + writel(enable, lpi2c_imx->base + LPI2C_MIER); }
> > +
> > +static int lpi2c_imx_bus_busy(struct lpi2c_imx_struct *lpi2c_imx) {
> > + unsigned long orig_jiffies = jiffies;
> > + unsigned int temp;
> > +
> > + while (1) {
> > + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> > +
> > + /* check for arbitration lost, clear if set */
> > + if (temp & MSR_ALF) {
> > + writel(temp, lpi2c_imx->base + LPI2C_MSR);
> > + return -EAGAIN;
> > + }
> > +
> > + if ((temp & MSR_BBF) && (temp & MSR_MBF))
>
> if (temp & (MSR_BBF | MSR_MBF))
Thanks, will change it in next version!
> > + break;
> > +
> > + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> > + dev_dbg(&lpi2c_imx->adapter.dev, "bus not work\n");
> > + return -ETIMEDOUT;
> > + }
> > + schedule();
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void lpi2c_imx_set_mode(struct lpi2c_imx_struct *lpi2c_imx) {
> > + enum lpi2c_imx_mode mode;
> > + unsigned int bitrate = lpi2c_imx->bitrate;
>
> unsigned int bitrate = lpi2c_imx->bitrate; enum lpi2c_imx_mode mode;
>
> If possible please use "reverse christmas tree" order while declaring local
> variables, this applies to some other functions below as well.
>
> I see that you mainly use "christmas tree" order, but this style isn't commonly
> used in the Linux kernel sources.
Thanks, will change it in next version.
> > +
> > + if (bitrate < STARDARD_MAX_BITRATE)
> > + mode = STANDARD;
> > + else if (bitrate < FAST_MAX_BITRATE)
> > + mode = FAST;
> > + else if (bitrate < FAST_PLUS_MAX_BITRATE)
> > + mode = FAST_PLUS;
> > + else if (bitrate < HIGHSPEED_MAX_BITRATE)
> > + mode = HS;
> > + else
> > + mode = ULTRA_FAST;
> > +
> > + lpi2c_imx->mode = mode;
> > +}
> > +
> > +static int lpi2c_imx_start(struct lpi2c_imx_struct *lpi2c_imx,
> > + struct i2c_msg *msgs)
> > +{
> > + u8 read;
> > + unsigned int temp;
> > +
> > + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> > + temp |= MCR_RRF | MCR_RTF;
> > + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> > + writel(0x7f00, lpi2c_imx->base + LPI2C_MSR);
> > +
> > + read = msgs->flags & I2C_M_RD;
> > + temp = (msgs->addr << 1 | read) | (GEN_START << 8);
> > + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> > +
> > + return lpi2c_imx_bus_busy(lpi2c_imx); }
> > +
> > +static void lpi2c_imx_stop(struct lpi2c_imx_struct *lpi2c_imx) {
> > + unsigned int temp;
> > + unsigned long orig_jiffies = jiffies;
> > +
> > + writel(GEN_STOP << 8, lpi2c_imx->base + LPI2C_MTDR);
>
> Add an empty line here.
Thanks!
> > + do {
> > + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> > + if (temp & MSR_SDF)
> > + break;
> > +
> > + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> > + dev_dbg(&lpi2c_imx->adapter.dev, "stop timeout\n");
> > + break;
> > + }
> > + schedule();
> > +
> > + } while (1);
> > +}
> > +
> > +
> > +/* CLKLO = I2C_CLK_RATIO * CLKHI, SETHOLD = CLKHI, DATAVD = CLKHI/2
> > +*/ static int lpi2c_imx_config(struct lpi2c_imx_struct *lpi2c_imx) {
> > + unsigned int temp;
> > + unsigned int per_clk_rate;
> > + unsigned int prescale, clk_high, clk_low, clk_cycle;
> > + enum lpi2c_imx_pincfg pincfg;
> > + struct lpi2c_imx_clkcfg clkcfg;
> > +
> > + lpi2c_imx_set_mode(lpi2c_imx);
> > + per_clk_rate = clk_get_rate(lpi2c_imx->per_clk);
> > +
> > + if (lpi2c_imx->mode == HS || lpi2c_imx->mode == ULTRA_FAST)
>
> if (lpi2c_imx->mode > FAST_PLUS)
>
> > + clkcfg.filtscl = clkcfg.filtsda = 0;
> > + else
> > + clkcfg.filtscl = clkcfg.filtsda = 2;
> > +
>
> Multiple assignments on a single line are not welcome, in this case one variable
> "filt" assigned to 0 or 2 should be enough.
Thanks, will change it in next version.
> Why do you need struct lpi2c_imx_clkcfg in general?
>
> clkcfg.filtscl, clkcfg.filtsda etc. are all used locally inside this function only, it
> should be sufficient to replace "clkcfg"
> with a number of local variables.
Yes, you are right. Will change it in next version. Thanks!
> > + for (prescale = 0; prescale <= 7; prescale++) {
> > + clk_cycle = per_clk_rate / ((1 << prescale) * lpi2c_imx->bitrate)
> > + - 3 - (clkcfg.filtscl >> 1);
> > + clk_high = (clk_cycle + I2C_CLK_RATIO) / (I2C_CLK_RATIO + 1);
> > + clk_low = clk_cycle - clk_high;
> > + if (clk_low < 64)
> > + break;
> > + }
> > +
> > + if (prescale > 7)
> > + return -EINVAL;
> > +
> > + clkcfg.prescale = prescale;
> > + clkcfg.sethold = clk_high;
> > + clkcfg.clklo = clk_low;
> > + clkcfg.clkhi = clk_high;
> > + clkcfg.datavd = clk_high >> 1;
>
> Useless duplication of variables, see a note above.
Thanks!
> > +
> > + /* set MCFGR1: PINCFG, PRESCALE, IGNACK */
> > + if (lpi2c_imx->mode == ULTRA_FAST)
> > + pincfg = TWO_PIN_OO;
> > + else
> > + pincfg = TWO_PIN_OD;
> > + temp = clkcfg.prescale | pincfg << 24;
> > +
> > + if (lpi2c_imx->mode == ULTRA_FAST)
> > + temp |= MCFGR1_IGNACK;
> > +
> > + writel(temp, lpi2c_imx->base + LPI2C_MCFGR1);
> > +
> > + /* set MCFGR2: FILTSDA, FILTSCL */
> > + temp = (clkcfg.filtscl << 16) | (clkcfg.filtsda << 24);
> > + writel(temp, lpi2c_imx->base + LPI2C_MCFGR2);
> > +
> > +
> > + /* set MCCR: DATAVD, SETHOLD, CLKHI, CLKLO */
> > + temp = clkcfg.datavd << 24 | clkcfg.sethold << 16 |
> > + clkcfg.clkhi << 8 | clkcfg.clklo;
> > +
> > + if (lpi2c_imx->mode == HS)
> > + writel(temp, lpi2c_imx->base + LPI2C_MCCR1);
> > + else
> > + writel(temp, lpi2c_imx->base + LPI2C_MCCR0);
> > +
> > + return 0;
> > +}
> > +
> > +static int lpi2c_imx_master_enable(struct lpi2c_imx_struct
> > +*lpi2c_imx) {
> > + int ret;
> > + unsigned int temp;
> > +
> > + ret = clk_prepare_enable(lpi2c_imx->per_clk);
>
> You can do clk_prepare() in probe function and clk_unprepare() in remove
> function to avoid potential sleeping in runtime, then here you just do
> clk_enable()/clk_disable().
Thanks, will change it in next version!
> > + if (ret)
> > + return ret;
> > +
> > + temp = MCR_RST;
> > + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> > + writel(0, lpi2c_imx->base + LPI2C_MCR);
> > +
> > + ret = lpi2c_imx_config(lpi2c_imx);
> > + if (ret)
> > + return ret;
> > +
> > + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> > + temp |= MCR_MEN;
> > + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> > +
> > + return 0;
> > +}
> > +
> > +static int lpi2c_imx_master_disable(struct lpi2c_imx_struct
> > +*lpi2c_imx) {
> > + unsigned int temp = 0;
> > +
> > + temp = readl(lpi2c_imx->base + LPI2C_MCR);
> > + temp &= ~MCR_MEN;
> > + writel(temp, lpi2c_imx->base + LPI2C_MCR);
> > +
> > + clk_disable_unprepare(lpi2c_imx->per_clk);
>
> See a note above.
Thanks!
> > +
> > + return 0;
> > +}
> > +
> > +static int lpi2c_imx_msg_complete(struct lpi2c_imx_struct *lpi2c_imx)
> > +{
> > + unsigned int timeout;
> > +
> > + timeout = wait_for_completion_timeout(&lpi2c_imx->complete, HZ);
> > +
> > + return timeout ? 0 : -ETIMEDOUT;
> > +}
> > +
> > +static int lpi2c_imx_txfifo_empty(struct lpi2c_imx_struct *lpi2c_imx)
> > +{
> > + u32 txcnt;
> > + unsigned long orig_jiffies = jiffies;
> > +
> > + do {
> > + txcnt = readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff;
> > +
> > + if (readl(lpi2c_imx->base + LPI2C_MSR) & MSR_NDF) {
> > + dev_dbg(&lpi2c_imx->adapter.dev, "NDF detected\n");
> > + return -EIO;
> > + }
> > +
> > + if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> > + dev_dbg(&lpi2c_imx->adapter.dev, "txfifo empty
> timeout\n");
> > + return -ETIMEDOUT;
> > + }
> > + schedule();
> > +
> > + } while (txcnt);
> > +
> > + return 0;
> > +}
> > +
> > +static void lpi2c_imx_set_tx_watermark(struct lpi2c_imx_struct
> > +*lpi2c_imx) {
> > + unsigned int temp;
> > +
> > + temp = LPI2C_TX_FIFOSIZE >> 1;
> > + writel(temp, lpi2c_imx->base + LPI2C_MFCR);
>
> writel(LPI2C_TX_FIFOSIZE >> 1, lpi2c_imx->base + LPI2C_MFCR);
Thanks, will change it in next version.
> > +}
> > +
> > +static void lpi2c_imx_set_rx_watermark(struct lpi2c_imx_struct
> > +*lpi2c_imx) {
> > + unsigned int temp, remaining;
> > +
> > + remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
> > +
> > + if (remaining > (LPI2C_RX_FIFOSIZE >> 1))
> > + temp = LPI2C_RX_FIFOSIZE >> 1;
> > + else
> > + temp = 0;
> > +
> > + writel(temp << 16, lpi2c_imx->base + LPI2C_MFCR); }
> > +
> > +static void lpi2c_imx_write_txfifo(struct lpi2c_imx_struct
> > +*lpi2c_imx) {
> > + unsigned int data, txcnt;
> > +
> > + txcnt = readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff;
>
> Add an empty line here.
Thanks!
> > + while (txcnt < LPI2C_TX_FIFOSIZE) {
> > + if (lpi2c_imx->delivered == lpi2c_imx->msglen)
> > + break;
>
> Add an empty line here.
Thanks!
> > + data = lpi2c_imx->tx_buf[lpi2c_imx->delivered++];
> > + writel(data, lpi2c_imx->base + LPI2C_MTDR);
> > + txcnt++;
> > + }
> > +
> > + if (lpi2c_imx->delivered < lpi2c_imx->msglen)
> > + lpi2c_imx_intctrl(lpi2c_imx, MIER_TDIE | MIER_NDIE);
> > + else
> > + complete(&lpi2c_imx->complete);
> > +}
> > +
> > +static void lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx)
> > +{
> > + unsigned int temp, data;
> > + unsigned int blocklen, remaining;
> > +
> > + do {
> > + data = readl(lpi2c_imx->base + LPI2C_MRDR);
> > + if (data & MRDR_RXEMPTY)
> > + break;
> Add an empty line here.
Thanks!
>
> > + lpi2c_imx->rx_buf[lpi2c_imx->delivered++] = data & 0xff;
> > + } while (1);
> > +
> > + /*
> > + * First byte is the length of remaining packet in the SMBus block
> > + * data read. Add it to msgs->len.
> > + */
> > + if (lpi2c_imx->block_data) {
> > + blocklen = lpi2c_imx->rx_buf[0];
> > + lpi2c_imx->msglen += blocklen;
> > + }
> > +
> > + remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
> > + /* not finished, still waiting for rx data */
>
> Please move the comment under if (remaining) condition.
Thanks, will change it in next version!
> > + if (remaining) {
> > + lpi2c_imx_set_rx_watermark(lpi2c_imx);
> > + /* multiple receive commands */
> > + if (lpi2c_imx->block_data) {
> > + lpi2c_imx->block_data = 0;
> > + temp = remaining;
> > + temp |= (RECV_DATA << 8);
> > + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> > + } else if (!(lpi2c_imx->delivered & 0xff)) {
> > + temp = remaining > CHUNK_DATA ?
> > + CHUNK_DATA - 1 : (remaining - 1);
> > + temp |= (RECV_DATA << 8);
> > + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> > + }
> > +
> > + lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE);
> > + } else
> > + complete(&lpi2c_imx->complete);
>
> Start it from
>
> if (!remaining) {
> complete(&lpi2c_imx->complete);
> return;
> }
>
> /* not finished, still waiting for rx data */ ....
>
> Then you get less indentations. Generally please use more return points instead
> of if-if-if constructions.
>
Thanks, will change it in next version!
> > +}
> > +
> > +static void lpi2c_imx_write(struct lpi2c_imx_struct *lpi2c_imx,
> > + struct i2c_msg *msgs)
> > +{
> > + lpi2c_imx->tx_buf = msgs->buf;
> > + lpi2c_imx_set_tx_watermark(lpi2c_imx);
> > + lpi2c_imx_write_txfifo(lpi2c_imx);
> > +}
> > +
> > +static void lpi2c_imx_read(struct lpi2c_imx_struct *lpi2c_imx,
> > + struct i2c_msg *msgs)
> > +{
> > + unsigned int temp;
> > +
> > + lpi2c_imx->rx_buf = msgs->buf;
> > + lpi2c_imx->block_data = msgs->flags & I2C_M_RECV_LEN;
> > +
> > + lpi2c_imx_set_rx_watermark(lpi2c_imx);
> > + temp = msgs->len > CHUNK_DATA ? CHUNK_DATA - 1 : msgs->len - 1;
> > + temp |= (RECV_DATA << 8);
> > + writel(temp, lpi2c_imx->base + LPI2C_MTDR);
> > +
> > + lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE | MIER_NDIE); }
> > +
> > +static int lpi2c_imx_xfer(struct i2c_adapter *adapter,
> > + struct i2c_msg *msgs, int num)
> > +{
> > + int i, result;
> > + unsigned int temp;
> > + struct lpi2c_imx_struct *lpi2c_imx = i2c_get_adapdata(adapter);
> > +
> > + result = lpi2c_imx_master_enable(lpi2c_imx);
> > + if (result)
> > + return result;
> > +
> > + for (i = 0; i < num; i++) {
> > + result = lpi2c_imx_start(lpi2c_imx, &msgs[i]);
> > + if (result)
> > + goto disable;
> > +
> > + /* quick smbus */
> > + if (num == 1 && msgs[0].len == 0)
> > + goto stop;
> > +
> > + lpi2c_imx->delivered = 0;
> > + lpi2c_imx->msglen = msgs[i].len;
> > + init_completion(&lpi2c_imx->complete);
> > +
> > + if (msgs[i].flags & I2C_M_RD)
> > + lpi2c_imx_read(lpi2c_imx, &msgs[i]);
> > + else
> > + lpi2c_imx_write(lpi2c_imx, &msgs[i]);
> > +
> > + result = lpi2c_imx_msg_complete(lpi2c_imx);
> > + if (result)
> > + goto stop;
> > +
> > + if (!(msgs[i].flags & I2C_M_RD)) {
> > + result = lpi2c_imx_txfifo_empty(lpi2c_imx);
> > + if (result)
> > + goto stop;
> > + }
> > + }
> > +
> > +stop:
> > + lpi2c_imx_stop(lpi2c_imx);
> > +
> > + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> > + if ((temp & MSR_NDF) && !result)
> > + result = -EIO;
>
> Zero-length transactions are not supported, right?
The driver support zero-length transactions. For zero-length transactions, the transfer direction field represents data field.
It is transferred with i2c start CMD.
> > +
> > +disable:
> > + lpi2c_imx_master_disable(lpi2c_imx);
> > +
> > + dev_dbg(&lpi2c_imx->adapter.dev, "<%s> exit with: %s: %d\n",
> __func__,
> > + (result < 0) ? "error" : "success msg",
> > + (result < 0) ? result : num);
> > +
> > + return (result < 0) ? result : num;
> > +}
> > +
> > +static irqreturn_t lpi2c_imx_isr(int irq, void *dev_id) {
> > + unsigned int temp;
> > + struct lpi2c_imx_struct *lpi2c_imx = dev_id;
> > +
> > + lpi2c_imx_intctrl(lpi2c_imx, 0);
> > + temp = readl(lpi2c_imx->base + LPI2C_MSR);
> > +
> > + if (temp & MSR_RDF) {
> > + lpi2c_imx_read_rxfifo(lpi2c_imx);
> > + return IRQ_HANDLED;
> > + }
> > +
> > + if (temp & MSR_TDF) {
> > + lpi2c_imx_write_txfifo(lpi2c_imx);
> > + return IRQ_HANDLED;
> > + }
> > +
> > + complete(&lpi2c_imx->complete);
>
> Add an empty line here.
Thanks, will change it in next version.
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static u32 lpi2c_imx_func(struct i2c_adapter *adapter) {
> > + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL
> > + | I2C_FUNC_SMBUS_READ_BLOCK_DATA;
>
> checkpatch does not complain? I expect it should be
>
> return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
> I2C_FUNC_SMBUS_READ_BLOCK_DATA;
>
Thanks, will change it in next version!
> > +}
> > +
> > +static struct i2c_algorithm lpi2c_imx_algo = {
> > + .master_xfer = lpi2c_imx_xfer,
> > + .functionality = lpi2c_imx_func,
> > +};
> > +
> > +static const struct of_device_id lpi2c_imx_of_match[] = {
> > + { .compatible = "fsl,imx8dv-lpi2c" },
> > + { .compatible = "fsl,imx7ulp-lpi2c" },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, lpi2c_imx_of_match)
> > +
> > +static int lpi2c_imx_probe(struct platform_device *pdev) {
> > + int irq, ret;
> > + void __iomem *base;
> > + struct resource *res;
> > + struct lpi2c_imx_struct *lpi2c_imx;
> > +
> > + lpi2c_imx = devm_kzalloc(&pdev->dev,
> > + sizeof(*lpi2c_imx), GFP_KERNEL);
> > + if (!lpi2c_imx)
> > + return -ENOMEM;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + base = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(base))
> > + return PTR_ERR(base);
> > +
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0) {
> > + dev_err(&pdev->dev, "can't get irq number\n");
> > + return irq;
> > + }
> > +
> > + lpi2c_imx->adapter.owner = THIS_MODULE;
> > + lpi2c_imx->adapter.algo = &lpi2c_imx_algo;
> > + lpi2c_imx->adapter.dev.parent = &pdev->dev;
> > + lpi2c_imx->adapter.nr = pdev->id;
>
> Do you really need it? Please consider to use i2c_add_adapter().
You are right, i2c_add_adapter() is a better option. Will change it in next version, Thanks.
> > + lpi2c_imx->base = base;
>
> For sake of consistency please initialize lpi2c_imx->adapter fields in a row.
>
> You don't need this local 'base' variable, use lpi2c_imx->base instead.
Thanks, Will change it in next version.
> > + lpi2c_imx->adapter.dev.of_node = pdev->dev.of_node;
> > + strlcpy(lpi2c_imx->adapter.name, pdev->name,
> > + sizeof(lpi2c_imx->adapter.name));
> > +
> > + lpi2c_imx->per_clk = devm_clk_get(&pdev->dev, NULL);
> > + if (IS_ERR(lpi2c_imx->per_clk)) {
> > + dev_err(&pdev->dev, "can't get I2C peripheral clock\n");
> > + return PTR_ERR(lpi2c_imx->per_clk);
> > + }
> > +
> > + ret = of_property_read_u32(pdev->dev.of_node,
> > + "clock-frequency", &lpi2c_imx->bitrate);
> > + if (ret)
> > + lpi2c_imx->bitrate = LPI2C_DEFAULT_RATE;
> > +
> > + ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr, 0,
> > + pdev->name, lpi2c_imx);
> > + if (ret) {
> > + dev_err(&pdev->dev, "can't claim irq %d\n", irq);
> > + goto ret;
>
> Just return ret;
Thanks!
> > + }
> > +
> > + i2c_set_adapdata(&lpi2c_imx->adapter, lpi2c_imx);
> > + platform_set_drvdata(pdev, lpi2c_imx);
> > +
> > + ret = i2c_add_numbered_adapter(&lpi2c_imx->adapter);
> > + if (ret) {
> > + dev_err(&pdev->dev, "registration failed\n");
> > + goto ret;
>
> Just return ret;
Thanks!
> > + }
> > +
> > + dev_info(&lpi2c_imx->adapter.dev, "LPI2C adapter registered\n");
> > +
> > +ret:
> > + return ret;
>
> return 0;
Thanks!
> > +}
> > +
> > +static int lpi2c_imx_remove(struct platform_device *pdev) {
> > + struct lpi2c_imx_struct *lpi2c_imx = platform_get_drvdata(pdev);
> > +
> > + i2c_del_adapter(&lpi2c_imx->adapter);
> > +
> > + return 0;
> > +}
> > +
> > +static struct platform_driver lpi2c_imx_driver = {
> > + .probe = lpi2c_imx_probe,
> > + .remove = lpi2c_imx_remove,
> > + .driver = {
> > + .name = DRIVER_NAME,
> > + .of_match_table = lpi2c_imx_of_match,
> > + },
> > +};
> > +
> > +static int __init i2c_adap_imx_init(void) {
> > + return platform_driver_register(&lpi2c_imx_driver);
> > +}
> > +module_init(i2c_adap_imx_init);
> > +
> > +static void __exit i2c_adap_imx_exit(void) {
> > + platform_driver_unregister(&lpi2c_imx_driver);
> > +}
> > +module_exit(i2c_adap_imx_exit);
> > +
>
> Please use module_platform_driver(lpi2c_imx_driver);
Thanks, will change it in next version!
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_AUTHOR("Gao Pan <pandy.gao@nxp.com>");
> MODULE_DESCRIPTION("I2C
> > +adapter driver for LPI2C bus"); MODULE_ALIAS("platform:"
> > +DRIVER_NAME);
> >
>
> Are you sure that the driver needs a platform alias here?
Thanks, will remove it in next version.
Thanks again for your precise review, it really helps to improve the code quality!
Best Regards
Gao Pan
^ permalink raw reply [flat|nested] 6+ messages in thread