From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liviu Dudau Subject: Re: [PATCH v6 6/6] pci: Add support for creating a generic host_bridge from device tree Date: Mon, 10 Mar 2014 16:33:44 +0000 Message-ID: <20140310163343.GY6457@e106497-lin.cambridge.arm.com> References: <1394020137-1830-1-git-send-email-Liviu.Dudau@arm.com> <201403081815.08829.arnd@arndb.de> <20140310144414.GT6457@e106497-lin.cambridge.arm.com> <2115856.ZTdgp5fkma@wuerfel> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <2115856.ZTdgp5fkma@wuerfel> Content-Disposition: inline Sender: linux-pci-owner@vger.kernel.org To: Arnd Bergmann Cc: "linux-arm-kernel@lists.infradead.org" , "devicetree@vger.kernel.org" , linaro-kernel , Catalin Marinas , linux-pci , Will Deacon , LKML , Tanmay Inamdar , Benjamin Herrenschmidt , Bjorn Helgaas List-Id: devicetree@vger.kernel.org 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 versi= on. > > 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 yo= u > > are talking about returning 'port'. >=20 > 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 interfac= es > in the kernel. >=20 > > 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 orde= r > > to avoid failure, even for the cases where the architecture has a 1= :1 > > mapping between IO and CPU addresses. >=20 > 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 >=3D 0xffff= ffff. 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 directl= y. $ git grep IO_SPACE_LIMIT | grep -i ffffffff arch/arm/include/asm/io.h:#define IO_SPACE_LIMIT ((resource_size_t)0xff= ffffff) 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 0xffffff= ff 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 0xfffffffffffffff= fUL arch/tile/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff >=20 > 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 >=20 > Arnd >=20 >=20 --=20 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- =C2=AF\_(=E3=83=84)_/=C2=AF