Date:   Thu, 3 Aug 2023 15:24:25 +0800
From:   ChiYuan Huang <cy_huang@...htek.com>
To:     Alina Yu <alina_yu@...htek.com>
CC:     <lgirdwood@...il.com>, <broonie@...nel.org>, <robh+dt@...nel.org>,
        <krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>
Subject: Re: [PATCH v5 2/2] regulator: rtq2208: Add Richtek RTQ2208 SubPMIC
 driver

Hi,
On Wed, Jul 26, 2023 at 03:13:12PM +0800, Alina Yu wrote:
> Add support for the RTQ2208 SubPMIC
> This ic integrates with configurable, synchrnous buck converters and two
> ldos.
> 
> Signed-off-by: Alina Yu <alina_yu@...htek.com>
> Reported-by: kernel test robot <lkp@...el.com>
This patch series is to add new drivers, not fix.
Why do you add the Reported-by tag?
> ---
> v5
> - Modify 'rdesc->fixed_uV' get, becasue "richtek,fixed-uV" is removed from yaml
> - Modify 'mtp_sel' get because read property is changed from "richtek,mtp-sel" to
>   "richtek,mtp-sel-high" in yaml
> - Add missing regulators_node points to regulator node in yaml
> - Include <linux/bitfield.h> for 'FIELD_PREP' reported by kernel test robot
> ---
>  drivers/regulator/Kconfig             |  11 +
>  drivers/regulator/Makefile            |   1 +
>  drivers/regulator/rtq2208-regulator.c | 549 ++++++++++++++++++++++++++++++++++
>  3 files changed, 561 insertions(+)
>  create mode 100644 drivers/regulator/rtq2208-regulator.c
> 
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index e5f3613..a6b2c84 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -1213,6 +1213,17 @@ config REGULATOR_RTQ6752
>  	  synchronous boost converters for PAVDD, and one synchronous NAVDD
>  	  buck-boost. This device is suitable for automotive TFT-LCD panel.
>  
> +config REGULATOR_RTQ2208
> +	tristate "Richtek RTQ2208 SubPMIC Regulator"
> +	depends on I2C
> +	select REGMAP_I2C
> +	help
> +	  This driver adds support for RTQ2208 SubPMIC regulators.
> +	  The RTQ2208 is a multi-phase, programmable power management IC that
> +	  integrate with dual multi-configurable, synchronous buck converters
> +	  and two ldos. It features wide output voltage range from 0.4V to 2.05V
> +	  and the capability to configure the corresponding power stages.
> +
>  config REGULATOR_S2MPA01
>  	tristate "Samsung S2MPA01 voltage regulator"
>  	depends on MFD_SEC_CORE || COMPILE_TEST
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 58dfe01..04cbad17 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -143,6 +143,7 @@ obj-$(CONFIG_REGULATOR_RT6245)	+= rt6245-regulator.o
>  obj-$(CONFIG_REGULATOR_RTMV20)	+= rtmv20-regulator.o
>  obj-$(CONFIG_REGULATOR_RTQ2134) += rtq2134-regulator.o
>  obj-$(CONFIG_REGULATOR_RTQ6752)	+= rtq6752-regulator.o
> +obj-$(CONFIG_REGULATOR_RTQ2208) += rtq2208-regulator.o
>  obj-$(CONFIG_REGULATOR_S2MPA01) += s2mpa01.o
>  obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o
>  obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
> diff --git a/drivers/regulator/rtq2208-regulator.c b/drivers/regulator/rtq2208-regulator.c
> new file mode 100644
> index 0000000..2cfbc68
> --- /dev/null
> +++ b/drivers/regulator/rtq2208-regulator.c
> @@ -0,0 +1,549 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <linux/bitops.h>
> +#include <linux/bitfield.h>
> +#include <linux/util_macros.h>
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/driver.h>
It seems you forgot to include mod_devicetable.h
> +
> +/* Register */
> +#define RTQ2208_REG_GLOBAL_INT1			0x12
> +#define RTQ2208_REG_FLT_RECORDBUCK_CB		0x18
> +#define RTQ2208_REG_GLOBAL_INT1_MASK		0x1D
> +#define RTQ2208_REG_FLT_MASKBUCK_CB		0x1F
> +#define RTQ2208_REG_BUCK_C_CFG0			0x32
> +#define RTQ2208_REG_BUCK_B_CFG0			0x42
> +#define RTQ2208_REG_BUCK_A_CFG0			0x52
> +#define RTQ2208_REG_BUCK_D_CFG0			0x62
> +#define RTQ2208_REG_BUCK_G_CFG0			0x72
> +#define RTQ2208_REG_BUCK_F_CFG0			0x82
> +#define RTQ2208_REG_BUCK_E_CFG0			0x92
> +#define RTQ2208_REG_BUCK_H_CFG0			0xA2
> +#define RTQ2208_REG_LDO1_CFG			0xB1
> +#define RTQ2208_REG_LDO2_CFG			0xC1
> +
> +/* Mask */
> +#define RTQ2208_BUCK_NR_MTP_SEL_MASK		GENMASK(7, 0)
> +#define RTQ2208_BUCK_EN_NR_MTP_SEL0_MASK	BIT(0)
> +#define RTQ2208_BUCK_EN_NR_MTP_SEL1_MASK	BIT(1)
> +#define RTQ2208_BUCK_RSPUP_MASK			GENMASK(6, 4)
> +#define RTQ2208_BUCK_RSPDN_MASK			GENMASK(2, 0)
> +#define RTQ2208_BUCK_NRMODE_MASK		BIT(5)
> +#define RTQ2208_BUCK_STRMODE_MASK		BIT(5)
> +#define RTQ2208_BUCK_EN_STR_MASK		BIT(0)
> +#define RTQ2208_LDO_EN_STR_MASK			BIT(7)
> +#define RTQ2208_EN_DIS_MASK			BIT(0)
> +#define RTQ2208_BUCK_RAMP_SEL_MASK		GENMASK(2, 0)
> +#define RTQ2208_HD_INT_MASK			BIT(0)
> +
> +/* Size */
> +#define RTQ2208_VOUT_MAXNUM			256
> +#define RTQ2208_BUCK_NUM_IRQ_REGS		5
> +#define RTQ2208_STS_NUM_IRQ_REGS		2
> +
> +/* Value */
> +#define RTQ2208_RAMP_VALUE_MIN_uV		500
> +#define RTQ2208_RAMP_VALUE_MAX_uV		64000
> +
> +#define RTQ2208_BUCK_MASK(uv_irq, ov_irq)	(1 << ((uv_irq) % 8) | 1 << ((ov_irq) % 8))
> +
> +enum {
> +	RTQ2208_BUCK_B = 0,
> +	RTQ2208_BUCK_C,
> +	RTQ2208_BUCK_D,
> +	RTQ2208_BUCK_A,
> +	RTQ2208_BUCK_F,
> +	RTQ2208_BUCK_G,
> +	RTQ2208_BUCK_H,
> +	RTQ2208_BUCK_E,
> +	RTQ2208_LDO2,
> +	RTQ2208_LDO1,
> +	RTQ2208_LDO_MAX,
> +};
> +
> +enum {
> +	RTQ2208_AUTO_MODE = 0,
> +	RTQ2208_FCCM,
> +};
> +
> +struct rtq2208_regulator_desc {
> +	struct regulator_desc desc;
> +	unsigned int mtp_sel_reg;
> +	unsigned int mtp_sel_mask;
> +	unsigned int mode_reg;
> +	unsigned int mode_mask;
> +	unsigned int suspend_config_reg;
> +	unsigned int suspend_enable_mask;
> +	unsigned int suspend_mode_mask;
> +	unsigned int fixed_uV;
This is duplicated with the member 'fixed_uV' in struct regulator_desc.
> +};
> +
> +struct rtq2208_rdev_map {
> +	struct regulator_dev *rdev[RTQ2208_LDO_MAX];
> +	struct regmap *regmap;
> +	struct device *dev;
> +};
> +
> +/* set Normal Auto/FCCM mode */
> +static int rtq2208_set_mode(struct regulator_dev *rdev, unsigned int mode)
> +{
> +	struct rtq2208_regulator_desc *rdesc =
> +		(struct rtq2208_regulator_desc *)rdev->desc;
> +	unsigned int val, shift;
> +
> +	switch (mode) {
> +	case REGULATOR_MODE_NORMAL:
> +		val = RTQ2208_AUTO_MODE;
> +		break;
> +	case REGULATOR_MODE_FAST:
> +		val = RTQ2208_FCCM;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	shift = ffs(rdesc->mode_mask) - 1;
> +	return regmap_update_bits(rdev->regmap, rdesc->mode_reg,
> +				  rdesc->mode_mask, val << shift);
> +}
> +
> +static unsigned int rtq2208_get_mode(struct regulator_dev *rdev)
> +{
> +	struct rtq2208_regulator_desc *rdesc =
> +		(struct rtq2208_regulator_desc *)rdev->desc;
> +	unsigned int mode_val;
> +	int ret;
> +
> +	ret = regmap_read(rdev->regmap, rdesc->mode_reg, &mode_val);
> +	if (ret)
> +		return REGULATOR_MODE_INVALID;
> +
> +	return (mode_val & rdesc->mode_mask) ? REGULATOR_MODE_FAST : REGULATOR_MODE_NORMAL;
> +}
> +
> +static int rtq2208_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
> +{
> +	const struct regulator_desc *rdesc = rdev->desc;
> +	unsigned int sel = 0, val;
> +
> +	ramp_delay = max(ramp_delay, RTQ2208_RAMP_VALUE_MIN_uV);
> +	ramp_delay = min(ramp_delay, RTQ2208_RAMP_VALUE_MAX_uV);
> +
> +	ramp_delay /= RTQ2208_RAMP_VALUE_MIN_uV;
> +
> +	/*
> +	 * fls(ramp_delay) - 1: doing LSB shift, let it starts from 0
> +	 *
> +	 * RTQ2208_BUCK_RAMP_SEL_MASK - sel: doing descending order shifting.
> +	 * Because the relation of seleltion and value is like that
> +	 *
> +	 * seletion: value
> +	 * 000: 64mv
> +	 * 001: 32mv
> +	 * ...
> +	 * 111: 0.5mv
> +	 *
> +	 * For example, if I would like to select 64mv, the fls(ramp_delay) - 1 will be 0b111,
> +	 * and I need to use 0b111 - sel to do the shifting
> +	 */
> +
> +	sel = fls(ramp_delay) - 1;
> +	sel = RTQ2208_BUCK_RAMP_SEL_MASK - sel;
> +
> +	val = FIELD_PREP(RTQ2208_BUCK_RSPUP_MASK, sel) | FIELD_PREP(RTQ2208_BUCK_RSPDN_MASK, sel);
> +
> +	return regmap_update_bits(rdev->regmap, rdesc->ramp_reg,
> +				  RTQ2208_BUCK_RSPUP_MASK | RTQ2208_BUCK_RSPDN_MASK, val);
> +}
> +
> +static int rtq2208_set_suspend_enable(struct regulator_dev *rdev)
> +{
> +	struct rtq2208_regulator_desc *rdesc =
> +		(struct rtq2208_regulator_desc *)rdev->desc;
> +
> +	return regmap_set_bits(rdev->regmap, rdesc->suspend_config_reg, rdesc->suspend_enable_mask);
> +}
> +
> +static int rtq2208_set_suspend_disable(struct regulator_dev *rdev)
> +{
> +	struct rtq2208_regulator_desc *rdesc =
> +		(struct rtq2208_regulator_desc *)rdev->desc;
> +
> +	return regmap_update_bits(rdev->regmap, rdesc->suspend_config_reg, rdesc->suspend_enable_mask, 0);
> +}
> +
> +static int rtq2208_set_suspend_mode(struct regulator_dev *rdev, unsigned int mode)
> +{
> +	struct rtq2208_regulator_desc *rdesc =
> +		(struct rtq2208_regulator_desc *)rdev->desc;
> +	unsigned int val, shift;
> +
> +	switch (mode) {
> +	case REGULATOR_MODE_NORMAL:
> +		val = RTQ2208_AUTO_MODE;
> +		break;
> +	case REGULATOR_MODE_FAST:
> +		val = RTQ2208_FCCM;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	shift = ffs(rdesc->suspend_mode_mask) - 1;
> +
> +	return regmap_update_bits(rdev->regmap, rdesc->suspend_config_reg,
> +			rdesc->suspend_mode_mask, val << shift);
> +}
> +
> +static int rtq2208_ldo_get_voltage(struct regulator_dev *rdev)
If you use the 'fixed_uV' in regulator_desc, then this 'get_voltage' API is not needed.
> +{
> +	struct rtq2208_regulator_desc *rdesc =
> +		(struct rtq2208_regulator_desc *)rdev->desc;
> +
> +	return rdesc->fixed_uV;
> +}
> +
> +static const struct regulator_ops rtq2208_regulator_buck_ops = {
> +	.enable = regulator_enable_regmap,
> +	.disable = regulator_disable_regmap,
> +	.is_enabled = regulator_is_enabled_regmap,
> +	.list_voltage = regulator_list_voltage_linear_range,
> +	.set_voltage_sel = regulator_set_voltage_sel_regmap,
> +	.get_voltage_sel = regulator_get_voltage_sel_regmap,
> +	.set_mode = rtq2208_set_mode,
> +	.get_mode = rtq2208_get_mode,
> +	.set_ramp_delay = rtq2208_set_ramp_delay,
> +	.set_active_discharge = regulator_set_active_discharge_regmap,
> +	.set_suspend_enable = rtq2208_set_suspend_enable,
> +	.set_suspend_disable = rtq2208_set_suspend_disable,
> +	.set_suspend_mode = rtq2208_set_suspend_mode,
> +};
> +
> +static const struct regulator_ops rtq2208_regulator_ldo_ops = {
> +	.enable = regulator_enable_regmap,
> +	.disable = regulator_disable_regmap,
> +	.is_enabled = regulator_is_enabled_regmap,
> +	.set_active_discharge = regulator_set_active_discharge_regmap,
> +	.set_suspend_enable = rtq2208_set_suspend_enable,
> +	.set_suspend_disable = rtq2208_set_suspend_disable,
> +	.get_voltage = rtq2208_ldo_get_voltage,
> +};
> +
> +static int rtq2208_of_parse_cb(struct device_node *np,
> +				const struct regulator_desc *desc,
> +				struct regulator_config *config)
> +{
> +	struct rtq2208_regulator_desc *rdesc =
> +		(struct rtq2208_regulator_desc *)desc;
> +	u32 min_uV, max_uV;
> +	int ret;
> +
> +	ret = of_property_read_u32(np, "regulator-min-microvolt", &min_uV);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_property_read_u32(np, "regulator-max-microvolt", &max_uV);
> +	if (ret)
> +		return ret;
> +
> +	if (min_uV == max_uV)
> +		rdesc->fixed_uV = min_uV;
> +
> +	return ret;
> +}
> +
> +static unsigned int rtq2208_of_map_mode(unsigned int mode)
> +{
> +	switch (mode) {
> +	case RTQ2208_AUTO_MODE:
> +		return REGULATOR_MODE_NORMAL;
> +	case RTQ2208_FCCM:
> +		return REGULATOR_MODE_FAST;
> +	default:
> +		return REGULATOR_MODE_INVALID;
> +	}
> +}
> +
> +static int rtq2208_init_irq_mask(struct rtq2208_rdev_map *rdev_map, unsigned int *buck_masks)
> +{
> +	unsigned char buck_clr_masks[5] = {0x33, 0x33, 0x33, 0x33, 0x33},
> +		      sts_clr_masks[2] = {0xE7, 0xF7}, sts_masks[2] = {0xE6, 0xF6};
> +	int ret;
> +
> +	/* write clear all buck irq once */
> +	ret = regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_FLT_RECORDBUCK_CB, buck_clr_masks, 5);
> +	if (ret)
> +		return dev_err_probe(rdev_map->dev, ret, "Failed to clr buck irqs\n");
> +
> +	/* write clear general irq once */
> +	ret = regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_GLOBAL_INT1, sts_clr_masks, 2);
> +	if (ret)
> +		return dev_err_probe(rdev_map->dev, ret, "Failed to clr general irqs\n");
> +
> +	/* unmask buck ov/uv irq */
> +	ret = regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_FLT_MASKBUCK_CB, buck_masks, 5);
> +	if (ret)
> +		return dev_err_probe(rdev_map->dev, ret, "Failed to unmask buck irqs\n");
> +
> +	/* unmask needed general irq */
> +	return regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_GLOBAL_INT1_MASK, sts_masks, 2);
> +}
> +
> +static irqreturn_t rtq2208_irq_handler(int irqno, void *devid)
> +{
> +	unsigned char buck_flags[RTQ2208_BUCK_NUM_IRQ_REGS], sts_flags[RTQ2208_STS_NUM_IRQ_REGS];
> +	int ret = 0, i, uv_bit, ov_bit;
> +	struct rtq2208_rdev_map *rdev_map = devid;
> +	struct regulator_dev *rdev;
> +
> +	if (!rdev_map)
> +		return IRQ_NONE;
> +
> +	/* read irq event */
> +	ret = regmap_bulk_read(rdev_map->regmap, RTQ2208_REG_FLT_RECORDBUCK_CB,
> +				buck_flags, ARRAY_SIZE(buck_flags));
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	ret = regmap_bulk_read(rdev_map->regmap, RTQ2208_REG_GLOBAL_INT1,
> +				sts_flags, ARRAY_SIZE(sts_flags));
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	/* clear irq event */
> +	ret = regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_FLT_RECORDBUCK_CB,
> +				buck_flags, ARRAY_SIZE(buck_flags));
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	ret = regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_GLOBAL_INT1,
> +				sts_flags, ARRAY_SIZE(sts_flags));
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	for (i = 0; i < RTQ2208_LDO_MAX; i++) {
> +		if (!rdev_map->rdev[i])
> +			continue;
> +
> +		rdev = rdev_map->rdev[i];
> +		/* uv irq */
> +		uv_bit = (i & 1) ? 4 : 0;
> +		if (buck_flags[i >> 1] & (1 << uv_bit))
> +			regulator_notifier_call_chain(rdev,
> +					REGULATOR_EVENT_UNDER_VOLTAGE, NULL);
> +		/* ov irq */
> +		ov_bit = uv_bit + 1;
> +		if (buck_flags[i >> 1] & (1 << ov_bit))
> +			regulator_notifier_call_chain(rdev,
> +					REGULATOR_EVENT_REGULATION_OUT, NULL);
> +
> +		/* hd irq */
> +		if (sts_flags[1] & RTQ2208_HD_INT_MASK)
> +			regulator_notifier_call_chain(rdev,
> +					REGULATOR_EVENT_OVER_TEMP, NULL);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +#define RTQ2208_REGULATOR_INFO(_name, _base) \
> +{ \
> +	.name = #_name, \
> +	.base = _base, \
> +}
> +#define BUCK_RG_BASE(_id)	RTQ2208_REG_BUCK_##_id##_CFG0
> +#define BUCK_RG_SHIFT(_base, _shift)	(_base + _shift)
> +#define LDO_RG_BASE(_id)	RTQ2208_REG_LDO##_id##_CFG
> +#define LDO_RG_SHIFT(_base, _shift)	(_base + _shift)
> +#define	VSEL_SHIFT(_sel)	(_sel ? 3 : 1)
> +#define MTP_SEL_MASK(_sel)	RTQ2208_BUCK_EN_NR_MTP_SEL##_sel##_MASK
> +
> +static const struct linear_range rtq2208_vout_range[] = {
> +	REGULATOR_LINEAR_RANGE(400000, 0, 180, 5000),
> +	REGULATOR_LINEAR_RANGE(1310000, 181, 255, 10000),
> +};
> +
> +static void rtq2208_init_regulator_desc(struct rtq2208_regulator_desc *rdesc, int mtp_sel, int idx)
> +{
> +	struct regulator_desc *desc;
> +	static const struct {
> +		char *name;
> +		int base;
> +	} regulator_info[] = {
> +		RTQ2208_REGULATOR_INFO(buck-b, BUCK_RG_BASE(B)),
> +		RTQ2208_REGULATOR_INFO(buck-c, BUCK_RG_BASE(C)),
> +		RTQ2208_REGULATOR_INFO(buck-d, BUCK_RG_BASE(D)),
> +		RTQ2208_REGULATOR_INFO(buck-a, BUCK_RG_BASE(A)),
> +		RTQ2208_REGULATOR_INFO(buck-f, BUCK_RG_BASE(F)),
> +		RTQ2208_REGULATOR_INFO(buck-g, BUCK_RG_BASE(G)),
> +		RTQ2208_REGULATOR_INFO(buck-h, BUCK_RG_BASE(H)),
> +		RTQ2208_REGULATOR_INFO(buck-e, BUCK_RG_BASE(E)),
> +		RTQ2208_REGULATOR_INFO(ldo2, LDO_RG_BASE(2)),
> +		RTQ2208_REGULATOR_INFO(ldo1, LDO_RG_BASE(1)),
> +	}, *curr_info;
> +
> +	curr_info = regulator_info + idx;
> +	desc = &rdesc->desc;
> +	desc->name = curr_info->name;
> +	desc->of_match = of_match_ptr(curr_info->name);
> +	desc->regulators_node = of_match_ptr("regulators");
> +	desc->id = idx;
> +	desc->owner = THIS_MODULE;
> +	desc->type = REGULATOR_VOLTAGE;
> +	desc->enable_mask = mtp_sel ? MTP_SEL_MASK(1) : MTP_SEL_MASK(0);
> +	desc->active_discharge_on = RTQ2208_EN_DIS_MASK;
> +	desc->active_discharge_off = 0;
> +	desc->active_discharge_mask = RTQ2208_EN_DIS_MASK;
> +
> +	rdesc->mode_mask = RTQ2208_BUCK_NRMODE_MASK;
> +
> +	if (idx >= RTQ2208_BUCK_B && idx <= RTQ2208_BUCK_E) {
> +		/* init buck desc */
> +		desc->enable_reg = BUCK_RG_SHIFT(curr_info->base, 2);
> +		desc->ops = &rtq2208_regulator_buck_ops;
> +		desc->vsel_reg = curr_info->base + VSEL_SHIFT(mtp_sel);
> +		desc->vsel_mask = RTQ2208_BUCK_NR_MTP_SEL_MASK;
> +		desc->n_voltages = RTQ2208_VOUT_MAXNUM;
> +		desc->linear_ranges = rtq2208_vout_range;
> +		desc->n_linear_ranges = ARRAY_SIZE(rtq2208_vout_range);
> +		desc->ramp_reg = BUCK_RG_SHIFT(curr_info->base, 5);
> +		desc->active_discharge_reg = curr_info->base;
> +		desc->of_map_mode = rtq2208_of_map_mode;
> +
> +		rdesc->mode_reg = BUCK_RG_SHIFT(curr_info->base, 2);
> +		rdesc->suspend_config_reg = BUCK_RG_SHIFT(curr_info->base, 4);
> +		rdesc->suspend_enable_mask = RTQ2208_BUCK_EN_STR_MASK;
> +		rdesc->suspend_mode_mask = RTQ2208_BUCK_STRMODE_MASK;
> +	} else {
> +		/* init ldo desc */
> +		desc->enable_reg = curr_info->base;
> +		desc->ops = &rtq2208_regulator_ldo_ops;
> +		desc->n_voltages = 1;
> +		desc->active_discharge_reg = LDO_RG_SHIFT(curr_info->base, 2);
> +		desc->of_parse_cb = rtq2208_of_parse_cb;
> +
> +		rdesc->suspend_config_reg = curr_info->base;
> +		rdesc->suspend_enable_mask = RTQ2208_LDO_EN_STR_MASK;
> +	}
> +}
> +
> +/** different slave address corresponds different used bucks
> + * slave address 0x10: BUCK[BCA FGE]
> + * slave address 0x20: BUCK[BC FGHE]
> + * slave address 0x40: BUCK[C G]
> + */
> +static int rtq2208_regulator_check(int slave_addr, int *num, int *regulator_idx_table, unsigned int *buck_masks)
> +{
> +	static bool rtq2208_used_table[3][RTQ2208_LDO_MAX] = {
> +		/* BUCK[BCA FGE], LDO[12] */
> +		{1, 1, 0, 1, 1, 1, 0, 1, 1, 1},
> +		/* BUCK[BC FGHE], LDO[12]*/
> +		{1, 1, 0, 0, 1, 1, 1, 1, 1, 1},
> +		/* BUCK[C G], LDO[12] */
> +		{0, 1, 0, 0, 0, 1, 0, 0, 1, 1},
> +	};
> +	int i, idx = ffs(slave_addr >> 4) - 1;
> +	u8 mask;
> +
> +	for (i = 0; i < RTQ2208_LDO_MAX; i++) {
> +		if (!rtq2208_used_table[idx][i])
> +			continue;
> +
> +		regulator_idx_table[(*num)++] = i;
> +
> +		mask = RTQ2208_BUCK_MASK(4 * i, 4 * i + 1);
> +		buck_masks[i >> 1] &= ~mask;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct regmap_config rtq2208_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = 0xEF,
> +};
> +
> +static int rtq2208_probe(struct i2c_client *i2c)
> +{
> +	struct device *dev = &i2c->dev;
> +	struct regmap *regmap;
> +	struct rtq2208_regulator_desc *rdesc;
> +	struct regulator_dev *rdev;
> +	struct regulator_config cfg;
> +	struct rtq2208_rdev_map *rdev_map;
> +	int i, ret = 0, idx, mtp_sel, n_regulator = 0;
> +	unsigned int regulator_idx_table[RTQ2208_LDO_MAX],
> +		     buck_masks[RTQ2208_BUCK_NUM_IRQ_REGS] = {0x33, 0x33, 0x33, 0x33, 0x33};
> +
> +	rdev_map = devm_kzalloc(dev, sizeof(struct rtq2208_rdev_map), GFP_KERNEL);
> +	if (!rdev_map)
> +		return -ENOMEM;
> +
> +	/* get mtp_sel0 or mtp_sel1 */
> +	mtp_sel = device_property_read_bool(dev, "richtek,mtp-sel-high");
> +
> +	regmap = devm_regmap_init_i2c(i2c, &rtq2208_regmap_config);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to allocate regmap\n");
> +
> +	/* get needed regulator */
> +	ret = rtq2208_regulator_check(i2c->addr, &n_regulator, regulator_idx_table, buck_masks);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to check used regulators\n");
> +
> +	rdev_map->regmap = regmap;
> +	rdev_map->dev = dev;
> +
> +	cfg.dev = dev;
> +
> +	for (i = 0; i < n_regulator; i++) {
> +		idx = regulator_idx_table[i];
> +
> +		rdesc = devm_kcalloc(dev, 1, sizeof(struct rtq2208_regulator_desc), GFP_KERNEL);
It's the same as devm_kzalloc(dev, sizeof(*rdesc), GFP_KERNEL). More simple?
> +		if (!rdesc)
> +			return -ENOMEM;
> +
> +		/* init regulator desc */
> +		rtq2208_init_regulator_desc(rdesc, mtp_sel, idx);
> +
> +		/* regiser regulator */
> +		rdev = devm_regulator_register(dev, &rdesc->desc, &cfg);
> +		if (IS_ERR(rdev))
> +			return PTR_ERR(rdev);
> +
> +		rdev_map->rdev[idx] = rdev;
> +	}
> +
> +	/* init interrupt mask */
> +	ret = rtq2208_init_irq_mask(rdev_map, buck_masks);
> +	if (ret)
> +		return ret;
> +
> +	/* register interrupt */
> +	ret = devm_request_threaded_irq(dev, i2c->irq, NULL, rtq2208_irq_handler,
> +					IRQF_ONESHOT, dev_name(dev), rdev_map);
> +
> +	return ret;
Why not directly write this as 'return devm_request_threaded_irq .......?
> +}
> +
> +static const struct of_device_id __maybe_unused rtq2208_device_tables[] = {
Remove __maybe_unused. it cannot be unsed.
The below 'of_match_table' will always need it.
> +	{ .compatible = "richtek,rtq2208", },
	                                ~~~
                               this comma can be removed.
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, rtq2208_device_tables);
> +
> +static struct i2c_driver rtq2208_driver = {
> +	.driver = {
> +		.name = "rtq2208",
> +		.of_match_table = rtq2208_device_tables,
> +	},
> +	.probe_new = rtq2208_probe,
> +};
> +module_i2c_driver(rtq2208_driver);
> +
> +MODULE_AUTHOR("Alina Yu <alina_yu@...htek.com>");
> +MODULE_DESCRIPTION("Richtek RTQ2208 Regulator Driver");
> +MODULE_LICENSE("GPL v2");
'GPL' only is fine. In SPDX license, you already said it 'GPL v2+'.
> -- 
> 2.7.4
> 

Powered by blists - more mailing lists