LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] staging: most: dim2: fix device registration
@ 2021-09-29 20:56 Nikita Yushchenko
  2021-10-05 10:27 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 11+ messages in thread
From: Nikita Yushchenko @ 2021-09-29 20:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Lee Jones
  Cc: linux-staging, linux-kernel, Nikita Yushchenko

Commit 723de0f9171e ("staging: most: remove device from interface
structure") moved registration of driver-provided struct device to
the most subsystem, but did not properly update dim2 driver to
work with that change.

After most subsystem passes driver's dev to register_device(), it
becomes refcounted, and can be only deallocated in the release method.
Provide that by:
- not using devres to allocate the device,
- moving shutdown actions from _remove() to the device release method,
- not calling shutdown actions in _probe() after the device becomes
  refcounted.

Also, driver used to register it's dev itself, to provide a custom
attribute. With the modified most subsystem, this causes duplicate
registration of the same device object. Fix that by adding that custom
attribute to the platform device - that is a better location for
a platform-specific attribute anyway.

Fixes: 723de0f9171e ("staging: most: remove device from interface structure")
Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
---
 drivers/staging/most/dim2/dim2.c  | 60 ++++++++++++++++++-------------
 drivers/staging/most/dim2/sysfs.c |  5 ++-
 2 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index e8b03fa90e80..7ef142b9faef 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -717,6 +717,23 @@ static int get_dim2_clk_speed(const char *clock_speed, u8 *val)
 	return -EINVAL;
 }
 
+static void dim2_release(struct device *d)
+{
+	struct dim2_hdm *dev = container_of(d, struct dim2_hdm, dev);
+	unsigned long flags;
+
+	kthread_stop(dev->netinfo_task);
+
+	spin_lock_irqsave(&dim_lock, flags);
+	dim_shutdown();
+	spin_unlock_irqrestore(&dim_lock, flags);
+
+	if (dev->disable_platform)
+		dev->disable_platform(to_platform_device(d->parent));
+
+	kfree(dev);
+}
+
 /*
  * dim2_probe - dim2 probe handler
  * @pdev: platform device structure
@@ -738,7 +755,7 @@ static int dim2_probe(struct platform_device *pdev)
 
 	enum { MLB_INT_IDX, AHB0_INT_IDX };
 
-	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
 	if (!dev)
 		return -ENOMEM;
 
@@ -750,19 +767,21 @@ static int dim2_probe(struct platform_device *pdev)
 				      "microchip,clock-speed", &clock_speed);
 	if (ret) {
 		dev_err(&pdev->dev, "missing dt property clock-speed\n");
-		return ret;
+		goto err_free_dev;
 	}
 
 	ret = get_dim2_clk_speed(clock_speed, &dev->clk_speed);
 	if (ret) {
 		dev_err(&pdev->dev, "bad dt property clock-speed\n");
-		return ret;
+		goto err_free_dev;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	dev->io_base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(dev->io_base))
-		return PTR_ERR(dev->io_base);
+	if (IS_ERR(dev->io_base)) {
+		ret = PTR_ERR(dev->io_base);
+		goto err_free_dev;
+	}
 
 	of_id = of_match_node(dim2_of_match, pdev->dev.of_node);
 	pdata = of_id->data;
@@ -770,7 +789,7 @@ static int dim2_probe(struct platform_device *pdev)
 		if (pdata->enable) {
 			ret = pdata->enable(pdev);
 			if (ret)
-				return ret;
+				goto err_free_dev;
 		}
 		dev->disable_platform = pdata->disable;
 		if (pdata->fcnt)
@@ -865,32 +884,34 @@ static int dim2_probe(struct platform_device *pdev)
 	dev->most_iface.request_netinfo = request_netinfo;
 	dev->most_iface.driver_dev = &pdev->dev;
 	dev->most_iface.dev = &dev->dev;
-	dev->dev.init_name = "dim2_state";
+	dev->dev.init_name = dev->name;
 	dev->dev.parent = &pdev->dev;
+	dev->dev.release = dim2_release;
 
 	ret = most_register_interface(&dev->most_iface);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register MOST interface\n");
-		goto err_stop_thread;
+		/* cleanup handled by dim2_release() */
+		return ret;
 	}
 
-	ret = dim2_sysfs_probe(&dev->dev);
+	ret = dim2_sysfs_probe(&pdev->dev);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to create sysfs attribute\n");
-		goto err_unreg_iface;
+		most_deregister_interface(&dev->most_iface);
+		/* cleanup handled by dim2_release() */
+		return ret;
 	}
 
 	return 0;
 
-err_unreg_iface:
-	most_deregister_interface(&dev->most_iface);
-err_stop_thread:
-	kthread_stop(dev->netinfo_task);
 err_shutdown_dim:
 	dim_shutdown();
 err_disable_platform:
 	if (dev->disable_platform)
 		dev->disable_platform(pdev);
+err_free_dev:
+	kfree(dev);
 
 	return ret;
 }
@@ -904,18 +925,9 @@ static int dim2_probe(struct platform_device *pdev)
 static int dim2_remove(struct platform_device *pdev)
 {
 	struct dim2_hdm *dev = platform_get_drvdata(pdev);
-	unsigned long flags;
 
-	dim2_sysfs_destroy(&dev->dev);
+	dim2_sysfs_destroy(&pdev->dev);
 	most_deregister_interface(&dev->most_iface);
-	kthread_stop(dev->netinfo_task);
-
-	spin_lock_irqsave(&dim_lock, flags);
-	dim_shutdown();
-	spin_unlock_irqrestore(&dim_lock, flags);
-
-	if (dev->disable_platform)
-		dev->disable_platform(pdev);
 
 	return 0;
 }
diff --git a/drivers/staging/most/dim2/sysfs.c b/drivers/staging/most/dim2/sysfs.c
index c85b2cdcdca3..22836c8ed554 100644
--- a/drivers/staging/most/dim2/sysfs.c
+++ b/drivers/staging/most/dim2/sysfs.c
@@ -39,11 +39,10 @@ static const struct attribute_group *dev_attr_groups[] = {
 
 int dim2_sysfs_probe(struct device *dev)
 {
-	dev->groups = dev_attr_groups;
-	return device_register(dev);
+	return sysfs_create_groups(&dev->kobj, dev_attr_groups);
 }
 
 void dim2_sysfs_destroy(struct device *dev)
 {
-	device_unregister(dev);
+	sysfs_remove_groups(&dev->kobj, dev_attr_groups);
 }
-- 
2.30.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] staging: most: dim2: fix device registration
  2021-09-29 20:56 [PATCH] staging: most: dim2: fix device registration Nikita Yushchenko
@ 2021-10-05 10:27 ` Greg Kroah-Hartman
  2021-10-05 13:33   ` Nikita Yushchenko
                     ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2021-10-05 10:27 UTC (permalink / raw)
  To: Nikita Yushchenko; +Cc: Lee Jones, linux-staging, linux-kernel

On Wed, Sep 29, 2021 at 11:56:20PM +0300, Nikita Yushchenko wrote:
> Commit 723de0f9171e ("staging: most: remove device from interface
> structure") moved registration of driver-provided struct device to
> the most subsystem, but did not properly update dim2 driver to
> work with that change.
> 
> After most subsystem passes driver's dev to register_device(), it
> becomes refcounted, and can be only deallocated in the release method.
> Provide that by:
> - not using devres to allocate the device,
> - moving shutdown actions from _remove() to the device release method,
> - not calling shutdown actions in _probe() after the device becomes
>   refcounted.

Should this be 3 patches?

> Also, driver used to register it's dev itself, to provide a custom
> attribute. With the modified most subsystem, this causes duplicate
> registration of the same device object. Fix that by adding that custom
> attribute to the platform device - that is a better location for
> a platform-specific attribute anyway.

Nope, it should be 4 patches.  Whenever you have to list a bunch of
different things you are doing in a single change, that's a hint that
this should be more than one patch.

Also, why have you not cc:ed the original author of the commit you are
"fixing" here?   They are the maintainer of this code, right?

One note on your change that would keep me from accepting it even if all
of the above was not an issue:

> diff --git a/drivers/staging/most/dim2/sysfs.c b/drivers/staging/most/dim2/sysfs.c
> index c85b2cdcdca3..22836c8ed554 100644
> --- a/drivers/staging/most/dim2/sysfs.c
> +++ b/drivers/staging/most/dim2/sysfs.c
> @@ -39,11 +39,10 @@ static const struct attribute_group *dev_attr_groups[] = {
>  
>  int dim2_sysfs_probe(struct device *dev)
>  {
> -	dev->groups = dev_attr_groups;
> -	return device_register(dev);
> +	return sysfs_create_groups(&dev->kobj, dev_attr_groups);

No driver code should ever be calling a sysfs_* function, which is a
huge hint that this is incorrect.

You also just raced with userspace and lost, please use the default
attributes for the driver or bus for this, but NEVER manually add and
remove sysfs files, that way lies madness and hard to maintain code.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] staging: most: dim2: fix device registration
  2021-10-05 10:27 ` Greg Kroah-Hartman
@ 2021-10-05 13:33   ` Nikita Yushchenko
  2021-10-05 13:49     ` Greg Kroah-Hartman
  2021-10-05 14:17     ` Dan Carpenter
  2021-10-05 14:34   ` [PATCH 1/2] staging: most: dim2: do not double-register the same device Nikita Yushchenko
  2021-10-05 14:34   ` [PATCH 2/2] staging: most: dim2: use device release method Nikita Yushchenko
  2 siblings, 2 replies; 11+ messages in thread
From: Nikita Yushchenko @ 2021-10-05 13:33 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Lee Jones, linux-staging, linux-kernel, Christian Gromm

>> Commit 723de0f9171e ("staging: most: remove device from interface
>> structure") moved registration of driver-provided struct device to
>> the most subsystem, but did not properly update dim2 driver to
>> work with that change.
>>
>> After most subsystem passes driver's dev to register_device(), it
>> becomes refcounted, and can be only deallocated in the release method.
>> Provide that by:
>> - not using devres to allocate the device,
>> - moving shutdown actions from _remove() to the device release method,
>> - not calling shutdown actions in _probe() after the device becomes
>>    refcounted.
> 
> Should this be 3 patches?

But these three items are deeply interconnected, and fix the issue together. Must not manually free 
device structure passed to register_device(), thus must not allocate via devres (because otherwise, 
devres will free it). Once not using devres for it, must deallocate it somehow else, thus must rework 
the release paths.

Perhaps I just shall not go into these details in the commit message.

>> Also, driver used to register it's dev itself, to provide a custom
>> attribute. With the modified most subsystem, this causes duplicate
>> registration of the same device object. Fix that by adding that custom
>> attribute to the platform device - that is a better location for
>> a platform-specific attribute anyway.
> 
> Nope, it should be 4 patches.

Unlike the above three, this item could be separated.
Will split into two patches now - the first for this (and with fix to the attributes issue noted below) 
and the second for proper device releasing.

> Also, why have you not cc:ed the original author of the commit you are
> "fixing" here?   They are the maintainer of this code, right?

I was under impression that "git send-email" does that automatically...

CCing them now.

> One note on your change that would keep me from accepting it even if all
> of the above was not an issue:
> 
>> diff --git a/drivers/staging/most/dim2/sysfs.c b/drivers/staging/most/dim2/sysfs.c
>> index c85b2cdcdca3..22836c8ed554 100644
>> --- a/drivers/staging/most/dim2/sysfs.c
>> +++ b/drivers/staging/most/dim2/sysfs.c
>> @@ -39,11 +39,10 @@ static const struct attribute_group *dev_attr_groups[] = {
>>   
>>   int dim2_sysfs_probe(struct device *dev)
>>   {
>> -	dev->groups = dev_attr_groups;
>> -	return device_register(dev);
>> +	return sysfs_create_groups(&dev->kobj, dev_attr_groups);
> 
> No driver code should ever be calling a sysfs_* function, which is a
> huge hint that this is incorrect.
> 
> You also just raced with userspace and lost, please use the default
> attributes for the driver or bus for this, but NEVER manually add and
> remove sysfs files, that way lies madness and hard to maintain code.
I'm aware of this race, but still creating attributes on device probe is under wide use in the kernel:

nikita@cobook:~/kernel$ grep -r device_create_file drivers | wc -l
448

Still, in case of dim2 driver, moving to driver's dev_groups is trivial. Preparing that patch now.

Nikita


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] staging: most: dim2: fix device registration
  2021-10-05 13:33   ` Nikita Yushchenko
@ 2021-10-05 13:49     ` Greg Kroah-Hartman
  2021-10-05 14:07       ` Greg Kroah-Hartman
  2021-10-05 14:17     ` Dan Carpenter
  1 sibling, 1 reply; 11+ messages in thread
From: Greg Kroah-Hartman @ 2021-10-05 13:49 UTC (permalink / raw)
  To: Nikita Yushchenko; +Cc: Lee Jones, linux-staging, linux-kernel, Christian Gromm

On Tue, Oct 05, 2021 at 04:33:02PM +0300, Nikita Yushchenko wrote:
> > > Commit 723de0f9171e ("staging: most: remove device from interface
> > > structure") moved registration of driver-provided struct device to
> > > the most subsystem, but did not properly update dim2 driver to
> > > work with that change.
> > > 
> > > After most subsystem passes driver's dev to register_device(), it
> > > becomes refcounted, and can be only deallocated in the release method.
> > > Provide that by:
> > > - not using devres to allocate the device,
> > > - moving shutdown actions from _remove() to the device release method,
> > > - not calling shutdown actions in _probe() after the device becomes
> > >    refcounted.
> > 
> > Should this be 3 patches?
> 
> But these three items are deeply interconnected, and fix the issue together.
> Must not manually free device structure passed to register_device(), thus
> must not allocate via devres (because otherwise, devres will free it). Once
> not using devres for it, must deallocate it somehow else, thus must rework
> the release paths.

Ok, but that was obvious.

> > > Also, driver used to register it's dev itself, to provide a custom
> > > attribute. With the modified most subsystem, this causes duplicate
> > > registration of the same device object. Fix that by adding that custom
> > > attribute to the platform device - that is a better location for
> > > a platform-specific attribute anyway.
> > 
> > Nope, it should be 4 patches.
> 
> Unlike the above three, this item could be separated.
> Will split into two patches now - the first for this (and with fix to the
> attributes issue noted below) and the second for proper device releasing.
> 
> > Also, why have you not cc:ed the original author of the commit you are
> > "fixing" here?   They are the maintainer of this code, right?
> 
> I was under impression that "git send-email" does that automatically...

Nope.

> CCing them now.

They don't see the change :(

> > One note on your change that would keep me from accepting it even if all
> > of the above was not an issue:
> > 
> > > diff --git a/drivers/staging/most/dim2/sysfs.c b/drivers/staging/most/dim2/sysfs.c
> > > index c85b2cdcdca3..22836c8ed554 100644
> > > --- a/drivers/staging/most/dim2/sysfs.c
> > > +++ b/drivers/staging/most/dim2/sysfs.c
> > > @@ -39,11 +39,10 @@ static const struct attribute_group *dev_attr_groups[] = {
> > >   int dim2_sysfs_probe(struct device *dev)
> > >   {
> > > -	dev->groups = dev_attr_groups;
> > > -	return device_register(dev);
> > > +	return sysfs_create_groups(&dev->kobj, dev_attr_groups);
> > 
> > No driver code should ever be calling a sysfs_* function, which is a
> > huge hint that this is incorrect.
> > 
> > You also just raced with userspace and lost, please use the default
> > attributes for the driver or bus for this, but NEVER manually add and
> > remove sysfs files, that way lies madness and hard to maintain code.
> I'm aware of this race, but still creating attributes on device probe is under wide use in the kernel:
> 
> nikita@cobook:~/kernel$ grep -r device_create_file drivers | wc -l
> 448

Just because there are bad examples, does not mean you should add more.

And not all of these are wrong, but do you know when they are not wrong?

> Still, in case of dim2 driver, moving to driver's dev_groups is
> trivial. Preparing that patch now.

That should have already been done, right?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] staging: most: dim2: fix device registration
  2021-10-05 13:49     ` Greg Kroah-Hartman
@ 2021-10-05 14:07       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2021-10-05 14:07 UTC (permalink / raw)
  To: Nikita Yushchenko; +Cc: Lee Jones, linux-staging, linux-kernel, Christian Gromm

On Tue, Oct 05, 2021 at 03:49:09PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Oct 05, 2021 at 04:33:02PM +0300, Nikita Yushchenko wrote:
> > > > Commit 723de0f9171e ("staging: most: remove device from interface
> > > > structure") moved registration of driver-provided struct device to
> > > > the most subsystem, but did not properly update dim2 driver to
> > > > work with that change.
> > > > 
> > > > After most subsystem passes driver's dev to register_device(), it
> > > > becomes refcounted, and can be only deallocated in the release method.
> > > > Provide that by:
> > > > - not using devres to allocate the device,
> > > > - moving shutdown actions from _remove() to the device release method,
> > > > - not calling shutdown actions in _probe() after the device becomes
> > > >    refcounted.
> > > 
> > > Should this be 3 patches?
> > 
> > But these three items are deeply interconnected, and fix the issue together.
> > Must not manually free device structure passed to register_device(), thus
> > must not allocate via devres (because otherwise, devres will free it). Once
> > not using devres for it, must deallocate it somehow else, thus must rework
> > the release paths.
> 
> Ok, but that was obvious.

That was *not* obvious.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] staging: most: dim2: fix device registration
  2021-10-05 13:33   ` Nikita Yushchenko
  2021-10-05 13:49     ` Greg Kroah-Hartman
@ 2021-10-05 14:17     ` Dan Carpenter
  1 sibling, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2021-10-05 14:17 UTC (permalink / raw)
  To: Nikita Yushchenko
  Cc: Greg Kroah-Hartman, Lee Jones, linux-staging, linux-kernel,
	Christian Gromm

On Tue, Oct 05, 2021 at 04:33:02PM +0300, Nikita Yushchenko wrote:
> > Also, why have you not cc:ed the original author of the commit you are
> > "fixing" here?   They are the maintainer of this code, right?
> 
> I was under impression that "git send-email" does that automatically...
> 

The ./scripts/get_maintainer.pl does try to CC them.  It's working for
me when I use the script on your patch.  I normally put the author in
the To: header so it's more likely they'll see my patch and Ack it.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] staging: most: dim2: do not double-register the same device
  2021-10-05 10:27 ` Greg Kroah-Hartman
  2021-10-05 13:33   ` Nikita Yushchenko
@ 2021-10-05 14:34   ` Nikita Yushchenko
  2021-10-10  6:27     ` Greg Kroah-Hartman
  2021-10-05 14:34   ` [PATCH 2/2] staging: most: dim2: use device release method Nikita Yushchenko
  2 siblings, 1 reply; 11+ messages in thread
From: Nikita Yushchenko @ 2021-10-05 14:34 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Christian Gromm, Lee Jones
  Cc: linux-staging, linux-kernel, Nikita Yushchenko

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@cogentembedded.com>
---
 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" : "");
 }
 
+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,
+};
+
 /**
  * dimcb_on_error - callback from HAL to report miscommunication between
  * HDM and HAL
@@ -874,16 +891,8 @@ static int dim2_probe(struct platform_device *pdev)
 		goto err_stop_thread;
 	}
 
-	ret = dim2_sysfs_probe(&dev->dev);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to create sysfs attribute\n");
-		goto err_unreg_iface;
-	}
-
 	return 0;
 
-err_unreg_iface:
-	most_deregister_interface(&dev->most_iface);
 err_stop_thread:
 	kthread_stop(dev->netinfo_task);
 err_shutdown_dim:
@@ -906,7 +915,6 @@ static int dim2_remove(struct platform_device *pdev)
 	struct dim2_hdm *dev = platform_get_drvdata(pdev);
 	unsigned long flags;
 
-	dim2_sysfs_destroy(&dev->dev);
 	most_deregister_interface(&dev->most_iface);
 	kthread_stop(dev->netinfo_task);
 
@@ -1100,6 +1108,7 @@ static struct platform_driver dim2_driver = {
 	.driver = {
 		.name = "hdm_dim2",
 		.of_match_table = dim2_of_match,
+		.dev_groups = dim2_attr_groups,
 	},
 };
 
diff --git a/drivers/staging/most/dim2/sysfs.c b/drivers/staging/most/dim2/sysfs.c
deleted file mode 100644
index c85b2cdcdca3..000000000000
--- a/drivers/staging/most/dim2/sysfs.c
+++ /dev/null
@@ -1,49 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * sysfs.c - MediaLB sysfs information
- *
- * Copyright (C) 2015, Microchip Technology Germany II GmbH & Co. KG
- */
-
-/* Author: Andrey Shvetsov <andrey.shvetsov@k2l.de> */
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/kernel.h>
-#include "sysfs.h"
-#include <linux/device.h>
-
-static ssize_t state_show(struct device *dev, struct device_attribute *attr,
-			  char *buf)
-{
-	bool state = dim2_sysfs_get_state_cb();
-
-	return sprintf(buf, "%s\n", state ? "locked" : "");
-}
-
-static DEVICE_ATTR_RO(state);
-
-static struct attribute *dev_attrs[] = {
-	&dev_attr_state.attr,
-	NULL,
-};
-
-static struct attribute_group dev_attr_group = {
-	.attrs = dev_attrs,
-};
-
-static const struct attribute_group *dev_attr_groups[] = {
-	&dev_attr_group,
-	NULL,
-};
-
-int dim2_sysfs_probe(struct device *dev)
-{
-	dev->groups = dev_attr_groups;
-	return device_register(dev);
-}
-
-void dim2_sysfs_destroy(struct device *dev)
-{
-	device_unregister(dev);
-}
diff --git a/drivers/staging/most/dim2/sysfs.h b/drivers/staging/most/dim2/sysfs.h
index 24277a17cff3..09115cf4ed00 100644
--- a/drivers/staging/most/dim2/sysfs.h
+++ b/drivers/staging/most/dim2/sysfs.h
@@ -16,15 +16,4 @@ struct medialb_bus {
 	struct kobject kobj_group;
 };
 
-struct device;
-
-int dim2_sysfs_probe(struct device *dev);
-void dim2_sysfs_destroy(struct device *dev);
-
-/*
- * callback,
- * must deliver MediaLB state as true if locked or false if unlocked
- */
-bool dim2_sysfs_get_state_cb(void);
-
 #endif	/* DIM2_SYSFS_H */
-- 
2.30.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/2] staging: most: dim2: use device release method
  2021-10-05 10:27 ` Greg Kroah-Hartman
  2021-10-05 13:33   ` Nikita Yushchenko
  2021-10-05 14:34   ` [PATCH 1/2] staging: most: dim2: do not double-register the same device Nikita Yushchenko
@ 2021-10-05 14:34   ` Nikita Yushchenko
  2 siblings, 0 replies; 11+ messages in thread
From: Nikita Yushchenko @ 2021-10-05 14:34 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Christian Gromm, Lee Jones
  Cc: linux-staging, linux-kernel, Nikita Yushchenko

Commit 723de0f9171e ("staging: most: remove device from interface
structure") moved registration of driver-provided struct device to
the most subsystem. This updated dim2 driver as well.

However, struct device passed to register_device() becomes refcounted,
and must not be explicitly deallocated, but must provide release method
instead. Which is incompatible with managing it via devres.

This patch makes the device structure allocated without devres, adds
device release method, and moves device destruction there.

Fixes: 723de0f9171e ("staging: most: remove device from interface structure")
Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
---
 drivers/staging/most/dim2/dim2.c | 55 +++++++++++++++++---------------
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index bb6dd508e531..4c0c3bc91ac0 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -734,6 +734,23 @@ static int get_dim2_clk_speed(const char *clock_speed, u8 *val)
 	return -EINVAL;
 }
 
+static void dim2_release(struct device *d)
+{
+	struct dim2_hdm *dev = container_of(d, struct dim2_hdm, dev);
+	unsigned long flags;
+
+	kthread_stop(dev->netinfo_task);
+
+	spin_lock_irqsave(&dim_lock, flags);
+	dim_shutdown();
+	spin_unlock_irqrestore(&dim_lock, flags);
+
+	if (dev->disable_platform)
+		dev->disable_platform(to_platform_device(d->parent));
+
+	kfree(dev);
+}
+
 /*
  * dim2_probe - dim2 probe handler
  * @pdev: platform device structure
@@ -755,7 +772,7 @@ static int dim2_probe(struct platform_device *pdev)
 
 	enum { MLB_INT_IDX, AHB0_INT_IDX };
 
-	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
 	if (!dev)
 		return -ENOMEM;
 
@@ -767,19 +784,21 @@ static int dim2_probe(struct platform_device *pdev)
 				      "microchip,clock-speed", &clock_speed);
 	if (ret) {
 		dev_err(&pdev->dev, "missing dt property clock-speed\n");
-		return ret;
+		goto err_free_dev;
 	}
 
 	ret = get_dim2_clk_speed(clock_speed, &dev->clk_speed);
 	if (ret) {
 		dev_err(&pdev->dev, "bad dt property clock-speed\n");
-		return ret;
+		goto err_free_dev;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	dev->io_base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(dev->io_base))
-		return PTR_ERR(dev->io_base);
+	if (IS_ERR(dev->io_base)) {
+		ret = PTR_ERR(dev->io_base);
+		goto err_free_dev;
+	}
 
 	of_id = of_match_node(dim2_of_match, pdev->dev.of_node);
 	pdata = of_id->data;
@@ -787,7 +806,7 @@ static int dim2_probe(struct platform_device *pdev)
 		if (pdata->enable) {
 			ret = pdata->enable(pdev);
 			if (ret)
-				return ret;
+				goto err_free_dev;
 		}
 		dev->disable_platform = pdata->disable;
 		if (pdata->fcnt)
@@ -882,24 +901,19 @@ static int dim2_probe(struct platform_device *pdev)
 	dev->most_iface.request_netinfo = request_netinfo;
 	dev->most_iface.driver_dev = &pdev->dev;
 	dev->most_iface.dev = &dev->dev;
-	dev->dev.init_name = "dim2_state";
+	dev->dev.init_name = dev->name;
 	dev->dev.parent = &pdev->dev;
+	dev->dev.release = dim2_release;
 
-	ret = most_register_interface(&dev->most_iface);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to register MOST interface\n");
-		goto err_stop_thread;
-	}
-
-	return 0;
+	return most_register_interface(&dev->most_iface);
 
-err_stop_thread:
-	kthread_stop(dev->netinfo_task);
 err_shutdown_dim:
 	dim_shutdown();
 err_disable_platform:
 	if (dev->disable_platform)
 		dev->disable_platform(pdev);
+err_free_dev:
+	kfree(dev);
 
 	return ret;
 }
@@ -913,17 +927,8 @@ static int dim2_probe(struct platform_device *pdev)
 static int dim2_remove(struct platform_device *pdev)
 {
 	struct dim2_hdm *dev = platform_get_drvdata(pdev);
-	unsigned long flags;
 
 	most_deregister_interface(&dev->most_iface);
-	kthread_stop(dev->netinfo_task);
-
-	spin_lock_irqsave(&dim_lock, flags);
-	dim_shutdown();
-	spin_unlock_irqrestore(&dim_lock, flags);
-
-	if (dev->disable_platform)
-		dev->disable_platform(pdev);
 
 	return 0;
 }
-- 
2.30.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] staging: most: dim2: do not double-register the same device
  2021-10-05 14:34   ` [PATCH 1/2] staging: most: dim2: do not double-register the same device Nikita Yushchenko
@ 2021-10-10  6:27     ` Greg Kroah-Hartman
  2021-10-11  6:11       ` [PATCH v2 " Nikita Yushchenko
  0 siblings, 1 reply; 11+ messages in thread
From: Greg Kroah-Hartman @ 2021-10-10  6:27 UTC (permalink / raw)
  To: Nikita Yushchenko; +Cc: Christian Gromm, Lee Jones, linux-staging, linux-kernel

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@cogentembedded.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/2] staging: most: dim2: do not double-register the same device
  2021-10-10  6:27     ` Greg Kroah-Hartman
@ 2021-10-11  6:11       ` Nikita Yushchenko
  2021-10-12 12:59         ` Christian.Gromm
  0 siblings, 1 reply; 11+ messages in thread
From: Nikita Yushchenko @ 2021-10-11  6:11 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Christian Gromm, Lee Jones
  Cc: linux-staging, linux-kernel, Nikita Yushchenko

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@cogentembedded.com>
---
Changes from v1:
- use ATTRIBUTE_GROUPS()
- use sysfs_emit()

 drivers/staging/most/dim2/Makefile |  2 +-
 drivers/staging/most/dim2/dim2.c   | 24 ++++++++-------
 drivers/staging/most/dim2/sysfs.c  | 49 ------------------------------
 drivers/staging/most/dim2/sysfs.h  | 11 -------
 4 files changed, 14 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..96cb5280a385 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,18 @@ bool dim2_sysfs_get_state_cb(void)
 	state = dim_get_lock_state();
 	spin_unlock_irqrestore(&dim_lock, flags);
 
-	return state;
+	return sysfs_emit(buf, "%s\n", state ? "locked" : "");
 }
 
+static DEVICE_ATTR_RO(state);
+
+static struct attribute *dim2_attrs[] = {
+	&dev_attr_state.attr,
+	NULL,
+};
+
+ATTRIBUTE_GROUPS(dim2);
+
 /**
  * dimcb_on_error - callback from HAL to report miscommunication between
  * HDM and HAL
@@ -874,16 +884,8 @@ static int dim2_probe(struct platform_device *pdev)
 		goto err_stop_thread;
 	}
 
-	ret = dim2_sysfs_probe(&dev->dev);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to create sysfs attribute\n");
-		goto err_unreg_iface;
-	}
-
 	return 0;
 
-err_unreg_iface:
-	most_deregister_interface(&dev->most_iface);
 err_stop_thread:
 	kthread_stop(dev->netinfo_task);
 err_shutdown_dim:
@@ -906,7 +908,6 @@ static int dim2_remove(struct platform_device *pdev)
 	struct dim2_hdm *dev = platform_get_drvdata(pdev);
 	unsigned long flags;
 
-	dim2_sysfs_destroy(&dev->dev);
 	most_deregister_interface(&dev->most_iface);
 	kthread_stop(dev->netinfo_task);
 
@@ -1100,6 +1101,7 @@ static struct platform_driver dim2_driver = {
 	.driver = {
 		.name = "hdm_dim2",
 		.of_match_table = dim2_of_match,
+		.dev_groups = dim2_groups,
 	},
 };
 
diff --git a/drivers/staging/most/dim2/sysfs.c b/drivers/staging/most/dim2/sysfs.c
deleted file mode 100644
index c85b2cdcdca3..000000000000
--- a/drivers/staging/most/dim2/sysfs.c
+++ /dev/null
@@ -1,49 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * sysfs.c - MediaLB sysfs information
- *
- * Copyright (C) 2015, Microchip Technology Germany II GmbH & Co. KG
- */
-
-/* Author: Andrey Shvetsov <andrey.shvetsov@k2l.de> */
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/kernel.h>
-#include "sysfs.h"
-#include <linux/device.h>
-
-static ssize_t state_show(struct device *dev, struct device_attribute *attr,
-			  char *buf)
-{
-	bool state = dim2_sysfs_get_state_cb();
-
-	return sprintf(buf, "%s\n", state ? "locked" : "");
-}
-
-static DEVICE_ATTR_RO(state);
-
-static struct attribute *dev_attrs[] = {
-	&dev_attr_state.attr,
-	NULL,
-};
-
-static struct attribute_group dev_attr_group = {
-	.attrs = dev_attrs,
-};
-
-static const struct attribute_group *dev_attr_groups[] = {
-	&dev_attr_group,
-	NULL,
-};
-
-int dim2_sysfs_probe(struct device *dev)
-{
-	dev->groups = dev_attr_groups;
-	return device_register(dev);
-}
-
-void dim2_sysfs_destroy(struct device *dev)
-{
-	device_unregister(dev);
-}
diff --git a/drivers/staging/most/dim2/sysfs.h b/drivers/staging/most/dim2/sysfs.h
index 24277a17cff3..09115cf4ed00 100644
--- a/drivers/staging/most/dim2/sysfs.h
+++ b/drivers/staging/most/dim2/sysfs.h
@@ -16,15 +16,4 @@ struct medialb_bus {
 	struct kobject kobj_group;
 };
 
-struct device;
-
-int dim2_sysfs_probe(struct device *dev);
-void dim2_sysfs_destroy(struct device *dev);
-
-/*
- * callback,
- * must deliver MediaLB state as true if locked or false if unlocked
- */
-bool dim2_sysfs_get_state_cb(void);
-
 #endif	/* DIM2_SYSFS_H */
-- 
2.30.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] staging: most: dim2: do not double-register the same device
  2021-10-11  6:11       ` [PATCH v2 " Nikita Yushchenko
@ 2021-10-12 12:59         ` Christian.Gromm
  0 siblings, 0 replies; 11+ messages in thread
From: Christian.Gromm @ 2021-10-12 12:59 UTC (permalink / raw)
  To: gregkh, nikita.yoush, lee.jones; +Cc: linux-kernel, linux-staging

On Mon, 2021-10-11 at 09:11 +0300, Nikita Yushchenko wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> 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@cogentembedded.com>
> ---
> Changes from v1:
> - use ATTRIBUTE_GROUPS()
> - use sysfs_emit()
> 
>  drivers/staging/most/dim2/Makefile |  2 +-
>  drivers/staging/most/dim2/dim2.c   | 24 ++++++++-------
>  drivers/staging/most/dim2/sysfs.c  | 49 ----------------------------
> --
>  drivers/staging/most/dim2/sysfs.h  | 11 -------
>  4 files changed, 14 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..96cb5280a385 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,18 @@ bool dim2_sysfs_get_state_cb(void)
>         state = dim_get_lock_state();
>         spin_unlock_irqrestore(&dim_lock, flags);
> 
> -       return state;
> +       return sysfs_emit(buf, "%s\n", state ? "locked" : "");
>  }
> 
> +static DEVICE_ATTR_RO(state);
> +
> +static struct attribute *dim2_attrs[] = {
> +       &dev_attr_state.attr,
> +       NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(dim2);
> +
>  /**
>   * dimcb_on_error - callback from HAL to report miscommunication
> between
>   * HDM and HAL
> @@ -874,16 +884,8 @@ static int dim2_probe(struct platform_device
> *pdev)
>                 goto err_stop_thread;
>         }
> 
> -       ret = dim2_sysfs_probe(&dev->dev);
> -       if (ret) {
> -               dev_err(&pdev->dev, "failed to create sysfs
> attribute\n");
> -               goto err_unreg_iface;
> -       }
> -
>         return 0;
> 
> -err_unreg_iface:
> -       most_deregister_interface(&dev->most_iface);
>  err_stop_thread:
>         kthread_stop(dev->netinfo_task);
>  err_shutdown_dim:
> @@ -906,7 +908,6 @@ static int dim2_remove(struct platform_device
> *pdev)
>         struct dim2_hdm *dev = platform_get_drvdata(pdev);
>         unsigned long flags;
> 
> -       dim2_sysfs_destroy(&dev->dev);
>         most_deregister_interface(&dev->most_iface);
>         kthread_stop(dev->netinfo_task);
> 
> @@ -1100,6 +1101,7 @@ static struct platform_driver dim2_driver = {
>         .driver = {
>                 .name = "hdm_dim2",
>                 .of_match_table = dim2_of_match,
> +               .dev_groups = dim2_groups,
>         },
>  };
> 
> diff --git a/drivers/staging/most/dim2/sysfs.c
> b/drivers/staging/most/dim2/sysfs.c
> deleted file mode 100644
> index c85b2cdcdca3..000000000000
> --- a/drivers/staging/most/dim2/sysfs.c
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * sysfs.c - MediaLB sysfs information
> - *
> - * Copyright (C) 2015, Microchip Technology Germany II GmbH & Co. KG
> - */
> -
> -/* Author: Andrey Shvetsov <andrey.shvetsov@k2l.de> */
> -
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> -#include <linux/kernel.h>
> -#include "sysfs.h"
> -#include <linux/device.h>
> -
> -static ssize_t state_show(struct device *dev, struct
> device_attribute *attr,
> -                         char *buf)
> -{
> -       bool state = dim2_sysfs_get_state_cb();
> -
> -       return sprintf(buf, "%s\n", state ? "locked" : "");
> -}
> -
> -static DEVICE_ATTR_RO(state);
> -
> -static struct attribute *dev_attrs[] = {
> -       &dev_attr_state.attr,
> -       NULL,
> -};
> -
> -static struct attribute_group dev_attr_group = {
> -       .attrs = dev_attrs,
> -};
> -
> -static const struct attribute_group *dev_attr_groups[] = {
> -       &dev_attr_group,
> -       NULL,
> -};
> -
> -int dim2_sysfs_probe(struct device *dev)
> -{
> -       dev->groups = dev_attr_groups;
> -       return device_register(dev);
> -}
> -
> -void dim2_sysfs_destroy(struct device *dev)
> -{
> -       device_unregister(dev);
> -}
> diff --git a/drivers/staging/most/dim2/sysfs.h
> b/drivers/staging/most/dim2/sysfs.h
> index 24277a17cff3..09115cf4ed00 100644
> --- a/drivers/staging/most/dim2/sysfs.h
> +++ b/drivers/staging/most/dim2/sysfs.h
> @@ -16,15 +16,4 @@ struct medialb_bus {
>         struct kobject kobj_group;
>  };
> 
> -struct device;
> -
> -int dim2_sysfs_probe(struct device *dev);
> -void dim2_sysfs_destroy(struct device *dev);
> -
> -/*
> - * callback,
> - * must deliver MediaLB state as true if locked or false if unlocked
> - */
> -bool dim2_sysfs_get_state_cb(void);
> -
>  #endif /* DIM2_SYSFS_H */
> --
> 2.30.2
> 
> 
> 

Acked-by: Christian Gromm <christian.gromm@microchip.com> 

thanks,
Chris

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-10-12 12:59 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29 20:56 [PATCH] staging: most: dim2: fix device registration Nikita Yushchenko
2021-10-05 10:27 ` Greg Kroah-Hartman
2021-10-05 13:33   ` Nikita Yushchenko
2021-10-05 13:49     ` Greg Kroah-Hartman
2021-10-05 14:07       ` Greg Kroah-Hartman
2021-10-05 14:17     ` Dan Carpenter
2021-10-05 14:34   ` [PATCH 1/2] staging: most: dim2: do not double-register the same device Nikita Yushchenko
2021-10-10  6:27     ` Greg Kroah-Hartman
2021-10-11  6:11       ` [PATCH v2 " Nikita Yushchenko
2021-10-12 12:59         ` Christian.Gromm
2021-10-05 14:34   ` [PATCH 2/2] staging: most: dim2: use device release method Nikita Yushchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).