Re: [PATCH v2 1/6] dt-bindings: ti, sci: Add property for partial-io-wakeup-sources

Markus Schneider-Pargmann
2024-09-05T09:52:54Z

On Thu, Sep 05, 2024 at 11:25:48AM GMT, Krzysztof Kozlowski wrote:
> On 05/09/2024 11:15, Krzysztof Kozlowski wrote:
> > On 05/09/2024 11:08, Markus Schneider-Pargmann wrote:
> >> On Tue, Aug 06, 2024 at 10:03:00AM GMT, Krzysztof Kozlowski wrote:
> >>> On 06/08/2024 09:11, Markus Schneider-Pargmann wrote:
> >>>> Hi Krzysztof,
> >>>>
> >>>> On Tue, Aug 06, 2024 at 08:18:01AM GMT, Krzysztof Kozlowski wrote:
> >>>>> On 29/07/2024 10:00, Markus Schneider-Pargmann wrote:
> >>>>>> Partial-IO is a very low power mode in which nearly everything is
> >>>>>> powered off. Only pins of a few hardware units are kept sensitive and
> >>>>>> are capable to wakeup the SoC. The device nodes are marked as
> >>>>>> 'wakeup-source' but so are a lot of other device nodes as well that are
> >>>>>> not able to do a wakeup from Partial-IO. This creates the need to
> >>>>>> describe the device nodes that are capable of wakeup from Partial-IO.
> >>>>>>
> >>>>>> This patch adds a property with a list of these nodes defining which
> >>>>>> devices can be used as wakeup sources in Partial-IO.
> >>>>>>
> >>>>>
> >>>>>
> >>>>> This is a friendly reminder during the review process.
> >>>>>
> >>>>> It seems my or other reviewer's previous comments were not fully
> >>>>> addressed. Maybe the feedback got lost between the quotes, maybe you
> >>>>> just forgot to apply it. Please go back to the previous discussion and
> >>>>> either implement all requested changes or keep discussing them.
> >>>>>
> >>>>> Thank you.
> >>>>>
> >>>>
> >>>> I tried to address your comment from last version by explaining more
> >>>> thoroughly what the binding is for as it seemed that my previous
> >>>> explanation wasn't really good.
> >>>>
> >>>> You are suggesting to use 'wakeup-source' exclusively. Unfortunately
> >>>> wakeup-source is a boolean property which covers two states. I have at
> >>>> least three states I need to describe:
> >>>>
> >>>> - wakeup-source for suspend to memory and other low power modes
> >>>> - wakeup-source for Partial-IO
> >>>> - no wakeup-source
> >>>
> >>> Maybe we need generic property or maybe custom TI would be fine, but in
> >>> any case - whether device is wakeup and what sort of wakeup it is, is a
> >>> property of the device.
> >>
> >> To continue on this, I currently only know of this Partial-IO mode that
> >> would require a special flag like this. So I think a custom TI property
> >> would work. For example a bool property like
> >>
> >> ti,partial-io-wakeup-source;
> >>
> >> in the device nodes for which it is relevant? This would be in addition
> >> to the 'wakeup-source' property.
> >
> > Rather oneOf. I don't think having two properties in a node brings any
> > more information.
> >
> > I would suggest finding one more user of this and making the
> > wakeup-source an enum - either string or integer with defines in a header.
>
> I am going through this thread again to write something in DT BoF but
> this is confusing:
>
> "Partial-IO is a very low power mode"
> "not able to do a wakeup from Partial-IO."
> "wakeup-source for Partial-IO"
>
> Are you waking up from Partial-IO or are you waking up into Partial-IO?
>
> And why the devices which are configured as wakeup-source cannot wake up
> from or for Partial-IO?

Sorry if this is confusing. Let me try again.

Partial-IO is a very low power mode. Only a small IO unit is switched on
to be sensitive on a small set of pins for any IO activity. The rest of
the SoC is powered off, including DDR. Any activity on these pins
switches on the power for the remaining SoC. This leads to a fresh boot,
not a resume of any kind. On am62 the pins that are sensitive and
therefore wakeup-source from this Partial-IO mode, are the pins of a few
CAN and UARTs from the MCU and Wkup section of the SoC.

These CAN and UART wakeup-sources are also wakeup-sources for other low
power suspend to ram modes. But wakeup-sources for suspend to ram modes
are typically not a wakeup-source for Partial-IO as they are not powered
in Partial-IO.

I hope this explains it better.

Best
Markus