| Message ID | 20250408103606.3679505-3-s-vadapalli@ti.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | J722S: Disable WIZ0 and WIZ1 in SoC file | expand |
On 4/8/2025 4:06 PM, Siddharth Vadapalli wrote: > Since "serdes0" and "serdes1" which are the sub-nodes of "serdes_wiz0" > and "serdes_wiz1" respectively, have been disabled in the SoC file already, > and, given that these sub-nodes will only be enabled in a board file if the > board utilizes any of the SERDES instances and the peripherals bound to > them, we end up in a situation where the board file doesn't explicitly > disable "serdes_wiz0" and "serdes_wiz1". As a consequence of this, the > following errors show up when booting Linux: > > wiz bus@f0000:phy@f000000: probe with driver wiz failed with error -12 > ... > wiz bus@f0000:phy@f010000: probe with driver wiz failed with error -12 > > To not only fix the above, but also, in order to follow the convention of > disabling device-tree nodes in the SoC file and enabling them in the board > files for those boards which require them, disable "serdes_wiz0" and > "serdes_wiz1" device-tree nodes. > > Fixes: 628e0a0118e6 ("arm64: dts: ti: k3-j722s-main: Add SERDES and PCIe support") > Cc: stable@vger.kernel.org > Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> > --- > > v1 of this patch is at: > https://lore.kernel.org/r/20250408060636.3413856-3-s-vadapalli@ti.com/ > Changes since v1: > - Added "Fixes" tag and updated commit message accordingly. > > Regards, > Siddharth. > > arch/arm64/boot/dts/ti/k3-j722s-main.dtsi | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi b/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi > index 6850f50530f1..beda9e40e931 100644 > --- a/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi > +++ b/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi > @@ -32,6 +32,8 @@ serdes_wiz0: phy@f000000 { > assigned-clocks = <&k3_clks 279 1>; > assigned-clock-parents = <&k3_clks 279 5>; > > + status = "disabled"; > + Since you are disabling parent node. Do you still want to carry status = "disabled" in child nodes serdes0 and serdes1. > serdes0: serdes@f000000 { > compatible = "ti,j721e-serdes-10g"; > reg = <0x0f000000 0x00010000>; > @@ -70,6 +72,8 @@ serdes_wiz1: phy@f010000 { > assigned-clocks = <&k3_clks 280 1>; > assigned-clock-parents = <&k3_clks 280 5>; > > + status = "disabled"; > + > serdes1: serdes@f010000 { > compatible = "ti,j721e-serdes-10g"; > reg = <0x0f010000 0x00010000>;
On Fri, Apr 11, 2025 at 07:31:52PM +0530, Kumar, Udit wrote: Hello Udit, > > On 4/8/2025 4:06 PM, Siddharth Vadapalli wrote: > > Since "serdes0" and "serdes1" which are the sub-nodes of "serdes_wiz0" > > and "serdes_wiz1" respectively, have been disabled in the SoC file already, > > and, given that these sub-nodes will only be enabled in a board file if the > > board utilizes any of the SERDES instances and the peripherals bound to > > them, we end up in a situation where the board file doesn't explicitly > > disable "serdes_wiz0" and "serdes_wiz1". As a consequence of this, the > > following errors show up when booting Linux: > > > > wiz bus@f0000:phy@f000000: probe with driver wiz failed with error -12 > > ... > > wiz bus@f0000:phy@f010000: probe with driver wiz failed with error -12 > > > > To not only fix the above, but also, in order to follow the convention of > > disabling device-tree nodes in the SoC file and enabling them in the board > > files for those boards which require them, disable "serdes_wiz0" and > > "serdes_wiz1" device-tree nodes. > > > > Fixes: 628e0a0118e6 ("arm64: dts: ti: k3-j722s-main: Add SERDES and PCIe support") > > Cc: stable@vger.kernel.org > > Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> > > --- > > > > v1 of this patch is at: > > https://lore.kernel.org/r/20250408060636.3413856-3-s-vadapalli@ti.com/ > > Changes since v1: > > - Added "Fixes" tag and updated commit message accordingly. > > > > Regards, > > Siddharth. > > > > arch/arm64/boot/dts/ti/k3-j722s-main.dtsi | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi b/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi > > index 6850f50530f1..beda9e40e931 100644 > > --- a/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi > > +++ b/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi > > @@ -32,6 +32,8 @@ serdes_wiz0: phy@f000000 { > > assigned-clocks = <&k3_clks 279 1>; > > assigned-clock-parents = <&k3_clks 279 5>; > > + status = "disabled"; > > + > > Since you are disabling parent node. > > Do you still want to carry status = "disabled" in child nodes serdes0 and > serdes1. I could drop it, but then the patches will look something like: 1) Patch 1: Same as the first patch in this series 2) Patch 2: Current patch + Remove status = "disabled" within serdes0/1 3) Patch 3: Removed redundant status = "okay" within serdes0/1 in k3-j722s-evm.dts Updated Patch 2 and the new Patch 3 mentioned above aren't necessarily a complete "Fix" and have other changes in addition to the "Fix". For that reason, the changes associated with the updated patch 2 and the new patch 3 could be a separate series, unless you believe that they should go together in the current series. Please let me know. Regards, Siddharth.
Hi On 4/11/2025 7:47 PM, Siddharth Vadapalli wrote: > On Fri, Apr 11, 2025 at 07:31:52PM +0530, Kumar, Udit wrote: > > Hello Udit, > >> On 4/8/2025 4:06 PM, Siddharth Vadapalli wrote: >>> Since "serdes0" and "serdes1" which are the sub-nodes of "serdes_wiz0" >>> and "serdes_wiz1" respectively, have been disabled in the SoC file already, >>> and, given that these sub-nodes will only be enabled in a board file if the >>> board utilizes any of the SERDES instances and the peripherals bound to >>> them, we end up in a situation where the board file doesn't explicitly >>> disable "serdes_wiz0" and "serdes_wiz1". As a consequence of this, the >>> following errors show up when booting Linux: >>> >>> wiz bus@f0000:phy@f000000: probe with driver wiz failed with error -12 >>> ... >>> wiz bus@f0000:phy@f010000: probe with driver wiz failed with error -12 >>> >>> To not only fix the above, but also, in order to follow the convention of >>> disabling device-tree nodes in the SoC file and enabling them in the board >>> files for those boards which require them, disable "serdes_wiz0" and >>> "serdes_wiz1" device-tree nodes. >>> >>> Fixes: 628e0a0118e6 ("arm64: dts: ti: k3-j722s-main: Add SERDES and PCIe support") >>> Cc: stable@vger.kernel.org >>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> >>> --- >>> >>> v1 of this patch is at: >>> https://lore.kernel.org/r/20250408060636.3413856-3-s-vadapalli@ti.com/ >>> Changes since v1: >>> - Added "Fixes" tag and updated commit message accordingly. >>> >>> Regards, >>> Siddharth. >>> >>> arch/arm64/boot/dts/ti/k3-j722s-main.dtsi | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi b/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi >>> index 6850f50530f1..beda9e40e931 100644 >>> --- a/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi >>> +++ b/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi >>> @@ -32,6 +32,8 @@ serdes_wiz0: phy@f000000 { >>> assigned-clocks = <&k3_clks 279 1>; >>> assigned-clock-parents = <&k3_clks 279 5>; >>> + status = "disabled"; >>> + >> Since you are disabling parent node. >> >> Do you still want to carry status = "disabled" in child nodes serdes0 and >> serdes1. > I could drop it, but then the patches will look something like: > 1) Patch 1: Same as the first patch in this series > 2) Patch 2: Current patch + Remove status = "disabled" within serdes0/1 > 3) Patch 3: Removed redundant status = "okay" within serdes0/1 in > k3-j722s-evm.dts > > Updated Patch 2 and the new Patch 3 mentioned above aren't necessarily a > complete "Fix" and have other changes in addition to the "Fix". For that > reason, the changes associated with the updated patch 2 and the new patch 3 > could be a separate series, unless you believe that they should go > together in the current series. Please let me know. I don't see any use case where serdes_wiz0 is enabled and serdes0 is disabled. So your comment 3) is valid. you can take this clean up in other series For now Reviewed-by: Udit Kumar <u-kumar1@ti.com> > Regards, > Siddharth.
On Fri, Apr 11, 2025 at 09:22:00PM +0530, Kumar, Udit wrote: > Hi > > On 4/11/2025 7:47 PM, Siddharth Vadapalli wrote: > > On Fri, Apr 11, 2025 at 07:31:52PM +0530, Kumar, Udit wrote: [...] > > > Since you are disabling parent node. > > > > > > Do you still want to carry status = "disabled" in child nodes serdes0 and > > > serdes1. > > I could drop it, but then the patches will look something like: > > 1) Patch 1: Same as the first patch in this series > > 2) Patch 2: Current patch + Remove status = "disabled" within serdes0/1 > > 3) Patch 3: Removed redundant status = "okay" within serdes0/1 in > > k3-j722s-evm.dts > > > > Updated Patch 2 and the new Patch 3 mentioned above aren't necessarily a > > complete "Fix" and have other changes in addition to the "Fix". For that > > reason, the changes associated with the updated patch 2 and the new patch 3 > > could be a separate series, unless you believe that they should go > > together in the current series. Please let me know. > > I don't see any use case where serdes_wiz0 is enabled and serdes0 is > disabled. > > So your comment 3) is valid. you can take this clean up in other series > > > For now > > Reviewed-by: Udit Kumar <u-kumar1@ti.com> I have posted the cleanup series at: https://lore.kernel.org/r/20250412052712.927626-1-s-vadapalli@ti.com/ The cleanup series applies on top of the current series. Regards, Siddharth.
diff --git a/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi b/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi index 6850f50530f1..beda9e40e931 100644 --- a/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi +++ b/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi @@ -32,6 +32,8 @@ serdes_wiz0: phy@f000000 { assigned-clocks = <&k3_clks 279 1>; assigned-clock-parents = <&k3_clks 279 5>; + status = "disabled"; + serdes0: serdes@f000000 { compatible = "ti,j721e-serdes-10g"; reg = <0x0f000000 0x00010000>; @@ -70,6 +72,8 @@ serdes_wiz1: phy@f010000 { assigned-clocks = <&k3_clks 280 1>; assigned-clock-parents = <&k3_clks 280 5>; + status = "disabled"; + serdes1: serdes@f010000 { compatible = "ti,j721e-serdes-10g"; reg = <0x0f010000 0x00010000>;
Since "serdes0" and "serdes1" which are the sub-nodes of "serdes_wiz0" and "serdes_wiz1" respectively, have been disabled in the SoC file already, and, given that these sub-nodes will only be enabled in a board file if the board utilizes any of the SERDES instances and the peripherals bound to them, we end up in a situation where the board file doesn't explicitly disable "serdes_wiz0" and "serdes_wiz1". As a consequence of this, the following errors show up when booting Linux: wiz bus@f0000:phy@f000000: probe with driver wiz failed with error -12 ... wiz bus@f0000:phy@f010000: probe with driver wiz failed with error -12 To not only fix the above, but also, in order to follow the convention of disabling device-tree nodes in the SoC file and enabling them in the board files for those boards which require them, disable "serdes_wiz0" and "serdes_wiz1" device-tree nodes. Fixes: 628e0a0118e6 ("arm64: dts: ti: k3-j722s-main: Add SERDES and PCIe support") Cc: stable@vger.kernel.org Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> --- v1 of this patch is at: https://lore.kernel.org/r/20250408060636.3413856-3-s-vadapalli@ti.com/ Changes since v1: - Added "Fixes" tag and updated commit message accordingly. Regards, Siddharth. arch/arm64/boot/dts/ti/k3-j722s-main.dtsi | 4 ++++ 1 file changed, 4 insertions(+)