Messages in this thread | | | From | Peter Geis <> | Date | Wed, 26 May 2021 14:41:00 -0400 | Subject | Re: [PATCH] regulator: fan53555: add back tcs4526 |
| |
On Wed, May 26, 2021 at 12:23 PM Rudi Heitbaum <rudi@heitbaum.com> wrote: > > > For rk3399pro boards the tcs4526 regulator supports the vdd_gpu > regulator. The tcs4526 regulator has a chip id of <0>. > Add the compatibile tcs,tcs4526 > > without this patch, the dmesg output is: > fan53555-regulator 0-0010: Chip ID 0 not supported! > fan53555-regulator 0-0010: Failed to setup device! > fan53555-regulator: probe of 0-0010 failed with error -22 > with this patch, the dmesg output is: > vdd_gpu: supplied by vcc5v0_sys > > The regulators are described as: > - Dedicated power management IC TCS4525 > - Lithium battery protection chip TCS4526 > > This has been tested with a Radxa Rock Pi N10. > > Fixes: f9028dcdf589 ("regulator: fan53555: only bind tcs4525 to correct chip id") > Signed-off-by: Rudi Heitbaum <rudi@heitbaum.com>
Considering the TCS4525 wasn't supported prior to its recent addition, and the TCS4526 wasn't supported by the driver at all, this isn't a fix but a feature addition. Binding only to the correct device ID exists for this reason, to prevent unsafe voltage setting.
I also don't see the TCS4525/TCS4526 regulators in the current linux-next device tree for the N10.
> --- > drivers/regulator/fan53555.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c > index 2695be617373..ddab9359ea20 100644 > --- a/drivers/regulator/fan53555.c > +++ b/drivers/regulator/fan53555.c > @@ -90,6 +90,7 @@ enum { > }; > > enum { > + TCS4525_CHIP_ID_00 = 0, > TCS4525_CHIP_ID_12 = 12,
This isn't a TCS4525, but a TCS4526.
> }; > > @@ -373,6 +374,7 @@ static int fan53555_voltages_setup_silergy(struct fan53555_device_info *di) > static int fan53526_voltages_setup_tcs(struct fan53555_device_info *di) > { > switch (di->chip_id) { > + case TCS4525_CHIP_ID_00: > case TCS4525_CHIP_ID_12: > di->slew_reg = TCS4525_TIME; > di->slew_mask = TCS_SLEW_MASK; > @@ -564,6 +566,9 @@ static const struct of_device_id __maybe_unused fan53555_dt_ids[] = { > }, { > .compatible = "tcs,tcs4525", > .data = (void *)FAN53526_VENDOR_TCS > + }, { > + .compatible = "tcs,tcs4526", > + .data = (void *)FAN53526_VENDOR_TCS
Since you aren't adding any functional code, is there a particular reason you can't just add the chip id and simply use the tcs4525 compatible? This will prevent you from needing to modify the dt-bindings as well.
> }, > { } > }; > @@ -672,6 +677,9 @@ static const struct i2c_device_id fan53555_id[] = { > }, { > .name = "tcs4525", > .driver_data = FAN53526_VENDOR_TCS > + }, { > + .name = "tcs4526", > + .driver_data = FAN53526_VENDOR_TCS > }, > { }, > }; > -- > 2.29.2 >
| |