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

Markus Schneider-Pargmann
2024-08-06T07:12:15Z

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

If something is a wakeup-source for Partial-IO it usually is a
wakeup-source for suspend to memory as well but not the other way
around.

This is the reason why I added a property that lists the devicenodes
that are capable of wakeup from Partial-IO.

Best
Markus