Hi Michael, On Sat, Dec 5, 2020 at 7:36 PM <michael.srba@xxxxxxxxx> wrote: > From: Michael Srba <Michael.Srba@xxxxxxxxx> > > 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@xxxxxxxxx> 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@xxxxxxxxxxxxxx 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