[PATCH 0/3] of: Increase compile coverage

[PATCH 0/3] of: Increase compile coverage

[Geert Uytterhoeven]

	Hi,

Currently the OF configuration symbol is explicitly selected on
architectures that support device trees and/or Open Firmware.
However, there's no technical reason to limit the device tree
infrastructure to these architectures.

Hence this series allows to enable OF when compile testing, to increase
compile coverage. It also fixes the fallout I encountered when IRQ
domain support is not enabled.

Geert Uytterhoeven (3):
  of: Allow OF to be enabled if COMPILE_TEST to increase coverage
  of: OF_IRQ should depend on IRQ_DOMAIN
  of: Add dummy of_irq_to_resource_table() for IRQ_OF=n

 drivers/of/Kconfig     | 8 ++++++--
 include/linux/of_irq.h | 9 +++++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

-- 
1.9.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH 1/3] of: Allow OF to be enabled if COMPILE_TEST to increase coverage

[Geert Uytterhoeven]

Currently the OF configuration symbol is explicitly selected on
architectures that support device trees and/or Open Firmware.
However, there's no technical reason to limit the device tree
infrastructure to these architectures. Hence allow OF to be enabled when
compile testing, to increase compile coverage.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/of/Kconfig | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 7bcaeec876c0c3a5..4c98f14694458794 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -2,7 +2,11 @@ config DTC
 	bool
 
 config OF
-	bool
+	bool "Device Tree and Open Firmware support" if COMPILE_TEST
+	help
+	  This option enables the device tree infrastructure.
+	  If is automatically selected by platforms that need it, but can
+	  be enabled manually to increase compile-coverage.
 
 menu "Device Tree and Open Firmware support"
 	depends on OF
-- 
1.9.1

[PATCH 2/3] of: OF_IRQ should depend on IRQ_DOMAIN

[Geert Uytterhoeven]

If CONFIG_IRQ_DOMAIN=n:

drivers/of/irq.c: In function ‘of_irq_get’:
drivers/of/irq.c:406: error: implicit declaration of function ‘irq_find_host’
drivers/of/irq.c:406: warning: assignment makes pointer from integer without a cast
make[2]: *** [drivers/of/irq.o] Error 1

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/of/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 4c98f14694458794..92adecd3ecb28fc7 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -50,7 +50,7 @@ config OF_ADDRESS_PCI
 
 config OF_IRQ
 	def_bool y
-	depends on !SPARC
+	depends on !SPARC && IRQ_DOMAIN
 
 config OF_NET
 	depends on NETDEVICES
-- 
1.9.1

[PATCH 3/3] of: Add dummy of_irq_to_resource_table() for IRQ_OF=n

[Geert Uytterhoeven]

If CONFIG_IRQ_OF=n:

drivers/built-in.o: In function `of_device_alloc':
(.text+0x72bce): undefined reference to `of_irq_to_resource_table'
make: *** [vmlinux] Error 1

of_device_alloc() calls of_irq_to_resource_table() with nr_irqs = 0 due
to of_irq_count() already being a dummy, so just add a dummy for
of_irq_to_resource_table(), too.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 include/linux/of_irq.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
index bfec136a6d1e60f8..d884929a77479782 100644
--- a/include/linux/of_irq.h
+++ b/include/linux/of_irq.h
@@ -37,8 +37,6 @@ extern int of_irq_parse_one(struct device_node *device, int index,
 extern unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data);
 extern int of_irq_to_resource(struct device_node *dev, int index,
 			      struct resource *r);
-extern int of_irq_to_resource_table(struct device_node *dev,
-		struct resource *res, int nr_irqs);
 
 extern void of_irq_init(const struct of_device_id *matches);
 
@@ -46,6 +44,8 @@ extern void of_irq_init(const struct of_device_id *matches);
 extern int of_irq_count(struct device_node *dev);
 extern int of_irq_get(struct device_node *dev, int index);
 extern int of_irq_get_byname(struct device_node *dev, const char *name);
+extern int of_irq_to_resource_table(struct device_node *dev,
+		struct resource *res, int nr_irqs);
 #else
 static inline int of_irq_count(struct device_node *dev)
 {
@@ -59,6 +59,11 @@ static inline int of_irq_get_byname(struct device_node *dev, const char *name)
 {
 	return 0;
 }
+static inline int of_irq_to_resource_table(struct device_node *dev,
+					   struct resource *res, int nr_irqs)
+{
+	return 0;
+}
 #endif
 
 #if defined(CONFIG_OF)
-- 
1.9.1

Re: [PATCH 1/3] of: Allow OF to be enabled if COMPILE_TEST to increase coverage

[Rob Herring]

+Pantelis

On Sun, Apr 5, 2015 at 9:59 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Currently the OF configuration symbol is explicitly selected on
> architectures that support device trees and/or Open Firmware.
> However, there's no technical reason to limit the device tree
> infrastructure to these architectures. Hence allow OF to be enabled when
> compile testing, to increase compile coverage.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>  drivers/of/Kconfig | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index 7bcaeec876c0c3a5..4c98f14694458794 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -2,7 +2,11 @@ config DTC
>         bool
>
>  config OF
> -       bool
> +       bool "Device Tree and Open Firmware support" if COMPILE_TEST

Actually, I think we want to just make this always visible. There are
now use cases with overlays where we may want to enable DT even if the
architecture is not booting with DT. However, there may be more work
needed to do that.

Rob

> +       help
> +         This option enables the device tree infrastructure.
> +         If is automatically selected by platforms that need it, but can
> +         be enabled manually to increase compile-coverage.
>
>  menu "Device Tree and Open Firmware support"
>         depends on OF
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] of: OF_IRQ should depend on IRQ_DOMAIN

[Arnd Bergmann]

On Sunday 05 April 2015 16:59:24 Geert Uytterhoeven wrote:
> If CONFIG_IRQ_DOMAIN=n:
> 
> drivers/of/irq.c: In function ‘of_irq_get’:
> drivers/of/irq.c:406: error: implicit declaration of function ‘irq_find_host’
> drivers/of/irq.c:406: warning: assignment makes pointer from integer without a cast
> make[2]: *** [drivers/of/irq.o] Error 1
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>  drivers/of/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index 4c98f14694458794..92adecd3ecb28fc7 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -50,7 +50,7 @@ config OF_ADDRESS_PCI
>  
>  config OF_IRQ
>         def_bool y
> -       depends on !SPARC
> +       depends on !SPARC && IRQ_DOMAIN
>  
>  config OF_NET
>         depends on NETDEVICES
> 

Sparc does not set IRQ_DOMAIN, so we can probably simplify this to 

config OF_IRQ
	def_bool IRQ_DOMAIN

unless you want to keep the sparc antidependency explicit.

	Arnd

Re: [PATCH 2/3] of: OF_IRQ should depend on IRQ_DOMAIN

[Rob Herring]

On Mon, Apr 6, 2015 at 9:40 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Sunday 05 April 2015 16:59:24 Geert Uytterhoeven wrote:
>> If CONFIG_IRQ_DOMAIN=n:
>>
>> drivers/of/irq.c: In function ‘of_irq_get’:
>> drivers/of/irq.c:406: error: implicit declaration of function ‘irq_find_host’
>> drivers/of/irq.c:406: warning: assignment makes pointer from integer without a cast
>> make[2]: *** [drivers/of/irq.o] Error 1
>>
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>>  drivers/of/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>> index 4c98f14694458794..92adecd3ecb28fc7 100644
>> --- a/drivers/of/Kconfig
>> +++ b/drivers/of/Kconfig
>> @@ -50,7 +50,7 @@ config OF_ADDRESS_PCI
>>
>>  config OF_IRQ
>>         def_bool y
>> -       depends on !SPARC
>> +       depends on !SPARC && IRQ_DOMAIN
>>
>>  config OF_NET
>>         depends on NETDEVICES
>>
>
> Sparc does not set IRQ_DOMAIN, so we can probably simplify this to
>
> config OF_IRQ
>         def_bool IRQ_DOMAIN
>
> unless you want to keep the sparc antidependency explicit.

IRQ_DOMAIN is selected in quite a few places. We'd need to make sure
none of those can be selected by Sparc.

Rob

Re: [PATCH 2/3] of: OF_IRQ should depend on IRQ_DOMAIN

[Geert Uytterhoeven]

Hi Arnd,

On Mon, Apr 6, 2015 at 4:40 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> --- a/drivers/of/Kconfig
>> +++ b/drivers/of/Kconfig
>> @@ -50,7 +50,7 @@ config OF_ADDRESS_PCI
>>
>>  config OF_IRQ
>>         def_bool y
>> -       depends on !SPARC
>> +       depends on !SPARC && IRQ_DOMAIN
>>
>>  config OF_NET
>>         depends on NETDEVICES
>>
>
> Sparc does not set IRQ_DOMAIN, so we can probably simplify this to
>
> config OF_IRQ
>         def_bool IRQ_DOMAIN
>
> unless you want to keep the sparc antidependency explicit.

Given drivers/base/platform.c:

int platform_get_irq(struct platform_device *dev, unsigned int num)
{
#ifdef CONFIG_SPARC
        ...
#else
        struct resource *r;
        if (IS_ENABLED(CONFIG_OF_IRQ) && dev->dev.of_node) {
                ...
        }
        ...
#endif
}

yes I do want to keep it explicit.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/3] of: OF_IRQ should depend on IRQ_DOMAIN

[Arnd Bergmann]

On Monday 06 April 2015 17:26:57 Geert Uytterhoeven wrote:
> Given drivers/base/platform.c:
> 
> int platform_get_irq(struct platform_device *dev, unsigned int num)
> {
> #ifdef CONFIG_SPARC
>         ...
> #else
>         struct resource *r;
>         if (IS_ENABLED(CONFIG_OF_IRQ) && dev->dev.of_node) {
>                 ...
>         }
>         ...
> #endif
> }
> 
> yes I do want to keep it explicit.
> 

Ok, makes sense, thanks!

	Arnd

Re: [PATCH 2/3] of: OF_IRQ should depend on IRQ_DOMAIN

[Rob Herring]

On Sun, Apr 5, 2015 at 9:59 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> If CONFIG_IRQ_DOMAIN=n:
>
> drivers/of/irq.c: In function ‘of_irq_get’:
> drivers/of/irq.c:406: error: implicit declaration of function ‘irq_find_host’
> drivers/of/irq.c:406: warning: assignment makes pointer from integer without a cast
> make[2]: *** [drivers/of/irq.o] Error 1
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

Applied. Thanks.

Rob

> ---
>  drivers/of/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index 4c98f14694458794..92adecd3ecb28fc7 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -50,7 +50,7 @@ config OF_ADDRESS_PCI
>
>  config OF_IRQ
>         def_bool y
> -       depends on !SPARC
> +       depends on !SPARC && IRQ_DOMAIN
>
>  config OF_NET
>         depends on NETDEVICES
> --
> 1.9.1
>

Re: [PATCH 3/3] of: Add dummy of_irq_to_resource_table() for IRQ_OF=n

[Rob Herring]

On Sun, Apr 5, 2015 at 9:59 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> If CONFIG_IRQ_OF=n:
>
> drivers/built-in.o: In function `of_device_alloc':
> (.text+0x72bce): undefined reference to `of_irq_to_resource_table'
> make: *** [vmlinux] Error 1
>
> of_device_alloc() calls of_irq_to_resource_table() with nr_irqs = 0 due
> to of_irq_count() already being a dummy, so just add a dummy for
> of_irq_to_resource_table(), too.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

Applied. Thanks.

Rob

> ---
>  include/linux/of_irq.h | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
> index bfec136a6d1e60f8..d884929a77479782 100644
> --- a/include/linux/of_irq.h
> +++ b/include/linux/of_irq.h
> @@ -37,8 +37,6 @@ extern int of_irq_parse_one(struct device_node *device, int index,
>  extern unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data);
>  extern int of_irq_to_resource(struct device_node *dev, int index,
>                               struct resource *r);
> -extern int of_irq_to_resource_table(struct device_node *dev,
> -               struct resource *res, int nr_irqs);
>
>  extern void of_irq_init(const struct of_device_id *matches);
>
> @@ -46,6 +44,8 @@ extern void of_irq_init(const struct of_device_id *matches);
>  extern int of_irq_count(struct device_node *dev);
>  extern int of_irq_get(struct device_node *dev, int index);
>  extern int of_irq_get_byname(struct device_node *dev, const char *name);
> +extern int of_irq_to_resource_table(struct device_node *dev,
> +               struct resource *res, int nr_irqs);
>  #else
>  static inline int of_irq_count(struct device_node *dev)
>  {
> @@ -59,6 +59,11 @@ static inline int of_irq_get_byname(struct device_node *dev, const char *name)
>  {
>         return 0;
>  }
> +static inline int of_irq_to_resource_table(struct device_node *dev,
> +                                          struct resource *res, int nr_irqs)
> +{
> +       return 0;
> +}
>  #endif
>
>  #if defined(CONFIG_OF)
> --
> 1.9.1
>

Re: [PATCH 1/3] of: Allow OF to be enabled if COMPILE_TEST to increase coverage

[Pantelis Antoniou]

Hi Rob,

> On Apr 6, 2015, at 16:13 , Rob Herring <robherring2@gmail.com> wrote:
> 
> +Pantelis
> 
> On Sun, Apr 5, 2015 at 9:59 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> Currently the OF configuration symbol is explicitly selected on
>> architectures that support device trees and/or Open Firmware.
>> However, there's no technical reason to limit the device tree
>> infrastructure to these architectures. Hence allow OF to be enabled when
>> compile testing, to increase compile coverage.
>> 
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>> drivers/of/Kconfig | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>> index 7bcaeec876c0c3a5..4c98f14694458794 100644
>> --- a/drivers/of/Kconfig
>> +++ b/drivers/of/Kconfig
>> @@ -2,7 +2,11 @@ config DTC
>>        bool
>> 
>> config OF
>> -       bool
>> +       bool "Device Tree and Open Firmware support" if COMPILE_TEST
> 
> Actually, I think we want to just make this always visible. There are
> now use cases with overlays where we may want to enable DT even if the
> architecture is not booting with DT. However, there may be more work
> needed to do that.
> 

We have to decouple the notion of CONFIG_OF of the following two different things.

1. I’m booting using a device tree blob.
2. I’m using the in-kernel live device tree to represent my hardware device state.

so CONFIG_OF_DTB_BLOB for the first case, and CONFIG_OF for the second.

Up to now we bundle those two together.

> Rob
> 

Regards

— Pantelis

>> +       help
>> +         This option enables the device tree infrastructure.
>> +         If is automatically selected by platforms that need it, but can
>> +         be enabled manually to increase compile-coverage.
>> 
>> menu "Device Tree and Open Firmware support"
>>        depends on OF
>> --
>> 1.9.1
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] of: Allow OF to be enabled if COMPILE_TEST to increase coverage

[Rob Herring]

On Thu, Apr 23, 2015 at 7:03 AM, Pantelis Antoniou
<pantelis.antoniou@konsulko.com> wrote:
> Hi Rob,
>
>> On Apr 6, 2015, at 16:13 , Rob Herring <robherring2@gmail.com> wrote:
>>
>> +Pantelis
>>
>> On Sun, Apr 5, 2015 at 9:59 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>> Currently the OF configuration symbol is explicitly selected on
>>> architectures that support device trees and/or Open Firmware.
>>> However, there's no technical reason to limit the device tree
>>> infrastructure to these architectures. Hence allow OF to be enabled when
>>> compile testing, to increase compile coverage.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>> ---
>>> drivers/of/Kconfig | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>>> index 7bcaeec876c0c3a5..4c98f14694458794 100644
>>> --- a/drivers/of/Kconfig
>>> +++ b/drivers/of/Kconfig
>>> @@ -2,7 +2,11 @@ config DTC
>>>        bool
>>>
>>> config OF
>>> -       bool
>>> +       bool "Device Tree and Open Firmware support" if COMPILE_TEST
>>
>> Actually, I think we want to just make this always visible. There are
>> now use cases with overlays where we may want to enable DT even if the
>> architecture is not booting with DT. However, there may be more work
>> needed to do that.
>>
>
> We have to decouple the notion of CONFIG_OF of the following two different things.
>
> 1. I’m booting using a device tree blob.
> 2. I’m using the in-kernel live device tree to represent my hardware device state.
>
> so CONFIG_OF_DTB_BLOB for the first case, and CONFIG_OF for the second.
>
> Up to now we bundle those two together.

They are separate already with CONFIG_OF_EARLY_FLATTREE

Rob

Re: [PATCH 1/3] of: Allow OF to be enabled if COMPILE_TEST to increase coverage

[Pantelis Antoniou]

Hi Rob,

> On Apr 23, 2015, at 21:22 , Rob Herring <robherring2@gmail.com> wrote:
> 
> On Thu, Apr 23, 2015 at 7:03 AM, Pantelis Antoniou
> <pantelis.antoniou@konsulko.com> wrote:
>> Hi Rob,
>> 
>>> On Apr 6, 2015, at 16:13 , Rob Herring <robherring2@gmail.com> wrote:
>>> 
>>> +Pantelis
>>> 
>>> On Sun, Apr 5, 2015 at 9:59 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>> Currently the OF configuration symbol is explicitly selected on
>>>> architectures that support device trees and/or Open Firmware.
>>>> However, there's no technical reason to limit the device tree
>>>> infrastructure to these architectures. Hence allow OF to be enabled when
>>>> compile testing, to increase compile coverage.
>>>> 
>>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>>> ---
>>>> drivers/of/Kconfig | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>>>> index 7bcaeec876c0c3a5..4c98f14694458794 100644
>>>> --- a/drivers/of/Kconfig
>>>> +++ b/drivers/of/Kconfig
>>>> @@ -2,7 +2,11 @@ config DTC
>>>>       bool
>>>> 
>>>> config OF
>>>> -       bool
>>>> +       bool "Device Tree and Open Firmware support" if COMPILE_TEST
>>> 
>>> Actually, I think we want to just make this always visible. There are
>>> now use cases with overlays where we may want to enable DT even if the
>>> architecture is not booting with DT. However, there may be more work
>>> needed to do that.
>>> 
>> 
>> We have to decouple the notion of CONFIG_OF of the following two different things.
>> 
>> 1. I’m booting using a device tree blob.
>> 2. I’m using the in-kernel live device tree to represent my hardware device state.
>> 
>> so CONFIG_OF_DTB_BLOB for the first case, and CONFIG_OF for the second.
>> 
>> Up to now we bundle those two together.
> 
> They are separate already with CONFIG_OF_EARLY_FLATTREE
> 

A bit cryptically named, no?

> Rob

Regards

— Pantelis