linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@s-opensource.com>
To: Pavel Machek <pavel@ucw.cz>
Cc: "Linux Media Mailing List" <linux-media@vger.kernel.org>,
	"Mauro Carvalho Chehab" <mchehab@infradead.org>,
	"Hans Verkuil" <hans.verkuil@cisco.com>,
	"Sakari Ailus" <sakari.ailus@linux.intel.com>,
	"Pali Rohár" <pali.rohar@gmail.com>,
	"Ramiro Oliveira" <Ramiro.Oliveira@synopsys.com>,
	"Todor Tomov" <todor.tomov@linaro.org>,
	"Robert Jarzmik" <robert.jarzmik@free.fr>,
	"Steve Longerbeam" <slongerbeam@gmail.com>,
	"Guennadi Liakhovetski" <g.liakhovetski@gmx.de>,
	"Hugues Fruchet" <hugues.fruchet@st.com>,
	"Bhumika Goyal" <bhumirks@gmail.com>
Subject: Re: [PATCH] [media] ov2640: make GPIOLIB an optional dependency
Date: Wed, 19 Apr 2017 11:03:12 -0300	[thread overview]
Message-ID: <20170419110300.2dbbf784@vento.lan> (raw)
In-Reply-To: <20170419132339.GA31747@amd>

Em Wed, 19 Apr 2017 15:23:39 +0200
Pavel Machek <pavel@ucw.cz> escreveu:

> Hi!
> 
> > As warned by kbuild test robot:
> > 	warning: (VIDEO_EM28XX_V4L2) selects VIDEO_OV2640 which has unmet direct dependencies (MEDIA_SUPPORT && VIDEO_V4L2 && I2C && GPIOLIB && MEDIA_CAMERA_SUPPORT)
> > 
> > The em28xx driver can use ov2640, but it doesn't depend
> > (or use) the GPIOLIB in order to power off/on the sensor.
> > 
> > So, as we want to allow both usages with and without
> > GPIOLIB, make its dependency optional.  
> 
> Umm. The driver will not work too well with sensor powered off, no?
> Will this result in some tricky-to-debug situations?
> 
> >  config VIDEO_OV2640
> >  	tristate "OmniVision OV2640 sensor support"
> > -	depends on VIDEO_V4L2 && I2C && GPIOLIB
> > +	depends on VIDEO_V4L2 && I2C
> >  	depends on MEDIA_CAMERA_SUPPORT
> >  	help
> >  	  This is a Video4Linux2 sensor-level driver for the
> >  	OmniVision  
> 
> 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.

Besides that, I won't doubt that, at least on some em28xx webcams,
the sensor is always on.

Converting it to use the gpiolib not an easy task, as it supports a
hundred different device models and several different types of devices:
webcams, analog TV, digital TV, hybrid devices (plus devices with FM
radio too).

Too much work for no gain and a high risk of regressions.


Thanks,
Mauro

  reply	other threads:[~2017-04-19 14:03 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-19 11:49 [PATCH] [media] ov2640: make GPIOLIB an optional dependency Mauro Carvalho Chehab
2017-04-19 13:23 ` Pavel Machek
2017-04-19 14:03   ` Mauro Carvalho Chehab [this message]
2017-04-21  6:33     ` Pavel Machek
2017-04-21 14:39       ` Mauro Carvalho Chehab
2017-04-24 14:44         ` Sakari Ailus
2017-04-24 15:50           ` Mauro Carvalho Chehab
2017-04-24 17:38             ` Sakari Ailus
2017-04-24 17:50               ` Sakari Ailus
2017-04-25  2:05               ` Mauro Carvalho Chehab
2017-04-25  8:57                 ` Sakari Ailus
2017-06-19  9:50       ` Hans Verkuil
2017-06-19 10:48         ` Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170419110300.2dbbf784@vento.lan \
    --to=mchehab@s-opensource.com \
    --cc=Ramiro.Oliveira@synopsys.com \
    --cc=bhumirks@gmail.com \
    --cc=g.liakhovetski@gmx.de \
    --cc=hans.verkuil@cisco.com \
    --cc=hugues.fruchet@st.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@infradead.org \
    --cc=pali.rohar@gmail.com \
    --cc=pavel@ucw.cz \
    --cc=robert.jarzmik@free.fr \
    --cc=sakari.ailus@linux.intel.com \
    --cc=slongerbeam@gmail.com \
    --cc=todor.tomov@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).