tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Merge branch 'net-stmmac-remove-mac_interface'

Russell King says:

====================
net: stmmac: remove mac_interface

The dwmac core supports a range of interfaces, but when it comes to
SerDes interfaces, the core itself does not include the SerDes block.
Consequently, it has to provide an interface suitable to interface such
a block to, and uses TBI for this.

The driver also uses "PCS" for RGMII, even though the dwmac PCS block
is not present for this interface type - it was a convenice for the
code structure as RGMII includes inband signalling of the PHY state,
much like Cisco SGMII does at a high level.

As such, the code refers to RGMII and SGMII modes for the PCS, and
there used to be STMMAC_PCS_TBI and STMMAC_PCS_RTBI constants as well
but these were never set, although they were used in the code.

The selection of the PCS mode was from mac_interface. Thus, it seems
that the original intention was for mac_interface to describe the
interface mode used within the dwmac core, and phy_interface to
describe the external world-accessible interface (e.g. which would
connect to a PHY or SFP cage.)

It appears that many glue drivers misinterpret these. A good exmple
is socfpga. This supports SGMII and 1000BASE-X, but does not include
the dwmac PCS, relying on the Lynx PCS instead. However, it makes use
of mac_interface to configure the dwmac core to its GMII/MII mode.

So, when operating in either of these modes, the dwmac is configured
for GMII mode to communicate with the Lynx PCS which then provides
the SGMII or 1000BASE-X interface mode to the external world.

Given that phy_interface is the external world interface, and
mac_interface is the dwmac core interface, selecting the interface
mode based on mac_interface being 1000BASEX makes no sense.

Moreover, mac_interface is initialised by the stmmac core code. If
the "mac-mode" property is set in DT, this will be used. Otherwise,
it will reflect the "phy-mode" property - meaning that it defaults
to phy_interface. As no in-kernel DT makes reference to a "mac-mode"
property, we can assume that for all in-kernel platforms, these two
interface variables are the same. The exception are those platform
glues which I reviwed and suggested they use phy_interface, setting
mac_interface to PHY_INTERFACE_MODE_NA.

The conclusion to all of this is that mac_interface serves no useful
purpose, and causes confusion as the platform glue code doesn't seem
to know which one should be used.

Thus, let's get rid of mac_interface, which makes all this code more
understandable. It also helps to untangle some of the questions such
as:
- should this be using the interface passed from phylink
- should we set the range of phylink supported interfaces to be
more than one
- when we get phylink PCS support for the dwmac PCS, should we be
selecting it based on mac_interface or phy_interface, and how
should we populate the PCS' supported_interface bitmap.

Having converted socfpga to use phy_interface, this turns out to
feel like "the right way" to do this - convert the external world
"phy_interface" to whatever phy_intf_sel value that the dwmac core
needs to achieve the connection to whatever hardware blocks are
integrated inside the SoC to achieve the requested external world
interface.

As an illustration why - consider that in the case of socfpga, it
_could_ have been implemented such that the dwmac PCS was used for
SGMII, and the Lynx PCS for 1000BASE-X, or vice versa. Only the
platform glue would know which it is.

I will also note that several cores provide their currently configured
interface mode via the ACTPHYIF field of Hardware Feature 0, and thus
can be read back in the platform-independent parts of the driver to
decide whether the internal PCS or the RGMII (or actually SMII) "PCS"
should be used.

This is hot-off-the-press, and has only been build tested. As I have
none of these platforms, this series has not been run-tested, so
please test on your hardware.
====================

Link: https://patch.msgid.link/aMrPpc8oRxqGtVPJ@shell.armlinux.org.uk
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Jakub Kicinski 0c2a4d30 6a46e4fa

+82 -67
+10 -10
drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
··· 72 72 struct imx_priv_data *dwmac = plat_dat->bsp_priv; 73 73 int val; 74 74 75 - switch (plat_dat->mac_interface) { 75 + switch (plat_dat->phy_interface) { 76 76 case PHY_INTERFACE_MODE_MII: 77 77 val = GPR_ENET_QOS_INTF_SEL_MII; 78 78 break; ··· 88 88 GPR_ENET_QOS_RGMII_EN; 89 89 break; 90 90 default: 91 - pr_debug("imx dwmac doesn't support %d interface\n", 92 - plat_dat->mac_interface); 91 + pr_debug("imx dwmac doesn't support %s interface\n", 92 + phy_modes(plat_dat->phy_interface)); 93 93 return -EINVAL; 94 94 } 95 95 ··· 112 112 struct imx_priv_data *dwmac = plat_dat->bsp_priv; 113 113 int val, ret; 114 114 115 - switch (plat_dat->mac_interface) { 115 + switch (plat_dat->phy_interface) { 116 116 case PHY_INTERFACE_MODE_MII: 117 117 val = MX93_GPR_ENET_QOS_INTF_SEL_MII; 118 118 break; ··· 134 134 val = MX93_GPR_ENET_QOS_INTF_SEL_RGMII; 135 135 break; 136 136 default: 137 - dev_dbg(dwmac->dev, "imx dwmac doesn't support %d interface\n", 138 - plat_dat->mac_interface); 137 + dev_dbg(dwmac->dev, "imx dwmac doesn't support %s interface\n", 138 + phy_modes(plat_dat->phy_interface)); 139 139 return -EINVAL; 140 140 } 141 141 ··· 197 197 { 198 198 struct imx_priv_data *dwmac = bsp_priv; 199 199 200 - interface = dwmac->plat_dat->mac_interface; 200 + interface = dwmac->plat_dat->phy_interface; 201 201 if (interface == PHY_INTERFACE_MODE_RMII || 202 202 interface == PHY_INTERFACE_MODE_MII) 203 203 return 0; ··· 215 215 plat_dat = dwmac->plat_dat; 216 216 217 217 if (dwmac->ops->mac_rgmii_txclk_auto_adj || 218 - (plat_dat->mac_interface == PHY_INTERFACE_MODE_RMII) || 219 - (plat_dat->mac_interface == PHY_INTERFACE_MODE_MII)) 218 + (plat_dat->phy_interface == PHY_INTERFACE_MODE_RMII) || 219 + (plat_dat->phy_interface == PHY_INTERFACE_MODE_MII)) 220 220 return; 221 221 222 222 rate = rgmii_clock(speed); ··· 274 274 value |= DMA_BUS_MODE_SFT_RESET; 275 275 writel(value, ioaddr + DMA_BUS_MODE); 276 276 277 - if (plat_dat->mac_interface == PHY_INTERFACE_MODE_RMII) { 277 + if (plat_dat->phy_interface == PHY_INTERFACE_MODE_RMII) { 278 278 usleep_range(100, 200); 279 279 writel(RMII_RESET_SPEED, ioaddr + MAC_CTRL_REG); 280 280 }
+15 -10
drivers/net/ethernet/stmicro/stmmac/dwmac-ingenic.c
··· 90 90 struct ingenic_mac *mac = plat_dat->bsp_priv; 91 91 unsigned int val; 92 92 93 - switch (plat_dat->mac_interface) { 93 + switch (plat_dat->phy_interface) { 94 94 case PHY_INTERFACE_MODE_MII: 95 95 val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) | 96 96 FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_MII); ··· 119 119 break; 120 120 121 121 default: 122 - dev_err(mac->dev, "Unsupported interface %d", plat_dat->mac_interface); 122 + dev_err(mac->dev, "Unsupported interface %s\n", 123 + phy_modes(plat_dat->phy_interface)); 123 124 return -EINVAL; 124 125 } 125 126 ··· 132 131 { 133 132 struct ingenic_mac *mac = plat_dat->bsp_priv; 134 133 135 - switch (plat_dat->mac_interface) { 134 + switch (plat_dat->phy_interface) { 136 135 case PHY_INTERFACE_MODE_RMII: 137 136 dev_dbg(mac->dev, "MAC PHY Control Register: PHY_INTERFACE_MODE_RMII\n"); 138 137 break; 139 138 140 139 default: 141 - dev_err(mac->dev, "Unsupported interface %d", plat_dat->mac_interface); 140 + dev_err(mac->dev, "Unsupported interface %s\n", 141 + phy_modes(plat_dat->phy_interface)); 142 142 return -EINVAL; 143 143 } 144 144 ··· 152 150 struct ingenic_mac *mac = plat_dat->bsp_priv; 153 151 unsigned int val; 154 152 155 - switch (plat_dat->mac_interface) { 153 + switch (plat_dat->phy_interface) { 156 154 case PHY_INTERFACE_MODE_RMII: 157 155 val = FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII); 158 156 dev_dbg(mac->dev, "MAC PHY Control Register: PHY_INTERFACE_MODE_RMII\n"); 159 157 break; 160 158 161 159 default: 162 - dev_err(mac->dev, "Unsupported interface %d", plat_dat->mac_interface); 160 + dev_err(mac->dev, "Unsupported interface %s\n", 161 + phy_modes(plat_dat->phy_interface)); 163 162 return -EINVAL; 164 163 } 165 164 ··· 173 170 struct ingenic_mac *mac = plat_dat->bsp_priv; 174 171 unsigned int val; 175 172 176 - switch (plat_dat->mac_interface) { 173 + switch (plat_dat->phy_interface) { 177 174 case PHY_INTERFACE_MODE_RMII: 178 175 val = FIELD_PREP(MACPHYC_MODE_SEL_MASK, MACPHYC_MODE_SEL_RMII) | 179 176 FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII); ··· 181 178 break; 182 179 183 180 default: 184 - dev_err(mac->dev, "Unsupported interface %d", plat_dat->mac_interface); 181 + dev_err(mac->dev, "Unsupported interface %s\n", 182 + phy_modes(plat_dat->phy_interface)); 185 183 return -EINVAL; 186 184 } 187 185 ··· 195 191 struct ingenic_mac *mac = plat_dat->bsp_priv; 196 192 unsigned int val; 197 193 198 - switch (plat_dat->mac_interface) { 194 + switch (plat_dat->phy_interface) { 199 195 case PHY_INTERFACE_MODE_RMII: 200 196 val = FIELD_PREP(MACPHYC_TX_SEL_MASK, MACPHYC_TX_SEL_ORIGIN) | 201 197 FIELD_PREP(MACPHYC_RX_SEL_MASK, MACPHYC_RX_SEL_ORIGIN) | ··· 225 221 break; 226 222 227 223 default: 228 - dev_err(mac->dev, "Unsupported interface %d", plat_dat->mac_interface); 224 + dev_err(mac->dev, "Unsupported interface %s\n", 225 + phy_modes(plat_dat->phy_interface)); 229 226 return -EINVAL; 230 227 } 231 228
-2
drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
··· 98 98 /* Set default value for multicast hash bins */ 99 99 plat->multicast_filter_bins = 256; 100 100 101 - plat->mac_interface = PHY_INTERFACE_MODE_NA; 102 - 103 101 /* Set default value for unicast filter entries */ 104 102 plat->unicast_filter_entries = 1; 105 103
-1
drivers/net/ethernet/stmicro/stmmac/dwmac-lpc18xx.c
··· 41 41 if (IS_ERR(plat_dat)) 42 42 return PTR_ERR(plat_dat); 43 43 44 - plat_dat->mac_interface = PHY_INTERFACE_MODE_NA; 45 44 plat_dat->has_gmac = true; 46 45 47 46 reg = syscon_regmap_lookup_by_compatible("nxp,lpc1850-creg");
+1 -1
drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
··· 234 234 235 235 static int socfpga_get_plat_phymode(struct socfpga_dwmac *dwmac) 236 236 { 237 - return dwmac->plat_dat->mac_interface; 237 + return dwmac->plat_dat->phy_interface; 238 238 } 239 239 240 240 static void socfpga_sgmii_config(struct socfpga_dwmac *dwmac, bool enable)
+3 -3
drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
··· 38 38 unsigned int mode; 39 39 int err; 40 40 41 - switch (plat_dat->mac_interface) { 41 + switch (plat_dat->phy_interface) { 42 42 case PHY_INTERFACE_MODE_RMII: 43 43 mode = STARFIVE_DWMAC_PHY_INFT_RMII; 44 44 break; ··· 51 51 break; 52 52 53 53 default: 54 - dev_err(dwmac->dev, "unsupported interface %d\n", 55 - plat_dat->mac_interface); 54 + dev_err(dwmac->dev, "unsupported interface %s\n", 55 + phy_modes(plat_dat->phy_interface)); 56 56 return -EINVAL; 57 57 } 58 58
+13 -13
drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
··· 171 171 { 172 172 struct stm32_dwmac *dwmac = plat_dat->bsp_priv; 173 173 174 - switch (plat_dat->mac_interface) { 174 + switch (plat_dat->phy_interface) { 175 175 case PHY_INTERFACE_MODE_MII: 176 176 dwmac->enable_eth_ck = dwmac->ext_phyclk; 177 177 return 0; ··· 193 193 default: 194 194 dwmac->enable_eth_ck = false; 195 195 dev_err(dwmac->dev, "Mode %s not supported", 196 - phy_modes(plat_dat->mac_interface)); 196 + phy_modes(plat_dat->phy_interface)); 197 197 return -EINVAL; 198 198 } 199 199 } ··· 206 206 if (!dwmac->enable_eth_ck) 207 207 return 0; 208 208 209 - switch (plat_dat->mac_interface) { 209 + switch (plat_dat->phy_interface) { 210 210 case PHY_INTERFACE_MODE_MII: 211 211 case PHY_INTERFACE_MODE_GMII: 212 212 if (clk_rate == ETH_CK_F_25M) ··· 228 228 } 229 229 230 230 dev_err(dwmac->dev, "Mode %s does not match eth-ck frequency %d Hz", 231 - phy_modes(plat_dat->mac_interface), clk_rate); 231 + phy_modes(plat_dat->phy_interface), clk_rate); 232 232 return -EINVAL; 233 233 } 234 234 ··· 238 238 u32 reg = dwmac->mode_reg; 239 239 int val = 0; 240 240 241 - switch (plat_dat->mac_interface) { 241 + switch (plat_dat->phy_interface) { 242 242 case PHY_INTERFACE_MODE_MII: 243 243 /* 244 244 * STM32MP15xx supports both MII and GMII, STM32MP13xx MII only. ··· 269 269 break; 270 270 default: 271 271 dev_err(dwmac->dev, "Mode %s not supported", 272 - phy_modes(plat_dat->mac_interface)); 272 + phy_modes(plat_dat->phy_interface)); 273 273 /* Do not manage others interfaces */ 274 274 return -EINVAL; 275 275 } 276 276 277 - dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->mac_interface)); 277 + dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->phy_interface)); 278 278 279 279 /* Shift value at correct ethernet MAC offset in SYSCFG_PMCSETR */ 280 280 val <<= ffs(dwmac->mode_mask) - ffs(SYSCFG_MP1_ETH_MASK); ··· 294 294 u32 reg = dwmac->mode_reg; 295 295 int val = 0; 296 296 297 - switch (plat_dat->mac_interface) { 297 + switch (plat_dat->phy_interface) { 298 298 case PHY_INTERFACE_MODE_MII: 299 299 /* ETH_REF_CLK_SEL bit in SYSCFG register is not applicable in MII mode */ 300 300 break; ··· 319 319 break; 320 320 default: 321 321 dev_err(dwmac->dev, "Mode %s not supported", 322 - phy_modes(plat_dat->mac_interface)); 322 + phy_modes(plat_dat->phy_interface)); 323 323 /* Do not manage others interfaces */ 324 324 return -EINVAL; 325 325 } 326 326 327 - dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->mac_interface)); 327 + dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->phy_interface)); 328 328 329 329 /* Select PTP (IEEE1588) clock selection from RCC (ck_ker_ethxptp) */ 330 330 val |= SYSCFG_ETHCR_ETH_PTP_CLK_SEL; ··· 359 359 u32 reg = dwmac->mode_reg; 360 360 int val; 361 361 362 - switch (plat_dat->mac_interface) { 362 + switch (plat_dat->phy_interface) { 363 363 case PHY_INTERFACE_MODE_MII: 364 364 val = SYSCFG_MCU_ETH_SEL_MII; 365 365 break; ··· 368 368 break; 369 369 default: 370 370 dev_err(dwmac->dev, "Mode %s not supported", 371 - phy_modes(plat_dat->mac_interface)); 371 + phy_modes(plat_dat->phy_interface)); 372 372 /* Do not manage others interfaces */ 373 373 return -EINVAL; 374 374 } 375 375 376 - dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->mac_interface)); 376 + dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->phy_interface)); 377 377 378 378 return regmap_update_bits(dwmac->regmap, reg, 379 379 SYSCFG_MCU_ETH_MASK, val << 23);
+2 -2
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
··· 974 974 } 975 975 } 976 976 977 - switch (plat->mac_interface) { 977 + switch (plat->phy_interface) { 978 978 case PHY_INTERFACE_MODE_MII: 979 979 /* default */ 980 980 break; ··· 989 989 break; 990 990 default: 991 991 dev_err(dev, "Unsupported interface mode: %s", 992 - phy_modes(plat->mac_interface)); 992 + phy_modes(plat->phy_interface)); 993 993 return -EINVAL; 994 994 } 995 995
+12 -12
drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
··· 56 56 struct thead_dwmac *dwmac = plat->bsp_priv; 57 57 u32 phyif; 58 58 59 - switch (plat->mac_interface) { 59 + switch (plat->phy_interface) { 60 60 case PHY_INTERFACE_MODE_MII: 61 61 phyif = PHY_INTF_MII_GMII; 62 62 break; ··· 67 67 phyif = PHY_INTF_RGMII; 68 68 break; 69 69 default: 70 - dev_err(dwmac->dev, "unsupported phy interface %d\n", 71 - plat->mac_interface); 70 + dev_err(dwmac->dev, "unsupported phy interface %s\n", 71 + phy_modes(plat->phy_interface)); 72 72 return -EINVAL; 73 73 } 74 74 ··· 81 81 struct thead_dwmac *dwmac = plat->bsp_priv; 82 82 u32 txclk_dir; 83 83 84 - switch (plat->mac_interface) { 84 + switch (plat->phy_interface) { 85 85 case PHY_INTERFACE_MODE_MII: 86 86 txclk_dir = TXCLK_DIR_INPUT; 87 87 break; ··· 92 92 txclk_dir = TXCLK_DIR_OUTPUT; 93 93 break; 94 94 default: 95 - dev_err(dwmac->dev, "unsupported phy interface %d\n", 96 - plat->mac_interface); 95 + dev_err(dwmac->dev, "unsupported phy interface %s\n", 96 + phy_modes(plat->phy_interface)); 97 97 return -EINVAL; 98 98 } 99 99 ··· 112 112 113 113 plat = dwmac->plat; 114 114 115 - switch (plat->mac_interface) { 115 + switch (plat->phy_interface) { 116 116 /* For MII, rxc/txc is provided by phy */ 117 117 case PHY_INTERFACE_MODE_MII: 118 118 return 0; ··· 143 143 return 0; 144 144 145 145 default: 146 - dev_err(dwmac->dev, "unsupported phy interface %d\n", 147 - plat->mac_interface); 146 + dev_err(dwmac->dev, "unsupported phy interface %s\n", 147 + phy_modes(plat->phy_interface)); 148 148 return -EINVAL; 149 149 } 150 150 } ··· 154 154 struct thead_dwmac *dwmac = plat->bsp_priv; 155 155 u32 reg, div; 156 156 157 - switch (plat->mac_interface) { 157 + switch (plat->phy_interface) { 158 158 case PHY_INTERFACE_MODE_MII: 159 159 reg = GMAC_RX_CLK_EN | GMAC_TX_CLK_EN; 160 160 break; ··· 177 177 break; 178 178 179 179 default: 180 - dev_err(dwmac->dev, "unsupported phy interface %d\n", 181 - plat->mac_interface); 180 + dev_err(dwmac->dev, "unsupported phy interface %s\n", 181 + phy_modes(plat->phy_interface)); 182 182 return -EINVAL; 183 183 } 184 184
+1 -1
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
··· 1118 1118 */ 1119 1119 static void stmmac_check_pcs_mode(struct stmmac_priv *priv) 1120 1120 { 1121 - int interface = priv->plat->mac_interface; 1121 + int interface = priv->plat->phy_interface; 1122 1122 1123 1123 if (priv->dma_cap.pcs) { 1124 1124 if ((interface == PHY_INTERFACE_MODE_RGMII) ||
+5 -1
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
··· 453 453 return ERR_PTR(phy_mode); 454 454 455 455 plat->phy_interface = phy_mode; 456 + 456 457 rc = stmmac_of_get_mac_mode(np); 457 - plat->mac_interface = rc < 0 ? plat->phy_interface : rc; 458 + if (rc >= 0 && rc != phy_mode) 459 + dev_warn(&pdev->dev, 460 + "\"mac-mode\" property used for %s but differs to \"phy-mode\" of %s, and will be ignored. Please report.\n", 461 + phy_modes(rc), phy_modes(phy_mode)); 458 462 459 463 /* Some wrapper drivers still rely on phy_node. Let's save it while 460 464 * they are not converted to phylink. */
+20 -11
include/linux/stmmac.h
··· 190 190 int bus_id; 191 191 int phy_addr; 192 192 /* MAC ----- optional PCS ----- SerDes ----- optional PHY ----- Media 193 - * ^ ^ 194 - * mac_interface phy_interface 193 + * ^ 194 + * phy_interface 195 195 * 196 - * mac_interface is the MAC-side interface, which may be the same 197 - * as phy_interface if there is no intervening PCS. If there is a 198 - * PCS, then mac_interface describes the interface mode between the 199 - * MAC and PCS, and phy_interface describes the interface mode 200 - * between the PCS and PHY. 201 - */ 202 - phy_interface_t mac_interface; 203 - /* phy_interface is the PHY-side interface - the interface used by 204 - * an attached PHY. 196 + * The Synopsys dwmac core only covers the MAC and an optional 197 + * integrated PCS. Where the integrated PCS is used with a SerDes, 198 + * e.g. for 1000base-X or Cisco SGMII, the connection between the 199 + * PCS and SerDes will be TBI. 200 + * 201 + * Where the Synopsys dwmac core has been instantiated with multiple 202 + * interface modes, these are selected via core-external configuration 203 + * which is sampled when the dwmac core is reset. How this is done is 204 + * platform glue specific, but this defines the interface used from 205 + * the Synopsys dwmac core to the rest of the SoC. 206 + * 207 + * Where PCS other than the optional integrated Synopsys dwmac PCS 208 + * is used, this counts as "the rest of the SoC" in the above 209 + * paragraph. 210 + * 211 + * phy_interface is the PHY-side interface - the interface used by 212 + * an attached PHY or SFP etc. This is equivalent to the interface 213 + * that phylink uses. 205 214 */ 206 215 phy_interface_t phy_interface; 207 216 struct stmmac_mdio_bus_data *mdio_bus_data;