* [PATCH] sound: don't use the deprecated function check_region
@ 2011-08-08 8:07 stufever
2011-08-08 8:22 ` Takashi Iwai
0 siblings, 1 reply; 8+ messages in thread
From: stufever @ 2011-08-08 8:07 UTC (permalink / raw)
To: linux-kernel; +Cc: tiwai, perex, Wang Shaoyan
From: Wang Shaoyan <wangshaoyan.pt@taobao.com>
sound/oss/pss.c: In function 'configure_nonsound_components':
sound/oss/pss.c:676: warning: 'check_region' is deprecated (declared at include/linux/ioport.h:201)
Signed-off-by: Wang Shaoyan <wangshaoyan.pt@taobao.com>
---
sound/oss/pss.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/sound/oss/pss.c b/sound/oss/pss.c
index 9b800ce..0fa26bd 100644
--- a/sound/oss/pss.c
+++ b/sound/oss/pss.c
@@ -673,9 +673,10 @@ static void configure_nonsound_components(void)
if (pss_cdrom_port == -1) { /* If cdrom port enablation wasn't requested */
printk(KERN_INFO "PSS: CDROM port not enabled.\n");
- } else if (check_region(pss_cdrom_port, 2)) {
+ } else if (!request_region(pss_cdrom_port, 2, "PSS CDROM")) {
printk(KERN_ERR "PSS: CDROM I/O port conflict.\n");
} else {
+ release_region(pss_cdrom_port, 2);
set_io_base(devc, CONF_CDROM, pss_cdrom_port);
printk(KERN_INFO "PSS: CDROM I/O port set to 0x%x.\n", pss_cdrom_port);
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] sound: don't use the deprecated function check_region
2011-08-08 8:07 [PATCH] sound: don't use the deprecated function check_region stufever
@ 2011-08-08 8:22 ` Takashi Iwai
2011-08-08 9:33 ` Wang Shaoyan
0 siblings, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2011-08-08 8:22 UTC (permalink / raw)
To: stufever; +Cc: linux-kernel, perex, Wang Shaoyan
At Mon, 8 Aug 2011 16:07:12 +0800,
stufever@gmail.com wrote:
>
> From: Wang Shaoyan <wangshaoyan.pt@taobao.com>
>
> sound/oss/pss.c: In function 'configure_nonsound_components':
> sound/oss/pss.c:676: warning: 'check_region' is deprecated (declared at include/linux/ioport.h:201)
>
> Signed-off-by: Wang Shaoyan <wangshaoyan.pt@taobao.com>
> ---
> sound/oss/pss.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/sound/oss/pss.c b/sound/oss/pss.c
> index 9b800ce..0fa26bd 100644
> --- a/sound/oss/pss.c
> +++ b/sound/oss/pss.c
> @@ -673,9 +673,10 @@ static void configure_nonsound_components(void)
>
> if (pss_cdrom_port == -1) { /* If cdrom port enablation wasn't requested */
> printk(KERN_INFO "PSS: CDROM port not enabled.\n");
> - } else if (check_region(pss_cdrom_port, 2)) {
> + } else if (!request_region(pss_cdrom_port, 2, "PSS CDROM")) {
> printk(KERN_ERR "PSS: CDROM I/O port conflict.\n");
> } else {
> + release_region(pss_cdrom_port, 2);
> set_io_base(devc, CONF_CDROM, pss_cdrom_port);
> printk(KERN_INFO "PSS: CDROM I/O port set to 0x%x.\n", pss_cdrom_port);
> }
Well, this just hides the warning but doesn't fix the possible race,
which is the reason check_region() is marked as deprecated after all.
Instead of releasing the resource there, keep it and release in the
destructor properly like other normal resources.
thanks,
Takashi
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sound: don't use the deprecated function check_region
2011-08-08 8:22 ` Takashi Iwai
@ 2011-08-08 9:33 ` Wang Shaoyan
2011-08-08 9:43 ` Takashi Iwai
0 siblings, 1 reply; 8+ messages in thread
From: Wang Shaoyan @ 2011-08-08 9:33 UTC (permalink / raw)
To: Takashi Iwai; +Cc: linux-kernel, perex, Wang Shaoyan
2011/8/8 Takashi Iwai <tiwai@suse.de>:
> At Mon, 8 Aug 2011 16:07:12 +0800,
> stufever@gmail.com wrote:
>>
>> From: Wang Shaoyan <wangshaoyan.pt@taobao.com>
>>
>> sound/oss/pss.c: In function 'configure_nonsound_components':
>> sound/oss/pss.c:676: warning: 'check_region' is deprecated (declared at include/linux/ioport.h:201)
>>
>> Signed-off-by: Wang Shaoyan <wangshaoyan.pt@taobao.com>
>> ---
>> sound/oss/pss.c | 3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/sound/oss/pss.c b/sound/oss/pss.c
>> index 9b800ce..0fa26bd 100644
>> --- a/sound/oss/pss.c
>> +++ b/sound/oss/pss.c
>> @@ -673,9 +673,10 @@ static void configure_nonsound_components(void)
>>
>> if (pss_cdrom_port == -1) { /* If cdrom port enablation wasn't requested */
>> printk(KERN_INFO "PSS: CDROM port not enabled.\n");
>> - } else if (check_region(pss_cdrom_port, 2)) {
>> + } else if (!request_region(pss_cdrom_port, 2, "PSS CDROM")) {
>> printk(KERN_ERR "PSS: CDROM I/O port conflict.\n");
>> } else {
>> + release_region(pss_cdrom_port, 2);
>> set_io_base(devc, CONF_CDROM, pss_cdrom_port);
>> printk(KERN_INFO "PSS: CDROM I/O port set to 0x%x.\n", pss_cdrom_port);
>> }
>
> Well, this just hides the warning but doesn't fix the possible race,
> which is the reason check_region() is marked as deprecated after all.
>
AFAIK, check_region() is deprecated because such usage:
check_region()
...do something <-- here another process may do check and request same region
request_region()
But nowadays, request_region has done the check internally, so use the
proper request_region to check is enough, am I miss something?
> Instead of releasing the resource there, keep it and release in the
> destructor properly like other normal resources.
>
maybe we can release here?
set_io_base(devc, CONF_CDROM, pss_cdrom_port);
printk(KERN_INFO "PSS: CDROM I/O port set to 0x%x.\n", pss_cdrom_port);
release_region(pss_cdrom_port, 2);
>
> thanks,
>
> Takashi
>
--
Wang Shaoyan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sound: don't use the deprecated function check_region
2011-08-08 9:33 ` Wang Shaoyan
@ 2011-08-08 9:43 ` Takashi Iwai
0 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2011-08-08 9:43 UTC (permalink / raw)
To: Wang Shaoyan; +Cc: linux-kernel, perex, Wang Shaoyan
At Mon, 8 Aug 2011 17:33:40 +0800,
Wang Shaoyan wrote:
>
> 2011/8/8 Takashi Iwai <tiwai@suse.de>:
> > At Mon, 8 Aug 2011 16:07:12 +0800,
> > stufever@gmail.com wrote:
> >>
> >> From: Wang Shaoyan <wangshaoyan.pt@taobao.com>
> >>
> >> sound/oss/pss.c: In function 'configure_nonsound_components':
> >> sound/oss/pss.c:676: warning: 'check_region' is deprecated (declared at include/linux/ioport.h:201)
> >>
> >> Signed-off-by: Wang Shaoyan <wangshaoyan.pt@taobao.com>
> >> ---
> >> sound/oss/pss.c | 3 ++-
> >> 1 files changed, 2 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/sound/oss/pss.c b/sound/oss/pss.c
> >> index 9b800ce..0fa26bd 100644
> >> --- a/sound/oss/pss.c
> >> +++ b/sound/oss/pss.c
> >> @@ -673,9 +673,10 @@ static void configure_nonsound_components(void)
> >>
> >> if (pss_cdrom_port == -1) { /* If cdrom port enablation wasn't requested */
> >> printk(KERN_INFO "PSS: CDROM port not enabled.\n");
> >> - } else if (check_region(pss_cdrom_port, 2)) {
> >> + } else if (!request_region(pss_cdrom_port, 2, "PSS CDROM")) {
> >> printk(KERN_ERR "PSS: CDROM I/O port conflict.\n");
> >> } else {
> >> + release_region(pss_cdrom_port, 2);
> >> set_io_base(devc, CONF_CDROM, pss_cdrom_port);
> >> printk(KERN_INFO "PSS: CDROM I/O port set to 0x%x.\n", pss_cdrom_port);
> >> }
> >
> > Well, this just hides the warning but doesn't fix the possible race,
> > which is the reason check_region() is marked as deprecated after all.
> >
>
> AFAIK, check_region() is deprecated because such usage:
> check_region()
> ...do something <-- here another process may do check and request same region
> request_region()
> But nowadays, request_region has done the check internally, so use the
> proper request_region to check is enough, am I miss something?
Right, if it uses request_region() and keeps the resource until the
end of the driver life, it's fine.
> > Instead of releasing the resource there, keep it and release in the
> > destructor properly like other normal resources.
> >
>
> maybe we can release here?
>
> set_io_base(devc, CONF_CDROM, pss_cdrom_port);
> printk(KERN_INFO "PSS: CDROM I/O port set to 0x%x.\n", pss_cdrom_port);
> release_region(pss_cdrom_port, 2);
No. What happens if another driver takes this resource and starts
accessing? It conflicts. The driver must keep the resource by itself
to protect from others as long as it uses.
thanks,
Takashi
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sound: don't use the deprecated function check_region
2011-08-08 11:10 stufever
@ 2011-08-08 12:33 ` Takashi Iwai
0 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2011-08-08 12:33 UTC (permalink / raw)
To: stufever; +Cc: linux-kernel, perex, Wang Shaoyan
At Mon, 8 Aug 2011 19:10:26 +0800,
stufever@gmail.com wrote:
>
> From: Wang Shaoyan <wangshaoyan.pt@taobao.com>
>
> sound/oss/pss.c: In function 'configure_nonsound_components':
> sound/oss/pss.c:676: warning: 'check_region' is deprecated (declared at include/linux/ioport.h:201)
>
> Signed-off-by: Wang Shaoyan <wangshaoyan.pt@taobao.com>
Applied now to sound git tree. Thanks.
Takashi
> ---
> sound/oss/pss.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/sound/oss/pss.c b/sound/oss/pss.c
> index 9b800ce..2fc0624 100644
> --- a/sound/oss/pss.c
> +++ b/sound/oss/pss.c
> @@ -673,7 +673,8 @@ static void configure_nonsound_components(void)
>
> if (pss_cdrom_port == -1) { /* If cdrom port enablation wasn't requested */
> printk(KERN_INFO "PSS: CDROM port not enabled.\n");
> - } else if (check_region(pss_cdrom_port, 2)) {
> + } else if (!request_region(pss_cdrom_port, 2, "PSS CDROM")) {
> + pss_cdrom_port = -1;
> printk(KERN_ERR "PSS: CDROM I/O port conflict.\n");
> } else {
> set_io_base(devc, CONF_CDROM, pss_cdrom_port);
> @@ -1232,7 +1233,8 @@ static void __exit cleanup_pss(void)
> if(pssmpu)
> unload_pss_mpu(&cfg_mpu);
> unload_pss(&cfg);
> - }
> + } else if (pss_cdrom_port != -1)
> + release_region(pss_cdrom_port, 2);
>
> if(!pss_keep_settings) /* Keep hardware settings if asked */
> {
> --
> 1.7.4.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] sound: don't use the deprecated function check_region
@ 2011-08-08 11:10 stufever
2011-08-08 12:33 ` Takashi Iwai
0 siblings, 1 reply; 8+ messages in thread
From: stufever @ 2011-08-08 11:10 UTC (permalink / raw)
To: linux-kernel; +Cc: tiwai, perex, Wang Shaoyan
From: Wang Shaoyan <wangshaoyan.pt@taobao.com>
sound/oss/pss.c: In function 'configure_nonsound_components':
sound/oss/pss.c:676: warning: 'check_region' is deprecated (declared at include/linux/ioport.h:201)
Signed-off-by: Wang Shaoyan <wangshaoyan.pt@taobao.com>
---
sound/oss/pss.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/sound/oss/pss.c b/sound/oss/pss.c
index 9b800ce..2fc0624 100644
--- a/sound/oss/pss.c
+++ b/sound/oss/pss.c
@@ -673,7 +673,8 @@ static void configure_nonsound_components(void)
if (pss_cdrom_port == -1) { /* If cdrom port enablation wasn't requested */
printk(KERN_INFO "PSS: CDROM port not enabled.\n");
- } else if (check_region(pss_cdrom_port, 2)) {
+ } else if (!request_region(pss_cdrom_port, 2, "PSS CDROM")) {
+ pss_cdrom_port = -1;
printk(KERN_ERR "PSS: CDROM I/O port conflict.\n");
} else {
set_io_base(devc, CONF_CDROM, pss_cdrom_port);
@@ -1232,7 +1233,8 @@ static void __exit cleanup_pss(void)
if(pssmpu)
unload_pss_mpu(&cfg_mpu);
unload_pss(&cfg);
- }
+ } else if (pss_cdrom_port != -1)
+ release_region(pss_cdrom_port, 2);
if(!pss_keep_settings) /* Keep hardware settings if asked */
{
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] sound: don't use the deprecated function check_region
2011-08-08 10:43 stufever
@ 2011-08-08 10:47 ` Takashi Iwai
0 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2011-08-08 10:47 UTC (permalink / raw)
To: stufever; +Cc: linux-kernel, perex, Wang Shaoyan
At Mon, 8 Aug 2011 18:43:15 +0800,
stufever@gmail.com wrote:
>
> From: Wang Shaoyan <wangshaoyan.pt@taobao.com>
>
> sound/oss/pss.c: In function 'configure_nonsound_components':
> sound/oss/pss.c:676: warning: 'check_region' is deprecated (declared at include/linux/ioport.h:201)
>
> Signed-off-by: Wang Shaoyan <wangshaoyan.pt@taobao.com>
Thanks, it's getting better, but a still bit things to fix...
> ---
> sound/oss/pss.c | 11 +++++------
> 1 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/sound/oss/pss.c b/sound/oss/pss.c
> index 9b800ce..55bfea6 100644
> --- a/sound/oss/pss.c
> +++ b/sound/oss/pss.c
> @@ -673,7 +673,7 @@ static void configure_nonsound_components(void)
>
> if (pss_cdrom_port == -1) { /* If cdrom port enablation wasn't requested */
> printk(KERN_INFO "PSS: CDROM port not enabled.\n");
> - } else if (check_region(pss_cdrom_port, 2)) {
> + } else if (!request_region(pss_cdrom_port, 2, "PSS CDROM")) {
> printk(KERN_ERR "PSS: CDROM I/O port conflict.\n");
In the error path, set pss_cdrom_port to -1 so that it won't be
released wrongly later.
> } else {
> set_io_base(devc, CONF_CDROM, pss_cdrom_port);
> @@ -1223,8 +1223,7 @@ static int __init init_pss(void)
>
> static void __exit cleanup_pss(void)
> {
> - if(!pss_no_sound)
> - {
> + if (!pss_no_sound) {
> if(fw_load && pss_synth)
> vfree(pss_synth);
> if(pssmss)
Not necessary to fix the coding style in the same patch.
> @@ -1232,10 +1231,10 @@ static void __exit cleanup_pss(void)
> if(pssmpu)
> unload_pss_mpu(&cfg_mpu);
> unload_pss(&cfg);
> - }
> + } else
> + release_region(pss_cdrom_port, 2);
Call this only when pss_cdrom_port has a valid value.
thanks,
Takashi
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] sound: don't use the deprecated function check_region
@ 2011-08-08 10:43 stufever
2011-08-08 10:47 ` Takashi Iwai
0 siblings, 1 reply; 8+ messages in thread
From: stufever @ 2011-08-08 10:43 UTC (permalink / raw)
To: linux-kernel; +Cc: tiwai, perex, Wang Shaoyan
From: Wang Shaoyan <wangshaoyan.pt@taobao.com>
sound/oss/pss.c: In function 'configure_nonsound_components':
sound/oss/pss.c:676: warning: 'check_region' is deprecated (declared at include/linux/ioport.h:201)
Signed-off-by: Wang Shaoyan <wangshaoyan.pt@taobao.com>
---
sound/oss/pss.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/sound/oss/pss.c b/sound/oss/pss.c
index 9b800ce..55bfea6 100644
--- a/sound/oss/pss.c
+++ b/sound/oss/pss.c
@@ -673,7 +673,7 @@ static void configure_nonsound_components(void)
if (pss_cdrom_port == -1) { /* If cdrom port enablation wasn't requested */
printk(KERN_INFO "PSS: CDROM port not enabled.\n");
- } else if (check_region(pss_cdrom_port, 2)) {
+ } else if (!request_region(pss_cdrom_port, 2, "PSS CDROM")) {
printk(KERN_ERR "PSS: CDROM I/O port conflict.\n");
} else {
set_io_base(devc, CONF_CDROM, pss_cdrom_port);
@@ -1223,8 +1223,7 @@ static int __init init_pss(void)
static void __exit cleanup_pss(void)
{
- if(!pss_no_sound)
- {
+ if (!pss_no_sound) {
if(fw_load && pss_synth)
vfree(pss_synth);
if(pssmss)
@@ -1232,10 +1231,10 @@ static void __exit cleanup_pss(void)
if(pssmpu)
unload_pss_mpu(&cfg_mpu);
unload_pss(&cfg);
- }
+ } else
+ release_region(pss_cdrom_port, 2);
- if(!pss_keep_settings) /* Keep hardware settings if asked */
- {
+ if (!pss_keep_settings) { /* Keep hardware settings if asked */
disable_all_emulations();
printk(KERN_INFO "Resetting PSS sound card configurations.\n");
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-08-08 12:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-08 8:07 [PATCH] sound: don't use the deprecated function check_region stufever
2011-08-08 8:22 ` Takashi Iwai
2011-08-08 9:33 ` Wang Shaoyan
2011-08-08 9:43 ` Takashi Iwai
2011-08-08 10:43 stufever
2011-08-08 10:47 ` Takashi Iwai
2011-08-08 11:10 stufever
2011-08-08 12:33 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).