Re: [PATCH 1/3] media: i2c: imx219: add support for specifying clock-frequencies

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Michael,

On Sat, Dec 5, 2020 at 7:36 PM <michael.srba@seznam.cz> wrote:
> From: Michael Srba <Michael.Srba@seznam.cz>
>
> This patch adds 1% tolerance on input clock, similar to other camera sensor
> drivers. It also allows for specifying the actual clock in the device tree,
> instead of relying on it being already set to the right frequency (which is
> often not the case).
>
> Signed-off-by: Michael Srba <Michael.Srba@seznam.cz>

Thanks for your patch!

> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -1443,13 +1443,26 @@ static int imx219_probe(struct i2c_client *client)
>                 return PTR_ERR(imx219->xclk);
>         }
>
> -       imx219->xclk_freq = clk_get_rate(imx219->xclk);
> -       if (imx219->xclk_freq != IMX219_XCLK_FREQ) {
> +       ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency", &imx219->xclk_freq);
> +       if (ret) {
> +               dev_err(dev, "could not get xclk frequency\n");
> +               return ret;

This breaks compatibility with existing DTBs, which do not have the
clock-frequency property.
For backwards compatibility, you should assume the default 24 MHz
instead of returning an error.

> +       }
> +
> +       /* this driver currently expects 24MHz; allow 1% tolerance */
> +       if (imx219->xclk_freq < 23760000 || imx219->xclk_freq > 24240000) {
>                 dev_err(dev, "xclk frequency not supported: %d Hz\n",
>                         imx219->xclk_freq);
>                 return -EINVAL;
>         }
>
> +       ret = clk_set_rate(imx219->xclk, imx219->xclk_freq);
> +       if (ret) {
> +               dev_err(dev, "could not set xclk frequency\n");
> +               return ret;
> +       }
> +
> +
>         ret = imx219_get_regulators(imx219);
>         if (ret) {
>                 dev_err(dev, "failed to get regulators\n");

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds