| Message ID | 1599474459-20853-2-git-send-email-gene.chen.richtek@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | leds: mt6360: Add LED driver for MT6360 | expand |
Gene On 9/7/20 5:27 AM, Gene Chen wrote: > From: Gene Chen <gene_chen@richtek.com> > > Add MT6360 LED driver include 2-channel Flash LED with torch/strobe mode, > and 4-channel RGB LED support Register/Flash/Breath Mode > > Signed-off-by: Gene Chen <gene_chen@richtek.com> > --- > drivers/leds/Kconfig | 11 + > drivers/leds/Makefile | 1 + > drivers/leds/leds-mt6360.c | 681 +++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 693 insertions(+) > create mode 100644 drivers/leds/leds-mt6360.c > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index 1c181df..94a6d83 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -271,6 +271,17 @@ config LEDS_MT6323 > This option enables support for on-chip LED drivers found on > Mediatek MT6323 PMIC. > > +config LEDS_MT6360 > + tristate "LED Support for Mediatek MT6360 PMIC" > + depends on LEDS_CLASS_FLASH && OF > + depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS > + depends on MFD_MT6360 > + help > + This option enables support for dual Flash LED drivers found on > + Mediatek MT6360 PMIC. > + Independent current sources supply for each flash LED support torch and strobe mode. > + Includes Low-VF and short protection. > + > config LEDS_S3C24XX > tristate "LED Support for Samsung S3C24XX GPIO LEDs" > depends on LEDS_CLASS > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index c2c7d7a..5596427 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -66,6 +66,7 @@ obj-$(CONFIG_LEDS_MIKROTIK_RB532) += leds-rb532.o > obj-$(CONFIG_LEDS_MLXCPLD) += leds-mlxcpld.o > obj-$(CONFIG_LEDS_MLXREG) += leds-mlxreg.o > obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o > +obj-$(CONFIG_LEDS_MT6360) += leds-mt6360.o > obj-$(CONFIG_LEDS_NET48XX) += leds-net48xx.o > obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o > obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o > diff --git a/drivers/leds/leds-mt6360.c b/drivers/leds/leds-mt6360.c > new file mode 100644 > index 0000000..bd6fa48 > --- /dev/null > +++ b/drivers/leds/leds-mt6360.c > @@ -0,0 +1,681 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// > +// Copyright (C) 2020 MediaTek Inc. > +// > +// Author: Gene Chen <gene_chen@richtek.com> > + > +#include <linux/delay.h> > +#include <linux/init.h> > +#include <linux/interrupt.h> > +#include <linux/kernel.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> > + > +enum { > + MT6360_LED_ISNK1 = 0, > + MT6360_LED_ISNK2, > + MT6360_LED_ISNK3, > + MT6360_LED_ISNK4, > + MT6360_LED_FLASH1, > + MT6360_LED_FLASH2, > + MT6360_MAX_LEDS, > +}; > + > +#define MT6360_REG_RGBEN 0x380 > +#define MT6360_REG_ISNK(_led_no) (0x381 + (_led_no)) > +#define MT6360_ISNK_ENMASK(_led_no) BIT(7 - (_led_no)) > +#define MT6360_ISNK_MASK 0x1F > +#define MT6360_CHRINDSEL_MASK BIT(3) > + > +#define MT6360_REG_FLEDEN 0x37E > +#define MT6360_REG_STRBTO 0x373 > +#define MT6360_REG_FLEDBASE(_id) (0x372 + 4 * (_id - MT6360_LED_FLASH1)) > +#define MT6360_REG_FLEDISTRB(_id) (MT6360_REG_FLEDBASE(_id) + 2) > +#define MT6360_REG_FLEDITOR(_id) (MT6360_REG_FLEDBASE(_id) + 3) > +#define MT6360_REG_CHGSTAT2 0x3E1 > +#define MT6360_REG_FLEDSTAT1 0x3E9 > +#define MT6360_ITORCH_MASK GENMASK(4, 0) > +#define MT6360_ISTROBE_MASK GENMASK(6, 0) > +#define MT6360_STRBTO_MASK GENMASK(6, 0) > +#define MT6360_TORCHEN_MASK BIT(3) > +#define MT6360_STROBEN_MASK BIT(2) > +#define MT6360_FLCSEN_MASK(_id) BIT(MT6360_LED_FLASH2 - _id) > +#define MT6360_FLEDCHGVINOVP_MASK BIT(3) > +#define MT6360_FLED1STRBTO_MASK BIT(11) > +#define MT6360_FLED2STRBTO_MASK BIT(10) > +#define MT6360_FLED1STRB_MASK BIT(9) > +#define MT6360_FLED2STRB_MASK BIT(8) > +#define MT6360_FLED1SHORT_MASK BIT(7) > +#define MT6360_FLED2SHORT_MASK BIT(6) > +#define MT6360_FLEDLVF_MASK BIT(3) > + > +/* 0 means led_off, add one for dummy */ > +#define MT6360_ISNK1_MAXLEVEL 13 > +#define MT6360_ISNK4_MAXLEVEL 31 > + > +#define MT6360_ITORCH_MIN 25000 > +#define MT6360_ITORCH_STEP 12500 > +#define MT6360_ITORCH_MAX 400000 > +#define MT6360_ISTRB_MIN 50000 > +#define MT6360_ISTRB_STEP 12500 > +#define MT6360_ISTRB_MAX 1500000 > +#define MT6360_STRBTO_MIN 64000 > +#define MT6360_STRBTO_STEP 32000 > +#define MT6360_STRBTO_MAX 2432000 > + > +#define FLED_TORCH_FLAG_MASK 0x0c > +#define FLED_TORCH_FLAG_SHFT 2 > +#define FLED_STROBE_FLAG_MASK 0x03 > + > +#define STATE_OFF 0 > +#define STATE_KEEP 1 > +#define STATE_ON 2 > + > +struct mt6360_led { > + union { > + struct led_classdev isnk; > + struct led_classdev_flash flash; > + }; > + struct v4l2_flash *v4l2_flash; > + struct mt6360_priv *priv; > + u32 led_no; > + u32 default_state; > +}; > + > +struct mt6360_priv { > + struct device *dev; > + struct regmap *regmap; > + struct mt6360_led *leds[MT6360_MAX_LEDS]; I would prefer to use a flexible array here as you are not guaranteed that the DT will contain 6 LED entries and it will simplify your probe and DT parsing. There are examples of using the flexible array https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/leds/leds-cr0014114.c https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/leds/leds-lm3697.c > + unsigned int fled_strobe_used; > + unsigned int fled_torch_used; > +}; > + > +static int mt6360_isnk_brightness_set(struct led_classdev *lcdev, enum led_brightness level) > +{ > + struct mt6360_led *led = container_of(lcdev, struct mt6360_led, isnk); > + struct mt6360_priv *priv = led->priv; > + u32 enable_mask = MT6360_ISNK_ENMASK(led->led_no); > + u32 val = level ? MT6360_ISNK_ENMASK(led->led_no) : 0; > + int ret; > + > + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); > + > + if (level) { > + ret = regmap_update_bits(priv->regmap, MT6360_REG_ISNK(led->led_no), > + MT6360_ISNK_MASK, level - 1); > + if (ret) > + return ret; > + } > + > + ret = regmap_update_bits(priv->regmap, MT6360_REG_RGBEN, enable_mask, val); > + if (ret) > + return ret; > + > + return 0; > +} > + > +static int mt6360_torch_brightness_set(struct led_classdev *lcdev, enum led_brightness level) > +{ > + struct mt6360_led *led = container_of(lcdev, struct mt6360_led, flash.led_cdev); > + struct mt6360_priv *priv = led->priv; > + u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no); > + u32 val = (level) ? MT6360_FLCSEN_MASK(led->led_no) : 0; > + u32 prev = priv->fled_torch_used, curr; > + int ret; > + > + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); > + if (priv->fled_strobe_used) { > + dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used); > + return -EINVAL; > + } > + > + if (level) > + curr = prev | BIT(led->led_no); > + else > + curr = prev & (~BIT(led->led_no)); > + > + if (curr) > + val |= MT6360_TORCHEN_MASK; > + > + if (level) { > + ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDITOR(led->led_no), > + MT6360_ITORCH_MASK, level - 1); > + if (ret) > + return ret; > + } > + > + ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDEN, enable_mask, val); > + if (ret) > + return ret; > + > + priv->fled_torch_used = curr; > + > + return 0; > +} > + > +static int mt6360_strobe_brightness_set(struct led_classdev_flash *fl_cdev, u32 brightness) > +{ > + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); > + struct led_classdev *lcdev = &fl_cdev->led_cdev; > + > + dev_dbg(lcdev->dev, "[%d] strobe brightness %d\n", led->led_no, brightness); > + return 0; > +} > + > +static int _mt6360_strobe_brightness_set(struct led_classdev_flash *fl_cdev, u32 brightness) > +{ > + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); > + struct mt6360_priv *priv = led->priv; > + struct led_flash_setting *s = &fl_cdev->brightness; > + u32 val = (brightness - s->min) / s->step; > + > + return regmap_update_bits(priv->regmap, MT6360_REG_FLEDISTRB(led->led_no), > + MT6360_ISTROBE_MASK, val); > +} > + > +static int mt6360_strobe_set(struct led_classdev_flash *fl_cdev, bool state) > +{ > + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); > + struct mt6360_priv *priv = led->priv; > + struct led_classdev *lcdev = &fl_cdev->led_cdev; > + struct led_flash_setting *s = &fl_cdev->brightness; > + u32 enable_mask = MT6360_STROBEN_MASK | MT6360_FLCSEN_MASK(led->led_no); > + u32 val = state ? MT6360_FLCSEN_MASK(led->led_no) : 0; > + u32 prev = priv->fled_strobe_used, curr; > + int ret; > + > + dev_dbg(lcdev->dev, "[%d] strobe state %d\n", led->led_no, state); > + if (priv->fled_torch_used) { > + dev_warn(lcdev->dev, "Please disable torch first [0x%x]\n", priv->fled_torch_used); > + return -EINVAL; > + } > + > + if (state) > + curr = prev | BIT(led->led_no); > + else > + curr = prev & (~BIT(led->led_no)); > + > + if (curr) > + val |= MT6360_STROBEN_MASK; > + > + ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDEN, enable_mask, val); > + if (ret) { > + dev_err(lcdev->dev, "[%d] control current source %d fail\n", led->led_no, state); > + return ret; > + } > + > + /* used to prevent flash current spike when torch on */ > + ret = _mt6360_strobe_brightness_set(fl_cdev, state ? s->val : s->min); > + if (ret) > + return ret; > + > + if (!prev && curr) > + usleep_range(5000, 6000); > + else if (prev && !curr) > + udelay(500); > + > + priv->fled_strobe_used = curr; > + return 0; > +} > + > +static int mt6360_strobe_get(struct led_classdev_flash *fl_cdev, bool *state) > +{ > + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); > + struct mt6360_priv *priv = led->priv; > + > + *state = !!(priv->fled_strobe_used & BIT(led->led_no)); > + return 0; > +} > + > +static int mt6360_timeout_set(struct led_classdev_flash *fl_cdev, u32 timeout) > +{ > + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); > + struct mt6360_priv *priv = led->priv; > + struct led_flash_setting *s = &fl_cdev->timeout; > + u32 val = (timeout - s->min) / s->step; > + > + return regmap_update_bits(priv->regmap, MT6360_REG_STRBTO, MT6360_STRBTO_MASK, val); > + > +} > + > +static int mt6360_fault_get(struct led_classdev_flash *fl_cdev, u32 *fault) > +{ > + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); > + struct mt6360_priv *priv = led->priv; > + u16 fled_stat; > + unsigned int chg_stat, strobe_timeout_mask, fled_short_mask; > + u32 rfault = 0; > + int ret; > + > + ret = regmap_read(priv->regmap, MT6360_REG_CHGSTAT2, &chg_stat); > + if (ret) > + return ret; > + > + ret = regmap_raw_read(priv->regmap, MT6360_REG_FLEDSTAT1, &fled_stat, sizeof(fled_stat)); > + if (ret) > + return ret; > + > + if (led->led_no == MT6360_LED_FLASH1) { > + strobe_timeout_mask = MT6360_FLED1STRBTO_MASK; > + fled_short_mask = MT6360_FLED1SHORT_MASK; > + > + } else { > + strobe_timeout_mask = MT6360_FLED2STRBTO_MASK; > + fled_short_mask = MT6360_FLED2SHORT_MASK; > + } > + > + if (chg_stat & MT6360_FLEDCHGVINOVP_MASK) > + rfault |= LED_FAULT_INPUT_VOLTAGE; > + > + if (fled_stat & strobe_timeout_mask) > + rfault |= LED_FAULT_TIMEOUT; > + > + if (fled_stat & fled_short_mask) > + rfault |= LED_FAULT_SHORT_CIRCUIT; > + > + if (fled_stat & MT6360_FLEDLVF_MASK) > + rfault |= LED_FAULT_UNDER_VOLTAGE; > + > + *fault = rfault; > + return 0; > +} > + > +static const struct led_flash_ops mt6360_flash_ops = { > + .flash_brightness_set = mt6360_strobe_brightness_set, > + .strobe_set = mt6360_strobe_set, > + .strobe_get = mt6360_strobe_get, > + .timeout_set = mt6360_timeout_set, > + .fault_get = mt6360_fault_get, > +}; > + > +static int mt6360_isnk_init_default_state(struct mt6360_led *led) > +{ > + struct mt6360_priv *priv = led->priv; > + unsigned int regval; > + u32 level; > + int ret; > + > + ret = regmap_read(priv->regmap, MT6360_REG_ISNK(led->led_no), ®val); > + if (ret) > + return ret; > + level = regval & MT6360_ISNK_MASK; > + > + ret = regmap_read(priv->regmap, MT6360_REG_RGBEN, ®val); > + if (ret) > + return ret; > + > + if (regval & MT6360_ISNK_ENMASK(led->led_no)) > + level += 1; > + else > + level = LED_OFF; > + > + switch (led->default_state) { > + case STATE_ON: > + led->isnk.brightness = led->isnk.max_brightness; > + break; > + case STATE_KEEP: > + led->isnk.brightness = min(level, led->isnk.max_brightness); > + break; > + default: > + led->isnk.brightness = LED_OFF; > + } > + > + return mt6360_isnk_brightness_set(&led->isnk, led->isnk.brightness); > +} > + > +static int mt6360_isnk_register(struct device *parent, struct mt6360_led *led, > + struct led_init_data *init_data) > +{ > + struct mt6360_priv *priv = led->priv; > + int ret; > + > + if (led->led_no == MT6360_LED_ISNK1) { > + /* change isink to sw mode */ > + ret = regmap_update_bits(priv->regmap, MT6360_REG_RGBEN, MT6360_CHRINDSEL_MASK, > + MT6360_CHRINDSEL_MASK); > + if (ret) { > + dev_err(parent, "Failed to config ISNK1 to SW mode\n"); > + return ret; > + } > + } > + > + ret = mt6360_isnk_init_default_state(led); > + if (ret) { > + dev_err(parent, "Failed to init %d isnk state\n", led->led_no); > + return ret; > + } > + > + ret = devm_led_classdev_register_ext(parent, &led->isnk, init_data); > + if (ret) { > + dev_err(parent, "Couldn't register isink %d\n", led->led_no); > + return ret; > + } > + > + return 0; > +} > + > +static int mt6360_flash_init_default_state(struct mt6360_led *led) > +{ > + struct led_classdev_flash *flash = &led->flash; > + struct mt6360_priv *priv = led->priv; > + u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no); > + u32 level; > + unsigned int regval; > + int ret; > + > + ret = regmap_read(priv->regmap, MT6360_REG_FLEDITOR(led->led_no), ®val); > + if (ret) > + return ret; > + level = regval & MT6360_ITORCH_MASK; > + > + ret = regmap_read(priv->regmap, MT6360_REG_FLEDEN, ®val); > + if (ret) > + return ret; > + > + if ((regval & enable_mask) == enable_mask) > + level += 1; > + else > + level = LED_OFF; > + > + switch (led->default_state) { > + case STATE_ON: > + flash->led_cdev.brightness = flash->led_cdev.max_brightness; > + break; > + case STATE_KEEP: > + flash->led_cdev.brightness = min(level, flash->led_cdev.max_brightness); > + break; > + default: > + flash->led_cdev.brightness = LED_OFF; > + } > + > + return mt6360_torch_brightness_set(&flash->led_cdev, flash->led_cdev.brightness); > +} > + > +#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS) > +static int mt6360_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable) > +{ > + struct led_classdev_flash *flash = v4l2_flash->fled_cdev; > + struct mt6360_led *led = container_of(flash, struct mt6360_led, flash); > + struct mt6360_priv *priv = led->priv; > + u32 enable_mask = MT6360_FLCSEN_MASK(led->led_no); > + int ret; > + > + ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDEN, enable_mask, > + enable ? enable_mask : 0); > + if (ret) > + return ret; > + > + if (enable) > + priv->fled_strobe_used |= BIT(led->led_no); > + else > + priv->fled_strobe_used &= (~BIT(led->led_no)); > + > + return 0; > +} > + > +static const struct v4l2_flash_ops v4l2_flash_ops = { > + .external_strobe_set = mt6360_flash_external_strobe_set, > +}; > + > +static void mt6360_flash_init_v4l2_config(struct mt6360_led *led, struct v4l2_flash_config *config) > +{ > + struct led_classdev_flash *flash = &led->flash; > + struct led_classdev *lcdev = &flash->led_cdev; > + struct led_flash_setting *s = &config->intensity; > + > + snprintf(config->dev_name, sizeof(config->dev_name), "%s", lcdev->name); > + > + s->min = MT6360_ITORCH_MIN; > + s->step = MT6360_ITORCH_STEP; > + s->val = s->max = (s->min) + (lcdev->max_brightness - 1) * s->step; > + > + config->has_external_strobe = 1; > +} > +#else > +static const struct v4l2_flash_ops v4l2_flash_ops; > + > +static void mt6360_flash_init_v4l2_config(struct mt6360_led *led, struct v4l2_flash_config *config) > +{ > +} > +#endif > + > +static int mt6360_flash_register(struct device *parent, struct mt6360_led *led, > + struct led_init_data *init_data) > +{ > + struct v4l2_flash_config v4l2_config = {0}; > + int ret; > + > + ret = mt6360_flash_init_default_state(led); > + if (ret) { > + dev_err(parent, "Failed to init %d flash state\n", led->led_no); > + return ret; > + } > + > + ret = devm_led_classdev_flash_register_ext(parent, &led->flash, init_data); > + if (ret) { > + dev_err(parent, "Couldn't register flash %d\n", led->led_no); > + return ret; > + } > + > + mt6360_flash_init_v4l2_config(led, &v4l2_config); > + led->v4l2_flash = v4l2_flash_init(parent, init_data->fwnode, &led->flash, &v4l2_flash_ops, > + &v4l2_config); > + if (IS_ERR(led->v4l2_flash)) { > + dev_err(parent, "Failed to register %d v4l2 sd\n", led->led_no); > + return PTR_ERR(led->v4l2_flash); > + } > + > + return 0; > +} > + > +static int mt6360_init_isnk_properties(struct mt6360_led *led, struct led_init_data *init_data) > +{ > + struct led_classdev *isnk = &led->isnk; > + > + if (led->led_no == MT6360_LED_ISNK4) > + isnk->max_brightness = MT6360_ISNK4_MAXLEVEL; > + else > + isnk->max_brightness = MT6360_ISNK1_MAXLEVEL; > + > + isnk->brightness_set_blocking = mt6360_isnk_brightness_set; > + > + fwnode_property_read_string(init_data->fwnode, "linux,default-trigger", > + &isnk->default_trigger); > + > + return 0; > +} > + > +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 mt6360_init_flash_properties(struct mt6360_led *led, struct led_init_data *init_data) > +{ > + struct led_classdev_flash *flash = &led->flash; > + struct led_classdev *lcdev = &flash->led_cdev; > + struct led_flash_setting *s; > + u32 val; > + int ret; > + > + ret = fwnode_property_read_u32(init_data->fwnode, "led-max-microamp", &val); > + if (ret) > + val = MT6360_ITORCH_MIN; > + else > + clamp_align(&val, MT6360_ITORCH_MIN, MT6360_ITORCH_MAX, MT6360_ITORCH_STEP); > + > + lcdev->max_brightness = (val - MT6360_ITORCH_MIN) / MT6360_ITORCH_STEP + 1; > + lcdev->brightness_set_blocking = mt6360_torch_brightness_set; > + lcdev->flags |= LED_DEV_CAP_FLASH; > + > + ret = fwnode_property_read_u32(init_data->fwnode, "flash-max-microamp", &val); > + if (ret) > + val = MT6360_ISTRB_MIN; > + else > + clamp_align(&val, MT6360_ISTRB_MIN, MT6360_ISTRB_MAX, MT6360_ISTRB_STEP); > + > + s = &flash->brightness; > + s->min = MT6360_ISTRB_MIN; > + s->step = MT6360_ISTRB_STEP; > + s->val = s->max = val; > + > + /* always configure as min level when off to prevent flash current spike */ > + ret = _mt6360_strobe_brightness_set(flash, s->min); > + if (ret) > + return ret; > + > + ret = fwnode_property_read_u32(init_data->fwnode, "flash-max-timeout-us", &val); > + if (ret) > + val = MT6360_STRBTO_MIN; > + else > + clamp_align(&val, MT6360_STRBTO_MIN, MT6360_STRBTO_MAX, MT6360_STRBTO_STEP); > + > + s = &flash->timeout; > + s->min = MT6360_STRBTO_MIN; > + s->step = MT6360_STRBTO_STEP; > + s->val = s->max = val; > + > + flash->ops = &mt6360_flash_ops; > + > + return 0; > +} > + > +static int mt6360_init_common_properties(struct mt6360_led *led, struct led_init_data *init_data) > +{ > + const char *str; > + > + if (!fwnode_property_read_string(init_data->fwnode, "default-state", &str)) { > + if (!strcmp(str, "on")) > + led->default_state = STATE_ON; > + else if (!strcmp(str, "keep")) > + led->default_state = STATE_KEEP; > + else > + led->default_state = STATE_OFF; > + } > + > + return 0; > +} > + > +static int mt6360_led_probe(struct platform_device *pdev) > +{ > + struct mt6360_priv *priv; > + struct fwnode_handle *child; > + int i, ret; > + > + ret = device_get_child_node_count(&pdev->dev); > + if (!ret || ret > MT6360_MAX_LEDS) > + return -EINVAL; > + > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->dev = &pdev->dev; > + > + priv->regmap = dev_get_regmap(pdev->dev.parent, NULL); > + if (!priv->regmap) { > + dev_err(&pdev->dev, "Failed to get parent regmap\n"); > + return -ENODEV; > + } > + > + device_for_each_child_node(&pdev->dev, child) { > + struct mt6360_led *led; > + struct led_init_data init_data = { .fwnode = child, }; > + u32 reg; > + > + ret = fwnode_property_read_u32(child, "reg", ®); > + if (ret || reg >= MT6360_MAX_LEDS || priv->leds[reg]) { > + ret = -EINVAL; > + goto out; > + } > + > + led = devm_kzalloc(&pdev->dev, sizeof(*led), GFP_KERNEL); Using the flexible array in the mt6360_priv will eliminate this allocation. Dan
Hi! > From: Gene Chen <gene_chen@richtek.com> > > Add MT6360 LED driver include 2-channel Flash LED with torch/strobe mode, > and 4-channel RGB LED support Register/Flash/Breath Mode > > Signed-off-by: Gene Chen <gene_chen@richtek.com> > --- > drivers/leds/Kconfig | 11 + > drivers/leds/Makefile | 1 + > drivers/leds/leds-mt6360.c | 681 +++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 693 insertions(+) > create mode 100644 drivers/leds/leds-mt6360.c > > + help > + This option enables support for dual Flash LED drivers found on > + Mediatek MT6360 PMIC. > + Independent current sources supply for each flash LED support torch and strobe mode. > + Includes Low-VF and short protection. > + 80 columns. And perhaps user does not need to know about protections... and actually about independend sources, either. "Enable this for RGB LED and flash LED support on..."? > +static int mt6360_torch_brightness_set(struct led_classdev *lcdev, enum led_brightness level) > +{ > + struct mt6360_led *led = container_of(lcdev, struct mt6360_led, flash.led_cdev); > + struct mt6360_priv *priv = led->priv; > + u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no); > + u32 val = (level) ? MT6360_FLCSEN_MASK(led->led_no) : 0; > + u32 prev = priv->fled_torch_used, curr; > + int ret; > + > + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); > + if (priv->fled_strobe_used) { > + dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used); > + return -EINVAL; > + } So... how does its userland interface look like? Best regards, Pavel
On Mon, Sep 7, 2020 at 1:31 PM Gene Chen <gene.chen.richtek@gmail.com> wrote: > > From: Gene Chen <gene_chen@richtek.com> > > Add MT6360 LED driver include 2-channel Flash LED with torch/strobe mode, > and 4-channel RGB LED support Register/Flash/Breath Mode I'm wondering why you don't use struct led_classdev_flash. ... > +// > +// Copyright (C) 2020 MediaTek Inc. > +// Do you really need these two // lines? ... > +enum { > + MT6360_LED_ISNK1 = 0, > + MT6360_LED_ISNK2, > + MT6360_LED_ISNK3, > + MT6360_LED_ISNK4, > + MT6360_LED_FLASH1, > + MT6360_LED_FLASH2, > + MT6360_MAX_LEDS, No comma for terminator entry. > +}; ... > +#define MT6360_ISNK_MASK 0x1F GENMASK() ... > +#define MT6360_ITORCH_MIN 25000 > +#define MT6360_ITORCH_STEP 12500 > +#define MT6360_ITORCH_MAX 400000 > +#define MT6360_ISTRB_MIN 50000 > +#define MT6360_ISTRB_STEP 12500 > +#define MT6360_ISTRB_MAX 1500000 > +#define MT6360_STRBTO_MIN 64000 > +#define MT6360_STRBTO_STEP 32000 > +#define MT6360_STRBTO_MAX 2432000 Add unit suffixes, please. ... > +#define FLED_TORCH_FLAG_MASK 0x0c > +#define FLED_STROBE_FLAG_MASK 0x03 GENMASK() ... > + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); Not production noise. ... > + ret = regmap_update_bits(priv->regmap, MT6360_REG_RGBEN, enable_mask, val); > + if (ret) > + return ret; > + > + return 0; return regmap... > + u32 val = (level) ? MT6360_FLCSEN_MASK(led->led_no) : 0; Why parens? ... > + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); Noise. ... > + if (priv->fled_strobe_used) { > + dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used); > + return -EINVAL; Hmm... Shouldn't be guaranteed by some framework? ... > + curr = prev & (~BIT(led->led_no)); Too many parens. ... > +static int mt6360_strobe_brightness_set(struct led_classdev_flash *fl_cdev, u32 brightness) > +{ > + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); > + struct led_classdev *lcdev = &fl_cdev->led_cdev; > + > + dev_dbg(lcdev->dev, "[%d] strobe brightness %d\n", led->led_no, brightness); Noise. Point of this entire function? > + return 0; > +} ... > + dev_dbg(lcdev->dev, "[%d] strobe state %d\n", led->led_no, state); Noise. If you wish to do it right, add trace events to the framework. ... > + if (priv->fled_torch_used) { > + dev_warn(lcdev->dev, "Please disable torch first [0x%x]\n", priv->fled_torch_used); Again, why the warning? Can this be a part of the framework? > + return -EINVAL; > + } ... > + curr = prev & (~BIT(led->led_no)); Too many parens. ... > + if (!prev && curr) > + usleep_range(5000, 6000); > + else if (prev && !curr) > + udelay(500); These delays must be explained. ... > + if (led->led_no == MT6360_LED_FLASH1) { > + strobe_timeout_mask = MT6360_FLED1STRBTO_MASK; > + fled_short_mask = MT6360_FLED1SHORT_MASK; > + Redundant blank line. > + } else { > + strobe_timeout_mask = MT6360_FLED2STRBTO_MASK; > + fled_short_mask = MT6360_FLED2SHORT_MASK; > + } ... > +static int mt6360_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable) > +{ > + struct led_classdev_flash *flash = v4l2_flash->fled_cdev; > + struct mt6360_led *led = container_of(flash, struct mt6360_led, flash); > + struct mt6360_priv *priv = led->priv; > + u32 enable_mask = MT6360_FLCSEN_MASK(led->led_no); enable_mask -> mask u32 value = enable ? mask : 0; > + int ret; > + > + ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDEN, enable_mask, > + enable ? enable_mask : 0); ret = ... mask, value); > + if (ret) > + return ret; > + > + if (enable) > + priv->fled_strobe_used |= BIT(led->led_no); > + else > + priv->fled_strobe_used &= (~BIT(led->led_no)); Too many parens. > + > + return 0; > +} ... > + s->val = s->max = (s->min) + (lcdev->max_brightness - 1) * s->step; Ditto. ... > +static void clamp_align(u32 *v, u32 min, u32 max, u32 step) Can we keep a similar API, i.e. return a new value rather than update old? > +{ > + *v = clamp_val(*v, min, max); I would rather use a temporary variable (and it actually will be required with above). > + if (step > 1) > + *v = (*v - min) / step * step + min; Sounds like open coded rounddown(). > +} ... > + lcdev->max_brightness = (val - MT6360_ITORCH_MIN) / MT6360_ITORCH_STEP + 1; DIV_ROUND_UP(val - MT6360_ITORCH_MIN, MT6360_ITORCH_STEP) ? ... > +static int mt6360_init_common_properties(struct mt6360_led *led, struct led_init_data *init_data) > +{ > + const char *str; > + > + if (!fwnode_property_read_string(init_data->fwnode, "default-state", &str)) { > + if (!strcmp(str, "on")) > + led->default_state = STATE_ON; > + else if (!strcmp(str, "keep")) > + led->default_state = STATE_KEEP; > + else I wouldn't allow some garbage to be off. > + led->default_state = STATE_OFF; > + } What about static const char * const states = { "on", "keep", "off" }; int ret; ret = match_string(states, ARRAY_SIZE(states), str); if (ret) ... default_state = ret; ? > + return 0; > +} ... > +static int mt6360_led_probe(struct platform_device *pdev) > +{ > + struct mt6360_priv *priv; > + struct fwnode_handle *child; > + int i, ret; > + > + priv->regmap = dev_get_regmap(pdev->dev.parent, NULL); > + if (!priv->regmap) { > + dev_err(&pdev->dev, "Failed to get parent regmap\n"); > + return -ENODEV; > + } ... > +out: out_flash_leds_release: ? > + for (i = MT6360_LED_FLASH1; i <= MT6360_LED_FLASH2; i++) { > + struct mt6360_led *led = priv->leds[i]; > + > + if (led && led->v4l2_flash) > + v4l2_flash_release(led->v4l2_flash); > + > + } ... > +static int mt6360_led_remove(struct platform_device *pdev) > +{ > + struct mt6360_priv *priv = platform_get_drvdata(pdev); > + int i; > + > + for (i = MT6360_LED_FLASH1; i <= MT6360_LED_FLASH2; i++) { > + struct mt6360_led *led = priv->leds[i]; > + > + if (led && led->v4l2_flash) > + v4l2_flash_release(led->v4l2_flash); > + > + } Looks like a code duplication. > + > + return 0; > +} > + > +static const struct of_device_id __maybe_unused mt6360_led_of_id[] = { > + { .compatible = "mediatek,mt6360-led", }, > + {}, No need comma. > +};
Pavel Machek <pavel@ucw.cz> 於 2020年9月9日 週三 上午6:25寫道： > > Hi! > > > From: Gene Chen <gene_chen@richtek.com> > > > > Add MT6360 LED driver include 2-channel Flash LED with torch/strobe mode, > > and 4-channel RGB LED support Register/Flash/Breath Mode > > > > Signed-off-by: Gene Chen <gene_chen@richtek.com> > > --- > > drivers/leds/Kconfig | 11 + > > drivers/leds/Makefile | 1 + > > drivers/leds/leds-mt6360.c | 681 +++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 693 insertions(+) > > create mode 100644 drivers/leds/leds-mt6360.c > > > > + help > > + This option enables support for dual Flash LED drivers found on > > + Mediatek MT6360 PMIC. > > + Independent current sources supply for each flash LED support torch and strobe mode. > > + Includes Low-VF and short protection. > > + > > 80 columns. And perhaps user does not need to know about protections... and actually > about independend sources, either. > ACK > "Enable this for RGB LED and flash LED support on..."? > > > +static int mt6360_torch_brightness_set(struct led_classdev *lcdev, enum led_brightness level) > > +{ > > + struct mt6360_led *led = container_of(lcdev, struct mt6360_led, flash.led_cdev); > > + struct mt6360_priv *priv = led->priv; > > + u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no); > > + u32 val = (level) ? MT6360_FLCSEN_MASK(led->led_no) : 0; > > + u32 prev = priv->fled_torch_used, curr; > > + int ret; > > + > > + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); > > + if (priv->fled_strobe_used) { > > + dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used); > > + return -EINVAL; > > + } > > So... how does its userland interface look like? > 1. set FLED1 brightness # echo 1 > /sys/class/leds/white:flash1/flash_brightness 2. enable FLED1 strobe # echo 1 > /sys/class/leds/white:flash1/flash_strobe 3 . turn off FLED1 strobe (just used to gaurantee the strobe mode flash leds must be turned off) # echo 0 > /sys/class/leds/white:flash1/flash_strobe > Best regards, > Pavel > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Andy Shevchenko <andy.shevchenko@gmail.com> 於 2020年9月9日 週三 下午9:48寫道： > > On Mon, Sep 7, 2020 at 1:31 PM Gene Chen <gene.chen.richtek@gmail.com> wrote: > > > > From: Gene Chen <gene_chen@richtek.com> > > > > Add MT6360 LED driver include 2-channel Flash LED with torch/strobe mode, > > and 4-channel RGB LED support Register/Flash/Breath Mode > > I'm wondering why you don't use struct led_classdev_flash. > > ... > Both Flash and RGB LED use led_classdev_flash by "devm_led_classdev_flash_register_ext". > > +// > > +// Copyright (C) 2020 MediaTek Inc. > > +// > > Do you really need these two // lines? > ACK, I will remove it > ... > > > +enum { > > + MT6360_LED_ISNK1 = 0, > > + MT6360_LED_ISNK2, > > + MT6360_LED_ISNK3, > > + MT6360_LED_ISNK4, > > + MT6360_LED_FLASH1, > > + MT6360_LED_FLASH2, > > > + MT6360_MAX_LEDS, > > No comma for terminator entry. > ACK > > +}; > > ... > > > +#define MT6360_ISNK_MASK 0x1F > > GENMASK() > > ... > > > +#define MT6360_ITORCH_MIN 25000 > > +#define MT6360_ITORCH_STEP 12500 > > +#define MT6360_ITORCH_MAX 400000 > > +#define MT6360_ISTRB_MIN 50000 > > +#define MT6360_ISTRB_STEP 12500 > > +#define MT6360_ISTRB_MAX 1500000 > > +#define MT6360_STRBTO_MIN 64000 > > +#define MT6360_STRBTO_STEP 32000 > > +#define MT6360_STRBTO_MAX 2432000 > > Add unit suffixes, please. > ACK > ... > > > +#define FLED_TORCH_FLAG_MASK 0x0c > > > +#define FLED_STROBE_FLAG_MASK 0x03 > > GENMASK() > ACK > ... > > > + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); > > Not production noise. > ACK > ... > > > + ret = regmap_update_bits(priv->regmap, MT6360_REG_RGBEN, enable_mask, val); > > + if (ret) > > + return ret; > > + > > + return 0; > > return regmap... > > > + u32 val = (level) ? MT6360_FLCSEN_MASK(led->led_no) : 0; > > Why parens? > ACK > ... > > > + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); > > Noise. > ACK > ... > > > + if (priv->fled_strobe_used) { > > + dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used); > > + return -EINVAL; > > Hmm... Shouldn't be guaranteed by some framework? > Because both Flash LED use single logically control. It doesn't exist one LED is torch mode, and the other is strobe mode. > ... > > > + curr = prev & (~BIT(led->led_no)); > > Too many parens. > ACK > ... > > > +static int mt6360_strobe_brightness_set(struct led_classdev_flash *fl_cdev, u32 brightness) > > +{ > > + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); > > + struct led_classdev *lcdev = &fl_cdev->led_cdev; > > + > > > + dev_dbg(lcdev->dev, "[%d] strobe brightness %d\n", led->led_no, brightness); > > Noise. Point of this entire function? > ACK, I will remove it, reserve function entry only for register ledcass_dev check ops exist > > + return 0; > > +} > > ... > > > + dev_dbg(lcdev->dev, "[%d] strobe state %d\n", led->led_no, state); > > Noise. > > If you wish to do it right, add trace events to the framework. > ACK, I will remove it. > ... > > > + if (priv->fled_torch_used) { > > > + dev_warn(lcdev->dev, "Please disable torch first [0x%x]\n", priv->fled_torch_used); > > Again, why the warning? Can this be a part of the framework? > Same as above. > > + return -EINVAL; > > + } > > ... > > > + curr = prev & (~BIT(led->led_no)); > > Too many parens. > ACK > ... > > > + if (!prev && curr) > > + usleep_range(5000, 6000); > > + else if (prev && !curr) > > + udelay(500); > > These delays must be explained. > ACK > ... > > > + if (led->led_no == MT6360_LED_FLASH1) { > > + strobe_timeout_mask = MT6360_FLED1STRBTO_MASK; > > + fled_short_mask = MT6360_FLED1SHORT_MASK; > > > + > > Redundant blank line. > ACK > > + } else { > > + strobe_timeout_mask = MT6360_FLED2STRBTO_MASK; > > + fled_short_mask = MT6360_FLED2SHORT_MASK; > > + } > > ... > > > +static int mt6360_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable) > > +{ > > + struct led_classdev_flash *flash = v4l2_flash->fled_cdev; > > + struct mt6360_led *led = container_of(flash, struct mt6360_led, flash); > > + struct mt6360_priv *priv = led->priv; > > > + u32 enable_mask = MT6360_FLCSEN_MASK(led->led_no); > > enable_mask -> mask > u32 value = enable ? mask : 0; > > > + int ret; > > + > > + ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDEN, enable_mask, > > > + enable ? enable_mask : 0); > > ret = ... mask, value); > ACK > > + if (ret) > > + return ret; > > + > > + if (enable) > > + priv->fled_strobe_used |= BIT(led->led_no); > > + else > > + priv->fled_strobe_used &= (~BIT(led->led_no)); > > Too many parens. > ACK > > + > > + return 0; > > +} > > ... > > > + s->val = s->max = (s->min) + (lcdev->max_brightness - 1) * s->step; > > Ditto. > ACK > ... > > > +static void clamp_align(u32 *v, u32 min, u32 max, u32 step) > > Can we keep a similar API, i.e. return a new value rather than update old? > > > +{ > > > + *v = clamp_val(*v, min, max); > > I would rather use a temporary variable (and it actually will be > required with above). > > > + if (step > 1) > > + *v = (*v - min) / step * step + min; > > Sounds like open coded rounddown(). > ACK > > +} > > ... > > > + lcdev->max_brightness = (val - MT6360_ITORCH_MIN) / MT6360_ITORCH_STEP + 1; > > DIV_ROUND_UP(val - MT6360_ITORCH_MIN, MT6360_ITORCH_STEP) ? > This is mapping 0~val to 1~max_brightness as level. I convert val below MT6360_ITORCH_STEP to 1 for ignore max_brightness = 0, because 0 means disable. There is a little difference from DIV_ROUND_UP. > ... > > > +static int mt6360_init_common_properties(struct mt6360_led *led, struct led_init_data *init_data) > > +{ > > + const char *str; > > + > > + if (!fwnode_property_read_string(init_data->fwnode, "default-state", &str)) { > > + if (!strcmp(str, "on")) > > + led->default_state = STATE_ON; > > + else if (!strcmp(str, "keep")) > > + led->default_state = STATE_KEEP; > > > + else > > I wouldn't allow some garbage to be off. > ACK > > + led->default_state = STATE_OFF; > > + } > > What about > > static const char * const states = { "on", "keep", "off" }; > > int ret; > > ret = match_string(states, ARRAY_SIZE(states), str); > if (ret) > ... > > default_state = ret; > > ? > > > + return 0; > > +} > ACK > ... > > > +static int mt6360_led_probe(struct platform_device *pdev) > > +{ > > + struct mt6360_priv *priv; > > + struct fwnode_handle *child; > > + int i, ret; > > + > > > + priv->regmap = dev_get_regmap(pdev->dev.parent, NULL); > > + if (!priv->regmap) { > > + dev_err(&pdev->dev, "Failed to get parent regmap\n"); > > + return -ENODEV; > > + } > > ... > > > +out: > > out_flash_leds_release: ? > ACK > > + for (i = MT6360_LED_FLASH1; i <= MT6360_LED_FLASH2; i++) { > > + struct mt6360_led *led = priv->leds[i]; > > + > > + if (led && led->v4l2_flash) > > + v4l2_flash_release(led->v4l2_flash); > > + > > + } > > ... > > > +static int mt6360_led_remove(struct platform_device *pdev) > > +{ > > + struct mt6360_priv *priv = platform_get_drvdata(pdev); > > + int i; > > + > > + for (i = MT6360_LED_FLASH1; i <= MT6360_LED_FLASH2; i++) { > > + struct mt6360_led *led = priv->leds[i]; > > + > > + if (led && led->v4l2_flash) > > + v4l2_flash_release(led->v4l2_flash); > > + > > + } > > Looks like a code duplication. > ACK > > + > > + return 0; > > +} > > + > > +static const struct of_device_id __maybe_unused mt6360_led_of_id[] = { > > + { .compatible = "mediatek,mt6360-led", }, > > > + {}, > > No need comma. > ACK > > +}; > > > -- > With Best Regards, > Andy Shevchenko
Hi! > > > +enum { > > > + MT6360_LED_ISNK1 = 0, > > > + MT6360_LED_ISNK2, > > > + MT6360_LED_ISNK3, > > > + MT6360_LED_ISNK4, > > > + MT6360_LED_FLASH1, > > > + MT6360_LED_FLASH2, > > > > > + MT6360_MAX_LEDS, > > > > No comma for terminator entry. > > > > ACK Actually, that comma is fine. Its absence would be fine, too. > > > +}; > > > > ... > > > > > +#define MT6360_ISNK_MASK 0x1F > > > > GENMASK() Again, that is fine. > > > +#define FLED_TORCH_FLAG_MASK 0x0c > > > > > +#define FLED_STROBE_FLAG_MASK 0x03 > > > > GENMASK() > > > > ACK Again, that is fine. > > > + return 0; > > > +} > > > + > > > +static const struct of_device_id __maybe_unused mt6360_led_of_id[] = { > > > + { .compatible = "mediatek,mt6360-led", }, > > > > > + {}, > > > > No need comma. > > ACK It is also no hurting comma. Best regards, Pavel
On Thu, Sep 10, 2020 at 11:18 AM Pavel Machek <pavel@ucw.cz> wrote: ... > > > > +enum { > > > > + MT6360_LED_ISNK1 = 0, > > > > + MT6360_LED_ISNK2, > > > > + MT6360_LED_ISNK3, > > > > + MT6360_LED_ISNK4, > > > > + MT6360_LED_FLASH1, > > > > + MT6360_LED_FLASH2, > > > > > > > + MT6360_MAX_LEDS, > > > > > > No comma for terminator entry. > > > > > > > ACK > > Actually, that comma is fine. Its absence would be fine, too. It is slightly better not to have to prevent (theoretical) rebase or other similar issues when a new item can go behind the terminator. In such a case compiler can easily tell you if something is wrong. > > > > +}; ... > > > > +static const struct of_device_id __maybe_unused mt6360_led_of_id[] = { > > > > + { .compatible = "mediatek,mt6360-led", }, > > > > > > > + {}, > > > > > > No need comma. > > > > ACK > > It is also no hurting comma. Same explanation. It doesn't hurt per se, but its absence might serve a purpose.
On Thu, Sep 10, 2020 at 11:11 AM Gene Chen <gene.chen.richtek@gmail.com> wrote: > Andy Shevchenko <andy.shevchenko@gmail.com> 於 2020年9月9日 週三 下午9:48寫道： > > On Mon, Sep 7, 2020 at 1:31 PM Gene Chen <gene.chen.richtek@gmail.com> wrote: > > > From: Gene Chen <gene_chen@richtek.com> ... > > > + if (priv->fled_strobe_used) { > > > + dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used); > > > + return -EINVAL; > > > > Hmm... Shouldn't be guaranteed by some framework? > > > > Because both Flash LED use single logically control. > It doesn't exist one LED is torch mode, and the other is strobe mode. You mean you have always an attribute for hardware even if it doesn't support a feature? Can you consider hiding attributes? ... > > > + lcdev->max_brightness = (val - MT6360_ITORCH_MIN) / MT6360_ITORCH_STEP + 1; > > > > DIV_ROUND_UP(val - MT6360_ITORCH_MIN, MT6360_ITORCH_STEP) ? > > > > This is mapping 0~val to 1~max_brightness as level. > I convert val below MT6360_ITORCH_STEP to 1 for ignore max_brightness > = 0, because 0 means disable. > There is a little difference from DIV_ROUND_UP. What div_round_up does is (x + y - 1) / y What do you do x / y + 1 = (x + y)/y = ((x + 1) + y - 1)/y = DIV_ROUND_UP(x+1,y) So, DIV_ROUND_UP(val - MT6360_ITORCH_MIN + 1, MT6360_ITORCH_STEP) ? (yes I made classical off-by-one error)
On Thu 2020-09-10 14:34:54, Andy Shevchenko wrote: > On Thu, Sep 10, 2020 at 11:18 AM Pavel Machek <pavel@ucw.cz> wrote: > > ... > > > > > > +enum { > > > > > + MT6360_LED_ISNK1 = 0, > > > > > + MT6360_LED_ISNK2, > > > > > + MT6360_LED_ISNK3, > > > > > + MT6360_LED_ISNK4, > > > > > + MT6360_LED_FLASH1, > > > > > + MT6360_LED_FLASH2, > > > > > > > > > + MT6360_MAX_LEDS, > > > > > > > > No comma for terminator entry. > > > > > > > > > > ACK > > > > Actually, that comma is fine. Its absence would be fine, too. > > It is slightly better not to have to prevent (theoretical) rebase or > other similar issues when a new item can go behind the terminator. In > such a case compiler can easily tell you if something is wrong. Okay, I see your point. Pavel
Hi! > > > +{ > > > + struct mt6360_led *led = container_of(lcdev, struct mt6360_led, flash.led_cdev); > > > + struct mt6360_priv *priv = led->priv; > > > + u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no); > > > + u32 val = (level) ? MT6360_FLCSEN_MASK(led->led_no) : 0; > > > + u32 prev = priv->fled_torch_used, curr; > > > + int ret; > > > + > > > + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); > > > + if (priv->fled_strobe_used) { > > > + dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used); > > > + return -EINVAL; > > > + } > > > > So... how does its userland interface look like? > > > > 1. set FLED1 brightness > # echo 1 > /sys/class/leds/white:flash1/flash_brightness > 2. enable FLED1 strobe > # echo 1 > /sys/class/leds/white:flash1/flash_strobe > 3 . turn off FLED1 strobe (just used to gaurantee the strobe mode > flash leds must be turned off) > # echo 0 > /sys/class/leds/white:flash1/flash_strobe I believe I'd preffer only exposing torch functionality in /sys/class/leds. .. strobe can be supported using v4l2 APIs. Best regards, Pavel
On 9/10/20 2:29 PM, Pavel Machek wrote: > Hi! > >>>> +{ >>>> + struct mt6360_led *led = container_of(lcdev, struct mt6360_led, flash.led_cdev); >>>> + struct mt6360_priv *priv = led->priv; >>>> + u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no); >>>> + u32 val = (level) ? MT6360_FLCSEN_MASK(led->led_no) : 0; >>>> + u32 prev = priv->fled_torch_used, curr; >>>> + int ret; >>>> + >>>> + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); >>>> + if (priv->fled_strobe_used) { >>>> + dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used); >>>> + return -EINVAL; >>>> + } >>> >>> So... how does its userland interface look like? >>> >> >> 1. set FLED1 brightness >> # echo 1 > /sys/class/leds/white:flash1/flash_brightness >> 2. enable FLED1 strobe >> # echo 1 > /sys/class/leds/white:flash1/flash_strobe >> 3 . turn off FLED1 strobe (just used to gaurantee the strobe mode >> flash leds must be turned off) >> # echo 0 > /sys/class/leds/white:flash1/flash_strobe > > I believe I'd preffer only exposing torch functionality in > /sys/class/leds. .. strobe can be supported using v4l2 APIs. Actually having LED flash class without strobe is pointless. If you looked at led_classdev_flash_register_ext() you would see that it fails with uninitialized strobe_set op. And V4L2 API for strobing flash calls strobe_set from LED flash class beneath. That was the idea behind LED and V4L2 flash API unification - there is one hardware driver needed, the V4L2 Flash layer just takes over control over it when needed.
Hi! > > > 1. set FLED1 brightness > > > # echo 1 > /sys/class/leds/white:flash1/flash_brightness > > > 2. enable FLED1 strobe > > > # echo 1 > /sys/class/leds/white:flash1/flash_strobe > > > 3 . turn off FLED1 strobe (just used to gaurantee the strobe mode > > > flash leds must be turned off) > > > # echo 0 > /sys/class/leds/white:flash1/flash_strobe > > > > I believe I'd preffer only exposing torch functionality in > > /sys/class/leds. .. strobe can be supported using v4l2 APIs. > > Actually having LED flash class without strobe is pointless. > If you looked at led_classdev_flash_register_ext() you would see that > it fails with uninitialized strobe_set op. And V4L2 API for strobing > flash calls strobe_set from LED flash class beneath. > > That was the idea behind LED and V4L2 flash API unification - there > is one hardware driver needed, the V4L2 Flash layer just takes over > control over it when needed. I agree that one driver is enough. But we should not need flash_strobe file in sysfs. Simply use V4L2 for that. Best regards, Pavel
On 9/10/20 10:25 PM, Pavel Machek wrote: > Hi! > >>>> 1. set FLED1 brightness >>>> # echo 1 > /sys/class/leds/white:flash1/flash_brightness >>>> 2. enable FLED1 strobe >>>> # echo 1 > /sys/class/leds/white:flash1/flash_strobe >>>> 3 . turn off FLED1 strobe (just used to gaurantee the strobe mode >>>> flash leds must be turned off) >>>> # echo 0 > /sys/class/leds/white:flash1/flash_strobe >>> >>> I believe I'd preffer only exposing torch functionality in >>> /sys/class/leds. .. strobe can be supported using v4l2 APIs. >> >> Actually having LED flash class without strobe is pointless. >> If you looked at led_classdev_flash_register_ext() you would see that >> it fails with uninitialized strobe_set op. And V4L2 API for strobing >> flash calls strobe_set from LED flash class beneath. >> >> That was the idea behind LED and V4L2 flash API unification - there >> is one hardware driver needed, the V4L2 Flash layer just takes over >> control over it when needed. > > I agree that one driver is enough. > > But we should not need flash_strobe file in sysfs. Simply use V4L2 for > that. Apart from the fact that we can't remove it from LED flash class ABI now, why would you like to prevent the user from exploiting main feature of the hardware?
Hi Gene, Thanks for the update. On 9/7/20 12:27 PM, Gene Chen wrote: > From: Gene Chen <gene_chen@richtek.com> > > Add MT6360 LED driver include 2-channel Flash LED with torch/strobe mode, > and 4-channel RGB LED support Register/Flash/Breath Mode > > Signed-off-by: Gene Chen <gene_chen@richtek.com> > --- > drivers/leds/Kconfig | 11 + > drivers/leds/Makefile | 1 + > drivers/leds/leds-mt6360.c | 681 +++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 693 insertions(+) > create mode 100644 drivers/leds/leds-mt6360.c > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index 1c181df..94a6d83 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -271,6 +271,17 @@ config LEDS_MT6323 > This option enables support for on-chip LED drivers found on > Mediatek MT6323 PMIC. > > +config LEDS_MT6360 > + tristate "LED Support for Mediatek MT6360 PMIC" > + depends on LEDS_CLASS_FLASH && OF > + depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS > + depends on MFD_MT6360 > + help > + This option enables support for dual Flash LED drivers found on > + Mediatek MT6360 PMIC. > + Independent current sources supply for each flash LED support torch and strobe mode. > + Includes Low-VF and short protection. > + > config LEDS_S3C24XX > tristate "LED Support for Samsung S3C24XX GPIO LEDs" > depends on LEDS_CLASS > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index c2c7d7a..5596427 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -66,6 +66,7 @@ obj-$(CONFIG_LEDS_MIKROTIK_RB532) += leds-rb532.o > obj-$(CONFIG_LEDS_MLXCPLD) += leds-mlxcpld.o > obj-$(CONFIG_LEDS_MLXREG) += leds-mlxreg.o > obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o > +obj-$(CONFIG_LEDS_MT6360) += leds-mt6360.o > obj-$(CONFIG_LEDS_NET48XX) += leds-net48xx.o > obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o > obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o > diff --git a/drivers/leds/leds-mt6360.c b/drivers/leds/leds-mt6360.c > new file mode 100644 > index 0000000..bd6fa48 > --- /dev/null > +++ b/drivers/leds/leds-mt6360.c > @@ -0,0 +1,681 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// > +// Copyright (C) 2020 MediaTek Inc. > +// > +// Author: Gene Chen <gene_chen@richtek.com> > + > +#include <linux/delay.h> > +#include <linux/init.h> > +#include <linux/interrupt.h> > +#include <linux/kernel.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> > + > +enum { > + MT6360_LED_ISNK1 = 0, > + MT6360_LED_ISNK2, > + MT6360_LED_ISNK3, > + MT6360_LED_ISNK4, > + MT6360_LED_FLASH1, > + MT6360_LED_FLASH2, > + MT6360_MAX_LEDS, > +}; > + > +#define MT6360_REG_RGBEN 0x380 > +#define MT6360_REG_ISNK(_led_no) (0x381 + (_led_no)) > +#define MT6360_ISNK_ENMASK(_led_no) BIT(7 - (_led_no)) > +#define MT6360_ISNK_MASK 0x1F > +#define MT6360_CHRINDSEL_MASK BIT(3) > + > +#define MT6360_REG_FLEDEN 0x37E > +#define MT6360_REG_STRBTO 0x373 > +#define MT6360_REG_FLEDBASE(_id) (0x372 + 4 * (_id - MT6360_LED_FLASH1)) > +#define MT6360_REG_FLEDISTRB(_id) (MT6360_REG_FLEDBASE(_id) + 2) > +#define MT6360_REG_FLEDITOR(_id) (MT6360_REG_FLEDBASE(_id) + 3) > +#define MT6360_REG_CHGSTAT2 0x3E1 > +#define MT6360_REG_FLEDSTAT1 0x3E9 > +#define MT6360_ITORCH_MASK GENMASK(4, 0) > +#define MT6360_ISTROBE_MASK GENMASK(6, 0) > +#define MT6360_STRBTO_MASK GENMASK(6, 0) > +#define MT6360_TORCHEN_MASK BIT(3) > +#define MT6360_STROBEN_MASK BIT(2) > +#define MT6360_FLCSEN_MASK(_id) BIT(MT6360_LED_FLASH2 - _id) > +#define MT6360_FLEDCHGVINOVP_MASK BIT(3) > +#define MT6360_FLED1STRBTO_MASK BIT(11) > +#define MT6360_FLED2STRBTO_MASK BIT(10) > +#define MT6360_FLED1STRB_MASK BIT(9) > +#define MT6360_FLED2STRB_MASK BIT(8) > +#define MT6360_FLED1SHORT_MASK BIT(7) > +#define MT6360_FLED2SHORT_MASK BIT(6) > +#define MT6360_FLEDLVF_MASK BIT(3) > + > +/* 0 means led_off, add one for dummy */ > +#define MT6360_ISNK1_MAXLEVEL 13 > +#define MT6360_ISNK4_MAXLEVEL 31 > + > +#define MT6360_ITORCH_MIN 25000 > +#define MT6360_ITORCH_STEP 12500 > +#define MT6360_ITORCH_MAX 400000 > +#define MT6360_ISTRB_MIN 50000 > +#define MT6360_ISTRB_STEP 12500 > +#define MT6360_ISTRB_MAX 1500000 > +#define MT6360_STRBTO_MIN 64000 > +#define MT6360_STRBTO_STEP 32000 > +#define MT6360_STRBTO_MAX 2432000 > + > +#define FLED_TORCH_FLAG_MASK 0x0c > +#define FLED_TORCH_FLAG_SHFT 2 > +#define FLED_STROBE_FLAG_MASK 0x03 > + > +#define STATE_OFF 0 > +#define STATE_KEEP 1 > +#define STATE_ON 2 > + > +struct mt6360_led { > + union { > + struct led_classdev isnk; > + struct led_classdev_flash flash; > + }; > + struct v4l2_flash *v4l2_flash; > + struct mt6360_priv *priv; > + u32 led_no; > + u32 default_state; > +}; > + > +struct mt6360_priv { > + struct device *dev; > + struct regmap *regmap; > + struct mt6360_led *leds[MT6360_MAX_LEDS]; > + unsigned int fled_strobe_used; > + unsigned int fled_torch_used; > +}; > + > +static int mt6360_isnk_brightness_set(struct led_classdev *lcdev, enum led_brightness level) > +{ > + struct mt6360_led *led = container_of(lcdev, struct mt6360_led, isnk); > + struct mt6360_priv *priv = led->priv; > + u32 enable_mask = MT6360_ISNK_ENMASK(led->led_no); > + u32 val = level ? MT6360_ISNK_ENMASK(led->led_no) : 0; > + int ret; > + > + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); > + > + if (level) { > + ret = regmap_update_bits(priv->regmap, MT6360_REG_ISNK(led->led_no), > + MT6360_ISNK_MASK, level - 1); > + if (ret) > + return ret; > + } > + > + ret = regmap_update_bits(priv->regmap, MT6360_REG_RGBEN, enable_mask, val); > + if (ret) > + return ret; > + > + return 0; > +} > + > +static int mt6360_torch_brightness_set(struct led_classdev *lcdev, enum led_brightness level) > +{ > + struct mt6360_led *led = container_of(lcdev, struct mt6360_led, flash.led_cdev); > + struct mt6360_priv *priv = led->priv; > + u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no); > + u32 val = (level) ? MT6360_FLCSEN_MASK(led->led_no) : 0; > + u32 prev = priv->fled_torch_used, curr; > + int ret; > + > + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); > + if (priv->fled_strobe_used) { > + dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used); Doesn't hardware handle that? IOW, what happens when you have enabled both torch and flash? If flash just overrides torch mode, than you should not prevent enabling torch in this case. > + return -EINVAL; > + } > + > + if (level) > + curr = prev | BIT(led->led_no); > + else > + curr = prev & (~BIT(led->led_no)); > + > + if (curr) > + val |= MT6360_TORCHEN_MASK; > + > + if (level) { > + ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDITOR(led->led_no), > + MT6360_ITORCH_MASK, level - 1); > + if (ret) > + return ret; > + } > + > + ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDEN, enable_mask, val); > + if (ret) > + return ret; > + > + priv->fled_torch_used = curr; > + > + return 0; > +} > + > +static int mt6360_strobe_brightness_set(struct led_classdev_flash *fl_cdev, u32 brightness) s/strobe_brightness_set/flash_brightness_set/ Let's stick to the op name. > +{ > + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); > + struct led_classdev *lcdev = &fl_cdev->led_cdev; It would be good to add comment here explaining why this function doesn't do anything. > + dev_dbg(lcdev->dev, "[%d] strobe brightness %d\n", led->led_no, brightness); > + return 0; > +} > + > +static int _mt6360_strobe_brightness_set(struct led_classdev_flash *fl_cdev, u32 brightness) s/strobe_brightness_set/flash_brightness_set/ > +{ > + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); > + struct mt6360_priv *priv = led->priv; > + struct led_flash_setting *s = &fl_cdev->brightness; > + u32 val = (brightness - s->min) / s->step; > + > + return regmap_update_bits(priv->regmap, MT6360_REG_FLEDISTRB(led->led_no), > + MT6360_ISTROBE_MASK, val); > +} > + > +static int mt6360_strobe_set(struct led_classdev_flash *fl_cdev, bool state) > +{ > + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); > + struct mt6360_priv *priv = led->priv; > + struct led_classdev *lcdev = &fl_cdev->led_cdev; > + struct led_flash_setting *s = &fl_cdev->brightness; > + u32 enable_mask = MT6360_STROBEN_MASK | MT6360_FLCSEN_MASK(led->led_no); > + u32 val = state ? MT6360_FLCSEN_MASK(led->led_no) : 0; > + u32 prev = priv->fled_strobe_used, curr; > + int ret; > + > + dev_dbg(lcdev->dev, "[%d] strobe state %d\n", led->led_no, state); > + if (priv->fled_torch_used) { > + dev_warn(lcdev->dev, "Please disable torch first [0x%x]\n", priv->fled_torch_used); Similar comment as in case of enabling torch mode above. If flash overrides torch then we're OK without this guard. > + return -EINVAL; > + } > + > + if (state) > + curr = prev | BIT(led->led_no); > + else > + curr = prev & (~BIT(led->led_no)); > + > + if (curr) > + val |= MT6360_STROBEN_MASK; > + > + ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDEN, enable_mask, val); > + if (ret) { > + dev_err(lcdev->dev, "[%d] control current source %d fail\n", led->led_no, state); > + return ret; > + } > + > + /* used to prevent flash current spike when torch on */ > + ret = _mt6360_strobe_brightness_set(fl_cdev, state ? s->val : s->min); > + if (ret) > + return ret; > + > + if (!prev && curr) > + usleep_range(5000, 6000); > + else if (prev && !curr) > + udelay(500); > + > + priv->fled_strobe_used = curr; > + return 0; > +} > + > +static int mt6360_strobe_get(struct led_classdev_flash *fl_cdev, bool *state) > +{ > + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); > + struct mt6360_priv *priv = led->priv; > + > + *state = !!(priv->fled_strobe_used & BIT(led->led_no)); > + return 0; > +} > + > +static int mt6360_timeout_set(struct led_classdev_flash *fl_cdev, u32 timeout) > +{ > + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); > + struct mt6360_priv *priv = led->priv; > + struct led_flash_setting *s = &fl_cdev->timeout; > + u32 val = (timeout - s->min) / s->step; > + > + return regmap_update_bits(priv->regmap, MT6360_REG_STRBTO, MT6360_STRBTO_MASK, val); > + > +} > + > +static int mt6360_fault_get(struct led_classdev_flash *fl_cdev, u32 *fault) > +{ > + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); > + struct mt6360_priv *priv = led->priv; > + u16 fled_stat; > + unsigned int chg_stat, strobe_timeout_mask, fled_short_mask; > + u32 rfault = 0; > + int ret; > + > + ret = regmap_read(priv->regmap, MT6360_REG_CHGSTAT2, &chg_stat); > + if (ret) > + return ret; > + > + ret = regmap_raw_read(priv->regmap, MT6360_REG_FLEDSTAT1, &fled_stat, sizeof(fled_stat)); > + if (ret) > + return ret; > + > + if (led->led_no == MT6360_LED_FLASH1) { > + strobe_timeout_mask = MT6360_FLED1STRBTO_MASK; > + fled_short_mask = MT6360_FLED1SHORT_MASK; > + > + } else { > + strobe_timeout_mask = MT6360_FLED2STRBTO_MASK; > + fled_short_mask = MT6360_FLED2SHORT_MASK; > + } > + > + if (chg_stat & MT6360_FLEDCHGVINOVP_MASK) > + rfault |= LED_FAULT_INPUT_VOLTAGE; > + > + if (fled_stat & strobe_timeout_mask) > + rfault |= LED_FAULT_TIMEOUT; > + > + if (fled_stat & fled_short_mask) > + rfault |= LED_FAULT_SHORT_CIRCUIT; > + > + if (fled_stat & MT6360_FLEDLVF_MASK) > + rfault |= LED_FAULT_UNDER_VOLTAGE; > + > + *fault = rfault; > + return 0; > +} > + > +static const struct led_flash_ops mt6360_flash_ops = { > + .flash_brightness_set = mt6360_strobe_brightness_set, > + .strobe_set = mt6360_strobe_set, > + .strobe_get = mt6360_strobe_get, > + .timeout_set = mt6360_timeout_set, > + .fault_get = mt6360_fault_get, > +}; > + > +static int mt6360_isnk_init_default_state(struct mt6360_led *led) > +{ > + struct mt6360_priv *priv = led->priv; > + unsigned int regval; > + u32 level; > + int ret; > + > + ret = regmap_read(priv->regmap, MT6360_REG_ISNK(led->led_no), ®val); > + if (ret) > + return ret; > + level = regval & MT6360_ISNK_MASK; > + > + ret = regmap_read(priv->regmap, MT6360_REG_RGBEN, ®val); > + if (ret) > + return ret; > + > + if (regval & MT6360_ISNK_ENMASK(led->led_no)) > + level += 1; > + else > + level = LED_OFF; > + > + switch (led->default_state) { > + case STATE_ON: > + led->isnk.brightness = led->isnk.max_brightness; > + break; > + case STATE_KEEP: > + led->isnk.brightness = min(level, led->isnk.max_brightness); > + break; > + default: > + led->isnk.brightness = LED_OFF; > + } > + > + return mt6360_isnk_brightness_set(&led->isnk, led->isnk.brightness); > +} > + > +static int mt6360_isnk_register(struct device *parent, struct mt6360_led *led, > + struct led_init_data *init_data) > +{ > + struct mt6360_priv *priv = led->priv; > + int ret; > + > + if (led->led_no == MT6360_LED_ISNK1) { > + /* change isink to sw mode */ > + ret = regmap_update_bits(priv->regmap, MT6360_REG_RGBEN, MT6360_CHRINDSEL_MASK, > + MT6360_CHRINDSEL_MASK); > + if (ret) { > + dev_err(parent, "Failed to config ISNK1 to SW mode\n"); > + return ret; > + } > + } > + > + ret = mt6360_isnk_init_default_state(led); > + if (ret) { > + dev_err(parent, "Failed to init %d isnk state\n", led->led_no); > + return ret; > + } > + > + ret = devm_led_classdev_register_ext(parent, &led->isnk, init_data); > + if (ret) { > + dev_err(parent, "Couldn't register isink %d\n", led->led_no); > + return ret; > + } You probably want also to register V4L2 indicator sub-device. See drivers/leds/leds-as3645a.c and look for the v4l2_flash_indicator_init() for reference. > + return 0; > +} > + > +static int mt6360_flash_init_default_state(struct mt6360_led *led) > +{ > + struct led_classdev_flash *flash = &led->flash; > + struct mt6360_priv *priv = led->priv; > + u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no); > + u32 level; > + unsigned int regval; > + int ret; > + > + ret = regmap_read(priv->regmap, MT6360_REG_FLEDITOR(led->led_no), ®val); > + if (ret) > + return ret; > + level = regval & MT6360_ITORCH_MASK; > + > + ret = regmap_read(priv->regmap, MT6360_REG_FLEDEN, ®val); > + if (ret) > + return ret; > + > + if ((regval & enable_mask) == enable_mask) > + level += 1; > + else > + level = LED_OFF; > + > + switch (led->default_state) { > + case STATE_ON: > + flash->led_cdev.brightness = flash->led_cdev.max_brightness; > + break; > + case STATE_KEEP: > + flash->led_cdev.brightness = min(level, flash->led_cdev.max_brightness); > + break; > + default: > + flash->led_cdev.brightness = LED_OFF; > + } > + > + return mt6360_torch_brightness_set(&flash->led_cdev, flash->led_cdev.brightness); > +} > + > +#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS) > +static int mt6360_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable) > +{ > + struct led_classdev_flash *flash = v4l2_flash->fled_cdev; > + struct mt6360_led *led = container_of(flash, struct mt6360_led, flash); > + struct mt6360_priv *priv = led->priv; > + u32 enable_mask = MT6360_FLCSEN_MASK(led->led_no); > + int ret; > + > + ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDEN, enable_mask, > + enable ? enable_mask : 0); > + if (ret) > + return ret; > + > + if (enable) > + priv->fled_strobe_used |= BIT(led->led_no); > + else > + priv->fled_strobe_used &= (~BIT(led->led_no)); > + > + return 0; > +} > + > +static const struct v4l2_flash_ops v4l2_flash_ops = { > + .external_strobe_set = mt6360_flash_external_strobe_set, > +}; > + > +static void mt6360_flash_init_v4l2_config(struct mt6360_led *led, struct v4l2_flash_config *config) > +{ > + struct led_classdev_flash *flash = &led->flash; > + struct led_classdev *lcdev = &flash->led_cdev; > + struct led_flash_setting *s = &config->intensity; > + > + snprintf(config->dev_name, sizeof(config->dev_name), "%s", lcdev->name); > + > + s->min = MT6360_ITORCH_MIN; > + s->step = MT6360_ITORCH_STEP; > + s->val = s->max = (s->min) + (lcdev->max_brightness - 1) * s->step; > + > + config->has_external_strobe = 1; > +} > +#else > +static const struct v4l2_flash_ops v4l2_flash_ops; > + > +static void mt6360_flash_init_v4l2_config(struct mt6360_led *led, struct v4l2_flash_config *config) > +{ > +} > +#endif > + > +static int mt6360_flash_register(struct device *parent, struct mt6360_led *led, > + struct led_init_data *init_data) > +{ > + struct v4l2_flash_config v4l2_config = {0}; > + int ret; > + > + ret = mt6360_flash_init_default_state(led); > + if (ret) { > + dev_err(parent, "Failed to init %d flash state\n", led->led_no); > + return ret; > + } > + > + ret = devm_led_classdev_flash_register_ext(parent, &led->flash, init_data); > + if (ret) { > + dev_err(parent, "Couldn't register flash %d\n", led->led_no); > + return ret; > + } > + > + mt6360_flash_init_v4l2_config(led, &v4l2_config); > + led->v4l2_flash = v4l2_flash_init(parent, init_data->fwnode, &led->flash, &v4l2_flash_ops, > + &v4l2_config); > + if (IS_ERR(led->v4l2_flash)) { > + dev_err(parent, "Failed to register %d v4l2 sd\n", led->led_no); > + return PTR_ERR(led->v4l2_flash); > + } > + > + return 0; > +} > + > +static int mt6360_init_isnk_properties(struct mt6360_led *led, struct led_init_data *init_data) > +{ > + struct led_classdev *isnk = &led->isnk; > + > + if (led->led_no == MT6360_LED_ISNK4) > + isnk->max_brightness = MT6360_ISNK4_MAXLEVEL; > + else > + isnk->max_brightness = MT6360_ISNK1_MAXLEVEL; > + > + isnk->brightness_set_blocking = mt6360_isnk_brightness_set; > + > + fwnode_property_read_string(init_data->fwnode, "linux,default-trigger", > + &isnk->default_trigger); > + > + return 0; > +} > + > +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 mt6360_init_flash_properties(struct mt6360_led *led, struct led_init_data *init_data) > +{ > + struct led_classdev_flash *flash = &led->flash; > + struct led_classdev *lcdev = &flash->led_cdev; > + struct led_flash_setting *s; > + u32 val; > + int ret; > + > + ret = fwnode_property_read_u32(init_data->fwnode, "led-max-microamp", &val); > + if (ret) > + val = MT6360_ITORCH_MIN; In case of missing property I would print warning and inform the user that we're defaulting to min value. Same applies to similar occurrences below. > + else > + clamp_align(&val, MT6360_ITORCH_MIN, MT6360_ITORCH_MAX, MT6360_ITORCH_STEP); > + > + lcdev->max_brightness = (val - MT6360_ITORCH_MIN) / MT6360_ITORCH_STEP + 1; > + lcdev->brightness_set_blocking = mt6360_torch_brightness_set; > + lcdev->flags |= LED_DEV_CAP_FLASH; > + > + ret = fwnode_property_read_u32(init_data->fwnode, "flash-max-microamp", &val); > + if (ret) > + val = MT6360_ISTRB_MIN; > + else > + clamp_align(&val, MT6360_ISTRB_MIN, MT6360_ISTRB_MAX, MT6360_ISTRB_STEP); > + > + s = &flash->brightness; > + s->min = MT6360_ISTRB_MIN; > + s->step = MT6360_ISTRB_STEP; > + s->val = s->max = val; > + > + /* always configure as min level when off to prevent flash current spike */ > + ret = _mt6360_strobe_brightness_set(flash, s->min); > + if (ret) > + return ret; > + > + ret = fwnode_property_read_u32(init_data->fwnode, "flash-max-timeout-us", &val); > + if (ret) > + val = MT6360_STRBTO_MIN; > + else > + clamp_align(&val, MT6360_STRBTO_MIN, MT6360_STRBTO_MAX, MT6360_STRBTO_STEP); > + > + s = &flash->timeout; > + s->min = MT6360_STRBTO_MIN; > + s->step = MT6360_STRBTO_STEP; > + s->val = s->max = val; > + > + flash->ops = &mt6360_flash_ops; > + > + return 0; > +} > + > +static int mt6360_init_common_properties(struct mt6360_led *led, struct led_init_data *init_data) > +{ > + const char *str; > + > + if (!fwnode_property_read_string(init_data->fwnode, "default-state", &str)) { > + if (!strcmp(str, "on")) > + led->default_state = STATE_ON; > + else if (!strcmp(str, "keep")) > + led->default_state = STATE_KEEP; > + else > + led->default_state = STATE_OFF; > + } > + > + return 0; > +} > + > +static int mt6360_led_probe(struct platform_device *pdev) > +{ > + struct mt6360_priv *priv; > + struct fwnode_handle *child; > + int i, ret; > + > + ret = device_get_child_node_count(&pdev->dev); > + if (!ret || ret > MT6360_MAX_LEDS) > + return -EINVAL; > + > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->dev = &pdev->dev; > + > + priv->regmap = dev_get_regmap(pdev->dev.parent, NULL); > + if (!priv->regmap) { > + dev_err(&pdev->dev, "Failed to get parent regmap\n"); > + return -ENODEV; > + } > + > + device_for_each_child_node(&pdev->dev, child) { > + struct mt6360_led *led; > + struct led_init_data init_data = { .fwnode = child, }; > + u32 reg; > + > + ret = fwnode_property_read_u32(child, "reg", ®); > + if (ret || reg >= MT6360_MAX_LEDS || priv->leds[reg]) { > + ret = -EINVAL; > + goto out; > + } > + > + led = devm_kzalloc(&pdev->dev, sizeof(*led), GFP_KERNEL); > + if (!led) { > + ret = -ENOMEM; > + goto out; > + } > + > + led->led_no = reg; > + led->priv = priv; > + > + ret = mt6360_init_common_properties(led, &init_data); > + if (ret) > + goto out; > + > + switch (reg) { > + case MT6360_LED_ISNK1 ... MT6360_LED_ISNK4: > + ret = mt6360_init_isnk_properties(led, &init_data); > + if (ret) > + goto out; > + > + ret = mt6360_isnk_register(&pdev->dev, led, &init_data); > + break; > + default: > + ret = mt6360_init_flash_properties(led, &init_data); > + if (ret) > + goto out; > + > + ret = mt6360_flash_register(&pdev->dev, led, &init_data); > + } > + > + if (ret) > + goto out; > + > + priv->leds[reg] = led; > + } > + > + platform_set_drvdata(pdev, priv); > + return 0; > +out: > + for (i = MT6360_LED_FLASH1; i <= MT6360_LED_FLASH2; i++) { > + struct mt6360_led *led = priv->leds[i]; > + > + if (led && led->v4l2_flash) > + v4l2_flash_release(led->v4l2_flash); > + > + } > + > + return ret; > +} > + > +static int mt6360_led_remove(struct platform_device *pdev) > +{ > + struct mt6360_priv *priv = platform_get_drvdata(pdev); > + int i; > + > + for (i = MT6360_LED_FLASH1; i <= MT6360_LED_FLASH2; i++) { > + struct mt6360_led *led = priv->leds[i]; > + > + if (led && led->v4l2_flash) > + v4l2_flash_release(led->v4l2_flash); > + > + } You're using this for loop twice, so it would be nice to wrap it with a function to avoid code redundancy. > + > + return 0; > +} > + > +static const struct of_device_id __maybe_unused mt6360_led_of_id[] = { > + { .compatible = "mediatek,mt6360-led", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, mt6360_led_of_id); > + > +static struct platform_driver mt6360_led_driver = { > + .driver = { > + .name = "mt6360-led", > + .of_match_table = mt6360_led_of_id, > + }, > + .probe = mt6360_led_probe, > + .remove = mt6360_led_remove, > +}; > +module_platform_driver(mt6360_led_driver); > + > +MODULE_AUTHOR("Gene Chen <gene_chen@richtek.com>"); > +MODULE_DESCRIPTION("MT6360 Led Driver"); > +MODULE_LICENSE("GPL v2"); >
Pavel Machek <pavel@ucw.cz> 於 2020年9月11日 週五 下午3:05寫道： > > Hi! > > > >+{ > > >+ struct mt6360_led *led = container_of(lcdev, struct mt6360_led, flash.led_cdev); > > >+ struct mt6360_priv *priv = led->priv; > > >+ u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no); > > >+ u32 val = (level) ? MT6360_FLCSEN_MASK(led->led_no) : 0; > > >+ u32 prev = priv->fled_torch_used, curr; > > >+ int ret; > > >+ > > >+ dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); > > >+ if (priv->fled_strobe_used) { > > >+ dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used); > > > > Doesn't hardware handle that? IOW, what happens when you have enabled > > both torch and flash? If flash just overrides torch mode, than you > > should not prevent enabling torch in this case. > > Yep, this is strange/confusing... and was reason why I asked for not > supporting strobe from sysfs. > > Could I get you to remove code you are not commenting at when > reviewing? > MT6360 FLED register define is STROBE_EN/TORCH_EN/CS1/CS2 (current source) 4 bits. The STROBE_EN/TORCH_EN is shared by FLED1 and FLED2. If I want to enable FLED1 torch mode, I set TORCH_EN and CS1 If I want to enable FLED2 strobe mode, I set STROBE_EN and CS2 For example I set FLED1 torch, then I set FLED2 strobe. When I set FLED2 strobe, I will see the strobe current is FLED2 add FLED1 current which is not I want. So I need disable FLED1 torch first. Considering every circumstances is complicated when share same H/W logic control. And the other problem is torch mode switch to strobe mode needs ramp time because strobe and torch mode can't be co-exist. > > >+MODULE_AUTHOR("Gene Chen <gene_chen@richtek.com>"); > > >+MODULE_DESCRIPTION("MT6360 Led Driver"); > > Led -> LED. > ACK > Pavel > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi! > >+{ > >+ struct mt6360_led *led = container_of(lcdev, struct mt6360_led, flash.led_cdev); > >+ struct mt6360_priv *priv = led->priv; > >+ u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no); > >+ u32 val = (level) ? MT6360_FLCSEN_MASK(led->led_no) : 0; > >+ u32 prev = priv->fled_torch_used, curr; > >+ int ret; > >+ > >+ dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); > >+ if (priv->fled_strobe_used) { > >+ dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used); > > Doesn't hardware handle that? IOW, what happens when you have enabled > both torch and flash? If flash just overrides torch mode, than you > should not prevent enabling torch in this case. Yep, this is strange/confusing... and was reason why I asked for not supporting strobe from sysfs. Could I get you to remove code you are not commenting at when reviewing? > >+MODULE_AUTHOR("Gene Chen <gene_chen@richtek.com>"); > >+MODULE_DESCRIPTION("MT6360 Led Driver"); Led -> LED. Pavel
Hi Pavel, On 9/11/20 9:05 AM, Pavel Machek wrote: > Hi! > >>> +{ >>> + struct mt6360_led *led = container_of(lcdev, struct mt6360_led, flash.led_cdev); >>> + struct mt6360_priv *priv = led->priv; >>> + u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no); >>> + u32 val = (level) ? MT6360_FLCSEN_MASK(led->led_no) : 0; >>> + u32 prev = priv->fled_torch_used, curr; >>> + int ret; >>> + >>> + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); >>> + if (priv->fled_strobe_used) { >>> + dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used); >> >> Doesn't hardware handle that? IOW, what happens when you have enabled >> both torch and flash? If flash just overrides torch mode, than you >> should not prevent enabling torch in this case. > > Yep, this is strange/confusing... and was reason why I asked for not > supporting strobe from sysfs. What you say now is even more confusing when we look at your ack under this patch: commit 7aea8389a77abf9fde254aca2434a605c7704f58 Author: Jacek Anaszewski <j.anaszewski@samsung.com> Date: Fri Jan 9 07:22:51 2015 -0800 leds: Add LED Flash class extension to the LED subsystem Some LED devices support two operation modes - torch and flash. This patch provides support for flash LED devices in the LED subsystem by introducing new sysfs attributes and kernel internal interface. The attributes being introduced are: flash_brightness, flash_strobe, flash_timeout, max_flash_timeout, max_flash_brightness, flash_fault, flash_sync_strobe and available_sync_leds. All the flash related features are placed in a separate module. The modifications aim to be compatible with V4L2 framework requirements related to the flash devices management. The design assumes that V4L2 sub-device can take of the LED class device control and communicate with it through the kernel internal interface. When V4L2 Flash sub-device file is opened, the LED class device sysfs interface is made unavailable. Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com> Acked-by: Kyungmin Park <kyungmin.park@samsung.com> Cc: Richard Purdie <rpurdie@rpsys.net> Acked-by: Pavel Machek <pavel@ucw.cz> Signed-off-by: Bryan Wu <cooloney@gmail.com> > Could I get you to remove code you are not commenting at when > reviewing? > >>> +MODULE_AUTHOR("Gene Chen <gene_chen@richtek.com>"); >>> +MODULE_DESCRIPTION("MT6360 Led Driver"); > > Led -> LED. > > Pavel >
On 9/11/20 1:24 AM, Gene Chen wrote: > Pavel Machek <pavel@ucw.cz> 於 2020年9月11日 週五 下午3:05寫道： >> >> Hi! >> >>>> +{ >>>> + struct mt6360_led *led = container_of(lcdev, struct mt6360_led, flash.led_cdev); >>>> + struct mt6360_priv *priv = led->priv; >>>> + u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no); >>>> + u32 val = (level) ? MT6360_FLCSEN_MASK(led->led_no) : 0; >>>> + u32 prev = priv->fled_torch_used, curr; >>>> + int ret; >>>> + >>>> + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); >>>> + if (priv->fled_strobe_used) { >>>> + dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used); >>> >>> Doesn't hardware handle that? IOW, what happens when you have enabled >>> both torch and flash? If flash just overrides torch mode, than you >>> should not prevent enabling torch in this case. >> >> Yep, this is strange/confusing... and was reason why I asked for not >> supporting strobe from sysfs. >> >> Could I get you to remove code you are not commenting at when >> reviewing? >> > > MT6360 FLED register define is STROBE_EN/TORCH_EN/CS1/CS2 (current > source) 4 bits. > The STROBE_EN/TORCH_EN is shared by FLED1 and FLED2. > If I want to enable FLED1 torch mode, I set TORCH_EN and CS1 > If I want to enable FLED2 strobe mode, I set STROBE_EN and CS2 > For example I set FLED1 torch, then I set FLED2 strobe. > When I set FLED2 strobe, I will see the strobe current is FLED2 add > FLED1 current which is not I want. > So I need disable FLED1 torch first. > Considering every circumstances is complicated when share same H/W > logic control. > And the other problem is torch mode switch to strobe mode needs ramp > time because strobe and torch mode can't be co-exist. Thank you for the explanation. So we have to keep your guards but I would return -EBUSY instead of -EINVAL. This would be also consistent with what drivers/media/v4l2-core/v4l2-flash-led-class.c does in its v4l2_flash_s_ctrl(), case V4L2_CID_FLASH_STROBE - it returns -EBUSY if __software_strobe_mode_inactive() returns false. The advantage of V4L2 Flash interface is that it has LED_MODE that can be set to torch or flash, but in LED subsystem we don't have the counterpart.
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 1c181df..94a6d83 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -271,6 +271,17 @@ config LEDS_MT6323 This option enables support for on-chip LED drivers found on Mediatek MT6323 PMIC. +config LEDS_MT6360 + tristate "LED Support for Mediatek MT6360 PMIC" + depends on LEDS_CLASS_FLASH && OF + depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS + depends on MFD_MT6360 + help + This option enables support for dual Flash LED drivers found on + Mediatek MT6360 PMIC. + Independent current sources supply for each flash LED support torch and strobe mode. + Includes Low-VF and short protection. + config LEDS_S3C24XX tristate "LED Support for Samsung S3C24XX GPIO LEDs" depends on LEDS_CLASS diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index c2c7d7a..5596427 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -66,6 +66,7 @@ obj-$(CONFIG_LEDS_MIKROTIK_RB532) += leds-rb532.o obj-$(CONFIG_LEDS_MLXCPLD) += leds-mlxcpld.o obj-$(CONFIG_LEDS_MLXREG) += leds-mlxreg.o obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o +obj-$(CONFIG_LEDS_MT6360) += leds-mt6360.o obj-$(CONFIG_LEDS_NET48XX) += leds-net48xx.o obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o diff --git a/drivers/leds/leds-mt6360.c b/drivers/leds/leds-mt6360.c new file mode 100644 index 0000000..bd6fa48 --- /dev/null +++ b/drivers/leds/leds-mt6360.c @@ -0,0 +1,681 @@ +// SPDX-License-Identifier: GPL-2.0-only +// +// Copyright (C) 2020 MediaTek Inc. +// +// Author: Gene Chen <gene_chen@richtek.com> + +#include <linux/delay.h> +#include <linux/init.h> +#include <linux/interrupt.h> +#include <linux/kernel.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> + +enum { + MT6360_LED_ISNK1 = 0, + MT6360_LED_ISNK2, + MT6360_LED_ISNK3, + MT6360_LED_ISNK4, + MT6360_LED_FLASH1, + MT6360_LED_FLASH2, + MT6360_MAX_LEDS, +}; + +#define MT6360_REG_RGBEN 0x380 +#define MT6360_REG_ISNK(_led_no) (0x381 + (_led_no)) +#define MT6360_ISNK_ENMASK(_led_no) BIT(7 - (_led_no)) +#define MT6360_ISNK_MASK 0x1F +#define MT6360_CHRINDSEL_MASK BIT(3) + +#define MT6360_REG_FLEDEN 0x37E +#define MT6360_REG_STRBTO 0x373 +#define MT6360_REG_FLEDBASE(_id) (0x372 + 4 * (_id - MT6360_LED_FLASH1)) +#define MT6360_REG_FLEDISTRB(_id) (MT6360_REG_FLEDBASE(_id) + 2) +#define MT6360_REG_FLEDITOR(_id) (MT6360_REG_FLEDBASE(_id) + 3) +#define MT6360_REG_CHGSTAT2 0x3E1 +#define MT6360_REG_FLEDSTAT1 0x3E9 +#define MT6360_ITORCH_MASK GENMASK(4, 0) +#define MT6360_ISTROBE_MASK GENMASK(6, 0) +#define MT6360_STRBTO_MASK GENMASK(6, 0) +#define MT6360_TORCHEN_MASK BIT(3) +#define MT6360_STROBEN_MASK BIT(2) +#define MT6360_FLCSEN_MASK(_id) BIT(MT6360_LED_FLASH2 - _id) +#define MT6360_FLEDCHGVINOVP_MASK BIT(3) +#define MT6360_FLED1STRBTO_MASK BIT(11) +#define MT6360_FLED2STRBTO_MASK BIT(10) +#define MT6360_FLED1STRB_MASK BIT(9) +#define MT6360_FLED2STRB_MASK BIT(8) +#define MT6360_FLED1SHORT_MASK BIT(7) +#define MT6360_FLED2SHORT_MASK BIT(6) +#define MT6360_FLEDLVF_MASK BIT(3) + +/* 0 means led_off, add one for dummy */ +#define MT6360_ISNK1_MAXLEVEL 13 +#define MT6360_ISNK4_MAXLEVEL 31 + +#define MT6360_ITORCH_MIN 25000 +#define MT6360_ITORCH_STEP 12500 +#define MT6360_ITORCH_MAX 400000 +#define MT6360_ISTRB_MIN 50000 +#define MT6360_ISTRB_STEP 12500 +#define MT6360_ISTRB_MAX 1500000 +#define MT6360_STRBTO_MIN 64000 +#define MT6360_STRBTO_STEP 32000 +#define MT6360_STRBTO_MAX 2432000 + +#define FLED_TORCH_FLAG_MASK 0x0c +#define FLED_TORCH_FLAG_SHFT 2 +#define FLED_STROBE_FLAG_MASK 0x03 + +#define STATE_OFF 0 +#define STATE_KEEP 1 +#define STATE_ON 2 + +struct mt6360_led { + union { + struct led_classdev isnk; + struct led_classdev_flash flash; + }; + struct v4l2_flash *v4l2_flash; + struct mt6360_priv *priv; + u32 led_no; + u32 default_state; +}; + +struct mt6360_priv { + struct device *dev; + struct regmap *regmap; + struct mt6360_led *leds[MT6360_MAX_LEDS]; + unsigned int fled_strobe_used; + unsigned int fled_torch_used; +}; + +static int mt6360_isnk_brightness_set(struct led_classdev *lcdev, enum led_brightness level) +{ + struct mt6360_led *led = container_of(lcdev, struct mt6360_led, isnk); + struct mt6360_priv *priv = led->priv; + u32 enable_mask = MT6360_ISNK_ENMASK(led->led_no); + u32 val = level ? MT6360_ISNK_ENMASK(led->led_no) : 0; + int ret; + + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); + + if (level) { + ret = regmap_update_bits(priv->regmap, MT6360_REG_ISNK(led->led_no), + MT6360_ISNK_MASK, level - 1); + if (ret) + return ret; + } + + ret = regmap_update_bits(priv->regmap, MT6360_REG_RGBEN, enable_mask, val); + if (ret) + return ret; + + return 0; +} + +static int mt6360_torch_brightness_set(struct led_classdev *lcdev, enum led_brightness level) +{ + struct mt6360_led *led = container_of(lcdev, struct mt6360_led, flash.led_cdev); + struct mt6360_priv *priv = led->priv; + u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no); + u32 val = (level) ? MT6360_FLCSEN_MASK(led->led_no) : 0; + u32 prev = priv->fled_torch_used, curr; + int ret; + + dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level); + if (priv->fled_strobe_used) { + dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used); + return -EINVAL; + } + + if (level) + curr = prev | BIT(led->led_no); + else + curr = prev & (~BIT(led->led_no)); + + if (curr) + val |= MT6360_TORCHEN_MASK; + + if (level) { + ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDITOR(led->led_no), + MT6360_ITORCH_MASK, level - 1); + if (ret) + return ret; + } + + ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDEN, enable_mask, val); + if (ret) + return ret; + + priv->fled_torch_used = curr; + + return 0; +} + +static int mt6360_strobe_brightness_set(struct led_classdev_flash *fl_cdev, u32 brightness) +{ + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); + struct led_classdev *lcdev = &fl_cdev->led_cdev; + + dev_dbg(lcdev->dev, "[%d] strobe brightness %d\n", led->led_no, brightness); + return 0; +} + +static int _mt6360_strobe_brightness_set(struct led_classdev_flash *fl_cdev, u32 brightness) +{ + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); + struct mt6360_priv *priv = led->priv; + struct led_flash_setting *s = &fl_cdev->brightness; + u32 val = (brightness - s->min) / s->step; + + return regmap_update_bits(priv->regmap, MT6360_REG_FLEDISTRB(led->led_no), + MT6360_ISTROBE_MASK, val); +} + +static int mt6360_strobe_set(struct led_classdev_flash *fl_cdev, bool state) +{ + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); + struct mt6360_priv *priv = led->priv; + struct led_classdev *lcdev = &fl_cdev->led_cdev; + struct led_flash_setting *s = &fl_cdev->brightness; + u32 enable_mask = MT6360_STROBEN_MASK | MT6360_FLCSEN_MASK(led->led_no); + u32 val = state ? MT6360_FLCSEN_MASK(led->led_no) : 0; + u32 prev = priv->fled_strobe_used, curr; + int ret; + + dev_dbg(lcdev->dev, "[%d] strobe state %d\n", led->led_no, state); + if (priv->fled_torch_used) { + dev_warn(lcdev->dev, "Please disable torch first [0x%x]\n", priv->fled_torch_used); + return -EINVAL; + } + + if (state) + curr = prev | BIT(led->led_no); + else + curr = prev & (~BIT(led->led_no)); + + if (curr) + val |= MT6360_STROBEN_MASK; + + ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDEN, enable_mask, val); + if (ret) { + dev_err(lcdev->dev, "[%d] control current source %d fail\n", led->led_no, state); + return ret; + } + + /* used to prevent flash current spike when torch on */ + ret = _mt6360_strobe_brightness_set(fl_cdev, state ? s->val : s->min); + if (ret) + return ret; + + if (!prev && curr) + usleep_range(5000, 6000); + else if (prev && !curr) + udelay(500); + + priv->fled_strobe_used = curr; + return 0; +} + +static int mt6360_strobe_get(struct led_classdev_flash *fl_cdev, bool *state) +{ + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); + struct mt6360_priv *priv = led->priv; + + *state = !!(priv->fled_strobe_used & BIT(led->led_no)); + return 0; +} + +static int mt6360_timeout_set(struct led_classdev_flash *fl_cdev, u32 timeout) +{ + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); + struct mt6360_priv *priv = led->priv; + struct led_flash_setting *s = &fl_cdev->timeout; + u32 val = (timeout - s->min) / s->step; + + return regmap_update_bits(priv->regmap, MT6360_REG_STRBTO, MT6360_STRBTO_MASK, val); + +} + +static int mt6360_fault_get(struct led_classdev_flash *fl_cdev, u32 *fault) +{ + struct mt6360_led *led = container_of(fl_cdev, struct mt6360_led, flash); + struct mt6360_priv *priv = led->priv; + u16 fled_stat; + unsigned int chg_stat, strobe_timeout_mask, fled_short_mask; + u32 rfault = 0; + int ret; + + ret = regmap_read(priv->regmap, MT6360_REG_CHGSTAT2, &chg_stat); + if (ret) + return ret; + + ret = regmap_raw_read(priv->regmap, MT6360_REG_FLEDSTAT1, &fled_stat, sizeof(fled_stat)); + if (ret) + return ret; + + if (led->led_no == MT6360_LED_FLASH1) { + strobe_timeout_mask = MT6360_FLED1STRBTO_MASK; + fled_short_mask = MT6360_FLED1SHORT_MASK; + + } else { + strobe_timeout_mask = MT6360_FLED2STRBTO_MASK; + fled_short_mask = MT6360_FLED2SHORT_MASK; + } + + if (chg_stat & MT6360_FLEDCHGVINOVP_MASK) + rfault |= LED_FAULT_INPUT_VOLTAGE; + + if (fled_stat & strobe_timeout_mask) + rfault |= LED_FAULT_TIMEOUT; + + if (fled_stat & fled_short_mask) + rfault |= LED_FAULT_SHORT_CIRCUIT; + + if (fled_stat & MT6360_FLEDLVF_MASK) + rfault |= LED_FAULT_UNDER_VOLTAGE; + + *fault = rfault; + return 0; +} + +static const struct led_flash_ops mt6360_flash_ops = { + .flash_brightness_set = mt6360_strobe_brightness_set, + .strobe_set = mt6360_strobe_set, + .strobe_get = mt6360_strobe_get, + .timeout_set = mt6360_timeout_set, + .fault_get = mt6360_fault_get, +}; + +static int mt6360_isnk_init_default_state(struct mt6360_led *led) +{ + struct mt6360_priv *priv = led->priv; + unsigned int regval; + u32 level; + int ret; + + ret = regmap_read(priv->regmap, MT6360_REG_ISNK(led->led_no), ®val); + if (ret) + return ret; + level = regval & MT6360_ISNK_MASK; + + ret = regmap_read(priv->regmap, MT6360_REG_RGBEN, ®val); + if (ret) + return ret; + + if (regval & MT6360_ISNK_ENMASK(led->led_no)) + level += 1; + else + level = LED_OFF; + + switch (led->default_state) { + case STATE_ON: + led->isnk.brightness = led->isnk.max_brightness; + break; + case STATE_KEEP: + led->isnk.brightness = min(level, led->isnk.max_brightness); + break; + default: + led->isnk.brightness = LED_OFF; + } + + return mt6360_isnk_brightness_set(&led->isnk, led->isnk.brightness); +} + +static int mt6360_isnk_register(struct device *parent, struct mt6360_led *led, + struct led_init_data *init_data) +{ + struct mt6360_priv *priv = led->priv; + int ret; + + if (led->led_no == MT6360_LED_ISNK1) { + /* change isink to sw mode */ + ret = regmap_update_bits(priv->regmap, MT6360_REG_RGBEN, MT6360_CHRINDSEL_MASK, + MT6360_CHRINDSEL_MASK); + if (ret) { + dev_err(parent, "Failed to config ISNK1 to SW mode\n"); + return ret; + } + } + + ret = mt6360_isnk_init_default_state(led); + if (ret) { + dev_err(parent, "Failed to init %d isnk state\n", led->led_no); + return ret; + } + + ret = devm_led_classdev_register_ext(parent, &led->isnk, init_data); + if (ret) { + dev_err(parent, "Couldn't register isink %d\n", led->led_no); + return ret; + } + + return 0; +} + +static int mt6360_flash_init_default_state(struct mt6360_led *led) +{ + struct led_classdev_flash *flash = &led->flash; + struct mt6360_priv *priv = led->priv; + u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no); + u32 level; + unsigned int regval; + int ret; + + ret = regmap_read(priv->regmap, MT6360_REG_FLEDITOR(led->led_no), ®val); + if (ret) + return ret; + level = regval & MT6360_ITORCH_MASK; + + ret = regmap_read(priv->regmap, MT6360_REG_FLEDEN, ®val); + if (ret) + return ret; + + if ((regval & enable_mask) == enable_mask) + level += 1; + else + level = LED_OFF; + + switch (led->default_state) { + case STATE_ON: + flash->led_cdev.brightness = flash->led_cdev.max_brightness; + break; + case STATE_KEEP: + flash->led_cdev.brightness = min(level, flash->led_cdev.max_brightness); + break; + default: + flash->led_cdev.brightness = LED_OFF; + } + + return mt6360_torch_brightness_set(&flash->led_cdev, flash->led_cdev.brightness); +} + +#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS) +static int mt6360_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable) +{ + struct led_classdev_flash *flash = v4l2_flash->fled_cdev; + struct mt6360_led *led = container_of(flash, struct mt6360_led, flash); + struct mt6360_priv *priv = led->priv; + u32 enable_mask = MT6360_FLCSEN_MASK(led->led_no); + int ret; + + ret = regmap_update_bits(priv->regmap, MT6360_REG_FLEDEN, enable_mask, + enable ? enable_mask : 0); + if (ret) + return ret; + + if (enable) + priv->fled_strobe_used |= BIT(led->led_no); + else + priv->fled_strobe_used &= (~BIT(led->led_no)); + + return 0; +} + +static const struct v4l2_flash_ops v4l2_flash_ops = { + .external_strobe_set = mt6360_flash_external_strobe_set, +}; + +static void mt6360_flash_init_v4l2_config(struct mt6360_led *led, struct v4l2_flash_config *config) +{ + struct led_classdev_flash *flash = &led->flash; + struct led_classdev *lcdev = &flash->led_cdev; + struct led_flash_setting *s = &config->intensity; + + snprintf(config->dev_name, sizeof(config->dev_name), "%s", lcdev->name); + + s->min = MT6360_ITORCH_MIN; + s->step = MT6360_ITORCH_STEP; + s->val = s->max = (s->min) + (lcdev->max_brightness - 1) * s->step; + + config->has_external_strobe = 1; +} +#else +static const struct v4l2_flash_ops v4l2_flash_ops; + +static void mt6360_flash_init_v4l2_config(struct mt6360_led *led, struct v4l2_flash_config *config) +{ +} +#endif + +static int mt6360_flash_register(struct device *parent, struct mt6360_led *led, + struct led_init_data *init_data) +{ + struct v4l2_flash_config v4l2_config = {0}; + int ret; + + ret = mt6360_flash_init_default_state(led); + if (ret) { + dev_err(parent, "Failed to init %d flash state\n", led->led_no); + return ret; + } + + ret = devm_led_classdev_flash_register_ext(parent, &led->flash, init_data); + if (ret) { + dev_err(parent, "Couldn't register flash %d\n", led->led_no); + return ret; + } + + mt6360_flash_init_v4l2_config(led, &v4l2_config); + led->v4l2_flash = v4l2_flash_init(parent, init_data->fwnode, &led->flash, &v4l2_flash_ops, + &v4l2_config); + if (IS_ERR(led->v4l2_flash)) { + dev_err(parent, "Failed to register %d v4l2 sd\n", led->led_no); + return PTR_ERR(led->v4l2_flash); + } + + return 0; +} + +static int mt6360_init_isnk_properties(struct mt6360_led *led, struct led_init_data *init_data) +{ + struct led_classdev *isnk = &led->isnk; + + if (led->led_no == MT6360_LED_ISNK4) + isnk->max_brightness = MT6360_ISNK4_MAXLEVEL; + else + isnk->max_brightness = MT6360_ISNK1_MAXLEVEL; + + isnk->brightness_set_blocking = mt6360_isnk_brightness_set; + + fwnode_property_read_string(init_data->fwnode, "linux,default-trigger", + &isnk->default_trigger); + + return 0; +} + +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 mt6360_init_flash_properties(struct mt6360_led *led, struct led_init_data *init_data) +{ + struct led_classdev_flash *flash = &led->flash; + struct led_classdev *lcdev = &flash->led_cdev; + struct led_flash_setting *s; + u32 val; + int ret; + + ret = fwnode_property_read_u32(init_data->fwnode, "led-max-microamp", &val); + if (ret) + val = MT6360_ITORCH_MIN; + else + clamp_align(&val, MT6360_ITORCH_MIN, MT6360_ITORCH_MAX, MT6360_ITORCH_STEP); + + lcdev->max_brightness = (val - MT6360_ITORCH_MIN) / MT6360_ITORCH_STEP + 1; + lcdev->brightness_set_blocking = mt6360_torch_brightness_set; + lcdev->flags |= LED_DEV_CAP_FLASH; + + ret = fwnode_property_read_u32(init_data->fwnode, "flash-max-microamp", &val); + if (ret) + val = MT6360_ISTRB_MIN; + else + clamp_align(&val, MT6360_ISTRB_MIN, MT6360_ISTRB_MAX, MT6360_ISTRB_STEP); + + s = &flash->brightness; + s->min = MT6360_ISTRB_MIN; + s->step = MT6360_ISTRB_STEP; + s->val = s->max = val; + + /* always configure as min level when off to prevent flash current spike */ + ret = _mt6360_strobe_brightness_set(flash, s->min); + if (ret) + return ret; + + ret = fwnode_property_read_u32(init_data->fwnode, "flash-max-timeout-us", &val); + if (ret) + val = MT6360_STRBTO_MIN; + else + clamp_align(&val, MT6360_STRBTO_MIN, MT6360_STRBTO_MAX, MT6360_STRBTO_STEP); + + s = &flash->timeout; + s->min = MT6360_STRBTO_MIN; + s->step = MT6360_STRBTO_STEP; + s->val = s->max = val; + + flash->ops = &mt6360_flash_ops; + + return 0; +} + +static int mt6360_init_common_properties(struct mt6360_led *led, struct led_init_data *init_data) +{ + const char *str; + + if (!fwnode_property_read_string(init_data->fwnode, "default-state", &str)) { + if (!strcmp(str, "on")) + led->default_state = STATE_ON; + else if (!strcmp(str, "keep")) + led->default_state = STATE_KEEP; + else + led->default_state = STATE_OFF; + } + + return 0; +} + +static int mt6360_led_probe(struct platform_device *pdev) +{ + struct mt6360_priv *priv; + struct fwnode_handle *child; + int i, ret; + + ret = device_get_child_node_count(&pdev->dev); + if (!ret || ret > MT6360_MAX_LEDS) + return -EINVAL; + + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->dev = &pdev->dev; + + priv->regmap = dev_get_regmap(pdev->dev.parent, NULL); + if (!priv->regmap) { + dev_err(&pdev->dev, "Failed to get parent regmap\n"); + return -ENODEV; + } + + device_for_each_child_node(&pdev->dev, child) { + struct mt6360_led *led; + struct led_init_data init_data = { .fwnode = child, }; + u32 reg; + + ret = fwnode_property_read_u32(child, "reg", ®); + if (ret || reg >= MT6360_MAX_LEDS || priv->leds[reg]) { + ret = -EINVAL; + goto out; + } + + led = devm_kzalloc(&pdev->dev, sizeof(*led), GFP_KERNEL); + if (!led) { + ret = -ENOMEM; + goto out; + } + + led->led_no = reg; + led->priv = priv; + + ret = mt6360_init_common_properties(led, &init_data); + if (ret) + goto out; + + switch (reg) { + case MT6360_LED_ISNK1 ... MT6360_LED_ISNK4: + ret = mt6360_init_isnk_properties(led, &init_data); + if (ret) + goto out; + + ret = mt6360_isnk_register(&pdev->dev, led, &init_data); + break; + default: + ret = mt6360_init_flash_properties(led, &init_data); + if (ret) + goto out; + + ret = mt6360_flash_register(&pdev->dev, led, &init_data); + } + + if (ret) + goto out; + + priv->leds[reg] = led; + } + + platform_set_drvdata(pdev, priv); + return 0; +out: + for (i = MT6360_LED_FLASH1; i <= MT6360_LED_FLASH2; i++) { + struct mt6360_led *led = priv->leds[i]; + + if (led && led->v4l2_flash) + v4l2_flash_release(led->v4l2_flash); + + } + + return ret; +} + +static int mt6360_led_remove(struct platform_device *pdev) +{ + struct mt6360_priv *priv = platform_get_drvdata(pdev); + int i; + + for (i = MT6360_LED_FLASH1; i <= MT6360_LED_FLASH2; i++) { + struct mt6360_led *led = priv->leds[i]; + + if (led && led->v4l2_flash) + v4l2_flash_release(led->v4l2_flash); + + } + + return 0; +} + +static const struct of_device_id __maybe_unused mt6360_led_of_id[] = { + { .compatible = "mediatek,mt6360-led", }, + {}, +}; +MODULE_DEVICE_TABLE(of, mt6360_led_of_id); + +static struct platform_driver mt6360_led_driver = { + .driver = { + .name = "mt6360-led", + .of_match_table = mt6360_led_of_id, + }, + .probe = mt6360_led_probe, + .remove = mt6360_led_remove, +}; +module_platform_driver(mt6360_led_driver); + +MODULE_AUTHOR("Gene Chen <gene_chen@richtek.com>"); +MODULE_DESCRIPTION("MT6360 Led Driver"); +MODULE_LICENSE("GPL v2");