1. Patchew
  2. linux
  3. View series

[PATCH v2] drivers/base: Introduce device_match_t for device finding APIs

Zijun Hu posted 1 patch 1 year, 6 months ago
There is a newer version of this series
drivers/base/auxiliary.c      | 2 +-
drivers/base/bus.c            | 2 +-
drivers/base/class.c          | 3 +--
drivers/base/driver.c         | 2 +-
include/linux/auxiliary_bus.h | 2 +-
include/linux/device/bus.h    | 5 +++--
include/linux/device/class.h  | 2 +-
include/linux/device/driver.h | 2 +-
8 files changed, 10 insertions(+), 10 deletions(-)
[PATCH v2] drivers/base: Introduce device_match_t for device finding APIs
Posted by Zijun Hu 1 year, 6 months ago 
From: Zijun Hu <quic_zijuhu@quicinc.com>

There are several drivers/base APIs for finding a specific device, and
they currently use the following good type for the @match parameter:
int (*match)(struct device *dev, const void *data)

Since these operations do not modify the caller-provided @*data, this
type is worthy of a dedicated typedef:
typedef int (*device_match_t)(struct device *dev, const void *data)

Advantages of using device_match_t:
 - Shorter API declarations and definitions
 - Prevent further APIs from using a bad type for @match

So introduce device_match_t and apply it to the existing
(bus|class|driver|auxiliary)_find_device() APIs.

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
Changes in v2:
- Git rebase over the following commit:
  bfa54a793ba7 ("driver core: bus: Fix double free in driver API bus_register()")
- Link to v1: https://lore.kernel.org/r/20240802-dev_match_api-v1-1-51e16d3bf031@quicinc.com
---
 drivers/base/auxiliary.c      | 2 +-
 drivers/base/bus.c            | 2 +-
 drivers/base/class.c          | 3 +--
 drivers/base/driver.c         | 2 +-
 include/linux/auxiliary_bus.h | 2 +-
 include/linux/device/bus.h    | 5 +++--
 include/linux/device/class.h  | 2 +-
 include/linux/device/driver.h | 2 +-
 8 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
index 54b92839e05c..7823888af4f6 100644
--- a/drivers/base/auxiliary.c
+++ b/drivers/base/auxiliary.c
@@ -352,7 +352,7 @@ EXPORT_SYMBOL_GPL(__auxiliary_device_add);
  */
 struct auxiliary_device *auxiliary_find_device(struct device *start,
 					       const void *data,
-					       int (*match)(struct device *dev, const void *data))
+					       device_match_t match)
 {
 	struct device *dev;
 
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index abf090ace833..657c93c38b0d 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -391,7 +391,7 @@ EXPORT_SYMBOL_GPL(bus_for_each_dev);
  */
 struct device *bus_find_device(const struct bus_type *bus,
 			       struct device *start, const void *data,
-			       int (*match)(struct device *dev, const void *data))
+			       device_match_t match)
 {
 	struct subsys_private *sp = bus_to_subsys(bus);
 	struct klist_iter i;
diff --git a/drivers/base/class.c b/drivers/base/class.c
index 7b38fdf8e1d7..ae22fa992c04 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -433,8 +433,7 @@ EXPORT_SYMBOL_GPL(class_for_each_device);
  * code.  There's no locking restriction.
  */
 struct device *class_find_device(const struct class *class, const struct device *start,
-				 const void *data,
-				 int (*match)(struct device *, const void *))
+				 const void *data, device_match_t match)
 {
 	struct subsys_private *sp = class_to_subsys(class);
 	struct class_dev_iter iter;
diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 88c6fd1f1992..b4eb5b89c4ee 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -150,7 +150,7 @@ EXPORT_SYMBOL_GPL(driver_for_each_device);
  */
 struct device *driver_find_device(const struct device_driver *drv,
 				  struct device *start, const void *data,
-				  int (*match)(struct device *dev, const void *data))
+				  device_match_t match)
 {
 	struct klist_iter i;
 	struct device *dev;
diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
index 662b8ae54b6a..31762324bcc9 100644
--- a/include/linux/auxiliary_bus.h
+++ b/include/linux/auxiliary_bus.h
@@ -271,6 +271,6 @@ void auxiliary_driver_unregister(struct auxiliary_driver *auxdrv);
 
 struct auxiliary_device *auxiliary_find_device(struct device *start,
 					       const void *data,
-					       int (*match)(struct device *dev, const void *data));
+					       device_match_t match);
 
 #endif /* _AUXILIARY_BUS_H_ */
diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
index 807831d6bf0f..970de5ee5562 100644
--- a/include/linux/device/bus.h
+++ b/include/linux/device/bus.h
@@ -126,6 +126,8 @@ struct bus_attribute {
 int __must_check bus_create_file(const struct bus_type *bus, struct bus_attribute *attr);
 void bus_remove_file(const struct bus_type *bus, struct bus_attribute *attr);
 
+/* Matching function type for drivers/base APIs to find a specific device */
+typedef int (*device_match_t)(struct device *dev, const void *data);
 /* Generic device matching functions that all busses can use to match with */
 int device_match_name(struct device *dev, const void *name);
 int device_match_of_node(struct device *dev, const void *np);
@@ -139,8 +141,7 @@ int device_match_any(struct device *dev, const void *unused);
 int bus_for_each_dev(const struct bus_type *bus, struct device *start, void *data,
 		     int (*fn)(struct device *dev, void *data));
 struct device *bus_find_device(const struct bus_type *bus, struct device *start,
-			       const void *data,
-			       int (*match)(struct device *dev, const void *data));
+			       const void *data, device_match_t match);
 /**
  * bus_find_device_by_name - device iterator for locating a particular device
  * of a specific name.
diff --git a/include/linux/device/class.h b/include/linux/device/class.h
index c576b49c55c2..518c9c83d64b 100644
--- a/include/linux/device/class.h
+++ b/include/linux/device/class.h
@@ -95,7 +95,7 @@ void class_dev_iter_exit(struct class_dev_iter *iter);
 int class_for_each_device(const struct class *class, const struct device *start, void *data,
 			  int (*fn)(struct device *dev, void *data));
 struct device *class_find_device(const struct class *class, const struct device *start,
-				 const void *data, int (*match)(struct device *, const void *));
+				 const void *data, device_match_t match);
 
 /**
  * class_find_device_by_name - device iterator for locating a particular device
diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
index 1fc8b68786de..5c04b8e3833b 100644
--- a/include/linux/device/driver.h
+++ b/include/linux/device/driver.h
@@ -157,7 +157,7 @@ int __must_check driver_for_each_device(struct device_driver *drv, struct device
 					void *data, int (*fn)(struct device *dev, void *));
 struct device *driver_find_device(const struct device_driver *drv,
 				  struct device *start, const void *data,
-				  int (*match)(struct device *dev, const void *data));
+				  device_match_t match);
 
 /**
  * driver_find_device_by_name - device iterator for locating a particular device

---
base-commit: bfa54a793ba77ef696755b66f3ac4ed00c7d1248
change-id: 20240801-dev_match_api-415a755fcffb

Best regards,
-- 
Zijun Hu <quic_zijuhu@quicinc.com>
Re: [PATCH v2] drivers/base: Introduce device_match_t for device finding APIs
Posted by Greg Kroah-Hartman 1 year, 5 months ago 
On Sun, Aug 11, 2024 at 10:15:16AM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> There are several drivers/base APIs for finding a specific device, and
> they currently use the following good type for the @match parameter:
> int (*match)(struct device *dev, const void *data)
> 
> Since these operations do not modify the caller-provided @*data, this
> type is worthy of a dedicated typedef:
> typedef int (*device_match_t)(struct device *dev, const void *data)
> 
> Advantages of using device_match_t:
>  - Shorter API declarations and definitions
>  - Prevent further APIs from using a bad type for @match
> 
> So introduce device_match_t and apply it to the existing
> (bus|class|driver|auxiliary)_find_device() APIs.
> 
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
> Changes in v2:
> - Git rebase over the following commit:
>   bfa54a793ba7 ("driver core: bus: Fix double free in driver API bus_register()")
> - Link to v1: https://lore.kernel.org/r/20240802-dev_match_api-v1-1-51e16d3bf031@quicinc.com
> ---
>  drivers/base/auxiliary.c      | 2 +-
>  drivers/base/bus.c            | 2 +-
>  drivers/base/class.c          | 3 +--
>  drivers/base/driver.c         | 2 +-
>  include/linux/auxiliary_bus.h | 2 +-
>  include/linux/device/bus.h    | 5 +++--
>  include/linux/device/class.h  | 2 +-
>  include/linux/device/driver.h | 2 +-
>  8 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
> index 54b92839e05c..7823888af4f6 100644
> --- a/drivers/base/auxiliary.c
> +++ b/drivers/base/auxiliary.c
> @@ -352,7 +352,7 @@ EXPORT_SYMBOL_GPL(__auxiliary_device_add);
>   */
>  struct auxiliary_device *auxiliary_find_device(struct device *start,
>  					       const void *data,
> -					       int (*match)(struct device *dev, const void *data))
> +					       device_match_t match)
>  {
>  	struct device *dev;
>  
> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index abf090ace833..657c93c38b0d 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -391,7 +391,7 @@ EXPORT_SYMBOL_GPL(bus_for_each_dev);
>   */
>  struct device *bus_find_device(const struct bus_type *bus,
>  			       struct device *start, const void *data,
> -			       int (*match)(struct device *dev, const void *data))
> +			       device_match_t match)
>  {
>  	struct subsys_private *sp = bus_to_subsys(bus);
>  	struct klist_iter i;
> diff --git a/drivers/base/class.c b/drivers/base/class.c
> index 7b38fdf8e1d7..ae22fa992c04 100644
> --- a/drivers/base/class.c
> +++ b/drivers/base/class.c
> @@ -433,8 +433,7 @@ EXPORT_SYMBOL_GPL(class_for_each_device);
>   * code.  There's no locking restriction.
>   */
>  struct device *class_find_device(const struct class *class, const struct device *start,
> -				 const void *data,
> -				 int (*match)(struct device *, const void *))
> +				 const void *data, device_match_t match)
>  {
>  	struct subsys_private *sp = class_to_subsys(class);
>  	struct class_dev_iter iter;
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index 88c6fd1f1992..b4eb5b89c4ee 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -150,7 +150,7 @@ EXPORT_SYMBOL_GPL(driver_for_each_device);
>   */
>  struct device *driver_find_device(const struct device_driver *drv,
>  				  struct device *start, const void *data,
> -				  int (*match)(struct device *dev, const void *data))
> +				  device_match_t match)
>  {
>  	struct klist_iter i;
>  	struct device *dev;
> diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
> index 662b8ae54b6a..31762324bcc9 100644
> --- a/include/linux/auxiliary_bus.h
> +++ b/include/linux/auxiliary_bus.h
> @@ -271,6 +271,6 @@ void auxiliary_driver_unregister(struct auxiliary_driver *auxdrv);
>  
>  struct auxiliary_device *auxiliary_find_device(struct device *start,
>  					       const void *data,
> -					       int (*match)(struct device *dev, const void *data));
> +					       device_match_t match);
>  
>  #endif /* _AUXILIARY_BUS_H_ */
> diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
> index 807831d6bf0f..970de5ee5562 100644
> --- a/include/linux/device/bus.h
> +++ b/include/linux/device/bus.h
> @@ -126,6 +126,8 @@ struct bus_attribute {
>  int __must_check bus_create_file(const struct bus_type *bus, struct bus_attribute *attr);
>  void bus_remove_file(const struct bus_type *bus, struct bus_attribute *attr);
>  
> +/* Matching function type for drivers/base APIs to find a specific device */
> +typedef int (*device_match_t)(struct device *dev, const void *data);
>  /* Generic device matching functions that all busses can use to match with */

Extra new-line after the typedef please.

And shouldn't this be in device.h instead?

thanks,

greg k-h
Re: [PATCH v2] drivers/base: Introduce device_match_t for device finding APIs
Posted by quic_zijuhu 1 year, 5 months ago 
On 8/13/2024 5:52 PM, Greg Kroah-Hartman wrote:
> On Sun, Aug 11, 2024 at 10:15:16AM +0800, Zijun Hu wrote:
>> From: Zijun Hu <quic_zijuhu@quicinc.com>
>>
>> There are several drivers/base APIs for finding a specific device, and
>> they currently use the following good type for the @match parameter:
>> int (*match)(struct device *dev, const void *data)
>>
>> Since these operations do not modify the caller-provided @*data, this
>> type is worthy of a dedicated typedef:
>> typedef int (*device_match_t)(struct device *dev, const void *data)
>>
>> Advantages of using device_match_t:
>>  - Shorter API declarations and definitions
>>  - Prevent further APIs from using a bad type for @match
>>
>> So introduce device_match_t and apply it to the existing
>> (bus|class|driver|auxiliary)_find_device() APIs.
>>
>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
>> ---
>> Changes in v2:
>> - Git rebase over the following commit:
>>   bfa54a793ba7 ("driver core: bus: Fix double free in driver API bus_register()")
>> - Link to v1: https://lore.kernel.org/r/20240802-dev_match_api-v1-1-51e16d3bf031@quicinc.com
>> ---
>>  drivers/base/auxiliary.c      | 2 +-
>>  drivers/base/bus.c            | 2 +-
>>  drivers/base/class.c          | 3 +--
>>  drivers/base/driver.c         | 2 +-
>>  include/linux/auxiliary_bus.h | 2 +-
>>  include/linux/device/bus.h    | 5 +++--
>>  include/linux/device/class.h  | 2 +-
>>  include/linux/device/driver.h | 2 +-
>>  8 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
>> index 54b92839e05c..7823888af4f6 100644
>> --- a/drivers/base/auxiliary.c
>> +++ b/drivers/base/auxiliary.c
>> @@ -352,7 +352,7 @@ EXPORT_SYMBOL_GPL(__auxiliary_device_add);
>>   */
>>  struct auxiliary_device *auxiliary_find_device(struct device *start,
>>  					       const void *data,
>> -					       int (*match)(struct device *dev, const void *data))
>> +					       device_match_t match)
>>  {
>>  	struct device *dev;
>>  
>> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
>> index abf090ace833..657c93c38b0d 100644
>> --- a/drivers/base/bus.c
>> +++ b/drivers/base/bus.c
>> @@ -391,7 +391,7 @@ EXPORT_SYMBOL_GPL(bus_for_each_dev);
>>   */
>>  struct device *bus_find_device(const struct bus_type *bus,
>>  			       struct device *start, const void *data,
>> -			       int (*match)(struct device *dev, const void *data))
>> +			       device_match_t match)
>>  {
>>  	struct subsys_private *sp = bus_to_subsys(bus);
>>  	struct klist_iter i;
>> diff --git a/drivers/base/class.c b/drivers/base/class.c
>> index 7b38fdf8e1d7..ae22fa992c04 100644
>> --- a/drivers/base/class.c
>> +++ b/drivers/base/class.c
>> @@ -433,8 +433,7 @@ EXPORT_SYMBOL_GPL(class_for_each_device);
>>   * code.  There's no locking restriction.
>>   */
>>  struct device *class_find_device(const struct class *class, const struct device *start,
>> -				 const void *data,
>> -				 int (*match)(struct device *, const void *))
>> +				 const void *data, device_match_t match)
>>  {
>>  	struct subsys_private *sp = class_to_subsys(class);
>>  	struct class_dev_iter iter;
>> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
>> index 88c6fd1f1992..b4eb5b89c4ee 100644
>> --- a/drivers/base/driver.c
>> +++ b/drivers/base/driver.c
>> @@ -150,7 +150,7 @@ EXPORT_SYMBOL_GPL(driver_for_each_device);
>>   */
>>  struct device *driver_find_device(const struct device_driver *drv,
>>  				  struct device *start, const void *data,
>> -				  int (*match)(struct device *dev, const void *data))
>> +				  device_match_t match)
>>  {
>>  	struct klist_iter i;
>>  	struct device *dev;
>> diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
>> index 662b8ae54b6a..31762324bcc9 100644
>> --- a/include/linux/auxiliary_bus.h
>> +++ b/include/linux/auxiliary_bus.h
>> @@ -271,6 +271,6 @@ void auxiliary_driver_unregister(struct auxiliary_driver *auxdrv);
>>  
>>  struct auxiliary_device *auxiliary_find_device(struct device *start,
>>  					       const void *data,
>> -					       int (*match)(struct device *dev, const void *data));
>> +					       device_match_t match);
>>  
>>  #endif /* _AUXILIARY_BUS_H_ */
>> diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
>> index 807831d6bf0f..970de5ee5562 100644
>> --- a/include/linux/device/bus.h
>> +++ b/include/linux/device/bus.h
>> @@ -126,6 +126,8 @@ struct bus_attribute {
>>  int __must_check bus_create_file(const struct bus_type *bus, struct bus_attribute *attr);
>>  void bus_remove_file(const struct bus_type *bus, struct bus_attribute *attr);
>>  
>> +/* Matching function type for drivers/base APIs to find a specific device */
>> +typedef int (*device_match_t)(struct device *dev, const void *data);
>>  /* Generic device matching functions that all busses can use to match with */
> 
> Extra new-line after the typedef please.
> 
okay, let me correct it with v2.
> And shouldn't this be in device.h instead?
> 
there is the good location to place with below reasons:

1) place this typedef and various match functions with the type
together, these functions immediately follow the type.

2) this type are also used by bus.c/class.c/driver.c which include bus.h
but don't include device.h

3) device.h have included bus.h

> thanks,
> 
> greg k-h

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.