[PATCH 01/19] pci: introduce an pci_ioremap(pdev, barnr) function

[PATCH 01/19] pci: introduce an pci_ioremap(pdev, barnr) function

[Arjan van de Ven]

The patch below introduces a pci_ioremap() function that should make it easier for
driver authors to do the right thing for the simple, common case.

There's also 18 patches that introduce users of this; to reduce lkml noise I've only
stuck them in a git tree at
git://git.kernel.org/pub/scm/linux/kernel/git/arjan/linux-2.6-pci_ioremap.git
(http://git.kernel.org/?p=linux/kernel/git/arjan/linux-2.6-pci_ioremap.git;a=summary)


>From fef1dd836bc7dc07962a0ae4019af9efd373c76f Mon Sep 17 00:00:00 2001
From: Arjan van de Ven <arjan@linux.intel.com>
Date: Fri, 26 Sep 2008 16:34:52 -0700
Subject: [PATCH 01/19] pci: introduce an pci_ioremap(pdev, barnr) function

A common thing in many PCI drivers is to ioremap() an entire bar.
This is a slightly fragile thing right now, needing both an address and a size,
and many driver writers do.. various things there.

This patch introduces an pci_ioremap() function taking just a PCI device struct
and the bar number as arguments, and figures this all out itself, in one place.
In addition, we can add various sanity checks to this function (the patch already
checks to make sure that the bar in question really is a MEM bar; few to no drivers
do that sort of thing).

Hopefully with this type of API we get less chance of mistakes in drivers with
ioremap() operations.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 include/linux/pci.h |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 709b8d4..3f8ce81 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1127,5 +1127,18 @@ static inline void pci_mmcfg_early_init(void) { }
 static inline void pci_mmcfg_late_init(void) { }
 #endif
 
+static inline void * pci_ioremap_bar(struct pci_dev *pdev, int bar)
+{
+	/*
+	 * Make sure the BAR is actually a memory resource, not an IO resource
+	 */
+	if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) {
+		WARN_ON(1);
+		return NULL;
+	}
+	return ioremap_nocache(pci_resource_start(pdev, bar),
+				     pci_resource_len(pdev, bar));
+}
+
 #endif /* __KERNEL__ */
 #endif /* LINUX_PCI_H */
-- 
1.5.5.1



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH 01/19] pci: introduce an pci_ioremap(pdev, barnr) function

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 1353 bytes --]

Arjan van de Ven wrote:
> The patch below introduces a pci_ioremap() function that should make it
> easier for driver authors to do the right thing for the simple, common
> case.
>
> There's also 18 patches that introduce users of this; to reduce lkml noise
> I've only stuck them in a git tree at
> git://git.kernel.org/pub/scm/linux/kernel/git/arjan/linux-2.6-pci_ioremap.g
>it
> (http://git.kernel.org/?p=linux/kernel/git/arjan/linux-2.6-pci_ioremap.git;
>a=summary)
>
>
> From fef1dd836bc7dc07962a0ae4019af9efd373c76f Mon Sep 17 00:00:00 2001
> From: Arjan van de Ven <arjan@linux.intel.com>
> Date: Fri, 26 Sep 2008 16:34:52 -0700
> Subject: [PATCH 01/19] pci: introduce an pci_ioremap(pdev, barnr) function
>
> A common thing in many PCI drivers is to ioremap() an entire bar.
> This is a slightly fragile thing right now, needing both an address and a
> size, and many driver writers do.. various things there.
>
> This patch introduces an pci_ioremap() function taking just a PCI device
> struct and the bar number as arguments, and figures this all out itself, in
> one place. In addition, we can add various sanity checks to this function
> (the patch already checks to make sure that the bar in question really is a
> MEM bar; few to no drivers do that sort of thing).

This is the same like pci_iomap(pdev, number, 0), no?

Greetings,

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 01/19] pci: introduce an pci_ioremap(pdev, barnr) function

[Jesse Barnes]

On Monday, September 29, 2008 10:02 am Rolf Eike Beer wrote:
> Arjan van de Ven wrote:
> > The patch below introduces a pci_ioremap() function that should make it
> > easier for driver authors to do the right thing for the simple, common
> > case.
> >
> > There's also 18 patches that introduce users of this; to reduce lkml
> > noise I've only stuck them in a git tree at
> > git://git.kernel.org/pub/scm/linux/kernel/git/arjan/linux-2.6-pci_ioremap
> >.g it
> > (http://git.kernel.org/?p=linux/kernel/git/arjan/linux-2.6-pci_ioremap.gi
> >t; a=summary)
> >
> >
> > From fef1dd836bc7dc07962a0ae4019af9efd373c76f Mon Sep 17 00:00:00 2001
> > From: Arjan van de Ven <arjan@linux.intel.com>
> > Date: Fri, 26 Sep 2008 16:34:52 -0700
> > Subject: [PATCH 01/19] pci: introduce an pci_ioremap(pdev, barnr)
> > function
> >
> > A common thing in many PCI drivers is to ioremap() an entire bar.
> > This is a slightly fragile thing right now, needing both an address and a
> > size, and many driver writers do.. various things there.
> >
> > This patch introduces an pci_ioremap() function taking just a PCI device
> > struct and the bar number as arguments, and figures this all out itself,
> > in one place. In addition, we can add various sanity checks to this
> > function (the patch already checks to make sure that the bar in question
> > really is a MEM bar; few to no drivers do that sort of thing).
>
> This is the same like pci_iomap(pdev, number, 0), no?

Yeah... Looks like that function isn't that widely used though.  Is the maxlen 
param really needed?  Looks like the drivers that use it often pass 0 or the 
BAR length anyway, and Arjan converted existing drivers too, which is where 
the real work is.

Jesse

Re: [PATCH 01/19] pci: introduce an pci_ioremap(pdev, barnr) function

[Alan Cox]

> > This is the same like pci_iomap(pdev, number, 0), no?
> 
> Yeah... Looks like that function isn't that widely used though.  Is the maxlen 
> param really needed?  Looks like the drivers that use it often pass 0 or the 

In some cases yes you do need the length.

> BAR length anyway, and Arjan converted existing drivers too, which is where 
> the real work is.

He could have converted them to the existing perfectly good API not
invented another one. He still can - I'm sure its a perl one liner to
redo them in terms of pci_iomap()

Alan

Re: [PATCH 01/19] pci: introduce an pci_ioremap(pdev, barnr) function

[Arjan van de Ven]

On Mon, 29 Sep 2008 18:45:19 +0100
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > > This is the same like pci_iomap(pdev, number, 0), no?
> > 
> > Yeah... Looks like that function isn't that widely used though.  Is
> > the maxlen param really needed?  Looks like the drivers that use it
> > often pass 0 or the 
> 
> In some cases yes you do need the length.

so we should have a 2nd api for those "some cases", that's ok.

> 
> > BAR length anyway, and Arjan converted existing drivers too, which
> > is where the real work is.
> 
> He could have converted them to the existing perfectly good API not
> invented another one. He still can - I'm sure its a perl one liner to
> redo them in terms of pci_iomap()

pci_iomap() assumes you're using the rest of the iomap API as well!

most drivers do not do that.




-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH 01/19] pci: introduce an pci_ioremap(pdev, barnr) function

[Jesse Barnes]

On Monday, September 29, 2008 11:15 am Arjan van de Ven wrote:
> On Mon, 29 Sep 2008 18:45:19 +0100
>
> Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > > > This is the same like pci_iomap(pdev, number, 0), no?
> > >
> > > Yeah... Looks like that function isn't that widely used though.  Is
> > > the maxlen param really needed?  Looks like the drivers that use it
> > > often pass 0 or the
> >
> > In some cases yes you do need the length.
>
> so we should have a 2nd api for those "some cases", that's ok.

Right, I'm definitely not suggesting that we don't provide a way to provide 
length, but if 90+% of callers don't need it, we should probably have a 
simpler API that does the bare minimum.

Jesse

Re: [PATCH 01/19] pci: introduce an pci_ioremap(pdev, barnr) function

[Jesse Barnes]

On Sunday, September 28, 2008 4:36 pm Arjan van de Ven wrote:
> The patch below introduces a pci_ioremap() function that should make it
> easier for driver authors to do the right thing for the simple, common
> case.
>
> There's also 18 patches that introduce users of this; to reduce lkml noise
> I've only stuck them in a git tree at
> git://git.kernel.org/pub/scm/linux/kernel/git/arjan/linux-2.6-pci_ioremap.g
>it
> (http://git.kernel.org/?p=linux/kernel/git/arjan/linux-2.6-pci_ioremap.git;
>a=summary)
>
>
> From fef1dd836bc7dc07962a0ae4019af9efd373c76f Mon Sep 17 00:00:00 2001
> From: Arjan van de Ven <arjan@linux.intel.com>
> Date: Fri, 26 Sep 2008 16:34:52 -0700
> Subject: [PATCH 01/19] pci: introduce an pci_ioremap(pdev, barnr) function
>
> A common thing in many PCI drivers is to ioremap() an entire bar.
> This is a slightly fragile thing right now, needing both an address and a
> size, and many driver writers do.. various things there.
>
> This patch introduces an pci_ioremap() function taking just a PCI device
> struct and the bar number as arguments, and figures this all out itself, in
> one place. In addition, we can add various sanity checks to this function
> (the patch already checks to make sure that the bar in question really is a
> MEM bar; few to no drivers do that sort of thing).
>
> Hopefully with this type of API we get less chance of mistakes in drivers
> with ioremap() operations.
>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

Applied to my linux-next branch, thanks.  I assume you'll have Linus pull the 
driver updates later?

Thanks,
Jesse

Re: [PATCH 01/19] pci: introduce an pci_ioremap(pdev, barnr) function

[Arjan van de Ven]

On Thu, 2 Oct 2008 11:53:40 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> >
> > Hopefully with this type of API we get less chance of mistakes in
> > drivers with ioremap() operations.
> >
> > Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> 
> Applied to my linux-next branch, thanks.  I assume you'll have Linus
> pull the driver updates later?

yeah I either need to do an akpm-like "spray the maintainers" or just
do one big pull, haven't figure it out yet
(would be nice if I could just dump the patches on Andrew who then does
the bombard-the-maintainers thing)


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org