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: > >Am Sonntag, den 26.06.2016, 09:05 -0500 schrieb Rob Herring: > >>On Sun, Jun 26, 2016 at 12:21 AM, Phil Reid <preid@xxxxxxxxxxxxxxxxx> wrote: > > *Snip* > > > >>>>>+- 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html