[PATCH v6 00/12] Add support for TISCI Interrupt controller drivers

[PATCH v6 00/12] Add support for TISCI Interrupt controller drivers

[Lokesh Vutla]

TI AM65x SoC based on K3 architecture introduced support for Events
which are message based interrupts with minimal latency. These events
are not compatible with regular interrupts and are valid only through
an event transport lane. An Interrupt Aggregator(INTA) is introduced
to convert these events to interrupts. INTA can also group 64 events
into a single interrupt. Now the SoC has many peripherals and a large
number of event sources (time sync or DMA), the use of events is
completely dependent on a user's specific application, which drives a
need for maximum flexibility in which event sources are used in the
system. It is also completely up to software control as to how the
events are serviced.

Because of the huge flexibility there are certain standard peripherals
(like GPIO etc)where all interrupts cannot be directly corrected to host
interrupt controller. For this purpose, Interrupt Router(INTR) is
introduced in the SoC. INTR just does a classic interrupt redirection.

So the SoC has 3 types of interrupt controllers:
- GIC500
- Interrupt Router
- Interrupt Aggregator

Below is a diagrammatic view of how SoC integration of these interrupt
controllers:(https://pastebin.ubuntu.com/p/9ngV3jdGj2/)

Device Index-x               Device Index-y
           |                         |
           |                         |
                      ....
            \                       /
             \                     /
              \  (global events)  /
          +---------------------------+   +---------+
          |                           |   |         |
          |             INTA          |   |  GPIO   |
          |                           |   |         |
          +---------------------------+   +---------+
                         |   (vint)            |
                         |                     |
                        \|/                    |
          +---------------------------+        |
          |                           |<-------+
          |           INTR            |
          |                           |
          +---------------------------+
                         |
                         |
                        \|/ (gic irq)
          +---------------------------+
          |                           |
          |             GIC           |
          |                           |
          +---------------------------+

While at it, TISCI abstracts the handling of all above IRQ routes where
interrupt sources are not directly connected to host interrupt controller.
That would be configuration of Interrupt Aggregator and Interrupt Router.

This series adds support for:
- TISCI commands needed for IRQ configuration
- Interrupt Router(INTR) driver.
- Interrupt Aggregator(INTA) driver.
- Interrupt Aggregator MSI bus layer.

Changes since v5:
- Each patch has respective changes mentioned.

Grygorii Strashko (1):
  firmware: ti_sci: Add support to get TISCI handle using of_phandle

Lokesh Vutla (10):
  firmware: ti_sci: Add support for RM core ops
  firmware: ti_sci: Add support for IRQ management
  firmware: ti_sci: Add helper apis to manage resources
  dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
  irqchip: ti-sci-intr: Add support for Interrupt Router driver
  dt-bindings: irqchip: Introduce TISCI Interrupt Aggregator bindings
  irqchip: ti-sci-inta: Add support for Interrupt Aggregator driver
  soc: ti: Add MSI domain bus support for Interrupt Aggregator
  irqchip: ti-sci-inta: Add msi domain support
  soc: ti: am6: Enable interrupt controller driver

Peter Ujfalusi (1):
  firmware: ti_sci: Add RM mapping table for am654

 .../bindings/arm/keystone/ti,sci.txt          |   3 +-
 .../interrupt-controller/ti,sci-inta.txt      |  66 ++
 .../interrupt-controller/ti,sci-intr.txt      |  83 +++
 MAINTAINERS                                   |   6 +
 drivers/firmware/ti_sci.c                     | 666 ++++++++++++++++++
 drivers/firmware/ti_sci.h                     | 102 +++
 drivers/irqchip/Kconfig                       |  23 +
 drivers/irqchip/Makefile                      |   2 +
 drivers/irqchip/irq-ti-sci-inta.c             | 659 +++++++++++++++++
 drivers/irqchip/irq-ti-sci-intr.c             | 289 ++++++++
 drivers/soc/ti/Kconfig                        |  11 +
 drivers/soc/ti/Makefile                       |   1 +
 drivers/soc/ti/ti_sci_inta_msi.c              | 167 +++++
 include/linux/irqdomain.h                     |   1 +
 include/linux/msi.h                           |   6 +
 include/linux/soc/ti/ti_sci_inta_msi.h        |  23 +
 include/linux/soc/ti/ti_sci_protocol.h        | 126 ++++
 17 files changed, 2233 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
 create mode 100644 drivers/irqchip/irq-ti-sci-inta.c
 create mode 100644 drivers/irqchip/irq-ti-sci-intr.c
 create mode 100644 drivers/soc/ti/ti_sci_inta_msi.c
 create mode 100644 include/linux/soc/ti/ti_sci_inta_msi.h

-- 
2.21.0

[PATCH v6 01/12] firmware: ti_sci: Add support to get TISCI handle using of_phandle

[Lokesh Vutla]

From: Grygorii Strashko <grygorii.strashko@ti.com>

TISCI has been updated to have support for Resource management(likes
interrupts etc..). And there can be multiple device instances of a
resource type in a SoC. So every driver corresponding to a resource type
should get a TISCI handle so that it can make TISCI calls. And each
DT node corresponding to a device should exist under its corresponding
bus node as per the SoC architecture.

But existing apis in TISCI library assumes that all TISCI users are
child nodes of TISCI. Which is not true in the above case. So introduce
(devm_)ti_sci_get_by_phandle() apis that can be used by TISCI users
to get TISCI handle using of phandle property.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
Changes since v5:
- None

 drivers/firmware/ti_sci.c              | 83 ++++++++++++++++++++++++++
 include/linux/soc/ti/ti_sci_protocol.h | 17 ++++++
 2 files changed, 100 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 3fbbb61012c4..852043531233 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -1764,6 +1764,89 @@ const struct ti_sci_handle *devm_ti_sci_get_handle(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(devm_ti_sci_get_handle);
 
+/**
+ * ti_sci_get_by_phandle() - Get the TI SCI handle using DT phandle
+ * @np:		device node
+ * @property:	property name containing phandle on TISCI node
+ *
+ * NOTE: The function does not track individual clients of the framework
+ * and is expected to be maintained by caller of TI SCI protocol library.
+ * ti_sci_put_handle must be balanced with successful ti_sci_get_by_phandle
+ * Return: pointer to handle if successful, else:
+ * -EPROBE_DEFER if the instance is not ready
+ * -ENODEV if the required node handler is missing
+ * -EINVAL if invalid conditions are encountered.
+ */
+const struct ti_sci_handle *ti_sci_get_by_phandle(struct device_node *np,
+						  const char *property)
+{
+	struct ti_sci_handle *handle = NULL;
+	struct device_node *ti_sci_np;
+	struct ti_sci_info *info;
+	struct list_head *p;
+
+	if (!np) {
+		pr_err("I need a device pointer\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	ti_sci_np = of_parse_phandle(np, property, 0);
+	if (!ti_sci_np)
+		return ERR_PTR(-ENODEV);
+
+	mutex_lock(&ti_sci_list_mutex);
+	list_for_each(p, &ti_sci_list) {
+		info = list_entry(p, struct ti_sci_info, node);
+		if (ti_sci_np == info->dev->of_node) {
+			handle = &info->handle;
+			info->users++;
+			break;
+		}
+	}
+	mutex_unlock(&ti_sci_list_mutex);
+	of_node_put(ti_sci_np);
+
+	if (!handle)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	return handle;
+}
+EXPORT_SYMBOL_GPL(ti_sci_get_by_phandle);
+
+/**
+ * devm_ti_sci_get_by_phandle() - Managed get handle using phandle
+ * @dev:	Device pointer requesting TISCI handle
+ * @property:	property name containing phandle on TISCI node
+ *
+ * NOTE: This releases the handle once the device resources are
+ * no longer needed. MUST NOT BE released with ti_sci_put_handle.
+ * The function does not track individual clients of the framework
+ * and is expected to be maintained by caller of TI SCI protocol library.
+ *
+ * Return: 0 if all went fine, else corresponding error.
+ */
+const struct ti_sci_handle *devm_ti_sci_get_by_phandle(struct device *dev,
+						       const char *property)
+{
+	const struct ti_sci_handle *handle;
+	const struct ti_sci_handle **ptr;
+
+	ptr = devres_alloc(devm_ti_sci_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+	handle = ti_sci_get_by_phandle(dev_of_node(dev), property);
+
+	if (!IS_ERR(handle)) {
+		*ptr = handle;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return handle;
+}
+EXPORT_SYMBOL_GPL(devm_ti_sci_get_by_phandle);
+
 static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
 				void *cmd)
 {
diff --git a/include/linux/soc/ti/ti_sci_protocol.h b/include/linux/soc/ti/ti_sci_protocol.h
index 18435e5c6364..515587e9d373 100644
--- a/include/linux/soc/ti/ti_sci_protocol.h
+++ b/include/linux/soc/ti/ti_sci_protocol.h
@@ -217,6 +217,10 @@ struct ti_sci_handle {
 const struct ti_sci_handle *ti_sci_get_handle(struct device *dev);
 int ti_sci_put_handle(const struct ti_sci_handle *handle);
 const struct ti_sci_handle *devm_ti_sci_get_handle(struct device *dev);
+const struct ti_sci_handle *ti_sci_get_by_phandle(struct device_node *np,
+						  const char *property);
+const struct ti_sci_handle *devm_ti_sci_get_by_phandle(struct device *dev,
+						       const char *property);
 
 #else	/* CONFIG_TI_SCI_PROTOCOL */
 
@@ -236,6 +240,19 @@ const struct ti_sci_handle *devm_ti_sci_get_handle(struct device *dev)
 	return ERR_PTR(-EINVAL);
 }
 
+static inline
+const struct ti_sci_handle *ti_sci_get_by_phandle(struct device_node *np,
+						  const char *property)
+{
+	return ERR_PTR(-EINVAL);
+}
+
+static inline
+const struct ti_sci_handle *devm_ti_sci_get_by_phandle(struct device *dev,
+						       const char *property)
+{
+	return ERR_PTR(-EINVAL);
+}
 #endif	/* CONFIG_TI_SCI_PROTOCOL */
 
 #endif	/* __TISCI_PROTOCOL_H */
-- 
2.21.0

[PATCH v6 02/12] firmware: ti_sci: Add support for RM core ops

[Lokesh Vutla]

TISCI provides support for getting the resources(IRQ, RING etc..)
assigned to a specific device. These resources can be handled by
the client and in turn sends TISCI cmd to configure the resources.

It is very important that client should keep track on usage of these
resources.

Add support for TISCI commands to get resource ranges.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
Changes since v5:
- None

 drivers/firmware/ti_sci.c              | 170 +++++++++++++++++++++++++
 drivers/firmware/ti_sci.h              |  42 ++++++
 include/linux/soc/ti/ti_sci_protocol.h |  27 ++++
 3 files changed, 239 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 852043531233..503195223c09 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -64,6 +64,22 @@ struct ti_sci_xfers_info {
 	spinlock_t xfer_lock;
 };
 
+/**
+ * struct ti_sci_rm_type_map - Structure representing TISCI Resource
+ *				management representation of dev_ids.
+ * @dev_id:	TISCI device ID
+ * @type:	Corresponding id as identified by TISCI RM.
+ *
+ * Note: This is used only as a work around for using RM range apis
+ *	for AM654 SoC. For future SoCs dev_id will be used as type
+ *	for RM range APIs. In order to maintain ABI backward compatibility
+ *	type is not being changed for AM654 SoC.
+ */
+struct ti_sci_rm_type_map {
+	u32 dev_id;
+	u16 type;
+};
+
 /**
  * struct ti_sci_desc - Description of SoC integration
  * @default_host_id:	Host identifier representing the compute entity
@@ -71,12 +87,14 @@ struct ti_sci_xfers_info {
  * @max_msgs: Maximum number of messages that can be pending
  *		  simultaneously in the system
  * @max_msg_size: Maximum size of data per message that can be handled.
+ * @rm_type_map: RM resource type mapping structure.
  */
 struct ti_sci_desc {
 	u8 default_host_id;
 	int max_rx_timeout_ms;
 	int max_msgs;
 	int max_msg_size;
+	struct ti_sci_rm_type_map *rm_type_map;
 };
 
 /**
@@ -1600,6 +1618,153 @@ static int ti_sci_cmd_core_reboot(const struct ti_sci_handle *handle)
 	return ret;
 }
 
+static int ti_sci_get_resource_type(struct ti_sci_info *info, u16 dev_id,
+				    u16 *type)
+{
+	struct ti_sci_rm_type_map *rm_type_map = info->desc->rm_type_map;
+	bool found = false;
+	int i;
+
+	/* If map is not provided then assume dev_id is used as type */
+	if (!rm_type_map) {
+		*type = dev_id;
+		return 0;
+	}
+
+	for (i = 0; rm_type_map[i].dev_id; i++) {
+		if (rm_type_map[i].dev_id == dev_id) {
+			*type = rm_type_map[i].type;
+			found = true;
+			break;
+		}
+	}
+
+	if (!found)
+		return -EINVAL;
+
+	return 0;
+}
+
+/**
+ * ti_sci_get_resource_range - Helper to get a range of resources assigned
+ *			       to a host. Resource is uniquely identified by
+ *			       type and subtype.
+ * @handle:		Pointer to TISCI handle.
+ * @dev_id:		TISCI device ID.
+ * @subtype:		Resource assignment subtype that is being requested
+ *			from the given device.
+ * @s_host:		Host processor ID to which the resources are allocated
+ * @range_start:	Start index of the resource range
+ * @range_num:		Number of resources in the range
+ *
+ * Return: 0 if all went fine, else return appropriate error.
+ */
+static int ti_sci_get_resource_range(const struct ti_sci_handle *handle,
+				     u32 dev_id, u8 subtype, u8 s_host,
+				     u16 *range_start, u16 *range_num)
+{
+	struct ti_sci_msg_resp_get_resource_range *resp;
+	struct ti_sci_msg_req_get_resource_range *req;
+	struct ti_sci_xfer *xfer;
+	struct ti_sci_info *info;
+	struct device *dev;
+	u16 type;
+	int ret = 0;
+
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
+	if (!handle)
+		return -EINVAL;
+
+	info = handle_to_ti_sci_info(handle);
+	dev = info->dev;
+
+	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_GET_RESOURCE_RANGE,
+				   TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
+				   sizeof(*req), sizeof(*resp));
+	if (IS_ERR(xfer)) {
+		ret = PTR_ERR(xfer);
+		dev_err(dev, "Message alloc failed(%d)\n", ret);
+		return ret;
+	}
+
+	ret = ti_sci_get_resource_type(info, dev_id, &type);
+	if (ret) {
+		dev_err(dev, "rm type lookup failed for %u\n", dev_id);
+		goto fail;
+	}
+
+	req = (struct ti_sci_msg_req_get_resource_range *)xfer->xfer_buf;
+	req->secondary_host = s_host;
+	req->type = type & MSG_RM_RESOURCE_TYPE_MASK;
+	req->subtype = subtype & MSG_RM_RESOURCE_SUBTYPE_MASK;
+
+	ret = ti_sci_do_xfer(info, xfer);
+	if (ret) {
+		dev_err(dev, "Mbox send fail %d\n", ret);
+		goto fail;
+	}
+
+	resp = (struct ti_sci_msg_resp_get_resource_range *)xfer->xfer_buf;
+
+	if (!ti_sci_is_response_ack(resp)) {
+		ret = -ENODEV;
+	} else if (!resp->range_start && !resp->range_num) {
+		ret = -ENODEV;
+	} else {
+		*range_start = resp->range_start;
+		*range_num = resp->range_num;
+	};
+
+fail:
+	ti_sci_put_one_xfer(&info->minfo, xfer);
+
+	return ret;
+}
+
+/**
+ * ti_sci_cmd_get_resource_range - Get a range of resources assigned to host
+ *				   that is same as ti sci interface host.
+ * @handle:		Pointer to TISCI handle.
+ * @dev_id:		TISCI device ID.
+ * @subtype:		Resource assignment subtype that is being requested
+ *			from the given device.
+ * @range_start:	Start index of the resource range
+ * @range_num:		Number of resources in the range
+ *
+ * Return: 0 if all went fine, else return appropriate error.
+ */
+static int ti_sci_cmd_get_resource_range(const struct ti_sci_handle *handle,
+					 u32 dev_id, u8 subtype,
+					 u16 *range_start, u16 *range_num)
+{
+	return ti_sci_get_resource_range(handle, dev_id, subtype,
+					 TI_SCI_IRQ_SECONDARY_HOST_INVALID,
+					 range_start, range_num);
+}
+
+/**
+ * ti_sci_cmd_get_resource_range_from_shost - Get a range of resources
+ *					      assigned to a specified host.
+ * @handle:		Pointer to TISCI handle.
+ * @dev_id:		TISCI device ID.
+ * @subtype:		Resource assignment subtype that is being requested
+ *			from the given device.
+ * @s_host:		Host processor ID to which the resources are allocated
+ * @range_start:	Start index of the resource range
+ * @range_num:		Number of resources in the range
+ *
+ * Return: 0 if all went fine, else return appropriate error.
+ */
+static
+int ti_sci_cmd_get_resource_range_from_shost(const struct ti_sci_handle *handle,
+					     u32 dev_id, u8 subtype, u8 s_host,
+					     u16 *range_start, u16 *range_num)
+{
+	return ti_sci_get_resource_range(handle, dev_id, subtype, s_host,
+					 range_start, range_num);
+}
+
 /*
  * ti_sci_setup_ops() - Setup the operations structures
  * @info:	pointer to TISCI pointer
@@ -1610,6 +1775,7 @@ static void ti_sci_setup_ops(struct ti_sci_info *info)
 	struct ti_sci_core_ops *core_ops = &ops->core_ops;
 	struct ti_sci_dev_ops *dops = &ops->dev_ops;
 	struct ti_sci_clk_ops *cops = &ops->clk_ops;
+	struct ti_sci_rm_core_ops *rm_core_ops = &ops->rm_core_ops;
 
 	core_ops->reboot_device = ti_sci_cmd_core_reboot;
 
@@ -1640,6 +1806,10 @@ static void ti_sci_setup_ops(struct ti_sci_info *info)
 	cops->get_best_match_freq = ti_sci_cmd_clk_get_match_freq;
 	cops->set_freq = ti_sci_cmd_clk_set_freq;
 	cops->get_freq = ti_sci_cmd_clk_get_freq;
+
+	rm_core_ops->get_range = ti_sci_cmd_get_resource_range;
+	rm_core_ops->get_range_from_shost =
+				ti_sci_cmd_get_resource_range_from_shost;
 }
 
 /**
diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
index 12bf316b68df..a043c4762791 100644
--- a/drivers/firmware/ti_sci.h
+++ b/drivers/firmware/ti_sci.h
@@ -35,6 +35,9 @@
 #define TI_SCI_MSG_QUERY_CLOCK_FREQ	0x010d
 #define TI_SCI_MSG_GET_CLOCK_FREQ	0x010e
 
+/* Resource Management Requests */
+#define TI_SCI_MSG_GET_RESOURCE_RANGE	0x1500
+
 /**
  * struct ti_sci_msg_hdr - Generic Message Header for All messages and responses
  * @type:	Type of messages: One of TI_SCI_MSG* values
@@ -461,4 +464,43 @@ struct ti_sci_msg_resp_get_clock_freq {
 	u64 freq_hz;
 } __packed;
 
+#define TI_SCI_IRQ_SECONDARY_HOST_INVALID	0xff
+
+/**
+ * struct ti_sci_msg_req_get_resource_range - Request to get a host's assigned
+ *					      range of resources.
+ * @hdr:		Generic Header
+ * @type:		Unique resource assignment type
+ * @subtype:		Resource assignment subtype within the resource type.
+ * @secondary_host:	Host processing entity to which the resources are
+ *			allocated. This is required only when the destination
+ *			host id id different from ti sci interface host id,
+ *			else TI_SCI_IRQ_SECONDARY_HOST_INVALID can be passed.
+ *
+ * Request type is TI_SCI_MSG_GET_RESOURCE_RANGE. Responded with requested
+ * resource range which is of type TI_SCI_MSG_GET_RESOURCE_RANGE.
+ */
+struct ti_sci_msg_req_get_resource_range {
+	struct ti_sci_msg_hdr hdr;
+#define MSG_RM_RESOURCE_TYPE_MASK	GENMASK(9, 0)
+#define MSG_RM_RESOURCE_SUBTYPE_MASK	GENMASK(5, 0)
+	u16 type;
+	u8 subtype;
+	u8 secondary_host;
+} __packed;
+
+/**
+ * struct ti_sci_msg_resp_get_resource_range - Response to resource get range.
+ * @hdr:		Generic Header
+ * @range_start:	Start index of the resource range.
+ * @range_num:		Number of resources in the range.
+ *
+ * Response to request TI_SCI_MSG_GET_RESOURCE_RANGE.
+ */
+struct ti_sci_msg_resp_get_resource_range {
+	struct ti_sci_msg_hdr hdr;
+	u16 range_start;
+	u16 range_num;
+} __packed;
+
 #endif /* __TI_SCI_H */
diff --git a/include/linux/soc/ti/ti_sci_protocol.h b/include/linux/soc/ti/ti_sci_protocol.h
index 515587e9d373..0c92a922db6a 100644
--- a/include/linux/soc/ti/ti_sci_protocol.h
+++ b/include/linux/soc/ti/ti_sci_protocol.h
@@ -192,15 +192,42 @@ struct ti_sci_clk_ops {
 			u64 *current_freq);
 };
 
+/**
+ * struct ti_sci_rm_core_ops - Resource management core operations
+ * @get_range:		Get a range of resources belonging to ti sci host.
+ * @get_rage_from_shost:	Get a range of resources belonging to
+ *				specified host id.
+ *			- s_host: Host processing entity to which the
+ *				  resources are allocated
+ *
+ * NOTE: for these functions, all the parameters are consolidated and defined
+ * as below:
+ * - handle:	Pointer to TISCI handle as retrieved by *ti_sci_get_handle
+ * - dev_id:	TISCI device ID.
+ * - subtype:	Resource assignment subtype that is being requested
+ *		from the given device.
+ * - range_start:	Start index of the resource range
+ * - range_end:		Number of resources in the range
+ */
+struct ti_sci_rm_core_ops {
+	int (*get_range)(const struct ti_sci_handle *handle, u32 dev_id,
+			 u8 subtype, u16 *range_start, u16 *range_num);
+	int (*get_range_from_shost)(const struct ti_sci_handle *handle,
+				    u32 dev_id, u8 subtype, u8 s_host,
+				    u16 *range_start, u16 *range_num);
+};
+
 /**
  * struct ti_sci_ops - Function support for TI SCI
  * @dev_ops:	Device specific operations
  * @clk_ops:	Clock specific operations
+ * @rm_core_ops:	Resource management core operations.
  */
 struct ti_sci_ops {
 	struct ti_sci_core_ops core_ops;
 	struct ti_sci_dev_ops dev_ops;
 	struct ti_sci_clk_ops clk_ops;
+	struct ti_sci_rm_core_ops rm_core_ops;
 };
 
 /**
-- 
2.21.0

[PATCH v6 03/12] firmware: ti_sci: Add support for IRQ management

[Lokesh Vutla]

TISCI abstracts the handling of IRQ routes where interrupt sources
are not directly connected to host interrupt controller. Add support
for the set of TISCI commands for requesting and releasing IRQs.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
Changes since v5:
- None

 drivers/firmware/ti_sci.c              | 260 +++++++++++++++++++++++++
 drivers/firmware/ti_sci.h              |  60 ++++++
 include/linux/soc/ti/ti_sci_protocol.h |  28 +++
 3 files changed, 348 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 503195223c09..d303f5a14da9 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -1765,6 +1765,260 @@ int ti_sci_cmd_get_resource_range_from_shost(const struct ti_sci_handle *handle,
 					 range_start, range_num);
 }
 
+/**
+ * ti_sci_manage_irq() - Helper api to configure/release the irq route between
+ *			 the requested source and destination
+ * @handle:		Pointer to TISCI handle.
+ * @valid_params:	Bit fields defining the validity of certain params
+ * @src_id:		Device ID of the IRQ source
+ * @src_index:		IRQ source index within the source device
+ * @dst_id:		Device ID of the IRQ destination
+ * @dst_host_irq:	IRQ number of the destination device
+ * @ia_id:		Device ID of the IA, if the IRQ flows through this IA
+ * @vint:		Virtual interrupt to be used within the IA
+ * @global_event:	Global event number to be used for the requesting event
+ * @vint_status_bit:	Virtual interrupt status bit to be used for the event
+ * @s_host:		Secondary host ID to which the irq/event is being
+ *			requested for.
+ * @type:		Request type irq set or release.
+ *
+ * Return: 0 if all went fine, else return appropriate error.
+ */
+static int ti_sci_manage_irq(const struct ti_sci_handle *handle,
+			     u32 valid_params, u16 src_id, u16 src_index,
+			     u16 dst_id, u16 dst_host_irq, u16 ia_id, u16 vint,
+			     u16 global_event, u8 vint_status_bit, u8 s_host,
+			     u16 type)
+{
+	struct ti_sci_msg_req_manage_irq *req;
+	struct ti_sci_msg_hdr *resp;
+	struct ti_sci_xfer *xfer;
+	struct ti_sci_info *info;
+	struct device *dev;
+	int ret = 0;
+
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
+	if (!handle)
+		return -EINVAL;
+
+	info = handle_to_ti_sci_info(handle);
+	dev = info->dev;
+
+	xfer = ti_sci_get_one_xfer(info, type, TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
+				   sizeof(*req), sizeof(*resp));
+	if (IS_ERR(xfer)) {
+		ret = PTR_ERR(xfer);
+		dev_err(dev, "Message alloc failed(%d)\n", ret);
+		return ret;
+	}
+	req = (struct ti_sci_msg_req_manage_irq *)xfer->xfer_buf;
+	req->valid_params = valid_params;
+	req->src_id = src_id;
+	req->src_index = src_index;
+	req->dst_id = dst_id;
+	req->dst_host_irq = dst_host_irq;
+	req->ia_id = ia_id;
+	req->vint = vint;
+	req->global_event = global_event;
+	req->vint_status_bit = vint_status_bit;
+	req->secondary_host = s_host;
+
+	ret = ti_sci_do_xfer(info, xfer);
+	if (ret) {
+		dev_err(dev, "Mbox send fail %d\n", ret);
+		goto fail;
+	}
+
+	resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf;
+
+	ret = ti_sci_is_response_ack(resp) ? 0 : -ENODEV;
+
+fail:
+	ti_sci_put_one_xfer(&info->minfo, xfer);
+
+	return ret;
+}
+
+/**
+ * ti_sci_set_irq() - Helper api to configure the irq route between the
+ *		      requested source and destination
+ * @handle:		Pointer to TISCI handle.
+ * @valid_params:	Bit fields defining the validity of certain params
+ * @src_id:		Device ID of the IRQ source
+ * @src_index:		IRQ source index within the source device
+ * @dst_id:		Device ID of the IRQ destination
+ * @dst_host_irq:	IRQ number of the destination device
+ * @ia_id:		Device ID of the IA, if the IRQ flows through this IA
+ * @vint:		Virtual interrupt to be used within the IA
+ * @global_event:	Global event number to be used for the requesting event
+ * @vint_status_bit:	Virtual interrupt status bit to be used for the event
+ * @s_host:		Secondary host ID to which the irq/event is being
+ *			requested for.
+ *
+ * Return: 0 if all went fine, else return appropriate error.
+ */
+static int ti_sci_set_irq(const struct ti_sci_handle *handle, u32 valid_params,
+			  u16 src_id, u16 src_index, u16 dst_id,
+			  u16 dst_host_irq, u16 ia_id, u16 vint,
+			  u16 global_event, u8 vint_status_bit, u8 s_host)
+{
+	pr_debug("%s: IRQ set with valid_params = 0x%x from src = %d, index = %d, to dst = %d, irq = %d,via ia_id = %d, vint = %d, global event = %d,status_bit = %d\n",
+		 __func__, valid_params, src_id, src_index,
+		 dst_id, dst_host_irq, ia_id, vint, global_event,
+		 vint_status_bit);
+
+	return ti_sci_manage_irq(handle, valid_params, src_id, src_index,
+				 dst_id, dst_host_irq, ia_id, vint,
+				 global_event, vint_status_bit, s_host,
+				 TI_SCI_MSG_SET_IRQ);
+}
+
+/**
+ * ti_sci_free_irq() - Helper api to free the irq route between the
+ *			   requested source and destination
+ * @handle:		Pointer to TISCI handle.
+ * @valid_params:	Bit fields defining the validity of certain params
+ * @src_id:		Device ID of the IRQ source
+ * @src_index:		IRQ source index within the source device
+ * @dst_id:		Device ID of the IRQ destination
+ * @dst_host_irq:	IRQ number of the destination device
+ * @ia_id:		Device ID of the IA, if the IRQ flows through this IA
+ * @vint:		Virtual interrupt to be used within the IA
+ * @global_event:	Global event number to be used for the requesting event
+ * @vint_status_bit:	Virtual interrupt status bit to be used for the event
+ * @s_host:		Secondary host ID to which the irq/event is being
+ *			requested for.
+ *
+ * Return: 0 if all went fine, else return appropriate error.
+ */
+static int ti_sci_free_irq(const struct ti_sci_handle *handle, u32 valid_params,
+			   u16 src_id, u16 src_index, u16 dst_id,
+			   u16 dst_host_irq, u16 ia_id, u16 vint,
+			   u16 global_event, u8 vint_status_bit, u8 s_host)
+{
+	pr_debug("%s: IRQ release with valid_params = 0x%x from src = %d, index = %d, to dst = %d, irq = %d,via ia_id = %d, vint = %d, global event = %d,status_bit = %d\n",
+		 __func__, valid_params, src_id, src_index,
+		 dst_id, dst_host_irq, ia_id, vint, global_event,
+		 vint_status_bit);
+
+	return ti_sci_manage_irq(handle, valid_params, src_id, src_index,
+				 dst_id, dst_host_irq, ia_id, vint,
+				 global_event, vint_status_bit, s_host,
+				 TI_SCI_MSG_FREE_IRQ);
+}
+
+/**
+ * ti_sci_cmd_set_irq() - Configure a host irq route between the requested
+ *			  source and destination.
+ * @handle:		Pointer to TISCI handle.
+ * @src_id:		Device ID of the IRQ source
+ * @src_index:		IRQ source index within the source device
+ * @dst_id:		Device ID of the IRQ destination
+ * @dst_host_irq:	IRQ number of the destination device
+ * @vint_irq:		Boolean specifying if this interrupt belongs to
+ *			Interrupt Aggregator.
+ *
+ * Return: 0 if all went fine, else return appropriate error.
+ */
+static int ti_sci_cmd_set_irq(const struct ti_sci_handle *handle, u16 src_id,
+			      u16 src_index, u16 dst_id, u16 dst_host_irq,
+			      bool vint_irq)
+{
+	u32 valid_params = MSG_FLAG_DST_ID_VALID | MSG_FLAG_DST_HOST_IRQ_VALID;
+
+	if (vint_irq) {
+		valid_params |= MSG_FLAG_IA_ID_VALID | MSG_FLAG_VINT_VALID;
+		return ti_sci_set_irq(handle, valid_params, 0, 0, dst_id,
+				      dst_host_irq, src_id, src_index, 0, 0, 0);
+	} else {
+		return ti_sci_set_irq(handle, valid_params, src_id, src_index,
+				      dst_id, dst_host_irq, 0, 0, 0, 0, 0);
+	}
+}
+
+/**
+ * ti_sci_cmd_set_event_map() - Configure an event based irq route between the
+ *				requested source and Interrupt Aggregator.
+ * @handle:		Pointer to TISCI handle.
+ * @src_id:		Device ID of the IRQ source
+ * @src_index:		IRQ source index within the source device
+ * @ia_id:		Device ID of the IA, if the IRQ flows through this IA
+ * @vint:		Virtual interrupt to be used within the IA
+ * @global_event:	Global event number to be used for the requesting event
+ * @vint_status_bit:	Virtual interrupt status bit to be used for the event
+ *
+ * Return: 0 if all went fine, else return appropriate error.
+ */
+static int ti_sci_cmd_set_event_map(const struct ti_sci_handle *handle,
+				    u16 src_id, u16 src_index, u16 ia_id,
+				    u16 vint, u16 global_event,
+				    u8 vint_status_bit)
+{
+	u32 valid_params = MSG_FLAG_IA_ID_VALID | MSG_FLAG_VINT_VALID |
+			   MSG_FLAG_GLB_EVNT_VALID |
+			   MSG_FLAG_VINT_STS_BIT_VALID;
+
+	return ti_sci_set_irq(handle, valid_params, src_id, src_index, 0, 0,
+			      ia_id, vint, global_event, vint_status_bit, 0);
+}
+
+/**
+ * ti_sci_cmd_free_irq() - Free a host irq route between the between the
+ *			   requested source and destination.
+ * @handle:		Pointer to TISCI handle.
+ * @src_id:		Device ID of the IRQ source
+ * @src_index:		IRQ source index within the source device
+ * @dst_id:		Device ID of the IRQ destination
+ * @dst_host_irq:	IRQ number of the destination device
+ * @vint_irq:		Boolean specifying if this interrupt belongs to
+ *			Interrupt Aggregator.
+ *
+ * Return: 0 if all went fine, else return appropriate error.
+ */
+static int ti_sci_cmd_free_irq(const struct ti_sci_handle *handle, u16 src_id,
+			       u16 src_index, u16 dst_id, u16 dst_host_irq,
+			       bool vint_irq)
+{
+	u32 valid_params = MSG_FLAG_DST_ID_VALID | MSG_FLAG_DST_HOST_IRQ_VALID;
+
+	if (vint_irq) {
+		valid_params |= MSG_FLAG_IA_ID_VALID | MSG_FLAG_VINT_VALID;
+		return ti_sci_free_irq(handle, valid_params, 0, 0, dst_id,
+				       dst_host_irq, src_id, src_index, 0, 0,
+				       0);
+	} else {
+		return ti_sci_free_irq(handle, valid_params, src_id, src_index,
+				       dst_id, dst_host_irq, 0, 0, 0, 0, 0);
+	}
+}
+
+/**
+ * ti_sci_cmd_free_event_map() - Free an event map between the requested source
+ *				 and Interrupt Aggregator.
+ * @handle:		Pointer to TISCI handle.
+ * @src_id:		Device ID of the IRQ source
+ * @src_index:		IRQ source index within the source device
+ * @ia_id:		Device ID of the IA, if the IRQ flows through this IA
+ * @vint:		Virtual interrupt to be used within the IA
+ * @global_event:	Global event number to be used for the requesting event
+ * @vint_status_bit:	Virtual interrupt status bit to be used for the event
+ *
+ * Return: 0 if all went fine, else return appropriate error.
+ */
+static int ti_sci_cmd_free_event_map(const struct ti_sci_handle *handle,
+				     u16 src_id, u16 src_index, u16 ia_id,
+				     u16 vint, u16 global_event,
+				     u8 vint_status_bit)
+{
+	u32 valid_params = MSG_FLAG_IA_ID_VALID |
+			   MSG_FLAG_VINT_VALID | MSG_FLAG_GLB_EVNT_VALID |
+			   MSG_FLAG_VINT_STS_BIT_VALID;
+
+	return ti_sci_free_irq(handle, valid_params, src_id, src_index, 0, 0,
+			       ia_id, vint, global_event, vint_status_bit, 0);
+}
+
 /*
  * ti_sci_setup_ops() - Setup the operations structures
  * @info:	pointer to TISCI pointer
@@ -1776,6 +2030,7 @@ static void ti_sci_setup_ops(struct ti_sci_info *info)
 	struct ti_sci_dev_ops *dops = &ops->dev_ops;
 	struct ti_sci_clk_ops *cops = &ops->clk_ops;
 	struct ti_sci_rm_core_ops *rm_core_ops = &ops->rm_core_ops;
+	struct ti_sci_rm_irq_ops *iops = &ops->rm_irq_ops;
 
 	core_ops->reboot_device = ti_sci_cmd_core_reboot;
 
@@ -1810,6 +2065,11 @@ static void ti_sci_setup_ops(struct ti_sci_info *info)
 	rm_core_ops->get_range = ti_sci_cmd_get_resource_range;
 	rm_core_ops->get_range_from_shost =
 				ti_sci_cmd_get_resource_range_from_shost;
+
+	iops->set_irq = ti_sci_cmd_set_irq;
+	iops->set_event_map = ti_sci_cmd_set_event_map;
+	iops->free_irq = ti_sci_cmd_free_irq;
+	iops->free_event_map = ti_sci_cmd_free_event_map;
 }
 
 /**
diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
index a043c4762791..4983827151bf 100644
--- a/drivers/firmware/ti_sci.h
+++ b/drivers/firmware/ti_sci.h
@@ -38,6 +38,10 @@
 /* Resource Management Requests */
 #define TI_SCI_MSG_GET_RESOURCE_RANGE	0x1500
 
+/* IRQ requests */
+#define TI_SCI_MSG_SET_IRQ		0x1000
+#define TI_SCI_MSG_FREE_IRQ		0x1001
+
 /**
  * struct ti_sci_msg_hdr - Generic Message Header for All messages and responses
  * @type:	Type of messages: One of TI_SCI_MSG* values
@@ -503,4 +507,60 @@ struct ti_sci_msg_resp_get_resource_range {
 	u16 range_num;
 } __packed;
 
+/**
+ * struct ti_sci_msg_req_manage_irq - Request to configure/release the route
+ *					between the dev and the host.
+ * @hdr:		Generic Header
+ * @valid_params:	Bit fields defining the validity of interrupt source
+ *			parameters. If a bit is not set, then corresponding
+ *			field is not valid and will not be used for route set.
+ *			Bit field definitions:
+ *			0 - Valid bit for @dst_id
+ *			1 - Valid bit for @dst_host_irq
+ *			2 - Valid bit for @ia_id
+ *			3 - Valid bit for @vint
+ *			4 - Valid bit for @global_event
+ *			5 - Valid bit for @vint_status_bit_index
+ *			31 - Valid bit for @secondary_host
+ * @src_id:		IRQ source peripheral ID.
+ * @src_index:		IRQ source index within the peripheral
+ * @dst_id:		IRQ Destination ID. Based on the architecture it can be
+ *			IRQ controller or host processor ID.
+ * @dst_host_irq:	IRQ number of the destination host IRQ controller
+ * @ia_id:		Device ID of the interrupt aggregator in which the
+ *			vint resides.
+ * @vint:		Virtual interrupt number if the interrupt route
+ *			is through an interrupt aggregator.
+ * @global_event:	Global event that is to be mapped to interrupt
+ *			aggregator virtual interrupt status bit.
+ * @vint_status_bit:	Virtual interrupt status bit if the interrupt route
+ *			utilizes an interrupt aggregator status bit.
+ * @secondary_host:	Host ID of the IRQ destination computing entity. This is
+ *			required only when destination host id is different
+ *			from ti sci interface host id.
+ *
+ * Request type is TI_SCI_MSG_SET/RELEASE_IRQ.
+ * Response is generic ACK / NACK message.
+ */
+struct ti_sci_msg_req_manage_irq {
+	struct ti_sci_msg_hdr hdr;
+#define MSG_FLAG_DST_ID_VALID			TI_SCI_MSG_FLAG(0)
+#define MSG_FLAG_DST_HOST_IRQ_VALID		TI_SCI_MSG_FLAG(1)
+#define MSG_FLAG_IA_ID_VALID			TI_SCI_MSG_FLAG(2)
+#define MSG_FLAG_VINT_VALID			TI_SCI_MSG_FLAG(3)
+#define MSG_FLAG_GLB_EVNT_VALID			TI_SCI_MSG_FLAG(4)
+#define MSG_FLAG_VINT_STS_BIT_VALID		TI_SCI_MSG_FLAG(5)
+#define MSG_FLAG_SHOST_VALID			TI_SCI_MSG_FLAG(31)
+	u32 valid_params;
+	u16 src_id;
+	u16 src_index;
+	u16 dst_id;
+	u16 dst_host_irq;
+	u16 ia_id;
+	u16 vint;
+	u16 global_event;
+	u8 vint_status_bit;
+	u8 secondary_host;
+} __packed;
+
 #endif /* __TI_SCI_H */
diff --git a/include/linux/soc/ti/ti_sci_protocol.h b/include/linux/soc/ti/ti_sci_protocol.h
index 0c92a922db6a..a0216c947acd 100644
--- a/include/linux/soc/ti/ti_sci_protocol.h
+++ b/include/linux/soc/ti/ti_sci_protocol.h
@@ -217,17 +217,45 @@ struct ti_sci_rm_core_ops {
 				    u16 *range_start, u16 *range_num);
 };
 
+/**
+ * struct ti_sci_rm_irq_ops: IRQ management operations
+ * @set_irq:		Set an IRQ route between the requested source
+ *			and destination
+ * @set_event_map:	Set an Event based peripheral irq to Interrupt
+ *			Aggregator.
+ * @free_irq:		Free an an IRQ route between the requested source
+ *			destination.
+ * @free_event_map:	Free an event based peripheral irq to Interrupt
+ *			Aggregator.
+ */
+struct ti_sci_rm_irq_ops {
+	int (*set_irq)(const struct ti_sci_handle *handle, u16 src_id,
+		       u16 src_index, u16 dst_id, u16 dst_host_irq,
+		       bool vint_irq);
+	int (*set_event_map)(const struct ti_sci_handle *handle, u16 src_id,
+			     u16 src_index, u16 ia_id, u16 vint,
+			     u16 global_event, u8 vint_status_bit);
+	int (*free_irq)(const struct ti_sci_handle *handle, u16 src_id,
+			u16 src_index, u16 dst_id, u16 dst_host_irq,
+			bool vint_irq);
+	int (*free_event_map)(const struct ti_sci_handle *handle, u16 src_id,
+			      u16 src_index, u16 ia_id, u16 vint,
+			      u16 global_event, u8 vint_status_bit);
+};
+
 /**
  * struct ti_sci_ops - Function support for TI SCI
  * @dev_ops:	Device specific operations
  * @clk_ops:	Clock specific operations
  * @rm_core_ops:	Resource management core operations.
+ * @rm_irq_ops:		IRQ management specific operations
  */
 struct ti_sci_ops {
 	struct ti_sci_core_ops core_ops;
 	struct ti_sci_dev_ops dev_ops;
 	struct ti_sci_clk_ops clk_ops;
 	struct ti_sci_rm_core_ops rm_core_ops;
+	struct ti_sci_rm_irq_ops rm_irq_ops;
 };
 
 /**
-- 
2.21.0

[PATCH v6 04/12] firmware: ti_sci: Add RM mapping table for am654

[Lokesh Vutla]

From: Peter Ujfalusi <peter.ujfalusi@ti.com>

Add the resource mapping table for AM654 SoC as defined in
http://downloads.ti.com/tisci/esd/latest/5_soc_doc/am6x/resasg_types.html
Introduce a new compatible for AM654 "ti,am654-sci" for using
this resource map table.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
Changes since v4:
- None

 .../bindings/arm/keystone/ti,sci.txt          |  3 ++-
 .../bindings/arm/keystone/ti,sci.txt          |  3 ++-
 drivers/firmware/ti_sci.c                     | 23 +++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt b/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
index b56a02c10ae6..6f0cd31c1520 100644
--- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
+++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
@@ -24,7 +24,8 @@ relationship between the TI-SCI parent node to the child node.
 
 Required properties:
 -------------------
-- compatible: should be "ti,k2g-sci"
+- compatible:	should be "ti,k2g-sci" for TI 66AK2G SoC
+		should be "ti,am654-sci" for for TI AM654 SoC
 - mbox-names:
 	"rx" - Mailbox corresponding to receive path
 	"tx" - Mailbox corresponding to transmit path
diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index d303f5a14da9..88e461498def 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -2297,10 +2297,33 @@ static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
 	/* Limited by MBOX_TX_QUEUE_LEN. K2G can handle upto 128 messages! */
 	.max_msgs = 20,
 	.max_msg_size = 64,
+	.rm_type_map = NULL,
+};
+
+static struct ti_sci_rm_type_map ti_sci_am654_rm_type_map[] = {
+	{.dev_id = 56, .type = 0x00b}, /* GIC_IRQ */
+	{.dev_id = 179, .type = 0x000}, /* MAIN_NAV_UDMASS_IA0 */
+	{.dev_id = 187, .type = 0x009}, /* MAIN_NAV_RA */
+	{.dev_id = 188, .type = 0x006}, /* MAIN_NAV_UDMAP */
+	{.dev_id = 194, .type = 0x007}, /* MCU_NAV_UDMAP */
+	{.dev_id = 195, .type = 0x00a}, /* MCU_NAV_RA */
+	{.dev_id = 0, .type = 0x000}, /* end of table */
+};
+
+/* Description for AM654 */
+static const struct ti_sci_desc ti_sci_pmmc_am654_desc = {
+	.default_host_id = 12,
+	/* Conservative duration */
+	.max_rx_timeout_ms = 10000,
+	/* Limited by MBOX_TX_QUEUE_LEN. K2G can handle upto 128 messages! */
+	.max_msgs = 20,
+	.max_msg_size = 60,
+	.rm_type_map = ti_sci_am654_rm_type_map,
 };
 
 static const struct of_device_id ti_sci_of_match[] = {
 	{.compatible = "ti,k2g-sci", .data = &ti_sci_pmmc_k2g_desc},
+	{.compatible = "ti,am654-sci", .data = &ti_sci_pmmc_am654_desc},
 	{ /* Sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, ti_sci_of_match);
-- 
2.21.0

[PATCH v6 05/12] firmware: ti_sci: Add helper apis to manage resources

[Lokesh Vutla]

Each resource with in the device can be uniquely identified as defined
by TISCI. Since this is generic across the devices, resource allocation
also can be made generic instead of each client driver handling the
resource. So add helper apis to manage the resource.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
Changes since v5:
- New patch. Abstracted out resource managemented into firmware driver after
 discussing with Nishanth and Marc.

 drivers/firmware/ti_sci.c              | 130 +++++++++++++++++++++++++
 include/linux/soc/ti/ti_sci_protocol.h |  54 ++++++++++
 2 files changed, 184 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 88e461498def..e4b01cfc5883 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -2277,6 +2277,136 @@ const struct ti_sci_handle *devm_ti_sci_get_by_phandle(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_ti_sci_get_by_phandle);
 
+/**
+ * ti_sci_get_free_resource() - Get a free resource from TISCI resource.
+ * @res:	Pointer to the TISCI resource
+ *
+ * Return: resource num if all went ok else TI_SCI_RESOURCE_NULL.
+ */
+u16 ti_sci_get_free_resource(struct ti_sci_resource *res)
+{
+	unsigned long flags;
+	u16 set, free_bit;
+
+	raw_spin_lock_irqsave(&res->lock, flags);
+	for (set = 0; set < res->sets; set++) {
+		free_bit = find_first_zero_bit(res->desc[set].res_map,
+					       res->desc[set].num);
+		if (free_bit != res->desc[set].num) {
+			set_bit(free_bit, res->desc[set].res_map);
+			raw_spin_unlock_irqrestore(&res->lock, flags);
+			return res->desc[set].start + free_bit;
+		}
+	}
+	raw_spin_unlock_irqrestore(&res->lock, flags);
+
+	return TI_SCI_RESOURCE_NULL;
+}
+EXPORT_SYMBOL_GPL(ti_sci_get_free_resource);
+
+/**
+ * ti_sci_release_resource() - Release a resource from TISCI resource.
+ * @res:	Pointer to the TISCI resource
+ * @id:		Resource id to be released.
+ */
+void ti_sci_release_resource(struct ti_sci_resource *res, u16 id)
+{
+	unsigned long flags;
+	u16 set;
+
+	raw_spin_lock_irqsave(&res->lock, flags);
+	for (set = 0; set < res->sets; set++) {
+		if (res->desc[set].start <= id &&
+		    (res->desc[set].num + res->desc[set].start) > id)
+			clear_bit(id - res->desc[set].start,
+				  res->desc[set].res_map);
+	}
+	raw_spin_unlock_irqrestore(&res->lock, flags);
+}
+EXPORT_SYMBOL_GPL(ti_sci_release_resource);
+
+/**
+ * ti_sci_get_num_resources() - Get the number of resources in TISCI resource
+ * @res:	Pointer to the TISCI resource
+ *
+ * Return: Total number of available resources.
+ */
+u32 ti_sci_get_num_resources(struct ti_sci_resource *res)
+{
+	u32 set, count = 0;
+
+	for (set = 0; set < res->sets; set++)
+		count += res->desc[set].num;
+
+	return count;
+}
+EXPORT_SYMBOL_GPL(ti_sci_get_num_resources);
+
+/**
+ * devm_ti_sci_get_of_resource() - Get a TISCI resource assigned to a device
+ * @handle:	TISCI handle
+ * @dev:	Device pointer to which the resource is assigned
+ * @dev_id:	TISCI device id to which the resource is assigned
+ * @of_prop:	property name by which the resource are represented
+ *
+ * Return: Pointer to ti_sci_resource if all went well else appropriate
+ *	   error pointer.
+ */
+struct ti_sci_resource *
+devm_ti_sci_get_of_resource(const struct ti_sci_handle *handle,
+			    struct device *dev, u32 dev_id, char *of_prop)
+{
+	struct ti_sci_resource *res;
+	u32 resource_subtype;
+	int i, ret;
+
+	res = devm_kzalloc(dev, sizeof(*res), GFP_KERNEL);
+	if (!res)
+		return ERR_PTR(-ENOMEM);
+
+	res->sets = of_property_count_elems_of_size(dev_of_node(dev), of_prop,
+						    sizeof(u32));
+	if (res->sets < 0) {
+		dev_err(dev, "%s resource type ids not available\n", of_prop);
+		return ERR_PTR(res->sets);
+	}
+
+	res->desc = devm_kcalloc(dev, res->sets, sizeof(*res->desc),
+				 GFP_KERNEL);
+	if (!res->desc)
+		return ERR_PTR(-ENOMEM);
+
+	for (i = 0; i < res->sets; i++) {
+		ret = of_property_read_u32_index(dev_of_node(dev), of_prop, i,
+						 &resource_subtype);
+		if (ret)
+			return ERR_PTR(-EINVAL);
+
+		ret = handle->ops.rm_core_ops.get_range(handle, dev_id,
+							resource_subtype,
+							&res->desc[i].start,
+							&res->desc[i].num);
+		if (ret) {
+			dev_err(dev, "dev = %d subtype %d not allocated for this host\n",
+				dev_id, resource_subtype);
+			return ERR_PTR(ret);
+		}
+
+		dev_dbg(dev, "dev = %d, subtype = %d, start = %d, num = %d\n",
+			dev_id, resource_subtype, res->desc[i].start,
+			res->desc[i].num);
+
+		res->desc[i].res_map =
+			devm_kzalloc(dev, BITS_TO_LONGS(res->desc[i].num) *
+				     sizeof(*res->desc[i].res_map), GFP_KERNEL);
+		if (!res->desc[i].res_map)
+			return ERR_PTR(-ENOMEM);
+	}
+	raw_spin_lock_init(&res->lock);
+
+	return res;
+}
+
 static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
 				void *cmd)
 {
diff --git a/include/linux/soc/ti/ti_sci_protocol.h b/include/linux/soc/ti/ti_sci_protocol.h
index a0216c947acd..277d98fab505 100644
--- a/include/linux/soc/ti/ti_sci_protocol.h
+++ b/include/linux/soc/ti/ti_sci_protocol.h
@@ -268,6 +268,33 @@ struct ti_sci_handle {
 	struct ti_sci_ops ops;
 };
 
+#define TI_SCI_RESOURCE_NULL	0xffff
+
+/**
+ * struct ti_sci_resource_desc - Description of TI SCI resource instance range.
+ * @start:	Start index of the resource.
+ * @num:	Number of resources.
+ * @res_map:	Bitmap to manage the allocation of these resources.
+ */
+struct ti_sci_resource_desc {
+	u16 start;
+	u16 num;
+	unsigned long *res_map;
+};
+
+/**
+ * struct ti_sci_resource - Structure representing a resource assigned
+ *			    to a device.
+ * @sets:	Number of sets available from this resource type
+ * @lock:	Lock to guard the res map in each set.
+ * @desc:	Array of resource descriptors.
+ */
+struct ti_sci_resource {
+	u16 sets;
+	raw_spinlock_t lock;
+	struct ti_sci_resource_desc *desc;
+};
+
 #if IS_ENABLED(CONFIG_TI_SCI_PROTOCOL)
 const struct ti_sci_handle *ti_sci_get_handle(struct device *dev);
 int ti_sci_put_handle(const struct ti_sci_handle *handle);
@@ -276,6 +303,12 @@ const struct ti_sci_handle *ti_sci_get_by_phandle(struct device_node *np,
 						  const char *property);
 const struct ti_sci_handle *devm_ti_sci_get_by_phandle(struct device *dev,
 						       const char *property);
+u16 ti_sci_get_free_resource(struct ti_sci_resource *res);
+void ti_sci_release_resource(struct ti_sci_resource *res, u16 id);
+u32 ti_sci_get_num_resources(struct ti_sci_resource *res);
+struct ti_sci_resource *
+devm_ti_sci_get_of_resource(const struct ti_sci_handle *handle,
+			    struct device *dev, u32 dev_id, char *of_prop);
 
 #else	/* CONFIG_TI_SCI_PROTOCOL */
 
@@ -308,6 +341,27 @@ const struct ti_sci_handle *devm_ti_sci_get_by_phandle(struct device *dev,
 {
 	return ERR_PTR(-EINVAL);
 }
+
+static inline u16 ti_sci_get_free_resource(struct ti_sci_resource *res)
+{
+	return TI_SCI_RESOURCE_NULL;
+}
+
+static inline void ti_sci_release_resource(struct ti_sci_resource *res, u16 id)
+{
+}
+
+static inline u32 ti_sci_get_num_resources(struct ti_sci_resource *res)
+{
+	return 0;
+}
+
+static inline struct ti_sci_resource *
+devm_ti_sci_get_of_resource(const struct ti_sci_handle *handle,
+			    struct device *dev, u32 dev_id, char *of_prop)
+{
+	return ERR_PTR(-EINVAL);
+}
 #endif	/* CONFIG_TI_SCI_PROTOCOL */
 
 #endif	/* __TISCI_PROTOCOL_H */
-- 
2.21.0

[PATCH v6 06/12] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings

[Lokesh Vutla]

Add the DT binding documentation for Interrupt router driver.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
Changes since v5:
- Introduced a new property for specifying router trigger type.
- Dropped the trigger type from interrupt cells property.

Marc,
	Firmware change to not differentiate INTA interrupts and peripheral
	interrupts might take couple more weeks, but it is going to come.
	I am posting this series to get a review on the INTA driver.

 .../interrupt-controller/ti,sci-intr.txt      | 83 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 84 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
new file mode 100644
index 000000000000..613911013db1
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
@@ -0,0 +1,83 @@
+Texas Instruments K3 Interrupt Router
+=====================================
+
+The Interrupt Router (INTR) module provides a mechanism to mux M
+interrupt inputs to N interrupt outputs, where all M inputs are selectable
+to be driven per N output. An Interrupt Router can either handle edge triggered
+or level triggered interrupts and that is fixed in hardware.
+
+                                 Interrupt Router
+                             +----------------------+
+                             |  Inputs     Outputs  |
+        +-------+            | +------+             |
+        | GPIO  |----------->| | irq0 |             |       Host IRQ
+        +-------+            | +------+             |      controller
+                             |    .        +-----+  |      +-------+
+        +-------+            |    .        |  0  |  |----->|  IRQ  |
+        | INTA  |----------->|    .        +-----+  |      +-------+
+        +-------+            |    .          .      |
+                             | +------+      .      |
+                             | | irqM |    +-----+  |
+                             | +------+    |  N  |  |
+                             |             +-----+  |
+                             +----------------------+
+
+There is one register per output (MUXCNTL_N) that controls the selection.
+Configuration of these MUXCNTL_N registers is done by a system controller
+(like the Device Memory and Security Controller on K3 AM654 SoC). System
+controller will keep track of the used and unused registers within the Router.
+Driver should request the system controller to get the range of GIC IRQs
+assigned to the requesting hosts. It is the drivers responsibility to keep
+track of Host IRQs.
+
+Communication between the host processor running an OS and the system
+controller happens through a protocol called TI System Control Interface
+(TISCI protocol). For more details refer:
+Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
+
+TISCI Interrupt Router Node:
+----------------------------
+Required Properties:
+- compatible:		Must be "ti,sci-intr".
+- ti,intr-trigger-type:	Should be 1 if the interrupt router supports edge
+			triggered interrupts, 4 for 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 3.
+			First cell should contain the TISCI device ID of source
+			Second cell should contain the interrupt source offset
+			within the device.
+			Third cell should be 1 if the irq is coming from the
+			interrupt aggregator else 0.
+- 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.
+
+For more details on TISCI IRQ resource management refer:
+http://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/rm/rm_irq.html
+
+Example:
+--------
+The following example demonstrates both interrupt router node and the consumer
+node(main gpio) on the AM654 SoC:
+
+main_intr: interrupt-controller0 {
+	compatible = "ti,sci-intr";
+	ti,intr-trigger-type = <1>;
+	interrupt-controller;
+	interrupt-parent = <&gic500>;
+	#interrupt-cells = <3>;
+	ti,sci = <&dmsc>;
+	ti,sci-dst-id = <56>;
+	ti,sci-rm-range-girq = <0x1>;
+};
+
+main_gpio0: gpio@600000 {
+	...
+	interrupt-parent = <&main_intr>;
+	interrupts = <57 256 0>, <57 257 0>, <57 258 0>,
+		     <57 259 0>, <57 260 0>, <57 261 0>;
+	...
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 2359e12e4c41..9c4e71187ca1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15349,6 +15349,7 @@ F:	Documentation/devicetree/bindings/reset/ti,sci-reset.txt
 F:	Documentation/devicetree/bindings/clock/ti,sci-clk.txt
 F:	drivers/clk/keystone/sci-clk.c
 F:	drivers/reset/reset-ti-sci.c
+F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
 
 Texas Instruments ASoC drivers
 M:	Peter Ujfalusi <peter.ujfalusi@ti.com>
-- 
2.21.0

[PATCH v6 07/12] irqchip: ti-sci-intr: Add support for Interrupt Router driver

[Lokesh Vutla]

Texas Instruments' K3 generation SoCs has an IP Interrupt Router
that does allows for redirection of input interrupts to host
interrupt controller. Interrupt Router inputs are either from a
peripheral or from an Interrupt Aggregator which is another
interrupt controller.

Configuration of the interrupt router registers can only be done by
a system co-processor and the driver needs to send a message to this
co processor over TISCI protocol.

Add support for Interrupt Router driver over TISCI protocol.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
Changes since v5:
- Updated to latest bindings

 MAINTAINERS                       |   1 +
 drivers/irqchip/Kconfig           |  11 ++
 drivers/irqchip/Makefile          |   1 +
 drivers/irqchip/irq-ti-sci-intr.c | 289 ++++++++++++++++++++++++++++++
 4 files changed, 302 insertions(+)
 create mode 100644 drivers/irqchip/irq-ti-sci-intr.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9c4e71187ca1..6f551053627f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15350,6 +15350,7 @@ F:	Documentation/devicetree/bindings/clock/ti,sci-clk.txt
 F:	drivers/clk/keystone/sci-clk.c
 F:	drivers/reset/reset-ti-sci.c
 F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
+F:	drivers/irqchip/irq-ti-sci-intr.c
 
 Texas Instruments ASoC drivers
 M:	Peter Ujfalusi <peter.ujfalusi@ti.com>
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 5438abb1baba..a1ff64c1d40d 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -425,6 +425,17 @@ config LS1X_IRQ
 	help
 	  Support for the Loongson-1 platform Interrupt Controller.
 
+config TI_SCI_INTR_IRQCHIP
+	bool
+	depends on TI_SCI_PROTOCOL && ARCH_K3
+	select IRQ_DOMAIN
+	select IRQ_DOMAIN_HIERARCHY
+	help
+	  This enables the irqchip driver support for K3 Interrupt router
+	  over TI System Control Interface available on some new TI's SoCs.
+	  If you wish to use interrupt router irq resources managed by the
+	  TI System Controller, say Y here. Otherwise, say N.
+
 endmenu
 
 config SIFIVE_PLIC
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 85972ae1bd7f..fa5c865788b5 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -97,3 +97,4 @@ obj-$(CONFIG_SIFIVE_PLIC)		+= irq-sifive-plic.o
 obj-$(CONFIG_IMX_IRQSTEER)		+= irq-imx-irqsteer.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-ti-sci-intr.c b/drivers/irqchip/irq-ti-sci-intr.c
new file mode 100644
index 000000000000..03d0e201153c
--- /dev/null
+++ b/drivers/irqchip/irq-ti-sci-intr.c
@@ -0,0 +1,289 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Texas Instruments' K3 Interrupt Router irqchip driver
+ *
+ * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
+ *	Lokesh Vutla <lokeshvutla@ti.com>
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/irqchip.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/irqdomain.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.
+ * @num_inta:	No. of Interrupt Aggregators attached to this Interrupt Router
+ * @inta_ids:	List of Interrupt Aggregator IDs.
+ * @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;
+	int num_inta;
+	u32 *inta_ids;
+	u32 type;
+};
+
+static struct irq_chip ti_sci_intr_irq_chip = {
+	.name			= "INTR",
+	.irq_eoi		= irq_chip_eoi_parent,
+	.irq_mask		= irq_chip_mask_parent,
+	.irq_unmask		= irq_chip_unmask_parent,
+	.irq_retrigger		= irq_chip_retrigger_hierarchy,
+	.irq_set_type		= irq_chip_set_type_parent,
+	.irq_set_affinity	= irq_chip_set_affinity_parent,
+};
+
+/**
+ * ti_sci_intr_irq_domain_translate() - Retrieve hwirq and type from
+ *					IRQ firmware specific handler.
+ * @domain:	Pointer to IRQ domain
+ * @fwspec:	Pointer to IRQ specific firmware structure
+ * @hwirq:	IRQ number identified by hardware
+ * @type:	IRQ type
+ *
+ * Return 0 if all went ok else appropriate error.
+ */
+static int ti_sci_intr_irq_domain_translate(struct irq_domain *domain,
+					    struct irq_fwspec *fwspec,
+					    unsigned long *hwirq,
+					    unsigned int *type)
+{
+	struct ti_sci_intr_irq_domain *intr = domain->host_data;
+
+	if (fwspec->param_count != 3)
+		return -EINVAL;
+
+	*hwirq = TO_HWIRQ(fwspec->param[0], fwspec->param[1]);
+	*type = intr->type;
+
+	/* Third parameter can be either 0 or 1 */
+	if (fwspec->param[2] != 0 && fwspec->param[2] != 1)
+		return -EINVAL;
+
+	return 0;
+}
+
+/**
+ * ti_sci_intr_irq_domain_free() - Free the specified IRQs from the domain.
+ * @domain:	Domain to which the irqs belong
+ * @virq:	Linux virtual IRQ to be freed.
+ * @nr_irqs:	Number of continuous irqs to be freed
+ */
+static void ti_sci_intr_irq_domain_free(struct irq_domain *domain,
+					unsigned int virq, unsigned int nr_irqs)
+{
+	struct ti_sci_intr_irq_domain *intr = domain->host_data;
+	struct irq_data *data, *parent_data;
+	u16 dev_id, irq_index;
+	u8 flags;
+
+	parent_data = irq_domain_get_irq_data(domain->parent, virq);
+	data = irq_domain_get_irq_data(domain, virq);
+	flags = (uintptr_t)irq_data_get_irq_chip_data(data);
+	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,
+					   flags);
+	ti_sci_release_resource(intr->dst_irq, parent_data->hwirq);
+	irq_domain_free_irqs_parent(domain, virq, 1);
+	irq_domain_reset_irq_data(data);
+}
+
+/**
+ * ti_sci_intr_alloc_gic_irq() - Allocate GIC specific 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
+ * @flags:	Corresponding flags to the IRQ
+ *
+ * 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, u8 flags)
+{
+	struct ti_sci_intr_irq_domain *intr = domain->host_data;
+	struct irq_fwspec fwspec;
+	u16 dev_id, irq_index;
+	u16 dst_irq;
+	int err;
+
+	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)
+		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;
+
+	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, flags);
+	if (err)
+		goto err_msg;
+
+	return 0;
+
+err_msg:
+	irq_domain_free_irqs_parent(domain, virq, 1);
+err_irqs:
+	ti_sci_release_resource(intr->dst_irq, dst_irq);
+	return err;
+}
+
+/**
+ * ti_sci_intr_irq_domain_alloc() - Allocate Interrupt router IRQs
+ * @domain:	Point to the interrupt router IRQ domain
+ * @virq:	Corresponding Linux virtual IRQ number
+ * @nr_irqs:	Continuous irqs to be allocated
+ * @data:	Pointer to firmware specifier
+ *
+ * Return 0 if all went well else appropriate error value.
+ */
+static int ti_sci_intr_irq_domain_alloc(struct irq_domain *domain,
+					unsigned int virq, unsigned int nr_irqs,
+					void *data)
+{
+	struct irq_fwspec *fwspec = data;
+	unsigned long hwirq;
+	unsigned int flags;
+	int err;
+
+	err = ti_sci_intr_irq_domain_translate(domain, fwspec, &hwirq, &flags);
+	if (err)
+		return err;
+
+	flags = fwspec->param[2];
+	err = ti_sci_intr_alloc_gic_irq(domain, virq, hwirq, flags);
+	if (err)
+		return err;
+
+	irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
+				      &ti_sci_intr_irq_chip,
+				      (void *)(uintptr_t)flags);
+
+	return 0;
+}
+
+static const struct irq_domain_ops ti_sci_intr_irq_domain_ops = {
+	.alloc		= ti_sci_intr_irq_domain_alloc,
+	.free		= ti_sci_intr_irq_domain_free,
+	.translate	= ti_sci_intr_irq_domain_translate,
+};
+
+static int ti_sci_intr_irq_domain_probe(struct platform_device *pdev)
+{
+	struct irq_domain *parent_domain, *domain;
+	struct ti_sci_intr_irq_domain *intr;
+	struct device_node *parent_node;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	parent_node = of_irq_find_parent(dev_of_node(dev));
+	if (!parent_node) {
+		dev_err(dev, "Failed to get IRQ parent node\n");
+		return -ENODEV;
+	}
+
+	parent_domain = irq_find_host(parent_node);
+	if (!parent_domain) {
+		dev_err(dev, "Failed to find IRQ parent domain\n");
+		return -ENODEV;
+	}
+
+	intr = devm_kzalloc(dev, sizeof(*intr), GFP_KERNEL);
+	if (!intr)
+		return -ENOMEM;
+
+	ret = of_property_read_u32(dev_of_node(dev), "ti,intr-trigger-type",
+				   &intr->type);
+	if (ret) {
+		dev_err(dev, "missing ti,intr-trigger-type property\n");
+		return -EINVAL;
+	}
+
+	intr->sci = devm_ti_sci_get_by_phandle(dev, "ti,sci");
+	if (IS_ERR(intr->sci)) {
+		ret = PTR_ERR(intr->sci);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "ti,sci read fail %d\n", ret);
+		intr->sci = NULL;
+		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)) {
+		dev_err(dev, "Destination irq resource allocation failed\n");
+		return PTR_ERR(intr->dst_irq);
+	}
+
+	domain = irq_domain_add_hierarchy(parent_domain, 0, 0, dev_of_node(dev),
+					  &ti_sci_intr_irq_domain_ops, intr);
+	if (!domain) {
+		dev_err(dev, "Failed to allocate IRQ domain\n");
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id ti_sci_intr_irq_domain_of_match[] = {
+	{ .compatible = "ti,sci-intr", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ti_sci_intr_irq_domain_of_match);
+
+static struct platform_driver ti_sci_intr_irq_domain_driver = {
+	.probe = ti_sci_intr_irq_domain_probe,
+	.driver = {
+		.name = "ti-sci-intr",
+		.of_match_table = ti_sci_intr_irq_domain_of_match,
+	},
+};
+module_platform_driver(ti_sci_intr_irq_domain_driver);
+
+MODULE_AUTHOR("Lokesh Vutla <lokeshvutla@ticom>");
+MODULE_DESCRIPTION("K3 Interrupt Router driver over TI SCI protocol");
+MODULE_LICENSE("GPL v2");
-- 
2.21.0

[PATCH v6 08/12] dt-bindings: irqchip: Introduce TISCI Interrupt Aggregator bindings

[Lokesh Vutla]

Add the DT binding documentation for Interrupt Aggregator driver.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
Changes since v5:
- Dropped interrupt-cells property
- Added msi controller property

 .../interrupt-controller/ti,sci-inta.txt      | 66 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt
new file mode 100644
index 000000000000..0c2cee3be45f
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt
@@ -0,0 +1,66 @@
+Texas Instruments K3 Interrupt Aggregator
+=========================================
+
+The Interrupt Aggregator (INTA) provides a centralized machine
+which handles the termination of system events to that they can
+be coherently processed by the host(s) in the system. A maximum
+of 64 events can be mapped to a single interrupt.
+
+
+                              Interrupt Aggregator
+                     +-----------------------------------------+
+                     |      Intmap            VINT             |
+                     | +--------------+  +------------+        |
+            m ------>| | vint  | bit  |  | 0 |.....|63| vint0  |
+               .     | +--------------+  +------------+        |       +------+
+               .     |         .               .               |       | HOST |
+Globalevents  ------>|         .               .               |------>| IRQ  |
+               .     |         .               .               |       | CTRL |
+               .     |         .               .               |       +------+
+            n ------>| +--------------+  +------------+        |
+                     | | vint  | bit  |  | 0 |.....|63| vintx  |
+                     | +--------------+  +------------+        |
+                     |                                         |
+                     +-----------------------------------------+
+
+Configuration of these Intmap registers that maps global events to vint is done
+by a system controller (like the Device Memory and Security Controller on K3
+AM654 SoC). Driver should request the system controller to get the range
+of global events and vints assigned to the requesting host. Management
+of these requested resources should be handled by driver and requests
+system controller to map specific global event to vint, bit pair.
+
+Communication between the host processor running an OS and the system
+controller happens through a protocol called TI System Control Interface
+(TISCI protocol). For more details refer:
+Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
+
+TISCI Interrupt Aggregator Node:
+-------------------------------
+- compatible:		Must be "ti,sci-inta".
+- reg:			Should contain registers location and length.
+- interrupt-controller:	Identifies the node as an interrupt controller
+- msi-controller:	Identifies the node as an MSI controller.
+- interrupt-parent:	phandle of irq parent.
+- ti,sci:		Phandle to TI-SCI compatible System controller node.
+- ti,sci-dev-id:	TISCI device ID of the Interrupt Aggregator.
+- ti,sci-rm-range-vint:	TISCI subtype ids representing the virtual interrupts
+			(vints) range within this IA, assigned to the
+			requesting host context.
+- ti,sci-rm-range-global-event:	TISCI subtype ids representing the global
+			events range reaching this IA and are assigned
+			to the requesting host context.
+
+Example:
+--------
+main_udmass_inta: interrupt-controller@33d00000 {
+	compatible = "ti,sci-inta";
+	reg = <0x0 0x33d00000 0x0 0x100000>;
+	interrupt-controller;
+	msi-controller;
+	interrupt-parent = <&main_navss_intr>;
+	ti,sci = <&dmsc>;
+	ti,sci-dev-id = <179>;
+	ti,sci-rm-range-vint = <0x0>;
+	ti,sci-rm-range-global-event = <0x1>;
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 6f551053627f..90173038f674 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15350,6 +15350,7 @@ F:	Documentation/devicetree/bindings/clock/ti,sci-clk.txt
 F:	drivers/clk/keystone/sci-clk.c
 F:	drivers/reset/reset-ti-sci.c
 F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
+F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt
 F:	drivers/irqchip/irq-ti-sci-intr.c
 
 Texas Instruments ASoC drivers
-- 
2.21.0

[PATCH v6 09/12] irqchip: ti-sci-inta: Add support for Interrupt Aggregator driver

[Lokesh Vutla]

Texas Instruments' K3 generation SoCs has an IP Interrupt Aggregator
which is an interrupt controller that does the following:
- Converts events to interrupts that can be understood by
  an interrupt router.
- Allows for multiplexing of events to interrupts.

Configuration of the interrupt aggregator registers can only be done by
a system co-processor and the driver needs to send a message to this
co processor over TISCI protocol.

Add support for Interrupt Aggregator driver with 2 IRQ Domains:
- INTA MSI domain that interfaces the devices using which interrupts can be
  allocated dynamically.
- INTA domain that is parent to the MSI domain that manages the interrupt
  resources.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
Changes since v5:
- Moved all the allocation of events and parent irqs to .irq_request_resources
  callback of irqchip. This is based on the offline discussion with Marc.

Marc,
	This version has a deviation from our discussion:
- Could not create separate irq_domains for each output(vint) of INTA, instead
  stick to a single irq_domain for the entire INTA. Because when creating
  msi_domain there is no parent available to attach. Even then I create
  irq_domains for all the available vints during probe, how do we decide
  which is the parent of msi_domain?


 MAINTAINERS                       |   1 +
 drivers/irqchip/Kconfig           |  11 +
 drivers/irqchip/Makefile          |   1 +
 drivers/irqchip/irq-ti-sci-inta.c | 622 ++++++++++++++++++++++++++++++
 4 files changed, 635 insertions(+)
 create mode 100644 drivers/irqchip/irq-ti-sci-inta.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 90173038f674..ba88b3033fe4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15352,6 +15352,7 @@ F:	drivers/reset/reset-ti-sci.c
 F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
 F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt
 F:	drivers/irqchip/irq-ti-sci-intr.c
+F:	drivers/irqchip/irq-ti-sci-inta.c
 
 Texas Instruments ASoC drivers
 M:	Peter Ujfalusi <peter.ujfalusi@ti.com>
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index a1ff64c1d40d..946c062fcec0 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -436,6 +436,17 @@ config TI_SCI_INTR_IRQCHIP
 	  If you wish to use interrupt router irq resources managed by the
 	  TI System Controller, say Y here. Otherwise, say N.
 
+config TI_SCI_INTA_IRQCHIP
+	bool
+	depends on TI_SCI_PROTOCOL && ARCH_K3
+	select IRQ_DOMAIN
+	select IRQ_DOMAIN_HIERARCHY
+	help
+	  This enables the irqchip driver support for K3 Interrupt aggregator
+	  over TI System Control Interface available on some new TI's SoCs.
+	  If you wish to use interrupt aggregator irq resources managed by the
+	  TI System Controller, say Y here. Otherwise, say N.
+
 endmenu
 
 config SIFIVE_PLIC
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index fa5c865788b5..8a33013da953 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -98,3 +98,4 @@ obj-$(CONFIG_IMX_IRQSTEER)		+= irq-imx-irqsteer.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
+obj-$(CONFIG_TI_SCI_INTA_IRQCHIP)	+= irq-ti-sci-inta.o
diff --git a/drivers/irqchip/irq-ti-sci-inta.c b/drivers/irqchip/irq-ti-sci-inta.c
new file mode 100644
index 000000000000..3eb935ebe10f
--- /dev/null
+++ b/drivers/irqchip/irq-ti-sci-inta.c
@@ -0,0 +1,622 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Texas Instruments' K3 Interrupt Aggregator irqchip driver
+ *
+ * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
+ *	Lokesh Vutla <lokeshvutla@ti.com>
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/msi.h>
+#include <linux/irqchip.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/irqdomain.h>
+#include <linux/interrupt.h>
+#include <linux/soc/ti/ti_sci_protocol.h>
+#include <linux/irqchip/chained_irq.h>
+#include <asm-generic/msi.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 MAX_EVENTS_PER_VINT	64
+#define VINT_ENABLE_SET_OFFSET	0x0
+#define VINT_ENABLE_CLR_OFFSET	0x8
+#define VINT_STATUS_OFFSET	0x18
+
+/**
+ * struct ti_sci_inta_event_desc - Description of an event coming to
+ *				   Interrupt Aggregator. This serves
+ *				   as a mapping table between global
+ *				   event and hwirq.
+ * @global_event:	Global event number corresponding to this event
+ * @hwirq:		Hwirq of the incoming interrupt
+ */
+struct ti_sci_inta_event_desc {
+	u16 global_event;
+	u32 hwirq;
+};
+
+/**
+ * struct ti_sci_inta_vint_desc - Description of a virtual interrupt coming out
+ *				  of Interrupt Aggregator.
+ * @domain:		Pointer to IRQ domain to which this vint belongs.
+ * @list:		List entry for the vint list
+ * @event_map:		Bitmap to manage the allocation of events to vint.
+ * @event_mutex:	Mutex to protect allocation of events.
+ * @events:		Array of event descriptors assigned to this vint.
+ * @parent_virq:	Linux IRQ number that gets attached to parent
+ * @vint_id:		TISCI vint ID
+ */
+struct ti_sci_inta_vint_desc {
+	struct irq_domain *domain;
+	struct list_head list;
+	unsigned long *event_map;
+	/* Mutex to protect allocation of events */
+	struct mutex event_mutex;
+	struct ti_sci_inta_event_desc events[MAX_EVENTS_PER_VINT];
+	unsigned int parent_virq;
+	u16 vint_id;
+};
+
+/**
+ * struct ti_sci_inta_irq_domain - Structure representing a TISCI based
+ *				   Interrupt Aggregator IRQ domain.
+ * @sci:		Pointer to TISCI handle
+ * @vint:		TISCI resource pointer representing IA inerrupts.
+ * @global_event:	TISCI resource pointer representing global events.
+ * @vint_list:		List of the vints active in the system
+ * @vint_mutex:		Mutex to protect vint_list
+ * @base:		Base address of the memory mapped IO registers
+ * @pdev:		Pointer to platform device.
+ */
+struct ti_sci_inta_irq_domain {
+	const struct ti_sci_handle *sci;
+	struct ti_sci_resource *vint;
+	struct ti_sci_resource *global_event;
+	struct list_head vint_list;
+	/* Mutex to protect vint list */
+	struct mutex vint_mutex;
+	void __iomem *base;
+	struct platform_device	*pdev;
+};
+
+/**
+ * ti_sci_inta_irq_handler() - Chained IRQ handler for the vint irqs
+ * @desc:	Pointer to irq_desc corresponding to the irq
+ */
+static void ti_sci_inta_irq_handler(struct irq_desc *desc)
+{
+	struct ti_sci_inta_vint_desc *vint_desc;
+	struct ti_sci_inta_irq_domain *inta;
+	struct irq_domain *domain;
+	unsigned int virq, bit;
+	u64 val;
+
+	vint_desc = irq_desc_get_handler_data(desc);
+	domain = vint_desc->domain;
+	inta = domain->host_data;
+
+	chained_irq_enter(irq_desc_get_chip(desc), desc);
+
+	val = readq_relaxed(inta->base + vint_desc->vint_id * 0x1000 +
+			    VINT_STATUS_OFFSET);
+
+	for (bit = 0; bit < MAX_EVENTS_PER_VINT; bit++) {
+		if (BIT(bit) & val) {
+			virq = irq_find_mapping(domain,
+						vint_desc->events[bit].hwirq);
+			if (virq)
+				generic_handle_irq(virq);
+		}
+	}
+
+	chained_irq_exit(irq_desc_get_chip(desc), desc);
+}
+
+/**
+ * ti_sci_inta_alloc_parent_irq() - Allocate parent irq to Interrupt aggregator
+ * @domain:	IRQ domain corresponding to Interrupt Aggregator
+ *
+ * Return 0 if all went well else corresponding error value.
+ */
+static struct ti_sci_inta_vint_desc
+*ti_sci_inta_alloc_parent_irq(struct irq_domain *domain)
+{
+	struct ti_sci_inta_irq_domain *inta = domain->host_data;
+	struct ti_sci_inta_vint_desc *vint_desc;
+	struct irq_fwspec parent_fwspec;
+	u16 vint_id;
+
+	vint_id = ti_sci_get_free_resource(inta->vint);
+	if (vint_id == TI_SCI_RESOURCE_NULL)
+		return ERR_PTR(-EINVAL);
+
+	vint_desc = kzalloc(sizeof(*vint_desc), GFP_KERNEL);
+	if (!vint_desc)
+		return ERR_PTR(-ENOMEM);
+
+	vint_desc->event_map = kcalloc(BITS_TO_LONGS(MAX_EVENTS_PER_VINT),
+				       sizeof(*vint_desc->event_map),
+				       GFP_KERNEL);
+	if (!vint_desc->event_map) {
+		kfree(vint_desc);
+		return ERR_PTR(-ENOMEM);
+	}
+	mutex_init(&vint_desc->event_mutex);
+
+	vint_desc->domain = domain;
+	vint_desc->vint_id = vint_id;
+	INIT_LIST_HEAD(&vint_desc->list);
+
+	mutex_lock(&inta->vint_mutex);
+	list_add_tail(&vint_desc->list, &inta->vint_list);
+	mutex_unlock(&inta->vint_mutex);
+
+	parent_fwspec.fwnode =
+	of_node_to_fwnode(of_irq_find_parent(dev_of_node(&inta->pdev->dev)));
+	parent_fwspec.param_count = 3;
+	parent_fwspec.param[0] = inta->pdev->id;
+	parent_fwspec.param[1] = vint_desc->vint_id;
+	parent_fwspec.param[2] = 1;
+
+	vint_desc->parent_virq = irq_create_fwspec_mapping(&parent_fwspec);
+	if (vint_desc->parent_virq <= 0)
+		return ERR_PTR(vint_desc->parent_virq);
+
+	irq_set_chained_handler_and_data(vint_desc->parent_virq,
+					 ti_sci_inta_irq_handler, vint_desc);
+
+	return vint_desc;
+}
+
+/**
+ * ti_sci_inta_alloc_event() - Attach an event to a IA vint.
+ * @inta:	Pointer to INTA domain descriptor
+ * @vint_desc:	Pointer to vint_desc to which the event gets attached
+ * @free_bit:	Bit inside vint to which event gets attached
+ * @hwirq:	hwirq of the input event
+ *
+ * Return global_event if all went ok else appropriate error value.
+ */
+static
+int ti_sci_inta_alloc_event(struct ti_sci_inta_irq_domain *inta,
+			    struct ti_sci_inta_vint_desc *vint_desc,
+			    u16 free_bit, u32 hwirq)
+{
+	struct ti_sci_inta_event_desc *event_desc;
+	u16 dev_id, dev_index;
+	int err;
+
+	dev_id = HWIRQ_TO_DEVID(hwirq);
+	dev_index = HWIRQ_TO_IRQID(hwirq);
+	event_desc = &vint_desc->events[free_bit];
+	mutex_lock(&vint_desc->event_mutex);
+	event_desc->hwirq = hwirq;
+	event_desc->global_event = ti_sci_get_free_resource(inta->global_event);
+	if (event_desc->global_event == TI_SCI_RESOURCE_NULL) {
+		err = -EINVAL;
+		goto free_vint_bit;
+	}
+
+	err = inta->sci->ops.rm_irq_ops.set_event_map(inta->sci,
+						      dev_id, dev_index,
+						      inta->pdev->id,
+						      vint_desc->vint_id,
+						      event_desc->global_event,
+						      free_bit);
+	if (err)
+		goto free_global_event;
+
+	mutex_unlock(&vint_desc->event_mutex);
+	return 0;
+free_global_event:
+	ti_sci_release_resource(inta->global_event, event_desc->global_event);
+free_vint_bit:
+	clear_bit(free_bit, vint_desc->event_map);
+	mutex_unlock(&vint_desc->event_mutex);
+	return err;
+}
+
+/**
+ * ti_sci_inta_alloc_irq() -  Allocate an irq within INTA domain
+ * @domain:	irq_domain pointer corresponding to INTA
+ * @hwirq:	hwirq of the input event
+ *
+ * Note: Allocation happens in the following manner:
+ *	- Find a free bit available in any of the vints available in the list.
+ *	- If not found, allocate a vint from the vint pool
+ *	- Attach the free bit to input hwirq.
+ * Return vint_desc if all went ok else appropriate error value.
+ */
+static struct ti_sci_inta_vint_desc
+*ti_sci_inta_alloc_irq(struct irq_domain *domain, u32 hwirq)
+{
+	struct ti_sci_inta_irq_domain *inta = domain->host_data;
+	struct ti_sci_inta_vint_desc *vint_desc = NULL;
+	u16 free_bit;
+	int ret;
+
+	mutex_lock(&inta->vint_mutex);
+	list_for_each_entry(vint_desc, &inta->vint_list, list) {
+		mutex_lock(&vint_desc->event_mutex);
+		free_bit = find_first_zero_bit(vint_desc->event_map,
+					       MAX_EVENTS_PER_VINT);
+		if (free_bit != MAX_EVENTS_PER_VINT) {
+			set_bit(free_bit, vint_desc->event_map);
+			mutex_unlock(&vint_desc->event_mutex);
+			mutex_unlock(&inta->vint_mutex);
+			goto alloc_event;
+		}
+		mutex_unlock(&vint_desc->event_mutex);
+	}
+	mutex_unlock(&inta->vint_mutex);
+
+	/* No free bits available. Allocate a new vint */
+	vint_desc = ti_sci_inta_alloc_parent_irq(domain);
+	if (IS_ERR(vint_desc))
+		return vint_desc;
+
+	mutex_lock(&vint_desc->event_mutex);
+	free_bit = find_first_zero_bit(vint_desc->event_map,
+				       MAX_EVENTS_PER_VINT);
+	set_bit(free_bit, vint_desc->event_map);
+	mutex_unlock(&vint_desc->event_mutex);
+
+alloc_event:
+	ret = ti_sci_inta_alloc_event(inta, vint_desc, free_bit, hwirq);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return vint_desc;
+}
+
+/**
+ * __get_event_index() - Convert the hwirq to corresponding bit inside vint.
+ * @vint_desc:	Pointer to vint_desc to which the hwirq is attached
+ * @hwirq:	hwirq number within the domain
+ *
+ * Return the vint bit to which the hwirq is attached.
+ */
+static int __get_event_index(struct ti_sci_inta_vint_desc *vint_desc, u32 hwirq)
+{
+	int i;
+
+	mutex_lock(&vint_desc->event_mutex);
+	for (i = 0; i < MAX_EVENTS_PER_VINT; i++)
+		if (vint_desc->events[i].hwirq == hwirq) {
+			mutex_unlock(&vint_desc->event_mutex);
+			return i;
+		}
+
+	mutex_unlock(&vint_desc->event_mutex);
+	return -ENODEV;
+}
+
+/**
+ * ti_sci_inta_free_parent_irq() - Free a parent irq to INTA
+ * domain:	domain corresponding to INTA
+ */
+static void ti_sci_inta_free_parent_irq(struct irq_domain *domain,
+					struct ti_sci_inta_vint_desc *vint_desc)
+{
+	struct ti_sci_inta_irq_domain *inta = domain->host_data;
+
+	mutex_lock(&inta->vint_mutex);
+	list_del(&vint_desc->list);
+	mutex_unlock(&inta->vint_mutex);
+	irq_dispose_mapping(vint_desc->parent_virq);
+	ti_sci_release_resource(inta->vint, vint_desc->vint_id);
+	kfree(vint_desc->event_map);
+	kfree(vint_desc);
+}
+
+/**
+ * ti_sci_inta_free_irq() - Free an IRQ within INTA domain
+ * domain:	Domain corresponding to INTA
+ * vint_desc:	Pointer to vint_desc from which irq needs to be freed
+ * hwirq:	Hwirq number within INTA domain that needs to be freed
+ */
+static void ti_sci_inta_free_irq(struct irq_domain *domain,
+				 struct ti_sci_inta_vint_desc *vint_desc,
+				 u32 hwirq)
+{
+	struct ti_sci_inta_irq_domain *inta = domain->host_data;
+	struct ti_sci_inta_event_desc *event_desc;
+	int event_index = 0;
+
+	/* free event irq */
+	event_index = __get_event_index(vint_desc, hwirq);
+	event_desc = &vint_desc->events[event_index];
+	mutex_lock(&vint_desc->event_mutex);
+	inta->sci->ops.rm_irq_ops.free_event_map(inta->sci,
+						 HWIRQ_TO_DEVID(hwirq),
+						 HWIRQ_TO_IRQID(hwirq),
+						 inta->pdev->id,
+						 vint_desc->vint_id,
+						 event_desc->global_event,
+						 event_index);
+
+	clear_bit(event_index, vint_desc->event_map);
+	ti_sci_release_resource(inta->global_event, event_desc->global_event);
+	event_desc->global_event = TI_SCI_RESOURCE_NULL;
+	event_desc->hwirq = 0;
+	mutex_unlock(&vint_desc->event_mutex);
+
+	/* Free parent irq */
+	if (find_first_bit(vint_desc->event_map, MAX_EVENTS_PER_VINT) ==
+	    MAX_EVENTS_PER_VINT)
+		ti_sci_inta_free_parent_irq(domain, vint_desc);
+}
+
+/**
+ * ti_sci_inta_request_resources() - Allocate resources for input irq
+ * @data: Pointer to corresponding irq_data
+ *
+ * Note: This is the core api where the actual allocation happens for input
+ *	 hwirq. This allocation involves creating a parent irq for vint.
+ *	 If this is done in irq_domain_ops.alloc() then a deadlock is reached
+ *	 for allocation. So this allocation is being done in request_resources()
+ *
+ * Return: 0 if all went well else corresponding error.
+ */
+static int ti_sci_inta_request_resources(struct irq_data *data)
+{
+	struct ti_sci_inta_vint_desc *vint_desc;
+
+	vint_desc = ti_sci_inta_alloc_irq(data->domain, data->hwirq);
+	if (IS_ERR(vint_desc))
+		return PTR_ERR(vint_desc);
+
+	data->chip_data = vint_desc;
+
+	return 0;
+}
+
+/**
+ * ti_sci_inta_release_resources - Release resources for input irq
+ * @data: Pointer to corresponding irq_data
+ *
+ * Note: Corresponding to request_resources(), all the unmapping and deletion
+ *	 of parent vint irqs happens in this api.
+ */
+static void ti_sci_inta_release_resources(struct irq_data *data)
+{
+	struct ti_sci_inta_vint_desc *vint_desc;
+
+	vint_desc = irq_data_get_irq_chip_data(data);
+	ti_sci_inta_free_irq(data->domain, vint_desc, data->hwirq);
+}
+
+/**
+ * __ti_sci_inta_manage_event() - Control the event based on the offset
+ * @data:	Pointer to corresponding irq_data
+ * @offset:	register offset using which event is controlled.
+ */
+static void __ti_sci_inta_manage_event(struct irq_data *data, u32 offset)
+{
+	struct ti_sci_inta_vint_desc *vint_desc;
+	struct ti_sci_inta_irq_domain *inta;
+	int event_index;
+
+	vint_desc = irq_data_get_irq_chip_data(data);
+	inta = data->domain->host_data;
+	event_index = __get_event_index(vint_desc, data->hwirq);
+	if (event_index < 0)
+		return;
+
+	writeq_relaxed(BIT(event_index), inta->base +
+		       vint_desc->vint_id * 0x1000 + offset);
+}
+
+/**
+ * ti_sci_inta_mask_irq() - Mask an event
+ * @data:	Pointer to corresponding irq_data
+ */
+static void ti_sci_inta_mask_irq(struct irq_data *data)
+{
+	__ti_sci_inta_manage_event(data, VINT_ENABLE_CLR_OFFSET);
+}
+
+/**
+ * ti_sci_inta_unmask_irq() - Unmask an event
+ * @data:	Pointer to corresponding irq_data
+ */
+static void ti_sci_inta_unmask_irq(struct irq_data *data)
+{
+	__ti_sci_inta_manage_event(data, VINT_ENABLE_SET_OFFSET);
+}
+
+/**
+ * ti_sci_inta_ack_irq() - Ack an event
+ * @data:	Pointer to corresponding irq_data
+ */
+static void ti_sci_inta_ack_irq(struct irq_data *data)
+{
+	__ti_sci_inta_manage_event(data, VINT_STATUS_OFFSET);
+}
+
+static int ti_sci_inta_set_affinity(struct irq_data *d,
+				    const struct cpumask *mask_val, bool force)
+{
+	return -EINVAL;
+}
+
+/**
+ * ti_sci_inta_set_type() - Update the trigger type of the irq.
+ * @data:	Pointer to corresponding irq_data
+ * @type:	Trigger type as specified by user
+ *
+ * Note: This updates the handle_irq callback for level msi.
+ *
+ * Return 0 if all went well else appropriate error.
+ */
+static int ti_sci_inta_set_type(struct irq_data *data, unsigned int type)
+{
+	struct irq_desc *desc = irq_to_desc(data->irq);
+
+	/*
+	 * .alloc default sets handle_edge_irq. But if the user specifies
+	 * that IRQ is level MSI, then update the handle to handle_level_irq
+	 */
+	if (type & IRQF_TRIGGER_HIGH)
+		desc->handle_irq = handle_level_irq;
+
+	return 0;
+}
+
+static struct irq_chip ti_sci_inta_irq_chip = {
+	.name			= "INTA",
+	.irq_mask		= ti_sci_inta_mask_irq,
+	.irq_unmask		= ti_sci_inta_unmask_irq,
+	.irq_ack		= ti_sci_inta_ack_irq,
+	.irq_set_affinity	= ti_sci_inta_set_affinity,
+	.irq_request_resources	= ti_sci_inta_request_resources,
+	.irq_release_resources	= ti_sci_inta_release_resources,
+	.irq_set_type		= ti_sci_inta_set_type,
+};
+
+/**
+ * ti_sci_inta_irq_domain_free() - Free an IRQ from the IRQ domain
+ * @domain:	Domain to which the irqs belong
+ * @virq:	base linux virtual IRQ to be freed.
+ * @nr_irqs:	Number of continuous irqs to be freed
+ */
+static void ti_sci_inta_irq_domain_free(struct irq_domain *domain,
+					unsigned int virq, unsigned int nr_irqs)
+{
+	struct irq_data *data = irq_domain_get_irq_data(domain, virq);
+
+	irq_domain_reset_irq_data(data);
+}
+
+/**
+ * ti_sci_inta_irq_domain_alloc() - Allocate Interrupt aggregator IRQs
+ * @domain:	Point to the interrupt aggregator IRQ domain
+ * @virq:	Corresponding Linux virtual IRQ number
+ * @nr_irqs:	Continuous irqs to be allocated
+ * @data:	Pointer to firmware specifier
+ *
+ * No actual allocation happens here.
+ *
+ * Return 0 if all went well else appropriate error value.
+ */
+static int ti_sci_inta_irq_domain_alloc(struct irq_domain *domain,
+					unsigned int virq, unsigned int nr_irqs,
+					void *data)
+{
+	msi_alloc_info_t *arg = data;
+
+	irq_domain_set_info(domain, virq, arg->hwirq, &ti_sci_inta_irq_chip,
+			    NULL, handle_edge_irq, NULL, NULL);
+
+	return 0;
+}
+
+static const struct irq_domain_ops ti_sci_inta_irq_domain_ops = {
+	.alloc		= ti_sci_inta_irq_domain_alloc,
+	.free		= ti_sci_inta_irq_domain_free,
+};
+
+static int ti_sci_inta_irq_domain_probe(struct platform_device *pdev)
+{
+	struct irq_domain *parent_domain, *domain;
+	struct device_node *parent_node, *node;
+	struct ti_sci_inta_irq_domain *inta;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	int ret;
+
+	node = dev_of_node(dev);
+	parent_node = of_irq_find_parent(node);
+	if (!parent_node) {
+		dev_err(dev, "Failed to get IRQ parent node\n");
+		return -ENODEV;
+	}
+
+	parent_domain = irq_find_host(parent_node);
+	if (!parent_domain)
+		return -EPROBE_DEFER;
+
+	inta = devm_kzalloc(dev, sizeof(*inta), GFP_KERNEL);
+	if (!inta)
+		return -ENOMEM;
+
+	inta->pdev = pdev;
+	inta->sci = devm_ti_sci_get_by_phandle(dev, "ti,sci");
+	if (IS_ERR(inta->sci)) {
+		ret = PTR_ERR(inta->sci);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "ti,sci read fail %d\n", ret);
+		inta->sci = NULL;
+		return ret;
+	}
+
+	ret = of_property_read_u32(dev->of_node, "ti,sci-dev-id", &pdev->id);
+	if (ret) {
+		dev_err(dev, "missing 'ti,sci-dev-id' property\n");
+		return -EINVAL;
+	}
+
+	inta->vint = devm_ti_sci_get_of_resource(inta->sci, dev, pdev->id,
+						 "ti,sci-rm-range-vint");
+	if (IS_ERR(inta->vint)) {
+		dev_err(dev, "VINT resource allocation failed\n");
+		return PTR_ERR(inta->vint);
+	}
+
+	inta->global_event =
+		devm_ti_sci_get_of_resource(inta->sci, dev, pdev->id,
+					    "ti,sci-rm-range-global-event");
+	if (IS_ERR(inta->global_event)) {
+		dev_err(dev, "Global event resource allocation failed\n");
+		return PTR_ERR(inta->global_event);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	inta->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(inta->base))
+		return -ENODEV;
+
+	domain = irq_domain_add_linear(dev_of_node(dev),
+				       ti_sci_get_num_resources(inta->vint),
+				       &ti_sci_inta_irq_domain_ops, inta);
+	if (!domain) {
+		dev_err(dev, "Failed to allocate IRQ domain\n");
+		return -ENOMEM;
+	}
+
+	INIT_LIST_HEAD(&inta->vint_list);
+	mutex_init(&inta->vint_mutex);
+
+	return 0;
+}
+
+static const struct of_device_id ti_sci_inta_irq_domain_of_match[] = {
+	{ .compatible = "ti,sci-inta", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ti_sci_inta_irq_domain_of_match);
+
+static struct platform_driver ti_sci_inta_irq_domain_driver = {
+	.probe = ti_sci_inta_irq_domain_probe,
+	.driver = {
+		.name = "ti-sci-inta",
+		.of_match_table = ti_sci_inta_irq_domain_of_match,
+	},
+};
+module_platform_driver(ti_sci_inta_irq_domain_driver);
+
+MODULE_AUTHOR("Lokesh Vutla <lokeshvutla@ticom>");
+MODULE_DESCRIPTION("K3 Interrupt Aggregator driver over TI SCI protocol");
+MODULE_LICENSE("GPL v2");
-- 
2.21.0

[PATCH v6 10/12] soc: ti: Add MSI domain bus support for Interrupt Aggregator

[Lokesh Vutla]

With the system coprocessor managing the range allocation of the
inputs to Interrupt Aggregator, it is difficult to represent
the device IRQs from DT.

The suggestion is to use MSI in such cases where devices wants
to allocate and group interrupts dynamically.

Create a MSI domain bus layer that allocates and frees MSIs for
a device.

APIs that are implemented:
- ti_sci_inta_msi_create_irq_domain() that creates a MSI domain
- ti_sci_inta_msi_domain_alloc_irqs() that creates MSIs for the
  specified device and resource.
- ti_sci_inta_msi_domain_free_irqs() frees the irqs attached to the device.
- ti_sci_inta_msi_get_virq() for getting the virq attached to a specific event.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
Changes since v5:
- Updated the input parametes to all apis
- Updated the default chip ops.
- Prefixed all the apis with ti_sci_inta_

Marc,
	Right now ti_sci_resource is being passed for irq allocatons.
I couldn't get to use resources attached to platform_device. Because
platform_device resources are allocated in of_device_alloc() and number
of resources are fixed in it. In order to update the resources, driver
has to do a krealloc(pdev->resources) and update the num of resources.
Is it allowed to update the pdev->resources during probe time? If yes,
Ill be happy to update the patch to use platform_dev resources.


 MAINTAINERS                            |   2 +
 drivers/soc/ti/Kconfig                 |   6 +
 drivers/soc/ti/Makefile                |   1 +
 drivers/soc/ti/ti_sci_inta_msi.c       | 167 +++++++++++++++++++++++++
 include/linux/irqdomain.h              |   1 +
 include/linux/msi.h                    |   6 +
 include/linux/soc/ti/ti_sci_inta_msi.h |  23 ++++
 7 files changed, 206 insertions(+)
 create mode 100644 drivers/soc/ti/ti_sci_inta_msi.c
 create mode 100644 include/linux/soc/ti/ti_sci_inta_msi.h

diff --git a/MAINTAINERS b/MAINTAINERS
index ba88b3033fe4..dd31d7cb2fc6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15353,6 +15353,8 @@ F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
 F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt
 F:	drivers/irqchip/irq-ti-sci-intr.c
 F:	drivers/irqchip/irq-ti-sci-inta.c
+F:	include/linux/soc/ti/ti_sci_inta_msi.h
+F:	drivers/soc/ti/ti_sci_inta_msi.c
 
 Texas Instruments ASoC drivers
 M:	Peter Ujfalusi <peter.ujfalusi@ti.com>
diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
index be4570baad96..82f110fe4953 100644
--- a/drivers/soc/ti/Kconfig
+++ b/drivers/soc/ti/Kconfig
@@ -73,4 +73,10 @@ config TI_SCI_PM_DOMAINS
 	  called ti_sci_pm_domains. Note this is needed early in boot before
 	  rootfs may be available.
 
+config TI_SCI_INTA_MSI_DOMAIN
+	bool
+	select GENERIC_MSI_IRQ_DOMAIN
+	help
+	  Driver to enable Interrupt Aggregator specific MSI Domain.
+
 endif # SOC_TI
diff --git a/drivers/soc/ti/Makefile b/drivers/soc/ti/Makefile
index a22edc0b258a..b3868d392d4f 100644
--- a/drivers/soc/ti/Makefile
+++ b/drivers/soc/ti/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_KEYSTONE_NAVIGATOR_DMA)	+= knav_dma.o
 obj-$(CONFIG_AMX3_PM)			+= pm33xx.o
 obj-$(CONFIG_WKUP_M3_IPC)		+= wkup_m3_ipc.o
 obj-$(CONFIG_TI_SCI_PM_DOMAINS)		+= ti_sci_pm_domains.o
+obj-$(CONFIG_TI_SCI_INTA_MSI_DOMAIN)	+= ti_sci_inta_msi.o
diff --git a/drivers/soc/ti/ti_sci_inta_msi.c b/drivers/soc/ti/ti_sci_inta_msi.c
new file mode 100644
index 000000000000..247a5e5f216b
--- /dev/null
+++ b/drivers/soc/ti/ti_sci_inta_msi.c
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Texas Instruments' K3 Interrupt Aggregator MSI bus
+ *
+ * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
+ *	Lokesh Vutla <lokeshvutla@ti.com>
+ */
+
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/msi.h>
+#include <linux/soc/ti/ti_sci_inta_msi.h>
+#include <linux/soc/ti/ti_sci_protocol.h>
+
+static void ti_sci_inta_msi_write_msg(struct irq_data *data,
+				      struct msi_msg *msg)
+{
+	/* Nothing to do */
+}
+
+static void ti_sci_inta_msi_compose_msi_msg(struct irq_data *data,
+					    struct msi_msg *msg)
+{
+	/* Nothing to do */
+}
+
+static int ti_sci_inta_msi_request_resources(struct irq_data *data)
+{
+	data = data->parent_data;
+
+	return data->chip->irq_request_resources(data);
+}
+
+static void ti_sci_inta_msi_release_resources(struct irq_data *data)
+{
+	data = data->parent_data;
+	data->chip->irq_release_resources(data);
+}
+
+static void ti_sci_inta_msi_update_chip_ops(struct msi_domain_info *info)
+{
+	struct irq_chip *chip = info->chip;
+
+	WARN_ON(!chip);
+	if (!chip->irq_mask)
+		chip->irq_mask = irq_chip_mask_parent;
+	if (!chip->irq_unmask)
+		chip->irq_unmask = irq_chip_unmask_parent;
+	if (!chip->irq_ack)
+		chip->irq_ack = irq_chip_ack_parent;
+	if (!chip->irq_set_type)
+		chip->irq_set_type = irq_chip_set_type_parent;
+	if (!chip->irq_write_msi_msg)
+		chip->irq_write_msi_msg = ti_sci_inta_msi_write_msg;
+	if (!chip->irq_compose_msi_msg)
+		chip->irq_compose_msi_msg = ti_sci_inta_msi_compose_msi_msg;
+	if (!chip->irq_request_resources)
+		chip->irq_request_resources = ti_sci_inta_msi_request_resources;
+	if (!chip->irq_release_resources)
+		chip->irq_release_resources = ti_sci_inta_msi_release_resources;
+}
+
+struct irq_domain
+*ti_sci_inta_msi_create_irq_domain(struct fwnode_handle *fwnode,
+				   struct msi_domain_info *info,
+				   struct irq_domain *parent)
+{
+	struct irq_domain *domain;
+
+	if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
+		ti_sci_inta_msi_update_chip_ops(info);
+
+	domain = msi_create_irq_domain(fwnode, info, parent);
+	if (domain)
+		irq_domain_update_bus_token(domain, DOMAIN_BUS_TI_SCI_INTA_MSI);
+
+	return domain;
+}
+EXPORT_SYMBOL_GPL(ti_sci_inta_msi_create_irq_domain);
+
+static void ti_sci_inta_msi_free_descs(struct device *dev)
+{
+	struct msi_desc *desc, *tmp;
+
+	list_for_each_entry_safe(desc, tmp, dev_to_msi_list(dev), list) {
+		list_del(&desc->list);
+		free_msi_entry(desc);
+	}
+}
+
+static int ti_sci_inta_msi_alloc_descs(struct device *dev, u32 dev_id,
+				       struct ti_sci_resource *res)
+{
+	struct msi_desc *msi_desc;
+	int set, i, count = 0;
+
+	for (set = 0; set < res->sets; set++) {
+		for (i = 0; i < res->desc[set].num; i++) {
+			msi_desc = alloc_msi_entry(dev, 1, NULL);
+			if (!msi_desc) {
+				ti_sci_inta_msi_free_descs(dev);
+				return -ENOMEM;
+			}
+
+			msi_desc->inta.index = res->desc[set].start + i;
+			msi_desc->inta.dev_id = dev_id;
+			INIT_LIST_HEAD(&msi_desc->list);
+			list_add_tail(&msi_desc->list, dev_to_msi_list(dev));
+			count++;
+		}
+	}
+
+	return count;
+}
+
+int ti_sci_inta_msi_domain_alloc_irqs(struct platform_device *pdev,
+				      struct ti_sci_resource *res)
+{
+	struct irq_domain *msi_domain;
+	int ret, nvec;
+
+	msi_domain = dev_get_msi_domain(&pdev->dev);
+	if (!msi_domain)
+		return -EINVAL;
+
+	if (pdev->id < 0)
+		return -ENODEV;
+
+	nvec = ti_sci_inta_msi_alloc_descs(&pdev->dev, pdev->id, res);
+	if (nvec <= 0)
+		return nvec;
+
+	ret = msi_domain_alloc_irqs(msi_domain, &pdev->dev, nvec);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to allocate IRQs %d\n", ret);
+		goto cleanup;
+	}
+
+	return 0;
+
+cleanup:
+	ti_sci_inta_msi_free_descs(&pdev->dev);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(ti_sci_inta_msi_domain_alloc_irqs);
+
+void ti_sci_inta_msi_domain_free_irqs(struct device *dev)
+{
+	msi_domain_free_irqs(dev->msi_domain, dev);
+	ti_sci_inta_msi_free_descs(dev);
+}
+EXPORT_SYMBOL_GPL(ti_sci_inta_msi_domain_free_irqs);
+
+unsigned int ti_sci_inta_msi_get_virq(struct platform_device *pdev, u32 index)
+{
+	struct msi_desc *desc;
+
+	for_each_msi_entry(desc, &pdev->dev)
+		if (desc->inta.index == index && desc->inta.dev_id == pdev->id)
+			return desc->irq;
+
+	return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(ti_sci_inta_msi_get_virq);
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 61706b430907..07ec8b390161 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -82,6 +82,7 @@ enum irq_domain_bus_token {
 	DOMAIN_BUS_NEXUS,
 	DOMAIN_BUS_IPI,
 	DOMAIN_BUS_FSL_MC_MSI,
+	DOMAIN_BUS_TI_SCI_INTA_MSI,
 };
 
 /**
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 7e9b81c3b50d..7e12dfc4cb39 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -47,6 +47,11 @@ struct fsl_mc_msi_desc {
 	u16				msi_index;
 };
 
+struct ti_sci_inta_msi_desc {
+	u16	dev_id;
+	u16	index;
+};
+
 /**
  * struct msi_desc - Descriptor structure for MSI based interrupts
  * @list:	List head for management
@@ -106,6 +111,7 @@ struct msi_desc {
 		 */
 		struct platform_msi_desc platform;
 		struct fsl_mc_msi_desc fsl_mc;
+		struct ti_sci_inta_msi_desc inta;
 	};
 };
 
diff --git a/include/linux/soc/ti/ti_sci_inta_msi.h b/include/linux/soc/ti/ti_sci_inta_msi.h
new file mode 100644
index 000000000000..b0ca20ab3f49
--- /dev/null
+++ b/include/linux/soc/ti/ti_sci_inta_msi.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Texas Instruments' K3 TI SCI INTA MSI helper
+ *
+ * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
+ *	Lokesh Vutla <lokeshvutla@ti.com>
+ */
+
+#ifndef __INCLUDE_LINUX_TI_SCI_INTA_MSI_H
+#define __INCLUDE_LINUX_TI_SCI_INTA_MSI_H
+
+#include <linux/msi.h>
+#include <linux/soc/ti/ti_sci_protocol.h>
+
+struct irq_domain
+*ti_sci_inta_msi_create_irq_domain(struct fwnode_handle *fwnode,
+				   struct msi_domain_info *info,
+				   struct irq_domain *parent);
+int ti_sci_inta_msi_domain_alloc_irqs(struct platform_device *pdev,
+				      struct ti_sci_resource *res);
+unsigned int ti_sci_inta_msi_get_virq(struct platform_device *dev, u32 index);
+void ti_sci_inta_msi_domain_free_irqs(struct device *dev);
+#endif /* __INCLUDE_LINUX_IRQCHIP_TI_SCI_INTA_H */
-- 
2.21.0

[PATCH v6 11/12] irqchip: ti-sci-inta: Add msi domain support

[Lokesh Vutla]

Add a msi domain that is child to the INTA domain. Clients
uses the INTA msi bus layer to allocate irqs in this
msi domain.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
Changes since v5:
- New patch. Seperated out msi domain part from the intial patch.

Marc,
	I feel this is too simple to be a separate driver. So I am
	sticking it into the same file.

 drivers/irqchip/Kconfig           |  1 +
 drivers/irqchip/irq-ti-sci-inta.c | 39 ++++++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 946c062fcec0..e0a1ec55ca93 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -441,6 +441,7 @@ config TI_SCI_INTA_IRQCHIP
 	depends on TI_SCI_PROTOCOL && ARCH_K3
 	select IRQ_DOMAIN
 	select IRQ_DOMAIN_HIERARCHY
+	select TI_SCI_INTA_MSI_DOMAIN
 	help
 	  This enables the irqchip driver support for K3 Interrupt aggregator
 	  over TI System Control Interface available on some new TI's SoCs.
diff --git a/drivers/irqchip/irq-ti-sci-inta.c b/drivers/irqchip/irq-ti-sci-inta.c
index 3eb935ebe10f..945e635847b2 100644
--- a/drivers/irqchip/irq-ti-sci-inta.c
+++ b/drivers/irqchip/irq-ti-sci-inta.c
@@ -18,6 +18,7 @@
 #include <linux/irqdomain.h>
 #include <linux/interrupt.h>
 #include <linux/soc/ti/ti_sci_protocol.h>
+#include <linux/soc/ti/ti_sci_inta_msi.h>
 #include <linux/irqchip/chained_irq.h>
 #include <asm-generic/msi.h>
 
@@ -28,6 +29,9 @@
 #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))
 
 #define MAX_EVENTS_PER_VINT	64
 #define VINT_ENABLE_SET_OFFSET	0x0
@@ -528,9 +532,32 @@ static const struct irq_domain_ops ti_sci_inta_irq_domain_ops = {
 	.free		= ti_sci_inta_irq_domain_free,
 };
 
+static struct irq_chip ti_sci_inta_msi_irq_chip = {
+	.name			= "MSI-INTA",
+	.flags			= IRQCHIP_SUPPORTS_LEVEL_MSI,
+};
+
+static void ti_sci_inta_msi_set_desc(msi_alloc_info_t *arg,
+				     struct msi_desc *desc)
+{
+	arg->desc = desc;
+	arg->hwirq = TO_HWIRQ(desc->inta.dev_id, desc->inta.index);
+}
+
+static struct msi_domain_ops ti_sci_inta_msi_ops = {
+	.set_desc	= ti_sci_inta_msi_set_desc,
+};
+
+static struct msi_domain_info ti_sci_inta_msi_domain_info = {
+	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+		   MSI_FLAG_LEVEL_CAPABLE),
+	.ops	= &ti_sci_inta_msi_ops,
+	.chip	= &ti_sci_inta_msi_irq_chip,
+};
+
 static int ti_sci_inta_irq_domain_probe(struct platform_device *pdev)
 {
-	struct irq_domain *parent_domain, *domain;
+	struct irq_domain *parent_domain, *domain, *msi_domain;
 	struct device_node *parent_node, *node;
 	struct ti_sci_inta_irq_domain *inta;
 	struct device *dev = &pdev->dev;
@@ -596,6 +623,16 @@ static int ti_sci_inta_irq_domain_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	msi_domain =
+		ti_sci_inta_msi_create_irq_domain(of_node_to_fwnode(node),
+						  &ti_sci_inta_msi_domain_info,
+						  domain);
+	if (!msi_domain) {
+		irq_domain_remove(domain);
+		dev_err(dev, "Failed to allocate msi domain\n");
+		return -ENOMEM;
+	}
+
 	INIT_LIST_HEAD(&inta->vint_list);
 	mutex_init(&inta->vint_mutex);
 
-- 
2.21.0

[PATCH v6 12/12] soc: ti: am6: Enable interrupt controller driver

[Lokesh Vutla]

Select the TISCI Interrupt Router, Aggregator drivers and all its
dependencies for AM6 SoC.

Suggested-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
Changes since v5:
- None

 drivers/soc/ti/Kconfig | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
index 82f110fe4953..a56cd70acbe1 100644
--- a/drivers/soc/ti/Kconfig
+++ b/drivers/soc/ti/Kconfig
@@ -5,6 +5,11 @@ if ARCH_K3
 
 config ARCH_K3_AM6_SOC
 	bool "K3 AM6 SoC"
+	select MAILBOX
+	select TI_MESSAGE_MANAGER
+	select TI_SCI_PROTOCOL
+	select TI_SCI_INTR_IRQCHIP
+	select TI_SCI_INTA_IRQCHIP
 	help
 	  Enable support for TI's AM6 SoC Family support
 
-- 
2.21.0

Re: [PATCH v6 04/12] firmware: ti_sci: Add RM mapping table for am654

[Tony Lindgren]

Hi,

* Lokesh Vutla <lokeshvutla@ti.com> [190410 04:15]:
> From: Peter Ujfalusi <peter.ujfalusi@ti.com>
> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt b/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
> index b56a02c10ae6..6f0cd31c1520 100644
> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
> @@ -24,7 +24,8 @@ relationship between the TI-SCI parent node to the child node.
>  
>  Required properties:
>  -------------------
> -- compatible: should be "ti,k2g-sci"
> +- compatible:	should be "ti,k2g-sci" for TI 66AK2G SoC
> +		should be "ti,am654-sci" for for TI AM654 SoC
>  - mbox-names:
>  	"rx" - Mailbox corresponding to receive path
>  	"tx" - Mailbox corresponding to transmit path
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index d303f5a14da9..88e461498def 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -2297,10 +2297,33 @@ static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
>  	/* Limited by MBOX_TX_QUEUE_LEN. K2G can handle upto 128 messages! */
>  	.max_msgs = 20,
>  	.max_msg_size = 64,
> +	.rm_type_map = NULL,
> +};
> +
> +static struct ti_sci_rm_type_map ti_sci_am654_rm_type_map[] = {
> +	{.dev_id = 56, .type = 0x00b}, /* GIC_IRQ */
> +	{.dev_id = 179, .type = 0x000}, /* MAIN_NAV_UDMASS_IA0 */
> +	{.dev_id = 187, .type = 0x009}, /* MAIN_NAV_RA */
> +	{.dev_id = 188, .type = 0x006}, /* MAIN_NAV_UDMAP */
> +	{.dev_id = 194, .type = 0x007}, /* MCU_NAV_UDMAP */
> +	{.dev_id = 195, .type = 0x00a}, /* MCU_NAV_RA */
> +	{.dev_id = 0, .type = 0x000}, /* end of table */
> +};
> +
> +/* Description for AM654 */
> +static const struct ti_sci_desc ti_sci_pmmc_am654_desc = {
> +	.default_host_id = 12,
> +	/* Conservative duration */
> +	.max_rx_timeout_ms = 10000,
> +	/* Limited by MBOX_TX_QUEUE_LEN. K2G can handle upto 128 messages! */
> +	.max_msgs = 20,
> +	.max_msg_size = 60,
> +	.rm_type_map = ti_sci_am654_rm_type_map,
>  };
>  
>  static const struct of_device_id ti_sci_of_match[] = {
>  	{.compatible = "ti,k2g-sci", .data = &ti_sci_pmmc_k2g_desc},
> +	{.compatible = "ti,am654-sci", .data = &ti_sci_pmmc_am654_desc},
>  	{ /* Sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, ti_sci_of_match);

Great, this approach with mapping table in the driver based on
the compatible looks good to me and avoids stuffing the IDs
into device tree:

Acked-by: Tony Lindgren <tony@atomide.com>

Re: [PATCH v6 06/12] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings

[Tony Lindgren]

Hi,

* Lokesh Vutla <lokeshvutla@ti.com> [190410 04:15]:
> +Example:
> +--------
> +The following example demonstrates both interrupt router node and the consumer
> +node(main gpio) on the AM654 SoC:
> +
> +main_intr: interrupt-controller0 {
> +	compatible = "ti,sci-intr";
> +	ti,intr-trigger-type = <1>;
> +	interrupt-controller;
> +	interrupt-parent = <&gic500>;
> +	#interrupt-cells = <3>;
> +	ti,sci = <&dmsc>;
> +	ti,sci-dst-id = <56>;
> +	ti,sci-rm-range-girq = <0x1>;
> +};

To me it seems there should not be too many of these interrupt
controller nodes for each SoC. Maybe you're already planning on
doing it, but I suggest that you just add more specific compatibles
and then look up the dst-id from a mapping table in the driver
similar to what patch 04/12 in this series is already doing.

That way you don't need to add custom TI specific (firmware
defined) device tree properties listed above ;)

Regards,

Tony

Re: [PATCH v6 04/12] firmware: ti_sci: Add RM mapping table for am654

[Lokesh Vutla]

On 11/04/19 8:24 PM, Tony Lindgren wrote:
> Hi,
> 
> * Lokesh Vutla <lokeshvutla@ti.com> [190410 04:15]:
>> From: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt b/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>> index b56a02c10ae6..6f0cd31c1520 100644
>> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>> @@ -24,7 +24,8 @@ relationship between the TI-SCI parent node to the child node.
>>  
>>  Required properties:
>>  -------------------
>> -- compatible: should be "ti,k2g-sci"
>> +- compatible:	should be "ti,k2g-sci" for TI 66AK2G SoC
>> +		should be "ti,am654-sci" for for TI AM654 SoC
>>  - mbox-names:
>>  	"rx" - Mailbox corresponding to receive path
>>  	"tx" - Mailbox corresponding to transmit path
>> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
>> index d303f5a14da9..88e461498def 100644
>> --- a/drivers/firmware/ti_sci.c
>> +++ b/drivers/firmware/ti_sci.c
>> @@ -2297,10 +2297,33 @@ static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
>>  	/* Limited by MBOX_TX_QUEUE_LEN. K2G can handle upto 128 messages! */
>>  	.max_msgs = 20,
>>  	.max_msg_size = 64,
>> +	.rm_type_map = NULL,
>> +};
>> +
>> +static struct ti_sci_rm_type_map ti_sci_am654_rm_type_map[] = {
>> +	{.dev_id = 56, .type = 0x00b}, /* GIC_IRQ */
>> +	{.dev_id = 179, .type = 0x000}, /* MAIN_NAV_UDMASS_IA0 */
>> +	{.dev_id = 187, .type = 0x009}, /* MAIN_NAV_RA */
>> +	{.dev_id = 188, .type = 0x006}, /* MAIN_NAV_UDMAP */
>> +	{.dev_id = 194, .type = 0x007}, /* MCU_NAV_UDMAP */
>> +	{.dev_id = 195, .type = 0x00a}, /* MCU_NAV_RA */
>> +	{.dev_id = 0, .type = 0x000}, /* end of table */
>> +};
>> +
>> +/* Description for AM654 */
>> +static const struct ti_sci_desc ti_sci_pmmc_am654_desc = {
>> +	.default_host_id = 12,
>> +	/* Conservative duration */
>> +	.max_rx_timeout_ms = 10000,
>> +	/* Limited by MBOX_TX_QUEUE_LEN. K2G can handle upto 128 messages! */
>> +	.max_msgs = 20,
>> +	.max_msg_size = 60,
>> +	.rm_type_map = ti_sci_am654_rm_type_map,
>>  };
>>  
>>  static const struct of_device_id ti_sci_of_match[] = {
>>  	{.compatible = "ti,k2g-sci", .data = &ti_sci_pmmc_k2g_desc},
>> +	{.compatible = "ti,am654-sci", .data = &ti_sci_pmmc_am654_desc},
>>  	{ /* Sentinel */ },
>>  };
>>  MODULE_DEVICE_TABLE(of, ti_sci_of_match);
> 
> Great, this approach with mapping table in the driver based on
> the compatible looks good to me and avoids stuffing the IDs
> into device tree:
> 
> Acked-by: Tony Lindgren <tony@atomide.com>
> 

Thanks, but I don't think you understood what the patch is actually doing.
Please look at the rest of the series on how this table is being used.

Thanks and regards,
Lokesh

Re: [PATCH v6 06/12] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings

[Lokesh Vutla]

On 11/04/19 8:30 PM, Tony Lindgren wrote:
> Hi,
> 
> * Lokesh Vutla <lokeshvutla@ti.com> [190410 04:15]:
>> +Example:
>> +--------
>> +The following example demonstrates both interrupt router node and the consumer
>> +node(main gpio) on the AM654 SoC:
>> +
>> +main_intr: interrupt-controller0 {
>> +	compatible = "ti,sci-intr";
>> +	ti,intr-trigger-type = <1>;
>> +	interrupt-controller;
>> +	interrupt-parent = <&gic500>;
>> +	#interrupt-cells = <3>;
>> +	ti,sci = <&dmsc>;
>> +	ti,sci-dst-id = <56>;
>> +	ti,sci-rm-range-girq = <0x1>;
>> +};
> 
> To me it seems there should not be too many of these interrupt
> controller nodes for each SoC. Maybe you're already planning on
> doing it, but I suggest that you just add more specific compatibles
> and then look up the dst-id from a mapping table in the driver
> similar to what patch 04/12 in this series is already doing.
> 
> That way you don't need to add custom TI specific (firmware
> defined) device tree properties listed above ;)

I am tired of arguing on this. We did close this topic in the previous version
of this series. Why do you want to keep re visiting the same. Sorry, I am not
going change unless I receive a Nack from Marc or Rob.

Thanks and regards,
Lokesh

Re: [PATCH v6 06/12] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings

[Tero Kristo]

On 12/04/2019 07:24, Lokesh Vutla wrote:
> 
> 
> On 11/04/19 8:30 PM, Tony Lindgren wrote:
>> Hi,
>>
>> * Lokesh Vutla <lokeshvutla@ti.com> [190410 04:15]:
>>> +Example:
>>> +--------
>>> +The following example demonstrates both interrupt router node and the consumer
>>> +node(main gpio) on the AM654 SoC:
>>> +
>>> +main_intr: interrupt-controller0 {
>>> +	compatible = "ti,sci-intr";
>>> +	ti,intr-trigger-type = <1>;
>>> +	interrupt-controller;
>>> +	interrupt-parent = <&gic500>;
>>> +	#interrupt-cells = <3>;
>>> +	ti,sci = <&dmsc>;
>>> +	ti,sci-dst-id = <56>;
>>> +	ti,sci-rm-range-girq = <0x1>;
>>> +};
>>
>> To me it seems there should not be too many of these interrupt
>> controller nodes for each SoC. Maybe you're already planning on
>> doing it, but I suggest that you just add more specific compatibles
>> and then look up the dst-id from a mapping table in the driver
>> similar to what patch 04/12 in this series is already doing.
>>
>> That way you don't need to add custom TI specific (firmware
>> defined) device tree properties listed above ;)
> 

< snip: I think Lokesh had a bad day or something :) >

Anyways, the reason why we want these as custom properties in the DT is 
that there are multiple instances of the routers within one SoC. On 
AM65x we have MAIN NAVSS, MCU NAVSS, GPIOs for both etc., if we add 
driver data for each of these, it easily explodes quite a bit. 
Especially going forward with new SoCs.

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

Re: [PATCH v6 04/12] firmware: ti_sci: Add RM mapping table for am654

[Tony Lindgren]

* Lokesh Vutla <lokeshvutla@ti.com> [190412 04:08]:
> 
> 
> On 11/04/19 8:24 PM, Tony Lindgren wrote:
> > Hi,
> > 
> > * Lokesh Vutla <lokeshvutla@ti.com> [190410 04:15]:
> >> From: Peter Ujfalusi <peter.ujfalusi@ti.com>
> >> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt b/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
> >> index b56a02c10ae6..6f0cd31c1520 100644
> >> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
> >> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
> >> @@ -24,7 +24,8 @@ relationship between the TI-SCI parent node to the child node.
> >>  
> >>  Required properties:
> >>  -------------------
> >> -- compatible: should be "ti,k2g-sci"
> >> +- compatible:	should be "ti,k2g-sci" for TI 66AK2G SoC
> >> +		should be "ti,am654-sci" for for TI AM654 SoC
> >>  - mbox-names:
> >>  	"rx" - Mailbox corresponding to receive path
> >>  	"tx" - Mailbox corresponding to transmit path
> >> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> >> index d303f5a14da9..88e461498def 100644
> >> --- a/drivers/firmware/ti_sci.c
> >> +++ b/drivers/firmware/ti_sci.c
> >> @@ -2297,10 +2297,33 @@ static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
> >>  	/* Limited by MBOX_TX_QUEUE_LEN. K2G can handle upto 128 messages! */
> >>  	.max_msgs = 20,
> >>  	.max_msg_size = 64,
> >> +	.rm_type_map = NULL,
> >> +};
> >> +
> >> +static struct ti_sci_rm_type_map ti_sci_am654_rm_type_map[] = {
> >> +	{.dev_id = 56, .type = 0x00b}, /* GIC_IRQ */
> >> +	{.dev_id = 179, .type = 0x000}, /* MAIN_NAV_UDMASS_IA0 */
> >> +	{.dev_id = 187, .type = 0x009}, /* MAIN_NAV_RA */
> >> +	{.dev_id = 188, .type = 0x006}, /* MAIN_NAV_UDMAP */
> >> +	{.dev_id = 194, .type = 0x007}, /* MCU_NAV_UDMAP */
> >> +	{.dev_id = 195, .type = 0x00a}, /* MCU_NAV_RA */
> >> +	{.dev_id = 0, .type = 0x000}, /* end of table */
> >> +};
> >> +
> >> +/* Description for AM654 */
> >> +static const struct ti_sci_desc ti_sci_pmmc_am654_desc = {
> >> +	.default_host_id = 12,
> >> +	/* Conservative duration */
> >> +	.max_rx_timeout_ms = 10000,
> >> +	/* Limited by MBOX_TX_QUEUE_LEN. K2G can handle upto 128 messages! */
> >> +	.max_msgs = 20,
> >> +	.max_msg_size = 60,
> >> +	.rm_type_map = ti_sci_am654_rm_type_map,
> >>  };
> >>  
> >>  static const struct of_device_id ti_sci_of_match[] = {
> >>  	{.compatible = "ti,k2g-sci", .data = &ti_sci_pmmc_k2g_desc},
> >> +	{.compatible = "ti,am654-sci", .data = &ti_sci_pmmc_am654_desc},
> >>  	{ /* Sentinel */ },
> >>  };
> >>  MODULE_DEVICE_TABLE(of, ti_sci_of_match);
> > 
> > Great, this approach with mapping table in the driver based on
> > the compatible looks good to me and avoids stuffing the IDs
> > into device tree:
> > 
> > Acked-by: Tony Lindgren <tony@atomide.com>
> > 
> 
> Thanks, but I don't think you understood what the patch is actually doing.
> Please look at the rest of the series on how this table is being used.

Oh OK yes I misunderstood. So you still need to use the dev_id also
in the dts.

Regards,

Tony

Re: [PATCH v6 06/12] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings

[Tony Lindgren]

* Tero Kristo <t-kristo@ti.com> [190412 08:43]:
> On 12/04/2019 07:24, Lokesh Vutla wrote:
> > 
> > 
> > On 11/04/19 8:30 PM, Tony Lindgren wrote:
> > > Hi,
> > > 
> > > * Lokesh Vutla <lokeshvutla@ti.com> [190410 04:15]:
> > > > +Example:
> > > > +--------
> > > > +The following example demonstrates both interrupt router node and the consumer
> > > > +node(main gpio) on the AM654 SoC:
> > > > +
> > > > +main_intr: interrupt-controller0 {
> > > > +	compatible = "ti,sci-intr";
> > > > +	ti,intr-trigger-type = <1>;
> > > > +	interrupt-controller;
> > > > +	interrupt-parent = <&gic500>;
> > > > +	#interrupt-cells = <3>;
> > > > +	ti,sci = <&dmsc>;
> > > > +	ti,sci-dst-id = <56>;
> > > > +	ti,sci-rm-range-girq = <0x1>;
> > > > +};
> > > 
> > > To me it seems there should not be too many of these interrupt
> > > controller nodes for each SoC. Maybe you're already planning on
> > > doing it, but I suggest that you just add more specific compatibles
> > > and then look up the dst-id from a mapping table in the driver
> > > similar to what patch 04/12 in this series is already doing.
> > > 
> > > That way you don't need to add custom TI specific (firmware
> > > defined) device tree properties listed above ;)
> > 
> 
> < snip: I think Lokesh had a bad day or something :) >

Certainly custom bindings need to be discussed properly on the lists :)

> Anyways, the reason why we want these as custom properties in the DT is that
> there are multiple instances of the routers within one SoC. On AM65x we have
> MAIN NAVSS, MCU NAVSS, GPIOs for both etc., if we add driver data for each
> of these, it easily explodes quite a bit. Especially going forward with new
> SoCs.

Yup and I keep getting worried about this every time I see it still. But like
I've mentioned when we chatted offline, as long as Arnd, Marc and Rob ack this
use, I can live with it.

I'm just trying to avoid another "ti,hwmods" type custom property here..
We are still dealing with it to replace it with just standard use of
"compatible" property, and every time Rob sees it he still comments on it :)

Reagrds,

Tony

Re: [PATCH v6 09/12] irqchip: ti-sci-inta: Add support for Interrupt Aggregator driver

[Lokesh Vutla]

On 10/04/19 9:43 AM, Lokesh Vutla wrote:
> Texas Instruments' K3 generation SoCs has an IP Interrupt Aggregator
> which is an interrupt controller that does the following:
> - Converts events to interrupts that can be understood by
>   an interrupt router.
> - Allows for multiplexing of events to interrupts.
> 
> Configuration of the interrupt aggregator registers can only be done by
> a system co-processor and the driver needs to send a message to this
> co processor over TISCI protocol.
> 
> Add support for Interrupt Aggregator driver with 2 IRQ Domains:
> - INTA MSI domain that interfaces the devices using which interrupts can be
>   allocated dynamically.
> - INTA domain that is parent to the MSI domain that manages the interrupt
>   resources.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
> Changes since v5:
> - Moved all the allocation of events and parent irqs to .irq_request_resources
>   callback of irqchip. This is based on the offline discussion with Marc.
> 
> Marc,
> 	This version has a deviation from our discussion:
> - Could not create separate irq_domains for each output(vint) of INTA, instead
>   stick to a single irq_domain for the entire INTA. Because when creating
>   msi_domain there is no parent available to attach. Even then I create
>   irq_domains for all the available vints during probe, how do we decide
>   which is the parent of msi_domain?
> 
> 
>  MAINTAINERS                       |   1 +
>  drivers/irqchip/Kconfig           |  11 +
>  drivers/irqchip/Makefile          |   1 +
>  drivers/irqchip/irq-ti-sci-inta.c | 622 ++++++++++++++++++++++++++++++
>  4 files changed, 635 insertions(+)
>  create mode 100644 drivers/irqchip/irq-ti-sci-inta.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 90173038f674..ba88b3033fe4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15352,6 +15352,7 @@ F:	drivers/reset/reset-ti-sci.c
>  F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>  F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt
>  F:	drivers/irqchip/irq-ti-sci-intr.c
> +F:	drivers/irqchip/irq-ti-sci-inta.c
>  
>  Texas Instruments ASoC drivers
>  M:	Peter Ujfalusi <peter.ujfalusi@ti.com>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index a1ff64c1d40d..946c062fcec0 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -436,6 +436,17 @@ config TI_SCI_INTR_IRQCHIP
>  	  If you wish to use interrupt router irq resources managed by the
>  	  TI System Controller, say Y here. Otherwise, say N.
>  
> +config TI_SCI_INTA_IRQCHIP
> +	bool
> +	depends on TI_SCI_PROTOCOL && ARCH_K3
> +	select IRQ_DOMAIN
> +	select IRQ_DOMAIN_HIERARCHY
> +	help
> +	  This enables the irqchip driver support for K3 Interrupt aggregator
> +	  over TI System Control Interface available on some new TI's SoCs.
> +	  If you wish to use interrupt aggregator irq resources managed by the
> +	  TI System Controller, say Y here. Otherwise, say N.
> +
>  endmenu
>  
>  config SIFIVE_PLIC
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index fa5c865788b5..8a33013da953 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -98,3 +98,4 @@ obj-$(CONFIG_IMX_IRQSTEER)		+= irq-imx-irqsteer.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
> +obj-$(CONFIG_TI_SCI_INTA_IRQCHIP)	+= irq-ti-sci-inta.o
> diff --git a/drivers/irqchip/irq-ti-sci-inta.c b/drivers/irqchip/irq-ti-sci-inta.c
> new file mode 100644
> index 000000000000..3eb935ebe10f
> --- /dev/null
> +++ b/drivers/irqchip/irq-ti-sci-inta.c
> @@ -0,0 +1,622 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Texas Instruments' K3 Interrupt Aggregator irqchip driver
> + *
> + * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
> + *	Lokesh Vutla <lokeshvutla@ti.com>
> + */
> +
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/msi.h>
> +#include <linux/irqchip.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/irqdomain.h>
> +#include <linux/interrupt.h>
> +#include <linux/soc/ti/ti_sci_protocol.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <asm-generic/msi.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 MAX_EVENTS_PER_VINT	64
> +#define VINT_ENABLE_SET_OFFSET	0x0
> +#define VINT_ENABLE_CLR_OFFSET	0x8
> +#define VINT_STATUS_OFFSET	0x18
> +
> +/**
> + * struct ti_sci_inta_event_desc - Description of an event coming to
> + *				   Interrupt Aggregator. This serves
> + *				   as a mapping table between global
> + *				   event and hwirq.
> + * @global_event:	Global event number corresponding to this event
> + * @hwirq:		Hwirq of the incoming interrupt
> + */
> +struct ti_sci_inta_event_desc {
> +	u16 global_event;
> +	u32 hwirq;
> +};
> +
> +/**
> + * struct ti_sci_inta_vint_desc - Description of a virtual interrupt coming out
> + *				  of Interrupt Aggregator.
> + * @domain:		Pointer to IRQ domain to which this vint belongs.
> + * @list:		List entry for the vint list
> + * @event_map:		Bitmap to manage the allocation of events to vint.
> + * @event_mutex:	Mutex to protect allocation of events.
> + * @events:		Array of event descriptors assigned to this vint.
> + * @parent_virq:	Linux IRQ number that gets attached to parent
> + * @vint_id:		TISCI vint ID
> + */
> +struct ti_sci_inta_vint_desc {
> +	struct irq_domain *domain;
> +	struct list_head list;
> +	unsigned long *event_map;
> +	/* Mutex to protect allocation of events */
> +	struct mutex event_mutex;
> +	struct ti_sci_inta_event_desc events[MAX_EVENTS_PER_VINT];
> +	unsigned int parent_virq;
> +	u16 vint_id;
> +};
> +
> +/**
> + * struct ti_sci_inta_irq_domain - Structure representing a TISCI based
> + *				   Interrupt Aggregator IRQ domain.
> + * @sci:		Pointer to TISCI handle
> + * @vint:		TISCI resource pointer representing IA inerrupts.
> + * @global_event:	TISCI resource pointer representing global events.
> + * @vint_list:		List of the vints active in the system
> + * @vint_mutex:		Mutex to protect vint_list
> + * @base:		Base address of the memory mapped IO registers
> + * @pdev:		Pointer to platform device.
> + */
> +struct ti_sci_inta_irq_domain {
> +	const struct ti_sci_handle *sci;
> +	struct ti_sci_resource *vint;
> +	struct ti_sci_resource *global_event;
> +	struct list_head vint_list;
> +	/* Mutex to protect vint list */
> +	struct mutex vint_mutex;
> +	void __iomem *base;
> +	struct platform_device	*pdev;
> +};
> +
> +/**
> + * ti_sci_inta_irq_handler() - Chained IRQ handler for the vint irqs
> + * @desc:	Pointer to irq_desc corresponding to the irq
> + */
> +static void ti_sci_inta_irq_handler(struct irq_desc *desc)
> +{
> +	struct ti_sci_inta_vint_desc *vint_desc;
> +	struct ti_sci_inta_irq_domain *inta;
> +	struct irq_domain *domain;
> +	unsigned int virq, bit;
> +	u64 val;
> +
> +	vint_desc = irq_desc_get_handler_data(desc);
> +	domain = vint_desc->domain;
> +	inta = domain->host_data;
> +
> +	chained_irq_enter(irq_desc_get_chip(desc), desc);
> +
> +	val = readq_relaxed(inta->base + vint_desc->vint_id * 0x1000 +
> +			    VINT_STATUS_OFFSET);
> +
> +	for (bit = 0; bit < MAX_EVENTS_PER_VINT; bit++) {
> +		if (BIT(bit) & val) {
> +			virq = irq_find_mapping(domain,
> +						vint_desc->events[bit].hwirq);
> +			if (virq)
> +				generic_handle_irq(virq);
> +		}
> +	}
> +
> +	chained_irq_exit(irq_desc_get_chip(desc), desc);
> +}
> +
> +/**
> + * ti_sci_inta_alloc_parent_irq() - Allocate parent irq to Interrupt aggregator
> + * @domain:	IRQ domain corresponding to Interrupt Aggregator
> + *
> + * Return 0 if all went well else corresponding error value.
> + */
> +static struct ti_sci_inta_vint_desc
> +*ti_sci_inta_alloc_parent_irq(struct irq_domain *domain)
> +{
> +	struct ti_sci_inta_irq_domain *inta = domain->host_data;
> +	struct ti_sci_inta_vint_desc *vint_desc;
> +	struct irq_fwspec parent_fwspec;
> +	u16 vint_id;
> +
> +	vint_id = ti_sci_get_free_resource(inta->vint);
> +	if (vint_id == TI_SCI_RESOURCE_NULL)
> +		return ERR_PTR(-EINVAL);
> +
> +	vint_desc = kzalloc(sizeof(*vint_desc), GFP_KERNEL);
> +	if (!vint_desc)
> +		return ERR_PTR(-ENOMEM);
> +
> +	vint_desc->event_map = kcalloc(BITS_TO_LONGS(MAX_EVENTS_PER_VINT),
> +				       sizeof(*vint_desc->event_map),
> +				       GFP_KERNEL);
> +	if (!vint_desc->event_map) {
> +		kfree(vint_desc);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +	mutex_init(&vint_desc->event_mutex);
> +
> +	vint_desc->domain = domain;
> +	vint_desc->vint_id = vint_id;
> +	INIT_LIST_HEAD(&vint_desc->list);
> +
> +	mutex_lock(&inta->vint_mutex);
> +	list_add_tail(&vint_desc->list, &inta->vint_list);
> +	mutex_unlock(&inta->vint_mutex);
> +
> +	parent_fwspec.fwnode =
> +	of_node_to_fwnode(of_irq_find_parent(dev_of_node(&inta->pdev->dev)));
> +	parent_fwspec.param_count = 3;
> +	parent_fwspec.param[0] = inta->pdev->id;
> +	parent_fwspec.param[1] = vint_desc->vint_id;
> +	parent_fwspec.param[2] = 1;
> +
> +	vint_desc->parent_virq = irq_create_fwspec_mapping(&parent_fwspec);
> +	if (vint_desc->parent_virq <= 0)
> +		return ERR_PTR(vint_desc->parent_virq);
> +
> +	irq_set_chained_handler_and_data(vint_desc->parent_virq,
> +					 ti_sci_inta_irq_handler, vint_desc);
> +
> +	return vint_desc;
> +}
> +
> +/**
> + * ti_sci_inta_alloc_event() - Attach an event to a IA vint.
> + * @inta:	Pointer to INTA domain descriptor
> + * @vint_desc:	Pointer to vint_desc to which the event gets attached
> + * @free_bit:	Bit inside vint to which event gets attached
> + * @hwirq:	hwirq of the input event
> + *
> + * Return global_event if all went ok else appropriate error value.
> + */
> +static
> +int ti_sci_inta_alloc_event(struct ti_sci_inta_irq_domain *inta,
> +			    struct ti_sci_inta_vint_desc *vint_desc,
> +			    u16 free_bit, u32 hwirq)
> +{
> +	struct ti_sci_inta_event_desc *event_desc;
> +	u16 dev_id, dev_index;
> +	int err;
> +
> +	dev_id = HWIRQ_TO_DEVID(hwirq);
> +	dev_index = HWIRQ_TO_IRQID(hwirq);
> +	event_desc = &vint_desc->events[free_bit];
> +	mutex_lock(&vint_desc->event_mutex);
> +	event_desc->hwirq = hwirq;
> +	event_desc->global_event = ti_sci_get_free_resource(inta->global_event);
> +	if (event_desc->global_event == TI_SCI_RESOURCE_NULL) {
> +		err = -EINVAL;
> +		goto free_vint_bit;
> +	}
> +
> +	err = inta->sci->ops.rm_irq_ops.set_event_map(inta->sci,
> +						      dev_id, dev_index,
> +						      inta->pdev->id,
> +						      vint_desc->vint_id,
> +						      event_desc->global_event,
> +						      free_bit);
> +	if (err)
> +		goto free_global_event;
> +
> +	mutex_unlock(&vint_desc->event_mutex);
> +	return 0;
> +free_global_event:
> +	ti_sci_release_resource(inta->global_event, event_desc->global_event);
> +free_vint_bit:
> +	clear_bit(free_bit, vint_desc->event_map);
> +	mutex_unlock(&vint_desc->event_mutex);
> +	return err;
> +}
> +
> +/**
> + * ti_sci_inta_alloc_irq() -  Allocate an irq within INTA domain
> + * @domain:	irq_domain pointer corresponding to INTA
> + * @hwirq:	hwirq of the input event
> + *
> + * Note: Allocation happens in the following manner:
> + *	- Find a free bit available in any of the vints available in the list.
> + *	- If not found, allocate a vint from the vint pool
> + *	- Attach the free bit to input hwirq.
> + * Return vint_desc if all went ok else appropriate error value.
> + */
> +static struct ti_sci_inta_vint_desc
> +*ti_sci_inta_alloc_irq(struct irq_domain *domain, u32 hwirq)
> +{
> +	struct ti_sci_inta_irq_domain *inta = domain->host_data;
> +	struct ti_sci_inta_vint_desc *vint_desc = NULL;
> +	u16 free_bit;
> +	int ret;
> +
> +	mutex_lock(&inta->vint_mutex);
> +	list_for_each_entry(vint_desc, &inta->vint_list, list) {
> +		mutex_lock(&vint_desc->event_mutex);
> +		free_bit = find_first_zero_bit(vint_desc->event_map,
> +					       MAX_EVENTS_PER_VINT);
> +		if (free_bit != MAX_EVENTS_PER_VINT) {
> +			set_bit(free_bit, vint_desc->event_map);
> +			mutex_unlock(&vint_desc->event_mutex);
> +			mutex_unlock(&inta->vint_mutex);
> +			goto alloc_event;
> +		}
> +		mutex_unlock(&vint_desc->event_mutex);
> +	}
> +	mutex_unlock(&inta->vint_mutex);
> +
> +	/* No free bits available. Allocate a new vint */
> +	vint_desc = ti_sci_inta_alloc_parent_irq(domain);
> +	if (IS_ERR(vint_desc))
> +		return vint_desc;
> +
> +	mutex_lock(&vint_desc->event_mutex);
> +	free_bit = find_first_zero_bit(vint_desc->event_map,
> +				       MAX_EVENTS_PER_VINT);
> +	set_bit(free_bit, vint_desc->event_map);
> +	mutex_unlock(&vint_desc->event_mutex);
> +
> +alloc_event:
> +	ret = ti_sci_inta_alloc_event(inta, vint_desc, free_bit, hwirq);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	return vint_desc;
> +}
> +
> +/**
> + * __get_event_index() - Convert the hwirq to corresponding bit inside vint.
> + * @vint_desc:	Pointer to vint_desc to which the hwirq is attached
> + * @hwirq:	hwirq number within the domain
> + *
> + * Return the vint bit to which the hwirq is attached.
> + */
> +static int __get_event_index(struct ti_sci_inta_vint_desc *vint_desc, u32 hwirq)
> +{
> +	int i;
> +
> +	mutex_lock(&vint_desc->event_mutex);

This should have been spinlock as this function gets called in atomic context.
Thanks Grygorii for pointing it. Will take care of this in next version.

Thanks and regards,
Lokesh

> +	for (i = 0; i < MAX_EVENTS_PER_VINT; i++)
> +		if (vint_desc->events[i].hwirq == hwirq) {
> +			mutex_unlock(&vint_desc->event_mutex);
> +			return i;
> +		}
> +
> +	mutex_unlock(&vint_desc->event_mutex);
> +	return -ENODEV;
> +}
> +
> +/**
> + * ti_sci_inta_free_parent_irq() - Free a parent irq to INTA
> + * domain:	domain corresponding to INTA
> + */
> +static void ti_sci_inta_free_parent_irq(struct irq_domain *domain,
> +					struct ti_sci_inta_vint_desc *vint_desc)
> +{
> +	struct ti_sci_inta_irq_domain *inta = domain->host_data;
> +
> +	mutex_lock(&inta->vint_mutex);
> +	list_del(&vint_desc->list);
> +	mutex_unlock(&inta->vint_mutex);
> +	irq_dispose_mapping(vint_desc->parent_virq);
> +	ti_sci_release_resource(inta->vint, vint_desc->vint_id);
> +	kfree(vint_desc->event_map);
> +	kfree(vint_desc);
> +}
> +
> +/**
> + * ti_sci_inta_free_irq() - Free an IRQ within INTA domain
> + * domain:	Domain corresponding to INTA
> + * vint_desc:	Pointer to vint_desc from which irq needs to be freed
> + * hwirq:	Hwirq number within INTA domain that needs to be freed
> + */
> +static void ti_sci_inta_free_irq(struct irq_domain *domain,
> +				 struct ti_sci_inta_vint_desc *vint_desc,
> +				 u32 hwirq)
> +{
> +	struct ti_sci_inta_irq_domain *inta = domain->host_data;
> +	struct ti_sci_inta_event_desc *event_desc;
> +	int event_index = 0;
> +
> +	/* free event irq */
> +	event_index = __get_event_index(vint_desc, hwirq);
> +	event_desc = &vint_desc->events[event_index];
> +	mutex_lock(&vint_desc->event_mutex);
> +	inta->sci->ops.rm_irq_ops.free_event_map(inta->sci,
> +						 HWIRQ_TO_DEVID(hwirq),
> +						 HWIRQ_TO_IRQID(hwirq),
> +						 inta->pdev->id,
> +						 vint_desc->vint_id,
> +						 event_desc->global_event,
> +						 event_index);
> +
> +	clear_bit(event_index, vint_desc->event_map);
> +	ti_sci_release_resource(inta->global_event, event_desc->global_event);
> +	event_desc->global_event = TI_SCI_RESOURCE_NULL;
> +	event_desc->hwirq = 0;
> +	mutex_unlock(&vint_desc->event_mutex);
> +
> +	/* Free parent irq */
> +	if (find_first_bit(vint_desc->event_map, MAX_EVENTS_PER_VINT) ==
> +	    MAX_EVENTS_PER_VINT)
> +		ti_sci_inta_free_parent_irq(domain, vint_desc);
> +}
> +
> +/**
> + * ti_sci_inta_request_resources() - Allocate resources for input irq
> + * @data: Pointer to corresponding irq_data
> + *
> + * Note: This is the core api where the actual allocation happens for input
> + *	 hwirq. This allocation involves creating a parent irq for vint.
> + *	 If this is done in irq_domain_ops.alloc() then a deadlock is reached
> + *	 for allocation. So this allocation is being done in request_resources()
> + *
> + * Return: 0 if all went well else corresponding error.
> + */
> +static int ti_sci_inta_request_resources(struct irq_data *data)
> +{
> +	struct ti_sci_inta_vint_desc *vint_desc;
> +
> +	vint_desc = ti_sci_inta_alloc_irq(data->domain, data->hwirq);
> +	if (IS_ERR(vint_desc))
> +		return PTR_ERR(vint_desc);
> +
> +	data->chip_data = vint_desc;
> +
> +	return 0;
> +}
> +
> +/**
> + * ti_sci_inta_release_resources - Release resources for input irq
> + * @data: Pointer to corresponding irq_data
> + *
> + * Note: Corresponding to request_resources(), all the unmapping and deletion
> + *	 of parent vint irqs happens in this api.
> + */
> +static void ti_sci_inta_release_resources(struct irq_data *data)
> +{
> +	struct ti_sci_inta_vint_desc *vint_desc;
> +
> +	vint_desc = irq_data_get_irq_chip_data(data);
> +	ti_sci_inta_free_irq(data->domain, vint_desc, data->hwirq);
> +}
> +
> +/**
> + * __ti_sci_inta_manage_event() - Control the event based on the offset
> + * @data:	Pointer to corresponding irq_data
> + * @offset:	register offset using which event is controlled.
> + */
> +static void __ti_sci_inta_manage_event(struct irq_data *data, u32 offset)
> +{
> +	struct ti_sci_inta_vint_desc *vint_desc;
> +	struct ti_sci_inta_irq_domain *inta;
> +	int event_index;
> +
> +	vint_desc = irq_data_get_irq_chip_data(data);
> +	inta = data->domain->host_data;
> +	event_index = __get_event_index(vint_desc, data->hwirq);
> +	if (event_index < 0)
> +		return;
> +
> +	writeq_relaxed(BIT(event_index), inta->base +
> +		       vint_desc->vint_id * 0x1000 + offset);
> +}
> +
> +/**
> + * ti_sci_inta_mask_irq() - Mask an event
> + * @data:	Pointer to corresponding irq_data
> + */
> +static void ti_sci_inta_mask_irq(struct irq_data *data)
> +{
> +	__ti_sci_inta_manage_event(data, VINT_ENABLE_CLR_OFFSET);
> +}
> +
> +/**
> + * ti_sci_inta_unmask_irq() - Unmask an event
> + * @data:	Pointer to corresponding irq_data
> + */
> +static void ti_sci_inta_unmask_irq(struct irq_data *data)
> +{
> +	__ti_sci_inta_manage_event(data, VINT_ENABLE_SET_OFFSET);
> +}
> +
> +/**
> + * ti_sci_inta_ack_irq() - Ack an event
> + * @data:	Pointer to corresponding irq_data
> + */
> +static void ti_sci_inta_ack_irq(struct irq_data *data)
> +{
> +	__ti_sci_inta_manage_event(data, VINT_STATUS_OFFSET);
> +}
> +
> +static int ti_sci_inta_set_affinity(struct irq_data *d,
> +				    const struct cpumask *mask_val, bool force)
> +{
> +	return -EINVAL;
> +}
> +
> +/**
> + * ti_sci_inta_set_type() - Update the trigger type of the irq.
> + * @data:	Pointer to corresponding irq_data
> + * @type:	Trigger type as specified by user
> + *
> + * Note: This updates the handle_irq callback for level msi.
> + *
> + * Return 0 if all went well else appropriate error.
> + */
> +static int ti_sci_inta_set_type(struct irq_data *data, unsigned int type)
> +{
> +	struct irq_desc *desc = irq_to_desc(data->irq);
> +
> +	/*
> +	 * .alloc default sets handle_edge_irq. But if the user specifies
> +	 * that IRQ is level MSI, then update the handle to handle_level_irq
> +	 */
> +	if (type & IRQF_TRIGGER_HIGH)
> +		desc->handle_irq = handle_level_irq;
> +
> +	return 0;
> +}
> +
> +static struct irq_chip ti_sci_inta_irq_chip = {
> +	.name			= "INTA",
> +	.irq_mask		= ti_sci_inta_mask_irq,
> +	.irq_unmask		= ti_sci_inta_unmask_irq,
> +	.irq_ack		= ti_sci_inta_ack_irq,
> +	.irq_set_affinity	= ti_sci_inta_set_affinity,
> +	.irq_request_resources	= ti_sci_inta_request_resources,
> +	.irq_release_resources	= ti_sci_inta_release_resources,
> +	.irq_set_type		= ti_sci_inta_set_type,
> +};
> +
> +/**
> + * ti_sci_inta_irq_domain_free() - Free an IRQ from the IRQ domain
> + * @domain:	Domain to which the irqs belong
> + * @virq:	base linux virtual IRQ to be freed.
> + * @nr_irqs:	Number of continuous irqs to be freed
> + */
> +static void ti_sci_inta_irq_domain_free(struct irq_domain *domain,
> +					unsigned int virq, unsigned int nr_irqs)
> +{
> +	struct irq_data *data = irq_domain_get_irq_data(domain, virq);
> +
> +	irq_domain_reset_irq_data(data);
> +}
> +
> +/**
> + * ti_sci_inta_irq_domain_alloc() - Allocate Interrupt aggregator IRQs
> + * @domain:	Point to the interrupt aggregator IRQ domain
> + * @virq:	Corresponding Linux virtual IRQ number
> + * @nr_irqs:	Continuous irqs to be allocated
> + * @data:	Pointer to firmware specifier
> + *
> + * No actual allocation happens here.
> + *
> + * Return 0 if all went well else appropriate error value.
> + */
> +static int ti_sci_inta_irq_domain_alloc(struct irq_domain *domain,
> +					unsigned int virq, unsigned int nr_irqs,
> +					void *data)
> +{
> +	msi_alloc_info_t *arg = data;
> +
> +	irq_domain_set_info(domain, virq, arg->hwirq, &ti_sci_inta_irq_chip,
> +			    NULL, handle_edge_irq, NULL, NULL);
> +
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops ti_sci_inta_irq_domain_ops = {
> +	.alloc		= ti_sci_inta_irq_domain_alloc,
> +	.free		= ti_sci_inta_irq_domain_free,
> +};
> +
> +static int ti_sci_inta_irq_domain_probe(struct platform_device *pdev)
> +{
> +	struct irq_domain *parent_domain, *domain;
> +	struct device_node *parent_node, *node;
> +	struct ti_sci_inta_irq_domain *inta;
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	int ret;
> +
> +	node = dev_of_node(dev);
> +	parent_node = of_irq_find_parent(node);
> +	if (!parent_node) {
> +		dev_err(dev, "Failed to get IRQ parent node\n");
> +		return -ENODEV;
> +	}
> +
> +	parent_domain = irq_find_host(parent_node);
> +	if (!parent_domain)
> +		return -EPROBE_DEFER;
> +
> +	inta = devm_kzalloc(dev, sizeof(*inta), GFP_KERNEL);
> +	if (!inta)
> +		return -ENOMEM;
> +
> +	inta->pdev = pdev;
> +	inta->sci = devm_ti_sci_get_by_phandle(dev, "ti,sci");
> +	if (IS_ERR(inta->sci)) {
> +		ret = PTR_ERR(inta->sci);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "ti,sci read fail %d\n", ret);
> +		inta->sci = NULL;
> +		return ret;
> +	}
> +
> +	ret = of_property_read_u32(dev->of_node, "ti,sci-dev-id", &pdev->id);
> +	if (ret) {
> +		dev_err(dev, "missing 'ti,sci-dev-id' property\n");
> +		return -EINVAL;
> +	}
> +
> +	inta->vint = devm_ti_sci_get_of_resource(inta->sci, dev, pdev->id,
> +						 "ti,sci-rm-range-vint");
> +	if (IS_ERR(inta->vint)) {
> +		dev_err(dev, "VINT resource allocation failed\n");
> +		return PTR_ERR(inta->vint);
> +	}
> +
> +	inta->global_event =
> +		devm_ti_sci_get_of_resource(inta->sci, dev, pdev->id,
> +					    "ti,sci-rm-range-global-event");
> +	if (IS_ERR(inta->global_event)) {
> +		dev_err(dev, "Global event resource allocation failed\n");
> +		return PTR_ERR(inta->global_event);
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	inta->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(inta->base))
> +		return -ENODEV;
> +
> +	domain = irq_domain_add_linear(dev_of_node(dev),
> +				       ti_sci_get_num_resources(inta->vint),
> +				       &ti_sci_inta_irq_domain_ops, inta);
> +	if (!domain) {
> +		dev_err(dev, "Failed to allocate IRQ domain\n");
> +		return -ENOMEM;
> +	}
> +
> +	INIT_LIST_HEAD(&inta->vint_list);
> +	mutex_init(&inta->vint_mutex);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ti_sci_inta_irq_domain_of_match[] = {
> +	{ .compatible = "ti,sci-inta", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, ti_sci_inta_irq_domain_of_match);
> +
> +static struct platform_driver ti_sci_inta_irq_domain_driver = {
> +	.probe = ti_sci_inta_irq_domain_probe,
> +	.driver = {
> +		.name = "ti-sci-inta",
> +		.of_match_table = ti_sci_inta_irq_domain_of_match,
> +	},
> +};
> +module_platform_driver(ti_sci_inta_irq_domain_driver);
> +
> +MODULE_AUTHOR("Lokesh Vutla <lokeshvutla@ticom>");
> +MODULE_DESCRIPTION("K3 Interrupt Aggregator driver over TI SCI protocol");
> +MODULE_LICENSE("GPL v2");
> 

Re: [PATCH v6 09/12] irqchip: ti-sci-inta: Add support for Interrupt Aggregator driver

[Marc Zyngier]

Hi Lokesh,

On 10/04/2019 05:13, Lokesh Vutla wrote:
> Texas Instruments' K3 generation SoCs has an IP Interrupt Aggregator
> which is an interrupt controller that does the following:
> - Converts events to interrupts that can be understood by
>   an interrupt router.
> - Allows for multiplexing of events to interrupts.
> 
> Configuration of the interrupt aggregator registers can only be done by
> a system co-processor and the driver needs to send a message to this
> co processor over TISCI protocol.
> 
> Add support for Interrupt Aggregator driver with 2 IRQ Domains:
> - INTA MSI domain that interfaces the devices using which interrupts can be
>   allocated dynamically.
> - INTA domain that is parent to the MSI domain that manages the interrupt
>   resources.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
> Changes since v5:
> - Moved all the allocation of events and parent irqs to .irq_request_resources
>   callback of irqchip. This is based on the offline discussion with Marc.
> 
> Marc,
> 	This version has a deviation from our discussion:
> - Could not create separate irq_domains for each output(vint) of INTA, instead
>   stick to a single irq_domain for the entire INTA. Because when creating
>   msi_domain there is no parent available to attach. Even then I create
>   irq_domains for all the available vints during probe, how do we decide
>   which is the parent of msi_domain?

Yeah, you're right. The top-down allocation assumes that we already have
setup the domain hierarchy, and this delayed allocation scheme breaks
it. Oh well, never mind.

> 
> 
>  MAINTAINERS                       |   1 +
>  drivers/irqchip/Kconfig           |  11 +
>  drivers/irqchip/Makefile          |   1 +
>  drivers/irqchip/irq-ti-sci-inta.c | 622 ++++++++++++++++++++++++++++++
>  4 files changed, 635 insertions(+)
>  create mode 100644 drivers/irqchip/irq-ti-sci-inta.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 90173038f674..ba88b3033fe4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15352,6 +15352,7 @@ F:	drivers/reset/reset-ti-sci.c
>  F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>  F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt
>  F:	drivers/irqchip/irq-ti-sci-intr.c
> +F:	drivers/irqchip/irq-ti-sci-inta.c
>  
>  Texas Instruments ASoC drivers
>  M:	Peter Ujfalusi <peter.ujfalusi@ti.com>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index a1ff64c1d40d..946c062fcec0 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -436,6 +436,17 @@ config TI_SCI_INTR_IRQCHIP
>  	  If you wish to use interrupt router irq resources managed by the
>  	  TI System Controller, say Y here. Otherwise, say N.
>  
> +config TI_SCI_INTA_IRQCHIP
> +	bool
> +	depends on TI_SCI_PROTOCOL && ARCH_K3
> +	select IRQ_DOMAIN
> +	select IRQ_DOMAIN_HIERARCHY
> +	help
> +	  This enables the irqchip driver support for K3 Interrupt aggregator
> +	  over TI System Control Interface available on some new TI's SoCs.
> +	  If you wish to use interrupt aggregator irq resources managed by the
> +	  TI System Controller, say Y here. Otherwise, say N.
> +
>  endmenu
>  
>  config SIFIVE_PLIC
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index fa5c865788b5..8a33013da953 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -98,3 +98,4 @@ obj-$(CONFIG_IMX_IRQSTEER)		+= irq-imx-irqsteer.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
> +obj-$(CONFIG_TI_SCI_INTA_IRQCHIP)	+= irq-ti-sci-inta.o
> diff --git a/drivers/irqchip/irq-ti-sci-inta.c b/drivers/irqchip/irq-ti-sci-inta.c
> new file mode 100644
> index 000000000000..3eb935ebe10f
> --- /dev/null
> +++ b/drivers/irqchip/irq-ti-sci-inta.c
> @@ -0,0 +1,622 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Texas Instruments' K3 Interrupt Aggregator irqchip driver
> + *
> + * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
> + *	Lokesh Vutla <lokeshvutla@ti.com>
> + */
> +
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/msi.h>
> +#include <linux/irqchip.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/irqdomain.h>
> +#include <linux/interrupt.h>
> +#include <linux/soc/ti/ti_sci_protocol.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <asm-generic/msi.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 MAX_EVENTS_PER_VINT	64
> +#define VINT_ENABLE_SET_OFFSET	0x0
> +#define VINT_ENABLE_CLR_OFFSET	0x8
> +#define VINT_STATUS_OFFSET	0x18
> +
> +/**
> + * struct ti_sci_inta_event_desc - Description of an event coming to
> + *				   Interrupt Aggregator. This serves
> + *				   as a mapping table between global
> + *				   event and hwirq.
> + * @global_event:	Global event number corresponding to this event
> + * @hwirq:		Hwirq of the incoming interrupt
> + */
> +struct ti_sci_inta_event_desc {
> +	u16 global_event;
> +	u32 hwirq;
> +};
> +
> +/**
> + * struct ti_sci_inta_vint_desc - Description of a virtual interrupt coming out
> + *				  of Interrupt Aggregator.
> + * @domain:		Pointer to IRQ domain to which this vint belongs.
> + * @list:		List entry for the vint list
> + * @event_map:		Bitmap to manage the allocation of events to vint.
> + * @event_mutex:	Mutex to protect allocation of events.
> + * @events:		Array of event descriptors assigned to this vint.
> + * @parent_virq:	Linux IRQ number that gets attached to parent
> + * @vint_id:		TISCI vint ID
> + */
> +struct ti_sci_inta_vint_desc {
> +	struct irq_domain *domain;
> +	struct list_head list;
> +	unsigned long *event_map;
> +	/* Mutex to protect allocation of events */
> +	struct mutex event_mutex;

As mentioned in another email, this needs to be a spinlock, as the lock
may be taken in atomic context.

> +	struct ti_sci_inta_event_desc events[MAX_EVENTS_PER_VINT];
> +	unsigned int parent_virq;
> +	u16 vint_id;
> +};
> +
> +/**
> + * struct ti_sci_inta_irq_domain - Structure representing a TISCI based
> + *				   Interrupt Aggregator IRQ domain.
> + * @sci:		Pointer to TISCI handle
> + * @vint:		TISCI resource pointer representing IA inerrupts.
> + * @global_event:	TISCI resource pointer representing global events.
> + * @vint_list:		List of the vints active in the system
> + * @vint_mutex:		Mutex to protect vint_list
> + * @base:		Base address of the memory mapped IO registers
> + * @pdev:		Pointer to platform device.
> + */
> +struct ti_sci_inta_irq_domain {
> +	const struct ti_sci_handle *sci;
> +	struct ti_sci_resource *vint;
> +	struct ti_sci_resource *global_event;
> +	struct list_head vint_list;
> +	/* Mutex to protect vint list */
> +	struct mutex vint_mutex;
> +	void __iomem *base;
> +	struct platform_device	*pdev;
> +};
> +
> +/**
> + * ti_sci_inta_irq_handler() - Chained IRQ handler for the vint irqs
> + * @desc:	Pointer to irq_desc corresponding to the irq
> + */
> +static void ti_sci_inta_irq_handler(struct irq_desc *desc)
> +{
> +	struct ti_sci_inta_vint_desc *vint_desc;
> +	struct ti_sci_inta_irq_domain *inta;
> +	struct irq_domain *domain;
> +	unsigned int virq, bit;
> +	u64 val;
> +
> +	vint_desc = irq_desc_get_handler_data(desc);
> +	domain = vint_desc->domain;
> +	inta = domain->host_data;
> +
> +	chained_irq_enter(irq_desc_get_chip(desc), desc);
> +
> +	val = readq_relaxed(inta->base + vint_desc->vint_id * 0x1000 +
> +			    VINT_STATUS_OFFSET);
> +
> +	for (bit = 0; bit < MAX_EVENTS_PER_VINT; bit++) {
> +		if (BIT(bit) & val) {
> +			virq = irq_find_mapping(domain,
> +						vint_desc->events[bit].hwirq);
> +			if (virq)
> +				generic_handle_irq(virq);
> +		}
> +	}
> +
> +	chained_irq_exit(irq_desc_get_chip(desc), desc);
> +}
> +
> +/**
> + * ti_sci_inta_alloc_parent_irq() - Allocate parent irq to Interrupt aggregator
> + * @domain:	IRQ domain corresponding to Interrupt Aggregator
> + *
> + * Return 0 if all went well else corresponding error value.
> + */
> +static struct ti_sci_inta_vint_desc
> +*ti_sci_inta_alloc_parent_irq(struct irq_domain *domain)

nit: do not split lines like this, specially not with the * on a
different line than the type it applies to.

> +{
> +	struct ti_sci_inta_irq_domain *inta = domain->host_data;
> +	struct ti_sci_inta_vint_desc *vint_desc;
> +	struct irq_fwspec parent_fwspec;
> +	u16 vint_id;
> +
> +	vint_id = ti_sci_get_free_resource(inta->vint);
> +	if (vint_id == TI_SCI_RESOURCE_NULL)
> +		return ERR_PTR(-EINVAL);
> +
> +	vint_desc = kzalloc(sizeof(*vint_desc), GFP_KERNEL);
> +	if (!vint_desc)
> +		return ERR_PTR(-ENOMEM);
> +
> +	vint_desc->event_map = kcalloc(BITS_TO_LONGS(MAX_EVENTS_PER_VINT),
> +				       sizeof(*vint_desc->event_map),
> +				       GFP_KERNEL);

You already have an array of MAX_EVENTS_PER_VINT (events) in this
structure. Why don't you simply declare it as a
MAX_EVENTS_PER_VINT-sized bitmap and save yourself the allocation?

> +	if (!vint_desc->event_map) {
> +		kfree(vint_desc);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +	mutex_init(&vint_desc->event_mutex);
> +
> +	vint_desc->domain = domain;
> +	vint_desc->vint_id = vint_id;
> +	INIT_LIST_HEAD(&vint_desc->list);
> +
> +	mutex_lock(&inta->vint_mutex);
> +	list_add_tail(&vint_desc->list, &inta->vint_list);
> +	mutex_unlock(&inta->vint_mutex);
> +
> +	parent_fwspec.fwnode =
> +	of_node_to_fwnode(of_irq_find_parent(dev_of_node(&inta->pdev->dev)));

single line.

> +	parent_fwspec.param_count = 3;
> +	parent_fwspec.param[0] = inta->pdev->id;
> +	parent_fwspec.param[1] = vint_desc->vint_id;
> +	parent_fwspec.param[2] = 1;
> +
> +	vint_desc->parent_virq = irq_create_fwspec_mapping(&parent_fwspec);
> +	if (vint_desc->parent_virq <= 0)
> +		return ERR_PTR(vint_desc->parent_virq);

Don't you need to free vint_desc (and its event_map) here?

> +
> +	irq_set_chained_handler_and_data(vint_desc->parent_virq,
> +					 ti_sci_inta_irq_handler, vint_desc);
> +
> +	return vint_desc;
> +}
> +
> +/**
> + * ti_sci_inta_alloc_event() - Attach an event to a IA vint.
> + * @inta:	Pointer to INTA domain descriptor
> + * @vint_desc:	Pointer to vint_desc to which the event gets attached
> + * @free_bit:	Bit inside vint to which event gets attached
> + * @hwirq:	hwirq of the input event
> + *
> + * Return global_event if all went ok else appropriate error value.
> + */
> +static
> +int ti_sci_inta_alloc_event(struct ti_sci_inta_irq_domain *inta,
> +			    struct ti_sci_inta_vint_desc *vint_desc,
> +			    u16 free_bit, u32 hwirq)
> +{
> +	struct ti_sci_inta_event_desc *event_desc;
> +	u16 dev_id, dev_index;
> +	int err;
> +
> +	dev_id = HWIRQ_TO_DEVID(hwirq);
> +	dev_index = HWIRQ_TO_IRQID(hwirq);
> +	event_desc = &vint_desc->events[free_bit];
> +	mutex_lock(&vint_desc->event_mutex);
> +	event_desc->hwirq = hwirq;
> +	event_desc->global_event = ti_sci_get_free_resource(inta->global_event);
> +	if (event_desc->global_event == TI_SCI_RESOURCE_NULL) {
> +		err = -EINVAL;
> +		goto free_vint_bit;
> +	}
> +
> +	err = inta->sci->ops.rm_irq_ops.set_event_map(inta->sci,
> +						      dev_id, dev_index,
> +						      inta->pdev->id,
> +						      vint_desc->vint_id,
> +						      event_desc->global_event,
> +						      free_bit);
> +	if (err)
> +		goto free_global_event;
> +
> +	mutex_unlock(&vint_desc->event_mutex);
> +	return 0;
> +free_global_event:
> +	ti_sci_release_resource(inta->global_event, event_desc->global_event);
> +free_vint_bit:
> +	clear_bit(free_bit, vint_desc->event_map);
> +	mutex_unlock(&vint_desc->event_mutex);
> +	return err;
> +}
> +
> +/**
> + * ti_sci_inta_alloc_irq() -  Allocate an irq within INTA domain
> + * @domain:	irq_domain pointer corresponding to INTA
> + * @hwirq:	hwirq of the input event
> + *
> + * Note: Allocation happens in the following manner:
> + *	- Find a free bit available in any of the vints available in the list.
> + *	- If not found, allocate a vint from the vint pool
> + *	- Attach the free bit to input hwirq.
> + * Return vint_desc if all went ok else appropriate error value.
> + */
> +static struct ti_sci_inta_vint_desc
> +*ti_sci_inta_alloc_irq(struct irq_domain *domain, u32 hwirq)
> +{
> +	struct ti_sci_inta_irq_domain *inta = domain->host_data;
> +	struct ti_sci_inta_vint_desc *vint_desc = NULL;
> +	u16 free_bit;
> +	int ret;
> +
> +	mutex_lock(&inta->vint_mutex);
> +	list_for_each_entry(vint_desc, &inta->vint_list, list) {
> +		mutex_lock(&vint_desc->event_mutex);
> +		free_bit = find_first_zero_bit(vint_desc->event_map,
> +					       MAX_EVENTS_PER_VINT);
> +		if (free_bit != MAX_EVENTS_PER_VINT) {
> +			set_bit(free_bit, vint_desc->event_map);
> +			mutex_unlock(&vint_desc->event_mutex);
> +			mutex_unlock(&inta->vint_mutex);
> +			goto alloc_event;
> +		}
> +		mutex_unlock(&vint_desc->event_mutex);
> +	}
> +	mutex_unlock(&inta->vint_mutex);
> +
> +	/* No free bits available. Allocate a new vint */
> +	vint_desc = ti_sci_inta_alloc_parent_irq(domain);
> +	if (IS_ERR(vint_desc))
> +		return vint_desc;
> +
> +	mutex_lock(&vint_desc->event_mutex);
> +	free_bit = find_first_zero_bit(vint_desc->event_map,
> +				       MAX_EVENTS_PER_VINT);
> +	set_bit(free_bit, vint_desc->event_map);
> +	mutex_unlock(&vint_desc->event_mutex);
> +
> +alloc_event:
> +	ret = ti_sci_inta_alloc_event(inta, vint_desc, free_bit, hwirq);
> +	if (ret)
> +		return ERR_PTR(ret);

Without freeing the allocated bit?

> +
> +	return vint_desc;
> +}
> +
> +/**
> + * __get_event_index() - Convert the hwirq to corresponding bit inside vint.
> + * @vint_desc:	Pointer to vint_desc to which the hwirq is attached
> + * @hwirq:	hwirq number within the domain
> + *
> + * Return the vint bit to which the hwirq is attached.
> + */
> +static int __get_event_index(struct ti_sci_inta_vint_desc *vint_desc, u32 hwirq)
> +{
> +	int i;
> +
> +	mutex_lock(&vint_desc->event_mutex);
> +	for (i = 0; i < MAX_EVENTS_PER_VINT; i++)
> +		if (vint_desc->events[i].hwirq == hwirq) {
> +			mutex_unlock(&vint_desc->event_mutex);
> +			return i;
> +		}
> +
> +	mutex_unlock(&vint_desc->event_mutex);
> +	return -ENODEV;

OK, this is terrible. Having this loop on every mask/unmask/ack, plus
the contention on the lock (64:1, great) is just going to ruin latency.

Can't you spare 5 bits in hwirq to encode the index? Somehow, I don't
really believe that you do have 16bits of DEVID *and* 16bits of IRQID
(whatever the later is). Or allocate some per interrupt data that allows
the retrieval of the index in O(1)?

It would also solve your locking issue...

> +}
> +
> +/**
> + * ti_sci_inta_free_parent_irq() - Free a parent irq to INTA
> + * domain:	domain corresponding to INTA
> + */
> +static void ti_sci_inta_free_parent_irq(struct irq_domain *domain,
> +					struct ti_sci_inta_vint_desc *vint_desc)
> +{
> +	struct ti_sci_inta_irq_domain *inta = domain->host_data;
> +
> +	mutex_lock(&inta->vint_mutex);
> +	list_del(&vint_desc->list);
> +	mutex_unlock(&inta->vint_mutex);
> +	irq_dispose_mapping(vint_desc->parent_virq);
> +	ti_sci_release_resource(inta->vint, vint_desc->vint_id);
> +	kfree(vint_desc->event_map);
> +	kfree(vint_desc);
> +}
> +
> +/**
> + * ti_sci_inta_free_irq() - Free an IRQ within INTA domain
> + * domain:	Domain corresponding to INTA
> + * vint_desc:	Pointer to vint_desc from which irq needs to be freed
> + * hwirq:	Hwirq number within INTA domain that needs to be freed
> + */
> +static void ti_sci_inta_free_irq(struct irq_domain *domain,
> +				 struct ti_sci_inta_vint_desc *vint_desc,
> +				 u32 hwirq)
> +{
> +	struct ti_sci_inta_irq_domain *inta = domain->host_data;
> +	struct ti_sci_inta_event_desc *event_desc;
> +	int event_index = 0;
> +
> +	/* free event irq */
> +	event_index = __get_event_index(vint_desc, hwirq);
> +	event_desc = &vint_desc->events[event_index];
> +	mutex_lock(&vint_desc->event_mutex);
> +	inta->sci->ops.rm_irq_ops.free_event_map(inta->sci,
> +						 HWIRQ_TO_DEVID(hwirq),
> +						 HWIRQ_TO_IRQID(hwirq),
> +						 inta->pdev->id,
> +						 vint_desc->vint_id,
> +						 event_desc->global_event,
> +						 event_index);
> +
> +	clear_bit(event_index, vint_desc->event_map);
> +	ti_sci_release_resource(inta->global_event, event_desc->global_event);
> +	event_desc->global_event = TI_SCI_RESOURCE_NULL;
> +	event_desc->hwirq = 0;
> +	mutex_unlock(&vint_desc->event_mutex);
> +
> +	/* Free parent irq */
> +	if (find_first_bit(vint_desc->event_map, MAX_EVENTS_PER_VINT) ==
> +	    MAX_EVENTS_PER_VINT)

Single line. Also, how do you ensure that this doesn't race with the
allocation if you're not holding the lock?

> +		ti_sci_inta_free_parent_irq(domain, vint_desc);
> +}
> +
> +/**
> + * ti_sci_inta_request_resources() - Allocate resources for input irq
> + * @data: Pointer to corresponding irq_data
> + *
> + * Note: This is the core api where the actual allocation happens for input
> + *	 hwirq. This allocation involves creating a parent irq for vint.
> + *	 If this is done in irq_domain_ops.alloc() then a deadlock is reached
> + *	 for allocation. So this allocation is being done in request_resources()
> + *
> + * Return: 0 if all went well else corresponding error.
> + */
> +static int ti_sci_inta_request_resources(struct irq_data *data)
> +{
> +	struct ti_sci_inta_vint_desc *vint_desc;
> +
> +	vint_desc = ti_sci_inta_alloc_irq(data->domain, data->hwirq);
> +	if (IS_ERR(vint_desc))
> +		return PTR_ERR(vint_desc);
> +
> +	data->chip_data = vint_desc;
> +
> +	return 0;
> +}
> +
> +/**
> + * ti_sci_inta_release_resources - Release resources for input irq
> + * @data: Pointer to corresponding irq_data
> + *
> + * Note: Corresponding to request_resources(), all the unmapping and deletion
> + *	 of parent vint irqs happens in this api.
> + */
> +static void ti_sci_inta_release_resources(struct irq_data *data)
> +{
> +	struct ti_sci_inta_vint_desc *vint_desc;
> +
> +	vint_desc = irq_data_get_irq_chip_data(data);
> +	ti_sci_inta_free_irq(data->domain, vint_desc, data->hwirq);
> +}
> +
> +/**
> + * __ti_sci_inta_manage_event() - Control the event based on the offset
> + * @data:	Pointer to corresponding irq_data
> + * @offset:	register offset using which event is controlled.
> + */
> +static void __ti_sci_inta_manage_event(struct irq_data *data, u32 offset)
> +{
> +	struct ti_sci_inta_vint_desc *vint_desc;
> +	struct ti_sci_inta_irq_domain *inta;
> +	int event_index;
> +
> +	vint_desc = irq_data_get_irq_chip_data(data);
> +	inta = data->domain->host_data;
> +	event_index = __get_event_index(vint_desc, data->hwirq);
> +	if (event_index < 0)
> +		return;
> +
> +	writeq_relaxed(BIT(event_index), inta->base +
> +		       vint_desc->vint_id * 0x1000 + offset);
> +}
> +
> +/**
> + * ti_sci_inta_mask_irq() - Mask an event
> + * @data:	Pointer to corresponding irq_data
> + */
> +static void ti_sci_inta_mask_irq(struct irq_data *data)
> +{
> +	__ti_sci_inta_manage_event(data, VINT_ENABLE_CLR_OFFSET);
> +}
> +
> +/**
> + * ti_sci_inta_unmask_irq() - Unmask an event
> + * @data:	Pointer to corresponding irq_data
> + */
> +static void ti_sci_inta_unmask_irq(struct irq_data *data)
> +{
> +	__ti_sci_inta_manage_event(data, VINT_ENABLE_SET_OFFSET);
> +}
> +
> +/**
> + * ti_sci_inta_ack_irq() - Ack an event
> + * @data:	Pointer to corresponding irq_data
> + */
> +static void ti_sci_inta_ack_irq(struct irq_data *data)
> +{
> +	__ti_sci_inta_manage_event(data, VINT_STATUS_OFFSET);
> +}
> +
> +static int ti_sci_inta_set_affinity(struct irq_data *d,
> +				    const struct cpumask *mask_val, bool force)
> +{
> +	return -EINVAL;
> +}
> +
> +/**
> + * ti_sci_inta_set_type() - Update the trigger type of the irq.
> + * @data:	Pointer to corresponding irq_data
> + * @type:	Trigger type as specified by user
> + *
> + * Note: This updates the handle_irq callback for level msi.
> + *
> + * Return 0 if all went well else appropriate error.
> + */
> +static int ti_sci_inta_set_type(struct irq_data *data, unsigned int type)
> +{
> +	struct irq_desc *desc = irq_to_desc(data->irq);
> +
> +	/*
> +	 * .alloc default sets handle_edge_irq. But if the user specifies
> +	 * that IRQ is level MSI, then update the handle to handle_level_irq
> +	 */
> +	if (type & IRQF_TRIGGER_HIGH)
> +		desc->handle_irq = handle_level_irq;
> +
> +	return 0;

How about invalid values?

> +}
> +
> +static struct irq_chip ti_sci_inta_irq_chip = {
> +	.name			= "INTA",
> +	.irq_mask		= ti_sci_inta_mask_irq,
> +	.irq_unmask		= ti_sci_inta_unmask_irq,
> +	.irq_ack		= ti_sci_inta_ack_irq,
> +	.irq_set_affinity	= ti_sci_inta_set_affinity,
> +	.irq_request_resources	= ti_sci_inta_request_resources,
> +	.irq_release_resources	= ti_sci_inta_release_resources,
> +	.irq_set_type		= ti_sci_inta_set_type,
> +};
> +
> +/**
> + * ti_sci_inta_irq_domain_free() - Free an IRQ from the IRQ domain
> + * @domain:	Domain to which the irqs belong
> + * @virq:	base linux virtual IRQ to be freed.
> + * @nr_irqs:	Number of continuous irqs to be freed
> + */
> +static void ti_sci_inta_irq_domain_free(struct irq_domain *domain,
> +					unsigned int virq, unsigned int nr_irqs)
> +{
> +	struct irq_data *data = irq_domain_get_irq_data(domain, virq);
> +
> +	irq_domain_reset_irq_data(data);
> +}
> +
> +/**
> + * ti_sci_inta_irq_domain_alloc() - Allocate Interrupt aggregator IRQs
> + * @domain:	Point to the interrupt aggregator IRQ domain
> + * @virq:	Corresponding Linux virtual IRQ number
> + * @nr_irqs:	Continuous irqs to be allocated
> + * @data:	Pointer to firmware specifier
> + *
> + * No actual allocation happens here.
> + *
> + * Return 0 if all went well else appropriate error value.
> + */
> +static int ti_sci_inta_irq_domain_alloc(struct irq_domain *domain,
> +					unsigned int virq, unsigned int nr_irqs,
> +					void *data)
> +{
> +	msi_alloc_info_t *arg = data;
> +
> +	irq_domain_set_info(domain, virq, arg->hwirq, &ti_sci_inta_irq_chip,
> +			    NULL, handle_edge_irq, NULL, NULL);
> +
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops ti_sci_inta_irq_domain_ops = {
> +	.alloc		= ti_sci_inta_irq_domain_alloc,
> +	.free		= ti_sci_inta_irq_domain_free,
> +};
> +
> +static int ti_sci_inta_irq_domain_probe(struct platform_device *pdev)
> +{
> +	struct irq_domain *parent_domain, *domain;
> +	struct device_node *parent_node, *node;
> +	struct ti_sci_inta_irq_domain *inta;
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	int ret;
> +
> +	node = dev_of_node(dev);
> +	parent_node = of_irq_find_parent(node);
> +	if (!parent_node) {
> +		dev_err(dev, "Failed to get IRQ parent node\n");
> +		return -ENODEV;
> +	}
> +
> +	parent_domain = irq_find_host(parent_node);
> +	if (!parent_domain)
> +		return -EPROBE_DEFER;
> +
> +	inta = devm_kzalloc(dev, sizeof(*inta), GFP_KERNEL);
> +	if (!inta)
> +		return -ENOMEM;
> +
> +	inta->pdev = pdev;
> +	inta->sci = devm_ti_sci_get_by_phandle(dev, "ti,sci");
> +	if (IS_ERR(inta->sci)) {
> +		ret = PTR_ERR(inta->sci);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "ti,sci read fail %d\n", ret);
> +		inta->sci = NULL;
> +		return ret;
> +	}
> +
> +	ret = of_property_read_u32(dev->of_node, "ti,sci-dev-id", &pdev->id);
> +	if (ret) {
> +		dev_err(dev, "missing 'ti,sci-dev-id' property\n");
> +		return -EINVAL;
> +	}
> +
> +	inta->vint = devm_ti_sci_get_of_resource(inta->sci, dev, pdev->id,
> +						 "ti,sci-rm-range-vint");
> +	if (IS_ERR(inta->vint)) {
> +		dev_err(dev, "VINT resource allocation failed\n");
> +		return PTR_ERR(inta->vint);
> +	}
> +
> +	inta->global_event =
> +		devm_ti_sci_get_of_resource(inta->sci, dev, pdev->id,
> +					    "ti,sci-rm-range-global-event");
> +	if (IS_ERR(inta->global_event)) {
> +		dev_err(dev, "Global event resource allocation failed\n");
> +		return PTR_ERR(inta->global_event);
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	inta->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(inta->base))
> +		return -ENODEV;
> +
> +	domain = irq_domain_add_linear(dev_of_node(dev),
> +				       ti_sci_get_num_resources(inta->vint),
> +				       &ti_sci_inta_irq_domain_ops, inta);
> +	if (!domain) {
> +		dev_err(dev, "Failed to allocate IRQ domain\n");
> +		return -ENOMEM;
> +	}
> +
> +	INIT_LIST_HEAD(&inta->vint_list);
> +	mutex_init(&inta->vint_mutex);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ti_sci_inta_irq_domain_of_match[] = {
> +	{ .compatible = "ti,sci-inta", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, ti_sci_inta_irq_domain_of_match);
> +
> +static struct platform_driver ti_sci_inta_irq_domain_driver = {
> +	.probe = ti_sci_inta_irq_domain_probe,
> +	.driver = {
> +		.name = "ti-sci-inta",
> +		.of_match_table = ti_sci_inta_irq_domain_of_match,
> +	},
> +};
> +module_platform_driver(ti_sci_inta_irq_domain_driver);
> +
> +MODULE_AUTHOR("Lokesh Vutla <lokeshvutla@ticom>");
> +MODULE_DESCRIPTION("K3 Interrupt Aggregator driver over TI SCI protocol");
> +MODULE_LICENSE("GPL v2");
> 

The biggest things to fix here is the hwirq thing, which I believe would
simplify the locking a bit.

Thanks,

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

Re: [PATCH v6 10/12] soc: ti: Add MSI domain bus support for Interrupt Aggregator

[Marc Zyngier]

On 10/04/2019 05:13, Lokesh Vutla wrote:
> With the system coprocessor managing the range allocation of the
> inputs to Interrupt Aggregator, it is difficult to represent
> the device IRQs from DT.
> 
> The suggestion is to use MSI in such cases where devices wants
> to allocate and group interrupts dynamically.
> 
> Create a MSI domain bus layer that allocates and frees MSIs for
> a device.
> 
> APIs that are implemented:
> - ti_sci_inta_msi_create_irq_domain() that creates a MSI domain
> - ti_sci_inta_msi_domain_alloc_irqs() that creates MSIs for the
>   specified device and resource.
> - ti_sci_inta_msi_domain_free_irqs() frees the irqs attached to the device.
> - ti_sci_inta_msi_get_virq() for getting the virq attached to a specific event.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
> Changes since v5:
> - Updated the input parametes to all apis
> - Updated the default chip ops.
> - Prefixed all the apis with ti_sci_inta_
> 
> Marc,
> 	Right now ti_sci_resource is being passed for irq allocatons.
> I couldn't get to use resources attached to platform_device. Because
> platform_device resources are allocated in of_device_alloc() and number
> of resources are fixed in it. In order to update the resources, driver
> has to do a krealloc(pdev->resources) and update the num of resources.
> Is it allowed to update the pdev->resources during probe time? If yes,
> Ill be happy to update the patch to use platform_dev resources.

My suggestion was for you to define your own bus, device type and co
(much like the fsl-mc stuff), and not reuse platform devices at all.

> 
> 
>  MAINTAINERS                            |   2 +
>  drivers/soc/ti/Kconfig                 |   6 +
>  drivers/soc/ti/Makefile                |   1 +
>  drivers/soc/ti/ti_sci_inta_msi.c       | 167 +++++++++++++++++++++++++
>  include/linux/irqdomain.h              |   1 +
>  include/linux/msi.h                    |   6 +
>  include/linux/soc/ti/ti_sci_inta_msi.h |  23 ++++
>  7 files changed, 206 insertions(+)
>  create mode 100644 drivers/soc/ti/ti_sci_inta_msi.c
>  create mode 100644 include/linux/soc/ti/ti_sci_inta_msi.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ba88b3033fe4..dd31d7cb2fc6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15353,6 +15353,8 @@ F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>  F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt
>  F:	drivers/irqchip/irq-ti-sci-intr.c
>  F:	drivers/irqchip/irq-ti-sci-inta.c
> +F:	include/linux/soc/ti/ti_sci_inta_msi.h
> +F:	drivers/soc/ti/ti_sci_inta_msi.c
>  
>  Texas Instruments ASoC drivers
>  M:	Peter Ujfalusi <peter.ujfalusi@ti.com>
> diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
> index be4570baad96..82f110fe4953 100644
> --- a/drivers/soc/ti/Kconfig
> +++ b/drivers/soc/ti/Kconfig
> @@ -73,4 +73,10 @@ config TI_SCI_PM_DOMAINS
>  	  called ti_sci_pm_domains. Note this is needed early in boot before
>  	  rootfs may be available.
>  
> +config TI_SCI_INTA_MSI_DOMAIN
> +	bool
> +	select GENERIC_MSI_IRQ_DOMAIN
> +	help
> +	  Driver to enable Interrupt Aggregator specific MSI Domain.
> +
>  endif # SOC_TI
> diff --git a/drivers/soc/ti/Makefile b/drivers/soc/ti/Makefile
> index a22edc0b258a..b3868d392d4f 100644
> --- a/drivers/soc/ti/Makefile
> +++ b/drivers/soc/ti/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_KEYSTONE_NAVIGATOR_DMA)	+= knav_dma.o
>  obj-$(CONFIG_AMX3_PM)			+= pm33xx.o
>  obj-$(CONFIG_WKUP_M3_IPC)		+= wkup_m3_ipc.o
>  obj-$(CONFIG_TI_SCI_PM_DOMAINS)		+= ti_sci_pm_domains.o
> +obj-$(CONFIG_TI_SCI_INTA_MSI_DOMAIN)	+= ti_sci_inta_msi.o
> diff --git a/drivers/soc/ti/ti_sci_inta_msi.c b/drivers/soc/ti/ti_sci_inta_msi.c
> new file mode 100644
> index 000000000000..247a5e5f216b
> --- /dev/null
> +++ b/drivers/soc/ti/ti_sci_inta_msi.c
> @@ -0,0 +1,167 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Texas Instruments' K3 Interrupt Aggregator MSI bus
> + *
> + * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
> + *	Lokesh Vutla <lokeshvutla@ti.com>
> + */
> +
> +#include <linux/of_device.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/msi.h>

Alphabetical ordering, please.

> +#include <linux/soc/ti/ti_sci_inta_msi.h>
> +#include <linux/soc/ti/ti_sci_protocol.h>
> +
> +static void ti_sci_inta_msi_write_msg(struct irq_data *data,
> +				      struct msi_msg *msg)
> +{
> +	/* Nothing to do */
> +}
> +
> +static void ti_sci_inta_msi_compose_msi_msg(struct irq_data *data,
> +					    struct msi_msg *msg)
> +{
> +	/* Nothing to do */
> +}
> +
> +static int ti_sci_inta_msi_request_resources(struct irq_data *data)
> +{
> +	data = data->parent_data;
> +
> +	return data->chip->irq_request_resources(data);
> +}
> +
> +static void ti_sci_inta_msi_release_resources(struct irq_data *data)
> +{
> +	data = data->parent_data;
> +	data->chip->irq_release_resources(data);
> +}

The two functions above are an implementation of
irq_chip_{request,release}_resource_parent(). Please make them generic
functions, use them and fix drivers/gpio/gpio-thunderx.c to use them too.

> +
> +static void ti_sci_inta_msi_update_chip_ops(struct msi_domain_info *info)
> +{
> +	struct irq_chip *chip = info->chip;
> +
> +	WARN_ON(!chip);

Just doing that isn't going to help, as you'll crash on the following
line...

> +	if (!chip->irq_mask)
> +		chip->irq_mask = irq_chip_mask_parent;
> +	if (!chip->irq_unmask)
> +		chip->irq_unmask = irq_chip_unmask_parent;
> +	if (!chip->irq_ack)
> +		chip->irq_ack = irq_chip_ack_parent;
> +	if (!chip->irq_set_type)
> +		chip->irq_set_type = irq_chip_set_type_parent;
> +	if (!chip->irq_write_msi_msg)
> +		chip->irq_write_msi_msg = ti_sci_inta_msi_write_msg;
> +	if (!chip->irq_compose_msi_msg)
> +		chip->irq_compose_msi_msg = ti_sci_inta_msi_compose_msi_msg;
> +	if (!chip->irq_request_resources)
> +		chip->irq_request_resources = ti_sci_inta_msi_request_resources;
> +	if (!chip->irq_release_resources)
> +		chip->irq_release_resources = ti_sci_inta_msi_release_resources;

Is there any case where a client driver wouldn't use the default all the
time?

> +}
> +
> +struct irq_domain
> +*ti_sci_inta_msi_create_irq_domain(struct fwnode_handle *fwnode,
> +				   struct msi_domain_info *info,
> +				   struct irq_domain *parent)
> +{
> +	struct irq_domain *domain;
> +
> +	if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
> +		ti_sci_inta_msi_update_chip_ops(info);

If the answer above is "no", then you can happily ignore this flag and
always populate the callbacks.

> +
> +	domain = msi_create_irq_domain(fwnode, info, parent);
> +	if (domain)
> +		irq_domain_update_bus_token(domain, DOMAIN_BUS_TI_SCI_INTA_MSI);
> +
> +	return domain;
> +}
> +EXPORT_SYMBOL_GPL(ti_sci_inta_msi_create_irq_domain);
> +
> +static void ti_sci_inta_msi_free_descs(struct device *dev)
> +{
> +	struct msi_desc *desc, *tmp;
> +
> +	list_for_each_entry_safe(desc, tmp, dev_to_msi_list(dev), list) {
> +		list_del(&desc->list);
> +		free_msi_entry(desc);
> +	}
> +}
> +
> +static int ti_sci_inta_msi_alloc_descs(struct device *dev, u32 dev_id,
> +				       struct ti_sci_resource *res)
> +{
> +	struct msi_desc *msi_desc;
> +	int set, i, count = 0;
> +
> +	for (set = 0; set < res->sets; set++) {
> +		for (i = 0; i < res->desc[set].num; i++) {
> +			msi_desc = alloc_msi_entry(dev, 1, NULL);
> +			if (!msi_desc) {
> +				ti_sci_inta_msi_free_descs(dev);
> +				return -ENOMEM;
> +			}
> +
> +			msi_desc->inta.index = res->desc[set].start + i;
> +			msi_desc->inta.dev_id = dev_id;

I'm highly suspiscious of this. See further down.

> +			INIT_LIST_HEAD(&msi_desc->list);
> +			list_add_tail(&msi_desc->list, dev_to_msi_list(dev));
> +			count++;
> +		}
> +	}
> +
> +	return count;
> +}
> +
> +int ti_sci_inta_msi_domain_alloc_irqs(struct platform_device *pdev,
> +				      struct ti_sci_resource *res)
> +{
> +	struct irq_domain *msi_domain;
> +	int ret, nvec;
> +
> +	msi_domain = dev_get_msi_domain(&pdev->dev);
> +	if (!msi_domain)
> +		return -EINVAL;
> +
> +	if (pdev->id < 0)
> +		return -ENODEV;
> +
> +	nvec = ti_sci_inta_msi_alloc_descs(&pdev->dev, pdev->id, res);
> +	if (nvec <= 0)
> +		return nvec;
> +
> +	ret = msi_domain_alloc_irqs(msi_domain, &pdev->dev, nvec);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to allocate IRQs %d\n", ret);
> +		goto cleanup;
> +	}
> +
> +	return 0;
> +
> +cleanup:
> +	ti_sci_inta_msi_free_descs(&pdev->dev);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(ti_sci_inta_msi_domain_alloc_irqs);
> +
> +void ti_sci_inta_msi_domain_free_irqs(struct device *dev)
> +{
> +	msi_domain_free_irqs(dev->msi_domain, dev);
> +	ti_sci_inta_msi_free_descs(dev);
> +}
> +EXPORT_SYMBOL_GPL(ti_sci_inta_msi_domain_free_irqs);
> +
> +unsigned int ti_sci_inta_msi_get_virq(struct platform_device *pdev, u32 index)
> +{
> +	struct msi_desc *desc;
> +
> +	for_each_msi_entry(desc, &pdev->dev)
> +		if (desc->inta.index == index && desc->inta.dev_id == pdev->id)

What is this "index"? Why isn't the right entry the index-th element in
the msi_desc list?  Worse, the dev_id check. The whole point of having a
per-device MSI list is that it is, well, per device.

> +			return desc->irq;
> +
> +	return -ENODEV;
> +}
> +EXPORT_SYMBOL_GPL(ti_sci_inta_msi_get_virq);
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index 61706b430907..07ec8b390161 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -82,6 +82,7 @@ enum irq_domain_bus_token {
>  	DOMAIN_BUS_NEXUS,
>  	DOMAIN_BUS_IPI,
>  	DOMAIN_BUS_FSL_MC_MSI,
> +	DOMAIN_BUS_TI_SCI_INTA_MSI,
>  };
>  
>  /**
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index 7e9b81c3b50d..7e12dfc4cb39 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -47,6 +47,11 @@ struct fsl_mc_msi_desc {
>  	u16				msi_index;
>  };
>  
> +struct ti_sci_inta_msi_desc {
> +	u16	dev_id;
> +	u16	index;
> +};
> +
>  /**
>   * struct msi_desc - Descriptor structure for MSI based interrupts
>   * @list:	List head for management
> @@ -106,6 +111,7 @@ struct msi_desc {
>  		 */
>  		struct platform_msi_desc platform;
>  		struct fsl_mc_msi_desc fsl_mc;
> +		struct ti_sci_inta_msi_desc inta;
>  	};
>  };
>  
> diff --git a/include/linux/soc/ti/ti_sci_inta_msi.h b/include/linux/soc/ti/ti_sci_inta_msi.h
> new file mode 100644
> index 000000000000..b0ca20ab3f49
> --- /dev/null
> +++ b/include/linux/soc/ti/ti_sci_inta_msi.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Texas Instruments' K3 TI SCI INTA MSI helper
> + *
> + * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
> + *	Lokesh Vutla <lokeshvutla@ti.com>
> + */
> +
> +#ifndef __INCLUDE_LINUX_TI_SCI_INTA_MSI_H
> +#define __INCLUDE_LINUX_TI_SCI_INTA_MSI_H
> +
> +#include <linux/msi.h>
> +#include <linux/soc/ti/ti_sci_protocol.h>
> +
> +struct irq_domain
> +*ti_sci_inta_msi_create_irq_domain(struct fwnode_handle *fwnode,
> +				   struct msi_domain_info *info,
> +				   struct irq_domain *parent);
> +int ti_sci_inta_msi_domain_alloc_irqs(struct platform_device *pdev,
> +				      struct ti_sci_resource *res);
> +unsigned int ti_sci_inta_msi_get_virq(struct platform_device *dev, u32 index);
> +void ti_sci_inta_msi_domain_free_irqs(struct device *dev);
> +#endif /* __INCLUDE_LINUX_IRQCHIP_TI_SCI_INTA_H */
> 

Thanks,

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

Re: [PATCH v6 09/12] irqchip: ti-sci-inta: Add support for Interrupt Aggregator driver

[Lokesh Vutla]

Hi Marc,

On 17/04/19 7:44 PM, Marc Zyngier wrote:
> Hi Lokesh,
> 
> On 10/04/2019 05:13, Lokesh Vutla wrote:
>> Texas Instruments' K3 generation SoCs has an IP Interrupt Aggregator
>> which is an interrupt controller that does the following:
>> - Converts events to interrupts that can be understood by
>>   an interrupt router.
>> - Allows for multiplexing of events to interrupts.
>>
>> Configuration of the interrupt aggregator registers can only be done by
>> a system co-processor and the driver needs to send a message to this
>> co processor over TISCI protocol.
>>
>> Add support for Interrupt Aggregator driver with 2 IRQ Domains:
>> - INTA MSI domain that interfaces the devices using which interrupts can be
>>   allocated dynamically.
>> - INTA domain that is parent to the MSI domain that manages the interrupt
>>   resources.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>> Changes since v5:
>> - Moved all the allocation of events and parent irqs to .irq_request_resources
>>   callback of irqchip. This is based on the offline discussion with Marc.
>>
>> Marc,
>> 	This version has a deviation from our discussion:
>> - Could not create separate irq_domains for each output(vint) of INTA, instead
>>   stick to a single irq_domain for the entire INTA. Because when creating
>>   msi_domain there is no parent available to attach. Even then I create
>>   irq_domains for all the available vints during probe, how do we decide
>>   which is the parent of msi_domain?
> 
> Yeah, you're right. The top-down allocation assumes that we already have
> setup the domain hierarchy, and this delayed allocation scheme breaks
> it. Oh well, never mind.
> 
>>
>>
>>  MAINTAINERS                       |   1 +
>>  drivers/irqchip/Kconfig           |  11 +
>>  drivers/irqchip/Makefile          |   1 +
>>  drivers/irqchip/irq-ti-sci-inta.c | 622 ++++++++++++++++++++++++++++++
>>  4 files changed, 635 insertions(+)
>>  create mode 100644 drivers/irqchip/irq-ti-sci-inta.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 90173038f674..ba88b3033fe4 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -15352,6 +15352,7 @@ F:	drivers/reset/reset-ti-sci.c
>>  F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>  F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt
>>  F:	drivers/irqchip/irq-ti-sci-intr.c
>> +F:	drivers/irqchip/irq-ti-sci-inta.c
>>  
>>  Texas Instruments ASoC drivers
>>  M:	Peter Ujfalusi <peter.ujfalusi@ti.com>
>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>> index a1ff64c1d40d..946c062fcec0 100644
>> --- a/drivers/irqchip/Kconfig
>> +++ b/drivers/irqchip/Kconfig
>> @@ -436,6 +436,17 @@ config TI_SCI_INTR_IRQCHIP
>>  	  If you wish to use interrupt router irq resources managed by the
>>  	  TI System Controller, say Y here. Otherwise, say N.
>>  
>> +config TI_SCI_INTA_IRQCHIP
>> +	bool
>> +	depends on TI_SCI_PROTOCOL && ARCH_K3
>> +	select IRQ_DOMAIN
>> +	select IRQ_DOMAIN_HIERARCHY
>> +	help
>> +	  This enables the irqchip driver support for K3 Interrupt aggregator
>> +	  over TI System Control Interface available on some new TI's SoCs.
>> +	  If you wish to use interrupt aggregator irq resources managed by the
>> +	  TI System Controller, say Y here. Otherwise, say N.
>> +
>>  endmenu
>>  
>>  config SIFIVE_PLIC
>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>> index fa5c865788b5..8a33013da953 100644
>> --- a/drivers/irqchip/Makefile
>> +++ b/drivers/irqchip/Makefile
>> @@ -98,3 +98,4 @@ obj-$(CONFIG_IMX_IRQSTEER)		+= irq-imx-irqsteer.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
>> +obj-$(CONFIG_TI_SCI_INTA_IRQCHIP)	+= irq-ti-sci-inta.o
>> diff --git a/drivers/irqchip/irq-ti-sci-inta.c b/drivers/irqchip/irq-ti-sci-inta.c
>> new file mode 100644
>> index 000000000000..3eb935ebe10f
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-ti-sci-inta.c
>> @@ -0,0 +1,622 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Texas Instruments' K3 Interrupt Aggregator irqchip driver
>> + *
>> + * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
>> + *	Lokesh Vutla <lokeshvutla@ti.com>
>> + */
>> +
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/msi.h>
>> +#include <linux/irqchip.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/module.h>
>> +#include <linux/moduleparam.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/soc/ti/ti_sci_protocol.h>
>> +#include <linux/irqchip/chained_irq.h>
>> +#include <asm-generic/msi.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 MAX_EVENTS_PER_VINT	64
>> +#define VINT_ENABLE_SET_OFFSET	0x0
>> +#define VINT_ENABLE_CLR_OFFSET	0x8
>> +#define VINT_STATUS_OFFSET	0x18
>> +
>> +/**
>> + * struct ti_sci_inta_event_desc - Description of an event coming to
>> + *				   Interrupt Aggregator. This serves
>> + *				   as a mapping table between global
>> + *				   event and hwirq.
>> + * @global_event:	Global event number corresponding to this event
>> + * @hwirq:		Hwirq of the incoming interrupt
>> + */
>> +struct ti_sci_inta_event_desc {
>> +	u16 global_event;
>> +	u32 hwirq;
>> +};
>> +
>> +/**
>> + * struct ti_sci_inta_vint_desc - Description of a virtual interrupt coming out
>> + *				  of Interrupt Aggregator.
>> + * @domain:		Pointer to IRQ domain to which this vint belongs.
>> + * @list:		List entry for the vint list
>> + * @event_map:		Bitmap to manage the allocation of events to vint.
>> + * @event_mutex:	Mutex to protect allocation of events.
>> + * @events:		Array of event descriptors assigned to this vint.
>> + * @parent_virq:	Linux IRQ number that gets attached to parent
>> + * @vint_id:		TISCI vint ID
>> + */
>> +struct ti_sci_inta_vint_desc {
>> +	struct irq_domain *domain;
>> +	struct list_head list;
>> +	unsigned long *event_map;
>> +	/* Mutex to protect allocation of events */
>> +	struct mutex event_mutex;
> 
> As mentioned in another email, this needs to be a spinlock, as the lock
> may be taken in atomic context.
> 
>> +	struct ti_sci_inta_event_desc events[MAX_EVENTS_PER_VINT];
>> +	unsigned int parent_virq;
>> +	u16 vint_id;
>> +};
>> +
>> +/**
>> + * struct ti_sci_inta_irq_domain - Structure representing a TISCI based
>> + *				   Interrupt Aggregator IRQ domain.
>> + * @sci:		Pointer to TISCI handle
>> + * @vint:		TISCI resource pointer representing IA inerrupts.
>> + * @global_event:	TISCI resource pointer representing global events.
>> + * @vint_list:		List of the vints active in the system
>> + * @vint_mutex:		Mutex to protect vint_list
>> + * @base:		Base address of the memory mapped IO registers
>> + * @pdev:		Pointer to platform device.
>> + */
>> +struct ti_sci_inta_irq_domain {
>> +	const struct ti_sci_handle *sci;
>> +	struct ti_sci_resource *vint;
>> +	struct ti_sci_resource *global_event;
>> +	struct list_head vint_list;
>> +	/* Mutex to protect vint list */
>> +	struct mutex vint_mutex;
>> +	void __iomem *base;
>> +	struct platform_device	*pdev;
>> +};
>> +
>> +/**
>> + * ti_sci_inta_irq_handler() - Chained IRQ handler for the vint irqs
>> + * @desc:	Pointer to irq_desc corresponding to the irq
>> + */
>> +static void ti_sci_inta_irq_handler(struct irq_desc *desc)
>> +{
>> +	struct ti_sci_inta_vint_desc *vint_desc;
>> +	struct ti_sci_inta_irq_domain *inta;
>> +	struct irq_domain *domain;
>> +	unsigned int virq, bit;
>> +	u64 val;
>> +
>> +	vint_desc = irq_desc_get_handler_data(desc);
>> +	domain = vint_desc->domain;
>> +	inta = domain->host_data;
>> +
>> +	chained_irq_enter(irq_desc_get_chip(desc), desc);
>> +
>> +	val = readq_relaxed(inta->base + vint_desc->vint_id * 0x1000 +
>> +			    VINT_STATUS_OFFSET);
>> +
>> +	for (bit = 0; bit < MAX_EVENTS_PER_VINT; bit++) {
>> +		if (BIT(bit) & val) {
>> +			virq = irq_find_mapping(domain,
>> +						vint_desc->events[bit].hwirq);
>> +			if (virq)
>> +				generic_handle_irq(virq);
>> +		}
>> +	}
>> +
>> +	chained_irq_exit(irq_desc_get_chip(desc), desc);
>> +}
>> +
>> +/**
>> + * ti_sci_inta_alloc_parent_irq() - Allocate parent irq to Interrupt aggregator
>> + * @domain:	IRQ domain corresponding to Interrupt Aggregator
>> + *
>> + * Return 0 if all went well else corresponding error value.
>> + */
>> +static struct ti_sci_inta_vint_desc
>> +*ti_sci_inta_alloc_parent_irq(struct irq_domain *domain)
> 
> nit: do not split lines like this, specially not with the * on a
> different line than the type it applies to.

okay. I was trying to make checkpatch script happy.

> 
>> +{
>> +	struct ti_sci_inta_irq_domain *inta = domain->host_data;
>> +	struct ti_sci_inta_vint_desc *vint_desc;
>> +	struct irq_fwspec parent_fwspec;
>> +	u16 vint_id;
>> +
>> +	vint_id = ti_sci_get_free_resource(inta->vint);
>> +	if (vint_id == TI_SCI_RESOURCE_NULL)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	vint_desc = kzalloc(sizeof(*vint_desc), GFP_KERNEL);
>> +	if (!vint_desc)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	vint_desc->event_map = kcalloc(BITS_TO_LONGS(MAX_EVENTS_PER_VINT),
>> +				       sizeof(*vint_desc->event_map),
>> +				       GFP_KERNEL);
> 
> You already have an array of MAX_EVENTS_PER_VINT (events) in this
> structure. Why don't you simply declare it as a
> MAX_EVENTS_PER_VINT-sized bitmap and save yourself the allocation?

okay. will fix it in next version.

> 
>> +	if (!vint_desc->event_map) {
>> +		kfree(vint_desc);
>> +		return ERR_PTR(-ENOMEM);
>> +	}
>> +	mutex_init(&vint_desc->event_mutex);
>> +
>> +	vint_desc->domain = domain;
>> +	vint_desc->vint_id = vint_id;
>> +	INIT_LIST_HEAD(&vint_desc->list);
>> +
>> +	mutex_lock(&inta->vint_mutex);
>> +	list_add_tail(&vint_desc->list, &inta->vint_list);
>> +	mutex_unlock(&inta->vint_mutex);
>> +
>> +	parent_fwspec.fwnode =
>> +	of_node_to_fwnode(of_irq_find_parent(dev_of_node(&inta->pdev->dev)));
> 
> single line.
> 
>> +	parent_fwspec.param_count = 3;
>> +	parent_fwspec.param[0] = inta->pdev->id;
>> +	parent_fwspec.param[1] = vint_desc->vint_id;
>> +	parent_fwspec.param[2] = 1;
>> +
>> +	vint_desc->parent_virq = irq_create_fwspec_mapping(&parent_fwspec);
>> +	if (vint_desc->parent_virq <= 0)
>> +		return ERR_PTR(vint_desc->parent_virq);
> 
> Don't you need to free vint_desc (and its event_map) here?

yeah, you are right.

> 
>> +
>> +	irq_set_chained_handler_and_data(vint_desc->parent_virq,
>> +					 ti_sci_inta_irq_handler, vint_desc);
>> +
>> +	return vint_desc;
>> +}
>> +
>> +/**
>> + * ti_sci_inta_alloc_event() - Attach an event to a IA vint.
>> + * @inta:	Pointer to INTA domain descriptor
>> + * @vint_desc:	Pointer to vint_desc to which the event gets attached
>> + * @free_bit:	Bit inside vint to which event gets attached
>> + * @hwirq:	hwirq of the input event
>> + *
>> + * Return global_event if all went ok else appropriate error value.
>> + */
>> +static
>> +int ti_sci_inta_alloc_event(struct ti_sci_inta_irq_domain *inta,
>> +			    struct ti_sci_inta_vint_desc *vint_desc,
>> +			    u16 free_bit, u32 hwirq)
>> +{
>> +	struct ti_sci_inta_event_desc *event_desc;
>> +	u16 dev_id, dev_index;
>> +	int err;
>> +
>> +	dev_id = HWIRQ_TO_DEVID(hwirq);
>> +	dev_index = HWIRQ_TO_IRQID(hwirq);
>> +	event_desc = &vint_desc->events[free_bit];
>> +	mutex_lock(&vint_desc->event_mutex);
>> +	event_desc->hwirq = hwirq;
>> +	event_desc->global_event = ti_sci_get_free_resource(inta->global_event);
>> +	if (event_desc->global_event == TI_SCI_RESOURCE_NULL) {
>> +		err = -EINVAL;
>> +		goto free_vint_bit;
>> +	}
>> +
>> +	err = inta->sci->ops.rm_irq_ops.set_event_map(inta->sci,
>> +						      dev_id, dev_index,
>> +						      inta->pdev->id,
>> +						      vint_desc->vint_id,
>> +						      event_desc->global_event,
>> +						      free_bit);
>> +	if (err)
>> +		goto free_global_event;
>> +
>> +	mutex_unlock(&vint_desc->event_mutex);
>> +	return 0;
>> +free_global_event:
>> +	ti_sci_release_resource(inta->global_event, event_desc->global_event);
>> +free_vint_bit:
>> +	clear_bit(free_bit, vint_desc->event_map);
>> +	mutex_unlock(&vint_desc->event_mutex);
>> +	return err;
>> +}
>> +
>> +/**
>> + * ti_sci_inta_alloc_irq() -  Allocate an irq within INTA domain
>> + * @domain:	irq_domain pointer corresponding to INTA
>> + * @hwirq:	hwirq of the input event
>> + *
>> + * Note: Allocation happens in the following manner:
>> + *	- Find a free bit available in any of the vints available in the list.
>> + *	- If not found, allocate a vint from the vint pool
>> + *	- Attach the free bit to input hwirq.
>> + * Return vint_desc if all went ok else appropriate error value.
>> + */
>> +static struct ti_sci_inta_vint_desc
>> +*ti_sci_inta_alloc_irq(struct irq_domain *domain, u32 hwirq)
>> +{
>> +	struct ti_sci_inta_irq_domain *inta = domain->host_data;
>> +	struct ti_sci_inta_vint_desc *vint_desc = NULL;
>> +	u16 free_bit;
>> +	int ret;
>> +
>> +	mutex_lock(&inta->vint_mutex);
>> +	list_for_each_entry(vint_desc, &inta->vint_list, list) {
>> +		mutex_lock(&vint_desc->event_mutex);
>> +		free_bit = find_first_zero_bit(vint_desc->event_map,
>> +					       MAX_EVENTS_PER_VINT);
>> +		if (free_bit != MAX_EVENTS_PER_VINT) {
>> +			set_bit(free_bit, vint_desc->event_map);
>> +			mutex_unlock(&vint_desc->event_mutex);
>> +			mutex_unlock(&inta->vint_mutex);
>> +			goto alloc_event;
>> +		}
>> +		mutex_unlock(&vint_desc->event_mutex);
>> +	}
>> +	mutex_unlock(&inta->vint_mutex);
>> +
>> +	/* No free bits available. Allocate a new vint */
>> +	vint_desc = ti_sci_inta_alloc_parent_irq(domain);
>> +	if (IS_ERR(vint_desc))
>> +		return vint_desc;
>> +
>> +	mutex_lock(&vint_desc->event_mutex);
>> +	free_bit = find_first_zero_bit(vint_desc->event_map,
>> +				       MAX_EVENTS_PER_VINT);
>> +	set_bit(free_bit, vint_desc->event_map);
>> +	mutex_unlock(&vint_desc->event_mutex);
>> +
>> +alloc_event:
>> +	ret = ti_sci_inta_alloc_event(inta, vint_desc, free_bit, hwirq);
>> +	if (ret)
>> +		return ERR_PTR(ret);
> 
> Without freeing the allocated bit?

ti_sci_inta_alloc_event() is taking care of freeing the allocated bit with the
lock acquired.

> 
>> +
>> +	return vint_desc;
>> +}
>> +
>> +/**
>> + * __get_event_index() - Convert the hwirq to corresponding bit inside vint.
>> + * @vint_desc:	Pointer to vint_desc to which the hwirq is attached
>> + * @hwirq:	hwirq number within the domain
>> + *
>> + * Return the vint bit to which the hwirq is attached.
>> + */
>> +static int __get_event_index(struct ti_sci_inta_vint_desc *vint_desc, u32 hwirq)
>> +{
>> +	int i;
>> +
>> +	mutex_lock(&vint_desc->event_mutex);
>> +	for (i = 0; i < MAX_EVENTS_PER_VINT; i++)
>> +		if (vint_desc->events[i].hwirq == hwirq) {
>> +			mutex_unlock(&vint_desc->event_mutex);
>> +			return i;
>> +		}
>> +
>> +	mutex_unlock(&vint_desc->event_mutex);
>> +	return -ENODEV;
> 
> OK, this is terrible. Having this loop on every mask/unmask/ack, plus
> the contention on the lock (64:1, great) is just going to ruin latency.
> 
> Can't you spare 5 bits in hwirq to encode the index? Somehow, I don't
> really believe that you do have 16bits of DEVID *and* 16bits of IRQID
> (whatever the later is). Or allocate some per interrupt data that allows
> the retrieval of the index in O(1)?

Ill prefer the later. Will try to re create event_index structure a bit and get
index in O(1).

> 
> It would also solve your locking issue...
> 
>> +}
>> +
>> +/**
>> + * ti_sci_inta_free_parent_irq() - Free a parent irq to INTA
>> + * domain:	domain corresponding to INTA
>> + */
>> +static void ti_sci_inta_free_parent_irq(struct irq_domain *domain,
>> +					struct ti_sci_inta_vint_desc *vint_desc)
>> +{
>> +	struct ti_sci_inta_irq_domain *inta = domain->host_data;
>> +
>> +	mutex_lock(&inta->vint_mutex);
>> +	list_del(&vint_desc->list);
>> +	mutex_unlock(&inta->vint_mutex);
>> +	irq_dispose_mapping(vint_desc->parent_virq);
>> +	ti_sci_release_resource(inta->vint, vint_desc->vint_id);
>> +	kfree(vint_desc->event_map);
>> +	kfree(vint_desc);
>> +}
>> +
>> +/**
>> + * ti_sci_inta_free_irq() - Free an IRQ within INTA domain
>> + * domain:	Domain corresponding to INTA
>> + * vint_desc:	Pointer to vint_desc from which irq needs to be freed
>> + * hwirq:	Hwirq number within INTA domain that needs to be freed
>> + */
>> +static void ti_sci_inta_free_irq(struct irq_domain *domain,
>> +				 struct ti_sci_inta_vint_desc *vint_desc,
>> +				 u32 hwirq)
>> +{
>> +	struct ti_sci_inta_irq_domain *inta = domain->host_data;
>> +	struct ti_sci_inta_event_desc *event_desc;
>> +	int event_index = 0;
>> +
>> +	/* free event irq */
>> +	event_index = __get_event_index(vint_desc, hwirq);
>> +	event_desc = &vint_desc->events[event_index];
>> +	mutex_lock(&vint_desc->event_mutex);
>> +	inta->sci->ops.rm_irq_ops.free_event_map(inta->sci,
>> +						 HWIRQ_TO_DEVID(hwirq),
>> +						 HWIRQ_TO_IRQID(hwirq),
>> +						 inta->pdev->id,
>> +						 vint_desc->vint_id,
>> +						 event_desc->global_event,
>> +						 event_index);
>> +
>> +	clear_bit(event_index, vint_desc->event_map);
>> +	ti_sci_release_resource(inta->global_event, event_desc->global_event);
>> +	event_desc->global_event = TI_SCI_RESOURCE_NULL;
>> +	event_desc->hwirq = 0;
>> +	mutex_unlock(&vint_desc->event_mutex);
>> +
>> +	/* Free parent irq */
>> +	if (find_first_bit(vint_desc->event_map, MAX_EVENTS_PER_VINT) ==
>> +	    MAX_EVENTS_PER_VINT)
> 
> Single line. Also, how do you ensure that this doesn't race with the
> allocation if you're not holding the lock?

will fix it in next version.

> 
>> +		ti_sci_inta_free_parent_irq(domain, vint_desc);
>> +}
>> +
>> +/**
>> + * ti_sci_inta_request_resources() - Allocate resources for input irq
>> + * @data: Pointer to corresponding irq_data
>> + *
>> + * Note: This is the core api where the actual allocation happens for input
>> + *	 hwirq. This allocation involves creating a parent irq for vint.
>> + *	 If this is done in irq_domain_ops.alloc() then a deadlock is reached
>> + *	 for allocation. So this allocation is being done in request_resources()
>> + *
>> + * Return: 0 if all went well else corresponding error.
>> + */
>> +static int ti_sci_inta_request_resources(struct irq_data *data)
>> +{
>> +	struct ti_sci_inta_vint_desc *vint_desc;
>> +
>> +	vint_desc = ti_sci_inta_alloc_irq(data->domain, data->hwirq);
>> +	if (IS_ERR(vint_desc))
>> +		return PTR_ERR(vint_desc);
>> +
>> +	data->chip_data = vint_desc;
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * ti_sci_inta_release_resources - Release resources for input irq
>> + * @data: Pointer to corresponding irq_data
>> + *
>> + * Note: Corresponding to request_resources(), all the unmapping and deletion
>> + *	 of parent vint irqs happens in this api.
>> + */
>> +static void ti_sci_inta_release_resources(struct irq_data *data)
>> +{
>> +	struct ti_sci_inta_vint_desc *vint_desc;
>> +
>> +	vint_desc = irq_data_get_irq_chip_data(data);
>> +	ti_sci_inta_free_irq(data->domain, vint_desc, data->hwirq);
>> +}
>> +
>> +/**
>> + * __ti_sci_inta_manage_event() - Control the event based on the offset
>> + * @data:	Pointer to corresponding irq_data
>> + * @offset:	register offset using which event is controlled.
>> + */
>> +static void __ti_sci_inta_manage_event(struct irq_data *data, u32 offset)
>> +{
>> +	struct ti_sci_inta_vint_desc *vint_desc;
>> +	struct ti_sci_inta_irq_domain *inta;
>> +	int event_index;
>> +
>> +	vint_desc = irq_data_get_irq_chip_data(data);
>> +	inta = data->domain->host_data;
>> +	event_index = __get_event_index(vint_desc, data->hwirq);
>> +	if (event_index < 0)
>> +		return;
>> +
>> +	writeq_relaxed(BIT(event_index), inta->base +
>> +		       vint_desc->vint_id * 0x1000 + offset);
>> +}
>> +
>> +/**
>> + * ti_sci_inta_mask_irq() - Mask an event
>> + * @data:	Pointer to corresponding irq_data
>> + */
>> +static void ti_sci_inta_mask_irq(struct irq_data *data)
>> +{
>> +	__ti_sci_inta_manage_event(data, VINT_ENABLE_CLR_OFFSET);
>> +}
>> +
>> +/**
>> + * ti_sci_inta_unmask_irq() - Unmask an event
>> + * @data:	Pointer to corresponding irq_data
>> + */
>> +static void ti_sci_inta_unmask_irq(struct irq_data *data)
>> +{
>> +	__ti_sci_inta_manage_event(data, VINT_ENABLE_SET_OFFSET);
>> +}
>> +
>> +/**
>> + * ti_sci_inta_ack_irq() - Ack an event
>> + * @data:	Pointer to corresponding irq_data
>> + */
>> +static void ti_sci_inta_ack_irq(struct irq_data *data)
>> +{
>> +	__ti_sci_inta_manage_event(data, VINT_STATUS_OFFSET);
>> +}
>> +
>> +static int ti_sci_inta_set_affinity(struct irq_data *d,
>> +				    const struct cpumask *mask_val, bool force)
>> +{
>> +	return -EINVAL;
>> +}
>> +
>> +/**
>> + * ti_sci_inta_set_type() - Update the trigger type of the irq.
>> + * @data:	Pointer to corresponding irq_data
>> + * @type:	Trigger type as specified by user
>> + *
>> + * Note: This updates the handle_irq callback for level msi.
>> + *
>> + * Return 0 if all went well else appropriate error.
>> + */
>> +static int ti_sci_inta_set_type(struct irq_data *data, unsigned int type)
>> +{
>> +	struct irq_desc *desc = irq_to_desc(data->irq);
>> +
>> +	/*
>> +	 * .alloc default sets handle_edge_irq. But if the user specifies
>> +	 * that IRQ is level MSI, then update the handle to handle_level_irq
>> +	 */
>> +	if (type & IRQF_TRIGGER_HIGH)
>> +		desc->handle_irq = handle_level_irq;
>> +
>> +	return 0;
> 
> How about invalid values?

Sure, will fix it in next version.


Thanks and reagrds,
Lokesh

Re: [PATCH v6 10/12] soc: ti: Add MSI domain bus support for Interrupt Aggregator

[Lokesh Vutla]

On 17/04/19 10:04 PM, Marc Zyngier wrote:
> On 10/04/2019 05:13, Lokesh Vutla wrote:
>> With the system coprocessor managing the range allocation of the
>> inputs to Interrupt Aggregator, it is difficult to represent
>> the device IRQs from DT.
>>
>> The suggestion is to use MSI in such cases where devices wants
>> to allocate and group interrupts dynamically.
>>
>> Create a MSI domain bus layer that allocates and frees MSIs for
>> a device.
>>
>> APIs that are implemented:
>> - ti_sci_inta_msi_create_irq_domain() that creates a MSI domain
>> - ti_sci_inta_msi_domain_alloc_irqs() that creates MSIs for the
>>   specified device and resource.
>> - ti_sci_inta_msi_domain_free_irqs() frees the irqs attached to the device.
>> - ti_sci_inta_msi_get_virq() for getting the virq attached to a specific event.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>> Changes since v5:
>> - Updated the input parametes to all apis
>> - Updated the default chip ops.
>> - Prefixed all the apis with ti_sci_inta_
>>
>> Marc,
>> 	Right now ti_sci_resource is being passed for irq allocatons.
>> I couldn't get to use resources attached to platform_device. Because
>> platform_device resources are allocated in of_device_alloc() and number
>> of resources are fixed in it. In order to update the resources, driver
>> has to do a krealloc(pdev->resources) and update the num of resources.
>> Is it allowed to update the pdev->resources during probe time? If yes,
>> Ill be happy to update the patch to use platform_dev resources.
> 
> My suggestion was for you to define your own bus, device type and co
> (much like the fsl-mc stuff), and not reuse platform devices at all.
> 
>>
>>
>>  MAINTAINERS                            |   2 +
>>  drivers/soc/ti/Kconfig                 |   6 +
>>  drivers/soc/ti/Makefile                |   1 +
>>  drivers/soc/ti/ti_sci_inta_msi.c       | 167 +++++++++++++++++++++++++
>>  include/linux/irqdomain.h              |   1 +
>>  include/linux/msi.h                    |   6 +
>>  include/linux/soc/ti/ti_sci_inta_msi.h |  23 ++++
>>  7 files changed, 206 insertions(+)
>>  create mode 100644 drivers/soc/ti/ti_sci_inta_msi.c
>>  create mode 100644 include/linux/soc/ti/ti_sci_inta_msi.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index ba88b3033fe4..dd31d7cb2fc6 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -15353,6 +15353,8 @@ F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>  F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt
>>  F:	drivers/irqchip/irq-ti-sci-intr.c
>>  F:	drivers/irqchip/irq-ti-sci-inta.c
>> +F:	include/linux/soc/ti/ti_sci_inta_msi.h
>> +F:	drivers/soc/ti/ti_sci_inta_msi.c
>>  
>>  Texas Instruments ASoC drivers
>>  M:	Peter Ujfalusi <peter.ujfalusi@ti.com>
>> diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
>> index be4570baad96..82f110fe4953 100644
>> --- a/drivers/soc/ti/Kconfig
>> +++ b/drivers/soc/ti/Kconfig
>> @@ -73,4 +73,10 @@ config TI_SCI_PM_DOMAINS
>>  	  called ti_sci_pm_domains. Note this is needed early in boot before
>>  	  rootfs may be available.
>>  
>> +config TI_SCI_INTA_MSI_DOMAIN
>> +	bool
>> +	select GENERIC_MSI_IRQ_DOMAIN
>> +	help
>> +	  Driver to enable Interrupt Aggregator specific MSI Domain.
>> +
>>  endif # SOC_TI
>> diff --git a/drivers/soc/ti/Makefile b/drivers/soc/ti/Makefile
>> index a22edc0b258a..b3868d392d4f 100644
>> --- a/drivers/soc/ti/Makefile
>> +++ b/drivers/soc/ti/Makefile
>> @@ -8,3 +8,4 @@ obj-$(CONFIG_KEYSTONE_NAVIGATOR_DMA)	+= knav_dma.o
>>  obj-$(CONFIG_AMX3_PM)			+= pm33xx.o
>>  obj-$(CONFIG_WKUP_M3_IPC)		+= wkup_m3_ipc.o
>>  obj-$(CONFIG_TI_SCI_PM_DOMAINS)		+= ti_sci_pm_domains.o
>> +obj-$(CONFIG_TI_SCI_INTA_MSI_DOMAIN)	+= ti_sci_inta_msi.o
>> diff --git a/drivers/soc/ti/ti_sci_inta_msi.c b/drivers/soc/ti/ti_sci_inta_msi.c
>> new file mode 100644
>> index 000000000000..247a5e5f216b
>> --- /dev/null
>> +++ b/drivers/soc/ti/ti_sci_inta_msi.c
>> @@ -0,0 +1,167 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Texas Instruments' K3 Interrupt Aggregator MSI bus
>> + *
>> + * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
>> + *	Lokesh Vutla <lokeshvutla@ti.com>
>> + */
>> +
>> +#include <linux/of_device.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/msi.h>
> 
> Alphabetical ordering, please.

Sure.

> 
>> +#include <linux/soc/ti/ti_sci_inta_msi.h>
>> +#include <linux/soc/ti/ti_sci_protocol.h>
>> +
>> +static void ti_sci_inta_msi_write_msg(struct irq_data *data,
>> +				      struct msi_msg *msg)
>> +{
>> +	/* Nothing to do */
>> +}
>> +
>> +static void ti_sci_inta_msi_compose_msi_msg(struct irq_data *data,
>> +					    struct msi_msg *msg)
>> +{
>> +	/* Nothing to do */
>> +}
>> +
>> +static int ti_sci_inta_msi_request_resources(struct irq_data *data)
>> +{
>> +	data = data->parent_data;
>> +
>> +	return data->chip->irq_request_resources(data);
>> +}
>> +
>> +static void ti_sci_inta_msi_release_resources(struct irq_data *data)
>> +{
>> +	data = data->parent_data;
>> +	data->chip->irq_release_resources(data);
>> +}
> 
> The two functions above are an implementation of
> irq_chip_{request,release}_resource_parent(). Please make them generic
> functions, use them and fix drivers/gpio/gpio-thunderx.c to use them too.

okay, will create  irq_chip_{request,release}_resource_parent() apis and use them.

> 
>> +
>> +static void ti_sci_inta_msi_update_chip_ops(struct msi_domain_info *info)
>> +{
>> +	struct irq_chip *chip = info->chip;
>> +
>> +	WARN_ON(!chip);
> 
> Just doing that isn't going to help, as you'll crash on the following
> line...

Checkpatch is scribbling about it. Will use BUG_ON() in next version.

> 
>> +	if (!chip->irq_mask)
>> +		chip->irq_mask = irq_chip_mask_parent;
>> +	if (!chip->irq_unmask)
>> +		chip->irq_unmask = irq_chip_unmask_parent;
>> +	if (!chip->irq_ack)
>> +		chip->irq_ack = irq_chip_ack_parent;
>> +	if (!chip->irq_set_type)
>> +		chip->irq_set_type = irq_chip_set_type_parent;
>> +	if (!chip->irq_write_msi_msg)
>> +		chip->irq_write_msi_msg = ti_sci_inta_msi_write_msg;
>> +	if (!chip->irq_compose_msi_msg)
>> +		chip->irq_compose_msi_msg = ti_sci_inta_msi_compose_msi_msg;
>> +	if (!chip->irq_request_resources)
>> +		chip->irq_request_resources = ti_sci_inta_msi_request_resources;
>> +	if (!chip->irq_release_resources)
>> +		chip->irq_release_resources = ti_sci_inta_msi_release_resources;
> 
> Is there any case where a client driver wouldn't use the default all the
> time?

I don't think so.

> 
>> +}
>> +
>> +struct irq_domain
>> +*ti_sci_inta_msi_create_irq_domain(struct fwnode_handle *fwnode,
>> +				   struct msi_domain_info *info,
>> +				   struct irq_domain *parent)
>> +{
>> +	struct irq_domain *domain;
>> +
>> +	if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
>> +		ti_sci_inta_msi_update_chip_ops(info);
> 
> If the answer above is "no", then you can happily ignore this flag and
> always populate the callbacks.

Okay, will ignore the flag and populate apis.

> 
>> +
>> +	domain = msi_create_irq_domain(fwnode, info, parent);
>> +	if (domain)
>> +		irq_domain_update_bus_token(domain, DOMAIN_BUS_TI_SCI_INTA_MSI);
>> +
>> +	return domain;
>> +}
>> +EXPORT_SYMBOL_GPL(ti_sci_inta_msi_create_irq_domain);
>> +
>> +static void ti_sci_inta_msi_free_descs(struct device *dev)
>> +{
>> +	struct msi_desc *desc, *tmp;
>> +
>> +	list_for_each_entry_safe(desc, tmp, dev_to_msi_list(dev), list) {
>> +		list_del(&desc->list);
>> +		free_msi_entry(desc);
>> +	}
>> +}
>> +
>> +static int ti_sci_inta_msi_alloc_descs(struct device *dev, u32 dev_id,
>> +				       struct ti_sci_resource *res)
>> +{
>> +	struct msi_desc *msi_desc;
>> +	int set, i, count = 0;
>> +
>> +	for (set = 0; set < res->sets; set++) {
>> +		for (i = 0; i < res->desc[set].num; i++) {
>> +			msi_desc = alloc_msi_entry(dev, 1, NULL);
>> +			if (!msi_desc) {
>> +				ti_sci_inta_msi_free_descs(dev);
>> +				return -ENOMEM;
>> +			}
>> +
>> +			msi_desc->inta.index = res->desc[set].start + i;
>> +			msi_desc->inta.dev_id = dev_id;
> 
> I'm highly suspiscious of this. See further down.

I need to pass dev_id and dev_index to my irqchip driver so that hwirq gets created.

> 
>> +			INIT_LIST_HEAD(&msi_desc->list);
>> +			list_add_tail(&msi_desc->list, dev_to_msi_list(dev));
>> +			count++;
>> +		}
>> +	}
>> +
>> +	return count;
>> +}
>> +
>> +int ti_sci_inta_msi_domain_alloc_irqs(struct platform_device *pdev,
>> +				      struct ti_sci_resource *res)
>> +{
>> +	struct irq_domain *msi_domain;
>> +	int ret, nvec;
>> +
>> +	msi_domain = dev_get_msi_domain(&pdev->dev);
>> +	if (!msi_domain)
>> +		return -EINVAL;
>> +
>> +	if (pdev->id < 0)
>> +		return -ENODEV;
>> +
>> +	nvec = ti_sci_inta_msi_alloc_descs(&pdev->dev, pdev->id, res);
>> +	if (nvec <= 0)
>> +		return nvec;
>> +
>> +	ret = msi_domain_alloc_irqs(msi_domain, &pdev->dev, nvec);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Failed to allocate IRQs %d\n", ret);
>> +		goto cleanup;
>> +	}
>> +
>> +	return 0;
>> +
>> +cleanup:
>> +	ti_sci_inta_msi_free_descs(&pdev->dev);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(ti_sci_inta_msi_domain_alloc_irqs);
>> +
>> +void ti_sci_inta_msi_domain_free_irqs(struct device *dev)
>> +{
>> +	msi_domain_free_irqs(dev->msi_domain, dev);
>> +	ti_sci_inta_msi_free_descs(dev);
>> +}
>> +EXPORT_SYMBOL_GPL(ti_sci_inta_msi_domain_free_irqs);
>> +
>> +unsigned int ti_sci_inta_msi_get_virq(struct platform_device *pdev, u32 index)
>> +{
>> +	struct msi_desc *desc;
>> +
>> +	for_each_msi_entry(desc, &pdev->dev)
>> +		if (desc->inta.index == index && desc->inta.dev_id == pdev->id)
> 
> What is this "index"? Why isn't the right entry the index-th element in
> the msi_desc list?  Worse, the dev_id check. The whole point of having a
> per-device MSI list is that it is, well, per device.

Might be wrong choice of word here. As you know, dev_index need not be
contiguous. ti_sci_resource will have the range of dev_index allocated to the
linux host. using this dev_index irqs gets configured. Even the client drivers
only track this dev_index. Isn't it correct to use this dev_index to translate
to virq?


Thanks and regards,
Lokesh

> 
>> +			return desc->irq;
>> +
>> +	return -ENODEV;
>> +}
>> +EXPORT_SYMBOL_GPL(ti_sci_inta_msi_get_virq);
>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
>> index 61706b430907..07ec8b390161 100644
>> --- a/include/linux/irqdomain.h
>> +++ b/include/linux/irqdomain.h
>> @@ -82,6 +82,7 @@ enum irq_domain_bus_token {
>>  	DOMAIN_BUS_NEXUS,
>>  	DOMAIN_BUS_IPI,
>>  	DOMAIN_BUS_FSL_MC_MSI,
>> +	DOMAIN_BUS_TI_SCI_INTA_MSI,
>>  };
>>  
>>  /**
>> diff --git a/include/linux/msi.h b/include/linux/msi.h
>> index 7e9b81c3b50d..7e12dfc4cb39 100644
>> --- a/include/linux/msi.h
>> +++ b/include/linux/msi.h
>> @@ -47,6 +47,11 @@ struct fsl_mc_msi_desc {
>>  	u16				msi_index;
>>  };
>>  
>> +struct ti_sci_inta_msi_desc {
>> +	u16	dev_id;
>> +	u16	index;
>> +};
>> +
>>  /**
>>   * struct msi_desc - Descriptor structure for MSI based interrupts
>>   * @list:	List head for management
>> @@ -106,6 +111,7 @@ struct msi_desc {
>>  		 */
>>  		struct platform_msi_desc platform;
>>  		struct fsl_mc_msi_desc fsl_mc;
>> +		struct ti_sci_inta_msi_desc inta;
>>  	};
>>  };
>>  
>> diff --git a/include/linux/soc/ti/ti_sci_inta_msi.h b/include/linux/soc/ti/ti_sci_inta_msi.h
>> new file mode 100644
>> index 000000000000..b0ca20ab3f49
>> --- /dev/null
>> +++ b/include/linux/soc/ti/ti_sci_inta_msi.h
>> @@ -0,0 +1,23 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Texas Instruments' K3 TI SCI INTA MSI helper
>> + *
>> + * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
>> + *	Lokesh Vutla <lokeshvutla@ti.com>
>> + */
>> +
>> +#ifndef __INCLUDE_LINUX_TI_SCI_INTA_MSI_H
>> +#define __INCLUDE_LINUX_TI_SCI_INTA_MSI_H
>> +
>> +#include <linux/msi.h>
>> +#include <linux/soc/ti/ti_sci_protocol.h>
>> +
>> +struct irq_domain
>> +*ti_sci_inta_msi_create_irq_domain(struct fwnode_handle *fwnode,
>> +				   struct msi_domain_info *info,
>> +				   struct irq_domain *parent);
>> +int ti_sci_inta_msi_domain_alloc_irqs(struct platform_device *pdev,
>> +				      struct ti_sci_resource *res);
>> +unsigned int ti_sci_inta_msi_get_virq(struct platform_device *dev, u32 index);
>> +void ti_sci_inta_msi_domain_free_irqs(struct device *dev);
>> +#endif /* __INCLUDE_LINUX_IRQCHIP_TI_SCI_INTA_H */
>>
> 
> Thanks,
> 
> 	M.
> 

Re: [PATCH v6 10/12] soc: ti: Add MSI domain bus support for Interrupt Aggregator

[Marc Zyngier]

On 17/04/2019 17:59, Lokesh Vutla wrote:
> 
> 
> On 17/04/19 10:04 PM, Marc Zyngier wrote:
>> On 10/04/2019 05:13, Lokesh Vutla wrote:
>>> With the system coprocessor managing the range allocation of the
>>> inputs to Interrupt Aggregator, it is difficult to represent
>>> the device IRQs from DT.
>>>
>>> The suggestion is to use MSI in such cases where devices wants
>>> to allocate and group interrupts dynamically.
>>>
>>> Create a MSI domain bus layer that allocates and frees MSIs for
>>> a device.
>>>
>>> APIs that are implemented:
>>> - ti_sci_inta_msi_create_irq_domain() that creates a MSI domain
>>> - ti_sci_inta_msi_domain_alloc_irqs() that creates MSIs for the
>>>   specified device and resource.
>>> - ti_sci_inta_msi_domain_free_irqs() frees the irqs attached to the device.
>>> - ti_sci_inta_msi_get_virq() for getting the virq attached to a specific event.
>>>
>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>> ---
>>> Changes since v5:
>>> - Updated the input parametes to all apis
>>> - Updated the default chip ops.
>>> - Prefixed all the apis with ti_sci_inta_
>>>
>>> Marc,
>>> 	Right now ti_sci_resource is being passed for irq allocatons.
>>> I couldn't get to use resources attached to platform_device. Because
>>> platform_device resources are allocated in of_device_alloc() and number
>>> of resources are fixed in it. In order to update the resources, driver
>>> has to do a krealloc(pdev->resources) and update the num of resources.
>>> Is it allowed to update the pdev->resources during probe time? If yes,
>>> Ill be happy to update the patch to use platform_dev resources.
>>
>> My suggestion was for you to define your own bus, device type and co
>> (much like the fsl-mc stuff), and not reuse platform devices at all.
>>
>>>
>>>
>>>  MAINTAINERS                            |   2 +
>>>  drivers/soc/ti/Kconfig                 |   6 +
>>>  drivers/soc/ti/Makefile                |   1 +
>>>  drivers/soc/ti/ti_sci_inta_msi.c       | 167 +++++++++++++++++++++++++
>>>  include/linux/irqdomain.h              |   1 +
>>>  include/linux/msi.h                    |   6 +
>>>  include/linux/soc/ti/ti_sci_inta_msi.h |  23 ++++
>>>  7 files changed, 206 insertions(+)
>>>  create mode 100644 drivers/soc/ti/ti_sci_inta_msi.c
>>>  create mode 100644 include/linux/soc/ti/ti_sci_inta_msi.h
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index ba88b3033fe4..dd31d7cb2fc6 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -15353,6 +15353,8 @@ F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>>  F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt
>>>  F:	drivers/irqchip/irq-ti-sci-intr.c
>>>  F:	drivers/irqchip/irq-ti-sci-inta.c
>>> +F:	include/linux/soc/ti/ti_sci_inta_msi.h
>>> +F:	drivers/soc/ti/ti_sci_inta_msi.c
>>>  
>>>  Texas Instruments ASoC drivers
>>>  M:	Peter Ujfalusi <peter.ujfalusi@ti.com>
>>> diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
>>> index be4570baad96..82f110fe4953 100644
>>> --- a/drivers/soc/ti/Kconfig
>>> +++ b/drivers/soc/ti/Kconfig
>>> @@ -73,4 +73,10 @@ config TI_SCI_PM_DOMAINS
>>>  	  called ti_sci_pm_domains. Note this is needed early in boot before
>>>  	  rootfs may be available.
>>>  
>>> +config TI_SCI_INTA_MSI_DOMAIN
>>> +	bool
>>> +	select GENERIC_MSI_IRQ_DOMAIN
>>> +	help
>>> +	  Driver to enable Interrupt Aggregator specific MSI Domain.
>>> +
>>>  endif # SOC_TI
>>> diff --git a/drivers/soc/ti/Makefile b/drivers/soc/ti/Makefile
>>> index a22edc0b258a..b3868d392d4f 100644
>>> --- a/drivers/soc/ti/Makefile
>>> +++ b/drivers/soc/ti/Makefile
>>> @@ -8,3 +8,4 @@ obj-$(CONFIG_KEYSTONE_NAVIGATOR_DMA)	+= knav_dma.o
>>>  obj-$(CONFIG_AMX3_PM)			+= pm33xx.o
>>>  obj-$(CONFIG_WKUP_M3_IPC)		+= wkup_m3_ipc.o
>>>  obj-$(CONFIG_TI_SCI_PM_DOMAINS)		+= ti_sci_pm_domains.o
>>> +obj-$(CONFIG_TI_SCI_INTA_MSI_DOMAIN)	+= ti_sci_inta_msi.o
>>> diff --git a/drivers/soc/ti/ti_sci_inta_msi.c b/drivers/soc/ti/ti_sci_inta_msi.c
>>> new file mode 100644
>>> index 000000000000..247a5e5f216b
>>> --- /dev/null
>>> +++ b/drivers/soc/ti/ti_sci_inta_msi.c
>>> @@ -0,0 +1,167 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Texas Instruments' K3 Interrupt Aggregator MSI bus
>>> + *
>>> + * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
>>> + *	Lokesh Vutla <lokeshvutla@ti.com>
>>> + */
>>> +
>>> +#include <linux/of_device.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/of_irq.h>
>>> +#include <linux/irq.h>
>>> +#include <linux/irqdomain.h>
>>> +#include <linux/msi.h>
>>
>> Alphabetical ordering, please.
> 
> Sure.
> 
>>
>>> +#include <linux/soc/ti/ti_sci_inta_msi.h>
>>> +#include <linux/soc/ti/ti_sci_protocol.h>
>>> +
>>> +static void ti_sci_inta_msi_write_msg(struct irq_data *data,
>>> +				      struct msi_msg *msg)
>>> +{
>>> +	/* Nothing to do */
>>> +}
>>> +
>>> +static void ti_sci_inta_msi_compose_msi_msg(struct irq_data *data,
>>> +					    struct msi_msg *msg)
>>> +{
>>> +	/* Nothing to do */
>>> +}
>>> +
>>> +static int ti_sci_inta_msi_request_resources(struct irq_data *data)
>>> +{
>>> +	data = data->parent_data;
>>> +
>>> +	return data->chip->irq_request_resources(data);
>>> +}
>>> +
>>> +static void ti_sci_inta_msi_release_resources(struct irq_data *data)
>>> +{
>>> +	data = data->parent_data;
>>> +	data->chip->irq_release_resources(data);
>>> +}
>>
>> The two functions above are an implementation of
>> irq_chip_{request,release}_resource_parent(). Please make them generic
>> functions, use them and fix drivers/gpio/gpio-thunderx.c to use them too.
> 
> okay, will create  irq_chip_{request,release}_resource_parent() apis and use them.
> 
>>
>>> +
>>> +static void ti_sci_inta_msi_update_chip_ops(struct msi_domain_info *info)
>>> +{
>>> +	struct irq_chip *chip = info->chip;
>>> +
>>> +	WARN_ON(!chip);
>>
>> Just doing that isn't going to help, as you'll crash on the following
>> line...
> 
> Checkpatch is scribbling about it. Will use BUG_ON() in next version.

Screw checkpatch, but don't use BUG_ON() either. Instead, do

	if (!WARN_ON(!chip))
		return;

> 
>>
>>> +	if (!chip->irq_mask)
>>> +		chip->irq_mask = irq_chip_mask_parent;
>>> +	if (!chip->irq_unmask)
>>> +		chip->irq_unmask = irq_chip_unmask_parent;
>>> +	if (!chip->irq_ack)
>>> +		chip->irq_ack = irq_chip_ack_parent;
>>> +	if (!chip->irq_set_type)
>>> +		chip->irq_set_type = irq_chip_set_type_parent;
>>> +	if (!chip->irq_write_msi_msg)
>>> +		chip->irq_write_msi_msg = ti_sci_inta_msi_write_msg;
>>> +	if (!chip->irq_compose_msi_msg)
>>> +		chip->irq_compose_msi_msg = ti_sci_inta_msi_compose_msi_msg;
>>> +	if (!chip->irq_request_resources)
>>> +		chip->irq_request_resources = ti_sci_inta_msi_request_resources;
>>> +	if (!chip->irq_release_resources)
>>> +		chip->irq_release_resources = ti_sci_inta_msi_release_resources;
>>
>> Is there any case where a client driver wouldn't use the default all the
>> time?
> 
> I don't think so.
> 
>>
>>> +}
>>> +
>>> +struct irq_domain
>>> +*ti_sci_inta_msi_create_irq_domain(struct fwnode_handle *fwnode,
>>> +				   struct msi_domain_info *info,
>>> +				   struct irq_domain *parent)
>>> +{
>>> +	struct irq_domain *domain;
>>> +
>>> +	if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
>>> +		ti_sci_inta_msi_update_chip_ops(info);
>>
>> If the answer above is "no", then you can happily ignore this flag and
>> always populate the callbacks.
> 
> Okay, will ignore the flag and populate apis.
> 
>>
>>> +
>>> +	domain = msi_create_irq_domain(fwnode, info, parent);
>>> +	if (domain)
>>> +		irq_domain_update_bus_token(domain, DOMAIN_BUS_TI_SCI_INTA_MSI);
>>> +
>>> +	return domain;
>>> +}
>>> +EXPORT_SYMBOL_GPL(ti_sci_inta_msi_create_irq_domain);
>>> +
>>> +static void ti_sci_inta_msi_free_descs(struct device *dev)
>>> +{
>>> +	struct msi_desc *desc, *tmp;
>>> +
>>> +	list_for_each_entry_safe(desc, tmp, dev_to_msi_list(dev), list) {
>>> +		list_del(&desc->list);
>>> +		free_msi_entry(desc);
>>> +	}
>>> +}
>>> +
>>> +static int ti_sci_inta_msi_alloc_descs(struct device *dev, u32 dev_id,
>>> +				       struct ti_sci_resource *res)
>>> +{
>>> +	struct msi_desc *msi_desc;
>>> +	int set, i, count = 0;
>>> +
>>> +	for (set = 0; set < res->sets; set++) {
>>> +		for (i = 0; i < res->desc[set].num; i++) {
>>> +			msi_desc = alloc_msi_entry(dev, 1, NULL);
>>> +			if (!msi_desc) {
>>> +				ti_sci_inta_msi_free_descs(dev);
>>> +				return -ENOMEM;
>>> +			}
>>> +
>>> +			msi_desc->inta.index = res->desc[set].start + i;
>>> +			msi_desc->inta.dev_id = dev_id;
>>
>> I'm highly suspiscious of this. See further down.
> 
> I need to pass dev_id and dev_index to my irqchip driver so that hwirq gets created.
> 
>>
>>> +			INIT_LIST_HEAD(&msi_desc->list);
>>> +			list_add_tail(&msi_desc->list, dev_to_msi_list(dev));
>>> +			count++;
>>> +		}
>>> +	}
>>> +
>>> +	return count;
>>> +}
>>> +
>>> +int ti_sci_inta_msi_domain_alloc_irqs(struct platform_device *pdev,
>>> +				      struct ti_sci_resource *res)
>>> +{
>>> +	struct irq_domain *msi_domain;
>>> +	int ret, nvec;
>>> +
>>> +	msi_domain = dev_get_msi_domain(&pdev->dev);
>>> +	if (!msi_domain)
>>> +		return -EINVAL;
>>> +
>>> +	if (pdev->id < 0)
>>> +		return -ENODEV;
>>> +
>>> +	nvec = ti_sci_inta_msi_alloc_descs(&pdev->dev, pdev->id, res);
>>> +	if (nvec <= 0)
>>> +		return nvec;
>>> +
>>> +	ret = msi_domain_alloc_irqs(msi_domain, &pdev->dev, nvec);
>>> +	if (ret) {
>>> +		dev_err(&pdev->dev, "Failed to allocate IRQs %d\n", ret);
>>> +		goto cleanup;
>>> +	}
>>> +
>>> +	return 0;
>>> +
>>> +cleanup:
>>> +	ti_sci_inta_msi_free_descs(&pdev->dev);
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(ti_sci_inta_msi_domain_alloc_irqs);
>>> +
>>> +void ti_sci_inta_msi_domain_free_irqs(struct device *dev)
>>> +{
>>> +	msi_domain_free_irqs(dev->msi_domain, dev);
>>> +	ti_sci_inta_msi_free_descs(dev);
>>> +}
>>> +EXPORT_SYMBOL_GPL(ti_sci_inta_msi_domain_free_irqs);
>>> +
>>> +unsigned int ti_sci_inta_msi_get_virq(struct platform_device *pdev, u32 index)
>>> +{
>>> +	struct msi_desc *desc;
>>> +
>>> +	for_each_msi_entry(desc, &pdev->dev)
>>> +		if (desc->inta.index == index && desc->inta.dev_id == pdev->id)
>>
>> What is this "index"? Why isn't the right entry the index-th element in
>> the msi_desc list?  Worse, the dev_id check. The whole point of having a
>> per-device MSI list is that it is, well, per device.
> 
> Might be wrong choice of word here. As you know, dev_index need not be
> contiguous. ti_sci_resource will have the range of dev_index allocated to the
> linux host. using this dev_index irqs gets configured. Even the client drivers
> only track this dev_index. Isn't it correct to use this dev_index to translate
> to virq?

OK. But what about the dev_id check? Surely all the MSIs allocated to a
single device have the same devid, right? and that id is equal to pdev->id?

Thanks,

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

Re: [PATCH v6 10/12] soc: ti: Add MSI domain bus support for Interrupt Aggregator

[Lokesh Vutla]

On 17/04/19 10:36 PM, Marc Zyngier wrote:
> On 17/04/2019 17:59, Lokesh Vutla wrote:
>>
>>
>> On 17/04/19 10:04 PM, Marc Zyngier wrote:
>>> On 10/04/2019 05:13, Lokesh Vutla wrote:
>>>> With the system coprocessor managing the range allocation of the
>>>> inputs to Interrupt Aggregator, it is difficult to represent
>>>> the device IRQs from DT.
>>>>
>>>> The suggestion is to use MSI in such cases where devices wants
>>>> to allocate and group interrupts dynamically.
>>>>
>>>> Create a MSI domain bus layer that allocates and frees MSIs for
>>>> a device.
>>>>
>>>> APIs that are implemented:
>>>> - ti_sci_inta_msi_create_irq_domain() that creates a MSI domain
>>>> - ti_sci_inta_msi_domain_alloc_irqs() that creates MSIs for the
>>>>   specified device and resource.
>>>> - ti_sci_inta_msi_domain_free_irqs() frees the irqs attached to the device.
>>>> - ti_sci_inta_msi_get_virq() for getting the virq attached to a specific event.
>>>>
>>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>>> ---
>>>> Changes since v5:
>>>> - Updated the input parametes to all apis
>>>> - Updated the default chip ops.
>>>> - Prefixed all the apis with ti_sci_inta_
>>>>
>>>> Marc,
>>>> 	Right now ti_sci_resource is being passed for irq allocatons.
>>>> I couldn't get to use resources attached to platform_device. Because
>>>> platform_device resources are allocated in of_device_alloc() and number
>>>> of resources are fixed in it. In order to update the resources, driver
>>>> has to do a krealloc(pdev->resources) and update the num of resources.
>>>> Is it allowed to update the pdev->resources during probe time? If yes,
>>>> Ill be happy to update the patch to use platform_dev resources.
>>>
>>> My suggestion was for you to define your own bus, device type and co
>>> (much like the fsl-mc stuff), and not reuse platform devices at all.
>>>
>>>>
>>>>
>>>>  MAINTAINERS                            |   2 +
>>>>  drivers/soc/ti/Kconfig                 |   6 +
>>>>  drivers/soc/ti/Makefile                |   1 +
>>>>  drivers/soc/ti/ti_sci_inta_msi.c       | 167 +++++++++++++++++++++++++
>>>>  include/linux/irqdomain.h              |   1 +
>>>>  include/linux/msi.h                    |   6 +
>>>>  include/linux/soc/ti/ti_sci_inta_msi.h |  23 ++++
>>>>  7 files changed, 206 insertions(+)
>>>>  create mode 100644 drivers/soc/ti/ti_sci_inta_msi.c
>>>>  create mode 100644 include/linux/soc/ti/ti_sci_inta_msi.h
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index ba88b3033fe4..dd31d7cb2fc6 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -15353,6 +15353,8 @@ F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>>>>  F:	Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt
>>>>  F:	drivers/irqchip/irq-ti-sci-intr.c
>>>>  F:	drivers/irqchip/irq-ti-sci-inta.c
>>>> +F:	include/linux/soc/ti/ti_sci_inta_msi.h
>>>> +F:	drivers/soc/ti/ti_sci_inta_msi.c
>>>>  
>>>>  Texas Instruments ASoC drivers
>>>>  M:	Peter Ujfalusi <peter.ujfalusi@ti.com>
>>>> diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
>>>> index be4570baad96..82f110fe4953 100644
>>>> --- a/drivers/soc/ti/Kconfig
>>>> +++ b/drivers/soc/ti/Kconfig
>>>> @@ -73,4 +73,10 @@ config TI_SCI_PM_DOMAINS
>>>>  	  called ti_sci_pm_domains. Note this is needed early in boot before
>>>>  	  rootfs may be available.
>>>>  
>>>> +config TI_SCI_INTA_MSI_DOMAIN
>>>> +	bool
>>>> +	select GENERIC_MSI_IRQ_DOMAIN
>>>> +	help
>>>> +	  Driver to enable Interrupt Aggregator specific MSI Domain.
>>>> +
>>>>  endif # SOC_TI
>>>> diff --git a/drivers/soc/ti/Makefile b/drivers/soc/ti/Makefile
>>>> index a22edc0b258a..b3868d392d4f 100644
>>>> --- a/drivers/soc/ti/Makefile
>>>> +++ b/drivers/soc/ti/Makefile
>>>> @@ -8,3 +8,4 @@ obj-$(CONFIG_KEYSTONE_NAVIGATOR_DMA)	+= knav_dma.o
>>>>  obj-$(CONFIG_AMX3_PM)			+= pm33xx.o
>>>>  obj-$(CONFIG_WKUP_M3_IPC)		+= wkup_m3_ipc.o
>>>>  obj-$(CONFIG_TI_SCI_PM_DOMAINS)		+= ti_sci_pm_domains.o
>>>> +obj-$(CONFIG_TI_SCI_INTA_MSI_DOMAIN)	+= ti_sci_inta_msi.o
>>>> diff --git a/drivers/soc/ti/ti_sci_inta_msi.c b/drivers/soc/ti/ti_sci_inta_msi.c
>>>> new file mode 100644
>>>> index 000000000000..247a5e5f216b
>>>> --- /dev/null
>>>> +++ b/drivers/soc/ti/ti_sci_inta_msi.c
>>>> @@ -0,0 +1,167 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Texas Instruments' K3 Interrupt Aggregator MSI bus
>>>> + *
>>>> + * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
>>>> + *	Lokesh Vutla <lokeshvutla@ti.com>
>>>> + */
>>>> +
>>>> +#include <linux/of_device.h>
>>>> +#include <linux/of_address.h>
>>>> +#include <linux/of_irq.h>
>>>> +#include <linux/irq.h>
>>>> +#include <linux/irqdomain.h>
>>>> +#include <linux/msi.h>
>>>
>>> Alphabetical ordering, please.
>>
>> Sure.
>>
>>>
>>>> +#include <linux/soc/ti/ti_sci_inta_msi.h>
>>>> +#include <linux/soc/ti/ti_sci_protocol.h>
>>>> +
>>>> +static void ti_sci_inta_msi_write_msg(struct irq_data *data,
>>>> +				      struct msi_msg *msg)
>>>> +{
>>>> +	/* Nothing to do */
>>>> +}
>>>> +
>>>> +static void ti_sci_inta_msi_compose_msi_msg(struct irq_data *data,
>>>> +					    struct msi_msg *msg)
>>>> +{
>>>> +	/* Nothing to do */
>>>> +}
>>>> +
>>>> +static int ti_sci_inta_msi_request_resources(struct irq_data *data)
>>>> +{
>>>> +	data = data->parent_data;
>>>> +
>>>> +	return data->chip->irq_request_resources(data);
>>>> +}
>>>> +
>>>> +static void ti_sci_inta_msi_release_resources(struct irq_data *data)
>>>> +{
>>>> +	data = data->parent_data;
>>>> +	data->chip->irq_release_resources(data);
>>>> +}
>>>
>>> The two functions above are an implementation of
>>> irq_chip_{request,release}_resource_parent(). Please make them generic
>>> functions, use them and fix drivers/gpio/gpio-thunderx.c to use them too.
>>
>> okay, will create  irq_chip_{request,release}_resource_parent() apis and use them.
>>
>>>
>>>> +
>>>> +static void ti_sci_inta_msi_update_chip_ops(struct msi_domain_info *info)
>>>> +{
>>>> +	struct irq_chip *chip = info->chip;
>>>> +
>>>> +	WARN_ON(!chip);
>>>
>>> Just doing that isn't going to help, as you'll crash on the following
>>> line...
>>
>> Checkpatch is scribbling about it. Will use BUG_ON() in next version.
> 
> Screw checkpatch, but don't use BUG_ON() either. Instead, do
> 
> 	if (!WARN_ON(!chip))
> 		return;
> 
>>
>>>
>>>> +	if (!chip->irq_mask)
>>>> +		chip->irq_mask = irq_chip_mask_parent;
>>>> +	if (!chip->irq_unmask)
>>>> +		chip->irq_unmask = irq_chip_unmask_parent;
>>>> +	if (!chip->irq_ack)
>>>> +		chip->irq_ack = irq_chip_ack_parent;
>>>> +	if (!chip->irq_set_type)
>>>> +		chip->irq_set_type = irq_chip_set_type_parent;
>>>> +	if (!chip->irq_write_msi_msg)
>>>> +		chip->irq_write_msi_msg = ti_sci_inta_msi_write_msg;
>>>> +	if (!chip->irq_compose_msi_msg)
>>>> +		chip->irq_compose_msi_msg = ti_sci_inta_msi_compose_msi_msg;
>>>> +	if (!chip->irq_request_resources)
>>>> +		chip->irq_request_resources = ti_sci_inta_msi_request_resources;
>>>> +	if (!chip->irq_release_resources)
>>>> +		chip->irq_release_resources = ti_sci_inta_msi_release_resources;
>>>
>>> Is there any case where a client driver wouldn't use the default all the
>>> time?
>>
>> I don't think so.
>>
>>>
>>>> +}
>>>> +
>>>> +struct irq_domain
>>>> +*ti_sci_inta_msi_create_irq_domain(struct fwnode_handle *fwnode,
>>>> +				   struct msi_domain_info *info,
>>>> +				   struct irq_domain *parent)
>>>> +{
>>>> +	struct irq_domain *domain;
>>>> +
>>>> +	if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
>>>> +		ti_sci_inta_msi_update_chip_ops(info);
>>>
>>> If the answer above is "no", then you can happily ignore this flag and
>>> always populate the callbacks.
>>
>> Okay, will ignore the flag and populate apis.
>>
>>>
>>>> +
>>>> +	domain = msi_create_irq_domain(fwnode, info, parent);
>>>> +	if (domain)
>>>> +		irq_domain_update_bus_token(domain, DOMAIN_BUS_TI_SCI_INTA_MSI);
>>>> +
>>>> +	return domain;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(ti_sci_inta_msi_create_irq_domain);
>>>> +
>>>> +static void ti_sci_inta_msi_free_descs(struct device *dev)
>>>> +{
>>>> +	struct msi_desc *desc, *tmp;
>>>> +
>>>> +	list_for_each_entry_safe(desc, tmp, dev_to_msi_list(dev), list) {
>>>> +		list_del(&desc->list);
>>>> +		free_msi_entry(desc);
>>>> +	}
>>>> +}
>>>> +
>>>> +static int ti_sci_inta_msi_alloc_descs(struct device *dev, u32 dev_id,
>>>> +				       struct ti_sci_resource *res)
>>>> +{
>>>> +	struct msi_desc *msi_desc;
>>>> +	int set, i, count = 0;
>>>> +
>>>> +	for (set = 0; set < res->sets; set++) {
>>>> +		for (i = 0; i < res->desc[set].num; i++) {
>>>> +			msi_desc = alloc_msi_entry(dev, 1, NULL);
>>>> +			if (!msi_desc) {
>>>> +				ti_sci_inta_msi_free_descs(dev);
>>>> +				return -ENOMEM;
>>>> +			}
>>>> +
>>>> +			msi_desc->inta.index = res->desc[set].start + i;
>>>> +			msi_desc->inta.dev_id = dev_id;
>>>
>>> I'm highly suspiscious of this. See further down.
>>
>> I need to pass dev_id and dev_index to my irqchip driver so that hwirq gets created.
>>
>>>
>>>> +			INIT_LIST_HEAD(&msi_desc->list);
>>>> +			list_add_tail(&msi_desc->list, dev_to_msi_list(dev));
>>>> +			count++;
>>>> +		}
>>>> +	}
>>>> +
>>>> +	return count;
>>>> +}
>>>> +
>>>> +int ti_sci_inta_msi_domain_alloc_irqs(struct platform_device *pdev,
>>>> +				      struct ti_sci_resource *res)
>>>> +{
>>>> +	struct irq_domain *msi_domain;
>>>> +	int ret, nvec;
>>>> +
>>>> +	msi_domain = dev_get_msi_domain(&pdev->dev);
>>>> +	if (!msi_domain)
>>>> +		return -EINVAL;
>>>> +
>>>> +	if (pdev->id < 0)
>>>> +		return -ENODEV;
>>>> +
>>>> +	nvec = ti_sci_inta_msi_alloc_descs(&pdev->dev, pdev->id, res);
>>>> +	if (nvec <= 0)
>>>> +		return nvec;
>>>> +
>>>> +	ret = msi_domain_alloc_irqs(msi_domain, &pdev->dev, nvec);
>>>> +	if (ret) {
>>>> +		dev_err(&pdev->dev, "Failed to allocate IRQs %d\n", ret);
>>>> +		goto cleanup;
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +
>>>> +cleanup:
>>>> +	ti_sci_inta_msi_free_descs(&pdev->dev);
>>>> +	return ret;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(ti_sci_inta_msi_domain_alloc_irqs);
>>>> +
>>>> +void ti_sci_inta_msi_domain_free_irqs(struct device *dev)
>>>> +{
>>>> +	msi_domain_free_irqs(dev->msi_domain, dev);
>>>> +	ti_sci_inta_msi_free_descs(dev);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(ti_sci_inta_msi_domain_free_irqs);
>>>> +
>>>> +unsigned int ti_sci_inta_msi_get_virq(struct platform_device *pdev, u32 index)
>>>> +{
>>>> +	struct msi_desc *desc;
>>>> +
>>>> +	for_each_msi_entry(desc, &pdev->dev)
>>>> +		if (desc->inta.index == index && desc->inta.dev_id == pdev->id)
>>>
>>> What is this "index"? Why isn't the right entry the index-th element in
>>> the msi_desc list?  Worse, the dev_id check. The whole point of having a
>>> per-device MSI list is that it is, well, per device.
>>
>> Might be wrong choice of word here. As you know, dev_index need not be
>> contiguous. ti_sci_resource will have the range of dev_index allocated to the
>> linux host. using this dev_index irqs gets configured. Even the client drivers
>> only track this dev_index. Isn't it correct to use this dev_index to translate
>> to virq?
> 
> OK. But what about the dev_id check? Surely all the MSIs allocated to a
> single device have the same devid, right? and that id is equal to pdev->id?

yeah, I can drop the dev_id check.

Thanks and regards,
Lokesh

> 
> Thanks,
> 
> 	M.
>