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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Krzysztof,

On 2024-12-22 07:27, Krzysztof Kozlowski wrote:
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:
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@xxxxxxxxx>
---
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@xxxxxxxxx/

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?

I'm quite surprised to hear you suggesting that some of the hardware
components should be omitted from a board dts(i) file.  Regardless of
some of the hardware components being fixed or adjustable, they should
all be described, for the sake of transcribing the board schematic into
the board dts(i) files accurately and completely.  Furthermore, some
fixed components may actually be needed in the board dts(i) files; for
example, a fixed regulator may be turned on at boot time, or turned on
and off at runtime.

Technically, we could omit the fixed regulators from the board dts(i)
files, _only_ if they aren't powered on and off at boot time or at
runtime, of course, but many more things could technically be omitted
as well from the dts(i) files if we'd follow that rule, which would
result in a huge per-board inconsistency, leading to a mess.




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux