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

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

From: Jonathan Cameron <jic23@kernel.org>
Date: 2021-05-11 14:18:45
Also in: linux-iio, lkml

On Mon, 10 May 2021 08:17:17 -0400
Krzysztof Kozlowski [off-list ref] wrote:
On 08/05/2021 11:59, Jonathan Cameron wrote:
quoted
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!
I dropped the required and applied to the togreg branch of iio.git.

Thanks,

Jonathan
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