On Fri, 9 Jun 2023 15:37:16 -0600 Sam Edwards <[email protected]> wrote:
Hi, > This is largely a cosmetic change, with one functional distinction: > We are now only setting BIT(0), and no longer clearing BIT(1). > > The A20 manual confirms the purpose and bitwidth of this field, and we > have also been doing it this way for a while in Linux-land: The prior > narrative about this initialization being about configuring a FIFO has > pretty much been debunked for years now. > > This cleanup also adds a TODO comment about runtime discovery > of the SYSCON base, per discussion with Andre. > > Signed-off-by: Sam Edwards <[email protected]> > Cc: Andre Przywara <[email protected]> > --- > drivers/usb/musb-new/sunxi.c | 38 +++++++++++++++++++++++++++--------- > 1 file changed, 29 insertions(+), 9 deletions(-) > > diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c > index 1111a67eaf..be7faaa11e 100644 > --- a/drivers/usb/musb-new/sunxi.c > +++ b/drivers/usb/musb-new/sunxi.c > @@ -171,15 +171,29 @@ static void USBC_ForceVbusValidToHigh(__iomem void > *base) > musb_writel(base, USBC_REG_o_ISCR, reg_val); > } > > -static void USBC_ConfigFIFO_Base(void) > -{ > - u32 reg_value; > +/****************************************************************************** > + * Non-USBC register access needed for initialization > + > ******************************************************************************/ > > - /* config usb fifo, 8kb mode */ > - reg_value = readl(SUNXI_SRAMC_BASE + 0x04); > - reg_value &= ~(0x03 << 0); > - reg_value |= BIT(0); > - writel(reg_value, SUNXI_SRAMC_BASE + 0x04); > +/* > + * A10(s), A13, GR8, A20: > + * switch ownership of SRAM block 'D' to the USB-OTG controller > + */ > +static void sunxi_musb_claim_sram(void) > +{ > + /* > + * TODO: Do not use hardcoded SUNXI_SRAMC_BASE; find the syscon base by > + * traversing this OTG device's `allwinner,sram` FDT property and > + * working upward to the system controller. > + */ > + void *syscon_base = (void *)SUNXI_SRAMC_BASE; Can you pass this in as a parameter, then call the function with SUNXI_SRAMC_BASE, for now? Because syscon_base will become a member of struct sunxi_glue, which we have readily accessible in sunxi_musb_init(). And no need for a comment, I think I will come up with something soon enough. Many thanks! Andre > + > + /* > + * BIT(0) of SRAM_CTRL_REG1 (syscon+0x04) controls SRAM-D ownership: > + * '0' -> exclusive access by CPU > + * '1' -> exclusive access by USB0 > + */ > + setbits_le32(syscon_base + 0x04, BIT(0)); > } > > > /****************************************************************************** > @@ -313,7 +327,13 @@ static int sunxi_musb_init(struct musb *musb) > musb->isr = sunxi_musb_interrupt; > > if (glue->cfg->has_sram) { > - USBC_ConfigFIFO_Base(); > + /* > + * This is an older USB-OTG controller that Allwinner did not > + * endow with a dedicated SRAM block; it instead uses SRAM > + * block 'D', ownership of which needs to be handed over by > + * the CPU > + */ > + sunxi_musb_claim_sram(); > } > > USBC_EnableDpDmPullUp(musb->mregs);

