On Tue, Jul 01, 2014 at 09:36:10PM +0200, Arnd Bergmann wrote: > On Tuesday 01 July 2014 19:43:28 Liviu Dudau wrote: > > +/* > > + * Record the PCI IO range (expressed as CPU physical address + size). > > + * Return a negative value if an error has occured, zero otherwise > > + */ > > +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size) > > +{ > > +#ifdef PCI_IOBASE > > + struct io_range *res; > > + resource_size_t allocated_size = 0; > > + > > + /* check if the range hasn't been previously recorded */ > > + list_for_each_entry(res, &io_range_list, list) { > > + if (addr >= res->start && addr + size <= res->start + size) > > + return 0; > > + allocated_size += res->size; > > + } > > + > > + /* range not registed yet, check for available space */ > > + if (allocated_size + size - 1 > IO_SPACE_LIMIT) > > + return -E2BIG; > > + > > + /* add the range to the list */ > > + res = kzalloc(sizeof(*res), GFP_KERNEL); > > + if (!res) > > + return -ENOMEM; > > + > > + res->start = addr; > > + res->size = size; > > + > > + list_add_tail(&res->list, &io_range_list); > > + > > + return 0; > > +#else > > + return -EINVAL; > > +#endif > > +} > > + > > unsigned long __weak pci_address_to_pio(phys_addr_t address) > > { > > +#ifdef PCI_IOBASE > > + struct io_range *res; > > + resource_size_t offset = 0; > > + > > + list_for_each_entry(res, &io_range_list, list) { > > + if (address >= res->start && > > + address < res->start + res->size) { > > + return res->start - address + offset; > > + } > > + offset += res->size; > > + } > > + > > + return (unsigned long)-1; > > +#else > > if (address > IO_SPACE_LIMIT) > > return (unsigned long)-1; > > > > return (unsigned long) address; > > +#endif > > } > > This still conflicts with the other allocator you have in patch 9 > for pci_remap_iospace: nothing guarantees that the mapping is the > same for both. > > Also, this is a completely pointless exercise at this moment, because > nobody cares about the result of pci_address_to_pio on architectures > that don't already provide this function. If we ever get a proper > Open Firmware implementation that wants to put hardcoded PCI devices > into DT, we can add an implementation, but for now this seems overkill. > > The allocator in pci_register_io_range seems reasonable, why not merge > this function with pci_remap_iospace() as I have asked you multiple > times before? Just make it return the io_offset so the caller can > put that into the PCI host resources. Hi Arnd, While I agree with you that at some moment the allocators were inconsistent wrt each other, for this version I would respectfully disagree on this. The allocator in pci_register_io_range() only makes sure that the ranges are not overlapping, it doesn't do any mapping whatsoever, while pci_remap_iospace() does only an ioremap_page_range(). The idea is that you get the offset out of pci_address_to_pio() and apply it to pci_remap_iospace(). Why do you think there are conflicts? Best regards, Liviu > > Arnd > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- ------------------- .oooO ( ) \ ( Oooo. \_) ( ) ) / (_/ One small step for me ... -- 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