Re: [PATCH v2 1/2] leds: flash: add driver to support flash LED module in QCOM PMICs
From: Fenglin Wu
Date: Mon Oct 10 2022 - 06:01:49 EST
On 2022/9/29 20:23, Krzysztof Kozlowski wrote:
On 29/09/2022 14:15, Fenglin Wu wrote:
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.
Signed-off-by: Fenglin Wu <quic_fenglinw@xxxxxxxxxxx>
---
drivers/leds/flash/Kconfig | 14 +
drivers/leds/flash/Makefile | 1 +
drivers/leds/flash/leds-qcom-flash.c | 707 +++++++++++++++++++++++++++
3 files changed, 722 insertions(+)
create mode 100644 drivers/leds/flash/leds-qcom-flash.c
diff --git a/drivers/leds/flash/Kconfig b/drivers/leds/flash/Kconfig
index d3eb689b193c..92773fa872dc 100644
--- a/drivers/leds/flash/Kconfig
+++ b/drivers/leds/flash/Kconfig
@@ -61,6 +61,20 @@ config LEDS_MT6360
Independent current sources supply for each flash LED support torch
and strobe mode.
+config LEDS_QCOM_FLASH
+ tristate "LED support for flash module inside Qualcomm Technologies, Inc. PMIC"
+ depends on MFD_SPMI_PMIC
|| COMPILE_TEST
(and actually test it, e.g. you might need here "select REGMAP")
I will add "COMPILE_TEST" here and compile with with "COMPILE_TEST" enabled.
+ depends on LEDS_CLASS && OF
+ depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
+ help
+ This option enables support for the flash module found in Qualcomm
+ Technologies, Inc. PMICs. The flash module can have 3 or 4 flash LED
+ channels and each channel is programmable to support up to 1.5 A full
+ scale current. It also supports connecting two channels' output together
+ to supply one LED component to achieve current up to 2 A. In such case,
+ the total LED current will be split symmetrically on each channel and
+ they will be enabled/disabled at the same time.
+
config LEDS_RT4505
tristate "LED support for RT4505 flashlight controller"
depends on I2C && OF
diff --git a/drivers/leds/flash/Makefile b/drivers/leds/flash/Makefile
index 0acbddc0b91b..8a60993f1a25 100644
--- a/drivers/leds/flash/Makefile
+++ b/drivers/leds/flash/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_LEDS_AS3645A) += leds-as3645a.o
obj-$(CONFIG_LEDS_KTD2692) += leds-ktd2692.o
obj-$(CONFIG_LEDS_LM3601X) += leds-lm3601x.o
obj-$(CONFIG_LEDS_MAX77693) += leds-max77693.o
+obj-$(CONFIG_LEDS_QCOM_FLASH) += leds-qcom-flash.o
obj-$(CONFIG_LEDS_RT4505) += leds-rt4505.o
obj-$(CONFIG_LEDS_RT8515) += leds-rt8515.o
obj-$(CONFIG_LEDS_SGM3140) += leds-sgm3140.o
diff --git a/drivers/leds/flash/leds-qcom-flash.c b/drivers/leds/flash/leds-qcom-flash.c
new file mode 100644
index 000000000000..7b941eb769fe
--- /dev/null
+++ b/drivers/leds/flash/leds-qcom-flash.c
@@ -0,0 +1,707 @@
+//SPDX-License-Identifier: GPL-2.0-only
Missing space after //
Thanks for catching it, will fix it.
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/leds.h>
+#include <linux/led-class-flash.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <media/v4l2-flash-led-class.h>
+
+/* registers definitions */
+#define FLASH_TYPE_REG 0x04
+#define FLASH_TYPE_VAL 0x18
+
+#define FLASH_SUBTYPE_REG 0x05
+#define FLASH_SUBTYPE_3CH_VAL 0x04
+#define FLASH_SUBTYPE_4CH_VAL 0x07
+
+#define FLASH_MODULE_EN_BIT BIT(7)
+
+#define FLASH_TIMER_EN_BIT BIT(7)
+#define FLASH_TIMER_VAL_MASK GENMASK(6, 0)
+#define FLASH_TIMER_STEP_MS 10
+
+#define FLASH_ITARGET_CURRENT_MASK GENMASK(6, 0)
+
+#define FLASH_STROBE_HW_SW_SEL_BIT BIT(2)
+#define SW_STROBE_VAL 0
+#define HW_STROBE_VAL 1
+#define FLASH_HW_STROBE_TRIGGER_SEL_BIT BIT(1)
+#define STROBE_LEVEL_TRIGGER_VAL 0
+#define STROBE_EDGE_TRIGGER_VAL 1
+#define FLASH_STROBE_POLARITY_BIT BIT(0)
+#define STROBE_ACTIVE_HIGH_VAL 1
+
+#define FLASH_IRES_MASK_4CH BIT(0)
+#define FLASH_IRES_MASK_3CH GENMASK(1, 0)
+#define FLASH_IRES_12P5MA_VAL 0
+#define FLASH_IRES_5MA_VAL_4CH 1
+#define FLASH_IRES_5MA_VAL_3CH 3
+
+/* constants */
+#define FLASH_CURRENT_MAX_UA 1500000
+#define TORCH_CURRENT_MAX_UA 500000
+#define FLASH_TOTAL_CURRENT_MAX_UA 2000000
+#define FLASH_CURRENT_DEFAULT_UA 1000000
+#define TORCH_CURRENT_DEFAULT_UA 200000
+
+#define TORCH_IRES_UA 5000
+#define FLASH_IRES_UA 12500
+
+#define FLASH_TIMEOUT_MAX_US 1280000
+#define FLASH_TIMEOUT_STEP_US 10000
+
+enum hw_type {
+ QCOM_MVFLASH_3CH,
+ QCOM_MVFLASH_4CH,
+};
+
+enum led_mode {
+ FLASH_MODE,
+ TORCH_MODE,
+};
+
+enum led_strobe {
+ SW_STROBE,
+ HW_STROBE,
+};
+
+struct qcom_flash_reg {
+ u8 module_en;
+ u8 chan_timer;
+ u8 itarget;
+ u8 iresolution;
+ u8 chan_strobe;
+ u8 chan_en;
+ u8 status1;
+ u8 status2;
+ u8 status3;
+};
+
+struct qcom_flash_led {
+ struct qcom_flash_chip *chip;
+ struct led_classdev_flash flash;
+ struct v4l2_flash *v4l2_flash;
+ u32 max_flash_current_ma;
+ u32 max_torch_current_ma;
+ u32 max_timeout_ms;
+ u32 flash_current_ma;
+ u32 flash_timeout_ms;
+ u8 *chan_id;
+ u8 chan_count;
+ bool enabled;
+};
+
+struct qcom_flash_chip {
+ struct qcom_flash_led *leds;
+ const struct qcom_flash_reg *reg;
+ struct device *dev;
+ struct regmap *regmap;
+ struct mutex lock;
+ enum hw_type hw_type;
+ u32 reg_base;
+ u8 leds_count;
+ u8 max_channels;
+ u8 chan_en_bits;
+};
+
+static const struct qcom_flash_reg mvflash_3ch_reg = {
+ .chan_timer = 0x40,
+ .itarget = 0x43,
+ .module_en = 0x46,
+ .iresolution = 0x47,
+ .chan_strobe = 0x49,
+ .chan_en = 0x4c,
+ .status1 = 0x08,
+ .status2 = 0x09,
+ .status3 = 0x0a,
+};
+
+static const struct qcom_flash_reg mvflash_4ch_reg = {
+ .chan_timer = 0x3e,
+ .itarget = 0x42,
+ .module_en = 0x46,
+ .iresolution = 0x49,
+ .chan_strobe = 0x4a,
+ .chan_en = 0x4e,
+ .status1 = 0x06,
+ .status2 = 0x07,
+ .status3 = 0x09,
Don't reinvent the wheel. Use regmap fields.
Do you mean defining regmap_filed pointer in struct qcom_flash_chip
for all these registers instead of creating the data structure
qcom_flash_reg to include all the registers? Similar like this
struct qcom_flash_chip {
....
struct regmap_field *chan_timer;
struct regmap_field *chan_timer;
struct regmap_field *itarget;
struct regmap_field *iresolution;
struct regmap_field *chan_strobe;
struct regmap_field *chan_en;
struct regmap_field *status1;
struct regmap_field *status2;
struct regmap_field *status3;
....
};
This won't make the code cleaner actually. If this is the preferred way,
I will update it accordingly.
Thanks
+};
+
+static int __set_flash_module_en(struct qcom_flash_led *led, bool en)
Drop __ prefix here and in other functions.
(...)
Done
+
+static int qcom_flash_led_remove(struct platform_device *pdev)
+{
+ struct qcom_flash_chip *chip = platform_get_drvdata(pdev);
+
+ while (chip->leds_count--)
+ v4l2_flash_release(chip->leds[chip->leds_count].v4l2_flash);
+
+ mutex_destroy(&chip->lock);
+ return 0;
+}
+
+static const struct of_device_id qcom_flash_led_match_table[] = {
+ { .compatible = "qcom,spmi-flash-led" },
Only this one is needed. Remove the rest:
Done
+ { .compatible = "qcom,pm8150c-flash-led" },
+ { .compatible = "qcom,pm8150l-flash-led" },
+ { .compatible = "qcom,pm8350c-flash-led" },
Best regards,
Krzysztof