On 05/09/2024 11:49, Markus Schneider-Pargmann wrote: > 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. >>>>>>>> >>>>>>> >>>>>>> <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
