arm64: dts: qcom: sm8150: add reset name for ethernet node

[PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node

Posted by Volodymyr Babchuk 1 year, 11 months ago

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

Re: [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node

Posted by Krzysztof Kozlowski 1 year, 11 months ago

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

Re: [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node

Posted by Dmitry Baryshkov 1 year, 11 months ago

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

Re: [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node

Posted by Krzysztof Kozlowski 1 year, 11 months ago

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

Re: [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node

Posted by Sumit Garg 1 year, 11 months ago

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

Re: [PATCH] arm64: dts: qcom: sm8150: add reset name for ethernet node

Posted by Sumit Garg 1 year, 11 months ago

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