| Message ID | 20230121033713.3535351-6-shravan.chippa@microchip.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: imx334: support lower bandwidth mode | expand |
Hi Shravan, On Sat, Jan 21, 2023 at 09:07:13AM +0530, shravan kumar wrote: > From: Shravan Chippa <shravan.chippa@microchip.com> > > Update pixel_rate and link frequency for 1920x1080@30 > while changing mode. > > Add dummy ctrl cases for pixel_rate and link frequency > to avoid error while changing the modes dynamically. > > Add support to handle multiple link frequencies. > > Suggested-by: Sakari Ailus <sakari.ailus@iki.fi> > Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com> > --- > drivers/media/i2c/imx334.c | 41 ++++++++++++++++++++++++++++---------- > 1 file changed, 30 insertions(+), 11 deletions(-) > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > index 309c706114d2..62b104eaa437 100644 > --- a/drivers/media/i2c/imx334.c > +++ b/drivers/media/i2c/imx334.c > @@ -49,7 +49,8 @@ > #define IMX334_INCLK_RATE 24000000 > > /* CSI2 HW configuration */ > -#define IMX334_LINK_FREQ 891000000 > +#define IMX334_LINK_FREQ_891M 891000000 > +#define IMX334_LINK_FREQ_445M 445500000 > #define IMX334_NUM_DATA_LANES 4 > > #define IMX334_REG_MIN 0x00 > @@ -139,12 +140,14 @@ struct imx334 { > u32 vblank; > const struct imx334_mode *cur_mode; > struct mutex mutex; > + unsigned long menu_skip_mask; > u32 cur_code; > bool streaming; > }; > > static const s64 link_freq[] = { > - IMX334_LINK_FREQ, > + IMX334_LINK_FREQ_891M, > + IMX334_LINK_FREQ_445M, > }; > > /* Sensor mode registers for 1920x1080@30fps */ > @@ -468,7 +471,7 @@ static const struct imx334_mode supported_modes[] = { > .vblank_min = 45, > .vblank_max = 132840, > .pclk = 297000000, > - .link_freq_idx = 0, > + .link_freq_idx = 1, > .reg_list = { > .num_of_regs = ARRAY_SIZE(mode_1920x1080_regs), > .regs = mode_1920x1080_regs, > @@ -598,6 +601,11 @@ static int imx334_update_controls(struct imx334 *imx334, > if (ret) > return ret; > > + ret = __v4l2_ctrl_modify_range(imx334->pclk_ctrl, mode->pclk, > + mode->pclk, 1, mode->pclk); > + if (ret) > + return ret; > + > ret = __v4l2_ctrl_modify_range(imx334->hblank_ctrl, mode->hblank, > mode->hblank, 1, mode->hblank); > if (ret) > @@ -698,6 +706,8 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl) > pm_runtime_put(imx334->dev); > > break; > + case V4L2_CID_PIXEL_RATE: > + case V4L2_CID_LINK_FREQ: > case V4L2_CID_HBLANK: > ret = 0; > break; > @@ -1047,7 +1057,7 @@ static int imx334_parse_hw_config(struct imx334 *imx334) > struct fwnode_handle *ep; > unsigned long rate; > int ret; > - int i; > + int i, j; unsigned int would be nicer. > > if (!fwnode) > return -ENXIO; > @@ -1097,11 +1107,20 @@ static int imx334_parse_hw_config(struct imx334 *imx334) > goto done_endpoint_free; > } > > - for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) > - if (bus_cfg.link_frequencies[i] == IMX334_LINK_FREQ) > + for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) { > + for (j = 0; j < ARRAY_SIZE(link_freq); j++) { > + if (bus_cfg.link_frequencies[i] == link_freq[j]) { > + set_bit(j, &imx334->menu_skip_mask); Is there a guarantee that you'll only be using the modes with the listed frequencies? I don't see one but I might have missed it. > + break; > + } > + } > + > + if (j == ARRAY_SIZE(link_freq)) { > + ret = dev_err_probe(imx334->dev, -EINVAL, > + "no supported link freq found\n"); > goto done_endpoint_free; > - > - ret = -EINVAL; > + } > + } > > done_endpoint_free: > v4l2_fwnode_endpoint_free(&bus_cfg); > @@ -1232,10 +1251,10 @@ static int imx334_init_controls(struct imx334 *imx334) > imx334->link_freq_ctrl = v4l2_ctrl_new_int_menu(ctrl_hdlr, > &imx334_ctrl_ops, > V4L2_CID_LINK_FREQ, > - ARRAY_SIZE(link_freq) - > - 1, > - mode->link_freq_idx, > + __fls(imx334->menu_skip_mask), > + __ffs(imx334->menu_skip_mask), > link_freq); > + > if (imx334->link_freq_ctrl) > imx334->link_freq_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; >
Hi Sakari, > -----Original Message----- > From: Sakari Ailus <sakari.ailus@iki.fi> > Sent: 24 January 2023 04:33 AM > To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com> > Cc: paul.j.murphy@intel.com; daniele.alessandrelli@intel.com; > mchehab@kernel.org; krzysztof.kozlowski+dt@linaro.org; > shawnguo@kernel.org; s.hauer@pengutronix.de; festevam@gmail.com; > kernel@pengutronix.de; linux-imx@nxp.com; linux-media@vger.kernel.org; > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- > kernel@lists.infradead.org > Subject: Re: [PATCH RESEND v10 5/5] media: i2c: imx334: update pixel and link > frequency > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > Hi Shravan, > > On Sat, Jan 21, 2023 at 09:07:13AM +0530, shravan kumar wrote: > > From: Shravan Chippa <shravan.chippa@microchip.com> > > > > Update pixel_rate and link frequency for 1920x1080@30 while changing > > mode. > > > > Add dummy ctrl cases for pixel_rate and link frequency to avoid error > > while changing the modes dynamically. > > > > Add support to handle multiple link frequencies. > > > > Suggested-by: Sakari Ailus <sakari.ailus@iki.fi> > > Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com> > > --- > > drivers/media/i2c/imx334.c | 41 > > ++++++++++++++++++++++++++++---------- > > 1 file changed, 30 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > > index 309c706114d2..62b104eaa437 100644 > > --- a/drivers/media/i2c/imx334.c > > +++ b/drivers/media/i2c/imx334.c > > @@ -49,7 +49,8 @@ > > #define IMX334_INCLK_RATE 24000000 > > > > /* CSI2 HW configuration */ > > -#define IMX334_LINK_FREQ 891000000 > > +#define IMX334_LINK_FREQ_891M 891000000 > > +#define IMX334_LINK_FREQ_445M 445500000 > > #define IMX334_NUM_DATA_LANES 4 > > > > #define IMX334_REG_MIN 0x00 > > @@ -139,12 +140,14 @@ struct imx334 { > > u32 vblank; > > const struct imx334_mode *cur_mode; > > struct mutex mutex; > > + unsigned long menu_skip_mask; > > u32 cur_code; > > bool streaming; > > }; > > > > static const s64 link_freq[] = { > > - IMX334_LINK_FREQ, > > + IMX334_LINK_FREQ_891M, > > + IMX334_LINK_FREQ_445M, > > }; > > > > /* Sensor mode registers for 1920x1080@30fps */ @@ -468,7 +471,7 @@ > > static const struct imx334_mode supported_modes[] = { > > .vblank_min = 45, > > .vblank_max = 132840, > > .pclk = 297000000, > > - .link_freq_idx = 0, > > + .link_freq_idx = 1, > > .reg_list = { > > .num_of_regs = ARRAY_SIZE(mode_1920x1080_regs), > > .regs = mode_1920x1080_regs, @@ -598,6 +601,11 > > @@ static int imx334_update_controls(struct imx334 *imx334, > > if (ret) > > return ret; > > > > + ret = __v4l2_ctrl_modify_range(imx334->pclk_ctrl, mode->pclk, > > + mode->pclk, 1, mode->pclk); > > + if (ret) > > + return ret; > > + > > ret = __v4l2_ctrl_modify_range(imx334->hblank_ctrl, mode->hblank, > > mode->hblank, 1, mode->hblank); > > if (ret) > > @@ -698,6 +706,8 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl) > > pm_runtime_put(imx334->dev); > > > > break; > > + case V4L2_CID_PIXEL_RATE: > > + case V4L2_CID_LINK_FREQ: > > case V4L2_CID_HBLANK: > > ret = 0; > > break; > > @@ -1047,7 +1057,7 @@ static int imx334_parse_hw_config(struct imx334 > *imx334) > > struct fwnode_handle *ep; > > unsigned long rate; > > int ret; > > - int i; > > + int i, j; > > unsigned int would be nicer. I will change. > > > > > if (!fwnode) > > return -ENXIO; > > @@ -1097,11 +1107,20 @@ static int imx334_parse_hw_config(struct imx334 > *imx334) > > goto done_endpoint_free; > > } > > > > - for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) > > - if (bus_cfg.link_frequencies[i] == IMX334_LINK_FREQ) > > + for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) { > > + for (j = 0; j < ARRAY_SIZE(link_freq); j++) { > > + if (bus_cfg.link_frequencies[i] == link_freq[j]) { > > + set_bit(j, &imx334->menu_skip_mask); > > Is there a guarantee that you'll only be using the modes with the listed > frequencies? I don't see one but I might have missed it. If I understand it correctly, the question here is, the listed freqeunceis and modes are one to one mapped? Then yes. Thanks. shravan > > > + break; > > + } > > + } > > + > > + if (j == ARRAY_SIZE(link_freq)) { > > + ret = dev_err_probe(imx334->dev, -EINVAL, > > + "no supported link freq > > + found\n"); > > goto done_endpoint_free; > > - > > - ret = -EINVAL; > > + } > > + } > > > > done_endpoint_free: > > v4l2_fwnode_endpoint_free(&bus_cfg); > > @@ -1232,10 +1251,10 @@ static int imx334_init_controls(struct imx334 > *imx334) > > imx334->link_freq_ctrl = v4l2_ctrl_new_int_menu(ctrl_hdlr, > > &imx334_ctrl_ops, > > V4L2_CID_LINK_FREQ, > > - ARRAY_SIZE(link_freq) - > > - 1, > > - mode->link_freq_idx, > > + __fls(imx334->menu_skip_mask), > > + > > + __ffs(imx334->menu_skip_mask), > > link_freq); > > + > > if (imx334->link_freq_ctrl) > > imx334->link_freq_ctrl->flags |= > > V4L2_CTRL_FLAG_READ_ONLY; > > > > -- > Kind regards, > > Sakari Ailus
Hi Shravan, On Tue, Jan 24, 2023 at 05:34:02AM +0000, Shravan.Chippa@microchip.com wrote: > Hi Sakari, > > > -----Original Message----- > > From: Sakari Ailus <sakari.ailus@iki.fi> > > Sent: 24 January 2023 04:33 AM > > To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com> > > Cc: paul.j.murphy@intel.com; daniele.alessandrelli@intel.com; > > mchehab@kernel.org; krzysztof.kozlowski+dt@linaro.org; > > shawnguo@kernel.org; s.hauer@pengutronix.de; festevam@gmail.com; > > kernel@pengutronix.de; linux-imx@nxp.com; linux-media@vger.kernel.org; > > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- > > kernel@lists.infradead.org > > Subject: Re: [PATCH RESEND v10 5/5] media: i2c: imx334: update pixel and link > > frequency > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > > content is safe > > > > Hi Shravan, > > > > On Sat, Jan 21, 2023 at 09:07:13AM +0530, shravan kumar wrote: > > > From: Shravan Chippa <shravan.chippa@microchip.com> > > > > > > Update pixel_rate and link frequency for 1920x1080@30 while changing > > > mode. > > > > > > Add dummy ctrl cases for pixel_rate and link frequency to avoid error > > > while changing the modes dynamically. > > > > > > Add support to handle multiple link frequencies. > > > > > > Suggested-by: Sakari Ailus <sakari.ailus@iki.fi> > > > Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com> > > > --- > > > drivers/media/i2c/imx334.c | 41 > > > ++++++++++++++++++++++++++++---------- > > > 1 file changed, 30 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > > > index 309c706114d2..62b104eaa437 100644 > > > --- a/drivers/media/i2c/imx334.c > > > +++ b/drivers/media/i2c/imx334.c > > > @@ -49,7 +49,8 @@ > > > #define IMX334_INCLK_RATE 24000000 > > > > > > /* CSI2 HW configuration */ > > > -#define IMX334_LINK_FREQ 891000000 > > > +#define IMX334_LINK_FREQ_891M 891000000 > > > +#define IMX334_LINK_FREQ_445M 445500000 > > > #define IMX334_NUM_DATA_LANES 4 > > > > > > #define IMX334_REG_MIN 0x00 > > > @@ -139,12 +140,14 @@ struct imx334 { > > > u32 vblank; > > > const struct imx334_mode *cur_mode; > > > struct mutex mutex; > > > + unsigned long menu_skip_mask; > > > u32 cur_code; > > > bool streaming; > > > }; > > > > > > static const s64 link_freq[] = { > > > - IMX334_LINK_FREQ, > > > + IMX334_LINK_FREQ_891M, > > > + IMX334_LINK_FREQ_445M, > > > }; > > > > > > /* Sensor mode registers for 1920x1080@30fps */ @@ -468,7 +471,7 @@ > > > static const struct imx334_mode supported_modes[] = { > > > .vblank_min = 45, > > > .vblank_max = 132840, > > > .pclk = 297000000, > > > - .link_freq_idx = 0, > > > + .link_freq_idx = 1, > > > .reg_list = { > > > .num_of_regs = ARRAY_SIZE(mode_1920x1080_regs), > > > .regs = mode_1920x1080_regs, @@ -598,6 +601,11 > > > @@ static int imx334_update_controls(struct imx334 *imx334, > > > if (ret) > > > return ret; > > > > > > + ret = __v4l2_ctrl_modify_range(imx334->pclk_ctrl, mode->pclk, > > > + mode->pclk, 1, mode->pclk); > > > + if (ret) > > > + return ret; > > > + > > > ret = __v4l2_ctrl_modify_range(imx334->hblank_ctrl, mode->hblank, > > > mode->hblank, 1, mode->hblank); > > > if (ret) > > > @@ -698,6 +706,8 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl) > > > pm_runtime_put(imx334->dev); > > > > > > break; > > > + case V4L2_CID_PIXEL_RATE: > > > + case V4L2_CID_LINK_FREQ: > > > case V4L2_CID_HBLANK: > > > ret = 0; > > > break; > > > @@ -1047,7 +1057,7 @@ static int imx334_parse_hw_config(struct imx334 > > *imx334) > > > struct fwnode_handle *ep; > > > unsigned long rate; > > > int ret; > > > - int i; > > > + int i, j; > > > > unsigned int would be nicer. > I will change. > > > > > > > > if (!fwnode) > > > return -ENXIO; > > > @@ -1097,11 +1107,20 @@ static int imx334_parse_hw_config(struct imx334 > > *imx334) > > > goto done_endpoint_free; > > > } > > > > > > - for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) > > > - if (bus_cfg.link_frequencies[i] == IMX334_LINK_FREQ) > > > + for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) { > > > + for (j = 0; j < ARRAY_SIZE(link_freq); j++) { > > > + if (bus_cfg.link_frequencies[i] == link_freq[j]) { > > > + set_bit(j, &imx334->menu_skip_mask); > > > > Is there a guarantee that you'll only be using the modes with the listed > > frequencies? I don't see one but I might have missed it. > > If I understand it correctly, the question here is, the listed > freqeunceis and modes are one to one mapped? Then yes. I don't see this being checked in imx334_set_pad_format(), for instance. If a frequency isn't in DT, the driver isn't supposed to be using it either. > > Thanks. > shravan > > > > > + break; > > > + } > > > + } > > > + > > > + if (j == ARRAY_SIZE(link_freq)) { > > > + ret = dev_err_probe(imx334->dev, -EINVAL, > > > + "no supported link freq > > > + found\n"); > > > goto done_endpoint_free; > > > - > > > - ret = -EINVAL; > > > + } > > > + } > > > > > > done_endpoint_free: > > > v4l2_fwnode_endpoint_free(&bus_cfg); > > > @@ -1232,10 +1251,10 @@ static int imx334_init_controls(struct imx334 > > *imx334) > > > imx334->link_freq_ctrl = v4l2_ctrl_new_int_menu(ctrl_hdlr, > > > &imx334_ctrl_ops, > > > V4L2_CID_LINK_FREQ, > > > - ARRAY_SIZE(link_freq) - > > > - 1, > > > - mode->link_freq_idx, > > > + __fls(imx334->menu_skip_mask), > > > + > > > + __ffs(imx334->menu_skip_mask), > > > link_freq); > > > + > > > if (imx334->link_freq_ctrl) > > > imx334->link_freq_ctrl->flags |= > > > V4L2_CTRL_FLAG_READ_ONLY;
Hi Sakari, > -----Original Message----- > From: Sakari Ailus <sakari.ailus@iki.fi> > Sent: 25 January 2023 03:12 PM > To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com> > Cc: paul.j.murphy@intel.com; daniele.alessandrelli@intel.com; > mchehab@kernel.org; krzysztof.kozlowski+dt@linaro.org; > shawnguo@kernel.org; s.hauer@pengutronix.de; festevam@gmail.com; > kernel@pengutronix.de; linux-imx@nxp.com; linux-media@vger.kernel.org; > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- > kernel@lists.infradead.org > Subject: Re: [PATCH RESEND v10 5/5] media: i2c: imx334: update pixel and link > frequency > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > Hi Shravan, > > On Tue, Jan 24, 2023 at 05:34:02AM +0000, Shravan.Chippa@microchip.com > wrote: > > Hi Sakari, > > > > > -----Original Message----- > > > From: Sakari Ailus <sakari.ailus@iki.fi> > > > Sent: 24 January 2023 04:33 AM > > > To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com> > > > Cc: paul.j.murphy@intel.com; daniele.alessandrelli@intel.com; > > > mchehab@kernel.org; krzysztof.kozlowski+dt@linaro.org; > > > shawnguo@kernel.org; s.hauer@pengutronix.de; festevam@gmail.com; > > > kernel@pengutronix.de; linux-imx@nxp.com; > > > linux-media@vger.kernel.org; linux-kernel@vger.kernel.org; > > > devicetree@vger.kernel.org; linux-arm- kernel@lists.infradead.org > > > Subject: Re: [PATCH RESEND v10 5/5] media: i2c: imx334: update pixel > > > and link frequency > > > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you > > > know the content is safe > > > > > > Hi Shravan, > > > > > > On Sat, Jan 21, 2023 at 09:07:13AM +0530, shravan kumar wrote: > > > > From: Shravan Chippa <shravan.chippa@microchip.com> > > > > > > > > Update pixel_rate and link frequency for 1920x1080@30 while > > > > changing mode. > > > > > > > > Add dummy ctrl cases for pixel_rate and link frequency to avoid > > > > error while changing the modes dynamically. > > > > > > > > Add support to handle multiple link frequencies. > > > > > > > > Suggested-by: Sakari Ailus <sakari.ailus@iki.fi> > > > > Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com> > > > > --- > > > > drivers/media/i2c/imx334.c | 41 > > > > ++++++++++++++++++++++++++++---------- > > > > 1 file changed, 30 insertions(+), 11 deletions(-) > > > > > > > > diff --git a/drivers/media/i2c/imx334.c > > > > b/drivers/media/i2c/imx334.c index 309c706114d2..62b104eaa437 > > > > 100644 > > > > --- a/drivers/media/i2c/imx334.c > > > > +++ b/drivers/media/i2c/imx334.c > > > > @@ -49,7 +49,8 @@ > > > > #define IMX334_INCLK_RATE 24000000 > > > > > > > > /* CSI2 HW configuration */ > > > > -#define IMX334_LINK_FREQ 891000000 > > > > +#define IMX334_LINK_FREQ_891M 891000000 > > > > +#define IMX334_LINK_FREQ_445M 445500000 > > > > #define IMX334_NUM_DATA_LANES 4 > > > > > > > > #define IMX334_REG_MIN 0x00 > > > > @@ -139,12 +140,14 @@ struct imx334 { > > > > u32 vblank; > > > > const struct imx334_mode *cur_mode; > > > > struct mutex mutex; > > > > + unsigned long menu_skip_mask; > > > > u32 cur_code; > > > > bool streaming; > > > > }; > > > > > > > > static const s64 link_freq[] = { > > > > - IMX334_LINK_FREQ, > > > > + IMX334_LINK_FREQ_891M, > > > > + IMX334_LINK_FREQ_445M, > > > > }; > > > > > > > > /* Sensor mode registers for 1920x1080@30fps */ @@ -468,7 +471,7 > > > > @@ static const struct imx334_mode supported_modes[] = { > > > > .vblank_min = 45, > > > > .vblank_max = 132840, > > > > .pclk = 297000000, > > > > - .link_freq_idx = 0, > > > > + .link_freq_idx = 1, > > > > .reg_list = { > > > > .num_of_regs = ARRAY_SIZE(mode_1920x1080_regs), > > > > .regs = mode_1920x1080_regs, @@ -598,6 > > > > +601,11 @@ static int imx334_update_controls(struct imx334 *imx334, > > > > if (ret) > > > > return ret; > > > > > > > > + ret = __v4l2_ctrl_modify_range(imx334->pclk_ctrl, mode->pclk, > > > > + mode->pclk, 1, mode->pclk); > > > > + if (ret) > > > > + return ret; > > > > + > > > > ret = __v4l2_ctrl_modify_range(imx334->hblank_ctrl, mode->hblank, > > > > mode->hblank, 1, mode->hblank); > > > > if (ret) > > > > @@ -698,6 +706,8 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl) > > > > pm_runtime_put(imx334->dev); > > > > > > > > break; > > > > + case V4L2_CID_PIXEL_RATE: > > > > + case V4L2_CID_LINK_FREQ: > > > > case V4L2_CID_HBLANK: > > > > ret = 0; > > > > break; > > > > @@ -1047,7 +1057,7 @@ static int imx334_parse_hw_config(struct > > > > imx334 > > > *imx334) > > > > struct fwnode_handle *ep; > > > > unsigned long rate; > > > > int ret; > > > > - int i; > > > > + int i, j; > > > > > > unsigned int would be nicer. > > I will change. > > > > > > > > > > > if (!fwnode) > > > > return -ENXIO; > > > > @@ -1097,11 +1107,20 @@ static int imx334_parse_hw_config(struct > > > > imx334 > > > *imx334) > > > > goto done_endpoint_free; > > > > } > > > > > > > > - for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) > > > > - if (bus_cfg.link_frequencies[i] == IMX334_LINK_FREQ) > > > > + for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) { > > > > + for (j = 0; j < ARRAY_SIZE(link_freq); j++) { > > > > + if (bus_cfg.link_frequencies[i] == link_freq[j]) { > > > > + set_bit(j, &imx334->menu_skip_mask); > > > > > > Is there a guarantee that you'll only be using the modes with the > > > listed frequencies? I don't see one but I might have missed it. > > > > If I understand it correctly, the question here is, the listed > > freqeunceis and modes are one to one mapped? Then yes. > > I don't see this being checked in imx334_set_pad_format(), for instance. > > If a frequency isn't in DT, the driver isn't supposed to be using it either. Yes, there is no check. But, if a frequency is not in DT, the driver will not add in menu items. So, the function imx334_set_pad_format() -> imx334_update_controls() fails, if we set the frequencies which are not there in the DT or menu items. Thanks, Shravan > > > > > Thanks. > > shravan > > > > > > > + break; > > > > + } > > > > + } > > > > + > > > > + if (j == ARRAY_SIZE(link_freq)) { > > > > + ret = dev_err_probe(imx334->dev, -EINVAL, > > > > + "no supported link freq > > > > + found\n"); > > > > goto done_endpoint_free; > > > > - > > > > - ret = -EINVAL; > > > > + } > > > > + } > > > > > > > > done_endpoint_free: > > > > v4l2_fwnode_endpoint_free(&bus_cfg); > > > > @@ -1232,10 +1251,10 @@ static int imx334_init_controls(struct > > > > imx334 > > > *imx334) > > > > imx334->link_freq_ctrl = v4l2_ctrl_new_int_menu(ctrl_hdlr, > > > > &imx334_ctrl_ops, > > > > V4L2_CID_LINK_FREQ, > > > > - ARRAY_SIZE(link_freq) - > > > > - 1, > > > > - mode->link_freq_idx, > > > > + > > > > + __fls(imx334->menu_skip_mask), > > > > + > > > > + __ffs(imx334->menu_skip_mask), > > > > link_freq); > > > > + > > > > if (imx334->link_freq_ctrl) > > > > imx334->link_freq_ctrl->flags |= > > > > V4L2_CTRL_FLAG_READ_ONLY; > > -- > Kind regards, > > Sakari Ailus
Hi Sakari, > -----Original Message----- > From: shravan Chippa - I35088 > Sent: 27 January 2023 05:40 AM > To: Sakari Ailus <sakari.ailus@iki.fi> > Cc: paul.j.murphy@intel.com; daniele.alessandrelli@intel.com; > mchehab@kernel.org; krzysztof.kozlowski+dt@linaro.org; > shawnguo@kernel.org; s.hauer@pengutronix.de; festevam@gmail.com; > kernel@pengutronix.de; linux-imx@nxp.com; linux-media@vger.kernel.org; > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- > kernel@lists.infradead.org > Subject: RE: [PATCH RESEND v10 5/5] media: i2c: imx334: update pixel and > link frequency > > Hi Sakari, > > > -----Original Message----- > > From: Sakari Ailus <sakari.ailus@iki.fi> > > Sent: 25 January 2023 03:12 PM > > To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com> > > Cc: paul.j.murphy@intel.com; daniele.alessandrelli@intel.com; > > mchehab@kernel.org; krzysztof.kozlowski+dt@linaro.org; > > shawnguo@kernel.org; s.hauer@pengutronix.de; festevam@gmail.com; > > kernel@pengutronix.de; linux-imx@nxp.com; linux- > media@vger.kernel.org; > > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- > > kernel@lists.infradead.org > > Subject: Re: [PATCH RESEND v10 5/5] media: i2c: imx334: update pixel > > and link frequency > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know > > the content is safe > > > > Hi Shravan, > > > > On Tue, Jan 24, 2023 at 05:34:02AM +0000, Shravan.Chippa@microchip.com > > wrote: > > > Hi Sakari, > > > > > > > -----Original Message----- > > > > From: Sakari Ailus <sakari.ailus@iki.fi> > > > > Sent: 24 January 2023 04:33 AM > > > > To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com> > > > > Cc: paul.j.murphy@intel.com; daniele.alessandrelli@intel.com; > > > > mchehab@kernel.org; krzysztof.kozlowski+dt@linaro.org; > > > > shawnguo@kernel.org; s.hauer@pengutronix.de; > festevam@gmail.com; > > > > kernel@pengutronix.de; linux-imx@nxp.com; > > > > linux-media@vger.kernel.org; linux-kernel@vger.kernel.org; > > > > devicetree@vger.kernel.org; linux-arm- kernel@lists.infradead.org > > > > Subject: Re: [PATCH RESEND v10 5/5] media: i2c: imx334: update > > > > pixel and link frequency > > > > > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you > > > > know the content is safe > > > > > > > > Hi Shravan, > > > > > > > > On Sat, Jan 21, 2023 at 09:07:13AM +0530, shravan kumar wrote: > > > > > From: Shravan Chippa <shravan.chippa@microchip.com> > > > > > > > > > > Update pixel_rate and link frequency for 1920x1080@30 while > > > > > changing mode. > > > > > > > > > > Add dummy ctrl cases for pixel_rate and link frequency to avoid > > > > > error while changing the modes dynamically. > > > > > > > > > > Add support to handle multiple link frequencies. > > > > > > > > > > Suggested-by: Sakari Ailus <sakari.ailus@iki.fi> > > > > > Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com> > > > > > --- > > > > > drivers/media/i2c/imx334.c | 41 > > > > > ++++++++++++++++++++++++++++---------- > > > > > 1 file changed, 30 insertions(+), 11 deletions(-) > > > > > > > > > > diff --git a/drivers/media/i2c/imx334.c > > > > > b/drivers/media/i2c/imx334.c index 309c706114d2..62b104eaa437 > > > > > 100644 > > > > > --- a/drivers/media/i2c/imx334.c > > > > > +++ b/drivers/media/i2c/imx334.c > > > > > @@ -49,7 +49,8 @@ > > > > > #define IMX334_INCLK_RATE 24000000 > > > > > > > > > > /* CSI2 HW configuration */ > > > > > -#define IMX334_LINK_FREQ 891000000 > > > > > +#define IMX334_LINK_FREQ_891M 891000000 > > > > > +#define IMX334_LINK_FREQ_445M 445500000 > > > > > #define IMX334_NUM_DATA_LANES 4 > > > > > > > > > > #define IMX334_REG_MIN 0x00 > > > > > @@ -139,12 +140,14 @@ struct imx334 { > > > > > u32 vblank; > > > > > const struct imx334_mode *cur_mode; > > > > > struct mutex mutex; > > > > > + unsigned long menu_skip_mask; > > > > > u32 cur_code; > > > > > bool streaming; > > > > > }; > > > > > > > > > > static const s64 link_freq[] = { > > > > > - IMX334_LINK_FREQ, > > > > > + IMX334_LINK_FREQ_891M, > > > > > + IMX334_LINK_FREQ_445M, > > > > > }; > > > > > > > > > > /* Sensor mode registers for 1920x1080@30fps */ @@ -468,7 > > > > > +471,7 @@ static const struct imx334_mode supported_modes[] = { > > > > > .vblank_min = 45, > > > > > .vblank_max = 132840, > > > > > .pclk = 297000000, > > > > > - .link_freq_idx = 0, > > > > > + .link_freq_idx = 1, > > > > > .reg_list = { > > > > > .num_of_regs = ARRAY_SIZE(mode_1920x1080_regs), > > > > > .regs = mode_1920x1080_regs, @@ -598,6 > > > > > +601,11 @@ static int imx334_update_controls(struct imx334 > > > > > +*imx334, > > > > > if (ret) > > > > > return ret; > > > > > > > > > > + ret = __v4l2_ctrl_modify_range(imx334->pclk_ctrl, mode->pclk, > > > > > + mode->pclk, 1, mode->pclk); > > > > > + if (ret) > > > > > + return ret; > > > > > + > > > > > ret = __v4l2_ctrl_modify_range(imx334->hblank_ctrl, mode- > >hblank, > > > > > mode->hblank, 1, mode->hblank); > > > > > if (ret) > > > > > @@ -698,6 +706,8 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl) > > > > > pm_runtime_put(imx334->dev); > > > > > > > > > > break; > > > > > + case V4L2_CID_PIXEL_RATE: > > > > > + case V4L2_CID_LINK_FREQ: > > > > > case V4L2_CID_HBLANK: > > > > > ret = 0; > > > > > break; > > > > > @@ -1047,7 +1057,7 @@ static int imx334_parse_hw_config(struct > > > > > imx334 > > > > *imx334) > > > > > struct fwnode_handle *ep; > > > > > unsigned long rate; > > > > > int ret; > > > > > - int i; > > > > > + int i, j; > > > > > > > > unsigned int would be nicer. > > > I will change. > > > > > > > > > > > > > > if (!fwnode) > > > > > return -ENXIO; > > > > > @@ -1097,11 +1107,20 @@ static int imx334_parse_hw_config(struct > > > > > imx334 > > > > *imx334) > > > > > goto done_endpoint_free; > > > > > } > > > > > > > > > > - for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) > > > > > - if (bus_cfg.link_frequencies[i] == IMX334_LINK_FREQ) > > > > > + for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) { > > > > > + for (j = 0; j < ARRAY_SIZE(link_freq); j++) { > > > > > + if (bus_cfg.link_frequencies[i] == link_freq[j]) { > > > > > + set_bit(j, > > > > > + &imx334->menu_skip_mask); > > > > > > > > Is there a guarantee that you'll only be using the modes with the > > > > listed frequencies? I don't see one but I might have missed it. > > > > > > If I understand it correctly, the question here is, the listed > > > freqeunceis and modes are one to one mapped? Then yes. > > > > I don't see this being checked in imx334_set_pad_format(), for instance. > > > > If a frequency isn't in DT, the driver isn't supposed to be using it either. > > Yes, there is no check. > > But, if a frequency is not in DT, the driver will not add in menu items. > So, the function imx334_set_pad_format() -> imx334_update_controls() > fails, if we set the frequencies which are not there in the DT or menu items. > Are you ok with the above explanation or any changes you are expecting? Please do let me know if there are any changes needed. I am planning to send the next version. Thanks, Shravan > > Thanks, > Shravan > > > > > > > > > Thanks. > > > shravan > > > > > > > > > + break; > > > > > + } > > > > > + } > > > > > + > > > > > + if (j == ARRAY_SIZE(link_freq)) { > > > > > + ret = dev_err_probe(imx334->dev, -EINVAL, > > > > > + "no supported link > > > > > + freq found\n"); > > > > > goto done_endpoint_free; > > > > > - > > > > > - ret = -EINVAL; > > > > > + } > > > > > + } > > > > > > > > > > done_endpoint_free: > > > > > v4l2_fwnode_endpoint_free(&bus_cfg); > > > > > @@ -1232,10 +1251,10 @@ static int imx334_init_controls(struct > > > > > imx334 > > > > *imx334) > > > > > imx334->link_freq_ctrl = v4l2_ctrl_new_int_menu(ctrl_hdlr, > > > > > &imx334_ctrl_ops, > > > > > V4L2_CID_LINK_FREQ, > > > > > - ARRAY_SIZE(link_freq) - > > > > > - 1, > > > > > - mode->link_freq_idx, > > > > > + > > > > > + __fls(imx334->menu_skip_mask), > > > > > + > > > > > + __ffs(imx334->menu_skip_mask), > > > > > > > > > > link_freq); > > > > > + > > > > > if (imx334->link_freq_ctrl) > > > > > imx334->link_freq_ctrl->flags |= > > > > > V4L2_CTRL_FLAG_READ_ONLY; > > > > -- > > Kind regards, > > > > Sakari Ailus
Hi Shravan, On Mon, Feb 06, 2023 at 04:43:42AM +0000, Shravan.Chippa@microchip.com wrote: > Hi Sakari, > > > > -----Original Message----- > > From: shravan Chippa - I35088 > > Sent: 27 January 2023 05:40 AM > > To: Sakari Ailus <sakari.ailus@iki.fi> > > Cc: paul.j.murphy@intel.com; daniele.alessandrelli@intel.com; > > mchehab@kernel.org; krzysztof.kozlowski+dt@linaro.org; > > shawnguo@kernel.org; s.hauer@pengutronix.de; festevam@gmail.com; > > kernel@pengutronix.de; linux-imx@nxp.com; linux-media@vger.kernel.org; > > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- > > kernel@lists.infradead.org > > Subject: RE: [PATCH RESEND v10 5/5] media: i2c: imx334: update pixel and > > link frequency > > > > Hi Sakari, > > > > > -----Original Message----- > > > From: Sakari Ailus <sakari.ailus@iki.fi> > > > Sent: 25 January 2023 03:12 PM > > > To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com> > > > Cc: paul.j.murphy@intel.com; daniele.alessandrelli@intel.com; > > > mchehab@kernel.org; krzysztof.kozlowski+dt@linaro.org; > > > shawnguo@kernel.org; s.hauer@pengutronix.de; festevam@gmail.com; > > > kernel@pengutronix.de; linux-imx@nxp.com; linux- > > media@vger.kernel.org; > > > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- > > > kernel@lists.infradead.org > > > Subject: Re: [PATCH RESEND v10 5/5] media: i2c: imx334: update pixel > > > and link frequency > > > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know > > > the content is safe > > > > > > Hi Shravan, > > > > > > On Tue, Jan 24, 2023 at 05:34:02AM +0000, Shravan.Chippa@microchip.com > > > wrote: > > > > Hi Sakari, > > > > > > > > > -----Original Message----- > > > > > From: Sakari Ailus <sakari.ailus@iki.fi> > > > > > Sent: 24 January 2023 04:33 AM > > > > > To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com> > > > > > Cc: paul.j.murphy@intel.com; daniele.alessandrelli@intel.com; > > > > > mchehab@kernel.org; krzysztof.kozlowski+dt@linaro.org; > > > > > shawnguo@kernel.org; s.hauer@pengutronix.de; > > festevam@gmail.com; > > > > > kernel@pengutronix.de; linux-imx@nxp.com; > > > > > linux-media@vger.kernel.org; linux-kernel@vger.kernel.org; > > > > > devicetree@vger.kernel.org; linux-arm- kernel@lists.infradead.org > > > > > Subject: Re: [PATCH RESEND v10 5/5] media: i2c: imx334: update > > > > > pixel and link frequency > > > > > > > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you > > > > > know the content is safe > > > > > > > > > > Hi Shravan, > > > > > > > > > > On Sat, Jan 21, 2023 at 09:07:13AM +0530, shravan kumar wrote: > > > > > > From: Shravan Chippa <shravan.chippa@microchip.com> > > > > > > > > > > > > Update pixel_rate and link frequency for 1920x1080@30 while > > > > > > changing mode. > > > > > > > > > > > > Add dummy ctrl cases for pixel_rate and link frequency to avoid > > > > > > error while changing the modes dynamically. > > > > > > > > > > > > Add support to handle multiple link frequencies. > > > > > > > > > > > > Suggested-by: Sakari Ailus <sakari.ailus@iki.fi> > > > > > > Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com> > > > > > > --- > > > > > > drivers/media/i2c/imx334.c | 41 > > > > > > ++++++++++++++++++++++++++++---------- > > > > > > 1 file changed, 30 insertions(+), 11 deletions(-) > > > > > > > > > > > > diff --git a/drivers/media/i2c/imx334.c > > > > > > b/drivers/media/i2c/imx334.c index 309c706114d2..62b104eaa437 > > > > > > 100644 > > > > > > --- a/drivers/media/i2c/imx334.c > > > > > > +++ b/drivers/media/i2c/imx334.c > > > > > > @@ -49,7 +49,8 @@ > > > > > > #define IMX334_INCLK_RATE 24000000 > > > > > > > > > > > > /* CSI2 HW configuration */ > > > > > > -#define IMX334_LINK_FREQ 891000000 > > > > > > +#define IMX334_LINK_FREQ_891M 891000000 > > > > > > +#define IMX334_LINK_FREQ_445M 445500000 > > > > > > #define IMX334_NUM_DATA_LANES 4 > > > > > > > > > > > > #define IMX334_REG_MIN 0x00 > > > > > > @@ -139,12 +140,14 @@ struct imx334 { > > > > > > u32 vblank; > > > > > > const struct imx334_mode *cur_mode; > > > > > > struct mutex mutex; > > > > > > + unsigned long menu_skip_mask; > > > > > > u32 cur_code; > > > > > > bool streaming; > > > > > > }; > > > > > > > > > > > > static const s64 link_freq[] = { > > > > > > - IMX334_LINK_FREQ, > > > > > > + IMX334_LINK_FREQ_891M, > > > > > > + IMX334_LINK_FREQ_445M, > > > > > > }; > > > > > > > > > > > > /* Sensor mode registers for 1920x1080@30fps */ @@ -468,7 > > > > > > +471,7 @@ static const struct imx334_mode supported_modes[] = { > > > > > > .vblank_min = 45, > > > > > > .vblank_max = 132840, > > > > > > .pclk = 297000000, > > > > > > - .link_freq_idx = 0, > > > > > > + .link_freq_idx = 1, > > > > > > .reg_list = { > > > > > > .num_of_regs = ARRAY_SIZE(mode_1920x1080_regs), > > > > > > .regs = mode_1920x1080_regs, @@ -598,6 > > > > > > +601,11 @@ static int imx334_update_controls(struct imx334 > > > > > > +*imx334, > > > > > > if (ret) > > > > > > return ret; > > > > > > > > > > > > + ret = __v4l2_ctrl_modify_range(imx334->pclk_ctrl, mode->pclk, > > > > > > + mode->pclk, 1, mode->pclk); > > > > > > + if (ret) > > > > > > + return ret; > > > > > > + > > > > > > ret = __v4l2_ctrl_modify_range(imx334->hblank_ctrl, mode- > > >hblank, > > > > > > mode->hblank, 1, mode->hblank); > > > > > > if (ret) > > > > > > @@ -698,6 +706,8 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl) > > > > > > pm_runtime_put(imx334->dev); > > > > > > > > > > > > break; > > > > > > + case V4L2_CID_PIXEL_RATE: > > > > > > + case V4L2_CID_LINK_FREQ: > > > > > > case V4L2_CID_HBLANK: > > > > > > ret = 0; > > > > > > break; > > > > > > @@ -1047,7 +1057,7 @@ static int imx334_parse_hw_config(struct > > > > > > imx334 > > > > > *imx334) > > > > > > struct fwnode_handle *ep; > > > > > > unsigned long rate; > > > > > > int ret; > > > > > > - int i; > > > > > > + int i, j; > > > > > > > > > > unsigned int would be nicer. > > > > I will change. > > > > > > > > > > > > > > > > > if (!fwnode) > > > > > > return -ENXIO; > > > > > > @@ -1097,11 +1107,20 @@ static int imx334_parse_hw_config(struct > > > > > > imx334 > > > > > *imx334) > > > > > > goto done_endpoint_free; > > > > > > } > > > > > > > > > > > > - for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) > > > > > > - if (bus_cfg.link_frequencies[i] == IMX334_LINK_FREQ) > > > > > > + for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) { > > > > > > + for (j = 0; j < ARRAY_SIZE(link_freq); j++) { > > > > > > + if (bus_cfg.link_frequencies[i] == link_freq[j]) { > > > > > > + set_bit(j, > > > > > > + &imx334->menu_skip_mask); > > > > > > > > > > Is there a guarantee that you'll only be using the modes with the > > > > > listed frequencies? I don't see one but I might have missed it. > > > > > > > > If I understand it correctly, the question here is, the listed > > > > freqeunceis and modes are one to one mapped? Then yes. > > > > > > I don't see this being checked in imx334_set_pad_format(), for instance. > > > > > > If a frequency isn't in DT, the driver isn't supposed to be using it either. > > > > Yes, there is no check. > > > > But, if a frequency is not in DT, the driver will not add in menu items. > > So, the function imx334_set_pad_format() -> imx334_update_controls() > > fails, if we set the frequencies which are not there in the DT or menu items. > > > > Are you ok with the above explanation or any changes you are expecting? > Please do let me know if there are any changes needed. > I am planning to send the next version. There doesn't seem to be anything that would prevent selecting a format with a wrong link frequency in these patches. Could you address that, or point me to where this is done? The control can't be changed by the user, but that's not enough.
diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c index 309c706114d2..62b104eaa437 100644 --- a/drivers/media/i2c/imx334.c +++ b/drivers/media/i2c/imx334.c @@ -49,7 +49,8 @@ #define IMX334_INCLK_RATE 24000000 /* CSI2 HW configuration */ -#define IMX334_LINK_FREQ 891000000 +#define IMX334_LINK_FREQ_891M 891000000 +#define IMX334_LINK_FREQ_445M 445500000 #define IMX334_NUM_DATA_LANES 4 #define IMX334_REG_MIN 0x00 @@ -139,12 +140,14 @@ struct imx334 { u32 vblank; const struct imx334_mode *cur_mode; struct mutex mutex; + unsigned long menu_skip_mask; u32 cur_code; bool streaming; }; static const s64 link_freq[] = { - IMX334_LINK_FREQ, + IMX334_LINK_FREQ_891M, + IMX334_LINK_FREQ_445M, }; /* Sensor mode registers for 1920x1080@30fps */ @@ -468,7 +471,7 @@ static const struct imx334_mode supported_modes[] = { .vblank_min = 45, .vblank_max = 132840, .pclk = 297000000, - .link_freq_idx = 0, + .link_freq_idx = 1, .reg_list = { .num_of_regs = ARRAY_SIZE(mode_1920x1080_regs), .regs = mode_1920x1080_regs, @@ -598,6 +601,11 @@ static int imx334_update_controls(struct imx334 *imx334, if (ret) return ret; + ret = __v4l2_ctrl_modify_range(imx334->pclk_ctrl, mode->pclk, + mode->pclk, 1, mode->pclk); + if (ret) + return ret; + ret = __v4l2_ctrl_modify_range(imx334->hblank_ctrl, mode->hblank, mode->hblank, 1, mode->hblank); if (ret) @@ -698,6 +706,8 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl) pm_runtime_put(imx334->dev); break; + case V4L2_CID_PIXEL_RATE: + case V4L2_CID_LINK_FREQ: case V4L2_CID_HBLANK: ret = 0; break; @@ -1047,7 +1057,7 @@ static int imx334_parse_hw_config(struct imx334 *imx334) struct fwnode_handle *ep; unsigned long rate; int ret; - int i; + int i, j; if (!fwnode) return -ENXIO; @@ -1097,11 +1107,20 @@ static int imx334_parse_hw_config(struct imx334 *imx334) goto done_endpoint_free; } - for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) - if (bus_cfg.link_frequencies[i] == IMX334_LINK_FREQ) + for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) { + for (j = 0; j < ARRAY_SIZE(link_freq); j++) { + if (bus_cfg.link_frequencies[i] == link_freq[j]) { + set_bit(j, &imx334->menu_skip_mask); + break; + } + } + + if (j == ARRAY_SIZE(link_freq)) { + ret = dev_err_probe(imx334->dev, -EINVAL, + "no supported link freq found\n"); goto done_endpoint_free; - - ret = -EINVAL; + } + } done_endpoint_free: v4l2_fwnode_endpoint_free(&bus_cfg); @@ -1232,10 +1251,10 @@ static int imx334_init_controls(struct imx334 *imx334) imx334->link_freq_ctrl = v4l2_ctrl_new_int_menu(ctrl_hdlr, &imx334_ctrl_ops, V4L2_CID_LINK_FREQ, - ARRAY_SIZE(link_freq) - - 1, - mode->link_freq_idx, + __fls(imx334->menu_skip_mask), + __ffs(imx334->menu_skip_mask), link_freq); + if (imx334->link_freq_ctrl) imx334->link_freq_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;