AS3645A fixes

DT bindings document the property "ams,input-max-microamp" that limits the chip's maximum input current. The driver and the DTS however used "peak-current-limit" property. Fix this by using the property documented in DT binding documentation.

Signed-off-by Sakari Ailus <[email protected]>

Use integer numbers for LEDs, 0 is the flash and 1 is the indicator.

Signed-off-by Sakari Ailus <[email protected]>

 arch/arm/boot/dts/omap3-n950-n9.dtsi |  8 ++++++--
 drivers/leds/leds-as3645a.c          | 26 ++++++++++++++++++++++++--

arch/arm/boot/dts/omap3-n950-n9.dtsi +6 -2

@@ -267,15 +267,19 @@
 	clock-frequency = <400000>;
 
 	as3645a@30 {
+		#address-cells = <1>;
+		#size-cells = <0>;
 		reg = <0x30>;
 		compatible = "ams,as3645a";
-		flash {
+		flash@0 {
+			reg = <0x0>;
 			flash-timeout-us = <150000>;
 			flash-max-microamp = <320000>;
 			led-max-microamp = <60000>;
 			ams,input-max-microamp = <1750000>;
 		};
-		indicator {
+		indicator@1 {
+			reg = <0x1>;
 			led-max-microamp = <10000>;
 		};
 	};

drivers/leds/leds-as3645a.c +24 -2

index e3f89c6130d2..605e0c64e974 100644
@@ -112,6 +112,10 @@
 #define AS_PEAK_mA_TO_REG(a) \
 	((min_t(u32, AS_PEAK_mA_MAX, a) - 1250) / 250)
 
+/* LED numbers for Devicetree */
+#define AS_LED_FLASH				0
+#define AS_LED_INDICATOR			1
+
 enum as_mode {
 	AS_MODE_EXT_TORCH = 0 << AS_CONTROL_MODE_SETTING_SHIFT,
 	AS_MODE_INDICATOR = 1 << AS_CONTROL_MODE_SETTING_SHIFT,

@@ -491,10 +495,29 @@ static int as3645a_parse_node(struct as3645a *flash,
 			      struct device_node *node)
 {
 	struct as3645a_config *cfg = &flash->cfg;
+	struct device_node *child;
 	const char *name;
 	int rval;
 
-	flash->flash_node = of_get_child_by_name(node, "flash");
+	for_each_child_of_node(node, child) {
+		u32 id = 0;
+
+		of_property_read_u32(child, "reg", &id);
+
+		switch (id) {
+		case AS_LED_FLASH:
+			flash->flash_node = of_node_get(child);
+			break;
+		case AS_LED_INDICATOR:
+			flash->indicator_node = of_node_get(child);
+			break;
+		default:
+			dev_warn(&flash->client->dev,
+				 "unknown LED %u encountered, ignoring\n", id);
+			break;
+		}
+	}
+
 	if (!flash->flash_node) {
 		dev_err(&flash->client->dev, "can't find flash node\n");
 		return -ENODEV;

@@ -538,7 +561,6 @@ static int as3645a_parse_node(struct as3645a *flash,
 			     &cfg->peak);
 	cfg->peak = AS_PEAK_mA_TO_REG(cfg->peak);
 
-	flash->indicator_node = of_get_child_by_name(node, "indicator");
 	if (!flash->indicator_node) {
 		dev_warn(&flash->client->dev,
 			 "can't find indicator node\n");

Quoted 3 lines · Sakari Ailus

Use integer numbers for LEDs, 0 is the flash and 1 is the indicator.

Signed-off-by Sakari Ailus <[email protected]>

Dunno. Old code is shorter, old device tree is shorter, ... IMO both versions are fine, because the LEDs are really different. Do we have documentation somewhere saying that reg= should be used for this? Are you doing this for consistency?

Hi Pavel,

Thanks for the review.

Quoted 5 lines · Pavel Machek

Quoted 3 lines · Sakari Ailus

Use integer numbers for LEDs, 0 is the flash and 1 is the indicator.

Signed-off-by Sakari Ailus <[email protected]>

Quoted 4 lines

Dunno. Old code is shorter, old device tree is shorter, ... IMO both versions are fine, because the LEDs are really different. Do we have documentation somewhere saying that reg= should be used for this? Are you doing this for consistency?

Well, actually for ACPI support. :-) It requires less driver changes this way. See 17th and 18th patches in "[PATCH v9 00/23] Unified fwnode endpoint parser, async sub-device notifier support, N9 flash DTS".

A number of chips have LED binding that is aligned, see e.g. Documentation/devicetree/bindings/leds/leds-bcm6328.txt .

Quoted 15 lines · Sakari Ailus

Hi Pavel,

Thanks for the review.

Quoted 9 lines · Pavel Machek

Quoted 3 lines · Sakari Ailus

Use integer numbers for LEDs, 0 is the flash and 1 is the indicator.

Signed-off-by Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/[email protected]>

Dunno. Old code is shorter, old device tree is shorter, ... IMO both versions are fine, because the LEDs are really different. Do we have documentation somewhere saying that reg= should be used for this? Are you doing this for consistency?

Quoted 3 lines

Well, actually for ACPI support. :-) It requires less driver changes this way. See 17th and 18th patches in "[PATCH v9 00/23] Unified fwnode endpoint parser, async sub-device notifier support, N9 flash DTS".

ACPI, I hate ACPI.

Quoted 3 lines

A number of chips have LED binding that is aligned, see e.g. Documentation/devicetree/bindings/leds/leds-bcm6328.txt .

Ok, yes, that's common way LED controllers are handled. Usually all the LEDs are "same", but...

Acked-by Pavel Machek <[email protected]>

Use integers (reg property) to tell the number of the LED to the driver instead of the node name. While both of these approaches are currently used by the LED bindings, using integers will require less driver changes for ACPI support. Additionally, it will make possible LED naming using chip and LED node names, effectively making the label property most useful for human-readable names only.

Signed-off-by Sakari Ailus <[email protected]>

---
 .../devicetree/bindings/leds/ams,as3645a.txt       | 28 ++++++++++++++--------
 1 file changed, 18 insertions(+), 10 deletions(-)

Hi folks,

Here are a few fixes for the as3645a DTS as well as changes in bindings. The driver is not in a release yet. I'd like to get these in as through the media tree fixes branch.

Sakari Ailus (3):
as3645a: Use ams,input-max-microamp as documented in DT bindings
dt: bindings: as3645a: Use LED number to refer to LEDs
as3645a: Use integer numbers for parsing LEDs

 .../devicetree/bindings/leds/ams,as3645a.txt       | 28 ++++++++++++++--------
 arch/arm/boot/dts/omap3-n950-n9.dtsi               | 10 +++++---
 drivers/leds/leds-as3645a.c                        | 28 +++++++++++++++++++---
 3 files changed, 50 insertions(+), 16 deletions(-)