linux-kernel - Re: [RFC 1/4] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors

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]

Date:	Thu, 12 Nov 2015 10:25:54 +0100
From:	Marc Titinger <mtitinger@...libre.com>
To:	Daniel Baluta <daniel.baluta@...il.com>
Cc:	Jonathan Cameron <jic23@...nel.org>,
	Hartmut Knaack <knaack.h@....de>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Peter Meerwald <pmeerw@...erw.net>,
	"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	mturquette@...libre.com, bcousson@...libre.com,
	ptitiano@...libre.com
Subject: Re: [RFC 1/4] iio: ina2xx: add direct IO support for TI INA2xx Power
 Monitors

On 11/11/2015 11:14, Daniel Baluta wrote:
> On Tue, Nov 10, 2015 at 6:07 PM, Marc Titinger <mtitinger@...libre.com> wrote:
>> Basic support or direct IO raw read, with averaging attribute.
>> Values are RAW, INT_PLUS_MICRO (Volt/Ampere/Watt).
>>

Hi Daniel,

thanks for the review,
comments bellow,

Marc.


>> IIO context has 1 devices:
> Is this from libiio right? Perhaps you should specify this:
>
> "libiio context has 1 devices"

It's the raw output from iio_info to give a quick overview of the features.

>
>>       iio:device0: ina226
>>        4 channels found:
>>          power3:  (input)
>>          1 channel-specific attributes found:
>>                  attr 0: raw value: 1.150000
>>          voltage0:  (input)
>>          1 channel-specific attributes found:
>>                  attr 0: raw value: 0.000003
>>          voltage1:  (input)
>>          1 channel-specific attributes found:
>>                  attr 0: raw value: 4.277500
>>          current2:  (input)
>>          1 channel-specific attributes found:
>>                  attr 0: raw value: 0.268000
>>        2 device-specific attributes found:
>>                  attr 0: in_calibscale value: 10000
>>                  attr 1: in_mean_raw value: 4
>>
>>
>
> Link to datasheet?

Ok, will add it.

http://www.ti.com/lit/gpn/ina226


...


>>
>> +config INA2XX_IIO
>> +        tristate "Texas Instruments INA2xx Power Monitors IIO driver"
>> +       depends on I2C
>
> Since you are using regmap you should select it here.

Right.

>
>> +        help
>> +          Say yes here to build support for TI INA2xx familly Power Monitors.
>> +
>> +         Note that this is not the existing hwmon interface for this device.
>> +
>> +
>
> Config symbol should be in alphabetical order.

Ok
>
...


>>   obj-$(CONFIG_AD7266) += ad7266.o
>> +obj-$(CONFIG_INA2XX_IIO) += ina2xx-iio.o
>
> The same here.

Ok.

>
>>   obj-$(CONFIG_AD7291) += ad7291.o
>>   obj-$(CONFIG_AD7298) += ad7298.o
>>   obj-$(CONFIG_AD7923) += ad7923.o
>> diff --git a/drivers/iio/adc/ina2xx-iio.c b/drivers/iio/adc/ina2xx-iio.c
>> new file mode 100644
>> index 0000000..257d8d5
>> --- /dev/null
>> +++ b/drivers/iio/adc/ina2xx-iio.c
>> @@ -0,0 +1,404 @@
>> +/*
>> + * INA2XX Current and Power Monitors
>> + *

...

>> + *
>> + * Licensed under the GPL-2 or later.
>
> If you know the I2C address its recommended to mention it here.

ok.

...

>> +
>> +static struct regmap_config ina2xx_regmap_config = {
>> +       .reg_bits = 8,
>> +       .val_bits = 16,
> Specify here which registers are cacheable, read/write.

ok.

...

>> +
>> +       return iio_device_register(indio_dev);
>
> If there is no reverse operation for ina2xx_init (e.g ina2xx_poweroff) then here
> you can use devm_iio_device_register and completely remove
> ina2xx_remove function.

right, thanks!


--
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/