Re: [PATCH 0/8] sunxi: Update H616 DRAM driver

[Andre Przywara <andre.przywara@arm.com>]

On Wed, 04 Jan 2023 22:02:33 +0100
Jernej Škrabec <jernej.skrabec@gmail.com> wrote:

Hi Jernej,

> Dne sreda, 04. januar 2023 ob 01:47:16 CET je Andre Przywara napisal(a):
> > On Sun, 11 Dec 2022 17:32:05 +0100
> > Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
> > 
> > Hi Jernej,
> >   
> > > Current H616 DRAM driver is completely customized to Orange Pi Zero2
> > > board, which is currently the only H616 board supported by U-Boot.
> > > Needless to say, this is not ideal for adding new boards. With changes
> > > in this series, all DDR3 boards are supported and all that is needed is
> > > just vendor DRAM values extracted from Android image. New DRAM types
> > > should also be easier to support, since a lot of constants used before
> > > are not really DRAM type dependent.
> > > 
> > > Changes were verified by decompiling driver and generated values were
> > > compared to previous, hard coded ones. This was done without dram_para
> > > structures, so compiler was able to heavily optimize code and produce
> > > constants.  
> > 
> > so many thanks again for putting this together!
> > I came to like (the idea of) this series very much lately, as this
> > removes timing/delay values from the code, and easily allows putting the
> > vendor provided values in the defconfig.
> > I used that approach as well for the D1 driver, and am wondering if we
> > should extend this to other SoCs, potentially unifying the Kconfig part?  
> 
> While it would be nice, I'm not sure it's worth the effort and there is a 
> chance that something might break during rework.

Fair enough, was just an idea.

> > And you hinted at a v2, can you provide an estimate for this? If you
> > send it still this week, I would like to put it into U-Boot's next
> > branch, otherwise it goes straight into master, should the merge window
> > open next week as planned.  
> 
> I have changes for v2 in my github repo. I don't have any estimation, since I 
> had some time off from programming lately

Sounds good!

> and I'm just only catching up. 
> Weekend at earliest, but no promises.

No worries, that was a genuine question, for my planning. I have plenty
of other things to do, and we have still plenty of time (till end of
January or so) to get things merged.

> > Btw., to verify the feasibility of drivers/ram/sunxi, I moved the H616
> > driver into there, together with the Kconfig parts, I wonder what you
> > think about this? An example of how this looks is in the D1 driver
> > patches.  
> 
> Looks good, but I don't know what are implications regarding interface. Is 
> just code move or that implies that some ram framework must be used?

I don't know what Simon and Tom will say, but I was cheekily just using
the directory, ignoring the current DM_SPL preference for code in
there. For the D1/T113s code this is probably a no-brainer, but
I also found it a nice and simple solution to declutter and cleanup
arch/arm/mach-sunxi and its Kconfig.
So yeah, it's just moving the files and Kconfig stanzas there, and
otherwise leaves the actual code untouched. We still call
sunxi_dram_init() from our legacy SPL code.

Cheers,
Andre


> 
> Best regards,
> Jernej
> 
> > 
> > Cheers,
> > Andre
> >   
> > > Please take a look.
> > > 
> > > Best regards,
> > > Jernej
> > > 
> > > Jernej Skrabec (8):
> > >   sunxi: Fix write to H616 DRAM CR register
> > >   sunxi: cosmetic: Fix H616 DRAM driver code style
> > >   sunxi: parameterize H616 DRAM ODT values
> > >   sunxi: Convert H616 DRAM options to single setting
> > >   sunxi: Always configure ODT on H616 DRAM
> > >   sunxi: Make bit delay function in H616 DRAM code void
> > >   sunxi: Parameterize bit delay code in H616 DRAM driver
> > >   sunxi: Parameterize H616 DRAM code some more
> > >  
> > >  .../include/asm/arch-sunxi/dram_sun50i_h616.h |  18 +
> > >  arch/arm/mach-sunxi/Kconfig                   |  67 +--
> > >  arch/arm/mach-sunxi/dram_sun50i_h616.c        | 445 +++++++++++-------
> > >  configs/orangepi_zero2_defconfig              |   8 +-
> > >  4 files changed, 348 insertions(+), 190 deletions(-)  
> 
> 
> 
>