diff mbox

[RESEND,1/4] leds: leds-ns2: move LED modes mapping outside of the driver

Message ID 1434640650-28086-2-git-send-email-simon.guinot@sequanux.org (mailing list archive)
State New, archived
Headers show

Commit Message

Simon Guinot June 18, 2015, 3:17 p.m. UTC
From: Vincent Donnefort <vdonnefort@gmail.com>

On the board n090401 (Seagate NAS 4-Bay), the LED mode mapping (GPIO
values to LED mode) is different from the one used on other boards
supported by the leds-ns2 driver.

With this patch the hardcoded mapping is removed from leds-ns2. Now,
it must be defined either in the platform data (if an old-fashion board
setup file is used) or in the DT node. In order to allow the later, this
patch also introduces a modes-map property for the leds-ns2 DT binding.

Signed-off-by: Vincent Donnefort <vdonnefort@gmail.com>
---
 .../devicetree/bindings/leds/leds-ns2.txt          |   9 ++
 drivers/leds/leds-ns2.c                            | 102 +++++++++++----------
 include/dt-bindings/leds/leds-ns2.h                |   8 ++
 include/linux/platform_data/leds-kirkwood-ns2.h    |  14 +++
 4 files changed, 85 insertions(+), 48 deletions(-)
 create mode 100644 include/dt-bindings/leds/leds-ns2.h

Comments

Jacek Anaszewski June 22, 2015, 2:32 p.m. UTC | #1
Hi Simon,

On 06/18/2015 05:17 PM, Simon Guinot wrote:
> From: Vincent Donnefort <vdonnefort@gmail.com>
>
> On the board n090401 (Seagate NAS 4-Bay), the LED mode mapping (GPIO
> values to LED mode) is different from the one used on other boards
> supported by the leds-ns2 driver.
>
> With this patch the hardcoded mapping is removed from leds-ns2. Now,
> it must be defined either in the platform data (if an old-fashion board
> setup file is used) or in the DT node. In order to allow the later, this
> patch also introduces a modes-map property for the leds-ns2 DT binding.
>
> Signed-off-by: Vincent Donnefort <vdonnefort@gmail.com>
> ---
>   .../devicetree/bindings/leds/leds-ns2.txt          |   9 ++
>   drivers/leds/leds-ns2.c                            | 102 +++++++++++----------
>   include/dt-bindings/leds/leds-ns2.h                |   8 ++
>   include/linux/platform_data/leds-kirkwood-ns2.h    |  14 +++
>   4 files changed, 85 insertions(+), 48 deletions(-)
>   create mode 100644 include/dt-bindings/leds/leds-ns2.h
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-ns2.txt b/Documentation/devicetree/bindings/leds/leds-ns2.txt
> index aef3aca34d2d..9f81258a5b6e 100644
> --- a/Documentation/devicetree/bindings/leds/leds-ns2.txt
> +++ b/Documentation/devicetree/bindings/leds/leds-ns2.txt
> @@ -8,6 +8,9 @@ Each LED is represented as a sub-node of the ns2-leds device.
>   Required sub-node properties:
>   - cmd-gpio: Command LED GPIO. See OF device-tree GPIO specification.
>   - slow-gpio: Slow LED GPIO. See OF device-tree GPIO specification.
> +- modes-map: A mapping between LED modes (off, on or SATA activity blinking) and
> +  the corresponding cmd-gpio/slow-gpio values. All the GPIO values combinations
> +  should be given in order to avoid having an unknown mode at driver probe time.
>
>   Optional sub-node properties:
>   - label: Name for this LED. If omitted, the label is taken from the node name.
> @@ -15,6 +18,8 @@ Optional sub-node properties:
>
>   Example:
>
> +#include <dt-bindings/leds/leds-ns2.h>
> +
>   ns2-leds {
>   	compatible = "lacie,ns2-leds";
>
> @@ -22,5 +27,9 @@ ns2-leds {
>   		label = "ns2:blue:sata";
>   		slow-gpio = <&gpio0 29 0>;
>   		cmd-gpio = <&gpio0 30 0>;
> +		modes-map = <NS_V2_LED_OFF  0 1
> +			     NS_V2_LED_ON   1 0
> +			     NS_V2_LED_ON   0 0
> +			     NS_V2_LED_SATA 1 1>;
>   	};
>   };

This looks good to me but please cc devicetree@vger.kernel.org when you
modify DT bindings.

> diff --git a/drivers/leds/leds-ns2.c b/drivers/leds/leds-ns2.c
> index 1fd6adbb43b7..b0bc03539dbb 100644
> --- a/drivers/leds/leds-ns2.c
> +++ b/drivers/leds/leds-ns2.c
> @@ -33,46 +33,20 @@
>   #include <linux/of_gpio.h>
>
>   /*
> - * The Network Space v2 dual-GPIO LED is wired to a CPLD and can blink in
> - * relation with the SATA activity. This capability is exposed through the
> - * "sata" sysfs attribute.
> - *
> - * The following array detail the different LED registers and the combination
> - * of their possible values:
> - *
> - *  cmd_led   |  slow_led  | /SATA active | LED state
> - *            |            |              |
> - *     1      |     0      |      x       |  off
> - *     -      |     1      |      x       |  on
> - *     0      |     0      |      1       |  on
> - *     0      |     0      |      0       |  blink (rate 300ms)
> + * The Network Space v2 dual-GPIO LED is wired to a CPLD. Three different LED
> + * modes are available: off, on and SATA activity blinking. The LED modes are
> + * controlled through two GPIOs (command and slow): each combination of values
> + * for the command/slow GPIOs corresponds to a LED mode.
>    */
>
> -enum ns2_led_modes {
> -	NS_V2_LED_OFF,
> -	NS_V2_LED_ON,
> -	NS_V2_LED_SATA,
> -};
> -
> -struct ns2_led_mode_value {
> -	enum ns2_led_modes	mode;
> -	int			cmd_level;
> -	int			slow_level;
> -};
> -
> -static struct ns2_led_mode_value ns2_led_modval[] = {
> -	{ NS_V2_LED_OFF	, 1, 0 },
> -	{ NS_V2_LED_ON	, 0, 1 },
> -	{ NS_V2_LED_ON	, 1, 1 },
> -	{ NS_V2_LED_SATA, 0, 0 },
> -};
> -
>   struct ns2_led_data {
>   	struct led_classdev	cdev;
>   	unsigned		cmd;
>   	unsigned		slow;
>   	unsigned char		sata; /* True when SATA mode active. */
>   	rwlock_t		rw_lock; /* Lock GPIOs. */
> +	int			num_modes;
> +	struct ns2_led_modval	*modval;
>   };
>
>   static int ns2_led_get_mode(struct ns2_led_data *led_dat,
> @@ -88,10 +62,10 @@ static int ns2_led_get_mode(struct ns2_led_data *led_dat,
>   	cmd_level = gpio_get_value(led_dat->cmd);
>   	slow_level = gpio_get_value(led_dat->slow);
>
> -	for (i = 0; i < ARRAY_SIZE(ns2_led_modval); i++) {
> -		if (cmd_level == ns2_led_modval[i].cmd_level &&
> -		    slow_level == ns2_led_modval[i].slow_level) {
> -			*mode = ns2_led_modval[i].mode;
> +	for (i = 0; i < led_dat->num_modes; i++) {
> +		if (cmd_level == led_dat->modval[i].cmd_level &&
> +		    slow_level == led_dat->modval[i].slow_level) {
> +			*mode = led_dat->modval[i].mode;
>   			ret = 0;
>   			break;
>   		}
> @@ -110,12 +84,12 @@ static void ns2_led_set_mode(struct ns2_led_data *led_dat,
>
>   	write_lock_irqsave(&led_dat->rw_lock, flags);
>
> -	for (i = 0; i < ARRAY_SIZE(ns2_led_modval); i++) {
> -		if (mode == ns2_led_modval[i].mode) {
> +	for (i = 0; i < led_dat->num_modes; i++) {
> +		if (mode == led_dat->modval[i].mode) {
>   			gpio_set_value(led_dat->cmd,
> -				       ns2_led_modval[i].cmd_level);
> +				       led_dat->modval[i].cmd_level);
>   			gpio_set_value(led_dat->slow,
> -				       ns2_led_modval[i].slow_level);
> +				       led_dat->modval[i].slow_level);
>   		}
>   	}
>
> @@ -228,6 +202,8 @@ create_ns2_led(struct platform_device *pdev, struct ns2_led_data *led_dat,
>   	led_dat->cdev.groups = ns2_led_groups;
>   	led_dat->cmd = template->cmd;
>   	led_dat->slow = template->slow;
> +	led_dat->modval = template->modval;
> +	led_dat->num_modes = template->num_modes;
>
>   	ret = ns2_led_get_mode(led_dat, &mode);
>   	if (ret < 0)
> @@ -259,9 +235,8 @@ ns2_leds_get_of_pdata(struct device *dev, struct ns2_led_platform_data *pdata)
>   {
>   	struct device_node *np = dev->of_node;
>   	struct device_node *child;
> -	struct ns2_led *leds;
> +	struct ns2_led *led, *leds;
>   	int num_leds = 0;
> -	int i = 0;
>
>   	num_leds = of_get_child_count(np);
>   	if (!num_leds)
> @@ -272,26 +247,57 @@ ns2_leds_get_of_pdata(struct device *dev, struct ns2_led_platform_data *pdata)
>   	if (!leds)
>   		return -ENOMEM;
>
> +	led = leds;
>   	for_each_child_of_node(np, child) {
>   		const char *string;
> -		int ret;
> +		int ret, i, num_modes;
> +		struct ns2_led_modval *modval;
>
>   		ret = of_get_named_gpio(child, "cmd-gpio", 0);
>   		if (ret < 0)
>   			return ret;
> -		leds[i].cmd = ret;
> +		led->cmd = ret;
>   		ret = of_get_named_gpio(child, "slow-gpio", 0);
>   		if (ret < 0)
>   			return ret;
> -		leds[i].slow = ret;
> +		led->slow = ret;
>   		ret = of_property_read_string(child, "label", &string);
> -		leds[i].name = (ret == 0) ? string : child->name;
> +		led->name = (ret == 0) ? string : child->name;
>   		ret = of_property_read_string(child, "linux,default-trigger",
>   					      &string);
>   		if (ret == 0)
> -			leds[i].default_trigger = string;
> +			led->default_trigger = string;
> +
> +		ret = of_property_count_u32_elems(child, "modes-map");

I think that we shouldn't fail if the property is absent, but default
to the mapping that is currently hard coded in the driver. Otherwise
we would break existing users.

> +		if (ret < 0 || ret % 3) {
> +			dev_err(dev,
> +				"Missing or malformed modes-map property\n");
> +			return -EINVAL;
> +		}
> +
> +		num_modes = ret / 3;
> +		modval = devm_kzalloc(dev,
> +				      num_modes * sizeof(struct ns2_led_modval),
> +				      GFP_KERNEL);
> +		if (!modval)
> +			return -ENOMEM;
> +
> +		for (i = 0; i < num_modes; i++) {
> +			of_property_read_u32_index(child,
> +						"modes-map", 3 * i,
> +						(u32 *) &modval[i].mode);
> +			of_property_read_u32_index(child,
> +						"modes-map", 3 * i + 1,
> +						(u32 *) &modval[i].cmd_level);
> +			of_property_read_u32_index(child,
> +						"modes-map", 3 * i + 2,
> +						(u32 *) &modval[i].slow_level);
> +		}
> +
> +		led->num_modes = num_modes;
> +		led->modval = modval;
>
> -		i++;
> +		led++;
>   	}
>
>   	pdata->leds = leds;
> diff --git a/include/dt-bindings/leds/leds-ns2.h b/include/dt-bindings/leds/leds-ns2.h
> new file mode 100644
> index 000000000000..491c5f974a92
> --- /dev/null
> +++ b/include/dt-bindings/leds/leds-ns2.h
> @@ -0,0 +1,8 @@
> +#ifndef _DT_BINDINGS_LEDS_NS2_H
> +#define _DT_BINDINGS_LEDS_NS2_H
> +
> +#define NS_V2_LED_OFF	0
> +#define NS_V2_LED_ON	1
> +#define NS_V2_LED_SATA	2
> +
> +#endif
> diff --git a/include/linux/platform_data/leds-kirkwood-ns2.h b/include/linux/platform_data/leds-kirkwood-ns2.h
> index 6a9fed57f346..eb8a6860e816 100644
> --- a/include/linux/platform_data/leds-kirkwood-ns2.h
> +++ b/include/linux/platform_data/leds-kirkwood-ns2.h
> @@ -9,11 +9,25 @@
>   #ifndef __LEDS_KIRKWOOD_NS2_H
>   #define __LEDS_KIRKWOOD_NS2_H
>
> +enum ns2_led_modes {
> +	NS_V2_LED_OFF,
> +	NS_V2_LED_ON,
> +	NS_V2_LED_SATA,
> +};
> +
> +struct ns2_led_modval {
> +	enum ns2_led_modes	mode;
> +	int			cmd_level;
> +	int			slow_level;
> +};
> +
>   struct ns2_led {
>   	const char	*name;
>   	const char	*default_trigger;
>   	unsigned	cmd;
>   	unsigned	slow;
> +	int		num_modes;
> +	struct ns2_led_modval *modval;
>   };
>
>   struct ns2_led_platform_data {
>
Simon Guinot June 23, 2015, 6:12 p.m. UTC | #2
On Mon, Jun 22, 2015 at 04:32:37PM +0200, Jacek Anaszewski wrote:
> Hi Simon,

Hi Jacek,

Thanks for taking care of this patches.

> 
> On 06/18/2015 05:17 PM, Simon Guinot wrote:
> >From: Vincent Donnefort <vdonnefort@gmail.com>
> >
> >On the board n090401 (Seagate NAS 4-Bay), the LED mode mapping (GPIO
> >values to LED mode) is different from the one used on other boards
> >supported by the leds-ns2 driver.
> >
> >With this patch the hardcoded mapping is removed from leds-ns2. Now,
> >it must be defined either in the platform data (if an old-fashion board
> >setup file is used) or in the DT node. In order to allow the later, this
> >patch also introduces a modes-map property for the leds-ns2 DT binding.
> >
> >Signed-off-by: Vincent Donnefort <vdonnefort@gmail.com>
> >---
> >  .../devicetree/bindings/leds/leds-ns2.txt          |   9 ++
> >  drivers/leds/leds-ns2.c                            | 102 +++++++++++----------
> >  include/dt-bindings/leds/leds-ns2.h                |   8 ++
> >  include/linux/platform_data/leds-kirkwood-ns2.h    |  14 +++
> >  4 files changed, 85 insertions(+), 48 deletions(-)
> >  create mode 100644 include/dt-bindings/leds/leds-ns2.h
> >
> >diff --git a/Documentation/devicetree/bindings/leds/leds-ns2.txt b/Documentation/devicetree/bindings/leds/leds-ns2.txt
> >index aef3aca34d2d..9f81258a5b6e 100644
> >--- a/Documentation/devicetree/bindings/leds/leds-ns2.txt
> >+++ b/Documentation/devicetree/bindings/leds/leds-ns2.txt
> >@@ -8,6 +8,9 @@ Each LED is represented as a sub-node of the ns2-leds device.
> >  Required sub-node properties:
> >  - cmd-gpio: Command LED GPIO. See OF device-tree GPIO specification.
> >  - slow-gpio: Slow LED GPIO. See OF device-tree GPIO specification.
> >+- modes-map: A mapping between LED modes (off, on or SATA activity blinking) and
> >+  the corresponding cmd-gpio/slow-gpio values. All the GPIO values combinations
> >+  should be given in order to avoid having an unknown mode at driver probe time.
> >
> >  Optional sub-node properties:
> >  - label: Name for this LED. If omitted, the label is taken from the node name.
> >@@ -15,6 +18,8 @@ Optional sub-node properties:
> >
> >  Example:
> >
> >+#include <dt-bindings/leds/leds-ns2.h>
> >+
> >  ns2-leds {
> >  	compatible = "lacie,ns2-leds";
> >
> >@@ -22,5 +27,9 @@ ns2-leds {
> >  		label = "ns2:blue:sata";
> >  		slow-gpio = <&gpio0 29 0>;
> >  		cmd-gpio = <&gpio0 30 0>;
> >+		modes-map = <NS_V2_LED_OFF  0 1
> >+			     NS_V2_LED_ON   1 0
> >+			     NS_V2_LED_ON   0 0
> >+			     NS_V2_LED_SATA 1 1>;
> >  	};
> >  };
> 
> This looks good to me but please cc devicetree@vger.kernel.org when you
> modify DT bindings.

OK.

> 
> >diff --git a/drivers/leds/leds-ns2.c b/drivers/leds/leds-ns2.c
> >index 1fd6adbb43b7..b0bc03539dbb 100644
> >--- a/drivers/leds/leds-ns2.c
> >+++ b/drivers/leds/leds-ns2.c
> >@@ -33,46 +33,20 @@
> >  #include <linux/of_gpio.h>
> >
> >  /*
> >- * The Network Space v2 dual-GPIO LED is wired to a CPLD and can blink in
> >- * relation with the SATA activity. This capability is exposed through the
> >- * "sata" sysfs attribute.
> >- *
> >- * The following array detail the different LED registers and the combination
> >- * of their possible values:
> >- *
> >- *  cmd_led   |  slow_led  | /SATA active | LED state
> >- *            |            |              |
> >- *     1      |     0      |      x       |  off
> >- *     -      |     1      |      x       |  on
> >- *     0      |     0      |      1       |  on
> >- *     0      |     0      |      0       |  blink (rate 300ms)
> >+ * The Network Space v2 dual-GPIO LED is wired to a CPLD. Three different LED
> >+ * modes are available: off, on and SATA activity blinking. The LED modes are
> >+ * controlled through two GPIOs (command and slow): each combination of values
> >+ * for the command/slow GPIOs corresponds to a LED mode.
> >   */
> >
> >-enum ns2_led_modes {
> >-	NS_V2_LED_OFF,
> >-	NS_V2_LED_ON,
> >-	NS_V2_LED_SATA,
> >-};
> >-
> >-struct ns2_led_mode_value {
> >-	enum ns2_led_modes	mode;
> >-	int			cmd_level;
> >-	int			slow_level;
> >-};
> >-
> >-static struct ns2_led_mode_value ns2_led_modval[] = {
> >-	{ NS_V2_LED_OFF	, 1, 0 },
> >-	{ NS_V2_LED_ON	, 0, 1 },
> >-	{ NS_V2_LED_ON	, 1, 1 },
> >-	{ NS_V2_LED_SATA, 0, 0 },
> >-};
> >-

...

> >+	led = leds;
> >  	for_each_child_of_node(np, child) {
> >  		const char *string;
> >-		int ret;
> >+		int ret, i, num_modes;
> >+		struct ns2_led_modval *modval;
> >
> >  		ret = of_get_named_gpio(child, "cmd-gpio", 0);
> >  		if (ret < 0)
> >  			return ret;
> >-		leds[i].cmd = ret;
> >+		led->cmd = ret;
> >  		ret = of_get_named_gpio(child, "slow-gpio", 0);
> >  		if (ret < 0)
> >  			return ret;
> >-		leds[i].slow = ret;
> >+		led->slow = ret;
> >  		ret = of_property_read_string(child, "label", &string);
> >-		leds[i].name = (ret == 0) ? string : child->name;
> >+		led->name = (ret == 0) ? string : child->name;
> >  		ret = of_property_read_string(child, "linux,default-trigger",
> >  					      &string);
> >  		if (ret == 0)
> >-			leds[i].default_trigger = string;
> >+			led->default_trigger = string;
> >+
> >+		ret = of_property_count_u32_elems(child, "modes-map");
> 
> I think that we shouldn't fail if the property is absent, but default
> to the mapping that is currently hard coded in the driver. Otherwise
> we would break existing users.

I don't think there is a risk of breaking existing users. On platforms
where the leds-ns2 driver is used, DTB will be updated with the kernel
image. Moreover, removing the hard coded mapping is a nice clean-up.

Let me know if you still want me to add a fallback.

Thanks,

Simon
Jacek Anaszewski June 24, 2015, 7:34 a.m. UTC | #3
Hi Simon,

On 06/23/2015 08:12 PM, Simon Guinot wrote:
[...]
>
>>> +	led = leds;
>>>   	for_each_child_of_node(np, child) {
>>>   		const char *string;
>>> -		int ret;
>>> +		int ret, i, num_modes;
>>> +		struct ns2_led_modval *modval;
>>>
>>>   		ret = of_get_named_gpio(child, "cmd-gpio", 0);
>>>   		if (ret < 0)
>>>   			return ret;
>>> -		leds[i].cmd = ret;
>>> +		led->cmd = ret;
>>>   		ret = of_get_named_gpio(child, "slow-gpio", 0);
>>>   		if (ret < 0)
>>>   			return ret;
>>> -		leds[i].slow = ret;
>>> +		led->slow = ret;
>>>   		ret = of_property_read_string(child, "label", &string);
>>> -		leds[i].name = (ret == 0) ? string : child->name;
>>> +		led->name = (ret == 0) ? string : child->name;
>>>   		ret = of_property_read_string(child, "linux,default-trigger",
>>>   					      &string);
>>>   		if (ret == 0)
>>> -			leds[i].default_trigger = string;
>>> +			led->default_trigger = string;
>>> +
>>> +		ret = of_property_count_u32_elems(child, "modes-map");
>>
>> I think that we shouldn't fail if the property is absent, but default
>> to the mapping that is currently hard coded in the driver. Otherwise
>> we would break existing users.
>
> I don't think there is a risk of breaking existing users. On platforms
> where the leds-ns2 driver is used, DTB will be updated with the kernel
> image. Moreover, removing the hard coded mapping is a nice clean-up.
>
> Let me know if you still want me to add a fallback.

Since you modify also dts file in this patch set this is OK.
You can keep this part as is.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/leds/leds-ns2.txt b/Documentation/devicetree/bindings/leds/leds-ns2.txt
index aef3aca34d2d..9f81258a5b6e 100644
--- a/Documentation/devicetree/bindings/leds/leds-ns2.txt
+++ b/Documentation/devicetree/bindings/leds/leds-ns2.txt
@@ -8,6 +8,9 @@  Each LED is represented as a sub-node of the ns2-leds device.
 Required sub-node properties:
 - cmd-gpio: Command LED GPIO. See OF device-tree GPIO specification.
 - slow-gpio: Slow LED GPIO. See OF device-tree GPIO specification.
+- modes-map: A mapping between LED modes (off, on or SATA activity blinking) and
+  the corresponding cmd-gpio/slow-gpio values. All the GPIO values combinations
+  should be given in order to avoid having an unknown mode at driver probe time.
 
 Optional sub-node properties:
 - label: Name for this LED. If omitted, the label is taken from the node name.
@@ -15,6 +18,8 @@  Optional sub-node properties:
 
 Example:
 
+#include <dt-bindings/leds/leds-ns2.h>
+
 ns2-leds {
 	compatible = "lacie,ns2-leds";
 
@@ -22,5 +27,9 @@  ns2-leds {
 		label = "ns2:blue:sata";
 		slow-gpio = <&gpio0 29 0>;
 		cmd-gpio = <&gpio0 30 0>;
+		modes-map = <NS_V2_LED_OFF  0 1
+			     NS_V2_LED_ON   1 0
+			     NS_V2_LED_ON   0 0
+			     NS_V2_LED_SATA 1 1>;
 	};
 };
diff --git a/drivers/leds/leds-ns2.c b/drivers/leds/leds-ns2.c
index 1fd6adbb43b7..b0bc03539dbb 100644
--- a/drivers/leds/leds-ns2.c
+++ b/drivers/leds/leds-ns2.c
@@ -33,46 +33,20 @@ 
 #include <linux/of_gpio.h>
 
 /*
- * The Network Space v2 dual-GPIO LED is wired to a CPLD and can blink in
- * relation with the SATA activity. This capability is exposed through the
- * "sata" sysfs attribute.
- *
- * The following array detail the different LED registers and the combination
- * of their possible values:
- *
- *  cmd_led   |  slow_led  | /SATA active | LED state
- *            |            |              |
- *     1      |     0      |      x       |  off
- *     -      |     1      |      x       |  on
- *     0      |     0      |      1       |  on
- *     0      |     0      |      0       |  blink (rate 300ms)
+ * The Network Space v2 dual-GPIO LED is wired to a CPLD. Three different LED
+ * modes are available: off, on and SATA activity blinking. The LED modes are
+ * controlled through two GPIOs (command and slow): each combination of values
+ * for the command/slow GPIOs corresponds to a LED mode.
  */
 
-enum ns2_led_modes {
-	NS_V2_LED_OFF,
-	NS_V2_LED_ON,
-	NS_V2_LED_SATA,
-};
-
-struct ns2_led_mode_value {
-	enum ns2_led_modes	mode;
-	int			cmd_level;
-	int			slow_level;
-};
-
-static struct ns2_led_mode_value ns2_led_modval[] = {
-	{ NS_V2_LED_OFF	, 1, 0 },
-	{ NS_V2_LED_ON	, 0, 1 },
-	{ NS_V2_LED_ON	, 1, 1 },
-	{ NS_V2_LED_SATA, 0, 0 },
-};
-
 struct ns2_led_data {
 	struct led_classdev	cdev;
 	unsigned		cmd;
 	unsigned		slow;
 	unsigned char		sata; /* True when SATA mode active. */
 	rwlock_t		rw_lock; /* Lock GPIOs. */
+	int			num_modes;
+	struct ns2_led_modval	*modval;
 };
 
 static int ns2_led_get_mode(struct ns2_led_data *led_dat,
@@ -88,10 +62,10 @@  static int ns2_led_get_mode(struct ns2_led_data *led_dat,
 	cmd_level = gpio_get_value(led_dat->cmd);
 	slow_level = gpio_get_value(led_dat->slow);
 
-	for (i = 0; i < ARRAY_SIZE(ns2_led_modval); i++) {
-		if (cmd_level == ns2_led_modval[i].cmd_level &&
-		    slow_level == ns2_led_modval[i].slow_level) {
-			*mode = ns2_led_modval[i].mode;
+	for (i = 0; i < led_dat->num_modes; i++) {
+		if (cmd_level == led_dat->modval[i].cmd_level &&
+		    slow_level == led_dat->modval[i].slow_level) {
+			*mode = led_dat->modval[i].mode;
 			ret = 0;
 			break;
 		}
@@ -110,12 +84,12 @@  static void ns2_led_set_mode(struct ns2_led_data *led_dat,
 
 	write_lock_irqsave(&led_dat->rw_lock, flags);
 
-	for (i = 0; i < ARRAY_SIZE(ns2_led_modval); i++) {
-		if (mode == ns2_led_modval[i].mode) {
+	for (i = 0; i < led_dat->num_modes; i++) {
+		if (mode == led_dat->modval[i].mode) {
 			gpio_set_value(led_dat->cmd,
-				       ns2_led_modval[i].cmd_level);
+				       led_dat->modval[i].cmd_level);
 			gpio_set_value(led_dat->slow,
-				       ns2_led_modval[i].slow_level);
+				       led_dat->modval[i].slow_level);
 		}
 	}
 
@@ -228,6 +202,8 @@  create_ns2_led(struct platform_device *pdev, struct ns2_led_data *led_dat,
 	led_dat->cdev.groups = ns2_led_groups;
 	led_dat->cmd = template->cmd;
 	led_dat->slow = template->slow;
+	led_dat->modval = template->modval;
+	led_dat->num_modes = template->num_modes;
 
 	ret = ns2_led_get_mode(led_dat, &mode);
 	if (ret < 0)
@@ -259,9 +235,8 @@  ns2_leds_get_of_pdata(struct device *dev, struct ns2_led_platform_data *pdata)
 {
 	struct device_node *np = dev->of_node;
 	struct device_node *child;
-	struct ns2_led *leds;
+	struct ns2_led *led, *leds;
 	int num_leds = 0;
-	int i = 0;
 
 	num_leds = of_get_child_count(np);
 	if (!num_leds)
@@ -272,26 +247,57 @@  ns2_leds_get_of_pdata(struct device *dev, struct ns2_led_platform_data *pdata)
 	if (!leds)
 		return -ENOMEM;
 
+	led = leds;
 	for_each_child_of_node(np, child) {
 		const char *string;
-		int ret;
+		int ret, i, num_modes;
+		struct ns2_led_modval *modval;
 
 		ret = of_get_named_gpio(child, "cmd-gpio", 0);
 		if (ret < 0)
 			return ret;
-		leds[i].cmd = ret;
+		led->cmd = ret;
 		ret = of_get_named_gpio(child, "slow-gpio", 0);
 		if (ret < 0)
 			return ret;
-		leds[i].slow = ret;
+		led->slow = ret;
 		ret = of_property_read_string(child, "label", &string);
-		leds[i].name = (ret == 0) ? string : child->name;
+		led->name = (ret == 0) ? string : child->name;
 		ret = of_property_read_string(child, "linux,default-trigger",
 					      &string);
 		if (ret == 0)
-			leds[i].default_trigger = string;
+			led->default_trigger = string;
+
+		ret = of_property_count_u32_elems(child, "modes-map");
+		if (ret < 0 || ret % 3) {
+			dev_err(dev,
+				"Missing or malformed modes-map property\n");
+			return -EINVAL;
+		}
+
+		num_modes = ret / 3;
+		modval = devm_kzalloc(dev,
+				      num_modes * sizeof(struct ns2_led_modval),
+				      GFP_KERNEL);
+		if (!modval)
+			return -ENOMEM;
+
+		for (i = 0; i < num_modes; i++) {
+			of_property_read_u32_index(child,
+						"modes-map", 3 * i,
+						(u32 *) &modval[i].mode);
+			of_property_read_u32_index(child,
+						"modes-map", 3 * i + 1,
+						(u32 *) &modval[i].cmd_level);
+			of_property_read_u32_index(child,
+						"modes-map", 3 * i + 2,
+						(u32 *) &modval[i].slow_level);
+		}
+
+		led->num_modes = num_modes;
+		led->modval = modval;
 
-		i++;
+		led++;
 	}
 
 	pdata->leds = leds;
diff --git a/include/dt-bindings/leds/leds-ns2.h b/include/dt-bindings/leds/leds-ns2.h
new file mode 100644
index 000000000000..491c5f974a92
--- /dev/null
+++ b/include/dt-bindings/leds/leds-ns2.h
@@ -0,0 +1,8 @@ 
+#ifndef _DT_BINDINGS_LEDS_NS2_H
+#define _DT_BINDINGS_LEDS_NS2_H
+
+#define NS_V2_LED_OFF	0
+#define NS_V2_LED_ON	1
+#define NS_V2_LED_SATA	2
+
+#endif
diff --git a/include/linux/platform_data/leds-kirkwood-ns2.h b/include/linux/platform_data/leds-kirkwood-ns2.h
index 6a9fed57f346..eb8a6860e816 100644
--- a/include/linux/platform_data/leds-kirkwood-ns2.h
+++ b/include/linux/platform_data/leds-kirkwood-ns2.h
@@ -9,11 +9,25 @@ 
 #ifndef __LEDS_KIRKWOOD_NS2_H
 #define __LEDS_KIRKWOOD_NS2_H
 
+enum ns2_led_modes {
+	NS_V2_LED_OFF,
+	NS_V2_LED_ON,
+	NS_V2_LED_SATA,
+};
+
+struct ns2_led_modval {
+	enum ns2_led_modes	mode;
+	int			cmd_level;
+	int			slow_level;
+};
+
 struct ns2_led {
 	const char	*name;
 	const char	*default_trigger;
 	unsigned	cmd;
 	unsigned	slow;
+	int		num_modes;
+	struct ns2_led_modval *modval;
 };
 
 struct ns2_led_platform_data {