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

[Yixun Lan <dlan@gentoo.org>]

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.

-- 
Yixun Lan (dlan)

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv