[PATCH 1/1] sunxi: dram: Fix incorrect ram size detection for some H6 boards

[Gunjan Gupta]

> >  bool mctl_mem_matches(u32 offset)
> >  {
> > +     dsb();
> This looks a bit odd, do you have an explanation for that? And are you
> sure that is really needed?
> I understand why we need the DSB after the writel's below, but before that?

I started with Ondrej Jirman's patch from LibreELEC's tree that had a
dsb call added
after the first writel call. That reduced the frequency of the errors
but didn't removed
it completely. My reason for moving it before the writel was to make
sure any memory
access is completed before starting the actual logic for the test.
That reduced the
frequency even further, but didn't resolve the issue. I did try
removing it leaving only
udelay added to the code, but that brings back the issue.

> The only thing I could think of is that we are missing a barrier in
> mctl_core_init() - which is the function called before mctl_mem_matches().
> Can you move that dsb(); into mctl_auto_detect_dram_size(), right after
> the mctl_core_init() call (where you add the udelay() below)? And I wonder
> if a dmb() would already be sufficient?

Sure, I will try experimenting with it.

> I noticed recently that the clr/setbit_le32() functions don't have a barrier at all, maybe
> that should be fixed instead?

I haven't done much of the low level programming myself. Mostly have
done user space
programming along with fixing minor kernel module compilation issues
due to kernel api
changes. So I wasn't sure what all places to debug. But if you point
me to places with
things to try, I surely can give time playing around testing the proposed fixes.

> > @@ -623,6 +623,8 @@ static void mctl_auto_detect_dram_size(struct dram_para *para)
> >       para->cols = 11;
> >       mctl_core_init(para);
> >
> > +     udelay(50);
> The location of that udelay() looks a bit odd, any chance that belongs at
> the end of mctl_channel_init() instead? And how did you come up with that
> and the value of 50? Just pure experimentation?

Before adding the udelay, I added 7 print statements to print all the
members of the para
struct. That itself solved the issue along with the dsb added to the
top of the mctl_mem_matches
function. This is what gave me the clue that a delay is needed there.
The value of 50 is
indeed from pure experimentation

Thanks & Regards
Gunjan Gupta