RE: [PATCH v1] power: regulator: Add support for NPCM845

["Jaehoon Chung" <jh80.chung@samsung.com>]

Hi,

> -----Original Message-----
> From: Jim Liu [mailto:jim.t90615@gmail.com]
> Sent: Tuesday, November 1, 2022 11:21 AM
> To: jim.t90615@gmail.com; JJLIU0@nuvoton.com; YSCHU@nuvoton.com; KWLIU@nuvoton.com;
> jh80.chung@samsung.com
> Cc: u-boot@lists.denx.de
> Subject: [PATCH v1] power: regulator: Add support for NPCM845
> 
> Add support for setting NPCM845 voltage supply
> 
> regulator-force-microvolt is npcm proprietary property to
> set the voltage level.

What is regulator-force-microvolt. I didn't find this property in u-boot / kernel side.
If I missed something, could you explain it?

And Is there  no pmic for this regulator?

> 
> Signed-off-by: Jim Liu <JJLIU0@nuvoton.com>
> ---
>  drivers/power/regulator/Kconfig             |   8 ++
>  drivers/power/regulator/Makefile            |   1 +
>  drivers/power/regulator/npcm8xx_regulator.c | 133 ++++++++++++++++++++
>  3 files changed, 142 insertions(+)
>  create mode 100644 drivers/power/regulator/npcm8xx_regulator.c
> 
> diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig
> index c519e066ef..e5f06874de 100644
> --- a/drivers/power/regulator/Kconfig
> +++ b/drivers/power/regulator/Kconfig
> @@ -128,6 +128,14 @@ config DM_REGULATOR_MAX77686
>  	features for REGULATOR MAX77686. The driver implements get/set api for:
>  	value, enable and mode.
> 
> +config DM_REGULATOR_NPCM8XX
> +	bool "Enable driver for NPCM8xx voltage supply"

"Enable Driver Model for ...."


> +	depends on DM_REGULATOR && ARCH_NPCM8XX
> +	help
> +	  Enable support for configuring voltage supply on NPCM8XX SoC. The
> +	  voltage supplies support two voltage levels and the driver implements
> +	  get/set api for setting the value.
> +
>  config DM_REGULATOR_FAN53555
>  	bool "Enable Driver Model for REGULATOR FAN53555"
>  	depends on DM_PMIC_FAN53555
> diff --git a/drivers/power/regulator/Makefile b/drivers/power/regulator/Makefile
> index bc736068bc..68e4c0f9dd 100644
> --- a/drivers/power/regulator/Makefile
> +++ b/drivers/power/regulator/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_REGULATOR_ACT8846) += act8846.o
>  obj-$(CONFIG_REGULATOR_AS3722)	+= as3722_regulator.o
>  obj-$(CONFIG_$(SPL_)DM_REGULATOR_DA9063) += da9063.o
>  obj-$(CONFIG_DM_REGULATOR_MAX77686) += max77686.o
> +obj-$(CONFIG_DM_REGULATOR_NPCM8XX) += npcm8xx_regulator.o
>  obj-$(CONFIG_$(SPL_)DM_PMIC_PFUZE100) += pfuze100.o
>  obj-$(CONFIG_$(SPL_)DM_REGULATOR_BD71837) += bd71837.o
>  obj-$(CONFIG_$(SPL_)DM_REGULATOR_PCA9450) += pca9450.o
> diff --git a/drivers/power/regulator/npcm8xx_regulator.c
> b/drivers/power/regulator/npcm8xx_regulator.c
> new file mode 100644
> index 0000000000..7903287132
> --- /dev/null
> +++ b/drivers/power/regulator/npcm8xx_regulator.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2022 Nuvoton Technology Corp.
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <asm/io.h>
> +#include <dm/device_compat.h>
> +#include <power/regulator.h>
> +
> +#define REG_VSRCR	0xf08000e8	/* Voltage Supply Control Register */
> +
> +/* Supported voltage levels (uV) */
> +static const u32 volts_type1[] = { 3300000, 1800000 };
> +static const u32 volts_type2[] = { 1000000, 1800000 };
> +#define VOLT_LEV0	0
> +#define VOLT_LEV1	1
> +
> +struct volt_supply {
> +	char *name;
> +	const u32 *volts;
> +	u32 reg_shift;	/* Register bit offset for setting voltage */
> +};
> +
> +static const struct volt_supply npcm8xx_volt_supps[] = {
> +	{"v1", volts_type1, 0},
> +	{"v2", volts_type1, 1},
> +	{"v3", volts_type1, 2},
> +	{"v4", volts_type1, 3},
> +	{"v5", volts_type1, 4},
> +	{"v6", volts_type1, 5},
> +	{"v7", volts_type1, 6},
> +	{"v8", volts_type1, 7},
> +	{"v9", volts_type1, 8},
> +	{"v10", volts_type1, 9},
> +	{"v11", volts_type2, 10},
> +	{"v12", volts_type1, 11},
> +	{"v13", volts_type1, 12},
> +	{"v14", volts_type2, 13},
> +	{"vsif", volts_type1, 14},
> +	{"vr2", volts_type1, 30},
> +};
> +
> +static const struct volt_supply *npcm8xx_volt_supply_get(const char *name)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(npcm8xx_volt_supps); i++) {
> +		if (!strcmp(npcm8xx_volt_supps[i].name, name))
> +			return &npcm8xx_volt_supps[i];
> +	}
> +
> +	return NULL;
> +}
> +
> +static int npcm8xx_regulator_set_value(struct udevice *dev, int uV)
> +{
> +	struct dm_regulator_uclass_plat *uc_pdata;
> +	const struct volt_supply *supp;
> +	u32 val, level;
> +
> +	uc_pdata = dev_get_uclass_plat(dev);
> +	if (!uc_pdata)
> +		return -ENXIO;
> +
> +	dev_dbg(dev, "%s set_value: %d\n", uc_pdata->name, uV);
> +	supp = npcm8xx_volt_supply_get(uc_pdata->name);
> +	if (!supp)
> +		return -ENOENT;
> +
> +	if (uV == supp->volts[VOLT_LEV0])
> +		level = VOLT_LEV0;
> +	else if (uV == supp->volts[VOLT_LEV1])
> +		level = VOLT_LEV1;
> +	else
> +		return -EINVAL;
> +
> +	/* Set voltage level */
> +	val = readl(REG_VSRCR);
> +	val &= ~BIT(supp->reg_shift);
> +	val |= level << supp->reg_shift;
> +	writel(val, REG_VSRCR);
> +
> +	return 0;
> +}
> +
> +static int npcm8xx_regulator_get_value(struct udevice *dev)
> +{
> +	struct dm_regulator_uclass_plat *uc_pdata;
> +	const struct volt_supply *supp;
> +	u32 val;
> +
> +	uc_pdata = dev_get_uclass_plat(dev);
> +	if (!uc_pdata)
> +		return -ENXIO;
> +
> +	supp = npcm8xx_volt_supply_get(uc_pdata->name);
> +	if (!supp)
> +		return -ENOENT;
> +
> +	val = readl(REG_VSRCR) & BIT(supp->reg_shift);

To Readable, add the one line.

> +	dev_dbg(dev, "%s get_value: %d\n", uc_pdata->name,
> +		val ? supp->volts[VOLT_LEV1] : supp->volts[VOLT_LEV0]);
> +	if (val)
> +		return supp->volts[VOLT_LEV1];
> +	else
> +		return supp->volts[VOLT_LEV0];

Return val ? supp->volts[VOLT_LEV1] : ...; 

> +}
> +
> +static int npcm8xx_regulator_set_enable(struct udevice *dev, bool enable)
> +{
> +	/* Always on */
> +	return 0;
> +}
> +
> +static const struct dm_regulator_ops npcm8xx_regulator_ops = {
> +	.set_value	= npcm8xx_regulator_set_value,
> +	.get_value	= npcm8xx_regulator_get_value,
> +	.set_enable	= npcm8xx_regulator_set_enable,
> +};
> +
> +static const struct udevice_id npcm8xx_regulator_ids[] = {
> +	{ .compatible = "regulator-npcm845" },
> +	{ },
> +};
> +
> +U_BOOT_DRIVER(regulator_npcm8xx) = {
> +	.name = "regulator_npcm845",
> +	.id = UCLASS_REGULATOR,
> +	.ops = &npcm8xx_regulator_ops,
> +	.of_match = npcm8xx_regulator_ids,
> +};
> --
> 2.17.1