Thread (6 messages) 6 messages, 4 authors, 2021-05-11

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

From: Krzysztof Kozlowski <hidden>
Date: 2021-05-10 13:02:29
Also in: linux-iio, lkml

On 08/05/2021 11:59, Jonathan Cameron wrote:
On Sat, 8 May 2021 00:44:58 +0200
Peter Rosin [off-list ref] wrote:
quoted
Hi!

On 2021-05-06 17:06, Krzysztof Kozlowski wrote:
quoted
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 <redacted>
---
 .../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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help