lkml.org 
[lkml]   [2022]   [Oct]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 1/2] leds: flash: add driver to support flash LED module in QCOM PMICs


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@quicinc.com>
>> ---
>> 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
>

\
 
 \ /
  Last update: 2022-10-10 12:02    [W:0.048 / U:0.520 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site