Re: [PATCH v16 2/2] leds: flash: mt6370: Add MediaTek MT6370 flashlight support

Re: [PATCH v16 2/2] leds: flash: mt6370: Add MediaTek MT6370 flashlight support

[Lee Jones]

Jacek, Andy,

On Tue, 17 Jan 2023, ChiaEn Wu wrote:

> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> The MediaTek MT6370 is a highly-integrated smart power management IC,
> which includes a single cell Li-Ion/Li-Polymer switching battery
> charger, a USB Type-C & Power Delivery (PD) controller, dual Flash
> LED current sources, a RGB LED driver, a backlight WLED driver,
> a display bias driver and a general LDO for portable devices.
> 
> Add support for the MT6370 Flash LED driver. Flash LED in MT6370
> has 2 channels and support torch/strobe mode.
> 
> Co-developed-by: Alice Chen <alice_chen@richtek.com>
> Signed-off-by: Alice Chen <alice_chen@richtek.com>
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> Signed-off-by: ChiaEn Wu <chiaen_wu@richtek.com>
> ---
> 
> v16
> - Remove blank line in the head text.
> - For enum, declared the first element as 0
> - Remove unusd default state and state define.
> - For mt6370_priv, 'dev' member only be used once. Remove it.
> - Refine assign operator for '?:', no need to use brackets for the
>   judgement.
> - Refine '_mt6370_flash_brightness_set' function for else case.
> - Modify 'struct led_flash_setting *setting' to 'struct
>   led_flash_setting *s'. This can reduce line count.
> - Remove the revert logic in 'mt6370_strobe_set' function.
>   No need to do it.
> - Remove mutex usage in timeout_set and fault_get.
>   Regmap lock already protect it.
> - Add default label for switch case in fault_get.
> - Directly assign fwnode as the parameter for 'mt6370_led_register' and
>   'mt6370_init_flash_config'.
> - Due to struct mt6370_priv 'dev' member removed, pass 'dev' as the
>   input parameter for 'mt6370_init_flash_config'.
> - For some if judgement, remove 'num > MT6370_MAX_LEDS'.
>   There's no need to check it. The below code already protect it.
> - Group mt6360 and mt6370 in Makefile.
> - SPDX already declared as 'GPL v2-only', change MODULE_LICENSE to
>   'GPL'.
> - Add one more author for ChiYuan Huang.
> ---
>  drivers/leds/flash/Kconfig             |  13 +
>  drivers/leds/flash/Makefile            |   1 +
>  drivers/leds/flash/leds-mt6370-flash.c | 596 +++++++++++++++++++++++++++++++++
>  3 files changed, 610 insertions(+)
>  create mode 100644 drivers/leds/flash/leds-mt6370-flash.c

I see that you guys have been super helpful reviewing this in the past.

Have your queries now been addressed?
 
> diff --git a/drivers/leds/flash/Kconfig b/drivers/leds/flash/Kconfig
> index d3eb689..49d8922 100644
> --- a/drivers/leds/flash/Kconfig
> +++ b/drivers/leds/flash/Kconfig
> @@ -61,6 +61,19 @@ config LEDS_MT6360
>  	  Independent current sources supply for each flash LED support torch
>  	  and strobe mode.
>  
> +config LEDS_MT6370_FLASH
> +	tristate "Flash LED Support for MediaTek MT6370 PMIC"
> +	depends on LEDS_CLASS
> +	depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
> +	depends on MFD_MT6370
> +	help
> +	  Support 2 channels and torch/strobe mode.
> +	  Say Y here to enable support for
> +	  MT6370_FLASH_LED device.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called "leds-mt6370-flash".
> +
>  config LEDS_RT4505
>  	tristate "LED support for RT4505 flashlight controller"
>  	depends on I2C && OF
> diff --git a/drivers/leds/flash/Makefile b/drivers/leds/flash/Makefile
> index 0acbddc..4665e8e 100644
> --- a/drivers/leds/flash/Makefile
> +++ b/drivers/leds/flash/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
>  obj-$(CONFIG_LEDS_MT6360)	+= leds-mt6360.o
> +obj-$(CONFIG_LEDS_MT6370_FLASH)	+= leds-mt6370-flash.o
>  obj-$(CONFIG_LEDS_AAT1290)	+= leds-aat1290.o
>  obj-$(CONFIG_LEDS_AS3645A)	+= leds-as3645a.o
>  obj-$(CONFIG_LEDS_KTD2692)	+= leds-ktd2692.o
> diff --git a/drivers/leds/flash/leds-mt6370-flash.c b/drivers/leds/flash/leds-mt6370-flash.c
> new file mode 100644
> index 00000000..e5c4c94
> --- /dev/null
> +++ b/drivers/leds/flash/leds-mt6370-flash.c
> @@ -0,0 +1,596 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 Richtek Technology Corp.
> + *
> + * Authors:
> + *   Alice Chen <alice_chen@richtek.com>
> + *   ChiYuan Huang <cy_huang@richtek.com>
> + */
> +
> +#include <linux/bitops.h>
> +#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/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +
> +#include <media/v4l2-flash-led-class.h>
> +
> +enum {
> +	MT6370_LED_FLASH1 = 0,
> +	MT6370_LED_FLASH2,
> +	MT6370_MAX_LEDS
> +};
> +
> +/* Virtual definition for multicolor */
> +
> +#define MT6370_REG_FLEDEN		0x17E
> +#define MT6370_REG_STRBTO		0x173
> +#define MT6370_REG_CHGSTAT2		0x1D1
> +#define MT6370_REG_FLEDSTAT1		0x1D9
> +#define MT6370_REG_FLEDISTRB(_id)	(0x174 + 4 * (_id))
> +#define MT6370_REG_FLEDITOR(_id)	(0x175 + 4 * (_id))
> +#define MT6370_ITORCH_MASK		GENMASK(4, 0)
> +#define MT6370_ISTROBE_MASK		GENMASK(6, 0)
> +#define MT6370_STRBTO_MASK		GENMASK(6, 0)
> +#define MT6370_TORCHEN_MASK		BIT(3)
> +#define MT6370_STROBEN_MASK		BIT(2)
> +#define MT6370_FLCSEN_MASK(_id)		BIT(MT6370_LED_FLASH2 - (_id))
> +#define MT6370_FLCSEN_MASK_ALL		GENMASK(1, 0)
> +#define MT6370_FLEDCHGVINOVP_MASK	BIT(3)
> +#define MT6370_FLED1STRBTO_MASK		BIT(11)
> +#define MT6370_FLED2STRBTO_MASK		BIT(10)
> +#define MT6370_FLED1STRB_MASK		BIT(9)
> +#define MT6370_FLED2STRB_MASK		BIT(8)
> +#define MT6370_FLED1SHORT_MASK		BIT(7)
> +#define MT6370_FLED2SHORT_MASK		BIT(6)
> +#define MT6370_FLEDLVF_MASK		BIT(3)
> +
> +#define MT6370_LED_JOINT		2
> +#define MT6370_RANGE_FLED_REG		4
> +#define MT6370_ITORCH_MIN_uA		25000
> +#define MT6370_ITORCH_STEP_uA		12500
> +#define MT6370_ITORCH_MAX_uA		400000
> +#define MT6370_ITORCH_DOUBLE_MAX_uA	800000
> +#define MT6370_ISTRB_MIN_uA		50000
> +#define MT6370_ISTRB_STEP_uA		12500
> +#define MT6370_ISTRB_MAX_uA		1500000
> +#define MT6370_ISTRB_DOUBLE_MAX_uA	3000000
> +#define MT6370_STRBTO_MIN_US		64000
> +#define MT6370_STRBTO_STEP_US		32000
> +#define MT6370_STRBTO_MAX_US		2432000
> +
> +#define to_mt6370_led(ptr, member) container_of(ptr, struct mt6370_led, member)
> +
> +struct mt6370_led {
> +	struct led_classdev_flash flash;
> +	struct v4l2_flash *v4l2_flash;
> +	struct mt6370_priv *priv;
> +	u8 led_no;
> +};
> +
> +struct mt6370_priv {
> +	struct regmap *regmap;
> +	struct mutex lock;
> +	unsigned int fled_strobe_used;
> +	unsigned int fled_torch_used;
> +	unsigned int leds_active;
> +	unsigned int leds_count;
> +	struct mt6370_led leds[];
> +};
> +
> +static int mt6370_torch_brightness_set(struct led_classdev *lcdev,
> +				       enum led_brightness level)
> +{
> +	struct mt6370_led *led = to_mt6370_led(lcdev, flash.led_cdev);
> +	struct mt6370_priv *priv = led->priv;
> +	u32 led_enable_mask = led->led_no == MT6370_LED_JOINT ?
> +			      MT6370_FLCSEN_MASK_ALL :
> +			      MT6370_FLCSEN_MASK(led->led_no);
> +	u32 enable_mask = MT6370_TORCHEN_MASK | led_enable_mask;
> +	u32 val = level ? led_enable_mask : 0;
> +	u32 curr;
> +	int ret, i;
> +
> +	mutex_lock(&priv->lock);
> +
> +	/*
> +	 * There is only one set of flash control logic, and this
> +	 * flag is used to check if 'strobe' is currently being used.
> +	 */
> +	if (priv->fled_strobe_used) {
> +		dev_warn(lcdev->dev, "Please disable strobe first [%d]\n",
> +			 priv->fled_strobe_used);
> +		ret = -EBUSY;
> +		goto unlock;
> +	}
> +
> +	if (level)
> +		curr = priv->fled_torch_used | BIT(led->led_no);
> +	else
> +		curr = priv->fled_torch_used & ~BIT(led->led_no);
> +
> +	if (curr)
> +		val |= MT6370_TORCHEN_MASK;
> +
> +	if (level) {
> +		level -= 1;
> +		if (led->led_no == MT6370_LED_JOINT) {
> +			u32 flevel[MT6370_MAX_LEDS];
> +
> +			flevel[0] = level / 2;
> +			flevel[1] = level - flevel[0];
> +			for (i = 0; i < MT6370_MAX_LEDS; i++) {
> +				ret = regmap_update_bits(priv->regmap,
> +						MT6370_REG_FLEDITOR(i),
> +						MT6370_ITORCH_MASK, flevel[i]);
> +				if (ret)
> +					goto unlock;
> +			}
> +		} else {
> +			ret = regmap_update_bits(priv->regmap,
> +					MT6370_REG_FLEDITOR(led->led_no),
> +					MT6370_ITORCH_MASK, level);
> +			if (ret)
> +				goto unlock;
> +		}
> +	}
> +
> +	ret = regmap_update_bits(priv->regmap, MT6370_REG_FLEDEN,
> +				 enable_mask, val);
> +	if (ret)
> +		goto unlock;
> +
> +	priv->fled_torch_used = curr;
> +
> +unlock:
> +	mutex_unlock(&priv->lock);
> +	return ret;
> +}
> +
> +static int mt6370_flash_brightness_set(struct led_classdev_flash *fl_cdev,
> +				       u32 brightness)
> +{
> +	/*
> +	 * Because of the current spikes when turning on the flash,
> +	 * the brightness should be kept by the LED framework. This
> +	 * empty function is used to prevent checking failure when
> +	 * led_classdev_flash registers ops.
> +	 */
> +	return 0;
> +}
> +
> +static int _mt6370_flash_brightness_set(struct led_classdev_flash *fl_cdev,
> +					u32 brightness)
> +{
> +	struct mt6370_led *led = to_mt6370_led(fl_cdev, flash);
> +	struct mt6370_priv *priv = led->priv;
> +	struct led_flash_setting *setting = &fl_cdev->brightness;
> +	u32 val = (brightness - setting->min) / setting->step;
> +	int ret, i;
> +
> +	if (led->led_no == MT6370_LED_JOINT) {
> +		u32 flevel[MT6370_MAX_LEDS];
> +
> +		flevel[0] = val / 2;
> +		flevel[1] = val - flevel[0];
> +		for (i = 0; i < MT6370_MAX_LEDS; i++) {
> +			ret = regmap_update_bits(priv->regmap,
> +						 MT6370_REG_FLEDISTRB(i),
> +						 MT6370_ISTROBE_MASK, flevel[i]);
> +			if (ret)
> +				break;
> +		}
> +	} else
> +		ret = regmap_update_bits(priv->regmap,
> +					 MT6370_REG_FLEDISTRB(led->led_no),
> +					 MT6370_ISTROBE_MASK, val);
> +
> +	return ret;
> +}
> +
> +static int mt6370_strobe_set(struct led_classdev_flash *fl_cdev, bool state)
> +{
> +	struct mt6370_led *led = to_mt6370_led(fl_cdev, flash);
> +	struct mt6370_priv *priv = led->priv;
> +	struct led_classdev *lcdev = &fl_cdev->led_cdev;
> +	struct led_flash_setting *s = &fl_cdev->brightness;
> +	u32 led_enable_mask = led->led_no == MT6370_LED_JOINT ?
> +			      MT6370_FLCSEN_MASK_ALL :
> +			      MT6370_FLCSEN_MASK(led->led_no);
> +	u32 enable_mask = MT6370_STROBEN_MASK | led_enable_mask;
> +	u32 val = state ? led_enable_mask : 0;
> +	u32 curr;
> +	int ret;
> +
> +	mutex_lock(&priv->lock);
> +
> +	/*
> +	 * There is only one set of flash control logic, and this
> +	 * flag is used to check if 'torch' is currently being used.
> +	 */
> +	if (priv->fled_torch_used) {
> +		dev_warn(lcdev->dev, "Please disable torch first [0x%x]\n",
> +				      priv->fled_torch_used);
> +		ret = -EBUSY;
> +		goto unlock;
> +	}
> +
> +	if (state)
> +		curr = priv->fled_strobe_used | BIT(led->led_no);
> +	else
> +		curr = priv->fled_strobe_used & ~BIT(led->led_no);
> +
> +	if (curr)
> +		val |= MT6370_STROBEN_MASK;
> +
> +	ret = regmap_update_bits(priv->regmap, MT6370_REG_FLEDEN, enable_mask,
> +				 val);
> +	if (ret) {
> +		dev_err(lcdev->dev, "[%d] control current source %d fail\n",
> +				     led->led_no, state);
> +		goto unlock;
> +	}
> +
> +	/*
> +	 * If the flash needs to turn on, configure the flash current to
> +	 * ramp up to the setting value. Otherwise, always revert to the
> +	 * minimum one.
> +	 */
> +	ret = _mt6370_flash_brightness_set(fl_cdev, state ? s->val : s->min);
> +	if (ret) {
> +		dev_err(lcdev->dev, "[%d] Failed to set brightness\n", led->led_no);
> +		goto unlock;
> +	}
> +
> +	/*
> +	 * For the flash to turn on/off, we must wait for HW ramping
> +	 * up/down time 5ms/500us to prevent the unexpected problem.
> +	 */
> +	if (!priv->fled_strobe_used && curr)
> +		usleep_range(5000, 6000);
> +	else if (priv->fled_strobe_used && !curr)
> +		usleep_range(500, 600);
> +
> +	priv->fled_strobe_used = curr;
> +
> +unlock:
> +	mutex_unlock(&priv->lock);
> +	return ret;
> +}
> +
> +static int mt6370_strobe_get(struct led_classdev_flash *fl_cdev, bool *state)
> +{
> +	struct mt6370_led *led = to_mt6370_led(fl_cdev, flash);
> +	struct mt6370_priv *priv = led->priv;
> +
> +	mutex_lock(&priv->lock);
> +	*state = !!(priv->fled_strobe_used & BIT(led->led_no));
> +	mutex_unlock(&priv->lock);
> +
> +	return 0;
> +}
> +
> +static int mt6370_timeout_set(struct led_classdev_flash *fl_cdev, u32 timeout)
> +{
> +	struct mt6370_led *led = to_mt6370_led(fl_cdev, flash);
> +	struct mt6370_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, MT6370_REG_STRBTO,
> +				  MT6370_STRBTO_MASK, val);
> +}
> +
> +static int mt6370_fault_get(struct led_classdev_flash *fl_cdev, u32 *fault)
> +{
> +	struct mt6370_led *led = to_mt6370_led(fl_cdev, flash);
> +	struct mt6370_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, MT6370_REG_CHGSTAT2, &chg_stat);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_raw_read(priv->regmap, MT6370_REG_FLEDSTAT1, &fled_stat,
> +			      sizeof(fled_stat));
> +	if (ret)
> +		return ret;
> +
> +	switch (led->led_no) {
> +	case MT6370_LED_FLASH1:
> +		strobe_timeout_mask = MT6370_FLED1STRBTO_MASK;
> +		fled_short_mask = MT6370_FLED1SHORT_MASK;
> +		break;
> +
> +	case MT6370_LED_FLASH2:
> +		strobe_timeout_mask = MT6370_FLED2STRBTO_MASK;
> +		fled_short_mask = MT6370_FLED2SHORT_MASK;
> +		break;
> +
> +	case MT6370_LED_JOINT:
> +		strobe_timeout_mask = MT6370_FLED1STRBTO_MASK |
> +				      MT6370_FLED2STRBTO_MASK;
> +		fled_short_mask = MT6370_FLED1SHORT_MASK |
> +				  MT6370_FLED2SHORT_MASK;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (chg_stat & MT6370_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 & MT6370_FLEDLVF_MASK)
> +		rfault |= LED_FAULT_UNDER_VOLTAGE;
> +
> +	*fault = rfault;
> +	return ret;
> +}
> +
> +static const struct led_flash_ops mt6370_flash_ops = {
> +	.flash_brightness_set = mt6370_flash_brightness_set,
> +	.strobe_set = mt6370_strobe_set,
> +	.strobe_get = mt6370_strobe_get,
> +	.timeout_set = mt6370_timeout_set,
> +	.fault_get = mt6370_fault_get,
> +};
> +
> +#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
> +static int mt6370_flash_external_strobe_set(struct v4l2_flash *v4l2_flash,
> +					    bool enable)
> +{
> +	struct led_classdev_flash *flash = v4l2_flash->fled_cdev;
> +	struct mt6370_led *led = to_mt6370_led(flash, flash);
> +	struct mt6370_priv *priv = led->priv;
> +	u32 mask = led->led_no == MT6370_LED_JOINT ? MT6370_FLCSEN_MASK_ALL :
> +		   MT6370_FLCSEN_MASK(led->led_no);
> +	u32 val = enable ? mask : 0;
> +	int ret;
> +
> +	mutex_lock(&priv->lock);
> +
> +	ret = regmap_update_bits(priv->regmap, MT6370_REG_FLEDEN, mask, val);
> +	if (ret)
> +		goto unlock;
> +
> +	if (enable)
> +		priv->fled_strobe_used |= BIT(led->led_no);
> +	else
> +		priv->fled_strobe_used &= ~BIT(led->led_no);
> +
> +unlock:
> +	mutex_unlock(&priv->lock);
> +	return ret;
> +}
> +
> +static const struct v4l2_flash_ops v4l2_flash_ops = {
> +	.external_strobe_set = mt6370_flash_external_strobe_set,
> +};
> +
> +static void mt6370_init_v4l2_flash_config(struct mt6370_led *led,
> +					  struct v4l2_flash_config *cfg)
> +{
> +	struct led_classdev *lcdev;
> +	struct led_flash_setting *s = &cfg->intensity;
> +
> +	lcdev = &led->flash.led_cdev;
> +
> +	s->min = MT6370_ITORCH_MIN_uA;
> +	s->step = MT6370_ITORCH_STEP_uA;
> +	s->val = s->max = s->min + (lcdev->max_brightness - 1) * s->step;
> +
> +	cfg->has_external_strobe = 1;
> +	strscpy(cfg->dev_name, dev_name(lcdev->dev), sizeof(cfg->dev_name));
> +
> +	cfg->flash_faults = LED_FAULT_SHORT_CIRCUIT | LED_FAULT_TIMEOUT |
> +			    LED_FAULT_INPUT_VOLTAGE | LED_FAULT_UNDER_VOLTAGE;
> +}
> +#else
> +static const struct v4l2_flash_ops v4l2_flash_ops;
> +static void mt6370_init_v4l2_flash_config(struct mt6370_led *led,
> +					  struct v4l2_flash_config *cfg)
> +{
> +}
> +#endif
> +
> +static void mt6370_v4l2_flash_release(void *v4l2_flash)
> +{
> +	v4l2_flash_release(v4l2_flash);
> +}
> +
> +static int mt6370_led_register(struct device *parent, struct mt6370_led *led,
> +				struct fwnode_handle *fwnode)
> +{
> +	struct led_init_data init_data = { .fwnode = fwnode };
> +	struct v4l2_flash_config v4l2_config = {};
> +	int ret;
> +
> +	ret = devm_led_classdev_flash_register_ext(parent, &led->flash,
> +						   &init_data);
> +	if (ret)
> +		return dev_err_probe(parent, ret,
> +				     "Couldn't register flash %d\n", led->led_no);
> +
> +	mt6370_init_v4l2_flash_config(led, &v4l2_config);
> +	led->v4l2_flash = v4l2_flash_init(parent, fwnode, &led->flash,
> +					  &v4l2_flash_ops, &v4l2_config);
> +	if (IS_ERR(led->v4l2_flash))
> +		return dev_err_probe(parent, PTR_ERR(led->v4l2_flash),
> +				     "Failed to register %d v4l2 sd\n", led->led_no);
> +
> +	return devm_add_action_or_reset(parent, mt6370_v4l2_flash_release,
> +					led->v4l2_flash);
> +}
> +
> +static u32 mt6370_clamp(u32 val, u32 min, u32 max, u32 step)
> +{
> +	u32 retval;
> +
> +	retval = clamp_val(val, min, max);
> +	if (step > 1)
> +		retval = rounddown(retval - min, step) + min;
> +
> +	return retval;
> +}
> +
> +static int mt6370_init_flash_properties(struct device *dev,
> +					struct mt6370_led *led,
> +					struct fwnode_handle *fwnode)
> +{
> +	struct led_classdev_flash *flash = &led->flash;
> +	struct led_classdev *lcdev = &flash->led_cdev;
> +	struct mt6370_priv *priv = led->priv;
> +	struct led_flash_setting *s;
> +	u32 sources[MT6370_MAX_LEDS];
> +	u32 max_ua, val;
> +	int i, ret, num;
> +
> +	num = fwnode_property_count_u32(fwnode, "led-sources");
> +	if (num < 1)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "Not specified or wrong number of led-sources\n");
> +
> +	ret = fwnode_property_read_u32_array(fwnode, "led-sources", sources, num);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < num; i++) {
> +		if (sources[i] >= MT6370_MAX_LEDS)
> +			return -EINVAL;
> +		if (priv->leds_active & BIT(sources[i]))
> +			return -EINVAL;
> +		priv->leds_active |= BIT(sources[i]);
> +	}
> +
> +	led->led_no = num == 2 ? MT6370_LED_JOINT : sources[0];
> +
> +	max_ua = num == 2 ? MT6370_ITORCH_DOUBLE_MAX_uA : MT6370_ITORCH_MAX_uA;
> +	val = MT6370_ITORCH_MIN_uA;
> +	ret = fwnode_property_read_u32(fwnode, "led-max-microamp", &val);
> +	if (!ret)
> +		val = mt6370_clamp(val, MT6370_ITORCH_MIN_uA, max_ua,
> +				   MT6370_ITORCH_STEP_uA);
> +
> +	lcdev->max_brightness = (val - MT6370_ITORCH_MIN_uA) /
> +				 MT6370_ITORCH_STEP_uA + 1;
> +	lcdev->brightness_set_blocking = mt6370_torch_brightness_set;
> +	lcdev->flags |= LED_DEV_CAP_FLASH;
> +
> +	max_ua = num == 2 ? MT6370_ISTRB_DOUBLE_MAX_uA : MT6370_ISTRB_MAX_uA;
> +	val = MT6370_ISTRB_MIN_uA;
> +	ret = fwnode_property_read_u32(fwnode, "flash-max-microamp", &val);
> +	if (!ret)
> +		val = mt6370_clamp(val, MT6370_ISTRB_MIN_uA, max_ua,
> +				   MT6370_ISTRB_STEP_uA);
> +
> +	s = &flash->brightness;
> +	s->min = MT6370_ISTRB_MIN_uA;
> +	s->step = MT6370_ISTRB_STEP_uA;
> +	s->val = s->max = val;
> +
> +	/*
> +	 * Always configure to the minimum level when
> +	 * off to prevent flash current spikes.
> +	 */
> +	ret = _mt6370_flash_brightness_set(flash, s->min);
> +	if (ret)
> +		return ret;
> +
> +
> +	val = MT6370_STRBTO_MIN_US;
> +	ret = fwnode_property_read_u32(fwnode, "flash-max-timeout-us", &val);
> +	if (!ret)
> +		val = mt6370_clamp(val, MT6370_STRBTO_MIN_US,
> +				   MT6370_STRBTO_MAX_US, MT6370_STRBTO_STEP_US);
> +
> +	s = &flash->timeout;
> +	s->min = MT6370_STRBTO_MIN_US;
> +	s->step = MT6370_STRBTO_STEP_US;
> +	s->val = s->max = val;
> +
> +	flash->ops = &mt6370_flash_ops;
> +
> +	return 0;
> +}
> +
> +static int mt6370_led_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct mt6370_priv *priv;
> +	struct fwnode_handle *child;
> +	size_t count;
> +	int i = 0, ret;
> +
> +	count = device_get_child_node_count(dev);
> +	if (!count || count > MT6370_MAX_LEDS)
> +		return dev_err_probe(dev, -EINVAL,
> +		       "No child node or node count over max led number %zu\n", count);
> +
> +	priv = devm_kzalloc(dev, struct_size(priv, leds, count), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->leds_count = count;
> +	mutex_init(&priv->lock);
> +
> +	priv->regmap = dev_get_regmap(dev->parent, NULL);
> +	if (!priv->regmap)
> +		return dev_err_probe(dev, -ENODEV, "Failed to get parent regmap\n");
> +
> +	device_for_each_child_node(dev, child) {
> +		struct mt6370_led *led = priv->leds + i;
> +
> +		led->priv = priv;
> +
> +		ret = mt6370_init_flash_properties(dev, led, child);
> +		if (ret) {
> +			fwnode_handle_put(child);
> +			return ret;
> +		}
> +
> +		ret = mt6370_led_register(dev, led, child);
> +		if (ret) {
> +			fwnode_handle_put(child);
> +			return ret;
> +		}
> +
> +		i++;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id mt6370_led_of_id[] = {
> +	{ .compatible = "mediatek,mt6370-flashlight" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mt6370_led_of_id);
> +
> +static struct platform_driver mt6370_led_driver = {
> +	.driver = {
> +		.name = "mt6370-flashlight",
> +		.of_match_table = mt6370_led_of_id,
> +	},
> +	.probe = mt6370_led_probe,
> +};
> +module_platform_driver(mt6370_led_driver);
> +
> +MODULE_AUTHOR("Alice Chen <alice_chen@richtek.com>");
> +MODULE_AUTHOR("ChiYuan Huang <cy_huang@richtek.com>");
> +MODULE_DESCRIPTION("MT6370 FLASH LED Driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.7.4
> 

-- 
Lee Jones [李琼斯]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v16 1/2] leds: rgb: mt6370: Add MediaTek MT6370 current sink type LED Indicator support

[Lee Jones]

Jacek, Andy,

On Tue, 17 Jan 2023, ChiaEn Wu wrote:

> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> The MediaTek MT6370 is a highly-integrated smart power management IC,
> which includes a single cell Li-Ion/Li-Polymer switching battery
> charger, a USB Type-C & Power Delivery (PD) controller, dual
> Flash LED current sources, a RGB LED driver, a backlight WLED driver,
> a display bias driver and a general LDO for portable devices.
> 
> Add support for the MediaTek MT6370 Current Sink Type LED Indicator
> driver. It can control four channels current-sink RGB LEDs with 3 modes:
> constant current, PWM, and breath mode.
> 
> Co-developed-by: Alice Chen <alice_chen@richtek.com>
> Signed-off-by: Alice Chen <alice_chen@richtek.com>
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> Signed-off-by: ChiaEn Wu <chiaen_wu@richtek.com>
> ---
> 
> v16
> - Remove blank line in the head text.
> - Move 'pwm_duty' to descrease the size of struct mt6370_pdata.
> - Move the field 'F_RGB_EN' write out from if/else in
>   'mt6370_isnk_brightness_set'.
> - Remove 'dev' in struct mt6370_priv.
> - Remove the proprety reading for 'linux,default-trigger', led core
>   already did it.
> - Change the module license from 'GPL v2' to 'GPL'
>   In the head text, already deslcred SPDX license as 'GPL-2.0-only'.
> ---
>  drivers/leds/rgb/Kconfig           |   13 +
>  drivers/leds/rgb/Makefile          |    1 +
>  drivers/leds/rgb/leds-mt6370-rgb.c | 1009 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 1023 insertions(+)
>  create mode 100644 drivers/leds/rgb/leds-mt6370-rgb.c

I see that you guys have been super helpful reviewing this in the past.
                                                                       
Have your queries now been addressed?

> diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig
> index 204cf47..7d86bb2 100644
> --- a/drivers/leds/rgb/Kconfig
> +++ b/drivers/leds/rgb/Kconfig
> @@ -26,4 +26,17 @@ config LEDS_QCOM_LPG
>  
>  	  If compiled as a module, the module will be named leds-qcom-lpg.
>  
> +config LEDS_MT6370_RGB
> +	tristate "LED Support for MediaTek MT6370 PMIC"
> +	depends on MFD_MT6370
> +	select LINEAR_RANGE
> +	help
> +	  Say Y here to enable support for MT6370_RGB LED device.
> +	  In MT6370, there are four channel current-sink LED drivers that
> +	  support hardware pattern for constant current, PWM, and breath mode.
> +	  Isink4 channel can also be used as a CHG_VIN power good indicator.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called "leds-mt6370-rgb".
> +
>  endif # LEDS_CLASS_MULTICOLOR
> diff --git a/drivers/leds/rgb/Makefile b/drivers/leds/rgb/Makefile
> index 0675bc0..8c01daf 100644
> --- a/drivers/leds/rgb/Makefile
> +++ b/drivers/leds/rgb/Makefile
> @@ -2,3 +2,4 @@
>  
>  obj-$(CONFIG_LEDS_PWM_MULTICOLOR)	+= leds-pwm-multicolor.o
>  obj-$(CONFIG_LEDS_QCOM_LPG)		+= leds-qcom-lpg.o
> +obj-$(CONFIG_LEDS_MT6370_RGB)		+= leds-mt6370-rgb.o
> diff --git a/drivers/leds/rgb/leds-mt6370-rgb.c b/drivers/leds/rgb/leds-mt6370-rgb.c
> new file mode 100644
> index 00000000..2a5d582
> --- /dev/null
> +++ b/drivers/leds/rgb/leds-mt6370-rgb.c
> @@ -0,0 +1,1009 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 Richtek Technology Corp.
> + *
> + * Authors:
> + *   ChiYuan Huang <cy_huang@richtek.com>
> + *   Alice Chen <alice_chen@richtek.com>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/led-class-multicolor.h>
> +#include <linux/linear_range.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/util_macros.h>
> +
> +#include <asm/unaligned.h>
> +
> +enum {
> +	MT6370_LED_ISNK1 = 0,
> +	MT6370_LED_ISNK2,
> +	MT6370_LED_ISNK3,
> +	MT6370_LED_ISNK4,
> +	MT6370_MAX_LEDS
> +};
> +
> +enum mt6370_led_mode {
> +	MT6370_LED_PWM_MODE = 0,
> +	MT6370_LED_BREATH_MODE,
> +	MT6370_LED_REG_MODE,
> +	MT6370_LED_MAX_MODE
> +};
> +
> +enum mt6370_led_field {
> +	F_RGB_EN = 0,
> +	F_CHGIND_EN,
> +	F_LED1_CURR,
> +	F_LED2_CURR,
> +	F_LED3_CURR,
> +	F_LED4_CURR,
> +	F_LED1_MODE,
> +	F_LED2_MODE,
> +	F_LED3_MODE,
> +	F_LED4_MODE,
> +	F_LED1_DUTY,
> +	F_LED2_DUTY,
> +	F_LED3_DUTY,
> +	F_LED4_DUTY,
> +	F_LED1_FREQ,
> +	F_LED2_FREQ,
> +	F_LED3_FREQ,
> +	F_LED4_FREQ,
> +	F_MAX_FIELDS
> +};
> +
> +enum mt6370_led_ranges {
> +	R_LED123_CURR = 0,
> +	R_LED4_CURR,
> +	R_LED_TRFON,
> +	R_LED_TOFF,
> +	R_MAX_RANGES
> +};
> +
> +enum mt6370_pattern {
> +	P_LED_TR1 = 0,
> +	P_LED_TR2,
> +	P_LED_TF1,
> +	P_LED_TF2,
> +	P_LED_TON,
> +	P_LED_TOFF,
> +	P_MAX_PATTERNS
> +};
> +
> +#define MT6370_REG_DEV_INFO			0x100
> +#define MT6370_REG_RGB1_DIM			0x182
> +#define MT6370_REG_RGB2_DIM			0x183
> +#define MT6370_REG_RGB3_DIM			0x184
> +#define MT6370_REG_RGB_EN			0x185
> +#define MT6370_REG_RGB1_ISNK			0x186
> +#define MT6370_REG_RGB2_ISNK			0x187
> +#define MT6370_REG_RGB3_ISNK			0x188
> +#define MT6370_REG_RGB1_TR			0x189
> +#define MT6370_REG_RGB_CHRIND_DIM		0x192
> +#define MT6370_REG_RGB_CHRIND_CTRL		0x193
> +#define MT6370_REG_RGB_CHRIND_TR		0x194
> +
> +#define MT6372_REG_RGB_EN			0x182
> +#define MT6372_REG_RGB1_ISNK			0x183
> +#define MT6372_REG_RGB2_ISNK			0x184
> +#define MT6372_REG_RGB3_ISNK			0x185
> +#define MT6372_REG_RGB4_ISNK			0x186
> +#define MT6372_REG_RGB1_DIM			0x187
> +#define MT6372_REG_RGB2_DIM			0x188
> +#define MT6372_REG_RGB3_DIM			0x189
> +#define MT6372_REG_RGB4_DIM			0x18A
> +#define MT6372_REG_RGB12_FREQ			0x18B
> +#define MT6372_REG_RGB34_FREQ			0x18C
> +#define MT6372_REG_RGB1_TR			0x18D
> +
> +#define MT6370_VENID_MASK			GENMASK(7, 4)
> +#define MT6370_CHEN_BIT(id)			BIT(MT6370_LED_ISNK4 - id)
> +#define MT6370_VIRTUAL_MULTICOLOR		5
> +#define MC_CHANNEL_NUM				3
> +#define MT6370_PWM_DUTY				(BIT(5) - 1)
> +#define MT6372_PWM_DUTY				(BIT(8) - 1)
> +
> +struct mt6370_led {
> +	/*
> +	 * If the color of the LED in DT is set to
> +	 *   - 'LED_COLOR_ID_RGB'
> +	 *   - 'LED_COLOR_ID_MULTI'
> +	 * The member 'index' of this struct will be set to
> +	 * 'MT6370_VIRTUAL_MULTICOLOR'.
> +	 * If so, this LED will choose 'struct led_classdev_mc mc' to use.
> +	 * Instead, if the member 'index' of this struct is set to
> +	 * 'MT6370_LED_ISNK1' ~ 'MT6370_LED_ISNK4', then this LED will choose
> +	 * 'struct led_classdev isink' to use.
> +	 */
> +	union {
> +		struct led_classdev isink;
> +		struct led_classdev_mc mc;
> +	};
> +	struct mt6370_priv *priv;
> +	enum led_default_state default_state;
> +	u32 index;
> +};
> +
> +struct mt6370_pdata {
> +	const unsigned int *tfreq;
> +	unsigned int tfreq_len;
> +	u16 reg_rgb1_tr;
> +	s16 reg_rgb_chrind_tr;
> +	u8 pwm_duty;
> +};
> +
> +struct mt6370_priv {
> +	/* Per LED access lock */
> +	struct mutex lock;
> +	struct regmap *regmap;
> +	struct regmap_field *fields[F_MAX_FIELDS];
> +	const struct reg_field *reg_fields;
> +	const struct linear_range *ranges;
> +	struct reg_cfg *reg_cfgs;
> +	const struct mt6370_pdata *pdata;
> +	unsigned int leds_count;
> +	unsigned int leds_active;
> +	struct mt6370_led leds[];
> +};
> +
> +static const struct reg_field common_reg_fields[F_MAX_FIELDS] = {
> +	[F_RGB_EN]	= REG_FIELD(MT6370_REG_RGB_EN, 4, 7),
> +	[F_CHGIND_EN]	= REG_FIELD(MT6370_REG_RGB_CHRIND_DIM, 7, 7),
> +	[F_LED1_CURR]	= REG_FIELD(MT6370_REG_RGB1_ISNK, 0, 2),
> +	[F_LED2_CURR]	= REG_FIELD(MT6370_REG_RGB2_ISNK, 0, 2),
> +	[F_LED3_CURR]	= REG_FIELD(MT6370_REG_RGB3_ISNK, 0, 2),
> +	[F_LED4_CURR]	= REG_FIELD(MT6370_REG_RGB_CHRIND_CTRL, 0, 1),
> +	[F_LED1_MODE]	= REG_FIELD(MT6370_REG_RGB1_DIM, 5, 6),
> +	[F_LED2_MODE]	= REG_FIELD(MT6370_REG_RGB2_DIM, 5, 6),
> +	[F_LED3_MODE]	= REG_FIELD(MT6370_REG_RGB3_DIM, 5, 6),
> +	[F_LED4_MODE]	= REG_FIELD(MT6370_REG_RGB_CHRIND_DIM, 5, 6),
> +	[F_LED1_DUTY]	= REG_FIELD(MT6370_REG_RGB1_DIM, 0, 4),
> +	[F_LED2_DUTY]	= REG_FIELD(MT6370_REG_RGB2_DIM, 0, 4),
> +	[F_LED3_DUTY]	= REG_FIELD(MT6370_REG_RGB3_DIM, 0, 4),
> +	[F_LED4_DUTY]	= REG_FIELD(MT6370_REG_RGB_CHRIND_DIM, 0, 4),
> +	[F_LED1_FREQ]	= REG_FIELD(MT6370_REG_RGB1_ISNK, 3, 5),
> +	[F_LED2_FREQ]	= REG_FIELD(MT6370_REG_RGB2_ISNK, 3, 5),
> +	[F_LED3_FREQ]	= REG_FIELD(MT6370_REG_RGB3_ISNK, 3, 5),
> +	[F_LED4_FREQ]	= REG_FIELD(MT6370_REG_RGB_CHRIND_CTRL, 2, 4),
> +};
> +
> +static const struct reg_field mt6372_reg_fields[F_MAX_FIELDS] = {
> +	[F_RGB_EN]	= REG_FIELD(MT6372_REG_RGB_EN, 4, 7),
> +	[F_CHGIND_EN]	= REG_FIELD(MT6372_REG_RGB_EN, 3, 3),
> +	[F_LED1_CURR]	= REG_FIELD(MT6372_REG_RGB1_ISNK, 0, 3),
> +	[F_LED2_CURR]	= REG_FIELD(MT6372_REG_RGB2_ISNK, 0, 3),
> +	[F_LED3_CURR]	= REG_FIELD(MT6372_REG_RGB3_ISNK, 0, 3),
> +	[F_LED4_CURR]	= REG_FIELD(MT6372_REG_RGB4_ISNK, 0, 3),
> +	[F_LED1_MODE]	= REG_FIELD(MT6372_REG_RGB1_ISNK, 6, 7),
> +	[F_LED2_MODE]	= REG_FIELD(MT6372_REG_RGB2_ISNK, 6, 7),
> +	[F_LED3_MODE]	= REG_FIELD(MT6372_REG_RGB3_ISNK, 6, 7),
> +	[F_LED4_MODE]	= REG_FIELD(MT6372_REG_RGB4_ISNK, 6, 7),
> +	[F_LED1_DUTY]	= REG_FIELD(MT6372_REG_RGB1_DIM, 0, 7),
> +	[F_LED2_DUTY]	= REG_FIELD(MT6372_REG_RGB2_DIM, 0, 7),
> +	[F_LED3_DUTY]	= REG_FIELD(MT6372_REG_RGB3_DIM, 0, 7),
> +	[F_LED4_DUTY]	= REG_FIELD(MT6372_REG_RGB4_DIM, 0, 7),
> +	[F_LED1_FREQ]	= REG_FIELD(MT6372_REG_RGB12_FREQ, 5, 7),
> +	[F_LED2_FREQ]	= REG_FIELD(MT6372_REG_RGB12_FREQ, 2, 4),
> +	[F_LED3_FREQ]	= REG_FIELD(MT6372_REG_RGB34_FREQ, 5, 7),
> +	[F_LED4_FREQ]	= REG_FIELD(MT6372_REG_RGB34_FREQ, 2, 4),
> +};
> +
> +/* Current unit: microamp, time unit: millisecond */
> +static const struct linear_range common_led_ranges[R_MAX_RANGES] = {
> +	[R_LED123_CURR]	= { 4000, 1, 6, 4000 },
> +	[R_LED4_CURR]	= { 2000, 1, 3, 2000 },
> +	[R_LED_TRFON]	= { 125, 0, 15, 200 },
> +	[R_LED_TOFF]	= { 250, 0, 15, 400 },
> +};
> +
> +static const struct linear_range mt6372_led_ranges[R_MAX_RANGES] = {
> +	[R_LED123_CURR]	= { 2000, 1, 14, 2000 },
> +	[R_LED4_CURR]	= { 2000, 1, 14, 2000 },
> +	[R_LED_TRFON]	= { 125, 0, 15, 250 },
> +	[R_LED_TOFF]	= { 250, 0, 15, 500 },
> +};
> +
> +static const unsigned int common_tfreqs[] = {
> +	10000, 5000, 2000, 1000, 500, 200, 5, 1,
> +};
> +
> +static const unsigned int mt6372_tfreqs[] = {
> +	8000, 4000, 2000, 1000, 500, 250, 8, 4,
> +};
> +
> +static const struct mt6370_pdata common_pdata = {
> +	.tfreq = common_tfreqs,
> +	.tfreq_len = ARRAY_SIZE(common_tfreqs),
> +	.pwm_duty = MT6370_PWM_DUTY,
> +	.reg_rgb1_tr = MT6370_REG_RGB1_TR,
> +	.reg_rgb_chrind_tr = MT6370_REG_RGB_CHRIND_TR,
> +};
> +
> +static const struct mt6370_pdata mt6372_pdata = {
> +	.tfreq = mt6372_tfreqs,
> +	.tfreq_len = ARRAY_SIZE(mt6372_tfreqs),
> +	.pwm_duty = MT6372_PWM_DUTY,
> +	.reg_rgb1_tr = MT6372_REG_RGB1_TR,
> +	.reg_rgb_chrind_tr = -1,
> +};
> +
> +static enum mt6370_led_field mt6370_get_led_current_field(unsigned int led_no)
> +{
> +	switch (led_no) {
> +	case MT6370_LED_ISNK1:
> +		return F_LED1_CURR;
> +	case MT6370_LED_ISNK2:
> +		return F_LED2_CURR;
> +	case MT6370_LED_ISNK3:
> +		return F_LED3_CURR;
> +	default:
> +		return F_LED4_CURR;
> +	}
> +}
> +
> +static int mt6370_set_led_brightness(struct mt6370_priv *priv,
> +				     unsigned int led_no, unsigned int level)
> +{
> +	enum mt6370_led_field sel_field;
> +
> +	sel_field = mt6370_get_led_current_field(led_no);
> +
> +	return regmap_field_write(priv->fields[sel_field], level);
> +}
> +
> +static int mt6370_get_led_brightness(struct mt6370_priv *priv,
> +				     unsigned int led_no, unsigned int *level)
> +{
> +	enum mt6370_led_field sel_field;
> +
> +	sel_field = mt6370_get_led_current_field(led_no);
> +
> +	return regmap_field_read(priv->fields[sel_field], level);
> +}
> +
> +static int mt6370_set_led_duty(struct mt6370_priv *priv, unsigned int led_no,
> +			       unsigned int ton, unsigned int toff)
> +{
> +	const struct mt6370_pdata *pdata = priv->pdata;
> +	enum mt6370_led_field sel_field;
> +	unsigned int divisor, ratio;
> +
> +	divisor = pdata->pwm_duty;
> +	ratio = ton * divisor / (ton + toff);
> +
> +	switch (led_no) {
> +	case MT6370_LED_ISNK1:
> +		sel_field = F_LED1_DUTY;
> +		break;
> +	case MT6370_LED_ISNK2:
> +		sel_field = F_LED2_DUTY;
> +		break;
> +	case MT6370_LED_ISNK3:
> +		sel_field = F_LED3_DUTY;
> +		break;
> +	default:
> +		sel_field = F_LED4_DUTY;
> +		break;
> +	}
> +
> +	return regmap_field_write(priv->fields[sel_field], ratio);
> +}
> +
> +static int mt6370_set_led_freq(struct mt6370_priv *priv, unsigned int led_no,
> +			       unsigned int ton, unsigned int toff)
> +{
> +	const struct mt6370_pdata *pdata = priv->pdata;
> +	enum mt6370_led_field sel_field;
> +	unsigned int tfreq_len = pdata->tfreq_len;
> +	unsigned int tsum, sel;
> +
> +	tsum = ton + toff;
> +
> +	if (tsum > pdata->tfreq[0] || tsum < pdata->tfreq[tfreq_len - 1])
> +		return -EOPNOTSUPP;
> +
> +	sel = find_closest_descending(tsum, pdata->tfreq, tfreq_len);
> +
> +	switch (led_no) {
> +	case MT6370_LED_ISNK1:
> +		sel_field = F_LED1_FREQ;
> +		break;
> +	case MT6370_LED_ISNK2:
> +		sel_field = F_LED2_FREQ;
> +		break;
> +	case MT6370_LED_ISNK3:
> +		sel_field = F_LED3_FREQ;
> +		break;
> +	default:
> +		sel_field = F_LED4_FREQ;
> +		break;
> +	}
> +
> +	return regmap_field_write(priv->fields[sel_field], sel);
> +}
> +
> +static void mt6370_get_breath_reg_base(struct mt6370_priv *priv,
> +				       unsigned int led_no, unsigned int *base)
> +{
> +	const struct mt6370_pdata *pdata = priv->pdata;
> +
> +	if (pdata->reg_rgb_chrind_tr < 0) {
> +		*base = pdata->reg_rgb1_tr + led_no * 3;
> +		return;
> +	}
> +
> +	switch (led_no) {
> +	case MT6370_LED_ISNK1:
> +	case MT6370_LED_ISNK2:
> +	case MT6370_LED_ISNK3:
> +		*base = pdata->reg_rgb1_tr + led_no * 3;
> +		break;
> +	default:
> +		*base = pdata->reg_rgb_chrind_tr;
> +		break;
> +	}
> +}
> +
> +static int mt6370_gen_breath_pattern(struct mt6370_priv *priv,
> +				     struct led_pattern *pattern, u32 len,
> +				     u8 *pattern_val, u32 val_len)
> +{
> +	enum mt6370_led_ranges sel_range;
> +	struct led_pattern *curr;
> +	unsigned int sel;
> +	u32 val = 0;
> +	int i;
> +
> +	if (len < P_MAX_PATTERNS && val_len < P_MAX_PATTERNS / 2)
> +		return -EINVAL;
> +
> +	/*
> +	 * Pattern list
> +	 * tr1:	 byte 0, b'[7: 4]
> +	 * tr2:	 byte 0, b'[3: 0]
> +	 * tf1:	 byte 1, b'[7: 4]
> +	 * tf2:	 byte 1, b'[3: 0]
> +	 * ton:	 byte 2, b'[7: 4]
> +	 * toff: byte 2, b'[3: 0]
> +	 */
> +	for (i = 0; i < P_MAX_PATTERNS; i++) {
> +		curr = pattern + i;
> +
> +		sel_range = i == P_LED_TOFF ? R_LED_TOFF : R_LED_TRFON;
> +
> +		linear_range_get_selector_within(priv->ranges + sel_range,
> +						 curr->delta_t, &sel);
> +
> +		if (i % 2) {
> +			val |= sel;
> +		} else {
> +			val <<= 8;
> +			val |= sel << 4;
> +		}
> +	}
> +
> +	put_unaligned_be24(val, pattern_val);
> +
> +	return 0;
> +}
> +
> +static int mt6370_set_led_mode(struct mt6370_priv *priv, unsigned int led_no,
> +			       enum mt6370_led_mode mode)
> +{
> +	enum mt6370_led_field sel_field;
> +
> +	switch (led_no) {
> +	case MT6370_LED_ISNK1:
> +		sel_field = F_LED1_MODE;
> +		break;
> +	case MT6370_LED_ISNK2:
> +		sel_field = F_LED2_MODE;
> +		break;
> +	case MT6370_LED_ISNK3:
> +		sel_field = F_LED3_MODE;
> +		break;
> +	default:
> +		sel_field = F_LED4_MODE;
> +		break;
> +	}
> +
> +	return regmap_field_write(priv->fields[sel_field], mode);
> +}
> +
> +static int mt6370_mc_brightness_set(struct led_classdev *lcdev,
> +				    enum led_brightness level)
> +{
> +	struct led_classdev_mc *mccdev = lcdev_to_mccdev(lcdev);
> +	struct mt6370_led *led = container_of(mccdev, struct mt6370_led, mc);
> +	struct mt6370_priv *priv = led->priv;
> +	struct mc_subled *subled;
> +	unsigned int enable, disable;
> +	int i, ret;
> +
> +	mutex_lock(&priv->lock);
> +
> +	led_mc_calc_color_components(mccdev, level);
> +
> +	ret = regmap_field_read(priv->fields[F_RGB_EN], &enable);
> +	if (ret)
> +		goto out_unlock;
> +
> +	disable = enable;
> +
> +	for (i = 0; i < mccdev->num_colors; i++) {
> +		u32 brightness;
> +
> +		subled = mccdev->subled_info + i;
> +		brightness = min(subled->brightness, lcdev->max_brightness);
> +		disable &= ~MT6370_CHEN_BIT(subled->channel);
> +
> +		if (level == 0) {
> +			enable &= ~MT6370_CHEN_BIT(subled->channel);
> +
> +			ret = mt6370_set_led_mode(priv, subled->channel,
> +						  MT6370_LED_REG_MODE);
> +			if (ret)
> +				goto out_unlock;
> +
> +			continue;
> +		}
> +
> +		if (brightness == 0) {
> +			enable &= ~MT6370_CHEN_BIT(subled->channel);
> +			continue;
> +		}
> +
> +		enable |= MT6370_CHEN_BIT(subled->channel);
> +
> +		ret = mt6370_set_led_brightness(priv, subled->channel,
> +						brightness);
> +		if (ret)
> +			goto out_unlock;
> +	}
> +
> +	ret = regmap_field_write(priv->fields[F_RGB_EN], disable);
> +	if (ret)
> +		goto out_unlock;
> +
> +	ret = regmap_field_write(priv->fields[F_RGB_EN], enable);
> +
> +out_unlock:
> +	mutex_unlock(&priv->lock);
> +
> +	return ret;
> +}
> +
> +static int mt6370_mc_blink_set(struct led_classdev *lcdev,
> +			       unsigned long *delay_on,
> +			       unsigned long *delay_off)
> +{
> +	struct led_classdev_mc *mccdev = lcdev_to_mccdev(lcdev);
> +	struct mt6370_led *led = container_of(mccdev, struct mt6370_led, mc);
> +	struct mt6370_priv *priv = led->priv;
> +	struct mc_subled *subled;
> +	unsigned int enable, disable;
> +	int i, ret;
> +
> +	mutex_lock(&priv->lock);
> +
> +	if (!*delay_on && !*delay_off)
> +		*delay_on = *delay_off = 500;
> +
> +	ret = regmap_field_read(priv->fields[F_RGB_EN], &enable);
> +	if (ret)
> +		goto out_unlock;
> +
> +	disable = enable;
> +
> +	for (i = 0; i < mccdev->num_colors; i++) {
> +		subled = mccdev->subled_info + i;
> +
> +		disable &= ~MT6370_CHEN_BIT(subled->channel);
> +
> +		ret = mt6370_set_led_duty(priv, subled->channel, *delay_on,
> +					  *delay_off);
> +		if (ret)
> +			goto out_unlock;
> +
> +		ret = mt6370_set_led_freq(priv, subled->channel, *delay_on,
> +					  *delay_off);
> +		if (ret)
> +			goto out_unlock;
> +
> +		ret = mt6370_set_led_mode(priv, subled->channel,
> +					  MT6370_LED_PWM_MODE);
> +		if (ret)
> +			goto out_unlock;
> +	}
> +
> +	/* Toggle to make pattern timing the same */
> +	ret = regmap_field_write(priv->fields[F_RGB_EN], disable);
> +	if (ret)
> +		goto out_unlock;
> +
> +	ret = regmap_field_write(priv->fields[F_RGB_EN], enable);
> +
> +out_unlock:
> +	mutex_unlock(&priv->lock);
> +
> +	return ret;
> +}
> +
> +static int mt6370_mc_pattern_set(struct led_classdev *lcdev,
> +			struct led_pattern *pattern, u32 len, int repeat)
> +{
> +	struct led_classdev_mc *mccdev = lcdev_to_mccdev(lcdev);
> +	struct mt6370_led *led = container_of(mccdev, struct mt6370_led, mc);
> +	struct mt6370_priv *priv = led->priv;
> +	struct mc_subled *subled;
> +	unsigned int reg_base, enable, disable;
> +	u8 params[P_MAX_PATTERNS / 2];
> +	int i, ret;
> +
> +	mutex_lock(&priv->lock);
> +
> +	ret = mt6370_gen_breath_pattern(priv, pattern, len, params,
> +					sizeof(params));
> +	if (ret)
> +		goto out_unlock;
> +
> +	ret = regmap_field_read(priv->fields[F_RGB_EN], &enable);
> +	if (ret)
> +		goto out_unlock;
> +
> +	disable = enable;
> +
> +	for (i = 0; i < mccdev->num_colors; i++) {
> +		subled = mccdev->subled_info + i;
> +
> +		mt6370_get_breath_reg_base(priv, subled->channel, &reg_base);
> +		disable &= ~MT6370_CHEN_BIT(subled->channel);
> +
> +		ret = regmap_raw_write(priv->regmap, reg_base, params,
> +				       sizeof(params));
> +		if (ret)
> +			goto out_unlock;
> +
> +		ret = mt6370_set_led_mode(priv, subled->channel,
> +					  MT6370_LED_BREATH_MODE);
> +		if (ret)
> +			goto out_unlock;
> +	}
> +
> +	/* Toggle to make pattern timing be the same */
> +	ret = regmap_field_write(priv->fields[F_RGB_EN], disable);
> +	if (ret)
> +		goto out_unlock;
> +
> +	ret = regmap_field_write(priv->fields[F_RGB_EN], enable);
> +
> +out_unlock:
> +	mutex_unlock(&priv->lock);
> +
> +	return ret;
> +}
> +
> +static inline int mt6370_mc_pattern_clear(struct led_classdev *lcdev)
> +{
> +	struct led_classdev_mc *mccdev = lcdev_to_mccdev(lcdev);
> +	struct mt6370_led *led = container_of(mccdev, struct mt6370_led, mc);
> +	struct mt6370_priv *priv = led->priv;
> +	struct mc_subled *subled;
> +	int i, ret;
> +
> +	mutex_lock(&led->priv->lock);
> +
> +	for (i = 0; i < mccdev->num_colors; i++) {
> +		subled = mccdev->subled_info + i;
> +
> +		ret = mt6370_set_led_mode(priv, subled->channel,
> +					  MT6370_LED_REG_MODE);
> +		if (ret)
> +			break;
> +	}
> +
> +	mutex_unlock(&led->priv->lock);
> +
> +	return ret;
> +}
> +
> +static int mt6370_isnk_brightness_set(struct led_classdev *lcdev,
> +				      enum led_brightness level)
> +{
> +	struct mt6370_led *led = container_of(lcdev, struct mt6370_led, isink);
> +	struct mt6370_priv *priv = led->priv;
> +	unsigned int enable;
> +	int ret;
> +
> +	mutex_lock(&priv->lock);
> +
> +	ret = regmap_field_read(priv->fields[F_RGB_EN], &enable);
> +	if (ret)
> +		goto out_unlock;
> +
> +	if (level == 0) {
> +		enable &= ~MT6370_CHEN_BIT(led->index);
> +
> +		ret = mt6370_set_led_mode(priv, led->index,
> +					  MT6370_LED_REG_MODE);
> +		if (ret)
> +			goto out_unlock;
> +	} else {
> +		enable |= MT6370_CHEN_BIT(led->index);
> +
> +		ret = mt6370_set_led_brightness(priv, led->index, level);
> +		if (ret)
> +			goto out_unlock;
> +	}
> +
> +	ret = regmap_field_write(priv->fields[F_RGB_EN], enable);
> +
> +out_unlock:
> +	mutex_unlock(&priv->lock);
> +
> +	return ret;
> +}
> +
> +static int mt6370_isnk_blink_set(struct led_classdev *lcdev,
> +				 unsigned long *delay_on,
> +				 unsigned long *delay_off)
> +{
> +	struct mt6370_led *led = container_of(lcdev, struct mt6370_led, isink);
> +	struct mt6370_priv *priv = led->priv;
> +	int ret;
> +
> +	mutex_lock(&priv->lock);
> +
> +	if (!*delay_on && !*delay_off)
> +		*delay_on = *delay_off = 500;
> +
> +	ret = mt6370_set_led_duty(priv, led->index, *delay_on, *delay_off);
> +	if (ret)
> +		goto out_unlock;
> +
> +	ret = mt6370_set_led_freq(priv, led->index, *delay_on, *delay_off);
> +	if (ret)
> +		goto out_unlock;
> +
> +	ret = mt6370_set_led_mode(priv, led->index, MT6370_LED_PWM_MODE);
> +
> +out_unlock:
> +	mutex_unlock(&priv->lock);
> +
> +	return ret;
> +}
> +
> +static int mt6370_isnk_pattern_set(struct led_classdev *lcdev,
> +				   struct led_pattern *pattern, u32 len,
> +				   int repeat)
> +{
> +	struct mt6370_led *led = container_of(lcdev, struct mt6370_led, isink);
> +	struct mt6370_priv *priv = led->priv;
> +	unsigned int reg_base;
> +	u8 params[P_MAX_PATTERNS / 2];
> +	int ret;
> +
> +	mutex_lock(&priv->lock);
> +
> +	ret = mt6370_gen_breath_pattern(priv, pattern, len, params,
> +					sizeof(params));
> +	if (ret)
> +		goto out_unlock;
> +
> +	mt6370_get_breath_reg_base(priv, led->index, &reg_base);
> +
> +	ret = regmap_raw_write(priv->regmap, reg_base, params, sizeof(params));
> +	if (ret)
> +		goto out_unlock;
> +
> +	ret = mt6370_set_led_mode(priv, led->index, MT6370_LED_BREATH_MODE);
> +
> +out_unlock:
> +	mutex_unlock(&priv->lock);
> +
> +	return ret;
> +}
> +
> +static inline int mt6370_isnk_pattern_clear(struct led_classdev *lcdev)
> +{
> +	struct mt6370_led *led = container_of(lcdev, struct mt6370_led, isink);
> +	struct mt6370_priv *priv = led->priv;
> +	int ret;
> +
> +	mutex_lock(&led->priv->lock);
> +	ret = mt6370_set_led_mode(priv, led->index, MT6370_LED_REG_MODE);
> +	mutex_unlock(&led->priv->lock);
> +
> +	return ret;
> +}
> +
> +static int mt6370_init_led_properties(struct device *dev,
> +				      struct mt6370_led *led,
> +				      struct led_init_data *init_data)
> +{
> +	struct mt6370_priv *priv = led->priv;
> +	struct led_classdev *lcdev;
> +	struct fwnode_handle *child;
> +	enum mt6370_led_ranges sel_range;
> +	u32 max_uA, max_level;
> +	int ret;
> +
> +	if (led->index == MT6370_VIRTUAL_MULTICOLOR) {
> +		struct mc_subled *sub_led;
> +		u32 num_color = 0;
> +
> +		sub_led = devm_kcalloc(dev, MC_CHANNEL_NUM, sizeof(*sub_led),
> +				       GFP_KERNEL);
> +		if (!sub_led)
> +			return -ENOMEM;
> +
> +		fwnode_for_each_child_node(init_data->fwnode, child) {
> +			u32 reg, color;
> +
> +			ret = fwnode_property_read_u32(child, "reg", &reg);
> +			if (ret || reg > MT6370_LED_ISNK3 ||
> +			    priv->leds_active & BIT(reg)) {
> +				fwnode_handle_put(child);
> +				return -EINVAL;
> +			}
> +
> +			ret = fwnode_property_read_u32(child, "color", &color);
> +			if (ret) {
> +				fwnode_handle_put(child);
> +				return dev_err_probe(dev, ret, "LED %d, no color specified\n",
> +						     led->index);
> +			}
> +
> +			priv->leds_active |= BIT(reg);
> +			sub_led[num_color].color_index = color;
> +			sub_led[num_color].channel = reg;
> +			sub_led[num_color].intensity = 0;
> +			num_color++;
> +		}
> +
> +		if (num_color < 2)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Multicolor must include 2 or more LED channels\n");
> +
> +		led->mc.num_colors = num_color;
> +		led->mc.subled_info = sub_led;
> +
> +		lcdev = &led->mc.led_cdev;
> +		lcdev->brightness_set_blocking = mt6370_mc_brightness_set;
> +		lcdev->blink_set = mt6370_mc_blink_set;
> +		lcdev->pattern_set = mt6370_mc_pattern_set;
> +		lcdev->pattern_clear = mt6370_mc_pattern_clear;
> +	} else {
> +		lcdev = &led->isink;
> +		lcdev->brightness_set_blocking = mt6370_isnk_brightness_set;
> +		lcdev->blink_set = mt6370_isnk_blink_set;
> +		lcdev->pattern_set = mt6370_isnk_pattern_set;
> +		lcdev->pattern_clear = mt6370_isnk_pattern_clear;
> +	}
> +
> +	ret = fwnode_property_read_u32(init_data->fwnode, "led-max-microamp",
> +				       &max_uA);
> +	if (ret) {
> +		dev_warn(dev, "Not specified led-max-microamp, config to the minimum\n");
> +		max_uA = 0;
> +	}
> +
> +	if (led->index == MT6370_LED_ISNK4)
> +		sel_range = R_LED4_CURR;
> +	else
> +		sel_range = R_LED123_CURR;
> +
> +	linear_range_get_selector_within(priv->ranges + sel_range, max_uA,
> +					 &max_level);
> +
> +	lcdev->max_brightness = max_level;
> +
> +	led->default_state = led_init_default_state_get(init_data->fwnode);
> +
> +	return 0;
> +}
> +
> +static int mt6370_isnk_init_default_state(struct mt6370_led *led)
> +{
> +	struct mt6370_priv *priv = led->priv;
> +	unsigned int enable, level;
> +	int ret;
> +
> +	ret = mt6370_get_led_brightness(priv, led->index, &level);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_field_read(priv->fields[F_RGB_EN], &enable);
> +	if (ret)
> +		return ret;
> +
> +	if (!(enable & MT6370_CHEN_BIT(led->index)))
> +		level = 0;
> +
> +	switch (led->default_state) {
> +	case LEDS_DEFSTATE_ON:
> +		led->isink.brightness = led->isink.max_brightness;
> +		break;
> +	case LEDS_DEFSTATE_KEEP:
> +		led->isink.brightness = min(level, led->isink.max_brightness);
> +		break;
> +	default:
> +		led->isink.brightness = 0;
> +		break;
> +	}
> +
> +	return mt6370_isnk_brightness_set(&led->isink, led->isink.brightness);
> +}
> +
> +static int mt6370_led_register(struct device *dev, struct mt6370_led *led,
> +			       struct led_init_data *init_data)
> +{
> +	struct mt6370_priv *priv = led->priv;
> +	int ret;
> +
> +	if (led->index == MT6370_VIRTUAL_MULTICOLOR) {
> +		ret = mt6370_mc_brightness_set(&led->mc.led_cdev, 0);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Couldn't set multicolor brightness\n");
> +
> +		ret = devm_led_classdev_multicolor_register_ext(dev, &led->mc,
> +								init_data);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Couldn't register multicolor\n");
> +	} else {
> +		if (led->index == MT6370_LED_ISNK4) {
> +			ret = regmap_field_write(priv->fields[F_CHGIND_EN], 1);
> +			if (ret)
> +				return dev_err_probe(dev, ret, "Failed to set CHRIND to SW\n");
> +		}
> +
> +		ret = mt6370_isnk_init_default_state(led);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Failed to init %d isnk state\n",
> +					     led->index);
> +
> +		ret = devm_led_classdev_register_ext(dev, &led->isink,
> +						     init_data);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Couldn't register isink %d\n", led->index);
> +	}
> +
> +	return 0;
> +}
> +
> +static int mt6370_check_vendor_info(struct mt6370_priv *priv)
> +{
> +	unsigned int devinfo, vid;
> +	int ret;
> +
> +	ret = regmap_read(priv->regmap, MT6370_REG_DEV_INFO, &devinfo);
> +	if (ret)
> +		return ret;
> +
> +	vid = FIELD_GET(MT6370_VENID_MASK, devinfo);
> +	if (vid == 0x9 || vid == 0xb) {
> +		priv->reg_fields = mt6372_reg_fields;
> +		priv->ranges = mt6372_led_ranges;
> +		priv->pdata = &mt6372_pdata;
> +	} else {
> +		/* Common for MT6370/71 */
> +		priv->reg_fields = common_reg_fields;
> +		priv->ranges = common_led_ranges;
> +		priv->pdata = &common_pdata;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mt6370_leds_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct mt6370_priv *priv;
> +	struct fwnode_handle *child;
> +	size_t count;
> +	unsigned int i = 0;
> +	int ret;
> +
> +	count = device_get_child_node_count(dev);
> +	if (!count || count > MT6370_MAX_LEDS)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "No child node or node count over max LED number %zu\n",
> +				      count);
> +
> +	priv = devm_kzalloc(dev, struct_size(priv, leds, count), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->leds_count = count;
> +	mutex_init(&priv->lock);
> +
> +	priv->regmap = dev_get_regmap(dev->parent, NULL);
> +	if (!priv->regmap)
> +		return dev_err_probe(dev, -ENODEV, "Failed to get parent regmap\n");
> +
> +	ret = mt6370_check_vendor_info(priv);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to check vendor info\n");
> +
> +	ret = devm_regmap_field_bulk_alloc(dev, priv->regmap, priv->fields,
> +					   priv->reg_fields, F_MAX_FIELDS);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to allocate regmap field\n");
> +
> +	device_for_each_child_node(dev, child) {
> +		struct mt6370_led *led = priv->leds + i++;
> +		struct led_init_data init_data = { .fwnode = child };
> +		u32 reg, color;
> +
> +		ret = fwnode_property_read_u32(child, "reg", &reg);
> +		if (ret) {
> +			fwnode_handle_put(child);
> +			return dev_err_probe(dev, ret, "Failed to parse reg property\n");
> +		}
> +
> +		if (reg >= MT6370_MAX_LEDS) {
> +			fwnode_handle_put(child);
> +			return dev_err_probe(dev, -EINVAL, "Error reg property number\n");
> +		}
> +
> +		ret = fwnode_property_read_u32(child, "color", &color);
> +		if (ret) {
> +			fwnode_handle_put(child);
> +			return dev_err_probe(dev, ret, "Failed to parse color property\n");
> +		}
> +
> +		if (color == LED_COLOR_ID_RGB || color == LED_COLOR_ID_MULTI)
> +			reg = MT6370_VIRTUAL_MULTICOLOR;
> +
> +		if (priv->leds_active & BIT(reg)) {
> +			fwnode_handle_put(child);
> +			return dev_err_probe(dev, -EINVAL, "Duplicate reg property\n");
> +		}
> +
> +		priv->leds_active |= BIT(reg);
> +
> +		led->index = reg;
> +		led->priv = priv;
> +
> +		ret = mt6370_init_led_properties(dev, led, &init_data);
> +		if (ret) {
> +			fwnode_handle_put(child);
> +			return ret;
> +		}
> +
> +		ret = mt6370_led_register(dev, led, &init_data);
> +		if (ret) {
> +			fwnode_handle_put(child);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id mt6370_rgbled_device_table[] = {
> +	{ .compatible = "mediatek,mt6370-indicator" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, mt6370_rgbled_device_table);
> +
> +static struct platform_driver mt6370_rgbled_driver = {
> +	.driver = {
> +		.name = "mt6370-indicator",
> +		.of_match_table = mt6370_rgbled_device_table,
> +	},
> +	.probe = mt6370_leds_probe,
> +};
> +module_platform_driver(mt6370_rgbled_driver);
> +
> +MODULE_AUTHOR("Alice Chen <alice_chen@richtek.com>");
> +MODULE_AUTHOR("ChiYuan Huang <cy_huang@richtek.com>");
> +MODULE_DESCRIPTION("MediaTek MT6370 RGB LED Driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.7.4
> 

-- 
Lee Jones [李琼斯]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v16 2/2] leds: flash: mt6370: Add MediaTek MT6370 flashlight support

[Andy Shevchenko]

On Fri, Jan 27, 2023 at 10:20:12AM +0000, Lee Jones wrote:
> On Tue, 17 Jan 2023, ChiaEn Wu wrote:

> I see that you guys have been super helpful reviewing this in the past.
> 
> Have your queries now been addressed?

I do not see any obvious issues that makes me suffer, if anything left
it can be addressed in the followups.

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v16 1/2] leds: rgb: mt6370: Add MediaTek MT6370 current sink type LED Indicator support

[Andy Shevchenko]

On Fri, Jan 27, 2023 at 10:21:12AM +0000, Lee Jones wrote:
> On Tue, 17 Jan 2023, ChiaEn Wu wrote:

> I see that you guys have been super helpful reviewing this in the past.
>
> Have your queries now been addressed?

As per previous answer, not anything visible right now.

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v16 1/2] leds: rgb: mt6370: Add MediaTek MT6370 current sink type LED Indicator support

[Jacek Anaszewski]

Hi ChiaEn,

On 1/17/23 09:53, ChiaEn Wu wrote:
> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> The MediaTek MT6370 is a highly-integrated smart power management IC,
> which includes a single cell Li-Ion/Li-Polymer switching battery
> charger, a USB Type-C & Power Delivery (PD) controller, dual
> Flash LED current sources, a RGB LED driver, a backlight WLED driver,
> a display bias driver and a general LDO for portable devices.
> 
> Add support for the MediaTek MT6370 Current Sink Type LED Indicator
> driver. It can control four channels current-sink RGB LEDs with 3 modes:
> constant current, PWM, and breath mode.
> 
> Co-developed-by: Alice Chen <alice_chen@richtek.com>
> Signed-off-by: Alice Chen <alice_chen@richtek.com>
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> Signed-off-by: ChiaEn Wu <chiaen_wu@richtek.com>
> ---
> 
> v16
> - Remove blank line in the head text.
> - Move 'pwm_duty' to descrease the size of struct mt6370_pdata.
> - Move the field 'F_RGB_EN' write out from if/else in
>    'mt6370_isnk_brightness_set'.
> - Remove 'dev' in struct mt6370_priv.
> - Remove the proprety reading for 'linux,default-trigger', led core
>    already did it.
> - Change the module license from 'GPL v2' to 'GPL'
>    In the head text, already deslcred SPDX license as 'GPL-2.0-only'.
> ---
>   drivers/leds/rgb/Kconfig           |   13 +
>   drivers/leds/rgb/Makefile          |    1 +
>   drivers/leds/rgb/leds-mt6370-rgb.c | 1009 ++++++++++++++++++++++++++++++++++++
>   3 files changed, 1023 insertions(+)
>   create mode 100644 drivers/leds/rgb/leds-mt6370-rgb.c
[...]
> +static int mt6370_gen_breath_pattern(struct mt6370_priv *priv,
> +				     struct led_pattern *pattern, u32 len,
> +				     u8 *pattern_val, u32 val_len)
> +{
> +	enum mt6370_led_ranges sel_range;
> +	struct led_pattern *curr;
> +	unsigned int sel;
> +	u32 val = 0;
> +	int i;
> +
> +	if (len < P_MAX_PATTERNS && val_len < P_MAX_PATTERNS / 2)
> +		return -EINVAL;
> +
> +	/*
> +	 * Pattern list
> +	 * tr1:	 byte 0, b'[7: 4]
> +	 * tr2:	 byte 0, b'[3: 0]
> +	 * tf1:	 byte 1, b'[7: 4]
> +	 * tf2:	 byte 1, b'[3: 0]
> +	 * ton:	 byte 2, b'[7: 4]
> +	 * toff: byte 2, b'[3: 0]
> +	 */

Please provide documentation for the hardware pattern format.
Compare [0] and [1].

Otherwise looks good to me:

Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

[0] Documentation/leds/leds-sc27xx.rst
[1] Documentation/leds/leds-qcom-lpg.rst

-- 
Best regards,
Jacek Anaszewski

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v16 2/2] leds: flash: mt6370: Add MediaTek MT6370 flashlight support

[Jacek Anaszewski]

Hi ChiaEn,

On 1/17/23 09:53, ChiaEn Wu wrote:
> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> The MediaTek MT6370 is a highly-integrated smart power management IC,
> which includes a single cell Li-Ion/Li-Polymer switching battery
> charger, a USB Type-C & Power Delivery (PD) controller, dual Flash
> LED current sources, a RGB LED driver, a backlight WLED driver,
> a display bias driver and a general LDO for portable devices.
> 
> Add support for the MT6370 Flash LED driver. Flash LED in MT6370
> has 2 channels and support torch/strobe mode.
> 
> Co-developed-by: Alice Chen <alice_chen@richtek.com>
> Signed-off-by: Alice Chen <alice_chen@richtek.com>
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> Signed-off-by: ChiaEn Wu <chiaen_wu@richtek.com>
> ---

Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

-- 
Best regards,
Jacek Anaszewski

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel