Re: [RFC PATCH v2 0/6] hw/i2c: i2c slave mode support

[Klaus Jensen <its@irrelevant.dk>]

[-- Attachment #1: Type: text/plain, Size: 1609 bytes --]

On Jun  2 17:40, Cédric Le Goater wrote:
> On 6/2/22 16:29, Jae Hyun Yoo wrote:
> > Hi Klaus,
> > 
> > On 6/2/2022 6:50 AM, Cédric Le Goater wrote:
> > > On 6/2/22 10:21, Klaus Jensen wrote:
> > > > 
> > > > There is an outstanding issue with the SLAVE_ADDR_RX_MATCH interrupt bit
> > > > (bit 7). Remember from my first series I had a workaround to make sure
> > > > it wasnt masked.
> > > > 
> > > > I posted this upstream to linux
> > > > 
> > > > https://lore.kernel.org/lkml/20220602054842.122271-1-its@irrelevant.dk/
> > > > 
> > > > Not sure if that is the right way to fix it.
> > > 
> > > That's weird. I would have thought it was already enabled [ Adding Jae ]
> > 
> > Slave mode support in Aspeed I2C driver is already enabled and it has
> > worked well so far. The fix Klaus made in the link is incorrect.
> > 
> > https://lore.kernel.org/lkml/20220602054842.122271-1-its@irrelevant.dk/
> > 
> > The patch is adding ASPEED_I2CD_INTR_SLAVE_MATCH as a mask bit for
> > I2CD0C (Interrupt Control Register) but actually this bit is part of
> > I2CD10 (Interrupt Status Register). Means that the slave match interrupt
> > can be enabled without enabling any mask bit in I2CD0C.
> 
> Thanks Jae.
> 
> So we should enable this interrupt always independently of the
> Interrupt Control Register value.
> 
> I would simply extend the mask value (bus->regs[intr_ctrl_reg])
> with the SLAVE_ADDR_RX_MATCH bit when interrupts are raised in
> aspeed_i2c_bus_raise_interrupt().
> 

Alright, so my "workaround" from v1 was actually the right fix - I'll
re-add it ;)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]