[PATCH 1/7] leds-bcm6328: code cleaning

[PATCH 1/7] leds-bcm6328: code cleaning

[Álvaro Fernández Rojas]

- Remove double whitespace
- Add missing brackets
- Add spinlock info

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/leds/leds-bcm6328.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
index 0329dee..1ef69c2 100644
--- a/drivers/leds/leds-bcm6328.c
+++ b/drivers/leds/leds-bcm6328.c
@@ -42,7 +42,7 @@
 #define BCM6328_LED_SHIFT_TEST		BIT(30)
 #define BCM6328_LED_TEST		BIT(31)
 #define BCM6328_INIT_MASK		(BCM6328_SERIAL_LED_EN | \
-					 BCM6328_SERIAL_LED_MUX  | \
+					 BCM6328_SERIAL_LED_MUX | \
 					 BCM6328_SERIAL_LED_CLK_NPOL | \
 					 BCM6328_SERIAL_LED_DATA_PPOL | \
 					 BCM6328_SERIAL_LED_SHIFT_DIR)
@@ -151,9 +151,9 @@ static int bcm6328_blink_set(struct led_classdev *led_cdev,
 	}
 
 	delay = *delay_on / BCM6328_LED_INTERVAL_MS;
-	if (delay == 0)
+	if (delay == 0) {
 		delay = 1;
-	else if (delay > BCM6328_LED_INTV_MASK) {
+	} else if (delay > BCM6328_LED_INTV_MASK) {
 		dev_dbg(led_cdev->dev,
 			"fallback to soft blinking (delay > %ums)\n",
 			BCM6328_LED_INTV_MASK * BCM6328_LED_INTERVAL_MS);
@@ -337,7 +337,7 @@ static int bcm6328_leds_probe(struct platform_device *pdev)
 	struct device_node *child;
 	struct resource *mem_r;
 	void __iomem *mem;
-	spinlock_t *lock;
+	spinlock_t *lock; /* memory lock */
 	unsigned long val, *blink_leds, *blink_delay;
 
 	mem_r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-- 
1.9.1

[PATCH 2/7] leds-bcm6328: remove unneded lock when checking initial LED status

[Álvaro Fernández Rojas]

This lock isn't really needed, since we're only reading the register and
changes made to other LEDs aren't relevant.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/leds/leds-bcm6328.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
index 1ef69c2..cd54264 100644
--- a/drivers/leds/leds-bcm6328.c
+++ b/drivers/leds/leds-bcm6328.c
@@ -264,7 +264,6 @@ static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
 		       unsigned long *blink_leds, unsigned long *blink_delay)
 {
 	struct bcm6328_led *led;
-	unsigned long flags;
 	const char *state;
 	int rc;
 
@@ -286,7 +285,6 @@ static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
 						    "linux,default-trigger",
 						    NULL);
 
-	spin_lock_irqsave(lock, flags);
 	if (!of_property_read_string(nc, "default-state", &state)) {
 		if (!strcmp(state, "on")) {
 			led->cdev.brightness = LED_FULL;
@@ -314,7 +312,6 @@ static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
 	} else {
 		led->cdev.brightness = LED_OFF;
 	}
-	spin_unlock_irqrestore(lock, flags);
 
 	bcm6328_led_set(&led->cdev, led->cdev.brightness);
 
-- 
1.9.1

[PATCH 3/7] leds-bcm6358: Use bcm6358_led_set() in order to get rid of the lock

[Álvaro Fernández Rojas]

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/leds/leds-bcm6358.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/leds-bcm6358.c b/drivers/leds/leds-bcm6358.c
index 82b4ee1..a98cc71 100644
--- a/drivers/leds/leds-bcm6358.c
+++ b/drivers/leds/leds-bcm6358.c
@@ -141,9 +141,10 @@ static int bcm6358_led(struct device *dev, struct device_node *nc, u32 reg,
 	} else {
 		led->cdev.brightness = LED_OFF;
 	}
-	bcm6358_led_mode(led, led->cdev.brightness);
 	spin_unlock_irqrestore(lock, flags);
 
+	bcm6358_led_set(&led->cdev, led->cdev.brightness);
+
 	led->cdev.brightness_set = bcm6358_led_set;
 
 	rc = led_classdev_register(dev, &led->cdev);
-- 
1.9.1

[PATCH 4/7] leds-bcm6358: remove unneded lock when checking initial LED status

[Álvaro Fernández Rojas]

This lock isn't really needed, since we're only reading the register and
changes made to other LEDs aren't relevant.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/leds/leds-bcm6358.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/leds/leds-bcm6358.c b/drivers/leds/leds-bcm6358.c
index a98cc71..ab09ea4 100644
--- a/drivers/leds/leds-bcm6358.c
+++ b/drivers/leds/leds-bcm6358.c
@@ -99,7 +99,6 @@ static int bcm6358_led(struct device *dev, struct device_node *nc, u32 reg,
 		       void __iomem *mem, spinlock_t *lock)
 {
 	struct bcm6358_led *led;
-	unsigned long flags;
 	const char *state;
 	int rc;
 
@@ -119,7 +118,6 @@ static int bcm6358_led(struct device *dev, struct device_node *nc, u32 reg,
 						    "linux,default-trigger",
 						    NULL);
 
-	spin_lock_irqsave(lock, flags);
 	if (!of_property_read_string(nc, "default-state", &state)) {
 		if (!strcmp(state, "on")) {
 			led->cdev.brightness = LED_FULL;
@@ -141,7 +139,6 @@ static int bcm6358_led(struct device *dev, struct device_node *nc, u32 reg,
 	} else {
 		led->cdev.brightness = LED_OFF;
 	}
-	spin_unlock_irqrestore(lock, flags);
 
 	bcm6358_led_set(&led->cdev, led->cdev.brightness);
 
-- 
1.9.1

[PATCH 5/7] leds-bcm6328: switch to __raw_readl() and __raw_writel()

[Álvaro Fernández Rojas]

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/leds/leds-bcm6328.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
index cd54264..876f0ce 100644
--- a/drivers/leds/leds-bcm6328.c
+++ b/drivers/leds/leds-bcm6328.c
@@ -76,12 +76,12 @@ struct bcm6328_led {
 
 static void bcm6328_led_write(void __iomem *reg, unsigned long data)
 {
-	iowrite32be(data, reg);
+	__raw_writel(data, reg);
 }
 
 static unsigned long bcm6328_led_read(void __iomem *reg)
 {
-	return ioread32be(reg);
+	return __raw_readl(reg);
 }
 
 /**
-- 
1.9.1

[PATCH 6/7] leds-bcm6358: switch to __raw_readl() and __raw_writel()

[Álvaro Fernández Rojas]

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/leds/leds-bcm6358.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-bcm6358.c b/drivers/leds/leds-bcm6358.c
index ab09ea4..ae32f94 100644
--- a/drivers/leds/leds-bcm6358.c
+++ b/drivers/leds/leds-bcm6358.c
@@ -49,12 +49,12 @@ struct bcm6358_led {
 
 static void bcm6358_led_write(void __iomem *reg, unsigned long data)
 {
-	iowrite32be(data, reg);
+	__raw_writel(data, reg);
 }
 
 static unsigned long bcm6358_led_read(void __iomem *reg)
 {
-	return ioread32be(reg);
+	return __raw_readl(reg);
 }
 
 static unsigned long bcm6358_led_busy(void __iomem *mem)
-- 
1.9.1

[PATCH 7/7] leds-bcm6328: simplify duplicated unlock in bcm6328_blink_set

[Álvaro Fernández Rojas]

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/leds/leds-bcm6328.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
index 876f0ce..a361829 100644
--- a/drivers/leds/leds-bcm6328.c
+++ b/drivers/leds/leds-bcm6328.c
@@ -138,6 +138,7 @@ static int bcm6328_blink_set(struct led_classdev *led_cdev,
 	struct bcm6328_led *led =
 		container_of(led_cdev, struct bcm6328_led, cdev);
 	unsigned long delay, flags;
+	int rc;
 
 	if (!*delay_on)
 		*delay_on = BCM6328_LED_DEF_DELAY;
@@ -175,16 +176,15 @@ static int bcm6328_blink_set(struct led_classdev *led_cdev,
 		bcm6328_led_write(led->mem + BCM6328_REG_INIT, val);
 
 		bcm6328_led_mode(led, BCM6328_LED_MODE_BLINK);
-
-		spin_unlock_irqrestore(led->lock, flags);
+		rc = 0;
 	} else {
-		spin_unlock_irqrestore(led->lock, flags);
 		dev_dbg(led_cdev->dev,
 			"fallback to soft blinking (delay already set)\n");
-		return -EINVAL;
+		rc = -EINVAL;
 	}
+	spin_unlock_irqrestore(led->lock, flags);
 
-	return 0;
+	return rc;
 }
 
 static int bcm6328_hwled(struct device *dev, struct device_node *nc, u32 reg,
-- 
1.9.1

Re: [PATCH 5/7] leds-bcm6328: switch to __raw_readl() and __raw_writel()

[Simon Arlott]

On 29/11/15 16:35, Álvaro Fernández Rojas wrote:
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  drivers/leds/leds-bcm6328.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
> index cd54264..876f0ce 100644
> --- a/drivers/leds/leds-bcm6328.c
> +++ b/drivers/leds/leds-bcm6328.c
> @@ -76,12 +76,12 @@ struct bcm6328_led {
>  
>  static void bcm6328_led_write(void __iomem *reg, unsigned long data)
>  {
> -	iowrite32be(data, reg);
> +	__raw_writel(data, reg);
>  }
>  
>  static unsigned long bcm6328_led_read(void __iomem *reg)
>  {
> -	return ioread32be(reg);
> +	return __raw_readl(reg);
>  }
>  
>  /**
> 

If the wrapper functions don't add anything, would it not be better to
just remove them completely?

-- 
Simon Arlott

Re: [PATCH 5/7] leds-bcm6328: switch to __raw_readl() and __raw_writel()

[Florian Fainelli]

Le 29/11/2015 08:35, Álvaro Fernández Rojas a écrit :
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

This driver has a slight chance of running on a BCM63138 SoC which is
ARM-based and little endian, so I would keep the wrapper around here
just so it is easy for somebody to add support for a different platform
later on.

> ---
>  drivers/leds/leds-bcm6328.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
> index cd54264..876f0ce 100644
> --- a/drivers/leds/leds-bcm6328.c
> +++ b/drivers/leds/leds-bcm6328.c
> @@ -76,12 +76,12 @@ struct bcm6328_led {
>  
>  static void bcm6328_led_write(void __iomem *reg, unsigned long data)
>  {
> -	iowrite32be(data, reg);
> +	__raw_writel(data, reg);
>  }
>  
>  static unsigned long bcm6328_led_read(void __iomem *reg)
>  {
> -	return ioread32be(reg);
> +	return __raw_readl(reg);
>  }
>  
>  /**
> 

Re: [PATCH 5/7] leds-bcm6328: switch to __raw_readl() and __raw_writel()

[Jacek Anaszewski]

Hi Florian, Alvaro

On 11/30/2015 05:32 AM, Florian Fainelli wrote:
> Le 29/11/2015 08:35, Álvaro Fernández Rojas a écrit :
>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
>
> This driver has a slight chance of running on a BCM63138 SoC which is
> ARM-based and little endian, so I would keep the wrapper around here
> just so it is easy for somebody to add support for a different platform
> later on.

Thanks for sharing this info.

Alvaro - is the performance gain resulting from this change
specifically important for some of your use cases?

>> ---
>>   drivers/leds/leds-bcm6328.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
>> index cd54264..876f0ce 100644
>> --- a/drivers/leds/leds-bcm6328.c
>> +++ b/drivers/leds/leds-bcm6328.c
>> @@ -76,12 +76,12 @@ struct bcm6328_led {
>>
>>   static void bcm6328_led_write(void __iomem *reg, unsigned long data)
>>   {
>> -	iowrite32be(data, reg);
>> +	__raw_writel(data, reg);
>>   }
>>
>>   static unsigned long bcm6328_led_read(void __iomem *reg)
>>   {
>> -	return ioread32be(reg);
>> +	return __raw_readl(reg);
>>   }
>>
>>   /**
>>
>
>


-- 
Best Regards,
Jacek Anaszewski

Re: [PATCH 1/7] leds-bcm6328: code cleaning

[Jacek Anaszewski]

Hi Alvaro,

Thanks for the patches. Please use "leds: bcm6328"
(or leds: bcm6358) prefix in the commit title. Please also add commit
description to the patches that are missing it.

On 11/29/2015 05:35 PM, Álvaro Fernández Rojas wrote:
> - Remove double whitespace
> - Add missing brackets
> - Add spinlock info
>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>   drivers/leds/leds-bcm6328.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
> index 0329dee..1ef69c2 100644
> --- a/drivers/leds/leds-bcm6328.c
> +++ b/drivers/leds/leds-bcm6328.c
> @@ -42,7 +42,7 @@
>   #define BCM6328_LED_SHIFT_TEST		BIT(30)
>   #define BCM6328_LED_TEST		BIT(31)
>   #define BCM6328_INIT_MASK		(BCM6328_SERIAL_LED_EN | \
> -					 BCM6328_SERIAL_LED_MUX  | \
> +					 BCM6328_SERIAL_LED_MUX | \
>   					 BCM6328_SERIAL_LED_CLK_NPOL | \
>   					 BCM6328_SERIAL_LED_DATA_PPOL | \
>   					 BCM6328_SERIAL_LED_SHIFT_DIR)
> @@ -151,9 +151,9 @@ static int bcm6328_blink_set(struct led_classdev *led_cdev,
>   	}
>
>   	delay = *delay_on / BCM6328_LED_INTERVAL_MS;
> -	if (delay == 0)
> +	if (delay == 0) {
>   		delay = 1;
> -	else if (delay > BCM6328_LED_INTV_MASK) {
> +	} else if (delay > BCM6328_LED_INTV_MASK) {
>   		dev_dbg(led_cdev->dev,
>   			"fallback to soft blinking (delay > %ums)\n",
>   			BCM6328_LED_INTV_MASK * BCM6328_LED_INTERVAL_MS);
> @@ -337,7 +337,7 @@ static int bcm6328_leds_probe(struct platform_device *pdev)
>   	struct device_node *child;
>   	struct resource *mem_r;
>   	void __iomem *mem;
> -	spinlock_t *lock;
> +	spinlock_t *lock; /* memory lock */
>   	unsigned long val, *blink_leds, *blink_delay;
>
>   	mem_r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>


-- 
Best Regards,
Jacek Anaszewski

[PATCH v2 0/8] leds: bcm6328/bcm6358: several fixes and improvements

[Álvaro Fernández Rojas]

These patches add several fixes and improvements for leds-bcm6328 and
leds-bcm6358.

v2: introduce changes suggested by Jacek and Florian
 - change "leds-bcm6328:" to "leds: bcm6328:"
 - change "leds-bcm6358:" to "leds: bcm6358:"
 - add little endian support
 - bcm6358: Merge bcm6358_led_mode into bcm6358_led_set

Álvaro Fernández Rojas (8):
  leds: bcm6328: code cleaning
  leds: bcm6328: remove unneded lock when checking initial LED
  leds: bcm6358: Use bcm6358_led_set() in order to get rid of the lock
  leds: bcm6358: remove unneded lock when checking initial LED status
  leds: bcm6328: add little endian support
  leds: bcm6358: add little endian support
  leds: bcm6328: simplify duplicated unlock in bcm6328_blink_set
  leds: bcm6358: merge bcm6358_led_mode and bcm6358_led_set

 drivers/leds/leds-bcm6328.c | 29 +++++++++++++++++------------
 drivers/leds/leds-bcm6358.c | 34 ++++++++++++++++------------------
 2 files changed, 33 insertions(+), 30 deletions(-)

-- 
1.9.1

[PATCH v2 1/8] leds: bcm6328: code cleaning

[Álvaro Fernández Rojas]

- Remove double whitespace
- Add missing brackets
- Add spinlock info

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 v2: no changes

 drivers/leds/leds-bcm6328.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
index 0329dee..1ef69c2 100644
--- a/drivers/leds/leds-bcm6328.c
+++ b/drivers/leds/leds-bcm6328.c
@@ -42,7 +42,7 @@
 #define BCM6328_LED_SHIFT_TEST		BIT(30)
 #define BCM6328_LED_TEST		BIT(31)
 #define BCM6328_INIT_MASK		(BCM6328_SERIAL_LED_EN | \
-					 BCM6328_SERIAL_LED_MUX  | \
+					 BCM6328_SERIAL_LED_MUX | \
 					 BCM6328_SERIAL_LED_CLK_NPOL | \
 					 BCM6328_SERIAL_LED_DATA_PPOL | \
 					 BCM6328_SERIAL_LED_SHIFT_DIR)
@@ -151,9 +151,9 @@ static int bcm6328_blink_set(struct led_classdev *led_cdev,
 	}
 
 	delay = *delay_on / BCM6328_LED_INTERVAL_MS;
-	if (delay == 0)
+	if (delay == 0) {
 		delay = 1;
-	else if (delay > BCM6328_LED_INTV_MASK) {
+	} else if (delay > BCM6328_LED_INTV_MASK) {
 		dev_dbg(led_cdev->dev,
 			"fallback to soft blinking (delay > %ums)\n",
 			BCM6328_LED_INTV_MASK * BCM6328_LED_INTERVAL_MS);
@@ -337,7 +337,7 @@ static int bcm6328_leds_probe(struct platform_device *pdev)
 	struct device_node *child;
 	struct resource *mem_r;
 	void __iomem *mem;
-	spinlock_t *lock;
+	spinlock_t *lock; /* memory lock */
 	unsigned long val, *blink_leds, *blink_delay;
 
 	mem_r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-- 
1.9.1

[PATCH v2 2/8] leds: bcm6328: remove unneded lock when checking initial LED

[Álvaro Fernández Rojas]

This lock isn't really needed, since we're only reading the register and
changes made to other LEDs aren't relevant.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 v2: no changes

 drivers/leds/leds-bcm6328.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
index 1ef69c2..cd54264 100644
--- a/drivers/leds/leds-bcm6328.c
+++ b/drivers/leds/leds-bcm6328.c
@@ -264,7 +264,6 @@ static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
 		       unsigned long *blink_leds, unsigned long *blink_delay)
 {
 	struct bcm6328_led *led;
-	unsigned long flags;
 	const char *state;
 	int rc;
 
@@ -286,7 +285,6 @@ static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
 						    "linux,default-trigger",
 						    NULL);
 
-	spin_lock_irqsave(lock, flags);
 	if (!of_property_read_string(nc, "default-state", &state)) {
 		if (!strcmp(state, "on")) {
 			led->cdev.brightness = LED_FULL;
@@ -314,7 +312,6 @@ static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
 	} else {
 		led->cdev.brightness = LED_OFF;
 	}
-	spin_unlock_irqrestore(lock, flags);
 
 	bcm6328_led_set(&led->cdev, led->cdev.brightness);
 
-- 
1.9.1

[PATCH v2 3/8] leds: bcm6358: Use bcm6358_led_set() in order to get rid of the lock

[Álvaro Fernández Rojas]

Replaces bcm6358_led_mode with bcm6358_led_set so we don't need to handle
the lock.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 v2: no changes

 drivers/leds/leds-bcm6358.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/leds-bcm6358.c b/drivers/leds/leds-bcm6358.c
index 82b4ee1..a98cc71 100644
--- a/drivers/leds/leds-bcm6358.c
+++ b/drivers/leds/leds-bcm6358.c
@@ -141,9 +141,10 @@ static int bcm6358_led(struct device *dev, struct device_node *nc, u32 reg,
 	} else {
 		led->cdev.brightness = LED_OFF;
 	}
-	bcm6358_led_mode(led, led->cdev.brightness);
 	spin_unlock_irqrestore(lock, flags);
 
+	bcm6358_led_set(&led->cdev, led->cdev.brightness);
+
 	led->cdev.brightness_set = bcm6358_led_set;
 
 	rc = led_classdev_register(dev, &led->cdev);
-- 
1.9.1

[PATCH v2 4/8] leds: bcm6358: remove unneded lock when checking initial LED status

[Álvaro Fernández Rojas]

This lock isn't really needed, since we're only reading the register and
changes made to other LEDs aren't relevant.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 v2: no changes

 drivers/leds/leds-bcm6358.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/leds/leds-bcm6358.c b/drivers/leds/leds-bcm6358.c
index a98cc71..ab09ea4 100644
--- a/drivers/leds/leds-bcm6358.c
+++ b/drivers/leds/leds-bcm6358.c
@@ -99,7 +99,6 @@ static int bcm6358_led(struct device *dev, struct device_node *nc, u32 reg,
 		       void __iomem *mem, spinlock_t *lock)
 {
 	struct bcm6358_led *led;
-	unsigned long flags;
 	const char *state;
 	int rc;
 
@@ -119,7 +118,6 @@ static int bcm6358_led(struct device *dev, struct device_node *nc, u32 reg,
 						    "linux,default-trigger",
 						    NULL);
 
-	spin_lock_irqsave(lock, flags);
 	if (!of_property_read_string(nc, "default-state", &state)) {
 		if (!strcmp(state, "on")) {
 			led->cdev.brightness = LED_FULL;
@@ -141,7 +139,6 @@ static int bcm6358_led(struct device *dev, struct device_node *nc, u32 reg,
 	} else {
 		led->cdev.brightness = LED_OFF;
 	}
-	spin_unlock_irqrestore(lock, flags);
 
 	bcm6358_led_set(&led->cdev, led->cdev.brightness);
 
-- 
1.9.1

[PATCH v2 5/8] leds: bcm6328: add little endian support

[Álvaro Fernández Rojas]

This adds little endian support while keeping big endian support.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 v2: add little endian support instead of switch to __raw io

 drivers/leds/leds-bcm6328.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
index cd54264..e13daa2 100644
--- a/drivers/leds/leds-bcm6328.c
+++ b/drivers/leds/leds-bcm6328.c
@@ -76,12 +76,20 @@ struct bcm6328_led {
 
 static void bcm6328_led_write(void __iomem *reg, unsigned long data)
 {
+#ifdef CONFIG_CPU_BIG_ENDIAN
 	iowrite32be(data, reg);
+#else
+	writel(data, reg);
+#endif
 }
 
 static unsigned long bcm6328_led_read(void __iomem *reg)
 {
+#ifdef CONFIG_CPU_BIG_ENDIAN
 	return ioread32be(reg);
+#else
+	return readl(reg);
+#endif
 }
 
 /**
-- 
1.9.1

[PATCH v2 6/8] leds: bcm6358: add little endian support

[Álvaro Fernández Rojas]

This adds little endian support while keeping big endian support.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 v2: add little endian support instead of switch to __raw io

 drivers/leds/leds-bcm6358.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/leds/leds-bcm6358.c b/drivers/leds/leds-bcm6358.c
index ab09ea4..ffc978d 100644
--- a/drivers/leds/leds-bcm6358.c
+++ b/drivers/leds/leds-bcm6358.c
@@ -49,12 +49,20 @@ struct bcm6358_led {
 
 static void bcm6358_led_write(void __iomem *reg, unsigned long data)
 {
+#ifdef CONFIG_CPU_BIG_ENDIAN
 	iowrite32be(data, reg);
+#else
+	writel(data, reg);
+#endif
 }
 
 static unsigned long bcm6358_led_read(void __iomem *reg)
 {
+#ifdef CONFIG_CPU_BIG_ENDIAN
 	return ioread32be(reg);
+#else
+	return readl(reg);
+#endif
 }
 
 static unsigned long bcm6358_led_busy(void __iomem *mem)
-- 
1.9.1

[PATCH v2 7/8] leds: bcm6328: simplify duplicated unlock in bcm6328_blink_set

[Álvaro Fernández Rojas]

Instead of duplicating the unlock we can store the return value and use a
single unlock before returning that value, which simplifies the code.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 v2: no changes

 drivers/leds/leds-bcm6328.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
index e13daa2..47f7c9f 100644
--- a/drivers/leds/leds-bcm6328.c
+++ b/drivers/leds/leds-bcm6328.c
@@ -146,6 +146,7 @@ static int bcm6328_blink_set(struct led_classdev *led_cdev,
 	struct bcm6328_led *led =
 		container_of(led_cdev, struct bcm6328_led, cdev);
 	unsigned long delay, flags;
+	int rc;
 
 	if (!*delay_on)
 		*delay_on = BCM6328_LED_DEF_DELAY;
@@ -183,16 +184,15 @@ static int bcm6328_blink_set(struct led_classdev *led_cdev,
 		bcm6328_led_write(led->mem + BCM6328_REG_INIT, val);
 
 		bcm6328_led_mode(led, BCM6328_LED_MODE_BLINK);
-
-		spin_unlock_irqrestore(led->lock, flags);
+		rc = 0;
 	} else {
-		spin_unlock_irqrestore(led->lock, flags);
 		dev_dbg(led_cdev->dev,
 			"fallback to soft blinking (delay already set)\n");
-		return -EINVAL;
+		rc = -EINVAL;
 	}
+	spin_unlock_irqrestore(led->lock, flags);
 
-	return 0;
+	return rc;
 }
 
 static int bcm6328_hwled(struct device *dev, struct device_node *nc, u32 reg,
-- 
1.9.1

[PATCH v2 8/8] leds: bcm6358: merge bcm6358_led_mode and bcm6358_led_set

[Álvaro Fernández Rojas]

bcm6358_led_mode is now only called by bcm6358_led_set, so merge both in one
single function.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 v2: introduce new patch to merge bcm6358_led_mode into bcm6358_led_set

 drivers/leds/leds-bcm6358.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/leds/leds-bcm6358.c b/drivers/leds/leds-bcm6358.c
index ffc978d..7413e3e 100644
--- a/drivers/leds/leds-bcm6358.c
+++ b/drivers/leds/leds-bcm6358.c
@@ -76,12 +76,15 @@ static unsigned long bcm6358_led_busy(void __iomem *mem)
 	return val;
 }
 
-static void bcm6358_led_mode(struct bcm6358_led *led, unsigned long value)
+static void bcm6358_led_set(struct led_classdev *led_cdev,
+			    enum led_brightness value)
 {
-	unsigned long val;
+	struct bcm6358_led *led =
+		container_of(led_cdev, struct bcm6358_led, cdev);
+	unsigned long flags, val;
 
+	spin_lock_irqsave(led->lock, flags);
 	bcm6358_led_busy(led->mem);
-
 	val = bcm6358_led_read(led->mem + BCM6358_REG_MODE);
 	if ((led->active_low && value == LED_OFF) ||
 	    (!led->active_low && value != LED_OFF))
@@ -89,17 +92,6 @@ static void bcm6358_led_mode(struct bcm6358_led *led, unsigned long value)
 	else
 		val &= ~(BIT(led->pin));
 	bcm6358_led_write(led->mem + BCM6358_REG_MODE, val);
-}
-
-static void bcm6358_led_set(struct led_classdev *led_cdev,
-			    enum led_brightness value)
-{
-	struct bcm6358_led *led =
-		container_of(led_cdev, struct bcm6358_led, cdev);
-	unsigned long flags;
-
-	spin_lock_irqsave(led->lock, flags);
-	bcm6358_led_mode(led, value);
 	spin_unlock_irqrestore(led->lock, flags);
 }
 
-- 
1.9.1

Re: [PATCH v2 0/8] leds: bcm6328/bcm6358: several fixes and improvements

[Jacek Anaszewski]

Hi Alvaro,

Thanks for the update, patch set applied.

On 12/15/2015 09:03 PM, Álvaro Fernández Rojas wrote:
> These patches add several fixes and improvements for leds-bcm6328 and
> leds-bcm6358.
>
> v2: introduce changes suggested by Jacek and Florian
>   - change "leds-bcm6328:" to "leds: bcm6328:"
>   - change "leds-bcm6358:" to "leds: bcm6358:"
>   - add little endian support
>   - bcm6358: Merge bcm6358_led_mode into bcm6358_led_set
>
> Álvaro Fernández Rojas (8):
>    leds: bcm6328: code cleaning
>    leds: bcm6328: remove unneded lock when checking initial LED
>    leds: bcm6358: Use bcm6358_led_set() in order to get rid of the lock
>    leds: bcm6358: remove unneded lock when checking initial LED status
>    leds: bcm6328: add little endian support
>    leds: bcm6358: add little endian support
>    leds: bcm6328: simplify duplicated unlock in bcm6328_blink_set
>    leds: bcm6358: merge bcm6358_led_mode and bcm6358_led_set
>
>   drivers/leds/leds-bcm6328.c | 29 +++++++++++++++++------------
>   drivers/leds/leds-bcm6358.c | 34 ++++++++++++++++------------------
>   2 files changed, 33 insertions(+), 30 deletions(-)
>


-- 
Best Regards,
Jacek Anaszewski