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. >>>>>> >>>>> >>>>> <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? Best regards, Krzysztof
