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

["Jernej Škrabec" <jernej.skrabec@gmail.com>]

Dne sobota, 11. november 2023 ob 08:43:22 CET je Mikhail Kalashnikov napisal(a):
> 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

Ok, thanks for double checking. I think next version can be merged and then
issues can be fixed if they appear.

Best regards,
Jernej