diff mbox series

[net-next,v3,4/8] net: sparx5: use phy_interface_mode_is_rgmii()

Message ID 20241118-sparx5-lan969x-switch-driver-4-v3-4-3cefee5e7e3a@microchip.com (mailing list archive)
State Deferred
Delegated to: Netdev Maintainers
Headers show
Series net: lan969x: add RGMII support | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
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: 3 this patch: 3
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 13 of 13 maintainers
netdev/build_clang success Errors and warnings before: 3 this patch: 3
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: 4 this patch: 4
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 24 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-11-18--21-00 (tests: 785)

Commit Message

Daniel Machon Nov. 18, 2024, 1 p.m. UTC 
Use the phy_interface_mode_is_rgmii() function to check if the PHY mode
is set to any of: RGMII, RGMII_ID, RGMII_RXID or RGMII_TXID in the
following places:

 - When selecting the MAC PCS, make sure we return NULL in case the PHY
   mode is RGMII (as there is no PCS to configure).

 - When doing a port config, make sure we do not do the low-speed device
   configuration, in case the PHY mode is RGMII.

Note that we could also have used is_port_rgmii() here, but it makes
more sense to me to use the phylink provided functions, as we are
called by phylink, and the RGMII modes have already been validated
against the supported interfaces of the ports.

Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com>
Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c | 3 +++
 drivers/net/ethernet/microchip/sparx5/sparx5_port.c    | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Russell King (Oracle) Nov. 19, 2024, 8:08 p.m. UTC | #1 
Hi,

On Mon, Nov 18, 2024 at 02:00:50PM +0100, Daniel Machon wrote:
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c b/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
> index f8562c1a894d..cb55e05e5611 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
> @@ -32,6 +32,9 @@ sparx5_phylink_mac_select_pcs(struct phylink_config *config,
>  {
>  	struct sparx5_port *port = netdev_priv(to_net_dev(config->dev));
>  
> +	if (phy_interface_mode_is_rgmii(interface))
> +		return NULL;
> +
>  	return &port->phylink_pcs;

Maybe turn this into positive logic - return the PCS only when the
interface mode requires the PCS?

Thanks.
Daniel Machon Nov. 20, 2024, 10:57 a.m. UTC | #2 
> Hi,
> 
> On Mon, Nov 18, 2024 at 02:00:50PM +0100, Daniel Machon wrote:
> > diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c b/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
> > index f8562c1a894d..cb55e05e5611 100644
> > --- a/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
> > +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
> > @@ -32,6 +32,9 @@ sparx5_phylink_mac_select_pcs(struct phylink_config *config,
> >  {
> >       struct sparx5_port *port = netdev_priv(to_net_dev(config->dev));
> >
> > +     if (phy_interface_mode_is_rgmii(interface))
> > +             return NULL;
> > +
> >       return &port->phylink_pcs;
> 
> Maybe turn this into positive logic - return the PCS only when the
> interface mode requires the PCS?

Sure. I can flip the logic and return the PCS for the interface modes that are
advertised as supported.

/Daniel
diff mbox series

Patch

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c b/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
index f8562c1a894d..cb55e05e5611 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
@@ -32,6 +32,9 @@  sparx5_phylink_mac_select_pcs(struct phylink_config *config,
 {
 	struct sparx5_port *port = netdev_priv(to_net_dev(config->dev));
 
+	if (phy_interface_mode_is_rgmii(interface))
+		return NULL;
+
 	return &port->phylink_pcs;
 }
 
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
index 8f4f19a2bd65..b494970752fd 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
@@ -994,6 +994,7 @@  int sparx5_port_config(struct sparx5 *sparx5,
 		       struct sparx5_port *port,
 		       struct sparx5_port_config *conf)
 {
+	bool rgmii = phy_interface_mode_is_rgmii(conf->phy_mode);
 	bool high_speed_dev = sparx5_is_baser(conf->portmode);
 	const struct sparx5_ops *ops = sparx5->data->ops;
 	int err, urgency, stop_wm;
@@ -1003,7 +1004,7 @@  int sparx5_port_config(struct sparx5 *sparx5,
 		return err;
 
 	/* high speed device is already configured */
-	if (!high_speed_dev)
+	if (!rgmii && !high_speed_dev)
 		sparx5_port_config_low_set(sparx5, port, conf);
 
 	/* Configure flow control */