Re: [PATCH v2 2/4] leds: Add driver for QCOM SPMI Flash LEDs
On 2/19/21 12:02 PM, Pavel Machek wrote: Hi! + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include Please sort includes alphabetically. No need to do that. Keeping the includes sorted eliminates the risk of introducing duplicates and allows for faster lookup. What gain is in having them unsorted? +#define FLASH_SAFETY_TIMER 0x40 Namespacing prefix is needed for macros, e.g. QCOM_FLASH*. No need for that in .c files. In general it eliminates the risk of name clash with other subsystems headers. And actually the prefix here should be QCOM_LED_FLASH to avoid ambiguity with flash memory. If you dropped the vendor prefix then you'd get possible name clash with led-class-flash.h namespace prefix. -- Best regards, Jacek Anaszewski
Re: [PATCH v2 2/4] leds: Add driver for QCOM SPMI Flash LEDs
Hi! > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > Please sort includes alphabetically. No need to do that. > > +#define FLASH_SAFETY_TIMER 0x40 > > Namespacing prefix is needed for macros, e.g. QCOM_FLASH*. No need for that in .c files. Best regards, Pavel -- http://www.livejournal.com/~pavelmachek signature.asc Description: PGP signature
Re: [PATCH v2 2/4] leds: Add driver for QCOM SPMI Flash LEDs
On Tue 2021-01-26 14:05:54, Nícolas F. R. A. Prado wrote:
> Add driver for the Qualcomm SPMI Flash LEDs. These are controlled
> through an SPMI bus and are part of the PM8941 PMIC. There are two LEDs
> present in the chip, and can be used independently as camera flash or
> together in torch mode to act as a lantern.
> drivers/leds/Kconfig|8 +
> drivers/leds/Makefile |1 +
> drivers/leds/leds-qcom-spmi-flash.c | 1153 +++
> 3 files changed, 1162 insertions(+)
Ok, please make this go to drivers/leds/flash/
> +static int qcom_flash_fled_regulator_operate(struct qcom_flash_device
> *leds_dev,
> + bool on)
> +{
> + int rc;
> +
> + if (!on)
> + goto regulator_turn_off;
> +
> + if (!leds_dev->flash_regulator_on) {
> + if (leds_dev->flash_boost_reg) {
> + rc = regulator_enable(leds_dev->flash_boost_reg);
> + if (rc) {
> + dev_err(_dev->pdev->dev,
> + "Regulator enable failed(%d)\n", rc);
> + return rc;
> + }
> + leds_dev->flash_regulator_on = true;
> + }
> + }
> +
> + return 0;
> +
> +regulator_turn_off:
> + if (leds_dev->flash_regulator_on) {
> + if (leds_dev->flash_boost_reg) {
> + rc = qcom_flash_masked_write(leds_dev,
> + FLASH_ENABLE_CONTROL,
> + FLASH_ENABLE_MASK,
> + FLASH_DISABLE_ALL);
> + if (rc)
> + dev_err(_dev->pdev->dev,
> + "Enable reg write failed(%d)\n", rc);
> +
> + rc = regulator_disable(leds_dev->flash_boost_reg);
> + if (rc) {
> + dev_err(_dev->pdev->dev,
> + "Regulator disable failed(%d)\n", rc);
> + return rc;
> + }
> + leds_dev->flash_regulator_on = false;
> + }
> + }
> +
> + return 0;
> +}
Try to find a way to write this without gotos and with less
indentation. Separate functions may be useful.
> +static int qcom_flash_fled_set(struct qcom_flash_led *led, bool on)
> +{
> + int rc, error;
> + u8 curr;
> + struct qcom_flash_device *leds_dev = led_to_leds_dev(led);
> + struct device *dev = _dev->pdev->dev;
> +
> + /* dump flash registers */
> + pr_debug("Regdump before\n");
> + qcom_flash_dump_regs(leds_dev, flash_debug_regs,
> + ARRAY_SIZE(flash_debug_regs));
I believe this kind of debugging is not needed for production.
> + /* Set led current */
> + if (on) {
> + if (led->torch_enable)
> + curr =
> qcom_flash_current_to_reg(led->cdev.led_cdev.brightness);
> + else
> + curr =
> qcom_flash_current_to_reg(led->cdev.brightness.val);
> +
> + if (led->torch_enable) {
> + if (leds_dev->peripheral_subtype == FLASH_SUBTYPE_DUAL)
> {
> + rc =
> qcom_flash_torch_regulator_operate(leds_dev, true);
> + if (rc) {
> + dev_err(dev,
> + "Torch regulator operate failed(%d)\n",
> + rc);
> + return rc;
> + }
No need to goto here?
> + } else if (leds_dev->peripheral_subtype ==
> FLASH_SUBTYPE_SINGLE) {
> + rc =
> qcom_flash_fled_regulator_operate(leds_dev, true);
> + if (rc) {
> + dev_err(dev,
> + "Flash regulator operate failed(%d)\n",
> + 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,
> + FLASH_ENABLE_CONTROL,
> + FLASH_ENABLE_MODULE_MASK,
> + FLASH_ENABLE_MODULE);
> + if (rc) {
> + dev_err(dev,
> + "Enable reg write failed(%d)\n",
> +
Re: [PATCH v2 2/4] leds: Add driver for QCOM SPMI Flash LEDs
Hi Nicolas, On 1/26/21 3:05 PM, Nícolas F. R. A. Prado wrote: Add driver for the Qualcomm SPMI Flash LEDs. These are controlled through an SPMI bus and are part of the PM8941 PMIC. There are two LEDs present in the chip, and can be used independently as camera flash or together in torch mode to act as a lantern. Signed-off-by: Nícolas F. R. A. Prado --- 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/Kconfig|8 + drivers/leds/Makefile |1 + drivers/leds/leds-qcom-spmi-flash.c | 1153 +++ 3 files changed, 1162 insertions(+) create mode 100644 drivers/leds/leds-qcom-spmi-flash.c diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 849d3c5f908e..ad1c7846f9b3 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -928,6 +928,14 @@ config LEDS_ACER_A500 This option enables support for the Power Button LED of Acer Iconia Tab A500. +config LEDS_QCOM_SPMI_FLASH + tristate "Support for QCOM SPMI Flash LEDs" + depends on SPMI + depends on LEDS_CLASS_FLASH + help + This driver supports the Flash/Torch LED present in Qualcomm's PM8941 + PMIC. + comment "LED Triggers" source "drivers/leds/trigger/Kconfig" diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 73e603e1727e..e86bcfba016b 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -93,6 +93,7 @@ obj-$(CONFIG_LEDS_TURRIS_OMNIA) += leds-turris-omnia.o obj-$(CONFIG_LEDS_WM831X_STATUS) += leds-wm831x-status.o obj-$(CONFIG_LEDS_WM8350) += leds-wm8350.o obj-$(CONFIG_LEDS_WRAP) += leds-wrap.o +obj-$(CONFIG_LEDS_QCOM_SPMI_FLASH) += leds-qcom-spmi-flash.o # LED SPI Drivers obj-$(CONFIG_LEDS_CR0014114) += leds-cr0014114.o diff --git a/drivers/leds/leds-qcom-spmi-flash.c b/drivers/leds/leds-qcom-spmi-flash.c new file mode 100644 index ..023fc107abce --- /dev/null +++ b/drivers/leds/leds-qcom-spmi-flash.c @@ -0,0 +1,1153 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Qualcomm SPMI Flash Driver + * + * Copyright (c) 2020, Nícolas F. R. A. Prado + * + * Based on QPNP LEDs driver from downstream MSM kernel sources. + * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include Please sort includes alphabetically. + +#define FLASH_SAFETY_TIMER 0x40 Namespacing prefix is needed for macros, e.g. QCOM_FLASH*. +#define FLASH_MAX_CURR 0x41 +#define FLASH_LED_0_CURR 0x42 +#define FLASH_LED_1_CURR 0x43 +#define FLASH_CLAMP_CURR 0x44 +#define FLASH_LED_TMR_CTRL 0x48 +#define FLASH_HEADROOM 0x4A +#define FLASH_STARTUP_DELAY0x4B +#define FLASH_MASK_ENABLE 0x4C +#define FLASH_VREG_OK_FORCE0x4F +#define FLASH_ENABLE_CONTROL 0x46 +#define FLASH_LED_STROBE_CTRL 0x47 +#define FLASH_LED_UNLOCK_SECURE0xD0 +#define FLASH_LED_TORCH0xE4 +#define FLASH_FAULT_DETECT 0x51 +#define FLASH_RAMP_RATE0x54 +#define FLASH_PERIPHERAL_SUBTYPE 0x05 +#define FLASH_VPH_PWR_DROOP0x5A + +#define FLASH_MAX_LEVEL0x4F +#define TORCH_MAX_LEVEL0x0F +#defineFLASH_NO_MASK 0x00 + +#define FLASH_MASK_1 0x20 +#define FLASH_MASK_REG_MASK0xE0 +#define FLASH_HEADROOM_MASK0x03 +#define FLASH_SAFETY_TIMER_MASK0x7F +#define FLASH_CURRENT_MASK 0xFF +#define FLASH_MAX_CURRENT_MASK 0x7F +#define FLASH_TMR_MASK 0x03 +#define FLASH_TMR_WATCHDOG 0x03 +#define FLASH_TMR_SAFETY 0x00 +#define FLASH_FAULT_DETECT_MASK0X80 +#define FLASH_HW_VREG_OK 0x40 +#define FLASH_VREG_MASK0xC0 +#define FLASH_STARTUP_DLY_MASK 0x02 +#define FLASH_RAMP_RATE_MASK 0xBF +#define FLASH_VPH_PWR_DROOP_MASK 0xF3 + +#define FLASH_ENABLE_ALL 0xE0 +#define FLASH_ENABLE_MODULE0x80 +#define FLASH_ENABLE_MODULE_MASK 0x80 +#define FLASH_DISABLE_ALL 0x00 +#define FLASH_ENABLE_MASK 0xE0 +#define FLASH_ENABLE_LED_0 0xC0 +#define FLASH_ENABLE_LED_1 0xA0 +#define FLASH_INIT_MASK0xE0 +#define FLASH_SELFCHECK_ENABLE 0x80 +#define FLASH_SELFCHECK_DISABLE0x00 + +#define FLASH_STROBE_SW0xC0 +#define FLASH_STROBE_HW
Re: [PATCH v2 2/4] leds: Add driver for QCOM SPMI Flash LEDs
On Tue 26 Jan 08:05 CST 2021, N?colas F. R. A. Prado wrote: > Add driver for the Qualcomm SPMI Flash LEDs. These are controlled > through an SPMI bus and are part of the PM8941 PMIC. There are two LEDs > present in the chip, and can be used independently as camera flash or > together in torch mode to act as a lantern. > > Signed-off-by: Nícolas F. R. A. Prado > --- > 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/Kconfig|8 + > drivers/leds/Makefile |1 + > drivers/leds/leds-qcom-spmi-flash.c | 1153 +++ > 3 files changed, 1162 insertions(+) > create mode 100644 drivers/leds/leds-qcom-spmi-flash.c > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index 849d3c5f908e..ad1c7846f9b3 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -928,6 +928,14 @@ config LEDS_ACER_A500 > This option enables support for the Power Button LED of > Acer Iconia Tab A500. > > +config LEDS_QCOM_SPMI_FLASH > + tristate "Support for QCOM SPMI Flash LEDs" > + depends on SPMI > + depends on LEDS_CLASS_FLASH depends on OF? > + help > + This driver supports the Flash/Torch LED present in Qualcomm's PM8941 > + PMIC. > + > comment "LED Triggers" > source "drivers/leds/trigger/Kconfig" > > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index 73e603e1727e..e86bcfba016b 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -93,6 +93,7 @@ obj-$(CONFIG_LEDS_TURRIS_OMNIA) += > leds-turris-omnia.o > obj-$(CONFIG_LEDS_WM831X_STATUS) += leds-wm831x-status.o > obj-$(CONFIG_LEDS_WM8350)+= leds-wm8350.o > obj-$(CONFIG_LEDS_WRAP) += leds-wrap.o > +obj-$(CONFIG_LEDS_QCOM_SPMI_FLASH) += leds-qcom-spmi-flash.o > > # LED SPI Drivers > obj-$(CONFIG_LEDS_CR0014114) += leds-cr0014114.o > diff --git a/drivers/leds/leds-qcom-spmi-flash.c > b/drivers/leds/leds-qcom-spmi-flash.c > new file mode 100644 > index ..023fc107abce > --- /dev/null > +++ b/drivers/leds/leds-qcom-spmi-flash.c > @@ -0,0 +1,1153 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Qualcomm SPMI Flash Driver > + * > + * Copyright (c) 2020, Nícolas F. R. A. Prado > + * > + * Based on QPNP LEDs driver from downstream MSM kernel sources. > + * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define FLASH_SAFETY_TIMER 0x40 > +#define FLASH_MAX_CURR 0x41 > +#define FLASH_LED_0_CURR 0x42 > +#define FLASH_LED_1_CURR 0x43 > +#define FLASH_CLAMP_CURR 0x44 > +#define FLASH_LED_TMR_CTRL 0x48 > +#define FLASH_HEADROOM 0x4A > +#define FLASH_STARTUP_DELAY 0x4B > +#define FLASH_MASK_ENABLE0x4C > +#define FLASH_VREG_OK_FORCE 0x4F > +#define FLASH_ENABLE_CONTROL 0x46 > +#define FLASH_LED_STROBE_CTRL0x47 > +#define FLASH_LED_UNLOCK_SECURE 0xD0 > +#define FLASH_LED_TORCH 0xE4 > +#define FLASH_FAULT_DETECT 0x51 > +#define FLASH_RAMP_RATE 0x54 > +#define FLASH_PERIPHERAL_SUBTYPE 0x05 > +#define FLASH_VPH_PWR_DROOP 0x5A > + > +#define FLASH_MAX_LEVEL 0x4F > +#define TORCH_MAX_LEVEL 0x0F > +#define FLASH_NO_MASK 0x00 > + > +#define FLASH_MASK_1 0x20 > +#define FLASH_MASK_REG_MASK 0xE0 > +#define FLASH_HEADROOM_MASK 0x03 > +#define FLASH_SAFETY_TIMER_MASK 0x7F > +#define FLASH_CURRENT_MASK 0xFF > +#define FLASH_MAX_CURRENT_MASK 0x7F > +#define FLASH_TMR_MASK 0x03 > +#define FLASH_TMR_WATCHDOG 0x03 > +#define FLASH_TMR_SAFETY 0x00 > +#define FLASH_FAULT_DETECT_MASK 0X80 > +#define FLASH_HW_VREG_OK 0x40 > +#define FLASH_VREG_MASK 0xC0 > +#define FLASH_STARTUP_DLY_MASK 0x02 > +#define FLASH_RAMP_RATE_MASK 0xBF > +#define FLASH_VPH_PWR_DROOP_MASK 0xF3 > + > +#define FLASH_ENABLE_ALL 0xE0 > +#define FLASH_ENABLE_MODULE 0x80 > +#define FLASH_ENABLE_MODULE_MASK 0x80 > +#define FLASH_DISABLE_ALL0x00 > +#define FLASH_ENABLE_MASK0xE0 > +#define FLASH_ENABLE_LED_0 0xC0 > +#define FLASH_ENABLE_LED_1 0xA0 > +#define FLASH_INIT_MASK 0xE0 > +#define FLASH_SELFCHECK_ENABLE 0x80 > +#define FLASH_SELFCHECK_DISABLE
[PATCH v2 2/4] leds: Add driver for QCOM SPMI Flash LEDs
Add driver for the Qualcomm SPMI Flash LEDs. These are controlled through an SPMI bus and are part of the PM8941 PMIC. There are two LEDs present in the chip, and can be used independently as camera flash or together in torch mode to act as a lantern. Signed-off-by: Nícolas F. R. A. Prado --- 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/Kconfig|8 + drivers/leds/Makefile |1 + drivers/leds/leds-qcom-spmi-flash.c | 1153 +++ 3 files changed, 1162 insertions(+) create mode 100644 drivers/leds/leds-qcom-spmi-flash.c diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 849d3c5f908e..ad1c7846f9b3 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -928,6 +928,14 @@ config LEDS_ACER_A500 This option enables support for the Power Button LED of Acer Iconia Tab A500. +config LEDS_QCOM_SPMI_FLASH + tristate "Support for QCOM SPMI Flash LEDs" + depends on SPMI + depends on LEDS_CLASS_FLASH + help + This driver supports the Flash/Torch LED present in Qualcomm's PM8941 + PMIC. + comment "LED Triggers" source "drivers/leds/trigger/Kconfig" diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 73e603e1727e..e86bcfba016b 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -93,6 +93,7 @@ obj-$(CONFIG_LEDS_TURRIS_OMNIA) += leds-turris-omnia.o obj-$(CONFIG_LEDS_WM831X_STATUS) += leds-wm831x-status.o obj-$(CONFIG_LEDS_WM8350) += leds-wm8350.o obj-$(CONFIG_LEDS_WRAP)+= leds-wrap.o +obj-$(CONFIG_LEDS_QCOM_SPMI_FLASH) += leds-qcom-spmi-flash.o # LED SPI Drivers obj-$(CONFIG_LEDS_CR0014114) += leds-cr0014114.o diff --git a/drivers/leds/leds-qcom-spmi-flash.c b/drivers/leds/leds-qcom-spmi-flash.c new file mode 100644 index ..023fc107abce --- /dev/null +++ b/drivers/leds/leds-qcom-spmi-flash.c @@ -0,0 +1,1153 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Qualcomm SPMI Flash Driver + * + * Copyright (c) 2020, Nícolas F. R. A. Prado + * + * Based on QPNP LEDs driver from downstream MSM kernel sources. + * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define FLASH_SAFETY_TIMER 0x40 +#define FLASH_MAX_CURR 0x41 +#define FLASH_LED_0_CURR 0x42 +#define FLASH_LED_1_CURR 0x43 +#define FLASH_CLAMP_CURR 0x44 +#define FLASH_LED_TMR_CTRL 0x48 +#define FLASH_HEADROOM 0x4A +#define FLASH_STARTUP_DELAY0x4B +#define FLASH_MASK_ENABLE 0x4C +#define FLASH_VREG_OK_FORCE0x4F +#define FLASH_ENABLE_CONTROL 0x46 +#define FLASH_LED_STROBE_CTRL 0x47 +#define FLASH_LED_UNLOCK_SECURE0xD0 +#define FLASH_LED_TORCH0xE4 +#define FLASH_FAULT_DETECT 0x51 +#define FLASH_RAMP_RATE0x54 +#define FLASH_PERIPHERAL_SUBTYPE 0x05 +#define FLASH_VPH_PWR_DROOP0x5A + +#define FLASH_MAX_LEVEL0x4F +#define TORCH_MAX_LEVEL0x0F +#defineFLASH_NO_MASK 0x00 + +#define FLASH_MASK_1 0x20 +#define FLASH_MASK_REG_MASK0xE0 +#define FLASH_HEADROOM_MASK0x03 +#define FLASH_SAFETY_TIMER_MASK0x7F +#define FLASH_CURRENT_MASK 0xFF +#define FLASH_MAX_CURRENT_MASK 0x7F +#define FLASH_TMR_MASK 0x03 +#define FLASH_TMR_WATCHDOG 0x03 +#define FLASH_TMR_SAFETY 0x00 +#define FLASH_FAULT_DETECT_MASK0X80 +#define FLASH_HW_VREG_OK 0x40 +#define FLASH_VREG_MASK0xC0 +#define FLASH_STARTUP_DLY_MASK 0x02 +#define FLASH_RAMP_RATE_MASK 0xBF +#define FLASH_VPH_PWR_DROOP_MASK 0xF3 + +#define FLASH_ENABLE_ALL 0xE0 +#define FLASH_ENABLE_MODULE0x80 +#define FLASH_ENABLE_MODULE_MASK 0x80 +#define FLASH_DISABLE_ALL 0x00 +#define FLASH_ENABLE_MASK 0xE0 +#define FLASH_ENABLE_LED_0 0xC0 +#define FLASH_ENABLE_LED_1 0xA0 +#define FLASH_INIT_MASK0xE0 +#define FLASH_SELFCHECK_ENABLE 0x80 +#define FLASH_SELFCHECK_DISABLE0x00 + +#define FLASH_STROBE_SW0xC0 +#define FLASH_STROBE_HW0x04 +#define FLASH_STROBE_MASK 0xC7 +#define FLASH_LED_0_OUTPUT 0x80 +#define FLASH_LED_1_OUTPUT 0x40 + +#define
