linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] irqchip/ti-sci-intr: Add support for INTR connecting to INTR
@ 2019-09-23  4:24 Lokesh Vutla
  2019-09-23  4:24 ` [RFC PATCH 1/2] dt-bindings: irqchip: Update bindings to drop the usage of gic as parent Lokesh Vutla
  2019-09-23  4:24 ` [RFC PATCH 2/2] irqchip/ti-sci-intr: Add support for INTR being a parent to INTR Lokesh Vutla
  0 siblings, 2 replies; 12+ messages in thread
From: Lokesh Vutla @ 2019-09-23  4:24 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Nishanth Menon, Lokesh Vutla, Linux ARM Mailing List

This series updates the driver and dt-bindings to accept INTR being a
parent to INTR and drop all the firmware related bindings.

Lokesh Vutla (2):
  dt-bindings: irqchip: Update bindings to drop the usage of gic as
    parent
  irqchip/ti-sci-intr: Add support for INTR being a parent to INTR

 .../interrupt-controller/ti,sci-intr.txt      |  28 ++--
 drivers/irqchip/irq-ti-sci-intr.c             | 137 +++++++++++-------
 2 files changed, 95 insertions(+), 70 deletions(-)

-- 
2.22.0


_______________________________________________
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] 12+ messages in thread

* [RFC PATCH 1/2] dt-bindings: irqchip: Update bindings to drop the usage of gic as parent
  2019-09-23  4:24 [RFC PATCH 0/2] irqchip/ti-sci-intr: Add support for INTR connecting to INTR Lokesh Vutla
@ 2019-09-23  4:24 ` Lokesh Vutla
  2019-10-02 12:57   ` Marc Zyngier
  2019-10-25 16:07   ` Peter Ujfalusi
  2019-09-23  4:24 ` [RFC PATCH 2/2] irqchip/ti-sci-intr: Add support for INTR being a parent to INTR Lokesh Vutla
  1 sibling, 2 replies; 12+ messages in thread
From: Lokesh Vutla @ 2019-09-23  4:24 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Nishanth Menon, Lokesh Vutla, Linux ARM Mailing List

Drop the firmware related dt-bindings and use the hardware specified
interrupt numbers within Interrupt Router. This ensures interrupt router
DT node need not assume any interrupt parent type.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 .../interrupt-controller/ti,sci-intr.txt      | 28 +++++++++----------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
index 1a8718f8855d..de5de2a4b467 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
@@ -44,15 +44,14 @@ Required Properties:
 			4: If intr supports level triggered interrupts.
 - interrupt-controller:	Identifies the node as an interrupt controller
 - #interrupt-cells:	Specifies the number of cells needed to encode an
-			interrupt source. The value should be 2.
-			First cell should contain the TISCI device ID of source
-			Second cell should contain the interrupt source offset
-			within the device.
+			interrupt source. The value should be 1.
+			First cell should contain interrupt router input number
+			as specified by hardware.
+- power-domains:	phandle pointing to the corresponding PM domain node
+			and an ID representing the device.
 - ti,sci:		Phandle to TI-SCI compatible System controller node.
-- ti,sci-dst-id:	TISCI device ID of the destination IRQ controller.
-- ti,sci-rm-range-girq:	Array of TISCI subtype ids representing the host irqs
-			assigned to this interrupt router. Each subtype id
-			corresponds to a range of host irqs.
+- interrupt-ranges:	Ranges that convert the INTR output interrupt numbers to
+			parent's interrupt number.
 
 For more details on TISCI IRQ resource management refer:
 http://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/rm/rm_irq.html
@@ -62,21 +61,20 @@ Example:
 The following example demonstrates both interrupt router node and the consumer
 node(main gpio) on the AM654 SoC:
 
-main_intr: interrupt-controller0 {
+main_gpio_intr: interrupt-controller0 {
 	compatible = "ti,sci-intr";
 	ti,intr-trigger-type = <1>;
 	interrupt-controller;
 	interrupt-parent = <&gic500>;
-	#interrupt-cells = <2>;
+	power-domains = <&k3_pds 131 TI_SCI_PD_SHARED>;
+	#interrupt-cells = <1>;
 	ti,sci = <&dmsc>;
-	ti,sci-dst-id = <56>;
-	ti,sci-rm-range-girq = <0x1>;
+	interrupt-ranges = <0 360 32>;
 };
 
 main_gpio0: gpio@600000 {
 	...
-	interrupt-parent = <&main_intr>;
-	interrupts = <57 256>, <57 257>, <57 258>,
-		     <57 259>, <57 260>, <57 261>;
+	interrupt-parent = <&main_gpio_intr>;
+	interrupts = <192>, <193>, <194>, <195>, <196>, <197>;
 	...
 };
-- 
2.22.0


_______________________________________________
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] 12+ messages in thread

* [RFC PATCH 2/2] irqchip/ti-sci-intr: Add support for INTR being a parent to INTR
  2019-09-23  4:24 [RFC PATCH 0/2] irqchip/ti-sci-intr: Add support for INTR connecting to INTR Lokesh Vutla
  2019-09-23  4:24 ` [RFC PATCH 1/2] dt-bindings: irqchip: Update bindings to drop the usage of gic as parent Lokesh Vutla
@ 2019-09-23  4:24 ` Lokesh Vutla
  2019-10-02 13:12   ` Marc Zyngier
  1 sibling, 1 reply; 12+ messages in thread
From: Lokesh Vutla @ 2019-09-23  4:24 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Nishanth Menon, Lokesh Vutla, Linux ARM Mailing List

Driver assumes that Interrupt parent to Interrupt router is always GIC.
This is not true always and a Interrupt Router can be a parent to
Interrupt Router. Update the driver to detect the parent and request the
parent irqs accordingly.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 drivers/irqchip/irq-ti-sci-intr.c | 137 ++++++++++++++++++------------
 1 file changed, 82 insertions(+), 55 deletions(-)

diff --git a/drivers/irqchip/irq-ti-sci-intr.c b/drivers/irqchip/irq-ti-sci-intr.c
index 59d51a20bbd8..1d8aaa871798 100644
--- a/drivers/irqchip/irq-ti-sci-intr.c
+++ b/drivers/irqchip/irq-ti-sci-intr.c
@@ -17,29 +17,18 @@
 #include <linux/of_irq.h>
 #include <linux/soc/ti/ti_sci_protocol.h>
 
-#define TI_SCI_DEV_ID_MASK	0xffff
-#define TI_SCI_DEV_ID_SHIFT	16
-#define TI_SCI_IRQ_ID_MASK	0xffff
-#define TI_SCI_IRQ_ID_SHIFT	0
-#define HWIRQ_TO_DEVID(hwirq)	(((hwirq) >> (TI_SCI_DEV_ID_SHIFT)) & \
-				 (TI_SCI_DEV_ID_MASK))
-#define HWIRQ_TO_IRQID(hwirq)	((hwirq) & (TI_SCI_IRQ_ID_MASK))
-#define TO_HWIRQ(dev, index)	((((dev) & TI_SCI_DEV_ID_MASK) << \
-				 TI_SCI_DEV_ID_SHIFT) | \
-				((index) & TI_SCI_IRQ_ID_MASK))
-
 /**
  * struct ti_sci_intr_irq_domain - Structure representing a TISCI based
  *				   Interrupt Router IRQ domain.
  * @sci:	Pointer to TISCI handle
- * @dst_irq:	TISCI resource pointer representing GIC irq controller.
- * @dst_id:	TISCI device ID of the GIC irq controller.
+ * @out_irqs:	TISCI resource pointer representing INTR irqs.
+ * @pdev:	Pointer to platform device.
  * @type:	Specifies the trigger type supported by this Interrupt Router
  */
 struct ti_sci_intr_irq_domain {
 	const struct ti_sci_handle *sci;
-	struct ti_sci_resource *dst_irq;
-	u32 dst_id;
+	struct ti_sci_resource *out_irqs;
+	struct platform_device *pdev;
 	u32 type;
 };
 
@@ -70,15 +59,43 @@ static int ti_sci_intr_irq_domain_translate(struct irq_domain *domain,
 {
 	struct ti_sci_intr_irq_domain *intr = domain->host_data;
 
-	if (fwspec->param_count != 2)
+	if (fwspec->param_count != 1)
 		return -EINVAL;
 
-	*hwirq = TO_HWIRQ(fwspec->param[0], fwspec->param[1]);
+	*hwirq = fwspec->param[0];
 	*type = intr->type;
 
 	return 0;
 }
 
+static int ti_sci_intr_xlate_irq(struct ti_sci_intr_irq_domain *intr,
+				 u32 irq, bool to_parent_hwirq)
+{
+	struct device_node *np = dev_of_node(&intr->pdev->dev);
+	u32 base, pbase, size, len;
+	const __be32 *range;
+
+	range = of_get_property(np, "interrupt-ranges", &len);
+	if (!range)
+		return irq;
+
+	for (len /= sizeof(*range); len >= 3; len -= 3) {
+		base = be32_to_cpu(*range++);
+		pbase = be32_to_cpu(*range++);
+		size = be32_to_cpu(*range++);
+
+		if (to_parent_hwirq) {
+			if (base <= irq && irq < base + size)
+				return irq - base + pbase;
+		} else {
+			if (pbase <= irq && irq < pbase + size)
+				return irq - pbase + base;
+		}
+	}
+
+	return -ENOENT;
+}
+
 /**
  * ti_sci_intr_irq_domain_free() - Free the specified IRQs from the domain.
  * @domain:	Domain to which the irqs belong
@@ -90,56 +107,72 @@ static void ti_sci_intr_irq_domain_free(struct irq_domain *domain,
 {
 	struct ti_sci_intr_irq_domain *intr = domain->host_data;
 	struct irq_data *data, *parent_data;
-	u16 dev_id, irq_index;
+	int out_irq;
 
 	parent_data = irq_domain_get_irq_data(domain->parent, virq);
+	out_irq = ti_sci_intr_xlate_irq(intr, parent_data->hwirq, false);
 	data = irq_domain_get_irq_data(domain, virq);
-	irq_index = HWIRQ_TO_IRQID(data->hwirq);
-	dev_id = HWIRQ_TO_DEVID(data->hwirq);
 
-	intr->sci->ops.rm_irq_ops.free_irq(intr->sci, dev_id, irq_index,
-					   intr->dst_id, parent_data->hwirq);
-	ti_sci_release_resource(intr->dst_irq, parent_data->hwirq);
+	intr->sci->ops.rm_irq_ops.free_irq(intr->sci,
+					   intr->pdev->id, data->hwirq,
+					   intr->pdev->id, out_irq);
+	ti_sci_release_resource(intr->out_irqs, out_irq);
 	irq_domain_free_irqs_parent(domain, virq, 1);
 	irq_domain_reset_irq_data(data);
 }
 
 /**
- * ti_sci_intr_alloc_gic_irq() - Allocate GIC specific IRQ
+ * ti_sci_intr_alloc_parent_irq() - Allocate parent IRQ
  * @domain:	Pointer to the interrupt router IRQ domain
  * @virq:	Corresponding Linux virtual IRQ number
  * @hwirq:	Corresponding hwirq for the IRQ within this IRQ domain
  *
  * Returns 0 if all went well else appropriate error pointer.
  */
-static int ti_sci_intr_alloc_gic_irq(struct irq_domain *domain,
-				     unsigned int virq, u32 hwirq)
+static int ti_sci_intr_alloc_parent_irq(struct irq_domain *domain,
+					unsigned int virq, u32 hwirq)
 {
 	struct ti_sci_intr_irq_domain *intr = domain->host_data;
+	struct device_node *parent_node;
 	struct irq_fwspec fwspec;
-	u16 dev_id, irq_index;
-	u16 dst_irq;
-	int err;
+	u16 out_irq, p_hwirq;
+	int err = 0;
 
-	dev_id = HWIRQ_TO_DEVID(hwirq);
-	irq_index = HWIRQ_TO_IRQID(hwirq);
-
-	dst_irq = ti_sci_get_free_resource(intr->dst_irq);
-	if (dst_irq == TI_SCI_RESOURCE_NULL)
+	out_irq = ti_sci_get_free_resource(intr->out_irqs);
+	if (out_irq == TI_SCI_RESOURCE_NULL)
 		return -EINVAL;
 
-	fwspec.fwnode = domain->parent->fwnode;
-	fwspec.param_count = 3;
-	fwspec.param[0] = 0;	/* SPI */
-	fwspec.param[1] = dst_irq - 32; /* SPI offset */
-	fwspec.param[2] = intr->type;
+	p_hwirq = ti_sci_intr_xlate_irq(intr, out_irq, true);
+	if (p_hwirq < 0)
+		goto err_irqs;
+
+	parent_node = of_irq_find_parent(dev_of_node(&intr->pdev->dev));
+	fwspec.fwnode = of_node_to_fwnode(parent_node);
+	if (of_property_read_u32(parent_node, "#interrupt-cells",
+				 &fwspec.param_count))
+		goto err_irqs;
+
+	if (fwspec.param_count == 1) {
+		/* Parent is Interrupt Router */
+		fwspec.param[0] = p_hwirq;
+	} else if (fwspec.param_count == 3) {
+		/* Parent is GIC */
+		fwspec.param[0] = 0;	/* SPI */
+		fwspec.param[1] = p_hwirq; /* SPI offset */
+		fwspec.param[2] = intr->type;
+	} else {
+		/* Invalid parent */
+		dev_err(&intr->pdev->dev, "Unknown IRQ parent to INTR\n");
+		goto err_irqs;
+	}
 
 	err = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
 	if (err)
 		goto err_irqs;
 
-	err = intr->sci->ops.rm_irq_ops.set_irq(intr->sci, dev_id, irq_index,
-						intr->dst_id, dst_irq);
+	err = intr->sci->ops.rm_irq_ops.set_irq(intr->sci,
+						intr->pdev->id, hwirq,
+						intr->pdev->id, out_irq);
 	if (err)
 		goto err_msg;
 
@@ -148,7 +181,7 @@ static int ti_sci_intr_alloc_gic_irq(struct irq_domain *domain,
 err_msg:
 	irq_domain_free_irqs_parent(domain, virq, 1);
 err_irqs:
-	ti_sci_release_resource(intr->dst_irq, dst_irq);
+	ti_sci_release_resource(intr->out_irqs, out_irq);
 	return err;
 }
 
@@ -174,7 +207,7 @@ static int ti_sci_intr_irq_domain_alloc(struct irq_domain *domain,
 	if (err)
 		return err;
 
-	err = ti_sci_intr_alloc_gic_irq(domain, virq, hwirq);
+	err = ti_sci_intr_alloc_parent_irq(domain, virq, hwirq);
 	if (err)
 		return err;
 
@@ -214,6 +247,7 @@ static int ti_sci_intr_irq_domain_probe(struct platform_device *pdev)
 	if (!intr)
 		return -ENOMEM;
 
+	intr->pdev = pdev;
 	ret = of_property_read_u32(dev_of_node(dev), "ti,intr-trigger-type",
 				   &intr->type);
 	if (ret) {
@@ -230,19 +264,10 @@ static int ti_sci_intr_irq_domain_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = of_property_read_u32(dev_of_node(dev), "ti,sci-dst-id",
-				   &intr->dst_id);
-	if (ret) {
-		dev_err(dev, "missing 'ti,sci-dst-id' property\n");
-		return -EINVAL;
-	}
-
-	intr->dst_irq = devm_ti_sci_get_of_resource(intr->sci, dev,
-						    intr->dst_id,
-						    "ti,sci-rm-range-girq");
-	if (IS_ERR(intr->dst_irq)) {
+	intr->out_irqs = devm_ti_sci_get_irq_range(intr->sci, dev);
+	if (IS_ERR(intr->out_irqs)) {
 		dev_err(dev, "Destination irq resource allocation failed\n");
-		return PTR_ERR(intr->dst_irq);
+		return PTR_ERR(intr->out_irqs);
 	}
 
 	domain = irq_domain_add_hierarchy(parent_domain, 0, 0, dev_of_node(dev),
@@ -252,6 +277,8 @@ static int ti_sci_intr_irq_domain_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	dev_info(dev, "Interrupt Router %d domain created\n", pdev->id);
+
 	return 0;
 }
 
-- 
2.22.0


_______________________________________________
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] 12+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: irqchip: Update bindings to drop the usage of gic as parent
  2019-09-23  4:24 ` [RFC PATCH 1/2] dt-bindings: irqchip: Update bindings to drop the usage of gic as parent Lokesh Vutla
@ 2019-10-02 12:57   ` Marc Zyngier
  2019-10-03 13:15     ` Lokesh Vutla
  2019-10-25 16:07   ` Peter Ujfalusi
  1 sibling, 1 reply; 12+ messages in thread
From: Marc Zyngier @ 2019-10-02 12:57 UTC (permalink / raw)
  To: Lokesh Vutla; +Cc: Nishanth Menon, Linux ARM Mailing List

On Mon, 23 Sep 2019 09:54:04 +0530
Lokesh Vutla <lokeshvutla@ti.com> wrote:

> Drop the firmware related dt-bindings and use the hardware specified
> interrupt numbers within Interrupt Router. This ensures interrupt router
> DT node need not assume any interrupt parent type.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  .../interrupt-controller/ti,sci-intr.txt      | 28 +++++++++----------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> index 1a8718f8855d..de5de2a4b467 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> @@ -44,15 +44,14 @@ Required Properties:
>  			4: If intr supports level triggered interrupts.
>  - interrupt-controller:	Identifies the node as an interrupt controller
>  - #interrupt-cells:	Specifies the number of cells needed to encode an
> -			interrupt source. The value should be 2.
> -			First cell should contain the TISCI device ID of source
> -			Second cell should contain the interrupt source offset
> -			within the device.
> +			interrupt source. The value should be 1.
> +			First cell should contain interrupt router input number
> +			as specified by hardware.

This is breaking compatibility with existing kernels. Why isn't that a
problem?

> +- power-domains:	phandle pointing to the corresponding PM domain node
> +			and an ID representing the device.

Why is this power-domain thing part of this patch? Is it related?

>  - ti,sci:		Phandle to TI-SCI compatible System controller node.
> -- ti,sci-dst-id:	TISCI device ID of the destination IRQ controller.
> -- ti,sci-rm-range-girq:	Array of TISCI subtype ids representing the host irqs
> -			assigned to this interrupt router. Each subtype id
> -			corresponds to a range of host irqs.
> +- interrupt-ranges:	Ranges that convert the INTR output interrupt numbers to
> +			parent's interrupt number.

This isn't a standard property, is it? If it isn't, it should be
documented a bit more than just that.

>  
>  For more details on TISCI IRQ resource management refer:
>  http://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/rm/rm_irq.html
> @@ -62,21 +61,20 @@ Example:
>  The following example demonstrates both interrupt router node and the consumer
>  node(main gpio) on the AM654 SoC:
>  
> -main_intr: interrupt-controller0 {
> +main_gpio_intr: interrupt-controller0 {
>  	compatible = "ti,sci-intr";
>  	ti,intr-trigger-type = <1>;
>  	interrupt-controller;
>  	interrupt-parent = <&gic500>;
> -	#interrupt-cells = <2>;
> +	power-domains = <&k3_pds 131 TI_SCI_PD_SHARED>;
> +	#interrupt-cells = <1>;
>  	ti,sci = <&dmsc>;
> -	ti,sci-dst-id = <56>;
> -	ti,sci-rm-range-girq = <0x1>;
> +	interrupt-ranges = <0 360 32>;
>  };
>  
>  main_gpio0: gpio@600000 {
>  	...
> -	interrupt-parent = <&main_intr>;
> -	interrupts = <57 256>, <57 257>, <57 258>,
> -		     <57 259>, <57 260>, <57 261>;
> +	interrupt-parent = <&main_gpio_intr>;
> +	interrupts = <192>, <193>, <194>, <195>, <196>, <197>;
>  	...
>  };

Thanks,

	M.
-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
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] 12+ messages in thread

* Re: [RFC PATCH 2/2] irqchip/ti-sci-intr: Add support for INTR being a parent to INTR
  2019-09-23  4:24 ` [RFC PATCH 2/2] irqchip/ti-sci-intr: Add support for INTR being a parent to INTR Lokesh Vutla
@ 2019-10-02 13:12   ` Marc Zyngier
  2019-10-04  3:37     ` Lokesh Vutla
  0 siblings, 1 reply; 12+ messages in thread
From: Marc Zyngier @ 2019-10-02 13:12 UTC (permalink / raw)
  To: Lokesh Vutla; +Cc: Nishanth Menon, Linux ARM Mailing List

On Mon, 23 Sep 2019 09:54:05 +0530
Lokesh Vutla <lokeshvutla@ti.com> wrote:

> Driver assumes that Interrupt parent to Interrupt router is always GIC.
> This is not true always and a Interrupt Router can be a parent to
> Interrupt Router. Update the driver to detect the parent and request the
> parent irqs accordingly.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  drivers/irqchip/irq-ti-sci-intr.c | 137 ++++++++++++++++++------------
>  1 file changed, 82 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-ti-sci-intr.c b/drivers/irqchip/irq-ti-sci-intr.c
> index 59d51a20bbd8..1d8aaa871798 100644
> --- a/drivers/irqchip/irq-ti-sci-intr.c
> +++ b/drivers/irqchip/irq-ti-sci-intr.c
> @@ -17,29 +17,18 @@
>  #include <linux/of_irq.h>
>  #include <linux/soc/ti/ti_sci_protocol.h>
>  
> -#define TI_SCI_DEV_ID_MASK	0xffff
> -#define TI_SCI_DEV_ID_SHIFT	16
> -#define TI_SCI_IRQ_ID_MASK	0xffff
> -#define TI_SCI_IRQ_ID_SHIFT	0
> -#define HWIRQ_TO_DEVID(hwirq)	(((hwirq) >> (TI_SCI_DEV_ID_SHIFT)) & \
> -				 (TI_SCI_DEV_ID_MASK))
> -#define HWIRQ_TO_IRQID(hwirq)	((hwirq) & (TI_SCI_IRQ_ID_MASK))
> -#define TO_HWIRQ(dev, index)	((((dev) & TI_SCI_DEV_ID_MASK) << \
> -				 TI_SCI_DEV_ID_SHIFT) | \
> -				((index) & TI_SCI_IRQ_ID_MASK))
> -
>  /**
>   * struct ti_sci_intr_irq_domain - Structure representing a TISCI based
>   *				   Interrupt Router IRQ domain.
>   * @sci:	Pointer to TISCI handle
> - * @dst_irq:	TISCI resource pointer representing GIC irq controller.
> - * @dst_id:	TISCI device ID of the GIC irq controller.
> + * @out_irqs:	TISCI resource pointer representing INTR irqs.
> + * @pdev:	Pointer to platform device.
>   * @type:	Specifies the trigger type supported by this Interrupt Router
>   */
>  struct ti_sci_intr_irq_domain {
>  	const struct ti_sci_handle *sci;
> -	struct ti_sci_resource *dst_irq;
> -	u32 dst_id;
> +	struct ti_sci_resource *out_irqs;
> +	struct platform_device *pdev;
>  	u32 type;
>  };
>  
> @@ -70,15 +59,43 @@ static int ti_sci_intr_irq_domain_translate(struct irq_domain *domain,
>  {
>  	struct ti_sci_intr_irq_domain *intr = domain->host_data;
>  
> -	if (fwspec->param_count != 2)
> +	if (fwspec->param_count != 1)
>  		return -EINVAL;

As I said in reply to the first patch, this breaks badly. Except that
this time this is a new kernel on an old DT.

>  
> -	*hwirq = TO_HWIRQ(fwspec->param[0], fwspec->param[1]);
> +	*hwirq = fwspec->param[0];
>  	*type = intr->type;
>  
>  	return 0;
>  }
>  
> +static int ti_sci_intr_xlate_irq(struct ti_sci_intr_irq_domain *intr,
> +				 u32 irq, bool to_parent_hwirq)
> +{
> +	struct device_node *np = dev_of_node(&intr->pdev->dev);
> +	u32 base, pbase, size, len;
> +	const __be32 *range;
> +
> +	range = of_get_property(np, "interrupt-ranges", &len);
> +	if (!range)
> +		return irq;
> +
> +	for (len /= sizeof(*range); len >= 3; len -= 3) {
> +		base = be32_to_cpu(*range++);
> +		pbase = be32_to_cpu(*range++);
> +		size = be32_to_cpu(*range++);
> +
> +		if (to_parent_hwirq) {
> +			if (base <= irq && irq < base + size)
> +				return irq - base + pbase;
> +		} else {
> +			if (pbase <= irq && irq < pbase + size)
> +				return irq - pbase + base;
> +		}

Using the same function to compute two different things is really ugly.
It'd be nicer if you had xlate_hwirq_to_parent() and
xlate_parent_to_hwirq(), each one doing their job.

Also, it isn't clear to me why there is only a single tuple in the
interrupt-ranges property. Is it a guarantee that you'll never have
multiple ranges?

> +	}
> +
> +	return -ENOENT;
> +}
> +
>  /**
>   * ti_sci_intr_irq_domain_free() - Free the specified IRQs from the domain.
>   * @domain:	Domain to which the irqs belong
> @@ -90,56 +107,72 @@ static void ti_sci_intr_irq_domain_free(struct irq_domain *domain,
>  {
>  	struct ti_sci_intr_irq_domain *intr = domain->host_data;
>  	struct irq_data *data, *parent_data;
> -	u16 dev_id, irq_index;
> +	int out_irq;
>  
>  	parent_data = irq_domain_get_irq_data(domain->parent, virq);
> +	out_irq = ti_sci_intr_xlate_irq(intr, parent_data->hwirq, false);
>  	data = irq_domain_get_irq_data(domain, virq);
> -	irq_index = HWIRQ_TO_IRQID(data->hwirq);
> -	dev_id = HWIRQ_TO_DEVID(data->hwirq);
>  
> -	intr->sci->ops.rm_irq_ops.free_irq(intr->sci, dev_id, irq_index,
> -					   intr->dst_id, parent_data->hwirq);
> -	ti_sci_release_resource(intr->dst_irq, parent_data->hwirq);
> +	intr->sci->ops.rm_irq_ops.free_irq(intr->sci,
> +					   intr->pdev->id, data->hwirq,
> +					   intr->pdev->id, out_irq);
> +	ti_sci_release_resource(intr->out_irqs, out_irq);
>  	irq_domain_free_irqs_parent(domain, virq, 1);
>  	irq_domain_reset_irq_data(data);
>  }
>  
>  /**
> - * ti_sci_intr_alloc_gic_irq() - Allocate GIC specific IRQ
> + * ti_sci_intr_alloc_parent_irq() - Allocate parent IRQ
>   * @domain:	Pointer to the interrupt router IRQ domain
>   * @virq:	Corresponding Linux virtual IRQ number
>   * @hwirq:	Corresponding hwirq for the IRQ within this IRQ domain
>   *
>   * Returns 0 if all went well else appropriate error pointer.
>   */
> -static int ti_sci_intr_alloc_gic_irq(struct irq_domain *domain,
> -				     unsigned int virq, u32 hwirq)
> +static int ti_sci_intr_alloc_parent_irq(struct irq_domain *domain,
> +					unsigned int virq, u32 hwirq)
>  {
>  	struct ti_sci_intr_irq_domain *intr = domain->host_data;
> +	struct device_node *parent_node;
>  	struct irq_fwspec fwspec;
> -	u16 dev_id, irq_index;
> -	u16 dst_irq;
> -	int err;
> +	u16 out_irq, p_hwirq;
> +	int err = 0;
>  
> -	dev_id = HWIRQ_TO_DEVID(hwirq);
> -	irq_index = HWIRQ_TO_IRQID(hwirq);
> -
> -	dst_irq = ti_sci_get_free_resource(intr->dst_irq);
> -	if (dst_irq == TI_SCI_RESOURCE_NULL)
> +	out_irq = ti_sci_get_free_resource(intr->out_irqs);
> +	if (out_irq == TI_SCI_RESOURCE_NULL)
>  		return -EINVAL;
>  
> -	fwspec.fwnode = domain->parent->fwnode;
> -	fwspec.param_count = 3;
> -	fwspec.param[0] = 0;	/* SPI */
> -	fwspec.param[1] = dst_irq - 32; /* SPI offset */
> -	fwspec.param[2] = intr->type;
> +	p_hwirq = ti_sci_intr_xlate_irq(intr, out_irq, true);
> +	if (p_hwirq < 0)
> +		goto err_irqs;
> +
> +	parent_node = of_irq_find_parent(dev_of_node(&intr->pdev->dev));
> +	fwspec.fwnode = of_node_to_fwnode(parent_node);
> +	if (of_property_read_u32(parent_node, "#interrupt-cells",
> +				 &fwspec.param_count))
> +		goto err_irqs;
> +
> +	if (fwspec.param_count == 1) {
> +		/* Parent is Interrupt Router */
> +		fwspec.param[0] = p_hwirq;
> +	} else if (fwspec.param_count == 3) {
> +		/* Parent is GIC */
> +		fwspec.param[0] = 0;	/* SPI */
> +		fwspec.param[1] = p_hwirq; /* SPI offset */
> +		fwspec.param[2] = intr->type;
> +	} else {

It really looks like you're flipping a coin and hope to hit the right
backend. It feels terribly fragile, and it would be a lot simpler if
the firmware actually exposed the "type" of parent controller, rather
than this guesswork.

> +		/* Invalid parent */
> +		dev_err(&intr->pdev->dev, "Unknown IRQ parent to INTR\n");
> +		goto err_irqs;
> +	}
>  
>  	err = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
>  	if (err)
>  		goto err_irqs;
>  
> -	err = intr->sci->ops.rm_irq_ops.set_irq(intr->sci, dev_id, irq_index,
> -						intr->dst_id, dst_irq);
> +	err = intr->sci->ops.rm_irq_ops.set_irq(intr->sci,
> +						intr->pdev->id, hwirq,
> +						intr->pdev->id, out_irq);
>  	if (err)
>  		goto err_msg;
>  
> @@ -148,7 +181,7 @@ static int ti_sci_intr_alloc_gic_irq(struct irq_domain *domain,
>  err_msg:
>  	irq_domain_free_irqs_parent(domain, virq, 1);
>  err_irqs:
> -	ti_sci_release_resource(intr->dst_irq, dst_irq);
> +	ti_sci_release_resource(intr->out_irqs, out_irq);
>  	return err;
>  }
>  
> @@ -174,7 +207,7 @@ static int ti_sci_intr_irq_domain_alloc(struct irq_domain *domain,
>  	if (err)
>  		return err;
>  
> -	err = ti_sci_intr_alloc_gic_irq(domain, virq, hwirq);
> +	err = ti_sci_intr_alloc_parent_irq(domain, virq, hwirq);
>  	if (err)
>  		return err;
>  
> @@ -214,6 +247,7 @@ static int ti_sci_intr_irq_domain_probe(struct platform_device *pdev)
>  	if (!intr)
>  		return -ENOMEM;
>  
> +	intr->pdev = pdev;
>  	ret = of_property_read_u32(dev_of_node(dev), "ti,intr-trigger-type",
>  				   &intr->type);
>  	if (ret) {
> @@ -230,19 +264,10 @@ static int ti_sci_intr_irq_domain_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	ret = of_property_read_u32(dev_of_node(dev), "ti,sci-dst-id",
> -				   &intr->dst_id);
> -	if (ret) {
> -		dev_err(dev, "missing 'ti,sci-dst-id' property\n");
> -		return -EINVAL;
> -	}
> -
> -	intr->dst_irq = devm_ti_sci_get_of_resource(intr->sci, dev,
> -						    intr->dst_id,
> -						    "ti,sci-rm-range-girq");
> -	if (IS_ERR(intr->dst_irq)) {
> +	intr->out_irqs = devm_ti_sci_get_irq_range(intr->sci, dev);
> +	if (IS_ERR(intr->out_irqs)) {
>  		dev_err(dev, "Destination irq resource allocation failed\n");
> -		return PTR_ERR(intr->dst_irq);
> +		return PTR_ERR(intr->out_irqs);
>  	}
>  
>  	domain = irq_domain_add_hierarchy(parent_domain, 0, 0, dev_of_node(dev),
> @@ -252,6 +277,8 @@ static int ti_sci_intr_irq_domain_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>  
> +	dev_info(dev, "Interrupt Router %d domain created\n", pdev->id);
> +
>  	return 0;
>  }
>  

Thanks,

	M.
-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
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] 12+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: irqchip: Update bindings to drop the usage of gic as parent
  2019-10-02 12:57   ` Marc Zyngier
@ 2019-10-03 13:15     ` Lokesh Vutla
  2019-10-09  5:00       ` Lokesh Vutla
  0 siblings, 1 reply; 12+ messages in thread
From: Lokesh Vutla @ 2019-10-03 13:15 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Nishanth Menon, Linux ARM Mailing List

Hi Marc,

On 02/10/19 6:27 PM, Marc Zyngier wrote:
> On Mon, 23 Sep 2019 09:54:04 +0530
> Lokesh Vutla <lokeshvutla@ti.com> wrote:
> 
>> Drop the firmware related dt-bindings and use the hardware specified
>> interrupt numbers within Interrupt Router. This ensures interrupt router
>> DT node need not assume any interrupt parent type.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>>  .../interrupt-controller/ti,sci-intr.txt      | 28 +++++++++----------
>>  1 file changed, 13 insertions(+), 15 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>> index 1a8718f8855d..de5de2a4b467 100644
>> --- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>> @@ -44,15 +44,14 @@ Required Properties:
>>  			4: If intr supports level triggered interrupts.
>>  - interrupt-controller:	Identifies the node as an interrupt controller
>>  - #interrupt-cells:	Specifies the number of cells needed to encode an
>> -			interrupt source. The value should be 2.
>> -			First cell should contain the TISCI device ID of source
>> -			Second cell should contain the interrupt source offset
>> -			within the device.
>> +			interrupt source. The value should be 1.
>> +			First cell should contain interrupt router input number
>> +			as specified by hardware.
> 
> This is breaking compatibility with existing kernels. Why isn't that a
> problem?

Yes, I am not denying that backward compatibility is broken. But I feel this is
a good cleanup for representing INTR interrupts. With this child nodes will pass
the INTR specific number rather the device specific offset. Actually I tried
following as what GIC is representing.

As there are only two users upstreamed, I though it is better to clean it up
asap. Do you prefer maintaining the existing usage with a different compatible?


> 
>> +- power-domains:	phandle pointing to the corresponding PM domain node
>> +			and an ID representing the device.
> 
> Why is this power-domain thing part of this patch? Is it related?
> 
>>  - ti,sci:		Phandle to TI-SCI compatible System controller node.
>> -- ti,sci-dst-id:	TISCI device ID of the destination IRQ controller.
>> -- ti,sci-rm-range-girq:	Array of TISCI subtype ids representing the host irqs
>> -			assigned to this interrupt router. Each subtype id
>> -			corresponds to a range of host irqs.
>> +- interrupt-ranges:	Ranges that convert the INTR output interrupt numbers to
>> +			parent's interrupt number.
> 
> This isn't a standard property, is it? If it isn't, it should be
> documented a bit more than just that.

Nope. Ill update it. So, the reason this is added is that this ranges converts
the output of INTR to parent interrupt number. This is to have consistent dt
convention in the following 2 scenarios:
- INTR -> GIC (GIC is parent of INTR)
- INTR -> INTR (INTR is parent of INTR)

Thanks and regards,
Lokesh

> 
>>  
>>  For more details on TISCI IRQ resource management refer:
>>  http://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/rm/rm_irq.html
>> @@ -62,21 +61,20 @@ Example:
>>  The following example demonstrates both interrupt router node and the consumer
>>  node(main gpio) on the AM654 SoC:
>>  
>> -main_intr: interrupt-controller0 {
>> +main_gpio_intr: interrupt-controller0 {
>>  	compatible = "ti,sci-intr";
>>  	ti,intr-trigger-type = <1>;
>>  	interrupt-controller;
>>  	interrupt-parent = <&gic500>;
>> -	#interrupt-cells = <2>;
>> +	power-domains = <&k3_pds 131 TI_SCI_PD_SHARED>;
>> +	#interrupt-cells = <1>;
>>  	ti,sci = <&dmsc>;
>> -	ti,sci-dst-id = <56>;
>> -	ti,sci-rm-range-girq = <0x1>;
>> +	interrupt-ranges = <0 360 32>;
>>  };
>>  
>>  main_gpio0: gpio@600000 {
>>  	...
>> -	interrupt-parent = <&main_intr>;
>> -	interrupts = <57 256>, <57 257>, <57 258>,
>> -		     <57 259>, <57 260>, <57 261>;
>> +	interrupt-parent = <&main_gpio_intr>;
>> +	interrupts = <192>, <193>, <194>, <195>, <196>, <197>;
>>  	...
>>  };
> 
> Thanks,
> 
> 	M.
> 

_______________________________________________
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] 12+ messages in thread

* Re: [RFC PATCH 2/2] irqchip/ti-sci-intr: Add support for INTR being a parent to INTR
  2019-10-02 13:12   ` Marc Zyngier
@ 2019-10-04  3:37     ` Lokesh Vutla
  0 siblings, 0 replies; 12+ messages in thread
From: Lokesh Vutla @ 2019-10-04  3:37 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Nishanth Menon, Linux ARM Mailing List

Hi Marc,

On 02/10/19 6:42 PM, Marc Zyngier wrote:
> On Mon, 23 Sep 2019 09:54:05 +0530
> Lokesh Vutla <lokeshvutla@ti.com> wrote:
> 
>> Driver assumes that Interrupt parent to Interrupt router is always GIC.
>> This is not true always and a Interrupt Router can be a parent to
>> Interrupt Router. Update the driver to detect the parent and request the
>> parent irqs accordingly.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>>  drivers/irqchip/irq-ti-sci-intr.c | 137 ++++++++++++++++++------------
>>  1 file changed, 82 insertions(+), 55 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-ti-sci-intr.c b/drivers/irqchip/irq-ti-sci-intr.c
>> index 59d51a20bbd8..1d8aaa871798 100644
>> --- a/drivers/irqchip/irq-ti-sci-intr.c
>> +++ b/drivers/irqchip/irq-ti-sci-intr.c
>> @@ -17,29 +17,18 @@
>>  #include <linux/of_irq.h>
>>  #include <linux/soc/ti/ti_sci_protocol.h>
>>  
>> -#define TI_SCI_DEV_ID_MASK	0xffff
>> -#define TI_SCI_DEV_ID_SHIFT	16
>> -#define TI_SCI_IRQ_ID_MASK	0xffff
>> -#define TI_SCI_IRQ_ID_SHIFT	0
>> -#define HWIRQ_TO_DEVID(hwirq)	(((hwirq) >> (TI_SCI_DEV_ID_SHIFT)) & \
>> -				 (TI_SCI_DEV_ID_MASK))
>> -#define HWIRQ_TO_IRQID(hwirq)	((hwirq) & (TI_SCI_IRQ_ID_MASK))
>> -#define TO_HWIRQ(dev, index)	((((dev) & TI_SCI_DEV_ID_MASK) << \
>> -				 TI_SCI_DEV_ID_SHIFT) | \
>> -				((index) & TI_SCI_IRQ_ID_MASK))
>> -
>>  /**
>>   * struct ti_sci_intr_irq_domain - Structure representing a TISCI based
>>   *				   Interrupt Router IRQ domain.
>>   * @sci:	Pointer to TISCI handle
>> - * @dst_irq:	TISCI resource pointer representing GIC irq controller.
>> - * @dst_id:	TISCI device ID of the GIC irq controller.
>> + * @out_irqs:	TISCI resource pointer representing INTR irqs.
>> + * @pdev:	Pointer to platform device.
>>   * @type:	Specifies the trigger type supported by this Interrupt Router
>>   */
>>  struct ti_sci_intr_irq_domain {
>>  	const struct ti_sci_handle *sci;
>> -	struct ti_sci_resource *dst_irq;
>> -	u32 dst_id;
>> +	struct ti_sci_resource *out_irqs;
>> +	struct platform_device *pdev;
>>  	u32 type;
>>  };
>>  
>> @@ -70,15 +59,43 @@ static int ti_sci_intr_irq_domain_translate(struct irq_domain *domain,
>>  {
>>  	struct ti_sci_intr_irq_domain *intr = domain->host_data;
>>  
>> -	if (fwspec->param_count != 2)
>> +	if (fwspec->param_count != 1)
>>  		return -EINVAL;
> 
> As I said in reply to the first patch, this breaks badly. Except that
> this time this is a new kernel on an old DT.
> 
>>  
>> -	*hwirq = TO_HWIRQ(fwspec->param[0], fwspec->param[1]);
>> +	*hwirq = fwspec->param[0];
>>  	*type = intr->type;
>>  
>>  	return 0;
>>  }
>>  
>> +static int ti_sci_intr_xlate_irq(struct ti_sci_intr_irq_domain *intr,
>> +				 u32 irq, bool to_parent_hwirq)
>> +{
>> +	struct device_node *np = dev_of_node(&intr->pdev->dev);
>> +	u32 base, pbase, size, len;
>> +	const __be32 *range;
>> +
>> +	range = of_get_property(np, "interrupt-ranges", &len);
>> +	if (!range)
>> +		return irq;
>> +
>> +	for (len /= sizeof(*range); len >= 3; len -= 3) {
>> +		base = be32_to_cpu(*range++);
>> +		pbase = be32_to_cpu(*range++);
>> +		size = be32_to_cpu(*range++);
>> +
>> +		if (to_parent_hwirq) {
>> +			if (base <= irq && irq < base + size)
>> +				return irq - base + pbase;
>> +		} else {
>> +			if (pbase <= irq && irq < pbase + size)
>> +				return irq - pbase + base;
>> +		}
> 
> Using the same function to compute two different things is really ugly.
> It'd be nicer if you had xlate_hwirq_to_parent() and
> xlate_parent_to_hwirq(), each one doing their job.

Sure will update with two separate functions.

> 
> Also, it isn't clear to me why there is only a single tuple in the
> interrupt-ranges property. Is it a guarantee that you'll never have
> multiple ranges?

Right there can be multiple ranges. This function handles multiple ranges as you
described.

> 
>> +	}
>> +
>> +	return -ENOENT;
>> +}
>> +
>>  /**
>>   * ti_sci_intr_irq_domain_free() - Free the specified IRQs from the domain.
>>   * @domain:	Domain to which the irqs belong
>> @@ -90,56 +107,72 @@ static void ti_sci_intr_irq_domain_free(struct irq_domain *domain,
>>  {
>>  	struct ti_sci_intr_irq_domain *intr = domain->host_data;
>>  	struct irq_data *data, *parent_data;
>> -	u16 dev_id, irq_index;
>> +	int out_irq;
>>  
>>  	parent_data = irq_domain_get_irq_data(domain->parent, virq);
>> +	out_irq = ti_sci_intr_xlate_irq(intr, parent_data->hwirq, false);
>>  	data = irq_domain_get_irq_data(domain, virq);
>> -	irq_index = HWIRQ_TO_IRQID(data->hwirq);
>> -	dev_id = HWIRQ_TO_DEVID(data->hwirq);
>>  
>> -	intr->sci->ops.rm_irq_ops.free_irq(intr->sci, dev_id, irq_index,
>> -					   intr->dst_id, parent_data->hwirq);
>> -	ti_sci_release_resource(intr->dst_irq, parent_data->hwirq);
>> +	intr->sci->ops.rm_irq_ops.free_irq(intr->sci,
>> +					   intr->pdev->id, data->hwirq,
>> +					   intr->pdev->id, out_irq);
>> +	ti_sci_release_resource(intr->out_irqs, out_irq);
>>  	irq_domain_free_irqs_parent(domain, virq, 1);
>>  	irq_domain_reset_irq_data(data);
>>  }
>>  
>>  /**
>> - * ti_sci_intr_alloc_gic_irq() - Allocate GIC specific IRQ
>> + * ti_sci_intr_alloc_parent_irq() - Allocate parent IRQ
>>   * @domain:	Pointer to the interrupt router IRQ domain
>>   * @virq:	Corresponding Linux virtual IRQ number
>>   * @hwirq:	Corresponding hwirq for the IRQ within this IRQ domain
>>   *
>>   * Returns 0 if all went well else appropriate error pointer.
>>   */
>> -static int ti_sci_intr_alloc_gic_irq(struct irq_domain *domain,
>> -				     unsigned int virq, u32 hwirq)
>> +static int ti_sci_intr_alloc_parent_irq(struct irq_domain *domain,
>> +					unsigned int virq, u32 hwirq)
>>  {
>>  	struct ti_sci_intr_irq_domain *intr = domain->host_data;
>> +	struct device_node *parent_node;
>>  	struct irq_fwspec fwspec;
>> -	u16 dev_id, irq_index;
>> -	u16 dst_irq;
>> -	int err;
>> +	u16 out_irq, p_hwirq;
>> +	int err = 0;
>>  
>> -	dev_id = HWIRQ_TO_DEVID(hwirq);
>> -	irq_index = HWIRQ_TO_IRQID(hwirq);
>> -
>> -	dst_irq = ti_sci_get_free_resource(intr->dst_irq);
>> -	if (dst_irq == TI_SCI_RESOURCE_NULL)
>> +	out_irq = ti_sci_get_free_resource(intr->out_irqs);
>> +	if (out_irq == TI_SCI_RESOURCE_NULL)
>>  		return -EINVAL;
>>  
>> -	fwspec.fwnode = domain->parent->fwnode;
>> -	fwspec.param_count = 3;
>> -	fwspec.param[0] = 0;	/* SPI */
>> -	fwspec.param[1] = dst_irq - 32; /* SPI offset */
>> -	fwspec.param[2] = intr->type;
>> +	p_hwirq = ti_sci_intr_xlate_irq(intr, out_irq, true);
>> +	if (p_hwirq < 0)
>> +		goto err_irqs;
>> +
>> +	parent_node = of_irq_find_parent(dev_of_node(&intr->pdev->dev));
>> +	fwspec.fwnode = of_node_to_fwnode(parent_node);
>> +	if (of_property_read_u32(parent_node, "#interrupt-cells",
>> +				 &fwspec.param_count))
>> +		goto err_irqs;
>> +
>> +	if (fwspec.param_count == 1) {
>> +		/* Parent is Interrupt Router */
>> +		fwspec.param[0] = p_hwirq;
>> +	} else if (fwspec.param_count == 3) {
>> +		/* Parent is GIC */
>> +		fwspec.param[0] = 0;	/* SPI */
>> +		fwspec.param[1] = p_hwirq; /* SPI offset */
>> +		fwspec.param[2] = intr->type;
>> +	} else {
> 
> It really looks like you're flipping a coin and hope to hit the right
> backend. It feels terribly fragile, and it would be a lot simpler if
> the firmware actually exposed the "type" of parent controller, rather
> than this guesswork.

hmm..Firmware is bumb in this respect. It only programs what ever values are
provided with a minor policing on values. User should know the IRQ hierarchy and
call firmware in each step. In this case firmware cannot specify the parent
"type" as there can be multiple parents routed to different cores(like A72 or R5).

Do you suggest any better alternative?

Thanks and regards,
Lokesh

> 
>> +		/* Invalid parent */
>> +		dev_err(&intr->pdev->dev, "Unknown IRQ parent to INTR\n");
>> +		goto err_irqs;
>> +	}
>>  
>>  	err = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
>>  	if (err)
>>  		goto err_irqs;
>>  
>> -	err = intr->sci->ops.rm_irq_ops.set_irq(intr->sci, dev_id, irq_index,
>> -						intr->dst_id, dst_irq);
>> +	err = intr->sci->ops.rm_irq_ops.set_irq(intr->sci,
>> +						intr->pdev->id, hwirq,
>> +						intr->pdev->id, out_irq);
>>  	if (err)
>>  		goto err_msg;
>>  
>> @@ -148,7 +181,7 @@ static int ti_sci_intr_alloc_gic_irq(struct irq_domain *domain,
>>  err_msg:
>>  	irq_domain_free_irqs_parent(domain, virq, 1);
>>  err_irqs:
>> -	ti_sci_release_resource(intr->dst_irq, dst_irq);
>> +	ti_sci_release_resource(intr->out_irqs, out_irq);
>>  	return err;
>>  }
>>  
>> @@ -174,7 +207,7 @@ static int ti_sci_intr_irq_domain_alloc(struct irq_domain *domain,
>>  	if (err)
>>  		return err;
>>  
>> -	err = ti_sci_intr_alloc_gic_irq(domain, virq, hwirq);
>> +	err = ti_sci_intr_alloc_parent_irq(domain, virq, hwirq);
>>  	if (err)
>>  		return err;
>>  
>> @@ -214,6 +247,7 @@ static int ti_sci_intr_irq_domain_probe(struct platform_device *pdev)
>>  	if (!intr)
>>  		return -ENOMEM;
>>  
>> +	intr->pdev = pdev;
>>  	ret = of_property_read_u32(dev_of_node(dev), "ti,intr-trigger-type",
>>  				   &intr->type);
>>  	if (ret) {
>> @@ -230,19 +264,10 @@ static int ti_sci_intr_irq_domain_probe(struct platform_device *pdev)
>>  		return ret;
>>  	}
>>  
>> -	ret = of_property_read_u32(dev_of_node(dev), "ti,sci-dst-id",
>> -				   &intr->dst_id);
>> -	if (ret) {
>> -		dev_err(dev, "missing 'ti,sci-dst-id' property\n");
>> -		return -EINVAL;
>> -	}
>> -
>> -	intr->dst_irq = devm_ti_sci_get_of_resource(intr->sci, dev,
>> -						    intr->dst_id,
>> -						    "ti,sci-rm-range-girq");
>> -	if (IS_ERR(intr->dst_irq)) {
>> +	intr->out_irqs = devm_ti_sci_get_irq_range(intr->sci, dev);
>> +	if (IS_ERR(intr->out_irqs)) {
>>  		dev_err(dev, "Destination irq resource allocation failed\n");
>> -		return PTR_ERR(intr->dst_irq);
>> +		return PTR_ERR(intr->out_irqs);
>>  	}
>>  
>>  	domain = irq_domain_add_hierarchy(parent_domain, 0, 0, dev_of_node(dev),
>> @@ -252,6 +277,8 @@ static int ti_sci_intr_irq_domain_probe(struct platform_device *pdev)
>>  		return -ENOMEM;
>>  	}
>>  
>> +	dev_info(dev, "Interrupt Router %d domain created\n", pdev->id);
>> +
>>  	return 0;
>>  }
>>  
> 
> Thanks,
> 
> 	M.
> 

_______________________________________________
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] 12+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: irqchip: Update bindings to drop the usage of gic as parent
  2019-10-03 13:15     ` Lokesh Vutla
@ 2019-10-09  5:00       ` Lokesh Vutla
  2019-10-24 10:22         ` Lokesh Vutla
  0 siblings, 1 reply; 12+ messages in thread
From: Lokesh Vutla @ 2019-10-09  5:00 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Nishanth Menon, Linux ARM Mailing List

Hi Marc,

On 03/10/19 6:45 PM, Lokesh Vutla wrote:
> Hi Marc,
> 
> On 02/10/19 6:27 PM, Marc Zyngier wrote:
>> On Mon, 23 Sep 2019 09:54:04 +0530
>> Lokesh Vutla <lokeshvutla@ti.com> wrote:
>>
>>> Drop the firmware related dt-bindings and use the hardware specified
>>> interrupt numbers within Interrupt Router. This ensures interrupt router
>>> DT node need not assume any interrupt parent type.
>>>
>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>> ---
>>>  .../interrupt-controller/ti,sci-intr.txt      | 28 +++++++++----------
>>>  1 file changed, 13 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>> index 1a8718f8855d..de5de2a4b467 100644
>>> --- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>> @@ -44,15 +44,14 @@ Required Properties:
>>>  			4: If intr supports level triggered interrupts.
>>>  - interrupt-controller:	Identifies the node as an interrupt controller
>>>  - #interrupt-cells:	Specifies the number of cells needed to encode an
>>> -			interrupt source. The value should be 2.
>>> -			First cell should contain the TISCI device ID of source
>>> -			Second cell should contain the interrupt source offset
>>> -			within the device.
>>> +			interrupt source. The value should be 1.
>>> +			First cell should contain interrupt router input number
>>> +			as specified by hardware.
>>
>> This is breaking compatibility with existing kernels. Why isn't that a
>> problem?
> 
> Yes, I am not denying that backward compatibility is broken. But I feel this is
> a good cleanup for representing INTR interrupts. With this child nodes will pass
> the INTR specific number rather the device specific offset. Actually I tried
> following as what GIC is representing.
> 
> As there are only two users upstreamed, I though it is better to clean it up
> asap. Do you prefer maintaining the existing usage with a different compatible?

Just looking for your opinion here. If I try to maintain DT backward compatible,
are you okay with the overall approach. Will clean and repost patches once
firmware is ready.

Thank and regards,
Lokesh

> 
> 
>>
>>> +- power-domains:	phandle pointing to the corresponding PM domain node
>>> +			and an ID representing the device.
>>
>> Why is this power-domain thing part of this patch? Is it related?
>>
>>>  - ti,sci:		Phandle to TI-SCI compatible System controller node.
>>> -- ti,sci-dst-id:	TISCI device ID of the destination IRQ controller.
>>> -- ti,sci-rm-range-girq:	Array of TISCI subtype ids representing the host irqs
>>> -			assigned to this interrupt router. Each subtype id
>>> -			corresponds to a range of host irqs.
>>> +- interrupt-ranges:	Ranges that convert the INTR output interrupt numbers to
>>> +			parent's interrupt number.
>>
>> This isn't a standard property, is it? If it isn't, it should be
>> documented a bit more than just that.
> 
> Nope. Ill update it. So, the reason this is added is that this ranges converts
> the output of INTR to parent interrupt number. This is to have consistent dt
> convention in the following 2 scenarios:
> - INTR -> GIC (GIC is parent of INTR)
> - INTR -> INTR (INTR is parent of INTR)
> 
> Thanks and regards,
> Lokesh
> 
>>
>>>  
>>>  For more details on TISCI IRQ resource management refer:
>>>  http://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/rm/rm_irq.html
>>> @@ -62,21 +61,20 @@ Example:
>>>  The following example demonstrates both interrupt router node and the consumer
>>>  node(main gpio) on the AM654 SoC:
>>>  
>>> -main_intr: interrupt-controller0 {
>>> +main_gpio_intr: interrupt-controller0 {
>>>  	compatible = "ti,sci-intr";
>>>  	ti,intr-trigger-type = <1>;
>>>  	interrupt-controller;
>>>  	interrupt-parent = <&gic500>;
>>> -	#interrupt-cells = <2>;
>>> +	power-domains = <&k3_pds 131 TI_SCI_PD_SHARED>;
>>> +	#interrupt-cells = <1>;
>>>  	ti,sci = <&dmsc>;
>>> -	ti,sci-dst-id = <56>;
>>> -	ti,sci-rm-range-girq = <0x1>;
>>> +	interrupt-ranges = <0 360 32>;
>>>  };
>>>  
>>>  main_gpio0: gpio@600000 {
>>>  	...
>>> -	interrupt-parent = <&main_intr>;
>>> -	interrupts = <57 256>, <57 257>, <57 258>,
>>> -		     <57 259>, <57 260>, <57 261>;
>>> +	interrupt-parent = <&main_gpio_intr>;
>>> +	interrupts = <192>, <193>, <194>, <195>, <196>, <197>;
>>>  	...
>>>  };
>>
>> Thanks,
>>
>> 	M.
>>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

_______________________________________________
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] 12+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: irqchip: Update bindings to drop the usage of gic as parent
  2019-10-09  5:00       ` Lokesh Vutla
@ 2019-10-24 10:22         ` Lokesh Vutla
  2019-10-25 10:46           ` Marc Zyngier
  0 siblings, 1 reply; 12+ messages in thread
From: Lokesh Vutla @ 2019-10-24 10:22 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Nishanth Menon, Linux ARM Mailing List

Hi Marc,

On 09/10/19 10:30 AM, Lokesh Vutla wrote:
> Hi Marc,
> 
> On 03/10/19 6:45 PM, Lokesh Vutla wrote:
>> Hi Marc,
>>
>> On 02/10/19 6:27 PM, Marc Zyngier wrote:
>>> On Mon, 23 Sep 2019 09:54:04 +0530
>>> Lokesh Vutla <lokeshvutla@ti.com> wrote:
>>>
>>>> Drop the firmware related dt-bindings and use the hardware specified
>>>> interrupt numbers within Interrupt Router. This ensures interrupt router
>>>> DT node need not assume any interrupt parent type.
>>>>
>>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>>> ---
>>>>  .../interrupt-controller/ti,sci-intr.txt      | 28 +++++++++----------
>>>>  1 file changed, 13 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>>> index 1a8718f8855d..de5de2a4b467 100644
>>>> --- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>>> @@ -44,15 +44,14 @@ Required Properties:
>>>>  			4: If intr supports level triggered interrupts.
>>>>  - interrupt-controller:	Identifies the node as an interrupt controller
>>>>  - #interrupt-cells:	Specifies the number of cells needed to encode an
>>>> -			interrupt source. The value should be 2.
>>>> -			First cell should contain the TISCI device ID of source
>>>> -			Second cell should contain the interrupt source offset
>>>> -			within the device.
>>>> +			interrupt source. The value should be 1.
>>>> +			First cell should contain interrupt router input number
>>>> +			as specified by hardware.
>>>
>>> This is breaking compatibility with existing kernels. Why isn't that a
>>> problem?
>>
>> Yes, I am not denying that backward compatibility is broken. But I feel this is
>> a good cleanup for representing INTR interrupts. With this child nodes will pass
>> the INTR specific number rather the device specific offset. Actually I tried
>> following as what GIC is representing.
>>
>> As there are only two users upstreamed, I though it is better to clean it up
>> asap. Do you prefer maintaining the existing usage with a different compatible?
> 
> Just looking for your opinion here. If I try to maintain DT backward compatible,
> are you okay with the overall approach. Will clean and repost patches once
> firmware is ready.

Gentle ping, can you provide your thoughts here?

Thanks and regards,
Lokesh

> 
> Thank and regards,
> Lokesh
> 
>>
>>
>>>
>>>> +- power-domains:	phandle pointing to the corresponding PM domain node
>>>> +			and an ID representing the device.
>>>
>>> Why is this power-domain thing part of this patch? Is it related?
>>>
>>>>  - ti,sci:		Phandle to TI-SCI compatible System controller node.
>>>> -- ti,sci-dst-id:	TISCI device ID of the destination IRQ controller.
>>>> -- ti,sci-rm-range-girq:	Array of TISCI subtype ids representing the host irqs
>>>> -			assigned to this interrupt router. Each subtype id
>>>> -			corresponds to a range of host irqs.
>>>> +- interrupt-ranges:	Ranges that convert the INTR output interrupt numbers to
>>>> +			parent's interrupt number.
>>>
>>> This isn't a standard property, is it? If it isn't, it should be
>>> documented a bit more than just that.
>>
>> Nope. Ill update it. So, the reason this is added is that this ranges converts
>> the output of INTR to parent interrupt number. This is to have consistent dt
>> convention in the following 2 scenarios:
>> - INTR -> GIC (GIC is parent of INTR)
>> - INTR -> INTR (INTR is parent of INTR)
>>
>> Thanks and regards,
>> Lokesh
>>
>>>
>>>>  
>>>>  For more details on TISCI IRQ resource management refer:
>>>>  http://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/rm/rm_irq.html
>>>> @@ -62,21 +61,20 @@ Example:
>>>>  The following example demonstrates both interrupt router node and the consumer
>>>>  node(main gpio) on the AM654 SoC:
>>>>  
>>>> -main_intr: interrupt-controller0 {
>>>> +main_gpio_intr: interrupt-controller0 {
>>>>  	compatible = "ti,sci-intr";
>>>>  	ti,intr-trigger-type = <1>;
>>>>  	interrupt-controller;
>>>>  	interrupt-parent = <&gic500>;
>>>> -	#interrupt-cells = <2>;
>>>> +	power-domains = <&k3_pds 131 TI_SCI_PD_SHARED>;
>>>> +	#interrupt-cells = <1>;
>>>>  	ti,sci = <&dmsc>;
>>>> -	ti,sci-dst-id = <56>;
>>>> -	ti,sci-rm-range-girq = <0x1>;
>>>> +	interrupt-ranges = <0 360 32>;
>>>>  };
>>>>  
>>>>  main_gpio0: gpio@600000 {
>>>>  	...
>>>> -	interrupt-parent = <&main_intr>;
>>>> -	interrupts = <57 256>, <57 257>, <57 258>,
>>>> -		     <57 259>, <57 260>, <57 261>;
>>>> +	interrupt-parent = <&main_gpio_intr>;
>>>> +	interrupts = <192>, <193>, <194>, <195>, <196>, <197>;
>>>>  	...
>>>>  };
>>>
>>> Thanks,
>>>
>>> 	M.
>>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

_______________________________________________
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] 12+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: irqchip: Update bindings to drop the usage of gic as parent
  2019-10-24 10:22         ` Lokesh Vutla
@ 2019-10-25 10:46           ` Marc Zyngier
  0 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2019-10-25 10:46 UTC (permalink / raw)
  To: Lokesh Vutla; +Cc: Nishanth Menon, Linux ARM Mailing List

On 2019-10-24 11:22, Lokesh Vutla wrote:
> Hi Marc,
>
> On 09/10/19 10:30 AM, Lokesh Vutla wrote:
>> Hi Marc,
>>
>> On 03/10/19 6:45 PM, Lokesh Vutla wrote:
>>> Hi Marc,
>>>
>>> On 02/10/19 6:27 PM, Marc Zyngier wrote:
>>>> On Mon, 23 Sep 2019 09:54:04 +0530
>>>> Lokesh Vutla <lokeshvutla@ti.com> wrote:
>>>>
>>>>> Drop the firmware related dt-bindings and use the hardware 
>>>>> specified
>>>>> interrupt numbers within Interrupt Router. This ensures interrupt 
>>>>> router
>>>>> DT node need not assume any interrupt parent type.
>>>>>
>>>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>>>> ---
>>>>>  .../interrupt-controller/ti,sci-intr.txt      | 28 
>>>>> +++++++++----------
>>>>>  1 file changed, 13 insertions(+), 15 deletions(-)
>>>>>
>>>>> diff --git 
>>>>> a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt 
>>>>> b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>>>> index 1a8718f8855d..de5de2a4b467 100644
>>>>> --- 
>>>>> a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>>>> +++ 
>>>>> b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>>>> @@ -44,15 +44,14 @@ Required Properties:
>>>>>  			4: If intr supports level triggered interrupts.
>>>>>  - interrupt-controller:	Identifies the node as an interrupt 
>>>>> controller
>>>>>  - #interrupt-cells:	Specifies the number of cells needed to 
>>>>> encode an
>>>>> -			interrupt source. The value should be 2.
>>>>> -			First cell should contain the TISCI device ID of source
>>>>> -			Second cell should contain the interrupt source offset
>>>>> -			within the device.
>>>>> +			interrupt source. The value should be 1.
>>>>> +			First cell should contain interrupt router input number
>>>>> +			as specified by hardware.
>>>>
>>>> This is breaking compatibility with existing kernels. Why isn't 
>>>> that a
>>>> problem?
>>>
>>> Yes, I am not denying that backward compatibility is broken. But I 
>>> feel this is
>>> a good cleanup for representing INTR interrupts. With this child 
>>> nodes will pass
>>> the INTR specific number rather the device specific offset. 
>>> Actually I tried
>>> following as what GIC is representing.
>>>
>>> As there are only two users upstreamed, I though it is better to 
>>> clean it up
>>> asap. Do you prefer maintaining the existing usage with a different 
>>> compatible?
>>
>> Just looking for your opinion here. If I try to maintain DT backward 
>> compatible,
>> are you okay with the overall approach. Will clean and repost 
>> patches once
>> firmware is ready.
>
> Gentle ping, can you provide your thoughts here?

Preserving backward compatibility is only required if you have 
deployment of
this in the wild. If nobody is using the stuff, then it's fine to break 
it.
If people are actively using it, then preserving compatibility is 
mandatory.

         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] 12+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: irqchip: Update bindings to drop the usage of gic as parent
  2019-09-23  4:24 ` [RFC PATCH 1/2] dt-bindings: irqchip: Update bindings to drop the usage of gic as parent Lokesh Vutla
  2019-10-02 12:57   ` Marc Zyngier
@ 2019-10-25 16:07   ` Peter Ujfalusi
  2019-10-29  6:20     ` Lokesh Vutla
  1 sibling, 1 reply; 12+ messages in thread
From: Peter Ujfalusi @ 2019-10-25 16:07 UTC (permalink / raw)
  To: Lokesh Vutla, Marc Zyngier; +Cc: Nishanth Menon, Linux ARM Mailing List

Hi Lokesh,

On 9/23/19 7:24 AM, Lokesh Vutla wrote:
> Drop the firmware related dt-bindings and use the hardware specified
> interrupt numbers within Interrupt Router. This ensures interrupt router
> DT node need not assume any interrupt parent type.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  .../interrupt-controller/ti,sci-intr.txt      | 28 +++++++++----------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> index 1a8718f8855d..de5de2a4b467 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> @@ -44,15 +44,14 @@ Required Properties:
>  			4: If intr supports level triggered interrupts.
>  - interrupt-controller:	Identifies the node as an interrupt controller
>  - #interrupt-cells:	Specifies the number of cells needed to encode an
> -			interrupt source. The value should be 2.
> -			First cell should contain the TISCI device ID of source
> -			Second cell should contain the interrupt source offset
> -			within the device.
> +			interrupt source. The value should be 1.
> +			First cell should contain interrupt router input number
> +			as specified by hardware.
> +- power-domains:	phandle pointing to the corresponding PM domain node
> +			and an ID representing the device.
>  - ti,sci:		Phandle to TI-SCI compatible System controller node.
> -- ti,sci-dst-id:	TISCI device ID of the destination IRQ controller.
> -- ti,sci-rm-range-girq:	Array of TISCI subtype ids representing the host irqs
> -			assigned to this interrupt router. Each subtype id
> -			corresponds to a range of host irqs.
> +- interrupt-ranges:	Ranges that convert the INTR output interrupt numbers to
> +			parent's interrupt number.

I believe this change is transparent for INTA for things inside NAVSS,
like DMA, rings, etc?

>  
>  For more details on TISCI IRQ resource management refer:
>  http://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/rm/rm_irq.html
> @@ -62,21 +61,20 @@ Example:
>  The following example demonstrates both interrupt router node and the consumer
>  node(main gpio) on the AM654 SoC:
>  
> -main_intr: interrupt-controller0 {
> +main_gpio_intr: interrupt-controller0 {
>  	compatible = "ti,sci-intr";
>  	ti,intr-trigger-type = <1>;
>  	interrupt-controller;
>  	interrupt-parent = <&gic500>;
> -	#interrupt-cells = <2>;
> +	power-domains = <&k3_pds 131 TI_SCI_PD_SHARED>;
> +	#interrupt-cells = <1>;
>  	ti,sci = <&dmsc>;
> -	ti,sci-dst-id = <56>;
> -	ti,sci-rm-range-girq = <0x1>;
> +	interrupt-ranges = <0 360 32>;
>  };
>  
>  main_gpio0: gpio@600000 {
>  	...
> -	interrupt-parent = <&main_intr>;
> -	interrupts = <57 256>, <57 257>, <57 258>,
> -		     <57 259>, <57 260>, <57 261>;
> +	interrupt-parent = <&main_gpio_intr>;
> +	interrupts = <192>, <193>, <194>, <195>, <196>, <197>;
>  	...
>  };
> 

- Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
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] 12+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: irqchip: Update bindings to drop the usage of gic as parent
  2019-10-25 16:07   ` Peter Ujfalusi
@ 2019-10-29  6:20     ` Lokesh Vutla
  0 siblings, 0 replies; 12+ messages in thread
From: Lokesh Vutla @ 2019-10-29  6:20 UTC (permalink / raw)
  To: Peter Ujfalusi, Marc Zyngier; +Cc: Nishanth Menon, Linux ARM Mailing List



On 25/10/19 9:37 PM, Peter Ujfalusi wrote:
> Hi Lokesh,
> 
> On 9/23/19 7:24 AM, Lokesh Vutla wrote:
>> Drop the firmware related dt-bindings and use the hardware specified
>> interrupt numbers within Interrupt Router. This ensures interrupt router
>> DT node need not assume any interrupt parent type.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>>  .../interrupt-controller/ti,sci-intr.txt      | 28 +++++++++----------
>>  1 file changed, 13 insertions(+), 15 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>> index 1a8718f8855d..de5de2a4b467 100644
>> --- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>> @@ -44,15 +44,14 @@ Required Properties:
>>  			4: If intr supports level triggered interrupts.
>>  - interrupt-controller:	Identifies the node as an interrupt controller
>>  - #interrupt-cells:	Specifies the number of cells needed to encode an
>> -			interrupt source. The value should be 2.
>> -			First cell should contain the TISCI device ID of source
>> -			Second cell should contain the interrupt source offset
>> -			within the device.
>> +			interrupt source. The value should be 1.
>> +			First cell should contain interrupt router input number
>> +			as specified by hardware.
>> +- power-domains:	phandle pointing to the corresponding PM domain node
>> +			and an ID representing the device.
>>  - ti,sci:		Phandle to TI-SCI compatible System controller node.
>> -- ti,sci-dst-id:	TISCI device ID of the destination IRQ controller.
>> -- ti,sci-rm-range-girq:	Array of TISCI subtype ids representing the host irqs
>> -			assigned to this interrupt router. Each subtype id
>> -			corresponds to a range of host irqs.
>> +- interrupt-ranges:	Ranges that convert the INTR output interrupt numbers to
>> +			parent's interrupt number.
> 
> I believe this change is transparent for INTA for things inside NAVSS,
> like DMA, rings, etc?

Right. This ranges converts only output to input.

Thanks and regards,
Lokesh

> 
>>  
>>  For more details on TISCI IRQ resource management refer:
>>  http://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/rm/rm_irq.html
>> @@ -62,21 +61,20 @@ Example:
>>  The following example demonstrates both interrupt router node and the consumer
>>  node(main gpio) on the AM654 SoC:
>>  
>> -main_intr: interrupt-controller0 {
>> +main_gpio_intr: interrupt-controller0 {
>>  	compatible = "ti,sci-intr";
>>  	ti,intr-trigger-type = <1>;
>>  	interrupt-controller;
>>  	interrupt-parent = <&gic500>;
>> -	#interrupt-cells = <2>;
>> +	power-domains = <&k3_pds 131 TI_SCI_PD_SHARED>;
>> +	#interrupt-cells = <1>;
>>  	ti,sci = <&dmsc>;
>> -	ti,sci-dst-id = <56>;
>> -	ti,sci-rm-range-girq = <0x1>;
>> +	interrupt-ranges = <0 360 32>;
>>  };
>>  
>>  main_gpio0: gpio@600000 {
>>  	...
>> -	interrupt-parent = <&main_intr>;
>> -	interrupts = <57 256>, <57 257>, <57 258>,
>> -		     <57 259>, <57 260>, <57 261>;
>> +	interrupt-parent = <&main_gpio_intr>;
>> +	interrupts = <192>, <193>, <194>, <195>, <196>, <197>;
>>  	...
>>  };
>>
> 
> - Peter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

_______________________________________________
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] 12+ messages in thread

end of thread, other threads:[~2019-10-29  6:21 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-23  4:24 [RFC PATCH 0/2] irqchip/ti-sci-intr: Add support for INTR connecting to INTR Lokesh Vutla
2019-09-23  4:24 ` [RFC PATCH 1/2] dt-bindings: irqchip: Update bindings to drop the usage of gic as parent Lokesh Vutla
2019-10-02 12:57   ` Marc Zyngier
2019-10-03 13:15     ` Lokesh Vutla
2019-10-09  5:00       ` Lokesh Vutla
2019-10-24 10:22         ` Lokesh Vutla
2019-10-25 10:46           ` Marc Zyngier
2019-10-25 16:07   ` Peter Ujfalusi
2019-10-29  6:20     ` Lokesh Vutla
2019-09-23  4:24 ` [RFC PATCH 2/2] irqchip/ti-sci-intr: Add support for INTR being a parent to INTR Lokesh Vutla
2019-10-02 13:12   ` Marc Zyngier
2019-10-04  3:37     ` Lokesh Vutla

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