On Mon, Mar 10, 2014 at 03:21:01PM +0000, Arnd Bergmann wrote: > On Monday 10 March 2014 14:44:14 Liviu Dudau wrote: > > I will try to improve the error handling in the next patchset version. > > However I am still confused about the earlier discussion on > > pci_register_io_range(). Your suggestion initially was to return an > > error in the default weak implementation, but in your last email you > > are talking about returning 'port'. > > You can do either one: 'port' should be positive or zero, while the > error would always be negative. We do the same thing in many interfaces > in the kernel. > > > My idea when I've introduced the > > helper function was that it would return an error if it fails to > > register the IO range and zero otherwise. I agree that we can treat > > the default 'do nothing with the IO range' case as an error, with > > the caveat that will force architectures that use this code to > > provide their own implementation of pci_register_io_range() in order > > to avoid failure, even for the cases where the architecture has a 1:1 > > mapping between IO and CPU addresses. > > Which architectures are you thinking of? The only one I know that > does this is ia64, and we won't ever have to support this helper > on that architecture. I was thinking about architectures that have IO_SPACE_LIMIT >= 0xffffffff. While not an absolute indicator, with the default pci_address_to_pio() that means that they can use the CPU MMIO address as IO address directly. $ git grep IO_SPACE_LIMIT | grep -i ffffffff arch/arm/include/asm/io.h:#define IO_SPACE_LIMIT ((resource_size_t)0xffffffff) arch/arm/mach-at91/include/mach/io.h:#define IO_SPACE_LIMIT 0xFFFFFFFF arch/arm/mach-omap1/include/mach/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/arm/mach-pxa/include/mach/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/arm/mach-s3c24xx/include/mach/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/avr32/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/frv/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/ia64/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffffffffffffUL arch/m32r/include/asm/io.h:#define IO_SPACE_LIMIT 0xFFFFFFFF arch/m68k/include/asm/io_no.h:#define IO_SPACE_LIMIT 0xffffffff arch/microblaze/include/asm/io.h:#define IO_SPACE_LIMIT (0xFFFFFFFF) arch/mn10300/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/sh/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/sparc/include/asm/io_32.h:#define IO_SPACE_LIMIT 0xffffffff arch/sparc/include/asm/io_64.h:#define IO_SPACE_LIMIT 0xffffffffffffffffUL arch/tile/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff > > I did not ask to treat 'do nothing with the IO range' as an error, > what I meant is that we should treat 'architecture cannot translate > from I/O space to memory space but DT lists a translation anyway' > as an error. On x86, you should never see an entry for the I/O space > in "ranges", so we will not call this function unless there is a > bug in DT. Ok, it's just that there is no "architecture cannot translate from I/O space to memory space" indicator anywhere and I don't want to make x86 a special case. So, my proposal is this: default weak implementation of pci_register_io_range() returns an error (meaning I have no idea how to translate IO addresses to memory space) and anyone that wants this function to return success will have to provide their implementation. I will send an updated series. Best regards, Liviu > > Arnd > > -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯ -- 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