| Message ID | 1351356317-16758-1-git-send-email-thomas.petazzoni@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Saturday 27 October 2012, Thomas Petazzoni wrote: > With the recent creation of the drivers/irqchip/ directory, it is > desirable to move irq controller drivers here. At the moment, the only > driver here is irq-bcm2835, the driver for the irq controller found in > the ARM BCM2835 SoC, present in Rasberry Pi systems. This irq > controller driver was exporting its initialization function and its > irq handling function through a header file in > <linux/irqchip/bcm2835.h>. Very nice series! I think it would be good if Thomas Gleixner as the IRQ subsystem maintainer could have a look as well. We should probably add the drivers/irqchip directory to that MAINTAINERS entry. Arnd
On Sat, 27 Oct 2012, Arnd Bergmann wrote: > On Saturday 27 October 2012, Thomas Petazzoni wrote: > > With the recent creation of the drivers/irqchip/ directory, it is > > desirable to move irq controller drivers here. At the moment, the only > > driver here is irq-bcm2835, the driver for the irq controller found in > > the ARM BCM2835 SoC, present in Rasberry Pi systems. This irq > > controller driver was exporting its initialization function and its > > irq handling function through a header file in > > <linux/irqchip/bcm2835.h>. > > Very nice series! > > I think it would be good if Thomas Gleixner as the IRQ subsystem maintainer > could have a look as well. We should probably add the drivers/irqchip > directory to that MAINTAINERS entry. I skimmed over the patches and they look very reasonable. I try to find a time slot to give it a more thorough look. So: Tentatively-acked-by-me
On 10/27/2012 10:45 AM, Thomas Petazzoni wrote: ... > So, upon the suggestion of Rob Herring and Arnd Bergmann, this commit > introduces a small infrastructure that defines a central > irqchip_init() function in drivers/irqchip/irqchip.c, which is meant > to be called as the ->init_irq() callback of ARM platforms. This patch. Reviewed-by: Stephen Warren <swarren@wwwdotorg.org> As a minor aside, I only received patch 2/3 in my inbox initially, and that made no sense to me without reading this patch too. It would have been helpful to have been CC'd on the whole series.
Arnd, On Sat, 27 Oct 2012 19:21:17 +0000, Arnd Bergmann wrote: > On Saturday 27 October 2012, Thomas Petazzoni wrote: > > With the recent creation of the drivers/irqchip/ directory, it is > > desirable to move irq controller drivers here. At the moment, the only > > driver here is irq-bcm2835, the driver for the irq controller found in > > the ARM BCM2835 SoC, present in Rasberry Pi systems. This irq > > controller driver was exporting its initialization function and its > > irq handling function through a header file in > > <linux/irqchip/bcm2835.h>. > > Very nice series! Thanks! > I think it would be good if Thomas Gleixner as the IRQ subsystem maintainer > could have a look as well. We should probably add the drivers/irqchip > directory to that MAINTAINERS entry. Sure. There are however two things I am not entirely happy with: (*) For each irqchip driver, we have to enclose the entry in the irqchip_of_match[] array between #ifdef CONFIG... #endif. Not really a problem, but not very pretty either, but I don't see a simple around it (I don't think we want to define yet another custom ELF section just for the purpose of irqchip drivers registration). (*) The fact that all the irqchip drivers have to mess around directly with handle_arch_irq, which is an internal/architecture specific symbol. But since the current drivers are only compiled when a ARM architecture is selected, maybe it isn't a problem. But that said, it seems like it is a good enough solution for a start. Trying to solve those two problems would probably lead to an over-engineered solution. Best regards, Thomas
On Sat, 27 Oct 2012 20:20:03 -0600, Stephen Warren wrote: > On 10/27/2012 10:45 AM, Thomas Petazzoni wrote: > ... > > So, upon the suggestion of Rob Herring and Arnd Bergmann, this commit > > introduces a small infrastructure that defines a central > > irqchip_init() function in drivers/irqchip/irqchip.c, which is meant > > to be called as the ->init_irq() callback of ARM platforms. > > This patch. > Reviewed-by: Stephen Warren <swarren@wwwdotorg.org> Thanks. > As a minor aside, I only received patch 2/3 in my inbox initially, and > that made no sense to me without reading this patch too. It would have > been helpful to have been CC'd on the whole series. I did Cc: you on patch 2/3 specifically because it was the one affecting the bcm2835 architecture you're maintaining. But agreed, it doesn't make much sense to see only this patch 2/3, so I'll Cc you on the entire series for the next version. Thanks! Thomas
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 1bb8bf6..e0ff166 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -1 +1,2 @@ -# empty +config USE_IRQCHIP + bool diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index 054321d..77adcb1 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -1 +1,2 @@ +obj-$(CONFIG_USE_IRQCHIP) += irqchip.o obj-$(CONFIG_ARCH_BCM2835) += irq-bcm2835.o diff --git a/drivers/irqchip/irqchip.c b/drivers/irqchip/irqchip.c new file mode 100644 index 0000000..410f99f --- /dev/null +++ b/drivers/irqchip/irqchip.c @@ -0,0 +1,23 @@ +/* + * Copyright (C) 2012 Thomas Petazzoni + * + * Thomas Petazzoni <thomas.petazzoni@free-electrons.com> + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include <linux/init.h> +#include <linux/of_irq.h> + +#include "irqchip.h" + +static const struct of_device_id irqchip_of_match[] __initconst = { + {}, +}; + +void __init irqchip_init(void) +{ + of_irq_init(irqchip_of_match); +} diff --git a/drivers/irqchip/irqchip.h b/drivers/irqchip/irqchip.h new file mode 100644 index 0000000..1e7a5c2 --- /dev/null +++ b/drivers/irqchip/irqchip.h @@ -0,0 +1,14 @@ +/* + * Copyright (C) 2012 Thomas Petazzoni + * + * Thomas Petazzoni <thomas.petazzoni@free-electrons.com> + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#ifndef _IRQCHIP_H +#define _IRQCHIP_H + +#endif diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h new file mode 100644 index 0000000..e0006f1 --- /dev/null +++ b/include/linux/irqchip.h @@ -0,0 +1,16 @@ +/* + * Copyright (C) 2012 Thomas Petazzoni + * + * Thomas Petazzoni <thomas.petazzoni@free-electrons.com> + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#ifndef _LINUX_IRQCHIP_H +#define _LINUX_IRQCHIP_H + +void irqchip_init(void); + +#endif
With the recent creation of the drivers/irqchip/ directory, it is desirable to move irq controller drivers here. At the moment, the only driver here is irq-bcm2835, the driver for the irq controller found in the ARM BCM2835 SoC, present in Rasberry Pi systems. This irq controller driver was exporting its initialization function and its irq handling function through a header file in <linux/irqchip/bcm2835.h>. When proposing to also move another irq controller driver in drivers/irqchip, Rob Herring raised the very valid point that moving things to drivers/irqchip was good in order to remove more stuff from arch/arm, but if it means adding gazillions of headers files in include/linux/irqchip/, it would not be very nice. So, upon the suggestion of Rob Herring and Arnd Bergmann, this commit introduces a small infrastructure that defines a central irqchip_init() function in drivers/irqchip/irqchip.c, which is meant to be called as the ->init_irq() callback of ARM platforms. This function calls of_irq_init() with an array that will progressively contain the compatible strings of each irq controller driver, and also a reference to the initialization functions of such drivers. The drivers/irqchip/irqchip.h header file, currently empty, is added to allow irq controller drivers to expose their initialization function to the main irqchip.c file. Note that the irq controller driver initialization function is responsible for setting the global handle_arch_irq() variable, so that ARM platforms no longer have to define the ->handle_irq field in their DT_MACHINE structure. A global header, <linux/irqchip.h> is also added to expose the single irqchip_init() function to the reset of the kernel. A further commit moves the BCM2835 irq controller driver to this new small infrastructure, therefore removing the include/linux/irqchip/ directory. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- drivers/irqchip/Kconfig | 3 ++- drivers/irqchip/Makefile | 1 + drivers/irqchip/irqchip.c | 23 +++++++++++++++++++++++ drivers/irqchip/irqchip.h | 14 ++++++++++++++ include/linux/irqchip.h | 16 ++++++++++++++++ 5 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 drivers/irqchip/irqchip.c create mode 100644 drivers/irqchip/irqchip.h create mode 100644 include/linux/irqchip.h