| Message ID | 20241207-imx214-v3-12-ab60af7ee915@apitzsch.eu (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: i2c: imx214: Miscellaneous cleanups and improvements | expand |
Acked-by: Ricardo Ribalda <ribalda@chromium.org> nit: media: i2c: imx214: Fix link frequency validation On Sat, Dec 7, 2024 at 9:49 PM André Apitzsch via B4 Relay <devnull+git.apitzsch.eu@kernel.org> wrote: > > From: André Apitzsch <git@apitzsch.eu> > > The driver defines IMX214_DEFAULT_LINK_FREQ 480000000, and then > IMX214_DEFAULT_PIXEL_RATE ((IMX214_DEFAULT_LINK_FREQ * 8LL) / 10), > which works out as 384MPix/s. (The 8 is 4 lanes and DDR.) > > Parsing the PLL registers with the defined 24MHz input. We're in single > PLL mode, so MIPI frequency is directly linked to pixel rate. VTCK ends > up being 1200MHz, and VTPXCK and OPPXCK both are 120MHz. Section 5.3 > "Frame rate calculation formula" says "Pixel rate > [pixels/s] = VTPXCK [MHz] * 4", so 120 * 4 = 480MPix/s, which basically > agrees with my number above. > > 3.1.4. MIPI global timing setting says "Output bitrate = OPPXCK * reg > 0x113[7:0]", so 120MHz * 10, or 1200Mbit/s. That would be a link > frequency of 600MHz due to DDR. > That also matches to 480MPix/s * 10bpp / 4 lanes / 2 for DDR. > > Keep the previous link frequency for backward compatibility. > > Signed-off-by: André Apitzsch <git@apitzsch.eu> > --- > drivers/media/i2c/imx214.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c > index 1330f13207beec0960c384681bf0b49e99fe860f..910ad03cda23345d3d10d13cd30f007954534e80 100644 > --- a/drivers/media/i2c/imx214.c > +++ b/drivers/media/i2c/imx214.c > @@ -31,7 +31,9 @@ > #define IMX214_REG_FAST_STANDBY_CTRL CCI_REG8(0x0106) > > #define IMX214_DEFAULT_CLK_FREQ 24000000 > -#define IMX214_DEFAULT_LINK_FREQ 480000000 > +#define IMX214_DEFAULT_LINK_FREQ 600000000 > +/* Keep wrong link frequency for backward compatibility */ > +#define IMX214_DEFAULT_LINK_FREQ_LEGACY 480000000 > #define IMX214_DEFAULT_PIXEL_RATE ((IMX214_DEFAULT_LINK_FREQ * 8LL) / 10) > #define IMX214_FPS 30 > > @@ -1216,18 +1218,22 @@ static int imx214_parse_fwnode(struct device *dev) > goto done; > } > Now that I think about this... We only support buf_cfg.no_of_link_frequencies ==1. Maybe you can add a check before the loop if (bus_cfg.nr_of_link_frequencies != 1) dev_warn(dev, "Only one link-frequency supported, please review your DT. Continuing anyway\n ") > - for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) > + for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) { > if (bus_cfg.link_frequencies[i] == IMX214_DEFAULT_LINK_FREQ) > break; > - > - if (i == bus_cfg.nr_of_link_frequencies) { > - dev_err_probe(dev, -EINVAL, > - "link-frequencies %d not supported, Please review your DT\n", > - IMX214_DEFAULT_LINK_FREQ); > - ret = -EINVAL; > - goto done; > + if (bus_cfg.link_frequencies[i] == IMX214_DEFAULT_LINK_FREQ_LEGACY) { > + dev_warn(dev, > + "link-frequencies %d not supported, please review your DT. Continuing anyway\n", > + IMX214_DEFAULT_LINK_FREQ); > + break; > + } > } > > + if (i == bus_cfg.nr_of_link_frequencies) > + ret = dev_err_probe(dev, -EINVAL, > + "link-frequencies %d not supported, please review your DT\n", > + IMX214_DEFAULT_LINK_FREQ); > + > done: > v4l2_fwnode_endpoint_free(&bus_cfg); > fwnode_handle_put(endpoint); > > -- > 2.47.1 > > >
diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c index 1330f13207beec0960c384681bf0b49e99fe860f..910ad03cda23345d3d10d13cd30f007954534e80 100644 --- a/drivers/media/i2c/imx214.c +++ b/drivers/media/i2c/imx214.c @@ -31,7 +31,9 @@ #define IMX214_REG_FAST_STANDBY_CTRL CCI_REG8(0x0106) #define IMX214_DEFAULT_CLK_FREQ 24000000 -#define IMX214_DEFAULT_LINK_FREQ 480000000 +#define IMX214_DEFAULT_LINK_FREQ 600000000 +/* Keep wrong link frequency for backward compatibility */ +#define IMX214_DEFAULT_LINK_FREQ_LEGACY 480000000 #define IMX214_DEFAULT_PIXEL_RATE ((IMX214_DEFAULT_LINK_FREQ * 8LL) / 10) #define IMX214_FPS 30 @@ -1216,18 +1218,22 @@ static int imx214_parse_fwnode(struct device *dev) goto done; } - for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) + for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) { if (bus_cfg.link_frequencies[i] == IMX214_DEFAULT_LINK_FREQ) break; - - if (i == bus_cfg.nr_of_link_frequencies) { - dev_err_probe(dev, -EINVAL, - "link-frequencies %d not supported, Please review your DT\n", - IMX214_DEFAULT_LINK_FREQ); - ret = -EINVAL; - goto done; + if (bus_cfg.link_frequencies[i] == IMX214_DEFAULT_LINK_FREQ_LEGACY) { + dev_warn(dev, + "link-frequencies %d not supported, please review your DT. Continuing anyway\n", + IMX214_DEFAULT_LINK_FREQ); + break; + } } + if (i == bus_cfg.nr_of_link_frequencies) + ret = dev_err_probe(dev, -EINVAL, + "link-frequencies %d not supported, please review your DT\n", + IMX214_DEFAULT_LINK_FREQ); + done: v4l2_fwnode_endpoint_free(&bus_cfg); fwnode_handle_put(endpoint);