linux-kernel - Re: [BUG] k3-j784s4-evm/phy-cadence-torrent: Shared reset using exclusive API

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: <46e635e6-b6bf-404c-87a2-57fe25b4855a@ti.com>
Date: Wed, 10 Jul 2024 10:29:46 +0530
From: Siddharth Vadapalli <s-vadapalli@...com>
To: Andrew Halaney <ahalaney@...hat.com>
CC: <nm@...com>, <vigneshr@...com>, <kristo@...nel.org>, <robh@...nel.org>,
        <krzk+dt@...nel.org>, <conor+dt@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <vkoul@...nel.org>,
        <kishon@...nel.org>, <sjakhade@...ence.com>, <rogerq@...nel.org>,
        <thomas.richard@...tlin.com>, <theo.lebrun@...tlin.com>,
        <make24@...as.ac.cn>, <linux-phy@...ts.infradead.org>,
        <s-vadapalli@...com>, <mranostay@...com>
Subject: Re: [BUG] k3-j784s4-evm/phy-cadence-torrent: Shared reset using
 exclusive API

On Tue, Jul 09, 2024 at 04:57:09PM -0500, Andrew Halaney wrote:
> Hi,
> 

[...]

> 
> this is because the devicetree has two[0][1] consumers of the same reset:
> 
> 	&serdes0 {
> 		status = "okay";
> 
> 		serdes0_pcie1_link: phy@0 {
> 			reg = <0>;
> 			cdns,num-lanes = <4>;
> 			#phy-cells = <0>;
> 			cdns,phy-type = <PHY_TYPE_PCIE>;
> 			resets = <&serdes_wiz0 1>, <&serdes_wiz0 2>,
> 				 <&serdes_wiz0 3>, <&serdes_wiz0 4>;
> 		};
> 	};
> 	...
> 	&serdes0 {
> 		status = "okay";
> 
> 		serdes0_usb_link: phy@3 {
> 			reg = <3>;
> 			cdns,num-lanes = <1>;
> 			#phy-cells = <0>;
> 			cdns,phy-type = <PHY_TYPE_USB3>;
> 			resets = <&serdes_wiz0 4>;
> 		};
> 	};
> 
> phy-cadence-torrent (the serdes0 consumer here) uses the exclusive consumer API[2],
> so this blows up.
> 
> Is the problem here that one of the above devicetree nodes is using the wrong
> reset, or does the driver need to look into using the shared API? I'm
> not sure where to find the reset definitions for the IP here unfortunately,
> so I'm hoping someone can help confirm if those are correct or not.

No, the resets are correct. Both PCIe1 and USB0 use the same instances
of SERDES which is SERDES0. I had posted the series for PCIe at:
https://lore.kernel.org/r/20240529082259.1619695-1-s-vadapalli@ti.com/
with all 4 Lanes of SERDES0 given to PCIe1. Similarly, Ravi had posted
the series for USB at:
https://lore.kernel.org/r/20240507095545.8210-1-r-gunasekaran@ti.com/
with lane 3 of SERDES0 given to USB0.

Since both of the series got merged on the same day (14 Jun 2024):
PCIe series:
https://lore.kernel.org/r/171826022277.240984.16790260886500529482.b4-ty@ti.com/
USB series:
https://lore.kernel.org/r/171826022274.240984.5150753966671933401.b4-ty@ti.com/
the dependency was unknown when the individual series were posted as
neither of them was a part of linux-next/ti-k3-dts-next when the other
one was posted.

> 
> Total aside, I think we should put the above dts snippet into one &serdes0 reference
> for readability sake. I'd post the patch but I'm hoping to get the above answered
> first in order to clean that up before shuffling things around for readability sake.

Yes, I agree that both sub-nodes should go into the same referenced
serdes0 node in k3-j784s4-evm.dts. The reason it didn't happen that way
to begin with is due to the fact that both series got merged on the same
day as I pointed out above.

The fix in this case will be to assign lanes 0 and 1 of SERDES0 to PCIe1
and lane 3 to USB0 with lane 2 left unused since PCIe doesn't have the
concept of a x3 link. In such a configuration, the device-tree nodes
will look like:

&serdes0 {
	status = "okay";

	serdes0_pcie1_link: phy@0 {
		reg = <0>;
		cdns,num-lanes = <2>;
		#phy-cells = <0>;
		cdns,phy-type = <PHY_TYPE_PCIE>;
		resets = <&serdes_wiz0 1>, <&serdes_wiz0 2>;
	};

	serdes0_usb_link: phy@3 {
		reg = <3>;
		cdns,num-lanes = <1>;
		#phy-cells = <0>;
		cdns,phy-type = <PHY_TYPE_USB3>;
		resets = <&serdes_wiz0 4>;
	};
};

Thank you for pointing out this issue. Please let me know if you plan to
post the patch with the above fix or you want me to post the patch for it.

Regards,
Siddharth.