Re: [RFC PATCH v2 02/13] dt-bindings: mfd: Document ROHM BD71828 bindings

["Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>]

On Wed, 2019-10-30 at 14:22 -0500, Rob Herring wrote:
> On Wed, Oct 30, 2019 at 3:27 AM Vaittinen, Matti
> <Matti.Vaittinen@fi.rohmeurope.com> wrote:
> > 
> > On Tue, 2019-10-29 at 14:34 -0500, Rob Herring wrote:
> > > On Fri, Oct 25, 2019 at 05:49:17AM +0000, Vaittinen, Matti wrote:
> > > > Hello Dan,
> > > > 
> > > > Thanks again for checking this :)
> > > > 
> > > > On Thu, 2019-10-24 at 14:35 -0500, Dan Murphy wrote:
> > > > > Matti
> > > > > 
> > > > > On 10/24/19 6:41 AM, Matti Vaittinen wrote:
> > > > > > ROHM BD71828 Power management IC integrates 7 buck
> > > > > > converters,
> > > > > > 7
> > > > > > LDOs,
> > > > > > a real-time clock (RTC), 3 GPO/regulator control pins, HALL
> > > > > > input
> > > > > > and a 32.768 kHz clock gate.
> > > > > > 
> > > > > > Document the dt bindings drivers are using.
> > > > > > 
> > > > > > Signed-off-by: Matti Vaittinen <
> > > > > > matti.vaittinen@fi.rohmeurope.com>
> > > > > > ---
> > > > > > 
> > > > > > No changes since v1
> > > > > > 
> > > > > >   .../bindings/mfd/rohm,bd71828-pmic.txt        | 180
> > > > > > ++++++++++++++++++
> > > > > >   1 file changed, 180 insertions(+)
> > > > > >   create mode 100644
> > > > > > Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.txt
> > > > > 
> > > > > I will let maintainers weigh in here but if this is new this
> > > > > should
> > > > > probably be in the yaml format to avoid conversion in the
> > > > > future
> > > > 
> > > > Oh... This is new to me. I guess there are reasons for this -
> > > > but I
> > > > must say I am not excited as I have never used yaml for
> > > > anything.
> > > > I'll
> > > > do as you suggest and wait for what others have to say :)
> > > > Thanks
> > > > for
> > > > pointing this out though.
> > > 
> > > Sorry for your lack of excitement. It could be XML...
> > 
> > Thanks, I appreciate that, apology accepted X-D
> > 
> > > There aren't many MFD examples yet, but there is max77650 in my
> > > tree
> > > and
> > > linux-next.
> > 
> > I looked at the max77650 MFD binding from linux-next. After that I
> > also
> > looked some of the generic documents for DT bindings (I know - I
> > should
> > have done that earlier and your job had been easier). But all that
> > left
> > me "slightly" puzzled. After some further wandering in the virtual
> > world I spotted this:
> > https://elinux.org/images/6/6b/LPC2018_json-schema_for_Devicetree.pdf
> > 
> > I think this link in some dt-yaml-binding-readme might be helpful.
> 
> Presentations bit rot, so I'd rather not. I'd hope that
> writing-schema.rst and example-schema.yaml capture what's in the
> presentation. What do you think is missing?

I personally wanted to understand "why?". Why not text doc. What is the
yaml thing aiming at? What are the problems we are solving here. And
maybe most crucially - I had no idea what is schema? It sure sounded
like some toolchain thingy or perhaps piece of new yaml representation
of dts (please note, I somehow thought that dts files were going to be
converted to yaml - maybe due to some reading about DTC getting yaml
support) which I thought would not need to be touched by me :) It took
me quite a while to understand that the old binding doc is actually a
schema. Without that piece finding out the new format of binding docs
was painful.

Also, binding and binding document were not completely same thing in my
mind. I thought that binding is actual piece of dt - probably living
under arch/x/boot/dts - binding document is what explains how that
should be construct and is under Documentation/devicetree/bindings/.
This is probably largely due to my ignorance and habit oh skipping much
of reading and just trying out things. But I hoped I had these cleared
in first documents I tried reading for creation binding docs..

...which brings me here. I looked at the
Documentation/devicetree/bindings folder and did read the 'writing-
bindings.txt' and 'submitting-patches.txt' from there. Then I also
checked the Documentation/devicetree/usage-model.txt None of which
helped me out. I did also open the 'writing-schema.rst' but I didn't
read it carefully enough. Probably because I thought after reading the
opening chapter that this described how to do actual dts in yaml.

Anyways, I might add some notes about using yaml format (and perhaps
shortly note that the yaml dt binding doc is called schema) in
Documentation/devicetree/bindings/writing-bindings.txt and
Documentation/devicetree/bindings/submitting-patches.txt

I could also appreciate some note about benefits/goals of using yaml
instead of text docs in writing-schema.rst - although I understand that
this may not be relevant for all readers.

> > So if I understand this correctly, idea is to convert the dts
> > sources
> > to use yaml (right?). This is seen better because more people
> > knowsubmitting-patches.txt
> > JSON/YAML than dts format(?) Fair enough. Although some of us know
> > dts
> > format decently well but have never used JSON or yaml. I guess dts
> > support is not going away though and yaml examples do not seem
> > terribly
> > hard at first sight.
> 
> No, nothing is changing for .dts files (other than fixing errors the
> schemas find). The free form, human readable only prose called
> binding
> documentation is changing to YAML formatted, json-schema vocabulary
> binding schema which can be used to validate dts files.

Thanks for sorting this out. It all makes more sense now.

> > What comes to binding docs - well, in my eyes (which may be biased)
> > writing documentation in anything intended to be interpreted by a
> > machine is still a step backwards for a human document reader. Sure
> > syntax validation or reviewing is easier if format is machine
> > readable
> > - but free text info is more, well, informative (form me at least).
> > I
> > for example wouldn't like reading a book written in any script or
> > markup language. Nor writing one. It is difficult for me to
> > understand
> > the documentation change to yaml, maybe because I am more often
> > using
> > the binding docs for composing DT for a device than reviewing them
> > ;)
> 
> ICYMI, all the kernel docs are in a markup language now...
> 
> Free form descriptions are easier to use because you can put in dts
> whatever you want. Nothing is going to check. There's been no
> shortage
> of errors and inconsistencies that we've already found.

I won't start arguing on this :)

> You can have as much description and comments as you like (though I'm
> trying to cut down on the copy-n-paste genericish 'clock for the
> module' type comments).

This is good to note. Thanks.

> > Anyways, I guess I'd better either try learning the yaml, figure
> > out
> > what are schemas and see how to convert yaml docs to text for nicer
> > reading (I assume this is doable) and how to verify yaml binding
> > docs
> > are Ok - or quit contributing. No one is forcing me to do this.
> > Continuing complaining on this is probably not getting us anywhere
> > so I
> > might as well shut up now :/
> 
> There is some notion to convert the DT spec to schema and then
> generate the spec from the schema. Take properties, their type, and
> descriptions and put that back into tables for example. Would love to
> have someone work on that. :)

I am glad to hear you have developed / are developing such tooling. I
really appreciate it. What comes to giving a helping hand - I'd better
to stick the simple C drivers for now ;) But if I ever get the feeling
that I don't know what to do I'll keep this in mind :] Let me do some
calculus... Only 11 years and my youngest son will probably leave our
house - do you think 2030 is a bit too late? Just let me know if this
is still relevant then - and I'll buy you a beer or write a tool (of
some kind) xD

Meanwhile... I have tried to convert the BD71828 DT doc from the RFC
patch to yaml - and I am having hard time. Especially with the
regulators node - which I would like to place in
Documentation/devicetree/bindings/regulator/rohm,bd71828-regulator.yaml

My problem is the
regulators {
buck1: BUCK1 {                                                   
                    regulator-name = "buck1";       
                    regulator-min-microvolt = <500000>;
                    regulator-max-microvolt = <2000000>;
                    regulator-ramp-delay = <2500>;
                    rohm,dvs-runlvl-ctrl;
                    rohm,dvs-runlevel0-voltage = <500000>;
                    rohm,dvs-runlevel1-voltage = <506250>;
                    rohm,dvs-runlevel2-voltage = <512500>;
                    rohm,dvs-runlevel3-voltage = <518750>;
                    regulator-boot-on;
    }; 
    ...
};
node which only contains BUCKX and LDOX sub-nodes. It has no own
properties.

From MFD yaml I did try:

  regulators:        
    $ref: ../regulator/rohm,bd71828-regulator.yaml
    description:          
      List of child nodes that specify the regulators.

and in rohm,bd71828-regulator.yaml

I tried doing:

patternProperties:
  "^BUCK[1-7]$":
    type: object
    description:
      Properties for single regulator.
    properties:
        ...

but this fails validation as properties: is not given.

[mvaittin@localhost linux]$ dt-doc-validate
Documentation/devicetree/bindings/regulator/rohm,bd71828-
regulator.yaml 
/home/mvaittin/torvalds/linux/Documentation/devicetree/bindings/regulat
or/rohm,bd71828-regulator.yaml: 'properties' is a required property

If I try and add:

properties:          
  foo: true

patternProperties:
    "^BUCK[1-7]$":
      type: object
      description:
        Properties for single regulator.
      properties:
        ...


then the validation goes (seemingly) smoothly.

I did not find any examples about handiling the empty regulators sub-
node in examples for regulators. Any pointers?

Br,
	Matti Vaittinen