Linux-LEDs Archive on lore.kernel.org
 help / color / Atom feed
From: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>
To: "robh@kernel.org" <robh@kernel.org>
Cc: "linux-leds@vger.kernel.org" <linux-leds@vger.kernel.org>,
	"dmurphy@ti.com" <dmurphy@ti.com>,
	"linux-rtc@vger.kernel.org" <linux-rtc@vger.kernel.org>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	"alexandre.belloni@bootlin.com" <alexandre.belloni@bootlin.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"mazziesaccount@gmail.com" <mazziesaccount@gmail.com>,
	"lgirdwood@gmail.com" <lgirdwood@gmail.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"jacek.anaszewski@gmail.com" <jacek.anaszewski@gmail.com>,
	"a.zummo@towertech.it" <a.zummo@towertech.it>,
	"linus.walleij@linaro.org" <linus.walleij@linaro.org>,
	"mturquette@baylibre.com" <mturquette@baylibre.com>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"bgolaszewski@baylibre.com" <bgolaszewski@baylibre.com>,
	"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>,
	"lee.jones@linaro.org" <lee.jones@linaro.org>,
	"pavel@ucw.cz" <pavel@ucw.cz>,
	"broonie@kernel.org" <broonie@kernel.org>,
	"sboyd@kernel.org" <sboyd@kernel.org>
Subject: Re: SPAM (R/EU IT) // Re: [RFC PATCH v3 03/15] dt-bindings: regulator: Document ROHM BD71282 regulator bindings
Date: Thu, 7 Nov 2019 06:50:17 +0000
Message-ID: <d6fbb812ae6f53d51b9f858523e2afe5bc03b940.camel@fi.rohmeurope.com> (raw)
In-Reply-To: <20191105205228.GB629@bogus>

Hello Rob,

On Tue, 2019-11-05 at 14:52 -0600, Rob Herring wrote:
> On Fri, Nov 01, 2019 at 01:31:46PM +0200, Matti Vaittinen wrote:
> > Document ROHM BD71828 PMIC regulator device tree bindings.
> > 
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > ---
> > 
> > Changes from v2 - my first encounter with yaml :/
> > 
> >  .../regulator/rohm,bd71828-regulator.yaml     | 123
> > ++++++++++++++++++
> >  1 file changed, 123 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/regulator/rohm,bd71828-
> > regulator.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/regulator/rohm,bd71828-
> > regulator.yaml
> > b/Documentation/devicetree/bindings/regulator/rohm,bd71828-
> > regulator.yaml
> > new file mode 100644
> > index 000000000000..60715d8b92df
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/rohm,bd71828-
> > regulator.yaml
> > @@ -0,0 +1,123 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: 
> > http://devicetree.org/schemas/regulator/rohm,bd71828-regulator.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ROHM BD71828 Power Management Integrated Circuit regulators
> > +
> > +maintainers:
> > +  - Liam Girdwood <lgirdwood@gmail.com>
> > +  - Mark Brown <broonie@kernel.org>
> > +  - Rob Herring <robh+dt@kernel.org>
> > +  - Mark Rutland <mark.rutland@arm.com>
> > +
> > +description: |
> > +  This module is part of the ROHM BD71828 MFD device. For more
> > details
> > +  see Documentation/devicetree/bindings/mfd/rohm,bd71828-
> > pmic.yaml.
> > +
> > +  The regulator controller is represented as a sub-node of the
> > PMIC node
> > +  on the device tree.
> > +
> > +  Regulator nodes should be named to BUCK_<number> and
> > LDO_<number>.
> > +  The valid names for BD71828 regulator nodes are
> > +  BUCK1, BUCK2, BUCK3, BUCK4, BUCK5, BUCK6, BUCK7
> > +  LDO1, LDO2, LDO3, LDO4, LDO5, LDO6, LDO7
> > +
> > +patternProperties:
> > +  "^LDO[1-7]$":
> > +    type: object
> > +    allOf:
> > +      - $ref: regulator.yaml#
> > +    description:
> > +      Properties for single LDO regulator.
> > +
> > +    properties:
> > +      #Is there a nice way to check the name is same as node name
> > but lower case
> 
> Well, lowercase nodenames are preferred... But still, no, there's
> not.

I'd like to follow the convention of using upper-case node names like
ROHM BD71837, BD71847 and BD70528 PMICs do.

> And I think you could just drop this and the nodename is used
> instead.

If I used lower-case, then yes. But if I follow what other BD-PMICs
did, then I guess I should keep this. (lowercase names for consumers
feel more correct to me). Someone once told me that naming is hard :|

> > +      regulator-name:
> > +        description:
> > +          should be "ldo1", ..., "ldo7"
> 
> You can at least do:
> 
> pattern: "^ldo[1-7]$"

Yep. Thanks :)

> > +
> > +  "^BUCK[1-7]$":
> > +    type: object
> > +    allOf:
> > +      - $ref: regulator.yaml#
> > +    description:
> > +      Properties for single BUCK regulator.
> > +
> > +    properties:
> > +      #Is there a nice way to check the name is same as node name
> > but lower case
> > +      regulator-name:
> > +        description:
> > +          should be "buck1", ..., "buck7"
> > +
> > +      rohm,dvs-run-voltage:
> > +        $ref: "/schemas/types.yaml#/definitions/uint32"
> > +        description:
> > +          PMIC default "RUN" state voltage in uV. See below table
> > for
> > +          bucks which support this.
> 
> Use standard unit-suffixes on all these (-microvolt). And then drop
> the 
> $ref.

Hmm.. The rohm,dvs-run-voltage, rohm,dvs-idle-voltage and rohm,dvs-
suspend-voltage are already defined in rohm,bd71837-regulator.txt. I am
a bit hesitant what comes to changing the existing properties as it
will probably cause some problems out there... On the other hand, I
don't like defining two different "rohm" DT entries for same thing. How
important you think having the correct suffix here is? Should the old
one(s) be still silently supported by driver(s) while changing the docs
also for bd71837 and bd71847 to contain the new ones? Although - if the
idea is to convert also docs for bd71837/47 to yaml - that would mean
at least breaking the build for someone who uses old DTS. I just don't
know what would be the right thing to do. (naming is ...).

> 
> Any constraints on the range?

Sure. Thanks. I'll add constrains.

> 
> > +
> > +      rohm,dvs-idle-voltage:
> > +        $ref: "/schemas/types.yaml#/definitions/uint32"
> > +        description:
> > +          PMIC default "IDLE" state voltage in uV. See below table
> > for
> > +          bucks which support this.
> > +
> > +      rohm,dvs-suspend-voltage:
> > +        $ref: "/schemas/types.yaml#/definitions/uint32"
> > +        description:
> > +          PMIC default "SUSPEND" state voltage in uV. See below
> > table for
> > +          bucks which support this.
> > +
> > +      rohm,dvs-lpsr-voltage:
> > +        $ref: "/schemas/types.yaml#/definitions/uint32"
> > +        description:
> > +          PMIC default "LPSR" state voltage in uV. See below table
> > for
> > +          bucks which support this.
> > +
> > +#Supported default DVS states:
> > +#buck		| run		| idle		| suspend	
> > | lpsr
> > +#-----------------------------------------------------------------
> > -----------
> > +#1, 2, 6, and 7	| supported	| supported	| 	supported
> > (*)
> > +#-----------------------------------------------------------------
> > -----------
> > +#3, 4, and 5	| 			supported (**)
> > +#-----------------------------------------------------------------
> > -----------
> > +#(*)  LPSR and SUSPEND states use same voltage but both states
> > have own enable /
> > +#     disable settings. Voltage 0 can be specified for a state to
> > make regulator
> > +#     disabled on that state.
> > +#(**) All states use same voltage but have own enable / disable
> > settings.
> > +#     Voltage 0 can be specified for a state to make regulator
> > disabled on that
> > +#     state.
> > +
> > +      rohm,dvs-runlvl-ctrl:
> > +        description: |
> > +          buck control is done based on run-level. Regulator is
> > not
> > +          individually controllable. See ../mfd/rohm,bd71828-
> > pmic.yaml for
> > +          how to specify run-level control mechanism. Only bucks
> > 1, 2, 6
> > +          and 7 support this.
> > +        type: boolean
> > +
> > +      rohm,dvs-runlevel0-voltage:
> > +        $ref: "/schemas/types.yaml#/definitions/uint32"
> > +        description:
> > +          voltage for run-level 0. Microvolts.
> > +
> > +      rohm,dvs-runlevel1-voltage:
> > +        $ref: "/schemas/types.yaml#/definitions/uint32"
> > +        description:
> > +          voltage for run-level 1. Microvolts.
> > +
> > +      rohm,dvs-runlevel2-voltage:
> > +        $ref: "/schemas/types.yaml#/definitions/uint32"
> > +        description:
> > +          voltage for run-level 2. Microvolts.
> > +
> > +      rohm,dvs-runlevel3-voltage:
> > +        $ref: "/schemas/types.yaml#/definitions/uint32"
> > +        description:
> > +          voltage for run-level 3. Microvolts.
> 
> Perhaps an array of 4 values for runlevel?

I'm not sure. From HW perspective giving all values is not required if
HW defaults are to be used. I'd like to keep giving any of these
optional. OTOH, I see the possible issue of having more than 4 run-
levels in the future. (I see possible issue, I do not see any product
from ROHM that would contain those - but I don't see too far to the
future from my low seat ;]) Having 10 run-level properties would look
horrible.

> > +
> > +    required:
> > +      - regulator-name
> > +  additionalProperties: false
> > +additionalProperties: false
> > -- 
> > 2.21.0
> > 
> > 
> > -- 
> > Matti Vaittinen, Linux device drivers
> > ROHM Semiconductors, Finland SWDC
> > Kiviharjunlenkki 1E
> > 90220 OULU
> > FINLAND
> > 
> > ~~~ "I don't think so," said Rene Descartes. Just then he vanished
> > ~~~
> > Simon says - in Latin please.
> > ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
> > Thanks to Simon Glass for the translation =] 


  reply index

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-01 11:28 [RFC PATCH v3 00/15] Support ROHM BD71828 PMIC Matti Vaittinen
2019-11-01 11:29 ` [RFC PATCH v3 01/15] mfd: bd71828: Support ROHM BD71828 PMIC - core Matti Vaittinen
2019-11-11 10:57   ` Lee Jones
2019-11-11 11:20     ` Vaittinen, Matti
2019-11-01 11:31 ` [RFC PATCH v3 02/15] dt-bindings: mfd: Document ROHM BD71828 bindings Matti Vaittinen
2019-11-05 20:43   ` Rob Herring
2019-11-06 12:55     ` SPAM (R/EU IT) // " Vaittinen, Matti
2019-11-01 11:31 ` [RFC PATCH v3 03/15] dt-bindings: regulator: Document ROHM BD71282 regulator bindings Matti Vaittinen
2019-11-05 20:52   ` Rob Herring
2019-11-07  6:50     ` Vaittinen, Matti [this message]
2019-11-01 11:32 ` [RFC PATCH v3 04/15] dt-bindings: leds: ROHM BD71282 PMIC LED driver Matti Vaittinen
2019-11-05 19:14   ` Dan Murphy
2019-11-05 20:59     ` Rob Herring
2019-11-06 13:05     ` Vaittinen, Matti
2019-11-05 20:57   ` Rob Herring
2019-11-01 11:33 ` [RFC PATCH v3 05/15] mfd: input: bd71828: Add power-key support Matti Vaittinen
2019-11-11 10:59   ` Lee Jones
2019-11-11 11:07     ` Vaittinen, Matti
2019-11-01 11:38 ` [RFC PATCH v3 06/15] clk: bd718x7: Support ROHM BD71828 clk block Matti Vaittinen
2019-11-01 11:39 ` [RFC PATCH v3 07/15] clk: bd718x7: simplify header dependencies Matti Vaittinen
2019-11-01 11:41 ` [RFC PATCH v3 08/15] regulator: bd718x7: Split driver to common and bd718x7 specific parts Matti Vaittinen
2019-11-01 11:42 ` [RFC PATCH v3 09/15] regulator: bd71828: Basic support for ROHM bd71828 PMIC regulators Matti Vaittinen
2019-11-01 11:43 ` [RFC PATCH v3 10/15] regulator: bd71828: Add GPIO based run-level control for regulators Matti Vaittinen
2019-11-03 22:27   ` Linus Walleij
2019-11-04  7:05     ` Vaittinen, Matti
2019-11-05 13:24       ` Linus Walleij
2019-11-05 14:07         ` Vaittinen, Matti
2019-11-01 11:44 ` [RFC PATCH v3 11/15] regulator: bd71828: enhanced run-level support Matti Vaittinen
2019-11-01 11:45 ` [RFC PATCH v3 12/15] regulator: bd71828: Support in-kernel APIs to change run-level Matti Vaittinen
2019-11-01 11:49 ` [RFC PATCH v3 13/15] rtc: bd70528 add BD71828 support Matti Vaittinen
2019-11-01 11:49 ` [RFC PATCH v3 14/15] gpio: Add definition for GPIO direction Matti Vaittinen
2019-11-03 22:30   ` Linus Walleij
2019-11-04  6:57     ` Vaittinen, Matti
2019-11-04 15:48     ` Vaittinen, Matti
2019-11-05 15:05       ` Linus Walleij
2019-11-06  6:51         ` Vaittinen, Matti
2019-11-01 11:51 ` [RFC PATCH v3 15/15] gpio: bd71828: Initial support for ROHM BD71828 PMIC GPIOs Matti Vaittinen
2019-11-01 11:53 ` [RFC PATCH v3 00/15] Support ROHM BD71828 PMIC Vaittinen, Matti

Reply instructions:

You may reply publically 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=d6fbb812ae6f53d51b9f858523e2afe5bc03b940.camel@fi.rohmeurope.com \
    --to=matti.vaittinen@fi.rohmeurope.com \
    --cc=a.zummo@towertech.it \
    --cc=alexandre.belloni@bootlin.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmurphy@ti.com \
    --cc=jacek.anaszewski@gmail.com \
    --cc=lee.jones@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=linux-rtc@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mazziesaccount@gmail.com \
    --cc=mturquette@baylibre.com \
    --cc=pavel@ucw.cz \
    --cc=robh@kernel.org \
    --cc=sboyd@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

Linux-LEDs Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-leds/0 linux-leds/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-leds linux-leds/ https://lore.kernel.org/linux-leds \
		linux-leds@vger.kernel.org
	public-inbox-index linux-leds

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-leds


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git