diff mbox series

[net-next] net: stmmac: dwmac-qcom-ethqos: Update link clock rate only for RGMII

Message ID 20240222125517.3356-1-quic_sarohasa@quicinc.com
State New
Headers
Series [net-next] net: stmmac: dwmac-qcom-ethqos: Update link clock rate only for RGMII |

Commit Message

Sarosh Hasan Feb. 22, 2024, 12:55 p.m. UTC 
  Updating link clock rate for different speeds is only needed when
using RGMII, as that mode requires changing clock speed when the link
speed changes. Let's restrict updating the link clock speed in
ethqos_update_link_clk() to just RGMII. Other modes such as SGMII
only need to enable the link clock (which is already done in probe).

Signed-off-by: Sarosh Hasan <quic_sarohasa@quicinc.com>
---
 .../stmicro/stmmac/dwmac-qcom-ethqos.c        | 26 ++++++++++---------
 1 file changed, 14 insertions(+), 12 deletions(-)

Comments

Konrad Dybcio Feb. 22, 2024, 1:01 p.m. UTC | #1 
On 2/22/24 13:55, Sarosh Hasan wrote:
> Updating link clock rate for different speeds is only needed when
> using RGMII, as that mode requires changing clock speed when the link
> speed changes. Let's restrict updating the link clock speed in
> ethqos_update_link_clk() to just RGMII. Other modes such as SGMII
> only need to enable the link clock (which is already done in probe).
> 
> Signed-off-by: Sarosh Hasan <quic_sarohasa@quicinc.com>
> ---
>   .../stmicro/stmmac/dwmac-qcom-ethqos.c        | 26 ++++++++++---------
>   1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> index 31631e3f89d0..9cd144fb3005 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> @@ -169,21 +169,23 @@ static void rgmii_dump(void *priv)
>   static void
>   ethqos_update_link_clk(struct qcom_ethqos *ethqos, unsigned int speed)
>   {
> -	switch (speed) {
> -	case SPEED_1000:
> -		ethqos->link_clk_rate =  RGMII_1000_NOM_CLK_FREQ;
> -		break;
> +	if (phy_interface_mode_is_rgmii(ethqos->phy_mode)) {
> +		switch (speed) {
> +		case SPEED_1000:
> +			ethqos->link_clk_rate =  RGMII_1000_NOM_CLK_FREQ;
> +			break;
>   
> -	case SPEED_100:
> -		ethqos->link_clk_rate =  RGMII_ID_MODE_100_LOW_SVS_CLK_FREQ;
> -		break;
> +		case SPEED_100:
> +			ethqos->link_clk_rate =  RGMII_ID_MODE_100_LOW_SVS_CLK_FREQ;
> +			break;
>   
> -	case SPEED_10:
> -		ethqos->link_clk_rate =  RGMII_ID_MODE_10_LOW_SVS_CLK_FREQ;
> -		break;
> -	}
> +		case SPEED_10:
> +			ethqos->link_clk_rate =  RGMII_ID_MODE_10_LOW_SVS_CLK_FREQ;
> +			break;
> +		}
>   
> -	clk_set_rate(ethqos->link_clk, ethqos->link_clk_rate);
> +		clk_set_rate(ethqos->link_clk, ethqos->link_clk_rate);
> +	}
>   }

if (!phy_interface_mode_is_rgmii(ethqos->phy_mode))
	return 0;

[leave the rest unchanged]

?

Konrad
Andrew Lunn Feb. 26, 2024, 11:05 p.m. UTC | #2 
> >   static void
> >   ethqos_update_link_clk(struct qcom_ethqos *ethqos, unsigned int speed)
> >   {
> > -	switch (speed) {
> > -	case SPEED_1000:
> > -		ethqos->link_clk_rate =  RGMII_1000_NOM_CLK_FREQ;
> > -		break;
> > +	if (phy_interface_mode_is_rgmii(ethqos->phy_mode)) {
> > +		switch (speed) {
> > +		case SPEED_1000:
> > +			ethqos->link_clk_rate =  RGMII_1000_NOM_CLK_FREQ;
> > +			break;
> > -	case SPEED_100:
> > -		ethqos->link_clk_rate =  RGMII_ID_MODE_100_LOW_SVS_CLK_FREQ;
> > -		break;
> > +		case SPEED_100:
> > +			ethqos->link_clk_rate =  RGMII_ID_MODE_100_LOW_SVS_CLK_FREQ;
> > +			break;
> > -	case SPEED_10:
> > -		ethqos->link_clk_rate =  RGMII_ID_MODE_10_LOW_SVS_CLK_FREQ;
> > -		break;
> > -	}
> > +		case SPEED_10:
> > +			ethqos->link_clk_rate =  RGMII_ID_MODE_10_LOW_SVS_CLK_FREQ;
> > +			break;
> > +		}
> > -	clk_set_rate(ethqos->link_clk, ethqos->link_clk_rate);
> > +		clk_set_rate(ethqos->link_clk, ethqos->link_clk_rate);
> > +	}
> >   }
> 
> if (!phy_interface_mode_is_rgmii(ethqos->phy_mode))
> 	return 0;

It is a void function, so no 0, but otherwise this does look less
invasive.

   Andrew
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index 31631e3f89d0..9cd144fb3005 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -169,21 +169,23 @@  static void rgmii_dump(void *priv)
 static void
 ethqos_update_link_clk(struct qcom_ethqos *ethqos, unsigned int speed)
 {
-	switch (speed) {
-	case SPEED_1000:
-		ethqos->link_clk_rate =  RGMII_1000_NOM_CLK_FREQ;
-		break;
+	if (phy_interface_mode_is_rgmii(ethqos->phy_mode)) {
+		switch (speed) {
+		case SPEED_1000:
+			ethqos->link_clk_rate =  RGMII_1000_NOM_CLK_FREQ;
+			break;
 
-	case SPEED_100:
-		ethqos->link_clk_rate =  RGMII_ID_MODE_100_LOW_SVS_CLK_FREQ;
-		break;
+		case SPEED_100:
+			ethqos->link_clk_rate =  RGMII_ID_MODE_100_LOW_SVS_CLK_FREQ;
+			break;
 
-	case SPEED_10:
-		ethqos->link_clk_rate =  RGMII_ID_MODE_10_LOW_SVS_CLK_FREQ;
-		break;
-	}
+		case SPEED_10:
+			ethqos->link_clk_rate =  RGMII_ID_MODE_10_LOW_SVS_CLK_FREQ;
+			break;
+		}
 
-	clk_set_rate(ethqos->link_clk, ethqos->link_clk_rate);
+		clk_set_rate(ethqos->link_clk, ethqos->link_clk_rate);
+	}
 }
 
 static void ethqos_set_func_clk_en(struct qcom_ethqos *ethqos)