linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>
To: "robh@kernel.org" <robh@kernel.org>
Cc: "mazziesaccount@gmail.com" <mazziesaccount@gmail.com>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"linus.walleij@linaro.org" <linus.walleij@linaro.org>,
	"broonie@kernel.org" <broonie@kernel.org>,
	"Mutanen, Mikko" <Mikko.Mutanen@fi.rohmeurope.com>,
	"sre@kernel.org" <sre@kernel.org>,
	"Laine, Markus" <Markus.Laine@fi.rohmeurope.com>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [RFC PATCH v2 2/5] dt_bindings: ROHM BD99954 Charger
Date: Wed, 19 Feb 2020 08:05:13 +0000	[thread overview]
Message-ID: <1da3415507c216d09eb72c30ad915bc139d2ff3d.camel@fi.rohmeurope.com> (raw)
In-Reply-To: <20200218202122.GA599@bogus>

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@fi.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


  reply	other threads:[~2020-02-19  8:05 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-14  7:29 [RFC PATCH v2 0/5] Support ROHM BD99954 charger IC Matti Vaittinen
2020-02-14  7:30 ` [RFC PATCH v2 1/5] dt-bindings: battry: add new battery parameters Matti Vaittinen
2020-02-19 19:57   ` Rob Herring
2020-02-20  6:39     ` Vaittinen, Matti
2020-02-14  7:36 ` [RFC PATCH v2 2/5] dt_bindings: ROHM BD99954 Charger Matti Vaittinen
2020-02-18 20:21   ` Rob Herring
2020-02-19  8:05     ` Vaittinen, Matti [this message]
2020-02-20 20:18       ` Rob Herring
2020-02-21  8:52         ` Vaittinen, Matti
2020-02-14  7:37 ` [RFC PATCH v2 3/5] power: Add linear_range helper Matti Vaittinen
2020-02-21 13:49   ` Linus Walleij
2020-02-22  8:33     ` Vaittinen, Matti
2020-02-14  7:38 ` [RFC PATCH v2 4/5] power: supply: add battery parameters Matti Vaittinen
2020-02-21 13:50   ` Linus Walleij
2020-02-14  7:39 ` [RFC PATCH v2 5/5] power: supply: Support ROHM bd99954 charger Matti Vaittinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1da3415507c216d09eb72c30ad915bc139d2ff3d.camel@fi.rohmeurope.com \
    --to=matti.vaittinen@fi.rohmeurope.com \
    --cc=Markus.Laine@fi.rohmeurope.com \
    --cc=Mikko.Mutanen@fi.rohmeurope.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mazziesaccount@gmail.com \
    --cc=robh@kernel.org \
    --cc=sre@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).