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 <arnd@arndb.de>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/net/ethernet/hisilicon/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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'
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?
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
Hi Arnd,
On 10/21/2015 07:39 AM, Arnd Bergmann wrote:
quoted hunk
On Wednesday 21 October 2015 07:29:33 Guenter Roeck wrote:
quoted
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'
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?
+}
+
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
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 <arnd@arndb.de>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
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.
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
On Wednesday 21 October 2015 10:03:05 Guenter Roeck wrote:
quoted
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
On Wed, Oct 21, 2015 at 09:11:53PM +0200, Arnd Bergmann wrote:
On Wednesday 21 October 2015 10:03:05 Guenter Roeck wrote:
quoted
quoted
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.
quoted
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
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:
quoted
On Wednesday 21 October 2015 10:03:05 Guenter Roeck wrote:
quoted
quoted
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.
quoted
quoted
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
On Friday 06 November 2015 11:16:52 Guenter Roeck wrote:
On Wed, Oct 21, 2015 at 02:53:20PM -0700, Guenter Roeck wrote:
quoted
On Wed, Oct 21, 2015 at 09:11:53PM +0200, Arnd Bergmann wrote:
quoted
On Wednesday 21 October 2015 10:03:05 Guenter Roeck wrote:
quoted
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
On Friday 06 November 2015 11:16:52 Guenter Roeck wrote:
quoted
On Wed, Oct 21, 2015 at 02:53:20PM -0700, Guenter Roeck wrote:
quoted
On Wed, Oct 21, 2015 at 09:11:53PM +0200, Arnd Bergmann wrote:
quoted
On Wednesday 21 October 2015 10:03:05 Guenter Roeck wrote:
quoted
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?