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

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

[Jaroslav Kysela]

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.

Also, it does not make sense to set the driver field to the
card name composed from DMI. This card name is longer in most
(all?) cases. Use a generic "ASoC-DMI" string here.

Signed-off-by: Jaroslav Kysela <perex@perex.cz>
---
 sound/soc/soc-core.c | 41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index e824ff1a9fc0..fd037208c222 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1840,21 +1840,22 @@ static void soc_check_tplg_fes(struct snd_soc_card *card)
 	}
 }
 
-#define soc_setup_card_name(name, name1, name2, norm)		\
-	__soc_setup_card_name(name, sizeof(name), name1, name2, norm)
-static void __soc_setup_card_name(char *name, int len,
-				  const char *name1, const char *name2,
-				  int normalization)
+#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", name1 ? name1 : name2);
+	snprintf(name, len, "%s", src);
 
-	if (!normalization)
+	if (name != card->snd_card->driver)
 		return;
 
 	/*
-	 * Name normalization
+	 * Name normalization (driver field)
 	 *
 	 * The driver name is somewhat special, as it's used as a key for
 	 * searches in the user-space.
@@ -1874,6 +1875,14 @@ static void __soc_setup_card_name(char *name, int len,
 			break;
 		}
 	}
+
+	/*
+	 * The driver field should contain a valid string from the user view.
+	 * The wrapping usually does not work so well here. Set a smaller string
+	 * in the specific ASoC driver.
+	 */
+	if (strlen(src) > len - 1)
+		dev_err(card->dev, "ASoC: driver name too long '%s' -> '%s'\n", src, name);
 }
 
 static void soc_cleanup_card_resources(struct snd_soc_card *card)
@@ -1928,6 +1937,7 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
 	struct snd_soc_pcm_runtime *rtd;
 	struct snd_soc_component *component;
 	struct snd_soc_dai_link *dai_link;
+	const char *fallback;
 	int ret, i;
 
 	mutex_lock(&client_mutex);
@@ -2041,12 +2051,15 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
 	/* try to set some sane longname if DMI is available */
 	snd_soc_set_dmi_name(card, NULL);
 
-	soc_setup_card_name(card->snd_card->shortname,
-			    card->name, NULL, 0);
-	soc_setup_card_name(card->snd_card->longname,
-			    card->long_name, card->name, 0);
-	soc_setup_card_name(card->snd_card->driver,
-			    card->driver_name, card->name, 1);
+	soc_setup_card_name(card, card->snd_card->shortname,
+			    card->name, NULL);
+	fallback = card->name;
+	soc_setup_card_name(card, card->snd_card->longname,
+			    card->long_name, fallback);
+	if (card->long_name == card->dmi_longname)
+		fallback = "ASoC-DMI";
+	soc_setup_card_name(card, card->snd_card->driver,
+			    card->driver_name, fallback);
 
 	if (card->components) {
 		/* the current implementation of snd_component_add() accepts */
-- 
2.35.3

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1088 bytes --]

On Thu, Sep 29, 2022 at 10:06:54AM +0200, Jaroslav Kysela wrote:
> 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.

This breaks at least an arm multi_v7_defconfig build:

/build/stage/linux/sound/soc/soc-core.c: In function ‘snd_soc_bind_card’:
/build/stage/linux/sound/soc/soc-core.c:2055:36: error: ‘struct snd_soc_card’ ha
s no member named ‘dmi_longname’
 2055 |         if (card->long_name == card->dmi_longname)
      |                                    ^~


> Also, it does not make sense to set the driver field to the
> card name composed from DMI. This card name is longer in most
> (all?) cases. Use a generic "ASoC-DMI" string here.

This should be a separate change, and DMI is a term specific to the
ACPI/EFI so I don't think we should be using it as a generic here, this
seems like a step back.  If we want to make a change there I'd expect it
to be more picking the actual card driver name.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Jaroslav Kysela]

On 29. 09. 22 15:38, Mark Brown wrote:
> On Thu, Sep 29, 2022 at 10:06:54AM +0200, Jaroslav Kysela wrote:
>> 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.
> 
> This breaks at least an arm multi_v7_defconfig build:
> 
> /build/stage/linux/sound/soc/soc-core.c: In function ‘snd_soc_bind_card’:
> /build/stage/linux/sound/soc/soc-core.c:2055:36: error: ‘struct snd_soc_card’ ha
> s no member named ‘dmi_longname’
>   2055 |         if (card->long_name == card->dmi_longname)
>        |                                    ^~
> 
> 
>> Also, it does not make sense to set the driver field to the
>> card name composed from DMI. This card name is longer in most
>> (all?) cases. Use a generic "ASoC-DMI" string here.
> 
> This should be a separate change, and DMI is a term specific to the
> ACPI/EFI so I don't think we should be using it as a generic here, this
> seems like a step back.  If we want to make a change there I'd expect it
> to be more picking the actual card driver name.

Thanks for the review. Yes, I made a mistake here. I wrongly mixed name and 
long name strings in my head. I removed the DMI check and posted v2 of the patch.

			Jaroslav


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

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

[Mark Brown]

On Thu, 29 Sep 2022 10:06:54 +0200, Jaroslav Kysela wrote:
> 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).
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: core: clarify the driver name initialization
      commit: c8d18e44022518ab026338ae86bf14cdf2e71887

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark