| Message ID | E1jqHGO-0006QN-Hw@rmk-PC.armlinux.org.uk |
|---|---|
| State | RFC |
| Delegated to: | David Miller |
| Headers | show |
| Series | Phylink PCS updates | expand |
Hi Russell, On Tue, 30 Jun 2020 at 17:29, Russell King <rmk+kernel@armlinux.org.uk> wrote: > > Add a way for MAC PCS to have private data while keeping independence > from struct phylink_config, which is used for the MAC itself. We need > this independence as we will have stand-alone code for PCS that is > independent of the MAC. Introduce struct phylink_pcs, which is > designed to be embedded in a driver private data structure. > > This structure does not include a mdio_device as there are PCS > implementations such as the Marvell DSA and network drivers where this > is not necessary. > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> > --- > drivers/net/phy/phylink.c | 25 ++++++++++++++++------ > include/linux/phylink.h | 45 ++++++++++++++++++++++++++------------- > 2 files changed, 48 insertions(+), 22 deletions(-) > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > index a31a00fb4974..fbc8591b474b 100644 > --- a/drivers/net/phy/phylink.c > +++ b/drivers/net/phy/phylink.c > @@ -43,6 +43,7 @@ struct phylink { > const struct phylink_mac_ops *mac_ops; > const struct phylink_pcs_ops *pcs_ops; > struct phylink_config *config; > + struct phylink_pcs *pcs; > struct device *dev; > unsigned int old_link_state:1; > > @@ -427,7 +428,7 @@ static void phylink_mac_pcs_an_restart(struct phylink *pl) > phy_interface_mode_is_8023z(pl->link_config.interface) && > phylink_autoneg_inband(pl->cur_link_an_mode)) { > if (pl->pcs_ops) > - pl->pcs_ops->pcs_an_restart(pl->config); > + pl->pcs_ops->pcs_an_restart(pl->pcs); > else > pl->mac_ops->mac_an_restart(pl->config); > } > @@ -453,7 +454,7 @@ static void phylink_change_interface(struct phylink *pl, bool restart, > phylink_mac_config(pl, state); > > if (pl->pcs_ops) { > - err = pl->pcs_ops->pcs_config(pl->config, pl->cur_link_an_mode, > + err = pl->pcs_ops->pcs_config(pl->pcs, pl->cur_link_an_mode, > state->interface, > state->advertising, > !!(pl->link_config.pause & > @@ -533,7 +534,7 @@ static void phylink_mac_pcs_get_state(struct phylink *pl, > state->link = 1; > > if (pl->pcs_ops) > - pl->pcs_ops->pcs_get_state(pl->config, state); > + pl->pcs_ops->pcs_get_state(pl->pcs, state); > else > pl->mac_ops->mac_pcs_get_state(pl->config, state); > } > @@ -604,7 +605,7 @@ static void phylink_link_up(struct phylink *pl, > pl->cur_interface = link_state.interface; > > if (pl->pcs_ops && pl->pcs_ops->pcs_link_up) > - pl->pcs_ops->pcs_link_up(pl->config, pl->cur_link_an_mode, > + pl->pcs_ops->pcs_link_up(pl->pcs, pl->cur_link_an_mode, > pl->cur_interface, > link_state.speed, link_state.duplex); > > @@ -863,11 +864,19 @@ struct phylink *phylink_create(struct phylink_config *config, > } > EXPORT_SYMBOL_GPL(phylink_create); > > -void phylink_add_pcs(struct phylink *pl, const struct phylink_pcs_ops *ops) > +/** > + * phylink_set_pcs() - set the current PCS for phylink to use > + * @pl: a pointer to a &struct phylink returned from phylink_create() > + * @pcs: a pointer to the &struct phylink_pcs > + * > + * Bind the MAC PCS to phylink. > + */ > +void phylink_set_pcs(struct phylink *pl, struct phylink_pcs *pcs) > { > - pl->pcs_ops = ops; > + pl->pcs = pcs; > + pl->pcs_ops = pcs->ops; > } > -EXPORT_SYMBOL_GPL(phylink_add_pcs); > +EXPORT_SYMBOL_GPL(phylink_set_pcs); > > /** > * phylink_destroy() - cleanup and destroy the phylink instance > @@ -1212,6 +1221,8 @@ void phylink_start(struct phylink *pl) > break; > case MLO_AN_INBAND: > poll |= pl->config->pcs_poll; > + if (pl->pcs) > + poll |= pl->pcs->poll; Do we see a need for yet another way to request phylink to poll the PCS for link status? > break; > } > if (poll) > diff --git a/include/linux/phylink.h b/include/linux/phylink.h > index 2f1315f32113..057f78263a46 100644 > --- a/include/linux/phylink.h > +++ b/include/linux/phylink.h > @@ -321,6 +321,21 @@ void mac_link_up(struct phylink_config *config, struct phy_device *phy, > int speed, int duplex, bool tx_pause, bool rx_pause); > #endif > > +struct phylink_pcs_ops; > + > +/** > + * struct phylink_pcs - PHYLINK PCS instance > + * @ops: a pointer to the &struct phylink_pcs_ops structure > + * @poll: poll the PCS for link changes > + * > + * This structure is designed to be embedded within the PCS private data, > + * and will be passed between phylink and the PCS. > + */ > +struct phylink_pcs { > + const struct phylink_pcs_ops *ops; > + bool poll; > +}; > + > /** > * struct phylink_pcs_ops - MAC PCS operations structure. > * @pcs_get_state: read the current MAC PCS link state from the hardware. > @@ -330,21 +345,21 @@ void mac_link_up(struct phylink_config *config, struct phy_device *phy, > * (where necessary). > */ > struct phylink_pcs_ops { > - void (*pcs_get_state)(struct phylink_config *config, > + void (*pcs_get_state)(struct phylink_pcs *pcs, > struct phylink_link_state *state); > - int (*pcs_config)(struct phylink_config *config, unsigned int mode, > + int (*pcs_config)(struct phylink_pcs *pcs, unsigned int mode, > phy_interface_t interface, > const unsigned long *advertising, > bool permit_pause_to_mac); > - void (*pcs_an_restart)(struct phylink_config *config); > - void (*pcs_link_up)(struct phylink_config *config, unsigned int mode, > + void (*pcs_an_restart)(struct phylink_pcs *pcs); > + void (*pcs_link_up)(struct phylink_pcs *pcs, unsigned int mode, > phy_interface_t interface, int speed, int duplex); > }; > > #if 0 /* For kernel-doc purposes only. */ > /** > * pcs_get_state() - Read the current inband link state from the hardware > - * @config: a pointer to a &struct phylink_config. > + * @pcs: a pointer to a &struct phylink_pcs. > * @state: a pointer to a &struct phylink_link_state. > * > * Read the current inband link state from the MAC PCS, reporting the > @@ -357,12 +372,12 @@ struct phylink_pcs_ops { > * When present, this overrides mac_pcs_get_state() in &struct > * phylink_mac_ops. > */ > -void pcs_get_state(struct phylink_config *config, > +void pcs_get_state(struct phylink_pcs *pcs, > struct phylink_link_state *state); > > /** > * pcs_config() - Configure the PCS mode and advertisement > - * @config: a pointer to a &struct phylink_config. > + * @pcs: a pointer to a &struct phylink_pcs. > * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND. > * @interface: interface mode to be used > * @advertising: adertisement ethtool link mode mask > @@ -382,21 +397,21 @@ void pcs_get_state(struct phylink_config *config, > * > * For most 10GBASE-R, there is no advertisement. > */ > -int (*pcs_config)(struct phylink_config *config, unsigned int mode, > - phy_interface_t interface, const unsigned long *advertising); > +int pcs_config(struct phylink_pcs *pcs, unsigned int mode, > + phy_interface_t interface, const unsigned long *advertising); > > /** > * pcs_an_restart() - restart 802.3z BaseX autonegotiation > - * @config: a pointer to a &struct phylink_config. > + * @pcs: a pointer to a &struct phylink_pcs. > * > * When PCS ops are present, this overrides mac_an_restart() in &struct > * phylink_mac_ops. > */ > -void (*pcs_an_restart)(struct phylink_config *config); > +void pcs_an_restart(struct phylink_pcs *pcs); > > /** > * pcs_link_up() - program the PCS for the resolved link configuration > - * @config: a pointer to a &struct phylink_config. > + * @pcs: a pointer to a &struct phylink_pcs. > * @mode: link autonegotiation mode > * @interface: link &typedef phy_interface_t mode > * @speed: link speed > @@ -407,14 +422,14 @@ void (*pcs_an_restart)(struct phylink_config *config); > * mode without in-band AN needs to be manually configured for the link > * and duplex setting. Otherwise, this should be a no-op. > */ > -void (*pcs_link_up)(struct phylink_config *config, unsigned int mode, > - phy_interface_t interface, int speed, int duplex); > +void pcs_link_up(struct phylink_pcs *pcs, unsigned int mode, > + phy_interface_t interface, int speed, int duplex); > #endif > > struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *, > phy_interface_t iface, > const struct phylink_mac_ops *mac_ops); > -void phylink_add_pcs(struct phylink *, const struct phylink_pcs_ops *ops); > +void phylink_set_pcs(struct phylink *, struct phylink_pcs *pcs); > void phylink_destroy(struct phylink *); > > int phylink_connect_phy(struct phylink *, struct phy_device *); > -- > 2.20.1 > Thank you, -Vladimir
On Wed, Jul 01, 2020 at 01:47:27PM +0300, Vladimir Oltean wrote: > Hi Russell, > > On Tue, 30 Jun 2020 at 17:29, Russell King <rmk+kernel@armlinux.org.uk> wrote: > > > > Add a way for MAC PCS to have private data while keeping independence > > from struct phylink_config, which is used for the MAC itself. We need > > this independence as we will have stand-alone code for PCS that is > > independent of the MAC. Introduce struct phylink_pcs, which is > > designed to be embedded in a driver private data structure. > > > > This structure does not include a mdio_device as there are PCS > > implementations such as the Marvell DSA and network drivers where this > > is not necessary. > > > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> > > --- > > drivers/net/phy/phylink.c | 25 ++++++++++++++++------ > > include/linux/phylink.h | 45 ++++++++++++++++++++++++++------------- > > 2 files changed, 48 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > > index a31a00fb4974..fbc8591b474b 100644 > > --- a/drivers/net/phy/phylink.c > > +++ b/drivers/net/phy/phylink.c > > @@ -43,6 +43,7 @@ struct phylink { > > const struct phylink_mac_ops *mac_ops; > > const struct phylink_pcs_ops *pcs_ops; > > struct phylink_config *config; > > + struct phylink_pcs *pcs; > > struct device *dev; > > unsigned int old_link_state:1; > > > > @@ -427,7 +428,7 @@ static void phylink_mac_pcs_an_restart(struct phylink *pl) > > phy_interface_mode_is_8023z(pl->link_config.interface) && > > phylink_autoneg_inband(pl->cur_link_an_mode)) { > > if (pl->pcs_ops) > > - pl->pcs_ops->pcs_an_restart(pl->config); > > + pl->pcs_ops->pcs_an_restart(pl->pcs); > > else > > pl->mac_ops->mac_an_restart(pl->config); > > } > > @@ -453,7 +454,7 @@ static void phylink_change_interface(struct phylink *pl, bool restart, > > phylink_mac_config(pl, state); > > > > if (pl->pcs_ops) { > > - err = pl->pcs_ops->pcs_config(pl->config, pl->cur_link_an_mode, > > + err = pl->pcs_ops->pcs_config(pl->pcs, pl->cur_link_an_mode, > > state->interface, > > state->advertising, > > !!(pl->link_config.pause & > > @@ -533,7 +534,7 @@ static void phylink_mac_pcs_get_state(struct phylink *pl, > > state->link = 1; > > > > if (pl->pcs_ops) > > - pl->pcs_ops->pcs_get_state(pl->config, state); > > + pl->pcs_ops->pcs_get_state(pl->pcs, state); > > else > > pl->mac_ops->mac_pcs_get_state(pl->config, state); > > } > > @@ -604,7 +605,7 @@ static void phylink_link_up(struct phylink *pl, > > pl->cur_interface = link_state.interface; > > > > if (pl->pcs_ops && pl->pcs_ops->pcs_link_up) > > - pl->pcs_ops->pcs_link_up(pl->config, pl->cur_link_an_mode, > > + pl->pcs_ops->pcs_link_up(pl->pcs, pl->cur_link_an_mode, > > pl->cur_interface, > > link_state.speed, link_state.duplex); > > > > @@ -863,11 +864,19 @@ struct phylink *phylink_create(struct phylink_config *config, > > } > > EXPORT_SYMBOL_GPL(phylink_create); > > > > -void phylink_add_pcs(struct phylink *pl, const struct phylink_pcs_ops *ops) > > +/** > > + * phylink_set_pcs() - set the current PCS for phylink to use > > + * @pl: a pointer to a &struct phylink returned from phylink_create() > > + * @pcs: a pointer to the &struct phylink_pcs > > + * > > + * Bind the MAC PCS to phylink. > > + */ > > +void phylink_set_pcs(struct phylink *pl, struct phylink_pcs *pcs) > > { > > - pl->pcs_ops = ops; > > + pl->pcs = pcs; > > + pl->pcs_ops = pcs->ops; > > } > > -EXPORT_SYMBOL_GPL(phylink_add_pcs); > > +EXPORT_SYMBOL_GPL(phylink_set_pcs); > > > > /** > > * phylink_destroy() - cleanup and destroy the phylink instance > > @@ -1212,6 +1221,8 @@ void phylink_start(struct phylink *pl) > > break; > > case MLO_AN_INBAND: > > poll |= pl->config->pcs_poll; > > + if (pl->pcs) > > + poll |= pl->pcs->poll; > > Do we see a need for yet another way to request phylink to poll the > PCS for link status? Please consider what the model looks like if we have the PCS almost self contained except for this property, which is in the MAC side. What if some PCS need to be polled but others do not. Why should the MAC need to have that knowledge - is it not a property of the PCS itself? The reason we stuffed it into phylink_config is that at the time, that was all that existed. That doesn't mean that when we change the model that we should be tied by that decision. So, for example, does the Lynx PCS IP support any kind of notification of link changes to its integrated system? If it does not, then having the Lynx PCS mark _itself_ as needing polling is entirely sane, rather than burying that information in the MAC driver.
On Wed, 1 Jul 2020 at 14:16, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: > > On Wed, Jul 01, 2020 at 01:47:27PM +0300, Vladimir Oltean wrote: > > Hi Russell, > > > > On Tue, 30 Jun 2020 at 17:29, Russell King <rmk+kernel@armlinux.org.uk> wrote: > > > > > > Add a way for MAC PCS to have private data while keeping independence > > > from struct phylink_config, which is used for the MAC itself. We need > > > this independence as we will have stand-alone code for PCS that is > > > independent of the MAC. Introduce struct phylink_pcs, which is > > > designed to be embedded in a driver private data structure. > > > > > > This structure does not include a mdio_device as there are PCS > > > implementations such as the Marvell DSA and network drivers where this > > > is not necessary. > > > > > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> > > > --- > > > drivers/net/phy/phylink.c | 25 ++++++++++++++++------ > > > include/linux/phylink.h | 45 ++++++++++++++++++++++++++------------- > > > 2 files changed, 48 insertions(+), 22 deletions(-) > > > > > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > > > index a31a00fb4974..fbc8591b474b 100644 > > > --- a/drivers/net/phy/phylink.c > > > +++ b/drivers/net/phy/phylink.c > > > @@ -43,6 +43,7 @@ struct phylink { > > > const struct phylink_mac_ops *mac_ops; > > > const struct phylink_pcs_ops *pcs_ops; > > > struct phylink_config *config; > > > + struct phylink_pcs *pcs; > > > struct device *dev; > > > unsigned int old_link_state:1; > > > > > > @@ -427,7 +428,7 @@ static void phylink_mac_pcs_an_restart(struct phylink *pl) > > > phy_interface_mode_is_8023z(pl->link_config.interface) && > > > phylink_autoneg_inband(pl->cur_link_an_mode)) { > > > if (pl->pcs_ops) > > > - pl->pcs_ops->pcs_an_restart(pl->config); > > > + pl->pcs_ops->pcs_an_restart(pl->pcs); > > > else > > > pl->mac_ops->mac_an_restart(pl->config); > > > } > > > @@ -453,7 +454,7 @@ static void phylink_change_interface(struct phylink *pl, bool restart, > > > phylink_mac_config(pl, state); > > > > > > if (pl->pcs_ops) { > > > - err = pl->pcs_ops->pcs_config(pl->config, pl->cur_link_an_mode, > > > + err = pl->pcs_ops->pcs_config(pl->pcs, pl->cur_link_an_mode, > > > state->interface, > > > state->advertising, > > > !!(pl->link_config.pause & > > > @@ -533,7 +534,7 @@ static void phylink_mac_pcs_get_state(struct phylink *pl, > > > state->link = 1; > > > > > > if (pl->pcs_ops) > > > - pl->pcs_ops->pcs_get_state(pl->config, state); > > > + pl->pcs_ops->pcs_get_state(pl->pcs, state); > > > else > > > pl->mac_ops->mac_pcs_get_state(pl->config, state); > > > } > > > @@ -604,7 +605,7 @@ static void phylink_link_up(struct phylink *pl, > > > pl->cur_interface = link_state.interface; > > > > > > if (pl->pcs_ops && pl->pcs_ops->pcs_link_up) > > > - pl->pcs_ops->pcs_link_up(pl->config, pl->cur_link_an_mode, > > > + pl->pcs_ops->pcs_link_up(pl->pcs, pl->cur_link_an_mode, > > > pl->cur_interface, > > > link_state.speed, link_state.duplex); > > > > > > @@ -863,11 +864,19 @@ struct phylink *phylink_create(struct phylink_config *config, > > > } > > > EXPORT_SYMBOL_GPL(phylink_create); > > > > > > -void phylink_add_pcs(struct phylink *pl, const struct phylink_pcs_ops *ops) > > > +/** > > > + * phylink_set_pcs() - set the current PCS for phylink to use > > > + * @pl: a pointer to a &struct phylink returned from phylink_create() > > > + * @pcs: a pointer to the &struct phylink_pcs > > > + * > > > + * Bind the MAC PCS to phylink. > > > + */ > > > +void phylink_set_pcs(struct phylink *pl, struct phylink_pcs *pcs) > > > { > > > - pl->pcs_ops = ops; > > > + pl->pcs = pcs; > > > + pl->pcs_ops = pcs->ops; > > > } > > > -EXPORT_SYMBOL_GPL(phylink_add_pcs); > > > +EXPORT_SYMBOL_GPL(phylink_set_pcs); > > > > > > /** > > > * phylink_destroy() - cleanup and destroy the phylink instance > > > @@ -1212,6 +1221,8 @@ void phylink_start(struct phylink *pl) > > > break; > > > case MLO_AN_INBAND: > > > poll |= pl->config->pcs_poll; > > > + if (pl->pcs) > > > + poll |= pl->pcs->poll; > > > > Do we see a need for yet another way to request phylink to poll the > > PCS for link status? > > Please consider what the model looks like if we have the PCS almost > self contained except for this property, which is in the MAC side. > What if some PCS need to be polled but others do not. Why should the > MAC need to have that knowledge - is it not a property of the PCS > itself? > > The reason we stuffed it into phylink_config is that at the time, that > was all that existed. That doesn't mean that when we change the model > that we should be tied by that decision. > > So, for example, does the Lynx PCS IP support any kind of notification > of link changes to its integrated system? If it does not, then having > the Lynx PCS mark _itself_ as needing polling is entirely sane, rather > than burying that information in the MAC driver. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! Well, there is a MAC register called IEVENT[PCS]. For some Lynx integrations, such as Felix, I know it doesn't fire an IRQ. Now, if it doesn't fire on _all_ SoCs which integrate Lynx, that I don't know. But the interrupt is going to be highly system-specific either way (on some MACs it's a regular IRQ line, on others it's an MSI). So, in the general case, I think this is system-specific and not a property of the PCS itself.
On 6/30/20 5:29 PM, Russell King wrote: > Add a way for MAC PCS to have private data while keeping independence > from struct phylink_config, which is used for the MAC itself. We need > this independence as we will have stand-alone code for PCS that is > independent of the MAC. Introduce struct phylink_pcs, which is > designed to be embedded in a driver private data structure. > > This structure does not include a mdio_device as there are PCS > implementations such as the Marvell DSA and network drivers where this > is not necessary. > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> Reviewed-by: Ioana Ciornei <ioana.ciornei@nxp.com> I integrated and used the phylink_pcs structure into the Lynx PCS just to see how everything fits. Pasting below the main parts so that we can catch early any possible different opinions on how to integrate this: The basic Lynx structure looks like below and the main idea is just to encapsulate the phylink_pcs structure and the mdio device (which in some other cases might not be needed). struct lynx_pcs { struct phylink_pcs pcs; struct mdio_device *mdio; phy_interface_t interface; }; The lynx_pcs structure is requested by the MAC driver with: struct lynx_pcs *lynx_pcs_create(struct mdio_device *mdio) { (...) lynx_pcs->mdio = mdio; lynx_pcs->pcs.ops = &lynx_pcs_phylink_ops; lynx_pcs->pcs.poll = true; return lynx_pcs; } And then passed to phylink with something like: phylink_set_pcs(pl, &lynx_pcs->pcs); For DSA it's a bit less straightforward because the .setup() callback from the dsa_switch_ops is run before any phylink structure has been created internally. For this, a new DSA helper can be created that just stores the phylink_pcs structure per port: void dsa_port_phylink_set_pcs(struct dsa_switch *ds, int port, struct phylink_pcs *pcs) { struct dsa_port *dp = dsa_to_port(ds, port); dp->pcs = pcs; but I do } and at the appropriate time, from dsa_slave_setup, it can really install the phylink_pcs with phylink_set_pcs. The other option would be to add a new dsa_switch ops that requests the phylink_pcs for a specific port - something like phylink_get_pcs. Ioana > --- > drivers/net/phy/phylink.c | 25 ++++++++++++++++------ > include/linux/phylink.h | 45 ++++++++++++++++++++++++++------------- > 2 files changed, 48 insertions(+), 22 deletions(-) > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > index a31a00fb4974..fbc8591b474b 100644 > --- a/drivers/net/phy/phylink.c > +++ b/drivers/net/phy/phylink.c > @@ -43,6 +43,7 @@ struct phylink { > const struct phylink_mac_ops *mac_ops; > const struct phylink_pcs_ops *pcs_ops; > struct phylink_config *config; > + struct phylink_pcs *pcs; > struct device *dev; > unsigned int old_link_state:1; > > @@ -427,7 +428,7 @@ static void phylink_mac_pcs_an_restart(struct phylink *pl) > phy_interface_mode_is_8023z(pl->link_config.interface) && > phylink_autoneg_inband(pl->cur_link_an_mode)) { > if (pl->pcs_ops) > - pl->pcs_ops->pcs_an_restart(pl->config); > + pl->pcs_ops->pcs_an_restart(pl->pcs); > else > pl->mac_ops->mac_an_restart(pl->config); > } > @@ -453,7 +454,7 @@ static void phylink_change_interface(struct phylink *pl, bool restart, > phylink_mac_config(pl, state); > > if (pl->pcs_ops) { > - err = pl->pcs_ops->pcs_config(pl->config, pl->cur_link_an_mode, > + err = pl->pcs_ops->pcs_config(pl->pcs, pl->cur_link_an_mode, > state->interface, > state->advertising, > !!(pl->link_config.pause & > @@ -533,7 +534,7 @@ static void phylink_mac_pcs_get_state(struct phylink *pl, > state->link = 1; > > if (pl->pcs_ops) > - pl->pcs_ops->pcs_get_state(pl->config, state); > + pl->pcs_ops->pcs_get_state(pl->pcs, state); > else > pl->mac_ops->mac_pcs_get_state(pl->config, state); > } > @@ -604,7 +605,7 @@ static void phylink_link_up(struct phylink *pl, > pl->cur_interface = link_state.interface; > > if (pl->pcs_ops && pl->pcs_ops->pcs_link_up) > - pl->pcs_ops->pcs_link_up(pl->config, pl->cur_link_an_mode, > + pl->pcs_ops->pcs_link_up(pl->pcs, pl->cur_link_an_mode, > pl->cur_interface, > link_state.speed, link_state.duplex); > > @@ -863,11 +864,19 @@ struct phylink *phylink_create(struct phylink_config *config, > } > EXPORT_SYMBOL_GPL(phylink_create); > > -void phylink_add_pcs(struct phylink *pl, const struct phylink_pcs_ops *ops) > +/** > + * phylink_set_pcs() - set the current PCS for phylink to use > + * @pl: a pointer to a &struct phylink returned from phylink_create() > + * @pcs: a pointer to the &struct phylink_pcs > + * > + * Bind the MAC PCS to phylink. > + */ > +void phylink_set_pcs(struct phylink *pl, struct phylink_pcs *pcs) > { > - pl->pcs_ops = ops; > + pl->pcs = pcs; > + pl->pcs_ops = pcs->ops; > } > -EXPORT_SYMBOL_GPL(phylink_add_pcs); > +EXPORT_SYMBOL_GPL(phylink_set_pcs); > > /** > * phylink_destroy() - cleanup and destroy the phylink instance > @@ -1212,6 +1221,8 @@ void phylink_start(struct phylink *pl) > break; > case MLO_AN_INBAND: > poll |= pl->config->pcs_poll; > + if (pl->pcs) > + poll |= pl->pcs->poll; > break; > } > if (poll) > diff --git a/include/linux/phylink.h b/include/linux/phylink.h > index 2f1315f32113..057f78263a46 100644 > --- a/include/linux/phylink.h > +++ b/include/linux/phylink.h > @@ -321,6 +321,21 @@ void mac_link_up(struct phylink_config *config, struct phy_device *phy, > int speed, int duplex, bool tx_pause, bool rx_pause); > #endif > > +struct phylink_pcs_ops; > + > +/** > + * struct phylink_pcs - PHYLINK PCS instance > + * @ops: a pointer to the &struct phylink_pcs_ops structure > + * @poll: poll the PCS for link changes > + * > + * This structure is designed to be embedded within the PCS private data, > + * and will be passed between phylink and the PCS. > + */ > +struct phylink_pcs { > + const struct phylink_pcs_ops *ops; > + bool poll; > +}; > + > /** > * struct phylink_pcs_ops - MAC PCS operations structure. > * @pcs_get_state: read the current MAC PCS link state from the hardware. > @@ -330,21 +345,21 @@ void mac_link_up(struct phylink_config *config, struct phy_device *phy, > * (where necessary). > */ > struct phylink_pcs_ops { > - void (*pcs_get_state)(struct phylink_config *config, > + void (*pcs_get_state)(struct phylink_pcs *pcs, > struct phylink_link_state *state); > - int (*pcs_config)(struct phylink_config *config, unsigned int mode, > + int (*pcs_config)(struct phylink_pcs *pcs, unsigned int mode, > phy_interface_t interface, > const unsigned long *advertising, > bool permit_pause_to_mac); > - void (*pcs_an_restart)(struct phylink_config *config); > - void (*pcs_link_up)(struct phylink_config *config, unsigned int mode, > + void (*pcs_an_restart)(struct phylink_pcs *pcs); > + void (*pcs_link_up)(struct phylink_pcs *pcs, unsigned int mode, > phy_interface_t interface, int speed, int duplex); > }; > > #if 0 /* For kernel-doc purposes only. */ > /** > * pcs_get_state() - Read the current inband link state from the hardware > - * @config: a pointer to a &struct phylink_config. > + * @pcs: a pointer to a &struct phylink_pcs. > * @state: a pointer to a &struct phylink_link_state. > * > * Read the current inband link state from the MAC PCS, reporting the > @@ -357,12 +372,12 @@ struct phylink_pcs_ops { > * When present, this overrides mac_pcs_get_state() in &struct > * phylink_mac_ops. > */ > -void pcs_get_state(struct phylink_config *config, > +void pcs_get_state(struct phylink_pcs *pcs, > struct phylink_link_state *state); > > /** > * pcs_config() - Configure the PCS mode and advertisement > - * @config: a pointer to a &struct phylink_config. > + * @pcs: a pointer to a &struct phylink_pcs. > * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND. > * @interface: interface mode to be used > * @advertising: adertisement ethtool link mode mask > @@ -382,21 +397,21 @@ void pcs_get_state(struct phylink_config *config, > * > * For most 10GBASE-R, there is no advertisement. > */ > -int (*pcs_config)(struct phylink_config *config, unsigned int mode, > - phy_interface_t interface, const unsigned long *advertising); > +int pcs_config(struct phylink_pcs *pcs, unsigned int mode, > + phy_interface_t interface, const unsigned long *advertising); > > /** > * pcs_an_restart() - restart 802.3z BaseX autonegotiation > - * @config: a pointer to a &struct phylink_config. > + * @pcs: a pointer to a &struct phylink_pcs. > * > * When PCS ops are present, this overrides mac_an_restart() in &struct > * phylink_mac_ops. > */ > -void (*pcs_an_restart)(struct phylink_config *config); > +void pcs_an_restart(struct phylink_pcs *pcs); > > /** > * pcs_link_up() - program the PCS for the resolved link configuration > - * @config: a pointer to a &struct phylink_config. > + * @pcs: a pointer to a &struct phylink_pcs. > * @mode: link autonegotiation mode > * @interface: link &typedef phy_interface_t mode > * @speed: link speed > @@ -407,14 +422,14 @@ void (*pcs_an_restart)(struct phylink_config *config); > * mode without in-band AN needs to be manually configured for the link > * and duplex setting. Otherwise, this should be a no-op. > */ > -void (*pcs_link_up)(struct phylink_config *config, unsigned int mode, > - phy_interface_t interface, int speed, int duplex); > +void pcs_link_up(struct phylink_pcs *pcs, unsigned int mode, > + phy_interface_t interface, int speed, int duplex); > #endif > > struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *, > phy_interface_t iface, > const struct phylink_mac_ops *mac_ops); > -void phylink_add_pcs(struct phylink *, const struct phylink_pcs_ops *ops); > +void phylink_set_pcs(struct phylink *, struct phylink_pcs *pcs); > void phylink_destroy(struct phylink *); > > int phylink_connect_phy(struct phylink *, struct phy_device *); >
On Mon, Jul 20, 2020 at 03:50:57PM +0000, Ioana Ciornei wrote: > On 6/30/20 5:29 PM, Russell King wrote: > > Add a way for MAC PCS to have private data while keeping independence > > from struct phylink_config, which is used for the MAC itself. We need > > this independence as we will have stand-alone code for PCS that is > > independent of the MAC. Introduce struct phylink_pcs, which is > > designed to be embedded in a driver private data structure. > > > > This structure does not include a mdio_device as there are PCS > > implementations such as the Marvell DSA and network drivers where this > > is not necessary. > > > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> > > Reviewed-by: Ioana Ciornei <ioana.ciornei@nxp.com> > > I integrated and used the phylink_pcs structure into the Lynx PCS just > to see how everything fits. Pasting below the main parts so that we can > catch early any possible different opinions on how to integrate this: > > The basic Lynx structure looks like below and the main idea is just to > encapsulate the phylink_pcs structure and the mdio device (which in some > other cases might not be needed). > > struct lynx_pcs { > struct phylink_pcs pcs; > struct mdio_device *mdio; > phy_interface_t interface; > }; > > The lynx_pcs structure is requested by the MAC driver with: > > struct lynx_pcs *lynx_pcs_create(struct mdio_device *mdio) > { > (...) > lynx_pcs->mdio = mdio; > lynx_pcs->pcs.ops = &lynx_pcs_phylink_ops; > lynx_pcs->pcs.poll = true; > > return lynx_pcs; > } > > And then passed to phylink with something like: > > phylink_set_pcs(pl, &lynx_pcs->pcs); > > > For DSA it's a bit less straightforward because the .setup() callback > from the dsa_switch_ops is run before any phylink structure has been > created internally. For this, a new DSA helper can be created that just > stores the phylink_pcs structure per port: > > void dsa_port_phylink_set_pcs(struct dsa_switch *ds, int port, > struct phylink_pcs *pcs) > { > struct dsa_port *dp = dsa_to_port(ds, port); > > dp->pcs = pcs; but I do > } > > and at the appropriate time, from dsa_slave_setup, it can really install > the phylink_pcs with phylink_set_pcs. > The other option would be to add a new dsa_switch ops that requests the > phylink_pcs for a specific port - something like phylink_get_pcs. It is entirely possible to set the PCS in the mac_prepare() or mac_config() callbacks - but DSA doesn't yet support the mac_prepare() callback (because it needs to be propagated through the DSA way of doing things.) An example of this can be found at: http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=mcbin&id=593d56ef8c7f7d395626752f6810210471a5f5c1 where we choose between the XLGMAC and GMAC pcs_ops structures depending on the interface mode we are configuring for. Note that this is one of the devices that the PCS does not appear as a distinctly separate entity in the register set, at least in the GMAC side of things.
On 7/20/20 7:26 PM, Russell King - ARM Linux admin wrote: > On Mon, Jul 20, 2020 at 03:50:57PM +0000, Ioana Ciornei wrote: >> On 6/30/20 5:29 PM, Russell King wrote: >>> Add a way for MAC PCS to have private data while keeping independence >>> from struct phylink_config, which is used for the MAC itself. We need >>> this independence as we will have stand-alone code for PCS that is >>> independent of the MAC. Introduce struct phylink_pcs, which is >>> designed to be embedded in a driver private data structure. >>> >>> This structure does not include a mdio_device as there are PCS >>> implementations such as the Marvell DSA and network drivers where this >>> is not necessary. >>> >>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> >> >> Reviewed-by: Ioana Ciornei <ioana.ciornei@nxp.com> >> >> I integrated and used the phylink_pcs structure into the Lynx PCS just >> to see how everything fits. Pasting below the main parts so that we can >> catch early any possible different opinions on how to integrate this: >> >> The basic Lynx structure looks like below and the main idea is just to >> encapsulate the phylink_pcs structure and the mdio device (which in some >> other cases might not be needed). >> >> struct lynx_pcs { >> struct phylink_pcs pcs; >> struct mdio_device *mdio; >> phy_interface_t interface; >> }; >> >> The lynx_pcs structure is requested by the MAC driver with: >> >> struct lynx_pcs *lynx_pcs_create(struct mdio_device *mdio) >> { >> (...) >> lynx_pcs->mdio = mdio; >> lynx_pcs->pcs.ops = &lynx_pcs_phylink_ops; >> lynx_pcs->pcs.poll = true; >> >> return lynx_pcs; >> } >> >> And then passed to phylink with something like: >> >> phylink_set_pcs(pl, &lynx_pcs->pcs); >> >> >> For DSA it's a bit less straightforward because the .setup() callback >> from the dsa_switch_ops is run before any phylink structure has been >> created internally. For this, a new DSA helper can be created that just >> stores the phylink_pcs structure per port: >> >> void dsa_port_phylink_set_pcs(struct dsa_switch *ds, int port, >> struct phylink_pcs *pcs) >> { >> struct dsa_port *dp = dsa_to_port(ds, port); >> >> dp->pcs = pcs; but I do >> } >> >> and at the appropriate time, from dsa_slave_setup, it can really install >> the phylink_pcs with phylink_set_pcs. >> The other option would be to add a new dsa_switch ops that requests the >> phylink_pcs for a specific port - something like phylink_get_pcs. > > It is entirely possible to set the PCS in the mac_prepare() or > mac_config() callbacks - but DSA doesn't yet support the mac_prepare() > callback (because it needs to be propagated through the DSA way of > doing things.) > > An example of this can be found at: > > http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=mcbin&id=593d56ef8c7f7d395626752f6810210471a5f5c1 > > where we choose between the XLGMAC and GMAC pcs_ops structures > depending on the interface mode we are configuring for. Note that > this is one of the devices that the PCS does not appear as a > distinctly separate entity in the register set, at least in the > GMAC side of things. > Thanks for the info, I didn't get that this is possible by reading the previous patch. Maybe this would be useful in the documentation of the callback? Back to the DSA, I don't feel like we gain much by setting up the phylink_pcs from another callback: we somehow force the driver to implement a phylink_mac_prepare dsa_switch_ops just so that it sets up the phylink_pcs, which for me at least would not be intuitive. Ioana
On Mon, Jul 20, 2020 at 04:59:08PM +0000, Ioana Ciornei wrote: > On 7/20/20 7:26 PM, Russell King - ARM Linux admin wrote: > > On Mon, Jul 20, 2020 at 03:50:57PM +0000, Ioana Ciornei wrote: > >> On 6/30/20 5:29 PM, Russell King wrote: > >>> Add a way for MAC PCS to have private data while keeping independence > >>> from struct phylink_config, which is used for the MAC itself. We need > >>> this independence as we will have stand-alone code for PCS that is > >>> independent of the MAC. Introduce struct phylink_pcs, which is > >>> designed to be embedded in a driver private data structure. > >>> > >>> This structure does not include a mdio_device as there are PCS > >>> implementations such as the Marvell DSA and network drivers where this > >>> is not necessary. > >>> > >>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> > >> > >> Reviewed-by: Ioana Ciornei <ioana.ciornei@nxp.com> > >> > >> I integrated and used the phylink_pcs structure into the Lynx PCS just > >> to see how everything fits. Pasting below the main parts so that we can > >> catch early any possible different opinions on how to integrate this: > >> > >> The basic Lynx structure looks like below and the main idea is just to > >> encapsulate the phylink_pcs structure and the mdio device (which in some > >> other cases might not be needed). > >> > >> struct lynx_pcs { > >> struct phylink_pcs pcs; > >> struct mdio_device *mdio; > >> phy_interface_t interface; > >> }; > >> > >> The lynx_pcs structure is requested by the MAC driver with: > >> > >> struct lynx_pcs *lynx_pcs_create(struct mdio_device *mdio) > >> { > >> (...) > >> lynx_pcs->mdio = mdio; > >> lynx_pcs->pcs.ops = &lynx_pcs_phylink_ops; > >> lynx_pcs->pcs.poll = true; > >> > >> return lynx_pcs; > >> } > >> > >> And then passed to phylink with something like: > >> > >> phylink_set_pcs(pl, &lynx_pcs->pcs); > >> > >> > >> For DSA it's a bit less straightforward because the .setup() callback > >> from the dsa_switch_ops is run before any phylink structure has been > >> created internally. For this, a new DSA helper can be created that just > >> stores the phylink_pcs structure per port: > >> > >> void dsa_port_phylink_set_pcs(struct dsa_switch *ds, int port, > >> struct phylink_pcs *pcs) > >> { > >> struct dsa_port *dp = dsa_to_port(ds, port); > >> > >> dp->pcs = pcs; but I do > >> } > >> > >> and at the appropriate time, from dsa_slave_setup, it can really install > >> the phylink_pcs with phylink_set_pcs. > >> The other option would be to add a new dsa_switch ops that requests the > >> phylink_pcs for a specific port - something like phylink_get_pcs. > > > > It is entirely possible to set the PCS in the mac_prepare() or > > mac_config() callbacks - but DSA doesn't yet support the mac_prepare() > > callback (because it needs to be propagated through the DSA way of > > doing things.) > > > > An example of this can be found at: > > > > http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=mcbin&id=593d56ef8c7f7d395626752f6810210471a5f5c1 > > > > where we choose between the XLGMAC and GMAC pcs_ops structures > > depending on the interface mode we are configuring for. Note that > > this is one of the devices that the PCS does not appear as a > > distinctly separate entity in the register set, at least in the > > GMAC side of things. > > > > Thanks for the info, I didn't get that this is possible by reading the > previous patch. Maybe this would be useful in the documentation of the > callback? > > Back to the DSA, I don't feel like we gain much by setting up the > phylink_pcs from another callback: we somehow force the driver to > implement a phylink_mac_prepare dsa_switch_ops just so that it sets up > the phylink_pcs, which for me at least would not be intuitive. As I said, you can set it in mac_config() as well, which is the absolute latest point. So, you don't actually need DSA to implement anything extra. I may need to add mac_prepare()/mac_finish() to DSA in any case if I convert Marvell DSA switches to phylink PCS - on the face of it, that looks like the logical thing, but there are a few quirks (like auto-media ports) that make it less trivial. Even so, we already have no way to properly cope with the Marvell auto-media ports today.
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index a31a00fb4974..fbc8591b474b 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -43,6 +43,7 @@ struct phylink { const struct phylink_mac_ops *mac_ops; const struct phylink_pcs_ops *pcs_ops; struct phylink_config *config; + struct phylink_pcs *pcs; struct device *dev; unsigned int old_link_state:1; @@ -427,7 +428,7 @@ static void phylink_mac_pcs_an_restart(struct phylink *pl) phy_interface_mode_is_8023z(pl->link_config.interface) && phylink_autoneg_inband(pl->cur_link_an_mode)) { if (pl->pcs_ops) - pl->pcs_ops->pcs_an_restart(pl->config); + pl->pcs_ops->pcs_an_restart(pl->pcs); else pl->mac_ops->mac_an_restart(pl->config); } @@ -453,7 +454,7 @@ static void phylink_change_interface(struct phylink *pl, bool restart, phylink_mac_config(pl, state); if (pl->pcs_ops) { - err = pl->pcs_ops->pcs_config(pl->config, pl->cur_link_an_mode, + err = pl->pcs_ops->pcs_config(pl->pcs, pl->cur_link_an_mode, state->interface, state->advertising, !!(pl->link_config.pause & @@ -533,7 +534,7 @@ static void phylink_mac_pcs_get_state(struct phylink *pl, state->link = 1; if (pl->pcs_ops) - pl->pcs_ops->pcs_get_state(pl->config, state); + pl->pcs_ops->pcs_get_state(pl->pcs, state); else pl->mac_ops->mac_pcs_get_state(pl->config, state); } @@ -604,7 +605,7 @@ static void phylink_link_up(struct phylink *pl, pl->cur_interface = link_state.interface; if (pl->pcs_ops && pl->pcs_ops->pcs_link_up) - pl->pcs_ops->pcs_link_up(pl->config, pl->cur_link_an_mode, + pl->pcs_ops->pcs_link_up(pl->pcs, pl->cur_link_an_mode, pl->cur_interface, link_state.speed, link_state.duplex); @@ -863,11 +864,19 @@ struct phylink *phylink_create(struct phylink_config *config, } EXPORT_SYMBOL_GPL(phylink_create); -void phylink_add_pcs(struct phylink *pl, const struct phylink_pcs_ops *ops) +/** + * phylink_set_pcs() - set the current PCS for phylink to use + * @pl: a pointer to a &struct phylink returned from phylink_create() + * @pcs: a pointer to the &struct phylink_pcs + * + * Bind the MAC PCS to phylink. + */ +void phylink_set_pcs(struct phylink *pl, struct phylink_pcs *pcs) { - pl->pcs_ops = ops; + pl->pcs = pcs; + pl->pcs_ops = pcs->ops; } -EXPORT_SYMBOL_GPL(phylink_add_pcs); +EXPORT_SYMBOL_GPL(phylink_set_pcs); /** * phylink_destroy() - cleanup and destroy the phylink instance @@ -1212,6 +1221,8 @@ void phylink_start(struct phylink *pl) break; case MLO_AN_INBAND: poll |= pl->config->pcs_poll; + if (pl->pcs) + poll |= pl->pcs->poll; break; } if (poll) diff --git a/include/linux/phylink.h b/include/linux/phylink.h index 2f1315f32113..057f78263a46 100644 --- a/include/linux/phylink.h +++ b/include/linux/phylink.h @@ -321,6 +321,21 @@ void mac_link_up(struct phylink_config *config, struct phy_device *phy, int speed, int duplex, bool tx_pause, bool rx_pause); #endif +struct phylink_pcs_ops; + +/** + * struct phylink_pcs - PHYLINK PCS instance + * @ops: a pointer to the &struct phylink_pcs_ops structure + * @poll: poll the PCS for link changes + * + * This structure is designed to be embedded within the PCS private data, + * and will be passed between phylink and the PCS. + */ +struct phylink_pcs { + const struct phylink_pcs_ops *ops; + bool poll; +}; + /** * struct phylink_pcs_ops - MAC PCS operations structure. * @pcs_get_state: read the current MAC PCS link state from the hardware. @@ -330,21 +345,21 @@ void mac_link_up(struct phylink_config *config, struct phy_device *phy, * (where necessary). */ struct phylink_pcs_ops { - void (*pcs_get_state)(struct phylink_config *config, + void (*pcs_get_state)(struct phylink_pcs *pcs, struct phylink_link_state *state); - int (*pcs_config)(struct phylink_config *config, unsigned int mode, + int (*pcs_config)(struct phylink_pcs *pcs, unsigned int mode, phy_interface_t interface, const unsigned long *advertising, bool permit_pause_to_mac); - void (*pcs_an_restart)(struct phylink_config *config); - void (*pcs_link_up)(struct phylink_config *config, unsigned int mode, + void (*pcs_an_restart)(struct phylink_pcs *pcs); + void (*pcs_link_up)(struct phylink_pcs *pcs, unsigned int mode, phy_interface_t interface, int speed, int duplex); }; #if 0 /* For kernel-doc purposes only. */ /** * pcs_get_state() - Read the current inband link state from the hardware - * @config: a pointer to a &struct phylink_config. + * @pcs: a pointer to a &struct phylink_pcs. * @state: a pointer to a &struct phylink_link_state. * * Read the current inband link state from the MAC PCS, reporting the @@ -357,12 +372,12 @@ struct phylink_pcs_ops { * When present, this overrides mac_pcs_get_state() in &struct * phylink_mac_ops. */ -void pcs_get_state(struct phylink_config *config, +void pcs_get_state(struct phylink_pcs *pcs, struct phylink_link_state *state); /** * pcs_config() - Configure the PCS mode and advertisement - * @config: a pointer to a &struct phylink_config. + * @pcs: a pointer to a &struct phylink_pcs. * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND. * @interface: interface mode to be used * @advertising: adertisement ethtool link mode mask @@ -382,21 +397,21 @@ void pcs_get_state(struct phylink_config *config, * * For most 10GBASE-R, there is no advertisement. */ -int (*pcs_config)(struct phylink_config *config, unsigned int mode, - phy_interface_t interface, const unsigned long *advertising); +int pcs_config(struct phylink_pcs *pcs, unsigned int mode, + phy_interface_t interface, const unsigned long *advertising); /** * pcs_an_restart() - restart 802.3z BaseX autonegotiation - * @config: a pointer to a &struct phylink_config. + * @pcs: a pointer to a &struct phylink_pcs. * * When PCS ops are present, this overrides mac_an_restart() in &struct * phylink_mac_ops. */ -void (*pcs_an_restart)(struct phylink_config *config); +void pcs_an_restart(struct phylink_pcs *pcs); /** * pcs_link_up() - program the PCS for the resolved link configuration - * @config: a pointer to a &struct phylink_config. + * @pcs: a pointer to a &struct phylink_pcs. * @mode: link autonegotiation mode * @interface: link &typedef phy_interface_t mode * @speed: link speed @@ -407,14 +422,14 @@ void (*pcs_an_restart)(struct phylink_config *config); * mode without in-band AN needs to be manually configured for the link * and duplex setting. Otherwise, this should be a no-op. */ -void (*pcs_link_up)(struct phylink_config *config, unsigned int mode, - phy_interface_t interface, int speed, int duplex); +void pcs_link_up(struct phylink_pcs *pcs, unsigned int mode, + phy_interface_t interface, int speed, int duplex); #endif struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *, phy_interface_t iface, const struct phylink_mac_ops *mac_ops); -void phylink_add_pcs(struct phylink *, const struct phylink_pcs_ops *ops); +void phylink_set_pcs(struct phylink *, struct phylink_pcs *pcs); void phylink_destroy(struct phylink *); int phylink_connect_phy(struct phylink *, struct phy_device *);
Add a way for MAC PCS to have private data while keeping independence from struct phylink_config, which is used for the MAC itself. We need this independence as we will have stand-alone code for PCS that is independent of the MAC. Introduce struct phylink_pcs, which is designed to be embedded in a driver private data structure. This structure does not include a mdio_device as there are PCS implementations such as the Marvell DSA and network drivers where this is not necessary. Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- drivers/net/phy/phylink.c | 25 ++++++++++++++++------ include/linux/phylink.h | 45 ++++++++++++++++++++++++++------------- 2 files changed, 48 insertions(+), 22 deletions(-)