Re: [PATCH] hwmon: (ltc4245) Read GPIO registers correctly
Ira W. Snyder <iws <at> ovro.caltech.edu>
2010-04-02 16:45:07 GMT
On Fri, Apr 02, 2010 at 06:24:38PM +0200, Jean Delvare wrote:
> On Fri, 2 Apr 2010 09:12:59 -0700, Ira W. Snyder wrote:
> > On Fri, Apr 02, 2010 at 10:13:41AM +0200, Jean Delvare wrote:
> > > Hi Ira,
> > >
> > > On Tue, 30 Mar 2010 15:57:04 -0700, Ira W. Snyder wrote:
> > > > The GPIO registers on the ltc4245 behave in a strage way. Every time the
> > > > ADC updates the values for one GPIO, it updates all three GPIO registers to
> > > > the same value.
> > >
> > > This is not how I read the datasheet. The datasheet says that only one
> > > of the GPIO pins can be routed to the 13th input of the ADC and thus
> > > used as an extra analog voltage input. It also says that there is ONE
> > > register (U) holding the result of the conversion. Apparently many
> > > registers can be access using more than one address, and register U can
> > > be accessed using 4 different addresses (0x1c, 0x1d, 0x1e, 0x1f) but
> > > this is still only one register. So the fact that all addresses return
> > > the same value is not a surprise.
> > >
> >
> > Ok, re-reading tables 6 and 15 in the datasheet, this makes sense now.
> >
> > > > To read all three GPIO registers correctly, we cache copies of each
> > > > register and update the GPIO MUX to read the next GPIO register. Each GPIO
> > > > sample will take 3 * HZ to read, but at least we are returning the correct
> > > > values to userspace.
> > >
> > > I think this is wrong. You should not expose 3 values to user-space,
> > > you should only expose one. The two other GPIOs are supposed to be used
> > > in digital mode. It's not even clear to me that you should always expose
> > > it, as the application may be using all 3 GPIO pins in digital mode and
> > > not be interested in sampling an analog voltage value from any of them.
> > > I admit I am a little surprised that there is no way to disable ADC
> > > operation on the GPIO pin.
> > >
> >
> > If I do this, how do I make userspace work?
> >
> > On my board, I have three different devices hooked up to the three GPIO
> > pins. They're all analog voltages. From userspace, I currently read
> > in{9,10,11}_input and report those in volts. It works fine, and was
> > pretty easy to implement.
> >
> > If I only report a single GPIO value to userspace, then I can only read
> > one device. How do I switch which GPIO pin is routed to the ADC? I could
> > open the /dev/i2c-0 device and poke the LTC4245_CONTROL register
> > directly, but that seems very anti-hwmon, doesn't it?
>
> You wired your chip in a way it was not meant to be. You shouldn't have.
>
Why do you think it is wired incorrectly? Nothing I've found in the
datasheet suggests that. If you can provide some insight into why you
think this is, maybe we can come to an agreement, instead of just making
me very frustrated.
On Page 24, "General Purpose Input/Outputs", the text says that any of
the three GPIO pins can be connected to the ADC. Page 32, Table 13 "GPIO
Register G" supports this. Each of the three GPIO pins has exactly the
same configurable settings as all the others.
Page 13 "Configuration, GPIO, and Precharge" also states: The GPIO1 to
GPIO3 pins can be used as general purpose inputs or outputs. One of the
pins can also be multiplexed to the GPIO channel of the ADC.
The way I read this, it means: only one of the pins can be multiplexed
onto the ADC ***at a single moment in time***. Why else would the GPIO
register (0x6) exist, other than to let the user switch between them at
runtime? This kind of design sucks, but it is common in the embedded
world, where we are always trying to use fewer parts to do the same job.
Ira
> >
> > > > Signed-off-by: Ira W. Snyder <iws <at> ovro.caltech.edu>
> > > > ---
> > > >
> > > > When I implemented the ltc4245 driver, I did not have the ability to test
> > > > the GPIO pins. I now have devices hooked up, and they don't work exactly as
> > > > the datasheet describes (though it is vague in this section). See the patch
> > > > description for more detail.
> > >
> > > I think it does behave as the datasheet describes, I'm not sure what
> > > you think is vague, it looks clear to me.
> > >
> > > > I'd really like to see this picked up. I hunch I am the only user of the
> > > > driver, or others would have complained. :) I wish the code wasn't as ugly
> > > > as it is, but it is the best I can come up with.
> > >
> > > I agree the driver is broken currently, but I don't agree with the
> > > proposed fix. Please just drop the LTC4245_GPIOADC2 and
> > > LTC4245_GPIOADC3 commands, which do not exist, and rename
> > > LTC4245_GPIOADC1 to LTC4245_GPIOADC.
> > >
> >
> > Ok, so I should use LTC4245_GPIOADC (0x1c) to read all of the GPIO
> > values. That seems fine to me, I can implement that.
> >
> > See above about how to report this to userspace. I don't know how you
> > want this exposed in sysfs.
>
> I don't want it exposed at all.
>
> --
> Jean Delvare
>
_______________________________________________
lm-sensors mailing list
lm-sensors <at> lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Luca
_______________________________________________
lm-sensors mailing list
lm-sensors <at> lm-sensors.org
RSS Feed