Re: [PATCH v1 1/5] riscv: dts: starfive: jh7110-common: replace syscrg clock assignments

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2615 bytes --]

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@freeshell.de>
> > > ---
> > >   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?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]