[PATCH] media: st-delta: Fix PM disable depth imbalance in delta_probe

[PATCH] media: st-delta: Fix PM disable depth imbalance in delta_probe

[Miaoqian Lin]

The pm_runtime_enable will increase power disable depth.
If the probe fails, we should use pm_runtime_disable() to balance
pm_runtime_enable().

Fixes: f386509 ("[media] st-delta: STiH4xx multi-format video decoder v4l2 driver")
Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
---
 drivers/media/platform/sti/delta/delta-v4l2.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/sti/delta/delta-v4l2.c b/drivers/media/platform/sti/delta/delta-v4l2.c
index c887a31ebb54..dbcfd0484da4 100644
--- a/drivers/media/platform/sti/delta/delta-v4l2.c
+++ b/drivers/media/platform/sti/delta/delta-v4l2.c
@@ -1898,6 +1898,8 @@ static int delta_probe(struct platform_device *pdev)
 	destroy_workqueue(delta->work_queue);
 err_v4l2:
 	v4l2_device_unregister(&delta->v4l2_dev);
+disable_pm_runtime:
+	pm_runtime_disable(dev);
 err:
 	return ret;
 }
-- 
2.17.1

Re: [PATCH] media: st-delta: Fix PM disable depth imbalance in delta_probe

[kernel test robot]

Hi Miaoqian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on media-tree/master]
[also build test WARNING on v5.16-rc8 next-20220105]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Miaoqian-Lin/media-st-delta-Fix-PM-disable-depth-imbalance-in-delta_probe/20220105-195600
base:   git://linuxtv.org/media_tree.git master
config: riscv-randconfig-r022-20220105 (https://download.01.org/0day-ci/archive/20220106/202201060446.2MV9VJaE-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d5b6e30ed3acad794dd0aec400e617daffc6cc3d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/df377b6adc1cca5963348f9ac16f033e5da299cb
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Miaoqian-Lin/media-st-delta-Fix-PM-disable-depth-imbalance-in-delta_probe/20220105-195600
        git checkout df377b6adc1cca5963348f9ac16f033e5da299cb
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/media/platform/sti/bdisp/ drivers/media/platform/sti/delta/ drivers/media/platform/sti/hva/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/media/platform/sti/delta/delta-v4l2.c:1901:1: warning: unused label 'disable_pm_runtime' [-Wunused-label]
   disable_pm_runtime:
   ^~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +/disable_pm_runtime +1901 drivers/media/platform/sti/delta/delta-v4l2.c

  1812	
  1813	static int delta_probe(struct platform_device *pdev)
  1814	{
  1815		struct delta_dev *delta;
  1816		struct device *dev = &pdev->dev;
  1817		int ret;
  1818	
  1819		delta = devm_kzalloc(dev, sizeof(*delta), GFP_KERNEL);
  1820		if (!delta) {
  1821			ret = -ENOMEM;
  1822			goto err;
  1823		}
  1824	
  1825		delta->dev = dev;
  1826		delta->pdev = pdev;
  1827		platform_set_drvdata(pdev, delta);
  1828	
  1829		mutex_init(&delta->lock);
  1830	
  1831		/* get clock resources */
  1832		delta->clk_delta = devm_clk_get(dev, "delta");
  1833		if (IS_ERR(delta->clk_delta)) {
  1834			dev_dbg(dev, "%s can't get delta clock\n", DELTA_PREFIX);
  1835			delta->clk_delta = NULL;
  1836		}
  1837	
  1838		delta->clk_st231 = devm_clk_get(dev, "delta-st231");
  1839		if (IS_ERR(delta->clk_st231)) {
  1840			dev_dbg(dev, "%s can't get delta-st231 clock\n", DELTA_PREFIX);
  1841			delta->clk_st231 = NULL;
  1842		}
  1843	
  1844		delta->clk_flash_promip = devm_clk_get(dev, "delta-flash-promip");
  1845		if (IS_ERR(delta->clk_flash_promip)) {
  1846			dev_dbg(dev, "%s can't get delta-flash-promip clock\n",
  1847				DELTA_PREFIX);
  1848			delta->clk_flash_promip = NULL;
  1849		}
  1850	
  1851		/* init pm_runtime used for power management */
  1852		pm_runtime_set_autosuspend_delay(dev, DELTA_HW_AUTOSUSPEND_DELAY_MS);
  1853		pm_runtime_use_autosuspend(dev);
  1854		pm_runtime_set_suspended(dev);
  1855		pm_runtime_enable(dev);
  1856	
  1857		/* init firmware ipc channel */
  1858		ret = delta_ipc_init(delta);
  1859		if (ret) {
  1860			dev_err(delta->dev, "%s failed to initialize firmware ipc channel\n",
  1861				DELTA_PREFIX);
  1862			goto err;
  1863		}
  1864	
  1865		/* register all available decoders */
  1866		register_decoders(delta);
  1867	
  1868		/* register all supported formats */
  1869		register_formats(delta);
  1870	
  1871		/* register on V4L2 */
  1872		ret = v4l2_device_register(dev, &delta->v4l2_dev);
  1873		if (ret) {
  1874			dev_err(delta->dev, "%s failed to register V4L2 device\n",
  1875				DELTA_PREFIX);
  1876			goto err;
  1877		}
  1878	
  1879		delta->work_queue = create_workqueue(DELTA_NAME);
  1880		if (!delta->work_queue) {
  1881			dev_err(delta->dev, "%s failed to allocate work queue\n",
  1882				DELTA_PREFIX);
  1883			ret = -ENOMEM;
  1884			goto err_v4l2;
  1885		}
  1886	
  1887		/* register device */
  1888		ret = delta_register_device(delta);
  1889		if (ret)
  1890			goto err_work_queue;
  1891	
  1892		dev_info(dev, "%s %s registered as /dev/video%d\n",
  1893			 DELTA_PREFIX, delta->vdev->name, delta->vdev->num);
  1894	
  1895		return 0;
  1896	
  1897	err_work_queue:
  1898		destroy_workqueue(delta->work_queue);
  1899	err_v4l2:
  1900		v4l2_device_unregister(&delta->v4l2_dev);
> 1901	disable_pm_runtime:
  1902		pm_runtime_disable(dev);
  1903	err:
  1904		return ret;
  1905	}
  1906	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH] media: st-delta: Fix PM disable depth imbalance in delta_probe

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5285 bytes --]

Hi Miaoqian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on media-tree/master]
[also build test WARNING on v5.16-rc8 next-20220105]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Miaoqian-Lin/media-st-delta-Fix-PM-disable-depth-imbalance-in-delta_probe/20220105-195600
base:   git://linuxtv.org/media_tree.git master
config: riscv-randconfig-r022-20220105 (https://download.01.org/0day-ci/archive/20220106/202201060446.2MV9VJaE-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d5b6e30ed3acad794dd0aec400e617daffc6cc3d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/df377b6adc1cca5963348f9ac16f033e5da299cb
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Miaoqian-Lin/media-st-delta-Fix-PM-disable-depth-imbalance-in-delta_probe/20220105-195600
        git checkout df377b6adc1cca5963348f9ac16f033e5da299cb
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/media/platform/sti/bdisp/ drivers/media/platform/sti/delta/ drivers/media/platform/sti/hva/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/media/platform/sti/delta/delta-v4l2.c:1901:1: warning: unused label 'disable_pm_runtime' [-Wunused-label]
   disable_pm_runtime:
   ^~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +/disable_pm_runtime +1901 drivers/media/platform/sti/delta/delta-v4l2.c

  1812	
  1813	static int delta_probe(struct platform_device *pdev)
  1814	{
  1815		struct delta_dev *delta;
  1816		struct device *dev = &pdev->dev;
  1817		int ret;
  1818	
  1819		delta = devm_kzalloc(dev, sizeof(*delta), GFP_KERNEL);
  1820		if (!delta) {
  1821			ret = -ENOMEM;
  1822			goto err;
  1823		}
  1824	
  1825		delta->dev = dev;
  1826		delta->pdev = pdev;
  1827		platform_set_drvdata(pdev, delta);
  1828	
  1829		mutex_init(&delta->lock);
  1830	
  1831		/* get clock resources */
  1832		delta->clk_delta = devm_clk_get(dev, "delta");
  1833		if (IS_ERR(delta->clk_delta)) {
  1834			dev_dbg(dev, "%s can't get delta clock\n", DELTA_PREFIX);
  1835			delta->clk_delta = NULL;
  1836		}
  1837	
  1838		delta->clk_st231 = devm_clk_get(dev, "delta-st231");
  1839		if (IS_ERR(delta->clk_st231)) {
  1840			dev_dbg(dev, "%s can't get delta-st231 clock\n", DELTA_PREFIX);
  1841			delta->clk_st231 = NULL;
  1842		}
  1843	
  1844		delta->clk_flash_promip = devm_clk_get(dev, "delta-flash-promip");
  1845		if (IS_ERR(delta->clk_flash_promip)) {
  1846			dev_dbg(dev, "%s can't get delta-flash-promip clock\n",
  1847				DELTA_PREFIX);
  1848			delta->clk_flash_promip = NULL;
  1849		}
  1850	
  1851		/* init pm_runtime used for power management */
  1852		pm_runtime_set_autosuspend_delay(dev, DELTA_HW_AUTOSUSPEND_DELAY_MS);
  1853		pm_runtime_use_autosuspend(dev);
  1854		pm_runtime_set_suspended(dev);
  1855		pm_runtime_enable(dev);
  1856	
  1857		/* init firmware ipc channel */
  1858		ret = delta_ipc_init(delta);
  1859		if (ret) {
  1860			dev_err(delta->dev, "%s failed to initialize firmware ipc channel\n",
  1861				DELTA_PREFIX);
  1862			goto err;
  1863		}
  1864	
  1865		/* register all available decoders */
  1866		register_decoders(delta);
  1867	
  1868		/* register all supported formats */
  1869		register_formats(delta);
  1870	
  1871		/* register on V4L2 */
  1872		ret = v4l2_device_register(dev, &delta->v4l2_dev);
  1873		if (ret) {
  1874			dev_err(delta->dev, "%s failed to register V4L2 device\n",
  1875				DELTA_PREFIX);
  1876			goto err;
  1877		}
  1878	
  1879		delta->work_queue = create_workqueue(DELTA_NAME);
  1880		if (!delta->work_queue) {
  1881			dev_err(delta->dev, "%s failed to allocate work queue\n",
  1882				DELTA_PREFIX);
  1883			ret = -ENOMEM;
  1884			goto err_v4l2;
  1885		}
  1886	
  1887		/* register device */
  1888		ret = delta_register_device(delta);
  1889		if (ret)
  1890			goto err_work_queue;
  1891	
  1892		dev_info(dev, "%s %s registered as /dev/video%d\n",
  1893			 DELTA_PREFIX, delta->vdev->name, delta->vdev->num);
  1894	
  1895		return 0;
  1896	
  1897	err_work_queue:
  1898		destroy_workqueue(delta->work_queue);
  1899	err_v4l2:
  1900		v4l2_device_unregister(&delta->v4l2_dev);
> 1901	disable_pm_runtime:
  1902		pm_runtime_disable(dev);
  1903	err:
  1904		return ret;
  1905	}
  1906	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[PATCH v2] media: st-delta: Fix PM disable depth imbalance in delta_probe

[Miaoqian Lin]

The pm_runtime_enable will increase power disable depth.
If the probe fails, we should use pm_runtime_disable() to balance
pm_runtime_enable().

Fixes: f386509 ("[media] st-delta: STiH4xx multi-format video decoder v4l2 driver")
Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
---
changes in v2:
- remove unused label.
---
 drivers/media/platform/sti/delta/delta-v4l2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/sti/delta/delta-v4l2.c b/drivers/media/platform/sti/delta/delta-v4l2.c
index c887a31ebb54..36ce0775f3f9 100644
--- a/drivers/media/platform/sti/delta/delta-v4l2.c
+++ b/drivers/media/platform/sti/delta/delta-v4l2.c
@@ -1899,6 +1899,7 @@ static int delta_probe(struct platform_device *pdev)
 err_v4l2:
 	v4l2_device_unregister(&delta->v4l2_dev);
 err:
+	pm_runtime_disable(dev);
 	return ret;
 }
 
-- 
2.17.1

Re: [PATCH v2] media: st-delta: Fix PM disable depth imbalance in delta_probe

[Hans Verkuil]

Hi Miaoqian Lin,

On 3/1/22 04:12, Miaoqian Lin wrote:
> The pm_runtime_enable will increase power disable depth.
> If the probe fails, we should use pm_runtime_disable() to balance
> pm_runtime_enable().
> 
> Fixes: f386509 ("[media] st-delta: STiH4xx multi-format video decoder v4l2 driver")
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
> ---
> changes in v2:
> - remove unused label.
> ---
>  drivers/media/platform/sti/delta/delta-v4l2.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/platform/sti/delta/delta-v4l2.c b/drivers/media/platform/sti/delta/delta-v4l2.c
> index c887a31ebb54..36ce0775f3f9 100644
> --- a/drivers/media/platform/sti/delta/delta-v4l2.c
> +++ b/drivers/media/platform/sti/delta/delta-v4l2.c
> @@ -1899,6 +1899,7 @@ static int delta_probe(struct platform_device *pdev)
>  err_v4l2:
>  	v4l2_device_unregister(&delta->v4l2_dev);
>  err:
> +	pm_runtime_disable(dev);

This isn't right. If the devm_kzalloc at the beginning fails, then it also jump
to this label, but at that time no pm_runtime_enable() has been called yet,
so this patch will just introduce another imbalance.

You *do* need a new label here (like you did in v1 of this patch), and update
the 'goto err;' instances after the call to pm_runtime_enable() to go to that
new label.

Regards,

	Hans

>  	return ret;
>  }
>  

[PATCH v3] media: st-delta: Fix PM disable depth imbalance in delta_probe

[Miaoqian Lin]

The pm_runtime_enable will decrease power disable depth.
If the probe fails, we should use pm_runtime_disable() to balance
pm_runtime_enable().

Fixes: f386509 ("[media] st-delta: STiH4xx multi-format video decoder v4l2 driver")
Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
---
changes in v2:
- remove unused label.
changes in v3:
- add err_pm_disable label and update related 'goto err'.
- update commit message
---
 drivers/media/platform/sti/delta/delta-v4l2.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/sti/delta/delta-v4l2.c b/drivers/media/platform/sti/delta/delta-v4l2.c
index c887a31ebb54..420ad4d8df5d 100644
--- a/drivers/media/platform/sti/delta/delta-v4l2.c
+++ b/drivers/media/platform/sti/delta/delta-v4l2.c
@@ -1859,7 +1859,7 @@ static int delta_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(delta->dev, "%s failed to initialize firmware ipc channel\n",
 			DELTA_PREFIX);
-		goto err;
+		goto err_pm_disable;
 	}
 
 	/* register all available decoders */
@@ -1873,7 +1873,7 @@ static int delta_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(delta->dev, "%s failed to register V4L2 device\n",
 			DELTA_PREFIX);
-		goto err;
+		goto err_pm_disable;
 	}
 
 	delta->work_queue = create_workqueue(DELTA_NAME);
@@ -1898,6 +1898,8 @@ static int delta_probe(struct platform_device *pdev)
 	destroy_workqueue(delta->work_queue);
 err_v4l2:
 	v4l2_device_unregister(&delta->v4l2_dev);
+err_pm_disable:
+	pm_runtime_disable(dev);
 err:
 	return ret;
 }
-- 
2.17.1

Re: [PATCH v3] media: st-delta: Fix PM disable depth imbalance in delta_probe

[Hugues FRUCHET - FOSS]

Hi Miaoqian Lin,

Thanks for the patch !

Acked-by: Hugues Fruchet <hugues.fruchet@foss.st.com>

BR,
Hugues.

On 3/7/22 9:08 AM, Miaoqian Lin wrote:
> The pm_runtime_enable will decrease power disable depth.
> If the probe fails, we should use pm_runtime_disable() to balance
> pm_runtime_enable().
> 
> Fixes: f386509 ("[media] st-delta: STiH4xx multi-format video decoder v4l2 driver")
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
> ---
> changes in v2:
> - remove unused label.
> changes in v3:
> - add err_pm_disable label and update related 'goto err'.
> - update commit message
> ---
>   drivers/media/platform/sti/delta/delta-v4l2.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/sti/delta/delta-v4l2.c b/drivers/media/platform/sti/delta/delta-v4l2.c
> index c887a31ebb54..420ad4d8df5d 100644
> --- a/drivers/media/platform/sti/delta/delta-v4l2.c
> +++ b/drivers/media/platform/sti/delta/delta-v4l2.c
> @@ -1859,7 +1859,7 @@ static int delta_probe(struct platform_device *pdev)
>   	if (ret) {
>   		dev_err(delta->dev, "%s failed to initialize firmware ipc channel\n",
>   			DELTA_PREFIX);
> -		goto err;
> +		goto err_pm_disable;
>   	}
>   
>   	/* register all available decoders */
> @@ -1873,7 +1873,7 @@ static int delta_probe(struct platform_device *pdev)
>   	if (ret) {
>   		dev_err(delta->dev, "%s failed to register V4L2 device\n",
>   			DELTA_PREFIX);
> -		goto err;
> +		goto err_pm_disable;
>   	}
>   
>   	delta->work_queue = create_workqueue(DELTA_NAME);
> @@ -1898,6 +1898,8 @@ static int delta_probe(struct platform_device *pdev)
>   	destroy_workqueue(delta->work_queue);
>   err_v4l2:
>   	v4l2_device_unregister(&delta->v4l2_dev);
> +err_pm_disable:
> +	pm_runtime_disable(dev);
>   err:
>   	return ret;
>   }
>