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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1276898048.2631.544.camel@groeck-laptop>
Date:	Fri, 18 Jun 2010 14:54:08 -0700
From:	Guenter Roeck <guenter.roeck@...csson.com>
To:	Jonathan Cameron <kernel@...23.retrosnub.co.uk>
CC:	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"lm-sensors@...sensors.org" <lm-sensors@...sensors.org>,
	Hans de Goede <hdegoede@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [lm-sensors] [PATCH 1/3] hwmon: Driver for SMM665 Six-Channel
 Active DC Output Controller/Monitor

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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/