On Mon, Jun 29, 2020 at 06:26:51PM +0100, Mark Brown wrote: > On Mon, Jun 29, 2020 at 05:42:07PM +0100, Sudeep Holla wrote: > > On Mon, Jun 29, 2020 at 05:14:50PM +0100, Mark Brown wrote: > > > On Mon, Jun 29, 2020 at 04:07:28PM +0100, Sudeep Holla wrote: > > > > > > > The specification states clearly: > > > > "... all devices in the system must be in a state that is compatible > > > > with entry into the system state. These preconditions are beyond the scope > > > > of this specification and are therefore not described here." > > > > "Prior to the call, the OS must disable all sources of wakeup other than > > > > those it needs to support for its implementation of suspend to RAM." > > > > This gets a bit circular for a generic OS since the OS needs some way to > > > figure out what it's supposed to do on a given platform - for example > > > the OS may be happy to use wakeup sources that the firmware is just > > > going to cut power on. > > > While I understand the sentiments here, PSCI is targeted to address CPU > > power state management mainly and system states like suspend/reset and > > poweroff which involves last CPU. This is one of the reason it is out of > > the scope of the specification. > > Sure, but as soon as we start talking about the last CPU stuff we're > inevitably talking about the system as a whole. > > > Here is my understanding. DT specifies all the wakeup sources. Linux > > can configure those and user may choose to enable a subset of them is > > wakeup. As stated in the spec and also based on what we already do in > > the kernel, we disable all other wakeup sources. > > > The PSCI firmware can then either read those from the interrupt controller > > or based on static platform understanding, must not disable those wakeup > > sources. > > That bit about static platform understanding isn't super helpful for the > OS, so long as the firmware might do that the OS is pretty much out of > luck. > I don't disagree. That's one of the reason I wanted to gather requirement few years back when PSCI system suspend was introduced. I couldn't convince PSCI spec authors myself. > > > > I see nothing has been fixed in the firmware too and we are still > > > > discussing the same after 3 years 😄. Clearly we should start trusting > > > > firmware and built capability to fix and replace it if there are bugs > > > > just like kernel and stop hacking around in the kernel to deal with > > > > just broken platform/psci firmware. > > > > This isn't just an issue of buggy firmware as far as I can see, it's > > > also a lack of ability for the OS and firmware to communicate > > > information about their intentions to each other. As things stand you'd > > > need to put static information in the DT. > > > It is easy for DT to get out of sync with firmware unless it is generated > > by the same firmware. That's the reason why I am against such multiple > > The ability for things to get out of sync also concerns me as I said > further back in the thread but I'm not sure we have much alternative, > realistically we're going to need some facility to work around firmware > that isn't ideal. > I understand and I agree. That's the main reason I want to understand this better and provide a generic solution. The current pm_suspend_via_firmware seem to have different intentions(at-least that's what I grasped quickly reading through the document) > > sources of information. I understand ACPI has more flexibility and I did > > ACPI has a much stronger idea of what the system looks like which helps > it a lot here. > Yes, I wanted something similar initially but didn't get good response both from community and PSCI spec authors. > > Each device or platform having its specific property in DT is not scalable. > > Not sure if it is a generic problem. If it is, I would like to understand > > more details on such lack of ability for communtication between OS and > > firmware. > > It seems like a generic issue from where I'm sitting. I can't disagree with that. The description of the issue and the solution in the thread is. We need make it more generic and provide generic solution,. I already see 2-3 threads addressing this in isolation as specific issue. Also we need to check with DT maintainers if there are fine with the idea of binding to address this issue. -- Regards, Sudeep