2020-11-08 14:09:30

by Sven Van Asbroeck

[permalink] [raw]
Subject: [PATCH v3] lan743x: correctly handle chips with internal PHY

From: Sven Van Asbroeck <[email protected]>

Commit 6f197fb63850 ("lan743x: Added fixed link and RGMII support")
assumes that chips with an internal PHY will never have a devicetree
entry. This is incorrect: even for these chips, a devicetree entry
can be useful e.g. to pass the mac address from bootloader to chip:

&pcie {
status = "okay";

host@0 {
reg = <0 0 0 0 0>;

#address-cells = <3>;
#size-cells = <2>;

lan7430: ethernet@0 {
/* LAN7430 with internal PHY */
compatible = "microchip,lan743x";
status = "okay";
reg = <0 0 0 0 0>;
/* filled in by bootloader */
local-mac-address = [00 00 00 00 00 00];
};
};
};

If a devicetree entry is present, the driver will not attach the chip
to its internal phy, and the chip will be non-operational.

Fix by tweaking the phy connection algorithm:
- first try to connect to a phy specified in the devicetree
(could be 'real' phy, or just a 'fixed-link')
- if that doesn't succeed, try to connect to an internal phy, even
if the chip has a devnode

Tested on a LAN7430 with internal PHY. I cannot test a device using
fixed-link, as I do not have access to one.

Fixes: 6f197fb63850 ("lan743x: Added fixed link and RGMII support")
Tested-by: Sven Van Asbroeck <[email protected]> # lan7430
Signed-off-by: Sven Van Asbroeck <[email protected]>
---

v2 -> v3:
Andrew Lunn: make patch truly minimal.

v1 -> v2:
Andrew Lunn: keep patch minimal and correct, so keep open-coded version
of of_phy_get_and_connect().

Jakub Kicinski: fix e-mail address case.

Tree: v5.10-rc2

To: Andrew Lunn <[email protected]>
To: Bryan Whitehead <[email protected]>
To: "David S. Miller" <[email protected]>
To: Jakub Kicinski <[email protected]>
Cc: Microchip Linux Driver Support <[email protected]>
Cc: Roelof Berg <[email protected]>
Cc: [email protected]
Cc: [email protected]

drivers/net/ethernet/microchip/lan743x_main.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index f2d13e8d20f0..54d721ef3084 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -1021,9 +1021,9 @@ static int lan743x_phy_open(struct lan743x_adapter *adapter)

netdev = adapter->netdev;
phynode = of_node_get(adapter->pdev->dev.of_node);
- adapter->phy_mode = PHY_INTERFACE_MODE_GMII;

if (phynode) {
+ /* try devicetree phy, or fixed link */
of_get_phy_mode(phynode, &adapter->phy_mode);

if (of_phy_is_fixed_link(phynode)) {
@@ -1039,13 +1039,15 @@ static int lan743x_phy_open(struct lan743x_adapter *adapter)
lan743x_phy_link_status_change, 0,
adapter->phy_mode);
of_node_put(phynode);
- if (!phydev)
- goto return_error;
- } else {
+ }
+
+ if (!phydev) {
+ /* try internal phy */
phydev = phy_find_first(adapter->mdiobus);
if (!phydev)
goto return_error;

+ adapter->phy_mode = PHY_INTERFACE_MODE_GMII;
ret = phy_connect_direct(netdev, phydev,
lan743x_phy_link_status_change,
adapter->phy_mode);
--
2.17.1


2020-11-08 16:00:19

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v3] lan743x: correctly handle chips with internal PHY

On Sun, Nov 08, 2020 at 09:06:53AM -0500, Sven Van Asbroeck wrote:
> From: Sven Van Asbroeck <[email protected]>
>
> Commit 6f197fb63850 ("lan743x: Added fixed link and RGMII support")
> assumes that chips with an internal PHY will never have a devicetree
> entry. This is incorrect: even for these chips, a devicetree entry
> can be useful e.g. to pass the mac address from bootloader to chip:
>
> &pcie {
> status = "okay";
>
> host@0 {
> reg = <0 0 0 0 0>;
>
> #address-cells = <3>;
> #size-cells = <2>;
>
> lan7430: ethernet@0 {
> /* LAN7430 with internal PHY */
> compatible = "microchip,lan743x";
> status = "okay";
> reg = <0 0 0 0 0>;
> /* filled in by bootloader */
> local-mac-address = [00 00 00 00 00 00];
> };
> };
> };
>
> If a devicetree entry is present, the driver will not attach the chip
> to its internal phy, and the chip will be non-operational.
>
> Fix by tweaking the phy connection algorithm:
> - first try to connect to a phy specified in the devicetree
> (could be 'real' phy, or just a 'fixed-link')
> - if that doesn't succeed, try to connect to an internal phy, even
> if the chip has a devnode
>
> Tested on a LAN7430 with internal PHY. I cannot test a device using
> fixed-link, as I do not have access to one.
>
> Fixes: 6f197fb63850 ("lan743x: Added fixed link and RGMII support")
> Tested-by: Sven Van Asbroeck <[email protected]> # lan7430
> Signed-off-by: Sven Van Asbroeck <[email protected]>
> ---
>
> v2 -> v3:
> Andrew Lunn: make patch truly minimal.

This is looking much better. Thanks.

>
> v1 -> v2:
> Andrew Lunn: keep patch minimal and correct, so keep open-coded version
> of of_phy_get_and_connect().
>
> Jakub Kicinski: fix e-mail address case.
>
> Tree: v5.10-rc2

Since this is a networking fix, it should be against:

git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git

Please also put in the subject [PATCH net v4] to make it clear it is
for the net tree. You can find more about this here:

https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html

Reviewed-by: Andrew Lunn <[email protected]>

Andrew