mfd: rt4831: Adds support for Richtek RT4831 core

From: ChiYuan Huang <[email protected]>

Adds support for Richtek RT4831 backlight.

Signed-off-by ChiYuan Huang <[email protected]>

---
 drivers/video/backlight/Kconfig            |   8 ++
 drivers/video/backlight/Makefile           |   1 +
 drivers/video/backlight/rt4831-backlight.c | 219 +++++++++++++++++++++++++++++
 3 files changed, 228 insertions(+)
 create mode 100644 drivers/video/backlight/rt4831-backlight.c

drivers/video/backlight/Kconfig +8 -0

index d83c87b..666bdb0 100644
@@ -289,6 +289,14 @@ config BACKLIGHT_QCOM_WLED
          If you have the Qualcomm PMIC, say Y to enable a driver for the
          WLED block. Currently it supports PM8941 and PMI8998.
 
+config BACKLIGHT_RT4831
+       tristate "Richtek RT4831 Backlight Driver"
+       depends on MFD_RT4831
+       help
+         This enables support for Richtek RT4831 Backlight driver.
+         It's commont used to drive the display WLED. There're four channels
+         inisde, and each channel can provide up to 30mA current.
+
 config BACKLIGHT_SAHARA
        tristate "Tabletkiosk Sahara Touch-iT Backlight Driver"
        depends on X86

drivers/video/backlight/Makefile +1 -0

index 685f3f1..cae2c83 100644
@@ -49,6 +49,7 @@ obj-$(CONFIG_BACKLIGHT_PANDORA)               += pandora_bl.o
 obj-$(CONFIG_BACKLIGHT_PCF50633)       += pcf50633-backlight.o
 obj-$(CONFIG_BACKLIGHT_PWM)            += pwm_bl.o
 obj-$(CONFIG_BACKLIGHT_QCOM_WLED)      += qcom-wled.o
+obj-$(CONFIG_BACKLIGHT_RT4831)         += rt4831-backlight.o
 obj-$(CONFIG_BACKLIGHT_SAHARA)         += kb3886_bl.o
 obj-$(CONFIG_BACKLIGHT_SKY81452)       += sky81452-backlight.o
 obj-$(CONFIG_BACKLIGHT_TOSA)           += tosa_bl.o

drivers/video/backlight/rt4831-backlight.c new file +219 -1

new file mode 100644
index 00000000..816c4d6
@@ -0,0 +1,219 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <dt-bindings/leds/rt4831-backlight.h>
+#include <linux/backlight.h>
+#include <linux/bitops.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#define RT4831_REG_BLCFG       0x02
+#define RT4831_REG_BLDIML      0x04
+#define RT4831_REG_ENABLE      0x08
+
+#define BL_MAX_BRIGHTNESS      2048
+
+#define RT4831_BLOVP_MASK      GENMASK(7, 5)
+#define RT4831_BLOVP_SHIFT     5
+#define RT4831_BLPWMEN_MASK    BIT(0)
+#define RT4831_BLEN_MASK       BIT(4)
+#define RT4831_BLCH_MASK       GENMASK(3, 0)
+#define RT4831_BLDIML_MASK     GENMASK(2, 0)
+#define RT4831_BLDIMH_MASK     GENMASK(10, 3)
+#define RT4831_BLDIMH_SHIFT    3
+
+struct rt4831_priv {
+       struct regmap *regmap;
+       struct mutex lock;
+       struct backlight_device *bl;
+};
+
+static int rt4831_bl_update_status(struct backlight_device *bl_dev)
+{
+       struct rt4831_priv *priv = bl_get_data(bl_dev);
+       int brightness = backlight_get_brightness(bl_dev);
+       unsigned int enable = brightness ? RT4831_BLEN_MASK : 0;
+       u8 v[2];
+       int ret;
+
+       mutex_lock(&priv->lock);
+
+       if (brightness) {
+               v[0] = (brightness - 1) & RT4831_BLDIML_MASK;
+               v[1] = ((brightness - 1) & RT4831_BLDIMH_MASK) >> RT4831_BLDIMH_SHIFT;
+
+               ret = regmap_raw_write(priv->regmap, RT4831_REG_BLDIML, v, sizeof(v));
+               if (ret)
+                       goto unlock;
+       }
+
+       ret = regmap_update_bits(priv->regmap, RT4831_REG_ENABLE, RT4831_BLEN_MASK, enable);
+
+unlock:
+       mutex_unlock(&priv->lock);
+       return ret;
+}
+
+static int rt4831_bl_get_brightness(struct backlight_device *bl_dev)
+{
+       struct rt4831_priv *priv = bl_get_data(bl_dev);
+       unsigned int val;
+       u8 v[2];
+       int ret;
+
+       mutex_lock(&priv->lock);
+
+       ret = regmap_read(priv->regmap, RT4831_REG_ENABLE, &val);
+       if (ret)
+               return ret;
+
+       if (!(val & RT4831_BLEN_MASK)) {
+               ret = 0;
+               goto unlock;
+       }
+
+       ret = regmap_raw_read(priv->regmap, RT4831_REG_BLDIML, v, sizeof(v));
+       if (ret)
+               goto unlock;
+
+       ret = (v[1] << RT4831_BLDIMH_SHIFT) + (v[0] & RT4831_BLDIML_MASK) + 1;
+
+unlock:
+       mutex_unlock(&priv->lock);
+       return ret;
+}
+
+static const struct backlight_ops rt4831_bl_ops = {
+       .options = BL_CORE_SUSPENDRESUME,
+       .update_status = rt4831_bl_update_status,
+       .get_brightness = rt4831_bl_get_brightness,
+};
+
+static int rt4831_init_device_properties(struct rt4831_priv *priv, struct device *dev,
+                                         struct backlight_properties *bl_props)
+{
+       u8 propval;
+       u32 brightness;
+       unsigned int val = 0;
+       int ret;
+
+       /* common properties */
+       ret = device_property_read_u32(dev, "max-brightness", &brightness);
+       if (ret) {
+               dev_warn(dev, "max-brightness DT property missing, use HW max as default\n");
+               brightness = BL_MAX_BRIGHTNESS;
+       }
+
+       bl_props->max_brightness = min_t(u32, brightness, BL_MAX_BRIGHTNESS);
+
+       ret = device_property_read_u32(dev, "default-brightness", &brightness);
+       if (ret) {
+               dev_warn(dev, "default-brightness DT property missing, use max limit as default\n");
+               brightness = bl_props->max_brightness;
+       }
+
+       bl_props->brightness = min_t(u32, brightness, bl_props->max_brightness);
+
+       /* vendor properties */
+       if (device_property_read_bool(dev, "richtek,pwm-enable"))
+               val = RT4831_BLPWMEN_MASK;
+
+       ret = regmap_update_bits(priv->regmap, RT4831_REG_BLCFG, RT4831_BLPWMEN_MASK, val);
+       if (ret)
+               return ret;
+
+       ret = device_property_read_u8(dev, "richtek,bled-ovp-sel", &propval);
+       if (ret) {
+               dev_warn(dev, "richtek,bled-ovp-sel DT property missing, use default 21V\n");
+               propval = RT4831_BLOVPLVL_21V;
+       }
+
+       propval = min_t(u8, propval, RT4831_BLOVPLVL_29V);
+       ret = regmap_update_bits(priv->regmap, RT4831_REG_BLCFG, RT4831_BLOVP_MASK,
+                                propval << RT4831_BLOVP_SHIFT);
+       if (ret)
+               return ret;
+
+       ret = device_property_read_u8(dev, "richtek,channel-use", &propval);
+       if (ret) {
+               dev_err(dev, "richtek,channel-use DT property missing\n");
+               return ret;
+       }
+
+       if (!(propval & RT4831_BLCH_MASK)) {
+               dev_err(dev, "No channel specified\n");
+               return -EINVAL;
+       }
+
+       return regmap_update_bits(priv->regmap, RT4831_REG_ENABLE, RT4831_BLCH_MASK, propval);
+}
+
+static int rt4831_bl_probe(struct platform_device *pdev)
+{
+       struct rt4831_priv *priv;
+       struct backlight_properties bl_props = { .type = BACKLIGHT_RAW, };
+       int ret;
+
+       priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+       if (!priv)
+               return -ENOMEM;
+
+       mutex_init(&priv->lock);
+
+       priv->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+       if (IS_ERR(priv->regmap)) {
+               dev_err(&pdev->dev, "Failed to init regmap\n");
+               return PTR_ERR(priv->regmap);
+       }
+
+       ret = rt4831_init_device_properties(priv, &pdev->dev, &bl_props);
+       if (ret) {
+               dev_err(&pdev->dev, "Failed to init device properties\n");
+               return ret;
+       }
+
+       priv->bl = devm_backlight_device_register(&pdev->dev, pdev->name, &pdev->dev, priv,
+                                                 &rt4831_bl_ops, &bl_props);
+       if (IS_ERR(priv->bl)) {
+               dev_err(&pdev->dev, "Failed to register backlight\n");
+               return PTR_ERR(priv->bl);
+       }
+
+       backlight_update_status(priv->bl);
+       platform_set_drvdata(pdev, priv);
+
+       return 0;
+}
+
+static int rt4831_bl_remove(struct platform_device *pdev)
+{
+       struct rt4831_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 rt4831_bl_of_match[] = {
+       { .compatible = "richtek,rt4831-backlight", },
+       {}
+};
+MODULE_DEVICE_TABLE(of, rt4831_bl_of_match);
+
+static struct platform_driver rt4831_bl_driver = {
+       .driver = {
+               .name = "rt4831-backlight",
+               .of_match_table = rt4831_bl_of_match,
+       },
+       .probe = rt4831_bl_probe,
+       .remove = rt4831_bl_remove,
+};
+module_platform_driver(rt4831_bl_driver);
+
+MODULE_AUTHOR("ChiYuan Huang <[email protected]>");
+MODULE_LICENSE("GPL v2");
--

2.7.4

Dear reviewers:

Didn't get any response about this backlight patch.
Is there any part need to be refined?

From: ChiYuan Huang <[email protected]>

Adds support for Richtek RT4831 backlight.

Signed-off-by ChiYuan Huang <[email protected]>

---
 drivers/video/backlight/Kconfig            |   8 ++
 drivers/video/backlight/Makefile           |   1 +
 drivers/video/backlight/rt4831-backlight.c | 219 +++++++++++++++++++++++++++++
 3 files changed, 228 insertions(+)
 create mode 100644 drivers/video/backlight/rt4831-backlight.c

drivers/video/backlight/Kconfig +0 -0

index d83c87b..666bdb0 100644

drivers/video/backlight/rt4831-backlight.c new file +7 -0

new file mode 100644
index 00000000..816c4d6
@@ -0,0 +1,219 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <dt-bindings/leds/rt4831-backlight.h>
+#include <linux/backlight.h>
+#include <linux/bitops.h>
+#include <linux/kernel.h>
+#include <linux/module.h>

drivers/video/backlight/rt4831-backlight.c new file +31 -0

+	struct backlight_device *bl;
+};
+
+static int rt4831_bl_update_status(struct backlight_device *bl_dev)
+{
+	struct rt4831_priv *priv = bl_get_data(bl_dev);
+	int brightness = backlight_get_brightness(bl_dev);
+	unsigned int enable = brightness ? RT4831_BLEN_MASK : 0;
+	u8 v[2];
+	int ret;
+
+	mutex_lock(&priv->lock);
+
+	if (brightness) {
+		v[0] = (brightness - 1) & RT4831_BLDIML_MASK;
+		v[1] = ((brightness - 1) & RT4831_BLDIMH_MASK) >> RT4831_BLDIMH_SHIFT;
+
+		ret = regmap_raw_write(priv->regmap, RT4831_REG_BLDIML, v, sizeof(v));
+		if (ret)
+			goto unlock;
+	}
+
+	ret = regmap_update_bits(priv->regmap, RT4831_REG_ENABLE, RT4831_BLEN_MASK, enable);
+
+unlock:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+static int rt4831_bl_get_brightness(struct backlight_device *bl_dev)
+{

drivers/video/backlight/rt4831-backlight.c new file +14 -0

+
+	if (!(val & RT4831_BLEN_MASK)) {
+		ret = 0;
+		goto unlock;
+	}
+
+	ret = regmap_raw_read(priv->regmap, RT4831_REG_BLDIML, v, sizeof(v));
+	if (ret)
+		goto unlock;
+
+	ret = (v[1] << RT4831_BLDIMH_SHIFT) + (v[0] & RT4831_BLDIML_MASK) + 1;
+
+unlock:
+	mutex_unlock(&priv->lock);

drivers/video/backlight/rt4831-backlight.c new file +16 -0

+		brightness = bl_props->max_brightness;
+	}
+
+	bl_props->brightness = min_t(u32, brightness, bl_props->max_brightness);
+
+	/* vendor properties */
+	if (device_property_read_bool(dev, "richtek,pwm-enable"))
+		val = RT4831_BLPWMEN_MASK;
+
+	ret = regmap_update_bits(priv->regmap, RT4831_REG_BLCFG, RT4831_BLPWMEN_MASK, val);
+	if (ret)
+		return ret;
+
+	ret = device_property_read_u8(dev, "richtek,bled-ovp-sel", &propval);
+	if (ret) {
+		dev_warn(dev, "richtek,bled-ovp-sel DT property missing,

use default 21V\n");o

drivers/video/backlight/rt4831-backlight.c new file +17 -0

+		propval = RT4831_BLOVPLVL_21V;
+	}
+
+	propval = min_t(u8, propval, RT4831_BLOVPLVL_29V);
+	ret = regmap_update_bits(priv->regmap, RT4831_REG_BLCFG, RT4831_BLOVP_MASK,
+				 propval << RT4831_BLOVP_SHIFT);
+	if (ret)
+		return ret;
+
+	ret = device_property_read_u8(dev, "richtek,channel-use", &propval);
+	if (ret) {
+		dev_err(dev, "richtek,channel-use DT property missing\n");
+		return ret;
+	}
+
+	if (!(propval & RT4831_BLCH_MASK)) {
+		dev_err(dev, "No channel specified\n");

This adds support Richtek RT4831 core. It includes four channel WLED driver and Display Bias Voltage outputs.

From: ChiYuan Huang <[email protected]>

This adds support Richtek RT4831 core. It includes four channel WLED driver and Display Bias Voltage outputs.

Signed-off-by ChiYuan Huang <[email protected]>

---
since v5
- Rename file name from rt4831-core.c to rt4831.c
- Change RICHTEK_VID to RICHTEK_VENDOR_ID.
- Change gpio_desc nameing from 'enable' to 'enable_gpio' in probe.
- Change variable 'val' to the meaningful name 'chip_id'.
- Refine the error log when vendor id is not matched.
- Remove of_match_ptr.

since v2
- Refine Kconfig descriptions.
- Add copyright.
- Refine error logs in probe.
- Refine comment lines in remove and shutdown.

---
 drivers/mfd/Kconfig  |  10 +++++
 drivers/mfd/Makefile |   1 +
 drivers/mfd/rt4831.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+)
 create mode 100644 drivers/mfd/rt4831.c

drivers/mfd/Kconfig +10 -0

index 8b99a13..dfb2640 100644
@@ -1088,6 +1088,16 @@ config MFD_RDC321X
 	  southbridge which provides access to GPIOs and Watchdog using the
 	  southbridge PCI device configuration space.
 
+config MFD_RT4831
+	tristate "Richtek RT4831 four channel WLED and Display Bias Voltage"
+	depends on I2C
+	select MFD_CORE
+	select REGMAP_I2C
+	help
+	  This enables support for the Richtek RT4831 that includes 4 channel
+	  WLED driving and Display Bias Voltage. It's commonly used to provide
+	  power to the LCD display and LCD backlight.
+
 config MFD_RT5033
 	tristate "Richtek RT5033 Power Management IC"
 	depends on I2C

drivers/mfd/Makefile +1 -0

index 1780019..28d247b 100644
@@ -235,6 +235,7 @@ obj-$(CONFIG_MFD_MENF21BMC)	+= menf21bmc.o
 obj-$(CONFIG_MFD_HI6421_PMIC)	+= hi6421-pmic-core.o
 obj-$(CONFIG_MFD_HI655X_PMIC)   += hi655x-pmic.o
 obj-$(CONFIG_MFD_DLN2)		+= dln2.o
+obj-$(CONFIG_MFD_RT4831)	+= rt4831.o
 obj-$(CONFIG_MFD_RT5033)	+= rt5033.o
 obj-$(CONFIG_MFD_SKY81452)	+= sky81452.o
 

drivers/mfd/rt4831.c new file +0 -0

new file mode 100644
index 00000000..2bf8364

drivers/mfd/rt4831.c new file +90 -0

+ * Author: ChiYuan Huang <[email protected]>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#define RT4831_REG_REVISION	0x01
+#define RT4831_REG_ENABLE	0x08
+#define RT4831_REG_I2CPROT	0x15
+
+#define RICHTEK_VENDOR_ID	0x03
+#define RT4831_VID_MASK		GENMASK(1, 0)
+#define RT4831_RESET_MASK	BIT(7)
+#define RT4831_I2CSAFETMR_MASK	BIT(0)
+
+static const struct mfd_cell rt4831_subdevs[] = {
+	OF_MFD_CELL("rt4831-backlight", NULL, NULL, 0, 0, "richtek,rt4831-backlight"),
+	MFD_CELL_NAME("rt4831-regulator")
+};
+
+static bool rt4831_is_accessible_reg(struct device *dev, unsigned int reg)
+{
+	if (reg >= RT4831_REG_REVISION && reg <= RT4831_REG_I2CPROT)
+		return true;
+	return false;
+}
+
+static const struct regmap_config rt4831_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = RT4831_REG_I2CPROT,
+
+	.readable_reg = rt4831_is_accessible_reg,
+	.writeable_reg = rt4831_is_accessible_reg,
+};
+
+static int rt4831_probe(struct i2c_client *client)
+{
+	struct gpio_desc *enable_gpio;
+	struct regmap *regmap;
+	unsigned int chip_id;
+	int ret;
+
+	enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_HIGH);
+	if (IS_ERR(enable_gpio)) {
+		dev_err(&client->dev, "Failed to get 'enable' GPIO\n");
+		return PTR_ERR(enable_gpio);
+	}
+
+	regmap = devm_regmap_init_i2c(client, &rt4831_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&client->dev, "Failed to initialize regmap\n");
+		return PTR_ERR(regmap);
+	}
+
+	ret = regmap_read(regmap, RT4831_REG_REVISION, &chip_id);
+	if (ret) {
+		dev_err(&client->dev, "Failed to get H/W revision\n");
+		return ret;
+	}
+
+	if ((chip_id & RT4831_VID_MASK) != RICHTEK_VENDOR_ID) {
+		dev_err(&client->dev, "Chip vendor ID 0x%02x not matched\n", chip_id);
+		return -ENODEV;
+	}
+
+	/*
+	 * Used to prevent the abnormal shutdown.
+	 * If SCL/SDA both keep low for one second to reset HW.
+	 */
+	ret = regmap_update_bits(regmap, RT4831_REG_I2CPROT, RT4831_I2CSAFETMR_MASK,
+				 RT4831_I2CSAFETMR_MASK);
+	if (ret) {
+		dev_err(&client->dev, "Failed to enable I2C safety timer\n");
+		return ret;
+	}
+
+	return devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO, rt4831_subdevs,
+				    ARRAY_SIZE(rt4831_subdevs), NULL, 0, NULL);
+}
+
+static int rt4831_remove(struct i2c_client *client)
+{
+	struct regmap *regmap = dev_get_regmap(&client->dev, NULL);
+
+	/* Disable WLED and DSV outputs */

drivers/mfd/rt4831.c new file +19 -0

+static const struct of_device_id __maybe_unused rt4831_of_match[] = {
+	{ .compatible = "richtek,rt4831", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, rt4831_of_match);
+
+static struct i2c_driver rt4831_driver = {
+	.driver = {
+		.name = "rt4831",
+		.of_match_table = rt4831_of_match,
+	},
+	.probe_new = rt4831_probe,
+	.remove = rt4831_remove,
+	.shutdown = rt4831_shutdown,
+};
+module_i2c_driver(rt4831_driver);
+
+MODULE_AUTHOR("ChiYuan Huang <[email protected]>");
+MODULE_LICENSE("GPL v2");

Quoted 61 lines

From: ChiYuan Huang <[email protected]>

This adds support Richtek RT4831 core. It includes four channel WLED driver and Display Bias Voltage outputs.

Signed-off-by ChiYuan Huang <[email protected]>

---
since v5
- Rename file name from rt4831-core.c to rt4831.c
- Change RICHTEK_VID to RICHTEK_VENDOR_ID.
- Change gpio_desc nameing from 'enable' to 'enable_gpio' in probe.
- Change variable 'val' to the meaningful name 'chip_id'.
- Refine the error log when vendor id is not matched.
- Remove of_match_ptr.

since v2
- Refine Kconfig descriptions.
- Add copyright.
- Refine error logs in probe.
- Refine comment lines in remove and shutdown.

---
 drivers/mfd/Kconfig  |  10 +++++
 drivers/mfd/Makefile |   1 +
 drivers/mfd/rt4831.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+)
 create mode 100644 drivers/mfd/rt4831.c

drivers/mfd/Kconfig +10 -0

index 8b99a13..dfb2640 100644
@@ -1088,6 +1088,16 @@ config MFD_RDC321X
        southbridge which provides access to GPIOs and Watchdog using the
        southbridge PCI device configuration space.
 
+config MFD_RT4831
+     tristate "Richtek RT4831 four channel WLED and Display Bias Voltage"
+     depends on I2C
+     select MFD_CORE
+     select REGMAP_I2C
+     help
+       This enables support for the Richtek RT4831 that includes 4 channel
+       WLED driving and Display Bias Voltage. It's commonly used to provide
+       power to the LCD display and LCD backlight.
+
 config MFD_RT5033
      tristate "Richtek RT5033 Power Management IC"
      depends on I2C

drivers/mfd/Makefile +1 -0

index 1780019..28d247b 100644
@@ -235,6 +235,7 @@ obj-$(CONFIG_MFD_MENF21BMC)       += menf21bmc.o
 obj-$(CONFIG_MFD_HI6421_PMIC)        += hi6421-pmic-core.o
 obj-$(CONFIG_MFD_HI655X_PMIC)   += hi655x-pmic.o
 obj-$(CONFIG_MFD_DLN2)               += dln2.o
+obj-$(CONFIG_MFD_RT4831)     += rt4831.o
 obj-$(CONFIG_MFD_RT5033)     += rt5033.o
 obj-$(CONFIG_MFD_SKY81452)   += sky81452.o
 

drivers/mfd/rt4831.c line 3

Quoted 9 lines

drivers/mfd/rt4831.c new file +3 -0

new file mode 100644
index 00000000..2bf8364
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020 Richtek Technology Corp.

Nit: If you respin this, please bump the date.

Quoted 94 lines

drivers/mfd/rt4831.c new file +94 -0

+ * Author: ChiYuan Huang <[email protected]>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#define RT4831_REG_REVISION  0x01
+#define RT4831_REG_ENABLE    0x08
+#define RT4831_REG_I2CPROT   0x15
+
+#define RICHTEK_VENDOR_ID    0x03
+#define RT4831_VID_MASK              GENMASK(1, 0)
+#define RT4831_RESET_MASK    BIT(7)
+#define RT4831_I2CSAFETMR_MASK       BIT(0)
+
+static const struct mfd_cell rt4831_subdevs[] = {
+     OF_MFD_CELL("rt4831-backlight", NULL, NULL, 0, 0, "richtek,rt4831-backlight"),
+     MFD_CELL_NAME("rt4831-regulator")
+};
+
+static bool rt4831_is_accessible_reg(struct device *dev, unsigned int reg)
+{
+     if (reg >= RT4831_REG_REVISION && reg <= RT4831_REG_I2CPROT)
+             return true;
+     return false;
+}
+
+static const struct regmap_config rt4831_regmap_config = {
+     .reg_bits = 8,
+     .val_bits = 8,
+     .max_register = RT4831_REG_I2CPROT,
+
+     .readable_reg = rt4831_is_accessible_reg,
+     .writeable_reg = rt4831_is_accessible_reg,
+};
+
+static int rt4831_probe(struct i2c_client *client)
+{
+     struct gpio_desc *enable_gpio;
+     struct regmap *regmap;
+     unsigned int chip_id;
+     int ret;
+
+     enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_HIGH);
+     if (IS_ERR(enable_gpio)) {
+             dev_err(&client->dev, "Failed to get 'enable' GPIO\n");
+             return PTR_ERR(enable_gpio);
+     }
+
+     regmap = devm_regmap_init_i2c(client, &rt4831_regmap_config);
+     if (IS_ERR(regmap)) {
+             dev_err(&client->dev, "Failed to initialize regmap\n");
+             return PTR_ERR(regmap);
+     }
+
+     ret = regmap_read(regmap, RT4831_REG_REVISION, &chip_id);
+     if (ret) {
+             dev_err(&client->dev, "Failed to get H/W revision\n");
+             return ret;
+     }
+
+     if ((chip_id & RT4831_VID_MASK) != RICHTEK_VENDOR_ID) {
+             dev_err(&client->dev, "Chip vendor ID 0x%02x not matched\n", chip_id);
+             return -ENODEV;
+     }
+
+     /*
+      * Used to prevent the abnormal shutdown.
+      * If SCL/SDA both keep low for one second to reset HW.
+      */
+     ret = regmap_update_bits(regmap, RT4831_REG_I2CPROT, RT4831_I2CSAFETMR_MASK,
+                              RT4831_I2CSAFETMR_MASK);
+     if (ret) {
+             dev_err(&client->dev, "Failed to enable I2C safety timer\n");
+             return ret;
+     }
+
+     return devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO, rt4831_subdevs,
+                                 ARRAY_SIZE(rt4831_subdevs), NULL, 0, NULL);
+}
+
+static int rt4831_remove(struct i2c_client *client)
+{
+     struct regmap *regmap = dev_get_regmap(&client->dev, NULL);
+
+     /* Disable WLED and DSV outputs */
+     return regmap_update_bits(regmap, RT4831_REG_ENABLE, RT4831_RESET_MASK, RT4831_RESET_MASK);
+}
+
+static void rt4831_shutdown(struct i2c_client *client)

drivers/mfd/rt4831.c line 104

Quoted 6 lines

drivers/mfd/rt4831.c new file +6 -0

+{
+     struct regmap *regmap = dev_get_regmap(&client->dev, NULL);
+
+     /* Disable WLED and DSV outputs */
+     regmap_update_bits(regmap, RT4831_REG_ENABLE, RT4831_RESET_MASK, RT4831_RESET_MASK);
+}

What is your reason for providing a .shutdown() routine?

Quoted 9 lines

drivers/mfd/rt4831.c new file +9 -0

+static const struct of_device_id __maybe_unused rt4831_of_match[] = {
+     { .compatible = "richtek,rt4831", },
+     {}
+};
+MODULE_DEVICE_TABLE(of, rt4831_of_match);
+
+static struct i2c_driver rt4831_driver = {
+     .driver = {
+             .name = "rt4831",

drivers/mfd/rt4831.c line 124

Quoted 10 lines

drivers/mfd/rt4831.c new file +10 -0

+             .of_match_table = rt4831_of_match,
+     },
+     .probe_new = rt4831_probe,
+     .remove = rt4831_remove,
+     .shutdown = rt4831_shutdown,
+};
+module_i2c_driver(rt4831_driver);
+
+MODULE_AUTHOR("ChiYuan Huang <[email protected]>");
+MODULE_LICENSE("GPL v2");

--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services

Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Lee Jones <[email protected]> 於 2021年1月13日 週三 下午8:21寫道：

Quoted 63 lines · cy_huang

Quoted 61 lines

From: ChiYuan Huang <[email protected]>

This adds support Richtek RT4831 core. It includes four channel WLED driver and Display Bias Voltage outputs.

Signed-off-by ChiYuan Huang <[email protected]>

---
since v5
- Rename file name from rt4831-core.c to rt4831.c
- Change RICHTEK_VID to RICHTEK_VENDOR_ID.
- Change gpio_desc nameing from 'enable' to 'enable_gpio' in probe.
- Change variable 'val' to the meaningful name 'chip_id'.
- Refine the error log when vendor id is not matched.
- Remove of_match_ptr.

since v2
- Refine Kconfig descriptions.
- Add copyright.
- Refine error logs in probe.
- Refine comment lines in remove and shutdown.

---
 drivers/mfd/Kconfig  |  10 +++++
 drivers/mfd/Makefile |   1 +
 drivers/mfd/rt4831.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+)
 create mode 100644 drivers/mfd/rt4831.c

drivers/mfd/Kconfig +10 -0

index 8b99a13..dfb2640 100644
@@ -1088,6 +1088,16 @@ config MFD_RDC321X
        southbridge which provides access to GPIOs and Watchdog using the
        southbridge PCI device configuration space.
 
+config MFD_RT4831
+     tristate "Richtek RT4831 four channel WLED and Display Bias Voltage"
+     depends on I2C
+     select MFD_CORE
+     select REGMAP_I2C
+     help
+       This enables support for the Richtek RT4831 that includes 4 channel
+       WLED driving and Display Bias Voltage. It's commonly used to provide
+       power to the LCD display and LCD backlight.
+
 config MFD_RT5033
      tristate "Richtek RT5033 Power Management IC"
      depends on I2C

drivers/mfd/Makefile +1 -0

index 1780019..28d247b 100644
@@ -235,6 +235,7 @@ obj-$(CONFIG_MFD_MENF21BMC)       += menf21bmc.o
 obj-$(CONFIG_MFD_HI6421_PMIC)        += hi6421-pmic-core.o
 obj-$(CONFIG_MFD_HI655X_PMIC)   += hi655x-pmic.o
 obj-$(CONFIG_MFD_DLN2)               += dln2.o
+obj-$(CONFIG_MFD_RT4831)     += rt4831.o
 obj-$(CONFIG_MFD_RT5033)     += rt5033.o
 obj-$(CONFIG_MFD_SKY81452)   += sky81452.o
 

Quoted 12 lines

drivers/mfd/rt4831.c line 3

Quoted 9 lines

drivers/mfd/rt4831.c new file +3 -0

new file mode 100644
index 00000000..2bf8364
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020 Richtek Technology Corp.

Nit: If you respin this, please bump the date.

Okay.

Quoted 94 lines

Quoted 94 lines

drivers/mfd/rt4831.c new file +94 -0

+ * Author: ChiYuan Huang <[email protected]>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#define RT4831_REG_REVISION  0x01
+#define RT4831_REG_ENABLE    0x08
+#define RT4831_REG_I2CPROT   0x15
+
+#define RICHTEK_VENDOR_ID    0x03
+#define RT4831_VID_MASK              GENMASK(1, 0)
+#define RT4831_RESET_MASK    BIT(7)
+#define RT4831_I2CSAFETMR_MASK       BIT(0)
+
+static const struct mfd_cell rt4831_subdevs[] = {
+     OF_MFD_CELL("rt4831-backlight", NULL, NULL, 0, 0, "richtek,rt4831-backlight"),
+     MFD_CELL_NAME("rt4831-regulator")
+};
+
+static bool rt4831_is_accessible_reg(struct device *dev, unsigned int reg)
+{
+     if (reg >= RT4831_REG_REVISION && reg <= RT4831_REG_I2CPROT)
+             return true;
+     return false;
+}
+
+static const struct regmap_config rt4831_regmap_config = {
+     .reg_bits = 8,
+     .val_bits = 8,
+     .max_register = RT4831_REG_I2CPROT,
+
+     .readable_reg = rt4831_is_accessible_reg,
+     .writeable_reg = rt4831_is_accessible_reg,
+};
+
+static int rt4831_probe(struct i2c_client *client)
+{
+     struct gpio_desc *enable_gpio;
+     struct regmap *regmap;
+     unsigned int chip_id;
+     int ret;
+
+     enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_HIGH);
+     if (IS_ERR(enable_gpio)) {
+             dev_err(&client->dev, "Failed to get 'enable' GPIO\n");
+             return PTR_ERR(enable_gpio);
+     }
+
+     regmap = devm_regmap_init_i2c(client, &rt4831_regmap_config);
+     if (IS_ERR(regmap)) {
+             dev_err(&client->dev, "Failed to initialize regmap\n");
+             return PTR_ERR(regmap);
+     }
+
+     ret = regmap_read(regmap, RT4831_REG_REVISION, &chip_id);
+     if (ret) {
+             dev_err(&client->dev, "Failed to get H/W revision\n");
+             return ret;
+     }
+
+     if ((chip_id & RT4831_VID_MASK) != RICHTEK_VENDOR_ID) {
+             dev_err(&client->dev, "Chip vendor ID 0x%02x not matched\n", chip_id);
+             return -ENODEV;
+     }
+
+     /*
+      * Used to prevent the abnormal shutdown.
+      * If SCL/SDA both keep low for one second to reset HW.
+      */
+     ret = regmap_update_bits(regmap, RT4831_REG_I2CPROT, RT4831_I2CSAFETMR_MASK,
+                              RT4831_I2CSAFETMR_MASK);
+     if (ret) {
+             dev_err(&client->dev, "Failed to enable I2C safety timer\n");
+             return ret;
+     }
+
+     return devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO, rt4831_subdevs,
+                                 ARRAY_SIZE(rt4831_subdevs), NULL, 0, NULL);
+}
+
+static int rt4831_remove(struct i2c_client *client)
+{
+     struct regmap *regmap = dev_get_regmap(&client->dev, NULL);
+
+     /* Disable WLED and DSV outputs */
+     return regmap_update_bits(regmap, RT4831_REG_ENABLE, RT4831_RESET_MASK, RT4831_RESET_MASK);
+}
+
+static void rt4831_shutdown(struct i2c_client *client)

Quoted 9 lines

drivers/mfd/rt4831.c line 104

Quoted 6 lines

drivers/mfd/rt4831.c new file +6 -0

+{
+     struct regmap *regmap = dev_get_regmap(&client->dev, NULL);
+
+     /* Disable WLED and DSV outputs */
+     regmap_update_bits(regmap, RT4831_REG_ENABLE, RT4831_RESET_MASK, RT4831_RESET_MASK);
+}

What is your reason for providing a .shutdown() routine?

Just like as remove routine to make sure all output are off for the safety. This chip only have one 'enable' pin and I2C as the control signal. As normal shutdown, it can be make sure 'enable' will be pull low. But for some case, if 'enable' always tied to high, like as ARM reset, chip reset only triggered during next booting phase.
The period from arm reset to next probe, if user doesn't call DSV and WLED off before machine shutdown/reboot, the WLED/DSV voltage boost circuit will be kept as on.
That's why I also put shutdown routine in the driver to reset the whole chip.

Quoted 25 lines

Quoted 9 lines

drivers/mfd/rt4831.c new file +9 -0

+static const struct of_device_id __maybe_unused rt4831_of_match[] = {
+     { .compatible = "richtek,rt4831", },
+     {}
+};
+MODULE_DEVICE_TABLE(of, rt4831_of_match);
+
+static struct i2c_driver rt4831_driver = {
+     .driver = {
+             .name = "rt4831",

drivers/mfd/rt4831.c line 124

Quoted 10 lines

drivers/mfd/rt4831.c new file +10 -0

+             .of_match_table = rt4831_of_match,
+     },
+     .probe_new = rt4831_probe,
+     .remove = rt4831_remove,
+     .shutdown = rt4831_shutdown,
+};
+module_i2c_driver(rt4831_driver);
+
+MODULE_AUTHOR("ChiYuan Huang <[email protected]>");
+MODULE_LICENSE("GPL v2");

--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services

Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

HI, Lee:

ChiYuan Huang <[email protected]> 於 2021年1月13日 週三 下午10:09寫道：

Quoted 181 lines

Lee Jones <[email protected]> 於 2021年1月13日 週三 下午8:21寫道：

Quoted 63 lines · cy_huang

Quoted 61 lines

From: ChiYuan Huang <[email protected]>

This adds support Richtek RT4831 core. It includes four channel WLED driver and Display Bias Voltage outputs.

Signed-off-by ChiYuan Huang <[email protected]>

---
since v5
- Rename file name from rt4831-core.c to rt4831.c
- Change RICHTEK_VID to RICHTEK_VENDOR_ID.
- Change gpio_desc nameing from 'enable' to 'enable_gpio' in probe.
- Change variable 'val' to the meaningful name 'chip_id'.
- Refine the error log when vendor id is not matched.
- Remove of_match_ptr.

since v2
- Refine Kconfig descriptions.
- Add copyright.
- Refine error logs in probe.
- Refine comment lines in remove and shutdown.

---
 drivers/mfd/Kconfig  |  10 +++++
 drivers/mfd/Makefile |   1 +
 drivers/mfd/rt4831.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+)
 create mode 100644 drivers/mfd/rt4831.c

drivers/mfd/Kconfig +10 -0

index 8b99a13..dfb2640 100644
@@ -1088,6 +1088,16 @@ config MFD_RDC321X
        southbridge which provides access to GPIOs and Watchdog using the
        southbridge PCI device configuration space.
 
+config MFD_RT4831
+     tristate "Richtek RT4831 four channel WLED and Display Bias Voltage"
+     depends on I2C
+     select MFD_CORE
+     select REGMAP_I2C
+     help
+       This enables support for the Richtek RT4831 that includes 4 channel
+       WLED driving and Display Bias Voltage. It's commonly used to provide
+       power to the LCD display and LCD backlight.
+
 config MFD_RT5033
      tristate "Richtek RT5033 Power Management IC"
      depends on I2C

drivers/mfd/Makefile +1 -0

index 1780019..28d247b 100644
@@ -235,6 +235,7 @@ obj-$(CONFIG_MFD_MENF21BMC)       += menf21bmc.o
 obj-$(CONFIG_MFD_HI6421_PMIC)        += hi6421-pmic-core.o
 obj-$(CONFIG_MFD_HI655X_PMIC)   += hi655x-pmic.o
 obj-$(CONFIG_MFD_DLN2)               += dln2.o
+obj-$(CONFIG_MFD_RT4831)     += rt4831.o
 obj-$(CONFIG_MFD_RT5033)     += rt5033.o
 obj-$(CONFIG_MFD_SKY81452)   += sky81452.o
 

Quoted 12 lines

drivers/mfd/rt4831.c line 3

Quoted 9 lines

drivers/mfd/rt4831.c new file +3 -0

new file mode 100644
index 00000000..2bf8364
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020 Richtek Technology Corp.

Nit: If you respin this, please bump the date.

Okay.

Quoted 94 lines

Quoted 94 lines

drivers/mfd/rt4831.c new file +94 -0

+ * Author: ChiYuan Huang <[email protected]>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#define RT4831_REG_REVISION  0x01
+#define RT4831_REG_ENABLE    0x08
+#define RT4831_REG_I2CPROT   0x15
+
+#define RICHTEK_VENDOR_ID    0x03
+#define RT4831_VID_MASK              GENMASK(1, 0)
+#define RT4831_RESET_MASK    BIT(7)
+#define RT4831_I2CSAFETMR_MASK       BIT(0)
+
+static const struct mfd_cell rt4831_subdevs[] = {
+     OF_MFD_CELL("rt4831-backlight", NULL, NULL, 0, 0, "richtek,rt4831-backlight"),
+     MFD_CELL_NAME("rt4831-regulator")
+};
+
+static bool rt4831_is_accessible_reg(struct device *dev, unsigned int reg)
+{
+     if (reg >= RT4831_REG_REVISION && reg <= RT4831_REG_I2CPROT)
+             return true;
+     return false;
+}
+
+static const struct regmap_config rt4831_regmap_config = {
+     .reg_bits = 8,
+     .val_bits = 8,
+     .max_register = RT4831_REG_I2CPROT,
+
+     .readable_reg = rt4831_is_accessible_reg,
+     .writeable_reg = rt4831_is_accessible_reg,
+};
+
+static int rt4831_probe(struct i2c_client *client)
+{
+     struct gpio_desc *enable_gpio;
+     struct regmap *regmap;
+     unsigned int chip_id;
+     int ret;
+
+     enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_HIGH);
+     if (IS_ERR(enable_gpio)) {
+             dev_err(&client->dev, "Failed to get 'enable' GPIO\n");
+             return PTR_ERR(enable_gpio);
+     }
+
+     regmap = devm_regmap_init_i2c(client, &rt4831_regmap_config);
+     if (IS_ERR(regmap)) {
+             dev_err(&client->dev, "Failed to initialize regmap\n");
+             return PTR_ERR(regmap);
+     }
+
+     ret = regmap_read(regmap, RT4831_REG_REVISION, &chip_id);
+     if (ret) {
+             dev_err(&client->dev, "Failed to get H/W revision\n");
+             return ret;
+     }
+
+     if ((chip_id & RT4831_VID_MASK) != RICHTEK_VENDOR_ID) {
+             dev_err(&client->dev, "Chip vendor ID 0x%02x not matched\n", chip_id);
+             return -ENODEV;
+     }
+
+     /*
+      * Used to prevent the abnormal shutdown.
+      * If SCL/SDA both keep low for one second to reset HW.
+      */
+     ret = regmap_update_bits(regmap, RT4831_REG_I2CPROT, RT4831_I2CSAFETMR_MASK,
+                              RT4831_I2CSAFETMR_MASK);
+     if (ret) {
+             dev_err(&client->dev, "Failed to enable I2C safety timer\n");
+             return ret;
+     }
+
+     return devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO, rt4831_subdevs,
+                                 ARRAY_SIZE(rt4831_subdevs), NULL, 0, NULL);
+}
+
+static int rt4831_remove(struct i2c_client *client)
+{
+     struct regmap *regmap = dev_get_regmap(&client->dev, NULL);
+
+     /* Disable WLED and DSV outputs */
+     return regmap_update_bits(regmap, RT4831_REG_ENABLE, RT4831_RESET_MASK, RT4831_RESET_MASK);
+}
+
+static void rt4831_shutdown(struct i2c_client *client)

Quoted 9 lines

drivers/mfd/rt4831.c line 104

Quoted 6 lines

drivers/mfd/rt4831.c new file +6 -0

+{
+     struct regmap *regmap = dev_get_regmap(&client->dev, NULL);
+
+     /* Disable WLED and DSV outputs */
+     regmap_update_bits(regmap, RT4831_REG_ENABLE, RT4831_RESET_MASK, RT4831_RESET_MASK);
+}

What is your reason for providing a .shutdown() routine?

Quoted 9 lines

Just like as remove routine to make sure all output are off for the safety. This chip only have one 'enable' pin and I2C as the control signal. As normal shutdown, it can be make sure 'enable' will be pull low. But for some case, if 'enable' always tied to high, like as ARM reset, chip reset only triggered during next booting phase.
The period from arm reset to next probe, if user doesn't call DSV and WLED off before machine shutdown/reboot, the WLED/DSV voltage boost circuit will be kept as on.
That's why I also put shutdown routine in the driver to reset the whole chip.

If the shutdown routine is not suitable, I think it can be removed. There's the HWEN pin inside this IC to make sure all function will be disabled.

There're changed part in my note
1. respin the header date.
2. Remove the shutdown routine

Anything else?