[PATCH 1/3] mdio-uclass.c: support fixed-link subnodes

[Heiko Schocher]

Hello Rasmus,

Am 05.10.2020 um 15:15 schrieb Rasmus Villemoes:
> When trying to port our mpc8309-based board to DM_ETH, on top of
> Heiko's patches, I found that nothing in mdio-uclass.c seems to
> support the use of a fixed-link subnode of the ethernet DT node. That
> is, the ethernet node looks like
> 
> 		enet0: ethernet at 2000 {
> 			device_type = "network";
> 			compatible = "ucc_geth";
> 			...
> 			fixed-link {
> 				reg = <0xffffffff>;
> 				speed = <100>;
> 				full-duplex;
> 			};
> 
> but the current code expects there to be phy-handle property. Adding
> that, i.e.
> 
> 			phy-handle = <&enet0phy>;
> 			enet0phy: fixed-link {
> 
> just makes the code break a few lines later since a fixed-link node
> doesn't have a reg property. Ignoring the dtc complaint and adding a
> dummy reg property, we of course hit "can't find MDIO bus for node
> ethernet at 2000" since indeed, the parent node of the phy node does not
> represent an MDIO bus. So that's obviously the wrong path.
> 
> Now, in linux, it seems that the fixed link case is treated specially;
> in the of_phy_get_and_connect() which roughly corresponds to
> dm_eth_connect_phy_handle() we have
> 
>      if (of_phy_is_fixed_link(np)) {
>          ret = of_phy_register_fixed_link(np);
>          ...
>      } else {
>          phy_np = of_parse_phandle(np, "phy-handle", 0);
> 	...
>      }
> 
>      phy = of_phy_connect(dev, phy_np, hndlr, 0, iface);
> 
> And U-Boot's phy_connect() does have support for fixed-link
> subnodes. Calling phy_connect() directly with NULL bus and a dummy
> address does seem to make the ethernet work.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
> ---
>   net/mdio-uclass.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/net/mdio-uclass.c b/net/mdio-uclass.c
> index 66ee2e1976..2f932b77df 100644
> --- a/net/mdio-uclass.c
> +++ b/net/mdio-uclass.c
> @@ -139,6 +139,12 @@ static struct phy_device *dm_eth_connect_phy_handle(struct udevice *ethdev,
>   	struct ofnode_phandle_args phandle = {.node = ofnode_null()};
>   	int i;
>   
> +	if (CONFIG_IS_ENABLED(PHY_FIXED) &&
> +	    ofnode_valid(dev_read_subnode(ethdev, "fixed-link"))) {
> +		phy = phy_connect(NULL, -1, ethdev, interface);
> +		goto out;
> +	}
> +
>   	for (i = 0; i < PHY_HANDLE_STR_CNT; i++)
>   		if (!dev_read_phandle_with_args(ethdev, phy_handle_str[i], NULL,
>   						0, 0, &phandle))
> @@ -168,6 +174,7 @@ static struct phy_device *dm_eth_connect_phy_handle(struct udevice *ethdev,
>   
>   	phy = dm_mdio_phy_connect(mdiodev, phy_addr, ethdev, interface);
>   
> +out:
>   	if (phy)
>   		phy->node = phandle.node;

Looks good to me... but I am not an expert in net subsystem...
nevertheless:

Reviewed-by: Heiko Schocher <hs at denx.de>

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de