diff mbox

[V3] i2c: imx: add low power i2c bus driver

Message ID 1471420784-11727-1-git-send-email-pandy.gao@nxp.com
State Superseded
Headers show

Commit Message

Gao Pan Aug. 17, 2016, 7:59 a.m. UTC
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(+)

Comments

Gao Pan Aug. 26, 2016, 7:37 a.m. UTC | #1
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
Gao Pan Sept. 9, 2016, 1:49 a.m. UTC | #2
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
Gao Pan Oct. 24, 2016, 5:51 a.m. UTC | #3
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
--
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
Vladimir Zapolskiy Oct. 24, 2016, 11:15 p.m. UTC | #4
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
--
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
Gao Pan Nov. 14, 2016, 9:21 a.m. UTC | #5
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
--
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
diff mbox

Patch

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);