Re: [PATCH v5 1/9] w1_therm: adding code comments and code reordering

From: Randy Dunlap
Date: Sat May 09 2020 - 22:33:06 EST


Hi--

On 5/9/20 4:57 PM, Akira Shimahara wrote:
> Adding code comments to split code in dedicated parts. After the global
> declarations (defines, macros and function declarations), code is organized
> as follow :
> - Device and family dependent structures and functions
> - Interfaces functions
> - Helpers functions
> - Hardware functions
> - Sysfs interface functions
>
> Signed-off-by: Akira Shimahara <akira215corp@xxxxxxxxx>
> ---
> Main motivation on the first patch of this serie is to clean up the code,
> document it and reorder it to prepare the next patches, which are clearer
> after this.
>
> One main point is to keep all device/family dependent code gather at the
> beginning to ease adding new devices if required.
>
> Changes in v5:
> - All patch serie in one .c file
> - Correcting some comments
> - adding <linux/string.h> include
>
> drivers/w1/slaves/w1_therm.c | 403 ++++++++++++++++++++---------------
> 1 file changed, 237 insertions(+), 166 deletions(-)
>
> diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
> index 18f08d7..f7147b2 100644
> --- a/drivers/w1/slaves/w1_therm.c
> +++ b/drivers/w1/slaves/w1_therm.c
> @@ -41,55 +41,99 @@
> static int w1_strong_pullup = 1;
> module_param_named(strong_pullup, w1_strong_pullup, int, 0);
>
> +/*-------------------------------Macros-------------------------------------*/
> +
> +/* return the address of the refcnt in the family data */
> +#define THERM_REFCNT(family_data) \
> + (&((struct w1_therm_family_data *)family_data)->refcnt)
> +
> +/*----------------------------------Structs---------------------------------*/
> +
> +/**
> + * struct w1_therm_family_converter
> + * \brief Used to bind standard function call
> + * to device specific function
> + * it could be routed to NULL if device don't support feature
> + */

Hi,

All of the struct and function documentation comments in all patches in
this patch series should be using kernel-doc notation instead of the above
(whatever it is; I don't know what it is).


> +struct w1_therm_family_converter {
> + u8 broken;
> + u16 reserved;
> + struct w1_family *f;
> + int (*convert)(u8 rom[9]);
> + int (*precision)(struct device *device, int val);
> + int (*eeprom)(struct device *device);
> +};
> +

thanks.
--
~Randy