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

[Alexander Kochetkov]

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 at 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 at 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:
> 
>