[PATCH v2] sunxi: H616: dram: fix LPDDR3 TRP6 parsing
Andre Przywara
andre.przywara at arm.com
Mon Apr 27 15:15:38 CEST 2026
Hi Paul,
On 4/10/26 17:54, Paul Kocialkowski wrote:
> Hi Andre,
>
> On Thu 09 Apr 26, 23:34, Andre Przywara wrote:
>> On Thu, 9 Apr 2026 17:10:19 +0200
>> Paul Kocialkowski <contact at paulk.fr> wrote:
>>
>> Hi Paul, Mikhail,
>>
>> thanks for the reply!
>>
>>> Hi,
>>>
>>> On Thu 09 Apr 26, 13:48, Mikhail Kalashnikov wrote:
>>>> On 4/8/26 00:47, Philippe Simons wrote:
>>>>> From: Jernej Skrabec <jernej.skrabec at gmail.com>
>>>>>
>>>>> Allwinner's BSP DRAM code uses parameter TPR6, presumably containing
>>>>> some "Vref" parameter, to encode the values for *all* four supported DRAM
>>>>> types. The code selects one byte based on the DRAM type used at runtime.
>>>>> To allow copying DRAM parameters from vendor firmware, we used this value
>>>>> and its encoding, but wrongly: the proper order of bytes is DDR3, DDR4,
>>>>> LPDDR3, LPDDR4, from LSB to MSB, cf. the A523 and A133 DRAM code.
>>>>>
>>>>> Correct the masking for LPDDR3 to fix DRAM operation on some boards
>>>>> using this DRAM type.
>>>>>
>>>>> With LPDDR3 TRP6 parsing fixed, adapt default DRAM_SUNXI_TPR6 value.
>>>>> Also change LPDDR4 default value to 0x38 used by A523 boards.
>>>>>
>>>>> Signed-off-by: Jernej Skrabec <jernej.skrabec at gmail.com>
>>>>> [adjusted commit message, update default value]
>>>>> Signed-off-by: Philippe Simons <simons.philippe at gmail.com>
>>>>> ---
>>>>> arch/arm/mach-sunxi/Kconfig | 2 +-
>>>>> arch/arm/mach-sunxi/dram_sun50i_h616.c | 2 +-
>>>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
>>>>> index e979ee4a2cc..a1ddc6a1fc8 100644
>>>>> --- a/arch/arm/mach-sunxi/Kconfig
>>>>> +++ b/arch/arm/mach-sunxi/Kconfig
>>>>> @@ -144,7 +144,7 @@ config DRAM_SUNXI_TPR3
>>>>> config DRAM_SUNXI_TPR6
>>>>> hex "DRAM TPR6 parameter"
>>>>> - default 0x3300c080
>>>>> + default 0x38c00080
>>>> I don't think this will be a good solution. The changes in this series
>>>> were originally made to improve compatibility with the vendor driver,
>>>> but now you are introducing an additional inconsistency by closing the
>>>> previous one. The default values are not just arbitrary values — they
>>>> are part of the vendor code that was obtained through binary analysis.
>>>> https://lore.kernel.org/all/20231018172109.085cce24@donnerap.manchester.arm.com/
>>>
>>> I tend to agree with you, it feels like the default value was taken from the
>>> reference code while the field parsing was just an issue that was introduced
>>> with the h616 code. So it should not affect the defaut value, which was likely
>>> mistakenly parsed before.
>>
>> Yes, that Tanix TX1 uses a wrong value because of this, but that's
>> about the only effect, I'd say.
>>
>> It seems like at this point the "default" value is just a convenience
>> value we introduced to avoid naming the same repeated TPR6 value in
>> every defconfig, as per my email. But it's a pure mainline U-Boot
>> technicality at this point, and we could easily just provide an
>> explicit value in each defconfig. I came up with this table, for the
>> currently mainline supported boards:
>>
>> board name SoC TPR6 value type Vref value
>> ------------------------------------------------------------------
>> liontron-h-a133l A133 0x48000000 LPDDR4 48xxxxxx
>> anbernic_rg35xx_h700 H616 0x40808080 LPDDR4 40xxxxxx
>> orangepi_zero2 H616 0x3300c080 d DDR3 xxxxxx80
>> orangepi_zero2w H616 0x48808080 LPDDR4 48xxxxxx
>> orangepi_zero3 H616 0x44000000 LPDDR4 44xxxxxx
>> tanix_tx1 H616 0x2fb08080 LPDDR3 xxb0xxxx
>> transpeed-8k618-t H616 0x3300c080 d DDR3 xxxxxx80
>> x96_mate H616 0x3300c080 d DDR3 xxxxxx80
>> x96q H616 0x3300c080 d DDR3 xxxxxx80
>> yuzukihd-chameleon H616 0x33808080 DDR3 xxxxxx80
>> avaota-a1 A523 0x38000000 LPDDR4 38xxxxxx
>> orangepi_4a A523 0x38000000 LPDDR4 38xxxxxx
>> radxa-cubie-a5e A523 0x38000000 LPDDR4 38xxxxxx
>> x96q_pro_plus A523 0x3380807e DDR3 xxxxxx7e
>
> A few more entries for board I could get running (not submitted yet):
>
> board name SoC TPR6 value type Vref value
> ------------------------------------------------------------------
> baijie-helperboard-a133 A133 0x48000000 LPDDR4 48xxxxxx
> logicom-la-tab-129 A133 0x2fb88080 LPDDR3 xxb8xxxx
> kickpi-k5c A133 0x33808080 DDR3 xxxxxx80
> dshanpi-rosx R818 0x48000000 LPDDR4 48xxxxxx
Ah, many thanks for that, good to have more examples.
And looking forward to the support patches for those boards ;-)
>> So the default value is just used for the old H616 DDR3 boards, where
>> only the LSB matters.
>>
>> So what about dropping the default completely, since now it spans three
>> SoCs already, and finding some reasonable default for all of them will
>> become more complicated?
>
> Yes honestly this is the only DRAM TPR value that has a non-zero default in
> Kconfig and it feels a bit weird. I'm in favor of explicitly defining all the
> board-specific parameters in the configs and using zero as Kconfig default.
Cool, I will do that!
>> And maybe you can confirm: this should not affect compatibility with
>> vendor parameters at all, I think they always contain an explicit TPR6
>> value, and "no TPR6 value provided" is not a thing?
>
> Yes I've always seen an explicit value defined, and since it's part of the
> dram_para array, there has to be something. However the A133 code has fallback
> values when it gets a zero from dram_para:
> https://source.denx.de/u-boot/custodians/u-boot-sunxi/-/blob/master/arch/arm/mach-sunxi/dram_sun50i_a133.c?ref_type=heads#L598
>
> This doesn't seem to be the case in the H616 and A523 code. The default is 0x80
> for DDR3/DDR4/LPDDR3 and 0x33 for LPDDR4. I'm not sure this is really a thing
> that boards might actually rely on though. I would rather be in favor of
> removing it too to avoid confusion.
From what I'have seen, it looks like the LPDDR4 value is always a
multiple of 8, at least no board uses 0x33.
But I'd say leave the A133 code in for now, it doesn't hurt either and
mimics the BSP.
>>>> There are several options here:
>>>> 1. If our goal is to improve compatibility, then we should not change
>>>> the default values for LPDDR4, and only keep the changes for LPDDR3.
>>
>> Fair, I don't really care about the MSB, the new value I suggested
>> seemed just like the most common value (see the table above). But it's
>> pointless, since those boards provide an explicit value, and don't use
>> the default.
>>
>>>> 2. Add the changes from option 2, and additionally introduce the default
>>>> values for the A523 platform that I made during the initial binary
>>>> analysis but for some reason did not get merged into mainline.
>>>> (https://github.com/iuncuim/u-boot/blob/b17c5c8911a4fce328b01e6332632a9ccd88ebc6/arch/arm/mach-sunxi/Kconfig#L102)
>>
>> Sorry, but this value doesn't seem right: The LPDDR4 boards use 0x38,
>> not 0x33, and the DDR3 board uses 0x7e, not 0x80. I don't know about
>> other boards, but this value don't seem to help - and it's unneeded
>> anyway, as the A523 board already provide an explicit TPR6 - and new
>> boards could do as well.
>
> I think we should just drop the default value instead of trying to make sense
> of what that default should be for different platforms.
Yes, will do that.
>>> It would be good to have different defaults for different platforms yes.
>>> Note that the a133 code has some fallback with defaults when the extracted
>>> field in tpr6 is zero. Maybe they should be used as default value for a133.
>>> We probably still need to keep these fallbacks in the code in case some
>>> board-specific tpr6 value does have zeroes and relies on this behavior.
>>>
>>>> 3. If we accept the incompatibility between the vendor driver and
>>>> the mainline driver, then this patch series is unnecessary because
>>>> they introduce additional inconsistency with the vendor driver.
>>>
>>> We definitely want to be able to take the dram parameter values straight from
>>> the BSP and put them directly into our Kconfig. Changing the order of fields
>>> would make porting new boards very painful so it should be avoided.
>>
>> Yes, I agree, we should take the DRAM parameters values verbatim. But
>> as explained above, I think the Kconfig default is a different matter.
>> For simplicity, I'd say we drop it and add TPR6 values into the
>> affected defconfigs. Providing separate defaults for different SoC just
>> makes things more complicated - for no reason.
>
> Agreed.
Thanks, will send a v3 to that effect.
Cheers,
Andre
>
> All the best,
>
> Paul
>
>>>
>>>> I think option two is the most correct, but option one is also suitable,
>>>> since the DRAM driver for the A523 only supports DDR3 and LPDDR4,
>>>> and the default values in this case are the same as for the H616
>>>> (A523 is 0x33808080, H616 is 0x33c00080).
>>>
>>> I suggest we keep the current default (without moving the lpddr3 bits) for h616
>>> and add new defaults in Kconfig for a523 and a133.
>>>
>>>>> help
>>>>> TPR6 value from vendor DRAM settings.
>>>>> diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c
>>>>> index 3345c9b8e82..42a0550e015 100644
>>>>> --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c
>>>>> +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c
>>>>> @@ -975,7 +975,7 @@ static bool mctl_phy_init(const struct dram_para *para,
>>>>> val = para->tpr6 & 0xff;
>>>>> break;
>>>>> case SUNXI_DRAM_TYPE_LPDDR3:
>>>>> - val = para->tpr6 >> 8 & 0xff;
>>>>> + val = para->tpr6 >> 16 & 0xff;
>>>
>>> This change is definitely necessary though.
>>
>> Yes, absolutely.
>>
>>
>> Cheers,
>> Andre
>>
>>>
>>>>> break;
>>>>> case SUNXI_DRAM_TYPE_LPDDR4:
>>>>> val = para->tpr6 >> 24 & 0xff;
>>>
>>> All the best,
>>>
>>> Paul
>>>
>>
>
More information about the U-Boot
mailing list