[PATCH 07/15] ARM: zynq: Add ethernet phy reset information to DT(zc702)

Michal Simek posted 15 patches 1 year ago
[PATCH 07/15] ARM: zynq: Add ethernet phy reset information to DT(zc702)
Posted by Michal Simek 1 year ago
Added phy reset gpio information for gem0.

Signed-off-by: Michal Simek <michal.simek@amd.com>
---

 arch/arm/boot/dts/xilinx/zynq-zc702.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/xilinx/zynq-zc702.dts b/arch/arm/boot/dts/xilinx/zynq-zc702.dts
index 424e78f6c148..975385f4ac01 100644
--- a/arch/arm/boot/dts/xilinx/zynq-zc702.dts
+++ b/arch/arm/boot/dts/xilinx/zynq-zc702.dts
@@ -79,6 +79,8 @@ &gem0 {
 	phy-handle = <&ethernet_phy>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_gem0_default>;
+	phy-reset-gpio = <&gpio0 11 0>;
+	phy-reset-active-low;
 
 	ethernet_phy: ethernet-phy@7 {
 		reg = <7>;
-- 
2.43.0
Re: [PATCH 07/15] ARM: zynq: Add ethernet phy reset information to DT(zc702)
Posted by Andrew Lunn 1 year ago
On Wed, Dec 11, 2024 at 01:41:26PM +0100, Michal Simek wrote:
> Added phy reset gpio information for gem0.
> 
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
> 
>  arch/arm/boot/dts/xilinx/zynq-zc702.dts | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/xilinx/zynq-zc702.dts b/arch/arm/boot/dts/xilinx/zynq-zc702.dts
> index 424e78f6c148..975385f4ac01 100644
> --- a/arch/arm/boot/dts/xilinx/zynq-zc702.dts
> +++ b/arch/arm/boot/dts/xilinx/zynq-zc702.dts
> @@ -79,6 +79,8 @@ &gem0 {
>  	phy-handle = <&ethernet_phy>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_gem0_default>;
> +	phy-reset-gpio = <&gpio0 11 0>;
> +	phy-reset-active-low;

Hi Michal

Could you point me at code which actually implements these two
properties.

What is more normal is a reset-gpios property in the PHY node, or a
reset-gpios in the MDIO node.

	Andrew
Re: [PATCH 07/15] ARM: zynq: Add ethernet phy reset information to DT(zc702)
Posted by Michal Simek 1 year ago
Hi Andrew,

On 12/11/24 14:45, Andrew Lunn wrote:
> On Wed, Dec 11, 2024 at 01:41:26PM +0100, Michal Simek wrote:
>> Added phy reset gpio information for gem0.
>>
>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>> ---
>>
>>   arch/arm/boot/dts/xilinx/zynq-zc702.dts | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/xilinx/zynq-zc702.dts b/arch/arm/boot/dts/xilinx/zynq-zc702.dts
>> index 424e78f6c148..975385f4ac01 100644
>> --- a/arch/arm/boot/dts/xilinx/zynq-zc702.dts
>> +++ b/arch/arm/boot/dts/xilinx/zynq-zc702.dts
>> @@ -79,6 +79,8 @@ &gem0 {
>>   	phy-handle = <&ethernet_phy>;
>>   	pinctrl-names = "default";
>>   	pinctrl-0 = <&pinctrl_gem0_default>;
>> +	phy-reset-gpio = <&gpio0 11 0>;
>> +	phy-reset-active-low;
> 
> Hi Michal
> 
> Could you point me at code which actually implements these two
> properties.

I have tracked it down and it was only used from 2016 to 2018 in Xilinx 
downstream Linux repository. After that it was removed.

If you are interested
https://gitenterprise.xilinx.com/Linux/linux-xlnx/commit/6f43a25c416e532530eaed897acff6f5249907e4

> 
> What is more normal is a reset-gpios property in the PHY node, or a
> reset-gpios in the MDIO node.

Thanks for review. This patch should be dropped from this series.

Thanks,
Michal
Re: [PATCH 07/15] ARM: zynq: Add ethernet phy reset information to DT(zc702)
Posted by Andrew Lunn 1 year ago
On Wed, Dec 11, 2024 at 03:04:28PM +0100, Michal Simek wrote:
> Hi Andrew,
> 
> On 12/11/24 14:45, Andrew Lunn wrote:
> > On Wed, Dec 11, 2024 at 01:41:26PM +0100, Michal Simek wrote:
> > > Added phy reset gpio information for gem0.
> > > 
> > > Signed-off-by: Michal Simek <michal.simek@amd.com>
> > > ---
> > > 
> > >   arch/arm/boot/dts/xilinx/zynq-zc702.dts | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/xilinx/zynq-zc702.dts b/arch/arm/boot/dts/xilinx/zynq-zc702.dts
> > > index 424e78f6c148..975385f4ac01 100644
> > > --- a/arch/arm/boot/dts/xilinx/zynq-zc702.dts
> > > +++ b/arch/arm/boot/dts/xilinx/zynq-zc702.dts
> > > @@ -79,6 +79,8 @@ &gem0 {
> > >   	phy-handle = <&ethernet_phy>;
> > >   	pinctrl-names = "default";
> > >   	pinctrl-0 = <&pinctrl_gem0_default>;
> > > +	phy-reset-gpio = <&gpio0 11 0>;
> > > +	phy-reset-active-low;
> > 
> > Hi Michal
> > 
> > Could you point me at code which actually implements these two
> > properties.
> 
> I have tracked it down and it was only used from 2016 to 2018 in Xilinx
> downstream Linux repository. After that it was removed.

O.K. So when you ran the DT lint tools to validate these DT changes,
why did it not give errors pointing this out?

I wounder how many other such errors there are in this patchset?

	Andrew
Re: [PATCH 07/15] ARM: zynq: Add ethernet phy reset information to DT(zc702)
Posted by Michal Simek 1 year ago

On 12/11/24 15:21, Andrew Lunn wrote:
> On Wed, Dec 11, 2024 at 03:04:28PM +0100, Michal Simek wrote:
>> Hi Andrew,
>>
>> On 12/11/24 14:45, Andrew Lunn wrote:
>>> On Wed, Dec 11, 2024 at 01:41:26PM +0100, Michal Simek wrote:
>>>> Added phy reset gpio information for gem0.
>>>>
>>>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>>>> ---
>>>>
>>>>    arch/arm/boot/dts/xilinx/zynq-zc702.dts | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/xilinx/zynq-zc702.dts b/arch/arm/boot/dts/xilinx/zynq-zc702.dts
>>>> index 424e78f6c148..975385f4ac01 100644
>>>> --- a/arch/arm/boot/dts/xilinx/zynq-zc702.dts
>>>> +++ b/arch/arm/boot/dts/xilinx/zynq-zc702.dts
>>>> @@ -79,6 +79,8 @@ &gem0 {
>>>>    	phy-handle = <&ethernet_phy>;
>>>>    	pinctrl-names = "default";
>>>>    	pinctrl-0 = <&pinctrl_gem0_default>;
>>>> +	phy-reset-gpio = <&gpio0 11 0>;
>>>> +	phy-reset-active-low;
>>>
>>> Hi Michal
>>>
>>> Could you point me at code which actually implements these two
>>> properties.
>>
>> I have tracked it down and it was only used from 2016 to 2018 in Xilinx
>> downstream Linux repository. After that it was removed.
> 
> O.K. So when you ran the DT lint tools to validate these DT changes,
> why did it not give errors pointing this out?
> 
> I wounder how many other such errors there are in this patchset?

That's definitely valid concern.
I was running it but for all files there are a lot of other issues generated 
that I missed this one.

../linux/arch/arm/boot/dts/xilinx/zynq-zc702.dtb: ethernet@e000b000: Unevaluated 
properties are not allowed ('phy-reset-active-low', 'phy-reset-gpio' were 
unexpected)

Thanks,
Michal