| Message ID | 20200331180630.5703-1-dafna.hirschfeld@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: imx219: Fix a bug in imx219_enum_frame_size | expand |
On 3/31/20 3:06 PM, Dafna Hirschfeld wrote: > When enumerating the frame sizes, the value sent to > imx219_get_format_code should be fse->code > (the code from the ioctl) and not imx219->fmt.code > which is the code set currently in the driver. > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> Reviewed-by: Helen Koike <helen.koike@collabora.com> > --- > drivers/media/i2c/imx219.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > index b1f30debe449..df2a6ed7c8ac 100644 > --- a/drivers/media/i2c/imx219.c > +++ b/drivers/media/i2c/imx219.c > @@ -781,7 +781,7 @@ static int imx219_enum_frame_size(struct v4l2_subdev *sd, > if (fse->index >= ARRAY_SIZE(supported_modes)) > return -EINVAL; > > - if (fse->code != imx219_get_format_code(imx219, imx219->fmt.code)) > + if (fse->code != imx219_get_format_code(imx219, fse->code)) > return -EINVAL; > > fse->min_width = supported_modes[fse->index].width; >
Hi Dafna Thanks for the patch. On Tue, 31 Mar 2020 at 19:06, Dafna Hirschfeld <dafna.hirschfeld@collabora.com> wrote: > > When enumerating the frame sizes, the value sent to > imx219_get_format_code should be fse->code > (the code from the ioctl) and not imx219->fmt.code > which is the code set currently in the driver. > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> Agreed that v4l2-ctl --list-formats-ext doesn't list the frame sizes for the mode that isn't selected without this patch. With this patch you get the full list. Does it warrant a "Fixes: 22da1d56e ("media: i2c: imx219: Add support for RAW8 bit bayer format")"? I'd probably say yes. Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > --- > drivers/media/i2c/imx219.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > index b1f30debe449..df2a6ed7c8ac 100644 > --- a/drivers/media/i2c/imx219.c > +++ b/drivers/media/i2c/imx219.c > @@ -781,7 +781,7 @@ static int imx219_enum_frame_size(struct v4l2_subdev *sd, > if (fse->index >= ARRAY_SIZE(supported_modes)) > return -EINVAL; > > - if (fse->code != imx219_get_format_code(imx219, imx219->fmt.code)) > + if (fse->code != imx219_get_format_code(imx219, fse->code)) > return -EINVAL; > > fse->min_width = supported_modes[fse->index].width; > -- > 2.17.1 >
On Fri, Apr 3, 2020 at 11:23 AM Dave Stevenson <dave.stevenson@raspberrypi.com> wrote: > > Hi Dafna > > Thanks for the patch. > > On Tue, 31 Mar 2020 at 19:06, Dafna Hirschfeld > <dafna.hirschfeld@collabora.com> wrote: > > > > When enumerating the frame sizes, the value sent to > > imx219_get_format_code should be fse->code > > (the code from the ioctl) and not imx219->fmt.code > > which is the code set currently in the driver. > > > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Agreed that v4l2-ctl --list-formats-ext doesn't list the frame sizes > for the mode that isn't selected without this patch. With this patch > you get the full list. > > Does it warrant a "Fixes: 22da1d56e ("media: i2c: imx219: Add support > for RAW8 bit bayer format")"? I'd probably say yes. > +1 Cheers, --Prabhakar > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > --- > > drivers/media/i2c/imx219.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > > index b1f30debe449..df2a6ed7c8ac 100644 > > --- a/drivers/media/i2c/imx219.c > > +++ b/drivers/media/i2c/imx219.c > > @@ -781,7 +781,7 @@ static int imx219_enum_frame_size(struct v4l2_subdev *sd, > > if (fse->index >= ARRAY_SIZE(supported_modes)) > > return -EINVAL; > > > > - if (fse->code != imx219_get_format_code(imx219, imx219->fmt.code)) > > + if (fse->code != imx219_get_format_code(imx219, fse->code)) > > return -EINVAL; > > > > fse->min_width = supported_modes[fse->index].width; > > -- > > 2.17.1 > >
On Fri, Apr 03, 2020 at 11:28:52AM +0100, Lad, Prabhakar wrote: > On Fri, Apr 3, 2020 at 11:23 AM Dave Stevenson > <dave.stevenson@raspberrypi.com> wrote: > > > > Hi Dafna > > > > Thanks for the patch. > > > > On Tue, 31 Mar 2020 at 19:06, Dafna Hirschfeld > > <dafna.hirschfeld@collabora.com> wrote: > > > > > > When enumerating the frame sizes, the value sent to > > > imx219_get_format_code should be fse->code > > > (the code from the ioctl) and not imx219->fmt.code > > > which is the code set currently in the driver. > > > > > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> > > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > Agreed that v4l2-ctl --list-formats-ext doesn't list the frame sizes > > for the mode that isn't selected without this patch. With this patch > > you get the full list. > > > > Does it warrant a "Fixes: 22da1d56e ("media: i2c: imx219: Add support > > for RAW8 bit bayer format")"? I'd probably say yes. > > > +1 Thanks, all! I've added: Fixes: 22da1d56e982 ("media: i2c: imx219: Add support for RAW8 bit bayer format")
diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c index b1f30debe449..df2a6ed7c8ac 100644 --- a/drivers/media/i2c/imx219.c +++ b/drivers/media/i2c/imx219.c @@ -781,7 +781,7 @@ static int imx219_enum_frame_size(struct v4l2_subdev *sd, if (fse->index >= ARRAY_SIZE(supported_modes)) return -EINVAL; - if (fse->code != imx219_get_format_code(imx219, imx219->fmt.code)) + if (fse->code != imx219_get_format_code(imx219, fse->code)) return -EINVAL; fse->min_width = supported_modes[fse->index].width;
When enumerating the frame sizes, the value sent to imx219_get_format_code should be fse->code (the code from the ioctl) and not imx219->fmt.code which is the code set currently in the driver. Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> --- drivers/media/i2c/imx219.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)