| Message ID | 1421529942-18437-2-git-send-email-sylvain.rochet@finsecur.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello.
On 01/18/2015 12:25 AM, Sylvain Rochet wrote:
There's little inconsistency in your patch subjects: you're using '_' but
the files you're modifying are named using '-'...
> This patch add suspend/resume support for Atmel EHCI, mostly
> about disabling and unpreparing clocks so USB PLL is stopped
> before entering sleep state.
> Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
> ---
> drivers/usb/host/ehci-atmel.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
> diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
> index 56a8850..2e0043b 100644
> --- a/drivers/usb/host/ehci-atmel.c
> +++ b/drivers/usb/host/ehci-atmel.c
[...]
> @@ -187,6 +217,8 @@ static struct platform_driver ehci_atmel_driver = {
> .probe = ehci_atmel_drv_probe,
> .remove = ehci_atmel_drv_remove,
> .shutdown = usb_hcd_platform_shutdown,
> + .suspend = ehci_atmel_drv_suspend,
> + .resume = ehci_atmel_drv_resume,
I think you should use 'struct dev_pm_ops' now.
WBR, Sergei
Hi Sergei, On Sun, Jan 18, 2015 at 01:22:38AM +0300, Sergei Shtylyov wrote: > > There's little inconsistency in your patch subjects: you're using > '_' but the files you're modifying are named using '-'... Indeed. > >@@ -187,6 +217,8 @@ static struct platform_driver ehci_atmel_driver = { > > .probe = ehci_atmel_drv_probe, > > .remove = ehci_atmel_drv_remove, > > .shutdown = usb_hcd_platform_shutdown, > >+ .suspend = ehci_atmel_drv_suspend, > >+ .resume = ehci_atmel_drv_resume, > > I think you should use 'struct dev_pm_ops' now. This way ? static int ehci_atmel_drv_suspend(struct device *dev) { struct usb_hcd *hcd = dev_get_drvdata(dev); (...) static SIMPLE_DEV_PM_OPS(ehci_atmel_pm_ops, ehci_atmel_drv_suspend, ehci_atmel_drv_resume); (...) .driver = { .pm = &ehci_atmel_pm_ops, } (...) Should I send a v4 or can I send this change separately on top of the previous change ? Sylvain
Hi Sylvain, On Sat, 17 Jan 2015 23:49:00 +0100 Sylvain Rochet <sylvain.rochet@finsecur.com> wrote: > Hi Sergei, > > > On Sun, Jan 18, 2015 at 01:22:38AM +0300, Sergei Shtylyov wrote: > > > > There's little inconsistency in your patch subjects: you're using > > '_' but the files you're modifying are named using '-'... > > Indeed. > > > > >@@ -187,6 +217,8 @@ static struct platform_driver ehci_atmel_driver = { > > > .probe = ehci_atmel_drv_probe, > > > .remove = ehci_atmel_drv_remove, > > > .shutdown = usb_hcd_platform_shutdown, > > >+ .suspend = ehci_atmel_drv_suspend, > > >+ .resume = ehci_atmel_drv_resume, > > > > I think you should use 'struct dev_pm_ops' now. > > This way ? > > static int ehci_atmel_drv_suspend(struct device *dev) > { > struct usb_hcd *hcd = dev_get_drvdata(dev); > (...) > > > static SIMPLE_DEV_PM_OPS(ehci_atmel_pm_ops, ehci_atmel_drv_suspend, ehci_atmel_drv_resume); > > (...) > .driver = { > .pm = &ehci_atmel_pm_ops, > } > (...) > > > Should I send a v4 or can I send this change separately on top of the > previous change ? I think it's better to send a v4 reworking this patch (you'll have to change your commit subject anyway ;-)). Best Regards, Boris
Hello. On 01/18/2015 01:49 AM, Sylvain Rochet wrote: >> There's little inconsistency in your patch subjects: you're using >> '_' but the files you're modifying are named using '-'... > Indeed. >>> @@ -187,6 +217,8 @@ static struct platform_driver ehci_atmel_driver = { >>> .probe = ehci_atmel_drv_probe, >>> .remove = ehci_atmel_drv_remove, >>> .shutdown = usb_hcd_platform_shutdown, >>> + .suspend = ehci_atmel_drv_suspend, >>> + .resume = ehci_atmel_drv_resume, >> >> I think you should use 'struct dev_pm_ops' now. I'm not sure but perhaps scripts/checkpatch.pl would complain about the old-style PM methods... should check. > This way ? > static int ehci_atmel_drv_suspend(struct device *dev) > { > struct usb_hcd *hcd = dev_get_drvdata(dev); > (...) > > > static SIMPLE_DEV_PM_OPS(ehci_atmel_pm_ops, ehci_atmel_drv_suspend, ehci_atmel_drv_resume); > > (...) > .driver = { > .pm = &ehci_atmel_pm_ops, > } > (...) Yes, probably. > Should I send a v4 or can I send this change separately on top of the > previous change ? v4 please. > Sylvain WBR, Sergei
diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c index 56a8850..2e0043b 100644 --- a/drivers/usb/host/ehci-atmel.c +++ b/drivers/usb/host/ehci-atmel.c @@ -37,6 +37,8 @@ static int clocked; static void atmel_start_clock(void) { + if (clocked) + return; if (IS_ENABLED(CONFIG_COMMON_CLK)) { clk_set_rate(uclk, 48000000); clk_prepare_enable(uclk); @@ -48,6 +50,8 @@ static void atmel_start_clock(void) static void atmel_stop_clock(void) { + if (!clocked) + return; clk_disable_unprepare(fclk); clk_disable_unprepare(iclk); if (IS_ENABLED(CONFIG_COMMON_CLK)) @@ -174,6 +178,32 @@ static int ehci_atmel_drv_remove(struct platform_device *pdev) return 0; } +#ifdef CONFIG_PM +static int ehci_atmel_drv_suspend(struct platform_device *pdev, pm_message_t mesg) +{ + struct usb_hcd *hcd = platform_get_drvdata(pdev); + int ret; + + ret = ehci_suspend(hcd, false); + if (ret) + return ret; + + atmel_stop_clock(); + return 0; +} + +static int ehci_atmel_drv_resume(struct platform_device *pdev) +{ + struct usb_hcd *hcd = platform_get_drvdata(pdev); + + atmel_start_clock(); + return ehci_resume(hcd, false); +} +#else +#define ehci_atmel_drv_suspend NULL +#define ehci_atmel_drv_resume NULL +#endif + #ifdef CONFIG_OF static const struct of_device_id atmel_ehci_dt_ids[] = { { .compatible = "atmel,at91sam9g45-ehci" }, @@ -187,6 +217,8 @@ static struct platform_driver ehci_atmel_driver = { .probe = ehci_atmel_drv_probe, .remove = ehci_atmel_drv_remove, .shutdown = usb_hcd_platform_shutdown, + .suspend = ehci_atmel_drv_suspend, + .resume = ehci_atmel_drv_resume, .driver = { .name = "atmel-ehci", .of_match_table = of_match_ptr(atmel_ehci_dt_ids),
This patch add suspend/resume support for Atmel EHCI, mostly about disabling and unpreparing clocks so USB PLL is stopped before entering sleep state. Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com> --- drivers/usb/host/ehci-atmel.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)