Message-ID: <8acb6d45-df74-3be2-93e0-d3053c90ab99@amd.com>
Date:   Wed, 15 Feb 2023 03:59:02 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH V2 2/8] soundwire: amd: Add support for AMD Manager driver
Content-Language: en-US
To:     Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
        vkoul@kernel.org
Cc:     amadeuszx.slawinski@linux.intel.com, Mario.Limonciello@amd.com,
        Sunil-kumar.Dommati@amd.com, Basavaraj.Hiregoudar@amd.com,
        Mastan.Katragadda@amd.com, Arungopal.kondaveeti@amd.com,
        Bard Liao <yung-chuan.liao@linux.intel.com>,
        Sanyog Kale <sanyog.r.kale@intel.com>,
        open list <linux-kernel@vger.kernel.org>,
        "moderated list:SOUNDWIRE SUBSYSTEM" <alsa-devel@alsa-project.org>
References: <20230213094031.2231058-1-Vijendar.Mukunda@amd.com>
 <20230213094031.2231058-3-Vijendar.Mukunda@amd.com>
 <a3a75ead-5430-ae32-a6ae-78314bc637f1@linux.intel.com>
 <7b5198f5-1894-5ab5-f84b-410cf102268d@amd.com>
 <e79dae39-faec-b2a4-82e6-e11325904f3e@linux.intel.com>
From:   "Mukunda,Vijendar" <vijendar.mukunda@amd.com>
In-Reply-To: <e79dae39-faec-b2a4-82e6-e11325904f3e@linux.intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WDZDVXJRVzBveUxVSGI0RHJ6ZG9rM2VSMG1HKzMrZ2RsQ1pmRUVFeHNjVzBP?=
 =?utf-8?B?aG9laWQ4Uys3LzVnSXNpQ2VnQmYzNXZ3VXducmRFVlVjNG5nM0Jkelh6Zm5I?=
 =?utf-8?B?SnViRDdiM202ckVRQlVieERwdll0RkRGaDRTeXIwRzR2dEJUVElSeFRIWUtQ?=
 =?utf-8?B?THJ2dUxaWWJWSnlGNFQvSjhvMU1mR2VKbGJvOE53U204TzNGQ2ZhdEhPWlcr?=
 =?utf-8?B?VFFkbkhkVmpqRUtyL29VQkdicnIwYWVPOGhVUmZWR1lXZ2krdnMwLzllK0g0?=
 =?utf-8?B?UkVHV0xHeGxyLzBaK0tDNlQ1cWxkU2gzVGY5UXJoZW8rL2ZwcVF6ckxSSGE4?=
 =?utf-8?B?VnEwWjMwQlloMjVwYzZVdDVGcEtWV2tLdmV2c0RsZ25GWWx5TWdIclRJMVgx?=
 =?utf-8?B?UktqbnQrc2hOa1VkbU5mM3ZaVk1kMnpmYnNpeDlsY1p1T1kyZ2o5YlFDM2Zh?=
 =?utf-8?B?ZG5FVC8rSXpjU25lSHhXTUViSGUwWWo2TmlWN1BMamdrUExpZWlvaGl4K3lX?=
 =?utf-8?B?YUJ5WFhtQldMTWNSM0E2UGxWODVoYzJkYjhUdFdDNlZibFZUQmwzOWd3aktZ?=
 =?utf-8?B?RFEzak4wNjNnb2ZqMXcxbkk2MTJBdnBDc2ErVFpHQjBkVG1CRjZ1VXJkNEpi?=
 =?utf-8?B?SnVRYWs3Vk85cGdpb0ZhY3VwTDU0MlpVRzBiZWNHamp4QjUyQ3FOeVF3MUJH?=
 =?utf-8?B?KzR6dnZ5ZVBCclBNdDNEZUN3N0ZjUE04WnpnRkd5RU1rWVlydHlLRUdVV3Z0?=
 =?utf-8?B?VllSOTJkWGVTQkJvTVM2MHJZcU11aTZmRWlLVjVJK3NzT1VSVHZxTjVrYVhJ?=
 =?utf-8?B?cFJ4emUxMlk0R2V0bCtHV083ZEEwQVFpVC91UktSVGxyOERYRzk0cVQxWWR0?=
 =?utf-8?B?NmVrSkc3RmQrVDgyZUVXeCtjOHZxVEI5akVIbHVNN2JoSlVTVUVyVldFSzJR?=
 =?utf-8?B?eWFRcVdrUjlTSzVSMCtUZUd4V0lMNGZDbWhUZlRUUFlYNzlVQS9DOTYrYmJw?=
 =?utf-8?B?VWRJdzkwbXFGMkQ2Q0NkaS9keVAvNnhFdTIwWU11U0VHTHZSRHdpaGxHWUkv?=
 =?utf-8?B?Vm53VVVMYWJqWFl0OG9XbmJ6M3RZTG9tR3hEMGVxMnVyMUtaUSttQlFqSTlL?=
 =?utf-8?B?RDg5RDBXUXRCTmJGbFpXRnNLbkw5RGRjdkU1TVVucmd3MjBQdi9HVE5iVkR4?=
 =?utf-8?B?M3FsTUVCSExVcnlmLzhlQ245OFNZdFhOb0VwYUg3UG1OK0p2T0xiaGNnZ0tJ?=
 =?utf-8?B?OW5EWVVVRFNoV1RxVWEwMDd0aHlCa09Vd3FmRXpab0liNG9qN3R4MVhzTmd5?=
 =?utf-8?B?RWZtUnVUcVdVUExZcUtENjBHZXpMbjE0Rm9udXZjejFHS1BPaWxURE5heWNa?=
 =?utf-8?B?QUlZWGpIejRoWnlEQldoVVc0TnR2YjRVWEUrOGFaeGluVEg4Zm43cytCcVBW?=
 =?utf-8?B?dVhhWjR0RlEvR0VHcUo4MllyTVZNNUgydjcwdHNzVlpodG9VRHFkT1ZWWlVC?=
 =?utf-8?B?WHQveGR1YkduRHhSWjAyMVJabVpLdERxVVFlb2dWRWRRazY5elhHS201QTRz?=
 =?utf-8?B?cWJ1RldIUGRZaWk1V2tBV0RlUWplQjRNMzFLclhtMUN4eENNanNCek9OaEla?=
 =?utf-8?B?R1RaYm9ZUk0yRlRCWTEzZ1FGdkVPZjZzZFB6bk1oa0RQdXhvN01XQzkyQmp4?=
 =?utf-8?B?ZFRDRmRPVTJVVU9YdjR0U2RxaThhQXk5MHRaazFYeEJNWG9BLzh1eEFjSWxU?=
 =?utf-8?B?ZWQvMVVWQVFHbE9WUUg1T2lvczlWSkUzbWtQU05YSFkvZ0t6bE56SGNnb09R?=
 =?utf-8?B?VmEvdjBuOE96ZGhFQVdtUGQ4TFFtQUJYNmtLTEp6ZXJZOHgybkdqRVFnd2Zu?=
 =?utf-8?B?UVJQRDhhT0ZwQmJFMFFvcG9ZdnE3dnlKZW9Fcm9WalVMZE83TXBqTVdzWkY3?=
 =?utf-8?B?M2d5cmdtSzZVWHJONEZOOU1rTjFEZFlhdmdoeTFKZXBkME9INTFJTEVPVUJn?=
 =?utf-8?B?V005S1JWdTVZanhtemZJTml4S3dLOFl6R2ZTdCtwYURJMTZCdGR4bENzUWdW?=
 =?utf-8?B?SzhZS0MyaHJlYng2Z1hydGhWUUNISDRQZHN3dFFxY3BsWUg1Tmo3RlU5Wndv?=
 =?utf-8?Q?gB/JV4L8XGIvZGjZLU4aicA72?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 49c74ff8-699b-4246-f614-08db0eda78bc
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB4123.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Feb 2023 22:26:09.8273
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: pd/XMlu88tLwwhShxYe8bG8SbUeZTuCJ+P6wnUsdYNQe5sd5hsdteiKrbjPSVbWD8hFp30GI6zDXXSpjHzZzKQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4484
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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.