diff mbox series

[net-next,10/11] net: stmmac: meson: switch to use set_clk_tx_rate() hook

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

Commit Message

Russell King (Oracle) Feb. 27, 2025, 9:17 a.m. UTC
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(-)

Comments

Andrew Lunn Feb. 27, 2025, 2:03 p.m. UTC | #1
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
Neil Armstrong Feb. 27, 2025, 2:18 p.m. UTC | #2
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);
>   }
Russell King (Oracle) Feb. 27, 2025, 2:35 p.m. UTC | #3
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.
diff mbox series

Patch

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);
 }