-----Original Message-----
From: Arnd Bergmann [mailto:arnd at arndb.de]
Sent: 18 November 2016 16:35
To: Gabriele Paoloni
Cc: linux-arm-kernel at lists.infradead.org; mark.rutland at arm.com;
benh at kernel.crashing.org; catalin.marinas at arm.com; liviu.dudau at arm.com;
Linuxarm; lorenzo.pieralisi at arm.com; xuwei (O); Jason Gunthorpe; linux-
serial at vger.kernel.org; linux-pci at vger.kernel.org;
devicetree at vger.kernel.org; minyard at acm.org; will.deacon at arm.com; John
Garry; zourongrong at gmail.com; robh+dt at kernel.org; bhelgaas at go og
le.com; kantyzc at 163.com; zhichang.yuan02 at gmail.com; T homas Petazzoni;
linux-kernel at vger.kernel.org; Yuanzhichang; olof at lixom.net
Subject: Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on
Hip06

On Friday, November 18, 2016 4:18:07 PM CET Gabriele Paoloni wrote:

quoted

From: Arnd Bergmann [mailto:arnd at arndb.de]

quoted

On Friday, November 18, 2016 12:53:08 PM CET Gabriele Paoloni

wrote:

quoted

quoted

quoted

From: Arnd Bergmann [mailto:arnd at arndb.de]

quoted

On Friday, November 18, 2016 12:07:28 PM CET Gabriele Paoloni
The easiest change compared to the v5 code would be to walk
a linked list of 'struct extio_ops' structures rather than
assuming there is only ever one of them. I think one of the
earlier versions actually did this.

Right but if my understanding is correct if we live in a multi-
domain I/O space world when you have an input addr in the I/O
accessors this addr can be duplicated (for example for the

standard

quoted

quoted

quoted

PCI IO domain and for our special LPC domain).
So effectively even if you walk a linked list there is a problem
of disambiguation...am I right?

No, unlike the PCI memory space, the PIO addresses are not
usually distinct, i.e. every PCI bus has its own 64K I/O
addresses starting at zero. We linearize them into the
Linux I/O space using the per-domain io_offset value.

I am going to summarize my understanding here below:

It seems to me that what is linearized is the virtual address
space associated to the IO address space. This address space
goes from PCI_IOBASE to (PCI_IOBASE + IO_SPACE_LIMIT).

The I/O accessors perform rd/wr operation on this address
space using a port IO token.

Each token map into a cpu physical address range
Each cpu physical address range maps to a bus address range
if the bus controller specifies a range property.

Devices under a bus controller specify the bus addresses that
they operate on in their reg property.

So each device can use the same bus addresses under two
separate bus controllers as long as the bus controller
use the range properties to map the same bus range to different
cpu address range.

Sounds all correct to me so far, yes.

quoted

quoted

For the ISA/LPC spaces there are only 4k of addresses, they
the bus addresses always overlap, but we can trivially
figure out the bus address from Linux I/O port number
by subtracting the start of the range.

Are you saying that our LPC controller should specify a
range property to map bus addresses into a cpu address range?

No. There is not CPU address associated with it, because it's
not memory mapped.

Instead, we need to associate a bus address with a logical
Linux port number, both in of_address_to_resource and
in inb()/outb().

I think this is effectively what we are doing so far with patch 2/3.
The problem with this patch is that we are carving out a "forbidden"
IO tokens range that goes from 0 to PCIBIOS_MIN_IO.

I think that the proper solution would be to have the LPC driver to
set the carveout threshold used in pci_register_io_range(), 
pci_pio_to_address(), pci_address_to_pio(), but this would impose
a probe dependency on the LPC itself that should be probed before
the PCI controller (or before any other devices calling these
functions...) 

quoted

quoted

quoted

quoted

Another option the IA64 approach mentioned in another subthread
today, looking up the operations based on an index from the
upper bits of the port number. If we do this, we probably
want to do that for all PIO access and replace the entire
virtual address remapping logic with that. I think Bjorn
in the past argued in favor of such an approach, while I
advocated the current scheme for simplicity based on how
every I/O space these days is just memory mapped (which now
turned out to be false, both on powerpc and arm64).

This seems really complex...I am a bit worried that possibly
we end up in having the maintainers saying that it is not worth
to re-invent the world just for this special LPC device...

It would clearly be a rather invasive change, but the
end-result isn't necessarily more complex than what we
have today, as we'd kill off the crazy pci_pio_to_address()
and pci_address_to_pio() hacks in address translation.

I have to look better into this...can you provide me a reference
to the Bjorn argument in favor of this approach?

The thread seems to have been pci: Introduce pci_register_io_range()
helper function, e.g. in https://lkml.org/lkml/2014/7/8/969

Ok many thanks I am going to look at it

quoted

quoted

quoted

To be honest with you I would keep things simple for this
LPC and introduce more complex reworks later if more devices
need to be introduced.

What if we stick on a single domain now where we introduce a
reserved threshold for the IO space (say INDIRECT_MAX_IO).

I said having a single domain is fine, but I still don't
like the idea of reserving low port numbers for this hack,
it would mean that the numbers change for everyone else.

I don't get this much...I/O tokens that are passed to the I/O
accessors are not fixed anyway and they vary depending on the order
of adding ranges to io_range_list...so I don't see a big issue
with this...

On machines with a legacy devices behind the PCI bridge,
there may still be a reason to have the low I/O port range
reserved for the primary bus, e.g. to get a VGA text console
to work.

On powerpc, this is called the "primary" PCI host, i.e. the
only one that is allowed to have an ISA bridge.

Yes but
1) isn't the PCI controller range property that defines how IO bus address
   map into physical CPU addresses?
2) How can you guarantee that the cpu range associated with this
   IO bus range is the first to be registered in pci_register_io_range()?
   ( i.e. are you saying that they are just relying on the fact that it is the
     only IO range in the system and by chance the IO tokens and corresponding
     bus addresses are the same? )

Thanks

Gab

	Arnd