* [PATCH 2/3] staging: dgap: adds missing iounmap for re_map_port in dgap_release_remap()
@ 2014-09-23 0:22 Daeseok Youn
2014-09-24 9:36 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Daeseok Youn @ 2014-09-23 0:22 UTC (permalink / raw)
To: lidza.louina, markh
Cc: markh, daeseok.youn, gregkh, driverdev-devel, devel, linux-kernel
Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
---
drivers/staging/dgap/dgap.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index 779d144..637ea8a 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -1026,6 +1026,7 @@ static void dgap_release_remap(struct board_t *brd)
release_mem_region(brd->membase, 0x200000);
release_mem_region(brd->membase + PCI_IO_OFFSET, 0x200000);
iounmap(brd->re_map_membase);
+ iounmap(brd->re_map_port);
}
/*****************************************************************************
*
--
1.7.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/3] staging: dgap: adds missing iounmap for re_map_port in dgap_release_remap()
2014-09-23 0:22 [PATCH 2/3] staging: dgap: adds missing iounmap for re_map_port in dgap_release_remap() Daeseok Youn
@ 2014-09-24 9:36 ` Dan Carpenter
2014-09-24 9:42 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2014-09-24 9:36 UTC (permalink / raw)
To: Daeseok Youn
Cc: lidza.louina, markh, devel, gregkh, driverdev-devel, linux-kernel
On Tue, Sep 23, 2014 at 09:22:08AM +0900, Daeseok Youn wrote:
> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> ---
> drivers/staging/dgap/dgap.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
> index 779d144..637ea8a 100644
> --- a/drivers/staging/dgap/dgap.c
> +++ b/drivers/staging/dgap/dgap.c
> @@ -1026,6 +1026,7 @@ static void dgap_release_remap(struct board_t *brd)
> release_mem_region(brd->membase, 0x200000);
> release_mem_region(brd->membase + PCI_IO_OFFSET, 0x200000);
> iounmap(brd->re_map_membase);
> + iounmap(brd->re_map_port);
I don't think it matters, but at least aesthetically it would be better
to unmap first and then release the regions. (You can do this in a
different patch).
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/3] staging: dgap: adds missing iounmap for re_map_port in dgap_release_remap()
2014-09-24 9:36 ` Dan Carpenter
@ 2014-09-24 9:42 ` Dan Carpenter
2014-09-25 0:21 ` DaeSeok Youn
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2014-09-24 9:42 UTC (permalink / raw)
To: Daeseok Youn; +Cc: devel, lidza.louina, driverdev-devel, linux-kernel, gregkh
On Wed, Sep 24, 2014 at 12:36:48PM +0300, Dan Carpenter wrote:
> On Tue, Sep 23, 2014 at 09:22:08AM +0900, Daeseok Youn wrote:
> > Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> > ---
> > drivers/staging/dgap/dgap.c | 1 +
> > 1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
> > index 779d144..637ea8a 100644
> > --- a/drivers/staging/dgap/dgap.c
> > +++ b/drivers/staging/dgap/dgap.c
> > @@ -1026,6 +1026,7 @@ static void dgap_release_remap(struct board_t *brd)
> > release_mem_region(brd->membase, 0x200000);
> > release_mem_region(brd->membase + PCI_IO_OFFSET, 0x200000);
> > iounmap(brd->re_map_membase);
> > + iounmap(brd->re_map_port);
>
>
> I don't think it matters, but at least aesthetically it would be better
> to unmap first and then release the regions. (You can do this in a
> different patch).
>
Also dgap_do_remap() unwinds in the wrong order and is sucky. It should
use gotos.
Also the name dgap_do_remap() isn't great. Remap is a verb not a noun
so the "_do" isn't needed. Just dgap_remap() and dgap_release_remap()
could be changed to dgap_unmap().
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/3] staging: dgap: adds missing iounmap for re_map_port in dgap_release_remap()
2014-09-24 9:42 ` Dan Carpenter
@ 2014-09-25 0:21 ` DaeSeok Youn
0 siblings, 0 replies; 4+ messages in thread
From: DaeSeok Youn @ 2014-09-25 0:21 UTC (permalink / raw)
To: Dan Carpenter; +Cc: devel, Lidza Louina, driverdev-devel, linux-kernel, Greg KH
Hi,
2014-09-24 18:42 GMT+09:00 Dan Carpenter <dan.carpenter@oracle.com>:
> On Wed, Sep 24, 2014 at 12:36:48PM +0300, Dan Carpenter wrote:
>> On Tue, Sep 23, 2014 at 09:22:08AM +0900, Daeseok Youn wrote:
>> > Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
>> > ---
>> > drivers/staging/dgap/dgap.c | 1 +
>> > 1 files changed, 1 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
>> > index 779d144..637ea8a 100644
>> > --- a/drivers/staging/dgap/dgap.c
>> > +++ b/drivers/staging/dgap/dgap.c
>> > @@ -1026,6 +1026,7 @@ static void dgap_release_remap(struct board_t *brd)
>> > release_mem_region(brd->membase, 0x200000);
>> > release_mem_region(brd->membase + PCI_IO_OFFSET, 0x200000);
>> > iounmap(brd->re_map_membase);
>> > + iounmap(brd->re_map_port);
>>
>>
>> I don't think it matters, but at least aesthetically it would be better
>> to unmap first and then release the regions. (You can do this in a
>> different patch).
Ok. I will make another patch as your comment.
>>
>
> Also dgap_do_remap() unwinds in the wrong order and is sucky. It should
> use gotos.
>
> Also the name dgap_do_remap() isn't great. Remap is a verb not a noun
> so the "_do" isn't needed. Just dgap_remap() and dgap_release_remap()
> could be changed to dgap_unmap().
OK. rename those functions.
Thanks.
regards,
Daeseok Youn.
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-09-25 0:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-23 0:22 [PATCH 2/3] staging: dgap: adds missing iounmap for re_map_port in dgap_release_remap() Daeseok Youn
2014-09-24 9:36 ` Dan Carpenter
2014-09-24 9:42 ` Dan Carpenter
2014-09-25 0:21 ` DaeSeok Youn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox