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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 15 Feb 2023 03:59:02 +0530
From:   "Mukunda,Vijendar" <vijendar.mukunda@....com>
To:     Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        vkoul@...nel.org
Cc:     amadeuszx.slawinski@...ux.intel.com, Mario.Limonciello@....com,
        Sunil-kumar.Dommati@....com, Basavaraj.Hiregoudar@....com,
        Mastan.Katragadda@....com, Arungopal.kondaveeti@....com,
        Bard Liao <yung-chuan.liao@...ux.intel.com>,
        Sanyog Kale <sanyog.r.kale@...el.com>,
        open list <linux-kernel@...r.kernel.org>,
        "moderated list:SOUNDWIRE SUBSYSTEM" <alsa-devel@...a-project.org>
Subject: Re: [PATCH V2 2/8] soundwire: amd: Add support for AMD Manager driver

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.