Re: [RFC PATCH v2 2/5] dt_bindings: ROHM BD99954 Charger

[Vaittinen, Matti]

Morning again Rob,

On Thu, 2020-02-20 at 14:18 -0600, Rob Herring wrote:
> On Wed, Feb 19, 2020 at 2:05 AM Vaittinen, Matti
> <Matti.Vaittinen@xxxxxxxxxxxxxxxxx> wrote:
> > Morning Rob,
> > 
> > On Tue, 2020-02-18 at 14:21 -0600, Rob Herring wrote:
> > > On Fri, 14 Feb 2020 09:36:47 +0200, Matti Vaittinen wrote:
> > > > The ROHM BD99954 is a Battery Management LSI for 1-4 cell
> > > > Lithium-
> > > > Ion
> > > > secondary battery. Intended to be used in space-constraint
> > > > equipment such
> > > > as Low profile Notebook PC, Tablets and other applications.
> > > > BD99954
> > > > provides a Dual-source Battery Charger, two port BC1.2
> > > > detection
> > > > and a
> > > > Battery Monitor.
> > > > 
> > > > Document the DT bindings for BD99954
> > > > 
> > > > Signed-off-by: Matti Vaittinen <
> > > > matti.vaittinen@xxxxxxxxxxxxxxxxx>
> > > > ---
> > > > 
> > It also looks like the 'core bindings' like regulators use
> > singular.
> > Hence I'll leave this to singular for v3 even though it fails the
> > validation - please let me know if this was wrong choice or if you
> > spot
> > any other oddities there. I can't see what else it could be but for
> > some reason I still find this yaml terribly hard :(
> > 
> > Hmm.. I wonder if I have some old checker tools installed and used
> > on
> > my PC? I also get validation failures for the example schemas :/
> 
> You do have to stay up to date.

Uh. So it seems.
I updated the multipleOf was recognized now. I also got nice warning
about missing #size-cells and #address-cells for the i2c bus. It kind
of is out of the scope of the charger binding (which should just sit in
the bus) but the warning was clear enough so I understood what it was
about. Nice job, thanks.

I just wonder if it would be big task to add version query (like dt-
doc-validate --version) to the toolings and include "required version"
in kernel makefiles so that the 'dt_binding_check' make target could
warn if one uses too old version? (Ignorant c-coders like me may not
know the dt-schema tooling is not included in kernel scripts and needs
to be updated on host PC.) Just a suggestion which might reduce errors
before patch submissions.

> > > warning: no schema found in file:
> > > Documentation/devicetree/bindings/power/supply/rohm,bd9995x.yaml
> > > /builds/robherring/linux-dt-
> > > review/Documentation/devicetree/bindings/power/supply/rohm,bd9995
> > > x.ya
> > > ml: ignoring, error parsing file
> > > Documentation/devicetree/bindings/display/simple-
> > > framebuffer.example.dts:21.16-37.11: Warning
> > > (chosen_node_is_root):
> > > /example-0/chosen: chosen node must be at root node
> > > Documentation/devicetree/bindings/power/supply/rohm,bd9995x.yaml:
> > >   wh
> > > ile scanning a simple key
> > >   in "<unicode string>", line 29, column 3
> > > could not find expected ':'
> > >   in "<unicode string>", line 30, column 1
> 
> Though this is just incorrect YAML and the tool version shouldn't
> matter.

Yes. That should be fixed now. And v4 will also have the multipleOf
uncommented and #size-cells and #address-cells for the i2c bus added.
I'll wait for a while for others to give feedback from the series
before sending it out though. Thanks for the help!

--Matti