Re: [PATCH v2 1/2] driver core: bus.h: document bus notifiers better

[Randy Dunlap]

Hi Greg,

On 1/10/23 06:53, Greg Kroah-Hartman wrote:
> The bus notifier values are not documented all that well, so clean this
> up and make a real enumerated type for them and document them much
> better.  Also change the values from being in hex to just decimal as it
> didn't make any sense to have them in hex.
> 
> Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
> v2: move the values to decimal from hex as pointed out by Rafael.
> 
>  include/linux/device/bus.h | 43 +++++++++++++++++++++++++-------------
>  1 file changed, 29 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
> index d529f644e92b..fbec1c7c34c0 100644
> --- a/include/linux/device/bus.h
> +++ b/include/linux/device/bus.h
> @@ -257,21 +257,36 @@ extern int bus_register_notifier(struct bus_type *bus,
>  extern int bus_unregister_notifier(struct bus_type *bus,
>  				   struct notifier_block *nb);
>  
> -/* All 4 notifers below get called with the target struct device *
> - * as an argument. Note that those functions are likely to be called
> - * with the device lock held in the core, so be careful.

If you want this to be kernel-doc format with no warnings,
(a) all of the " * BUS_NOTIFY_..." lines should be " * @BUS_NOTIFY_...";
(b) all of the " * @BUS_NOTIFY_..." lines should be immediately after the
second ("enum") line. (at [1])
(c) In the heading "enum" line, s/: / - /, but that's just for consistency
and to follow kernel-doc documented format; it seems that kernel-doc takes
that separator either way.

The patch below (on top of this one) makes all of these changes.

> +/**
> + * enum bus_notifier_event: Bus Notifier events that have happened
[1]
> + *
> + * These are the value passed to a bus notifier when a specific event happens.
> + *
> + * Note that bus notifiers are likely to be called with the device lock already
> + * held by the driver core, so be careful in any notifier callback as to what
> + * you do with the device structure.
> + *
> + * All bus notifiers are called with the target struct device * as an argument.
> + *
> + * BUS_NOTIFY_ADD_DEVICE: device is added to this bus
> + * BUS_NOTIFY_DEL_DEVICE: device is about to be removed from this bus
> + * BUS_NOTIFY_REMOVED_DEVICE: device is successfully removed from this bus
> + * BUS_NOTIFY_BIND_DRIVER: a driver is about to be bound to this device on this bus
> + * BUS_NOTIFY_BOUND_DRIVER: a driver is successfully bound to this device on this bus
> + * BUS_NOTIFY_UNBIND_DRIVER: a driver is about to be unbound from this device on this bus
> + * BUS_NOTIFY_UNBOUND_DRIVER: a driver is successfully unbound from this device on this bus
> + * BUS_NOTIFY_DRIVER_NOT_BOUND: a driver failed to be bound to this device on this bus
>   */
> -#define BUS_NOTIFY_ADD_DEVICE		0x00000001 /* device added */
> -#define BUS_NOTIFY_DEL_DEVICE		0x00000002 /* device to be removed */
> -#define BUS_NOTIFY_REMOVED_DEVICE	0x00000003 /* device removed */
> -#define BUS_NOTIFY_BIND_DRIVER		0x00000004 /* driver about to be
> -						      bound */
> -#define BUS_NOTIFY_BOUND_DRIVER		0x00000005 /* driver bound to device */
> -#define BUS_NOTIFY_UNBIND_DRIVER	0x00000006 /* driver about to be
> -						      unbound */
> -#define BUS_NOTIFY_UNBOUND_DRIVER	0x00000007 /* driver is unbound
> -						      from the device */
> -#define BUS_NOTIFY_DRIVER_NOT_BOUND	0x00000008 /* driver fails to be bound */
> +enum bus_notifier_event {
> +	BUS_NOTIFY_ADD_DEVICE =		1,
> +	BUS_NOTIFY_DEL_DEVICE =		2,
> +	BUS_NOTIFY_REMOVED_DEVICE =	3,
> +	BUS_NOTIFY_BIND_DRIVER =	4,
> +	BUS_NOTIFY_BOUND_DRIVER =	5,
> +	BUS_NOTIFY_UNBIND_DRIVER =	6,
> +	BUS_NOTIFY_UNBOUND_DRIVER =	7,
> +	BUS_NOTIFY_DRIVER_NOT_BOUND =	8,
> +};
>  
>  extern struct kset *bus_get_kset(struct bus_type *bus);
>  

Signed-off-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
---
 include/linux/device/bus.h |   19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff -- a/include/linux/device/bus.h b/include/linux/device/bus.h
--- a/include/linux/device/bus.h
+++ b/include/linux/device/bus.h
@@ -271,7 +271,15 @@ extern int bus_unregister_notifier(struc
 				   struct notifier_block *nb);
 
 /**
- * enum bus_notifier_event: Bus Notifier events that have happened
+ * enum bus_notifier_event - Bus Notifier events that have happened
+ * @BUS_NOTIFY_ADD_DEVICE: device is added to this bus
+ * @BUS_NOTIFY_DEL_DEVICE: device is about to be removed from this bus
+ * @BUS_NOTIFY_REMOVED_DEVICE: device is successfully removed from this bus
+ * @BUS_NOTIFY_BIND_DRIVER: a driver is about to be bound to this device on this bus
+ * @BUS_NOTIFY_BOUND_DRIVER: a driver is successfully bound to this device on this bus
+ * @BUS_NOTIFY_UNBIND_DRIVER: a driver is about to be unbound from this device on this bus
+ * @BUS_NOTIFY_UNBOUND_DRIVER: a driver is successfully unbound from this device on this bus
+ * @BUS_NOTIFY_DRIVER_NOT_BOUND: a driver failed to be bound to this device on this bus
  *
  * These are the value passed to a bus notifier when a specific event happens.
  *
@@ -280,15 +288,6 @@ extern int bus_unregister_notifier(struc
  * you do with the device structure.
  *
  * All bus notifiers are called with the target struct device * as an argument.
- *
- * BUS_NOTIFY_ADD_DEVICE: device is added to this bus
- * BUS_NOTIFY_DEL_DEVICE: device is about to be removed from this bus
- * BUS_NOTIFY_REMOVED_DEVICE: device is successfully removed from this bus
- * BUS_NOTIFY_BIND_DRIVER: a driver is about to be bound to this device on this bus
- * BUS_NOTIFY_BOUND_DRIVER: a driver is successfully bound to this device on this bus
- * BUS_NOTIFY_UNBIND_DRIVER: a driver is about to be unbound from this device on this bus
- * BUS_NOTIFY_UNBOUND_DRIVER: a driver is successfully unbound from this device on this bus
- * BUS_NOTIFY_DRIVER_NOT_BOUND: a driver failed to be bound to this device on this bus
  */
 enum bus_notifier_event {
 	BUS_NOTIFY_ADD_DEVICE =		1,


-- 
~Randy