* [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects
@ 2025-01-11 9:57 Baojun Xu
2025-01-12 8:26 ` Takashi Iwai
0 siblings, 1 reply; 6+ messages in thread
From: Baojun Xu @ 2025-01-11 9:57 UTC (permalink / raw)
To: tiwai
Cc: robh+dt, andriy.shevchenko, lgirdwood, perex, shenghao-ding,
navada, 13916275206, v-hampiholi, v-po, linux-sound, linux-kernel,
liam.r.girdwood, yung-chuan.liao, baojun.xu, broonie, antheas.dk,
stuart.a.hayhurst, dan.carpenter
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects
2025-01-11 9:57 [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects Baojun Xu
@ 2025-01-12 8:26 ` Takashi Iwai
2025-01-13 5:33 ` Dan Carpenter
2025-01-13 6:41 ` [EXTERNAL] " Xu, Baojun
0 siblings, 2 replies; 6+ messages in thread
From: Takashi Iwai @ 2025-01-12 8:26 UTC (permalink / raw)
To: Baojun Xu
Cc: tiwai, robh+dt, andriy.shevchenko, lgirdwood, perex,
shenghao-ding, navada, 13916275206, v-hampiholi, v-po,
linux-sound, linux-kernel, liam.r.girdwood, yung-chuan.liao,
broonie, antheas.dk, stuart.a.hayhurst, dan.carpenter
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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects
2025-01-12 8:26 ` Takashi Iwai
@ 2025-01-13 5:33 ` Dan Carpenter
2025-01-13 6:41 ` [EXTERNAL] " Xu, Baojun
1 sibling, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2025-01-13 5:33 UTC (permalink / raw)
To: Takashi Iwai
Cc: Baojun Xu, robh+dt, andriy.shevchenko, lgirdwood, perex,
shenghao-ding, navada, 13916275206, v-hampiholi, v-po,
linux-sound, linux-kernel, liam.r.girdwood, yung-chuan.liao,
broonie, antheas.dk, stuart.a.hayhurst
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [EXTERNAL] Re: [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects
2025-01-12 8:26 ` Takashi Iwai
2025-01-13 5:33 ` Dan Carpenter
@ 2025-01-13 6:41 ` Xu, Baojun
2025-01-13 7:47 ` Takashi Iwai
1 sibling, 1 reply; 6+ messages in thread
From: Xu, Baojun @ 2025-01-13 6:41 UTC (permalink / raw)
To: Takashi Iwai
Cc: robh+dt@kernel.org, andriy.shevchenko@linux.intel.com,
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,
dan.carpenter@linaro.org
> ________________________________________
> From: Takashi Iwai <tiwai@suse.de>
> Sent: 12 January 2025 16:26
> To: Xu, Baojun
> Cc: tiwai@suse.de; robh+dt@kernel.org; andriy.shevchenko@linux.intel.com; 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; dan.carpenter@linaro.org
> Subject: [EXTERNAL] Re: [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects
>
> 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
> 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!vxdrHfwF_qfzogYE_xtoX7lTozH70XOKA0HRu-ioBvUvCv8QNkmAsR1WurvR9Zlh7GmU7A$>
> Report Suspicious
>
> ZjQcmQRYFpfptBannerEnd
>
> 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.
>
Because it will have Smatch Static Checker warning:
sound/pci/hda/tas2781_hda_i2c.c:146 tas2781_read_acpi()
warn: strncmp() with weird length: 7 vs 8
sound/pci/hda/tas2781_hda_i2c.c
144 if (IS_ERR(sub)) {
145 /* No subsys id in older tas2563 projects. */
--> 146 if (!strncmp(hid, "INT8866", sizeof("INT8866")))
This patch will avoid this warning, and this hid will be a null terminated
"INT8866", so have no different when this code was go through.
>
> 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
> >
> >
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [EXTERNAL] Re: [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects
2025-01-13 6:41 ` [EXTERNAL] " Xu, Baojun
@ 2025-01-13 7:47 ` Takashi Iwai
0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2025-01-13 7:47 UTC (permalink / raw)
To: Xu, Baojun
Cc: Takashi Iwai, robh+dt@kernel.org,
andriy.shevchenko@linux.intel.com, 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, dan.carpenter@linaro.org
On Mon, 13 Jan 2025 07:41:02 +0100,
Xu, Baojun wrote:
>
> > ________________________________________
> > From: Takashi Iwai <tiwai@suse.de>
> > Sent: 12 January 2025 16:26
> > To: Xu, Baojun
> > Cc: tiwai@suse.de; robh+dt@kernel.org; andriy.shevchenko@linux.intel.com; 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; dan.carpenter@linaro.org
> > Subject: [EXTERNAL] Re: [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects
> >
> > 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
> > 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!vxdrHfwF_qfzogYE_xtoX7lTozH70XOKA0HRu-ioBvUvCv8QNkmAsR1WurvR9Zlh7GmU7A$>
> > Report Suspicious
> >
> > ZjQcmQRYFpfptBannerEnd
> >
> > 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.
> >
>
> Because it will have Smatch Static Checker warning:
>
> sound/pci/hda/tas2781_hda_i2c.c:146 tas2781_read_acpi()
> warn: strncmp() with weird length: 7 vs 8
>
> sound/pci/hda/tas2781_hda_i2c.c
> 144 if (IS_ERR(sub)) {
> 145 /* No subsys id in older tas2563 projects. */
> --> 146 if (!strncmp(hid, "INT8866", sizeof("INT8866")))
>
> This patch will avoid this warning, and this hid will be a null terminated
> "INT8866", so have no different when this code was go through.
Ah, so your change is only about the last line in the sentence.
The former two lines describe what the current driver already does.
As the driver behavior won't change, your current patch subject and
description are way too confusing. Please rephrase them and
resubmit. It's nothing but just a code refactoring that eventually
shuts up the Smatch warning, after all.
thanks,
Takashi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [EXTERNAL] Re: [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects
2025-01-13 8:37 ` Andy Shevchenko
@ 2025-01-13 8:45 ` Xu, Baojun
0 siblings, 0 replies; 6+ messages in thread
From: Xu, Baojun @ 2025-01-13 8:45 UTC (permalink / raw)
To: Andy Shevchenko
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
_> _______________________________________
> 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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-01-13 8:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-11 9:57 [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects Baojun Xu
2025-01-12 8:26 ` Takashi Iwai
2025-01-13 5:33 ` Dan Carpenter
2025-01-13 6:41 ` [EXTERNAL] " Xu, Baojun
2025-01-13 7:47 ` Takashi Iwai
-- strict thread matches above, loose matches on Subject: below --
2024-12-30 6:49 Baojun Xu
2025-01-13 8:37 ` Andy Shevchenko
2025-01-13 8:45 ` [EXTERNAL] " Xu, Baojun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox