LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Russell King <rmk+lkml@arm.linux.org.uk>
To: Adam Belay <ambx1@neo.rr.com>,
	Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>,
	Grant Grundler <grundler@parisc-linux.org>,
	Andrew Morton <akpm@osdl.org>, Greg KH <greg@kroah.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-pci@atrey.karlin.mff.cuni.cz
Subject: Re: [PATCH 0/4] PCI legacy I/O port free driver (take4)
Date: Fri, 10 Mar 2006 08:33:24 +0000
Message-ID: <20060310083324.GA25092@flint.arm.linux.org.uk> (raw)
In-Reply-To: <20060310021009.GA2506@neo.rr.com>

On Thu, Mar 09, 2006 at 09:10:10PM -0500, Adam Belay wrote:
> On Thu, Mar 02, 2006 at 07:34:41PM +0000, Russell King wrote:
> > I have another question (brought up by someone working on a series of
> > ARM machines which make heavy use of MMIO.)
> > 
> > Why isn't pci_enable_device_bars() sufficient - why do we have to
> > have another interface to say "we don't want BARs XXX" ?
> > 
> > Let's say that we have a device driver which does this sequence (with,
> > of course, error checking):
> > 
> > 	pci_enable_device_bars(dev, 1<<1);
> > 	pci_request_regions(dev);
> > 
> > (a) should PCI remember that only BAR 1 has been requested to be enabled,
> >     and as such shouldn't pci_request_regions() ignore BAR 0?
> > 
> > (b) should the PCI driver pass into pci_request_regions() (or even
> >     pci_request_regions_bars()) a bitmask of the BARs it wants to have
> >     requested, and similarly for pci_release_regions().
> > 
> > Basically, if BAR0 hasn't been enabled, has pci_request_regions() got
> > any business requesting it from the resource tree?
> 
> I understand the point you're making, but I think this misrepresents what
> is actually happening.  From my understanding of the spec, it's not possible
> to disable individual bars (with the exception of the expansion ROM).  Rather
> there is one bit for IO enable and one bit for IOMMU enable.  Therefore, we
> can enable or disable all I/O ports, but there's really no in between.  If
> the device uses even one I/O port, it's still a huge loss because of the
> potential bridge window dependency.  Also, if a device has several I/O ports
> but the driver only wants to use one, all of the others must still be
> assigned.

Agreed, but despite claiming that you understand my point, I don't
think you actually do.

1. It is already the case that drivers need to know the type of each BAR
   which the device presents - eg, most, if not all drivers take the start
   address of BAR0, throw that directly at ioremap, or use inb etc on it
   directly without first checking whether it's a MMIO or IO resource.

   So, if a driver knows that BAR0 is IO and BAR1 is MMIO, it can use
   pci_enable_device_bars(dev, BAR1) to only enable MMIO access.

2. pci_enable_device_bars() (which pre-exists for dealing with IDE) when
   passed the appropriate BAR mask, can be used to "enable" (or setup,
   program, or whatever, see below) only all MMIO or all IO BARs.

3. It is defined by the PCI subsystem that, for drivers, PCI resources
   are not valid prior to pci_enable_device*() being called, and are
   valid immediately after this call returns - pci_enable_device*()
   might be used by a PCI subsystem implementation to assign or
   reassign, and program PCI resources.

   Therefore, if you request pci_enable_device() (or pci_enable_device_bars
   with a full-bar mask) it is expected that _all_ BARs (or those
   requested) will become valid.  Adding this "no_io" device flag
   breaks this expectation.

4. I'm not suggesting that pci_enable_device_bars() should be (or can be)
   used to "selectively enable BARs" which I think is how you're reading
   this.  Yes, it does apparantly have the capacity to do this (and is
   used like this for IDE) but inappropriate use like that is a driver
   bug, and is already a driver bug *today*.

I'm merely suggesting using an established interface which has some
agreed appropriate driver expectations for the purpose of solving
this new problem.  It fits perfectly, it's clean, it doesn't add lots
of complexity, and there's no reason what so ever not to use it.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

  parent reply index

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-03-02 15:12 Kenji Kaneshige
2006-03-02 15:14 ` [PATCH 1/4] PCI legacy I/O port free driver (take4) - Add no_ioport flag into pci_dev Kenji Kaneshige
2006-03-02 15:16 ` [PATCH 2/4] PCI legacy I/O port free driver (take4) - Update Documentation/pci.txt Kenji Kaneshige
2006-03-02 15:18 ` [PATCH 3/4] PCI legacy I/O port free driver (take4) - Make Intel e1000 driver legacy I/O port free Kenji Kaneshige
2006-03-02 15:20 ` [PATCH 4/4] PCI legacy I/O port free driver (take4) - Make Emulex lpfc " Kenji Kaneshige
2006-03-02 15:50 ` [PATCH 0/4] PCI legacy I/O port free driver (take4) Russell King
2006-03-02 16:23   ` Kenji Kaneshige
2006-03-02 16:41     ` Greg KH
2006-03-02 17:24   ` Grant Grundler
2006-03-02 18:00     ` Russell King
2006-03-02 18:12       ` Jeff Garzik
2006-03-02 19:13         ` Russell King
2006-03-02 20:01           ` Jeff Garzik
2006-03-02 19:23       ` Grant Grundler
2006-03-02 19:34     ` Russell King
2006-03-02 19:50       ` Roland Dreier
2006-03-03  3:17       ` Kenji Kaneshige
2006-03-03  6:59         ` Kenji Kaneshige
2006-03-06  1:38           ` Kenji Kaneshige
2006-03-10  2:10       ` Adam Belay
2006-03-10  4:10         ` Kenji Kaneshige
2006-03-10  7:49           ` Russell King
2006-03-10  8:33         ` Russell King [this message]
2006-03-13  5:47           ` Kenji Kaneshige

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060310083324.GA25092@flint.arm.linux.org.uk \
    --to=rmk+lkml@arm.linux.org.uk \
    --cc=akpm@osdl.org \
    --cc=ambx1@neo.rr.com \
    --cc=greg@kroah.com \
    --cc=grundler@parisc-linux.org \
    --cc=kaneshige.kenji@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@atrey.karlin.mff.cuni.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git