[PATCH v2 0/2] leds: leds-ipaq-micro: Use resource managed functions and fix coding style.

[PATCH v2 0/2] leds: leds-ipaq-micro: Use resource managed functions and fix coding style.

[Muhammad Falak R Wani]

Hi,
Based on the suggestion from Jaeck, the orginal patch is split into
two, one patch fixes the coding style issues, and the other
uses resource managed function, devm_led_classdev_register.

Muhammad Falak R Wani (2):
  leds: leds-ipaq-micro: Use devm_led_classdev_register
  leds: leds-ipaq-micro: Fix coding style issues

 drivers/leds/leds-ipaq-micro.c | 47 ++++++++++++++++++------------------------
 1 file changed, 20 insertions(+), 27 deletions(-)

-- 
1.9.1

[PATCH v2 1/2] leds: leds-ipaq-micro: Use devm_led_classdev_register

[Muhammad Falak R Wani]

Use of resource-managed function devm_led_classdev_register
instead of led_classdev_register is preferred, consequently
remove redundant function micro_leds_remove.

Signed-off-by: Muhammad Falak R Wani <falakreyaz@gmail.com>
---
 drivers/leds/leds-ipaq-micro.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/leds/leds-ipaq-micro.c b/drivers/leds/leds-ipaq-micro.c
index 3776f51..1206215 100644
--- a/drivers/leds/leds-ipaq-micro.c
+++ b/drivers/leds/leds-ipaq-micro.c
@@ -111,7 +111,7 @@ static int micro_leds_probe(struct platform_device *pdev)
 {
 	int ret;
 
-	ret = led_classdev_register(&pdev->dev, &micro_led);
+	ret = devm_led_classdev_register(&pdev->dev, &micro_led);
 	if (ret) {
 		dev_err(&pdev->dev, "registering led failed: %d\n", ret);
 		return ret;
@@ -121,18 +121,11 @@ static int micro_leds_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int micro_leds_remove(struct platform_device *pdev)
-{
-	led_classdev_unregister(&micro_led);
-	return 0;
-}
-
 static struct platform_driver micro_leds_device_driver = {
 	.driver = {
 		.name    = "ipaq-micro-leds",
 	},
 	.probe   = micro_leds_probe,
-	.remove  = micro_leds_remove,
 };
 module_platform_driver(micro_leds_device_driver);
 
-- 
1.9.1

[PATCH v2 2/2] leds: leds-ipaq-micro: Fix coding style issues

[Muhammad Falak R Wani]

Spaces at the starting of a line are removed, indentation
using tab, instead of space. Also, warnings related to 
line width of more than 80 characters is also taken care of.
Two warnings have been left alone to aid better readability.

Signed-off-by: Muhammad Falak R Wani <falakreyaz@gmail.com>
---
 drivers/leds/leds-ipaq-micro.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/leds/leds-ipaq-micro.c b/drivers/leds/leds-ipaq-micro.c
index 1206215..86716ea 100644
--- a/drivers/leds/leds-ipaq-micro.c
+++ b/drivers/leds/leds-ipaq-micro.c
@@ -16,9 +16,9 @@
 #define LED_YELLOW	0x00
 #define LED_GREEN	0x01
 
-#define LED_EN          (1 << 4)        /* LED ON/OFF 0:off, 1:on                       */
-#define LED_AUTOSTOP    (1 << 5)        /* LED ON/OFF auto stop set 0:disable, 1:enable */
-#define LED_ALWAYS      (1 << 6)        /* LED Interrupt Mask 0:No mask, 1:mask         */
+#define LED_EN	      (1 << 4) /* LED ON/OFF 0:off, 1:on	      */
+#define LED_AUTOSTOP  (1 << 5) /* LED ON/OFF auto stop set 0:disable,1:enable*/
+#define LED_ALWAYS    (1 << 6) /* LED Interrupt Mask 0:No mask, 1:mask	 */
 
 static void micro_leds_brightness_set(struct led_classdev *led_cdev,
 				      enum led_brightness value)
@@ -27,14 +27,14 @@ static void micro_leds_brightness_set(struct led_classdev *led_cdev,
 	/*
 	 * In this message:
 	 * Byte 0 = LED color: 0 = yellow, 1 = green
-	 *          yellow LED is always ~30 blinks per minute
+	 *	  yellow LED is always ~30 blinks per minute
 	 * Byte 1 = duration (flags?) appears to be ignored
 	 * Byte 2 = green ontime in 1/10 sec (deciseconds)
-	 *          1 = 1/10 second
-	 *          0 = 256/10 second
+	 *	  1 = 1/10 second
+	 *	  0 = 256/10 second
 	 * Byte 3 = green offtime in 1/10 sec (deciseconds)
-	 *          1 = 1/10 second
-	 *          0 = 256/10 seconds
+	 *	  1 = 1/10 second
+	 *	  0 = 256/10 seconds
 	 */
 	struct ipaq_micro_msg msg = {
 		.id = MSG_NOTIFY_LED,
@@ -64,14 +64,14 @@ static int micro_leds_blink_set(struct led_classdev *led_cdev,
 	/*
 	 * In this message:
 	 * Byte 0 = LED color: 0 = yellow, 1 = green
-	 *          yellow LED is always ~30 blinks per minute
+	 *	  yellow LED is always ~30 blinks per minute
 	 * Byte 1 = duration (flags?) appears to be ignored
 	 * Byte 2 = green ontime in 1/10 sec (deciseconds)
-	 *          1 = 1/10 second
-	 *          0 = 256/10 second
+	 *	  1 = 1/10 second
+	 *	  0 = 256/10 second
 	 * Byte 3 = green offtime in 1/10 sec (deciseconds)
-	 *          1 = 1/10 second
-	 *          0 = 256/10 seconds
+	 *	  1 = 1/10 second
+	 *	  0 = 256/10 seconds
 	 */
 	struct ipaq_micro_msg msg = {
 		.id = MSG_NOTIFY_LED,
@@ -79,14 +79,14 @@ static int micro_leds_blink_set(struct led_classdev *led_cdev,
 	};
 
 	msg.tx_data[0] = LED_GREEN;
-        if (*delay_on > IPAQ_LED_MAX_DUTY ||
+	if (*delay_on > IPAQ_LED_MAX_DUTY ||
 	    *delay_off > IPAQ_LED_MAX_DUTY)
-                return -EINVAL;
+		return -EINVAL;
 
-        if (*delay_on == 0 && *delay_off == 0) {
-                *delay_on = 100;
-                *delay_off = 100;
-        }
+	if (*delay_on == 0 && *delay_off == 0) {
+		*delay_on = 100;
+		*delay_off = 100;
+	}
 
 	msg.tx_data[1] = 0;
 	if (*delay_on >= IPAQ_LED_MAX_DUTY)
-- 
1.9.1

Re: [PATCH v2 2/2] leds: leds-ipaq-micro: Fix coding style issues

[Jacek Anaszewski]

Hi Muhammad,

On 09/07/2015 04:13 PM, Muhammad Falak R Wani wrote:
> Spaces at the starting of a line are removed, indentation
> using tab, instead of space. Also, warnings related to
> line width of more than 80 characters is also taken care of.
> Two warnings have been left alone to aid better readability.
>
> Signed-off-by: Muhammad Falak R Wani <falakreyaz@gmail.com>
> ---
>   drivers/leds/leds-ipaq-micro.c | 38 +++++++++++++++++++-------------------
>   1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/leds/leds-ipaq-micro.c b/drivers/leds/leds-ipaq-micro.c
> index 1206215..86716ea 100644
> --- a/drivers/leds/leds-ipaq-micro.c
> +++ b/drivers/leds/leds-ipaq-micro.c
> @@ -16,9 +16,9 @@
>   #define LED_YELLOW	0x00
>   #define LED_GREEN	0x01
>
> -#define LED_EN          (1 << 4)        /* LED ON/OFF 0:off, 1:on                       */
> -#define LED_AUTOSTOP    (1 << 5)        /* LED ON/OFF auto stop set 0:disable, 1:enable */
> -#define LED_ALWAYS      (1 << 6)        /* LED Interrupt Mask 0:No mask, 1:mask         */
> +#define LED_EN	      (1 << 4) /* LED ON/OFF 0:off, 1:on	      */
> +#define LED_AUTOSTOP  (1 << 5) /* LED ON/OFF auto stop set 0:disable,1:enable*/
> +#define LED_ALWAYS    (1 << 6) /* LED Interrupt Mask 0:No mask, 1:mask	 */

Please keep comments ending in the same column.

>
>   static void micro_leds_brightness_set(struct led_classdev *led_cdev,
>   				      enum led_brightness value)
> @@ -27,14 +27,14 @@ static void micro_leds_brightness_set(struct led_classdev *led_cdev,
>   	/*
>   	 * In this message:
>   	 * Byte 0 = LED color: 0 = yellow, 1 = green
> -	 *          yellow LED is always ~30 blinks per minute
> +	 *	  yellow LED is always ~30 blinks per minute
>   	 * Byte 1 = duration (flags?) appears to be ignored
>   	 * Byte 2 = green ontime in 1/10 sec (deciseconds)
> -	 *          1 = 1/10 second
> -	 *          0 = 256/10 second
> +	 *	  1 = 1/10 second
> +	 *	  0 = 256/10 second
>   	 * Byte 3 = green offtime in 1/10 sec (deciseconds)
> -	 *          1 = 1/10 second
> -	 *          0 = 256/10 seconds
> +	 *	  1 = 1/10 second
> +	 *	  0 = 256/10 seconds
>   	 */
>   	struct ipaq_micro_msg msg = {
>   		.id = MSG_NOTIFY_LED,
> @@ -64,14 +64,14 @@ static int micro_leds_blink_set(struct led_classdev *led_cdev,
>   	/*
>   	 * In this message:
>   	 * Byte 0 = LED color: 0 = yellow, 1 = green
> -	 *          yellow LED is always ~30 blinks per minute
> +	 *	  yellow LED is always ~30 blinks per minute
>   	 * Byte 1 = duration (flags?) appears to be ignored
>   	 * Byte 2 = green ontime in 1/10 sec (deciseconds)
> -	 *          1 = 1/10 second
> -	 *          0 = 256/10 second
> +	 *	  1 = 1/10 second
> +	 *	  0 = 256/10 second
>   	 * Byte 3 = green offtime in 1/10 sec (deciseconds)
> -	 *          1 = 1/10 second
> -	 *          0 = 256/10 seconds
> +	 *	  1 = 1/10 second
> +	 *	  0 = 256/10 seconds
>   	 */

This looks worse after applying the patch. Why actually did you change
it? AFAICS checkpatch.pl doesn't complain here.

>   	struct ipaq_micro_msg msg = {
>   		.id = MSG_NOTIFY_LED,
> @@ -79,14 +79,14 @@ static int micro_leds_blink_set(struct led_classdev *led_cdev,
>   	};
>
>   	msg.tx_data[0] = LED_GREEN;
> -        if (*delay_on > IPAQ_LED_MAX_DUTY ||
> +	if (*delay_on > IPAQ_LED_MAX_DUTY ||
>   	    *delay_off > IPAQ_LED_MAX_DUTY)
> -                return -EINVAL;
> +		return -EINVAL;
>
> -        if (*delay_on == 0 && *delay_off == 0) {
> -                *delay_on = 100;
> -                *delay_off = 100;
> -        }
> +	if (*delay_on == 0 && *delay_off == 0) {
> +		*delay_on = 100;
> +		*delay_off = 100;
> +	}
>
>   	msg.tx_data[1] = 0;
>   	if (*delay_on >= IPAQ_LED_MAX_DUTY)
>


-- 
Best Regards,
Jacek Anaszewski

Re: [PATCH v2 1/2] leds: leds-ipaq-micro: Use devm_led_classdev_register

[Jacek Anaszewski]

On 09/07/2015 04:13 PM, Muhammad Falak R Wani wrote:
> Use of resource-managed function devm_led_classdev_register
> instead of led_classdev_register is preferred, consequently
> remove redundant function micro_leds_remove.
>
> Signed-off-by: Muhammad Falak R Wani <falakreyaz@gmail.com>
> ---
>   drivers/leds/leds-ipaq-micro.c | 9 +--------
>   1 file changed, 1 insertion(+), 8 deletions(-)

Merged, thanks.

-- 
Best Regards,
Jacek Anaszewski