Re: [PATCH v2] ALSA: hda/realtek: cs35l41: Fix internal speaker support for ASUS UM3402 with missing DSD

[Takashi Iwai <tiwai@xxxxxxx>]

On Wed, 07 Feb 2024 18:48:13 +0100,
Jean-Loïc Charroud wrote:
> 
> Le 07/02/2024 à 16:10, Takashi Iwai a écrit :
> > Jean-Loïc Charroud wrote:
> >> Fix device ID for "ASUS UM3402" and "ASUS UM6702RA/RC".
> > This change is only about the string in the table, not the actual
> > behavior, right?
> 
> Well, not sure it doesn't matter.
> Before the device ID were swapped, my syslog reported :
>     cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: Error: ACPI _DSD
>     Properties are missing for HID CSC3551.
> And then, after the patch 51d976079976c800ef19ed1b542602fcf63f0edb, it
> reports :
>     cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: Failed property
>     cirrus,dev-index: -22
> So, as it looks like a regression, it makes me think that the model name
> may be checked along the device ID...

It can't be.  The name string is discarded (not compiled at all)
unless CONFIG_SND_DEBUG is set, so it's really optional.

That is, the swap of the name strings must not influence on the driver
behavior.  Try it out.

So, splitting the patches into logical pieces helps to improve the
understanding of the problems more clearly.  You see the effect.

> > The name string there is only for debug info, so
> > it's no big problem even if it's not 100% right.
> > That is, this can be again split to another patch -- with the
> > additional Fixes tag to the commit that introduced the entries.
> > 
> >> Add DSD values for "ASUS UM3402" to cs35l41_config_table[].
> > ... and this one is the mandatory fix for your device.  It should be
> > the patch 1.  Then we'll have two more, one for correcting the entry
> > names, and another for sorting the entries.
> I already submitted that last one
> > 
> > I'm a bit picky, but now you see how the things work.
> > Divide-and-conquer is the basic strategy.
> Yes, but not smooth, as I have to workaround my broken mailer...

Good that you spotted out, it had to be fixed in anyway :)


Takashi