Messages in this thread | | | Date | Fri, 31 Mar 2023 13:35:10 +0530 | Subject | Re: [PATCH net-next 2/2] net: ethernet: ti: am65-cpsw: Enable USXGMII mode for J784S4 CPSW9G | From | Siddharth Vadapalli <> |
| |
Hello Russell,
Thank you for reviewing the patch.
On 31/03/23 13:27, Russell King (Oracle) wrote: > On Fri, Mar 31, 2023 at 12:21:10PM +0530, Siddharth Vadapalli wrote: >> TI's J784S4 SoC supports USXGMII mode. Add USXGMII mode to the >> extra_modes member of the J784S4 SoC data. Additionally, configure the >> MAC Control register for supporting USXGMII mode. Also, for USXGMII >> mode, include MAC_5000FD in the "mac_capabilities" member of struct >> "phylink_config". > > I don't think TI "get" phylink at all... > >> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c >> index 4b4d06199b45..ab33e6fe5b1a 100644 >> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c >> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c >> @@ -1555,6 +1555,8 @@ static void am65_cpsw_nuss_mac_link_up(struct phylink_config *config, struct phy >> mac_control |= CPSW_SL_CTL_GIG; >> if (interface == PHY_INTERFACE_MODE_SGMII) >> mac_control |= CPSW_SL_CTL_EXT_EN; >> + if (interface == PHY_INTERFACE_MODE_USXGMII) >> + mac_control |= CPSW_SL_CTL_XGIG | CPSW_SL_CTL_XGMII_EN; > > The configuration of the interface mode should *not* happen in > mac_link_up(), but should happen in e.g. mac_config().
I will move all the interface mode associated configurations to mac_config() in the v2 series.
> >> if (speed == SPEED_10 && phy_interface_mode_is_rgmii(interface)) >> /* Can be used with in band mode only */ >> mac_control |= CPSW_SL_CTL_EXT_EN; >> @@ -2175,6 +2177,7 @@ am65_cpsw_nuss_init_port_ndev(struct am65_cpsw_common *common, u32 port_idx) >> >> case PHY_INTERFACE_MODE_QSGMII: >> case PHY_INTERFACE_MODE_SGMII: >> + case PHY_INTERFACE_MODE_USXGMII: >> if (common->pdata.extra_modes & BIT(port->slave.phy_if)) { >> __set_bit(port->slave.phy_if, >> port->slave.phylink_config.supported_interfaces); >> @@ -2182,6 +2185,9 @@ am65_cpsw_nuss_init_port_ndev(struct am65_cpsw_common *common, u32 port_idx) >> dev_err(dev, "selected phy-mode is not supported\n"); >> return -EOPNOTSUPP; >> } >> + /* For USXGMII mode, enable MAC_5000FD */ >> + if (port->slave.phy_if == PHY_INTERFACE_MODE_USXGMII) >> + port->slave.phylink_config.mac_capabilities |= MAC_5000FD; > > MAC capabilities should not be conditional in the interface mode. > Phylink already knows the capabilities of each interface mode, and > will mask the mac_capabilities accordingly. Phylink wants to know > what speeds the MAC itself is capable of unbound by the interface > mode. > > The interface modes that you already support (RGMII, RMII, QSGMII > and SGMII) do not support anything faster than 1G, so only > mac_capabilities up to and including 1G speeds will be permitted > for those interface modes internally by phylink. > > So, making this conditional on USXGMII is just repeating logic that > is already present internally in phylink.
Thank you for clarifying. I will fix this in the v2 series.
Regards, Siddharth.
| |