Message ID | E1tMBRF-006vae-WC@rmk-PC.armlinux.org.uk (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [net-next,1/5] net: phylink: add support for PCS supported_interfaces bitmap | expand |
Hi Russell, On Fri, 13 Dec 2024 19:35:01 +0000 "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk> wrote: > Fill in the new PCS supported_interfaces member with the interfaces > that the Mediatek LynxI supports. > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > --- > drivers/net/pcs/pcs-mtk-lynxi.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/net/pcs/pcs-mtk-lynxi.c b/drivers/net/pcs/pcs-mtk-lynxi.c > index 7de804535229..1377fb78eaa1 100644 > --- a/drivers/net/pcs/pcs-mtk-lynxi.c > +++ b/drivers/net/pcs/pcs-mtk-lynxi.c > @@ -306,6 +306,11 @@ struct phylink_pcs *mtk_pcs_lynxi_create(struct device *dev, > mpcs->pcs.poll = true; > mpcs->interface = PHY_INTERFACE_MODE_NA; > > + __set_bit(PHY_INTERFACE_MODE_SGMII, mpcs->pcs.supported_interfaces); > + __set_bit(PHY_INTERFACE_MODE_QSGMII, mpcs->pcs.supported_interfaces); I'm sorry if I missed something, but I don't find where the QSGMII support comes from based on the current codebase :/ I didn't spot that in the inband_caps commit, sorry :( > + __set_bit(PHY_INTERFACE_MODE_1000BASEX, mpcs->pcs.supported_interfaces); > + __set_bit(PHY_INTERFACE_MODE_2500BASEX, mpcs->pcs.supported_interfaces); Thanks, Maxime
On Tue, Dec 17, 2024 at 02:15:47PM +0100, Maxime Chevallier wrote: > Hi Russell, > > On Fri, 13 Dec 2024 19:35:01 +0000 > "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk> wrote: > > > Fill in the new PCS supported_interfaces member with the interfaces > > that the Mediatek LynxI supports. > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > --- > > drivers/net/pcs/pcs-mtk-lynxi.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/net/pcs/pcs-mtk-lynxi.c b/drivers/net/pcs/pcs-mtk-lynxi.c > > index 7de804535229..1377fb78eaa1 100644 > > --- a/drivers/net/pcs/pcs-mtk-lynxi.c > > +++ b/drivers/net/pcs/pcs-mtk-lynxi.c > > @@ -306,6 +306,11 @@ struct phylink_pcs *mtk_pcs_lynxi_create(struct device *dev, > > mpcs->pcs.poll = true; > > mpcs->interface = PHY_INTERFACE_MODE_NA; > > > > + __set_bit(PHY_INTERFACE_MODE_SGMII, mpcs->pcs.supported_interfaces); > > + __set_bit(PHY_INTERFACE_MODE_QSGMII, mpcs->pcs.supported_interfaces); > > I'm sorry if I missed something, but I don't find where the QSGMII > support comes from based on the current codebase :/ > > I didn't spot that in the inband_caps commit, sorry :( > > > + __set_bit(PHY_INTERFACE_MODE_1000BASEX, mpcs->pcs.supported_interfaces); > > + __set_bit(PHY_INTERFACE_MODE_2500BASEX, mpcs->pcs.supported_interfaces); This list comes from the behaviour of the PCS as it stood before any of these changes - the PCS code itself never validates the interface it's passed, except for the call to phylink_mii_c22_pcs_encode_advertisement() and checking that the return value is non-negative. This is the only place that the interfaces will be restricted - and they will be restricted to the four interfaces I've listed above. I don't have information on the hardware; so I can only go by the behaviour of the existing code when making changes - and I take the approach when adding new stuff of trying to avoid changing the code behaviour, even if the existing code is doing something wrong. I think, therefore, that a patch to remove stuff that isn't actually supported should come after these patches, because that changes the driver behaviour - otherwise the reason why QSGMII isn't included in the patch would have needed to be described in each commit adding extra code dealing with the interface mode. It would've been nice had the driver implemented .pcs_validate() from the start, which would've made it obvious which interface modes were supported!
diff --git a/drivers/net/pcs/pcs-mtk-lynxi.c b/drivers/net/pcs/pcs-mtk-lynxi.c index 7de804535229..1377fb78eaa1 100644 --- a/drivers/net/pcs/pcs-mtk-lynxi.c +++ b/drivers/net/pcs/pcs-mtk-lynxi.c @@ -306,6 +306,11 @@ struct phylink_pcs *mtk_pcs_lynxi_create(struct device *dev, mpcs->pcs.poll = true; mpcs->interface = PHY_INTERFACE_MODE_NA; + __set_bit(PHY_INTERFACE_MODE_SGMII, mpcs->pcs.supported_interfaces); + __set_bit(PHY_INTERFACE_MODE_QSGMII, mpcs->pcs.supported_interfaces); + __set_bit(PHY_INTERFACE_MODE_1000BASEX, mpcs->pcs.supported_interfaces); + __set_bit(PHY_INTERFACE_MODE_2500BASEX, mpcs->pcs.supported_interfaces); + return &mpcs->pcs; } EXPORT_SYMBOL(mtk_pcs_lynxi_create);
Fill in the new PCS supported_interfaces member with the interfaces that the Mediatek LynxI supports. Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- drivers/net/pcs/pcs-mtk-lynxi.c | 5 +++++ 1 file changed, 5 insertions(+)