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. > > 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. > > 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