2020-11-08 17:14:18

by Sven Van Asbroeck

[permalink] [raw]
Subject: [PATCH net v4] 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
Reviewed-by: Andrew Lunn <[email protected]>
Signed-off-by: Sven Van Asbroeck <[email protected]>
---

v3 -> v4:
Andrew Lunn:
- create patch against davem/net tree
- add his Reviewed-by tag

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: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git # 4e0396c59559

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 a1938842f828..bd77877cf1cc 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -1026,9 +1026,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)) {
@@ -1044,13 +1044,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-10 23:54:45

by Jakub Kicinski

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

On Sun, 8 Nov 2020 12:12:24 -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
> Reviewed-by: Andrew Lunn <[email protected]>
> Signed-off-by: Sven Van Asbroeck <[email protected]>

Applied, thank you!