Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v2 1/3] ASoC: soc-acpi: add comp_ids field for machine
 driver matching
To:     Brent Lu <brent.lu@intel.com>, <alsa-devel@alsa-project.org>
CC:     Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Jaroslav Kysela <perex@perex.cz>,
        Takashi Iwai <tiwai@suse.com>,
        "Pierre-Louis Bossart" <pierre-louis.bossart@linux.intel.com>,
        Jie Yang <yang.jie@linux.intel.com>,
        Kai Vehmanen <kai.vehmanen@linux.intel.com>,
        Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>,
        Yong Zhi <yong.zhi@intel.com>,
        Vamshi Krishna Gopal <vamshi.krishna.gopal@intel.com>,
        <linux-kernel@vger.kernel.org>,
        Rander Wang <rander.wang@intel.com>,
        "Bard Liao" <bard.liao@intel.com>,
        Malik_Hsu <malik_hsu@wistron.corp-partner.google.com>,
        Libin Yang <libin.yang@intel.com>,
        Hans de Goede <hdegoede@redhat.com>,
        Charles Keepax <ckeepax@opensource.cirrus.com>,
        Paul Olaru <paul.olaru@oss.nxp.com>,
        "Curtis Malainey" <cujomalainey@chromium.org>,
        Mac Chiang <mac.chiang@intel.com>,
        Gongjun Song <gongjun.song@intel.com>
References: <20211007133516.1464655-1-brent.lu@intel.com>
 <20211007133516.1464655-2-brent.lu@intel.com>
From:   Cezary Rojewski <cezary.rojewski@intel.com>
Message-ID: <7e6cf92f-b889-aed9-b13a-615d4d5f9b93@intel.com>
Date:   Thu, 7 Oct 2021 19:05:39 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Firefox/78.0 Thunderbird/78.13.0
In-Reply-To: <20211007133516.1464655-2-brent.lu@intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bEpQK05GVE1Fa3Qva2pLVG9mVDZHK3ROemtva2Ntek9mcFk1cDZUUHV0aVJY?=
 =?utf-8?B?OHpBZVozMnBzQ2RpL2NReVBscWRBYXN2MzV0KzJEbmxFMGdzRHYwRnlhNGdR?=
 =?utf-8?B?SzZyc0FjWkFnSjVtQ2ZhN2RiQk5LekIxWDFkK2ppYmRmNE5sbU1ZNGR5Zzhv?=
 =?utf-8?B?U2MzRUo4QkRrMEMzNEh3YzRnT2N2WGcyK0czVERNbS9KVXFZeG5SWWhUaklQ?=
 =?utf-8?B?TTRWZU5iWEdySC9wZlRMeXVTVTBKaHEwdGZVeVFQVTcvRlhFZjVTK0JpNUl6?=
 =?utf-8?B?Q2I5Z3FITnFrbmZKR21UQWY4VUgzWFYwVk9Vb243YkJRU0ZzM0FUYU9hMTkw?=
 =?utf-8?B?a04vc2RFSXY4TjJGS2hsZG8zTEpTcEpvU2tjYStHRVh1LzZ5VWk5SGJPSkgv?=
 =?utf-8?B?KzBIYVRpcDFaTlFYandrV2habWt4N3BBYVlsM2pBY2xnWGdlcHFhSUwrZU0z?=
 =?utf-8?B?Z1lrL0FML1c4ZFIxR0hFVUFzRlUyRzFFUXlKYklYNFFKQ3Fyd2pZL21GMjh0?=
 =?utf-8?B?c1dIYW5HbUlKcE9Da3Q0TzZkS1lqS0V1N1ZmNTRqMGN5VTBSc2RTbi8xaUQw?=
 =?utf-8?B?UXhXRkR4elFrUDNYd0tBSEpVQjNjR0RtODV4WnhteDhmamxjdUNjSkRON0xx?=
 =?utf-8?B?MDRWcTlCVUtFUnlrbTdSMzdhRFpIU3dmbkJQYlFpYzNNNEJtRVo1dHJCcXgr?=
 =?utf-8?B?QkJoMXpwVG9qZTh6a3Z1SGFwcEdPZENuQ25CSEhqVm84MVN1YVZ4TUVXTHIw?=
 =?utf-8?B?bVRGVGhLTGpsOEJhb095Y1ppWEVZSWxNTFpvSnNBTG5PUjljRDRVOEovYXNk?=
 =?utf-8?B?K0s0WXhRZDlRazZGcEJielVLdUtTQ3Rpc1oxTFVYTzdtQ1pyWGhvODZLa0pK?=
 =?utf-8?B?S1AvNFNpYUxSajJWR1JKd1YvMWI4Q09ic09TS0xmMTd5TUNsSThSZkhtU3Z6?=
 =?utf-8?B?ekExT1p5TnBoUVpMUmFsL3JQVjRJdTJ4Smd1K1BucktiVlV4RWY1ZFA4eS8r?=
 =?utf-8?B?eWtDalMzdVlJM0tSWEU2UE4wK01SaDV4RmMvNUp4dVZjS0ZwcUIvT3BoeTk2?=
 =?utf-8?B?bVEyWDdUQmRLdWg0S2tNV0gxNitnT2dhNUhBYVk1NE9XTExOb21UcHl2bDhT?=
 =?utf-8?B?YWI5ZXEvQkVTYnZMR0dJZzQ1NDRlMFAzOUltUWVocTZjZ09RL3hJNkVMeFla?=
 =?utf-8?B?SnJpdG9Pd3VrQkNZZW1uS253b3lQdVUzRVNKWGF0WVUrSXNxQ0dpTmI0a0N3?=
 =?utf-8?B?MkZxazJMdlZFeHRNd2hQUXZFOXN4N2NLUkN3NXBxck94VTFuMzBjU3pFemdI?=
 =?utf-8?B?VnY5OFcvTkNZeTJ0Z1NaaGhXZHF6cXhsdjlqSmhWYmFkSDZiTVUyM3ZJUWN3?=
 =?utf-8?B?eFV6SHdRNzczN1E1ekVwRkJta3ZuMHV6TTluZE9KRE5sWGJjUmJVMC9XUUVP?=
 =?utf-8?B?ODRudDdTOGdMRjYxWW9GWmI3Sml5cjMrMTR1QzNWYlRkdXdHL05TTWNFeGVk?=
 =?utf-8?B?MVhIeThzcmFrRmVZTWtWR2NsSGZZRnZ5eEJSNnR5cWVaT0M1VHgzRXVCcnVl?=
 =?utf-8?B?TE8yRWEzSmVuTTFsTzVEUkUwajRIMzVmMklyVW53SWxTbXZwSWVUWkJic1Vt?=
 =?utf-8?B?OWdHeldHSFVZS3VHYkhSaEZ0SnJXNWd6ajJ3SWdFS2FDZmFSS2ZjVTREZXBX?=
 =?utf-8?B?OXoreGt6dFBOM2dGVjNPZDgxMUcvTlJ3M3FvNVIyek9XcHBPNVhLUHlRZy8z?=
 =?utf-8?Q?o2NtTn3oDf+iEOmpml4K1IJpKjdLAXfO+XUrVeP?=
X-MS-Exchange-CrossTenant-Network-Message-Id: ae614808-1c82-400a-0309-08d989b4b58e
X-MS-Exchange-CrossTenant-AuthSource: BN6PR11MB4049.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Oct 2021 17:05:48.6315
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: jaL2zJFLAce6SMEEam8rKCyQXOs6B4vgcVwBvXCrGkq9rUqbog9HU4IKihbhfN6BsGW/V4TIq9whJEnO9vm3M3khuJdRY/2ZDfGJeOSNEx4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN7PR11MB2625
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2021-10-07 3:35 PM, Brent Lu wrote:

...

>   
> +static bool snd_soc_acpi_id_present(struct snd_soc_acpi_mach *machine)
> +{
> +	struct snd_soc_acpi_codecs *comp_ids = machine->comp_ids;
> +	int i;
> +
> +	if (machine->id[0]) {
> +		if (acpi_dev_present(machine->id, NULL, -1))
> +			return true;
> +	}
> +
> +	if (comp_ids) {
> +		for (i = 0; i < comp_ids->num_codecs; i++) {
> +			if (acpi_dev_present(comp_ids->codecs[i], NULL, -1))
> +				return true;
> +		}
> +	}
> +
> +	return false;
> +}

In cover letter you mention:
"- can use 'comp_ids' field alone to enumerate driver"

which leads me to an opinion that field 'id' should be removed, 
entirely. With 'comp_ids' added, 'id' is basically rendered 
optional/redundant.

> +
>   struct snd_soc_acpi_mach *
>   snd_soc_acpi_find_machine(struct snd_soc_acpi_mach *machines)
>   {
>   	struct snd_soc_acpi_mach *mach;
>   	struct snd_soc_acpi_mach *mach_alt;
>   
> -	for (mach = machines; mach->id[0]; mach++) {
> -		if (acpi_dev_present(mach->id, NULL, -1)) {
> +	for (mach = machines; mach->id[0] || mach->comp_ids; mach++) {

Such loops are hard to maintain i.e. 'comp_ids' acts here like a flex 
array that follows 'id'. Removal of 'id' field and streamlining code to 
only use 'comp_ids' should make this loop more intuitive.

> +		if (snd_soc_acpi_id_present(mach)) {
>   			if (mach->machine_quirk) {
>   				mach_alt = mach->machine_quirk(mach);
>   				if (!mach_alt)
>