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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1da3415507c216d09eb72c30ad915bc139d2ff3d.camel@fi.rohmeurope.com>
Date:   Wed, 19 Feb 2020 08:05:13 +0000
From:   "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>
To:     "robh@...nel.org" <robh@...nel.org>
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

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.

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.
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 :/

> 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
> Documentation/devicetree/bindings/Makefile:12: recipe for target
> 'Documentation/devicetree/bindings/power/supply/rohm,bd9995x.example.
> dts' failed
> make[1]: ***
> [Documentation/devicetree/bindings/power/supply/rohm,bd9995x.example.
> dts] Error 1
> Makefile:1263: recipe for target 'dt_binding_check' failed
> make: *** [dt_binding_check] Error 2
> 
> See https://patchwork.ozlabs.org/patch/1237902
> Please check and re-submit.

I have the RFC v3 almost finished. Hope to find the time to finish and
submit it still today :)

Thanks and regards
	Matti