diff mbox series

[v3,11/28] dt-bindings: usb: dwc3: Add snps,num-hc-interrupters definition

Message ID 20230308235751.495-12-quic_wcheng@quicinc.com
State Superseded
Headers show
Series Introduce QC USB SND audio offloading support | expand

Commit Message

Wesley Cheng March 8, 2023, 11:57 p.m. UTC
Add a new definition for specifying how many XHCI secondary interrupters
can be allocated.  XHCI in general can potentially support up to 1024
interrupters, which some uses may want to limit depending on how many
users utilize the interrupters.

Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
---
 .../devicetree/bindings/usb/snps,dwc3.yaml          | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Krzysztof Kozlowski March 10, 2023, 8:50 a.m. UTC | #1
On 09/03/2023 00:57, Wesley Cheng wrote:
> Add a new definition for specifying how many XHCI secondary interrupters
> can be allocated.  XHCI in general can potentially support up to 1024
> interrupters, which some uses may want to limit depending on how many
> users utilize the interrupters.
> 
> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
> ---


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Rob Herring (Arm) March 11, 2023, 1:40 p.m. UTC | #2
On Wed, Mar 08, 2023 at 03:57:34PM -0800, Wesley Cheng wrote:
> Add a new definition for specifying how many XHCI secondary interrupters
> can be allocated.  XHCI in general can potentially support up to 1024
> interrupters, which some uses may want to limit depending on how many
> users utilize the interrupters.
> 
> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
> ---
>  .../devicetree/bindings/usb/snps,dwc3.yaml          | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> index be36956af53b..4e2417191f93 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> @@ -359,6 +359,19 @@ properties:
>      items:
>        enum: [1, 4, 8, 16, 32, 64, 128, 256]
>  
> +  snps,num-hc-interrupters:
> +    description:
> +      Defines the maximum number of XHCI host controller interrupters that can
> +      be supported.  The XHCI host controller has support to allocate multiple
> +      event rings, which can be assigned to different clients/users.  The DWC3
> +      controller has a maximum of 8 interrupters.  If this is not defined then
> +      the value will be defaulted to 1.  This parameter is used only when
> +      operating in host mode.

Is this an XHCI or DWC3 feature? The former should be added to the XHCI 
binding.

> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    minimum: 1
> +    maximum: 8
> +    default: 1
> +
>    port:
>      $ref: /schemas/graph.yaml#/properties/port
>      description:
Wesley Cheng Aug. 29, 2023, 2:05 a.m. UTC | #3
Hi Rob,

On 3/11/2023 5:40 AM, Rob Herring wrote:
> On Wed, Mar 08, 2023 at 03:57:34PM -0800, Wesley Cheng wrote:
>> Add a new definition for specifying how many XHCI secondary interrupters
>> can be allocated.  XHCI in general can potentially support up to 1024
>> interrupters, which some uses may want to limit depending on how many
>> users utilize the interrupters.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>> ---
>>   .../devicetree/bindings/usb/snps,dwc3.yaml          | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> index be36956af53b..4e2417191f93 100644
>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> @@ -359,6 +359,19 @@ properties:
>>       items:
>>         enum: [1, 4, 8, 16, 32, 64, 128, 256]
>>   
>> +  snps,num-hc-interrupters:
>> +    description:
>> +      Defines the maximum number of XHCI host controller interrupters that can
>> +      be supported.  The XHCI host controller has support to allocate multiple
>> +      event rings, which can be assigned to different clients/users.  The DWC3
>> +      controller has a maximum of 8 interrupters.  If this is not defined then
>> +      the value will be defaulted to 1.  This parameter is used only when
>> +      operating in host mode.
> 
> Is this an XHCI or DWC3 feature? The former should be added to the XHCI
> binding.
> 

Sorry for the late response...message got routed to a folder I don't 
frequently check...

This is a XHCI feature, but the DWC3 design is built in a way that DWC3 
host initializes the XHCI device and populates the properties associated 
to XHCI dev.

Will update the XHCI yaml as well for non-DWC3 based SoCs.

Thanks
Wesley Cheng
Krzysztof Kozlowski Aug. 29, 2023, 6:33 a.m. UTC | #4
On 29/08/2023 04:05, Wesley Cheng wrote:
> Hi Rob,
> 
> On 3/11/2023 5:40 AM, Rob Herring wrote:
>> On Wed, Mar 08, 2023 at 03:57:34PM -0800, Wesley Cheng wrote:
>>> Add a new definition for specifying how many XHCI secondary interrupters
>>> can be allocated.  XHCI in general can potentially support up to 1024
>>> interrupters, which some uses may want to limit depending on how many
>>> users utilize the interrupters.
>>>
>>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>>> ---
>>>   .../devicetree/bindings/usb/snps,dwc3.yaml          | 13 +++++++++++++
>>>   1 file changed, 13 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>> index be36956af53b..4e2417191f93 100644
>>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>> @@ -359,6 +359,19 @@ properties:
>>>       items:
>>>         enum: [1, 4, 8, 16, 32, 64, 128, 256]
>>>   
>>> +  snps,num-hc-interrupters:
>>> +    description:
>>> +      Defines the maximum number of XHCI host controller interrupters that can
>>> +      be supported.  The XHCI host controller has support to allocate multiple
>>> +      event rings, which can be assigned to different clients/users.  The DWC3
>>> +      controller has a maximum of 8 interrupters.  If this is not defined then
>>> +      the value will be defaulted to 1.  This parameter is used only when
>>> +      operating in host mode.
>>
>> Is this an XHCI or DWC3 feature? The former should be added to the XHCI
>> binding.
>>
> 
> Sorry for the late response...message got routed to a folder I don't 
> frequently check...

That is quite unfortunate, to put our feedback somewhere deep and then
send new versions of patches thinking there is apparently no feedback.
Fix your email process, so our reviews are not ignored. If they are, we
obviously should ignore your patches.

> 
> This is a XHCI feature, but the DWC3 design is built in a way that DWC3 
> host initializes the XHCI device and populates the properties associated 
> to XHCI dev.

You speak about driver now, not bindings. If driver has limitations,
change it. Not really problem of bindings.


Best regards,
Krzysztof
Wesley Cheng Aug. 29, 2023, 7:19 a.m. UTC | #5
Hi Krzysztof,

On 8/28/2023 11:33 PM, Krzysztof Kozlowski wrote:
> On 29/08/2023 04:05, Wesley Cheng wrote:
>> Hi Rob,
>>
>> On 3/11/2023 5:40 AM, Rob Herring wrote:
>>> On Wed, Mar 08, 2023 at 03:57:34PM -0800, Wesley Cheng wrote:
>>>> Add a new definition for specifying how many XHCI secondary interrupters
>>>> can be allocated.  XHCI in general can potentially support up to 1024
>>>> interrupters, which some uses may want to limit depending on how many
>>>> users utilize the interrupters.
>>>>
>>>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>>>> ---
>>>>    .../devicetree/bindings/usb/snps,dwc3.yaml          | 13 +++++++++++++
>>>>    1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>>> index be36956af53b..4e2417191f93 100644
>>>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>>> @@ -359,6 +359,19 @@ properties:
>>>>        items:
>>>>          enum: [1, 4, 8, 16, 32, 64, 128, 256]
>>>>    
>>>> +  snps,num-hc-interrupters:
>>>> +    description:
>>>> +      Defines the maximum number of XHCI host controller interrupters that can
>>>> +      be supported.  The XHCI host controller has support to allocate multiple
>>>> +      event rings, which can be assigned to different clients/users.  The DWC3
>>>> +      controller has a maximum of 8 interrupters.  If this is not defined then
>>>> +      the value will be defaulted to 1.  This parameter is used only when
>>>> +      operating in host mode.
>>>
>>> Is this an XHCI or DWC3 feature? The former should be added to the XHCI
>>> binding.
>>>
>>
>> Sorry for the late response...message got routed to a folder I don't
>> frequently check...
> 
> That is quite unfortunate, to put our feedback somewhere deep and then
> send new versions of patches thinking there is apparently no feedback.
> Fix your email process, so our reviews are not ignored. If they are, we
> obviously should ignore your patches.
> 

Spent some time fixing filters around, so should be addressed now.

>>
>> This is a XHCI feature, but the DWC3 design is built in a way that DWC3
>> host initializes the XHCI device and populates the properties associated
>> to XHCI dev.
> 
> You speak about driver now, not bindings. If driver has limitations,
> change it. Not really problem of bindings.
> 

Its the limitation of the HW.  Depending on how the SNPS DWC3 controller 
was implemented the vendor it can support less than the value mentioned 
in the Synopsys databook.

Thanks
Wesley Cheng
Krzysztof Kozlowski Aug. 29, 2023, 7:42 a.m. UTC | #6
On 29/08/2023 09:19, Wesley Cheng wrote:
>>>
>>> This is a XHCI feature, but the DWC3 design is built in a way that DWC3
>>> host initializes the XHCI device and populates the properties associated
>>> to XHCI dev.
>>
>> You speak about driver now, not bindings. If driver has limitations,
>> change it. Not really problem of bindings.
>>
> 
> Its the limitation of the HW.  Depending on how the SNPS DWC3 controller 
> was implemented the vendor it can support less than the value mentioned 
> in the Synopsys databook.

Then what is "XHCI dev"? A Linux device? Then a driver topic. What is
"populates the properties" - what or whose properties? Of Linux device?
Then a driver topic...

Best regards,
Krzysztof
Wesley Cheng Aug. 29, 2023, 7:50 a.m. UTC | #7
On 8/29/2023 12:42 AM, Krzysztof Kozlowski wrote:
> On 29/08/2023 09:19, Wesley Cheng wrote:
>>>>
>>>> This is a XHCI feature, but the DWC3 design is built in a way that DWC3
>>>> host initializes the XHCI device and populates the properties associated
>>>> to XHCI dev.
>>>
>>> You speak about driver now, not bindings. If driver has limitations,
>>> change it. Not really problem of bindings.
>>>
>>
>> Its the limitation of the HW.  Depending on how the SNPS DWC3 controller
>> was implemented the vendor it can support less than the value mentioned
>> in the Synopsys databook.
> 
> Then what is "XHCI dev"? A Linux device? Then a driver topic. What is
> "populates the properties" - what or whose properties? Of Linux device?
> Then a driver topic...
> 

I will reword it to using XHCI host controller versus using terms like 
device and properties for the next rev.

Thanks
Wesley Cheng
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index be36956af53b..4e2417191f93 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -359,6 +359,19 @@  properties:
     items:
       enum: [1, 4, 8, 16, 32, 64, 128, 256]
 
+  snps,num-hc-interrupters:
+    description:
+      Defines the maximum number of XHCI host controller interrupters that can
+      be supported.  The XHCI host controller has support to allocate multiple
+      event rings, which can be assigned to different clients/users.  The DWC3
+      controller has a maximum of 8 interrupters.  If this is not defined then
+      the value will be defaulted to 1.  This parameter is used only when
+      operating in host mode.
+    $ref: /schemas/types.yaml#/definitions/uint8
+    minimum: 1
+    maximum: 8
+    default: 1
+
   port:
     $ref: /schemas/graph.yaml#/properties/port
     description: