linux-kernel - Re: [PATCH v2 4/4] media: i2c: imx214: Remove hard-coded external clock frequency

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: <aBnHI1APgjfcj2xG@kekkonen.localdomain>
Date: Tue, 6 May 2025 08:24:03 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: git@...tzsch.eu
Cc: Ricardo Ribalda <ribalda@...nel.org>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	~postmarketos/upstreaming@...ts.sr.ht, phone-devel@...r.kernel.org,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
	laurent.pinchart@...asonboard.com
Subject: Re: [PATCH v2 4/4] media: i2c: imx214: Remove hard-coded external
 clock frequency

Hi André,

On Mon, May 05, 2025 at 11:05:56PM +0200, André Apitzsch via B4 Relay wrote:
> From: André Apitzsch <git@...tzsch.eu>
> 
> Instead rely on the rate set on the clock (using assigned-clock-rates
> etc.)
> 
> Signed-off-by: André Apitzsch <git@...tzsch.eu>
> ---
>  drivers/media/i2c/imx214.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
> index 9e9be47394ec768a5b34d44b06b5bbb0988da5a1..c12996e294dccebb18c608254f1e0d14dc064423 100644
> --- a/drivers/media/i2c/imx214.c
> +++ b/drivers/media/i2c/imx214.c
> @@ -32,7 +32,6 @@
>  
>  #define IMX214_REG_FAST_STANDBY_CTRL	CCI_REG8(0x0106)
>  
> -#define IMX214_DEFAULT_CLK_FREQ	24000000
>  #define IMX214_DEFAULT_LINK_FREQ	600000000
>  /* Keep wrong link frequency for backward compatibility */
>  #define IMX214_DEFAULT_LINK_FREQ_LEGACY	480000000
> @@ -1405,11 +1404,6 @@ static int imx214_probe(struct i2c_client *client)
>  		return dev_err_probe(dev, PTR_ERR(imx214->xclk),
>  				     "failed to get xclk\n");
>  
> -	ret = clk_set_rate(imx214->xclk, IMX214_DEFAULT_CLK_FREQ);
> -	if (ret)
> -		return dev_err_probe(dev, ret,
> -				     "failed to set xclk frequency\n");
> -

Oops. I missed this is what the driver was doing already. Indeed, this is
one of the historic sensor drivers that do set the frequency in DT systems.

The driver never used the clock-frequency property and instead used a fixed
frequency. Changing the behaviour now could be problematic.

There are options here that I think we could do:

1) use your v1 patch (4) which uses "clock-frequency" if it exists and
otherwise uses the default, fixed frequency or

2) set the frequency only if the "clock-frequency" property exists. The DT
currently requires clock-frequency and the YAML conversion was done in 2020
whereas the driver is from 2018. If we do this, the clock-frequency should
be deprecated (or even removed from bingings).

I wonder what others think. Cc'd Laurent in any case.

>  	ret = imx214_get_regulators(dev, imx214);
>  	if (ret < 0)
>  		return dev_err_probe(dev, ret, "failed to get regulators\n");
> 

-- 
Regards,

Sakari Ailus