Re: [PATCH v3 1/4] regulator: dt-bindings: Add fan53555 allowed modes

[Victor Krawiec]

On 7/28/26 09:28, Krzysztof Kozlowski wrote:
> 
> Feedback of Sashiko looks unanswered, so dropping this from Patchwork.
> 
> Best regards,
> Krzysztof
> 

Sorry I didn't know I had to answer to the AI bot. Should I send again the
patch series since it is dropped from Patchwork ?

Anyway here are the answers to the Sashiko review

On 7/23/26 11:53, sashiko-bot@xxxxxxxxxx wrote:
> New issues:
> - [Medium] The device tree binding introduces the `regulator-initial-mode` property, but the corresponding driver lacks the `.of_map_mode` callback required to use it.

This patch serie includes a commit that adds the `.of_map_mode` callback to
the driver. The callback is added at runtime dynamically depending on the
vendor. Maybe that's why the bot did not notice it. Indeed the driver is
used by several devices clone from different vendors. The `.of_map_mode`
was restricted to Silergy vendor as it is the only vendor I am able to
test.

> - [Low] The patch defines constraints for `regulator-initial-mode` but fails to define constraints for `regulator-allowed-modes`.

According to documentation `regulator-allowed-modes` is the list of
operating modes that software is allowed to configure for the regulator at
run-time. No one has expressed the need to be able to change mode by
software on this regulator for now. The need covered here is to change the
initial mode during regulator init.

> Pre-existing issues:
> - [High] A preexisting bug in `fan53555_set_mode()` corrupts the voltage register when setting `REGULATOR_MODE_NORMAL` on certain vendor variants like RK8602.

For Silergy vendor the `fan53555_set_mode()` writes to the correct register
since `di->mode_reg` is the same as `di->vol_reg`.

> For variants like the RK8602 where mode_reg and vol_reg are distinct
> hardware registers, clearing di->mode_mask (e.g. BIT(6)) in di->vol_reg
> could silently drop the output voltage by 64 steps (400mV). Could this lead
> to severe undervolting and system crashes if a consumer attempts to change
> the mode?

It is true that using `di->vol_reg` in this function seems wrong for
vendors that have distinct registers. However I don't have access to those
devices to test such a change. The submitted patch is perfectly safe as it
ignores the mode change for these untested vendors.