usb: host: xhci: some changes for xhci-plat.c
Subsystems (2)
v2 cover usb: host: xhci: some changes for xhci-plat.c
Hi Mathias,
This series mainly includes some changes for xhci-plat.c. Some of the patches were reviewed by you, some were sent during your holiday. To easy your review and save, I re-send them together. The patches are already rebased with the lastest usb/next, thanks.
Changes for v2:
- Do not set wakeup defaulted enabled since the device's wakeup enable will not turn off its power dowain. [Patch 5/7]
https://www.spinics.net/lists/linux-usb/msg199303.html
- Change the comments for this behavior accordingly [Patch 6/7]
Peter
Peter Chen (7):
usb: host: xhci-plat: add platform data support
usb: host: xhci-plat: add .suspend_quirk for struct xhci_plat_priv
usb: host: xhci-plat: delete the unnecessary code
usb: host: xhci-plat: add priv quirk for skip PHY initialization
usb: host: xhci-plat: add wakeup entry at sysfs
usb: host: xhci-plat: improve the comments for xhci_plat_suspend
usb: host: xhci: avoid calling contineous two times for xhci_suspend
drivers/usb/host/xhci-plat.c | 44 +++++++++++++++++++++++++++--------- drivers/usb/host/xhci-plat.h | 1 + drivers/usb/host/xhci.c | 7 ++++-- drivers/usb/host/xhci.h | 1 + 4 files changed, 40 insertions(+), 13 deletions(-)
Signature · 1 line
2.17.1
PATCH v2 1/7 usb: host: xhci-plat: add platform data support
Some xhci hosts (eg dwc3 and cdns3) do not use OF to create platform device, they create xhci-plat platform device runtime. And these platforms may also have quirks, and the quirks could be supplied by their parent device through platform data.
--- drivers/usb/host/xhci-plat.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/usb/host/xhci-plat.c +5 -1
@@ -264,7 +264,11 @@ static int xhci_plat_probe(struct platform_device *pdev)if (ret) goto disable_reg_clk; - priv_match =of_device_get_match_data (&pdev->dev); +if (pdev->dev.of_node) + priv_match =of_device_get_match_data (&pdev->dev); +else + priv_match =dev_get_platdata (&pdev->dev); +if (priv_match) {struct xhci_plat_priv *priv =hcd_to_xhci_priv (hcd);
Signature · 1 line
2.17.1
PATCH v2 2/7 usb: host: xhci-plat: add .suspend_quirk for struct xhci_plat_priv
In reply to Peter ChenSome platforms (eg cdns3) may have special sequences between xhci_bus_suspend and xhci_suspend, add .suspend_quick for it.
--- drivers/usb/host/xhci-plat.c | 19 +++++++++++++++++++ drivers/usb/host/xhci-plat.h | 1 + 2 files changed, 20 insertions(+)
drivers/usb/host/xhci-plat.c +19 -0
@@ -54,6 +54,16 @@ static int xhci_priv_init_quirk(struct usb_hcd *hcd)return priv->init_quirk (hcd); } +static int xhci_priv_suspend_quirk (struct usb_hcd *hcd) +{ +struct xhci_plat_priv *priv =hcd_to_xhci_priv (hcd); + +if (!priv->suspend_quirk) +return 0 ; + +return priv->suspend_quirk (hcd); +} +static int xhci_priv_resume_quirk (struct usb_hcd *hcd) {struct xhci_plat_priv *priv =hcd_to_xhci_priv (hcd);
@@ -401,7 +411,11 @@ static int __maybe_unused xhci_plat_suspend(struct device *dev) {struct usb_hcd *hcd =dev_get_drvdata (dev);struct xhci_hcd *xhci =hcd_to_xhci (hcd); +int ret; + ret =xhci_priv_suspend_quirk (hcd); +if (ret) +return ret;/* * xhci_suspend() needs `do_wakeup` to know whether host is allowed * to do wakeup during suspend. Since xhci_plat_suspend is currently
@@ -438,6 +452,11 @@ static int __maybe_unused xhci_plat_runtime_suspend(struct device *dev) {struct usb_hcd *hcd =dev_get_drvdata (dev);struct xhci_hcd *xhci =hcd_to_xhci (hcd); +int ret; + + ret =xhci_priv_suspend_quirk (hcd); +if (ret) +return ret;return xhci_suspend (xhci, true); }
drivers/usb/host/xhci-plat.h +1 -0
@@ -15,6 +15,7 @@ struct xhci_plat_priv {unsigned long long quirks;void (*plat_start)(struct usb_hcd *);int (*init_quirk)(struct usb_hcd *); +int (*suspend_quirk)(struct usb_hcd *);int (*resume_quirk)(struct usb_hcd *); };
Signature · 1 line
2.17.1
PATCH v2 3/7 usb: host: xhci-plat: delete the unnecessary code
In reply to Peter ChenThe if {} condition is duplicated with outer if {} condition.
--- drivers/usb/host/xhci-plat.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/usb/host/xhci-plat.c +1 -2
@@ -283,8 +283,7 @@ static int xhci_plat_probe(struct platform_device *pdev)struct xhci_plat_priv *priv =hcd_to_xhci_priv (hcd);/* Just copy data for now */ -if (priv_match) - *priv = *priv_match; + *priv = *priv_match; }device_wakeup_enable (hcd->self.controller);
Signature · 1 line
2.17.1
PATCH v2 4/7 usb: host: xhci-plat: add priv quirk for skip PHY initialization
In reply to Peter ChenSome DRD controllers (eg, dwc3 & cdns3) have PHY management at their own driver to cover both device and host mode, so add one priv quirk for such users to skip PHY management from HCD core.
--- drivers/usb/host/xhci-plat.c | 8 ++++++-- drivers/usb/host/xhci.h | 1 + 2 files changed, 7 insertions(+), 2 deletions(-)
drivers/usb/host/xhci-plat.c +6 -2
@@ -183,6 +183,8 @@ static int xhci_plat_probe(struct platform_device *pdev)struct usb_hcd *hcd;int ret;int irq; +struct xhci_plat_priv *priv =NULL ; +if (usb_disabled ())return -ENODEV ;
@@ -280,8 +282,7 @@ static int xhci_plat_probe(struct platform_device *pdev) priv_match =dev_get_platdata (&pdev->dev);if (priv_match) { -struct xhci_plat_priv *priv =hcd_to_xhci_priv (hcd); - + priv =hcd_to_xhci_priv (hcd);/* Just copy data for now */ *priv = *priv_match; }
@@ -329,6 +330,9 @@ static int xhci_plat_probe(struct platform_device *pdev) hcd->tpl_support =of_usb_host_tpl_support (sysdev->of_node); xhci->shared_hcd->tpl_support = hcd->tpl_support; +if (priv && (priv->quirks &XHCI_SKIP_PHY_INIT )) + hcd->skip_phy_initialization =1 ; + ret =usb_add_hcd (hcd, irq,IRQF_SHARED );if (ret) goto disable_usb_phy;
drivers/usb/host/xhci.h +1 -0
@@ -1874,6 +1874,7 @@ struct xhci_hcd {#define XHCI_RESET_PLL_ON_DISCONNECT BIT_ULL(34)#define XHCI_SNPS_BROKEN_SUSPEND BIT_ULL(35)#define XHCI_RENESAS_FW_QUIRK BIT_ULL(36) +#define XHCI_SKIP_PHY_INIT BIT_ULL(37)unsigned int num_active_eps;unsigned int limit_active_eps;
Signature · 1 line
2.17.1
PATCH v2 5/7 usb: host: xhci-plat: add wakeup entry at sysfs
In reply to Peter ChenWith this change, there will be a wakeup entry at /sys/../power/wakeup, and the user could use this entry to choose whether enable xhci wakeup features (wake up system from suspend) or not.
--- drivers/usb/host/xhci-plat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/host/xhci-plat.c +1 -1
@@ -287,7 +287,7 @@ static int xhci_plat_probe(struct platform_device *pdev) *priv = *priv_match; } -device_wakeup_enable (hcd->self.controller); +device_set_wakeup_capable (&pdev->dev, true); xhci->main_hcd = hcd; xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
Signature · 1 line
2.17.1
Re: [PATCH v2 5/7] usb: host: xhci-plat: add wakeup entry at sysfs
Quoted 14 lines · Peter Chen
With this change, there will be a wakeup entry at
/sys/../power/wakeup, and the user could use this entry to choose whether enable xhci wakeup features (wake up system from suspend) or not.Signed-off-by Peter Chen <[email protected]>--- drivers/usb/host/xhci-plat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/usb/host/xhci-plat.c +0
-0
drivers/usb/host/xhci-plat.c static int xhci_plat_probe(struct platform_device *pdev) · line 290
Quoted 5 lines
drivers/usb/host/xhci-plat.c +1
-1@@ -287,7 +287,7 @@ static int xhci_plat_probe(struct platform_device *pdev) *priv = *priv_match; } -device_wakeup_enable (hcd->self.controller); +device_set_wakeup_capable (&pdev->dev, true);
All other patches seem pretty straight forward, but with this one I have to some more digging.
I saw you had a discussion with Alan about this topic about a month ago, but I'm not sure if there was any conclusion about this.
-Mathias
RE: [PATCH v2 5/7] usb: host: xhci-plat: add wakeup entry at sysfs
Quoted 15 lines
Quoted 15 lines
Signed-off-by Peter Chen <[email protected]>--- drivers/usb/host/xhci-plat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)(quoted diff) +0
-0
b/drivers/usb/host/xhci-plat.cindex14ff65a387e8..cfca6fc8947c100644drivers/usb/host/xhci-plat.c +0
-0@@ -287,7 +287,7 @@ static int xhci_plat_probe(struct platform_device *pdev) *priv = *priv_match; }
Quoted 8 lines
drivers/usb/host/xhci-plat.c static int xhci_plat_probe(struct platform_device *pdev) · line 290
Quoted 2 lines
drivers/usb/host/xhci-plat.c +1
-1-device_wakeup_enable (hcd->self.controller); +device_set_wakeup_capable (&pdev->dev, true);All other patches seem pretty straight forward, but with this one I have to some more digging.
I saw you had a discussion with Alan about this topic about a month ago, but I'm not sure if there was any conclusion about this.
Alan suggested set power/wakeup as default enabled before, but it will cause the power domain of this device (and parent) will not be off during the system suspend/resume since the power management code thinks that the power should not be off if the device needs to respond the wakeup during the system suspends. We have agreed to set power/wakeup as default disabled.
https://www.spinics.net/lists/linux-usb/msg199316.html
Peter
Re: [PATCH v2 5/7] usb: host: xhci-plat: add wakeup entry at sysfs
Quoted 24 lines · Peter Chen
Quoted 15 lines
Quoted 15 lines
Signed-off-by Peter Chen <[email protected]>--- drivers/usb/host/xhci-plat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)(quoted diff) +0
-0
b/drivers/usb/host/xhci-plat.cindex14ff65a387e8..cfca6fc8947c100644drivers/usb/host/xhci-plat.c +0
-0@@ -287,7 +287,7 @@ static int xhci_plat_probe(struct platform_device *pdev) *priv = *priv_match; }Quoted 8 lines
drivers/usb/host/xhci-plat.c static int xhci_plat_probe(struct platform_device *pdev) · line 290
Quoted 2 lines
drivers/usb/host/xhci-plat.c +1
-1-device_wakeup_enable (hcd->self.controller); +device_set_wakeup_capable (&pdev->dev, true);All other patches seem pretty straight forward, but with this one I have to some more digging.
I saw you had a discussion with Alan about this topic about a month ago, but I'm not sure if there was any conclusion about this.
Quoted 9 lines
Alan suggested set power/wakeup as default enabled before, but it will cause the power domain of this device (and parent) will not be off during the system suspend/resume since the power management code thinks that the power should not be off if the device needs to respond the wakeup during the system suspends. We have agreed to set power/wakeup as default disabled.
https://www.spinics.net/lists/linux-usb/msg199316.html
Peter
Ok, thanks, got it.
-Mathias
Re: [PATCH v2 5/7] usb: host: xhci-plat: add wakeup entry at sysfs
In reply to Peter ChenQuoted 5 lines · Peter Chen
With this change, there will be a wakeup entry at
/sys/../power/wakeup, and the user could use this entry to choose whether enable xhci wakeup features (wake up system from suspend) or not.Signed-off-by Peter Chen <[email protected]>
PATCH v2 6/7 usb: host: xhci-plat: improve the comments for xhci_plat_suspend
In reply to Peter ChenTo reflect the current code status.
--- drivers/usb/host/xhci-plat.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/usb/host/xhci-plat.c +1 -5
@@ -421,11 +421,7 @@ static int __maybe_unused xhci_plat_suspend(struct device *dev)return ret;/* * xhci_suspend() needs `do_wakeup` to know whether host is allowed -* to do wakeup during suspend. Since xhci_plat_suspend is currently -* only designed for system suspend, device_may_wakeup() is enough -* to dertermine whether host is allowed to do wakeup. Need to -* reconsider this when xhci_plat_suspend enlarges its scope, e.g., -* also applies to runtime suspend. +* to do wakeup during suspend. */ return xhci_suspend (xhci,device_may_wakeup (dev)); }
Signature · 1 line
2.17.1
PATCH v2 7/7 usb: host: xhci: avoid calling contineous two times for xhci_suspend
In reply to Peter ChenIf the xhci-plat.c is the platform driver, after the runtime pm is enabled, the xhci_suspend is called if nothing is connected on the port. When the system goes to suspend, it will call xhci_suspend again if USB wakeup is enabled.
Since the runtime suspend wakeup setting is not always the same with system suspend wakeup setting, eg, at runtime suspend, we always need wakeup if the controller is in low power mode; but at suspend system, we may not need wakeup. So, we move the judgement after changing wakeup setting.
--- drivers/usb/host/xhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/usb/host/xhci.c +5 -2
@@ -982,12 +982,15 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup) xhci->shared_hcd->state != HC_STATE_SUSPENDED)return -EINVAL ; -xhci_dbc_suspend (xhci); -/* Clear root port wake on bits if wakeup not allowed. */ if (!do_wakeup)xhci_disable_port_wake_on_bits (xhci); +if (!HCD_HW_ACCESSIBLE (hcd)) +return 0 ; + +xhci_dbc_suspend (xhci); +/* Don't poll the roothubs on bus suspend. */ xhci_dbg (xhci,"%s: stopping port polling.\n" , __func__);clear_bit (HCD_FLAG_POLL_RH , &hcd->flags);
Signature · 1 line
2.17.1