phy: lynx-28g: don't concatenate lynx_28g_lane_rmw() argument "reg" with "val" and "mask"

The last step in having lynx_28g_lane_rmw() arguments that fully point
to their definitions is the removal of the current concatenation logic,
by which e.g. "LNaTGCR0, N_RATE_QUARTER, N_RATE_MSK" is expanded to
"LNaTGCR0, LNaTGCR0_N_RATE_QUARTER, LNaTGCR0_N_RATE_MSK".

There are pros and cons to the above. An advantage is the impossibility
to mix up fields of one register with fields of another. For example
both LNaTGCR0 and LNaRGCR0 contain an N_RATE_QUARTER field (one for the
lane RX direction, one for the lane TX).

But the two notable disadvantages are:

1. the impossibility to write expressions such as logical OR between
multiple fields. Practically, this forces us to perform more accesses
to hardware registers than would otherwise be needed. See the LNaGCR0
access for example.

2. the necessity to invent fields that don't exist, like SGMIIaCR1_SGPCS_DIS,
in order to clear SGMIIaCR1_SGPCS_EN (the real field name). This is
confusing, because sometimes, fields that end with _DIS really exist,
and it's best to not invent new field names.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Link: https://patch.msgid.link/20251125114847.804961-7-vladimir.oltean@nxp.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>

authored by

Vladimir Oltean and committed by

Vinod Koul 6 months ago 6e3d3e87 13a5f7e3

+38 -22

1 changed file

expand all

drivers

phy

freescale

phy-fsl-lynx-28g.c

+38 -22

drivers/phy/freescale/phy-fsl-lynx-28g.c

··· 103 103 104 104 #define SGMIIaCR1(lane) (0x1804 + (lane) * 0x10) 105 105 #define SGMIIaCR1_SGPCS_EN BIT(11) 106 - #define SGMIIaCR1_SGPCS_DIS 0x0 107 106 #define SGMIIaCR1_SGPCS_MSK BIT(11) 108 107 109 108 struct lynx_28g_priv; ··· 149 150 } 150 151 151 152 #define lynx_28g_lane_rmw(lane, reg, val, mask) \ 152 - lynx_28g_rmw((lane)->priv, reg(lane->id), \ 153 - reg##_##val, reg##_##mask) 153 + lynx_28g_rmw((lane)->priv, reg(lane->id), val, mask) 154 154 #define lynx_28g_lane_read(lane, reg) \ 155 155 ioread32((lane)->priv->base + reg((lane)->id)) 156 156 #define lynx_28g_pll_read(pll, reg) \ ··· 203 205 switch (intf) { 204 206 case PHY_INTERFACE_MODE_SGMII: 205 207 case PHY_INTERFACE_MODE_1000BASEX: 206 - lynx_28g_lane_rmw(lane, LNaTGCR0, N_RATE_QUARTER, N_RATE_MSK); 207 - lynx_28g_lane_rmw(lane, LNaRGCR0, N_RATE_QUARTER, N_RATE_MSK); 208 + lynx_28g_lane_rmw(lane, LNaTGCR0, 209 + LNaTGCR0_N_RATE_QUARTER, 210 + LNaTGCR0_N_RATE_MSK); 211 + lynx_28g_lane_rmw(lane, LNaRGCR0, 212 + LNaRGCR0_N_RATE_QUARTER, 213 + LNaRGCR0_N_RATE_MSK); 208 214 break; 209 215 default: 210 216 break; ··· 218 216 switch (intf) { 219 217 case PHY_INTERFACE_MODE_10GBASER: 220 218 case PHY_INTERFACE_MODE_USXGMII: 221 - lynx_28g_lane_rmw(lane, LNaTGCR0, N_RATE_FULL, N_RATE_MSK); 222 - lynx_28g_lane_rmw(lane, LNaRGCR0, N_RATE_FULL, N_RATE_MSK); 219 + lynx_28g_lane_rmw(lane, LNaTGCR0, LNaTGCR0_N_RATE_FULL, 220 + LNaTGCR0_N_RATE_MSK); 221 + lynx_28g_lane_rmw(lane, LNaRGCR0, LNaRGCR0_N_RATE_FULL, 222 + LNaRGCR0_N_RATE_MSK); 223 223 break; 224 224 default: 225 225 break; ··· 236 232 struct lynx_28g_pll *pll) 237 233 { 238 234 if (pll->id == 0) { 239 - lynx_28g_lane_rmw(lane, LNaTGCR0, USE_PLLF, USE_PLL_MSK); 240 - lynx_28g_lane_rmw(lane, LNaRGCR0, USE_PLLF, USE_PLL_MSK); 235 + lynx_28g_lane_rmw(lane, LNaTGCR0, LNaTGCR0_USE_PLLF, 236 + LNaTGCR0_USE_PLL_MSK); 237 + lynx_28g_lane_rmw(lane, LNaRGCR0, LNaRGCR0_USE_PLLF, 238 + LNaRGCR0_USE_PLL_MSK); 241 239 } else { 242 - lynx_28g_lane_rmw(lane, LNaTGCR0, USE_PLLS, USE_PLL_MSK); 243 - lynx_28g_lane_rmw(lane, LNaRGCR0, USE_PLLS, USE_PLL_MSK); 240 + lynx_28g_lane_rmw(lane, LNaTGCR0, LNaTGCR0_USE_PLLS, 241 + LNaTGCR0_USE_PLL_MSK); 242 + lynx_28g_lane_rmw(lane, LNaRGCR0, LNaRGCR0_USE_PLLS, 243 + LNaRGCR0_USE_PLL_MSK); 244 244 } 245 245 } 246 246 ··· 285 277 GENMASK(3, 0) << lane_offset); 286 278 287 279 /* Setup the protocol select and SerDes parallel interface width */ 288 - lynx_28g_lane_rmw(lane, LNaGCR0, PROTO_SEL_SGMII, PROTO_SEL_MSK); 289 - lynx_28g_lane_rmw(lane, LNaGCR0, IF_WIDTH_10_BIT, IF_WIDTH_MSK); 280 + lynx_28g_lane_rmw(lane, LNaGCR0, 281 + LNaGCR0_PROTO_SEL_SGMII | LNaGCR0_IF_WIDTH_10_BIT, 282 + LNaGCR0_PROTO_SEL_MSK | LNaGCR0_IF_WIDTH_MSK); 290 283 291 284 /* Find the PLL that works with this interface type */ 292 285 pll = lynx_28g_pll_get(priv, PHY_INTERFACE_MODE_SGMII); ··· 301 292 lynx_28g_lane_set_nrate(lane, pll, PHY_INTERFACE_MODE_SGMII); 302 293 303 294 /* Enable the SGMII PCS */ 304 - lynx_28g_lane_rmw(lane, SGMIIaCR1, SGPCS_EN, SGPCS_MSK); 295 + lynx_28g_lane_rmw(lane, SGMIIaCR1, SGMIIaCR1_SGPCS_EN, 296 + SGMIIaCR1_SGPCS_MSK); 305 297 306 298 /* Configure the appropriate equalization parameters for the protocol */ 307 299 iowrite32(0x00808006, priv->base + LNaTECR0(lane->id)); ··· 327 317 GENMASK(3, 0) << lane_offset); 328 318 329 319 /* Setup the protocol select and SerDes parallel interface width */ 330 - lynx_28g_lane_rmw(lane, LNaGCR0, PROTO_SEL_XFI, PROTO_SEL_MSK); 331 - lynx_28g_lane_rmw(lane, LNaGCR0, IF_WIDTH_20_BIT, IF_WIDTH_MSK); 320 + lynx_28g_lane_rmw(lane, LNaGCR0, 321 + LNaGCR0_PROTO_SEL_XFI | LNaGCR0_IF_WIDTH_20_BIT, 322 + LNaGCR0_PROTO_SEL_MSK | LNaGCR0_IF_WIDTH_MSK); 332 323 333 324 /* Find the PLL that works with this interface type */ 334 325 pll = lynx_28g_pll_get(priv, PHY_INTERFACE_MODE_10GBASER); ··· 343 332 lynx_28g_lane_set_nrate(lane, pll, PHY_INTERFACE_MODE_10GBASER); 344 333 345 334 /* Disable the SGMII PCS */ 346 - lynx_28g_lane_rmw(lane, SGMIIaCR1, SGPCS_DIS, SGPCS_MSK); 335 + lynx_28g_lane_rmw(lane, SGMIIaCR1, 0, SGMIIaCR1_SGPCS_MSK); 347 336 348 337 /* Configure the appropriate equalization parameters for the protocol */ 349 338 iowrite32(0x10808307, priv->base + LNaTECR0(lane->id)); ··· 363 352 return 0; 364 353 365 354 /* Issue a halt request */ 366 - lynx_28g_lane_rmw(lane, LNaTRSTCTL, HLT_REQ, HLT_REQ); 367 - lynx_28g_lane_rmw(lane, LNaRRSTCTL, HLT_REQ, HLT_REQ); 355 + lynx_28g_lane_rmw(lane, LNaTRSTCTL, LNaTRSTCTL_HLT_REQ, 356 + LNaTRSTCTL_HLT_REQ); 357 + lynx_28g_lane_rmw(lane, LNaRRSTCTL, LNaRRSTCTL_HLT_REQ, 358 + LNaRRSTCTL_HLT_REQ); 368 359 369 360 /* Wait until the halting process is complete */ 370 361 do { ··· 389 376 return 0; 390 377 391 378 /* Issue a reset request on the lane */ 392 - lynx_28g_lane_rmw(lane, LNaTRSTCTL, RST_REQ, RST_REQ); 393 - lynx_28g_lane_rmw(lane, LNaRRSTCTL, RST_REQ, RST_REQ); 379 + lynx_28g_lane_rmw(lane, LNaTRSTCTL, LNaTRSTCTL_RST_REQ, 380 + LNaTRSTCTL_RST_REQ); 381 + lynx_28g_lane_rmw(lane, LNaRRSTCTL, LNaRRSTCTL_RST_REQ, 382 + LNaRRSTCTL_RST_REQ); 394 383 395 384 /* Wait until the reset sequence is completed */ 396 385 do { ··· 552 537 553 538 rrstctl = lynx_28g_lane_read(lane, LNaRRSTCTL); 554 539 if (!(rrstctl & LNaRRSTCTL_CDR_LOCK)) { 555 - lynx_28g_lane_rmw(lane, LNaRRSTCTL, RST_REQ, RST_REQ); 540 + lynx_28g_lane_rmw(lane, LNaRRSTCTL, LNaRRSTCTL_RST_REQ, 541 + LNaRRSTCTL_RST_REQ); 556 542 do { 557 543 rrstctl = lynx_28g_lane_read(lane, LNaRRSTCTL); 558 544 } while (!(rrstctl & LNaRRSTCTL_RST_DONE));

Configure Feed

Configure Feed