Re: [PATCH] [media] ov2640: make GPIOLIB an optional dependency

[Hans Verkuil <hverkuil@xxxxxxxxx>]

Hi Pavel,

I'm dropping this from patchwork since this no longer applies now that ov2640
has been moved out of soc_camera.

If you still want this (it is a reasonable patch), then please respin.

Regards,

	Hans

On 04/21/2017 08:33 AM, Pavel Machek wrote:
> Hi!
>
> > > Better solution would be for VIDEO_EM28XX_V4L2 to depend on GPIOLIB,
> > > too, no? If not, should there be BUG_ON(priv->pwdn_gpio);
> > > BUG_ON(priv->resetb_gpio);?
> >
> > Pavel,
> >
> > The em28xx driver was added upstream several years the gpio driver.
> > It controls GPIO using a different logic. It makes no sense to make
> > it dependent on GPIOLIB, except if someone converts it to use it.
>
> At least comment in the sourcecode...? Remove pwdn_gpio fields from
> structure in !GPIOLIB case, because otherwise they are trap for the
> programmer trying to understand what is going on?
>
> Plus, something like this, because otherwise it is quite confusing?
>
> Thanks,
> 								Pavel
>
> diff --git a/drivers/media/i2c/soc_camera/ov2640.c b/drivers/media/i2c/soc_camera/ov2640.c
> index 56de182..85620e1 100644
> --- a/drivers/media/i2c/soc_camera/ov2640.c
> +++ b/drivers/media/i2c/soc_camera/ov2640.c
> @@ -1060,7 +1060,7 @@ static int ov2640_hw_reset(struct device *dev)
>   		/* Active the resetb pin to perform a reset pulse */
>   		gpiod_direction_output(priv->resetb_gpio, 1);
>   		usleep_range(3000, 5000);
> -		gpiod_direction_output(priv->resetb_gpio, 0);
> +		gpiod_set_value(priv->resetb_gpio, 0);
>   	}
>   
>   	return 0;