[PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Frank Li]

Add device tree binding allow platform overwrite default value of *REQIN in
GSBUSCFG0.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 .../devicetree/bindings/usb/snps,dwc3.yaml    | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index 8f5d250070c78..43e7fea3f6798 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -439,6 +439,42 @@ properties:
     items:
       enum: [1, 4, 8, 16, 32, 64, 128, 256]
 
+  snps,des-wr-reqinfo:
+    description: Value for DESEWRREQIN of GSBUSCFG0 register.
+      ----------------------------------------------------------------
+       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
+      ----------------------------------------------------------------
+      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)
+    $ref: /schemas/types.yaml#/definitions/uint8
+    maxItem: 15
+
+  snps,des-rd-reqinfo:
+    description: Value for DESRDREQIN of GSBUSCFG0 register. ref
+      snps,des-wr-reqinfo
+    $ref: /schemas/types.yaml#/definitions/uint8
+    maxItem: 15
+
+  snps,dat-wr-reqinfo:
+    description: Value for DATWRREQIN of GSBUSCFG0 register. ref
+      snps,des-wr-reqinfo
+    $ref: /schemas/types.yaml#/definitions/uint8
+    maxItem: 15
+
+  snps,des-wr-reqinfo:
+    description: Value for DATWRREQIN of GSBUSCFG0 register. ref
+      snps,des-wr-reqinfo
+    $ref: /schemas/types.yaml#/definitions/uint8
+    maxItem: 15
+
   num-hc-interrupters:
     maximum: 8
     default: 1
-- 
2.34.1

[PATCH 2/2] usb: dwc3: core: allow overwrite reqinfo in GSBUSCFG0 by dts

[Frank Li]

Add function dwc3_set_bus_req_info() to allow dts overwrite bus request
info in GSBUSCFG0. Some platform (such as Layerscape) require correct
setting for dma-coherent. But default value is wrong in chips.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/usb/dwc3/core.c | 33 +++++++++++++++++++++++++++++++++
 drivers/usb/dwc3/core.h |  4 ++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 3b68e8e45b8b9..c72929b122014 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -948,6 +948,37 @@ static void dwc3_core_setup_global_control(struct dwc3 *dwc)
 static int dwc3_core_get_phy(struct dwc3 *dwc);
 static int dwc3_core_ulpi_init(struct dwc3 *dwc);
 
+static void dwc3_set_bus_req_info(struct dwc3 *dwc)
+{
+	struct device *dev = dwc->dev;
+	u32 cfg;
+	u8 val;
+
+	cfg = dwc3_readl(dwc->regs, DWC3_GSBUSCFG0);
+
+	if (!device_property_read_u8(dev, "snps,des-wr-reqinfo", &val)) {
+		cfg &= ~DWC3_GSBUSCFG0_DESWRREQINFO_MASK;
+		cfg |= FIELD_PREP(DWC3_GSBUSCFG0_DESWRREQINFO_MASK, val);
+	}
+
+	if (!device_property_read_u8(dev, "snps,des-rd-reqinfo", &val)) {
+		cfg &= ~DWC3_GSBUSCFG0_DESRDREQINFO_MASK;
+		cfg |= FIELD_PREP(DWC3_GSBUSCFG0_DESRDREQINFO_MASK, val);
+	}
+
+	if (!device_property_read_u8(dev, "snps,dat-wr-reqinfo", &val)) {
+		cfg &= ~DWC3_GSBUSCFG0_DATWRREQINFO_MASK;
+		cfg |= FIELD_PREP(DWC3_GSBUSCFG0_DATWRREQINFO_MASK, val);
+	}
+
+	if (!device_property_read_u8(dev, "snps,dat-rd-reqinfo", &val)) {
+		cfg &= ~DWC3_GSBUSCFG0_DATRDREQINFO_MASK;
+		cfg |= FIELD_PREP(DWC3_GSBUSCFG0_DATRDREQINFO_MASK, val);
+	}
+
+	dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, cfg);
+}
+
 /* set global incr burst type configuration registers */
 static void dwc3_set_incr_burst_type(struct dwc3 *dwc)
 {
@@ -1256,6 +1287,8 @@ static int dwc3_core_init(struct dwc3 *dwc)
 
 	dwc3_set_incr_burst_type(dwc);
 
+	dwc3_set_bus_req_info(dwc);
+
 	ret = dwc3_phy_power_on(dwc);
 	if (ret)
 		goto err_exit_phy;
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index df544ec730d22..cf64cfc0f208a 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -186,6 +186,10 @@
 #define DWC3_GSBUSCFG0_INCR4BRSTENA	(1 << 1) /* INCR4 burst */
 #define DWC3_GSBUSCFG0_INCRBRSTENA	(1 << 0) /* undefined length enable */
 #define DWC3_GSBUSCFG0_INCRBRST_MASK	0xff
+#define DWC3_GSBUSCFG0_DESWRREQINFO_MASK	GENMASK(19, 16)
+#define DWC3_GSBUSCFG0_DATWRREQINFO_MASK	GENMASK(23, 20)
+#define DWC3_GSBUSCFG0_DESRDREQINFO_MASK	GENMASK(27, 24)
+#define DWC3_GSBUSCFG0_DATRDREQINFO_MASK	GENMASK(31, 28)
 
 /* Global Debug LSP MUX Select */
 #define DWC3_GDBGLSPMUX_ENDBC		BIT(15)	/* Host only */
-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Conor Dooley]

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

On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
> Add device tree binding allow platform overwrite default value of *REQIN in
> GSBUSCFG0.

Why might a platform actually want to do this? Why does this need to be
set at the board level and being aware of which SoC is in use is not
sufficient for the driver to set the correct values?

Thanks,
Conor.

> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  .../devicetree/bindings/usb/snps,dwc3.yaml    | 36 +++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> index 8f5d250070c78..43e7fea3f6798 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> @@ -439,6 +439,42 @@ properties:
>      items:
>        enum: [1, 4, 8, 16, 32, 64, 128, 256]
>  
> +  snps,des-wr-reqinfo:
> +    description: Value for DESEWRREQIN of GSBUSCFG0 register.
> +      ----------------------------------------------------------------
> +       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
> +      ----------------------------------------------------------------
> +      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)
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    maxItem: 15
> +
> +  snps,des-rd-reqinfo:
> +    description: Value for DESRDREQIN of GSBUSCFG0 register. ref
> +      snps,des-wr-reqinfo
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    maxItem: 15
> +
> +  snps,dat-wr-reqinfo:
> +    description: Value for DATWRREQIN of GSBUSCFG0 register. ref
> +      snps,des-wr-reqinfo
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    maxItem: 15
> +
> +  snps,des-wr-reqinfo:
> +    description: Value for DATWRREQIN of GSBUSCFG0 register. ref
> +      snps,des-wr-reqinfo
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    maxItem: 15
> +
>    num-hc-interrupters:
>      maximum: 8
>      default: 1
> -- 
> 2.34.1
> 

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

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Frank Li]

On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
> On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
> > Add device tree binding allow platform overwrite default value of *REQIN in
> > GSBUSCFG0.
> 
> Why might a platform actually want to do this? Why does this need to be
> set at the board level and being aware of which SoC is in use is not
> sufficient for the driver to set the correct values?

In snps,dwc3.yaml, there are already similary proptery, such as
snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
driver keep consistent. And not all platform try enable hardware
dma_cohenrence. It is configable for difference platform.

Frank

> 
> Thanks,
> Conor.
> 
> > 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  .../devicetree/bindings/usb/snps,dwc3.yaml    | 36 +++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > index 8f5d250070c78..43e7fea3f6798 100644
> > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > @@ -439,6 +439,42 @@ properties:
> >      items:
> >        enum: [1, 4, 8, 16, 32, 64, 128, 256]
> >  
> > +  snps,des-wr-reqinfo:
> > +    description: Value for DESEWRREQIN of GSBUSCFG0 register.
> > +      ----------------------------------------------------------------
> > +       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
> > +      ----------------------------------------------------------------
> > +      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)
> > +    $ref: /schemas/types.yaml#/definitions/uint8
> > +    maxItem: 15
> > +
> > +  snps,des-rd-reqinfo:
> > +    description: Value for DESRDREQIN of GSBUSCFG0 register. ref
> > +      snps,des-wr-reqinfo
> > +    $ref: /schemas/types.yaml#/definitions/uint8
> > +    maxItem: 15
> > +
> > +  snps,dat-wr-reqinfo:
> > +    description: Value for DATWRREQIN of GSBUSCFG0 register. ref
> > +      snps,des-wr-reqinfo
> > +    $ref: /schemas/types.yaml#/definitions/uint8
> > +    maxItem: 15
> > +
> > +  snps,des-wr-reqinfo:
> > +    description: Value for DATWRREQIN of GSBUSCFG0 register. ref
> > +      snps,des-wr-reqinfo
> > +    $ref: /schemas/types.yaml#/definitions/uint8
> > +    maxItem: 15
> > +
> >    num-hc-interrupters:
> >      maximum: 8
> >      default: 1
> > -- 
> > 2.34.1
> > 

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Conor Dooley]

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

On Tue, Jan 23, 2024 at 12:49:27PM -0500, Frank Li wrote:
> On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
> > On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
> > > Add device tree binding allow platform overwrite default value of *REQIN in
> > > GSBUSCFG0.
> > 
> > Why might a platform actually want to do this? Why does this need to be
> > set at the board level and being aware of which SoC is in use is not
> > sufficient for the driver to set the correct values?
> 
> In snps,dwc3.yaml, there are already similary proptery, such as
> snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
> driver keep consistent. And not all platform try enable hardware
> dma_cohenrence. It is configable for difference platform.

When you say "platform", what do you mean? I understand that term to
mean a combination of board, soc and firmware.

> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > >  .../devicetree/bindings/usb/snps,dwc3.yaml    | 36 +++++++++++++++++++
> > >  1 file changed, 36 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > > index 8f5d250070c78..43e7fea3f6798 100644
> > > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > > @@ -439,6 +439,42 @@ properties:
> > >      items:
> > >        enum: [1, 4, 8, 16, 32, 64, 128, 256]
> > >  
> > > +  snps,des-wr-reqinfo:
> > > +    description: Value for DESEWRREQIN of GSBUSCFG0 register.
> > > +      ----------------------------------------------------------------
> > > +       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
> > > +      ----------------------------------------------------------------
> > > +      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)
> > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > +    maxItem: 15
> > > +
> > > +  snps,des-rd-reqinfo:
> > > +    description: Value for DESRDREQIN of GSBUSCFG0 register. ref
> > > +      snps,des-wr-reqinfo
> > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > +    maxItem: 15
> > > +
> > > +  snps,dat-wr-reqinfo:
> > > +    description: Value for DATWRREQIN of GSBUSCFG0 register. ref
> > > +      snps,des-wr-reqinfo
> > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > +    maxItem: 15
> > > +
> > > +  snps,des-wr-reqinfo:
> > > +    description: Value for DATWRREQIN of GSBUSCFG0 register. ref
> > > +      snps,des-wr-reqinfo
> > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > +    maxItem: 15
> > > +
> > >    num-hc-interrupters:
> > >      maximum: 8
> > >      default: 1
> > > -- 
> > > 2.34.1
> > > 
> 
> 

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

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Frank Li]

On Tue, Jan 23, 2024 at 05:51:48PM +0000, Conor Dooley wrote:
> On Tue, Jan 23, 2024 at 12:49:27PM -0500, Frank Li wrote:
> > On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
> > > On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
> > > > Add device tree binding allow platform overwrite default value of *REQIN in
> > > > GSBUSCFG0.
> > > 
> > > Why might a platform actually want to do this? Why does this need to be
> > > set at the board level and being aware of which SoC is in use is not
> > > sufficient for the driver to set the correct values?
> > 
> > In snps,dwc3.yaml, there are already similary proptery, such as
> > snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
> > driver keep consistent. And not all platform try enable hardware
> > dma_cohenrence. It is configable for difference platform.
> 
> When you say "platform", what do you mean? I understand that term to
> mean a combination of board, soc and firmware.

In my company's environment, "platform" is "board". I will use "board" in
future. Is it big difference here?

Frank 

> 
> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > >  .../devicetree/bindings/usb/snps,dwc3.yaml    | 36 +++++++++++++++++++
> > > >  1 file changed, 36 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > > > index 8f5d250070c78..43e7fea3f6798 100644
> > > > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > > > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > > > @@ -439,6 +439,42 @@ properties:
> > > >      items:
> > > >        enum: [1, 4, 8, 16, 32, 64, 128, 256]
> > > >  
> > > > +  snps,des-wr-reqinfo:
> > > > +    description: Value for DESEWRREQIN of GSBUSCFG0 register.
> > > > +      ----------------------------------------------------------------
> > > > +       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
> > > > +      ----------------------------------------------------------------
> > > > +      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)
> > > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > > +    maxItem: 15
> > > > +
> > > > +  snps,des-rd-reqinfo:
> > > > +    description: Value for DESRDREQIN of GSBUSCFG0 register. ref
> > > > +      snps,des-wr-reqinfo
> > > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > > +    maxItem: 15
> > > > +
> > > > +  snps,dat-wr-reqinfo:
> > > > +    description: Value for DATWRREQIN of GSBUSCFG0 register. ref
> > > > +      snps,des-wr-reqinfo
> > > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > > +    maxItem: 15
> > > > +
> > > > +  snps,des-wr-reqinfo:
> > > > +    description: Value for DATWRREQIN of GSBUSCFG0 register. ref
> > > > +      snps,des-wr-reqinfo
> > > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > > +    maxItem: 15
> > > > +
> > > >    num-hc-interrupters:
> > > >      maximum: 8
> > > >      default: 1
> > > > -- 
> > > > 2.34.1
> > > > 
> > 
> > 

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Conor Dooley]

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

On Tue, Jan 23, 2024 at 01:02:21PM -0500, Frank Li wrote:
> On Tue, Jan 23, 2024 at 05:51:48PM +0000, Conor Dooley wrote:
> > On Tue, Jan 23, 2024 at 12:49:27PM -0500, Frank Li wrote:
> > > On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
> > > > On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
> > > > > Add device tree binding allow platform overwrite default value of *REQIN in
> > > > > GSBUSCFG0.
> > > > 
> > > > Why might a platform actually want to do this? Why does this need to be
> > > > set at the board level and being aware of which SoC is in use is not
> > > > sufficient for the driver to set the correct values?
> > > 
> > > In snps,dwc3.yaml, there are already similary proptery, such as
> > > snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
> > > driver keep consistent. And not all platform try enable hardware
> > > dma_cohenrence. It is configable for difference platform.
> > 
> > When you say "platform", what do you mean? I understand that term to
> > mean a combination of board, soc and firmware.
> 
> In my company's environment, "platform" is "board". I will use "board" in
> future. Is it big difference here?

Nah, that's close enough that it makes no difference here.

I'd still like an explanation for why a platform would need to actually
set these properties though, and why information about coherency cannot
be determined from whether or not the boss the usb controller is on is
communicated to be dma coherent via the existing devicetree properties
for that purpose.

Thanks,
Conor.

> > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > ---
> > > > >  .../devicetree/bindings/usb/snps,dwc3.yaml    | 36 +++++++++++++++++++
> > > > >  1 file changed, 36 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > > > > index 8f5d250070c78..43e7fea3f6798 100644
> > > > > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > > > > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > > > > @@ -439,6 +439,42 @@ properties:
> > > > >      items:
> > > > >        enum: [1, 4, 8, 16, 32, 64, 128, 256]
> > > > >  
> > > > > +  snps,des-wr-reqinfo:
> > > > > +    description: Value for DESEWRREQIN of GSBUSCFG0 register.
> > > > > +      ----------------------------------------------------------------
> > > > > +       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
> > > > > +      ----------------------------------------------------------------
> > > > > +      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)
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > > > +    maxItem: 15
> > > > > +
> > > > > +  snps,des-rd-reqinfo:
> > > > > +    description: Value for DESRDREQIN of GSBUSCFG0 register. ref
> > > > > +      snps,des-wr-reqinfo
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > > > +    maxItem: 15
> > > > > +
> > > > > +  snps,dat-wr-reqinfo:
> > > > > +    description: Value for DATWRREQIN of GSBUSCFG0 register. ref
> > > > > +      snps,des-wr-reqinfo
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > > > +    maxItem: 15
> > > > > +
> > > > > +  snps,des-wr-reqinfo:
> > > > > +    description: Value for DATWRREQIN of GSBUSCFG0 register. ref
> > > > > +      snps,des-wr-reqinfo
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > > > +    maxItem: 15
> > > > > +
> > > > >    num-hc-interrupters:
> > > > >      maximum: 8
> > > > >      default: 1
> > > > > -- 
> > > > > 2.34.1
> > > > > 
> > > 
> > > 
> 
> 

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

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Frank Li]

On Tue, Jan 23, 2024 at 06:42:27PM +0000, Conor Dooley wrote:
> On Tue, Jan 23, 2024 at 01:02:21PM -0500, Frank Li wrote:
> > On Tue, Jan 23, 2024 at 05:51:48PM +0000, Conor Dooley wrote:
> > > On Tue, Jan 23, 2024 at 12:49:27PM -0500, Frank Li wrote:
> > > > On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
> > > > > On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
> > > > > > Add device tree binding allow platform overwrite default value of *REQIN in
> > > > > > GSBUSCFG0.
> > > > > 
> > > > > Why might a platform actually want to do this? Why does this need to be
> > > > > set at the board level and being aware of which SoC is in use is not
> > > > > sufficient for the driver to set the correct values?
> > > > 
> > > > In snps,dwc3.yaml, there are already similary proptery, such as
> > > > snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
> > > > driver keep consistent. And not all platform try enable hardware
> > > > dma_cohenrence. It is configable for difference platform.
> > > 
> > > When you say "platform", what do you mean? I understand that term to
> > > mean a combination of board, soc and firmware.
> > 
> > In my company's environment, "platform" is "board". I will use "board" in
> > future. Is it big difference here?
> 
> Nah, that's close enough that it makes no difference here.
> 
> I'd still like an explanation for why a platform would need to actually
> set these properties though, and why information about coherency cannot
> be determined from whether or not the boss the usb controller is on is
> communicated to be dma coherent via the existing devicetree properties
> for that purpose.

Actually, I am not very clear about reason. I guest maybe treat off power
consumption and performance.

What's your judgement about proptery, which should be in dts. Such as
reg, clk, reset, dma and irq, which is tighted with SOC. It is the fixed
value for every SOC. The board dts never change these.

But for this "snps,*-reqinfo", you don't like put into dts? 

There are similar case at 2019,
https://lore.kernel.org/linux-devicetree/CADRPPNRHOda+ZfB25CeqReXBb-MrB1oAeTHF-3muXVExn+G+Dg@mail.gmail.com/
look like, this thread prefer quirk proptery method.

Frank

> 
> Thanks,
> Conor.
> 
> > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > > ---
> > > > > >  .../devicetree/bindings/usb/snps,dwc3.yaml    | 36 +++++++++++++++++++
> > > > > >  1 file changed, 36 insertions(+)
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > > > > > index 8f5d250070c78..43e7fea3f6798 100644
> > > > > > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > > > > > @@ -439,6 +439,42 @@ properties:
> > > > > >      items:
> > > > > >        enum: [1, 4, 8, 16, 32, 64, 128, 256]
> > > > > >  
> > > > > > +  snps,des-wr-reqinfo:
> > > > > > +    description: Value for DESEWRREQIN of GSBUSCFG0 register.
> > > > > > +      ----------------------------------------------------------------
> > > > > > +       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
> > > > > > +      ----------------------------------------------------------------
> > > > > > +      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)
> > > > > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > > > > +    maxItem: 15
> > > > > > +
> > > > > > +  snps,des-rd-reqinfo:
> > > > > > +    description: Value for DESRDREQIN of GSBUSCFG0 register. ref
> > > > > > +      snps,des-wr-reqinfo
> > > > > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > > > > +    maxItem: 15
> > > > > > +
> > > > > > +  snps,dat-wr-reqinfo:
> > > > > > +    description: Value for DATWRREQIN of GSBUSCFG0 register. ref
> > > > > > +      snps,des-wr-reqinfo
> > > > > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > > > > +    maxItem: 15
> > > > > > +
> > > > > > +  snps,des-wr-reqinfo:
> > > > > > +    description: Value for DATWRREQIN of GSBUSCFG0 register. ref
> > > > > > +      snps,des-wr-reqinfo
> > > > > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > > > > +    maxItem: 15
> > > > > > +
> > > > > >    num-hc-interrupters:
> > > > > >      maximum: 8
> > > > > >      default: 1
> > > > > > -- 
> > > > > > 2.34.1
> > > > > > 
> > > > 
> > > > 
> > 
> > 

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Krzysztof Kozlowski]

On 23/01/2024 20:22, Frank Li wrote:
> On Tue, Jan 23, 2024 at 06:42:27PM +0000, Conor Dooley wrote:
>> On Tue, Jan 23, 2024 at 01:02:21PM -0500, Frank Li wrote:
>>> On Tue, Jan 23, 2024 at 05:51:48PM +0000, Conor Dooley wrote:
>>>> On Tue, Jan 23, 2024 at 12:49:27PM -0500, Frank Li wrote:
>>>>> On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
>>>>>> On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
>>>>>>> Add device tree binding allow platform overwrite default value of *REQIN in
>>>>>>> GSBUSCFG0.
>>>>>>
>>>>>> Why might a platform actually want to do this? Why does this need to be
>>>>>> set at the board level and being aware of which SoC is in use is not
>>>>>> sufficient for the driver to set the correct values?
>>>>>
>>>>> In snps,dwc3.yaml, there are already similary proptery, such as
>>>>> snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
>>>>> driver keep consistent. And not all platform try enable hardware
>>>>> dma_cohenrence. It is configable for difference platform.
>>>>
>>>> When you say "platform", what do you mean? I understand that term to
>>>> mean a combination of board, soc and firmware.
>>>
>>> In my company's environment, "platform" is "board". I will use "board" in
>>> future. Is it big difference here?
>>
>> Nah, that's close enough that it makes no difference here.
>>
>> I'd still like an explanation for why a platform would need to actually
>> set these properties though, and why information about coherency cannot
>> be determined from whether or not the boss the usb controller is on is
>> communicated to be dma coherent via the existing devicetree properties
>> for that purpose.
> 
> Actually, I am not very clear about reason. I guest maybe treat off power
> consumption and performance.
> 
> What's your judgement about proptery, which should be in dts. Such as
> reg, clk, reset, dma and irq, which is tighted with SOC. It is the fixed
> value for every SOC. The board dts never change these.

Then it can be deduced from the compatible and there is no need for new
properties.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Frank Li]

On Tue, Jan 23, 2024 at 10:46:39PM +0100, Krzysztof Kozlowski wrote:
> On 23/01/2024 20:22, Frank Li wrote:
> > On Tue, Jan 23, 2024 at 06:42:27PM +0000, Conor Dooley wrote:
> >> On Tue, Jan 23, 2024 at 01:02:21PM -0500, Frank Li wrote:
> >>> On Tue, Jan 23, 2024 at 05:51:48PM +0000, Conor Dooley wrote:
> >>>> On Tue, Jan 23, 2024 at 12:49:27PM -0500, Frank Li wrote:
> >>>>> On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
> >>>>>> On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
> >>>>>>> Add device tree binding allow platform overwrite default value of *REQIN in
> >>>>>>> GSBUSCFG0.
> >>>>>>
> >>>>>> Why might a platform actually want to do this? Why does this need to be
> >>>>>> set at the board level and being aware of which SoC is in use is not
> >>>>>> sufficient for the driver to set the correct values?
> >>>>>
> >>>>> In snps,dwc3.yaml, there are already similary proptery, such as
> >>>>> snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
> >>>>> driver keep consistent. And not all platform try enable hardware
> >>>>> dma_cohenrence. It is configable for difference platform.
> >>>>
> >>>> When you say "platform", what do you mean? I understand that term to
> >>>> mean a combination of board, soc and firmware.
> >>>
> >>> In my company's environment, "platform" is "board". I will use "board" in
> >>> future. Is it big difference here?
> >>
> >> Nah, that's close enough that it makes no difference here.
> >>
> >> I'd still like an explanation for why a platform would need to actually
> >> set these properties though, and why information about coherency cannot
> >> be determined from whether or not the boss the usb controller is on is
> >> communicated to be dma coherent via the existing devicetree properties
> >> for that purpose.
> > 
> > Actually, I am not very clear about reason. I guest maybe treat off power
> > consumption and performance.
> > 
> > What's your judgement about proptery, which should be in dts. Such as
> > reg, clk, reset, dma and irq, which is tighted with SOC. It is the fixed
> > value for every SOC. The board dts never change these.
> 
> Then it can be deduced from the compatible and there is no need for new
> properties.

Okay, I think "*reqinfo" match this. When new Soc(using compatible dwc usb
controller) appear regardless dma-cohorence or not, connect by AXI3 or
AXI4, needn't add new propterties. 

Frank

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Frank Li]

On Tue, Jan 23, 2024 at 05:23:53PM -0500, Frank Li wrote:
> On Tue, Jan 23, 2024 at 10:46:39PM +0100, Krzysztof Kozlowski wrote:
> > On 23/01/2024 20:22, Frank Li wrote:
> > > On Tue, Jan 23, 2024 at 06:42:27PM +0000, Conor Dooley wrote:
> > >> On Tue, Jan 23, 2024 at 01:02:21PM -0500, Frank Li wrote:
> > >>> On Tue, Jan 23, 2024 at 05:51:48PM +0000, Conor Dooley wrote:
> > >>>> On Tue, Jan 23, 2024 at 12:49:27PM -0500, Frank Li wrote:
> > >>>>> On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
> > >>>>>> On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
> > >>>>>>> Add device tree binding allow platform overwrite default value of *REQIN in
> > >>>>>>> GSBUSCFG0.
> > >>>>>>
> > >>>>>> Why might a platform actually want to do this? Why does this need to be
> > >>>>>> set at the board level and being aware of which SoC is in use is not
> > >>>>>> sufficient for the driver to set the correct values?
> > >>>>>
> > >>>>> In snps,dwc3.yaml, there are already similary proptery, such as
> > >>>>> snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
> > >>>>> driver keep consistent. And not all platform try enable hardware
> > >>>>> dma_cohenrence. It is configable for difference platform.
> > >>>>
> > >>>> When you say "platform", what do you mean? I understand that term to
> > >>>> mean a combination of board, soc and firmware.
> > >>>
> > >>> In my company's environment, "platform" is "board". I will use "board" in
> > >>> future. Is it big difference here?
> > >>
> > >> Nah, that's close enough that it makes no difference here.
> > >>
> > >> I'd still like an explanation for why a platform would need to actually
> > >> set these properties though, and why information about coherency cannot
> > >> be determined from whether or not the boss the usb controller is on is
> > >> communicated to be dma coherent via the existing devicetree properties
> > >> for that purpose.
> > > 
> > > Actually, I am not very clear about reason. I guest maybe treat off power
> > > consumption and performance.
> > > 
> > > What's your judgement about proptery, which should be in dts. Such as
> > > reg, clk, reset, dma and irq, which is tighted with SOC. It is the fixed
> > > value for every SOC. The board dts never change these.
> > 
> > Then it can be deduced from the compatible and there is no need for new
> > properties.
> 
> Okay, I think "*reqinfo" match this. When new Soc(using compatible dwc usb
> controller) appear regardless dma-cohorence or not, connect by AXI3 or
> AXI4, needn't add new propterties. 

Anyone have objection? I will prepare v2 to fix rob's bot error.

Frank

> 
> Frank
> 
> > 
> > Best regards,
> > Krzysztof
> > 

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Conor Dooley]

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

On Mon, Jan 29, 2024 at 10:19:24AM -0500, Frank Li wrote:
> On Tue, Jan 23, 2024 at 05:23:53PM -0500, Frank Li wrote:
> > On Tue, Jan 23, 2024 at 10:46:39PM +0100, Krzysztof Kozlowski wrote:
> > > On 23/01/2024 20:22, Frank Li wrote:
> > > > On Tue, Jan 23, 2024 at 06:42:27PM +0000, Conor Dooley wrote:
> > > >> On Tue, Jan 23, 2024 at 01:02:21PM -0500, Frank Li wrote:
> > > >>> On Tue, Jan 23, 2024 at 05:51:48PM +0000, Conor Dooley wrote:
> > > >>>> On Tue, Jan 23, 2024 at 12:49:27PM -0500, Frank Li wrote:
> > > >>>>> On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
> > > >>>>>> On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
> > > >>>>>>> Add device tree binding allow platform overwrite default value of *REQIN in
> > > >>>>>>> GSBUSCFG0.
> > > >>>>>>
> > > >>>>>> Why might a platform actually want to do this? Why does this need to be
> > > >>>>>> set at the board level and being aware of which SoC is in use is not
> > > >>>>>> sufficient for the driver to set the correct values?
> > > >>>>>
> > > >>>>> In snps,dwc3.yaml, there are already similary proptery, such as
> > > >>>>> snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
> > > >>>>> driver keep consistent. And not all platform try enable hardware
> > > >>>>> dma_cohenrence. It is configable for difference platform.
> > > >>>>
> > > >>>> When you say "platform", what do you mean? I understand that term to
> > > >>>> mean a combination of board, soc and firmware.
> > > >>>
> > > >>> In my company's environment, "platform" is "board". I will use "board" in
> > > >>> future. Is it big difference here?
> > > >>
> > > >> Nah, that's close enough that it makes no difference here.
> > > >>
> > > >> I'd still like an explanation for why a platform would need to actually
> > > >> set these properties though, and why information about coherency cannot
> > > >> be determined from whether or not the boss the usb controller is on is
> > > >> communicated to be dma coherent via the existing devicetree properties
> > > >> for that purpose.
> > > > 
> > > > Actually, I am not very clear about reason. I guest maybe treat off power
> > > > consumption and performance.
> > > > 
> > > > What's your judgement about proptery, which should be in dts. Such as
> > > > reg, clk, reset, dma and irq, which is tighted with SOC. It is the fixed
> > > > value for every SOC. The board dts never change these.
> > > 
> > > Then it can be deduced from the compatible and there is no need for new
> > > properties.
> > 
> > Okay, I think "*reqinfo" match this. When new Soc(using compatible dwc usb
> > controller) appear regardless dma-cohorence or not, connect by AXI3 or
> > AXI4, needn't add new propterties. 
> 
> Anyone have objection? I will prepare v2 to fix rob's bot error.

I'm not sure what you want me to object to/not object to.
Your last message said "needn't add new propterties", seemingly in
agreement with Krzysztoff saying that it can be deduced from the
compatible. That seems like a good way forward for me.

Thanks,
Conor.

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

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Frank Li]

On Mon, Jan 29, 2024 at 04:49:21PM +0000, Conor Dooley wrote:
> On Mon, Jan 29, 2024 at 10:19:24AM -0500, Frank Li wrote:
> > On Tue, Jan 23, 2024 at 05:23:53PM -0500, Frank Li wrote:
> > > On Tue, Jan 23, 2024 at 10:46:39PM +0100, Krzysztof Kozlowski wrote:
> > > > On 23/01/2024 20:22, Frank Li wrote:
> > > > > On Tue, Jan 23, 2024 at 06:42:27PM +0000, Conor Dooley wrote:
> > > > >> On Tue, Jan 23, 2024 at 01:02:21PM -0500, Frank Li wrote:
> > > > >>> On Tue, Jan 23, 2024 at 05:51:48PM +0000, Conor Dooley wrote:
> > > > >>>> On Tue, Jan 23, 2024 at 12:49:27PM -0500, Frank Li wrote:
> > > > >>>>> On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
> > > > >>>>>> On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
> > > > >>>>>>> Add device tree binding allow platform overwrite default value of *REQIN in
> > > > >>>>>>> GSBUSCFG0.
> > > > >>>>>>
> > > > >>>>>> Why might a platform actually want to do this? Why does this need to be
> > > > >>>>>> set at the board level and being aware of which SoC is in use is not
> > > > >>>>>> sufficient for the driver to set the correct values?
> > > > >>>>>
> > > > >>>>> In snps,dwc3.yaml, there are already similary proptery, such as
> > > > >>>>> snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
> > > > >>>>> driver keep consistent. And not all platform try enable hardware
> > > > >>>>> dma_cohenrence. It is configable for difference platform.
> > > > >>>>
> > > > >>>> When you say "platform", what do you mean? I understand that term to
> > > > >>>> mean a combination of board, soc and firmware.
> > > > >>>
> > > > >>> In my company's environment, "platform" is "board". I will use "board" in
> > > > >>> future. Is it big difference here?
> > > > >>
> > > > >> Nah, that's close enough that it makes no difference here.
> > > > >>
> > > > >> I'd still like an explanation for why a platform would need to actually
> > > > >> set these properties though, and why information about coherency cannot
> > > > >> be determined from whether or not the boss the usb controller is on is
> > > > >> communicated to be dma coherent via the existing devicetree properties
> > > > >> for that purpose.
> > > > > 
> > > > > Actually, I am not very clear about reason. I guest maybe treat off power
> > > > > consumption and performance.
> > > > > 
> > > > > What's your judgement about proptery, which should be in dts. Such as
> > > > > reg, clk, reset, dma and irq, which is tighted with SOC. It is the fixed
> > > > > value for every SOC. The board dts never change these.
> > > > 
> > > > Then it can be deduced from the compatible and there is no need for new
> > > > properties.
> > > 
> > > Okay, I think "*reqinfo" match this. When new Soc(using compatible dwc usb
> > > controller) appear regardless dma-cohorence or not, connect by AXI3 or
> > > AXI4, needn't add new propterties. 
> > 
> > Anyone have objection? I will prepare v2 to fix rob's bot error.
> 
> I'm not sure what you want me to object to/not object to.
> Your last message said "needn't add new propterties", seemingly in
> agreement with Krzysztoff saying that it can be deduced from the
> compatible. That seems like a good way forward for me.

Okay, let me clear it again. dwc usb is quite common IP. The below is
what reason why need "*reginfo* instead of using compatible string.

1. *reginfo* property is decscript hardware behevior, which will be changed
at difference SOC.
2. it may change at board level according to if enable dma coherence.
3. dwc core part is quite common, all SOC using common "snps, dwc3" as
core-part, all soc specific "nxp, dwc3 *", "qcom, dwc3*" is used for glue
logic part.
4. using *reginfo* can reduce add more strange compatible string such as
"nxp, dwc3-core" ...
5. *reginfo* property likes "reg", "clk", and align what Kryzystoff said.
"reg", "clk" is fixed for specfic SOC. These can help reduce "compatible"
string number. "reginfo" do the same work as "reg", "clk" ..

Frank


> 
> Thanks,
> Conor.

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Krzysztof Kozlowski]

On 29/01/2024 18:41, Frank Li wrote:
> On Mon, Jan 29, 2024 at 04:49:21PM +0000, Conor Dooley wrote:
>> On Mon, Jan 29, 2024 at 10:19:24AM -0500, Frank Li wrote:
>>> On Tue, Jan 23, 2024 at 05:23:53PM -0500, Frank Li wrote:
>>>> On Tue, Jan 23, 2024 at 10:46:39PM +0100, Krzysztof Kozlowski wrote:
>>>>> On 23/01/2024 20:22, Frank Li wrote:
>>>>>> On Tue, Jan 23, 2024 at 06:42:27PM +0000, Conor Dooley wrote:
>>>>>>> On Tue, Jan 23, 2024 at 01:02:21PM -0500, Frank Li wrote:
>>>>>>>> On Tue, Jan 23, 2024 at 05:51:48PM +0000, Conor Dooley wrote:
>>>>>>>>> On Tue, Jan 23, 2024 at 12:49:27PM -0500, Frank Li wrote:
>>>>>>>>>> On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
>>>>>>>>>>> On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
>>>>>>>>>>>> Add device tree binding allow platform overwrite default value of *REQIN in
>>>>>>>>>>>> GSBUSCFG0.
>>>>>>>>>>>
>>>>>>>>>>> Why might a platform actually want to do this? Why does this need to be
>>>>>>>>>>> set at the board level and being aware of which SoC is in use is not
>>>>>>>>>>> sufficient for the driver to set the correct values?
>>>>>>>>>>
>>>>>>>>>> In snps,dwc3.yaml, there are already similary proptery, such as
>>>>>>>>>> snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
>>>>>>>>>> driver keep consistent. And not all platform try enable hardware
>>>>>>>>>> dma_cohenrence. It is configable for difference platform.
>>>>>>>>>
>>>>>>>>> When you say "platform", what do you mean? I understand that term to
>>>>>>>>> mean a combination of board, soc and firmware.
>>>>>>>>
>>>>>>>> In my company's environment, "platform" is "board". I will use "board" in
>>>>>>>> future. Is it big difference here?
>>>>>>>
>>>>>>> Nah, that's close enough that it makes no difference here.
>>>>>>>
>>>>>>> I'd still like an explanation for why a platform would need to actually
>>>>>>> set these properties though, and why information about coherency cannot
>>>>>>> be determined from whether or not the boss the usb controller is on is
>>>>>>> communicated to be dma coherent via the existing devicetree properties
>>>>>>> for that purpose.
>>>>>>
>>>>>> Actually, I am not very clear about reason. I guest maybe treat off power
>>>>>> consumption and performance.
>>>>>>
>>>>>> What's your judgement about proptery, which should be in dts. Such as
>>>>>> reg, clk, reset, dma and irq, which is tighted with SOC. It is the fixed
>>>>>> value for every SOC. The board dts never change these.
>>>>>
>>>>> Then it can be deduced from the compatible and there is no need for new
>>>>> properties.
>>>>
>>>> Okay, I think "*reqinfo" match this. When new Soc(using compatible dwc usb
>>>> controller) appear regardless dma-cohorence or not, connect by AXI3 or
>>>> AXI4, needn't add new propterties. 
>>>
>>> Anyone have objection? I will prepare v2 to fix rob's bot error.
>>
>> I'm not sure what you want me to object to/not object to.
>> Your last message said "needn't add new propterties", seemingly in
>> agreement with Krzysztoff saying that it can be deduced from the
>> compatible. That seems like a good way forward for me.
> 
> Okay, let me clear it again. dwc usb is quite common IP. The below is
> what reason why need "*reginfo* instead of using compatible string.
> 
> 1. *reginfo* property is decscript hardware behevior, which will be changed
> at difference SOC.
> 2. it may change at board level according to if enable dma coherence.

dma coherence is not a board property. Anyway, you said it will never
change in the board.

> 3. dwc core part is quite common, all SOC using common "snps, dwc3" as
> core-part, all soc specific "nxp, dwc3 *", "qcom, dwc3*" is used for glue
> logic part.

And all should be having dedicated compatibles.

> 4. using *reginfo* can reduce add more strange compatible string such as
> "nxp, dwc3-core" ...
> 5. *reginfo* property likes "reg", "clk", and align what Kryzystoff said.
> "reg", "clk" is fixed for specfic SOC. These can help reduce "compatible"
> string number. "reginfo" do the same work as "reg", "clk" ..

So again, reginfo is fixed for specific SoC? So it can be deduced from
compatible.

I don't know what to say more here... so let's be clear that you
understood me:

NAK

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Frank Li]

On Tue, Jan 30, 2024 at 08:40:29AM +0100, Krzysztof Kozlowski wrote:
> On 29/01/2024 18:41, Frank Li wrote:
> > On Mon, Jan 29, 2024 at 04:49:21PM +0000, Conor Dooley wrote:
> >> On Mon, Jan 29, 2024 at 10:19:24AM -0500, Frank Li wrote:
> >>> On Tue, Jan 23, 2024 at 05:23:53PM -0500, Frank Li wrote:
> >>>> On Tue, Jan 23, 2024 at 10:46:39PM +0100, Krzysztof Kozlowski wrote:
> >>>>> On 23/01/2024 20:22, Frank Li wrote:
> >>>>>> On Tue, Jan 23, 2024 at 06:42:27PM +0000, Conor Dooley wrote:
> >>>>>>> On Tue, Jan 23, 2024 at 01:02:21PM -0500, Frank Li wrote:
> >>>>>>>> On Tue, Jan 23, 2024 at 05:51:48PM +0000, Conor Dooley wrote:
> >>>>>>>>> On Tue, Jan 23, 2024 at 12:49:27PM -0500, Frank Li wrote:
> >>>>>>>>>> On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
> >>>>>>>>>>> On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
> >>>>>>>>>>>> Add device tree binding allow platform overwrite default value of *REQIN in
> >>>>>>>>>>>> GSBUSCFG0.
> >>>>>>>>>>>
> >>>>>>>>>>> Why might a platform actually want to do this? Why does this need to be
> >>>>>>>>>>> set at the board level and being aware of which SoC is in use is not
> >>>>>>>>>>> sufficient for the driver to set the correct values?
> >>>>>>>>>>
> >>>>>>>>>> In snps,dwc3.yaml, there are already similary proptery, such as
> >>>>>>>>>> snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
> >>>>>>>>>> driver keep consistent. And not all platform try enable hardware
> >>>>>>>>>> dma_cohenrence. It is configable for difference platform.
> >>>>>>>>>
> >>>>>>>>> When you say "platform", what do you mean? I understand that term to
> >>>>>>>>> mean a combination of board, soc and firmware.
> >>>>>>>>
> >>>>>>>> In my company's environment, "platform" is "board". I will use "board" in
> >>>>>>>> future. Is it big difference here?
> >>>>>>>
> >>>>>>> Nah, that's close enough that it makes no difference here.
> >>>>>>>
> >>>>>>> I'd still like an explanation for why a platform would need to actually
> >>>>>>> set these properties though, and why information about coherency cannot
> >>>>>>> be determined from whether or not the boss the usb controller is on is
> >>>>>>> communicated to be dma coherent via the existing devicetree properties
> >>>>>>> for that purpose.
> >>>>>>
> >>>>>> Actually, I am not very clear about reason. I guest maybe treat off power
> >>>>>> consumption and performance.
> >>>>>>
> >>>>>> What's your judgement about proptery, which should be in dts. Such as
> >>>>>> reg, clk, reset, dma and irq, which is tighted with SOC. It is the fixed
> >>>>>> value for every SOC. The board dts never change these.
> >>>>>
> >>>>> Then it can be deduced from the compatible and there is no need for new
> >>>>> properties.
> >>>>
> >>>> Okay, I think "*reqinfo" match this. When new Soc(using compatible dwc usb
> >>>> controller) appear regardless dma-cohorence or not, connect by AXI3 or
> >>>> AXI4, needn't add new propterties. 
> >>>
> >>> Anyone have objection? I will prepare v2 to fix rob's bot error.
> >>
> >> I'm not sure what you want me to object to/not object to.
> >> Your last message said "needn't add new propterties", seemingly in
> >> agreement with Krzysztoff saying that it can be deduced from the
> >> compatible. That seems like a good way forward for me.
> > 
> > Okay, let me clear it again. dwc usb is quite common IP. The below is
> > what reason why need "*reginfo* instead of using compatible string.
> > 
> > 1. *reginfo* property is decscript hardware behevior, which will be changed
> > at difference SOC.
> > 2. it may change at board level according to if enable dma coherence.
> 
> dma coherence is not a board property. Anyway, you said it will never
> change in the board.

Sorry, let's correct what my previous said. There are two kinds bus in
system, one is dma_coherence, the other is none dma_coherence. There are
some dwc usb core ip, which is the exact the same. Some connect to
dma_coherence bus, some connect to non-dma_coherence bus.

So dma_coherence will be varible for this case. we need set *reginfo* for
dwc usb core, which connnect to dma_coherence bus.  not set "reginfo* for
the dwc usb core, which connect to none dma_coherence bus.

If use difference compatible string, that's means using difference
compatible string to descript one same hardware. You reject this method at
my svc-i3c-master\svc-i3c-target patches.

Frank

> 
> > 3. dwc core part is quite common, all SOC using common "snps, dwc3" as
> > core-part, all soc specific "nxp, dwc3 *", "qcom, dwc3*" is used for glue
> > logic part.
> 
> And all should be having dedicated compatibles.
> 
> > 4. using *reginfo* can reduce add more strange compatible string such as
> > "nxp, dwc3-core" ...
> > 5. *reginfo* property likes "reg", "clk", and align what Kryzystoff said.
> > "reg", "clk" is fixed for specfic SOC. These can help reduce "compatible"
> > string number. "reginfo" do the same work as "reg", "clk" ..
> 
> So again, reginfo is fixed for specific SoC? So it can be deduced from
> compatible.
> 
> I don't know what to say more here... so let's be clear that you
> understood me:
> 
> NAK
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Krzysztof Kozlowski]

On 30/01/2024 16:20, Frank Li wrote:
> On Tue, Jan 30, 2024 at 08:40:29AM +0100, Krzysztof Kozlowski wrote:
>> On 29/01/2024 18:41, Frank Li wrote:
>>> On Mon, Jan 29, 2024 at 04:49:21PM +0000, Conor Dooley wrote:
>>>> On Mon, Jan 29, 2024 at 10:19:24AM -0500, Frank Li wrote:
>>>>> On Tue, Jan 23, 2024 at 05:23:53PM -0500, Frank Li wrote:
>>>>>> On Tue, Jan 23, 2024 at 10:46:39PM +0100, Krzysztof Kozlowski wrote:
>>>>>>> On 23/01/2024 20:22, Frank Li wrote:
>>>>>>>> On Tue, Jan 23, 2024 at 06:42:27PM +0000, Conor Dooley wrote:
>>>>>>>>> On Tue, Jan 23, 2024 at 01:02:21PM -0500, Frank Li wrote:
>>>>>>>>>> On Tue, Jan 23, 2024 at 05:51:48PM +0000, Conor Dooley wrote:
>>>>>>>>>>> On Tue, Jan 23, 2024 at 12:49:27PM -0500, Frank Li wrote:
>>>>>>>>>>>> On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
>>>>>>>>>>>>> On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
>>>>>>>>>>>>>> Add device tree binding allow platform overwrite default value of *REQIN in
>>>>>>>>>>>>>> GSBUSCFG0.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Why might a platform actually want to do this? Why does this need to be
>>>>>>>>>>>>> set at the board level and being aware of which SoC is in use is not
>>>>>>>>>>>>> sufficient for the driver to set the correct values?
>>>>>>>>>>>>
>>>>>>>>>>>> In snps,dwc3.yaml, there are already similary proptery, such as
>>>>>>>>>>>> snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
>>>>>>>>>>>> driver keep consistent. And not all platform try enable hardware
>>>>>>>>>>>> dma_cohenrence. It is configable for difference platform.
>>>>>>>>>>>
>>>>>>>>>>> When you say "platform", what do you mean? I understand that term to
>>>>>>>>>>> mean a combination of board, soc and firmware.
>>>>>>>>>>
>>>>>>>>>> In my company's environment, "platform" is "board". I will use "board" in
>>>>>>>>>> future. Is it big difference here?
>>>>>>>>>
>>>>>>>>> Nah, that's close enough that it makes no difference here.
>>>>>>>>>
>>>>>>>>> I'd still like an explanation for why a platform would need to actually
>>>>>>>>> set these properties though, and why information about coherency cannot
>>>>>>>>> be determined from whether or not the boss the usb controller is on is
>>>>>>>>> communicated to be dma coherent via the existing devicetree properties
>>>>>>>>> for that purpose.
>>>>>>>>
>>>>>>>> Actually, I am not very clear about reason. I guest maybe treat off power
>>>>>>>> consumption and performance.
>>>>>>>>
>>>>>>>> What's your judgement about proptery, which should be in dts. Such as
>>>>>>>> reg, clk, reset, dma and irq, which is tighted with SOC. It is the fixed
>>>>>>>> value for every SOC. The board dts never change these.
>>>>>>>
>>>>>>> Then it can be deduced from the compatible and there is no need for new
>>>>>>> properties.
>>>>>>
>>>>>> Okay, I think "*reqinfo" match this. When new Soc(using compatible dwc usb
>>>>>> controller) appear regardless dma-cohorence or not, connect by AXI3 or
>>>>>> AXI4, needn't add new propterties. 
>>>>>
>>>>> Anyone have objection? I will prepare v2 to fix rob's bot error.
>>>>
>>>> I'm not sure what you want me to object to/not object to.
>>>> Your last message said "needn't add new propterties", seemingly in
>>>> agreement with Krzysztoff saying that it can be deduced from the
>>>> compatible. That seems like a good way forward for me.
>>>
>>> Okay, let me clear it again. dwc usb is quite common IP. The below is
>>> what reason why need "*reginfo* instead of using compatible string.
>>>
>>> 1. *reginfo* property is decscript hardware behevior, which will be changed
>>> at difference SOC.
>>> 2. it may change at board level according to if enable dma coherence.
>>
>> dma coherence is not a board property. Anyway, you said it will never
>> change in the board.
> 
> Sorry, let's correct what my previous said. There are two kinds bus in
> system, one is dma_coherence, the other is none dma_coherence. There are
> some dwc usb core ip, which is the exact the same. Some connect to
> dma_coherence bus, some connect to non-dma_coherence bus.
> 
> So dma_coherence will be varible for this case. we need set *reginfo* for
> dwc usb core, which connnect to dma_coherence bus.  not set "reginfo* for
> the dwc usb core, which connect to none dma_coherence bus.

OK, that makes sense. Please provide link to upstream DTS
(mainline/next/lore link/other upstream projects) showing this.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Frank Li]

On Tue, Jan 30, 2024 at 05:10:54PM +0100, Krzysztof Kozlowski wrote:
> On 30/01/2024 16:20, Frank Li wrote:
> > On Tue, Jan 30, 2024 at 08:40:29AM +0100, Krzysztof Kozlowski wrote:
> >> On 29/01/2024 18:41, Frank Li wrote:
> >>> On Mon, Jan 29, 2024 at 04:49:21PM +0000, Conor Dooley wrote:
> >>>> On Mon, Jan 29, 2024 at 10:19:24AM -0500, Frank Li wrote:
> >>>>> On Tue, Jan 23, 2024 at 05:23:53PM -0500, Frank Li wrote:
> >>>>>> On Tue, Jan 23, 2024 at 10:46:39PM +0100, Krzysztof Kozlowski wrote:
> >>>>>>> On 23/01/2024 20:22, Frank Li wrote:
> >>>>>>>> On Tue, Jan 23, 2024 at 06:42:27PM +0000, Conor Dooley wrote:
> >>>>>>>>> On Tue, Jan 23, 2024 at 01:02:21PM -0500, Frank Li wrote:
> >>>>>>>>>> On Tue, Jan 23, 2024 at 05:51:48PM +0000, Conor Dooley wrote:
> >>>>>>>>>>> On Tue, Jan 23, 2024 at 12:49:27PM -0500, Frank Li wrote:
> >>>>>>>>>>>> On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
> >>>>>>>>>>>>> On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
> >>>>>>>>>>>>>> Add device tree binding allow platform overwrite default value of *REQIN in
> >>>>>>>>>>>>>> GSBUSCFG0.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Why might a platform actually want to do this? Why does this need to be
> >>>>>>>>>>>>> set at the board level and being aware of which SoC is in use is not
> >>>>>>>>>>>>> sufficient for the driver to set the correct values?
> >>>>>>>>>>>>
> >>>>>>>>>>>> In snps,dwc3.yaml, there are already similary proptery, such as
> >>>>>>>>>>>> snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
> >>>>>>>>>>>> driver keep consistent. And not all platform try enable hardware
> >>>>>>>>>>>> dma_cohenrence. It is configable for difference platform.
> >>>>>>>>>>>
> >>>>>>>>>>> When you say "platform", what do you mean? I understand that term to
> >>>>>>>>>>> mean a combination of board, soc and firmware.
> >>>>>>>>>>
> >>>>>>>>>> In my company's environment, "platform" is "board". I will use "board" in
> >>>>>>>>>> future. Is it big difference here?
> >>>>>>>>>
> >>>>>>>>> Nah, that's close enough that it makes no difference here.
> >>>>>>>>>
> >>>>>>>>> I'd still like an explanation for why a platform would need to actually
> >>>>>>>>> set these properties though, and why information about coherency cannot
> >>>>>>>>> be determined from whether or not the boss the usb controller is on is
> >>>>>>>>> communicated to be dma coherent via the existing devicetree properties
> >>>>>>>>> for that purpose.
> >>>>>>>>
> >>>>>>>> Actually, I am not very clear about reason. I guest maybe treat off power
> >>>>>>>> consumption and performance.
> >>>>>>>>
> >>>>>>>> What's your judgement about proptery, which should be in dts. Such as
> >>>>>>>> reg, clk, reset, dma and irq, which is tighted with SOC. It is the fixed
> >>>>>>>> value for every SOC. The board dts never change these.
> >>>>>>>
> >>>>>>> Then it can be deduced from the compatible and there is no need for new
> >>>>>>> properties.
> >>>>>>
> >>>>>> Okay, I think "*reqinfo" match this. When new Soc(using compatible dwc usb
> >>>>>> controller) appear regardless dma-cohorence or not, connect by AXI3 or
> >>>>>> AXI4, needn't add new propterties. 
> >>>>>
> >>>>> Anyone have objection? I will prepare v2 to fix rob's bot error.
> >>>>
> >>>> I'm not sure what you want me to object to/not object to.
> >>>> Your last message said "needn't add new propterties", seemingly in
> >>>> agreement with Krzysztoff saying that it can be deduced from the
> >>>> compatible. That seems like a good way forward for me.
> >>>
> >>> Okay, let me clear it again. dwc usb is quite common IP. The below is
> >>> what reason why need "*reginfo* instead of using compatible string.
> >>>
> >>> 1. *reginfo* property is decscript hardware behevior, which will be changed
> >>> at difference SOC.
> >>> 2. it may change at board level according to if enable dma coherence.
> >>
> >> dma coherence is not a board property. Anyway, you said it will never
> >> change in the board.
> > 
> > Sorry, let's correct what my previous said. There are two kinds bus in
> > system, one is dma_coherence, the other is none dma_coherence. There are
> > some dwc usb core ip, which is the exact the same. Some connect to
> > dma_coherence bus, some connect to non-dma_coherence bus.
> > 
> > So dma_coherence will be varible for this case. we need set *reginfo* for
> > dwc usb core, which connnect to dma_coherence bus.  not set "reginfo* for
> > the dwc usb core, which connect to none dma_coherence bus.
> 
> OK, that makes sense. Please provide link to upstream DTS
> (mainline/next/lore link/other upstream projects) showing this.

Look like I still have not found the real case yet. But I think it is
existed (maybe not upstream yet). Can I forward using this method?

Frank

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Krzysztof Kozlowski]

On 08/02/2024 22:20, Frank Li wrote:
> On Tue, Jan 30, 2024 at 05:10:54PM +0100, Krzysztof Kozlowski wrote:
>> On 30/01/2024 16:20, Frank Li wrote:
>>> On Tue, Jan 30, 2024 at 08:40:29AM +0100, Krzysztof Kozlowski wrote:
>>>> On 29/01/2024 18:41, Frank Li wrote:
>>>>> On Mon, Jan 29, 2024 at 04:49:21PM +0000, Conor Dooley wrote:
>>>>>> On Mon, Jan 29, 2024 at 10:19:24AM -0500, Frank Li wrote:
>>>>>>> On Tue, Jan 23, 2024 at 05:23:53PM -0500, Frank Li wrote:
>>>>>>>> On Tue, Jan 23, 2024 at 10:46:39PM +0100, Krzysztof Kozlowski wrote:
>>>>>>>>> On 23/01/2024 20:22, Frank Li wrote:
>>>>>>>>>> On Tue, Jan 23, 2024 at 06:42:27PM +0000, Conor Dooley wrote:
>>>>>>>>>>> On Tue, Jan 23, 2024 at 01:02:21PM -0500, Frank Li wrote:
>>>>>>>>>>>> On Tue, Jan 23, 2024 at 05:51:48PM +0000, Conor Dooley wrote:
>>>>>>>>>>>>> On Tue, Jan 23, 2024 at 12:49:27PM -0500, Frank Li wrote:
>>>>>>>>>>>>>> On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
>>>>>>>>>>>>>>> On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
>>>>>>>>>>>>>>>> Add device tree binding allow platform overwrite default value of *REQIN in
>>>>>>>>>>>>>>>> GSBUSCFG0.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Why might a platform actually want to do this? Why does this need to be
>>>>>>>>>>>>>>> set at the board level and being aware of which SoC is in use is not
>>>>>>>>>>>>>>> sufficient for the driver to set the correct values?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> In snps,dwc3.yaml, there are already similary proptery, such as
>>>>>>>>>>>>>> snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
>>>>>>>>>>>>>> driver keep consistent. And not all platform try enable hardware
>>>>>>>>>>>>>> dma_cohenrence. It is configable for difference platform.
>>>>>>>>>>>>>
>>>>>>>>>>>>> When you say "platform", what do you mean? I understand that term to
>>>>>>>>>>>>> mean a combination of board, soc and firmware.
>>>>>>>>>>>>
>>>>>>>>>>>> In my company's environment, "platform" is "board". I will use "board" in
>>>>>>>>>>>> future. Is it big difference here?
>>>>>>>>>>>
>>>>>>>>>>> Nah, that's close enough that it makes no difference here.
>>>>>>>>>>>
>>>>>>>>>>> I'd still like an explanation for why a platform would need to actually
>>>>>>>>>>> set these properties though, and why information about coherency cannot
>>>>>>>>>>> be determined from whether or not the boss the usb controller is on is
>>>>>>>>>>> communicated to be dma coherent via the existing devicetree properties
>>>>>>>>>>> for that purpose.
>>>>>>>>>>
>>>>>>>>>> Actually, I am not very clear about reason. I guest maybe treat off power
>>>>>>>>>> consumption and performance.
>>>>>>>>>>
>>>>>>>>>> What's your judgement about proptery, which should be in dts. Such as
>>>>>>>>>> reg, clk, reset, dma and irq, which is tighted with SOC. It is the fixed
>>>>>>>>>> value for every SOC. The board dts never change these.
>>>>>>>>>
>>>>>>>>> Then it can be deduced from the compatible and there is no need for new
>>>>>>>>> properties.
>>>>>>>>
>>>>>>>> Okay, I think "*reqinfo" match this. When new Soc(using compatible dwc usb
>>>>>>>> controller) appear regardless dma-cohorence or not, connect by AXI3 or
>>>>>>>> AXI4, needn't add new propterties. 
>>>>>>>
>>>>>>> Anyone have objection? I will prepare v2 to fix rob's bot error.
>>>>>>
>>>>>> I'm not sure what you want me to object to/not object to.
>>>>>> Your last message said "needn't add new propterties", seemingly in
>>>>>> agreement with Krzysztoff saying that it can be deduced from the
>>>>>> compatible. That seems like a good way forward for me.
>>>>>
>>>>> Okay, let me clear it again. dwc usb is quite common IP. The below is
>>>>> what reason why need "*reginfo* instead of using compatible string.
>>>>>
>>>>> 1. *reginfo* property is decscript hardware behevior, which will be changed
>>>>> at difference SOC.
>>>>> 2. it may change at board level according to if enable dma coherence.
>>>>
>>>> dma coherence is not a board property. Anyway, you said it will never
>>>> change in the board.
>>>
>>> Sorry, let's correct what my previous said. There are two kinds bus in
>>> system, one is dma_coherence, the other is none dma_coherence. There are
>>> some dwc usb core ip, which is the exact the same. Some connect to
>>> dma_coherence bus, some connect to non-dma_coherence bus.
>>>
>>> So dma_coherence will be varible for this case. we need set *reginfo* for
>>> dwc usb core, which connnect to dma_coherence bus.  not set "reginfo* for
>>> the dwc usb core, which connect to none dma_coherence bus.
>>
>> OK, that makes sense. Please provide link to upstream DTS
>> (mainline/next/lore link/other upstream projects) showing this.
> 
> Look like I still have not found the real case yet. But I think it is
> existed (maybe not upstream yet). Can I forward using this method?

No, because downstream code does things wrong and does not follow basic
principles...


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Rob Herring]

On Tue, 23 Jan 2024 12:02:05 -0500, Frank Li wrote:
> Add device tree binding allow platform overwrite default value of *REQIN in
> GSBUSCFG0.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  .../devicetree/bindings/usb/snps,dwc3.yaml    | 36 +++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/usb/snps,dwc3.yaml:447:43: [error] syntax error: mapping values are not allowed here (syntax)

dtschema/dtc warnings/errors:
make[2]: *** Deleting file 'Documentation/devicetree/bindings/usb/snps,dwc3.example.dts'
Documentation/devicetree/bindings/usb/snps,dwc3.yaml:447:43: mapping values are not allowed in this context
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/usb/snps,dwc3.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml:
mapping values are not allowed in this context
  in "<unicode string>", line 447, column 43
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/samsung,exynos-dwc3.yaml:
mapping values are not allowed in this context
  in "<unicode string>", line 447, column 43
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/fsl,imx8mp-dwc3.yaml:
mapping values are not allowed in this context
  in "<unicode string>", line 447, column 43
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml:
mapping values are not allowed in this context
  in "<unicode string>", line 447, column 43
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/dwc3-xilinx.yaml:
mapping values are not allowed in this context
  in "<unicode string>", line 447, column 43
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml:
mapping values are not allowed in this context
  in "<unicode string>", line 447, column 43
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml:
mapping values are not allowed in this context
  in "<unicode string>", line 447, column 43
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml:
mapping values are not allowed in this context
  in "<unicode string>", line 447, column 43
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml:
mapping values are not allowed in this context
  in "<unicode string>", line 447, column 43
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/realtek,rtd-dwc3.yaml:
mapping values are not allowed in this context
  in "<unicode string>", line 447, column 43
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/fsl,imx8mq-dwc3.yaml:
mapping values are not allowed in this context
  in "<unicode string>", line 447, column 43
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/intel,keembay-dwc3.yaml:
mapping values are not allowed in this context
  in "<unicode string>", line 447, column 43
./Documentation/devicetree/bindings/usb/snps,dwc3.yaml:447:43: mapping values are not allowed in this context
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/snps,dwc3.yaml: ignoring, error parsing file
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1428: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240123170206.3702413-1-Frank.Li@nxp.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.