From: ChiYuan Huang <[email protected]>
Add compatible and use DID to check rt5733.
The only difference bwtween rt5733 and rt5739 is the output range and
voltage step. These two chips can be distinguished from the DIE id.
Signed-off-by: ChiYuan Huang <[email protected]>
---
drivers/regulator/rt5739.c | 49 +++++++++++++++++++++++++++++++-------
1 file changed, 41 insertions(+), 8 deletions(-)
diff --git a/drivers/regulator/rt5739.c b/drivers/regulator/rt5739.c
index 74fc5bf6d87e..c930ea27e51f 100644
--- a/drivers/regulator/rt5739.c
+++ b/drivers/regulator/rt5739.c
@@ -31,10 +31,17 @@
#define RT5739_MODEVSEL1_MASK BIT(1)
#define RT5739_MODEVSEL0_MASK BIT(0)
#define RT5739_VID_MASK GENMASK(7, 5)
+#define RT5739_DID_MASK GENMASK(3, 0)
#define RT5739_ACTD_MASK BIT(7)
#define RT5739_ENVSEL1_MASK BIT(1)
#define RT5739_ENVSEL0_MASK BIT(0)
+#define RT5733_CHIPDIE_ID 0x1
+#define RT5733_VOLT_MINUV 270000
+#define RT5733_VOLT_MAXUV 1401250
+#define RT5733_VOLT_STPUV 6250
+#define RT5733_N_VOLTS 182
+
#define RT5739_VOLT_MINUV 300000
#define RT5739_VOLT_MAXUV 1300000
#define RT5739_VOLT_STPUV 5000
@@ -92,9 +99,26 @@ static int rt5739_set_suspend_voltage(struct regulator_dev *rdev, int uV)
{
const struct regulator_desc *desc = rdev->desc;
struct regmap *regmap = rdev_get_regmap(rdev);
- unsigned int reg, vsel;
+ unsigned int did, reg, vsel;
+ int min_uV, max_uV, step_uV, ret;
+
+ ret = regmap_read(regmap, RT5739_REG_ID1, &did);
+ if (ret)
+ return ret;
+
+ did &= RT5739_DID_MASK;
+
+ if (did == RT5733_CHIPDIE_ID) {
+ min_uV = RT5733_VOLT_MINUV;
+ max_uV = RT5733_VOLT_MAXUV;
+ step_uV = RT5733_VOLT_STPUV;
+ } else {
+ min_uV = RT5739_VOLT_MINUV;
+ max_uV = RT5739_VOLT_MAXUV;
+ step_uV = RT5739_VOLT_STPUV;
+ }
- if (uV < RT5739_VOLT_MINUV || uV > RT5739_VOLT_MAXUV)
+ if (uV < min_uV || uV > max_uV)
return -EINVAL;
if (desc->vsel_reg == RT5739_REG_NSEL0)
@@ -102,7 +126,7 @@ static int rt5739_set_suspend_voltage(struct regulator_dev *rdev, int uV)
else
reg = RT5739_REG_NSEL0;
- vsel = (uV - RT5739_VOLT_MINUV) / RT5739_VOLT_STPUV;
+ vsel = (uV - min_uV) / step_uV;
return regmap_write(regmap, reg, vsel);
}
@@ -189,15 +213,12 @@ static unsigned int rt5739_of_map_mode(unsigned int mode)
}
static void rt5739_init_regulator_desc(struct regulator_desc *desc,
- bool vsel_active_high)
+ bool vsel_active_high, u8 did)
{
/* Fixed */
desc->name = "rt5739-regulator";
desc->owner = THIS_MODULE;
desc->ops = &rt5739_regulator_ops;
- desc->n_voltages = RT5739_N_VOLTS;
- desc->min_uV = RT5739_VOLT_MINUV;
- desc->uV_step = RT5739_VOLT_STPUV;
desc->vsel_mask = RT5739_VSEL_MASK;
desc->enable_reg = RT5739_REG_CNTL2;
desc->active_discharge_reg = RT5739_REG_CNTL1;
@@ -213,6 +234,17 @@ static void rt5739_init_regulator_desc(struct regulator_desc *desc,
desc->vsel_reg = RT5739_REG_NSEL0;
desc->enable_mask = RT5739_ENVSEL0_MASK;
}
+
+ /* Assigned by CHIPDIE ID */
+ if (did == RT5733_CHIPDIE_ID) {
+ desc->n_voltages = RT5733_N_VOLTS;
+ desc->min_uV = RT5733_VOLT_MINUV;
+ desc->uV_step = RT5733_VOLT_STPUV;
+ } else {
+ desc->n_voltages = RT5739_N_VOLTS;
+ desc->min_uV = RT5739_VOLT_MINUV;
+ desc->uV_step = RT5739_VOLT_STPUV;
+ }
}
static const struct regmap_config rt5739_regmap_config = {
@@ -258,7 +290,7 @@ static int rt5739_probe(struct i2c_client *i2c)
vsel_acth = device_property_read_bool(dev, "richtek,vsel-active-high");
- rt5739_init_regulator_desc(desc, vsel_acth);
+ rt5739_init_regulator_desc(desc, vsel_acth, vid & RT5739_DID_MASK);
cfg.dev = dev;
cfg.of_node = dev_of_node(dev);
@@ -271,6 +303,7 @@ static int rt5739_probe(struct i2c_client *i2c)
}
static const struct of_device_id rt5739_device_table[] = {
+ { .compatible = "richtek,rt5733" },
{ .compatible = "richtek,rt5739" },
{ /* sentinel */ }
};
--
2.40.1
On Wed, Jun 28, 2023 at 04:47:17PM +0800, [email protected] wrote:
> + if (did == RT5733_CHIPDIE_ID) {
> + min_uV = RT5733_VOLT_MINUV;
> + max_uV = RT5733_VOLT_MAXUV;
> + step_uV = RT5733_VOLT_STPUV;
> + } else {
> + min_uV = RT5739_VOLT_MINUV;
> + max_uV = RT5739_VOLT_MAXUV;
> + step_uV = RT5739_VOLT_STPUV;
> + }
It would be better to write these as switch statements so if any more
variants turn up they can be added more easily.
On Wed, Jun 28, 2023 at 12:47:50PM +0100, Mark Brown wrote:
> On Wed, Jun 28, 2023 at 04:47:17PM +0800, [email protected] wrote:
>
> > + if (did == RT5733_CHIPDIE_ID) {
> > + min_uV = RT5733_VOLT_MINUV;
> > + max_uV = RT5733_VOLT_MAXUV;
> > + step_uV = RT5733_VOLT_STPUV;
> > + } else {
> > + min_uV = RT5739_VOLT_MINUV;
> > + max_uV = RT5739_VOLT_MAXUV;
> > + step_uV = RT5739_VOLT_STPUV;
> > + }
>
> It would be better to write these as switch statements so if any more
> variants turn up they can be added more easily.
Since the IC difference is only voltage range and step, They can be retrieved from the
regulator description.
To check DID here may not a good coding.
I may rewrite it as below
max_uV = desc->min_uV + desc->uV_step * (desc->n_voltages - 1);
And put a switch case for DID check in 'init_regulator_desc'.
Is it better?