On 10/26/22 16:34, Andy Shevchenko wrote:
On Wed, Oct 26, 2022 at 03:39:11PM +0300, Jarkko Nikula wrote:
i2c_dw_disable_int() and disable_int pointer in struct dw_i2c_dev were
introduced by the commit 90312351fd1e ("i2c: designware: MASTER mode as
separated driver") but it looks i2c_dw_disable_int() was never called
through the pointer.
But the last part is not true... See below.
Well, perhaps needs clarification, commenting more below. I need to
update patch anyway since I realized I forgot to remove kernel doc
comment for disable_int pointer from i2c-designware-core.h.
Since i2c_dw_disable_int() is just masking interrupts and the direct
DW_IC_INTR_MASK register write looks more clear in the code use that and
remove from common code.
...
else if (unlikely(dev->flags & ACCESS_INTR_MASK)) {
/* Workaround to trigger pending interrupt */
regmap_read(dev->map, DW_IC_INTR_MASK, &stat);
- i2c_dw_disable_int(dev);
+ regmap_write(dev->map, DW_IC_INTR_MASK, 0);
regmap_write(dev->map, DW_IC_INTR_MASK, stat);
Not sure I understood this dance. What exactly happen for the interrupts
that are getting masked and immediately unmasked? Is that the core of
the above mentioned workaround?
This workaround was introduced by commit 2d244c81481f ("i2c: designware:
fix IO timeout issue for AMD controller").
}
...
- dev->disable_int(dev);
+ regmap_write(dev->map, DW_IC_INTR_MASK, 0);
Called via pointer, didn't it?
Well, a bit messy by itself since pointer is unconditionally set and
used in the same module.
I guess original patch was accidentally copying the same idea than with
->init pointer that will be pointing to different master and slave init
functions.
