Re: [PATCH v2] ASoC: core: clarify the driver name initialization

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 20. 10. 22 18:27, Pierre-Louis Bossart wrote:
Hi Jaroslav,

The driver field in the struct snd_ctl_card_info is a valid
user space identifier. Actually, many ASoC drivers do not care
and let to initialize this field using a standard wrapping method.
Unfortunately, in this way, this field becomes unusable and
unreadable for the drivers with longer card names. Also,
there is a possibility to have clashes (driver field has
only limit of 15 characters).

This change will print an error when the wrapping is used.
The developers of the affected drivers should fix the problem.

How should we fix this problem?

I see all kinds of errors thrown in our first CI results based on
6.1-rc1:

[   12.684893] kernel: cml_rt1011_rt5682 cml_rt1011_rt5682: ASoC: driver
name too long 'sof-cml_rt1011_rt5682' -> 'sof-cml_rt1011_'

[   12.219660] kernel: bxt_da7219_max98357a glk_da7219_mx98357a: ASoC:
driver name too long 'sof-glkda7219max' -> 'sof-glkda7219ma'

I have no idea what is expected here in terms of naming. It's far from
obvious to respect the 15-character limit AND have something
readable/sensible given the proliferation of hardware skews.

Any suggestions?

The question is, how deep the driver name should describe the hardware
details. The two drivers covering the majority hardware use "HDA-Intel"
and "USB-Audio" strings here and there are a lot of variants of the
codec / user space devices / mixer controls. The codec chain and
eventually the audio bridge can be described in other identification
strings like card components or the card name (note that most end users
are not able to identify the references to hardware - it's a GUI string).

I would use "SOF-Intel-CML" and "SOF-Intel-GLK" strings or just
"SOF-Intel" or any other variant as you like (lower case etc.). My
opinion is that it's not necessary to have an unique string per driver
here (the drivers should have just something common like the SOF core
code).

ok, adding 'SOF-Intel' would be fine, but remind me again how to set
the driver name?

We already set the card name to e.g.
	broxton_audio_card.name = "glkda7219max";
then the sof-prefix gets added, and that's what we use for UCM [1]

how would I modify the driver name?

If I just go ahead and set .driver_name = "SOF-Intel" in the card
declaration, isn't this going to impact the calls to

	soc_setup_card_name(card, card->snd_card->shortname,
			    card->name, NULL);
	soc_setup_card_name(card, card->snd_card->longname,
			    card->long_name, card->name);
	soc_setup_card_name(card, card->snd_card->driver,
			    card->driver_name, card->name);


#define soc_setup_card_name(card, name, name1, name2) \
	__soc_setup_card_name(card, name, sizeof(name), name1, name2)
static void __soc_setup_card_name(struct snd_soc_card *card,
				  char *name, int len,
				  const char *name1, const char *name2)
{
	const char *src = name1 ? name1 : name2;
	int i;

	snprintf(name, len, "%s", src);

	if (name != card->snd_card->driver)
		return;

so the shortname and longname would never be used?

Nope. It's just a short path for the non-driver field to not further process the destination string (the name argument). The snprintf() call sets all field types (it's before the condition). Just set the driver_name field in the soc card structure and you will be fine.

The UCM config must be updated to handle the new driver name. The fine selection key should probably use the card name, because long name is set from DMI:

old:

1 [sofglkda7219max]: sof-glkda7219ma - sof-glkda7219max
                     Google-Phaser360-rev4

new:

1 [sofglkda7219max]: SOF-Intel - sof-glkda7219max
                     Google-Phaser360-rev4

UCM substitutions:

1 [${CardId}      ]: ${CardDriver} - ${CardName}
                     ${CardLongName}

UCM conf:

mkdir -p ucm2/conf.d/SOF-Intel
cat > ucm2/conf.d/SOF-Intel/SOF-Intel.conf <<EOF
Syntax 6
Include.0.File "/Intel/\${CardName}/\${CardName}.conf"
EOF

						Jaroslav

--
Jaroslav Kysela <perex@xxxxxxxx>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux