diff mbox series

[2/3] irqchip: Add support to remove irqchip driver modules.

Message ID 20210427113136.12469-3-anirudha.sarangi@xilinx.com (mailing list archive)
State New, archived
Headers show
Series Updates in irqchip framework to remove irqchip | expand

Commit Message

Anirudha Sarangi April 27, 2021, 11:31 a.m. UTC
The existing irqchip implementation does not fully support use cases
where an irqchip driver has to be used as a module. In particular there
is no support to remove an irqchip driver module.
The use cases where an irqchip driver has to be loaded and then removed
as a module are really relevant in fpga world. A user can decide to
have a irqchip as part of a removable partial fpga region. In such cases
not only the corresponding irqchip driver has to be loaded as a module,
but must also be removed when the removable partial region is removed.

The existing implementation updates the existing framework to achieve
the above said goal.

Signed-off-by: Anirudha Sarangi <anirudha.sarangi@xilinx.com>
---
 drivers/irqchip/irqchip.c | 38 +++++++++++++++++++++++++++++++++++---
 include/linux/irq.h       | 13 +++++++++++++
 include/linux/of_irq.h    |  1 +
 3 files changed, 49 insertions(+), 3 deletions(-)

Comments

Marc Zyngier April 27, 2021, 12:35 p.m. UTC | #1
On Tue, 27 Apr 2021 12:31:35 +0100,
Anirudha Sarangi <anirudha.sarangi@xilinx.com> wrote:
> 
> The existing irqchip implementation does not fully support use cases
> where an irqchip driver has to be used as a module. In particular there
> is no support to remove an irqchip driver module.
> The use cases where an irqchip driver has to be loaded and then removed
> as a module are really relevant in fpga world. A user can decide to
> have a irqchip as part of a removable partial fpga region. In such cases
> not only the corresponding irqchip driver has to be loaded as a module,
> but must also be removed when the removable partial region is removed.
> 
> The existing implementation updates the existing framework to achieve
> the above said goal.
> 
> Signed-off-by: Anirudha Sarangi <anirudha.sarangi@xilinx.com>

There is absolutely no way we can entertain the removal of an
interrupt controller based on *this*.

What happen to the irqdesc structures? What happen when a client
driver decides to do a disable_irq(), or any other interaction with
the interrupt controller that now has dangling pointers everywhere (if
your third patch is supposed to be an example of how to use this
functionality)?

So no, you can't do that until you figure out all the dependencies
that need to be accounted for to safely remove an interrupt
controller.

	M.
kernel test robot April 27, 2021, 2:46 p.m. UTC | #2
Hi Anirudha,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tip/irq/core]
[also build test WARNING on robh/for-next linus/master v5.12 next-20210427]
[cannot apply to xlnx/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Anirudha-Sarangi/Updates-in-irqchip-framework-to-remove-irqchip/20210427-193255
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 765822e1569a37aab5e69736c52d4ad4a289eba6
config: mips-randconfig-r033-20210427 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project d7308da4a5aaded897a7e0c06e7e88d81fc64879)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install mips cross compiling tool for clang build
        # apt-get install binutils-mips-linux-gnu
        # https://github.com/0day-ci/linux/commit/70da936a840d5e21d42f09996e9fc8c56e6a44ce
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Anirudha-Sarangi/Updates-in-irqchip-framework-to-remove-irqchip/20210427-193255
        git checkout 70da936a840d5e21d42f09996e9fc8c56e6a44ce
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/irqchip/irqchip.c:78:5: warning: no previous prototype for function 'platform_irqchip_remove' [-Wmissing-prototypes]
   int platform_irqchip_remove(struct platform_device *pdev)
       ^
   drivers/irqchip/irqchip.c:78:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int platform_irqchip_remove(struct platform_device *pdev)
   ^
   static 
   1 warning generated.


vim +/platform_irqchip_remove +78 drivers/irqchip/irqchip.c

    77	
  > 78	int platform_irqchip_remove(struct platform_device *pdev)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/irqchip/irqchip.c b/drivers/irqchip/irqchip.c
index 3570f0a588c4..8687131e7268 100644
--- a/drivers/irqchip/irqchip.c
+++ b/drivers/irqchip/irqchip.c
@@ -15,6 +15,11 @@ 
 #include <linux/irqchip.h>
 #include <linux/platform_device.h>
 
+struct platform_irqchip_instance {
+	of_irq_init_cb_t irq_init_cb;
+	of_irq_remove_cb_t irq_remove_cb;
+};
+
 /*
  * This special of_device_id is the sentinel at the end of the
  * of_device_id[] array of all irqchips. It is automatically placed at
@@ -34,11 +39,22 @@  void __init irqchip_init(void)
 
 int platform_irqchip_probe(struct platform_device *pdev)
 {
+	struct platform_irqchip_instance *irqchip;
+	const struct irqc_init_remove_funps *irqchip_funps;
 	struct device_node *np = pdev->dev.of_node;
 	struct device_node *par_np = of_irq_find_parent(np);
-	of_irq_init_cb_t irq_init_cb = of_device_get_match_data(&pdev->dev);
 
-	if (!irq_init_cb)
+	irqchip = devm_kzalloc(&pdev->dev, sizeof(*irqchip), GFP_KERNEL);
+	if (!irqchip)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, irqchip);
+
+	irqchip_funps = of_device_get_match_data(&pdev->dev);
+	irqchip->irq_init_cb =	irqchip_funps->irqchip_initp;
+	irqchip->irq_remove_cb = irqchip_funps->irqchip_removep;
+
+	if (!irqchip->irq_init_cb)
 		return -EINVAL;
 
 	if (par_np == np)
@@ -55,6 +71,22 @@  int platform_irqchip_probe(struct platform_device *pdev)
 	if (par_np && !irq_find_matching_host(par_np, DOMAIN_BUS_ANY))
 		return -EPROBE_DEFER;
 
-	return irq_init_cb(np, par_np);
+	return irqchip->irq_init_cb(np, par_np);
 }
 EXPORT_SYMBOL_GPL(platform_irqchip_probe);
+
+int platform_irqchip_remove(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *par_np = of_irq_find_parent(np);
+	struct platform_irqchip_instance *irqchip = platform_get_drvdata(pdev);
+
+	if (!irqchip->irq_remove_cb)
+		return -EINVAL;
+
+	if (par_np == np)
+		par_np = NULL;
+
+	return irqchip->irq_remove_cb(np, par_np);
+}
+EXPORT_SYMBOL_GPL(platform_irqchip_remove);
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 252fab8074de..d46d28f56bc0 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1098,6 +1098,19 @@  struct irq_domain_chip_generic {
 	struct irq_chip_generic	*gc[];
 };
 
+/**
+ * struct irqc_init_remove_funps - Stores function pointers for irqc init
+ * and remove APIs. Used when the irqchip driver is to be used as a module.
+ * @irqchip_initp:	Function pointer for init/entry point of a irqchip driver.
+ * @irqchip_removep:Function pointer for irqchip driver remove function.
+ */
+struct irqc_init_remove_funps {
+	int (*irqchip_initp)(struct device_node *irqc,
+			     struct device_node *parent);
+	int (*irqchip_removep)(struct device_node *irqc,
+			       struct device_node *parent);
+};
+
 /* Generic chip callback functions */
 void irq_gc_noop(struct irq_data *d);
 void irq_gc_mask_disable_reg(struct irq_data *d);
diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
index aaf219bd0354..4210ac64956f 100644
--- a/include/linux/of_irq.h
+++ b/include/linux/of_irq.h
@@ -10,6 +10,7 @@ 
 #include <linux/of.h>
 
 typedef int (*of_irq_init_cb_t)(struct device_node *, struct device_node *);
+typedef int (*of_irq_remove_cb_t)(struct device_node *, struct device_node *);
 
 /*
  * Workarounds only applied to 32bit powermac machines