Re: [lm-sensors] [PATCH 1/3] hwmon: Driver for SMM665 Six-Channel Active DC Output Controller/Monitor

[Guenter Roeck <guenter.roeck@ericsson.com>]

On Fri, 2010-06-18 at 17:37 -0400, Jonathan Cameron wrote:
> On 06/18/10 21:56, Guenter Roeck wrote:
> > [...]
> >>> +             /*
> >>> +              * Algorithm for reading ADC, per SMM665 datasheet
> >>> +              *
> >>> +              *  {[S][addr][W][Ack]} {[offset][Ack]} {[S][addr][R][Nack]}
> >>> +              * [wait 70 uS]
> >>> +              *  {[S][addr][R][Ack]} {[datahi][Ack]} {[datalo][Ack][P]}
> >>> +              *
> >>> +              * To implement the first part of this exchange,
> >>> +              * do a full read transaction and expect a failure/Nack.
> >>> +              * This sets up the address pointer on the SMM665
> >>> +              * and starts the ADC conversion.
> >>> +              * Then do a two-byte read transaction.
> >>> +              */
> >> Is there no better way of handling this? There are protocol mangling hacks
> >> to tell the i2c core to ignore a NAKs under some circumstances.
> >>
> >>> +             rv = i2c_smbus_read_byte_data(client, adc << 3);
> >>> +             if (rv >= 0) {
> >>> +                     /* No error, something is wrong. Retry. */
> >>> +                     rv = -1;
> >>> +                     continue;
> >>> +             }
> > 
> > I looked through the core i2c code, but did not find anything I can
> > use. 
> > 
> > Problem is that per smm665 specification, the first NACK is expected. So
> > we do not just want to ignore this NACK, we want to actively check if
> > the command "failed" as expected, and report an error if it did _not_
> > fail.
> > 
> > Guenter
> To my mind this looks like a case for adding another 'mangling' flag
> to the core, but I guess that might require bus driver implementation
> which would obviously be a pain.  Perhaps the approach you have taken
> is the best plan.  My issue with it at the moment is that you are
> detecting any error rather than specifically an unexpected NACK.

Yes, but looking through bus implementations, I don't think there is a
consistent way to detect the exact error reason. How about if I weed out
-EOPNOTSUPP, -ETIMEDOUT, and -EINVAL ? 

Guenter