2006-09-30 22:34:22

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 3/4] Char: mxser_new, pci_request_region for pci regions

mxser_new, pci_request_region for pci regions

Use pci_request_region instead of standard request_region for pci device
regions. More checking, simplier use.

Signed-off-by: Jiri Slaby <[email protected]>

---
commit 1a717bdb06cef859dfbd426f46ea24a9c740e5c5
tree 85460f01008e9fa2edea675a73b394c48139df4a
parent d4f99406c592fb7ce2a65645d7c1f98ebe599238
author Jiri Slaby <[email protected]> Sat, 30 Sep 2006 01:20:12 +0200
committer Jiri Slaby <[email protected]> Sat, 30 Sep 2006 01:20:12 +0200

drivers/char/mxser_new.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/char/mxser_new.c b/drivers/char/mxser_new.c
index dfef9ce..c566cd0 100644
--- a/drivers/char/mxser_new.c
+++ b/drivers/char/mxser_new.c
@@ -526,8 +526,8 @@ static void __exit mxser_module_exit(voi
pdev = mxser_boards[i].pdev;
free_irq(mxser_boards[i].irq, &mxser_boards[i]);
if (pdev != NULL) { /* PCI */
- release_region(pci_resource_start(pdev, 2), pci_resource_len(pdev, 2));
- release_region(pci_resource_start(pdev, 3), pci_resource_len(pdev, 3));
+ pci_release_region(pdev, 2);
+ pci_release_region(pdev, 3);
pci_dev_put(pdev);
} else {
release_region(mxser_boards[i].ports[0].ioaddr, 8 * mxser_boards[i].nports);
@@ -627,16 +627,14 @@ static int __init mxser_get_PCI_conf(int
brd->board_type = board_type;
brd->nports = mxser_numports[board_type - 1];
ioaddress = pci_resource_start(pdev, 2);
- request_region(pci_resource_start(pdev, 2), pci_resource_len(pdev, 2),
- "mxser(IO)");
+ pci_request_region(pdev, 2, "mxser(IO)");

for (i = 0; i < brd->nports; i++)
brd->ports[i].ioaddr = ioaddress + 8 * i;

/* vector */
ioaddress = pci_resource_start(pdev, 3);
- request_region(pci_resource_start(pdev, 3), pci_resource_len(pdev, 3),
- "mxser(vector)");
+ pci_request_region(pdev, 3, "mxser(vector)");
brd->vector = ioaddress;

/* irq */


2006-10-04 13:44:31

by Rolf Eike Beer

[permalink] [raw]
Subject: Re: [PATCH 3/4] Char: mxser_new, pci_request_region for pci regions

Jiri Slaby wrote:
> mxser_new, pci_request_region for pci regions
>
> Use pci_request_region instead of standard request_region for pci device
> regions. More checking, simplier use.
>
> Signed-off-by: Jiri Slaby <[email protected]>
>
> ---
> commit 1a717bdb06cef859dfbd426f46ea24a9c740e5c5
> tree 85460f01008e9fa2edea675a73b394c48139df4a
> parent d4f99406c592fb7ce2a65645d7c1f98ebe599238
> author Jiri Slaby <[email protected]> Sat, 30 Sep 2006 01:20:12 +0200
> committer Jiri Slaby <[email protected]> Sat, 30 Sep 2006 01:20:12
> +0200
>
> drivers/char/mxser_new.c | 10 ++++------
> 1 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/char/mxser_new.c b/drivers/char/mxser_new.c
> index dfef9ce..c566cd0 100644
> --- a/drivers/char/mxser_new.c
> +++ b/drivers/char/mxser_new.c
> @@ -526,8 +526,8 @@ static void __exit mxser_module_exit(voi
> pdev = mxser_boards[i].pdev;
> free_irq(mxser_boards[i].irq, &mxser_boards[i]);
> if (pdev != NULL) { /* PCI */
> - release_region(pci_resource_start(pdev, 2), pci_resource_len(pdev,
> 2)); - release_region(pci_resource_start(pdev, 3),
> pci_resource_len(pdev, 3)); + pci_release_region(pdev, 2);
> + pci_release_region(pdev, 3);
> pci_dev_put(pdev);
> } else {
> release_region(mxser_boards[i].ports[0].ioaddr, 8 *
> mxser_boards[i].nports);
> @@ -627,16 +627,14 @@ static int __init
> mxser_get_PCI_conf(int
> brd->board_type = board_type;
> brd->nports = mxser_numports[board_type - 1];
> ioaddress = pci_resource_start(pdev, 2);
> - request_region(pci_resource_start(pdev, 2), pci_resource_len(pdev, 2),
> - "mxser(IO)");
> + pci_request_region(pdev, 2, "mxser(IO)");
>
> for (i = 0; i < brd->nports; i++)
> brd->ports[i].ioaddr = ioaddress + 8 * i;
>
> /* vector */
> ioaddress = pci_resource_start(pdev, 3);
> - request_region(pci_resource_start(pdev, 3), pci_resource_len(pdev, 3),
> - "mxser(vector)");
> + pci_request_region(pdev, 3, "mxser(vector)");
> brd->vector = ioaddress;
>
> /* irq */

Correct me if I'm wrong, but that use of ioaddress looks totally wrong to me.
Isn't there a pci_iomap() or something missing?

Eike


Attachments:
(No filename) (2.13 kB)
(No filename) (189.00 B)
Download all attachments

2006-10-04 16:03:51

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 3/4] Char: mxser_new, pci_request_region for pci regions

Rolf Eike Beer wrote:
> Jiri Slaby wrote:
>> mxser_new, pci_request_region for pci regions
>>
>> Use pci_request_region instead of standard request_region for pci device
>> regions. More checking, simplier use.
>>
>> Signed-off-by: Jiri Slaby <[email protected]>
>>
>> ---
>> commit 1a717bdb06cef859dfbd426f46ea24a9c740e5c5
>> tree 85460f01008e9fa2edea675a73b394c48139df4a
>> parent d4f99406c592fb7ce2a65645d7c1f98ebe599238
>> author Jiri Slaby <[email protected]> Sat, 30 Sep 2006 01:20:12 +0200
>> committer Jiri Slaby <[email protected]> Sat, 30 Sep 2006 01:20:12
>> +0200
>>
>> drivers/char/mxser_new.c | 10 ++++------
>> 1 files changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/char/mxser_new.c b/drivers/char/mxser_new.c
>> index dfef9ce..c566cd0 100644
>> --- a/drivers/char/mxser_new.c
>> +++ b/drivers/char/mxser_new.c
>> @@ -526,8 +526,8 @@ static void __exit mxser_module_exit(voi
>> pdev = mxser_boards[i].pdev;
>> free_irq(mxser_boards[i].irq, &mxser_boards[i]);
>> if (pdev != NULL) { /* PCI */
>> - release_region(pci_resource_start(pdev, 2), pci_resource_len(pdev,
>> 2)); - release_region(pci_resource_start(pdev, 3),
>> pci_resource_len(pdev, 3)); + pci_release_region(pdev, 2);
>> + pci_release_region(pdev, 3);
>> pci_dev_put(pdev);
>> } else {
>> release_region(mxser_boards[i].ports[0].ioaddr, 8 *
>> mxser_boards[i].nports);
>> @@ -627,16 +627,14 @@ static int __init
>> mxser_get_PCI_conf(int
>> brd->board_type = board_type;
>> brd->nports = mxser_numports[board_type - 1];
>> ioaddress = pci_resource_start(pdev, 2);
>> - request_region(pci_resource_start(pdev, 2), pci_resource_len(pdev, 2),
>> - "mxser(IO)");
>> + pci_request_region(pdev, 2, "mxser(IO)");
>>
>> for (i = 0; i < brd->nports; i++)
>> brd->ports[i].ioaddr = ioaddress + 8 * i;
>>
>> /* vector */
>> ioaddress = pci_resource_start(pdev, 3);
>> - request_region(pci_resource_start(pdev, 3), pci_resource_len(pdev, 3),
>> - "mxser(vector)");
>> + pci_request_region(pdev, 3, "mxser(vector)");
>> brd->vector = ioaddress;
>>
>> /* irq */
>
> Correct me if I'm wrong, but that use of ioaddress looks totally wrong to me.
> Isn't there a pci_iomap() or something missing?

Both brd->vector and brd->ports[i].ioaddr are used in inb and outb function
calls. They themselves remap a small range (one byte in these cases) and require
ulong addresses as a parameter, not pointer to virtual space, correct?

thanks,
--
http://www.fi.muni.cz/~xslaby/ Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8 22A0 32CC 55C3 39D4 7A7E

2006-10-05 05:26:25

by Rolf Eike Beer

[permalink] [raw]
Subject: Re: [PATCH 3/4] Char: mxser_new, pci_request_region for pci regions

Jiri Slaby wrote:
> Rolf Eike Beer wrote:
> > Jiri Slaby wrote:
> >> mxser_new, pci_request_region for pci regions
> >>
> >> Use pci_request_region instead of standard request_region for pci device
> >> regions. More checking, simplier use.
> >>
> >> Signed-off-by: Jiri Slaby <[email protected]>
> >>
> >> ---
> >> commit 1a717bdb06cef859dfbd426f46ea24a9c740e5c5
> >> tree 85460f01008e9fa2edea675a73b394c48139df4a
> >> parent d4f99406c592fb7ce2a65645d7c1f98ebe599238
> >> author Jiri Slaby <[email protected]> Sat, 30 Sep 2006 01:20:12 +0200
> >> committer Jiri Slaby <[email protected]> Sat, 30 Sep 2006
> >> 01:20:12 +0200
> >>
> >> drivers/char/mxser_new.c | 10 ++++------
> >> 1 files changed, 4 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/char/mxser_new.c b/drivers/char/mxser_new.c
> >> index dfef9ce..c566cd0 100644
> >> --- a/drivers/char/mxser_new.c
> >> +++ b/drivers/char/mxser_new.c
> >> @@ -526,8 +526,8 @@ static void __exit mxser_module_exit(voi
> >> pdev = mxser_boards[i].pdev;
> >> free_irq(mxser_boards[i].irq, &mxser_boards[i]);
> >> if (pdev != NULL) { /* PCI */
> >> - release_region(pci_resource_start(pdev, 2), pci_resource_len(pdev,
> >> 2)); - release_region(pci_resource_start(pdev, 3),
> >> pci_resource_len(pdev, 3)); + pci_release_region(pdev, 2);
> >> + pci_release_region(pdev, 3);
> >> pci_dev_put(pdev);
> >> } else {
> >> release_region(mxser_boards[i].ports[0].ioaddr, 8 *
> >> mxser_boards[i].nports);
> >> @@ -627,16 +627,14 @@ static int __init
> >> mxser_get_PCI_conf(int
> >> brd->board_type = board_type;
> >> brd->nports = mxser_numports[board_type - 1];
> >> ioaddress = pci_resource_start(pdev, 2);
> >> - request_region(pci_resource_start(pdev, 2), pci_resource_len(pdev, 2),
> >> - "mxser(IO)");
> >> + pci_request_region(pdev, 2, "mxser(IO)");
> >>
> >> for (i = 0; i < brd->nports; i++)
> >> brd->ports[i].ioaddr = ioaddress + 8 * i;
> >>
> >> /* vector */
> >> ioaddress = pci_resource_start(pdev, 3);
> >> - request_region(pci_resource_start(pdev, 3), pci_resource_len(pdev, 3),
> >> - "mxser(vector)");
> >> + pci_request_region(pdev, 3, "mxser(vector)");
> >> brd->vector = ioaddress;
> >>
> >> /* irq */
> >
> > Correct me if I'm wrong, but that use of ioaddress looks totally wrong to
> > me. Isn't there a pci_iomap() or something missing?
>
> Both brd->vector and brd->ports[i].ioaddr are used in inb and outb function
> calls. They themselves remap a small range (one byte in these cases) and
> require ulong addresses as a parameter, not pointer to virtual space,
> correct?

Possible. I try to ignore that interface completely :) I would feel much more
comfortable if this would be done the other way. But this would probably
introduce more code because this one seems to work on non-PCI devices also.
So just ignore me :)

Eike


Attachments:
(No filename) (2.82 kB)
(No filename) (189.00 B)
Download all attachments