RE: [PATCH V2 1/2] usb: dwc3: Add node to update cache type setting

[Ran Wang <ran.wang_1@nxp.com>]

Hi Rob, Felipe,

On Tuesday, September 24, 2019 00:38, Yang Li wrote:
> 
> On Thu, Jul 25, 2019 at 4:56 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Wed, Jul 24, 2019 at 8:29 PM Ran Wang <ran.wang_1@nxp.com> wrote:
> > >
> > > Hi Rob,
> > >
> > > On Thursday, July 25, 2019 04:42 Rob Herring <robh@kernel.org> wrote:
> > > >
> > > > On Fri, Jul 12, 2019 at 02:42:05PM +0800, Ran Wang wrote:
> > > > > Some Layerscape paltforms (such as LS1088A, LS2088A, etc)
> > > > > encounter USB detect failues when adding dma-coherent to DWC3
> > > > > node. This is because the HW default cache type configuration of
> > > > > those SoC are not right, need to be updated in DTS.
> > > > >
> > > > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> > > > > ---
> > > > > Change in v2:
> > > > >     - New file.
> > > > >
> > > > >  Documentation/devicetree/bindings/usb/dwc3.txt | 43
> > > > > ++++++++++++++++++++++++++
> > > > >  1 file changed, 43 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
> > > > > b/Documentation/devicetree/bindings/usb/dwc3.txt
> > > > > index 8e5265e..7bc1cef 100644
> > > > > --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> > > > > +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> > > > > @@ -110,6 +110,43 @@ Optional properties:
> > > > >   - in addition all properties from usb-xhci.txt from the current directory
> are
> > > > >     supported as well
> > > > >
> > > > > +* Cache type nodes (optional)
> > > > > +
> > > > > +The Cache type node is used to tell how to configure cache type
> > > > > +on 4 different transfer types: Data Read, Desc Read, Data Write
> > > > > +and Desc write. For each treasfer type, controller has a 4-bit
> > > > > +register field to enable different cache type. Quoted from DWC3
> > > > > +data book Table 6-5
> > > > Cache Type Bit Assignments:
> > > > > +----------------------------------------------------------------
> > > > > +MBUS_TYPE| bit[3]       |bit[2]       |bit[1]     |bit[0]
> > > > > +----------------------------------------------------------------
> > > > > +AHB      |Cacheable     |Bufferable   |Privilegge |Data
> > > > > +AXI3     |Write Allocate|Read Allocate|Cacheable  |Bufferable
> > > > > +AXI4     |Allocate Other|Allocate     |Modifiable |Bufferable
> > > > > +AXI4     |Other Allocate|Allocate     |Modifiable |Bufferable
> > > > > +Native   |Same as AXI   |Same as AXI  |Same as AXI|Same as AXI
> > > > > +---------------------------------------------------------------
> > > > > +-
> > > > > +Note: The AHB, AXI3, AXI4, and PCIe busses use different names
> > > > > +for certain signals, which have the same meaning:
> > > > > +  Bufferable = Posted
> > > > > +  Cacheable = Modifiable = Snoop (negation of No Snoop)
> > > >
> > > > This should all be implied from the SoC specific compatible strings.
> > >
> > > Did you mean I could implement a soc driver which can be matched by
> compatible of 'fsl,ls1088a-dwc3' which will pass known bus type to DWC3 driver?
> If yes, how to pass?
> >
> > Yes. The DT match table can have data associated with that compatible
> > string. Beyond that, I'm not really familiar with the DWC3 driver.
> 
> Hi Rob,
> 
> It looks like that the current dwc3 binding perfers to define general quirks in
> device tree properties instead of trying to rely on the compatible string to
> determine quirks.  In this case, can we keep following the existing preference
> instead of choosing the other way?

Looks like you have different opinions on this solution, so I 'd like to have all opens
get settled here to help me find a acceptable solution for both of you. 
Please let me explain more about this:

1. This feature (configure cache type) is natively from DWC3 IP (we can find it from data book),
    not a SoC specific feature (more details please see v1 discussion: https://lore.kernel.org/patchwork/patch/851306/)

2. However, in most SoC, the HW default setting looks fine (no need driver help), but some case
    (like Layerscape) might need driver to do some programming to fix USB no detect issue.
    That's why I implement this patch.

3. For now, Rob think this should be handled by SoC specific code rather than adding a property to
    control it (by DWC3 core driver). And Felipe prefer to avoid using glue diver. So I am not sure how
    to meet this requirements at the same time. 
   
Any further suggestion are really appreciated. Thank you.

Regards,
Ran