Hi Sakari
On Mon, Jan 08, 2024 at 09:52:19AM +0200, Sakari Ailus wrote:
> Use the v4l2_link_freq_to_bitmap() helper to figure out which
> driver-supported link frequencies can be used on a given system.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> drivers/media/i2c/imx334.c | 41 +++++++++++---------------------------
> 1 file changed, 12 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
> index 6725b3e2a73e..40863d87d341 100644
> --- a/drivers/media/i2c/imx334.c
> +++ b/drivers/media/i2c/imx334.c
> @@ -136,7 +136,7 @@ struct imx334_mode {
> * @vblank: Vertical blanking in lines
> * @cur_mode: Pointer to current selected sensor mode
> * @mutex: Mutex for serializing sensor controls
> - * @menu_skip_mask: Menu skip mask for link_freq_ctrl
> + * @link_freq_bitmap: Menu bitmap for link_freq_ctrl
> * @cur_code: current selected format code
> */
> struct imx334 {
> @@ -158,7 +158,7 @@ struct imx334 {
> u32 vblank;
> const struct imx334_mode *cur_mode;
> struct mutex mutex;
> - unsigned long menu_skip_mask;
> + unsigned long link_freq_bitmap;
> u32 cur_code;
> };
>
> @@ -954,9 +954,9 @@ static int imx334_init_state(struct v4l2_subdev *sd,
> imx334_fill_pad_format(imx334, imx334->cur_mode, &fmt);
>
> __v4l2_ctrl_modify_range(imx334->link_freq_ctrl, 0,
> - __fls(imx334->menu_skip_mask),
> - ~(imx334->menu_skip_mask),
> - __ffs(imx334->menu_skip_mask));
> + __fls(imx334->link_freq_bitmap),
> + ~(imx334->link_freq_bitmap),
> + __ffs(imx334->link_freq_bitmap));
>
> mutex_unlock(&imx334->mutex);
>
> @@ -1112,7 +1112,6 @@ static int imx334_parse_hw_config(struct imx334 *imx334)
> };
> struct fwnode_handle *ep;
> unsigned long rate;
> - unsigned int i, j;
> int ret;
>
> if (!fwnode)
> @@ -1157,26 +1156,10 @@ static int imx334_parse_hw_config(struct imx334 *imx334)
> goto done_endpoint_free;
> }
>
> - if (!bus_cfg.nr_of_link_frequencies) {
> - dev_err(imx334->dev, "no link frequencies defined");
> - ret = -EINVAL;
> - goto done_endpoint_free;
> - }
> -
> - 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 = v4l2_link_freq_to_bitmap(imx334->dev, bus_cfg.link_frequencies,
> + bus_cfg.nr_of_link_frequencies,
> + link_freq, ARRAY_SIZE(link_freq),
> + &imx334->link_freq_bitmap);
>
> done_endpoint_free:
> v4l2_fwnode_endpoint_free(&bus_cfg);
> @@ -1310,8 +1293,8 @@ 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,
> - __fls(imx334->menu_skip_mask),
> - __ffs(imx334->menu_skip_mask),
> + __fls(imx334->link_freq_bitmap),
> + __ffs(imx334->link_freq_bitmap),
How does this work if
driver_freqs = {
FREQ1,
FREQ2,
FREQ3,
FREQ4
};
fw_freqs = {
FREQ2,
FREQ4
};
Will FREQ3 be selectable by userspace ?
> link_freq);
>
> if (imx334->link_freq_ctrl)
> @@ -1386,7 +1369,7 @@ static int imx334_probe(struct i2c_client *client)
> }
>
> /* Set default mode to max resolution */
> - imx334->cur_mode = &supported_modes[__ffs(imx334->menu_skip_mask)];
> + imx334->cur_mode = &supported_modes[__ffs(imx334->link_freq_bitmap)];
> imx334->cur_code = imx334_mbus_codes[0];
> imx334->vblank = imx334->cur_mode->vblank;
>
> --
> 2.39.2
>
