v4l2-fwnode: status, plans for merge, any branch to merge against?

Message ID 20170614194128.GA5669@amd (mailing list archive)
State Superseded, archived
Delegated to: Laurent Pinchart
Headers

Commit Message

Pavel Machek June 14, 2017, 7:41 p.m. UTC
  Hi!

> > > > Are there any news about the fwnode branch?
> > > > 
> > > > I have quite usable camera, but it is still based on
> > > > 982e8e40390d26430ef106fede41594139a4111c (that's v4.10). It would be
> > > > good to see fwnode stuff upstream... are there any plans for that?
> > > > 
> > > > Is there stable branch to which I could move the stuff?
> > > 
> > > What's relevant for most V4L2 drivers is in linux-media right now.
> > > 
> > > There are new features that will take some time to get in. The trouble has
> > > been, and continue to be, that the patches need to go through various trees
> > > so it'll take some time for them to be merged.
> > > 
> > > I expect to have most of them in during the next merge window.
> > 
> > So git://linuxtv.org/media_tree.git branch master is the right one to
> > work one?
> 
> I also pushed the rebased ccp2 branch there:
> 
> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=ccp2>
> 
> It's now right on the top of media-tree master.

Thanks, that's what I was looking for.

Unfortunately, it does not compile.

  CC      drivers/media/platform/omap3isp/ispcsiphy.o
  drivers/media/platform/omap3isp/isp.c: In function
  'isp_fwnode_parse':
  drivers/media/platform/omap3isp/isp.c:2029:35: error: 'fwn'
  undeclared (first use in this function)
  drivers/media/platform/omap3isp/isp.c:2029:35: note: each undeclared
  identifier is reported only once for each function it appears in
  drivers/media/platform/omap3isp/isp.c:2029:2: error: incompatible
  type for argument 2 of 'v4l2_fwnode_endpoint_parse'
  In file included from drivers/media/platform/omap3isp/isp.c:67:0:
  ./include/media/v4l2-fwnode.h:112:5: note: expected 'struct
  v4l2_fwnode_endpoint *' but argument is of type 'struct
  v4l2_fwnode_endpoint'
  scripts/Makefile.build:302: recipe for target
  'drivers/media/platform/omap3isp/isp.o' failed
  make[4]: *** [drivers/media/platform/omap3isp/isp.o] Error 1
  make[4]: *** Waiting for unfinished jobs....
  scripts/Makefile.build:561: recipe for target
  'drivers/media/platform/omap3isp' failed
  make[3]: *** [drivers/media/platform/omap3isp] Error 2

You can get my config if needed. Now let me try to fix it... It was
not too bad, good.

commit 364340e7aa037535a65d2ef2a1711c97d233fede
Author: Pavel <pavel@ucw.cz>
Date:   Wed Jun 14 21:40:37 2017 +0200

Fix compilation of omap3isp/isp.c.
    
Signed-off-by: Pavel Machek <pavel@ucw.cz>


									Pavel
  

Comments

Sakari Ailus June 15, 2017, 10:07 p.m. UTC | #1
On Wed, Jun 14, 2017 at 09:41:29PM +0200, Pavel Machek wrote:
> diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
> index 4ca3fc9..b80debf 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2026,7 +2026,7 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
>  
>  	isd->bus = buscfg;
>  
> -	ret = v4l2_fwnode_endpoint_parse(fwn, vep);
> +	ret = v4l2_fwnode_endpoint_parse(fwnode, &vep);
>  	if (ret)
>  		return ret;

I just pushed the fix there.

Btw. I think we should probably drop the change allocating the sub-device
configuration separately. It's better to associate the lens, flash and
eeprom (where it exists) to the sensor than to the CSI-2 receiver. In that
case there are no async sub-devices without bus configuration.
  
Pavel Machek June 16, 2017, 6:23 a.m. UTC | #2
On Fri 2017-06-16 01:07:00, Sakari Ailus wrote:
> On Wed, Jun 14, 2017 at 09:41:29PM +0200, Pavel Machek wrote:
> > diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
> > index 4ca3fc9..b80debf 100644
> > --- a/drivers/media/platform/omap3isp/isp.c
> > +++ b/drivers/media/platform/omap3isp/isp.c
> > @@ -2026,7 +2026,7 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
> >  
> >  	isd->bus = buscfg;
> >  
> > -	ret = v4l2_fwnode_endpoint_parse(fwn, vep);
> > +	ret = v4l2_fwnode_endpoint_parse(fwnode, &vep);
> >  	if (ret)
> >  		return ret;
> 
> I just pushed the fix there.
> 
> Btw. I think we should probably drop the change allocating the sub-device
> configuration separately. It's better to associate the lens, flash and
> eeprom (where it exists) to the sensor than to the CSI-2 receiver. In that
> case there are no async sub-devices without bus configuration.

Actually I thought about that a bit, and am not sure about that.

CSI-2 receiver may not be good place to associate lens and flash with,
agreed.

But is sensor a good place? In particular, phones with two cameras
cooperating (for example one black&white and one color) are getting
common. It seems to be true that each sensor has a lens and autofocus
motor associated, but flash LED is common, and both sensors are
designed to work as one device.

But yes, that's still better than placing it at CSI-2 receiver. But I
guess we should make sure that flash LED can associated with more than
one sensor, and maybe we should have some kind of "camera package"
entity.

Best regards,

									Pavel
  

Patch

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 4ca3fc9..b80debf 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2026,7 +2026,7 @@  static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
 
 	isd->bus = buscfg;
 
-	ret = v4l2_fwnode_endpoint_parse(fwn, vep);
+	ret = v4l2_fwnode_endpoint_parse(fwnode, &vep);
 	if (ret)
 		return ret;