On Mon, Aug 15, 2011 at 04:51:27PM +0200, Sebastian Andrzej Siewior wrote:
> usb_hcd_pci_pm_ops provides run-time functionality for the USB HCDs.
> It expects that driver's data is a struct usb_hcd. This does not work
> if we create the HCD device is a platform device and its parent is the
> PCI device. We create the pci_dev from device and usb_hcd from parent's
> driver data.
> This patch is preparation to let the xhci core work under a platform
> device.
This patch looks a bit messy, with all the nearly-duplicate function
names like check_root_hub_suspended and _check_root_hub_suspended.
I think the code might be more readable if check_root_hub_suspended was
renamed to check_common_root_hub_suspended and you added two new
functions: the check_plat_pci_root_hub_suspended function you already
have, and another function called check_pci_root_hub_suspended.
You could do similar things with all the functions in this patch.
Alan, what do you think?
Sarah Sharp
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
> drivers/usb/core/hcd-pci.c | 133 ++++++++++++++++++++++++++++++++++++++-----
> include/linux/usb/hcd.h | 1 +
> 2 files changed, 118 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
> index ce22f4a..01f8955 100644
> --- a/drivers/usb/core/hcd-pci.c
> +++ b/drivers/usb/core/hcd-pci.c
> @@ -19,6 +19,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> +#include <linux/platform_device.h>
> #include <linux/usb.h>
> #include <linux/usb/hcd.h>
>
> @@ -358,11 +359,8 @@ static inline void powermac_set_asic(struct pci_dev *pci_dev, int enable)
>
> #endif /* CONFIG_PPC_PMAC */
>
> -static int check_root_hub_suspended(struct device *dev)
> +static int _check_root_hub_suspended(struct device *dev, struct usb_hcd *hcd)
> {
> - struct pci_dev *pci_dev = to_pci_dev(dev);
> - struct usb_hcd *hcd = pci_get_drvdata(pci_dev);
> -
> if (HCD_RH_RUNNING(hcd)) {
> dev_warn(dev, "Root hub is not suspended\n");
> return -EBUSY;
> @@ -377,10 +375,26 @@ static int check_root_hub_suspended(struct device *dev)
> return 0;
> }
>
> -static int suspend_common(struct device *dev, bool do_wakeup)
> +static int check_root_hub_suspended(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> struct usb_hcd *hcd = pci_get_drvdata(pci_dev);
> +
> + return _check_root_hub_suspended(dev, hcd);
> +}
> +
> +static int check_plat_pci_root_hub_suspended(struct device *dev)
> +{
> + struct usb_hcd *hcd = dev_get_drvdata(dev->parent);
> +
> + return _check_root_hub_suspended(dev, hcd);
> +}
> +
> +
> +static int _suspend_common(struct device *dev, bool do_wakeup,
> + struct usb_hcd *hcd)
> +{
> + struct pci_dev *pci_dev = to_pci_dev(dev);
> int retval;
>
> /* Root hub suspend should have stopped all downstream traffic,
> @@ -388,7 +402,7 @@ static int suspend_common(struct device *dev, bool do_wakeup)
> * and the stub usb_device (which we check here). But maybe it
> * didn't; writing sysfs power/state files ignores such rules...
> */
> - retval = check_root_hub_suspended(dev);
> + retval = _check_root_hub_suspended(dev, hcd);
> if (retval)
> return retval;
>
> @@ -432,10 +446,9 @@ static int suspend_common(struct device *dev, bool do_wakeup)
> return retval;
> }
>
> -static int resume_common(struct device *dev, int event)
> +static int _resume_common(struct device *dev, int event, struct usb_hcd *hcd)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> - struct usb_hcd *hcd = pci_get_drvdata(pci_dev);
> int retval;
>
> if (HCD_RH_RUNNING(hcd) ||
> @@ -477,16 +490,25 @@ static int resume_common(struct device *dev, int event)
>
> static int hcd_pci_suspend(struct device *dev)
> {
> - return suspend_common(dev, device_may_wakeup(dev));
> + struct pci_dev *pci_dev = to_pci_dev(dev);
> + struct usb_hcd *hcd = pci_get_drvdata(pci_dev);
> +
> + return _suspend_common(dev, device_may_wakeup(dev), hcd);
> }
>
> -static int hcd_pci_suspend_noirq(struct device *dev)
> +static int hcd_plat_pci_suspend(struct device *dev)
> +{
> + struct usb_hcd *hcd = dev_get_drvdata(dev->parent);
> +
> + return _suspend_common(dev, device_may_wakeup(dev), hcd);
> +}
> +
> +static int _hcd_pci_suspend_noirq(struct device *dev, struct usb_hcd *hcd)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> - struct usb_hcd *hcd = pci_get_drvdata(pci_dev);
> int retval;
>
> - retval = check_root_hub_suspended(dev);
> + retval = _check_root_hub_suspended(dev, hcd);
> if (retval)
> return retval;
>
> @@ -519,6 +541,21 @@ static int hcd_pci_suspend_noirq(struct device *dev)
> return retval;
> }
>
> +static int hcd_pci_suspend_noirq(struct device *dev)
> +{
> + struct pci_dev *pci_dev = to_pci_dev(dev);
> + struct usb_hcd *hcd = pci_get_drvdata(pci_dev);
> +
> + return _hcd_pci_suspend_noirq(dev, hcd);
> +}
> +
> +static int hcd_plat_pci_suspend_noirq(struct device *dev)
> +{
> + struct usb_hcd *hcd = dev_get_drvdata(dev->parent);
> +
> + return _hcd_pci_suspend_noirq(dev, hcd);
> +}
> +
> static int hcd_pci_resume_noirq(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> @@ -532,12 +569,32 @@ static int hcd_pci_resume_noirq(struct device *dev)
>
> static int hcd_pci_resume(struct device *dev)
> {
> - return resume_common(dev, PM_EVENT_RESUME);
> + struct pci_dev *pci_dev = to_pci_dev(dev);
> + struct usb_hcd *hcd = pci_get_drvdata(pci_dev);
> +
> + return _resume_common(dev, PM_EVENT_RESUME, hcd);
> +}
> +
> +static int hcd_plat_pci_resume(struct device *dev)
> +{
> + struct usb_hcd *hcd = dev_get_drvdata(dev->parent);
> +
> + return _resume_common(dev, PM_EVENT_RESUME, hcd);
> }
>
> static int hcd_pci_restore(struct device *dev)
> {
> - return resume_common(dev, PM_EVENT_RESTORE);
> + struct pci_dev *pci_dev = to_pci_dev(dev);
> + struct usb_hcd *hcd = pci_get_drvdata(pci_dev);
> +
> + return _resume_common(dev, PM_EVENT_RESTORE, hcd);
> +}
> +
> +static int hcd_plat_pci_restore(struct device *dev)
> +{
> + struct usb_hcd *hcd = dev_get_drvdata(dev->parent);
> +
> + return _resume_common(dev, PM_EVENT_RESTORE, hcd);
> }
>
> #else
> @@ -548,35 +605,61 @@ static int hcd_pci_restore(struct device *dev)
> #define hcd_pci_resume NULL
> #define hcd_pci_restore NULL
>
> +#define hcd_plat_pci_suspend NULL
> +#define hcd_plat_pci_suspend_noirq NULL
> +#define hcd_plat_pci_resume_noirq NULL
> +#define hcd_plat_pci_resume NULL
> +#define hcd_plat_pci_restore NULL
> +
> #endif /* CONFIG_PM_SLEEP */
>
> #ifdef CONFIG_PM_RUNTIME
>
> static int hcd_pci_runtime_suspend(struct device *dev)
> {
> + struct pci_dev *pci_dev = to_pci_dev(dev);
> + struct usb_hcd *hcd = pci_get_drvdata(pci_dev);
> int retval;
>
> - retval = suspend_common(dev, true);
> + retval = _suspend_common(dev, true, hcd);
> if (retval == 0)
> powermac_set_asic(to_pci_dev(dev), 0);
> dev_dbg(dev, "hcd_pci_runtime_suspend: %d\n", retval);
> return retval;
> }
>
> +static int hcd_plat_pci_runtime_suspend(struct device *dev)
> +{
> + struct usb_hcd *hcd = dev_get_drvdata(dev->parent);
> +
> + return _suspend_common(dev, true, hcd);
> +}
> +
> static int hcd_pci_runtime_resume(struct device *dev)
> {
> + struct pci_dev *pci_dev = to_pci_dev(dev);
> + struct usb_hcd *hcd = pci_get_drvdata(pci_dev);
> int retval;
>
> powermac_set_asic(to_pci_dev(dev), 1);
> - retval = resume_common(dev, PM_EVENT_AUTO_RESUME);
> + retval = _resume_common(dev, PM_EVENT_AUTO_RESUME, hcd);
> dev_dbg(dev, "hcd_pci_runtime_resume: %d\n", retval);
> return retval;
> }
>
> +static int hcd_plat_pci_runtime_resume(struct device *dev)
> +{
> + struct usb_hcd *hcd = dev_get_drvdata(dev->parent);
> +
> + return _resume_common(dev, PM_EVENT_AUTO_RESUME, hcd);
> +}
> +
> #else
>
> #define hcd_pci_runtime_suspend NULL
> #define hcd_pci_runtime_resume NULL
> +#define hcd_plat_pci_runtime_suspend NULL
> +#define hcd_plat_pci_runtime_resume NULL
>
> #endif /* CONFIG_PM_RUNTIME */
>
> @@ -598,4 +681,22 @@ const struct dev_pm_ops usb_hcd_pci_pm_ops = {
> };
> EXPORT_SYMBOL_GPL(usb_hcd_pci_pm_ops);
>
> +const struct dev_pm_ops usb_hcd_plat_pci_pm_ops = {
> + .suspend = hcd_plat_pci_suspend,
> + .suspend_noirq = hcd_plat_pci_suspend_noirq,
> + .resume_noirq = hcd_pci_resume_noirq,
> + .resume = hcd_plat_pci_resume,
> + .freeze = check_plat_pci_root_hub_suspended,
> + .freeze_noirq = check_plat_pci_root_hub_suspended,
> + .thaw_noirq = NULL,
> + .thaw = NULL,
> + .poweroff = hcd_plat_pci_suspend,
> + .poweroff_noirq = hcd_plat_pci_suspend_noirq,
> + .restore_noirq = hcd_pci_resume_noirq,
> + .restore = hcd_plat_pci_restore,
> + .runtime_suspend = hcd_plat_pci_runtime_suspend,
> + .runtime_resume = hcd_plat_pci_runtime_resume,
> +};
> +EXPORT_SYMBOL_GPL(usb_hcd_plat_pci_pm_ops);
> +
> #endif /* CONFIG_PM */
> diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
> index 0097136..61181a4 100644
> --- a/include/linux/usb/hcd.h
> +++ b/include/linux/usb/hcd.h
> @@ -396,6 +396,7 @@ extern void usb_hcd_pci_shutdown(struct pci_dev *dev);
>
> #ifdef CONFIG_PM_SLEEP
> extern const struct dev_pm_ops usb_hcd_pci_pm_ops;
> +extern const struct dev_pm_ops usb_hcd_plat_pci_pm_ops;
> #endif
> #endif /* CONFIG_PCI */
>
> --
> 1.7.4.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
