Re: [PATCH] media: i2c: ov13858: Enable clock frequency macro

[Sakari Ailus]

Hi Sunny,

On Sun, Apr 06, 2025 at 08:13:22PM +0530, Sunny Patel wrote:
> Defines a macro `OV13858_MCLK` for the clock frequency (19200000 Hz).
> Replaces the hardcoded clock frequency value in the probe function.

Thanks for the patch.

While the patch seems fine, the subject and the commit message would
benefit from some attention. For instance, don't use third person without a
subject. Please see examples in existing commit messages, too.

The clock seems to be called "EXTCLK" in the datasheet, too.

> 
> Signed-off-by: Sunny Patel <nueralspacetech@xxxxxxxxx>
> ---
>  drivers/media/i2c/ov13858.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
> index 7a3fc1d28514..a1242a90cdc9 100644
> --- a/drivers/media/i2c/ov13858.c
> +++ b/drivers/media/i2c/ov13858.c
> @@ -21,6 +21,8 @@
>  #define OV13858_REG_SOFTWARE_RST	0x0103
>  #define OV13858_SOFTWARE_RST		0x01
>  
> +#define OV13858_MCLK				19200000
> +
>  /* PLL1 generates PCLK and MIPI_PHY_CLK */
>  #define OV13858_REG_PLL1_CTRL_0		0x0300
>  #define OV13858_REG_PLL1_CTRL_1		0x0301
> @@ -1664,7 +1666,7 @@ static int ov13858_probe(struct i2c_client *client)
>  	u32 val = 0;
>  
>  	device_property_read_u32(&client->dev, "clock-frequency", &val);
> -	if (val != 19200000)
> +	if (val != OV13858_MCLK)
>  		return -EINVAL;
>  
>  	ov13858 = devm_kzalloc(&client->dev, sizeof(*ov13858), GFP_KERNEL);

-- 
Kind regards,

Sakari Ailus