[v1,1/2] dt-bindings: hwmon: ina2xx: add supply property

  Add supply property.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml | 4 ++++
 1 file changed, 4 insertions(+)
  

On 08/03/2023 10:40, Svyatoslav Ryhel wrote:
> Add supply property.

You have entire commit msg to explain and give background, but instead
there is just sentence duplicating subject. And since you did not
explain anything, we have questions... like: INA238 does not have VDD,
so this does not look correct.


Best regards,
Krzysztof
  

ср, 8 бер. 2023 р. о 12:27 Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> пише:
>
> On 08/03/2023 10:40, Svyatoslav Ryhel wrote:
> > Add supply property.
>
> You have entire commit msg to explain and give background, but instead
> there is just sentence duplicating subject. And since you did not
> explain anything, we have questions... like: INA238 does not have VDD,
> so this does not look correct.
>

This is why a regulator is not mandatory. If ina238 does not have vdd
then one who needs ina238 may omit this prop. How about looking from
this perspective?

>
> Best regards,
> Krzysztof
>

Best regards,
Svyatoslav R.
  

On 08/03/2023 11:32, Svyatoslav Ryhel wrote:
> ср, 8 бер. 2023 р. о 12:27 Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> пише:
>>
>> On 08/03/2023 10:40, Svyatoslav Ryhel wrote:
>>> Add supply property.
>>
>> You have entire commit msg to explain and give background, but instead
>> there is just sentence duplicating subject. And since you did not
>> explain anything, we have questions... like: INA238 does not have VDD,
>> so this does not look correct.
>>
> 
> This is why a regulator is not mandatory. If ina238 does not have vdd
> then one who needs ina238 may omit this prop. How about looking from
> this perspective?

Not required means "optional", not "not existing" or "totally invalid".

Best regards,
Krzysztof
  

On 3/8/23 02:32, Svyatoslav Ryhel wrote:
> ср, 8 бер. 2023 р. о 12:27 Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> пише:
>>
>> On 08/03/2023 10:40, Svyatoslav Ryhel wrote:
>>> Add supply property.
>>
>> You have entire commit msg to explain and give background, but instead
>> there is just sentence duplicating subject. And since you did not
>> explain anything, we have questions... like: INA238 does not have VDD,
>> so this does not look correct.
>>
> 
> This is why a regulator is not mandatory. If ina238 does not have vdd
> then one who needs ina238 may omit this prop. How about looking from
> this perspective?
> 

If a chip does not have VDD, the driver should not even try to get it
for that chip. INS238 is supported by a different driver, so that does
not require special code, but it needs to be spelled out in the devicetree
bindings. Devicetree has a means to specify if a property is valid for
a given device.

Having said this, as it turns out, _none_ of the chips supported by
the ina2xx and the ina238 drivers have VDD. All of them have Vs, and
all but INA219 have Vbus. The bindings don't even explain which one
of those is supposed to refer to "VDD".

Also, regulator_get_optional() returns -ENODEV if CONFIG_REGULATOR
is not enabled, so it isn't entirely optional. We can not suddenly fail
to load the driver on systems with CONFIG_REGULATOR=n, so some conditional
code will unfortunately be necessary.

Guenter

>>
>> Best regards,
>> Krzysztof
>>
> 
> Best regards,
> Svyatoslav R.
  

8 березня 2023 р. 13:35:46 GMT+02:00, Guenter Roeck <linux@roeck-us.net> написав(-ла):
>On 3/8/23 02:32, Svyatoslav Ryhel wrote:
>> ср, 8 бер. 2023 р. о 12:27 Krzysztof Kozlowski
>> <krzysztof.kozlowski@linaro.org> пише:
>>> 
>>> On 08/03/2023 10:40, Svyatoslav Ryhel wrote:
>>>> Add supply property.
>>> 
>>> You have entire commit msg to explain and give background, but instead
>>> there is just sentence duplicating subject. And since you did not
>>> explain anything, we have questions... like: INA238 does not have VDD,
>>> so this does not look correct.
>>> 
>> 
>> This is why a regulator is not mandatory. If ina238 does not have vdd
>> then one who needs ina238 may omit this prop. How about looking from
>> this perspective?
>> 
>
>If a chip does not have VDD, the driver should not even try to get it
>for that chip. INS238 is supported by a different driver, so that does
>not require special code, but it needs to be spelled out in the devicetree
>bindings. Devicetree has a means to specify if a property is valid for
>a given device.
>
>Having said this, as it turns out, _none_ of the chips supported by
>the ina2xx and the ina238 drivers have VDD. All of them have Vs, and
>all but INA219 have Vbus. The bindings don't even explain which one
>of those is supposed to refer to "VDD".
>

So you refer to vdd as to a real name. Now I understand this misunderstand. Regulator I am interested in is Vs. Since you confirmed that Vs is supported by all ina2xx there should be no contraversions further.

>Also, regulator_get_optional() returns -ENODEV if CONFIG_REGULATOR
>is not enabled, so it isn't entirely optional. We can not suddenly fail
>to load the driver on systems with CONFIG_REGULATOR=n, so some conditional
>code will unfortunately be necessary.
>
>Guenter
>

Hm, then Lars-Peter Clausen suggestion should be applicable in this case.

>>> 
>>> Best regards,
>>> Krzysztof
>>> 
>> 
>> Best regards,
>> Svyatoslav R.
>
  

On Wed, Mar 08, 2023 at 11:40:23AM +0200, Svyatoslav Ryhel wrote:
> Add supply property.

> +  vdd-supply: true
> +
>  required:
>    - compatible
>    - reg

Unless the device can work without power the supply should be required.
  

8 березня 2023 р. 14:54:34 GMT+02:00, Mark Brown <broonie@kernel.org> написав(-ла):
>On Wed, Mar 08, 2023 at 11:40:23AM +0200, Svyatoslav Ryhel wrote:
>> Add supply property.
>
>> +  vdd-supply: true
>> +
>>  required:
>>    - compatible
>>    - reg
>
>Unless the device can work without power the supply should be required.

Device can work without supply defined on most devices, but in my case power is gated with gpio and devices will not work without fixed regulator.
  

On 08/03/2023 13:58, Svyatoslav Ryhel wrote:
> 
> 
> 8 березня 2023 р. 14:54:34 GMT+02:00, Mark Brown <broonie@kernel.org> написав(-ла):
>> On Wed, Mar 08, 2023 at 11:40:23AM +0200, Svyatoslav Ryhel wrote:
>>> Add supply property.
>>
>>> +  vdd-supply: true
>>> +
>>>  required:
>>>    - compatible
>>>    - reg
>>
>> Unless the device can work without power the supply should be required.
> 
> Device can work without supply defined on most devices,

Are you sure they can work without any power? INA231 does not say VS
supply is optional. Datasheet says:
"The INA231 is typically powered by a separate supply that can range
from 2.7 V to 5.5 V."

Although it uses word "typically" which could suggest other design, but
are you sure it can work without it? From where it gets the power?


>  but in my case power is gated with gpio and devices will not work
without fixed regulator.

BTW, wrap your lines to match mailing list style.

Best regards,
Krzysztof
  

On Wed, Mar 08, 2023 at 02:58:20PM +0200, Svyatoslav Ryhel wrote:
> 8 березня 2023 р. 14:54:34 GMT+02:00, Mark Brown <broonie@kernel.org> написав(-ла):
> >On Wed, Mar 08, 2023 at 11:40:23AM +0200, Svyatoslav Ryhel wrote:
> >> Add supply property.

> >> +  vdd-supply: true
> >> +
> >>  required:
> >>    - compatible
> >>    - reg

> >Unless the device can work without power the supply should be required.

> Device can work without supply defined on most devices, but in my case power is gated with gpio and devices will not work without fixed regulator.

If there are devices that work without any source of power at all that
would be very surprising.  It doesn't matter if a particular system has
a non-controllable regulator, the binding should still make it mandatory
to describe that.
  

8 березня 2023 р. 15:46:52 GMT+02:00, Mark Brown <broonie@kernel.org> написав(-ла):
>On Wed, Mar 08, 2023 at 02:58:20PM +0200, Svyatoslav Ryhel wrote:
>> 8 березня 2023 р. 14:54:34 GMT+02:00, Mark Brown <broonie@kernel.org> написав(-ла):
>> >On Wed, Mar 08, 2023 at 11:40:23AM +0200, Svyatoslav Ryhel wrote:
>> >> Add supply property.
>
>> >> +  vdd-supply: true
>> >> +
>> >>  required:
>> >>    - compatible
>> >>    - reg
>
>> >Unless the device can work without power the supply should be required.
>
>> Device can work without supply defined on most devices, but in my case power is gated with gpio and devices will not work without fixed regulator.
>
>If there are devices that work without any source of power at all that
>would be very surprising.  It doesn't matter if a particular system has
>a non-controllable regulator, the binding should still make it mandatory
>to describe that.

Then question is WHY and WHO passed driver without power supply system implemented? Why it pops only now?
  

On 08/03/2023 15:01, Svyatoslav Ryhel wrote:
>>>>> +  vdd-supply: true
>>>>> +
>>>>>  required:
>>>>>    - compatible
>>>>>    - reg
>>
>>>> Unless the device can work without power the supply should be required.
>>
>>> Device can work without supply defined on most devices, but in my case power is gated with gpio and devices will not work without fixed regulator.
>>
>> If there are devices that work without any source of power at all that
>> would be very surprising.  It doesn't matter if a particular system has
>> a non-controllable regulator, the binding should still make it mandatory
>> to describe that.
> 
> Then question is WHY and WHO passed driver without power supply system implemented? Why it pops only now?

Why do you mention driver? We talk about bindings and device. What the
driver does, matters less here.

Best regards,
Krzysztof
  

On Wed, Mar 08, 2023 at 04:01:44PM +0200, Svyatoslav Ryhel wrote:
> 8 березня 2023 р. 15:46:52 GMT+02:00, Mark Brown <broonie@kernel.org> написав(-ла):

> >If there are devices that work without any source of power at all that
> >would be very surprising.  It doesn't matter if a particular system has
> >a non-controllable regulator, the binding should still make it mandatory
> >to describe that.

> Then question is WHY and WHO passed driver without power supply system implemented? Why it pops only now?

You are defining a supply property.  When you define a supply property
that supply property should be mandatory if it's physically mandatory
for the device.

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.