Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1376157

Re: [PATCH 1/1] iio: adc: Add driver for the TI INA3221 Triple Current/Voltage Monitor

Show key headers only | View raw

On Mon, Apr 11, 2016 at 10:48:27AM -0500, Andrew F. Davis wrote:
> On 04/10/2016 06:57 AM, Jonathan Cameron wrote:
> > On 08/04/16 19:19, Andrew F. Davis wrote:
> >> The INA3221 is a three-channel, high-side current and bus voltage monitor
> >> with an I2C and SMBUS compatible interface.
> >>
> >> Signed-off-by: Andrew F. Davis <afd@ti.com>
> > Hi Andrew,
> > 
> > My immediate thought on this one is whether it would be better off in hwmon.
> > I'm not convinced either way from a quick glance at the data sheet, but the
> > question needs to be addressed.
> > 
> 
> I was on the fence with this also, I was leaning towards hwmod until I
> looked onto how the ina2xx driver has been ported to iio. This is almost
> the same part but the ina3x has three monitors vs one. In addition it
> looks like NVIDIA has written a hwmod driver for this part
> (https://github.com/Bogdacutu/STLinux-Kernel/blob/master/drivers/hwmon/ina3221.c)
> but then also ported it over to IIO (although doesn't appear to be
> upstream ready or ever has been submitted for such)
> (https://github.com/SuperPichu/shield-tablet-kernel/blob/master/drivers/staging/iio/meter/ina3221.c)
> So I figured this was the way things are moving, but I have no problem
> working this as a hwmod driver. The IIO work is already done here, I'll
> write the hwmod version also but keep working this, I see no reason we
> cant have both if needed. (unless using this and just using iio_hwmod.c
> if needed is more acceptable?)
> 

You can not have both since they would conflict with each other.
ina2xx has possibly created a bad precedent. I am not inclined to accept
a hwmon driver if an iio driver already exists. If you want an iio driver,
fine with me, but then you (and its users) will have to live with its
limitations when it comes to hardware monitoring.

I don't really mind if things are going all towards iio if that is where
the community wants to go. However, if that is the case, I would suggest
that someone should spend the time to define a clear sense of 'limits'
as well as alert handling in iio, in a way that is exportable to other
subsystems (hwmon and thermal come into mind).

Guenter

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 1/1] iio: adc: Add driver for the TI INA3221 Triple Current/Voltage Monitor "Andrew F. Davis" <afd@ti.com> - 2016-04-08 20:30 +0200
  Re: [PATCH 1/1] iio: adc: Add driver for the TI INA3221 Triple  Current/Voltage Monitor Jonathan Cameron <jic23@kernel.org> - 2016-04-10 14:00 +0200
    Re: [PATCH 1/1] iio: adc: Add driver for the TI INA3221 Triple  Current/Voltage Monitor Guenter Roeck <linux@roeck-us.net> - 2016-04-10 17:20 +0200
      Re: [PATCH 1/1] iio: adc: Add driver for the TI INA3221 Triple  Current/Voltage Monitor "Andrew F. Davis" <afd@ti.com> - 2016-04-11 18:10 +0200
        Re: [PATCH 1/1] iio: adc: Add driver for the TI INA3221 Triple  Current/Voltage Monitor Guenter Roeck <linux@roeck-us.net> - 2016-04-11 18:30 +0200
    Re: [PATCH 1/1] iio: adc: Add driver for the TI INA3221 Triple  Current/Voltage Monitor "Andrew F. Davis" <afd@ti.com> - 2016-04-11 17:50 +0200
      Re: [PATCH 1/1] iio: adc: Add driver for the TI INA3221 Triple  Current/Voltage Monitor Guenter Roeck <linux@roeck-us.net> - 2016-04-11 18:40 +0200
        Re: [PATCH 1/1] iio: adc: Add driver for the TI INA3221 Triple  Current/Voltage Monitor "Andrew F. Davis" <afd@ti.com> - 2016-04-11 18:50 +0200
          Re: [PATCH 1/1] iio: adc: Add driver for the TI INA3221 Triple  Current/Voltage Monitor Guenter Roeck <linux@roeck-us.net> - 2016-04-11 20:10 +0200
            Re: [PATCH 1/1] iio: adc: Add driver for the TI INA3221 Triple  Current/Voltage Monitor jic23@jic23.retrosnub.co.uk - 2016-04-12 10:40 +0200
              Re: [PATCH 1/1] iio: adc: Add driver for the TI INA3221 Triple  Current/Voltage Monitor jic23@jic23.retrosnub.co.uk - 2016-04-12 11:00 +0200
      Re: [PATCH 1/1] iio: adc: Add driver for the TI INA3221 Triple  Current/Voltage Monitor jic23@jic23.retrosnub.co.uk - 2016-04-12 10:40 +0200
        Re: [PATCH 1/1] iio: adc: Add driver for the TI INA3221 Triple  Current/Voltage Monitor "Andrew F. Davis" <afd@ti.com> - 2016-04-12 18:00 +0200

csiph-web