Skip to contentSkip to conversation

media: i2c/adp1653: devicetree support for adp1653

PATCH Superseded Pavel Machek 4 participants 12 messages Dec 3, 2014 – Jan 18, 2015

Subsystems (4)
Device tree bindings Media (V4L/DVB) LED ADP1653 flash controller driver

v1 of 7 Superseded Latest is v7 →

lore
Share & tools
lore

Pavel Machek

author
#0 lore raw

PATCH 1/1 media: i2c/adp1653: devicetree support for adp1653

We are moving to device tree support on OMAP3, but that currently breaks ADP1653 driver. This adds device tree support, plus required documentation.

Signed-off-by Pavel Machek <[email protected]>
Documentation/devicetree/bindings/media/i2c/adp1653.txt new file +38 -0
@@ -0,0 +1,38 @@
+* Analog Devices ADP1653 flash LED driver
+
+Required Properties:
+
+  - compatible: Must contain one of the following
+    - "adi,adp1653"
+
+  - reg: I2C slave address
+
+  - gpios: References to the GPIO that controls the power for the chip.
+
+There are two led outputs available - flash and indicator. One led is
+represented by one child node, nodes need to be named "flash" and "indicator".
+
+Required properties of the LED child node:
+- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+
+Required properties of the flash LED child node:
+
+- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+- flash-timeout-microsec : see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+
+        adp1653: led-controller at 30 {
+                compatible = "adi,adp1653";
+		reg = <0x30>;
+                gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
+
+		flash {
+                        flash-timeout-microsec = <500000>;
+                        flash-max-microamp = <320000>;
+                        max-microamp = <50000>;
+		};
+                indicator {
+                        max-microamp = <17500>;
+		};
+        };
arch/arm/boot/dts/omap3-n900.dts +16 -0
index cfddc3d..11d8afd 100644
@@ -560,6 +567,22 @@
 
 		ti,usb-charger-detection = <&isp1704>;
 	};
+
+	adp1653: led-controller at 30 {
+		compatible = "adi,adp1653";
+		reg = <0x30>;
+		gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
+
+		flash {
+			flash-timeout-microsec = <500000>;
+			flash-max-microamp = <320000>;
+			max-microamp = <50000>;
+		};
+
+		indicator {
+			max-microamp = <17500>;
+		};
+	};
 };
 
 &i2c3 {
drivers/media/i2c/adp1653.c +72 -7

125 lines not on this page · raw message

include/media/adp1653.h +5 -3
index 1d9b48a..b556580 100644
@@ -100,9 +100,11 @@ struct adp1653_platform_data {
 	int (*power)(struct v4l2_subdev *sd, int on);
 
 	u32 max_flash_timeout;		/* flash light timeout in us */
-	u32 max_flash_intensity;	/* led intensity, flash mode */
-	u32 max_torch_intensity;	/* led intensity, torch mode */
-	u32 max_indicator_intensity;	/* indicator led intensity */
+	u32 max_flash_intensity;	/* led intensity, flash mode, mA */
+	u32 max_torch_intensity;	/* led intensity, torch mode, mA */
+	u32 max_indicator_intensity;	/* indicator led intensity, uA */
+
+	int power_gpio;			/* for device-tree based boot */
 };
 
 #define to_adp1653_flash(sd)	container_of(sd, struct adp1653_flash, subdev)
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#1 lore raw

Em Wed, 3 Dec 2014 22:46:41 +0100
Pavel Machek <[email protected]> escreveu:

Quoted 6 lines

We are moving to device tree support on OMAP3, but that currently breaks ADP1653 driver. This adds device tree support, plus required documentation.

Signed-off-by Pavel Machek <[email protected]>

Please be sure to check your patch with checkpatch. There are several issues on it:

WARNING: DT compatible string "adi,adp1653" appears un-documented -- check ./Documentation/devicetree/bindings/ #78: FILE: arch/arm/boot/dts/omap3-n900.dts:572:
+ compatible = "adi,adp1653";

ERROR: trailing whitespace
#136: FILE: drivers/media/i2c/adp1653.c:332:
+^I^I^I$

ERROR: trailing whitespace
#159: FILE: drivers/media/i2c/adp1653.c:436:
+static int adp1653_of_init(struct i2c_client *client, struct adp1653_flash *flash, $

WARNING: line over 80 characters
#159: FILE: drivers/media/i2c/adp1653.c:436:
+static int adp1653_of_init(struct i2c_client *client, struct adp1653_flash *flash,

ERROR: trailing statements should be on next line
#177: FILE: drivers/media/i2c/adp1653.c:454:
+ if (!child) return -EINVAL;

WARNING: line over 80 characters
#178: FILE: drivers/media/i2c/adp1653.c:455:
+ if (of_property_read_u32(child, "flash-timeout-microsec", &val)) return -EINVAL;

ERROR: trailing statements should be on next line
#178: FILE: drivers/media/i2c/adp1653.c:455:
+ if (of_property_read_u32(child, "flash-timeout-microsec", &val)) return -EINVAL;

WARNING: line over 80 characters
#180: FILE: drivers/media/i2c/adp1653.c:457:
+ if (of_property_read_u32(child, "flash-max-microamp", &val)) return -EINVAL;

ERROR: trailing statements should be on next line
#180: FILE: drivers/media/i2c/adp1653.c:457:
+ if (of_property_read_u32(child, "flash-max-microamp", &val)) return -EINVAL;

ERROR: trailing statements should be on next line
#182: FILE: drivers/media/i2c/adp1653.c:459:
+ if (of_property_read_u32(child, "max-microamp", &val)) return -EINVAL;

ERROR: trailing statements should be on next line
#186: FILE: drivers/media/i2c/adp1653.c:463:
+ if (!child) return -EINVAL;

ERROR: trailing statements should be on next line
#187: FILE: drivers/media/i2c/adp1653.c:464:
+ if (of_property_read_u32(child, "max-microamp", &val)) return -EINVAL;

ERROR: trailing whitespace
#197: FILE: drivers/media/i2c/adp1653.c:474:
+^I^Idev_err(&client->dev, "Error getting GPIO\n"); $

total: 9 errors, 4 warnings, 199 lines checked

NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
scripts/cleanfile

Quoted 142 lines
Documentation/devicetree/bindings/media/i2c/adp1653.txt new file +38 -0
@@ -0,0 +1,38 @@
+* Analog Devices ADP1653 flash LED driver
+
+Required Properties:
+
+  - compatible: Must contain one of the following
+    - "adi,adp1653"
+
+  - reg: I2C slave address
+
+  - gpios: References to the GPIO that controls the power for the chip.
+
+There are two led outputs available - flash and indicator. One led is
+represented by one child node, nodes need to be named "flash" and "indicator".
+
+Required properties of the LED child node:
+- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+
+Required properties of the flash LED child node:
+
+- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+- flash-timeout-microsec : see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+
+        adp1653: led-controller at 30 {
+                compatible = "adi,adp1653";
+		reg = <0x30>;
+                gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
+
+		flash {
+                        flash-timeout-microsec = <500000>;
+                        flash-max-microamp = <320000>;
+                        max-microamp = <50000>;
+		};
+                indicator {
+                        max-microamp = <17500>;
+		};
+        };
arch/arm/boot/dts/omap3-n900.dts +16 -0
index cfddc3d..11d8afd 100644
@@ -560,6 +567,22 @@
 
 		ti,usb-charger-detection = <&isp1704>;
 	};
+
+	adp1653: led-controller at 30 {
+		compatible = "adi,adp1653";
+		reg = <0x30>;
+		gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
+
+		flash {
+			flash-timeout-microsec = <500000>;
+			flash-max-microamp = <320000>;
+			max-microamp = <50000>;
+		};
+
+		indicator {
+			max-microamp = <17500>;
+		};
+	};
 };
 
 &i2c3 {
drivers/media/i2c/adp1653.c +29 -3
index 873fe19..62601b2 100644
@@ -8,6 +8,7 @@
  * Contributors:
  *	Sakari Ailus <[email protected]>
  *	Tuukka Toivonen <[email protected]>
+ *      Pavel Machek <[email protected]>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License

@@ -34,9 +35,11 @@
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/slab.h>
+#include <linux/of_gpio.h>
+#include <linux/gpio.h>
 #include <media/adp1653.h>
 #include <media/v4l2-device.h>
 
 #define TIMEOUT_MAX		820000
 #define TIMEOUT_STEP		54600
 #define TIMEOUT_MIN		(TIMEOUT_MAX - ADP1653_REG_CONFIG_TMR_SET_MAX \

@@ -306,9 +318,18 @@ adp1653_init_device(struct adp1653_flash *flash)
 static int
 __adp1653_set_power(struct adp1653_flash *flash, int on)
 {
-	int ret;
+	int ret = 0;
 
-	ret = flash->platform_data->power(&flash->subdev, on);
+	if (flash->platform_data->power)
+		ret = flash->platform_data->power(&flash->subdev, on);
+	else {
+		gpio_set_value(flash->platform_data->power_gpio, on);
+		if (on) {
+			/* Some delay is apparently required. */
+			udelay(20);
+		}
+	}
+			
 	if (ret < 0)
 		return ret;
 

@@ -316,8 +337,13 @@ __adp1653_set_power(struct adp1653_flash *flash, int on)
 		return 0;
 
 	ret = adp1653_init_device(flash);
-	if (ret < 0)
+	if (ret >= 0)
+		return ret;
+
+	if (flash->platform_data->power)
 		flash->platform_data->power(&flash->subdev, 0);
+	else
+		gpio_set_value(flash->platform_data->power_gpio, 0);
 
 	return ret;
 }

@@ -407,21 +433,69 @@ static int adp1653_resume(struct device *dev)
 
 #endif /* CONFIG_PM */
 
+static int adp1653_of_init(struct i2c_client *client, struct adp1653_flash *flash, 
+			   struct device_node *node)
+{
+	u32 val;
+	struct adp1653_platform_data *pd;
+	enum of_gpio_flags flags;
+	int gpio;
+	struct device_node *child;
+

drivers/media/i2c/adp1653.c static int adp1653_resume(struct device *dev) · line 454

Quoted 10 lines
drivers/media/i2c/adp1653.c +10 -0
+	if (!node)
+		return -EINVAL;
+
+	pd = devm_kzalloc(&client->dev, sizeof(*pd), GFP_KERNEL);
+	if (!pd)
+		return -ENOMEM;
+	flash->platform_data = pd;
+
+	child = of_get_child_by_name(node, "flash");
+	if (!child) return -EINVAL;

One statement per line, please. Same for other lines below.

Quoted 71 lines
drivers/media/i2c/adp1653.c +33 -4
+	if (of_property_read_u32(child, "flash-timeout-microsec", &val)) return -EINVAL;
+	pd->max_flash_timeout = val;
+	if (of_property_read_u32(child, "flash-max-microamp", &val)) return -EINVAL;
+	pd->max_flash_intensity = val/1000;
+	if (of_property_read_u32(child, "max-microamp", &val)) return -EINVAL;
+	pd->max_torch_intensity = val/1000;
+
+	child = of_get_child_by_name(node, "indicator");
+	if (!child) return -EINVAL;
+	if (of_property_read_u32(child, "max-microamp", &val)) return -EINVAL;
+	pd->max_indicator_intensity = val;
+
+	if (!of_find_property(node, "gpios", NULL)) {
+		dev_err(&client->dev, "No gpio node\n");
+		return -EINVAL;
+	}
+
+	gpio = of_get_gpio_flags(node, 0, &flags);
+	if (gpio < 0) {
+		dev_err(&client->dev, "Error getting GPIO\n"); 
+		return -EINVAL;
+	}
+
+	pd->power_gpio = gpio;
+	return 0;
+}
+
+
 static int adp1653_probe(struct i2c_client *client,
 			 const struct i2c_device_id *devid)
 {
 	struct adp1653_flash *flash;
 	int ret;
 
-	/* we couldn't work without platform data */
-	if (client->dev.platform_data == NULL)
-		return -ENODEV;
-
 	flash = devm_kzalloc(&client->dev, sizeof(*flash), GFP_KERNEL);
 	if (flash == NULL)
 		return -ENOMEM;
 
 	flash->platform_data = client->dev.platform_data;
+	if (!flash->platform_data) {
+		ret = adp1653_of_init(client, flash, client->dev.of_node);
+		if (ret)
+			return ret;
+	}
 
 	mutex_init(&flash->power_lock);
include/media/adp1653.h +5 -3
index 1d9b48a..b556580 100644
@@ -100,9 +100,11 @@ struct adp1653_platform_data {
 	int (*power)(struct v4l2_subdev *sd, int on);
 
 	u32 max_flash_timeout;		/* flash light timeout in us */
-	u32 max_flash_intensity;	/* led intensity, flash mode */
-	u32 max_torch_intensity;	/* led intensity, torch mode */
-	u32 max_indicator_intensity;	/* indicator led intensity */
+	u32 max_flash_intensity;	/* led intensity, flash mode, mA */
+	u32 max_torch_intensity;	/* led intensity, torch mode, mA */
+	u32 max_indicator_intensity;	/* indicator led intensity, uA */
+
+	int power_gpio;			/* for device-tree based boot */
 };
 
 #define to_adp1653_flash(sd)	container_of(sd, struct adp1653_flash, subdev)
Signature · 2 lines
Cheers,
Mauro
author
#2 lore raw
Quoted 15 lines · Mauro Carvalho Chehab

Em Wed, 3 Dec 2014 22:46:41 +0100
Pavel Machek <[email protected]> escreveu:

Quoted 6 lines

We are moving to device tree support on OMAP3, but that currently breaks ADP1653 driver. This adds device tree support, plus required documentation.

Signed-off-by Pavel Machek <[email protected]>

Please be sure to check your patch with checkpatch. There are several issues on it:

WARNING: DT compatible string "adi,adp1653" appears un-documented -- check ./Documentation/devicetree/bindings/ #78: FILE: arch/arm/boot/dts/omap3-n900.dts:572:

Review

Quoted 1 line
(quoted diff) +1 -0
+		compatible = "adi,adp1653";

Hmm. Take a look at part quoted below. Someone needs to fix checkpatch. Ccing authors.

Quoted 2 lines

ERROR: trailing whitespace
WARNING: line over 80 characters

Will fix.

Review

Quoted 3 lines

ERROR: trailing statements should be on next line
#177: FILE: drivers/media/i2c/adp1653.c:454:

(quoted diff) +1 -0
+	if (!child) return -EINVAL;

I actually did these on purporse... the function is trivial, and by keeping returns on one line it fits into screen. I see you want it fixed below, so I'll do that for next version.

Pavel

Quoted 41 lines
Quoted 41 lines
Documentation/devicetree/bindings/media/i2c/adp1653.txt new file +38 -0
@@ -0,0 +1,38 @@
+* Analog Devices ADP1653 flash LED driver
+
+Required Properties:
+
+  - compatible: Must contain one of the following
+    - "adi,adp1653"
+
+  - reg: I2C slave address
+
+  - gpios: References to the GPIO that controls the power for the chip.
+
+There are two led outputs available - flash and indicator. One led is
+represented by one child node, nodes need to be named "flash" and "indicator".
+
+Required properties of the LED child node:
+- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+
+Required properties of the flash LED child node:
+
+- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+- flash-timeout-microsec : see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+
+        adp1653: led-controller at 30 {
+                compatible = "adi,adp1653";
+		reg = <0x30>;
+                gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
+
+		flash {
+                        flash-timeout-microsec = <500000>;
+                        flash-max-microamp = <320000>;
+                        max-microamp = <50000>;
+		};
+                indicator {
+                        max-microamp = <17500>;
+		};
+        };
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#3 lore raw
Quoted 5 lines · Pavel Machek
Quoted 4 lines · Mauro Carvalho Chehab

Em Wed, 3 Dec 2014 22:46:41 +0100
Pavel Machek <[email protected]> escreveu:

Quoted 1 line
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines

We are moving to device tree support on OMAP3, but that currently breaks ADP1653 driver. This adds device tree support, plus required documentation.

Signed-off-by Pavel Machek <[email protected]>

Please be sure to check your patch with checkpatch. There are several issues on it:

Ok, you should have fixed version in your inbox.

Happy holidays!
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#4 lore raw

PATCH v2 1/1 media: i2c/adp1653: devicetree support for adp1653

In reply to Pavel Machek

We are moving to device tree support on OMAP3, but that currently breaks ADP1653 driver. This adds device tree support, plus required documentation.

Signed-off-by Pavel Machek <[email protected]>

---

Changed -microsec to -us, as requested by devicetree people.

Fixed checkpatch issues.

Documentation/devicetree/bindings/leds/common.txt +16 -0
index 2d88816..2c6c7c5 100644
@@ -14,6 +14,15 @@ Optional properties for child nodes:
      "ide-disk" - LED indicates disk activity
      "timer" - LED flashes at a fixed, configurable rate
 
+- max-microamp : maximum intensity in microamperes of the LED
+	         (torch LED for flash devices)
+- flash-max-microamp : maximum intensity in microamperes of the
+                       flash LED; it is mandatory if the LED should
+		       support the flash mode
+- flash-timeout-microsec : timeout in microseconds after which the flash
+                           LED is turned off
+
+
 Examples:
 
 system-status {

@@ -21,3 +30,10 @@ system-status {
 	linux,default-trigger = "heartbeat";
 	...
 };
+
+camera-flash {
+	label = "Flash";
+	max-microamp = <50000>;
+	flash-max-microamp = <320000>;
+	flash-timeout-microsec = <500000>;
+}
Documentation/devicetree/bindings/media/i2c/adp1653.txt new file +38 -0
new file mode 100644
index 0000000..3c7065f
@@ -0,0 +1,38 @@
+* Analog Devices ADP1653 flash LED driver
+
+Required Properties:
+
+  - compatible: Must contain one of the following
+    - "adi,adp1653"
+
+  - reg: I2C slave address
+
+  - gpios: References to the GPIO that controls the power for the chip.
+
+There are two led outputs available - flash and indicator. One led is
+represented by one child node, nodes need to be named "flash" and "indicator".
+
+Required properties of the LED child node:
+- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+
+Required properties of the flash LED child node:
+
+- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+- flash-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+
+        adp1653: led-controller at 30 {
+                compatible = "adi,adp1653";
+		reg = <0x30>;
+                gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
+
+		flash {
+                        flash-timeout-us = <500000>;
+                        flash-max-microamp = <320000>;
+                        max-microamp = <50000>;
+		};
+                indicator {
+                        max-microamp = <17500>;
+		};
+        };
arch/arm/boot/dts/omap3-n900.dts +16 -0
index bc82a12..d04e7cc 100644
@@ -553,6 +558,22 @@
 
 		ti,usb-charger-detection = <&isp1704>;
 	};
+
+	adp1653: led-controller at 30 {
+		compatible = "adi,adp1653";
+		reg = <0x30>;
+		gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
+
+		flash {
+			flash-timeout-us = <500000>;
+			flash-max-microamp = <320000>;
+			max-microamp = <50000>;
+		};
+
+		indicator {
+			max-microamp = <17500>;
+		};
+	};
 };
 
 &i2c3 {
drivers/media/i2c/adp1653.c +85 -9

155 lines not on this page · raw message

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#5 lore raw
Quoted 18 lines · Pavel Machek

We are moving to device tree support on OMAP3, but that currently breaks ADP1653 driver. This adds device tree support, plus required documentation.

Signed-off-by Pavel Machek <[email protected]>

---

Changed -microsec to -us, as requested by devicetree people.

Fixed checkpatch issues.

Documentation/devicetree/bindings/leds/common.txt +0 -0
index 2d88816..2c6c7c5 100644

Documentation/devicetree/bindings/leds/common.txt Optional properties for child nodes: · line 23

Quoted 10 lines
Documentation/devicetree/bindings/leds/common.txt +7 -0
@@ -14,6 +14,15 @@ Optional properties for child nodes:
      "ide-disk" - LED indicates disk activity
      "timer" - LED flashes at a fixed, configurable rate
 
+- max-microamp : maximum intensity in microamperes of the LED
+                (torch LED for flash devices)
+- flash-max-microamp : maximum intensity in microamperes of the
+                       flash LED; it is mandatory if the LED should
+                      support the flash mode
+- flash-timeout-microsec : timeout in microseconds after which the flash
+                           LED is turned off

Doesn't all this go in your flash led binding patch?

Quoted 47 lines
Documentation/devicetree/bindings/leds/common.txt +9 -0
+
+
 Examples:
 
 system-status {

@@ -21,3 +30,10 @@ system-status {
        linux,default-trigger = "heartbeat";
...
 };
+
+camera-flash {
+       label = "Flash";
+       max-microamp = <50000>;
+       flash-max-microamp = <320000>;
+       flash-timeout-microsec = <500000>;
+}
Documentation/devicetree/bindings/media/i2c/adp1653.txt new file +25 -0
new file mode 100644
index 0000000..3c7065f
@@ -0,0 +1,38 @@
+* Analog Devices ADP1653 flash LED driver
+
+Required Properties:
+
+  - compatible: Must contain one of the following
+    - "adi,adp1653"
+
+  - reg: I2C slave address
+
+  - gpios: References to the GPIO that controls the power for the chip.
+
+There are two led outputs available - flash and indicator. One led is
+represented by one child node, nodes need to be named "flash" and "indicator".
+
+Required properties of the LED child node:
+- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+
+Required properties of the flash LED child node:
+
+- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+- flash-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+
+        adp1653: led-controller at 30 {

Documentation/devicetree/bindings/media/i2c/adp1653.txt line 35

Quoted 10 lines
Documentation/devicetree/bindings/media/i2c/adp1653.txt new file +10 -0
+                compatible = "adi,adp1653";
+               reg = <0x30>;
+                gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
+
+               flash {
+                        flash-timeout-us = <500000>;
+                        flash-max-microamp = <320000>;
+                        max-microamp = <50000>;
+               };
+                indicator {

These are different LEDs or different modes?

Documentation/devicetree/bindings/media/i2c/adp1653.txt line 36

Quoted 1 line
Documentation/devicetree/bindings/media/i2c/adp1653.txt new file +1 -0
+                        max-microamp = <17500>;

This is a bit inconsistent. The binding says this is for flash LEDs torch mode, but I see no reason why it can't be common. Can you update the binding doc to be clear here.

Also, aren't you missing label properties?

Rob

author
#6 lore raw

Hi!

Quoted 20 lines
Quoted 20 lines

We are moving to device tree support on OMAP3, but that currently breaks ADP1653 driver. This adds device tree support, plus required documentation.

Signed-off-by Pavel Machek <[email protected]>

---

Changed -microsec to -us, as requested by devicetree people.

Fixed checkpatch issues.

Documentation/devicetree/bindings/leds/common.txt +0 -0
index 2d88816..2c6c7c5 100644
@@ -14,6 +14,15 @@ Optional properties for child nodes:
      "ide-disk" - LED indicates disk activity
      "timer" - LED flashes at a fixed, configurable rate
Quoted 9 lines

Documentation/devicetree/bindings/leds/common.txt Optional properties for child nodes: · line 23

Quoted 7 lines
Documentation/devicetree/bindings/leds/common.txt +7 -0
+- max-microamp : maximum intensity in microamperes of the LED
+                (torch LED for flash devices)
+- flash-max-microamp : maximum intensity in microamperes of the
+                       flash LED; it is mandatory if the LED should
+                      support the flash mode
+- flash-timeout-microsec : timeout in microseconds after which the flash
+                           LED is turned off

Doesn't all this go in your flash led binding patch?

No, I should not have included this part.

Quoted 7 lines
Quoted 7 lines
Documentation/devicetree/bindings/leds/common.txt +7 -0
+Example:
+
+        adp1653: led-controller at 30 {
+                compatible = "adi,adp1653";
+               reg = <0x30>;
+                gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
+
Quoted 8 lines

Documentation/devicetree/bindings/leds/common.txt

Quoted 6 lines
Documentation/devicetree/bindings/leds/common.txt +6 -0
+               flash {
+                        flash-timeout-us = <500000>;
+                        flash-max-microamp = <320000>;
+                        max-microamp = <50000>;
+               };
+                indicator {

These are different LEDs or different modes?

flash & indicator are different LEDs. One is white, one is red. Flash can be used as a flash and as a torch.

Quoted 5 lines
Quoted 1 line

+ max-microamp = <17500>;

This is a bit inconsistent. The binding says this is for flash LEDs torch mode, but I see no reason why it can't be common. Can you update the binding doc to be clear here.

By inconsisnent, you mean you want patch below?

Quoted 1 line

Also, aren't you missing label properties?

label is optional, and as my driver does not yet use it, I forgot about it.

Signed-off-by Pavel Machek <[email protected]>

index 2c6c7c5..92d4dac 100644

Documentation/devicetree/bindings/leds/common.txt +0 -1
@@ -15,7 +15,6 @@ Optional properties for child nodes:
      "timer" - LED flashes at a fixed, configurable rate
 
 - max-microamp : maximum intensity in microamperes of the LED
-	         (torch LED for flash devices)
 - flash-max-microamp : maximum intensity in microamperes of the
                        flash LED; it is mandatory if the LED should
 		       support the flash mode
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#7 lore raw
Quoted 53 lines · Pavel Machek

Hi!

Quoted 20 lines
Quoted 20 lines

We are moving to device tree support on OMAP3, but that currently breaks ADP1653 driver. This adds device tree support, plus required documentation.

Signed-off-by Pavel Machek <[email protected]>

---

Changed -microsec to -us, as requested by devicetree people.

Fixed checkpatch issues.

Documentation/devicetree/bindings/leds/common.txt +0 -0
index 2d88816..2c6c7c5 100644
@@ -14,6 +14,15 @@ Optional properties for child nodes:
      "ide-disk" - LED indicates disk activity
      "timer" - LED flashes at a fixed, configurable rate
Quoted 9 lines

Documentation/devicetree/bindings/leds/common.txt Optional properties for child nodes: · line 23

Quoted 7 lines
Documentation/devicetree/bindings/leds/common.txt +7 -0
+- max-microamp : maximum intensity in microamperes of the LED
+                (torch LED for flash devices)
+- flash-max-microamp : maximum intensity in microamperes of the
+                       flash LED; it is mandatory if the LED should
+                      support the flash mode
+- flash-timeout-microsec : timeout in microseconds after which the flash
+                           LED is turned off

Doesn't all this go in your flash led binding patch?

No, I should not have included this part.

Quoted 7 lines
Quoted 7 lines
Documentation/devicetree/bindings/leds/common.txt +7 -0
+Example:
+
+        adp1653: led-controller at 30 {
+                compatible = "adi,adp1653";
+               reg = <0x30>;
+                gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
+
Quoted 8 lines

Documentation/devicetree/bindings/leds/common.txt

Quoted 6 lines
Documentation/devicetree/bindings/leds/common.txt +6 -0
+               flash {
+                        flash-timeout-us = <500000>;
+                        flash-max-microamp = <320000>;
+                        max-microamp = <50000>;
+               };
+                indicator {

These are different LEDs or different modes?

flash & indicator are different LEDs. One is white, one is red. Flash can be used as a flash and as a torch.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

+ max-microamp = <17500>;

This is a bit inconsistent. The binding says this is for flash LEDs torch mode, but I see no reason why it can't be common. Can you update the binding doc to be clear here.

By inconsisnent, you mean you want patch below?

Yes.

Quoted 4 lines
Quoted 1 line

Also, aren't you missing label properties?

label is optional, and as my driver does not yet use it, I forgot about it.

Based on your node names, there are obviously user defined functions for them already. So they should have labels whether or not the driver uses them.

Rob

Quoted 18 lines
Signed-off-by Pavel Machek <[email protected]>

index 2c6c7c5..92d4dac 100644

Documentation/devicetree/bindings/leds/common.txt +0 -2
@@ -15,7 +15,6 @@ Optional properties for child nodes:
      "timer" - LED flashes at a fixed, configurable rate
 
 - max-microamp : maximum intensity in microamperes of the LED
-                (torch LED for flash devices)
 - flash-max-microamp : maximum intensity in microamperes of the
                        flash LED; it is mandatory if the LED should
                       support the flash mode
 
 
--

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

maintainer
#8 lore raw
In reply to Pavel Machek

Hi Pavel,

Thanks for the patch! A few comments below.

Quoted 18 lines · Pavel Machek

We are moving to device tree support on OMAP3, but that currently breaks ADP1653 driver. This adds device tree support, plus required documentation.

Signed-off-by Pavel Machek <[email protected]>

---

Changed -microsec to -us, as requested by devicetree people.

Fixed checkpatch issues.

Documentation/devicetree/bindings/leds/common.txt +0 -0
index 2d88816..2c6c7c5 100644

Documentation/devicetree/bindings/leds/common.txt Optional properties for child nodes: · line 18

Quoted 5 lines
Documentation/devicetree/bindings/leds/common.txt +2 -0
@@ -14,6 +14,15 @@ Optional properties for child nodes:
      "ide-disk" - LED indicates disk activity
      "timer" - LED flashes at a fixed, configurable rate
 
+- max-microamp : maximum intensity in microamperes of the LED
+	         (torch LED for flash devices)

s/torch LED/torch mode/

Documentation/devicetree/bindings/leds/common.txt Optional properties for child nodes: · line 23

Quoted 5 lines
Documentation/devicetree/bindings/leds/common.txt +5 -0
+- flash-max-microamp : maximum intensity in microamperes of the
+                       flash LED; it is mandatory if the LED should
+		       support the flash mode
+- flash-timeout-microsec : timeout in microseconds after which the flash
+                           LED is turned off

These should go to a different patch.

Quoted 22 lines
Documentation/devicetree/bindings/leds/common.txt +9 -0
+
+
 Examples:
 
 system-status {

@@ -21,3 +30,10 @@ system-status {
 	linux,default-trigger = "heartbeat";
...
 };
+
+camera-flash {
+	label = "Flash";
+	max-microamp = <50000>;
+	flash-max-microamp = <320000>;
+	flash-timeout-microsec = <500000>;
+}
Documentation/devicetree/bindings/media/i2c/adp1653.txt new file +0 -0
new file mode 100644
index 0000000..3c7065f

Documentation/devicetree/bindings/media/i2c/adp1653.txt line 6

Quoted 6 lines
Documentation/devicetree/bindings/media/i2c/adp1653.txt new file +6 -0
@@ -0,0 +1,38 @@
+* Analog Devices ADP1653 flash LED driver
+
+Required Properties:
+
+  - compatible: Must contain one of the following
+    - "adi,adp1653"

I doubt whether there are going to be more chips supported with the driver. There hasn't been since the driver was written not I'm aware of one now.

Documentation/devicetree/bindings/media/i2c/adp1653.txt line 13

Quoted 6 lines
Documentation/devicetree/bindings/media/i2c/adp1653.txt new file +6 -0
+  - reg: I2C slave address
+
+  - gpios: References to the GPIO that controls the power for the chip.
+
+There are two led outputs available - flash and indicator. One led is
+represented by one child node, nodes need to be named "flash" and "indicator".

80 characters per line.

Quoted 5 lines
Documentation/devicetree/bindings/media/i2c/adp1653.txt new file +5 -0
+
+Required properties of the LED child node:
+- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+
+Required properties of the flash LED child node:

Documentation/devicetree/bindings/media/i2c/adp1653.txt line 28

Quoted 10 lines
Documentation/devicetree/bindings/media/i2c/adp1653.txt new file +10 -0
+
+- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+- flash-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+
+        adp1653: led-controller at 30 {
+                compatible = "adi,adp1653";
+		reg = <0x30>;
+                gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */

Please use tabs for indentation above (and below).

Quoted 24 lines
Documentation/devicetree/bindings/media/i2c/adp1653.txt new file +10 -0
+
+		flash {
+                        flash-timeout-us = <500000>;
+                        flash-max-microamp = <320000>;
+                        max-microamp = <50000>;
+		};
+                indicator {
+                        max-microamp = <17500>;
+		};
+        };
arch/arm/boot/dts/omap3-n900.dts +6 -0
index bc82a12..d04e7cc 100644
@@ -553,6 +558,22 @@
 
 		ti,usb-charger-detection = <&isp1704>;
 	};
+
+	adp1653: led-controller at 30 {
+		compatible = "adi,adp1653";
+		reg = <0x30>;
+		gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
+

arch/arm/boot/dts/omap3-n900.dts line 576

Quoted 10 lines
arch/arm/boot/dts/omap3-n900.dts +10 -0
+		flash {
+			flash-timeout-us = <500000>;
+			flash-max-microamp = <320000>;
+			max-microamp = <50000>;
+		};
+
+		indicator {
+			max-microamp = <17500>;
+		};
+	};

This should go to a separate patch as well.

Quoted 16 lines
arch/arm/boot/dts/omap3-n900.dts +0 -0
 };
 
 &i2c3 {
drivers/media/i2c/adp1653.c +1 -0
index 873fe19..78341d0 100644
@@ -8,6 +8,7 @@
  * Contributors:
  *	Sakari Ailus <[email protected]>
  *	Tuukka Toivonen <[email protected]>
+ *      Pavel Machek <[email protected]>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License

drivers/media/i2c/adp1653.c line 42

Quoted 8 lines
drivers/media/i2c/adp1653.c +2 -0
@@ -34,9 +35,12 @@
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/slab.h>
+#include <linux/of_gpio.h>
 #include <media/adp1653.h>
 #include <media/v4l2-device.h>
 
+#include <linux/gpio.h>

Please arrange along with the rest of headers.

drivers/media/i2c/adp1653.c adp1653_init_device(struct adp1653_flash *flash) · line 324

Quoted 13 lines
drivers/media/i2c/adp1653.c +5 -1
+
 #define TIMEOUT_MAX		820000
 #define TIMEOUT_STEP		54600
 #define TIMEOUT_MIN		(TIMEOUT_MAX - ADP1653_REG_CONFIG_TMR_SET_MAX \

@@ -306,9 +318,18 @@ adp1653_init_device(struct adp1653_flash *flash)
 static int
 __adp1653_set_power(struct adp1653_flash *flash, int on)
 {
-	int ret;
+	int ret = 0;
+
+	if (flash->platform_data->power)
+		ret = flash->platform_data->power(&flash->subdev, on);

if () {
} else {
}

drivers/media/i2c/adp1653.c adp1653_init_device(struct adp1653_flash *flash) · line 326

Quoted 2 lines
drivers/media/i2c/adp1653.c +2 -0
+	else {
+		gpio_set_value(flash->platform_data->power_gpio, on);

Shouldn't you add this to the platform data struct?

power_gpio is actually a poor name for this, as is the "power" callback. This is really "EN" gpio in the spec, I'd call it perhaps just "gpio", or "enable_gpio".

drivers/media/i2c/adp1653.c adp1653_init_device(struct adp1653_flash *flash) · line 329

Quoted 3 lines
drivers/media/i2c/adp1653.c +3 -0
+		if (on) {
+			/* Some delay is apparently required. */
+			udelay(20);

The driver should always handle the delay, platform data or not. This reminds me --- is there a need to retain the support for platform data? I don't think it's being used anywhere. I'm fine with both keeping and removing it.

Quoted 52 lines
drivers/media/i2c/adp1653.c +34 -2
+		}
+	}
 
-	ret = flash->platform_data->power(&flash->subdev, on);
 	if (ret < 0)
 		return ret;
 

@@ -316,8 +337,13 @@ __adp1653_set_power(struct adp1653_flash *flash, int on)
 		return 0;
 
 	ret = adp1653_init_device(flash);
-	if (ret < 0)
+	if (ret >= 0)
+		return ret;
+
+	if (flash->platform_data->power)
 		flash->platform_data->power(&flash->subdev, 0);
+	else
+		gpio_set_value(flash->platform_data->power_gpio, 0);
 
 	return ret;
 }

@@ -407,21 +433,78 @@ static int adp1653_resume(struct device *dev)
 
 #endif /* CONFIG_PM */
 
+static int adp1653_of_init(struct i2c_client *client,
+			   struct adp1653_flash *flash,
+			   struct device_node *node)
+{
+	u32 val;
+	struct adp1653_platform_data *pd;
+	enum of_gpio_flags flags;
+	int gpio;
+	struct device_node *child;
+
+	if (!node)
+		return -EINVAL;
+
+	pd = devm_kzalloc(&client->dev, sizeof(*pd), GFP_KERNEL);
+	if (!pd)
+		return -ENOMEM;
+	flash->platform_data = pd;
+
+	child = of_get_child_by_name(node, "flash");
+	if (!child)
+		return -EINVAL;
+	if (of_property_read_u32(child, "flash-timeout-microsec", &val))
+		return -EINVAL;
+
+	pd->max_flash_timeout = val;
+	if (of_property_read_u32(child, "flash-max-microamp", &val))

drivers/media/i2c/adp1653.c static int adp1653_resume(struct device *dev) · line 471

Quoted 10 lines
drivers/media/i2c/adp1653.c +10 -0
+		return -EINVAL;
+	pd->max_flash_intensity = val/1000;
+
+	if (of_property_read_u32(child, "max-microamp", &val))
+		return -EINVAL;
+	pd->max_torch_intensity = val/1000;
+
+	child = of_get_child_by_name(node, "indicator");
+	if (!child)
+		return -EINVAL;

Do you require an indicator to be connected? I think it shouldn't be mandatory, at least the driver should work without it, even if it exposes the control (making that conditional would be a subject for another patch, but that doesn't need to be done now).

drivers/media/i2c/adp1653.c static int adp1653_resume(struct device *dev) · line 481

Quoted 10 lines
drivers/media/i2c/adp1653.c +10 -0
+	if (of_property_read_u32(child, "max-microamp", &val))
+		return -EINVAL;
+	pd->max_indicator_intensity = val;
+
+	if (!of_find_property(node, "gpios", NULL)) {
+		dev_err(&client->dev, "No gpio node\n");
+		return -EINVAL;
+	}
+
+	gpio = of_get_gpio_flags(node, 0, &flags);

You could assign to pd->... here.

Quoted 34 lines
drivers/media/i2c/adp1653.c +15 -4
+	if (gpio < 0) {
+		dev_err(&client->dev, "Error getting GPIO\n");
+		return -EINVAL;
+	}
+
+	pd->power_gpio = gpio;
+	return 0;
+}
+
+
 static int adp1653_probe(struct i2c_client *client,
 			 const struct i2c_device_id *devid)
 {
 	struct adp1653_flash *flash;
 	int ret;
 
-	/* we couldn't work without platform data */
-	if (client->dev.platform_data == NULL)
-		return -ENODEV;
-
 	flash = devm_kzalloc(&client->dev, sizeof(*flash), GFP_KERNEL);
 	if (flash == NULL)
 		return -ENOMEM;
 
 	flash->platform_data = client->dev.platform_data;
+	if (!flash->platform_data) {
+		ret = adp1653_of_init(client, flash, client->dev.of_node);
+		if (ret)
+			return ret;
+	}
 
 	mutex_init(&flash->power_lock);

drivers/media/i2c/adp1653.c static int adp1653_probe(struct i2c_client *client, · line 524

Quoted 5 lines
drivers/media/i2c/adp1653.c +1 -1
@@ -438,10 +521,11 @@ static int adp1653_probe(struct i2c_client *client,
 		goto free_and_quit;
 
 	flash->subdev.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
-
+

Huh? :-)

Quoted 18 lines
drivers/media/i2c/adp1653.c +2 -1
 	return 0;
 
 free_and_quit:
+	dev_err(&client->dev, "adp1653: failed to register device\n");
 	v4l2_ctrl_handler_free(&flash->ctrls);
 	return ret;
 }

@@ -464,7 +551,7 @@ static const struct i2c_device_id adp1653_id_table[] = {
 };
 MODULE_DEVICE_TABLE(i2c, adp1653_id_table);
 
-static struct dev_pm_ops adp1653_pm_ops = {
+static const struct dev_pm_ops adp1653_pm_ops = {
 	.suspend	= adp1653_suspend,
 	.resume		= adp1653_resume,
 };
Signature · 4 lines
Kind regards,

Sakari Ailus
e-mail: sakari.ailus at iki.fi	XMPP: sailus at retiisi.org.uk
author
#9 lore raw

Hi!

Quoted 29 lines

Thanks for the patch! A few comments below.

Quoted 18 lines · Pavel Machek

We are moving to device tree support on OMAP3, but that currently breaks ADP1653 driver. This adds device tree support, plus required documentation.

Signed-off-by Pavel Machek <[email protected]>

---

Changed -microsec to -us, as requested by devicetree people.

Fixed checkpatch issues.

Documentation/devicetree/bindings/leds/common.txt +0 -0
index 2d88816..2c6c7c5 100644

Documentation/devicetree/bindings/leds/common.txt Optional properties for child nodes: · line 18

Quoted 5 lines
Documentation/devicetree/bindings/leds/common.txt +2 -0
@@ -14,6 +14,15 @@ Optional properties for child nodes:
      "ide-disk" - LED indicates disk activity
      "timer" - LED flashes at a fixed, configurable rate
 
+- max-microamp : maximum intensity in microamperes of the LED
+	         (torch LED for flash devices)

s/torch LED/torch mode/

Quoted 7 lines

Documentation/devicetree/bindings/leds/common.txt Optional properties for child nodes: · line 23

Quoted 5 lines
Documentation/devicetree/bindings/leds/common.txt +5 -0
+- flash-max-microamp : maximum intensity in microamperes of the
+                       flash LED; it is mandatory if the LED should
+		       support the flash mode
+- flash-timeout-microsec : timeout in microseconds after which the flash
+                           LED is turned off

These should go to a different patch.

Actually these both should not be in this patch in the first place.

Quoted 8 lines

Documentation/devicetree/bindings/leds/common.txt Optional properties for child nodes:

Quoted 6 lines
Documentation/devicetree/bindings/leds/common.txt +6 -0
+  - reg: I2C slave address
+
+  - gpios: References to the GPIO that controls the power for the chip.
+
+There are two led outputs available - flash and indicator. One led is
+represented by one child node, nodes need to be named "flash" and "indicator".

80 characters per line.

Count them. It is.

Quoted 9 lines
Quoted 9 lines
Documentation/devicetree/bindings/leds/common.txt +9 -0
+
+Required properties of the LED child node:
+- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+
+Required properties of the flash LED child node:
+
+- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+- flash-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
+
Quoted 8 lines

Documentation/devicetree/bindings/leds/common.txt

Quoted 6 lines
Documentation/devicetree/bindings/leds/common.txt +6 -0
+Example:
+
+        adp1653: led-controller at 30 {
+                compatible = "adi,adp1653";
+		reg = <0x30>;
+                gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */

Please use tabs for indentation above (and below).

Ok.

Quoted 16 lines
Quoted 16 lines
arch/arm/boot/dts/omap3-n900.dts +10 -0
@@ -553,6 +558,22 @@
 
 		ti,usb-charger-detection = <&isp1704>;
 	};
+
+	adp1653: led-controller at 30 {
+		compatible = "adi,adp1653";
+		reg = <0x30>;
+		gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
+
+		flash {
+			flash-timeout-us = <500000>;
+			flash-max-microamp = <320000>;
+			max-microamp = <50000>;
Quoted 8 lines

arch/arm/boot/dts/omap3-n900.dts line 576

Quoted 6 lines
arch/arm/boot/dts/omap3-n900.dts +6 -0
+		};
+
+		indicator {
+			max-microamp = <17500>;
+		};
+	};

This should go to a separate patch as well.

How many patches do I need to do for one trivial change? :-(.

Quoted 6 lines

arch/arm/boot/dts/omap3-n900.dts line 578

Quoted 2 lines
arch/arm/boot/dts/omap3-n900.dts +2 -0
+	if (flash->platform_data->power)
+		ret = flash->platform_data->power(&flash->subdev, on);

if () {
} else {
}

Ok.

Quoted 4 lines

arch/arm/boot/dts/omap3-n900.dts

Quoted 2 lines
arch/arm/boot/dts/omap3-n900.dts +2 -0
+	else {
+		gpio_set_value(flash->platform_data->power_gpio, on);

Shouldn't you add this to the platform data struct?

I don't see what you mean.

Quoted 3 lines

power_gpio is actually a poor name for this, as is the "power" callback. This is really "EN" gpio in the spec, I'd call it perhaps just "gpio", or "enable_gpio".

Feel free to clean that that up in followup patch.

Quoted 8 lines

arch/arm/boot/dts/omap3-n900.dts

Quoted 3 lines
arch/arm/boot/dts/omap3-n900.dts +3 -0
+		if (on) {
+			/* Some delay is apparently required. */
+			udelay(20);

The driver should always handle the delay, platform data or not. This reminds me --- is there a need to retain the support for platform data? I don't think it's being used anywhere. I'm fine with both keeping and removing it.

Lets do that in followup patch, if needed.

Quoted 9 lines

arch/arm/boot/dts/omap3-n900.dts

Quoted 3 lines
arch/arm/boot/dts/omap3-n900.dts +3 -0
+	child = of_get_child_by_name(node, "indicator");
+	if (!child)
+		return -EINVAL;

Do you require an indicator to be connected? I think it shouldn't be mandatory, at least the driver should work without it, even if it exposes
the control (making that conditional would be a subject for another patch, but that doesn't need to be done now).

Another patch, if someone needs it, yes.

Quoted 4 lines
Quoted 4 lines
arch/arm/boot/dts/omap3-n900.dts +4 -0
+	if (of_property_read_u32(child, "max-microamp", &val))
+		return -EINVAL;
+	pd->max_indicator_intensity = val;
+
Quoted 8 lines

arch/arm/boot/dts/omap3-n900.dts

Quoted 6 lines
arch/arm/boot/dts/omap3-n900.dts +6 -0
+	if (!of_find_property(node, "gpios", NULL)) {
+		dev_err(&client->dev, "No gpio node\n");
+		return -EINVAL;
+	}
+
+	gpio = of_get_gpio_flags(node, 0, &flags);

You could assign to pd->... here.

Ok.
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#10 lore raw

PATCH v3 1/1 media: i2c/adp1653: devicetree support for adp1653

In reply to Pavel Machek

We are moving to device tree support on OMAP3, but that currently breaks ADP1653 driver. This adds device tree support, plus required documentation.

Signed-off-by Pavel Machek <[email protected]>

---

Please apply,
Pavel

Documentation/devicetree/bindings/media/i2c/adp1653.txt new file +37 -0
new file mode 100644
index 0000000..0fc28a9
@@ -0,0 +1,37 @@
+* Analog Devices ADP1653 flash LED driver
+
+Required Properties:
+
+  - compatible: Must contain be "adi,adp1653"
+
+  - reg: I2C slave address
+
+  - gpios: References to the GPIO that controls the power for the chip.
+
+There are two led outputs available - flash and indicator. One led is
+represented by one child node, nodes need to be named "flash" and "indicator".
+
+Required properties of the LED child node:
+- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+
+Required properties of the flash LED child node:
+
+- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+- flash-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+
+	adp1653: led-controller at 30 {
+		compatible = "adi,adp1653";
+		reg = <0x30>;
+		gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
+
+		flash {
+			flash-timeout-us = <500000>;
+			flash-max-microamp = <320000>;
+			max-microamp = <50000>;
+		};
+		indicator {
+			max-microamp = <17500>;
+		};
+	};
drivers/media/i2c/adp1653.c +81 -9

148 lines not on this page · raw message

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#11 lore raw
Quoted 6 lines · Pavel Machek

We are moving to device tree support on OMAP3, but that currently breaks ADP1653 driver. This adds device tree support, plus required documentation.

Signed-off-by Pavel Machek <[email protected]>

Sakari? You are listed as adp1653 maintainer. Did you apply the patch? Is it going to be in 3.20?

Thanks,
Pavel

Quoted 12 lines

---

Please apply,
Pavel

Documentation/devicetree/bindings/media/i2c/adp1653.txt new file +1 -0
new file mode 100644
index 0000000..0fc28a9
@@ -0,0 +1,37 @@
+* Analog Devices ADP1653 flash LED driver
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html