Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Mon, 24 Jan 2022 10:57:44 +0100 | Subject | Re: [PATCH] drivers: irqchip: add irq-type-changer |
| |
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
| |