On Thu, Sep 05, 2024 at 12:41:05PM GMT, Krzysztof Kozlowski wrote: > 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... It's not obvious for me. Maybe you can elaborate a bit. The hardware has a different set of wakeup sources depending on the power mode it is in and I would like to describe these different sets of wakeup sources in the devicetree as for me this is a hardware property, not a driver thing. Best Markus
