[PATCH] drivers: irqchip: add irq-type-changer

[PATCH] drivers: irqchip: add irq-type-changer

[Nikita Yushchenko]

Irq type changer is a virtual irqchip useful to support boards that
change (e.g. invert) interrupt signal between producer and consumer.

Usage example, for Kingfisher extension board for Renesas Gen-3 Soc,
that has WiFi interrupt delivered over inverting level-shifter:

/ {
	gpio1_25_inverted: inverter {
		compatible = "linux,irq-type-changer";
		interrupt-controller;
		#interrupt-cells = <2>;
		interrupt-parent = <&gpio1>;
		interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
	};
};

&wlcore {
        interrupt-parent = <&gpio1_25_inverted>;
	interrupts = <0 IRQ_TYPE_EDGE_RISING>;
};

Then, wlcore driver observes IRQ_TYPE_EDGE_RISING trigger type and
configures interrupt output as such. At the same time, gpio-rcar driver
gets interrupt configured for IRQ_TYPE_EDGE_FALLING.

This version uses hierarchical irq_domain API, and works only with
parent interrupt domains compatible with that API.

Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
---
My previous attempt to solve the issue:
  https://lore.kernel.org/lkml/20211228165642.2514766-1-nikita.yoush@cogentembedded.com/

Additional patches required for this to work with gpio-rcar:
  https://lore.kernel.org/lkml/20220119160715.650535-1-nikita.yoush@cogentembedded.com/
  https://lore.kernel.org/lkml/20220119143211.633399-1-nikita.yoush@cogentembedded.com/

 drivers/irqchip/Kconfig            |  18 ++++
 drivers/irqchip/Makefile           |   1 +
 drivers/irqchip/irq-type-changer.c | 162 +++++++++++++++++++++++++++++
 3 files changed, 181 insertions(+)
 create mode 100644 drivers/irqchip/irq-type-changer.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 7038957f4a77..7f348016e82c 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -617,4 +617,22 @@ config MCHP_EIC
 	help
 	  Support for Microchip External Interrupt Controller.
 
+config IRQ_TYPE_CHANGER
+	bool "Interrupt trigger type changer"
+	select IRQ_DOMAIN
+	select IRQ_DOMAIN_HIERARCHY
+	help
+	  Interrupt trigger type changer is designed to support boards that
+	  modify (e.g. invert) signal between interrupt source and interrupt
+	  controller input. So trigger type configured by a driver for some
+	  interrupt output pin does not match trigger type that shall be used
+	  to configure interrupt controller's input where that pin is connected.
+
+	  In this case, board device tree shall add an interrupt trigger
+	  type changer node and use it as the interrupt parent for the node
+	  representing interrupt source. Then, interrupt trigger type defined
+	  in the interrupt source node will be visible for the interrupt source
+	  driver, and (different) trigger type defined inside the changer node
+	  will be used to configure the interrupt controller.
+
 endmenu
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index c1f611cbfbf8..57a664837857 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -117,3 +117,4 @@ obj-$(CONFIG_WPCM450_AIC)		+= irq-wpcm450-aic.o
 obj-$(CONFIG_IRQ_IDT3243X)		+= irq-idt3243x.o
 obj-$(CONFIG_APPLE_AIC)			+= irq-apple-aic.o
 obj-$(CONFIG_MCHP_EIC)			+= irq-mchp-eic.o
+obj-$(CONFIG_IRQ_TYPE_CHANGER)		+= irq-type-changer.o
diff --git a/drivers/irqchip/irq-type-changer.c b/drivers/irqchip/irq-type-changer.c
new file mode 100644
index 000000000000..731ea61e7fcc
--- /dev/null
+++ b/drivers/irqchip/irq-type-changer.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+#include <linux/of_irq.h>
+
+struct changer {
+	unsigned long count;
+	struct {
+		struct irq_fwspec fwspec;
+		unsigned int type;
+	} out[0];
+};
+
+static int changer_set_type(struct irq_data *data, unsigned int type)
+{
+	struct changer *ch = data->domain->host_data;
+	struct irq_data *parent_data = data->parent_data;
+
+	return parent_data->chip->irq_set_type(parent_data,
+					       ch->out[data->hwirq].type);
+}
+
+static struct irq_chip changer_chip = {
+	.name			= "type-changer",
+	.irq_mask		= irq_chip_mask_parent,
+	.irq_unmask		= irq_chip_unmask_parent,
+	.irq_eoi		= irq_chip_eoi_parent,
+	.irq_set_type		= changer_set_type,
+	.irq_retrigger		= irq_chip_retrigger_hierarchy,
+	.irq_set_affinity	= irq_chip_set_affinity_parent,
+	.irq_set_wake		= irq_chip_set_wake_parent,
+};
+
+static int changer_domain_translate(struct irq_domain *domain,
+				    struct irq_fwspec *fwspec,
+				    unsigned long *hwirq,
+				    unsigned int *type)
+{
+	struct changer *ch = domain->host_data;
+
+	if (fwspec->param_count != 2)
+		return -EINVAL;
+	if (fwspec->param[0] >= ch->count)
+		return -ENXIO;
+
+	*hwirq = fwspec->param[0];
+	*type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
+	return 0;
+}
+
+static int changer_domain_alloc(struct irq_domain *domain, unsigned int virq,
+				unsigned int nr_irqs, void *arg)
+{
+	struct changer *ch = domain->host_data;
+	struct irq_fwspec *fwspec = arg;
+	irq_hw_number_t hwirq;
+	unsigned int type;
+	int ret;
+
+	if (WARN_ON(nr_irqs != 1))
+		return -EINVAL;
+
+	ret = changer_domain_translate(domain, fwspec, &hwirq, &type);
+	if (ret)
+		return ret;
+
+	irq_domain_set_hwirq_and_chip(domain, virq, hwirq, &changer_chip, ch);
+
+	return irq_domain_alloc_irqs_parent(domain, virq, 1,
+					    &ch->out[hwirq].fwspec);
+}
+
+static const struct irq_domain_ops changer_domain_ops = {
+	.translate	= changer_domain_translate,
+	.alloc		= changer_domain_alloc,
+	.free		= irq_domain_free_irqs_common,
+};
+
+static int __init changer_of_init(struct device_node *node,
+				  struct device_node *parent)
+{
+	struct irq_domain *domain, *parent_domain;
+	int count, i, ret;
+	struct changer *ch;
+	struct of_phandle_args pargs;
+	irq_hw_number_t unused;
+
+	if (!parent) {
+		pr_err("%pOF: no parent node\n", node);
+		return -EINVAL;
+	}
+
+	parent_domain = irq_find_host(parent);
+	if (!parent_domain) {
+		pr_err("%pOF: no parent domain\n", node);
+		return -EINVAL;
+	}
+
+	if (WARN_ON(!parent_domain->ops->translate))
+		return -EINVAL;
+
+	count = of_irq_count(node);
+	if (count < 1) {
+		pr_err("%pOF: no interrupts defined\n", node);
+		return -EINVAL;
+	}
+
+	ch = kzalloc(GFP_KERNEL, sizeof(*ch) + count * sizeof(ch->out[0]));
+	if (!ch)
+		return -ENOMEM;
+	ch->count = count;
+
+	for (i = 0; i < count; i++) {
+		ret = of_irq_parse_one(node, i, &pargs);
+		if (ret) {
+			pr_err("%pOF: interrupt %d: error %d parsing\n",
+			       node, i, ret);
+			goto out_free;
+		}
+		of_phandle_args_to_fwspec(pargs.np, pargs.args,
+					  pargs.args_count,
+					  &ch->out[i].fwspec);
+		ret = parent_domain->ops->translate(parent_domain,
+						    &ch->out[i].fwspec,
+						    &unused,
+						    &ch->out[i].type);
+		if (ret) {
+			pr_err("%pOF: interrupt %d: error %d extracting type\n",
+			       node, i, ret);
+			goto out_free;
+		}
+		if (ch->out[i].type == IRQ_TYPE_NONE) {
+			pr_err("%pOF: interrupt %d: no type\n", node, i);
+			ret = -ENXIO;
+			goto out_free;
+		}
+	}
+
+	domain = irq_domain_create_hierarchy(parent_domain, 0, count,
+					     of_node_to_fwnode(node),
+					     &changer_domain_ops, ch);
+	if (!domain) {
+		ret = -ENOMEM;
+		goto out_free;
+	}
+
+	return 0;
+
+out_free:
+	kfree(ch);
+	return ret;
+}
+
+IRQCHIP_PLATFORM_DRIVER_BEGIN(changer)
+IRQCHIP_MATCH("linux,irq-type-changer", changer_of_init)
+IRQCHIP_PLATFORM_DRIVER_END(changer)
+MODULE_AUTHOR("Nikita Yushchenko <nikita.yoush.cogentembedded.com>");
+MODULE_DESCRIPTION("Virtual irqchip to support trigger type change in route");
+MODULE_LICENSE("GPL v2");
-- 
2.30.2

Re: [PATCH] drivers: irqchip: add irq-type-changer

[kernel test robot]

Hi Nikita,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tip/irq/core]
[also build test WARNING on v5.17-rc1 next-20220124]
[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/Nikita-Yushchenko/drivers-irqchip-add-irq-type-changer/20220120-041926
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 67d50b5f9114ae55d45e08e1fd1d6ae152622bf3
config: microblaze-randconfig-s032-20220124 (https://download.01.org/0day-ci/archive/20220124/202201241716.mt2XBMZQ-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 11.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/08692091f6fa9d1ee51baef78ce2adf983b2248a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Nikita-Yushchenko/drivers-irqchip-add-irq-type-changer/20220120-041926
        git checkout 08692091f6fa9d1ee51baef78ce2adf983b2248a
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=microblaze SHELL=/bin/bash drivers/irqchip/

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


sparse warnings: (new ones prefixed by >>)
>> drivers/irqchip/irq-type-changer.c:111:22: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] size @@     got restricted gfp_t @@
   drivers/irqchip/irq-type-changer.c:111:22: sparse:     expected unsigned int [usertype] size
   drivers/irqchip/irq-type-changer.c:111:22: sparse:     got restricted gfp_t
>> drivers/irqchip/irq-type-changer.c:111:46: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted gfp_t [usertype] flags @@     got unsigned int @@
   drivers/irqchip/irq-type-changer.c:111:46: sparse:     expected restricted gfp_t [usertype] flags
   drivers/irqchip/irq-type-changer.c:111:46: sparse:     got unsigned int

vim +111 drivers/irqchip/irq-type-changer.c

    81	
    82	static int __init changer_of_init(struct device_node *node,
    83					  struct device_node *parent)
    84	{
    85		struct irq_domain *domain, *parent_domain;
    86		int count, i, ret;
    87		struct changer *ch;
    88		struct of_phandle_args pargs;
    89		irq_hw_number_t unused;
    90	
    91		if (!parent) {
    92			pr_err("%pOF: no parent node\n", node);
    93			return -EINVAL;
    94		}
    95	
    96		parent_domain = irq_find_host(parent);
    97		if (!parent_domain) {
    98			pr_err("%pOF: no parent domain\n", node);
    99			return -EINVAL;
   100		}
   101	
   102		if (WARN_ON(!parent_domain->ops->translate))
   103			return -EINVAL;
   104	
   105		count = of_irq_count(node);
   106		if (count < 1) {
   107			pr_err("%pOF: no interrupts defined\n", node);
   108			return -EINVAL;
   109		}
   110	
 > 111		ch = kzalloc(GFP_KERNEL, sizeof(*ch) + count * sizeof(ch->out[0]));
   112		if (!ch)
   113			return -ENOMEM;
   114		ch->count = count;
   115	
   116		for (i = 0; i < count; i++) {
   117			ret = of_irq_parse_one(node, i, &pargs);
   118			if (ret) {
   119				pr_err("%pOF: interrupt %d: error %d parsing\n",
   120				       node, i, ret);
   121				goto out_free;
   122			}
   123			of_phandle_args_to_fwspec(pargs.np, pargs.args,
   124						  pargs.args_count,
   125						  &ch->out[i].fwspec);
   126			ret = parent_domain->ops->translate(parent_domain,
   127							    &ch->out[i].fwspec,
   128							    &unused,
   129							    &ch->out[i].type);
   130			if (ret) {
   131				pr_err("%pOF: interrupt %d: error %d extracting type\n",
   132				       node, i, ret);
   133				goto out_free;
   134			}
   135			if (ch->out[i].type == IRQ_TYPE_NONE) {
   136				pr_err("%pOF: interrupt %d: no type\n", node, i);
   137				ret = -ENXIO;
   138				goto out_free;
   139			}
   140		}
   141	
   142		domain = irq_domain_create_hierarchy(parent_domain, 0, count,
   143						     of_node_to_fwnode(node),
   144						     &changer_domain_ops, ch);
   145		if (!domain) {
   146			ret = -ENOMEM;
   147			goto out_free;
   148		}
   149	
   150		return 0;
   151	
   152	out_free:
   153		kfree(ch);
   154		return ret;
   155	}
   156	

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

Re: [PATCH] drivers: irqchip: add irq-type-changer

[Geert Uytterhoeven]

Hi Nikita,

On Wed, Jan 19, 2022 at 9:17 PM Nikita Yushchenko
<nikita.yoush@cogentembedded.com> wrote:
> Irq type changer is a virtual irqchip useful to support boards that
> change (e.g. invert) interrupt signal between producer and consumer.
>
> Usage example, for Kingfisher extension board for Renesas Gen-3 Soc,
> that has WiFi interrupt delivered over inverting level-shifter:
>
> / {
>         gpio1_25_inverted: inverter {
>                 compatible = "linux,irq-type-changer";
>                 interrupt-controller;
>                 #interrupt-cells = <2>;
>                 interrupt-parent = <&gpio1>;
>                 interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
>         };
> };
>
> &wlcore {
>         interrupt-parent = <&gpio1_25_inverted>;
>         interrupts = <0 IRQ_TYPE_EDGE_RISING>;
> };
>
> Then, wlcore driver observes IRQ_TYPE_EDGE_RISING trigger type and
> configures interrupt output as such. At the same time, gpio-rcar driver
> gets interrupt configured for IRQ_TYPE_EDGE_FALLING.
>
> This version uses hierarchical irq_domain API, and works only with
> parent interrupt domains compatible with that API.
>
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/irqchip/irq-type-changer.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/irqchip.h>
> +#include <linux/irqdomain.h>
> +#include <linux/of_irq.h>
> +
> +struct changer {
> +       unsigned long count;
> +       struct {
> +               struct irq_fwspec fwspec;
> +               unsigned int type;
> +       } out[0];

Please use [] instead of [0] for flexible arrays, else the compiler
doesn't realize it is a flexible array, and won't complain if you add
more members below.

> +};

> +static int __init changer_of_init(struct device_node *node,
> +                                 struct device_node *parent)
> +{
> +       struct irq_domain *domain, *parent_domain;
> +       int count, i, ret;
> +       struct changer *ch;
> +       struct of_phandle_args pargs;
> +       irq_hw_number_t unused;
> +
> +       if (!parent) {
> +               pr_err("%pOF: no parent node\n", node);
> +               return -EINVAL;
> +       }
> +
> +       parent_domain = irq_find_host(parent);
> +       if (!parent_domain) {
> +               pr_err("%pOF: no parent domain\n", node);
> +               return -EINVAL;
> +       }
> +
> +       if (WARN_ON(!parent_domain->ops->translate))
> +               return -EINVAL;
> +
> +       count = of_irq_count(node);
> +       if (count < 1) {
> +               pr_err("%pOF: no interrupts defined\n", node);
> +               return -EINVAL;
> +       }
> +
> +       ch = kzalloc(GFP_KERNEL, sizeof(*ch) + count * sizeof(ch->out[0]));

Oops, wrong parameter order, as detected by the  kernel test robot.
Please use struct_size() to simplify and harden the size calculation.


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