diff mbox series

[v2,5/5] usb: host: xhci-plat: Add support for XHCI_WRITE_64_HI_LO_QUIRK

Message ID 1717135657-120818-6-git-send-email-dh10.jung@samsung.com (mailing list archive)
State Superseded
Headers show
Series usb: Add quirk for writing high-low order | expand

Commit Message

Jung Daehwan May 31, 2024, 6:07 a.m. UTC
This is set by dwc3 parent node to support writing high-low order.

Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
---
 drivers/usb/host/xhci-plat.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Krzysztof Kozlowski May 31, 2024, 8:12 a.m. UTC | #1
On 31/05/2024 08:07, Daehwan Jung wrote:
> This is set by dwc3 parent node to support writing high-low order.
> 
> Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
> ---
>  drivers/usb/host/xhci-plat.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 3d071b8..31bdfa5 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -256,6 +256,9 @@ int xhci_plat_probe(struct platform_device *pdev, struct device *sysdev, const s
>  		if (device_property_read_bool(tmpdev, "xhci-sg-trb-cache-size-quirk"))
>  			xhci->quirks |= XHCI_SG_TRB_CACHE_SIZE_QUIRK;
>  
> +		if (device_property_read_bool(tmpdev, "write-64-hi-lo-quirk"))
> +			xhci->quirks |= XHCI_WRITE_64_HI_LO;

Where is any user of this property (DTS)? Just to clarify: your
downstream does not matter really.

Best regards,
Krzysztof
Jung Daehwan June 3, 2024, 3:44 a.m. UTC | #2
On Fri, May 31, 2024 at 10:12:36AM +0200, Krzysztof Kozlowski wrote:
> On 31/05/2024 08:07, Daehwan Jung wrote:
> > This is set by dwc3 parent node to support writing high-low order.
> > 
> > Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
> > ---
> >  drivers/usb/host/xhci-plat.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> > index 3d071b8..31bdfa5 100644
> > --- a/drivers/usb/host/xhci-plat.c
> > +++ b/drivers/usb/host/xhci-plat.c
> > @@ -256,6 +256,9 @@ int xhci_plat_probe(struct platform_device *pdev, struct device *sysdev, const s
> >  		if (device_property_read_bool(tmpdev, "xhci-sg-trb-cache-size-quirk"))
> >  			xhci->quirks |= XHCI_SG_TRB_CACHE_SIZE_QUIRK;
> >  
> > +		if (device_property_read_bool(tmpdev, "write-64-hi-lo-quirk"))
> > +			xhci->quirks |= XHCI_WRITE_64_HI_LO;
> 
> Where is any user of this property (DTS)? Just to clarify: your
> downstream does not matter really.
> 

This is set by dwc3 parent node by software node.

[PATCH v2 1/5] dt-bindings: usb: snps,dwc3: Add 'snps,xhci-write-64-hi-lo-quirk' quirk
https://lore.kernel.org/r/1717135657-120818-2-git-send-email-dh10.jung@samsung.com/

Best Regards,
Jung Daehwan

> Best regards,
> Krzysztof
> 
>
Krzysztof Kozlowski June 3, 2024, 6:56 a.m. UTC | #3
On 03/06/2024 05:44, Jung Daehwan wrote:
> On Fri, May 31, 2024 at 10:12:36AM +0200, Krzysztof Kozlowski wrote:
>> On 31/05/2024 08:07, Daehwan Jung wrote:
>>> This is set by dwc3 parent node to support writing high-low order.
>>>
>>> Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
>>> ---
>>>  drivers/usb/host/xhci-plat.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>>> index 3d071b8..31bdfa5 100644
>>> --- a/drivers/usb/host/xhci-plat.c
>>> +++ b/drivers/usb/host/xhci-plat.c
>>> @@ -256,6 +256,9 @@ int xhci_plat_probe(struct platform_device *pdev, struct device *sysdev, const s
>>>  		if (device_property_read_bool(tmpdev, "xhci-sg-trb-cache-size-quirk"))
>>>  			xhci->quirks |= XHCI_SG_TRB_CACHE_SIZE_QUIRK;
>>>  
>>> +		if (device_property_read_bool(tmpdev, "write-64-hi-lo-quirk"))
>>> +			xhci->quirks |= XHCI_WRITE_64_HI_LO;
>>
>> Where is any user of this property (DTS)? Just to clarify: your
>> downstream does not matter really.
>>
> 
> This is set by dwc3 parent node by software node.
> 
> [PATCH v2 1/5] dt-bindings: usb: snps,dwc3: Add 'snps,xhci-write-64-hi-lo-quirk' quirk
> https://lore.kernel.org/r/1717135657-120818-2-git-send-email-dh10.jung@samsung.com/

This is not a patch to DTS.


Best regards,
Krzysztof
Jung Daehwan June 3, 2024, 8:51 a.m. UTC | #4
On Mon, Jun 03, 2024 at 08:56:09AM +0200, Krzysztof Kozlowski wrote:
> On 03/06/2024 05:44, Jung Daehwan wrote:
> > On Fri, May 31, 2024 at 10:12:36AM +0200, Krzysztof Kozlowski wrote:
> >> On 31/05/2024 08:07, Daehwan Jung wrote:
> >>> This is set by dwc3 parent node to support writing high-low order.
> >>>
> >>> Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
> >>> ---
> >>>  drivers/usb/host/xhci-plat.c | 3 +++
> >>>  1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> >>> index 3d071b8..31bdfa5 100644
> >>> --- a/drivers/usb/host/xhci-plat.c
> >>> +++ b/drivers/usb/host/xhci-plat.c
> >>> @@ -256,6 +256,9 @@ int xhci_plat_probe(struct platform_device *pdev, struct device *sysdev, const s
> >>>  		if (device_property_read_bool(tmpdev, "xhci-sg-trb-cache-size-quirk"))
> >>>  			xhci->quirks |= XHCI_SG_TRB_CACHE_SIZE_QUIRK;
> >>>  
> >>> +		if (device_property_read_bool(tmpdev, "write-64-hi-lo-quirk"))
> >>> +			xhci->quirks |= XHCI_WRITE_64_HI_LO;
> >>
> >> Where is any user of this property (DTS)? Just to clarify: your
> >> downstream does not matter really.
> >>
> > 
> > This is set by dwc3 parent node by software node.
> > 
> > [PATCH v2 1/5] dt-bindings: usb: snps,dwc3: Add 'snps,xhci-write-64-hi-lo-quirk' quirk
> > https://lore.kernel.org/r/1717135657-120818-2-git-send-email-dh10.jung@samsung.com/
> 
> This is not a patch to DTS.
> 
> 

This is set by software node from dwc3. That's why I think this patch doesn't
need DTS patch. I would add DTS patch in dwc3 not xhci if it's needed.

Best Regards,
Jung Daehwan

> Best regards,
> Krzysztof
> 
>
Krzysztof Kozlowski June 4, 2024, 6:20 a.m. UTC | #5
On 03/06/2024 10:51, Jung Daehwan wrote:
>>>>>  
>>>>> +		if (device_property_read_bool(tmpdev, "write-64-hi-lo-quirk"))
>>>>> +			xhci->quirks |= XHCI_WRITE_64_HI_LO;
>>>>
>>>> Where is any user of this property (DTS)? Just to clarify: your
>>>> downstream does not matter really.
>>>>
>>>
>>> This is set by dwc3 parent node by software node.
>>>
>>> [PATCH v2 1/5] dt-bindings: usb: snps,dwc3: Add 'snps,xhci-write-64-hi-lo-quirk' quirk
>>> https://lore.kernel.org/r/1717135657-120818-2-git-send-email-dh10.jung@samsung.com/
>>
>> This is not a patch to DTS.
>>
>>
> 
> This is set by software node from dwc3. That's why I think this patch doesn't
> need DTS patch. I would add DTS patch in dwc3 not xhci if it's needed.
> 

What?

I asked you question which upstream SoC (link to DTS) uses it, and you
say that "no need for DTS patch"? That's not an answer.

Best regards,
Krzysztof
Jung Daehwan June 5, 2024, 1:50 a.m. UTC | #6
On Tue, Jun 04, 2024 at 08:20:44AM +0200, Krzysztof Kozlowski wrote:
> On 03/06/2024 10:51, Jung Daehwan wrote:
> >>>>>  
> >>>>> +		if (device_property_read_bool(tmpdev, "write-64-hi-lo-quirk"))
> >>>>> +			xhci->quirks |= XHCI_WRITE_64_HI_LO;
> >>>>
> >>>> Where is any user of this property (DTS)? Just to clarify: your
> >>>> downstream does not matter really.
> >>>>
> >>>
> >>> This is set by dwc3 parent node by software node.
> >>>
> >>> [PATCH v2 1/5] dt-bindings: usb: snps,dwc3: Add 'snps,xhci-write-64-hi-lo-quirk' quirk
> >>> https://lore.kernel.org/r/1717135657-120818-2-git-send-email-dh10.jung@samsung.com/
> >>
> >> This is not a patch to DTS.
> >>
> >>
> > 
> > This is set by software node from dwc3. That's why I think this patch doesn't
> > need DTS patch. I would add DTS patch in dwc3 not xhci if it's needed.
> > 
> 
> What?
> 
> I asked you question which upstream SoC (link to DTS) uses it, and you
> say that "no need for DTS patch"? That's not an answer.
> 
> Best regards,
> Krzysztof
> 
> 

I'm sorry I didn't get your point. I've been working on Exynos SoC.
But there's no upstream user of this property yet in this patchset.

Best Regards,
Jung Daehwan
Krzysztof Kozlowski June 5, 2024, 6:44 a.m. UTC | #7
On 05/06/2024 03:50, Jung Daehwan wrote:
> On Tue, Jun 04, 2024 at 08:20:44AM +0200, Krzysztof Kozlowski wrote:
>> On 03/06/2024 10:51, Jung Daehwan wrote:
>>>>>>>  
>>>>>>> +		if (device_property_read_bool(tmpdev, "write-64-hi-lo-quirk"))
>>>>>>> +			xhci->quirks |= XHCI_WRITE_64_HI_LO;
>>>>>>
>>>>>> Where is any user of this property (DTS)? Just to clarify: your
>>>>>> downstream does not matter really.
>>>>>>
>>>>>
>>>>> This is set by dwc3 parent node by software node.
>>>>>
>>>>> [PATCH v2 1/5] dt-bindings: usb: snps,dwc3: Add 'snps,xhci-write-64-hi-lo-quirk' quirk
>>>>> https://lore.kernel.org/r/1717135657-120818-2-git-send-email-dh10.jung@samsung.com/
>>>>
>>>> This is not a patch to DTS.
>>>>
>>>>
>>>
>>> This is set by software node from dwc3. That's why I think this patch doesn't
>>> need DTS patch. I would add DTS patch in dwc3 not xhci if it's needed.
>>>
>>
>> What?
>>
>> I asked you question which upstream SoC (link to DTS) uses it, and you
>> say that "no need for DTS patch"? That's not an answer.
>>
>> Best regards,
>> Krzysztof
>>
>>
> 
> I'm sorry I didn't get your point. I've been working on Exynos SoC.
> But there's no upstream user of this property yet in this patchset.

That's the point... it makes quite tricky to evaluate whether the
binding is reasonable or not. Upstream your DTS. Otherwise I say this
can be deduced from existing compatible and property is not needed.

Sorry, that's a no.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 3d071b8..31bdfa5 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -256,6 +256,9 @@  int xhci_plat_probe(struct platform_device *pdev, struct device *sysdev, const s
 		if (device_property_read_bool(tmpdev, "xhci-sg-trb-cache-size-quirk"))
 			xhci->quirks |= XHCI_SG_TRB_CACHE_SIZE_QUIRK;
 
+		if (device_property_read_bool(tmpdev, "write-64-hi-lo-quirk"))
+			xhci->quirks |= XHCI_WRITE_64_HI_LO;
+
 		device_property_read_u32(tmpdev, "imod-interval-ns",
 					 &xhci->imod_interval);
 	}