public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
To: Sakari Ailus <sakari.ailus@iki.fi>
Cc: Hans Verkuil <hverkuil@xs4all.nl>,
	Linux Media Mailing List <linux-media@vger.kernel.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: Re: Any reason why media_entity_pads_init() isn't void?
Date: Mon, 14 Mar 2016 08:52:51 -0300	[thread overview]
Message-ID: <20160314085251.19698ae8@recife.lan> (raw)
In-Reply-To: <20160314114332.GR11084@valkosipuli.retiisi.org.uk>

Em Mon, 14 Mar 2016 13:43:33 +0200
Sakari Ailus <sakari.ailus@iki.fi> escreveu:

> Hi Mauro,
> 
> On Mon, Mar 14, 2016 at 08:27:38AM -0300, Mauro Carvalho Chehab wrote:
> > Em Mon, 14 Mar 2016 12:36:44 +0200
> > Sakari Ailus <sakari.ailus@iki.fi> escreveu:
> >   
> > > Hi Hans,
> > > 
> > > On Mon, Mar 14, 2016 at 09:25:51AM +0100, Hans Verkuil wrote:  
> > > > I was fixing a sparse warning in media_entity_pads_init() and I noticed
> > > > that that function always returns 0. Any reason why this can't be changed
> > > > to a void function?    
> > > 
> > > I was thinking of the same function but I had a different question: why
> > > would one call this *after* entity->graph_obj.mdev is set? It is set by
> > > media_device_register_entity(), but once mdev it's set, you're not expected
> > > to call pads_init anymore...  
> > 
> > Ideally, drivers should *first* create mdev, and then create the
> > graph objects, as all objects should be registered at the mdev, in
> > order to get their object ID and to be registered at the mdev's object
> > lists.  
> 
> Right. I think it wouldn't hurt to have some comment hints in what's there
> for legacy use cases... I can submit patches for some of these.

Yeah, feel free to submit a patch for it. It could be good to add a
warn_once() that would hit for the legacy case too.

> 
> Currently what works is that you can register graph objects until the media
> device node is exposed to the user.

Yes.

> We don't have proper serialisation in
> place to do that, do we? At least the framework functions leave it up to the
> caller.
> 
> I think it wouldn't be a bad idea either to think about the serialisation
> model a bit. It's been unchanged from the day one basically.

Actually, the async logic does some sort of serialization, although it
doesn't enforce it.

Javier touched on some cases where the logic was not right, but he
didn't change everything. 

I agree with you here: it would be great to have this fixed in a better
way.

That's said, this affects only non-PCI/USB devices. On PCI/USB, the
main/bridge driver is always called first, and the subdev init only
happens after it registers the I2C bus.

-- 
Thanks,
Mauro

  reply	other threads:[~2016-03-14 11:52 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-14  8:25 Any reason why media_entity_pads_init() isn't void? Hans Verkuil
2016-03-14 10:24 ` Mauro Carvalho Chehab
2016-03-14 10:36 ` Sakari Ailus
2016-03-14 11:27   ` Mauro Carvalho Chehab
2016-03-14 11:43     ` Sakari Ailus
2016-03-14 11:52       ` Mauro Carvalho Chehab [this message]
2016-03-14 13:05         ` Mauro Carvalho Chehab
2016-03-14 18:16           ` Mauro Carvalho Chehab

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=20160314085251.19698ae8@recife.lan \
    --to=mchehab@osg.samsung.com \
    --cc=hverkuil@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=sakari.ailus@iki.fi \
    /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