Re: [PATCH v5 1/6] mfd: rt4831: Adds support for Richtek RT4831 core

On Thu, 17 Dec 2020, cy_huang wrote:

From: ChiYuan Huang cy_huang@richtek.com

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

Signed-off-by: ChiYuan Huang cy_huang@richtek.com

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

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 8b99a13..dfb2640 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -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 diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 1780019..28d247b 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -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

diff --git a/drivers/mfd/rt4831.c b/drivers/mfd/rt4831.c new file mode 100644 index 00000000..2bf8364 --- /dev/null +++ b/drivers/mfd/rt4831.c @@ -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.

• • Author: ChiYuan Huang cy_huang@richtek.com
• */

+#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) +{

• 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?

+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 cy_huang@richtek.com"); +MODULE_LICENSE("GPL v2");