[PATCH] irqchip: Provide stronger type checking for IRQCHIP_MATCH/IRQCHIP_DECLARE

[PATCH] irqchip: Provide stronger type checking for IRQCHIP_MATCH/IRQCHIP_DECLARE

[Marc Zyngier]

Both IRQCHIP_DECLARE() and IRQCHIP_MATCH() use an underlying of_device_id()
structure to encode the matching property and the init callback.
However, this callback is stored in as a void * pointer, which obviously
defeat any attempt at stronger type checking.

Work around this by providing a new macro that builds on top of the
__typecheck() primitive, and that can be used to warn when there is
a discrepency between the drivers and core code.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 include/linux/irqchip.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h
index 67351aac65ef..5de0dfc5d64d 100644
--- a/include/linux/irqchip.h
+++ b/include/linux/irqchip.h
@@ -14,8 +14,15 @@
 #include <linux/acpi.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/platform_device.h>
 
+/* Undefined on purpose */
+extern of_irq_init_cb_t typecheck_irq_init_cb;
+
+#define typecheck_irq_init_cb(fn)					\
+	(__typecheck(typecheck_irq_init_cb, &fn) ? fn : fn)
+
 /*
  * This macro must be used by the different irqchip drivers to declare
  * the association between their DT compatible string and their
@@ -26,14 +33,16 @@
  * @compstr: compatible string of the irqchip driver
  * @fn: initialization function
  */
-#define IRQCHIP_DECLARE(name, compat, fn) OF_DECLARE_2(irqchip, name, compat, fn)
+#define IRQCHIP_DECLARE(name, compat, fn)	\
+	OF_DECLARE_2(irqchip, name, compat, typecheck_irq_init_cb(fn))
 
 extern int platform_irqchip_probe(struct platform_device *pdev);
 
 #define IRQCHIP_PLATFORM_DRIVER_BEGIN(drv_name) \
 static const struct of_device_id drv_name##_irqchip_match_table[] = {
 
-#define IRQCHIP_MATCH(compat, fn) { .compatible = compat, .data = fn },
+#define IRQCHIP_MATCH(compat, fn) { .compatible = compat,		\
+				    .data = typecheck_irq_init_cb(fn), },
 
 #define IRQCHIP_PLATFORM_DRIVER_END(drv_name)				\
 	{},								\
-- 
2.30.2

[irqchip: irq/irqchip-next] irqchip: Provide stronger type checking for IRQCHIP_MATCH/IRQCHIP_DECLARE

[irqchip-bot for Marc Zyngier]

The following commit has been merged into the irq/irqchip-next branch of irqchip:

Commit-ID:     f1985002839af80d6c84e9537834a81fb1364d6e
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/f1985002839af80d6c84e9537834a81fb1364d6e
Author:        Marc Zyngier <maz@kernel.org>
AuthorDate:    Wed, 20 Oct 2021 09:33:21 +01:00
Committer:     Marc Zyngier <maz@kernel.org>
CommitterDate: Wed, 20 Oct 2021 19:33:53 +01:00

irqchip: Provide stronger type checking for IRQCHIP_MATCH/IRQCHIP_DECLARE

Both IRQCHIP_DECLARE() and IRQCHIP_MATCH() use an underlying of_device_id()
structure to encode the matching property and the init callback.
However, this callback is stored in as a void * pointer, which obviously
defeat any attempt at stronger type checking.

Work around this by providing a new macro that builds on top of the
__typecheck() primitive, and that can be used to warn when there is
a discrepency between the drivers and core code.

Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20211020104527.3066268-1-maz@kernel.org
---
 include/linux/irqchip.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h
index 67351aa..5de0dfc 100644
--- a/include/linux/irqchip.h
+++ b/include/linux/irqchip.h
@@ -14,8 +14,15 @@
 #include <linux/acpi.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/platform_device.h>
 
+/* Undefined on purpose */
+extern of_irq_init_cb_t typecheck_irq_init_cb;
+
+#define typecheck_irq_init_cb(fn)					\
+	(__typecheck(typecheck_irq_init_cb, &fn) ? fn : fn)
+
 /*
  * This macro must be used by the different irqchip drivers to declare
  * the association between their DT compatible string and their
@@ -26,14 +33,16 @@
  * @compstr: compatible string of the irqchip driver
  * @fn: initialization function
  */
-#define IRQCHIP_DECLARE(name, compat, fn) OF_DECLARE_2(irqchip, name, compat, fn)
+#define IRQCHIP_DECLARE(name, compat, fn)	\
+	OF_DECLARE_2(irqchip, name, compat, typecheck_irq_init_cb(fn))
 
 extern int platform_irqchip_probe(struct platform_device *pdev);
 
 #define IRQCHIP_PLATFORM_DRIVER_BEGIN(drv_name) \
 static const struct of_device_id drv_name##_irqchip_match_table[] = {
 
-#define IRQCHIP_MATCH(compat, fn) { .compatible = compat, .data = fn },
+#define IRQCHIP_MATCH(compat, fn) { .compatible = compat,		\
+				    .data = typecheck_irq_init_cb(fn), },
 
 #define IRQCHIP_PLATFORM_DRIVER_END(drv_name)				\
 	{},								\

Re: [PATCH] irqchip: Provide stronger type checking for IRQCHIP_MATCH/IRQCHIP_DECLARE

[Florian Fainelli]

On 10/20/21 3:45 AM, Marc Zyngier wrote:
> Both IRQCHIP_DECLARE() and IRQCHIP_MATCH() use an underlying of_device_id()
> structure to encode the matching property and the init callback.
> However, this callback is stored in as a void * pointer, which obviously
> defeat any attempt at stronger type checking.
> 
> Work around this by providing a new macro that builds on top of the
> __typecheck() primitive, and that can be used to warn when there is
> a discrepency between the drivers and core code.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH] irqchip: Provide stronger type checking for IRQCHIP_MATCH/IRQCHIP_DECLARE

[Guenter Roeck]

On Wed, Oct 20, 2021 at 11:45:27AM +0100, Marc Zyngier wrote:
> Both IRQCHIP_DECLARE() and IRQCHIP_MATCH() use an underlying of_device_id()
> structure to encode the matching property and the init callback.
> However, this callback is stored in as a void * pointer, which obviously
> defeat any attempt at stronger type checking.
> 
> Work around this by providing a new macro that builds on top of the
> __typecheck() primitive, and that can be used to warn when there is
> a discrepency between the drivers and core code.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>

This patch results in:

In file included from include/linux/irq.h:589,
                 from include/linux/of_irq.h:7,
                 from include/linux/irqchip.h:17,
                 from arch/h8300/include/asm/irq.h:5,
                 from arch/h8300/kernel/traps.c:27:
include/linux/irqdesc.h:113:33: error: 'NR_IRQS' undeclared here (not in a function)
  113 | extern struct irq_desc irq_desc[NR_IRQS];

and many similar errors when trying to build h8300 images.

Guenter

> ---
>  include/linux/irqchip.h | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h
> index 67351aac65ef..5de0dfc5d64d 100644
> --- a/include/linux/irqchip.h
> +++ b/include/linux/irqchip.h
> @@ -14,8 +14,15 @@
>  #include <linux/acpi.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_irq.h>
>  #include <linux/platform_device.h>
>  
> +/* Undefined on purpose */
> +extern of_irq_init_cb_t typecheck_irq_init_cb;
> +
> +#define typecheck_irq_init_cb(fn)					\
> +	(__typecheck(typecheck_irq_init_cb, &fn) ? fn : fn)
> +
>  /*
>   * This macro must be used by the different irqchip drivers to declare
>   * the association between their DT compatible string and their
> @@ -26,14 +33,16 @@
>   * @compstr: compatible string of the irqchip driver
>   * @fn: initialization function
>   */
> -#define IRQCHIP_DECLARE(name, compat, fn) OF_DECLARE_2(irqchip, name, compat, fn)
> +#define IRQCHIP_DECLARE(name, compat, fn)	\
> +	OF_DECLARE_2(irqchip, name, compat, typecheck_irq_init_cb(fn))
>  
>  extern int platform_irqchip_probe(struct platform_device *pdev);
>  
>  #define IRQCHIP_PLATFORM_DRIVER_BEGIN(drv_name) \
>  static const struct of_device_id drv_name##_irqchip_match_table[] = {
>  
> -#define IRQCHIP_MATCH(compat, fn) { .compatible = compat, .data = fn },
> +#define IRQCHIP_MATCH(compat, fn) { .compatible = compat,		\
> +				    .data = typecheck_irq_init_cb(fn), },
>  
>  #define IRQCHIP_PLATFORM_DRIVER_END(drv_name)				\
>  	{},								\
> -- 
> 2.30.2
> 

Re: [PATCH] irqchip: Provide stronger type checking for IRQCHIP_MATCH/IRQCHIP_DECLARE

[Marc Zyngier]

On 2021-10-28 18:28, Guenter Roeck wrote:
> On Wed, Oct 20, 2021 at 11:45:27AM +0100, Marc Zyngier wrote:
>> Both IRQCHIP_DECLARE() and IRQCHIP_MATCH() use an underlying 
>> of_device_id()
>> structure to encode the matching property and the init callback.
>> However, this callback is stored in as a void * pointer, which 
>> obviously
>> defeat any attempt at stronger type checking.
>> 
>> Work around this by providing a new macro that builds on top of the
>> __typecheck() primitive, and that can be used to warn when there is
>> a discrepency between the drivers and core code.
>> 
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
> 
> This patch results in:
> 
> In file included from include/linux/irq.h:589,
>                  from include/linux/of_irq.h:7,
>                  from include/linux/irqchip.h:17,
>                  from arch/h8300/include/asm/irq.h:5,
>                  from arch/h8300/kernel/traps.c:27:
> include/linux/irqdesc.h:113:33: error: 'NR_IRQS' undeclared here (not
> in a function)
>   113 | extern struct irq_desc irq_desc[NR_IRQS];
> 
> and many similar errors when trying to build h8300 images.

Sigh... Please give this [1] a go.

         M.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=irq/misc-5.16&id=0646880e517c06f0e0746665ca1e0e6dd36c406e

-- 
Jazz is not dead. It just smells funny...

Re: [PATCH] irqchip: Provide stronger type checking for IRQCHIP_MATCH/IRQCHIP_DECLARE

[Guenter Roeck]

On Thu, Oct 28, 2021 at 07:43:16PM +0100, Marc Zyngier wrote:
> On 2021-10-28 18:28, Guenter Roeck wrote:
> > On Wed, Oct 20, 2021 at 11:45:27AM +0100, Marc Zyngier wrote:
> > > Both IRQCHIP_DECLARE() and IRQCHIP_MATCH() use an underlying
> > > of_device_id()
> > > structure to encode the matching property and the init callback.
> > > However, this callback is stored in as a void * pointer, which
> > > obviously
> > > defeat any attempt at stronger type checking.
> > > 
> > > Work around this by providing a new macro that builds on top of the
> > > __typecheck() primitive, and that can be used to warn when there is
> > > a discrepency between the drivers and core code.
> > > 
> > > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > 
> > This patch results in:
> > 
> > In file included from include/linux/irq.h:589,
> >                  from include/linux/of_irq.h:7,
> >                  from include/linux/irqchip.h:17,
> >                  from arch/h8300/include/asm/irq.h:5,
> >                  from arch/h8300/kernel/traps.c:27:
> > include/linux/irqdesc.h:113:33: error: 'NR_IRQS' undeclared here (not
> > in a function)
> >   113 | extern struct irq_desc irq_desc[NR_IRQS];
> > 
> > and many similar errors when trying to build h8300 images.
> 
> Sigh... Please give this [1] a go.
> 
>         M.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=irq/misc-5.16&id=0646880e517c06f0e0746665ca1e0e6dd36c406e

Yes, that does the trick.

Tested-by: Guenter Roeck <linux@roeck-us.net>

Thanks,
Guenter

[irqchip: irq/irqchip-next] h8300: Fix linux/irqchip.h include mess

[irqchip-bot for Marc Zyngier]

The following commit has been merged into the irq/irqchip-next branch of irqchip:

Commit-ID:     837d7a8fe852cf93fff1cd3b73d707b3a6ae340f
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/837d7a8fe852cf93fff1cd3b73d707b3a6ae340f
Author:        Marc Zyngier <maz@kernel.org>
AuthorDate:    Thu, 28 Oct 2021 19:24:25 +01:00
Committer:     Marc Zyngier <maz@kernel.org>
CommitterDate: Thu, 28 Oct 2021 21:02:48 +01:00

h8300: Fix linux/irqchip.h include mess

h8300 drags linux/irqchip.h from asm/irq.h, which is in general a bad
idea (asm/*.h should avoid dragging linux/*.h, as it is usually supposed
to work the other way around).

Move the inclusion of linux/irqchip.h to the single location where it
actually matters in the arch code.

Reported-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20211028172849.GA701812@roeck-us.net
---
 arch/h8300/include/asm/irq.h | 2 --
 arch/h8300/kernel/irq.c      | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/h8300/include/asm/irq.h b/arch/h8300/include/asm/irq.h
index 5fc5b43..776cf06 100644
--- a/arch/h8300/include/asm/irq.h
+++ b/arch/h8300/include/asm/irq.h
@@ -2,8 +2,6 @@
 #ifndef _H8300_IRQ_H_
 #define _H8300_IRQ_H_
 
-#include <linux/irqchip.h>
-
 #if defined(CONFIG_CPU_H8300H)
 #define NR_IRQS 64
 #define IRQ_CHIP h8300h_irq_chip
diff --git a/arch/h8300/kernel/irq.c b/arch/h8300/kernel/irq.c
index 834e4d7..8ad6d70 100644
--- a/arch/h8300/kernel/irq.c
+++ b/arch/h8300/kernel/irq.c
@@ -8,6 +8,7 @@
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
+#include <linux/irqchip.h>
 #include <linux/irqdomain.h>
 #include <linux/of_irq.h>
 #include <asm/traps.h>