| Message ID | E1svfMA-005ZI3-Va@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | net: pcs: xpcs: cleanups batch 1 | expand |
Hi Russell On Tue, Oct 01, 2024 at 05:04:10PM GMT, Russell King (Oracle) wrote: > Move the PCS reset to .pcs_pre_config() rather than at creation time, > which means we call the reset function with the interface that we're > actually going to be using to talk to the downstream device. > > Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com> > Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com> # sja1105 > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> Continuing the RFC discussion. As I mentioned here: https://lore.kernel.org/netdev/mykeabksgikgk6otbub2i3ksfettbozuhqy3gt5vyezmemvttg@cpjn5bcfiwei/ The PCS-reset procedure actually can be converted to being independent from the PHY-interface. Thus you won't need to move the PCS resetting to the pre_config() method, and get rid from the pointer to dw_xpcs_compat utilization each time the reset is required. So why not to merge in my patch to your series as a pre-requisite change and then this patch can be converted to just dropping the xpcs_find_compat() method call from the xpcs_init_iface() function? Alternatively the dropping can be just incorporated into my patch. -Serge(y) > --- > drivers/net/pcs/pcs-xpcs.c | 39 +++++++++++++++++++++++++++--------- > include/linux/pcs/pcs-xpcs.h | 1 + > 2 files changed, 30 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c > index 82463f9d50c8..7c6c40ddf722 100644 > --- a/drivers/net/pcs/pcs-xpcs.c > +++ b/drivers/net/pcs/pcs-xpcs.c > @@ -659,6 +659,30 @@ int xpcs_config_eee(struct dw_xpcs *xpcs, int mult_fact_100ns, int enable) > } > EXPORT_SYMBOL_GPL(xpcs_config_eee); > > +static void xpcs_pre_config(struct phylink_pcs *pcs, phy_interface_t interface) > +{ > + struct dw_xpcs *xpcs = phylink_pcs_to_xpcs(pcs); > + const struct dw_xpcs_compat *compat; > + int ret; > + > + if (!xpcs->need_reset) > + return; > + > + compat = xpcs_find_compat(xpcs->desc, interface); > + if (!compat) { > + dev_err(&xpcs->mdiodev->dev, "unsupported interface %s\n", > + phy_modes(interface)); > + return; > + } > + > + ret = xpcs_soft_reset(xpcs, compat); > + if (ret) > + dev_err(&xpcs->mdiodev->dev, "soft reset failed: %pe\n", > + ERR_PTR(ret)); > + > + xpcs->need_reset = false; > +} > + > static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs, > unsigned int neg_mode) > { > @@ -1365,6 +1389,7 @@ static const struct dw_xpcs_desc xpcs_desc_list[] = { > > static const struct phylink_pcs_ops xpcs_phylink_ops = { > .pcs_validate = xpcs_validate, > + .pcs_pre_config = xpcs_pre_config, > .pcs_config = xpcs_config, > .pcs_get_state = xpcs_get_state, > .pcs_an_restart = xpcs_an_restart, > @@ -1460,18 +1485,12 @@ static int xpcs_init_id(struct dw_xpcs *xpcs) > > static int xpcs_init_iface(struct dw_xpcs *xpcs, phy_interface_t interface) > { > - const struct dw_xpcs_compat *compat; > - > - compat = xpcs_find_compat(xpcs->desc, interface); > - if (!compat) > - return -EINVAL; > - > - if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID) { > + if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID) > xpcs->pcs.poll = false; > - return 0; > - } > + else > + xpcs->need_reset = true; > > - return xpcs_soft_reset(xpcs, compat); > + return 0; > } > > static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev, > diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h > index b4a4eb6c8866..fd75d0605bb6 100644 > --- a/include/linux/pcs/pcs-xpcs.h > +++ b/include/linux/pcs/pcs-xpcs.h > @@ -61,6 +61,7 @@ struct dw_xpcs { > struct clk_bulk_data clks[DW_XPCS_NUM_CLKS]; > struct phylink_pcs pcs; > phy_interface_t interface; > + bool need_reset; > }; > > int xpcs_get_an_mode(struct dw_xpcs *xpcs, phy_interface_t interface); > -- > 2.30.2 > >
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c index 82463f9d50c8..7c6c40ddf722 100644 --- a/drivers/net/pcs/pcs-xpcs.c +++ b/drivers/net/pcs/pcs-xpcs.c @@ -659,6 +659,30 @@ int xpcs_config_eee(struct dw_xpcs *xpcs, int mult_fact_100ns, int enable) } EXPORT_SYMBOL_GPL(xpcs_config_eee); +static void xpcs_pre_config(struct phylink_pcs *pcs, phy_interface_t interface) +{ + struct dw_xpcs *xpcs = phylink_pcs_to_xpcs(pcs); + const struct dw_xpcs_compat *compat; + int ret; + + if (!xpcs->need_reset) + return; + + compat = xpcs_find_compat(xpcs->desc, interface); + if (!compat) { + dev_err(&xpcs->mdiodev->dev, "unsupported interface %s\n", + phy_modes(interface)); + return; + } + + ret = xpcs_soft_reset(xpcs, compat); + if (ret) + dev_err(&xpcs->mdiodev->dev, "soft reset failed: %pe\n", + ERR_PTR(ret)); + + xpcs->need_reset = false; +} + static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs, unsigned int neg_mode) { @@ -1365,6 +1389,7 @@ static const struct dw_xpcs_desc xpcs_desc_list[] = { static const struct phylink_pcs_ops xpcs_phylink_ops = { .pcs_validate = xpcs_validate, + .pcs_pre_config = xpcs_pre_config, .pcs_config = xpcs_config, .pcs_get_state = xpcs_get_state, .pcs_an_restart = xpcs_an_restart, @@ -1460,18 +1485,12 @@ static int xpcs_init_id(struct dw_xpcs *xpcs) static int xpcs_init_iface(struct dw_xpcs *xpcs, phy_interface_t interface) { - const struct dw_xpcs_compat *compat; - - compat = xpcs_find_compat(xpcs->desc, interface); - if (!compat) - return -EINVAL; - - if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID) { + if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID) xpcs->pcs.poll = false; - return 0; - } + else + xpcs->need_reset = true; - return xpcs_soft_reset(xpcs, compat); + return 0; } static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev, diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h index b4a4eb6c8866..fd75d0605bb6 100644 --- a/include/linux/pcs/pcs-xpcs.h +++ b/include/linux/pcs/pcs-xpcs.h @@ -61,6 +61,7 @@ struct dw_xpcs { struct clk_bulk_data clks[DW_XPCS_NUM_CLKS]; struct phylink_pcs pcs; phy_interface_t interface; + bool need_reset; }; int xpcs_get_an_mode(struct dw_xpcs *xpcs, phy_interface_t interface);