[RFC,v2,02/13] dt-bindings: mfd: Document ROHM BD71828 bindings
diff mbox series

Message ID 0182df3c49c6c804ee20ef32fc4b85b50ff45fca.1571915550.git.matti.vaittinen@fi.rohmeurope.com
State New
Headers show
Series
  • Support ROHM BD71828 PMIC
Related show

Commit Message

Vaittinen, Matti Oct. 24, 2019, 11:41 a.m. UTC
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

Comments

Dan Murphy Oct. 24, 2019, 7:35 p.m. UTC | #1
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


> diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.txt
> new file mode 100644
> index 000000000000..125efa9f3de0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.txt
> @@ -0,0 +1,180 @@
> +* ROHM BD71828 Power Management Integrated Circuit bindings
> +
> +BD71828GW is a single-chip power management IC for battery-powered portable
> +devices. The IC integrates 7 buck converters, 7 LDOs, and a 1500 mA single-cell
> +linear charger. Also included is a Coulomb counter, a real-time clock (RTC),
> +and a 32.768 kHz clock gate.
> +
> +Required properties:
> + - compatible			: Should be "rohm,bd71828".
> + - reg				: I2C slave address.
> + - interrupt-parent		: Phandle to the parent interrupt controller.
> + - interrupts			: The interrupt line the device is connected to.
> + - clocks			: The parent clock connected to PMIC.
> + - #clock-cells			: Should be 0.
> + - regulators			: List of child nodes that specify the
> +				  regulators. Please see
> +				  ../regulator/rohm,bd71828-regulator.txt
> + - gpio-controller		: To indicate BD71828 acts as a GPIO controller.
> + - #gpio-cells			: Should be 2. The first cell is the pin number
> +				  and the second cell is used to specify flags.
> +				  See ../gpio/gpio.txt for more information.
> +
> +The BD71828 RUN state is divided into 4 configurable run-levels named RUN0,
> +RUN1, RUN2 and RUN3. Bucks 1, 2, 6 and 7 can be either controlled individually
> +via I2C, or some/all of them can be bound to run-levels and controlled as a
> +group. If bucks are controlled individually these run-levels are ignored. See
> +../regulator/rohm,bd71828-regulator.txt for how to define regulator voltages
The rohm,bd71828-regulator.txt should be yaml if the maintainers want it 
that way.
> +for run-levels. Run-levels can be changed by I2C or GPIO depending on PMIC's OTP
> +configuration.
> +
> +Optional properties:
> +- clock-output-names		: Should contain name for output clock.
> +- rohm,dvs-vsel-gpios		: GPIOs used to control PMIC run-levels. Should
> +				  describe two GPIOs. (See run-level control in
> +				  data-sheet). If this property is omitted but
> +				  some bucks are marked to be controlled by
> +				  run-levels - then OTP option allowing
> +				  run-level control via I2C is assumed.
> +- gpio-reserved-ranges		: Usage of GPIO pins can be changed via OTP.
> +				  This property can be used to mark the pins
> +				  which should not be configured for GPIO.
> +				  Please see the ../gpio/gpio.txt for more
> +				  information.
> +
> +Example:
> +

This example does not look right.

I see that I2C is referenced above so the example could look like this

osc: oscillator {
                 compatible = "fixed-clock";
                 #clock-cells = <1>;
                 clock-frequency  = <32768>;
                 clock-output-names = "osc";
         };

This is an external oscillator and is not really part of the pmic 
itself.  I am not sure you even need to define that since it is not part 
of the pmic.

i2c {

         pmic@4b {

                 [...]

         };

};

Dan
Vaittinen, Matti Oct. 25, 2019, 5:49 a.m. UTC | #2
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.

> > diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-
> > pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd71828-
> > pmic.txt
> > new file mode 100644
> > index 000000000000..125efa9f3de0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.txt
> > @@ -0,0 +1,180 @@
> > +* ROHM BD71828 Power Management Integrated Circuit bindings
> > +
> > +BD71828GW is a single-chip power management IC for battery-powered 
> > portable
> > +devices. The IC integrates 7 buck converters, 7 LDOs, and a 1500
> > mA single-cell
> > +linear charger. Also included is a Coulomb counter, a real-time
> > clock (RTC),
> > +and a 32.768 kHz clock gate.
> > +
> > +Required properties:
> > + - compatible			: Should be "rohm,bd71828".
> > + - reg				: I2C slave address.
> > + - interrupt-parent		: Phandle to the parent
> > interrupt controller.
> > + - interrupts			: The interrupt line the device
> > is connected to.
> > + - clocks			: The parent clock connected to PMIC.
> > + - #clock-cells			: Should be 0.
> > + - regulators			: List of child nodes that
> > specify the
> > +				  regulators. Please see
> > +				  ../regulator/rohm,bd71828-
> > regulator.txt
> > + - gpio-controller		: To indicate BD71828 acts as a GPIO
> > controller.
> > + - #gpio-cells			: Should be 2. The first cell
> > is the pin number
> > +				  and the second cell is used to
> > specify flags.
> > +				  See ../gpio/gpio.txt for more
> > information.
> > +
> > +The BD71828 RUN state is divided into 4 configurable run-levels
> > named RUN0,
> > +RUN1, RUN2 and RUN3. Bucks 1, 2, 6 and 7 can be either controlled
> > individually
> > +via I2C, or some/all of them can be bound to run-levels and
> > controlled as a
> > +group. If bucks are controlled individually these run-levels are
> > ignored. See
> > +../regulator/rohm,bd71828-regulator.txt for how to define
> > regulator voltages

> The rohm,bd71828-regulator.txt should be yaml if the maintainers want
> it 
> that way.

Let's see if this should be changed then :)

> > +for run-levels. Run-levels can be changed by I2C or GPIO depending
> > on PMIC's OTP
> > +configuration.
> > +
> > +Optional properties:
> > +- clock-output-names		: Should contain name for
> > output clock.
> > +- rohm,dvs-vsel-gpios		: GPIOs used to control PMIC
> > run-levels. Should
> > +				  describe two GPIOs. (See run-level
> > control in
> > +				  data-sheet). If this property is
> > omitted but
> > +				  some bucks are marked to be
> > controlled by
> > +				  run-levels - then OTP option allowing
> > +				  run-level control via I2C is assumed.
> > +- gpio-reserved-ranges		: Usage of GPIO pins can be
> > changed via OTP.
> > +				  This property can be used to mark the
> > pins
> > +				  which should not be configured for
> > GPIO.
> > +				  Please see the ../gpio/gpio.txt for
> > more
> > +				  information.
> > +
> > +Example:
> > +
> 
> This example does not look right.
> 
> I see that I2C is referenced above so the example could look like
> this
> 
> osc: oscillator {
>                  compatible = "fixed-clock";
>                  #clock-cells = <1>;
>                  clock-frequency  = <32768>;
>                  clock-output-names = "osc";
>          };
> 
> This is an external oscillator and is not really part of the pmic 
> itself.  I am not sure you even need to define that since it is not
> part 
> of the pmic.

I think you are correct. I'll drop this oscillator for next patch.

> 
> i2c {
> 
>          pmic@4b {
> 
>                  [...]
> 
>          };
> 
> };

I don't think the I2C node is needed in example. It is not part of the
PMIC - and I don't see the containing bus in other examples I just
opened. (the two other rohm,xxx PMIC docs - well, biased as I wrote
them), da9150.txt, lp3943.txt, max77686.txt, tps6507x.txt, tps65910.txt
...

Br,
	Matti Vaittinen
Lee Jones Oct. 29, 2019, 12:08 p.m. UTC | #3
On Fri, 25 Oct 2019, 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

Yes please.

> 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.
Rob Herring Oct. 29, 2019, 7:34 p.m. UTC | #4
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...

There aren't many MFD examples yet, but there is max77650 in my tree and 
linux-next.

> > > diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-
> > > pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd71828-
> > > pmic.txt
> > > new file mode 100644
> > > index 000000000000..125efa9f3de0
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.txt
> > > @@ -0,0 +1,180 @@
> > > +* ROHM BD71828 Power Management Integrated Circuit bindings
> > > +
> > > +BD71828GW is a single-chip power management IC for battery-powered 
> > > portable
> > > +devices. The IC integrates 7 buck converters, 7 LDOs, and a 1500
> > > mA single-cell
> > > +linear charger. Also included is a Coulomb counter, a real-time
> > > clock (RTC),
> > > +and a 32.768 kHz clock gate.
> > > +
> > > +Required properties:
> > > + - compatible			: Should be "rohm,bd71828".
> > > + - reg				: I2C slave address.
> > > + - interrupt-parent		: Phandle to the parent
> > > interrupt controller.
> > > + - interrupts			: The interrupt line the device
> > > is connected to.
> > > + - clocks			: The parent clock connected to PMIC.
> > > + - #clock-cells			: Should be 0.
> > > + - regulators			: List of child nodes that
> > > specify the
> > > +				  regulators. Please see
> > > +				  ../regulator/rohm,bd71828-
> > > regulator.txt
> > > + - gpio-controller		: To indicate BD71828 acts as a GPIO
> > > controller.
> > > + - #gpio-cells			: Should be 2. The first cell
> > > is the pin number
> > > +				  and the second cell is used to
> > > specify flags.
> > > +				  See ../gpio/gpio.txt for more
> > > information.
> > > +
> > > +The BD71828 RUN state is divided into 4 configurable run-levels
> > > named RUN0,
> > > +RUN1, RUN2 and RUN3. Bucks 1, 2, 6 and 7 can be either controlled
> > > individually
> > > +via I2C, or some/all of them can be bound to run-levels and
> > > controlled as a
> > > +group. If bucks are controlled individually these run-levels are
> > > ignored. See
> > > +../regulator/rohm,bd71828-regulator.txt for how to define
> > > regulator voltages
> 
> > The rohm,bd71828-regulator.txt should be yaml if the maintainers want
> > it 
> > that way.
> 
> Let's see if this should be changed then :)
> 
> > > +for run-levels. Run-levels can be changed by I2C or GPIO depending
> > > on PMIC's OTP
> > > +configuration.
> > > +
> > > +Optional properties:
> > > +- clock-output-names		: Should contain name for
> > > output clock.
> > > +- rohm,dvs-vsel-gpios		: GPIOs used to control PMIC
> > > run-levels. Should
> > > +				  describe two GPIOs. (See run-level
> > > control in
> > > +				  data-sheet). If this property is
> > > omitted but
> > > +				  some bucks are marked to be
> > > controlled by
> > > +				  run-levels - then OTP option allowing
> > > +				  run-level control via I2C is assumed.
> > > +- gpio-reserved-ranges		: Usage of GPIO pins can be
> > > changed via OTP.
> > > +				  This property can be used to mark the
> > > pins
> > > +				  which should not be configured for
> > > GPIO.
> > > +				  Please see the ../gpio/gpio.txt for
> > > more
> > > +				  information.
> > > +
> > > +Example:
> > > +
> > 
> > This example does not look right.
> > 
> > I see that I2C is referenced above so the example could look like
> > this
> > 
> > osc: oscillator {
> >                  compatible = "fixed-clock";
> >                  #clock-cells = <1>;
> >                  clock-frequency  = <32768>;
> >                  clock-output-names = "osc";
> >          };
> > 
> > This is an external oscillator and is not really part of the pmic 
> > itself.  I am not sure you even need to define that since it is not
> > part 
> > of the pmic.
> 
> I think you are correct. I'll drop this oscillator for next patch.
> 
> > 
> > i2c {
> > 
> >          pmic@4b {
> > 
> >                  [...]
> > 
> >          };
> > 
> > };
> 
> I don't think the I2C node is needed in example. It is not part of the
> PMIC - and I don't see the containing bus in other examples I just
> opened. (the two other rohm,xxx PMIC docs - well, biased as I wrote
> them), da9150.txt, lp3943.txt, max77686.txt, tps6507x.txt, tps65910.txt

It will be needed for the schema because the examples are compiled and 
validated.

Rob
Vaittinen, Matti Oct. 30, 2019, 8:26 a.m. UTC | #5
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.

So if I understand this correctly, idea is to convert the dts sources
to use yaml (right?). This is seen better because more people know
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.

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 ;)

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

And Sorry Rob. I am seeing you have been really close to this yaml/JSON
change so my wondering may be frustrating. I don't intend to be
disrespectful - I see that you have done huge work with this. I am
just... ...Slightly set in my ways. Little bit pig-headed and somewhat
a smart-arse - so I couldn't just let it go without giving out an
opinion.

> > > i2c {
> > > 
> > >          pmic@4b {
> > > 
> > >                  [...]
> > > 
> > >          };
> > > 
> > > };
> > 
> > I don't think the I2C node is needed in example. It is not part of
> > the
> > PMIC - and I don't see the containing bus in other examples I just
> > opened. (the two other rohm,xxx PMIC docs - well, biased as I wrote
> > them), da9150.txt, lp3943.txt, max77686.txt, tps6507x.txt,
> > tps65910.txt
> 
> It will be needed for the schema because the examples are compiled
> and 
> validated.

Thanks for explaining the reason.

Br,
	Matti Vaittinen
Rob Herring Oct. 30, 2019, 7:22 p.m. UTC | #6
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?

> So if I understand this correctly, idea is to convert the dts sources
> to use yaml (right?). This is seen better because more people know
> 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.

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

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).

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

> And Sorry Rob. I am seeing you have been really close to this yaml/JSON
> change so my wondering may be frustrating. I don't intend to be
> disrespectful - I see that you have done huge work with this. I am
> just... ...Slightly set in my ways. Little bit pig-headed and somewhat
> a smart-arse - so I couldn't just let it go without giving out an
> opinion.

Everyone is welcome to their opinion.

Rob
Vaittinen, Matti Oct. 31, 2019, 12:54 p.m. UTC | #7
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
Rob Herring Oct. 31, 2019, 5:50 p.m. UTC | #8
On Thu, Oct 31, 2019 at 7:54 AM Vaittinen, Matti
<Matti.Vaittinen@fi.rohmeurope.com> wrote:
>
>
> 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.

I guess 'why' is easy enough to address.

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

Things are a bit scattered around I'll admit. I feel like we need a
'start here', but the challenge is people have different starting
points.

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

TBC, I have not and am not. It's just an idea. There's been nothing
done beyond experimenting if rST could be embedded into yaml.

> 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

I've scheduled you in for 2030. :)

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

That's a case of needing to adjust the meta-schema (the schema that
checks the schemas). It's a bit overly restrictive just to try to
contain what's allowed. I've fixed it now. Update dtschema and it
should work now.

BTW, what you will also need is to reference the common schema:

"^BUCK[1-7]$":
  type: object
  allOf:
    - $ref: regulator.yaml#
  properties:
   rohm,dvs-runlvl-ctrl:
     type: boolean
     description: ...
   ...

Rob
Vaittinen, Matti Nov. 1, 2019, 12:52 p.m. UTC | #9
On Thu, 2019-10-31 at 12:50 -0500, Rob Herring wrote:
> On Thu, Oct 31, 2019 at 7:54 AM Vaittinen, Matti
> <Matti.Vaittinen@fi.rohmeurope.com> wrote:
> > 
> > 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 
> > ...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.
> 
> Things are a bit scattered around I'll admit. I feel like we need a
> 'start here', but the challenge is people have different starting
> points.

cross-referencing? =)

I guess that if yaml is what is expected to be used as patch format,
then we should probably mention this in submitting-patches.txt and
writing-bindings.txt. Actyually, I think that writing-bindings.txt
could be combined with writing-schema.rst - they are about the same
thing, right?

> > > 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.
> 
> TBC, I have not and am not. It's just an idea. There's been nothing
> done beyond experimenting if rST could be embedded into yaml.
> 
> > 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
> 
> I've scheduled you in for 2030. :)

Fine. Let's see if it is a beer or a tool then :]

> > 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/reg
> > ulat
> > 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:
> >         ...
> 
> That's a case of needing to adjust the meta-schema (the schema that
> checks the schemas). It's a bit overly restrictive just to try to
> contain what's allowed. I've fixed it now. Update dtschema and it
> should work now.

Thanks. At least the make dt_binding_check passed now. dt-doc-validate
is not able to locate the regulator.yaml and errors out - but it does
no longer complain about missing 'properties:'.

> BTW, what you will also need is to reference the common schema:
> 
> "^BUCK[1-7]$":
>   type: object
>   allOf:
>     - $ref: regulator.yaml#
>   properties:
>    rohm,dvs-runlvl-ctrl:
>      type: boolean
>      description: ...
>    ...

Thanks for the pointers ;) I just submitted the RFC v3 :)

Br,
	Matti Vaittinen
Rob Herring Nov. 4, 2019, 7:28 p.m. UTC | #10
On Fri, Nov 1, 2019 at 7:52 AM Vaittinen, Matti
<Matti.Vaittinen@fi.rohmeurope.com> wrote:
>
>
> On Thu, 2019-10-31 at 12:50 -0500, Rob Herring wrote:
> > On Thu, Oct 31, 2019 at 7:54 AM Vaittinen, Matti
> > <Matti.Vaittinen@fi.rohmeurope.com> wrote:
> > >
> > > 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
> > > ...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.
> >
> > Things are a bit scattered around I'll admit. I feel like we need a
> > 'start here', but the challenge is people have different starting
> > points.
>
> cross-referencing? =)
>
> I guess that if yaml is what is expected to be used as patch format,
> then we should probably mention this in submitting-patches.txt and
> writing-bindings.txt.

I've actually added that to submitting-patches.txt already and
checkpatch.pl now warns for non-schema bindings. Both are pending for
5.5.

> Actyually, I think that writing-bindings.txt
> could be combined with writing-schema.rst - they are about the same
> thing, right?

Not really. writing-bindings.txt is about how to design bindings. It's
the DO's and DON'Ts that I give in review comments over and over and
over. Mostly an attempt to ensure I'm saying things that are
documented somewhere. Probably need to come up with better names, but
naming is hard(TM).


> > > > 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.
> >
> > TBC, I have not and am not. It's just an idea. There's been nothing
> > done beyond experimenting if rST could be embedded into yaml.
> >
> > > 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
> >
> > I've scheduled you in for 2030. :)
>
> Fine. Let's see if it is a beer or a tool then :]
>
> > > 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/reg
> > > ulat
> > > 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:
> > >         ...
> >
> > That's a case of needing to adjust the meta-schema (the schema that
> > checks the schemas). It's a bit overly restrictive just to try to
> > contain what's allowed. I've fixed it now. Update dtschema and it
> > should work now.
>
> Thanks. At least the make dt_binding_check passed now. dt-doc-validate
> is not able to locate the regulator.yaml and errors out - but it does
> no longer complain about missing 'properties:'.

Either look at how Kbuild calls dt-doc-validate or just use 'make
dt_binding_check'. (It needs the base path to the schema passed in.)

Rob

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.txt
new file mode 100644
index 000000000000..125efa9f3de0
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.txt
@@ -0,0 +1,180 @@ 
+* ROHM BD71828 Power Management Integrated Circuit bindings
+
+BD71828GW is a single-chip power management IC for battery-powered portable
+devices. The IC integrates 7 buck converters, 7 LDOs, and a 1500 mA single-cell
+linear charger. Also included is a Coulomb counter, a real-time clock (RTC),
+and a 32.768 kHz clock gate.
+
+Required properties:
+ - compatible			: Should be "rohm,bd71828".
+ - reg				: I2C slave address.
+ - interrupt-parent		: Phandle to the parent interrupt controller.
+ - interrupts			: The interrupt line the device is connected to.
+ - clocks			: The parent clock connected to PMIC.
+ - #clock-cells			: Should be 0.
+ - regulators			: List of child nodes that specify the
+				  regulators. Please see
+				  ../regulator/rohm,bd71828-regulator.txt
+ - gpio-controller		: To indicate BD71828 acts as a GPIO controller.
+ - #gpio-cells			: Should be 2. The first cell is the pin number
+				  and the second cell is used to specify flags.
+				  See ../gpio/gpio.txt for more information.
+
+The BD71828 RUN state is divided into 4 configurable run-levels named RUN0,
+RUN1, RUN2 and RUN3. Bucks 1, 2, 6 and 7 can be either controlled individually
+via I2C, or some/all of them can be bound to run-levels and controlled as a
+group. If bucks are controlled individually these run-levels are ignored. See
+../regulator/rohm,bd71828-regulator.txt for how to define regulator voltages
+for run-levels. Run-levels can be changed by I2C or GPIO depending on PMIC's OTP
+configuration.
+
+Optional properties:
+- clock-output-names		: Should contain name for output clock.
+- rohm,dvs-vsel-gpios		: GPIOs used to control PMIC run-levels. Should
+				  describe two GPIOs. (See run-level control in
+				  data-sheet). If this property is omitted but
+				  some bucks are marked to be controlled by
+				  run-levels - then OTP option allowing
+				  run-level control via I2C is assumed.
+- gpio-reserved-ranges		: Usage of GPIO pins can be changed via OTP.
+				  This property can be used to mark the pins
+				  which should not be configured for GPIO.
+				  Please see the ../gpio/gpio.txt for more
+				  information.
+
+Example:
+
+        /* external oscillator node */
+        osc: oscillator {
+                compatible = "fixed-clock";
+                #clock-cells = <1>;
+                clock-frequency  = <32768>;
+                clock-output-names = "osc";
+        };
+
+	pmic: pmic@4b {
+		compatible = "rohm,bd71828";
+		reg = <0x4b>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <29 GPIO_ACTIVE_LOW>;
+		clocks = <&osc 0>;
+		#clock-cells = <0>;
+		clock-output-names = "bd71828-32k-out";
+		gpio-controller;
+		#gpio-cells = <2>;
+		ngpios = <4>;
+		gpio-reserved-ranges = <0 1 2 1>;
+		gpio-line-names = "EPDEN";
+		rohm,dvs-vsel-gpios = <&gpio1 12 0>,
+				      <&gpio1 13 0>;
+		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;
+			};
+			buck2: BUCK2 {
+				regulator-name = "buck2";
+				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;
+			};
+			buck3: BUCK3 {
+				regulator-name = "buck3";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <2000000>;
+				regulator-boot-on;
+			};
+			buck4: BUCK4 {
+				regulator-name = "buck4";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-boot-on;
+			};
+			buck5: BUCK5 {
+				regulator-name = "buck5";
+				regulator-min-microvolt = <2500000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+			};
+			buck6: BUCK6 {
+				regulator-name = "buck6";
+				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;
+			};
+			buck7: BUCK7 {
+				regulator-name = "buck7";
+				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;
+			};
+			ldo1: LDO1 {
+				regulator-name = "ldo1";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+			};
+			ldo2: LDO2 {
+				regulator-name = "ldo2";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+			};
+			ldo3: LDO3 {
+				regulator-name = "ldo3";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+			};
+			ldo4: LDO4 {
+				regulator-name = "ldo4";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+			};
+			ldo5: LDO5 {
+				regulator-name = "ldo5";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+			};
+			ldo6: LDO6 {
+				regulator-name = "ldo6";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-boot-on;
+			};
+			ldo7_reg: LDO7 {
+				regulator-name = "ldo7";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+			};
+		};
+	};