2021-04-07 00:59:37

by Clark Wang

[permalink] [raw]
Subject: [PATCH V2 00/18] i2c: imx-lpi2c: New features and bug fixes

Hi,

According to V1's feedback, V2 has been modified.
For summary of the changes, please refer to the header of each patch file.

Added some dt-bindings and dts patches.
At the same time, a patch has been added to fix the problem that data larger
than 256 bytes cannot be sent in one frame in PIO mode.

Clark Wang (14):
i2c: imx-lpi2c: add ipg clk for lpi2c driver
ARM: dts: imx7ulp: add the missing lpi2c ipg clock
ARM: dts: imx7ulp: add the missing lpi2c nodes
ARM64: dts: imx8: add the missing lpi2c ipg clock
ARM64: dts: imx8: change i2c irq number to non-combined
i2c: imx-lpi2c: increase PM timeout to avoid operate clk frequently
i2c: imx-lpi2c: add bus recovery feature
dt-bindings: i2c: imx-lpi2c: Add bus recovery example
i2c: imx-lpi2c: fix i2c timing issue
i2c: imx-lpi2c: fix type char overflow issue when calculating the
clock cycle
i2c: imx-lpi2c: add edma mode support
dt-bindings: i2c: imx-lpi2c: Add dma configuration example
ARM: dts: imx7ulp: add dma configurations for lpi2c
ARM: dts: imx7ulp: add the missing status property of lpi2c5
i2c: imx-lpi2c: fix pio mode cannot send 256+ bytes in one frame

Fugang Duan (1):
i2c: imx-lpi2c: manage irq resource request/release in runtime pm

Gao Pan (2):
i2c: imx-lpi2c: directly retrun ISR when detect a NACK
i2c: imx-lpi2c: add debug message when i2c peripheral clk doesn't work

.../bindings/i2c/i2c-imx-lpi2c.yaml | 26 +
arch/arm/boot/dts/imx7ulp.dtsi | 50 +-
.../arm64/boot/dts/freescale/imx8-ss-dma.dtsi | 32 +-
drivers/i2c/busses/i2c-imx-lpi2c.c | 506 ++++++++++++++++--
4 files changed, 548 insertions(+), 66 deletions(-)

--
2.25.1


2021-04-07 00:59:40

by Clark Wang

[permalink] [raw]
Subject: [PATCH V2 01/18] i2c: imx-lpi2c: directly retrun ISR when detect a NACK

From: Gao Pan <[email protected]>

A NACK flag in ISR means i2c bus error. In such codition,
there is no need to do read/write operation. It's better
to return ISR directly and then stop i2c transfer.

Signed-off-by: Gao Pan <[email protected]>
Signed-off-by: Clark Wang <[email protected]>
Reviewed-by: Dong Aisheng <[email protected]>
---
V2 changes:
- No change. Has been reviewed by Dong Aisheng <[email protected]> in V1.
---
drivers/i2c/busses/i2c-imx-lpi2c.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 9db6ccded5e9..bbf44ac95021 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -507,15 +507,17 @@ static irqreturn_t lpi2c_imx_isr(int irq, void *dev_id)
lpi2c_imx_intctrl(lpi2c_imx, 0);
temp = readl(lpi2c_imx->base + LPI2C_MSR);

+ if (temp & MSR_NDF) {
+ complete(&lpi2c_imx->complete);
+ goto ret;
+ }
+
if (temp & MSR_RDF)
lpi2c_imx_read_rxfifo(lpi2c_imx);
-
- if (temp & MSR_TDF)
+ else if (temp & MSR_TDF)
lpi2c_imx_write_txfifo(lpi2c_imx);

- if (temp & MSR_NDF)
- complete(&lpi2c_imx->complete);
-
+ret:
return IRQ_HANDLED;
}

--
2.25.1

2021-04-07 01:14:27

by Clark Wang

[permalink] [raw]
Subject: [PATCH V2 06/18] ARM64: dts: imx8: change i2c irq number to non-combined

Combined interrupt number may cause unexcepted irq event when using DMA
and too many interrupts will be generated.
So change all i2c interrupts number to non-combined for
imx8qxp/8qm/8dxl.

Signed-off-by: Clark Wang <[email protected]>
---
V2 changes:
- New patch added in V2
---
arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
index b5ed12a06538..9ba57f04859b 100644
--- a/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
@@ -110,7 +110,7 @@ uart3_lpcg: clock-controller@5a490000 {

i2c0: i2c@5a800000 {
reg = <0x5a800000 0x4000>;
- interrupts = <GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <GIC_SPI 340 IRQ_TYPE_LEVEL_HIGH>;
interrupt-parent = <&gic>;
clocks = <&i2c0_lpcg IMX_LPCG_CLK_0>,
<&i2c0_lpcg IMX_LPCG_CLK_4>;
@@ -123,7 +123,7 @@ i2c0: i2c@5a800000 {

i2c1: i2c@5a810000 {
reg = <0x5a810000 0x4000>;
- interrupts = <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <GIC_SPI 341 IRQ_TYPE_LEVEL_HIGH>;
interrupt-parent = <&gic>;
clocks = <&i2c1_lpcg IMX_LPCG_CLK_0>,
<&i2c1_lpcg IMX_LPCG_CLK_4>;
@@ -136,7 +136,7 @@ i2c1: i2c@5a810000 {

i2c2: i2c@5a820000 {
reg = <0x5a820000 0x4000>;
- interrupts = <GIC_SPI 222 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <GIC_SPI 342 IRQ_TYPE_LEVEL_HIGH>;
interrupt-parent = <&gic>;
clocks = <&i2c2_lpcg IMX_LPCG_CLK_0>,
<&i2c2_lpcg IMX_LPCG_CLK_4>;
@@ -149,7 +149,7 @@ i2c2: i2c@5a820000 {

i2c3: i2c@5a830000 {
reg = <0x5a830000 0x4000>;
- interrupts = <GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <GIC_SPI 343 IRQ_TYPE_LEVEL_HIGH>;
interrupt-parent = <&gic>;
clocks = <&i2c3_lpcg IMX_LPCG_CLK_0>,
<&i2c3_lpcg IMX_LPCG_CLK_4>;
--
2.25.1

2021-04-07 01:14:26

by Clark Wang

[permalink] [raw]
Subject: [PATCH V2 05/18] ARM64: dts: imx8: add the missing lpi2c ipg clock

The lpi2c driver has add the missing ipg clock.
So add the ipg clock here for all lpi2c nodes.

Signed-off-by: Clark Wang <[email protected]>
---
V2 changes:
- New patch added in V2
---
.../arm64/boot/dts/freescale/imx8-ss-dma.dtsi | 24 ++++++++++++-------
1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
index 960a802b8b6e..b5ed12a06538 100644
--- a/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
@@ -111,8 +111,10 @@ uart3_lpcg: clock-controller@5a490000 {
i2c0: i2c@5a800000 {
reg = <0x5a800000 0x4000>;
interrupts = <GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&i2c0_lpcg IMX_LPCG_CLK_0>;
- clock-names = "per";
+ interrupt-parent = <&gic>;
+ clocks = <&i2c0_lpcg IMX_LPCG_CLK_0>,
+ <&i2c0_lpcg IMX_LPCG_CLK_4>;
+ clock-names = "per", "ipg";
assigned-clocks = <&clk IMX_SC_R_I2C_0 IMX_SC_PM_CLK_PER>;
assigned-clock-rates = <24000000>;
power-domains = <&pd IMX_SC_R_I2C_0>;
@@ -122,8 +124,10 @@ i2c0: i2c@5a800000 {
i2c1: i2c@5a810000 {
reg = <0x5a810000 0x4000>;
interrupts = <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&i2c1_lpcg IMX_LPCG_CLK_0>;
- clock-names = "per";
+ interrupt-parent = <&gic>;
+ clocks = <&i2c1_lpcg IMX_LPCG_CLK_0>,
+ <&i2c1_lpcg IMX_LPCG_CLK_4>;
+ clock-names = "per", "ipg";
assigned-clocks = <&clk IMX_SC_R_I2C_1 IMX_SC_PM_CLK_PER>;
assigned-clock-rates = <24000000>;
power-domains = <&pd IMX_SC_R_I2C_1>;
@@ -133,8 +137,10 @@ i2c1: i2c@5a810000 {
i2c2: i2c@5a820000 {
reg = <0x5a820000 0x4000>;
interrupts = <GIC_SPI 222 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&i2c2_lpcg IMX_LPCG_CLK_0>;
- clock-names = "per";
+ interrupt-parent = <&gic>;
+ clocks = <&i2c2_lpcg IMX_LPCG_CLK_0>,
+ <&i2c2_lpcg IMX_LPCG_CLK_4>;
+ clock-names = "per", "ipg";
assigned-clocks = <&clk IMX_SC_R_I2C_2 IMX_SC_PM_CLK_PER>;
assigned-clock-rates = <24000000>;
power-domains = <&pd IMX_SC_R_I2C_2>;
@@ -144,8 +150,10 @@ i2c2: i2c@5a820000 {
i2c3: i2c@5a830000 {
reg = <0x5a830000 0x4000>;
interrupts = <GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&i2c3_lpcg IMX_LPCG_CLK_0>;
- clock-names = "per";
+ interrupt-parent = <&gic>;
+ clocks = <&i2c3_lpcg IMX_LPCG_CLK_0>,
+ <&i2c3_lpcg IMX_LPCG_CLK_4>;
+ clock-names = "per", "ipg";
assigned-clocks = <&clk IMX_SC_R_I2C_3 IMX_SC_PM_CLK_PER>;
assigned-clock-rates = <24000000>;
power-domains = <&pd IMX_SC_R_I2C_3>;
--
2.25.1

2021-04-07 01:14:28

by Clark Wang

[permalink] [raw]
Subject: [PATCH V2 08/18] i2c: imx-lpi2c: add debug message when i2c peripheral clk doesn't work

From: Gao Pan <[email protected]>

add debug message when i2c peripheral clk rate is 0, then
directly return -EINVAL.

Signed-off-by: Clark Wang <[email protected]>
Signed-off-by: Gao Pan <[email protected]>
Reviewed-by: Andy Duan <[email protected]>
---
V2 changes:
- Add my signed-off.
---
drivers/i2c/busses/i2c-imx-lpi2c.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 333209ba81c1..dfec334712c2 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -210,6 +210,11 @@ static int lpi2c_imx_config(struct lpi2c_imx_struct *lpi2c_imx)
lpi2c_imx_set_mode(lpi2c_imx);

clk_rate = clk_get_rate(lpi2c_imx->clk_per);
+ if (!clk_rate) {
+ dev_dbg(&lpi2c_imx->adapter.dev, "clk_per rate is 0\n");
+ return -EINVAL;
+ }
+
if (lpi2c_imx->mode == HS || lpi2c_imx->mode == ULTRA_FAST)
filt = 0;
else
--
2.25.1

2021-04-07 01:25:49

by Clark Wang

[permalink] [raw]
Subject: [PATCH V2 16/18] ARM: dts: imx7ulp: add dma configurations for lpi2c

Enable dma support for all lpi2c nodes.

Signed-off-by: Clark Wang <[email protected]>
---
V2 changes:
- New patch added in V2
---
arch/arm/boot/dts/imx7ulp.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/imx7ulp.dtsi b/arch/arm/boot/dts/imx7ulp.dtsi
index 0c51fa79c0bc..a6681836ca05 100644
--- a/arch/arm/boot/dts/imx7ulp.dtsi
+++ b/arch/arm/boot/dts/imx7ulp.dtsi
@@ -157,6 +157,8 @@ lpi2c4: lpi2c4@402b0000 {
assigned-clocks = <&pcc2 IMX7ULP_CLK_LPI2C4>;
assigned-clock-parents = <&scg1 IMX7ULP_CLK_FIRC>;
assigned-clock-rates = <48000000>;
+ dmas = <&edma1 0 10>, <&edma1 0 9>;
+ dma-names = "tx","rx";
status = "disabled";
};

@@ -170,6 +172,8 @@ lpi2c5: lpi2c5@402c0000 {
assigned-clocks = <&pcc2 IMX7ULP_CLK_LPI2C5>;
assigned-clock-parents = <&scg1 IMX7ULP_CLK_FIRC>;
assigned-clock-rates = <48000000>;
+ dmas = <&edma1 0 12>, <&edma1 0 11>;
+ dma-names = "tx","rx";
};

lpuart4: serial@402d0000 {
@@ -361,6 +365,8 @@ lpi2c6: i2c@40a40000 {
assigned-clocks = <&pcc3 IMX7ULP_CLK_LPI2C6>;
assigned-clock-parents = <&scg1 IMX7ULP_CLK_FIRC>;
assigned-clock-rates = <48000000>;
+ dmas = <&edma1 0 14>, <&edma1 0 13>;
+ dma-names = "tx","rx";
status = "disabled";
};

@@ -374,6 +380,8 @@ lpi2c7: i2c@40a50000 {
assigned-clocks = <&pcc3 IMX7ULP_CLK_LPI2C7>;
assigned-clock-parents = <&scg1 IMX7ULP_CLK_FIRC>;
assigned-clock-rates = <48000000>;
+ dmas = <&edma1 0 16>, <&edma1 0 15>;
+ dma-names = "tx","rx";
status = "disabled";
};

--
2.25.1

2021-04-07 01:28:19

by Clark Wang

[permalink] [raw]
Subject: [PATCH V2 14/18] i2c: imx-lpi2c: add edma mode support

Add eDMA receive and send mode support.
Support to read and write data larger than 256 bytes in one frame.

Signed-off-by: Clark Wang <[email protected]>
Reviewed-by: Li Jun <[email protected]>
---
V2 changes:
- change marco I2C_USE_PIO to DMA_ERR_I2C_USE_PIO. It is a error code defined
in this driver to
---
drivers/i2c/busses/i2c-imx-lpi2c.c | 290 ++++++++++++++++++++++++++++-
1 file changed, 288 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index c2f8e49660ea..d1a56d52f19f 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -8,6 +8,8 @@
#include <linux/clk.h>
#include <linux/completion.h>
#include <linux/delay.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
#include <linux/err.h>
#include <linux/errno.h>
#include <linux/i2c.h>
@@ -31,6 +33,7 @@
#define LPI2C_MCR 0x10 /* i2c contrl register */
#define LPI2C_MSR 0x14 /* i2c status register */
#define LPI2C_MIER 0x18 /* i2c interrupt enable */
+#define LPI2C_MDER 0x1C /* i2c DMA enable */
#define LPI2C_MCFGR0 0x20 /* i2c master configuration */
#define LPI2C_MCFGR1 0x24 /* i2c master configuration */
#define LPI2C_MCFGR2 0x28 /* i2c master configuration */
@@ -72,11 +75,15 @@
#define MCFGR1_AUTOSTOP BIT(8)
#define MCFGR1_IGNACK BIT(9)
#define MRDR_RXEMPTY BIT(14)
+#define MDER_TDDE BIT(0)
+#define MDER_RDDE BIT(1)

#define I2C_CLK_RATIO 24 / 59
#define CHUNK_DATA 256

#define I2C_PM_TIMEOUT 1000 /* ms */
+#define I2C_DMA_THRESHOLD 16 /* bytes */
+#define DMA_ERR_I2C_USE_PIO (-150)

enum lpi2c_imx_mode {
STANDARD, /* <=100Kbps */
@@ -95,6 +102,7 @@ enum lpi2c_imx_pincfg {

struct lpi2c_imx_struct {
struct i2c_adapter adapter;
+ resource_size_t phy_addr;
int irq;
struct clk *clk_per;
struct clk *clk_ipg;
@@ -114,6 +122,17 @@ struct lpi2c_imx_struct {
struct pinctrl *pinctrl;
struct pinctrl_state *pinctrl_pins_default;
struct pinctrl_state *pinctrl_pins_gpio;
+
+ bool can_use_dma;
+ bool using_dma;
+ bool xferred;
+ struct i2c_msg *msg;
+ dma_addr_t dma_addr;
+ struct dma_chan *dma_tx;
+ struct dma_chan *dma_rx;
+ enum dma_data_direction dma_direction;
+ u8 *dma_buf;
+ unsigned int dma_len;
};

static void lpi2c_imx_intctrl(struct lpi2c_imx_struct *lpi2c_imx,
@@ -289,6 +308,9 @@ static int lpi2c_imx_master_enable(struct lpi2c_imx_struct *lpi2c_imx)
if (ret)
goto rpm_put;

+ if (lpi2c_imx->can_use_dma)
+ writel(MDER_TDDE | MDER_RDDE, lpi2c_imx->base + LPI2C_MDER);
+
temp = readl(lpi2c_imx->base + LPI2C_MCR);
temp |= MCR_MEN;
writel(temp, lpi2c_imx->base + LPI2C_MCR);
@@ -462,6 +484,154 @@ static void lpi2c_imx_read(struct lpi2c_imx_struct *lpi2c_imx,
lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE | MIER_NDIE);
}

+static void lpi2c_dma_unmap(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ struct dma_chan *chan = lpi2c_imx->dma_direction == DMA_FROM_DEVICE
+ ? lpi2c_imx->dma_rx : lpi2c_imx->dma_tx;
+
+ dma_unmap_single(chan->device->dev, lpi2c_imx->dma_addr,
+ lpi2c_imx->dma_len, lpi2c_imx->dma_direction);
+
+ lpi2c_imx->dma_direction = DMA_NONE;
+}
+
+static void lpi2c_cleanup_dma(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ if (lpi2c_imx->dma_direction == DMA_NONE)
+ return;
+ else if (lpi2c_imx->dma_direction == DMA_FROM_DEVICE)
+ dmaengine_terminate_all(lpi2c_imx->dma_rx);
+ else if (lpi2c_imx->dma_direction == DMA_TO_DEVICE)
+ dmaengine_terminate_all(lpi2c_imx->dma_tx);
+
+ lpi2c_dma_unmap(lpi2c_imx);
+}
+
+static void lpi2c_dma_callback(void *data)
+{
+ struct lpi2c_imx_struct *lpi2c_imx = (struct lpi2c_imx_struct *)data;
+
+ lpi2c_dma_unmap(lpi2c_imx);
+ writel(GEN_STOP << 8, lpi2c_imx->base + LPI2C_MTDR);
+ lpi2c_imx->xferred = true;
+
+ complete(&lpi2c_imx->complete);
+}
+
+static int lpi2c_dma_submit(struct lpi2c_imx_struct *lpi2c_imx,
+ struct i2c_msg *msg)
+{
+ bool read = msg->flags & I2C_M_RD;
+ enum dma_data_direction dir = read ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+ struct dma_chan *chan = read ? lpi2c_imx->dma_rx : lpi2c_imx->dma_tx;
+ struct dma_async_tx_descriptor *txdesc;
+ dma_cookie_t cookie;
+
+ lpi2c_imx->dma_len = read ? msg->len - 1 : msg->len;
+ lpi2c_imx->msg = msg;
+ lpi2c_imx->dma_direction = dir;
+
+ if (IS_ERR(chan))
+ return PTR_ERR(chan);
+
+ lpi2c_imx->dma_addr = dma_map_single(chan->device->dev,
+ lpi2c_imx->dma_buf,
+ lpi2c_imx->dma_len, dir);
+ if (dma_mapping_error(chan->device->dev, lpi2c_imx->dma_addr)) {
+ dev_err(&lpi2c_imx->adapter.dev, "dma map failed, use pio\n");
+ return -EINVAL;
+ }
+
+ txdesc = dmaengine_prep_slave_single(chan, lpi2c_imx->dma_addr,
+ lpi2c_imx->dma_len, read ?
+ DMA_DEV_TO_MEM : DMA_MEM_TO_DEV,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ if (!txdesc) {
+ dev_err(&lpi2c_imx->adapter.dev, "dma prep slave sg failed, use pio\n");
+ lpi2c_cleanup_dma(lpi2c_imx);
+ return -EINVAL;
+ }
+
+ reinit_completion(&lpi2c_imx->complete);
+ txdesc->callback = lpi2c_dma_callback;
+ txdesc->callback_param = (void *)lpi2c_imx;
+
+ cookie = dmaengine_submit(txdesc);
+ if (dma_submit_error(cookie)) {
+ dev_err(&lpi2c_imx->adapter.dev, "submitting dma failed, use pio\n");
+ lpi2c_cleanup_dma(lpi2c_imx);
+ return -EINVAL;
+ }
+
+ lpi2c_imx_intctrl(lpi2c_imx, MIER_NDIE);
+
+ dma_async_issue_pending(chan);
+
+ return 0;
+}
+
+static bool is_use_dma(struct lpi2c_imx_struct *lpi2c_imx, struct i2c_msg *msg)
+{
+ if (!lpi2c_imx->can_use_dma)
+ return false;
+
+ if (msg->len < I2C_DMA_THRESHOLD)
+ return false;
+
+ return true;
+}
+
+static int lpi2c_imx_dma_push_rx_cmd(struct lpi2c_imx_struct *lpi2c_imx,
+ struct i2c_msg *msg)
+{
+ unsigned int temp, rx_remain;
+ unsigned long orig_jiffies = jiffies;
+
+ if ((msg->flags & I2C_M_RD)) {
+ rx_remain = msg->len;
+ do {
+ temp = rx_remain > CHUNK_DATA ?
+ CHUNK_DATA - 1 : rx_remain - 1;
+ temp |= (RECV_DATA << 8);
+ while ((readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff) > (lpi2c_imx->rxfifosize >> 1)) {
+ if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(1000))) {
+ dev_dbg(&lpi2c_imx->adapter.dev, "txfifo empty timeout\n");
+ if (lpi2c_imx->adapter.bus_recovery_info)
+ i2c_recover_bus(&lpi2c_imx->adapter);
+ return -ETIMEDOUT;
+ }
+ schedule();
+ }
+ writel(temp, lpi2c_imx->base + LPI2C_MTDR);
+ rx_remain = rx_remain - (temp & 0xff) - 1;
+ } while (rx_remain > 0);
+ }
+
+ return 0;
+}
+
+static int lpi2c_dma_xfer(struct lpi2c_imx_struct *lpi2c_imx,
+ struct i2c_msg *msg)
+{
+ int result;
+
+ result = lpi2c_dma_submit(lpi2c_imx, msg);
+ if (!result) {
+ result = lpi2c_imx_dma_push_rx_cmd(lpi2c_imx, msg);
+ if (result)
+ return result;
+ result = lpi2c_imx_msg_complete(lpi2c_imx);
+ return result;
+ }
+
+ /* DMA xfer failed, try to use PIO, clean up dma things */
+ i2c_put_dma_safe_msg_buf(lpi2c_imx->dma_buf, lpi2c_imx->msg,
+ lpi2c_imx->xferred);
+ lpi2c_cleanup_dma(lpi2c_imx);
+
+ return DMA_ERR_I2C_USE_PIO;
+}
+
static int lpi2c_imx_xfer(struct i2c_adapter *adapter,
struct i2c_msg *msgs, int num)
{
@@ -474,6 +644,9 @@ static int lpi2c_imx_xfer(struct i2c_adapter *adapter,
return result;

for (i = 0; i < num; i++) {
+ lpi2c_imx->xferred = false;
+ lpi2c_imx->using_dma = false;
+
result = lpi2c_imx_start(lpi2c_imx, &msgs[i]);
if (result)
goto disable;
@@ -482,9 +655,24 @@ static int lpi2c_imx_xfer(struct i2c_adapter *adapter,
if (num == 1 && msgs[0].len == 0)
goto stop;

+ if (is_use_dma(lpi2c_imx, &msgs[i])) {
+ lpi2c_imx->using_dma = true;
+
+ writel(0x1, lpi2c_imx->base + LPI2C_MFCR);
+
+ lpi2c_imx->dma_buf = i2c_get_dma_safe_msg_buf(&msgs[i],
+ I2C_DMA_THRESHOLD);
+ if (lpi2c_imx->dma_buf) {
+ result = lpi2c_dma_xfer(lpi2c_imx, &msgs[i]);
+ if (result != DMA_ERR_I2C_USE_PIO)
+ goto stop;
+ }
+ }
+
+ lpi2c_imx->using_dma = false;
lpi2c_imx->delivered = 0;
lpi2c_imx->msglen = msgs[i].len;
- init_completion(&lpi2c_imx->complete);
+ reinit_completion(&lpi2c_imx->complete);

if (msgs[i].flags & I2C_M_RD)
lpi2c_imx_read(lpi2c_imx, &msgs[i]);
@@ -503,7 +691,16 @@ static int lpi2c_imx_xfer(struct i2c_adapter *adapter,
}

stop:
- lpi2c_imx_stop(lpi2c_imx);
+ if (!lpi2c_imx->using_dma)
+ lpi2c_imx_stop(lpi2c_imx);
+ else {
+ i2c_put_dma_safe_msg_buf(lpi2c_imx->dma_buf, lpi2c_imx->msg,
+ lpi2c_imx->xferred);
+ if (result) {
+ lpi2c_cleanup_dma(lpi2c_imx);
+ writel(GEN_STOP << 8, lpi2c_imx->base + LPI2C_MTDR);
+ }
+ }

temp = readl(lpi2c_imx->base + LPI2C_MSR);
if ((temp & MSR_NDF) && !result)
@@ -528,6 +725,10 @@ static irqreturn_t lpi2c_imx_isr(int irq, void *dev_id)
temp = readl(lpi2c_imx->base + LPI2C_MSR);

if (temp & MSR_NDF) {
+ if (lpi2c_imx->using_dma) {
+ lpi2c_cleanup_dma(lpi2c_imx);
+ writel(GEN_STOP << 8, lpi2c_imx->base + LPI2C_MTDR);
+ }
complete(&lpi2c_imx->complete);
goto ret;
}
@@ -623,6 +824,77 @@ static const struct of_device_id lpi2c_imx_of_match[] = {
};
MODULE_DEVICE_TABLE(of, lpi2c_imx_of_match);

+static void lpi2c_dma_exit(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ if (lpi2c_imx->dma_rx) {
+ dma_release_channel(lpi2c_imx->dma_rx);
+ lpi2c_imx->dma_rx = NULL;
+ }
+
+ if (lpi2c_imx->dma_tx) {
+ dma_release_channel(lpi2c_imx->dma_tx);
+ lpi2c_imx->dma_tx = NULL;
+ }
+}
+
+static int lpi2c_dma_init(struct device *dev,
+ struct lpi2c_imx_struct *lpi2c_imx)
+{
+ int ret;
+ struct dma_slave_config dma_sconfig;
+
+ /* Prepare for TX DMA: */
+ lpi2c_imx->dma_tx = dma_request_chan(dev, "tx");
+ if (IS_ERR(lpi2c_imx->dma_tx)) {
+ ret = PTR_ERR(lpi2c_imx->dma_tx);
+ dev_err(dev, "can't get the TX DMA channel, error %d!\n", ret);
+ lpi2c_imx->dma_tx = NULL;
+ goto err;
+ }
+
+ dma_sconfig.dst_addr = lpi2c_imx->phy_addr + LPI2C_MTDR;
+ dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+ dma_sconfig.dst_maxburst = 1;
+ dma_sconfig.direction = DMA_MEM_TO_DEV;
+ ret = dmaengine_slave_config(lpi2c_imx->dma_tx, &dma_sconfig);
+ if (ret < 0) {
+ dev_err(dev, "can't configure tx channel (%d)\n", ret);
+ goto fail_tx;
+ }
+
+ /* Prepare for RX DMA: */
+ lpi2c_imx->dma_rx = dma_request_chan(dev, "rx");
+ if (IS_ERR(lpi2c_imx->dma_rx)) {
+ ret = PTR_ERR(lpi2c_imx->dma_rx);
+ dev_err(dev, "can't get the RX DMA channel, error %d\n", ret);
+ lpi2c_imx->dma_rx = NULL;
+ goto fail_tx;
+ }
+
+ dma_sconfig.src_addr = lpi2c_imx->phy_addr + LPI2C_MRDR;
+ dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+ dma_sconfig.src_maxburst = 1;
+ dma_sconfig.direction = DMA_DEV_TO_MEM;
+ ret = dmaengine_slave_config(lpi2c_imx->dma_rx, &dma_sconfig);
+ if (ret < 0) {
+ dev_err(dev, "can't configure rx channel (%d)\n", ret);
+ goto fail_rx;
+ }
+
+ lpi2c_imx->can_use_dma = true;
+ lpi2c_imx->using_dma = false;
+
+ return 0;
+fail_rx:
+ dma_release_channel(lpi2c_imx->dma_rx);
+fail_tx:
+ dma_release_channel(lpi2c_imx->dma_tx);
+err:
+ lpi2c_dma_exit(lpi2c_imx);
+ lpi2c_imx->can_use_dma = false;
+ return ret;
+}
+
static int lpi2c_imx_clocks_prepare(struct lpi2c_imx_struct *lpi2c_imx)
{
int ret = 0;
@@ -656,15 +928,18 @@ static int lpi2c_imx_probe(struct platform_device *pdev)
struct lpi2c_imx_struct *lpi2c_imx;
unsigned int temp;
int ret;
+ struct resource *res;

lpi2c_imx = devm_kzalloc(&pdev->dev, sizeof(*lpi2c_imx), GFP_KERNEL);
if (!lpi2c_imx)
return -ENOMEM;

+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
lpi2c_imx->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(lpi2c_imx->base))
return PTR_ERR(lpi2c_imx->base);

+ lpi2c_imx->phy_addr = (dma_addr_t)res->start;
lpi2c_imx->irq = platform_get_irq(pdev, 0);
if (lpi2c_imx->irq < 0)
return lpi2c_imx->irq;
@@ -716,6 +991,17 @@ static int lpi2c_imx_probe(struct platform_device *pdev)
if (ret == -EPROBE_DEFER)
goto rpm_disable;

+ /* Init DMA */
+ lpi2c_imx->dma_direction = DMA_NONE;
+ ret = lpi2c_dma_init(&pdev->dev, lpi2c_imx);
+ if (ret) {
+ dev_err_probe(&pdev->dev, ret, "dma setup error %d, use pio\n", ret);
+ if (ret == -EPROBE_DEFER)
+ goto rpm_disable;
+ }
+
+ init_completion(&lpi2c_imx->complete);
+
ret = i2c_add_adapter(&lpi2c_imx->adapter);
if (ret)
goto rpm_disable;
--
2.25.1

2021-05-11 03:02:47

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH V2 05/18] ARM64: dts: imx8: add the missing lpi2c ipg clock

On Tue, Apr 06, 2021 at 07:32:53PM +0800, Clark Wang wrote:
> The lpi2c driver has add the missing ipg clock.
> So add the ipg clock here for all lpi2c nodes.
>
> Signed-off-by: Clark Wang <[email protected]>

Historically, we use 'arm64: dts: ...' as subject prefix for arm64 DTS
patch, and 'ARM: dts: ...' for arm.

Shawn

> ---
> V2 changes:
> - New patch added in V2
> ---
> .../arm64/boot/dts/freescale/imx8-ss-dma.dtsi | 24 ++++++++++++-------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
> index 960a802b8b6e..b5ed12a06538 100644
> --- a/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
> @@ -111,8 +111,10 @@ uart3_lpcg: clock-controller@5a490000 {
> i2c0: i2c@5a800000 {
> reg = <0x5a800000 0x4000>;
> interrupts = <GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>;
> - clocks = <&i2c0_lpcg IMX_LPCG_CLK_0>;
> - clock-names = "per";
> + interrupt-parent = <&gic>;
> + clocks = <&i2c0_lpcg IMX_LPCG_CLK_0>,
> + <&i2c0_lpcg IMX_LPCG_CLK_4>;
> + clock-names = "per", "ipg";
> assigned-clocks = <&clk IMX_SC_R_I2C_0 IMX_SC_PM_CLK_PER>;
> assigned-clock-rates = <24000000>;
> power-domains = <&pd IMX_SC_R_I2C_0>;
> @@ -122,8 +124,10 @@ i2c0: i2c@5a800000 {
> i2c1: i2c@5a810000 {
> reg = <0x5a810000 0x4000>;
> interrupts = <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
> - clocks = <&i2c1_lpcg IMX_LPCG_CLK_0>;
> - clock-names = "per";
> + interrupt-parent = <&gic>;
> + clocks = <&i2c1_lpcg IMX_LPCG_CLK_0>,
> + <&i2c1_lpcg IMX_LPCG_CLK_4>;
> + clock-names = "per", "ipg";
> assigned-clocks = <&clk IMX_SC_R_I2C_1 IMX_SC_PM_CLK_PER>;
> assigned-clock-rates = <24000000>;
> power-domains = <&pd IMX_SC_R_I2C_1>;
> @@ -133,8 +137,10 @@ i2c1: i2c@5a810000 {
> i2c2: i2c@5a820000 {
> reg = <0x5a820000 0x4000>;
> interrupts = <GIC_SPI 222 IRQ_TYPE_LEVEL_HIGH>;
> - clocks = <&i2c2_lpcg IMX_LPCG_CLK_0>;
> - clock-names = "per";
> + interrupt-parent = <&gic>;
> + clocks = <&i2c2_lpcg IMX_LPCG_CLK_0>,
> + <&i2c2_lpcg IMX_LPCG_CLK_4>;
> + clock-names = "per", "ipg";
> assigned-clocks = <&clk IMX_SC_R_I2C_2 IMX_SC_PM_CLK_PER>;
> assigned-clock-rates = <24000000>;
> power-domains = <&pd IMX_SC_R_I2C_2>;
> @@ -144,8 +150,10 @@ i2c2: i2c@5a820000 {
> i2c3: i2c@5a830000 {
> reg = <0x5a830000 0x4000>;
> interrupts = <GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>;
> - clocks = <&i2c3_lpcg IMX_LPCG_CLK_0>;
> - clock-names = "per";
> + interrupt-parent = <&gic>;
> + clocks = <&i2c3_lpcg IMX_LPCG_CLK_0>,
> + <&i2c3_lpcg IMX_LPCG_CLK_4>;
> + clock-names = "per", "ipg";
> assigned-clocks = <&clk IMX_SC_R_I2C_3 IMX_SC_PM_CLK_PER>;
> assigned-clock-rates = <24000000>;
> power-domains = <&pd IMX_SC_R_I2C_3>;
> --
> 2.25.1
>

2021-05-21 09:55:14

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH V2 06/18] ARM64: dts: imx8: change i2c irq number to non-combined

> From: Clark Wang <[email protected]>
> Sent: Tuesday, April 6, 2021 7:33 PM
>
> Combined interrupt number may cause unexcepted irq event when using DMA
> and too many interrupts will be generated.
> So change all i2c interrupts number to non-combined for imx8qxp/8qm/8dxl.

Still no mx8dxl support in upstream, I guess imx8 is enough.
BTW, pls changing tile format as below as pointed by Shawn in another patch:
arm64: dts: xxxx

Otherwise:
Reviewed-by: Dong Aisheng <[email protected]>

Regards
Aisheng

>
> Signed-off-by: Clark Wang <[email protected]>
> ---
> V2 changes:
> - New patch added in V2
> ---
> arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
> b/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
> index b5ed12a06538..9ba57f04859b 100644
> --- a/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
> @@ -110,7 +110,7 @@ uart3_lpcg: clock-controller@5a490000 {
>
> i2c0: i2c@5a800000 {
> reg = <0x5a800000 0x4000>;
> - interrupts = <GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>;
> + interrupts = <GIC_SPI 340 IRQ_TYPE_LEVEL_HIGH>;
> interrupt-parent = <&gic>;
> clocks = <&i2c0_lpcg IMX_LPCG_CLK_0>,
> <&i2c0_lpcg IMX_LPCG_CLK_4>;
> @@ -123,7 +123,7 @@ i2c0: i2c@5a800000 {
>
> i2c1: i2c@5a810000 {
> reg = <0x5a810000 0x4000>;
> - interrupts = <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
> + interrupts = <GIC_SPI 341 IRQ_TYPE_LEVEL_HIGH>;
> interrupt-parent = <&gic>;
> clocks = <&i2c1_lpcg IMX_LPCG_CLK_0>,
> <&i2c1_lpcg IMX_LPCG_CLK_4>;
> @@ -136,7 +136,7 @@ i2c1: i2c@5a810000 {
>
> i2c2: i2c@5a820000 {
> reg = <0x5a820000 0x4000>;
> - interrupts = <GIC_SPI 222 IRQ_TYPE_LEVEL_HIGH>;
> + interrupts = <GIC_SPI 342 IRQ_TYPE_LEVEL_HIGH>;
> interrupt-parent = <&gic>;
> clocks = <&i2c2_lpcg IMX_LPCG_CLK_0>,
> <&i2c2_lpcg IMX_LPCG_CLK_4>;
> @@ -149,7 +149,7 @@ i2c2: i2c@5a820000 {
>
> i2c3: i2c@5a830000 {
> reg = <0x5a830000 0x4000>;
> - interrupts = <GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>;
> + interrupts = <GIC_SPI 343 IRQ_TYPE_LEVEL_HIGH>;
> interrupt-parent = <&gic>;
> clocks = <&i2c3_lpcg IMX_LPCG_CLK_0>,
> <&i2c3_lpcg IMX_LPCG_CLK_4>;
> --
> 2.25.1

2021-05-21 18:21:24

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH V2 05/18] ARM64: dts: imx8: add the missing lpi2c ipg clock

> From: Clark Wang <[email protected]>
> Sent: Tuesday, April 6, 2021 7:33 PM
>
> The lpi2c driver has add the missing ipg clock.
> So add the ipg clock here for all lpi2c nodes.
>
> Signed-off-by: Clark Wang <[email protected]>
> ---
> V2 changes:
> - New patch added in V2
> ---
> .../arm64/boot/dts/freescale/imx8-ss-dma.dtsi | 24 ++++++++++++-------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
> b/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
> index 960a802b8b6e..b5ed12a06538 100644
> --- a/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
> @@ -111,8 +111,10 @@ uart3_lpcg: clock-controller@5a490000 {
> i2c0: i2c@5a800000 {
> reg = <0x5a800000 0x4000>;
> interrupts = <GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>;
> - clocks = <&i2c0_lpcg IMX_LPCG_CLK_0>;
> - clock-names = "per";
> + interrupt-parent = <&gic>;

Added by mistake?

> + clocks = <&i2c0_lpcg IMX_LPCG_CLK_0>,
> + <&i2c0_lpcg IMX_LPCG_CLK_4>;
> + clock-names = "per", "ipg";
> assigned-clocks = <&clk IMX_SC_R_I2C_0 IMX_SC_PM_CLK_PER>;
> assigned-clock-rates = <24000000>;
> power-domains = <&pd IMX_SC_R_I2C_0>; @@ -122,8 +124,10 @@
> i2c0: i2c@5a800000 {
> i2c1: i2c@5a810000 {
> reg = <0x5a810000 0x4000>;
> interrupts = <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
> - clocks = <&i2c1_lpcg IMX_LPCG_CLK_0>;
> - clock-names = "per";
> + interrupt-parent = <&gic>;

Ditto

> + clocks = <&i2c1_lpcg IMX_LPCG_CLK_0>,
> + <&i2c1_lpcg IMX_LPCG_CLK_4>;
> + clock-names = "per", "ipg";
> assigned-clocks = <&clk IMX_SC_R_I2C_1 IMX_SC_PM_CLK_PER>;
> assigned-clock-rates = <24000000>;
> power-domains = <&pd IMX_SC_R_I2C_1>; @@ -133,8 +137,10 @@
> i2c1: i2c@5a810000 {
> i2c2: i2c@5a820000 {
> reg = <0x5a820000 0x4000>;
> interrupts = <GIC_SPI 222 IRQ_TYPE_LEVEL_HIGH>;
> - clocks = <&i2c2_lpcg IMX_LPCG_CLK_0>;
> - clock-names = "per";
> + interrupt-parent = <&gic>;
> + clocks = <&i2c2_lpcg IMX_LPCG_CLK_0>,
> + <&i2c2_lpcg IMX_LPCG_CLK_4>;
> + clock-names = "per", "ipg";
> assigned-clocks = <&clk IMX_SC_R_I2C_2 IMX_SC_PM_CLK_PER>;
> assigned-clock-rates = <24000000>;
> power-domains = <&pd IMX_SC_R_I2C_2>; @@ -144,8 +150,10 @@
> i2c2: i2c@5a820000 {
> i2c3: i2c@5a830000 {
> reg = <0x5a830000 0x4000>;
> interrupts = <GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>;
> - clocks = <&i2c3_lpcg IMX_LPCG_CLK_0>;
> - clock-names = "per";
> + interrupt-parent = <&gic>;
> + clocks = <&i2c3_lpcg IMX_LPCG_CLK_0>,
> + <&i2c3_lpcg IMX_LPCG_CLK_4>;
> + clock-names = "per", "ipg";
> assigned-clocks = <&clk IMX_SC_R_I2C_3 IMX_SC_PM_CLK_PER>;
> assigned-clock-rates = <24000000>;
> power-domains = <&pd IMX_SC_R_I2C_3>;
> --
> 2.25.1

2021-05-21 20:08:13

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH V2 08/18] i2c: imx-lpi2c: add debug message when i2c peripheral clk doesn't work

> From: Clark Wang <[email protected]>
> Sent: Tuesday, April 6, 2021 7:33 PM
>
> add debug message when i2c peripheral clk rate is 0, then directly return
> -EINVAL.
>
> Signed-off-by: Clark Wang <[email protected]>

You sign-off should be put after the original patch author

> Signed-off-by: Gao Pan <[email protected]>
> Reviewed-by: Andy Duan <[email protected]>

Drop the original review when they're significant changes

> ---
> V2 changes:
> - Add my signed-off.
> ---
> drivers/i2c/busses/i2c-imx-lpi2c.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c
> b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index 333209ba81c1..dfec334712c2 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -210,6 +210,11 @@ static int lpi2c_imx_config(struct lpi2c_imx_struct
> *lpi2c_imx)
> lpi2c_imx_set_mode(lpi2c_imx);
>
> clk_rate = clk_get_rate(lpi2c_imx->clk_per);
> + if (!clk_rate) {
> + dev_dbg(&lpi2c_imx->adapter.dev, "clk_per rate is 0\n");

s/dev_dbg/dev_err

> + return -EINVAL;
> + }
> +
> if (lpi2c_imx->mode == HS || lpi2c_imx->mode == ULTRA_FAST)
> filt = 0;
> else
> --
> 2.25.1