* [PATCH] i2c: designware: retry transfer on transient failure @ 2015-12-23 16:43 Baruch Siach 2015-12-23 16:51 ` Andy Shevchenko ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Baruch Siach @ 2015-12-23 16:43 UTC (permalink / raw) To: Andy Shevchenko, Jarkko Nikula, Mika Westerberg Cc: linux-i2c, Wolfram Sang, Rolland Chau, Baruch Siach Set the i2c_adapter retries field to a sensible value. This allows the i2c core to retry master_xfer() when it returns -EAGAIN. Currently the i2c-designware driver returns -EAGAIN only on Tx arbitration failure (DW_IC_TX_ARB_LOST). Reported-by: Rolland Chau <zourongrong@gmail.com> Signed-off-by: Baruch Siach <baruch@tkos.co.il> --- drivers/i2c/busses/i2c-designware-core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c index de7fbbb374cd..f7b34b360dc9 100644 --- a/drivers/i2c/busses/i2c-designware-core.c +++ b/drivers/i2c/busses/i2c-designware-core.c @@ -860,6 +860,7 @@ int i2c_dw_probe(struct dw_i2c_dev *dev) snprintf(adap->name, sizeof(adap->name), "Synopsys DesignWare I2C adapter"); + adap->retries = 3; adap->algo = &i2c_dw_algo; adap->dev.parent = dev->dev; i2c_set_adapdata(adap, dev); -- 2.6.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] i2c: designware: retry transfer on transient failure 2015-12-23 16:43 [PATCH] i2c: designware: retry transfer on transient failure Baruch Siach @ 2015-12-23 16:51 ` Andy Shevchenko 2015-12-28 16:34 ` Andy Shevchenko 2016-01-10 8:25 ` Wolfram Sang 2 siblings, 0 replies; 8+ messages in thread From: Andy Shevchenko @ 2015-12-23 16:51 UTC (permalink / raw) To: Baruch Siach, Jarkko Nikula, Mika Westerberg Cc: linux-i2c, Wolfram Sang, Rolland Chau On Wed, 2015-12-23 at 18:43 +0200, Baruch Siach wrote: > Set the i2c_adapter retries field to a sensible value. This allows > the i2c core > to retry master_xfer() when it returns -EAGAIN. Currently the i2c- > designware > driver returns -EAGAIN only on Tx arbitration failure > (DW_IC_TX_ARB_LOST). While this patch looks okay I have another proposal. Let me time I'm going to mock up the idea. > > Reported-by: Rolland Chau <zourongrong@gmail.com> > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > --- > drivers/i2c/busses/i2c-designware-core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/i2c/busses/i2c-designware-core.c > b/drivers/i2c/busses/i2c-designware-core.c > index de7fbbb374cd..f7b34b360dc9 100644 > --- a/drivers/i2c/busses/i2c-designware-core.c > +++ b/drivers/i2c/busses/i2c-designware-core.c > @@ -860,6 +860,7 @@ int i2c_dw_probe(struct dw_i2c_dev *dev) > > snprintf(adap->name, sizeof(adap->name), > "Synopsys DesignWare I2C adapter"); > + adap->retries = 3; > adap->algo = &i2c_dw_algo; > adap->dev.parent = dev->dev; > i2c_set_adapdata(adap, dev); -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] i2c: designware: retry transfer on transient failure 2015-12-23 16:43 [PATCH] i2c: designware: retry transfer on transient failure Baruch Siach 2015-12-23 16:51 ` Andy Shevchenko @ 2015-12-28 16:34 ` Andy Shevchenko 2016-01-04 19:49 ` Wolfram Sang 2016-01-10 8:25 ` Wolfram Sang 2 siblings, 1 reply; 8+ messages in thread From: Andy Shevchenko @ 2015-12-28 16:34 UTC (permalink / raw) To: Baruch Siach, Jarkko Nikula, Mika Westerberg Cc: linux-i2c, Wolfram Sang, Rolland Chau On Wed, 2015-12-23 at 18:43 +0200, Baruch Siach wrote: > Set the i2c_adapter retries field to a sensible value. This allows > the i2c core > to retry master_xfer() when it returns -EAGAIN. Currently the i2c- > designware > driver returns -EAGAIN only on Tx arbitration failure > (DW_IC_TX_ARB_LOST). Wolfram, regarding to this patch I have the following idea (I would like to discuss a road map before step in implementing that). First of all I would like to refactor the existing API, i.e. i2c_parse_fw_timings(). So, do exactly two things: a) embed struct i2c_timings into struct i2c_adapter; b) change prototype to be i2c_parse_fw_timings(struct i2c_adapter *adapter, bool use_defaults). After that, introduce a new property 'linux,i2c-retry-count' to be used as retries field in struct i2c_adapter. Then introduce where we do similar stuff void i2c_parse_linux_retries(struct i2c_adapter *adapter, bool use_defaults) { struct device *dev = adapter->dev.parent; int ret; ret = device_property_read_u{8,16,32?}(dev, "linux,i2c-retry-count", &adapter->retries); if (ret && use_defaults) adapter->retries = 3; } And replace adapter.retries = 3 in the drivers by i2c_parse_linux_retries(&adapter, true); So, what do you think? > > Reported-by: Rolland Chau <zourongrong@gmail.com> > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > --- > drivers/i2c/busses/i2c-designware-core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/i2c/busses/i2c-designware-core.c > b/drivers/i2c/busses/i2c-designware-core.c > index de7fbbb374cd..f7b34b360dc9 100644 > --- a/drivers/i2c/busses/i2c-designware-core.c > +++ b/drivers/i2c/busses/i2c-designware-core.c > @@ -860,6 +860,7 @@ int i2c_dw_probe(struct dw_i2c_dev *dev) > > snprintf(adap->name, sizeof(adap->name), > "Synopsys DesignWare I2C adapter"); > + adap->retries = 3; > adap->algo = &i2c_dw_algo; > adap->dev.parent = dev->dev; > i2c_set_adapdata(adap, dev); -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] i2c: designware: retry transfer on transient failure 2015-12-28 16:34 ` Andy Shevchenko @ 2016-01-04 19:49 ` Wolfram Sang 2016-01-05 12:22 ` Andy Shevchenko 0 siblings, 1 reply; 8+ messages in thread From: Wolfram Sang @ 2016-01-04 19:49 UTC (permalink / raw) To: Andy Shevchenko Cc: Baruch Siach, Jarkko Nikula, Mika Westerberg, linux-i2c, Rolland Chau [-- Attachment #1: Type: text/plain, Size: 495 bytes --] > After that, introduce a new property 'linux,i2c-retry-count' to be used > as retries field in struct i2c_adapter. Hmm, to be honest, I always have difficulties with this "retries" parameter; to me "try x milliseconds" makes more sense than "try 5 times". It is there for ages, so we have to stick with it, but frankly, I wouldn't like to expose it too much :) I'm okay with the original patch putting some "sane" initial value. It can be modified at runtime via a i2c-dev ioctl if needed. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] i2c: designware: retry transfer on transient failure 2016-01-04 19:49 ` Wolfram Sang @ 2016-01-05 12:22 ` Andy Shevchenko 2016-01-06 4:40 ` Baruch Siach 0 siblings, 1 reply; 8+ messages in thread From: Andy Shevchenko @ 2016-01-05 12:22 UTC (permalink / raw) To: Wolfram Sang Cc: Baruch Siach, Jarkko Nikula, Mika Westerberg, linux-i2c, Rolland Chau On Mon, 2016-01-04 at 20:49 +0100, Wolfram Sang wrote: > > After that, introduce a new property 'linux,i2c-retry-count' to be > > used > > as retries field in struct i2c_adapter. > > Hmm, to be honest, I always have difficulties with this "retries" > parameter; to me "try x milliseconds" makes more sense than "try 5 > times". It is there for ages, so we have to stick with it, but > frankly, > I wouldn't like to expose it too much :) Point taken. > I'm okay with the original patch putting some "sane" initial value. > It > can be modified at runtime via a i2c-dev ioctl if needed. Ah, good. So, I'm fine with it if no one has strong argument. -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] i2c: designware: retry transfer on transient failure 2016-01-05 12:22 ` Andy Shevchenko @ 2016-01-06 4:40 ` Baruch Siach 2016-01-07 12:32 ` Andy Shevchenko 0 siblings, 1 reply; 8+ messages in thread From: Baruch Siach @ 2016-01-06 4:40 UTC (permalink / raw) To: Andy Shevchenko Cc: Wolfram Sang, Jarkko Nikula, Mika Westerberg, linux-i2c, Rolland Chau Hi Andy, On Tue, Jan 05, 2016 at 02:22:28PM +0200, Andy Shevchenko wrote: > On Mon, 2016-01-04 at 20:49 +0100, Wolfram Sang wrote: > > I'm okay with the original patch putting some "sane" initial value. > > It can be modified at runtime via a i2c-dev ioctl if needed. > > Ah, good. So, I'm fine with it if no one has strong argument. So can you give your ack to the original patch then? Thanks, baruch -- http://baruch.siach.name/blog/ ~. .~ Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il - ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] i2c: designware: retry transfer on transient failure 2016-01-06 4:40 ` Baruch Siach @ 2016-01-07 12:32 ` Andy Shevchenko 0 siblings, 0 replies; 8+ messages in thread From: Andy Shevchenko @ 2016-01-07 12:32 UTC (permalink / raw) To: Baruch Siach Cc: Wolfram Sang, Jarkko Nikula, Mika Westerberg, linux-i2c, Rolland Chau On Wed, 2016-01-06 at 06:40 +0200, Baruch Siach wrote: > Hi Andy, > > On Tue, Jan 05, 2016 at 02:22:28PM +0200, Andy Shevchenko wrote: > > On Mon, 2016-01-04 at 20:49 +0100, Wolfram Sang wrote: > > > I'm okay with the original patch putting some "sane" initial > > > value. > > > It can be modified at runtime via a i2c-dev ioctl if needed. > > > > Ah, good. So, I'm fine with it if no one has strong argument. > > So can you give your ack to the original patch then? Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] i2c: designware: retry transfer on transient failure 2015-12-23 16:43 [PATCH] i2c: designware: retry transfer on transient failure Baruch Siach 2015-12-23 16:51 ` Andy Shevchenko 2015-12-28 16:34 ` Andy Shevchenko @ 2016-01-10 8:25 ` Wolfram Sang 2 siblings, 0 replies; 8+ messages in thread From: Wolfram Sang @ 2016-01-10 8:25 UTC (permalink / raw) To: Baruch Siach Cc: Andy Shevchenko, Jarkko Nikula, Mika Westerberg, linux-i2c, Rolland Chau [-- Attachment #1: Type: text/plain, Size: 446 bytes --] On Wed, Dec 23, 2015 at 06:43:24PM +0200, Baruch Siach wrote: > Set the i2c_adapter retries field to a sensible value. This allows the i2c core > to retry master_xfer() when it returns -EAGAIN. Currently the i2c-designware > driver returns -EAGAIN only on Tx arbitration failure (DW_IC_TX_ARB_LOST). > > Reported-by: Rolland Chau <zourongrong@gmail.com> > Signed-off-by: Baruch Siach <baruch@tkos.co.il> Applied to for-next, thanks! [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-01-10 8:25 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-23 16:43 [PATCH] i2c: designware: retry transfer on transient failure Baruch Siach 2015-12-23 16:51 ` Andy Shevchenko 2015-12-28 16:34 ` Andy Shevchenko 2016-01-04 19:49 ` Wolfram Sang 2016-01-05 12:22 ` Andy Shevchenko 2016-01-06 4:40 ` Baruch Siach 2016-01-07 12:32 ` Andy Shevchenko 2016-01-10 8:25 ` Wolfram Sang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.