Re: [PATCH v3] ASoC: Intel: soc-acpi-intel-tgl-match: add cs42l43 and cs35l56 support

[Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>]

On 04/12/2023 14:55, Pierre-Louis Bossart wrote:
> On 12/4/23 07:56, Richard Fitzgerald wrote:
> > From: Bard Liao <yung-chuan.liao@xxxxxxxxxxxxxxx>
> >
> > This is a test configuration for UpExtreme with Cirrus Logic
> > CS35L56-EIGHT-C board.
> >
> > The codec layout is configured as:
> >      - Link3: CS42L43 Jack
> >      - Link0: 2x CS35L56 Speaker (amps 1 and 2)
> >      - Link1: 2x CS35L56 Speaker (amps 7 and 8)
> >
> > Signed-off-by: Bard Liao <yung-chuan.liao@xxxxxxxxxxxxxxx>
> > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxxxxxxxxxxx>
> > Signed-off-by: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>
> > ---
> > Changes since V1:
> > - Changed ALSA prefixes for CS35L56 to 'AMPn'
> > - Renumbered the CS35L56 prefixes to match the numbering of the
> >    OUTn speaker sockets on the EIGHT-C board
> > ---
> >   .../intel/common/soc-acpi-intel-tgl-match.c   | 78 +++++++++++++++++++
> >   1 file changed, 78 insertions(+)
> >
> > diff --git a/sound/soc/intel/common/soc-acpi-intel-tgl-match.c b/sound/soc/intel/common/soc-acpi-intel-tgl-match.c
> > index 5804926c8b56..e5f721ba5ed4 100644
> > --- a/sound/soc/intel/common/soc-acpi-intel-tgl-match.c
> > +++ b/sound/soc/intel/common/soc-acpi-intel-tgl-match.c
>
> > +static const struct snd_soc_acpi_adr_device cs35l56_0_adr[] = {
> > +	{
> > +		.adr = 0x00003301FA355601ull,
> > +		.num_endpoints = 1,
> > +		.endpoints = &spk_r_endpoint,
> > +		.name_prefix = "AMP1"
> > +	},
> > +	{
> > +		.adr = 0x00003201FA355601ull,
> > +		.num_endpoints = 1,
> > +		.endpoints = &spk_3_endpoint,
> > +		.name_prefix = "AMP2"
> > +	}
> > +};
> > +
> > +static const struct snd_soc_acpi_adr_device cs35l56_1_adr[] = {
> > +	{
> > +		.adr = 0x00013701FA355601ull,
> > +		.num_endpoints = 1,
> > +		.endpoints = &spk_l_endpoint,
> > +		.name_prefix = "AMP8"
> > +	},
> > +	{
> > +		.adr = 0x00013601FA355601ull,
> > +		.num_endpoints = 1,
> > +		.endpoints = &spk_2_endpoint,
> > +		.name_prefix = "AMP7"
> > +	}
> > +};
>
> Don't we need the same change of name_prefix for
> soc-acpi-intel-mtl-match.c? I see this in the existing code:

The chain that Peter sent only has CS35L56 added to tgl-match.

I've checked the history of mtl-match and the patch that added CS35L56
there was not cc'd to any cirrus.com email address so we didn't notice
it. Otherwise we would have picked this up earlier. alsa-devel is noisy.

I haven't got a MTL platform to test on but I can send a separate patch
to change mtl-match if you are happy with this prefix change.

FWIW I was going to switch to using "cs35l56-n" prefix but while editing
all our unit test cases to the new prefix I realized that's a problem if
we make similar devices with the same controls. We often make a family
of devices that have the same (or very similar) software interface - for
example CS35L41/L51 and the CS47L15/35/85/90/92/93/48L92 family. If we
use the chip ID as the prefix it would create a bogus change in the ALSA
control names even though they are the same controls. OTOH if we used
"cs35l56-n" for all cs35l56-like devices (to avoid the name change) it's
strange that there's a control named for a chip you don't have...

If it quacks like a duck it would be nice if it looks like a duck :)