lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <db20b5ac25803020d7579c6bc8d5cd06572c3b12.camel@dylanvanassche.be>
Date:   Wed, 02 Nov 2022 15:26:56 +0100
From:   Dylan Van Assche <me@...anvanassche.be>
To:     Fenglin Wu <quic_fenglinw@...cinc.com>,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
        krzysztof.kozlowski@...aro.org, Pavel Machek <pavel@....cz>,
        Gene Chen <gene_chen@...htek.com>,
        Jacek Anaszewski <jacek.anaszewski@...il.com>,
        linux-leds@...r.kernel.org
Cc:     quic_collinsd@...cinc.com, quic_subbaram@...cinc.com,
        Luca Weiss <luca.weiss@...rphone.com>
Subject: Re: [PATCH v4 1/2] leds: flash: add driver to support flash LED
 module in QCOM PMICs

Hi Fenglin,


On Wed, 2022-11-02 at 19:06 +0800, Fenglin Wu wrote:
> 
> 
> On 2022/11/2 16:14, Dylan Van Assche wrote:
> > 
> > Hi Fenglin,
> > 
> > > Hi Dylan,
> > > 
> > > 
> > > On 2022/11/2 1:39, Dylan Van Assche wrote:
> > > > 
> > > > Hi Fenglin,
> > > > 
> > > > > On 2022/10/31 18:34, Dylan Van Assche wrote:
> > > > > > 
> > > > > > Hi Fenglin,
> > > > > > 
> > > > > > Thanks for submitting these patches! They are really nice!
> > > > > > I tested these patches on an SDM845 based device as the
> > > > > > downstream
> > > > > > driver is the same for the PMI8998 compared to the
> > > > > > mentioned
> > > > > > PMICs
> > > > > > in
> > > > > > this patchseries.
> > > > > > 
> > > > > > Tested-by: Dylan Van Assche <me@...anvanassche.be> #
> > > > > > sdm845-
> > > > > > shift-
> > > > > > axolotl + pmi8998
> > > > > > 
> > > > > Thanks for testing the patch at your side, Dylan!
> > > > > 
> > > > > > > > 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@...cinc.com>
> > > > > > > > Tested-by: Luca Weiss <luca.weiss@...rphone.com> # >
> > > > > > > > sm7225-
> > > > > > > > fairphone-fp4 + pm6150l
> > > > > > > > ---
> > > > > > > >     drivers/leds/flash/Kconfig           |  15 +
> > > > > > > >     drivers/leds/flash/Makefile          |   1 +
> > > > > > > >     drivers/leds/flash/leds-qcom-flash.c | 700 >
> > > > > > > > +++++++++++++++++++++++++++
> > > > > > > >     3 files changed, 716 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..f36a60409290 100644
> > > > > > > > --- a/drivers/leds/flash/Kconfig
> > > > > > > > +++ b/drivers/leds/flash/Kconfig
> > > > > > > > @@ -61,6 +61,21 @@ 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
> > > > > > > > +       depends on LEDS_CLASS && OF
> > > > > > > > +       depends on V4L2_FLASH_LED_CLASS ||
> > > > > > > > !V4L2_FLASH_LED_CLASS
> > > > > > > > +       select REGMAP
> > > > > > > > +       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 currentwill 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..a9fcf61600c3
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/drivers/leds/flash/leds-qcom-flash.c
> > > > > > > > @@ -0,0 +1,700 @@
> > > > > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > > > > +/*
> > > > > > > > + * 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
> > > > > > 
> > > > > > PMI8998 has also a 3 channel flash subtype, but reports
> > > > > > 0x03 as
> > > > > > subtype. The driver won't probe because of this since the
> > > > > > subtype
> > > > > > did
> > > > > > not match. Changing this value to 0x03 made this driver
> > > > > > work on
> > > > > > SDM845
> > > > > > based device as well with PMI8998. Would you consider
> > > > > > supporting
> > > > > > SDM845+PMI8998 as well?
> > > > > > 
> > > > > I just double checked, flash module in PMI8998 (with subtype
> > > > > 0x3)
> > > > > has
> > > > > very similar register definition as the flash module in
> > > > > PM8150C
> > > > > (with
> > > > > subtype 0x4) that the driver is intended to support, it's
> > > > > good
> > > > > know
> > > > > that
> > > > > it's 99% work.
> > > > > I can add the 0x3 subtype support but the only problem is I
> > > > > couldn't
> > > > > find a device with PMI8998 to test it out. Would it be good
> > > > > if I
> > > > > add
> > > > > the
> > > > > support and rely on your test result?
> > > > 
> > > > Sure! I already have a branch locally to test your patches,
> > > > happy
> > > > to
> > > > help! I have an SDM845 device here with PMI8998 with 2 LEDs:
> > > > cold
> > > > and
> > > > warm white.
> > > > 
> > > Thank you for the help!
> > > > > 
> > > > > > > > +#define FLASH_SUBTYPE_4CH_VAL          0x07
> > > > > > > > +
> > > > > > > > +#define FLASH_TIMER_EN_BIT             BIT(7)
> > > > > > > > +#define FLASH_TIMER_VAL_MASK           GENMASK(6, 0)
> > > > > > > > +#define FLASH_TIMER_STEP_MS            10
> > > > > > > > +
> > > > > > > > +#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,
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +enum {
> > > > > > > > +       REG_STATUS1,
> > > > > > > > +       REG_STATUS2,
> > > > > > > > +       REG_STATUS3,
> > > > > > > > +       REG_CHAN_TIMER,
> > > > > > > > +       REG_ITARGET,
> > > > > > > > +       REG_MODULE_EN,
> > > > > > > > +       REG_IRESOLUTION,
> > > > > > > > +       REG_CHAN_STROBE,
> > > > > > > > +       REG_CHAN_EN,
> > > > > > > > +       REG_MAX_COUNT,
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +struct reg_field mvflash_3ch_regs[REG_MAX_COUNT] = {
> > > > > > > > +       REG_FIELD(0x08, 0, 7),                  /*
> > > > > > > > status1      */
> > > > > > > > +       REG_FIELD(0x09, 0, 7),                  /*
> > > > > > > > status2      */
> > > > > > > > +       REG_FIELD(0x0a, 0, 7),                  /*
> > > > > > > > status3      */
> > > > > > > > +       REG_FIELD_ID(0x40, 0, 7, 3, 1),         /*
> > > > > > > > chan_timer   */
> > > > > > > > +       REG_FIELD_ID(0x43, 0, 6, 3, 1),         /*
> > > > > > > > itarget      */
> > > > > > > > +       REG_FIELD(0x46, 7, 7),                  /*
> > > > > > > > module_en    */
> > > > > > > > +       REG_FIELD(0x47, 0, 5),                  /*
> > > > > > > > iresolution  */
> > > > > > > > +       REG_FIELD_ID(0x49, 0, 2, 3, 1),         /*
> > > > > > > > chan_strobe  */
> > > > > > > > +       REG_FIELD(0x4c, 0, 2),                  /*
> > > > > > > > chan_en      */
> > > > > > > > +};
> > > > > > 
> > > > > > These registers map perfectly for SDM845 with PMI8998 as
> > > > > > well,
> > > > > > adding
> > > > > > support for this subtype (0x03) would be great.
> > > > > > 
> > > > > > > > +
> > > > > > > > +struct reg_field mvflash_4ch_regs[REG_MAX_COUNT] = {
> > > > > > > > +       REG_FIELD(0x06, 0, 7),                  /*
> > > > > > > > status1      */
> > > > > > > > +       REG_FIELD(0x07, 0, 6),                  /*
> > > > > > > > status2      */
> > > > > > > > +       REG_FIELD(0x09, 0, 7),                  /*
> > > > > > > > status3      */
> > > > > > > > +       REG_FIELD_ID(0x3e, 0, 7, 4, 1),         /*
> > > > > > > > chan_timer   */
> > > > > > > > +       REG_FIELD_ID(0x42, 0, 6, 4, 1),         /*
> > > > > > > > itarget      */
> > > > > > > > +       REG_FIELD(0x46, 7, 7),                  /*
> > > > > > > > module_en    */
> > > > > > > > +       REG_FIELD(0x49, 0, 3),                  /*
> > > > > > > > iresolution  */
> > > > > > > > +       REG_FIELD_ID(0x4a, 0, 6, 4, 1),         /*
> > > > > > > > chan_strobe  */
> > > > > > > > +       REG_FIELD(0x4e, 0, 3),                  /*
> > > > > > > > chan_en      */
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +struct qcom_flash_led {
> > > > > > > > +       struct qcom_flash_chip          *chip;
> > > > > > > > +       struct led_classdev_flash       flash;
> > > > > > > > +       struct v4l2_flash               *v4l2_flash;
> > > > > > > > +       u32                             max_flash_curre
> > > > > > > > nt_m
> > > > > > > > a;
> > > > > > > > +       u32                             max_torch_curre
> > > > > > > > nt_m
> > > > > > > > a;
> > > > > > > > +       u32                             max_timeout_ms;
> > > > > > > > +       u32                             flash_current_m
> > > > > > > > a;
> > > > > > > > +       u32                             flash_timeout_m
> > > > > > > > s;
> > > > > > > > +       u8                              *chan_id;
> > > > > > > > +       u8                              chan_count;
> > > > > > > > +       bool                            enabled;
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +struct qcom_flash_chip {
> > > > > > > > +       struct qcom_flash_led   *leds;
> > > > > > > > +       struct regmap_field    
> > > > > > > > *r_fields[REG_MAX_COUNT];
> > > > > > > > +       struct device           *dev;
> > > > > > > > +       struct mutex            lock;
> > > > > > > > +       enum hw_type            hw_type;
> > > > > > > > +       u8                      leds_count;
> > > > > > > > +       u8                      max_channels;
> > > > > > > > +       u8                      chan_en_bits;
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +static int set_flash_module_en(struct qcom_flash_led
> > > > > > > > *led,
> > > > > > > > bool
> > > > > > > > en)
> > > > > > > > +{
> > > > > > > > +       struct qcom_flash_chip *chip= led->chip;
> > > > > > > > +       u8 led_mask = 0, val;
> > > > > > > > +       int i, rc;
> > > > > > > > +
> > > > > > > > +       for (i = 0; i < led->chan_count; i++)
> > > > > > > > +               led_mask |= BIT(led->chan_id[i] - 1);
> > > > > > > > +
> > > > > > > > +       mutex_lock(&chip->lock);
> > > > > > > > +       if (en)
> > > > > > > > +               chip->chan_en_bits |= led_mask;
> > > > > > > > +       else
> > > > > > > > +               chip->chan_en_bits &= ~led_mask;
> > > > > > > > +
> > > > > > > > +       val = !!chip->chan_en_bits;
> > > > > > > > +       rc = regmap_field_write(chip-
> > > > > > > > > r_fields[REG_MODULE_EN],
> > > > > > > > val);
> > > > > > > > +       if (rc < 0)
> > > > > > > > +               dev_err(chip->dev, "write module_en
> > > > > > > > failed,
> > > > > > > > rc=%d\n", > rc);
> > > > > > > > +       mutex_unlock(&chip->lock);
> > > > > > > > +
> > > > > > > > +       return rc;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int set_flash_current(struct qcom_flash_led
> > > > > > > > *led,
> > > > > > > > u32 >
> > > > > > > > current_ma, enum led_mode mode)
> > > > > > > > +{
> > > > > > > > +       struct qcom_flash_chip *chip= led->chip;
> > > > > > > > +       u32 itarg_ua = current_ma * 1000 / led-
> > > > > > > > >chan_count
> > > > > > > > + 1;
> > > > > > > > +       u32 ires_ua = (mode ==FLASH_MODE) ?
> > > > > > > > FLASH_IRES_UA :
> > > > > > > > > 
> > > > > > > > TORCH_IRES_UA;
> > > > > > > > +       u8 val, shift, ires_mask =0, ires_val = 0,
> > > > > > > > chan_id;
> > > > > > > > +       int i, rc;
> > > > > > > > +
> > > > > > > > +       /*
> > > > > > > > +        * Split the current across the channels and
> > > > > > > > set
> > > > > > > > the
> > > > > > > > +        * IRESOLUTION and ITARGET registers
> > > > > > > > accordingly.
> > > > > > > > +        */
> > > > > > > > +       for (i = 0; i < led->chan_count; i++) {
> > > > > > > > +               chan_id = led->chan_id[i];
> > > > > > > > +               if (itarg_ua < ires_ua)
> > > > > > > > +                       val = 0;
> > > > > > > > +               else
> > > > > > > > +                       val = itarg_ua / ires_ua - 1;
> > > > > > > > +
> > > > > > > > +               rc = regmap_fields_write(chip-
> > > > > > > > > r_fields[REG_ITARGET], > chan_id - 1, val);
> > > > > > > > +               if (rc < 0)
> > > > > > > > +                       returnrc;
> > > > > > > > +
> > > > > > > > +               if (chip->hw_type == QCOM_MVFLASH_3CH)
> > > > > > > > {
> > > > > > > > +                       shift = (chan_id - 1) * 2;
> > > > > > > > +                       ires_mask |=
> > > > > > > > FLASH_IRES_MASK_3CH <<
> > > > > > > > shift;
> > > > > > > > +                       ires_val |= ((mode ==
> > > > > > > > FLASH_MODE) ?
> > > > > > > > +                               (FLASH_IRES_12P5MA_VAL
> > > > > > > > <<
> > > > > > > > shift) :
> > > > > > > > +                               (FLASH_IRES_5MA_VAL_3CH
> > > > > > > > <<
> > > > > > > > shift));
> > > > > > > > +               } else if (chip->hw_type ==
> > > > > > > > QCOM_MVFLASH_4CH) {
> > > > > > > > +                       shift = chan_id - 1;
> > > > > > > > +                       ires_mask |=
> > > > > > > > FLASH_IRES_MASK_4CH <<
> > > > > > > > shift;
> > > > > > > > +                       ires_val |= ((mode ==
> > > > > > > > FLASH_MODE) ?
> > > > > > > > +                               (FLASH_IRES_12P5MA_VAL
> > > > > > > > <<
> > > > > > > > shift) :
> > > > > > > > +                               (FLASH_IRES_5MA_VAL_4CH
> > > > > > > > <<
> > > > > > > > shift));
> > > > > > > > +               }
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       return > regmap_field_update_bits(chip-
> > > > > > > > > r_fields[REG_IRESOLUTION], ires_mask, > ires_val);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int set_flash_timeout(struct qcom_flash_led
> > > > > > > > *led,
> > > > > > > > u32 >
> > > > > > > > timeout_ms)
> > > > > > > > +{
> > > > > > > > +       struct qcom_flash_chip *chip= led->chip;
> > > > > > > > +       u8 val, chan_id;
> > > > > > > > +       int rc, i;
> > > > > > > > +
> > > > > > > > +       /* set SAFETY_TIMER for all the channels
> > > > > > > > connected
> > > > > > > > to
> > > > > > > > the >
> > > > > > > > same LED */
> > > > > > > > +       timeout_ms = min_t(u32, timeout_ms, led-
> > > > > > > > > max_timeout_ms);
> > > > > > > > +       for (i = 0; i < led->chan_count; i++) {
> > > > > > > > +               chan_id = led->chan_id[i];
> > > > > > > > +               val = timeout_ms / FLASH_TIMER_STEP_MS;
> > > > > > > > +               val = clamp_t(u8, val, 0,
> > > > > > > > FLASH_TIMER_VAL_MASK);
> > > > > > > > +               if (timeout_ms)
> > > > > > > > +                       val |= FLASH_TIMER_EN_BIT;
> > > > > > > > +
> > > > > > > > +               rc = > regmap_fields_write(chip-
> > > > > > > > > r_fields[REG_CHAN_TIMER], chan_id - 1, > val);
> > > > > > > > +               if (rc < 0)
> > > > > > > > +                       returnrc;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int set_flash_strobe(struct qcom_flash_led
> > > > > > > > *led,
> > > > > > > > enum >
> > > > > > > > led_strobe strobe, bool state)
> > > > > > > > +{
> > > > > > > > +       struct qcom_flash_chip *chip= led->chip;
> > > > > > > > +       u8 mask, val, chan_id = 0,chan_mask = 0;
> > > > > > > > +       int rc, i;
> > > > > > > > +
> > > > > > > > +       /* Set SW strobe config for all channels
> > > > > > > > connected
> > > > > > > > to
> > > > > > > > the
> > > > > > > > LED > */
> > > > > > > > +       for (i = 0; i < led->chan_count; i++) {
> > > > > > > > +               chan_id = led->chan_id[i];
> > > > > > > > +               if (strobe == SW_STROBE)
> > > > > > > > +                       val =
> > > > > > > > FIELD_PREP(FLASH_STROBE_HW_SW_SEL_BIT, >
> > > > > > > > SW_STROBE_VAL);
> > > > > > > > +               else
> > > > > > > > +                       val =
> > > > > > > > FIELD_PREP(FLASH_STROBE_HW_SW_SEL_BIT, >
> > > > > > > > HW_STROBE_VAL);
> > > > > > > > +
> > > > > > > > +               val |=
> > > > > > > > FIELD_PREP(FLASH_HW_STROBE_TRIGGER_SEL_BIT,
> > > > > > > > > STROBE_LEVEL_TRIGGER_VAL) |
> > > > > > > > +                       FIELD_PREP(FLASH_STROBE_POLARIT
> > > > > > > > Y_BI
> > > > > > > > T, >
> > > > > > > > STROBE_ACTIVE_HIGH_VAL);
> > > > > > > > +               rc = > regmap_fields_write(chip-
> > > > > > > > > r_fields[REG_CHAN_STROBE], chan_id - 1, > val);
> > > > > > > > +               if (rc < 0)
> > > > > > > > +                       returnrc;
> > > > > > > > +
> > > > > > > > +               chan_mask |= BIT(chan_id - 1);
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       /* enable/disable flash channels */
> > > > > > > > +       mask = chan_mask;
> > > > > > > > +       val = state ? mask : 0;
> > > > > > > > +       rc = regmap_field_update_bits(chip-
> > > > > > > > > r_fields[REG_CHAN_EN],
> > > > > > > > > mask, val);
> > > > > > > > +       if (rc < 0)
> > > > > > > > +               return rc;
> > > > > > > > +
> > > > > > > > +       led->enabled = state;
> > > > > > > > +       return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int qcom_flash_brightness_set(struct
> > > > > > > > led_classdev_flash
> > > > > > > > > 
> > > > > > > > *fled_cdev, u32 brightness)
> > > > > > > > +{
> > > > > > > > +       struct qcom_flash_led *led =
> > > > > > > > container_of(fled_cdev,
> > > > > > > > struct
> > > > > > > > > qcom_flash_led, flash);
> > > > > > > > +
> > > > > > > > +       led->flash_current_ma = min_t(u32, led-
> > > > > > > > > max_flash_current_ma, > brightness / 1000);
> > > > > > > > +       return 0;
> > > > > > > > +}
> > > > > > 
> > > > > > This doesn't seem to work in torch mode for me on PMI8998.
> > > > > > If
> > > > > > the
> > > > > > brightness is 0, the torch is OFF as expected, but when the
> > > > > > torch
> > > > > > is
> > > > > > ON, you cannot control the brightness. The brightness is
> > > > > > the
> > > > > > same
> > > > > > for
> > > > > > [1, 255]. If the hardware cannot change the brightness in
> > > > > > torch
> > > > > > mode,
> > > > > > it would be nice to report a max_brightness value of 1
> > > > > > instead
> > > > > > of
> > > > > > 255
> > > > > > for userspace. This could be a limitation of the PMI8998,
> > > > > > not
> > > > > > sure.
> > > > > > Do you have any insights here? This driver is already 99%
> > > > > > working
> > > > > > for
> > > > > > SDM845+PMI8998.
> > > > > > 
> > > > > > Flash mode works great, timeout & brightness can be
> > > > > > configured
> > > > > > just
> > > > > > fine.
> > > > > > 
> > > > > 
> > > > > Here, function qcom_flash_brightness_set() is used for
> > > > > setting
> > > > > brightness in flash mode and it was supposed to be working
> > > > > with
> > > > > following commands combination:
> > > > >           echo xxx > flash_brightness
> > > > >           echo 1 > flash_strobe  (you will need to toggle
> > > > > flash_strobe
> > > > > when you
> > > > > enabling it again)
> > > > > 
> > > > > Can you check if you can see the brightness change in flash
> > > > > mode
> > > > > when
> > > > > you updating the flash_brightness value with these commands?
> > > > > 
> > > > > You can echo any value between [0, max_flash_brightness] into
> > > > > the
> > > > > flash_brightness, there is a "max_flash_brightness" sysfs
> > > > > node
> > > > > and
> > > > > the
> > > > > value comes from DT property "flash-max-microamp", ex. my
> > > > > dtsi
> > > > > node
> > > > > has
> > > > > "flash-max-microamp = <2000000>;" defined so I am having
> > > > > following
> > > > > value
> > > > > for max_flash_brightness:
> > > > > 
> > > > > kalama:/sys/class/leds/white:flash-0 # cat
> > > > > max_flash_brightness
> > > > > 2000000
> > > > > 
> > > > > And by default flash mode uses 12500uA resolution so you
> > > > > won't
> > > > > see
> > > > > brightness change when you update it with values between [1,
> > > > > 12500].
> > > > 
> > > > Yes, this works perfectly fine on PMI8998. I can adjust the
> > > > flash
> > > > brightness and flash timeout perfectly.
> > > > 
> > > > > 
> > > > > If you want to update the brightness for torch node, you can
> > > > > directly
> > > > > update the "brightness" node with values between [0, 255],
> > > > > and
> > > > > it's
> > > > > mapping to the torch current between [0, led-max-microamp].
> > > > > "led-max-microamp" is also has value coming from the DT
> > > > > property.
> > > > > 
> > > > > This worked at my side on both pm8150C and pm8550.I think it
> > > > > should
> > > > > work
> > > > > on PMI8998 as well because the flash module in it is very
> > > > > similar
> > > > > to
> > > > > the
> > > > > one in PM8150c. Let me know if you still see such issues at
> > > > > your
> > > > > side.
> > > > > Thanks
> > > > 
> > > > Only here I have an issue with PMI8998: the "brightness" nodes
> > > > should
> > > > change the brightness of the torch, but it doesn't make a
> > > > difference
> > > > here. When I do:
> > > > 
> > > > - echo 0 > brightness --> LED turns OFF
> > > > - echo 255 > brightness --> LED turns ON
> > > > - echo 100 > brightness --> LED is still ON, but brightness is
> > > > the
> > > > same
> > > > as with 255.
> > > > 
> > > > Could it be that PMI8998 is slightly different here than with
> > > > PM8150c
> > > > for example? Maybe it doesn't support brightness for torch?
> > > > 
> > > > Here's my DTS:
> > > > 
> > > > *PMI8998*
> > > > 
> > > > pmi8998_flash: led-controller@...0 {
> > > >          compatible = "qcom,pm6150l-flash-led", "qcom,spmi-
> > > > flash-
> > > > led";
> > > >          reg = <0xd300>;
> > > >          status = "disabled";
> > > > };
> > > > 
> > > > *SDM845 SHIFT axolotl*
> > > > 
> > > > &pmi8998_flash {
> > > >          status = "okay";
> > > > 
> > > >          led-0 {
> > > >                  function = LED_FUNCTION_FLASH;
> > > >                  color = <LED_COLOR_ID_WHITE>;
> > > >                  led-sources = <1>;
> > > >                  led-max-microamp = <180000>;
> > > >                  flash-max-microamp = <1000000>;
> > > >                  flash-max-timeout-us = <1280000>;
> > > >          };
> > > > 
> > > >          led-1 {
> > > >                  function = LED_FUNCTION_FLASH;
> > > >                  color = <LED_COLOR_ID_YELLOW>;
> > > >                  led-sources = <2>;
> > > >                  led-max-microamp = <180000>;
> > > >                  flash-max-microamp = <1000000>;
> > > >                  flash-max-timeout-us = <1280000>;
> > > >          };
> > > > };
> > > > 
> > > Thank you for getting back all the details. The devicetree node
> > > looks
> > > good to me.
> > > 
> > > I checked again and confirmed that the flash modules in PMI8998
> > > and
> > > PM8150C have the same register definition for ITARGETx (0xD343 +
> > > x)
> > > and
> > > IRESOLUTION (0xD347), these are the only 2 settings would impact
> > > LED
> > > brightness, and in torch mode, IRESOLUTION is fixed to 5mA and
> > > only
> > > ITARGET is updated accordingly.
> > > 
> > > I updated my workspace to use the same current for torch mode as
> > > your
> > > settings in devicetree and tried again on my PM8150C device,I
> > > could
> > > notice the brightness change when echoing different values to the
> > > brightness sysfs node, however, when I updating values between
> > > 255
> > > and
> > > 200, it wasn't a very noticeable brightness change with naked
> > > eys,
> > > but I
> > > could see the ITRAGETx register changed accordingly when reading
> > > its
> > > value back from the regmap debugfs node.
> > > 
> > > Can you try the same and see if the register got updated
> > > accordingly
> > > when you updating brightness values? If yes, I would wonder if
> > > the
> > > LED
> > > component on your device has upper current limit in torch mode
> > > close
> > > to
> > > (100 / 255 ) * 180mA so you that you can't observe the brightness
> > > change
> > > when updating between 100 to 255. Another easier thing to try,
> > > can
> > > you
> > > echo a lower brightness value, such as 10, to see if you can
> > > notice
> > > the
> > > brightness change?
> > > 
> > 
> > Aha! It does seem to work partially!
> > 
> > I tried it with brightness 0 --> 255 --> 1. This order keeps the
> > highest brightness at all times, matching 255. The other way
> > around: 0
> > --> 1 --> 255 keeps the lowest brightness, matching 1.
> > Changing the brightness only works if the LED was turned OFF first:
> > 
> > 0 --> 255 --> 0 --> 1
> > 0 --> 1 --> 0 --> 255
> > 
> > both work fine, verified it with the following shell commands
> > (assumed
> > that the LED was turned OFF to start):
> > 
> > echo 1 > brightness && sleep 3 && echo 0 > brightness && echo 255 >
> > brightness
> > echo 255 > brightness && sleep 3 &&  echo 0 > brightness && echo 1
> > >
> > brightness
> > 
> > Maybe a register needs to be reset when the brightness changes?
> > 
> 
> Hi Dylan,
> 
> Would you mind to help me testing this small change on PMI8998? It 
> simply toggles the CHANNEL_EN bit when updating LED brightness. If it
> works, I will add it specifically for PMI8998 in patch v5.
> 
> @@ -241,8 +243,21 @@ static int set_flash_strobe(struct
> qcom_flash_led 
> *led, enum led_strobe s
> trobe,
>                  chan_mask |= BIT(chan_id - 1);
>          }
> 
> -       /* enable/disable flash channels */
>          mask = chan_mask;
> +       /*
> +        * For flash module inside PMI8998, if strobe(true) is called
> when
> +        * the LED is already enabled, disable the channel 1st and
> then
> +        * enable it again.  This could happen when updating LED
> brightness
> +        * after LED is turned on.
> +        */
> +       if (led->enabled && (led->enabled == state)) {
> +               rc = 
> regmap_field_update_bits(chip->r_fields[REG_CHAN_EN], mask, 0);
> +               if (rc < 0)
> +                       return rc;
> +       }
> +
> +       /* enable/disable flash channels */
>          val = state ? mask : 0;
>          rc = regmap_field_update_bits(chip->r_fields[REG_CHAN_EN], 
> mask, val);
>          if (rc < 0)
> 
> Thanks
> Fenglin Wu

With this change I can indeed change the brightness.
However, it works now 50% of the time:

- echo 255 > brightness --> LED turns ON, max brightness: 255
- echo 1 > brightness --> LED turns OFF
- echo 1 > brightness --> LED turns ON, min brightness: 1

This behavior didn't occur before I applied this change.
I had a look at the change but I cannot really pinpoint why this is
happening...

Kind regards,
Dylan Van Assche

> 
> > > > 
> > > > > 
> > > > > > > > +
> > > > > > > > +static int qcom_flash_timeout_set(struct
> > > > > > > > led_classdev_flash >
> > > > > > > > *fled_cdev, u32 timeout)
> > > > > > > > +{
> > > > > > > > +       struct qcom_flash_led *led =
> > > > > > > > container_of(fled_cdev,
> > > > > > > > struct
> > > > > > > > > qcom_flash_led, flash);
> > > > > > > > +
> > > > > > > > +       led->flash_timeout_ms = timeout / 1000;
> > > > > > > > +       return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int qcom_flash_strobe_set(struct
> > > > > > > > led_classdev_flash
> > > > > > > > > 
> > > > > > > > *fled_cdev, bool state)
> > > > > > > > +{
> > > > > > > > +       struct qcom_flash_led *led =
> > > > > > > > container_of(fled_cdev,
> > > > > > > > struct
> > > > > > > > > qcom_flash_led, flash);
> > > > > > > > +       int rc;
> > > > > > > > +
> > > > > > > > +       rc = set_flash_current(led, led-
> > > > > > > > >flash_current_ma,
> > > > > > > > > 
> > > > > > > > FLASH_MODE);
> > > > > > > > +       if (rc < 0)
> > > > > > > > +               return rc;
> > > > > > > > +
> > > > > > > > +       rc = set_flash_timeout(led, led-
> > > > > > > > >flash_timeout_ms);
> > > > > > > > +       if (rc < 0)
> > > > > > > > +               return rc;
> > > > > > > > +
> > > > > > > > +       rc = set_flash_module_en(led, state);
> > > > > > > > +       if (rc < 0)
> > > > > > > > +               return rc;
> > > > > > > > +
> > > > > > > > +       return set_flash_strobe(led,SW_STROBE, state);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int qcom_flash_strobe_get(struct
> > > > > > > > led_classdev_flash
> > > > > > > > > 
> > > > > > > > *fled_cdev, bool *state)
> > > > > > > > +{
> > > > > > > > +       struct qcom_flash_led *led =
> > > > > > > > container_of(fled_cdev,
> > > > > > > > struct
> > > > > > > > > qcom_flash_led, flash);
> > > > > > > > +
> > > > > > > > +       *state = led->enabled;
> > > > > > > > +       return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int qcom_flash_fault_get(struct
> > > > > > > > led_classdev_flash
> > > > > > > > > 
> > > > > > > > *fled_cdev, u32 *fault)
> > > > > > > > +{
> > > > > > > > +       struct qcom_flash_led *led =
> > > > > > > > container_of(fled_cdev,
> > > > > > > > struct
> > > > > > > > > qcom_flash_led, flash);
> > > > > > > > +       struct qcom_flash_chip *chip= led->chip;
> > > > > > > > +       u8 shift, chan_id = 0, chan_mask = 0;
> > > > > > > > +       u8 ot_mask = 0, oc_mask = 0, uv_mask = 0;
> > > > > > > > +       u32 val, fault_sts = 0;
> > > > > > > > +       int i, rc;
> > > > > > > > +
> > > > > > > > +       rc = regmap_field_read(chip-
> > > > > > > > >r_fields[REG_STATUS1],
> > > > > > > > &val);
> > > > > > > > +       if (rc < 0)
> > > > > > > > +               return rc;
> > > > > > > > +
> > > > > > > > +       for (i = 0; i < led->chan_count; i++) {
> > > > > > > > +               chan_id = led->chan_id[i];
> > > > > > > > +               shift = (chan_id - 1) * 2;
> > > > > > > > +               if (val & BIT(shift))
> > > > > > > > +                       fault_sts |=
> > > > > > > > LED_FAULT_SHORT_CIRCUIT;
> > > > > > > > +               chan_mask |= BIT(chan_id - 1);
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       rc = regmap_field_read(chip-
> > > > > > > > >r_fields[REG_STATUS2],
> > > > > > > > &val);
> > > > > > > > +       if (rc < 0)
> > > > > > > > +               return rc;
> > > > > > > > +
> > > > > > > > +       if (chip->hw_type == QCOM_MVFLASH_3CH) {
> > > > > > > > +               ot_mask = 0x0f;
> > > > > > > > +               oc_mask = 0xe0;
> > > > > > > > +               uv_mask = 0x10;
> > > > > > > > +       } else if (chip->hw_type == QCOM_MVFLASH_4CH) {
> > > > > > > > +               ot_mask = 0x70;
> > > > > > > > +               oc_mask = 0x0e;
> > > > > > > > +               uv_mask = 0x01;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       if (val & ot_mask)
> > > > > > > > +               fault_sts |=
> > > > > > > > LED_FAULT_OVER_TEMPERATURE;
> > > > > > > > +       if (val & oc_mask)
> > > > > > > > +               fault_sts |= LED_FAULT_OVER_CURRENT;
> > > > > > > > +       if (val & uv_mask)
> > > > > > > > +               fault_sts |= LED_FAULT_INPUT_VOLTAGE;
> > > > > > > > +
> > > > > > > > +       rc = regmap_field_read(chip-
> > > > > > > > >r_fields[REG_STATUS3],
> > > > > > > > &val);
> > > > > > > > +       if (rc < 0)
> > > > > > > > +               return rc;
> > > > > > > > +
> > > > > > > > +       if (chip->hw_type == QCOM_MVFLASH_3CH) {
> > > > > > > > +               if (val & chan_mask)
> > > > > > > > +                       fault_sts |= LED_FAULT_TIMEOUT;
> > > > > > > > +       } else if (chip->hw_type == QCOM_MVFLASH_4CH) {
> > > > > > > > +               for (i = 0; i < led->chan_count; i++) {
> > > > > > > > +                       chan_id = led->chan_id[i];
> > > > > > > > +                       shift = (chan_id - 1) * 2;
> > > > > > > > +                       if (val & BIT(shift))
> > > > > > > > +                               fault_sts |=
> > > > > > > > LED_FAULT_TIMEOUT;
> > > > > > > > +               }
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       *fault = fault_sts;
> > > > > > > > +       return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int qcom_flash_led_brightness_set(struct
> > > > > > > > led_classdev >
> > > > > > > > *led_cdev,
> > > > > > > > +                                       enum
> > > > > > > > led_brightness
> > > > > > > > > 
> > > > > > > > brightness)
> > > > > > > > +{
> > > > > > > > +       struct led_classdev_flash *fled_cdev =
> > > > > > > > +               container_of(led_cdev, struct
> > > > > > > > led_classdev_flash, >
> > > > > > > > led_cdev);
> > > > > > > > +       struct qcom_flash_led *led =
> > > > > > > > +               container_of(fled_cdev, struct
> > > > > > > > qcom_flash_led,
> > > > > > > > > 
> > > > > > > > flash);
> > > > > > > > +       u32 current_ma = brightness * led-
> > > > > > > > > max_torch_current_ma
> > > > > > > > / >
> > > > > > > > LED_FULL;
> > > > > > > > +       bool enable = !!brightness;
> > > > > > > > +       int rc;
> > > > > > > > +
> > > > > > > > +       rc = set_flash_current(led, current_ma,
> > > > > > > > TORCH_MODE);
> > > > > > > > +       if (rc < 0)
> > > > > > > > +               return rc;
> > > > > > > > +
> > > > > > > > +       /* disable flash timeout fortorch LED */
> > > > > > > > +       rc = set_flash_timeout(led, 0);
> > > > > > > > +       if (rc < 0)
> > > > > > > > +               return rc;
> > > > > > > > +
> > > > > > > > +       rc = set_flash_module_en(led, enable);
> > > > > > > > +       if (rc < 0)
> > > > > > > > +               return rc;
> > > > > > > > +
> > > > > > > > +       return set_flash_strobe(led,SW_STROBE, enable);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static const struct led_flash_ops qcom_flash_ops = {
> > > > > > > > +       .flash_brightness_set =
> > > > > > > > qcom_flash_brightness_set,
> > > > > > > > +       .strobe_set = qcom_flash_strobe_set,
> > > > > > > > +       .strobe_get = qcom_flash_strobe_get,
> > > > > > > > +       .timeout_set = qcom_flash_timeout_set,
> > > > > > > > +       .fault_get = qcom_flash_fault_get,
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
> > > > > > > > +static int qcom_flash_external_strobe_set(struct
> > > > > > > > v4l2_flash >
> > > > > > > > *v4l2_flash, bool enable)
> > > > > > > > +{
> > > > > > > > +       struct led_classdev_flash *flash = v4l2_flash-
> > > > > > > > > fled_cdev;
> > > > > > > > +       struct qcom_flash_led *led =
> > > > > > > > container_of(flash,
> > > > > > > > struct
> > > > > > > > > 
> > > > > > > > qcom_flash_led, flash);
> > > > > > > > +       int rc;
> > > > > > > > +
> > > > > > > > +       rc = set_flash_module_en(led, enable);
> > > > > > > > +       if (rc < 0)
> > > > > > > > +               return rc;
> > > > > > > > +
> > > > > > > > +       if (enable)
> > > > > > > > +               return set_flash_strobe(led, HW_STROBE,
> > > > > > > > true);
> > > > > > > > +       else
> > > > > > > > +               return set_flash_strobe(led, SW_STROBE,
> > > > > > > > false);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static enum led_brightness
> > > > > > > > qcom_flash_intensity_to_led_brightness(
> > > > > > > > +               struct v4l2_flash *v4l2_flash, s32
> > > > > > > > intensity)
> > > > > > > > +{
> > > > > > > > +       struct led_classdev_flash *flash = v4l2_flash-
> > > > > > > > > fled_cdev;
> > > > > > > > +       struct qcom_flash_led *led =
> > > > > > > > container_of(flash,
> > > > > > > > struct
> > > > > > > > > 
> > > > > > > > qcom_flash_led, flash);
> > > > > > > > +       u32 current_ma = intensity/ 1000;
> > > > > > > > +
> > > > > > > > +       current_ma = min_t(u32, current_ma, > led-
> > > > > > > > > max_torch_current_ma);
> > > > > > > > +       if (!current_ma)
> > > > > > > > +               return LED_OFF;
> > > > > > > > +
> > > > > > > > +       return current_ma * LED_FULL/ led-
> > > > > > > > > max_torch_current_ma;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static s32
> > > > > > > > qcom_flash_brightness_to_led_intensity(struct
> > > > > > > > v4l2_flash > *v4l2_flash,
> > > > > > > > +                                       enum
> > > > > > > > led_brightness
> > > > > > > > > 
> > > > > > > > brightness)
> > > > > > > > +{
> > > > > > > > +       struct led_classdev_flash *flash = v4l2_flash-
> > > > > > > > > fled_cdev;
> > > > > > > > +       struct qcom_flash_led *led =
> > > > > > > > container_of(flash,
> > > > > > > > struct
> > > > > > > > > 
> > > > > > > > qcom_flash_led, flash);
> > > > > > > > +
> > > > > > > > +       return (brightness * led->max_torch_current_ma
> > > > > > > > *
> > > > > > > > 1000)
> > > > > > > > / >
> > > > > > > > LED_FULL;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static const struct v4l2_flash_ops qcom_v4l2_flash_ops
> > > > > > > > = {
> > > > > > > > +       .external_strobe_set =
> > > > > > > > qcom_flash_external_strobe_set,
> > > > > > > > +       .intensity_to_led_brightness= >
> > > > > > > > qcom_flash_intensity_to_led_brightness,
> > > > > > > > +       .led_brightness_to_intensity= >
> > > > > > > > qcom_flash_brightness_to_led_intensity,
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +static int qcom_flash_v4l2_init(struct qcom_flash_led
> > > > > > > > *led,
> > > > > > > > struct
> > > > > > > > > fwnode_handle *fwnode)
> > > > > > > > +{
> > > > > > > > +       struct v4l2_flash_config v4l2_cfg = {0};
> > > > > > > > +       struct led_flash_setting *s =
> > > > > > > > &v4l2_cfg.intensity;
> > > > > > > > +
> > > > > > > > +       if (!(led->flash.led_cdev.flags &
> > > > > > > > LED_DEV_CAP_FLASH))
> > > > > > > > +               return 0;
> > > > > > > > +
> > > > > > > > +       s->min = s->step = TORCH_IRES_UA * led-
> > > > > > > > >chan_count;
> > > > > > > > +       s->max = led->max_torch_current_ma * 1000;
> > > > > > > > +       s->val = min_t(u32, s->max,
> > > > > > > > TORCH_CURRENT_DEFAULT_UA);
> > > > > > > > +
> > > > > > > > +       strscpy(v4l2_cfg.dev_name, >led-
> > > > > > > > > flash.led_cdev.dev-
> > > > > > > > > kobj.name,
> > > > > > > > +                                       sizeof(v4l2_cfg
> > > > > > > > .dev
> > > > > > > > _nam
> > > > > > > > e));
> > > > > > > > +       v4l2_cfg.has_external_strobe= 1;
> > > > > > > > +       v4l2_cfg.flash_faults = LED_FAULT_INPUT_VOLTAGE
> > > > > > > > | >
> > > > > > > > LED_FAULT_OVER_CURRENT |
> > > > > > > > +               LED_FAULT_SHORT_CIRCUIT |
> > > > > > > > LED_FAULT_OVER_TEMPERATURE > | LED_FAULT_TIMEOUT;
> > > > > > > > +
> > > > > > > > +       led->v4l2_flash = v4l2_flash_init(led->chip-
> > > > > > > > >dev,
> > > > > > > > fwnode, >
> > > > > > > > &led->flash,
> > > > > > > > +                                       &qcom_v4l2_flas
> > > > > > > > h_op
> > > > > > > > s, >
> > > > > > > > &v4l2_cfg);
> > > > > > > > +       return PTR_ERR_OR_ZERO(led->v4l2_flash);
> > > > > > > > +}
> > > > > > > > +# else
> > > > > > > > +static int qcom_flash_v4l2_init(struct qcom_flash_led
> > > > > > > > *led,
> > > > > > > > struct
> > > > > > > > > fwnode_handle *fwnode)
> > > > > > > > +{
> > > > > > > > +       return 0;
> > > > > > > > +}
> > > > > > > > +#endif
> > > > > > > > +
> > > > > > > > +static int qcom_flash_register_led_device(struct
> > > > > > > > device
> > > > > > > > *parent,
> > > > > > > > +               struct fwnode_handle *node, struct
> > > > > > > > qcom_flash_led >
> > > > > > > > *led)
> > > > > > > > +{
> > > > > > > > +       struct qcom_flash_chip *chip= led->chip;
> > > > > > > > +       struct led_init_data init_data;
> > > > > > > > +       struct led_classdev_flash *flash;
> > > > > > > > +       struct led_flash_setting *s;
> > > > > > > > +       u32 count, val;
> > > > > > > > +       u32 channels[4];
> > > > > > > > +       int i, rc;
> > > > > > > > +
> > > > > > > > +       flash = &led->flash;
> > > > > > > > +       count = fwnode_property_count_u32(node, "led-
> > > > > > > > sources");
> > > > > > > > +       if (count <= 0) {
> > > > > > > > +               dev_err(chip->dev, "No led-sources
> > > > > > > > specified\n");
> > > > > > > > +               return -ENODEV;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       if (count > chip->max_channels) {
> > > > > > > > +               dev_err(chip->dev, "led-sources count
> > > > > > > > %u
> > > > > > > > exceeds >
> > > > > > > > maximum channel count %u\n",
> > > > > > > > +                               count, chip-
> > > > > > > > >max_channels);
> > > > > > > > +               return -EINVAL;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       rc = fwnode_property_read_u32_array(node, "led-
> > > > > > > > sources", >
> > > > > > > > channels, count);
> > > > > > > > +       if (rc < 0) {
> > > > > > > > +               dev_err(chip->dev, "get led-sources
> > > > > > > > failed,
> > > > > > > > rc=%d\n", > rc);
> > > > > > > > +               return rc;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       led->chan_count = count;
> > > > > > > > +       led->chan_id = devm_kcalloc(chip->dev, count,
> > > > > > > > sizeof(u8), >
> > > > > > > > GFP_KERNEL);
> > > > > > > > +       if (!led->chan_id)
> > > > > > > > +               return -ENOMEM;
> > > > > > > > +
> > > > > > > > +       for (i = 0; i < count; i++) {
> > > > > > > > +               if (channels[i] > chip->max_channels) {
> > > > > > > > +                       dev_err(chip->dev, "led-source
> > > > > > > > out
> > > > > > > > of
> > > > > > > > HW >
> > > > > > > > support range [1-%u]\n",
> > > > > > > > +                                       chip-
> > > > > > > > > max_channels);
> > > > > > > > +                       return-EINVAL;
> > > > > > > > +               }
> > > > > > > > +
> > > > > > > > +               led->chan_id[i] = channels[i];
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       rc = fwnode_property_read_u32(node, "led-max-
> > > > > > > > microamp",
> > > > > > > > > 
> > > > > > > > &val);
> > > > > > > > +       if (rc < 0) {
> > > > > > > > +               dev_err(chip->dev, "Get led-max-
> > > > > > > > microamp
> > > > > > > > failed, >
> > > > > > > > rc=%d\n", rc);
> > > > > > > > +               return rc;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       if (!val) {
> > > > > > > > +               dev_err(chip->dev, "led-max-microamp
> > > > > > > > shouldn't
> > > > > > > > be >
> > > > > > > > 0\n");
> > > > > > > > +               return -EINVAL;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       val = min_t(u32, val, TORCH_CURRENT_MAX_UA * >
> > > > > > > > led-
> > > > > > > > > chan_count);
> > > > > > > > +       led->max_torch_current_ma = val / 1000;
> > > > > > > > +
> > > > > > > > +       if (fwnode_property_present(node, "flash-max-
> > > > > > > > microamp")) {
> > > > > > > > +               flash->led_cdev.flags |=
> > > > > > > > LED_DEV_CAP_FLASH;
> > > > > > > > +               rc = fwnode_property_read_u32(node, >
> > > > > > > > "flash-
> > > > > > > > max-
> > > > > > > > microamp", &val);
> > > > > > > > +               if (rc < 0) {
> > > > > > > > +                       dev_err(chip->dev, "Get flash-
> > > > > > > > max-
> > > > > > > > microamp
> > > > > > > > > failed, rc=%d\n", rc);
> > > > > > > > +                       returnrc;
> > > > > > > > +               }
> > > > > > > > +
> > > > > > > > +               val = min_t(u32, val,
> > > > > > > > FLASH_CURRENT_MAX_UA
> > > > > > > > * >
> > > > > > > > led-
> > > > > > > > > chan_count);
> > > > > > > > +               val = min_t(u32, val,
> > > > > > > > FLASH_TOTAL_CURRENT_MAX_UA);
> > > > > > > > +               s = &flash->brightness;
> > > > > > > > +               s->min = s->step = FLASH_IRES_UA * led-
> > > > > > > > > chan_count;
> > > > > > > > +               s->max = val;
> > > > > > > > +               s->val = min_t(u32, val,
> > > > > > > > FLASH_CURRENT_DEFAULT_UA);
> > > > > > > > +               led->max_flash_current_ma = val / 1000;
> > > > > > > > +               led->flash_current_ma = s->val / 1000;
> > > > > > > > +
> > > > > > > > +               rc = fwnode_property_read_u32(node, >
> > > > > > > > "flash-
> > > > > > > > max-
> > > > > > > > timeout-us", &val);
> > > > > > > > +               if (rc < 0) {
> > > > > > > > +                       dev_err(chip->dev, "Get flash-
> > > > > > > > max-
> > > > > > > > timeout-
> > > > > > > > us > failed, rc=%d\n", rc);
> > > > > > > > +                       returnrc;
> > > > > > > > +               }
> > > > > > > > +
> > > > > > > > +               val = min_t(u32, val,
> > > > > > > > FLASH_TIMEOUT_MAX_US);
> > > > > > > > +               s = &flash->timeout;
> > > > > > > > +               s->min = s->step =
> > > > > > > > FLASH_TIMEOUT_STEP_US;
> > > > > > > > +               s->val = s->max = val;
> > > > > > > > +               led->max_timeout_ms = led-
> > > > > > > > >flash_timeout_ms
> > > > > > > > =
> > > > > > > > val /
> > > > > > > > > 1000;
> > > > > > > > +
> > > > > > > > +               flash->ops = &qcom_flash_ops;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       flash->led_cdev.brightness_set_blocking = >
> > > > > > > > qcom_flash_led_brightness_set;
> > > > > > > > +       init_data.fwnode = node;
> > > > > > > > +       init_data.devicename = NULL;
> > > > > > > > +       init_data.default_label = NULL;
> > > > > > > > +       init_data.devname_mandatory = false;
> > > > > > > > +       rc =
> > > > > > > > devm_led_classdev_flash_register_ext(parent,
> > > > > > > > flash, >
> > > > > > > > &init_data);
> > > > > > > > +       if (rc < 0) {
> > > > > > > > +               dev_err(chip->dev, "Register flash LED
> > > > > > > > classdev
> > > > > > > > > 
> > > > > > > > failed, rc=%d\n", rc);
> > > > > > > > +               return rc;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       return qcom_flash_v4l2_init(led, node);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int qcom_flash_led_probe(struct platform_device
> > > > > > > > *pdev)
> > > > > > > > +{
> > > > > > > > +       struct qcom_flash_chip *chip;
> > > > > > > > +       struct qcom_flash_led *led;
> > > > > > > > +       struct fwnode_handle *child;
> > > > > > > > +       struct device *dev = &pdev->dev;
> > > > > > > > +       struct regmap *map;
> > > > > > > > +       struct reg_field *regs;
> > > > > > > > +       int count, i, rc;
> > > > > > > > +       u32 val, reg_base;
> > > > > > > > +
> > > > > > > > +       chip = devm_kzalloc(dev, sizeof(*chip),
> > > > > > > > GFP_KERNEL);
> > > > > > > > +       if (!chip)
> > > > > > > > +               return -ENOMEM;
> > > > > > > > +
> > > > > > > > +       map = dev_get_regmap(dev->parent, NULL);
> > > > > > > > +       if (!map) {
> > > > > > > > +               dev_err(dev, "Failed to get parent
> > > > > > > > regmap\n");
> > > > > > > > +               return -EINVAL;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       rc = fwnode_property_read_u32(dev->fwnode,
> > > > > > > > "reg",
> > > > > > > > &reg_base);
> > > > > > > > +       if (rc < 0) {
> > > > > > > > +               dev_err(dev, "Failed to get register
> > > > > > > > base
> > > > > > > > address,
> > > > > > > > > rc=%d\n", rc);
> > > > > > > > +               return rc;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       rc = regmap_read(map, reg_base +
> > > > > > > > FLASH_TYPE_REG,
> > > > > > > > &val);
> > > > > > > > +       if (rc < 0) {
> > > > > > > > +               dev_err(dev, "Read flash module type
> > > > > > > > failed, >
> > > > > > > > rc=%d\n", rc);
> > > > > > > > +               return rc;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       if (val != FLASH_TYPE_VAL){
> > > > > > > > +               dev_err(dev, "type %#x is not a flash
> > > > > > > > module\n", >
> > > > > > > > val);
> > > > > > > > +               return -ENODEV;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       rc = regmap_read(map, reg_base +
> > > > > > > > FLASH_SUBTYPE_REG,
> > > > > > > > &val);
> > > > > > > > +       if (rc < 0) {
> > > > > > > > +               dev_err(dev, "Read flash module subtype
> > > > > > > > failed,
> > > > > > > > > 
> > > > > > > > rc=%d\n", rc);
> > > > > > > > +               return rc;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       if (val == FLASH_SUBTYPE_3CH_VAL) {
> > > > > > > > +               chip->hw_type = QCOM_MVFLASH_3CH;
> > > > > > > > +               chip->max_channels = 3;
> > > > > > > > +               regs = mvflash_3ch_regs;
> > > > > > > > +       } else if (val == FLASH_SUBTYPE_4CH_VAL) {
> > > > > > > > +               chip->hw_type = QCOM_MVFLASH_4CH;
> > > > > > > > +               chip->max_channels = 4;
> > > > > > > > +               regs = mvflash_4ch_regs;
> > > > > > > > +       } else {
> > > > > > > > +               dev_err(dev, "flash subtype %#x is not
> > > > > > > > yet
> > > > > > > > > 
> > > > > > > > supported\n", val);
> > > > > > > > +               return -ENODEV;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       for (i = 0; i < REG_MAX_COUNT; i++)
> > > > > > > > +               regs[i].reg += reg_base;
> > > > > > > > +
> > > > > > > > +       rc = devm_regmap_field_bulk_alloc(dev, map,
> > > > > > > > chip-
> > > > > > > > > r_fields,
> > > > > > > > > regs, REG_MAX_COUNT);
> > > > > > > > +       if (rc < 0) {
> > > > > > > > +               dev_err(dev, "failed to alloc regmap
> > > > > > > > filed,
> > > > > > > > rc=%d\n", > rc);
> > > > > > > > +               return rc;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       chip->dev = dev;
> > > > > > > > +       platform_set_drvdata(pdev, chip);
> > > > > > > > +       mutex_init(&chip->lock);
> > > > > > > > +       count = device_get_child_node_count(dev);
> > > > > > > > +       if (count == 0 || count > chip->max_channels) {
> > > > > > > > +               dev_err(dev, "No child or child count
> > > > > > > > exceeds
> > > > > > > > %d\n", > chip->max_channels);
> > > > > > > > +               return -EINVAL;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       chip->leds = devm_kcalloc(dev, count,
> > > > > > > > sizeof(*chip-
> > > > > > > > > leds),
> > > > > > > > > GFP_KERNEL);
> > > > > > > > +       if (!chip->leds)
> > > > > > > > +               return -ENOMEM;
> > > > > > > > +
> > > > > > > > +       device_for_each_child_node(dev, child) {
> > > > > > > > +               led = &chip->leds[chip->leds_count];
> > > > > > > > +               led->chip = chip;
> > > > > > > > +               rc =
> > > > > > > > qcom_flash_register_led_device(dev,
> > > > > > > > child,
> > > > > > > > led);
> > > > > > > > +               if (rc < 0)
> > > > > > > > +                       goto release;
> > > > > > > > +
> > > > > > > > +               chip->leds_count++;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       return 0;
> > > > > > > > +release:
> > > > > > > > +       while (chip->leds && chip->leds_count--)
> > > > > > > > +               v4l2_flash_release(chip->leds[chip-
> > > > > > > > > leds_count].v4l2_> flash);
> > > > > > > > +       return rc;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +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" },
> > > > > > > > +       { }
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +static struct platform_driver qcom_flash_led_driver =
> > > > > > > > {
> > > > > > > > +       .driver = {
> > > > > > > > +               .name = "leds-qcom-flash",
> > > > > > > > +               .of_match_table =
> > > > > > > > qcom_flash_led_match_table,
> > > > > > > > +       },
> > > > > > > > +       .probe = qcom_flash_led_probe,
> > > > > > > > +       .remove = qcom_flash_led_remove,
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +module_platform_driver(qcom_flash_led_driver);
> > > > > > > > +
> > > > > > > > +MODULE_DESCRIPTION("QCOM Flash LED driver");
> > > > > > > > +MODULE_LICENSE("GPL");
> > > > > > 
> > > > > > Kind regards,
> > > > > > Dylan Van Assche
> > > > 
> > > > Kind regards,
> > > > Dylan Van Assche
> > > > 
> > 
> > Kind regards,
> > Dylan Van Assche
> > 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ