Re: [RFC PATCH 27/57] drivers: Unify the match prototype for bus_find_device with class_find_device

From: Suzuki K Poulose
Date: Tue Jun 04 2019 - 07:43:53 EST

On 04/06/2019 12:26, Rafael J. Wysocki wrote:
On Mon, Jun 3, 2019 at 5:51 PM Suzuki K Poulose <suzuki.poulose@xxxxxxx> 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
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.".

Agreed, I will reword the description.

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.

Sure. I started with the helpers in the hope that, I would need fewer
changes to individual subsystems, once I convert them to use the
new helpers.

i.e, driver A -> use new helper and the change the new helper.

rather than

change all callers of *_find_device() and then all to switch to new helper.

Anyways, looks like the latter is better in terms of splitting the series.
I will rework the series.

Thanks a lot for your input