On Thu, 06 Jan 2022, Stephen Boyd swboyd@chromium.org wrote:
This allows aggregate driver writers to use the device passed to their probe/remove/shutdown functions properly instead of treating it as an opaque pointer.
You say it like having opaque pointers with interfaces instead of exposed data is a bad thing.
Data is not an interface. IMO if you can get by with keeping the types private, go for it. Unless I'm missing something, you only need the parent dev pointer. Maybe add a helper function for it?
It's trivial to expose the guts like this, but it's usually a lot of hard work to go the other way. Look at the dependencies of component.h now. To keep it self-contained, i.e. buildable without implicit dependencies, you'd need to add #include <device.h>, which goes on to include the world. Then have a look at [1].
Please at least let's not do this lightly.
BR, Jani.
[1] https://lwn.net/ml/linux-kernel/YdIfz+LMewetSaEB@gmail.com/
Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: "Rafael J. Wysocki" rafael@kernel.org Cc: Rob Clark robdclark@gmail.com Cc: Russell King rmk+kernel@arm.linux.org.uk Cc: Saravana Kannan saravanak@google.com Signed-off-by: Stephen Boyd swboyd@chromium.org
drivers/base/component.c | 15 --------------- include/linux/component.h | 19 ++++++++++++++++--- 2 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/base/component.c b/drivers/base/component.c index eec82caeae5e..dc38a8939ae6 100644 --- a/drivers/base/component.c +++ b/drivers/base/component.c @@ -56,21 +56,6 @@ struct component_match { struct component_match_array *compare; };
-struct aggregate_device {
- const struct component_master_ops *ops;
- struct device *parent;
- struct device dev;
- struct component_match *match;
- struct aggregate_driver *adrv;
- int id;
-};
-static inline struct aggregate_device *to_aggregate_device(struct device *d) -{
- return container_of(d, struct aggregate_device, dev);
-}
struct component { struct list_head node; struct aggregate_device *adev; diff --git a/include/linux/component.h b/include/linux/component.h index 95d1b23ede8a..e99cf8e910f0 100644 --- a/include/linux/component.h +++ b/include/linux/component.h @@ -5,6 +5,8 @@ #include <linux/stddef.h> #include <linux/device.h>
+struct component_match;
/**
- struct component_ops - callbacks for component drivers
@@ -39,8 +41,6 @@ void component_del(struct device *, const struct component_ops *); int component_bind_all(struct device *master, void *master_data); void component_unbind_all(struct device *master, void *master_data);
-struct aggregate_device;
/**
- struct component_master_ops - callback for the aggregate driver
@@ -80,7 +80,20 @@ struct component_master_ops { void (*unbind)(struct device *master); };
-struct component_match; +struct aggregate_device {
- const struct component_master_ops *ops;
- struct device *parent;
- struct device dev;
- struct component_match *match;
- struct aggregate_driver *adrv;
- int id;
+};
+static inline struct aggregate_device *to_aggregate_device(struct device *d) +{
- return container_of(d, struct aggregate_device, dev);
+}
/**
- struct aggregate_driver - Aggregate driver (made up of other drivers)