* [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node @ 2024-03-06 20:09 Volodymyr Babchuk 2024-03-07 7:02 ` Sumit Garg ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Volodymyr Babchuk @ 2024-03-06 20:09 UTC (permalink / raw) Cc: Sumit Garg, Volodymyr Babchuk, Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, devicetree, linux-kernel Add reset-names property to the ethernet@20000 node. This patch does not change behavior on Linux, but it is needed for U-Boot, as it tries to find the reset by name, not by index. Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> --- arch/arm64/boot/dts/qcom/sm8150.dtsi | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi index 761a6757dc26f..c2e65d6a2ac62 100644 --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi @@ -951,6 +951,7 @@ ethernet: ethernet@20000 { power-domains = <&gcc EMAC_GDSC>; resets = <&gcc GCC_EMAC_BCR>; + resets-names = "emac"; iommus = <&apps_smmu 0x3c0 0x0>; -- 2.43.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node 2024-03-06 20:09 [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node Volodymyr Babchuk @ 2024-03-07 7:02 ` Sumit Garg 2024-03-07 7:10 ` Dmitry Baryshkov 2024-03-07 7:53 ` Krzysztof Kozlowski 2 siblings, 0 replies; 8+ messages in thread From: Sumit Garg @ 2024-03-07 7:02 UTC (permalink / raw) To: Volodymyr Babchuk Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, devicetree, linux-kernel On Thu, 7 Mar 2024 at 01:39, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote: > > Add reset-names property to the ethernet@20000 node. This patch does > not change behavior on Linux, but it is needed for U-Boot, as it tries > to find the reset by name, not by index. > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> > --- > arch/arm64/boot/dts/qcom/sm8150.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi > index 761a6757dc26f..c2e65d6a2ac62 100644 > --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi > @@ -951,6 +951,7 @@ ethernet: ethernet@20000 { > > power-domains = <&gcc EMAC_GDSC>; > resets = <&gcc GCC_EMAC_BCR>; > + resets-names = "emac"; I suppose you meant reset-names here instead? -Sumit > > iommus = <&apps_smmu 0x3c0 0x0>; > > -- > 2.43.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node 2024-03-06 20:09 [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node Volodymyr Babchuk 2024-03-07 7:02 ` Sumit Garg @ 2024-03-07 7:10 ` Dmitry Baryshkov 2024-03-07 7:33 ` Sumit Garg 2024-03-07 7:54 ` Krzysztof Kozlowski 2024-03-07 7:53 ` Krzysztof Kozlowski 2 siblings, 2 replies; 8+ messages in thread From: Dmitry Baryshkov @ 2024-03-07 7:10 UTC (permalink / raw) To: Volodymyr Babchuk Cc: Sumit Garg, Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, devicetree, linux-kernel On Thu, 7 Mar 2024 at 00:22, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote: > > Add reset-names property to the ethernet@20000 node. This patch does > not change behavior on Linux, but it is needed for U-Boot, as it tries > to find the reset by name, not by index. > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> > --- > arch/arm64/boot/dts/qcom/sm8150.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi > index 761a6757dc26f..c2e65d6a2ac62 100644 > --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi > @@ -951,6 +951,7 @@ ethernet: ethernet@20000 { > > power-domains = <&gcc EMAC_GDSC>; > resets = <&gcc GCC_EMAC_BCR>; > + resets-names = "emac"; According to the snps,dwmac.yaml schema the "emac" is invalid here. Only "stmmaceth" and / or "ahb" are permitted here. > > iommus = <&apps_smmu 0x3c0 0x0>; > > -- > 2.43.0 > -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node 2024-03-07 7:10 ` Dmitry Baryshkov @ 2024-03-07 7:33 ` Sumit Garg 2024-03-07 13:13 ` Volodymyr Babchuk 2024-03-07 7:54 ` Krzysztof Kozlowski 1 sibling, 1 reply; 8+ messages in thread From: Sumit Garg @ 2024-03-07 7:33 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Volodymyr Babchuk, Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, devicetree, linux-kernel On Thu, 7 Mar 2024 at 12:40, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Thu, 7 Mar 2024 at 00:22, Volodymyr Babchuk > <Volodymyr_Babchuk@epam.com> wrote: > > > > Add reset-names property to the ethernet@20000 node. This patch does > > not change behavior on Linux, but it is needed for U-Boot, as it tries > > to find the reset by name, not by index. > > > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> > > --- > > arch/arm64/boot/dts/qcom/sm8150.dtsi | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi > > index 761a6757dc26f..c2e65d6a2ac62 100644 > > --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi > > @@ -951,6 +951,7 @@ ethernet: ethernet@20000 { > > > > power-domains = <&gcc EMAC_GDSC>; > > resets = <&gcc GCC_EMAC_BCR>; > > + resets-names = "emac"; > > According to the snps,dwmac.yaml schema the "emac" is invalid here. > Only "stmmaceth" and / or "ahb" are permitted here. Okay, it looks like earlier the Linux kernel on Qcom SoCs always assumed that the EMAC reset signal is deserted by prior boot stages. So I suppose we can reuse "stmmaceth" here instead of "emac" with a corresponding change to U-Boot driver as well. -Sumit > > > > > iommus = <&apps_smmu 0x3c0 0x0>; > > > > -- > > 2.43.0 > > > > > -- > With best wishes > Dmitry ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node 2024-03-07 7:33 ` Sumit Garg @ 2024-03-07 13:13 ` Volodymyr Babchuk 2024-03-07 13:50 ` Sumit Garg 0 siblings, 1 reply; 8+ messages in thread From: Volodymyr Babchuk @ 2024-03-07 13:13 UTC (permalink / raw) To: Sumit Garg Cc: Dmitry Baryshkov, Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, devicetree, linux-kernel Hello Linux Maintainers, Sumit First, I am terribly sorry about this half-assed patch. Generally I am doing all the required checks. But this change seemed so trivial... Anyways, lesson taken, this will not happen anymore. Sumit Garg <sumit.garg@linaro.org> writes: > On Thu, 7 Mar 2024 at 12:40, Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: >> >> On Thu, 7 Mar 2024 at 00:22, Volodymyr Babchuk >> <Volodymyr_Babchuk@epam.com> wrote: >> > >> > Add reset-names property to the ethernet@20000 node. This patch does >> > not change behavior on Linux, but it is needed for U-Boot, as it tries >> > to find the reset by name, not by index. >> > >> > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> >> > --- >> > arch/arm64/boot/dts/qcom/sm8150.dtsi | 1 + >> > 1 file changed, 1 insertion(+) >> > >> > diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi >> > index 761a6757dc26f..c2e65d6a2ac62 100644 >> > --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi >> > +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi >> > @@ -951,6 +951,7 @@ ethernet: ethernet@20000 { >> > >> > power-domains = <&gcc EMAC_GDSC>; >> > resets = <&gcc GCC_EMAC_BCR>; >> > + resets-names = "emac"; >> >> According to the snps,dwmac.yaml schema the "emac" is invalid here. >> Only "stmmaceth" and / or "ahb" are permitted here. > > Okay, it looks like earlier the Linux kernel on Qcom SoCs always > assumed that the EMAC reset signal is deserted by prior boot stages. > So I suppose we can reuse "stmmaceth" here instead of "emac" with a > corresponding change to U-Boot driver as well. Maybe it would be better to access reset in U-Boot by index, in the same way as linux kernel does? I am not sure that "stmmaceth" will be correct from the semantic point of view. As I understand, "stmmac" name is used due to historical reasons in Linux, as this driver was introduced for STM SoC initially. But the same IP block is being used in many different SoCs made by different vendors and there is nothing STM-specific left in it anymore. Especially taking into account that this IP-block was designed not by STM but by Synopsys/DesignWare. -- WBR, Volodymyr ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node 2024-03-07 13:13 ` Volodymyr Babchuk @ 2024-03-07 13:50 ` Sumit Garg 0 siblings, 0 replies; 8+ messages in thread From: Sumit Garg @ 2024-03-07 13:50 UTC (permalink / raw) To: Volodymyr Babchuk Cc: Dmitry Baryshkov, Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, devicetree, linux-kernel On Thu, 7 Mar 2024 at 18:43, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote: > > > Hello Linux Maintainers, Sumit > > First, I am terribly sorry about this half-assed patch. Generally I am > doing all the required checks. But this change seemed so > trivial... Anyways, lesson taken, this will not happen anymore. > > Sumit Garg <sumit.garg@linaro.org> writes: > > > On Thu, 7 Mar 2024 at 12:40, Dmitry Baryshkov > > <dmitry.baryshkov@linaro.org> wrote: > >> > >> On Thu, 7 Mar 2024 at 00:22, Volodymyr Babchuk > >> <Volodymyr_Babchuk@epam.com> wrote: > >> > > >> > Add reset-names property to the ethernet@20000 node. This patch does > >> > not change behavior on Linux, but it is needed for U-Boot, as it tries > >> > to find the reset by name, not by index. > >> > > >> > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> > >> > --- > >> > arch/arm64/boot/dts/qcom/sm8150.dtsi | 1 + > >> > 1 file changed, 1 insertion(+) > >> > > >> > diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi > >> > index 761a6757dc26f..c2e65d6a2ac62 100644 > >> > --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi > >> > +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi > >> > @@ -951,6 +951,7 @@ ethernet: ethernet@20000 { > >> > > >> > power-domains = <&gcc EMAC_GDSC>; > >> > resets = <&gcc GCC_EMAC_BCR>; > >> > + resets-names = "emac"; > >> > >> According to the snps,dwmac.yaml schema the "emac" is invalid here. > >> Only "stmmaceth" and / or "ahb" are permitted here. > > > > Okay, it looks like earlier the Linux kernel on Qcom SoCs always > > assumed that the EMAC reset signal is deserted by prior boot stages. > > So I suppose we can reuse "stmmaceth" here instead of "emac" with a > > corresponding change to U-Boot driver as well. > > Maybe it would be better to access reset in U-Boot by index, in the > same way as linux kernel does? I am not sure that "stmmaceth" will be > correct from the semantic point of view. I can't see the Linux kernel driver accessing reset by index in this case (see [1]). It worked for the Qcom case since the reset signal was by default deserted. IMO, that's an incorrect assumption (you never know what state the bootloader leaves the reset signal in) to start with. This should be fixed via explicit MAC reset desertion in the kernel driver. IOW, this patch would ideally be a fix for the Linux kernel driver rather than replicating what U-Boot does. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c#n645 > > As I understand, "stmmac" name is used due to historical reasons in > Linux, as this driver was introduced for STM SoC initially. But the same > IP block is being used in many different SoCs made by different vendors > and there is nothing STM-specific left in it anymore. Especially taking > into account that this IP-block was designed not by STM but by > Synopsys/DesignWare. As DT bindings are the ABI and we have no choice but to live with it. "stmmaceth" is already being used for "reg-names" and "clock-names" too. -Sumit > > -- > WBR, Volodymyr ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node 2024-03-07 7:10 ` Dmitry Baryshkov 2024-03-07 7:33 ` Sumit Garg @ 2024-03-07 7:54 ` Krzysztof Kozlowski 1 sibling, 0 replies; 8+ messages in thread From: Krzysztof Kozlowski @ 2024-03-07 7:54 UTC (permalink / raw) To: Dmitry Baryshkov, Volodymyr Babchuk Cc: Sumit Garg, Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, devicetree, linux-kernel On 07/03/2024 08:10, Dmitry Baryshkov wrote: > On Thu, 7 Mar 2024 at 00:22, Volodymyr Babchuk > <Volodymyr_Babchuk@epam.com> wrote: >> >> Add reset-names property to the ethernet@20000 node. This patch does >> not change behavior on Linux, but it is needed for U-Boot, as it tries >> to find the reset by name, not by index. >> >> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> >> --- >> arch/arm64/boot/dts/qcom/sm8150.dtsi | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi >> index 761a6757dc26f..c2e65d6a2ac62 100644 >> --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi >> @@ -951,6 +951,7 @@ ethernet: ethernet@20000 { >> >> power-domains = <&gcc EMAC_GDSC>; >> resets = <&gcc GCC_EMAC_BCR>; >> + resets-names = "emac"; > > According to the snps,dwmac.yaml schema the "emac" is invalid here. > Only "stmmaceth" and / or "ahb" are permitted here. If only there was a tool which can tell this, without the need to involve reviewers... Best regards, Krzysztof ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node 2024-03-06 20:09 [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node Volodymyr Babchuk 2024-03-07 7:02 ` Sumit Garg 2024-03-07 7:10 ` Dmitry Baryshkov @ 2024-03-07 7:53 ` Krzysztof Kozlowski 2 siblings, 0 replies; 8+ messages in thread From: Krzysztof Kozlowski @ 2024-03-07 7:53 UTC (permalink / raw) To: Volodymyr Babchuk Cc: Sumit Garg, Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, devicetree, linux-kernel On 06/03/2024 21:09, Volodymyr Babchuk wrote: > Add reset-names property to the ethernet@20000 node. This patch does > not change behavior on Linux, but it is needed for U-Boot, as it tries > to find the reset by name, not by index. > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> > --- > arch/arm64/boot/dts/qcom/sm8150.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi > index 761a6757dc26f..c2e65d6a2ac62 100644 > --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi > @@ -951,6 +951,7 @@ ethernet: ethernet@20000 { > > power-domains = <&gcc EMAC_GDSC>; > resets = <&gcc GCC_EMAC_BCR>; > + resets-names = "emac"; It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). Best regards, Krzysztof ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-03-07 13:50 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-03-06 20:09 [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node Volodymyr Babchuk 2024-03-07 7:02 ` Sumit Garg 2024-03-07 7:10 ` Dmitry Baryshkov 2024-03-07 7:33 ` Sumit Garg 2024-03-07 13:13 ` Volodymyr Babchuk 2024-03-07 13:50 ` Sumit Garg 2024-03-07 7:54 ` Krzysztof Kozlowski 2024-03-07 7:53 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).