On Thu, Jul 03, 2025 at 10:52:15PM +0200, Niklas Söderlund wrote:
> Future rework to get rid of .s_power requires the state mutex to be
> held for multiple operations where initializing the device is one of
> them.
>
> Move lock handling outside init_device() but enforce the lock is held
> with a lockdep_assert_held().
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/i2c/adv7180.c | 25 +++++++++++++------------
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index 0c5511a7667d..bef708916190 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -893,7 +893,7 @@ static int init_device(struct adv7180_state *state)
> {
> int ret;
>
> - mutex_lock(&state->mutex);
> + lockdep_assert_held(&state->mutex);
>
> adv7180_set_power_pin(state, true);
> adv7180_set_reset_pin(state, false);
> @@ -903,11 +903,11 @@ static int init_device(struct adv7180_state *state)
>
> ret = state->chip_info->init(state);
> if (ret)
> - goto out_unlock;
> + return ret;
>
> ret = adv7180_program_std(state);
> if (ret)
> - goto out_unlock;
> + return ret;
>
> adv7180_set_field_mode(state);
>
> @@ -918,31 +918,28 @@ static int init_device(struct adv7180_state *state)
> ADV7180_ICONF1_ACTIVE_LOW |
> ADV7180_ICONF1_PSYNC_ONLY);
> if (ret < 0)
> - goto out_unlock;
> + return ret;
>
> ret = adv7180_write(state, ADV7180_REG_IMR1, 0);
> if (ret < 0)
> - goto out_unlock;
> + return ret;
>
> ret = adv7180_write(state, ADV7180_REG_IMR2, 0);
> if (ret < 0)
> - goto out_unlock;
> + return ret;
>
> /* enable AD change interrupts */
> ret = adv7180_write(state, ADV7180_REG_IMR3,
> ADV7180_IRQ3_AD_CHANGE);
> if (ret < 0)
> - goto out_unlock;
> + return ret;
>
> ret = adv7180_write(state, ADV7180_REG_IMR4, 0);
> if (ret < 0)
> - goto out_unlock;
> + return ret;
> }
>
> -out_unlock:
> - mutex_unlock(&state->mutex);
> -
> - return ret;
> + return 0;
> }
>
> static int adv7180_s_stream(struct v4l2_subdev *sd, int enable)
> @@ -1493,7 +1490,9 @@ static int adv7180_probe(struct i2c_client *client)
> if (ret)
> goto err_free_ctrl;
>
> + mutex_lock(&state->mutex);
> ret = init_device(state);
> + mutex_unlock(&state->mutex);
> if (ret)
> goto err_media_entity_cleanup;
>
> @@ -1576,6 +1575,8 @@ static int adv7180_resume(struct device *dev)
> struct adv7180_state *state = to_state(sd);
> int ret;
>
> + guard(mutex)(&state->mutex);
> +
> ret = init_device(state);
> if (ret < 0)
> return ret;
--
Regards,
Laurent Pinchart
