Re: [PATCH] dt-bindings: iio: afe: current-sense-shunt: add io-channel-cells

[Jonathan Cameron]

On Mon, 10 May 2021 08:17:17 -0400
Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxx> wrote:

> On 08/05/2021 11:59, Jonathan Cameron wrote:
> > On Sat, 8 May 2021 00:44:58 +0200
> > Peter Rosin <peda@xxxxxxxxxx> 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 <krzysztof.kozlowski@xxxxxxxxxxxxx>
> >>> ---
> >>>  .../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