[PATCH v3 04/11] imx8mp: power-domain: Don't power off pd_bus

[Sumit Garg]

On Thu, 21 Mar 2024 at 11:06, Marek Vasut <marex at denx.de> wrote:
>
> On 3/15/24 11:41 AM, Sumit Garg wrote:
> > On Fri, 15 Mar 2024 at 14:53, Marek Vasut <marex at denx.de> wrote:
> >>
> >> On 3/15/24 6:31 AM, Sumit Garg wrote:
> >>> On Thu, 14 Mar 2024 at 09:45, Marek Vasut <marex at denx.de> wrote:
> >>>>
> >>>> On 3/12/24 8:03 AM, Sumit Garg wrote:
> >>>>> power_domain_on/off() isn't refcounted and power domain bus shouldn't be
> >>>>> turned off for a single peripheral domain as it would negatively affect
> >>>>> other peripheral domains. So lets just skip turning off bus power
> >>>>> domain.
> >>>>
> >>>> What exactly is the issue and how did you trigger it ?
> >>>>
> >>>> Details please.
> >>>
> >>> I suppose the issue can be triggered via the "=> usb start => usb
> >>> stop" sequence where one of the USB controllers is configured in
> >>> peripheral mode.
> >>
> >> 'usb start ; usb stop' causes no problems on MX8MP , maybe the test case
> >> is more extensive ?
> >>
> >> Please, write down the necessary steps to reproduce this problem, and
> >> what happens when that problem occurs.
> >
> > After digging in more, it looks like dev_power_domain_off() is never
> > (U-Boot life-cycle) invoked for USB controller devices derived from
> > DT. So this USB power domain sequence is never reachable.
>
> The imx8mp_hsiomix_off() is never called on 'usb stop' command ?
>

Yeah, that's the case.

> But then why would the 'usb start ; usb stop' test break power domain
> state here ?

It won't break with current implementation, earlier I made this
assumption that 'usb stop' turns down the power domain.

>
> > BTW, dev_power_domain_on() is invoked when USB controller devices are
> > added based on DT.
>
> I would expect imx8mp_hsiomix_off() to be called either on 'usb stop' or
> just before Linux boots .
>
> [...]
>
> >>>> Why not add counter into imx8mp_hsiomix_priv structure in this driver ?
> >>>
> >>> Sure I can do that but do you think the current approach can have any
> >>> side effects?
> >>
> >> Bus domain not getting cycled (which can leave it in some odd state),
> >> and increased power consumption if the next stage doesn't turn the
> >> domain off.
> >
> > Given above, would you like me to drop power domain off path entirely
> > here?
>
> Can the series go in without this patch ?

Okay let me drop this patch.

>
> > I think if people are concerned about power consumption then it
> > should be implemented properly in U-Boot to remove all the DT based
> > devices before passing on control to the next stage.
>
> I would expect imx8mp_hsiomix_off() to be called either on 'usb stop' or
> just before Linux boots (esp. at that point), so if you do not power off
> the bus domain before booting Linux, you may hand over a device which
> was not fully power cycled.

Unfortunately that's the current situation I see. IMO, the better
solution would be to just remove all the DT devices before passing on
control to Linux. That should automatically power off devices.

>
> And sorry for the delay, I was a bit busy.

No worries.

-Sumit