Hi,

Am Sonntag, 10. Januar 2021, 16:37:15 CET schrieb Chen-Yu Tsai:

quoted

quoted

+     vcc_sd: sdmmc-regulator {
+             compatible = "regulator-fixed";
+             gpio = <&gpio0 RK_PD6 GPIO_ACTIVE_LOW>;
+             pinctrl-names = "default";
+             pinctrl-0 = <&sdmmc0m1_pin>;

quoted

+             regulator-boot-on;
+             regulator-name = "vcc_sd";

regulator-name above other regulator properties

That is actually what I was used to, but some other rockchip dts files
have all the properties sorted alphabetically. So I stuck with what I
saw.

I try to keep it alphabetical except for the exceptions :-D .

regulator-name is such an exception. Similar to compatibles, the
regulator-name is an entry needed to see if you're at the right node,
so I really like it being the topmost regulator-foo property - just makes
reading easier.

(same for the compatible first, then regs, interrupts parts, as well
as "status-last")

But oftentimes, I just fix the ordering when applying - but seem to have
missed this somewhere in those "other Rockchip dts files" ;-) .

quoted

regulator voltage missing
make things as complete as possible

from fixed-regulator.yaml:

description:
  Any property defined as part of the core regulator binding, defined in
  regulator.yaml, can also be used. However a fixed voltage regulator is
  expected to have the regulator-min-microvolt and regulator-max-microvolt
  to be the same.

However this is not a real regulator; it is merely an on/off switch.
I believe in this case it should just pass through the voltage from
its upstream.

regulator-voltages are not marked required so can stay away if it's just
a dumb switch. I guess it's ok both ways and for individual board-
devicetrees the impact either way is minimal.

quoted

quoted

+&i2c1 {
+     status = "okay";
+
+     rk805: pmic@18 {
+             compatible = "rockchip,rk805";
+             reg = <0x18>;
+             interrupt-parent = <&gpio2>;
+             interrupts = <6 IRQ_TYPE_LEVEL_LOW>;

quoted

+             #clock-cells = <1>;

all thing that start with "#" down the list

Is there a proper "preferred" sorting method defined somewhere?

I struggle with that often as well, but normally I'd do #clocks to clocks
with out "#", but really don't have a hard preference here.

especially as just ignoring the "#" would make #address-cells + #size-cells
look strangely sorted ... so more of a common sense thingy.

quoted

quoted

+             eth_phy_int_pin: eth-phy-int-pin {
+                     rockchip,pins = <1 RK_PD0 RK_FUNC_GPIO &pcfg_pull_down>;
+             };
+
+             eth_phy_reset_pin: eth-phy-reset-pin {
+                     rockchip,pins = <1 RK_PC2 RK_FUNC_GPIO &pcfg_pull_down>;
+             };
+     };
+
+     leds {
+             led_pin: led-pin {
+                     rockchip,pins = <3 RK_PA5 RK_FUNC_GPIO &pcfg_pull_none>;
+             };
+     };
+
+     pmic {
+             pmic_int_l: pmic-int-l {
+                     rockchip,pins = <2 RK_PA6 RK_FUNC_GPIO &pcfg_pull_up>;
+             };
+     };
+

quoted

+     usb3 {

usb

Last numbers in nodenames are more related to the sort order then to
capabillity.
ie: mmc0, mmc1
All usb pin related things here.

I'd say it is more related to functionality in this case, as in "this group
is for USB3 related pins". Makes more sense if the board supported both USB2
and USB3.

I'd agree :-) ... especially as usb controllers on Rockchip boards are not
really numbered and I think we already have precedent for
usb2 -> usb version 2 pins in some other boards ;-)

quoted

quoted

+     cap-sd-highspeed;
+     disable-wp;
+     pinctrl-names = "default";
+     pinctrl-0 = <&sdmmc0_clk>, <&sdmmc0_cmd>, <&sdmmc0_dectn>, <&sdmmc0_bus4>;
+     vmmc-supply = <&vcc_sd>;
+     status = "okay";
+};
+

quoted

+&saradc {
+     vref-supply = <&vcc_18>;
+     status = "okay";
+};

What happened to the recovery key from the schematic?

I believe I originally planned on adding it, but failed to find a proper
key event for it. Any suggestions?

Most boards seem to use the KEY_VENDOR keycode.


Heiko