Re: [PATCH 1/2] staging: most: dim2: do not double-register the same device

[Greg Kroah-Hartman]

On Tue, Oct 05, 2021 at 05:34:48PM +0300, Nikita Yushchenko wrote:
> Commit 723de0f9171e ("staging: most: remove device from interface
> structure") moved registration of driver-provided struct device to
> the most subsystem.
> 
> Dim2 used to register the same struct device to provide a custom device
> attribute. This causes double-registration of the same struct device.
> 
> Fix that by moving the custom attribute to driver's dev_groups.
> This moves attribute to the platform_device object, which is a better
> location for platform-specific attributes anyway.
> 
> Fixes: 723de0f9171e ("staging: most: remove device from interface structure")
> Signed-off-by: Nikita Yushchenko <nikita.yoush@xxxxxxxxxxxxxxxxxx>
> ---
>  drivers/staging/most/dim2/Makefile |  2 +-
>  drivers/staging/most/dim2/dim2.c   | 31 ++++++++++++-------
>  drivers/staging/most/dim2/sysfs.c  | 49 ------------------------------
>  drivers/staging/most/dim2/sysfs.h  | 11 -------
>  4 files changed, 21 insertions(+), 72 deletions(-)
>  delete mode 100644 drivers/staging/most/dim2/sysfs.c
> 
> diff --git a/drivers/staging/most/dim2/Makefile b/drivers/staging/most/dim2/Makefile
> index 861adacf6c72..5f9612af3fa3 100644
> --- a/drivers/staging/most/dim2/Makefile
> +++ b/drivers/staging/most/dim2/Makefile
> @@ -1,4 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_MOST_DIM2) += most_dim2.o
>  
> -most_dim2-objs := dim2.o hal.o sysfs.o
> +most_dim2-objs := dim2.o hal.o
> diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
> index e8b03fa90e80..bb6dd508e531 100644
> --- a/drivers/staging/most/dim2/dim2.c
> +++ b/drivers/staging/most/dim2/dim2.c
> @@ -118,7 +118,8 @@ struct dim2_platform_data {
>  	(((p)[1] == 0x18) && ((p)[2] == 0x05) && ((p)[3] == 0x0C) && \
>  	 ((p)[13] == 0x3C) && ((p)[14] == 0x00) && ((p)[15] == 0x0A))
>  
> -bool dim2_sysfs_get_state_cb(void)
> +static ssize_t state_show(struct device *dev, struct device_attribute *attr,
> +			  char *buf)
>  {
>  	bool state;
>  	unsigned long flags;
> @@ -127,9 +128,25 @@ bool dim2_sysfs_get_state_cb(void)
>  	state = dim_get_lock_state();
>  	spin_unlock_irqrestore(&dim_lock, flags);
>  
> -	return state;
> +	return sprintf(buf, "%s\n", state ? "locked" : "");

sysfs_emit()?

>  }
>  
> +static DEVICE_ATTR_RO(state);
> +
> +static struct attribute *dim2_dev_attrs[] = {
> +	&dev_attr_state.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group dim2_attr_group = {
> +	.attrs = dim2_dev_attrs,
> +};
> +
> +static const struct attribute_group *dim2_attr_groups[] = {
> +	&dim2_attr_group,
> +	NULL,
> +};

ATTRIBUTE_GROUPS()?

Other than these minor things, looks good, thanks for doing this!

greg k-h