| Message ID | 20250104074035.1611136-2-anarsoul@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: allwinner: a64: fix video output on Pinebook | expand |
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
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
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 >
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
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 >
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 > > >
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,
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
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
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
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(+)