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.
Guenter
