Re: [PATCH v2 6/6] i2c: spacemit: introduce pio for k1

Troy Mitchell
2025-09-27T04:07:21Z

On Fri, Sep 26, 2025 at 06:47:14PM +0200, Aurelien Jarno wrote:
> Hi Troy,
>
> On 2025-09-25 10:02, Troy Mitchell wrote:
> > This patch introduces I2C PIO functionality for the Spacemit K1 SoC,
> > enabling the use of I2C with interrupts disabled.
> >
> > Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> > ---
> > drivers/i2c/busses/i2c-k1.c | 164 +++++++++++++++++++++++++++++++++++++-------
> > 1 file changed, 140 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
> > index 6b918770e612e098b8ad17418f420d87c94df166..e403eb7d6f329f4fe5c5242f94cc21094dff105c 100644
> > --- a/drivers/i2c/busses/i2c-k1.c
> > +++ b/drivers/i2c/busses/i2c-k1.c
>
> [snip]
>
> > @@ -293,10 +307,54 @@ static void spacemit_i2c_start(struct spacemit_i2c_dev *i2c)
> > /* send start pulse */
> > val = readl(i2c->base + SPACEMIT_ICR);
> > val &= ~SPACEMIT_CR_STOP;
> > - val |= SPACEMIT_CR_START | SPACEMIT_CR_TB | SPACEMIT_CR_DTEIE;
> > + val |= SPACEMIT_CR_START | SPACEMIT_CR_TB;
> > +
> > + if (!i2c->is_pio)
> > + val |= SPACEMIT_CR_DTEIE;
> > +
> > writel(val, i2c->base + SPACEMIT_ICR);
> > }
> >
> > +static irqreturn_t spacemit_i2c_irq_handler(int irq, void *devid);
> > +static int spacemit_i2c_wait_pio_xfer(struct spacemit_i2c_dev *i2c)
> > +{
> > + u32 msec = jiffies_to_msecs(i2c->adapt.timeout);
> > + ktime_t timeout = ktime_add_ms(ktime_get(), msec);
> > + int ret;
> > +
> > + while (i2c->unprocessed && ktime_compare(ktime_get(), timeout) < 0) {
> > + udelay(10);
> > + i2c->status = readl(i2c->base + SPACEMIT_ISR);
> > +
> > + spacemit_i2c_clear_int_status(i2c, i2c->status);
> > +
> > + if (!(i2c->status & SPACEMIT_SR_IRF) && !(i2c->status & SPACEMIT_SR_ITE))
> > + continue;
> > +
> > + spacemit_i2c_irq_handler(0, i2c);
> > +
> > + i2c->status = readl(i2c->base + SPACEMIT_ISR);
> > +
> > + /*
> > + * This is the last byte to write of the current message.
> > + * If we do not wait here, control will proceed directly to start(),
> > + * which would overwrite the current data.
> > + */
> > + if (!i2c->read && !i2c->unprocessed) {
> > + ret = readl_poll_timeout(i2c->base + SPACEMIT_ISR,
> > + i2c->status, i2c->status & SPACEMIT_SR_ITE,
> > + 30, 1000);
>
> This needs to be readl_poll_timeout_atomic(), like you changed in
> spacemit_i2c_wait_bus_idle().
Yes, nice catch!
That's my mistake... I forgot this.
>
> > + if (ret)
> > + return 0;
> > + }
> > + }
> > +
> > + if (i2c->unprocessed)
> > + return 0;
> > +
> > + return 1;
> > +}
> > +
> > static int spacemit_i2c_xfer_msg(struct spacemit_i2c_dev *i2c)
> > {
> > unsigned long time_left;
>
> [snip]
>
> > @@ -479,15 +578,21 @@ static void spacemit_i2c_calc_timeout(struct spacemit_i2c_dev *i2c)
> > i2c->adapt.timeout = usecs_to_jiffies(timeout + USEC_PER_SEC / 10) / i2c->msg_num;
> > }
> >
> > -static int spacemit_i2c_xfer(struct i2c_adapter *adapt, struct i2c_msg *msgs, int num)
> > +static inline int
> > +spacemit_i2c_xfer(struct i2c_adapter *adapt, struct i2c_msg *msgs, int num, bool is_pio)
> > {
> > struct spacemit_i2c_dev *i2c = i2c_get_adapdata(adapt);
> > int ret;
> >
> > + i2c->is_pio = is_pio;
> > +
> > i2c->msgs = msgs;
> > i2c->msg_num = num;
> >
> > - spacemit_i2c_calc_timeout(i2c);
> > + if (!i2c->is_pio)
> > + spacemit_i2c_calc_timeout(i2c);
> > + else
> > + i2c->adapt.timeout = SPACEMIT_WAIT_TIMEOUT;
>
> Thanks for fixing that, however i2c->adapt.timeout needs to be in
> jiffies, so you want to use msecs_to_jiffies(SPACEMIT_WAIT_TIMEOUT).
Thanks for pointing this out!

- Troy
>
> > spacemit_i2c_init(i2c);
> >
>
> Regards
> Aurelien
>
> --
> Aurelien Jarno GPG: 4096R/1DDD8C9B
> aurelien@aurel32.net http://aurel32.net
>