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

[Suzuki K Poulose]

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
> >    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.".

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

Cheers
Suzuki