| Message ID | 20220508224848.2384723-5-hauke@hauke-m.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: realtek: rtl8365mb: Add SGMII and HSGMII support | expand |
On Mon, May 09, 2022 at 12:48:48AM +0200, Hauke Mehrtens wrote: > This adds support for SGMII and HSGMII on RTL8367S switches. > HSGMII is configured using the 2500BASEX mode. > This is baed on the rtl8367c driver found in OpenWrt. > > For (H)SGMII mode we have to load a firmware into some memory which gets > executed on the integrated 8051. The firmware binary was added as a > array into the driver with a GPL license notice on top. > > This was tested on RTL8367S (ver=0x00a0, opt=0x0001). > > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > --- > drivers/net/dsa/realtek/rtl8365mb.c | 356 +++++++++++++++++++++++++++- > 1 file changed, 345 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c > index f9b690251155..5504a34fffeb 100644 > --- a/drivers/net/dsa/realtek/rtl8365mb.c > +++ b/drivers/net/dsa/realtek/rtl8365mb.c > @@ -3,6 +3,7 @@ > * > * Copyright (C) 2021 Alvin Šipraga <alsi@bang-olufsen.dk> > * Copyright (C) 2021 Michael Rasmussen <mir@bang-olufsen.dk> > + * Copyright (C) 2022 Hauke Mehrtens <hauke@hauke-m.de> > * > * The RTL8365MB-VC is a 4+1 port 10/100/1000M switch controller. It includes 4 > * integrated PHYs for the user facing ports, and an extension interface which > @@ -98,6 +99,7 @@ > #include <linux/of_irq.h> > #include <linux/regmap.h> > #include <linux/if_bridge.h> > +#include <linux/firmware.h> > > #include "realtek.h" > > @@ -135,6 +137,7 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, > > /* Chip reset register */ > #define RTL8365MB_CHIP_RESET_REG 0x1322 > +#define RTL8365MB_CHIP_RESET_DW8051 BIT(4) > #define RTL8365MB_CHIP_RESET_SW_MASK 0x0002 > #define RTL8365MB_CHIP_RESET_HW_MASK 0x0001 > > @@ -278,6 +281,29 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, > #define RTL8365MB_DIGITAL_INTERFACE_FORCE_DUPLEX_MASK 0x0004 > #define RTL8365MB_DIGITAL_INTERFACE_FORCE_SPEED_MASK 0x0003 > > +#define RTL8365MB_SDS_MISC 0x1d11 > +#define RTL8365MB_CFG_SGMII_RXFC 0x4000 > +#define RTL8365MB_CFG_SGMII_TXFC 0x2000 > +#define RTL8365MB_INB_ARB 0x1000 > +#define RTL8365MB_CFG_MAC8_SEL_HSGMII 0x0800 > +#define RTL8365MB_CFG_SGMII_FDUP 0x0400 > +#define RTL8365MB_CFG_SGMII_LINK 0x0200 > +#define RTL8365MB_CFG_SGMII_SPD 0x0180 > +#define RTL8365MB_CFG_MAC8_SEL_SGMII 0x0040 > +#define RTL8365MB_CFG_INB_SEL 0x0038 > +#define RTL8365MB_CFG_SDS_MODE_18C 0x0007 > + > +#define RTL8365MB_SDS_INDACS_CMD 0x6600 > +#define RTL8365MB_SDS_INDACS_ADR 0x6601 > +#define RTL8365MB_SDS_INDACS_DATA 0x6602 > + > +#define RTL8365MB_MISC_CFG0 0x130c > +#define RTL8365MB_MISC_CFG0_DW8051_EN BIT(5) > + > +#define RTL8365MB_DW8051_RDY 0x1336 > +#define RTL8365MB_DW8051_RDY_IROM_MSB BIT(2) > +#define RTL8365MB_DW8051_RDY_ACS_IROM_EN BIT(1) > + > /* CPU port mask register - controls which ports are treated as CPU ports */ > #define RTL8365MB_CPU_PORT_MASK_REG 0x1219 > #define RTL8365MB_CPU_PORT_MASK_MASK 0x07FF > @@ -296,6 +322,8 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, > #define RTL8365MB_CFG0_MAX_LEN_REG 0x088C > #define RTL8365MB_CFG0_MAX_LEN_MASK 0x3FFF > > +#define RTL8365MB_BYPASS_LINE_RATE 0x03f7 > + > /* Port learning limit registers */ > #define RTL8365MB_LUT_PORT_LEARN_LIMIT_BASE 0x0A20 > #define RTL8365MB_LUT_PORT_LEARN_LIMIT_REG(_physport) \ > @@ -493,6 +521,39 @@ static const struct rtl8365mb_jam_tbl_entry rtl8365mb_init_jam_common[] = { > { 0x1D32, 0x0002 }, > }; > > +struct rtl8365mb_sds_init { > + unsigned int data; > + unsigned int addr; > +}; > + > +static const struct rtl8365mb_sds_init redData[] = { > + {0x04D7, 0x0480}, {0xF994, 0x0481}, {0x21A2, 0x0482}, {0x6960, 0x0483}, > + {0x9728, 0x0484}, {0x9D85, 0x0423}, {0xD810, 0x0424}, {0x83F2, 0x002E} > +}; > + > +static const struct rtl8365mb_sds_init redDataSB[] = { > + {0x04D7, 0x0480}, {0xF994, 0x0481}, {0x31A2, 0x0482}, {0x6960, 0x0483}, > + {0x9728, 0x0484}, {0x9D85, 0x0423}, {0xD810, 0x0424}, {0x83F2, 0x002E} > +}; > + > +static const struct rtl8365mb_sds_init redData1_5_6[] = { > + {0x82F1, 0x0500}, {0xF195, 0x0501}, {0x31A2, 0x0502}, {0x796C, 0x0503}, > + {0x9728, 0x0504}, {0x9D85, 0x0423}, {0xD810, 0x0424}, {0x0F80, 0x0001}, > + {0x83F2, 0x002E} > +}; > + > +static const struct rtl8365mb_sds_init redData8_9[] = { > + {0x82F1, 0x0500}, {0xF995, 0x0501}, {0x31A2, 0x0502}, {0x796C, 0x0503}, > + {0x9728, 0x0504}, {0x9D85, 0x0423}, {0xD810, 0x0424}, {0x0F80, 0x0001}, > + {0x83F2, 0x002E} > +}; > + > +static const struct rtl8365mb_sds_init redDataHB[] = { > + {0x82F0, 0x0500}, {0xF195, 0x0501}, {0x31A2, 0x0502}, {0x7960, 0x0503}, > + {0x9728, 0x0504}, {0x9D85, 0x0423}, {0xD810, 0x0424}, {0x0F80, 0x0001}, > + {0x83F2, 0x002E} > +}; > + > enum rtl8365mb_stp_state { > RTL8365MB_STP_STATE_DISABLED = 0, > RTL8365MB_STP_STATE_BLOCKING = 1, > @@ -801,6 +862,232 @@ rtl8365mb_get_tag_protocol(struct dsa_switch *ds, int port, > return DSA_TAG_PROTO_RTL8_4; > } > > +static int rtl8365mb_sds_indacs_write(struct realtek_priv *priv, unsigned int addr, > + unsigned int data) > +{ > + int ret; > + > + ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_DATA, data); > + if (ret) > + return ret; > + > + ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_ADR, addr); > + if (ret) > + return ret; > + > + return regmap_write(priv->map, RTL8365MB_SDS_INDACS_CMD, 0x00C0); > +} > + > +static int rtl8365mb_sds_indacs_read(struct realtek_priv *priv, unsigned int addr, > + unsigned int *data) > +{ > + int ret; > + > + ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_ADR, addr); > + if (ret) > + return ret; > + > + ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_CMD, 0x00C0); > + if (ret) > + return ret; > + > + return regmap_write(priv->map, RTL8365MB_SDS_INDACS_DATA, *data); > +} > + > +static int rtl8365mb_ext_init_sgmii_fw(struct realtek_priv *priv) > +{ > + struct device *dev = priv->dev; > + const struct firmware *fw; > + int ret; > + int i; > + > + ret = request_firmware(&fw, "rtl_switch/rtl8367s-sgmii.bin", dev); > + if (ret) { > + dev_err(dev, "failed to load firmware rtl_switch/rtl8367s-sgmii.bin, error: %i\n", Can you put the firmware name here and in the MODULE_FIRMWARE definition behind a common macro? > + ret); > + return ret; > + } > + > + ret = regmap_update_bits(priv->map, RTL8365MB_CHIP_RESET_REG, > + RTL8365MB_CHIP_RESET_DW8051, > + FIELD_PREP(RTL8365MB_CHIP_RESET_DW8051, 1)); > + if (ret) > + goto release_fw; > + > + ret = regmap_update_bits(priv->map, RTL8365MB_MISC_CFG0, > + RTL8365MB_MISC_CFG0_DW8051_EN, > + FIELD_PREP(RTL8365MB_MISC_CFG0_DW8051_EN, 1)); > + if (ret) > + goto release_fw; > + > + ret = regmap_update_bits(priv->map, RTL8365MB_DW8051_RDY, > + RTL8365MB_DW8051_RDY_ACS_IROM_EN, > + FIELD_PREP(RTL8365MB_DW8051_RDY_ACS_IROM_EN, 1)); > + if (ret) > + goto release_fw; > + > + ret = regmap_update_bits(priv->map, RTL8365MB_DW8051_RDY, > + RTL8365MB_DW8051_RDY_IROM_MSB, > + FIELD_PREP(RTL8365MB_DW8051_RDY_IROM_MSB, 0)); > + if (ret) > + goto release_fw; > + > + for (i = 0; i < fw->size; i++) { > + ret = regmap_write(priv->map, 0xE000 + i, fw->data[i]); > + if (ret) > + goto release_fw; > + } > + > + ret = regmap_update_bits(priv->map, RTL8365MB_DW8051_RDY, > + RTL8365MB_DW8051_RDY_IROM_MSB, > + FIELD_PREP(RTL8365MB_DW8051_RDY_IROM_MSB, 0)); > + if (ret) > + goto release_fw; > + > + ret = regmap_update_bits(priv->map, RTL8365MB_DW8051_RDY, > + RTL8365MB_DW8051_RDY_ACS_IROM_EN, > + FIELD_PREP(RTL8365MB_DW8051_RDY_ACS_IROM_EN, 0)); > + if (ret) > + goto release_fw; > + > + ret = regmap_update_bits(priv->map, RTL8365MB_CHIP_RESET_REG, > + RTL8365MB_CHIP_RESET_DW8051, > + FIELD_PREP(RTL8365MB_CHIP_RESET_DW8051, 0)); > + > +release_fw: > + release_firmware(fw); > + return ret; > +} > + > +static int rtl8365mb_ext_init_sgmii(struct realtek_priv *priv, int port, phy_interface_t interface) > +{ > + struct rtl8365mb *mb; > + int interface_mode; > + int sds_mode; > + const struct rtl8365mb_sds_init *sds_init; > + size_t sds_init_len; > + int ext_int; > + int ret; > + int i; > + int val; > + int mask; Please keep variables sorted longest to shortest. > + > + mb = priv->chip_data; > + > + if (mb->chip_id != RTL8365MB_CHIP_ID_8365MB_VC) > + return -EINVAL; > + > + ext_int = rtl8365mb_extint_port_map[port]; > + if (ext_int != 1) > + return -EINVAL; > + > + if (interface == PHY_INTERFACE_MODE_SGMII) { > + sds_mode = FIELD_PREP(RTL8365MB_CFG_MAC8_SEL_SGMII, 1); > + interface_mode = RTL8365MB_EXT_PORT_MODE_SGMII; > + > + if (mb->chip_option == 0) { > + sds_init = redData; > + sds_init_len = ARRAY_SIZE(redData); > + } else { > + sds_init = redDataSB; > + sds_init_len = ARRAY_SIZE(redDataSB); > + } > + } else if (interface == PHY_INTERFACE_MODE_2500BASEX) { > + sds_mode = FIELD_PREP(RTL8365MB_CFG_MAC8_SEL_HSGMII, 1); > + interface_mode = RTL8365MB_EXT_PORT_MODE_HSGMII; > + > + if (mb->chip_option == 0) { > + switch (mb->chip_ver & 0x00F0) { > + case 0x0010: > + case 0x0050: > + case 0x0060: > + sds_init = redData1_5_6; > + sds_init_len = ARRAY_SIZE(redData1_5_6); > + break; > + case 0x0080: > + case 0x0090: > + sds_init = redData8_9; > + sds_init_len = ARRAY_SIZE(redData8_9); > + break; > + default: > + return -EINVAL; > + } > + } else { > + sds_init = redDataHB; > + sds_init_len = ARRAY_SIZE(redDataHB); > + } > + } else { > + return -EINVAL; > + } > + > + for (i = 0; i < sds_init_len; i++) { > + ret = rtl8365mb_sds_indacs_write(priv, sds_init[i].addr, sds_init[i].data); > + if (ret) > + return ret; > + } > + > + mask = RTL8365MB_CFG_MAC8_SEL_SGMII | RTL8365MB_CFG_MAC8_SEL_HSGMII; > + ret = regmap_update_bits(priv->map, > + RTL8365MB_SDS_MISC, > + mask, > + sds_mode); > + if (ret) > + return ret; > + > + mask = RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_MASK(ext_int); > + val = interface_mode << RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_OFFSET(ext_int); > + ret = regmap_update_bits(priv->map, > + RTL8365MB_DIGITAL_INTERFACE_SELECT_REG(ext_int), > + mask, > + val); > + if (ret) > + return ret; > + > + ret = regmap_write(priv->map, RTL8365MB_BYPASS_LINE_RATE, 0x0); > + if (ret) > + return ret; > + > + /* Serdes not reset */ > + ret = rtl8365mb_sds_indacs_write(priv, 0x0003, 0x7106); > + if (ret) > + return ret; > + > + return rtl8365mb_ext_init_sgmii_fw(priv); > +} > + > +static int rtl8365mb_ext_sgmii_nway(struct realtek_priv *priv, bool state) What is "nway"? In-band autoneg by any chance? If so, could you pass "phylink_autoneg_inband(mode)" rather than "false" as "state"? > +{ > + u32 running; > + u32 regValue; > + int ret; > + > + ret = regmap_read(priv->map, RTL8365MB_MISC_CFG0, &running); > + if (running & RTL8365MB_MISC_CFG0_DW8051_EN) { > + ret = regmap_update_bits(priv->map, RTL8365MB_MISC_CFG0, > + RTL8365MB_MISC_CFG0_DW8051_EN, > + FIELD_PREP(RTL8365MB_MISC_CFG0_DW8051_EN, 0)); > + if (ret) > + return ret; > + } > + > + ret = rtl8365mb_sds_indacs_read(priv, 0x0002, ®Value); > + if (ret) > + return ret; > + > + if (state) > + regValue |= 0x0200; > + else > + regValue &= ~0x0200; No camelCase please. > + regValue |= 0x0100; > + > + ret = rtl8365mb_sds_indacs_write(priv, 0x0002, regValue); Any idea what the magic numbers are? At least 0x0200 looks like BIT(9). > + if (ret) > + return ret; > + return regmap_update_bits(priv->map, RTL8365MB_MISC_CFG0, > + RTL8365MB_MISC_CFG0_DW8051_EN, > + FIELD_PREP(RTL8365MB_MISC_CFG0_DW8051_EN, 1)); > +} > + > static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port, > phy_interface_t interface) > { > @@ -886,6 +1173,7 @@ static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port, > } > > static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, > + phy_interface_t interface, > bool link, int speed, int duplex, > bool tx_pause, bool rx_pause) > { > @@ -911,7 +1199,7 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, > r_rx_pause = rx_pause ? 1 : 0; > r_tx_pause = tx_pause ? 1 : 0; > > - if (speed == SPEED_1000) { > + if (speed == SPEED_1000 || speed == SPEED_2500) { > r_speed = RTL8365MB_PORT_SPEED_1000M; > } else if (speed == SPEED_100) { > r_speed = RTL8365MB_PORT_SPEED_100M; > @@ -941,6 +1229,25 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, > r_duplex = 0; > } > > + if (interface == PHY_INTERFACE_MODE_SGMII || > + interface == PHY_INTERFACE_MODE_2500BASEX) { > + val = FIELD_PREP(RTL8365MB_CFG_SGMII_FDUP, r_duplex) | > + FIELD_PREP(RTL8365MB_CFG_SGMII_SPD, r_speed) | > + FIELD_PREP(RTL8365MB_CFG_SGMII_LINK, r_link) | > + FIELD_PREP(RTL8365MB_CFG_SGMII_TXFC, r_tx_pause) | > + FIELD_PREP(RTL8365MB_CFG_SGMII_RXFC, r_rx_pause); > + ret = regmap_update_bits(priv->map, > + RTL8365MB_SDS_MISC, > + RTL8365MB_CFG_SGMII_FDUP | > + RTL8365MB_CFG_SGMII_SPD | > + RTL8365MB_CFG_SGMII_LINK | > + RTL8365MB_CFG_SGMII_TXFC | > + RTL8365MB_CFG_SGMII_RXFC, > + val); > + if (ret) > + return ret; > + } > + > val = FIELD_PREP(RTL8365MB_DIGITAL_INTERFACE_FORCE_EN_MASK, 1) | > FIELD_PREP(RTL8365MB_DIGITAL_INTERFACE_FORCE_TXPAUSE_MASK, > r_tx_pause) | > @@ -972,10 +1279,15 @@ static bool rtl8365mb_phy_mode_supported(struct dsa_switch *ds, int port, > interface == PHY_INTERFACE_MODE_GMII)) > /* Internal PHY */ > return true; > - else if ((ext_int >= 1) && > - phy_interface_mode_is_rgmii(interface)) > + else if ((ext_int == 1) && > + (phy_interface_mode_is_rgmii(interface) || > + interface == PHY_INTERFACE_MODE_SGMII || > + interface == PHY_INTERFACE_MODE_2500BASEX)) > /* Extension MAC */ > return true; > + else if ((ext_int >= 2) && > + phy_interface_mode_is_rgmii(interface)) > + return true; > > return false; > } > @@ -983,14 +1295,25 @@ static bool rtl8365mb_phy_mode_supported(struct dsa_switch *ds, int port, > static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port, > struct phylink_config *config) > { > - if (dsa_is_user_port(ds, port)) > + int ext_int = rtl8365mb_extint_port_map[port]; > + > + config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | > + MAC_10 | MAC_100 | MAC_1000FD; > + > + if (dsa_is_user_port(ds, port)) { > __set_bit(PHY_INTERFACE_MODE_INTERNAL, > config->supported_interfaces); > - else if (dsa_is_cpu_port(ds, port)) > + } else if (dsa_is_cpu_port(ds, port)) { What does the quality of being a user port or a CPU port have to do with which interfaces are supported? > + if (ext_int == 1) { > + __set_bit(PHY_INTERFACE_MODE_SGMII, > + config->supported_interfaces); > + __set_bit(PHY_INTERFACE_MODE_2500BASEX, > + config->supported_interfaces); > + config->mac_capabilities |= MAC_2500FD; > + } > phy_interface_set_rgmii(config->supported_interfaces); > + } > > - config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | > - MAC_10 | MAC_100 | MAC_1000FD; > } > > static void rtl8365mb_phylink_mac_config(struct dsa_switch *ds, int port, > @@ -1020,6 +1343,10 @@ static void rtl8365mb_phylink_mac_config(struct dsa_switch *ds, int port, > "failed to configure RGMII mode on port %d: %d\n", > port, ret); > return; > + } else if (state->interface == PHY_INTERFACE_MODE_SGMII || > + state->interface == PHY_INTERFACE_MODE_2500BASEX) { > + rtl8365mb_ext_init_sgmii(priv, port, state->interface); > + rtl8365mb_ext_sgmii_nway(priv, false); > } > > /* TODO: Implement MII and RMII modes, which the RTL8365MB-VC also > @@ -1040,8 +1367,11 @@ static void rtl8365mb_phylink_mac_link_down(struct dsa_switch *ds, int port, > p = &mb->ports[port]; > cancel_delayed_work_sync(&p->mib_work); > > - if (phy_interface_mode_is_rgmii(interface)) { > - ret = rtl8365mb_ext_config_forcemode(priv, port, false, 0, 0, > + if (phy_interface_mode_is_rgmii(interface) || > + interface == PHY_INTERFACE_MODE_SGMII || > + interface == PHY_INTERFACE_MODE_2500BASEX) { If in-band autoneg is possible, there will have to be an additional condition here, to only force the mode if it is disabled (which it now is) > + ret = rtl8365mb_ext_config_forcemode(priv, port, interface, > + false, 0, 0, > false, false); > if (ret) > dev_err(priv->dev, > @@ -1068,8 +1398,11 @@ static void rtl8365mb_phylink_mac_link_up(struct dsa_switch *ds, int port, > p = &mb->ports[port]; > schedule_delayed_work(&p->mib_work, 0); > > - if (phy_interface_mode_is_rgmii(interface)) { > - ret = rtl8365mb_ext_config_forcemode(priv, port, true, speed, > + if (phy_interface_mode_is_rgmii(interface) || > + interface == PHY_INTERFACE_MODE_SGMII || > + interface == PHY_INTERFACE_MODE_2500BASEX) { > + ret = rtl8365mb_ext_config_forcemode(priv, port, interface, > + true, speed, > duplex, tx_pause, > rx_pause); > if (ret) > @@ -2156,6 +2489,7 @@ const struct realtek_variant rtl8365mb_variant = { > }; > EXPORT_SYMBOL_GPL(rtl8365mb_variant); > > +MODULE_FIRMWARE("rtl_switch/rtl8367s-sgmii.bin"); > MODULE_AUTHOR("Alvin Šipraga <alsi@bang-olufsen.dk>"); > MODULE_DESCRIPTION("Driver for RTL8365MB-VC ethernet switch"); > MODULE_LICENSE("GPL"); > -- > 2.30.2 >
On Mon, May 09, 2022 at 12:48:48AM +0200, Hauke Mehrtens wrote: > This adds support for SGMII and HSGMII on RTL8367S switches. > HSGMII is configured using the 2500BASEX mode. > This is baed on the rtl8367c driver found in OpenWrt. > > For (H)SGMII mode we have to load a firmware into some memory which gets > executed on the integrated 8051. The firmware binary was added as a > array into the driver with a GPL license notice on top. > > This was tested on RTL8367S (ver=0x00a0, opt=0x0001). > > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > --- > drivers/net/dsa/realtek/rtl8365mb.c | 356 +++++++++++++++++++++++++++- > 1 file changed, 345 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c > index f9b690251155..5504a34fffeb 100644 > --- a/drivers/net/dsa/realtek/rtl8365mb.c > +++ b/drivers/net/dsa/realtek/rtl8365mb.c > @@ -3,6 +3,7 @@ > * > * Copyright (C) 2021 Alvin ┼áipraga <alsi@bang-olufsen.dk> > * Copyright (C) 2021 Michael Rasmussen <mir@bang-olufsen.dk> > + * Copyright (C) 2022 Hauke Mehrtens <hauke@hauke-m.de> > * > * The RTL8365MB-VC is a 4+1 port 10/100/1000M switch controller. It includes 4 > * integrated PHYs for the user facing ports, and an extension interface which > @@ -98,6 +99,7 @@ > #include <linux/of_irq.h> > #include <linux/regmap.h> > #include <linux/if_bridge.h> > +#include <linux/firmware.h> > > #include "realtek.h" > > @@ -135,6 +137,7 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, > > /* Chip reset register */ > #define RTL8365MB_CHIP_RESET_REG 0x1322 > +#define RTL8365MB_CHIP_RESET_DW8051 BIT(4) Please stick with the convention and use 0x0010. > #define RTL8365MB_CHIP_RESET_SW_MASK 0x0002 > #define RTL8365MB_CHIP_RESET_HW_MASK 0x0001 > > @@ -278,6 +281,29 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, > #define RTL8365MB_DIGITAL_INTERFACE_FORCE_DUPLEX_MASK 0x0004 > #define RTL8365MB_DIGITAL_INTERFACE_FORCE_SPEED_MASK 0x0003 > > +#define RTL8365MB_SDS_MISC 0x1d11 > +#define RTL8365MB_CFG_SGMII_RXFC 0x4000 Please stick to the existing naming conventions of the Linux driver rather than the Realtek (OpenWrt) one. The convention is roughly something like this: reg name _REG because it's a register vvvvvvvvvv vvv #define RTL8365MB_COOL_STUFF_REG 0xC001 < all-caps hex #define RTL8365MB_COOL_STUFF_RINGTONE_SELECT_MASK 0x8000 ^^ ^^^^^^^^^^ ^^^^^^^^^^^^^^^ ^^^^ 2 space indent reg name field name _MASK because it's a mask You are of course at liberty to use a different register name to the Realtek driver if you think it fits better. > +#define RTL8365MB_CFG_SGMII_TXFC 0x2000 > +#define RTL8365MB_INB_ARB 0x1000 > +#define RTL8365MB_CFG_MAC8_SEL_HSGMII 0x0800 > +#define RTL8365MB_CFG_SGMII_FDUP 0x0400 > +#define RTL8365MB_CFG_SGMII_LINK 0x0200 > +#define RTL8365MB_CFG_SGMII_SPD 0x0180 > +#define RTL8365MB_CFG_MAC8_SEL_SGMII 0x0040 > +#define RTL8365MB_CFG_INB_SEL 0x0038 > +#define RTL8365MB_CFG_SDS_MODE_18C 0x0007 > + > +#define RTL8365MB_SDS_INDACS_CMD 0x6600 > +#define RTL8365MB_SDS_INDACS_ADR 0x6601 > +#define RTL8365MB_SDS_INDACS_DATA 0x6602 > + > +#define RTL8365MB_MISC_CFG0 0x130c > +#define RTL8365MB_MISC_CFG0_DW8051_EN BIT(5) > + > +#define RTL8365MB_DW8051_RDY 0x1336 > +#define RTL8365MB_DW8051_RDY_IROM_MSB BIT(2) > +#define RTL8365MB_DW8051_RDY_ACS_IROM_EN BIT(1) Similar comments apply here, registers with _REG, masks with _MASK, no BIT(), more indentation, etc. Also, if you know a little bit about what the register(s) do, a comment is always appreciated. For example I had a look at this register map from Realtek: #define RTL8367C_REG_DW8051_RDY 0x1336 #define RTL8367C_VIAROM_WRITE_EN_OFFSET 9 #define RTL8367C_VIAROM_WRITE_EN_MASK 0x200 #define RTL8367C_SPIF_CK2_OFFSET 8 #define RTL8367C_SPIF_CK2_MASK 0x100 #define RTL8367C_RRCP_MDOE_OFFSET 7 #define RTL8367C_RRCP_MDOE_MASK 0x80 #define RTL8367C_DW8051_RATE_OFFSET 4 #define RTL8367C_DW8051_RATE_MASK 0x70 #define RTL8367C_IROM_MSB_OFFSET 2 #define RTL8367C_IROM_MSB_MASK 0xC #define RTL8367C_ACS_IROM_ENABLE_OFFSET 1 #define RTL8367C_ACS_IROM_ENABLE_MASK 0x2 #define RTL8367C_DW8051_READY_OFFSET 0 #define RTL8367C_DW8051_READY_MASK 0x1 Here it would appear that the register was originally called DW8051_READY because it contained a single READY bit at offset 0. Then Realtek added some other stuff completely unrelated to readiness. So you might want to call it something more generic like RTL8365MB_DW8051_REG. And a comment explaining what an 8051 is doing here, etc. > + > /* CPU port mask register - controls which ports are treated as CPU ports */ > #define RTL8365MB_CPU_PORT_MASK_REG 0x1219 > #define RTL8365MB_CPU_PORT_MASK_MASK 0x07FF > @@ -296,6 +322,8 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, > #define RTL8365MB_CFG0_MAX_LEN_REG 0x088C > #define RTL8365MB_CFG0_MAX_LEN_MASK 0x3FFF > > +#define RTL8365MB_BYPASS_LINE_RATE 0x03f7 Please document the type of value this can hold by specifying the relevant masks. Also you could point out that it is only used for TMII mode (Turbo MII?). > + > /* Port learning limit registers */ > #define RTL8365MB_LUT_PORT_LEARN_LIMIT_BASE 0x0A20 > #define RTL8365MB_LUT_PORT_LEARN_LIMIT_REG(_physport) \ > @@ -493,6 +521,39 @@ static const struct rtl8365mb_jam_tbl_entry rtl8365mb_init_jam_common[] = { > { 0x1D32, 0x0002 }, > }; > > +struct rtl8365mb_sds_init { > + unsigned int data; > + unsigned int addr; > +}; I would swap the order here to be consistent with struct rtl8365mb_jam_tbl_entry. Also make it u16 since the values are truly 16 bit. Actually, this is just a jam table entry, just for a different register map (via serdes indirect access). Maybe you can just follow the same model as the regular jam tables and add a const pointer to one of your arrays below based on the option in the rtl8365mb_detect function? > + > +static const struct rtl8365mb_sds_init redData[] = { What does redData stand for? Also we don't use camelCase in the driver. > + {0x04D7, 0x0480}, {0xF994, 0x0481}, {0x21A2, 0x0482}, {0x6960, 0x0483}, > + {0x9728, 0x0484}, {0x9D85, 0x0423}, {0xD810, 0x0424}, {0x83F2, 0x002E} > +}; > + > +static const struct rtl8365mb_sds_init redDataSB[] = { > + {0x04D7, 0x0480}, {0xF994, 0x0481}, {0x31A2, 0x0482}, {0x6960, 0x0483}, > + {0x9728, 0x0484}, {0x9D85, 0x0423}, {0xD810, 0x0424}, {0x83F2, 0x002E} > +}; > + > +static const struct rtl8365mb_sds_init redData1_5_6[] = { > + {0x82F1, 0x0500}, {0xF195, 0x0501}, {0x31A2, 0x0502}, {0x796C, 0x0503}, > + {0x9728, 0x0504}, {0x9D85, 0x0423}, {0xD810, 0x0424}, {0x0F80, 0x0001}, > + {0x83F2, 0x002E} > +}; > + > +static const struct rtl8365mb_sds_init redData8_9[] = { > + {0x82F1, 0x0500}, {0xF995, 0x0501}, {0x31A2, 0x0502}, {0x796C, 0x0503}, > + {0x9728, 0x0504}, {0x9D85, 0x0423}, {0xD810, 0x0424}, {0x0F80, 0x0001}, > + {0x83F2, 0x002E} > +}; > + > +static const struct rtl8365mb_sds_init redDataHB[] = { How about SB/HB? > + {0x82F0, 0x0500}, {0xF195, 0x0501}, {0x31A2, 0x0502}, {0x7960, 0x0503}, > + {0x9728, 0x0504}, {0x9D85, 0x0423}, {0xD810, 0x0424}, {0x0F80, 0x0001}, > + {0x83F2, 0x002E} > +}; > + > enum rtl8365mb_stp_state { > RTL8365MB_STP_STATE_DISABLED = 0, > RTL8365MB_STP_STATE_BLOCKING = 1, > @@ -801,6 +862,232 @@ rtl8365mb_get_tag_protocol(struct dsa_switch *ds, int port, > return DSA_TAG_PROTO_RTL8_4; > } > > +static int rtl8365mb_sds_indacs_writee(struct realtek_priv *priv, unsigned int addr, > + unsigned int data) Likewise I would make these unsigned ints into u16's, and why not just serdes_read/write? That it is indirect access (indacs) is not really relevant. > +{ > + int ret; > + > + ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_DATA, data); > + if (ret) > + return ret; > + > + ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_ADR, addr); Sorry to drone on about naming but since we use the abbreviation addr here, the register name ought to also be ADDR and not ADR. > + if (ret) > + return ret; > + > + return regmap_write(priv->map, RTL8365MB_SDS_INDACS_CMD, 0x00C0); Avoid magic values by populating the relevant macro fields: #define RTL8367C_REG_SDS_INDACS_CMD 0x6600 #define RTL8367C_SDS_CMD_BUSY_OFFSET 8 #define RTL8367C_SDS_CMD_BUSY_MASK 0x100 #define RTL8367C_SDS_CMD_OFFSET 7 #define RTL8367C_SDS_CMD_MASK 0x80 #define RTL8367C_SDS_RWOP_OFFSET 6 #define RTL8367C_SDS_RWOP_MASK 0x40 #define RTL8367C_SDS_INDEX_OFFSET 0 #define RTL8367C_SDS_INDEX_MASK 0x3F So 0x00C0 would correspond to RWOP=1 and CMD=1 I think. This is similar to the indirect access for the PHY registers. From rtl8365mb_phy_ocp_write(): /* Execute write operation */ val = FIELD_PREP(RTL8365MB_INDIRECT_ACCESS_CTRL_CMD_MASK, RTL8365MB_INDIRECT_ACCESS_CTRL_CMD_VALUE) | FIELD_PREP(RTL8365MB_INDIRECT_ACCESS_CTRL_RW_MASK, RTL8365MB_INDIRECT_ACCESS_CTRL_RW_WRITE); ret = regmap_write(priv->map_nolock, RTL8365MB_INDIRECT_ACCESS_CTRL_REG, val); So maybe you can take inspiration from there. > +} > + > +static int rtl8365mb_sds_indacs_read(struct realtek_priv *priv, unsigned int addr, > + unsigned int *data) > +{ By the way, we had issues with indirect access of the PHY registers colliding with ordinary register access on SMP systems and returning nonsense data. Could you please acquire the priv->map_lock directly in these serdes read/write functions and use the priv->map_nolock regmap instead? See the phy access functions for how it works or check the recent git history. > + int ret; > + > + ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_ADR, addr); > + if (ret) > + return ret; > + > + ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_CMD, 0x00C0); > + if (ret) > + return ret; > + > + return regmap_write(priv->map, RTL8365MB_SDS_INDACS_DATA, *data); Shouldn't this be a read? Did you test the relevant code that relies on serdes read? > +} > + > +static int rtl8365mb_ext_init_sgmii_fw(struct realtek_priv *priv) > +{ > + struct device *dev = priv->dev; This variable is not really needed > + const struct firmware *fw; > + int ret; > + int i; > + > + ret = request_firmware(&fw, "rtl_switch/rtl8367s-sgmii.bin", dev); Since the firmware is generic to the whole family, I would name this rtl8365mb-sgmii.bin, since we have tacitly agreed to refer to the family by rtl8365mb. Other ideas welcome but this looks a bit odd. Also, how about not hardcoding the FW name here but putting it in a macro? > + if (ret) { > + dev_err(dev, "failed to load firmware rtl_switch/rtl8367s-sgmii.bin, error: %i\n", > + ret); AFAIK request_firmware will print errors for you, so you can just return ret; here. > + return ret; > + } > + > + ret = regmap_update_bits(priv->map, RTL8365MB_CHIP_RESET_REG, > + RTL8365MB_CHIP_RESET_DW8051, > + FIELD_PREP(RTL8365MB_CHIP_RESET_DW8051, 1)); > + if (ret) > + goto release_fw; > + > + ret = regmap_update_bits(priv->map, RTL8365MB_MISC_CFG0, > + RTL8365MB_MISC_CFG0_DW8051_EN, > + FIELD_PREP(RTL8365MB_MISC_CFG0_DW8051_EN, 1)); > + if (ret) > + goto release_fw; > + > + ret = regmap_update_bits(priv->map, RTL8365MB_DW8051_RDY, > + RTL8365MB_DW8051_RDY_ACS_IROM_EN, > + FIELD_PREP(RTL8365MB_DW8051_RDY_ACS_IROM_EN, 1)); > + if (ret) > + goto release_fw; > + > + ret = regmap_update_bits(priv->map, RTL8365MB_DW8051_RDY, > + RTL8365MB_DW8051_RDY_IROM_MSB, > + FIELD_PREP(RTL8365MB_DW8051_RDY_IROM_MSB, 0)); Can you do this and the last write in one go? Is this timing sensitive somehow? > + if (ret) > + goto release_fw; > + > + for (i = 0; i < fw->size; i++) { Perhaps you ought to do a size check on the firmware file? > + ret = regmap_write(priv->map, 0xE000 + i, fw->data[i]); Avoid magic values please :) > + if (ret) > + goto release_fw; > + } > + > + ret = regmap_update_bits(priv->map, RTL8365MB_DW8051_RDY, > + RTL8365MB_DW8051_RDY_IROM_MSB, > + FIELD_PREP(RTL8365MB_DW8051_RDY_IROM_MSB, 0)); > + if (ret) > + goto release_fw; > + > + ret = regmap_update_bits(priv->map, RTL8365MB_DW8051_RDY, > + RTL8365MB_DW8051_RDY_ACS_IROM_EN, > + FIELD_PREP(RTL8365MB_DW8051_RDY_ACS_IROM_EN, 0)); > + if (ret) > + goto release_fw; > + > + ret = regmap_update_bits(priv->map, RTL8365MB_CHIP_RESET_REG, > + RTL8365MB_CHIP_RESET_DW8051, > + FIELD_PREP(RTL8365MB_CHIP_RESET_DW8051, 0)); It would be cool to document here in this function that you are actually loading firmware into the 8051. > + > +release_fw: > + release_firmware(fw); > + return ret; Newline before the return. > +} > + > +static int rtl8365mb_ext_init_sgmii(struct realtek_priv *priv, int port, phy_interface_t interface) > +{ > + struct rtl8365mb *mb; > + int interface_mode; > + int sds_mode; > + const struct rtl8365mb_sds_init *sds_init; > + size_t sds_init_len; Consider putting this in the mb struct like the jam table. > + int ext_int; > + int ret; > + int i; > + int val; > + int mask; Also remember to use reverse christmas tree order. > + > + mb = priv->chip_data; > + > + if (mb->chip_id != RTL8365MB_CHIP_ID_8365MB_VC) > + return -EINVAL; Why this check? > + > + ext_int = rtl8365mb_extint_port_map[port]; > + if (ext_int != 1) > + return -EINVAL; Also this needs some explanation > + > + if (interface == PHY_INTERFACE_MODE_SGMII) { > + sds_mode = FIELD_PREP(RTL8365MB_CFG_MAC8_SEL_SGMII, 1); > + interface_mode = RTL8365MB_EXT_PORT_MODE_SGMII; > + > + if (mb->chip_option == 0) { > + sds_init = redData; > + sds_init_len = ARRAY_SIZE(redData); > + } else { > + sds_init = redDataSB; > + sds_init_len = ARRAY_SIZE(redDataSB); > + } > + } else if (interface == PHY_INTERFACE_MODE_2500BASEX) { > + sds_mode = FIELD_PREP(RTL8365MB_CFG_MAC8_SEL_HSGMII, 1); > + interface_mode = RTL8365MB_EXT_PORT_MODE_HSGMII; > + > + if (mb->chip_option == 0) { > + switch (mb->chip_ver & 0x00F0) { Rather than magic masks like 0x00F0, why not just extend the _MASKs under the CHIP_VER_REG macro? Here is what the vendor driver says the subfields are: #define RTL8367C_REG_CHIP_VER 0x1301 #define RTL8367C_VERID_OFFSET 12 #define RTL8367C_VERID_MASK 0xF000 #define RTL8367C_MCID_OFFSET 8 #define RTL8367C_MCID_MASK 0xF00 #define RTL8367C_MODEL_ID_OFFSET 4 #define RTL8367C_MODEL_ID_MASK 0xF0 #define RTL8367C_AFE_VERSION_OFFSET 0 #define RTL8367C_AFE_VERSION_MASK 0x1 So here you are testing the model and picking a specific serdes init. (Sadly Realtek said they can't provide me with a mapping of chip ID/ver value <-> chip name, but perhaps one day we can make better sense of this...) > + case 0x0010: > + case 0x0050: > + case 0x0060: > + sds_init = redData1_5_6; > + sds_init_len = ARRAY_SIZE(redData1_5_6); > + break; > + case 0x0080: > + case 0x0090: > + sds_init = redData8_9; > + sds_init_len = ARRAY_SIZE(redData8_9); > + break; > + default: > + return -EINVAL; > + } > + } else { > + sds_init = redDataHB; > + sds_init_len = ARRAY_SIZE(redDataHB); > + } > + } else { > + return -EINVAL; > + } > + > + for (i = 0; i < sds_init_len; i++) { > + ret = rtl8365mb_sds_indacs_write(priv, sds_init[i].addr, sds_init[i].data); Try to stick to 80 columns unless it looks really ugly > + if (ret) > + return ret; > + } > + > + mask = RTL8365MB_CFG_MAC8_SEL_SGMII | RTL8365MB_CFG_MAC8_SEL_HSGMII; > + ret = regmap_update_bits(priv->map, > + RTL8365MB_SDS_MISC, > + mask, > + sds_mode); > + if (ret) > + return ret; Can the interface be changed at runtime with DSA? (I don't know.) If so then I think you need to do some cleaning up in the case where (H)SGMII was configured, and then e.g. RGMII is later configured. e.g. then this SDS_MISC bits SEL_SGMII or SEL_HSGMII should be zeroed out. > + > + mask = RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_MASK(ext_int); > + val = interface_mode << RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_OFFSET(ext_int); > + ret = regmap_update_bits(priv->map, > + RTL8365MB_DIGITAL_INTERFACE_SELECT_REG(ext_int), > + mask, > + val); > + if (ret) > + return ret; > + > + ret = regmap_write(priv->map, RTL8365MB_BYPASS_LINE_RATE, 0x0); > + if (ret) > + return ret; > + > + /* Serdes not reset */ > + ret = rtl8365mb_sds_indacs_write(priv, 0x0003, 0x7106); If this is random Realtek voodoo you do not understand, please say so rather than copying the cryptic comment from the driver. That way we know what we don't know. > + if (ret) > + return ret; > + > + return rtl8365mb_ext_init_sgmii_fw(priv); We don't really end functions like this in the driver unless they are (nearly) one-liners, but rather: ret = rtl8365mb_ext_init_sgmii_fw(priv); if(ret) return ret; return 0; Not a big deal though. > +} > + > +static int rtl8365mb_ext_sgmii_nway(struct realtek_priv *priv, bool state) > +{ > + u32 running; > + u32 regValue; Reverse christmas tree & camelCase again. s/regValue/val/ per convention. > + int ret; > + > + ret = regmap_read(priv->map, RTL8365MB_MISC_CFG0, &running); I think this line is a good indication that the register macro is poorly named. > + if (running & RTL8365MB_MISC_CFG0_DW8051_EN) { > + ret = regmap_update_bits(priv->map, RTL8365MB_MISC_CFG0, > + RTL8365MB_MISC_CFG0_DW8051_EN, > + FIELD_PREP(RTL8365MB_MISC_CFG0_DW8051_EN, 0)); > + if (ret) > + return ret; > + } Is there any harm in just setting the bit to 0 unconditionally? And can we even enter this function with the 8051 enabled? > + > + ret = rtl8365mb_sds_indacs_read(priv, 0x0002, ®Value); > + if (ret) > + return ret; > + > + if (state) > + regValue |= 0x0200; > + else > + regValue &= ~0x0200; > + regValue |= 0x0100; Any idea what serdes register 0x0002 and its value means? > + > + ret = rtl8365mb_sds_indacs_write(priv, 0x0002, regValue); > + if (ret) > + return ret; > + return regmap_update_bits(priv->map, RTL8365MB_MISC_CFG0, Again please don't end functions like this but rather with a return 0;. Also you are missing a newline. > + RTL8365MB_MISC_CFG0_DW8051_EN, > + FIELD_PREP(RTL8365MB_MISC_CFG0_DW8051_EN, 1)); > +} > + > static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port, > phy_interface_t interface) > { > @@ -886,6 +1173,7 @@ static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port, > } > > static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, > + phy_interface_t interface, > bool link, int speed, int duplex, > bool tx_pause, bool rx_pause) > { > @@ -911,7 +1199,7 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, > r_rx_pause = rx_pause ? 1 : 0; > r_tx_pause = tx_pause ? 1 : 0; > > - if (speed == SPEED_1000) { > + if (speed == SPEED_1000 || speed == SPEED_2500) { > r_speed = RTL8365MB_PORT_SPEED_1000M; This looks odd, maybe a comment is in order? > } else if (speed == SPEED_100) { > r_speed = RTL8365MB_PORT_SPEED_100M; > @@ -941,6 +1229,25 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, > r_duplex = 0; > } > > + if (interface == PHY_INTERFACE_MODE_SGMII || > + interface == PHY_INTERFACE_MODE_2500BASEX) { > + val = FIELD_PREP(RTL8365MB_CFG_SGMII_FDUP, r_duplex) | > + FIELD_PREP(RTL8365MB_CFG_SGMII_SPD, r_speed) | > + FIELD_PREP(RTL8365MB_CFG_SGMII_LINK, r_link) | > + FIELD_PREP(RTL8365MB_CFG_SGMII_TXFC, r_tx_pause) | > + FIELD_PREP(RTL8365MB_CFG_SGMII_RXFC, r_rx_pause); > + ret = regmap_update_bits(priv->map, > + RTL8365MB_SDS_MISC, > + RTL8365MB_CFG_SGMII_FDUP | > + RTL8365MB_CFG_SGMII_SPD | > + RTL8365MB_CFG_SGMII_LINK | > + RTL8365MB_CFG_SGMII_TXFC | > + RTL8365MB_CFG_SGMII_RXFC, > + val); > + if (ret) > + return ret; > + } > + > val = FIELD_PREP(RTL8365MB_DIGITAL_INTERFACE_FORCE_EN_MASK, 1) | > FIELD_PREP(RTL8365MB_DIGITAL_INTERFACE_FORCE_TXPAUSE_MASK, > r_tx_pause) | > @@ -972,10 +1279,15 @@ static bool rtl8365mb_phy_mode_supported(struct dsa_switch *ds, int port, > interface == PHY_INTERFACE_MODE_GMII)) > /* Internal PHY */ > return true; > - else if ((ext_int >= 1) && > - phy_interface_mode_is_rgmii(interface)) > + else if ((ext_int == 1) && I'm again curious why you limit these changes to EXT port 1. > + (phy_interface_mode_is_rgmii(interface) || > + interface == PHY_INTERFACE_MODE_SGMII || > + interface == PHY_INTERFACE_MODE_2500BASEX)) > /* Extension MAC */ > return true; > + else if ((ext_int >= 2) && > + phy_interface_mode_is_rgmii(interface)) > + return true; > > return false; > } > @@ -983,14 +1295,25 @@ static bool rtl8365mb_phy_mode_supported(struct dsa_switch *ds, int port, > static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port, > struct phylink_config *config) > { > - if (dsa_is_user_port(ds, port)) > + int ext_int = rtl8365mb_extint_port_map[port]; > + > + config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | > + MAC_10 | MAC_100 | MAC_1000FD; > + > + if (dsa_is_user_port(ds, port)) { > __set_bit(PHY_INTERFACE_MODE_INTERNAL, > config->supported_interfaces); > - else if (dsa_is_cpu_port(ds, port)) > + } else if (dsa_is_cpu_port(ds, port)) { > + if (ext_int == 1) { > + __set_bit(PHY_INTERFACE_MODE_SGMII, > + config->supported_interfaces); > + __set_bit(PHY_INTERFACE_MODE_2500BASEX, > + config->supported_interfaces); > + config->mac_capabilities |= MAC_2500FD; > + } I am not satisfied with this check because not all switches in this family support (H)SGMII. Also testing the ext_int ID is not particularly elucidating to the reader. > phy_interface_set_rgmii(config->supported_interfaces); Hmm, I guess this is actually not true for your RTL8367S? Isn't that (H)SGMII-only on its first extension port? I guess this was a bug already, though. > + } > > - config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | > - MAC_10 | MAC_100 | MAC_1000FD; > } > > static void rtl8365mb_phylink_mac_config(struct dsa_switch *ds, int port, > @@ -1020,6 +1343,10 @@ static void rtl8365mb_phylink_mac_config(struct dsa_switch *ds, int port, > "failed to configure RGMII mode on port %d: %d\n", > port, ret); > return; > + } else if (state->interface == PHY_INTERFACE_MODE_SGMII || > + state->interface == PHY_INTERFACE_MODE_2500BASEX) { > + rtl8365mb_ext_init_sgmii(priv, port, state->interface); > + rtl8365mb_ext_sgmii_nway(priv, false); > } > > /* TODO: Implement MII and RMII modes, which the RTL8365MB-VC also > @@ -1040,8 +1367,11 @@ static void rtl8365mb_phylink_mac_link_down(struct dsa_switch *ds, int port, > p = &mb->ports[port]; > cancel_delayed_work_sync(&p->mib_work); > > - if (phy_interface_mode_is_rgmii(interface)) { > - ret = rtl8365mb_ext_config_forcemode(priv, port, false, 0, 0, > + if (phy_interface_mode_is_rgmii(interface) || > + interface == PHY_INTERFACE_MODE_SGMII || > + interface == PHY_INTERFACE_MODE_2500BASEX) { > + ret = rtl8365mb_ext_config_forcemode(priv, port, interface, > + false, 0, 0, > false, false); > if (ret) > dev_err(priv->dev, > @@ -1068,8 +1398,11 @@ static void rtl8365mb_phylink_mac_link_up(struct dsa_switch *ds, int port, > p = &mb->ports[port]; > schedule_delayed_work(&p->mib_work, 0); > > - if (phy_interface_mode_is_rgmii(interface)) { > - ret = rtl8365mb_ext_config_forcemode(priv, port, true, speed, > + if (phy_interface_mode_is_rgmii(interface) || > + interface == PHY_INTERFACE_MODE_SGMII || > + interface == PHY_INTERFACE_MODE_2500BASEX) { > + ret = rtl8365mb_ext_config_forcemode(priv, port, interface, > + true, speed, > duplex, tx_pause, > rx_pause); > if (ret) > @@ -2156,6 +2489,7 @@ const struct realtek_variant rtl8365mb_variant = { > }; > EXPORT_SYMBOL_GPL(rtl8365mb_variant); > > +MODULE_FIRMWARE("rtl_switch/rtl8367s-sgmii.bin"); Do you happen to know the kernel workflow for merging patches which depend on firmware? Should the firmware land before or after this lands in net-next and/or mainline? Kind regards, Alvin > MODULE_AUTHOR("Alvin ┼áipraga <alsi@bang-olufsen.dk>"); > MODULE_DESCRIPTION("Driver for RTL8365MB-VC ethernet switch"); > MODULE_LICENSE("GPL"); > -- > 2.30.2 >
On Tue, May 10, 2022 at 08:29:10PM +0300, Vladimir Oltean wrote: > On Mon, May 09, 2022 at 12:48:48AM +0200, Hauke Mehrtens wrote: > > @@ -983,14 +1295,25 @@ static bool rtl8365mb_phy_mode_supported(struct dsa_switch *ds, int port, > > static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port, > > struct phylink_config *config) > > { > > - if (dsa_is_user_port(ds, port)) > > + int ext_int = rtl8365mb_extint_port_map[port]; > > + > > + config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | > > + MAC_10 | MAC_100 | MAC_1000FD; > > + > > + if (dsa_is_user_port(ds, port)) { > > __set_bit(PHY_INTERFACE_MODE_INTERNAL, > > config->supported_interfaces); > > - else if (dsa_is_cpu_port(ds, port)) > > + } else if (dsa_is_cpu_port(ds, port)) { > > What does the quality of being a user port or a CPU port have to do with > which interfaces are supported? Right, I think this function was actually broken already in a few ways. The switch will have ports with integrated PHYs, and ports with extension interfaces like RGMII or SGMII etc. But which of those ports one uses as a CPU port, user port, or (one day) DSA port, is of no concern to the switch. The supported interface of a given port is a static property and simply a function of the port number and switch model. All switch models in the family have between 1 and 2 ports with an extension interface. Luiz introduced this map: /* valid for all 6-port or less variants */ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, -1, -1}; ... which I think is actually what we ought to test on. It can be improved, but currently it is correct for all supported models. So something like this would be correct: static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port, struct phylink_config *config) { int ext_int = rtl8365mb_extint_port_map[port]; if (ext_int == -1) { /* integrated PHY, set PHY_INTERFACE_MODE_INTERNAL etc. */ } else { /* extension interface available, but here one should really * check the model based on the chip ID/version, because it * varies a lot */ if (model == RTL8365MB && ext_int == 1) /* RGMII */ else if (model == RTL8367S && ext_int == 1) /* SGMII / HSGMII */ else if (model == RTL8367S && ext_int == 2) /* RGMII */ /* etc */ } /* ... */ } There are of course various ways to do this. Hauke, do you follow what I mean? Would you like me to prepare a patch for the current supported models/interfaces and then you can rebase your changes on top of that to indicate support for (H)SGMII? Or do you want to give it a try yourself? Kind regards, Alvin > > > + if (ext_int == 1) { > > + __set_bit(PHY_INTERFACE_MODE_SGMII, > > + config->supported_interfaces); > > + __set_bit(PHY_INTERFACE_MODE_2500BASEX, > > + config->supported_interfaces); > > + config->mac_capabilities |= MAC_2500FD; > > + } > > phy_interface_set_rgmii(config->supported_interfaces); > > + } > > > > - config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | > > - MAC_10 | MAC_100 | MAC_1000FD; > > } > > > > static void rtl8365mb_phylink_mac_config(struct dsa_switch *ds, int port,
> On Tue, May 10, 2022 at 08:29:10PM +0300, Vladimir Oltean wrote: > > On Mon, May 09, 2022 at 12:48:48AM +0200, Hauke Mehrtens wrote: > > > @@ -983,14 +1295,25 @@ static bool rtl8365mb_phy_mode_supported(struct dsa_switch *ds, int port, > > > static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port, > > > struct phylink_config *config) > > > { > > > - if (dsa_is_user_port(ds, port)) > > > + int ext_int = rtl8365mb_extint_port_map[port]; > > > + > > > + config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | > > > + MAC_10 | MAC_100 | MAC_1000FD; > > > + > > > + if (dsa_is_user_port(ds, port)) { > > > __set_bit(PHY_INTERFACE_MODE_INTERNAL, > > > config->supported_interfaces); > > > - else if (dsa_is_cpu_port(ds, port)) > > > + } else if (dsa_is_cpu_port(ds, port)) { > > > > What does the quality of being a user port or a CPU port have to do with > > which interfaces are supported? > > Right, I think this function was actually broken already in a few ways. The > switch will have ports with integrated PHYs, and ports with extension interfaces > like RGMII or SGMII etc. But which of those ports one uses as a CPU port, user > port, or (one day) DSA port, is of no concern to the switch. The supported > interface of a given port is a static property and simply a function of the port > number and switch model. All switch models in the family have between 1 and 2 > ports with an extension interface. > > Luiz introduced this map: > > /* valid for all 6-port or less variants */ > static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, -1, -1}; > > ... which I think is actually what we ought to test on. It can be improved, but > currently it is correct for all supported models. I was playing some time ago with a more detailed description of ports/supported modes I manually extracted from Realtek docs: https://github.com/luizluca/linux/commit/d64201989803274bf6ba8bb784e2bf500114cff5 It might have more details than the driver really needs. Although there are a lot of new lines with duplicated parts, I don't believe that this family will grow too much. I can get this upstream-ready if it looks the way to go. Another approach would be to check if the switch can describe its capabilities programmatically. The Realtek rtl8367c code has lots and comes and goes, reads mysterious registers, but maybe deep down there might be a way the switch can tell how many ports it has, which one are really in use and what modes each port does support. Regards, Luiz
diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c index f9b690251155..5504a34fffeb 100644 --- a/drivers/net/dsa/realtek/rtl8365mb.c +++ b/drivers/net/dsa/realtek/rtl8365mb.c @@ -3,6 +3,7 @@ * * Copyright (C) 2021 Alvin Šipraga <alsi@bang-olufsen.dk> * Copyright (C) 2021 Michael Rasmussen <mir@bang-olufsen.dk> + * Copyright (C) 2022 Hauke Mehrtens <hauke@hauke-m.de> * * The RTL8365MB-VC is a 4+1 port 10/100/1000M switch controller. It includes 4 * integrated PHYs for the user facing ports, and an extension interface which @@ -98,6 +99,7 @@ #include <linux/of_irq.h> #include <linux/regmap.h> #include <linux/if_bridge.h> +#include <linux/firmware.h> #include "realtek.h" @@ -135,6 +137,7 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, /* Chip reset register */ #define RTL8365MB_CHIP_RESET_REG 0x1322 +#define RTL8365MB_CHIP_RESET_DW8051 BIT(4) #define RTL8365MB_CHIP_RESET_SW_MASK 0x0002 #define RTL8365MB_CHIP_RESET_HW_MASK 0x0001 @@ -278,6 +281,29 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, #define RTL8365MB_DIGITAL_INTERFACE_FORCE_DUPLEX_MASK 0x0004 #define RTL8365MB_DIGITAL_INTERFACE_FORCE_SPEED_MASK 0x0003 +#define RTL8365MB_SDS_MISC 0x1d11 +#define RTL8365MB_CFG_SGMII_RXFC 0x4000 +#define RTL8365MB_CFG_SGMII_TXFC 0x2000 +#define RTL8365MB_INB_ARB 0x1000 +#define RTL8365MB_CFG_MAC8_SEL_HSGMII 0x0800 +#define RTL8365MB_CFG_SGMII_FDUP 0x0400 +#define RTL8365MB_CFG_SGMII_LINK 0x0200 +#define RTL8365MB_CFG_SGMII_SPD 0x0180 +#define RTL8365MB_CFG_MAC8_SEL_SGMII 0x0040 +#define RTL8365MB_CFG_INB_SEL 0x0038 +#define RTL8365MB_CFG_SDS_MODE_18C 0x0007 + +#define RTL8365MB_SDS_INDACS_CMD 0x6600 +#define RTL8365MB_SDS_INDACS_ADR 0x6601 +#define RTL8365MB_SDS_INDACS_DATA 0x6602 + +#define RTL8365MB_MISC_CFG0 0x130c +#define RTL8365MB_MISC_CFG0_DW8051_EN BIT(5) + +#define RTL8365MB_DW8051_RDY 0x1336 +#define RTL8365MB_DW8051_RDY_IROM_MSB BIT(2) +#define RTL8365MB_DW8051_RDY_ACS_IROM_EN BIT(1) + /* CPU port mask register - controls which ports are treated as CPU ports */ #define RTL8365MB_CPU_PORT_MASK_REG 0x1219 #define RTL8365MB_CPU_PORT_MASK_MASK 0x07FF @@ -296,6 +322,8 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, #define RTL8365MB_CFG0_MAX_LEN_REG 0x088C #define RTL8365MB_CFG0_MAX_LEN_MASK 0x3FFF +#define RTL8365MB_BYPASS_LINE_RATE 0x03f7 + /* Port learning limit registers */ #define RTL8365MB_LUT_PORT_LEARN_LIMIT_BASE 0x0A20 #define RTL8365MB_LUT_PORT_LEARN_LIMIT_REG(_physport) \ @@ -493,6 +521,39 @@ static const struct rtl8365mb_jam_tbl_entry rtl8365mb_init_jam_common[] = { { 0x1D32, 0x0002 }, }; +struct rtl8365mb_sds_init { + unsigned int data; + unsigned int addr; +}; + +static const struct rtl8365mb_sds_init redData[] = { + {0x04D7, 0x0480}, {0xF994, 0x0481}, {0x21A2, 0x0482}, {0x6960, 0x0483}, + {0x9728, 0x0484}, {0x9D85, 0x0423}, {0xD810, 0x0424}, {0x83F2, 0x002E} +}; + +static const struct rtl8365mb_sds_init redDataSB[] = { + {0x04D7, 0x0480}, {0xF994, 0x0481}, {0x31A2, 0x0482}, {0x6960, 0x0483}, + {0x9728, 0x0484}, {0x9D85, 0x0423}, {0xD810, 0x0424}, {0x83F2, 0x002E} +}; + +static const struct rtl8365mb_sds_init redData1_5_6[] = { + {0x82F1, 0x0500}, {0xF195, 0x0501}, {0x31A2, 0x0502}, {0x796C, 0x0503}, + {0x9728, 0x0504}, {0x9D85, 0x0423}, {0xD810, 0x0424}, {0x0F80, 0x0001}, + {0x83F2, 0x002E} +}; + +static const struct rtl8365mb_sds_init redData8_9[] = { + {0x82F1, 0x0500}, {0xF995, 0x0501}, {0x31A2, 0x0502}, {0x796C, 0x0503}, + {0x9728, 0x0504}, {0x9D85, 0x0423}, {0xD810, 0x0424}, {0x0F80, 0x0001}, + {0x83F2, 0x002E} +}; + +static const struct rtl8365mb_sds_init redDataHB[] = { + {0x82F0, 0x0500}, {0xF195, 0x0501}, {0x31A2, 0x0502}, {0x7960, 0x0503}, + {0x9728, 0x0504}, {0x9D85, 0x0423}, {0xD810, 0x0424}, {0x0F80, 0x0001}, + {0x83F2, 0x002E} +}; + enum rtl8365mb_stp_state { RTL8365MB_STP_STATE_DISABLED = 0, RTL8365MB_STP_STATE_BLOCKING = 1, @@ -801,6 +862,232 @@ rtl8365mb_get_tag_protocol(struct dsa_switch *ds, int port, return DSA_TAG_PROTO_RTL8_4; } +static int rtl8365mb_sds_indacs_write(struct realtek_priv *priv, unsigned int addr, + unsigned int data) +{ + int ret; + + ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_DATA, data); + if (ret) + return ret; + + ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_ADR, addr); + if (ret) + return ret; + + return regmap_write(priv->map, RTL8365MB_SDS_INDACS_CMD, 0x00C0); +} + +static int rtl8365mb_sds_indacs_read(struct realtek_priv *priv, unsigned int addr, + unsigned int *data) +{ + int ret; + + ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_ADR, addr); + if (ret) + return ret; + + ret = regmap_write(priv->map, RTL8365MB_SDS_INDACS_CMD, 0x00C0); + if (ret) + return ret; + + return regmap_write(priv->map, RTL8365MB_SDS_INDACS_DATA, *data); +} + +static int rtl8365mb_ext_init_sgmii_fw(struct realtek_priv *priv) +{ + struct device *dev = priv->dev; + const struct firmware *fw; + int ret; + int i; + + ret = request_firmware(&fw, "rtl_switch/rtl8367s-sgmii.bin", dev); + if (ret) { + dev_err(dev, "failed to load firmware rtl_switch/rtl8367s-sgmii.bin, error: %i\n", + ret); + return ret; + } + + ret = regmap_update_bits(priv->map, RTL8365MB_CHIP_RESET_REG, + RTL8365MB_CHIP_RESET_DW8051, + FIELD_PREP(RTL8365MB_CHIP_RESET_DW8051, 1)); + if (ret) + goto release_fw; + + ret = regmap_update_bits(priv->map, RTL8365MB_MISC_CFG0, + RTL8365MB_MISC_CFG0_DW8051_EN, + FIELD_PREP(RTL8365MB_MISC_CFG0_DW8051_EN, 1)); + if (ret) + goto release_fw; + + ret = regmap_update_bits(priv->map, RTL8365MB_DW8051_RDY, + RTL8365MB_DW8051_RDY_ACS_IROM_EN, + FIELD_PREP(RTL8365MB_DW8051_RDY_ACS_IROM_EN, 1)); + if (ret) + goto release_fw; + + ret = regmap_update_bits(priv->map, RTL8365MB_DW8051_RDY, + RTL8365MB_DW8051_RDY_IROM_MSB, + FIELD_PREP(RTL8365MB_DW8051_RDY_IROM_MSB, 0)); + if (ret) + goto release_fw; + + for (i = 0; i < fw->size; i++) { + ret = regmap_write(priv->map, 0xE000 + i, fw->data[i]); + if (ret) + goto release_fw; + } + + ret = regmap_update_bits(priv->map, RTL8365MB_DW8051_RDY, + RTL8365MB_DW8051_RDY_IROM_MSB, + FIELD_PREP(RTL8365MB_DW8051_RDY_IROM_MSB, 0)); + if (ret) + goto release_fw; + + ret = regmap_update_bits(priv->map, RTL8365MB_DW8051_RDY, + RTL8365MB_DW8051_RDY_ACS_IROM_EN, + FIELD_PREP(RTL8365MB_DW8051_RDY_ACS_IROM_EN, 0)); + if (ret) + goto release_fw; + + ret = regmap_update_bits(priv->map, RTL8365MB_CHIP_RESET_REG, + RTL8365MB_CHIP_RESET_DW8051, + FIELD_PREP(RTL8365MB_CHIP_RESET_DW8051, 0)); + +release_fw: + release_firmware(fw); + return ret; +} + +static int rtl8365mb_ext_init_sgmii(struct realtek_priv *priv, int port, phy_interface_t interface) +{ + struct rtl8365mb *mb; + int interface_mode; + int sds_mode; + const struct rtl8365mb_sds_init *sds_init; + size_t sds_init_len; + int ext_int; + int ret; + int i; + int val; + int mask; + + mb = priv->chip_data; + + if (mb->chip_id != RTL8365MB_CHIP_ID_8365MB_VC) + return -EINVAL; + + ext_int = rtl8365mb_extint_port_map[port]; + if (ext_int != 1) + return -EINVAL; + + if (interface == PHY_INTERFACE_MODE_SGMII) { + sds_mode = FIELD_PREP(RTL8365MB_CFG_MAC8_SEL_SGMII, 1); + interface_mode = RTL8365MB_EXT_PORT_MODE_SGMII; + + if (mb->chip_option == 0) { + sds_init = redData; + sds_init_len = ARRAY_SIZE(redData); + } else { + sds_init = redDataSB; + sds_init_len = ARRAY_SIZE(redDataSB); + } + } else if (interface == PHY_INTERFACE_MODE_2500BASEX) { + sds_mode = FIELD_PREP(RTL8365MB_CFG_MAC8_SEL_HSGMII, 1); + interface_mode = RTL8365MB_EXT_PORT_MODE_HSGMII; + + if (mb->chip_option == 0) { + switch (mb->chip_ver & 0x00F0) { + case 0x0010: + case 0x0050: + case 0x0060: + sds_init = redData1_5_6; + sds_init_len = ARRAY_SIZE(redData1_5_6); + break; + case 0x0080: + case 0x0090: + sds_init = redData8_9; + sds_init_len = ARRAY_SIZE(redData8_9); + break; + default: + return -EINVAL; + } + } else { + sds_init = redDataHB; + sds_init_len = ARRAY_SIZE(redDataHB); + } + } else { + return -EINVAL; + } + + for (i = 0; i < sds_init_len; i++) { + ret = rtl8365mb_sds_indacs_write(priv, sds_init[i].addr, sds_init[i].data); + if (ret) + return ret; + } + + mask = RTL8365MB_CFG_MAC8_SEL_SGMII | RTL8365MB_CFG_MAC8_SEL_HSGMII; + ret = regmap_update_bits(priv->map, + RTL8365MB_SDS_MISC, + mask, + sds_mode); + if (ret) + return ret; + + mask = RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_MASK(ext_int); + val = interface_mode << RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_OFFSET(ext_int); + ret = regmap_update_bits(priv->map, + RTL8365MB_DIGITAL_INTERFACE_SELECT_REG(ext_int), + mask, + val); + if (ret) + return ret; + + ret = regmap_write(priv->map, RTL8365MB_BYPASS_LINE_RATE, 0x0); + if (ret) + return ret; + + /* Serdes not reset */ + ret = rtl8365mb_sds_indacs_write(priv, 0x0003, 0x7106); + if (ret) + return ret; + + return rtl8365mb_ext_init_sgmii_fw(priv); +} + +static int rtl8365mb_ext_sgmii_nway(struct realtek_priv *priv, bool state) +{ + u32 running; + u32 regValue; + int ret; + + ret = regmap_read(priv->map, RTL8365MB_MISC_CFG0, &running); + if (running & RTL8365MB_MISC_CFG0_DW8051_EN) { + ret = regmap_update_bits(priv->map, RTL8365MB_MISC_CFG0, + RTL8365MB_MISC_CFG0_DW8051_EN, + FIELD_PREP(RTL8365MB_MISC_CFG0_DW8051_EN, 0)); + if (ret) + return ret; + } + + ret = rtl8365mb_sds_indacs_read(priv, 0x0002, ®Value); + if (ret) + return ret; + + if (state) + regValue |= 0x0200; + else + regValue &= ~0x0200; + regValue |= 0x0100; + + ret = rtl8365mb_sds_indacs_write(priv, 0x0002, regValue); + if (ret) + return ret; + return regmap_update_bits(priv->map, RTL8365MB_MISC_CFG0, + RTL8365MB_MISC_CFG0_DW8051_EN, + FIELD_PREP(RTL8365MB_MISC_CFG0_DW8051_EN, 1)); +} + static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port, phy_interface_t interface) { @@ -886,6 +1173,7 @@ static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port, } static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, + phy_interface_t interface, bool link, int speed, int duplex, bool tx_pause, bool rx_pause) { @@ -911,7 +1199,7 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, r_rx_pause = rx_pause ? 1 : 0; r_tx_pause = tx_pause ? 1 : 0; - if (speed == SPEED_1000) { + if (speed == SPEED_1000 || speed == SPEED_2500) { r_speed = RTL8365MB_PORT_SPEED_1000M; } else if (speed == SPEED_100) { r_speed = RTL8365MB_PORT_SPEED_100M; @@ -941,6 +1229,25 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, r_duplex = 0; } + if (interface == PHY_INTERFACE_MODE_SGMII || + interface == PHY_INTERFACE_MODE_2500BASEX) { + val = FIELD_PREP(RTL8365MB_CFG_SGMII_FDUP, r_duplex) | + FIELD_PREP(RTL8365MB_CFG_SGMII_SPD, r_speed) | + FIELD_PREP(RTL8365MB_CFG_SGMII_LINK, r_link) | + FIELD_PREP(RTL8365MB_CFG_SGMII_TXFC, r_tx_pause) | + FIELD_PREP(RTL8365MB_CFG_SGMII_RXFC, r_rx_pause); + ret = regmap_update_bits(priv->map, + RTL8365MB_SDS_MISC, + RTL8365MB_CFG_SGMII_FDUP | + RTL8365MB_CFG_SGMII_SPD | + RTL8365MB_CFG_SGMII_LINK | + RTL8365MB_CFG_SGMII_TXFC | + RTL8365MB_CFG_SGMII_RXFC, + val); + if (ret) + return ret; + } + val = FIELD_PREP(RTL8365MB_DIGITAL_INTERFACE_FORCE_EN_MASK, 1) | FIELD_PREP(RTL8365MB_DIGITAL_INTERFACE_FORCE_TXPAUSE_MASK, r_tx_pause) | @@ -972,10 +1279,15 @@ static bool rtl8365mb_phy_mode_supported(struct dsa_switch *ds, int port, interface == PHY_INTERFACE_MODE_GMII)) /* Internal PHY */ return true; - else if ((ext_int >= 1) && - phy_interface_mode_is_rgmii(interface)) + else if ((ext_int == 1) && + (phy_interface_mode_is_rgmii(interface) || + interface == PHY_INTERFACE_MODE_SGMII || + interface == PHY_INTERFACE_MODE_2500BASEX)) /* Extension MAC */ return true; + else if ((ext_int >= 2) && + phy_interface_mode_is_rgmii(interface)) + return true; return false; } @@ -983,14 +1295,25 @@ static bool rtl8365mb_phy_mode_supported(struct dsa_switch *ds, int port, static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port, struct phylink_config *config) { - if (dsa_is_user_port(ds, port)) + int ext_int = rtl8365mb_extint_port_map[port]; + + config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | + MAC_10 | MAC_100 | MAC_1000FD; + + if (dsa_is_user_port(ds, port)) { __set_bit(PHY_INTERFACE_MODE_INTERNAL, config->supported_interfaces); - else if (dsa_is_cpu_port(ds, port)) + } else if (dsa_is_cpu_port(ds, port)) { + if (ext_int == 1) { + __set_bit(PHY_INTERFACE_MODE_SGMII, + config->supported_interfaces); + __set_bit(PHY_INTERFACE_MODE_2500BASEX, + config->supported_interfaces); + config->mac_capabilities |= MAC_2500FD; + } phy_interface_set_rgmii(config->supported_interfaces); + } - config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | - MAC_10 | MAC_100 | MAC_1000FD; } static void rtl8365mb_phylink_mac_config(struct dsa_switch *ds, int port, @@ -1020,6 +1343,10 @@ static void rtl8365mb_phylink_mac_config(struct dsa_switch *ds, int port, "failed to configure RGMII mode on port %d: %d\n", port, ret); return; + } else if (state->interface == PHY_INTERFACE_MODE_SGMII || + state->interface == PHY_INTERFACE_MODE_2500BASEX) { + rtl8365mb_ext_init_sgmii(priv, port, state->interface); + rtl8365mb_ext_sgmii_nway(priv, false); } /* TODO: Implement MII and RMII modes, which the RTL8365MB-VC also @@ -1040,8 +1367,11 @@ static void rtl8365mb_phylink_mac_link_down(struct dsa_switch *ds, int port, p = &mb->ports[port]; cancel_delayed_work_sync(&p->mib_work); - if (phy_interface_mode_is_rgmii(interface)) { - ret = rtl8365mb_ext_config_forcemode(priv, port, false, 0, 0, + if (phy_interface_mode_is_rgmii(interface) || + interface == PHY_INTERFACE_MODE_SGMII || + interface == PHY_INTERFACE_MODE_2500BASEX) { + ret = rtl8365mb_ext_config_forcemode(priv, port, interface, + false, 0, 0, false, false); if (ret) dev_err(priv->dev, @@ -1068,8 +1398,11 @@ static void rtl8365mb_phylink_mac_link_up(struct dsa_switch *ds, int port, p = &mb->ports[port]; schedule_delayed_work(&p->mib_work, 0); - if (phy_interface_mode_is_rgmii(interface)) { - ret = rtl8365mb_ext_config_forcemode(priv, port, true, speed, + if (phy_interface_mode_is_rgmii(interface) || + interface == PHY_INTERFACE_MODE_SGMII || + interface == PHY_INTERFACE_MODE_2500BASEX) { + ret = rtl8365mb_ext_config_forcemode(priv, port, interface, + true, speed, duplex, tx_pause, rx_pause); if (ret) @@ -2156,6 +2489,7 @@ const struct realtek_variant rtl8365mb_variant = { }; EXPORT_SYMBOL_GPL(rtl8365mb_variant); +MODULE_FIRMWARE("rtl_switch/rtl8367s-sgmii.bin"); MODULE_AUTHOR("Alvin Šipraga <alsi@bang-olufsen.dk>"); MODULE_DESCRIPTION("Driver for RTL8365MB-VC ethernet switch"); MODULE_LICENSE("GPL");
This adds support for SGMII and HSGMII on RTL8367S switches. HSGMII is configured using the 2500BASEX mode. This is baed on the rtl8367c driver found in OpenWrt. For (H)SGMII mode we have to load a firmware into some memory which gets executed on the integrated 8051. The firmware binary was added as a array into the driver with a GPL license notice on top. This was tested on RTL8367S (ver=0x00a0, opt=0x0001). Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> --- drivers/net/dsa/realtek/rtl8365mb.c | 356 +++++++++++++++++++++++++++- 1 file changed, 345 insertions(+), 11 deletions(-)