On 7/14/20 11:50 PM, Vinod Koul wrote:
On 01-07-20, 10:25, Pierre-Louis Bossart wrote:
+ * wake up master and slave so that slave can notify master
+ * the wakeen event and let codec driver check codec status
+ */
+ list_for_each_entry(slave, &bus->slaves, node) {
+ /*
+ * discard devices that are defined in ACPI tables but
+ * not physically present and devices that cannot
+ * generate wakes
+ */
+ if (slave->dev_num_sticky && slave->prop.wake_capable)
+ pm_request_resume(&slave->dev);
Hmmm, shouldn't slave do this? would it not make sense to notify the
slave thru callback and then slave decides to resume or not..?
In this mode, the bus is clock-stop mode, and events are detected with level
detector tied to PCI events. The master and slave devices are all in
pm_runtime suspended states. The codec cannot make any decisions on its own
since the bus is stopped, it needs to first resume, which assumes that the
master resumes first and the enumeration re-done before it can access any of
its registers.
By looping through the list of devices that can generate events, you end-up
first forcing the master to resume, and then each slave resumes and can
check who generated the event and what happened while suspended. if the
codec didn't generate the event it will go back to suspended mode after the
usual timeout.
We can add a callback but that callback would only be used for Intel
solutions, but internally it would only do a pm_request_resume() since the
codec cannot make any decisions before first resuming. In other words, it
would be an Intel-specific callback that is implemented using generic resume
operations. It's probably better to keep this in Intel-specific code, no?
I do not like the idea that a device would be woken up, that kind of
defeats the whole idea behind the runtime pm. Waking up a device to
check the events is a generic sdw concept, I don't see that as Intel
specific one.
In this case, this in an Intel-specific mode that's beyond what MIPI
defined. This is not the traditional in-band SoundWire wake defined in the
SoundWire specification. The bus is completely down, the master IP is
powered-off and all context lost. There is still the ability for a Slave
device to throw a wake as defined by MIPI in clock-stop-mode1, but this is
handled with a separate level detector and the wake detection is handled by
the PCI subsystem. On a wake, the master IP needs to be powered-up, the
entire bus needs to be restarted and the Slave devices re-enumerated.
Right and I would expect that Slave device would do runtime_get_sync()
first thing in the callback. That would ensure that the Master is
powered up, Slave is powered up, all enumeration is complete. This is
more standard way to deal with this, we expect devices to be so we
low powered or off so cannot do read/write unless we resume.
As shared privately with you, we don't need to deal with device PM or
add a callback, it's enough to resume the master, which will indirectly
restart the bus and result in devices being resumed when they report
their interrupt status. We'll share the update from [1] in the v2.
[1] https://github.com/thesofproject/linux/pull/2247
