On Wednesday, November 23, 2016 3:22:33 PM CET Gabriele Paoloni wrote: > From: Arnd Bergmann [mailto:arnd@xxxxxxxx] > > On Friday, November 18, 2016 5:03:11 PM CET Gabriele Paoloni wrote: > > > 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...) > > > > Why do you think the order matters? My point was that we should > > be able to register any region of logical port numbers for any > > bus here. > > Maybe I have not followed well so let's roll back to your previous > comment... > > "we need to associate a bus address with a logical Linux port number, > both in of_address_to_resource and in inb()/outb()" > > Actually of_address_to_resource() returns the port number to used > in inb/outb(); inb() and outb() add the port number to PCI_IOBASE > to rd/wr to the right virtual address. Correct. > Our LPC cannot operate on the virtual address and it operates on > a bus address range that for LPC is also equal to the cpu address > range and goes from 0 to 0x1000. There is no "cpu address" here, otherwise this is correct. > Now as I understand it is risky and not appropriate to reserve > the logical port numbers from 0 to 0x1000 or to whatever other > upper bound because existing systems may rely on these port numbers > retrieved by __of_address_to_resource(). Right again. > In this scenario I think the best thing to do would be > in the probe function of the LPC driver: > 1) call pci_register_io_range() passing [0, 0x1000] (that is the > range for LPC) pci_register_io_range() takes a physical address, not a port number, so that would not be appropriate as you say above. We can however add a variant that reserves a range of port numbers in io_range_list for an indirect access method. > 2) retrieve the logical port numbers associated to the LPC range > by calling pci_address_to_pio() for 0 and 0x1000 and assign > them to extio_ops_node->start and extio_ops_node->end Again, calling pci_address_to_pio() doesn't seem right here, because we don't have a phys_addr_t address > 3) implement the LPC accessors to operate on the logical ports > associated to the LPC range (in practice in the accessors > implementation we will call pci_pio_to_address to retrieve > the cpu address to operate on) Please don't proliferate the use of pci_pio_to_address/pci_address_to_pio here, computing the physical address from the logical address is trivial, you just need to subtract the start of the range that you already use when matching the port number range. The only thing we need here is to make of_address_to_resource() return the correct logical port number that was registered for a given host device when asked to translate an address that does not have a CPU address associated with it. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html