linux-kernel - Re: [PATCH 01/23] arm64: zynqmp: Describe TI phy as ethernet-phy-id

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9e94c275-4752-061c-3090-144d83769eb6@amd.com>
Date:   Wed, 10 May 2023 09:11:37 +0200
From:   Michal Simek <michal.simek@....com>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:     linux-kernel@...r.kernel.org, monstr@...str.eu,
        michal.simek@...inx.com, git@...inx.com,
        Amit Kumar Mahapatra <amit.kumar-mahapatra@...inx.com>,
        Ashok Reddy Soma <ashok.reddy.soma@...inx.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Parth Gajjar <parth.gajjar@....com>,
        Rob Herring <robh+dt@...nel.org>,
        Vishal Sagar <vishal.sagar@....com>,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 01/23] arm64: zynqmp: Describe TI phy as ethernet-phy-id

Hi Laurent,

On 5/10/23 08:52, Laurent Pinchart wrote:
> Hi Michal,
> 
> Thank you for the patch.
> 
> On Tue, May 02, 2023 at 03:35:29PM +0200, Michal Simek wrote:
>> TI DP83867 is using strapping based on MIO pins. Tristate setup can influce

And typo here.

>> PHY address. That's why switch description with ethernet-phy-id compatible
>> string which enable calling reset. PHY itself setups phy address after
>> power up or reset.
> 
> I'm sorry but I don't understand this :-(

What exactly is not clear? Phy has some pins which is using for strapping for 
phy address after phy reset or power on. Pretty much it is resistor array which 
based on datasheet is decoded to certain phy address.
And because some phy pins are also used as data pin for RGMII they are connected 
via MIO pins on a silicon. That's why IO block output setting really matter here 
because it changes resistor array and it moves phy address.
That's why there is a need to do proper IO pin setup and after it call phy reset 
to get it to address which was decided by PCB designer.

> 
>> Phy reset is done via gpio.
>>
>> Signed-off-by: Michal Simek <michal.simek@....com>
>> ---
>>
>> Checkpatch is reporting issue
>> warning: DT compatible string "ethernet-phy-id2000.a231" appears un-documented
>> but it should be fully aligned with
>> Documentation/devicetree/bindings/net/ethernet-phy.yaml
>> ---
>>   .../boot/dts/xilinx/zynqmp-zcu102-revA.dts    | 23 +++++++++++------
>>   .../boot/dts/xilinx/zynqmp-zcu102-revB.dts    | 25 +++++++++++--------
>>   .../boot/dts/xilinx/zynqmp-zcu104-revA.dts    | 22 ++++++++++------
>>   .../boot/dts/xilinx/zynqmp-zcu104-revC.dts    | 22 ++++++++++------
>>   .../boot/dts/xilinx/zynqmp-zcu106-revA.dts    | 22 ++++++++++------
>>   .../boot/dts/xilinx/zynqmp-zcu111-revA.dts    | 22 ++++++++++------
>>   6 files changed, 90 insertions(+), 46 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
>> index 13c43324f1d2..c193579400cf 100644
>> --- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
>> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
>> @@ -2,7 +2,8 @@
>>   /*
>>    * dts file for Xilinx ZynqMP ZCU102 RevA
>>    *
>> - * (C) Copyright 2015 - 2021, Xilinx, Inc.
>> + * (C) Copyright 2015 - 2022, Xilinx, Inc.
>> + * (C) Copyright 2022 - 2023, Advanced Micro Devices, Inc.
>>    *
>>    * Michal Simek <michal.simek@...inx.com>
>>    */
>> @@ -200,13 +201,19 @@ &gem3 {
>>   	phy-mode = "rgmii-id";
>>   	pinctrl-names = "default";
>>   	pinctrl-0 = <&pinctrl_gem3_default>;
>> -	phy0: ethernet-phy@21 {
>> -		reg = <21>;
>> -		ti,rx-internal-delay = <0x8>;
>> -		ti,tx-internal-delay = <0xa>;
>> -		ti,fifo-depth = <0x1>;
>> -		ti,dp83867-rxctrl-strap-quirk;
>> -		/* reset-gpios = <&tca6416_u97 6 GPIO_ACTIVE_LOW>; */
>> +	mdio: mdio {
> 
> The "mdio" label isn't needed. Same below.

I am doing it by purpose to be able to reference all nodes and add/remove 
property in an easy way. There shouldn't be any conflict with dt spec to have 
labels even they are not used in current DT.

Thanks,
Michal