Re: [PATCH v3 1/1] sunxi: H616: add LPDDR4 DRAM support

[Mikhail Kalashnikov <iuncuim@gmail.com>]

Hi Jernej,

On 04.11.2023 11:31, Jernej Škrabec wrote:
> Hi Mikhail,
>
> I have some notes on LPDDR4 from a long time ago. It mostly matches to your
> code, but please take a look at comments below.
>
> Note that I have extra code, not included in this patch, for
> mctl_phy_read_training(). At the very beginning, before any other register
> access is done, this should be added:
>
> if (para->type == SUNXI_DRAM_TYPE_LPDDR4) {
> 	writel(0, SUNXI_DRAM_PHY0_BASE + 0x800);
> 	writel(0, SUNXI_DRAM_PHY0_BASE + 0x81c);
> }
>
> Maybe that will help with other LPDDR4 boards?

Current H618 and t507 boards do not yet require this feature. Yes, this
feature may be required for future devices. I will add it in the next 
version.

>> @@ -246,8 +251,13 @@ static void mctl_phy_configure_odt(const struct
>> dram_para *para) {
>>   	uint32_t val_lo, val_hi;
>>
>> +	clrsetbits_le32(SUNXI_DRAM_PHY0_BASE + 0x390, BIT(5), BIT(4));
>> +	clrsetbits_le32(SUNXI_DRAM_PHY0_BASE + 0x3d0, BIT(5), BIT(4));
>> +	clrsetbits_le32(SUNXI_DRAM_PHY0_BASE + 0x410, BIT(5), BIT(4));
>> +	clrsetbits_le32(SUNXI_DRAM_PHY0_BASE + 0x450, BIT(5), BIT(4));
>
> I don't have above block. Something from newer libdram? Is it applicable to
> all DRAM types?

This function is present in the RE code for all types of memory. Binary 
file dumped from orangepi zero3 nor spi.I paid attention to this area 
code when comparing the sunxi u-boot dump (md.l) with the resulting 
mainline u-boot. Now I checked my tvbox with H616 and LPDDR3 and it 
doesn't have this block. In this case, maybe it would be better to make 
it only for devices with LPDDR4?
>> +	case SUNXI_DRAM_TYPE_LPDDR4:
>> +		writel(0x0, &mctl_ctl->mrctrl1);
>> +		writel(0x80000030, &mctl_ctl->mrctrl0);
>> +		mctl_await_completion(&mctl_ctl->mrctrl0, BIT(31), 0);
> I don't have above block. Maybe difference from newer version of libdram 
> code?

This block was present in the sunxi t507 binaries for which the driver 
was originally intended.  Now I checked original dump from orangepi zero3 NOR SPI.
This is also present here.


>>   	writel(reg_val, &mctl_ctl->odtcfg);
>>   	writel(reg_val, &mctl_ctl->unk_0x2240);
>>   	writel(reg_val, &mctl_ctl->unk_0x3240);
> I have extra "writel(BIT(31), &mctl_com->cr);" after this block for all DRAM
> types. Not sure if it changes anything.
Looks like this has already been done in previous patches.
>> +	u16 trfc	= ns_to_t(280);
> My notes say 180 instead of 280. Did you check if register value matches if
> booted with boot0
I booted from NOR SPI of the sunxi u-boot and made a dump with md.l 
0x047fb064. It show me value 0030006f, where 0x6f trfc value. Based on 
the transformation function - 111*1000/(792/2)=280,30(30) So, obtained 
value looks correct. Best regards,
Mikhail