| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <B256D81BAE5131468A838E5D7A243641BF592EF5@penmbx01>
Date: Mon, 20 Oct 2014 01:59:05 +0000
From: "Yang, Wenyou" <Wenyou.Yang@...el.com>
To: Kevin Hilman <khilman@...nel.org>
CC: "broonie@...nel.org" <broonie@...nel.org>,
"linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Ferre, Nicolas" <Nicolas.FERRE@...el.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH] spi/atmel: add support for runtime PM
> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@...nel.org]
> Sent: Friday, October 17, 2014 9:03 PM
> To: Yang, Wenyou
> Cc: broonie@...nel.org; linux-spi@...r.kernel.org; linux-kernel@...r.kernel.org;
> Ferre, Nicolas; linux-arm-kernel@...ts.infradead.org
> Subject: Re: [PATCH] spi/atmel: add support for runtime PM
>
> Wenyou Yang <wenyou.yang@...el.com> writes:
>
> > Drivers should put the device into low power states proactively
> > whenever the device is not in use. Thus implement support for runtime
> > PM and use the autosuspend feature to make sure that we can still
> > perform well in case we see lots of SPI traffic within short period of time.
> >
> > Signed-off-by: Wenyou Yang <wenyou.yang@...el.com>
> > ---
> > drivers/spi/spi-atmel.c | 62
> +++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 55 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c index
> > 649dcb5..aca285e 100644
> > --- a/drivers/spi/spi-atmel.c
> > +++ b/drivers/spi/spi-atmel.c
> > @@ -26,6 +26,7 @@
> > #include <linux/io.h>
> > #include <linux/gpio.h>
> > #include <linux/pinctrl/consumer.h>
> > +#include <linux/pm_runtime.h>
> >
> > /* SPI register offsets */
> > #define SPI_CR 0x0000
> > @@ -191,6 +192,8 @@
> >
> > #define SPI_DMA_TIMEOUT (msecs_to_jiffies(1000))
> >
> > +#define AUTOSUSPEND_TIMEOUT 2000
> > +
> > struct atmel_spi_dma {
> > struct dma_chan *chan_rx;
> > struct dma_chan *chan_tx;
> > @@ -1313,6 +1316,7 @@ static int atmel_spi_probe(struct platform_device
> *pdev)
> > master->setup = atmel_spi_setup;
> > master->transfer_one_message = atmel_spi_transfer_one_message;
> > master->cleanup = atmel_spi_cleanup;
> > + master->auto_runtime_pm = true;
> > platform_set_drvdata(pdev, master);
> >
> > as = spi_master_get_devdata(master); @@ -1385,6 +1389,11 @@ static
> > int atmel_spi_probe(struct platform_device *pdev)
> > dev_info(&pdev->dev, "Atmel SPI Controller at 0x%08lx (irq %d)\n",
> > (unsigned long)regs->start, irq);
> >
> > + pm_runtime_set_autosuspend_delay(&pdev->dev,
> AUTOSUSPEND_TIMEOUT);
> > + pm_runtime_use_autosuspend(&pdev->dev);
> > + pm_runtime_set_active(&pdev->dev);
> > + pm_runtime_enable(&pdev->dev);
> > +
> > ret = devm_spi_register_master(&pdev->dev, master);
> > if (ret)
> > goto out_free_dma;
> > @@ -1392,6 +1401,9 @@ static int atmel_spi_probe(struct platform_device
> *pdev)
> > return 0;
> >
> > out_free_dma:
> > + pm_runtime_disable(&pdev->dev);
> > + pm_runtime_set_suspended(&pdev->dev);
> > +
> > if (as->use_dma)
> > atmel_spi_release_dma(as);
> >
> > @@ -1413,6 +1425,8 @@ static int atmel_spi_remove(struct platform_device
> *pdev)
> > struct spi_master *master = platform_get_drvdata(pdev);
> > struct atmel_spi *as = spi_master_get_devdata(master);
> >
> > + pm_runtime_get_sync(&pdev->dev);
> > +
> > /* reset the hardware and block queue progress */
> > spin_lock_irq(&as->lock);
> > if (as->use_dma) {
> > @@ -1430,9 +1444,13 @@ static int atmel_spi_remove(struct
> > platform_device *pdev)
> >
> > clk_disable_unprepare(as->clk);
> >
> > + pm_runtime_put_noidle(&pdev->dev);
>
> Why the _noidle version?
Because invoked from the _remove function, only decrement the device's usage counter, no further idle operation need.
>
> > + pm_runtime_disable(&pdev->dev);
> > +
> > return 0;
> > }
> >
> > +#ifdef CONFIG_PM
>
> Why CONFIG_PM? CONFIG_PM_SLEEP causes CONFIG_PM=y, so this seems
> redundant.
Accepted, I will remove it.
>
> > #ifdef CONFIG_PM_SLEEP
> > static int atmel_spi_suspend(struct device *dev) { @@ -1447,9
> > +1465,10 @@ static int atmel_spi_suspend(struct device *dev)
> > return ret;
> > }
> >
> > - clk_disable_unprepare(as->clk);
> > -
> > - pinctrl_pm_select_sleep_state(dev);
> > + if (!pm_runtime_suspended(dev)) {
> > + clk_disable_unprepare(as->clk);
> > + pinctrl_pm_select_sleep_state(dev);
> > + }
>
> a.k.a. pm_runtime_put_sync() since the ->runtime_suspend() callback does the
> same thing.
>
> > return 0;
> > }
> > @@ -1460,9 +1479,10 @@ static int atmel_spi_resume(struct device *dev)
> > struct atmel_spi *as = spi_master_get_devdata(master);
> > int ret;
> >
> > - pinctrl_pm_select_default_state(dev);
> > -
> > - clk_prepare_enable(as->clk);
> > + if (!pm_runtime_suspended(dev)) {
> > + pinctrl_pm_select_default_state(dev);
> > + clk_prepare_enable(as->clk);
> > + }
>
> a.k.a. pm_runtime_get_sync()
>
> Kevin
>
> > /* Start the queue running */
> > ret = spi_master_resume(master);
> > @@ -1471,9 +1491,37 @@ static int atmel_spi_resume(struct device *dev)
> >
> > return ret;
> > }
> > +#endif
> >
> > -static SIMPLE_DEV_PM_OPS(atmel_spi_pm_ops, atmel_spi_suspend,
> > atmel_spi_resume);
> > +#ifdef CONFIG_PM_RUNTIME
> > +static int atmel_spi_runtime_suspend(struct device *dev) {
> > + struct spi_master *master = dev_get_drvdata(dev);
> > + struct atmel_spi *as = spi_master_get_devdata(master);
> >
> > + clk_disable_unprepare(as->clk);
> > + pinctrl_pm_select_sleep_state(dev);
> > +
> > + return 0;
> > +}
> > +
> > +static int atmel_spi_runtime_resume(struct device *dev) {
> > + struct spi_master *master = dev_get_drvdata(dev);
> > + struct atmel_spi *as = spi_master_get_devdata(master);
> > +
> > + pinctrl_pm_select_default_state(dev);
> > + clk_prepare_enable(as->clk);
> > +
> > + return 0;
> > +}
> > +#endif
> > +
> > +static const struct dev_pm_ops atmel_spi_pm_ops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(atmel_spi_suspend, atmel_spi_resume)
> > + SET_RUNTIME_PM_OPS(atmel_spi_runtime_suspend,
> > + atmel_spi_runtime_resume, NULL) };
> > #define ATMEL_SPI_PM_OPS (&atmel_spi_pm_ops)
> > #else
> > #define ATMEL_SPI_PM_OPS NULL
Best Regards,
Wenyou Yang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists