| Message ID | 20220715112607.591-12-peterwu.pub@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add MediaTek MT6370 PMIC support | expand |
Il 15/07/22 13:26, ChiaEn Wu ha scritto: > 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. > > In MediaTek MT6370, there are four channel current-sink RGB LEDs that > support hardware pattern for constant current, PWM, and breath mode. > Isink4 channel can also be used as a CHG_VIN power good indicator. > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > --- > drivers/leds/Kconfig | 14 + > drivers/leds/Makefile | 1 + > drivers/leds/leds-mt6370.c | 994 +++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 1009 insertions(+) > create mode 100644 drivers/leds/leds-mt6370.c > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index a49979f..71bacb5 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -244,6 +244,20 @@ config LEDS_MT6323 > This option enables support for on-chip LED drivers found on > Mediatek MT6323 PMIC. > > +config LEDS_MT6370_RGB > + tristate "LED Support for MediaTek MT6370 PMIC" > + depends on LEDS_CLASS > + 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.ko". > + > 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 4fd2f92..557be42 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -62,6 +62,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_MT6370_RGB) += leds-mt6370.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-mt6370.c b/drivers/leds/leds-mt6370.c > new file mode 100644 > index 0000000..1038232 > --- /dev/null > +++ b/drivers/leds/leds-mt6370.c > @@ -0,0 +1,994 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2022 Richtek Technology Corp. > + * > + * Author: Alice Chen <alice_chen@richtek.com> > + */ > + > +#include <linux/bitops.h> > +#include <linux/kernel.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> > + > +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 31 > +#define MT6372_PMW_DUTY 255 Please fix this typo: PMW -> PWM > + > +#define STATE_OFF 0 > +#define STATE_KEEP 1 > +#define STATE_ON 2 I propose, instead: enum mt6370_state { STATE_OFF = 0, STATE_KEEP, STATE_ON, STATE_MAX, }; > + > +struct mt6370_led { > + union { > + struct led_classdev isink; > + struct led_classdev_mc mc; > + }; > + struct mt6370_priv *priv; > + u32 default_state; > + u32 index; > +}; > + > +struct mt6370_priv { > + /* Per LED access lock */ > + struct mutex lock; > + struct device *dev; > + 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; > + unsigned int leds_count; > + unsigned int leds_active; > + bool is_mt6372; Instead of using a bool for that, you can enhance the flexibility of this driver (and also somewhat reduce cpu cycles) by using: struct mt6370_pdata *pdata; where a good definition of that structure would be... struct mt6370_pdata { const unsigned int *tfreq; unsigned int tfreq_len; u8 pwm_duty; u8 reg_rgb1_tr; /* * You can set this one to -1 on MT6372 to indicate that * this register does not exist (hence use that in function * mt6370_get_breath_reg_base() to replace priv->is_mt6372) */ s16 reg_rgb_chrind_tr; }; > + 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 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) > +{ > + enum mt6370_led_field sel_field; > + unsigned int divisor, ratio; > + > + divisor = priv->is_mt6372 ? MT6372_PMW_DUTY : MT6370_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; > + } > + > + return regmap_field_write(priv->fields[sel_field], ratio); > +} > + > +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 int mt6370_set_led_freq(struct mt6370_priv *priv, unsigned int led_no, > + unsigned int ton, unsigned int toff) > +{ > + enum mt6370_led_field sel_field; > + const unsigned int *tfreq; > + unsigned int tfreq_len, tsum; > + int i; > + ...so, if you have pdata, here you'll do: struct mt6370_pdata *pdata = priv->pdata; > + if (priv->is_mt6372) { > + tfreq = mt6372_tfreqs; > + tfreq_len = ARRAY_SIZE(mt6372_tfreqs); > + } else { > + tfreq = common_tfreqs; > + tfreq_len = ARRAY_SIZE(common_tfreqs); > + } > + > + tsum = ton + toff; > + > + if (tsum > tfreq[0] || tsum < tfreq[tfreq_len - 1]) if (tsum > pdata->tfreq[0] || tsum < pdata->tfreq[pdata->tfreq_len - 1]) ( etc etc etc ) > + return -EOPNOTSUPP; > + > + for (i = 0; i < tfreq_len; i++) { > + if (tsum >= tfreq[i]) > + break; > + } > + > + 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; > + } > + > + return regmap_field_write(priv->fields[sel_field], i); > +} > + > +static void mt6370_get_breath_reg_base(struct mt6370_priv *priv, > + unsigned int led_no, unsigned int *base) > +{ if (pdata->reg_rgb_chrind_tr < 0) { *base = pdata->reg_rgb1_tr + led_no * 3; return; } > + if (priv->is_mt6372) { > + *base = MT6372_REG_RGB1_TR + led_no * 3; > + return; > + } > + > + switch (led_no) { > + case MT6370_LED_ISNK1: > + case MT6370_LED_ISNK2: > + case MT6370_LED_ISNK3: > + *base = MT6370_REG_RGB1_TR + led_no * 3; > + break; > + default: > + *base = MT6370_REG_RGB_CHRIND_TR; > + } > +} > + > +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; > + u8 val[P_MAX_PATTERNS / 2] = {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); > + > + val[i / 2] |= sel << (4 * ((i + 1) % 2)); > + } > + > + memcpy(pattern_val, val, 3); > + 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; > + } > + > + 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; > + > + 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 == LED_OFF) { > + enable &= ~MT6370_CHEN_BIT(subled->channel); > + > + ret = mt6370_set_led_mode(priv, subled->channel, > + MT6370_LED_REG_MODE); > + if (ret) > + goto out; > + > + continue; > + } > + > + if (brightness == LED_OFF) { > + 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; > + } > + > + ret = regmap_field_write(priv->fields[F_RGB_EN], disable); > + if (ret) > + goto out; > + > + ret = regmap_field_write(priv->fields[F_RGB_EN], enable); > + > +out: > + 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; > + > + 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; > + > + ret = mt6370_set_led_freq(priv, subled->channel, *delay_on, > + *delay_off); > + if (ret) > + goto out; > + > + ret = mt6370_set_led_mode(priv, subled->channel, > + MT6370_LED_PWM_MODE); > + if (ret) > + goto out; > + } > + > + /* Toggle to make pattern timing the same */ > + ret = regmap_field_write(priv->fields[F_RGB_EN], disable); > + if (ret) > + goto out; > + > + ret = regmap_field_write(priv->fields[F_RGB_EN], enable); > + > +out: > + 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; > + > + ret = regmap_field_read(priv->fields[F_RGB_EN], &enable); > + if (ret) > + goto out; > + > + disable = enable; > + > + for (i = 0; i < mccdev->num_colors; i++) { > + subled = mccdev->subled_info + i; > + > + mt6370_get_breath_reg_base(priv, subled->channel, ®_base); > + disable &= ~MT6370_CHEN_BIT(subled->channel); > + > + ret = regmap_raw_write(priv->regmap, reg_base, params, > + sizeof(params)); > + if (ret) > + goto out; > + > + ret = mt6370_set_led_mode(priv, subled->channel, > + MT6370_LED_BREATH_MODE); > + if (ret) > + goto out; > + } > + > + /* Toggle to make pattern timing be the same */ > + ret = regmap_field_write(priv->fields[F_RGB_EN], disable); > + if (ret) > + goto out; > + > + ret = regmap_field_write(priv->fields[F_RGB_EN], enable); > + > +out: > + 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 = 0; > + > + 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; > + > + if (level == LED_OFF) { > + enable &= ~MT6370_CHEN_BIT(led->index); > + > + ret = mt6370_set_led_mode(priv, led->index, > + MT6370_LED_REG_MODE); > + if (ret) > + goto out; > + > + ret = regmap_field_write(priv->fields[F_RGB_EN], enable); > + } else { > + enable |= MT6370_CHEN_BIT(led->index); > + > + ret = mt6370_set_led_brightness(priv, led->index, level); > + if (ret) > + goto out; > + > + ret = regmap_field_write(priv->fields[F_RGB_EN], enable); > + } > + > +out: > + 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; > + > + ret = mt6370_set_led_freq(priv, led->index, *delay_on, *delay_off); > + if (ret) > + goto out; > + > + ret = mt6370_set_led_mode(priv, led->index, MT6370_LED_PWM_MODE); > + > +out: > + 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; > + > + mt6370_get_breath_reg_base(priv, led->index, ®_base); > + > + ret = regmap_raw_write(priv->regmap, reg_base, params, sizeof(params)); > + if (ret) > + goto out; > + > + ret = mt6370_set_led_mode(priv, led->index, MT6370_LED_BREATH_MODE); > + > +out: > + 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 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; > + const char * const states[] = { "off", "keep", "on" }; > + const char *stat_str; > + int ret; > + > + if (led->index == MT6370_VIRTUAL_MULTICOLOR) { > + struct mc_subled *sub_led; > + u32 num_color = 0; > + > + sub_led = devm_kzalloc(priv->dev, > + sizeof(*sub_led) * MC_CHANNEL_NUM, > + 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", ®); > + if (ret || reg > MT6370_LED_ISNK3 || > + priv->leds_active & BIT(reg)) > + return -EINVAL; > + > + ret = fwnode_property_read_u32(child, "color", &color); > + if (ret) { > + dev_err(priv->dev, > + "led %d, no color specified\n", > + led->index); > + return ret; > + } > + > + priv->leds_active |= BIT(reg); > + sub_led[num_color].color_index = color; > + sub_led[num_color].channel = reg; > + num_color++; > + } > + > + if (num_color < 2) { > + dev_err(priv->dev, > + "Multicolor must include 2 or more led channel\n"); > + return -EINVAL; > + } > + > + 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(priv->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; > + > + fwnode_property_read_string(init_data->fwnode, "linux,default-trigger", > + &lcdev->default_trigger); > + > + if (!fwnode_property_read_string(init_data->fwnode, "default-state", > + &stat_str)) { > + ret = match_string(states, ARRAY_SIZE(states), stat_str); > + if (ret < 0) > + ret = STATE_OFF; > + > + led->default_state = ret; > + } > + > + 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 = LED_OFF; > + > + switch (led->default_state) { > + case STATE_ON: > + led->isink.brightness = led->isink.max_brightness; > + break; > + case STATE_KEEP: > + led->isink.brightness = min(level, led->isink.max_brightness); > + break; > + default: > + led->isink.brightness = LED_OFF; > + } > + > + return mt6370_isnk_brightness_set(&led->isink, led->isink.brightness); > +} > + > +static int mt6370_led_register(struct device *parent, 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, LED_OFF); > + if (ret) { > + dev_err(parent, "Couldn't set multicolor brightness\n"); > + return ret; > + } > + > + ret = devm_led_classdev_multicolor_register_ext(parent, > + &led->mc, > + init_data); > + if (ret) { > + dev_err(parent, "Couldn't register multicolor\n"); > + return ret; > + } > + } else { > + if (led->index == MT6370_LED_ISNK4) { > + ret = regmap_field_write(priv->fields[F_CHGIND_EN], 1); > + if (ret) { > + dev_err(parent, "Failed to set CHRIND to SW\n"); > + return ret; > + } > + } > + > + ret = mt6370_isnk_init_default_state(led); > + if (ret) { > + dev_err(parent, "Failed to init %d isnk state\n", > + led->index); > + return ret; > + } > + > + ret = devm_led_classdev_register_ext(parent, &led->isink, > + init_data); > + if (ret) { > + dev_err(parent, "Couldn't register isink %d\n", > + led->index); > + return ret; > + } > + } > + > + 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->is_mt6372 = true; priv->pdata = &mt6372_pdata; > + } else { > + priv->reg_fields = common_reg_fields; > + priv->ranges = common_led_ranges; priv->pdata = &mt6370_pdata; > + } > + > + return 0; > +} Regards, Angelo
On Fri, Jul 15, 2022 at 2:40 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > Il 15/07/22 13:26, ChiaEn Wu ha scritto: ... > > +#define STATE_OFF 0 > > +#define STATE_KEEP 1 > > +#define STATE_ON 2 > > I propose, instead: > > enum mt6370_state { > STATE_OFF = 0, > STATE_KEEP, > STATE_ON, > STATE_MAX, Usually we don't put commas at the terminator entries. > };
On Fri, Jul 15, 2022 at 1:29 PM ChiaEn Wu <peterwu.pub@gmail.com> 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. > > In MediaTek MT6370, there are four channel current-sink RGB LEDs 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 so, the > + will be called "leds-mt6370.ko". No ".ko". Why did you ignore these comments? Please go and fix _everywhere_ in your series. It's basically the rule of thumb, if the reviewer gives a comment against an occurrence of something, go through entire series and check if there are other places like commented one and address them all. ... > + * Author: Alice Chen <alice_chen@richtek.com> Strange, the commit message doesn't have a corresponding SoB, why? ... > +#define MT6370_PWM_DUTY 31 > +#define MT6372_PMW_DUTY 255 Looks like these are limits by hardware? Check with the datasheet if (BIT(x) - 1) makes more sense here. ... > + 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; Missed break; > + } ... > + 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; Ditto. > + } ... > + switch (led_no) { > + case MT6370_LED_ISNK1: > + case MT6370_LED_ISNK2: > + case MT6370_LED_ISNK3: > + *base = MT6370_REG_RGB1_TR + led_no * 3; > + break; > + default: > + *base = MT6370_REG_RGB_CHRIND_TR; Ditto. It seems you dropped them for all switch-cases. It's not goot, please restore them back. > + } ... > + u8 val[P_MAX_PATTERNS / 2] = {0}; { } should suffice > + /* > + * 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); > + > + val[i / 2] |= sel << (4 * ((i + 1) % 2)); > + } > + > + memcpy(pattern_val, val, 3); > + return 0; > +} ... > +out: out_unlock: > + mutex_unlock(&priv->lock); > + > + return ret; ... > +out: Ditto. And so on. > + mutex_unlock(&priv->lock); > + > + return ret; ... > + sub_led = devm_kzalloc(priv->dev, > + sizeof(*sub_led) * MC_CHANNEL_NUM, > + GFP_KERNEL); NIH devm_kcalloc(). Also check if you really need zeroed data. > + if (!sub_led) > + return -ENOMEM; ... > + ret = fwnode_property_read_u32(child, "color", &color); > + if (ret) { > + dev_err(priv->dev, > + "led %d, no color specified\n", > + led->index); > + return ret; return dev_err_probe(...) ; ? Ditto for many places in your entire series. > + } ... > + priv = devm_kzalloc(&pdev->dev, > + struct_size(priv, leds, count), GFP_KERNEL); At least one parameter can be placed on the previous line. > + if (!priv) > + return -ENOMEM;
Hi! > 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. > > In MediaTek MT6370, there are four channel current-sink RGB LEDs that > support hardware pattern for constant current, PWM, and breath mode. > Isink4 channel can also be used as a CHG_VIN power good indicator. > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > index a49979f..71bacb5 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -244,6 +244,20 @@ config LEDS_MT6323 > This option enables support for on-chip LED drivers found on > Mediatek MT6323 PMIC. > > +config LEDS_MT6370_RGB > + tristate "LED Support for MediaTek MT6370 PMIC" > + depends on LEDS_CLASS > + 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 Should this go to leds/rgb directory, and should it depend on multicolor framework? Best regards, Pavel
On Fri, Jul 15, 2022 at 08:29:42PM +0200, Andy Shevchenko wrote: > On Fri, Jul 15, 2022 at 1:29 PM ChiaEn Wu <peterwu.pub@gmail.com> 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. > > > > In MediaTek MT6370, there are four channel current-sink RGB LEDs 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 > > so, the > > > + will be called "leds-mt6370.ko". > > No ".ko". > > Why did you ignore these comments? Please go and fix _everywhere_ in > your series. > It's basically the rule of thumb, if the reviewer gives a comment > against an occurrence of something, go through entire series and check > if there are other places like commented one and address them all. > > ... > > > + * Author: Alice Chen <alice_chen@richtek.com> > > Strange, the commit message doesn't have a corresponding SoB, why? > Yes, there're two authors Alice and me. I'll correct it in next. > ... > > > +#define MT6370_PWM_DUTY 31 > > +#define MT6372_PMW_DUTY 255 > > Looks like these are limits by hardware? > Check with the datasheet if (BIT(x) - 1) makes more sense here. > > ... > > > + 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; > > Missed break; > > > + } > > ... > > > + 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; > > Ditto. > > > + } > > ... > > > + switch (led_no) { > > + case MT6370_LED_ISNK1: > > + case MT6370_LED_ISNK2: > > + case MT6370_LED_ISNK3: > > + *base = MT6370_REG_RGB1_TR + led_no * 3; > > + break; > > + default: > > + *base = MT6370_REG_RGB_CHRIND_TR; > > Ditto. > It seems you dropped them for all switch-cases. It's not goot, please > restore them back. > > > + } > > ... > > > + u8 val[P_MAX_PATTERNS / 2] = {0}; > > { } should suffice > > In the above range selector, we use the 'logic or' to generate the pattern values. If to change it from '{0} to '{ }', is it correct? > > + /* > > + * 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); > > + > > + val[i / 2] |= sel << (4 * ((i + 1) % 2)); > > + } > > + > > + memcpy(pattern_val, val, 3); > > + return 0; > > +} > > ... > > > +out: > > out_unlock: > > > + mutex_unlock(&priv->lock); > > + > > + return ret; > > ... > > > +out: > > Ditto. And so on. > > > + mutex_unlock(&priv->lock); > > + > > + return ret; > > ... > > > + sub_led = devm_kzalloc(priv->dev, > > + sizeof(*sub_led) * MC_CHANNEL_NUM, > > + GFP_KERNEL); > > NIH devm_kcalloc(). Also check if you really need zeroed data. > Ok, and after the check, I also need to add one line to set the intensity to 0. > > + if (!sub_led) > > + return -ENOMEM; > > ... > > > + ret = fwnode_property_read_u32(child, "color", &color); > > + if (ret) { > > + dev_err(priv->dev, > > + "led %d, no color specified\n", > > + led->index); > > + return ret; > > return dev_err_probe(...) ; ? > > Ditto for many places in your entire series. > > > + } > > ... > > > + priv = devm_kzalloc(&pdev->dev, > > + struct_size(priv, leds, count), GFP_KERNEL); > > At least one parameter can be placed on the previous line. > > > + if (!priv) > > + return -ENOMEM; > > -- > With Best Regards, > Andy Shevchenko
ChiYuan Huang <u0084500@gmail.com> 於 2022年7月20日 週三 下午5:45寫道： > > On Fri, Jul 15, 2022 at 08:29:42PM +0200, Andy Shevchenko wrote: > > On Fri, Jul 15, 2022 at 1:29 PM ChiaEn Wu <peterwu.pub@gmail.com> 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. > > > > > > In MediaTek MT6370, there are four channel current-sink RGB LEDs 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 > > > > so, the > > > > > + will be called "leds-mt6370.ko". > > > > No ".ko". > > > > Why did you ignore these comments? Please go and fix _everywhere_ in > > your series. > > It's basically the rule of thumb, if the reviewer gives a comment > > against an occurrence of something, go through entire series and check > > if there are other places like commented one and address them all. > > > > ... > > > > > + * Author: Alice Chen <alice_chen@richtek.com> > > > > Strange, the commit message doesn't have a corresponding SoB, why? > > > Yes, there're two authors Alice and me. > I'll correct it in next. > > ... > > > > > +#define MT6370_PWM_DUTY 31 > > > +#define MT6372_PMW_DUTY 255 > > > > Looks like these are limits by hardware? > > Check with the datasheet if (BIT(x) - 1) makes more sense here. > > > > ... > > > > > + 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; > > > > Missed break; > > > > > + } > > > > ... > > > > > + 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; > > > > Ditto. > > > > > + } > > > > ... > > > > > + switch (led_no) { > > > + case MT6370_LED_ISNK1: > > > + case MT6370_LED_ISNK2: > > > + case MT6370_LED_ISNK3: > > > + *base = MT6370_REG_RGB1_TR + led_no * 3; > > > + break; > > > + default: > > > + *base = MT6370_REG_RGB_CHRIND_TR; > > > > Ditto. > > It seems you dropped them for all switch-cases. It's not goot, please > > restore them back. > > > > > + } > > > > ... > > > > > + u8 val[P_MAX_PATTERNS / 2] = {0}; > > > > { } should suffice > > > > > In the above range selector, we use the 'logic or' to generate the typo, it's 'below'. > pattern values. > > If to change it from '{0} to '{ }', is it correct? > > > + /* > > > + * 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); > > > + > > > + val[i / 2] |= sel << (4 * ((i + 1) % 2)); > > > + } > > > + > > > + memcpy(pattern_val, val, 3); > > > + return 0; > > > +} > > > > ... > > > > > +out: > > > > out_unlock: > > > > > + mutex_unlock(&priv->lock); > > > + > > > + return ret; > > > > ... > > > > > +out: > > > > Ditto. And so on. > > > > > + mutex_unlock(&priv->lock); > > > + > > > + return ret; > > > > ... > > > > > + sub_led = devm_kzalloc(priv->dev, > > > + sizeof(*sub_led) * MC_CHANNEL_NUM, > > > + GFP_KERNEL); > > > > NIH devm_kcalloc(). Also check if you really need zeroed data. > > > Ok, and after the check, I also need to add one line to set the intensity to 0. > > > + if (!sub_led) > > > + return -ENOMEM; > > > > ... > > > > > + ret = fwnode_property_read_u32(child, "color", &color); > > > + if (ret) { > > > + dev_err(priv->dev, > > > + "led %d, no color specified\n", > > > + led->index); > > > + return ret; > > > > return dev_err_probe(...) ; ? > > > > Ditto for many places in your entire series. > > > > > + } > > > > ... > > > > > + priv = devm_kzalloc(&pdev->dev, > > > + struct_size(priv, leds, count), GFP_KERNEL); > > > > At least one parameter can be placed on the previous line. > > > > > + if (!priv) > > > + return -ENOMEM; > > > > -- > > With Best Regards, > > Andy Shevchenko
ChiYuan Huang <u0084500@gmail.com> 於 2022年7月20日 週三 下午5:48寫道： > > ChiYuan Huang <u0084500@gmail.com> 於 2022年7月20日 週三 下午5:45寫道： > > > > On Fri, Jul 15, 2022 at 08:29:42PM +0200, Andy Shevchenko wrote: > > > On Fri, Jul 15, 2022 at 1:29 PM ChiaEn Wu <peterwu.pub@gmail.com> 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. > > > > > > > > In MediaTek MT6370, there are four channel current-sink RGB LEDs 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 > > > > > > so, the > > > > > > > + will be called "leds-mt6370.ko". > > > > > > No ".ko". > > > > > > Why did you ignore these comments? Please go and fix _everywhere_ in > > > your series. > > > It's basically the rule of thumb, if the reviewer gives a comment > > > against an occurrence of something, go through entire series and check > > > if there are other places like commented one and address them all. > > > > > > ... > > > > > > > + * Author: Alice Chen <alice_chen@richtek.com> > > > > > > Strange, the commit message doesn't have a corresponding SoB, why? > > > > > Yes, there're two authors Alice and me. > > I'll correct it in next. > > > ... > > > > > > > +#define MT6370_PWM_DUTY 31 > > > > +#define MT6372_PMW_DUTY 255 > > > > > > Looks like these are limits by hardware? > > > Check with the datasheet if (BIT(x) - 1) makes more sense here. > > > > > > ... > > > > > > > + 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; > > > > > > Missed break; > > > > > > > + } > > > > > > ... > > > > > > > + 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; > > > > > > Ditto. > > > > > > > + } > > > > > > ... > > > > > > > + switch (led_no) { > > > > + case MT6370_LED_ISNK1: > > > > + case MT6370_LED_ISNK2: > > > > + case MT6370_LED_ISNK3: > > > > + *base = MT6370_REG_RGB1_TR + led_no * 3; > > > > + break; > > > > + default: > > > > + *base = MT6370_REG_RGB_CHRIND_TR; > > > > > > Ditto. > > > It seems you dropped them for all switch-cases. It's not goot, please > > > restore them back. > > > > > > > + } > > > > > > ... > > > > > > > + u8 val[P_MAX_PATTERNS / 2] = {0}; > > > > > > { } should suffice > > > > > > > > In the above range selector, we use the 'logic or' to generate the > typo, it's 'below'. > > pattern values. > > Ah, found in c11 standard 6.7.9 item 21 It is the same as 'static storage duration'. I will follow your comment to revise it. Thanks. > > If to change it from '{0} to '{ }', is it correct? > > > > + /* > > > > + * 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); > > > > + > > > > + val[i / 2] |= sel << (4 * ((i + 1) % 2)); > > > > + } > > > > + > > > > + memcpy(pattern_val, val, 3); > > > > + return 0; > > > > +} > > > > > > ... > > > > > > > +out: > > > > > > out_unlock: > > > > > > > + mutex_unlock(&priv->lock); > > > > + > > > > + return ret; > > > > > > ... > > > > > > > +out: > > > > > > Ditto. And so on. > > > > > > > + mutex_unlock(&priv->lock); > > > > + > > > > + return ret; > > > > > > ... > > > > > > > + sub_led = devm_kzalloc(priv->dev, > > > > + sizeof(*sub_led) * MC_CHANNEL_NUM, > > > > + GFP_KERNEL); > > > > > > NIH devm_kcalloc(). Also check if you really need zeroed data. > > > > > Ok, and after the check, I also need to add one line to set the intensity to 0. > > > > + if (!sub_led) > > > > + return -ENOMEM; > > > > > > ... > > > > > > > + ret = fwnode_property_read_u32(child, "color", &color); > > > > + if (ret) { > > > > + dev_err(priv->dev, > > > > + "led %d, no color specified\n", > > > > + led->index); > > > > + return ret; > > > > > > return dev_err_probe(...) ; ? > > > > > > Ditto for many places in your entire series. > > > > > > > + } > > > > > > ... > > > > > > > + priv = devm_kzalloc(&pdev->dev, > > > > + struct_size(priv, leds, count), GFP_KERNEL); > > > > > > At least one parameter can be placed on the previous line. > > > > > > > + if (!priv) > > > > + return -ENOMEM; > > > > > > -- > > > With Best Regards, > > > Andy Shevchenko
On Sun, Jul 17, 2022 at 10:46:43AM +0200, Pavel Machek wrote: > Hi! > > > 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. > > > > In MediaTek MT6370, there are four channel current-sink RGB LEDs that > > support hardware pattern for constant current, PWM, and breath mode. > > Isink4 channel can also be used as a CHG_VIN power good indicator. > > > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > > > index a49979f..71bacb5 100644 > > --- a/drivers/leds/Kconfig > > +++ b/drivers/leds/Kconfig > > @@ -244,6 +244,20 @@ config LEDS_MT6323 > > This option enables support for on-chip LED drivers found on > > Mediatek MT6323 PMIC. > > > > +config LEDS_MT6370_RGB > > + tristate "LED Support for MediaTek MT6370 PMIC" > > + depends on LEDS_CLASS > > + 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 > > Should this go to leds/rgb directory, and should it depend on > multicolor framework? Yes, and I may also want to change the file name from 'leds-mt6370' to 'leds-mt6370-rgb'. Is it ok? > > Best regards, > Pavel > -- > People of Russia, stop Putin before his war on Ukraine escalates.
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index a49979f..71bacb5 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -244,6 +244,20 @@ config LEDS_MT6323 This option enables support for on-chip LED drivers found on Mediatek MT6323 PMIC. +config LEDS_MT6370_RGB + tristate "LED Support for MediaTek MT6370 PMIC" + depends on LEDS_CLASS + 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.ko". + 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 4fd2f92..557be42 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -62,6 +62,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_MT6370_RGB) += leds-mt6370.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-mt6370.c b/drivers/leds/leds-mt6370.c new file mode 100644 index 0000000..1038232 --- /dev/null +++ b/drivers/leds/leds-mt6370.c @@ -0,0 +1,994 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022 Richtek Technology Corp. + * + * Author: Alice Chen <alice_chen@richtek.com> + */ + +#include <linux/bitops.h> +#include <linux/kernel.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> + +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 31 +#define MT6372_PMW_DUTY 255 + +#define STATE_OFF 0 +#define STATE_KEEP 1 +#define STATE_ON 2 + +struct mt6370_led { + union { + struct led_classdev isink; + struct led_classdev_mc mc; + }; + struct mt6370_priv *priv; + u32 default_state; + u32 index; +}; + +struct mt6370_priv { + /* Per LED access lock */ + struct mutex lock; + struct device *dev; + 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; + unsigned int leds_count; + unsigned int leds_active; + bool is_mt6372; + 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 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) +{ + enum mt6370_led_field sel_field; + unsigned int divisor, ratio; + + divisor = priv->is_mt6372 ? MT6372_PMW_DUTY : MT6370_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; + } + + return regmap_field_write(priv->fields[sel_field], ratio); +} + +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 int mt6370_set_led_freq(struct mt6370_priv *priv, unsigned int led_no, + unsigned int ton, unsigned int toff) +{ + enum mt6370_led_field sel_field; + const unsigned int *tfreq; + unsigned int tfreq_len, tsum; + int i; + + if (priv->is_mt6372) { + tfreq = mt6372_tfreqs; + tfreq_len = ARRAY_SIZE(mt6372_tfreqs); + } else { + tfreq = common_tfreqs; + tfreq_len = ARRAY_SIZE(common_tfreqs); + } + + tsum = ton + toff; + + if (tsum > tfreq[0] || tsum < tfreq[tfreq_len - 1]) + return -EOPNOTSUPP; + + for (i = 0; i < tfreq_len; i++) { + if (tsum >= tfreq[i]) + break; + } + + 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; + } + + return regmap_field_write(priv->fields[sel_field], i); +} + +static void mt6370_get_breath_reg_base(struct mt6370_priv *priv, + unsigned int led_no, unsigned int *base) +{ + if (priv->is_mt6372) { + *base = MT6372_REG_RGB1_TR + led_no * 3; + return; + } + + switch (led_no) { + case MT6370_LED_ISNK1: + case MT6370_LED_ISNK2: + case MT6370_LED_ISNK3: + *base = MT6370_REG_RGB1_TR + led_no * 3; + break; + default: + *base = MT6370_REG_RGB_CHRIND_TR; + } +} + +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; + u8 val[P_MAX_PATTERNS / 2] = {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); + + val[i / 2] |= sel << (4 * ((i + 1) % 2)); + } + + memcpy(pattern_val, val, 3); + 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; + } + + 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; + + 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 == LED_OFF) { + enable &= ~MT6370_CHEN_BIT(subled->channel); + + ret = mt6370_set_led_mode(priv, subled->channel, + MT6370_LED_REG_MODE); + if (ret) + goto out; + + continue; + } + + if (brightness == LED_OFF) { + 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; + } + + ret = regmap_field_write(priv->fields[F_RGB_EN], disable); + if (ret) + goto out; + + ret = regmap_field_write(priv->fields[F_RGB_EN], enable); + +out: + 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; + + 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; + + ret = mt6370_set_led_freq(priv, subled->channel, *delay_on, + *delay_off); + if (ret) + goto out; + + ret = mt6370_set_led_mode(priv, subled->channel, + MT6370_LED_PWM_MODE); + if (ret) + goto out; + } + + /* Toggle to make pattern timing the same */ + ret = regmap_field_write(priv->fields[F_RGB_EN], disable); + if (ret) + goto out; + + ret = regmap_field_write(priv->fields[F_RGB_EN], enable); + +out: + 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; + + ret = regmap_field_read(priv->fields[F_RGB_EN], &enable); + if (ret) + goto out; + + disable = enable; + + for (i = 0; i < mccdev->num_colors; i++) { + subled = mccdev->subled_info + i; + + mt6370_get_breath_reg_base(priv, subled->channel, ®_base); + disable &= ~MT6370_CHEN_BIT(subled->channel); + + ret = regmap_raw_write(priv->regmap, reg_base, params, + sizeof(params)); + if (ret) + goto out; + + ret = mt6370_set_led_mode(priv, subled->channel, + MT6370_LED_BREATH_MODE); + if (ret) + goto out; + } + + /* Toggle to make pattern timing be the same */ + ret = regmap_field_write(priv->fields[F_RGB_EN], disable); + if (ret) + goto out; + + ret = regmap_field_write(priv->fields[F_RGB_EN], enable); + +out: + 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 = 0; + + 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; + + if (level == LED_OFF) { + enable &= ~MT6370_CHEN_BIT(led->index); + + ret = mt6370_set_led_mode(priv, led->index, + MT6370_LED_REG_MODE); + if (ret) + goto out; + + ret = regmap_field_write(priv->fields[F_RGB_EN], enable); + } else { + enable |= MT6370_CHEN_BIT(led->index); + + ret = mt6370_set_led_brightness(priv, led->index, level); + if (ret) + goto out; + + ret = regmap_field_write(priv->fields[F_RGB_EN], enable); + } + +out: + 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; + + ret = mt6370_set_led_freq(priv, led->index, *delay_on, *delay_off); + if (ret) + goto out; + + ret = mt6370_set_led_mode(priv, led->index, MT6370_LED_PWM_MODE); + +out: + 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; + + mt6370_get_breath_reg_base(priv, led->index, ®_base); + + ret = regmap_raw_write(priv->regmap, reg_base, params, sizeof(params)); + if (ret) + goto out; + + ret = mt6370_set_led_mode(priv, led->index, MT6370_LED_BREATH_MODE); + +out: + 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 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; + const char * const states[] = { "off", "keep", "on" }; + const char *stat_str; + int ret; + + if (led->index == MT6370_VIRTUAL_MULTICOLOR) { + struct mc_subled *sub_led; + u32 num_color = 0; + + sub_led = devm_kzalloc(priv->dev, + sizeof(*sub_led) * MC_CHANNEL_NUM, + 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", ®); + if (ret || reg > MT6370_LED_ISNK3 || + priv->leds_active & BIT(reg)) + return -EINVAL; + + ret = fwnode_property_read_u32(child, "color", &color); + if (ret) { + dev_err(priv->dev, + "led %d, no color specified\n", + led->index); + return ret; + } + + priv->leds_active |= BIT(reg); + sub_led[num_color].color_index = color; + sub_led[num_color].channel = reg; + num_color++; + } + + if (num_color < 2) { + dev_err(priv->dev, + "Multicolor must include 2 or more led channel\n"); + return -EINVAL; + } + + 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(priv->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; + + fwnode_property_read_string(init_data->fwnode, "linux,default-trigger", + &lcdev->default_trigger); + + if (!fwnode_property_read_string(init_data->fwnode, "default-state", + &stat_str)) { + ret = match_string(states, ARRAY_SIZE(states), stat_str); + if (ret < 0) + ret = STATE_OFF; + + led->default_state = ret; + } + + 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 = LED_OFF; + + switch (led->default_state) { + case STATE_ON: + led->isink.brightness = led->isink.max_brightness; + break; + case STATE_KEEP: + led->isink.brightness = min(level, led->isink.max_brightness); + break; + default: + led->isink.brightness = LED_OFF; + } + + return mt6370_isnk_brightness_set(&led->isink, led->isink.brightness); +} + +static int mt6370_led_register(struct device *parent, 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, LED_OFF); + if (ret) { + dev_err(parent, "Couldn't set multicolor brightness\n"); + return ret; + } + + ret = devm_led_classdev_multicolor_register_ext(parent, + &led->mc, + init_data); + if (ret) { + dev_err(parent, "Couldn't register multicolor\n"); + return ret; + } + } else { + if (led->index == MT6370_LED_ISNK4) { + ret = regmap_field_write(priv->fields[F_CHGIND_EN], 1); + if (ret) { + dev_err(parent, "Failed to set CHRIND to SW\n"); + return ret; + } + } + + ret = mt6370_isnk_init_default_state(led); + if (ret) { + dev_err(parent, "Failed to init %d isnk state\n", + led->index); + return ret; + } + + ret = devm_led_classdev_register_ext(parent, &led->isink, + init_data); + if (ret) { + dev_err(parent, "Couldn't register isink %d\n", + led->index); + return ret; + } + } + + 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->is_mt6372 = true; + } else { + priv->reg_fields = common_reg_fields; + priv->ranges = common_led_ranges; + } + + return 0; +} + +static int mt6370_leds_probe(struct platform_device *pdev) +{ + struct mt6370_priv *priv; + struct fwnode_handle *child; + size_t count; + int i = 0, ret; + + count = device_get_child_node_count(&pdev->dev); + if (!count || count > MT6370_MAX_LEDS) { + dev_err(&pdev->dev, + "No child node or node count over max led number %zu\n", + count); + return -EINVAL; + } + + priv = devm_kzalloc(&pdev->dev, + struct_size(priv, leds, count), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->leds_count = count; + priv->dev = &pdev->dev; + mutex_init(&priv->lock); + + priv->regmap = dev_get_regmap(pdev->dev.parent, NULL); + if (!priv->regmap) + return dev_err_probe(&pdev->dev, -ENODEV, + "Failed to get parent regmap\n"); + + ret = mt6370_check_vendor_info(priv); + if (ret) { + dev_err(&pdev->dev, "Failed to check vendor info (%d)\n", ret); + return ret; + } + + ret = devm_regmap_field_bulk_alloc(&pdev->dev, priv->regmap, + priv->fields, priv->reg_fields, + F_MAX_FIELDS); + if (ret) + return dev_err_probe(&pdev->dev, ret, + "Failed to allocate regmap field\n"); + + device_for_each_child_node(&pdev->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", ®); + if (ret) + return dev_err_probe(&pdev->dev, ret, + "Failed to parse reg property\n"); + + if (reg >= MT6370_MAX_LEDS) + return dev_err_probe(&pdev->dev, -EINVAL, + "Error reg property number\n"); + + ret = fwnode_property_read_u32(child, "color", &color); + if (ret) { + dev_err(&pdev->dev, "Failed to parse color property\n"); + return ret; + } + + if (color == LED_COLOR_ID_RGB || color == LED_COLOR_ID_MULTI) + reg = MT6370_VIRTUAL_MULTICOLOR; + + if (priv->leds_active & BIT(reg)) + return dev_err_probe(&pdev->dev, -EINVAL, + "Duplicat reg property\n"); + + priv->leds_active |= BIT(reg); + + led->index = reg; + led->priv = priv; + + ret = mt6370_init_led_properties(led, &init_data); + if (ret) + return ret; + + ret = mt6370_led_register(&pdev->dev, led, &init_data); + if (ret) + 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_DESCRIPTION("MediaTek MT6370 RGB Led Driver"); +MODULE_LICENSE("GPL v2");