Re: [PATCH v9 09/10] leds: flash: mt6370: Add MediaTek MT6370 flashlight support
On Fri, 2022-09-23 at 14:34 -0700, Han Jingoo wrote: > On Wed, Sep 21, 2022 Andy Shevchenko wrote: > > > > On Wed, Sep 21, 2022 at 4:48 AM ChiaEn Wu wrote: > > > On Sun, Sep 18, 2022 at 3:22 AM Han Jingoo wrote: > > > > On Mon, Aug 29, 2022 ChiaEn Wu wrote: > > > > > > > +#define MT6370_ITORCH_MIN_uA 25000 > > > > > +#define MT6370_ITORCH_STEP_uA 12500 > > > > > +#define MT6370_ITORCH_MAX_uA 40 > > > > > +#define MT6370_ITORCH_DOUBLE_MAX_uA80 > > > > > +#define MT6370_ISTRB_MIN_uA5 > > > > > +#define MT6370_ISTRB_STEP_uA 12500 > > > > > +#define MT6370_ISTRB_MAX_uA150 > > > > > +#define MT6370_ISTRB_DOUBLE_MAX_uA 300 > > > > > > > > Use upper letters as below: > > > > For microseconds (and other -seconds) the common practice (I assume > > historically) is to use upper letters, indeed. But for current it's > > more natural to use small letters for unit multiplier as it's easier > > to read and understand. I think it's fine. see: commit 22735ce857a2d9f4e6eec37c36be3fcf9d21d154 Author: Joe Perches Date: Wed Jul 3 15:05:33 2013 -0700 checkpatch: ignore SI unit CamelCase variants like "_uV" Many existing variable names use SI like variants that should be otherwise obvious and acceptable.
Re: [PATCH v9 09/10] leds: flash: mt6370: Add MediaTek MT6370 flashlight support
On Wed, Sep 21, 2022 Andy Shevchenko wrote: > > On Wed, Sep 21, 2022 at 4:48 AM ChiaEn Wu wrote: > > On Sun, Sep 18, 2022 at 3:22 AM Han Jingoo wrote: > > > On Mon, Aug 29, 2022 ChiaEn Wu wrote: > > > > > +#define MT6370_ITORCH_MIN_uA 25000 > > > > +#define MT6370_ITORCH_STEP_uA 12500 > > > > +#define MT6370_ITORCH_MAX_uA 40 > > > > +#define MT6370_ITORCH_DOUBLE_MAX_uA80 > > > > +#define MT6370_ISTRB_MIN_uA5 > > > > +#define MT6370_ISTRB_STEP_uA 12500 > > > > +#define MT6370_ISTRB_MAX_uA150 > > > > +#define MT6370_ISTRB_DOUBLE_MAX_uA 300 > > > > > > Use upper letters as below: > > For microseconds (and other -seconds) the common practice (I assume > historically) is to use upper letters, indeed. But for current it's > more natural to use small letters for unit multiplier as it's easier > to read and understand. (CC'ed Linus Torvalds, Andrew Morton, Joe Perches, Julia Lawall, Krzysztof Kozlowski,) Yep, it is common practice. Long time ago, I met the same problem on how to present micro-ampere: visibility vs coding practice. At that time, I followed the coding practice. So, was there anyone who rejected this decision to mix upper and lower letters when you gave your comment last July? If there is no objection, or most of maintainers and long-term contributors agree with that, I am ok with that. To Tovalds, Andrew, Joe, Julia, Krzysztof, I just need your feedback on coding styles. Are you ok with mixing upper and lower letters for visibility to present micro-seconds or micro-ampere? Andy (one of very-active contributors) gives his opinion that mixing upper and lower letters can be acceptable. I remain neutral on this coding style issue. e.g., #define MT6370_ITORCH_DOUBLE_MAX_uA80 Thank you. Best regards, Jingoo Han > > > > #define MT6370_ITORCH_MIN_UA 25000 > > > #define MT6370_ITORCH_STEP_UA 12500 > > > #define MT6370_ITORCH_MAX_UA 40 > > > #define MT6370_ITORCH_DOUBLE_MAX_UA80 > > > #define MT6370_ISTRB_MIN_UA5 > > > #define MT6370_ISTRB_STEP_UA 12500 > > > #define MT6370_ISTRB_MAX_UA150 > > > #define MT6370_ISTRB_DOUBLE_MAX_UA 300 > > > > > > > +#define MT6370_STRBTO_MIN_US 64000 > > > > +#define MT6370_STRBTO_STEP_US 32000 > > > > +#define MT6370_STRBTO_MAX_US 2432000 > > > > Hi Jingoo, > > > > This coding style is in accordance with Andy's opinion in this mail: > > https://lore.kernel.org/linux-arm-kernel/CAHp75Vciq4M4kVrabNV9vTLLcd1vR=bme8jledaf9mkrtpc...@mail.gmail.com/ > > True. > > -- > With Best Regards, > Andy Shevchenko
Re: [PATCH v9 09/10] leds: flash: mt6370: Add MediaTek MT6370 flashlight support
On Wed, Sep 21, 2022 at 4:48 AM ChiaEn Wu wrote: > On Sun, Sep 18, 2022 at 3:22 AM Han Jingoo wrote: > > On Mon, Aug 29, 2022 ChiaEn Wu wrote: > > > +#define MT6370_ITORCH_MIN_uA 25000 > > > +#define MT6370_ITORCH_STEP_uA 12500 > > > +#define MT6370_ITORCH_MAX_uA 40 > > > +#define MT6370_ITORCH_DOUBLE_MAX_uA80 > > > +#define MT6370_ISTRB_MIN_uA5 > > > +#define MT6370_ISTRB_STEP_uA 12500 > > > +#define MT6370_ISTRB_MAX_uA150 > > > +#define MT6370_ISTRB_DOUBLE_MAX_uA 300 > > > > Use upper letters as below: For microseconds (and other -seconds) the common practice (I assume historically) is to use upper letters, indeed. But for current it's more natural to use small letters for unit multiplier as it's easier to read and understand. > > #define MT6370_ITORCH_MIN_UA 25000 > > #define MT6370_ITORCH_STEP_UA 12500 > > #define MT6370_ITORCH_MAX_UA 40 > > #define MT6370_ITORCH_DOUBLE_MAX_UA80 > > #define MT6370_ISTRB_MIN_UA5 > > #define MT6370_ISTRB_STEP_UA 12500 > > #define MT6370_ISTRB_MAX_UA150 > > #define MT6370_ISTRB_DOUBLE_MAX_UA 300 > > > > > +#define MT6370_STRBTO_MIN_US 64000 > > > +#define MT6370_STRBTO_STEP_US 32000 > > > +#define MT6370_STRBTO_MAX_US 2432000 > > Hi Jingoo, > > This coding style is in accordance with Andy's opinion in this mail: > https://lore.kernel.org/linux-arm-kernel/CAHp75Vciq4M4kVrabNV9vTLLcd1vR=bme8jledaf9mkrtpc...@mail.gmail.com/ True. -- With Best Regards, Andy Shevchenko
Re: [PATCH v9 09/10] leds: flash: mt6370: Add MediaTek MT6370 flashlight support
On Sun, Sep 18, 2022 at 3:22 AM Han Jingoo wrote: > > On Mon, Aug 29, 2022 ChiaEn Wu wrote: ... > > +#define MT6370_ITORCH_MIN_uA 25000 > > +#define MT6370_ITORCH_STEP_uA 12500 > > +#define MT6370_ITORCH_MAX_uA 40 > > +#define MT6370_ITORCH_DOUBLE_MAX_uA80 > > +#define MT6370_ISTRB_MIN_uA5 > > +#define MT6370_ISTRB_STEP_uA 12500 > > +#define MT6370_ISTRB_MAX_uA150 > > +#define MT6370_ISTRB_DOUBLE_MAX_uA 300 > > Use upper letters as below: > > #define MT6370_ITORCH_MIN_UA 25000 > #define MT6370_ITORCH_STEP_UA 12500 > #define MT6370_ITORCH_MAX_UA 40 > #define MT6370_ITORCH_DOUBLE_MAX_UA80 > #define MT6370_ISTRB_MIN_UA5 > #define MT6370_ISTRB_STEP_UA 12500 > #define MT6370_ISTRB_MAX_UA150 > #define MT6370_ISTRB_DOUBLE_MAX_UA 300 > > > > +#define MT6370_STRBTO_MIN_US 64000 > > +#define MT6370_STRBTO_STEP_US 32000 > > +#define MT6370_STRBTO_MAX_US 2432000 > > + Hi Jingoo, This coding style is in accordance with Andy's opinion in this mail: https://lore.kernel.org/linux-arm-kernel/CAHp75Vciq4M4kVrabNV9vTLLcd1vR=bme8jledaf9mkrtpc...@mail.gmail.com/ And I will revise other parts in v12. Thanks for your review! -- Best Regards, ChiaEn Wu
Re: [PATCH v9 09/10] leds: flash: mt6370: Add MediaTek MT6370 flashlight support
Il 30/08/22 05:40, ChiaEn Wu ha scritto:
From: Alice Chen
The MediaTek MT6370 is a highly-integrated smart power management IC,
which includes a single cell Li-Ion/Li-Polymer switching battery
charger, a USB Type-C & Power Delivery (PD) controller, dual Flash
LED current sources, a RGB LED driver, a backlight WLED driver,
a display bias driver and a general LDO for portable devices.
Add support for the MT6370 Flash LED driver. Flash LED in MT6370
has 2 channels and support torch/strobe mode.
Signed-off-by: Alice Chen
Signed-off-by: ChiaEn Wu
---
v9
- Revise the format of the comments.
---
drivers/leds/flash/Kconfig | 14 +
drivers/leds/flash/Makefile| 1 +
drivers/leds/flash/leds-mt6370-flash.c | 632 +
3 files changed, 647 insertions(+)
create mode 100644 drivers/leds/flash/leds-mt6370-flash.c
diff --git a/drivers/leds/flash/Kconfig b/drivers/leds/flash/Kconfig
index d3eb689..405bd16 100644
--- a/drivers/leds/flash/Kconfig
+++ b/drivers/leds/flash/Kconfig
@@ -61,6 +61,20 @@ config LEDS_MT6360
Independent current sources supply for each flash LED support torch
and strobe mode.
+config LEDS_MT6370_FLASH
+ tristate "Flash LED Support for MediaTek MT6370 PMIC"
+ depends on LEDS_CLASS && OF
+ depends on LEDS_CLASS_FLASH || !LEDS_CLASS_FLASH
+ depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
+ depends on MFD_MT6370
+ help
+ Support 2 channels and torch/strobe mode.
+ Say Y here to enable support for
+ MT6370_FLASH_LED device.
+
+ This driver can also be built as a module. If so, the module
+ will be called "leds-mt6370-flash".
+
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 0acbddc..0c1f3c5 100644
--- a/drivers/leds/flash/Makefile
+++ b/drivers/leds/flash/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_LEDS_MAX77693) += leds-max77693.o
obj-$(CONFIG_LEDS_RT4505) += leds-rt4505.o
obj-$(CONFIG_LEDS_RT8515) += leds-rt8515.o
obj-$(CONFIG_LEDS_SGM3140)+= leds-sgm3140.o
+obj-$(CONFIG_LEDS_MT6370_FLASH)+= leds-mt6370-flash.o
diff --git a/drivers/leds/flash/leds-mt6370-flash.c
b/drivers/leds/flash/leds-mt6370-flash.c
new file mode 100644
index 000..a8ec7e1
--- /dev/null
+++ b/drivers/leds/flash/leds-mt6370-flash.c
@@ -0,0 +1,632 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Richtek Technology Corp.
+ *
+ * Author: Alice Chen
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+enum {
+ MT6370_LED_FLASH1,
+ MT6370_LED_FLASH2,
+ MT6370_MAX_LEDS
+};
+
+/* Virtual definition for multicolor */
+
+#define MT6370_REG_FLEDEN 0x17E
+#define MT6370_REG_STRBTO 0x173
+#define MT6370_REG_CHGSTAT20x1D1
+#define MT6370_REG_FLEDSTAT1 0x1D9
+#define MT6370_REG_FLEDISTRB(_id) (0x174 + 4 * (_id))
+#define MT6370_REG_FLEDITOR(_id) (0x175 + 4 * (_id))
+#define MT6370_ITORCH_MASK GENMASK(4, 0)
+#define MT6370_ISTROBE_MASKGENMASK(6, 0)
+#define MT6370_STRBTO_MASK GENMASK(6, 0)
+#define MT6370_TORCHEN_MASKBIT(3)
+#define MT6370_STROBEN_MASKBIT(2)
+#define MT6370_FLCSEN_MASK(_id)BIT(MT6370_LED_FLASH2 - (_id))
+#define MT6370_FLCSEN_MASK_ALL GENMASK(1, 0)
+#define MT6370_FLEDCHGVINOVP_MASK BIT(3)
+#define MT6370_FLED1STRBTO_MASKBIT(11)
+#define MT6370_FLED2STRBTO_MASKBIT(10)
+#define MT6370_FLED1STRB_MASK BIT(9)
+#define MT6370_FLED2STRB_MASK BIT(8)
+#define MT6370_FLED1SHORT_MASK BIT(7)
+#define MT6370_FLED2SHORT_MASK BIT(6)
+#define MT6370_FLEDLVF_MASKBIT(3)
+
+#define MT6370_LED_JOINT 2
+#define MT6370_RANGE_FLED_REG 4
+#define MT6370_ITORCH_MIN_uA 25000
+#define MT6370_ITORCH_STEP_uA 12500
+#define MT6370_ITORCH_MAX_uA 40
+#define MT6370_ITORCH_DOUBLE_MAX_uA80
+#define MT6370_ISTRB_MIN_uA5
+#define MT6370_ISTRB_STEP_uA 12500
+#define MT6370_ISTRB_MAX_uA150
+#define MT6370_ISTRB_DOUBLE_MAX_uA 300
+#define MT6370_STRBTO_MIN_US 64000
+#define MT6370_STRBTO_STEP_US 32000
+#define MT6370_STRBTO_MAX_US 2432000
+
+#define STATE_OFF 0
+#define STATE_KEEP 1
+#define STATE_ON 2
+
+#define to_mt6370_led(ptr, member) container_of(ptr, struct mt6370_led, member)
+
+struct mt6370_led {
+ struct led_classdev_flash flash;
+ struct v4l2_flash *v4l2_flash;
+ struct mt6370_priv *priv;
+ u32 led_no;
The maximum value that we will
[PATCH v9 09/10] leds: flash: mt6370: Add MediaTek MT6370 flashlight support
From: Alice Chen
The MediaTek MT6370 is a highly-integrated smart power management IC,
which includes a single cell Li-Ion/Li-Polymer switching battery
charger, a USB Type-C & Power Delivery (PD) controller, dual Flash
LED current sources, a RGB LED driver, a backlight WLED driver,
a display bias driver and a general LDO for portable devices.
Add support for the MT6370 Flash LED driver. Flash LED in MT6370
has 2 channels and support torch/strobe mode.
Signed-off-by: Alice Chen
Signed-off-by: ChiaEn Wu
---
v9
- Revise the format of the comments.
---
drivers/leds/flash/Kconfig | 14 +
drivers/leds/flash/Makefile| 1 +
drivers/leds/flash/leds-mt6370-flash.c | 632 +
3 files changed, 647 insertions(+)
create mode 100644 drivers/leds/flash/leds-mt6370-flash.c
diff --git a/drivers/leds/flash/Kconfig b/drivers/leds/flash/Kconfig
index d3eb689..405bd16 100644
--- a/drivers/leds/flash/Kconfig
+++ b/drivers/leds/flash/Kconfig
@@ -61,6 +61,20 @@ config LEDS_MT6360
Independent current sources supply for each flash LED support torch
and strobe mode.
+config LEDS_MT6370_FLASH
+ tristate "Flash LED Support for MediaTek MT6370 PMIC"
+ depends on LEDS_CLASS && OF
+ depends on LEDS_CLASS_FLASH || !LEDS_CLASS_FLASH
+ depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
+ depends on MFD_MT6370
+ help
+ Support 2 channels and torch/strobe mode.
+ Say Y here to enable support for
+ MT6370_FLASH_LED device.
+
+ This driver can also be built as a module. If so, the module
+ will be called "leds-mt6370-flash".
+
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 0acbddc..0c1f3c5 100644
--- a/drivers/leds/flash/Makefile
+++ b/drivers/leds/flash/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_LEDS_MAX77693) += leds-max77693.o
obj-$(CONFIG_LEDS_RT4505) += leds-rt4505.o
obj-$(CONFIG_LEDS_RT8515) += leds-rt8515.o
obj-$(CONFIG_LEDS_SGM3140) += leds-sgm3140.o
+obj-$(CONFIG_LEDS_MT6370_FLASH)+= leds-mt6370-flash.o
diff --git a/drivers/leds/flash/leds-mt6370-flash.c
b/drivers/leds/flash/leds-mt6370-flash.c
new file mode 100644
index 000..a8ec7e1
--- /dev/null
+++ b/drivers/leds/flash/leds-mt6370-flash.c
@@ -0,0 +1,632 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Richtek Technology Corp.
+ *
+ * Author: Alice Chen
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+enum {
+ MT6370_LED_FLASH1,
+ MT6370_LED_FLASH2,
+ MT6370_MAX_LEDS
+};
+
+/* Virtual definition for multicolor */
+
+#define MT6370_REG_FLEDEN 0x17E
+#define MT6370_REG_STRBTO 0x173
+#define MT6370_REG_CHGSTAT20x1D1
+#define MT6370_REG_FLEDSTAT1 0x1D9
+#define MT6370_REG_FLEDISTRB(_id) (0x174 + 4 * (_id))
+#define MT6370_REG_FLEDITOR(_id) (0x175 + 4 * (_id))
+#define MT6370_ITORCH_MASK GENMASK(4, 0)
+#define MT6370_ISTROBE_MASKGENMASK(6, 0)
+#define MT6370_STRBTO_MASK GENMASK(6, 0)
+#define MT6370_TORCHEN_MASKBIT(3)
+#define MT6370_STROBEN_MASKBIT(2)
+#define MT6370_FLCSEN_MASK(_id)BIT(MT6370_LED_FLASH2 - (_id))
+#define MT6370_FLCSEN_MASK_ALL GENMASK(1, 0)
+#define MT6370_FLEDCHGVINOVP_MASK BIT(3)
+#define MT6370_FLED1STRBTO_MASKBIT(11)
+#define MT6370_FLED2STRBTO_MASKBIT(10)
+#define MT6370_FLED1STRB_MASK BIT(9)
+#define MT6370_FLED2STRB_MASK BIT(8)
+#define MT6370_FLED1SHORT_MASK BIT(7)
+#define MT6370_FLED2SHORT_MASK BIT(6)
+#define MT6370_FLEDLVF_MASKBIT(3)
+
+#define MT6370_LED_JOINT 2
+#define MT6370_RANGE_FLED_REG 4
+#define MT6370_ITORCH_MIN_uA 25000
+#define MT6370_ITORCH_STEP_uA 12500
+#define MT6370_ITORCH_MAX_uA 40
+#define MT6370_ITORCH_DOUBLE_MAX_uA80
+#define MT6370_ISTRB_MIN_uA5
+#define MT6370_ISTRB_STEP_uA 12500
+#define MT6370_ISTRB_MAX_uA150
+#define MT6370_ISTRB_DOUBLE_MAX_uA 300
+#define MT6370_STRBTO_MIN_US 64000
+#define MT6370_STRBTO_STEP_US 32000
+#define MT6370_STRBTO_MAX_US 2432000
+
+#define STATE_OFF 0
+#define STATE_KEEP 1
+#define STATE_ON 2
+
+#define to_mt6370_led(ptr, member) container_of(ptr, struct mt6370_led, member)
+
+struct mt6370_led {
+ struct led_classdev_flash flash;
+ struct v4l2_flash *v4l2_flash;
+ struct mt6370_priv *priv;
+ u32 led_no;
+ u32 default_state;
+};
+
+struct mt6370_priv {
+ struct device *dev;
+
