Re: [PATCH 3/3] i2c: imx_lpi2c: Support read transfers longer than 256 bytes

[Fedor Ross]

On Wed, Jul 24, 2024 at 12:57 PM Marek Vasut <ma...@denx.de> wrote:

Hello Marek,

I'll do the changes. Thanks for your input.

Best regards,
Fedor

> On 7/24/24 11:59 AM, fedorr...@gmail.com wrote:
> > From: Fedor Ross <fedor.r...@ifm.com>
> >
> > The TXFIFO register of LPI2C only has one byte length, and if the length
> > of the data that needs to be read exceeds 256 bytes, it needs to be
> > written to TXFIFO multiple times.
> >
> > Signed-off-by: Fedor Ross <fedor.r...@ifm.com>
>
> Add ---
>
> > Cc: Heiko Schocher <h...@denx.de>
> > Cc: Tom Rini <tr...@konsulko.com>
> > Cc: Marek Vasut <ma...@denx.de>
> > ---
> >   drivers/i2c/imx_lpi2c.c | 72 +++++++++++++++++++++++++++--------------
> >   1 file changed, 47 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/i2c/imx_lpi2c.c b/drivers/i2c/imx_lpi2c.c
> > index 7b10b2b252d..dd97228ddc7 100644
> > --- a/drivers/i2c/imx_lpi2c.c
> > +++ b/drivers/i2c/imx_lpi2c.c
> > @@ -19,6 +19,8 @@
> >   #define LPI2C_NACK_TOUT_MS 1
> >   #define LPI2C_TIMEOUT_MS 100
> >
> > +#define CHUNK_DATA      256
>
> Use LPI2C_CHUNK_DATA to namespace this macro.
>
> >   static int bus_i2c_init(struct udevice *bus, int speed);
> >
> >   /* Weak linked function for overridden by some SoC power function */
> > @@ -118,8 +120,10 @@ static int bus_i2c_send(struct udevice *bus, u8 
> > *txbuf, int len)
> >
> >   static int bus_i2c_receive(struct udevice *bus, u8 *rxbuf, int len)
> >   {
> > +     struct dm_i2c_bus *i2c = dev_get_uclass_priv(bus);
> >       struct imx_lpi2c_bus *i2c_bus = dev_get_priv(bus);
> >       struct imx_lpi2c_reg *regs = (struct imx_lpi2c_reg *)(i2c_bus->base);
> > +     unsigned int chunk_len, rx_remain, timeout;
> >       lpi2c_status_t result = LPI2C_SUCESS;
> >       u32 val;
> >       ulong start_time = get_timer(0);
> > @@ -128,33 +132,51 @@ static int bus_i2c_receive(struct udevice *bus, u8 
> > *rxbuf, int len)
> >       if (!len)
> >               return result;
> >
> > -     result = bus_i2c_wait_for_tx_ready(regs);
> > -     if (result) {
> > -             debug("i2c: receive wait for tx ready: %d\n", result);
> > -             return result;
> > -     }
> > +     /*
> > +      * Extend the timeout for a bulk read if needed.
> > +      * The calculated timeout is the result of multiplying the
> > +      * transfer length with 8 bit + ACK + one clock of extra time,
> > +      * considering the I2C bus frequency.
> > +      */
> > +     timeout = max(len * 10 * 1000 / i2c->speed_hz, LPI2C_TIMEOUT_MS);
> >
> > -     /* clear all status flags */
> > -     writel(0x7f00, &regs->msr);
> > -     /* send receive command */
> > -     val = LPI2C_MTDR_CMD(0x1) | LPI2C_MTDR_DATA(len - 1);
> > -     writel(val, &regs->mtdr);
> > +     rx_remain = len;
> > +     while (rx_remain > 0) {
> > +             chunk_len = clamp(rx_remain, 1, CHUNK_DATA) - 1;
> >
> > -     while (len--) {
> > -             do {
> > -                     result = imx_lpci2c_check_clear_error(regs);
> > -                     if (result) {
> > -                             debug("i2c: receive check clear error: %d\n",
> > -                                   result);
> > -                             return result;
> > -                     }
> > -                     if (get_timer(start_time) > LPI2C_TIMEOUT_MS) {
> > -                             debug("i2c: receive mrdr: timeout\n");
> > -                             return -1;
> > -                     }
> > -                     val = readl(&regs->mrdr);
> > -             } while (val & LPI2C_MRDR_RXEMPTY_MASK);
> > -             *rxbuf++ = LPI2C_MRDR_DATA(val);
> > +             result = bus_i2c_wait_for_tx_ready(regs);
> > +             if (result) {
> > +                     debug("i2c: receive wait for tx ready: %d\n", result);
> > +                     return result;
> > +             }
> > +
> > +             /* clear all status flags */
> > +             writel(0x7f00, &regs->msr);
> > +             /* send receive command */
> > +             val = LPI2C_MTDR_CMD(0x1) | LPI2C_MTDR_DATA(chunk_len);
> > +             writel(val, &regs->mtdr);
>
> writel(LPI2C_MTDR_CMD(0x1) | LPI2C_MTDR_DATA(chunk_len);, &regs->mtdr);
>
> and drop the val = assignment here.
>
> > +             rx_remain = rx_remain - (chunk_len & 0xff) - 1;
> > +
> > +             while (len--) {
> > +                     do {
> > +                             result = imx_lpci2c_check_clear_error(regs);
> > +                             if (result) {
> > +                                     debug("i2c: receive check clear 
> > error: %d\n",
> > +                                           result);
> > +                                     return result;
> > +                             }
> > +                             if (get_timer(start_time) > timeout) {
> > +                                     debug("i2c: receive mrdr: timeout\n");
> > +                                     return -1;
> > +                             }
> > +                             val = readl(&regs->mrdr);
> > +                     } while (val & LPI2C_MRDR_RXEMPTY_MASK);
> > +                     *rxbuf++ = LPI2C_MRDR_DATA(val);
> > +
> > +                     /* send next receive command before controller NACKs 
> > last byte */
> > +                     if ((len - rx_remain) < 2 && rx_remain > 0)
> > +                             break;
> > +             }
> >       }