| Message ID | 20240830034640.7049-5-kfting@nuvoton.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Andi Shyti |
| Headers | show |
| Series | i2c: npcm: Bug fixes read/write operation, checkpatch | expand |
Hi Tyrone, On Fri, Aug 30, 2024 at 11:46:37AM GMT, Tyrone Ting wrote: > Increase the timeout and treat it as the total timeout, including retries. > The total timeout is 2 seconds now. Why? > The i2c core layer will have chances to retry to call the i2c driver > transfer function if the i2c driver reports that the bus is busy and > returns EAGAIN. > > Fixes: 48acf8292280 ("i2c: Remove redundant comparison in npcm_i2c_reg_slave") What is the bug you are fixing? > Signed-off-by: Tyrone Ting <kfting@nuvoton.com> Still... can someone from the nuvoton supporters/reviewers check this patch? Thanks, Andi
Hi Andi, On Fri, Sep 6, 2024 at 12:39 AM Andi Shyti <andi.shyti@kernel.org> wrote: > > Hi Tyrone, > > On Fri, Aug 30, 2024 at 11:46:37AM GMT, Tyrone Ting wrote: > > Increase the timeout and treat it as the total timeout, including retries. > > The total timeout is 2 seconds now. > > Why? The users want to connect a lot of masters on the same bus. This timeout is used to determine the time it takes to take bus ownership. The transactions are very long, so waiting 35ms is not enough. > > > The i2c core layer will have chances to retry to call the i2c driver > > transfer function if the i2c driver reports that the bus is busy and > > returns EAGAIN. > > > > Fixes: 48acf8292280 ("i2c: Remove redundant comparison in npcm_i2c_reg_slave") > > What is the bug you are fixing? > > > Signed-off-by: Tyrone Ting <kfting@nuvoton.com> > > Still... can someone from the nuvoton supporters/reviewers check > this patch? > > Thanks, > Andi Thanks, Tali Perry, Nuvoton Technologies.
Hi Andi: Thank you for your review. Tali Perry <tali.perry1@gmail.com> 於 2024年9月8日 週日 下午6:48寫道： > > Hi Andi, > > On Fri, Sep 6, 2024 at 12:39 AM Andi Shyti <andi.shyti@kernel.org> wrote: > > > > Hi Tyrone, > > > > On Fri, Aug 30, 2024 at 11:46:37AM GMT, Tyrone Ting wrote: > > > Increase the timeout and treat it as the total timeout, including retries. > > > The total timeout is 2 seconds now. > > > > Why? > > The users want to connect a lot of masters on the same bus. > This timeout is used to determine the time it takes to take bus ownership. > The transactions are very long, so waiting 35ms is not enough. > > > > > > The i2c core layer will have chances to retry to call the i2c driver > > > transfer function if the i2c driver reports that the bus is busy and > > > returns EAGAIN. > > > > > > Fixes: 48acf8292280 ("i2c: Remove redundant comparison in npcm_i2c_reg_slave") > > > > What is the bug you are fixing? > > I'll remove the Fixes tag in the next patch set. > > > Signed-off-by: Tyrone Ting <kfting@nuvoton.com> > > > > Still... can someone from the nuvoton supporters/reviewers check > > this patch? > > > > Thanks, > > Andi > > Thanks, > Tali Perry, > Nuvoton Technologies. Thank you. Regards, Tyrone
diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c index 2d034503d8bc..68f3d47323ab 100644 --- a/drivers/i2c/busses/i2c-npcm7xx.c +++ b/drivers/i2c/busses/i2c-npcm7xx.c @@ -2132,19 +2132,12 @@ static int npcm_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, } } - /* - * Adaptive TimeOut: estimated time in usec + 100% margin: - * 2: double the timeout for clock stretching case - * 9: bits per transaction (including the ack/nack) - */ - timeout_usec = (2 * 9 * USEC_PER_SEC / bus->bus_freq) * (2 + nread + nwrite); - timeout = max_t(unsigned long, bus->adap.timeout, usecs_to_jiffies(timeout_usec)); if (nwrite >= 32 * 1024 || nread >= 32 * 1024) { dev_err(bus->dev, "i2c%d buffer too big\n", bus->num); return -EINVAL; } - time_left = jiffies + timeout + 1; + time_left = jiffies + bus->adap.timeout / bus->adap.retries + 1; do { /* * we must clear slave address immediately when the bus is not @@ -2183,6 +2176,14 @@ static int npcm_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, if (npcm_i2c_master_start_xmit(bus, slave_addr, nwrite, nread, write_data, read_data, read_PEC, read_block)) { + /* + * Adaptive TimeOut: estimated time in usec + 100% margin: + * 2: double the timeout for clock stretching case + * 9: bits per transaction (including the ack/nack) + */ + timeout_usec = (2 * 9 * USEC_PER_SEC / bus->bus_freq) * (2 + nread + nwrite); + timeout = max_t(unsigned long, bus->adap.timeout / bus->adap.retries, + usecs_to_jiffies(timeout_usec)); time_left = wait_for_completion_timeout(&bus->cmd_complete, timeout); @@ -2308,7 +2309,7 @@ static int npcm_i2c_probe_bus(struct platform_device *pdev) adap = &bus->adap; adap->owner = THIS_MODULE; adap->retries = 3; - adap->timeout = msecs_to_jiffies(35); + adap->timeout = 2 * HZ; adap->algo = &npcm_i2c_algo; adap->quirks = &npcm_i2c_quirks; adap->algo_data = bus;
Increase the timeout and treat it as the total timeout, including retries. The total timeout is 2 seconds now. The i2c core layer will have chances to retry to call the i2c driver transfer function if the i2c driver reports that the bus is busy and returns EAGAIN. Fixes: 48acf8292280 ("i2c: Remove redundant comparison in npcm_i2c_reg_slave") Signed-off-by: Tyrone Ting <kfting@nuvoton.com> --- drivers/i2c/busses/i2c-npcm7xx.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)