Re: [PATCH v3 3/4] usb: dwc3: Add device property sgl-trb-cache-size-quirk

From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Tejas Joglekar <Tejas.Joglekar@synopsys.com>,
	Rob Herring <robh+dt@kernel.org>, Rob Herring <robh@kernel.org>
Cc: Felipe Balbi <balbi@kernel.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	John Youn <John.Youn@synopsys.com>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [PATCH v3 3/4] usb: dwc3: Add device property sgl-trb-cache-size-quirk
Date: Tue, 4 Aug 2020 00:44:41 +0000	[thread overview]
Message-ID: <d4f895da-38c7-d553-cb3a-835da00ae687@synopsys.com> (raw)
In-Reply-To: <7ae08b3d-3b5e-73cf-d196-325501a44e44@synopsys.com>

Hi Rob,

Tejas Joglekar wrote:
> Hello Rob,
> On 7/21/2020 10:27 PM, Tejas Joglekar wrote:
>> Hello,
>>
>> On 7/21/2020 3:17 PM, Felipe Balbi wrote:
>>> Tejas Joglekar <Tejas.Joglekar@synopsys.com> writes:
>>>
>>>> Hi Rob,
>>>>
>>>> On 7/6/2020 12:13 PM, Felipe Balbi wrote:
>>>>> Hi,
>>>>>
>>>>> Tejas Joglekar <Tejas.Joglekar@synopsys.com> writes:
>>>>>>> @@ -95,6 +95,10 @@ int dwc3_host_init(struct dwc3 *dwc)
>>>>>>>  	if (dwc->usb2_lpm_disable)
>>>>>>>  		props[prop_idx++] = PROPERTY_ENTRY_BOOL("usb2-lpm-disable");
>>>>>>>  
>>>>>>> +	if (dwc->sgl_trb_cache_size_quirk)
>>>>>>> +		props[prop_idx++] =
>>>>>>> +			PROPERTY_ENTRY_BOOL("sgl-trb-cache-size-quirk");
>>>>>>> +
>>>>>>>  	/**
>>>>>>>  	 * WORKAROUND: dwc3 revisions <=3.00a have a limitation
>>>>>>>  	 * where Port Disable command doesn't work.
>>>>>>>
>>>>>> Does this implementation looks good to you? Rob has some concerned over the DT entries,
>>>>>> you suggested using compatible string with this quirk addition.
>>>>>> Can you please brief about how you would like to have this quirk implemented?
>>>>>> I can send the updated patch. My patch series is pending for merge just because of the
>>>>>> DT and quirk issue. Can you please help?
>>>>> Yeah, you need to get into an agreement with Rob :-) I don't mind having
>>>>> extra DT flags for things which can't be detected in runtime, Rob
>>>>> disagrees.
>>>>>
>>>> The compatible string is not suitable option as it does not work with platform drivers
>>>> with PCI based system. Also Synopsys controllers IP version register is not visible to xhci
>>>> driver and hence we don't have separate compatible string for each Synopsys version on the
>>>> xhci driver side. 
>>>> Due to which I depend on DT flag addition for the quirk. Can we add these DT flags and quirk?
>>> As I said, I'm well aware of the situation regarding usage of compatible
>>> strings and the fact that dwc3 must work on PCI and non-PCI systems (I
>>> wrote the thing as it is after all). The person blocking new quirk flags
>>> is Rob, not me. You need to convince Rob that this is the way to go.
>>>
>> @Felipe: Sorry for confusion if any, previous mail was intended for Rob asking about his approval.
>>
>>> Rob, ball's in your court. Sorry.> 
>> @Rob: As I and Felipe have mentioned before, it is very much necessary to have quirk flags
>> for the current changes as compatible string would not be a solution for PCI and non-PCI
>> systems. Can you please approve this change ? If you have any concern about naming or any
>> other thing, please let us know.
>>
> Can you please comment?
>
> Thanks & Regards,
> Tejas Joglekar
>  
>

Can you help provide pointers and changes that Tejas can make to help
bring this issue to conclusion?

Thanks,
Thinh