> On 07.01.25 04:08, Conor Dooley wrote: > On Sat, Jan 04, 2025 at 01:04:30PM -0800, E Shattow wrote: > > Hi, Conor (added CC: Minda Chen, Hal Feng) > > > > On 1/4/25 10:33, Conor Dooley wrote: > > > On Thu, Jan 02, 2025 at 11:45:07AM -0800, E Shattow wrote: > > > > Replace syscrg assignments of clocks, clock parents, and rates, > > > > for compatibility with downstream boot loader SPL secondary > > > > program loader. > > > > > > > > Signed-off-by: E Shattow <e@xxxxxxxxxxxx> > > > > --- > > > > arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 12 +++++++++--- > > > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi > > > > b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi > > > > index 48fb5091b817..55c6743100a7 100644 > > > > --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi > > > > +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi > > > > @@ -359,9 +359,15 @@ spi_dev0: spi@0 { > > > > }; > > > > &syscrg { > > > > - assigned-clocks = <&syscrg JH7110_SYSCLK_CPU_CORE>, > > > > - <&pllclk JH7110_PLLCLK_PLL0_OUT>; > > > > - assigned-clock-rates = <500000000>, <1500000000>; > > > > + assigned-clocks = <&syscrg JH7110_SYSCLK_CPU_ROOT>, > > > > + <&syscrg JH7110_SYSCLK_BUS_ROOT>, > > > > + <&syscrg JH7110_SYSCLK_PERH_ROOT>, > > > > + <&syscrg JH7110_SYSCLK_QSPI_REF>; > > > > + assigned-clock-parents = <&pllclk JH7110_PLLCLK_PLL0_OUT>, > > > > + <&pllclk JH7110_PLLCLK_PLL2_OUT>, > > > > + <&pllclk JH7110_PLLCLK_PLL2_OUT>, > > > > + <&syscrg JH7110_SYSCLK_QSPI_REF_SRC>; > > > > + assigned-clock-rates = <0>, <0>, <0>, <0>; > > > > > > Why is assigned rates here 0s, rather than the property just removed? > > > > > > > }; > > > > &sysgpio { > > > > -- > > > > 2.45.2 > > > > > > > > Assigned rates all zeroes is how it is in U-Boot. Removing the > > assigned-clock-rates property as suggested does work in U-Boot and > > Linux both. > > > > For context, U-Boot fails when replacing assigned-clocks to > > JH7110_SYSCLK_CPU_CORE (500MHz) and JH7110_PLLCLK_PLL0_OUT > (1500MHz) > > from Linux. So I tried to merge all properties together and in testing > > then U-Boot failed (or I did it wrong). However replacing the Linux > > properties with the U-Boot configuration (above) on Linux does work for > both. > > > > I do not know if this is correct but I can test any suggestions and > > report if they are working. > > > > Do these changes make sense? Are there other variations I should test? > > I'd like the commit message to at least explain why these clocks need to be > set to zero (I assume that means disabled?). Maybe the StarFive folks know > why it is required? Here "assigned-clock-rates = <0>, ..." means skipping setting clock rates. You can refer to https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/clock/clock.yaml/ Linux here setting JH7110_SYSCLK_CPU_CORE to 500MHz and JH7110_PLLCLK_PLL0_OUT to 1500MHz are for increasing the CPU frequency to 1500MHz. VF2 u-boot is still running at 1000MHz now. You failed to set JH7110_PLLCLK_PLL0_OUT to 1500MHz, because CPU power supply voltage needs to be increased before running at 1500MHz. I think a better choice now is changing Linux device tree as follows: &syscrg { assigned-clocks = <&syscrg JH7110_SYSCLK_CPU_ROOT>, <&syscrg JH7110_SYSCLK_BUS_ROOT>, <&syscrg JH7110_SYSCLK_PERH_ROOT>, <&syscrg JH7110_SYSCLK_QSPI_REF>, <&syscrg JH7110_SYSCLK_CPU_CORE>, <&pllclk JH7110_PLLCLK_PLL0_OUT>; assigned-clock-parents = <&pllclk JH7110_PLLCLK_PLL0_OUT>, <&pllclk JH7110_PLLCLK_PLL2_OUT>, <&pllclk JH7110_PLLCLK_PLL2_OUT>, <&syscrg JH7110_SYSCLK_QSPI_REF_SRC>; assigned-clock-rates = <0>, <0>, <0>, <0>, <500000000>, <1500000000>; }; For u-boot, if there is no requirement to run u-boot at 1500MHz, just keep &syscrg { assigned-clock-rates = <0>, <0>, <0>, <0>; }; in u-boot device tree. If we need running 1500MHz in u-boot, I will send a patch to implement it and then &syscrg{...} in u-boot device tree can be dropped. Best regards, Hal