Re: [PATCH 00/15] ARM: pxa: switch to DMA slave maps

[Ulf Hansson]

On 4 April 2018 at 21:56, Boris Brezillon <boris.brezil...@bootlin.com> wrote:
> On Wed, 04 Apr 2018 21:49:26 +0200
> Robert Jarzmik <robert.jarz...@free.fr> wrote:
>
>> Ulf Hansson <ulf.hans...@linaro.org> writes:
>>
>> > On 2 April 2018 at 16:26, Robert Jarzmik <robert.jarz...@free.fr> wrote:
>> >> Hi,
>> >>
>> >> This serie is aimed at removing the dmaengine slave compat use, and 
>> >> transfer
>> >> knowledge of the DMA requestors into architecture code.
>> >> As this looks like a patch bomb, each maintainer expressing for his tree 
>> >> either
>> >> an Ack or "I want to take through my tree" will be spared in the next 
>> >> iterations
>> >> of this serie.
>> >
>> > Perhaps an option is to send this hole series as PR for 3.17 rc1, that
>> > would removed some churns and make this faster/easier? Well, if you
>> > receive the needed acks of course.
>> For 3.17-rc1 it looks a bit optimistic with the review time ... If I have all
>
> Especially since 3.17-rc1 has been released more than 3 years ago :-),
> but I guess you meant 4.17-rc1.

Yeah, I realize that I was a bit lost in time yesterday. Even more
people have been having fun about it (me too). :-)

Kind regards
Uffe

Re: [PATCH 00/15] ARM: pxa: switch to DMA slave maps

[Ulf Hansson]

On 2 April 2018 at 16:26, Robert Jarzmik <robert.jarz...@free.fr> wrote:
> Hi,
>
> This serie is aimed at removing the dmaengine slave compat use, and transfer
> knowledge of the DMA requestors into architecture code.
>
> This was discussed/advised by Arnd a couple of years back, it's almost time.
>
> The serie is divided in 3 phasees :
>  - phase 1 : patch 1/15 and patch 2/15
>=> this is the preparation work
>  - phase 2 : patches 3/15 .. 10/15
>=> this is the switch of all the drivers
>=> this one will require either an Ack of the maintainers or be taken by 
> them
>   once phase 1 is merged
>  - phase 3 : patches 11/15
>=> this is the last part, cleanup and removal of export of the DMA filter
>   function
>
> As this looks like a patch bomb, each maintainer expressing for his tree 
> either
> an Ack or "I want to take through my tree" will be spared in the next 
> iterations
> of this serie.

Perhaps an option is to send this hole series as PR for 3.17 rc1, that
would removed some churns and make this faster/easier? Well, if you
receive the needed acks of course.

For the mmc change:

Acked-by: Ulf Hansson <ulf.hans...@linaro.org>

Kind regards
Uffe

>
> Several of these changes have been tested on actual hardware, including :
>  - pxamci
>  - pxa_camera
>  - smc*
>  - ASoC and SSP
>
> Happy review.
>
> Robert Jarzmik (15):
>   dmaengine: pxa: use a dma slave map
>   ARM: pxa: add dma slave map
>   mmc: pxamci: remove the dmaengine compat need
>   media: pxa_camera: remove the dmaengine compat need
>   mtd: nand: pxa3xx: remove the dmaengine compat need
>   net: smc911x: remove the dmaengine compat need
>   net: smc91x: remove the dmaengine compat need
>   ASoC: pxa: remove the dmaengine compat need
>   net: irda: pxaficp_ir: remove the dmaengine compat need
>   ata: pata_pxa: remove the dmaengine compat need
>   dmaengine: pxa: document pxad_param
>   dmaengine: pxa: make the filter function internal
>   ARM: pxa: remove the DMA IO resources
>   ARM: pxa: change SSP devices allocation
>   ARM: pxa: change SSP DMA channels allocation
>
>  arch/arm/mach-pxa/devices.c   | 269 
> ++
>  arch/arm/mach-pxa/devices.h   |  14 +-
>  arch/arm/mach-pxa/include/mach/audio.h|  12 ++
>  arch/arm/mach-pxa/pxa25x.c|   4 +-
>  arch/arm/mach-pxa/pxa27x.c|   4 +-
>  arch/arm/mach-pxa/pxa3xx.c|   5 +-
>  arch/arm/plat-pxa/ssp.c   |  50 +-
>  drivers/ata/pata_pxa.c|  10 +-
>  drivers/dma/pxa_dma.c |  13 +-
>  drivers/media/platform/pxa_camera.c   |  22 +--
>  drivers/mmc/host/pxamci.c |  29 +---
>  drivers/mtd/nand/pxa3xx_nand.c|  10 +-
>  drivers/net/ethernet/smsc/smc911x.c   |  16 +-
>  drivers/net/ethernet/smsc/smc91x.c|  12 +-
>  drivers/net/ethernet/smsc/smc91x.h|   1 -
>  drivers/staging/irda/drivers/pxaficp_ir.c |  14 +-
>  include/linux/dma/pxa-dma.h   |  20 +--
>  include/linux/platform_data/mmp_dma.h |   4 +
>  include/linux/pxa2xx_ssp.h|   4 +-
>  sound/arm/pxa2xx-ac97.c   |  14 +-
>  sound/arm/pxa2xx-pcm-lib.c|   6 +-
>  sound/soc/pxa/pxa-ssp.c   |   5 +-
>  sound/soc/pxa/pxa2xx-ac97.c   |  32 +---
>  23 files changed, 196 insertions(+), 374 deletions(-)
>
> --
> 2.11.0
>

Re: [PATCH 04/12] exynos-gsc: Make runtime PM callbacks available for CONFIG_PM

[Ulf Hansson]

On 11 November 2016 at 10:47, Ulf Hansson <ulf.hans...@linaro.org> wrote:
> On 10 November 2016 at 04:44, kbuild test robot <l...@intel.com> wrote:
>> Hi Ulf,
>>
>> [auto build test ERROR on linuxtv-media/master]
>> [also build test ERROR on v4.9-rc4 next-20161109]
>> [if your patch is applied to the wrong git tree, please drop us a note to 
>> help improve the system]
>>
>> url:
>> https://github.com/0day-ci/linux/commits/Marek-Szyprowski/media-Exynos-GScaller-driver-fixes/20161110-48
>> base:   git://linuxtv.org/media_tree.git master
>> config: openrisc-allyesconfig (attached as .config)
>> compiler: or32-linux-gcc (GCC) 4.5.1-or32-1.0rc1
>> reproduce:
>> wget 
>> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
>>  -O ~/bin/make.cross
>> chmod +x ~/bin/make.cross
>> # save the attached .config to linux build tree
>> make.cross ARCH=openrisc
>>
>> Note: the 
>> linux-review/Marek-Szyprowski/media-Exynos-GScaller-driver-fixes/20161110-48
>>  HEAD 92b20676ac75659d1ea1d83b00e8028f45ea84e9 builds fine.
>>   It only hurts bisectibility.
>>
>> All errors (new ones prefixed by >>):
>>
>>drivers/media/platform/exynos-gsc/gsc-core.c: In function 'gsc_resume':
>>>> drivers/media/platform/exynos-gsc/gsc-core.c:1183:3: error: implicit 
>>>> declaration of function 'gsc_runtime_resume'
>>drivers/media/platform/exynos-gsc/gsc-core.c: In function 'gsc_suspend':
>>>> drivers/media/platform/exynos-gsc/gsc-core.c:1198:3: error: implicit 
>>>> declaration of function 'gsc_runtime_suspend'
>>
>
> Marek, to avoid the bisectibility issue, we could squash patch 4/12 with 6/12.

Urgh, ignore this.

What I meant was instead to squash these patches:
[PATCH 04/12] exynos-gsc: Make runtime PM callbacks available for CONFIG_PM
[PATCH 07/12] exynos-gsc: Make system PM callbacks available for CONFIG_PM_SLEEP

So, do you want to deal with it or you prefer me?

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/12] exynos-gsc: Make runtime PM callbacks available for CONFIG_PM

[Ulf Hansson]

On 10 November 2016 at 04:44, kbuild test robot  wrote:
> Hi Ulf,
>
> [auto build test ERROR on linuxtv-media/master]
> [also build test ERROR on v4.9-rc4 next-20161109]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Marek-Szyprowski/media-Exynos-GScaller-driver-fixes/20161110-48
> base:   git://linuxtv.org/media_tree.git master
> config: openrisc-allyesconfig (attached as .config)
> compiler: or32-linux-gcc (GCC) 4.5.1-or32-1.0rc1
> reproduce:
> wget 
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
>  -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=openrisc
>
> Note: the 
> linux-review/Marek-Szyprowski/media-Exynos-GScaller-driver-fixes/20161110-48
>  HEAD 92b20676ac75659d1ea1d83b00e8028f45ea84e9 builds fine.
>   It only hurts bisectibility.
>
> All errors (new ones prefixed by >>):
>
>drivers/media/platform/exynos-gsc/gsc-core.c: In function 'gsc_resume':
>>> drivers/media/platform/exynos-gsc/gsc-core.c:1183:3: error: implicit 
>>> declaration of function 'gsc_runtime_resume'
>drivers/media/platform/exynos-gsc/gsc-core.c: In function 'gsc_suspend':
>>> drivers/media/platform/exynos-gsc/gsc-core.c:1198:3: error: implicit 
>>> declaration of function 'gsc_runtime_suspend'
>

Marek, to avoid the bisectibility issue, we could squash patch 4/12 with 6/12.

Do you want to do it, or you prefer me to re-spin the series?

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/12] exynos-gsc: Simplify system PM even more

[Ulf Hansson]

On 9 November 2016 at 15:23, Marek Szyprowski <m.szyprow...@samsung.com> wrote:
> System pm callbacks only ensures that device is runtime suspended/resumed,
> so remove them and use generic pm_runtime_force_suspend/resume helper.
>
> Signed-off-by: Marek Szyprowski <m.szyprow...@samsung.com>

Reviewed-by: Ulf Hansson <ulf.hans...@linaro.org>

Kind regards
Uffe

> ---
>  drivers/media/platform/exynos-gsc/gsc-core.c | 21 ++---
>  1 file changed, 2 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
> b/drivers/media/platform/exynos-gsc/gsc-core.c
> index 4859727..1e8b216 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c
> @@ -1166,26 +1166,9 @@ static int gsc_runtime_suspend(struct device *dev)
>  }
>  #endif
>
> -#ifdef CONFIG_PM_SLEEP
> -static int gsc_resume(struct device *dev)
> -{
> -   if (!pm_runtime_suspended(dev))
> -   return gsc_runtime_resume(dev);
> -
> -   return 0;
> -}
> -
> -static int gsc_suspend(struct device *dev)
> -{
> -   if (!pm_runtime_suspended(dev))
> -   return gsc_runtime_suspend(dev);
> -
> -   return 0;
> -}
> -#endif
> -
>  static const struct dev_pm_ops gsc_pm_ops = {
> -   SET_SYSTEM_SLEEP_PM_OPS(gsc_suspend, gsc_resume)
> +   SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +   pm_runtime_force_resume)
> SET_RUNTIME_PM_OPS(gsc_runtime_suspend, gsc_runtime_resume, NULL)
>  };
>
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/13] mmc: omap_hsmmc: No need to check DMA channel validity at module remove

[Ulf Hansson]

On 26 May 2015 at 15:25, Peter Ujfalusi peter.ujfal...@ti.com wrote:
 The driver will not probe without valid DMA channels so no need to check
 if they are valid when the module is removed.

 Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com
 CC: Ulf Hansson ulf.hans...@linaro.org

Acked-by: Ulf Hansson ulf.hans...@linaro.org

Kind regards
Uffe

 ---
  drivers/mmc/host/omap_hsmmc.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

 diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
 index 2cd828f42151..57bb85930f81 100644
 --- a/drivers/mmc/host/omap_hsmmc.c
 +++ b/drivers/mmc/host/omap_hsmmc.c
 @@ -2190,10 +2190,8 @@ static int omap_hsmmc_remove(struct platform_device 
 *pdev)
 if (host-use_reg)
 omap_hsmmc_reg_put(host);

 -   if (host-tx_chan)
 -   dma_release_channel(host-tx_chan);
 -   if (host-rx_chan)
 -   dma_release_channel(host-rx_chan);
 +   dma_release_channel(host-tx_chan);
 +   dma_release_channel(host-rx_chan);

 pm_runtime_put_sync(host-dev);
 pm_runtime_disable(host-dev);
 --
 2.3.5

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/13] mmc: omap_hsmmc: Support for deferred probing when requesting DMA channels

[Ulf Hansson]

On 26 May 2015 at 15:26, Peter Ujfalusi peter.ujfal...@ti.com wrote:
 Switch to use ma_request_slave_channel_compat_reason() to request the DMA

I guess it should be dma_request_slave_... huh, that was a long name. :-)

 channels. In case of error, return the error code we received including
 -EPROBE_DEFER

 Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com
 CC: Ulf Hansson ulf.hans...@linaro.org

With the minor change above.

Acked-by: Ulf Hansson ulf.hans...@linaro.org

Kind regards
Uffe

 ---
  drivers/mmc/host/omap_hsmmc.c | 22 ++
  1 file changed, 10 insertions(+), 12 deletions(-)

 diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
 index 57bb85930f81..d252478391ee 100644
 --- a/drivers/mmc/host/omap_hsmmc.c
 +++ b/drivers/mmc/host/omap_hsmmc.c
 @@ -2088,23 +2088,21 @@ static int omap_hsmmc_probe(struct platform_device 
 *pdev)
 dma_cap_zero(mask);
 dma_cap_set(DMA_SLAVE, mask);

 -   host-rx_chan =
 -   dma_request_slave_channel_compat(mask, omap_dma_filter_fn,
 -rx_req, pdev-dev, rx);
 +   host-rx_chan = dma_request_slave_channel_compat_reason(mask,
 +   omap_dma_filter_fn, rx_req, pdev-dev, 
 rx);

 -   if (!host-rx_chan) {
 +   if (IS_ERR(host-rx_chan)) {
 dev_err(mmc_dev(host-mmc), unable to obtain RX DMA engine 
 channel %u\n, rx_req);
 -   ret = -ENXIO;
 +   ret = PTR_ERR(host-rx_chan);
 goto err_irq;
 }

 -   host-tx_chan =
 -   dma_request_slave_channel_compat(mask, omap_dma_filter_fn,
 -tx_req, pdev-dev, tx);
 +   host-tx_chan = dma_request_slave_channel_compat_reason(mask,
 +   omap_dma_filter_fn, tx_req, pdev-dev, 
 tx);

 -   if (!host-tx_chan) {
 +   if (IS_ERR(host-tx_chan)) {
 dev_err(mmc_dev(host-mmc), unable to obtain TX DMA engine 
 channel %u\n, tx_req);
 -   ret = -ENXIO;
 +   ret = PTR_ERR(host-tx_chan);
 goto err_irq;
 }

 @@ -2166,9 +2164,9 @@ err_slot_name:
 if (host-use_reg)
 omap_hsmmc_reg_put(host);
  err_irq:
 -   if (host-tx_chan)
 +   if (!IS_ERR_OR_NULL(host-tx_chan))
 dma_release_channel(host-tx_chan);
 -   if (host-rx_chan)
 +   if (!IS_ERR_OR_NULL(host-rx_chan))
 dma_release_channel(host-rx_chan);
 pm_runtime_put_sync(host-dev);
 pm_runtime_disable(host-dev);
 --
 2.3.5

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/13] mmc: davinci_mmc: Support for deferred probing when requesting DMA channels

[Ulf Hansson]

On 26 May 2015 at 15:26, Peter Ujfalusi peter.ujfal...@ti.com wrote:
 Switch to use ma_request_slave_channel_compat_reason() to request the DMA
 channels. Only fall back to pio mode if the error code returned is not
 -EPROBE_DEFER, otherwise return from the probe with the -EPROBE_DEFER.

 Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com
 CC: Ulf Hansson ulf.hans...@linaro.org

Acked-by: Ulf Hansson ulf.hans...@linaro.org

Kind regards
Uffe

 ---
  drivers/mmc/host/davinci_mmc.c | 26 +++---
  1 file changed, 15 insertions(+), 11 deletions(-)

 diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
 index b2b3f8bbfd8c..df81e4e2f662 100644
 --- a/drivers/mmc/host/davinci_mmc.c
 +++ b/drivers/mmc/host/davinci_mmc.c
 @@ -530,20 +530,20 @@ static int __init davinci_acquire_dma_channels(struct 
 mmc_davinci_host *host)
 dma_cap_zero(mask);
 dma_cap_set(DMA_SLAVE, mask);

 -   host-dma_tx =
 -   dma_request_slave_channel_compat(mask, edma_filter_fn,
 -   host-txdma, mmc_dev(host-mmc), tx);
 -   if (!host-dma_tx) {
 +   host-dma_tx = dma_request_slave_channel_compat_reason(mask,
 +   edma_filter_fn, host-txdma,
 +   mmc_dev(host-mmc), tx);
 +   if (IS_ERR(host-dma_tx)) {
 dev_err(mmc_dev(host-mmc), Can't get dma_tx channel\n);
 -   return -ENODEV;
 +   return PTR_ERR(host-dma_tx);
 }

 -   host-dma_rx =
 -   dma_request_slave_channel_compat(mask, edma_filter_fn,
 -   host-rxdma, mmc_dev(host-mmc), rx);
 -   if (!host-dma_rx) {
 +   host-dma_rx = dma_request_slave_channel_compat_reason(mask,
 +   edma_filter_fn, host-rxdma,
 +   mmc_dev(host-mmc), rx);
 +   if (IS_ERR(host-dma_rx)) {
 dev_err(mmc_dev(host-mmc), Can't get dma_rx channel\n);
 -   r = -ENODEV;
 +   r = PTR_ERR(host-dma_rx);
 goto free_master_write;
 }

 @@ -1307,8 +1307,12 @@ static int __init davinci_mmcsd_probe(struct 
 platform_device *pdev)
 host-mmc_irq = irq;
 host-sdio_irq = platform_get_irq(pdev, 1);

 -   if (host-use_dma  davinci_acquire_dma_channels(host) != 0)
 +   if (host-use_dma) {
 +   ret = davinci_acquire_dma_channels(host);
 +   if (ret == -EPROBE_DEFER)
 +   goto out;
 host-use_dma = 0;
 +   }

 /* REVISIT:  someday, support IRQ-driven card detection.  */
 mmc-caps |= MMC_CAP_NEEDS_POLL;
 --
 2.3.5

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 0/8] [media] exynos-gsc: Fixup PM support

[Ulf Hansson]

On 19 January 2015 at 14:22, Ulf Hansson ulf.hans...@linaro.org wrote:
 Changes in v2:
 - Rebase patches.
 - Adapt to changes for the PM core. Especially, the Kconfig option for
   CONFIG_PM_RUNTIME has been removed and the runtime PM core is now
   build for CONFIG_PM.

 This patchset fixup the PM support and adds some minor improvements to
 potentially save some more power at runtime PM suspend.


 Ulf Hansson (8):
   [media] exynos-gsc: Simplify clock management
   [media] exynos-gsc: Convert gsc_m2m_resume() from int to void
   [media] exynos-gsc: Make driver functional when CONFIG_PM is unset
   [media] exynos-gsc: Make runtime PM callbacks available for CONFIG_PM
   [media] exynos-gsc: Fixup clock management at -remove()
   [media] exynos-gsc: Do full clock gating at runtime PM suspend
   [media] exynos-gsc: Make system PM callbacks available for
 CONFIG_PM_SLEEP
   [media] exynos-gsc: Simplify system PM

  drivers/media/platform/exynos-gsc/gsc-core.c | 183 
 +++
  drivers/media/platform/exynos-gsc/gsc-core.h |   3 -
  2 files changed, 72 insertions(+), 114 deletions(-)

 --
 1.9.1


I guess you guys have been busy, but it would be nice to get some
feedback of these patches.

Kind regards
Uffe
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 2/8] [media] exynos-gsc: Convert gsc_m2m_resume() from int to void

[Ulf Hansson]

Since gsc_m2m_resume() always returns 0, convert it to a void instead.

Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index bd769d4..1865738 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1025,7 +1025,7 @@ static int gsc_m2m_suspend(struct gsc_dev *gsc)
return timeout == 0 ? -EAGAIN : 0;
 }
 
-static int gsc_m2m_resume(struct gsc_dev *gsc)
+static void gsc_m2m_resume(struct gsc_dev *gsc)
 {
struct gsc_ctx *ctx;
unsigned long flags;
@@ -1038,8 +1038,6 @@ static int gsc_m2m_resume(struct gsc_dev *gsc)
 
if (test_and_clear_bit(ST_M2M_SUSPENDED, gsc-state))
gsc_m2m_job_finish(ctx, VB2_BUF_STATE_ERROR);
-
-   return 0;
 }
 
 static int gsc_probe(struct platform_device *pdev)
@@ -1168,8 +1166,9 @@ static int gsc_runtime_resume(struct device *dev)
 
gsc_hw_set_sw_reset(gsc);
gsc_wait_reset(gsc);
+   gsc_m2m_resume(gsc);
 
-   return gsc_m2m_resume(gsc);
+   return 0;
 }
 
 static int gsc_runtime_suspend(struct device *dev)
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 5/8] [media] exynos-gsc: Fixup clock management at -remove()

[Ulf Hansson]

To make sure the clock is fully gated in -remove(), we first need to
to bring the device into full power by invoking pm_runtime_get_sync().

Then, let's both unprepare and disable the clock.

Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index e84bc35..5d3cfe8 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1104,12 +1104,15 @@ static int gsc_remove(struct platform_device *pdev)
 {
struct gsc_dev *gsc = platform_get_drvdata(pdev);
 
+   pm_runtime_get_sync(pdev-dev);
+
gsc_unregister_m2m_device(gsc);
v4l2_device_unregister(gsc-v4l2_dev);
-
vb2_dma_contig_cleanup_ctx(gsc-alloc_ctx);
+   clk_disable_unprepare(gsc-clock);
+
pm_runtime_disable(pdev-dev);
-   clk_unprepare(gsc-clock);
+   pm_runtime_put_noidle(pdev-dev);
 
dev_dbg(pdev-dev, %s driver unloaded\n, pdev-name);
return 0;
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 1/8] [media] exynos-gsc: Simplify clock management

[Ulf Hansson]

Instead of having separate functions that fecthes, prepares and
unprepares the clock, let's encapsulate this code into -probe().

This makes error handling easier and decreases the lines of code.

Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 49 
 1 file changed, 14 insertions(+), 35 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index fd2891c..bd769d4 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1003,36 +1003,6 @@ static void *gsc_get_drv_data(struct platform_device 
*pdev)
return driver_data;
 }
 
-static void gsc_clk_put(struct gsc_dev *gsc)
-{
-   if (!IS_ERR(gsc-clock))
-   clk_unprepare(gsc-clock);
-}
-
-static int gsc_clk_get(struct gsc_dev *gsc)
-{
-   int ret;
-
-   dev_dbg(gsc-pdev-dev, gsc_clk_get Called\n);
-
-   gsc-clock = devm_clk_get(gsc-pdev-dev, GSC_CLOCK_GATE_NAME);
-   if (IS_ERR(gsc-clock)) {
-   dev_err(gsc-pdev-dev, failed to get clock~~~: %s\n,
-   GSC_CLOCK_GATE_NAME);
-   return PTR_ERR(gsc-clock);
-   }
-
-   ret = clk_prepare(gsc-clock);
-   if (ret  0) {
-   dev_err(gsc-pdev-dev, clock prepare failed for clock: %s\n,
-   GSC_CLOCK_GATE_NAME);
-   gsc-clock = ERR_PTR(-EINVAL);
-   return ret;
-   }
-
-   return 0;
-}
-
 static int gsc_m2m_suspend(struct gsc_dev *gsc)
 {
unsigned long flags;
@@ -1101,7 +1071,6 @@ static int gsc_probe(struct platform_device *pdev)
init_waitqueue_head(gsc-irq_queue);
spin_lock_init(gsc-slock);
mutex_init(gsc-lock);
-   gsc-clock = ERR_PTR(-EINVAL);
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
gsc-regs = devm_ioremap_resource(dev, res);
@@ -1114,9 +1083,19 @@ static int gsc_probe(struct platform_device *pdev)
return -ENXIO;
}
 
-   ret = gsc_clk_get(gsc);
-   if (ret)
+   gsc-clock = devm_clk_get(dev, GSC_CLOCK_GATE_NAME);
+   if (IS_ERR(gsc-clock)) {
+   dev_err(dev, failed to get clock~~~: %s\n,
+   GSC_CLOCK_GATE_NAME);
+   return PTR_ERR(gsc-clock);
+   }
+
+   ret = clk_prepare(gsc-clock);
+   if (ret) {
+   dev_err(gsc-pdev-dev, clock prepare failed for clock: %s\n,
+   GSC_CLOCK_GATE_NAME);
return ret;
+   }
 
ret = devm_request_irq(dev, res-start, gsc_irq_handler,
0, pdev-name, gsc);
@@ -1157,7 +1136,7 @@ err_m2m:
 err_v4l2:
v4l2_device_unregister(gsc-v4l2_dev);
 err_clk:
-   gsc_clk_put(gsc);
+   clk_unprepare(gsc-clock);
return ret;
 }
 
@@ -1170,7 +1149,7 @@ static int gsc_remove(struct platform_device *pdev)
 
vb2_dma_contig_cleanup_ctx(gsc-alloc_ctx);
pm_runtime_disable(pdev-dev);
-   gsc_clk_put(gsc);
+   clk_unprepare(gsc-clock);
 
dev_dbg(pdev-dev, %s driver unloaded\n, pdev-name);
return 0;
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 3/8] [media] exynos-gsc: Make driver functional when CONFIG_PM is unset

[Ulf Hansson]

The driver depended on CONFIG_PM to be functional. Let's remove that
dependency, by enable the runtime PM resourses during -probe() and
update the device's runtime PM status to reflect this.

Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index 1865738..532daa8 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1088,7 +1088,7 @@ static int gsc_probe(struct platform_device *pdev)
return PTR_ERR(gsc-clock);
}
 
-   ret = clk_prepare(gsc-clock);
+   ret = clk_prepare_enable(gsc-clock);
if (ret) {
dev_err(gsc-pdev-dev, clock prepare failed for clock: %s\n,
GSC_CLOCK_GATE_NAME);
@@ -,30 +,29 @@ static int gsc_probe(struct platform_device *pdev)
goto err_v4l2;
 
platform_set_drvdata(pdev, gsc);
-   pm_runtime_enable(dev);
-   ret = pm_runtime_get_sync(pdev-dev);
-   if (ret  0)
-   goto err_m2m;
+
+   gsc_hw_set_sw_reset(gsc);
+   gsc_wait_reset(gsc);
 
/* Initialize continious memory allocator */
gsc-alloc_ctx = vb2_dma_contig_init_ctx(dev);
if (IS_ERR(gsc-alloc_ctx)) {
ret = PTR_ERR(gsc-alloc_ctx);
-   goto err_pm;
+   goto err_m2m;
}
 
dev_dbg(dev, gsc-%d registered successfully\n, gsc-id);
 
-   pm_runtime_put(dev);
+   pm_runtime_set_active(dev);
+   pm_runtime_enable(dev);
+
return 0;
-err_pm:
-   pm_runtime_put(dev);
 err_m2m:
gsc_unregister_m2m_device(gsc);
 err_v4l2:
v4l2_device_unregister(gsc-v4l2_dev);
 err_clk:
-   clk_unprepare(gsc-clock);
+   clk_disable_unprepare(gsc-clock);
return ret;
 }
 
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 0/8] [media] exynos-gsc: Fixup PM support

[Ulf Hansson]

Changes in v2:
- Rebase patches.
- Adapt to changes for the PM core. Especially, the Kconfig option for
  CONFIG_PM_RUNTIME has been removed and the runtime PM core is now
  build for CONFIG_PM.

This patchset fixup the PM support and adds some minor improvements to
potentially save some more power at runtime PM suspend.


Ulf Hansson (8):
  [media] exynos-gsc: Simplify clock management
  [media] exynos-gsc: Convert gsc_m2m_resume() from int to void
  [media] exynos-gsc: Make driver functional when CONFIG_PM is unset
  [media] exynos-gsc: Make runtime PM callbacks available for CONFIG_PM
  [media] exynos-gsc: Fixup clock management at -remove()
  [media] exynos-gsc: Do full clock gating at runtime PM suspend
  [media] exynos-gsc: Make system PM callbacks available for
CONFIG_PM_SLEEP
  [media] exynos-gsc: Simplify system PM

 drivers/media/platform/exynos-gsc/gsc-core.c | 183 +++
 drivers/media/platform/exynos-gsc/gsc-core.h |   3 -
 2 files changed, 72 insertions(+), 114 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 4/8] [media] exynos-gsc: Make runtime PM callbacks available for CONFIG_PM

[Ulf Hansson]

There are no need to set up the runtime PM callbacks unless they are
being used. It also causes compiler warnings about unused functions.

Silence the warnings by making them available for CONFIG_PM.

Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 79 ++--
 1 file changed, 40 insertions(+), 39 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index 532daa8..e84bc35 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1003,43 +1003,6 @@ static void *gsc_get_drv_data(struct platform_device 
*pdev)
return driver_data;
 }
 
-static int gsc_m2m_suspend(struct gsc_dev *gsc)
-{
-   unsigned long flags;
-   int timeout;
-
-   spin_lock_irqsave(gsc-slock, flags);
-   if (!gsc_m2m_pending(gsc)) {
-   spin_unlock_irqrestore(gsc-slock, flags);
-   return 0;
-   }
-   clear_bit(ST_M2M_SUSPENDED, gsc-state);
-   set_bit(ST_M2M_SUSPENDING, gsc-state);
-   spin_unlock_irqrestore(gsc-slock, flags);
-
-   timeout = wait_event_timeout(gsc-irq_queue,
-test_bit(ST_M2M_SUSPENDED, gsc-state),
-GSC_SHUTDOWN_TIMEOUT);
-
-   clear_bit(ST_M2M_SUSPENDING, gsc-state);
-   return timeout == 0 ? -EAGAIN : 0;
-}
-
-static void gsc_m2m_resume(struct gsc_dev *gsc)
-{
-   struct gsc_ctx *ctx;
-   unsigned long flags;
-
-   spin_lock_irqsave(gsc-slock, flags);
-   /* Clear for full H/W setup in first run after resume */
-   ctx = gsc-m2m.ctx;
-   gsc-m2m.ctx = NULL;
-   spin_unlock_irqrestore(gsc-slock, flags);
-
-   if (test_and_clear_bit(ST_M2M_SUSPENDED, gsc-state))
-   gsc_m2m_job_finish(ctx, VB2_BUF_STATE_ERROR);
-}
-
 static int gsc_probe(struct platform_device *pdev)
 {
struct gsc_dev *gsc;
@@ -1152,6 +1115,44 @@ static int gsc_remove(struct platform_device *pdev)
return 0;
 }
 
+#ifdef CONFIG_PM
+static int gsc_m2m_suspend(struct gsc_dev *gsc)
+{
+   unsigned long flags;
+   int timeout;
+
+   spin_lock_irqsave(gsc-slock, flags);
+   if (!gsc_m2m_pending(gsc)) {
+   spin_unlock_irqrestore(gsc-slock, flags);
+   return 0;
+   }
+   clear_bit(ST_M2M_SUSPENDED, gsc-state);
+   set_bit(ST_M2M_SUSPENDING, gsc-state);
+   spin_unlock_irqrestore(gsc-slock, flags);
+
+   timeout = wait_event_timeout(gsc-irq_queue,
+test_bit(ST_M2M_SUSPENDED, gsc-state),
+GSC_SHUTDOWN_TIMEOUT);
+
+   clear_bit(ST_M2M_SUSPENDING, gsc-state);
+   return timeout == 0 ? -EAGAIN : 0;
+}
+
+static void gsc_m2m_resume(struct gsc_dev *gsc)
+{
+   struct gsc_ctx *ctx;
+   unsigned long flags;
+
+   spin_lock_irqsave(gsc-slock, flags);
+   /* Clear for full H/W setup in first run after resume */
+   ctx = gsc-m2m.ctx;
+   gsc-m2m.ctx = NULL;
+   spin_unlock_irqrestore(gsc-slock, flags);
+
+   if (test_and_clear_bit(ST_M2M_SUSPENDED, gsc-state))
+   gsc_m2m_job_finish(ctx, VB2_BUF_STATE_ERROR);
+}
+
 static int gsc_runtime_resume(struct device *dev)
 {
struct gsc_dev *gsc = dev_get_drvdata(dev);
@@ -1182,6 +1183,7 @@ static int gsc_runtime_suspend(struct device *dev)
pr_debug(gsc%d: state: 0x%lx, gsc-id, gsc-state);
return ret;
 }
+#endif
 
 static int gsc_resume(struct device *dev)
 {
@@ -1223,8 +1225,7 @@ static int gsc_suspend(struct device *dev)
 static const struct dev_pm_ops gsc_pm_ops = {
.suspend= gsc_suspend,
.resume = gsc_resume,
-   .runtime_suspend= gsc_runtime_suspend,
-   .runtime_resume = gsc_runtime_resume,
+   SET_RUNTIME_PM_OPS(gsc_runtime_suspend, gsc_runtime_resume, NULL)
 };
 
 static struct platform_driver gsc_driver = {
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 7/8] [media] exynos-gsc: Make system PM callbacks available for CONFIG_PM_SLEEP

[Ulf Hansson]

There are no need to set up the system PM callbacks unless they are
being used. It also causes compiler warnings about unused functions.

Silence the warnings by making them available for CONFIG_PM_SLEEP.

Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index 0b126eb..194f9fc 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1188,6 +1188,7 @@ static int gsc_runtime_suspend(struct device *dev)
 }
 #endif
 
+#ifdef CONFIG_PM_SLEEP
 static int gsc_resume(struct device *dev)
 {
struct gsc_dev *gsc = dev_get_drvdata(dev);
@@ -1224,10 +1225,10 @@ static int gsc_suspend(struct device *dev)
 
return 0;
 }
+#endif
 
 static const struct dev_pm_ops gsc_pm_ops = {
-   .suspend= gsc_suspend,
-   .resume = gsc_resume,
+   SET_SYSTEM_SLEEP_PM_OPS(gsc_suspend, gsc_resume)
SET_RUNTIME_PM_OPS(gsc_runtime_suspend, gsc_runtime_resume, NULL)
 };
 
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 8/8] [media] exynos-gsc: Simplify system PM

[Ulf Hansson]

It's not needed to keep a local flag about the current system PM state.
Let's just remove that code and the corresponding debug print.

Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 21 -
 drivers/media/platform/exynos-gsc/gsc-core.h |  3 ---
 2 files changed, 24 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index 194f9fc..71b227c 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1191,20 +1191,6 @@ static int gsc_runtime_suspend(struct device *dev)
 #ifdef CONFIG_PM_SLEEP
 static int gsc_resume(struct device *dev)
 {
-   struct gsc_dev *gsc = dev_get_drvdata(dev);
-   unsigned long flags;
-
-   pr_debug(gsc%d: state: 0x%lx, gsc-id, gsc-state);
-
-   /* Do not resume if the device was idle before system suspend */
-   spin_lock_irqsave(gsc-slock, flags);
-   if (!test_and_clear_bit(ST_SUSPEND, gsc-state) ||
-   !gsc_m2m_opened(gsc)) {
-   spin_unlock_irqrestore(gsc-slock, flags);
-   return 0;
-   }
-   spin_unlock_irqrestore(gsc-slock, flags);
-
if (!pm_runtime_suspended(dev))
return gsc_runtime_resume(dev);
 
@@ -1213,13 +1199,6 @@ static int gsc_resume(struct device *dev)
 
 static int gsc_suspend(struct device *dev)
 {
-   struct gsc_dev *gsc = dev_get_drvdata(dev);
-
-   pr_debug(gsc%d: state: 0x%lx, gsc-id, gsc-state);
-
-   if (test_and_set_bit(ST_SUSPEND, gsc-state))
-   return 0;
-
if (!pm_runtime_suspended(dev))
return gsc_runtime_suspend(dev);
 
diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h 
b/drivers/media/platform/exynos-gsc/gsc-core.h
index fa572aa..2f62271 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.h
+++ b/drivers/media/platform/exynos-gsc/gsc-core.h
@@ -48,9 +48,6 @@
 #defineGSC_CTX_ABORT   (1  7)
 
 enum gsc_dev_flags {
-   /* for global */
-   ST_SUSPEND,
-
/* for m2m node */
ST_M2M_OPEN,
ST_M2M_RUN,
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 6/8] [media] exynos-gsc: Do full clock gating at runtime PM suspend

[Ulf Hansson]

To potentially save more power in runtime PM suspend state, let's also
prepare/unprepare the clock from the runtime PM callbacks.

Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index 5d3cfe8..0b126eb 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1163,7 +1163,7 @@ static int gsc_runtime_resume(struct device *dev)
 
pr_debug(gsc%d: state: 0x%lx, gsc-id, gsc-state);
 
-   ret = clk_enable(gsc-clock);
+   ret = clk_prepare_enable(gsc-clock);
if (ret)
return ret;
 
@@ -1181,7 +1181,7 @@ static int gsc_runtime_suspend(struct device *dev)
 
ret = gsc_m2m_suspend(gsc);
if (!ret)
-   clk_disable(gsc-clock);
+   clk_disable_unprepare(gsc-clock);
 
pr_debug(gsc%d: state: 0x%lx, gsc-id, gsc-state);
return ret;
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] [media] coda: Improve runtime PM support

[Ulf Hansson]

On 23 October 2014 13:57, Philipp Zabel p.za...@pengutronix.de wrote:
 Hi Ulf,

 Am Montag, den 22.09.2014, 20:44 +0200 schrieb Ulf Hansson:
 On 22 September 2014 18:05, Philipp Zabel p.za...@pengutronix.de wrote:
  From: Ulf Hansson ulf.hans...@linaro.org
 
  For several reasons it's good practice to leave devices in runtime PM
  active state while those have been probed.
 
  In this cases we also want to prevent the device from going inactive,
  until the firmware has been completely installed, especially when using
  a PM domain.
 
  Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
  Signed-off-by: Philipp Zabel p.za...@pengutronix.de

 Thanks for moving this to the next version, I have been a bit busy the
 last week.

 Changes looking good!

 If I load the coda module on v3.18-rc1 with the GPC power domain patch
 applied (at this point the power domain is disabled), the domain's
 poweron callback is never called. It does work tough if I switch back to
 explicitly calling pm_runtime_get_sync:

 diff --git a/drivers/media/platform/coda/coda-common.c 
 b/drivers/media/platform/coda/coda-common.c
 index ac71e11..5421969 100644
 --- a/drivers/media/platform/coda/coda-common.c
 +++ b/drivers/media/platform/coda/coda-common.c
 @@ -2393,9 +2393,8 @@ static int coda_probe(struct platform_device *pdev)
  * coda_fw_callback regardless of whether CONFIG_PM_RUNTIME is
  * enabled or whether the device is associated with a PM domain.
  */
 -   pm_runtime_get_noresume(pdev-dev);
 -   pm_runtime_set_active(pdev-dev);
 pm_runtime_enable(pdev-dev);
 +   pm_runtime_get_sync(pdev-dev);

 return coda_firmware_request(dev);
  }

 At what point is the pm domain supposed to be enabled when I load the
 module?

Hi Philipp,

The PM domain shall be powered on prior your driver starts probing.
This is a common problem when using the generic PM domain. The
workaround, which is causing other issues, is a pm_runtime_get_sync().

Now, could you please try to apply the below patchset, that should
hopefully fix your issue:

[PATCH v3 0/9] PM / Domains: Fix race conditions during boot
http://marc.info/?l=linux-pmm=141320895122707w=2

Kind regards
Uffe
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/7] [media] exynos-gsc: Fixup PM support

[Ulf Hansson]

This patchset fixup the PM support and adds some minor improvements to
potentially save some more power at runtime PM suspend.

Some background to this patchset, which are related to the generic PM domain:
http://marc.info/?l=linux-pmm=141217452218592w=2
http://marc.info/?t=141217462200011r=1w=2

The conserns from the above discussions are intended to be solved by a reworked
approach for the generic PM domain, link below.
http://marc.info/?l=linux-pmm=141320895122707w=2

Ulf Hansson (7):
  [media] exynos-gsc: Simplify clock management
  [media] exynos-gsc: Convert gsc_m2m_resume() from int to void
  [media] exynos-gsc: Make driver functional without CONFIG_PM_RUNTIME
  [media] exynos-gsc: Make runtime PM callbacks available for CONFIG_PM
  [media] exynos-gsc: Fixup system PM
  [media] exynos-gsc: Fixup clock management at -remove()
  [media] exynos-gsc: Do full clock gating at runtime PM suspend

 drivers/media/platform/exynos-gsc/gsc-core.c | 127 ---
 drivers/media/platform/exynos-gsc/gsc-core.h |   3 -
 2 files changed, 36 insertions(+), 94 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/7] [media] exynos-gsc: Do full clock gating at runtime PM suspend

[Ulf Hansson]

To potentially save more power in runtime PM suspend state, let's also
prepare/unprepare the clock from the runtime PM callbacks.

Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index e48aefa..e90ba09 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1161,7 +1161,7 @@ static int gsc_runtime_resume(struct device *dev)
 
pr_debug(gsc%d: state: 0x%lx, gsc-id, gsc-state);
 
-   ret = clk_enable(gsc-clock);
+   ret = clk_prepare_enable(gsc-clock);
if (ret)
return ret;
 
@@ -1179,7 +1179,7 @@ static int gsc_runtime_suspend(struct device *dev)
 
ret = gsc_m2m_suspend(gsc);
if (!ret)
-   clk_disable(gsc-clock);
+   clk_disable_unprepare(gsc-clock);
 
pr_debug(gsc%d: state: 0x%lx, gsc-id, gsc-state);
return ret;
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/7] [media] exynos-gsc: Make runtime PM callbacks available for CONFIG_PM

[Ulf Hansson]

There are no need to set up the runtime PM callbacks unless they are
being used. Let's make them available for CONFIG_PM.

Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index c3a050e..361a807 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1150,6 +1150,7 @@ static int gsc_remove(struct platform_device *pdev)
return 0;
 }
 
+#ifdef CONFIG_PM
 static int gsc_runtime_resume(struct device *dev)
 {
struct gsc_dev *gsc = dev_get_drvdata(dev);
@@ -1180,6 +1181,7 @@ static int gsc_runtime_suspend(struct device *dev)
pr_debug(gsc%d: state: 0x%lx, gsc-id, gsc-state);
return ret;
 }
+#endif
 
 static int gsc_resume(struct device *dev)
 {
@@ -1221,8 +1223,7 @@ static int gsc_suspend(struct device *dev)
 static const struct dev_pm_ops gsc_pm_ops = {
.suspend= gsc_suspend,
.resume = gsc_resume,
-   .runtime_suspend= gsc_runtime_suspend,
-   .runtime_resume = gsc_runtime_resume,
+   SET_PM_RUNTIME_PM_OPS(gsc_runtime_suspend, gsc_runtime_resume, NULL)
 };
 
 static struct platform_driver gsc_driver = {
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/7] [media] exynos-gsc: Convert gsc_m2m_resume() from int to void

[Ulf Hansson]

Since gsc_m2m_resume() always returns 0, convert it into void instead.

Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index 3fca4fd..13d0226 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1022,7 +1022,7 @@ static int gsc_m2m_suspend(struct gsc_dev *gsc)
return timeout == 0 ? -EAGAIN : 0;
 }
 
-static int gsc_m2m_resume(struct gsc_dev *gsc)
+static void gsc_m2m_resume(struct gsc_dev *gsc)
 {
struct gsc_ctx *ctx;
unsigned long flags;
@@ -1035,8 +1035,6 @@ static int gsc_m2m_resume(struct gsc_dev *gsc)
 
if (test_and_clear_bit(ST_M2M_SUSPENDED, gsc-state))
gsc_m2m_job_finish(ctx, VB2_BUF_STATE_ERROR);
-
-   return 0;
 }
 
 static int gsc_probe(struct platform_device *pdev)
@@ -1165,8 +1163,9 @@ static int gsc_runtime_resume(struct device *dev)
 
gsc_hw_set_sw_reset(gsc);
gsc_wait_reset(gsc);
+   gsc_m2m_resume(gsc);
 
-   return gsc_m2m_resume(gsc);
+   return 0;
 }
 
 static int gsc_runtime_suspend(struct device *dev)
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/7] [media] exynos-gsc: Fixup system PM

[Ulf Hansson]

We had several issues with the system PM support.
1) It were depending on CONFIG_PM_RUNTIME.
2) It unnecessarily tracked the suspend state in a flag.
3) If userspace through sysfs prevents runtime PM operations, could
cause the device to stay in low power after a system PM resume, which
is not reflected properly.

Solve all the above issues by using pm_runtime_force_suspend|resume() as
the system PM callbacks.

Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 41 ++--
 drivers/media/platform/exynos-gsc/gsc-core.h |  3 --
 2 files changed, 2 insertions(+), 42 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index 361a807..1b9f3d7 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1183,46 +1183,9 @@ static int gsc_runtime_suspend(struct device *dev)
 }
 #endif
 
-static int gsc_resume(struct device *dev)
-{
-   struct gsc_dev *gsc = dev_get_drvdata(dev);
-   unsigned long flags;
-
-   pr_debug(gsc%d: state: 0x%lx, gsc-id, gsc-state);
-
-   /* Do not resume if the device was idle before system suspend */
-   spin_lock_irqsave(gsc-slock, flags);
-   if (!test_and_clear_bit(ST_SUSPEND, gsc-state) ||
-   !gsc_m2m_opened(gsc)) {
-   spin_unlock_irqrestore(gsc-slock, flags);
-   return 0;
-   }
-   spin_unlock_irqrestore(gsc-slock, flags);
-
-   if (!pm_runtime_suspended(dev))
-   return gsc_runtime_resume(dev);
-
-   return 0;
-}
-
-static int gsc_suspend(struct device *dev)
-{
-   struct gsc_dev *gsc = dev_get_drvdata(dev);
-
-   pr_debug(gsc%d: state: 0x%lx, gsc-id, gsc-state);
-
-   if (test_and_set_bit(ST_SUSPEND, gsc-state))
-   return 0;
-
-   if (!pm_runtime_suspended(dev))
-   return gsc_runtime_suspend(dev);
-
-   return 0;
-}
-
 static const struct dev_pm_ops gsc_pm_ops = {
-   .suspend= gsc_suspend,
-   .resume = gsc_resume,
+   SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+   pm_runtime_force_resume)
SET_PM_RUNTIME_PM_OPS(gsc_runtime_suspend, gsc_runtime_resume, NULL)
 };
 
diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h 
b/drivers/media/platform/exynos-gsc/gsc-core.h
index ef0a656..2dbaa20 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.h
+++ b/drivers/media/platform/exynos-gsc/gsc-core.h
@@ -48,9 +48,6 @@
 #defineGSC_CTX_ABORT   (1  7)
 
 enum gsc_dev_flags {
-   /* for global */
-   ST_SUSPEND,
-
/* for m2m node */
ST_M2M_OPEN,
ST_M2M_RUN,
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/7] [media] exynos-gsc: Make driver functional without CONFIG_PM_RUNTIME

[Ulf Hansson]

The driver depended on CONFIG_PM_RUNTIME to be functional, which isn't
necessary.

The solution to the above is to enable all runtime PM resourses during
probe and update the device's runtime PM status to active.

Since driver core invokes pm_request_idle() after -probe(), unused gsc
devices will be runtime PM suspended and thus we will still benefit
from using CONFIG_PM_RUNTIME.

Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index 13d0226..c3a050e 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1085,7 +1085,7 @@ static int gsc_probe(struct platform_device *pdev)
return PTR_ERR(gsc-clock);
}
 
-   ret = clk_prepare(gsc-clock);
+   ret = clk_prepare_enable(gsc-clock);
if (ret) {
dev_err(gsc-pdev-dev, clock prepare failed for clock: %s\n,
GSC_CLOCK_GATE_NAME);
@@ -1108,30 +1108,30 @@ static int gsc_probe(struct platform_device *pdev)
goto err_v4l2;
 
platform_set_drvdata(pdev, gsc);
-   pm_runtime_enable(dev);
-   ret = pm_runtime_get_sync(pdev-dev);
-   if (ret  0)
-   goto err_m2m;
+
+   gsc_hw_set_sw_reset(gsc);
+   gsc_wait_reset(gsc);
+   gsc_m2m_resume(gsc);
 
/* Initialize continious memory allocator */
gsc-alloc_ctx = vb2_dma_contig_init_ctx(dev);
if (IS_ERR(gsc-alloc_ctx)) {
ret = PTR_ERR(gsc-alloc_ctx);
-   goto err_pm;
+   goto err_m2m;
}
 
dev_dbg(dev, gsc-%d registered successfully\n, gsc-id);
 
-   pm_runtime_put(dev);
+   pm_runtime_set_active(dev);
+   pm_runtime_enable(dev);
+
return 0;
-err_pm:
-   pm_runtime_put(dev);
 err_m2m:
gsc_unregister_m2m_device(gsc);
 err_v4l2:
v4l2_device_unregister(gsc-v4l2_dev);
 err_clk:
-   clk_unprepare(gsc-clock);
+   clk_disable_unprepare(gsc-clock);
return ret;
 }
 
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/7] [media] exynos-gsc: Fixup clock management at -remove()

[Ulf Hansson]

We want to make sure that the clock is fully gated after -remove(). To
do this, we need to bring the device into full power and not only
unprepare the clock, but also disable it.

Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index 1b9f3d7..e48aefa 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1139,12 +1139,15 @@ static int gsc_remove(struct platform_device *pdev)
 {
struct gsc_dev *gsc = platform_get_drvdata(pdev);
 
+   pm_runtime_get_sync(pdev-dev);
+
gsc_unregister_m2m_device(gsc);
v4l2_device_unregister(gsc-v4l2_dev);
-
vb2_dma_contig_cleanup_ctx(gsc-alloc_ctx);
+   clk_disable_unprepare(gsc-clock);
+
pm_runtime_disable(pdev-dev);
-   clk_unprepare(gsc-clock);
+   pm_runtime_put_noidle(pdev-dev);
 
dev_dbg(pdev-dev, %s driver unloaded\n, pdev-name);
return 0;
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/7] [media] exynos-gsc: Simplify clock management

[Ulf Hansson]

Instead of having separate functions that fecthes, prepares and
unprepares the clock, let's encapsulate this code into -probe().

This makes error handling easier and decreases the lines of code.

Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 49 
 1 file changed, 14 insertions(+), 35 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index b4c9f1d..3fca4fd 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1000,36 +1000,6 @@ static void *gsc_get_drv_data(struct platform_device 
*pdev)
return driver_data;
 }
 
-static void gsc_clk_put(struct gsc_dev *gsc)
-{
-   if (!IS_ERR(gsc-clock))
-   clk_unprepare(gsc-clock);
-}
-
-static int gsc_clk_get(struct gsc_dev *gsc)
-{
-   int ret;
-
-   dev_dbg(gsc-pdev-dev, gsc_clk_get Called\n);
-
-   gsc-clock = devm_clk_get(gsc-pdev-dev, GSC_CLOCK_GATE_NAME);
-   if (IS_ERR(gsc-clock)) {
-   dev_err(gsc-pdev-dev, failed to get clock~~~: %s\n,
-   GSC_CLOCK_GATE_NAME);
-   return PTR_ERR(gsc-clock);
-   }
-
-   ret = clk_prepare(gsc-clock);
-   if (ret  0) {
-   dev_err(gsc-pdev-dev, clock prepare failed for clock: %s\n,
-   GSC_CLOCK_GATE_NAME);
-   gsc-clock = ERR_PTR(-EINVAL);
-   return ret;
-   }
-
-   return 0;
-}
-
 static int gsc_m2m_suspend(struct gsc_dev *gsc)
 {
unsigned long flags;
@@ -1098,7 +1068,6 @@ static int gsc_probe(struct platform_device *pdev)
init_waitqueue_head(gsc-irq_queue);
spin_lock_init(gsc-slock);
mutex_init(gsc-lock);
-   gsc-clock = ERR_PTR(-EINVAL);
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
gsc-regs = devm_ioremap_resource(dev, res);
@@ -,9 +1080,19 @@ static int gsc_probe(struct platform_device *pdev)
return -ENXIO;
}
 
-   ret = gsc_clk_get(gsc);
-   if (ret)
+   gsc-clock = devm_clk_get(dev, GSC_CLOCK_GATE_NAME);
+   if (IS_ERR(gsc-clock)) {
+   dev_err(dev, failed to get clock~~~: %s\n,
+   GSC_CLOCK_GATE_NAME);
+   return PTR_ERR(gsc-clock);
+   }
+
+   ret = clk_prepare(gsc-clock);
+   if (ret) {
+   dev_err(gsc-pdev-dev, clock prepare failed for clock: %s\n,
+   GSC_CLOCK_GATE_NAME);
return ret;
+   }
 
ret = devm_request_irq(dev, res-start, gsc_irq_handler,
0, pdev-name, gsc);
@@ -1154,7 +1133,7 @@ err_m2m:
 err_v4l2:
v4l2_device_unregister(gsc-v4l2_dev);
 err_clk:
-   gsc_clk_put(gsc);
+   clk_unprepare(gsc-clock);
return ret;
 }
 
@@ -1167,7 +1146,7 @@ static int gsc_remove(struct platform_device *pdev)
 
vb2_dma_contig_cleanup_ctx(gsc-alloc_ctx);
pm_runtime_disable(pdev-dev);
-   gsc_clk_put(gsc);
+   clk_unprepare(gsc-clock);
 
dev_dbg(pdev-dev, %s driver unloaded\n, pdev-name);
return 0;
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] [media] coda: Improve runtime PM support

[Ulf Hansson]

On 22 September 2014 18:05, Philipp Zabel p.za...@pengutronix.de wrote:
 From: Ulf Hansson ulf.hans...@linaro.org

 For several reasons it's good practice to leave devices in runtime PM
 active state while those have been probed.

 In this cases we also want to prevent the device from going inactive,
 until the firmware has been completely installed, especially when using
 a PM domain.

 Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
 Signed-off-by: Philipp Zabel p.za...@pengutronix.de

Thanks for moving this to the next version, I have been a bit busy the
last week.

Changes looking good!

Kind regards
Uffe


 ---
 Changes since v1:
  - Deactivate PM domain on error
  - Added a comment to runtime PM setup
 ---
  drivers/media/platform/coda/coda-common.c | 55 
 ---
  1 file changed, 21 insertions(+), 34 deletions(-)

 diff --git a/drivers/media/platform/coda/coda-common.c 
 b/drivers/media/platform/coda/coda-common.c
 index 0997b5c..ced4760 100644
 --- a/drivers/media/platform/coda/coda-common.c
 +++ b/drivers/media/platform/coda/coda-common.c
 @@ -1688,7 +1688,7 @@ static void coda_fw_callback(const struct firmware *fw, 
 void *context)

 if (!fw) {
 v4l2_err(dev-v4l2_dev, firmware request failed\n);
 -   return;
 +   goto put_pm;
 }

 /* allocate auxiliary per-device code buffer for the BIT processor */
 @@ -1696,50 +1696,27 @@ static void coda_fw_callback(const struct firmware 
 *fw, void *context)
  dev-debugfs_root);
 if (ret  0) {
 dev_err(pdev-dev, failed to allocate code buffer\n);
 -   return;
 +   goto put_pm;
 }

 /* Copy the whole firmware image to the code buffer */
 memcpy(dev-codebuf.vaddr, fw-data, fw-size);
 release_firmware(fw);

 -   if (pm_runtime_enabled(pdev-dev)  pdev-dev.pm_domain) {
 -   /*
 -* Enabling power temporarily will cause coda_hw_init to be
 -* called via coda_runtime_resume by the pm domain.
 -*/
 -   ret = pm_runtime_get_sync(dev-plat_dev-dev);
 -   if (ret  0) {
 -   v4l2_err(dev-v4l2_dev, failed to power on: %d\n,
 -ret);
 -   return;
 -   }
 -
 -   ret = coda_check_firmware(dev);
 -   if (ret  0)
 -   return;
 -
 -   pm_runtime_put_sync(dev-plat_dev-dev);
 -   } else {
 -   /*
 -* If runtime pm is disabled or pm_domain is not set,
 -* initialize once manually.
 -*/
 -   ret = coda_hw_init(dev);
 -   if (ret  0) {
 -   v4l2_err(dev-v4l2_dev, HW initialization 
 failed\n);
 -   return;
 -   }
 -
 -   ret = coda_check_firmware(dev);
 -   if (ret  0)
 -   return;
 +   ret = coda_hw_init(dev);
 +   if (ret  0) {
 +   v4l2_err(dev-v4l2_dev, HW initialization failed\n);
 +   goto put_pm;
 }

 +   ret = coda_check_firmware(dev);
 +   if (ret  0)
 +   goto put_pm;
 +
 dev-alloc_ctx = vb2_dma_contig_init_ctx(pdev-dev);
 if (IS_ERR(dev-alloc_ctx)) {
 v4l2_err(dev-v4l2_dev, Failed to alloc vb2 context\n);
 -   return;
 +   goto put_pm;
 }

 dev-m2m_dev = v4l2_m2m_init(coda_m2m_ops);
 @@ -1771,12 +1748,15 @@ static void coda_fw_callback(const struct firmware 
 *fw, void *context)
 v4l2_info(dev-v4l2_dev, codec registered as /dev/video[%d-%d]\n,
   dev-vfd[0].num, dev-vfd[1].num);

 +   pm_runtime_put_sync(pdev-dev);
 return;

  rel_m2m:
 v4l2_m2m_release(dev-m2m_dev);
  rel_ctx:
 vb2_dma_contig_cleanup_ctx(dev-alloc_ctx);
 +put_pm:
 +   pm_runtime_put_sync(pdev-dev);
  }

  static int coda_firmware_request(struct coda_dev *dev)
 @@ -1998,6 +1978,13 @@ static int coda_probe(struct platform_device *pdev)

 platform_set_drvdata(pdev, dev);

 +   /*
 +* Start activated so we can directly call coda_hw_init in
 +* coda_fw_callback regardless of whether CONFIG_PM_RUNTIME is
 +* enabled or whether the device is associated with a PM domain.
 +*/
 +   pm_runtime_get_noresume(pdev-dev);
 +   pm_runtime_set_active(pdev-dev);
 pm_runtime_enable(pdev-dev);

 return coda_firmware_request(dev);
 --
 2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html