| Message ID | 20211202222732.2453851-2-swboyd@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | component: Make into an aggregate bus | expand |
On Thu, Dec 02, 2021 at 02:26:59PM -0800, Stephen Boyd wrote: > Replace 'struct master' with 'struct aggregate_device' and then rename > 'master' to 'adev' everywhere in the code. While we're here, put a > struct device inside the aggregate device so that we can register it > with a bus_type in the next patch. Do not do a "while we are here" type change please. Do the rename/replace first, and then make the other change. > The diff is large but that's because this is mostly a rename, where > sometimes 'master' is replaced with 'adev' and other times it is > replaced with 'parent' to indicate that the struct device that was being > used is actually the parent of the aggregate device and driver. The diff is 15 lines, how is that "large"? thanks, greg k-h
On Thu, Dec 02, 2021 at 02:26:59PM -0800, Stephen Boyd wrote: > Replace 'struct master' with 'struct aggregate_device' and then rename > 'master' to 'adev' everywhere in the code. While we're here, put a > struct device inside the aggregate device so that we can register it > with a bus_type in the next patch. > > The diff is large but that's because this is mostly a rename, where > sometimes 'master' is replaced with 'adev' and other times it is > replaced with 'parent' to indicate that the struct device that was being > used is actually the parent of the aggregate device and driver. > > 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 | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/base/component.c b/drivers/base/component.c > index 2d25a6416587..d25048d04b70 100644 > --- a/drivers/base/component.c > +++ b/drivers/base/component.c > @@ -9,6 +9,7 @@ > */ > #include <linux/component.h> > #include <linux/device.h> > +#include <linux/idr.h> > #include <linux/list.h> > #include <linux/mutex.h> > #include <linux/slab.h> > @@ -63,7 +64,10 @@ struct master { > > const struct component_master_ops *ops; > struct device *parent; > + struct device dev; Who initializes this new structure? > struct component_match *match; > + > + int id; > }; > > struct component { > @@ -79,6 +83,7 @@ struct component { > static DEFINE_MUTEX(component_mutex); > static LIST_HEAD(component_list); > static LIST_HEAD(masters); > +static DEFINE_IDA(aggregate_ida); > > #ifdef CONFIG_DEBUG_FS > > @@ -440,6 +445,7 @@ static void free_master(struct master *master) > } > } > > + ida_free(&aggregate_ida, master->id); > kfree(master); > } > > @@ -460,7 +466,7 @@ int component_master_add_with_match(struct device *parent, > struct component_match *match) > { > struct master *master; > - int ret; > + int ret, id; > > /* Reallocate the match array for its true size */ > ret = component_match_realloc(match, match->num); > @@ -471,9 +477,17 @@ int component_master_add_with_match(struct device *parent, > if (!master) > return -ENOMEM; > > + id = ida_alloc(&aggregate_ida, GFP_KERNEL); > + if (id < 0) { > + kfree(master); > + return id; > + } > + > + master->id = id; > master->parent = parent; > master->ops = ops; > master->match = match; > + dev_set_name(&master->dev, "aggregate%d", id); You set the name yet the device is not "real"? I don't understand this patch at all, sorry. greg k-h
Quoting Greg Kroah-Hartman (2021-12-21 01:28:10) > On Thu, Dec 02, 2021 at 02:26:59PM -0800, Stephen Boyd wrote: > > Replace 'struct master' with 'struct aggregate_device' and then rename > > 'master' to 'adev' everywhere in the code. While we're here, put a > > struct device inside the aggregate device so that we can register it > > with a bus_type in the next patch. > > Do not do a "while we are here" type change please. Do the > rename/replace first, and then make the other change. > > > The diff is large but that's because this is mostly a rename, where > > sometimes 'master' is replaced with 'adev' and other times it is > > replaced with 'parent' to indicate that the struct device that was being > > used is actually the parent of the aggregate device and driver. > > The diff is 15 lines, how is that "large"? > Looks like I forgot to remove this chunk of commit text while splitting the change earlier. Will fix it up now.
Quoting Greg Kroah-Hartman (2021-12-21 01:29:59) > > @@ -471,9 +477,17 @@ int component_master_add_with_match(struct device *parent, > > if (!master) > > return -ENOMEM; > > > > + id = ida_alloc(&aggregate_ida, GFP_KERNEL); > > + if (id < 0) { > > + kfree(master); > > + return id; > > + } > > + > > + master->id = id; > > master->parent = parent; > > master->ops = ops; > > master->match = match; > > + dev_set_name(&master->dev, "aggregate%d", id); > > You set the name yet the device is not "real"? > > I don't understand this patch at all, sorry. The device is registered in the third patch. I'll squash this patch with that one and move the rename patch to be the first patch in the series. Then it should be clear why a struct device is introduced along with the bus type it's associated with.
diff --git a/drivers/base/component.c b/drivers/base/component.c index 2d25a6416587..d25048d04b70 100644 --- a/drivers/base/component.c +++ b/drivers/base/component.c @@ -9,6 +9,7 @@ */ #include <linux/component.h> #include <linux/device.h> +#include <linux/idr.h> #include <linux/list.h> #include <linux/mutex.h> #include <linux/slab.h> @@ -63,7 +64,10 @@ struct master { const struct component_master_ops *ops; struct device *parent; + struct device dev; struct component_match *match; + + int id; }; struct component { @@ -79,6 +83,7 @@ struct component { static DEFINE_MUTEX(component_mutex); static LIST_HEAD(component_list); static LIST_HEAD(masters); +static DEFINE_IDA(aggregate_ida); #ifdef CONFIG_DEBUG_FS @@ -440,6 +445,7 @@ static void free_master(struct master *master) } } + ida_free(&aggregate_ida, master->id); kfree(master); } @@ -460,7 +466,7 @@ int component_master_add_with_match(struct device *parent, struct component_match *match) { struct master *master; - int ret; + int ret, id; /* Reallocate the match array for its true size */ ret = component_match_realloc(match, match->num); @@ -471,9 +477,17 @@ int component_master_add_with_match(struct device *parent, if (!master) return -ENOMEM; + id = ida_alloc(&aggregate_ida, GFP_KERNEL); + if (id < 0) { + kfree(master); + return id; + } + + master->id = id; master->parent = parent; master->ops = ops; master->match = match; + dev_set_name(&master->dev, "aggregate%d", id); component_master_debugfs_add(master); /* Add to the list of available masters. */
Replace 'struct master' with 'struct aggregate_device' and then rename 'master' to 'adev' everywhere in the code. While we're here, put a struct device inside the aggregate device so that we can register it with a bus_type in the next patch. The diff is large but that's because this is mostly a rename, where sometimes 'master' is replaced with 'adev' and other times it is replaced with 'parent' to indicate that the struct device that was being used is actually the parent of the aggregate device and driver. 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 | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)