On 1/12/23 09:29, Limonciello, Mario wrote: > On 1/12/2023 08:54, Pierre-Louis Bossart wrote: >> >> >> On 1/12/23 05:02, Mukunda,Vijendar wrote: >>> On 11/01/23 21:32, Pierre-Louis Bossart wrote: >>>> On 1/11/23 03:02, Vijendar Mukunda wrote: >>>>> To avoid ACP entering into D3 state during slave enumeration and >>>>> initialization on two soundwire controller instances for multiple >>>>> codecs, >>>>> increase the runtime suspend delay to 3 seconds. >>>> You have a parent PCI device and a set of child devices for each >>>> manager. The parent PCI device cannot suspend before all its children >>>> are also suspended, so shouldn't the delay be modified at the >>>> manager level? >>>> >>>> Not getting what this delay is and how this would deal with a lengthy >>>> enumeration/initialization process. >>> Yes agreed. Until Child devices are suspended, parent device will >>> be in D0 state. We will rephrase the commit message. >>> >>> Machine driver node will be created by ACP PCI driver. >>> We have added delay in machine driver to make sure >>> two manager instances completes codec enumeration and >>> peripheral initialization before registering the sound card. >>> Without adding delay in machine driver will result early card >>> registration before codec initialization is completed. Manager >>> will enter in to bad state due to codec read/write failures. >>> We are intended to keep the ACP in D0 state, till sound card >>> is created and jack controls are initialized. To handle, at manager >>> level increased runtime suspend delay. >> >> This doesn't look too good. You should not assume any timing >> dependencies in the machine driver probe. I made that mistake in earlier >> versions and we had to revisit all this to make sure drivers could be >> bound/unbound at any time. > > Rather than a timing dependency, could you perhaps prohibit runtime PM > and have a codec make a callback to indicate it's fully initialized and > then allow runtime PM again? We already have enumeration and initialization 'struct completion' that are used by codec drivers to know if the hardware is usable. We also have pm_runtime_get_sync() is the bus layer to make sure the codec is resumed before being accessed. The explanations above confuse card registration and manager probe/initialization. These are two different things. Maybe there's indeed a missing part in the SoundWire PM assumptions, but I am not getting what the issue is.
