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

Groups > linux.kernel > #1376161

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

Show key headers only | View raw

On 04/11/2016 11:38 AM, Guenter Roeck wrote:
> 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.
> 

Hmm, my plan was an MFD device core, that then could mediate the two
sub-drivers, but I'm not sure about this yet.

> 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).
> 

I agree. I think both frameworks offer useful interfaces, so I would
hate to limit users to one, but for now I'll just post the hwmon version
here in a bit and live with that until someone requests the IIO support.

Andrew

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