[PATCH] IDE: palm_bk3710: use ioremap instead of arch-specific IO_ADDRESS()

[PATCH] IDE: palm_bk3710: use ioremap instead of arch-specific IO_ADDRESS()

[Kevin Hilman]

Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
---
 drivers/ide/palm_bk3710.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/ide/palm_bk3710.c b/drivers/ide/palm_bk3710.c
index f38aac7..be9c904 100644
--- a/drivers/ide/palm_bk3710.c
+++ b/drivers/ide/palm_bk3710.c
@@ -380,7 +380,11 @@ static int __init palm_bk3710_probe(struct platform_device *pdev)
 		return -EBUSY;
 	}
 
-	base = IO_ADDRESS(mem->start);
+	base = ioremap(mem->start, mem->end);
+	if (!base) {
+		printk(KERN_ERR "failed to map IO memory\n");
+		return -ENOMEM;
+	}
 
 	/* Configure the Palm Chip controller */
 	palm_bk3710_chipinit(base);
-- 
1.6.1

Re: [PATCH] IDE: palm_bk3710: use ioremap instead of arch-specific IO_ADDRESS()

[Sergei Shtylyov]

Hello.

Kevin Hilman wrote:

> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
>   

   Sorry, NAK.

> diff --git a/drivers/ide/palm_bk3710.c b/drivers/ide/palm_bk3710.c
> index f38aac7..be9c904 100644
> --- a/drivers/ide/palm_bk3710.c
> +++ b/drivers/ide/palm_bk3710.c
> @@ -380,7 +380,11 @@ static int __init palm_bk3710_probe(struct platform_device *pdev)
>  		return -EBUSY;
>  	}
>  
> -	base = IO_ADDRESS(mem->start);
> +	base = ioremap(mem->start, mem->end);
> +	if (!base) {
> +		printk(KERN_ERR "failed to map IO memory\n");
> +		return -ENOMEM;
>   

   Leaks requested memory region.

MBR, Sergei

Re: [PATCH] IDE: palm_bk3710: use ioremap instead of arch-specific IO_ADDRESS()

[Kevin Hilman]

Sergei Shtylyov <sshtylyov@ru.mvista.com> writes:

> Hello.
>
> Kevin Hilman wrote:
>
>> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
>>   
>
>   Sorry, NAK.
>
>> diff --git a/drivers/ide/palm_bk3710.c b/drivers/ide/palm_bk3710.c
>> index f38aac7..be9c904 100644
>> --- a/drivers/ide/palm_bk3710.c
>> +++ b/drivers/ide/palm_bk3710.c
>> @@ -380,7 +380,11 @@ static int __init palm_bk3710_probe(struct platform_device *pdev)
>>  		return -EBUSY;
>>  	}
>>  -	base = IO_ADDRESS(mem->start);
>> +	base = ioremap(mem->start, mem->end);
>> +	if (!base) {
>> +		printk(KERN_ERR "failed to map IO memory\n");
>> +		return -ENOMEM;
>>   
>
>   Leaks requested memory region.
>

Where would you suggest putting the iounmap?  If the driver had a
remove hook, I would've put it there.

Which brings up a bigger question, why isn't there a remove hook.  I
vaguely remember there being some problem way back in the 2.6.18 time
frame that wouldn't allow this driver to be removed.  Is that still a
problem in newer kernels?

Kevin

Re: [PATCH] IDE: palm_bk3710: use ioremap instead of arch-specific IO_ADDRESS()

[Sergei Shtylyov]

Hello.

Kevin Hilman wrote:

>> Hello.
>>
>> Kevin Hilman wrote:
>>
>>     
>>> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
>>>   
>>>       
>>   Sorry, NAK.
>>
>>     
>>> diff --git a/drivers/ide/palm_bk3710.c b/drivers/ide/palm_bk3710.c
>>> index f38aac7..be9c904 100644
>>> --- a/drivers/ide/palm_bk3710.c
>>> +++ b/drivers/ide/palm_bk3710.c
>>> @@ -380,7 +380,11 @@ static int __init palm_bk3710_probe(struct platform_device *pdev)
>>>  		return -EBUSY;
>>>  	}
>>>  -	base = IO_ADDRESS(mem->start);
>>> +	base = ioremap(mem->start, mem->end);
>>> +	if (!base) {
>>> +		printk(KERN_ERR "failed to map IO memory\n");
>>> +		return -ENOMEM;
>>>   
>>>       
>>   Leaks requested memory region.
> Where would you suggest putting the iounmap?

   You misunderstood -- you don't call release_mem_region() on the error 
path.

> Which brings up a bigger question, why isn't there a remove hook.  I
> vaguely remember there being some problem way back in the 2.6.18 time
> frame that wouldn't allow this driver to be removed.

   The IDE drivers were generally undremovable back then.

> Is that still a problem in newer kernels?
>   

   No, shouldn't be -- they've been made removable since.

> Kevin

MBR, Sergei

Re: [PATCH] IDE: palm_bk3710: use ioremap instead of arch-specific IO_ADDRESS()

[Kevin Hilman]

Sergei Shtylyov <sshtylyov@ru.mvista.com> writes:

> Hello.
>
> Kevin Hilman wrote:
>
>>> Hello.
>>>
>>> Kevin Hilman wrote:
>>>
>>>     
>>>> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
>>>>         
>>>   Sorry, NAK.
>>>
>>>     
>>>> diff --git a/drivers/ide/palm_bk3710.c b/drivers/ide/palm_bk3710.c
>>>> index f38aac7..be9c904 100644
>>>> --- a/drivers/ide/palm_bk3710.c
>>>> +++ b/drivers/ide/palm_bk3710.c
>>>> @@ -380,7 +380,11 @@ static int __init palm_bk3710_probe(struct platform_device *pdev)
>>>>  		return -EBUSY;
>>>>  	}
>>>>  -	base = IO_ADDRESS(mem->start);
>>>> +	base = ioremap(mem->start, mem->end);
>>>> +	if (!base) {
>>>> +		printk(KERN_ERR "failed to map IO memory\n");
>>>> +		return -ENOMEM;
>>>>         
>>>   Leaks requested memory region.
>> Where would you suggest putting the iounmap?
>
>   You misunderstood -- you don't call release_mem_region() on the
> error path.

Ah, I see.  Good catch.  I also noticed ioremap() takes 'size' and not
'end' so that needs to be fixed too.  Will resend.

Kevin

>> Which brings up a bigger question, why isn't there a remove hook.  I
>> vaguely remember there being some problem way back in the 2.6.18 time
>> frame that wouldn't allow this driver to be removed.
>
>   The IDE drivers were generally undremovable back then.
>
>> Is that still a problem in newer kernels?
>>   
>
>   No, shouldn't be -- they've been made removable since.
>
>> Kevin
>
> MBR, Sergei