| Message ID | 20220630102041.25555-12-arun.ramadoss@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: microchip: DSA Driver support for LAN937x | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| 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/cc_maintainers | success | CCed 21 of 21 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| 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, 149 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Thu, Jun 30, 2022 at 03:50:39PM +0530, Arun Ramadoss wrote: > +static void lan937x_config_gbit(struct ksz_device *dev, bool gbit, u8 *data) > +{ > + if (gbit) > + *data &= ~PORT_MII_NOT_1GBIT; > + else > + *data |= PORT_MII_NOT_1GBIT; > +} > + > +static void lan937x_config_interface(struct ksz_device *dev, int port, > + int speed, int duplex, > + bool tx_pause, bool rx_pause) > +{ > + u8 xmii_ctrl0, xmii_ctrl1; > + > + ksz_pread8(dev, port, REG_PORT_XMII_CTRL_0, &xmii_ctrl0); > + ksz_pread8(dev, port, REG_PORT_XMII_CTRL_1, &xmii_ctrl1); > + > + switch (speed) { > + case SPEED_1000: > + lan937x_config_gbit(dev, true, &xmii_ctrl1); > + break; > + case SPEED_100: > + lan937x_config_gbit(dev, false, &xmii_ctrl1); > + xmii_ctrl0 |= PORT_MII_100MBIT; > + break; > + case SPEED_10: > + lan937x_config_gbit(dev, false, &xmii_ctrl1); > + xmii_ctrl0 &= ~PORT_MII_100MBIT; > + break; > + default: > + dev_err(dev->dev, "Unsupported speed on port %d: %d\n", > + port, speed); > + return; > + } Isn't this: if (speed == SPEED_1000) xmii_ctrl1 &= ~PORT_MII_NOT_1GBIT; else xmii_ctrl1 |= PORT_MII_NOT_1GBIT; if (speed == SPEED_100) xmii_ctrl0 |= PORT_MII_100MBIT; else xmii_ctrl0 &= ~PORT_MII_100MBIT; There isn't much need to validate that "speed" is correct, you've already told phylink that you only support 1G, 100M and 10M so you're not going to get called with anything except one of those. > + > + if (duplex) > + xmii_ctrl0 |= PORT_MII_FULL_DUPLEX; > + else > + xmii_ctrl0 &= ~PORT_MII_FULL_DUPLEX; > + > + if (tx_pause) > + xmii_ctrl0 |= PORT_MII_TX_FLOW_CTRL; > + else > + xmii_ctrl1 &= ~PORT_MII_TX_FLOW_CTRL; It seems weird to set a bit in one register and clear it in a different register. I suspect you mean xmii_ctrl0 here. > + > + if (rx_pause) > + xmii_ctrl0 |= PORT_MII_RX_FLOW_CTRL; > + else > + xmii_ctrl0 &= ~PORT_MII_RX_FLOW_CTRL; > + > + ksz_pwrite8(dev, port, REG_PORT_XMII_CTRL_0, xmii_ctrl0); > + ksz_pwrite8(dev, port, REG_PORT_XMII_CTRL_1, xmii_ctrl1); > +} > + Thanks!
Hi Russell, Thanks for the review comment. On Thu, 2022-06-30 at 12:42 +0100, Russell King (Oracle) wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On Thu, Jun 30, 2022 at 03:50:39PM +0530, Arun Ramadoss wrote: > > +static void lan937x_config_gbit(struct ksz_device *dev, bool gbit, > > u8 *data) > > +{ > > + if (gbit) > > + *data &= ~PORT_MII_NOT_1GBIT; > > + else > > + *data |= PORT_MII_NOT_1GBIT; > > +} > > + > > +static void lan937x_config_interface(struct ksz_device *dev, int > > port, > > + int speed, int duplex, > > + bool tx_pause, bool rx_pause) > > +{ > > + u8 xmii_ctrl0, xmii_ctrl1; > > + > > + ksz_pread8(dev, port, REG_PORT_XMII_CTRL_0, &xmii_ctrl0); > > + ksz_pread8(dev, port, REG_PORT_XMII_CTRL_1, &xmii_ctrl1); > > + > > + switch (speed) { > > + case SPEED_1000: > > + lan937x_config_gbit(dev, true, &xmii_ctrl1); > > + break; > > + case SPEED_100: > > + lan937x_config_gbit(dev, false, &xmii_ctrl1); > > + xmii_ctrl0 |= PORT_MII_100MBIT; > > + break; > > + case SPEED_10: > > + lan937x_config_gbit(dev, false, &xmii_ctrl1); > > + xmii_ctrl0 &= ~PORT_MII_100MBIT; > > + break; > > + default: > > + dev_err(dev->dev, "Unsupported speed on port %d: > > %d\n", > > + port, speed); > > + return; > > + } > > Isn't this: > > if (speed == SPEED_1000) > xmii_ctrl1 &= ~PORT_MII_NOT_1GBIT; > else > xmii_ctrl1 |= PORT_MII_NOT_1GBIT; > > if (speed == SPEED_100) > xmii_ctrl0 |= PORT_MII_100MBIT; > else > xmii_ctrl0 &= ~PORT_MII_100MBIT; > > There isn't much need to validate that "speed" is correct, you've > already told phylink that you only support 1G, 100M and 10M so you're > not going to get called with anything except one of those. Ok, I will update the code. > > > + > > + if (duplex) > > + xmii_ctrl0 |= PORT_MII_FULL_DUPLEX; > > + else > > + xmii_ctrl0 &= ~PORT_MII_FULL_DUPLEX; > > + > > + if (tx_pause) > > + xmii_ctrl0 |= PORT_MII_TX_FLOW_CTRL; > > + else > > + xmii_ctrl1 &= ~PORT_MII_TX_FLOW_CTRL; > > It seems weird to set a bit in one register and clear it in a > different > register. I suspect you mean xmii_ctrl0 here. Sorry, its a typo mistake. I will update the register. > > > + > > + if (rx_pause) > > + xmii_ctrl0 |= PORT_MII_RX_FLOW_CTRL; > > + else > > + xmii_ctrl0 &= ~PORT_MII_RX_FLOW_CTRL; > > + > > + ksz_pwrite8(dev, port, REG_PORT_XMII_CTRL_0, xmii_ctrl0); > > + ksz_pwrite8(dev, port, REG_PORT_XMII_CTRL_1, xmii_ctrl1); > > +} > > + > > Thanks! > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index ca7ca327285d..9972b2fabf27 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -221,6 +221,7 @@ static const struct ksz_dev_ops lan937x_dev_ops = { .mirror_add = ksz9477_port_mirror_add, .mirror_del = ksz9477_port_mirror_del, .get_caps = lan937x_phylink_get_caps, + .phylink_mac_link_up = lan937x_phylink_mac_link_up, .fdb_dump = ksz9477_fdb_dump, .fdb_add = ksz9477_fdb_add, .fdb_del = ksz9477_fdb_del, @@ -1340,6 +1341,20 @@ static int ksz_max_mtu(struct dsa_switch *ds, int port) return dev->dev_ops->max_mtu(dev, port); } +static void ksz_phylink_mac_link_up(struct dsa_switch *ds, int port, + unsigned int mode, + phy_interface_t interface, + struct phy_device *phydev, int speed, + int duplex, bool tx_pause, bool rx_pause) +{ + struct ksz_device *dev = ds->priv; + + if (dev->dev_ops->phylink_mac_link_up) + dev->dev_ops->phylink_mac_link_up(dev, port, mode, interface, + phydev, speed, duplex, + tx_pause, rx_pause); +} + static int ksz_switch_detect(struct ksz_device *dev) { u8 id1, id2; @@ -1413,6 +1428,7 @@ static const struct dsa_switch_ops ksz_switch_ops = { .phy_read = ksz_phy_read16, .phy_write = ksz_phy_write16, .phylink_get_caps = ksz_phylink_get_caps, + .phylink_mac_link_up = ksz_phylink_mac_link_up, .phylink_mac_link_down = ksz_mac_link_down, .port_enable = ksz_enable_port, .get_strings = ksz_get_strings, diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h index bf4f3f3922a5..f449feab5499 100644 --- a/drivers/net/dsa/microchip/ksz_common.h +++ b/drivers/net/dsa/microchip/ksz_common.h @@ -271,6 +271,11 @@ struct ksz_dev_ops { int (*max_mtu)(struct ksz_device *dev, int port); void (*freeze_mib)(struct ksz_device *dev, int port, bool freeze); void (*port_init_cnt)(struct ksz_device *dev, int port); + void (*phylink_mac_link_up)(struct ksz_device *dev, int port, + unsigned int mode, + phy_interface_t interface, + struct phy_device *phydev, int speed, + int duplex, bool tx_pause, bool rx_pause); void (*config_cpu_port)(struct dsa_switch *ds); int (*enable_stp_addr)(struct ksz_device *dev); int (*reset)(struct ksz_device *dev); diff --git a/drivers/net/dsa/microchip/lan937x.h b/drivers/net/dsa/microchip/lan937x.h index d4207e97a130..145770aec963 100644 --- a/drivers/net/dsa/microchip/lan937x.h +++ b/drivers/net/dsa/microchip/lan937x.h @@ -17,4 +17,8 @@ void lan937x_w_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 val); int lan937x_change_mtu(struct ksz_device *dev, int port, int new_mtu); void lan937x_phylink_get_caps(struct ksz_device *dev, int port, struct phylink_config *config); +void lan937x_phylink_mac_link_up(struct ksz_device *dev, int port, + unsigned int mode, phy_interface_t interface, + struct phy_device *phydev, int speed, + int duplex, bool tx_pause, bool rx_pause); #endif diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c index 8cb46caf5340..95700e0233de 100644 --- a/drivers/net/dsa/microchip/lan937x_main.c +++ b/drivers/net/dsa/microchip/lan937x_main.c @@ -312,6 +312,60 @@ int lan937x_change_mtu(struct ksz_device *dev, int port, int new_mtu) return 0; } +static void lan937x_config_gbit(struct ksz_device *dev, bool gbit, u8 *data) +{ + if (gbit) + *data &= ~PORT_MII_NOT_1GBIT; + else + *data |= PORT_MII_NOT_1GBIT; +} + +static void lan937x_config_interface(struct ksz_device *dev, int port, + int speed, int duplex, + bool tx_pause, bool rx_pause) +{ + u8 xmii_ctrl0, xmii_ctrl1; + + ksz_pread8(dev, port, REG_PORT_XMII_CTRL_0, &xmii_ctrl0); + ksz_pread8(dev, port, REG_PORT_XMII_CTRL_1, &xmii_ctrl1); + + switch (speed) { + case SPEED_1000: + lan937x_config_gbit(dev, true, &xmii_ctrl1); + break; + case SPEED_100: + lan937x_config_gbit(dev, false, &xmii_ctrl1); + xmii_ctrl0 |= PORT_MII_100MBIT; + break; + case SPEED_10: + lan937x_config_gbit(dev, false, &xmii_ctrl1); + xmii_ctrl0 &= ~PORT_MII_100MBIT; + break; + default: + dev_err(dev->dev, "Unsupported speed on port %d: %d\n", + port, speed); + return; + } + + if (duplex) + xmii_ctrl0 |= PORT_MII_FULL_DUPLEX; + else + xmii_ctrl0 &= ~PORT_MII_FULL_DUPLEX; + + if (tx_pause) + xmii_ctrl0 |= PORT_MII_TX_FLOW_CTRL; + else + xmii_ctrl1 &= ~PORT_MII_TX_FLOW_CTRL; + + if (rx_pause) + xmii_ctrl0 |= PORT_MII_RX_FLOW_CTRL; + else + xmii_ctrl0 &= ~PORT_MII_RX_FLOW_CTRL; + + ksz_pwrite8(dev, port, REG_PORT_XMII_CTRL_0, xmii_ctrl0); + ksz_pwrite8(dev, port, REG_PORT_XMII_CTRL_1, xmii_ctrl1); +} + void lan937x_phylink_get_caps(struct ksz_device *dev, int port, struct phylink_config *config) { @@ -324,6 +378,19 @@ void lan937x_phylink_get_caps(struct ksz_device *dev, int port, } } +void lan937x_phylink_mac_link_up(struct ksz_device *dev, int port, + unsigned int mode, phy_interface_t interface, + struct phy_device *phydev, int speed, + int duplex, bool tx_pause, bool rx_pause) +{ + /* Internal PHYs */ + if (dev->info->internal_phy[port]) + return; + + lan937x_config_interface(dev, port, speed, duplex, + tx_pause, rx_pause); +} + int lan937x_setup(struct dsa_switch *ds) { struct ksz_device *dev = ds->priv; diff --git a/drivers/net/dsa/microchip/lan937x_reg.h b/drivers/net/dsa/microchip/lan937x_reg.h index 19f3aa344228..c187d0a3e7fa 100644 --- a/drivers/net/dsa/microchip/lan937x_reg.h +++ b/drivers/net/dsa/microchip/lan937x_reg.h @@ -139,6 +139,17 @@ #define PORT_MII_RX_FLOW_CTRL BIT(3) #define PORT_GRXC_ENABLE BIT(0) +#define REG_PORT_XMII_CTRL_1 0x0301 +#define PORT_MII_NOT_1GBIT BIT(6) +#define PORT_MII_SEL_EDGE BIT(5) +#define PORT_RGMII_ID_IG_ENABLE BIT(4) +#define PORT_RGMII_ID_EG_ENABLE BIT(3) +#define PORT_MII_MAC_MODE BIT(2) +#define PORT_MII_SEL_M 0x3 +#define PORT_RGMII_SEL 0x0 +#define PORT_RMII_SEL 0x1 +#define PORT_MII_SEL 0x2 + /* 4 - MAC */ #define REG_PORT_MAC_CTRL_0 0x0400 #define PORT_CHECK_LENGTH BIT(2)
This patch add support for phylink_mac_link_up. It configures the mac for the speed, flow control and duplex mode. Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com> --- drivers/net/dsa/microchip/ksz_common.c | 16 ++++++ drivers/net/dsa/microchip/ksz_common.h | 5 ++ drivers/net/dsa/microchip/lan937x.h | 4 ++ drivers/net/dsa/microchip/lan937x_main.c | 67 ++++++++++++++++++++++++ drivers/net/dsa/microchip/lan937x_reg.h | 11 ++++ 5 files changed, 103 insertions(+)