On Wed, May 26, 2021 at 12:23 PM Rudi Heitbaum <rudi@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
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
>
