On 05/09/2024 11:49, Markus Schneider-Pargmann wrote:

On Thu, Sep 05, 2024 at 11:25:48AM GMT, Krzysztof Kozlowski wrote:

quoted

On 05/09/2024 11:15, Krzysztof Kozlowski wrote:

quoted

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

quoted

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

quoted

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.

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.

Yeah, it's kind of obvious now that just use wakeup-source. Your
hardware does not have two different methods of waking up. System is
sleeping - either S2R or partial-IO or whatever - and you want it to be
woken up.

Entire property is unnecessary... and as I said before - you added it
only for your driver. If same feedback is repeated and repeated, there
is something in it...

Best regards,
Krzysztof