| Message ID | 20170524000907.13061-2-niklas.soderlund@ragnatech.se (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hi Niklas, On Wed, May 24, 2017 at 02:09:06AM +0200, Niklas Söderlund wrote: > From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > The optional operation can be used by entities to report how it maps its > fwnode endpoints to media pad numbers. This is useful for devices which > require advanced mappings of pads. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > include/media/media-entity.h | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > index c7c254c5bca1761b..2aea22b0409d1070 100644 > --- a/include/media/media-entity.h > +++ b/include/media/media-entity.h > @@ -21,6 +21,7 @@ > > #include <linux/bitmap.h> > #include <linux/bug.h> > +#include <linux/fwnode.h> > #include <linux/kernel.h> > #include <linux/list.h> > #include <linux/media.h> > @@ -171,6 +172,9 @@ struct media_pad { > > /** > * struct media_entity_operations - Media entity operations > + * @pad_from_fwnode: Return the pad number based on a fwnode endpoint. > + * This operation can be used to map a fwnode to a > + * media pad number. Optional. > * @link_setup: Notify the entity of link changes. The operation can > * return an error, in which case link setup will be > * cancelled. Optional. > @@ -184,6 +188,8 @@ struct media_pad { > * mutex held. > */ > struct media_entity_operations { > + int (*pad_from_fwnode)(struct fwnode_endpoint *endpoint, > + unsigned int *pad); Hmm. How about calling this get_fwnode_pad for instance? I wonder what others think. You could just return the pad number still, and a negative value on error. I think we won't have more than INT_MAX pads. :-) > int (*link_setup)(struct media_entity *entity, > const struct media_pad *local, > const struct media_pad *remote, u32 flags);
Hi Sakari, Thanks for your feedback. On 2017-05-24 16:21:37 +0300, Sakari Ailus wrote: > Hi Niklas, > > On Wed, May 24, 2017 at 02:09:06AM +0200, Niklas Söderlund wrote: > > From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > > > The optional operation can be used by entities to report how it maps its > > fwnode endpoints to media pad numbers. This is useful for devices which > > require advanced mappings of pads. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > --- > > include/media/media-entity.h | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > > index c7c254c5bca1761b..2aea22b0409d1070 100644 > > --- a/include/media/media-entity.h > > +++ b/include/media/media-entity.h > > @@ -21,6 +21,7 @@ > > > > #include <linux/bitmap.h> > > #include <linux/bug.h> > > +#include <linux/fwnode.h> > > #include <linux/kernel.h> > > #include <linux/list.h> > > #include <linux/media.h> > > @@ -171,6 +172,9 @@ struct media_pad { > > > > /** > > * struct media_entity_operations - Media entity operations > > + * @pad_from_fwnode: Return the pad number based on a fwnode endpoint. > > + * This operation can be used to map a fwnode to a > > + * media pad number. Optional. > > * @link_setup: Notify the entity of link changes. The operation can > > * return an error, in which case link setup will be > > * cancelled. Optional. > > @@ -184,6 +188,8 @@ struct media_pad { > > * mutex held. > > */ > > struct media_entity_operations { > > + int (*pad_from_fwnode)(struct fwnode_endpoint *endpoint, > > + unsigned int *pad); > > Hmm. How about calling this get_fwnode_pad for instance? I wonder what > others think. I'm OK with this name change, will update for next version. > > You could just return the pad number still, and a negative value on error. I > think we won't have more than INT_MAX pads. :-) I did that at first but then I remembered all the review comments I have gotten earlier about using int as the type for pads :-) If you and others agree in this case returning the pad as int or a negative value as error I have no problem chaining this for the next version. > > > int (*link_setup)(struct media_entity *entity, > > const struct media_pad *local, > > const struct media_pad *remote, u32 flags); > > -- > Regards, > > Sakari Ailus > e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
Hejssan, On Wed, May 24, 2017 at 04:07:36PM +0200, Niklas Söderlund wrote: > Hi Sakari, > > Thanks for your feedback. > > On 2017-05-24 16:21:37 +0300, Sakari Ailus wrote: > > Hi Niklas, > > > > On Wed, May 24, 2017 at 02:09:06AM +0200, Niklas Söderlund wrote: > > > From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > > > > > The optional operation can be used by entities to report how it maps its > > > fwnode endpoints to media pad numbers. This is useful for devices which > > > require advanced mappings of pads. > > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > > --- > > > include/media/media-entity.h | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > > > index c7c254c5bca1761b..2aea22b0409d1070 100644 > > > --- a/include/media/media-entity.h > > > +++ b/include/media/media-entity.h > > > @@ -21,6 +21,7 @@ > > > > > > #include <linux/bitmap.h> > > > #include <linux/bug.h> > > > +#include <linux/fwnode.h> > > > #include <linux/kernel.h> > > > #include <linux/list.h> > > > #include <linux/media.h> > > > @@ -171,6 +172,9 @@ struct media_pad { > > > > > > /** > > > * struct media_entity_operations - Media entity operations > > > + * @pad_from_fwnode: Return the pad number based on a fwnode endpoint. > > > + * This operation can be used to map a fwnode to a > > > + * media pad number. Optional. > > > * @link_setup: Notify the entity of link changes. The operation can > > > * return an error, in which case link setup will be > > > * cancelled. Optional. > > > @@ -184,6 +188,8 @@ struct media_pad { > > > * mutex held. > > > */ > > > struct media_entity_operations { > > > + int (*pad_from_fwnode)(struct fwnode_endpoint *endpoint, > > > + unsigned int *pad); > > > > Hmm. How about calling this get_fwnode_pad for instance? I wonder what > > others think. > > I'm OK with this name change, will update for next version. > > > > > You could just return the pad number still, and a negative value on error. I > > think we won't have more than INT_MAX pads. :-) > > I did that at first but then I remembered all the review comments I have > gotten earlier about using int as the type for pads :-) If you and > others agree in this case returning the pad as int or a negative value > as error I have no problem chaining this for the next version. unsigned int was proposed for there was no need for negative values. In this case there is. I don't really have a strong opinion either way. I wonder what Hans or Laurent thinks.
diff --git a/include/media/media-entity.h b/include/media/media-entity.h index c7c254c5bca1761b..2aea22b0409d1070 100644 --- a/include/media/media-entity.h +++ b/include/media/media-entity.h @@ -21,6 +21,7 @@ #include <linux/bitmap.h> #include <linux/bug.h> +#include <linux/fwnode.h> #include <linux/kernel.h> #include <linux/list.h> #include <linux/media.h> @@ -171,6 +172,9 @@ struct media_pad { /** * struct media_entity_operations - Media entity operations + * @pad_from_fwnode: Return the pad number based on a fwnode endpoint. + * This operation can be used to map a fwnode to a + * media pad number. Optional. * @link_setup: Notify the entity of link changes. The operation can * return an error, in which case link setup will be * cancelled. Optional. @@ -184,6 +188,8 @@ struct media_pad { * mutex held. */ struct media_entity_operations { + int (*pad_from_fwnode)(struct fwnode_endpoint *endpoint, + unsigned int *pad); int (*link_setup)(struct media_entity *entity, const struct media_pad *local, const struct media_pad *remote, u32 flags);