On Mon, Mar 25, 2024 at 5:42 PM John Watts <contact@xxxxxxxxxx> wrote: > > Hello there, > > On Mon, Mar 25, 2024 at 03:49:44PM -0700, Saravana Kannan wrote: > > Ok, I think I understand now what's going on. fw_devlink does not know > > that "sound" device will not populate "multi" as a child device. > > Typically in such situations, "sound" would probe as a device and add > > its child DT nodes devices. At that point, the cycle is only between > > "multi" and "test_codec" and fw_devlink will detect that and not > > enforce any ordering. However, in this case, "sound" doesn't have any > > child devices and just depends on the remote endpoints directly. > > > > We already have "ports", "in-ports" and "out-ports". Is there a reason > > none of them will work for your use case and it has to be "multi"? > > When you use one of those 3 recognized node names, things are handled > > correctly. > > audio-graph-card2 uses 'multi' to define DAI links that have multiple > endpoints. It also suports codec2codec and dpcm. > > > I think the right fix is the use of post-init-providers. Because even > > if you do the above, all it does is let fw_devlink see that there's a > > cyclic dependency in DT. And it'll stop enforcing the probe and > > suspend/resume ordering. Ideally we want to enforce a specific order > > here. test_codec first and then sound. > > Is there a way to do this automatically so all the existing audio-graph-card2 > device trees aren't broken? As it stands it seems like this driver is now > broken due to this change. Ok, I have a solution. Have the audio-graph-card2 find the fwnode of "multi" and mark it as "not a device" by doing something like this in the driver. That should help fw_devlink handle this correctly. fwnode.flags |= FWNODE_FLAG_NOT_DEVICE; > > > Maybe. But the logs would be more helpful. > > If you have a way for me to get more logs please tell me. > > > > > post-init-provider = <&multi>; > > > > Did you try this? Did it help? > > > > -Saravana > > No I haven't tried this yet. I shall try it soon. But I wouldn't consider > this a useful fix as it requires upgrading existing device trees. Definitely do this though as a forward looking improvement. It'll help make the suspend/resume more deterministic and will eventually let things happen in an async manner. -Saravana
