[PATCH 0/2] usb: dwc3: Add cache type configuration support for freescale layerscape

[PATCH 0/2] usb: dwc3: Add cache type configuration support for freescale layerscape

[Frank Li]

There are several attempt to upstream this code in past year.

The first attempt:
https://lore.kernel.org/linux-usb/20191121095356.GB7503@b29397-desktop/

cache type is dwc core setting, not glue layer. So It'd better change in
dwc core instead of glue layer code.

The second attempt:
https://lore.kernel.org/linux-usb/20240123170206.3702413-1-Frank.Li@nxp.com/#t

DT team think there are not variable for property 'snps,dat-wr-reqinfo'.
And suggest use vendor compatible string.

This is third attempt:
Compared with first attempt:
- reduce compatible string to one 'fsl,ls-dwc3' because all setting are the
same.
- move update burst type code into dwc3_set_incr_burst_type(). and check
compatible string 'fsl,ls-dwc3'.
- Using bit field help macro.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Ran Wang (2):
      dt-bindings: usb: Add chip-specific compatible string 'fsl,ls-dwc3'
      usb: dwc3: Set cache type to 'snoop' for freescale layerscape chip

 Documentation/devicetree/bindings/usb/snps,dwc3.yaml |  1 +
 drivers/usb/dwc3/core.c                              | 12 ++++++++++++
 drivers/usb/dwc3/core.h                              |  4 ++++
 3 files changed, 17 insertions(+)
---
base-commit: 0b58e108042b0ed28a71cd7edf5175999955b233
change-id: 20240703-dwc-5be3e378ddbe

Best regards,
---
Frank Li <Frank.Li@nxp.com>

[PATCH 1/2] dt-bindings: usb: Add chip-specific compatible string 'fsl,ls-dwc3'

[Frank Li]

From: Ran Wang <ran.wang_1@nxp.com>

Some Layerscape paltforms (such as LS1088A, LS2088A, etc) require update HW
default cache type configuration to fix DWC3 init failure when applying
property dma-coherent.

The cache type configuration is actually native feature of DWC3, not
additional desgin (or glue layer) coming from SoC, so add compatible string
'fsl,ls-dwc3'.

Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
Reviewed-by: Jun Li <jun.li@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index 1cd0ca90127d9..02cb986d0fd32 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -32,6 +32,7 @@ properties:
   compatible:
     contains:
       oneOf:
+        - const: fsl,ls-dwc3
         - const: snps,dwc3
         - const: synopsys,dwc3
           deprecated: true

-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: usb: Add chip-specific compatible string 'fsl,ls-dwc3'

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1871 bytes --]

On Wed, Jul 03, 2024 at 07:06:54PM -0400, Frank Li wrote:
> From: Ran Wang <ran.wang_1@nxp.com>
> 
> Some Layerscape paltforms (such as LS1088A, LS2088A, etc) require update HW
> default cache type configuration to fix DWC3 init failure when applying
> property dma-coherent.

Your driver patch looks odd because it doesn't actually check if
dma-coherent is present before setting this configuration. Is it okay to
set this configuration when dma-coherent is not present?

> The cache type configuration is actually native feature of DWC3, not
> additional desgin (or glue layer) coming from SoC, so add compatible string
> 'fsl,ls-dwc3'.

You'd need to add specific compatibles here for the broken platforms,
otherwise it is not clear to users what platforms to actually use this
one.

> Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> Reviewed-by: Jun Li <jun.li@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> index 1cd0ca90127d9..02cb986d0fd32 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> @@ -32,6 +32,7 @@ properties:
>    compatible:
>      contains:
>        oneOf:
> +        - const: fsl,ls-dwc3

This clearly was not actually tested properly - your driver patch never
changes of_dwc3_match, so providing only this property won't work & your
expectation is actually that there is a fallback to snps,dwc3. The
binding does not allow that fallback as written.

Thanks,
Conor.

>          - const: snps,dwc3
>          - const: synopsys,dwc3
>            deprecated: true
> 
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 2/2] usb: dwc3: Set cache type to 'snoop' for freescale layerscape chip

[Frank Li]

From: Ran Wang <ran.wang_1@nxp.com>

dwc3 have 4 different transfer types: Data Read, Desc Read, Data Write and
Desc write. For each transfer 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)

In most cases, driver work well with default value. But USB device detect
failure sometime on Layerscape platforms if bit[1] not set and enable
dma-coherent.

Set bit[1] "Snoop" for Data Read, Desc Read, Data Write and Desc write if
compatible string contain fsl,ls-dwc3.

Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
Reviewed-by: Jun Li <jun.li@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/usb/dwc3/core.c | 12 ++++++++++++
 drivers/usb/dwc3/core.h |  4 ++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index cb82557678ddd..2b474de83b783 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1061,6 +1061,18 @@ static void dwc3_set_incr_burst_type(struct dwc3 *dwc)
 
 	cfg = dwc3_readl(dwc->regs, DWC3_GSBUSCFG0);
 
+	if (of_device_is_compatible(dev->of_node, "fsl,ls-dwc3")) {
+
+		cfg &= ~(DWC3_GSBUSCFG0_DATARD | DWC3_GSBUSCFG0_DESCRD |
+			 DWC3_GSBUSCFG0_DATAWR | DWC3_GSBUSCFG0_DESCWR);
+		cfg |= FIELD_PREP(DWC3_GSBUSCFG0_DATARD, 2) |
+		       FIELD_PREP(DWC3_GSBUSCFG0_DESCRD, 2) |
+		       FIELD_PREP(DWC3_GSBUSCFG0_DATAWR, 2) |
+		       FIELD_PREP(DWC3_GSBUSCFG0_DESCWR, 2);
+
+		dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, cfg);
+	}
+
 	/*
 	 * Handle property "snps,incr-burst-type-adjustment".
 	 * Get the number of value from this property:
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 3781c736c1a17..99edfabd89673 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -184,6 +184,10 @@
 /* Bit fields */
 
 /* Global SoC Bus Configuration INCRx Register 0 */
+#define DWC3_GSBUSCFG0_DATARD		GENMASK(31, 28)
+#define DWC3_GSBUSCFG0_DESCRD		GENMASK(27, 24)
+#define DWC3_GSBUSCFG0_DATAWR		GENMASK(23, 20)
+#define DWC3_GSBUSCFG0_DESCWR		GENMASK(19, 16)
 #define DWC3_GSBUSCFG0_INCR256BRSTENA	(1 << 7) /* INCR256 burst */
 #define DWC3_GSBUSCFG0_INCR128BRSTENA	(1 << 6) /* INCR128 burst */
 #define DWC3_GSBUSCFG0_INCR64BRSTENA	(1 << 5) /* INCR64 burst */

-- 
2.34.1

Re: [PATCH 0/2] usb: dwc3: Add cache type configuration support for freescale layerscape

[Thinh Nguyen]

Hi Frank,

On Wed, Jul 03, 2024, Frank Li wrote:
> There are several attempt to upstream this code in past year.
> 
> The first attempt:
> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20191121095356.GB7503@b29397-desktop/__;!!A4F2R9G_pg!cdNydVZ64br9EHG13lgP3lKWe5VuXQvvfvA1CzKJqhXZZB1H9vcJlwI1vj1EF3ynRFl2u8tD3lYqRvrJFSs2$ 
> 
> cache type is dwc core setting, not glue layer. So It'd better change in
> dwc core instead of glue layer code.

Why not glue layer?

> 
> The second attempt:
> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20240123170206.3702413-1-Frank.Li@nxp.com/*t__;Iw!!A4F2R9G_pg!cdNydVZ64br9EHG13lgP3lKWe5VuXQvvfvA1CzKJqhXZZB1H9vcJlwI1vj1EF3ynRFl2u8tD3lYqRh7SiHmM$ 
> 
> DT team think there are not variable for property 'snps,dat-wr-reqinfo'.
> And suggest use vendor compatible string.
> 
> This is third attempt:
> Compared with first attempt:
> - reduce compatible string to one 'fsl,ls-dwc3' because all setting are the
> same.
> - move update burst type code into dwc3_set_incr_burst_type(). and check
> compatible string 'fsl,ls-dwc3'.
> - Using bit field help macro.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Ran Wang (2):
>       dt-bindings: usb: Add chip-specific compatible string 'fsl,ls-dwc3'
>       usb: dwc3: Set cache type to 'snoop' for freescale layerscape chip
> 
>  Documentation/devicetree/bindings/usb/snps,dwc3.yaml |  1 +
>  drivers/usb/dwc3/core.c                              | 12 ++++++++++++
>  drivers/usb/dwc3/core.h                              |  4 ++++
>  3 files changed, 17 insertions(+)
> ---
> base-commit: 0b58e108042b0ed28a71cd7edf5175999955b233
> change-id: 20240703-dwc-5be3e378ddbe
> 
> Best regards,
> ---
> Frank Li <Frank.Li@nxp.com>
> 

This may blow up the dwc3 core from various platforms and compatible
strings. This can be handled in the glue driver and create the software
property instead.

Radhey Shyam is working on this also. You can check his work here:

https://lore.kernel.org/linux-usb/1717657279-2631757-1-git-send-email-radhey.shyam.pandey@amd.com/

Thanks,
Thinh

Re: [PATCH 0/2] usb: dwc3: Add cache type configuration support for freescale layerscape

[Frank Li]

On Wed, Jul 03, 2024 at 11:33:23PM +0000, Thinh Nguyen wrote:
> Hi Frank,
> 
> On Wed, Jul 03, 2024, Frank Li wrote:
> > There are several attempt to upstream this code in past year.
> > 
> > The first attempt:
> > https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20191121095356.GB7503@b29397-desktop/__;!!A4F2R9G_pg!cdNydVZ64br9EHG13lgP3lKWe5VuXQvvfvA1CzKJqhXZZB1H9vcJlwI1vj1EF3ynRFl2u8tD3lYqRvrJFSs2$ 
> > 
> > cache type is dwc core setting, not glue layer. So It'd better change in
> > dwc core instead of glue layer code.
> 
> Why not glue layer?

I understand glue layer is what provide clock, reset, power and other
control for dwc usb core and have a seperate mmio register space. All dwc
core register access should belong to dwc-core part. You may have
difference defination for glue layer.

> 
> > 
> > The second attempt:
> > https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20240123170206.3702413-1-Frank.Li@nxp.com/*t__;Iw!!A4F2R9G_pg!cdNydVZ64br9EHG13lgP3lKWe5VuXQvvfvA1CzKJqhXZZB1H9vcJlwI1vj1EF3ynRFl2u8tD3lYqRh7SiHmM$ 
> > 
> > DT team think there are not variable for property 'snps,dat-wr-reqinfo'.
> > And suggest use vendor compatible string.
> > 
> > This is third attempt:
> > Compared with first attempt:
> > - reduce compatible string to one 'fsl,ls-dwc3' because all setting are the
> > same.
> > - move update burst type code into dwc3_set_incr_burst_type(). and check
> > compatible string 'fsl,ls-dwc3'.
> > - Using bit field help macro.
> > 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > Ran Wang (2):
> >       dt-bindings: usb: Add chip-specific compatible string 'fsl,ls-dwc3'
> >       usb: dwc3: Set cache type to 'snoop' for freescale layerscape chip
> > 
> >  Documentation/devicetree/bindings/usb/snps,dwc3.yaml |  1 +
> >  drivers/usb/dwc3/core.c                              | 12 ++++++++++++
> >  drivers/usb/dwc3/core.h                              |  4 ++++
> >  3 files changed, 17 insertions(+)
> > ---
> > base-commit: 0b58e108042b0ed28a71cd7edf5175999955b233
> > change-id: 20240703-dwc-5be3e378ddbe
> > 
> > Best regards,
> > ---
> > Frank Li <Frank.Li@nxp.com>
> > 
> 
> This may blow up the dwc3 core from various platforms and compatible
> strings. This can be handled in the glue driver and create the software
> property instead.
> 
> Radhey Shyam is working on this also. You can check his work here:
> 
> https://lore.kernel.org/linux-usb/1717657279-2631757-1-git-send-email-radhey.shyam.pandey@amd.com/

This is quite good.

Frank

> 
> Thanks,
> Thinh