[PATCH] arm64: dts: rockchip: convert blue LED to "pwd-leds" for Radxa ROCK 5A/5C

Dragan Simic dsimic at manjaro.org
Mon Dec 9 08:21:59 PST 2024


Hello Fukaumi,

On 2024-12-09 14:41, FUKAUMI Naoki wrote:
> convert blue(heartbeat) LED from "gpio-leds" to "pwm-leds". user can
> change brightness.

Please, start all sentences with uppercase letters, because when
there are periods (i.e. the '.' characters), there should also be
matching starting uppercase letters.  As we know, that basically
defines the primary syntax of a sentence in English.

I'm sorry if I'm stating the obvious, but this patch description
simply doesn't read well.

> Signed-off-by: FUKAUMI Naoki <naoki at radxa.com>

I haven't checked the entire patch in detail, but as the blue LED
(a.k.a. the IO_LED) on the ROCK 5A and 5C is powered by a SoC pin
capable of generating PWM output, according to the board schematics
that I checked, making it possible to adjust the LED brightness
makes sense to me, and describes the hardware a bit better.

Thus, please feel free to include my:

Acked-by: Dragan Simic <dsimic at manjaro.org>

... and please send a v2 with the patch description improved as
already suggested above.  That suggestion also applies to pretty
much all other patches that you sent recently.

> ---
> this patch depends on [1] which depends on [2].
> 
> [1] 
> https://patchwork.kernel.org/project/linux-rockchip/cover/20241209132406.4232-1-naoki@radxa.com/
> [2] 
> https://patchwork.kernel.org/project/linux-rockchip/cover/20241209125131.4101-1-naoki@radxa.com/
> ---
>  .../boot/dts/rockchip/rk3588s-rock-5.dtsi     | 34 ++++++++++++-------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
> index d0b9513d56a7..d72314d917da 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
> @@ -46,7 +46,7 @@ hdmi0_con_in: endpoint {
>  	leds {
>  		compatible = "gpio-leds";
>  		pinctrl-names = "default";
> -		pinctrl-0 = <&led_pins>;
> +		pinctrl-0 = <&led_pin>;
> 
>  		led-0 {
>  			color = <LED_COLOR_ID_GREEN>;
> @@ -54,14 +54,6 @@ led-0 {
>  			function = LED_FUNCTION_POWER;
>  			gpios = <&gpio3 RK_PC4 GPIO_ACTIVE_HIGH>;
>  		};
> -
> -		led-1 {
> -			color = <LED_COLOR_ID_BLUE>;
> -			default-state = "on";
> -			function = LED_FUNCTION_STATUS;
> -			gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
> -			linux,default-trigger = "heartbeat";
> -		};
>  	};
> 
>  	fan: pwm-fan {
> @@ -72,6 +64,19 @@ fan: pwm-fan {
>  		pwms = <&pwm3 0 60000 0>;
>  	};
> 
> +	pwm-leds {
> +		compatible = "pwm-leds";
> +
> +		led-1 {
> +			color = <LED_COLOR_ID_BLUE>;
> +			default-state = "on";
> +			function = LED_FUNCTION_STATUS;
> +			linux,default-trigger = "heartbeat";
> +			pwms = <&pwm11 0 1000000 0>;
> +			max-brightness = <255>;
> +		};
> +	};
> +
>  	vbus_typec: regulator-vbus-typec {
>  		compatible = "regulator-fixed";
>  		regulator-name = "vbus_typec";
> @@ -422,9 +427,8 @@ &pcie2x1l2 {
> 
>  &pinctrl {
>  	leds {
> -		led_pins: led-pins {
> -			rockchip,pins = <3 RK_PC4 RK_FUNC_GPIO &pcfg_pull_none>,
> -					<3 RK_PD5 RK_FUNC_GPIO &pcfg_pull_none>;
> +		led_pin: led-pin {
> +			rockchip,pins = <3 RK_PC4 RK_FUNC_GPIO &pcfg_pull_none>;
>  		};
>  	};
> 
> @@ -467,6 +471,12 @@ &pwm3 {
>  	status = "okay";
>  };
> 
> +&pwm11 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pwm11m3_pins>;
> +	status = "okay";
> +};
> +
>  &saradc {
>  	vref-supply = <&vcca_1v8_s0>;
>  	status = "okay";



More information about the Linux-rockchip mailing list