linux-kernel - Re: [PATCH] arm64: dts: ti: k3-am69-sk: remove assigned-clock-parents for unused VP

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bf271671-861d-43d4-a24f-de49256ef9d0@ideasonboard.com>
Date: Wed, 3 Jan 2024 15:05:40 +0200
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Jayesh Choudhary <j-choudhary@...com>, nm@...com, vigneshr@...com
Cc: kristo@...nel.org, robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
 conor+dt@...nel.org, a-bhatia1@...com, rogerq@...nel.org, sabiya.d@...com,
 u-kumar1@...com, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] arm64: dts: ti: k3-am69-sk: remove assigned-clock-parents
 for unused VP

Hi,

On 21/12/2023 13:30, Jayesh Choudhary wrote:
> VP2 and VP3 are unused video ports and VP3 share the same parent
> clock as VP1 causing issue with pixel clock setting for HDMI (VP1).
> So remove the parent clocks for unused VPs.
> 
> Fixes: 6f8605fd7d11 ("arm64: dts: ti: k3-am69-sk: Add DP and HDMI support")
> Reported-by: Nishanth Menon <nm@...com>
> Closes: https://storage.kernelci.org/mainline/master/v6.7-rc6/arm64/defconfig/gcc-10/lab-ti/baseline-nfs-am69_sk-fs.txt
> Signed-off-by: Jayesh Choudhary <j-choudhary@...com>
> ---
> 
> Local testing log for HDMI on AM69-SK:
> <https://gist.github.com/Jayesh2000/517395cd85eb28d65b8ee4568cefb809>
> 
>   arch/arm64/boot/dts/ti/k3-am69-sk.dts | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am69-sk.dts b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
> index 8da591579868..370980eb59b0 100644
> --- a/arch/arm64/boot/dts/ti/k3-am69-sk.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
> @@ -918,13 +918,9 @@ &dss {
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&dss_vout0_pins_default>;
>   	assigned-clocks = <&k3_clks 218 2>,
> -			  <&k3_clks 218 5>,
> -			  <&k3_clks 218 14>,
> -			  <&k3_clks 218 18>;
> +			  <&k3_clks 218 5>;
>   	assigned-clock-parents = <&k3_clks 218 3>,
> -				 <&k3_clks 218 7>,
> -				 <&k3_clks 218 16>,
> -				 <&k3_clks 218 22>;
> +				 <&k3_clks 218 7>;
>   };
>   
>   &serdes_wiz4 {

The SK has two outputs, using VP0 and VP1, so the above kind of makes 
sense. Then again, setting up 4 clocks here really shouldn't break the 
SK, should it? The AM69 has 4 available VPs. How does one configure the 
clocks for a board that uses 4 VPs, or possibly a different selection of 
VPs?

I think the patch desc should explain why this doesn't work. Afaik, the 
dts is not wrong as such, but there's an underlying issue that breaks 
the clocking if all four clocks are set up here.

So, with the desc updated, as this fixes an issue and is not wrong:

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>

But I also feel this is dodging a firmware (?) issue.

  Tomi