[PATCH v2 0/2] Add sgm3140 flash led driver

[PATCH v2 0/2] Add sgm3140 flash led driver

[Luca Weiss]

This series introduces a driver for the SGMICRO SGM3140 charge pump
used in the PinePhone smartphone.

Luca Weiss (2):
  dt-bindings: leds: Add binding for sgm3140
  leds: add sgm3140 driver

 .../bindings/leds/leds-sgm3140.yaml           |  61 ++++
 drivers/leds/Kconfig                          |   9 +
 drivers/leds/Makefile                         |   1 +
 drivers/leds/leds-sgm3140.c                   | 317 ++++++++++++++++++
 4 files changed, 388 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-sgm3140.yaml
 create mode 100644 drivers/leds/leds-sgm3140.c

-- 
2.26.0

[PATCH v2 1/2] dt-bindings: leds: Add binding for sgm3140

[Luca Weiss]

Add YAML devicetree binding for SGMICRO SGM3140 charge pump used for
camera flash LEDs.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
Changes since v1:
- Add vin-supply
- Add led subnode (common.yaml)

 .../bindings/leds/leds-sgm3140.yaml           | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-sgm3140.yaml

diff --git a/Documentation/devicetree/bindings/leds/leds-sgm3140.yaml b/Documentation/devicetree/bindings/leds/leds-sgm3140.yaml
new file mode 100644
index 000000000000..24ca178e5d0a
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-sgm3140.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-sgm3140.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SGMICRO SGM3140 500mA Buck/Boost Charge Pump LED Driver
+
+maintainers:
+  - Luca Weiss <luca@z3ntu.xyz>
+
+description: |
+  The SGM3140 is a current-regulated charge pump which can regulate two current
+  levels for Flash and Torch modes.
+
+  The data sheet can be found at:
+    http://www.sg-micro.com/uploads/soft/20190626/1561535688.pdf
+
+properties:
+  compatible:
+    const: sgmicro,sgm3140
+
+  enable-gpios:
+    maxItems: 1
+    description: A connection to the 'EN' pin.
+
+  flash-gpios:
+    maxItems: 1
+    description: A connection to the 'FLASH' pin.
+
+  vin-supply:
+    description: Regulator providing power to the 'VIN' pin.
+
+  led:
+    allOf:
+      - $ref: common.yaml#
+
+required:
+  - compatible
+  - flash-gpios
+  - enable-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/leds/common.h>
+
+    sgm3140 {
+        compatible = "sgmicro,sgm3140";
+        flash-gpios = <&pio 3 24 GPIO_ACTIVE_HIGH>; /* PD24 */
+        enable-gpios = <&pio 2 3 GPIO_ACTIVE_HIGH>; /* PC3 */
+        vin-supply = <&reg_dcdc1>;
+
+        sgm3140_flash: led {
+            function = LED_FUNCTION_FLASH;
+            color = <LED_COLOR_ID_WHITE>;
+            flash-max-timeout-us = <250000>;
+        };
+    };
-- 
2.26.0

Re: [PATCH v2 1/2] dt-bindings: leds: Add binding for sgm3140

[Dan Murphy]

Luca

On 3/30/20 2:47 PM, Luca Weiss wrote:
> Add YAML devicetree binding for SGMICRO SGM3140 charge pump used for
> camera flash LEDs.
>
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> ---
> Changes since v1:
> - Add vin-supply
> - Add led subnode (common.yaml)
>
>   .../bindings/leds/leds-sgm3140.yaml           | 61 +++++++++++++++++++
>   1 file changed, 61 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/leds/leds-sgm3140.yaml
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-sgm3140.yaml b/Documentation/devicetree/bindings/leds/leds-sgm3140.yaml
> new file mode 100644
> index 000000000000..24ca178e5d0a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-sgm3140.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/leds-sgm3140.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SGMICRO SGM3140 500mA Buck/Boost Charge Pump LED Driver
> +
> +maintainers:
> +  - Luca Weiss <luca@z3ntu.xyz>
> +
> +description: |
> +  The SGM3140 is a current-regulated charge pump which can regulate two current
> +  levels for Flash and Torch modes.
> +
> +  The data sheet can be found at:
> +    http://www.sg-micro.com/uploads/soft/20190626/1561535688.pdf
> +
> +properties:
> +  compatible:
> +    const: sgmicro,sgm3140
> +
> +  enable-gpios:
> +    maxItems: 1
> +    description: A connection to the 'EN' pin.
> +
> +  flash-gpios:
> +    maxItems: 1
> +    description: A connection to the 'FLASH' pin.
> +
> +  vin-supply:
> +    description: Regulator providing power to the 'VIN' pin.
> +
> +  led:
> +    allOf:
> +      - $ref: common.yaml#
> +
> +required:
> +  - compatible
> +  - flash-gpios
> +  - enable-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/leds/common.h>
> +
> +    sgm3140 {
> +        compatible = "sgmicro,sgm3140";
> +        flash-gpios = <&pio 3 24 GPIO_ACTIVE_HIGH>; /* PD24 */
> +        enable-gpios = <&pio 2 3 GPIO_ACTIVE_HIGH>; /* PC3 */
> +        vin-supply = <&reg_dcdc1>;
> +
> +        sgm3140_flash: led {
> +            function = LED_FUNCTION_FLASH;
> +            color = <LED_COLOR_ID_WHITE>;
> +            flash-max-timeout-us = <250000>;
> +        };
> +    };


Reviewed-by: Dan Murphy <dmurphy@ti.com>

Re: [PATCH v2 1/2] dt-bindings: leds: Add binding for sgm3140

[Rob Herring]

On Mon, Mar 30, 2020 at 09:47:56PM +0200, Luca Weiss wrote:
> Add YAML devicetree binding for SGMICRO SGM3140 charge pump used for
> camera flash LEDs.
> 
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> ---
> Changes since v1:
> - Add vin-supply
> - Add led subnode (common.yaml)
> 
>  .../bindings/leds/leds-sgm3140.yaml           | 61 +++++++++++++++++++
>  1 file changed, 61 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-sgm3140.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-sgm3140.yaml b/Documentation/devicetree/bindings/leds/leds-sgm3140.yaml
> new file mode 100644
> index 000000000000..24ca178e5d0a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-sgm3140.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/leds-sgm3140.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SGMICRO SGM3140 500mA Buck/Boost Charge Pump LED Driver
> +
> +maintainers:
> +  - Luca Weiss <luca@z3ntu.xyz>
> +
> +description: |
> +  The SGM3140 is a current-regulated charge pump which can regulate two current
> +  levels for Flash and Torch modes.
> +
> +  The data sheet can be found at:
> +    http://www.sg-micro.com/uploads/soft/20190626/1561535688.pdf
> +
> +properties:
> +  compatible:
> +    const: sgmicro,sgm3140
> +
> +  enable-gpios:
> +    maxItems: 1
> +    description: A connection to the 'EN' pin.
> +
> +  flash-gpios:
> +    maxItems: 1
> +    description: A connection to the 'FLASH' pin.
> +
> +  vin-supply:
> +    description: Regulator providing power to the 'VIN' pin.
> +
> +  led:

Needs 'type: object'

With that,

Reviewed-by: Rob Herring <robh@kernel.org>

> +    allOf:
> +      - $ref: common.yaml#
> +
> +required:
> +  - compatible
> +  - flash-gpios
> +  - enable-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/leds/common.h>
> +
> +    sgm3140 {
> +        compatible = "sgmicro,sgm3140";
> +        flash-gpios = <&pio 3 24 GPIO_ACTIVE_HIGH>; /* PD24 */
> +        enable-gpios = <&pio 2 3 GPIO_ACTIVE_HIGH>; /* PC3 */
> +        vin-supply = <&reg_dcdc1>;
> +
> +        sgm3140_flash: led {
> +            function = LED_FUNCTION_FLASH;
> +            color = <LED_COLOR_ID_WHITE>;
> +            flash-max-timeout-us = <250000>;
> +        };
> +    };
> -- 
> 2.26.0
> 

[PATCH v2 2/2] leds: add sgm3140 driver

[Luca Weiss]

Add a driver for the SGMICRO SGM3140 Buck/Boost Charge Pump LED driver.

This device is controlled by two GPIO pins, one for enabling and the
second one for switching between torch and flash mode.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
Changes since v1:
- Add vin-supply (keep track of 'enabled' state for that)
- Wrap lines
- static const -ify some structs and methods
- use strscpy instead of strlcpy
- remove u32 cast by adding 'U' suffix to constants
- rebase on linux-next

 drivers/leds/Kconfig        |   9 +
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-sgm3140.c | 317 ++++++++++++++++++++++++++++++++++++
 3 files changed, 327 insertions(+)
 create mode 100644 drivers/leds/leds-sgm3140.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 7599dbee8de1..f5beeff16bdd 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -871,6 +871,15 @@ config LEDS_IP30
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-ip30.
 
+config LEDS_SGM3140
+	tristate "LED support for the SGM3140"
+	depends on LEDS_CLASS_FLASH
+	depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
+	depends on OF
+	help
+	  This option enables support for the SGM3140 500mA Buck/Boost Charge
+	  Pump LED Driver.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index fd61421f7d40..f60ed0c09d4c 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_LEDS_PWM)			+= leds-pwm.o
 obj-$(CONFIG_LEDS_REGULATOR)		+= leds-regulator.o
 obj-$(CONFIG_LEDS_S3C24XX)		+= leds-s3c24xx.o
 obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
+obj-$(CONFIG_LEDS_SGM3140)		+= leds-sgm3140.o
 obj-$(CONFIG_LEDS_SUNFIRE)		+= leds-sunfire.o
 obj-$(CONFIG_LEDS_SYSCON)		+= leds-syscon.o
 obj-$(CONFIG_LEDS_TCA6507)		+= leds-tca6507.o
diff --git a/drivers/leds/leds-sgm3140.c b/drivers/leds/leds-sgm3140.c
new file mode 100644
index 000000000000..28fe5e34f931
--- /dev/null
+++ b/drivers/leds/leds-sgm3140.c
@@ -0,0 +1,317 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2020 Luca Weiss <luca@z3ntu.xyz>
+
+#include <linux/gpio/consumer.h>
+#include <linux/led-class-flash.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regulator/consumer.h>
+#include <linux/platform_device.h>
+
+#include <media/v4l2-flash-led-class.h>
+
+#define FLASH_TIMEOUT_DEFAULT		250000U /* 250ms */
+#define FLASH_MAX_TIMEOUT_DEFAULT	300000U /* 300ms */
+
+struct sgm3140 {
+	bool enabled;
+	struct gpio_desc *flash_gpio;
+	struct gpio_desc *enable_gpio;
+	struct regulator *vin_regulator;
+
+	/* current timeout in us */
+	u32 timeout;
+	/* maximum timeout in us */
+	u32 max_timeout;
+
+	struct led_classdev_flash fled_cdev;
+	struct v4l2_flash *v4l2_flash;
+
+	struct timer_list powerdown_timer;
+};
+
+static struct sgm3140 *flcdev_to_sgm3140(struct led_classdev_flash *flcdev)
+{
+	return container_of(flcdev, struct sgm3140, fled_cdev);
+}
+
+static int sgm3140_strobe_set(struct led_classdev_flash *fled_cdev, bool state)
+{
+	struct sgm3140 *priv = flcdev_to_sgm3140(fled_cdev);
+	int ret;
+
+	if (priv->enabled == state)
+		return 0;
+
+	if (state) {
+		ret = regulator_enable(priv->vin_regulator);
+		if (ret) {
+			dev_err(fled_cdev->led_cdev.dev,
+				"failed to enable regulator: %d\n", ret);
+			return ret;
+		}
+		gpiod_set_value_cansleep(priv->flash_gpio, 1);
+		gpiod_set_value_cansleep(priv->enable_gpio, 1);
+		mod_timer(&priv->powerdown_timer,
+			  jiffies + usecs_to_jiffies(priv->timeout));
+	} else {
+		del_timer_sync(&priv->powerdown_timer);
+		gpiod_set_value_cansleep(priv->enable_gpio, 0);
+		gpiod_set_value_cansleep(priv->flash_gpio, 0);
+		ret = regulator_disable(priv->vin_regulator);
+		if (ret) {
+			dev_err(fled_cdev->led_cdev.dev,
+				"failed to disable regulator: %d\n", ret);
+			return ret;
+		}
+	}
+
+	priv->enabled = state;
+
+	return 0;
+}
+
+static int sgm3140_strobe_get(struct led_classdev_flash *fled_cdev, bool *state)
+{
+	struct sgm3140 *priv = flcdev_to_sgm3140(fled_cdev);
+
+	*state = timer_pending(&priv->powerdown_timer);
+
+	return 0;
+}
+
+static int sgm3140_timeout_set(struct led_classdev_flash *fled_cdev,
+			       u32 timeout)
+{
+	struct sgm3140 *priv = flcdev_to_sgm3140(fled_cdev);
+
+	priv->timeout = timeout;
+
+	return 0;
+}
+
+static const struct led_flash_ops sgm3140_flash_ops = {
+	.strobe_set = sgm3140_strobe_set,
+	.strobe_get = sgm3140_strobe_get,
+	.timeout_set = sgm3140_timeout_set,
+};
+
+static int sgm3140_brightness_set(struct led_classdev *led_cdev,
+				  enum led_brightness brightness)
+{
+	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
+	struct sgm3140 *priv = flcdev_to_sgm3140(fled_cdev);
+	bool enable = brightness == LED_ON;
+	int ret;
+
+	if (priv->enabled == enable)
+		return 0;
+
+	if (enable) {
+		ret = regulator_enable(priv->vin_regulator);
+		if (ret) {
+			dev_err(led_cdev->dev,
+				"failed to enable regulator: %d\n", ret);
+			return ret;
+		}
+		gpiod_set_value_cansleep(priv->enable_gpio, 1);
+	} else {
+		gpiod_set_value_cansleep(priv->enable_gpio, 0);
+		ret = regulator_disable(priv->vin_regulator);
+		if (ret) {
+			dev_err(led_cdev->dev,
+				"failed to disable regulator: %d\n", ret);
+			return ret;
+		}
+	}
+
+	priv->enabled = enable;
+
+	return 0;
+}
+
+static void sgm3140_powerdown_timer(struct timer_list *t)
+{
+	struct sgm3140 *priv = from_timer(priv, t, powerdown_timer);
+
+	gpiod_set_value(priv->enable_gpio, 0);
+	gpiod_set_value(priv->flash_gpio, 0);
+	regulator_disable(priv->vin_regulator);
+
+	priv->enabled = false;
+}
+
+static void sgm3140_init_flash_timeout(struct sgm3140 *priv)
+{
+	struct led_classdev_flash *fled_cdev = &priv->fled_cdev;
+	struct led_flash_setting *s;
+
+	/* Init flash timeout setting */
+	s = &fled_cdev->timeout;
+	s->min = 1;
+	s->max = priv->max_timeout;
+	s->step = 1;
+	s->val = FLASH_TIMEOUT_DEFAULT;
+}
+
+#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
+static void sgm3140_init_v4l2_flash_config(struct sgm3140 *priv,
+					struct v4l2_flash_config *v4l2_sd_cfg)
+{
+	struct led_classdev *led_cdev = &priv->fled_cdev.led_cdev;
+	struct led_flash_setting *s;
+
+	strscpy(v4l2_sd_cfg->dev_name, led_cdev->dev->kobj.name,
+		sizeof(v4l2_sd_cfg->dev_name));
+
+	/* Init flash intensity setting */
+	s = &v4l2_sd_cfg->intensity;
+	s->min = 0;
+	s->max = 1;
+	s->step = 1;
+	s->val = 1;
+}
+
+#else
+static void sgm3140_init_v4l2_flash_config(struct sgm3140 *priv,
+					struct v4l2_flash_config *v4l2_sd_cfg)
+{
+}
+#endif
+
+static int sgm3140_probe(struct platform_device *pdev)
+{
+	struct sgm3140 *priv;
+	struct led_classdev *led_cdev;
+	struct led_classdev_flash *fled_cdev;
+	struct led_init_data init_data = {};
+	struct device_node *child_node;
+	struct v4l2_flash_config v4l2_sd_cfg = {};
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->flash_gpio = devm_gpiod_get(&pdev->dev, "flash", GPIOD_OUT_LOW);
+	ret = PTR_ERR_OR_ZERO(priv->flash_gpio);
+	if (ret) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev,
+				"Failed to request flash gpio: %d\n", ret);
+		return ret;
+	}
+
+	priv->enable_gpio = devm_gpiod_get(&pdev->dev, "enable", GPIOD_OUT_LOW);
+	ret = PTR_ERR_OR_ZERO(priv->enable_gpio);
+	if (ret) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev,
+				"Failed to request enable gpio: %d\n", ret);
+		return ret;
+	}
+
+	priv->vin_regulator = devm_regulator_get(&pdev->dev, "vin");
+	ret = PTR_ERR_OR_ZERO(priv->vin_regulator);
+	if (ret) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev,
+				"Failed to request regulator: %d\n", ret);
+		return ret;
+	}
+
+	child_node = of_get_next_available_child(pdev->dev.of_node, NULL);
+	if (!child_node) {
+		dev_err(&pdev->dev,
+			"No DT child node found for connected LED.\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32(child_node, "flash-max-timeout-us",
+				   &priv->max_timeout);
+	if (ret) {
+		priv->max_timeout = FLASH_MAX_TIMEOUT_DEFAULT;
+		dev_warn(&pdev->dev,
+			 "flash-max-timeout-us DT property missing\n");
+	}
+
+	/*
+	 * Set default timeout to FLASH_DEFAULT_TIMEOUT except if max_timeout
+	 * from DT is lower.
+	 */
+	priv->timeout = min(priv->max_timeout, FLASH_TIMEOUT_DEFAULT);
+
+	timer_setup(&priv->powerdown_timer, sgm3140_powerdown_timer, 0);
+
+	fled_cdev = &priv->fled_cdev;
+	led_cdev = &fled_cdev->led_cdev;
+
+	fled_cdev->ops = &sgm3140_flash_ops;
+
+	led_cdev->brightness_set_blocking = sgm3140_brightness_set;
+	led_cdev->max_brightness = LED_ON;
+	led_cdev->flags |= LED_DEV_CAP_FLASH;
+
+	sgm3140_init_flash_timeout(priv);
+
+	init_data.fwnode = of_fwnode_handle(child_node);
+
+	platform_set_drvdata(pdev, priv);
+
+	/* Register in the LED subsystem */
+	ret = devm_led_classdev_flash_register_ext(&pdev->dev,
+						   fled_cdev, &init_data);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register flash device: %d\n",
+			ret);
+		goto err;
+	}
+
+	sgm3140_init_v4l2_flash_config(priv, &v4l2_sd_cfg);
+
+	/* Create V4L2 Flash subdev */
+	priv->v4l2_flash = v4l2_flash_init(&pdev->dev,
+					   of_fwnode_handle(child_node),
+					   fled_cdev, NULL,
+					   &v4l2_sd_cfg);
+	if (IS_ERR(priv->v4l2_flash)) {
+		ret = PTR_ERR(priv->v4l2_flash);
+		goto err;
+	}
+
+err:
+	of_node_put(child_node);
+	return ret;
+}
+
+static int sgm3140_remove(struct platform_device *pdev)
+{
+	struct sgm3140 *priv = platform_get_drvdata(pdev);
+
+	del_timer_sync(&priv->powerdown_timer);
+
+	v4l2_flash_release(priv->v4l2_flash);
+
+	return 0;
+}
+
+static const struct of_device_id sgm3140_dt_match[] = {
+	{ .compatible = "sgmicro,sgm3140" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, sgm3140_dt_match);
+
+static struct platform_driver sgm3140_driver = {
+	.probe	= sgm3140_probe,
+	.remove	= sgm3140_remove,
+	.driver	= {
+		.name	= "sgm3140",
+		.of_match_table = sgm3140_dt_match,
+	},
+};
+
+module_platform_driver(sgm3140_driver);
+
+MODULE_AUTHOR("Luca Weiss <luca@z3ntu.xyz>");
+MODULE_DESCRIPTION("SG Micro SGM3140 charge pump led driver");
+MODULE_LICENSE("GPL v2");
-- 
2.26.0

Re: [PATCH v2 2/2] leds: add sgm3140 driver

[Dan Murphy]

Luca

On 3/30/20 2:47 PM, Luca Weiss wrote:
> Add a driver for the SGMICRO SGM3140 Buck/Boost Charge Pump LED driver.
>
> This device is controlled by two GPIO pins, one for enabling and the
> second one for switching between torch and flash mode.
>
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> ---
> Changes since v1:
> - Add vin-supply (keep track of 'enabled' state for that)
> - Wrap lines
> - static const -ify some structs and methods
> - use strscpy instead of strlcpy
> - remove u32 cast by adding 'U' suffix to constants
> - rebase on linux-next
>
>   drivers/leds/Kconfig        |   9 +
>   drivers/leds/Makefile       |   1 +
>   drivers/leds/leds-sgm3140.c | 317 ++++++++++++++++++++++++++++++++++++
>   3 files changed, 327 insertions(+)
>   create mode 100644 drivers/leds/leds-sgm3140.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 7599dbee8de1..f5beeff16bdd 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -871,6 +871,15 @@ config LEDS_IP30
>   	  To compile this driver as a module, choose M here: the module
>   	  will be called leds-ip30.
>   
> +config LEDS_SGM3140
> +	tristate "LED support for the SGM3140"
> +	depends on LEDS_CLASS_FLASH
> +	depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
> +	depends on OF
> +	help
> +	  This option enables support for the SGM3140 500mA Buck/Boost Charge
> +	  Pump LED Driver.
> +
>   comment "LED Triggers"
>   source "drivers/leds/trigger/Kconfig"
>   
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index fd61421f7d40..f60ed0c09d4c 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -77,6 +77,7 @@ obj-$(CONFIG_LEDS_PWM)			+= leds-pwm.o
>   obj-$(CONFIG_LEDS_REGULATOR)		+= leds-regulator.o
>   obj-$(CONFIG_LEDS_S3C24XX)		+= leds-s3c24xx.o
>   obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
> +obj-$(CONFIG_LEDS_SGM3140)		+= leds-sgm3140.o
>   obj-$(CONFIG_LEDS_SUNFIRE)		+= leds-sunfire.o
>   obj-$(CONFIG_LEDS_SYSCON)		+= leds-syscon.o
>   obj-$(CONFIG_LEDS_TCA6507)		+= leds-tca6507.o
> diff --git a/drivers/leds/leds-sgm3140.c b/drivers/leds/leds-sgm3140.c
> new file mode 100644
> index 000000000000..28fe5e34f931
> --- /dev/null
> +++ b/drivers/leds/leds-sgm3140.c
> @@ -0,0 +1,317 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2020 Luca Weiss <luca@z3ntu.xyz>
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/led-class-flash.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/platform_device.h>
> +
> +#include <media/v4l2-flash-led-class.h>
> +
> +#define FLASH_TIMEOUT_DEFAULT		250000U /* 250ms */
> +#define FLASH_MAX_TIMEOUT_DEFAULT	300000U /* 300ms */
> +
> +struct sgm3140 {
> +	bool enabled;
> +	struct gpio_desc *flash_gpio;
> +	struct gpio_desc *enable_gpio;
> +	struct regulator *vin_regulator;
> +
> +	/* current timeout in us */
> +	u32 timeout;
> +	/* maximum timeout in us */
> +	u32 max_timeout;
> +
> +	struct led_classdev_flash fled_cdev;
> +	struct v4l2_flash *v4l2_flash;
> +
> +	struct timer_list powerdown_timer;
> +};
> +
> +static struct sgm3140 *flcdev_to_sgm3140(struct led_classdev_flash *flcdev)
> +{
> +	return container_of(flcdev, struct sgm3140, fled_cdev);
> +}
> +
> +static int sgm3140_strobe_set(struct led_classdev_flash *fled_cdev, bool state)
> +{
> +	struct sgm3140 *priv = flcdev_to_sgm3140(fled_cdev);
> +	int ret;
> +
> +	if (priv->enabled == state)
> +		return 0;
> +
> +	if (state) {
> +		ret = regulator_enable(priv->vin_regulator);
> +		if (ret) {
> +			dev_err(fled_cdev->led_cdev.dev,
> +				"failed to enable regulator: %d\n", ret);
> +			return ret;
> +		}
> +		gpiod_set_value_cansleep(priv->flash_gpio, 1);
> +		gpiod_set_value_cansleep(priv->enable_gpio, 1);
> +		mod_timer(&priv->powerdown_timer,
> +			  jiffies + usecs_to_jiffies(priv->timeout));
> +	} else {
> +		del_timer_sync(&priv->powerdown_timer);
> +		gpiod_set_value_cansleep(priv->enable_gpio, 0);
> +		gpiod_set_value_cansleep(priv->flash_gpio, 0);
> +		ret = regulator_disable(priv->vin_regulator);
> +		if (ret) {
> +			dev_err(fled_cdev->led_cdev.dev,
> +				"failed to disable regulator: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	priv->enabled = state;
> +
> +	return 0;
> +}
> +
> +static int sgm3140_strobe_get(struct led_classdev_flash *fled_cdev, bool *state)
> +{
> +	struct sgm3140 *priv = flcdev_to_sgm3140(fled_cdev);
> +
> +	*state = timer_pending(&priv->powerdown_timer);
> +
> +	return 0;
> +}
> +
> +static int sgm3140_timeout_set(struct led_classdev_flash *fled_cdev,
> +			       u32 timeout)
> +{
> +	struct sgm3140 *priv = flcdev_to_sgm3140(fled_cdev);
> +
> +	priv->timeout = timeout;
> +
> +	return 0;
> +}
> +
> +static const struct led_flash_ops sgm3140_flash_ops = {
> +	.strobe_set = sgm3140_strobe_set,
> +	.strobe_get = sgm3140_strobe_get,
> +	.timeout_set = sgm3140_timeout_set,
> +};
> +
> +static int sgm3140_brightness_set(struct led_classdev *led_cdev,
> +				  enum led_brightness brightness)
> +{
> +	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
> +	struct sgm3140 *priv = flcdev_to_sgm3140(fled_cdev);
> +	bool enable = brightness == LED_ON;
> +	int ret;
> +
> +	if (priv->enabled == enable)
> +		return 0;
> +
> +	if (enable) {
> +		ret = regulator_enable(priv->vin_regulator);
> +		if (ret) {
> +			dev_err(led_cdev->dev,
> +				"failed to enable regulator: %d\n", ret);
> +			return ret;
> +		}
> +		gpiod_set_value_cansleep(priv->enable_gpio, 1);
> +	} else {
> +		gpiod_set_value_cansleep(priv->enable_gpio, 0);
> +		ret = regulator_disable(priv->vin_regulator);
> +		if (ret) {
> +			dev_err(led_cdev->dev,
> +				"failed to disable regulator: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	priv->enabled = enable;
> +
> +	return 0;
> +}
> +
> +static void sgm3140_powerdown_timer(struct timer_list *t)
> +{
> +	struct sgm3140 *priv = from_timer(priv, t, powerdown_timer);
> +
> +	gpiod_set_value(priv->enable_gpio, 0);
> +	gpiod_set_value(priv->flash_gpio, 0);
> +	regulator_disable(priv->vin_regulator);
> +
> +	priv->enabled = false;
> +}
> +
> +static void sgm3140_init_flash_timeout(struct sgm3140 *priv)
> +{
> +	struct led_classdev_flash *fled_cdev = &priv->fled_cdev;
> +	struct led_flash_setting *s;
> +
> +	/* Init flash timeout setting */
> +	s = &fled_cdev->timeout;
> +	s->min = 1;
> +	s->max = priv->max_timeout;
> +	s->step = 1;
> +	s->val = FLASH_TIMEOUT_DEFAULT;
> +}
> +
> +#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
> +static void sgm3140_init_v4l2_flash_config(struct sgm3140 *priv,
> +					struct v4l2_flash_config *v4l2_sd_cfg)
> +{
> +	struct led_classdev *led_cdev = &priv->fled_cdev.led_cdev;
> +	struct led_flash_setting *s;
> +
> +	strscpy(v4l2_sd_cfg->dev_name, led_cdev->dev->kobj.name,
> +		sizeof(v4l2_sd_cfg->dev_name));
> +
> +	/* Init flash intensity setting */
> +	s = &v4l2_sd_cfg->intensity;
> +	s->min = 0;
> +	s->max = 1;
> +	s->step = 1;
> +	s->val = 1;
> +}
> +
> +#else
> +static void sgm3140_init_v4l2_flash_config(struct sgm3140 *priv,
> +					struct v4l2_flash_config *v4l2_sd_cfg)
> +{
> +}
> +#endif
> +
> +static int sgm3140_probe(struct platform_device *pdev)
> +{
> +	struct sgm3140 *priv;
> +	struct led_classdev *led_cdev;
> +	struct led_classdev_flash *fled_cdev;
> +	struct led_init_data init_data = {};
> +	struct device_node *child_node;
> +	struct v4l2_flash_config v4l2_sd_cfg = {};
> +	int ret;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->flash_gpio = devm_gpiod_get(&pdev->dev, "flash", GPIOD_OUT_LOW);
> +	ret = PTR_ERR_OR_ZERO(priv->flash_gpio);
> +	if (ret) {
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(&pdev->dev,
> +				"Failed to request flash gpio: %d\n", ret);
> +		return ret;
> +	}
> +
> +	priv->enable_gpio = devm_gpiod_get(&pdev->dev, "enable", GPIOD_OUT_LOW);
> +	ret = PTR_ERR_OR_ZERO(priv->enable_gpio);
> +	if (ret) {
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(&pdev->dev,
> +				"Failed to request enable gpio: %d\n", ret);
> +		return ret;
> +	}
> +
> +	priv->vin_regulator = devm_regulator_get(&pdev->dev, "vin");
> +	ret = PTR_ERR_OR_ZERO(priv->vin_regulator);
> +	if (ret) {
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(&pdev->dev,
> +				"Failed to request regulator: %d\n", ret);
> +		return ret;
This regulator is optional so why would you return here?  You should 
only return if -EPROBE_DEFER.
> +	}
> +
> +	child_node = of_get_next_available_child(pdev->dev.of_node, NULL);

Maybe this should be the first check before doing all the processing to 
make sure that the DT is not

malformed.

> +	if (!child_node) {
> +		dev_err(&pdev->dev,
> +			"No DT child node found for connected LED.\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32(child_node, "flash-max-timeout-us",
> +				   &priv->max_timeout);
> +	if (ret) {
> +		priv->max_timeout = FLASH_MAX_TIMEOUT_DEFAULT;
> +		dev_warn(&pdev->dev,
> +			 "flash-max-timeout-us DT property missing\n");
> +	}
> +
> +	/*
> +	 * Set default timeout to FLASH_DEFAULT_TIMEOUT except if max_timeout
> +	 * from DT is lower.
> +	 */
> +	priv->timeout = min(priv->max_timeout, FLASH_TIMEOUT_DEFAULT);
> +
> +	timer_setup(&priv->powerdown_timer, sgm3140_powerdown_timer, 0);
> +
> +	fled_cdev = &priv->fled_cdev;
> +	led_cdev = &fled_cdev->led_cdev;
> +
> +	fled_cdev->ops = &sgm3140_flash_ops;
> +
> +	led_cdev->brightness_set_blocking = sgm3140_brightness_set;
> +	led_cdev->max_brightness = LED_ON;
> +	led_cdev->flags |= LED_DEV_CAP_FLASH;
> +
> +	sgm3140_init_flash_timeout(priv);
> +
> +	init_data.fwnode = of_fwnode_handle(child_node);
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	/* Register in the LED subsystem */
> +	ret = devm_led_classdev_flash_register_ext(&pdev->dev,
> +						   fled_cdev, &init_data);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to register flash device: %d\n",
> +			ret);
> +		goto err;
> +	}
> +
> +	sgm3140_init_v4l2_flash_config(priv, &v4l2_sd_cfg);
> +
> +	/* Create V4L2 Flash subdev */
> +	priv->v4l2_flash = v4l2_flash_init(&pdev->dev,
> +					   of_fwnode_handle(child_node),
> +					   fled_cdev, NULL,
> +					   &v4l2_sd_cfg);
> +	if (IS_ERR(priv->v4l2_flash)) {
> +		ret = PTR_ERR(priv->v4l2_flash);
> +		goto err;
Not sure why this is here you are not in a for loop and this will fall 
through anyway to the err label.
> +	}
> +
> +err:
> +	of_node_put(child_node);
> +	return ret;
> +}
> +
Dan

Re: [PATCH v2 2/2] leds: add sgm3140 driver

[Luca Weiss]

Hi Dan,

On Freitag, 3. April 2020 19:31:52 CEST Dan Murphy wrote:
> Luca
> 
> On 3/30/20 2:47 PM, Luca Weiss wrote:
> > Add a driver for the SGMICRO SGM3140 Buck/Boost Charge Pump LED driver.
> > 
> > This device is controlled by two GPIO pins, one for enabling and the
> > second one for switching between torch and flash mode.
> > 
> > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > ---
> > Changes since v1:
> > - Add vin-supply (keep track of 'enabled' state for that)
> > - Wrap lines
> > - static const -ify some structs and methods
> > - use strscpy instead of strlcpy
> > - remove u32 cast by adding 'U' suffix to constants
> > - rebase on linux-next
> > 
> >   drivers/leds/Kconfig        |   9 +
> >   drivers/leds/Makefile       |   1 +
> >   drivers/leds/leds-sgm3140.c | 317 ++++++++++++++++++++++++++++++++++++
> >   3 files changed, 327 insertions(+)
> >   create mode 100644 drivers/leds/leds-sgm3140.c
> > 
-snip-
> > +
> > +	priv->vin_regulator = devm_regulator_get(&pdev->dev, "vin");
> > +	ret = PTR_ERR_OR_ZERO(priv->vin_regulator);
> > +	if (ret) {
> > +		if (ret != -EPROBE_DEFER)
> > +			dev_err(&pdev->dev,
> > +				"Failed to request regulator: 
%d\n", ret);
> > +		return ret;
> 
> This regulator is optional so why would you return here?  You should
> only return if -EPROBE_DEFER.

If the regulator is not specified in the dts, then a dummy regulator will be 
used:

[    1.027114] sgm3140 sgm3140: sgm3140 supply vin not found, using dummy 
regulator

So this code will only be called if something really failed (or was defered)

> 
> > +	}
> > +
> > +	child_node = of_get_next_available_child(pdev->dev.of_node, NULL);
> 
> Maybe this should be the first check before doing all the processing to
> make sure that the DT is not
> 
> malformed.

If e.g. the devm_gpiod_get calls fail (because the gpios weren't declared in 
the dts) then the dt is also "malformed" which isn't as different as the 
subnode being missing imo. I don't think it matters much here. And this way I 
don't have to care about calling of_node_put in case of an error for the 
statements above.

> 
> > +	if (!child_node) {
> > +		dev_err(&pdev->dev,
> > +			"No DT child node found for connected LED.
\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = of_property_read_u32(child_node, "flash-max-timeout-us",
> > +				   &priv->max_timeout);
> > +	if (ret) {
> > +		priv->max_timeout = FLASH_MAX_TIMEOUT_DEFAULT;
> > +		dev_warn(&pdev->dev,
> > +			 "flash-max-timeout-us DT property 
missing\n");
> > +	}
> > +
> > +	/*
> > +	 * Set default timeout to FLASH_DEFAULT_TIMEOUT except if 
max_timeout
> > +	 * from DT is lower.
> > +	 */
> > +	priv->timeout = min(priv->max_timeout, FLASH_TIMEOUT_DEFAULT);
> > +
> > +	timer_setup(&priv->powerdown_timer, sgm3140_powerdown_timer, 0);
> > +
> > +	fled_cdev = &priv->fled_cdev;
> > +	led_cdev = &fled_cdev->led_cdev;
> > +
> > +	fled_cdev->ops = &sgm3140_flash_ops;
> > +
> > +	led_cdev->brightness_set_blocking = sgm3140_brightness_set;
> > +	led_cdev->max_brightness = LED_ON;
> > +	led_cdev->flags |= LED_DEV_CAP_FLASH;
> > +
> > +	sgm3140_init_flash_timeout(priv);
> > +
> > +	init_data.fwnode = of_fwnode_handle(child_node);
> > +
> > +	platform_set_drvdata(pdev, priv);
> > +
> > +	/* Register in the LED subsystem */
> > +	ret = devm_led_classdev_flash_register_ext(&pdev->dev,
> > +						   
fled_cdev, &init_data);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to register flash device: 
%d\n",
> > +			ret);
> > +		goto err;
> > +	}
> > +
> > +	sgm3140_init_v4l2_flash_config(priv, &v4l2_sd_cfg);
> > +
> > +	/* Create V4L2 Flash subdev */
> > +	priv->v4l2_flash = v4l2_flash_init(&pdev->dev,
> > +					   
of_fwnode_handle(child_node),
> > +					   fled_cdev, NULL,
> > +					   &v4l2_sd_cfg);
> > +	if (IS_ERR(priv->v4l2_flash)) {
> > +		ret = PTR_ERR(priv->v4l2_flash);
> > +		goto err;
> 
> Not sure why this is here you are not in a for loop and this will fall
> through anyway to the err label.
> 

I kept the goto in, in case more code is added below that statement so the 
author doesn't forget that this error needs to be handled.
If wanted I can remove it of course.

> > +	}
> > +
> > +err:
> > +	of_node_put(child_node);
> > +	return ret;
> > +}
> > +
> 
> Dan

Regards
Luca

Re: [PATCH v2 2/2] leds: add sgm3140 driver

[Dan Murphy]

Luca

On 4/4/20 4:36 AM, Luca Weiss wrote:
> + fled_cdev, NULL, 
<snip>
>>> +					   &v4l2_sd_cfg);
>>> +	if (IS_ERR(priv->v4l2_flash)) {
>>> +		ret = PTR_ERR(priv->v4l2_flash);
>>> +		goto err;
>> Not sure why this is here you are not in a for loop and this will fall
>> through anyway to the err label.
>>
> I kept the goto in, in case more code is added below that statement so the
> author doesn't forget that this error needs to be handled.
> If wanted I can remove it of course.
>
I am ok with all the reasoning in the previous comments.  This one I 
would say just fall through to out.

If there is other code added after this then it can be added in.

Dan

Re: [PATCH v2 2/2] leds: add sgm3140 driver

[Andy Shevchenko]

On Mon, Mar 30, 2020 at 10:49 PM Luca Weiss <luca@z3ntu.xyz> wrote:
>
> Add a driver for the SGMICRO SGM3140 Buck/Boost Charge Pump LED driver.
>
> This device is controlled by two GPIO pins, one for enabling and the
> second one for switching between torch and flash mode.

...

> +config LEDS_SGM3140
> +       tristate "LED support for the SGM3140"
> +       depends on LEDS_CLASS_FLASH
> +       depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS

> +       depends on OF

depends on OF || COMPILE_TEST ?
But hold on...

...

> +#include <linux/of.h>

Perhaps switch this to property.h and replace OF with more generic
device property / fwnode API?

...

> +struct sgm3140 {
> +       bool enabled;
> +       struct gpio_desc *flash_gpio;
> +       struct gpio_desc *enable_gpio;
> +       struct regulator *vin_regulator;
> +
> +       /* current timeout in us */
> +       u32 timeout;
> +       /* maximum timeout in us */
> +       u32 max_timeout;
> +

> +       struct led_classdev_flash fled_cdev;

I guess it might be slightly better to make it first member of the
struct (I didn't check but the rationale is to put more often used
members at the beginning to utilize cachelines).

> +       struct v4l2_flash *v4l2_flash;
> +
> +       struct timer_list powerdown_timer;
> +};

...

> +static struct sgm3140 *flcdev_to_sgm3140(struct led_classdev_flash *flcdev)
> +{
> +       return container_of(flcdev, struct sgm3140, fled_cdev);
> +}

...and this becomes a no-op AFAICS (doesn't mean you need to remove it).

...

> +       struct device_node *child_node;

> +       child_node = of_get_next_available_child(pdev->dev.of_node, NULL);

> +       ret = of_property_read_u32(child_node, "flash-max-timeout-us",
> +                                  &priv->max_timeout);

> +       init_data.fwnode = of_fwnode_handle(child_node);

> +       of_node_put(child_node);

Device property / fwnode API?

--
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] leds: add sgm3140 driver

[Luca Weiss]

Hi Andy,

On Samstag, 4. April 2020 11:58:31 CEST Andy Shevchenko wrote:
> On Mon, Mar 30, 2020 at 10:49 PM Luca Weiss <luca@z3ntu.xyz> wrote:
> > Add a driver for the SGMICRO SGM3140 Buck/Boost Charge Pump LED driver.
> > 
> > This device is controlled by two GPIO pins, one for enabling and the
> > second one for switching between torch and flash mode.
> 
> ...
> 
> > +config LEDS_SGM3140
> > +       tristate "LED support for the SGM3140"
> > +       depends on LEDS_CLASS_FLASH
> > +       depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
> > 
> > +       depends on OF
> 
> depends on OF || COMPILE_TEST ?
> But hold on...
> 
> ...
> 
> > +#include <linux/of.h>
> 
> Perhaps switch this to property.h and replace OF with more generic
> device property / fwnode API?
> 

I didn't find clear documentation on this, the functions in drivers/base/
property.c can be used instead of the of_* (device tree) functions?

As far as I can tell, the device_property_* functions are supposed to be used 
for simple "give me a property for this 'struct device*'" while the fwnode_* 
functions are used as generic equivalent of the of_* functions?

So in this case I can replace 

struct device_node *child_node;
child_node = of_get_next_available_child(pdev->dev.of_node, NULL);

with

struct fwnode_handle *child_node;
child_node = fwnode_get_next_available_child_node(pdev->dev.fwnode, NULL);

and then instead of

ret = of_property_read_u32(child_node, "flash-max-timeout-us",
		   &priv->max_timeout);

use

ret = fwnode_property_read_u32(child_node, "flash-max-timeout-us",
		            &priv->max_timeout);

and finally instead of

init_data.fwnode = of_fwnode_handle(child_node);

I can probably directly do

init_data.fwnode = child_node;

Does that sound correct?

> ...
> 
> > +struct sgm3140 {
> > +       bool enabled;
> > +       struct gpio_desc *flash_gpio;
> > +       struct gpio_desc *enable_gpio;
> > +       struct regulator *vin_regulator;
> > +
> > +       /* current timeout in us */
> > +       u32 timeout;
> > +       /* maximum timeout in us */
> > +       u32 max_timeout;
> > +
> > 
> > +       struct led_classdev_flash fled_cdev;
> 
> I guess it might be slightly better to make it first member of the
> struct (I didn't check but the rationale is to put more often used
> members at the beginning to utilize cachelines).
> 
> > +       struct v4l2_flash *v4l2_flash;
> > +
> > +       struct timer_list powerdown_timer;
> > +};
> 
> ...
> 
> > +static struct sgm3140 *flcdev_to_sgm3140(struct led_classdev_flash
> > *flcdev) +{
> > +       return container_of(flcdev, struct sgm3140, fled_cdev);
> > +}
> 
> ...and this becomes a no-op AFAICS (doesn't mean you need to remove it).
> 
> ...
> 
> > +       struct device_node *child_node;
> > 
> > +       child_node = of_get_next_available_child(pdev->dev.of_node, NULL);
> > 
> > +       ret = of_property_read_u32(child_node, "flash-max-timeout-us",
> > +                                  &priv->max_timeout);
> > 
> > +       init_data.fwnode = of_fwnode_handle(child_node);
> > 
> > +       of_node_put(child_node);
> 
> Device property / fwnode API?
> 
> --
> With Best Regards,
> Andy Shevchenko

Regards
Luca