diff mbox series

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

Message ID 20240123170206.3702413-1-Frank.Li@nxp.com
State New
Headers show
Series [1/2] dt-bindings: usb: dwc3: Add system bus request info | expand

Commit Message

Frank Li Jan. 23, 2024, 5:02 p.m. UTC
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(+)

Comments

Conor Dooley Jan. 23, 2024, 5:27 p.m. UTC | #1
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
>
Conor Dooley Jan. 23, 2024, 5:51 p.m. UTC | #2
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
> > > 
> 
>
Frank Li Jan. 23, 2024, 6:02 p.m. UTC | #3
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
> > > > 
> > 
> >
Conor Dooley Jan. 23, 2024, 6:42 p.m. UTC | #4
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
> > > > > 
> > > 
> > > 
> 
>
Krzysztof Kozlowski Jan. 23, 2024, 9:46 p.m. UTC | #5
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
Frank Li Jan. 23, 2024, 10:23 p.m. UTC | #6
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
>
Frank Li Jan. 29, 2024, 3:19 p.m. UTC | #7
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
> >
Conor Dooley Jan. 29, 2024, 4:49 p.m. UTC | #8
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.
Frank Li Jan. 29, 2024, 5:41 p.m. UTC | #9
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.
Frank Li Jan. 30, 2024, 3:20 p.m. UTC | #10
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
>
Krzysztof Kozlowski Jan. 30, 2024, 4:10 p.m. UTC | #11
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
diff mbox series

Patch

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