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@xxxxxxxx> 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@xxxxxxxx> (v1 code) > > Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx> (v1 commit message) > > Cc: Ramalingam C <ramalingam.c@xxxxxxxxx> > > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > Cc: Russell King <rmk+kernel@xxxxxxxxxxxxxxxx> > > Cc: Rafael J. Wysocki <rafael@xxxxxxxxxx> > > Cc: Jaroslav Kysela <perex@xxxxxxxx> > > Cc: Takashi Iwai <tiwai@xxxxxxxx> > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > --- > > 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, so calling one unconditionally blows up. Can still be streamlined a bit I think. -Daniel > > > } > > > > return NULL; > > @@ -192,7 +199,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; > > @@ -327,30 +334,12 @@ static int component_match_realloc(struct device *dev, > > return 0; > > } > > > > -/** > > - * component_match_add_release - add a component match with release callback > > - * @master: device with the aggregate driver > > - * @matchptr: pointer to the list of component matches > > - * @release: release function for @compare_data > > - * @compare: compare function to match against all components > > - * @compare_data: opaque pointer passed to the @compare function > > - * > > - * This adds a new component match to the list stored in @matchptr, which the > > "This" appears to be redundant here (and in some places below too). -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel