2021-09-28 20:15:22

by Tom Rix

[permalink] [raw]
Subject: [PATCH] of: remove duplicate declaration of of_iomap()

From: Tom Rix <[email protected]>

A ranconfig produces this linker error
irq-al-fic.c:252: undefined reference to `of_iomap'

The declaration of of_iomap() is dependent on OF
The definition of of_iomap() is dependent on OF_ADDRESS
These should match. There are duplicate declarations
of of_iomap(), remove of_iomap() and the
of_address_to_resource() duplicate.

Signed-off-by: Tom Rix <[email protected]>
---
include/linux/of_address.h | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 45598dbec269..a190996b4b0b 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -122,13 +122,7 @@ static inline bool of_dma_is_coherent(struct device_node *np)
{
return false;
}
-#endif /* CONFIG_OF_ADDRESS */

-#ifdef CONFIG_OF
-extern int of_address_to_resource(struct device_node *dev, int index,
- struct resource *r);
-void __iomem *of_iomap(struct device_node *node, int index);
-#else
static inline int of_address_to_resource(struct device_node *dev, int index,
struct resource *r)
{
@@ -139,7 +133,7 @@ static inline void __iomem *of_iomap(struct device_node *device, int index)
{
return NULL;
}
-#endif
+#endif /* CONFIG_OF_ADDRESS */
#define of_range_parser_init of_pci_range_parser_init

static inline const __be32 *of_get_address(struct device_node *dev, int index,
--
2.26.3


2021-09-28 20:37:48

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] of: remove duplicate declaration of of_iomap()

On 9/28/21 1:12 PM, [email protected] wrote:
> From: Tom Rix <[email protected]>
>
> A ranconfig produces this linker error

randconfig .. on what arch, please?

> irq-al-fic.c:252: undefined reference to `of_iomap'
>
> The declaration of of_iomap() is dependent on OF
> The definition of of_iomap() is dependent on OF_ADDRESS
> These should match. There are duplicate declarations
> of of_iomap(), remove of_iomap() and the
> of_address_to_resource() duplicate.
>
> Signed-off-by: Tom Rix <[email protected]>
> ---
> include/linux/of_address.h | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>


thanks.
--
~Randy

2021-09-28 20:46:58

by Tom Rix

[permalink] [raw]
Subject: Re: [PATCH] of: remove duplicate declaration of of_iomap()


On 9/28/21 1:33 PM, Randy Dunlap wrote:
> On 9/28/21 1:12 PM, [email protected] wrote:
>> From: Tom Rix <[email protected]>
>>
>> A ranconfig produces this linker error
>
> randconfig .. on what arch, please?
s390
>
>> irq-al-fic.c:252: undefined reference to `of_iomap'
>>
>> The declaration of of_iomap() is dependent on OF
>> The definition of of_iomap() is dependent on OF_ADDRESS
>> These should match.  There are duplicate declarations
>> of of_iomap(), remove of_iomap() and the
>> of_address_to_resource() duplicate.
>>
>> Signed-off-by: Tom Rix <[email protected]>
>> ---
>>   include/linux/of_address.h | 8 +-------
>>   1 file changed, 1 insertion(+), 7 deletions(-)
>>
>
>
> thanks.

2021-09-29 16:01:11

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] of: remove duplicate declaration of of_iomap()

On Wed, Sep 29, 2021 at 9:26 AM Rob Herring <[email protected]> wrote:
>
> On Tue, Sep 28, 2021 at 3:12 PM <[email protected]> wrote:
> >
> > From: Tom Rix <[email protected]>
> >
> > A ranconfig produces this linker error
> > irq-al-fic.c:252: undefined reference to `of_iomap'
> >
> > The declaration of of_iomap() is dependent on OF
> > The definition of of_iomap() is dependent on OF_ADDRESS
> > These should match. There are duplicate declarations
> > of of_iomap(), remove of_iomap() and the
> > of_address_to_resource() duplicate.
> >
> > Signed-off-by: Tom Rix <[email protected]>
> > ---
> > include/linux/of_address.h | 8 +-------
> > 1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> > index 45598dbec269..a190996b4b0b 100644
> > --- a/include/linux/of_address.h
> > +++ b/include/linux/of_address.h
> > @@ -122,13 +122,7 @@ static inline bool of_dma_is_coherent(struct device_node *np)
> > {
> > return false;
> > }
> > -#endif /* CONFIG_OF_ADDRESS */
> >
> > -#ifdef CONFIG_OF
> > -extern int of_address_to_resource(struct device_node *dev, int index,
> > - struct resource *r);
> > -void __iomem *of_iomap(struct device_node *node, int index);
>
> This is going to break sparc which has !OF_ADDRESS and its own
> of_iomap and of_address_to_resource implementations. I don't want to
> add CONFIG_SPARC in here, so I think we should solve this in kconfig.
> OF and !OF_ADDRESS is supposed to mean the arch provides these
> functions.
>
> I'd really like to do away with HAS_IOMEM. It doesn't serve much
> purpose other than disabling a bunch of drivers.

Following UML (the only other arch with configurable HAS_IOMEM), I
think dummy ioremap() implementations is the way to go here. UML added
its own dummy functions, but it would be better if we can just do
this:

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index e93375c710b9..a9d02403b2fe 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -946,7 +946,7 @@ static inline void *phys_to_virt(unsigned long address)
* can provide stricter non-posted write semantics if the architecture
* implements them.
*/
-#ifndef CONFIG_MMU
+#if !defined(CONFIG_GENERIC_IOREMAP)
#ifndef ioremap
#define ioremap ioremap
static inline void __iomem *ioremap(phys_addr_t offset, size_t size)
@@ -961,7 +961,7 @@ static inline void iounmap(void __iomem *addr)
{
}
#endif
-#elif defined(CONFIG_GENERIC_IOREMAP)
+#else /* CONFIG_GENERIC_IOREMAP */
#include <linux/pgtable.h>

void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot);
@@ -972,7 +972,7 @@ static inline void __iomem *ioremap(phys_addr_t
addr, size_t size)
/* _PAGE_IOREMAP needs to be supplied by the architecture */
return ioremap_prot(addr, size, _PAGE_IOREMAP);
}
-#endif /* !CONFIG_MMU || CONFIG_GENERIC_IOREMAP */
+#endif /* CONFIG_GENERIC_IOREMAP */

#ifndef ioremap_wc
#define ioremap_wc ioremap


Then we can apply the following patch:

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 3dfeae8912df..4a2453e4d4f3 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -64,7 +64,7 @@ config OF_DYNAMIC

config OF_ADDRESS
def_bool y
- depends on !SPARC && (HAS_IOMEM || UML)
+ depends on !SPARC

config OF_IRQ
def_bool y

2021-09-29 17:16:24

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] of: remove duplicate declaration of of_iomap()

On Tue, Sep 28, 2021 at 3:12 PM <[email protected]> wrote:
>
> From: Tom Rix <[email protected]>
>
> A ranconfig produces this linker error
> irq-al-fic.c:252: undefined reference to `of_iomap'
>
> The declaration of of_iomap() is dependent on OF
> The definition of of_iomap() is dependent on OF_ADDRESS
> These should match. There are duplicate declarations
> of of_iomap(), remove of_iomap() and the
> of_address_to_resource() duplicate.
>
> Signed-off-by: Tom Rix <[email protected]>
> ---
> include/linux/of_address.h | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> index 45598dbec269..a190996b4b0b 100644
> --- a/include/linux/of_address.h
> +++ b/include/linux/of_address.h
> @@ -122,13 +122,7 @@ static inline bool of_dma_is_coherent(struct device_node *np)
> {
> return false;
> }
> -#endif /* CONFIG_OF_ADDRESS */
>
> -#ifdef CONFIG_OF
> -extern int of_address_to_resource(struct device_node *dev, int index,
> - struct resource *r);
> -void __iomem *of_iomap(struct device_node *node, int index);

This is going to break sparc which has !OF_ADDRESS and its own
of_iomap and of_address_to_resource implementations. I don't want to
add CONFIG_SPARC in here, so I think we should solve this in kconfig.
OF and !OF_ADDRESS is supposed to mean the arch provides these
functions.

I'd really like to do away with HAS_IOMEM. It doesn't serve much
purpose other than disabling a bunch of drivers.

> -#else
> static inline int of_address_to_resource(struct device_node *dev, int index,
> struct resource *r)
> {
> @@ -139,7 +133,7 @@ static inline void __iomem *of_iomap(struct device_node *device, int index)
> {
> return NULL;
> }
> -#endif
> +#endif /* CONFIG_OF_ADDRESS */
> #define of_range_parser_init of_pci_range_parser_init
>
> static inline const __be32 *of_get_address(struct device_node *dev, int index,
> --
> 2.26.3
>