[PATCH v2 0/4] arm64: allwinner: a64: fix video output on Pinebook

[PATCH v2 0/4] arm64: allwinner: a64: fix video output on Pinebook

[Vasily Khoruzhick]

Since commit ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI
in TCON0 mux"), TCON0 clock parent is always set to PLL_MIPI, but
unfortunately it breaks video output on Pinebook.

I did an experiment: I manually configured PLL_MIPI and PLL_VIDEO0_2X
to the same clock rate and flipped the switch with devmem. Experiment
clearly showed that whenever PLL_MIPI is selected as TCON0 clock
parent, the video output stops working.

To fix the issue, I partially reverted mentioned commit and added explicit
TCON0 clock parent assignment to device tree. By default, it will be
PLL_MIPI, and the only users with RGB output - Pinebook and Teres-I will
override it in their dts.

v2:
  - split device tree binding change into its own patch
  - add rationale into commit message and into comment in the code
  - gather the tags

Vasily Khoruzhick (4):
  dt-bindings: clock: sunxi: Export PLL_VIDEO_2X and PLL_MIPI
  clk: sunxi-ng: a64: drop redundant CLK_PLL_VIDEO0_2X and CLK_PLL_MIPI
  arm64: dts: allwinner: a64: explicitly assign clock parent for TCON0
  clk: sunxi-ng: a64: stop force-selecting PLL-MIPI as TCON0 parent

 .../boot/dts/allwinner/sun50i-a64-pinebook.dts      |  2 ++
 .../arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts |  2 ++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi       |  2 ++
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c               | 13 ++++---------
 drivers/clk/sunxi-ng/ccu-sun50i-a64.h               |  2 --
 include/dt-bindings/clock/sun50i-a64-ccu.h          |  2 ++
 6 files changed, 12 insertions(+), 11 deletions(-)

-- 
2.47.1

[PATCH v2 1/4] dt-bindings: clock: sunxi: Export PLL_VIDEO_2X and PLL_MIPI

[Vasily Khoruzhick]

Export PLL_VIDEO_2X and PLL_MIPI, these will be used to explicitly
select TCON0 clock parent in dts

Fixes: ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux")
Reviewed-by: Dragan Simic <dsimic@manjaro.org>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Tested-by: Frank Oltmanns <frank@oltmanns.dev> # on PinePhone
Tested-by: Stuart Gathman <stuart@gathman.org> # on OG Pinebook
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 include/dt-bindings/clock/sun50i-a64-ccu.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/dt-bindings/clock/sun50i-a64-ccu.h b/include/dt-bindings/clock/sun50i-a64-ccu.h
index 175892189e9d..4f220ea7a23c 100644
--- a/include/dt-bindings/clock/sun50i-a64-ccu.h
+++ b/include/dt-bindings/clock/sun50i-a64-ccu.h
@@ -44,7 +44,9 @@
 #define _DT_BINDINGS_CLK_SUN50I_A64_H_
 
 #define CLK_PLL_VIDEO0		7
+#define CLK_PLL_VIDEO0_2X	8
 #define CLK_PLL_PERIPH0		11
+#define CLK_PLL_MIPI		17
 
 #define CLK_CPUX		21
 #define CLK_BUS_MIPI_DSI	28
-- 
2.47.1

[PATCH v2 2/4] clk: sunxi-ng: a64: drop redundant CLK_PLL_VIDEO0_2X and CLK_PLL_MIPI

[Vasily Khoruzhick]

Drop redundant CLK_PLL_VIDEO0_2X and CLK_PLL.MIPI. These are now
defined in dt-bindings/clock/sun50i-a64-ccu.h

Fixes: ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux")
Reviewed-by: Dragan Simic <dsimic@manjaro.org>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Tested-by: Frank Oltmanns <frank@oltmanns.dev> # on pinephone
Tested-by: Stuart Gathman <stuart@gathman.org> # on OG pinebook
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 drivers/clk/sunxi-ng/ccu-sun50i-a64.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.h b/drivers/clk/sunxi-ng/ccu-sun50i-a64.h
index a8c11c0b4e06..dfba88a5ad0f 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.h
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.h
@@ -21,7 +21,6 @@
 
 /* PLL_VIDEO0 exported for HDMI PHY */
 
-#define CLK_PLL_VIDEO0_2X		8
 #define CLK_PLL_VE			9
 #define CLK_PLL_DDR0			10
 
@@ -32,7 +31,6 @@
 #define CLK_PLL_PERIPH1_2X		14
 #define CLK_PLL_VIDEO1			15
 #define CLK_PLL_GPU			16
-#define CLK_PLL_MIPI			17
 #define CLK_PLL_HSIC			18
 #define CLK_PLL_DE			19
 #define CLK_PLL_DDR1			20
-- 
2.47.1

[PATCH v2 3/4] arm64: dts: allwinner: a64: explicitly assign clock parent for TCON0

[Vasily Khoruzhick]

TCON0 seems to need a different clock parent depending on output type.
For RGB it has to be PLL-VIDEO0-2X, while for DSI it has to be PLL-MIPI,
so select it explicitly.

Video output doesn't work if incorrect clock is assigned.

On my Pinebook I manually configured PLL-VIDEO0-2X and PLL-MIPI to the same
rate, and while video output works fine with PLL-VIDEO0-2X, it doesn't
work at all (as in no picture) with PLL-MIPI.

Fixes: ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux")
Reviewed-by: Dragan Simic <dsimic@manjaro.org>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Tested-by: Frank Oltmanns <frank@oltmanns.dev> # on PinePhone
Tested-by: Stuart Gathman <stuart@gathman.org> # on OG Pinebook
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts | 2 ++
 arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts  | 2 ++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi         | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
index 379c2c8466f5..86d44349e095 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
@@ -390,6 +390,8 @@ &sound {
 &tcon0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&lcd_rgb666_pins>;
+	assigned-clocks = <&ccu CLK_TCON0>;
+	assigned-clock-parents = <&ccu CLK_PLL_VIDEO0_2X>;
 
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
index b407e1dd08a7..ec055510af8b 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
@@ -369,6 +369,8 @@ &sound {
 &tcon0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&lcd_rgb666_pins>;
+	assigned-clocks = <&ccu CLK_TCON0>;
+	assigned-clock-parents = <&ccu CLK_PLL_VIDEO0_2X>;
 
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index a5c3920e0f04..0fecf0abb204 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -445,6 +445,8 @@ tcon0: lcd-controller@1c0c000 {
 			clock-names = "ahb", "tcon-ch0";
 			clock-output-names = "tcon-data-clock";
 			#clock-cells = <0>;
+			assigned-clocks = <&ccu CLK_TCON0>;
+			assigned-clock-parents = <&ccu CLK_PLL_MIPI>;
 			resets = <&ccu RST_BUS_TCON0>, <&ccu RST_BUS_LVDS>;
 			reset-names = "lcd", "lvds";
 
-- 
2.47.1

[PATCH v2 4/4] clk: sunxi-ng: a64: stop force-selecting PLL-MIPI as TCON0 parent

[Vasily Khoruzhick]

Stop force-selecting PLL-MIPI as TCON0 parent, since it breaks video
output on Pinebook that uses RGB to eDP bridge.

Partially revert commit ca1170b69968 ("clk: sunxi-ng: a64: force
select PLL_MIPI in TCON0 mux"), while still leaving
CLK_SET_RATE_NO_REPARENT flag set, since we do not want the clock to
be reparented.

The issue is that apparently different TCON0 outputs require a different
clock, or the mux might be selecting the output type.

I did an experiment: I manually configured PLL_MIPI and PLL_VIDEO0_2X
to the same clock rate and flipped the switch with devmem. Experiment
clearly showed that whenever PLL_MIPI is selected as TCON0 clock parent,
the video output stops working.

Therefore, TCON0 clock parent corresponding to the output type must be
assigned in the device tree.

Fixes: ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux")
Reviewed-by: Dragan Simic <dsimic@manjaro.org>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Tested-by: Frank Oltmanns <frank@oltmanns.dev> # on PinePhone
Tested-by: Stuart Gathman <stuart@gathman.org> # on OG Pinebook
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
index 3a7d61c81667..ba1ad267f123 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
@@ -535,11 +535,11 @@ static SUNXI_CCU_M_WITH_MUX_GATE(de_clk, "de", de_parents,
 				 CLK_SET_RATE_PARENT);
 
 /*
- * DSI output seems to work only when PLL_MIPI selected. Set it and prevent
- * the mux from reparenting.
+ * Experiments showed that RGB output requires pll-video0-2x, while DSI
+ * requires pll-mipi. It will not work with incorrect clock, the screen will
+ * be blank.
+ * sun50i-a64.dtsi assigns pll-mipi as TCON0 parent by default
  */
-#define SUN50I_A64_TCON0_CLK_REG	0x118
-
 static const char * const tcon0_parents[] = { "pll-mipi", "pll-video0-2x" };
 static const u8 tcon0_table[] = { 0, 2, };
 static SUNXI_CCU_MUX_TABLE_WITH_GATE_CLOSEST(tcon0_clk, "tcon0", tcon0_parents,
@@ -959,11 +959,6 @@ static int sun50i_a64_ccu_probe(struct platform_device *pdev)
 
 	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
 
-	/* Set PLL MIPI as parent for TCON0 */
-	val = readl(reg + SUN50I_A64_TCON0_CLK_REG);
-	val &= ~GENMASK(26, 24);
-	writel(val | (0 << 24), reg + SUN50I_A64_TCON0_CLK_REG);
-
 	ret = devm_sunxi_ccu_probe(&pdev->dev, reg, &sun50i_a64_ccu_desc);
 	if (ret)
 		return ret;
-- 
2.47.1

Re: [PATCH v2 1/4] dt-bindings: clock: sunxi: Export PLL_VIDEO_2X and PLL_MIPI

[Krzysztof Kozlowski]

On Fri, Jan 03, 2025 at 11:36:57PM -0800, Vasily Khoruzhick wrote:
> Export PLL_VIDEO_2X and PLL_MIPI, these will be used to explicitly
> select TCON0 clock parent in dts
> 
> Fixes: ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux")
> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>

Where did this happen?

> Tested-by: Frank Oltmanns <frank@oltmanns.dev> # on PinePhone
> Tested-by: Stuart Gathman <stuart@gathman.org> # on OG Pinebook

And these? I cannot find traces on the list.

Best regards,
Krzysztof

Re: [PATCH v2 1/4] dt-bindings: clock: sunxi: Export PLL_VIDEO_2X and PLL_MIPI

[Krzysztof Kozlowski]

On 04/01/2025 11:16, Krzysztof Kozlowski wrote:
> On Fri, Jan 03, 2025 at 11:36:57PM -0800, Vasily Khoruzhick wrote:
>> Export PLL_VIDEO_2X and PLL_MIPI, these will be used to explicitly
>> select TCON0 clock parent in dts
>>
>> Fixes: ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux")
>> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
>> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> 
> Where did this happen?
> 
>> Tested-by: Frank Oltmanns <frank@oltmanns.dev> # on PinePhone
>> Tested-by: Stuart Gathman <stuart@gathman.org> # on OG Pinebook
> 
> And these? I cannot find traces on the list.

I found them, cover letter.

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH v2 1/4] dt-bindings: clock: sunxi: Export PLL_VIDEO_2X and PLL_MIPI

[Chen-Yu Tsai]

On Sat, Jan 4, 2025 at 3:40 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>
> Export PLL_VIDEO_2X and PLL_MIPI, these will be used to explicitly
> select TCON0 clock parent in dts
>
> Fixes: ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux")
> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> Tested-by: Frank Oltmanns <frank@oltmanns.dev> # on PinePhone
> Tested-by: Stuart Gathman <stuart@gathman.org> # on OG Pinebook
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  include/dt-bindings/clock/sun50i-a64-ccu.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/include/dt-bindings/clock/sun50i-a64-ccu.h b/include/dt-bindings/clock/sun50i-a64-ccu.h
> index 175892189e9d..4f220ea7a23c 100644
> --- a/include/dt-bindings/clock/sun50i-a64-ccu.h
> +++ b/include/dt-bindings/clock/sun50i-a64-ccu.h
> @@ -44,7 +44,9 @@
>  #define _DT_BINDINGS_CLK_SUN50I_A64_H_
>
>  #define CLK_PLL_VIDEO0         7
> +#define CLK_PLL_VIDEO0_2X      8
>  #define CLK_PLL_PERIPH0                11
> +#define CLK_PLL_MIPI           17

You can't really split code movement into two patches.

With this patch applied the clk driver will fail to build because
the macros are now redefined in both header files.

Barring recombining the patches, please add a patch before this
adding #ifndef's around the two macros that are moved.

ChenYu


>  #define CLK_CPUX               21
>  #define CLK_BUS_MIPI_DSI       28
> --
> 2.47.1
>

Re: [PATCH v2 1/4] dt-bindings: clock: sunxi: Export PLL_VIDEO_2X and PLL_MIPI

[Krzysztof Kozlowski]

On 04/01/2025 11:23, Chen-Yu Tsai wrote:
> On Sat, Jan 4, 2025 at 3:40 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>>
>> Export PLL_VIDEO_2X and PLL_MIPI, these will be used to explicitly
>> select TCON0 clock parent in dts
>>
>> Fixes: ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux")
>> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
>> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
>> Tested-by: Frank Oltmanns <frank@oltmanns.dev> # on PinePhone
>> Tested-by: Stuart Gathman <stuart@gathman.org> # on OG Pinebook
>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>> ---
>>  include/dt-bindings/clock/sun50i-a64-ccu.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/include/dt-bindings/clock/sun50i-a64-ccu.h b/include/dt-bindings/clock/sun50i-a64-ccu.h
>> index 175892189e9d..4f220ea7a23c 100644
>> --- a/include/dt-bindings/clock/sun50i-a64-ccu.h
>> +++ b/include/dt-bindings/clock/sun50i-a64-ccu.h
>> @@ -44,7 +44,9 @@
>>  #define _DT_BINDINGS_CLK_SUN50I_A64_H_
>>
>>  #define CLK_PLL_VIDEO0         7
>> +#define CLK_PLL_VIDEO0_2X      8
>>  #define CLK_PLL_PERIPH0                11
>> +#define CLK_PLL_MIPI           17
> 
> You can't really split code movement into two patches.
> 
> With this patch applied the clk driver will fail to build because
> the macros are now redefined in both header files.

Are you sure? The values seem the same to me... I don't see how this
could fail.

> 
> Barring recombining the patches, please add a patch before this
> adding #ifndef's around the two macros that are moved.
> 

No, not necessary, just churn,


Best regards,
Krzysztof

Re: [PATCH v2 1/4] dt-bindings: clock: sunxi: Export PLL_VIDEO_2X and PLL_MIPI

[Andre Przywara]

On Sat, 4 Jan 2025 11:33:23 +0100
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 04/01/2025 11:23, Chen-Yu Tsai wrote:
> > On Sat, Jan 4, 2025 at 3:40 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:  
> >>
> >> Export PLL_VIDEO_2X and PLL_MIPI, these will be used to explicitly
> >> select TCON0 clock parent in dts
> >>
> >> Fixes: ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux")
> >> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> >> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> >> Tested-by: Frank Oltmanns <frank@oltmanns.dev> # on PinePhone
> >> Tested-by: Stuart Gathman <stuart@gathman.org> # on OG Pinebook
> >> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> >> ---
> >>  include/dt-bindings/clock/sun50i-a64-ccu.h | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/include/dt-bindings/clock/sun50i-a64-ccu.h b/include/dt-bindings/clock/sun50i-a64-ccu.h
> >> index 175892189e9d..4f220ea7a23c 100644
> >> --- a/include/dt-bindings/clock/sun50i-a64-ccu.h
> >> +++ b/include/dt-bindings/clock/sun50i-a64-ccu.h
> >> @@ -44,7 +44,9 @@
> >>  #define _DT_BINDINGS_CLK_SUN50I_A64_H_
> >>
> >>  #define CLK_PLL_VIDEO0         7
> >> +#define CLK_PLL_VIDEO0_2X      8
> >>  #define CLK_PLL_PERIPH0                11
> >> +#define CLK_PLL_MIPI           17  
> > 
> > You can't really split code movement into two patches.
> > 
> > With this patch applied the clk driver will fail to build because
> > the macros are now redefined in both header files.  
> 
> Are you sure? The values seem the same to me... I don't see how this
> could fail.

Yes, IIRC the C standard allows repeated definitions with the same
value. And I definitely tested this before (and hence recommended this
approach to Vasily) and it compiled without any warnings here.

Cheers,
Andre

> 
> > 
> > Barring recombining the patches, please add a patch before this
> > adding #ifndef's around the two macros that are moved.
> >   
> 
> No, not necessary, just churn,
> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 1/4] dt-bindings: clock: sunxi: Export PLL_VIDEO_2X and PLL_MIPI

[Chen-Yu Tsai]

On Sat, Jan 4, 2025 at 8:04 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Sat, 4 Jan 2025 11:33:23 +0100
> Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> > On 04/01/2025 11:23, Chen-Yu Tsai wrote:
> > > On Sat, Jan 4, 2025 at 3:40 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> > >>
> > >> Export PLL_VIDEO_2X and PLL_MIPI, these will be used to explicitly
> > >> select TCON0 clock parent in dts
> > >>
> > >> Fixes: ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux")
> > >> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> > >> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> > >> Tested-by: Frank Oltmanns <frank@oltmanns.dev> # on PinePhone
> > >> Tested-by: Stuart Gathman <stuart@gathman.org> # on OG Pinebook
> > >> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > >> ---
> > >>  include/dt-bindings/clock/sun50i-a64-ccu.h | 2 ++
> > >>  1 file changed, 2 insertions(+)
> > >>
> > >> diff --git a/include/dt-bindings/clock/sun50i-a64-ccu.h b/include/dt-bindings/clock/sun50i-a64-ccu.h
> > >> index 175892189e9d..4f220ea7a23c 100644
> > >> --- a/include/dt-bindings/clock/sun50i-a64-ccu.h
> > >> +++ b/include/dt-bindings/clock/sun50i-a64-ccu.h
> > >> @@ -44,7 +44,9 @@
> > >>  #define _DT_BINDINGS_CLK_SUN50I_A64_H_
> > >>
> > >>  #define CLK_PLL_VIDEO0         7
> > >> +#define CLK_PLL_VIDEO0_2X      8
> > >>  #define CLK_PLL_PERIPH0                11
> > >> +#define CLK_PLL_MIPI           17
> > >
> > > You can't really split code movement into two patches.
> > >
> > > With this patch applied the clk driver will fail to build because
> > > the macros are now redefined in both header files.
> >
> > Are you sure? The values seem the same to me... I don't see how this
> > could fail.
>
> Yes, IIRC the C standard allows repeated definitions with the same
> value. And I definitely tested this before (and hence recommended this
> approach to Vasily) and it compiled without any warnings here.

Hmm. Didn't know that. Good to know, and I just tried it on my end.

Thanks
ChenYu

> Cheers,
> Andre
>
> >
> > >
> > > Barring recombining the patches, please add a patch before this
> > > adding #ifndef's around the two macros that are moved.
> > >
> >
> > No, not necessary, just churn,
> >
> >
> > Best regards,
> > Krzysztof
> >
>

Re: [PATCH v2 1/4] dt-bindings: clock: sunxi: Export PLL_VIDEO_2X and PLL_MIPI

[Dragan Simic]

Hello all,

On 2025-01-04 13:02, Andre Przywara wrote:
> On Sat, 4 Jan 2025 11:33:23 +0100 Krzysztof Kozlowski <krzk@kernel.org> 
> wrote:
>> On 04/01/2025 11:23, Chen-Yu Tsai wrote:
>> > On Sat, Jan 4, 2025 at 3:40 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>> >>
>> >> Export PLL_VIDEO_2X and PLL_MIPI, these will be used to explicitly
>> >> select TCON0 clock parent in dts
>> >>
>> >> Fixes: ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux")
>> >> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
>> >> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
>> >> Tested-by: Frank Oltmanns <frank@oltmanns.dev> # on PinePhone
>> >> Tested-by: Stuart Gathman <stuart@gathman.org> # on OG Pinebook
>> >> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>> >> ---
>> >>  include/dt-bindings/clock/sun50i-a64-ccu.h | 2 ++
>> >>  1 file changed, 2 insertions(+)
>> >>
>> >> diff --git a/include/dt-bindings/clock/sun50i-a64-ccu.h b/include/dt-bindings/clock/sun50i-a64-ccu.h
>> >> index 175892189e9d..4f220ea7a23c 100644
>> >> --- a/include/dt-bindings/clock/sun50i-a64-ccu.h
>> >> +++ b/include/dt-bindings/clock/sun50i-a64-ccu.h
>> >> @@ -44,7 +44,9 @@
>> >>  #define _DT_BINDINGS_CLK_SUN50I_A64_H_
>> >>
>> >>  #define CLK_PLL_VIDEO0         7
>> >> +#define CLK_PLL_VIDEO0_2X      8
>> >>  #define CLK_PLL_PERIPH0                11
>> >> +#define CLK_PLL_MIPI           17
>> >
>> > You can't really split code movement into two patches.
>> >
>> > With this patch applied the clk driver will fail to build because
>> > the macros are now redefined in both header files.
>> 
>> Are you sure? The values seem the same to me... I don't see how this
>> could fail.
> 
> Yes, IIRC the C standard allows repeated definitions with the same
> value. And I definitely tested this before (and hence recommended this
> approach to Vasily) and it compiled without any warnings here.

FWIW, I also tested that approach when it was recommended, to make
sure that the C compiler(s) are fine with that.  And it worked. :)

>> > Barring recombining the patches, please add a patch before this
>> > adding #ifndef's around the two macros that are moved.
>> 
>> No, not necessary, just churn,

Re: [PATCH v2 1/4] dt-bindings: clock: sunxi: Export PLL_VIDEO_2X and PLL_MIPI

[Vasily Khoruzhick]

On Sat, Jan 4, 2025 at 2:18 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 04/01/2025 11:16, Krzysztof Kozlowski wrote:
> > On Fri, Jan 03, 2025 at 11:36:57PM -0800, Vasily Khoruzhick wrote:
> >> Export PLL_VIDEO_2X and PLL_MIPI, these will be used to explicitly
> >> select TCON0 clock parent in dts
> >>
> >> Fixes: ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI in TCON0 mux")
> >> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> >> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> >
> > Where did this happen?
> >
> >> Tested-by: Frank Oltmanns <frank@oltmanns.dev> # on PinePhone
> >> Tested-by: Stuart Gathman <stuart@gathman.org> # on OG Pinebook
> >
> > And these? I cannot find traces on the list.
>
> I found them, cover letter.

Yeah, most of the tags are replies to v1 cover letter. Stuart's
Tested-by comes from IRC. However all the mentioned people are on CC
list, it's not like I'm trying to sneak in counterfeit tags :)


> Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> Best regards,
> Krzysztof

Re: [PATCH v2 1/4] dt-bindings: clock: sunxi: Export PLL_VIDEO_2X and PLL_MIPI

[Vasily Khoruzhick]

On Sat, Jan 4, 2025 at 4:14 AM Chen-Yu Tsai <wens@csie.org> wrote:

> > Yes, IIRC the C standard allows repeated definitions with the same
> > value. And I definitely tested this before (and hence recommended this
> > approach to Vasily) and it compiled without any warnings here.
>
> Hmm. Didn't know that. Good to know, and I just tried it on my end.

Yeah, I don't like it either. It's an artefact of pretending that the
device tree directory(ies) is a separate tree.

> Thanks
> ChenYu

Re: [PATCH v2 0/4] arm64: allwinner: a64: fix video output on Pinebook

[Chen-Yu Tsai]

On Fri, 03 Jan 2025 23:36:56 -0800, Vasily Khoruzhick wrote:
> Since commit ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI
> in TCON0 mux"), TCON0 clock parent is always set to PLL_MIPI, but
> unfortunately it breaks video output on Pinebook.
> 
> I did an experiment: I manually configured PLL_MIPI and PLL_VIDEO0_2X
> to the same clock rate and flipped the switch with devmem. Experiment
> clearly showed that whenever PLL_MIPI is selected as TCON0 clock
> parent, the video output stops working.
> 
> [...]

Applied to sunxi/for-next in sunxi/linux.git, thanks!

[1/4] dt-bindings: clock: sunxi: Export PLL_VIDEO_2X and PLL_MIPI
      https://git.kernel.org/sunxi/linux/c/9897831de614
[2/4] clk: sunxi-ng: a64: drop redundant CLK_PLL_VIDEO0_2X and CLK_PLL_MIPI
      https://git.kernel.org/sunxi/linux/c/0f368cb7ef10
[3/4] arm64: dts: allwinner: a64: explicitly assign clock parent for TCON0
      https://git.kernel.org/sunxi/linux/c/8715c91a8365
[4/4] clk: sunxi-ng: a64: stop force-selecting PLL-MIPI as TCON0 parent
      https://git.kernel.org/sunxi/linux/c/383ca7bee8a9

Best regards,
-- 
Chen-Yu Tsai <wens@csie.org>