diff mbox series

[v4,06/39] clock/qcom: qcs404: fix clk_set_rate

Message ID 20240215-b4-qcom-common-target-v4-6-ed06355c634a@linaro.org
State Accepted
Commit 641237bf99f701ffdf3748f5fe1e118eb2d7b781
Headers show
Series Qualcomm generic board support | expand

Commit Message

Caleb Connolly Feb. 15, 2024, 8:52 p.m. UTC
We should be returning the rate that we set the clock to, drivers like
MMC rely on this. So fix it.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 drivers/clk/qcom/clock-qcs404.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

Comments

Neil Armstrong Feb. 19, 2024, 9:46 a.m. UTC | #1
On 15/02/2024 21:52, Caleb Connolly wrote:
> We should be returning the rate that we set the clock to, drivers like
> MMC rely on this. So fix it.
> 
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>   drivers/clk/qcom/clock-qcs404.c | 25 +++++++++++++------------
>   1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/clk/qcom/clock-qcs404.c b/drivers/clk/qcom/clock-qcs404.c
> index f5b352803927..958312b88842 100644
> --- a/drivers/clk/qcom/clock-qcs404.c
> +++ b/drivers/clk/qcom/clock-qcs404.c
> @@ -193,24 +193,18 @@ static ulong qcs404_clk_set_rate(struct clk *clk, ulong rate)
>   
>   	switch (clk->id) {
>   	case GCC_BLSP1_UART2_APPS_CLK:
> -		/* UART: 115200 */
> +		/* UART: 1843200Hz for a fixed 115200 baudrate (19200000 * (12/125)) */
>   		clk_rcg_set_rate_mnd(priv->base, &uart2_regs, 0, 12, 125,
>   				     CFG_CLK_SRC_CXO, 16);
>   		clk_enable_cbc(priv->base + BLSP1_UART2_APPS_CBCR);
> -		break;
> -	case GCC_BLSP1_AHB_CLK:
> -		clk_enable_vote_clk(priv->base, &gcc_blsp1_ahb_clk);
> -		break;
> +		return 1843200;
>   	case GCC_SDCC1_APPS_CLK:
>   		/* SDCC1: 200MHz */
>   		clk_rcg_set_rate_mnd(priv->base, &sdc_regs, 7, 0, 0,
>   				     CFG_CLK_SRC_GPLL0, 8);
>   		clk_enable_gpll0(priv->base, &gpll0_vote_clk);
>   		clk_enable_cbc(priv->base + SDCC_APPS_CBCR(1));
> -		break;
> -	case GCC_SDCC1_AHB_CLK:
> -		clk_enable_cbc(priv->base + SDCC_AHB_CBCR(1));
> -		break;
> +		return rate;
>   	case GCC_ETH_RGMII_CLK:
>   		if (rate == 250000000)
>   			clk_rcg_set_rate_mnd(priv->base, &emac_regs, 3, 0, 0,
> @@ -224,11 +218,15 @@ static ulong qcs404_clk_set_rate(struct clk *clk, ulong rate)
>   		else if (rate == 5000000)
>   			clk_rcg_set_rate_mnd(priv->base, &emac_regs, 3, 1, 50,
>   					     CFG_CLK_SRC_GPLL1, 8);
> -		break;
> -	default:
> -		return 0;
> +		return rate;
>   	}
>   
> +	/* There is a bug only seeming to affect this board where the MMC driver somehow calls
> +	 * clk_set_rate() on a clock with id 0 which is associated with the qcom_clk device.
> +	 * The only clock with ID 0 is the xo_board clock which should not be associated with
> +	 * this device...
> +	 */
> +	log_debug("Unknown clock id %ld\n", clk->id);
>   	return 0;
>   }
>   
> @@ -305,6 +303,9 @@ static int qcs404_clk_enable(struct clk *clk)
>   		clk_rcg_set_rate(priv->base, &blsp1_qup4_i2c_apps_regs, 0,
>   				 CFG_CLK_SRC_CXO);
>   		break;
> +	case GCC_SDCC1_AHB_CLK:
> +		clk_enable_cbc(priv->base + SDCC_AHB_CBCR(1));
> +		break;
>   	default:
>   		return 0;
>   	}
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Sumit Garg Feb. 20, 2024, 6:02 a.m. UTC | #2
On Fri, 16 Feb 2024 at 02:22, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> We should be returning the rate that we set the clock to, drivers like
> MMC rely on this. So fix it.
>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>  drivers/clk/qcom/clock-qcs404.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
>

Reviewed-by: Sumit Garg <sumit.garg@linaro.org>

-Sumit

> diff --git a/drivers/clk/qcom/clock-qcs404.c b/drivers/clk/qcom/clock-qcs404.c
> index f5b352803927..958312b88842 100644
> --- a/drivers/clk/qcom/clock-qcs404.c
> +++ b/drivers/clk/qcom/clock-qcs404.c
> @@ -193,24 +193,18 @@ static ulong qcs404_clk_set_rate(struct clk *clk, ulong rate)
>
>         switch (clk->id) {
>         case GCC_BLSP1_UART2_APPS_CLK:
> -               /* UART: 115200 */
> +               /* UART: 1843200Hz for a fixed 115200 baudrate (19200000 * (12/125)) */
>                 clk_rcg_set_rate_mnd(priv->base, &uart2_regs, 0, 12, 125,
>                                      CFG_CLK_SRC_CXO, 16);
>                 clk_enable_cbc(priv->base + BLSP1_UART2_APPS_CBCR);
> -               break;
> -       case GCC_BLSP1_AHB_CLK:
> -               clk_enable_vote_clk(priv->base, &gcc_blsp1_ahb_clk);
> -               break;
> +               return 1843200;
>         case GCC_SDCC1_APPS_CLK:
>                 /* SDCC1: 200MHz */
>                 clk_rcg_set_rate_mnd(priv->base, &sdc_regs, 7, 0, 0,
>                                      CFG_CLK_SRC_GPLL0, 8);
>                 clk_enable_gpll0(priv->base, &gpll0_vote_clk);
>                 clk_enable_cbc(priv->base + SDCC_APPS_CBCR(1));
> -               break;
> -       case GCC_SDCC1_AHB_CLK:
> -               clk_enable_cbc(priv->base + SDCC_AHB_CBCR(1));
> -               break;
> +               return rate;
>         case GCC_ETH_RGMII_CLK:
>                 if (rate == 250000000)
>                         clk_rcg_set_rate_mnd(priv->base, &emac_regs, 3, 0, 0,
> @@ -224,11 +218,15 @@ static ulong qcs404_clk_set_rate(struct clk *clk, ulong rate)
>                 else if (rate == 5000000)
>                         clk_rcg_set_rate_mnd(priv->base, &emac_regs, 3, 1, 50,
>                                              CFG_CLK_SRC_GPLL1, 8);
> -               break;
> -       default:
> -               return 0;
> +               return rate;
>         }
>
> +       /* There is a bug only seeming to affect this board where the MMC driver somehow calls
> +        * clk_set_rate() on a clock with id 0 which is associated with the qcom_clk device.
> +        * The only clock with ID 0 is the xo_board clock which should not be associated with
> +        * this device...
> +        */
> +       log_debug("Unknown clock id %ld\n", clk->id);
>         return 0;
>  }
>
> @@ -305,6 +303,9 @@ static int qcs404_clk_enable(struct clk *clk)
>                 clk_rcg_set_rate(priv->base, &blsp1_qup4_i2c_apps_regs, 0,
>                                  CFG_CLK_SRC_CXO);
>                 break;
> +       case GCC_SDCC1_AHB_CLK:
> +               clk_enable_cbc(priv->base + SDCC_AHB_CBCR(1));
> +               break;
>         default:
>                 return 0;
>         }
>
> --
> 2.43.1
>
diff mbox series

Patch

diff --git a/drivers/clk/qcom/clock-qcs404.c b/drivers/clk/qcom/clock-qcs404.c
index f5b352803927..958312b88842 100644
--- a/drivers/clk/qcom/clock-qcs404.c
+++ b/drivers/clk/qcom/clock-qcs404.c
@@ -193,24 +193,18 @@  static ulong qcs404_clk_set_rate(struct clk *clk, ulong rate)
 
 	switch (clk->id) {
 	case GCC_BLSP1_UART2_APPS_CLK:
-		/* UART: 115200 */
+		/* UART: 1843200Hz for a fixed 115200 baudrate (19200000 * (12/125)) */
 		clk_rcg_set_rate_mnd(priv->base, &uart2_regs, 0, 12, 125,
 				     CFG_CLK_SRC_CXO, 16);
 		clk_enable_cbc(priv->base + BLSP1_UART2_APPS_CBCR);
-		break;
-	case GCC_BLSP1_AHB_CLK:
-		clk_enable_vote_clk(priv->base, &gcc_blsp1_ahb_clk);
-		break;
+		return 1843200;
 	case GCC_SDCC1_APPS_CLK:
 		/* SDCC1: 200MHz */
 		clk_rcg_set_rate_mnd(priv->base, &sdc_regs, 7, 0, 0,
 				     CFG_CLK_SRC_GPLL0, 8);
 		clk_enable_gpll0(priv->base, &gpll0_vote_clk);
 		clk_enable_cbc(priv->base + SDCC_APPS_CBCR(1));
-		break;
-	case GCC_SDCC1_AHB_CLK:
-		clk_enable_cbc(priv->base + SDCC_AHB_CBCR(1));
-		break;
+		return rate;
 	case GCC_ETH_RGMII_CLK:
 		if (rate == 250000000)
 			clk_rcg_set_rate_mnd(priv->base, &emac_regs, 3, 0, 0,
@@ -224,11 +218,15 @@  static ulong qcs404_clk_set_rate(struct clk *clk, ulong rate)
 		else if (rate == 5000000)
 			clk_rcg_set_rate_mnd(priv->base, &emac_regs, 3, 1, 50,
 					     CFG_CLK_SRC_GPLL1, 8);
-		break;
-	default:
-		return 0;
+		return rate;
 	}
 
+	/* There is a bug only seeming to affect this board where the MMC driver somehow calls
+	 * clk_set_rate() on a clock with id 0 which is associated with the qcom_clk device.
+	 * The only clock with ID 0 is the xo_board clock which should not be associated with
+	 * this device...
+	 */
+	log_debug("Unknown clock id %ld\n", clk->id);
 	return 0;
 }
 
@@ -305,6 +303,9 @@  static int qcs404_clk_enable(struct clk *clk)
 		clk_rcg_set_rate(priv->base, &blsp1_qup4_i2c_apps_regs, 0,
 				 CFG_CLK_SRC_CXO);
 		break;
+	case GCC_SDCC1_AHB_CLK:
+		clk_enable_cbc(priv->base + SDCC_AHB_CBCR(1));
+		break;
 	default:
 		return 0;
 	}