diff mbox

usb: host: xhci-plat: Iterate over parent nodes for finding quirks

Message ID 1528212056-28390-1-git-send-email-anurag.kumar.vulisha@xilinx.com (mailing list archive)
State New, archived
Headers show

Commit Message

Anurag Kumar Vulisha June 5, 2018, 3:20 p.m. UTC
In xhci_plat_probe() both sysdev and pdev->dev are being used
for finding quirks. There are some drivers(like dwc3 host.c)
which adds quirks(like usb3-lpm-capable) into pdev and the logic
present in xhci_plat_probe() checks for quirks in either sysdev
or pdev for finding the quirks. Because of this logic, some of
the quirks are getting missed(usb3-lpm-capable quirk added by dwc3
host.c driver is getting missed).This patch fixes this by iterating
over all the available parents for finding the quirks. In this way
all the quirks which are present in child or parent are correctly
updated.

Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
---
 drivers/usb/host/xhci-plat.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

Comments

Greg KH June 5, 2018, 3:25 p.m. UTC | #1
On Tue, Jun 05, 2018 at 08:50:56PM +0530, Anurag Kumar Vulisha wrote:
> In xhci_plat_probe() both sysdev and pdev->dev are being used
> for finding quirks. There are some drivers(like dwc3 host.c)
> which adds quirks(like usb3-lpm-capable) into pdev and the logic
> present in xhci_plat_probe() checks for quirks in either sysdev
> or pdev for finding the quirks. Because of this logic, some of
> the quirks are getting missed(usb3-lpm-capable quirk added by dwc3
> host.c driver is getting missed).This patch fixes this by iterating
> over all the available parents for finding the quirks. In this way
> all the quirks which are present in child or parent are correctly
> updated.
> 
> Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>

So this is a bugfix?  If so, how far back in the kernel releases should
it go to?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anurag Kumar Vulisha June 5, 2018, 6:41 p.m. UTC | #2
Hi Greg,

>-----Original Message-----
>From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
>Sent: Tuesday, June 05, 2018 8:55 PM
>To: Anurag Kumar Vulisha <anuragku@xilinx.com>
>Cc: Mathias Nyman <mathias.nyman@intel.com>; linux-usb@vger.kernel.org; linux-
>kernel@vger.kernel.org
>Subject: Re: [PATCH] usb: host: xhci-plat: Iterate over parent nodes for finding quirks
>
>On Tue, Jun 05, 2018 at 08:50:56PM +0530, Anurag Kumar Vulisha wrote:
>> In xhci_plat_probe() both sysdev and pdev->dev are being used for
>> finding quirks. There are some drivers(like dwc3 host.c) which adds
>> quirks(like usb3-lpm-capable) into pdev and the logic present in
>> xhci_plat_probe() checks for quirks in either sysdev or pdev for
>> finding the quirks. Because of this logic, some of the quirks are
>> getting missed(usb3-lpm-capable quirk added by dwc3 host.c driver is
>> getting missed).This patch fixes this by iterating over all the
>> available parents for finding the quirks. In this way all the quirks
>> which are present in child or parent are correctly updated.
>>
>> Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
>
>So this is a bugfix?  If so, how far back in the kernel releases should it go to?
>
I feel this is as a bugfix. This problem started with this patch
"usb: xhci: use bus->sysdev for DMA configuration (4c39d4b949d36faf)"
where pdev->dev is replaced with sysdev. If I am not wrong this bug is
present since 4.12 kernel release.

Thanks,
Anurag kumar Vulisha

>thanks,
>
>greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anurag Kumar Vulisha June 12, 2018, 7:03 a.m. UTC | #3
Hi Greg,

Please let me know if you have any comments on this patch. 

Thanks,
Anurag Kumar Vulisha

>-----Original Message-----
>From: Anurag Kumar Vulisha
>Sent: Wednesday, June 06, 2018 12:11 AM
>To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>Cc: Mathias Nyman <mathias.nyman@intel.com>; linux-usb@vger.kernel.org; linux-
>kernel@vger.kernel.org
>Subject: RE: [PATCH] usb: host: xhci-plat: Iterate over parent nodes for finding quirks
>
>Hi Greg,
>
>>-----Original Message-----
>>From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
>>Sent: Tuesday, June 05, 2018 8:55 PM
>>To: Anurag Kumar Vulisha <anuragku@xilinx.com>
>>Cc: Mathias Nyman <mathias.nyman@intel.com>; linux-usb@vger.kernel.org;
>>linux- kernel@vger.kernel.org
>>Subject: Re: [PATCH] usb: host: xhci-plat: Iterate over parent nodes
>>for finding quirks
>>
>>On Tue, Jun 05, 2018 at 08:50:56PM +0530, Anurag Kumar Vulisha wrote:
>>> In xhci_plat_probe() both sysdev and pdev->dev are being used for
>>> finding quirks. There are some drivers(like dwc3 host.c) which adds
>>> quirks(like usb3-lpm-capable) into pdev and the logic present in
>>> xhci_plat_probe() checks for quirks in either sysdev or pdev for
>>> finding the quirks. Because of this logic, some of the quirks are
>>> getting missed(usb3-lpm-capable quirk added by dwc3 host.c driver is
>>> getting missed).This patch fixes this by iterating over all the
>>> available parents for finding the quirks. In this way all the quirks
>>> which are present in child or parent are correctly updated.
>>>
>>> Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
>>
>>So this is a bugfix?  If so, how far back in the kernel releases should it go to?
>>
>I feel this is as a bugfix. This problem started with this patch
>"usb: xhci: use bus->sysdev for DMA configuration (4c39d4b949d36faf)"
>where pdev->dev is replaced with sysdev. If I am not wrong this bug is present since
>4.12 kernel release.
>
>Thanks,
>Anurag kumar Vulisha
>
>>thanks,
>>
>>greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH June 12, 2018, 8:02 a.m. UTC | #4
On Tue, Jun 12, 2018 at 07:03:39AM +0000, Anurag Kumar Vulisha wrote:
> Hi Greg,
> 
> Please let me know if you have any comments on this patch. 

It is the middle of the merge window, I can not do anything with any
patches until after 4.18-rc1 is out.

Please be patient.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anurag Kumar Vulisha July 14, 2018, 3:37 p.m. UTC | #5
HI Greg,

Ping!
Thought of giving a friendly remainder about this patch.

Thanks,
Anurag Kumar Vulisha

>-----Original Message-----
>From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
>Sent: Tuesday, June 12, 2018 1:33 PM
>To: Anurag Kumar Vulisha <anuragku@xilinx.com>
>Cc: Mathias Nyman <mathias.nyman@intel.com>; linux-usb@vger.kernel.org; linux-
>kernel@vger.kernel.org
>Subject: Re: [PATCH] usb: host: xhci-plat: Iterate over parent nodes for finding quirks
>
>On Tue, Jun 12, 2018 at 07:03:39AM +0000, Anurag Kumar Vulisha wrote:
>> Hi Greg,
>>
>> Please let me know if you have any comments on this patch.
>
>It is the middle of the merge window, I can not do anything with any patches until
>after 4.18-rc1 is out.
>
>Please be patient.
>
>thanks,
>
>greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH July 15, 2018, 8:02 a.m. UTC | #6
On Sat, Jul 14, 2018 at 03:37:20PM +0000, Anurag Kumar Vulisha wrote:
> HI Greg,
> 
> Ping!
> Thought of giving a friendly remainder about this patch.

I am not the xhci maintainer, I need to wait from an ack from him before
I can take it.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH July 15, 2018, 8:11 a.m. UTC | #7
On Sun, Jul 15, 2018 at 10:02:06AM +0200, Greg Kroah-Hartman wrote:
> On Sat, Jul 14, 2018 at 03:37:20PM +0000, Anurag Kumar Vulisha wrote:
> > HI Greg,
> > 
> > Ping!
> > Thought of giving a friendly remainder about this patch.
> 
> I am not the xhci maintainer, I need to wait from an ack from him before
> I can take it.

And he's on vacation until later this week, so just relax.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mathias Nyman July 20, 2018, 2:42 p.m. UTC | #8
Hi

Sorry about the delay with this patch,
I have a couple comments inline.

On 05.06.2018 18:20, Anurag Kumar Vulisha wrote:
> In xhci_plat_probe() both sysdev and pdev->dev are being used
> for finding quirks. There are some drivers(like dwc3 host.c)
> which adds quirks(like usb3-lpm-capable) into pdev and the logic
> present in xhci_plat_probe() checks for quirks in either sysdev
> or pdev for finding the quirks. Because of this logic, some of
> the quirks are getting missed(usb3-lpm-capable quirk added by dwc3
> host.c driver is getting missed).This patch fixes this by iterating
> over all the available parents for finding the quirks. In this way
> all the quirks which are present in child or parent are correctly
> updated.
> 
> Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
> ---
>   drivers/usb/host/xhci-plat.c | 29 ++++++++++++++++++-----------
>   1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index c1b22fc..0cd0489 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -152,7 +152,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>   {
>   	const struct xhci_plat_priv *priv_match;
>   	const struct hc_driver	*driver;
> -	struct device		*sysdev;
> +	struct device		*sysdev, *tmpdev;
>   	struct xhci_hcd		*xhci;
>   	struct resource         *res;
>   	struct usb_hcd		*hcd;
> @@ -272,19 +272,26 @@ static int xhci_plat_probe(struct platform_device *pdev)
>   		goto disable_clk;
>   	}
>   
> -	if (device_property_read_bool(sysdev, "usb2-lpm-disable"))
> -		xhci->quirks |= XHCI_HW_LPM_DISABLE;
> +	/* Iterate over all parent nodes for finding quirks */
> +	for (tmpdev = &pdev->dev; tmpdev; tmpdev = tmpdev->parent) {

Isn't sysdev at this point the topmost device that can have any of those
device properties set?
We could loop from &pdev->dev up to and including sysdev.

It doesn't matter much but maybe avoid walking some extra parents.
   
>   
> -	if (device_property_read_bool(sysdev, "usb3-lpm-capable"))
> -		xhci->quirks |= XHCI_LPM_SUPPORT;
> +		if (device_property_read_bool(tmpdev, "usb2-lpm-disable"))
> +			xhci->quirks |= XHCI_HW_LPM_DISABLE;
>   
> -	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
> -		xhci->quirks |= XHCI_BROKEN_PORT_PED;
> +		if (device_property_read_bool(tmpdev, "usb3-lpm-capable"))
> +			xhci->quirks |= XHCI_LPM_SUPPORT;
>   
> -	/* imod_interval is the interrupt moderation value in nanoseconds. */
> -	xhci->imod_interval = 40000;

Setting the default imod_interval could be moved before the for() loop

> -	device_property_read_u32(sysdev, "imod-interval-ns",
> -				 &xhci->imod_interval);
> +		if (device_property_read_bool(tmpdev, "quirk-broken-port-ped"))
> +			xhci->quirks |= XHCI_BROKEN_PORT_PED;
> +
> +		/*
> +		 * imod_interval is the interrupt moderation
> +		 * value in nanoseconds.
> +		 */
> +		xhci->imod_interval = 40000;
> +		device_property_read_u32(tmpdev, "imod-interval-ns",
> +					 &xhci->imod_interval);
> +	}
>   
>   	hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);
>   	if (IS_ERR(hcd->usb_phy)) {
> 

Otherwise everything looks fine.
I will unfortunately be away again for another two weeks.

-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mathias Nyman July 20, 2018, 2:53 p.m. UTC | #9
On 15.07.2018 11:11, Greg Kroah-Hartman wrote:
> On Sun, Jul 15, 2018 at 10:02:06AM +0200, Greg Kroah-Hartman wrote:
>> On Sat, Jul 14, 2018 at 03:37:20PM +0000, Anurag Kumar Vulisha wrote:
>>> HI Greg,
>>>
>>> Ping!
>>> Thought of giving a friendly remainder about this patch.
>>
>> I am not the xhci maintainer, I need to wait from an ack from him before
>> I can take it.
> 
> And he's on vacation until later this week, so just relax.
> 

I added a couple minor comments.
I will be away for two more weeks, and I don't want to block this,
so if it is important to get this to 4.18 I won't object picking it as is.

-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anurag Kumar Vulisha July 20, 2018, 3:39 p.m. UTC | #10
Hi Mathias,

Thanks for providing your comments, please find my comments inline

>-----Original Message-----

>From: Mathias Nyman [mailto:mathias.nyman@intel.com]

>Sent: Friday, July 20, 2018 8:13 PM

>To: Anurag Kumar Vulisha <anuragku@xilinx.com>; Greg Kroah-Hartman

><gregkh@linuxfoundation.org>

>Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org

>Subject: Re: [PATCH] usb: host: xhci-plat: Iterate over parent nodes for finding quirks

>

>Hi

>

>Sorry about the delay with this patch,

>I have a couple comments inline.

>


No probs, I  can understand. I was having some internal dependency on this patch,
so pinged again. Sorry for troubling you.

>On 05.06.2018 18:20, Anurag Kumar Vulisha wrote:

>> In xhci_plat_probe() both sysdev and pdev->dev are being used for

>> finding quirks. There are some drivers(like dwc3 host.c) which adds

>> quirks(like usb3-lpm-capable) into pdev and the logic present in

>> xhci_plat_probe() checks for quirks in either sysdev or pdev for

>> finding the quirks. Because of this logic, some of the quirks are

>> getting missed(usb3-lpm-capable quirk added by dwc3 host.c driver is

>> getting missed).This patch fixes this by iterating over all the

>> available parents for finding the quirks. In this way all the quirks

>> which are present in child or parent are correctly updated.

>>

>> Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>

>> ---

>>   drivers/usb/host/xhci-plat.c | 29 ++++++++++++++++++-----------

>>   1 file changed, 18 insertions(+), 11 deletions(-)

>>

>> diff --git a/drivers/usb/host/xhci-plat.c

>> b/drivers/usb/host/xhci-plat.c index c1b22fc..0cd0489 100644

>> --- a/drivers/usb/host/xhci-plat.c

>> +++ b/drivers/usb/host/xhci-plat.c

>> @@ -152,7 +152,7 @@ static int xhci_plat_probe(struct platform_device *pdev)

>>   {

>>   	const struct xhci_plat_priv *priv_match;

>>   	const struct hc_driver	*driver;

>> -	struct device		*sysdev;

>> +	struct device		*sysdev, *tmpdev;

>>   	struct xhci_hcd		*xhci;

>>   	struct resource         *res;

>>   	struct usb_hcd		*hcd;

>> @@ -272,19 +272,26 @@ static int xhci_plat_probe(struct platform_device *pdev)

>>   		goto disable_clk;

>>   	}

>>

>> -	if (device_property_read_bool(sysdev, "usb2-lpm-disable"))

>> -		xhci->quirks |= XHCI_HW_LPM_DISABLE;

>> +	/* Iterate over all parent nodes for finding quirks */

>> +	for (tmpdev = &pdev->dev; tmpdev; tmpdev = tmpdev->parent) {

>

>Isn't sysdev at this point the topmost device that can have any of those device

>properties set?

>We could loop from &pdev->dev up to and including sysdev.

>

>It doesn't matter much but maybe avoid walking some extra parents.

>


I have seen some drivers ( like dwc3 host.c) which create a child dev, which is
the pdev that is being passed to xhci_plat_probe(). So, sysdev may not be the
topmost parent. There could be some properties which may be present in parent
and may not be populated in the child. So, I made this change to search on all
available parents for finding a valid property

>>

>> -	if (device_property_read_bool(sysdev, "usb3-lpm-capable"))

>> -		xhci->quirks |= XHCI_LPM_SUPPORT;

>> +		if (device_property_read_bool(tmpdev, "usb2-lpm-disable"))

>> +			xhci->quirks |= XHCI_HW_LPM_DISABLE;

>>

>> -	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))

>> -		xhci->quirks |= XHCI_BROKEN_PORT_PED;

>> +		if (device_property_read_bool(tmpdev, "usb3-lpm-capable"))

>> +			xhci->quirks |= XHCI_LPM_SUPPORT;

>>

>> -	/* imod_interval is the interrupt moderation value in nanoseconds. */

>> -	xhci->imod_interval = 40000;

>

>Setting the default imod_interval could be moved before the for() loop

>


I thought it would be better if everything related to imod_interval is in one place, 
so kept it as is. I can fix this in v2 if you suggest.

>> -	device_property_read_u32(sysdev, "imod-interval-ns",

>> -				 &xhci->imod_interval);

>> +		if (device_property_read_bool(tmpdev, "quirk-broken-port-ped"))

>> +			xhci->quirks |= XHCI_BROKEN_PORT_PED;

>> +

>> +		/*

>> +		 * imod_interval is the interrupt moderation

>> +		 * value in nanoseconds.

>> +		 */

>> +		xhci->imod_interval = 40000;

>> +		device_property_read_u32(tmpdev, "imod-interval-ns",

>> +					 &xhci->imod_interval);

>> +	}

>>

>>   	hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);

>>   	if (IS_ERR(hcd->usb_phy)) {

>>

>

>Otherwise everything looks fine.

>I will unfortunately be away again for another two weeks.


No probs. If you feel there is no harm in applying and satisfied with the changes, it
would be helpful if you can apply this patch. This is because I have some dependency
on this patch waiting  to be applied. Sorry for troubling you again.

Thanks,
Anurag Kumar Vulisha

>

>-Mathias
Anurag Kumar Vulisha Aug. 6, 2018, 7:58 a.m. UTC | #11
Hi Greg,

>-----Original Message-----
>From: Mathias Nyman [mailto:mathias.nyman@intel.com]
>Sent: Friday, July 20, 2018 8:24 PM
>To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Anurag Kumar Vulisha
><anuragku@xilinx.com>
>Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org;
>v.anuragkumar@gmail.com
>Subject: Re: [PATCH] usb: host: xhci-plat: Iterate over parent nodes for finding quirks
>
>On 15.07.2018 11:11, Greg Kroah-Hartman wrote:
>> On Sun, Jul 15, 2018 at 10:02:06AM +0200, Greg Kroah-Hartman wrote:
>>> On Sat, Jul 14, 2018 at 03:37:20PM +0000, Anurag Kumar Vulisha wrote:
>>>> HI Greg,
>>>>
>>>> Ping!
>>>> Thought of giving a friendly remainder about this patch.
>>>
>>> I am not the xhci maintainer, I need to wait from an ack from him
>>> before I can take it.
>>
>> And he's on vacation until later this week, so just relax.
>>
>
>I added a couple minor comments.
>I will be away for two more weeks, and I don't want to block this, so if it is important
>to get this to 4.18 I won't object picking it as is.
>

I have replied to the comments that Mathias has previously given on this patch and waiting
on his reply, but looks like Mathias is on vacation. Since he is okay with applying the patch,
would it be possible to get this patch applied? If you have any suggestions I can work on
fixing them and re-send the patch. Thanks for your help in this regard.

Regards,
Anurag Kumar Vulisha
 
>-Mathias
Mathias Nyman Aug. 6, 2018, 10:50 a.m. UTC | #12
Hi,

Back from vacation

On 20.07.2018 18:39, Anurag Kumar Vulisha wrote:
> 
> Hi Mathias,
> 
> Thanks for providing your comments, please find my comments inline
> 
>>>
>>> -	if (device_property_read_bool(sysdev, "usb2-lpm-disable"))
>>> -		xhci->quirks |= XHCI_HW_LPM_DISABLE;
>>> +	/* Iterate over all parent nodes for finding quirks */
>>> +	for (tmpdev = &pdev->dev; tmpdev; tmpdev = tmpdev->parent) {
>>
>> Isn't sysdev at this point the topmost device that can have any of those device
>> properties set?
>> We could loop from &pdev->dev up to and including sysdev.
>>
>> It doesn't matter much but maybe avoid walking some extra parents.
>>> 
> I have seen some drivers ( like dwc3 host.c) which create a child dev, which is
> the pdev that is being passed to xhci_plat_probe(). So, sysdev may not be the
> topmost parent. There could be some properties which may be present in parent
> and may not be populated in the child. So, I made this change to search on all
> available parents for finding a valid property

Ok, lets keep it like you wrote it

> 
>>>
>>> -	if (device_property_read_bool(sysdev, "usb3-lpm-capable"))
>>> -		xhci->quirks |= XHCI_LPM_SUPPORT;
>>> +		if (device_property_read_bool(tmpdev, "usb2-lpm-disable"))
>>> +			xhci->quirks |= XHCI_HW_LPM_DISABLE;
>>>
>>> -	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
>>> -		xhci->quirks |= XHCI_BROKEN_PORT_PED;
>>> +		if (device_property_read_bool(tmpdev, "usb3-lpm-capable"))
>>> +			xhci->quirks |= XHCI_LPM_SUPPORT;
>>>
>>> -	/* imod_interval is the interrupt moderation value in nanoseconds. */
>>> -	xhci->imod_interval = 40000;
>>
>> Setting the default imod_interval could be moved before the for() loop
>>
> 
> I thought it would be better if everything related to imod_interval is in one place,
> so kept it as is. I can fix this in v2 if you suggest.

Default imod_interval needs to be set before the for loop to avoid overwriting a value
got from device property

So a v2 is needed

-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mathias Nyman Aug. 6, 2018, 11:11 a.m. UTC | #13
On 06.08.2018 10:58, Anurag Kumar Vulisha wrote:
> Hi Greg,
> 
>> -----Original Message-----
>> From: Mathias Nyman [mailto:mathias.nyman@intel.com]
>> Sent: Friday, July 20, 2018 8:24 PM
>> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Anurag Kumar Vulisha
>> <anuragku@xilinx.com>
>> Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org;
>> v.anuragkumar@gmail.com
>> Subject: Re: [PATCH] usb: host: xhci-plat: Iterate over parent nodes for finding quirks
>>
>> On 15.07.2018 11:11, Greg Kroah-Hartman wrote:
>>> On Sun, Jul 15, 2018 at 10:02:06AM +0200, Greg Kroah-Hartman wrote:
>>>> On Sat, Jul 14, 2018 at 03:37:20PM +0000, Anurag Kumar Vulisha wrote:
>>>>> HI Greg,
>>>>>
>>>>> Ping!
>>>>> Thought of giving a friendly remainder about this patch.
>>>>
>>>> I am not the xhci maintainer, I need to wait from an ack from him
>>>> before I can take it.
>>>
>>> And he's on vacation until later this week, so just relax.
>>>
>>
>> I added a couple minor comments.
>> I will be away for two more weeks, and I don't want to block this, so if it is important
>> to get this to 4.18 I won't object picking it as is.
>>
> 
> I have replied to the comments that Mathias has previously given on this patch and waiting
> on his reply, but looks like Mathias is on vacation. Since he is okay with applying the patch,
> would it be possible to get this patch applied? If you have any suggestions I can work on
> fixing them and re-send the patch. Thanks for your help in this regard.
> 

I'm back.
Just noted that there is one change still needed, (commented on actual patch)
I'll pick up the next version of the patch, but we are in rc8 already so your fix will go
to 4.19-rc2 earliest.

-Mathias


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anurag Kumar Vulisha Aug. 6, 2018, 11:14 a.m. UTC | #14
Hi Mathais,
>>
>>>>
>>>> -	if (device_property_read_bool(sysdev, "usb3-lpm-capable"))
>>>> -		xhci->quirks |= XHCI_LPM_SUPPORT;
>>>> +		if (device_property_read_bool(tmpdev, "usb2-lpm-disable"))
>>>> +			xhci->quirks |= XHCI_HW_LPM_DISABLE;
>>>>
>>>> -	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
>>>> -		xhci->quirks |= XHCI_BROKEN_PORT_PED;
>>>> +		if (device_property_read_bool(tmpdev, "usb3-lpm-capable"))
>>>> +			xhci->quirks |= XHCI_LPM_SUPPORT;
>>>>
>>>> -	/* imod_interval is the interrupt moderation value in nanoseconds. */
>>>> -	xhci->imod_interval = 40000;
>>>
>>> Setting the default imod_interval could be moved before the for()
>>> loop
>>>
>>
>> I thought it would be better if everything related to imod_interval is
>> in one place, so kept it as is. I can fix this in v2 if you suggest.
>
>Default imod_interval needs to be set before the for loop to avoid overwriting a value
>got from device property
>
>So a v2 is needed
>

Thanks for correcting, will fix and send v2

Thanks,
Anurag Kumar Vulisha

>-Mathias
Anurag Kumar Vulisha Aug. 6, 2018, 11:41 a.m. UTC | #15
>>
>> I have replied to the comments that Mathias has previously given on
>> this patch and waiting on his reply, but looks like Mathias is on
>> vacation. Since he is okay with applying the patch, would it be
>> possible to get this patch applied? If you have any suggestions I can work on fixing
>them and re-send the patch. Thanks for your help in this regard.
>>
>
>I'm back.
>Just noted that there is one change still needed, (commented on actual patch) I'll pick
>up the next version of the patch, but we are in rc8 already so your fix will go to 4.19-
>rc2 earliest.
>
Hi Mathias,

4.19-rc2 is fine. I have corrected the patch and sent a v2. Thanks a lot for
your help,

Regards,
Anurag Kumar Vulisha
diff mbox

Patch

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index c1b22fc..0cd0489 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -152,7 +152,7 @@  static int xhci_plat_probe(struct platform_device *pdev)
 {
 	const struct xhci_plat_priv *priv_match;
 	const struct hc_driver	*driver;
-	struct device		*sysdev;
+	struct device		*sysdev, *tmpdev;
 	struct xhci_hcd		*xhci;
 	struct resource         *res;
 	struct usb_hcd		*hcd;
@@ -272,19 +272,26 @@  static int xhci_plat_probe(struct platform_device *pdev)
 		goto disable_clk;
 	}
 
-	if (device_property_read_bool(sysdev, "usb2-lpm-disable"))
-		xhci->quirks |= XHCI_HW_LPM_DISABLE;
+	/* Iterate over all parent nodes for finding quirks */
+	for (tmpdev = &pdev->dev; tmpdev; tmpdev = tmpdev->parent) {
 
-	if (device_property_read_bool(sysdev, "usb3-lpm-capable"))
-		xhci->quirks |= XHCI_LPM_SUPPORT;
+		if (device_property_read_bool(tmpdev, "usb2-lpm-disable"))
+			xhci->quirks |= XHCI_HW_LPM_DISABLE;
 
-	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
-		xhci->quirks |= XHCI_BROKEN_PORT_PED;
+		if (device_property_read_bool(tmpdev, "usb3-lpm-capable"))
+			xhci->quirks |= XHCI_LPM_SUPPORT;
 
-	/* imod_interval is the interrupt moderation value in nanoseconds. */
-	xhci->imod_interval = 40000;
-	device_property_read_u32(sysdev, "imod-interval-ns",
-				 &xhci->imod_interval);
+		if (device_property_read_bool(tmpdev, "quirk-broken-port-ped"))
+			xhci->quirks |= XHCI_BROKEN_PORT_PED;
+
+		/*
+		 * imod_interval is the interrupt moderation
+		 * value in nanoseconds.
+		 */
+		xhci->imod_interval = 40000;
+		device_property_read_u32(tmpdev, "imod-interval-ns",
+					 &xhci->imod_interval);
+	}
 
 	hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);
 	if (IS_ERR(hcd->usb_phy)) {