Hyvää huomenta Laurent,
On Sat, Jul 09, 2022 at 09:49:32PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
>
> On Sat, Jul 09, 2022 at 07:01:23PM +0300, Sakari Ailus wrote:
> > For camera sensor devices the firmware information comes from non-DT (or
>
> Did you mean "camera sensor devices whose formation information comes
> from non-DT" ?
I'd say "of which", but yes.
>
> > some ACPI variants), the kernel makes the information visible to the
> > drivers in a form similar to DT. This takes place through device's
> > secondary fwnodes, in which case also the secondary fwnode needs to be
> > heterogenously (endpoint vs. device) matched.
> >
> > Fixes: 1f391df44607 ("media: v4l2-async: Use endpoints in __v4l2_async_nf_add_fwnode_remote()")
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > ---
> > With correct indentation this time.
> >
> > drivers/media/v4l2-core/v4l2-async.c | 49 ++++++++++++++++------------
> > 1 file changed, 28 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> > index c6995718237a4..2db5d7a8af82b 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -66,8 +66,10 @@ static bool match_i2c(struct v4l2_async_notifier *notifier,
> > #endif
> > }
> >
> > -static bool match_fwnode(struct v4l2_async_notifier *notifier,
> > - struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> > +static bool
> > +match_fwnode_one(struct v4l2_async_notifier *notifier,
> > + struct v4l2_subdev *sd, struct fwnode_handle *sd_fwnode,
> > + struct v4l2_async_subdev *asd)
> > {
> > struct fwnode_handle *other_fwnode;
> > struct fwnode_handle *dev_fwnode;
> > @@ -75,22 +77,6 @@ static bool match_fwnode(struct v4l2_async_notifier *notifier,
> > bool sd_fwnode_is_ep;
> > struct device *dev;
> >
> > - /*
> > - * Both the subdev and the async subdev can provide either an endpoint
> > - * fwnode or a device fwnode. Start with the simple case of direct
> > - * fwnode matching.
> > - */
> > - if (sd->fwnode == asd->match.fwnode)
> > - return true;
> > -
> > - /*
> > - * Check the same situation for any possible secondary assigned to the
> > - * subdev's fwnode
> > - */
> > - if (!IS_ERR_OR_NULL(sd->fwnode->secondary) &&
> > - sd->fwnode->secondary == asd->match.fwnode)
> > - return true;
>
> This check is now gone, is it not needed ?
It is. I'll send v3, moving the direct check here.
Thanks for the review!
>
> > -
> > /*
> > * Otherwise, check if the sd fwnode and the asd fwnode refer to an
> > * endpoint or a device. If they're of the same type, there's no match.
> > @@ -99,7 +85,7 @@ static bool match_fwnode(struct v4l2_async_notifier *notifier,
> > * ACPI. This won't make a difference, as drivers should not try to
> > * match unconnected endpoints.
> > */
> > - sd_fwnode_is_ep = fwnode_graph_is_endpoint(sd->fwnode);
> > + sd_fwnode_is_ep = fwnode_graph_is_endpoint(sd_fwnode);
> > asd_fwnode_is_ep = fwnode_graph_is_endpoint(asd->match.fwnode);
> >
> > if (sd_fwnode_is_ep == asd_fwnode_is_ep)
> > @@ -110,11 +96,11 @@ static bool match_fwnode(struct v4l2_async_notifier *notifier,
> > * parent of the endpoint fwnode, and compare it with the other fwnode.
> > */
> > if (sd_fwnode_is_ep) {
> > - dev_fwnode = fwnode_graph_get_port_parent(sd->fwnode);
> > + dev_fwnode = fwnode_graph_get_port_parent(sd_fwnode);
> > other_fwnode = asd->match.fwnode;
> > } else {
> > dev_fwnode = fwnode_graph_get_port_parent(asd->match.fwnode);
> > - other_fwnode = sd->fwnode;
> > + other_fwnode = sd_fwnode;
> > }
> >
> > fwnode_handle_put(dev_fwnode);
> > @@ -143,6 +129,27 @@ static bool match_fwnode(struct v4l2_async_notifier *notifier,
> > return true;
> > }
> >
> > +static bool match_fwnode(struct v4l2_async_notifier *notifier,
> > + struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> > +{
> > + /*
> > + * Both the subdev and the async subdev can provide either an endpoint
> > + * fwnode or a device fwnode. Start with the simple case of direct
> > + * fwnode matching.
> > + */
> > + if (sd->fwnode == asd->match.fwnode)
> > + return true;
> > +
> > + if (match_fwnode_one(notifier, sd, sd->fwnode, asd))
> > + return true;
> > +
> > + /* Also check the secondary fwnode. */
> > + if (IS_ERR_OR_NULL(sd->fwnode->secondary))
> > + return false;
> > +
> > + return match_fwnode_one(notifier, sd, sd->fwnode->secondary, asd);
> > +}
> > +
> > static LIST_HEAD(subdev_list);
> > static LIST_HEAD(notifier_list);
> > static DEFINE_MUTEX(list_lock);
--
Terveisin,
Sakari Ailus
