Re: [PATCH 1/8] dt-bindings: regulator: ROHM BD73800 regulators

[Matti Vaittinen]

Hi Rob,

Thanks (again) for the review!

On 01/07/2026 22:25, Rob Herring wrote:
> On Wed, Jul 01, 2026 at 03:41:11PM +0300, Matti Vaittinen wrote:
> > From: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
> >
> > Add bindings for the BUCKs and LDOs on ROHM BD73800. The PMIC state
> > specific voltages can be set in same fashion as with a few other ROHM
> > PMICs (for example with BD718[15,28,37,47,50,79]). Same properties are
> > recycled :)
> >
> > The LDOs 1 and 4 can use different voltage ranges depending on the OTP
> > configuration.
> >
> > Signed-off-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
> > ---
> >   .../regulator/rohm,bd73800-regulator.yaml     | 119 ++++++++++++++++++
> >   1 file changed, 119 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml b/Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml
> > new file mode 100644
> > index 000000000000..c427a04098ec
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml
>
>
> > +      rohm,dvs-run-voltage:
> > +        description:
> > +          PMIC default "RUN" state voltage in uV. 0 means disabled. See the
> > +          explanation below for regulator specific details.
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        minimum: 0
> > +        maximum: 3500000
>
> [...]
>
> > +      rohm,dvs-run-voltage:
> > +        description:
> > +          Set the default output state at PMIC's "RUN" state.
> > +          0 is disabled, 1 is enabled.
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        minimum: 0
> > +        maximum: 1
>
> Same property name with 2 different meanings. Not a good design pattern.

Hmm. They do actually have the same meaning. Setting the "RUN" -state 
voltage. Values '0' and '1' have special meaning "disable" and "enable" 
- also for BUCKs.

For LDOs on this PMIC, only the enable/disable configuration can be set 
for each hardware-state as the LDO voltage is same for all hardware 
states. Hence only subset of the property values (1/0) are supported for 
the LDOs.

> Also, if these properties are copied from other schemas, don't duplicate
> them. Put them in a common schema and reference it here.

Ah. I think this is a great idea, and I should've thought that already a 
few PMICs ago :) We have been reviewing and discussing these properties 
with you since ... maybe 2018, as I've added new PMICs re-using them... ;)

Thanks! I'll rework this but v2 is likely to be out only at August.

Yours,
	-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~