[PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Jon Hunter]

Many systems do not use ACPI and hence do not provide a DMI table. On
non-ACPI systems a warning, such as the following, is printed on boot.

 WARNING KERN tegra-audio-graph-card sound: ASoC: no DMI vendor name!

The variable 'dmi_available' is not exported and so currently cannot be
used by kernel modules without adding an accessor. However, it is
possible to use the function is_acpi_device_node() to determine if the
sound card is an ACPI device and hence indicate if we expect a DMI table
to be present. Therefore, call is_acpi_device_node() to see if we are
using ACPI and only parse the DMI table if we are booting with ACPI.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
Changes since V1:
- Use is_acpi_device_node() to determine if we expect the DMI table to
  be present.

 sound/soc/soc-core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index f6d4e99b590c..0cffc9527e28 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -31,6 +31,7 @@
 #include <linux/of.h>
 #include <linux/of_graph.h>
 #include <linux/dmi.h>
+#include <linux/acpi.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -1573,6 +1574,9 @@ int snd_soc_set_dmi_name(struct snd_soc_card *card, const char *flavour)
 	if (card->long_name)
 		return 0; /* long name already set by driver or from DMI */
 
+	if (!is_acpi_device_node(card->dev->fwnode))
+		return 0;
+
 	/* make up dmi long name as: vendor-product-version-board */
 	vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
 	if (!vendor || !is_dmi_valid(vendor)) {
-- 
2.25.1

Re: [PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Mark Brown]

On Wed, 3 Mar 2021 11:55:26 +0000, Jon Hunter wrote:
> Many systems do not use ACPI and hence do not provide a DMI table. On
> non-ACPI systems a warning, such as the following, is printed on boot.
> 
>  WARNING KERN tegra-audio-graph-card sound: ASoC: no DMI vendor name!
> 
> The variable 'dmi_available' is not exported and so currently cannot be
> used by kernel modules without adding an accessor. However, it is
> possible to use the function is_acpi_device_node() to determine if the
> sound card is an ACPI device and hence indicate if we expect a DMI table
> to be present. Therefore, call is_acpi_device_node() to see if we are
> using ACPI and only parse the DMI table if we are booting with ACPI.

Applied to

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

Thanks!

[1/1] ASoC: soc-core: Prevent warning if no DMI table is present
      commit: c014170408bcd2e8fc726802ed16794d358742ff

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

Re: [PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Pierre-Louis Bossart]

On 3/3/21 5:55 AM, Jon Hunter wrote:
> Many systems do not use ACPI and hence do not provide a DMI table. On
> non-ACPI systems a warning, such as the following, is printed on boot.
> 
>   WARNING KERN tegra-audio-graph-card sound: ASoC: no DMI vendor name!
> 
> The variable 'dmi_available' is not exported and so currently cannot be
> used by kernel modules without adding an accessor. However, it is
> possible to use the function is_acpi_device_node() to determine if the
> sound card is an ACPI device and hence indicate if we expect a DMI table
> to be present. Therefore, call is_acpi_device_node() to see if we are
> using ACPI and only parse the DMI table if we are booting with ACPI.

That change introduces a regression on all our tests: the names are not 
correctly set, which will prevent UCM from loading the relevant profiles 
based on the long name.

Before:
  0 [sofhdadsp      ]: sof-hda-dsp - sof-hda-dsp
                       AAEON-UP_WHL01-V1.0

After:
root@plb-UP-WHL01:/proc/asound# more cards
  0 [sofhdadsp      ]: sof-hda-dsp - sof-hda-dsp
                       sof-hda-dsp

The problem is that the cards are platform devices created by the parent 
(which itself may be a PCI or ACPI device) and have nothing to do with ACPI.

Could we flip the logic and instead explicitly detect OF devices? That 
restores functionality for us.

Jon, would this work for you?

Thanks!

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 16ba54eb8164..5c40b4548413 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1574,7 +1574,7 @@ int snd_soc_set_dmi_name(struct snd_soc_card 
*card, const char *flavour)
         if (card->long_name)
                 return 0; /* long name already set by driver or from DMI */

-       if (!is_acpi_device_node(card->dev->fwnode))
+       if (is_of_node(card->dev->fwnode))
                 return 0;

         /* make up dmi long name as: vendor-product-version-board */


> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> Changes since V1:
> - Use is_acpi_device_node() to determine if we expect the DMI table to
>    be present.
> 
>   sound/soc/soc-core.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index f6d4e99b590c..0cffc9527e28 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -31,6 +31,7 @@
>   #include <linux/of.h>
>   #include <linux/of_graph.h>
>   #include <linux/dmi.h>
> +#include <linux/acpi.h>
>   #include <sound/core.h>
>   #include <sound/pcm.h>
>   #include <sound/pcm_params.h>
> @@ -1573,6 +1574,9 @@ int snd_soc_set_dmi_name(struct snd_soc_card *card, const char *flavour)
>   	if (card->long_name)
>   		return 0; /* long name already set by driver or from DMI */
>   
> +	if (!is_acpi_device_node(card->dev->fwnode))
> +		return 0;
> +
>   	/* make up dmi long name as: vendor-product-version-board */
>   	vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
>   	if (!vendor || !is_dmi_valid(vendor)) {
> 

Re: [PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Mark Brown]

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

On Tue, Mar 09, 2021 at 01:41:45PM -0600, Pierre-Louis Bossart wrote:

> The problem is that the cards are platform devices created by the parent
> (which itself may be a PCI or ACPI device) and have nothing to do with ACPI.

> Could we flip the logic and instead explicitly detect OF devices? That
> restores functionality for us.

Just change it to a system level check for ACPI, checking for OF would
leave problems for board files or any other alternative firmware
interfaces.

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

Re: [PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Pierre-Louis Bossart]

On 3/10/21 7:35 AM, Mark Brown wrote:
> On Tue, Mar 09, 2021 at 01:41:45PM -0600, Pierre-Louis Bossart wrote:
> 
>> The problem is that the cards are platform devices created by the parent
>> (which itself may be a PCI or ACPI device) and have nothing to do with ACPI.
> 
>> Could we flip the logic and instead explicitly detect OF devices? That
>> restores functionality for us.
> 
> Just change it to a system level check for ACPI, checking for OF would
> leave problems for board files or any other alternative firmware
> interfaces.

did you mean if (!IS_ENABLED(CONFIG_ACPI)) ?

Re: [PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Mark Brown]

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

On Wed, Mar 10, 2021 at 09:44:07AM -0600, Pierre-Louis Bossart wrote:
> On 3/10/21 7:35 AM, Mark Brown wrote:

> > Just change it to a system level check for ACPI, checking for OF would
> > leave problems for board files or any other alternative firmware
> > interfaces.

> did you mean if (!IS_ENABLED(CONFIG_ACPI)) ?

Is there a runtime check?

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

Re: [PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Takashi Iwai]

On Wed, 10 Mar 2021 17:18:14 +0100,
Mark Brown wrote:
> 
> On Wed, Mar 10, 2021 at 09:44:07AM -0600, Pierre-Louis Bossart wrote:
> > On 3/10/21 7:35 AM, Mark Brown wrote:
> 
> > > Just change it to a system level check for ACPI, checking for OF would
> > > leave problems for board files or any other alternative firmware
> > > interfaces.
> 
> > did you mean if (!IS_ENABLED(CONFIG_ACPI)) ?
> 
> Is there a runtime check?

Well, basically both DMI and ACPI are completely different things, so
I don't think it's right to check the availability of ACPI as a signal
of the availability of DMI.


Takashi

Re: [PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Pierre-Louis Bossart]

On 3/10/21 10:37 AM, Takashi Iwai wrote:
> On Wed, 10 Mar 2021 17:18:14 +0100,
> Mark Brown wrote:
>>
>> On Wed, Mar 10, 2021 at 09:44:07AM -0600, Pierre-Louis Bossart wrote:
>>> On 3/10/21 7:35 AM, Mark Brown wrote:
>>
>>>> Just change it to a system level check for ACPI, checking for OF would
>>>> leave problems for board files or any other alternative firmware
>>>> interfaces.
>>
>>> did you mean if (!IS_ENABLED(CONFIG_ACPI)) ?
>>
>> Is there a runtime check?
> 
> Well, basically both DMI and ACPI are completely different things, so
> I don't think it's right to check the availability of ACPI as a signal
> of the availability of DMI.

would this work?

if (!IS_ENABLED(CONFIG_DMI))
     return 0;

Re: [PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Mark Brown]

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

On Wed, Mar 10, 2021 at 10:41:18AM -0600, Pierre-Louis Bossart wrote:

> would this work?

> if (!IS_ENABLED(CONFIG_DMI))
>     return 0;

Build time dependencies aren't going to help anything, arm64 (and to my
understanding some future x86 systems, LynxPoint IIRC) supports both DT
and ACPI and so you have kernels built with support for both.

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

Re: [PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Pierre-Louis Bossart]

On 3/10/21 10:52 AM, Mark Brown wrote:
> On Wed, Mar 10, 2021 at 10:41:18AM -0600, Pierre-Louis Bossart wrote:
> 
>> would this work?
> 
>> if (!IS_ENABLED(CONFIG_DMI))
>>      return 0;
> 
> Build time dependencies aren't going to help anything, arm64 (and to my
> understanding some future x86 systems, LynxPoint IIRC) supports both DT
> and ACPI and so you have kernels built with support for both.

well, that's what I suggested initially:
        if (is_of_node(card->dev->fwnode))

I used the of_node test as a proxy for 'no DMI' since I am not aware of 
any means to detect if DMI is enabled at run-time.

Re: [PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Mark Brown]

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

On Wed, Mar 10, 2021 at 11:50:13AM -0600, Pierre-Louis Bossart wrote:
> On 3/10/21 10:52 AM, Mark Brown wrote:

> > Build time dependencies aren't going to help anything, arm64 (and to my
> > understanding some future x86 systems, LynxPoint IIRC) supports both DT
> > and ACPI and so you have kernels built with support for both.

> well, that's what I suggested initially:
>        if (is_of_node(card->dev->fwnode))

> I used the of_node test as a proxy for 'no DMI' since I am not aware of any
> means to detect if DMI is enabled at run-time.

Can we not fix the DMI code so it lets us check dmi_available either
directly or with an accessor?  I don't understand why all the proposals
are dancing around local bodges here.

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

Re: [PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Pierre-Louis Bossart]

>>> Build time dependencies aren't going to help anything, arm64 (and to my
>>> understanding some future x86 systems, LynxPoint IIRC) supports both DT
>>> and ACPI and so you have kernels built with support for both.
> 
>> well, that's what I suggested initially:
>>         if (is_of_node(card->dev->fwnode))
> 
>> I used the of_node test as a proxy for 'no DMI' since I am not aware of any
>> means to detect if DMI is enabled at run-time.
> 
> Can we not fix the DMI code so it lets us check dmi_available either
> directly or with an accessor?  I don't understand why all the proposals
> are dancing around local bodges here.

something like this then (compile-tested only)?

diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index d51ca0428bb8..f191a1f901ac 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -166,6 +166,7 @@ static int __init dmi_checksum(const u8 *buf, u8 len)
  static const char *dmi_ident[DMI_STRING_MAX];
  static LIST_HEAD(dmi_devices);
  int dmi_available;
+EXPORT_SYMBOL_GPL(dmi_available);

  /*
   *     Save a DMI string
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 16ba54eb8164..c7e4600b2dd4 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1574,7 +1574,7 @@ int snd_soc_set_dmi_name(struct snd_soc_card 
*card, const char *flavour)
         if (card->long_name)
                 return 0; /* long name already set by driver or from DMI */

-       if (!is_acpi_device_node(card->dev->fwnode))
+       if (!dmi_available)
                 return 0;

         /* make up dmi long name as: vendor-product-version-board */

Re: [PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Jon Hunter]

On 10/03/2021 18:37, Pierre-Louis Bossart wrote:
> 
>>>> Build time dependencies aren't going to help anything, arm64 (and to my
>>>> understanding some future x86 systems, LynxPoint IIRC) supports both DT
>>>> and ACPI and so you have kernels built with support for both.
>>
>>> well, that's what I suggested initially:
>>>         if (is_of_node(card->dev->fwnode))
>>
>>> I used the of_node test as a proxy for 'no DMI' since I am not aware
>>> of any
>>> means to detect if DMI is enabled at run-time.
>>
>> Can we not fix the DMI code so it lets us check dmi_available either
>> directly or with an accessor?  I don't understand why all the proposals
>> are dancing around local bodges here.
> 
> something like this then (compile-tested only)?
> 
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index d51ca0428bb8..f191a1f901ac 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -166,6 +166,7 @@ static int __init dmi_checksum(const u8 *buf, u8 len)
>  static const char *dmi_ident[DMI_STRING_MAX];
>  static LIST_HEAD(dmi_devices);
>  int dmi_available;
> +EXPORT_SYMBOL_GPL(dmi_available);
> 
>  /*
>   *     Save a DMI string
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 16ba54eb8164..c7e4600b2dd4 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1574,7 +1574,7 @@ int snd_soc_set_dmi_name(struct snd_soc_card
> *card, const char *flavour)
>         if (card->long_name)
>                 return 0; /* long name already set by driver or from DMI */
> 
> -       if (!is_acpi_device_node(card->dev->fwnode))
> +       if (!dmi_available)
>                 return 0;
> 
>         /* make up dmi long name as: vendor-product-version-board */


Sounds good to me. I would have done the same if I had known that the
current solution would have caused this regression.

Cheers
Jon

-- 
nvpublic

Re: [PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Mark Brown]

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

On Wed, Mar 10, 2021 at 07:17:13PM +0000, Jon Hunter wrote:
> On 10/03/2021 18:37, Pierre-Louis Bossart wrote:

> > +EXPORT_SYMBOL_GPL(dmi_available);

> > -       if (!is_acpi_device_node(card->dev->fwnode))
> > +       if (!dmi_available)

> Sounds good to me. I would have done the same if I had known that the
> current solution would have caused this regression.

Yes, this looks good.

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

Re: [PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Mark Brown]

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

On Wed, Mar 10, 2021 at 05:37:25PM +0100, Takashi Iwai wrote:
> Mark Brown wrote:

> > > did you mean if (!IS_ENABLED(CONFIG_ACPI)) ?

> > Is there a runtime check?

> Well, basically both DMI and ACPI are completely different things, so
> I don't think it's right to check the availability of ACPI as a signal
> of the availability of DMI.

In theory they are only somewhat related, but in practice they're both
part of a holistic system model - ACPI users complain if their system
does not also provide DMI information.

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