| Message ID | E1tna0z-0052tN-O1@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | net: stmmac: cleanup transmit clock setting | expand |
On Thu, Feb 27, 2025 at 09:17:09AM +0000, Russell King (Oracle) wrote: > Switch from using the fix_mac_speed() hook to set_clk_tx_rate() to > manage the transmit clock. > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
Hi, On 27/02/2025 10:17, Russell King (Oracle) wrote: > Switch from using the fix_mac_speed() hook to set_clk_tx_rate() to > manage the transmit clock. > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > --- > drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c > index b115b7873cef..07c504d07604 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c > @@ -22,9 +22,10 @@ struct meson_dwmac { > void __iomem *reg; > }; > > -static void meson6_dwmac_fix_mac_speed(void *priv, int speed, unsigned int mode) > +static int meson6_dwmac_set_clk_tx_rate(void *bsp_priv, struct clk *clk_tx_i, > + phy_interface_t interface, int speed) You can keep priv as first argument name and remove the next changes Neil > { > - struct meson_dwmac *dwmac = priv; > + struct meson_dwmac *dwmac = bsp_priv; > unsigned int val; > > val = readl(dwmac->reg); > @@ -39,6 +40,8 @@ static void meson6_dwmac_fix_mac_speed(void *priv, int speed, unsigned int mode) > } > > writel(val, dwmac->reg); > + > + return 0; > } > > static int meson6_dwmac_probe(struct platform_device *pdev) > @@ -65,7 +68,7 @@ static int meson6_dwmac_probe(struct platform_device *pdev) > return PTR_ERR(dwmac->reg); > > plat_dat->bsp_priv = dwmac; > - plat_dat->fix_mac_speed = meson6_dwmac_fix_mac_speed; > + plat_dat->set_clk_tx_rate = meson6_dwmac_set_clk_tx_rate; > > return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res); > }
On Thu, Feb 27, 2025 at 03:18:22PM +0100, Neil Armstrong wrote: > Hi, > > On 27/02/2025 10:17, Russell King (Oracle) wrote: > > Switch from using the fix_mac_speed() hook to set_clk_tx_rate() to > > manage the transmit clock. > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > --- > > drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c > > index b115b7873cef..07c504d07604 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c > > @@ -22,9 +22,10 @@ struct meson_dwmac { > > void __iomem *reg; > > }; > > -static void meson6_dwmac_fix_mac_speed(void *priv, int speed, unsigned int mode) > > +static int meson6_dwmac_set_clk_tx_rate(void *bsp_priv, struct clk *clk_tx_i, > > + phy_interface_t interface, int speed) > > You can keep priv as first argument name and remove the next changes I *can* but I don't want to. Inside the bulk of the stmmac driver, "priv" is used with struct stmmac_priv. "plat_dat" is used with struct plat_stmmacenet_data. Having different parts of the driver use the same local variable name for different structures is confusing, and has already lead to errors. Consistency is key. This is called "bsp_priv" in struct plat_stmmacenet_data, and therefore it should be referred to as "bsp_priv". I am not yet going to be doing a big rename, but it *will* come in time.
On 27/02/2025 15:35, Russell King (Oracle) wrote: > On Thu, Feb 27, 2025 at 03:18:22PM +0100, Neil Armstrong wrote: >> Hi, >> >> On 27/02/2025 10:17, Russell King (Oracle) wrote: >>> Switch from using the fix_mac_speed() hook to set_clk_tx_rate() to >>> manage the transmit clock. >>> >>> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> >>> --- >>> drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c | 9 ++++++--- >>> 1 file changed, 6 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c >>> index b115b7873cef..07c504d07604 100644 >>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c >>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c >>> @@ -22,9 +22,10 @@ struct meson_dwmac { >>> void __iomem *reg; >>> }; >>> -static void meson6_dwmac_fix_mac_speed(void *priv, int speed, unsigned int mode) >>> +static int meson6_dwmac_set_clk_tx_rate(void *bsp_priv, struct clk *clk_tx_i, >>> + phy_interface_t interface, int speed) >> >> You can keep priv as first argument name and remove the next changes > > I *can* but I don't want to. Inside the bulk of the stmmac driver, > "priv" is used with struct stmmac_priv. "plat_dat" is used with > struct plat_stmmacenet_data. Right, it's still an unrelated change in this case. > > Having different parts of the driver use the same local variable > name for different structures is confusing, and has already lead to > errors. Consistency is key. This is called "bsp_priv" in > struct plat_stmmacenet_data, and therefore it should be referred to > as "bsp_priv". > > I am not yet going to be doing a big rename, but it *will* come in > time. > Doing it in a big rename patch would be much better indeed. Neil
On Thu, Feb 27, 2025 at 03:38:53PM +0100, Neil Armstrong wrote: > On 27/02/2025 15:35, Russell King (Oracle) wrote: > > On Thu, Feb 27, 2025 at 03:18:22PM +0100, Neil Armstrong wrote: > > > Hi, > > > > > > On 27/02/2025 10:17, Russell King (Oracle) wrote: > > > > Switch from using the fix_mac_speed() hook to set_clk_tx_rate() to > > > > manage the transmit clock. > > > > > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > > > --- > > > > drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c | 9 ++++++--- > > > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c > > > > index b115b7873cef..07c504d07604 100644 > > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c > > > > @@ -22,9 +22,10 @@ struct meson_dwmac { > > > > void __iomem *reg; > > > > }; > > > > -static void meson6_dwmac_fix_mac_speed(void *priv, int speed, unsigned int mode) > > > > +static int meson6_dwmac_set_clk_tx_rate(void *bsp_priv, struct clk *clk_tx_i, > > > > + phy_interface_t interface, int speed) > > > > > > You can keep priv as first argument name and remove the next changes > > > > I *can* but I don't want to. Inside the bulk of the stmmac driver, > > "priv" is used with struct stmmac_priv. "plat_dat" is used with > > struct plat_stmmacenet_data. > > Right, it's still an unrelated change in this case. This is a new method, even though it happens to have mostly the same body. All instances of this new method use "bsp_priv" for this argument, therefore it is consistent.
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c index b115b7873cef..07c504d07604 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c @@ -22,9 +22,10 @@ struct meson_dwmac { void __iomem *reg; }; -static void meson6_dwmac_fix_mac_speed(void *priv, int speed, unsigned int mode) +static int meson6_dwmac_set_clk_tx_rate(void *bsp_priv, struct clk *clk_tx_i, + phy_interface_t interface, int speed) { - struct meson_dwmac *dwmac = priv; + struct meson_dwmac *dwmac = bsp_priv; unsigned int val; val = readl(dwmac->reg); @@ -39,6 +40,8 @@ static void meson6_dwmac_fix_mac_speed(void *priv, int speed, unsigned int mode) } writel(val, dwmac->reg); + + return 0; } static int meson6_dwmac_probe(struct platform_device *pdev) @@ -65,7 +68,7 @@ static int meson6_dwmac_probe(struct platform_device *pdev) return PTR_ERR(dwmac->reg); plat_dat->bsp_priv = dwmac; - plat_dat->fix_mac_speed = meson6_dwmac_fix_mac_speed; + plat_dat->set_clk_tx_rate = meson6_dwmac_set_clk_tx_rate; return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res); }
Switch from using the fix_mac_speed() hook to set_clk_tx_rate() to manage the transmit clock. Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)