netdev - Re: drivers/of/of_mdio.c needs a small modification

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: <76f88763-54b0-eb03-3bc8-3e5022173163@arf.net.pl>
Date:   Sat, 29 Aug 2020 17:37:29 +0200
From:   Adam Rudziński <adam.rudzinski@....net.pl>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        netdev <netdev@...r.kernel.org>, robh+dt@...nel.org,
        frowand.list@...il.com
Subject: Re: drivers/of/of_mdio.c needs a small modification

W dniu 2020-08-29 o 17:15, Andrew Lunn pisze:
>> The driver would be able to add the new PHYs to the shared MDIO bus by
>> calling of_mdiobus_register_children. Then the device tree looks like this,
>> which is more reasonable in my opinion:
>>
>> &fec2 {
>> (...)
>>      mdio {
>>          (phy for fec2 here)
>>      };
>> (...)
>> };
>>
>> &fec1 {
>> (...)
>>      mdio {
>>          (phy for fec1 here)
>>      };
>> (...)
>> };
> DT describes hardware, and the topology of the hardware. The hardware really is:
>
> ethernet1@...ec000 {
> 	compatible = "fsl,imx51-fec", "fsl,imx27-fec";
> 	reg = <0x83fec000 0x4000>;
> 	interrupts = <87>;
> 	phy-mode = "mii";
> 	phy-reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>; /* GPIO2_14 */
> 	local-mac-address = [00 04 9F 01 1B B9];
> 	phy-supply = <&reg_fec_supply>;
> 	phy-handle = <&ethphy1>;
> 	mdio {
> 	        clock-frequency = <5000000>;
> 		ethphy1: ethernet-phy@1 {
> 			compatible = "ethernet-phy-ieee802.3-c22";
> 			reg = <1>;
> 			max-speed = <100>;
> 		};
> 		ethphy2: ethernet-phy@2 {
> 			compatible = "ethernet-phy-ieee802.3-c22";
> 			reg = <2>;
> 			max-speed = <100>;
> 		};
> 	};
> };
>
> ethernet2@...ec000 {
> 	compatible = "fsl,imx51-fec", "fsl,imx27-fec";
> 	reg = <0x83fec000 0x4000>;
> 	interrupts = <87>;
> 	phy-mode = "mii";
> 	phy-reset-gpios = <&gpio2 15 GPIO_ACTIVE_LOW>; /* GPIO2_15 */
> 	local-mac-address = [00 04 9F 01 1B BA];
> 	phy-supply = <&reg_fec_supply>;
> 	phy-handle = <&ethphy2>;
> };

This is true assuming that the PHYs are always and forever connected to 
one specific MDIO bus. This is probably reasonable. Although, in i.MX 
the MDIO bus of FEC1 and FEC2 shares the pins. So with this "split" 
description one can just comment out FEC2 and still enjoy operational 
ethernet (FEC1 + its PHY). This may be simpler for hardware guys, like 
me, who don't have that much experience with device trees. But yes, that 
doesn't necessarily mean it's the good way to go.

> Also look at drivers/net/phy/micrel.c. It has code to look up a FEC
> clock and use it. But that code assumes the PHY responds to MDIO reads
> when the clock is not ticking. It sounds like your PHY does not?
> Please double check that. If it does not, you need to add clock code
> to the PHY core. Florians patchset will help with that.
>
I'm sure of that - LAN8720A needs to have the clock from FEC or external 
generator to be able to talk over MDIO.

I can see that one way or another, it's still some modification of the 
source code. You know the ropes, you decide if and which one makes sense.

Best regards,
Adam