diff mbox series

[2/2] rockchip: clk: rk3188: enable bwadj for rk3188 DPLL

Message ID 20200622131710.31168-3-al.kochet@gmail.com
State Changes Requested
Delegated to: Kever Yang
Headers show
Series v2: rr3188: change APP to 600MHz and enable bwadj for DPLL | expand

Commit Message

Alexander Kochetkov June 22, 2020, 1:17 p.m. UTC
Empirically, I found that DPLL on rk3188 has bwadj registers.
Configuring DPLL with bwadj increase DPLL stability. Because
of DPLL provide clock for ethernet, enabling bwaj reduces
the number of errors on the ethernet.

Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
 drivers/clk/rockchip/clk_rk3188.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Kever Yang June 27, 2020, 3:17 p.m. UTC | #1
Hi Alex,

     I think it will be better to update the rk3188_clk_probe() function 
instead of

what you have modified if the RK3188 and RK3188A has the same PLL(I'm 
not sure

about it now).


Thanks,

- Kever

On 2020/6/22 下午9:17, Alexander Kochetkov wrote:
> Empirically, I found that DPLL on rk3188 has bwadj registers.
> Configuring DPLL with bwadj increase DPLL stability. Because
> of DPLL provide clock for ethernet, enabling bwaj reduces
> the number of errors on the ethernet.
>
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> ---
>   drivers/clk/rockchip/clk_rk3188.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/rockchip/clk_rk3188.c b/drivers/clk/rockchip/clk_rk3188.c
> index 4fc5c78563..ee5782d25d 100644
> --- a/drivers/clk/rockchip/clk_rk3188.c
> +++ b/drivers/clk/rockchip/clk_rk3188.c
> @@ -117,7 +117,7 @@ static int rkclk_set_pll(struct rk3188_cru *cru, enum rk_clk_id clk_id,
>   }
>   
>   static int rkclk_configure_ddr(struct rk3188_cru *cru, struct rk3188_grf *grf,
> -			       unsigned int hz, bool has_bwadj)
> +			       unsigned int hz)
>   {
>   	static const struct pll_div dpll_cfg[] = {
>   		{.nf = 75, .nr = 1, .no = 6},
> @@ -149,7 +149,8 @@ static int rkclk_configure_ddr(struct rk3188_cru *cru, struct rk3188_grf *grf,
>   	rk_clrsetreg(&cru->cru_mode_con, DPLL_MODE_MASK << DPLL_MODE_SHIFT,
>   		     DPLL_MODE_SLOW << DPLL_MODE_SHIFT);
>   
> -	rkclk_set_pll(cru, CLK_DDR, &dpll_cfg[cfg], has_bwadj);
> +	/* rk3188 and rk3188a DPLL has bwadj */
> +	rkclk_set_pll(cru, CLK_DDR, &dpll_cfg[cfg], 1);
>   
>   	/* wait for pll lock */
>   	while (!(readl(&grf->soc_status0) & SOCSTS_DPLL_LOCK))
> @@ -504,8 +505,7 @@ static ulong rk3188_clk_set_rate(struct clk *clk, ulong rate)
>   					       priv->has_bwadj);
>   		break;
>   	case CLK_DDR:
> -		new_rate = rkclk_configure_ddr(priv->cru, priv->grf, rate,
> -					       priv->has_bwadj);
> +		new_rate = rkclk_configure_ddr(priv->cru, priv->grf, rate);
>   		break;
>   	case HCLK_EMMC:
>   	case HCLK_SDMMC:
Alexander Kochetkov June 27, 2020, 3:58 p.m. UTC | #2
Hi Kever,

Strange… Then I tested a year ago I saw, that writing into bwadj registers had no effect
for some PLLs. But now I did another test. See patch and output. Looks like rk3188 allow
writing into bwadj fields.


So I do something like 'priv->has_bwadj = 1'  in the rk3188_clk_probe() and send try 2.

--------------------------------------
--- a/drivers/clk/rockchip/clk_rk3188.c
+++ b/drivers/clk/rockchip/clk_rk3188.c
@@ -91,7 +91,7 @@ static int rkclk_set_pll(struct rk3188_cru *cru, enum rk_clk_id clk_id,
        uint vco_hz = OSC_HZ / 1000 * div->nf / div->nr * 1000;
        uint output_hz = vco_hz / div->no;
 
-       debug("PLL at %x: nf=%d, nr=%d, no=%d, vco=%u Hz, output=%u Hz\n",
+       printf("PLL at %x: nf=%d, nr=%d, no=%d, vco=%u Hz, output=%u Hz\n",
              (uint)pll, div->nf, div->nr, div->no, vco_hz, output_hz);
        assert(vco_hz >= VCO_MIN_HZ && vco_hz <= VCO_MAX_HZ &&
               output_hz >= OUTPUT_MIN_HZ && output_hz <= OUTPUT_MAX_HZ &&
@@ -105,8 +105,12 @@ static int rkclk_set_pll(struct rk3188_cru *cru, enum rk_clk_id clk_id,
                     ((div->nr - 1) << CLKR_SHIFT) | (div->no - 1));
        rk_clrsetreg(&pll->con1, CLKF_MASK, div->nf - 1);
 
-       if (has_bwadj)
+       if (has_bwadj) {
+               printf("before: %p: con2=%x, clr=%x, set=%x\n",
+                       &pll->con2, readl(&pll->con2), PLL_BWADJ_MASK, (div->nf >> 1) - 1);
                rk_clrsetreg(&pll->con2, PLL_BWADJ_MASK, (div->nf >> 1) - 1);
+               printf("after: %p: con2=%x\n", &pll->con2, readl(&pll->con2));
+       }
 
        udelay(10);
 
@@ -552,7 +556,9 @@ static int rk3188_clk_probe(struct udevice *dev)
        priv->grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
        if (IS_ERR(priv->grf))
                return PTR_ERR(priv->grf);
-       priv->has_bwadj = (type == RK3188A_CRU) ? 1 : 0;
+       printf("type %d\n", type);
+       //priv->has_bwadj = (type == RK3188A_CRU) ? 1 : 0;
+       priv->has_bwadj = 1;
 —————————————————————
type 0
PLL at 20000030: nf=64, nr=1, no=2, vco=1536000000 Hz, output=768000000 Hz
before: 20000038: con2=94, clr=fff, set=1f
after: 20000038: con2=1f
PLL at 20000020: nf=32, nr=1, no=2, vco=768000000 Hz, output=384000000 Hz
before: 20000028: con2=63, clr=fff, set=f
after: 20000028: con2=f
PLL at 20000000: nf=50, nr=1, no=2, vco=1200000000 Hz, output=600000000 Hz
before: 20000008: con2=18, clr=fff, set=18
after: 20000008: con2=18
PLL at 20000010: nf=75, nr=1, no=4, vco=1800000000 Hz, output=450000000 Hz
before: 20000018: con2=10b, clr=fff, set=24
after: 20000018: con2=24



> 27 июня 2020 г., в 18:17, Kever Yang <kever.yang@rock-chips.com> написал(а):
> 
> Hi Alex,
> 
>     I think it will be better to update the rk3188_clk_probe() function instead of
> 
> what you have modified if the RK3188 and RK3188A has the same PLL(I'm not sure
> 
> about it now).
> 
> 
> Thanks,
> 
> - Kever
> 
> On 2020/6/22 下午9:17, Alexander Kochetkov wrote:
>> Empirically, I found that DPLL on rk3188 has bwadj registers.
>> Configuring DPLL with bwadj increase DPLL stability. Because
>> of DPLL provide clock for ethernet, enabling bwaj reduces
>> the number of errors on the ethernet.
>> 
>> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
>> ---
>>  drivers/clk/rockchip/clk_rk3188.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/clk/rockchip/clk_rk3188.c b/drivers/clk/rockchip/clk_rk3188.c
>> index 4fc5c78563..ee5782d25d 100644
>> --- a/drivers/clk/rockchip/clk_rk3188.c
>> +++ b/drivers/clk/rockchip/clk_rk3188.c
>> @@ -117,7 +117,7 @@ static int rkclk_set_pll(struct rk3188_cru *cru, enum rk_clk_id clk_id,
>>  }
>>    static int rkclk_configure_ddr(struct rk3188_cru *cru, struct rk3188_grf *grf,
>> -			       unsigned int hz, bool has_bwadj)
>> +			       unsigned int hz)
>>  {
>>  	static const struct pll_div dpll_cfg[] = {
>>  		{.nf = 75, .nr = 1, .no = 6},
>> @@ -149,7 +149,8 @@ static int rkclk_configure_ddr(struct rk3188_cru *cru, struct rk3188_grf *grf,
>>  	rk_clrsetreg(&cru->cru_mode_con, DPLL_MODE_MASK << DPLL_MODE_SHIFT,
>>  		     DPLL_MODE_SLOW << DPLL_MODE_SHIFT);
>>  -	rkclk_set_pll(cru, CLK_DDR, &dpll_cfg[cfg], has_bwadj);
>> +	/* rk3188 and rk3188a DPLL has bwadj */
>> +	rkclk_set_pll(cru, CLK_DDR, &dpll_cfg[cfg], 1);
>>    	/* wait for pll lock */
>>  	while (!(readl(&grf->soc_status0) & SOCSTS_DPLL_LOCK))
>> @@ -504,8 +505,7 @@ static ulong rk3188_clk_set_rate(struct clk *clk, ulong rate)
>>  					       priv->has_bwadj);
>>  		break;
>>  	case CLK_DDR:
>> -		new_rate = rkclk_configure_ddr(priv->cru, priv->grf, rate,
>> -					       priv->has_bwadj);
>> +		new_rate = rkclk_configure_ddr(priv->cru, priv->grf, rate);
>>  		break;
>>  	case HCLK_EMMC:
>>  	case HCLK_SDMMC:
> 
>
diff mbox series

Patch

diff --git a/drivers/clk/rockchip/clk_rk3188.c b/drivers/clk/rockchip/clk_rk3188.c
index 4fc5c78563..ee5782d25d 100644
--- a/drivers/clk/rockchip/clk_rk3188.c
+++ b/drivers/clk/rockchip/clk_rk3188.c
@@ -117,7 +117,7 @@  static int rkclk_set_pll(struct rk3188_cru *cru, enum rk_clk_id clk_id,
 }
 
 static int rkclk_configure_ddr(struct rk3188_cru *cru, struct rk3188_grf *grf,
-			       unsigned int hz, bool has_bwadj)
+			       unsigned int hz)
 {
 	static const struct pll_div dpll_cfg[] = {
 		{.nf = 75, .nr = 1, .no = 6},
@@ -149,7 +149,8 @@  static int rkclk_configure_ddr(struct rk3188_cru *cru, struct rk3188_grf *grf,
 	rk_clrsetreg(&cru->cru_mode_con, DPLL_MODE_MASK << DPLL_MODE_SHIFT,
 		     DPLL_MODE_SLOW << DPLL_MODE_SHIFT);
 
-	rkclk_set_pll(cru, CLK_DDR, &dpll_cfg[cfg], has_bwadj);
+	/* rk3188 and rk3188a DPLL has bwadj */
+	rkclk_set_pll(cru, CLK_DDR, &dpll_cfg[cfg], 1);
 
 	/* wait for pll lock */
 	while (!(readl(&grf->soc_status0) & SOCSTS_DPLL_LOCK))
@@ -504,8 +505,7 @@  static ulong rk3188_clk_set_rate(struct clk *clk, ulong rate)
 					       priv->has_bwadj);
 		break;
 	case CLK_DDR:
-		new_rate = rkclk_configure_ddr(priv->cru, priv->grf, rate,
-					       priv->has_bwadj);
+		new_rate = rkclk_configure_ddr(priv->cru, priv->grf, rate);
 		break;
 	case HCLK_EMMC:
 	case HCLK_SDMMC: