diff mbox series

[v2,15/15] video: backlight: mt6370: Add Mediatek MT6370 support

Message ID 20220613111146.25221-16-peterwu.pub@gmail.com (mailing list archive)
State Superseded
Headers show
Series Add Mediatek MT6370 PMIC support | expand

Commit Message

ChiaEn Wu June 13, 2022, 11:11 a.m. UTC
From: ChiaEn Wu <chiaen_wu@richtek.com>

Add Mediatek MT6370 Backlight support.

Signed-off-by: ChiaEn Wu <chiaen_wu@richtek.com>
---
 drivers/video/backlight/Kconfig            |   9 +
 drivers/video/backlight/Makefile           |   1 +
 drivers/video/backlight/mt6370-backlight.c | 339 +++++++++++++++++++++
 3 files changed, 349 insertions(+)
 create mode 100644 drivers/video/backlight/mt6370-backlight.c

Comments

Daniel Thompson June 13, 2022, 5:08 p.m. UTC | #1
On Mon, Jun 13, 2022 at 07:11:46PM +0800, ChiaEn Wu wrote:
> +static int mt6370_init_backlight_properties(struct mt6370_priv *priv,
> +					    struct backlight_properties *props)

Most of the changes in this version looks good... but it looks the new
code in this function has a number of problems. See below...


> +{
> +	struct device *dev = priv->dev;
> +	u8 prop_val;
> +	u32 brightness;
> +	unsigned int mask, val;
> +	int ret;
> +
> +	/* Vendor optional properties
> +	 * if property not exist, keep value in default.
> +	 */

That's not the right strategy for booleans. Not existing means false
(e.g. flags should actively be unset).


> +	if (device_property_read_bool(dev, "mediatek,bled-pwm-enable")) {
> +		ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_PWM,
> +					 MT6370_BL_PWM_EN_MASK,
> +					 MT6370_BL_PWM_EN_MASK);
> +		if (ret)
> +			return ret;
> +	}

As above comment... all of the boolean properties are now being read
incorrectly.


> +
> +	if (device_property_read_bool(dev, "mediatek,bled-pwm-hys-enable")) {
> +		ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_PWM,
> +					 MT6370_BL_PWM_HYS_EN_MASK,
> +					 MT6370_BL_PWM_HYS_EN_MASK);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = device_property_read_u8(dev, "mediatek,bled-pwm-hys-input-bit",
> +				      &prop_val);
> +	if (!ret) {
> +		val = min_t(u8, prop_val, 3)
> +		      << (ffs(MT6370_BL_PWM_HYS_SEL_MASK) - 1);
> +		ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_PWM,
> +					 MT6370_BL_PWM_HYS_SEL_MASK, val);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = device_property_read_u8(dev, "mediatek,bled-ovp-microvolt",
> +				      &prop_val);
> +	if (!ret) {
> +		val = min_t(u8, prop_val, 3)
> +		      << (ffs(MT6370_BL_OVP_SEL_MASK) - 1);

This has been renamed but still seems to the using 0, 1, 2, 3 rather
than an actual value in microvolts.


> +		ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_BSTCTRL,
> +					 MT6370_BL_OVP_SEL_MASK, val);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (device_property_read_bool(dev, "mediatek,bled-ovp-shutdown")) {
> +		ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_BSTCTRL,
> +					 MT6370_BL_OVP_EN_MASK,
> +					 MT6370_BL_OVP_EN_MASK);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = device_property_read_u8(dev, "mediatek,bled-ocp-microamp",
> +				      &prop_val);
> +	if (!ret) {
> +		val = min_t(u8, prop_val, 3)
> +		      << (ffs(MT6370_BL_OC_SEL_MASK) - 1);

Likewise, should this be accepting a value in microamps?


> +		ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_BSTCTRL,
> +					 MT6370_BL_OC_SEL_MASK, val);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (device_property_read_bool(dev, "mediatek,bled-ocp-shutdown")) {
> +		ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_BSTCTRL,
> +					 MT6370_BL_OC_EN_MASK,
> +					 MT6370_BL_OC_EN_MASK);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* Common properties */
> +	ret = device_property_read_u32(dev, "max-brightness", &brightness);
> +	if (ret)
> +		brightness = MT6370_BL_MAX_BRIGHTNESS;
> +
> +	props->max_brightness = min_t(u32, brightness,
> +				      MT6370_BL_MAX_BRIGHTNESS);
> +
> +	ret = device_property_read_u32(dev, "default-brightness", &brightness);
> +	if (ret)
> +		brightness = props->max_brightness;
> +
> +	props->brightness = min_t(u32, brightness, props->max_brightness);
> +
> +
> +	ret = device_property_read_u8(dev, "mediatek,bled-channel-use",
> +				      &prop_val);
> +	if (ret) {
> +		dev_err(dev, "mediatek,bled-channel-use DT property missing\n");
> +		return ret;
> +	}
> +
> +	if (!prop_val || prop_val > MT6370_BL_MAX_CH) {
> +		dev_err(dev, "No channel specified (ch_val:%d)\n", prop_val);

Error string has not been updated to match condition that triggers it.


> +		return -EINVAL;
> +	}


Daniel.
Randy Dunlap June 13, 2022, 8:21 p.m. UTC | #2
On 6/13/22 04:11, ChiaEn Wu wrote:
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index a003e02e13ce..ec1589ad88bb 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -268,6 +268,15 @@ config BACKLIGHT_MAX8925
>  	  If you have a LCD backlight connected to the WLED output of MAX8925
>  	  WLED output, say Y here to enable this driver.
>  
> +config BACKLIGHT_MT6370
> +	tristate "Mediatek MT6370 Backlight Driver"
> +	depends on MFD_MT6370
> +	help
> +	  This enables support for Mediatek MT6370 Backlight driver.
> +	  It's commonly used to drive the display WLED. There're 4 channels

	                                                There are 4 channels

> +	  inisde, and each channel supports up to 30mA of current capability

	  inside,

> +	  with 2048 current steps in exponential or linear mapping curves.
ChiaEn Wu June 17, 2022, 9:34 a.m. UTC | #3
Hi Daniel,

Thanks for your helpful feedback!

Daniel Thompson <daniel.thompson@linaro.org> 於 2022年6月14日 週二 凌晨1:08寫道:
>
> On Mon, Jun 13, 2022 at 07:11:46PM +0800, ChiaEn Wu wrote:
> > +static int mt6370_init_backlight_properties(struct mt6370_priv *priv,
> > +                                         struct backlight_properties *props)
>
> Most of the changes in this version looks good... but it looks the new
> code in this function has a number of problems. See below...
>
>
> > +{
> > +     struct device *dev = priv->dev;
> > +     u8 prop_val;
> > +     u32 brightness;
> > +     unsigned int mask, val;
> > +     int ret;
> > +
> > +     /* Vendor optional properties
> > +      * if property not exist, keep value in default.
> > +      */
>
> That's not the right strategy for booleans. Not existing means false
> (e.g. flags should actively be unset).
>

I am so sorry for making these mistakes...
I will try to refine them in the right strategy in the next patch!

>
> > +     if (device_property_read_bool(dev, "mediatek,bled-pwm-enable")) {
> > +             ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_PWM,
> > +                                      MT6370_BL_PWM_EN_MASK,
> > +                                      MT6370_BL_PWM_EN_MASK);
> > +             if (ret)
> > +                     return ret;
> > +     }
>
> As above comment... all of the boolean properties are now being read
> incorrectly.
>
>
> > +
> > +     if (device_property_read_bool(dev, "mediatek,bled-pwm-hys-enable")) {
> > +             ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_PWM,
> > +                                      MT6370_BL_PWM_HYS_EN_MASK,
> > +                                      MT6370_BL_PWM_HYS_EN_MASK);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     ret = device_property_read_u8(dev, "mediatek,bled-pwm-hys-input-bit",
> > +                                   &prop_val);
> > +     if (!ret) {
> > +             val = min_t(u8, prop_val, 3)
> > +                   << (ffs(MT6370_BL_PWM_HYS_SEL_MASK) - 1);
> > +             ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_PWM,
> > +                                      MT6370_BL_PWM_HYS_SEL_MASK, val);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     ret = device_property_read_u8(dev, "mediatek,bled-ovp-microvolt",
> > +                                   &prop_val);
> > +     if (!ret) {
> > +             val = min_t(u8, prop_val, 3)
> > +                   << (ffs(MT6370_BL_OVP_SEL_MASK) - 1);
>
> This has been renamed but still seems to the using 0, 1, 2, 3 rather
> than an actual value in microvolts.

I’m so sorry for using the not actual value in microvolts and microamps.
I will refine these mistakes along with DT in the next patch. Thank you!

>
>
> > +             ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_BSTCTRL,
> > +                                      MT6370_BL_OVP_SEL_MASK, val);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     if (device_property_read_bool(dev, "mediatek,bled-ovp-shutdown")) {
> > +             ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_BSTCTRL,
> > +                                      MT6370_BL_OVP_EN_MASK,
> > +                                      MT6370_BL_OVP_EN_MASK);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     ret = device_property_read_u8(dev, "mediatek,bled-ocp-microamp",
> > +                                   &prop_val);
> > +     if (!ret) {
> > +             val = min_t(u8, prop_val, 3)
> > +                   << (ffs(MT6370_BL_OC_SEL_MASK) - 1);
>
> Likewise, should this be accepting a value in microamps?
>
>
> > +             ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_BSTCTRL,
> > +                                      MT6370_BL_OC_SEL_MASK, val);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     if (device_property_read_bool(dev, "mediatek,bled-ocp-shutdown")) {
> > +             ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_BSTCTRL,
> > +                                      MT6370_BL_OC_EN_MASK,
> > +                                      MT6370_BL_OC_EN_MASK);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     /* Common properties */
> > +     ret = device_property_read_u32(dev, "max-brightness", &brightness);
> > +     if (ret)
> > +             brightness = MT6370_BL_MAX_BRIGHTNESS;
> > +
> > +     props->max_brightness = min_t(u32, brightness,
> > +                                   MT6370_BL_MAX_BRIGHTNESS);
> > +
> > +     ret = device_property_read_u32(dev, "default-brightness", &brightness);
> > +     if (ret)
> > +             brightness = props->max_brightness;
> > +
> > +     props->brightness = min_t(u32, brightness, props->max_brightness);
> > +
> > +
> > +     ret = device_property_read_u8(dev, "mediatek,bled-channel-use",
> > +                                   &prop_val);
> > +     if (ret) {
> > +             dev_err(dev, "mediatek,bled-channel-use DT property missing\n");
> > +             return ret;
> > +     }
> > +
> > +     if (!prop_val || prop_val > MT6370_BL_MAX_CH) {
> > +             dev_err(dev, "No channel specified (ch_val:%d)\n", prop_val);
>
> Error string has not been updated to match condition that triggers it.
>

I will refine this wrong error string in the next patch, thanks!

>
> > +             return -EINVAL;
> > +     }
>
>
> Daniel.

Best regards,
ChiaEn Wu
diff mbox series

Patch

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index a003e02e13ce..ec1589ad88bb 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -268,6 +268,15 @@  config BACKLIGHT_MAX8925
 	  If you have a LCD backlight connected to the WLED output of MAX8925
 	  WLED output, say Y here to enable this driver.
 
+config BACKLIGHT_MT6370
+	tristate "Mediatek MT6370 Backlight Driver"
+	depends on MFD_MT6370
+	help
+	  This enables support for Mediatek MT6370 Backlight driver.
+	  It's commonly used to drive the display WLED. There're 4 channels
+	  inisde, and each channel supports up to 30mA of current capability
+	  with 2048 current steps in exponential or linear mapping curves.
+
 config BACKLIGHT_APPLE
 	tristate "Apple Backlight Driver"
 	depends on X86 && ACPI
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index cae2c83422ae..e815f3f1deff 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -44,6 +44,7 @@  obj-$(CONFIG_BACKLIGHT_LP855X)		+= lp855x_bl.o
 obj-$(CONFIG_BACKLIGHT_LP8788)		+= lp8788_bl.o
 obj-$(CONFIG_BACKLIGHT_LV5207LP)	+= lv5207lp.o
 obj-$(CONFIG_BACKLIGHT_MAX8925)		+= max8925_bl.o
+obj-$(CONFIG_BACKLIGHT_MT6370)		+= mt6370-backlight.o
 obj-$(CONFIG_BACKLIGHT_OMAP1)		+= omap1_bl.o
 obj-$(CONFIG_BACKLIGHT_PANDORA)		+= pandora_bl.o
 obj-$(CONFIG_BACKLIGHT_PCF50633)	+= pcf50633-backlight.o
diff --git a/drivers/video/backlight/mt6370-backlight.c b/drivers/video/backlight/mt6370-backlight.c
new file mode 100644
index 000000000000..a443c677cb7e
--- /dev/null
+++ b/drivers/video/backlight/mt6370-backlight.c
@@ -0,0 +1,339 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/backlight.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define MT6370_REG_DEV_INFO		0x100
+#define MT6370_REG_BL_EN		0x1A0
+#define MT6370_REG_BL_BSTCTRL		0x1A1
+#define MT6370_REG_BL_PWM		0x1A2
+#define MT6370_REG_BL_DIM2		0x1A4
+
+#define MT6370_VENID_MASK		GENMASK(7, 4)
+#define MT6370_BL_EXT_EN_MASK		BIT(7)
+#define MT6370_BL_EN_MASK		BIT(6)
+#define MT6370_BL_CONFIG_MASK		BIT(0)
+#define MT6370_BL_CH_MASK		GENMASK(5, 2)
+#define MT6370_BL_DIM2_MASK		GENMASK(2, 0)
+#define MT6370_BL_DUMMY_6372_MASK	GENMASK(2, 0)
+#define MT6370_BL_DIM2_6372_SHIFT	3
+#define MT6370_BL_PWM_EN_MASK		BIT(7)
+#define MT6370_BL_PWM_HYS_EN_MASK	BIT(2)
+#define MT6370_BL_PWM_HYS_SEL_MASK	GENMASK(1, 0)
+#define MT6370_BL_OVP_EN_MASK		BIT(7)
+#define MT6370_BL_OVP_SEL_MASK		GENMASK(6, 5)
+#define MT6370_BL_OC_EN_MASK		BIT(3)
+#define MT6370_BL_OC_SEL_MASK		GENMASK(2, 1)
+
+#define MT6370_BL_MAX_BRIGHTNESS	2048
+#define MT6370_BL_MAX_CH		15
+
+enum {
+	MT6370_VID_COMMON = 0,
+	MT6370_VID_6372,
+};
+
+struct mt6370_priv {
+	int vid_type;
+	struct backlight_device *bl;
+	struct device *dev;
+	struct gpio_desc *enable_gpio;
+	struct regmap *regmap;
+};
+
+static int mt6370_bl_update_status(struct backlight_device *bl_dev)
+{
+	struct mt6370_priv *priv = bl_get_data(bl_dev);
+	int brightness = backlight_get_brightness(bl_dev);
+	unsigned int enable_val;
+	u8 brightness_val[2];
+	int ret;
+
+	if (brightness) {
+		brightness_val[0] = (brightness - 1) & MT6370_BL_DIM2_MASK;
+		brightness_val[1] = (brightness - 1)
+				    >> fls(MT6370_BL_DIM2_MASK);
+
+		/*
+		 * To make MT6372 using 14 bits to control the brightness
+		 * backward compatible with 11 bits brightness control
+		 * (like MT6370 and MT6371 do), we left shift the value
+		 * and pad with 1 to remaining bits. Hence, the MT6372's
+		 * backlight brightness will be almost the same as MT6370's
+		 * and MT6371's.
+		 */
+		if (priv->vid_type == MT6370_VID_6372) {
+			brightness_val[0] <<= MT6370_BL_DIM2_6372_SHIFT;
+			brightness_val[0] |= MT6370_BL_DUMMY_6372_MASK;
+		}
+
+		ret = regmap_raw_write(priv->regmap, MT6370_REG_BL_DIM2,
+				       brightness_val, sizeof(brightness_val));
+		if (ret)
+			return ret;
+	}
+
+	if (priv->enable_gpio)
+		gpiod_set_value(priv->enable_gpio, brightness ? 1 : 0);
+
+	enable_val = brightness ? MT6370_BL_EN_MASK : 0;
+	return regmap_update_bits(priv->regmap, MT6370_REG_BL_EN,
+				  MT6370_BL_EN_MASK, enable_val);
+}
+
+static int mt6370_bl_get_brightness(struct backlight_device *bl_dev)
+{
+	struct mt6370_priv *priv = bl_get_data(bl_dev);
+	unsigned int enable;
+	u8 brightness_val[2];
+	int brightness, ret;
+
+	ret = regmap_read(priv->regmap, MT6370_REG_BL_EN, &enable);
+	if (ret)
+		return ret;
+
+	if (!(enable & MT6370_BL_EN_MASK))
+		return 0;
+
+	ret = regmap_raw_read(priv->regmap, MT6370_REG_BL_DIM2,
+			      brightness_val, sizeof(brightness_val));
+	if (ret)
+		return ret;
+
+	if (priv->vid_type == MT6370_VID_6372)
+		brightness_val[0] >>= MT6370_BL_DIM2_6372_SHIFT;
+
+	brightness = brightness_val[1] << fls(MT6370_BL_DIM2_MASK);
+	brightness += (brightness_val[0] & MT6370_BL_DIM2_MASK);
+
+	return brightness + 1;
+}
+
+static const struct backlight_ops mt6370_bl_ops = {
+	.options = BL_CORE_SUSPENDRESUME,
+	.update_status = mt6370_bl_update_status,
+	.get_brightness = mt6370_bl_get_brightness,
+};
+
+static int mt6370_init_backlight_properties(struct mt6370_priv *priv,
+					    struct backlight_properties *props)
+{
+	struct device *dev = priv->dev;
+	u8 prop_val;
+	u32 brightness;
+	unsigned int mask, val;
+	int ret;
+
+	/* Vendor optional properties
+	 * if property not exist, keep value in default.
+	 */
+	if (device_property_read_bool(dev, "mediatek,bled-pwm-enable")) {
+		ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_PWM,
+					 MT6370_BL_PWM_EN_MASK,
+					 MT6370_BL_PWM_EN_MASK);
+		if (ret)
+			return ret;
+	}
+
+	if (device_property_read_bool(dev, "mediatek,bled-pwm-hys-enable")) {
+		ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_PWM,
+					 MT6370_BL_PWM_HYS_EN_MASK,
+					 MT6370_BL_PWM_HYS_EN_MASK);
+		if (ret)
+			return ret;
+	}
+
+	ret = device_property_read_u8(dev, "mediatek,bled-pwm-hys-input-bit",
+				      &prop_val);
+	if (!ret) {
+		val = min_t(u8, prop_val, 3)
+		      << (ffs(MT6370_BL_PWM_HYS_SEL_MASK) - 1);
+		ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_PWM,
+					 MT6370_BL_PWM_HYS_SEL_MASK, val);
+		if (ret)
+			return ret;
+	}
+
+	ret = device_property_read_u8(dev, "mediatek,bled-ovp-microvolt",
+				      &prop_val);
+	if (!ret) {
+		val = min_t(u8, prop_val, 3)
+		      << (ffs(MT6370_BL_OVP_SEL_MASK) - 1);
+		ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_BSTCTRL,
+					 MT6370_BL_OVP_SEL_MASK, val);
+		if (ret)
+			return ret;
+	}
+
+	if (device_property_read_bool(dev, "mediatek,bled-ovp-shutdown")) {
+		ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_BSTCTRL,
+					 MT6370_BL_OVP_EN_MASK,
+					 MT6370_BL_OVP_EN_MASK);
+		if (ret)
+			return ret;
+	}
+
+	ret = device_property_read_u8(dev, "mediatek,bled-ocp-microamp",
+				      &prop_val);
+	if (!ret) {
+		val = min_t(u8, prop_val, 3)
+		      << (ffs(MT6370_BL_OC_SEL_MASK) - 1);
+		ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_BSTCTRL,
+					 MT6370_BL_OC_SEL_MASK, val);
+		if (ret)
+			return ret;
+	}
+
+	if (device_property_read_bool(dev, "mediatek,bled-ocp-shutdown")) {
+		ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_BSTCTRL,
+					 MT6370_BL_OC_EN_MASK,
+					 MT6370_BL_OC_EN_MASK);
+		if (ret)
+			return ret;
+	}
+
+	/* Common properties */
+	ret = device_property_read_u32(dev, "max-brightness", &brightness);
+	if (ret)
+		brightness = MT6370_BL_MAX_BRIGHTNESS;
+
+	props->max_brightness = min_t(u32, brightness,
+				      MT6370_BL_MAX_BRIGHTNESS);
+
+	ret = device_property_read_u32(dev, "default-brightness", &brightness);
+	if (ret)
+		brightness = props->max_brightness;
+
+	props->brightness = min_t(u32, brightness, props->max_brightness);
+
+
+	ret = device_property_read_u8(dev, "mediatek,bled-channel-use",
+				      &prop_val);
+	if (ret) {
+		dev_err(dev, "mediatek,bled-channel-use DT property missing\n");
+		return ret;
+	}
+
+	if (!prop_val || prop_val > MT6370_BL_MAX_CH) {
+		dev_err(dev, "No channel specified (ch_val:%d)\n", prop_val);
+		return -EINVAL;
+	}
+
+	mask = MT6370_BL_EXT_EN_MASK | MT6370_BL_CH_MASK;
+	val = prop_val << (ffs(MT6370_BL_CH_MASK) - 1);
+
+	if (priv->enable_gpio)
+		val |= MT6370_BL_EXT_EN_MASK;
+
+	return regmap_update_bits(priv->regmap, MT6370_REG_BL_EN, mask, val);
+}
+
+static int mt6370_check_vendor_info(struct mt6370_priv *priv)
+{
+	/*
+	 * MT6372 uses 14 bits to control the brightness but MT6370 and MT6371
+	 * use 11 bits. They are different so we have to use this function to
+	 * check the vendor ID and use different methods to calculate the
+	 * brightness.
+	 */
+	unsigned int dev_info, vid;
+	int ret;
+
+	ret = regmap_read(priv->regmap, MT6370_REG_DEV_INFO, &dev_info);
+	if (ret)
+		return ret;
+
+	vid = FIELD_GET(MT6370_VENID_MASK, dev_info);
+	if (vid == 0x9 || vid == 0xb)
+		priv->vid_type = MT6370_VID_6372;
+	else
+		priv->vid_type = MT6370_VID_COMMON;
+
+	return 0;
+}
+
+static int mt6370_bl_probe(struct platform_device *pdev)
+{
+	struct mt6370_priv *priv;
+	struct backlight_properties props = {
+		.type = BACKLIGHT_RAW,
+		.scale = BACKLIGHT_SCALE_LINEAR,
+	};
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = &pdev->dev;
+
+	priv->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!priv->regmap)
+		dev_err_probe(&pdev->dev, -ENODEV, "Failed to get regmap\n");
+
+	ret = mt6370_check_vendor_info(priv);
+	if (ret)
+		dev_err_probe(&pdev->dev, ret, "Failed to check vendor info\n");
+
+	priv->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
+						    GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->enable_gpio))
+		dev_err_probe(&pdev->dev, PTR_ERR(priv->enable_gpio),
+			      "Failed to get 'enable' gpio\n");
+
+	ret = mt6370_init_backlight_properties(priv, &props);
+	if (ret)
+		dev_err_probe(&pdev->dev, ret,
+			      "Failed to init backlight properties\n");
+
+	priv->bl = devm_backlight_device_register(&pdev->dev, pdev->name,
+						  &pdev->dev, priv,
+						  &mt6370_bl_ops, &props);
+	if (IS_ERR(priv->bl))
+		dev_err_probe(&pdev->dev, PTR_ERR(priv->bl),
+			      "Failed to register backlight\n");
+
+	backlight_update_status(priv->bl);
+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+}
+
+static int mt6370_bl_remove(struct platform_device *pdev)
+{
+	struct mt6370_priv *priv = platform_get_drvdata(pdev);
+	struct backlight_device *bl_dev = priv->bl;
+
+	bl_dev->props.brightness = 0;
+	backlight_update_status(priv->bl);
+
+	return 0;
+}
+
+static const struct of_device_id __maybe_unused mt6370_bl_of_match[] = {
+	{ .compatible = "mediatek,mt6370-backlight", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mt6370_bl_of_match);
+
+static struct platform_driver mt6370_bl_driver = {
+	.driver = {
+		.name = "mt6370-backlight",
+		.of_match_table = mt6370_bl_of_match,
+	},
+	.probe = mt6370_bl_probe,
+	.remove = mt6370_bl_remove,
+};
+module_platform_driver(mt6370_bl_driver);
+
+MODULE_AUTHOR("ChiaEn Wu <chiaen_wu@richtek.com>");
+MODULE_DESCRIPTION("Mediatek MT6370 Backlight Driver");
+MODULE_LICENSE("GPL v2");