[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b819a5fc-82aa-dc9c-4272-fecdacbd5880@intel.com>
Date: Fri, 20 Jul 2018 17:42:30 +0300
From: Mathias Nyman <mathias.nyman@...el.com>
To: Anurag Kumar Vulisha <anurag.kumar.vulisha@...inx.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: linux-usb@...r.kernel.org, linux-kernel@...r.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.
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@...inx.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
Powered by blists - more mailing lists