| 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
| ||
|
Message-ID: <278ea1e8-8b21-457d-78d7-fbb32544fe0a@gmail.com>
Date: Sun, 29 Aug 2021 13:15:45 +0200
From: Jacek Anaszewski <jacek.anaszewski@...il.com>
To: Nícolas F. R. A. Prado
<nfraprado@...labora.com>
Cc: Pavel Machek <pavel@....cz>, Dan Murphy <dmurphy@...com>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Andy Gross <agross@...nel.org>,
Rob Herring <robh+dt@...nel.org>, linux-leds@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
Brian Masney <masneyb@...tation.org>,
Luca Weiss <luca@...tu.xyz>,
Russell King <linux@...linux.org.uk>,
Georgi Djakov <georgi.djakov@...aro.org>,
linux-kernel@...r.kernel.org, phone-devel@...r.kernel.org,
~postmarketos/upstreaming@...ts.sr.ht, ~lkcamp/patches@...ts.sr.ht,
André Almeida <andrealmeid@...labora.com>,
kernel@...labora.com
Subject: Re: [PATCH v3 2/5] leds: Add driver for QCOM SPMI Flash LEDs
Hi Nicolas,
On 8/24/21 11:45 PM, Nícolas F. R. A. Prado wrote:
> Hi Jacek,
>
> Thank you for the review. I'll answer below.
>
> On Tue, Aug 17, 2021 at 12:03:49AM +0200, Jacek Anaszewski wrote:
>> Hi Nicolas,
>>
>> Thanks for the update. See my comments below.
>>
>> On 8/3/21 6:26 PM, Nícolas F. R. A. Prado wrote:
>>> Add driver for Qualcomm's SPMI Flash LEDs. These are controlled
>>> through an SPMI bus and are part of the PM8941 PMIC. There are two LEDs
>>> present on the chip, which can be used independently as camera flash or,
>>> when in torch mode, as a lantern.
>>>
>>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@...labora.com>
>>> ---
>>>
>>> Changes in v3:
>>> - The two LEDs can now be operated independently even when in torch mode
>>> - The flashes can now strobe consecutive times without needing to manually
>>> disable them between strobes
>>> - Implemented strobe_get()
>>> - Moved dt parsing to a separate function
>>> - Split multiplexed torch/flash on/off and torch/flash regulator on/off
>>> functions
>>> - Set current on brightness callback and timeout on timeout callback, instead of
>>> setting everything every time when strobing/turning torch on
>>> - And a whole lot of other minor changes
>>>
>>> Changes in v2:
>>> - Thanks to Jacek:
>>> - Implemented flash LED class framework
>>> - Thanks to Bjorn:
>>> - Renamed driver to "qcom spmi flash"
>>> - Refactored code
>>> - Added missing copyright
>>>
>>> drivers/leds/flash/Kconfig | 8 +
>>> drivers/leds/flash/Makefile | 1 +
>>> drivers/leds/flash/leds-qcom-spmi-flash.c | 1251 +++++++++++++++++++++
>>> 3 files changed, 1260 insertions(+)
>>> create mode 100644 drivers/leds/flash/leds-qcom-spmi-flash.c
>>>
>>> diff --git a/drivers/leds/flash/Kconfig b/drivers/leds/flash/Kconfig
>>> index 3f49f3edbffb..72f61323cd2a 100644
>>> --- a/drivers/leds/flash/Kconfig
>>> +++ b/drivers/leds/flash/Kconfig
>>> @@ -24,4 +24,12 @@ config LEDS_RT8515
>>> To compile this driver as a module, choose M here: the module
>>> will be called leds-rt8515.
>>> +config LEDS_QCOM_SPMI_FLASH
>>> + tristate "Support for QCOM SPMI Flash LEDs"
>>> + depends on REGMAP_SPMI
>>> + depends on OF
>>> + help
>>> + This option enables support for the flash/torch LEDs present in
>>> + Qualcomm's PM8941 PMIC.
>>> +
>>> endif # LEDS_CLASS_FLASH
>>> diff --git a/drivers/leds/flash/Makefile b/drivers/leds/flash/Makefile
>>> index 09aee561f769..c141d277e9b6 100644
>>> --- a/drivers/leds/flash/Makefile
>>> +++ b/drivers/leds/flash/Makefile
>>> @@ -2,3 +2,4 @@
>>> obj-$(CONFIG_LEDS_RT4505) += leds-rt4505.o
>>> obj-$(CONFIG_LEDS_RT8515) += leds-rt8515.o
>>> +obj-$(CONFIG_LEDS_QCOM_SPMI_FLASH) += leds-qcom-spmi-flash.o
>>> diff --git a/drivers/leds/flash/leds-qcom-spmi-flash.c b/drivers/leds/flash/leds-qcom-spmi-flash.c
>>> new file mode 100644
>>> index 000000000000..9763707bb986
>>> --- /dev/null
>>> +++ b/drivers/leds/flash/leds-qcom-spmi-flash.c
>>> @@ -0,0 +1,1251 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Qualcomm SPMI Flash LEDs Driver
>>> + *
>>> + * Copyright (c) 2020-2021, Nícolas F. R. A. Prado <n@...aprado.net>
>>> + * Copyright (c) 2021, Collabora Ltd.
>>> + *
>>> + * Based on QPNP LEDs driver from downstream MSM kernel sources.
>>> + * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
>>> + */
>>> +
>>> +#include <linux/delay.h>
>>> +#include <linux/device.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/led-class-flash.h>
>>> +#include <linux/module.h>
>>> +#include <linux/mutex.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/regulator/consumer.h>
>>> +#include <linux/spmi.h>
>>> +#include <linux/string.h>
>>> +#include <linux/sysfs.h>
>>> +#include <linux/types.h>
>>> +
>>> +#define QCOM_FLASH_ADDR_PERIPHERAL_SUBTYPE 0x05
>>> +#define QCOM_FLASH_ADDR_STATUS 0x10
>>> +#define QCOM_FLASH_ADDR_SAFETY_TIMER 0x40
>>> +#define QCOM_FLASH_ADDR_MAX_CURR 0x41
>>> +#define QCOM_FLASH_ADDR_CURR_LED0 0x42
>>> +#define QCOM_FLASH_ADDR_CURR_LED1 0x43
>>> +#define QCOM_FLASH_ADDR_CLAMP_CURR 0x44
>>> +#define QCOM_FLASH_ADDR_ENABLE_CONTROL 0x46
>>> +#define QCOM_FLASH_ADDR_LED_STROBE_CTRL 0x47
>>> +#define QCOM_FLASH_ADDR_LED_TMR_CTRL 0x48
>>> +#define QCOM_FLASH_ADDR_HEADROOM 0x4A
>>> +#define QCOM_FLASH_ADDR_STARTUP_DELAY 0x4B
>>> +#define QCOM_FLASH_ADDR_MASK_ENABLE 0x4C
>>> +#define QCOM_FLASH_ADDR_VREG_OK_FORCE 0x4F
>>> +#define QCOM_FLASH_ADDR_LED_UNLOCK_SECURE 0xD0
>>> +#define QCOM_FLASH_ADDR_LED_TORCH 0xE4
>>> +#define QCOM_FLASH_ADDR_FAULT_DETECT 0x51
>>> +#define QCOM_FLASH_ADDR_RAMP_RATE 0x54
>>> +#define QCOM_FLASH_ADDR_VPH_PWR_DROOP 0x5A
>>> +
>>> +#define QCOM_FLASH_MAX_LEVEL 0x4F
>>> +#define QCOM_FLASH_TORCH_MAX_LEVEL 0x0F
>>> +#define QCOM_FLASH_NO_MASK 0x00
>>> +
>>> +#define QCOM_FLASH_MASK_1 0x20
>>> +#define QCOM_FLASH_MASK_REG_MASK 0xE0
>>> +#define QCOM_FLASH_HEADROOM_MASK 0x03
>>> +#define QCOM_FLASH_SAFETY_TIMER_MASK 0x7F
>>> +#define QCOM_FLASH_CURRENT_MASK 0xFF
>>> +#define QCOM_FLASH_MAX_CURRENT_MASK 0x7F
>>> +#define QCOM_FLASH_TMR_MASK 0x03
>>> +#define QCOM_FLASH_TMR_WATCHDOG 0x03
>>> +#define QCOM_FLASH_TMR_SAFETY 0x00
>>> +#define QCOM_FLASH_FAULT_DETECT_MASK 0x80
>>> +#define QCOM_FLASH_HW_VREG_OK 0x40
>>> +#define QCOM_FLASH_VREG_MASK 0xC0
>>> +#define QCOM_FLASH_STARTUP_DLY_MASK 0x02
>>> +#define QCOM_FLASH_RAMP_RATE_MASK 0xBF
>>> +#define QCOM_FLASH_VPH_PWR_DROOP_MASK 0xF3
>>> +
>>> +#define QCOM_FLASH_ENABLE_ALL 0xE0
>>> +#define QCOM_FLASH_ENABLE_MODULE 0x80
>>> +#define QCOM_FLASH_ENABLE_MODULE_MASK 0x80
>>> +#define QCOM_FLASH_DISABLE_ALL 0x00
>>> +#define QCOM_FLASH_ENABLE_MASK 0xE0
>>> +#define QCOM_FLASH_ENABLE_LED0 0xC0
>>> +#define QCOM_FLASH_ENABLE_LED1 0xA0
>>> +#define QCOM_FLASH_INIT_MASK 0xE0
>>> +#define QCOM_FLASH_SELFCHECK_ENABLE 0x80
>>> +#define QCOM_FLASH_SELFCHECK_DISABLE 0x00
>>> +
>>> +#define QCOM_FLASH_STROBE_SW 0xC0
>>> +#define QCOM_FLASH_STROBE_HW 0x04
>>> +#define QCOM_FLASH_STROBE_MASK 0xC7
>>> +#define QCOM_FLASH_STROBE_LED0 0x80
>>> +#define QCOM_FLASH_STROBE_LED1 0x40
>>> +
>>> +#define QCOM_FLASH_TORCH_MASK 0x03
>>> +#define QCOM_FLASH_LED_TORCH_ENABLE 0x00
>>> +#define QCOM_FLASH_LED_TORCH_DISABLE 0x03
>>> +#define QCOM_FLASH_UNLOCK_SECURE 0xA5
>>> +#define QCOM_FLASH_SECURE_MASK 0xFF
>>> +
>>> +#define QCOM_FLASH_SUBTYPE_DUAL 0x01
>>> +#define QCOM_FLASH_SUBTYPE_SINGLE 0x02
>>> +
>>> +#define QCOM_FLASH_DURATION_STEP 10000
>>> +#define QCOM_FLASH_DURATION_MIN 10000
>>> +#define QCOM_FLASH_DURATION_DEFAULT 200000
>>> +
>>> +#define QCOM_FLASH_CURRENT_STEP 12500
>>> +#define QCOM_FLASH_CURRENT_MIN 12500
>>> +
>>> +#define QCOM_FLASH_DEFAULT_CLAMP 200000
>>> +
>>> +#define QCOM_FLASH_MASK_ON_LED0 0x8
>>> +#define QCOM_FLASH_MASK_ON_LED1 0x2
>>> +#define QCOM_FLASH_MASK_STATUS_TIMEDOUT 0x5
>>> +
>>> +enum qcom_flash_headroom {
>>> + QCOM_FLASH_HEADROOM_250MV,
>>> + QCOM_FLASH_HEADROOM_300MV,
>>> + QCOM_FLASH_HEADROOM_400MV,
>>> + QCOM_FLASH_HEADROOM_500MV,
>>> +};
>>> +
>>> +enum qcom_flash_startup_dly {
>>> + QCOM_FLASH_STARTUP_DLY_10US,
>>> + QCOM_FLASH_STARTUP_DLY_32US,
>>> + QCOM_FLASH_STARTUP_DLY_64US,
>>> + QCOM_FLASH_STARTUP_DLY_128US,
>>> +};
>>> +
>>> +enum qcom_flash_ids {
>>> + QCOM_FLASH_ID_LED0,
>>> + QCOM_FLASH_ID_LED1,
>>> +};
>>> +
>>> +/**
>>> + * struct qcom_flash_led - Represents each individual flash LED
>>> + * @fled_cdev: flash LED classdev
>>> + * @id: LED ID as given by enum qcom_flash_ids
>>> + * @default_on: default state for the LED
>>> + * @flash_enable_cmd: enable command for particular flash
>>> + * @flash_strobe_cmd: strobe command for particular flash
>>> + * @current_addr: address to write for current
>>> + * @mask_led_on: bitmask for STATUS register that shows if LED is on
>>> + * @flash_current_invalidated: whether the flash current in the current register
>>> + * was invalidated by torch usage
>>> + */
>>> +struct qcom_flash_led {
>>> + struct led_classdev_flash fled_cdev;
>>> + enum qcom_flash_ids id;
>>> + bool default_on;
>>> + u8 flash_enable_cmd;
>>> + u8 flash_strobe_cmd;
>>> + u16 current_addr;
>>> + u8 mask_led_on;
>>> + bool flash_current_invalidated;
>>> +};
>>> +
>>> +/**
>>> + * struct qcom_flash_device - QCOM SPMI Flash device, contains 2 flash LEDs
>>> + * @regmap: regmap used to access LED registers over SPMI
>>> + * @base: base register given in device tree
>>> + * @dev: device from devicetree
>>> + * @flash_supply: voltage regulator to supply the flashes
>>> + * @torch_supply: voltage regulator to supply torch mode
>>> + * @leds: flash LEDs
>>> + * @num_leds: number of LEDs registered (between 0 and 2)
>>> + * @lock: lock to protect SPMI transactions
>>> + * @torch_enable_cmd: enable flash LED torch mode
>>> + * @peripheral_subtype: module peripheral subtype
>>> + * @flash_regulator_on: flash regulator status
>>> + * @torch_regulator_on: torch regulator status
>>> + * @torch_enabled: whether torch mode is enabled
>>> + */
>>> +struct qcom_flash_device {
>>> + struct regmap *regmap;
>>> + unsigned int base;
>>> + struct device *dev;
>>> + struct regulator *flash_supply;
>>> + struct regulator *torch_supply;
>>> + struct qcom_flash_led leds[2];
>>> + u8 num_leds;
>>> + struct mutex lock;
>>> + u8 torch_enable_cmd;
>>> + unsigned int peripheral_subtype;
>>> + bool flash_regulator_on;
>>> + bool torch_regulator_on;
>>> + bool torch_enabled;
>>> +};
>>> +
>>> +struct qcom_flash_config {
>>> + unsigned int base;
>>> + struct regulator *flash_supply;
>>> + struct regulator *torch_supply;
>>> + unsigned int num_leds;
>>> +
>>> + enum qcom_flash_ids id[2];
>>> + bool default_on[2];
>>> + u32 led_max_brightness[2];
>>> + u32 flash_max_brightness[2];
>>> + u32 flash_max_timeout[2];
>>> +};
>>> +
>>> +static inline struct qcom_flash_led *flcdev_to_led(struct led_classdev_flash *fled_cdev)
>>> +{
>>> + return container_of(fled_cdev, struct qcom_flash_led, fled_cdev);
>>> +}
>>> +
>>> +static inline struct qcom_flash_device *led_to_leds_dev(struct qcom_flash_led *led)
>>> +{
>>> + return container_of(led, struct qcom_flash_device, leds[led->id]);
>>> +}
>>> +
>>> +static inline int qcom_flash_read_reg(struct qcom_flash_device *leds_dev,
>>> + unsigned int reg, unsigned int *val)
>>> +{
>>> + return regmap_read(leds_dev->regmap, leds_dev->base + reg, val);
>>> +}
>>> +
>>> +static inline int qcom_flash_masked_write(struct qcom_flash_device *leds_dev,
>>> + unsigned int reg, unsigned int mask,
>>> + unsigned int val)
>>> +{
>>> + return regmap_update_bits(leds_dev->regmap, leds_dev->base + reg, mask,
>>> + val);
>>> +}
>>> +
>>> +static u8 qcom_flash_duration_to_reg(u32 us)
>>> +{
>>> + if (us < QCOM_FLASH_DURATION_MIN)
>>> + us = QCOM_FLASH_DURATION_MIN;
>>> + return (us - QCOM_FLASH_DURATION_MIN) / QCOM_FLASH_DURATION_STEP;
>>> +}
>>> +
>>> +static u8 qcom_flash_current_to_reg(u32 ua)
>>> +{
>>> + if (ua < QCOM_FLASH_CURRENT_MIN)
>>> + ua = QCOM_FLASH_CURRENT_MIN;
>>> + return (ua - QCOM_FLASH_CURRENT_MIN) / QCOM_FLASH_CURRENT_STEP;
>>> +}
>>> +
>>> +static void clamp_align(u32 *v, u32 min, u32 max, u32 step)
>>> +{
>>> + *v = clamp_val(*v, min, max);
>>> + if (step > 1)
>>> + *v = (*v - min) / step * step + min;
>>> +}
>>> +
>>> +static int qcom_flash_status_get(struct qcom_flash_led *led)
>>> +{
>>> + struct qcom_flash_device *leds_dev = led_to_leds_dev(led);
>>> + unsigned int status;
>>> + int rc;
>>> +
>>> + rc = qcom_flash_read_reg(leds_dev, QCOM_FLASH_ADDR_STATUS, &status);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Failure reading status, rc = %d\n",
>>> + rc);
>>> + return rc;
>>> + }
>>> +
>>> + return status & led->mask_led_on;
>>> +}
>>> +
>>> +static int qcom_flash_status_clear(struct qcom_flash_device *leds_dev)
>>> +{
>>> + unsigned int enable_val;
>>> + int rc;
>>> +
>>> + rc = qcom_flash_read_reg(leds_dev, QCOM_FLASH_ADDR_ENABLE_CONTROL,
>>> + &enable_val);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Enable reg read failed(%d)\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_ENABLE_CONTROL,
>>> + QCOM_FLASH_ENABLE_MASK, QCOM_FLASH_DISABLE_ALL);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Enable reg write failed(%d)\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_ENABLE_CONTROL,
>>> + QCOM_FLASH_ENABLE_MASK, enable_val);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Enable reg write failed(%d)\n", rc);
>>
>> It would be good to have different error messages to discern between
>> the two above calls' failures.
>
> Indeed.
>
>>
>>> + return rc;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int qcom_flash_check_timedout(struct qcom_flash_device *leds_dev)
>>> +{
>>> + unsigned int status;
>>> + int rc;
>>> +
>>> + rc = qcom_flash_read_reg(leds_dev, QCOM_FLASH_ADDR_STATUS, &status);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Failure reading status, rc = %d\n",
>>> + rc);
>>> + return rc;
>>> + }
>>> +
>>> + return status & QCOM_FLASH_MASK_STATUS_TIMEDOUT;
>>> +}
>>> +
>>> +static int qcom_flash_torch_reg_enable(struct qcom_flash_device *leds_dev,
>>> + bool state)
>>> +{
>>> + int rc;
>>> +
>>> + if (leds_dev->torch_enabled == state)
>>> + return 0;
>>> +
>>> + /*
>>> + * For the TORCH register (0xE4) to become writable, the UNLOCK_SECURE
>>> + * register (0xD0) needs to be written with the UNLOCK_SECURE value
>>> + * (0xA5) first.
>>> + * It gets re-locked after any register write.
>>> + */
>>> + rc = qcom_flash_masked_write(leds_dev,
>>> + QCOM_FLASH_ADDR_LED_UNLOCK_SECURE,
>>> + QCOM_FLASH_SECURE_MASK,
>>> + QCOM_FLASH_UNLOCK_SECURE);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Secure reg write failed(%d)\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_LED_TORCH,
>>> + QCOM_FLASH_TORCH_MASK,
>>> + state ? QCOM_FLASH_LED_TORCH_ENABLE :
>>> + QCOM_FLASH_LED_TORCH_DISABLE);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Torch reg write failed(%d)\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int qcom_flash_max_brightness_set(struct qcom_flash_led *led,
>>> + unsigned int brightness)
>>> +{
>>> + struct qcom_flash_device *leds_dev = led_to_leds_dev(led);
>>> + struct device *dev = leds_dev->dev;
>>> + int rc;
>>> +
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_MAX_CURR,
>>> + QCOM_FLASH_CURRENT_MASK, brightness);
>>> + if (rc) {
>>> + dev_err(dev, "Max current reg write failed(%d)\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int qcom_flash_brightness_set(struct qcom_flash_led *led,
>>> + unsigned int brightness)
>>> +{
>>> + struct qcom_flash_device *leds_dev = led_to_leds_dev(led);
>>> + struct device *dev = leds_dev->dev;
>>> + int rc;
>>> +
>>> + rc = qcom_flash_masked_write(leds_dev, led->current_addr,
>>> + QCOM_FLASH_CURRENT_MASK, brightness);
>>> + if (rc) {
>>> + dev_err(dev, "Current reg write failed(%d)\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int qcom_flash_fled_regulator_on(struct qcom_flash_device *leds_dev)
>>> +{
>>> + int rc;
>>> +
>>> + if (leds_dev->flash_regulator_on)
>>> + return 0;
>>> +
>>> + rc = regulator_enable(leds_dev->flash_supply);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Regulator enable failed(%d)\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + leds_dev->flash_regulator_on = true;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int qcom_flash_fled_regulator_off(struct qcom_flash_device *leds_dev)
>>> +{
>>> + unsigned int i;
>>> + u8 enable = 0;
>>> + int rc;
>>> +
>>> + if (!leds_dev->flash_regulator_on)
>>> + return 0;
>>> +
>>> + for (i = 0; i < leds_dev->num_leds; i++) {
>>> + rc = qcom_flash_status_get(&leds_dev->leds[i]);
>>> + if (rc < 0)
>>> + return rc;
>>> +
>>> + if (!rc)
>>> + continue;
>>> +
>>> + enable |= leds_dev->leds[i].flash_enable_cmd;
>>> + }
>>> +
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_ENABLE_CONTROL,
>>> + QCOM_FLASH_ENABLE_MASK, enable);
>>> + if (rc)
>>> + dev_err(leds_dev->dev, "Enable reg write failed(%d)\n", rc);
>>> +
>>> + if (enable)
>>> + return 0;
>>> +
>>> + rc = regulator_disable(leds_dev->flash_supply);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Regulator disable failed(%d)\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + leds_dev->flash_regulator_on = false;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int qcom_flash_torch_regulator_on(struct qcom_flash_device *leds_dev)
>>> +{
>>> + int rc;
>>> +
>>> + if (leds_dev->torch_regulator_on)
>>> + return 0;
>>> +
>>> + rc = regulator_enable(leds_dev->torch_supply);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Regulator enable failed(%d)\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + leds_dev->torch_regulator_on = true;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int qcom_flash_torch_regulator_off(struct qcom_flash_device *leds_dev)
>>> +{
>>> + int rc;
>>> +
>>> + if (!leds_dev->torch_regulator_on)
>>> + return 0;
>>> +
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_ENABLE_CONTROL,
>>> + QCOM_FLASH_ENABLE_MODULE_MASK,
>>> + QCOM_FLASH_DISABLE_ALL);
>>> + if (rc)
>>> + dev_err(leds_dev->dev, "Enable reg write failed(%d)\n", rc);
>>> +
>>> + rc = regulator_disable(leds_dev->torch_supply);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Regulator disable failed(%d)\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + leds_dev->torch_regulator_on = false;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int qcom_flash_fled_on(struct qcom_flash_led *led)
>>> +{
>>> + struct qcom_flash_device *leds_dev = led_to_leds_dev(led);
>>> + struct device *dev = leds_dev->dev;
>>> + int rc, error;
>>> +
>>> + rc = qcom_flash_fled_regulator_on(leds_dev);
>>> + if (rc)
>>> + goto error_flash_set;
>>> +
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_ENABLE_CONTROL,
>>> + led->flash_enable_cmd,
>>> + led->flash_enable_cmd);
>>> + if (rc) {
>>> + dev_err(dev, "Enable reg write failed(%d)\n", rc);
>>> + goto error_flash_set;
>>> + }
>>> +
>>> + /*
>>> + * TODO The downstream driver also allowed HW strobe. Add support for it
>>> + * after v4l2 support is added and ISP can be used
>>> + */
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_LED_STROBE_CTRL,
>>> + led->flash_strobe_cmd,
>>> + led->flash_strobe_cmd);
>>> + if (rc) {
>>> + dev_err(dev, "LED %d strobe reg write failed(%d)\n", led->id,
>>> + rc);
>>> + goto error_flash_set;
>>> + }
>>> +
>>> + return 0;
>>> +
>>> +error_flash_set:
>>> + error = qcom_flash_fled_regulator_off(leds_dev);
>>> + if (error)
>>> + return error;
>>> + return rc;
>>> +}
>>> +
>>> +static int qcom_flash_fled_off(struct qcom_flash_led *led)
>>> +{
>>> + struct qcom_flash_device *leds_dev = led_to_leds_dev(led);
>>> + struct device *dev = leds_dev->dev;
>>> + int rc, error;
>>> +
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_LED_STROBE_CTRL,
>>> + led->flash_strobe_cmd,
>>> + QCOM_FLASH_DISABLE_ALL);
>>> + if (rc)
>>> + dev_err(dev, "LED %d flash write failed(%d)\n", led->id, rc);
>>> +
>>> + error = qcom_flash_fled_regulator_off(leds_dev);
>>> + if (error)
>>> + return error;
>>> + return rc;
>>> +}
>>> +
>>> +static int qcom_flash_torch_on(struct qcom_flash_led *led)
>>> +{
>>> + int rc, error;
>>> + struct qcom_flash_device *leds_dev = led_to_leds_dev(led);
>>> + struct device *dev = leds_dev->dev;
>>> +
>>> + if (leds_dev->peripheral_subtype == QCOM_FLASH_SUBTYPE_DUAL) {
>>> + rc = qcom_flash_torch_regulator_on(leds_dev);
>>> + if (rc)
>>> + goto error_reg_write;
>>> + } else if (leds_dev->peripheral_subtype == QCOM_FLASH_SUBTYPE_SINGLE) {
>>> + rc = qcom_flash_fled_regulator_on(leds_dev);
>>
>> Why for torch mode you need to enable fled regulator?
>
> Based on [1], apparently the hardware present in the Single variant of the PMIC
> has some limitation that requires the use of the flash regulator and the value
> QCOM_FLASH_ENABLE_ALL to enable the LEDs for the torch mode. The Dual variant on
> the other hand can just use the torch regulator and enables the LEDs with
> QCOM_FLASH_ENABLE_MODULE.
>
> [1] https://github.com/AICP/kernel_lge_hammerhead/commit/0f47c747c074993655d0bfebd045e8ddd228fe4c
>
> I'm honestly not sure what the impact is on using the different regulators and
> enable values. I have tested enabling the Dual PMIC with different enable values
> and all seemed to work the same, so must be some hardware detail.
>
> I left that Single codepath in the hope that it is useful for devices that have
> that variant of the hardware, but I have only actually tested the Dual PMIC,
> which is the one present on the Nexus 5.
Thanks for the explanation. Just wanted to confirm that it was not
a mistake.
>>
>>> + if (rc)
>>> + goto error_flash_set;
>>> +
>>> + /*
>>> + * Write 0x80 to MODULE_ENABLE before writing
>>> + * 0xE0 in order to avoid a hardware bug caused
>>> + * by register value going from 0x00 to 0xE0.
>>> + */
>>> + rc = qcom_flash_masked_write(leds_dev,
>>> + QCOM_FLASH_ADDR_ENABLE_CONTROL,
>>> + QCOM_FLASH_ENABLE_MODULE_MASK,
>>> + QCOM_FLASH_ENABLE_MODULE);
>>> + if (rc) {
>>> + dev_err(dev, "Enable reg write failed(%d)\n", rc);
>>> + goto error_flash_set;
>>> + }
>>> + }
>>> +
>>> + rc = qcom_flash_torch_reg_enable(leds_dev, true);
>>> + if (rc)
>>> + goto error_reg_write;
>>> +
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_ENABLE_CONTROL,
>>> + QCOM_FLASH_ENABLE_MASK,
>>> + leds_dev->torch_enable_cmd);
>>> + if (rc) {
>>> + dev_err(dev, "Enable reg write failed(%d)\n", rc);
>>> + goto error_reg_write;
>>> + }
>>> +
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_LED_STROBE_CTRL,
>>> + led->flash_strobe_cmd,
>>> + led->flash_strobe_cmd);
>>
>> Just to make sure - the hardware requires strobe cmd to enable torch?
>
> Yes. The strobe value is the one that actually turns each of the LEDs on,
> doesn't matter if it's on flash or torch mode. The difference in torch mode is
> actually just that the timeout on the LEDs is disabled (done by writing 0x00
> into the TORCH, 0xE4, register).
> So for both modes, the LEDs are turned on by writing to the STROBE_CTRL, 0x47,
> register. If torch is on they'll stay on indefinitely, while on flash mode
> they'll turn off after the timeout.
>
> Perhaps it's just a naming issue?
I propose to add these comments next to the calls in question.
>>
>>> + if (rc) {
>>> + dev_err(dev, "LED %d strobe reg write failed(%d)\n", led->id,
>>> + rc);
>>> + goto error_reg_write;
>>> + }
>>> +
>>> + leds_dev->torch_enabled = true;
>>> +
>>> + return 0;
>>> +
>>> +error_reg_write:
>>> + if (leds_dev->peripheral_subtype == QCOM_FLASH_SUBTYPE_SINGLE)
>>> + goto error_flash_set;
>>> +
>>> + error = qcom_flash_torch_regulator_off(leds_dev);
>>> + if (error)
>>> + return error;
>>> + return rc;
>>> +
>>> +error_flash_set:
>>> + error = qcom_flash_fled_regulator_off(leds_dev);
>>> + if (error)
>>> + return error;
>>> + return rc;
>>> +}
>>> +
>>> +static int qcom_flash_torch_off(struct qcom_flash_led *led)
>>> +{
>>> + struct qcom_flash_device *leds_dev = led_to_leds_dev(led);
>>> + struct device *dev = leds_dev->dev;
>>> + int rc, error;
>>> + unsigned int i;
>>> +
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_LED_STROBE_CTRL,
>>> + led->flash_strobe_cmd, QCOM_FLASH_DISABLE_ALL);
>>> + if (rc) {
>>> + dev_err(dev, "LED %d flash write failed(%d)\n", led->id, rc);
>>> + goto error_torch_set;
>>> + }
>>> +
>>> + /* Keep torch on if the other LED is still on */
>>> + for (i = 0; i < leds_dev->num_leds; i++)
>>> + if (leds_dev->leds[i].fled_cdev.led_cdev.brightness)
>>> + return 0;
>>> +
>>> + rc = qcom_flash_torch_reg_enable(leds_dev, false);
>>> + if (rc)
>>> + goto error_torch_set;
>>> +
>>> + if (leds_dev->peripheral_subtype == QCOM_FLASH_SUBTYPE_DUAL) {
>>> + rc = qcom_flash_torch_regulator_off(leds_dev);
>>> + if (rc)
>>> + return rc;
>>> + } else if (leds_dev->peripheral_subtype == QCOM_FLASH_SUBTYPE_SINGLE) {
>>> + rc = qcom_flash_fled_regulator_off(leds_dev);
>>> + if (rc)
>>> + return rc;
>>> + }
>>> +
>>> + leds_dev->torch_enabled = false;
>>> +
>>> + return 0;
>>> +
>>> +error_torch_set:
>>> + error = qcom_flash_torch_regulator_off(leds_dev);
>>> + if (error)
>>> + return error;
>>> + return rc;
>>> +}
>>> +
>>> +static int qcom_flash_ledcdev_brightness_set(struct led_classdev *led_cdev,
>>> + enum led_brightness value)
>>> +{
>>> + struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
>>> + struct qcom_flash_led *led = flcdev_to_led(fled_cdev);
>>> + struct qcom_flash_device *leds_dev = led_to_leds_dev(led);
>>> + unsigned int max_brightness;
>>> + int rc;
>>> +
>>> + if (value > led_cdev->max_brightness) {
>>
>> LED framework takes care of it. You can skip this.
>
> Ok.
>
>>
>>> + dev_err(leds_dev->dev, "Invalid brightness value\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + mutex_lock(&leds_dev->lock);
>>> + if (!value) {
>>> + rc = qcom_flash_torch_off(led);
>>> + } else {
>>> + /*
>>> + * The minimum on value for the brightness register is 0, but for
>>> + * led_classdev is 1, as 0 there means off.
>>> + */
>>> + rc = qcom_flash_brightness_set(led, led_cdev->brightness - 1);
>>> + if (rc)
>>> + goto unlock;
>>> +
>>> + led->flash_current_invalidated = true;
>>> +
>>> + if (leds_dev->torch_enabled) {
>>> + /* Clear status to update brightness */
>>> + rc = qcom_flash_status_clear(leds_dev);
>>> + if (rc)
>>> + goto unlock;
>>> + } else {
>>> + /*
>>> + * Clear status from previous flash strobe so the LED
>>> + * can turn on
>>> + */
>>> + rc = qcom_flash_check_timedout(leds_dev);
>>> + if (rc > 0)
>>> + rc = qcom_flash_status_clear(leds_dev);
>>> + else if (rc < 0)
>>> + goto unlock;
>>> +
>>> + max_brightness = led_cdev->max_brightness - 1;
>>> + rc = qcom_flash_max_brightness_set(led, max_brightness);
>>> + if (rc)
>>> + goto unlock;
>>> + }
>>> + rc = qcom_flash_torch_on(led);
>>> + }
>>> +
>>> +unlock:
>>> + mutex_unlock(&leds_dev->lock);
>>> + return rc;
>>> +}
>>> +
>>> +static int qcom_flash_flcdev_brightness_set(struct led_classdev_flash *fled_cdev,
>>> + u32 brightness)
>>> +{
>>> + struct qcom_flash_led *led = flcdev_to_led(fled_cdev);
>>> + struct qcom_flash_device *leds_dev = led_to_leds_dev(led);
>>> + unsigned int bright;
>>> + int rc;
>>> +
>>> + /* Can't operate on flash if torch is on */
>>> + if (leds_dev->torch_enabled)
>>> + return -EBUSY;
>>> +
>>> + clamp_align(&brightness, QCOM_FLASH_CURRENT_MIN,
>>> + fled_cdev->brightness.max, QCOM_FLASH_CURRENT_STEP);
>>> + fled_cdev->brightness.val = brightness;
>>> +
>>> + bright = qcom_flash_current_to_reg(brightness);
>>> +
>>> + mutex_lock(&leds_dev->lock);
>>> + rc = qcom_flash_brightness_set(led, bright);
>>> + if (rc)
>>> + goto unlock;
>>> +
>>> + if (led->flash_current_invalidated) {
>>> + bright = qcom_flash_current_to_reg(fled_cdev->brightness.max);
>>> + rc = qcom_flash_max_brightness_set(led, bright);
>>> + if (rc)
>>> + goto unlock;
>>> + }
>>> +
>>> + led->flash_current_invalidated = false;
>>> +
>>> +unlock:
>>> + mutex_unlock(&leds_dev->lock);
>>> + return rc;
>>> +}
>>> +
>>> +static int qcom_flash_flcdev_strobe_set(struct led_classdev_flash *fled_cdev,
>>> + bool state)
>>> +{
>>> + struct qcom_flash_led *led = flcdev_to_led(fled_cdev);
>>> + struct qcom_flash_device *leds_dev = led_to_leds_dev(led);
>>> + unsigned int bright;
>>> + unsigned int i;
>>> + int rc;
>>> +
>>> + /* Can't operate on flash if torch is on */
>>> + if (leds_dev->torch_enabled)
>>> + return -EBUSY;
>>> +
>>> + mutex_lock(&leds_dev->lock);
>>> + if (!state) {
>>> + rc = qcom_flash_fled_off(led);
>>> + } else {
>>> + /*
>>> + * Turn off flash LEDs from previous strobe
>>> + */
>>> + rc = qcom_flash_check_timedout(leds_dev);
>>> + if (rc > 0) {
>>> + for (i = 0; i < leds_dev->num_leds; i++) {
>>> + rc = qcom_flash_fled_off(&leds_dev->leds[i]);
>>> + if (rc)
>>> + goto unlock;
>>> + }
>>> + } else if (rc < 0) {
>>> + goto unlock;
>>> + }
>>
>> What if flash gets timed out after this check here? Why do you need to
>> call qcom_flash_fled_off() if it has already timed out?
>
> The issue is that after the flash times out, the hardware is not ready for
> another strobe.
>
> When I strobe LED0 for example, the STATUS register, 0x10, gets set to 0x08
> indicating the LED0 is on. After the timeout, it changes to 0x04. At that point
> if I try to strobe LED0 again, it doesn't work. When I turn the LED0 off (write
> 0x00 to either the ENABLE or STROBE register), the STATUS is reset to 0x00. Now
> I'm able to strobe the LED0 again.
>
> I'm not sure if this is the normal behavior on other flash LED controllers, and
> maybe there's even some configuration of this PMIC that resets the LED status
> automatically after the strobe timeout, but I have not been able to do that. So
> that's why I reset the status manually everytime it's needed.
My point was that the flash may time out after reading STATUS register
and before writing QCOM_FLASH_ADDR_LED_STROBE_CTRL.
You can't be 100% sure that you know the exact STATUS state just
a moment before strobing.
To alleviate that I propose to avoid checking the status and always
calling qcom_flash_fled_off() before initiating a new strobe.
>>
>>> + if (led->flash_current_invalidated) {
>>> + bright = qcom_flash_current_to_reg(fled_cdev->brightness.val);
>>> + rc = qcom_flash_brightness_set(led, bright);
>>> + if (rc)
>>> + goto unlock;
>>> +
>>> + bright = qcom_flash_current_to_reg(fled_cdev->brightness.max);
>>> + rc = qcom_flash_max_brightness_set(led, bright);
>>> + if (rc)
>>> + goto unlock;
>>> +
>>> + led->flash_current_invalidated = false;
>>> + }
>>> +
>>> + rc = qcom_flash_fled_on(led);
>>> + }
>>> +
>>> +unlock:
>>> + mutex_unlock(&leds_dev->lock);
>>> + return rc;
>>> +}
>>> +
>>> +static int qcom_flash_flcdev_strobe_get(struct led_classdev_flash *fled_cdev,
>>> + bool *state)
>>> +{
>>> + struct qcom_flash_led *led = flcdev_to_led(fled_cdev);
>>> + struct qcom_flash_device *leds_dev = led_to_leds_dev(led);
>>> + int status;
>>> +
>>> + mutex_lock(&leds_dev->lock);
>>> + status = qcom_flash_status_get(led);
>>> + mutex_unlock(&leds_dev->lock);
>>> + if (status < 0)
>>> + return status;
>>> +
>>> + *state = status && !leds_dev->torch_enabled;
>>> + return 0;
>>> +}
>>> +
>>> +static int qcom_flash_flcdev_timeout_set(struct led_classdev_flash *fled_cdev,
>>> + u32 timeout)
>>> +{
>>> + struct qcom_flash_led *led = flcdev_to_led(fled_cdev);
>>> + struct qcom_flash_device *leds_dev = led_to_leds_dev(led);
>>> + unsigned int time, i;
>>> + int rc;
>>> +
>>> + clamp_align(&timeout, QCOM_FLASH_DURATION_MIN, fled_cdev->timeout.max,
>>> + QCOM_FLASH_DURATION_STEP);
>>> +
>>> + /* Since the timeout register is shared between LEDs, update for all */
>>> + for (i = 0; i < leds_dev->num_leds; i++)
>>> + leds_dev->leds[i].fled_cdev.timeout.val = timeout;
>>> +
>>> + time = qcom_flash_duration_to_reg(fled_cdev->timeout.val);
>>> +
>>> + mutex_lock(&leds_dev->lock);
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_SAFETY_TIMER,
>>> + QCOM_FLASH_SAFETY_TIMER_MASK, time);
>>> + if (rc)
>>> + dev_err(leds_dev->dev, "Safety timer reg write failed(%d)\n",
>>> + rc);
>>> + mutex_unlock(&leds_dev->lock);
>>> +
>>> + return rc;
>>> +}
>>> +
>>> +static int qcom_flash_flcdev_fault_get(struct led_classdev_flash *fled_cdev,
>>> + u32 *fault)
>>> +{
>>> + /*
>>> + * TODO Add fault detection when we find out how to. No clue from
>>> + * inspecting the SPMI registers
>>> + */
>>> + *fault = 0;
>>> + return 0;
>>> +}
>>> +
>>> +static const struct led_flash_ops flash_ops = {
>>> + .flash_brightness_set = qcom_flash_flcdev_brightness_set,
>>> + .strobe_set = qcom_flash_flcdev_strobe_set,
>>> + .strobe_get = qcom_flash_flcdev_strobe_get,
>>> + .timeout_set = qcom_flash_flcdev_timeout_set,
>>> + .fault_get = qcom_flash_flcdev_fault_get,
>>> +};
>>> +
>>> +static int qcom_flash_setup_flcdev(struct qcom_flash_config *cfg,
>>> + struct qcom_flash_led *led,
>>> + struct device_node *node)
>>> +{
>>> + int rc;
>>> + struct device *dev = led_to_leds_dev(led)->dev;
>>> + struct led_classdev_flash *fled_cdev = &led->fled_cdev;
>>> + struct led_classdev *led_cdev = &fled_cdev->led_cdev;
>>> + struct led_init_data init_data = {};
>>> + struct led_flash_setting *setting;
>>> +
>>> + init_data.fwnode = of_fwnode_handle(node);
>>> +
>>> + led_cdev->brightness_set_blocking = qcom_flash_ledcdev_brightness_set;
>>> +
>>> + led_cdev->max_brightness = cfg->led_max_brightness[led->id];
>>> + led_cdev->max_brightness /= QCOM_FLASH_CURRENT_STEP;
>>
>> Just assign the result of division:
>>
>> led_cdev->max_brightness = cfg->led_max_brightness[led->id] /
>> QCOM_FLASH_CURRENT_STEP;
>>
>> Two consecutive assignments look a bit convoluted.
>
> Sure.
>
>>
>>> +
>>> + setting = &fled_cdev->brightness;
>>> + setting->max = cfg->flash_max_brightness[led->id];
>>> + setting->min = QCOM_FLASH_CURRENT_MIN;
>>> + setting->step = QCOM_FLASH_CURRENT_STEP;
>>> + setting->val = setting->min;
>>> +
>>> + setting = &fled_cdev->timeout;
>>> + setting->max = cfg->flash_max_timeout[led->id];
>>> + setting->min = QCOM_FLASH_DURATION_MIN;
>>> + setting->step = QCOM_FLASH_DURATION_STEP;
>>> + setting->val = QCOM_FLASH_DURATION_DEFAULT;
>>> +
>>> + fled_cdev->ops = &flash_ops;
>>> + led_cdev->flags |= LED_DEV_CAP_FLASH;
>>> +
>>> + rc = devm_led_classdev_flash_register_ext(dev, fled_cdev, &init_data);
>>> + if (rc)
>>> + dev_err(dev, "unable to register led %d,rc=%d\n", led->id, rc);
>>> +
>>> + return rc;
>>> +}
>>> +
>>> +static int qcom_flash_setup_fled(struct qcom_flash_config *cfg,
>>> + struct qcom_flash_led *led,
>>> + enum qcom_flash_ids id)
>>> +{
>>> + led->id = cfg->id[id];
>>> + led->default_on = cfg->default_on[id];
>>> + led->flash_current_invalidated = true;
>>> +
>>> + if (led->id == QCOM_FLASH_ID_LED0) {
>>> + led->flash_enable_cmd = QCOM_FLASH_ENABLE_LED0;
>>> + led->flash_strobe_cmd = QCOM_FLASH_STROBE_LED0;
>>> + led->current_addr = QCOM_FLASH_ADDR_CURR_LED0;
>>> + led->mask_led_on = QCOM_FLASH_MASK_ON_LED0;
>>> + } else {
>>> + led->flash_enable_cmd = QCOM_FLASH_ENABLE_LED1;
>>> + led->flash_strobe_cmd = QCOM_FLASH_STROBE_LED1;
>>> + led->current_addr = QCOM_FLASH_ADDR_CURR_LED1;
>>> + led->mask_led_on = QCOM_FLASH_MASK_ON_LED1;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int qcom_flash_setup_regs(struct qcom_flash_device *leds_dev)
>>> +{
>>> + int rc;
>>> +
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_LED_STROBE_CTRL,
>>> + QCOM_FLASH_STROBE_MASK,
>>> + QCOM_FLASH_DISABLE_ALL);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "LED flash write failed(%d)\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + /* Disable flash LED module */
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_ENABLE_CONTROL,
>>> + QCOM_FLASH_ENABLE_MASK,
>>> + QCOM_FLASH_DISABLE_ALL);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Enable reg write failed(%d)\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + /* Set Vreg force */
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_VREG_OK_FORCE,
>>> + QCOM_FLASH_VREG_MASK,
>>> + QCOM_FLASH_HW_VREG_OK);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Vreg OK reg write failed(%d)\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + /* Set self fault check */
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_FAULT_DETECT,
>>> + QCOM_FLASH_FAULT_DETECT_MASK,
>>> + QCOM_FLASH_SELFCHECK_DISABLE);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Fault detect reg write failed(%d)\n",
>>> + rc);
>>> + return rc;
>>> + }
>>> +
>>> + /* Set mask enable */
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_MASK_ENABLE,
>>> + QCOM_FLASH_MASK_REG_MASK,
>>> + QCOM_FLASH_MASK_1);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Mask enable reg write failed(%d)\n",
>>> + rc);
>>> + return rc;
>>> + }
>>> +
>>> + /* Set ramp rate */
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_RAMP_RATE,
>>> + QCOM_FLASH_RAMP_RATE_MASK, 0xBF);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Ramp rate reg write failed(%d)\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + /* Enable VPH_PWR_DROOP and set threshold to 2.9V */
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_VPH_PWR_DROOP,
>>> + QCOM_FLASH_VPH_PWR_DROOP_MASK, 0xC2);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "VPH_PWR_DROOP reg write failed(%d)\n",
>>> + rc);
>>> + return rc;
>>> + }
>>> +
>>> + /* Set headroom */
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_HEADROOM,
>>> + QCOM_FLASH_HEADROOM_MASK,
>>> + QCOM_FLASH_HEADROOM_500MV);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Headroom reg write failed(%d)\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + /* Set startup delay */
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_STARTUP_DELAY,
>>> + QCOM_FLASH_STARTUP_DLY_MASK,
>>> + QCOM_FLASH_STARTUP_DLY_10US);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "Startup delay reg write failed(%d)\n",
>>> + rc);
>>> + return rc;
>>> + }
>>> +
>>> + /*
>>> + * TODO The downstream driver also supported watchdog mode. Not sure
>>> + * about the difference, so only support safety timer for now
>>> + */
>>> + /* Set timer control */
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_LED_TMR_CTRL,
>>> + QCOM_FLASH_TMR_MASK,
>>> + QCOM_FLASH_TMR_SAFETY);
>>> + if (rc) {
>>> + dev_err(leds_dev->dev, "LED timer ctrl reg write failed(%d)\n",
>>> + rc);
>>> + return rc;
>>> + }
>>> +
>>> + /* Set clamp current */
>>> + rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_CLAMP_CURR,
>>> + QCOM_FLASH_CURRENT_MASK,
>>> + qcom_flash_current_to_reg(QCOM_FLASH_DEFAULT_CLAMP));
>>> + if (rc)
>>> + dev_err(leds_dev->dev, "Clamp current reg write failed(%d)\n",
>>> + rc);
>>> +
>>> + return rc;
>>> +}
>>> +
>>> +static int qcom_flash_setup_led(struct qcom_flash_config *cfg,
>>> + struct qcom_flash_led *led,
>>> + enum qcom_flash_ids id,
>>> + struct device_node *node)
>>> +{
>>> + struct qcom_flash_device *leds_dev = led_to_leds_dev(led);
>>> + struct led_classdev *led_cdev = &led->fled_cdev.led_cdev;
>>> + int rc;
>>> +
>>> + rc = qcom_flash_setup_fled(cfg, led, id);
>>> + if (rc)
>>> + return rc;
>>> +
>>> + rc = qcom_flash_setup_flcdev(cfg, led, node);
>>> + if (rc)
>>> + return rc;
>>> +
>>> + /* configure default state */
>>> + if (!led->default_on) {
>>> + led_cdev->brightness = LED_OFF;
>>> + } else {
>>> + led_cdev->brightness = led_cdev->max_brightness;
>>> + mutex_lock(&leds_dev->lock);
>>> + rc = qcom_flash_torch_on(led);
>>> + mutex_unlock(&leds_dev->lock);
>>> + if (rc)
>>> + return rc;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int qcom_flash_setup_leds_device(struct qcom_flash_device *leds_dev,
>>> + struct qcom_flash_config *cfg,
>>> + struct device *dev)
>>> +{
>>> + int rc;
>>> +
>>> + leds_dev->dev = dev;
>>> +
>>> + leds_dev->base = cfg->base;
>>> + leds_dev->num_leds = cfg->num_leds;
>>> +
>>> + leds_dev->regmap = dev_get_regmap(dev->parent, NULL);
>>> + if (!leds_dev->regmap) {
>>> + dev_err(dev, "Failure getting regmap\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + rc = qcom_flash_setup_regs(leds_dev);
>>> + if (rc)
>>> + return rc;
>>> +
>>> + rc = qcom_flash_read_reg(leds_dev, QCOM_FLASH_ADDR_PERIPHERAL_SUBTYPE,
>>> + &leds_dev->peripheral_subtype);
>>> + if (rc) {
>>> + dev_err(dev, "Unable to read from addr=%x, rc(%d)\n",
>>> + QCOM_FLASH_ADDR_PERIPHERAL_SUBTYPE, rc);
>>> + return rc;
>>> + }
>>> +
>>> + leds_dev->flash_supply = cfg->flash_supply;
>>> +
>>> + if (leds_dev->peripheral_subtype == QCOM_FLASH_SUBTYPE_DUAL) {
>>> + leds_dev->torch_supply = cfg->torch_supply;
>>> + leds_dev->torch_enable_cmd = QCOM_FLASH_ENABLE_MODULE;
>>> + } else {
>>> + leds_dev->torch_enable_cmd = QCOM_FLASH_ENABLE_ALL;
>>> + }
>>> +
>>> + mutex_init(&leds_dev->lock);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int qcom_flash_parse_dt(struct device *dev,
>>> + struct qcom_flash_config *cfg,
>>> + struct device_node *sub_nodes[])
>>> +{
>>> + struct device_node *node = dev->of_node;
>>> + struct device_node *child_node;
>>> + const char *temp_string;
>>> + int rc, parsed_leds = 0;
>>> + u32 val;
>>> +
>>> + rc = of_property_read_u32(node, "reg", &val);
>>> + if (rc < 0) {
>>> + dev_err(dev, "Failure reading reg, rc = %d\n", rc);
>>> + return rc;
>>> + }
>>> + cfg->base = val;
>>> +
>>> + cfg->flash_supply = devm_regulator_get(dev, "flash-boost");
>>> + cfg->torch_supply = devm_regulator_get(dev, "torch-boost");
>>> +
>>> + for_each_available_child_of_node(node, child_node) {
>>> + /* LED properties */
>>> +
>>> + rc = of_property_read_u32(child_node, "reg",
>>> + &cfg->id[parsed_leds]);
>>> + if (rc) {
>>> + dev_err(dev, "Failure reading led id, rc = %d\n", rc);
>>> + of_node_put(child_node);
>>> + return rc;
>>> + }
>>> +
>>> + /* Assume LED IDs are ordered in DT for simplicity */
>>
>> You need to mention this requirement in DT bindings, but I am not sure
>> if DT child nodes parsing order is guaranteed to be top-down.
>
> Ok, I'll take a look at the documentation for the DT bindings if there's any
> clue on that, and check what other drivers normally do. I did that just because
> it's simpler, but I guess I could use a linked list for the nodes instead of an
> array, or maybe even simpler, have an array for the LED IDs read, so we can
> remove this restriction.
Right, just follow what other drivers do in similar cases.
There's no need to reinvent the wheel.
--
Best regards,
Jacek Anaszewski
Powered by blists - more mailing lists