2020-03-15 10:28:11

by Lad, Prabhakar

Subject: [PATCH 1/2] media: v4l2-async: Pass pointer to struct v4l2_subdev in match_custom callback

Passing a pointer to struct device for the match_custom callback is of no
use as in the bridge driver to match the fwnode, so instead pass the
struct v4l2_subdev pointer so that the bridge driver has enough
information to match against the subdevices.

Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/media/v4l2-core/v4l2-async.c | 2 +-
include/media/v4l2-async.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 8bde33c21ce4..f897d4025f97 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -80,7 +80,7 @@ static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
/* Match always */
return true;

- return asd->match.custom.match(sd->dev, asd);
+ return asd->match.custom.match(sd, asd);
}

static LIST_HEAD(subdev_list);
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 8319284c93cb..8c014e3bbd6c 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -86,8 +86,8 @@ struct v4l2_async_subdev {
unsigned short address;
} i2c;
struct {
- bool (*match)(struct device *dev,
- struct v4l2_async_subdev *sd);
+ bool (*match)(struct v4l2_subdev *sd,
+ struct v4l2_async_subdev *asd);
void *priv;
} custom;
} match;
--
2.20.1

2020-03-15 10:57:06

by Laurent Pinchart

Subject: Re: [PATCH 1/2] media: v4l2-async: Pass pointer to struct v4l2_subdev in match_custom callback

Hi Prabhakar,

Thank you for the patch.

On Sun, Mar 15, 2020 at 10:27:23AM +0000, Lad Prabhakar wrote:
> Passing a pointer to struct device for the match_custom callback is of no
> use as in the bridge driver to match the fwnode, so instead pass the
> struct v4l2_subdev pointer so that the bridge driver has enough
> information to match against the subdevices.

I'm not sure I like this. Conceptually speaking, the driver that
registers the notifier wants to get v4l2_subdev instances corresponding
to devices. A struct device is thus all it should need. Giving the match
function access to the subdev opens the door to all kind of nasty hacks.

In any case, I don't think is is required, see my reply to patch 2/2.

> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> drivers/media/v4l2-core/v4l2-async.c | 2 +-
> include/media/v4l2-async.h | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index 8bde33c21ce4..f897d4025f97 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -80,7 +80,7 @@ static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> /* Match always */
> return true;
>
> - return asd->match.custom.match(sd->dev, asd);
> + return asd->match.custom.match(sd, asd);
> }
>
> static LIST_HEAD(subdev_list);
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 8319284c93cb..8c014e3bbd6c 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -86,8 +86,8 @@ struct v4l2_async_subdev {
> unsigned short address;
> } i2c;
> struct {
> - bool (*match)(struct device *dev,
> - struct v4l2_async_subdev *sd);
> + bool (*match)(struct v4l2_subdev *sd,
> + struct v4l2_async_subdev *asd);
> void *priv;
> } custom;
> } match;

--
Regards,

Laurent Pinchart