| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<ZQ2PR01MB130736F5C893337606FD6937E6F1A@ZQ2PR01MB1307.CHNPR01.prod.partner.outlook.cn>
Date: Fri, 7 Feb 2025 08:17:04 +0000
From: Hal Feng <hal.feng@...rfivetech.com>
To: E Shattow <e@...eshell.de>, Conor Dooley <conor@...nel.org>
CC: Emil Renner Berthing <kernel@...il.dk>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Paul Walmsley
<paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>, Albert Ou
<aou@...s.berkeley.edu>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "linux-riscv@...ts.infradead.org"
<linux-riscv@...ts.infradead.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>, Minda Chen <minda.chen@...rfivetech.com>
Subject: RE: [PATCH v1 1/5] riscv: dts: starfive: jh7110-common: replace
syscrg clock assignments
> On 24.01.25 18:46, E Shattow wrote:
> On 1/15/25 01:35, Conor Dooley wrote:
> > On Wed, Jan 15, 2025 at 06:33:08AM +0000, Hal Feng wrote:
> >>> 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@...eshell.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?
> >>
> >> Here "assigned-clock-rates = <0>, ..." means skipping setting clock rates.
> >> You can refer to
> >> https://github.com/devicetree-org/dt-
> schema/blob/main/dtschema/schema
> >> s/clock/clock.yaml/
> >
> > If you check the wording there, it says:
> > To skip setting parent or rate of a clock its corresponding entry should be
> > set to 0, or can be omitted if it is not followed by any non-zero entry.
> > Since all clocks are being set to 0 above, we should be in the "can be
> > omitted" case for the entire property, no? That would...
> >
> >> 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.
>
> Conor / Hal, how does this work:
>
> Are these the minimum and maximum values for CPU frequency, CPU CORE
> and
> PLL0 OUT ?
>
> >>
> >> 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>; };
> >
> > ...make this one a reasonable change...
> >
> >> For u-boot, if there is no requirement to run u-boot at 1500MHz, just
> >> keep &syscrg {
> >> assigned-clock-rates = <0>, <0>, <0>, <0>; };
> >
> > ...but not this one.
> >
> >> 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
>
> Hal,
>
> What voltage (and speed) has less heat output?
>
> I think it must be configured to be *reliable first* as it is pictured being a
> developer board on a person's desk (with no extra accessory - no fans,
> heatsink, etc.) and for example doing work as of a Linux kernel compile with
> using all cores.
>
> We should not force a configuration that will fail for the common situation.
> What voltage and clock speed is the best for continuous reliable operation of a
> JH7110 CPU with no added heatsink or fan and at ambient temperature?
>
> Does CPU frequency range need to be given in devicetree? Can instead this be
> done with userspace tool or Linux subsystem at runtime?
>
> The thermal dissipation of JH7110 CPU will be depend on the board layout and
> physical application. So, my opinion here: I think any minimum and maximum
> CPU frequency limit that is given by manufacturer recommendation of safe
> continuous operation is okay for dts include. If some part of this frequency
> range is not reliable (from undervolt, or overheat) then it is a "no" from me in
> Linux and in U-Boot. The fact of failing to boot (in U-Boot) because JH7110
> CPU default voltage is not high enough for 1500MHz operation tells me this is
> a wrong idea or there is missing code in U-Boot to select a default sane clock
> speed and voltage.
CPU_FREQ is enabled in Linux, so the CPU frequency and power supply can be adjusted
automatically. JH7110 CPUs frequency can adjusted to 375 / 500 / 750 / 1500 MHz, and
the CPU power supply should be adjusted to 0.8 / 0.8 / 0.8 / 1.04 V respectively.
Please see node opp-table-0 in arch/riscv/boot/dts/starfive/jh7110.dtsi.
Please don't drop clock rate assignments, it will cause CPUs can not run at
375 / 500 / 750 / 1500MHz but 250/333/500/1000MHz. You can modify as I suggested above.
For U-Boot, CPU_FREQ is not enabled and CPUs are running at 1000MHz. CPU power
supply is 0.9V which is the default value. If we need to run U-Boot at 1500MHz, the
CPU power supply should be changed to 1.04V first.
So I suggest
Linux:
&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>;
};
U-Boot:
&syscrg {
assigned-clock-rates = <0>, <0>, <0>, <0>;
};
Best regards,
Hal
Powered by blists - more mailing lists