On Tue, Aug 06, 2024 at 10:03:00AM GMT, Krzysztof Kozlowski wrote:

On 06/08/2024 09:11, Markus Schneider-Pargmann wrote:

quoted

Hi Krzysztof,

On Tue, Aug 06, 2024 at 08:18:01AM GMT, Krzysztof Kozlowski wrote:

quoted

On 29/07/2024 10:00, Markus Schneider-Pargmann wrote:

quoted

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.

<form letter>
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.
</form letter>

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.

Best
Markus

quoted

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.

I understand, makes sense. The trouble is that your driver code does not
indicate any of this.

quoted

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

This property looks purely to satisfy your driver design.

Best regards,
Krzysztof