| Message ID | 1417166286-27685-5-git-send-email-j.anaszewski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri 2014-11-28 10:17:56, Jacek Anaszewski wrote: > It is useful to have an access to the async sub-device > being matched, not only to the related struct device. > Change match callback argument from struct device > to struct v4l2_subdev. It will allow e.g. for matching > a sub-device by its "name" property. > > Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Hans Verkuil <hans.verkuil@cisco.com> Acked-by: Pavel Machek <pavel@ucw.cz>
Hi Jacek, Thank you for the patch. On Friday 28 November 2014 10:17:56 Jacek Anaszewski wrote: > It is useful to have an access to the async sub-device > being matched, not only to the related struct device. > Change match callback argument from struct device > to struct v4l2_subdev. It will allow e.g. for matching > a sub-device by its "name" property. In principle I agree. However, we will need to reimplement v4l2-async based on the component (drivers/base/component.c) framework at some point. As the component framework is based on struct device, will it still be possible to match on subdev name in that case ? If not, we might need to try to find another approach to the issue. > Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Hans Verkuil <hans.verkuil@cisco.com> > --- > drivers/media/v4l2-core/v4l2-async.c | 16 ++++++++-------- > include/media/v4l2-async.h | 2 +- > 2 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-async.c > b/drivers/media/v4l2-core/v4l2-async.c index 85a6a34..8140992 100644 > --- a/drivers/media/v4l2-core/v4l2-async.c > +++ b/drivers/media/v4l2-core/v4l2-async.c > @@ -22,10 +22,10 @@ > #include <media/v4l2-device.h> > #include <media/v4l2-subdev.h> > > -static bool match_i2c(struct device *dev, struct v4l2_async_subdev *asd) > +static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev > *asd) { > #if IS_ENABLED(CONFIG_I2C) > - struct i2c_client *client = i2c_verify_client(dev); > + struct i2c_client *client = i2c_verify_client(sd->dev); > return client && > asd->match.i2c.adapter_id == client->adapter->nr && > asd->match.i2c.address == client->addr; > @@ -34,14 +34,14 @@ static bool match_i2c(struct device *dev, struct > v4l2_async_subdev *asd) #endif > } > > -static bool match_devname(struct device *dev, struct v4l2_async_subdev > *asd) +static bool match_devname(struct v4l2_subdev *sd, struct > v4l2_async_subdev *asd) { > - return !strcmp(asd->match.device_name.name, dev_name(dev)); > + return !strcmp(asd->match.device_name.name, dev_name(sd->dev)); > } > > -static bool match_of(struct device *dev, struct v4l2_async_subdev *asd) > +static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd) > { > - return dev->of_node == asd->match.of.node; > + return sd->dev->of_node == asd->match.of.node; > } > > static LIST_HEAD(subdev_list); > @@ -52,7 +52,7 @@ static struct v4l2_async_subdev *v4l2_async_belongs(struct > v4l2_async_notifier * struct v4l2_subdev *sd) > { > struct v4l2_async_subdev *asd; > - bool (*match)(struct device *, struct v4l2_async_subdev *); > + bool (*match)(struct v4l2_subdev *, struct v4l2_async_subdev *); > > list_for_each_entry(asd, ¬ifier->waiting, list) { > /* bus_type has been verified valid before */ > @@ -79,7 +79,7 @@ static struct v4l2_async_subdev *v4l2_async_belongs(struct > v4l2_async_notifier * } > > /* match cannot be NULL here */ > - if (match(sd->dev, asd)) > + if (match(sd, asd)) > return asd; > } > > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h > index 7683569..1c0b586 100644 > --- a/include/media/v4l2-async.h > +++ b/include/media/v4l2-async.h > @@ -51,7 +51,7 @@ struct v4l2_async_subdev { > unsigned short address; > } i2c; > struct { > - bool (*match)(struct device *, > + bool (*match)(struct v4l2_subdev *, > struct v4l2_async_subdev *); > void *priv; > } custom;
Hi Laurent, On 11/29/2014 05:38 PM, Laurent Pinchart wrote: > Hi Jacek, > > Thank you for the patch. > > On Friday 28 November 2014 10:17:56 Jacek Anaszewski wrote: >> It is useful to have an access to the async sub-device >> being matched, not only to the related struct device. >> Change match callback argument from struct device >> to struct v4l2_subdev. It will allow e.g. for matching >> a sub-device by its "name" property. > > In principle I agree. However, we will need to reimplement v4l2-async based on > the component (drivers/base/component.c) framework at some point. As the > component framework is based on struct device, will it still be possible to > match on subdev name in that case ? If not, we might need to try to find > another approach to the issue. There were reservations raised [1] concerning the way of matching by name, as the labels are not guaranteed to be globally unique across Device Tree. I admit, this issue has to be solved in a different way. Especially in view of prospective transition to using drivers/base/component.c I propose to add a new structure: struct v4l2_asd_match{ bool (*match)(struct v4l2_async_subdev *, void *); void *priv; } and a function: v4l2_async_register_subdev_with_match(struct v4l2_subdev *sd, struct v4l2_asd_match*). This way we could pass DT sub-node related to a sub-led in a priv field of v4l2_asd_match upon registration. This is similar approach as in case of drivers/base/component.c. Best Regards, Jacek Anaszewski [1] http://www.spinics.net/lists/linux-leds/msg02532.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index 85a6a34..8140992 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -22,10 +22,10 @@ #include <media/v4l2-device.h> #include <media/v4l2-subdev.h> -static bool match_i2c(struct device *dev, struct v4l2_async_subdev *asd) +static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd) { #if IS_ENABLED(CONFIG_I2C) - struct i2c_client *client = i2c_verify_client(dev); + struct i2c_client *client = i2c_verify_client(sd->dev); return client && asd->match.i2c.adapter_id == client->adapter->nr && asd->match.i2c.address == client->addr; @@ -34,14 +34,14 @@ static bool match_i2c(struct device *dev, struct v4l2_async_subdev *asd) #endif } -static bool match_devname(struct device *dev, struct v4l2_async_subdev *asd) +static bool match_devname(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd) { - return !strcmp(asd->match.device_name.name, dev_name(dev)); + return !strcmp(asd->match.device_name.name, dev_name(sd->dev)); } -static bool match_of(struct device *dev, struct v4l2_async_subdev *asd) +static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd) { - return dev->of_node == asd->match.of.node; + return sd->dev->of_node == asd->match.of.node; } static LIST_HEAD(subdev_list); @@ -52,7 +52,7 @@ static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier * struct v4l2_subdev *sd) { struct v4l2_async_subdev *asd; - bool (*match)(struct device *, struct v4l2_async_subdev *); + bool (*match)(struct v4l2_subdev *, struct v4l2_async_subdev *); list_for_each_entry(asd, ¬ifier->waiting, list) { /* bus_type has been verified valid before */ @@ -79,7 +79,7 @@ static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier * } /* match cannot be NULL here */ - if (match(sd->dev, asd)) + if (match(sd, asd)) return asd; } diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h index 7683569..1c0b586 100644 --- a/include/media/v4l2-async.h +++ b/include/media/v4l2-async.h @@ -51,7 +51,7 @@ struct v4l2_async_subdev { unsigned short address; } i2c; struct { - bool (*match)(struct device *, + bool (*match)(struct v4l2_subdev *, struct v4l2_async_subdev *); void *priv; } custom;