| Message ID | 20241026100310.52679-4-honyuenkwun@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: rockchip: Add Orange Pi 5 Max board | expand |
On 26/10/2024 11:48, Jimmy Hon wrote: > Describe the Orange Pi 5 Max 2 status LEDs in its device tree. > > Signed-off-by: Jimmy Hon <honyuenkwun@gmail.com> > --- > .../dts/rockchip/rk3588-orangepi-5-max.dts | 35 +++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts > index 53a34cb37487..83a118e52bb0 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts This is a new board. Why do you add incomplete board in first patch? Best regards, Krzysztof
On Sat, Oct 26, 2024 at 12:59 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 26/10/2024 11:48, Jimmy Hon wrote: > > Describe the Orange Pi 5 Max 2 status LEDs in its device tree. > > > > Signed-off-by: Jimmy Hon <honyuenkwun@gmail.com> > > --- > > .../dts/rockchip/rk3588-orangepi-5-max.dts | 35 +++++++++++++++++++ > > 1 file changed, 35 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts > > index 53a34cb37487..83a118e52bb0 100644 > > --- a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts > > +++ b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts > > This is a new board. Why do you add incomplete board in first patch? That's an artifact of the development process. It's easier for git to keep track of which nodes in the DTS goes with which function. I originally started this 6-weeks ago. But the schematic was only published this week. So I was hoping the Orange Pi 5 Max was a subset of the Orange Pi 5 Plus (which both use full RK3588). However, I was wrong, a lot of pins are different. I compared the differences in the vendor kernel between the two boards [1][2], and applied it to the mainline Orange Pi 5 Plus dts [3] So in the process of trimming the nodes down to the ones actually used by the OPI5Max,I got it down to the first patch and had a working board using microSD. For the LED functionality in particular. I originally just used leds-gpio like the vendor kernel [2]. However, when the schematic came out [4] page 26, I noticed the pins were called PWM_LED1 and PWM_LED2, so I did the same thing as the mainline OPI5Plus, and used leds-pwm. Luckily, PWM4 and PWM5 are not available from the 40-pin GPIO header. [5] So there's not much downside. On other boards, the PWM used to drive the LED could have been used to drive a pin on the GPIO header. But basically, if I was told it would be better to revert back to using gpio-leds like the vendor kernel, it was simpler to keep track of the nodes in the same commit. Side Note: since it's actually an RGB led, and not three individual leds, it's tempting to use the leds-pwm-multicolor. However, the RED portion is hardwired on when the power is on and not connected to PWM. I still have some more functions that I'm working on (i.e. WIFi), so those commits are not ready. I'm afraid that the Android BCMDHD driver may need DTS nodes that are different from the DTS nodes the bcrmfmac driver would use. Anyways, should I squash the commits together in the new revision? I was able to test the majority of the functionality, I don't have the additional hardware to test fan, rtc, and eMMC. [1] https://github.com/orangepi-xunlong/linux-orangepi/blob/orange-pi-6.1-rk35xx/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts#L18-L37 [2] https://github.com/orangepi-xunlong/linux-orangepi/blob/orange-pi-6.1-rk35xx/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts#L394-L413 [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts#n96 [4] http://www.orangepi.org/html/hardWare/computerAndMicrocontrollers/service-and-support/Orange-Pi-5-Max.html [5] http://www.orangepi.org/img/pi5-max/max-15.png > > Best regards, > Krzysztof > Jimmy
On 26/10/2024 22:17, Jimmy Hon wrote: > leds, it's tempting to use the leds-pwm-multicolor. However, the RED > portion is hardwired on when the power is on and not connected to PWM. > > I still have some more functions that I'm working on (i.e. WIFi), so > those commits are not ready. I'm afraid that the Android BCMDHD driver > may need DTS nodes that are different from the DTS nodes the bcrmfmac > driver would use. > > Anyways, should I squash the commits together in the new revision? I > was able to test the majority of the functionality, I don't have the > additional hardware to test fan, rtc, and eMMC. That's what I would expect. Look at qcom commits. First publish of new SoC is one commit, new board is another one. Feel free to grow it later (release early, release often), but that's not the case here. Best regards, Krzysztof
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts index 53a34cb37487..83a118e52bb0 100644 --- a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts +++ b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts @@ -3,6 +3,7 @@ /dts-v1/; #include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/leds/common.h> #include <dt-bindings/pinctrl/rockchip.h> #include "rk3588.dtsi" @@ -19,6 +20,28 @@ chosen { stdout-path = "serial2:1500000n8"; }; + pwm-leds { + compatible = "pwm-leds"; + + blue_led: led-1 { + color = <LED_COLOR_ID_BLUE>; + function = LED_FUNCTION_STATUS; + linux,default-trigger = "heartbeat"; + max-brightness = <255>; + /* PWM_LED1 */ + pwms = <&pwm4 0 25000 0>; + }; + + green_led: led-2 { + color = <LED_COLOR_ID_GREEN>; + function = LED_FUNCTION_STATUS; + linux,default-trigger = "heartbeat"; + max-brightness = <255>; + /* PWM_LED2 */ + pwms = <&pwm5 0 25000 0>; + }; + }; + /* PMIC_EXT_EN */ vcc_1v1_nldo_s3: vcc-1v1-ndlo-s3-regulator { compatible = "regulator-fixed"; @@ -127,6 +150,18 @@ regulator-state-mem { }; }; +&pwm4 { + pinctrl-names = "default"; + pinctrl-0 = <&pwm4m0_pins>; + status = "okay"; +}; + +&pwm5 { + pinctrl-names = "default"; + pinctrl-0 = <&pwm5m1_pins>; + status = "okay"; +}; + &saradc { vref-supply = <&vcca_1v8_s0>; status = "okay";
Describe the Orange Pi 5 Max 2 status LEDs in its device tree. Signed-off-by: Jimmy Hon <honyuenkwun@gmail.com> --- .../dts/rockchip/rk3588-orangepi-5-max.dts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+)