Re: [PATCH RFC v3 1/5] regulator: move monitor handling into own function

[Benjamin Bara <bbara93@gmail.com>]

Hi Matti,

thanks for the feedback!

On Tue, 23 May 2023 at 11:46, Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> As far as I see, this changes the existing logic. Previously the
> monitoring was unconditionally enabled for all regulators, now it gets
> only enabled for regulators which are marked as enabled.
>
> Furthermore, if I am not reading this wrong, the code tries to disable
> all protections if regulator is not enabled at startup(?)
>
> I am not saying this is wrong. I am just saying that things will
> change here and likely to break something.
>
> There are PMICs like ROHM BD9576, where the protection can not be
> disabled.

Thanks for letting me know! I dropped my initial "disable monitor while
disabling the regulator" property, and activated it per default instead.
But this basically means something like that will be required. I guess
it might make sense to have a property which is called something like
"monitor always on", to let the driver inform the core that the monitors
cannot or should not be disabled, instead.
Except if you think there is a general problem with keeping monitors
disabled while the regulator is disabled, then I might have to do it
differently.


> I am unsure if we might also have cases where some regulator could
> really be enabled w/o core knowing it. 

Unfortunately, I am not 100% sure what you mean by that.
On the da9063, for example, it might be possible that a monitor is
activated by the OTP, without that the kernel actually activates it.
I think it is not recommended, but it is possible.


> There can also be a problem if we have hardware where monitoring is
> common for all regulators, eg either globally enabled / disabled.

Yes, but I think in this case it should be the responsibility of the
driver to ensure that either all or no regulator is monitored, because
the same requirement is valid for implementing the protection ops.

Best regards,
Benjamin