| 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: <9111c947-ef0f-4c4a-a09f-6da0901a8510@ti.com>
Date: Thu, 18 Dec 2025 11:05:55 +0530
From: Rishikesh Donadkar <r-donadkar@...com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, <jai.luthra@...ux.dev>,
<laurent.pinchart@...asonboard.com>, <mripard@...nel.org>
CC: <y-abhilashchandra@...com>, <devarsht@...com>, <s-jain1@...com>,
<vigneshr@...com>, <mchehab@...nel.org>, <robh@...nel.org>,
<krzk+dt@...nel.org>, <p.zabel@...gutronix.de>, <conor+dt@...nel.org>,
<sakari.ailus@...ux.intel.com>, <hverkuil-cisco@...all.nl>,
<jai.luthra@...asonboard.com>, <changhuang.liang@...rfivetech.com>,
<jack.zhu@...rfivetech.com>, <sjoerd@...labora.com>,
<dan.carpenter@...aro.org>, <hverkuil+cisco@...nel.org>,
<linux-kernel@...r.kernel.org>, <linux-media@...r.kernel.org>,
<devicetree@...r.kernel.org>
Subject: Re: [PATCH v8 16/18] media: cadence: csi2rx: Support runtime PM
On 01/12/25 18:41, Tomi Valkeinen wrote:
> Hi,
Hi Tomi,
Thank you for the review !
>
> On 12/11/2025 13:54, Rishikesh Donadkar wrote:
>> From: Changhuang Liang <changhuang.liang@...rfivetech.com>
>>
>> Use runtime power management hooks to save power when CSI-RX is not in
>> use.
>>
>> Signed-off-by: Changhuang Liang <changhuang.liang@...rfivetech.com>
>> Tested-by: Rishikesh Donadkar <r-donadkar@...com>
>> Reviewed-by: Rishikesh Donadkar <r-donadkar@...com>
>> Signed-off-by: Jai Luthra <jai.luthra@...asonboard.com>
>> Signed-off-by: Rishikesh Donadkar <r-donadkar@...com>
>> ---
>> drivers/media/platform/cadence/Kconfig | 1 +
>> drivers/media/platform/cadence/cdns-csi2rx.c | 129 ++++++++++++-------
>> 2 files changed, 83 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/media/platform/cadence/Kconfig b/drivers/media/platform/cadence/Kconfig
>> index 1aa608c00dbce..ea85ef82760e6 100644
>> --- a/drivers/media/platform/cadence/Kconfig
>> +++ b/drivers/media/platform/cadence/Kconfig
>> @@ -5,6 +5,7 @@ comment "Cadence media platform drivers"
>> config VIDEO_CADENCE_CSI2RX
>> tristate "Cadence MIPI-CSI2 RX Controller"
>> depends on VIDEO_DEV
>> + depends on PM
>> select MEDIA_CONTROLLER
>> select VIDEO_V4L2_SUBDEV_API
>> select V4L2_FWNODE
>> diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
>> index 833bc134f17cb..6447c225ba354 100644
>> --- a/drivers/media/platform/cadence/cdns-csi2rx.c
>> +++ b/drivers/media/platform/cadence/cdns-csi2rx.c
>> @@ -337,11 +337,6 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
>> u32 reg;
>> int ret;
>>
>> - ret = clk_prepare_enable(csi2rx->p_clk);
>> - if (ret)
>> - return ret;
>> -
>> - reset_control_deassert(csi2rx->p_rst);
>> csi2rx_reset(csi2rx);
>>
>> if (csi2rx->error_irq >= 0)
>> @@ -382,7 +377,7 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
>> if (ret) {
>> dev_err(csi2rx->dev,
>> "Failed to configure external DPHY: %d\n", ret);
>> - goto err_disable_pclk;
>> + return ret;
>> }
>> }
>>
>> @@ -397,12 +392,6 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
>> * hence the reference counting.
>> */
>> for (i = 0; i < csi2rx->max_streams; i++) {
>> - ret = clk_prepare_enable(csi2rx->pixel_clk[i]);
>> - if (ret)
>> - goto err_disable_pixclk;
>> -
>> - reset_control_deassert(csi2rx->pixel_rst[i]);
>> -
>> writel(CSI2RX_STREAM_CFG_FIFO_MODE_LARGE_BUF |
>> FIELD_PREP(CSI2RX_STREAM_CFG_NUM_PIXELS_MASK,
>> csi2rx->num_pixels[i]),
>> @@ -415,30 +404,8 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
>> csi2rx->base + CSI2RX_STREAM_CTRL_REG(i));
>> }
>>
>> - ret = clk_prepare_enable(csi2rx->sys_clk);
>> - if (ret)
>> - goto err_disable_pixclk;
>> -
>> - reset_control_deassert(csi2rx->sys_rst);
>> -
>> - clk_disable_unprepare(csi2rx->p_clk);
>>
>> return 0;
>> -
>> -err_disable_pixclk:
>> - for (; i > 0; i--) {
>> - reset_control_assert(csi2rx->pixel_rst[i - 1]);
>> - clk_disable_unprepare(csi2rx->pixel_clk[i - 1]);
>> - }
>> -
>> - if (csi2rx->dphy) {
>> - writel(0, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG);
>> - phy_power_off(csi2rx->dphy);
>> - }
>> -err_disable_pclk:
>> - clk_disable_unprepare(csi2rx->p_clk);
>> -
>> - return ret;
>> }
>>
>> static void csi2rx_stop(struct csi2rx_priv *csi2rx)
>> @@ -447,10 +414,6 @@ static void csi2rx_stop(struct csi2rx_priv *csi2rx)
>> u32 val;
>> int ret;
>>
>> - clk_prepare_enable(csi2rx->p_clk);
>> - reset_control_assert(csi2rx->sys_rst);
>> - clk_disable_unprepare(csi2rx->sys_clk);
>> -
>> writel(0, csi2rx->base + CSI2RX_ERROR_IRQS_MASK_REG);
>>
>> for (i = 0; i < csi2rx->max_streams; i++) {
>> @@ -465,14 +428,8 @@ static void csi2rx_stop(struct csi2rx_priv *csi2rx)
>> if (ret)
>> dev_warn(csi2rx->dev,
>> "Failed to stop streaming on pad%u\n", i);
>> -
>> - reset_control_assert(csi2rx->pixel_rst[i]);
>> - clk_disable_unprepare(csi2rx->pixel_clk[i]);
>> }
>>
>> - reset_control_assert(csi2rx->p_rst);
>> - clk_disable_unprepare(csi2rx->p_clk);
>> -
>> if (csi2rx->dphy) {
>> writel(0, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG);
>>
>> @@ -548,10 +505,17 @@ static int csi2rx_enable_streams(struct v4l2_subdev *subdev,
>> * enable the whole controller.
>> */
>> if (!csi2rx->count) {
>> + ret = pm_runtime_resume_and_get(csi2rx->dev);
>> + if (ret < 0)
>> + return ret;
>> +
>> csi2rx_update_vc_select(csi2rx, state);
>> +
>> ret = csi2rx_start(csi2rx);
>> - if (ret)
>> + if (ret) {
>> + pm_runtime_put(csi2rx->dev);
>> return ret;
>> + }
>> }
>>
>> /* Start streaming on the source */
>> @@ -561,8 +525,10 @@ static int csi2rx_enable_streams(struct v4l2_subdev *subdev,
>> dev_err(csi2rx->dev,
>> "Failed to start streams %#llx on subdev\n",
>> sink_streams);
>> - if (!csi2rx->count)
>> + if (!csi2rx->count) {
>> csi2rx_stop(csi2rx);
>> + pm_runtime_put(csi2rx->dev);
> Probably time to add 'goto' based error handling to this func... Maybe
> in this patch, maybe as a separate patch before this.
Okay, I will do it in this patch itself.
Rishikesh
>
> Tomi
>
>> + }
>> return ret;
>> }
>>
>> @@ -589,8 +555,10 @@ static int csi2rx_disable_streams(struct v4l2_subdev *subdev,
>> csi2rx->count--;
>>
>> /* Let the last user turn off the lights. */
>> - if (!csi2rx->count)
>> + if (!csi2rx->count) {
>> csi2rx_stop(csi2rx);
>> + pm_runtime_put(csi2rx->dev);
>> + }
>>
>> return 0;
>> }
>> @@ -1092,6 +1060,7 @@ static int csi2rx_probe(struct platform_device *pdev)
>> if (ret)
>> goto err_cleanup;
>>
>> + pm_runtime_enable(csi2rx->dev);
>> ret = v4l2_async_register_subdev(&csi2rx->subdev);
>> if (ret < 0)
>> goto err_free_state;
>> @@ -1106,6 +1075,7 @@ static int csi2rx_probe(struct platform_device *pdev)
>>
>> err_free_state:
>> v4l2_subdev_cleanup(&csi2rx->subdev);
>> + pm_runtime_disable(csi2rx->dev);
>> err_cleanup:
>> v4l2_async_nf_unregister(&csi2rx->notifier);
>> v4l2_async_nf_cleanup(&csi2rx->notifier);
>> @@ -1124,9 +1094,73 @@ static void csi2rx_remove(struct platform_device *pdev)
>> v4l2_async_unregister_subdev(&csi2rx->subdev);
>> v4l2_subdev_cleanup(&csi2rx->subdev);
>> media_entity_cleanup(&csi2rx->subdev.entity);
>> + pm_runtime_disable(csi2rx->dev);
>> kfree(csi2rx);
>> }
>>
>> +static int csi2rx_runtime_suspend(struct device *dev)
>> +{
>> + struct csi2rx_priv *csi2rx = dev_get_drvdata(dev);
>> + unsigned int i;
>> +
>> + reset_control_assert(csi2rx->sys_rst);
>> + clk_disable_unprepare(csi2rx->sys_clk);
>> +
>> + for (i = 0; i < csi2rx->max_streams; i++) {
>> + reset_control_assert(csi2rx->pixel_rst[i]);
>> + clk_disable_unprepare(csi2rx->pixel_clk[i]);
>> + }
>> +
>> + reset_control_assert(csi2rx->p_rst);
>> + clk_disable_unprepare(csi2rx->p_clk);
>> +
>> + return 0;
>> +}
>> +
>> +static int csi2rx_runtime_resume(struct device *dev)
>> +{
>> + struct csi2rx_priv *csi2rx = dev_get_drvdata(dev);
>> + unsigned int i;
>> + int ret;
>> +
>> + ret = clk_prepare_enable(csi2rx->p_clk);
>> + if (ret)
>> + return ret;
>> +
>> + reset_control_deassert(csi2rx->p_rst);
>> +
>> + for (i = 0; i < csi2rx->max_streams; i++) {
>> + ret = clk_prepare_enable(csi2rx->pixel_clk[i]);
>> + if (ret)
>> + goto err_disable_pixclk;
>> +
>> + reset_control_deassert(csi2rx->pixel_rst[i]);
>> + }
>> +
>> + ret = clk_prepare_enable(csi2rx->sys_clk);
>> + if (ret)
>> + goto err_disable_pixclk;
>> +
>> + reset_control_deassert(csi2rx->sys_rst);
>> +
>> + return 0;
>> +
>> +err_disable_pixclk:
>> + for (; i > 0; i--) {
>> + reset_control_assert(csi2rx->pixel_rst[i - 1]);
>> + clk_disable_unprepare(csi2rx->pixel_clk[i - 1]);
>> + }
>> +
>> + reset_control_assert(csi2rx->p_rst);
>> + clk_disable_unprepare(csi2rx->p_clk);
>> +
>> + return ret;
>> +}
>> +
>> +static const struct dev_pm_ops csi2rx_pm_ops = {
>> + RUNTIME_PM_OPS(csi2rx_runtime_suspend, csi2rx_runtime_resume, NULL)
>> +};
>> +
>> static const struct of_device_id csi2rx_of_table[] = {
>> { .compatible = "starfive,jh7110-csi2rx" },
>> { .compatible = "cdns,csi2rx" },
>> @@ -1141,6 +1175,7 @@ static struct platform_driver csi2rx_driver = {
>> .driver = {
>> .name = "cdns-csi2rx",
>> .of_match_table = csi2rx_of_table,
>> + .pm = &csi2rx_pm_ops,
>> },
>> };
>> module_platform_driver(csi2rx_driver);
Powered by blists - more mailing lists