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" ?
> 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 ?
> -
> /*
> * 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);
--
Regards,
Laurent Pinchart
