| Message ID | 20171130143007.30258-4-a.hajda@samsung.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | i2c: exynos5: bus recovery implementation | expand |
On Thu, Nov 30, 2017 at 03:30:07PM +0100, Andrzej Hajda wrote: > HSI2C_MASTER_ST_LOSE state is not documented properly, extensive tests > show that hardware is usually able to recover from this state without > interrupting the transfer. On the other side enforcing transfer repetition > in such case does not help in many situations, especially on busy systems > and causes -EAGAIN and -ETIMEOUT errors. Moreover documentation says that > such state can be caused by slave clock stretching, and should not be treated > as an error. > > Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> Can this be applied independently of my comments to patch 2?
On 15.01.2018 21:53, Wolfram Sang wrote: > On Thu, Nov 30, 2017 at 03:30:07PM +0100, Andrzej Hajda wrote: >> HSI2C_MASTER_ST_LOSE state is not documented properly, extensive tests >> show that hardware is usually able to recover from this state without >> interrupting the transfer. On the other side enforcing transfer repetition >> in such case does not help in many situations, especially on busy systems >> and causes -EAGAIN and -ETIMEOUT errors. Moreover documentation says that >> such state can be caused by slave clock stretching, and should not be treated >> as an error. >> >> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> > Can this be applied independently of my comments to patch 2? > Yes, please apply it alone. I will continue work on patch 2. -- Regards Andrzej
On Tue, Jan 16, 2018 at 10:40:36AM +0100, Andrzej Hajda wrote: > On 15.01.2018 21:53, Wolfram Sang wrote: > > On Thu, Nov 30, 2017 at 03:30:07PM +0100, Andrzej Hajda wrote: > >> HSI2C_MASTER_ST_LOSE state is not documented properly, extensive tests > >> show that hardware is usually able to recover from this state without > >> interrupting the transfer. On the other side enforcing transfer repetition > >> in such case does not help in many situations, especially on busy systems > >> and causes -EAGAIN and -ETIMEOUT errors. Moreover documentation says that > >> such state can be caused by slave clock stretching, and should not be treated > >> as an error. > >> > >> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> > > Can this be applied independently of my comments to patch 2? > > > Yes, please apply it alone. I will continue work on patch 2. I just thought it might be nice to have a comment where you removed the code summarizing your findings. So we will remember about this in the future. Makes sense?
On 18.01.2018 00:23, Wolfram Sang wrote: > On Tue, Jan 16, 2018 at 10:40:36AM +0100, Andrzej Hajda wrote: >> On 15.01.2018 21:53, Wolfram Sang wrote: >>> On Thu, Nov 30, 2017 at 03:30:07PM +0100, Andrzej Hajda wrote: >>>> HSI2C_MASTER_ST_LOSE state is not documented properly, extensive tests >>>> show that hardware is usually able to recover from this state without >>>> interrupting the transfer. On the other side enforcing transfer repetition >>>> in such case does not help in many situations, especially on busy systems >>>> and causes -EAGAIN and -ETIMEOUT errors. Moreover documentation says that >>>> such state can be caused by slave clock stretching, and should not be treated >>>> as an error. >>>> >>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> >>> Can this be applied independently of my comments to patch 2? >>> >> Yes, please apply it alone. I will continue work on patch 2. > I just thought it might be nice to have a comment where you removed the > code summarizing your findings. So we will remember about this in the > future. Makes sense? > Forgive me delayed response - holiday. The code removed was something extra, so I am not sure if it is necessary to add comment to the code, git log should be enough. Anyway I will post patches dealing with HSI2C_MASTER_ST_LOSE on transaction start very soon, so I can add relevant comment there. Regards Andrzej
Hi Andrzej, > Forgive me delayed response - holiday. No worries! > The code removed was something extra, so I am not sure if it is > necessary to add comment to the code, git log should be enough. If somebody later discovers this "new bit" and implements support for it, I am not sure this person will check git log to see if there has ever been support added and later removed. Also, it helps me reviewing when gory details are explicitly mentioned in the code. > Anyway I will post patches dealing with HSI2C_MASTER_ST_LOSE on > transaction start very soon, so I can add relevant comment there. Please do. Kind regards, Wolfram
diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c index 4ca43980e2ed..d579b45b3092 100644 --- a/drivers/i2c/busses/i2c-exynos5.c +++ b/drivers/i2c/busses/i2c-exynos5.c @@ -445,12 +445,6 @@ static irqreturn_t exynos5_i2c_irq(int irqno, void *dev_id) i2c->state = -ETIMEDOUT; goto stop; } - - trans_status = readl(i2c->regs + HSI2C_TRANS_STATUS); - if ((trans_status & HSI2C_MASTER_ST_MASK) == HSI2C_MASTER_ST_LOSE) { - i2c->state = -EAGAIN; - goto stop; - } } else if (int_status & HSI2C_INT_I2C) { trans_status = readl(i2c->regs + HSI2C_TRANS_STATUS); if (trans_status & HSI2C_NO_DEV_ACK) {
HSI2C_MASTER_ST_LOSE state is not documented properly, extensive tests show that hardware is usually able to recover from this state without interrupting the transfer. On the other side enforcing transfer repetition in such case does not help in many situations, especially on busy systems and causes -EAGAIN and -ETIMEOUT errors. Moreover documentation says that such state can be caused by slave clock stretching, and should not be treated as an error. Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> --- drivers/i2c/busses/i2c-exynos5.c | 6 ------ 1 file changed, 6 deletions(-)