Re: [PATCH V2 1/4] dt-bindings: hwmon: ina3221: Convert to json-schema

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 01/09/2023 17:44, Guenter Roeck wrote:
On Fri, Sep 01, 2023 at 05:34:00PM +0100, Jon Hunter wrote:

On 26/08/2023 09:53, Krzysztof Kozlowski wrote:
On 25/08/2023 18:42, Ninad Malwade wrote:
Convert the TI INA3221 bindings from the free-form text format to
json-schema.

Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
Signed-off-by: Ninad Malwade <nmalwade@xxxxxxxxxx>

...

+            compatible = "ti,ina3221";
+            reg = <0x40>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            input@0 {
+                reg = <0x0>;
+                status = "disabled";

Why is this node present? Binding said nodes are optional, so I assume
it can be just skipped. If all children must be there, then you should
actually require them in the binding (and mention it briefly in commit msg).


I have taken a look at this and if the 'input@0' is omitted above the driver
still enables it. It only disables it or marks as disconnected if the node
is present but no enabled. So we can mark these as required.

Is there a better way to mark them as required apart from listing all input
channels under required?


Channels should by default be enabled because they are enabled by default
in the chip. Requiring that all nodes be listed in devicetree just to
select the default behavior would be overkill.

Jusat because the chip has the ability to disable channels, that should
really not be made the default.


Yes and that is fine. What we are trying to sort out here is what should the example contain in the dt-binding doc? The original example from the text binding doc was replicated to the yaml version and based upon the above that seems like a valid example.

What I could do is add an extra comment under the 'input' property that "Input channels default to enabled in the chip. Unless channels are explicitly disabled in device-tree, input channels will be enabled".

Jon

--
nvpublic



[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux