Re: [PATCH 2/3] components: multiple components for a device

Rafael J. Wysocki
2019-02-07T22:48:54Z

On Thu, Feb 7, 2019 at 11:40 PM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Wed, Feb 06, 2019 at 11:57:04PM +0100, Rafael J. Wysocki wrote:
> > ) On Wed, Feb 6, 2019 at 5:46 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > >
> > > 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.
> > >
> > > v2: Add documenation.
> > >
> > > v3: Rebase on top of updated documenation.
> > >
> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v1 code)
> > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> (v1 commit message)
> > > Cc: Ramalingam C <ramalingam.c@intel.com>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> > > Cc: Rafael J. Wysocki <rafael@kernel.org>
> > > Cc: Jaroslav Kysela <perex@perex.cz>
> > > Cc: Takashi Iwai <tiwai@suse.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > ---
> > > drivers/base/component.c | 160 +++++++++++++++++++++++++++++---------
> > > include/linux/component.h | 9 ++-
> > > 2 files changed, 129 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/drivers/base/component.c b/drivers/base/component.c
> > > index f34d4b784709..68ccd5a0d5d6 100644
> > > --- a/drivers/base/component.c
> > > +++ b/drivers/base/component.c
> > > @@ -47,6 +47,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;
> > > @@ -74,6 +75,7 @@ struct component {
> > > bool bound;
> > >
> > > const struct component_ops *ops;
> > > + int subcomponent;
> > > struct device *dev;
> > > };
> > >
> > > @@ -158,7 +160,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;
> > >
> > > @@ -166,8 +168,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))
> >
> > This line break looks kind of weird to me,
> >
> > > + return c;
> > > + } else if (mc->compare(c->dev, mc->data)) {
> > > return c;
> > > + }
> >
> > Also, why don't you do
> >
> > if (mc->compare(c->dev, mc->data) || (mc->compare_typed &&
> > mc->compare_typed(c->dev, c->subcomponent, mc->data)))
> > return c;
> >
> > The only difference is that ->compare() will run first and if it finds
> > a match, c will be returned right away. Does it matter?
>
> Tried it, yes it does matter: We have either ->compare or ->compare_typed,

Well, that's not obvious. :-)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org