[PATCH v2 0/2] power: pmic: add TPS65913 support

[Svyatoslav Ryhel]

16 липня 2023 р. 17:27:39 GMT+03:00, Jonas Karlman <jonas at kwiboo.se> написав(-ла):
>On 2023-07-16 15:17, Svyatoslav Ryhel wrote:
>> 
>> 
>> 16 липня 2023 р. 16:07:55 GMT+03:00, Simon Glass <sjg at chromium.org> написав(-ла):
>>> Hi Svyatoslav,
>>>
>>> On Sun, 16 Jul 2023 at 07:04, Svyatoslav Ryhel <clamor95 at gmail.com> wrote:
>>>>
>>>>
>>>>
>>>> 16 липня 2023 р. 11:07:15 GMT+03:00, Jonas Karlman <jonas at kwiboo.se> написав(-ла):
>>>>> On 2023-07-16 05:57, Svyatoslav Ryhel wrote:
>>>>>>
>>>>>>
>>>>>> 16 липня 2023 р. 02:37:39 GMT+03:00, Jonas Karlman <jonas at kwiboo.se> написав(-ла):
>>>>>>> On 2023-07-15 20:34, Svyatoslav Ryhel wrote:
>>>>>>>> Existing PALMAS PMIC driver is fully compatible with TI TPS65913
>>>>>>>> PMIC found in many Tegra 4 devices, like Tegra Note 7 and ASUS
>>>>>>>> TF701T. Add TPS65913 dts compatible with TPS659038 data.
>>>>>>>>
>>>>>>>> Issue with regulators is more general then I though initially.
>>>>>>>> It touches all pmic regulators.
>>>>>>>>
>>>>>>>> Currently device tree entries of regulators are completely
>>>>>>>> ignored and regulators are probed only if they are called
>>>>>>>> by the device which uses it. This results into two issues:
>>>>>>>> regulators which must run under boot-on or always-on mode
>>>>>>>> are ignored and not enabled; dts props like voltage are
>>>>>>>> not applied to the regulator so the regulator may be enabled
>>>>>>>> with random actual voltage, which may have unexpected
>>>>>>>> consequences.
>>>>>>>
>>>>>>> This sounds like something a call to regulators_enable_boot_on like is
>>>>>>> done on other platforms/boards could solve?
>>>>>>
>>>>>> Yes, regulators_enable_boot_on can solve this if called from the board, but why should every device call this from the board if this should be done automatically? What should do devices without board? Isn't u-boot moving towards all in device tree setup?
>>>>>
>>>>> Main diff is that regulators_enable_boot_on covers all regulators,
>>>>> including fixed/gpio/pwm regulators, not just regulators that are
>>>>> children of a pmic. Meaning platforms/boards would still need to somehow
>>>>> autoset non-pmic regulators.
>>>>
>>>> This is a good point. Non pmic regulators are not probed/set.
>>>>
>>>>> Maybe regulators_enable_boot_on could be called from the default
>>>>> power_init_board or similar, when a Kconfig is enabled, to cover more
>>>>> platforms/boards.
>>>>
>>>> Since u-boot is moving towards device trees this should be undesireable approach. What is possible is to set all non pmic regulators to probe on post bind and on post probe call regulator_autoset.
>>>
>>> Yes we should handle this automatically without board-specific C code,
>>> where possible.
>> 
>> I will look into this and reload patches once done. Only downfall I can see right now is that strongly bound regulators (like pmic-regulator complex) will require more attention. From what I can remember such bounds may be battery-regulator and charger-regulator. I will exclude those from probing as well. But if regulator is felf sustained device. I see no downsides.
>
>Great, keep in mind that after commit 4fcba5d556b4 ("regulator: implement
>basic reference counter"), calling autoset multiple times on fixed/gpio
>regulator would increase their reference counter.
>
>Meaning regulators_enable_boot_on should probably be if calling autoset
>is moved to regulator uclass post probe or similar. Else we may end up
>with boot-on regulators that start with a reference counter of >1
>instead of =1.

Thanks for a warning.

Ideally regulators_enable_boot_on/regulators_enable_boot_off should be removed at all. Then all regulators stuff will be handled within regulators uclass.

>Regards,
>Jonas
>
>> 
>>> Regards,
>>> Simon
>