[PATCH v5 04/12] arm64: dts: rockchip: Rename regulator for pcie2x1l2 for Radxa ROCK 5C

[Krzysztof Kozlowski]

On 22/12/2024 04:18, FUKAUMI Naoki wrote:
> On 12/22/24 05:15, Krzysztof Kozlowski wrote:
>> On 20/12/2024 07:51, FUKAUMI Naoki wrote:
>>> Hi Krzysztof,
>>>
>>> Could you please reply to this email?
>>> (Not for me, but for everyone)
>>
>> You have me how much time... 3 days to reply?
> 
> sorry.
> 
>>> Best regards,
>>>
>>> --
>>> FUKAUMI Naoki
>>> Radxa Computer (Shenzhen) Co., Ltd.
>>>
>>> On 12/17/24 10:11, FUKAUMI Naoki wrote:
>>>> On 12/16/24 22:38, Krzysztof Kozlowski wrote:
>>>>> On 16/12/2024 12:30, FUKAUMI Naoki wrote:
>>>>>> Use consistent name with other regulators. No functional change.
>>>>>>
>>>>>> Fixes: 3ddf5cdb77e6 ("arm64: dts: rockchip: add Radxa ROCK 5C")
>>>>>> Signed-off-by: FUKAUMI Naoki <naoki at radxa.com>
>>>>>> ---
>>>>>> Changes in v5:
>>>>>> - Reword commit message
>>>>>> Changes in v4:
>>>>>> - reword commit message
>>>>>> Changes in v3:
>>>>>> - none
>>>>>> Changes in v2:
>>>>>> - new
>>>>>> ---
>>>>>>    arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts | 6 +++---
>>>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts b/arch/
>>>>>> arm64/boot/dts/rockchip/rk3588s-rock-5c.dts
>>>>>> index 85589d1a6d3b..61d75ab503b2 100644
>>>>>> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts
>>>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts
>>>>>> @@ -76,13 +76,13 @@ pwm-fan {
>>>>>>            pwms = <&pwm3 0 60000 0>;
>>>>>>        };
>>>>>> -    pcie2x1l2_3v3: regulator-pcie2x1l2-3v3 {
>>>>>> +    vcc3v3_pcie2x1l2: regulator-vcc3v3_pcie2x1l2 {
>>>>>
>>>>> No, neither explained, nor correct. See DTS coding style.
>>>>>
>>>>> Please use name for all fixed regulators which matches current format
>>>>> recommendation: 'regulator-[0-9]v[0-9]'
>>>>>
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
>>>>> tree/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml?
>>>>> h=v6.11-rc1#n46
>>>>
>>>> 'regulator-[0-9]v[0-9]' is preferred, but 'regulator-[0-9a-z-]+' is also
>>>> permitted, right?
>>>>
>>>> i.e. regulator-vcc3v3_pcie2x1l2 should be regulator-vcc3v3-pcie2x1l2
>>>>
>>>>
>>>> Or, should we revert below patch and use 'regulator-[0-9]v[0-9]'?
>>>>
>>>>    https://lore.kernel.org/
>>>> all/0ae40493-93e9-40cd-9ca9-990ae064f21a at gmail.com/
>>>>
>>>> Is 'regulator-0v0' valid?
>>
>> Why would it be valid? Can you have regulator with 0 volts?
> 
> There may be a .dtsi that is shared across multiple .dts, so some 
> regulators may not be able to set a specific voltage in the .dtsi.
> 
> How should I describe it?

How fixed voltage regulator can have non-specific voltage? Sorry, that's
impossible. Shared DTSI with a regulator means that regulator is shared,
so it cannot be flexible.


> 
>>>> Is 'regulator-12v0' invalid?
>>
>> Read the binding. I gave you very specific link.
> 
> 46|       - description: Preferred name is 'regulator-[0-9]v[0-9]'
> 47|         pattern: '^regulator(-[0-9]+v[0-9]+|-[0-9a-z-]+)?$'
> 
> The "description" and "pattern" don't match. What you provided is a link 
> to the "description".

It's pretty obvious still...

> 
>>>> How should we handle multiple 1v8/3v3/5v0 regulators?
>>
>> Just add suffix. But usually more than one suffix, vcc+3v3+pcie_2x1l2,
>> means you created a very specific name.
> 
> So shouldn't we refer to the schematic?

So that's the argument you define 10 fixed, uncontrollable regulators in
DTS? What is the benefit of that exactly? Just unnecessary bloat in DTS,
in kernel memory and for probe time?


Best regards,
Krzysztof