[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190204160025.GV3271@phenom.ffwll.local>
Date: Mon, 4 Feb 2019 17:00:25 +0100
From: Daniel Vetter <daniel@...ll.ch>
To: DRI Development <dri-devel@...ts.freedesktop.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Daniel Vetter <daniel.vetter@...ll.ch>,
Ramalingam C <ramalingam.c@...el.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Russell King <rmk+kernel@....linux.org.uk>,
"Rafael J . Wysocki" <rafael@...nel.org>,
Jaroslav Kysela <perex@...ex.cz>,
Takashi Iwai <tiwai@...e.com>,
Rodrigo Vivi <rodrigo.vivi@...el.com>,
Jani Nikula <jani.nikula@...ux.intel.com>
Subject: Re: [PATCH 2/2] components: multiple components for a device
On Thu, Jan 31, 2019 at 03:46:40PM +0100, Daniel Vetter 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.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@...ll.ch> (v1 code)
> Signed-off-by: Ramalingam C <ramalingam.c@...el.com> (v1 commit message)
> Cc: Ramalingam C <ramalingam.c@...el.com>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: Russell King <rmk+kernel@....linux.org.uk>
> Cc: Rafael J. Wysocki <rafael@...nel.org>
> Cc: Jaroslav Kysela <perex@...ex.cz>
> Cc: Takashi Iwai <tiwai@...e.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@...el.com>
> Cc: Jani Nikula <jani.nikula@...ux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@...ll.ch>
Hi Greg&Russell,
Now that there's nice documentation on everything, any comments on these
two patches?
Thanks, Daniel
> ---
> drivers/base/component.c | 159 +++++++++++++++++++++++++++++---------
> include/linux/component.h | 10 ++-
> 2 files changed, 130 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/base/component.c b/drivers/base/component.c
> index e5b04bce8544..eb7915fc5278 100644
> --- a/drivers/base/component.c
> +++ b/drivers/base/component.c
> @@ -48,6 +48,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;
> @@ -75,6 +76,7 @@ struct component {
> bool bound;
>
> const struct component_ops *ops;
> + int subcomponent;
> struct device *dev;
> };
>
> @@ -159,7 +161,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;
>
> @@ -167,8 +169,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;
> @@ -193,7 +200,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;
> @@ -328,29 +335,12 @@ static int component_match_realloc(struct device *dev,
> return 0;
> }
>
> -/**
> - * component_match_add_release - add a compent 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
> - * @master aggregate driver needs to function. @matchptr must be initialized to
> - * NULL before adding the first match.
> - *
> - * The allocated match list in @matchptr is automatically released using devm
> - * actions. At that point @release will be called, to free any references held
> - * by @compare_data, e.g. when @compare_data is a &device_node that must be
> - * released with of_node_put().
> - *
> - * See also component_match_add().
> - */
> -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;
>
> @@ -382,13 +372,69 @@ 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++;
> }
> +
> +/**
> + * component_match_add_release - add a compent 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
> + * @master aggregate driver needs to function. @matchptr must be initialized to
> + * NULL before adding the first match.
> + *
> + * The allocated match list in @matchptr is automatically released using devm
> + * actions. At that point @release will be called, to free any references held
> + * by @compare_data, e.g. when @compare_data is a &device_node that must be
> + * released with of_node_put(). This only matches against components
> + * added with component_add().
> + *
> + * See also component_match_add() and component_match_add_typed().
> + */
> +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);
>
> +/**
> + * component_match_add_typed - add a compent match for a typed component
> + * @master: device with the aggregate driver
> + * @matchptr: pointer to the list of component matches
> + * @compare_typed: compare function to match against all typed components
> + * @compare_data: opaque pointer passed to the @compare function
> + *
> + * This adds a new component match to the list stored in @matchptr, which the
> + * @master aggregate driver needs to function. @matchptr must be initialized to
> + * NULL before adding the first match. This only matches against components
> + * added with component_add_typed().
> + *
> + * The allocated match list in @matchptr is automatically released using devm
> + * actions.
> + *
> + * See also component_match_add_release() and component_match_add_typed().
> + */
> +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;
> @@ -617,19 +663,8 @@ int component_bind_all(struct device *master_dev, void *data)
> }
> EXPORT_SYMBOL_GPL(component_bind_all);
>
> -/**
> - * component_add - register a component
> - * @dev: component device
> - * @ops: component callbacks
> - *
> - * Register a new component for @dev. Functions in @ops will be call when the
> - * aggregate driver is ready to bind the overall driver by calling
> - * component_bind_all(). See also &struct component_ops.
> - *
> - * The component needs to be unregistered again at driver unload/disconnect by
> - * calling component_del().
> - */
> -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;
> @@ -640,6 +675,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);
>
> @@ -658,6 +694,55 @@ int component_add(struct device *dev, const struct component_ops *ops)
>
> return ret < 0 ? ret : 0;
> }
> +
> +/**
> + * component_add_typed - register a component
> + * @dev: component device
> + * @ops: component callbacks
> + * @subcomponent: nonzero identifier for subcomponents
> + *
> + * Register a new component for @dev. Functions in @ops will be call when the
> + * aggregate driver is ready to bind the overall driver by calling
> + * component_bind_all(). See also &struct component_ops.
> + *
> + * @subcomponent must be nonzero and is used to differentiate between multiple
> + * components registerd on the same device @dev. These components are match
> + * using component_match_add_typed().
> + *
> + * The component needs to be unregistered again at driver unload/disconnect by
> + * calling component_del().
> + *
> + * See also component_add().
> + */
> +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);
> +
> +/**
> + * component_add - register a component
> + * @dev: component device
> + * @ops: component callbacks
> + *
> + * Register a new component for @dev. Functions in @ops will be call when the
> + * aggregate driver is ready to bind the overall driver by calling
> + * component_bind_all(). See also &struct component_ops.
> + *
> + * The component needs to be unregistered again at driver unload/disconnect by
> + * calling component_del().
> + *
> + * See also component_add_typed() for a variant that allows multipled different
> + * components on the same device.
> + */
> +int component_add(struct device *dev, const struct component_ops *ops)
> +{
> + return __component_add(dev, ops, 0);
> +}
> EXPORT_SYMBOL_GPL(component_add);
>
> /**
> diff --git a/include/linux/component.h b/include/linux/component.h
> index 67a899dd2e10..9e69e2117f0b 100644
> --- a/include/linux/component.h
> +++ b/include/linux/component.h
> @@ -34,6 +34,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);
> @@ -91,6 +93,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);
>
> /**
> * component_match_add - add a compent match
> @@ -101,12 +106,13 @@ void component_match_add_release(struct device *master,
> *
> * This adds a new component match to the list stored in @matchptr, which the
> * @master aggregate driver needs to function. @matchptr must be initialized to
> - * NULL before adding the first match.
> + * NULL before adding the first match. This only matches against components
> + * added with component_add().
> *
> * The allocated match list in @matchptr is automatically released using devm
> * actions.
> *
> - * See also component_match_add_release().
> + * See also component_match_add_release() and component_match_add_typed().
> */
> static inline void component_match_add(struct device *master,
> struct component_match **matchptr,
> --
> 2.20.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Powered by blists - more mailing lists