[PATCH v10 01/39] components: multiple components for a device

Greg Kroah-Hartman gregkh at linuxfoundation.org
Tue Jan 29 16:11:54 UTC 2019


On Tue, Jan 29, 2019 at 05:11:16PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Jan 29, 2019 at 05:01:50PM +0100, Daniel Vetter wrote:
> > On Tue, Jan 29, 2019 at 4:22 PM Greg Kroah-Hartman
> > <gregkh at linuxfoundation.org> wrote:
> > >
> > > On Tue, Jan 29, 2019 at 08:12:49PM +0530, Ramalingam C wrote:
> > > > From: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > >
> > > > Component framework is extended to support multiple components for
> > > > a struct device. These will be matched with different masters based on
> > > > its sub component value.
> > > >
> > > > We are introducing this, as I915 needs two different components
> > > > with different subcomponent value, which will be matched to two
> > > > different component masters(Audio and HDCP) based on the subcomponent
> > > > values.
> > > >
> > > > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > > Signed-off-by: Ramalingam C <ramalingam.c at intel.com>
> > > > cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> > > > cc: Rafael J. Wysocki <rafael at kernel.org>
> > > > cc: Jaroslav Kysela <perex at perex.cz>
> > > > cc: Takashi Iwai <tiwai at suse.com>
> > > > cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > > cc: Jani Nikula <jani.nikula at linux.intel.com>
> > > > cc: Lin, Mengdong <mengdong.lin at intel.com>
> > > > cc: Libin Yang <libin.yang at linux.intel.com>
> > > > ---
> > > >  drivers/base/component.c  | 67 +++++++++++++++++++++++++++++++++++++++--------
> > > >  include/linux/component.h |  5 ++++
> > > >  2 files changed, 61 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/base/component.c b/drivers/base/component.c
> > > > index ddcea8739c12..30309c0449ed 100644
> > > > --- a/drivers/base/component.c
> > > > +++ b/drivers/base/component.c
> > > > @@ -21,6 +21,7 @@ struct component;
> > > >  struct component_match_array {
> > > >       void *data;
> > > >       int (*compare)(struct device *, void *);
> > > > +     int (*compare_typed)(struct device *, int, void *);
> > > >       void (*release)(struct device *, void *);
> > > >       struct component *component;
> > > >       bool duplicate;
> > > > @@ -48,6 +49,7 @@ struct component {
> > > >       bool bound;
> > > >
> > > >       const struct component_ops *ops;
> > > > +     int subcomponent;
> > > >       struct device *dev;
> > > >  };
> > > >
> > > > @@ -132,7 +134,7 @@ static struct master *__master_find(struct device *dev,
> > > >  }
> > > >
> > > >  static struct component *find_component(struct master *master,
> > > > -     int (*compare)(struct device *, void *), void *compare_data)
> > > > +     struct component_match_array *mc)
> > > >  {
> > > >       struct component *c;
> > > >
> > > > @@ -140,8 +142,13 @@ static struct component *find_component(struct master *master,
> > > >               if (c->master && c->master != master)
> > > >                       continue;
> > > >
> > > > -             if (compare(c->dev, compare_data))
> > > > +             if (mc->compare_typed) {
> > > > +                     if (mc->compare_typed(c->dev, c->subcomponent,
> > > > +                                           mc->data))
> > > > +                             return c;
> > > > +             } else if (mc->compare(c->dev, mc->data)) {
> > > >                       return c;
> > > > +             }
> > > >       }
> > > >
> > > >       return NULL;
> > > > @@ -166,7 +173,7 @@ static int find_components(struct master *master)
> > > >               if (match->compare[i].component)
> > > >                       continue;
> > > >
> > > > -             c = find_component(master, mc->compare, mc->data);
> > > > +             c = find_component(master, mc);
> > > >               if (!c) {
> > > >                       ret = -ENXIO;
> > > >                       break;
> > > > @@ -301,15 +308,12 @@ static int component_match_realloc(struct device *dev,
> > > >       return 0;
> > > >  }
> > > >
> > > > -/*
> > > > - * Add a component to be matched, with a release function.
> > > > - *
> > > > - * The match array is first created or extended if necessary.
> > > > - */
> > > > -void component_match_add_release(struct device *master,
> > > > +static void __component_match_add(struct device *master,
> > > >       struct component_match **matchptr,
> > > >       void (*release)(struct device *, void *),
> > > > -     int (*compare)(struct device *, void *), void *compare_data)
> > > > +     int (*compare)(struct device *, void *),
> > > > +     int (*compare_typed)(struct device *, int, void *),
> > > > +     void *compare_data)
> > > >  {
> > > >       struct component_match *match = *matchptr;
> > > >
> > > > @@ -341,13 +345,37 @@ void component_match_add_release(struct device *master,
> > > >       }
> > > >
> > > >       match->compare[match->num].compare = compare;
> > > > +     match->compare[match->num].compare_typed = compare_typed;
> > > >       match->compare[match->num].release = release;
> > > >       match->compare[match->num].data = compare_data;
> > > >       match->compare[match->num].component = NULL;
> > > >       match->num++;
> > > >  }
> > > > +
> > > > +/*
> > > > + * Add a component to be matched, with a release function.
> > > > + *
> > > > + * The match array is first created or extended if necessary.
> > > > + */
> > > > +void component_match_add_release(struct device *master,
> > > > +     struct component_match **matchptr,
> > > > +     void (*release)(struct device *, void *),
> > > > +     int (*compare)(struct device *, void *), void *compare_data)
> > > > +{
> > > > +     __component_match_add(master, matchptr, release, compare, NULL,
> > > > +                           compare_data);
> > > > +}
> > > >  EXPORT_SYMBOL(component_match_add_release);
> > > >
> > > > +void component_match_add_typed(struct device *master,
> > > > +     struct component_match **matchptr,
> > > > +     int (*compare_typed)(struct device *, int, void *), void *compare_data)
> > > > +{
> > > > +     __component_match_add(master, matchptr, NULL, NULL, compare_typed,
> > > > +                           compare_data);
> > > > +}
> > > > +EXPORT_SYMBOL(component_match_add_typed);
> > > > +
> > > >  static void free_master(struct master *master)
> > > >  {
> > > >       struct component_match *match = master->match;
> > > > @@ -537,7 +565,8 @@ int component_bind_all(struct device *master_dev, void *data)
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(component_bind_all);
> > > >
> > > > -int component_add(struct device *dev, const struct component_ops *ops)
> > > > +static int __component_add(struct device *dev, const struct component_ops *ops,
> > > > +     int subcomponent)
> > > >  {
> > > >       struct component *component;
> > > >       int ret;
> > > > @@ -548,6 +577,7 @@ int component_add(struct device *dev, const struct component_ops *ops)
> > > >
> > > >       component->ops = ops;
> > > >       component->dev = dev;
> > > > +     component->subcomponent = subcomponent;
> > > >
> > > >       dev_dbg(dev, "adding component (ops %ps)\n", ops);
> > > >
> > > > @@ -566,6 +596,21 @@ int component_add(struct device *dev, const struct component_ops *ops)
> > > >
> > > >       return ret < 0 ? ret : 0;
> > > >  }
> > > > +
> > > > +int component_add_typed(struct device *dev, const struct component_ops *ops,
> > > > +     int subcomponent)
> > > > +{
> > > > +     if (WARN_ON(subcomponent == 0))
> > > > +             return -EINVAL;
> > > > +
> > > > +     return __component_add(dev, ops, subcomponent);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(component_add_typed);
> > > > +
> > > > +int component_add(struct device *dev, const struct component_ops *ops)
> > > > +{
> > > > +     return __component_add(dev, ops, 0);
> > > > +}
> > > >  EXPORT_SYMBOL_GPL(component_add);
> > > >
> > > >  void component_del(struct device *dev, const struct component_ops *ops)
> > > > diff --git a/include/linux/component.h b/include/linux/component.h
> > > > index e71fbbbc74e2..6d7516f0546e 100644
> > > > --- a/include/linux/component.h
> > > > +++ b/include/linux/component.h
> > > > @@ -14,6 +14,8 @@ struct component_ops {
> > > >  };
> > > >
> > > >  int component_add(struct device *, const struct component_ops *);
> > > > +int component_add_typed(struct device *dev, const struct component_ops *ops,
> > > > +     int subcomponent);
> > > >  void component_del(struct device *, const struct component_ops *);
> > > >
> > > >  int component_bind_all(struct device *master, void *master_data);
> > > > @@ -37,6 +39,9 @@ void component_match_add_release(struct device *master,
> > > >       struct component_match **matchptr,
> > > >       void (*release)(struct device *, void *),
> > > >       int (*compare)(struct device *, void *), void *compare_data);
> > > > +void component_match_add_typed(struct device *master,
> > > > +     struct component_match **matchptr,
> > > > +     int (*compare_typed)(struct device *, int, void *), void *compare_data);
> > >
> > > I think we need some documentation somewhere as to exactly what these
> > > functions do, as I can not determine it from the patch :(
> > 
> > I did notice too that component.c doesn't have kerneldoc, and it's
> > somewhere on my backburner. Do we need this first? Or just docs for
> > the new stuff (which isn't really going to be any use without docs for
> > the existing stuff I think).
> 
> Ideally, docs for the whole thing as I have no idea how it works, so
> it's hard to review any patches for it.  Ideally you would cc: Russell
> on any changes here, as he is the original creator of this code.

Sorry hit send too early...

"Ideally, yes", but I would settle for just at least documenting new
stuff, otherwise it will never get done.

thanks,

greg k-h


More information about the Intel-gfx-trybot mailing list