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

2017-10-18 Thread Jonathan Cameron 
On Tue, 17 Oct 2017 13:58:21 -0700
Guenter Roeck  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 
> > > ---
> > >  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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

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

2017-10-17 Thread Guenter Roeck 
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 
> > ---
> >  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 linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2017-10-17 Thread Rob Herring 
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 
> ---
>  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 linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2017-10-12 Thread Maciej Purski 



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  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 
---
  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 linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2017-10-12 Thread Guenter Roeck 

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  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 
---
  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 linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2017-10-12 Thread Maciej Purski 



On 10/12/2017 02:39 PM, Krzysztof Kozlowski wrote:

On Thu, Oct 12, 2017 at 2:36 PM, 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 
---
  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 linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2017-10-12 Thread Krzysztof Kozlowski 
On Thu, Oct 12, 2017 at 2:36 PM, 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 
> ---
>  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 linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2017-10-12 Thread Maciej Purski 
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 
---
 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
+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-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html