linux-leds.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Simon Arlott <simon@fire.lp0.eu>
To: Richard Purdie <rpurdie@rpsys.net>, j.anaszewski@samsung.com
Cc: linux-leds@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: [PATCH] leds: bcm6328: Handle default-state of LEDs correctly
Date: Sat, 24 Oct 2015 17:53:45 +0100	[thread overview]
Message-ID: <562BB799.7000708@simon.arlott.org.uk> (raw)

The default-state handler assumes that the LED is active low and omits
use of the shift macro causing "keep" to misdetect the state.

Determine the brightness and then use the led set function to apply it.

Update the documentation to indicate that this driver works for the
BCM63168 (which has many active high LEDs) too.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
 .../devicetree/bindings/leds/leds-bcm6328.txt      |  2 +-
 drivers/leds/leds-bcm6328.c                        | 23 ++++++++++++----------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-bcm6328.txt b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
index f9e36ad..d00260d 100644
--- a/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
+++ b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
@@ -1,6 +1,6 @@
 LEDs connected to Broadcom BCM6328 controller
 
-This controller is present on BCM6318, BCM6328, BCM6362 and BCM63268.
+This controller is present on BCM6318, BCM6328, BCM6362, BCM63168 and BCM63268.
 In these SoCs it's possible to control LEDs both as GPIOs or by hardware.
 However, on some devices there are Serial LEDs (LEDs connected to a 74x164
 controller), which can either be controlled by software (exporting the 74x164
diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
index 1793727..771c171 100644
--- a/drivers/leds/leds-bcm6328.c
+++ b/drivers/leds/leds-bcm6328.c
@@ -259,7 +259,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;
 
@@ -282,13 +281,12 @@ static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
 						    NULL);
 
 	if (!of_property_read_string(nc, "default-state", &state)) {
-		spin_lock_irqsave(lock, flags);
 		if (!strcmp(state, "on")) {
 			led->cdev.brightness = LED_FULL;
-			bcm6328_led_mode(led, BCM6328_LED_MODE_ON);
 		} else if (!strcmp(state, "keep")) {
 			void __iomem *mode;
 			unsigned long val, shift;
+			unsigned long flags;
 
 			shift = bcm6328_pin2shift(led->pin);
 			if (shift / 16)
@@ -296,19 +294,24 @@ static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
 			else
 				mode = mem + BCM6328_REG_MODE_LO;
 
-			val = bcm6328_led_read(mode) >> (shift % 16);
+			spin_lock_irqsave(lock, flags);
+			val = bcm6328_led_read(mode);
+			spin_unlock_irqrestore(lock, flags);
+
+			val >>= BCM6328_LED_SHIFT(shift % 16);
 			val &= BCM6328_LED_MODE_MASK;
-			if (val == BCM6328_LED_MODE_ON)
+
+			dev_info(dev, "pin %lu = %08lx\n", led->pin, val);
+			if ((led->active_low && val == BCM6328_LED_MODE_ON) ||
+			    (!led->active_low && val == BCM6328_LED_MODE_OFF))
 				led->cdev.brightness = LED_FULL;
-			else {
+			else
 				led->cdev.brightness = LED_OFF;
-				bcm6328_led_mode(led, BCM6328_LED_MODE_OFF);
-			}
 		} else {
 			led->cdev.brightness = LED_OFF;
-			bcm6328_led_mode(led, BCM6328_LED_MODE_OFF);
 		}
-		spin_unlock_irqrestore(lock, flags);
+
+		bcm6328_led_set(&led->cdev, led->cdev.brightness);
 	}
 
 	led->cdev.brightness_set = bcm6328_led_set;
-- 
2.1.4

-- 
Simon Arlott

             reply	other threads:[~2015-10-24 16:53 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-24 16:53 Simon Arlott [this message]
2015-10-26  8:45 ` [PATCH] leds: bcm6328: Handle default-state of LEDs correctly Jacek Anaszewski
2015-10-26 12:36   ` Simon Arlott
2015-10-28 10:56     ` Jacek Anaszewski
2015-10-29 19:48       ` [PATCH] leds-bcm6328: Reuse bcm6328_led_set() instead of copying its functionality Simon Arlott
2015-11-04 15:41         ` Jacek Anaszewski
2015-11-04 15:46           ` Álvaro Fernández Rojas
2015-11-05 10:41             ` Jacek Anaszewski
2015-11-15 13:32               ` [PATCH 1/2] " Simon Arlott
2015-11-15 13:34                 ` [PATCH 2/2] leds-bcm6328: Swap LED ON and OFF definitions Simon Arlott
2015-11-23 10:20                   ` Jacek Anaszewski
2015-11-16 14:38                 ` [PATCH 1/2] leds-bcm6328: Reuse bcm6328_led_set() instead of copying its functionality Jacek Anaszewski
2015-11-16 20:24                   ` [PATCH 1/2 (v2)] " Simon Arlott
2015-11-16 21:33                     ` Álvaro Fernández Rojas
2015-11-17  7:42                       ` Simon Arlott
2015-11-17  8:06                         ` Jacek Anaszewski
2015-11-17  8:15                           ` Jacek Anaszewski
2015-11-22 20:40                             ` [PATCH 1/2 (v3)] " Simon Arlott
2015-11-23 10:19                               ` Jacek Anaszewski
2015-11-23 10:20                     ` [PATCH 1/2 (v2)] " Jacek Anaszewski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=562BB799.7000708@simon.arlott.org.uk \
    --to=simon@fire.lp0.eu \
    --cc=j.anaszewski@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=rpurdie@rpsys.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).