| Message ID | 20230208124025.5828-1-guanwentao@uniontech.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: stmmac: get phydev->interface from mac for mdio phy init | expand |
On Wed, Feb 08, 2023 at 08:40:25PM +0800, Guan Wentao wrote: > The phy->interface from mdiobus_get_phy is default from phy_device_create. > In some phy devices like at803x, use phy->interface to init rgmii delay. > Use plat->phy_interface to init if know from stmmac_probe_config_dt. > > Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic") > Signed-off-by: Guan Wentao <guanwentao@uniontech.com> > --- This is v2 of this patch, so let me make some comments about that. * Firstly, unless asked to repost by a reviewer/maintainer, it's generally bad practice to post a patch(set) more than once within 24h. * If it is a networking but fix, then it should be targeted at the 'net' tree. Otherwise, networking patches should be targeted at the 'net-next' tree. In either case this should be noted in the subject. Also, v2 (and so on), should be noted in the subject. Something like this: [PATCH v2 net-next] net: stmmac: get phydev->interface from mac for mdio phy * When posting revised patches, it's important to note what has changed. typically that goes below the scissors ('---'). Something like this; v2: * Fixed blah * Updated foo * Please read the FAQ https://kernel.org/doc/html/latest/process/maintainer-netdev.html > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 1a5b8dab5e9b..debfcb045c22 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -1162,6 +1162,12 @@ static int stmmac_init_phy(struct net_device *dev) > return -ENODEV; > } > > + /* If we know the interface, it defines which PHY interface */ > + if (priv->plat->phy_interface > 0) { > + phydev->interface = priv->plat->phy_interface; > + netdev_dbg(priv->dev, "Override default phy interface\n"); > + } > + > ret = phylink_connect_phy(priv->phylink, phydev); > } > > -- > 2.20.1 >
On Wed, Feb 08, 2023 at 02:11:52PM +0100, Simon Horman wrote: > On Wed, Feb 08, 2023 at 08:40:25PM +0800, Guan Wentao wrote: > > The phy->interface from mdiobus_get_phy is default from phy_device_create. > > In some phy devices like at803x, use phy->interface to init rgmii delay. > > Use plat->phy_interface to init if know from stmmac_probe_config_dt. > > > > Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic") > > Signed-off-by: Guan Wentao <guanwentao@uniontech.com> > > --- > > This is v2 of this patch, so let me make some comments about that. > > * Firstly, unless asked to repost by a reviewer/maintainer, > it's generally bad practice to post a patch(set) more than once within 24h. Hi Guan I just showed you why there is this 24 hour rule by replying to your first version... Andrew
[Not fully over covid but I spotted this and don't agree with this change] On Wed, Feb 08, 2023 at 08:40:25PM +0800, Guan Wentao wrote: > The phy->interface from mdiobus_get_phy is default from phy_device_create. > In some phy devices like at803x, use phy->interface to init rgmii delay. > Use plat->phy_interface to init if know from stmmac_probe_config_dt. > > Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic") > Signed-off-by: Guan Wentao <guanwentao@uniontech.com> > --- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 1a5b8dab5e9b..debfcb045c22 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -1162,6 +1162,12 @@ static int stmmac_init_phy(struct net_device *dev) > return -ENODEV; > } > > + /* If we know the interface, it defines which PHY interface */ > + if (priv->plat->phy_interface > 0) { > + phydev->interface = priv->plat->phy_interface; > + netdev_dbg(priv->dev, "Override default phy interface\n"); > + } > + Why do you need to do this? You call phylink_create() with ->phy_interface, which tells phylink which interface you want to use. Then, phylink_connect_phy(). phylink will then call phylink_attach_phy() and then phy_attach_direct() with the interface you asked for (which was ->phy_interface). phy_attach_direct() will then set phydev->interface to that interface mode. So, I think what you have above is a hack rather than a proper fix, and the real problem is elsewhere.
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 1a5b8dab5e9b..debfcb045c22 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1162,6 +1162,12 @@ static int stmmac_init_phy(struct net_device *dev) return -ENODEV; } + /* If we know the interface, it defines which PHY interface */ + if (priv->plat->phy_interface > 0) { + phydev->interface = priv->plat->phy_interface; + netdev_dbg(priv->dev, "Override default phy interface\n"); + } + ret = phylink_connect_phy(priv->phylink, phydev); }
The phy->interface from mdiobus_get_phy is default from phy_device_create. In some phy devices like at803x, use phy->interface to init rgmii delay. Use plat->phy_interface to init if know from stmmac_probe_config_dt. Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic") Signed-off-by: Guan Wentao <guanwentao@uniontech.com> --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++++ 1 file changed, 6 insertions(+)