From: Zijun Hu <quic_zijuhu@quicinc.com>

Constify the following driver API:
struct device *device_find_child(struct device *dev, void *data,
		int (*match)(struct device *dev, void *data));
to
struct device *device_find_child(struct device *dev, const void *data,
                                 device_match_t match);
typedef int (*device_match_t)(struct device *dev, const void *data);
Since it should not modify caller's match data @*data.

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/base/core.c    | 11 +++--------
 include/linux/device.h |  4 ++--
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 3f3ebdb5aa0b..f152e0f8fb03 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4062,8 +4062,8 @@ EXPORT_SYMBOL_GPL(device_for_each_child_reverse);
  *
  * NOTE: you will need to drop the reference with put_device() after use.
  */
-struct device *device_find_child(struct device *parent, void *data,
-				 int (*match)(struct device *dev, void *data))
+struct device *device_find_child(struct device *parent, const void *data,
+				 device_match_t match)
 {
 	struct klist_iter i;
 	struct device *child;
@@ -4108,11 +4108,6 @@ struct device *device_find_child_by_name(struct device *parent,
 }
 EXPORT_SYMBOL_GPL(device_find_child_by_name);
 
-static int match_any(struct device *dev, void *unused)
-{
-	return 1;
-}
-
 /**
  * device_find_any_child - device iterator for locating a child device, if any.
  * @parent: parent struct device
@@ -4124,7 +4119,7 @@ static int match_any(struct device *dev, void *unused)
  */
 struct device *device_find_any_child(struct device *parent)
 {
-	return device_find_child(parent, NULL, match_any);
+	return device_find_child(parent, NULL, device_match_any);
 }
 EXPORT_SYMBOL_GPL(device_find_any_child);
 
diff --git a/include/linux/device.h b/include/linux/device.h
index b2423fca3d45..76f10bdbb4ea 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1073,8 +1073,8 @@ int device_for_each_child(struct device *dev, void *data,
 			  int (*fn)(struct device *dev, void *data));
 int device_for_each_child_reverse(struct device *dev, void *data,
 				  int (*fn)(struct device *dev, void *data));
-struct device *device_find_child(struct device *dev, void *data,
-				 int (*match)(struct device *dev, void *data));
+struct device *device_find_child(struct device *dev, const void *data,
+				 device_match_t match);
 struct device *device_find_child_by_name(struct device *parent,
 					 const char *name);
 struct device *device_find_any_child(struct device *parent);

-- 
2.34.1

On Sun, Aug 11, 2024 at 10:24:52AM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> Constify the following driver API:
> struct device *device_find_child(struct device *dev, void *data,
> 		int (*match)(struct device *dev, void *data));
> to
> struct device *device_find_child(struct device *dev, const void *data,
>                                  device_match_t match);
> typedef int (*device_match_t)(struct device *dev, const void *data);
> Since it should not modify caller's match data @*data.
> 
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
>  drivers/base/core.c    | 11 +++--------
>  include/linux/device.h |  4 ++--
>  2 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 3f3ebdb5aa0b..f152e0f8fb03 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -4062,8 +4062,8 @@ EXPORT_SYMBOL_GPL(device_for_each_child_reverse);
>   *
>   * NOTE: you will need to drop the reference with put_device() after use.
>   */
> -struct device *device_find_child(struct device *parent, void *data,
> -				 int (*match)(struct device *dev, void *data))
> +struct device *device_find_child(struct device *parent, const void *data,
> +				 device_match_t match)
>  {
>  	struct klist_iter i;
>  	struct device *child;
> @@ -4108,11 +4108,6 @@ struct device *device_find_child_by_name(struct device *parent,
>  }
>  EXPORT_SYMBOL_GPL(device_find_child_by_name);
>  
> -static int match_any(struct device *dev, void *unused)
> -{
> -	return 1;
> -}
> -
>  /**
>   * device_find_any_child - device iterator for locating a child device, if any.
>   * @parent: parent struct device
> @@ -4124,7 +4119,7 @@ static int match_any(struct device *dev, void *unused)
>   */
>  struct device *device_find_any_child(struct device *parent)
>  {
> -	return device_find_child(parent, NULL, match_any);
> +	return device_find_child(parent, NULL, device_match_any);
>  }
>  EXPORT_SYMBOL_GPL(device_find_any_child);
>  
> diff --git a/include/linux/device.h b/include/linux/device.h
> index b2423fca3d45..76f10bdbb4ea 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -1073,8 +1073,8 @@ int device_for_each_child(struct device *dev, void *data,
>  			  int (*fn)(struct device *dev, void *data));
>  int device_for_each_child_reverse(struct device *dev, void *data,
>  				  int (*fn)(struct device *dev, void *data));
> -struct device *device_find_child(struct device *dev, void *data,
> -				 int (*match)(struct device *dev, void *data));
> +struct device *device_find_child(struct device *dev, const void *data,
> +				 device_match_t match);
>  struct device *device_find_child_by_name(struct device *parent,
>  					 const char *name);
>  struct device *device_find_any_child(struct device *parent);
> 
> -- 
> 2.34.1
> 

This patch breaks the build:

./include/linux/device.h:1077:6: error: unknown type name 'device_match_t'
 1077 |                                  device_match_t match);
      |                                  ^
1 error generated.
make[2]: *** [scripts/Makefile.build:117: arch/x86/kernel/asm-offsets.s] Error 1
make[1]: *** [/mnt/fast_t2/linux/work/driver-core/Makefile:1193: prepare0] Error 2

How did you test it?

And as you are changing the parameters here, doesn't the build break
also because of that?

thanks,

greg k-h

On 8/13/2024 5:48 PM, Greg Kroah-Hartman wrote:
> On Sun, Aug 11, 2024 at 10:24:52AM +0800, Zijun Hu wrote:
>> From: Zijun Hu <quic_zijuhu@quicinc.com>
>>
>> Constify the following driver API:
>> struct device *device_find_child(struct device *dev, void *data,
>> 		int (*match)(struct device *dev, void *data));
>> to
>> struct device *device_find_child(struct device *dev, const void *data,
>>                                  device_match_t match);
>> typedef int (*device_match_t)(struct device *dev, const void *data);
>> Since it should not modify caller's match data @*data.
>>
>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
>> ---
>>  drivers/base/core.c    | 11 +++--------
>>  include/linux/device.h |  4 ++--
>>  2 files changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index 3f3ebdb5aa0b..f152e0f8fb03 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -4062,8 +4062,8 @@ EXPORT_SYMBOL_GPL(device_for_each_child_reverse);
>>   *
>>   * NOTE: you will need to drop the reference with put_device() after use.
>>   */
>> -struct device *device_find_child(struct device *parent, void *data,
>> -				 int (*match)(struct device *dev, void *data))
>> +struct device *device_find_child(struct device *parent, const void *data,
>> +				 device_match_t match)
>>  {
>>  	struct klist_iter i;
>>  	struct device *child;
>> @@ -4108,11 +4108,6 @@ struct device *device_find_child_by_name(struct device *parent,
>>  }
>>  EXPORT_SYMBOL_GPL(device_find_child_by_name);
>>  
>> -static int match_any(struct device *dev, void *unused)
>> -{
>> -	return 1;
>> -}
>> -
>>  /**
>>   * device_find_any_child - device iterator for locating a child device, if any.
>>   * @parent: parent struct device
>> @@ -4124,7 +4119,7 @@ static int match_any(struct device *dev, void *unused)
>>   */
>>  struct device *device_find_any_child(struct device *parent)
>>  {
>> -	return device_find_child(parent, NULL, match_any);
>> +	return device_find_child(parent, NULL, device_match_any);
>>  }
>>  EXPORT_SYMBOL_GPL(device_find_any_child);
>>  
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index b2423fca3d45..76f10bdbb4ea 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -1073,8 +1073,8 @@ int device_for_each_child(struct device *dev, void *data,
>>  			  int (*fn)(struct device *dev, void *data));
>>  int device_for_each_child_reverse(struct device *dev, void *data,
>>  				  int (*fn)(struct device *dev, void *data));
>> -struct device *device_find_child(struct device *dev, void *data,
>> -				 int (*match)(struct device *dev, void *data));
>> +struct device *device_find_child(struct device *dev, const void *data,
>> +				 device_match_t match);
>>  struct device *device_find_child_by_name(struct device *parent,
>>  					 const char *name);
>>  struct device *device_find_any_child(struct device *parent);
>>
>> -- 
>> 2.34.1
>>
> 
> This patch breaks the build:
> 
> ./include/linux/device.h:1077:6: error: unknown type name 'device_match_t'
>  1077 |                                  device_match_t match);
>       |                                  ^
> 1 error generated.
> make[2]: *** [scripts/Makefile.build:117: arch/x86/kernel/asm-offsets.s] Error 1
> make[1]: *** [/mnt/fast_t2/linux/work/driver-core/Makefile:1193: prepare0] Error 2
> 
> How did you test it?
> 
> And as you are changing the parameters here, doesn't the build break
> also because of that?
> 

it seems these dependency patches listed within [PATCH 00/27] are not
picked up.

let me summarize my recently patch serials

A)
Subject: [PATCH v2] drivers/base: Introduce device_match_t for device
finding APIs
https://lore.kernel.org/all/20240811-dev_match_api-v2-1-dd22ff555a30@quicinc.com/

B)
[PATCH 0/5] driver core: Prevent device_find_child() from modifying
caller's match data
https://lore.kernel.org/all/20240811-const_dfc_prepare-v1-0-d67cc416b3d3@quicinc.com/

C)  // this depends on both A) and B)
Subject: [PATCH 00/27] driver core: Make device_find_child()'s match
function take a const pointer
https://lore.kernel.org/all/20240811-const_dfc_done-v1-0-9d85e3f943cb@quicinc.com/

D)
Subject: [PATCH v2] driver core: Simplify driver API
device_find_child_by_name() implementation
https://lore.kernel.org/all/20240811-simply_api_dfcbn-v2-1-d0398acdc366@quicinc.com/


ALL these patch serials are based on driver-core tree with branch
driver-core-next, ALL of them can be built PASS.

> thanks,
> 
> greg k-h

On Tue, Aug 13, 2024 at 06:39:48PM +0800, quic_zijuhu wrote:
> On 8/13/2024 5:48 PM, Greg Kroah-Hartman wrote:
> > On Sun, Aug 11, 2024 at 10:24:52AM +0800, Zijun Hu wrote:
> >> From: Zijun Hu <quic_zijuhu@quicinc.com>
> >>
> >> Constify the following driver API:
> >> struct device *device_find_child(struct device *dev, void *data,
> >> 		int (*match)(struct device *dev, void *data));
> >> to
> >> struct device *device_find_child(struct device *dev, const void *data,
> >>                                  device_match_t match);
> >> typedef int (*device_match_t)(struct device *dev, const void *data);
> >> Since it should not modify caller's match data @*data.
> >>
> >> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> >> ---
> >>  drivers/base/core.c    | 11 +++--------
> >>  include/linux/device.h |  4 ++--
> >>  2 files changed, 5 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/base/core.c b/drivers/base/core.c
> >> index 3f3ebdb5aa0b..f152e0f8fb03 100644
> >> --- a/drivers/base/core.c
> >> +++ b/drivers/base/core.c
> >> @@ -4062,8 +4062,8 @@ EXPORT_SYMBOL_GPL(device_for_each_child_reverse);
> >>   *
> >>   * NOTE: you will need to drop the reference with put_device() after use.
> >>   */
> >> -struct device *device_find_child(struct device *parent, void *data,
> >> -				 int (*match)(struct device *dev, void *data))
> >> +struct device *device_find_child(struct device *parent, const void *data,
> >> +				 device_match_t match)
> >>  {
> >>  	struct klist_iter i;
> >>  	struct device *child;
> >> @@ -4108,11 +4108,6 @@ struct device *device_find_child_by_name(struct device *parent,
> >>  }
> >>  EXPORT_SYMBOL_GPL(device_find_child_by_name);
> >>  
> >> -static int match_any(struct device *dev, void *unused)
> >> -{
> >> -	return 1;
> >> -}
> >> -
> >>  /**
> >>   * device_find_any_child - device iterator for locating a child device, if any.
> >>   * @parent: parent struct device
> >> @@ -4124,7 +4119,7 @@ static int match_any(struct device *dev, void *unused)
> >>   */
> >>  struct device *device_find_any_child(struct device *parent)
> >>  {
> >> -	return device_find_child(parent, NULL, match_any);
> >> +	return device_find_child(parent, NULL, device_match_any);
> >>  }
> >>  EXPORT_SYMBOL_GPL(device_find_any_child);
> >>  
> >> diff --git a/include/linux/device.h b/include/linux/device.h
> >> index b2423fca3d45..76f10bdbb4ea 100644
> >> --- a/include/linux/device.h
> >> +++ b/include/linux/device.h
> >> @@ -1073,8 +1073,8 @@ int device_for_each_child(struct device *dev, void *data,
> >>  			  int (*fn)(struct device *dev, void *data));
> >>  int device_for_each_child_reverse(struct device *dev, void *data,
> >>  				  int (*fn)(struct device *dev, void *data));
> >> -struct device *device_find_child(struct device *dev, void *data,
> >> -				 int (*match)(struct device *dev, void *data));
> >> +struct device *device_find_child(struct device *dev, const void *data,
> >> +				 device_match_t match);
> >>  struct device *device_find_child_by_name(struct device *parent,
> >>  					 const char *name);
> >>  struct device *device_find_any_child(struct device *parent);
> >>
> >> -- 
> >> 2.34.1
> >>
> > 
> > This patch breaks the build:
> > 
> > ./include/linux/device.h:1077:6: error: unknown type name 'device_match_t'
> >  1077 |                                  device_match_t match);
> >       |                                  ^
> > 1 error generated.
> > make[2]: *** [scripts/Makefile.build:117: arch/x86/kernel/asm-offsets.s] Error 1
> > make[1]: *** [/mnt/fast_t2/linux/work/driver-core/Makefile:1193: prepare0] Error 2
> > 
> > How did you test it?
> > 
> > And as you are changing the parameters here, doesn't the build break
> > also because of that?
> > 
> 
> it seems these dependency patches listed within [PATCH 00/27] are not
> picked up.

Of course not, please send stand-alone series if you need them to be
applied in a specific order.

thanks,

greg k-h