[PATCH 0/6] irqchip: Hybrid probing

[PATCH 0/6] irqchip: Hybrid probing

[Marc Zyngier]

A recent attempt at converting a couple of interrupt controllers from
early probing to standard platform drivers have badly failed, as it
became evident that although an interrupt controller can easily probe
late, device drivers for the endpoints connected to it are rarely
equipped to deal with probe deferral. Changes were swiftly reverted.

However, there is some value in *optionally* enabling this, if only
for development purposes, as there is otherwise a "chicken and egg"
problem, and a few people (cc'd) are working on a potential solution.

This short series enables the infrastructure for modular building
whilst retaining the usual early probing for monolithic build, and
introduces it to the three drivers that were previously made to probe
as platform drivers.

As I don't have any of the HW, this series is fully untested and I'd
welcome some feedback on it.

Thanks,

	M.

Marc Zyngier (6):
  of: Add basic infrastructure to create early probe arrays
  irqchip: Make IRQCHIP_MATCH() type safe
  irqchip: Introduce IRQCHIP_HYBRID_DRIVER_{BEGIN,END} macros
  irqchip/mtk-cirq: Allow modular build
  irqchip/mtk-sysirq: Allow modular build
  irqchip/qcom-pdc: Allow modular build

 drivers/irqchip/irq-mtk-cirq.c   |  4 +++-
 drivers/irqchip/irq-mtk-sysirq.c |  4 +++-
 drivers/irqchip/qcom-pdc.c       |  4 +++-
 include/linux/irqchip.h          | 14 +++++++++++++-
 include/linux/of.h               | 15 +++++++++++++++
 5 files changed, 37 insertions(+), 4 deletions(-)

-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/6] of: Add basic infrastructure to create early probe arrays

[Marc Zyngier]

We currently probe interrupt controller and timers that need
to be available very early using an infratstructure that creates
struct of_device_id instances in a special section. These are
individual structures that are ultimately collated by the linker.

In order to facilitate further use of this infrastructure for
drivers that can either be built modular or as an early driver,
let's add a couple of helpers that will make it look like a
"normal" device_id array, like this:

_OF_DECLARE_ARRAY_START(table, name)
_OF_DECLARE_ELMT("compat-1", probe, type)
_OF_DECLARE_ELMT("compat-2", probe, type)
_OF_DECLARE_ELMT("compat-3", other_probe, type)
_OF_DECLARE_ARRAY_END

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 include/linux/of.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/linux/of.h b/include/linux/of.h
index 5cf7ae0465d1..08f78da95378 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1291,20 +1291,35 @@ static inline int of_get_available_child_count(const struct device_node *np)
 	return num;
 }
 
+#define __OF_DECLARE_ARRAY_START(name, section)				\
+	static const struct of_device_id __of_table_##name[]		\
+		__used __section(section) = {
+
 #if defined(CONFIG_OF) && !defined(MODULE)
 #define _OF_DECLARE(table, name, compat, fn, fn_type)			\
 	static const struct of_device_id __of_table_##name		\
 		__used __section(__##table##_of_table)			\
 		 = { .compatible = compat,				\
 		     .data = (fn == (fn_type)NULL) ? fn : fn  }
+#define _OF_DECLARE_ARRAY_START(table, name)				\
+	__OF_DECLARE_ARRAY_START(name, __##table##_of_table)
 #else
 #define _OF_DECLARE(table, name, compat, fn, fn_type)			\
 	static const struct of_device_id __of_table_##name		\
 		__attribute__((unused))					\
 		 = { .compatible = compat,				\
 		     .data = (fn == (fn_type)NULL) ? fn : fn }
+#define _OF_DECLARE_ARRAY_START(table, name)				\
+	__OF_DECLARE_ARRAY_START(name, unused)
 #endif
 
+#define _OF_DECLARE_ARRAY_END	}
+#define _OF_DECLARE_ELMT(compat, fn, fn_type)				\
+	{								\
+		.compatible = compat,					\
+		.data = (fn == (fn_type)NULL) ? fn : fn,		\
+	},
+
 typedef int (*of_init_fn_2)(struct device_node *, struct device_node *);
 typedef int (*of_init_fn_1_ret)(struct device_node *);
 typedef void (*of_init_fn_1)(struct device_node *);
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] of: Add basic infrastructure to create early probe arrays

[Bjorn Andersson]

On Sat 12 Sep 07:51 CDT 2020, Marc Zyngier wrote:

> We currently probe interrupt controller and timers that need
> to be available very early using an infratstructure that creates
> struct of_device_id instances in a special section. These are
> individual structures that are ultimately collated by the linker.
> 
> In order to facilitate further use of this infrastructure for
> drivers that can either be built modular or as an early driver,
> let's add a couple of helpers that will make it look like a
> "normal" device_id array, like this:
> 
> _OF_DECLARE_ARRAY_START(table, name)
> _OF_DECLARE_ELMT("compat-1", probe, type)
> _OF_DECLARE_ELMT("compat-2", probe, type)
> _OF_DECLARE_ELMT("compat-3", other_probe, type)
> _OF_DECLARE_ARRAY_END
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  include/linux/of.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 5cf7ae0465d1..08f78da95378 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -1291,20 +1291,35 @@ static inline int of_get_available_child_count(const struct device_node *np)
>  	return num;
>  }
>  
> +#define __OF_DECLARE_ARRAY_START(name, section)				\
> +	static const struct of_device_id __of_table_##name[]		\
> +		__used __section(section) = {
> +
>  #if defined(CONFIG_OF) && !defined(MODULE)
>  #define _OF_DECLARE(table, name, compat, fn, fn_type)			\
>  	static const struct of_device_id __of_table_##name		\
>  		__used __section(__##table##_of_table)			\
>  		 = { .compatible = compat,				\
>  		     .data = (fn == (fn_type)NULL) ? fn : fn  }
> +#define _OF_DECLARE_ARRAY_START(table, name)				\
> +	__OF_DECLARE_ARRAY_START(name, __##table##_of_table)
>  #else
>  #define _OF_DECLARE(table, name, compat, fn, fn_type)			\
>  	static const struct of_device_id __of_table_##name		\
>  		__attribute__((unused))					\
>  		 = { .compatible = compat,				\
>  		     .data = (fn == (fn_type)NULL) ? fn : fn }
> +#define _OF_DECLARE_ARRAY_START(table, name)				\
> +	__OF_DECLARE_ARRAY_START(name, unused)
>  #endif
>  
> +#define _OF_DECLARE_ARRAY_END	}
> +#define _OF_DECLARE_ELMT(compat, fn, fn_type)				\
> +	{								\
> +		.compatible = compat,					\
> +		.data = (fn == (fn_type)NULL) ? fn : fn,		\
> +	},
> +
>  typedef int (*of_init_fn_2)(struct device_node *, struct device_node *);
>  typedef int (*of_init_fn_1_ret)(struct device_node *);
>  typedef void (*of_init_fn_1)(struct device_node *);
> -- 
> 2.28.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] of: Add basic infrastructure to create early probe arrays

[Bjorn Andersson]

On Sat 12 Sep 07:51 CDT 2020, Marc Zyngier wrote:

> We currently probe interrupt controller and timers that need
> to be available very early using an infratstructure that creates

s/infratstructure/infrastructure/

> struct of_device_id instances in a special section. These are
> individual structures that are ultimately collated by the linker.
> 
> In order to facilitate further use of this infrastructure for
> drivers that can either be built modular or as an early driver,
> let's add a couple of helpers that will make it look like a
> "normal" device_id array, like this:
> 
> _OF_DECLARE_ARRAY_START(table, name)
> _OF_DECLARE_ELMT("compat-1", probe, type)
> _OF_DECLARE_ELMT("compat-2", probe, type)
> _OF_DECLARE_ELMT("compat-3", other_probe, type)
> _OF_DECLARE_ARRAY_END
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  include/linux/of.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 5cf7ae0465d1..08f78da95378 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -1291,20 +1291,35 @@ static inline int of_get_available_child_count(const struct device_node *np)
>  	return num;
>  }
>  
> +#define __OF_DECLARE_ARRAY_START(name, section)				\
> +	static const struct of_device_id __of_table_##name[]		\
> +		__used __section(section) = {
> +
>  #if defined(CONFIG_OF) && !defined(MODULE)
>  #define _OF_DECLARE(table, name, compat, fn, fn_type)			\
>  	static const struct of_device_id __of_table_##name		\
>  		__used __section(__##table##_of_table)			\
>  		 = { .compatible = compat,				\
>  		     .data = (fn == (fn_type)NULL) ? fn : fn  }
> +#define _OF_DECLARE_ARRAY_START(table, name)				\
> +	__OF_DECLARE_ARRAY_START(name, __##table##_of_table)
>  #else
>  #define _OF_DECLARE(table, name, compat, fn, fn_type)			\
>  	static const struct of_device_id __of_table_##name		\
>  		__attribute__((unused))					\
>  		 = { .compatible = compat,				\
>  		     .data = (fn == (fn_type)NULL) ? fn : fn }
> +#define _OF_DECLARE_ARRAY_START(table, name)				\
> +	__OF_DECLARE_ARRAY_START(name, unused)
>  #endif
>  
> +#define _OF_DECLARE_ARRAY_END	}
> +#define _OF_DECLARE_ELMT(compat, fn, fn_type)				\
> +	{								\
> +		.compatible = compat,					\
> +		.data = (fn == (fn_type)NULL) ? fn : fn,		\
> +	},
> +
>  typedef int (*of_init_fn_2)(struct device_node *, struct device_node *);
>  typedef int (*of_init_fn_1_ret)(struct device_node *);
>  typedef void (*of_init_fn_1)(struct device_node *);
> -- 
> 2.28.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/6] irqchip: Make IRQCHIP_MATCH() type safe

[Marc Zyngier]

IRQCHIP_DECLARE() is backed by macros that perform some elementary
type checking on the probe function. Unfortunately, IRQCHIP_MATCH()
doesn't, risking difficult debugging sessions...

Rewrite IRQCHIP_MATCH() in terms of _OF_DECLARE_ELMT() restore
the missing type safety.

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

diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h
index 67351aac65ef..f8f25e9f8200 100644
--- a/include/linux/irqchip.h
+++ b/include/linux/irqchip.h
@@ -33,7 +33,7 @@ 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) _OF_DECLARE_ELMT(compat, fn, of_init_fn_2)
 
 #define IRQCHIP_PLATFORM_DRIVER_END(drv_name)				\
 	{},								\
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] irqchip: Make IRQCHIP_MATCH() type safe

[Bjorn Andersson]

On Sat 12 Sep 07:51 CDT 2020, Marc Zyngier wrote:

> IRQCHIP_DECLARE() is backed by macros that perform some elementary
> type checking on the probe function. Unfortunately, IRQCHIP_MATCH()
> doesn't, risking difficult debugging sessions...
> 
> Rewrite IRQCHIP_MATCH() in terms of _OF_DECLARE_ELMT() restore
> the missing type safety.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  include/linux/irqchip.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h
> index 67351aac65ef..f8f25e9f8200 100644
> --- a/include/linux/irqchip.h
> +++ b/include/linux/irqchip.h
> @@ -33,7 +33,7 @@ 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) _OF_DECLARE_ELMT(compat, fn, of_init_fn_2)
>  
>  #define IRQCHIP_PLATFORM_DRIVER_END(drv_name)				\
>  	{},								\
> -- 
> 2.28.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/6] irqchip: Introduce IRQCHIP_HYBRID_DRIVER_{BEGIN, END} macros

[Marc Zyngier]

Although we are trying to move to a world where a large number
of irqchip drivers can safely be built as platform drivers
the reality is that most endpoint drivers are not ready for that,
and will fail to probe as they expect their interrupt controller
to be up and running.

A halfway house solution is to let the driver indicate that if
it is built-in (i.e. not a module), then it must use the earily
probe mechanism, IRQCHIP_DECLARE() style. Otherwise, it is a
normal module implemenenting a platform driver, and we can
fallback to the existing code.

Hopefully we'll one day be able to drop this code altogether,
but that's not for tomorrow.

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

diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h
index f8f25e9f8200..31fc9d00101f 100644
--- a/include/linux/irqchip.h
+++ b/include/linux/irqchip.h
@@ -50,6 +50,18 @@ static struct platform_driver drv_name##_driver = {		\
 };									\
 builtin_platform_driver(drv_name##_driver)
 
+#ifdef MODULE
+#define IRQCHIP_HYBRID_DRIVER_BEGIN(drv)	\
+	IRQCHIP_PLATFORM_DRIVER_BEGIN(drv)
+#define IRQCHIP_HYBRID_DRIVER_END(drv)		\
+	IRQCHIP_PLATFORM_DRIVER_END(drv)
+#else
+#define IRQCHIP_HYBRID_DRIVER_BEGIN(drv)	\
+	_OF_DECLARE_ARRAY_START(irqchip, drv)
+#define IRQCHIP_HYBRID_DRIVER_END(drv)		\
+	_OF_DECLARE_ARRAY_END;
+#endif
+
 /*
  * This macro must be used by the different irqchip drivers to declare
  * the association between their version and their initialization function.
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/6] irqchip: Introduce IRQCHIP_HYBRID_DRIVER_{BEGIN, END} macros

[Bjorn Andersson]

On Sat 12 Sep 07:51 CDT 2020, Marc Zyngier wrote:

> Although we are trying to move to a world where a large number
> of irqchip drivers can safely be built as platform drivers
> the reality is that most endpoint drivers are not ready for that,
> and will fail to probe as they expect their interrupt controller
> to be up and running.
> 
> A halfway house solution is to let the driver indicate that if
> it is built-in (i.e. not a module), then it must use the earily
> probe mechanism, IRQCHIP_DECLARE() style. Otherwise, it is a
> normal module implemenenting a platform driver, and we can
> fallback to the existing code.
> 
> Hopefully we'll one day be able to drop this code altogether,
> but that's not for tomorrow.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  include/linux/irqchip.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h
> index f8f25e9f8200..31fc9d00101f 100644
> --- a/include/linux/irqchip.h
> +++ b/include/linux/irqchip.h
> @@ -50,6 +50,18 @@ static struct platform_driver drv_name##_driver = {		\
>  };									\
>  builtin_platform_driver(drv_name##_driver)
>  
> +#ifdef MODULE
> +#define IRQCHIP_HYBRID_DRIVER_BEGIN(drv)	\
> +	IRQCHIP_PLATFORM_DRIVER_BEGIN(drv)
> +#define IRQCHIP_HYBRID_DRIVER_END(drv)		\
> +	IRQCHIP_PLATFORM_DRIVER_END(drv)
> +#else
> +#define IRQCHIP_HYBRID_DRIVER_BEGIN(drv)	\
> +	_OF_DECLARE_ARRAY_START(irqchip, drv)
> +#define IRQCHIP_HYBRID_DRIVER_END(drv)		\
> +	_OF_DECLARE_ARRAY_END;
> +#endif
> +
>  /*
>   * This macro must be used by the different irqchip drivers to declare
>   * the association between their version and their initialization function.
> -- 
> 2.28.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/6] irqchip/mtk-cirq: Allow modular build

[Marc Zyngier]

Switch the driver to a "hybrid probe" model which preserves the
built-in behaviour while allowing the driver to be optionnally
built as a module for development purposes.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-mtk-cirq.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-mtk-cirq.c b/drivers/irqchip/irq-mtk-cirq.c
index 69ba8ce3c178..43e880b63ed2 100644
--- a/drivers/irqchip/irq-mtk-cirq.c
+++ b/drivers/irqchip/irq-mtk-cirq.c
@@ -295,4 +295,6 @@ static int __init mtk_cirq_of_init(struct device_node *node,
 	return ret;
 }
 
-IRQCHIP_DECLARE(mtk_cirq, "mediatek,mtk-cirq", mtk_cirq_of_init);
+IRQCHIP_HYBRID_DRIVER_BEGIN(mtk_cirq)
+IRQCHIP_MATCH("mediatek,mtk-cirq", mtk_cirq_of_init)
+IRQCHIP_HYBRID_DRIVER_END(mtk_cirq)
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/6] irqchip/mtk-cirq: Allow modular build

[Bjorn Andersson]

On Sat 12 Sep 07:51 CDT 2020, Marc Zyngier wrote:

> Switch the driver to a "hybrid probe" model which preserves the
> built-in behaviour while allowing the driver to be optionnally
> built as a module for development purposes.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/irqchip/irq-mtk-cirq.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-mtk-cirq.c b/drivers/irqchip/irq-mtk-cirq.c
> index 69ba8ce3c178..43e880b63ed2 100644
> --- a/drivers/irqchip/irq-mtk-cirq.c
> +++ b/drivers/irqchip/irq-mtk-cirq.c
> @@ -295,4 +295,6 @@ static int __init mtk_cirq_of_init(struct device_node *node,
>  	return ret;
>  }
>  
> -IRQCHIP_DECLARE(mtk_cirq, "mediatek,mtk-cirq", mtk_cirq_of_init);
> +IRQCHIP_HYBRID_DRIVER_BEGIN(mtk_cirq)
> +IRQCHIP_MATCH("mediatek,mtk-cirq", mtk_cirq_of_init)
> +IRQCHIP_HYBRID_DRIVER_END(mtk_cirq)
> -- 
> 2.28.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/6] irqchip/mtk-cirq: Allow modular build

[Enric Balletbo Serra]

Missatge de Bjorn Andersson <bjorn.andersson@linaro.org> del dia dg.,
13 de set. 2020 a les 1:26:
>
> On Sat 12 Sep 07:51 CDT 2020, Marc Zyngier wrote:
>
> > Switch the driver to a "hybrid probe" model which preserves the
> > built-in behaviour while allowing the driver to be optionnally
> > built as a module for development purposes.
> >
>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>
> > Signed-off-by: Marc Zyngier <maz@kernel.org>

I've tested this on mt8173 and mt8183, and this time, the patches
didn't break booting on these platforms. For MediaTek, right now, only
makes sense the driver to be built-in as other drivers that use it are
not ready for deferring their probe. So,

Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Thanks
  Enric

> > ---
> >  drivers/irqchip/irq-mtk-cirq.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/irq-mtk-cirq.c b/drivers/irqchip/irq-mtk-cirq.c
> > index 69ba8ce3c178..43e880b63ed2 100644
> > --- a/drivers/irqchip/irq-mtk-cirq.c
> > +++ b/drivers/irqchip/irq-mtk-cirq.c
> > @@ -295,4 +295,6 @@ static int __init mtk_cirq_of_init(struct device_node *node,
> >       return ret;
> >  }
> >
> > -IRQCHIP_DECLARE(mtk_cirq, "mediatek,mtk-cirq", mtk_cirq_of_init);
> > +IRQCHIP_HYBRID_DRIVER_BEGIN(mtk_cirq)
> > +IRQCHIP_MATCH("mediatek,mtk-cirq", mtk_cirq_of_init)
> > +IRQCHIP_HYBRID_DRIVER_END(mtk_cirq)
> > --
> > 2.28.0
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/6] irqchip/mtk-sysirq: Allow modular build

[Marc Zyngier]

Switch the driver to a "hybrid probe" model which preserves the
built-in behaviour while allowing the driver to be optionnally
built as a module for development purposes.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-mtk-sysirq.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-mtk-sysirq.c b/drivers/irqchip/irq-mtk-sysirq.c
index 6ff98b87e5c0..ee45d8f71ec3 100644
--- a/drivers/irqchip/irq-mtk-sysirq.c
+++ b/drivers/irqchip/irq-mtk-sysirq.c
@@ -231,4 +231,6 @@ static int __init mtk_sysirq_of_init(struct device_node *node,
 	kfree(chip_data);
 	return ret;
 }
-IRQCHIP_DECLARE(mtk_sysirq, "mediatek,mt6577-sysirq", mtk_sysirq_of_init);
+IRQCHIP_HYBRID_DRIVER_BEGIN(mtk_sysirq)
+IRQCHIP_MATCH("mediatek,mt6577-sysirq", mtk_sysirq_of_init)
+IRQCHIP_HYBRID_DRIVER_END(mtk_sysirq)
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/6] irqchip/mtk-sysirq: Allow modular build

[Bjorn Andersson]

On Sat 12 Sep 07:51 CDT 2020, Marc Zyngier wrote:

> Switch the driver to a "hybrid probe" model which preserves the
> built-in behaviour while allowing the driver to be optionnally
> built as a module for development purposes.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/irqchip/irq-mtk-sysirq.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-mtk-sysirq.c b/drivers/irqchip/irq-mtk-sysirq.c
> index 6ff98b87e5c0..ee45d8f71ec3 100644
> --- a/drivers/irqchip/irq-mtk-sysirq.c
> +++ b/drivers/irqchip/irq-mtk-sysirq.c
> @@ -231,4 +231,6 @@ static int __init mtk_sysirq_of_init(struct device_node *node,
>  	kfree(chip_data);
>  	return ret;
>  }
> -IRQCHIP_DECLARE(mtk_sysirq, "mediatek,mt6577-sysirq", mtk_sysirq_of_init);
> +IRQCHIP_HYBRID_DRIVER_BEGIN(mtk_sysirq)
> +IRQCHIP_MATCH("mediatek,mt6577-sysirq", mtk_sysirq_of_init)
> +IRQCHIP_HYBRID_DRIVER_END(mtk_sysirq)
> -- 
> 2.28.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/6] irqchip/mtk-sysirq: Allow modular build

[Enric Balletbo Serra]

Missatge de Bjorn Andersson <bjorn.andersson@linaro.org> del dia dg.,
13 de set. 2020 a les 1:25:
>
> On Sat 12 Sep 07:51 CDT 2020, Marc Zyngier wrote:
>
> > Switch the driver to a "hybrid probe" model which preserves the
> > built-in behaviour while allowing the driver to be optionnally
> > built as a module for development purposes.
> >
>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>
> > Signed-off-by: Marc Zyngier <maz@kernel.org>

I've tested this on mt8173 and mt8183, and this time, the patches
didn't break booting on these platforms. For MediaTek, right now, only
makes sense the driver to be built-in as other drivers that use it are
not ready for deferring their probe. So,

Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Thanks
  Enric

> > ---
> >  drivers/irqchip/irq-mtk-sysirq.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/irq-mtk-sysirq.c b/drivers/irqchip/irq-mtk-sysirq.c
> > index 6ff98b87e5c0..ee45d8f71ec3 100644
> > --- a/drivers/irqchip/irq-mtk-sysirq.c
> > +++ b/drivers/irqchip/irq-mtk-sysirq.c
> > @@ -231,4 +231,6 @@ static int __init mtk_sysirq_of_init(struct device_node *node,
> >       kfree(chip_data);
> >       return ret;
> >  }
> > -IRQCHIP_DECLARE(mtk_sysirq, "mediatek,mt6577-sysirq", mtk_sysirq_of_init);
> > +IRQCHIP_HYBRID_DRIVER_BEGIN(mtk_sysirq)
> > +IRQCHIP_MATCH("mediatek,mt6577-sysirq", mtk_sysirq_of_init)
> > +IRQCHIP_HYBRID_DRIVER_END(mtk_sysirq)
> > --
> > 2.28.0
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/6] irqchip/qcom-pdc: Allow modular build

[Marc Zyngier]

Switch the driver to a "hybrid probe" model which preserves the
built-in behaviour while allowing the driver to be optionnally
built as a module for development purposes.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/qcom-pdc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 6ae9e1f0819d..8543fa23da10 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -430,4 +430,6 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
 	return ret;
 }
 
-IRQCHIP_DECLARE(qcom_pdc, "qcom,pdc", qcom_pdc_init);
+IRQCHIP_HYBRID_DRIVER_BEGIN(qcom_pdc)
+IRQCHIP_MATCH("qcom,pdc", qcom_pdc_init)
+IRQCHIP_HYBRID_DRIVER_END(qcom_pdc)
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/6] irqchip/qcom-pdc: Allow modular build

[Bjorn Andersson]

On Sat 12 Sep 07:51 CDT 2020, Marc Zyngier wrote:

> Switch the driver to a "hybrid probe" model which preserves the
> built-in behaviour while allowing the driver to be optionnally
> built as a module for development purposes.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/irqchip/qcom-pdc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
> index 6ae9e1f0819d..8543fa23da10 100644
> --- a/drivers/irqchip/qcom-pdc.c
> +++ b/drivers/irqchip/qcom-pdc.c
> @@ -430,4 +430,6 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
>  	return ret;
>  }
>  
> -IRQCHIP_DECLARE(qcom_pdc, "qcom,pdc", qcom_pdc_init);
> +IRQCHIP_HYBRID_DRIVER_BEGIN(qcom_pdc)
> +IRQCHIP_MATCH("qcom,pdc", qcom_pdc_init)
> +IRQCHIP_HYBRID_DRIVER_END(qcom_pdc)
> -- 
> 2.28.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/6] irqchip: Hybrid probing

[John Stultz]

On Sat, Sep 12, 2020 at 5:52 AM Marc Zyngier <maz@kernel.org> wrote:
>
> A recent attempt at converting a couple of interrupt controllers from
> early probing to standard platform drivers have badly failed, as it
> became evident that although an interrupt controller can easily probe
> late, device drivers for the endpoints connected to it are rarely
> equipped to deal with probe deferral. Changes were swiftly reverted.
>
> However, there is some value in *optionally* enabling this, if only
> for development purposes, as there is otherwise a "chicken and egg"
> problem, and a few people (cc'd) are working on a potential solution.
>
> This short series enables the infrastructure for modular building
> whilst retaining the usual early probing for monolithic build, and
> introduces it to the three drivers that were previously made to probe
> as platform drivers.
>
> As I don't have any of the HW, this series is fully untested and I'd
> welcome some feedback on it.

I've tested this on db845c along with a small follow-on patch I'll
send to you which sets the qcom-pdc as a tristate in the Kconfig, both
as a module and as a built-in.

Tested-by: John Stultz <john.stultz@linaro.org>

thanks
-john

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/6] irqchip: Hybrid probing

[Rob Herring]

On Sat, Sep 12, 2020 at 01:51:42PM +0100, Marc Zyngier wrote:
> A recent attempt at converting a couple of interrupt controllers from
> early probing to standard platform drivers have badly failed, as it
> became evident that although an interrupt controller can easily probe
> late, device drivers for the endpoints connected to it are rarely
> equipped to deal with probe deferral. Changes were swiftly reverted.
>
> However, there is some value in *optionally* enabling this, if only
> for development purposes, as there is otherwise a "chicken and egg"
> problem, and a few people (cc'd) are working on a potential solution.
> 
> This short series enables the infrastructure for modular building
> whilst retaining the usual early probing for monolithic build, and
> introduces it to the three drivers that were previously made to probe
> as platform drivers.

I hardly expected more OF_DECLARE macros when I opened this up. Given 
desires to get rid of them, I don't think adding to it is the way 
forward. That wrapping a platform driver around OF_DECLARE looks pretty 
horrible IMO. 

I browsed some of the discussion around this. It didn't seem like it's 
a large number of drivers that have to be fixed to defer probe 
correctly. Am I missing something?

I'd rather keep the pressure on getting fw_devlink on by default.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/6] irqchip: Hybrid probing

[Marc Zyngier]

On 2020-09-15 22:13, Rob Herring wrote:
> On Sat, Sep 12, 2020 at 01:51:42PM +0100, Marc Zyngier wrote:
>> A recent attempt at converting a couple of interrupt controllers from
>> early probing to standard platform drivers have badly failed, as it
>> became evident that although an interrupt controller can easily probe
>> late, device drivers for the endpoints connected to it are rarely
>> equipped to deal with probe deferral. Changes were swiftly reverted.
>> 
>> However, there is some value in *optionally* enabling this, if only
>> for development purposes, as there is otherwise a "chicken and egg"
>> problem, and a few people (cc'd) are working on a potential solution.
>> 
>> This short series enables the infrastructure for modular building
>> whilst retaining the usual early probing for monolithic build, and
>> introduces it to the three drivers that were previously made to probe
>> as platform drivers.
> 
> I hardly expected more OF_DECLARE macros when I opened this up. Given
> desires to get rid of them, I don't think adding to it is the way
> forward. That wrapping a platform driver around OF_DECLARE looks pretty
> horrible IMO.

Nobody said it was cute. It's a band aid that allows us to move from the
status-quo that exists today. How would you propose we allow people to
go and start "fixing" drivers if you don't give them the opportunity
to even start trying?

> I browsed some of the discussion around this. It didn't seem like it's
> a large number of drivers that have to be fixed to defer probe
> correctly. Am I missing something?

Well, that was enough drivers for the two platforms that had it enabled
to break horribly, without a way to go back to a working state. Do you
find that acceptable? I don't.

> I'd rather keep the pressure on getting fw_devlink on by default.

So far, fw_devlink breaks everything under the sun, even without modular
irqchips. Most of my systems fail to boot if I enable it. So yes, it
really needs some work. And this series allows this work to happen.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/6] irqchip: Hybrid probing

[Rob Herring]

On Wed, Sep 16, 2020 at 2:51 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On 2020-09-15 22:13, Rob Herring wrote:
> > On Sat, Sep 12, 2020 at 01:51:42PM +0100, Marc Zyngier wrote:
> >> A recent attempt at converting a couple of interrupt controllers from
> >> early probing to standard platform drivers have badly failed, as it
> >> became evident that although an interrupt controller can easily probe
> >> late, device drivers for the endpoints connected to it are rarely
> >> equipped to deal with probe deferral. Changes were swiftly reverted.
> >>
> >> However, there is some value in *optionally* enabling this, if only
> >> for development purposes, as there is otherwise a "chicken and egg"
> >> problem, and a few people (cc'd) are working on a potential solution.
> >>
> >> This short series enables the infrastructure for modular building
> >> whilst retaining the usual early probing for monolithic build, and
> >> introduces it to the three drivers that were previously made to probe
> >> as platform drivers.
> >
> > I hardly expected more OF_DECLARE macros when I opened this up. Given
> > desires to get rid of them, I don't think adding to it is the way
> > forward. That wrapping a platform driver around OF_DECLARE looks pretty
> > horrible IMO.
>
> Nobody said it was cute. It's a band aid that allows us to move from the
> status-quo that exists today. How would you propose we allow people to
> go and start "fixing" drivers if you don't give them the opportunity
> to even start trying?

Apply the reverted patches and start fixing the drivers.

> > I browsed some of the discussion around this. It didn't seem like it's
> > a large number of drivers that have to be fixed to defer probe
> > correctly. Am I missing something?
>
> Well, that was enough drivers for the two platforms that had it enabled
> to break horribly, without a way to go back to a working state. Do you
> find that acceptable? I don't.

I understand reverting for v5.9, that was the right choice. But
Mediatek had 3 drivers broken. Is there more to it than getting
EPROBE_DEFER handled correctly in those drivers?

> > I'd rather keep the pressure on getting fw_devlink on by default.
>
> So far, fw_devlink breaks everything under the sun, even without modular
> irqchips. Most of my systems fail to boot if I enable it. So yes, it
> really needs some work. And this series allows this work to happen.

I think we can do something more simple here. We just need to
instantiate the irqchip devices earlier to get them to probe first and
not cause deferrals. That's just a matter of calling
of_platform_device_create() in an earlier initcall (<=
arch_initcall_sync ('=' because of link order)). Then once dependent
drivers are all fixed, all that has to be done is rip out that
initcall and the default of_platform_populate call will create the
device instead.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel