[PATCH 0/4] media: i2c: st-vgxy61: Power up sequence fixes

[PATCH 0/4] media: i2c: st-vgxy61: Power up sequence fixes

[Benjamin Mugnier]

Hi,

This series contains fixes for some issues I ran into, mostly related to
power up, plus a small cleanup patch for a hardcoded value while at it.

Thanks,
Benjamin

Benjamin Mugnier (4):
  media: i2c: st-vgxy61: Remove duplicate default mode set on probe
  media: i2c: st-vgxy61: Move 'detect' call to 'power_on'
  media: i2c: st-vgxy61: Fix control flow error on probe
  media: i2c: st-vgxy61: Use VGXY61_NB_POLARITIES instead of hardcoded
    value in tx_from_ep

 drivers/media/i2c/st-vgxy61.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

-- 
2.25.1

[PATCH 1/4] media: i2c: st-vgxy61: Remove duplicate default mode set on probe

[Benjamin Mugnier]

At this stage the default mode is unknown.
This is done correctly by vgxy61_fill_framefmt right after.

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

diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c
index 826baf4e064d..df5483f14560 100644
--- a/drivers/media/i2c/st-vgxy61.c
+++ b/drivers/media/i2c/st-vgxy61.c
@@ -1332,7 +1332,6 @@ static int vgxy61_init_cfg(struct v4l2_subdev *sd,
 	struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
 	struct v4l2_subdev_format fmt = { 0 };
 
-	sensor->current_mode = sensor->default_mode;
 	vgxy61_fill_framefmt(sensor, sensor->current_mode, &fmt.format,
 			     VGXY61_MEDIA_BUS_FMT_DEF);
 
-- 
2.25.1

[PATCH 2/4] media: i2c: st-vgxy61: Move 'detect' call to 'power_on'

[Benjamin Mugnier]

Previously the device detection was performed after patching.
Move it right after the reset to make sure we have the correct sensor
before trying to patch it.

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

diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c
index df5483f14560..e51234aebb16 100644
--- a/drivers/media/i2c/st-vgxy61.c
+++ b/drivers/media/i2c/st-vgxy61.c
@@ -1732,6 +1732,12 @@ static int vgxy61_power_on(struct device *dev)
 		}
 	}
 
+	ret = vgxy61_detect(sensor);
+	if (ret) {
+		dev_err(&client->dev, "sensor detect failed %d\n", ret);
+		goto disable_clock;
+	}
+
 	ret = vgxy61_patch(sensor);
 	if (ret) {
 		dev_err(&client->dev, "sensor patch failed %d\n", ret);
@@ -1858,12 +1864,6 @@ static int vgxy61_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
-	ret = vgxy61_detect(sensor);
-	if (ret) {
-		dev_err(&client->dev, "sensor detect failed %d\n", ret);
-		return ret;
-	}
-
 	vgxy61_fill_sensor_param(sensor);
 	vgxy61_fill_framefmt(sensor, sensor->current_mode, &sensor->fmt,
 			     VGXY61_MEDIA_BUS_FMT_DEF);
-- 
2.25.1

[PATCH 3/4] media: i2c: st-vgxy61: Fix control flow error on probe

[Benjamin Mugnier]

In case of error 'update_hdr' now goes through 'power_off' instead of
returning, effectively shutting down the sensor.

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

diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c
index e51234aebb16..2988ba60be94 100644
--- a/drivers/media/i2c/st-vgxy61.c
+++ b/drivers/media/i2c/st-vgxy61.c
@@ -1868,11 +1868,11 @@ static int vgxy61_probe(struct i2c_client *client)
 	vgxy61_fill_framefmt(sensor, sensor->current_mode, &sensor->fmt,
 			     VGXY61_MEDIA_BUS_FMT_DEF);
 
+	mutex_init(&sensor->lock);
+
 	ret = vgxy61_update_hdr(sensor, sensor->hdr);
 	if (ret)
-		return ret;
-
-	mutex_init(&sensor->lock);
+		goto error_power_off;
 
 	ret = vgxy61_init_controls(sensor);
 	if (ret) {
@@ -1911,8 +1911,8 @@ static int vgxy61_probe(struct i2c_client *client)
 	media_entity_cleanup(&sensor->sd.entity);
 error_handler_free:
 	v4l2_ctrl_handler_free(sensor->sd.ctrl_handler);
-	mutex_destroy(&sensor->lock);
 error_power_off:
+	mutex_destroy(&sensor->lock);
 	vgxy61_power_off(dev);
 
 	return ret;
-- 
2.25.1

[PATCH 4/4] media: i2c: st-vgxy61: Use VGXY61_NB_POLARITIES instead of hardcoded value in tx_from_ep

[Benjamin Mugnier]

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

diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c
index 2988ba60be94..33ff2cf20fbf 100644
--- a/drivers/media/i2c/st-vgxy61.c
+++ b/drivers/media/i2c/st-vgxy61.c
@@ -1546,7 +1546,7 @@ static int vgxy61_tx_from_ep(struct vgxy61_dev *sensor,
 	sensor->nb_of_lane = l_nb;
 
 	dev_dbg(&client->dev, "tx uses %d lanes", l_nb);
-	for (i = 0; i < 5; i++) {
+	for (i = 0; i < VGXY61_NB_POLARITIES; i++) {
 		dev_dbg(&client->dev, "log2phy[%d] = %d\n", i, log2phy[i]);
 		dev_dbg(&client->dev, "phy2log[%d] = %d\n", i, phy2log[i]);
 		dev_dbg(&client->dev, "polarity[%d] = %d\n", i, polarities[i]);
-- 
2.25.1

Re: [PATCH 4/4] media: i2c: st-vgxy61: Use VGXY61_NB_POLARITIES instead of hardcoded value in tx_from_ep

[Sakari Ailus]

Hi Benjamin,

On Wed, Feb 01, 2023 at 03:04:17PM +0100, Benjamin Mugnier wrote:
> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>

I know this is a simple patch, but please add a commit message nonetheless.

Not having one produces a checkpatch.pl warning. Did you use it? :-)

-- 
Kind regards,

Sakari Ailus

Re: [PATCH 4/4] media: i2c: st-vgxy61: Use VGXY61_NB_POLARITIES instead of hardcoded value in tx_from_ep

[Benjamin Mugnier]

Hi Sakari,

On 2/14/23 00:01, Sakari Ailus wrote:
> Hi Benjamin,
> 
> On Wed, Feb 01, 2023 at 03:04:17PM +0100, Benjamin Mugnier wrote:
>> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
> 
> I know this is a simple patch, but please add a commit message nonetheless.
> 
> Not having one produces a checkpatch.pl warning. Did you use it? :-)
> 

It's even worse, I remember using checkpatch.pl on the source file
directly but not on commits, so I missed it :'( I apologize.

Here is the long commit message:
tx_from_ep's for loop uses '5' as bound, while in fact it refers to the
number of polarities. Replace it by VGXY61_NB_POLARITIES for factorization.

Can you apply it directly to your tree, or do you want me to send a v2
with this fix?

Thanks a ton.

-- 
Regards,

Benjamin