Hi,
On Wed, Oct 10, 2012 at 07:35:49PM +0530, Vikas C Sajjan wrote:
> From: Vikas Sajjan <vikas.sajjan@xxxxxxxxxx>
>
> Adding the suspend and resume functionality to exynos dwc3 driver
>
> Signed-off-by: Abhilash Kesavan <a.kesavan@xxxxxxxxxxx>
> Signed-off-by: Vikas C Sajjan <vikas.sajjan@xxxxxxxxxx>
> CC: Doug Anderson <dianders@xxxxxxxxxxxx>
> ---
> drivers/usb/dwc3/dwc3-exynos.c | 60 ++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 60 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> index ca65978..1154cee 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -15,6 +15,7 @@
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/slab.h>
> +#include <linux/pm_runtime.h>
> #include <linux/platform_device.h>
> #include <linux/platform_data/dwc3-exynos.h>
> #include <linux/dma-mapping.h>
> @@ -200,11 +201,70 @@ static int __devexit dwc3_exynos_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_PM
> +static int dwc3_exynos_suspend(struct device *dev)
> +{
> + struct dwc3_exynos_data *pdata = dev->platform_data;
> + struct dwc3_exynos *exynos;
> + struct platform_device *pdev = to_platform_device(dev);
you shouldn't have to access platform device here.
> +
> + exynos = dev_get_drvdata(dev);
> +
> + if (!exynos)
> + return -EINVAL;
this shouldn't ever be the case and if it is, it deserves the Oops.
> +
> + if (pdata && pdata->phy_exit)
> + pdata->phy_exit(pdev, pdata->phy_type);
NAK, you should introduce a proper PHY driver and handle PHY suspend
generically in dwc3/core.c
> + clk_disable(exynos->clk);
> +
> + return 0;
> +}
> +
> +static int dwc3_exynos_resume(struct device *dev)
> +{
> + struct dwc3_exynos_data *pdata = dev->platform_data;
> + struct dwc3_exynos *exynos;
> + struct platform_device *pdev = to_platform_device(dev);
ditto
> + exynos = dev_get_drvdata(dev);
> +
> + if (!exynos)
> + return -EINVAL;
ditto
> + clk_enable(exynos->clk);
> +
> + /* PHY initialization */
> + if (!pdata) {
> + dev_dbg(&pdev->dev, "missing platform data\n");
> + } else {
> + if (pdata->phy_init)
> + pdata->phy_init(pdev, pdata->phy_type);
> + }
missing PHY driver
> + /* runtime set active to reflect active state. */
> + pm_runtime_disable(dev);
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops dwc3_exynos_pm_ops = {
> + .suspend = dwc3_exynos_suspend,
> + .resume = dwc3_exynos_resume,
> +};
right here you should:
#define DWC3_EXYNOS_DEV_PM_OPS &(dwc3_exynos_pm_ops)
#else
#define DWC3_EXYNOS_DEV_PM_OPS NULL
> +#endif /* CONFIG_PM */
> +
> static struct platform_driver dwc3_exynos_driver = {
> .probe = dwc3_exynos_probe,
> .remove = __devexit_p(dwc3_exynos_remove),
> .driver = {
> .name = "exynos-dwc3",
> +#ifdef CONFIG_PM
> + .pm = &dwc3_exynos_pm_ops,
.pm = DWC3_EXYNOS_DEV_PM_OPS,
> +#endif
avoid the ifdeferry all over, see above.
--
balbi
Attachment:
signature.asc
Description: Digital signature
