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+rene...@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+rene...@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.ai...@iki.fi   XMPP: sai...@retiisi.org.uk

-- 
Regards,
Niklas Söderlund

Reply via email to