Thread (24 messages) 24 messages, 6 authors, 2024-12-05

Re: [PATCH v2 00/32] driver core: Constify API device_find_child() and adapt for various existing usages

From: James Bottomley <James.Bottomley@HansenPartnership.com>
Date: 2024-12-04 16:42:44
Also in: arm-scmi, dri-devel, linux-block, linux-cxl, linux-efi, linux-gpio, linux-hwmon, linux-media, linux-mediatek, linux-pwm, linux-remoteproc, linux-scsi, linux-serial, linux-sound, linux-usb, lkml, nvdimm, sparclinux

On Wed, 2024-12-04 at 20:26 +0800, Zijun Hu wrote:
On 2024/12/3 23:34, James Bottomley wrote:
quoted
quoted
quoted
This also enables an incremental migration.
change the API prototype from:
device_find_child(..., void *data_0, int (*match)(struct device
*dev, void *data));

to:
device_find_child(..., const void *data_0, int (*match)(struct
device *dev, const void *data));

For @data_0,  void * -> const void * is okay.
but for @match, the problem is function pointer type
incompatibility.

there are two solutions base on discussions.

1) squashing likewise Greg mentioned.
   Do all of the "prep work" first, and then
   do the const change at the very end, all at once.

2)  as changing platform_driver's remove() prototype.
Commit: e70140ba0d2b ("Get rid of 'remove_new' relic from
platform driver struct")

 introduce extra device_find_child_new() which is constified  ->
use *_new() replace ALL device_find_child() instances one by one
-> remove device_find_child() -> rename *_new() to
device_find_child() once.
Why bother with the last step, which churns the entire code base
again?
keep the good API name device_find_child().
Well, I think it's a good opportunity to rename the API better, but if
that's the goal, you can still do it with _Generic() without churning
the code base a second time.  The example is in
slab.h:kmem_cache_create
quoted
Why not call the new function device_find_child_const() and simply
keep it (it's descriptive of its function).  That way you can have
a patch series without merging and at the end simply remove the old
function.
device_find_child is a good name for the API, 'find' already means
const.
Not to me it doesn't, but that's actually not what I think is wrong
with the API name: it actually only returns the first match, so I'd
marginally prefer it to be called device_find_first_child() ... not
enough to churn the code to change it, but since you're doing that
anyway it might make sense as an update.

Regards,

James
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help