Re: [PATCH v4] media: i2c: imx471: return 0 from get_selection after filling crop

Hi John,

Thank you for the update.

Please also run
 $ ./scripts/checkpatch.pl --strict --max-line-length=80
to check the patch style and format.

On Sun, Aug 23, 2026 at 5:03 AM John Cronin <[email protected]> wrote:
>
> libcamera issues VIDIOC_SUBDEV_G_SELECTION with target CROP (rectangle
> 0) during CameraSensorLegacy bring-up. imx471_get_selection filled the
> rectangle then fell out of the switch and returned -EINVAL.
>


> Fill every supported target, break, and return 0 after the switch
> (default: -EINVAL), as Kate Hsuan suggested.
This section can be moved to the changes description. We only put
descriptions of fixes or new features in the commit message.

>
> Compute analog crop in set_pad_format: 2x binning, centered in the
> native array, same idea as imx219. IMX471 2x2-binned 1928x1088 then
> reports CROP as 3856x2176 centered in 4672x3512.
>

> Rebased onto media-committers next (4900cad020c0). v2 was against the
> Fedora tree and did not apply here.
And, here can be dropped.

>
> Tested on ThinkPad X9-15 Gen 1 (SONY471A / IPU7), Fedora 44
> kernel 7.1.8-200.fc44.
You can test it against the upstream kernel :)

>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2454119
> Signed-off-by: John Cronin <[email protected]>
> Signed-off-by: John Cronin <[email protected]>

> ---
> v4: add Signed-off-by for [email protected] (Gmail From) so
>     media-patchstyle matches the author Patchwork records

Describe the changes of the revisions here.

> v3: rebase on media-committers next
> v2: compute analog crop in set_pad_format; return 0 after switch
>
>  drivers/media/i2c/imx471.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/imx471.c b/drivers/media/i2c/imx471.c
> index 4053aed84..96d96256d 100644
> --- a/drivers/media/i2c/imx471.c
> +++ b/drivers/media/i2c/imx471.c
> @@ -431,6 +431,21 @@ static int imx471_set_pad_format(struct v4l2_subdev *sd,
>
>         *v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format;

>

> +       /* Analog crop: 2x bin, centered in the native array (same idea as imx219). */
> +       {
Drop the brace

> +               struct v4l2_rect *crop;
> +               u8 bin_h, bin_v, binning;
and move the variable declaration to the beginning of the function and
sort them by line length.

> +
> +               crop = v4l2_subdev_state_get_crop(sd_state, fmt->pad);
> +               bin_h = min_t(u32, IMX471_PIXEL_ARRAY_WIDTH / fmt->format.width, 2);
> +               bin_v = min_t(u32, IMX471_PIXEL_ARRAY_HEIGHT / fmt->format.height, 2);
> +               binning = min(bin_h, bin_v);
> +               crop->width = fmt->format.width * binning;
> +               crop->height = fmt->format.height * binning;
> +               crop->left = (IMX471_NATIVE_WIDTH - crop->width) / 2;
> +               crop->top = (IMX471_NATIVE_HEIGHT - crop->height) / 2;
> +       }
Drop the brace.

> +
>         if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
>                 return 0;
>
> @@ -468,7 +483,7 @@ static int imx471_get_selection(struct v4l2_subdev *sd,
>                 sel->r.left = 0;
>                 sel->r.width = IMX471_NATIVE_WIDTH;
>                 sel->r.height = IMX471_NATIVE_HEIGHT;
> -               return 0;
> +               break;
>
>         case V4L2_SEL_TGT_CROP_DEFAULT:
>         case V4L2_SEL_TGT_CROP_BOUNDS:
> @@ -476,10 +491,13 @@ static int imx471_get_selection(struct v4l2_subdev *sd,
>                 sel->r.left = IMX471_PIXEL_ARRAY_LEFT;
>                 sel->r.width = IMX471_PIXEL_ARRAY_WIDTH;
>                 sel->r.height = IMX471_PIXEL_ARRAY_HEIGHT;
> -               return 0;
> +               break;
> +
> +       default:
> +               return -EINVAL;
>         }
>
> -       return -EINVAL;
> +       return 0;
>  }
>
>  static int imx471_init_state(struct v4l2_subdev *sd,
> --
> 2.55.0
>

One question. Do you send a new patch using the "git send-email" command?


-- 
BR,
Kate