RE: [PATCH v3] i2c: designware: call i2c_dw_read_clear_intrbits_slave() once

[Michael.Wu]

Hi,

> On 10/23/20 8:40 AM, Michael Wu wrote:
> > If some bits were cleared by i2c_dw_read_clear_intrbits_slave() in
> > i2c_dw_isr_slave() and not handled immediately, those cleared bits would
> > not be shown again by later i2c_dw_read_clear_intrbits_slave(). They
> > therefore were forgotten to be handled.
> >
> > i2c_dw_read_clear_intrbits_slave() should be called once in an ISR and take
> > its returned state for all later handlings.
> > ---
> >   drivers/i2c/busses/i2c-designware-slave.c | 7 +--
> >   1 file changed, 1 insertion(+), 6 deletions(-)
> >
> Acked-by: Jarkko Nikula 

There is another logic issue about the I2C_SLAVE_WRITE_REQUESTED reporting.
May I give patches based on this commit?

Michael Wu

Re: [PATCH v3] i2c: designware: call i2c_dw_read_clear_intrbits_slave() once

[Jarkko Nikula]

On 10/23/20 8:40 AM, Michael Wu wrote:

If some bits were cleared by i2c_dw_read_clear_intrbits_slave() in
i2c_dw_isr_slave() and not handled immediately, those cleared bits would
not be shown again by later i2c_dw_read_clear_intrbits_slave(). They
therefore were forgotten to be handled.

i2c_dw_read_clear_intrbits_slave() should be called once in an ISR and take
its returned state for all later handlings.

Signed-off-by: Michael Wu 
---

Change in v3:
  - revert deleted braces of 'else' branch in v2

Change in v2:
  - revert moving I2C_SLAVE_WRITE_REQUESTED reporting in v1

  drivers/i2c/busses/i2c-designware-slave.c | 7 +--
  1 file changed, 1 insertion(+), 6 deletions(-)


Acked-by: Jarkko Nikula 

[PATCH v3] i2c: designware: call i2c_dw_read_clear_intrbits_slave() once

[Michael Wu]

If some bits were cleared by i2c_dw_read_clear_intrbits_slave() in
i2c_dw_isr_slave() and not handled immediately, those cleared bits would
not be shown again by later i2c_dw_read_clear_intrbits_slave(). They
therefore were forgotten to be handled.

i2c_dw_read_clear_intrbits_slave() should be called once in an ISR and take
its returned state for all later handlings.

Signed-off-by: Michael Wu 
---

Change in v3:
 - revert deleted braces of 'else' branch in v2

Change in v2:
 - revert moving I2C_SLAVE_WRITE_REQUESTED reporting in v1

 drivers/i2c/busses/i2c-designware-slave.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-slave.c 
b/drivers/i2c/busses/i2c-designware-slave.c
index 44974b53a626..13de01a0f75f 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -159,7 +159,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
u32 raw_stat, stat, enabled, tmp;
u8 val = 0, slave_activity;
 
-   regmap_read(dev->map, DW_IC_INTR_STAT, );
regmap_read(dev->map, DW_IC_ENABLE, );
regmap_read(dev->map, DW_IC_RAW_INTR_STAT, _stat);
regmap_read(dev->map, DW_IC_STATUS, );
@@ -168,6 +167,7 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY) || !dev->slave)
return 0;
 
+   stat = i2c_dw_read_clear_intrbits_slave(dev);
dev_dbg(dev->dev,
"%#x STATUS SLAVE_ACTIVITY=%#x : RAW_INTR_STAT=%#x : 
INTR_STAT=%#x\n",
enabled, slave_activity, raw_stat, stat);
@@ -188,11 +188,9 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
 val);
}
regmap_read(dev->map, DW_IC_CLR_RD_REQ, );
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
} else {
regmap_read(dev->map, DW_IC_CLR_RD_REQ, );
regmap_read(dev->map, DW_IC_CLR_RX_UNDER, );
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
}
if (!i2c_slave_event(dev->slave,
 I2C_SLAVE_READ_REQUESTED,
@@ -207,7 +205,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
regmap_read(dev->map, DW_IC_CLR_RX_DONE, );
 
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, );
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
return 1;
}
 
@@ -219,7 +216,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
dev_vdbg(dev->dev, "Byte %X acked!", val);
} else {
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, );
-   stat = i2c_dw_read_clear_intrbits_slave(dev);
}
 
return 1;
@@ -230,7 +226,6 @@ static irqreturn_t i2c_dw_isr_slave(int this_irq, void 
*dev_id)
struct dw_i2c_dev *dev = dev_id;
int ret;
 
-   i2c_dw_read_clear_intrbits_slave(dev);
ret = i2c_dw_irq_handler_slave(dev);
if (ret > 0)
complete(>cmd_complete);
-- 
2.17.1