| Message ID | 20241203124323.155866-10-maxime.chevallier@bootlin.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | net: freescale: ucc_geth: Phylink conversion | expand |
> +static bool phy_interface_mode_is_reduced(phy_interface_t interface) > +{ > + return phy_interface_mode_is_rgmii(interface) || > + interface == PHY_INTERFACE_MODE_RMII || > + interface == PHY_INTERFACE_MODE_RTBI; > +} I wounder if this is useful anywhere else? Did you take a look around other MAC drivers? Maybe this should be in phy.h? Andrew
Hello Andrew, On Wed, 4 Dec 2024 03:15:52 +0100 Andrew Lunn <andrew@lunn.ch> wrote: > > +static bool phy_interface_mode_is_reduced(phy_interface_t interface) > > +{ > > + return phy_interface_mode_is_rgmii(interface) || > > + interface == PHY_INTERFACE_MODE_RMII || > > + interface == PHY_INTERFACE_MODE_RTBI; > > +} > > I wounder if this is useful anywhere else? Did you take a look around > other MAC drivers? Maybe this should be in phy.h? Yes I did consider it but it looks like ucc_geth is the only driver that has a configuration that applies to all R(MII/GMII/TBI) interfaces :/ I didn't even know about RTBI mode before looking at that driver and it does look like it's not supported by that many devices... I'd be happy to put that in phy.h but I think ucc_geth is going to be the sole user :) Thanks, Maxime
On Wed, Dec 04, 2024 at 09:22:32AM +0100, Maxime Chevallier wrote: > Hello Andrew, > > On Wed, 4 Dec 2024 03:15:52 +0100 > Andrew Lunn <andrew@lunn.ch> wrote: > > > > +static bool phy_interface_mode_is_reduced(phy_interface_t interface) > > > +{ > > > + return phy_interface_mode_is_rgmii(interface) || > > > + interface == PHY_INTERFACE_MODE_RMII || > > > + interface == PHY_INTERFACE_MODE_RTBI; > > > +} > > > > I wounder if this is useful anywhere else? Did you take a look around > > other MAC drivers? Maybe this should be in phy.h? > > Yes I did consider it but it looks like ucc_geth is the only driver > that has a configuration that applies to all R(MII/GMII/TBI) interfaces O.K. What is important is you considered it. Thanks. Too many developers are focus on just their driver and don't think about other drivers and code reuse. Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Wed, 4 Dec 2024 16:41:33 +0100 Andrew Lunn <andrew@lunn.ch> wrote: > On Wed, Dec 04, 2024 at 09:22:32AM +0100, Maxime Chevallier wrote: > > Hello Andrew, > > > > On Wed, 4 Dec 2024 03:15:52 +0100 > > Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > +static bool phy_interface_mode_is_reduced(phy_interface_t interface) > > > > +{ > > > > + return phy_interface_mode_is_rgmii(interface) || > > > > + interface == PHY_INTERFACE_MODE_RMII || > > > > + interface == PHY_INTERFACE_MODE_RTBI; > > > > +} > > > > > > I wounder if this is useful anywhere else? Did you take a look around > > > other MAC drivers? Maybe this should be in phy.h? > > > > Yes I did consider it but it looks like ucc_geth is the only driver > > that has a configuration that applies to all R(MII/GMII/TBI) interfaces > > O.K. What is important is you considered it. Thanks. Too many > developers are focus on just their driver and don't think about other > drivers and code reuse. > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> Thanks Andrew. I should have indicated that in the commit log or in the cover in the first place though, I'll make sure to do it next time. Maxime
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c index f6dd36dc03fe..57debcba124c 100644 --- a/drivers/net/ethernet/freescale/ucc_geth.c +++ b/drivers/net/ethernet/freescale/ucc_geth.c @@ -1258,6 +1258,13 @@ static int init_min_frame_len(u16 min_frame_length, return 0; } +static bool phy_interface_mode_is_reduced(phy_interface_t interface) +{ + return phy_interface_mode_is_rgmii(interface) || + interface == PHY_INTERFACE_MODE_RMII || + interface == PHY_INTERFACE_MODE_RTBI; +} + static int adjust_enet_interface(struct ucc_geth_private *ugeth) { struct ucc_geth_info *ug_info; @@ -1290,12 +1297,7 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth) upsmr = in_be32(&uf_regs->upsmr); upsmr &= ~(UCC_GETH_UPSMR_RPM | UCC_GETH_UPSMR_R10M | UCC_GETH_UPSMR_TBIM | UCC_GETH_UPSMR_RMM); - if ((ugeth->phy_interface == PHY_INTERFACE_MODE_RMII) || - (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII) || - (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_ID) || - (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID) || - (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) || - (ugeth->phy_interface == PHY_INTERFACE_MODE_RTBI)) { + if (phy_interface_mode_is_reduced(ugeth->phy_interface)) { if (ugeth->phy_interface != PHY_INTERFACE_MODE_RMII) upsmr |= UCC_GETH_UPSMR_RPM; switch (ugeth->max_speed) { @@ -1594,9 +1596,7 @@ static void ugeth_link_up(struct ucc_geth_private *ugeth, ~(MACCFG2_INTERFACE_MODE_MASK)) | MACCFG2_INTERFACE_MODE_NIBBLE); /* if reduced mode, re-set UPSMR.R10M */ - if (interface == PHY_INTERFACE_MODE_RMII || - phy_interface_mode_is_rgmii(interface) || - interface == PHY_INTERFACE_MODE_RTBI) { + if (phy_interface_mode_is_reduced(interface)) { if (speed == SPEED_10) upsmr |= UCC_GETH_UPSMR_R10M; else
A number of parallel MII interfaces also exist in a "Reduced" mode, usually with higher clock rates and fewer data lines, to ease the hardware design. This is what the 'R' stands for in RGMII, RMII, RTBI, RXAUI, etc. The UCC Geth controller has a special configuration bit that needs to be set when the MII mode is one of the supported reduced modes. Add a local helper for that. Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- V3: No changes drivers/net/ethernet/freescale/ucc_geth.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)