[PATCH] drivers: media: i2c: imx335: Fix frame size enumeration

[PATCH] drivers: media: i2c: imx335: Fix frame size enumeration

[Kieran Bingham]

In commit cfa49ff0558a ("media: i2c: imx335: Support 2592x1940 10-bit
mode") the IMX335 driver was extended to support multiple output
bitdepth modes.

This incorrectly extended the frame size enumeration to check against
the supported mbus_codes array instead of the supported mode/frame
array. This has the unwanted side effect of reporting the currently
supported frame size 2592x1944 three times.

Fix the check accordingly to report a frame size for each supported
size, which is presently only a single entry.

Fixes: cfa49ff0558a ("media: i2c: imx335: Support 2592x1940 10-bit mode")
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---

I expect to post a 2/2 binning mode series on top of this later to then
further extend the supported_mode into an array ...

So I won't dwell on tracking fsize->index against the 'size' or number
of supported modes yet... but this fixes an otherwise annoying glitch
;-)

 drivers/media/i2c/imx335.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c
index 0418875e996c..6369bdbd2b09 100644
--- a/drivers/media/i2c/imx335.c
+++ b/drivers/media/i2c/imx335.c
@@ -720,7 +720,8 @@ static int imx335_enum_frame_size(struct v4l2_subdev *sd,
 	struct imx335 *imx335 = to_imx335(sd);
 	u32 code;
 
-	if (fsize->index > ARRAY_SIZE(imx335_mbus_codes))
+	/* Only a single supported_mode available. */
+	if (fsize->index > 0)
 		return -EINVAL;
 
 	code = imx335_get_format_code(imx335, fsize->code);
-- 
2.48.1

Re: [PATCH] drivers: media: i2c: imx335: Fix frame size enumeration

[Sakari Ailus]

Hi Kieran,

On Wed, Apr 30, 2025 at 08:36:49AM +0100, Kieran Bingham wrote:
> In commit cfa49ff0558a ("media: i2c: imx335: Support 2592x1940 10-bit
> mode") the IMX335 driver was extended to support multiple output
> bitdepth modes.
> 
> This incorrectly extended the frame size enumeration to check against
> the supported mbus_codes array instead of the supported mode/frame
> array. This has the unwanted side effect of reporting the currently
> supported frame size 2592x1944 three times.
> 
> Fix the check accordingly to report a frame size for each supported
> size, which is presently only a single entry.
> 
> Fixes: cfa49ff0558a ("media: i2c: imx335: Support 2592x1940 10-bit mode")
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Thanks for the patch.

Cc: stable is required these days with Fixes: tag, I've added it this time.

-- 
Sakari Ailus

Re: [PATCH] drivers: media: i2c: imx335: Fix frame size enumeration

[Kieran Bingham]

Quoting Sakari Ailus (2025-04-30 09:45:09)
> Hi Kieran,
> 
> On Wed, Apr 30, 2025 at 08:36:49AM +0100, Kieran Bingham wrote:
> > In commit cfa49ff0558a ("media: i2c: imx335: Support 2592x1940 10-bit
> > mode") the IMX335 driver was extended to support multiple output
> > bitdepth modes.
> > 
> > This incorrectly extended the frame size enumeration to check against
> > the supported mbus_codes array instead of the supported mode/frame
> > array. This has the unwanted side effect of reporting the currently
> > supported frame size 2592x1944 three times.
> > 
> > Fix the check accordingly to report a frame size for each supported
> > size, which is presently only a single entry.
> > 
> > Fixes: cfa49ff0558a ("media: i2c: imx335: Support 2592x1940 10-bit mode")
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> Thanks for the patch.
> 
> Cc: stable is required these days with Fixes: tag, I've added it this time.


I didn't know that. I thought stable tree picked up from the fixes tags.
(Or I remember there was some AI system that was picking them out too?)

I'll see if I can automate/add that to my fixes helper.

Does it need to be in this format?

Cc: stable@vger.kernel.org      # 6.8

--
Kieran


> 
> -- 
> Sakari Ailus