[PATCH v8 0/2] Make AUX gpio pin optional for ktd2692

[PATCH v8 0/2] Make AUX gpio pin optional for ktd2692

[Markuss Broks]

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 the amount of current going through the LED.

Also convert the txt device-tree bindings to yaml.

v2:
- fix the dt_binding_check
v3:
- set the aux_gpio to NULL to avoid passing ERR_PTR as a gpio
v4:
- maintainership -> maintenance (description)
- remove the if (led->aux_gpio)
- use devm_gpiod_get_optional for aux gpio
v5:
- use ret to pass a correct error return code (Christophe)
v6:
- use PTR_ERR properly (when IS_ERR is true) (Christophe)
v6-a:
- ended up being a mess, corrected-v2 should have proper tags now
v7:
- drop the MAINTAINERS part
v8:
- introduce a patch changing dev_err in gpio access to dev_err_probe

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

-- 
2.35.1

[PATCH v8 1/3] leds: ktd2692: Avoid duplicate error messages on probe deferral

[Markuss Broks]

Use dev_err_probe instead of dev_err to avoid duplicate error
messages if the GPIO allocation makes the probe defer.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 drivers/leds/flash/leds-ktd2692.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/leds/flash/leds-ktd2692.c b/drivers/leds/flash/leds-ktd2692.c
index ed1f20a58bf6..8b99742cb68a 100644
--- a/drivers/leds/flash/leds-ktd2692.c
+++ b/drivers/leds/flash/leds-ktd2692.c
@@ -279,17 +279,13 @@ static int ktd2692_parse_dt(struct ktd2692_context *led, struct device *dev,
 
 	led->ctrl_gpio = devm_gpiod_get(dev, "ctrl", GPIOD_ASIS);
 	ret = PTR_ERR_OR_ZERO(led->ctrl_gpio);
-	if (ret) {
-		dev_err(dev, "cannot get ctrl-gpios %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "cannot get ctrl-gpios\n");
 
 	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)
+		return dev_err_probe(dev, PTR_ERR(led->aux_gpio), "cannot get aux-gpios\n");
 
 	led->regulator = devm_regulator_get(dev, "vin");
 	if (IS_ERR(led->regulator))
-- 
2.35.1

Re: [PATCH v8 1/3] leds: ktd2692: Avoid duplicate error messages on probe deferral

[Andy Shevchenko]

On Thu, May 5, 2022 at 5:25 PM Markuss Broks <markuss.broks@gmail.com> wrote:
>
> Use dev_err_probe instead of dev_err to avoid duplicate error
> messages if the GPIO allocation makes the probe defer.

Thanks!

There are two improvements we can make here.
1) adding a Fixes tag, so it can be backported to stable kernels;
2) see below.

...

>         led->ctrl_gpio = devm_gpiod_get(dev, "ctrl", GPIOD_ASIS);
>         ret = PTR_ERR_OR_ZERO(led->ctrl_gpio);
> -       if (ret) {
> -               dev_err(dev, "cannot get ctrl-gpios %d\n", ret);
> -               return ret;
> -       }
> +       if (ret)
> +               return dev_err_probe(dev, ret, "cannot get ctrl-gpios\n");

You may improve this and simultaneously prepare for the next change to
be smaller

       led->ctrl_gpio = devm_gpiod_get(dev, "ctrl", GPIOD_ASIS);
       if (IS_ERR(led->ctrl_gpio))
               return dev_err_probe(dev, PTR_ERR(led->ctrl_gpio),
"cannot get ctrl-gpios\n");

All the same for the other case.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 1/3] leds: ktd2692: Avoid duplicate error messages on probe deferral

[Andy Shevchenko]

On Thu, May 5, 2022 at 7:29 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, May 5, 2022 at 5:25 PM Markuss Broks <markuss.broks@gmail.com> wrote:

...

> 1) adding a Fixes tag, so it can be backported to stable kernels;

To avoid additional versions in case you don't know how Fixes tag
should look like:

Fixes: b7da8c5c725c ("leds: Add ktd2692 flash LED driver")

Just add this before other tags, like SoB or Reviewed-by (no blank
lines in the tag block).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 1/3] leds: ktd2692: Avoid duplicate error messages on probe deferral

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 458 bytes --]

Hi!

> > Use dev_err_probe instead of dev_err to avoid duplicate error
> > messages if the GPIO allocation makes the probe defer.
> 
> Thanks!
> 
> There are two improvements we can make here.
> 1) adding a Fixes tag, so it can be backported to stable kernels;
> 2) see below.

I don't believe this is severe-enough error to be worth stable.

Best regards,
								Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH v8 1/3] leds: ktd2692: Avoid duplicate error messages on probe deferral

[Andy Shevchenko]

On Sat, May 7, 2022 at 11:14 PM Pavel Machek <pavel@ucw.cz> wrote:
>
> Hi!
>
> > > Use dev_err_probe instead of dev_err to avoid duplicate error
> > > messages if the GPIO allocation makes the probe defer.
> >
> > Thanks!
> >
> > There are two improvements we can make here.
> > 1) adding a Fixes tag, so it can be backported to stable kernels;
> > 2) see below.
>
> I don't believe this is severe-enough error to be worth stable.

Fixes tag is good to have in either case, but I agree that is not so critical.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v8 2/3] dt-bindings: leds: convert ktd2692 bindings to yaml

[Markuss Broks]

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

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 .../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

diff --git a/Documentation/devicetree/bindings/leds/kinetic,ktd2692.yaml b/Documentation/devicetree/bindings/leds/kinetic,ktd2692.yaml
new file mode 100644
index 000000000000..bac95a51afa1
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/kinetic,ktd2692.yaml
@@ -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 <markuss.broks@gmail.com>
+
+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:
+    description: 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:
+        maximum: 300000
+        description: Flash LED maximum current
+          Formula - I(uA) = 15000000 / 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>;
+      };
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/leds/leds-ktd2692.txt b/Documentation/devicetree/bindings/leds/leds-ktd2692.txt
deleted file mode 100644
index 853737452580..000000000000
--- a/Documentation/devicetree/bindings/leds/leds-ktd2692.txt
+++ /dev/null
@@ -1,50 +0,0 @@
-* Kinetic Technologies - KTD2692 Flash LED Driver
-
-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.
-
-Required properties:
-- compatible : Should be "kinetic,ktd2692".
-- ctrl-gpios : Specifier of the GPIO connected to CTRL pin.
-- aux-gpios : Specifier of the GPIO connected to AUX pin.
-
-Optional properties:
-- vin-supply : "vin" LED supply (2.7V to 5.5V).
-  See Documentation/devicetree/bindings/regulator/regulator.txt
-
-A discrete LED element connected to the device must be represented by a child
-node - See Documentation/devicetree/bindings/leds/common.txt
-
-Required properties for flash LED child nodes:
-  See Documentation/devicetree/bindings/leds/common.txt
-- led-max-microamp : Minimum Threshold for Timer protection
-  is defined internally (Maximum 300mA).
-- flash-max-microamp : Flash LED maximum current
-  Formula : I(mA) = 15000 / Rset.
-- flash-max-timeout-us : Flash LED maximum timeout.
-
-Optional properties for flash LED child nodes:
-- label : See Documentation/devicetree/bindings/leds/common.txt
-
-Example:
-
-ktd2692 {
-	compatible = "kinetic,ktd2692";
-	ctrl-gpios = <&gpc0 1 0>;
-	aux-gpios = <&gpc0 2 0>;
-	vin-supply = <&vbat>;
-
-	flash-led {
-		label = "ktd2692-flash";
-		led-max-microamp = <300000>;
-		flash-max-microamp = <1500000>;
-		flash-max-timeout-us = <1835000>;
-	};
-};
-- 
2.35.1

[PATCH v8 3/3] leds: ktd2692: Make aux-gpios optional

[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.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 drivers/leds/flash/leds-ktd2692.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/flash/leds-ktd2692.c b/drivers/leds/flash/leds-ktd2692.c
index 8b99742cb68a..670f3bf2e906 100644
--- a/drivers/leds/flash/leds-ktd2692.c
+++ b/drivers/leds/flash/leds-ktd2692.c
@@ -282,9 +282,8 @@ static int ktd2692_parse_dt(struct ktd2692_context *led, struct device *dev,
 	if (ret)
 		return dev_err_probe(dev, ret, "cannot get ctrl-gpios\n");
 
-	led->aux_gpio = devm_gpiod_get(dev, "aux", GPIOD_ASIS);
-	ret = PTR_ERR_OR_ZERO(led->aux_gpio);
-	if (ret)
+	led->aux_gpio = devm_gpiod_get_optional(dev, "aux", GPIOD_ASIS);
+	if (IS_ERR(led->aux_gpio))
 		return dev_err_probe(dev, PTR_ERR(led->aux_gpio), "cannot get aux-gpios\n");
 
 	led->regulator = devm_regulator_get(dev, "vin");
-- 
2.35.1

Re: [PATCH v8 0/2] Make AUX gpio pin optional for ktd2692

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 567 bytes --]

Hi!

> 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 the amount of current going through the LED.
> 
> Also convert the txt device-tree bindings to yaml.

Thank you, applied.

Best regards,
								Pavel
								
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]