[PATCH] ASoC: rt1015p: add acpi device id for rt1015p

[PATCH] ASoC: rt1015p: add acpi device id for rt1015p

[jack.yu]

From: Jack Yu <jack.yu@realtek.com>

Add acpi device id for rt1015p.

Signed-off-by: Jack Yu <jack.yu@realtek.com>
---
 sound/soc/codecs/rt1015p.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/sound/soc/codecs/rt1015p.c b/sound/soc/codecs/rt1015p.c
index 671f2a2130fe..4a00e7a4ef6f 100644
--- a/sound/soc/codecs/rt1015p.c
+++ b/sound/soc/codecs/rt1015p.c
@@ -4,6 +4,7 @@
 //
 // Copyright 2020 The Linux Foundation. All rights reserved.
 
+#include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -130,10 +131,21 @@ static const struct of_device_id rt1015p_device_id[] = {
 MODULE_DEVICE_TABLE(of, rt1015p_device_id);
 #endif
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id rt1015p_acpi_match[] = {
+	{ "RT1015", 0},
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, rt1015p_acpi_match);
+#endif
+
 static struct platform_driver rt1015p_platform_driver = {
 	.driver = {
 		.name = "rt1015p",
 		.of_match_table = of_match_ptr(rt1015p_device_id),
+#ifdef CONFIG_ACPI
+		.acpi_match_table = ACPI_PTR(rt1015p_acpi_match),
+#endif
 	},
 	.probe = rt1015p_platform_probe,
 };
-- 
2.29.0

Re: [PATCH] ASoC: rt1015p: add acpi device id for rt1015p

[Tzung-Bi Shih]

On Thu, Feb 25, 2021 at 4:47 PM <jack.yu@realtek.com> wrote:
>  static struct platform_driver rt1015p_platform_driver = {
>         .driver = {
>                 .name = "rt1015p",
>                 .of_match_table = of_match_ptr(rt1015p_device_id),
> +#ifdef CONFIG_ACPI
> +               .acpi_match_table = ACPI_PTR(rt1015p_acpi_match),
> +#endif

You don't need the guard because ACPI_PTR() returns NULL if
CONFIG_ACPI is not defined.

Re: [PATCH] ASoC: rt1015p: add acpi device id for rt1015p

[Pierre-Louis Bossart]

> +static const struct acpi_device_id rt1015p_acpi_match[] = {
> +	{ "RT1015", 0},

This is not an ACPI compliant HID, please don't do this. acpica-tools 
will flag an error on this.

And this is super confusing since you already have this in rt1015.c

static struct acpi_device_id rt1015_acpi_match[] = {
	{"10EC1015", 0,},
	{},
};

Re: [PATCH] ASoC: rt1015p: add acpi device id for rt1015p

[Mark Brown]

On Fri, 26 Feb 2021 10:05:17 +0800, jack.yu@realtek.com wrote:
> Add acpi device id for rt1015p.

Applied to

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

Thanks!

[1/1] ASoC: rt1015p: add acpi device id for rt1015p
      commit: 464b489a7885d6a89b0dfa7b7f1f56e2db80700d

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] ASoC: rt1015p: add acpi device id for rt1015p

[Pierre-Louis Bossart]

>> It seems that the options are limited. The HID needs to be made of two parts,
>> vendor ID and part ID.
>>
>> For the vendor ID, you may use the PNP ID (RTL) or the PCI ID (10EC).
>>
>> For the part ID, you are limited to 4 hex-digits. I didn't get any positive answers
>> on how to support '1015P', the only solution seems to be to create a new part
>> number, or detect at run-time which of the '1015' skews is actually used.
>>
> 
> Actually this driver is also used for rt1015 IC, the only difference is that rt1015.c is for i2c mode
> and rt1015p is for non-i2c mode of rt1015 IC, so I think it's ok to use the same part ID (different vendor ID) for two drivers.
> How do you think if we use "RTL1015" instead?

that would work indeed, e.g. 10EC1015 refers to the I2C version and 
RTL1015 to the non-I2C version. Thanks!

RE: [PATCH] ASoC: rt1015p: add acpi device id for rt1015p

[Jack Yu]

> >>> +#ifdef CONFIG_ACPI
> >>> +static const struct acpi_device_id rt1015p_acpi_match[] = {
> >>> +	{ "ALCP1015", 0},
> >>
> >> This isn't much better, ALC is not a vendor ID as defined by
> >> https://uefi.org/PNP_ACPI_Registry
> >>
> >> I've ask for guidance from Intel ACPI folks on this, please stay tuned.
> >>
> >
> > Hi Pierre,
> >
> > Is there any update from Intel ACPI team regarding to this ID format issue?
> 
> Sorry about the delay.
> 
> It seems that the options are limited. The HID needs to be made of two parts,
> vendor ID and part ID.
> 
> For the vendor ID, you may use the PNP ID (RTL) or the PCI ID (10EC).
> 
> For the part ID, you are limited to 4 hex-digits. I didn't get any positive answers
> on how to support '1015P', the only solution seems to be to create a new part
> number, or detect at run-time which of the '1015' skews is actually used.
> 

Actually this driver is also used for rt1015 IC, the only difference is that rt1015.c is for i2c mode
and rt1015p is for non-i2c mode of rt1015 IC, so I think it's ok to use the same part ID (different vendor ID) for two drivers.
How do you think if we use "RTL1015" instead?

Re: [PATCH] ASoC: rt1015p: add acpi device id for rt1015p

[Pierre-Louis Bossart]

>>> +#ifdef CONFIG_ACPI
>>> +static const struct acpi_device_id rt1015p_acpi_match[] = {
>>> +	{ "ALCP1015", 0},
>>
>> This isn't much better, ALC is not a vendor ID as defined by
>> https://uefi.org/PNP_ACPI_Registry
>>
>> I've ask for guidance from Intel ACPI folks on this, please stay tuned.
>>
> 
> Hi Pierre,
> 
> Is there any update from Intel ACPI team regarding to this ID format issue?

Sorry about the delay.

It seems that the options are limited. The HID needs to be made of two 
parts, vendor ID and part ID.

For the vendor ID, you may use the PNP ID (RTL) or the PCI ID (10EC).

For the part ID, you are limited to 4 hex-digits. I didn't get any 
positive answers on how to support '1015P', the only solution seems to 
be to create a new part number, or detect at run-time which of the 
'1015' skews is actually used.

RE: [PATCH] ASoC: rt1015p: add acpi device id for rt1015p

[Jack Yu]

> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Sent: Saturday, February 27, 2021 5:48 AM
> To: Jack Yu <jack.yu@realtek.com>; broonie@kernel.org;
> lgirdwood@gmail.com
> Cc: Oder Chiou <oder_chiou@realtek.com>; alsa-devel@alsa-project.org;
> lars@metafoo.de; kent_chen@realtek.com [陳建宏]
> <kent_chen@realtek.com>; 陳昶志 <kenny_chen@realtek.com>; Derek [方德
> 義] <derek.fang@realtek.com>; Shuming [范書銘] <shumingf@realtek.com>;
> Flove(HsinFu) <flove@realtek.com>
> Subject: Re: [PATCH] ASoC: rt1015p: add acpi device id for rt1015p
> 
> On 2/25/21 8:05 PM, jack.yu@realtek.com wrote:
> > From: Jack Yu <jack.yu@realtek.com>
> >
> > Add acpi device id for rt1015p.
> >
> > Signed-off-by: Jack Yu <jack.yu@realtek.com>
> > ---
> >   sound/soc/codecs/rt1015p.c | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> >
> > diff --git a/sound/soc/codecs/rt1015p.c b/sound/soc/codecs/rt1015p.c
> > index 671f2a2130fe..61694de080e6 100644
> > --- a/sound/soc/codecs/rt1015p.c
> > +++ b/sound/soc/codecs/rt1015p.c
> > @@ -4,6 +4,7 @@
> >   //
> >   // Copyright 2020 The Linux Foundation. All rights reserved.
> >
> > +#include <linux/acpi.h>
> >   #include <linux/delay.h>
> >   #include <linux/device.h>
> >   #include <linux/err.h>
> > @@ -130,10 +131,19 @@ static const struct of_device_id
> rt1015p_device_id[] = {
> >   MODULE_DEVICE_TABLE(of, rt1015p_device_id);
> >   #endif
> >
> > +#ifdef CONFIG_ACPI
> > +static const struct acpi_device_id rt1015p_acpi_match[] = {
> > +	{ "ALCP1015", 0},
> 
> This isn't much better, ALC is not a vendor ID as defined by
> https://uefi.org/PNP_ACPI_Registry
> 
> I've ask for guidance from Intel ACPI folks on this, please stay tuned.
> 

Hi Pierre,

Is there any update from Intel ACPI team regarding to this ID format issue?


BR,
Jack

> > +	{ },
> > +};
> > +MODULE_DEVICE_TABLE(acpi, rt1015p_acpi_match); #endif
> > +
> >   static struct platform_driver rt1015p_platform_driver = {
> >   	.driver = {
> >   		.name = "rt1015p",
> >   		.of_match_table = of_match_ptr(rt1015p_device_id),
> > +		.acpi_match_table = ACPI_PTR(rt1015p_acpi_match),
> >   	},
> >   	.probe = rt1015p_platform_probe,
> >   };
> >
> 
> ------Please consider the environment before printing this e-mail.

Re: [PATCH] ASoC: rt1015p: add acpi device id for rt1015p

[Pierre-Louis Bossart]

On 2/25/21 8:05 PM, jack.yu@realtek.com wrote:
> From: Jack Yu <jack.yu@realtek.com>
> 
> Add acpi device id for rt1015p.
> 
> Signed-off-by: Jack Yu <jack.yu@realtek.com>
> ---
>   sound/soc/codecs/rt1015p.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/sound/soc/codecs/rt1015p.c b/sound/soc/codecs/rt1015p.c
> index 671f2a2130fe..61694de080e6 100644
> --- a/sound/soc/codecs/rt1015p.c
> +++ b/sound/soc/codecs/rt1015p.c
> @@ -4,6 +4,7 @@
>   //
>   // Copyright 2020 The Linux Foundation. All rights reserved.
>   
> +#include <linux/acpi.h>
>   #include <linux/delay.h>
>   #include <linux/device.h>
>   #include <linux/err.h>
> @@ -130,10 +131,19 @@ static const struct of_device_id rt1015p_device_id[] = {
>   MODULE_DEVICE_TABLE(of, rt1015p_device_id);
>   #endif
>   
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id rt1015p_acpi_match[] = {
> +	{ "ALCP1015", 0},

This isn't much better, ALC is not a vendor ID as defined by 
https://uefi.org/PNP_ACPI_Registry

I've ask for guidance from Intel ACPI folks on this, please stay tuned.

> +	{ },
> +};
> +MODULE_DEVICE_TABLE(acpi, rt1015p_acpi_match);
> +#endif
> +
>   static struct platform_driver rt1015p_platform_driver = {
>   	.driver = {
>   		.name = "rt1015p",
>   		.of_match_table = of_match_ptr(rt1015p_device_id),
> +		.acpi_match_table = ACPI_PTR(rt1015p_acpi_match),
>   	},
>   	.probe = rt1015p_platform_probe,
>   };
> 

[PATCH] ASoC: rt1015p: add acpi device id for rt1015p

[jack.yu]

From: Jack Yu <jack.yu@realtek.com>

Add acpi device id for rt1015p.

Signed-off-by: Jack Yu <jack.yu@realtek.com>
---
 sound/soc/codecs/rt1015p.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/sound/soc/codecs/rt1015p.c b/sound/soc/codecs/rt1015p.c
index 671f2a2130fe..61694de080e6 100644
--- a/sound/soc/codecs/rt1015p.c
+++ b/sound/soc/codecs/rt1015p.c
@@ -4,6 +4,7 @@
 //
 // Copyright 2020 The Linux Foundation. All rights reserved.
 
+#include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -130,10 +131,19 @@ static const struct of_device_id rt1015p_device_id[] = {
 MODULE_DEVICE_TABLE(of, rt1015p_device_id);
 #endif
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id rt1015p_acpi_match[] = {
+	{ "ALCP1015", 0},
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, rt1015p_acpi_match);
+#endif
+
 static struct platform_driver rt1015p_platform_driver = {
 	.driver = {
 		.name = "rt1015p",
 		.of_match_table = of_match_ptr(rt1015p_device_id),
+		.acpi_match_table = ACPI_PTR(rt1015p_acpi_match),
 	},
 	.probe = rt1015p_platform_probe,
 };
-- 
2.29.0