| Message ID | 20250307120004.959980-1-amadeus@jmu.edu.cn (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm64: dts: rockchip: Add pwm nodes for RK3528 | expand |
On Fri, 07 Mar 2025 20:00:02 +0800, Chukun Pan wrote: > Add pwm nodes for RK3528. Most rk3528 boards use pwm-regulator to > supply to CPU, add node to enable them. The PWM core on RK3528 is > the same as RK3328, but the driver doesn't support interrupts yet. > > Unlike other SoCs, pinctrl-names need to be in "active" state, > I'm not sure about this, but otherwise the pwm-regulator will > not work properly. > > [...] Applied, thanks! [2/2] arm64: dts: rockchip: Add pwm nodes for RK3528 commit: 2973d077aedfc114affab96c3b2c7286163cc8c9 Best regards,
Hi Heiko, On 2025-03-12 08:45, Heiko Stuebner wrote: > > On Fri, 07 Mar 2025 20:00:02 +0800, Chukun Pan wrote: >> Add pwm nodes for RK3528. Most rk3528 boards use pwm-regulator to >> supply to CPU, add node to enable them. The PWM core on RK3528 is >> the same as RK3328, but the driver doesn't support interrupts yet. >> >> Unlike other SoCs, pinctrl-names need to be in "active" state, >> I'm not sure about this, but otherwise the pwm-regulator will >> not work properly. >> >> [...] > > Applied, thanks! The pinctrl-names should be changed to "default" and not "active", something you can fixup or do you want a patch? In commit 96d8d3253246 ("arm64: dts: rockchip: Fix PWM pinctrl names") similar issue was corrected for current rockchip boards. Regards, Jonas > > [2/2] arm64: dts: rockchip: Add pwm nodes for RK3528 > commit: 2973d077aedfc114affab96c3b2c7286163cc8c9 > > Best regards,
Am Mittwoch, 12. März 2025, 16:00:00 MEZ schrieb Jonas Karlman: > Hi Chukun, > > On 2025-03-12 15:35, Chukun Pan wrote: > > Hi, > > > >> The pinctrl-names should be changed to "default" and not "active", > >> something you can fixup or do you want a patch? so yes of course the pinctrl needs to be default - simply because that's the only pinctrl state mainline supports. But judging by the fact that you're discussing working vs. non-working below, can you please check if we should drop the patch for 6.15 till that is solved? Thanks a lot Heiko > > Sorry I've been a bit busy this week and forgot to send the v2 patch. > > In rk3528.dtsi, the uart and upcoming i2c nodes do not have pinctrl, > > so I prefer to remove them. > > > >>> Unlike other SoCs, pinctrl-names need to be in "active" state, > >>> I'm not sure about this, but otherwise the pwm-regulator will > >>> not work properly. > > > > BTW, setting the pinctrl of pwm corresponding to pwm-regulator > > to "default" will cause kernel boot suspended. > > Sorry but do you know why? > > Not an issue I have seen, do you have any more logs or details? E.g. > what board you use, full regulator node, do you have operating points > defined etc. > > I have runtime tested a branch at [1], that use pinctrl-names = default, > have vdd_arm and vdd_logic defined, also an opp table for cpu and gpu. > > For E20C there is a commit to enable the vdd_logic, however without gpu > enabled and a mali-supply the pwm-regulator is initialized to > max-microvolt by Linux. Have instead updated U-Boot to initialize the > pwm-regulator's: > > ``` > &vdd_arm { > regulator-init-microvolt = <953000>; > }; > > &vdd_logic { > regulator-init-microvolt = <900000>; > }; > ``` > > [1] https://github.com/Kwiboo/linux-rockchip/commits/next-20250311-rk3528/ > > Regards, > Jonas > > > > > e.g. > > ``` > > vdd_arm: regulator-vdd-arm { > > compatible = "pwm-regulator"; > > pwms = <&pwm1 0 5000 1>; > > ... > > }; > > > > &pwm1 { > > pinctrl-0 = <&pwm1m0_pins>; > > pinctrl-names = "default"; > > status = "okay"; > > }; > > ``` > > > > Thanks, > > Chukun > > > > -- > > 2.25.1 > > > >
Hi, > so yes of course the pinctrl needs to be default - simply because > that's the only pinctrl state mainline supports. > But judging by the fact that you're discussing working vs. non-working > below, can you please check if we should drop the patch for 6.15 till > that is solved? I suggest dropping this patch first, I will send v2 when this issue is solved. Thanks, Chukun -- 2.25.1
Hi, On 2025-03-13 08:10, Chukun Pan wrote: > Hi, > >> so yes of course the pinctrl needs to be default - simply because >> that's the only pinctrl state mainline supports. > >> But judging by the fact that you're discussing working vs. non-working >> below, can you please check if we should drop the patch for 6.15 till >> that is solved? > > I suggest dropping this patch first, I will send v2 > when this issue is solved. I noticed that the pwm nodes and saradc node ended up in wrong mmio address order, so yes please drop the patch (or reorder nodes) ;-) Chukun, please get back with more details about the issue you are having. E.g. a stack trace, what board, DT and U-Boot you are using etc. I have not seen any issue with PWM using the merged patch having pinctrl-names=default. Please see my Linux tree [1] and U-Boot tree [2], those are little ahead of what has been posted on ML, e.g. it has working USB2.0 host, CPU opp, Hantro VPU, GPU + opp, arm and logic pwm regulators for E20C, ROCK 2A/2F and Sige1. Will try to post USB2.0 host and Hantro VPU patches later today. CPU opp and GPU + opp depend on PWM and I was planning to post that together with initial thermal support. [1] https://github.com/Kwiboo/linux-rockchip/commits/next-20250311-rk3528/ [2] https://source.denx.de/u-boot/contributors/kwiboo/u-boot/-/commits/rk3528 Regards, Jonas > > Thanks, > Chukun > > -- > 2.25.1 >
Hi, > I have not seen any issue with PWM using the merged patch having > pinctrl-names=default. > > Please see my Linux tree [1] and U-Boot tree [2], those are little ahead > of what has been posted on ML, e.g. it has working USB2.0 host, CPU opp, > Hantro VPU, GPU + opp, arm and logic pwm regulators for E20C, ROCK 2A/2F > and Sige1. > > Please see my Linux tree [1] and U-Boot tree [2], those are little ahead > of what has been posted on ML, e.g. it has working USB2.0 host, CPU opp, > Hantro VPU, GPU + opp, arm and logic pwm regulators for E20C, ROCK 2A/2F > and Sige1. > ... > [1] https://github.com/Kwiboo/linux-rockchip/commits/next-20250311-rk3528/ > [2] https://source.denx.de/u-boot/contributors/kwiboo/u-boot/-/commits/rk3528 I tested your kernel device tree on E20C earlier today and still have the same issue. But if I replace u-boot with the link [2] you provided, it will work fine. For reference, I was using v2025.01 plus this series of patches [3]. So it looks like u-boot does something and kernel doesn't? [3] https://lore.kernel.org/u-boot/20250123224844.3104592-1-jonas@kwiboo.se/ Thanks, Chukun -- 2.25.1
Hi, On 2025-03-13 10:01, Chukun Pan wrote: > Hi, > >> I have not seen any issue with PWM using the merged patch having >> pinctrl-names=default. >> >> Please see my Linux tree [1] and U-Boot tree [2], those are little ahead >> of what has been posted on ML, e.g. it has working USB2.0 host, CPU opp, >> Hantro VPU, GPU + opp, arm and logic pwm regulators for E20C, ROCK 2A/2F >> and Sige1. >> >> Please see my Linux tree [1] and U-Boot tree [2], those are little ahead >> of what has been posted on ML, e.g. it has working USB2.0 host, CPU opp, >> Hantro VPU, GPU + opp, arm and logic pwm regulators for E20C, ROCK 2A/2F >> and Sige1. >> ... >> [1] https://github.com/Kwiboo/linux-rockchip/commits/next-20250311-rk3528/ >> [2] https://source.denx.de/u-boot/contributors/kwiboo/u-boot/-/commits/rk3528 > > I tested your kernel device tree on E20C earlier today and still have > the same issue. But if I replace u-boot with the link [2] you provided, > it will work fine. For reference, I was using v2025.01 plus this series > of patches [3]. So it looks like u-boot does something and kernel doesn't? Interesting, good that it works with the updated U-Boot. Main change compared to v1 is that it now use clock/reset id and DT closer to what has been merged in mainline Linux. It also has DT params to help initialize the two pwm regulators used by these boards. I will try with the old v1 U-Boot series and see if I can replicated your issue. Regards, Jonas > > [3] https://lore.kernel.org/u-boot/20250123224844.3104592-1-jonas@kwiboo.se/ > > Thanks, > Chukun > > -- > 2.25.1 >
Hi, > Interesting, good that it works with the updated U-Boot. Main change > compared to v1 is that it now use clock/reset id and DT closer to what > has been merged in mainline Linux. It also has DT params to help > initialize the two pwm regulators used by these boards. > > I will try with the old v1 U-Boot series and see if I can replicated > your issue. It is easy to reproduce this issue. Make the following changes in the new series of u-boot: ``` --- a/configs/radxa-e20c-rk3528_defconfig +++ b/configs/radxa-e20c-rk3528_defconfig @@ -47,9 +47,7 @@ CONFIG_DM_MDIO=y CONFIG_DWC_ETH_QOS=y CONFIG_DWC_ETH_QOS_ROCKCHIP=y CONFIG_PHY_ROCKCHIP_INNO_USB2=y -CONFIG_REGULATOR_PWM=y CONFIG_DM_REGULATOR_GPIO=y -CONFIG_PWM_ROCKCHIP=y CONFIG_BAUDRATE=1500000 CONFIG_DEBUG_UART_SHIFT=2 CONFIG_SYS_NS16550_MEM32=y ``` Or change dts: ``` --- a/arch/arm/dts/rk3528-radxa-e20c-u-boot.dtsi +++ b/arch/arm/dts/rk3528-radxa-e20c-u-boot.dtsi @@ -8,9 +8,9 @@ }; &vdd_arm { - regulator-init-microvolt = <953000>; + status = "disabled"; }; &vdd_logic { - regulator-init-microvolt = <900000>; + status = "disabled"; }; ``` Then the kernel will hang when loading the gpio driver: [ 0.162618] gpio gpiochip2: Static allocation of GPIO bae is deprecated, use dynamic allocation. [ 0.163558] rockchip-gpio ffb00000.gpio: probed /soc/pinctrl/gpio@ffb00000 [ 0.164322] gpio gpiochip3: Static allocation of GPIO base is deprecated, use dynamic allocation. [ 0.165231] rockchip-gpio ffb10000.gpio: probed /soc/pinctrl/gpio@ffb10000 [ 0.165977] gpio gpiochip4: Static allocation of GPIO base is deprecated, use dynamic allocation. [ 0.166886] rockhip-gpio ffb20000.gpio: probed /soc/pinctrl/gpio@ffb20000 [ 0.170342] Internal error: Oos - Undefined instruction: 0000000002000000 [#1] SMP Changing the debug level: [ 0.175260] rockchip-pinctrl soc:pinctrl: setting mux of GPIO4-14 to 0 [ 0.175356] rockchip-pinctrl soc:pinctrl: setting mux of GPIO4-20 to 1 [ 0.175968] rockchip-pinctrl soc:pinctrl: setting mux of GPIO4-14 to 0 [ 0.176849] rockchip-pinctrl soc:pinctrl: setting mux of GPIO4-21 to 1 [ 0.178453] rockchip-pinctrl soc:pinctrl: setting mux of GPIO4-13 to 0 (hang) Thanks, Chukun -- 2.25.1
Add pwm nodes for RK3528. Most rk3528 boards use pwm-regulator to supply to CPU, add node to enable them. The PWM core on RK3528 is the same as RK3328, but the driver doesn't support interrupts yet. Unlike other SoCs, pinctrl-names need to be in "active" state, I'm not sure about this, but otherwise the pwm-regulator will not work properly. --- vdd_arm: regulator-vdd-arm { compatible = "pwm-regulator"; pwms = <&pwm1 0 5000 1>; pwm-supply = <&vcc5v0_sys>; regulator-name = "vdd_arm"; regulator-min-microvolt = <746000>; regulator-max-microvolt = <1201000>; regulator-always-on; regulator-boot-on; regulator-settling-time-up-us = <250>; }; &cpu0 { cpu-supply = <&vdd_arm>; }; &pwm1 { status = "okay"; }; --- Chukun Pan (2): dt-bindings: pwm: rockchip: Add rockchip,rk3528-pwm arm64: dts: rockchip: Add pwm nodes for RK3528 .../devicetree/bindings/pwm/pwm-rockchip.yaml | 1 + arch/arm64/boot/dts/rockchip/rk3528.dtsi | 88 +++++++++++++++++++ 2 files changed, 89 insertions(+)