Re: [PATCH 2/2] hwmon: Aspeed AST2400/AST2500 ADC

[Guenter Roeck]

On 03/01/2017 07:29 PM, Rick Altherr wrote:
> Resending in plain text.
>
> On Tue, Feb 28, 2017 at 7:45 PM, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> > On 02/28/2017 04:49 PM, Joel Stanley wrote:
> > > On Wed, Mar 1, 2017 at 6:44 AM, Rick Altherr <raltherr@xxxxxxxxxx> wrote:
> > > > Aspeed AST2400/AST2500 BMC SoCs include a 16 channel, 10-bit ADC. This
> > > > driver implements reading the ADC values, enabling channels via device
> > > > tree, and optionally providing channel labels via device tree.  Low and
> > > > high threshold interrupts are supported by the hardware but not
> > > > implemented.
> > > >
> > > > Signed-off-by: Rick Altherr <raltherr@xxxxxxxxxx>
> > >
> > >
> > > Looks good. Some minor comments below.
> > >
> > > Is there a reason you wrote a hwmon driver instead of an iio driver? I
> > > wasn't sure what the recommended subsystem is.
> >
> >
> > Excellent point. Question is really if there is a plan to add support for
> > thresholds. If not, an iio driver might be more appropriate.
> >
> > Guenter
>
> The hardware supports firing interrupts on high and low thresholds.
> I'm not planning to use that feature yet so I didn't implement it.
> Would you prefer that I leave it as is (maybe with a TODO), implement
> the thresholding, or change to iio?

Let's try to determine the intended use of the ADC. I don't find the datasheet
online; all I can find is brief summaries which don't me tell much, but suggest
that it is a general purpose ADC and not specifically intended for hardware
monitoring. What is the minimum sampling rate ? That should give us an idea.
If it is in the uS range, iio would be more appropriate (and the iio-hwmon
bridge could be used if a channel is in fact used for hardware monitoring).

Thanks,
Guenter