The .bypass_val_on setting does not match the .bypass_mask setting, so the
.bypass_mask bit will never get set. Fix it by removing .bypass_val_on
setting, the regulator_set_bypass_regmap and regulator_get_bypass_regmap
helpers will use rdev->desc->bypass_mask as val_on if the val_on is 0.
Signed-off-by: Axel Lin <[email protected]>
---
Hi Jerome,
I don't have this h/w to test, please help to review and test the patch.
thanks,
Axel
drivers/regulator/tps65219-regulator.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/regulator/tps65219-regulator.c b/drivers/regulator/tps65219-regulator.c
index ab16e6665625..7054d8805dd4 100644
--- a/drivers/regulator/tps65219-regulator.c
+++ b/drivers/regulator/tps65219-regulator.c
@@ -117,7 +117,6 @@ struct tps65219_regulator_irq_data {
.fixed_uV = _fuv, \
.bypass_reg = _vr, \
.bypass_mask = _bpm, \
- .bypass_val_on = 1, \
} \
static const struct linear_range bucks_ranges[] = {
--
2.34.1
On Sun, 28 Aug 2022 20:01:53 +0800, Axel Lin wrote:
> The .bypass_val_on setting does not match the .bypass_mask setting, so the
> .bypass_mask bit will never get set. Fix it by removing .bypass_val_on
> setting, the regulator_set_bypass_regmap and regulator_get_bypass_regmap
> helpers will use rdev->desc->bypass_mask as val_on if the val_on is 0.
>
>
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[1/1] regulator: tps65219: Fix .bypass_val_on setting
commit: 69a673c9e54d952cf404f80169d3100b7a9645bb
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 Axel,
On 28/08/2022 14:01, Axel Lin wrote:
> The .bypass_val_on setting does not match the .bypass_mask setting, so the
> .bypass_mask bit will never get set. Fix it by removing .bypass_val_on
> setting, the regulator_set_bypass_regmap and regulator_get_bypass_regmap
> helpers will use rdev->desc->bypass_mask as val_on if the val_on is 0.
I think this will result in exact same behavior. val would be assigned
to 1 when enable is set and 0 otherwise. Anyway you are right this line
is useless.
> I don't have this h/w to test, please help to review and test the patch.
I'll double check on the board.
jerome Neanne <[email protected]> 於 2022年9月12日 週一 下午5:49寫道：
>
> Hi Axel,
>
> On 28/08/2022 14:01, Axel Lin wrote:
> > The .bypass_val_on setting does not match the .bypass_mask setting, so the
> > .bypass_mask bit will never get set. Fix it by removing .bypass_val_on
> > setting, the regulator_set_bypass_regmap and regulator_get_bypass_regmap
> > helpers will use rdev->desc->bypass_mask as val_on if the val_on is 0.
> I think this will result in exact same behavior. val would be assigned
> to 1 when enable is set and 0 otherwise. Anyway you are right this line
> is useless.
Setting .bypass_val_on=1 won't set TPS65219_LDOS_BYP_CONFIG_MASK bit.
The TPS65219_LDOS_BYP_CONFIG_MASK is BIT(6), so you need to set BIT(6)
instead of 1 for .bypass_val_on.
Remove .bypass_val_on setting then it will use .bypass_mask as
.bypass_val_on setting.
Regards,
Axel
Got it! And validate it.
On 13/09/2022 01:48, Axel Lin wrote:
> jerome Neanne <[email protected]> 於 2022年9月12日 週一 下午5:49寫道：
>> Hi Axel,
>>
>> On 28/08/2022 14:01, Axel Lin wrote:
>>> The .bypass_val_on setting does not match the .bypass_mask setting, so the
>>> .bypass_mask bit will never get set. Fix it by removing .bypass_val_on
>>> setting, the regulator_set_bypass_regmap and regulator_get_bypass_regmap
>>> helpers will use rdev->desc->bypass_mask as val_on if the val_on is 0.
>> I think this will result in exact same behavior. val would be assigned
>> to 1 when enable is set and 0 otherwise. Anyway you are right this line
>> is useless.
> Setting .bypass_val_on=1 won't set TPS65219_LDOS_BYP_CONFIG_MASK bit.
> The TPS65219_LDOS_BYP_CONFIG_MASK is BIT(6), so you need to set BIT(6)
> instead of 1 for .bypass_val_on.
> Remove .bypass_val_on setting then it will use .bypass_mask as
> .bypass_val_on setting.
Got your point. The issue is on the get path not on the set path...
I misinterpreted val_on usage.
I was able to confirm on board that cat
/sys/class/regulator/regulator.10/bypass
always return disabled before your fix is applied.
After your fix is applied, it accurately return enabled/disabled
depending how LDO is set (forced at probe).
side note:
- I added virtual regulator and userspace consumer to validate (that's
why VDDSHV_SD_IO_PMIC is regulator.10)
- This bug was missed because I did not re-validated the bypass after I
changed from custom to standard helpers (I apologize).
Thanks for raising this
Regards,
Jerome.