Re: [PATCH 2/4] media: i2c: ov02c10: Make reset gpio logic optional

[Hans de Goede <hdegoede@redhat.com>]

Hi Bryan,

On 15-Mar-25 2:40 PM, Bryan O'Donoghue wrote:
> The reset gpio is optional. Only trigger the reset logic if the reset gpio
> pin is valid.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  drivers/media/i2c/ov02c10.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
> index f6542cdf7472..595998e60b22 100644
> --- a/drivers/media/i2c/ov02c10.c
> +++ b/drivers/media/i2c/ov02c10.c
> @@ -664,7 +664,8 @@ static int ov02c10_power_off(struct device *dev)
>  	struct v4l2_subdev *sd = dev_get_drvdata(dev);
>  	struct ov02c10 *ov02c10 = to_ov02c10(sd);
>  
> -	gpiod_set_value_cansleep(ov02c10->reset, 1);
> +	if (ov02c10->reset)
> +		gpiod_set_value_cansleep(ov02c10->reset, 1);

This is not necessary, gpoid functions generally speaking
will happily take the NULL returned by gpiod_get_optional()
when the GPIO is not there without logging or returning
any errors. Note calling them with a PTR_ERR is not ok,
but we don't do that.



>  	regulator_bulk_disable(ARRAY_SIZE(ov02c10_supply_names),
>  			       ov02c10->supplies);
> @@ -694,8 +695,10 @@ static int ov02c10_power_on(struct device *dev)
>  		return ret;
>  	}
>  
> -	gpiod_set_value_cansleep(ov02c10->reset, 0);
> -	usleep_range(1500, 1800);
> +	if (ov02c10->reset) {
> +		gpiod_set_value_cansleep(ov02c10->reset, 0);
> +		usleep_range(1500, 1800);
> +	}

Same here for the gpiod_set_value_cansleep() call, as for
the sleep() I think we want to sleep even without a reset
since we have also just enabled the clk + powerrails...

Regards,

Hans