The current-sense-shunt is an IIO provider thus can be referenced by IIO
consumers (via "io-channels" property in consumer device node).
Such provider is required to describe number of cells used in phandle
lookup with "io-channel-cells" property. This also fixes dtbs_check
warnings like:
arch/arm/boot/dts/s5pv210-fascinate4g.dt.yaml: current-sense-shunt:
'#io-channel-cells' does not match any of the regexes: 'pinctrl-[0-9]+'
Fixes: ce66e52b6c16 ("dt-bindings:iio:afe:current-sense-shunt: txt to yaml conversion.")
Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
.../devicetree/bindings/iio/afe/current-sense-shunt.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml b/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
index 90439a8dc785..05166d8a3124 100644
--- a/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
+++ b/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
@@ -24,12 +24,16 @@ properties:
description: |
Channel node of a voltage io-channel.
+ "#io-channel-cells":
+ const: 0
+
shunt-resistor-micro-ohms:
description: The shunt resistance.
required:
- compatible
- io-channels
+ - "#io-channel-cells"
- shunt-resistor-micro-ohms
additionalProperties: false
@@ -57,6 +61,7 @@ examples:
sysi {
compatible = "current-sense-shunt";
io-channels = <&tiadc 0>;
+ #io-channel-cells = <0>;
/* Divide the voltage by 3300000/1000000 (or 3.3) for the current. */
shunt-resistor-micro-ohms = <3300000>;
--
2.25.1
On Thu, 06 May 2021 11:06:37 -0400, Krzysztof Kozlowski wrote:
> The current-sense-shunt is an IIO provider thus can be referenced by IIO
> consumers (via "io-channels" property in consumer device node).
> Such provider is required to describe number of cells used in phandle
> lookup with "io-channel-cells" property. This also fixes dtbs_check
> warnings like:
>
> arch/arm/boot/dts/s5pv210-fascinate4g.dt.yaml: current-sense-shunt:
> '#io-channel-cells' does not match any of the regexes: 'pinctrl-[0-9]+'
>
> Fixes: ce66e52b6c16 ("dt-bindings:iio:afe:current-sense-shunt: txt to yaml conversion.")
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> .../devicetree/bindings/iio/afe/current-sense-shunt.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
Reviewed-by: Rob Herring <[email protected]>
Hi!
On 2021-05-06 17:06, Krzysztof Kozlowski wrote:
> The current-sense-shunt is an IIO provider thus can be referenced by IIO
> consumers (via "io-channels" property in consumer device node).
> Such provider is required to describe number of cells used in phandle
> lookup with "io-channel-cells" property. This also fixes dtbs_check
> warnings like:
>
> arch/arm/boot/dts/s5pv210-fascinate4g.dt.yaml: current-sense-shunt:
> '#io-channel-cells' does not match any of the regexes: 'pinctrl-[0-9]+'
>
> Fixes: ce66e52b6c16 ("dt-bindings:iio:afe:current-sense-shunt: txt to yaml conversion.")
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> .../devicetree/bindings/iio/afe/current-sense-shunt.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml b/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> index 90439a8dc785..05166d8a3124 100644
> --- a/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> +++ b/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> @@ -24,12 +24,16 @@ properties:
> description: |
> Channel node of a voltage io-channel.
>
> + "#io-channel-cells":
> + const: 0
> +
> shunt-resistor-micro-ohms:
> description: The shunt resistance.
>
> required:
> - compatible
> - io-channels
> + - "#io-channel-cells"
> - shunt-resistor-micro-ohms
I know I'm listed as maintainer and all, but I have not kept up with the yaml
conversion. Sorry. So, given that I might very well fundamentally misunderstand
something, it does not sound correct that #io-channel-cells is now "required".
I regard it as optional, and only needed if some other in-kernel driver is
consuming the sensed current. What am I missing?
Also, whatever is done in this binding should preferably also be done in the
two "sister" afe bindings, i.e. current-sense-amplifier and voltage-divider.
Cheers,
Peter
> additionalProperties: false
> @@ -57,6 +61,7 @@ examples:
> sysi {
> compatible = "current-sense-shunt";
> io-channels = <&tiadc 0>;
> + #io-channel-cells = <0>;
>
> /* Divide the voltage by 3300000/1000000 (or 3.3) for the current. */
> shunt-resistor-micro-ohms = <3300000>;
>
On Sat, 8 May 2021 00:44:58 +0200
Peter Rosin <[email protected]> wrote:
> Hi!
>
> On 2021-05-06 17:06, Krzysztof Kozlowski wrote:
> > The current-sense-shunt is an IIO provider thus can be referenced by IIO
> > consumers (via "io-channels" property in consumer device node).
> > Such provider is required to describe number of cells used in phandle
> > lookup with "io-channel-cells" property. This also fixes dtbs_check
> > warnings like:
> >
> > arch/arm/boot/dts/s5pv210-fascinate4g.dt.yaml: current-sense-shunt:
> > '#io-channel-cells' does not match any of the regexes: 'pinctrl-[0-9]+'
> >
> > Fixes: ce66e52b6c16 ("dt-bindings:iio:afe:current-sense-shunt: txt to yaml conversion.")
> > Signed-off-by: Krzysztof Kozlowski <[email protected]>
> > ---
> > .../devicetree/bindings/iio/afe/current-sense-shunt.yaml | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml b/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> > index 90439a8dc785..05166d8a3124 100644
> > --- a/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> > +++ b/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> > @@ -24,12 +24,16 @@ properties:
> > description: |
> > Channel node of a voltage io-channel.
> >
> > + "#io-channel-cells":
> > + const: 0
> > +
> > shunt-resistor-micro-ohms:
> > description: The shunt resistance.
> >
> > required:
> > - compatible
> > - io-channels
> > + - "#io-channel-cells"
> > - shunt-resistor-micro-ohms
>
> I know I'm listed as maintainer and all, but I have not kept up with the yaml
> conversion. Sorry. So, given that I might very well fundamentally misunderstand
> something, it does not sound correct that #io-channel-cells is now "required".
> I regard it as optional, and only needed if some other in-kernel driver is
> consuming the sensed current. What am I missing?
>
Agreed. This should be optional and I have deliberately not introduced it
into all the bindings that could in theory support being used as providers.
So far I've not pushed it out in a blanket fashion into existing bindings
even as optional.
> Also, whatever is done in this binding should preferably also be done in the
> two "sister" afe bindings, i.e. current-sense-amplifier and voltage-divider.
This particular case is squashing an error, so whilst I'm happy to have those
gain the binding addition, I would like to see them in a separate patch as
less likely they'd get back ported.
If Kryysztof is fine with me just dropping the required I can pick up this patch.
Thanks,
Jonathan
>
> Cheers,
> Peter
>
> > additionalProperties: false
> > @@ -57,6 +61,7 @@ examples:
> > sysi {
> > compatible = "current-sense-shunt";
> > io-channels = <&tiadc 0>;
> > + #io-channel-cells = <0>;
> >
> > /* Divide the voltage by 3300000/1000000 (or 3.3) for the current. */
> > shunt-resistor-micro-ohms = <3300000>;
> >
On 08/05/2021 11:59, Jonathan Cameron wrote:
> On Sat, 8 May 2021 00:44:58 +0200
> Peter Rosin <[email protected]> wrote:
>
>> Hi!
>>
>> On 2021-05-06 17:06, Krzysztof Kozlowski wrote:
>>> The current-sense-shunt is an IIO provider thus can be referenced by IIO
>>> consumers (via "io-channels" property in consumer device node).
>>> Such provider is required to describe number of cells used in phandle
>>> lookup with "io-channel-cells" property. This also fixes dtbs_check
>>> warnings like:
>>>
>>> arch/arm/boot/dts/s5pv210-fascinate4g.dt.yaml: current-sense-shunt:
>>> '#io-channel-cells' does not match any of the regexes: 'pinctrl-[0-9]+'
>>>
>>> Fixes: ce66e52b6c16 ("dt-bindings:iio:afe:current-sense-shunt: txt to yaml conversion.")
>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>> ---
>>> .../devicetree/bindings/iio/afe/current-sense-shunt.yaml | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml b/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
>>> index 90439a8dc785..05166d8a3124 100644
>>> --- a/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
>>> +++ b/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
>>> @@ -24,12 +24,16 @@ properties:
>>> description: |
>>> Channel node of a voltage io-channel.
>>>
>>> + "#io-channel-cells":
>>> + const: 0
>>> +
>>> shunt-resistor-micro-ohms:
>>> description: The shunt resistance.
>>>
>>> required:
>>> - compatible
>>> - io-channels
>>> + - "#io-channel-cells"
>>> - shunt-resistor-micro-ohms
>>
>> I know I'm listed as maintainer and all, but I have not kept up with the yaml
>> conversion. Sorry. So, given that I might very well fundamentally misunderstand
>> something, it does not sound correct that #io-channel-cells is now "required".
>> I regard it as optional, and only needed if some other in-kernel driver is
>> consuming the sensed current. What am I missing?
>>
>
> Agreed. This should be optional and I have deliberately not introduced it
> into all the bindings that could in theory support being used as providers.
>
> So far I've not pushed it out in a blanket fashion into existing bindings
> even as optional.
>
>> Also, whatever is done in this binding should preferably also be done in the
>> two "sister" afe bindings, i.e. current-sense-amplifier and voltage-divider.
>
> This particular case is squashing an error, so whilst I'm happy to have those
> gain the binding addition, I would like to see them in a separate patch as
> less likely they'd get back ported.
>
> If Kryysztof is fine with me just dropping the required I can pick up this patch.
Having here required number of cells helps any DT-user to seamlessly
integrate with it (e.g. with his in-tree or out-of-tree DTS, with
overlays). However it also can be added with such DTS or overlay, so in
general I don't mind dropping the required piece. Thanks!
Best regards,
Krzysztof
On Mon, 10 May 2021 08:17:17 -0400
Krzysztof Kozlowski <[email protected]> wrote:
> On 08/05/2021 11:59, Jonathan Cameron wrote:
> > On Sat, 8 May 2021 00:44:58 +0200
> > Peter Rosin <[email protected]> wrote:
> >
> >> Hi!
> >>
> >> On 2021-05-06 17:06, Krzysztof Kozlowski wrote:
> >>> The current-sense-shunt is an IIO provider thus can be referenced by IIO
> >>> consumers (via "io-channels" property in consumer device node).
> >>> Such provider is required to describe number of cells used in phandle
> >>> lookup with "io-channel-cells" property. This also fixes dtbs_check
> >>> warnings like:
> >>>
> >>> arch/arm/boot/dts/s5pv210-fascinate4g.dt.yaml: current-sense-shunt:
> >>> '#io-channel-cells' does not match any of the regexes: 'pinctrl-[0-9]+'
> >>>
> >>> Fixes: ce66e52b6c16 ("dt-bindings:iio:afe:current-sense-shunt: txt to yaml conversion.")
> >>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> >>> ---
> >>> .../devicetree/bindings/iio/afe/current-sense-shunt.yaml | 5 +++++
> >>> 1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml b/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> >>> index 90439a8dc785..05166d8a3124 100644
> >>> --- a/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> >>> +++ b/Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> >>> @@ -24,12 +24,16 @@ properties:
> >>> description: |
> >>> Channel node of a voltage io-channel.
> >>>
> >>> + "#io-channel-cells":
> >>> + const: 0
> >>> +
> >>> shunt-resistor-micro-ohms:
> >>> description: The shunt resistance.
> >>>
> >>> required:
> >>> - compatible
> >>> - io-channels
> >>> + - "#io-channel-cells"
> >>> - shunt-resistor-micro-ohms
> >>
> >> I know I'm listed as maintainer and all, but I have not kept up with the yaml
> >> conversion. Sorry. So, given that I might very well fundamentally misunderstand
> >> something, it does not sound correct that #io-channel-cells is now "required".
> >> I regard it as optional, and only needed if some other in-kernel driver is
> >> consuming the sensed current. What am I missing?
> >>
> >
> > Agreed. This should be optional and I have deliberately not introduced it
> > into all the bindings that could in theory support being used as providers.
> >
> > So far I've not pushed it out in a blanket fashion into existing bindings
> > even as optional.
> >
> >> Also, whatever is done in this binding should preferably also be done in the
> >> two "sister" afe bindings, i.e. current-sense-amplifier and voltage-divider.
> >
> > This particular case is squashing an error, so whilst I'm happy to have those
> > gain the binding addition, I would like to see them in a separate patch as
> > less likely they'd get back ported.
> >
> > If Kryysztof is fine with me just dropping the required I can pick up this patch.
>
> Having here required number of cells helps any DT-user to seamlessly
> integrate with it (e.g. with his in-tree or out-of-tree DTS, with
> overlays). However it also can be added with such DTS or overlay, so in
> general I don't mind dropping the required piece. Thanks!
I dropped the required and applied to the togreg branch of iio.git.
Thanks,
Jonathan
>
> Best regards,
> Krzysztof