>>> You could say why do we need wait itself in the first place. >>> >>> The reason we need wait in first place is because, there is a danger of >>> codec accessing registers even before enumeration is finished. Because >>> most of the ASoC codec registration happens as part of codec/component >>> driver probe function rather than status callback. >>> >>> I hope this answers your questions. >> >> >> Humm, not really. >> >> First, you're using this TIMEOUT_MS value in 3 unrelated places, and >> using 2 different struct completion, which means there are side effects >> beyond autoenumeration. > > 2 of these 3 are autoenum ones, one is in probe path and other in bus > reset path during pm. > > The final one is broadcast timeout, new timeout value should be okay for > that too, given that we need 10ms timeout. probably better to make things explicit with a different timeout value for the broadcast case. 100ms for a broadcast is really eons, it's supposed to be immediate really. >> And then I don't get what you do on a timeout. in the enumeration part, > > We can't do much on the timeout. > >> the timeout value is not checked for, so I guess enumeration proceeds >> without the hardware being available? That seems to contradict the >> assertion that you don't want to access registers before the hardware is >> properly initialized. > > Even if enumeration timeout, we will not access the registers because > the ASoC codec is not registered yet from WCD938x component master. What happens when the codec is registered then? the autoenumeration still didn't complete, so what prevents the read/writes from failing then? >> And then on broadcast you have this error handling: >> >> ret = wait_for_completion_timeout(&swrm->broadcast, >> msecs_to_jiffies(TIMEOUT_MS)); >> if (!ret) >> ret = SDW_CMD_IGNORED; >> else >> ret = SDW_CMD_OK; >> >> Which is equally confusing since SDW_CMD_IGNORED is really an error, and >> the bus layer does not really handle it well - not to mention that I >> vaguely recall the qcom hardware having its own definition of IGNORED? > QCom hardware alteast the version based on which this driver was written > did not have any support to report errors type back on register > read/writes. > > In this case a broad cast read or write did not generate a complete > interrupt its assumed that its ignored, as there is no way to say if its > error or ignored. ok, that should be fine. The code in bus.c mostly ignores -ENODATA for the suspend cases. For the bank switch, I forgot that we also ignore it, Bard added a patch in 2021. The only case where we abort a transfer is on a real error.
