Re: [PATCH -next] net: hisilicon: Never build on SPARC
On 11/06/2015 12:30 PM, Arnd Bergmann wrote: On Friday 06 November 2015 11:16:52 Guenter Roeck wrote: On Wed, Oct 21, 2015 at 02:53:20PM -0700, Guenter Roeck wrote: On Wed, Oct 21, 2015 at 09:11:53PM +0200, Arnd Bergmann wrote: On Wednesday 21 October 2015 10:03:05 Guenter Roeck wrote: Anyway, if it gets that complicated, I think we should stick with just returning OF_BAD_ADDR. The above really suggests the need for an architecture specific solution. Probably no harm in this really: the far more common of_address_to_resource() and of_iomap() helpers are equally broken on SPARC and we just return a runtime error for those as well without CONFIG_OF_ADDRESS rather than breaking the build. Agreed. Given this, returning OF_BAD_ADDR sounds like a better choice. Arnd, do you know if a fix for this problem is pending in some branch ? Mainline sparc builds are now affected. I don't think anyone wrote the patch to do this. Can you send one? I'll see what I can do. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
On Friday 06 November 2015 11:16:52 Guenter Roeck wrote: > On Wed, Oct 21, 2015 at 02:53:20PM -0700, Guenter Roeck wrote: > > On Wed, Oct 21, 2015 at 09:11:53PM +0200, Arnd Bergmann wrote: > > > On Wednesday 21 October 2015 10:03:05 Guenter Roeck wrote: > > > > Anyway, if it gets that complicated, I think we should stick with > > > > just returning OF_BAD_ADDR. The above really suggests the need for > > > > an architecture specific solution. > > > > > > Probably no harm in this really: the far more common > > > of_address_to_resource() and of_iomap() helpers are equally > > > broken on SPARC and we just return a runtime error for those > > > as well without CONFIG_OF_ADDRESS rather than breaking the build. > > > > > Agreed. Given this, returning OF_BAD_ADDR sounds like a better choice. > > > Arnd, > > do you know if a fix for this problem is pending in some branch ? > Mainline sparc builds are now affected. > I don't think anyone wrote the patch to do this. Can you send one? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
Arnd, On Wed, Oct 21, 2015 at 02:53:20PM -0700, Guenter Roeck wrote: > On Wed, Oct 21, 2015 at 09:11:53PM +0200, Arnd Bergmann wrote: > > On Wednesday 21 October 2015 10:03:05 Guenter Roeck wrote: > > > > > > > > Something like this? > > > > > > > > static inline u64 of_translate_address(struct device_node *np, const > > > > __be32 *addr) > > > > { > > > > #if defined(CONFIG_SPARC) || defined(CONFIG_M68K) > > > > int pna = of_n_addr_cells(np); > > > > u64 ret = be32_to_cpu(addr[pna - 1]); > > > > > > > > if (pna > 1) > > > > ret += (u64)be32_to_cpu(addr[pna - 2]) << 32; > > > > > > > > return ret; > > > > > > That suggests that sparc would need a translation after all, which > > > seems to contradict what David said earlier. > > > > No, not a translation: the value is used without any offset that > > factors in the location of the bus, the above is just the shortest > > possible way to read the 64-bit number from a big-endian property > > of variable length. > > > Out of my realm .. David would have to comment on that. > > > > Anyway, if it gets that complicated, I think we should stick with > > > just returning OF_BAD_ADDR. The above really suggests the need for > > > an architecture specific solution. > > > > Probably no harm in this really: the far more common > > of_address_to_resource() and of_iomap() helpers are equally > > broken on SPARC and we just return a runtime error for those > > as well without CONFIG_OF_ADDRESS rather than breaking the build. > > > Agreed. Given this, returning OF_BAD_ADDR sounds like a better choice. > Arnd, do you know if a fix for this problem is pending in some branch ? Mainline sparc builds are now affected. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
On 11/06/2015 12:30 PM, Arnd Bergmann wrote: On Friday 06 November 2015 11:16:52 Guenter Roeck wrote: On Wed, Oct 21, 2015 at 02:53:20PM -0700, Guenter Roeck wrote: On Wed, Oct 21, 2015 at 09:11:53PM +0200, Arnd Bergmann wrote: On Wednesday 21 October 2015 10:03:05 Guenter Roeck wrote: Anyway, if it gets that complicated, I think we should stick with just returning OF_BAD_ADDR. The above really suggests the need for an architecture specific solution. Probably no harm in this really: the far more common of_address_to_resource() and of_iomap() helpers are equally broken on SPARC and we just return a runtime error for those as well without CONFIG_OF_ADDRESS rather than breaking the build. Agreed. Given this, returning OF_BAD_ADDR sounds like a better choice. Arnd, do you know if a fix for this problem is pending in some branch ? Mainline sparc builds are now affected. I don't think anyone wrote the patch to do this. Can you send one? I'll see what I can do. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
Arnd, On Wed, Oct 21, 2015 at 02:53:20PM -0700, Guenter Roeck wrote: > On Wed, Oct 21, 2015 at 09:11:53PM +0200, Arnd Bergmann wrote: > > On Wednesday 21 October 2015 10:03:05 Guenter Roeck wrote: > > > > > > > > Something like this? > > > > > > > > static inline u64 of_translate_address(struct device_node *np, const > > > > __be32 *addr) > > > > { > > > > #if defined(CONFIG_SPARC) || defined(CONFIG_M68K) > > > > int pna = of_n_addr_cells(np); > > > > u64 ret = be32_to_cpu(addr[pna - 1]); > > > > > > > > if (pna > 1) > > > > ret += (u64)be32_to_cpu(addr[pna - 2]) << 32; > > > > > > > > return ret; > > > > > > That suggests that sparc would need a translation after all, which > > > seems to contradict what David said earlier. > > > > No, not a translation: the value is used without any offset that > > factors in the location of the bus, the above is just the shortest > > possible way to read the 64-bit number from a big-endian property > > of variable length. > > > Out of my realm .. David would have to comment on that. > > > > Anyway, if it gets that complicated, I think we should stick with > > > just returning OF_BAD_ADDR. The above really suggests the need for > > > an architecture specific solution. > > > > Probably no harm in this really: the far more common > > of_address_to_resource() and of_iomap() helpers are equally > > broken on SPARC and we just return a runtime error for those > > as well without CONFIG_OF_ADDRESS rather than breaking the build. > > > Agreed. Given this, returning OF_BAD_ADDR sounds like a better choice. > Arnd, do you know if a fix for this problem is pending in some branch ? Mainline sparc builds are now affected. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
On Friday 06 November 2015 11:16:52 Guenter Roeck wrote: > On Wed, Oct 21, 2015 at 02:53:20PM -0700, Guenter Roeck wrote: > > On Wed, Oct 21, 2015 at 09:11:53PM +0200, Arnd Bergmann wrote: > > > On Wednesday 21 October 2015 10:03:05 Guenter Roeck wrote: > > > > Anyway, if it gets that complicated, I think we should stick with > > > > just returning OF_BAD_ADDR. The above really suggests the need for > > > > an architecture specific solution. > > > > > > Probably no harm in this really: the far more common > > > of_address_to_resource() and of_iomap() helpers are equally > > > broken on SPARC and we just return a runtime error for those > > > as well without CONFIG_OF_ADDRESS rather than breaking the build. > > > > > Agreed. Given this, returning OF_BAD_ADDR sounds like a better choice. > > > Arnd, > > do you know if a fix for this problem is pending in some branch ? > Mainline sparc builds are now affected. > I don't think anyone wrote the patch to do this. Can you send one? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
From: Guenter Roeck Date: Wed, 21 Oct 2015 10:03:05 -0700 > On 10/21/2015 08:57 AM, Arnd Bergmann wrote: >> On Wednesday 21 October 2015 08:33:11 David Miller wrote: >>> From: Guenter Roeck >>> Date: Wed, 21 Oct 2015 07:56:18 -0700 >>> > @@ -57,6 +57,11 @@ extern int of_dma_get_range(struct device_node *np, > u64 *dma_addr, > u64 *paddr, u64 *size); >extern bool of_dma_is_coherent(struct device_node *np); >#else /* CONFIG_OF_ADDRESS */ > +static inline u64 of_translate_address(struct device_node *np, const > __be32 *addr) > +{ > +return 0; Maybe return OF_BAD_ADDR ? >>> >>> The thing to really do on sparc, is just return the address raw >>> untranslated >>> because that just works. >>> >> >> We still need to check #address-cells, right? >> >> Something like this? >> >> static inline u64 of_translate_address(struct device_node *np, const >> __be32 *addr) >> { >> #if defined(CONFIG_SPARC) || defined(CONFIG_M68K) >> int pna = of_n_addr_cells(np); >> u64 ret = be32_to_cpu(addr[pna - 1]); >> >> if (pna > 1) >> ret += (u64)be32_to_cpu(addr[pna - 2]) << 32; >> >> return ret; > > That suggests that sparc would need a translation after all, which > seems to contradict what David said earlier. It's not being translated, the code above is just figuring out what size the object in the property is. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
On Wed, Oct 21, 2015 at 09:11:53PM +0200, Arnd Bergmann wrote: > On Wednesday 21 October 2015 10:03:05 Guenter Roeck wrote: > > > > > > Something like this? > > > > > > static inline u64 of_translate_address(struct device_node *np, const > > > __be32 *addr) > > > { > > > #if defined(CONFIG_SPARC) || defined(CONFIG_M68K) > > > int pna = of_n_addr_cells(np); > > > u64 ret = be32_to_cpu(addr[pna - 1]); > > > > > > if (pna > 1) > > > ret += (u64)be32_to_cpu(addr[pna - 2]) << 32; > > > > > > return ret; > > > > That suggests that sparc would need a translation after all, which > > seems to contradict what David said earlier. > > No, not a translation: the value is used without any offset that > factors in the location of the bus, the above is just the shortest > possible way to read the 64-bit number from a big-endian property > of variable length. > Out of my realm .. David would have to comment on that. > > Anyway, if it gets that complicated, I think we should stick with > > just returning OF_BAD_ADDR. The above really suggests the need for > > an architecture specific solution. > > Probably no harm in this really: the far more common > of_address_to_resource() and of_iomap() helpers are equally > broken on SPARC and we just return a runtime error for those > as well without CONFIG_OF_ADDRESS rather than breaking the build. > Agreed. Given this, returning OF_BAD_ADDR sounds like a better choice. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
On Wednesday 21 October 2015 10:03:05 Guenter Roeck wrote: > > > > Something like this? > > > > static inline u64 of_translate_address(struct device_node *np, const __be32 > > *addr) > > { > > #if defined(CONFIG_SPARC) || defined(CONFIG_M68K) > > int pna = of_n_addr_cells(np); > > u64 ret = be32_to_cpu(addr[pna - 1]); > > > > if (pna > 1) > > ret += (u64)be32_to_cpu(addr[pna - 2]) << 32; > > > > return ret; > > That suggests that sparc would need a translation after all, which > seems to contradict what David said earlier. No, not a translation: the value is used without any offset that factors in the location of the bus, the above is just the shortest possible way to read the 64-bit number from a big-endian property of variable length. > Anyway, if it gets that complicated, I think we should stick with > just returning OF_BAD_ADDR. The above really suggests the need for > an architecture specific solution. Probably no harm in this really: the far more common of_address_to_resource() and of_iomap() helpers are equally broken on SPARC and we just return a runtime error for those as well without CONFIG_OF_ADDRESS rather than breaking the build. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
On 10/21/2015 08:57 AM, Arnd Bergmann wrote: On Wednesday 21 October 2015 08:33:11 David Miller wrote: From: Guenter Roeck Date: Wed, 21 Oct 2015 07:56:18 -0700 @@ -57,6 +57,11 @@ extern int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size); extern bool of_dma_is_coherent(struct device_node *np); #else /* CONFIG_OF_ADDRESS */ +static inline u64 of_translate_address(struct device_node *np, const __be32 *addr) +{ +return 0; Maybe return OF_BAD_ADDR ? The thing to really do on sparc, is just return the address raw untranslated because that just works. We still need to check #address-cells, right? Something like this? static inline u64 of_translate_address(struct device_node *np, const __be32 *addr) { #if defined(CONFIG_SPARC) || defined(CONFIG_M68K) int pna = of_n_addr_cells(np); u64 ret = be32_to_cpu(addr[pna - 1]); if (pna > 1) ret += (u64)be32_to_cpu(addr[pna - 2]) << 32; return ret; That suggests that sparc would need a translation after all, which seems to contradict what David said earlier. Anyway, if it gets that complicated, I think we should stick with just returning OF_BAD_ADDR. The above really suggests the need for an architecture specific solution. Guenter #else return OF_BAD_ADDR; #endif } Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
On Wednesday 21 October 2015 08:33:11 David Miller wrote: > From: Guenter Roeck > Date: Wed, 21 Oct 2015 07:56:18 -0700 > > >> @@ -57,6 +57,11 @@ extern int of_dma_get_range(struct device_node *np, > >> u64 *dma_addr, > >> u64 *paddr, u64 *size); > >> extern bool of_dma_is_coherent(struct device_node *np); > >> #else /* CONFIG_OF_ADDRESS */ > >> +static inline u64 of_translate_address(struct device_node *np, const > >> __be32 *addr) > >> +{ > >> +return 0; > > > > Maybe return OF_BAD_ADDR ? > > The thing to really do on sparc, is just return the address raw untranslated > because that just works. > We still need to check #address-cells, right? Something like this? static inline u64 of_translate_address(struct device_node *np, const __be32 *addr) { #if defined(CONFIG_SPARC) || defined(CONFIG_M68K) int pna = of_n_addr_cells(np); u64 ret = be32_to_cpu(addr[pna - 1]); if (pna > 1) ret += (u64)be32_to_cpu(addr[pna - 2]) << 32; return ret; #else return OF_BAD_ADDR; #endif } Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
From: Guenter Roeck Date: Wed, 21 Oct 2015 07:56:18 -0700 >> @@ -57,6 +57,11 @@ extern int of_dma_get_range(struct device_node *np, >> u64 *dma_addr, >> u64 *paddr, u64 *size); >> extern bool of_dma_is_coherent(struct device_node *np); >> #else /* CONFIG_OF_ADDRESS */ >> +static inline u64 of_translate_address(struct device_node *np, const >> __be32 *addr) >> +{ >> +return 0; > > Maybe return OF_BAD_ADDR ? The thing to really do on sparc, is just return the address raw untranslated because that just works. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
From: Arnd Bergmann Date: Wed, 21 Oct 2015 16:39:02 +0200 > How about this version? Yes, that is a million times better. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
From: Guenter Roeck Date: Wed, 21 Oct 2015 07:29:33 -0700 > The Hisilicon network driver does not build for Sparc. Enabling > COMPILE_TEST for it causes Sparc allmodconfig/allyesconfig builds > to fail with > > drivers/net/ethernet/hisilicon/hns_mdio.c: In function 'hns_mdio_bus_name': > drivers/net/ethernet/hisilicon/hns_mdio.c:409:3: error: > implicit declaration of function 'of_translate_address' > > Fixes: 876133d3161d ("net: hisilicon: add OF dependency") > Cc: Arnd Bergmann > Signed-off-by: Guenter Roeck I wish we would really resolve this properly instead of hacking crap like this all the time, it's stupid. SPARC simply never needs to "translate" OF addresses, since all OF resources are fully translated already at boot time during OF tree import. All IRQs are fully resolved as well. So we could simply make of_translate_address() a NOP. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
Hi Arnd, On 10/21/2015 07:39 AM, Arnd Bergmann wrote: On Wednesday 21 October 2015 07:29:33 Guenter Roeck wrote: The Hisilicon network driver does not build for Sparc. Enabling COMPILE_TEST for it causes Sparc allmodconfig/allyesconfig builds to fail with drivers/net/ethernet/hisilicon/hns_mdio.c: In function 'hns_mdio_bus_name': drivers/net/ethernet/hisilicon/hns_mdio.c:409:3: error: implicit declaration of function 'of_translate_address' I see. Fixes: 876133d3161d ("net: hisilicon: add OF dependency") Cc: Arnd Bergmann Signed-off-by: Guenter Roeck --- drivers/net/ethernet/hisilicon/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/hisilicon/Kconfig b/drivers/net/ethernet/hisilicon/Kconfig index f250dec488fd..413935085591 100644 --- a/drivers/net/ethernet/hisilicon/Kconfig +++ b/drivers/net/ethernet/hisilicon/Kconfig @@ -5,7 +5,7 @@ config NET_VENDOR_HISILICON bool "Hisilicon devices" default y - depends on OF && (ARM || ARM64 || COMPILE_TEST) + depends on OF && (ARM || ARM64 || COMPILE_TEST) && !SPARC ---help--- If you have a network (Ethernet) card belonging to this class, say Y. This looks fragile to me. Checking the declaration of of_translate_address, I see now that it actually depends on CONFIG_OF_ADDRESS, which is defined using "depends on !SPARC && HAS_IOMEM". This means we would get the same problem on SCORE, Tile, and UML. How about this version? diff --git a/include/linux/of_address.h b/include/linux/of_address.h index d88e81be6368..f2f7986cac45 100644 --- a/include/linux/of_address.h +++ b/include/linux/of_address.h @@ -57,6 +57,11 @@ extern int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size); extern bool of_dma_is_coherent(struct device_node *np); #else /* CONFIG_OF_ADDRESS */ +static inline u64 of_translate_address(struct device_node *np, const __be32 *addr) +{ + return 0; Maybe return OF_BAD_ADDR ? +} + static inline struct device_node *of_find_matching_node_by_address( struct device_node *from, const struct of_device_id *matches, It looks like it's in line with the other wrappers here. Alternatively, we could decide to use CONFIG_OF_ADDRESS instead of CONFIG_OF as the dependency. You are right, both of those would be better than my patch. My preference would be to introduce the dummy function. This would solve the problem for good (it isn't the first time this happens). Are you going to submit that patch ? Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
On Wednesday 21 October 2015 07:29:33 Guenter Roeck wrote: > The Hisilicon network driver does not build for Sparc. Enabling > COMPILE_TEST for it causes Sparc allmodconfig/allyesconfig builds > to fail with > > drivers/net/ethernet/hisilicon/hns_mdio.c: In function 'hns_mdio_bus_name': > drivers/net/ethernet/hisilicon/hns_mdio.c:409:3: error: > implicit declaration of function 'of_translate_address' I see. > Fixes: 876133d3161d ("net: hisilicon: add OF dependency") > Cc: Arnd Bergmann > Signed-off-by: Guenter Roeck > --- > drivers/net/ethernet/hisilicon/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/hisilicon/Kconfig > b/drivers/net/ethernet/hisilicon/Kconfig > index f250dec488fd..413935085591 100644 > --- a/drivers/net/ethernet/hisilicon/Kconfig > +++ b/drivers/net/ethernet/hisilicon/Kconfig > @@ -5,7 +5,7 @@ > config NET_VENDOR_HISILICON > bool "Hisilicon devices" > default y > - depends on OF && (ARM || ARM64 || COMPILE_TEST) > + depends on OF && (ARM || ARM64 || COMPILE_TEST) && !SPARC > ---help--- > If you have a network (Ethernet) card belonging to this class, say > Y. This looks fragile to me. Checking the declaration of of_translate_address, I see now that it actually depends on CONFIG_OF_ADDRESS, which is defined using "depends on !SPARC && HAS_IOMEM". This means we would get the same problem on SCORE, Tile, and UML. How about this version? diff --git a/include/linux/of_address.h b/include/linux/of_address.h index d88e81be6368..f2f7986cac45 100644 --- a/include/linux/of_address.h +++ b/include/linux/of_address.h @@ -57,6 +57,11 @@ extern int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size); extern bool of_dma_is_coherent(struct device_node *np); #else /* CONFIG_OF_ADDRESS */ +static inline u64 of_translate_address(struct device_node *np, const __be32 *addr) +{ + return 0; +} + static inline struct device_node *of_find_matching_node_by_address( struct device_node *from, const struct of_device_id *matches, It looks like it's in line with the other wrappers here. Alternatively, we could decide to use CONFIG_OF_ADDRESS instead of CONFIG_OF as the dependency. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
From: Guenter RoeckDate: Wed, 21 Oct 2015 07:56:18 -0700 >> @@ -57,6 +57,11 @@ extern int of_dma_get_range(struct device_node *np, >> u64 *dma_addr, >> u64 *paddr, u64 *size); >> extern bool of_dma_is_coherent(struct device_node *np); >> #else /* CONFIG_OF_ADDRESS */ >> +static inline u64 of_translate_address(struct device_node *np, const >> __be32 *addr) >> +{ >> +return 0; > > Maybe return OF_BAD_ADDR ? The thing to really do on sparc, is just return the address raw untranslated because that just works. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
On Wednesday 21 October 2015 07:29:33 Guenter Roeck wrote: > The Hisilicon network driver does not build for Sparc. Enabling > COMPILE_TEST for it causes Sparc allmodconfig/allyesconfig builds > to fail with > > drivers/net/ethernet/hisilicon/hns_mdio.c: In function 'hns_mdio_bus_name': > drivers/net/ethernet/hisilicon/hns_mdio.c:409:3: error: > implicit declaration of function 'of_translate_address' I see. > Fixes: 876133d3161d ("net: hisilicon: add OF dependency") > Cc: Arnd Bergmann> Signed-off-by: Guenter Roeck > --- > drivers/net/ethernet/hisilicon/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/hisilicon/Kconfig > b/drivers/net/ethernet/hisilicon/Kconfig > index f250dec488fd..413935085591 100644 > --- a/drivers/net/ethernet/hisilicon/Kconfig > +++ b/drivers/net/ethernet/hisilicon/Kconfig > @@ -5,7 +5,7 @@ > config NET_VENDOR_HISILICON > bool "Hisilicon devices" > default y > - depends on OF && (ARM || ARM64 || COMPILE_TEST) > + depends on OF && (ARM || ARM64 || COMPILE_TEST) && !SPARC > ---help--- > If you have a network (Ethernet) card belonging to this class, say > Y. This looks fragile to me. Checking the declaration of of_translate_address, I see now that it actually depends on CONFIG_OF_ADDRESS, which is defined using "depends on !SPARC && HAS_IOMEM". This means we would get the same problem on SCORE, Tile, and UML. How about this version? diff --git a/include/linux/of_address.h b/include/linux/of_address.h index d88e81be6368..f2f7986cac45 100644 --- a/include/linux/of_address.h +++ b/include/linux/of_address.h @@ -57,6 +57,11 @@ extern int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size); extern bool of_dma_is_coherent(struct device_node *np); #else /* CONFIG_OF_ADDRESS */ +static inline u64 of_translate_address(struct device_node *np, const __be32 *addr) +{ + return 0; +} + static inline struct device_node *of_find_matching_node_by_address( struct device_node *from, const struct of_device_id *matches, It looks like it's in line with the other wrappers here. Alternatively, we could decide to use CONFIG_OF_ADDRESS instead of CONFIG_OF as the dependency. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
Hi Arnd, On 10/21/2015 07:39 AM, Arnd Bergmann wrote: On Wednesday 21 October 2015 07:29:33 Guenter Roeck wrote: The Hisilicon network driver does not build for Sparc. Enabling COMPILE_TEST for it causes Sparc allmodconfig/allyesconfig builds to fail with drivers/net/ethernet/hisilicon/hns_mdio.c: In function 'hns_mdio_bus_name': drivers/net/ethernet/hisilicon/hns_mdio.c:409:3: error: implicit declaration of function 'of_translate_address' I see. Fixes: 876133d3161d ("net: hisilicon: add OF dependency") Cc: Arnd BergmannSigned-off-by: Guenter Roeck --- drivers/net/ethernet/hisilicon/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/hisilicon/Kconfig b/drivers/net/ethernet/hisilicon/Kconfig index f250dec488fd..413935085591 100644 --- a/drivers/net/ethernet/hisilicon/Kconfig +++ b/drivers/net/ethernet/hisilicon/Kconfig @@ -5,7 +5,7 @@ config NET_VENDOR_HISILICON bool "Hisilicon devices" default y - depends on OF && (ARM || ARM64 || COMPILE_TEST) + depends on OF && (ARM || ARM64 || COMPILE_TEST) && !SPARC ---help--- If you have a network (Ethernet) card belonging to this class, say Y. This looks fragile to me. Checking the declaration of of_translate_address, I see now that it actually depends on CONFIG_OF_ADDRESS, which is defined using "depends on !SPARC && HAS_IOMEM". This means we would get the same problem on SCORE, Tile, and UML. How about this version? diff --git a/include/linux/of_address.h b/include/linux/of_address.h index d88e81be6368..f2f7986cac45 100644 --- a/include/linux/of_address.h +++ b/include/linux/of_address.h @@ -57,6 +57,11 @@ extern int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size); extern bool of_dma_is_coherent(struct device_node *np); #else /* CONFIG_OF_ADDRESS */ +static inline u64 of_translate_address(struct device_node *np, const __be32 *addr) +{ + return 0; Maybe return OF_BAD_ADDR ? +} + static inline struct device_node *of_find_matching_node_by_address( struct device_node *from, const struct of_device_id *matches, It looks like it's in line with the other wrappers here. Alternatively, we could decide to use CONFIG_OF_ADDRESS instead of CONFIG_OF as the dependency. You are right, both of those would be better than my patch. My preference would be to introduce the dummy function. This would solve the problem for good (it isn't the first time this happens). Are you going to submit that patch ? Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
From: Guenter RoeckDate: Wed, 21 Oct 2015 07:29:33 -0700 > The Hisilicon network driver does not build for Sparc. Enabling > COMPILE_TEST for it causes Sparc allmodconfig/allyesconfig builds > to fail with > > drivers/net/ethernet/hisilicon/hns_mdio.c: In function 'hns_mdio_bus_name': > drivers/net/ethernet/hisilicon/hns_mdio.c:409:3: error: > implicit declaration of function 'of_translate_address' > > Fixes: 876133d3161d ("net: hisilicon: add OF dependency") > Cc: Arnd Bergmann > Signed-off-by: Guenter Roeck I wish we would really resolve this properly instead of hacking crap like this all the time, it's stupid. SPARC simply never needs to "translate" OF addresses, since all OF resources are fully translated already at boot time during OF tree import. All IRQs are fully resolved as well. So we could simply make of_translate_address() a NOP. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
From: Arnd BergmannDate: Wed, 21 Oct 2015 16:39:02 +0200 > How about this version? Yes, that is a million times better. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
On Wednesday 21 October 2015 08:33:11 David Miller wrote: > From: Guenter Roeck> Date: Wed, 21 Oct 2015 07:56:18 -0700 > > >> @@ -57,6 +57,11 @@ extern int of_dma_get_range(struct device_node *np, > >> u64 *dma_addr, > >> u64 *paddr, u64 *size); > >> extern bool of_dma_is_coherent(struct device_node *np); > >> #else /* CONFIG_OF_ADDRESS */ > >> +static inline u64 of_translate_address(struct device_node *np, const > >> __be32 *addr) > >> +{ > >> +return 0; > > > > Maybe return OF_BAD_ADDR ? > > The thing to really do on sparc, is just return the address raw untranslated > because that just works. > We still need to check #address-cells, right? Something like this? static inline u64 of_translate_address(struct device_node *np, const __be32 *addr) { #if defined(CONFIG_SPARC) || defined(CONFIG_M68K) int pna = of_n_addr_cells(np); u64 ret = be32_to_cpu(addr[pna - 1]); if (pna > 1) ret += (u64)be32_to_cpu(addr[pna - 2]) << 32; return ret; #else return OF_BAD_ADDR; #endif } Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
On 10/21/2015 08:57 AM, Arnd Bergmann wrote: On Wednesday 21 October 2015 08:33:11 David Miller wrote: From: Guenter RoeckDate: Wed, 21 Oct 2015 07:56:18 -0700 @@ -57,6 +57,11 @@ extern int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size); extern bool of_dma_is_coherent(struct device_node *np); #else /* CONFIG_OF_ADDRESS */ +static inline u64 of_translate_address(struct device_node *np, const __be32 *addr) +{ +return 0; Maybe return OF_BAD_ADDR ? The thing to really do on sparc, is just return the address raw untranslated because that just works. We still need to check #address-cells, right? Something like this? static inline u64 of_translate_address(struct device_node *np, const __be32 *addr) { #if defined(CONFIG_SPARC) || defined(CONFIG_M68K) int pna = of_n_addr_cells(np); u64 ret = be32_to_cpu(addr[pna - 1]); if (pna > 1) ret += (u64)be32_to_cpu(addr[pna - 2]) << 32; return ret; That suggests that sparc would need a translation after all, which seems to contradict what David said earlier. Anyway, if it gets that complicated, I think we should stick with just returning OF_BAD_ADDR. The above really suggests the need for an architecture specific solution. Guenter #else return OF_BAD_ADDR; #endif } Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
On Wednesday 21 October 2015 10:03:05 Guenter Roeck wrote: > > > > Something like this? > > > > static inline u64 of_translate_address(struct device_node *np, const __be32 > > *addr) > > { > > #if defined(CONFIG_SPARC) || defined(CONFIG_M68K) > > int pna = of_n_addr_cells(np); > > u64 ret = be32_to_cpu(addr[pna - 1]); > > > > if (pna > 1) > > ret += (u64)be32_to_cpu(addr[pna - 2]) << 32; > > > > return ret; > > That suggests that sparc would need a translation after all, which > seems to contradict what David said earlier. No, not a translation: the value is used without any offset that factors in the location of the bus, the above is just the shortest possible way to read the 64-bit number from a big-endian property of variable length. > Anyway, if it gets that complicated, I think we should stick with > just returning OF_BAD_ADDR. The above really suggests the need for > an architecture specific solution. Probably no harm in this really: the far more common of_address_to_resource() and of_iomap() helpers are equally broken on SPARC and we just return a runtime error for those as well without CONFIG_OF_ADDRESS rather than breaking the build. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
On Wed, Oct 21, 2015 at 09:11:53PM +0200, Arnd Bergmann wrote: > On Wednesday 21 October 2015 10:03:05 Guenter Roeck wrote: > > > > > > Something like this? > > > > > > static inline u64 of_translate_address(struct device_node *np, const > > > __be32 *addr) > > > { > > > #if defined(CONFIG_SPARC) || defined(CONFIG_M68K) > > > int pna = of_n_addr_cells(np); > > > u64 ret = be32_to_cpu(addr[pna - 1]); > > > > > > if (pna > 1) > > > ret += (u64)be32_to_cpu(addr[pna - 2]) << 32; > > > > > > return ret; > > > > That suggests that sparc would need a translation after all, which > > seems to contradict what David said earlier. > > No, not a translation: the value is used without any offset that > factors in the location of the bus, the above is just the shortest > possible way to read the 64-bit number from a big-endian property > of variable length. > Out of my realm .. David would have to comment on that. > > Anyway, if it gets that complicated, I think we should stick with > > just returning OF_BAD_ADDR. The above really suggests the need for > > an architecture specific solution. > > Probably no harm in this really: the far more common > of_address_to_resource() and of_iomap() helpers are equally > broken on SPARC and we just return a runtime error for those > as well without CONFIG_OF_ADDRESS rather than breaking the build. > Agreed. Given this, returning OF_BAD_ADDR sounds like a better choice. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] net: hisilicon: Never build on SPARC
From: Guenter RoeckDate: Wed, 21 Oct 2015 10:03:05 -0700 > On 10/21/2015 08:57 AM, Arnd Bergmann wrote: >> On Wednesday 21 October 2015 08:33:11 David Miller wrote: >>> From: Guenter Roeck >>> Date: Wed, 21 Oct 2015 07:56:18 -0700 >>> > @@ -57,6 +57,11 @@ extern int of_dma_get_range(struct device_node *np, > u64 *dma_addr, > u64 *paddr, u64 *size); >extern bool of_dma_is_coherent(struct device_node *np); >#else /* CONFIG_OF_ADDRESS */ > +static inline u64 of_translate_address(struct device_node *np, const > __be32 *addr) > +{ > +return 0; Maybe return OF_BAD_ADDR ? >>> >>> The thing to really do on sparc, is just return the address raw >>> untranslated >>> because that just works. >>> >> >> We still need to check #address-cells, right? >> >> Something like this? >> >> static inline u64 of_translate_address(struct device_node *np, const >> __be32 *addr) >> { >> #if defined(CONFIG_SPARC) || defined(CONFIG_M68K) >> int pna = of_n_addr_cells(np); >> u64 ret = be32_to_cpu(addr[pna - 1]); >> >> if (pna > 1) >> ret += (u64)be32_to_cpu(addr[pna - 2]) << 32; >> >> return ret; > > That suggests that sparc would need a translation after all, which > seems to contradict what David said earlier. It's not being translated, the code above is just figuring out what size the object in the property is. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/