Re: [PATCH 2/8] dt-bindings: mfd: ROHM BD73800 PMIC

Hi dee Ho Linus,

The way too short summer holiday is now gone, so I am back at this :) 
Thanks again for the comments, I am trying to improve for v2 ;)

On 03/07/2026 23:46, Linus Walleij wrote:
> On Wed, Jul 1, 2026 at 2:41 PM Matti Vaittinen
> <[email protected]> wrote:
> 
>> +  # The GPIO1, CLKOUT (GPIO2), FAULT_B and EXTEN_OUT pins can be
>> +  # configured to interrupt pins by OTP.
> 
> Maybe move this helpful comment into the top description: instead?
> It's kind of generic helpful info.
> 
>> +# The GPIO1, CLKOUT, FAULT_B and EXTEN_OUT pins may be configured for a
>> +# specific purpose (like ADC input, 32.768 clk output, fault indicator or
>> +# delivering power sequence to a companion PMIC when multiple PMICs are
>> +# used) - but also to be either a GPO or GPI. (When used as a GPI the pin
>> +# can also be used as an IRQ source). The pin purpose is determined by
>> +# OTP (One Time Programmable memory), typically during device manufacturing.
>> +# The OTP can't be read at runtime so device-tree should describe the pins.
>> +  rohm,pin-gpio1:
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    description:
>> +      Indicate if the GPIO1 pin has been set to GPI or GPO at manufacturing.
>> +    enum: [gpi, gpo]
>> +
>> +  rohm,pin-clkout:
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    description:
>> +      Indicate if the CLKOUT pin has been set to GPI or GPO at manufacturing.
>> +    enum: [gpi, gpo]
>> +
>> +  rohm,pin-fault-b:
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    description:
>> +      Indicate if the FAULT_B pin has been set to GPI or GPO at manufacturing.
>> +    enum: [gpi, gpo]
>> +
>> +  rohm,pin-exten:
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    description:
>> +      Indicate if the EXTEN_OUT pin has been set to GPI or GPO at
>> +      manufacturing.
>> +    enum: [gpi, gpo]
> 
> Can we explain what "GPI" and "GPO" means in this context?
> 
> I read it as "general purpose input" and "general purpose output", but...
> you just describe the exact purpose? So what is "general purpose"
> about them in that case?

These property names (pin-gpio1, pin-clkout, pin-fault-b, pin-exten) do 
not define the purpose of the pin, but they match the pin name in the 
data-sheet. The idea is indeed to be able to say "the fault-b -pin is 
not a fault signal, but a general purpose input" - if the IC we are 
describing here has OTP configuration enabling this.

I am re-using the approach from the BD72720 here. I think I will add a 
common binding file with these, which can then be referred by multiple 
rohm ICs (in same fashion I added the 
Documentation/devicetree/bindings/regulator/rohm,pmic-states.yaml for 
commonly used ROHM regulator properties). I'll see if it looks Ok (to 
me), and send it in v2 :)

> I would re-use "input-enable" and "output-enable" from:
> Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
> (I mean don't $rf that, just use these strings).
> 
> I suppose:
> enum: [input-enable, output-enable]
> 
>> +  rohm,clkout-open-drain:
>> +    description: clk32kout mode. Set to 1 for "open-drain" or 0 for "cmos".
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 0
>> +    maximum: 1
> 
> Here I would also reuse the generic pinconf properties,
> something like;
> 
> rohm,clkout-drive-type:
>    enum: [drive-push-pull, drive-open-drain]

As I mentioned in my very hasty original reply, this is also an existing 
binding used in quite a few PMIC device-trees. Changing it now sounds 
like asking for problems, for (in my opinion) little benefit. Yet, since 
it is used by a few PMICs, I could perhaps put it in a common rohm 
binding file as well. Then it would be more obvious it is an existing 
property if new models re-use this.

> (Push-pull is what is colloquially referred to as "cmos".)

I will at least add a.k.a "push-pull" to the description :)

> 
>> +            rohm,pin-gpio1 = "gpo";
>> +            rohm,pin-exten = "gpi";
> 
> If you instead use nodes with properties you can do this:
> 
> rohm,pin-clkout {
>      output-enable;
>      drive-push-pull;
> };
> 
> This collects the clkout config in one place and make
> it obvious what is going on. But I don't know what the DT
> maintainers think about this idea.

I believe you mean I could translate:

rohm,pin-clkout = "gpo";
rohm,clkout-open-drain = <0>;

to
rohm,pin-clkout {
	output-enable;
	drive-push-pull;
};
right?

I am actually not sure if this would work. The data-sheet made me to 
assume it might not. There is separate "OUT32K" register, which controls 
the clock gate. This, as far as I understand, is not usable when the OTP 
variant sets the CLKOUT -pin to GPO. The mode (open-drain / cmos) 
configuration resides in this clock gate register.

[Just to complete picture, when OTP is set to GPO, the pin output is 
controlled by GPIO_OUT register instead. When OTP has set the pin to clk 
output (or to GPI), then the GPIO_OUT register writes have no impact.]

Thus, in case of the BD73800, the:

rohm,pin-clkout = "gpo";
rohm,clkout-open-drain = <0>;

might actually be contradicting.

...

Now, I will make a side-note... The data-sheet front page says:
> 4 GPIOs
> - OTP Option for GPIOs instead of I/F signals:
> EXTEN_OUT, FAULT_B, CLKOUT, GPIO1
> - GPO Supports Open Drain and CMOS Output

I, however, see no register control for the GPO output types. I _assume_ 
the output type (when pins are used for GPO) depends on OTP again. I 
will see if I can clarify this.

Yours,
	-- Matti

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

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