Re: [PATCH 1/3] driver: clean up comment formatting and spacing

[Bo Sun <bo@mboxify.com>]

On 2025-07-11 20:39, Ahmad Fatoum wrote:
> Hello Bo,
> 
> On 7/11/25 14:27, Bo Sun wrote:
>> - Fixed a missing closing parenthesis in the comment for 
>> DEVICE_ID_SINGLE
>> - Cleaned up indentation and alignment of multi-line comments in 
>> struct device
>> - Removed redundant spaces in function pointer declarations
>> 
>> No functional changes.
>> 
>> Signed-off-by: Bo Sun <bo@mboxify.com>
> 
> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
>> ---
>>  include/device.h | 18 ++++++++++--------
>>  include/driver.h |  8 ++++----
>>  2 files changed, 14 insertions(+), 12 deletions(-)
>> 
>> diff --git a/include/device.h b/include/device.h
>> index bc3a348e2e..97fa040365 100644
>> --- a/include/device.h
>> +++ b/include/device.h
>> @@ -37,7 +37,7 @@ struct device {
>>  	/*! This member is used to store device's unique name as
>>  	 *  obtained by calling dev_id(). Internal field, do not
>>  	 *  access it directly.
>> -	  */
>> +	 */
> 
> Would you like to use the occasion to switch this over to kerneldoc
> style? Kerneldoc is much more common in newer barebox code and would
> allow us using the same infrastructure in future.

Sure, will do.

Thanks,
Bo

> 
> Cheers,
> Ahmad
> 
>>  	char *unique_name;
>>  	/*! The id is used to uniquely identify a device in the system. The 
>> id
>>  	 * will show up under /dev/ as the device's name. Usually this is
>> @@ -58,14 +58,16 @@ struct device {
>>  		void *priv;
>>  		void *driver_data;
>>  	};
>> -	void *type_data;     /*! In case this device is a specific device, 
>> this pointer
>> -			      * points to the type specific device, i.e. eth_device
>> -			      */
>> +	/*! In case this device is a specific device, this pointer
>> +	 * points to the type specific device, i.e. eth_device
>> +	 */
>> +	void *type_data;
>> +
>>  	struct driver *driver; /*! The driver for this device */
>> 
>>  	struct list_head list;     /* The list of all devices */
>> -	struct list_head bus_list; /* our bus            */
>> -	struct list_head children; /* our children            */
>> +	struct list_head bus_list; /* our bus */
>> +	struct list_head children; /* our children */
>>  	struct list_head sibling;
>>  	struct list_head active;   /* The list of all devices which have a 
>> driver */
>> 
>> @@ -102,8 +104,8 @@ struct device {
>>  	 * For devices which take longer to probe this is called
>>  	 * when the driver should actually detect client devices
>>  	 */
>> -	int     (*detect) (struct device *);
>> -	void	(*rescan) (struct device *);
>> +	int (*detect)(struct device *);
>> +	void (*rescan)(struct device *);
>> 
>>  	/*
>>  	 * if a driver probe is deferred, this stores the last error
>> diff --git a/include/driver.h b/include/driver.h
>> index dd50a7aa3c..510a32186d 100644
>> --- a/include/driver.h
>> +++ b/include/driver.h
>> @@ -36,13 +36,13 @@ struct driver {
>>  	const char *name;
>> 
>>  	struct list_head list;
>> -	struct list_head bus_list; /* our bus            */
>> +	struct list_head bus_list; /* our bus */
>> 
>>  	/*! Called if an instance of a device is found */
>> -	int     (*probe) (struct device *);
>> +	int (*probe)(struct device *);
>> 
>>  	/*! Called if an instance of a device is gone. */
>> -	void     (*remove)(struct device *);
>> +	void (*remove)(struct device *);
>> 
>>  	struct bus_type *bus;
>> 
>> @@ -61,7 +61,7 @@ struct driver {
>> 
>>  /* dynamically assign the next free id */
>>  #define DEVICE_ID_DYNAMIC	-2
>> -/* do not use an id (only one device available */
>> +/* do not use an id (only one device available) */
>>  #define DEVICE_ID_SINGLE	-1
>> 
>>  /* Register devices and drivers.
>> 
>>