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

FUKAUMI Naoki
2024-12-22T03:20:12Z

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@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@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?

>>> 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".

>>> 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?

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

> Best regards,
> Krzysztof