Re: [PATCH v8 1/2] leds: flash: add driver to support flash LED module in QCOM PMICs

From: Fenglin Wu
Date: Tue Mar 28 2023 - 04:02:39 EST




On 3/26/2023 1:33 AM, Pavel Machek wrote:
Hi!

Add initial driver to support flash LED module found in Qualcomm
Technologies, Inc. PMICs. The flash module can have 3 or 4 channels
and each channel can be controlled indepedently and support full scale
current up to 1.5 A. It also supports connecting two channels together
to supply one LED component with full scale current up to 2 A. In that
case, the current will be split on each channel symmetrically and the
channels will be enabled and disabled at the same time.


+static int qcom_flash_strobe_set(struct led_classdev_flash *fled_cdev, bool state)
+{
+ struct qcom_flash_led *led = flcdev_to_qcom_fled(fled_cdev);
+ int rc;
+
+ rc = set_flash_current(led, led->flash_current_ma, FLASH_MODE);
+ if (rc)
+ return rc;
+
+ rc = set_flash_timeout(led, led->flash_timeout_ms);
+ if (rc)
+ return rc;
+
+ rc = set_flash_module_en(led, state);
+ if (rc)
+ return rc;
+
+ return set_flash_strobe(led, SW_STROBE, state);
+}

Should we disable the module before setting the current? It might be
already active due to torch mode...

The module enabling status should be kept as it is because it may be still controlling other LED channels, I can strobe off the LED before setting the current to cover such case. I will update it as a following patch.


+ return -EINVAL;
+ }
+
+ flash_data->v4l2_flash = devm_kcalloc(dev, count,
+ sizeof(*flash_data->v4l2_flash), GFP_KERNEL);
+ if (!flash_data->v4l2_flash)
+ return -ENOMEM;
+
+ device_for_each_child_node(dev, child) {
+ led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
+ if (!led) {
+ rc = -ENOMEM;
+ goto release;
+ }
+
+ led->flash_data = flash_data;
+ rc = qcom_flash_register_led_device(dev, child, led);
+ if (rc < 0)
+ goto release;
+
+ flash_data->leds_count++;
+ }

Do you need to do of_node_put in error path
I will update this as well.


BR,
Pavel