leds: leds-fsg: Use devm_led_classdev_register

PATCH Superseded Vaishali Thakkar 2 participants 7 messages Aug 15, 2015 – Aug 18, 2015 1 message not in the archive LED

lore

Share & tools

lore

Vaishali Thakkar

author

2015-08-15 10:21 UTC #0 lore raw

PATCH 1/1 leds: leds-fsg: Use devm_led_classdev_register

Use resource-managed function devm_led_classdev_register instead of led_classdev_register to make the error-path simpler.

To be compatible with the change, various gotos are replced with direct returns and unneeded labels are dropped. Also, remove fsg_led_remove as it is now redundant.

Signed-off-by Vaishali Thakkar <[email protected]>

---
 drivers/leds/leds-fsg.c | 51 ++++++++++++-------------------------------------
 1 file changed, 12 insertions(+), 39 deletions(-)

drivers/leds/leds-fsg.c +12 ~~-39~~

index 2b4dc73..df0e5da 100644
@@ -156,60 +156,33 @@ static int fsg_led_probe(struct platform_device *pdev)
 	latch_value = 0xffff;
 	*latch_address = latch_value;
 
-	ret = led_classdev_register(&pdev->dev, &fsg_wlan_led);
+	ret = devm_led_classdev_register(&pdev->dev, &fsg_wlan_led);
 	if (ret < 0)
-		goto failwlan;
+		return ret;
 
-	ret = led_classdev_register(&pdev->dev, &fsg_wan_led);
+	ret = devm_led_classdev_register(&pdev->dev, &fsg_wan_led);
 	if (ret < 0)
-		goto failwan;
+		return ret;
 
-	ret = led_classdev_register(&pdev->dev, &fsg_sata_led);
+	ret = devm_led_classdev_register(&pdev->dev, &fsg_sata_led);
 	if (ret < 0)
-		goto failsata;
+		return ret;
 
-	ret = led_classdev_register(&pdev->dev, &fsg_usb_led);
+	ret = devm_led_classdev_register(&pdev->dev, &fsg_usb_led);
 	if (ret < 0)
-		goto failusb;
+		return ret;
 
-	ret = led_classdev_register(&pdev->dev, &fsg_sync_led);
+	ret = devm_classdev_register(&pdev->dev, &fsg_sync_led);
 	if (ret < 0)
-		goto failsync;
+		return ret;
 
-	ret = led_classdev_register(&pdev->dev, &fsg_ring_led);
+	ret = devm_led_classdev_register(&pdev->dev, &fsg_ring_led);
 	if (ret < 0)
-		goto failring;
-
-	return ret;
-
- failring:
-	led_classdev_unregister(&fsg_sync_led);
- failsync:
-	led_classdev_unregister(&fsg_usb_led);
- failusb:
-	led_classdev_unregister(&fsg_sata_led);
- failsata:
-	led_classdev_unregister(&fsg_wan_led);
- failwan:
-	led_classdev_unregister(&fsg_wlan_led);
- failwlan:
+		return ret;
 
 	return ret;
 }
 
-static int fsg_led_remove(struct platform_device *pdev)
-{
-	led_classdev_unregister(&fsg_wlan_led);
-	led_classdev_unregister(&fsg_wan_led);
-	led_classdev_unregister(&fsg_sata_led);
-	led_classdev_unregister(&fsg_usb_led);
-	led_classdev_unregister(&fsg_sync_led);
-	led_classdev_unregister(&fsg_ring_led);
-
-	return 0;
-}
-
-
 static struct platform_driver fsg_led_driver = {
 	.probe		= fsg_led_probe,
 	.remove		= fsg_led_remove,

Signature · 1 line

1.9.1

Jacek Anaszewski

2015-08-17 08:27 UTC #1 lore raw

Hi Vaishali,

Thanks for the patch. Unfortunately it causes build break. Please fix the issues and resubmit.

Quoted 90 lines · Vaishali Thakkar

Use resource-managed function devm_led_classdev_register instead of led_classdev_register to make the error-path simpler.

To be compatible with the change, various gotos are replced with direct returns and unneeded labels are dropped. Also, remove fsg_led_remove as it is now redundant.

Signed-off-by Vaishali Thakkar <[email protected]>

---
  drivers/leds/leds-fsg.c | 51 ++++++++++++-------------------------------------
  1 file changed, 12 insertions(+), 39 deletions(-)

drivers/leds/leds-fsg.c +12 ~~-39~~

index 2b4dc73..df0e5da 100644
@@ -156,60 +156,33 @@ static int fsg_led_probe(struct platform_device *pdev)
  	latch_value = 0xffff;
  	*latch_address = latch_value;
 
-	ret = led_classdev_register(&pdev->dev, &fsg_wlan_led);
+	ret = devm_led_classdev_register(&pdev->dev, &fsg_wlan_led);
  	if (ret < 0)
-		goto failwlan;
+		return ret;
 
-	ret = led_classdev_register(&pdev->dev, &fsg_wan_led);
+	ret = devm_led_classdev_register(&pdev->dev, &fsg_wan_led);
  	if (ret < 0)
-		goto failwan;
+		return ret;
 
-	ret = led_classdev_register(&pdev->dev, &fsg_sata_led);
+	ret = devm_led_classdev_register(&pdev->dev, &fsg_sata_led);
  	if (ret < 0)
-		goto failsata;
+		return ret;
 
-	ret = led_classdev_register(&pdev->dev, &fsg_usb_led);
+	ret = devm_led_classdev_register(&pdev->dev, &fsg_usb_led);
  	if (ret < 0)
-		goto failusb;
+		return ret;
 
-	ret = led_classdev_register(&pdev->dev, &fsg_sync_led);
+	ret = devm_classdev_register(&pdev->dev, &fsg_sync_led);
  	if (ret < 0)
-		goto failsync;
+		return ret;
 
-	ret = led_classdev_register(&pdev->dev, &fsg_ring_led);
+	ret = devm_led_classdev_register(&pdev->dev, &fsg_ring_led);
  	if (ret < 0)
-		goto failring;
-
-	return ret;
-
- failring:
-	led_classdev_unregister(&fsg_sync_led);
- failsync:
-	led_classdev_unregister(&fsg_usb_led);
- failusb:
-	led_classdev_unregister(&fsg_sata_led);
- failsata:
-	led_classdev_unregister(&fsg_wan_led);
- failwan:
-	led_classdev_unregister(&fsg_wlan_led);
- failwlan:
+		return ret;
 
  	return ret;
  }
 
-static int fsg_led_remove(struct platform_device *pdev)
-{
-	led_classdev_unregister(&fsg_wlan_led);
-	led_classdev_unregister(&fsg_wan_led);
-	led_classdev_unregister(&fsg_sata_led);
-	led_classdev_unregister(&fsg_usb_led);
-	led_classdev_unregister(&fsg_sync_led);
-	led_classdev_unregister(&fsg_ring_led);
-
-	return 0;
-}
-
-
  static struct platform_driver fsg_led_driver = {
  	.probe		= fsg_led_probe,
  	.remove		= fsg_led_remove,

Signature · 2 lines

Best Regards,
Jacek Anaszewski

Vaishali Thakkar

author

2015-08-18 05:46 UTC #3 lore raw

In reply to a message not in the archive

Quoted 5 lines · Vaishali Thakkar

Quoted 3 lines · Jacek Anaszewski
Hi Vaishali,

Quoted 7 lines

Quoted 2 lines
Thanks for the patch. Unfortunately it causes build break. Please fix the issues and resubmit.
Oops! I am really very sorry. By mistake I sent a basic version of patch instead of final one. Thanks for the review.
I'll send v2 with the fix of that build error.

Hi Jacek,

Can you please tell me what are those build errors? After looking at my patch, I can expect that as I forgot to
remove '.remove = fsg_led_remove' from structure,
it can cause an error. Also, there is one typing mistake in function name. So, both of these can be a reason for breaking a build.

Also to be on safe side, I tried to cross compile it for arm architecture. But to my surprise, it is not compiling properly. So, it would be good if you can point me to the script you used for compilation so that I can check it before sending a new version.

Thank You.

Quoted 100 lines

Quoted 100 lines · Vaishali Thakkar

Quoted 91 lines

Use resource-managed function devm_led_classdev_register instead of led_classdev_register to make the error-path simpler.

To be compatible with the change, various gotos are replced with direct returns and unneeded labels are dropped. Also, remove fsg_led_remove as it is now redundant.

Signed-off-by Vaishali Thakkar <[email protected]>

---
  drivers/leds/leds-fsg.c | 51
++++++++++++-------------------------------------
  1 file changed, 12 insertions(+), 39 deletions(-)

drivers/leds/leds-fsg.c +12 ~~-39~~

index 2b4dc73..df0e5da 100644
@@ -156,60 +156,33 @@ static int fsg_led_probe(struct platform_device
 *pdev)
        latch_value = 0xffff;
        *latch_address = latch_value;
 
-       ret = led_classdev_register(&pdev->dev, &fsg_wlan_led);
+       ret = devm_led_classdev_register(&pdev->dev, &fsg_wlan_led);
        if (ret < 0)
-               goto failwlan;
+               return ret;
 
-       ret = led_classdev_register(&pdev->dev, &fsg_wan_led);
+       ret = devm_led_classdev_register(&pdev->dev, &fsg_wan_led);
        if (ret < 0)
-               goto failwan;
+               return ret;
 
-       ret = led_classdev_register(&pdev->dev, &fsg_sata_led);
+       ret = devm_led_classdev_register(&pdev->dev, &fsg_sata_led);
        if (ret < 0)
-               goto failsata;
+               return ret;
 
-       ret = led_classdev_register(&pdev->dev, &fsg_usb_led);
+       ret = devm_led_classdev_register(&pdev->dev, &fsg_usb_led);
        if (ret < 0)
-               goto failusb;
+               return ret;
 
-       ret = led_classdev_register(&pdev->dev, &fsg_sync_led);
+       ret = devm_classdev_register(&pdev->dev, &fsg_sync_led);
        if (ret < 0)
-               goto failsync;
+               return ret;
 
-       ret = led_classdev_register(&pdev->dev, &fsg_ring_led);
+       ret = devm_led_classdev_register(&pdev->dev, &fsg_ring_led);
        if (ret < 0)
-               goto failring;
-
-       return ret;
-
- failring:
-       led_classdev_unregister(&fsg_sync_led);
- failsync:
-       led_classdev_unregister(&fsg_usb_led);
- failusb:
-       led_classdev_unregister(&fsg_sata_led);
- failsata:
-       led_classdev_unregister(&fsg_wan_led);
- failwan:
-       led_classdev_unregister(&fsg_wlan_led);
- failwlan:
+               return ret;
 
        return ret;
  }
 
-static int fsg_led_remove(struct platform_device *pdev)
-{
-       led_classdev_unregister(&fsg_wlan_led);
-       led_classdev_unregister(&fsg_wan_led);
-       led_classdev_unregister(&fsg_sata_led);
-       led_classdev_unregister(&fsg_usb_led);
-       led_classdev_unregister(&fsg_sync_led);
-       led_classdev_unregister(&fsg_ring_led);
-
-       return 0;
-}
-
-
  static struct platform_driver fsg_led_driver = {
        .probe          = fsg_led_probe,

Quoted 2 lines

.remove = fsg_led_remove,

--
Best Regards,
Jacek Anaszewski

Signature · 1 line

Vaishali

Jacek Anaszewski

2015-08-18 07:27 UTC #4 lore raw

Hi Vaishali,

Quoted 15 lines · Vaishali Thakkar

Quoted 5 lines · Vaishali Thakkar
Quoted 3 lines · Jacek Anaszewski
Hi Vaishali,
Quoted 7 lines
Quoted 2 lines
Thanks for the patch. Unfortunately it causes build break. Please fix the issues and resubmit.
Oops! I am really very sorry. By mistake I sent a basic version of patch instead of final one. Thanks for the review.
I'll send v2 with the fix of that build error.

Quoted 8 lines

Hi Jacek,
Can you please tell me what are those build errors? After looking at my patch, I can expect that as I forgot to
remove '.remove = fsg_led_remove' from structure,
it can cause an error. Also, there is one typing mistake in function name. So, both of these can be a reason for breaking a build.

Those was the errors I experienced too.

Quoted 5 lines

Also to be on safe side, I tried to cross compile it for arm architecture. But to my surprise, it is not compiling properly. So, it would be good if you can point me to the script you used for compilation so that I can check it before sending a new version.

I am using arm-linux-gnueabi cross toolchain.

config: arch/arm/configs/ixp4xx_defconfig

Build command:

make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- all

After fixing the errors compilation succeeded.

Do you have a hardware to test the modifications against?

Signature · 2 lines

Best Regards,
Jacek Anaszewski

Vaishali Thakkar

author

2015-08-18 12:14 UTC #5 lore raw

Quoted 43 lines · Jacek Anaszewski

Hi Vaishali,
Quoted 21 lines · Vaishali Thakkar
Quoted 8 lines · Vaishali Thakkar
Quoted 4 lines · Jacek Anaszewski
Hi Vaishali,
Quoted 8 lines
Quoted 2 lines
Thanks for the patch. Unfortunately it causes build break. Please fix the issues and resubmit.
Oops! I am really very sorry. By mistake I sent a basic version of patch instead of final one. Thanks for the review.
I'll send v2 with the fix of that build error.
Quoted 8 lines
Hi Jacek,
Can you please tell me what are those build errors? After looking at my patch, I can expect that as I forgot to
remove '.remove = fsg_led_remove' from structure,
it can cause an error. Also, there is one typing mistake in function name. So, both of these can be a reason for breaking a build.
Those was the errors I experienced too.
Quoted 5 lines
Also to be on safe side, I tried to cross compile it for arm architecture. But to my surprise, it is not compiling properly. So, it would be good if you can point me to the script you used for compilation so that I can check it before sending a new version.

Quoted 7 lines

I am using arm-linux-gnueabi cross toolchain.
config: arch/arm/configs/ixp4xx_defconfig
Build command:
make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- all

Ok. Thanks for the information. I was using
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross.

Quoted 2 lines

After fixing the errors compilation succeeded. Do you have a hardware to test the modifications against?

Ok. I checked with arm-linux-gnueabi cross toolchain and fixing these 2 errors allow us to compile the file successfully.

No, I don't have a hardware to test these modifications. I think this change should not lead to any breakage. But yes it is good to test such changes. Can you please test it for me (if you have a hardware), so that I can send v2 with fixing those errors?

Quoted 4 lines

--
Best Regards,
Jacek Anaszewski

Signature · 1 line

Vaishali

Jacek Anaszewski

2015-08-18 12:36 UTC #6 lore raw

Quoted 63 lines · Vaishali Thakkar

Quoted 43 lines · Jacek Anaszewski
Hi Vaishali,
Quoted 21 lines · Vaishali Thakkar
Quoted 8 lines · Vaishali Thakkar
Quoted 4 lines · Jacek Anaszewski
Hi Vaishali,
Quoted 8 lines
Quoted 2 lines
Thanks for the patch. Unfortunately it causes build break. Please fix the issues and resubmit.
Oops! I am really very sorry. By mistake I sent a basic version of patch instead of final one. Thanks for the review.
I'll send v2 with the fix of that build error.
Quoted 8 lines
Hi Jacek,
Can you please tell me what are those build errors? After looking at my patch, I can expect that as I forgot to
remove '.remove = fsg_led_remove' from structure,
it can cause an error. Also, there is one typing mistake in function name. So, both of these can be a reason for breaking a build.
Those was the errors I experienced too.
Quoted 5 lines
Also to be on safe side, I tried to cross compile it for arm architecture. But to my surprise, it is not compiling properly. So, it would be good if you can point me to the script you used for compilation so that I can check it before sending a new version.
Quoted 7 lines
I am using arm-linux-gnueabi cross toolchain.
config: arch/arm/configs/ixp4xx_defconfig
Build command:
make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- all
Ok. Thanks for the information. I was using
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross.
Quoted 2 lines
After fixing the errors compilation succeeded. Do you have a hardware to test the modifications against?
Ok. I checked with arm-linux-gnueabi cross toolchain and fixing these 2 errors allow us to compile the file successfully.

Quoted 5 lines

No, I don't have a hardware to test these modifications. I think this change should not lead to any breakage. But yes it is good to test such changes. Can you please test it for me (if you have a hardware), so that I can send v2 with fixing those errors?

I don't have the hardware either. I assumed that you had a personal interest in modifying this driver, and that you might have had the hardware. The modifications are pretty straightforward, compile-testing should suffice. Please submit v2.

Signature · 2 lines

Best Regards,
Jacek Anaszewski

Vaishali Thakkar

author

2015-08-18 13:41 UTC #7 lore raw

Quoted 83 lines · Jacek Anaszewski

Quoted 76 lines · Vaishali Thakkar
Quoted 52 lines · Jacek Anaszewski
Hi Vaishali,
Quoted 27 lines · Vaishali Thakkar
Quoted 15 lines · Vaishali Thakkar
Quoted 7 lines · Jacek Anaszewski
Hi Vaishali,
Thanks for the patch. Unfortunately it causes build break. Please fix the issues and resubmit.
Quoted 5 lines
Oops! I am really very sorry. By mistake I sent a basic version of patch
instead of final one. Thanks for the review.
I'll send v2 with the fix of that build error.
Quoted 8 lines
Hi Jacek,
Can you please tell me what are those build errors? After looking at my patch, I can expect that as I forgot to
remove '.remove = fsg_led_remove' from structure,
it can cause an error. Also, there is one typing mistake in function name. So, both of these can be a reason for breaking a build.
Those was the errors I experienced too.
Quoted 5 lines
Also to be on safe side, I tried to cross compile it for arm architecture. But to my surprise, it is not compiling properly. So, it would be good if you can point me to the script you used for compilation so that I can check it before sending a new version.
Quoted 7 lines
I am using arm-linux-gnueabi cross toolchain.
config: arch/arm/configs/ixp4xx_defconfig
Build command:
make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- all
Ok. Thanks for the information. I was using
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross.
Quoted 2 lines
After fixing the errors compilation succeeded. Do you have a hardware to test the modifications against?
Ok. I checked with arm-linux-gnueabi cross toolchain and fixing these 2 errors allow us to compile the file successfully.
Quoted 5 lines
No, I don't have a hardware to test these modifications. I think this change should not lead to any breakage. But yes it is good to test such changes. Can you please test it for me (if you have a hardware), so that I can send v2 with fixing those errors?

Quoted 5 lines

I don't have the hardware either. I assumed that you had a personal interest in modifying this driver, and that you might have had the hardware. The modifications are pretty straightforward, compile-testing should suffice. Please submit v2.

I would love to work on this driver but only if someone having hardware for this driver is ready to give it to me for testing. :) For now, I am just working as an Outreachy intern on project Coccinelle and this case came across while working with devm functions.

Anyways, thanks for the review. I'll send a v2.

Quoted 3 lines

--
Best Regards,
Jacek Anaszewski

Signature · 1 line

Vaishali