Re: [PATCH V2 2/8] soundwire: amd: Add support for AMD Manager driver

Pierre-Louis Bossart
2023-02-14T19:24:22Z

>>> +static void amd_sdw_probe_work(struct work_struct *work)
>>> +{
>>> + struct amd_sdw_manager *amd_manager = container_of(work, struct amd_sdw_manager,
>>> + probe_work);
>>> + struct sdw_master_prop *prop;
>>> + int ret;
>>> +
>>> + prop = &amd_manager->bus.prop;
>>> + if (!prop->hw_disabled) {
>>> + amd_enable_sdw_pads(amd_manager);
>>> + ret = amd_init_sdw_manager(amd_manager);
>>> + if (ret)
>>> + return;
>>> + amd_enable_sdw_interrupts(amd_manager);
>>> + ret = amd_enable_sdw_manager(amd_manager);
>>> + if (ret)
>>> + return;
>>> + amd_sdw_set_frameshape(amd_manager);
>>> + }
>>> +}
>> There should be an explanation as to why you need a workqueue to
>> complete the probe.
> We want to separate the manager probe sequence and start up sequence.
> we will add the comment.

Do you need to split in two? For Intel, on some platforms we had a clear
power dependency, we had to wait until parts of the DSP were powered
before accessing SHIM registers, so we called the startup() when those
dependencies were resolved.

I am not sure you can count on the probe_work to enforce any kind of
delay, worst case the work function could be scheduled immediately.