Re: [PATCH 1/2] dt-bindings: power: Add regulator-pd yaml file

[Krzysztof Kozlowski]

On 28/08/2023 21:09, Shenwei Wang wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
>> Sent: Monday, August 28, 2023 1:53 PM
>> To: Shenwei Wang <shenwei.wang@xxxxxxx>; Ulf Hansson
>> <ulf.hansson@xxxxxxxxxx>
>> Cc: Rob Herring <robh+dt@xxxxxxxxxx>; Krzysztof Kozlowski
>> <krzysztof.kozlowski+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>;
>> Liam Girdwood <lgirdwood@xxxxxxxxx>; Mark Brown <broonie@xxxxxxxxxx>;
>> imx@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
>> dl-linux-imx <linux-imx@xxxxxxx>
>> Subject: [EXT] Re: [PATCH 1/2] dt-bindings: power: Add regulator-pd yaml file
>>>>>>>>> Are you suggesting to move the regulator-pd to the imx directory
>>>>>>>>> and add a company prefix to the compatible string?
>>>>>>>>
>>>>>>>> There is no such part of iMX processor as such
>>>>>>>> regulator-power-domain, so I don't recommend that approach. DTS
>>>>>>>> nodes represent hardware, not your SW layers.
>>>>>>>>
>>>>>>>
>>>>>>> That's not always the case, as we do sometimes need a virtual device.
>>>>>>> As an example, the "regulator-fixed" acts as a software
>>>>>>> abstraction layer to create virtual regulator devices by
>>>>>>> interfacing with the underlying
>>>>>> GPIO drivers.
>>>>>>
>>>>>> Not true. This is a real regulator device. Real hardware on the board.
>>>>>> You can even see and touch it.
>>>>>>
>>>>>
>>>>> The physical hardware component is the GPIO pin, which is what you
>>>>> can only
>>>> touch.
>>>>
>>>> No. The regulator is the chip.
>>>>
>>>
>>> In the definition of dts node below, where is the chip? The real hardware is just
>> a GPIO Pin.
>>>     reg1: regulator-1 {
>>>       compatible = "regulator-fixed";
>>>       regulator-name = "REG1";
>>>       regulator-min-microvolt = <3000000>;
>>>       regulator-max-microvolt = <3000000>;
>>>       gpio = <&lsio_gpio4 19 GPIO_ACTIVE_HIGH>;
>>>       enable-active-high;
>>>     };
>>
>> There is a chip. This is the chip. If you have there only GPIO pin, then your DTS is
>> just wrong. Drop it. If you learn from wrong DTS, then sure, power-domain-
>> regulator seems like great idea...
>>
> 
> When you talk about the chip, can you please be more specific?

What to say more? The device node you quoted above is the regulator. You
brought specific example and now claim this is not a regulator, but just
GPIO. Please fix your DTS.

> 
> Regarding the dts node, how about the example in the fixed-regulator.yaml under the bindings directory.

That's an example, how is it related to anything?

> 
>     reg_1v8: regulator-1v8 {
>       compatible = "regulator-fixed";
>       regulator-name = "1v8";
>       regulator-min-microvolt = <1800000>;
>       regulator-max-microvolt = <1800000>;
>       gpio = <&gpio1 16 0>;
>       startup-delay-us = <70000>;
>       enable-active-high;
>       regulator-boot-on;
>       gpio-open-drain;
>       vin-supply = <&parent_reg>;
>     };
> 
> If you take a look at the fixed regulator driver (fixed.c), I don't think you'll find anything related to a hardware 
> component (chip) other than the GPIO Pin.

That's a driver. How is it related to this discussion? Bindings are not
about drivers.


Best regards,
Krzysztof