diff mbox series

[net-next,3/5] net: pcs: mtk-lynxi: fill in PCS supported_interfaces

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

Checks

Context Check Description
netdev/series_format warning Series does not have a cover letter
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 7 maintainers not CCed: daniel@makrotopia.org matthias.bgg@gmail.com hkallweit1@gmail.com andrew@lunn.ch linux-mediatek@lists.infradead.org angelogioacchino.delregno@collabora.com lynxis@fe80.eu
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 11 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-12-15--09-00 (tests: 795)

Commit Message

Russell King (Oracle) Dec. 13, 2024, 7:35 p.m. UTC
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(+)

Comments

Maxime Chevallier Dec. 17, 2024, 1:15 p.m. UTC | #1
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
Russell King (Oracle) Jan. 2, 2025, 9:16 a.m. UTC | #2
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 mbox series

Patch

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);