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

[Mukunda,Vijendar]

On 14/02/23 18:51, Pierre-Louis Bossart wrote:
>>>> +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.
As of today, for legacy driver(no DSP's are enabled) instead of
having lengthy execution of probe() call, we want to split in two.