Re: [PATCH 1/2] net: dsa: fix phydev->speed being uninitialized for the CPU port fixed PHY

[Tim Harvey <tharvey@gateworks.com>]

On Sat, Dec 4, 2021 at 3:01 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> If the DSA API is going to allow drivers to do things such as:
>
> - phy_config in dsa_ops :: port_probe
> - phy_startup in dsa_ops :: port_enable
>
> then it would actually be good if the ->port_probe() method would
> actually be called in all cases before the ->port_enable() is.
>
> Currently this is true for user ports, but not true for the CPU port,
> because the CPU port does not have a udevice registered for it (this is
> all part of DSA's design). So the current issue is that after
> phy_startup has finished for the CPU port, its phydev->speed is an
> uninitialized value, because phy_config() was never called for the
> priv->cpu_port_fixed_phy, and it is precisely phy_config() who copies
> the speed into the phydev in the case of the fixed PHY driver.
>
> So we need to simulate a probing event for the CPU port by manually
> calling the driver's ->port_probe() method for the CPU port.
>
> Fixes: 8a2982574854 ("net: dsa: introduce a .port_probe() method in struct dsa_ops")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  net/dsa-uclass.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
> index 606b1539a776..9ff55a02fb23 100644
> --- a/net/dsa-uclass.c
> +++ b/net/dsa-uclass.c
> @@ -466,6 +466,8 @@ static int dsa_pre_probe(struct udevice *dev)
>  {
>         struct dsa_pdata *pdata = dev_get_uclass_plat(dev);
>         struct dsa_priv *priv = dev_get_uclass_priv(dev);
> +       struct dsa_ops *ops = dsa_get_ops(dev);
> +       int err;
>
>         priv->num_ports = pdata->num_ports;
>         priv->cpu_port = pdata->cpu_port;
> @@ -477,6 +479,15 @@ static int dsa_pre_probe(struct udevice *dev)
>
>         uclass_find_device_by_ofnode(UCLASS_ETH, pdata->master_node,
>                                      &priv->master_dev);
> +
> +       /* Simulate a probing event for the CPU port */
> +       if (ops->port_probe) {
> +               err = ops->port_probe(dev, priv->cpu_port,
> +                                     priv->cpu_port_fixed_phy);
> +               if (err)
> +                       return err;
> +       }
> +
>         return 0;
>  }
>
> --
> 2.25.1
>

Vladimir,

I came across this while looking for the best place to configure cpu
port interface mode (ie rgmii id) for the mv88e61xx dsa driver I'm
working on. Note that this patch causes port_probe to be called on the
cpu port 'before' the master device has been probed. I'm not sure if
this is intended or not?

In my case I was looking to configure the cpu port interface mode when
the port was probed but I can't do that because it happens before the
switch is probed because of some things that need to happen there.
Best Regards,

Tim