[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL_Jsq+x+5RCT0L5HGocdSOuve2qm5JvqSYXAwDbJ4qP9wr9TA@mail.gmail.com>
Date: Thu, 20 Feb 2020 14:18:17 -0600
From: Rob Herring <robh@...nel.org>
To: "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>
Cc: "mazziesaccount@...il.com" <mazziesaccount@...il.com>,
"mark.rutland@....com" <mark.rutland@....com>,
"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
"broonie@...nel.org" <broonie@...nel.org>,
"Mutanen, Mikko" <Mikko.Mutanen@...rohmeurope.com>,
"sre@...nel.org" <sre@...nel.org>,
"Laine, Markus" <Markus.Laine@...rohmeurope.com>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [RFC PATCH v2 2/5] dt_bindings: ROHM BD99954 Charger
On Wed, Feb 19, 2020 at 2:05 AM Vaittinen, Matti
<Matti.Vaittinen@...rohmeurope.com> 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@...rohmeurope.com>
> > > ---
> > >
> > > It would probably be nice if the charger DT binding yaml could
> > > somehow
> > > be listing and evaluating properties that it can use from static
> > > battery
> > > nodes - and perhaps some warning could be emitted if unsupported
> > > properties are given from battery nodes(?) Just some thinking here.
> > > What if the charger ignores for example the current limits from
> > > battery
> > > node (I am not sure but I think a few may ignore) - I guess it
> > > would be
> > > nice to give a nudge to a person who added those properties in his
> > > DT
> > > if they won't have any impact? Any thoughts?
> > >
> > > .../bindings/power/supply/rohm,bd9995x.yaml | 167
> > > ++++++++++++++++++
> > > 1 file changed, 167 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/power/supply/rohm,bd9995x.yaml
> > >
> >
> > My bot found errors running 'make dt_binding_check' on your patch:
>
> Ouch ... sorry. There is some leftover block from another text based
> binding document which I used as an example while writing out the
> battery parameters BD99954 uses.
>
> There's two other hiccups when I try running make dt_binding_check. I
> assume they are false positives.
>
> <SIDE NOTE>
> Although... Back in my Nokia days I joined in a trainer-training. I had
> excellent British coach - Graham if I remember correctly - who told us
> never to assume. He explained where word ass-u-me comes from. I can
> still hear his very British accent: "It makes an ass out of u and me".
> I still do so though. I'm not learning easily it seems.
> </SIDE NOTE>
>
> 1. It seems to me the multipleOf: is not recognized. I guess it
> should(?) I will comment it out in v3 though until I get confirmation
> it should work.
Yes, it should work. I reworked allowed keywords recently. Is dtschema
up to date?
>
> 2. schema validation for:
>
> rohm,vsys-regulation-microvolt:
> description: system specific lower limit for system voltage.
> minimum: 2560000
> maximum: 19200000
> #multipleOf: 64000
>
> fails. But when I change this to
> rohm,vsys-regulation-microvolts: (plural)
> it seems to be passing the validation. A git grep under
> Documentation/devicetree/bindings reveals that both plural and singular
> are used - but the singular seems to be far more popular than plural.
Only in one case that got it wrong.
> 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.
> > 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,bd9995x.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.
Rob
Powered by blists - more mailing lists