Re: [PATCH v2 1/2] i2c: designware: fix poll-after-enable regression

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/30/2018 07:08 PM, Ben Gardner wrote:
On Sat, Apr 28, 2018 at 8:56 AM, Alexander Monakov <amonakov@xxxxxxxxx> wrote:
Not all revisions of DW I2C controller implement the enable status register.
On platforms where that's the case (e.g. BG2CD and SPEAr ARM SoCs), waiting
for enable will time out as reading the unimplemented register yields zero.

It was observed that reading the IC_ENABLE_STATUS register once suffices to
avoid getting it stuck on Bay Trail hardware, so replace polling with one
dummy read of the register.

Cc: Ben Gardner <gardner.ben@xxxxxxxxx>
Cc: Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx>
Cc: Wolfram Sang <wsa@xxxxxxxxxxxxx>
Fixes: fba4adbbf670 ("i2c: designware: must wait for enable")
Tested-by: Ben Gardner <gardner.ben@xxxxxxxxx>
Signed-off-by: Alexander Monakov <amonakov@xxxxxxxxx>
---
  drivers/i2c/busses/i2c-designware-master.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index fd36c39ddf4e..0cdba29ae0a9 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -209,7 +209,10 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
         i2c_dw_disable_int(dev);

         /* Enable the adapter */
-       __i2c_dw_enable_and_wait(dev, true);
+       __i2c_dw_enable(dev, true);
+
+       /* Dummy read to avoid the register getting stuck on Bay Trail */
+       dw_readl(dev, DW_IC_ENABLE_STATUS);

         /* Clear and enable interrupts */
         dw_readl(dev, DW_IC_CLR_INTR);
--
Tested-by: Ben Gardner <gardner.ben@xxxxxxxxx>

I noticed the loop in __i2c_dw_enable_and_wait() iterated twice on a Baytrail I have when enabling. On a few other machines only once. I guess that's the reason why above dummy read works here. Hopefully it doesn't need longer setup time on some platform.

I also debugged does IC_STATUS and IC_RAW_INTR_STAT show similar delay but they turned instantly so I guess they cannot be used as an alternative for DW_IC_ENABLE_STATUS polling directly for all platforms.

So I think we could try this patch since it fixes the issue and we don't know what platforms implement enable status in IC_ENABLE_STATUS.

Acked-by: Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx>



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux