[PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects

[PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects

[Baojun Xu]

Driver will return error if no SUBSYS_ID found in BIOS(acpi).
It will cause error in tas2563 projects, which have no SUBSYS_ID.

Signed-off-by: Baojun Xu <baojun.xu@ti.com>
---
 sound/pci/hda/tas2781_hda_i2c.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c
index 0af015806aba..0e42b87dadb8 100644
--- a/sound/pci/hda/tas2781_hda_i2c.c
+++ b/sound/pci/hda/tas2781_hda_i2c.c
@@ -142,6 +142,9 @@ static int tas2781_read_acpi(struct tasdevice_priv *p, const char *hid)
 	}
 	sub = acpi_get_subsystem_id(ACPI_HANDLE(physdev));
 	if (IS_ERR(sub)) {
+		/* No subsys id in older tas2563 projects. */
+		if (!strncmp(hid, "INT8866", sizeof("INT8866")))
+			goto end_2563;
 		dev_err(p->dev, "Failed to get SUBSYS ID.\n");
 		ret = PTR_ERR(sub);
 		goto err;
@@ -164,6 +167,7 @@ static int tas2781_read_acpi(struct tasdevice_priv *p, const char *hid)
 		p->speaker_id = NULL;
 	}
 
+end_2563:
 	acpi_dev_free_resource_list(&resources);
 	strscpy(p->dev_name, hid, sizeof(p->dev_name));
 	put_device(physdev);
-- 
2.43.0

Re: [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects

[Takashi Iwai]

On Mon, 30 Dec 2024 07:49:10 +0100,
Baojun Xu wrote:
> 
> Driver will return error if no SUBSYS_ID found in BIOS(acpi).
> It will cause error in tas2563 projects, which have no SUBSYS_ID.
> 
> Signed-off-by: Baojun Xu <baojun.xu@ti.com>

Thanks, applied now.


Takashi

Re: [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects

[Andy Shevchenko]

On Mon, Dec 30, 2024 at 02:49:10PM +0800, Baojun Xu wrote:
> Driver will return error if no SUBSYS_ID found in BIOS(acpi).
> It will cause error in tas2563 projects, which have no SUBSYS_ID.

...

> +		/* No subsys id in older tas2563 projects. */
> +		if (!strncmp(hid, "INT8866", sizeof("INT8866")))
> +			goto end_2563;

Is this ID was used on old platforms?

-- 
With Best Regards,
Andy Shevchenko

Re: [EXTERNAL] Re: [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects

[Xu, Baojun]

_> _______________________________________
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Sent: 13 January 2025 16:37
> To: Xu, Baojun
> Cc: tiwai@suse.de; robh+dt@kernel.org; lgirdwood@gmail.com; perex@perex.cz; Ding, Shenghao; Navada Kanyana, Mukund; 13916275206@139.com; Hampiholi, Vallabha; P O, Vijeth; linux-sound@vger.kernel.org; linux-kernel@vger.kernel.org; liam.r.girdwood@intel.com; yung-chuan.liao@linux.intel.com; broonie@kernel.org; antheas.dk@gmail.com; stuart.a.hayhurst@gmail.com
> Subject: [EXTERNAL] Re: [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects
> 
> On Mon, Dec 30, 2024 at 02: 49: 10PM +0800, Baojun Xu wrote: > Driver will return error if no SUBSYS_ID found in BIOS(acpi). > It will cause error in tas2563 projects, which have no SUBSYS_ID. .. . > + /* No subsys id in older tas2563
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source of this email and know the content is safe.
> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK!uBdnXZsDVuK1AUis0BxokFYl0U8msKJqQQTam8jBr5s6Aqzw9WEAd5LLFN0eVy0N20B_F7wSVDAFGD70$>
> Report Suspicious
> 
> ZjQcmQRYFpfptBannerEnd
> 
> On Mon, Dec 30, 2024 at 02:49:10PM +0800, Baojun Xu wrote:
> > Driver will return error if no SUBSYS_ID found in BIOS(acpi).
> > It will cause error in tas2563 projects, which have no SUBSYS_ID.
> 
> ...
> 
> > +             /* No subsys id in older tas2563 projects. */
> > +             if (!strncmp(hid, "INT8866", sizeof("INT8866")))
> > +                     goto end_2563;
> 
> Is this ID was used on old platforms?

Yes. And it will never be used in our product again.

> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 

[PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects

[Baojun Xu]

Driver will return error if no SUBSYS_ID found in BIOS(acpi).
It will cause error in tas2563 projects, which have no SUBSYS_ID.
Change strncmp to strcmp to avoid warning for weird length.

Signed-off-by: Baojun Xu <baojun.xu@ti.com>
---
 sound/pci/hda/tas2781_hda_i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c
index 0e42b87dadb8..61aec92f6536 100644
--- a/sound/pci/hda/tas2781_hda_i2c.c
+++ b/sound/pci/hda/tas2781_hda_i2c.c
@@ -143,7 +143,7 @@ static int tas2781_read_acpi(struct tasdevice_priv *p, const char *hid)
 	sub = acpi_get_subsystem_id(ACPI_HANDLE(physdev));
 	if (IS_ERR(sub)) {
 		/* No subsys id in older tas2563 projects. */
-		if (!strncmp(hid, "INT8866", sizeof("INT8866")))
+		if (!strcmp(hid, "INT8866"))
 			goto end_2563;
 		dev_err(p->dev, "Failed to get SUBSYS ID.\n");
 		ret = PTR_ERR(sub);
-- 
2.43.0

Re: [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects

[Takashi Iwai]

On Sat, 11 Jan 2025 10:57:28 +0100,
Baojun Xu wrote:
> 
> Driver will return error if no SUBSYS_ID found in BIOS(acpi).
> It will cause error in tas2563 projects, which have no SUBSYS_ID.
> Change strncmp to strcmp to avoid warning for weird length.

I don't understand the logic.
The use of strncmp() there already matches only with the exact string
"INT8866", not the substring, because you use sizeof("INT8866") which
is 8 including the NUL-terminator.
The only merit of strncmp() in this case is that it can be used for an
unterminated char array.

In which situation do you see the problem and how does your patch
improve / fix it?  Please give a more concrete example.


thanks,

Takashi

> 
> Signed-off-by: Baojun Xu <baojun.xu@ti.com>
> ---
>  sound/pci/hda/tas2781_hda_i2c.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c
> index 0e42b87dadb8..61aec92f6536 100644
> --- a/sound/pci/hda/tas2781_hda_i2c.c
> +++ b/sound/pci/hda/tas2781_hda_i2c.c
> @@ -143,7 +143,7 @@ static int tas2781_read_acpi(struct tasdevice_priv *p, const char *hid)
>  	sub = acpi_get_subsystem_id(ACPI_HANDLE(physdev));
>  	if (IS_ERR(sub)) {
>  		/* No subsys id in older tas2563 projects. */
> -		if (!strncmp(hid, "INT8866", sizeof("INT8866")))
> +		if (!strcmp(hid, "INT8866"))
>  			goto end_2563;
>  		dev_err(p->dev, "Failed to get SUBSYS ID.\n");
>  		ret = PTR_ERR(sub);
> -- 
> 2.43.0
> 
> 

Re: [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects

[Dan Carpenter]

On Sun, Jan 12, 2025 at 09:26:37AM +0100, Takashi Iwai wrote:
> On Sat, 11 Jan 2025 10:57:28 +0100,
> Baojun Xu wrote:
> > 
> > Driver will return error if no SUBSYS_ID found in BIOS(acpi).
> > It will cause error in tas2563 projects, which have no SUBSYS_ID.
> > Change strncmp to strcmp to avoid warning for weird length.
> 
> I don't understand the logic.
> The use of strncmp() there already matches only with the exact string
> "INT8866", not the substring, because you use sizeof("INT8866") which
> is 8 including the NUL-terminator.
> The only merit of strncmp() in this case is that it can be used for an
> unterminated char array.
> 
> In which situation do you see the problem and how does your patch
> improve / fix it?  Please give a more concrete example.
> 

I was just confused by the initial code and wondered if it was
intended to be: if (!strncmp(hid, "INT8866", sizeof("INT8866") - 1)).
This change doesn't affect anything, it's just a clarification.

But the subject "Ignore SUBSYS_ID not found for tas2563 projects" implies
that it's a change.

regards,
dan carpenter