Re: [PATCH] usb: dwc3: core: enable CCI support

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

On Fri, Nov 17, 2023, Mehta, Piyush wrote:
> Hi,
> 
> > -----Original Message-----
> > From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> > Sent: Friday, November 10, 2023 8:07 AM
> > To: Mehta, Piyush <piyush.mehta@amd.com>
> > Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>;
> > gregkh@linuxfoundation.org; Simek, Michal <michal.simek@amd.com>; linux-
> > usb@vger.kernel.org; linux-kernel@vger.kernel.org; Paladugu, Siva Durga Prasad
> > <siva.durga.prasad.paladugu@amd.com>; Pandey, Radhey Shyam
> > <radhey.shyam.pandey@amd.com>
> > Subject: Re: [PATCH] usb: dwc3: core: enable CCI support
> > 
> > On Mon, Nov 06, 2023, Mehta, Piyush wrote:
> > > Hi Thinh,
> > >
> > > > -----Original Message-----
> > > > From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> > > > Sent: Saturday, October 21, 2023 4:46 AM
> > > > To: Mehta, Piyush <piyush.mehta@amd.com>
> > > > Cc: gregkh@linuxfoundation.org; Simek, Michal
> > > > <michal.simek@amd.com>; Thinh Nguyen <Thinh.Nguyen@synopsys.com>;
> > > > linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; git
> > > > (AMD-Xilinx) <git@amd.com>
> > > > Subject: Re: [PATCH] usb: dwc3: core: enable CCI support
> > > >
> > > > On Fri, Oct 13, 2023, Piyush Mehta wrote:
> > > > > The GSBUSCFG0 register bits [31:16] are used to configure the
> > > > > cache type settings of the descriptor and data write/read
> > > > > transfers (Cacheable, Bufferable/ Posted). When CCI is enabled in
> > > > > the design,
> > > > > DWC3 core GSBUSCFG0 cache bits must be updated to support CCI
> > > > > enabled
> > > > transfers in USB.
> > > > >
> > > > > Signed-off-by: Piyush Mehta <piyush.mehta@amd.com>
> > > > > ---
> > > > > DWC3 Register Map Link:
> > > > > https://urldefense.com/v3/__https://docs.xilinx.com/r/en-US/ug1087
> > > > > -zynq-
> > > > ultrascale-registers/GSBUSCFG0-USB3_XHCI-
> > > >
> > Register__;!!A4F2R9G_pg!cbLbYLk9xu54DZMkiTQzMhdCaPA4gmIVInes4ZeNb28
> > > > AI2pgesEvHNYtjoZfWvE_t7wlUdZvYn-Pio8WjA55ekal$
> > > > > Register Name	GSBUSCFG0
> > > > > Description	Global SoC Bus Configuration Register 0
> > > > >
> > > > > GSBUSCFG0 (USB3_XHCI) Register Bit-Field:
> > > > > DATRDREQINFO	31:28
> > > > > DESRDREQINFO	27:24
> > > > > DATWRREQINFO	23:20
> > > > > DESWRREQINFO	19:16
> > > > > ---
> > > > >  drivers/usb/dwc3/core.c | 17 +++++++++++++++++
> > > > > drivers/usb/dwc3/core.h |  5 +++++
> > > > >  2 files changed, 22 insertions(+)
> > > > >
> > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > > index 9c6bf054f15d..fc6892c63abf 100644
> > > > > --- a/drivers/usb/dwc3/core.c
> > > > > +++ b/drivers/usb/dwc3/core.c
> > > > > @@ -23,6 +23,7 @@
> > > > >  #include <linux/delay.h>
> > > > >  #include <linux/dma-mapping.h>
> > > > >  #include <linux/of.h>
> > > > > +#include <linux/of_address.h>
> > > > >  #include <linux/of_graph.h>
> > > > >  #include <linux/acpi.h>
> > > > >  #include <linux/pinctrl/consumer.h> @@ -559,6 +560,20 @@ static
> > > > > void dwc3_cache_hwparams(struct dwc3
> > > > *dwc)
> > > > >  		parms->hwparams9 = dwc3_readl(dwc->regs,
> > > > DWC3_GHWPARAMS9);  }
> > > > >
> > > > > +static void dwc3_config_soc_bus(struct dwc3 *dwc) {
> > > > > +	if (of_dma_is_coherent(dwc->dev->of_node)) {
> > > > > +		u32 reg;
> > > > > +
> > > > > +		reg = dwc3_readl(dwc->regs, DWC3_GSBUSCFG0);
> > > > > +		reg |= DWC3_GSBUSCFG0_DATRDREQINFO_MASK |
> > > > > +			DWC3_GSBUSCFG0_DESRDREQINFO_MASK |
> > > > > +			DWC3_GSBUSCFG0_DATWRREQINFO_MASK |
> > > > > +			DWC3_GSBUSCFG0_DESWRREQINFO_MASK;
> > > > > +		dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, reg);
> > > > > +	}
> > > > > +}
> > > > > +
> > > >
> > > > You're overwriting default values for all platforms. Don't do that.
> > > > Not every platform supports this setting. Only apply these specific
> > > > settings to your platform.
> > >
> > > This DWC3_GSBUSCFG0 register is part of the DWC3/Core system. In
> > > glue/vendor drivers, I couldn't find a way to access the DWC3/Core
> > > access register.  Could you please provide a suggestion for the Xilinx glue
> > driver (dwc3/dwc3-xilinx.c) to access dwc3 registers?
> > 
> > We may need to pass the hardware configuration from the dt binding.
> 
> DWC3_GSBUSCFG0 register is implementation defined and part of DWC3/Core (Databook 2.90). Below is example code
> to make this logic platform specific. It conditionally validates the dma-coherent property and only performs
> DWC3_GSBUSCFG0 configuration for AMD/Xilinx platform. (DT compatible "xlnx,zynqmp-dwc3").
> 
> Example Code:
>        if (of_dma_is_coherent(dwc->dev->of_node) && of_device_is_compatible(dwc->dev->parent->of_node, "xlnx,zynqmp-dwc3")) {
>                u32 reg;                                                       
>                                                                               
>                reg = dwc3_readl(dwc->regs, DWC3_GSBUSCFG0);                   
>                reg |= DWC3_GSBUSCFG0_DATRDREQINFO |                           
>                       DWC3_GSBUSCFG0_DESRDREQINFO |                           
>                       DWC3_GSBUSCFG0_DATWRREQINFO |                           
>                       DWC3_GSBUSCFG0_DESWRREQINFO;                           
>                dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, reg);                   
>        }

I just realized I missed this in the inbox after your latest patch.
Sorry for the delay response.

Since this is platform specific, can we do this in your glue driver?
Split and share some of the defined register definitions from core.h to
regs.h. You can temporarily memory map and update this register in your
Xilinx glue driver. Its value should retain after soft reset.

Thanks,
Thinh