Subject: Re: Please revert: PCI: fix IDE legacy mode resources
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reply-To: benh@kernel.crashing.org
To: Yoichi Yuasa <yoichi_yuasa@tripeaks.co.jp>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
       Greg KH <gregkh@suse.de>, Ralf Baechle <ralf@linux-mips.org>,
       Linus Torvalds <torvalds@linux-foundation.org>
In-Reply-To: <200712060558.lB65wAFu016256@po-mbox304.hop.2iij.net>
References: <200710122305.l9CN5tFI008240@hera.kernel.org>
	 <1196899818.7033.11.camel@pasglop>
	 <200712060434.lB64YUtc023934@po-mbox305.hop.2iij.net>
	 <1196917447.7033.17.camel@pasglop>
	 <200712060558.lB65wAFu016256@po-mbox304.hop.2iij.net>
Content-Type: text/plain
Date: Thu, 06 Dec 2007 17:24:22 +1100
Message-Id: <1196922262.7033.33.camel@pasglop>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3723
Lines: 102


On Thu, 2007-12-06 at 14:58 +0900, Yoichi Yuasa wrote:
> > What I don't understand is thus why you are calling resource_to_bus
> on 0x1f0
> > which is -not- a resource value, but is already a BAR value...
> 
> 0x1f0 is resource value on MIPS Cobalt.
> All RAW BAR values contain the offset(0x10000000) on it.
> 
> Do the BAR values on your target contain the offset?

No, and I don't understand... raw BAR values don't contain such offset.
The physical address where the PIO is mapped might, but that's not what
we put in struct resource for IO resources and definitely not the BAR
value.

The legacy IDE device will decode cycles at 0x1f0, not cycles at
0x100001f0.

Take for example PowerPC. Imagine that I have a bus whose IO space is
mapped at 0xf0000000 in the processor physical address space (this is a
real example, my powermac does that for the x16 PCI-Express slot though
other slots use other offsets).

Now, the kernel on ppc64 will map that virtually at some allocated
virtual address that we'll call we call bus_io_virt for this
explanation. In addition, inb() and outb() will apply an offset (which
can be different) that we call _IO_BASE to the port numbers. In general,
bus_io_virt of the first bus == _IO_BASE on ppc32 but that's not a
strict rule.

Let's say for the sake of the example, that _IO_BASE is 0xd0000000 and
our bus has been mapped at 0xd0010000 (bus_io_virt). So 0xd0010000 maps
to 0xf0000000 via the MMU.

When we scan the bus, we read the BAR content. So for example, a device
whose IOs have been assigned at 0x1000 will read that as a RAW bar value
and pci_scan_slot() (or whoever does the reading) will put 0x1000 in the
struct resource. In a similar vein, such a legacy controller would thus
be expected to have 0x1f0 in the resource.

Later, when we fixup (in a head quirk on ppc32 and in pcibios_fixup_bus
on ppc64, though that's changing wit 2.6.25 to use the same mechanism),
we see an IO resource, and we fixup by adding to it basically
(bus_io_virt - _IO_BASE).

That is, for our device that has a 0x1000 BAR value, we'll do:

	resource = 0x1000 + (0xd0010000 - 0xd0000000) = 0x11000

And for the legacy IDE:

	resource = 0x1f0 + (0xd0010000 - 0xd0000000) = 0x101f0

Now, if you do an inb or outb to one of these, the inb() and oub()
accessors will add _IO_BASE, which is 0xd0000000 in our example, so
you'll end up doing accesses to respectively:

	access = 0xd0011000 for the example device or
	access = 0xd00101f0 for the IDE controller

That translates via the MMU to

	phys = 0xf0001000 for the example device or
	phys = 0xf00001f0 for the IDE controller

Which translates on to the bus into an IO cycle at the raw BAR
address (which is what is in the BAR content or hard-decoded in the case
of the legacy IDE):

	bus = 0x1000 for the example device
	bus = 0x01f0 for the IDE controller

which ... is what we started with.

Now I don't understand how MIPS does things differently, but I can't see
how it can be legal to call pci_resource_to_bus() on 0x1f0 in
pci_setup_device(), because at this stage, we are putting raw BAR values
in the struct resource (that is PCI bus addresses) and 0x1f0 _is_ such a
value.

Calling pci_resource_to_bus() would somewhat mean that 0x1f0 is not, and
instead is some kind of already fixed up resource value that we want
converted back into a BAR value, which is not the case.

So I suspect your patch works by accident more than by design, or I am
missing something...

Cheers,
Ben.


Ben.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/