diff mbox series

[RFC,v1,20/21] RISC-V: ACPI: Create PLIC platform device

Message ID 20230803175916.3174453-21-sunilvl@ventanamicro.com (mailing list archive)
State Changes Requested
Headers show
Series RISC-V: ACPI: Add external interrupt controller support | expand

Commit Message

Sunil V L Aug. 3, 2023, 5:59 p.m. UTC 
Since PLIC needs to be a platform device, probe the
MADT and create platform devices for each PLIC in the
system. Use software node framework for the fwnode
which allows to create properties and hence the
actual irqchip driver doesn't need to do anything
different for ACPI vs DT.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
Co-developed-by: Haibo Xu <haibo1.xu@intel.com>
Signed-off-by: Haibo Xu <haibo1.xu@intel.com>
---
 Documentation/riscv/acpi.rst  |   6 ++
 arch/riscv/include/asm/acpi.h |   3 +
 drivers/acpi/riscv/init.c     |   1 +
 drivers/acpi/riscv/init.h     |   1 +
 drivers/acpi/riscv/irqchip.c  | 198 ++++++++++++++++++++++++++++++++++
 5 files changed, 209 insertions(+)

Comments

Conor Dooley Aug. 8, 2023, 8:41 a.m. UTC | #1 
On Thu, Aug 03, 2023 at 11:29:15PM +0530, Sunil V L wrote:
> Since PLIC needs to be a platform device

For the unwashed, why does the PLCI need to be a platform device?
(And while you're at that, please try to make use of the extra ~20
characters per line that you can use here.)

> probe the
> MADT and create platform devices for each PLIC in the
> system. Use software node framework for the fwnode
> which allows to create properties and hence the
> actual irqchip driver doesn't need to do anything
> different for ACPI vs DT.
> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> Co-developed-by: Haibo Xu <haibo1.xu@intel.com>
> Signed-off-by: Haibo Xu <haibo1.xu@intel.com>

> +static struct fwnode_handle *acpi_plic_create_fwnode(struct acpi_madt_plic *plic)
> +{
> +	struct fwnode_handle *fwnode, *parent_fwnode;
> +	struct riscv_irqchip_list *plic_element;
> +	struct software_node_ref_args *refs;
> +	struct property_entry props[8] = {};
> +	int nr_contexts;
> +
> +	props[0] = PROPERTY_ENTRY_U32("riscv,gsi-base", plic->gsi_base);
> +	props[1] = PROPERTY_ENTRY_U32("riscv,ndev", plic->num_irqs);
> +	props[2] = PROPERTY_ENTRY_U32("riscv,max_prio", plic->max_prio);

My OCD wants to know why this gets an _ but the others have a -.

> +	props[3] = PROPERTY_ENTRY_U8("riscv,plic-id", plic->id);
> +	props[4] = PROPERTY_ENTRY_U64("riscv,plic-base", plic->base_addr);
> +	props[5] = PROPERTY_ENTRY_U32("riscv,plic-size", plic->size);
Anup Patel Aug. 8, 2023, 10:57 a.m. UTC | #2 
On Tue, Aug 8, 2023 at 2:12 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Thu, Aug 03, 2023 at 11:29:15PM +0530, Sunil V L wrote:
> > Since PLIC needs to be a platform device
>
> For the unwashed, why does the PLCI need to be a platform device?
> (And while you're at that, please try to make use of the extra ~20
> characters per line that you can use here.)

As suggested by Marc Z, only timers and IPIs need to be probed early.
Everything else needs to be a platform device. The devlink feature of
Linux DD framework ensures that platform devices are probed in the
right order based on the interdependency.

The PATCH5 of the v7 AIA series converts the PLIC driver into a
platform driver. This works perfectly fine.

>
> > probe the
> > MADT and create platform devices for each PLIC in the
> > system. Use software node framework for the fwnode
> > which allows to create properties and hence the
> > actual irqchip driver doesn't need to do anything
> > different for ACPI vs DT.
> >
> > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > Co-developed-by: Haibo Xu <haibo1.xu@intel.com>
> > Signed-off-by: Haibo Xu <haibo1.xu@intel.com>
>
> > +static struct fwnode_handle *acpi_plic_create_fwnode(struct acpi_madt_plic *plic)
> > +{
> > +     struct fwnode_handle *fwnode, *parent_fwnode;
> > +     struct riscv_irqchip_list *plic_element;
> > +     struct software_node_ref_args *refs;
> > +     struct property_entry props[8] = {};
> > +     int nr_contexts;
> > +
> > +     props[0] = PROPERTY_ENTRY_U32("riscv,gsi-base", plic->gsi_base);
> > +     props[1] = PROPERTY_ENTRY_U32("riscv,ndev", plic->num_irqs);
> > +     props[2] = PROPERTY_ENTRY_U32("riscv,max_prio", plic->max_prio);
>
> My OCD wants to know why this gets an _ but the others have a -.
>
> > +     props[3] = PROPERTY_ENTRY_U8("riscv,plic-id", plic->id);
> > +     props[4] = PROPERTY_ENTRY_U64("riscv,plic-base", plic->base_addr);
> > +     props[5] = PROPERTY_ENTRY_U32("riscv,plic-size", plic->size);
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Regards,
Anup
Conor Dooley Aug. 8, 2023, 11:30 a.m. UTC | #3 
On Tue, Aug 08, 2023 at 04:27:16PM +0530, Anup Patel wrote:
> On Tue, Aug 8, 2023 at 2:12 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Thu, Aug 03, 2023 at 11:29:15PM +0530, Sunil V L wrote:
> > > Since PLIC needs to be a platform device
> >
> > For the unwashed, why does the PLCI need to be a platform device?
> > (And while you're at that, please try to make use of the extra ~20
> > characters per line that you can use here.)
> 
> As suggested by Marc Z, only timers and IPIs need to be probed early.
> Everything else needs to be a platform device. The devlink feature of
> Linux DD framework ensures that platform devices are probed in the
> right order based on the interdependency.
> 
> The PATCH5 of the v7 AIA series converts the PLIC driver into a
> platform driver. This works perfectly fine.

To be clear, I want the explanation of why the "PLIC needs to be a
platform device" to be in the commit message.

Thanks,
Conor.

> 
> >
> > > probe the
> > > MADT and create platform devices for each PLIC in the
> > > system. Use software node framework for the fwnode
> > > which allows to create properties and hence the
> > > actual irqchip driver doesn't need to do anything
> > > different for ACPI vs DT.
> > >
> > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > > Co-developed-by: Haibo Xu <haibo1.xu@intel.com>
> > > Signed-off-by: Haibo Xu <haibo1.xu@intel.com>
> >
> > > +static struct fwnode_handle *acpi_plic_create_fwnode(struct acpi_madt_plic *plic)
> > > +{
> > > +     struct fwnode_handle *fwnode, *parent_fwnode;
> > > +     struct riscv_irqchip_list *plic_element;
> > > +     struct software_node_ref_args *refs;
> > > +     struct property_entry props[8] = {};
> > > +     int nr_contexts;
> > > +
> > > +     props[0] = PROPERTY_ENTRY_U32("riscv,gsi-base", plic->gsi_base);
> > > +     props[1] = PROPERTY_ENTRY_U32("riscv,ndev", plic->num_irqs);
> > > +     props[2] = PROPERTY_ENTRY_U32("riscv,max_prio", plic->max_prio);
> >
> > My OCD wants to know why this gets an _ but the others have a -.
> >
> > > +     props[3] = PROPERTY_ENTRY_U8("riscv,plic-id", plic->id);
> > > +     props[4] = PROPERTY_ENTRY_U64("riscv,plic-base", plic->base_addr);
> > > +     props[5] = PROPERTY_ENTRY_U32("riscv,plic-size", plic->size);
Sunil V L Aug. 9, 2023, 5:47 a.m. UTC | #4 
Hi Conor,

On Tue, Aug 08, 2023 at 09:41:45AM +0100, Conor Dooley wrote:
> On Thu, Aug 03, 2023 at 11:29:15PM +0530, Sunil V L wrote:
> > Since PLIC needs to be a platform device
> 
> For the unwashed, why does the PLCI need to be a platform device?
> (And while you're at that, please try to make use of the extra ~20
> characters per line that you can use here.)
>
Sure, let me add more details and use extra characters.
 
> > probe the
> > MADT and create platform devices for each PLIC in the
> > system. Use software node framework for the fwnode
> > which allows to create properties and hence the
> > actual irqchip driver doesn't need to do anything
> > different for ACPI vs DT.
> > 
> > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > Co-developed-by: Haibo Xu <haibo1.xu@intel.com>
> > Signed-off-by: Haibo Xu <haibo1.xu@intel.com>
> 
> > +static struct fwnode_handle *acpi_plic_create_fwnode(struct acpi_madt_plic *plic)
> > +{
> > +	struct fwnode_handle *fwnode, *parent_fwnode;
> > +	struct riscv_irqchip_list *plic_element;
> > +	struct software_node_ref_args *refs;
> > +	struct property_entry props[8] = {};
> > +	int nr_contexts;
> > +
> > +	props[0] = PROPERTY_ENTRY_U32("riscv,gsi-base", plic->gsi_base);
> > +	props[1] = PROPERTY_ENTRY_U32("riscv,ndev", plic->num_irqs);
> > +	props[2] = PROPERTY_ENTRY_U32("riscv,max_prio", plic->max_prio);
> 
> My OCD wants to know why this gets an _ but the others have a -.
> 
It should be -. But with Marc's recommendation, this is no longer
required.

Thanks!
Sunil
diff mbox series

Patch

diff --git a/Documentation/riscv/acpi.rst b/Documentation/riscv/acpi.rst
index 9ea9008288ea..007483dfddc1 100644
--- a/Documentation/riscv/acpi.rst
+++ b/Documentation/riscv/acpi.rst
@@ -35,3 +35,9 @@  to this hart.
 
 ``riscv,gsi-base`` - The global system interrupt number where this APLIC’s
 interrupt inputs start.
+
+3) RISC-V Platform Level Interrupt Controller (PLIC)
+-----------
+
+``riscv,gsi-base`` - The global system interrupt number where this PLIC’s
+interrupt inputs start.
diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h
index 6dde3d63dc0e..163b8eefa744 100644
--- a/arch/riscv/include/asm/acpi.h
+++ b/arch/riscv/include/asm/acpi.h
@@ -71,6 +71,7 @@  int acpi_get_cbo_block_size(struct acpi_table_header *table, unsigned int cpu, u
 struct fwnode_handle *acpi_rintc_create_irqchip_fwnode(struct acpi_madt_rintc *rintc);
 struct fwnode_handle *acpi_imsic_create_fwnode(struct acpi_madt_imsic *imsic);
 struct fwnode_handle *acpi_riscv_get_msi_fwnode(struct device *dev);
+int acpi_plic_get_context_id(u8 plic_id, u16 idx);
 #else
 static inline void acpi_init_rintc_map(void) { }
 static inline struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu)
@@ -95,6 +96,8 @@  static inline struct fwnode_handle *acpi_riscv_get_msi_fwnode(struct device *dev
 {
 	return NULL;
 }
+
+static inline int acpi_plic_get_context_id(u8 plic_id, u16 idx) { return idx; }
 #endif /* CONFIG_ACPI */
 
 #endif /*_ASM_ACPI_H*/
diff --git a/drivers/acpi/riscv/init.c b/drivers/acpi/riscv/init.c
index ee747211174f..cc733ea9ef1d 100644
--- a/drivers/acpi/riscv/init.c
+++ b/drivers/acpi/riscv/init.c
@@ -12,4 +12,5 @@  void __init acpi_riscv_init(void)
 {
 	riscv_acpi_imsic_platform_init();
 	riscv_acpi_aplic_platform_init();
+	riscv_acpi_plic_platform_init();
 }
diff --git a/drivers/acpi/riscv/init.h b/drivers/acpi/riscv/init.h
index 17bcf0baaadb..b4b305d83b3a 100644
--- a/drivers/acpi/riscv/init.h
+++ b/drivers/acpi/riscv/init.h
@@ -3,3 +3,4 @@ 
 
 void __init riscv_acpi_imsic_platform_init(void);
 void __init riscv_acpi_aplic_platform_init(void);
+void __init riscv_acpi_plic_platform_init(void);
diff --git a/drivers/acpi/riscv/irqchip.c b/drivers/acpi/riscv/irqchip.c
index 7fb7befdb303..cb70f0f2294b 100644
--- a/drivers/acpi/riscv/irqchip.c
+++ b/drivers/acpi/riscv/irqchip.c
@@ -25,6 +25,8 @@  static struct fwnode_handle *imsic_acpi_fwnode;
 
 LIST_HEAD(aplic_list);
 
+LIST_HEAD(plic_list);
+
 struct fwnode_handle *acpi_rintc_create_irqchip_fwnode(struct acpi_madt_rintc *rintc)
 {
 	struct property_entry props[6] = {};
@@ -307,3 +309,199 @@  void __init riscv_acpi_aplic_platform_init(void)
 		acpi_set_gsi_to_irq_fallback(aplic_gsi_to_irq);
 	}
 }
+
+static int acpi_plic_get_nr_contexts(u8 plic_id)
+{
+	struct riscv_irqchip_list *rintc_element;
+	struct fwnode_handle *fwnode;
+	struct list_head *i, *tmp;
+	u32 id;
+	int rc, nr_contexts = 0;
+
+	list_for_each_safe(i, tmp, &rintc_list) {
+		rintc_element = list_entry(i, struct riscv_irqchip_list, list);
+		fwnode = rintc_element->fwnode;
+		rc = fwnode_property_read_u32_array(fwnode, "riscv,ext-intc-id", &id, 1);
+		if (rc)
+			continue;
+
+		if (APLIC_PLIC_ID(id) == plic_id)
+			nr_contexts++;
+	}
+
+	return nr_contexts * 2;
+}
+
+int acpi_plic_get_context_id(u8 plic_id, u16 idx)
+{
+	struct riscv_irqchip_list *rintc_element;
+	struct fwnode_handle *fwnode;
+	struct list_head *i, *tmp;
+	u32 id;
+	int rc, nr_contexts = -1;
+
+	list_for_each_safe(i, tmp, &rintc_list) {
+		rintc_element = list_entry(i, struct riscv_irqchip_list, list);
+		fwnode = rintc_element->fwnode;
+		rc = fwnode_property_read_u32_array(fwnode, "riscv,ext-intc-id", &id, 1);
+		if (rc)
+			continue;
+
+		if (APLIC_PLIC_ID(id) == plic_id)
+			nr_contexts++;
+		if (nr_contexts == idx)
+			return IDC_CONTEXT_ID(id);
+	}
+
+	return -1;
+}
+
+static struct fwnode_handle *acpi_plic_create_fwnode(struct acpi_madt_plic *plic)
+{
+	struct fwnode_handle *fwnode, *parent_fwnode;
+	struct riscv_irqchip_list *plic_element;
+	struct software_node_ref_args *refs;
+	struct property_entry props[8] = {};
+	int nr_contexts;
+
+	props[0] = PROPERTY_ENTRY_U32("riscv,gsi-base", plic->gsi_base);
+	props[1] = PROPERTY_ENTRY_U32("riscv,ndev", plic->num_irqs);
+	props[2] = PROPERTY_ENTRY_U32("riscv,max_prio", plic->max_prio);
+	props[3] = PROPERTY_ENTRY_U8("riscv,plic-id", plic->id);
+	props[4] = PROPERTY_ENTRY_U64("riscv,plic-base", plic->base_addr);
+	props[5] = PROPERTY_ENTRY_U32("riscv,plic-size", plic->size);
+
+	nr_contexts = acpi_plic_get_nr_contexts(plic->id);
+	if (nr_contexts) {
+		refs = kcalloc(nr_contexts, sizeof(*refs), GFP_KERNEL);
+		for (int i = 0; i < nr_contexts / 2; i++) {
+			int context_id = acpi_plic_get_context_id(plic->id, i);
+
+			parent_fwnode = acpi_ext_intc_get_rintc_fwnode(plic->id, context_id);
+			refs[2 * i] = SOFTWARE_NODE_REFERENCE(to_software_node(parent_fwnode), 0);
+			refs[2 * i + 1] = SOFTWARE_NODE_REFERENCE(to_software_node(parent_fwnode),
+								  RV_IRQ_EXT);
+		}
+		props[6] = PROPERTY_ENTRY_REF_ARRAY_LEN("interrupts-extended", refs, nr_contexts);
+	}
+
+	fwnode = fwnode_create_software_node_early(props, NULL);
+
+	if (fwnode) {
+		plic_element = kzalloc(sizeof(*plic_element), GFP_KERNEL);
+		if (!plic_element) {
+			fwnode_remove_software_node(fwnode);
+			return NULL;
+		}
+
+		plic_element->fwnode = fwnode;
+		list_add_tail(&plic_element->list, &plic_list);
+	}
+
+	return fwnode;
+}
+
+static struct fwnode_handle *plic_get_gsi_domain_id(u32 gsi)
+{
+	struct riscv_irqchip_list *plic_element;
+	struct fwnode_handle *fwnode;
+	struct list_head *i, *tmp;
+	u32 gsi_base;
+	u32 nr_irqs;
+	int rc;
+
+	list_for_each_safe(i, tmp, &plic_list) {
+		plic_element = list_entry(i, struct riscv_irqchip_list, list);
+		fwnode = plic_element->fwnode;
+		rc = fwnode_property_read_u32_array(fwnode, "riscv,gsi-base", &gsi_base, 1);
+		if (!rc) {
+			rc = fwnode_property_read_u32_array(fwnode, "riscv,ndev", &nr_irqs, 1);
+			if (!rc && (gsi >= gsi_base && gsi < gsi_base + nr_irqs))
+				return fwnode;
+		}
+	}
+
+	return NULL;
+}
+
+static u32 plic_gsi_to_irq(u32 gsi)
+{
+	return acpi_register_gsi(NULL, gsi, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_HIGH);
+}
+
+static int __init plic_create_platform_device(struct fwnode_handle *fwnode)
+{
+	struct platform_device *pdev;
+	u32 plic_size;
+	u8  plic_id;
+	struct resource *res;
+	u64 plic_base;
+	int ret;
+
+	if (!fwnode)
+		return -ENODEV;
+
+	ret = fwnode_property_read_u64_array(fwnode, "riscv,plic-base", &plic_base, 1);
+	if (ret)
+		return -ENODEV;
+
+	ret = fwnode_property_read_u32_array(fwnode, "riscv,plic-size", &plic_size, 1);
+	if (ret)
+		return -ENODEV;
+
+	ret = fwnode_property_read_u8_array(fwnode, "riscv,plic-id", &plic_id, 1);
+	if (ret)
+		return -ENODEV;
+
+	pdev = platform_device_alloc("riscv-plic", plic_id);
+	if (!pdev)
+		return -ENOMEM;
+
+	res = kcalloc(1, sizeof(*res), GFP_KERNEL);
+	if (!res) {
+		ret = -ENOMEM;
+		goto dev_put;
+	}
+
+	res->start = plic_base;
+	res->end = res->start + plic_size - 1;
+	res->flags = IORESOURCE_MEM;
+	ret = platform_device_add_resources(pdev, res, 1);
+	/*
+	 * Resources are duplicated in platform_device_add_resources,
+	 * free their allocated memory
+	 */
+	kfree(res);
+
+	pdev->dev.fwnode = fwnode;
+	ret = platform_device_add(pdev);
+	if (ret)
+		goto dev_put;
+
+	return 0;
+
+dev_put:
+	platform_device_put(pdev);
+	return ret;
+}
+
+static int __init plic_parse_madt(union acpi_subtable_headers *header,
+				  const unsigned long end)
+{
+	struct acpi_madt_plic *plic = (struct acpi_madt_plic *)header;
+	struct fwnode_handle *fwnode;
+
+	fwnode = acpi_plic_create_fwnode(plic);
+	if (fwnode)
+		plic_create_platform_device(fwnode);
+
+	return 0;
+}
+
+void __init riscv_acpi_plic_platform_init(void)
+{
+	if (acpi_table_parse_madt(ACPI_MADT_TYPE_PLIC, plic_parse_madt, 0) > 0) {
+		acpi_set_irq_model(ACPI_IRQ_MODEL_PLIC, plic_get_gsi_domain_id);
+		acpi_set_gsi_to_irq_fallback(plic_gsi_to_irq);
+	}
+}