net: stmmac: mdio related cleanups
Subsystems (5)
v2 cover net: stmmac: mdio related cleanups
Hi,
The first four patches clean up the MDC clock divisor selection code, turning the three different ways we choose a divisor into tabular form, rather than doing the selection purely in code.
Convert MDIO to use field_prep() which allows a non-constant mask to be used when preparing fields.
Then use u32 and the associated typed GENMASK for MDIO register field definitions.
Finally, an extra couple of patches that use appropriate types in struct mdio_bus_data.
v2:
- add comments about 300MHz to commit message in patch 1
- fix incorrect usage of stmmac_sun8i_csr_to_mdc in patch 3
- fix BIT_u32()->BIT_U32() in patch 8
drivers/net/ethernet/stmicro/stmmac/common.h | 10 +- drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-loongson.c | 11 +-- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 9 +- .../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 9 +- .../net/ethernet/stmicro/stmmac/dwmac100_core.c | 9 +- drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 9 +- .../net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 18 ++-- drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 106 ++++++++++++--------- include/linux/stmmac.h | 6 +- 10 files changed, 92 insertions(+), 97 deletions(-)
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
PATCH v2 1/8 net: stmmac: mdio: convert MDC clock divisor selection to tables
Convert the MDC clock divisor selection to tabular format.
Note that there is a change for 300MHz, but this is not a problem, as the MDC clock remains within the useable ranges, which are:
STMMAC_CSR_500_800M /324 1.54 - 2.47MHz STMMAC_CSR_300_500M /204 1.47 - 2.45MHz STMMAC_CSR_250_300M /124 2.02 - 2.42MHz STMMAC_CSR_150_250M /102 1.47 - 2.45MHz STMMAC_CSR_100_150M /62 1.61 - 2.42MHz STMMAC_CSR_60_100M /42 1.43 - 2.38MHz STMMAC_CSR_35_60M /26 1.35 - 2.31MHz STMMAC_CSR_20_35M /16 1.25 - 2.19MHz
Thus, with the change of divisor for exactly 300MHz, MDC temporarily changes from 2.42MHz to 1.47MHz for the sake of consistency.
The databook does not specify whether the frequency limits for the CSR divider are inclusive or exclusive.
--- .../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 98 ++++++++++++------- 1 file changed, 62 insertions(+), 36 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +62 -36
@@ -473,6 +473,52 @@ void stmmac_pcs_clean(struct net_device *ndev) priv->hw->xpcs =NULL ; } +struct stmmac_clk_rate { +unsigned long rate; +u8 cr; +}; + +/* The standard clk_csr_i to GMII_Address CR field mapping. The rate provided +* in this table is the exclusive maximum frequency for the divisor. The +* comments for each entry give the divisor and the resulting range of MDC +* clock frequencies. +*/ +static const struct stmmac_clk_rate stmmac_std_csr_to_mdc[] = { + {CSR_F_35M ,STMMAC_CSR_20_35M }, + {CSR_F_60M ,STMMAC_CSR_35_60M }, + {CSR_F_100M ,STMMAC_CSR_60_100M }, + {CSR_F_150M ,STMMAC_CSR_100_150M }, + {CSR_F_250M ,STMMAC_CSR_150_250M }, + {CSR_F_300M ,STMMAC_CSR_250_300M }, + {CSR_F_500M ,STMMAC_CSR_300_500M }, + {CSR_F_800M ,STMMAC_CSR_500_800M }, + { }, +}; + +/* The sun8i clk_csr_i to GMII_Address CR field mapping uses rate as the +* exclusive minimum frequency for the divisor. Note that the last entry +* is valid and also acts as the sentinel. +*/ +static const struct stmmac_clk_rate stmmac_sun8i_csr_to_mdc[] = { + {160000000 ,3 }, + {80000000 ,2 }, + {40000000 ,1 }, + {0 ,0 }, +}; + +/* The xgmac clk_csr_i to GMII_Address CR field mapping similarly uses rate +* as the exclusive minimum frequency for the divisor, and again the last +* entry is valid and also the sentinel. +*/ +static const struct stmmac_clk_rate stmmac_xgmac_csr_to_mdc[] = { + {400000000 ,5 }, + {350000000 ,4 }, + {300000000 ,3 }, + {250000000 ,2 }, + {150000000 ,1 }, + {0 ,0 }, +}; +/** * stmmac_clk_csr_set - dynamically set the MDC clock * @priv: driver private structure
@@ -490,6 +536,7 @@ static u32 stmmac_clk_csr_set(struct stmmac_priv *priv) {unsigned long clk_rate;u32 value = ~0 ; +int i; clk_rate =clk_get_rate (priv->plat->stmmac_clk);
@@ -500,47 +547,26 @@ static u32 stmmac_clk_csr_set(struct stmmac_priv *priv)* the frequency of clk_csr_i. So we do not change the default * divider. */ -if (clk_rate <CSR_F_35M ) - value =STMMAC_CSR_20_35M ; -else if (clk_rate <CSR_F_60M ) - value =STMMAC_CSR_35_60M ; -else if (clk_rate <CSR_F_100M ) - value =STMMAC_CSR_60_100M ; -else if (clk_rate <CSR_F_150M ) - value =STMMAC_CSR_100_150M ; -else if (clk_rate <CSR_F_250M ) - value =STMMAC_CSR_150_250M ; -else if (clk_rate <=CSR_F_300M ) - value =STMMAC_CSR_250_300M ; -else if (clk_rate <CSR_F_500M ) - value =STMMAC_CSR_300_500M ; -else if (clk_rate <CSR_F_800M ) - value =STMMAC_CSR_500_800M ; +for (i =0 ; stmmac_std_csr_to_mdc[i].rate; i++) +if (clk_rate < stmmac_std_csr_to_mdc[i].rate) { + value = stmmac_std_csr_to_mdc[i].cr; +break ; + }if (priv->plat->flags &STMMAC_FLAG_HAS_SUN8I ) { -if (clk_rate >160000000 ) - value =0x03 ; -else if (clk_rate >80000000 ) - value =0x02 ; -else if (clk_rate >40000000 ) - value =0x01 ; -else - value =0 ; +/* Note the different test - this is intentional. */ +for (i =0 ; stmmac_sun8i_csr_to_mdc[i].rate; i++) +if (clk_rate > stmmac_sun8i_csr_to_mdc[i].rate) +break ; + value = stmmac_sun8i_csr_to_mdc[i].cr; }if (priv->plat->core_type ==DWMAC_CORE_XGMAC ) { -if (clk_rate >400000000 ) - value =0x5 ; -else if (clk_rate >350000000 ) - value =0x4 ; -else if (clk_rate >300000000 ) - value =0x3 ; -else if (clk_rate >250000000 ) - value =0x2 ; -else if (clk_rate >150000000 ) - value =0x1 ; -else - value =0x0 ; +/* Note the different test - this is intentional. */ +for (i =0 ; stmmac_xgmac_csr_to_mdc[i].rate; i++) +if (clk_rate > stmmac_xgmac_csr_to_mdc[i].rate) +break ; + value = stmmac_xgmac_csr_to_mdc[i].cr; }return value;
Signature · 1 line
2.47.3
PATCH v2 2/8 net: stmmac: mdio: use same test for MDC clock divisor lookups
In reply to Russell King (Oracle)Use the same frequency test for all clk_csr value lookups (clock rate > table rate). This has the side effect that the standard rate table results in the divider being used for the maximum frequency for the divider rather than the next higher divider. This still allows MDC to meet the IEE 802.3 specification, but at a rate closer to 2.5MHz for these frequencies.
--- drivers/net/ethernet/stmicro/stmmac/common.h | 1 + .../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 27 +++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-)
drivers/net/ethernet/stmicro/stmmac/common.h +1 -0
@@ -257,6 +257,7 @@ struct stmmac_safety_stats { (sizeof (struct stmmac_safety_stats ) /sizeof (unsigned long )) /* CSR Frequency Access Defines*/ +#define CSR_F_20M 20000000#define CSR_F_35M 35000000#define CSR_F_60M 60000000#define CSR_F_100M 100000000
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +13 -14
@@ -484,15 +484,16 @@ struct stmmac_clk_rate {* clock frequencies. */ static const struct stmmac_clk_rate stmmac_std_csr_to_mdc[] = { - {CSR_F_35M ,STMMAC_CSR_20_35M }, - {CSR_F_60M ,STMMAC_CSR_35_60M }, - {CSR_F_100M ,STMMAC_CSR_60_100M }, - {CSR_F_150M ,STMMAC_CSR_100_150M }, - {CSR_F_250M ,STMMAC_CSR_150_250M }, - {CSR_F_300M ,STMMAC_CSR_250_300M }, - {CSR_F_500M ,STMMAC_CSR_300_500M }, - {CSR_F_800M ,STMMAC_CSR_500_800M }, - { }, + {CSR_F_800M , ~0 }, + {CSR_F_500M ,STMMAC_CSR_500_800M }, + {CSR_F_300M ,STMMAC_CSR_300_500M }, + {CSR_F_250M ,STMMAC_CSR_250_300M }, + {CSR_F_150M ,STMMAC_CSR_150_250M }, + {CSR_F_100M ,STMMAC_CSR_100_150M }, + {CSR_F_60M ,STMMAC_CSR_60_100M }, + {CSR_F_35M ,STMMAC_CSR_35_60M }, + {CSR_F_20M ,STMMAC_CSR_20_35M }, + {0 , ~0 }, };/* The sun8i clk_csr_i to GMII_Address CR field mapping uses rate as the
@@ -548,13 +549,12 @@ static u32 stmmac_clk_csr_set(struct stmmac_priv *priv)* divider. */ for (i =0 ; stmmac_std_csr_to_mdc[i].rate; i++) -if (clk_rate < stmmac_std_csr_to_mdc[i].rate) { - value = stmmac_std_csr_to_mdc[i].cr; +if (clk_rate > stmmac_std_csr_to_mdc[i].rate)break ; - } +if (stmmac_std_csr_to_mdc[i].cr != (u8 )~0 ) + value = stmmac_std_csr_to_mdc[i].cr;if (priv->plat->flags &STMMAC_FLAG_HAS_SUN8I ) { -/* Note the different test - this is intentional. */ for (i =0 ; stmmac_sun8i_csr_to_mdc[i].rate; i++)if (clk_rate > stmmac_sun8i_csr_to_mdc[i].rate)break ;
@@ -562,7 +562,6 @@ static u32 stmmac_clk_csr_set(struct stmmac_priv *priv) }if (priv->plat->core_type ==DWMAC_CORE_XGMAC ) { -/* Note the different test - this is intentional. */ for (i =0 ; stmmac_xgmac_csr_to_mdc[i].rate; i++)if (clk_rate > stmmac_xgmac_csr_to_mdc[i].rate)break ;
Signature · 1 line
2.47.3
PATCH v2 3/8 net: stmmac: mdio: simplify MDC clock divisor lookup
In reply to Russell King (Oracle)As each lookup now iterates over each table in the same way, simplfy the code to select the table, and then walk that table.
--- .../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 29 +++++++------------ 1 file changed, 11 insertions(+), 18 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +11 -18
@@ -535,6 +535,7 @@ static const struct stmmac_clk_rate stmmac_xgmac_csr_to_mdc[] = {*/ static u32 stmmac_clk_csr_set (struct stmmac_priv *priv) { +const struct stmmac_clk_rate *rates;unsigned long clk_rate;u32 value = ~0 ;int i;
@@ -548,25 +549,17 @@ static u32 stmmac_clk_csr_set(struct stmmac_priv *priv)* the frequency of clk_csr_i. So we do not change the default * divider. */ -for (i =0 ; stmmac_std_csr_to_mdc[i].rate; i++) -if (clk_rate > stmmac_std_csr_to_mdc[i].rate) -break ; -if (stmmac_std_csr_to_mdc[i].cr != (u8 )~0 ) - value = stmmac_std_csr_to_mdc[i].cr; - -if (priv->plat->flags &STMMAC_FLAG_HAS_SUN8I ) { -for (i =0 ; stmmac_sun8i_csr_to_mdc[i].rate; i++) -if (clk_rate > stmmac_sun8i_csr_to_mdc[i].rate) -break ; - value = stmmac_sun8i_csr_to_mdc[i].cr; - } + rates = stmmac_std_csr_to_mdc; +if (priv->plat->flags &STMMAC_FLAG_HAS_SUN8I ) + rates = stmmac_sun8i_csr_to_mdc; +if (priv->plat->core_type ==DWMAC_CORE_XGMAC ) + rates = stmmac_xgmac_csr_to_mdc; -if (priv->plat->core_type ==DWMAC_CORE_XGMAC ) { -for (i =0 ; stmmac_xgmac_csr_to_mdc[i].rate; i++) -if (clk_rate > stmmac_xgmac_csr_to_mdc[i].rate) -break ; - value = stmmac_xgmac_csr_to_mdc[i].cr; - } +for (i =0 ; rates[i].rate; i++) +if (clk_rate > rates[i].rate) +break ; +if (rates[i].cr != (u8 )~0 ) + value = rates[i].cr;return value; }
Signature · 1 line
2.47.3
PATCH v2 4/8 net: stmmac: mdio: convert field prep to use field_prep()
In reply to Russell King (Oracle)Convert the MDIO field preparation to use field_prep(), which removes the need to store separate mask and shifts. Also convert the clk_csr value using __ffs() to do the shift as we need to detect overflows for this.
--- drivers/net/ethernet/stmicro/stmmac/common.h | 3 --- drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c | 3 --- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 3 --- drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 3 --- drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c | 3 --- drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 3 --- drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 6 ------ drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 6 +++--- 8 files changed, 3 insertions(+), 27 deletions(-)
drivers/net/ethernet/stmicro/stmmac/common.h +0 -3
@@ -608,11 +608,8 @@ struct mac_link {struct mii_regs {unsigned int addr;/* MII Address */ unsigned int data;/* MII Data */ -unsigned int addr_shift;/* MII address shift */ -unsigned int reg_shift;/* MII reg shift */ unsigned int addr_mask;/* MII address mask */ unsigned int reg_mask;/* MII reg mask */ -unsigned int clk_csr_shift;unsigned int clk_csr_mask; };
drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +0 -3
@@ -367,11 +367,8 @@ static int loongson_dwmac_setup(void *apriv, struct mac_device_info *mac) mac->link.speed_mask =GMAC_CONTROL_PS |GMAC_CONTROL_FES ; mac->mii.addr =GMAC_MII_ADDR ; mac->mii.data =GMAC_MII_DATA ; - mac->mii.addr_shift =11 ; mac->mii.addr_mask =0x0000F800 ; - mac->mii.reg_shift =6 ; mac->mii.reg_mask =0x000007C0 ; - mac->mii.clk_csr_shift =2 ; mac->mii.clk_csr_mask =GENMASK (5 ,2 );return 0 ;
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +0 -3
@@ -1063,11 +1063,8 @@ static int sun8i_dwmac_setup(void *ppriv, struct mac_device_info *mac) mac->link.duplex =EMAC_DUPLEX_FULL ; mac->mii.addr =EMAC_MDIO_CMD ; mac->mii.data =EMAC_MDIO_DATA ; - mac->mii.reg_shift =4 ; mac->mii.reg_mask =GENMASK (8 ,4 ); - mac->mii.addr_shift =12 ; mac->mii.addr_mask =GENMASK (16 ,12 ); - mac->mii.clk_csr_shift =20 ; mac->mii.clk_csr_mask =GENMASK (22 ,20 ); mac->unicast_filter_entries =8 ;
drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +0 -3
@@ -496,11 +496,8 @@ int dwmac1000_setup(struct stmmac_priv *priv) mac->link.speed_mask =GMAC_CONTROL_PS |GMAC_CONTROL_FES ; mac->mii.addr =GMAC_MII_ADDR ; mac->mii.data =GMAC_MII_DATA ; - mac->mii.addr_shift =11 ; mac->mii.addr_mask =0x0000F800 ; - mac->mii.reg_shift =6 ; mac->mii.reg_mask =0x000007C0 ; - mac->mii.clk_csr_shift =2 ; mac->mii.clk_csr_mask =GENMASK (5 ,2 );return 0 ;
drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c +0 -3
@@ -184,11 +184,8 @@ int dwmac100_setup(struct stmmac_priv *priv) mac->link.speed_mask =MAC_CONTROL_PS ; mac->mii.addr =MAC_MII_ADDR ; mac->mii.data =MAC_MII_DATA ; - mac->mii.addr_shift =11 ; mac->mii.addr_mask =0x0000F800 ; - mac->mii.reg_shift =6 ; mac->mii.reg_mask =0x000007C0 ; - mac->mii.clk_csr_shift =2 ; mac->mii.clk_csr_mask =GENMASK (5 ,2 );return 0 ;
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +0 -3
@@ -1030,11 +1030,8 @@ int dwmac4_setup(struct stmmac_priv *priv) mac->link.speed_mask =GMAC_CONFIG_FES |GMAC_CONFIG_PS ; mac->mii.addr =GMAC_MDIO_ADDR ; mac->mii.data =GMAC_MDIO_DATA ; - mac->mii.addr_shift =21 ; mac->mii.addr_mask =GENMASK (25 ,21 ); - mac->mii.reg_shift =16 ; mac->mii.reg_mask =GENMASK (20 ,16 ); - mac->mii.clk_csr_shift =8 ; mac->mii.clk_csr_mask =GENMASK (11 ,8 ); mac->num_vlan =stmmac_get_num_vlan (priv->ioaddr);
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +0 -6
@@ -1551,11 +1551,8 @@ int dwxgmac2_setup(struct stmmac_priv *priv) mac->mii.addr =XGMAC_MDIO_ADDR ; mac->mii.data =XGMAC_MDIO_DATA ; - mac->mii.addr_shift =16 ; mac->mii.addr_mask =GENMASK (20 ,16 ); - mac->mii.reg_shift =0 ; mac->mii.reg_mask =GENMASK (15 ,0 ); - mac->mii.clk_csr_shift =19 ; mac->mii.clk_csr_mask =GENMASK (21 ,19 ); mac->num_vlan =stmmac_get_num_vlan (priv->ioaddr);
@@ -1594,11 +1591,8 @@ int dwxlgmac2_setup(struct stmmac_priv *priv) mac->mii.addr =XGMAC_MDIO_ADDR ; mac->mii.data =XGMAC_MDIO_DATA ; - mac->mii.addr_shift =16 ; mac->mii.addr_mask =GENMASK (20 ,16 ); - mac->mii.reg_shift =0 ; mac->mii.reg_mask =GENMASK (15 ,0 ); - mac->mii.clk_csr_shift =19 ; mac->mii.clk_csr_mask =GENMASK (21 ,19 );return 0 ;
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +3 -3
@@ -234,8 +234,8 @@ static u32 stmmac_mdio_format_addr(struct stmmac_priv *priv, {const struct mii_regs *mii_regs = &priv->hw->mii; -return ((pa << mii_regs->addr_shift) & mii_regs->addr_mask) | - ((gr << mii_regs->reg_shift) & mii_regs->reg_mask) | +return field_prep (mii_regs->addr_mask, pa) | +field_prep (mii_regs->reg_mask, gr) | priv->gmii_address_bus_config |MII_ADDR_GBUSY ; }
@@ -577,7 +577,7 @@ static void stmmac_mdio_bus_config(struct stmmac_priv *priv)else value =stmmac_clk_csr_set (priv); - value <<= priv->hw->mii.clk_csr_shift; + value <<=__ffs (priv->hw->mii.clk_csr_mask);if (value & ~priv->hw->mii.clk_csr_mask) dev_warn(priv->device,
Signature · 1 line
2.47.3
PATCH v2 5/8 net: stmmac: use u32 for MDIO register field masks
In reply to Russell King (Oracle)MDIO registers are 32-bit, so use u32 to describe the masks for these registers. Convert the GENMASK() initialisers to GENMASK_U32() for type compatibility.
--- drivers/net/ethernet/stmicro/stmmac/common.h | 6 +++--- drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c | 2 +- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 6 +++--- drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 2 +- drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c | 2 +- drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 6 +++--- drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 12 ++++++------ 7 files changed, 18 insertions(+), 18 deletions(-)
drivers/net/ethernet/stmicro/stmmac/common.h +3 -3
@@ -608,9 +608,9 @@ struct mac_link {struct mii_regs {unsigned int addr;/* MII Address */ unsigned int data;/* MII Data */ -unsigned int addr_mask;/* MII address mask */ -unsigned int reg_mask;/* MII reg mask */ -unsigned int clk_csr_mask; +u32 addr_mask;/* MII address mask */ +u32 reg_mask;/* MII reg mask */ +u32 clk_csr_mask; };struct mac_device_info {
drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +1 -1
@@ -369,7 +369,7 @@ static int loongson_dwmac_setup(void *apriv, struct mac_device_info *mac) mac->mii.data =GMAC_MII_DATA ; mac->mii.addr_mask =0x0000F800 ; mac->mii.reg_mask =0x000007C0 ; - mac->mii.clk_csr_mask =GENMASK (5 ,2 ); + mac->mii.clk_csr_mask =GENMASK_U32 (5 ,2 );return 0 ; }
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +3 -3
@@ -1063,9 +1063,9 @@ static int sun8i_dwmac_setup(void *ppriv, struct mac_device_info *mac) mac->link.duplex =EMAC_DUPLEX_FULL ; mac->mii.addr =EMAC_MDIO_CMD ; mac->mii.data =EMAC_MDIO_DATA ; - mac->mii.reg_mask =GENMASK (8 ,4 ); - mac->mii.addr_mask =GENMASK (16 ,12 ); - mac->mii.clk_csr_mask =GENMASK (22 ,20 ); + mac->mii.reg_mask =GENMASK_U32 (8 ,4 ); + mac->mii.addr_mask =GENMASK_U32 (16 ,12 ); + mac->mii.clk_csr_mask =GENMASK_U32 (22 ,20 ); mac->unicast_filter_entries =8 ;/* Synopsys Id is not available */
drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +1 -1
@@ -498,7 +498,7 @@ int dwmac1000_setup(struct stmmac_priv *priv) mac->mii.data =GMAC_MII_DATA ; mac->mii.addr_mask =0x0000F800 ; mac->mii.reg_mask =0x000007C0 ; - mac->mii.clk_csr_mask =GENMASK (5 ,2 ); + mac->mii.clk_csr_mask =GENMASK_U32 (5 ,2 );return 0 ; }
drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c +1 -1
@@ -186,7 +186,7 @@ int dwmac100_setup(struct stmmac_priv *priv) mac->mii.data =MAC_MII_DATA ; mac->mii.addr_mask =0x0000F800 ; mac->mii.reg_mask =0x000007C0 ; - mac->mii.clk_csr_mask =GENMASK (5 ,2 ); + mac->mii.clk_csr_mask =GENMASK_U32 (5 ,2 );return 0 ; }
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +3 -3
@@ -1030,9 +1030,9 @@ int dwmac4_setup(struct stmmac_priv *priv) mac->link.speed_mask =GMAC_CONFIG_FES |GMAC_CONFIG_PS ; mac->mii.addr =GMAC_MDIO_ADDR ; mac->mii.data =GMAC_MDIO_DATA ; - mac->mii.addr_mask =GENMASK (25 ,21 ); - mac->mii.reg_mask =GENMASK (20 ,16 ); - mac->mii.clk_csr_mask =GENMASK (11 ,8 ); + mac->mii.addr_mask =GENMASK_U32 (25 ,21 ); + mac->mii.reg_mask =GENMASK_U32 (20 ,16 ); + mac->mii.clk_csr_mask =GENMASK_U32 (11 ,8 ); mac->num_vlan =stmmac_get_num_vlan (priv->ioaddr);return 0 ;
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +6 -6
@@ -1551,9 +1551,9 @@ int dwxgmac2_setup(struct stmmac_priv *priv) mac->mii.addr =XGMAC_MDIO_ADDR ; mac->mii.data =XGMAC_MDIO_DATA ; - mac->mii.addr_mask =GENMASK (20 ,16 ); - mac->mii.reg_mask =GENMASK (15 ,0 ); - mac->mii.clk_csr_mask =GENMASK (21 ,19 ); + mac->mii.addr_mask =GENMASK_U32 (20 ,16 ); + mac->mii.reg_mask =GENMASK_U32 (15 ,0 ); + mac->mii.clk_csr_mask =GENMASK_U32 (21 ,19 ); mac->num_vlan =stmmac_get_num_vlan (priv->ioaddr);return 0 ;
@@ -1591,9 +1591,9 @@ int dwxlgmac2_setup(struct stmmac_priv *priv) mac->mii.addr =XGMAC_MDIO_ADDR ; mac->mii.data =XGMAC_MDIO_DATA ; - mac->mii.addr_mask =GENMASK (20 ,16 ); - mac->mii.reg_mask =GENMASK (15 ,0 ); - mac->mii.clk_csr_mask =GENMASK (21 ,19 ); + mac->mii.addr_mask =GENMASK_U32 (20 ,16 ); + mac->mii.reg_mask =GENMASK_U32 (15 ,0 ); + mac->mii.clk_csr_mask =GENMASK_U32 (21 ,19 );return 0 ; }
Signature · 1 line
2.47.3
PATCH v2 6/8 net: stmmac: use GENMASK_U32() for mdio bitfields
In reply to Russell King (Oracle)Rather than using hex numbers, use GENMASK() for mdio bitfields.
--- drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c | 4 ++-- drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 4 ++-- drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +2 -2
@@ -367,8 +367,8 @@ static int loongson_dwmac_setup(void *apriv, struct mac_device_info *mac) mac->link.speed_mask =GMAC_CONTROL_PS |GMAC_CONTROL_FES ; mac->mii.addr =GMAC_MII_ADDR ; mac->mii.data =GMAC_MII_DATA ; - mac->mii.addr_mask =0x0000F800 ; - mac->mii.reg_mask =0x000007C0 ; + mac->mii.addr_mask =GENMASK_U32 (15 ,11 ); + mac->mii.reg_mask =GENMASK_U32 (10 ,6 ); mac->mii.clk_csr_mask =GENMASK_U32 (5 ,2 );return 0 ;
drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +2 -2
@@ -496,8 +496,8 @@ int dwmac1000_setup(struct stmmac_priv *priv) mac->link.speed_mask =GMAC_CONTROL_PS |GMAC_CONTROL_FES ; mac->mii.addr =GMAC_MII_ADDR ; mac->mii.data =GMAC_MII_DATA ; - mac->mii.addr_mask =0x0000F800 ; - mac->mii.reg_mask =0x000007C0 ; + mac->mii.addr_mask =GENMASK_U32 (15 ,11 ); + mac->mii.reg_mask =GENMASK_U32 (10 ,6 ); mac->mii.clk_csr_mask =GENMASK_U32 (5 ,2 );return 0 ;
drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c +2 -2
@@ -184,8 +184,8 @@ int dwmac100_setup(struct stmmac_priv *priv) mac->link.speed_mask =MAC_CONTROL_PS ; mac->mii.addr =MAC_MII_ADDR ; mac->mii.data =MAC_MII_DATA ; - mac->mii.addr_mask =0x0000F800 ; - mac->mii.reg_mask =0x000007C0 ; + mac->mii.addr_mask =GENMASK_U32 (15 ,11 ); + mac->mii.reg_mask =GENMASK_U32 (10 ,6 ); mac->mii.clk_csr_mask =GENMASK_U32 (5 ,2 );return 0 ;
Signature · 1 line
2.47.3
PATCH v2 7/8 net: stmmac: mdio_bus_data->default_an_inband is boolean
In reply to Russell King (Oracle)default_an_inband is declared as an unsigned int, but is set to true/ false and is assigned to phylink_config's member of the same name which is a bool. Declare this also as a bool for consistency.
--- include/linux/stmmac.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/stmmac.h +1 -1
@@ -86,10 +86,10 @@ struct stmmac_priv;struct stmmac_mdio_bus_data {unsigned int phy_mask;unsigned int pcs_mask; -unsigned int default_an_inband;int *irqs;int probed_phy_irq;bool needs_reset; +bool default_an_inband; };struct stmmac_dma_cfg {
Signature · 1 line
2.47.3
PATCH v2 8/8 net: stmmac: make pcs_mask and phy_mask u32
In reply to Russell King (Oracle)The PCS and PHY masks are passed to the mdio bus layer as phy_mask to prevent bus addresses between 0 and 31 inclusive being scanned, and this is declared as u32. Also declare these as u32 in stmmac for type consistency.
Since this is a u32, use BIT_U32() rather than BIT() to generate values for these fields.
--- drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c | 2 +- drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c | 2 +- include/linux/stmmac.h | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c +1 -1
@@ -699,7 +699,7 @@ static int intel_mgbe_common_data(struct pci_dev *pdev,/* Intel mgbe SGMII interface uses pcs-xcps */ if (plat->phy_interface ==PHY_INTERFACE_MODE_SGMII || plat->phy_interface ==PHY_INTERFACE_MODE_1000BASEX ) { - plat->mdio_bus_data->pcs_mask =BIT (INTEL_MGBE_XPCS_ADDR ); + plat->mdio_bus_data->pcs_mask =BIT_U32 (INTEL_MGBE_XPCS_ADDR ); plat->mdio_bus_data->default_an_inband = true; plat->select_pcs = intel_mgbe_select_pcs; }
drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +1 -1
@@ -168,7 +168,7 @@ static int loongson_gnet_data(struct pci_dev *pdev,loongson_default_data (pdev, plat); plat->phy_interface =PHY_INTERFACE_MODE_GMII ; - plat->mdio_bus_data->phy_mask = ~(u32 )BIT (2 ); + plat->mdio_bus_data->phy_mask = ~BIT_U32 (2 ); plat->fix_mac_speed = loongson_gnet_fix_speed;return 0 ;
include/linux/stmmac.h +2 -2
@@ -84,8 +84,8 @@ struct stmmac_priv;/* Platfrom data for platform device structure's platform_data field */ struct stmmac_mdio_bus_data { -unsigned int phy_mask; -unsigned int pcs_mask; +u32 phy_mask; +u32 pcs_mask;int *irqs;int probed_phy_irq;bool needs_reset;
Signature · 1 line
2.47.3
Re: [PATCH net-next v2 1/8] net: stmmac: mdio: convert MDC clock divisor selection to tables
In reply to Russell King (Oracle)Quoted 4 lines · Russell King (Oracle)
Convert the MDC clock divisor selection to tabular format.
Note that there is a change for 300MHz, but this is not a problem, as the MDC clock remains within the useable ranges, which are:
I notice that checkpatch complains about "useable", which it should not do. "useable" is perfectly correct, especially in British English. As I'm British, I won't be fixing this checkpatch complaint.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Jakub Kicinski
maintainerRe: [PATCH net-next v2 1/8] net: stmmac: mdio: convert MDC clock divisor selection to tables
Quoted 9 lines · On Thu, 5 Mar 2026 12:11:06 +0000 Russell King (Oracle) wrote:
Quoted 4 lines · Russell King (Oracle)
Convert the MDC clock divisor selection to tabular format.
Note that there is a change for 300MHz, but this is not a problem, as the MDC clock remains within the useable ranges, which are:
I notice that checkpatch complains about "useable", which it should not do. "useable" is perfectly correct, especially in British English. As I'm British, I won't be fixing this checkpatch complaint.
Yesterday it reported a warning that someone repeated the word 'very'. As in "this is very very wrong". checkpatch, eh? gold.
Re: [PATCH net-next v2 3/8] net: stmmac: mdio: simplify MDC clock divisor lookup
In reply to Russell King (Oracle)Quoted 43 lines · Russell King (Oracle)
As each lookup now iterates over each table in the same way, simplfy the code to select the table, and then walk that table.
Signed-off-by Russell King (Oracle) <[email protected]>--- .../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 29 +++++++------------ 1 file changed, 11 insertions(+), 18 deletions(-)drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +6
-13@@ -535,6 +535,7 @@ static const struct stmmac_clk_rate stmmac_xgmac_csr_to_mdc[] = {*/ static u32 stmmac_clk_csr_set (struct stmmac_priv *priv) { +const struct stmmac_clk_rate *rates;unsigned long clk_rate;u32 value = ~0 ;int i;@@ -548,25 +549,17 @@ static u32 stmmac_clk_csr_set(struct stmmac_priv *priv)* the frequency of clk_csr_i. So we do not change the default * divider. */ -for (i =0 ; stmmac_std_csr_to_mdc[i].rate; i++) -if (clk_rate > stmmac_std_csr_to_mdc[i].rate) -break ; -if (stmmac_std_csr_to_mdc[i].cr != (u8 )~0 ) - value = stmmac_std_csr_to_mdc[i].cr; - -if (priv->plat->flags &STMMAC_FLAG_HAS_SUN8I ) { -for (i =0 ; stmmac_sun8i_csr_to_mdc[i].rate; i++) -if (clk_rate > stmmac_sun8i_csr_to_mdc[i].rate) -break ; - value = stmmac_sun8i_csr_to_mdc[i].cr; - } + rates = stmmac_std_csr_to_mdc; +if (priv->plat->flags &STMMAC_FLAG_HAS_SUN8I ) + rates = stmmac_sun8i_csr_to_mdc; +if (priv->plat->core_type ==DWMAC_CORE_XGMAC ) + rates = stmmac_xgmac_csr_to_mdc; -if (priv->plat->core_type ==DWMAC_CORE_XGMAC ) {
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c static u32 stmmac_clk_csr_set(struct stmmac_priv *priv) · line 562
Quoted 10 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +5
-5-for (i =0 ; stmmac_xgmac_csr_to_mdc[i].rate; i++) -if (clk_rate > stmmac_xgmac_csr_to_mdc[i].rate) -break ; - value = stmmac_xgmac_csr_to_mdc[i].cr; - } +for (i =0 ; rates[i].rate; i++) +if (clk_rate > rates[i].rate) +break ; +if (rates[i].cr != (u8 )~0 ) + value = rates[i].cr;
Looking at the AI review, it seems to me that the AI review is incorrect.
With reference to the full original code which can be seen in:
https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
If we look at the original code, then we can see that the intention is that if clk_rate is smaller than the last real entry for the sun8i and xgmac cases, the value should end up as zero - and this is exactly what the new code does. So, the behaviour is preserved, even though AI thinks it isn't.
AI seems to think that reaching the last entry for sun8i and xgmac means we have an invalid rate. That is where AI is going wrong - that is an incorrect "assumption".
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next v2 4/8] net: stmmac: mdio: convert field prep to use field_prep()
In reply to Russell King (Oracle)Hi,
Quoted 6 lines · Russell King (Oracle)
Convert the MDIO field preparation to use
field_prep(), which removes the need to store separate mask and shifts. Also convert theclk_csrvalue using__ffs()to do the shift as we need to detect overflows for this.Signed-off-by Russell King (Oracle) <[email protected]>
Maxime
Re: [PATCH net-next v2 6/8] net: stmmac: use GENMASK_U32() for mdio bitfields
In reply to Russell King (Oracle)Hi,
Quoted 3 lines · Russell King (Oracle)
Rather than using hex numbers, use
GENMASK()for mdio bitfields.Signed-off-by Russell King (Oracle) <[email protected]>
Maxime
Re: [PATCH net-next v2 7/8] net: stmmac: mdio_bus_data->default_an_inband is boolean
In reply to Russell King (Oracle)Hi,
Quoted 5 lines · Russell King (Oracle)
default_an_inbandis declared as an unsigned int, but is set to true/ false and is assigned to phylink_config's member of the same name which is a bool. Declare this also as a bool for consistency.Signed-off-by Russell King (Oracle) <[email protected]>
Maxime
Re: [PATCH net-next v2 8/8] net: stmmac: make pcs_mask and phy_mask u32
In reply to Russell King (Oracle)Hi,
Quoted 9 lines · Russell King (Oracle)
The PCS and PHY masks are passed to the mdio bus layer as
phy_maskto prevent bus addresses between 0 and 31 inclusive being scanned, and this is declared as u32. Also declare these as u32 in stmmac for type consistency.Since this is a u32, use
BIT_U32()rather thanBIT()to generate values for these fields.Signed-off-by Russell King (Oracle) <[email protected]>
Maxime
Hi,
Quoted 12 lines · Russell King (Oracle)
Hi,
The first four patches clean up the MDC clock divisor selection code, turning the three different ways we choose a divisor into tabular form, rather than doing the selection purely in code.
Convert MDIO to use
field_prep()which allows a non-constant mask to be used when preparing fields.Then use u32 and the associated typed GENMASK for MDIO register field definitions.
Quoted 7 lines
Finally, an extra couple of patches that use appropriate types in struct
mdio_bus_data.v2:
- add comments about 300MHz to commit message in patch 1
- fix incorrect usage ofstmmac_sun8i_csr_to_mdcin patch 3
- fix BIT_u32()->BIT_U32() in patch 8
I have tested this series as a whole on 2 platforms using the dwmac mdio driver (socfpga - KSZ9031 / imx8mp + KSZ9131), no regressions found. I'm not stressing the 300M case though.
feel free to add my :
Maxime
Quoted 12 lines
drivers/net/ethernet/stmicro/stmmac/common.h | 10 +- drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-loongson.c | 11 +-- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 9 +- .../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 9 +- .../net/ethernet/stmicro/stmmac/dwmac100_core.c | 9 +- drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 9 +- .../net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 18 ++-- drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 106 ++++++++++++--------- include/linux/stmmac.h | 6 +- 10 files changed, 92 insertions(+), 97 deletions(-)
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:
Hello:
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:
Quoted 10 lines · On Thu, 5 Mar 2026 10:42:03 +0000 you wrote:
Hi,
The first four patches clean up the MDC clock divisor selection code, turning the three different ways we choose a divisor into tabular form, rather than doing the selection purely in code.
Convert MDIO to use
field_prep()which allows a non-constant mask to be used when preparing fields.[...]
Here is the summary with links:
- [net-next,v2,1/8] net: stmmac: mdio: convert MDC clock divisor selection to tables
https://git.kernel.org/netdev/net-next/c/4c7e0e081889 - [net-next,v2,2/8] net: stmmac: mdio: use same test for MDC clock divisor lookups https://git.kernel.org/netdev/net-next/c/b6687ef97603 - [net-next,v2,3/8] net: stmmac: mdio: simplify MDC clock divisor lookup https://git.kernel.org/netdev/net-next/c/506f78f43c58 - [net-next,v2,4/8] net: stmmac: mdio: convert field prep to use field_prep() https://git.kernel.org/netdev/net-next/c/58bd0039002b - [net-next,v2,5/8] net: stmmac: use u32 for MDIO register field masks https://git.kernel.org/netdev/net-next/c/df388b4d3913 - [net-next,v2,6/8] net: stmmac: use GENMASK_U32() for mdio bitfields https://git.kernel.org/netdev/net-next/c/a64d927aecf1 - [net-next,v2,7/8] net: stmmac: mdio_bus_data->default_an_inband is boolean https://git.kernel.org/netdev/net-next/c/3cd963fa915c - [net-next,v2,8/8] net: stmmac: make pcs_mask and phy_mask u32 https://git.kernel.org/netdev/net-next/c/e4fd855c52ec
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html