Skip to contentSkip to conversation

Make AUX gpio pin optional for ktd2692

PATCH Superseded Markuss Broks 3 participants 5 messages Apr 7, 2022 – Apr 8, 2022

Subsystems (2)

v1 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

cover Make AUX gpio pin optional for ktd2692

Some appliances of ktd2692 don't have the AUX pin connected to a GPIO. Specifically, Samsung Galaxy J5 (2015), which uses ktd2692 for driving the front flash LED, has the pin not connected anywhere on schematics. Make specifying the AUX pin optional, since it is additional functionality and only affects amount of current going through the LED.

Also convert the txt device-tree bindings to yaml and pick up maintainership over the yaml binding and the driver itself.

Markuss Broks (2):
dt-bindings: leds: convert ktd2692 bindings to yaml
leds: ktd2692: Make aux-gpios optional

 .../bindings/leds/kinetic,ktd2692.yaml        | 87 +++++++++++++++++++
 .../devicetree/bindings/leds/leds-ktd2692.txt | 50 -----------
 MAINTAINERS                                   |  6 ++
 drivers/leds/flash/leds-ktd2692.c             | 18 ++--
 4 files changed, 103 insertions(+), 58 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/kinetic,ktd2692.yaml
 delete mode 100644 Documentation/devicetree/bindings/leds/leds-ktd2692.txt
Signature · 1 line
2.35.1
#1 lore raw

PATCH 1/2 dt-bindings: leds: convert ktd2692 bindings to yaml

This patch converts the leds-ktd2692.txt bindings to modern yaml style device-tree bindings.

---
 .../bindings/leds/kinetic,ktd2692.yaml        | 87 +++++++++++++++++++
 .../devicetree/bindings/leds/leds-ktd2692.txt | 50 -----------
 2 files changed, 87 insertions(+), 50 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/kinetic,ktd2692.yaml
 delete mode 100644 Documentation/devicetree/bindings/leds/leds-ktd2692.txt
Documentation/devicetree/bindings/leds/kinetic,ktd2692.yaml new file +87 -0
new file mode 100644
index 000000000000..e9058316a5ba
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/kinetic,ktd2692.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: KTD2692 Flash LED Driver from Kinetic Technologies
+
+maintainers:
+  - Markuss Broks <[email protected]>
+
+description: |
+  KTD2692 is the ideal power solution for high-power flash LEDs.
+  It uses ExpressWire single-wire programming for maximum flexibility.
+
+  The ExpressWire interface through CTRL pin can control LED on/off and
+  enable/disable the IC, Movie(max 1/3 of Flash current) / Flash mode current,
+  Flash timeout, LVP(low voltage protection).
+
+  Also, When the AUX pin is pulled high while CTRL pin is high,
+  LED current will be ramped up to the flash-mode current level.
+
+properties:
+  compatible:
+    const: kinetic,ktd2692
+
+  ctrl-gpios:
+    maxItems: 1
+    description: Specifier of the GPIO connected to CTRL pin.
+
+  aux-gpios:
+    maxItems: 1
+    description: Specifier of the GPIO connected to CTRL pin.
+
+  vin-supply:
+    maxItems: 1
+    description: "vin" LED supply (2.7V to 5.5V).
+
+  led:
+    type: object
+    $ref: common.yaml#
+    description: Properties for the LED.
+      properties:
+        function: true
+        color: true
+        flash-max-timeout-us:
+          description: Flash LED maximum timeout.
+
+        led-max-microamp:
+          maximum: 300000
+          description: Minimum Threshold for Timer protection
+            is defined internally (Maximum 300mA).
+
+        flash-max-microamp:
+          description: Flash LED maximum current
+            Formula - I(mA) = 15000 / Rset.
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - ctrl-gpios
+  - led
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/leds/common.h>
+
+    ktd2692 {
+      compatible = "kinetic,ktd2692";
+      ctrl-gpios = <&gpc0 1 0>;
+      aux-gpios = <&gpc0 2 0>;
+      vin-supply = <&vbat>;
+
+      led {
+        function = LED_FUNCTION_FLASH;
+        color = <LED_COLOR_ID_WHITE>;
+        flash-max-timeout-us = <250000>;
+        flash-max-microamp = <150000>;
+        led-max-microamp = <25000>;
+      };
+    };
+
+...
Documentation/devicetree/bindings/leds/leds-ktd2692.txt deleted +0 -50

50 lines not on this page · raw message

Signature · 1 line
2.35.1
maintainer
#2 lore raw

Re: [PATCH 1/2] dt-bindings: leds: convert ktd2692 bindings to yaml

Quoted 11 lines · Markuss Broks

This patch converts the leds-ktd2692.txt bindings to modern yaml style device-tree bindings.

---
 .../bindings/leds/kinetic,ktd2692.yaml        | 87 +++++++++++++++++++
 .../devicetree/bindings/leds/leds-ktd2692.txt | 50 -----------
 2 files changed, 87 insertions(+), 50 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/kinetic,ktd2692.yaml
 delete mode 100644 Documentation/devicetree/bindings/leds/leds-ktd2692.txt

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/leds/kinetic,ktd2692.yaml:37:24: [error] syntax error: expected <block end>, but found '<scalar>' (syntax)

dtschema/dtc warnings/errors:
make[1]: *** Deleting file 'Documentation/devicetree/bindings/leds/kinetic,ktd2692.example.dts'
Traceback (most recent call last):
File "/usr/local/bin/dt-extract-example", line 52, in <module>
binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 434, in load
return constructor.get_single_data()
File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 119, in get_single_data
node = self.composer.get_single_node()
File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
File "_ruamel_yaml.pyx", line 891, in _ruamel_yaml.CParser._compose_mapping_node
File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
ruamel.yaml.parser.ParserError: while parsing a block mapping
in "<unicode string>", line 36, column 5
did not find expected key
in "<unicode string>", line 37, column 24
make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/leds/kinetic,ktd2692.example.dts] Error 1 make[1]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/leds/kinetic,ktd2692.yaml: while parsing a block mapping
in "<unicode string>", line 36, column 5
did not find expected key
in "<unicode string>", line 37, column 24
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/kinetic,ktd2692.yaml: ignoring, error parsing file
make: *** [Makefile:1401: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit.

#3 lore raw

PATCH 2/2 leds: ktd2692: Make aux-gpios optional

In reply to Markuss Broks

Make the AUX pin optional, since it isn't a core part of functionality, and the device is designed to be operational with only one CTRL pin.

Also pick up maintainership for the LED driver and the yaml bindings.

---
 MAINTAINERS                       |  6 ++++++
 drivers/leds/flash/leds-ktd2692.c | 18 ++++++++++--------
 2 files changed, 16 insertions(+), 8 deletions(-)
MAINTAINERS +6 -0
index 2db49ea7ae55..8ef5667a1d98 100644
@@ -10479,6 +10479,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml
 F:	drivers/video/backlight/ktd253-backlight.c
 
+KTD2692 FLASH LED DRIVER
+M:	Markuss Broks <[email protected]>
+S:	Maintained
+F:	Documentation/devicetree/bindings/leds/backlight/kinetic,ktd2692.yaml
+F:	drivers/leds/flash/leds-ktd2692.yaml
+
 KTEST
 M:	Steven Rostedt <[email protected]>
 M:	John Hawley <[email protected]>
drivers/leds/flash/leds-ktd2692.c +10 -8
index f341da1503a4..f30c4b11c84b 100644
@@ -163,7 +163,8 @@ static int ktd2692_led_brightness_set(struct led_classdev *led_cdev,
 
 	if (brightness == LED_OFF) {
 		led->mode = KTD2692_MODE_DISABLE;
-		gpiod_direction_output(led->aux_gpio, KTD2692_LOW);
+		if (led->aux_gpio)
+			gpiod_direction_output(led->aux_gpio, KTD2692_LOW);
 	} else {
 		ktd2692_expresswire_write(led, brightness |
 					KTD2692_REG_MOVIE_CURRENT_BASE);
@@ -191,10 +192,12 @@ static int ktd2692_led_flash_strobe_set(struct led_classdev_flash *fled_cdev,
 				| KTD2692_REG_FLASH_TIMEOUT_BASE);
 
 		led->mode = KTD2692_MODE_FLASH;
-		gpiod_direction_output(led->aux_gpio, KTD2692_HIGH);
+		if (led->aux_gpio)
+			gpiod_direction_output(led->aux_gpio, KTD2692_HIGH);
 	} else {
 		led->mode = KTD2692_MODE_DISABLE;
-		gpiod_direction_output(led->aux_gpio, KTD2692_LOW);
+		if (led->aux_gpio)
+			gpiod_direction_output(led->aux_gpio, KTD2692_LOW);
 	}
 
 	ktd2692_expresswire_write(led, led->mode | KTD2692_REG_MODE_BASE);
@@ -248,7 +251,8 @@ static void ktd2692_setup(struct ktd2692_context *led)
 {
 	led->mode = KTD2692_MODE_DISABLE;
 	ktd2692_expresswire_reset(led);
-	gpiod_direction_output(led->aux_gpio, KTD2692_LOW);
+	if (led->aux_gpio)
+		gpiod_direction_output(led->aux_gpio, KTD2692_LOW);
 
 	ktd2692_expresswire_write(led, (KTD2692_MM_MIN_CURR_THRESHOLD_SCALE - 1)
 				 | KTD2692_REG_MM_MIN_CURR_THRESHOLD_BASE);
@@ -286,10 +290,8 @@ static int ktd2692_parse_dt(struct ktd2692_context *led, struct device *dev,
 
 	led->aux_gpio = devm_gpiod_get(dev, "aux", GPIOD_ASIS);
 	ret = PTR_ERR_OR_ZERO(led->aux_gpio);
-	if (ret) {
-		dev_err(dev, "cannot get aux-gpios %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		dev_info(dev, "aux-gpios not available, flash mode current might be reduced\n");
 
 	led->regulator = devm_regulator_get(dev, "vin");
 	if (IS_ERR(led->regulator))
Signature · 1 line
2.35.1
#4 lore raw

Re: [PATCH 2/2] leds: ktd2692: Make aux-gpios optional

Quoted 5 lines · Markuss Broks

Make the AUX pin optional, since it isn't a core part of functionality, and the device is designed to be operational with only one CTRL pin.

Also pick up maintainership for the LED driver and the yaml bindings.

maintenance?

...

drivers/leds/flash/leds-ktd2692.c static int ktd2692_led_brightness_set(struct led_classdev *led_cdev, · line 167

Quoted 3 lines
drivers/leds/flash/leds-ktd2692.c +2 -1
-               gpiod_direction_output(led->aux_gpio, KTD2692_LOW);
+               if (led->aux_gpio)
+                       gpiod_direction_output(led->aux_gpio, KTD2692_LOW);

Redundant change.

...

drivers/leds/flash/leds-ktd2692.c static int ktd2692_led_flash_strobe_set(struct led_classdev_flash *fled_cdev, · line 196

Quoted 3 lines
drivers/leds/flash/leds-ktd2692.c +2 -1
-               gpiod_direction_output(led->aux_gpio, KTD2692_HIGH);
+               if (led->aux_gpio)
+                       gpiod_direction_output(led->aux_gpio, KTD2692_HIGH);

Ditto.

...

drivers/leds/flash/leds-ktd2692.c static int ktd2692_led_brightness_set(struct led_classdev *led_cdev, · line 167

Quoted 3 lines
drivers/leds/flash/leds-ktd2692.c +2 -1
-               gpiod_direction_output(led->aux_gpio, KTD2692_LOW);
+               if (led->aux_gpio)
+                       gpiod_direction_output(led->aux_gpio, KTD2692_LOW);

Ditto.

...

drivers/leds/flash/leds-ktd2692.c static int ktd2692_led_brightness_set(struct led_classdev *led_cdev, · line 167

Quoted 3 lines
drivers/leds/flash/leds-ktd2692.c +2 -1
-       gpiod_direction_output(led->aux_gpio, KTD2692_LOW);
+       if (led->aux_gpio)
+               gpiod_direction_output(led->aux_gpio, KTD2692_LOW);

Ditto.

...

drivers/leds/flash/leds-ktd2692.c static int ktd2692_parse_dt(struct ktd2692_context *led, struct device *dev, · line 294

Quoted 7 lines
        ret = PTR_ERR_OR_ZERO(led->aux_gpio);
drivers/leds/flash/leds-ktd2692.c +2 -4
-       if (ret) {
-               dev_err(dev, "cannot get aux-gpios %d\n", ret);
-               return ret;
-       }
+       if (ret)
+               dev_info(dev, "aux-gpios not available, flash mode current might be reduced\n");

dev_warn()?

After this change the use of PTR_ERR_OR_ZERO() becomes unjustified, what about

        if (IS_ERR(led->aux_gpio))

?

But the problem here is the ignorance of important error codes, such a deferred probe.

What you need is to switch to devm_gpiod_get_optional().

Signature · 2 lines
With Best Regards,
Andy Shevchenko