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

Troy Mitchell
2025-09-28T08:11:09Z

On Sun, Sep 28, 2025 at 10:54:00AM +0800, Yixun Lan wrote:
> Hi Troy,
>
> On 09:17 Sun 28 Sep , Troy Mitchell wrote:
> > On Sat, Sep 27, 2025 at 06:56:16PM +0800, Yixun Lan wrote:
> > > On 10:02 Thu 25 Sep , 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
> > > > @@ -97,6 +97,9 @@
> > > >
> > > ..
> > > > static int spacemit_i2c_xfer_msg(struct spacemit_i2c_dev *i2c)
> > > > {
> > > > unsigned long time_left;
> > > > @@ -310,10 +368,28 @@ static int spacemit_i2c_xfer_msg(struct spacemit_i2c_dev *i2c)
> > > >
> > > > reinit_completion(&i2c->complete);
> > > >
> > > > - spacemit_i2c_start(i2c);
> > > > + if (i2c->is_pio) {
> > > > + /* We disable the interrupt to avoid unintended spurious triggers. */
> > > the comment is suspicious, and seems wrong..
> > > > + disable_irq(i2c->irq);
> > > > +
> > > I guess this doesn't disable interrupt in the hardware layer, it will still
> > > fire interrupt once enabled, so instead of calling disable_irq(), why not
> > > dealing with ISR setting of the controller? I mean those "IE bits"(interrupt
> > > enableing) of ICR REGISTER, disabling them should prevent the interrupt
> > > triggered?
> > For example, take MSD (master stop detect).
> > If we disable this interrupt, even the interrupt status bit will never be triggered.
> No, this is not something I understand..
> > Then how are we supposed to know when the transfer has completed?
> > That’s why we disable the global interrupt here, but still keep the pending bit.
> checking 18.1.4.1 ICR REGISTER, there is Master Stop Deteced Enable - MSDE (Bit[26]) and
> Master Stop Deteced Interrupt Enable - MSDIE (BIT[25])
> you can disable MSDIE but still enable MSDE, and check status of MSD (BIT[26]) of ISR REGISTER?
> it should still work with interrupt disabled, otherwise I'd consider the hardware is broken.

Okay..maybe I misunderstood before.
I'll remove disable/enable the controller IRQ around PIO transfers.

BTW, the K1 I2C patch [1] for ILCR as the SCL clock hasn't gotten
any replies in ages. Could you take a look?

Link:
https://lore.kernel.org/all/20250814-k1-i2c-ilcr-v3-1-317723e74bcd@linux.spacemit.com/ [1]
>
> --
> Yixun Lan (dlan)
>