On Wed, Feb 14, 2018 at 8:42 AM, John Garry <john.garry@xxxxxxxxxx> wrote: > On 13/02/2018 22:57, dann frazier wrote: >> >> On Wed, Feb 14, 2018 at 01:45:28AM +0800, John Garry wrote: >>> >>> From: Zhichang Yuan <yuanzhichang@xxxxxxxxxxxxx> >>> >>> After introducing the new generic I/O space management in logic pio, the >>> original PCI MMIO relevant helpers need to be updated based on the new >>> interfaces. >>> This patch adapts the corresponding code to match the changes introduced >>> by logic pio. >>> >>> Signed-off-by: Zhichang Yuan <yuanzhichang@xxxxxxxxxxxxx> >>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@xxxxxxxxxx> >>> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx> #earlier draft >>> Acked-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> >> >> >> I saw that Bjorn Acked this back in v6, but it seems like the code in >> pci.c was reworked a bit for v7 onwards and I didn't see a follow-up >> review (apologies if I just missed it). In which case, maybe his Ack >> should have the "#earlier draft" tag as well? >> > > Hi Dann, > > I see Bjorn acked again later on in the series, here: > https://lkml.org/lkml/2017/5/26/612 Yep, you're right, sorry for missing that :) > >>> --- >>> drivers/pci/pci.c | 95 >>> +++++++++--------------------------------------- >>> include/asm-generic/io.h | 2 +- >>> 2 files changed, 18 insertions(+), 79 deletions(-) >>> >>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c >>> index 07290a3..8aa5c54 100644 >>> --- a/drivers/pci/pci.c >>> +++ b/drivers/pci/pci.c >>> @@ -22,6 +22,7 @@ >>> #include <linux/spinlock.h> >>> #include <linux/string.h> >>> #include <linux/log2.h> >>> +#include <linux/logic_pio.h> >>> #include <linux/pci-aspm.h> >>> #include <linux/pm_wakeup.h> >>> #include <linux/interrupt.h> >>> @@ -3440,17 +3441,6 @@ int pci_request_regions_exclusive(struct pci_dev >>> *pdev, const char *res_name) >>> } >>> EXPORT_SYMBOL(pci_request_regions_exclusive); >>> >>> -#ifdef PCI_IOBASE >>> -struct io_range { >>> - struct list_head list; >>> - phys_addr_t start; >>> - resource_size_t size; >>> -}; >>> - >>> -static LIST_HEAD(io_range_list); >>> -static DEFINE_SPINLOCK(io_range_lock); >>> -#endif >>> - >>> /* >>> * Record the PCI IO range (expressed as CPU physical address + size). >>> * Return a negative value if an error has occured, zero otherwise >>> @@ -3458,51 +3448,28 @@ struct io_range { >>> int pci_register_io_range(struct fwnode_handle *fwnode, phys_addr_t >>> addr, >>> resource_size_t size) >>> { >>> - int err = 0; >>> - >>> + int ret = 0; >>> #ifdef PCI_IOBASE >>> - struct io_range *range; >>> - resource_size_t allocated_size = 0; >>> - >>> - /* check if the range hasn't been previously recorded */ >>> - spin_lock(&io_range_lock); >>> - list_for_each_entry(range, &io_range_list, list) { >>> - if (addr >= range->start && addr + size <= range->start + >>> size) { >>> - /* range already registered, bail out */ >>> - goto end_register; >>> - } >>> - allocated_size += range->size; >>> - } >>> + struct logic_pio_hwaddr *range; >>> >>> - /* range not registed yet, check for available space */ >>> - if (allocated_size + size - 1 > IO_SPACE_LIMIT) { >>> - /* if it's too big check if 64K space can be reserved */ >>> - if (allocated_size + SZ_64K - 1 > IO_SPACE_LIMIT) { >>> - err = -E2BIG; >>> - goto end_register; >>> - } >>> - >>> - size = SZ_64K; >>> - pr_warn("Requested IO range too big, new size set to >>> 64K\n"); >>> - } >>> + if (!size || addr + size < addr) >>> + return -EINVAL; >>> >>> - /* add the range to the list */ >>> range = kzalloc(sizeof(*range), GFP_ATOMIC); >>> - if (!range) { >>> - err = -ENOMEM; >>> - goto end_register; >>> - } >>> + if (!range) >>> + return -ENOMEM; >>> >>> - range->start = addr; >>> + range->fwnode = fwnode; >>> range->size = size; >>> + range->hw_start = addr; >>> + range->flags = PIO_CPU_MMIO; >>> >>> - list_add_tail(&range->list, &io_range_list); >>> - >>> -end_register: >>> - spin_unlock(&io_range_lock); >>> + ret = logic_pio_register_range(range); >>> + if (ret) >>> + kfree(range); >>> #endif >>> >>> - return err; >>> + return ret; >>> } >>> >>> phys_addr_t pci_pio_to_address(unsigned long pio) >>> @@ -3510,21 +3477,10 @@ phys_addr_t pci_pio_to_address(unsigned long pio) >>> phys_addr_t address = (phys_addr_t)OF_BAD_ADDR; >>> >>> #ifdef PCI_IOBASE >>> - struct io_range *range; >>> - resource_size_t allocated_size = 0; >>> - >>> - if (pio > IO_SPACE_LIMIT) >>> + if (pio >= MMIO_UPPER_LIMIT) >>> return address; >>> >>> - spin_lock(&io_range_lock); >>> - list_for_each_entry(range, &io_range_list, list) { >>> - if (pio >= allocated_size && pio < allocated_size + >>> range->size) { >>> - address = range->start + pio - allocated_size; >>> - break; >>> - } >>> - allocated_size += range->size; >>> - } >>> - spin_unlock(&io_range_lock); >>> + address = logic_pio_to_hwaddr(pio); >>> #endif >>> >>> return address; >>> @@ -3533,25 +3489,8 @@ phys_addr_t pci_pio_to_address(unsigned long pio) >>> unsigned long __weak pci_address_to_pio(phys_addr_t address) >>> { >>> #ifdef PCI_IOBASE >>> - struct io_range *res; >>> - resource_size_t offset = 0; >>> - unsigned long addr = -1; >>> - >>> - spin_lock(&io_range_lock); >>> - list_for_each_entry(res, &io_range_list, list) { >>> - if (address >= res->start && address < res->start + >>> res->size) { >>> - addr = address - res->start + offset; >>> - break; >>> - } >>> - offset += res->size; >>> - } >>> - spin_unlock(&io_range_lock); >>> - >>> - return addr; >>> + return logic_pio_trans_cpuaddr(address); >>> #else >>> - if (address > IO_SPACE_LIMIT) >>> - return (unsigned long)-1; >>> - >> >> >> Why is this check now safe to drop in the !PCI_IOBASE case? > > > So I have been studying the (long) patchset history and I don't see the > reason for removing this check, and I think it can be reinstated. > > The address and pio are analogous here, and pio should not exceed the IO > space limit, so just a safety check. Sounds good. -dann >> >> -dann >> >>> return (unsigned long) address; >>> #endif >>> } >>> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h >>> index b7996a79..5a59931 100644 >>> --- a/include/asm-generic/io.h >>> +++ b/include/asm-generic/io.h >>> @@ -901,7 +901,7 @@ static inline void iounmap(void __iomem *addr) >>> #define ioport_map ioport_map >>> static inline void __iomem *ioport_map(unsigned long port, unsigned int >>> nr) >>> { > > > Thanks, > John > >>> - return PCI_IOBASE + (port & IO_SPACE_LIMIT); >>> + return PCI_IOBASE + (port & MMIO_UPPER_LIMIT); >>> } >>> #endif >>> >> >> . >> > > -- 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
