linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4 RESEND 0/2] irqchip: add NXP INTMUX interrupt controller
@ 2020-01-13  7:08 Joakim Zhang
  2020-01-13  7:08 ` [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer Joakim Zhang
  2020-01-13  7:08 ` [PATCH V4 RESEND 2/2] drivers/irqchip: add NXP INTMUX interrupt multiplexer support Joakim Zhang
  0 siblings, 2 replies; 10+ messages in thread
From: Joakim Zhang @ 2020-01-13  7:08 UTC (permalink / raw)
  To: maz, jason, tglx, robh+dt, mark.rutland, shawnguo, s.hauer
  Cc: devicetree, fugang.duan, linux-kernel, Joakim Zhang, linux-imx,
	kernel, festevam, linux-arm-kernel

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer
  2020-01-13  7:08 [PATCH V4 RESEND 0/2] irqchip: add NXP INTMUX interrupt controller Joakim Zhang
@ 2020-01-13  7:08 ` Joakim Zhang
  2020-01-13  9:16   ` Marc Zyngier
  2020-01-13 21:03   ` Rob Herring
  2020-01-13  7:08 ` [PATCH V4 RESEND 2/2] drivers/irqchip: add NXP INTMUX interrupt multiplexer support Joakim Zhang
  1 sibling, 2 replies; 10+ messages in thread
From: Joakim Zhang @ 2020-01-13  7:08 UTC (permalink / raw)
  To: maz, jason, tglx, robh+dt, mark.rutland, shawnguo, s.hauer
  Cc: devicetree, fugang.duan, linux-kernel, Joakim Zhang, linux-imx,
	kernel, festevam, linux-arm-kernel

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

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 .../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 <maz@kernel.org>
+
+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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH V4 RESEND 2/2] drivers/irqchip: add NXP INTMUX interrupt multiplexer support
  2020-01-13  7:08 [PATCH V4 RESEND 0/2] irqchip: add NXP INTMUX interrupt controller Joakim Zhang
  2020-01-13  7:08 ` [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer Joakim Zhang
@ 2020-01-13  7:08 ` Joakim Zhang
  1 sibling, 0 replies; 10+ messages in thread
From: Joakim Zhang @ 2020-01-13  7:08 UTC (permalink / raw)
  To: maz, jason, tglx, robh+dt, mark.rutland, shawnguo, s.hauer
  Cc: devicetree, fugang.duan, Shengjiu Wang, linux-kernel,
	Joakim Zhang, linux-imx, kernel, festevam, linux-arm-kernel

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 <shengjiu.wang@nxp.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer
  2020-01-13  7:08 ` [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer Joakim Zhang
@ 2020-01-13  9:16   ` Marc Zyngier
  2020-01-13  9:59     ` Joakim Zhang
  2020-01-13 21:03   ` Rob Herring
  1 sibling, 1 reply; 10+ messages in thread
From: Marc Zyngier @ 2020-01-13  9:16 UTC (permalink / raw)
  To: Joakim Zhang
  Cc: mark.rutland, devicetree, fugang.duan, jason, festevam, s.hauer,
	linux-kernel, robh+dt, linux-imx, kernel, tglx, shawnguo,
	linux-arm-kernel

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 <qiangqing.zhang@nxp.com>
> ---
>  .../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 <maz@kernel.org>

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...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer
  2020-01-13  9:16   ` Marc Zyngier
@ 2020-01-13  9:59     ` Joakim Zhang
  0 siblings, 0 replies; 10+ messages in thread
From: Joakim Zhang @ 2020-01-13  9:59 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: mark.rutland, devicetree, Andy Duan, jason, festevam, s.hauer,
	linux-kernel, robh+dt, dl-linux-imx, kernel, tglx, shawnguo,
	linux-arm-kernel


> -----Original Message-----
> From: Marc Zyngier <maz@kernel.org>
> Sent: 2020年1月13日 17:16
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: jason@lakedaemon.net; tglx@linutronix.de; robh+dt@kernel.org;
> mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; dl-linux-imx
> <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> 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 <qiangqing.zhang@nxp.com>
> > ---
> >  .../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 <maz@kernel.org>
> 
> 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...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer
  2020-01-13  7:08 ` [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer Joakim Zhang
  2020-01-13  9:16   ` Marc Zyngier
@ 2020-01-13 21:03   ` Rob Herring
  2020-01-14  2:43     ` Joakim Zhang
  1 sibling, 1 reply; 10+ messages in thread
From: Rob Herring @ 2020-01-13 21:03 UTC (permalink / raw)
  To: Joakim Zhang
  Cc: mark.rutland, devicetree, festevam, fugang.duan, jason, maz,
	s.hauer, linux-kernel, linux-imx, kernel, tglx, shawnguo,
	linux-arm-kernel

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 <qiangqing.zhang@nxp.com>
> ---
>  .../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 <maz@kernel.org>
> +
> +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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer
  2020-01-13 21:03   ` Rob Herring
@ 2020-01-14  2:43     ` Joakim Zhang
  2020-01-14  8:22       ` Joakim Zhang
  0 siblings, 1 reply; 10+ messages in thread
From: Joakim Zhang @ 2020-01-14  2:43 UTC (permalink / raw)
  To: Rob Herring
  Cc: mark.rutland, devicetree, festevam, Andy Duan, jason, maz,
	s.hauer, linux-kernel, dl-linux-imx, kernel, tglx, shawnguo,
	linux-arm-kernel


> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 2020年1月14日 5:04
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; dl-linux-imx
> <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> 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 <qiangqing.zhang@nxp.com>
> > ---
> >  .../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 <maz@kernel.org>
> > +
> > +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
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer
  2020-01-14  2:43     ` Joakim Zhang
@ 2020-01-14  8:22       ` Joakim Zhang
  2020-01-14 14:11         ` Rob Herring
  0 siblings, 1 reply; 10+ messages in thread
From: Joakim Zhang @ 2020-01-14  8:22 UTC (permalink / raw)
  To: Rob Herring
  Cc: mark.rutland, devicetree, festevam, Andy Duan, jason, maz,
	s.hauer, linux-kernel, dl-linux-imx, kernel, tglx, shawnguo,
	linux-arm-kernel


> -----Original Message-----
> From: Joakim Zhang <qiangqing.zhang@nxp.com>
> Sent: 2020年1月14日 10:44
> To: Rob Herring <robh@kernel.org>
> Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; dl-linux-imx
> <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> Subject: RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP
> INTMUX interrupt multiplexer
> 
> 
> > -----Original Message-----
> > From: Rob Herring <robh@kernel.org>
> > Sent: 2020年1月14日 5:04
> > To: Joakim Zhang <qiangqing.zhang@nxp.com>
> > Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> > mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > kernel@pengutronix.de; festevam@gmail.com;
> > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; dl-linux-imx
> > <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> > 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 <qiangqing.zhang@nxp.com>
> > > ---
> > >  .../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 <maz@kernel.org>
> > > +
> > > +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
> > >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer
  2020-01-14  8:22       ` Joakim Zhang
@ 2020-01-14 14:11         ` Rob Herring
  2020-01-15  2:27           ` Joakim Zhang
  0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2020-01-14 14:11 UTC (permalink / raw)
  To: Joakim Zhang
  Cc: mark.rutland, devicetree, festevam, Andy Duan, jason, maz,
	s.hauer, linux-kernel, dl-linux-imx, kernel, tglx, shawnguo,
	linux-arm-kernel

On Tue, Jan 14, 2020 at 2:22 AM Joakim Zhang <qiangqing.zhang@nxp.com> wrote:
>
>
> > -----Original Message-----
> > From: Joakim Zhang <qiangqing.zhang@nxp.com>
> > Sent: 2020年1月14日 10:44
> > To: Rob Herring <robh@kernel.org>
> > Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> > mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > kernel@pengutronix.de; festevam@gmail.com; linux-kernel@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; dl-linux-imx
> > <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> > Subject: RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP
> > INTMUX interrupt multiplexer
> >
> >
> > > -----Original Message-----
> > > From: Rob Herring <robh@kernel.org>
> > > Sent: 2020年1月14日 5:04
> > > To: Joakim Zhang <qiangqing.zhang@nxp.com>
> > > Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> > > mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > kernel@pengutronix.de; festevam@gmail.com;
> > > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> > > linux-arm-kernel@lists.infradead.org; dl-linux-imx
> > > <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> > > 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 <qiangqing.zhang@nxp.com>
> > > > ---
> > > >  .../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 <maz@kernel.org>
> > > > +
> > > > +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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer
  2020-01-14 14:11         ` Rob Herring
@ 2020-01-15  2:27           ` Joakim Zhang
  0 siblings, 0 replies; 10+ messages in thread
From: Joakim Zhang @ 2020-01-15  2:27 UTC (permalink / raw)
  To: Rob Herring
  Cc: mark.rutland, devicetree, festevam, Andy Duan, jason, maz,
	s.hauer, linux-kernel, dl-linux-imx, kernel, tglx, shawnguo,
	linux-arm-kernel


> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 2020年1月14日 22:12
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; dl-linux-imx
> <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> 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 <qiangqing.zhang@nxp.com>
> wrote:
> >
> >
> > > -----Original Message-----
> > > From: Joakim Zhang <qiangqing.zhang@nxp.com>
> > > Sent: 2020年1月14日 10:44
> > > To: Rob Herring <robh@kernel.org>
> > > Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> > > mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > kernel@pengutronix.de; festevam@gmail.com;
> > > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> > > linux-arm-kernel@lists.infradead.org; dl-linux-imx
> > > <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> > > Subject: RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for
> > > NXP INTMUX interrupt multiplexer
> > >
> > >
> > > > -----Original Message-----
> > > > From: Rob Herring <robh@kernel.org>
> > > > Sent: 2020年1月14日 5:04
> > > > To: Joakim Zhang <qiangqing.zhang@nxp.com>
> > > > Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> > > > mark.rutland@arm.com; shawnguo@kernel.org;
> s.hauer@pengutronix.de;
> > > > kernel@pengutronix.de; festevam@gmail.com;
> > > > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> > > > linux-arm-kernel@lists.infradead.org; dl-linux-imx
> > > > <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> > > > 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 <qiangqing.zhang@nxp.com>
> > > > > ---
> > > > >  .../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 <maz@kernel.org>
> > > > > +
> > > > > +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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-01-15  2:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-13  7:08 [PATCH V4 RESEND 0/2] irqchip: add NXP INTMUX interrupt controller Joakim Zhang
2020-01-13  7:08 ` [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer Joakim Zhang
2020-01-13  9:16   ` Marc Zyngier
2020-01-13  9:59     ` Joakim Zhang
2020-01-13 21:03   ` Rob Herring
2020-01-14  2:43     ` Joakim Zhang
2020-01-14  8:22       ` Joakim Zhang
2020-01-14 14:11         ` Rob Herring
2020-01-15  2:27           ` Joakim Zhang
2020-01-13  7:08 ` [PATCH V4 RESEND 2/2] drivers/irqchip: add NXP INTMUX interrupt multiplexer support Joakim Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).