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

[Rafael J. Wysocki]

) On Wed, Feb 6, 2019 at 5:46 PM Daniel Vetter <daniel.vetter at 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 at ffwll.ch> (v1 code)
> Signed-off-by: Ramalingam C <ramalingam.c at intel.com> (v1 commit message)
> Cc: Ramalingam C <ramalingam.c at intel.com>
> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> Cc: Russell King <rmk+kernel at arm.linux.org.uk>
> 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>
> Signed-off-by: Daniel Vetter <daniel.vetter at 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?

>         }
>
>         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).