Re: [PATCH v2 4/6] arm64: dts: rockchip: Add ArmSoM Sige1

Jonas Karlman
2025-07-11T09:56:41Z

Hi,

On 7/11/2025 10:00 AM, Chukun Pan wrote:
> Hi,
>
>> + leds {
>> + compatible = "gpio-leds";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&g_led>, <&r_led>;
>> +
>> + led-green {
>
> Should this label be named as led-0/led-1?

The nodes must include 'led' anywhere in their name according to schema:

"""
patternProperties:
# The first form is preferred, but fall back to just 'led' anywhere in the
# node name to at least catch some child nodes.
"(^led-[0-9a-f]$|led)":
"""

Using the color name similar to the the pin labels in schematics made
most sense to me.

Do you want me to change these to conform to the more restricted
^led-[0-9a-f]$ pattern?

>
>> +&sdio0 {
>> + bus-width = <4>;
>> + cap-sd-highspeed;
>> + cap-sdio-irq;
>> + disable-wp;
>> + keep-power-in-suspend;
>> + mmc-pwrseq = <&sdio_pwrseq>;
>> + no-mmc;
>> + no-sd;
>> + non-removable;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&sdio0_bus4>, <&sdio0_clk>, <&sdio0_cmd>;
>
> This pinctrl is the same as that of rk3528.dtsi, we can remove it.

I will drop the pinctrl props in a v3.

>
> BTW there is a fan connector on the board (connected to pwm6m2),
> so pwm6 should be enabled.

I only try to enable controllers for devices that are properly described,
so I ignored the pwm-fan and its required pwm6 controller. Mostly
because it make more sense to describe the fan once thermal support is
completed, work [1] currently being blocked by a rk356x otp series [2]
and that depends on a rk3576 thermal series [3].

[1] https://github.com/Kwiboo/linux-rockchip/commits/next-20250620-rk3528/
[2] https://lore.kernel.org/r/20250415103203.82972-1-kever.yang@rock-chips.com
[3] https://lore.kernel.org/r/20250610-rk3576-tsadc-upstream-v6-0-b6e9efbf1015@collabora.com

>
>> +&uart2 {
>> + dma-names = "tx", "rx";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&uart2m1_xfer>, <&uart2m1_ctsn>, <&uart2m1_rtsn>;
>> + uart-has-rtscts;
>> + status = "okay";
>> +
>> + bluetooth {
>> + compatible = "brcm,bcm43438-bt";
>> + clocks = <&cru CLK_DEEPSLOW>;
>> + clock-names = "lpo";
>> + device-wakeup-gpios = <&gpio3 RK_PC3 GPIO_ACTIVE_HIGH>;
>
> Is host-wakeup-gpios needed?

The host-wakeup interrupt is described using interrupt* props, not
using the deprecated host-wakeup-gpios prop.

Regards,
Jonas

>
> Thanks,
> Chukun
>
> --
> 2.25.1
>