Re: [PATCH 2/2] arm: dts: rockchip: turn LEDs on at boot for Radxa boards

[Dragan Simic <dsimic@manjaro.org>]

Hello Fukaumi,

Please see a comment below.

On 2024-08-08 11:27, FUKAUMI Naoki wrote:
> Basically, Radxa's boards are intended to turn LEDs on at U-Boot.
> add `default-state = "on"` to u-boot.dtsi.
> 
> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
> ---
>  .../arm/dts/rk3328-rock-pi-e-base-u-boot.dtsi |  6 +++++
>  arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi  | 25 ++++++++++++++++++-
>  arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi     |  8 ++++++
>  arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi  |  8 ++++++
>  arch/arm/dts/rk3568-radxa-e25-u-boot.dtsi     | 23 +++++++++++++++++
>  arch/arm/dts/rk3568-rock-3a-u-boot.dtsi       |  8 ++++++
>  arch/arm/dts/rk3588-rock-5b-u-boot.dtsi       |  8 ++++++
>  arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi      |  8 ++++++
>  8 files changed, 93 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/dts/rk3328-rock-pi-e-base-u-boot.dtsi
> b/arch/arm/dts/rk3328-rock-pi-e-base-u-boot.dtsi
> index 39bb66c4fcb..a30adb0be98 100644
> --- a/arch/arm/dts/rk3328-rock-pi-e-base-u-boot.dtsi
> +++ b/arch/arm/dts/rk3328-rock-pi-e-base-u-boot.dtsi
> @@ -6,6 +6,12 @@
>  #include "rk3328-u-boot.dtsi"
> 
>  / {
> +	leds {
> +		led-0 {
> +			default-state = "on";
> +		};
> +	};
> +
>  	smbios {
>  		compatible = "u-boot,sysinfo-smbios";
> 
> diff --git a/arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi
> b/arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi
> index 5ec15a845c1..614e4ce0fa8 100644
> --- a/arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi
> +++ b/arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi
> @@ -2,7 +2,20 @@
>  /*
>   * Copyright (c) 2023 Radxa Limited
>   */
> -#include "rk3399-rock-pi-4-u-boot.dtsi"
> +#include "rk3399-u-boot.dtsi"
> +#include "rk3399-sdram-lpddr4-100.dtsi"
> +
> +/ {
> +	leds {
> +		led-0 {
> +			default-state = "on";
> +		};
> +
> +		led-1 {
> +			default-state = "on";
> +		};
> +	};
> +};
> 
>  &pcfg_pull_none_18ma {
>  	bootph-pre-ram;
> @@ -14,6 +27,12 @@
>  	bootph-some-ram;
>  };
> 
> +&sdhci {
> +	cap-mmc-highspeed;
> +	mmc-ddr-1_8v;
> +	mmc-hs200-1_8v;
> +};
> +

This change should be taken out of the patch, because it doesn't
match the patch description.  If the change is still needed, it
should be submitted as a separate patch.

>  &spi1 {
>  	status = "okay";
> 
> @@ -25,3 +44,7 @@
>  		spi-max-frequency = <10000000>;
>  	};
>  };
> +
> +&vdd_log {
> +	regulator-init-microvolt = <950000>;
> +};
> diff --git a/arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi
> b/arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi
> index b3bfc77f756..4861574636e 100644
> --- a/arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi
> +++ b/arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi
> @@ -6,6 +6,14 @@
>  #include "rk3399-u-boot.dtsi"
>  #include "rk3399-sdram-lpddr4-100.dtsi"
> 
> +/ {
> +	leds {
> +		led-0 {
> +			default-state = "on";
> +		};
> +	};
> +};
> +
>  &sdhci {
>  	cap-mmc-highspeed;
>  	mmc-ddr-1_8v;
> diff --git a/arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi
> b/arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi
> index e0e501deccf..412f9e06aff 100644
> --- a/arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi
> +++ b/arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi
> @@ -5,6 +5,14 @@
> 
>  #include "rk356x-u-boot.dtsi"
> 
> +/ {
> +	leds {
> +		led-1 {
> +			default-state = "on";
> +		};
> +	};
> +};
> +
>  &sdhci {
>  	cap-mmc-highspeed;
>  };
> diff --git a/arch/arm/dts/rk3568-radxa-e25-u-boot.dtsi
> b/arch/arm/dts/rk3568-radxa-e25-u-boot.dtsi
> index 74755a44eae..efeab626d8a 100644
> --- a/arch/arm/dts/rk3568-radxa-e25-u-boot.dtsi
> +++ b/arch/arm/dts/rk3568-radxa-e25-u-boot.dtsi
> @@ -2,6 +2,29 @@
> 
>  #include "rk356x-u-boot.dtsi"
> 
> +/ {
> +	gpio-leds {
> +		led-0 {
> +			default-state = "on";
> +		};
> +
> +		led-red {
> +			default-state = "on";
> +			gpios = <&gpio0 RK_PC0 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		led-green {
> +			default-state = "on";
> +			gpios = <&gpio0 RK_PC1 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		led-blue {
> +			default-state = "on";
> +			gpios = <&gpio4 RK_PC5 GPIO_ACTIVE_HIGH>;
> +		};
> +	};
> +};
> +
>  &pcie3x1 {
>  	pinctrl-0 = <&pcie30x1_reset_h>;
>  };
> diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi
> b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi
> index 9d18f5d0b36..c493352abc1 100644
> --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi
> +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi
> @@ -6,6 +6,14 @@
> 
>  #include "rk356x-u-boot.dtsi"
> 
> +/ {
> +	leds {
> +		led-0 {
> +			default-state = "on";
> +		};
> +	};
> +};
> +
>  &pcie3x2 {
>  	pinctrl-0 = <&pcie3x2_reset_h>;
>  };
> diff --git a/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
> b/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
> index 8e318e624a8..8ebba36f90a 100644
> --- a/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
> +++ b/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
> @@ -5,6 +5,14 @@
> 
>  #include "rk3588-u-boot.dtsi"
> 
> +/ {
> +	leds {
> +		led_rgb_b {
> +			default-state = "on";
> +		};
> +	};
> +};
> +
>  &fspim2_pins {
>  	bootph-pre-ram;
>  	bootph-some-ram;
> diff --git a/arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi
> b/arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi
> index efba0c359ba..a75ae296386 100644
> --- a/arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi
> +++ b/arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi
> @@ -5,6 +5,14 @@
> 
>  #include "rk3588s-u-boot.dtsi"
> 
> +/ {
> +	leds {
> +		io-led {
> +			default-state = "on";
> +		};
> +	};
> +};
> +
>  &sdhci {
>  	cap-mmc-highspeed;
>  	mmc-hs200-1_8v;