Re: [RFC PATCH 1/4] dt-bindings: usb: usb-xhci: Add xhci-snps-quirks

From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Rob Herring <robh@kernel.org>, Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Cc: Felipe Balbi <balbi@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Mathias Nyman <mathias.nyman@intel.com>,
	John Youn <John.Youn@synopsys.com>,
	Pavan Kondeti <quic_pkondeti@quicinc.com>
Subject: Re: [RFC PATCH 1/4] dt-bindings: usb: usb-xhci: Add xhci-snps-quirks
Date: Thu, 9 Jun 2022 18:11:51 +0000	[thread overview]
Message-ID: <6873a12f-448f-214a-ca01-a08f65ddb0ee@synopsys.com> (raw)
In-Reply-To: <20220609174840.GA4015807-robh@kernel.org>

Hi Rob,

Rob Herring wrote:
> On Fri, Jun 03, 2022 at 07:48:08PM -0700, Thinh Nguyen wrote:
>> Set this property to use xhci-snps extension to handle common Synopsys
>> DWC_usb3x host quirks.
> 
> I don't see why this needs to be in DT.
> 
> The DWC3 stuff is a mess of quirks which doesn't work well. Quirk 
> properties in DT require either knowing the quirk up front (You don't 
> always) or updating the DT on a platform when you find one. Quirks 
> should be implied by the compatible string(s) instead.
> 

Since different vendors share the same Synopsys DWC_usb3x IPs, the
controller's behavior is predictable based on the IP versions. Just
using the compatible strings will become unmanageable when we have the
common behavior across different vendors.

Can we rename this property to "xhci-snps-DWC_usb3x-ip" or something
similar?

The main goal for this device property is to indicate that it's
Synopsys's DWC_usb3x IP. As long as we know this, the xhci-snps glue
extension can handle the fine tuning for the controller's behavior.

We could use compatible string for this goal also, but that would mean
the host devices that go through the dwc3 driver path may not have the
compatible string. (e.g. host on pci bus but get recreated as platform
device). Then we would need a different way to determine that. We could
match the parent device driver for "dwc3", but that implementation looks
fragile.

So, will the device property "xhci-snps-DWC_usb3x-ip" work for you?

Thanks,
Thinh

>>
>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>> ---
>>  Documentation/devicetree/bindings/usb/usb-xhci.yaml | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.yaml b/Documentation/devicetree/bindings/usb/usb-xhci.yaml
>> index 965f87fef702..540044a087a7 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-xhci.yaml
>> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.yaml
>> @@ -29,6 +29,10 @@ properties:
>>      description: Interrupt moderation interval
>>      default: 5000
>>  
>> +  xhci-snps-quirks:
>> +    description: Handles common Synopsys DWC_usb3x host quirks
>> +    type: boolean
>> +
>>  additionalProperties: true
>>  
>>  examples:
>> -- 
>> 2.28.0
>>
>>