Hi Sakari, On Tue, May 06, 2025 at 08:24:03AM +0000, Sakari Ailus wrote: > On Mon, May 05, 2025 at 11:05:56PM +0200, André Apitzsch via B4 Relay wrote: > > From: André Apitzsch <g...@apitzsch.eu> > > > > Instead rely on the rate set on the clock (using assigned-clock-rates > > etc.) > > > > Signed-off-by: André Apitzsch <g...@apitzsch.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.
Maybe I'm missing something, but I don't really see the issue here. The clock-frequency DT property is currently ignored, and this patch doesn't change that situation, does it ? The change of behaviour here is related to the assigned-clock-rates property. If that property is specified today, it will set the clock rate, and the driver will override it to 24MHz right after. With this patch, the clock rate won't be overridden. I think the risk of regression is very low here, as I don't expect systems to set assigned-clock-rates in DT to a value different than 24MHz and expect the driver to override it. > > ret = imx214_get_regulators(dev, imx214); > > if (ret < 0) > > return dev_err_probe(dev, ret, "failed to get regulators\n"); -- Regards, Laurent Pinchart
