[PATCH v3 4/7] arm64: dts: allwinner: a64: reset pll-video0 rate

Roman Beranek posted 7 patches 2 years, 7 months ago
There is a newer version of this series
[PATCH v3 4/7] arm64: dts: allwinner: a64: reset pll-video0 rate
Posted by Roman Beranek 2 years, 7 months ago
With pll-mipi as its source clock, the exact rate to which TCON0's data
clock can be set to is constrained by the current rate of pll-video0.
Unless changed on a request of another consumer, the rate of pll-video0
is left as inherited from the bootloader.

The default rate on reset is 297 MHz, a value preferable to what it is
later set to in u-boot (294 MHz). This happens unintentionally though,
as u-boot, for the sake of simplicity, rounds the rate requested by DE2
driver (297 MHz) to 6 MHz steps.

Reset the PLL to its default rate of 297 MHz.

Signed-off-by: Roman Beranek <me@crly.cz>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index e6a194db420d..cfc60dce80b0 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -667,6 +667,9 @@ ccu: clock@1c20000 {
 			clock-names = "hosc", "losc";
 			#clock-cells = <1>;
 			#reset-cells = <1>;
+
+			assigned-clocks = <&ccu CLK_PLL_VIDEO0>;
+			assigned-clock-rates = <297000000>;
 		};
 
 		pio: pinctrl@1c20800 {
-- 
2.34.1
Re: [PATCH v3 4/7] arm64: dts: allwinner: a64: reset pll-video0 rate
Posted by Jernej Škrabec 2 years, 7 months ago
Dne četrtek, 27. april 2023 ob 11:16:08 CEST je Roman Beranek napisal(a):
> With pll-mipi as its source clock, the exact rate to which TCON0's data
> clock can be set to is constrained by the current rate of pll-video0.
> Unless changed on a request of another consumer, the rate of pll-video0
> is left as inherited from the bootloader.
> 
> The default rate on reset is 297 MHz, a value preferable to what it is
> later set to in u-boot (294 MHz). This happens unintentionally though,
> as u-boot, for the sake of simplicity, rounds the rate requested by DE2
> driver (297 MHz) to 6 MHz steps.
> 
> Reset the PLL to its default rate of 297 MHz.

Why would that be preferable? You actually dropped "clk: sunxi-ng: a64: 
propagate rate change from pll-mipi" patch which would take care for adjusting 
parent rate to correct value.

Best regards,
Jernej

> 
> Signed-off-by: Roman Beranek <me@crly.cz>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi index
> e6a194db420d..cfc60dce80b0 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -667,6 +667,9 @@ ccu: clock@1c20000 {
>  			clock-names = "hosc", "losc";
>  			#clock-cells = <1>;
>  			#reset-cells = <1>;
> +
> +			assigned-clocks = <&ccu CLK_PLL_VIDEO0>;
> +			assigned-clock-rates = <297000000>;
>  		};
> 
>  		pio: pinctrl@1c20800 {
Re: [PATCH v3 4/7] arm64: dts: allwinner: a64: reset pll-video0 rate
Posted by Frank Oltmanns 2 years, 7 months ago
Hi Jernej,

On 2023-04-28 at 08:43:29 +0200, Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
> Dne četrtek, 27. april 2023 ob 11:16:08 CEST je Roman Beranek napisal(a):
>> With pll-mipi as its source clock, the exact rate to which TCON0's data
>> clock can be set to is constrained by the current rate of pll-video0.
>> Unless changed on a request of another consumer, the rate of pll-video0
>> is left as inherited from the bootloader.
>>
>> The default rate on reset is 297 MHz, a value preferable to what it is
>> later set to in u-boot (294 MHz). This happens unintentionally though,
>> as u-boot, for the sake of simplicity, rounds the rate requested by DE2
>> driver (297 MHz) to 6 MHz steps.
>>
>> Reset the PLL to its default rate of 297 MHz.
>
> Why would that be preferable? You actually dropped "clk: sunxi-ng: a64:
> propagate rate change from pll-mipi" patch which would take care for adjusting
> parent rate to correct value.

For me, on the pinephone, it somehow doesn't. Please see here:
https://lore.kernel.org/all/87cz3uzpx1.fsf@oltmanns.dev/

I haven't figured out yet why that is. But hopefully, I'll find time in
the coming days / weeks to look into that.

Best regards,
  Frank

> Best regards,
> Jernej
>
>>
>> Signed-off-by: Roman Beranek <me@crly.cz>
>> ---
>>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi index
>> e6a194db420d..cfc60dce80b0 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> @@ -667,6 +667,9 @@ ccu: clock@1c20000 {
>>  			clock-names = "hosc", "losc";
>>  			#clock-cells = <1>;
>>  			#reset-cells = <1>;
>> +
>> +			assigned-clocks = <&ccu CLK_PLL_VIDEO0>;
>> +			assigned-clock-rates = <297000000>;
>>  		};
>>
>>  		pio: pinctrl@1c20800 {
Re: [PATCH v3 4/7] arm64: dts: allwinner: a64: reset pll-video0 rate
Posted by Maxime Ripard 2 years, 7 months ago
Hi,

I'm not sure I understand what you're doing here

On Thu, Apr 27, 2023 at 11:16:08AM +0200, Roman Beranek wrote:
> With pll-mipi as its source clock, the exact rate to which TCON0's data
> clock can be set to is constrained by the current rate of pll-video0.

What in the TCON exactly is constrained by pll-video0 rate?

> Unless changed on a request of another consumer, the rate of pll-video0
> is left as inherited from the bootloader.
>
> The default rate on reset is 297 MHz, a value preferable to what it is
> later set to in u-boot (294 MHz). This happens unintentionally though,
> as u-boot, for the sake of simplicity, rounds the rate requested by DE2
> driver (297 MHz) to 6 MHz steps.
>
> Reset the PLL to its default rate of 297 MHz.
> 
> Signed-off-by: Roman Beranek <me@crly.cz>

If the driver depends on the value being the reset value, then that's
the issue we should fix, either by reading the clock rate and adjusting
to it, or enforcing the rate we expect in the TCON driver.

Maxime