Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] dt-bindings: iio: afe: current-sense-shunt: add
 io-channel-cells
To:     Jonathan Cameron <jic23@kernel.org>, Peter Rosin <peda@axentia.se>
Cc:     Lars-Peter Clausen <lars@metafoo.de>,
        Rob Herring <robh+dt@kernel.org>, linux-iio@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20210506150637.35288-1-krzysztof.kozlowski@canonical.com>
 <0e68ca18-7d8c-12ab-59b1-56404b29be77@axentia.se>
 <20210508165944.2e3d8d91@jic23-huawei>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Message-ID: <1e8651a3-e730-411b-18a8-800e9bd9304e@canonical.com>
Date:   Mon, 10 May 2021 08:17:17 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.1
MIME-Version: 1.0
In-Reply-To: <20210508165944.2e3d8d91@jic23-huawei>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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