diff mbox series

[v3,11/15] ASoC: Intel: avs: Machine board registration

Message ID 20220516101116.190192-12-cezary.rojewski@intel.com
State Accepted
Commit beed983621fbdfd291e6e3a0cdc4d10517e60af8
Headers show
Series ASoC: Intel: avs: Driver core and PCM operations | expand

Commit Message

Cezary Rojewski May 16, 2022, 10:11 a.m. UTC
AVS driver operates with granular audio card division in mind.
Super-card approach (e.g.: I2S, DMIC and HDA DAIs combined) is
deprecated in favour of individual cards - one per each device. This
provides necessary dynamism, especially for configurations with number
of codecs present and makes it easier to survive auxiliary devices
failures - one card failing to probe does not prevent others from
succeeding.

All boards spawned by AVS are unregistered on ->remove(). This includes
dummy codecs such as DMIC.

As all machine boards found in sound/soc/intel/boards are irreversibly
tied to 'super-card' approach, new boards are going to be introduced.
This temporarily increases number of boards available under /intel
directory until skylake-driver becomes deprecated and removed.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/avs/Makefile          |   2 +-
 sound/soc/intel/avs/avs.h             |   3 +
 sound/soc/intel/avs/board_selection.c | 501 ++++++++++++++++++++++++++
 3 files changed, 505 insertions(+), 1 deletion(-)
 create mode 100644 sound/soc/intel/avs/board_selection.c

Comments

Guenter Roeck May 26, 2022, 4:24 p.m. UTC | #1
On Mon, May 16, 2022 at 12:11:12PM +0200, Cezary Rojewski wrote:
> AVS driver operates with granular audio card division in mind.
> Super-card approach (e.g.: I2S, DMIC and HDA DAIs combined) is
> deprecated in favour of individual cards - one per each device. This
> provides necessary dynamism, especially for configurations with number
> of codecs present and makes it easier to survive auxiliary devices
> failures - one card failing to probe does not prevent others from
> succeeding.
> 
> All boards spawned by AVS are unregistered on ->remove(). This includes
> dummy codecs such as DMIC.
> 
> As all machine boards found in sound/soc/intel/boards are irreversibly
> tied to 'super-card' approach, new boards are going to be introduced.
> This temporarily increases number of boards available under /intel
> directory until skylake-driver becomes deprecated and removed.
> 
> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> ---
[ ... ]

> +
> +static int avs_register_i2s_board(struct avs_dev *adev, struct snd_soc_acpi_mach *mach)
> +{
> +	struct platform_device *board;
> +	int num_ssps;
> +	char *name;
> +	int ret;
> +
> +	num_ssps = adev->hw_cfg.i2s_caps.ctrl_count;
> +	if (fls(mach->mach_params.i2s_link_mask) > num_ssps) {
> +		dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",

   sound/soc/intel/avs/board_selection.c: In function 'avs_register_i2s_board':
>> sound/soc/intel/avs/board_selection.c:328:36: warning: format '%ld' expects argument of type 'long int', but argument 5 has type 'int' [-Wformat=]
     328 |                 dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
                                                                                                  ^^^

Reported by 0-day but still made it into mainline.

Guenter
Guenter Roeck May 26, 2022, 5:09 p.m. UTC | #2
On 5/26/22 09:31, Cezary Rojewski wrote:
> On 2022-05-26 6:24 PM, Guenter Roeck wrote:
>> On Mon, May 16, 2022 at 12:11:12PM +0200, Cezary Rojewski wrote:
>>> AVS driver operates with granular audio card division in mind.
>>> Super-card approach (e.g.: I2S, DMIC and HDA DAIs combined) is
>>> deprecated in favour of individual cards - one per each device. This
>>> provides necessary dynamism, especially for configurations with number
>>> of codecs present and makes it easier to survive auxiliary devices
>>> failures - one card failing to probe does not prevent others from
>>> succeeding.
>>>
>>> All boards spawned by AVS are unregistered on ->remove(). This includes
>>> dummy codecs such as DMIC.
>>>
>>> As all machine boards found in sound/soc/intel/boards are irreversibly
>>> tied to 'super-card' approach, new boards are going to be introduced.
>>> This temporarily increases number of boards available under /intel
>>> directory until skylake-driver becomes deprecated and removed.
>>>
>>> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
>>> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
>>> ---
> 
> 
>>> +
>>> +static int avs_register_i2s_board(struct avs_dev *adev, struct snd_soc_acpi_mach *mach)
>>> +{
>>> +    struct platform_device *board;
>>> +    int num_ssps;
>>> +    char *name;
>>> +    int ret;
>>> +
>>> +    num_ssps = adev->hw_cfg.i2s_caps.ctrl_count;
>>> +    if (fls(mach->mach_params.i2s_link_mask) > num_ssps) {
>>> +        dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
>>
>>     sound/soc/intel/avs/board_selection.c: In function 'avs_register_i2s_board':
>>>> sound/soc/intel/avs/board_selection.c:328:36: warning: format '%ld' expects argument of type 'long int', but argument 5 has type 'int' [-Wformat=]
>>       328 |                 dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
>>                                                                                                    ^^^
>>
>> Reported by 0-day but still made it into mainline.
> 
> 
> Hello,
> 
> Thanks for the report. This seems to be a false-positive and is being addressed by a separate patchset [1]. Perhaps the cover letter for the mentioned series could have looped 0-day.

No, this is not a false positive. More like a false assumption.

> If I'm wrong about this, feel free to correct me.
> 
> 
> [1]: https://lore.kernel.org/lkml/20220525144844.1571705-1-amadeuszx.slawinski@linux.intel.com/
> 
This is incomplete. It does not address all definitions of __fls() for arc, and it
does not address the build failure for sparc64 (arch/sparc/include/asm/bitops_64.h).

Guenter
Uwe Kleine-König May 29, 2022, 5:48 a.m. UTC | #3
Hello Mark,

On Thu, May 26, 2022 at 06:44:38PM +0100, Mark Brown wrote:
> On Thu, May 26, 2022 at 09:24:43AM -0700, Guenter Roeck wrote:
> > On Mon, May 16, 2022 at 12:11:12PM +0200, Cezary Rojewski wrote:
> 
> > > +	if (fls(mach->mach_params.i2s_link_mask) > num_ssps) {
> > > +		dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
> 
> >    sound/soc/intel/avs/board_selection.c: In function 'avs_register_i2s_board':
> > >> sound/soc/intel/avs/board_selection.c:328:36: warning: format '%ld' expects argument of type 'long int', but argument 5 has type 'int' [-Wformat=]
> >      328 |                 dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
> >                                                                                                   ^^^
> > Reported by 0-day but still made it into mainline.
> 
> FWIW given how hard the 0-day reports have become to read I'd not rely
> on people paying attention to things that are clearly pure build
> coverage based off a 0-day report alone.

I'm unsure if I understand your sentiment correctly. Are you saying it
doesn't matter if a patch breaks the build on some arch using a
randconfig?

My position is: The commit under discussion (i.e. beed983621fb ("ASoC:
Intel: avs: Machine board registration")) breaks an allmodconfig build
on all platforms where __fls doesn't return a long int (i.e. arc, m68k,
and sparc). This actively hurts people who do build tests using
allmodconfig (or allyesconfig) for their patch series (e.g. me).

I agree that some reports by the 0-day bot are hard to parse. But still,
if there is a build problem someone should look into that. If you (with
your maintainer hat on) don't have the resource to do that, that's IMHO
fine. But I'd wish you'd push back on the patch submitter in that case
and don't apply the patch until the problem is resolved. If this is a
corner case randconfig issue, applying might be fine despite the build
error but breaking allmodconfig on 3 archs is bad.

The fix would be 

diff --git a/sound/soc/intel/avs/board_selection.c b/sound/soc/intel/avs/board_selection.c
index 80cb0164678a..f189f71b8e1e 100644
--- a/sound/soc/intel/avs/board_selection.c
+++ b/sound/soc/intel/avs/board_selection.c
@@ -325,8 +325,8 @@ static int avs_register_i2s_board(struct avs_dev *adev, struct snd_soc_acpi_mach
 
 	num_ssps = adev->hw_cfg.i2s_caps.ctrl_count;
 	if (fls(mach->mach_params.i2s_link_mask) > num_ssps) {
-		dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
-			num_ssps, mach->drv_name, __fls(mach->mach_params.i2s_link_mask));
+		dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%d\n",
+			num_ssps, mach->drv_name, fls(mach->mach_params.i2s_link_mask));
 		return -ENODEV;
 	}
 
which uses fls instead of __fls (as is done in the test triggering the
error). The former returns an int on all platforms.

Tell me if you don't want to squash this into beed983621fb and prefer a
formal patch.

Best regards
Uwe
Guenter Roeck May 29, 2022, 6:21 a.m. UTC | #4
On 5/28/22 23:05, Uwe Kleine-König wrote:
> On some platforms (i.e. arc, m68k and sparc) __fls returns an int (while
> on most platforms it returns an unsigned long). This triggers a format
> warning on these few platforms as the driver uses %ld to print a warning.
> 
> Replace it by fls (and %d) which returns an int everywhere and which is
> already used in the if condition triggering the warning.
> 
> Fixes: beed983621fb ("ASoC: Intel: avs: Machine board registration")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

FWIW:

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Hello again
> 
> On Sun, May 29, 2022 at 07:48:18AM +0200, Uwe Kleine-König wrote:
>> Tell me if you don't want to squash this into beed983621fb and prefer a
>> formal patch.
> 
> I just realized this isn't a problem in next only any more, but the
> commit is already in Linus Torvald's tree. So I guess this isn't fixed
> by a fixup of said commit and here comes a proper patch.
> 
>   sound/soc/intel/avs/board_selection.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/intel/avs/board_selection.c b/sound/soc/intel/avs/board_selection.c
> index 80cb0164678a..f189f71b8e1e 100644
> --- a/sound/soc/intel/avs/board_selection.c
> +++ b/sound/soc/intel/avs/board_selection.c
> @@ -325,8 +325,8 @@ static int avs_register_i2s_board(struct avs_dev *adev, struct snd_soc_acpi_mach
>   
>   	num_ssps = adev->hw_cfg.i2s_caps.ctrl_count;
>   	if (fls(mach->mach_params.i2s_link_mask) > num_ssps) {
> -		dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
> -			num_ssps, mach->drv_name, __fls(mach->mach_params.i2s_link_mask));
> +		dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%d\n",
> +			num_ssps, mach->drv_name, fls(mach->mach_params.i2s_link_mask));
>   		return -ENODEV;
>   	}
>   
> 
> base-commit: beed983621fbdfd291e6e3a0cdc4d10517e60af8
Cezary Rojewski May 29, 2022, 1:24 p.m. UTC | #5
On 2022-05-29 7:48 AM, Uwe Kleine-König wrote:
> Hello Mark,
> 
> On Thu, May 26, 2022 at 06:44:38PM +0100, Mark Brown wrote:

>> FWIW given how hard the 0-day reports have become to read I'd not rely
>> on people paying attention to things that are clearly pure build
>> coverage based off a 0-day report alone.
> 
> I'm unsure if I understand your sentiment correctly. Are you saying it
> doesn't matter if a patch breaks the build on some arch using a
> randconfig?
> 
> My position is: The commit under discussion (i.e. beed983621fb ("ASoC:
> Intel: avs: Machine board registration")) breaks an allmodconfig build
> on all platforms where __fls doesn't return a long int (i.e. arc, m68k,
> and sparc). This actively hurts people who do build tests using
> allmodconfig (or allyesconfig) for their patch series (e.g. me).
> 
> I agree that some reports by the 0-day bot are hard to parse. But still,
> if there is a build problem someone should look into that. If you (with
> your maintainer hat on) don't have the resource to do that, that's IMHO
> fine. But I'd wish you'd push back on the patch submitter in that case
> and don't apply the patch until the problem is resolved. If this is a
> corner case randconfig issue, applying might be fine despite the build
> error but breaking allmodconfig on 3 archs is bad.
> 
> The fix would be


Hello Uwe,

I don't believe anyone here wanted to break the build-configurations you 
did mention above. I also believe it's important to mention that below 
is not a fix but a W/A. Developer should be able to use __fls() if 
required. Value returned by fls() differs from one returned by __fls(), 
and using fls() in below context is misleading (hurts the debug-ability).

Yes, the faulty print should be devoid of __fls() until the function is 
fixed for all the archs. It's too late for that and I'm sorry for any 
inconvenience caused by the change.
To my knowledge the real fix has been provided on LKML by Amadeo [1] and 
is under review since Friday. I'd kindly appreciate helping fix the root 
cause of the problem. If there's anything missing in the series let us know.


[1]: 
https://lore.kernel.org/lkml/20220527115345.2588775-3-amadeuszx.slawinski@linux.intel.com/T/


Regards,
Czarek


> diff --git a/sound/soc/intel/avs/board_selection.c b/sound/soc/intel/avs/board_selection.c
> index 80cb0164678a..f189f71b8e1e 100644
> --- a/sound/soc/intel/avs/board_selection.c
> +++ b/sound/soc/intel/avs/board_selection.c
> @@ -325,8 +325,8 @@ static int avs_register_i2s_board(struct avs_dev *adev, struct snd_soc_acpi_mach
>   
>   	num_ssps = adev->hw_cfg.i2s_caps.ctrl_count;
>   	if (fls(mach->mach_params.i2s_link_mask) > num_ssps) {
> -		dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
> -			num_ssps, mach->drv_name, __fls(mach->mach_params.i2s_link_mask));
> +		dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%d\n",
> +			num_ssps, mach->drv_name, fls(mach->mach_params.i2s_link_mask));
>   		return -ENODEV;
>   	}
>   
> which uses fls instead of __fls (as is done in the test triggering the
> error). The former returns an int on all platforms.
> 
> Tell me if you don't want to squash this into beed983621fb and prefer a
> formal patch.
> 
> Best regards
> Uwe
>
Uwe Kleine-König May 29, 2022, 1:50 p.m. UTC | #6
On Sun, May 29, 2022 at 03:24:48PM +0200, Cezary Rojewski wrote:
> On 2022-05-29 7:48 AM, Uwe Kleine-König wrote:
> > Hello Mark,
> > 
> > On Thu, May 26, 2022 at 06:44:38PM +0100, Mark Brown wrote:
> 
> > > FWIW given how hard the 0-day reports have become to read I'd not rely
> > > on people paying attention to things that are clearly pure build
> > > coverage based off a 0-day report alone.
> > 
> > I'm unsure if I understand your sentiment correctly. Are you saying it
> > doesn't matter if a patch breaks the build on some arch using a
> > randconfig?
> > 
> > My position is: The commit under discussion (i.e. beed983621fb ("ASoC:
> > Intel: avs: Machine board registration")) breaks an allmodconfig build
> > on all platforms where __fls doesn't return a long int (i.e. arc, m68k,
> > and sparc). This actively hurts people who do build tests using
> > allmodconfig (or allyesconfig) for their patch series (e.g. me).
> > 
> > I agree that some reports by the 0-day bot are hard to parse. But still,
> > if there is a build problem someone should look into that. If you (with
> > your maintainer hat on) don't have the resource to do that, that's IMHO
> > fine. But I'd wish you'd push back on the patch submitter in that case
> > and don't apply the patch until the problem is resolved. If this is a
> > corner case randconfig issue, applying might be fine despite the build
> > error but breaking allmodconfig on 3 archs is bad.
> > 
> > The fix would be
> 
> 
> Hello Uwe,
> 
> I don't believe anyone here wanted to break the build-configurations you did
> mention above. I also believe it's important to mention that below is not a
> fix but a W/A. Developer should be able to use __fls() if required. Value
> returned by fls() differs from one returned by __fls(), and using fls() in
> below context is misleading (hurts the debug-ability).
> 
> Yes, the faulty print should be devoid of __fls() until the function is
> fixed for all the archs. It's too late for that and I'm sorry for any
> inconvenience caused by the change.
> To my knowledge the real fix has been provided on LKML by Amadeo [1] and is
> under review since Friday. I'd kindly appreciate helping fix the root cause
> of the problem. If there's anything missing in the series let us know.

Hmm, I doubt this (i.e. unify the return value of __fls) will be merged
during this cycle. I'd want that to cook a bit in next before it hits
mainline, there might be similar problems introduced by changing __fls
to the one under discussion here.

A more short-term fix would be:

diff --git a/sound/soc/intel/avs/board_selection.c b/sound/soc/intel/avs/board_selection.c
--- a/sound/soc/intel/avs/board_selection.c
+++ b/sound/soc/intel/avs/board_selection.c
@@ -325,8 +325,8 @@ static int avs_register_i2s_board(struct avs_dev *adev, struct snd_soc_acpi_mach
  	num_ssps = adev->hw_cfg.i2s_caps.ctrl_count;
  	if (fls(mach->mach_params.i2s_link_mask) > num_ssps) {
 		dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
-			num_ssps, mach->drv_name, __fls(mach->mach_params.i2s_link_mask));
+			num_ssps, mach->drv_name, (unsigned long)__fls(mach->mach_params.i2s_link_mask));
  		return -ENODEV;
  	}

i.e. explicitly cast the return value of __fls to unsigned long.

Best regards
Uwe
Cezary Rojewski May 29, 2022, 1:56 p.m. UTC | #7
On 2022-05-29 3:50 PM, Uwe Kleine-König wrote:

> A more short-term fix would be:
> 
> diff --git a/sound/soc/intel/avs/board_selection.c b/sound/soc/intel/avs/board_selection.c
> --- a/sound/soc/intel/avs/board_selection.c
> +++ b/sound/soc/intel/avs/board_selection.c
> @@ -325,8 +325,8 @@ static int avs_register_i2s_board(struct avs_dev *adev, struct snd_soc_acpi_mach
>    	num_ssps = adev->hw_cfg.i2s_caps.ctrl_count;
>    	if (fls(mach->mach_params.i2s_link_mask) > num_ssps) {
>   		dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
> -			num_ssps, mach->drv_name, __fls(mach->mach_params.i2s_link_mask));
> +			num_ssps, mach->drv_name, (unsigned long)__fls(mach->mach_params.i2s_link_mask));
>    		return -ENODEV;
>    	}
> 
> i.e. explicitly cast the return value of __fls to unsigned long.

This looks very good indeed. Will you be updating the patch you had sent 
earlier today or would you like me to send it instead?

If you choose the former, feel free to append:

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>


Regards,
Czarek
Mark Brown May 30, 2022, 1:29 p.m. UTC | #8
On Sun, May 29, 2022 at 07:48:07AM +0200, Uwe Kleine-König wrote:
> On Thu, May 26, 2022 at 06:44:38PM +0100, Mark Brown wrote:

> > FWIW given how hard the 0-day reports have become to read I'd not rely
> > on people paying attention to things that are clearly pure build
> > coverage based off a 0-day report alone.

> I'm unsure if I understand your sentiment correctly. Are you saying it
> doesn't matter if a patch breaks the build on some arch using a
> randconfig?

It matters, but in practice something that is only reported by an
automated tool with a hard to read report against a a randconfig
on an obscure platform that you need to go find a toolchain for
is a lot more likely to get buried than something that doesn't
have those properties.  Similarly if a tool is sending a lot of
reports with those properties it's a lot easier for even reports
in more important issues from that tool to get dropped on the
floor.

> My position is: The commit under discussion (i.e. beed983621fb ("ASoC:
> Intel: avs: Machine board registration")) breaks an allmodconfig build
> on all platforms where __fls doesn't return a long int (i.e. arc, m68k,
> and sparc). This actively hurts people who do build tests using
> allmodconfig (or allyesconfig) for their patch series (e.g. me).

> I agree that some reports by the 0-day bot are hard to parse. But still,
> if there is a build problem someone should look into that. If you (with
> your maintainer hat on) don't have the resource to do that, that's IMHO
> fine. But I'd wish you'd push back on the patch submitter in that case
> and don't apply the patch until the problem is resolved. If this is a
> corner case randconfig issue, applying might be fine despite the build
> error but breaking allmodconfig on 3 archs is bad.

My general approach here is that I'm expecting the reports from 0
day to be enough so I don't explicitly push back, but I don't
generally apply anything that looks like it has a live issue.  In
this case it looks like what happened was that 0-day only
reported against v2 and not v3 so it wasn't apparent to me that
the report hadn't been addressed in the new version, there
weren't any reports against v3 that I can see.

The other thing here is that if something did get missed and ends
up getting applied it really helps to get an explicit report that
there's a problem in -next, clearly something got missed and it's
much easier to get something dropped just after it gets applied.
TBH I hadn't realised from the followup that this was an
all*config issue, my best guess was that it was a randconfig
thing thatg got missed.
diff mbox series

Patch

diff --git a/sound/soc/intel/avs/Makefile b/sound/soc/intel/avs/Makefile
index 38285e73e75d..592d4dc02c56 100644
--- a/sound/soc/intel/avs/Makefile
+++ b/sound/soc/intel/avs/Makefile
@@ -1,7 +1,7 @@ 
 # SPDX-License-Identifier: GPL-2.0-only
 
 snd-soc-avs-objs := dsp.o ipc.o messages.o utils.o core.o loader.o \
-		    topology.o path.o pcm.o
+		    topology.o path.o pcm.o board_selection.o
 snd-soc-avs-objs += cldma.o
 
 snd-soc-avs-objs += trace.o
diff --git a/sound/soc/intel/avs/avs.h b/sound/soc/intel/avs/avs.h
index 583f46d5a9bc..92e8d647b3f9 100644
--- a/sound/soc/intel/avs/avs.h
+++ b/sound/soc/intel/avs/avs.h
@@ -310,6 +310,9 @@  int avs_i2s_platform_register(struct avs_dev *adev, const char *name, unsigned l
 			      unsigned long *tdms);
 int avs_hda_platform_register(struct avs_dev *adev, const char *name);
 
+int avs_register_all_boards(struct avs_dev *adev);
+void avs_unregister_all_boards(struct avs_dev *adev);
+
 /* Firmware tracing helpers */
 
 unsigned int __kfifo_fromio_locked(struct kfifo *fifo, const void __iomem *src, unsigned int len,
diff --git a/sound/soc/intel/avs/board_selection.c b/sound/soc/intel/avs/board_selection.c
new file mode 100644
index 000000000000..80cb0164678a
--- /dev/null
+++ b/sound/soc/intel/avs/board_selection.c
@@ -0,0 +1,501 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Copyright(c) 2021-2022 Intel Corporation. All rights reserved.
+//
+// Authors: Cezary Rojewski <cezary.rojewski@intel.com>
+//          Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>
+//
+
+#include <linux/acpi.h>
+#include <linux/module.h>
+#include <linux/dmi.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <sound/hda_codec.h>
+#include <sound/hda_register.h>
+#include <sound/intel-nhlt.h>
+#include <sound/soc-acpi.h>
+#include <sound/soc-component.h>
+#include "avs.h"
+
+static bool i2s_test;
+module_param(i2s_test, bool, 0444);
+MODULE_PARM_DESC(i2s_test, "Probe I2S test-board and skip all other I2S boards");
+
+static const struct dmi_system_id kbl_dmi_table[] = {
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Intel Corporation"),
+			DMI_MATCH(DMI_BOARD_NAME, "Skylake Y LPDDR3 RVP3"),
+		},
+	},
+	{}
+};
+
+static const struct dmi_system_id kblr_dmi_table[] = {
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Intel Corporation"),
+			DMI_MATCH(DMI_BOARD_NAME, "Kabylake R DDR4 RVP"),
+		},
+	},
+	{}
+};
+
+static struct snd_soc_acpi_mach *dmi_match_quirk(void *arg)
+{
+	struct snd_soc_acpi_mach *mach = arg;
+	const struct dmi_system_id *dmi_id;
+	struct dmi_system_id *dmi_table;
+
+	if (mach->quirk_data == NULL)
+		return mach;
+
+	dmi_table = (struct dmi_system_id *)mach->quirk_data;
+
+	dmi_id = dmi_first_match(dmi_table);
+	if (!dmi_id)
+		return NULL;
+
+	return mach;
+}
+
+#define AVS_SSP(x)		(BIT(x))
+#define AVS_SSP_RANGE(a, b)	(GENMASK(b, a))
+
+/* supported I2S board codec configurations */
+static struct snd_soc_acpi_mach avs_skl_i2s_machines[] = {
+	{
+		.id = "INT343A",
+		.drv_name = "avs_rt286",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(0),
+		},
+		.tplg_filename = "rt286-tplg.bin",
+	},
+	{
+		.id = "10508825",
+		.drv_name = "avs_nau8825",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(1),
+		},
+		.tplg_filename = "nau8825-tplg.bin",
+	},
+	{
+		.id = "INT343B",
+		.drv_name = "avs_ssm4567",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(0),
+		},
+		.tplg_filename = "ssm4567-tplg.bin",
+	},
+	{
+		.id = "MX98357A",
+		.drv_name = "avs_max98357a",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(0),
+		},
+		.tplg_filename = "max98357a-tplg.bin",
+	},
+	{},
+};
+
+static struct snd_soc_acpi_mach avs_kbl_i2s_machines[] = {
+	{
+		.id = "INT343A",
+		.drv_name = "avs_rt286",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(0),
+		},
+		.quirk_data = &kbl_dmi_table,
+		.machine_quirk = dmi_match_quirk,
+		.tplg_filename = "rt286-tplg.bin",
+	},
+	{
+		.id = "INT343A",
+		.drv_name = "avs_rt298",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(0),
+		},
+		.quirk_data = &kblr_dmi_table,
+		.machine_quirk = dmi_match_quirk,
+		.tplg_filename = "rt298-tplg.bin",
+	},
+	{
+		.id = "MX98373",
+		.drv_name = "avs_max98373",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(0),
+		},
+		.tplg_filename = "max98373-tplg.bin",
+	},
+	{
+		.id = "DLGS7219",
+		.drv_name = "avs_da7219",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(1),
+		},
+		.tplg_filename = "da7219-tplg.bin",
+	},
+	{},
+};
+
+static struct snd_soc_acpi_mach avs_apl_i2s_machines[] = {
+	{
+		.id = "INT343A",
+		.drv_name = "avs_rt298",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(5),
+		},
+		.tplg_filename = "rt298-tplg.bin",
+	},
+	{
+		.id = "INT34C3",
+		.drv_name = "avs_tdf8532",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP_RANGE(0, 5),
+		},
+		.pdata = (unsigned long[]){ 0, 0, 0x14, 0, 0, 0 }, /* SSP2 TDMs */
+		.tplg_filename = "tdf8532-tplg.bin",
+	},
+	{
+		.id = "MX98357A",
+		.drv_name = "avs_max98357a",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(5),
+		},
+		.tplg_filename = "max98357a-tplg.bin",
+	},
+	{
+		.id = "DLGS7219",
+		.drv_name = "avs_da7219",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(1),
+		},
+		.tplg_filename = "da7219-tplg.bin",
+	},
+	{},
+};
+
+static struct snd_soc_acpi_mach avs_gml_i2s_machines[] = {
+	{
+		.id = "INT343A",
+		.drv_name = "avs_rt298",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(2),
+		},
+		.tplg_filename = "rt298-tplg.bin",
+	},
+	{},
+};
+
+static struct snd_soc_acpi_mach avs_test_i2s_machines[] = {
+	{
+		.drv_name = "avs_i2s_test",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(0),
+		},
+		.tplg_filename = "i2s-test-tplg.bin",
+	},
+	{
+		.drv_name = "avs_i2s_test",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(1),
+		},
+		.tplg_filename = "i2s-test-tplg.bin",
+	},
+	{
+		.drv_name = "avs_i2s_test",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(2),
+		},
+		.tplg_filename = "i2s-test-tplg.bin",
+	},
+	{
+		.drv_name = "avs_i2s_test",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(3),
+		},
+		.tplg_filename = "i2s-test-tplg.bin",
+	},
+	{
+		.drv_name = "avs_i2s_test",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(4),
+		},
+		.tplg_filename = "i2s-test-tplg.bin",
+	},
+	{
+		.drv_name = "avs_i2s_test",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(5),
+		},
+		.tplg_filename = "i2s-test-tplg.bin",
+	},
+	/* no NULL terminator, as we depend on ARRAY SIZE due to .id == NULL */
+};
+
+struct avs_acpi_boards {
+	int id;
+	struct snd_soc_acpi_mach *machs;
+};
+
+#define AVS_MACH_ENTRY(_id, _mach) \
+	{ .id = (_id), .machs = (_mach), }
+
+/* supported I2S boards per platform */
+static const struct avs_acpi_boards i2s_boards[] = {
+	AVS_MACH_ENTRY(0x9d70, avs_skl_i2s_machines), /* SKL */
+	AVS_MACH_ENTRY(0x9d71, avs_kbl_i2s_machines), /* KBL */
+	AVS_MACH_ENTRY(0x5a98, avs_apl_i2s_machines), /* APL */
+	AVS_MACH_ENTRY(0x3198, avs_gml_i2s_machines), /* GML */
+	{},
+};
+
+static const struct avs_acpi_boards *avs_get_i2s_boards(struct avs_dev *adev)
+{
+	int id, i;
+
+	id = adev->base.pci->device;
+	for (i = 0; i < ARRAY_SIZE(i2s_boards); i++)
+		if (i2s_boards[i].id == id)
+			return &i2s_boards[i];
+	return NULL;
+}
+
+/* platform devices owned by AVS audio are removed with this hook */
+static void board_pdev_unregister(void *data)
+{
+	platform_device_unregister(data);
+}
+
+static int avs_register_dmic_board(struct avs_dev *adev)
+{
+	struct platform_device *codec, *board;
+	struct snd_soc_acpi_mach mach = {{0}};
+	int ret;
+
+	if (!adev->nhlt ||
+	    !intel_nhlt_has_endpoint_type(adev->nhlt, NHLT_LINK_DMIC)) {
+		dev_dbg(adev->dev, "no DMIC endpoints present\n");
+		return 0;
+	}
+
+	codec = platform_device_register_simple("dmic-codec", PLATFORM_DEVID_NONE, NULL, 0);
+	if (IS_ERR(codec)) {
+		dev_err(adev->dev, "dmic codec register failed\n");
+		return PTR_ERR(codec);
+	}
+
+	ret = devm_add_action(adev->dev, board_pdev_unregister, codec);
+	if (ret < 0) {
+		platform_device_unregister(codec);
+		return ret;
+	}
+
+	ret = avs_dmic_platform_register(adev, "dmic-platform");
+	if (ret < 0)
+		return ret;
+
+	mach.tplg_filename = "dmic-tplg.bin";
+	mach.mach_params.platform = "dmic-platform";
+
+	board = platform_device_register_data(NULL, "avs_dmic", PLATFORM_DEVID_NONE,
+					(const void *)&mach, sizeof(mach));
+	if (IS_ERR(board)) {
+		dev_err(adev->dev, "dmic board register failed\n");
+		return PTR_ERR(board);
+	}
+
+	ret = devm_add_action(adev->dev, board_pdev_unregister, board);
+	if (ret < 0) {
+		platform_device_unregister(board);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int avs_register_i2s_board(struct avs_dev *adev, struct snd_soc_acpi_mach *mach)
+{
+	struct platform_device *board;
+	int num_ssps;
+	char *name;
+	int ret;
+
+	num_ssps = adev->hw_cfg.i2s_caps.ctrl_count;
+	if (fls(mach->mach_params.i2s_link_mask) > num_ssps) {
+		dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
+			num_ssps, mach->drv_name, __fls(mach->mach_params.i2s_link_mask));
+		return -ENODEV;
+	}
+
+	name = devm_kasprintf(adev->dev, GFP_KERNEL, "%s.%d-platform", mach->drv_name,
+			      mach->mach_params.i2s_link_mask);
+	if (!name)
+		return -ENOMEM;
+
+	ret = avs_i2s_platform_register(adev, name, mach->mach_params.i2s_link_mask, mach->pdata);
+	if (ret < 0)
+		return ret;
+
+	mach->mach_params.platform = name;
+
+	board = platform_device_register_data(NULL, mach->drv_name, mach->mach_params.i2s_link_mask,
+					      (const void *)mach, sizeof(*mach));
+	if (IS_ERR(board)) {
+		dev_err(adev->dev, "ssp board register failed\n");
+		return PTR_ERR(board);
+	}
+
+	ret = devm_add_action(adev->dev, board_pdev_unregister, board);
+	if (ret < 0) {
+		platform_device_unregister(board);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int avs_register_i2s_boards(struct avs_dev *adev)
+{
+	const struct avs_acpi_boards *boards;
+	struct snd_soc_acpi_mach *mach;
+	int ret;
+
+	if (!adev->nhlt || !intel_nhlt_has_endpoint_type(adev->nhlt, NHLT_LINK_SSP)) {
+		dev_dbg(adev->dev, "no I2S endpoints present\n");
+		return 0;
+	}
+
+	if (i2s_test) {
+		int i, num_ssps;
+
+		num_ssps = adev->hw_cfg.i2s_caps.ctrl_count;
+		/* constrain just in case FW says there can be more SSPs than possible */
+		num_ssps = min_t(int, ARRAY_SIZE(avs_test_i2s_machines), num_ssps);
+
+		mach = avs_test_i2s_machines;
+
+		for (i = 0; i < num_ssps; i++) {
+			ret = avs_register_i2s_board(adev, &mach[i]);
+			if (ret < 0)
+				dev_warn(adev->dev, "register i2s %s failed: %d\n", mach->drv_name,
+					 ret);
+		}
+		return 0;
+	}
+
+	boards = avs_get_i2s_boards(adev);
+	if (!boards) {
+		dev_dbg(adev->dev, "no I2S endpoints supported\n");
+		return 0;
+	}
+
+	for (mach = boards->machs; mach->id[0]; mach++) {
+		if (!acpi_dev_present(mach->id, NULL, -1))
+			continue;
+
+		if (mach->machine_quirk)
+			if (!mach->machine_quirk(mach))
+				continue;
+
+		ret = avs_register_i2s_board(adev, mach);
+		if (ret < 0)
+			dev_warn(adev->dev, "register i2s %s failed: %d\n", mach->drv_name, ret);
+	}
+
+	return 0;
+}
+
+static int avs_register_hda_board(struct avs_dev *adev, struct hda_codec *codec)
+{
+	struct snd_soc_acpi_mach mach = {{0}};
+	struct platform_device *board;
+	struct hdac_device *hdev = &codec->core;
+	char *pname;
+	int ret, id;
+
+	pname = devm_kasprintf(adev->dev, GFP_KERNEL, "%s-platform", dev_name(&hdev->dev));
+	if (!pname)
+		return -ENOMEM;
+
+	ret = avs_hda_platform_register(adev, pname);
+	if (ret < 0)
+		return ret;
+
+	mach.pdata = codec;
+	mach.mach_params.platform = pname;
+	mach.tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL, "hda-%08x-tplg.bin",
+					    hdev->vendor_id);
+	if (!mach.tplg_filename)
+		return -ENOMEM;
+
+	id = adev->base.core.idx * HDA_MAX_CODECS + hdev->addr;
+	board = platform_device_register_data(NULL, "avs_hdaudio", id, (const void *)&mach,
+					      sizeof(mach));
+	if (IS_ERR(board)) {
+		dev_err(adev->dev, "hda board register failed\n");
+		return PTR_ERR(board);
+	}
+
+	ret = devm_add_action(adev->dev, board_pdev_unregister, board);
+	if (ret < 0) {
+		platform_device_unregister(board);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int avs_register_hda_boards(struct avs_dev *adev)
+{
+	struct hdac_bus *bus = &adev->base.core;
+	struct hdac_device *hdev;
+	int ret;
+
+	if (!bus->num_codecs) {
+		dev_dbg(adev->dev, "no HDA endpoints present\n");
+		return 0;
+	}
+
+	list_for_each_entry(hdev, &bus->codec_list, list) {
+		struct hda_codec *codec;
+
+		codec = dev_to_hda_codec(&hdev->dev);
+
+		ret = avs_register_hda_board(adev, codec);
+		if (ret < 0)
+			dev_warn(adev->dev, "register hda-%08x failed: %d\n",
+				 codec->core.vendor_id, ret);
+	}
+
+	return 0;
+}
+
+int avs_register_all_boards(struct avs_dev *adev)
+{
+	int ret;
+
+	ret = avs_register_dmic_board(adev);
+	if (ret < 0)
+		dev_warn(adev->dev, "enumerate DMIC endpoints failed: %d\n",
+			 ret);
+
+	ret = avs_register_i2s_boards(adev);
+	if (ret < 0)
+		dev_warn(adev->dev, "enumerate I2S endpoints failed: %d\n",
+			 ret);
+
+	ret = avs_register_hda_boards(adev);
+	if (ret < 0)
+		dev_warn(adev->dev, "enumerate HDA endpoints failed: %d\n",
+			 ret);
+
+	return 0;
+}
+
+void avs_unregister_all_boards(struct avs_dev *adev)
+{
+	snd_soc_unregister_component(adev->dev);
+}