Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Tue, 4 Jun 2019 13:26:37 +0200 | Subject | Re: [RFC PATCH 27/57] drivers: Unify the match prototype for bus_find_device with class_find_device |
| |
On Mon, Jun 3, 2019 at 5:51 PM Suzuki K Poulose <suzuki.poulose@arm.com> wrote: > > We have iterators for devices by bus and class, with a supplied > "match" function to do the comparison. However, both of the helper > function have slightly different prototype for the "match" argument. > > int (*) (struct device *dev, void *data) // bus_find_device > vs > int (*) (struct device *dev, const void *data) // class_find_device > > Unify the prototype by promoting the match function to use that of > the class_find_device(). This will allow us to share the generic > match helpers with class_find_device() users.
The patch looks good to me, but the changelog might be a bit better.
It seems to be all about the bus_find_device() and class_find_device() prototype consolidation, so that the same pair of data and match() arguments can be passed to both of them, which then will allow some optimizations to be made, so what about the following:
"There is an arbitrary difference between the prototypes of bus_find_device() and class_find_device() preventing their callers from passing the same pair of data and match() arguments to both of them, which is the const qualifier used in the prototype of class_find_device(). If that qualifier is also used in the bus_find_device() prototype, it will be possible to pass the same match() callback function to both bus_find_device() and class_find_device(), which will allow some optimizations to be made in order to avoid code duplication going forward.
For this reason, change the prototype of bus_find_device() to match the prototype of class_find_device() and adjust its callers to use the const qualifier in accordance with the new prototype of it.".
Also, it looks like there is no need to make all of the following changes in the series along with this one in one go and making them separately would be *much* better from the patch review perspective.
| |