This series adds support for the below ECR approved by ASWG.
1) MADT - https://drive.google.com/file/d/1oMGPyOD58JaPgMl1pKasT-VKsIKia7zR/view?usp=sharing
The series primarily enables irqchip drivers for RISC-V ACPI based
platforms.
The series can be broadly categorized like below.
1) PCI ACPI related functions are migrated from arm64 to common file so
that we don't need to duplicate them for RISC-V.
2) Added support for re-ordering the probe of interrupt controllers when
IRQCHIP_ACPI_DECLARE is used.
3) To ensure probe order between interrupt controllers and devices,
implicit dependency is created similar to when _DEP is present.
4) Added 8250 serial driver for a generic 16550 UART which is enumerated
as ACPI platform device.
5) ACPI support added in RISC-V interrupt controller drivers.
Changes since RFC v4:
1) Removed RFC tag as the RFCv4 design looked reasonable.
2) Dropped PCI patch needed to avoid warning when there is no MSI
controller. This will be sent later separately after the
current series.
3) Dropped PNP handling of _DEP since there is new ACPI ID for
generic 16550 UART. Added the serial driver patch instead.
4) Rebased to latest linux-next.
5) Reordered/squashed patches in the series
Changes since RFC v3:
1) Moved to _DEP method instead of fw_devlink.
2) PLIC/APLIC driver probe using namespace devices.
3) Handling PNP devices as part of clearing dependency.
4) Rebased to latest linux-next to get AIA DT drivers.
Changes since RFC v2:
1) Introduced fw_devlink for ACPI nodes for IRQ dependency.
2) Dropped patches in drivers which are not required due to
fw_devlink support.
3) Dropped pci_set_msi() patch and added a patch in
pci_create_root_bus().
4) Updated pnp_irq() patch so that none of the actual PNP
drivers need to change.
Changes since RFC v1:
1) Abandoned swnode approach as per Marc's feedback.
2) To cope up with AIA series changes which changed irqchip driver
probe from core_initcall() to platform_driver, added patches
to support deferred probing.
3) Rebased on top of Anup's AIA v11 and added tags.
To test the series,
1) qemu should be built using the riscv_acpi_namespace_v2 branch at
https://github.com/vlsunil/qemu.git
2) EDK2 should be built using the instructions at:
https://github.com/tianocore/edk2/blob/master/OvmfPkg/RiscVVirt/README.md
NOTE: One should be able to use u-boot as well as per instructions from Björn.
https://lore.kernel.org/lkml/[email protected]/
3) Build Linux using this series.
Run Qemu:
qemu-system-riscv64 \
-M virt,pflash0=pflash0,pflash1=pflash1,aia=aplic-imsic \
-m 2G -smp 8 \
-serial mon:stdio \
-device virtio-gpu-pci -full-screen \
-device qemu-xhci \
-device usb-kbd \
-blockdev node-name=pflash0,driver=file,read-only=on,filename=RISCV_VIRT_CODE.fd \
-blockdev node-name=pflash1,driver=file,filename=RISCV_VIRT_VARS.fd \
-netdev user,id=net0 -device virtio-net-pci,netdev=net0 \
-kernel arch/riscv/boot/Image \
-initrd rootfs.cpio \
-append "root=/dev/ram ro console=ttyS0 rootwait earlycon=uart8250,mmio,0x10000000"
To boot with APLIC only, use aia=aplic.
To boot with PLIC, remove aia= option.
This series is also available in acpi_b2_v5 branch at
https://github.com/vlsunil/linux.git
Based-on: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tag/?h=next-20240501
Sunil V L (17):
arm64: PCI: Migrate ACPI related functions to pci-acpi.c
ACPI: scan: Add a weak function to reorder the IRQCHIP probe
ACPI: bus: Add acpi_riscv_init function
ACPI: scan: Refactor dependency creation
ACPI: scan: Add RISC-V interrupt controllers to honor list
ACPI: scan: Define weak function to populate dependencies
ACPI: bus: Add RINTC IRQ model for RISC-V
ACPI: pci_link: Clear the dependencies after probe
ACPI: RISC-V: Implement PCI related functionality
ACPI: RISC-V: Implement function to reorder irqchip probe entries
ACPI: RISC-V: Initialize GSI mapping structures
ACPI: RISC-V: Implement function to add implicit dependencies
irqchip/riscv-intc: Add ACPI support for AIA
irqchip/riscv-imsic: Add ACPI support
irqchip/riscv-aplic: Add ACPI support
irqchip/sifive-plic: Add ACPI support
serial: 8250: Add 8250_acpi driver
arch/arm64/kernel/pci.c | 191 ------------
arch/riscv/Kconfig | 3 +
arch/riscv/configs/defconfig | 1 +
arch/riscv/include/asm/irq.h | 57 ++++
arch/riscv/kernel/acpi.c | 31 +-
drivers/acpi/Kconfig | 3 +
drivers/acpi/bus.c | 4 +
drivers/acpi/pci_link.c | 3 +
drivers/acpi/riscv/Makefile | 2 +-
drivers/acpi/riscv/init.c | 14 +
drivers/acpi/riscv/init.h | 4 +
drivers/acpi/riscv/irq.c | 329 +++++++++++++++++++++
drivers/acpi/scan.c | 65 ++--
drivers/irqchip/irq-riscv-aplic-direct.c | 20 +-
drivers/irqchip/irq-riscv-aplic-main.c | 70 +++--
drivers/irqchip/irq-riscv-aplic-main.h | 1 +
drivers/irqchip/irq-riscv-aplic-msi.c | 9 +-
drivers/irqchip/irq-riscv-imsic-early.c | 52 +++-
drivers/irqchip/irq-riscv-imsic-platform.c | 32 +-
drivers/irqchip/irq-riscv-imsic-state.c | 115 ++++---
drivers/irqchip/irq-riscv-imsic-state.h | 2 +-
drivers/irqchip/irq-riscv-intc.c | 102 ++++++-
drivers/irqchip/irq-sifive-plic.c | 89 ++++--
drivers/pci/pci-acpi.c | 182 ++++++++++++
drivers/tty/serial/8250/8250_acpi.c | 96 ++++++
drivers/tty/serial/8250/Kconfig | 8 +
drivers/tty/serial/8250/Makefile | 1 +
include/acpi/acpi_bus.h | 2 +
include/linux/acpi.h | 9 +
include/linux/irqchip/riscv-imsic.h | 10 +
30 files changed, 1155 insertions(+), 352 deletions(-)
create mode 100644 drivers/acpi/riscv/init.c
create mode 100644 drivers/acpi/riscv/init.h
create mode 100644 drivers/acpi/riscv/irq.c
create mode 100644 drivers/tty/serial/8250/8250_acpi.c
--
2.40.1
The functions defined in arm64 for ACPI support are required
for RISC-V also. To avoid duplication, move these functions
to common location.
Signed-off-by: Sunil V L <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
---
arch/arm64/kernel/pci.c | 191 ----------------------------------------
drivers/pci/pci-acpi.c | 182 ++++++++++++++++++++++++++++++++++++++
2 files changed, 182 insertions(+), 191 deletions(-)
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index f872c57e9909..fd9a7bed83ce 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -6,28 +6,7 @@
* Copyright (C) 2014 ARM Ltd.
*/
-#include <linux/acpi.h>
-#include <linux/init.h>
-#include <linux/io.h>
-#include <linux/kernel.h>
-#include <linux/mm.h>
#include <linux/pci.h>
-#include <linux/pci-acpi.h>
-#include <linux/pci-ecam.h>
-#include <linux/slab.h>
-
-#ifdef CONFIG_ACPI
-/*
- * Try to assign the IRQ number when probing a new device
- */
-int pcibios_alloc_irq(struct pci_dev *dev)
-{
- if (!acpi_disabled)
- acpi_pci_irq_enable(dev);
-
- return 0;
-}
-#endif
/*
* raw_pci_read/write - Platform-specific PCI config space access.
@@ -61,173 +40,3 @@ int pcibus_to_node(struct pci_bus *bus)
EXPORT_SYMBOL(pcibus_to_node);
#endif
-
-#ifdef CONFIG_ACPI
-
-struct acpi_pci_generic_root_info {
- struct acpi_pci_root_info common;
- struct pci_config_window *cfg; /* config space mapping */
-};
-
-int acpi_pci_bus_find_domain_nr(struct pci_bus *bus)
-{
- struct pci_config_window *cfg = bus->sysdata;
- struct acpi_device *adev = to_acpi_device(cfg->parent);
- struct acpi_pci_root *root = acpi_driver_data(adev);
-
- return root->segment;
-}
-
-int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
-{
- struct pci_config_window *cfg;
- struct acpi_device *adev;
- struct device *bus_dev;
-
- if (acpi_disabled)
- return 0;
-
- cfg = bridge->bus->sysdata;
-
- /*
- * On Hyper-V there is no corresponding ACPI device for a root bridge,
- * therefore ->parent is set as NULL by the driver. And set 'adev' as
- * NULL in this case because there is no proper ACPI device.
- */
- if (!cfg->parent)
- adev = NULL;
- else
- adev = to_acpi_device(cfg->parent);
-
- bus_dev = &bridge->bus->dev;
-
- ACPI_COMPANION_SET(&bridge->dev, adev);
- set_dev_node(bus_dev, acpi_get_node(acpi_device_handle(adev)));
-
- return 0;
-}
-
-static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci)
-{
- struct resource_entry *entry, *tmp;
- int status;
-
- status = acpi_pci_probe_root_resources(ci);
- resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
- if (!(entry->res->flags & IORESOURCE_WINDOW))
- resource_list_destroy_entry(entry);
- }
- return status;
-}
-
-/*
- * Lookup the bus range for the domain in MCFG, and set up config space
- * mapping.
- */
-static struct pci_config_window *
-pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root)
-{
- struct device *dev = &root->device->dev;
- struct resource *bus_res = &root->secondary;
- u16 seg = root->segment;
- const struct pci_ecam_ops *ecam_ops;
- struct resource cfgres;
- struct acpi_device *adev;
- struct pci_config_window *cfg;
- int ret;
-
- ret = pci_mcfg_lookup(root, &cfgres, &ecam_ops);
- if (ret) {
- dev_err(dev, "%04x:%pR ECAM region not found\n", seg, bus_res);
- return NULL;
- }
-
- adev = acpi_resource_consumer(&cfgres);
- if (adev)
- dev_info(dev, "ECAM area %pR reserved by %s\n", &cfgres,
- dev_name(&adev->dev));
- else
- dev_warn(dev, FW_BUG "ECAM area %pR not reserved in ACPI namespace\n",
- &cfgres);
-
- cfg = pci_ecam_create(dev, &cfgres, bus_res, ecam_ops);
- if (IS_ERR(cfg)) {
- dev_err(dev, "%04x:%pR error %ld mapping ECAM\n", seg, bus_res,
- PTR_ERR(cfg));
- return NULL;
- }
-
- return cfg;
-}
-
-/* release_info: free resources allocated by init_info */
-static void pci_acpi_generic_release_info(struct acpi_pci_root_info *ci)
-{
- struct acpi_pci_generic_root_info *ri;
-
- ri = container_of(ci, struct acpi_pci_generic_root_info, common);
- pci_ecam_free(ri->cfg);
- kfree(ci->ops);
- kfree(ri);
-}
-
-/* Interface called from ACPI code to setup PCI host controller */
-struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
-{
- struct acpi_pci_generic_root_info *ri;
- struct pci_bus *bus, *child;
- struct acpi_pci_root_ops *root_ops;
- struct pci_host_bridge *host;
-
- ri = kzalloc(sizeof(*ri), GFP_KERNEL);
- if (!ri)
- return NULL;
-
- root_ops = kzalloc(sizeof(*root_ops), GFP_KERNEL);
- if (!root_ops) {
- kfree(ri);
- return NULL;
- }
-
- ri->cfg = pci_acpi_setup_ecam_mapping(root);
- if (!ri->cfg) {
- kfree(ri);
- kfree(root_ops);
- return NULL;
- }
-
- root_ops->release_info = pci_acpi_generic_release_info;
- root_ops->prepare_resources = pci_acpi_root_prepare_resources;
- root_ops->pci_ops = (struct pci_ops *)&ri->cfg->ops->pci_ops;
- bus = acpi_pci_root_create(root, root_ops, &ri->common, ri->cfg);
- if (!bus)
- return NULL;
-
- /* If we must preserve the resource configuration, claim now */
- host = pci_find_host_bridge(bus);
- if (host->preserve_config)
- pci_bus_claim_resources(bus);
-
- /*
- * Assign whatever was left unassigned. If we didn't claim above,
- * this will reassign everything.
- */
- pci_assign_unassigned_root_bus_resources(bus);
-
- list_for_each_entry(child, &bus->children, node)
- pcie_bus_configure_settings(child);
-
- return bus;
-}
-
-void pcibios_add_bus(struct pci_bus *bus)
-{
- acpi_pci_add_bus(bus);
-}
-
-void pcibios_remove_bus(struct pci_bus *bus)
-{
- acpi_pci_remove_bus(bus);
-}
-
-#endif
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 004575091596..e8d84fa435da 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -15,6 +15,7 @@
#include <linux/pci_hotplug.h>
#include <linux/module.h>
#include <linux/pci-acpi.h>
+#include <linux/pci-ecam.h>
#include <linux/pm_runtime.h>
#include <linux/pm_qos.h>
#include <linux/rwsem.h>
@@ -1519,3 +1520,184 @@ static int __init acpi_pci_init(void)
return 0;
}
arch_initcall(acpi_pci_init);
+
+#if defined(CONFIG_ARM64)
+
+/*
+ * Try to assign the IRQ number when probing a new device
+ */
+int pcibios_alloc_irq(struct pci_dev *dev)
+{
+ if (!acpi_disabled)
+ acpi_pci_irq_enable(dev);
+
+ return 0;
+}
+
+struct acpi_pci_generic_root_info {
+ struct acpi_pci_root_info common;
+ struct pci_config_window *cfg; /* config space mapping */
+};
+
+int acpi_pci_bus_find_domain_nr(struct pci_bus *bus)
+{
+ struct pci_config_window *cfg = bus->sysdata;
+ struct acpi_device *adev = to_acpi_device(cfg->parent);
+ struct acpi_pci_root *root = acpi_driver_data(adev);
+
+ return root->segment;
+}
+
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+ struct pci_config_window *cfg;
+ struct acpi_device *adev;
+ struct device *bus_dev;
+
+ if (acpi_disabled)
+ return 0;
+
+ cfg = bridge->bus->sysdata;
+
+ /*
+ * On Hyper-V there is no corresponding ACPI device for a root bridge,
+ * therefore ->parent is set as NULL by the driver. And set 'adev' as
+ * NULL in this case because there is no proper ACPI device.
+ */
+ if (!cfg->parent)
+ adev = NULL;
+ else
+ adev = to_acpi_device(cfg->parent);
+
+ bus_dev = &bridge->bus->dev;
+
+ ACPI_COMPANION_SET(&bridge->dev, adev);
+ set_dev_node(bus_dev, acpi_get_node(acpi_device_handle(adev)));
+
+ return 0;
+}
+
+static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci)
+{
+ struct resource_entry *entry, *tmp;
+ int status;
+
+ status = acpi_pci_probe_root_resources(ci);
+ resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
+ if (!(entry->res->flags & IORESOURCE_WINDOW))
+ resource_list_destroy_entry(entry);
+ }
+ return status;
+}
+
+/*
+ * Lookup the bus range for the domain in MCFG, and set up config space
+ * mapping.
+ */
+static struct pci_config_window *
+pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root)
+{
+ struct device *dev = &root->device->dev;
+ struct resource *bus_res = &root->secondary;
+ u16 seg = root->segment;
+ const struct pci_ecam_ops *ecam_ops;
+ struct resource cfgres;
+ struct acpi_device *adev;
+ struct pci_config_window *cfg;
+ int ret;
+
+ ret = pci_mcfg_lookup(root, &cfgres, &ecam_ops);
+ if (ret) {
+ dev_err(dev, "%04x:%pR ECAM region not found\n", seg, bus_res);
+ return NULL;
+ }
+
+ adev = acpi_resource_consumer(&cfgres);
+ if (adev)
+ dev_info(dev, "ECAM area %pR reserved by %s\n", &cfgres,
+ dev_name(&adev->dev));
+ else
+ dev_warn(dev, FW_BUG "ECAM area %pR not reserved in ACPI namespace\n",
+ &cfgres);
+
+ cfg = pci_ecam_create(dev, &cfgres, bus_res, ecam_ops);
+ if (IS_ERR(cfg)) {
+ dev_err(dev, "%04x:%pR error %ld mapping ECAM\n", seg, bus_res,
+ PTR_ERR(cfg));
+ return NULL;
+ }
+
+ return cfg;
+}
+
+/* release_info: free resources allocated by init_info */
+static void pci_acpi_generic_release_info(struct acpi_pci_root_info *ci)
+{
+ struct acpi_pci_generic_root_info *ri;
+
+ ri = container_of(ci, struct acpi_pci_generic_root_info, common);
+ pci_ecam_free(ri->cfg);
+ kfree(ci->ops);
+ kfree(ri);
+}
+
+/* Interface called from ACPI code to setup PCI host controller */
+struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
+{
+ struct acpi_pci_generic_root_info *ri;
+ struct pci_bus *bus, *child;
+ struct acpi_pci_root_ops *root_ops;
+ struct pci_host_bridge *host;
+
+ ri = kzalloc(sizeof(*ri), GFP_KERNEL);
+ if (!ri)
+ return NULL;
+
+ root_ops = kzalloc(sizeof(*root_ops), GFP_KERNEL);
+ if (!root_ops) {
+ kfree(ri);
+ return NULL;
+ }
+
+ ri->cfg = pci_acpi_setup_ecam_mapping(root);
+ if (!ri->cfg) {
+ kfree(ri);
+ kfree(root_ops);
+ return NULL;
+ }
+
+ root_ops->release_info = pci_acpi_generic_release_info;
+ root_ops->prepare_resources = pci_acpi_root_prepare_resources;
+ root_ops->pci_ops = (struct pci_ops *)&ri->cfg->ops->pci_ops;
+ bus = acpi_pci_root_create(root, root_ops, &ri->common, ri->cfg);
+ if (!bus)
+ return NULL;
+
+ /* If we must preserve the resource configuration, claim now */
+ host = pci_find_host_bridge(bus);
+ if (host->preserve_config)
+ pci_bus_claim_resources(bus);
+
+ /*
+ * Assign whatever was left unassigned. If we didn't claim above,
+ * this will reassign everything.
+ */
+ pci_assign_unassigned_root_bus_resources(bus);
+
+ list_for_each_entry(child, &bus->children, node)
+ pcie_bus_configure_settings(child);
+
+ return bus;
+}
+
+void pcibios_add_bus(struct pci_bus *bus)
+{
+ acpi_pci_add_bus(bus);
+}
+
+void pcibios_remove_bus(struct pci_bus *bus)
+{
+ acpi_pci_remove_bus(bus);
+}
+
+#endif
--
2.40.1
Unlike OF framework, the irqchip probe using IRQCHIP_ACPI_DECLARE has no
order defined. Depending on the Makefile is not a good idea. So,
usually it is worked around by mandating only root interrupt controller
probed using IRQCHIP_ACPI_DECLARE and other interrupt controllers are
probed via cascade mechanism.
However, this is also not a clean solution because if there are multiple
root controllers (ex: RINTC in RISC-V which is per CPU) which need to be
probed first, then the cascade will happen for every root controller.
So, introduce a architecture specific weak function to order the probing
of the interrupt controllers which can be implemented by different
architectures as per their interrupt controller hierarchy.
Signed-off-by: Sunil V L <[email protected]>
---
drivers/acpi/scan.c | 3 +++
include/linux/acpi.h | 2 ++
2 files changed, 5 insertions(+)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 4804a2ad1578..837b8fc89dfb 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2744,6 +2744,8 @@ static int __init acpi_match_madt(union acpi_subtable_headers *header,
return 0;
}
+void __weak arch_sort_irqchip_probe(struct acpi_probe_entry *ap_head, int nr) { }
+
int __init __acpi_probe_device_table(struct acpi_probe_entry *ap_head, int nr)
{
int count = 0;
@@ -2752,6 +2754,7 @@ int __init __acpi_probe_device_table(struct acpi_probe_entry *ap_head, int nr)
return 0;
mutex_lock(&acpi_probe_mutex);
+ arch_sort_irqchip_probe(ap_head, nr);
for (ape = ap_head; nr; ape++, nr--) {
if (ACPI_COMPARE_NAMESEG(ACPI_SIG_MADT, ape->id)) {
acpi_probe_count = 0;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index c2ae33b8dbb6..1afa289f1f4e 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1339,6 +1339,8 @@ struct acpi_probe_entry {
kernel_ulong_t driver_data;
};
+void arch_sort_irqchip_probe(struct acpi_probe_entry *ap_head, int nr);
+
#define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, \
valid, data, fn) \
static const struct acpi_probe_entry __acpi_probe_##name \
--
2.40.1
Add a new function for RISC-V to do any architecture specific
initialization. This function will be used to create platform devices
like APLIC, PLIC, RISC-V IOMMU etc. This is similar to acpi_arm_init().
Signed-off-by: Sunil V L <[email protected]>
---
drivers/acpi/bus.c | 1 +
drivers/acpi/riscv/Makefile | 2 +-
drivers/acpi/riscv/init.c | 12 ++++++++++++
include/linux/acpi.h | 6 ++++++
4 files changed, 20 insertions(+), 1 deletion(-)
create mode 100644 drivers/acpi/riscv/init.c
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 844c46447914..17ee483c3bf4 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1446,6 +1446,7 @@ static int __init acpi_init(void)
acpi_hest_init();
acpi_ghes_init();
acpi_arm_init();
+ acpi_riscv_init();
acpi_scan_init();
acpi_ec_init();
acpi_debugfs_init();
diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile
index 86b0925f612d..877de00d1b50 100644
--- a/drivers/acpi/riscv/Makefile
+++ b/drivers/acpi/riscv/Makefile
@@ -1,4 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
-obj-y += rhct.o
+obj-y += rhct.o init.o
obj-$(CONFIG_ACPI_PROCESSOR_IDLE) += cpuidle.o
obj-$(CONFIG_ACPI_CPPC_LIB) += cppc.o
diff --git a/drivers/acpi/riscv/init.c b/drivers/acpi/riscv/init.c
new file mode 100644
index 000000000000..5f7571143245
--- /dev/null
+++ b/drivers/acpi/riscv/init.c
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023-2024, Ventana Micro Systems Inc
+ * Author: Sunil V L <[email protected]>
+ *
+ */
+
+#include <linux/acpi.h>
+
+void __init acpi_riscv_init(void)
+{
+}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 1afa289f1f4e..846a4001b5e0 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1527,6 +1527,12 @@ void acpi_arm_init(void);
static inline void acpi_arm_init(void) { }
#endif
+#ifdef CONFIG_RISCV
+void acpi_riscv_init(void);
+#else
+static inline void acpi_riscv_init(void) { }
+#endif
+
#ifdef CONFIG_ACPI_PCC
void acpi_init_pcc(void);
#else
--
2.40.1
Some architectures like RISC-V will use implicit dependencies like GSI
map to create dependencies between interrupt controller and devices. To
support doing that, the function which creates the dependency, is
refactored bit and made public so that dependency can be added from
outside of scan.c as well.
Signed-off-by: Sunil V L <[email protected]>
---
drivers/acpi/scan.c | 48 ++++++++++++++++++++++++-----------------
include/acpi/acpi_bus.h | 1 +
2 files changed, 29 insertions(+), 20 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 837b8fc89dfb..3e3320ddb3da 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2023,33 +2023,18 @@ static void acpi_scan_init_hotplug(struct acpi_device *adev)
}
}
-static u32 acpi_scan_check_dep(acpi_handle handle)
+int acpi_scan_add_dep(acpi_handle handle, struct acpi_handle_list *dep_devices)
{
- struct acpi_handle_list dep_devices;
u32 count;
int i;
- /*
- * Check for _HID here to avoid deferring the enumeration of:
- * 1. PCI devices.
- * 2. ACPI nodes describing USB ports.
- * Still, checking for _HID catches more then just these cases ...
- */
- if (!acpi_has_method(handle, "_DEP") || !acpi_has_method(handle, "_HID"))
- return 0;
-
- if (!acpi_evaluate_reference(handle, "_DEP", NULL, &dep_devices)) {
- acpi_handle_debug(handle, "Failed to evaluate _DEP.\n");
- return 0;
- }
-
- for (count = 0, i = 0; i < dep_devices.count; i++) {
+ for (count = 0, i = 0; i < dep_devices->count; i++) {
struct acpi_device_info *info;
struct acpi_dep_data *dep;
bool skip, honor_dep;
acpi_status status;
- status = acpi_get_object_info(dep_devices.handles[i], &info);
+ status = acpi_get_object_info(dep_devices->handles[i], &info);
if (ACPI_FAILURE(status)) {
acpi_handle_debug(handle, "Error reading _DEP device info\n");
continue;
@@ -2068,7 +2053,7 @@ static u32 acpi_scan_check_dep(acpi_handle handle)
count++;
- dep->supplier = dep_devices.handles[i];
+ dep->supplier = dep_devices->handles[i];
dep->consumer = handle;
dep->honor_dep = honor_dep;
@@ -2077,7 +2062,30 @@ static u32 acpi_scan_check_dep(acpi_handle handle)
mutex_unlock(&acpi_dep_list_lock);
}
- acpi_handle_list_free(&dep_devices);
+ acpi_handle_list_free(dep_devices);
+ return count;
+}
+
+static u32 acpi_scan_check_dep(acpi_handle handle)
+{
+ struct acpi_handle_list dep_devices;
+ u32 count = 0;
+
+ /*
+ * Check for _HID here to avoid deferring the enumeration of:
+ * 1. PCI devices.
+ * 2. ACPI nodes describing USB ports.
+ * Still, checking for _HID catches more then just these cases ...
+ */
+ if (!acpi_has_method(handle, "_DEP") || !acpi_has_method(handle, "_HID"))
+ return count;
+
+ if (!acpi_evaluate_reference(handle, "_DEP", NULL, &dep_devices)) {
+ acpi_handle_debug(handle, "Failed to evaluate _DEP.\n");
+ return count;
+ }
+
+ count += acpi_scan_add_dep(handle, &dep_devices);
return count;
}
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 1a4dfd7a1c4a..28a9b87c23fa 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -993,6 +993,7 @@ static inline void acpi_put_acpi_dev(struct acpi_device *adev)
int acpi_wait_for_acpi_ipmi(void);
+int acpi_scan_add_dep(acpi_handle handle, struct acpi_handle_list *dep_devices);
#else /* CONFIG_ACPI */
static inline int register_acpi_bus_type(void *bus) { return 0; }
--
2.40.1
RISC-V PLIC and APLIC will have dependency from devices using GSI. So, add
these devices to the honor list.
Signed-off-by: Sunil V L <[email protected]>
---
drivers/acpi/scan.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 3e3320ddb3da..beded069cb0a 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -832,6 +832,8 @@ static const char * const acpi_honor_dep_ids[] = {
"INTC1095", /* IVSC (ADL) driver must be loaded to allow i2c access to camera sensors */
"INTC100A", /* IVSC (RPL) driver must be loaded to allow i2c access to camera sensors */
"INTC10CF", /* IVSC (MTL) driver must be loaded to allow i2c access to camera sensors */
+ "RSCV0001", /* RISC-V PLIC */
+ "RSCV0002", /* RISC-V APLIC */
NULL
};
--
2.40.1
Some architectures like RISC-V need to add dependencies without explicit
_DEP. Define a weak function which can be implemented by the architecture.
Signed-off-by: Sunil V L <[email protected]>
---
drivers/acpi/scan.c | 11 +++++++++++
include/acpi/acpi_bus.h | 1 +
2 files changed, 12 insertions(+)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index beded069cb0a..3eeb4ce39fcc 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2068,11 +2068,22 @@ int acpi_scan_add_dep(acpi_handle handle, struct acpi_handle_list *dep_devices)
return count;
}
+u32 __weak arch_acpi_add_auto_dep(acpi_handle handle) { return 0; }
+
static u32 acpi_scan_check_dep(acpi_handle handle)
{
struct acpi_handle_list dep_devices;
u32 count = 0;
+ /*
+ * Some architectures like RISC-V need to add dependencies for
+ * all devices which use GSI to the interrupt controller so that
+ * interrupt controller is probed before any of those devices.
+ * Instead of mandating _DEP on all the devices, detect the
+ * dependency and add automatically.
+ */
+ count += arch_acpi_add_auto_dep(handle);
+
/*
* Check for _HID here to avoid deferring the enumeration of:
* 1. PCI devices.
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 28a9b87c23fa..5fba4075d764 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -994,6 +994,7 @@ static inline void acpi_put_acpi_dev(struct acpi_device *adev)
int acpi_wait_for_acpi_ipmi(void);
int acpi_scan_add_dep(acpi_handle handle, struct acpi_handle_list *dep_devices);
+u32 arch_acpi_add_auto_dep(acpi_handle handle);
#else /* CONFIG_ACPI */
static inline int register_acpi_bus_type(void *bus) { return 0; }
--
2.40.1
Add the IRQ model for RISC-V INTC so that acpi_set_irq_model can use this
for RISC-V.
Signed-off-by: Sunil V L <[email protected]>
---
drivers/acpi/bus.c | 3 +++
include/linux/acpi.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 17ee483c3bf4..6739db258a95 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1190,6 +1190,9 @@ static int __init acpi_bus_init_irq(void)
case ACPI_IRQ_MODEL_LPIC:
message = "LPIC";
break;
+ case ACPI_IRQ_MODEL_RINTC:
+ message = "RINTC";
+ break;
default:
pr_info("Unknown interrupt routing model\n");
return -ENODEV;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 846a4001b5e0..c1a01fd02873 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -107,6 +107,7 @@ enum acpi_irq_model_id {
ACPI_IRQ_MODEL_PLATFORM,
ACPI_IRQ_MODEL_GIC,
ACPI_IRQ_MODEL_LPIC,
+ ACPI_IRQ_MODEL_RINTC,
ACPI_IRQ_MODEL_COUNT
};
--
2.40.1
RISC-V platforms need to use dependencies between PCI host bridge, Link
devices and the interrupt controllers to ensure probe order. The
dependency is like below.
Interrupt controller <-- Link Device <-- PCI Host bridge.
If there is no dependency added between Link device and PCI Host Bridge,
then the PCI end points can get probed prior to link device, unable to
get mapping for INTx.
So, add the link device's HID to dependency honor list and also clear it
after its probe.
Since this is required only for architectures like RISC-V, enable this
code under a new config option and set this only in RISC-V.
Signed-off-by: Sunil V L <[email protected]>
---
arch/riscv/Kconfig | 1 +
drivers/acpi/Kconfig | 3 +++
drivers/acpi/pci_link.c | 3 +++
drivers/acpi/scan.c | 1 +
4 files changed, 8 insertions(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index f961449ca077..f7a36d79ff1a 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -14,6 +14,7 @@ config RISCV
def_bool y
select ACPI_GENERIC_GSI if ACPI
select ACPI_REDUCED_HARDWARE_ONLY if ACPI
+ select ARCH_ACPI_DEFERRED_GSI if ACPI
select ARCH_DMA_DEFAULT_COHERENT
select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index e3a7c2aedd5f..ebec1707f662 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -587,6 +587,9 @@ config ACPI_PRMT
substantially increase computational overhead related to the
initialization of some server systems.
+config ARCH_ACPI_DEFERRED_GSI
+ bool
+
endif # ACPI
config X86_PM_TIMER
diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index aa1038b8aec4..48cdcedafad6 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -748,6 +748,9 @@ static int acpi_pci_link_add(struct acpi_device *device,
if (result)
kfree(link);
+ if (IS_ENABLED(CONFIG_ARCH_ACPI_DEFERRED_GSI))
+ acpi_dev_clear_dependencies(device);
+
return result < 0 ? result : 1;
}
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 3eeb4ce39fcc..67677a6ff8e3 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -834,6 +834,7 @@ static const char * const acpi_honor_dep_ids[] = {
"INTC10CF", /* IVSC (MTL) driver must be loaded to allow i2c access to camera sensors */
"RSCV0001", /* RISC-V PLIC */
"RSCV0002", /* RISC-V APLIC */
+ "PNP0C0F", /* PCI Link Device */
NULL
};
--
2.40.1
Replace the dummy implementation for PCI related functions with actual
implementation. This needs ECAM and MCFG CONFIG options to be enabled
for RISC-V.
Signed-off-by: Sunil V L <[email protected]>
---
arch/riscv/Kconfig | 2 ++
arch/riscv/kernel/acpi.c | 31 ++++++++++++++-----------------
drivers/pci/pci-acpi.c | 2 +-
3 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index f7a36d79ff1a..09a86256ddfa 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -13,6 +13,7 @@ config 32BIT
config RISCV
def_bool y
select ACPI_GENERIC_GSI if ACPI
+ select ACPI_MCFG if (ACPI && PCI)
select ACPI_REDUCED_HARDWARE_ONLY if ACPI
select ARCH_ACPI_DEFERRED_GSI if ACPI
select ARCH_DMA_DEFAULT_COHERENT
@@ -176,6 +177,7 @@ config RISCV
select OF_EARLY_FLATTREE
select OF_IRQ
select PCI_DOMAINS_GENERIC if PCI
+ select PCI_ECAM if (ACPI && PCI)
select PCI_MSI if PCI
select RISCV_ALTERNATIVE if !XIP_KERNEL
select RISCV_APLIC
diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c
index e619edc8b0cc..41aa77c8484b 100644
--- a/arch/riscv/kernel/acpi.c
+++ b/arch/riscv/kernel/acpi.c
@@ -306,29 +306,26 @@ void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
#ifdef CONFIG_PCI
/*
- * These interfaces are defined just to enable building ACPI core.
- * TODO: Update it with actual implementation when external interrupt
- * controller support is added in RISC-V ACPI.
+ * raw_pci_read/write - Platform-specific PCI config space access.
*/
-int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int devfn,
- int reg, int len, u32 *val)
+int raw_pci_read(unsigned int domain, unsigned int bus,
+ unsigned int devfn, int reg, int len, u32 *val)
{
- return PCIBIOS_DEVICE_NOT_FOUND;
-}
+ struct pci_bus *b = pci_find_bus(domain, bus);
-int raw_pci_write(unsigned int domain, unsigned int bus, unsigned int devfn,
- int reg, int len, u32 val)
-{
- return PCIBIOS_DEVICE_NOT_FOUND;
+ if (!b)
+ return PCIBIOS_DEVICE_NOT_FOUND;
+ return b->ops->read(b, devfn, reg, len, val);
}
-int acpi_pci_bus_find_domain_nr(struct pci_bus *bus)
+int raw_pci_write(unsigned int domain, unsigned int bus,
+ unsigned int devfn, int reg, int len, u32 val)
{
- return -1;
-}
+ struct pci_bus *b = pci_find_bus(domain, bus);
-struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
-{
- return NULL;
+ if (!b)
+ return PCIBIOS_DEVICE_NOT_FOUND;
+ return b->ops->write(b, devfn, reg, len, val);
}
+
#endif /* CONFIG_PCI */
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index e8d84fa435da..b5892d0fa68c 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -1521,7 +1521,7 @@ static int __init acpi_pci_init(void)
}
arch_initcall(acpi_pci_init);
-#if defined(CONFIG_ARM64)
+#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV)
/*
* Try to assign the IRQ number when probing a new device
--
2.40.1
ACPI MADT entries for interrupt controllers don't have a way to describe
the hierarchy. However, the hierarchy is known to the architecture and
on RISC-V platforms, the MADT sub table types are ordered in the
incremental order from the root controller which is RINTC. So, add
architecture function for RISC-V to reorder the interrupt controller
probing as per the hierarchy as below.
Signed-off-by: Sunil V L <[email protected]>
---
drivers/acpi/riscv/Makefile | 2 +-
drivers/acpi/riscv/irq.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 33 insertions(+), 1 deletion(-)
create mode 100644 drivers/acpi/riscv/irq.c
diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile
index 877de00d1b50..a96fdf1e2cb8 100644
--- a/drivers/acpi/riscv/Makefile
+++ b/drivers/acpi/riscv/Makefile
@@ -1,4 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
-obj-y += rhct.o init.o
+obj-y += rhct.o init.o irq.o
obj-$(CONFIG_ACPI_PROCESSOR_IDLE) += cpuidle.o
obj-$(CONFIG_ACPI_CPPC_LIB) += cppc.o
diff --git a/drivers/acpi/riscv/irq.c b/drivers/acpi/riscv/irq.c
new file mode 100644
index 000000000000..f56e103a501f
--- /dev/null
+++ b/drivers/acpi/riscv/irq.c
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023-2024, Ventana Micro Systems Inc
+ * Author: Sunil V L <[email protected]>
+ *
+ */
+
+#include <linux/acpi.h>
+#include <linux/sort.h>
+
+static int irqchip_cmp_func(const void *in0, const void *in1)
+{
+ struct acpi_probe_entry *elem0 = (struct acpi_probe_entry *)in0;
+ struct acpi_probe_entry *elem1 = (struct acpi_probe_entry *)in1;
+
+ return (elem0->type > elem1->type) - (elem0->type < elem1->type);
+}
+
+/*
+ * RISC-V irqchips in MADT of ACPI spec are defined in the same order how
+ * they should be probed. Since IRQCHIP_ACPI_DECLARE doesn't define any
+ * order, this arch function will reorder the probe functions as per the
+ * required order for the architecture.
+ */
+void arch_sort_irqchip_probe(struct acpi_probe_entry *ap_head, int nr)
+{
+ struct acpi_probe_entry *ape = ap_head;
+
+ if (nr == 1 || !ACPI_COMPARE_NAMESEG(ACPI_SIG_MADT, ape->id))
+ return;
+ sort(ape, nr, sizeof(*ape), irqchip_cmp_func, NULL);
+}
--
2.40.1
RISC-V has PLIC and APLIC in MADT as well as namespace devices.
Initialize the list of those structures using MADT and namespace devices
to create mapping between the ACPI handle and the GSI ranges. This will
be used later to add dependencies.
Signed-off-by: Sunil V L <[email protected]>
---
arch/riscv/include/asm/irq.h | 22 ++++++
drivers/acpi/riscv/init.c | 2 +
drivers/acpi/riscv/init.h | 4 +
drivers/acpi/riscv/irq.c | 142 +++++++++++++++++++++++++++++++++++
4 files changed, 170 insertions(+)
create mode 100644 drivers/acpi/riscv/init.h
diff --git a/arch/riscv/include/asm/irq.h b/arch/riscv/include/asm/irq.h
index 8e10a94430a2..44a0b128c602 100644
--- a/arch/riscv/include/asm/irq.h
+++ b/arch/riscv/include/asm/irq.h
@@ -16,4 +16,26 @@ void riscv_set_intc_hwnode_fn(struct fwnode_handle *(*fn)(void));
struct fwnode_handle *riscv_get_intc_hwnode(void);
+#ifdef CONFIG_ACPI
+
+enum riscv_irqchip_type {
+ ACPI_RISCV_IRQCHIP_INTC = 0x00,
+ ACPI_RISCV_IRQCHIP_IMSIC = 0x01,
+ ACPI_RISCV_IRQCHIP_PLIC = 0x02,
+ ACPI_RISCV_IRQCHIP_APLIC = 0x03,
+};
+
+int riscv_acpi_get_gsi_info(struct fwnode_handle *fwnode, u32 *gsi_base,
+ u32 *id, u32 *nr_irqs, u32 *nr_idcs);
+struct fwnode_handle *riscv_acpi_get_gsi_domain_id(u32 gsi);
+
+#else
+static inline int riscv_acpi_get_gsi_info(struct fwnode_handle *fwnode, u32 *gsi_base,
+ u32 *id, u32 *nr_irqs, u32 *nr_idcs)
+{
+ return 0;
+}
+
+#endif /* CONFIG_ACPI */
+
#endif /* _ASM_RISCV_IRQ_H */
diff --git a/drivers/acpi/riscv/init.c b/drivers/acpi/riscv/init.c
index 5f7571143245..22db97f7a772 100644
--- a/drivers/acpi/riscv/init.c
+++ b/drivers/acpi/riscv/init.c
@@ -6,7 +6,9 @@
*/
#include <linux/acpi.h>
+#include "init.h"
void __init acpi_riscv_init(void)
{
+ riscv_acpi_init_gsi_mapping();
}
diff --git a/drivers/acpi/riscv/init.h b/drivers/acpi/riscv/init.h
new file mode 100644
index 000000000000..0b9a07e4031f
--- /dev/null
+++ b/drivers/acpi/riscv/init.h
@@ -0,0 +1,4 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include <linux/init.h>
+
+void __init riscv_acpi_init_gsi_mapping(void);
diff --git a/drivers/acpi/riscv/irq.c b/drivers/acpi/riscv/irq.c
index f56e103a501f..0473428e8d1e 100644
--- a/drivers/acpi/riscv/irq.c
+++ b/drivers/acpi/riscv/irq.c
@@ -7,6 +7,21 @@
#include <linux/acpi.h>
#include <linux/sort.h>
+#include <linux/irq.h>
+
+#include "init.h"
+
+struct riscv_ext_intc_list {
+ acpi_handle handle;
+ u32 gsi_base;
+ u32 nr_irqs;
+ u32 nr_idcs;
+ u32 id;
+ u32 type;
+ struct list_head list;
+};
+
+LIST_HEAD(ext_intc_list);
static int irqchip_cmp_func(const void *in0, const void *in1)
{
@@ -30,3 +45,130 @@ void arch_sort_irqchip_probe(struct acpi_probe_entry *ap_head, int nr)
return;
sort(ape, nr, sizeof(*ape), irqchip_cmp_func, NULL);
}
+
+static void riscv_acpi_update_gsi_handle(u32 gsi_base, acpi_handle handle)
+{
+ struct riscv_ext_intc_list *ext_intc_element;
+ struct list_head *i, *tmp;
+
+ list_for_each_safe(i, tmp, &ext_intc_list) {
+ ext_intc_element = list_entry(i, struct riscv_ext_intc_list, list);
+ if (gsi_base == ext_intc_element->gsi_base) {
+ ext_intc_element->handle = handle;
+ return;
+ }
+ }
+
+ acpi_handle_err(handle, "failed to find the GSI mapping entry\n");
+}
+
+int riscv_acpi_get_gsi_info(struct fwnode_handle *fwnode, u32 *gsi_base,
+ u32 *id, u32 *nr_irqs, u32 *nr_idcs)
+{
+ struct riscv_ext_intc_list *ext_intc_element;
+ struct list_head *i, *tmp;
+
+ list_for_each_safe(i, tmp, &ext_intc_list) {
+ ext_intc_element = list_entry(i, struct riscv_ext_intc_list, list);
+ if (ext_intc_element->handle == ACPI_HANDLE_FWNODE(fwnode)) {
+ *gsi_base = ext_intc_element->gsi_base;
+ *id = ext_intc_element->id;
+ *nr_irqs = ext_intc_element->nr_irqs;
+ if (nr_idcs)
+ *nr_idcs = ext_intc_element->nr_idcs;
+
+ return 0;
+ }
+ }
+
+ return -ENODEV;
+}
+
+struct fwnode_handle *riscv_acpi_get_gsi_domain_id(u32 gsi)
+{
+ struct riscv_ext_intc_list *ext_intc_element;
+ struct acpi_device *adev;
+ struct list_head *i, *tmp;
+
+ list_for_each_safe(i, tmp, &ext_intc_list) {
+ ext_intc_element = list_entry(i, struct riscv_ext_intc_list, list);
+ if (gsi >= ext_intc_element->gsi_base &&
+ gsi < (ext_intc_element->gsi_base + ext_intc_element->nr_irqs)) {
+ adev = acpi_fetch_acpi_dev(ext_intc_element->handle);
+ if (!adev)
+ return NULL;
+
+ return acpi_fwnode_handle(adev);
+ }
+ }
+
+ return NULL;
+}
+
+static int __init riscv_acpi_register_ext_intc(u32 gsi_base, u32 nr_irqs, u32 nr_idcs,
+ u32 id, u32 type)
+{
+ struct riscv_ext_intc_list *ext_intc_element;
+
+ ext_intc_element = kzalloc(sizeof(*ext_intc_element), GFP_KERNEL);
+ if (!ext_intc_element)
+ return -ENOMEM;
+
+ ext_intc_element->gsi_base = gsi_base;
+ ext_intc_element->nr_irqs = nr_irqs;
+ ext_intc_element->nr_idcs = nr_idcs;
+ ext_intc_element->id = id;
+ list_add_tail(&ext_intc_element->list, &ext_intc_list);
+ return 0;
+}
+
+static acpi_status __init riscv_acpi_create_gsi_map(acpi_handle handle, u32 level,
+ void *context, void **return_value)
+{
+ acpi_status status;
+ u64 gbase;
+
+ if (!acpi_has_method(handle, "_GSB")) {
+ acpi_handle_err(handle, "_GSB method not found\n");
+ return AE_OK;
+ }
+
+ status = acpi_evaluate_integer(handle, "_GSB", NULL, &gbase);
+ if (ACPI_FAILURE(status)) {
+ acpi_handle_err(handle, "failed to evaluate _GSB method\n");
+ return AE_OK;
+ }
+
+ riscv_acpi_update_gsi_handle((u32)gbase, handle);
+ return AE_OK;
+}
+
+static int __init riscv_acpi_aplic_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_aplic *aplic = (struct acpi_madt_aplic *)header;
+
+ return riscv_acpi_register_ext_intc(aplic->gsi_base, aplic->num_sources, aplic->num_idcs,
+ aplic->id, ACPI_RISCV_IRQCHIP_APLIC);
+}
+
+static int __init riscv_acpi_plic_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_plic *plic = (struct acpi_madt_plic *)header;
+
+ return riscv_acpi_register_ext_intc(plic->gsi_base, plic->num_irqs, 0,
+ plic->id, ACPI_RISCV_IRQCHIP_PLIC);
+}
+
+void __init riscv_acpi_init_gsi_mapping(void)
+{
+ /* There can be either PLIC or APLIC */
+ if (acpi_table_parse_madt(ACPI_MADT_TYPE_PLIC, riscv_acpi_plic_parse_madt, 0) > 0) {
+ acpi_get_devices("RSCV0001", riscv_acpi_create_gsi_map, NULL, NULL);
+ return;
+ }
+
+ if (acpi_table_parse_madt(ACPI_MADT_TYPE_APLIC, riscv_acpi_aplic_parse_madt, 0) > 0)
+ acpi_get_devices("RSCV0002", riscv_acpi_create_gsi_map, NULL, NULL);
+}
--
2.40.1
The RINTC subtype structure in MADT also has information about other
interrupt controllers. Save this information and provide interfaces to
retrieve them when required by corresponding drivers.
Signed-off-by: Sunil V L <[email protected]>
---
arch/riscv/include/asm/irq.h | 35 +++++++++++
drivers/irqchip/irq-riscv-intc.c | 102 ++++++++++++++++++++++++++++++-
2 files changed, 135 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/include/asm/irq.h b/arch/riscv/include/asm/irq.h
index 44a0b128c602..6bd578b1ffc9 100644
--- a/arch/riscv/include/asm/irq.h
+++ b/arch/riscv/include/asm/irq.h
@@ -25,9 +25,22 @@ enum riscv_irqchip_type {
ACPI_RISCV_IRQCHIP_APLIC = 0x03,
};
+/*
+ * The ext_intc_id format is as follows:
+ * Bits [31:24] APLIC/PLIC ID
+ * Bits [15:0] APLIC IDC ID / PLIC S-Mode Context ID for this hart
+ */
+#define APLIC_PLIC_ID(x) ((x) >> 24)
+#define IDC_CONTEXT_ID(x) ((x) & 0x0000ffff)
+
int riscv_acpi_get_gsi_info(struct fwnode_handle *fwnode, u32 *gsi_base,
u32 *id, u32 *nr_irqs, u32 *nr_idcs);
struct fwnode_handle *riscv_acpi_get_gsi_domain_id(u32 gsi);
+int __init acpi_get_intc_index_hartid(u32 index, unsigned long *hartid);
+int acpi_get_ext_intc_parent_hartid(u8 id, u32 idx, unsigned long *hartid);
+void acpi_get_plic_nr_contexts(u8 id, int *nr_contexts);
+int acpi_get_plic_context(u8 id, u32 idx, int *context_id);
+int __init acpi_get_imsic_mmio_info(u32 index, struct resource *res);
#else
static inline int riscv_acpi_get_gsi_info(struct fwnode_handle *fwnode, u32 *gsi_base,
@@ -36,6 +49,28 @@ static inline int riscv_acpi_get_gsi_info(struct fwnode_handle *fwnode, u32 *gsi
return 0;
}
+static inline int __init acpi_get_intc_index_hartid(u32 index, unsigned long *hartid)
+{
+ return -EINVAL;
+}
+
+static inline int acpi_get_ext_intc_parent_hartid(u8 id, u32 idx, unsigned long *hartid)
+{
+ return -EINVAL;
+}
+
+static inline void acpi_get_plic_nr_contexts(u8 id, int *nr_contexts) { }
+
+static inline int acpi_get_plic_context(u8 id, u32 idx, int *context_id)
+{
+ return -EINVAL;
+}
+
+static inline int __init acpi_get_imsic_mmio_info(u32 index, struct resource *res)
+{
+ return 0;
+}
+
#endif /* CONFIG_ACPI */
#endif /* _ASM_RISCV_IRQ_H */
diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
index 9e71c4428814..af7a2f78f0ee 100644
--- a/drivers/irqchip/irq-riscv-intc.c
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -249,14 +249,105 @@ IRQCHIP_DECLARE(riscv, "riscv,cpu-intc", riscv_intc_init);
IRQCHIP_DECLARE(andes, "andestech,cpu-intc", riscv_intc_init);
#ifdef CONFIG_ACPI
+struct rintc_data {
+ u32 ext_intc_id;
+ unsigned long hart_id;
+ u64 imsic_addr;
+ u32 imsic_size;
+};
+
+static u32 nr_rintc;
+static struct rintc_data *rintc_acpi_data[NR_CPUS];
+
+int acpi_get_intc_index_hartid(u32 index, unsigned long *hartid)
+{
+ if (index >= nr_rintc)
+ return -1;
+
+ *hartid = rintc_acpi_data[index]->hart_id;
+ return 0;
+}
+
+int acpi_get_ext_intc_parent_hartid(u8 id, u32 idx, unsigned long *hartid)
+{
+ int i, j = 0;
+
+ for (i = 0; i < nr_rintc; i++) {
+ if (APLIC_PLIC_ID(rintc_acpi_data[i]->ext_intc_id) == id) {
+ if (idx == j) {
+ *hartid = rintc_acpi_data[i]->hart_id;
+ return 0;
+ }
+ j++;
+ }
+ }
+
+ return -1;
+}
+
+void acpi_get_plic_nr_contexts(u8 id, int *nr_contexts)
+{
+ int i, j = 0;
+
+ for (i = 0; i < nr_rintc; i++) {
+ if (APLIC_PLIC_ID(rintc_acpi_data[i]->ext_intc_id) == id)
+ j++;
+ }
+
+ *nr_contexts = j;
+}
+
+int acpi_get_plic_context(u8 id, u32 idx, int *context_id)
+{
+ int i, j = 0;
+
+ for (i = 0; i < nr_rintc; i++) {
+ if (APLIC_PLIC_ID(rintc_acpi_data[i]->ext_intc_id) == id) {
+ if (idx == j) {
+ *context_id = IDC_CONTEXT_ID(rintc_acpi_data[i]->ext_intc_id);
+ return 0;
+ }
+
+ j++;
+ }
+ }
+
+ return -1;
+}
+
+int acpi_get_imsic_mmio_info(u32 index, struct resource *res)
+{
+ if (index >= nr_rintc)
+ return -1;
+
+ res->start = rintc_acpi_data[index]->imsic_addr;
+ res->end = res->start + rintc_acpi_data[index]->imsic_size - 1;
+ res->flags = IORESOURCE_MEM;
+ return 0;
+}
+
+static struct fwnode_handle *ext_entc_get_gsi_domain_id(u32 gsi)
+{
+ return riscv_acpi_get_gsi_domain_id(gsi);
+}
static int __init riscv_intc_acpi_init(union acpi_subtable_headers *header,
const unsigned long end)
{
- struct fwnode_handle *fn;
struct acpi_madt_rintc *rintc;
+ struct fwnode_handle *fn;
+ int rc;
rintc = (struct acpi_madt_rintc *)header;
+ rintc_acpi_data[nr_rintc] = kzalloc(sizeof(*rintc_acpi_data[0]), GFP_KERNEL);
+ if (!rintc_acpi_data[nr_rintc])
+ return -ENOMEM;
+
+ rintc_acpi_data[nr_rintc]->ext_intc_id = rintc->ext_intc_id;
+ rintc_acpi_data[nr_rintc]->hart_id = rintc->hart_id;
+ rintc_acpi_data[nr_rintc]->imsic_addr = rintc->imsic_addr;
+ rintc_acpi_data[nr_rintc]->imsic_size = rintc->imsic_size;
+ nr_rintc++;
/*
* The ACPI MADT will have one INTC for each CPU (or HART)
@@ -273,7 +364,14 @@ static int __init riscv_intc_acpi_init(union acpi_subtable_headers *header,
return -ENOMEM;
}
- return riscv_intc_init_common(fn, &riscv_intc_chip);
+ rc = riscv_intc_init_common(fn, &riscv_intc_chip);
+ if (rc) {
+ irq_domain_free_fwnode(fn);
+ return rc;
+ }
+
+ acpi_set_irq_model(ACPI_IRQ_MODEL_RINTC, ext_entc_get_gsi_domain_id);
+ return 0;
}
IRQCHIP_ACPI_DECLARE(riscv_intc, ACPI_MADT_TYPE_RINTC, NULL,
--
2.40.1
RISC-V IMSIC interrupt controller provides IPI and MSI support.
Currently, DT based drivers setup the IPI feature early during boot but
defer setting up the MSI functionality. However, in ACPI systems, ACPI,
both IPI and MSI features need to be initialized early itself.
Signed-off-by: Sunil V L <[email protected]>
---
drivers/irqchip/irq-riscv-imsic-early.c | 52 +++++++++-
drivers/irqchip/irq-riscv-imsic-platform.c | 32 ++++--
drivers/irqchip/irq-riscv-imsic-state.c | 115 ++++++++++-----------
drivers/irqchip/irq-riscv-imsic-state.h | 2 +-
include/linux/irqchip/riscv-imsic.h | 10 ++
5 files changed, 144 insertions(+), 67 deletions(-)
diff --git a/drivers/irqchip/irq-riscv-imsic-early.c b/drivers/irqchip/irq-riscv-imsic-early.c
index 886418ec06cb..d8161243791d 100644
--- a/drivers/irqchip/irq-riscv-imsic-early.c
+++ b/drivers/irqchip/irq-riscv-imsic-early.c
@@ -5,13 +5,16 @@
*/
#define pr_fmt(fmt) "riscv-imsic: " fmt
+#include <linux/acpi.h>
#include <linux/cpu.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/irq.h>
#include <linux/irqchip.h>
#include <linux/irqchip/chained_irq.h>
+#include <linux/irqchip/riscv-imsic.h>
#include <linux/module.h>
+#include <linux/pci.h>
#include <linux/spinlock.h>
#include <linux/smp.h>
@@ -182,7 +185,7 @@ static int __init imsic_early_dt_init(struct device_node *node, struct device_no
int rc;
/* Setup IMSIC state */
- rc = imsic_setup_state(fwnode);
+ rc = imsic_setup_state(fwnode, NULL);
if (rc) {
pr_err("%pfwP: failed to setup state (error %d)\n", fwnode, rc);
return rc;
@@ -199,3 +202,50 @@ static int __init imsic_early_dt_init(struct device_node *node, struct device_no
}
IRQCHIP_DECLARE(riscv_imsic, "riscv,imsics", imsic_early_dt_init);
+
+#ifdef CONFIG_ACPI
+
+static struct fwnode_handle *imsic_acpi_fwnode;
+
+struct fwnode_handle *imsic_acpi_get_fwnode(struct device *dev)
+{
+ return imsic_acpi_fwnode;
+}
+
+static int __init imsic_early_acpi_init(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_imsic *imsic = (struct acpi_madt_imsic *)header;
+ int rc;
+
+ imsic_acpi_fwnode = irq_domain_alloc_named_fwnode("imsic");
+ if (!imsic_acpi_fwnode) {
+ pr_err("unable to allocate IMSIC FW node\n");
+ return -ENOMEM;
+ }
+
+ /* Setup IMSIC state */
+ rc = imsic_setup_state(imsic_acpi_fwnode, (void *)imsic);
+ if (rc) {
+ pr_err("%pfwP: failed to setup state (error %d)\n", imsic_acpi_fwnode, rc);
+ return rc;
+ }
+
+ /* Do early setup of IMSIC state and IPIs */
+ rc = imsic_early_probe(imsic_acpi_fwnode);
+ if (rc)
+ return rc;
+
+ rc = imsic_platform_acpi_probe(imsic_acpi_fwnode);
+
+#ifdef CONFIG_PCI
+ if (!rc)
+ pci_msi_register_fwnode_provider(&imsic_acpi_get_fwnode);
+#endif
+
+ return rc;
+}
+
+IRQCHIP_ACPI_DECLARE(riscv_imsic, ACPI_MADT_TYPE_IMSIC, NULL,
+ 1, imsic_early_acpi_init);
+#endif
diff --git a/drivers/irqchip/irq-riscv-imsic-platform.c b/drivers/irqchip/irq-riscv-imsic-platform.c
index 11723a763c10..64905e6f52d7 100644
--- a/drivers/irqchip/irq-riscv-imsic-platform.c
+++ b/drivers/irqchip/irq-riscv-imsic-platform.c
@@ -5,6 +5,7 @@
*/
#define pr_fmt(fmt) "riscv-imsic: " fmt
+#include <linux/acpi.h>
#include <linux/bitmap.h>
#include <linux/cpu.h>
#include <linux/interrupt.h>
@@ -348,18 +349,37 @@ int imsic_irqdomain_init(void)
return 0;
}
-static int imsic_platform_probe(struct platform_device *pdev)
+static int imsic_platform_probe_common(struct fwnode_handle *fwnode)
{
- struct device *dev = &pdev->dev;
-
- if (imsic && imsic->fwnode != dev->fwnode) {
- dev_err(dev, "fwnode mismatch\n");
+ if (imsic && imsic->fwnode != fwnode) {
+ pr_err("%pfwP: fwnode mismatch\n", fwnode);
return -ENODEV;
}
return imsic_irqdomain_init();
}
+static int imsic_platform_dt_probe(struct platform_device *pdev)
+{
+ return imsic_platform_probe_common(pdev->dev.fwnode);
+}
+
+#ifdef CONFIG_ACPI
+
+/*
+ * On ACPI based systems, PCI enumeration happens early during boot in
+ * acpi_scan_init(). PCI enumeration expects MSI domain setup before
+ * it calls pci_set_msi_domain(). Hence, unlike in DT where
+ * imsic-platform drive probe happens late during boot, ACPI based
+ * systems need to setup the MSI domain early.
+ */
+int imsic_platform_acpi_probe(struct fwnode_handle *fwnode)
+{
+ return imsic_platform_probe_common(fwnode);
+}
+
+#endif
+
static const struct of_device_id imsic_platform_match[] = {
{ .compatible = "riscv,imsics" },
{}
@@ -370,6 +390,6 @@ static struct platform_driver imsic_platform_driver = {
.name = "riscv-imsic",
.of_match_table = imsic_platform_match,
},
- .probe = imsic_platform_probe,
+ .probe = imsic_platform_dt_probe,
};
builtin_platform_driver(imsic_platform_driver);
diff --git a/drivers/irqchip/irq-riscv-imsic-state.c b/drivers/irqchip/irq-riscv-imsic-state.c
index 5479f872e62b..608b87dd0784 100644
--- a/drivers/irqchip/irq-riscv-imsic-state.c
+++ b/drivers/irqchip/irq-riscv-imsic-state.c
@@ -5,6 +5,7 @@
*/
#define pr_fmt(fmt) "riscv-imsic: " fmt
+#include <linux/acpi.h>
#include <linux/cpu.h>
#include <linux/bitmap.h>
#include <linux/interrupt.h>
@@ -516,12 +517,8 @@ static int __init imsic_get_parent_hartid(struct fwnode_handle *fwnode,
struct of_phandle_args parent;
int rc;
- /*
- * Currently, only OF fwnode is supported so extend this
- * function for ACPI support.
- */
if (!is_of_node(fwnode))
- return -EINVAL;
+ return acpi_get_intc_index_hartid(index, hartid);
rc = of_irq_parse_one(to_of_node(fwnode), index, &parent);
if (rc)
@@ -540,12 +537,8 @@ static int __init imsic_get_parent_hartid(struct fwnode_handle *fwnode,
static int __init imsic_get_mmio_resource(struct fwnode_handle *fwnode,
u32 index, struct resource *res)
{
- /*
- * Currently, only OF fwnode is supported so extend this
- * function for ACPI support.
- */
if (!is_of_node(fwnode))
- return -EINVAL;
+ return acpi_get_imsic_mmio_info(index, res);
return of_address_to_resource(to_of_node(fwnode), index, res);
}
@@ -553,20 +546,15 @@ static int __init imsic_get_mmio_resource(struct fwnode_handle *fwnode,
static int __init imsic_parse_fwnode(struct fwnode_handle *fwnode,
struct imsic_global_config *global,
u32 *nr_parent_irqs,
- u32 *nr_mmios)
+ u32 *nr_mmios,
+ void *opaque)
{
+ struct acpi_madt_imsic *imsic = (struct acpi_madt_imsic *)opaque;
unsigned long hartid;
struct resource res;
int rc;
u32 i;
- /*
- * Currently, only OF fwnode is supported so extend this
- * function for ACPI support.
- */
- if (!is_of_node(fwnode))
- return -EINVAL;
-
*nr_parent_irqs = 0;
*nr_mmios = 0;
@@ -578,51 +566,60 @@ static int __init imsic_parse_fwnode(struct fwnode_handle *fwnode,
return -EINVAL;
}
- /* Find number of guest index bits in MSI address */
- rc = of_property_read_u32(to_of_node(fwnode), "riscv,guest-index-bits",
- &global->guest_index_bits);
- if (rc)
- global->guest_index_bits = 0;
+ if (is_of_node(fwnode)) {
+ /* Find number of guest index bits in MSI address */
+ rc = of_property_read_u32(to_of_node(fwnode), "riscv,guest-index-bits",
+ &global->guest_index_bits);
+ if (rc)
+ global->guest_index_bits = 0;
- /* Find number of HART index bits */
- rc = of_property_read_u32(to_of_node(fwnode), "riscv,hart-index-bits",
- &global->hart_index_bits);
- if (rc) {
- /* Assume default value */
- global->hart_index_bits = __fls(*nr_parent_irqs);
- if (BIT(global->hart_index_bits) < *nr_parent_irqs)
- global->hart_index_bits++;
- }
+ /* Find number of HART index bits */
+ rc = of_property_read_u32(to_of_node(fwnode), "riscv,hart-index-bits",
+ &global->hart_index_bits);
+ if (rc) {
+ /* Assume default value */
+ global->hart_index_bits = __fls(*nr_parent_irqs);
+ if (BIT(global->hart_index_bits) < *nr_parent_irqs)
+ global->hart_index_bits++;
+ }
- /* Find number of group index bits */
- rc = of_property_read_u32(to_of_node(fwnode), "riscv,group-index-bits",
- &global->group_index_bits);
- if (rc)
- global->group_index_bits = 0;
+ /* Find number of group index bits */
+ rc = of_property_read_u32(to_of_node(fwnode), "riscv,group-index-bits",
+ &global->group_index_bits);
+ if (rc)
+ global->group_index_bits = 0;
- /*
- * Find first bit position of group index.
- * If not specified assumed the default APLIC-IMSIC configuration.
- */
- rc = of_property_read_u32(to_of_node(fwnode), "riscv,group-index-shift",
- &global->group_index_shift);
- if (rc)
- global->group_index_shift = IMSIC_MMIO_PAGE_SHIFT * 2;
+ /*
+ * Find first bit position of group index.
+ * If not specified assumed the default APLIC-IMSIC configuration.
+ */
+ rc = of_property_read_u32(to_of_node(fwnode), "riscv,group-index-shift",
+ &global->group_index_shift);
+ if (rc)
+ global->group_index_shift = IMSIC_MMIO_PAGE_SHIFT * 2;
+
+ /* Find number of interrupt identities */
+ rc = of_property_read_u32(to_of_node(fwnode), "riscv,num-ids",
+ &global->nr_ids);
+ if (rc) {
+ pr_err("%pfwP: number of interrupt identities not found\n", fwnode);
+ return rc;
+ }
- /* Find number of interrupt identities */
- rc = of_property_read_u32(to_of_node(fwnode), "riscv,num-ids",
- &global->nr_ids);
- if (rc) {
- pr_err("%pfwP: number of interrupt identities not found\n", fwnode);
- return rc;
+ /* Find number of guest interrupt identities */
+ rc = of_property_read_u32(to_of_node(fwnode), "riscv,num-guest-ids",
+ &global->nr_guest_ids);
+ if (rc)
+ global->nr_guest_ids = global->nr_ids;
+ } else {
+ global->guest_index_bits = imsic->guest_index_bits;
+ global->hart_index_bits = imsic->hart_index_bits;
+ global->group_index_bits = imsic->group_index_bits;
+ global->group_index_shift = imsic->group_index_shift;
+ global->nr_ids = imsic->num_ids;
+ global->nr_guest_ids = imsic->num_guest_ids;
}
- /* Find number of guest interrupt identities */
- rc = of_property_read_u32(to_of_node(fwnode), "riscv,num-guest-ids",
- &global->nr_guest_ids);
- if (rc)
- global->nr_guest_ids = global->nr_ids;
-
/* Sanity check guest index bits */
i = BITS_PER_LONG - IMSIC_MMIO_PAGE_SHIFT;
if (i < global->guest_index_bits) {
@@ -688,7 +685,7 @@ static int __init imsic_parse_fwnode(struct fwnode_handle *fwnode,
return 0;
}
-int __init imsic_setup_state(struct fwnode_handle *fwnode)
+int __init imsic_setup_state(struct fwnode_handle *fwnode, void *opaque)
{
u32 i, j, index, nr_parent_irqs, nr_mmios, nr_handlers = 0;
struct imsic_global_config *global;
@@ -729,7 +726,7 @@ int __init imsic_setup_state(struct fwnode_handle *fwnode)
}
/* Parse IMSIC fwnode */
- rc = imsic_parse_fwnode(fwnode, global, &nr_parent_irqs, &nr_mmios);
+ rc = imsic_parse_fwnode(fwnode, global, &nr_parent_irqs, &nr_mmios, opaque);
if (rc)
goto out_free_local;
diff --git a/drivers/irqchip/irq-riscv-imsic-state.h b/drivers/irqchip/irq-riscv-imsic-state.h
index 5ae2f69b035b..391e44280827 100644
--- a/drivers/irqchip/irq-riscv-imsic-state.h
+++ b/drivers/irqchip/irq-riscv-imsic-state.h
@@ -102,7 +102,7 @@ void imsic_vector_debug_show_summary(struct seq_file *m, int ind);
void imsic_state_online(void);
void imsic_state_offline(void);
-int imsic_setup_state(struct fwnode_handle *fwnode);
+int imsic_setup_state(struct fwnode_handle *fwnode, void *opaque);
int imsic_irqdomain_init(void);
#endif
diff --git a/include/linux/irqchip/riscv-imsic.h b/include/linux/irqchip/riscv-imsic.h
index faf0b800b1b0..e08680b1932b 100644
--- a/include/linux/irqchip/riscv-imsic.h
+++ b/include/linux/irqchip/riscv-imsic.h
@@ -84,4 +84,14 @@ static inline const struct imsic_global_config *imsic_get_global_config(void)
#endif
+#ifdef CONFIG_ACPI
+int imsic_platform_acpi_probe(struct fwnode_handle *fwnode);
+struct fwnode_handle *imsic_acpi_get_fwnode(struct device *dev);
+#else
+static inline struct fwnode_handle *imsic_acpi_get_fwnode(struct device *dev)
+{
+ return NULL;
+}
+#endif
+
#endif
--
2.40.1
RISC-V has non-PNP generic 16550A compatible UART which needs to be
enumerated as ACPI platform device. Add driver support for such devices
similar to 8250_of.
The driver is enabled when the CONFIG_SERIAL_ACPI_PLATFORM option is
enabled. Enable this option for RISC-V.
Signed-off-by: Sunil V L <[email protected]>
---
arch/riscv/configs/defconfig | 1 +
drivers/tty/serial/8250/8250_acpi.c | 96 +++++++++++++++++++++++++++++
drivers/tty/serial/8250/Kconfig | 8 +++
drivers/tty/serial/8250/Makefile | 1 +
4 files changed, 106 insertions(+)
create mode 100644 drivers/tty/serial/8250/8250_acpi.c
diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index 3cae018f9315..bea8241f52eb 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -150,6 +150,7 @@ CONFIG_SERIAL_8250=y
CONFIG_SERIAL_8250_CONSOLE=y
CONFIG_SERIAL_8250_DW=y
CONFIG_SERIAL_OF_PLATFORM=y
+CONFIG_SERIAL_ACPI_PLATFORM=y
CONFIG_SERIAL_SH_SCI=y
CONFIG_SERIAL_EARLYCON_RISCV_SBI=y
CONFIG_VIRTIO_CONSOLE=y
diff --git a/drivers/tty/serial/8250/8250_acpi.c b/drivers/tty/serial/8250/8250_acpi.c
new file mode 100644
index 000000000000..3682443bb69c
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_acpi.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Serial Port driver for ACPI platform devices
+ *
+ * This driver is for generic 16550 compatible UART enumerated via ACPI
+ * platform bus instead of PNP bus like PNP0501. This is not a full
+ * driver but mostly provides the ACPI wrapper and uses generic
+ * 8250 framework for rest of the functionality.
+ */
+
+#include <linux/acpi.h>
+#include <linux/serial_reg.h>
+#include <linux/serial_8250.h>
+
+#include "8250.h"
+
+struct acpi_serial_info {
+ int line;
+};
+
+static int acpi_platform_serial_probe(struct platform_device *pdev)
+{
+ struct acpi_serial_info *data;
+ struct uart_8250_port port8250;
+ struct device *dev = &pdev->dev;
+ struct resource *regs;
+
+ int ret, irq;
+
+ regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!regs) {
+ dev_err(dev, "no registers defined\n");
+ return -EINVAL;
+ }
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ memset(&port8250, 0, sizeof(port8250));
+
+ spin_lock_init(&port8250.port.lock);
+
+ port8250.port.mapbase = regs->start;
+ port8250.port.irq = irq;
+ port8250.port.type = PORT_16550A;
+ port8250.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_FIXED_PORT |
+ UPF_IOREMAP | UPF_FIXED_TYPE;
+ port8250.port.dev = dev;
+ port8250.port.mapsize = resource_size(regs);
+ port8250.port.iotype = UPIO_MEM;
+ port8250.port.irqflags = IRQF_SHARED;
+
+ port8250.port.membase = devm_ioremap(dev, port8250.port.mapbase, port8250.port.mapsize);
+ if (!port8250.port.membase)
+ return -ENOMEM;
+
+ ret = uart_read_and_validate_port_properties(&port8250.port);
+ if (ret)
+ return -EINVAL;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->line = serial8250_register_8250_port(&port8250);
+ if (data->line < 0)
+ return data->line;
+
+ platform_set_drvdata(pdev, data);
+ return 0;
+}
+
+static void acpi_platform_serial_remove(struct platform_device *pdev)
+{
+ struct acpi_serial_info *data = platform_get_drvdata(pdev);
+
+ serial8250_unregister_port(data->line);
+}
+
+static const struct acpi_device_id acpi_platform_serial_table[] = {
+ { "RSCV0003", 0 },
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, acpi_platform_serial_table);
+
+static struct platform_driver acpi_platform_serial_driver = {
+ .driver = {
+ .name = "acpi_serial",
+ .acpi_match_table = ACPI_PTR(acpi_platform_serial_table),
+ },
+ .probe = acpi_platform_serial_probe,
+ .remove_new = acpi_platform_serial_remove,
+};
+
+builtin_platform_driver(acpi_platform_serial_driver);
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 47ff50763c04..fbfe4d3501b1 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -576,3 +576,11 @@ config SERIAL_OF_PLATFORM
are probed through devicetree, including Open Firmware based
PowerPC systems and embedded systems on architectures using the
flattened device tree format.
+
+config SERIAL_ACPI_PLATFORM
+ tristate "ACPI platform bus based probing for 8250 ports"
+ depends on SERIAL_8250 && ACPI
+ default n
+ help
+ This option is used for generic 8250 compatible serial ports
+ that are enumerated through ACPI platform bus.
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index ea2e81f58eac..8c0ef357fc4e 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_SERIAL_8250_CONSOLE) += 8250_early.o
obj-$(CONFIG_SERIAL_8250_ACCENT) += 8250_accent.o
obj-$(CONFIG_SERIAL_8250_ACORN) += 8250_acorn.o
+obj-$(CONFIG_SERIAL_ACPI_PLATFORM) += 8250_acpi.o
obj-$(CONFIG_SERIAL_8250_ASPEED_VUART) += 8250_aspeed_vuart.o
obj-$(CONFIG_SERIAL_8250_BCM2835AUX) += 8250_bcm2835aux.o
obj-$(CONFIG_SERIAL_8250_BCM7271) += 8250_bcm7271.o
--
2.40.1
RISC-V interrupt controllers for wired interrupts are platform devices
and hence their driver will be probed late. Also, APLIC which is one
such interrupt controller can not be probed early since it needs MSI
services. This needs a probing order between the interrupt controller
driver and the device drivers.
_DEP is typically used to indicate such dependencies. However, the
dependency may be already available like GSI mapping. Hence, instead of
an explicit _DEP, architecture can find the implicit dependencies and
add to the dependency list.
For RISC-V, add the dependencies for below use cases.
1) For devices which has IRQ resource, find out the interrupt controller
using GSI number map and add the dependency.
2) For PCI host bridges:
a) If _PRT indicate PCI link devices, add dependency on the link
device.
b) If _PRT indicates GSI, find out the interrupt controller
using GSI number map and add the dependency.
Signed-off-by: Sunil V L <[email protected]>
---
drivers/acpi/riscv/irq.c | 155 +++++++++++++++++++++++++++++++++++++++
1 file changed, 155 insertions(+)
diff --git a/drivers/acpi/riscv/irq.c b/drivers/acpi/riscv/irq.c
index 0473428e8d1e..2878ae48131f 100644
--- a/drivers/acpi/riscv/irq.c
+++ b/drivers/acpi/riscv/irq.c
@@ -21,6 +21,12 @@ struct riscv_ext_intc_list {
struct list_head list;
};
+struct acpi_irq_dep_ctx {
+ int rc;
+ unsigned int index;
+ acpi_handle handle;
+};
+
LIST_HEAD(ext_intc_list);
static int irqchip_cmp_func(const void *in0, const void *in1)
@@ -62,6 +68,21 @@ static void riscv_acpi_update_gsi_handle(u32 gsi_base, acpi_handle handle)
acpi_handle_err(handle, "failed to find the GSI mapping entry\n");
}
+static acpi_handle riscv_acpi_get_gsi_handle(u32 gsi)
+{
+ struct riscv_ext_intc_list *ext_intc_element;
+ struct list_head *i, *tmp;
+
+ list_for_each_safe(i, tmp, &ext_intc_list) {
+ ext_intc_element = list_entry(i, struct riscv_ext_intc_list, list);
+ if (gsi >= ext_intc_element->gsi_base &&
+ gsi < (ext_intc_element->gsi_base + ext_intc_element->nr_irqs))
+ return ext_intc_element->handle;
+ }
+
+ return NULL;
+}
+
int riscv_acpi_get_gsi_info(struct fwnode_handle *fwnode, u32 *gsi_base,
u32 *id, u32 *nr_irqs, u32 *nr_idcs)
{
@@ -172,3 +193,137 @@ void __init riscv_acpi_init_gsi_mapping(void)
if (acpi_table_parse_madt(ACPI_MADT_TYPE_APLIC, riscv_acpi_aplic_parse_madt, 0) > 0)
acpi_get_devices("RSCV0002", riscv_acpi_create_gsi_map, NULL, NULL);
}
+
+static acpi_status riscv_acpi_irq_get_parent(struct acpi_resource *ares, void *context)
+{
+ struct acpi_irq_dep_ctx *ctx = context;
+ struct acpi_resource_irq *irq;
+ struct acpi_resource_extended_irq *eirq;
+
+ switch (ares->type) {
+ case ACPI_RESOURCE_TYPE_IRQ:
+ irq = &ares->data.irq;
+ if (ctx->index >= irq->interrupt_count) {
+ ctx->index -= irq->interrupt_count;
+ return AE_OK;
+ }
+ ctx->handle = riscv_acpi_get_gsi_handle(irq->interrupts[ctx->index]);
+ return AE_CTRL_TERMINATE;
+ case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
+ eirq = &ares->data.extended_irq;
+ if (eirq->producer_consumer == ACPI_PRODUCER)
+ return AE_OK;
+
+ if (ctx->index >= eirq->interrupt_count) {
+ ctx->index -= eirq->interrupt_count;
+ return AE_OK;
+ }
+
+ /* Support GSIs only */
+ if (eirq->resource_source.string_length)
+ return AE_OK;
+
+ ctx->handle = riscv_acpi_get_gsi_handle(eirq->interrupts[ctx->index]);
+ return AE_CTRL_TERMINATE;
+ }
+
+ return AE_OK;
+}
+
+static int riscv_acpi_irq_get_dep(acpi_handle handle, unsigned int index, acpi_handle *gsi_handle)
+{
+ struct acpi_irq_dep_ctx ctx = {-EINVAL, index, NULL};
+
+ if (!gsi_handle)
+ return 0;
+
+ acpi_walk_resources(handle, METHOD_NAME__CRS, riscv_acpi_irq_get_parent, &ctx);
+ *gsi_handle = ctx.handle;
+ if (*gsi_handle)
+ return 1;
+
+ return 0;
+}
+
+static u32 riscv_acpi_add_prt_dep(acpi_handle handle)
+{
+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+ struct acpi_pci_routing_table *entry;
+ struct acpi_handle_list dep_devices;
+ acpi_handle gsi_handle;
+ acpi_handle link_handle;
+ acpi_status status;
+ u32 count = 0;
+
+ status = acpi_get_irq_routing_table(handle, &buffer);
+ if (ACPI_FAILURE(status)) {
+ acpi_handle_err(handle, "failed to get IRQ routing table\n");
+ kfree(buffer.pointer);
+ return 0;
+ }
+
+ entry = buffer.pointer;
+ while (entry && (entry->length > 0)) {
+ if (entry->source[0]) {
+ acpi_get_handle(handle, entry->source, &link_handle);
+ dep_devices.count = 1;
+ dep_devices.handles = kcalloc(1, sizeof(*dep_devices.handles), GFP_KERNEL);
+ if (!dep_devices.handles) {
+ acpi_handle_err(handle, "failed to allocate memory\n");
+ continue;
+ }
+
+ dep_devices.handles[0] = link_handle;
+ count += acpi_scan_add_dep(handle, &dep_devices);
+ } else {
+ gsi_handle = riscv_acpi_get_gsi_handle(entry->source_index);
+ dep_devices.count = 1;
+ dep_devices.handles = kcalloc(1, sizeof(*dep_devices.handles), GFP_KERNEL);
+ if (!dep_devices.handles) {
+ acpi_handle_err(handle, "failed to allocate memory\n");
+ continue;
+ }
+
+ dep_devices.handles[0] = gsi_handle;
+ count += acpi_scan_add_dep(handle, &dep_devices);
+ }
+
+ entry = (struct acpi_pci_routing_table *)
+ ((unsigned long)entry + entry->length);
+ }
+
+ kfree(buffer.pointer);
+ return count;
+}
+
+static u32 riscv_acpi_add_irq_dep(acpi_handle handle)
+{
+ struct acpi_handle_list dep_devices;
+ acpi_handle gsi_handle;
+ u32 count = 0;
+ int i;
+
+ for (i = 0;
+ riscv_acpi_irq_get_dep(handle, i, &gsi_handle);
+ i++) {
+ dep_devices.count = 1;
+ dep_devices.handles = kcalloc(1, sizeof(*dep_devices.handles), GFP_KERNEL);
+ if (!dep_devices.handles) {
+ acpi_handle_err(handle, "failed to allocate memory\n");
+ continue;
+ }
+
+ dep_devices.handles[0] = gsi_handle;
+ count += acpi_scan_add_dep(handle, &dep_devices);
+ }
+
+ return count;
+}
+
+u32 arch_acpi_add_auto_dep(acpi_handle handle)
+{
+ if (acpi_has_method(handle, "_PRT"))
+ return riscv_acpi_add_prt_dep(handle);
+
+ return riscv_acpi_add_irq_dep(handle);
+}
--
2.40.1
Add ACPI support in APLIC drivers. Use the mapping created early during
boot to get the details about the APLIC.
Signed-off-by: Sunil V L <[email protected]>
---
drivers/irqchip/irq-riscv-aplic-direct.c | 20 ++++---
drivers/irqchip/irq-riscv-aplic-main.c | 70 ++++++++++++++++--------
drivers/irqchip/irq-riscv-aplic-main.h | 1 +
drivers/irqchip/irq-riscv-aplic-msi.c | 9 ++-
4 files changed, 67 insertions(+), 33 deletions(-)
diff --git a/drivers/irqchip/irq-riscv-aplic-direct.c b/drivers/irqchip/irq-riscv-aplic-direct.c
index 4a3ffe856d6c..e24c2d3c78f6 100644
--- a/drivers/irqchip/irq-riscv-aplic-direct.c
+++ b/drivers/irqchip/irq-riscv-aplic-direct.c
@@ -4,6 +4,7 @@
* Copyright (C) 2022 Ventana Micro Systems Inc.
*/
+#include <linux/acpi.h>
#include <linux/bitfield.h>
#include <linux/bitops.h>
#include <linux/cpu.h>
@@ -189,17 +190,20 @@ static int aplic_direct_starting_cpu(unsigned int cpu)
}
static int aplic_direct_parse_parent_hwirq(struct device *dev, u32 index,
- u32 *parent_hwirq, unsigned long *parent_hartid)
+ u32 *parent_hwirq, unsigned long *parent_hartid,
+ struct aplic_priv *priv)
{
struct of_phandle_args parent;
int rc;
- /*
- * Currently, only OF fwnode is supported so extend this
- * function for ACPI support.
- */
- if (!is_of_node(dev->fwnode))
- return -EINVAL;
+ if (!is_of_node(dev->fwnode)) {
+ rc = acpi_get_ext_intc_parent_hartid(priv->id, index, parent_hartid);
+ if (rc)
+ return rc;
+
+ *parent_hwirq = RV_IRQ_EXT;
+ return 0;
+ }
rc = of_irq_parse_one(to_of_node(dev->fwnode), index, &parent);
if (rc)
@@ -237,7 +241,7 @@ int aplic_direct_setup(struct device *dev, void __iomem *regs)
/* Setup per-CPU IDC and target CPU mask */
current_cpu = get_cpu();
for (i = 0; i < priv->nr_idcs; i++) {
- rc = aplic_direct_parse_parent_hwirq(dev, i, &hwirq, &hartid);
+ rc = aplic_direct_parse_parent_hwirq(dev, i, &hwirq, &hartid, priv);
if (rc) {
dev_warn(dev, "parent irq for IDC%d not found\n", i);
continue;
diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
index 774a0c97fdab..c1fd328ddf7d 100644
--- a/drivers/irqchip/irq-riscv-aplic-main.c
+++ b/drivers/irqchip/irq-riscv-aplic-main.c
@@ -4,8 +4,10 @@
* Copyright (C) 2022 Ventana Micro Systems Inc.
*/
+#include <linux/acpi.h>
#include <linux/bitfield.h>
#include <linux/irqchip/riscv-aplic.h>
+#include <linux/irqchip/riscv-imsic.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_irq.h>
@@ -125,39 +127,50 @@ static void aplic_init_hw_irqs(struct aplic_priv *priv)
writel(0, priv->regs + APLIC_DOMAINCFG);
}
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id aplic_acpi_match[] = {
+ { "RSCV0002", 0 },
+ {}
+};
+MODULE_DEVICE_TABLE(acpi, aplic_acpi_match);
+
+#endif
+
int aplic_setup_priv(struct aplic_priv *priv, struct device *dev, void __iomem *regs)
{
struct of_phandle_args parent;
int rc;
- /*
- * Currently, only OF fwnode is supported so extend this
- * function for ACPI support.
- */
- if (!is_of_node(dev->fwnode))
- return -EINVAL;
-
/* Save device pointer and register base */
priv->dev = dev;
priv->regs = regs;
- /* Find out number of interrupt sources */
- rc = of_property_read_u32(to_of_node(dev->fwnode), "riscv,num-sources",
- &priv->nr_irqs);
- if (rc) {
- dev_err(dev, "failed to get number of interrupt sources\n");
- return rc;
- }
-
- /*
- * Find out number of IDCs based on parent interrupts
- *
- * If "msi-parent" property is present then we ignore the
- * APLIC IDCs which forces the APLIC driver to use MSI mode.
- */
- if (!of_property_present(to_of_node(dev->fwnode), "msi-parent")) {
- while (!of_irq_parse_one(to_of_node(dev->fwnode), priv->nr_idcs, &parent))
- priv->nr_idcs++;
+ if (is_of_node(dev->fwnode)) {
+ /* Find out number of interrupt sources */
+ rc = of_property_read_u32(to_of_node(dev->fwnode), "riscv,num-sources",
+ &priv->nr_irqs);
+ if (rc) {
+ dev_err(dev, "failed to get number of interrupt sources\n");
+ return rc;
+ }
+
+ /*
+ * Find out number of IDCs based on parent interrupts
+ *
+ * If "msi-parent" property is present then we ignore the
+ * APLIC IDCs which forces the APLIC driver to use MSI mode.
+ */
+ if (!of_property_present(to_of_node(dev->fwnode), "msi-parent")) {
+ while (!of_irq_parse_one(to_of_node(dev->fwnode), priv->nr_idcs, &parent))
+ priv->nr_idcs++;
+ }
+ } else {
+ rc = riscv_acpi_get_gsi_info(dev->fwnode, &priv->gsi_base, &priv->id,
+ &priv->nr_irqs, &priv->nr_idcs);
+ if (rc) {
+ dev_err(dev, "failed to find GSI mapping\n");
+ return rc;
+ }
}
/* Setup initial state APLIC interrupts */
@@ -186,6 +199,9 @@ static int aplic_probe(struct platform_device *pdev)
*/
if (is_of_node(dev->fwnode))
msi_mode = of_property_present(to_of_node(dev->fwnode), "msi-parent");
+ else
+ msi_mode = imsic_acpi_get_fwnode(NULL) ? 1 : 0;
+
if (msi_mode)
rc = aplic_msi_setup(dev, regs);
else
@@ -193,6 +209,11 @@ static int aplic_probe(struct platform_device *pdev)
if (rc)
dev_err(dev, "failed to setup APLIC in %s mode\n", msi_mode ? "MSI" : "direct");
+#ifdef CONFIG_ACPI
+ if (!acpi_disabled)
+ acpi_dev_clear_dependencies(ACPI_COMPANION(dev));
+#endif
+
return rc;
}
@@ -205,6 +226,7 @@ static struct platform_driver aplic_driver = {
.driver = {
.name = "riscv-aplic",
.of_match_table = aplic_match,
+ .acpi_match_table = ACPI_PTR(aplic_acpi_match),
},
.probe = aplic_probe,
};
diff --git a/drivers/irqchip/irq-riscv-aplic-main.h b/drivers/irqchip/irq-riscv-aplic-main.h
index 4393927d8c80..9fbf45c7b4f7 100644
--- a/drivers/irqchip/irq-riscv-aplic-main.h
+++ b/drivers/irqchip/irq-riscv-aplic-main.h
@@ -28,6 +28,7 @@ struct aplic_priv {
u32 gsi_base;
u32 nr_irqs;
u32 nr_idcs;
+ u32 id;
void __iomem *regs;
struct aplic_msicfg msicfg;
};
diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
index 028444af48bd..f5020241e0ed 100644
--- a/drivers/irqchip/irq-riscv-aplic-msi.c
+++ b/drivers/irqchip/irq-riscv-aplic-msi.c
@@ -157,6 +157,7 @@ static const struct msi_domain_template aplic_msi_template = {
int aplic_msi_setup(struct device *dev, void __iomem *regs)
{
const struct imsic_global_config *imsic_global;
+ struct irq_domain *msi_domain;
struct aplic_priv *priv;
struct aplic_msicfg *mc;
phys_addr_t pa;
@@ -239,8 +240,14 @@ int aplic_msi_setup(struct device *dev, void __iomem *regs)
* IMSIC and the IMSIC MSI domains are created later through
* the platform driver probing so we set it explicitly here.
*/
- if (is_of_node(dev->fwnode))
+ if (is_of_node(dev->fwnode)) {
of_msi_configure(dev, to_of_node(dev->fwnode));
+ } else {
+ msi_domain = irq_find_matching_fwnode(imsic_acpi_get_fwnode(dev),
+ DOMAIN_BUS_PLATFORM_MSI);
+ if (msi_domain)
+ dev_set_msi_domain(dev, msi_domain);
+ }
}
if (!msi_create_device_irq_domain(dev, MSI_DEFAULT_DOMAIN, &aplic_msi_template,
--
2.40.1
Add ACPI support in PLIC driver. Use the mapping created early during
boot to get details about the PLIC.
Signed-off-by: Sunil V L <[email protected]>
Co-developed-by: Haibo Xu <[email protected]>
Signed-off-by: Haibo Xu <[email protected]>
---
drivers/irqchip/irq-sifive-plic.c | 89 +++++++++++++++++++++++--------
1 file changed, 68 insertions(+), 21 deletions(-)
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index 8fb183ced1e7..b6b04b5923c2 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -3,6 +3,7 @@
* Copyright (C) 2017 SiFive
* Copyright (C) 2018 Christoph Hellwig
*/
+#include <linux/acpi.h>
#include <linux/cpu.h>
#include <linux/interrupt.h>
#include <linux/io.h>
@@ -70,6 +71,8 @@ struct plic_priv {
unsigned long plic_quirks;
unsigned int nr_irqs;
unsigned long *prio_save;
+ u32 gsi_base;
+ int id;
};
struct plic_handler {
@@ -324,6 +327,10 @@ static int plic_irq_domain_translate(struct irq_domain *d,
{
struct plic_priv *priv = d->host_data;
+ /* For DT, gsi_base is always zero. */
+ if (fwspec->param[0] >= priv->gsi_base)
+ fwspec->param[0] = fwspec->param[0] - priv->gsi_base;
+
if (test_bit(PLIC_QUIRK_EDGE_INTERRUPT, &priv->plic_quirks))
return irq_domain_translate_twocell(d, fwspec, hwirq, type);
@@ -424,18 +431,32 @@ static const struct of_device_id plic_match[] = {
{}
};
+#ifdef CONFIG_ACPI
+
+static const struct acpi_device_id plic_acpi_match[] = {
+ { "RSCV0001", 0 },
+ {}
+};
+MODULE_DEVICE_TABLE(acpi, plic_acpi_match);
+
+#endif
static int plic_parse_nr_irqs_and_contexts(struct platform_device *pdev,
- u32 *nr_irqs, u32 *nr_contexts)
+ u32 *nr_irqs, u32 *nr_contexts,
+ u32 *gsi_base, u32 *id)
{
struct device *dev = &pdev->dev;
int rc;
- /*
- * Currently, only OF fwnode is supported so extend this
- * function for ACPI support.
- */
- if (!is_of_node(dev->fwnode))
- return -EINVAL;
+ if (!is_of_node(dev->fwnode)) {
+ riscv_acpi_get_gsi_info(dev->fwnode, gsi_base, id, nr_irqs, NULL);
+ acpi_get_plic_nr_contexts(*id, nr_contexts);
+ if (WARN_ON(!*nr_contexts)) {
+ dev_err(dev, "no PLIC context available\n");
+ return -EINVAL;
+ }
+
+ return 0;
+ }
rc = of_property_read_u32(to_of_node(dev->fwnode), "riscv,ndev", nr_irqs);
if (rc) {
@@ -449,23 +470,29 @@ static int plic_parse_nr_irqs_and_contexts(struct platform_device *pdev,
return -EINVAL;
}
+ *gsi_base = 0;
+ *id = 0;
+
return 0;
}
static int plic_parse_context_parent(struct platform_device *pdev, u32 context,
- u32 *parent_hwirq, int *parent_cpu)
+ u32 *parent_hwirq, int *parent_cpu, u32 id)
{
struct device *dev = &pdev->dev;
struct of_phandle_args parent;
unsigned long hartid;
int rc;
- /*
- * Currently, only OF fwnode is supported so extend this
- * function for ACPI support.
- */
- if (!is_of_node(dev->fwnode))
- return -EINVAL;
+ if (!is_of_node(dev->fwnode)) {
+ rc = acpi_get_ext_intc_parent_hartid(id, context, &hartid);
+ if (rc)
+ return rc;
+
+ *parent_cpu = riscv_hartid_to_cpuid(hartid);
+ *parent_hwirq = RV_IRQ_EXT;
+ return 0;
+ }
rc = of_irq_parse_one(to_of_node(dev->fwnode), context, &parent);
if (rc)
@@ -490,7 +517,9 @@ static int plic_probe(struct platform_device *pdev)
struct irq_domain *domain;
struct plic_priv *priv;
irq_hw_number_t hwirq;
+ int id, context_id;
bool cpuhp_setup;
+ u32 gsi_base;
if (is_of_node(dev->fwnode)) {
const struct of_device_id *id;
@@ -500,7 +529,7 @@ static int plic_probe(struct platform_device *pdev)
plic_quirks = (unsigned long)id->data;
}
- error = plic_parse_nr_irqs_and_contexts(pdev, &nr_irqs, &nr_contexts);
+ error = plic_parse_nr_irqs_and_contexts(pdev, &nr_irqs, &nr_contexts, &gsi_base, &id);
if (error)
return error;
@@ -511,6 +540,8 @@ static int plic_probe(struct platform_device *pdev)
priv->dev = dev;
priv->plic_quirks = plic_quirks;
priv->nr_irqs = nr_irqs;
+ priv->gsi_base = gsi_base;
+ priv->id = id;
priv->regs = devm_platform_ioremap_resource(pdev, 0);
if (WARN_ON(!priv->regs))
@@ -521,12 +552,22 @@ static int plic_probe(struct platform_device *pdev)
return -ENOMEM;
for (i = 0; i < nr_contexts; i++) {
- error = plic_parse_context_parent(pdev, i, &parent_hwirq, &cpu);
+ error = plic_parse_context_parent(pdev, i, &parent_hwirq, &cpu, priv->id);
if (error) {
dev_warn(dev, "hwirq for context%d not found\n", i);
continue;
}
+ if (is_of_node(dev->fwnode)) {
+ context_id = i;
+ } else {
+ error = acpi_get_plic_context(priv->id, i, &context_id);
+ if (error) {
+ dev_warn(dev, "invalid context id for context%d\n", i);
+ continue;
+ }
+ }
+
/*
* Skip contexts other than external interrupts for our
* privilege level.
@@ -572,10 +613,10 @@ static int plic_probe(struct platform_device *pdev)
cpumask_set_cpu(cpu, &priv->lmask);
handler->present = true;
handler->hart_base = priv->regs + CONTEXT_BASE +
- i * CONTEXT_SIZE;
+ context_id * CONTEXT_SIZE;
raw_spin_lock_init(&handler->enable_lock);
handler->enable_base = priv->regs + CONTEXT_ENABLE_BASE +
- i * CONTEXT_ENABLE_SIZE;
+ context_id * CONTEXT_ENABLE_SIZE;
handler->priv = priv;
handler->enable_save = devm_kcalloc(dev, DIV_ROUND_UP(nr_irqs, 32),
@@ -591,8 +632,8 @@ static int plic_probe(struct platform_device *pdev)
nr_handlers++;
}
- priv->irqdomain = irq_domain_add_linear(to_of_node(dev->fwnode), nr_irqs + 1,
- &plic_irqdomain_ops, priv);
+ priv->irqdomain = irq_domain_create_linear(dev->fwnode, nr_irqs + 1,
+ &plic_irqdomain_ops, priv);
if (WARN_ON(!priv->irqdomain))
goto fail_cleanup_contexts;
@@ -619,13 +660,18 @@ static int plic_probe(struct platform_device *pdev)
}
}
+#ifdef CONFIG_ACPI
+ if (!acpi_disabled)
+ acpi_dev_clear_dependencies(ACPI_COMPANION(dev));
+#endif
+
dev_info(dev, "mapped %d interrupts with %d handlers for %d contexts.\n",
nr_irqs, nr_handlers, nr_contexts);
return 0;
fail_cleanup_contexts:
for (i = 0; i < nr_contexts; i++) {
- if (plic_parse_context_parent(pdev, i, &parent_hwirq, &cpu))
+ if (plic_parse_context_parent(pdev, i, &parent_hwirq, &cpu, priv->id))
continue;
if (parent_hwirq != RV_IRQ_EXT || cpu < 0)
continue;
@@ -644,6 +690,7 @@ static struct platform_driver plic_driver = {
.driver = {
.name = "riscv-plic",
.of_match_table = plic_match,
+ .acpi_match_table = ACPI_PTR(plic_acpi_match),
},
.probe = plic_probe,
};
--
2.40.1
On Wed, May 01, 2024 at 05:47:26PM +0530, Sunil V L wrote:
> The functions defined in arm64 for ACPI support are required
> for RISC-V also. To avoid duplication, move these functions
> to common location.
>
> Signed-off-by: Sunil V L <[email protected]>
> Acked-by: Bjorn Helgaas <[email protected]>
> ---
> arch/arm64/kernel/pci.c | 191 ----------------------------------------
> drivers/pci/pci-acpi.c | 182 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 182 insertions(+), 191 deletions(-)
Looks like a straight-forward move of the code, so:
Acked-by: Will Deacon <[email protected]>
Will
On Wed, May 01, 2024 at 05:47:33PM +0530, Sunil V L wrote:
> RISC-V platforms need to use dependencies between PCI host bridge, Link
> devices and the interrupt controllers to ensure probe order. The
> dependency is like below.
>
> Interrupt controller <-- Link Device <-- PCI Host bridge.
>
> If there is no dependency added between Link device and PCI Host Bridge,
> then the PCI end points can get probed prior to link device, unable to
> get mapping for INTx.
>
> So, add the link device's HID to dependency honor list and also clear it
> after its probe.
>
> Since this is required only for architectures like RISC-V, enable this
> code under a new config option and set this only in RISC-V.
>
> Signed-off-by: Sunil V L <[email protected]>
> ---
> arch/riscv/Kconfig | 1 +
> drivers/acpi/Kconfig | 3 +++
> drivers/acpi/pci_link.c | 3 +++
> drivers/acpi/scan.c | 1 +
> 4 files changed, 8 insertions(+)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index f961449ca077..f7a36d79ff1a 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -14,6 +14,7 @@ config RISCV
> def_bool y
> select ACPI_GENERIC_GSI if ACPI
> select ACPI_REDUCED_HARDWARE_ONLY if ACPI
> + select ARCH_ACPI_DEFERRED_GSI if ACPI
> select ARCH_DMA_DEFAULT_COHERENT
> select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
> select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index e3a7c2aedd5f..ebec1707f662 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -587,6 +587,9 @@ config ACPI_PRMT
> substantially increase computational overhead related to the
> initialization of some server systems.
>
> +config ARCH_ACPI_DEFERRED_GSI
> + bool
> +
> endif # ACPI
>
> config X86_PM_TIMER
> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
> index aa1038b8aec4..48cdcedafad6 100644
> --- a/drivers/acpi/pci_link.c
> +++ b/drivers/acpi/pci_link.c
> @@ -748,6 +748,9 @@ static int acpi_pci_link_add(struct acpi_device *device,
> if (result)
> kfree(link);
>
> + if (IS_ENABLED(CONFIG_ARCH_ACPI_DEFERRED_GSI))
> + acpi_dev_clear_dependencies(device);
This is really a question for Rafael, but it doesn't seem right that
this completely depends on a config option.
Is there a reason this wouldn't work for all architectures, i.e., what
would happen if you just called acpi_dev_clear_dependencies()
unconditionally?
> +
> return result < 0 ? result : 1;
> }
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 3eeb4ce39fcc..67677a6ff8e3 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -834,6 +834,7 @@ static const char * const acpi_honor_dep_ids[] = {
> "INTC10CF", /* IVSC (MTL) driver must be loaded to allow i2c access to camera sensors */
> "RSCV0001", /* RISC-V PLIC */
> "RSCV0002", /* RISC-V APLIC */
> + "PNP0C0F", /* PCI Link Device */
> NULL
> };
>
> --
> 2.40.1
>
>
On Wed, May 01, 2024 at 05:47:42PM +0530, Sunil V L wrote:
> RISC-V has non-PNP generic 16550A compatible UART which needs to be
> enumerated as ACPI platform device. Add driver support for such devices
> similar to 8250_of.
..
> + * This driver is for generic 16550 compatible UART enumerated via ACPI
> + * platform bus instead of PNP bus like PNP0501. This is not a full
This has to be told in the commit message. Anyway, we don't need a duplication
code, please use 8250_pnp.
..
> + { "RSCV0003", 0 },
Does it have _CID to be PNP0501?
If not, add this ID to the 8250_pnp.
..
P.S.
The code you submitted has a lot of small style issues, I can comment on them
if you want, but as I said this code is not needed at all.
--
With Best Regards,
Andy Shevchenko
On Wed, May 01, 2024 at 05:47:29PM +0530, Sunil V L wrote:
> Some architectures like RISC-V will use implicit dependencies like GSI
> map to create dependencies between interrupt controller and devices. To
> support doing that, the function which creates the dependency, is
> refactored bit and made public so that dependency can be added from
> outside of scan.c as well.
Side note: If you haven't used --histogram diff algo when preparing
the patches, do it from now on.
--
With Best Regards,
Andy Shevchenko
On Wed, May 01, 2024 at 11:56:15AM -0500, Bjorn Helgaas wrote:
> On Wed, May 01, 2024 at 05:47:33PM +0530, Sunil V L wrote:
> > RISC-V platforms need to use dependencies between PCI host bridge, Link
> > devices and the interrupt controllers to ensure probe order. The
> > dependency is like below.
> >
> > Interrupt controller <-- Link Device <-- PCI Host bridge.
> >
> > If there is no dependency added between Link device and PCI Host Bridge,
> > then the PCI end points can get probed prior to link device, unable to
> > get mapping for INTx.
> >
> > So, add the link device's HID to dependency honor list and also clear it
> > after its probe.
> >
> > Since this is required only for architectures like RISC-V, enable this
> > code under a new config option and set this only in RISC-V.
..
> > + if (IS_ENABLED(CONFIG_ARCH_ACPI_DEFERRED_GSI))
> > + acpi_dev_clear_dependencies(device);
>
> This is really a question for Rafael, but it doesn't seem right that
> this completely depends on a config option.
+1 here, fells like a hack and looks like a hack.
> Is there a reason this wouldn't work for all architectures, i.e., what
> would happen if you just called acpi_dev_clear_dependencies()
> unconditionally?
--
With Best Regards,
Andy Shevchenko
On Wed, May 01, 2024 at 05:47:26PM +0530, Sunil V L wrote:
> The functions defined in arm64 for ACPI support are required
> for RISC-V also. To avoid duplication, move these functions
> to common location.
..
There are -M -C parameters to git format-patch. Use them in the next version of
the series. (Note, you may add percentage numbers to each of those parameters
to get prettier result.)
--
With Best Regards,
Andy Shevchenko
On Thu, May 02, 2024 at 12:25:35PM +0300, Andy Shevchenko wrote:
> On Wed, May 01, 2024 at 11:56:15AM -0500, Bjorn Helgaas wrote:
> > On Wed, May 01, 2024 at 05:47:33PM +0530, Sunil V L wrote:
> > > RISC-V platforms need to use dependencies between PCI host bridge, Link
> > > devices and the interrupt controllers to ensure probe order. The
> > > dependency is like below.
> > >
> > > Interrupt controller <-- Link Device <-- PCI Host bridge.
> > >
> > > If there is no dependency added between Link device and PCI Host Bridge,
> > > then the PCI end points can get probed prior to link device, unable to
> > > get mapping for INTx.
> > >
> > > So, add the link device's HID to dependency honor list and also clear it
> > > after its probe.
> > >
> > > Since this is required only for architectures like RISC-V, enable this
> > > code under a new config option and set this only in RISC-V.
>
> ...
>
> > > + if (IS_ENABLED(CONFIG_ARCH_ACPI_DEFERRED_GSI))
> > > + acpi_dev_clear_dependencies(device);
> >
> > This is really a question for Rafael, but it doesn't seem right that
> > this completely depends on a config option.
>
> +1 here, fells like a hack and looks like a hack.
>
I can remove the config option. I just thought this would probably never
required to be called on other architectures.
Unless there is an objection, I will remove it in next version.
Thanks!
Sunil
> > Is there a reason this wouldn't work for all architectures, i.e., what
> > would happen if you just called acpi_dev_clear_dependencies()
> > unconditionally?
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
On Thu, May 02, 2024 at 12:17:59PM +0300, Andy Shevchenko wrote:
> On Wed, May 01, 2024 at 05:47:42PM +0530, Sunil V L wrote:
> > RISC-V has non-PNP generic 16550A compatible UART which needs to be
> > enumerated as ACPI platform device. Add driver support for such devices
> > similar to 8250_of.
>
> ...
>
> > + * This driver is for generic 16550 compatible UART enumerated via ACPI
> > + * platform bus instead of PNP bus like PNP0501. This is not a full
>
> This has to be told in the commit message. Anyway, we don't need a duplication
> code, please use 8250_pnp.
>
Hi Andy,
Thank you for the review!. Major issue with PNP0501 is, it gets enumerated
in a different way which causes issue to get _DEP to work.
pnpacpi_init() creates PNP data structures which gets skipped if the
UART puts _DEP on the GSI provider (interrupt controller). In that case,
we need to somehow reinitialize such PNP devices after interrupt
controller gets probed. I tried a solution [1] but it required several
functions to be moved out of __init.
This driver is not a duplicate of 8250_pnp. It just relies on UART
enumerated as platform device instead of using PNP interfaces.
Isn't it better and simple to have an option to enumerate as platform
device instead of PNP?
[1] - https://patchwork.kernel.org/project/linux-pci/patch/[email protected]/
Thanks,
Sunil
> ...
>
> > + { "RSCV0003", 0 },
>
> Does it have _CID to be PNP0501?
> If not, add this ID to the 8250_pnp.
>
> ...
>
> P.S.
> The code you submitted has a lot of small style issues, I can comment on them
> if you want, but as I said this code is not needed at all.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
On Thu, May 02, 2024 at 12:24:14PM +0300, Andy Shevchenko wrote:
> On Wed, May 01, 2024 at 05:47:28PM +0530, Sunil V L wrote:
> > Add a new function for RISC-V to do any architecture specific
> > initialization. This function will be used to create platform devices
> > like APLIC, PLIC, RISC-V IOMMU etc. This is similar to acpi_arm_init().
>
> What is the special about this architecture that it requires a separate
> initialization that is _not_ going to be in other cases?
> Please, elaborate.
>
This init function will be used to create GSI mapping structures and in
future may be others like iommu. Like I mentioned, ARM already has
similar function acpi_arm_init(). So, it is not new right?
Thanks,
Sunil
On Thu, May 02, 2024 at 12:20:06PM +0300, Andy Shevchenko wrote:
> On Wed, May 01, 2024 at 05:47:29PM +0530, Sunil V L wrote:
> > Some architectures like RISC-V will use implicit dependencies like GSI
> > map to create dependencies between interrupt controller and devices. To
> > support doing that, the function which creates the dependency, is
> > refactored bit and made public so that dependency can be added from
> > outside of scan.c as well.
>
> Side note: If you haven't used --histogram diff algo when preparing
> the patches, do it from now on.
>
No, I didn't use. Thank you very much for letting me know. I will use in
next version.
Thanks!
Sunil
On Thu, May 02, 2024 at 03:20:08PM +0530, Sunil V L wrote:
> On Thu, May 02, 2024 at 12:17:59PM +0300, Andy Shevchenko wrote:
> > On Wed, May 01, 2024 at 05:47:42PM +0530, Sunil V L wrote:
..
> > > + * This driver is for generic 16550 compatible UART enumerated via ACPI
> > > + * platform bus instead of PNP bus like PNP0501. This is not a full
> >
> > This has to be told in the commit message. Anyway, we don't need a duplication
> > code, please use 8250_pnp.
>
> Thank you for the review!. Major issue with PNP0501 is, it gets enumerated
> in a different way which causes issue to get _DEP to work.
> pnpacpi_init() creates PNP data structures which gets skipped if the
> UART puts _DEP on the GSI provider (interrupt controller). In that case,
> we need to somehow reinitialize such PNP devices after interrupt
> controller gets probed.
Then fix that code, we don't want a hack driver on top of the existing one for
the same.
What I might think out of head is that used IRQ core for your case should
return a deferred probe error code when it's not ready, then 8250_pnp will
get reprobed.
> I tried a solution [1] but it required several
> functions to be moved out of __init.
> This driver is not a duplicate of 8250_pnp. It just relies on UART
> enumerated as platform device instead of using PNP interfaces.
> Isn't it better and simple to have an option to enumerate as platform
> device instead of PNP?
Ah, then extract platform driver first from 8250_core.c.
> [1] - https://patchwork.kernel.org/project/linux-pci/patch/[email protected]/
--
With Best Regards,
Andy Shevchenko
On Thu, May 02, 2024 at 12:22:28PM +0300, Andy Shevchenko wrote:
> On Wed, May 01, 2024 at 05:47:26PM +0530, Sunil V L wrote:
> > The functions defined in arm64 for ACPI support are required
> > for RISC-V also. To avoid duplication, move these functions
> > to common location.
>
> ...
>
> There are -M -C parameters to git format-patch. Use them in the next version of
> the series. (Note, you may add percentage numbers to each of those parameters
> to get prettier result.)
>
Yeah, I tried different combination of percentage after you told me last
time. I just didn't see any difference in the patch generated. Let me
try again while sending next version. Thanks!
On Thu, May 02, 2024 at 03:32:25PM +0530, Sunil V L wrote:
> On Thu, May 02, 2024 at 12:24:14PM +0300, Andy Shevchenko wrote:
> > On Wed, May 01, 2024 at 05:47:28PM +0530, Sunil V L wrote:
> > > Add a new function for RISC-V to do any architecture specific
> > > initialization. This function will be used to create platform devices
> > > like APLIC, PLIC, RISC-V IOMMU etc. This is similar to acpi_arm_init().
> >
> > What is the special about this architecture that it requires a separate
> > initialization that is _not_ going to be in other cases?
> > Please, elaborate.
> >
> This init function will be used to create GSI mapping structures and in
> future may be others like iommu. Like I mentioned, ARM already has
> similar function acpi_arm_init(). So, it is not new right?
>
Just to add:
This is to initialise everything around all the arch specific tables
which you will not have on any other architectures. We could execute
on all architectures but the tables will never be found. The main point
is why do we want to do that if we can optimise and skip on all other
archs.
--
Regards,
Sudeep
On Thu, May 02, 2024 at 11:12:43AM +0100, Sudeep Holla wrote:
> On Thu, May 02, 2024 at 03:32:25PM +0530, Sunil V L wrote:
> > On Thu, May 02, 2024 at 12:24:14PM +0300, Andy Shevchenko wrote:
> > > On Wed, May 01, 2024 at 05:47:28PM +0530, Sunil V L wrote:
> > > > Add a new function for RISC-V to do any architecture specific
> > > > initialization. This function will be used to create platform devices
> > > > like APLIC, PLIC, RISC-V IOMMU etc. This is similar to acpi_arm_init().
> > >
> > > What is the special about this architecture that it requires a separate
> > > initialization that is _not_ going to be in other cases?
> > > Please, elaborate.
> > >
> > This init function will be used to create GSI mapping structures and in
> > future may be others like iommu. Like I mentioned, ARM already has
> > similar function acpi_arm_init(). So, it is not new right?
>
> Just to add:
>
> This is to initialise everything around all the arch specific tables
> which you will not have on any other architectures. We could execute
> on all architectures but the tables will never be found. The main point
> is why do we want to do that if we can optimise and skip on all other
> archs.
You need to carefully write the commit messages. Some kind of the above
paragraphs has to be in there.
--
With Best Regards,
Andy Shevchenko
On Thu, May 02, 2024 at 01:19:55PM +0300, Andy Shevchenko wrote:
> On Thu, May 02, 2024 at 11:12:43AM +0100, Sudeep Holla wrote:
> > On Thu, May 02, 2024 at 03:32:25PM +0530, Sunil V L wrote:
> > > On Thu, May 02, 2024 at 12:24:14PM +0300, Andy Shevchenko wrote:
> > > > On Wed, May 01, 2024 at 05:47:28PM +0530, Sunil V L wrote:
> > > > > Add a new function for RISC-V to do any architecture specific
> > > > > initialization. This function will be used to create platform devices
> > > > > like APLIC, PLIC, RISC-V IOMMU etc. This is similar to acpi_arm_init().
> > > >
> > > > What is the special about this architecture that it requires a separate
> > > > initialization that is _not_ going to be in other cases?
> > > > Please, elaborate.
> > > >
> > > This init function will be used to create GSI mapping structures and in
> > > future may be others like iommu. Like I mentioned, ARM already has
> > > similar function acpi_arm_init(). So, it is not new right?
> >
> > Just to add:
> >
> > This is to initialise everything around all the arch specific tables
> > which you will not have on any other architectures. We could execute
> > on all architectures but the tables will never be found. The main point
> > is why do we want to do that if we can optimise and skip on all other
> > archs.
>
> You need to carefully write the commit messages. Some kind of the above
> paragraphs has to be in there.
>
Sure, let me update the commit message on similar lines.
Thanks,
Sunil
On Thu, May 02, 2024 at 01:09:57PM +0300, Andy Shevchenko wrote:
> On Thu, May 02, 2024 at 03:20:08PM +0530, Sunil V L wrote:
> > On Thu, May 02, 2024 at 12:17:59PM +0300, Andy Shevchenko wrote:
> > > On Wed, May 01, 2024 at 05:47:42PM +0530, Sunil V L wrote:
>
> ...
>
> > > > + * This driver is for generic 16550 compatible UART enumerated via ACPI
> > > > + * platform bus instead of PNP bus like PNP0501. This is not a full
> > >
> > > This has to be told in the commit message. Anyway, we don't need a duplication
> > > code, please use 8250_pnp.
> >
> > Thank you for the review!. Major issue with PNP0501 is, it gets enumerated
> > in a different way which causes issue to get _DEP to work.
> > pnpacpi_init() creates PNP data structures which gets skipped if the
> > UART puts _DEP on the GSI provider (interrupt controller). In that case,
> > we need to somehow reinitialize such PNP devices after interrupt
> > controller gets probed.
>
> Then fix that code, we don't want a hack driver on top of the existing one for
> the same.
>
> What I might think out of head is that used IRQ core for your case should
> return a deferred probe error code when it's not ready, then 8250_pnp will
> get reprobed.
>
Deferred probe was ruled out in prior discussion. Also, deferred probe
will not work with _DEP approach. The reason is, PNP devices itself are
not getting created from the ACPI name space when they have _DEP. Hence,
serial_pnp_probe() will not be called at all.
> > I tried a solution [1] but it required several
> > functions to be moved out of __init.
>
> > This driver is not a duplicate of 8250_pnp. It just relies on UART
> > enumerated as platform device instead of using PNP interfaces.
> > Isn't it better and simple to have an option to enumerate as platform
> > device instead of PNP?
>
> Ah, then extract platform driver first from 8250_core.c.
>
Let me know if I understand your suggestion correctly. Do you mean call
something like serial8250_acpi_init() from serial8250_init() and
register the driver directly in serial8250_acpi_init()?
Thanks,
Sunil
On Wed, May 01, 2024 at 05:47:28PM +0530, Sunil V L wrote:
> Add a new function for RISC-V to do any architecture specific
> initialization. This function will be used to create platform devices
> like APLIC, PLIC, RISC-V IOMMU etc. This is similar to acpi_arm_init().
What is the special about this architecture that it requires a separate
initialization that is _not_ going to be in other cases?
Please, elaborate.
--
With Best Regards,
Andy Shevchenko
On Thu, May 02, 2024 at 04:50:57PM +0530, Sunil V L wrote:
> On Thu, May 02, 2024 at 01:09:57PM +0300, Andy Shevchenko wrote:
> > On Thu, May 02, 2024 at 03:20:08PM +0530, Sunil V L wrote:
> > > On Thu, May 02, 2024 at 12:17:59PM +0300, Andy Shevchenko wrote:
..
> > > This driver is not a duplicate of 8250_pnp. It just relies on UART
> > > enumerated as platform device instead of using PNP interfaces.
> > > Isn't it better and simple to have an option to enumerate as platform
> > > device instead of PNP?
> >
> > Ah, then extract platform driver first from 8250_core.c.
> >
> Let me know if I understand your suggestion correctly. Do you mean call
> something like serial8250_acpi_init() from serial8250_init() and
> register the driver directly in serial8250_acpi_init()?
Extract the code to be 8250_platform.c and update that file.
I have locally the extraction of RSA code, I will see if I can help you
with the rest.
--
With Best Regards,
Andy Shevchenko
On Thu, May 02, 2024 at 06:35:52PM +0300, Andy Shevchenko wrote:
> On Thu, May 02, 2024 at 04:50:57PM +0530, Sunil V L wrote:
> > On Thu, May 02, 2024 at 01:09:57PM +0300, Andy Shevchenko wrote:
> > > On Thu, May 02, 2024 at 03:20:08PM +0530, Sunil V L wrote:
> > > > On Thu, May 02, 2024 at 12:17:59PM +0300, Andy Shevchenko wrote:
>
> ...
>
> > > > This driver is not a duplicate of 8250_pnp. It just relies on UART
> > > > enumerated as platform device instead of using PNP interfaces.
> > > > Isn't it better and simple to have an option to enumerate as platform
> > > > device instead of PNP?
> > >
> > > Ah, then extract platform driver first from 8250_core.c.
> > >
> > Let me know if I understand your suggestion correctly. Do you mean call
> > something like serial8250_acpi_init() from serial8250_init() and
> > register the driver directly in serial8250_acpi_init()?
>
> Extract the code to be 8250_platform.c and update that file.
> I have locally the extraction of RSA code, I will see if I can help you
> with the rest.
>
Thanks!. That will be helpful. TBH, I don't understand what to do for
extracting the platform driver code. There are already several vendor
specific UART drivers (ex: 8250_fsl.c) which are enumerated as platform
devices. 8250_core.c looks cleanly supporting such drivers which can
register themselves with the core. For generic UART, DT has 8250_of.c
and ACPI has 8250_pnp.c. But 8250_pnp.c comes with baggage of PNP
contract. So, the driver in this patch is similar to vendor specific
drivers to support generic uart devices which are enumerated as platform
device. I can rename 8250_acpi.c to 8250_platform.c if that is better.
Could you please help with a patch even if not compiled so that I can
understand your suggestion better?
Thanks for your help!
Sunil
On Fri, May 03, 2024 at 07:29:35PM +0530, Sunil V L wrote:
> On Thu, May 02, 2024 at 06:35:52PM +0300, Andy Shevchenko wrote:
> > On Thu, May 02, 2024 at 04:50:57PM +0530, Sunil V L wrote:
> > > On Thu, May 02, 2024 at 01:09:57PM +0300, Andy Shevchenko wrote:
> > > > On Thu, May 02, 2024 at 03:20:08PM +0530, Sunil V L wrote:
> > > > > On Thu, May 02, 2024 at 12:17:59PM +0300, Andy Shevchenko wrote:
..
> > > > > This driver is not a duplicate of 8250_pnp. It just relies on UART
> > > > > enumerated as platform device instead of using PNP interfaces.
> > > > > Isn't it better and simple to have an option to enumerate as platform
> > > > > device instead of PNP?
> > > >
> > > > Ah, then extract platform driver first from 8250_core.c.
> > > >
> > > Let me know if I understand your suggestion correctly. Do you mean call
> > > something like serial8250_acpi_init() from serial8250_init() and
> > > register the driver directly in serial8250_acpi_init()?
> >
> > Extract the code to be 8250_platform.c and update that file.
> > I have locally the extraction of RSA code, I will see if I can help you
> > with the rest.
> >
> Thanks!. That will be helpful. TBH, I don't understand what to do for
> extracting the platform driver code. There are already several vendor
> specific UART drivers (ex: 8250_fsl.c) which are enumerated as platform
> devices. 8250_core.c looks cleanly supporting such drivers which can
> register themselves with the core. For generic UART, DT has 8250_of.c
> and ACPI has 8250_pnp.c. But 8250_pnp.c comes with baggage of PNP
> contract. So, the driver in this patch is similar to vendor specific
> drivers to support generic uart devices which are enumerated as platform
> device.
> I can rename 8250_acpi.c to 8250_platform.c if that is better.
No, that's not what I meant. We _already_ have a generic platform driver,
it's just inline in the 8250_core.c and needs to be extracted. When it's done,
you may simply add an ACPI table to it.
> Could you please help with a patch even if not compiled so that I can
> understand your suggestion better?
--
With Best Regards,
Andy Shevchenko
On Wed, May 01, 2024 at 05:47:42PM +0530, Sunil V L wrote:
> RISC-V has non-PNP generic 16550A compatible UART which needs to be
> enumerated as ACPI platform device. Add driver support for such devices
> similar to 8250_of.
>
> The driver is enabled when the CONFIG_SERIAL_ACPI_PLATFORM option is
> enabled. Enable this option for RISC-V.
>
> Signed-off-by: Sunil V L <[email protected]>
> ---
> arch/riscv/configs/defconfig | 1 +
> drivers/tty/serial/8250/8250_acpi.c | 96 +++++++++++++++++++++++++++++
> drivers/tty/serial/8250/Kconfig | 8 +++
> drivers/tty/serial/8250/Makefile | 1 +
> 4 files changed, 106 insertions(+)
> create mode 100644 drivers/tty/serial/8250/8250_acpi.c
>
> diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
> index 3cae018f9315..bea8241f52eb 100644
> --- a/arch/riscv/configs/defconfig
> +++ b/arch/riscv/configs/defconfig
> @@ -150,6 +150,7 @@ CONFIG_SERIAL_8250=y
> CONFIG_SERIAL_8250_CONSOLE=y
> CONFIG_SERIAL_8250_DW=y
> CONFIG_SERIAL_OF_PLATFORM=y
> +CONFIG_SERIAL_ACPI_PLATFORM=y
> CONFIG_SERIAL_SH_SCI=y
> CONFIG_SERIAL_EARLYCON_RISCV_SBI=y
> CONFIG_VIRTIO_CONSOLE=y
> diff --git a/drivers/tty/serial/8250/8250_acpi.c b/drivers/tty/serial/8250/8250_acpi.c
> new file mode 100644
> index 000000000000..3682443bb69c
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_acpi.c
> @@ -0,0 +1,96 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Serial Port driver for ACPI platform devices
> + *
> + * This driver is for generic 16550 compatible UART enumerated via ACPI
> + * platform bus instead of PNP bus like PNP0501. This is not a full
> + * driver but mostly provides the ACPI wrapper and uses generic
> + * 8250 framework for rest of the functionality.
No copyright line? Odd, but ok, I'll take it, glad to see your company
finally realizes the lack of needing them :)
And as Andy said, please use the existing driver and extend what you
need, don't write a new one, we really don't need a new one.
> +static int acpi_platform_serial_probe(struct platform_device *pdev)
> +{
> + struct acpi_serial_info *data;
> + struct uart_8250_port port8250;
> + struct device *dev = &pdev->dev;
> + struct resource *regs;
> +
> + int ret, irq;
> +
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!regs) {
> + dev_err(dev, "no registers defined\n");
> + return -EINVAL;
> + }
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return irq;
> +
> + memset(&port8250, 0, sizeof(port8250));
> +
> + spin_lock_init(&port8250.port.lock);
Are you sure this works?
thanks,
greg k-h
On Fri, May 03, 2024 at 06:32:10PM +0300, Andy Shevchenko wrote:
> On Fri, May 03, 2024 at 07:29:35PM +0530, Sunil V L wrote:
> > On Thu, May 02, 2024 at 06:35:52PM +0300, Andy Shevchenko wrote:
> > > On Thu, May 02, 2024 at 04:50:57PM +0530, Sunil V L wrote:
> > > > On Thu, May 02, 2024 at 01:09:57PM +0300, Andy Shevchenko wrote:
> > > > > On Thu, May 02, 2024 at 03:20:08PM +0530, Sunil V L wrote:
> > > > > > On Thu, May 02, 2024 at 12:17:59PM +0300, Andy Shevchenko wrote:
>
> ...
>
> > > > > > This driver is not a duplicate of 8250_pnp. It just relies on UART
> > > > > > enumerated as platform device instead of using PNP interfaces.
> > > > > > Isn't it better and simple to have an option to enumerate as platform
> > > > > > device instead of PNP?
> > > > >
> > > > > Ah, then extract platform driver first from 8250_core.c.
> > > > >
> > > > Let me know if I understand your suggestion correctly. Do you mean call
> > > > something like serial8250_acpi_init() from serial8250_init() and
> > > > register the driver directly in serial8250_acpi_init()?
> > >
> > > Extract the code to be 8250_platform.c and update that file.
> > > I have locally the extraction of RSA code, I will see if I can help you
> > > with the rest.
> > >
> > Thanks!. That will be helpful. TBH, I don't understand what to do for
> > extracting the platform driver code. There are already several vendor
> > specific UART drivers (ex: 8250_fsl.c) which are enumerated as platform
> > devices. 8250_core.c looks cleanly supporting such drivers which can
> > register themselves with the core. For generic UART, DT has 8250_of.c
> > and ACPI has 8250_pnp.c. But 8250_pnp.c comes with baggage of PNP
> > contract. So, the driver in this patch is similar to vendor specific
> > drivers to support generic uart devices which are enumerated as platform
> > device.
>
> > I can rename 8250_acpi.c to 8250_platform.c if that is better.
>
> No, that's not what I meant. We _already_ have a generic platform driver,
> it's just inline in the 8250_core.c and needs to be extracted. When it's done,
> you may simply add an ACPI table to it.
>
> > Could you please help with a patch even if not compiled so that I can
> > understand your suggestion better?
>
Okay, Thanks! Andy.
IIUC, you want to extract serial8250_isa_driver from 8250_core and
then add ACPI support. I was not sure about that since I thought it was
only for legacy ISA devices and they seem to be created by OS itself
instead of discovery. ISA driver has some ordering dependency on PNP
driver as well. Anyway, let me try if that is the acceptable way
forward.
Thanks,
Sunil
On Wed, May 01 2024 at 17:47, Sunil V L wrote:
> diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> index 9e71c4428814..af7a2f78f0ee 100644
> --- a/drivers/irqchip/irq-riscv-intc.c
> +++ b/drivers/irqchip/irq-riscv-intc.c
> @@ -249,14 +249,105 @@ IRQCHIP_DECLARE(riscv, "riscv,cpu-intc", riscv_intc_init);
> IRQCHIP_DECLARE(andes, "andestech,cpu-intc", riscv_intc_init);
>
> #ifdef CONFIG_ACPI
> +struct rintc_data {
> + u32 ext_intc_id;
> + unsigned long hart_id;
> + u64 imsic_addr;
> + u32 imsic_size;
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers
> +};
> +
> +static u32 nr_rintc;
> +static struct rintc_data *rintc_acpi_data[NR_CPUS];
> +
> +int acpi_get_intc_index_hartid(u32 index, unsigned long *hartid)
Why int? All of these functions have strictly boolean return values:
success = true, fail = false, no?
Either bool or get rid of the pointer and let the function return
either the real hart id or an invalid one.
> +{
> + if (index >= nr_rintc)
> + return -1;
> +
> + *hartid = rintc_acpi_data[index]->hart_id;
> + return 0;
I.e.
return index >= nr_rintc ? rintc_acpi_data[index]->hart_id : INVALID_HART_ID;
> +int acpi_get_ext_intc_parent_hartid(u8 id, u32 idx, unsigned long *hartid)
> +{
> + int i, j = 0;
> +
> + for (i = 0; i < nr_rintc; i++) {
> + if (APLIC_PLIC_ID(rintc_acpi_data[i]->ext_intc_id) == id) {
> + if (idx == j) {
> + *hartid = rintc_acpi_data[i]->hart_id;
> + return 0;
> + }
> + j++;
> + }
> + }
> +
> + return -1;
> +}
> +
> +void acpi_get_plic_nr_contexts(u8 id, int *nr_contexts)
> +{
> + int i, j = 0;
> +
> + for (i = 0; i < nr_rintc; i++) {
> + if (APLIC_PLIC_ID(rintc_acpi_data[i]->ext_intc_id) == id)
> + j++;
> + }
> +
> + *nr_contexts = j;
> +}
> +
> +int acpi_get_plic_context(u8 id, u32 idx, int *context_id)
> +{
> + int i, j = 0;
> +
> + for (i = 0; i < nr_rintc; i++) {
> + if (APLIC_PLIC_ID(rintc_acpi_data[i]->ext_intc_id) == id) {
> + if (idx == j) {
> + *context_id = IDC_CONTEXT_ID(rintc_acpi_data[i]->ext_intc_id);
> + return 0;
> + }
> +
> + j++;
> + }
> + }
So that's the third incarnation of the same loop with the truly self
explaining variable and argument names.
j is actually the index of the context which is associated to a
given PLIC ID.
idx is the context index to search for
Right? So why can't these things be named in a way which makes the
intent of the code clear?
Also why are all the arguments u8/u32? There is no hardware
involved. Simple 'unsigned int' is just fine and the u8/u32 is not bying
you anything here.
Aside of that these ugly macros can be completely avoided and the code
can be written without a copy & pasta orgy.
struct rintc_data {
union {
u32 ext_intc_id;
struct {
u32 context_id : 16,
: 8,
aplic_plic_id : 8;
}
};
unsigned long hart_id;
u64 imsic_addr;
u32 imsic_size;
};
#define for_each_matching_plic(_plic, _plic_id) \
for (_plic = 0; _plic < nr_rintc; _plict++) \
if (rintc_acpi_data[_plic]->aplic_plic_id != _plic_id) \
continue; \
else
unsigned int acpi_get_plic_nr_contexts(unsigned int plic_id)
{
unsigned int nctx = 0;
for_each_matching_plic(plic, plic_id)
nctx++;
return nctx;
}
static struct rintc_data *get_plic_context(unsigned int plic_id, unsigned int ctxt_idx)
{
unsigned int ctxt = 0;
for_each_matching_plic(plic, plic_id) {
if (ctxt == ctxt_idx)
return rintc_acpi_data + plic;
}
return NULL;
}
unsigned long acpi_get_ext_intc_parent_hartid(unsigned int plic_id, unsigned int ctxt_idx)
{
struct rintc_data *data = get_plic_context(plic_id, ctxt_idx);
return data ? data->hart_id : INVALID_HART_ID;
}
unsigned int acpi_get_plic_context(unsigned int plic_id, unsigned int ctxt_idx)
{
struct rintc_data *data = get_plic_context(plic_id, ctxt_idx);
return data ? data->context_id : INVALID_CONTEXT;
}
Or something like that. Hmm?
> +int acpi_get_imsic_mmio_info(u32 index, struct resource *res)
> +{
> + if (index >= nr_rintc)
> + return -1;
> +
> + res->start = rintc_acpi_data[index]->imsic_addr;
> + res->end = res->start + rintc_acpi_data[index]->imsic_size - 1;
> + res->flags = IORESOURCE_MEM;
> + return 0;
> +}
> +
> +static struct fwnode_handle *ext_entc_get_gsi_domain_id(u32 gsi)
> +{
> + return riscv_acpi_get_gsi_domain_id(gsi);
> +}
This wrapper is required because using riscv_acpi_get_gsi_domain_id()
directly is too obvious, right?
> static int __init riscv_intc_acpi_init(union acpi_subtable_headers *header,
> const unsigned long end)
> {
> - struct fwnode_handle *fn;
> struct acpi_madt_rintc *rintc;
> + struct fwnode_handle *fn;
> + int rc;
>
> rintc = (struct acpi_madt_rintc *)header;
> + rintc_acpi_data[nr_rintc] = kzalloc(sizeof(*rintc_acpi_data[0]), GFP_KERNEL);
> + if (!rintc_acpi_data[nr_rintc])
> + return -ENOMEM;
> +
> + rintc_acpi_data[nr_rintc]->ext_intc_id = rintc->ext_intc_id;
> + rintc_acpi_data[nr_rintc]->hart_id = rintc->hart_id;
> + rintc_acpi_data[nr_rintc]->imsic_addr = rintc->imsic_addr;
> + rintc_acpi_data[nr_rintc]->imsic_size = rintc->imsic_size;
> + nr_rintc++;
>
> /*
> * The ACPI MADT will have one INTC for each CPU (or HART)
> @@ -273,7 +364,14 @@ static int __init riscv_intc_acpi_init(union acpi_subtable_headers *header,
> return -ENOMEM;
> }
>
> - return riscv_intc_init_common(fn, &riscv_intc_chip);
> + rc = riscv_intc_init_common(fn, &riscv_intc_chip);
> + if (rc) {
> + irq_domain_free_fwnode(fn);
> + return rc;
> + }
This looks like a completely unrelated bug fix. Please don't mix functional
changes and fixes.
Thanks,
tglx
On Wed, May 01, 2024 at 05:47:32PM +0530, Sunil V L wrote:
> Add the IRQ model for RISC-V INTC so that acpi_set_irq_model can use this
> for RISC-V.
>
> Signed-off-by: Sunil V L <[email protected]>
> ---
> drivers/acpi/bus.c | 3 +++
> include/linux/acpi.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 17ee483c3bf4..6739db258a95 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -1190,6 +1190,9 @@ static int __init acpi_bus_init_irq(void)
> case ACPI_IRQ_MODEL_LPIC:
> message = "LPIC";
> break;
> + case ACPI_IRQ_MODEL_RINTC:
> + message = "RINTC";
> + break;
> default:
> pr_info("Unknown interrupt routing model\n");
> return -ENODEV;
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 846a4001b5e0..c1a01fd02873 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -107,6 +107,7 @@ enum acpi_irq_model_id {
> ACPI_IRQ_MODEL_PLATFORM,
> ACPI_IRQ_MODEL_GIC,
> ACPI_IRQ_MODEL_LPIC,
> + ACPI_IRQ_MODEL_RINTC,
Is the ACPI_IRQ_MODEL_RINTC value documented somewhere? Maybe an ECR
for the ACPI spec?
acpi_bus_init_irq() is going to pass ACPI_IRQ_MODEL_RINTC to _PIC, and
ACPI r6.5, sec 5.8.1 only mentions the ACPI_IRQ_MODEL_PIC,
ACPI_IRQ_MODEL_IOAPIC, and ACPI_IRQ_MODEL_IOSAPIC values.
Even the existing ACPI_IRQ_MODEL_PLATFORM, ACPI_IRQ_MODEL_GIC, and
ACPI_IRQ_MODEL_LPIC values aren't mentioned in ACPI r6.5.
> ACPI_IRQ_MODEL_COUNT
> };
>
> --
> 2.40.1
>
On Wed, May 01 2024 at 17:47, Sunil V L wrote:
> RISC-V IMSIC interrupt controller provides IPI and MSI support.
> Currently, DT based drivers setup the IPI feature early during boot but
> defer setting up the MSI functionality. However, in ACPI systems, ACPI,
> both IPI and MSI features need to be initialized early itself.
Why?
> +
> +#ifdef CONFIG_ACPI
> +
> +static struct fwnode_handle *imsic_acpi_fwnode;
> +
> +struct fwnode_handle *imsic_acpi_get_fwnode(struct device *dev)
Why is this function global? It's only used in the very same file and
under the same CONFIG_ACPI #ifdef, no?
> +{
> + return imsic_acpi_fwnode;
> +}
> +
> +static int __init imsic_early_acpi_init(union acpi_subtable_headers *header,
> + const unsigned long end)
> +{
> + struct acpi_madt_imsic *imsic = (struct acpi_madt_imsic *)header;
> + int rc;
> +
> + imsic_acpi_fwnode = irq_domain_alloc_named_fwnode("imsic");
> + if (!imsic_acpi_fwnode) {
> + pr_err("unable to allocate IMSIC FW node\n");
> + return -ENOMEM;
> + }
> +
> + /* Setup IMSIC state */
> + rc = imsic_setup_state(imsic_acpi_fwnode, (void *)imsic);
Pointless (void *) cast.
> + if (rc) {
> + pr_err("%pfwP: failed to setup state (error %d)\n", imsic_acpi_fwnode, rc);
> + return rc;
> + }
> +
> + /* Do early setup of IMSIC state and IPIs */
> + rc = imsic_early_probe(imsic_acpi_fwnode);
> + if (rc)
> + return rc;
> +
> + rc = imsic_platform_acpi_probe(imsic_acpi_fwnode);
> +
> +#ifdef CONFIG_PCI
> + if (!rc)
> + pci_msi_register_fwnode_provider(&imsic_acpi_get_fwnode);
> +#endif
> +
> + return rc;
Any error return in this function leaks the firmware node and probably
some more stuff.
> +}
> +
> +IRQCHIP_ACPI_DECLARE(riscv_imsic, ACPI_MADT_TYPE_IMSIC, NULL,
> + 1, imsic_early_acpi_init);
> +#endif
..
> - /* Find number of interrupt identities */
> - rc = of_property_read_u32(to_of_node(fwnode), "riscv,num-ids",
> - &global->nr_ids);
> - if (rc) {
> - pr_err("%pfwP: number of interrupt identities not found\n", fwnode);
> - return rc;
> + /* Find number of guest interrupt identities */
> + rc = of_property_read_u32(to_of_node(fwnode), "riscv,num-guest-ids",
> + &global->nr_guest_ids);
> + if (rc)
> + global->nr_guest_ids = global->nr_ids;
> + } else {
> + global->guest_index_bits = imsic->guest_index_bits;
> + global->hart_index_bits = imsic->hart_index_bits;
> + global->group_index_bits = imsic->group_index_bits;
> + global->group_index_shift = imsic->group_index_shift;
> + global->nr_ids = imsic->num_ids;
> + global->nr_guest_ids = imsic->num_guest_ids;
> }
Seriously?
Why can't you just split out the existing DT code into a separate
function in an initial patch which avoulds all of this unreviewable
churn of making the DT stuff indented ?
> +#ifdef CONFIG_ACPI
> +int imsic_platform_acpi_probe(struct fwnode_handle *fwnode);
> +struct fwnode_handle *imsic_acpi_get_fwnode(struct device *dev);
> +#else
> +static inline struct fwnode_handle *imsic_acpi_get_fwnode(struct device *dev)
> +{
> + return NULL;
> +}
> +#endif
Oh well.
> #endif
Thanks,
tglx
On Thu, May 23, 2024 at 04:59:03PM -0500, Bjorn Helgaas wrote:
> On Wed, May 01, 2024 at 05:47:32PM +0530, Sunil V L wrote:
> > Add the IRQ model for RISC-V INTC so that acpi_set_irq_model can use this
> > for RISC-V.
> >
> > Signed-off-by: Sunil V L <[email protected]>
> > ---
> > drivers/acpi/bus.c | 3 +++
> > include/linux/acpi.h | 1 +
> > 2 files changed, 4 insertions(+)
> >
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > index 17ee483c3bf4..6739db258a95 100644
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -1190,6 +1190,9 @@ static int __init acpi_bus_init_irq(void)
> > case ACPI_IRQ_MODEL_LPIC:
> > message = "LPIC";
> > break;
> > + case ACPI_IRQ_MODEL_RINTC:
> > + message = "RINTC";
> > + break;
> > default:
> > pr_info("Unknown interrupt routing model\n");
> > return -ENODEV;
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index 846a4001b5e0..c1a01fd02873 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -107,6 +107,7 @@ enum acpi_irq_model_id {
> > ACPI_IRQ_MODEL_PLATFORM,
> > ACPI_IRQ_MODEL_GIC,
> > ACPI_IRQ_MODEL_LPIC,
> > + ACPI_IRQ_MODEL_RINTC,
>
> Is the ACPI_IRQ_MODEL_RINTC value documented somewhere? Maybe an ECR
> for the ACPI spec?
>
> acpi_bus_init_irq() is going to pass ACPI_IRQ_MODEL_RINTC to _PIC, and
> ACPI r6.5, sec 5.8.1 only mentions the ACPI_IRQ_MODEL_PIC,
> ACPI_IRQ_MODEL_IOAPIC, and ACPI_IRQ_MODEL_IOSAPIC values.
>
> Even the existing ACPI_IRQ_MODEL_PLATFORM, ACPI_IRQ_MODEL_GIC, and
> ACPI_IRQ_MODEL_LPIC values aren't mentioned in ACPI r6.5.
>
Yeah, I also noticed it. I don't know the history behind this. Rafael or
someone else might have better knowledge. IMO, it is better to update
ACPI spec _PIC with all these values. If I don't see any objections, I
will raise an ECR to update the spec.
Thanks,
Sunl
Hi Thomas,
On Thu, May 23, 2024 at 11:47:06PM +0200, Thomas Gleixner wrote:
> On Wed, May 01 2024 at 17:47, Sunil V L wrote:
> > diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> > index 9e71c4428814..af7a2f78f0ee 100644
> > --- a/drivers/irqchip/irq-riscv-intc.c
> > +++ b/drivers/irqchip/irq-riscv-intc.c
> > @@ -249,14 +249,105 @@ IRQCHIP_DECLARE(riscv, "riscv,cpu-intc", riscv_intc_init);
> > IRQCHIP_DECLARE(andes, "andestech,cpu-intc", riscv_intc_init);
> >
> > #ifdef CONFIG_ACPI
> > +struct rintc_data {
> > + u32 ext_intc_id;
> > + unsigned long hart_id;
> > + u64 imsic_addr;
> > + u32 imsic_size;
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers
>
Sure, thanks!
> > +};
> > +
> > +static u32 nr_rintc;
> > +static struct rintc_data *rintc_acpi_data[NR_CPUS];
> > +
> > +int acpi_get_intc_index_hartid(u32 index, unsigned long *hartid)
>
> Why int? All of these functions have strictly boolean return values:
> success = true, fail = false, no?
>
> Either bool or get rid of the pointer and let the function return
> either the real hart id or an invalid one.
>
Sure. I just tried to keep it similar to the parent function. But let me
go with your suggestion in the next revision.
> > +{
> > + if (index >= nr_rintc)
> > + return -1;
> > +
> > + *hartid = rintc_acpi_data[index]->hart_id;
> > + return 0;
>
> I.e.
>
> return index >= nr_rintc ? rintc_acpi_data[index]->hart_id : INVALID_HART_ID;
>
Sure.
> > +int acpi_get_ext_intc_parent_hartid(u8 id, u32 idx, unsigned long *hartid)
> > +{
> > + int i, j = 0;
> > +
> > + for (i = 0; i < nr_rintc; i++) {
> > + if (APLIC_PLIC_ID(rintc_acpi_data[i]->ext_intc_id) == id) {
> > + if (idx == j) {
> > + *hartid = rintc_acpi_data[i]->hart_id;
> > + return 0;
> > + }
> > + j++;
> > + }
> > + }
> > +
> > + return -1;
> > +}
> > +
> > +void acpi_get_plic_nr_contexts(u8 id, int *nr_contexts)
> > +{
> > + int i, j = 0;
> > +
> > + for (i = 0; i < nr_rintc; i++) {
> > + if (APLIC_PLIC_ID(rintc_acpi_data[i]->ext_intc_id) == id)
> > + j++;
> > + }
> > +
> > + *nr_contexts = j;
> > +}
> > +
> > +int acpi_get_plic_context(u8 id, u32 idx, int *context_id)
> > +{
> > + int i, j = 0;
> > +
> > + for (i = 0; i < nr_rintc; i++) {
> > + if (APLIC_PLIC_ID(rintc_acpi_data[i]->ext_intc_id) == id) {
> > + if (idx == j) {
> > + *context_id = IDC_CONTEXT_ID(rintc_acpi_data[i]->ext_intc_id);
> > + return 0;
> > + }
> > +
> > + j++;
> > + }
> > + }
>
> So that's the third incarnation of the same loop with the truly self
> explaining variable and argument names.
>
> j is actually the index of the context which is associated to a
> given PLIC ID.
>
> idx is the context index to search for
>
> Right? So why can't these things be named in a way which makes the
> intent of the code clear?
>
> Also why are all the arguments u8/u32? There is no hardware
> involved. Simple 'unsigned int' is just fine and the u8/u32 is not bying
> you anything here.
>
> Aside of that these ugly macros can be completely avoided and the code
> can be written without a copy & pasta orgy.
>
> struct rintc_data {
> union {
> u32 ext_intc_id;
> struct {
> u32 context_id : 16,
> : 8,
> aplic_plic_id : 8;
> }
> };
> unsigned long hart_id;
> u64 imsic_addr;
> u32 imsic_size;
> };
>
> #define for_each_matching_plic(_plic, _plic_id) \
> for (_plic = 0; _plic < nr_rintc; _plict++) \
> if (rintc_acpi_data[_plic]->aplic_plic_id != _plic_id) \
> continue; \
> else
>
> unsigned int acpi_get_plic_nr_contexts(unsigned int plic_id)
> {
> unsigned int nctx = 0;
>
> for_each_matching_plic(plic, plic_id)
> nctx++;
>
> return nctx;
> }
>
> static struct rintc_data *get_plic_context(unsigned int plic_id, unsigned int ctxt_idx)
> {
> unsigned int ctxt = 0;
>
> for_each_matching_plic(plic, plic_id) {
> if (ctxt == ctxt_idx)
> return rintc_acpi_data + plic;
> }
> return NULL;
> }
>
> unsigned long acpi_get_ext_intc_parent_hartid(unsigned int plic_id, unsigned int ctxt_idx)
> {
> struct rintc_data *data = get_plic_context(plic_id, ctxt_idx);
>
> return data ? data->hart_id : INVALID_HART_ID;
> }
>
> unsigned int acpi_get_plic_context(unsigned int plic_id, unsigned int ctxt_idx)
> {
> struct rintc_data *data = get_plic_context(plic_id, ctxt_idx);
>
> return data ? data->context_id : INVALID_CONTEXT;
> }
>
> Or something like that. Hmm?
>
Nice!. Yes, this is better. Thanks a lot for the suggestion. Let me
update in the next revision.
> > +int acpi_get_imsic_mmio_info(u32 index, struct resource *res)
> > +{
> > + if (index >= nr_rintc)
> > + return -1;
> > +
> > + res->start = rintc_acpi_data[index]->imsic_addr;
> > + res->end = res->start + rintc_acpi_data[index]->imsic_size - 1;
> > + res->flags = IORESOURCE_MEM;
> > + return 0;
> > +}
> > +
> > +static struct fwnode_handle *ext_entc_get_gsi_domain_id(u32 gsi)
> > +{
> > + return riscv_acpi_get_gsi_domain_id(gsi);
> > +}
>
> This wrapper is required because using riscv_acpi_get_gsi_domain_id()
> directly is too obvious, right?
>
:-). Let me remove it.
> > static int __init riscv_intc_acpi_init(union acpi_subtable_headers *header,
> > const unsigned long end)
> > {
> > - struct fwnode_handle *fn;
> > struct acpi_madt_rintc *rintc;
> > + struct fwnode_handle *fn;
> > + int rc;
> >
> > rintc = (struct acpi_madt_rintc *)header;
> > + rintc_acpi_data[nr_rintc] = kzalloc(sizeof(*rintc_acpi_data[0]), GFP_KERNEL);
> > + if (!rintc_acpi_data[nr_rintc])
> > + return -ENOMEM;
> > +
> > + rintc_acpi_data[nr_rintc]->ext_intc_id = rintc->ext_intc_id;
> > + rintc_acpi_data[nr_rintc]->hart_id = rintc->hart_id;
> > + rintc_acpi_data[nr_rintc]->imsic_addr = rintc->imsic_addr;
> > + rintc_acpi_data[nr_rintc]->imsic_size = rintc->imsic_size;
> > + nr_rintc++;
> >
> > /*
> > * The ACPI MADT will have one INTC for each CPU (or HART)
> > @@ -273,7 +364,14 @@ static int __init riscv_intc_acpi_init(union acpi_subtable_headers *header,
> > return -ENOMEM;
> > }
> >
> > - return riscv_intc_init_common(fn, &riscv_intc_chip);
> > + rc = riscv_intc_init_common(fn, &riscv_intc_chip);
> > + if (rc) {
> > + irq_domain_free_fwnode(fn);
> > + return rc;
> > + }
>
> This looks like a completely unrelated bug fix. Please don't mix functional
> changes and fixes.
>
Makes sense. Let me create separate patch.
Thanks a lot for the review!
Sunil
On Fri, May 24, 2024 at 12:00:21AM +0200, Thomas Gleixner wrote:
> On Wed, May 01 2024 at 17:47, Sunil V L wrote:
>
> > RISC-V IMSIC interrupt controller provides IPI and MSI support.
> > Currently, DT based drivers setup the IPI feature early during boot but
> > defer setting up the MSI functionality. However, in ACPI systems, ACPI,
> > both IPI and MSI features need to be initialized early itself.
>
> Why?
>
Sorry, commit message got truncated by mistake. Basically, in ACPI PCI
scan happens very early and there is no concept of msi-parent/dependency
on MSI controller like in DT. It just assumes MSI is setup already. Due
to this, we need to setup MSI controller early as well.
> > +
> > +#ifdef CONFIG_ACPI
> > +
> > +static struct fwnode_handle *imsic_acpi_fwnode;
> > +
> > +struct fwnode_handle *imsic_acpi_get_fwnode(struct device *dev)
>
> Why is this function global? It's only used in the very same file and
> under the same CONFIG_ACPI #ifdef, no?
>
For platform devices using MSIs, we need a way to determine the MSI
domain. This function is exported so that platform device like
APLIC/IOMMU can find the MSI irqdomain.
For PCI, pci_msi_register_fwnode_provider() is registered by the MSI
driver for this purpose.
Let me know if this can be made better.
> > +{
> > + return imsic_acpi_fwnode;
> > +}
> > +
> > +static int __init imsic_early_acpi_init(union acpi_subtable_headers *header,
> > + const unsigned long end)
> > +{
> > + struct acpi_madt_imsic *imsic = (struct acpi_madt_imsic *)header;
> > + int rc;
> > +
> > + imsic_acpi_fwnode = irq_domain_alloc_named_fwnode("imsic");
> > + if (!imsic_acpi_fwnode) {
> > + pr_err("unable to allocate IMSIC FW node\n");
> > + return -ENOMEM;
> > + }
> > +
> > + /* Setup IMSIC state */
> > + rc = imsic_setup_state(imsic_acpi_fwnode, (void *)imsic);
>
> Pointless (void *) cast.
>
Okay.
> > + if (rc) {
> > + pr_err("%pfwP: failed to setup state (error %d)\n", imsic_acpi_fwnode, rc);
> > + return rc;
> > + }
> > +
> > + /* Do early setup of IMSIC state and IPIs */
> > + rc = imsic_early_probe(imsic_acpi_fwnode);
> > + if (rc)
> > + return rc;
> > +
> > + rc = imsic_platform_acpi_probe(imsic_acpi_fwnode);
> > +
> > +#ifdef CONFIG_PCI
> > + if (!rc)
> > + pci_msi_register_fwnode_provider(&imsic_acpi_get_fwnode);
> > +#endif
> > +
> > + return rc;
>
> Any error return in this function leaks the firmware node and probably
> some more stuff.
>
Yeah, fwnode needs free up and need to update the code a bit. Thanks!
> > +}
> > +
> > +IRQCHIP_ACPI_DECLARE(riscv_imsic, ACPI_MADT_TYPE_IMSIC, NULL,
> > + 1, imsic_early_acpi_init);
> > +#endif
>
> ...
>
> > - /* Find number of interrupt identities */
> > - rc = of_property_read_u32(to_of_node(fwnode), "riscv,num-ids",
> > - &global->nr_ids);
> > - if (rc) {
> > - pr_err("%pfwP: number of interrupt identities not found\n", fwnode);
> > - return rc;
> > + /* Find number of guest interrupt identities */
> > + rc = of_property_read_u32(to_of_node(fwnode), "riscv,num-guest-ids",
> > + &global->nr_guest_ids);
> > + if (rc)
> > + global->nr_guest_ids = global->nr_ids;
> > + } else {
> > + global->guest_index_bits = imsic->guest_index_bits;
> > + global->hart_index_bits = imsic->hart_index_bits;
> > + global->group_index_bits = imsic->group_index_bits;
> > + global->group_index_shift = imsic->group_index_shift;
> > + global->nr_ids = imsic->num_ids;
> > + global->nr_guest_ids = imsic->num_guest_ids;
> > }
>
> Seriously?
>
> Why can't you just split out the existing DT code into a separate
> function in an initial patch which avoulds all of this unreviewable
> churn of making the DT stuff indented ?
>
Sure, makes sense. let me create separate patch first as you suggested.
> > +#ifdef CONFIG_ACPI
> > +int imsic_platform_acpi_probe(struct fwnode_handle *fwnode);
> > +struct fwnode_handle *imsic_acpi_get_fwnode(struct device *dev);
> > +#else
> > +static inline struct fwnode_handle *imsic_acpi_get_fwnode(struct device *dev)
> > +{
> > + return NULL;
> > +}
> > +#endif
>
> Oh well.
>
I guess this is related to your prior comment about the need to make
this public function. Let me know if I am missing something.
Thanks!
Sunil