| Message ID | 20180216041705.6340-1-d-gerlach@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Friday 16 February 2018 09:47 AM, Dave Gerlach wrote: > Commit 2dc4940360d4 ("regulator: tps65218: Remove all the compatibles") > changes the probe function of drivers/regulator/tps65218-regulator.c so > that it iterates through all available regulators and assumes that the > regulator IDs are sequential and match the order present in the enum > tps65218_regulator_id. However, for some reason the much older commit > c0ea88b890d6 ("regulator: tps65218: add support for LS3 current > regulator") updated all arrays with LS3 at the end but added it second > to last for the enum. > > Because of this long standing mismatch in order between the > tps65218_regulator_id enum and the regulator_desc array in the tps65218 > regulator driver, the new probe function causes the strobe values to be > associated with the wrong regulator ID. This causes LDO1 to fail to > suspend in tps65218_pmic_set_suspend_disable due to not having anything > probes for its strobe value. Fix the order in the enum so the probe > function works as the update intended. Reviewed-by: Keerthy <j-keerthy@ti.com> > > Fixes: 2dc4940360d4 ("regulator: tps65218: Remove all the compatibles") > Signed-off-by: Dave Gerlach <d-gerlach@ti.com> > --- > include/linux/mfd/tps65218.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/mfd/tps65218.h b/include/linux/mfd/tps65218.h > index f069c518c0ed..c204d9a79436 100644 > --- a/include/linux/mfd/tps65218.h > +++ b/include/linux/mfd/tps65218.h > @@ -205,10 +205,10 @@ enum tps65218_regulator_id { > TPS65218_DCDC_4, > TPS65218_DCDC_5, > TPS65218_DCDC_6, > - /* LS's */ > - TPS65218_LS_3, > /* LDOs */ > TPS65218_LDO_1, > + /* LS's */ > + TPS65218_LS_3, > }; > > #define TPS65218_MAX_REG_ID TPS65218_LDO_1 >
On Thu, 15 Feb 2018, Dave Gerlach wrote: > Commit 2dc4940360d4 ("regulator: tps65218: Remove all the compatibles") > changes the probe function of drivers/regulator/tps65218-regulator.c so > that it iterates through all available regulators and assumes that the > regulator IDs are sequential and match the order present in the enum > tps65218_regulator_id. However, for some reason the much older commit > c0ea88b890d6 ("regulator: tps65218: add support for LS3 current > regulator") updated all arrays with LS3 at the end but added it second > to last for the enum. > > Because of this long standing mismatch in order between the > tps65218_regulator_id enum and the regulator_desc array in the tps65218 > regulator driver, the new probe function causes the strobe values to be > associated with the wrong regulator ID. This causes LDO1 to fail to > suspend in tps65218_pmic_set_suspend_disable due to not having anything > probes for its strobe value. Fix the order in the enum so the probe > function works as the update intended. This sounds super fragile. I'm willing to accept this patch if it 'makes stuff work', but we should look into why a dynamically assigned enum is sensitive to ordering. At the very least you should consider providing a comment with working to that effect. > Fixes: 2dc4940360d4 ("regulator: tps65218: Remove all the compatibles") > Signed-off-by: Dave Gerlach <d-gerlach@ti.com> > --- > include/linux/mfd/tps65218.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Applied, thanks.
diff --git a/include/linux/mfd/tps65218.h b/include/linux/mfd/tps65218.h index f069c518c0ed..c204d9a79436 100644 --- a/include/linux/mfd/tps65218.h +++ b/include/linux/mfd/tps65218.h @@ -205,10 +205,10 @@ enum tps65218_regulator_id { TPS65218_DCDC_4, TPS65218_DCDC_5, TPS65218_DCDC_6, - /* LS's */ - TPS65218_LS_3, /* LDOs */ TPS65218_LDO_1, + /* LS's */ + TPS65218_LS_3, }; #define TPS65218_MAX_REG_ID TPS65218_LDO_1
Commit 2dc4940360d4 ("regulator: tps65218: Remove all the compatibles") changes the probe function of drivers/regulator/tps65218-regulator.c so that it iterates through all available regulators and assumes that the regulator IDs are sequential and match the order present in the enum tps65218_regulator_id. However, for some reason the much older commit c0ea88b890d6 ("regulator: tps65218: add support for LS3 current regulator") updated all arrays with LS3 at the end but added it second to last for the enum. Because of this long standing mismatch in order between the tps65218_regulator_id enum and the regulator_desc array in the tps65218 regulator driver, the new probe function causes the strobe values to be associated with the wrong regulator ID. This causes LDO1 to fail to suspend in tps65218_pmic_set_suspend_disable due to not having anything probes for its strobe value. Fix the order in the enum so the probe function works as the update intended. Fixes: 2dc4940360d4 ("regulator: tps65218: Remove all the compatibles") Signed-off-by: Dave Gerlach <d-gerlach@ti.com> --- include/linux/mfd/tps65218.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)