Hi Jacob,
On Mon, Nov 27, 2017 at 03:03:59PM +0800, Jacob Chen wrote:
> Hi,
>
> On 2017年11月25日星期六 CST 下午1:05:42 you wrote:
> > Hi,
> >
> > On 2017年11月24日星期五 CST 下午6:19:36 you wrote:
> > > On Fri, Nov 24, 2017 at 6:17 PM, Sakari Ailus
> > >
> > > <sakari.ailus@xxxxxxxxxxxxxxx> wrote:
> > > > Hi Tomasz,
> > > >
> > > > On Fri, Nov 24, 2017 at 06:03:26PM +0900, Tomasz Figa wrote:
> > > >> Hi Sakari,
> > > >>
> > > >> We have the following graph:
> > > >> ISP (registers notifier for v4l2_dev)
> > > >>
> > > >>
> > > >>
> > > >> PHY (registers notifier for v4l2_subdev, just like sensors for
> > > >>
> > > >> flash/focuser)
> > > >>
> > > >> | \
> > > >>
> > > >> sensor0 sensor1
> > > >>
> > > >> ...
> > > >>
> > > >> Both ISP and PHY are completely separate drivers not directly aware of
> > > >> each other, since we have several different PHY IP blocks that we need
> > > >> to support and some of them are multi-functional, such as CSI+DSI PHY
> > > >> and need to be supported by drivers independent from the ISP driver.
> > > >
> > > > That should work fine. In the above case there are two notifiers,
> > > > indeed,
> > > > but they're not expecting the *same* sub-devices.
> > >
> > > Got it.
> > >
> > > Jacob, could you make sure there are no mistakes in the Device Tree
> > > source?
> > >
> > > Best regards,
> > > Tomasz
> >
> > I see...
> > This problem might be sloved by moving `v4l2_async_subdev_notifier_register`
> > after `v4l2_async_register_subdev`. I will test it.
> >
> > > > What this could be about is that in some version of the set I disabled
> > > > the
> > > > complete callback on the sub-notifiers for two reasons: there was no
> > > > need
> > > > seen for them and the complete callback is problematic in general
> > > > (there's
> > > > been discussion on that, mostly related to earlier versions of the
> > > > fwnode
> > > > parsing patchset, on #v4l and along the Renesas rcar-csi2 patchsets).
> > > >
> > > >> Best regards,
> > > >> Tomasz
> > > >>
> > > >>
> > > >> On Fri, Nov 24, 2017 at 5:55 PM, Sakari Ailus
> > > >>
> > > >> <sakari.ailus@xxxxxxxxxxxxxxx> wrote:
> > > >> > Hi Jacob,
> > > >> >
> > > >> > On Fri, Nov 24, 2017 at 09:00:14AM +0800, Jacob Chen wrote:
> > > >> >> Hi Sakari,
> > > >> >>
> > > >> >> I encountered a problem when using async sub-notifiers.
> > > >> >>
> > > >> >> It's like that:
> > > >> >> There are two notifiers, and they are waiting for one subdev.
> > > >> >> When this subdev is probing, only one notifier is completed and
> > > >> >>
> > > >> >> the other one is skipped.
> > > >> >
> > > >> > Do you have a graph that has two master drivers (that register the
> > > >> > notifier) and both are connected to the same sub-device? Could you
> > > >> > provide
> > > >> > exact graph you have?
> > > >> >
> > > >> >> I found that in v15 of patch "v4l: async: Allow binding notifiers to
> > > >> >> sub-devices", "v4l2_async_notifier_complete" is replaced by
> > > >> >> v4l2_async_notifier_call_complete, which make it only complete one
> > > >> >> notifier.
> > > >> >>
> > > >> >> Why is it changed? Can this be fixed?
> > > >> >
> > > >> > --
> > > >> > Sakari Ailus
> > > >> > sakari.ailus@xxxxxxxxxxxxxxx
> > > >
> > > > --
> > > > Sakari Ailus
> > > > sakari.ailus@xxxxxxxxxxxxxxx
>
> I make a mistake, they are not expecting same subdev.....
>
> The problem is that a notifier regsitered by
> `v4l2_async_subdev_notifier_register` will never be completed, becuase
> 1.`notifier->waiting` is always not empty, so v4l2_async_notifier_try_complete
> won't be called.
> 2. In old code, it's called by its parent, but now it won't.
Could you provide a bit more context, what exactly fails and in which order
the notifiers are registered and async sub-device matches are found?
>
>
> > static int v4l2_async_notifier_try_complete(
> > struct v4l2_async_notifier *notifier)
> > {
> > /* Quick check whether there are still more sub-devices here. */
> > if (!list_empty(¬ifier->waiting))
> > return 0;
>
> not empty
>
> > static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
> > struct v4l2_device *v4l2_dev,
> > struct v4l2_subdev *sd,
> > struct v4l2_async_subdev *asd)
> > {
> > struct v4l2_async_notifier *subdev_notifier;
> > int ret;
> >
> > ret = v4l2_device_register_subdev(v4l2_dev, sd);
> > if (ret < 0)
> > return ret;
> >
> > ret = v4l2_async_notifier_call_bound(notifier, sd, asd);
> > if (ret < 0) {
> > v4l2_device_unregister_subdev(sd);
> > return ret;
> > }
> >
> > /* Remove from the waiting list */
> > list_del(&asd->list);
>
> asd is removed from the waiting list in `v4l2_async_match_notify`, but
>
> >static int v4l2_async_notifier_try_all_subdevs(
> > struct v4l2_async_notifier *notifier)
> >{
> > ...
> > if (!v4l2_dev)
> > return 0;
>
> A notifier regsitered by `v4l2_async_subdev_notifier_register` will return in
> here.
>
> > ...
> > ret = v4l2_async_match_notify(notifier, v4l2_dev, sd, asd);
>
>
> >int v4l2_async_register_subdev(struct v4l2_subdev *sd)
> >{
> > ...
> > list_for_each_entry(notifier, ¬ifier_list, list) {
> > struct v4l2_device *v4l2_dev =
> > v4l2_async_notifier_find_v4l2_dev(notifier);
> > struct v4l2_async_subdev *asd;
> >
> > if (!v4l2_dev)
> > continue;
> >
>
> same
>
> > asd = v4l2_async_find_match(notifier, sd);
> > if (!asd)
> > continue;
> >
> > ret = v4l2_async_match_notify(notifier, notifier->v4l2_dev, sd,
> > asd);
>
>
>
>
--
Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxx
