[PATCH] leds: leds-fsg: Use devm_led_classdev_register

[PATCH] leds: leds-fsg: Use devm_led_classdev_register

[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 <vthakkar1994@gmail.com>
---
 drivers/leds/leds-fsg.c | 51 ++++++++++++-------------------------------------
 1 file changed, 12 insertions(+), 39 deletions(-)

diff --git a/drivers/leds/leds-fsg.c b/drivers/leds/leds-fsg.c
index 2b4dc73..df0e5da 100644
--- a/drivers/leds/leds-fsg.c
+++ b/drivers/leds/leds-fsg.c
@@ -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,
-- 
1.9.1

Re: [PATCH] leds: leds-fsg: Use devm_led_classdev_register

[Jacek Anaszewski]

Hi Vaishali,

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

On 08/15/2015 12:21 PM, Vaishali Thakkar wrote:
> 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 <vthakkar1994@gmail.com>
> ---
>   drivers/leds/leds-fsg.c | 51 ++++++++++++-------------------------------------
>   1 file changed, 12 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/leds/leds-fsg.c b/drivers/leds/leds-fsg.c
> index 2b4dc73..df0e5da 100644
> --- a/drivers/leds/leds-fsg.c
> +++ b/drivers/leds/leds-fsg.c
> @@ -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,
>


-- 
Best Regards,
Jacek Anaszewski

Re: [PATCH] leds: leds-fsg: Use devm_led_classdev_register

[Vaishali Thakkar]

On Mon, Aug 17, 2015 at 2:02 PM, Vaishali Thakkar
<vthakkar1994@gmail.com> wrote:
>
> On 17 Aug 2015 13:57, "Jacek Anaszewski" <j.anaszewski@samsung.com> wrote:
>>
>> Hi Vaishali,
>>
>> 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.

>>
>> On 08/15/2015 12:21 PM, Vaishali Thakkar wrote:
>>>
>>> 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 <vthakkar1994@gmail.com>
>>> ---
>>>   drivers/leds/leds-fsg.c | 51
>>> ++++++++++++-------------------------------------
>>>   1 file changed, 12 insertions(+), 39 deletions(-)
>>>
>>> diff --git a/drivers/leds/leds-fsg.c b/drivers/leds/leds-fsg.c
>>> index 2b4dc73..df0e5da 100644
>>> --- a/drivers/leds/leds-fsg.c
>>> +++ b/drivers/leds/leds-fsg.c
>>> @@ -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,
>>>
>>
>>
>> --
>> Best Regards,
>> Jacek Anaszewski



-- 
Vaishali

Re: [PATCH] leds: leds-fsg: Use devm_led_classdev_register

[Jacek Anaszewski]

Hi Vaishali,

On 08/18/2015 07:46 AM, Vaishali Thakkar wrote:
> On Mon, Aug 17, 2015 at 2:02 PM, Vaishali Thakkar
> <vthakkar1994@gmail.com> wrote:
>>
>> On 17 Aug 2015 13:57, "Jacek Anaszewski" <j.anaszewski@samsung.com> wrote:
>>>
>>> Hi Vaishali,
>>>
>>> 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.

Those was the errors I experienced too.

> 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?

-- 
Best Regards,
Jacek Anaszewski

Re: [PATCH] leds: leds-fsg: Use devm_led_classdev_register

[Vaishali Thakkar]

On Tue, Aug 18, 2015 at 12:57 PM, Jacek Anaszewski
<j.anaszewski@samsung.com> wrote:
> Hi Vaishali,
>
> On 08/18/2015 07:46 AM, Vaishali Thakkar wrote:
>>
>> On Mon, Aug 17, 2015 at 2:02 PM, Vaishali Thakkar
>> <vthakkar1994@gmail.com> wrote:
>>>
>>>
>>> On 17 Aug 2015 13:57, "Jacek Anaszewski" <j.anaszewski@samsung.com>
>>> wrote:
>>>>
>>>>
>>>> Hi Vaishali,
>>>>
>>>> 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.
>
>
> Those was the errors I experienced too.
>
>> 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

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

> 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?

>
> --
> Best Regards,
> Jacek Anaszewski



-- 
Vaishali

Re: [PATCH] leds: leds-fsg: Use devm_led_classdev_register

[Jacek Anaszewski]

On 08/18/2015 02:14 PM, Vaishali Thakkar wrote:
> On Tue, Aug 18, 2015 at 12:57 PM, Jacek Anaszewski
> <j.anaszewski@samsung.com> wrote:
>> Hi Vaishali,
>>
>> On 08/18/2015 07:46 AM, Vaishali Thakkar wrote:
>>>
>>> On Mon, Aug 17, 2015 at 2:02 PM, Vaishali Thakkar
>>> <vthakkar1994@gmail.com> wrote:
>>>>
>>>>
>>>> On 17 Aug 2015 13:57, "Jacek Anaszewski" <j.anaszewski@samsung.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> Hi Vaishali,
>>>>>
>>>>> 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.
>>
>>
>> Those was the errors I experienced too.
>>
>>> 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
>
> Ok. Thanks for the information. I was using
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross.
>
>> 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?

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.

-- 
Best Regards,
Jacek Anaszewski

Re: [PATCH] leds: leds-fsg: Use devm_led_classdev_register

[Vaishali Thakkar]

On Tue, Aug 18, 2015 at 6:06 PM, Jacek Anaszewski
<j.anaszewski@samsung.com> wrote:
> On 08/18/2015 02:14 PM, Vaishali Thakkar wrote:
>>
>> On Tue, Aug 18, 2015 at 12:57 PM, Jacek Anaszewski
>> <j.anaszewski@samsung.com> wrote:
>>>
>>> Hi Vaishali,
>>>
>>> On 08/18/2015 07:46 AM, Vaishali Thakkar wrote:
>>>>
>>>>
>>>> On Mon, Aug 17, 2015 at 2:02 PM, Vaishali Thakkar
>>>> <vthakkar1994@gmail.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 17 Aug 2015 13:57, "Jacek Anaszewski" <j.anaszewski@samsung.com>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Hi Vaishali,
>>>>>>
>>>>>> 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.
>>>
>>>
>>>
>>> Those was the errors I experienced too.
>>>
>>>> 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
>>
>>
>> Ok. Thanks for the information. I was using
>>
>> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross.
>>
>>> 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?
>
> 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.

> --
> Best Regards,
> Jacek Anaszewski



-- 
Vaishali