[U-Boot] [PATCH] sunxi: Fix H3 DRAM impedance calibration on rev. A chips

[Jens Kuske]

On 30.09.2016 14:24, Jagan Teki wrote:
> On Wed, Sep 21, 2016 at 11:38 PM, Jens Kuske <jenskuske at gmail.com> wrote:
>> H3 seems to have a silicon bug breaking the impedance calibration.
>> This is currently worked around in software by multiple steps
>> combining the results to replace the wrong values.
>>
>> Revision A chips need a different workaround, which is present in
>> the vendor bootloader too, but got overlooked in lack of
>> information and affected boards till now.
>> This commit adds a simplified version without correction factor,
>> which would be 1.00 for all known boards anyway.
>>
>> Signed-off-by: Jens Kuske <jenskuske at gmail.com>
>> ---
>>
>> Hi,
>>
>> This has been tested by an Armbian user:
>> http://forum.armbian.com/index.php/topic/872-beelink-x2-with-armbian-possible/?p=16061
>>
>> It looks like only few boards have revision A chips, probably only
>> non-development boards, otherwise we would have found this earlier.
>> The idea that these are different chip revision is based on:
>> https://github.com/igorpecovnik/linux/blob/sun8i/arch/arm/mach-sunxi/sunxi-chip.c#L341
>>
>> Regards,
>> Jens
>>
>> ---
>>  arch/arm/mach-sunxi/dram_sun8i_h3.c | 64 +++++++++++++++++++++++++------------
>>  1 file changed, 43 insertions(+), 21 deletions(-)
>>
>> diff --git a/arch/arm/mach-sunxi/dram_sun8i_h3.c b/arch/arm/mach-sunxi/dram_sun8i_h3.c
>> index 2020d75..b08b8e6 100644
>> --- a/arch/arm/mach-sunxi/dram_sun8i_h3.c
>> +++ b/arch/arm/mach-sunxi/dram_sun8i_h3.c
>> @@ -217,35 +217,57 @@ static void mctl_zq_calibration(struct dram_para *para)
>>         struct sunxi_mctl_ctl_reg * const mctl_ctl =
>>                         (struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;
>>
>> -       int i;
>> -       u16 zq_val[6];
>> -       u8 val;
>> +       if ((readl(SUNXI_SRAMC_BASE + 0x24) & 0xff) == 0 &&
>> +           (readl(SUNXI_SRAMC_BASE + 0xf0) & 0x1) == 0) {
> 
> I think we can unlock before reading the sram version, do you think
> so? and lock same after.

We don't have to unlock to read this field, it's even documented in the
datasheet. Only to read the full version in bits 31:16 it has to be
unlocked. Or what did you mean?

> 
>> +               u32 reg_val;
>>
>> -       writel(0x0a0a0a0a, &mctl_ctl->zqdr[2]);
>> -
>> -       for (i = 0; i < 6; i++) {
>> -               u8 zq = (CONFIG_DRAM_ZQ >> (i * 4)) & 0xf;
>> -
>> -               writel((zq << 20) | (zq << 16) | (zq << 12) |
>> -                               (zq << 8) | (zq << 4) | (zq << 0),
>> -                               &mctl_ctl->zqcr);
>> +               clrsetbits_le32(&mctl_ctl->zqcr, 0xffff,
>> +                               CONFIG_DRAM_ZQ & 0xffff);
>>
>>                 writel(PIR_CLRSR, &mctl_ctl->pir);
>>                 mctl_phy_init(PIR_ZCAL);
>>
>> -               zq_val[i] = readl(&mctl_ctl->zqdr[0]) & 0xff;
>> -               writel(REPEAT_BYTE(zq_val[i]), &mctl_ctl->zqdr[2]);
>> +               reg_val = readl(&mctl_ctl->zqdr[0]);
>> +               reg_val &= (0x1f << 16) | (0x1f << 0);
>> +               reg_val |= reg_val << 8;
>> +               writel(reg_val, &mctl_ctl->zqdr[0]);
>>
>> -               writel(PIR_CLRSR, &mctl_ctl->pir);
>> -               mctl_phy_init(PIR_ZCAL);
>> +               reg_val = readl(&mctl_ctl->zqdr[1]);
>> +               reg_val &= (0x1f << 16) | (0x1f << 0);
>> +               reg_val |= reg_val << 8;
>> +               writel(reg_val, &mctl_ctl->zqdr[1]);
>> +               writel(reg_val, &mctl_ctl->zqdr[2]);
> 
> Was this write call for zqdr[2] true?

Yes, it is correct, calibration for zqdr[2] doesn't work at all, so
zqdr[1] gets copied. Both are for the data lanes, so this is reasonable.

> 
> thanks!
>