| Message ID | 20220623115631.22209-1-peterwu.pub@gmail.com |
|---|---|
| Headers | show |
| Series | Add Mediatek MT6370 PMIC support | expand |
On Thu, Jun 23, 2022 at 07:56:22PM +0800, ChiaEn Wu wrote: > From: ChiYuan Huang <cy_huang@richtek.com> > > Add mt6370 backlight binding documentation. > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > --- > > v3 > - Rename "mediatek,bled-pwm-hys-input-threshold-steps" to > "mediatek,bled-pwm-hys-input-th-steps" > - Refine "bled-pwm-hys-input-th-steps", "bled-ovp-microvolt", > "bled-ocp-microamp" enum values > --- > .../leds/backlight/mediatek,mt6370-backlight.yaml | 92 ++++++++++++++++++++++ > 1 file changed, 92 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml > > diff --git a/Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml > new file mode 100644 > index 0000000..26563ae > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml > @@ -0,0 +1,92 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/backlight/mediatek,mt6370-backlight.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Mediatek MT6370 Backlight > + > +maintainers: > + - ChiaEn Wu <chiaen_wu@richtek.com> > + > +description: | > + This module is part of the MT6370 MFD device. > + The MT6370 Backlight WLED driver supports up to a 29V output voltage for > + 4 channels of 8 series WLEDs. Each channel supports up to 30mA of current > + capability with 2048 current steps (11 bits) in exponential or linear > + mapping curves. > + > +allOf: > + - $ref: common.yaml# > + > +properties: > + compatible: > + const: mediatek,mt6370-backlight > + > + default-brightness: > + minimum: 0 > + maximum: 2048 > + > + max-brightness: > + minimum: 0 > + maximum: 2048 > + > + enable-gpios: > + description: External backlight 'enable' pin > + maxItems: 1 > + > + mediatek,bled-pwm-enable: > + description: | > + Enable external PWM input for backlight dimming > + type: boolean > + > + mediatek,bled-pwm-hys-enable: > + description: | > + Enable the backlight input-hysteresis for PWM mode > + type: boolean > + > + mediatek,bled-pwm-hys-input-th-steps: > + $ref: /schemas/types.yaml#/definitions/uint8 > + enum: [1, 4, 16, 64] > + description: | > + The selection of the upper and lower bounds threshold of backlight > + PWM resolution. If we choose selection 64, the variation of PWM > + resolution needs over than 64 steps. more than? Thanks, Joe > + > + mediatek,bled-ovp-shutdown: > + description: | > + Enable the backlight shutdown when OVP level triggered > + type: boolean > + > + mediatek,bled-ovp-microvolt: > + enum: [17000000, 21000000, 25000000, 29000000] > + description: | > + Backlight OVP level selection. > + > + mediatek,bled-ocp-shutdown: > + description: | > + Enable the backlight shutdown when OCP level triggerred. > + type: boolean > + > + mediatek,bled-ocp-microamp: > + enum: [900000, 1200000, 1500000, 1800000] > + description: | > + Backlight OC level selection. > + > + mediatek,bled-channel-use: > + $ref: /schemas/types.yaml#/definitions/uint8 > + description: | > + Backlight LED channel to be used. > + Each bit mapping to: > + - 0: CH4 > + - 1: CH3 > + - 2: CH2 > + - 3: CH1 > + minimum: 1 > + maximum: 15 > + > +required: > + - compatible > + - mediatek,bled-channel-use > + > +additionalProperties: false > -- > 2.7.4 >
On Thu, 23 Jun 2022 19:56:17 +0800, ChiaEn Wu wrote: > From: ChiaEn Wu <chiaen_wu@richtek.com> > > This patch series add Mediatek MT6370 PMIC support. The 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. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next Thanks! [09/14] regulator: mt6370: Add mt6370 DisplayBias and VibLDO support commit: 8171c93bac1bf9e98269b2efb19ef4e6c4e55ed7 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
On Thu, Jun 23, 2022 at 1:59 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote: > > From: ChiYuan Huang <cy_huang@richtek.com> > > Add Mediatek MT6370 MFD support. ... > +config MFD_MT6370 > + tristate "Mediatek MT6370 SubPMIC" > + select MFD_CORE > + select REGMAP_I2C > + select REGMAP_IRQ > + depends on I2C > + help > + Say Y here to enable MT6370 SubPMIC functional support. > + It consists of a single cell battery charger with ADC monitoring, RGB > + LEDs, dual channel flashlight, WLED backlight driver, display bias > + voltage supply, one general purpose LDO, and the USB Type-C & PD > + controller complies with the latest USB Type-C and PD standards. What will be the module name in case it's chosen to be built as a module? ... > obj-$(CONFIG_INTEL_SOC_PMIC_BXTWC) += intel_soc_pmic_bxtwc.o > obj-$(CONFIG_INTEL_SOC_PMIC_CHTWC) += intel_soc_pmic_chtwc.o > obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI) += intel_soc_pmic_chtdc_ti.o > obj-$(CONFIG_MFD_MT6360) += mt6360-core.o > +obj-$(CONFIG_MFD_MT6370) += mt6370.o > mt6397-objs := mt6397-core.o mt6397-irq.o mt6358-irq.o > obj-$(CONFIG_MFD_MT6397) += mt6397.o > obj-$(CONFIG_INTEL_SOC_PMIC_MRFLD) += intel_soc_pmic_mrfld.o This whole bunch of drivers is in the wrong place in Makefile. https://lore.kernel.org/all/20220616182524.7956-2-andriy.shevchenko@linux.intel.com/ ... > +#define MT6370_REG_MAXADDR 0x1FF Wondering if (BIT(10) - 1) gives a better hint on how hardware limits this (so it will be clear it's 10-bit address). ... > +static int mt6370_check_vendor_info(struct mt6370_info *info) > +{ > + unsigned int devinfo; > + int ret; > + > + ret = regmap_read(info->regmap, MT6370_REG_DEV_INFO, &devinfo); > + if (ret) > + return ret; > + > + switch (FIELD_GET(MT6370_VENID_MASK, devinfo)) { > + case MT6370_VENID_RT5081: > + case MT6370_VENID_RT5081A: > + case MT6370_VENID_MT6370: > + case MT6370_VENID_MT6371: > + case MT6370_VENID_MT6372P: > + case MT6370_VENID_MT6372CP: return 0; > + break; > + default: > + dev_err(info->dev, "Unknown Vendor ID 0x%02x\n", devinfo); > + return -ENODEV; > + } > + > + return 0; ...and drop these two lines? > +} ... > + bank_idx = *(u8 *)reg_buf; > + bank_addr = *(u8 *)(reg_buf + 1); Why not const u8 *u8_buf = reg_buf; bank_idx = u8_buf[0]; bank_addr = u8_buf[1]; ? ... > + if (ret < 0) > + return ret; > + else if (ret != val_size) Redundant 'else'. > + return -EIO; ... > + bank_idx = *(u8 *)data; > + bank_addr = *(u8 *)(data + 1); As per above.
On 6/23/22 04:56, ChiaEn Wu wrote: > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index a49979f..a8c58c3 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -244,6 +244,17 @@ 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 const current, PWM, and breath mode. Spell out "constant" (if that is what "const" means here). ? > + Isink4 channel can also be used as a CHG_VIN power good indicator.
Hi ChiaEn! Thanks for your patch! On Thu, Jun 23, 2022 at 1:58 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote: > From: ChiYuan Huang <cy_huang@richtek.com> > > Add Mediatek MT6370 current sink type LED Indicator driver. > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> (...) > drivers/leds/Kconfig | 11 + > drivers/leds/Makefile | 1 + > drivers/leds/leds-mt6370.c | 989 +++++++++++++++++++++++++++++++++++++++++++++ There is a drivers/leds/flash subdirectory these days, put the driver in that directory instead. Yours, Linus Walleij
On Fri, Jun 24, 2022 at 8:23 AM Linus Walleij <linus.walleij@linaro.org> wrote: > Thanks for your patch! > > On Thu, Jun 23, 2022 at 1:58 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote: > > > From: ChiYuan Huang <cy_huang@richtek.com> > > > > Add Mediatek MT6370 current sink type LED Indicator driver. > > > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > (...) > > drivers/leds/Kconfig | 11 + > > drivers/leds/Makefile | 1 + > > drivers/leds/leds-mt6370.c | 989 +++++++++++++++++++++++++++++++++++++++++++++ > > There is a drivers/leds/flash subdirectory these days, put the driver > in that directory instead. Sorry I'm commenting on the wrong patch. I meant this one. Move that into drivers/leds/flash drivers/leds/flash/leds-mt6370-flash.c | 657 ++++++++++++ Yours, Linus Walleij
Hi Linus, Thank you for the comment. Linus Walleij <linus.walleij@linaro.org> 於 2022年6月24日 週五 下午2:25寫道： > > On Fri, Jun 24, 2022 at 8:23 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > Thanks for your patch! > > > > On Thu, Jun 23, 2022 at 1:58 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote: > > > > > From: ChiYuan Huang <cy_huang@richtek.com> > > > > > > Add Mediatek MT6370 current sink type LED Indicator driver. > > > > > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > > (...) > > > drivers/leds/Kconfig | 11 + > > > drivers/leds/Makefile | 1 + > > > drivers/leds/leds-mt6370.c | 989 +++++++++++++++++++++++++++++++++++++++++++++ > > > > There is a drivers/leds/flash subdirectory these days, put the driver > > in that directory instead. > > Sorry I'm commenting on the wrong patch. > > I meant this one. Move that into drivers/leds/flash > drivers/leds/flash/leds-mt6370-flash.c | 657 ++++++++++++ In next version, I'll use "leds: flash: ......" instead of "leds: flashlight: ......" in subject. May I confirm that the driver has already in the drivers/leds/flash, so I don’t have to move it in next version? Sincerely, Alice Chen
Hi Joe, Joe Simmons-Talbott <joetalbott@gmail.com> 於 2022年6月23日 週四 晚上9:17寫道： > > On Thu, Jun 23, 2022 at 07:56:22PM +0800, ChiaEn Wu wrote: > > From: ChiYuan Huang <cy_huang@richtek.com> > > > > Add mt6370 backlight binding documentation. > > > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > > --- > > > > v3 > > - Rename "mediatek,bled-pwm-hys-input-threshold-steps" to > > "mediatek,bled-pwm-hys-input-th-steps" > > - Refine "bled-pwm-hys-input-th-steps", "bled-ovp-microvolt", > > "bled-ocp-microamp" enum values > > --- > > .../leds/backlight/mediatek,mt6370-backlight.yaml | 92 ++++++++++++++++++++++ > > 1 file changed, 92 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml > > > > diff --git a/Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml > > new file mode 100644 > > index 0000000..26563ae > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml > > @@ -0,0 +1,92 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/leds/backlight/mediatek,mt6370-backlight.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Mediatek MT6370 Backlight > > + > > +maintainers: > > + - ChiaEn Wu <chiaen_wu@richtek.com> > > + > > +description: | > > + This module is part of the MT6370 MFD device. > > + The MT6370 Backlight WLED driver supports up to a 29V output voltage for > > + 4 channels of 8 series WLEDs. Each channel supports up to 30mA of current > > + capability with 2048 current steps (11 bits) in exponential or linear > > + mapping curves. > > + > > +allOf: > > + - $ref: common.yaml# > > + > > +properties: > > + compatible: > > + const: mediatek,mt6370-backlight > > + > > + default-brightness: > > + minimum: 0 > > + maximum: 2048 > > + > > + max-brightness: > > + minimum: 0 > > + maximum: 2048 > > + > > + enable-gpios: > > + description: External backlight 'enable' pin > > + maxItems: 1 > > + > > + mediatek,bled-pwm-enable: > > + description: | > > + Enable external PWM input for backlight dimming > > + type: boolean > > + > > + mediatek,bled-pwm-hys-enable: > > + description: | > > + Enable the backlight input-hysteresis for PWM mode > > + type: boolean > > + > > + mediatek,bled-pwm-hys-input-th-steps: > > + $ref: /schemas/types.yaml#/definitions/uint8 > > + enum: [1, 4, 16, 64] > > + description: | > > + The selection of the upper and lower bounds threshold of backlight > > + PWM resolution. If we choose selection 64, the variation of PWM > > + resolution needs over than 64 steps. > > more than? > > Thanks, > Joe > Thanks for your helpful comments! I will revise this in the next patch. Thanks! > > + > > + mediatek,bled-ovp-shutdown: > > + description: | > > + Enable the backlight shutdown when OVP level triggered > > + type: boolean > > + > > + mediatek,bled-ovp-microvolt: > > + enum: [17000000, 21000000, 25000000, 29000000] > > + description: | > > + Backlight OVP level selection. > > + > > + mediatek,bled-ocp-shutdown: > > + description: | > > + Enable the backlight shutdown when OCP level triggerred. > > + type: boolean > > + > > + mediatek,bled-ocp-microamp: > > + enum: [900000, 1200000, 1500000, 1800000] > > + description: | > > + Backlight OC level selection. > > + > > + mediatek,bled-channel-use: > > + $ref: /schemas/types.yaml#/definitions/uint8 > > + description: | > > + Backlight LED channel to be used. > > + Each bit mapping to: > > + - 0: CH4 > > + - 1: CH3 > > + - 2: CH2 > > + - 3: CH1 > > + minimum: 1 > > + maximum: 15 > > + > > +required: > > + - compatible > > + - mediatek,bled-channel-use > > + > > +additionalProperties: false > > -- > > 2.7.4 > > Best regards, ChiaEn Wu
Hi Andy, Thanks for your helpful comments! We have some questions below. Andy Shevchenko <andy.shevchenko@gmail.com> 於 2022年6月24日 週五 凌晨2:01寫道： > > On Thu, Jun 23, 2022 at 1:59 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote: > > > > From: ChiYuan Huang <cy_huang@richtek.com> > > > > Add Mediatek MT6370 MFD support. > > ... > > > +config MFD_MT6370 > > + tristate "Mediatek MT6370 SubPMIC" > > + select MFD_CORE > > + select REGMAP_I2C > > + select REGMAP_IRQ > > + depends on I2C > > + help > > + Say Y here to enable MT6370 SubPMIC functional support. > > + It consists of a single cell battery charger with ADC monitoring, RGB > > + LEDs, dual channel flashlight, WLED backlight driver, display bias > > + voltage supply, one general purpose LDO, and the USB Type-C & PD > > + controller complies with the latest USB Type-C and PD standards. > > What will be the module name in case it's chosen to be built as a module? OK, we will add related text in the next patch! Thanks! > > ... > > > obj-$(CONFIG_INTEL_SOC_PMIC_BXTWC) += intel_soc_pmic_bxtwc.o > > obj-$(CONFIG_INTEL_SOC_PMIC_CHTWC) += intel_soc_pmic_chtwc.o > > obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI) += intel_soc_pmic_chtdc_ti.o > > obj-$(CONFIG_MFD_MT6360) += mt6360-core.o > > +obj-$(CONFIG_MFD_MT6370) += mt6370.o > > mt6397-objs := mt6397-core.o mt6397-irq.o mt6358-irq.o > > obj-$(CONFIG_MFD_MT6397) += mt6397.o > > obj-$(CONFIG_INTEL_SOC_PMIC_MRFLD) += intel_soc_pmic_mrfld.o > > This whole bunch of drivers is in the wrong place in Makefile. > > https://lore.kernel.org/all/20220616182524.7956-2-andriy.shevchenko@linux.intel.com/ > hmm... So shall we need to cherry-pick your this patch first, then modify the Makefile before the next submission?? > ... > > > +#define MT6370_REG_MAXADDR 0x1FF > > Wondering if (BIT(10) - 1) gives a better hint on how hardware limits > this (so it will be clear it's 10-bit address). well... This "0x1FF" is just a virtual mapping value to map the max address of the PMU bank(0x1XX). So, I feel its means is different from using (BIT(10) - 1) here. > > ... > > > +static int mt6370_check_vendor_info(struct mt6370_info *info) > > +{ > > + unsigned int devinfo; > > + int ret; > > + > > + ret = regmap_read(info->regmap, MT6370_REG_DEV_INFO, &devinfo); > > + if (ret) > > + return ret; > > + > > + switch (FIELD_GET(MT6370_VENID_MASK, devinfo)) { > > + case MT6370_VENID_RT5081: > > + case MT6370_VENID_RT5081A: > > + case MT6370_VENID_MT6370: > > + case MT6370_VENID_MT6371: > > + case MT6370_VENID_MT6372P: > > + case MT6370_VENID_MT6372CP: > > return 0; > > > + break; > > + default: > > + dev_err(info->dev, "Unknown Vendor ID 0x%02x\n", devinfo); > > + return -ENODEV; > > + } > > + > > + return 0; > > ...and drop these two lines? OK! We will refine it in the next patch! > > > +} > > ... > > > + bank_idx = *(u8 *)reg_buf; > > + bank_addr = *(u8 *)(reg_buf + 1); > > Why not > > const u8 *u8_buf = reg_buf; > > bank_idx = u8_buf[0]; > bank_addr = u8_buf[1]; > > ? We will refine it in the next patch! Thanks! > > ... > > > + if (ret < 0) > > + return ret; > > + else if (ret != val_size) > > Redundant 'else'. I'm not quite sure what you mean, so I made the following changes first. ------------------------------------ if (ret < 0) return ret; if (ret != val_size) return -EIO; ------------------------------------ I don't know if it meets your expectations?? > > > + return -EIO; > > ... > > > + bank_idx = *(u8 *)data; > > + bank_addr = *(u8 *)(data + 1); > > As per above. > > -- > With Best Regards, > Andy Shevchenko Best regards, ChiaEn Wu
Hi Andy, Thanks for your helpful comments! Andy Shevchenko <andy.shevchenko@gmail.com> 於 2022年6月24日 週五 凌晨2:19寫道： > > On Thu, Jun 23, 2022 at 2:00 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote: > > > > From: ChiYuan Huang <cy_huang@richtek.com> > > > > Add mt6370 DisplayBias and VibLDO support. > > ... > > > +#include <linux/bits.h> > > +#include <linux/gpio/consumer.h> > > +#include <linux/interrupt.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > > +#include <linux/of.h> > > Any users of this? (See below) > > > +#include <linux/platform_device.h> > > +#include <linux/regmap.h> > > +#include <linux/regulator/driver.h> > > +#include <linux/regulator/machine.h> > > ... > > > +#define MT6370_LDO_MINUV 1600000 > > +#define MT6370_LDO_STPUV 200000 > > +#define MT6370_LDO_N_VOLT 13 > > +#define MT6370_DBVBOOST_MINUV 4000000 > > +#define MT6370_DBVBOOST_STPUV 50000 > > +#define MT6370_DBVBOOST_N_VOLT 45 > > +#define MT6370_DBVOUT_MINUV 4000000 > > +#define MT6370_DBVOUT_STPUV 50000 > > +#define MT6370_DBVOUT_N_VOLT 41 > > If UV is a unit suffix, make it _UV. > > ... > > > + .of_match = of_match_ptr("dsvbst"), > > Would it even be called / used if CONFIG_OF=n? We got a notification from Mark telling us that this patch has been applied to git. ( https://lore.kernel.org/linux-arm-kernel/165599931844.321775.8085559092337130067.b4-ty@kernel.org/ ) So, should we need to make any other changes in the next submission? > > ... > > > + .regulators_node = of_match_ptr("regulators"), > > Ditto. > > ... > > > + for (i = 0; i < ARRAY_SIZE(mt6370_irqs); i++) { > > + irq = platform_get_irq_byname(pdev, mt6370_irqs[i].name); > > + > > + rdev = priv->rdev[mt6370_irqs[i].rid]; > > + > > + ret = devm_request_threaded_irq(priv->dev, irq, NULL, > > + mt6370_irqs[i].handler, 0, > > + mt6370_irqs[i].name, rdev); > > + if (ret) { > > > + dev_err(priv->dev, > > + "Failed to register (%d) interrupt\n", i); > > + return ret; > > return dev_err_probe(...); ? > > > + } > > + } > > ... > > > + for (i = 0; i < MT6370_MAX_IDX; i++) { > > + rdev = devm_regulator_register(priv->dev, > > + mt6370_regulator_descs + i, > > + &cfg); > > + if (IS_ERR(rdev)) { > > > + dev_err(priv->dev, > > + "Failed to register (%d) regulator\n", i); > > + return PTR_ERR(rdev); > > return dev_err_probe(...); ? > > > + } > > + > > + priv->rdev[i] = rdev; > > + } > > ... > > > + if (!priv->regmap) { > > + dev_err(&pdev->dev, "Failed to init regmap\n"); > > + return -ENODEV; > > + } > > return dev_err_probe(...); > > -- > With Best Regards, > Andy Shevchenko Best regards, ChiaEn Wu
On 23/06/2022 13:56, ChiaEn Wu wrote: > From: ChiYuan Huang <cy_huang@richtek.com> > > Add Mediatek mt6370 current sink type LED indicator binding documentation. > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > --- > > v3 > - Use leds-class-multicolor.yaml instead of common.yaml. > - Split multi-led and led node. > - Add subdevice "led" in "multi-led". > --- > .../bindings/leds/mediatek,mt6370-indicator.yaml | 77 ++++++++++++++++++++++ > 1 file changed, 77 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml > > diff --git a/Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml b/Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml > new file mode 100644 > index 0000000..45030f3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml > @@ -0,0 +1,77 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/mediatek,mt6370-indicator.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: LED driver for MT6370 PMIC from MediaTek Integrated. > + > +maintainers: > + - Alice Chen <alice_chen@richtek.com> > + > +description: | > + This module is part of the MT6370 MFD device. > + Add MT6370 LED driver include 4-channel RGB LED support Register/PWM/Breath Mode > + > +allOf: > + - $ref: leds-class-multicolor.yaml# > + > +properties: > + compatible: > + const: mediatek,mt6370-indicator > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > +patternProperties: > + "^multi-led@[0-3]$": > + type: object Here as well unevaluatedProperties:false (on the type level) > + > + properties: > + reg: > + enum: [0, 1, 2, 3] > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + patternProperties: > + "^led@[0-2]$": > + type: object > + $ref: common.yaml# > + unevaluatedProperties: false > + > + required: > + - reg > + - color > + > + required: > + - reg > + - color > + - "#address-cells" > + - "#size-cells" > + > + "^led@[0-3]$": > + type: object > + $ref: common.yaml# > + unevaluatedProperties: false > + > + properties: > + reg: > + enum: [0, 1, 2, 3] > + > + required: > + - reg > + - color > + > +required: > + - compatible > + - "#address-cells" > + - "#size-cells" > + > +additionalProperties: false Best regards, Krzysztof
On 24/06/2022 12:35, Krzysztof Kozlowski wrote: > On 23/06/2022 13:56, ChiaEn Wu wrote: >> From: ChiYuan Huang <cy_huang@richtek.com> >> >> Add Mediatek mt6370 current sink type LED indicator binding documentation. >> >> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> >> --- >> >> v3 >> - Use leds-class-multicolor.yaml instead of common.yaml. >> - Split multi-led and led node. >> - Add subdevice "led" in "multi-led". >> --- >> .../bindings/leds/mediatek,mt6370-indicator.yaml | 77 ++++++++++++++++++++++ >> 1 file changed, 77 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml >> >> diff --git a/Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml b/Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml >> new file mode 100644 >> index 0000000..45030f3 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml >> @@ -0,0 +1,77 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/leds/mediatek,mt6370-indicator.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: LED driver for MT6370 PMIC from MediaTek Integrated. >> + >> +maintainers: >> + - Alice Chen <alice_chen@richtek.com> >> + >> +description: | >> + This module is part of the MT6370 MFD device. >> + Add MT6370 LED driver include 4-channel RGB LED support Register/PWM/Breath Mode >> + >> +allOf: >> + - $ref: leds-class-multicolor.yaml# >> + >> +properties: >> + compatible: >> + const: mediatek,mt6370-indicator >> + >> + "#address-cells": >> + const: 1 >> + >> + "#size-cells": >> + const: 0 >> + >> +patternProperties: >> + "^multi-led@[0-3]$": >> + type: object > > Here as well unevaluatedProperties:false (on the type level) Ah, no, it does not work currently. Your code looks good. Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On Thu, Jun 23, 2022 at 07:56:25PM +0800, ChiaEn Wu wrote: > --- /dev/null > +++ b/drivers/usb/typec/tcpm/tcpci_mt6370.c > @@ -0,0 +1,212 @@ > +// SPDX-License-Identifier: GPL-2.0+ Are you sure you mean "+" here? I have to ask, sorry. And no copyright line? Your company is ok with that, nice! :) thanks, greg k-h
On Fri, Jun 24, 2022 at 9:20 AM szuni chen <szunichen@gmail.com> wrote: > > I meant this one. Move that into drivers/leds/flash > > drivers/leds/flash/leds-mt6370-flash.c | 657 ++++++++++++ > > In next version, I'll use "leds: flash: ......" instead of "leds: > flashlight: ......" in subject. > May I confirm that the driver has already in the drivers/leds/flash, > so I don’t have to move it in next version? Yeah you're right, I am just writing wrong comments today, it is already correct. Sorry! Yours, Linus Walleij
On Fri, Jun 24, 2022 at 12:19 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote: > Andy Shevchenko <andy.shevchenko@gmail.com> 於 2022年6月24日 週五 凌晨2:01寫道： > > On Thu, Jun 23, 2022 at 1:59 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote: ... > > > obj-$(CONFIG_INTEL_SOC_PMIC_BXTWC) += intel_soc_pmic_bxtwc.o > > > obj-$(CONFIG_INTEL_SOC_PMIC_CHTWC) += intel_soc_pmic_chtwc.o > > > obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI) += intel_soc_pmic_chtdc_ti.o > > > obj-$(CONFIG_MFD_MT6360) += mt6360-core.o > > > +obj-$(CONFIG_MFD_MT6370) += mt6370.o > > > mt6397-objs := mt6397-core.o mt6397-irq.o mt6358-irq.o > > > obj-$(CONFIG_MFD_MT6397) += mt6397.o > > > obj-$(CONFIG_INTEL_SOC_PMIC_MRFLD) += intel_soc_pmic_mrfld.o > > > > This whole bunch of drivers is in the wrong place in Makefile. > > > > https://lore.kernel.org/all/20220616182524.7956-2-andriy.shevchenko@linux.intel.com/ > > hmm... So shall we need to cherry-pick your this patch first, > then modify the Makefile before the next submission?? I don't know what Lee's preferences are, but at least I have these options in mind: 1) wait until Lee applies my series; 2) take that single patch to your tree as a precursor. In the second case you will need to send the series with that patch as well. ... > > > +#define MT6370_REG_MAXADDR 0x1FF > > > > Wondering if (BIT(10) - 1) gives a better hint on how hardware limits > > this (so it will be clear it's 10-bit address). > > well... This "0x1FF" is just a virtual mapping value to map the max > address of the PMU bank(0x1XX). > So, I feel its means is different from using (BIT(10) - 1) here. Perhaps a comment then? ... > > > + if (ret < 0) > > > + return ret; > > > + else if (ret != val_size) > > > > Redundant 'else'. > > I'm not quite sure what you mean, so I made the following changes first. > ------------------------------------ > if (ret < 0) > return ret; > if (ret != val_size) > return -EIO; > ------------------------------------ > I don't know if it meets your expectations?? Yes. > > > + return -EIO;
From: ChiaEn Wu <chiaen_wu@richtek.com> This patch series add Mediatek MT6370 PMIC support. The 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. Among with this we took some changes and refined the device tree files to comply with DT specifications. Thank you, ChiaEn Wu --- Changes in v3: - Remove ADC ABI file, which is added in v2 Patch 7 - In patch 02/14: - Add items and remove maxItems of io-channels - Add io-channel-names and describe each item - Add "unevaluatedProperties: false" in "usb-otg-vbus-regulator" - Rename "enable-gpio" to "enable-gpios" in "usb-otg-vbus-regulator" - In patch 03/14: - Use leds-class-multicolor.yaml instead of common.yaml. - Split multi-led and led node. - Add subdevice "led" in "multi-led". - In patch 04/14: - Remove the description of enum. - In patch 05/14: - Rename "mediatek,bled-pwm-hys-input-threshold-steps" to "mediatek,bled-pwm-hys-input-th-steps" - Refine "bled-pwm-hys-input-th-steps", "bled-ovp-microvolt", "bled-ocp-microamp" enum values - In patch 06/14: - Use " in entire patchset - Refine ADC description - Rename "enable-gpio" to "enable-gpios" in "regualtor" - Change "/schemas/" to "../" in every reference of all MT6370 modules - In patch 07/14: - Refine Kconfig help text - Refine error message of unknown vendor ID in mt6370_check_vendor_info() - Refine return value handling of mt6370_regmap_read() - Refine all probe error by using dev_err_probe() - Refine "bank_idx" and "bank_addr" in mt6370_regmap_read() and mt6370_regmap_write() - Add "#define VENID*" and drop the comments in mt6370_check_vendor_info() - Drop "MFD" in MODULE_DESCRIPTION() - In patch 09/14: - Refine Kconfig help text - In patch 10/14: - Refine Kconfig help text - Refine all channel value in read_scale() a. current: uA --> mA b. voltage: uV --> mV c. temperature: degrees Celsius --> milli degrees Celsius - Add "default:" condition of switch statement in read_scale() and read_raw() - Add error message for reading ADC register failed - Add the comment for adc_lock - Add <linux/mod_devicetable.h> header file for struct of_device_id - Replace "adc" text with "ADC" in all of the error messages - In patch 12/14: - Refine the grammer of the Kconfig. - Change reg mode to the const current mode. - In patch 14/14: - Refine bool properties parsing (pwm-enable, ovp-shutdown, ocp-shutdown) in DT parsing function - Refine u32 and u8 properties parsing (pwm-hys-input-th-steps, ovp-microvolt, ocp-microamp), from using register value to using actual value - Refine error string of "channle-use" parsing failed - Refine Kconfig help text Changes in v2: - In patch 01/15: - Add "unevaluatedProperties: false". - Delete "DT bindings". - Refine the description to fit in 80 columns. - Skip the connector description. - In patch 02/15: - Refine items description of interrupt-name - Rename "usb-otg-vbus" to "usb-otg-vbus-regulator" - Add constraint properties for ADC - In patch 03/15: - Skip not useful description of "^(multi-)?led@[0-3]$" and reg. - Due to the dependency, remove the mention of mfd document directory. - Delete Soft-start property. In design aspect, we think soft-restart should always be enabled, our new chip has deleted the related setting register , also, we don’t allow user adjust this parameter in this chip. - Refine the commit message. - In patch 04/15: - Skip not useful description of "^led@[0-1]$" and reg. - Add apace after '#'. - Refine the commit message. - In patch 05/15: - Remove "binding documentation" in subject title - Refine description of mt6370 backlight binding document - Refine properties name(bled-pwm-hys-input-bit, bled-ovp-microvolt, bled-ocp-microamp) and their description - In patch 06/15: - Refine ADC and Regulator descriptions - Refine include header usage in example - Refine node name to generic node name("pmic@34") - Refine led example indentation - Refine license of mediatek,mt6370_adc.h - Rename the dts example from IRQ define to number. - Remove mediatek,mt6370.h - In patch 07/15: - Add ABI documentation for mt6370 non-standard ADC sysfs interfaces. - In patch 08/15: - Add all IRQ define into mt6370.c. - Refine include header usage - In patch 09/15: - No changes. - In patch 10/15: - Use 'gpiod_get_from_of_node' to replace 'fwnode_gpiod_get_index'. - In patch 11/15: - Refine Kconfig mt6370 help text - Refine mask&shift to FIELD_PREP() - Refine mutex lock name ("lock" -> "adc_lock") - Refine mt6370_adc_read_scale() - Refine mt6370_adc_read_offset() - Refine mt6370_channel_labels[] by using enum to index chan spec - Refine MT6370_ADC_CHAN() - Refine indio_dev->name - Remove useless include header files - In patch 12/15: - Refine mt6370_chg_otg_rdesc.of_match ("mt6370,otg-vbus" -> "usb-otg-vbus-regulator") to match DT binding - In patch 13/15: - Refine Kconfig description. - Remove include "linux/of.h" and use "linux/mod_devicetable.h". - Place a comma for the last element of the const unsigned int array. - Add a comment line for the mutex 'lock'. - In probe function, use 'dev_err_probe' in some judgement to reduce the LOC. - Refine include header usage. BIT/GENMASK -> linux/bits.h FIELD_GET -> linux/bitfield.h - In patch 14/15: - Add blank line. - Replace container_of() with to_mt6370_led() . - Refine description of ramping. - Refine the mt6370_init_common_properties function. - Refine the probe return. - In patch 15/15: - Refine MT6370 help text in Kconfig - Refine DT Parse function - Remove useless enum - Add comment for 6372 backward compatible in bl_update_status() and check_vendor_info() - Using dev_err_probe(); insteads dev_err()&return; in the probe() Alice Chen (2): dt-bindings: leds: Add Mediatek MT6370 flashlight leds: flashlight: mt6370: Add Mediatek MT6370 flashlight support ChiYuan Huang (8): dt-bindings: usb: Add Mediatek MT6370 TCPC dt-bindings: leds: mt6370: Add Mediatek mt6370 current sink type LED indicator dt-bindings: backlight: Add Mediatek MT6370 backlight dt-bindings: mfd: Add Mediatek MT6370 mfd: mt6370: Add Mediatek MT6370 support usb: typec: tcpci_mt6370: Add Mediatek MT6370 tcpci driver regulator: mt6370: Add mt6370 DisplayBias and VibLDO support leds: mt6370: Add Mediatek MT6370 current sink type LED Indicator support ChiaEn Wu (4): dt-bindings: power: supply: Add Mediatek MT6370 Charger iio: adc: mt6370: Add Mediatek MT6370 support power: supply: mt6370: Add Mediatek MT6370 charger driver video: backlight: mt6370: Add Mediatek MT6370 support .../leds/backlight/mediatek,mt6370-backlight.yaml | 92 ++ .../bindings/leds/mediatek,mt6370-flashlight.yaml | 41 + .../bindings/leds/mediatek,mt6370-indicator.yaml | 77 ++ .../devicetree/bindings/mfd/mediatek,mt6370.yaml | 280 +++++ .../power/supply/mediatek,mt6370-charger.yaml | 87 ++ .../bindings/usb/mediatek,mt6370-tcpc.yaml | 36 + drivers/iio/adc/Kconfig | 9 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/mt6370-adc.c | 275 +++++ drivers/leds/Kconfig | 11 + drivers/leds/Makefile | 1 + drivers/leds/flash/Kconfig | 9 + drivers/leds/flash/Makefile | 1 + drivers/leds/flash/leds-mt6370-flash.c | 657 ++++++++++++ drivers/leds/leds-mt6370.c | 989 +++++++++++++++++ drivers/mfd/Kconfig | 13 + drivers/mfd/Makefile | 1 + drivers/mfd/mt6370.c | 358 +++++++ drivers/power/supply/Kconfig | 11 + drivers/power/supply/Makefile | 1 + drivers/power/supply/mt6370-charger.c | 1132 ++++++++++++++++++++ drivers/regulator/Kconfig | 8 + drivers/regulator/Makefile | 1 + drivers/regulator/mt6370-regulator.c | 388 +++++++ drivers/usb/typec/tcpm/Kconfig | 8 + drivers/usb/typec/tcpm/Makefile | 1 + drivers/usb/typec/tcpm/tcpci_mt6370.c | 212 ++++ drivers/video/backlight/Kconfig | 9 + drivers/video/backlight/Makefile | 1 + drivers/video/backlight/mt6370-backlight.c | 346 ++++++ include/dt-bindings/iio/adc/mediatek,mt6370_adc.h | 18 + 31 files changed, 5074 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml create mode 100644 Documentation/devicetree/bindings/leds/mediatek,mt6370-flashlight.yaml create mode 100644 Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mt6370.yaml create mode 100644 Documentation/devicetree/bindings/power/supply/mediatek,mt6370-charger.yaml create mode 100644 Documentation/devicetree/bindings/usb/mediatek,mt6370-tcpc.yaml create mode 100644 drivers/iio/adc/mt6370-adc.c create mode 100644 drivers/leds/flash/leds-mt6370-flash.c create mode 100644 drivers/leds/leds-mt6370.c create mode 100644 drivers/mfd/mt6370.c create mode 100644 drivers/power/supply/mt6370-charger.c create mode 100644 drivers/regulator/mt6370-regulator.c create mode 100644 drivers/usb/typec/tcpm/tcpci_mt6370.c create mode 100644 drivers/video/backlight/mt6370-backlight.c create mode 100644 include/dt-bindings/iio/adc/mediatek,mt6370_adc.h