* [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.