Message ID | 20250412052712.927626-1-s-vadapalli@ti.com (mailing list archive) |
---|---|
Headers | show |
Series | J722S: DT Node cleanup for serdes0 and serdes1 | expand |
Thanks Siddharth On 4/12/2025 10:57 AM, Siddharth Vadapalli wrote: > Hello, > > This series is based on the following series: > https://patchwork.kernel.org/project/linux-arm-kernel/cover/20250408103606.3679505-1-s-vadapalli@ti.com/ > Based on the discussion in the above series which disabled 'serdes_wiz0' > and 'serdes_wiz1' nodes in the SoC file and enabled them in the board > file, Udit pointed out that it wasn't necessary to disable 'serdes0' and > 'serdes1' in the SoC file anymore, since that is not a working > configuration - serdes_wizX enabled and serdesX disabled doesn't work. > > Hence, this series aims to cleanup the serdesX nodes after the changes > made by the above series. > > Regards, > Siddharth. > > Siddharth Vadapalli (2): > arm64: dts: ti: k3-j722s-main: don't disable serdes0 and serdes1 > arm64: dts: ti: k3-j722s-evm: drop redundant status within > serdes0/serdes1 For series Reviewed-by: Udit Kumar <u-kumar1@ti.com> > arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 2 -- > arch/arm64/boot/dts/ti/k3-j722s-main.dtsi | 4 ---- > 2 files changed, 6 deletions(-) >
On 10:57-20250412, Siddharth Vadapalli wrote: > Hello, > > This series is based on the following series: > https://patchwork.kernel.org/project/linux-arm-kernel/cover/20250408103606.3679505-1-s-vadapalli@ti.com/ > Based on the discussion in the above series which disabled 'serdes_wiz0' > and 'serdes_wiz1' nodes in the SoC file and enabled them in the board > file, Udit pointed out that it wasn't necessary to disable 'serdes0' and > 'serdes1' in the SoC file anymore, since that is not a working > configuration - serdes_wizX enabled and serdesX disabled doesn't work. > > Hence, this series aims to cleanup the serdesX nodes after the changes > made by the above series. > > Regards, > Siddharth. > > Siddharth Vadapalli (2): > arm64: dts: ti: k3-j722s-main: don't disable serdes0 and serdes1 > arm64: dts: ti: k3-j722s-evm: drop redundant status within > serdes0/serdes1 > > arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 2 -- > arch/arm64/boot/dts/ti/k3-j722s-main.dtsi | 4 ---- > 2 files changed, 6 deletions(-) > > -- > 2.34.1 > I do not understand the logic here. serdes cannot operate without wiz nodes, correct? why would we leave serdes on by default?
On Mon, Apr 14, 2025 at 07:09:30AM -0500, Nishanth Menon wrote: Hello Nishanth, > On 10:57-20250412, Siddharth Vadapalli wrote: > > Hello, > > > > This series is based on the following series: > > https://patchwork.kernel.org/project/linux-arm-kernel/cover/20250408103606.3679505-1-s-vadapalli@ti.com/ > > Based on the discussion in the above series which disabled 'serdes_wiz0' > > and 'serdes_wiz1' nodes in the SoC file and enabled them in the board > > file, Udit pointed out that it wasn't necessary to disable 'serdes0' and > > 'serdes1' in the SoC file anymore, since that is not a working > > configuration - serdes_wizX enabled and serdesX disabled doesn't work. > > > > Hence, this series aims to cleanup the serdesX nodes after the changes > > made by the above series. > > > > Regards, > > Siddharth. > > > > Siddharth Vadapalli (2): > > arm64: dts: ti: k3-j722s-main: don't disable serdes0 and serdes1 > > arm64: dts: ti: k3-j722s-evm: drop redundant status within > > serdes0/serdes1 > > > > arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 2 -- > > arch/arm64/boot/dts/ti/k3-j722s-main.dtsi | 4 ---- > > 2 files changed, 6 deletions(-) > > > > -- > > 2.34.1 > > > > > I do not understand the logic here. serdes cannot operate without wiz > nodes, correct? why would we leave serdes on by default? Yes, serdesX requires serdes_wizX, but at the same time, serdesX is the child node of serdes_wizX. Therefore, without enabling serdes_wizX, we cannot enable serdesX. Prior to this series, but with the dependent series at: https://patchwork.kernel.org/project/linux-arm-kernel/cover/20250408103606.3679505-1-s-vadapalli@ti.com/ applied, the nodes look like: serdes_wizX { ... status = "disabled"; serdesX { ... status = "disabled"; }; }; The dependent series fixes 'serdes_wizX' by disabling it in the SoC file k3-j722s-main.dtsi. But after the fix, we have a 'status = "disabled";' within the serdesX node which isn't required since: a) serdes_wizX enabled but serdesX disabled is non-functional and unusable b) serdes_wizX disabled in DT implies that serdesX is also disabled Regards, Siddharth.
On 18:30-20250414, Siddharth Vadapalli wrote: > On Mon, Apr 14, 2025 at 07:09:30AM -0500, Nishanth Menon wrote: > > Hello Nishanth, > > > On 10:57-20250412, Siddharth Vadapalli wrote: > > > Hello, > > > > > > This series is based on the following series: > > > https://patchwork.kernel.org/project/linux-arm-kernel/cover/20250408103606.3679505-1-s-vadapalli@ti.com/ > > > Based on the discussion in the above series which disabled 'serdes_wiz0' > > > and 'serdes_wiz1' nodes in the SoC file and enabled them in the board > > > file, Udit pointed out that it wasn't necessary to disable 'serdes0' and > > > 'serdes1' in the SoC file anymore, since that is not a working > > > configuration - serdes_wizX enabled and serdesX disabled doesn't work. > > > > > > Hence, this series aims to cleanup the serdesX nodes after the changes > > > made by the above series. > > > > > > Regards, > > > Siddharth. > > > > > > Siddharth Vadapalli (2): > > > arm64: dts: ti: k3-j722s-main: don't disable serdes0 and serdes1 > > > arm64: dts: ti: k3-j722s-evm: drop redundant status within > > > serdes0/serdes1 > > > > > > arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 2 -- > > > arch/arm64/boot/dts/ti/k3-j722s-main.dtsi | 4 ---- > > > 2 files changed, 6 deletions(-) > > > > > > -- > > > 2.34.1 > > > > > > > > > I do not understand the logic here. serdes cannot operate without wiz > > nodes, correct? why would we leave serdes on by default? > > Yes, serdesX requires serdes_wizX, but at the same time, serdesX is the > child node of serdes_wizX. Therefore, without enabling serdes_wizX, we > cannot enable serdesX. > > Prior to this series, but with the dependent series at: > https://patchwork.kernel.org/project/linux-arm-kernel/cover/20250408103606.3679505-1-s-vadapalli@ti.com/ > applied, the nodes look like: > > serdes_wizX { > ... > status = "disabled"; > > serdesX { > ... > status = "disabled"; > }; > }; > > The dependent series fixes 'serdes_wizX' by disabling it in the SoC file > k3-j722s-main.dtsi. But after the fix, we have a 'status = "disabled";' > within the serdesX node which isn't required since: > a) serdes_wizX enabled but serdesX disabled is non-functional and > unusable > b) serdes_wizX disabled in DT implies that serdesX is also disabled Can we handle all of this in one series instead of two series?
On Mon, Apr 14, 2025 at 09:39:16AM -0500, Nishanth Menon wrote: > On 18:30-20250414, Siddharth Vadapalli wrote: > > On Mon, Apr 14, 2025 at 07:09:30AM -0500, Nishanth Menon wrote: > > > > Hello Nishanth, > > > > > On 10:57-20250412, Siddharth Vadapalli wrote: > > > > Hello, > > > > > > > > This series is based on the following series: > > > > https://patchwork.kernel.org/project/linux-arm-kernel/cover/20250408103606.3679505-1-s-vadapalli@ti.com/ > > > > Based on the discussion in the above series which disabled 'serdes_wiz0' > > > > and 'serdes_wiz1' nodes in the SoC file and enabled them in the board > > > > file, Udit pointed out that it wasn't necessary to disable 'serdes0' and > > > > 'serdes1' in the SoC file anymore, since that is not a working > > > > configuration - serdes_wizX enabled and serdesX disabled doesn't work. > > > > > > > > Hence, this series aims to cleanup the serdesX nodes after the changes > > > > made by the above series. > > > > > > > > Regards, > > > > Siddharth. > > > > > > > > Siddharth Vadapalli (2): > > > > arm64: dts: ti: k3-j722s-main: don't disable serdes0 and serdes1 > > > > arm64: dts: ti: k3-j722s-evm: drop redundant status within > > > > serdes0/serdes1 > > > > > > > > arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 2 -- > > > > arch/arm64/boot/dts/ti/k3-j722s-main.dtsi | 4 ---- > > > > 2 files changed, 6 deletions(-) > > > > > > > > -- > > > > 2.34.1 > > > > > > > > > > > > > I do not understand the logic here. serdes cannot operate without wiz > > > nodes, correct? why would we leave serdes on by default? > > > > Yes, serdesX requires serdes_wizX, but at the same time, serdesX is the > > child node of serdes_wizX. Therefore, without enabling serdes_wizX, we > > cannot enable serdesX. > > > > Prior to this series, but with the dependent series at: > > https://patchwork.kernel.org/project/linux-arm-kernel/cover/20250408103606.3679505-1-s-vadapalli@ti.com/ > > applied, the nodes look like: > > > > serdes_wizX { > > ... > > status = "disabled"; > > > > serdesX { > > ... > > status = "disabled"; > > }; > > }; > > > > The dependent series fixes 'serdes_wizX' by disabling it in the SoC file > > k3-j722s-main.dtsi. But after the fix, we have a 'status = "disabled";' > > within the serdesX node which isn't required since: > > a) serdes_wizX enabled but serdesX disabled is non-functional and > > unusable > > b) serdes_wizX disabled in DT implies that serdesX is also disabled > > > Can we handle all of this in one series instead of two series? The idea behind splitting it was that the dependent series is a "Fix": - Fix to follow the convention of disabling nodes in SoC file and enable them in the board file. while the current series is a "cleanup": - No fixes are introduced by this series and therefore it doesn't require a backport. I can squash the two series and post them as a v2 if that helps. Regards, Siddharth.
On Mon, Apr 14, 2025 at 08:13:19PM +0530, Siddharth Vadapalli wrote: > On Mon, Apr 14, 2025 at 09:39:16AM -0500, Nishanth Menon wrote: > > On 18:30-20250414, Siddharth Vadapalli wrote: > > > On Mon, Apr 14, 2025 at 07:09:30AM -0500, Nishanth Menon wrote: > > > > > > Hello Nishanth, > > > > > > > On 10:57-20250412, Siddharth Vadapalli wrote: > > > > > Hello, > > > > > > > > > > This series is based on the following series: > > > > > https://patchwork.kernel.org/project/linux-arm-kernel/cover/20250408103606.3679505-1-s-vadapalli@ti.com/ > > > > > Based on the discussion in the above series which disabled 'serdes_wiz0' > > > > > and 'serdes_wiz1' nodes in the SoC file and enabled them in the board > > > > > file, Udit pointed out that it wasn't necessary to disable 'serdes0' and > > > > > 'serdes1' in the SoC file anymore, since that is not a working > > > > > configuration - serdes_wizX enabled and serdesX disabled doesn't work. > > > > > > > > > > Hence, this series aims to cleanup the serdesX nodes after the changes > > > > > made by the above series. > > > > > > > > > > Regards, > > > > > Siddharth. > > > > > > > > > > Siddharth Vadapalli (2): > > > > > arm64: dts: ti: k3-j722s-main: don't disable serdes0 and serdes1 > > > > > arm64: dts: ti: k3-j722s-evm: drop redundant status within > > > > > serdes0/serdes1 > > > > > > > > > > arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 2 -- > > > > > arch/arm64/boot/dts/ti/k3-j722s-main.dtsi | 4 ---- > > > > > 2 files changed, 6 deletions(-) > > > > > > > > > > -- > > > > > 2.34.1 > > > > > > > > > > > > > > > > > I do not understand the logic here. serdes cannot operate without wiz > > > > nodes, correct? why would we leave serdes on by default? > > > > > > Yes, serdesX requires serdes_wizX, but at the same time, serdesX is the > > > child node of serdes_wizX. Therefore, without enabling serdes_wizX, we > > > cannot enable serdesX. > > > > > > Prior to this series, but with the dependent series at: > > > https://patchwork.kernel.org/project/linux-arm-kernel/cover/20250408103606.3679505-1-s-vadapalli@ti.com/ > > > applied, the nodes look like: > > > > > > serdes_wizX { > > > ... > > > status = "disabled"; > > > > > > serdesX { > > > ... > > > status = "disabled"; > > > }; > > > }; > > > > > > The dependent series fixes 'serdes_wizX' by disabling it in the SoC file > > > k3-j722s-main.dtsi. But after the fix, we have a 'status = "disabled";' > > > within the serdesX node which isn't required since: > > > a) serdes_wizX enabled but serdesX disabled is non-functional and > > > unusable > > > b) serdes_wizX disabled in DT implies that serdesX is also disabled > > > > > > Can we handle all of this in one series instead of two series? > > The idea behind splitting it was that the dependent series is a "Fix": > - Fix to follow the convention of disabling nodes in SoC file and enable > them in the board file. > while the current series is a "cleanup": > - No fixes are introduced by this series and therefore it doesn't > require a backport. > > I can squash the two series and post them as a v2 if that helps. I have squashed the series and have posted it at: https://lore.kernel.org/r/20250417123246.2733923-1-s-vadapalli@ti.com Regards, Siddharth.