On Tue, 2024-12-03 at 22:56 +0800, Zijun Hu wrote: > On 2024/12/3 22:07, Thomas Weißschuh wrote: > > On 2024-12-03 08:58:26-0500, James Bottomley wrote: > > > On Tue, 2024-12-03 at 21:02 +0800, Zijun Hu wrote: > > > > On 2024/12/3 20:41, Greg Kroah-Hartman wrote: > > > > > On Tue, Dec 03, 2024 at 08:23:45PM +0800, Zijun Hu wrote: > > > [...] > > > > > > or squash such patch series into a single patch ? > > > > > > > > > > > > various subsystem maintainers may not like squashing way. > > > > > > > > > > Agreed, so look into either doing it in a bisectable way if > > > > > at all possible. As I don't see a full series here, I can't > > > > > suggest how it needs to happen :( > > > > > > > > > > > > > let me send you a full series later and discuss how to solve > > > > this issue. > > > > > > It's only slightly more complex than what we normally do: modify > > > all instances and then change the API. In this case you have an > > > additional problem because the prototype "const void *" will > > > cause a mismatch if a function has "void *". The easiest way to > > > solve this is probably to make device_find_child a macro that > > > coerces its function argument to having a non const "void *" and > > > then passes off to the real function. If you do that in the > > > first patch, then you can constify all the consumers and finally > > > remove the macro coercion in the last patch. > > > > Casting function pointers like that should be detected and trapped > > by control flow integrity checking (KCFI). > > > > Another possibility would be to use a macro and _Generic to > > dispatch to two different backing functions. See __BIN_ATTR() in > > include/linux/sysfs.h for an inspiration. That's way over complicated for this conversion: done properly there should be no need for _Generic() compile time type matching at all. > this way may fix building error issue but does not achieve our > purpose. our purpose is that there are only constified > device_find_child(). > > > > 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? 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. Regards, James