[PATCH v3 0/4] media: i2c: vd55g1: Miscellaneous fixes

[PATCH v3 0/4] media: i2c: vd55g1: Miscellaneous fixes

[Benjamin Mugnier]

This series provides small fixes and style improvements to the vd55g1
driver. Nothing fancy really, just to keep everything up to date.

Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
---
Changes in v3:
- Drop *_def variables
- Remove bad line breaks in commit messages
- Link to v2: https://lore.kernel.org/r/20250610-fix_vd55g1-v2-0-328305748417@foss.st.com

Changes in v2:
- Add commit description to commits missing one
- Link to v1: https://lore.kernel.org/r/20250609-fix_vd55g1-v1-0-594f1134e3fb@foss.st.com

---
Benjamin Mugnier (4):
      media: i2c: vd55g1: Fix RATE macros not being expressed in bps
      media: i2c: vd55g1: Fix return code in vd55g1_enable_streams error path
      media: i2c: vd55g1: Setup sensor external clock before patching
      media: i2c: vd55g1: Use first index of mbus codes array as default

 drivers/media/i2c/vd55g1.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)
---
base-commit: 5e1ff2314797bf53636468a97719a8222deca9ae
change-id: 20250609-fix_vd55g1-83e924648d85

Best regards,
-- 
Benjamin Mugnier <benjamin.mugnier@foss.st.com>

[PATCH v3 1/4] media: i2c: vd55g1: Fix RATE macros not being expressed in bps

[Benjamin Mugnier]

As a bit rate is expressed in bps, use MEGA instead of HZ_PER_MHZ.

Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
---
 drivers/media/i2c/vd55g1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
index 25e2fc88a0367bf6a28bb22d209323ace99299f2..78dd22d9cab03edf4ff3e5a301f8d045e930c997 100644
--- a/drivers/media/i2c/vd55g1.c
+++ b/drivers/media/i2c/vd55g1.c
@@ -129,8 +129,8 @@
 #define VD55G1_FWPATCH_REVISION_MINOR			9
 #define VD55G1_XCLK_FREQ_MIN				(6 * HZ_PER_MHZ)
 #define VD55G1_XCLK_FREQ_MAX				(27 * HZ_PER_MHZ)
-#define VD55G1_MIPI_RATE_MIN				(250 * HZ_PER_MHZ)
-#define VD55G1_MIPI_RATE_MAX				(1200 * HZ_PER_MHZ)
+#define VD55G1_MIPI_RATE_MIN				(250 * MEGA)
+#define VD55G1_MIPI_RATE_MAX				(1200 * MEGA)
 
 static const u8 patch_array[] = {
 	0x44, 0x03, 0x09, 0x02, 0xe6, 0x01, 0x42, 0x00, 0xea, 0x01, 0x42, 0x00,

-- 
2.25.1

[PATCH v3 2/4] media: i2c: vd55g1: Fix return code in vd55g1_enable_streams error path

[Benjamin Mugnier]

Enable stream was returning success even if an error occurred, fix it by
modifying the err_rpm_put return value to -EINVAL.

Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
---
 drivers/media/i2c/vd55g1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
index 78dd22d9cab03edf4ff3e5a301f8d045e930c997..336dc3c85ac9e695f22aa524e0df6138dc76e45c 100644
--- a/drivers/media/i2c/vd55g1.c
+++ b/drivers/media/i2c/vd55g1.c
@@ -1084,7 +1084,7 @@ static int vd55g1_enable_streams(struct v4l2_subdev *sd,
 
 err_rpm_put:
 	pm_runtime_put(sensor->dev);
-	return 0;
+	return -EINVAL;
 }
 
 static int vd55g1_disable_streams(struct v4l2_subdev *sd,

-- 
2.25.1

Re: [PATCH v3 2/4] media: i2c: vd55g1: Fix return code in vd55g1_enable_streams error path

[Sakari Ailus]

Hi Benjamin,

On Wed, Jun 11, 2025 at 10:48:31AM +0200, Benjamin Mugnier wrote:
> Enable stream was returning success even if an error occurred, fix it by
> modifying the err_rpm_put return value to -EINVAL.
> 
> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>

This looks like something that should be backported btw. What's the Fixes:
header? I can add that while applying.

-- 
Regards,

Sakari Ailus

Re: [PATCH v3 2/4] media: i2c: vd55g1: Fix return code in vd55g1_enable_streams error path

[Benjamin Mugnier]

On 6/12/25 08:24, Sakari Ailus wrote:
> Hi Benjamin,
> 
> On Wed, Jun 11, 2025 at 10:48:31AM +0200, Benjamin Mugnier wrote:
>> Enable stream was returning success even if an error occurred, fix it by
>> modifying the err_rpm_put return value to -EINVAL.
>>
>> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
> 
> This looks like something that should be backported btw. What's the Fixes:
> header? I can add that while applying.
> 

You're correct, here it is :

Fixes: e56616d7b23c ("media: i2c: Add driver for ST VD55G1 camera sensor")

Thank you.

-- 
Regards,
Benjamin

[PATCH v3 3/4] media: i2c: vd55g1: Setup sensor external clock before patching

[Benjamin Mugnier]

Proper clock configuration is required to advance through FSM states.
Prior than this having a different clock value than default sensor's
value was used (12 MHz) could prevent the sensor from booting.

Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
---
 drivers/media/i2c/vd55g1.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
index 336dc3c85ac9e695f22aa524e0df6138dc76e45c..dec6e3e231d54a742bdd08ff2a506c152bb89429 100644
--- a/drivers/media/i2c/vd55g1.c
+++ b/drivers/media/i2c/vd55g1.c
@@ -1038,8 +1038,6 @@ static int vd55g1_enable_streams(struct v4l2_subdev *sd,
 	if (ret < 0)
 		return ret;
 
-	vd55g1_write(sensor, VD55G1_REG_EXT_CLOCK, sensor->xclk_freq, &ret);
-
 	/* Configure output */
 	vd55g1_write(sensor, VD55G1_REG_MIPI_DATA_RATE,
 		     sensor->mipi_rate, &ret);
@@ -1613,6 +1611,9 @@ static int vd55g1_power_on(struct device *dev)
 		goto disable_clock;
 	}
 
+	/* Setup clock now to advance through system FSM states */
+	vd55g1_write(sensor, VD55G1_REG_EXT_CLOCK, sensor->xclk_freq, &ret);
+
 	ret = vd55g1_patch(sensor);
 	if (ret) {
 		dev_err(dev, "Sensor patch failed %d\n", ret);

-- 
2.25.1

[PATCH v3 4/4] media: i2c: vd55g1: Use first index of mbus codes array as default

[Benjamin Mugnier]

Factorize code and prevent future errors in case of media bus codes
change. Rename VD55G1_DEFAULT_MODE to VD55G1_MODE_DEF to mimic other
macros and reduce vd55g1_update_img_pad_format() name to
vd55g1_update_pad_fmt() to stay within the 80 characters limit.

Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
---
 drivers/media/i2c/vd55g1.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
index dec6e3e231d54a742bdd08ff2a506c152bb89429..c0754fd03b1d5e947dc5e24e9f54d98a67a23376 100644
--- a/drivers/media/i2c/vd55g1.c
+++ b/drivers/media/i2c/vd55g1.c
@@ -111,9 +111,9 @@
 
 #define VD55G1_WIDTH					804
 #define VD55G1_HEIGHT					704
-#define VD55G1_DEFAULT_MODE				0
+#define VD55G1_MODE_DEF					0
 #define VD55G1_NB_GPIOS					4
-#define VD55G1_MEDIA_BUS_FMT_DEF			MEDIA_BUS_FMT_Y8_1X8
+#define VD55G1_MBUS_CODE_DEF				0
 #define VD55G1_DGAIN_DEF				256
 #define VD55G1_AGAIN_DEF				19
 #define VD55G1_EXPO_MAX_TERM				64
@@ -883,10 +883,9 @@ static int vd55g1_apply_cold_start(struct vd55g1 *sensor,
 	return ret;
 }
 
-static void vd55g1_update_img_pad_format(struct vd55g1 *sensor,
-					 const struct vd55g1_mode *mode,
-					 u32 code,
-					 struct v4l2_mbus_framefmt *fmt)
+static void vd55g1_update_pad_fmt(struct vd55g1 *sensor,
+				  const struct vd55g1_mode *mode, u32 code,
+				  struct v4l2_mbus_framefmt *fmt)
 {
 	fmt->code = code;
 	fmt->width = mode->width;
@@ -1229,8 +1228,8 @@ static int vd55g1_set_pad_fmt(struct v4l2_subdev *sd,
 					  width, height, sd_fmt->format.width,
 					  sd_fmt->format.height);
 
-	vd55g1_update_img_pad_format(sensor, new_mode, sd_fmt->format.code,
-				     &sd_fmt->format);
+	vd55g1_update_pad_fmt(sensor, new_mode, sd_fmt->format.code,
+			      &sd_fmt->format);
 
 	/*
 	 * Use binning to maximize the crop rectangle size, and centre it in the
@@ -1260,7 +1259,6 @@ static int vd55g1_set_pad_fmt(struct v4l2_subdev *sd,
 static int vd55g1_init_state(struct v4l2_subdev *sd,
 			     struct v4l2_subdev_state *sd_state)
 {
-	unsigned int def_mode = VD55G1_DEFAULT_MODE;
 	struct vd55g1 *sensor = to_vd55g1(sd);
 	struct v4l2_subdev_format fmt = { 0 };
 	struct v4l2_subdev_route routes[] = {
@@ -1277,8 +1275,9 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
 	if (ret)
 		return ret;
 
-	vd55g1_update_img_pad_format(sensor, &vd55g1_supported_modes[def_mode],
-				     VD55G1_MEDIA_BUS_FMT_DEF, &fmt.format);
+	vd55g1_update_pad_fmt(sensor, &vd55g1_supported_modes[VD55G1_MODE_DEF],
+			      vd55g1_mbus_codes[VD55G1_MBUS_CODE_DEF].code,
+			      &fmt.format);
 
 	return vd55g1_set_pad_fmt(sd, sd_state, &fmt);
 }

-- 
2.25.1