Re: [PATCH-RFC 07/10] parisc: switch to GENERIC_PCI_IOMAP

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Dec 05, 2011 at 09:33:29AM -0600, James Bottomley wrote:
> On Sun, 2011-12-04 at 16:42 +0200, Michael S. Tsirkin wrote:
> > On Sun, Dec 04, 2011 at 08:32:24AM -0600, James Bottomley wrote:
> > > On Sun, 2011-12-04 at 16:30 +0200, Michael S. Tsirkin wrote:
> > > > On Sun, Dec 04, 2011 at 08:22:34AM -0600, James Bottomley wrote:
> > > > > On Sun, 2011-12-04 at 12:48 +0200, Michael S. Tsirkin wrote:
> > > > > > On Thu, Nov 24, 2011 at 10:19:34PM +0200, Michael S. Tsirkin wrote:
> > > > > > > parisc copied pci_iomap from generic code, probably to avoid
> > > > > > > pulling the rest of iomap.c in.  Since that's in
> > > > > > > a separate file now, we can reuse the common implementation.
> > > > > > > 
> > > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > > > 
> > > > > > Sorry to nag, any ACKs/NACKs on the parisc part?
> > > > > > I intend to send this to Linus if there are no
> > > > > > objections. Thanks!
> > > > > 
> > > > > Next time, send the patch to linux-arch or linux-parisc ... it helps to
> > > > > get people to review it. Although this one looks completely trivial,
> > > > > unless I'm missing something?
> > > > 
> > > > I think it's trivial but I don't have the setup to even compile it so
> > > > depend on someone to ack.
> > > > 
> > > > > Obviously there was a bit of a brain fart when this was done: the parisc
> > > > > piece shouldn't be a copy of the generic code: we don't have native i/o
> > > > > ports and emulation is pretty expensive, so we should prefer
> > > > > IORESOURCE_MEM over IORESOURCE_IO when both are available  ... but even
> > > > > on x86, memory mapping is faster (although not by the order of magnitude
> > > > > it is on parisc), so it looks like you could fix this in the generic
> > > > > code.
> > > > > 
> > > > > James
> > > > 
> > > > AFAIK, on PCI a resource is either IO or MEM, but never both.
> > > > What am I missing?
> > > 
> > > Could be ... I know we've had issues in the past with standard drivers
> > > like the sym53c8xx which map via both and operate far less efficiently
> > > over I/O ... but that could have been different bars I suppose (it's so
> > > long ago that I can't remember).
> > 
> > Probably, it looks like it has both mmio and io:
> > 
> > drivers/scsi/sym53c8xx_2/sym_glue.c
> > 
> > #ifdef CONFIG_SCSI_SYM53C8XX_MMIO
> >         if (device->mmio_base)
> >                 device->s.ioaddr = pci_iomap(pdev, 1,
> >                                                 pci_resource_len(pdev, 1));
> > #endif
> >         if (!device->s.ioaddr)
> >                 device->s.ioaddr = pci_iomap(pdev, 0,
> >                                                 pci_resource_len(pdev, 0));
> > 
> > 
> > so ... ACK?
> 
> Um, well, it looks like a trivial code shift, so it doesn't really need
> one (it can just go through the trivial tree).

It's part of a larger set not all of which is trivial.

>  Unless there's some
> reason actually to test it out on parisc because something will break
> (in which case, I'll need the actual patch, not just a quoted one).
> 
> James

A build test would be nice. Just forwarded you a patch.
It's also on linux-next.

-- 
MST