Re: [PATCH v2 01/10] dt-bindings: mfd: common ROHM PMIC properties

Linus Walleij <[email protected]> Fri, 7 Aug 2026 22:23:15 +0200
Newsgroups dev.linux.lists.mfd,org.kernel.vger.linux-clk,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-gpio,org.kernel.vger.linux-kernel,org.kernel.vger.linux-rtc
Message-ID <CAD++jLky237PRXNmyURjNEOPPgFV00ox1HBeBev=qADxKM3nWQ@mail.gmail.com>
Hi Matti,

thanks for your patch (and the other comments in reply to the
old review).

On Tue, Aug 4, 2026 at 12:20 PM Matti Vaittinen
<[email protected]> wrote:

> Sometimes the existing properties aren't 100% what reviewers would
> prefer. When issues are minor or just cosmetic, changing the existing
> properties is not feasible. Reviewers can't be expected to know which
> properties are new, and which are existing - and this can lead to
> unnecessary review discussion wasting time and energy from everyone.
>
> Adding a common file for re-used ROHM PMIC properties should make it
> clearly visible that a property is re-used, when a new PMIC binding
> refers to this file instead of (re-)describing all the existing
> bindings. This will also help keeping the common properties identical
> across the variants.
>
> Add new file listing commonly used ROHM PMIC properties.

I see the upside of this, the DT binding maintainers will decide
on it. It has the upside you describe, but it has the downside that
new components will always (ab-)use the old properties maybe
even when there would be a good opportunity to modernize
the syntax, and then the reviewers will not catch it.

But well, maybe that is not a big deal in the universe.

What about patching those old bindings:
Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.yaml:
rohm,clkout-open-drain:
Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml:
rohm,clkout-open-drain:
Documentation/devicetree/bindings/mfd/rohm,bd72720-pmic.yaml:
rohm,clkout-open-drain:
...to include this file as part of this patch, and delete the old defines from
those three files?

Then it is clear what is going on.

Maybe this happens in later patches, but anyway it should be part
of this patch I think.

> +properties:
> +  rohm,clkout-open-drain:
> +    description: clk32kout mode. Set to 1 for "open-drain" or 0 for "cmos"
> +      a.k.a "push-pull".
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    maximum: 1

Yeah I see now that this is already in use in three other bindings...

Today we would for a new component use something like
rohm,clkout-drive-mode = "drive-push-pull";
rohm,clkout-drive-mode = "drive-open-drain";
reflecting pin control.

I do understand the idea to keep using the same bindings
for all of Rohm MFDs. But when a completely new design
arrives with many new properties etc, maybe we can think
of something new?

Yours,
Linus Walleij