The tcs4525 voltage calculation is incorrect, which leads to a deadlock
on the rk3566-quartz64 board when loading cpufreq.
Fix the voltage calculation to correct the deadlock.
While we are at it, add a safety check and clean up the function names
to be more accurate.
Peter Geis (3):
regulator: fan53555: fix TCS4525 voltage calulation
regulator: fan53555: only bind tcs4525 to correct chip id
regulator: fan53555: fix tcs4525 function names
drivers/regulator/fan53555.c | 44 ++++++++++++++++++++++--------------
1 file changed, 27 insertions(+), 17 deletions(-)
--
2.25.1
The tcs4525 regulator has a chip id of <12>.
Only allow the driver to bind to the correct chip id for safety, in
accordance with the other supported devices.
Signed-off-by: Peter Geis <[email protected]>
---
drivers/regulator/fan53555.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c
index 26f06f685b1b..16f28f9df6a1 100644
--- a/drivers/regulator/fan53555.c
+++ b/drivers/regulator/fan53555.c
@@ -89,6 +89,10 @@ enum {
FAN53555_CHIP_ID_08 = 8,
};
+enum {
+ TCS4525_CHIP_ID_12 = 12,
+};
+
/* IC mask revision */
enum {
FAN53555_CHIP_REV_00 = 0x3,
@@ -368,14 +372,21 @@ static int fan53555_voltages_setup_silergy(struct fan53555_device_info *di)
static int fan53555_voltages_setup_tcs(struct fan53555_device_info *di)
{
- di->slew_reg = TCS4525_TIME;
- di->slew_mask = TCS_SLEW_MASK;
- di->slew_shift = TCS_SLEW_MASK;
+ switch (di->chip_id) {
+ case TCS4525_CHIP_ID_12:
+ di->slew_reg = TCS4525_TIME;
+ di->slew_mask = TCS_SLEW_MASK;
+ di->slew_shift = TCS_SLEW_MASK;
- /* Init voltage range and step */
- di->vsel_min = 600000;
- di->vsel_step = 6250;
- di->vsel_count = FAN53526_NVOLTAGES;
+ /* Init voltage range and step */
+ di->vsel_min = 600000;
+ di->vsel_step = 6250;
+ di->vsel_count = FAN53526_NVOLTAGES;
+ break;
+ default:
+ dev_err(di->dev, "Chip ID %d not supported!\n", di->chip_id);
+ return -EINVAL;
+ }
return 0;
}
--
2.25.1
The tcs4525 is based off the fan53526.
Rename the tcs4525 functions to align with this.
Signed-off-by: Peter Geis <[email protected]>
---
drivers/regulator/fan53555.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c
index 16f28f9df6a1..2695be617373 100644
--- a/drivers/regulator/fan53555.c
+++ b/drivers/regulator/fan53555.c
@@ -67,7 +67,7 @@ enum fan53555_vendor {
FAN53526_VENDOR_FAIRCHILD = 0,
FAN53555_VENDOR_FAIRCHILD,
FAN53555_VENDOR_SILERGY,
- FAN53555_VENDOR_TCS,
+ FAN53526_VENDOR_TCS,
};
enum {
@@ -233,7 +233,7 @@ static int fan53555_set_ramp(struct regulator_dev *rdev, int ramp)
slew_rate_t = slew_rates;
slew_rate_n = ARRAY_SIZE(slew_rates);
break;
- case FAN53555_VENDOR_TCS:
+ case FAN53526_VENDOR_TCS:
slew_rate_t = tcs_slew_rates;
slew_rate_n = ARRAY_SIZE(tcs_slew_rates);
break;
@@ -370,7 +370,7 @@ static int fan53555_voltages_setup_silergy(struct fan53555_device_info *di)
return 0;
}
-static int fan53555_voltages_setup_tcs(struct fan53555_device_info *di)
+static int fan53526_voltages_setup_tcs(struct fan53555_device_info *di)
{
switch (di->chip_id) {
case TCS4525_CHIP_ID_12:
@@ -420,7 +420,7 @@ static int fan53555_device_setup(struct fan53555_device_info *di,
return -EINVAL;
}
break;
- case FAN53555_VENDOR_TCS:
+ case FAN53526_VENDOR_TCS:
switch (pdata->sleep_vsel_id) {
case FAN53555_VSEL_ID_0:
di->sleep_reg = TCS4525_VSEL0;
@@ -459,7 +459,7 @@ static int fan53555_device_setup(struct fan53555_device_info *di,
di->mode_reg = di->vol_reg;
di->mode_mask = VSEL_MODE;
break;
- case FAN53555_VENDOR_TCS:
+ case FAN53526_VENDOR_TCS:
di->mode_reg = TCS4525_COMMAND;
switch (pdata->sleep_vsel_id) {
@@ -487,8 +487,8 @@ static int fan53555_device_setup(struct fan53555_device_info *di,
case FAN53555_VENDOR_SILERGY:
ret = fan53555_voltages_setup_silergy(di);
break;
- case FAN53555_VENDOR_TCS:
- ret = fan53555_voltages_setup_tcs(di);
+ case FAN53526_VENDOR_TCS:
+ ret = fan53526_voltages_setup_tcs(di);
break;
default:
dev_err(di->dev, "vendor %d not supported!\n", di->vendor);
@@ -563,7 +563,7 @@ static const struct of_device_id __maybe_unused fan53555_dt_ids[] = {
.data = (void *)FAN53555_VENDOR_SILERGY,
}, {
.compatible = "tcs,tcs4525",
- .data = (void *)FAN53555_VENDOR_TCS
+ .data = (void *)FAN53526_VENDOR_TCS
},
{ }
};
@@ -671,7 +671,7 @@ static const struct i2c_device_id fan53555_id[] = {
.driver_data = FAN53555_VENDOR_SILERGY
}, {
.name = "tcs4525",
- .driver_data = FAN53555_VENDOR_TCS
+ .driver_data = FAN53526_VENDOR_TCS
},
{ },
};
--
2.25.1
The TCS4525 has 128 voltage steps. With the calculation set to 127 the
most significant bit is disregarded which leads to a miscalculation of
the voltage by about 200mv.
Fix the calculation to end deadlock on the rk3566-quartz64 which uses
this as the cpu regulator.
Fixes: 914df8faa7d6 ("regulator: fan53555: Add TCS4525 DCDC support")
Signed-off-by: Peter Geis <[email protected]>
---
drivers/regulator/fan53555.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c
index f3918f03aaf3..26f06f685b1b 100644
--- a/drivers/regulator/fan53555.c
+++ b/drivers/regulator/fan53555.c
@@ -55,7 +55,6 @@
#define FAN53555_NVOLTAGES 64 /* Numbers of voltages */
#define FAN53526_NVOLTAGES 128
-#define TCS4525_NVOLTAGES 127 /* Numbers of voltages */
#define TCS_VSEL_NSEL_MASK 0x7f
#define TCS_VSEL0_MODE (1 << 7)
@@ -376,7 +375,7 @@ static int fan53555_voltages_setup_tcs(struct fan53555_device_info *di)
/* Init voltage range and step */
di->vsel_min = 600000;
di->vsel_step = 6250;
- di->vsel_count = TCS4525_NVOLTAGES;
+ di->vsel_count = FAN53526_NVOLTAGES;
return 0;
}
--
2.25.1
On Tue, 11 May 2021 17:13:32 -0400, Peter Geis wrote:
> The tcs4525 voltage calculation is incorrect, which leads to a deadlock
> on the rk3566-quartz64 board when loading cpufreq.
> Fix the voltage calculation to correct the deadlock.
> While we are at it, add a safety check and clean up the function names
> to be more accurate.
>
> Peter Geis (3):
> regulator: fan53555: fix TCS4525 voltage calulation
> regulator: fan53555: only bind tcs4525 to correct chip id
> regulator: fan53555: fix tcs4525 function names
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[1/3] regulator: fan53555: fix TCS4525 voltage calulation
commit: f8c8871f5eff3981eeb13421aca2c1cfda4a5204
[2/3] regulator: fan53555: only bind tcs4525 to correct chip id
commit: f9028dcdf589f4ab528372088623aa4e8d324df2
[3/3] regulator: fan53555: fix tcs4525 function names
commit: b3cc8ec04f50d9c860534fe4e3617a8d10ed9ea9
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
Hi Peter,
On Wed, 2021-05-12 at 18:04 +0100, Mark Brown wrote:
> On Tue, 11 May 2021 17:13:32 -0400, Peter Geis wrote:
> > The tcs4525 voltage calculation is incorrect, which leads to a deadlock
> > on the rk3566-quartz64 board when loading cpufreq.
> > Fix the voltage calculation to correct the deadlock.
> > While we are at it, add a safety check and clean up the function names
> > to be more accurate.
> >
> > Peter Geis (3):
> > regulator: fan53555: fix TCS4525 voltage calulation
> > regulator: fan53555: only bind tcs4525 to correct chip id
> > regulator: fan53555: fix tcs4525 function names
> >
> > [...]
>
> Applied to
>
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
>
> Thanks!
>
> [1/3] regulator: fan53555: fix TCS4525 voltage calulation
> commit: f8c8871f5eff3981eeb13421aca2c1cfda4a5204
> [2/3] regulator: fan53555: only bind tcs4525 to correct chip id
> commit: f9028dcdf589f4ab528372088623aa4e8d324df2
> [3/3] regulator: fan53555: fix tcs4525 function names
> commit: b3cc8ec04f50d9c860534fe4e3617a8d10ed9ea9
>
I know this is already applied, but since I tested it,
this indeed fixes cpufreq for me:
Tested-by: Ezequiel Garcia <[email protected]>
Thanks a lot for taking care of it!