[v2,3/4] dt-bindings: hwmon: Add ti-max-expected-current-microamp property to ina2xx

Message ID 1507811765-31005-4-git-send-email-m.purski@samsung.com
State Changes Requested
Headers show
Series
  • Make max expected current configurable for ina2xx drivers
Related show

Commit Message

Maciej Purski Oct. 12, 2017, 12:36 p.m.
Add optional max expected current property which allows calibrating
the ina sensor in order to achieve requested measure scale. Document
the changes in Documentation/hwmon/ina2xx.

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 Documentation/devicetree/bindings/hwmon/ina2xx.txt | 4 +++-
 Documentation/hwmon/ina2xx                         | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Krzysztof Kozlowski Oct. 12, 2017, 12:39 p.m. | #1
On Thu, Oct 12, 2017 at 2:36 PM, Maciej Purski <m.purski@samsung.com> wrote:
> Add optional max expected current property which allows calibrating
> the ina sensor in order to achieve requested measure scale. Document
> the changes in Documentation/hwmon/ina2xx.
>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  Documentation/devicetree/bindings/hwmon/ina2xx.txt | 4 +++-
>  Documentation/hwmon/ina2xx                         | 3 +++
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/ina2xx.txt b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
> index 02af0d9..49ef0be 100644
> --- a/Documentation/devicetree/bindings/hwmon/ina2xx.txt
> +++ b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
> @@ -14,11 +14,13 @@ Optional properties:
>
>  - shunt-resistor
>         Shunt resistor value in micro-Ohm
> -
> +- ti-max-expected-current-microamp
> +       Max expected current value in mA
>  Example:
>
>  ina220@44 {
>         compatible = "ti,ina220";
>         reg = <0x44>;
>         shunt-resistor = <1000>;
> +       ti-max-expected-current-microamp = <3000>;
>  };
> diff --git a/Documentation/hwmon/ina2xx b/Documentation/hwmon/ina2xx
> index cfd31d9..30620e8 100644
> --- a/Documentation/hwmon/ina2xx
> +++ b/Documentation/hwmon/ina2xx
> @@ -55,6 +55,9 @@ The shunt value in micro-ohms can be set via platform data or device tree at
>  compile-time or via the shunt_resistor attribute in sysfs at run-time. Please
>  refer to the Documentation/devicetree/bindings/i2c/ina2xx.txt for bindings
>  if the device tree is used.
> +The max expected current value in miliamp can be set via platform data

mili or micro?

BR,
Krzysztof

> +or device tree at compile-time or via currX_max attribute in sysfs
> +at run-time.
>
>  Additionally ina226 supports update_interval attribute as described in
>  Documentation/hwmon/sysfs-interface. Internally the interval is the sum of
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maciej Purski Oct. 12, 2017, 1 p.m. | #2
On 10/12/2017 02:39 PM, Krzysztof Kozlowski wrote:
> On Thu, Oct 12, 2017 at 2:36 PM, Maciej Purski <m.purski@samsung.com> wrote:
>> Add optional max expected current property which allows calibrating
>> the ina sensor in order to achieve requested measure scale. Document
>> the changes in Documentation/hwmon/ina2xx.
>>
>> Signed-off-by: Maciej Purski <m.purski@samsung.com>
>> ---
>>   Documentation/devicetree/bindings/hwmon/ina2xx.txt | 4 +++-
>>   Documentation/hwmon/ina2xx                         | 3 +++
>>   2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/ina2xx.txt b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
>> index 02af0d9..49ef0be 100644
>> --- a/Documentation/devicetree/bindings/hwmon/ina2xx.txt
>> +++ b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
>> @@ -14,11 +14,13 @@ Optional properties:
>>
>>   - shunt-resistor
>>          Shunt resistor value in micro-Ohm
>> -
>> +- ti-max-expected-current-microamp
>> +       Max expected current value in mA
>>   Example:
>>
>>   ina220@44 {
>>          compatible = "ti,ina220";
>>          reg = <0x44>;
>>          shunt-resistor = <1000>;
>> +       ti-max-expected-current-microamp = <3000>;
>>   };
>> diff --git a/Documentation/hwmon/ina2xx b/Documentation/hwmon/ina2xx
>> index cfd31d9..30620e8 100644
>> --- a/Documentation/hwmon/ina2xx
>> +++ b/Documentation/hwmon/ina2xx
>> @@ -55,6 +55,9 @@ The shunt value in micro-ohms can be set via platform data or device tree at
>>   compile-time or via the shunt_resistor attribute in sysfs at run-time. Please
>>   refer to the Documentation/devicetree/bindings/i2c/ina2xx.txt for bindings
>>   if the device tree is used.
>> +The max expected current value in miliamp can be set via platform data
> 
> mili or micro?
> 
> BR,
> Krzysztof

Sorry, these should be mili everywhere. I'll fix this.

> 
>> +or device tree at compile-time or via currX_max attribute in sysfs
>> +at run-time.
>>
>>   Additionally ina226 supports update_interval attribute as described in
>>   Documentation/hwmon/sysfs-interface. Internally the interval is the sum of
>> --
>> 2.7.4
>>
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Guenter Roeck Oct. 12, 2017, 1:55 p.m. | #3
On 10/12/2017 06:00 AM, Maciej Purski wrote:
> 
> 
> On 10/12/2017 02:39 PM, Krzysztof Kozlowski wrote:
>> On Thu, Oct 12, 2017 at 2:36 PM, Maciej Purski <m.purski@samsung.com> wrote:
>>> Add optional max expected current property which allows calibrating
>>> the ina sensor in order to achieve requested measure scale. Document
>>> the changes in Documentation/hwmon/ina2xx.
>>>
>>> Signed-off-by: Maciej Purski <m.purski@samsung.com>
>>> ---
>>>   Documentation/devicetree/bindings/hwmon/ina2xx.txt | 4 +++-
>>>   Documentation/hwmon/ina2xx                         | 3 +++
>>>   2 files changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/ina2xx.txt b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
>>> index 02af0d9..49ef0be 100644
>>> --- a/Documentation/devicetree/bindings/hwmon/ina2xx.txt
>>> +++ b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
>>> @@ -14,11 +14,13 @@ Optional properties:
>>>
>>>   - shunt-resistor
>>>          Shunt resistor value in micro-Ohm
>>> -
>>> +- ti-max-expected-current-microamp
>>> +       Max expected current value in mA
>>>   Example:
>>>
>>>   ina220@44 {
>>>          compatible = "ti,ina220";
>>>          reg = <0x44>;
>>>          shunt-resistor = <1000>;
>>> +       ti-max-expected-current-microamp = <3000>;
>>>   };
>>> diff --git a/Documentation/hwmon/ina2xx b/Documentation/hwmon/ina2xx
>>> index cfd31d9..30620e8 100644
>>> --- a/Documentation/hwmon/ina2xx
>>> +++ b/Documentation/hwmon/ina2xx
>>> @@ -55,6 +55,9 @@ The shunt value in micro-ohms can be set via platform data or device tree at
>>>   compile-time or via the shunt_resistor attribute in sysfs at run-time. Please
>>>   refer to the Documentation/devicetree/bindings/i2c/ina2xx.txt for bindings
>>>   if the device tree is used.
>>> +The max expected current value in miliamp can be set via platform data
>>
>> mili or micro?
>>
>> BR,
>> Krzysztof
> 
> Sorry, these should be mili everywhere. I'll fix this.
> 

You sure ? I think DT usually uses micro.

Guenter

>>
>>> +or device tree at compile-time or via currX_max attribute in sysfs
>>> +at run-time.
>>>
>>>   Additionally ina226 supports update_interval attribute as described in
>>>   Documentation/hwmon/sysfs-interface. Internally the interval is the sum of
>>> -- 
>>> 2.7.4
>>>
>>
>>
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maciej Purski Oct. 12, 2017, 2:13 p.m. | #4
On 10/12/2017 03:55 PM, Guenter Roeck wrote:
> On 10/12/2017 06:00 AM, Maciej Purski wrote:
>>
>>
>> On 10/12/2017 02:39 PM, Krzysztof Kozlowski wrote:
>>> On Thu, Oct 12, 2017 at 2:36 PM, Maciej Purski <m.purski@samsung.com> wrote:
>>>> Add optional max expected current property which allows calibrating
>>>> the ina sensor in order to achieve requested measure scale. Document
>>>> the changes in Documentation/hwmon/ina2xx.
>>>>
>>>> Signed-off-by: Maciej Purski <m.purski@samsung.com>
>>>> ---
>>>>   Documentation/devicetree/bindings/hwmon/ina2xx.txt | 4 +++-
>>>>   Documentation/hwmon/ina2xx                         | 3 +++
>>>>   2 files changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/hwmon/ina2xx.txt 
>>>> b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
>>>> index 02af0d9..49ef0be 100644
>>>> --- a/Documentation/devicetree/bindings/hwmon/ina2xx.txt
>>>> +++ b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
>>>> @@ -14,11 +14,13 @@ Optional properties:
>>>>
>>>>   - shunt-resistor
>>>>          Shunt resistor value in micro-Ohm
>>>> -
>>>> +- ti-max-expected-current-microamp
>>>> +       Max expected current value in mA
>>>>   Example:
>>>>
>>>>   ina220@44 {
>>>>          compatible = "ti,ina220";
>>>>          reg = <0x44>;
>>>>          shunt-resistor = <1000>;
>>>> +       ti-max-expected-current-microamp = <3000>;
>>>>   };
>>>> diff --git a/Documentation/hwmon/ina2xx b/Documentation/hwmon/ina2xx
>>>> index cfd31d9..30620e8 100644
>>>> --- a/Documentation/hwmon/ina2xx
>>>> +++ b/Documentation/hwmon/ina2xx
>>>> @@ -55,6 +55,9 @@ The shunt value in micro-ohms can be set via platform data 
>>>> or device tree at
>>>>   compile-time or via the shunt_resistor attribute in sysfs at run-time. Please
>>>>   refer to the Documentation/devicetree/bindings/i2c/ina2xx.txt for bindings
>>>>   if the device tree is used.
>>>> +The max expected current value in miliamp can be set via platform data
>>>
>>> mili or micro?
>>>
>>> BR,
>>> Krzysztof
>>
>> Sorry, these should be mili everywhere. I'll fix this.
>>
> 
> You sure ? I think DT usually uses micro.
> 
> Guenter

Yeah, you're right. I was intending to make it milli, but I haven't checked that
all DTS use micro. Sorry for confusion.

Best Regards,

Maciej

> 
>>>
>>>> +or device tree at compile-time or via currX_max attribute in sysfs
>>>> +at run-time.
>>>>
>>>>   Additionally ina226 supports update_interval attribute as described in
>>>>   Documentation/hwmon/sysfs-interface. Internally the interval is the sum of
>>>> -- 
>>>> 2.7.4
>>>>
>>>
>>>
>>>
>>
> 
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring Oct. 17, 2017, 8:36 p.m. | #5
On Thu, Oct 12, 2017 at 02:36:04PM +0200, Maciej Purski wrote:
> Add optional max expected current property which allows calibrating
> the ina sensor in order to achieve requested measure scale. Document
> the changes in Documentation/hwmon/ina2xx.
> 
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  Documentation/devicetree/bindings/hwmon/ina2xx.txt | 4 +++-
>  Documentation/hwmon/ina2xx                         | 3 +++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/ina2xx.txt b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
> index 02af0d9..49ef0be 100644
> --- a/Documentation/devicetree/bindings/hwmon/ina2xx.txt
> +++ b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
> @@ -14,11 +14,13 @@ Optional properties:
>  
>  - shunt-resistor
>  	Shunt resistor value in micro-Ohm
> -
> +- ti-max-expected-current-microamp
> +	Max expected current value in mA

ti,max-...

The property name is a bit long. Does "expected" add anything? Is there 
a max unexpected current?

>  Example:
>  
>  ina220@44 {
>  	compatible = "ti,ina220";
>  	reg = <0x44>;
>  	shunt-resistor = <1000>;
> +	ti-max-expected-current-microamp = <3000>;
>  };
> diff --git a/Documentation/hwmon/ina2xx b/Documentation/hwmon/ina2xx
> index cfd31d9..30620e8 100644
> --- a/Documentation/hwmon/ina2xx
> +++ b/Documentation/hwmon/ina2xx
> @@ -55,6 +55,9 @@ The shunt value in micro-ohms can be set via platform data or device tree at
>  compile-time or via the shunt_resistor attribute in sysfs at run-time. Please
>  refer to the Documentation/devicetree/bindings/i2c/ina2xx.txt for bindings
>  if the device tree is used.
> +The max expected current value in miliamp can be set via platform data
> +or device tree at compile-time or via currX_max attribute in sysfs
> +at run-time.
>  
>  Additionally ina226 supports update_interval attribute as described in
>  Documentation/hwmon/sysfs-interface. Internally the interval is the sum of
> -- 
> 2.7.4
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Guenter Roeck Oct. 17, 2017, 8:58 p.m. | #6
On Tue, Oct 17, 2017 at 03:36:31PM -0500, Rob Herring wrote:
> On Thu, Oct 12, 2017 at 02:36:04PM +0200, Maciej Purski wrote:
> > Add optional max expected current property which allows calibrating
> > the ina sensor in order to achieve requested measure scale. Document
> > the changes in Documentation/hwmon/ina2xx.
> > 
> > Signed-off-by: Maciej Purski <m.purski@samsung.com>
> > ---
> >  Documentation/devicetree/bindings/hwmon/ina2xx.txt | 4 +++-
> >  Documentation/hwmon/ina2xx                         | 3 +++
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/hwmon/ina2xx.txt b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
> > index 02af0d9..49ef0be 100644
> > --- a/Documentation/devicetree/bindings/hwmon/ina2xx.txt
> > +++ b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
> > @@ -14,11 +14,13 @@ Optional properties:
> >  
> >  - shunt-resistor
> >  	Shunt resistor value in micro-Ohm
> > -
> > +- ti-max-expected-current-microamp
> > +	Max expected current value in mA
> 
> ti,max-...
> 
> The property name is a bit long. Does "expected" add anything? Is there 
> a max unexpected current?
> 
I am not too happy with it either. To me it suggests that there _can_ be
an unexpected current (why specify a max _expected_ current otherwise ?),
and that unexpected current won't be measurable and thus not reported
because it is ... well, unexpected.

Guenter

> >  Example:
> >  
> >  ina220@44 {
> >  	compatible = "ti,ina220";
> >  	reg = <0x44>;
> >  	shunt-resistor = <1000>;
> > +	ti-max-expected-current-microamp = <3000>;
> >  };
> > diff --git a/Documentation/hwmon/ina2xx b/Documentation/hwmon/ina2xx
> > index cfd31d9..30620e8 100644
> > --- a/Documentation/hwmon/ina2xx
> > +++ b/Documentation/hwmon/ina2xx
> > @@ -55,6 +55,9 @@ The shunt value in micro-ohms can be set via platform data or device tree at
> >  compile-time or via the shunt_resistor attribute in sysfs at run-time. Please
> >  refer to the Documentation/devicetree/bindings/i2c/ina2xx.txt for bindings
> >  if the device tree is used.
> > +The max expected current value in miliamp can be set via platform data
> > +or device tree at compile-time or via currX_max attribute in sysfs
> > +at run-time.
> >  
> >  Additionally ina226 supports update_interval attribute as described in
> >  Documentation/hwmon/sysfs-interface. Internally the interval is the sum of
> > -- 
> > 2.7.4
> > 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jonathan Cameron Oct. 18, 2017, 8:12 a.m. | #7
On Tue, 17 Oct 2017 13:58:21 -0700
Guenter Roeck <linux@roeck-us.net> wrote:

> On Tue, Oct 17, 2017 at 03:36:31PM -0500, Rob Herring wrote:
> > On Thu, Oct 12, 2017 at 02:36:04PM +0200, Maciej Purski wrote:  
> > > Add optional max expected current property which allows calibrating
> > > the ina sensor in order to achieve requested measure scale. Document
> > > the changes in Documentation/hwmon/ina2xx.
> > > 
> > > Signed-off-by: Maciej Purski <m.purski@samsung.com>
> > > ---
> > >  Documentation/devicetree/bindings/hwmon/ina2xx.txt | 4 +++-
> > >  Documentation/hwmon/ina2xx                         | 3 +++
> > >  2 files changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/hwmon/ina2xx.txt b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
> > > index 02af0d9..49ef0be 100644
> > > --- a/Documentation/devicetree/bindings/hwmon/ina2xx.txt
> > > +++ b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
> > > @@ -14,11 +14,13 @@ Optional properties:
> > >  
> > >  - shunt-resistor
> > >  	Shunt resistor value in micro-Ohm
> > > -
> > > +- ti-max-expected-current-microamp
> > > +	Max expected current value in mA  
> > 
> > ti,max-...
> > 
> > The property name is a bit long. Does "expected" add anything? Is there 
> > a max unexpected current?
> >   
> I am not too happy with it either. To me it suggests that there _can_ be
> an unexpected current (why specify a max _expected_ current otherwise ?),
> and that unexpected current won't be measurable and thus not reported
> because it is ... well, unexpected.

After calibration I 'think' this corresponds to the maximum current
that the device can measure (it's applying scaling inside to make full use
of available range of the register - there are some arguments that there is
an optimum value after which we could do better in software).

I guess the issue here is that this that we might need to separate the maximum
current the device is capable of measuring from what it is currently configured
to measure.  No idea how to describe that :)

Jonathan

> 
> Guenter
> 
> > >  Example:
> > >  
> > >  ina220@44 {
> > >  	compatible = "ti,ina220";
> > >  	reg = <0x44>;
> > >  	shunt-resistor = <1000>;
> > > +	ti-max-expected-current-microamp = <3000>;
> > >  };
> > > diff --git a/Documentation/hwmon/ina2xx b/Documentation/hwmon/ina2xx
> > > index cfd31d9..30620e8 100644
> > > --- a/Documentation/hwmon/ina2xx
> > > +++ b/Documentation/hwmon/ina2xx
> > > @@ -55,6 +55,9 @@ The shunt value in micro-ohms can be set via platform data or device tree at
> > >  compile-time or via the shunt_resistor attribute in sysfs at run-time. Please
> > >  refer to the Documentation/devicetree/bindings/i2c/ina2xx.txt for bindings
> > >  if the device tree is used.
> > > +The max expected current value in miliamp can be set via platform data
> > > +or device tree at compile-time or via currX_max attribute in sysfs
> > > +at run-time.
> > >  
> > >  Additionally ina226 supports update_interval attribute as described in
> > >  Documentation/hwmon/sysfs-interface. Internally the interval is the sum of
> > > -- 
> > > 2.7.4
> > >   
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/devicetree/bindings/hwmon/ina2xx.txt b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
index 02af0d9..49ef0be 100644
--- a/Documentation/devicetree/bindings/hwmon/ina2xx.txt
+++ b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
@@ -14,11 +14,13 @@  Optional properties:
 
 - shunt-resistor
 	Shunt resistor value in micro-Ohm
-
+- ti-max-expected-current-microamp
+	Max expected current value in mA
 Example:
 
 ina220@44 {
 	compatible = "ti,ina220";
 	reg = <0x44>;
 	shunt-resistor = <1000>;
+	ti-max-expected-current-microamp = <3000>;
 };
diff --git a/Documentation/hwmon/ina2xx b/Documentation/hwmon/ina2xx
index cfd31d9..30620e8 100644
--- a/Documentation/hwmon/ina2xx
+++ b/Documentation/hwmon/ina2xx
@@ -55,6 +55,9 @@  The shunt value in micro-ohms can be set via platform data or device tree at
 compile-time or via the shunt_resistor attribute in sysfs at run-time. Please
 refer to the Documentation/devicetree/bindings/i2c/ina2xx.txt for bindings
 if the device tree is used.
+The max expected current value in miliamp can be set via platform data
+or device tree at compile-time or via currX_max attribute in sysfs
+at run-time.
 
 Additionally ina226 supports update_interval attribute as described in
 Documentation/hwmon/sysfs-interface. Internally the interval is the sum of