On 11/16/23 2:33 PM, Heiko Stuebner wrote:
Am Donnerstag, 16. November 2023, 21:23:20 CET schrieb Krzysztof Kozlowski:
On 16/11/2023 21:03, Heiko Stuebner wrote:
I guess the only thing I do have questions about is the part
+4. All properties with values
+5. Boolean properties
Is there a rationale for it? Because with it things like regulator-*
properties then end up in two different blocks.
Good point. It is only a matter of style that this:
foo {
compatible = "foo";
reg = <0x1>;
clocks = <&clk>;
wakeup-source;
key-autorepeat;
}
looks better to me than:
foo {
compatible = "foo";
reg = <0x1>;
key-autorepeat;
wakeup-source;
clocks = <&clk>;
}
But you have good point that similar properties should be usually
grouped together.
About which regulator properties are you thinking now? You mean the
supplies or the provider?
I was thinking about the provider. There are
regulator-min-microvolt = <>;
and friends, but also
regulator-boot-on;
These are in regulator provider nodes and above guideline would keep
logical order:
regulator-name = "vdd_kfc";
regulator-min-microvolt = <800000>;
regulator-max-microvolt = <1500000>;
regulator-always-on;
regulator-boot-on;
regulator-state-mem {
regulator-off-in-suspend;
};
What exactly would be here misordered?
going with the vcc5v0_host regulator of the rk3588-quartzpro64 and
+1. compatible
+2. reg
+3. ranges
+4. All properties with values
+5. Boolean properties
+6. status (if applicable)
+7. Child nodes
we'd end up with
vcc5v0_host: vcc5v0-host-regulator {
/* 1. */ compatible = "regulator-fixed";
/* 4. */ gpio = <&gpio4 RK_PB0 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
pinctrl-0 = <&vcc5v0_host_en>;
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
regulator-name = "vcc5v0_host";
vin-supply = <&vcc5v0_usb>;
/* 5. */ enable-active-high;
regulator-always-on;
regulator-boot-on;
};
How about grouping like properties (defined in the same schema),
then sorting within that group. Would also allow for defining
where to add spacing.
1. compatible
2. reg
3. ranges
4. All property groups
4.1 Properties with values
4.2 Boolean properties
4.3 Separating space
6. status (if applicable)
7. Child nodes
Your node then would look like we expect:
vcc5v0_host: vcc5v0-host-regulator {
/* 1 */ compatible = "regulator-fixed";
/* 4.1 */ pinctrl-names = "default";
/* 4.1 */ pinctrl-0 = <&vcc5v0_host_en>;
/* 4.3 */
/* 4.1 */ regulator-min-microvolt = <5000000>;
/* 4.1 */ regulator-max-microvolt = <5000000>;
/* 4.1 */ regulator-name = "vcc5v0_host";
/* 4.2 */ regulator-always-on;
/* 4.2 */ regulator-boot-on;
/* 4.2 */ enable-active-high;
/* 4.3 */
/* 4.1 */ gpio = <&gpio4 RK_PB0 GPIO_ACTIVE_HIGH>;
...
};
Andrew
which I find somewhat counter-intuitive ;-) .
Heiko
