Re: [PATCH v2 1/3] ASoC: soc-acpi: add comp_ids field for machine driver matching

From: Cezary Rojewski
Date: Thu Oct 07 2021 - 13:08:32 EST


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)