2020-01-13 07:14:18

by Joakim Zhang

[permalink] [raw]
Subject: [PATCH V4 RESEND 0/2] irqchip: add NXP INTMUX interrupt controller

This patch set adds driver for NXP INTMUX interrupt controller.

ChangeLogs:
V3->V4:
*set IRQ_TYPE_LEVEL_HIGH flag in .xlate callback.
*fix comment format.
*use an intermediate variable for irq_domain_add_linear().
*disable interrupts before enabling chained interrupt.
*disable interrupt in imx_remove() for level interrupt.
*convert binding to DT schema.

V2->V3:
*impletement .xlate and .select callback.

V1->V2:
*squash patches:
drivers/irqchip: enable INTMUX interrupt controller driver
drivers/irqchip: add NXP INTMUX interrupt multiplexer support
*remove properity "fsl,intmux_chans", only support channel 0 by
default.
*delete two unused macros.
*align the various field in struct intmux_data.
*turn to spin lock _irqsave version.
*delete struct intmux_irqchip_data.
*disable interrupt in probe stage and clear pending status in remove
stage.

Joakim Zhang (2):
dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer
drivers/irqchip: add NXP INTMUX interrupt multiplexer support

.../interrupt-controller/fsl,intmux.yaml | 77 +++++
drivers/irqchip/Kconfig | 6 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-imx-intmux.c | 309 ++++++++++++++++++
4 files changed, 393 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
create mode 100644 drivers/irqchip/irq-imx-intmux.c

--
2.17.1


2020-01-13 07:14:25

by Joakim Zhang

[permalink] [raw]
Subject: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer

This patch adds the DT bindings for the NXP INTMUX interrupt multiplexer
for i.MX8 family SoCs.

Signed-off-by: Joakim Zhang <[email protected]>
---
.../interrupt-controller/fsl,intmux.yaml | 77 +++++++++++++++++++
1 file changed, 77 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml

diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
new file mode 100644
index 000000000000..4dba532fe0bd
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/fsl,intmux.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale INTMUX interrupt multiplexer
+
+maintainers:
+ - Marc Zyngier <[email protected]>
+
+properties:
+ compatible:
+ items:
+ const: fsl,imx-intmux
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ minItems: 1
+ maxItems: 8
+ description: |
+ Should contain the parent interrupt lines (up to 8) used to multiplex
+ the input interrupts.
+
+ interrupt-controller: true
+
+ '#interrupt-cells':
+ const: 2
+
+ clocks:
+ maxItems: 1
+ description: ipg clock.
+
+ clock-names:
+ items:
+ const: ipg
+
+ fsl,intmux_chans:
+ maxItems: 1
+ description: |
+ The number of channels used for interrupt source. The Maximum value is 8.
+ If this property is not set in DT then driver uses 1 channel by default.
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - interrupt-controller
+ - '#interrupt-cells'
+ - clocks
+ - clock-name
+ - fsl,intmux_chans
+
+additionalProperties: false
+
+Example:
+
+ intmux@37400000 {
+ compatible = "fsl,imx-intmux";
+ reg = <0x37400000 0x1000>;
+ interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ clocks = <&clk IMX8QM_CM40_IPG_CLK>;
+ clock-names = "ipg";
+ fsl,intmux_chans = <8>;
+ };
+
--
2.17.1

2020-01-13 07:14:29

by Joakim Zhang

[permalink] [raw]
Subject: [PATCH V4 RESEND 2/2] drivers/irqchip: add NXP INTMUX interrupt multiplexer support

The Interrupt Multiplexer (INTMUX) expands the number of peripherals
that can interrupt the core:
* The INTMUX has 8 channels that are assigned to 8 NVIC interrupt slots.
* Each INTMUX channel can receive up to 32 interrupt sources and has 1
interrupt output.
* The INTMUX routes the interrupt sources to the interrupt outputs.

Signed-off-by: Shengjiu Wang <[email protected]>
Signed-off-by: Joakim Zhang <[email protected]>
---
drivers/irqchip/Kconfig | 6 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-imx-intmux.c | 309 +++++++++++++++++++++++++++++++
3 files changed, 316 insertions(+)
create mode 100644 drivers/irqchip/irq-imx-intmux.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index ba152954324b..7e2b1e9d0b45 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -457,6 +457,12 @@ config IMX_IRQSTEER
help
Support for the i.MX IRQSTEER interrupt multiplexer/remapper.

+config IMX_INTMUX
+ def_bool y if ARCH_MXC
+ select IRQ_DOMAIN
+ help
+ Support for the i.MX INTMUX interrupt multiplexer.
+
config LS1X_IRQ
bool "Loongson-1 Interrupt Controller"
depends on MACH_LOONGSON32
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e806dda690ea..af976a79d1fb 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_CSKY_MPINTC) += irq-csky-mpintc.o
obj-$(CONFIG_CSKY_APB_INTC) += irq-csky-apb-intc.o
obj-$(CONFIG_SIFIVE_PLIC) += irq-sifive-plic.o
obj-$(CONFIG_IMX_IRQSTEER) += irq-imx-irqsteer.o
+obj-$(CONFIG_IMX_INTMUX) += irq-imx-intmux.o
obj-$(CONFIG_MADERA_IRQ) += irq-madera.o
obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
diff --git a/drivers/irqchip/irq-imx-intmux.c b/drivers/irqchip/irq-imx-intmux.c
new file mode 100644
index 000000000000..bfd5a94ddd6d
--- /dev/null
+++ b/drivers/irqchip/irq-imx-intmux.c
@@ -0,0 +1,309 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright 2017 NXP
+
+/* INTMUX Block Diagram
+ *
+ * ________________
+ * interrupt source # 0 +---->| |
+ * | | |
+ * interrupt source # 1 +++-->| |
+ * ... | | | channel # 0 |--------->interrupt out # 0
+ * ... | | | |
+ * ... | | | |
+ * interrupt source # X-1 +++-->|________________|
+ * | | |
+ * | | |
+ * | | | ________________
+ * +---->| |
+ * | | | | |
+ * | +-->| |
+ * | | | | channel # 1 |--------->interrupt out # 1
+ * | | +>| |
+ * | | | | |
+ * | | | |________________|
+ * | | |
+ * | | |
+ * | | | ...
+ * | | | ...
+ * | | |
+ * | | | ________________
+ * +---->| |
+ * | | | |
+ * +-->| |
+ * | | channel # N |--------->interrupt out # N
+ * +>| |
+ * | |
+ * |________________|
+ *
+ *
+ * N: Interrupt Channel Instance Number (N=7)
+ * X: Interrupt Source Number for each channel (X=32)
+ *
+ * The INTMUX interrupt multiplexer has 8 channels, each channel receives 32
+ * interrupt sources and generates 1 interrupt output.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/spinlock.h>
+
+#define CHANIER(n) (0x10 + (0x40 * n))
+#define CHANIPR(n) (0x20 + (0x40 * n))
+
+#define CHAN_MAX_NUM 0x8
+
+struct intmux_irqchip_data {
+ int chanidx;
+ int irq;
+ struct irq_domain *domain;
+};
+
+struct intmux_data {
+ raw_spinlock_t lock;
+ void __iomem *regs;
+ struct clk *ipg_clk;
+ int channum;
+ struct intmux_irqchip_data irqchip_data[];
+};
+
+static void imx_intmux_irq_mask(struct irq_data *d)
+{
+ struct intmux_irqchip_data *irqchip_data = d->chip_data;
+ int idx = irqchip_data->chanidx;
+ struct intmux_data *data = container_of(irqchip_data, struct intmux_data,
+ irqchip_data[idx]);
+ unsigned long flags;
+ void __iomem *reg;
+ u32 val;
+
+ raw_spin_lock_irqsave(&data->lock, flags);
+ reg = data->regs + CHANIER(idx);
+ val = readl_relaxed(reg);
+ /* disable the interrupt source of this channel */
+ val &= ~BIT(d->hwirq);
+ writel_relaxed(val, reg);
+ raw_spin_unlock_irqrestore(&data->lock, flags);
+}
+
+static void imx_intmux_irq_unmask(struct irq_data *d)
+{
+ struct intmux_irqchip_data *irqchip_data = d->chip_data;
+ int idx = irqchip_data->chanidx;
+ struct intmux_data *data = container_of(irqchip_data, struct intmux_data,
+ irqchip_data[idx]);
+ unsigned long flags;
+ void __iomem *reg;
+ u32 val;
+
+ raw_spin_lock_irqsave(&data->lock, flags);
+ reg = data->regs + CHANIER(idx);
+ val = readl_relaxed(reg);
+ /* enable the interrupt source of this channel */
+ val |= BIT(d->hwirq);
+ writel_relaxed(val, reg);
+ raw_spin_unlock_irqrestore(&data->lock, flags);
+}
+
+static struct irq_chip imx_intmux_irq_chip = {
+ .name = "intmux",
+ .irq_mask = imx_intmux_irq_mask,
+ .irq_unmask = imx_intmux_irq_unmask,
+};
+
+static int imx_intmux_irq_map(struct irq_domain *h, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ irq_set_chip_data(irq, h->host_data);
+ irq_set_chip_and_handler(irq, &imx_intmux_irq_chip, handle_level_irq);
+
+ return 0;
+}
+
+static int imx_intmux_irq_xlate(struct irq_domain *d, struct device_node *node,
+ const u32 *intspec, unsigned int intsize,
+ unsigned long *out_hwirq, unsigned int *out_type)
+{
+ struct intmux_irqchip_data *irqchip_data = d->host_data;
+ int idx = irqchip_data->chanidx;
+ struct intmux_data *data = container_of(irqchip_data, struct intmux_data,
+ irqchip_data[idx]);
+
+ /*
+ * two cells needed in interrupt specifier:
+ * the 1st cell: hw interrupt number
+ * the 2nd cell: channel index
+ */
+ if (WARN_ON(intsize != 2))
+ return -EINVAL;
+
+ if (WARN_ON(intspec[1] >= data->channum))
+ return -EINVAL;
+
+ *out_hwirq = intspec[0];
+ *out_type = IRQ_TYPE_LEVEL_HIGH;
+
+ return 0;
+}
+
+static int imx_intmux_irq_select(struct irq_domain *d, struct irq_fwspec *fwspec,
+ enum irq_domain_bus_token bus_token)
+{
+ struct intmux_irqchip_data *irqchip_data = d->host_data;
+
+ /* Not for us */
+ if (fwspec->fwnode != d->fwnode)
+ return false;
+
+ return irqchip_data->chanidx == fwspec->param[1];
+}
+
+static const struct irq_domain_ops imx_intmux_domain_ops = {
+ .map = imx_intmux_irq_map,
+ .xlate = imx_intmux_irq_xlate,
+ .select = imx_intmux_irq_select,
+};
+
+static void imx_intmux_irq_handler(struct irq_desc *desc)
+{
+ struct intmux_irqchip_data *irqchip_data = irq_desc_get_handler_data(desc);
+ int idx = irqchip_data->chanidx;
+ struct intmux_data *data = container_of(irqchip_data, struct intmux_data,
+ irqchip_data[idx]);
+ unsigned long irqstat;
+ int pos, virq;
+
+ chained_irq_enter(irq_desc_get_chip(desc), desc);
+
+ /* read the interrupt source pending status of this channel */
+ irqstat = readl_relaxed(data->regs + CHANIPR(idx));
+
+ for_each_set_bit(pos, &irqstat, 32) {
+ virq = irq_find_mapping(irqchip_data->domain, pos);
+ if (virq)
+ generic_handle_irq(virq);
+ }
+
+ chained_irq_exit(irq_desc_get_chip(desc), desc);
+}
+
+static int imx_intmux_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct irq_domain *domain;
+ struct intmux_data *data;
+ int channum;
+ int i, ret;
+
+ ret = of_property_read_u32(np, "fsl,intmux_chans", &channum);
+ if (ret) {
+ channum = 1;
+ } else if (channum > CHAN_MAX_NUM) {
+ dev_err(&pdev->dev, "supports up to %d multiplex channels\n",
+ CHAN_MAX_NUM);
+ return -EINVAL;
+ }
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data) +
+ channum * sizeof(data->irqchip_data[0]), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->regs = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(data->regs)) {
+ dev_err(&pdev->dev, "failed to initialize reg\n");
+ return PTR_ERR(data->regs);
+ }
+
+ data->ipg_clk = devm_clk_get(&pdev->dev, "ipg");
+ if (IS_ERR(data->ipg_clk)) {
+ ret = PTR_ERR(data->ipg_clk);
+ if (ret != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "failed to get ipg clk: %d\n", ret);
+ return ret;
+ }
+
+ data->channum = channum;
+ raw_spin_lock_init(&data->lock);
+
+ ret = clk_prepare_enable(data->ipg_clk);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to enable ipg clk: %d\n", ret);
+ return ret;
+ }
+
+ for (i = 0; i < channum; i++) {
+ data->irqchip_data[i].chanidx = i;
+
+ data->irqchip_data[i].irq = irq_of_parse_and_map(np, i);
+ if (data->irqchip_data[i].irq <= 0) {
+ ret = -EINVAL;
+ dev_err(&pdev->dev, "failed to get irq\n");
+ goto out;
+ }
+
+ domain = irq_domain_add_linear(np, 32, &imx_intmux_domain_ops,
+ &data->irqchip_data[i]);
+ if (!domain) {
+ ret = -ENOMEM;
+ dev_err(&pdev->dev, "failed to create IRQ domain\n");
+ goto out;
+ }
+ data->irqchip_data[i].domain = domain;
+
+ /* disable all interrupt sources of this channel firstly */
+ writel_relaxed(0, data->regs + CHANIER(i));
+
+ irq_set_chained_handler_and_data(data->irqchip_data[i].irq,
+ imx_intmux_irq_handler,
+ &data->irqchip_data[i]);
+ }
+
+ platform_set_drvdata(pdev, data);
+
+ return 0;
+out:
+ clk_disable_unprepare(data->ipg_clk);
+ return ret;
+}
+
+static int imx_intmux_remove(struct platform_device *pdev)
+{
+ struct intmux_data *data = platform_get_drvdata(pdev);
+ int i;
+
+ for (i = 0; i < data->channum; i++) {
+ /* disable all interrupt sources of this channel */
+ writel_relaxed(0, data->regs + CHANIER(i));
+
+ irq_set_chained_handler_and_data(data->irqchip_data[i].irq,
+ NULL, NULL);
+
+ irq_domain_remove(data->irqchip_data[i].domain);
+ }
+
+ clk_disable_unprepare(data->ipg_clk);
+
+ return 0;
+}
+
+static const struct of_device_id imx_intmux_id[] = {
+ { .compatible = "fsl,imx-intmux", },
+ { /* sentinel */ },
+};
+
+static struct platform_driver imx_intmux_driver = {
+ .driver = {
+ .name = "imx-intmux",
+ .of_match_table = imx_intmux_id,
+ },
+ .probe = imx_intmux_probe,
+ .remove = imx_intmux_remove,
+};
+builtin_platform_driver(imx_intmux_driver);
--
2.17.1

2020-01-13 09:17:43

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer

On 2020-01-13 07:08, Joakim Zhang wrote:
> This patch adds the DT bindings for the NXP INTMUX interrupt
> multiplexer
> for i.MX8 family SoCs.
>
> Signed-off-by: Joakim Zhang <[email protected]>
> ---
> .../interrupt-controller/fsl,intmux.yaml | 77 +++++++++++++++++++
> 1 file changed, 77 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
> b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
> new file mode 100644
> index 000000000000..4dba532fe0bd
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id:
> http://devicetree.org/schemas/interrupt-controller/fsl,intmux.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale INTMUX interrupt multiplexer
> +
> +maintainers:
> + - Marc Zyngier <[email protected]>

Err... No. I have absolutely no desire to maintain this binding on its
own.
Feel free to add yourself as the maintainer for this file, as I'm merely
the conduit for updates to irqchip DT bindings.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

2020-01-13 10:00:55

by Joakim Zhang

[permalink] [raw]
Subject: RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer


> -----Original Message-----
> From: Marc Zyngier <[email protected]>
> Sent: 2020??1??13?? 17:16
> To: Joakim Zhang <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>; Andy Duan <[email protected]>
> Subject: Re: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP
> INTMUX interrupt multiplexer
>
> On 2020-01-13 07:08, Joakim Zhang wrote:
> > This patch adds the DT bindings for the NXP INTMUX interrupt
> > multiplexer for i.MX8 family SoCs.
> >
> > Signed-off-by: Joakim Zhang <[email protected]>
> > ---
> > .../interrupt-controller/fsl,intmux.yaml | 77 +++++++++++++++++++
> > 1 file changed, 77 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.ya
> > ml
> > b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.ya
> > ml
> > new file mode 100644
> > index 000000000000..4dba532fe0bd
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.ya
> > ml
> > @@ -0,0 +1,77 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevic
> >
> etree.org%2Fschemas%2Finterrupt-controller%2Ffsl%2Cintmux.yaml%23&am
> p;
> >
> data=02%7C01%7Cqiangqing.zhang%40nxp.com%7C3f74ea5d4ee84dfcd49908
> d7980
> >
> 941f2%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637145037851
> 545924&
> >
> amp;sdata=4wYFFndzX9ecexaAkGEE3hQL2wuHL17LUrWp84wHyY4%3D&amp;
> reserved=
> > 0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=02%7C01%7Cqia
> ngqing
> >
> +.zhang%40nxp.com%7C3f74ea5d4ee84dfcd49908d7980941f2%7C686ea1d3b
> c2b4c6
> >
> +fa92cd99c5c301635%7C0%7C0%7C637145037851545924&amp;sdata=2NbrC
> AgjDBrp
> > +lzYPulVsY%2FmHgVupsonwwK1gn%2BsdB7o%3D&amp;reserved=0
> > +
> > +title: Freescale INTMUX interrupt multiplexer
> > +
> > +maintainers:
> > + - Marc Zyngier <[email protected]>
>
> Err... No. I have absolutely no desire to maintain this binding on its own.
> Feel free to add yourself as the maintainer for this file, as I'm merely the
> conduit for updates to irqchip DT bindings.

Got it! Thanks:-)

Best Regards,
Joakim Zhang
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...

2020-01-13 21:05:54

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer

On Mon, Jan 13, 2020 at 03:08:40PM +0800, Joakim Zhang wrote:
> This patch adds the DT bindings for the NXP INTMUX interrupt multiplexer
> for i.MX8 family SoCs.
>
> Signed-off-by: Joakim Zhang <[email protected]>
> ---
> .../interrupt-controller/fsl,intmux.yaml | 77 +++++++++++++++++++
> 1 file changed, 77 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml

Please run 'make dt_binding_check' and fix the errors:

Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml:
while scanning for the next token found character that cannot start any token
in "<unicode string>", line 60, column 1

>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
> new file mode 100644
> index 000000000000..4dba532fe0bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/fsl,intmux.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale INTMUX interrupt multiplexer
> +
> +maintainers:
> + - Marc Zyngier <[email protected]>
> +
> +properties:
> + compatible:
> + items:
> + const: fsl,imx-intmux
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 1
> + maxItems: 8
> + description: |
> + Should contain the parent interrupt lines (up to 8) used to multiplex
> + the input interrupts.
> +
> + interrupt-controller: true
> +
> + '#interrupt-cells':
> + const: 2
> +
> + clocks:
> + maxItems: 1
> + description: ipg clock.
> +
> + clock-names:
> + items:
> + const: ipg
> +
> + fsl,intmux_chans:

Don't use '_' in property names.

Is this any different from the length of 'interrupts' which you can
count?

> + maxItems: 1
> + description: |
> + The number of channels used for interrupt source. The Maximum value is 8.
> + If this property is not set in DT then driver uses 1 channel by default.
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - interrupt-controller
> + - '#interrupt-cells'
> + - clocks
> + - clock-name
> + - fsl,intmux_chans
> +
> +additionalProperties: false
> +
> +Example:
> +
> + intmux@37400000 {

interrupt-controller@...

> + compatible = "fsl,imx-intmux";
> + reg = <0x37400000 0x1000>;
> + interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + clocks = <&clk IMX8QM_CM40_IPG_CLK>;
> + clock-names = "ipg";
> + fsl,intmux_chans = <8>;
> + };
> +
> --
> 2.17.1
>

2020-01-14 02:45:37

by Joakim Zhang

[permalink] [raw]
Subject: RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer


> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: 2020??1??14?? 5:04
> To: Joakim Zhang <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>; Andy Duan <[email protected]>
> Subject: Re: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP
> INTMUX interrupt multiplexer
>
> On Mon, Jan 13, 2020 at 03:08:40PM +0800, Joakim Zhang wrote:
> > This patch adds the DT bindings for the NXP INTMUX interrupt
> > multiplexer for i.MX8 family SoCs.
> >
> > Signed-off-by: Joakim Zhang <[email protected]>
> > ---
> > .../interrupt-controller/fsl,intmux.yaml | 77 +++++++++++++++++++
> > 1 file changed, 77 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
>
> Please run 'make dt_binding_check' and fix the errors:
>
> Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml:
> while scanning for the next token found character that cannot start any token
> in "<unicode string>", line 60, column 1
Got it. Will keep in mind. Thanks.

> >
> > diff --git
> > a/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.ya
> > ml
> > b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.ya
> > ml
> > new file mode 100644
> > index 000000000000..4dba532fe0bd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmu
> > +++ x.yaml
> > @@ -0,0 +1,77 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Finterrupt-controller%2Ffsl%2Cintmux.yaml%23&a
> m
> >
> +p;data=02%7C01%7Cqiangqing.zhang%40nxp.com%7Cdc2443dc111149805c7
> 208d7
> >
> +986c157f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63714546
> 2291934
> >
> +492&amp;sdata=Ao4iuj2D48KAeC%2FvQvJqUUxGJEjSY0HyL5ZlT2XrSrg%3D&
> amp;re
> > +served=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=02%7C01%7Cqia
> ngqing
> >
> +.zhang%40nxp.com%7Cdc2443dc111149805c7208d7986c157f%7C686ea1d3b
> c2b4c6
> >
> +fa92cd99c5c301635%7C0%7C0%7C637145462291934492&amp;sdata=YoHb
> TO5C8Nlq
> > +YYoWTNufaIxnvdtPUZaKzvwK49I9Zdc%3D&amp;reserved=0
> > +
> > +title: Freescale INTMUX interrupt multiplexer
> > +
> > +maintainers:
> > + - Marc Zyngier <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + items:
> > + const: fsl,imx-intmux
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + minItems: 1
> > + maxItems: 8
> > + description: |
> > + Should contain the parent interrupt lines (up to 8) used to multiplex
> > + the input interrupts.
> > +
> > + interrupt-controller: true
> > +
> > + '#interrupt-cells':
> > + const: 2
> > +
> > + clocks:
> > + maxItems: 1
> > + description: ipg clock.
> > +
> > + clock-names:
> > + items:
> > + const: ipg
> > +
> > + fsl,intmux_chans:
>
> Don't use '_' in property names.
Got it.

> Is this any different from the length of 'interrupts' which you can count?
A bit different. Such as, the length of 'interrupts' is 8, but we can set fsl,intmux_chans value is 4. That means there are 8 channels, but actually we only use 4 channels.
If you think this make no sense, due to we can assign 4 items for 'interrupts' to get the same result. So we can count the length of 'interrupts' to get the channels configured, then this property is no need.
Which one do you think is better?
interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
fsl,intmux_chans = <4>;

Best Regards,
Joakim Zhang
> > + maxItems: 1
> > + description: |
> > + The number of channels used for interrupt source. The Maximum
> value is 8.
> > + If this property is not set in DT then driver uses 1 channel by default.
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - interrupt-controller
> > + - '#interrupt-cells'
> > + - clocks
> > + - clock-name
> > + - fsl,intmux_chans
> > +
> > +additionalProperties: false
> > +
> > +Example:
> > +
> > + intmux@37400000 {
>
> interrupt-controller@...
>
> > + compatible = "fsl,imx-intmux";
> > + reg = <0x37400000 0x1000>;
> > + interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
> > + interrupt-controller;
> > + #interrupt-cells = <2>;
> > + clocks = <&clk IMX8QM_CM40_IPG_CLK>;
> > + clock-names = "ipg";
> > + fsl,intmux_chans = <8>;
> > + };
> > +
> > --
> > 2.17.1
> >

2020-01-14 08:24:24

by Joakim Zhang

[permalink] [raw]
Subject: RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer


> -----Original Message-----
> From: Joakim Zhang <[email protected]>
> Sent: 2020??1??14?? 10:44
> To: Rob Herring <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>; Andy Duan <[email protected]>
> Subject: RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP
> INTMUX interrupt multiplexer
>
>
> > -----Original Message-----
> > From: Rob Herring <[email protected]>
> > Sent: 2020??1??14?? 5:04
> > To: Joakim Zhang <[email protected]>
> > Cc: [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; dl-linux-imx
> > <[email protected]>; Andy Duan <[email protected]>
> > Subject: Re: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for
> > NXP INTMUX interrupt multiplexer
> >
> > On Mon, Jan 13, 2020 at 03:08:40PM +0800, Joakim Zhang wrote:
> > > This patch adds the DT bindings for the NXP INTMUX interrupt
> > > multiplexer for i.MX8 family SoCs.
> > >
> > > Signed-off-by: Joakim Zhang <[email protected]>
> > > ---
> > > .../interrupt-controller/fsl,intmux.yaml | 77
> +++++++++++++++++++
> > > 1 file changed, 77 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.ya
> > > ml
> >
> > Please run 'make dt_binding_check' and fix the errors:
> >
> > Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml:
> > while scanning for the next token found character that cannot start any token
> > in "<unicode string>", line 60, column 1
> Got it. Will keep in mind. Thanks.
>
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.
> > > ya
> > > ml
> > > b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.
> > > ya
> > > ml
> > > new file mode 100644
> > > index 000000000000..4dba532fe0bd
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,int
> > > +++ mu
> > > +++ x.yaml
> > > @@ -0,0 +1,77 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > > +---
> > > +$id:
> > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fde
> > > +vi
> > >
> >
> +cetree.org%2Fschemas%2Finterrupt-controller%2Ffsl%2Cintmux.yaml%23&a
> > m
> > >
> >
> +p;data=02%7C01%7Cqiangqing.zhang%40nxp.com%7Cdc2443dc111149805c7
> > 208d7
> > >
> >
> +986c157f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63714546
> > 2291934
> > >
> >
> +492&amp;sdata=Ao4iuj2D48KAeC%2FvQvJqUUxGJEjSY0HyL5ZlT2XrSrg%3D&
> > amp;re
> > > +served=0
> > > +$schema:
> > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fde
> > > +vi
> > >
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=02%7C01%7Cqia
> > ngqing
> > >
> >
> +.zhang%40nxp.com%7Cdc2443dc111149805c7208d7986c157f%7C686ea1d3b
> > c2b4c6
> > >
> >
> +fa92cd99c5c301635%7C0%7C0%7C637145462291934492&amp;sdata=YoHb
> > TO5C8Nlq
> > > +YYoWTNufaIxnvdtPUZaKzvwK49I9Zdc%3D&amp;reserved=0
> > > +
> > > +title: Freescale INTMUX interrupt multiplexer
> > > +
> > > +maintainers:
> > > + - Marc Zyngier <[email protected]>
> > > +
> > > +properties:
> > > + compatible:
> > > + items:
> > > + const: fsl,imx-intmux
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + interrupts:
> > > + minItems: 1
> > > + maxItems: 8
> > > + description: |
> > > + Should contain the parent interrupt lines (up to 8) used to multiplex
> > > + the input interrupts.
> > > +
> > > + interrupt-controller: true
> > > +
> > > + '#interrupt-cells':
> > > + const: 2
> > > +
> > > + clocks:
> > > + maxItems: 1
> > > + description: ipg clock.
> > > +
> > > + clock-names:
> > > + items:
> > > + const: ipg
> > > +
> > > + fsl,intmux_chans:
> >
> > Don't use '_' in property names.
> Got it.
>
> > Is this any different from the length of 'interrupts' which you can count?
> A bit different. Such as, the length of 'interrupts' is 8, but we can set
> fsl,intmux_chans value is 4. That means there are 8 channels, but actually we
> only use 4 channels.
> If you think this make no sense, due to we can assign 4 items for 'interrupts' to
> get the same result. So we can count the length of 'interrupts' to get the
> channels configured, then this property is no need.
> Which one do you think is better?
> interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
> fsl,intmux_chans = <4>;

One more add, the number of channels is fixed to 8. It will make more clear to users that it supports 8 channels with 8 items for 'interrupts', and users can decide how many
channels they use with 'fsl,intmux_chans' property.

Best Regards,
Joakim Zhang
> Best Regards,
> Joakim Zhang
> > > + maxItems: 1
> > > + description: |
> > > + The number of channels used for interrupt source. The Maximum
> > value is 8.
> > > + If this property is not set in DT then driver uses 1 channel by
> default.
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - interrupts
> > > + - interrupt-controller
> > > + - '#interrupt-cells'
> > > + - clocks
> > > + - clock-name
> > > + - fsl,intmux_chans
> > > +
> > > +additionalProperties: false
> > > +
> > > +Example:
> > > +
> > > + intmux@37400000 {
> >
> > interrupt-controller@...
> >
> > > + compatible = "fsl,imx-intmux";
> > > + reg = <0x37400000 0x1000>;
> > > + interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
> > > + <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>,
> > > + <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
> > > + <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>,
> > > + <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>,
> > > + <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>,
> > > + <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
> > > + <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
> > > + interrupt-controller;
> > > + #interrupt-cells = <2>;
> > > + clocks = <&clk IMX8QM_CM40_IPG_CLK>;
> > > + clock-names = "ipg";
> > > + fsl,intmux_chans = <8>;
> > > + };
> > > +
> > > --
> > > 2.17.1
> > >

2020-01-14 14:14:13

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer

On Tue, Jan 14, 2020 at 2:22 AM Joakim Zhang <[email protected]> wrote:
>
>
> > -----Original Message-----
> > From: Joakim Zhang <[email protected]>
> > Sent: 2020年1月14日 10:44
> > To: Rob Herring <[email protected]>
> > Cc: [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; dl-linux-imx
> > <[email protected]>; Andy Duan <[email protected]>
> > Subject: RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP
> > INTMUX interrupt multiplexer
> >
> >
> > > -----Original Message-----
> > > From: Rob Herring <[email protected]>
> > > Sent: 2020年1月14日 5:04
> > > To: Joakim Zhang <[email protected]>
> > > Cc: [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; dl-linux-imx
> > > <[email protected]>; Andy Duan <[email protected]>
> > > Subject: Re: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for
> > > NXP INTMUX interrupt multiplexer
> > >
> > > On Mon, Jan 13, 2020 at 03:08:40PM +0800, Joakim Zhang wrote:
> > > > This patch adds the DT bindings for the NXP INTMUX interrupt
> > > > multiplexer for i.MX8 family SoCs.
> > > >
> > > > Signed-off-by: Joakim Zhang <[email protected]>
> > > > ---
> > > > .../interrupt-controller/fsl,intmux.yaml | 77
> > +++++++++++++++++++
> > > > 1 file changed, 77 insertions(+)
> > > > create mode 100644
> > > > Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.ya
> > > > ml
> > >
> > > Please run 'make dt_binding_check' and fix the errors:
> > >
> > > Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml:
> > > while scanning for the next token found character that cannot start any token
> > > in "<unicode string>", line 60, column 1
> > Got it. Will keep in mind. Thanks.
> >
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.
> > > > ya
> > > > ml
> > > > b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.
> > > > ya
> > > > ml
> > > > new file mode 100644
> > > > index 000000000000..4dba532fe0bd
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,int
> > > > +++ mu
> > > > +++ x.yaml
> > > > @@ -0,0 +1,77 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > > > +---
> > > > +$id:
> > > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fde
> > > > +vi
> > > >
> > >
> > +cetree.org%2Fschemas%2Finterrupt-controller%2Ffsl%2Cintmux.yaml%23&a
> > > m
> > > >
> > >
> > +p;data=02%7C01%7Cqiangqing.zhang%40nxp.com%7Cdc2443dc111149805c7
> > > 208d7
> > > >
> > >
> > +986c157f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63714546
> > > 2291934
> > > >
> > >
> > +492&amp;sdata=Ao4iuj2D48KAeC%2FvQvJqUUxGJEjSY0HyL5ZlT2XrSrg%3D&
> > > amp;re
> > > > +served=0
> > > > +$schema:
> > > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fde
> > > > +vi
> > > >
> > >
> > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=02%7C01%7Cqia
> > > ngqing
> > > >
> > >
> > +.zhang%40nxp.com%7Cdc2443dc111149805c7208d7986c157f%7C686ea1d3b
> > > c2b4c6
> > > >
> > >
> > +fa92cd99c5c301635%7C0%7C0%7C637145462291934492&amp;sdata=YoHb
> > > TO5C8Nlq
> > > > +YYoWTNufaIxnvdtPUZaKzvwK49I9Zdc%3D&amp;reserved=0
> > > > +
> > > > +title: Freescale INTMUX interrupt multiplexer
> > > > +
> > > > +maintainers:
> > > > + - Marc Zyngier <[email protected]>
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + items:
> > > > + const: fsl,imx-intmux
> > > > +
> > > > + reg:
> > > > + maxItems: 1
> > > > +
> > > > + interrupts:
> > > > + minItems: 1
> > > > + maxItems: 8
> > > > + description: |
> > > > + Should contain the parent interrupt lines (up to 8) used to multiplex
> > > > + the input interrupts.
> > > > +
> > > > + interrupt-controller: true
> > > > +
> > > > + '#interrupt-cells':
> > > > + const: 2
> > > > +
> > > > + clocks:
> > > > + maxItems: 1
> > > > + description: ipg clock.
> > > > +
> > > > + clock-names:
> > > > + items:
> > > > + const: ipg
> > > > +
> > > > + fsl,intmux_chans:
> > >
> > > Don't use '_' in property names.
> > Got it.
> >
> > > Is this any different from the length of 'interrupts' which you can count?
> > A bit different. Such as, the length of 'interrupts' is 8, but we can set
> > fsl,intmux_chans value is 4. That means there are 8 channels, but actually we
> > only use 4 channels.
> > If you think this make no sense, due to we can assign 4 items for 'interrupts' to
> > get the same result. So we can count the length of 'interrupts' to get the
> > channels configured, then this property is no need.
> > Which one do you think is better?
> > interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
> > <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>,
> > <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
> > <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>,
> > <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>,
> > <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>,
> > <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
> > <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
> > fsl,intmux_chans = <4>;
>
> One more add, the number of channels is fixed to 8. It will make more clear to users that it supports 8 channels with 8 items for 'interrupts', and users can decide how many
> channels they use with 'fsl,intmux_chans' property.

How does one decide how many? Why would you not use as many channels
as possible (other than muxing interrupts or not doesn't really make a
bit difference in servicing overhead)?

If you wanted to configure how many parent interrupts, wouldn't you
also want to configure the routing of child interrupts to specific
parent interrupts?

So I would drop this property. You can define both how many parents
and the routing with interrupt-map property, though I would not do
that until you have a real need.

Rob

2020-01-15 02:30:33

by Joakim Zhang

[permalink] [raw]
Subject: RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer


> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: 2020年1月14日 22:12
> To: Joakim Zhang <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>; Andy Duan <[email protected]>
> Subject: Re: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP
> INTMUX interrupt multiplexer
>
> On Tue, Jan 14, 2020 at 2:22 AM Joakim Zhang <[email protected]>
> wrote:
> >
> >
> > > -----Original Message-----
> > > From: Joakim Zhang <[email protected]>
> > > Sent: 2020年1月14日 10:44
> > > To: Rob Herring <[email protected]>
> > > Cc: [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; dl-linux-imx
> > > <[email protected]>; Andy Duan <[email protected]>
> > > Subject: RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for
> > > NXP INTMUX interrupt multiplexer
> > >
> > >
> > > > -----Original Message-----
> > > > From: Rob Herring <[email protected]>
> > > > Sent: 2020年1月14日 5:04
> > > > To: Joakim Zhang <[email protected]>
> > > > Cc: [email protected]; [email protected]; [email protected];
> > > > [email protected]; [email protected];
> [email protected];
> > > > [email protected]; [email protected];
> > > > [email protected]; [email protected];
> > > > [email protected]; dl-linux-imx
> > > > <[email protected]>; Andy Duan <[email protected]>
> > > > Subject: Re: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding
> > > > for NXP INTMUX interrupt multiplexer
> > > >
> > > > On Mon, Jan 13, 2020 at 03:08:40PM +0800, Joakim Zhang wrote:
> > > > > This patch adds the DT bindings for the NXP INTMUX interrupt
> > > > > multiplexer for i.MX8 family SoCs.
> > > > >
> > > > > Signed-off-by: Joakim Zhang <[email protected]>
> > > > > ---
> > > > > .../interrupt-controller/fsl,intmux.yaml | 77
> > > +++++++++++++++++++
> > > > > 1 file changed, 77 insertions(+) create mode 100644
> > > > > Documentation/devicetree/bindings/interrupt-controller/fsl,intmu
> > > > > x.ya
> > > > > ml
> > > >
> > > > Please run 'make dt_binding_check' and fix the errors:
> > > >
> > > > Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml:
> > > > while scanning for the next token found character that cannot start any
> token
> > > > in "<unicode string>", line 60, column 1
> > > Got it. Will keep in mind. Thanks.
> > >
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.
> > > > > ya
> > > > > ml
> > > > > b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.
> > > > > ya
> > > > > ml
> > > > > new file mode 100644
> > > > > index 000000000000..4dba532fe0bd
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl
> > > > > +++ ,int
> > > > > +++ mu
> > > > > +++ x.yaml
> > > > > @@ -0,0 +1,77 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > > > > +---
> > > > > +$id:
> > > > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%
> > > > > +2Fde
> > > > > +vi
> > > > >
> > > >
> > >
> +cetree.org%2Fschemas%2Finterrupt-controller%2Ffsl%2Cintmux.yaml%23&
> > > +a
> > > > m
> > > > >
> > > >
> > >
> +p;data=02%7C01%7Cqiangqing.zhang%40nxp.com%7Cdc2443dc111149805c7
> > > > 208d7
> > > > >
> > > >
> > >
> +986c157f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63714546
> > > > 2291934
> > > > >
> > > >
> > >
> +492&amp;sdata=Ao4iuj2D48KAeC%2FvQvJqUUxGJEjSY0HyL5ZlT2XrSrg%3D&
> > > > amp;re
> > > > > +served=0
> > > > > +$schema:
> > > > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%
> > > > > +2Fde
> > > > > +vi
> > > > >
> > > >
> > >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=02%7C01%7Cqia
> > > > ngqing
> > > > >
> > > >
> > >
> +.zhang%40nxp.com%7Cdc2443dc111149805c7208d7986c157f%7C686ea1d3b
> > > > c2b4c6
> > > > >
> > > >
> > >
> +fa92cd99c5c301635%7C0%7C0%7C637145462291934492&amp;sdata=YoHb
> > > > TO5C8Nlq
> > > > > +YYoWTNufaIxnvdtPUZaKzvwK49I9Zdc%3D&amp;reserved=0
> > > > > +
> > > > > +title: Freescale INTMUX interrupt multiplexer
> > > > > +
> > > > > +maintainers:
> > > > > + - Marc Zyngier <[email protected]>
> > > > > +
> > > > > +properties:
> > > > > + compatible:
> > > > > + items:
> > > > > + const: fsl,imx-intmux
> > > > > +
> > > > > + reg:
> > > > > + maxItems: 1
> > > > > +
> > > > > + interrupts:
> > > > > + minItems: 1
> > > > > + maxItems: 8
> > > > > + description: |
> > > > > + Should contain the parent interrupt lines (up to 8) used to
> multiplex
> > > > > + the input interrupts.
> > > > > +
> > > > > + interrupt-controller: true
> > > > > +
> > > > > + '#interrupt-cells':
> > > > > + const: 2
> > > > > +
> > > > > + clocks:
> > > > > + maxItems: 1
> > > > > + description: ipg clock.
> > > > > +
> > > > > + clock-names:
> > > > > + items:
> > > > > + const: ipg
> > > > > +
> > > > > + fsl,intmux_chans:
> > > >
> > > > Don't use '_' in property names.
> > > Got it.
> > >
> > > > Is this any different from the length of 'interrupts' which you can count?
> > > A bit different. Such as, the length of 'interrupts' is 8, but we
> > > can set fsl,intmux_chans value is 4. That means there are 8
> > > channels, but actually we only use 4 channels.
> > > If you think this make no sense, due to we can assign 4 items for
> > > 'interrupts' to get the same result. So we can count the length of
> > > 'interrupts' to get the channels configured, then this property is no need.
> > > Which one do you think is better?
> > > interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
> > > <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>,
> > > <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
> > > <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>,
> > > <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>,
> > > <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>,
> > > <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
> > > <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
> > > fsl,intmux_chans = <4>;
> >
> > One more add, the number of channels is fixed to 8. It will make more
> > clear to users that it supports 8 channels with 8 items for 'interrupts', and
> users can decide how many channels they use with 'fsl,intmux_chans' property.
>
> How does one decide how many? Why would you not use as many channels as
> possible (other than muxing interrupts or not doesn't really make a bit
> difference in servicing overhead)?
>
> If you wanted to configure how many parent interrupts, wouldn't you also want
> to configure the routing of child interrupts to specific parent interrupts?
>
> So I would drop this property. You can define both how many parents and the
> routing with interrupt-map property, though I would not do that until you have
> a real need.

Thanks Rob. Agree. I will update both bindings and driver.

Best Regards,
Joakim Zhang
> Rob