| Message ID | 20250319145114.50771-1-francesco@dolcini.it |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] i2c: lpi2c: implement master_xfer_atomic callback | expand |
Hello, On Wed, Mar 19, 2025 at 03:51:14PM +0100, Francesco Dolcini wrote: > From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> > > Rework the read and write code paths in the driver to support operation > in atomic contexts. To achieve this, the driver must not rely on IRQs > or perform any scheduling, e.g., via a sleep or schedule routine. Even > jiffies do not advance in atomic contexts, so timeouts based on them > are substituted with delays. > > Implement atomic, sleep-free, and IRQ-less operation. This increases > complexity but is necessary for atomic I2C transfers required by some > hardware configurations, e.g., to trigger reboots on an external PMIC chip. > > Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> Any comment on this? Francesco
> -----Original Message----- > From: Francesco Dolcini <francesco@dolcini.it> > Sent: Friday, April 11, 2025 7:48 PM > To: Aisheng Dong <aisheng.dong@nxp.com>; Andi Shyti > <andi.shyti@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > <s.hauer@pengutronix.de> > Cc: Francesco Dolcini <francesco@dolcini.it>; Pengutronix Kernel Team > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Emanuele > Ghidoli <emanuele.ghidoli@toradex.com>; linux-i2c@vger.kernel.org; > imx@lists.linux.dev; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; Francesco Dolcini > <francesco.dolcini@toradex.com> > Subject: [EXT] Re: [PATCH v1] i2c: lpi2c: implement master_xfer_atomic callback > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report this > email' button > > > Hello, > > On Wed, Mar 19, 2025 at 03:51:14PM +0100, Francesco Dolcini wrote: > > From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> > > > > Rework the read and write code paths in the driver to support > > operation in atomic contexts. To achieve this, the driver must not > > rely on IRQs or perform any scheduling, e.g., via a sleep or schedule > > routine. Even jiffies do not advance in atomic contexts, so timeouts > > based on them are substituted with delays. > > > > Implement atomic, sleep-free, and IRQ-less operation. This increases > > complexity but is necessary for atomic I2C transfers required by some > > hardware configurations, e.g., to trigger reboots on an external PMIC chip. > > > > Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> > > Any comment on this? > > Francesco Hi, Looks good. Thank you for your work! Do you test it at some board? How can we test simply? BR Carlos
Hello, On Fri, Apr 11, 2025 at 11:55:31AM +0000, Carlos Song wrote: > > On Wed, Mar 19, 2025 at 03:51:14PM +0100, Francesco Dolcini wrote: > > > From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> > > > > > > Rework the read and write code paths in the driver to support > > > operation in atomic contexts. To achieve this, the driver must not > > > rely on IRQs or perform any scheduling, e.g., via a sleep or schedule > > > routine. Even jiffies do not advance in atomic contexts, so timeouts > > > based on them are substituted with delays. > > > > > > Implement atomic, sleep-free, and IRQ-less operation. This increases > > > complexity but is necessary for atomic I2C transfers required by some > > > hardware configurations, e.g., to trigger reboots on an external PMIC chip. > > > > > > Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> > > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> > > > > Any comment on this? > Looks good. Thank you for your work! > Do you test it at some board? How can we test simply? It was tested on Toradex SMARC iMX95, there to power-off/reset the board we have some I2C communication required [1]. [1] https://lore.kernel.org/all/20250407114947.41421-3-francesco@dolcini.it/ Francesco
On 11/04/2025 13:55, Carlos Song wrote: >> On Wed, Mar 19, 2025 at 03:51:14PM +0100, Francesco Dolcini wrote: >>> From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> >>> >>> Rework the read and write code paths in the driver to support >>> operation in atomic contexts. To achieve this, the driver must not >>> rely on IRQs or perform any scheduling, e.g., via a sleep or schedule >>> routine. Even jiffies do not advance in atomic contexts, so timeouts >>> based on them are substituted with delays. >>> >>> Implement atomic, sleep-free, and IRQ-less operation. This increases >>> complexity but is necessary for atomic I2C transfers required by some >>> hardware configurations, e.g., to trigger reboots on an external PMIC chip. >>> >>> Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> >>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> >> >> Any comment on this? > Looks good. Thank you for your work! > Do you test it at some board? How can we test simply? We tested it also by using the xfer_atomic for "normal" transfers (.master_xfer = lpi2c_imx_xfer_atomic). The driver is used to drive multiple buses with different devices. Emanuele
Hi Francesco, I'm sorry for the late reply on this. Can someone from NXP help with the review? Carlos? Dong? On Wed, Mar 19, 2025 at 03:51:14PM +0100, Francesco Dolcini wrote: > From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> > > Rework the read and write code paths in the driver to support operation > in atomic contexts. To achieve this, the driver must not rely on IRQs > or perform any scheduling, e.g., via a sleep or schedule routine. Even > jiffies do not advance in atomic contexts, so timeouts based on them > are substituted with delays. > > Implement atomic, sleep-free, and IRQ-less operation. This increases > complexity but is necessary for atomic I2C transfers required by some > hardware configurations, e.g., to trigger reboots on an external PMIC chip. > > Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> > --- > drivers/i2c/busses/i2c-imx-lpi2c.c | 258 +++++++++++++++++++---------- > 1 file changed, 173 insertions(+), 85 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c > index 0d4b3935e687..f34b6f07e9a4 100644 > --- a/drivers/i2c/busses/i2c-imx-lpi2c.c > +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c > @@ -16,6 +16,7 @@ > #include <linux/init.h> > #include <linux/interrupt.h> > #include <linux/io.h> > +#include <linux/iopoll.h> > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/of.h> > @@ -187,36 +188,35 @@ struct lpi2c_imx_struct { > struct i2c_client *target; > }; > > +#define READL_POLL_TIMEOUT(atomic, addr, val, cond, delay_us, timeout_us) \ READ_POLL_TIMEOUT is not really a name that belongs to this driver. Could we name it something like lpi2c_imx_read_poll_timeout()? I'd prefer lowercase, but I won't object to capital letters. Additionally, the timeout_us value is always 500000, could we just drop it from the parameter list? Same goes for LPI2C_MSR. Thanks, Andi
> -----Original Message----- > From: Andi Shyti <andi.shyti@kernel.org> > Sent: Tuesday, May 6, 2025 7:02 AM > To: Francesco Dolcini <francesco@dolcini.it> > Cc: Aisheng Dong <aisheng.dong@nxp.com>; Shawn Guo > <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>; > Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam > <festevam@gmail.com>; Emanuele Ghidoli <emanuele.ghidoli@toradex.com>; > linux-i2c@vger.kernel.org; imx@lists.linux.dev; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Francesco > Dolcini <francesco.dolcini@toradex.com> > Subject: [EXT] Re: [PATCH v1] i2c: lpi2c: implement master_xfer_atomic callback > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report this > email' button > > > Hi Francesco, > > I'm sorry for the late reply on this. > > Can someone from NXP help with the review? Carlos? Dong? > Hi, Yes. I reviewed this. It is ok for me! Thank you. Reviewed-by: Carlos Song <carlos.song@nxp.com> > On Wed, Mar 19, 2025 at 03:51:14PM +0100, Francesco Dolcini wrote: > > From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> > > > > Rework the read and write code paths in the driver to support > > operation in atomic contexts. To achieve this, the driver must not > > rely on IRQs or perform any scheduling, e.g., via a sleep or schedule > > routine. Even jiffies do not advance in atomic contexts, so timeouts > > based on them are substituted with delays. > > > > Implement atomic, sleep-free, and IRQ-less operation. This increases > > complexity but is necessary for atomic I2C transfers required by some > > hardware configurations, e.g., to trigger reboots on an external PMIC chip. > > > > Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> > > --- > > drivers/i2c/busses/i2c-imx-lpi2c.c | 258 > > +++++++++++++++++++---------- > > 1 file changed, 173 insertions(+), 85 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c > > b/drivers/i2c/busses/i2c-imx-lpi2c.c > > index 0d4b3935e687..f34b6f07e9a4 100644 > > --- a/drivers/i2c/busses/i2c-imx-lpi2c.c > > +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c > > @@ -16,6 +16,7 @@ > > #include <linux/init.h> > > #include <linux/interrupt.h> > > #include <linux/io.h> > > +#include <linux/iopoll.h> > > #include <linux/kernel.h> > > #include <linux/module.h> > > #include <linux/of.h> > > @@ -187,36 +188,35 @@ struct lpi2c_imx_struct { > > struct i2c_client *target; > > }; > > > > +#define READL_POLL_TIMEOUT(atomic, addr, val, cond, delay_us, > > +timeout_us) \ > > READ_POLL_TIMEOUT is not really a name that belongs to this driver. Could we > name it something like lpi2c_imx_read_poll_timeout()? I'd prefer lowercase, but > I won't object to capital letters. > > Additionally, the timeout_us value is always 500000, could we just drop it from > the parameter list? Same goes for LPI2C_MSR. > > Thanks, > Andi
Hi Francesco and Emanuele, On Wed, Mar 19, 2025 at 03:51:14PM +0100, Francesco Dolcini wrote: > From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> > > Rework the read and write code paths in the driver to support operation > in atomic contexts. To achieve this, the driver must not rely on IRQs > or perform any scheduling, e.g., via a sleep or schedule routine. Even > jiffies do not advance in atomic contexts, so timeouts based on them > are substituted with delays. > > Implement atomic, sleep-free, and IRQ-less operation. This increases > complexity but is necessary for atomic I2C transfers required by some > hardware configurations, e.g., to trigger reboots on an external PMIC chip. > > Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> this patch is causing a build regression. I'm going to revert it, please check the test report that has been reported and you are cc'ed. Andi
Il 14 maggio 2025 17:14:32 CEST, Andi Shyti <andi.shyti@kernel.org> ha scritto: >Hi Francesco and Emanuele, > >On Wed, Mar 19, 2025 at 03:51:14PM +0100, Francesco Dolcini wrote: >> From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> >> >> Rework the read and write code paths in the driver to support operation >> in atomic contexts. To achieve this, the driver must not rely on IRQs >> or perform any scheduling, e.g., via a sleep or schedule routine. Even >> jiffies do not advance in atomic contexts, so timeouts based on them >> are substituted with delays. >> >> Implement atomic, sleep-free, and IRQ-less operation. This increases >> complexity but is necessary for atomic I2C transfers required by some >> hardware configurations, e.g., to trigger reboots on an external PMIC chip. >> >> Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> >> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> > >this patch is causing a build regression. I'm going to revert it, >please check the test report that has been reported and you are >cc'ed. > >Andi I am looking at it, it's a warning with W=1, not a build error. I would not revert this patch, just wait for a follow up patch or comment that will address that warning. Francesco
Hi Francesco, On Wed, May 14, 2025 at 05:51:27PM +0200, Francesco Dolcini wrote: > Il 14 maggio 2025 17:14:32 CEST, Andi Shyti <andi.shyti@kernel.org> ha scritto: > >On Wed, Mar 19, 2025 at 03:51:14PM +0100, Francesco Dolcini wrote: > >> Rework the read and write code paths in the driver to support operation > >> in atomic contexts. To achieve this, the driver must not rely on IRQs > >> or perform any scheduling, e.g., via a sleep or schedule routine. Even > >> jiffies do not advance in atomic contexts, so timeouts based on them > >> are substituted with delays. > >> > >> Implement atomic, sleep-free, and IRQ-less operation. This increases > >> complexity but is necessary for atomic I2C transfers required by some > >> hardware configurations, e.g., to trigger reboots on an external PMIC chip. > >> > >> Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com> > >> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> > > > >this patch is causing a build regression. I'm going to revert it, > >please check the test report that has been reported and you are > >cc'ed. > > I am looking at it, it's a warning with W=1, not a build error. > I would not revert this patch, just wait for a follow up patch > or comment that will address that warning. please send a v2 already fixed I don't want to keep a regression even if it's a small warning. We still have time until the merge window and this patch is already reviewed by Carlos. Thanks, Andi
diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c index 0d4b3935e687..f34b6f07e9a4 100644 --- a/drivers/i2c/busses/i2c-imx-lpi2c.c +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c @@ -16,6 +16,7 @@ #include <linux/init.h> #include <linux/interrupt.h> #include <linux/io.h> +#include <linux/iopoll.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/of.h> @@ -187,36 +188,35 @@ struct lpi2c_imx_struct { struct i2c_client *target; }; +#define READL_POLL_TIMEOUT(atomic, addr, val, cond, delay_us, timeout_us) \ + (atomic ? readl_poll_timeout_atomic(addr, val, cond, delay_us, timeout_us) : \ + readl_poll_timeout(addr, val, cond, delay_us, timeout_us)) + 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) +static int lpi2c_imx_bus_busy(struct lpi2c_imx_struct *lpi2c_imx, bool atomic) { - unsigned long orig_jiffies = jiffies; unsigned int temp; - while (1) { - temp = readl(lpi2c_imx->base + LPI2C_MSR); + READL_POLL_TIMEOUT(atomic, lpi2c_imx->base + LPI2C_MSR, temp, + temp & (MSR_ALF | MSR_BBF | MSR_MBF), 1, 500000); - /* check for arbitration lost, clear if set */ - if (temp & MSR_ALF) { - writel(temp, lpi2c_imx->base + LPI2C_MSR); - return -EAGAIN; - } - - if (temp & (MSR_BBF | MSR_MBF)) - break; + /* check for arbitration lost, clear if set */ + if (temp & MSR_ALF) { + writel(temp, lpi2c_imx->base + LPI2C_MSR); + return -EAGAIN; + } - if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) { - dev_dbg(&lpi2c_imx->adapter.dev, "bus not work\n"); - if (lpi2c_imx->adapter.bus_recovery_info) - i2c_recover_bus(&lpi2c_imx->adapter); - return -ETIMEDOUT; - } - schedule(); + /* check for bus not busy */ + if (!(temp & (MSR_BBF | MSR_MBF))) { + dev_dbg(&lpi2c_imx->adapter.dev, "bus not work\n"); + if (lpi2c_imx->adapter.bus_recovery_info) + i2c_recover_bus(&lpi2c_imx->adapter); + return -ETIMEDOUT; } return 0; @@ -242,7 +242,7 @@ static void lpi2c_imx_set_mode(struct lpi2c_imx_struct *lpi2c_imx) } static int lpi2c_imx_start(struct lpi2c_imx_struct *lpi2c_imx, - struct i2c_msg *msgs) + struct i2c_msg *msgs, bool atomic) { unsigned int temp; @@ -254,30 +254,23 @@ static int lpi2c_imx_start(struct lpi2c_imx_struct *lpi2c_imx, temp = i2c_8bit_addr_from_msg(msgs) | (GEN_START << 8); writel(temp, lpi2c_imx->base + LPI2C_MTDR); - return lpi2c_imx_bus_busy(lpi2c_imx); + return lpi2c_imx_bus_busy(lpi2c_imx, atomic); } -static void lpi2c_imx_stop(struct lpi2c_imx_struct *lpi2c_imx) +static void lpi2c_imx_stop(struct lpi2c_imx_struct *lpi2c_imx, bool atomic) { - unsigned long orig_jiffies = jiffies; unsigned int temp; 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"); - if (lpi2c_imx->adapter.bus_recovery_info) - i2c_recover_bus(&lpi2c_imx->adapter); - break; - } - schedule(); + READL_POLL_TIMEOUT(atomic, lpi2c_imx->base + LPI2C_MSR, temp, + temp & MSR_SDF, 1, 500000); - } while (1); + if (!(temp & MSR_SDF)) { + dev_dbg(&lpi2c_imx->adapter.dev, "stop timeout\n"); + if (lpi2c_imx->adapter.bus_recovery_info) + i2c_recover_bus(&lpi2c_imx->adapter); + } } /* CLKLO = I2C_CLK_RATIO * CLKHI, SETHOLD = CLKHI, DATAVD = CLKHI/2 */ @@ -391,28 +384,31 @@ static int lpi2c_imx_pio_msg_complete(struct lpi2c_imx_struct *lpi2c_imx) return time_left ? 0 : -ETIMEDOUT; } -static int lpi2c_imx_txfifo_empty(struct lpi2c_imx_struct *lpi2c_imx) +static u32 txfifo_cnt(struct lpi2c_imx_struct *lpi2c_imx) { - unsigned long orig_jiffies = jiffies; - u32 txcnt; + return readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff; +} - do { - txcnt = readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff; +static int lpi2c_imx_txfifo_empty(struct lpi2c_imx_struct *lpi2c_imx, bool atomic) +{ + unsigned int temp; + int err; - if (readl(lpi2c_imx->base + LPI2C_MSR) & MSR_NDF) { - dev_dbg(&lpi2c_imx->adapter.dev, "NDF detected\n"); - return -EIO; - } + err = READL_POLL_TIMEOUT(atomic, lpi2c_imx->base + LPI2C_MSR, temp, + (temp & MSR_NDF) || !txfifo_cnt(lpi2c_imx), + 1, 500000); - if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) { - 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(); + if (temp & MSR_NDF) { + dev_dbg(&lpi2c_imx->adapter.dev, "NDF detected\n"); + return -EIO; + } - } while (txcnt); + if (err) { + 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; + } return 0; } @@ -436,7 +432,7 @@ static void lpi2c_imx_set_rx_watermark(struct lpi2c_imx_struct *lpi2c_imx) writel(temp << 16, lpi2c_imx->base + LPI2C_MFCR); } -static void lpi2c_imx_write_txfifo(struct lpi2c_imx_struct *lpi2c_imx) +static bool lpi2c_imx_write_txfifo(struct lpi2c_imx_struct *lpi2c_imx, bool atomic) { unsigned int data, txcnt; @@ -451,13 +447,19 @@ static void lpi2c_imx_write_txfifo(struct lpi2c_imx_struct *lpi2c_imx) txcnt++; } - if (lpi2c_imx->delivered < lpi2c_imx->msglen) - lpi2c_imx_intctrl(lpi2c_imx, MIER_TDIE | MIER_NDIE); - else + if (lpi2c_imx->delivered < lpi2c_imx->msglen) { + if (!atomic) + lpi2c_imx_intctrl(lpi2c_imx, MIER_TDIE | MIER_NDIE); + return false; + } + + if (!atomic) complete(&lpi2c_imx->complete); + + return true; } -static void lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx) +static bool lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx, bool atomic) { unsigned int blocklen, remaining; unsigned int temp, data; @@ -482,8 +484,9 @@ static void lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx) remaining = lpi2c_imx->msglen - lpi2c_imx->delivered; if (!remaining) { - complete(&lpi2c_imx->complete); - return; + if (!atomic) + complete(&lpi2c_imx->complete); + return true; } /* not finished, still waiting for rx data */ @@ -501,7 +504,10 @@ static void lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx) writel(temp, lpi2c_imx->base + LPI2C_MTDR); } - lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE); + if (!atomic) + lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE); + + return false; } static void lpi2c_imx_write(struct lpi2c_imx_struct *lpi2c_imx, @@ -509,11 +515,29 @@ static void lpi2c_imx_write(struct lpi2c_imx_struct *lpi2c_imx, { lpi2c_imx->tx_buf = msgs->buf; lpi2c_imx_set_tx_watermark(lpi2c_imx); - lpi2c_imx_write_txfifo(lpi2c_imx); + lpi2c_imx_write_txfifo(lpi2c_imx, false); } -static void lpi2c_imx_read(struct lpi2c_imx_struct *lpi2c_imx, - struct i2c_msg *msgs) +static int lpi2c_imx_write_atomic(struct lpi2c_imx_struct *lpi2c_imx, + struct i2c_msg *msgs) +{ + u32 temp; + int err; + + lpi2c_imx->tx_buf = msgs->buf; + + err = READL_POLL_TIMEOUT(true, lpi2c_imx->base + LPI2C_MSR, temp, + (temp & MSR_NDF) || lpi2c_imx_write_txfifo(lpi2c_imx, true), + 5, 1000000); + + if (temp & MSR_NDF) + return -EIO; + + return err; +} + +static void lpi2c_imx_read_init(struct lpi2c_imx_struct *lpi2c_imx, + struct i2c_msg *msgs) { unsigned int temp; @@ -524,8 +548,43 @@ static void lpi2c_imx_read(struct lpi2c_imx_struct *lpi2c_imx, temp = msgs->len > CHUNK_DATA ? CHUNK_DATA - 1 : msgs->len - 1; temp |= (RECV_DATA << 8); writel(temp, lpi2c_imx->base + LPI2C_MTDR); +} + +static bool lpi2c_imx_read_chunk_atomic(struct lpi2c_imx_struct *lpi2c_imx) +{ + u32 rxcnt; + + rxcnt = (readl(lpi2c_imx->base + LPI2C_MFSR) >> 16) & 0xFF; + if (!rxcnt) + return false; + + if (!lpi2c_imx_read_rxfifo(lpi2c_imx, true)) + return false; - lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE | MIER_NDIE); + return true; +} + +static int lpi2c_imx_read_atomic(struct lpi2c_imx_struct *lpi2c_imx, + struct i2c_msg *msgs) +{ + u32 temp; + int tmo_us; + + tmo_us = 1000000; + do { + if (lpi2c_imx_read_chunk_atomic(lpi2c_imx)) + return 0; + + temp = readl(lpi2c_imx->base + LPI2C_MSR); + + if (temp & MSR_NDF) + return -EIO; + + udelay(100); + tmo_us -= 100; + } while (tmo_us > 0); + + return -ETIMEDOUT; } static bool is_use_dma(struct lpi2c_imx_struct *lpi2c_imx, struct i2c_msg *msg) @@ -545,14 +604,27 @@ static int lpi2c_imx_pio_xfer(struct lpi2c_imx_struct *lpi2c_imx, { reinit_completion(&lpi2c_imx->complete); - if (msg->flags & I2C_M_RD) - lpi2c_imx_read(lpi2c_imx, msg); - else + if (msg->flags & I2C_M_RD) { + lpi2c_imx_read_init(lpi2c_imx, msg); + lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE | MIER_NDIE); + } else { lpi2c_imx_write(lpi2c_imx, msg); + } return lpi2c_imx_pio_msg_complete(lpi2c_imx); } +static int lpi2c_imx_pio_xfer_atomic(struct lpi2c_imx_struct *lpi2c_imx, + struct i2c_msg *msg) +{ + if (msg->flags & I2C_M_RD) { + lpi2c_imx_read_init(lpi2c_imx, msg); + return lpi2c_imx_read_atomic(lpi2c_imx, msg); + } + + return lpi2c_imx_write_atomic(lpi2c_imx, msg); +} + static int lpi2c_imx_dma_timeout_calculate(struct lpi2c_imx_struct *lpi2c_imx) { unsigned long time = 0; @@ -947,8 +1019,8 @@ static int lpi2c_imx_dma_xfer(struct lpi2c_imx_struct *lpi2c_imx, return ret; } -static int lpi2c_imx_xfer(struct i2c_adapter *adapter, - struct i2c_msg *msgs, int num) +static int lpi2c_imx_xfer_common(struct i2c_adapter *adapter, + struct i2c_msg *msgs, int num, bool atomic) { struct lpi2c_imx_struct *lpi2c_imx = i2c_get_adapdata(adapter); unsigned int temp; @@ -959,7 +1031,7 @@ static int lpi2c_imx_xfer(struct i2c_adapter *adapter, return result; for (i = 0; i < num; i++) { - result = lpi2c_imx_start(lpi2c_imx, &msgs[i]); + result = lpi2c_imx_start(lpi2c_imx, &msgs[i], atomic); if (result) goto disable; @@ -971,28 +1043,33 @@ static int lpi2c_imx_xfer(struct i2c_adapter *adapter, lpi2c_imx->tx_buf = NULL; lpi2c_imx->delivered = 0; lpi2c_imx->msglen = msgs[i].len; - init_completion(&lpi2c_imx->complete); - if (is_use_dma(lpi2c_imx, &msgs[i])) { - result = lpi2c_imx_dma_xfer(lpi2c_imx, &msgs[i]); - if (result && lpi2c_imx->dma->using_pio_mode) - result = lpi2c_imx_pio_xfer(lpi2c_imx, &msgs[i]); + if (atomic) { + result = lpi2c_imx_pio_xfer_atomic(lpi2c_imx, &msgs[i]); } else { - result = lpi2c_imx_pio_xfer(lpi2c_imx, &msgs[i]); + init_completion(&lpi2c_imx->complete); + + if (is_use_dma(lpi2c_imx, &msgs[i])) { + result = lpi2c_imx_dma_xfer(lpi2c_imx, &msgs[i]); + if (result && lpi2c_imx->dma->using_pio_mode) + result = lpi2c_imx_pio_xfer(lpi2c_imx, &msgs[i]); + } else { + result = lpi2c_imx_pio_xfer(lpi2c_imx, &msgs[i]); + } } if (result) goto stop; if (!(msgs[i].flags & I2C_M_RD)) { - result = lpi2c_imx_txfifo_empty(lpi2c_imx); + result = lpi2c_imx_txfifo_empty(lpi2c_imx, atomic); if (result) goto stop; } } stop: - lpi2c_imx_stop(lpi2c_imx); + lpi2c_imx_stop(lpi2c_imx, atomic); temp = readl(lpi2c_imx->base + LPI2C_MSR); if ((temp & MSR_NDF) && !result) @@ -1008,6 +1085,16 @@ static int lpi2c_imx_xfer(struct i2c_adapter *adapter, return (result < 0) ? result : num; } +static int lpi2c_imx_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) +{ + return lpi2c_imx_xfer_common(adapter, msgs, num, false); +} + +static int lpi2c_imx_xfer_atomic(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) +{ + return lpi2c_imx_xfer_common(adapter, msgs, num, true); +} + static irqreturn_t lpi2c_imx_target_isr(struct lpi2c_imx_struct *lpi2c_imx, u32 ssr, u32 sier_filter) { @@ -1070,9 +1157,9 @@ static irqreturn_t lpi2c_imx_master_isr(struct lpi2c_imx_struct *lpi2c_imx) if (temp & MSR_NDF) complete(&lpi2c_imx->complete); else if (temp & MSR_RDF) - lpi2c_imx_read_rxfifo(lpi2c_imx); + lpi2c_imx_read_rxfifo(lpi2c_imx, false); else if (temp & MSR_TDF) - lpi2c_imx_write_txfifo(lpi2c_imx); + lpi2c_imx_write_txfifo(lpi2c_imx, false); return IRQ_HANDLED; } @@ -1268,10 +1355,11 @@ static u32 lpi2c_imx_func(struct i2c_adapter *adapter) } static const struct i2c_algorithm lpi2c_imx_algo = { - .master_xfer = lpi2c_imx_xfer, - .functionality = lpi2c_imx_func, - .reg_target = lpi2c_imx_register_target, - .unreg_target = lpi2c_imx_unregister_target, + .master_xfer = lpi2c_imx_xfer, + .master_xfer_atomic = lpi2c_imx_xfer_atomic, + .functionality = lpi2c_imx_func, + .reg_target = lpi2c_imx_register_target, + .unreg_target = lpi2c_imx_unregister_target, }; static const struct of_device_id lpi2c_imx_of_match[] = {