| Message ID | 20220531113058.23708-3-s-vadapalli@ti.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add support for QSGMII mode to am65-cpsw driver | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On Tue, May 31, 2022 at 05:00:57PM +0530, Siddharth Vadapalli wrote: > static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned int mode, > const struct phylink_link_state *state) > { > - /* Currently not used */ > + struct am65_cpsw_slave_data *slave = container_of(config, struct am65_cpsw_slave_data, > + phylink_config); > + struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave); > + > + if (state->interface == PHY_INTERFACE_MODE_QSGMII) > + writel(AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE, > + port->sgmii_base + AM65_CPSW_SGMII_CONTROL_REG); What about writing this register when the interface mode isn't QSGMII?
Hello Russell, On 31/05/22 17:20, Russell King (Oracle) wrote: > On Tue, May 31, 2022 at 05:00:57PM +0530, Siddharth Vadapalli wrote: >> static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned int mode, >> const struct phylink_link_state *state) >> { >> - /* Currently not used */ >> + struct am65_cpsw_slave_data *slave = container_of(config, struct am65_cpsw_slave_data, >> + phylink_config); >> + struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave); >> + >> + if (state->interface == PHY_INTERFACE_MODE_QSGMII) >> + writel(AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE, >> + port->sgmii_base + AM65_CPSW_SGMII_CONTROL_REG); > > What about writing this register when the interface mode isn't QSGMII? In TI's J7200 device, there are two CPSW MACs namely CPSW2G and CPSW5G. While CPSW5G supports QSGMII mode, CPSW2G does not. The same am65-cpsw-nuss driver is used to control both CPSW2G and CPSW5G. Thus, the am65_cpsw_nuss_mac_config() function is called for both CPSW2G and CPSW5G MACs. The SGMII CONTROL Register is only present for CPSW5G. Thus, the write should only be performed for QSGMII mode which is supported only by CPSW5G. Functionally, always writing to the register even if the mode is not QSGMII does not cause any problems, as long as it is CPSW5G MAC that is being used. Thinking about it again, I will add a compatible to differentiate CPSW2G ports from CPSW5G ports in order to make it cleaner, so that the register can always be written to if the CPSW5G ports are used, irrespective of the interface mode. Thanks, Siddharth.
diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index 77bdda97b2b0..462f63313fb3 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -74,6 +74,9 @@ #define AM65_CPSW_PORTN_REG_TS_VLAN_LTYPE_REG 0x318 #define AM65_CPSW_PORTN_REG_TS_CTL_LTYPE2 0x31C +#define AM65_CPSW_SGMII_CONTROL_REG 0x010 +#define AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE BIT(0) + #define AM65_CPSW_CTL_VLAN_AWARE BIT(1) #define AM65_CPSW_CTL_P0_ENABLE BIT(2) #define AM65_CPSW_CTL_P0_TX_CRC_REMOVE BIT(13) @@ -1409,7 +1412,13 @@ static const struct net_device_ops am65_cpsw_nuss_netdev_ops = { static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned int mode, const struct phylink_link_state *state) { - /* Currently not used */ + struct am65_cpsw_slave_data *slave = container_of(config, struct am65_cpsw_slave_data, + phylink_config); + struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave); + + if (state->interface == PHY_INTERFACE_MODE_QSGMII) + writel(AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE, + port->sgmii_base + AM65_CPSW_SGMII_CONTROL_REG); } static void am65_cpsw_nuss_mac_link_down(struct phylink_config *config, unsigned int mode, @@ -1846,6 +1855,7 @@ static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common) port->common = common; port->port_base = common->cpsw_base + AM65_CPSW_NU_PORTS_BASE + AM65_CPSW_NU_PORTS_OFFSET * (port_id); + port->sgmii_base = common->ss_base + AM65_CPSW_SGMII_BASE * (port_id); port->stat_base = common->cpsw_base + AM65_CPSW_NU_STATS_BASE + (AM65_CPSW_NU_STATS_PORT_OFFSET * port_id); port->name = of_get_property(port_np, "label", NULL); @@ -1981,6 +1991,7 @@ am65_cpsw_nuss_init_port_ndev(struct am65_cpsw_common *common, u32 port_idx) port->slave.phylink_config.mac_capabilities = MAC_SYM_PAUSE | MAC_10 | MAC_100 | MAC_1000FD; phy_interface_set_rgmii(port->slave.phylink_config.supported_interfaces); + __set_bit(PHY_INTERFACE_MODE_QSGMII, port->slave.phylink_config.supported_interfaces); phylink = phylink_create(&port->slave.phylink_config, of_node_to_fwnode(port->slave.phy_node), diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.h b/drivers/net/ethernet/ti/am65-cpsw-nuss.h index ac945631bf2f..8b6297e268ec 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.h +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.h @@ -46,6 +46,7 @@ struct am65_cpsw_port { const char *name; u32 port_id; void __iomem *port_base; + void __iomem *sgmii_base; void __iomem *stat_base; void __iomem *fetch_ram_base; bool disabled;
Enable QSGMII mode in am65-cpsw driver. Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> --- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 13 ++++++++++++- drivers/net/ethernet/ti/am65-cpsw-nuss.h | 1 + 2 files changed, 13 insertions(+), 1 deletion(-)