All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] sunxi: Fix H3 DRAM impedance calibration on rev. A chips
@ 2016-09-21 18:08 Jens Kuske
  2016-09-30 12:24 ` Jagan Teki
  2016-10-10  8:09 ` Hans de Goede
  0 siblings, 2 replies; 7+ messages in thread
From: Jens Kuske @ 2016-09-21 18:08 UTC (permalink / raw)
  To: u-boot

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@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) {
+		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]);
+	} else {
+		int i;
+		u16 zq_val[6];
+		u8 val;
 
-		val = readl(&mctl_ctl->zqdr[0]) >> 24;
-		zq_val[i] |= bin_to_mgray(mgray_to_bin(val) - 1) << 8;
-	}
+		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);
 
-	writel((zq_val[1] << 16) | zq_val[0], &mctl_ctl->zqdr[0]);
-	writel((zq_val[3] << 16) | zq_val[2], &mctl_ctl->zqdr[1]);
-	writel((zq_val[5] << 16) | zq_val[4], &mctl_ctl->zqdr[2]);
+			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]);
+
+			writel(PIR_CLRSR, &mctl_ctl->pir);
+			mctl_phy_init(PIR_ZCAL);
+
+			val = readl(&mctl_ctl->zqdr[0]) >> 24;
+			zq_val[i] |= bin_to_mgray(mgray_to_bin(val) - 1) << 8;
+		}
+
+		writel((zq_val[1] << 16) | zq_val[0], &mctl_ctl->zqdr[0]);
+		writel((zq_val[3] << 16) | zq_val[2], &mctl_ctl->zqdr[1]);
+		writel((zq_val[5] << 16) | zq_val[4], &mctl_ctl->zqdr[2]);
+	}
 }
 
 static void mctl_set_cr(struct dram_para *para)
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH] sunxi: Fix H3 DRAM impedance calibration on rev. A chips
  2016-09-21 18:08 [U-Boot] [PATCH] sunxi: Fix H3 DRAM impedance calibration on rev. A chips Jens Kuske
@ 2016-09-30 12:24 ` Jagan Teki
  2016-09-30 12:50   ` Jens Kuske
  2016-10-10  8:09 ` Hans de Goede
  1 sibling, 1 reply; 7+ messages in thread
From: Jagan Teki @ 2016-09-30 12:24 UTC (permalink / raw)
  To: u-boot

On Wed, Sep 21, 2016 at 11:38 PM, Jens Kuske <jenskuske@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@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.

> +               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?

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH] sunxi: Fix H3 DRAM impedance calibration on rev. A chips
  2016-09-30 12:24 ` Jagan Teki
@ 2016-09-30 12:50   ` Jens Kuske
  2016-09-30 13:17     ` Jagan Teki
  0 siblings, 1 reply; 7+ messages in thread
From: Jens Kuske @ 2016-09-30 12:50 UTC (permalink / raw)
  To: u-boot

On 30.09.2016 14:24, Jagan Teki wrote:
> On Wed, Sep 21, 2016 at 11:38 PM, Jens Kuske <jenskuske@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@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!
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH] sunxi: Fix H3 DRAM impedance calibration on rev. A chips
  2016-09-30 12:50   ` Jens Kuske
@ 2016-09-30 13:17     ` Jagan Teki
  2016-09-30 14:25       ` Jens Kuske
  0 siblings, 1 reply; 7+ messages in thread
From: Jagan Teki @ 2016-09-30 13:17 UTC (permalink / raw)
  To: u-boot

On Fri, Sep 30, 2016 at 6:20 PM, Jens Kuske <jenskuske@gmail.com> wrote:
> On 30.09.2016 14:24, Jagan Teki wrote:
>> On Wed, Sep 21, 2016 at 11:38 PM, Jens Kuske <jenskuske@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@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?

This code is detecting H3 rev.A chip, correct? I'm thinking we need to
detect the H3(0x1680) first and check for rev.A for safety. Can you
point me where it documented, unable to find the same.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH] sunxi: Fix H3 DRAM impedance calibration on rev. A chips
  2016-09-30 13:17     ` Jagan Teki
@ 2016-09-30 14:25       ` Jens Kuske
  2016-09-30 20:46         ` Jagan Teki
  0 siblings, 1 reply; 7+ messages in thread
From: Jens Kuske @ 2016-09-30 14:25 UTC (permalink / raw)
  To: u-boot

On 30.09.2016 15:17, Jagan Teki wrote:
> On Fri, Sep 30, 2016 at 6:20 PM, Jens Kuske <jenskuske@gmail.com> wrote:
>> On 30.09.2016 14:24, Jagan Teki wrote:
>>> On Wed, Sep 21, 2016 at 11:38 PM, Jens Kuske <jenskuske@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@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?
> 
> This code is detecting H3 rev.A chip, correct? I'm thinking we need to
> detect the H3(0x1680) first and check for rev.A for safety.

All code in this file must only be run on H3, otherwise everything else
will fail too. So we don't have extra checks for that anywhere, the SoC
version is a compiletime option for all sunxi code.

> Can you
> point me where it documented, unable to find the same.

H3 Datasheet v1.2 page 152 (VER_BITS), but only the 0x24, 0xf0 is from
https://github.com/igorpecovnik/linux/blob/sun8i/arch/arm/mach-sunxi/sunxi-chip.c#L338-L341

> 
> thanks!
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH] sunxi: Fix H3 DRAM impedance calibration on rev. A chips
  2016-09-30 14:25       ` Jens Kuske
@ 2016-09-30 20:46         ` Jagan Teki
  0 siblings, 0 replies; 7+ messages in thread
From: Jagan Teki @ 2016-09-30 20:46 UTC (permalink / raw)
  To: u-boot

On Fri, Sep 30, 2016 at 7:55 PM, Jens Kuske <jenskuske@gmail.com> wrote:
> On 30.09.2016 15:17, Jagan Teki wrote:
>> On Fri, Sep 30, 2016 at 6:20 PM, Jens Kuske <jenskuske@gmail.com> wrote:
>>> On 30.09.2016 14:24, Jagan Teki wrote:
>>>> On Wed, Sep 21, 2016 at 11:38 PM, Jens Kuske <jenskuske@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@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?
>>
>> This code is detecting H3 rev.A chip, correct? I'm thinking we need to
>> detect the H3(0x1680) first and check for rev.A for safety.
>
> All code in this file must only be run on H3, otherwise everything else
> will fail too. So we don't have extra checks for that anywhere, the SoC
> version is a compiletime option for all sunxi code.

OK.

>
>> Can you
>> point me where it documented, unable to find the same.
>
> H3 Datasheet v1.2 page 152 (VER_BITS), but only the 0x24, 0xf0 is from
> https://github.com/igorpecovnik/linux/blob/sun8i/arch/arm/mach-sunxi/sunxi-chip.c#L338-L341

Thanks.

Reviewed-by: Jagan Teki <jteki@openedev.com>

-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH] sunxi: Fix H3 DRAM impedance calibration on rev. A chips
  2016-09-21 18:08 [U-Boot] [PATCH] sunxi: Fix H3 DRAM impedance calibration on rev. A chips Jens Kuske
  2016-09-30 12:24 ` Jagan Teki
@ 2016-10-10  8:09 ` Hans de Goede
  1 sibling, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2016-10-10  8:09 UTC (permalink / raw)
  To: u-boot

Hi,

On 21-09-16 20:08, Jens Kuske 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@gmail.com>

Thank you for the patch. I've applied this patch to my local
tree and it will be part of the pull-request I'm about to send
out.

Regards,

Hans




> ---
>
> 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) {
> +		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]);
> +	} else {
> +		int i;
> +		u16 zq_val[6];
> +		u8 val;
>
> -		val = readl(&mctl_ctl->zqdr[0]) >> 24;
> -		zq_val[i] |= bin_to_mgray(mgray_to_bin(val) - 1) << 8;
> -	}
> +		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);
>
> -	writel((zq_val[1] << 16) | zq_val[0], &mctl_ctl->zqdr[0]);
> -	writel((zq_val[3] << 16) | zq_val[2], &mctl_ctl->zqdr[1]);
> -	writel((zq_val[5] << 16) | zq_val[4], &mctl_ctl->zqdr[2]);
> +			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]);
> +
> +			writel(PIR_CLRSR, &mctl_ctl->pir);
> +			mctl_phy_init(PIR_ZCAL);
> +
> +			val = readl(&mctl_ctl->zqdr[0]) >> 24;
> +			zq_val[i] |= bin_to_mgray(mgray_to_bin(val) - 1) << 8;
> +		}
> +
> +		writel((zq_val[1] << 16) | zq_val[0], &mctl_ctl->zqdr[0]);
> +		writel((zq_val[3] << 16) | zq_val[2], &mctl_ctl->zqdr[1]);
> +		writel((zq_val[5] << 16) | zq_val[4], &mctl_ctl->zqdr[2]);
> +	}
>  }
>
>  static void mctl_set_cr(struct dram_para *para)
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-10-10  8:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-21 18:08 [U-Boot] [PATCH] sunxi: Fix H3 DRAM impedance calibration on rev. A chips Jens Kuske
2016-09-30 12:24 ` Jagan Teki
2016-09-30 12:50   ` Jens Kuske
2016-09-30 13:17     ` Jagan Teki
2016-09-30 14:25       ` Jens Kuske
2016-09-30 20:46         ` Jagan Teki
2016-10-10  8:09 ` Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.