Received-SPF: pass (google.com: domain of linux-kernel+bounces-72498-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <779d72f1-832a-4fef-a529-ebe8100b04f9@amd.com>
Date: Tue, 20 Feb 2024 13:24:17 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] ASoC: SOF: amd: fix soundwire dependencies
Content-Language: en-US
To: Arnd Bergmann <arnd@arndb.de>, Arnd Bergmann <arnd@kernel.org>,
 Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
 Liam Girdwood <lgirdwood@gmail.com>,
 Peter Ujfalusi <peter.ujfalusi@linux.intel.com>,
 Bard Liao <yung-chuan.liao@linux.intel.com>,
 Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
 Daniel Baluta <daniel.baluta@nxp.com>, Mark Brown <broonie@kernel.org>
Cc: Kai Vehmanen <kai.vehmanen@linux.intel.com>,
 Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
 Venkata Prasad Potturu <venkataprasad.potturu@amd.com>,
 sound-open-firmware@alsa-project.org, linux-sound@vger.kernel.org,
 linux-kernel@vger.kernel.org,
 "Dommati, Sunil-kumar" <Sunil-kumar.Dommati@amd.com>
References: <20240219093900.644574-1-arnd@kernel.org>
 <465d1076-163c-4933-a9b5-e4e8736f5748@amd.com>
 <412c0a42-4c19-48dd-aa4e-de39b6ec3099@app.fastmail.com>
 <343f7b1d-1b4e-402e-bd54-4473b91b6f79@amd.com>
 <65f13ad3-989a-4699-9695-d7b1a95297eb@app.fastmail.com>
From: "Mukunda,Vijendar" <vijendar.mukunda@amd.com>
In-Reply-To: <65f13ad3-989a-4699-9695-d7b1a95297eb@app.fastmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?RU13SnBnVTdXNWo2Um0yV3piU0YyRWkvdnExcHY0NjhmUFF2M3d4K1kvSnJj?=
 =?utf-8?B?K0Q5eHZUa1dNakx1QWdjeEVZNTVUMnNRbFEzQXg4NS9NNHVTM3pnbkRBcnky?=
 =?utf-8?B?U2piZmVqQ2JzUmRtRFZkVUY2cFNxNmF6VFhQc0MrZFYyN1dGdEZicG1yVzFz?=
 =?utf-8?B?Qmk4dWY5QnBzb05JQjBVeU9JZHJZRldWQUxwQ0wrL2xUbzYxY0plTVcrRUpG?=
 =?utf-8?B?UkJRT2t1aTA4WkxiSzJKYi9OVnFEdzVYR3dvU1g3NEdIY0orNVZWeitqSXA0?=
 =?utf-8?B?Y0I1SW5hK1ZSTW1Gd3dkV3liNGEyeXFtcnJHL0h0MTRyT0ptQ2FSTUg0OUc5?=
 =?utf-8?B?VG5GVllpOHVOb1lLNFIvSHlvQmtSazc3c3MxeWd5Q09pNTUyTU9qWlFRd3JJ?=
 =?utf-8?B?c1dmVjN2YUhMbVlncWEvZW1wK1hLQ1NKWVA4eFdRMzc2RnVwU0ZTUzM4ZXQw?=
 =?utf-8?B?aEkvY21jbHFTM2FwZUFmaUw2V2NyWW42MzFITjBzYTJNU2Q0ckYwbHEvc1BC?=
 =?utf-8?B?KzdLVktxbFdsaFNqamwzS2c0WGh5c3BxR2gwV2lYTTBlQlVBZkNxVEY2UWRF?=
 =?utf-8?B?NklubnMxSG9PakxjTjV2NWxUUHBnWitxRlFIWnMrUzVEczBiMHd4ZVZSWWp3?=
 =?utf-8?B?ZzJoVks2eVJCV0RoWTJYeUkxeCtucGRQUE1HYllaV2JyTmdVOEVuRno0d3Z5?=
 =?utf-8?B?aTBuaWZmdTNKWlZ3eG92OTltY3NoZGphalBXZG91aWo4b2hFTEdqaU1CWmx3?=
 =?utf-8?B?M3Z4Nk9uSThzUStUbCtQWnFxMjl1NzJsQ0lQMjJZSnVpdExkaTRtRXUzSm5L?=
 =?utf-8?B?WHpuZG9sWGRxUDZ5YTFkUzB4dldoZEFzTDVPYmtWN2N3ZkwwUGpXR3lxYnpU?=
 =?utf-8?B?VE1XZlRUZEJRY1VBQWZsRWcrZzdqdmZKQmQ3SEhyQkpCUmhlY0pmdkl6aEFs?=
 =?utf-8?B?T0ZQTHdubkp4Y1IyWGlMWnpRY3k2dVpIVEdYN3YrTjJETEx4a0lpcVRnTXNT?=
 =?utf-8?B?VEFRUTRiQkdXd001QkVZYytvTzEzT0d2Q3lMOVVIdFdGMkJObjF2RDduTWNx?=
 =?utf-8?B?enE0RDRrU09iSHpzc2tzQnZyRUlUWDZZQjJ6endZOFVoaW9vdmJOWElOL3Jj?=
 =?utf-8?B?QkdLTnZkcG8yMHFUK09DN3d1ZGVRRXFkSmxhd29TVWxWazFBWGJKZE0wQ3By?=
 =?utf-8?B?S20xcTZRWGNTNnU2N0NEWVZxYTRWSWlwQUFHdjJBYkVoOW1SK0hidml3MWVL?=
 =?utf-8?B?czBHRkQybGhhUGxuYlh6cHlsa3M3cG5QTlR5YWZ6TDhPYTkrVnZ2bFNxeGo4?=
 =?utf-8?B?cm0xRDU4bHNoRHFNUEd3VFFYdVpEUDBQOE5yRzN4QXh3NTZUOGFjVE9hUmdV?=
 =?utf-8?B?a0gxb3pXbE5heVpUdkRmYm5MTWsycVVUZ3F3b0NCM0krZnJPNUVkb25Vcks0?=
 =?utf-8?B?bEVxWm0rRSsydWQxRFVrRkg0ZDRaTWduZDU0TWdRQmxkY0xCVWtTNWJnSVky?=
 =?utf-8?B?MWtYZnhPWEFWT3FiQnZ0bk8wTm9GeTU2M2Z0VWNnWERPdlBuRDJqT1djRmRs?=
 =?utf-8?B?MG5OcTlZdnJxbzc5c3FiN0owakNHeHNaQWZYTVpVaUJ1WGJtSVlZbDM4UUht?=
 =?utf-8?B?WE5LTEIwUGc4TmMrMWFQSXFZekk3WDVMdjUrdjM1QWtORW1NYnJOaU5sVGRX?=
 =?utf-8?B?REphQmR1dTZtYk9mRVkwWGJaMnoyMXI4NFNTOUlXU1UrMjFKek1FdDlPRGUr?=
 =?utf-8?B?RWtRQmVqak4yZXkySW1WekZGQjdXNUhJN1VmK0FmZ2MxN2VrMTdpdFJ6VTIy?=
 =?utf-8?B?WHFhUkswUllMN28vYmpEQWljZDRTcXJiSFZsSEllSEcwRUw2d29SUGtpcDRU?=
 =?utf-8?B?UVhxcVluS3BPbEJ4YnR5WEZCV1RTdFYzeXRPMk9TNkNnbXVVNWpWVjlITzRZ?=
 =?utf-8?B?dGltVkFpTExHUEdJay9JbFY0L1plenFPd0EvdkVEc3pDeGpLTVVBa1lrVEl1?=
 =?utf-8?B?UlNZcmFTUFBVSnJ0ZHhkNWdxTTlvUFNKZzlCR3lYaGlSdmxiOWtLNnQvM0Zr?=
 =?utf-8?B?VTAvc3ZzbUVZa05XbjZYb215ZHdkMmd2QnVhVWFCRll5SmtGU2UvUWdtejZp?=
 =?utf-8?Q?jRo7QQn3lgR8nL1J9c+qRLAB8?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d135c386-d24a-46d1-dd28-08dc31e929c2
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB4123.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Feb 2024 07:54:28.0988
 (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: n+izv+0LTEgipajYm3bco7nUIeS2nqHPUoRxqAlL8rNtGWv6B8eo+YpCL3CL9azYB9tHmUPNKKiOpuPgs4Sqtg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV8PR12MB9420

On 20/02/24 12:40, Arnd Bergmann wrote:
> On Tue, Feb 20, 2024, at 07:23, Mukunda,Vijendar wrote:
>> On 20/02/24 11:43, Arnd Bergmann wrote:
>>> On Tue, Feb 20, 2024, at 06:57, Mukunda,Vijendar wrote:
>>>> On 19/02/24 15:08, Arnd Bergmann wrote:
>>>>> From: Arnd Bergmann <arnd@arndb.de>
>>>>> In normal configs, they should all either be built-in or all loadable
>>>>> modules anyway, so this simplification does not limit any real usecases.
>>>> Tested this patch. SOUNWIRE_AMD flag is not selected by default causing
>>>> AMD SOF driver for ACP 6.3 platform is build without enabling SoundWire.
>>> Yes, that is what I described. But as SOUNWIRE_AMD is a user visible
>>> symbol, there is no problem in expecting users to enable it when they
>>> have this hardware, and distros just enable all the drivers anyway.
>> Want to set SOUNDWIRE_AMD flag by default, similar to Intel & Qcom
>> platforms instead of explicitly enabling the Kconfig option.
> Maybe use 'default SND_SOC_SOF_AMD_TOPLEVEL' then?
Didn't get your point.

Even with the current patch, if we select 'SOUNDWIRE_AMD' flag explicitly
AMD ACP63 SOF driver Kconfig option is not visible for user configuration.
This results in ACP63 SOF driver won't be built at all.


>
> I don't think copying the mistake from the intel driver
> is helpful, though I agree it would be nice to be consistent
> between them.
>
> As a general rule, you should not have a Kconfig symbol that
> is both user visible and also selected by the drivers that
> depend on it.
>
> To avoid the dependency problems from coming back and keep
> the complexity to a minimum, I think there are two logical
> ways to handle soundwire:
>
> a) keep the current drivers/soundwire/Kconfig contents and
>    change all the 'select SOUNDWIRE_foo' to 'depends on'.
Current patch already using 'depends on SOUNDWIRE_AMD" for
SND_SOC_SOF_AMD_SOUNDWIRE Kconfig option.
Still we couldn't see SND_SOC_SOF_AMD_ACP63 Kconfig option
is enabled.
>
> b) keep all the 'select SOUNDWIRE_foo' but drop the prompts,
>    requiring that all drivers that use soundwire have the
>    correct select statements, with the main CONFIG_SOUNDWIRE
>    getting selected by the individual drivers.
Didn't get your point. Could you please elaborate?

>     Arnd