Re: [PATCH 1/3] usb: musb-new: sunxi: do not attempt to access NULL SRAMC

[Sam Edwards <cfsworks@gmail.com>]

Hey Andre,

On 6/7/23 04:45, Andre Przywara wrote:
> "syscon" really just means "a bunch of gates where AW didn't know where
> else to put them".

The good ol' "kitchen sink" register block, eh? Its lack of clear, 
definite purpose is even reflected in the name, because when you think 
about it, isn't *every* MMIO register for "system control"? :)

> Yeah, it seems this way, though nobody knows for sure ;-) I believe this
> is code that came from the original Allwinner BSP, which tends to do,
> well, weird stuff at times ;-)

I've gone ahead and submitted a patch for rewording this in the driver 
source anyway. In the commit message, I've included a way we could 
verify this experimentally, though I don't have access to an early 
Allwinner SoC with which to try it.

> Yeah, so there are three steps, in increasing order of complexity:
> 1) Change SUNXI_SRAMC_BASE to 0x3000000. That's done already in my tree.

I've done it over here as well.

> 2) Introduce some has_sram property in U-Boot's MUSB driver and only poke
> the SRAM D bit for those SoCs that need it.

Done, a patch for that has been sent in.

> 3) Follow the allwinner,sram phandle in the MUSB driver and fish out the
> syscon base address from the DTB, to avoid hardcoding it, at least for the
> MUSB driver. We need it elsewhere, to access the version register and
> print the SoC name.

One of the patches I just sent in also adds a "TODO" comment to do this. 
I also kept the function separate specifically so someone coming along 
later wanting to add this has plenty of breathing room to do so.

> Can you confirm that just changing the SUNXI_SRAMC_BASE to 0x3000000 avoids
> the crash you saw, and removes the immediate need for this very patch here?

Yes: I reverted enough changes to make the crash reappear, and then 
changed the base register. Updating SUNXI_SRAM_BASE to 0x03000000 
resolves the crash with no side effects: it seems (BIT(0) of) 0x03000004 
is RAZ/WI.

> And if I get this right, you already have 2) implemented? I would love to
> see it on the list, then.

See Patchwork series #358672.

Thanks for your continued efforts,
Sam