Re: [PATCH] media: stm32-dcmi: add power saving support
Hi Hugues,
I love your patch! Yet something to improve:
[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.17 next-20180608]
[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/Hugues-Fruchet/media-stm32-dcmi-add-power-saving-support/20180611-174016
base: git://linuxtv.org/media_tree.git master
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=sparc64
All errors (new ones prefixed by >>):
drivers/media//platform/stm32/stm32-dcmi.c: In function 'dcmi_suspend':
drivers/media//platform/stm32/stm32-dcmi.c:1886:2: error: implicit
declaration of function 'pinctrl_pm_select_sleep_state'
[-Werror=implicit-function-declaration]
pinctrl_pm_select_sleep_state(dev);
^
drivers/media//platform/stm32/stm32-dcmi.c: In function 'dcmi_resume':
>> drivers/media//platform/stm32/stm32-dcmi.c:1894:2: error: implicit
>> declaration of function 'pinctrl_pm_select_default_state'; did you mean
>> 'irq_set_default_host'? [-Werror=implicit-function-declaration]
pinctrl_pm_select_default_state(dev);
^~~
irq_set_default_host
cc1: some warnings being treated as errors
vim +1894 drivers/media//platform/stm32/stm32-dcmi.c
1879
1880 static __maybe_unused int dcmi_suspend(struct device *dev)
1881 {
1882 /* disable clock */
1883 pm_runtime_force_suspend(dev);
1884
1885 /* change pinctrl state */
> 1886 pinctrl_pm_select_sleep_state(dev);
1887
1888 return 0;
1889 }
1890
1891 static __maybe_unused int dcmi_resume(struct device *dev)
1892 {
1893 /* restore pinctl default state */
> 1894 pinctrl_pm_select_default_state(dev);
1895
1896 /* clock enable */
1897 pm_runtime_force_resume(dev);
1898
1899 return 0;
1900 }
1901
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH] media: stm32-dcmi: add power saving support
Hi Hugues,
I love your patch! Yet something to improve:
[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.17 next-20180608]
[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/Hugues-Fruchet/media-stm32-dcmi-add-power-saving-support/20180611-174016
base: git://linuxtv.org/media_tree.git master
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/media/platform/stm32/stm32-dcmi.c: In function 'dcmi_suspend':
>> drivers/media/platform/stm32/stm32-dcmi.c:1886:2: error: implicit
>> declaration of function 'pinctrl_pm_select_sleep_state'
>> [-Werror=implicit-function-declaration]
pinctrl_pm_select_sleep_state(dev);
^
drivers/media/platform/stm32/stm32-dcmi.c: In function 'dcmi_resume':
>> drivers/media/platform/stm32/stm32-dcmi.c:1894:2: error: implicit
>> declaration of function 'pinctrl_pm_select_default_state'
>> [-Werror=implicit-function-declaration]
pinctrl_pm_select_default_state(dev);
^~~
cc1: some warnings being treated as errors
vim +/pinctrl_pm_select_sleep_state +1886
drivers/media/platform/stm32/stm32-dcmi.c
1879
1880 static __maybe_unused int dcmi_suspend(struct device *dev)
1881 {
1882 /* disable clock */
1883 pm_runtime_force_suspend(dev);
1884
1885 /* change pinctrl state */
> 1886 pinctrl_pm_select_sleep_state(dev);
1887
1888 return 0;
1889 }
1890
1891 static __maybe_unused int dcmi_resume(struct device *dev)
1892 {
1893 /* restore pinctl default state */
> 1894 pinctrl_pm_select_default_state(dev);
1895
1896 /* clock enable */
1897 pm_runtime_force_resume(dev);
1898
1899 return 0;
1900 }
1901
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
[PATCH] media: stm32-dcmi: add power saving support
Implements runtime & system sleep power management ops.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 80 ---
1 file changed, 64 insertions(+), 16 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index 2e1933d..68da9ec 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -22,6 +22,7 @@
#include
#include
#include
#include
+#include
#include
#include
@@ -578,9 +579,9 @@ static int dcmi_start_streaming(struct vb2_queue *vq,
unsigned int count)
u32 val = 0;
int ret;
- ret = clk_enable(dcmi->mclk);
+ ret = pm_runtime_get_sync(dcmi->dev);
if (ret) {
- dev_err(dcmi->dev, "%s: Failed to start streaming, cannot
enable clock\n",
+ dev_err(dcmi->dev, "%s: Failed to start streaming, cannot get
sync\n",
__func__);
goto err_release_buffers;
}
@@ -590,7 +591,7 @@ static int dcmi_start_streaming(struct vb2_queue *vq,
unsigned int count)
if (ret && ret != -ENOIOCTLCMD) {
dev_err(dcmi->dev, "%s: Failed to start streaming, subdev
streamon error",
__func__);
- goto err_disable_clock;
+ goto err_pm_put;
}
spin_lock_irq(>irqlock);
@@ -675,8 +676,8 @@ static int dcmi_start_streaming(struct vb2_queue *vq,
unsigned int count)
err_subdev_streamoff:
v4l2_subdev_call(dcmi->entity.subdev, video, s_stream, 0);
-err_disable_clock:
- clk_disable(dcmi->mclk);
+err_pm_put:
+ pm_runtime_put(dcmi->dev);
err_release_buffers:
spin_lock_irq(>irqlock);
@@ -749,7 +750,7 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
/* Stop all pending DMA operations */
dmaengine_terminate_all(dcmi->dma_chan);
- clk_disable(dcmi->mclk);
+ pm_runtime_put(dcmi->dev);
if (dcmi->errors_count)
dev_warn(dcmi->dev, "Some errors found while streaming:
errors=%d (overrun=%d), buffers=%d\n",
@@ -1751,12 +1752,6 @@ static int dcmi_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
- ret = clk_prepare(mclk);
- if (ret) {
- dev_err(>dev, "Unable to prepare mclk %p\n", mclk);
- goto err_dma_release;
- }
-
spin_lock_init(>irqlock);
mutex_init(>lock);
init_completion(>complete);
@@ -1772,7 +1767,7 @@ static int dcmi_probe(struct platform_device *pdev)
/* Initialize the top-level structure */
ret = v4l2_device_register(>dev, >v4l2_dev);
if (ret)
- goto err_clk_unprepare;
+ goto err_dma_release;
dcmi->vdev = video_device_alloc();
if (!dcmi->vdev) {
@@ -1832,14 +1827,15 @@ static int dcmi_probe(struct platform_device *pdev)
dev_info(>dev, "Probe done\n");
platform_set_drvdata(pdev, dcmi);
+
+ pm_runtime_enable(>dev);
+
return 0;
err_device_release:
video_device_release(dcmi->vdev);
err_device_unregister:
v4l2_device_unregister(>v4l2_dev);
-err_clk_unprepare:
- clk_unprepare(dcmi->mclk);
err_dma_release:
dma_release_channel(dcmi->dma_chan);
@@ -1850,20 +1846,72 @@ static int dcmi_remove(struct platform_device *pdev)
{
struct stm32_dcmi *dcmi = platform_get_drvdata(pdev);
+ pm_runtime_disable(>dev);
+
v4l2_async_notifier_unregister(>notifier);
v4l2_device_unregister(>v4l2_dev);
- clk_unprepare(dcmi->mclk);
+
dma_release_channel(dcmi->dma_chan);
return 0;
}
+static __maybe_unused int dcmi_runtime_suspend(struct device *dev)
+{
+ struct stm32_dcmi *dcmi = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(dcmi->mclk);
+
+ return 0;
+}
+
+static __maybe_unused int dcmi_runtime_resume(struct device *dev)
+{
+ struct stm32_dcmi *dcmi = dev_get_drvdata(dev);
+ int ret;
+
+ ret = clk_prepare_enable(dcmi->mclk);
+ if (ret)
+ dev_err(dev, "%s: Failed to prepare_enable clock\n", __func__);
+
+ return ret;
+}
+
+static __maybe_unused int dcmi_suspend(struct device *dev)
+{
+ /* disable clock */
+ pm_runtime_force_suspend(dev);
+
+ /* change pinctrl state */
+ pinctrl_pm_select_sleep_state(dev);
+
+ return 0;
+}
+
+static __maybe_unused int dcmi_resume(struct device *dev)
+{
+ /* restore pinctl default state */
+ pinctrl_pm_select_default_state(dev);
+
+ /* clock enable */
+ pm_runtime_force_resume(dev);
+
+ return 0;
+}
+
+static const struct dev_pm_ops dcmi_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(dcmi_suspend, dcmi_resume)
+ SET_RUNTIME_PM_OPS(dcmi_runtime_suspend,
+ dcmi_runtime_resume, NULL)
+};
+
static struct platform_driver
