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

[Jonas Karlman]

On 2023-07-16 20:57, 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.
> 
> Commit, Jonas Karlman refers to 4fcba5d556b4 ("regulator: implement
> basic reference counter") introduces too much conflicting.
> 
> 1. It should not have been merged in the current state and instead expanded for all uclass. Counting regulator users is not fixed/gpio regulator specific and can be beneficial for all devices.

I think it was limited to fixed/gpio regulators just to not affect too
many arch/boards. And can always be extended to core regulator uclass in
the future.

There is also drivers that do not currently keep enable/disable balanced,
will shortly send a series with fixes for a few I have discovered.

> 
> 2. Usage of board and device tree call to set up regulators will not work well with counting enables/disables. The correct solution should be to remove board calls for regulator setup but it will involve rework of many boards.

This was main reason why I mentioned this commit, it currently works, but
if you introduce new code that call regulator_autoset from post probe,
existing code must be adjusted or removed.

E.g. regulators_enable_boot_on called in a rockchip arch shared
board_init could be removed. Or logic in regulator_autoset can be
changed to only run code once for each regulator. Or add a Kconfig to
enable post probe autoset and disable autoset in regulators_enable_boot_on.

Regards,
Jonas

> 
>> Regards,
>> Simon