linux-kernel - Re: [PATCH v4 11/15] media: rkvdec: Enable all clocks without naming them

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <74866576-8834-49e6-adb6-b0189a1b43fe@kwiboo.se>
Date: Wed, 22 Oct 2025 23:18:12 +0200
From: Jonas Karlman <jonas@...boo.se>
To: Detlev Casanova <detlev.casanova@...labora.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
 Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
 Heiko Stuebner <heiko@...ech.de>, Ricardo Ribalda <ribalda@...omium.org>,
 Hans Verkuil <hverkuil@...nel.org>, Hans de Goede <hansg@...nel.org>,
 Yunke Cao <yunkec@...gle.com>, Jonathan Corbet <corbet@....net>,
 Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>,
 James Cowgill <james.cowgill@...ize.com>, linux-media@...r.kernel.org,
 linux-rockchip@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
 kernel@...labora.com, Nicolas Dufresne <nicolas.dufresne@...labora.com>,
 Diederik de Haas <didi.debian@...ow.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 11/15] media: rkvdec: Enable all clocks without naming
 them

Hi Detlev,

On 10/22/2025 7:45 PM, Detlev Casanova wrote:
> For other variants, the clock names and number will differ.
> 
> There is no need to keep track of the clock names in the driver so drop
> them to avoid having a list for each variant.
> 
> Tested-by: Diederik de Haas <didi.debian@...ow.org>  # Rock 5B
> Signed-off-by: Detlev Casanova <detlev.casanova@...labora.com>
> ---
>  .../media/platform/rockchip/rkvdec/rkvdec.c   | 24 +++++--------------
>  .../media/platform/rockchip/rkvdec/rkvdec.h   |  1 +
>  2 files changed, 7 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec.c b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> index b0e90c9b826e..aa92b586429e 100644
> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> @@ -1311,15 +1311,10 @@ static const struct of_device_id of_rkvdec_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, of_rkvdec_match);
>  
> -static const char * const rkvdec_clk_names[] = {
> -	"axi", "ahb", "cabac", "core"
> -};
> -
>  static int rkvdec_probe(struct platform_device *pdev)
>  {
>  	const struct rkvdec_variant *variant;
>  	struct rkvdec_dev *rkvdec;
> -	unsigned int i;
>  	int ret, irq;
>  
>  	variant = of_device_get_match_data(&pdev->dev);
> @@ -1336,19 +1331,12 @@ static int rkvdec_probe(struct platform_device *pdev)
>  	mutex_init(&rkvdec->vdev_lock);
>  	INIT_DELAYED_WORK(&rkvdec->watchdog_work, rkvdec_watchdog_func);
>  
> -	rkvdec->clocks = devm_kcalloc(&pdev->dev, ARRAY_SIZE(rkvdec_clk_names),
> -				      sizeof(*rkvdec->clocks), GFP_KERNEL);
> -	if (!rkvdec->clocks)
> -		return -ENOMEM;
> -
> -	for (i = 0; i < ARRAY_SIZE(rkvdec_clk_names); i++)
> -		rkvdec->clocks[i].id = rkvdec_clk_names[i];
> -
> -	ret = devm_clk_bulk_get(&pdev->dev, ARRAY_SIZE(rkvdec_clk_names),
> -				rkvdec->clocks);
> -	if (ret)
> +	ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &rkvdec->clocks);
> +	if (ret < 0)
>  		return ret;
>  
> +	rkvdec->clk_count = ret;
> +
>  	rkvdec->regs = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(rkvdec->regs))
>  		return PTR_ERR(rkvdec->regs);
> @@ -1426,7 +1414,7 @@ static int rkvdec_runtime_resume(struct device *dev)
>  {
>  	struct rkvdec_dev *rkvdec = dev_get_drvdata(dev);
>  
> -	return clk_bulk_prepare_enable(ARRAY_SIZE(rkvdec_clk_names),
> +	return clk_bulk_prepare_enable(rkvdec->clk_count,
>  				       rkvdec->clocks);

This can probably now fit on one line.

>  }
>  
> @@ -1434,7 +1422,7 @@ static int rkvdec_runtime_suspend(struct device *dev)
>  {
>  	struct rkvdec_dev *rkvdec = dev_get_drvdata(dev);
>  
> -	clk_bulk_disable_unprepare(ARRAY_SIZE(rkvdec_clk_names),
> +	clk_bulk_disable_unprepare(rkvdec->clk_count,
>  				   rkvdec->clocks);

Same here.

>  	return 0;
>  }
> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec.h b/drivers/media/platform/rockchip/rkvdec/rkvdec.h
> index dfafb2c2cbc1..4094f92f8a44 100644
> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec.h
> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec.h
> @@ -131,6 +131,7 @@ struct rkvdec_dev {
>  	struct v4l2_m2m_dev *m2m_dev;
>  	struct device *dev;
>  	struct clk_bulk_data *clocks;
> +	unsigned int clk_count;

Please name this num_clocks to match similar count/num variables used by
the driver and also closer matches the clk_bulk_data name.

Regards,
Jonas

>  	void __iomem *regs;
>  	struct mutex vdev_lock; /* serializes ioctls */
>  	struct delayed_work watchdog_work;