diff mbox

leds: Add DT support for leds-pca9532

Message ID 1410269280-8761-1-git-send-email-stigge@antcom.de
State Superseded, archived
Headers show

Commit Message

stigge@antcom.de Sept. 9, 2014, 1:28 p.m. UTC 
This patch adds DT support for leds-pca9532.

Signed-off-by: Roland Stigge <stigge@antcom.de>

---
Applies to v3.17-rc4

 Documentation/devicetree/bindings/leds/leds-pca9532.txt |   43 ++++++++++++++
 drivers/leds/leds-pca9532.c                             |   47 ++++++++++++++++
 2 files changed, 90 insertions(+)

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Arnd Bergmann Sept. 9, 2014, 6:08 p.m. UTC | #1 
On Tuesday 09 September 2014 15:28:00 Roland Stigge wrote:
> +#ifdef CONFIG_OF
> +static struct pca9532_platform_data *pca9532_parse_dt(struct device *dev)
> +{
...
> +
>  static int pca9532_probe(struct i2c_client *client,
>         const struct i2c_device_id *id)
>  {
> @@ -449,6 +489,11 @@ static int pca9532_probe(struct i2c_clie
>         struct pca9532_platform_data *pca9532_pdata =
>                         dev_get_platdata(&client->dev);
>  
> +#ifdef CONFIG_OF
> +       if (!pca9532_pdata && client->dev.of_node)
> +               pca9532_pdata = pca9532_parse_dt(&client->dev);
> +#endif
> +

It would be better to remove the two #ifdef statements and instead
do this in C language as

	if (IS_ENABLED(CONFIG_OF) && !pca9532_pdata && client->dev.of_node)
		pca9532_pdata = pca9532_parse_dt(&client->dev);

This gives you extra compile-time coverage when CONFIG_OF is not
enabled.

I would probably also remove the !pca9532_pdata check there, since
we rarely do auxdata for new devices these days, but that is a separate
matter, and the check does not hurt.

> +       if (!of_property_read_u32_array(np, "nxp,pwm", &pwm[0], 2)) {
> +               for (i = 0; i < 2; i++)
> +                       pca9532_pdata->pwm[i] = pwm[i];
> +       }

Is this a full PWM controller? If it is, it would be better to also
provide a #pwm-cells property and register the device with the PWM
subsystem to allow arbitrary users.

If the PWM is only really usable for LED, it's probably enough to
leave this used by the LED subsystem.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
stigge@antcom.de Sept. 10, 2014, 3:22 p.m. UTC | #2 
On 09/09/2014 08:08 PM, Arnd Bergmann wrote:
> Is this a full PWM controller? If it is, it would be better to also
> provide a #pwm-cells property and register the device with the PWM
> subsystem to allow arbitrary users.
> 
> If the PWM is only really usable for LED, it's probably enough to
> leave this used by the LED subsystem.

Right, this PWM is really only intended for LED use (as already done w/o
DT).

Sending the updated patch.

Roland
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- /dev/null
+++ linux-2.6/Documentation/devicetree/bindings/leds/leds-pca9532.txt
@@ -0,0 +1,43 @@ 
+NXP PCA9532 LED (incl. GPIO) controller
+
+Required properties:
+- compatible: must be "nxp,pca9532"
+- reg: I2C address
+- gpio-controller: Marks the device node as a GPIO controller.
+- #gpio-cells: Should be 2:
+   1) pin number
+   2) optional parameters:
+      - bit 0 specifies polarity (0 for normal, 1 for inverted)
+- nxp,typecodes: groups of 2 bits for each of the 16 pins:
+  0: None
+  1: LED
+  2: Beep
+  3: GPIO
+- nxp,statecodes: groups of 2 bits for each of the 16 pins:
+  0: off
+  1: on
+  2: pwm0
+  3: pwm1
+
+Optional properties:
+- nxp,psc: array of 2 numbers: PSC values for the two PWM channels (see datasheet)
+- nxp,pwm: array of 2 numbers: PWM values for the two PWM channels (see datasheet)
+
+Example:
+
+	pca9532: pca9532@60 {
+		compatible = "nxp,pca9532";
+		gpio-controller;
+		#gpio-cells = <2>;
+		nxp,typecodes = <0xffffffff>;
+		nxp,statecodes = <0x00000000>;
+		reg = <0x60>;
+	};
+
+	sd@20098000 {
+		wp-gpios = <&pca9532 5 0>;
+		cd-gpios = <&pca9532 4 0>;
+		cd-inverted;
+		bus-width = <4>;
+		status = "okay";
+	};
--- linux-2.6.orig/drivers/leds/leds-pca9532.c
+++ linux-2.6/drivers/leds/leds-pca9532.c
@@ -21,6 +21,7 @@ 
 #include <linux/workqueue.h>
 #include <linux/leds-pca9532.h>
 #include <linux/gpio.h>
+#include <linux/of.h>
 
 /* m =  num_leds*/
 #define PCA9532_REG_INPUT(i)	((i) >> 3)
@@ -442,6 +443,45 @@  exit:
 	return err;
 }
 
+#ifdef CONFIG_OF
+static struct pca9532_platform_data *pca9532_parse_dt(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct pca9532_platform_data *pca9532_pdata;
+	u32 typecodes, statecodes;
+	u32 pwm[2];
+	u32 psc[2];
+	int i;
+
+	pca9532_pdata = devm_kzalloc(dev, sizeof(*pca9532_pdata), GFP_KERNEL);
+	if (!pca9532_pdata)
+		return NULL;
+
+	if (!of_property_read_u32(np, "nxp,typecodes", &typecodes)) {
+		for (i = 0; i < 16; i++)
+			pca9532_pdata->leds[i].type =
+				(typecodes >> (2 * i)) & 0x3;
+	}
+	if (!of_property_read_u32(np, "nxp,statecodes", &statecodes)) {
+		for (i = 0; i < 16; i++)
+			pca9532_pdata->leds[i].state =
+				(statecodes >> (2 * i)) & 0x3;
+	}
+	if (!of_property_read_u32_array(np, "nxp,pwm", &pwm[0], 2)) {
+		for (i = 0; i < 2; i++)
+			pca9532_pdata->pwm[i] = pwm[i];
+	}
+	if (!of_property_read_u32_array(np, "nxp,psc", &psc[0], 2)) {
+		for (i = 0; i < 2; i++)
+			pca9532_pdata->psc[i] = psc[i];
+	}
+
+	pca9532_pdata->gpio_base = -1; /* dynamically assign gpio base */
+
+	return pca9532_pdata;
+}
+#endif
+
 static int pca9532_probe(struct i2c_client *client,
 	const struct i2c_device_id *id)
 {
@@ -449,6 +489,11 @@  static int pca9532_probe(struct i2c_clie
 	struct pca9532_platform_data *pca9532_pdata =
 			dev_get_platdata(&client->dev);
 
+#ifdef CONFIG_OF
+	if (!pca9532_pdata && client->dev.of_node)
+		pca9532_pdata = pca9532_parse_dt(&client->dev);
+#endif
+
 	if (!pca9532_pdata)
 		return -EIO;