On Tue, Jun 28, 2016 at 09:13:21AM +0800, Phil Reid wrote:

G'day Rob,

On 28/06/2016 05:10, Karl-Heinz Schneider wrote:

quoted

Am Sonntag, den 26.06.2016, 09:05 -0500 schrieb Rob Herring:

quoted

On Sun, Jun 26, 2016 at 12:21 AM, Phil Reid [off-list ref] wrote:

*Snip*

quoted

quoted

quoted

quoted

quoted

+- reg: integer, i2c address of the device. Should be <0xa>.
+
+Optional properties:
+- sbsm,i2c-retry-count: integer, number of retries for trying to read or
write
+    to registers. Default: 1

Seems like a driver setting. Is having a retry in the driver a problem
if the h/w works and never actually needs it?

Similarly the sbs-battery driver specifies the same same retry behaviour.
And is a model for this implementation.

I've found the ltc1760 and sbs batteries to be problematic when
communicating to them.
A lot of drivers (and the associated hardware) don't handle multiple bus
masters well.
The bus arbitation doesn't seem to work correctly.
Retries where the only thing I could do to to get things to work reliably.
Mostly means the driver needs fixing, but in one case the designware core
hardware seemed to be the problem for me.

I'm not questioning the need for a retry. I'm questioning the need to
limit the retries and tune per platform. What would be the issue if
the driver hardcodes the number of retries to 10? This will work for
any h/w that needs 0, 1, 2, ..., or 10 retries. The only issue would
be how long until it errors out.

And yes, I can confirm DW i2c h/w is a POS at least for some versions.

So your suggesting we hardcode the retry value in the driver and not provide a
configuration option in the binding?

Yes.

My only thought with allowing a dt setitng to customise the value is it allows the
integrator to select how many retires they want to try before failing, which kind of
limits the elapse time until a failure is reported.

It is easier to add later and can't be removed later. If we do want 
something like this, then it should be a common property for i2c 
devices/buses.

Rob