| Message ID | 20250302-04-gpio-irq-threecell-v2-1-34f13ad37ea4@gentoo.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: irq: support describing three-cell interrupts | expand |
| Context | Check | Description |
|---|---|---|
| bjorn/pre-ci_am | success | Success |
| bjorn/build-rv32-defconfig | success | build-rv32-defconfig |
| bjorn/build-rv64-clang-allmodconfig | success | build-rv64-clang-allmodconfig |
| bjorn/build-rv64-gcc-allmodconfig | success | build-rv64-gcc-allmodconfig |
| bjorn/build-rv64-nommu-k210-defconfig | success | build-rv64-nommu-k210-defconfig |
| bjorn/build-rv64-nommu-k210-virt | success | build-rv64-nommu-k210-virt |
| bjorn/checkpatch | success | checkpatch |
| bjorn/dtb-warn-rv64 | success | dtb-warn-rv64 |
| bjorn/header-inline | success | header-inline |
| bjorn/kdoc | success | kdoc |
| bjorn/module-param | success | module-param |
| bjorn/verify-fixes | success | verify-fixes |
| bjorn/verify-signedoff | success | verify-signedoff |
On Sun, Mar 02 2025 at 07:15, Yixun Lan wrote: > The is a prerequisite patch to support parsing three-cell > interrupts which encoded as <instance hwirq irqflag>, > the translate function will always retrieve irq number and > flag from last two cells. > > In this patch, we introduce a generic interrupt cells translation > function, others functions will be inline version. Please read: https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-changes > +int irq_domain_translate_cells(struct irq_domain *d, > + struct irq_fwspec *fwspec, > + unsigned long *out_hwirq, > + unsigned int *out_type); Please get rid of the extra line breaks. You have 100 (99) characters available. > +static inline int irq_domain_translate_onecell(struct irq_domain *d, > + struct irq_fwspec *fwspec, > + unsigned long *out_hwirq, > + unsigned int *out_type) > +{ > + return irq_domain_translate_cells(d, fwspec, out_hwirq, out_type); > +} > + > +static inline int irq_domain_translate_twocell(struct irq_domain *d, > + struct irq_fwspec *fwspec, > + unsigned long *out_hwirq, > + unsigned int *out_type) > +{ > + return irq_domain_translate_cells(d, fwspec, out_hwirq, out_type); > +} > + > +static inline int irq_domain_translate_threecell(struct irq_domain *d, > + struct irq_fwspec *fwspec, > + unsigned long *out_hwirq, > + unsigned int *out_type) > +{ > + return irq_domain_translate_cells(d, fwspec, out_hwirq, out_type); > +} What's this for? It's not used. The onecell/twocell wrappers are just there to keep the current code working. > +int irq_domain_translate_cells(struct irq_domain *d, > + struct irq_fwspec *fwspec, > + unsigned long *out_hwirq, > + unsigned int *out_type) Please remove the extra line breaks. int irq_domain_translate_cells(struct irq_domain *d, struct irq_fwspec *fwspec, unsigned long *out_hwirq, unsigned int *out_type) is perfectly fine. > { > - if (WARN_ON(fwspec->param_count < 1)) > - return -EINVAL; > - *out_hwirq = fwspec->param[0]; > - *out_type = IRQ_TYPE_NONE; > - return 0; > -} > -EXPORT_SYMBOL_GPL(irq_domain_translate_onecell); > + unsigned int cells = fwspec->param_count; > > -/** > - * irq_domain_translate_twocell() - Generic translate for direct two cell > - * bindings > - * @d: Interrupt domain involved in the translation > - * @fwspec: The firmware interrupt specifier to translate > - * @out_hwirq: Pointer to storage for the hardware interrupt number > - * @out_type: Pointer to storage for the interrupt type > - * > - * Device Tree IRQ specifier translation function which works with two cell > - * bindings where the cell values map directly to the hwirq number > - * and linux irq flags. > - */ > -int irq_domain_translate_twocell(struct irq_domain *d, > - struct irq_fwspec *fwspec, > - unsigned long *out_hwirq, > - unsigned int *out_type) > -{ > - if (WARN_ON(fwspec->param_count < 2)) > + switch (cells) { > + case 1: > + *out_hwirq = fwspec->param[0]; > + *out_type = IRQ_TYPE_NONE; > + return 0; > + case 2 ... 3: I have second thoughts about this when looking deeper. The current one/two cell implementations validate that param_count is at least the number of parameters. Which means that the parameter count could be larger, but only evaluates the first one or the first two. I have no idea whether this matters or not, but arguably a two cell fwspec could be successfully fed into translate_onecell(), no? And that triggers a related question. Why is the three cell translation not following the one/two cell scheme and has the parameters at the same place (index 0,1), i.e. adding the extra information at the end? That makes sense to me as the extra cell is obviously not directly related to the interrupt mapping. Thanks, tglx
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h index e432b6a12a32f9f16ec1ea2fa8e24a649d55caae..d96796263a2e177140f928cb369656a44dd45dda 100644 --- a/include/linux/irqdomain.h +++ b/include/linux/irqdomain.h @@ -572,15 +572,34 @@ int irq_domain_xlate_onetwocell(struct irq_domain *d, struct device_node *ctrlr, const u32 *intspec, unsigned int intsize, irq_hw_number_t *out_hwirq, unsigned int *out_type); -int irq_domain_translate_twocell(struct irq_domain *d, - struct irq_fwspec *fwspec, - unsigned long *out_hwirq, - unsigned int *out_type); - -int irq_domain_translate_onecell(struct irq_domain *d, - struct irq_fwspec *fwspec, - unsigned long *out_hwirq, - unsigned int *out_type); +int irq_domain_translate_cells(struct irq_domain *d, + struct irq_fwspec *fwspec, + unsigned long *out_hwirq, + unsigned int *out_type); + +static inline int irq_domain_translate_onecell(struct irq_domain *d, + struct irq_fwspec *fwspec, + unsigned long *out_hwirq, + unsigned int *out_type) +{ + return irq_domain_translate_cells(d, fwspec, out_hwirq, out_type); +} + +static inline int irq_domain_translate_twocell(struct irq_domain *d, + struct irq_fwspec *fwspec, + unsigned long *out_hwirq, + unsigned int *out_type) +{ + return irq_domain_translate_cells(d, fwspec, out_hwirq, out_type); +} + +static inline int irq_domain_translate_threecell(struct irq_domain *d, + struct irq_fwspec *fwspec, + unsigned long *out_hwirq, + unsigned int *out_type) +{ + return irq_domain_translate_cells(d, fwspec, out_hwirq, out_type); +} /* IPI functions */ int irq_reserve_ipi(struct irq_domain *domain, const struct cpumask *dest); diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index ec6d8e72d980f604ded2bfa2143420e0e0095920..8d3b357b7dedbb2c274d4761c315e430b1d35610 100644 --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -1171,50 +1171,38 @@ const struct irq_domain_ops irq_domain_simple_ops = { EXPORT_SYMBOL_GPL(irq_domain_simple_ops); /** - * irq_domain_translate_onecell() - Generic translate for direct one cell + * irq_domain_translate_cells() - Generic translate for up to three cells * bindings * @d: Interrupt domain involved in the translation * @fwspec: The firmware interrupt specifier to translate * @out_hwirq: Pointer to storage for the hardware interrupt number * @out_type: Pointer to storage for the interrupt type */ -int irq_domain_translate_onecell(struct irq_domain *d, - struct irq_fwspec *fwspec, - unsigned long *out_hwirq, - unsigned int *out_type) +int irq_domain_translate_cells(struct irq_domain *d, + struct irq_fwspec *fwspec, + unsigned long *out_hwirq, + unsigned int *out_type) { - if (WARN_ON(fwspec->param_count < 1)) - return -EINVAL; - *out_hwirq = fwspec->param[0]; - *out_type = IRQ_TYPE_NONE; - return 0; -} -EXPORT_SYMBOL_GPL(irq_domain_translate_onecell); + unsigned int cells = fwspec->param_count; -/** - * irq_domain_translate_twocell() - Generic translate for direct two cell - * bindings - * @d: Interrupt domain involved in the translation - * @fwspec: The firmware interrupt specifier to translate - * @out_hwirq: Pointer to storage for the hardware interrupt number - * @out_type: Pointer to storage for the interrupt type - * - * Device Tree IRQ specifier translation function which works with two cell - * bindings where the cell values map directly to the hwirq number - * and linux irq flags. - */ -int irq_domain_translate_twocell(struct irq_domain *d, - struct irq_fwspec *fwspec, - unsigned long *out_hwirq, - unsigned int *out_type) -{ - if (WARN_ON(fwspec->param_count < 2)) + switch (cells) { + case 1: + *out_hwirq = fwspec->param[0]; + *out_type = IRQ_TYPE_NONE; + return 0; + case 2 ... 3: + /* + * For multi cell translations the hardware interrupt number + * and type are in the last two cells. + */ + *out_hwirq = fwspec->param[cells - 2]; + *out_type = fwspec->param[cells - 1] & IRQ_TYPE_SENSE_MASK; + return 0; + default: return -EINVAL; - *out_hwirq = fwspec->param[0]; - *out_type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK; - return 0; + } } -EXPORT_SYMBOL_GPL(irq_domain_translate_twocell); +EXPORT_SYMBOL_GPL(irq_domain_translate_cells); int irq_domain_alloc_descs(int virq, unsigned int cnt, irq_hw_number_t hwirq, int node, const struct irq_affinity_desc *affinity)
The is a prerequisite patch to support parsing three-cell interrupts which encoded as <instance hwirq irqflag>, the translate function will always retrieve irq number and flag from last two cells. In this patch, we introduce a generic interrupt cells translation function, others functions will be inline version. Signed-off-by: Yixun Lan <dlan@gentoo.org> --- include/linux/irqdomain.h | 37 +++++++++++++++++++++++-------- kernel/irq/irqdomain.c | 56 +++++++++++++++++++---------------------------- 2 files changed, 50 insertions(+), 43 deletions(-)