This series adds support for the below ECRs approved by ASWG recently.
1) MADT - https://drive.google.com/file/d/1oMGPyOD58JaPgMl1pKasT-VKsIKia7zR/view?usp=sharing
2) RHCT - https://drive.google.com/file/d/1sKbOa8m1UZw1JkquZYe3F1zQBN1xXsaf/view?usp=sharing
The series primarily adds below features.
1) ACPI support for external interrupt controller drivers (IMSIC, APLIC and PLIC).
2) Get CBO block sizes from RHCT.
3) Set timer_can_not_wakeup in timer driver based on the flag in RHCT.
PCI ACPI related functions are migrated from arm64 to common file
so that we don't need to duplicate them for RISC-V.
It uses software node framework to create the fwnode for the interrupt
controllers. This helps in keeping the actual drivers code mostly common
for DT and ACPI.
This series is based on Anup's AIA v7 series. The first 2 ACPICA
patches in this series will be merged via ACPICA release process. PATCH3 is a
fix patch. These patches are included in this series only to enable build.
To test the series,
1) Qemu should be built using the riscv_acpi_b2_v1_plic 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
3) Build Linux using this series on top of Anup's AIA v7 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 riscv_acpi_b2_v1 brach at
https://github.com/vlsunil/linux.git
Based-on: [email protected]
(https://lore.kernel.org/lkml/[email protected]/)
Anup Patel (1):
swnode: Add support to create early during boot
Sunil V L (20):
ACPICA: MADT: Add RISC-V external interrupt controllers
ACPICA: RHCT: Add flags, CMO and MMU nodes
RISC-V: ACPI: Fix acpi_os_ioremap to return iomem address
RISC-V: ACPI: Enhance acpi_os_ioremap with MMIO remapping
arm64: PCI: Migrate ACPI related functions to pci-acpi.c
RISC-V: ACPI: Implement PCI related functionality
RISC-V: Kconfig: Select ECAM and MCFG
RISC-V: ACPI: RHCT: Add function to get CBO block sizes
RISC-V: cacheflush: Initialize CBO variables on ACPI systems
clocksource/timer-riscv: ACPI: Add timer_cannot_wakeup_cpu
irqchip/riscv-intc: Use swnode framework to create fwnode
irqchip/riscv-imsic-early: Add ACPI support
ACPI: bus: Add acpi_riscv_init function
ACPI: RISC-V: Create IMSIC platform device
ACPI: Add APLIC IRQ model for RISC-V
ACPI: RISC-V: Create APLIC platform device
irqchip/irq-riscv-aplic-msi: Add ACPI support
ACPI: bus: Add PLIC IRQ model
RISC-V: ACPI: Create PLIC platform device
irqchip/sifive-plic: Add GSI conversion support
Documentation/riscv/acpi.rst | 33 ++
arch/arm64/kernel/pci.c | 193 ---------
arch/riscv/Kconfig | 3 +
arch/riscv/include/asm/acpi.h | 21 +-
arch/riscv/kernel/acpi.c | 120 +++++-
arch/riscv/mm/cacheflush.c | 37 +-
drivers/acpi/bus.c | 7 +
drivers/acpi/riscv/Makefile | 2 +-
drivers/acpi/riscv/init.c | 16 +
drivers/acpi/riscv/init.h | 6 +
drivers/acpi/riscv/irqchip.c | 507 ++++++++++++++++++++++++
drivers/acpi/riscv/rhct.c | 61 +++
drivers/base/swnode.c | 117 +++++-
drivers/clocksource/timer-riscv.c | 4 +
drivers/irqchip/irq-riscv-aplic-msi.c | 14 +-
drivers/irqchip/irq-riscv-imsic-early.c | 28 ++
drivers/irqchip/irq-riscv-imsic-state.c | 33 +-
drivers/irqchip/irq-riscv-intc.c | 12 +-
drivers/irqchip/irq-sifive-plic.c | 16 +
drivers/pci/pci-acpi.c | 182 +++++++++
include/acpi/actbl2.h | 76 +++-
include/linux/acpi.h | 8 +
include/linux/property.h | 3 +
23 files changed, 1248 insertions(+), 251 deletions(-)
create mode 100644 drivers/acpi/riscv/init.c
create mode 100644 drivers/acpi/riscv/init.h
create mode 100644 drivers/acpi/riscv/irqchip.c
--
2.39.2
By using swnode framework, all data from ACPI tables can
be populated as properties of the swnode. This simplifies
the driver code and removes the need for ACPI vs DT checks.
Use this framework for RISC-V INTC driver.
Signed-off-by: Sunil V L <[email protected]>
---
Documentation/riscv/acpi.rst | 21 +++++++++++++++
arch/riscv/include/asm/acpi.h | 1 +
drivers/acpi/riscv/Makefile | 2 +-
drivers/acpi/riscv/irqchip.c | 46 ++++++++++++++++++++++++++++++++
drivers/irqchip/irq-riscv-intc.c | 12 ++++-----
5 files changed, 75 insertions(+), 7 deletions(-)
create mode 100644 drivers/acpi/riscv/irqchip.c
diff --git a/Documentation/riscv/acpi.rst b/Documentation/riscv/acpi.rst
index 9870a282815b..e2406546bc16 100644
--- a/Documentation/riscv/acpi.rst
+++ b/Documentation/riscv/acpi.rst
@@ -8,3 +8,24 @@ The ISA string parsing rules for ACPI are defined by `Version ASCIIDOC
Conversion, 12/2022 of the RISC-V specifications, as defined by tag
"riscv-isa-release-1239329-2023-05-23" (commit 1239329
) <https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-isa-release-1239329-2023-05-23>`_
+
+Interrupt Controller Drivers
+=======
+
+ACPI drivers for RISC-V interrupt controllers use software node framework to
+create the fwnode for the interrupt controllers. Below properties are
+additionally required for some firmware nodes apart from the properties
+defined by the device tree bindings for these interrupt controllers. The
+properties are created using the data in MADT table.
+
+1) RISC-V Interrupt Controller (INTC)
+-----------
+``hartid`` - Hart ID of the hart this interrupt controller belongs to.
+
+``riscv,imsic-addr`` - Physical base address of the Incoming MSI Controller
+(IMSIC) MMIO region of this hart.
+
+``riscv,imsic-size`` - Size in bytes of the IMSIC MMIO region of this hart.
+
+``riscv,ext-intc-id`` - The unique ID of the external interrupts connected
+to this hart.
diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h
index 0c4e8b35103e..0ac2df2dd194 100644
--- a/arch/riscv/include/asm/acpi.h
+++ b/arch/riscv/include/asm/acpi.h
@@ -68,6 +68,7 @@ int acpi_get_riscv_isa(struct acpi_table_header *table,
static inline int acpi_numa_get_nid(unsigned int cpu) { return NUMA_NO_NODE; }
int acpi_get_cbo_block_size(struct acpi_table_header *table, unsigned int cpu, u32 *cbom_size,
u32 *cboz_size, u32 *cbop_size);
+struct fwnode_handle *acpi_rintc_create_irqchip_fwnode(struct acpi_madt_rintc *rintc);
#else
static inline void acpi_init_rintc_map(void) { }
static inline struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu)
diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile
index 8b3b126e0b94..8b664190d172 100644
--- a/drivers/acpi/riscv/Makefile
+++ b/drivers/acpi/riscv/Makefile
@@ -1,2 +1,2 @@
# SPDX-License-Identifier: GPL-2.0-only
-obj-y += rhct.o
+obj-y += rhct.o irqchip.o
diff --git a/drivers/acpi/riscv/irqchip.c b/drivers/acpi/riscv/irqchip.c
new file mode 100644
index 000000000000..36f066a2cad5
--- /dev/null
+++ b/drivers/acpi/riscv/irqchip.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023, Ventana Micro Systems Inc
+ * Author: Sunil V L <[email protected]>
+ *
+ */
+
+#include <linux/acpi.h>
+#include <linux/fwnode.h>
+#include <linux/irqdomain.h>
+#include <linux/list.h>
+#include <linux/property.h>
+
+struct riscv_irqchip_list {
+ struct fwnode_handle *fwnode;
+ struct list_head list;
+};
+
+LIST_HEAD(rintc_list);
+
+struct fwnode_handle *acpi_rintc_create_irqchip_fwnode(struct acpi_madt_rintc *rintc)
+{
+ struct property_entry props[6] = {};
+ struct fwnode_handle *fwnode;
+ struct riscv_irqchip_list *rintc_element;
+
+ props[0] = PROPERTY_ENTRY_U64("hartid", rintc->hart_id);
+ props[1] = PROPERTY_ENTRY_U32("riscv,ext-intc-id", rintc->ext_intc_id);
+ props[2] = PROPERTY_ENTRY_U64("riscv,imsic-addr", rintc->imsic_addr);
+ props[3] = PROPERTY_ENTRY_U32("riscv,imsic-size", rintc->imsic_size);
+ props[4] = PROPERTY_ENTRY_U32("#interrupt-cells", 1);
+
+ fwnode = fwnode_create_software_node_early(props, NULL);
+ if (fwnode) {
+ rintc_element = kzalloc(sizeof(*rintc_element), GFP_KERNEL);
+ if (!rintc_element) {
+ fwnode_remove_software_node(fwnode);
+ return NULL;
+ }
+
+ rintc_element->fwnode = fwnode;
+ list_add_tail(&rintc_element->list, &rintc_list);
+ }
+
+ return fwnode;
+}
diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
index 1a0fc87152c5..1ef9cada1ed3 100644
--- a/drivers/irqchip/irq-riscv-intc.c
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -203,6 +203,12 @@ static int __init riscv_intc_acpi_init(union acpi_subtable_headers *header,
rintc = (struct acpi_madt_rintc *)header;
+ fn = acpi_rintc_create_irqchip_fwnode(rintc);
+ if (!fn) {
+ pr_err("unable to create INTC FW node\n");
+ return -ENOMEM;
+ }
+
/*
* The ACPI MADT will have one INTC for each CPU (or HART)
* so riscv_intc_acpi_init() function will be called once
@@ -212,12 +218,6 @@ static int __init riscv_intc_acpi_init(union acpi_subtable_headers *header,
if (riscv_hartid_to_cpuid(rintc->hart_id) != smp_processor_id())
return 0;
- fn = irq_domain_alloc_named_fwnode("RISCV-INTC");
- if (!fn) {
- pr_err("unable to allocate INTC FW node\n");
- return -ENOMEM;
- }
-
return riscv_intc_init_common(fn);
}
--
2.39.2
From: Anup Patel <[email protected]>
swnode framework can be used to create fwnode for interrupt
controllers. This helps in keeping the drivers same for both
DT and ACPI. To enable this, enhance the swnode framework so
that it can be created early during boot without dependency
on sysfs.
Signed-off-by: Anup Patel <[email protected]>
Co-developed-by: Sunil V L <[email protected]>
Signed-off-by: Sunil V L <[email protected]>
---
drivers/base/swnode.c | 117 +++++++++++++++++++++++++++++++++------
include/linux/property.h | 3 +
2 files changed, 103 insertions(+), 17 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 1886995a0b3a..43f191a38980 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -10,6 +10,7 @@
#include <linux/kernel.h>
#include <linux/property.h>
#include <linux/slab.h>
+#include <linux/spinlock.h>
#include "base.h"
@@ -21,6 +22,7 @@ struct swnode {
/* hierarchy */
struct ida child_ids;
+ struct list_head early;
struct list_head entry;
struct list_head children;
struct swnode *parent;
@@ -32,6 +34,9 @@ struct swnode {
static DEFINE_IDA(swnode_root_ids);
static struct kset *swnode_kset;
+static DEFINE_SPINLOCK(swnode_early_lock);
+static LIST_HEAD(swnode_early_list);
+
#define kobj_to_swnode(_kobj_) container_of(_kobj_, struct swnode, kobj)
static const struct fwnode_operations software_node_ops;
@@ -73,6 +78,17 @@ software_node_to_swnode(const struct software_node *node)
if (!node)
return NULL;
+ spin_lock(&swnode_early_lock);
+
+ list_for_each_entry(swnode, &swnode_early_list, early) {
+ if (swnode->node == node) {
+ spin_unlock(&swnode_early_lock);
+ return swnode;
+ }
+ }
+
+ spin_unlock(&swnode_early_lock);
+
spin_lock(&swnode_kset->list_lock);
list_for_each_entry(k, &swnode_kset->list, entry) {
@@ -698,6 +714,19 @@ software_node_find_by_name(const struct software_node *parent, const char *name)
if (!name)
return NULL;
+ spin_lock(&swnode_early_lock);
+
+ list_for_each_entry(swnode, &swnode_early_list, early) {
+ if (parent == swnode->node->parent && swnode->node->name &&
+ !strcmp(name, swnode->node->name)) {
+ kobject_get(&swnode->kobj);
+ spin_unlock(&swnode_early_lock);
+ return swnode->node;
+ }
+ }
+
+ spin_unlock(&swnode_early_lock);
+
spin_lock(&swnode_kset->list_lock);
list_for_each_entry(k, &swnode_kset->list, entry) {
@@ -742,10 +771,16 @@ static void software_node_free(const struct software_node *node)
kfree(node);
}
-static void software_node_release(struct kobject *kobj)
+static void software_node_release_common(struct kobject *kobj, bool early)
{
struct swnode *swnode = kobj_to_swnode(kobj);
+ if (early) {
+ spin_lock(&swnode_early_lock);
+ list_del(&swnode->early);
+ spin_unlock(&swnode_early_lock);
+ }
+
if (swnode->parent) {
ida_simple_remove(&swnode->parent->child_ids, swnode->id);
list_del(&swnode->entry);
@@ -760,6 +795,20 @@ static void software_node_release(struct kobject *kobj)
kfree(swnode);
}
+static void software_node_release(struct kobject *kobj)
+{
+ software_node_release_common(kobj, false);
+}
+
+static void software_node_release_early(struct kobject *kobj)
+{
+ software_node_release_common(kobj, true);
+}
+
+static const struct kobj_type software_node_type_early = {
+ .release = software_node_release_early
+};
+
static const struct kobj_type software_node_type = {
.release = software_node_release,
.sysfs_ops = &kobj_sysfs_ops,
@@ -767,7 +816,7 @@ static const struct kobj_type software_node_type = {
static struct fwnode_handle *
swnode_register(const struct software_node *node, struct swnode *parent,
- unsigned int allocated)
+ unsigned int allocated, unsigned int early)
{
struct swnode *swnode;
int ret;
@@ -786,21 +835,39 @@ swnode_register(const struct software_node *node, struct swnode *parent,
swnode->id = ret;
swnode->node = node;
swnode->parent = parent;
- swnode->kobj.kset = swnode_kset;
+ swnode->kobj.kset = (!early) ? swnode_kset : NULL;
fwnode_init(&swnode->fwnode, &software_node_ops);
ida_init(&swnode->child_ids);
+ INIT_LIST_HEAD(&swnode->early);
INIT_LIST_HEAD(&swnode->entry);
INIT_LIST_HEAD(&swnode->children);
- if (node->name)
- ret = kobject_init_and_add(&swnode->kobj, &software_node_type,
- parent ? &parent->kobj : NULL,
- "%s", node->name);
- else
- ret = kobject_init_and_add(&swnode->kobj, &software_node_type,
- parent ? &parent->kobj : NULL,
- "node%d", swnode->id);
+ if (early) {
+ ret = 0;
+ kobject_init(&swnode->kobj, &software_node_type_early);
+ swnode->kobj.parent = parent ? &parent->kobj : NULL;
+ if (node->name)
+ ret = kobject_set_name(&swnode->kobj,
+ "%s", node->name);
+ else
+ ret = kobject_set_name(&swnode->kobj,
+ "node%d", swnode->id);
+ if (!ret) {
+ spin_lock(&swnode_early_lock);
+ list_add_tail(&swnode->early, &swnode_early_list);
+ spin_unlock(&swnode_early_lock);
+ }
+ } else {
+ if (node->name)
+ ret = kobject_init_and_add(&swnode->kobj, &software_node_type,
+ parent ? &parent->kobj : NULL,
+ "%s", node->name);
+ else
+ ret = kobject_init_and_add(&swnode->kobj, &software_node_type,
+ parent ? &parent->kobj : NULL,
+ "node%d", swnode->id);
+ }
if (ret) {
kobject_put(&swnode->kobj);
return ERR_PTR(ret);
@@ -815,7 +882,8 @@ swnode_register(const struct software_node *node, struct swnode *parent,
if (parent)
list_add_tail(&swnode->entry, &parent->children);
- kobject_uevent(&swnode->kobj, KOBJ_ADD);
+ if (!early)
+ kobject_uevent(&swnode->kobj, KOBJ_ADD);
return &swnode->fwnode;
}
@@ -892,7 +960,7 @@ int software_node_register(const struct software_node *node)
if (node->parent && !parent)
return -EINVAL;
- return PTR_ERR_OR_ZERO(swnode_register(node, parent, 0));
+ return PTR_ERR_OR_ZERO(swnode_register(node, parent, 0, 0));
}
EXPORT_SYMBOL_GPL(software_node_register);
@@ -910,9 +978,10 @@ void software_node_unregister(const struct software_node *node)
}
EXPORT_SYMBOL_GPL(software_node_unregister);
-struct fwnode_handle *
-fwnode_create_software_node(const struct property_entry *properties,
- const struct fwnode_handle *parent)
+static struct fwnode_handle *
+fwnode_create_software_node_common(const struct property_entry *properties,
+ const struct fwnode_handle *parent,
+ bool early)
{
struct fwnode_handle *fwnode;
struct software_node *node;
@@ -931,12 +1000,26 @@ fwnode_create_software_node(const struct property_entry *properties,
node->parent = p ? p->node : NULL;
- fwnode = swnode_register(node, p, 1);
+ fwnode = swnode_register(node, p, 1, early);
if (IS_ERR(fwnode))
software_node_free(node);
return fwnode;
}
+
+struct fwnode_handle *
+fwnode_create_software_node_early(const struct property_entry *properties,
+ const struct fwnode_handle *parent)
+{
+ return fwnode_create_software_node_common(properties, parent, true);
+}
+
+struct fwnode_handle *
+fwnode_create_software_node(const struct property_entry *properties,
+ const struct fwnode_handle *parent)
+{
+ return fwnode_create_software_node_common(properties, parent, false);
+}
EXPORT_SYMBOL_GPL(fwnode_create_software_node);
void fwnode_remove_software_node(struct fwnode_handle *fwnode)
diff --git a/include/linux/property.h b/include/linux/property.h
index 8c3c6685a2ae..7137338bfabb 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -503,6 +503,9 @@ void software_node_unregister_node_group(const struct software_node **node_group
int software_node_register(const struct software_node *node);
void software_node_unregister(const struct software_node *node);
+struct fwnode_handle *
+fwnode_create_software_node_early(const struct property_entry *properties,
+ const struct fwnode_handle *parent);
struct fwnode_handle *
fwnode_create_software_node(const struct property_entry *properties,
const struct fwnode_handle *parent);
--
2.39.2
ACPICA commit 2eded5a6a13d892b7dc3be6096e7b1e8d4407600
Update RHCT table with below details.
1) Add additional structure to describe the Cache Management
Operation (CMO) related information.
2) Add structure to describe MMU type.
3) Convert the current reserved field to flags and define
a flag to indicate timer capability.
This codefirst ECR is approved by UEFI forum and will
be part of next ACPI spec version.
Link: https://github.com/acpica/acpica/commit/2eded5a6
Signed-off-by: Sunil V L <[email protected]>
Signed-off-by: Bob Moore <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
include/acpi/actbl2.h | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 280ab4c7f77a..3751ae69432f 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -2778,12 +2778,15 @@ enum acpi_rgrt_image_type {
struct acpi_table_rhct {
struct acpi_table_header header; /* Common ACPI table header */
- u32 reserved;
+ u32 flags; /* RHCT flags */
u64 time_base_freq;
u32 node_count;
u32 node_offset;
};
+/* RHCT Flags */
+
+#define ACPI_RHCT_TIMER_CANNOT_WAKEUP_CPU (1)
/*
* RHCT subtables
*/
@@ -2797,6 +2800,9 @@ struct acpi_rhct_node_header {
enum acpi_rhct_node_type {
ACPI_RHCT_NODE_TYPE_ISA_STRING = 0x0000,
+ ACPI_RHCT_NODE_TYPE_CMO = 0x0001,
+ ACPI_RHCT_NODE_TYPE_MMU = 0x0002,
+ ACPI_RHCT_NODE_TYPE_RESERVED = 0x0003,
ACPI_RHCT_NODE_TYPE_HART_INFO = 0xFFFF,
};
@@ -2810,6 +2816,24 @@ struct acpi_rhct_isa_string {
char isa[];
};
+struct acpi_rhct_cmo_node {
+ u8 reserved; /* Must be zero */
+ u8 cbom_size; /* CBOM size in powerof 2 */
+ u8 cbop_size; /* CBOP size in powerof 2 */
+ u8 cboz_size; /* CBOZ size in powerof 2 */
+};
+
+struct acpi_rhct_mmu_node {
+ u8 reserved; /* Must be zero */
+ u8 mmu_type; /* Virtual Address Scheme */
+};
+
+enum acpi_rhct_mmu_type {
+ ACPI_RHCT_MMU_TYPE_SV39 = 0,
+ ACPI_RHCT_MMU_TYPE_SV48 = 1,
+ ACPI_RHCT_MMU_TYPE_SV57 = 2
+};
+
/* Hart Info node structure */
struct acpi_rhct_hart_info {
u16 num_offsets;
--
2.39.2
ACPICA commit 8c048cee4ea7b9ded8db3e1b3b9c14e21e084a2c
This adds 3 different external interrupt controller
definitions in MADT for RISC-V.
1) RISC-V PLIC is a platform interrupt controller for
handling wired interrupt in a RISC-V systems.
2) RISC-V IMSIC is MSI interrupt controller to
support MSI interrupts.
3) RISC-V APLIC has dual functionality. First it can
act like PLIC and direct all wired interrupts to
the CPU which doesn't have MSI controller. Second,
when the CPU has MSI controller (IMSIC), it will
act as a converter from wired interrupts to MSI.
Update the existing RINTC structure also to support
these external interrupt controllers.
This codefirst ECR is approved by UEFI forum and will
be part of next ACPI spec version.
Link: https://github.com/acpica/acpica/commit/8c048cee
Co-developed-by: Haibo Xu <[email protected]>
Signed-off-by: Haibo Xu <[email protected]>
Signed-off-by: Sunil V L <[email protected]>
Signed-off-by: Bob Moore <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
include/acpi/actbl2.h | 50 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 0029336775a9..280ab4c7f77a 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -893,7 +893,10 @@ enum acpi_madt_type {
ACPI_MADT_TYPE_BIO_PIC = 22,
ACPI_MADT_TYPE_LPC_PIC = 23,
ACPI_MADT_TYPE_RINTC = 24,
- ACPI_MADT_TYPE_RESERVED = 25, /* 25 to 0x7F are reserved */
+ ACPI_MADT_TYPE_IMSIC = 25,
+ ACPI_MADT_TYPE_APLIC = 26,
+ ACPI_MADT_TYPE_PLIC = 27,
+ ACPI_MADT_TYPE_RESERVED = 28, /* 28 to 0x7F are reserved */
ACPI_MADT_TYPE_OEM_RESERVED = 0x80 /* 0x80 to 0xFF are reserved for OEM use */
};
@@ -1261,6 +1264,9 @@ struct acpi_madt_rintc {
u32 flags;
u64 hart_id;
u32 uid; /* ACPI processor UID */
+ u32 ext_intc_id; /* External INTC Id */
+ u64 imsic_addr; /* IMSIC base address */
+ u32 imsic_size; /* IMSIC size */
};
/* Values for RISC-V INTC Version field above */
@@ -1271,6 +1277,48 @@ enum acpi_madt_rintc_version {
ACPI_MADT_RINTC_VERSION_RESERVED = 2 /* 2 and greater are reserved */
};
+/* 25: RISC-V IMSIC */
+struct acpi_madt_imsic {
+ struct acpi_subtable_header header;
+ u8 version;
+ u8 reserved;
+ u32 flags;
+ u16 num_ids;
+ u16 num_guest_ids;
+ u8 guest_index_bits;
+ u8 hart_index_bits;
+ u8 group_index_bits;
+ u8 group_index_shift;
+};
+
+/* 26: RISC-V APLIC */
+struct acpi_madt_aplic {
+ struct acpi_subtable_header header;
+ u8 version;
+ u8 id;
+ u32 flags;
+ u8 hw_id[8];
+ u16 num_idcs;
+ u16 num_sources;
+ u32 gsi_base;
+ u64 base_addr;
+ u32 size;
+};
+
+/* 27: RISC-V PLIC */
+struct acpi_madt_plic {
+ struct acpi_subtable_header header;
+ u8 version;
+ u8 id;
+ u8 hw_id[8];
+ u16 num_irqs;
+ u16 max_prio;
+ u32 flags;
+ u32 size;
+ u64 base_addr;
+ u32 gsi_base;
+};
+
/* 80: OEM data */
struct acpi_madt_oem_data {
--
2.39.2
Add the IRQ model for RISC-V PLIC so that acpi_set_irq_model
can use this for RISC-V.
Signed-off-by: Haibo Xu <[email protected]>
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 dd9d80630bf6..68b8d38ec48a 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1135,6 +1135,9 @@ static int __init acpi_bus_init_irq(void)
case ACPI_IRQ_MODEL_APLIC:
message = "APLIC";
break;
+ case ACPI_IRQ_MODEL_PLIC:
+ message = "PLIC";
+ break;
default:
pr_info("Unknown interrupt routing model\n");
return -ENODEV;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 698d120a1bd2..31f080df179d 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -96,6 +96,7 @@ enum acpi_irq_model_id {
ACPI_IRQ_MODEL_GIC,
ACPI_IRQ_MODEL_LPIC,
ACPI_IRQ_MODEL_APLIC,
+ ACPI_IRQ_MODEL_PLIC,
ACPI_IRQ_MODEL_COUNT
};
--
2.39.2
acpi_os_ioremap() currently is a wrapper to memremap() on
RISC-V. But the callers of acpi_os_ioremap() expect it to
return __iomem address and hence sparse tool reports a new
warning. Fix this issue by type casting to __iomem type.
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Fixes: a91a9ffbd3a5 ("RISC-V: Add support to build the ACPI core")
Signed-off-by: Sunil V L <[email protected]>
---
arch/riscv/include/asm/acpi.h | 2 +-
arch/riscv/kernel/acpi.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h
index f71ce21ff684..d5604d2073bc 100644
--- a/arch/riscv/include/asm/acpi.h
+++ b/arch/riscv/include/asm/acpi.h
@@ -19,7 +19,7 @@ typedef u64 phys_cpuid_t;
#define PHYS_CPUID_INVALID INVALID_HARTID
/* ACPI table mapping after acpi_permanent_mmap is set */
-void *acpi_os_ioremap(acpi_physical_address phys, acpi_size size);
+void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size);
#define acpi_os_ioremap acpi_os_ioremap
#define acpi_strict 1 /* No out-of-spec workarounds on RISC-V */
diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c
index 5ee03ebab80e..56cb2c986c48 100644
--- a/arch/riscv/kernel/acpi.c
+++ b/arch/riscv/kernel/acpi.c
@@ -215,9 +215,9 @@ void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
early_iounmap(map, size);
}
-void *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
+void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
{
- return memremap(phys, size, MEMREMAP_WB);
+ return (void __iomem *)memremap(phys, size, MEMREMAP_WB);
}
#ifdef CONFIG_PCI
--
2.39.2
Add a new function for RISC-V to do any architecture
specific initialization. This function will be used
to create platform devices like APLIC, IMSIC MSI controller,
RISC-V IOMMU etc.
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 2fc2b43a4ed3..9a8c16170a4b 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1388,6 +1388,7 @@ static int __init acpi_init(void)
pci_mmcfg_late_init();
acpi_arm_init();
+ acpi_riscv_init();
acpi_viot_early_init();
acpi_hest_init();
acpi_ghes_init();
diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile
index 8b664190d172..3433a19c421d 100644
--- a/drivers/acpi/riscv/Makefile
+++ b/drivers/acpi/riscv/Makefile
@@ -1,2 +1,2 @@
# SPDX-License-Identifier: GPL-2.0-only
-obj-y += rhct.o irqchip.o
+obj-y += rhct.o irqchip.o init.o
diff --git a/drivers/acpi/riscv/init.c b/drivers/acpi/riscv/init.c
new file mode 100644
index 000000000000..b5807bbdb171
--- /dev/null
+++ b/drivers/acpi/riscv/init.c
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023, 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 641dc4843987..d16739928888 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1511,6 +1511,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.39.2
Using new interface to get the CBO block size information in
RHCT, initialize the variables on ACPI platforms.
Signed-off-by: Sunil V L <[email protected]>
---
arch/riscv/mm/cacheflush.c | 37 +++++++++++++++++++++++++++++++------
1 file changed, 31 insertions(+), 6 deletions(-)
diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
index fbc59b3f69f2..63bb56819b37 100644
--- a/arch/riscv/mm/cacheflush.c
+++ b/arch/riscv/mm/cacheflush.c
@@ -4,6 +4,8 @@
*/
#include <linux/of.h>
+#include <linux/acpi.h>
+#include <asm/acpi.h>
#include <asm/cacheflush.h>
#ifdef CONFIG_SMP
@@ -131,15 +133,38 @@ void __init riscv_init_cbo_blocksizes(void)
unsigned long cbom_hartid, cboz_hartid;
u32 cbom_block_size = 0, cboz_block_size = 0;
struct device_node *node;
+ struct acpi_table_header *rhct;
+ acpi_status status;
+ unsigned int cpu;
+
+ if (!acpi_disabled) {
+ status = acpi_get_table(ACPI_SIG_RHCT, 0, &rhct);
+ if (ACPI_FAILURE(status))
+ return;
+ }
- for_each_of_cpu_node(node) {
- /* set block-size for cbom and/or cboz extension if available */
- cbo_get_block_size(node, "riscv,cbom-block-size",
- &cbom_block_size, &cbom_hartid);
- cbo_get_block_size(node, "riscv,cboz-block-size",
- &cboz_block_size, &cboz_hartid);
+ for_each_possible_cpu(cpu) {
+ if (acpi_disabled) {
+ node = of_cpu_device_node_get(cpu);
+ if (!node) {
+ pr_warn("Unable to find cpu node\n");
+ continue;
+ }
+
+ /* set block-size for cbom and/or cboz extension if available */
+ cbo_get_block_size(node, "riscv,cbom-block-size",
+ &cbom_block_size, &cbom_hartid);
+ cbo_get_block_size(node, "riscv,cboz-block-size",
+ &cboz_block_size, &cboz_hartid);
+ } else {
+ acpi_get_cbo_block_size(rhct, cpu, &cbom_block_size,
+ &cboz_block_size, NULL);
+ }
}
+ if (!acpi_disabled && rhct)
+ acpi_put_table((struct acpi_table_header *)rhct);
+
if (cbom_block_size)
riscv_cbom_block_size = cbom_block_size;
--
2.39.2
The MSI controller functionality of the RISC-V IMSIC is
probed as a platform device by the driver. So, create the
IMSIC platform device if the IMSIC was discovered in MADT
during early IMSIC driver probe.
Signed-off-by: Sunil V L <[email protected]>
---
drivers/acpi/riscv/init.c | 2 ++
drivers/acpi/riscv/init.h | 4 ++++
drivers/acpi/riscv/irqchip.c | 23 +++++++++++++++++++++++
3 files changed, 29 insertions(+)
create mode 100644 drivers/acpi/riscv/init.h
diff --git a/drivers/acpi/riscv/init.c b/drivers/acpi/riscv/init.c
index b5807bbdb171..be61c08ea385 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_imsic_platform_init();
}
diff --git a/drivers/acpi/riscv/init.h b/drivers/acpi/riscv/init.h
new file mode 100644
index 000000000000..a2f72bb294d3
--- /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_imsic_platform_init(void);
diff --git a/drivers/acpi/riscv/irqchip.c b/drivers/acpi/riscv/irqchip.c
index 6e15d45cb229..ca96bf109cf7 100644
--- a/drivers/acpi/riscv/irqchip.c
+++ b/drivers/acpi/riscv/irqchip.c
@@ -9,7 +9,10 @@
#include <linux/fwnode.h>
#include <linux/irqdomain.h>
#include <linux/list.h>
+#include <linux/msi.h>
+#include <linux/platform_device.h>
#include <linux/property.h>
+#include "../../../drivers/pci/pci.h"
struct riscv_irqchip_list {
struct fwnode_handle *fwnode;
@@ -101,3 +104,23 @@ struct fwnode_handle *acpi_riscv_get_msi_fwnode(struct device *dev)
{
return imsic_acpi_fwnode;
}
+
+void __init riscv_acpi_imsic_platform_init(void)
+{
+ struct platform_device *pdev;
+ int ret;
+
+ if (!acpi_riscv_get_msi_fwnode(NULL)) {
+ pci_no_msi();
+ return;
+ }
+
+ pdev = platform_device_alloc("riscv-imsic", 0);
+ if (!pdev)
+ return;
+
+ pdev->dev.fwnode = acpi_riscv_get_msi_fwnode(NULL);
+ ret = platform_device_add(pdev);
+ if (ret)
+ platform_device_put(pdev);
+}
--
2.39.2
Enable ECAM and MCFG to support PCIe on ACPI based
platforms.
Signed-off-by: Sunil V L <[email protected]>
---
arch/riscv/Kconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index e19f32c12a68..79dc35e89d5f 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_DMA_DEFAULT_COHERENT
select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
@@ -147,6 +148,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
--
2.39.2
Search and configure the MSI domain for the APLIC
on ACPI based systems.
Signed-off-by: Sunil V L <[email protected]>
---
drivers/irqchip/irq-riscv-aplic-msi.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
index 086d00e0429e..1948444c9e0c 100644
--- a/drivers/irqchip/irq-riscv-aplic-msi.c
+++ b/drivers/irqchip/irq-riscv-aplic-msi.c
@@ -16,6 +16,7 @@
#include <linux/platform_device.h>
#include <linux/printk.h>
#include <linux/smp.h>
+#include <asm/acpi.h>
#include "irq-riscv-aplic-main.h"
@@ -178,6 +179,8 @@ static void aplic_msi_write_msg(struct msi_desc *desc, struct msi_msg *msg)
int aplic_msi_setup(struct device *dev, void __iomem *regs)
{
const struct imsic_global_config *imsic_global;
+ struct irq_domain *msi_domain = NULL;
+ struct fwnode_handle *msi_fwnode;
struct irq_domain *irqdomain;
struct aplic_priv *priv;
struct aplic_msicfg *mc;
@@ -261,8 +264,17 @@ 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_fwnode = acpi_riscv_get_msi_fwnode(dev);
+ if (msi_fwnode)
+ msi_domain = irq_find_matching_fwnode(msi_fwnode,
+ DOMAIN_BUS_PLATFORM_MSI);
+
+ if (msi_domain)
+ dev_set_msi_domain(dev, msi_domain);
+ }
}
/* Create irq domain instance for the APLIC MSI-mode */
--
2.39.2
The functions defined in arm64 for ACPI support are required
for RISC-V also. To avoid duplication, copy these functions
to common location.
Signed-off-by: Sunil V L <[email protected]>
---
arch/arm64/kernel/pci.c | 193 ----------------------------------------
drivers/pci/pci-acpi.c | 182 +++++++++++++++++++++++++++++++++++++
2 files changed, 182 insertions(+), 193 deletions(-)
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 2276689b5411..fd9a7bed83ce 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -6,30 +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/of_pci.h>
-#include <linux/of_platform.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.
@@ -63,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 a05350a4e49c..d6b2d64b8237 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>
@@ -1517,4 +1518,185 @@ 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.39.2
CBO related block size in ACPI is provided by RHCT. Add
support to read the CMO node in RHCT to get this information.
Signed-off-by: Sunil V L <[email protected]>
---
arch/riscv/include/asm/acpi.h | 9 ++++++
drivers/acpi/riscv/rhct.c | 61 +++++++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+)
diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h
index d5604d2073bc..0c4e8b35103e 100644
--- a/arch/riscv/include/asm/acpi.h
+++ b/arch/riscv/include/asm/acpi.h
@@ -66,6 +66,8 @@ int acpi_get_riscv_isa(struct acpi_table_header *table,
unsigned int cpu, const char **isa);
static inline int acpi_numa_get_nid(unsigned int cpu) { return NUMA_NO_NODE; }
+int acpi_get_cbo_block_size(struct acpi_table_header *table, unsigned int cpu, u32 *cbom_size,
+ u32 *cboz_size, u32 *cbop_size);
#else
static inline void acpi_init_rintc_map(void) { }
static inline struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu)
@@ -79,6 +81,13 @@ static inline int acpi_get_riscv_isa(struct acpi_table_header *table,
return -EINVAL;
}
+static inline int acpi_get_cbo_block_size(struct acpi_table_header *table,
+ unsigned int cpu, u32 *cbom_size,
+ u32 *cboz_size, u32 *cbop_size)
+{
+ return -EINVAL;
+}
+
#endif /* CONFIG_ACPI */
#endif /*_ASM_ACPI_H*/
diff --git a/drivers/acpi/riscv/rhct.c b/drivers/acpi/riscv/rhct.c
index b280b3e9c7d9..07309525f277 100644
--- a/drivers/acpi/riscv/rhct.c
+++ b/drivers/acpi/riscv/rhct.c
@@ -81,3 +81,64 @@ int acpi_get_riscv_isa(struct acpi_table_header *table, unsigned int cpu, const
return -1;
}
+
+/*
+ * During early boot, the caller should call acpi_get_table() and pass its pointer to
+ * these functions(and free up later). At run time, since this table can be used
+ * multiple times, pass NULL so that the table remains in memory
+ */
+int acpi_get_cbo_block_size(struct acpi_table_header *table, unsigned int cpu,
+ u32 *cbom_size, u32 *cboz_size, u32 *cbop_size)
+{
+ struct acpi_rhct_node_header *node, *ref_node, *end;
+ u32 size_hdr = sizeof(struct acpi_rhct_node_header);
+ u32 size_hartinfo = sizeof(struct acpi_rhct_hart_info);
+ struct acpi_rhct_hart_info *hart_info;
+ struct acpi_rhct_cmo_node *cmo_node;
+ struct acpi_table_rhct *rhct;
+ u32 *hart_info_node_offset;
+ u32 acpi_cpu_id = get_acpi_id_for_cpu(cpu);
+
+ BUG_ON(acpi_disabled);
+
+ if (!table) {
+ rhct = (struct acpi_table_rhct *)acpi_get_rhct();
+ if (!rhct)
+ return -ENOENT;
+ } else {
+ rhct = (struct acpi_table_rhct *)table;
+ }
+
+ end = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->header.length);
+
+ for (node = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->node_offset);
+ node < end;
+ node = ACPI_ADD_PTR(struct acpi_rhct_node_header, node, node->length)) {
+ if (node->type == ACPI_RHCT_NODE_TYPE_HART_INFO) {
+ hart_info = ACPI_ADD_PTR(struct acpi_rhct_hart_info, node, size_hdr);
+ hart_info_node_offset = ACPI_ADD_PTR(u32, hart_info, size_hartinfo);
+ if (acpi_cpu_id != hart_info->uid)
+ continue;
+
+ for (int i = 0; i < hart_info->num_offsets; i++) {
+ ref_node = ACPI_ADD_PTR(struct acpi_rhct_node_header,
+ rhct, hart_info_node_offset[i]);
+ if (ref_node->type == ACPI_RHCT_NODE_TYPE_CMO) {
+ cmo_node = ACPI_ADD_PTR(struct acpi_rhct_cmo_node,
+ ref_node, size_hdr);
+ if (cbom_size)
+ *cbom_size = 1 << cmo_node->cbom_size;
+
+ if (cboz_size)
+ *cboz_size = 1 << cmo_node->cboz_size;
+
+ if (cbop_size)
+ *cbop_size = 1 << cmo_node->cbop_size;
+ return 0;
+ }
+ }
+ }
+ }
+
+ return -ENOENT;
+}
--
2.39.2
Since PLIC needs to be a platform device, probe the
MADT and create platform devices for each PLIC in the
system. Use software node framework for the fwnode
which allows to create properties and hence the
actual irqchip driver doesn't need to do anything
different for ACPI vs DT.
Signed-off-by: Sunil V L <[email protected]>
Co-developed-by: Haibo Xu <[email protected]>
Signed-off-by: Haibo Xu <[email protected]>
---
Documentation/riscv/acpi.rst | 6 ++
arch/riscv/include/asm/acpi.h | 3 +
drivers/acpi/riscv/init.c | 1 +
drivers/acpi/riscv/init.h | 1 +
drivers/acpi/riscv/irqchip.c | 198 ++++++++++++++++++++++++++++++++++
5 files changed, 209 insertions(+)
diff --git a/Documentation/riscv/acpi.rst b/Documentation/riscv/acpi.rst
index 9ea9008288ea..007483dfddc1 100644
--- a/Documentation/riscv/acpi.rst
+++ b/Documentation/riscv/acpi.rst
@@ -35,3 +35,9 @@ to this hart.
``riscv,gsi-base`` - The global system interrupt number where this APLIC’s
interrupt inputs start.
+
+3) RISC-V Platform Level Interrupt Controller (PLIC)
+-----------
+
+``riscv,gsi-base`` - The global system interrupt number where this PLIC’s
+interrupt inputs start.
diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h
index 6dde3d63dc0e..163b8eefa744 100644
--- a/arch/riscv/include/asm/acpi.h
+++ b/arch/riscv/include/asm/acpi.h
@@ -71,6 +71,7 @@ int acpi_get_cbo_block_size(struct acpi_table_header *table, unsigned int cpu, u
struct fwnode_handle *acpi_rintc_create_irqchip_fwnode(struct acpi_madt_rintc *rintc);
struct fwnode_handle *acpi_imsic_create_fwnode(struct acpi_madt_imsic *imsic);
struct fwnode_handle *acpi_riscv_get_msi_fwnode(struct device *dev);
+int acpi_plic_get_context_id(u8 plic_id, u16 idx);
#else
static inline void acpi_init_rintc_map(void) { }
static inline struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu)
@@ -95,6 +96,8 @@ static inline struct fwnode_handle *acpi_riscv_get_msi_fwnode(struct device *dev
{
return NULL;
}
+
+static inline int acpi_plic_get_context_id(u8 plic_id, u16 idx) { return idx; }
#endif /* CONFIG_ACPI */
#endif /*_ASM_ACPI_H*/
diff --git a/drivers/acpi/riscv/init.c b/drivers/acpi/riscv/init.c
index ee747211174f..cc733ea9ef1d 100644
--- a/drivers/acpi/riscv/init.c
+++ b/drivers/acpi/riscv/init.c
@@ -12,4 +12,5 @@ void __init acpi_riscv_init(void)
{
riscv_acpi_imsic_platform_init();
riscv_acpi_aplic_platform_init();
+ riscv_acpi_plic_platform_init();
}
diff --git a/drivers/acpi/riscv/init.h b/drivers/acpi/riscv/init.h
index 17bcf0baaadb..b4b305d83b3a 100644
--- a/drivers/acpi/riscv/init.h
+++ b/drivers/acpi/riscv/init.h
@@ -3,3 +3,4 @@
void __init riscv_acpi_imsic_platform_init(void);
void __init riscv_acpi_aplic_platform_init(void);
+void __init riscv_acpi_plic_platform_init(void);
diff --git a/drivers/acpi/riscv/irqchip.c b/drivers/acpi/riscv/irqchip.c
index 7fb7befdb303..cb70f0f2294b 100644
--- a/drivers/acpi/riscv/irqchip.c
+++ b/drivers/acpi/riscv/irqchip.c
@@ -25,6 +25,8 @@ static struct fwnode_handle *imsic_acpi_fwnode;
LIST_HEAD(aplic_list);
+LIST_HEAD(plic_list);
+
struct fwnode_handle *acpi_rintc_create_irqchip_fwnode(struct acpi_madt_rintc *rintc)
{
struct property_entry props[6] = {};
@@ -307,3 +309,199 @@ void __init riscv_acpi_aplic_platform_init(void)
acpi_set_gsi_to_irq_fallback(aplic_gsi_to_irq);
}
}
+
+static int acpi_plic_get_nr_contexts(u8 plic_id)
+{
+ struct riscv_irqchip_list *rintc_element;
+ struct fwnode_handle *fwnode;
+ struct list_head *i, *tmp;
+ u32 id;
+ int rc, nr_contexts = 0;
+
+ list_for_each_safe(i, tmp, &rintc_list) {
+ rintc_element = list_entry(i, struct riscv_irqchip_list, list);
+ fwnode = rintc_element->fwnode;
+ rc = fwnode_property_read_u32_array(fwnode, "riscv,ext-intc-id", &id, 1);
+ if (rc)
+ continue;
+
+ if (APLIC_PLIC_ID(id) == plic_id)
+ nr_contexts++;
+ }
+
+ return nr_contexts * 2;
+}
+
+int acpi_plic_get_context_id(u8 plic_id, u16 idx)
+{
+ struct riscv_irqchip_list *rintc_element;
+ struct fwnode_handle *fwnode;
+ struct list_head *i, *tmp;
+ u32 id;
+ int rc, nr_contexts = -1;
+
+ list_for_each_safe(i, tmp, &rintc_list) {
+ rintc_element = list_entry(i, struct riscv_irqchip_list, list);
+ fwnode = rintc_element->fwnode;
+ rc = fwnode_property_read_u32_array(fwnode, "riscv,ext-intc-id", &id, 1);
+ if (rc)
+ continue;
+
+ if (APLIC_PLIC_ID(id) == plic_id)
+ nr_contexts++;
+ if (nr_contexts == idx)
+ return IDC_CONTEXT_ID(id);
+ }
+
+ return -1;
+}
+
+static struct fwnode_handle *acpi_plic_create_fwnode(struct acpi_madt_plic *plic)
+{
+ struct fwnode_handle *fwnode, *parent_fwnode;
+ struct riscv_irqchip_list *plic_element;
+ struct software_node_ref_args *refs;
+ struct property_entry props[8] = {};
+ int nr_contexts;
+
+ props[0] = PROPERTY_ENTRY_U32("riscv,gsi-base", plic->gsi_base);
+ props[1] = PROPERTY_ENTRY_U32("riscv,ndev", plic->num_irqs);
+ props[2] = PROPERTY_ENTRY_U32("riscv,max_prio", plic->max_prio);
+ props[3] = PROPERTY_ENTRY_U8("riscv,plic-id", plic->id);
+ props[4] = PROPERTY_ENTRY_U64("riscv,plic-base", plic->base_addr);
+ props[5] = PROPERTY_ENTRY_U32("riscv,plic-size", plic->size);
+
+ nr_contexts = acpi_plic_get_nr_contexts(plic->id);
+ if (nr_contexts) {
+ refs = kcalloc(nr_contexts, sizeof(*refs), GFP_KERNEL);
+ for (int i = 0; i < nr_contexts / 2; i++) {
+ int context_id = acpi_plic_get_context_id(plic->id, i);
+
+ parent_fwnode = acpi_ext_intc_get_rintc_fwnode(plic->id, context_id);
+ refs[2 * i] = SOFTWARE_NODE_REFERENCE(to_software_node(parent_fwnode), 0);
+ refs[2 * i + 1] = SOFTWARE_NODE_REFERENCE(to_software_node(parent_fwnode),
+ RV_IRQ_EXT);
+ }
+ props[6] = PROPERTY_ENTRY_REF_ARRAY_LEN("interrupts-extended", refs, nr_contexts);
+ }
+
+ fwnode = fwnode_create_software_node_early(props, NULL);
+
+ if (fwnode) {
+ plic_element = kzalloc(sizeof(*plic_element), GFP_KERNEL);
+ if (!plic_element) {
+ fwnode_remove_software_node(fwnode);
+ return NULL;
+ }
+
+ plic_element->fwnode = fwnode;
+ list_add_tail(&plic_element->list, &plic_list);
+ }
+
+ return fwnode;
+}
+
+static struct fwnode_handle *plic_get_gsi_domain_id(u32 gsi)
+{
+ struct riscv_irqchip_list *plic_element;
+ struct fwnode_handle *fwnode;
+ struct list_head *i, *tmp;
+ u32 gsi_base;
+ u32 nr_irqs;
+ int rc;
+
+ list_for_each_safe(i, tmp, &plic_list) {
+ plic_element = list_entry(i, struct riscv_irqchip_list, list);
+ fwnode = plic_element->fwnode;
+ rc = fwnode_property_read_u32_array(fwnode, "riscv,gsi-base", &gsi_base, 1);
+ if (!rc) {
+ rc = fwnode_property_read_u32_array(fwnode, "riscv,ndev", &nr_irqs, 1);
+ if (!rc && (gsi >= gsi_base && gsi < gsi_base + nr_irqs))
+ return fwnode;
+ }
+ }
+
+ return NULL;
+}
+
+static u32 plic_gsi_to_irq(u32 gsi)
+{
+ return acpi_register_gsi(NULL, gsi, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_HIGH);
+}
+
+static int __init plic_create_platform_device(struct fwnode_handle *fwnode)
+{
+ struct platform_device *pdev;
+ u32 plic_size;
+ u8 plic_id;
+ struct resource *res;
+ u64 plic_base;
+ int ret;
+
+ if (!fwnode)
+ return -ENODEV;
+
+ ret = fwnode_property_read_u64_array(fwnode, "riscv,plic-base", &plic_base, 1);
+ if (ret)
+ return -ENODEV;
+
+ ret = fwnode_property_read_u32_array(fwnode, "riscv,plic-size", &plic_size, 1);
+ if (ret)
+ return -ENODEV;
+
+ ret = fwnode_property_read_u8_array(fwnode, "riscv,plic-id", &plic_id, 1);
+ if (ret)
+ return -ENODEV;
+
+ pdev = platform_device_alloc("riscv-plic", plic_id);
+ if (!pdev)
+ return -ENOMEM;
+
+ res = kcalloc(1, sizeof(*res), GFP_KERNEL);
+ if (!res) {
+ ret = -ENOMEM;
+ goto dev_put;
+ }
+
+ res->start = plic_base;
+ res->end = res->start + plic_size - 1;
+ res->flags = IORESOURCE_MEM;
+ ret = platform_device_add_resources(pdev, res, 1);
+ /*
+ * Resources are duplicated in platform_device_add_resources,
+ * free their allocated memory
+ */
+ kfree(res);
+
+ pdev->dev.fwnode = fwnode;
+ ret = platform_device_add(pdev);
+ if (ret)
+ goto dev_put;
+
+ return 0;
+
+dev_put:
+ platform_device_put(pdev);
+ return ret;
+}
+
+static int __init plic_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_plic *plic = (struct acpi_madt_plic *)header;
+ struct fwnode_handle *fwnode;
+
+ fwnode = acpi_plic_create_fwnode(plic);
+ if (fwnode)
+ plic_create_platform_device(fwnode);
+
+ return 0;
+}
+
+void __init riscv_acpi_plic_platform_init(void)
+{
+ if (acpi_table_parse_madt(ACPI_MADT_TYPE_PLIC, plic_parse_madt, 0) > 0) {
+ acpi_set_irq_model(ACPI_IRQ_MODEL_PLIC, plic_get_gsi_domain_id);
+ acpi_set_gsi_to_irq_fallback(plic_gsi_to_irq);
+ }
+}
--
2.39.2
Add support to probe the IMSIC early driver on ACPI
based RISC-V platforms.
Signed-off-by: Sunil V L <[email protected]>
---
arch/riscv/include/asm/acpi.h | 6 +++
drivers/acpi/riscv/irqchip.c | 57 +++++++++++++++++++++++++
drivers/irqchip/irq-riscv-imsic-early.c | 28 ++++++++++++
drivers/irqchip/irq-riscv-imsic-state.c | 33 +++++++++++---
4 files changed, 119 insertions(+), 5 deletions(-)
diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h
index 0ac2df2dd194..6dde3d63dc0e 100644
--- a/arch/riscv/include/asm/acpi.h
+++ b/arch/riscv/include/asm/acpi.h
@@ -69,6 +69,8 @@ static inline int acpi_numa_get_nid(unsigned int cpu) { return NUMA_NO_NODE; }
int acpi_get_cbo_block_size(struct acpi_table_header *table, unsigned int cpu, u32 *cbom_size,
u32 *cboz_size, u32 *cbop_size);
struct fwnode_handle *acpi_rintc_create_irqchip_fwnode(struct acpi_madt_rintc *rintc);
+struct fwnode_handle *acpi_imsic_create_fwnode(struct acpi_madt_imsic *imsic);
+struct fwnode_handle *acpi_riscv_get_msi_fwnode(struct device *dev);
#else
static inline void acpi_init_rintc_map(void) { }
static inline struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu)
@@ -89,6 +91,10 @@ static inline int acpi_get_cbo_block_size(struct acpi_table_header *table,
return -EINVAL;
}
+static inline struct fwnode_handle *acpi_riscv_get_msi_fwnode(struct device *dev)
+{
+ return NULL;
+}
#endif /* CONFIG_ACPI */
#endif /*_ASM_ACPI_H*/
diff --git a/drivers/acpi/riscv/irqchip.c b/drivers/acpi/riscv/irqchip.c
index 36f066a2cad5..6e15d45cb229 100644
--- a/drivers/acpi/riscv/irqchip.c
+++ b/drivers/acpi/riscv/irqchip.c
@@ -18,6 +18,8 @@ struct riscv_irqchip_list {
LIST_HEAD(rintc_list);
+static struct fwnode_handle *imsic_acpi_fwnode;
+
struct fwnode_handle *acpi_rintc_create_irqchip_fwnode(struct acpi_madt_rintc *rintc)
{
struct property_entry props[6] = {};
@@ -44,3 +46,58 @@ struct fwnode_handle *acpi_rintc_create_irqchip_fwnode(struct acpi_madt_rintc *r
return fwnode;
}
+
+static struct fwnode_handle *acpi_imsic_get_rintc_fwnode(u32 idx)
+{
+ struct riscv_irqchip_list *rintc_element;
+ struct fwnode_handle *fwnode;
+ struct list_head *i, *tmp;
+ unsigned int j = 0;
+
+ list_for_each_safe(i, tmp, &rintc_list) {
+ rintc_element = list_entry(i, struct riscv_irqchip_list, list);
+ fwnode = rintc_element->fwnode;
+
+ if (j == idx)
+ return fwnode;
+
+ j++;
+ }
+
+ return NULL;
+}
+
+struct fwnode_handle *acpi_imsic_create_fwnode(struct acpi_madt_imsic *imsic)
+{
+ struct property_entry props[8] = {};
+ struct software_node_ref_args *refs;
+ struct fwnode_handle *parent_fwnode;
+ unsigned int nr_rintc, i;
+
+ props[0] = PROPERTY_ENTRY_U32("riscv,guest-index-bits", imsic->guest_index_bits);
+ props[1] = PROPERTY_ENTRY_U32("riscv,hart-index-bits", imsic->hart_index_bits);
+ props[2] = PROPERTY_ENTRY_U32("riscv,group-index-bits", imsic->group_index_bits);
+ props[3] = PROPERTY_ENTRY_U32("riscv,group-index-shift", imsic->group_index_shift);
+ props[4] = PROPERTY_ENTRY_U32("riscv,num-ids", imsic->num_ids);
+ props[5] = PROPERTY_ENTRY_U32("riscv,num-guest-ids", imsic->num_guest_ids);
+
+ nr_rintc = list_count_nodes(&rintc_list);
+ refs = kcalloc(nr_rintc, sizeof(*refs), GFP_KERNEL);
+ if (!refs)
+ return NULL;
+
+ for (i = 0; i < nr_rintc; i++) {
+ parent_fwnode = acpi_imsic_get_rintc_fwnode(i);
+ refs[i] = SOFTWARE_NODE_REFERENCE(to_software_node(parent_fwnode), RV_IRQ_EXT);
+ }
+ props[6] = PROPERTY_ENTRY_REF_ARRAY_LEN("interrupts-extended", refs, nr_rintc);
+
+ imsic_acpi_fwnode = fwnode_create_software_node_early(props, NULL);
+
+ return imsic_acpi_fwnode;
+}
+
+struct fwnode_handle *acpi_riscv_get_msi_fwnode(struct device *dev)
+{
+ return imsic_acpi_fwnode;
+}
diff --git a/drivers/irqchip/irq-riscv-imsic-early.c b/drivers/irqchip/irq-riscv-imsic-early.c
index 1de89ce1ec2f..93f4d748ca6d 100644
--- a/drivers/irqchip/irq-riscv-imsic-early.c
+++ b/drivers/irqchip/irq-riscv-imsic-early.c
@@ -12,6 +12,7 @@
#include <linux/irqchip.h>
#include <linux/irqchip/chained_irq.h>
#include <linux/module.h>
+#include <linux/pci.h>
#include <linux/spinlock.h>
#include <linux/smp.h>
@@ -256,3 +257,30 @@ static int __init imsic_early_dt_init(struct device_node *node,
return 0;
}
IRQCHIP_DECLARE(riscv_imsic, "riscv,imsics", imsic_early_dt_init);
+
+#ifdef CONFIG_ACPI
+static int __init imsic_early_acpi_init(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct fwnode_handle *fwnode;
+ int rc;
+
+ /*
+ * There should be only one IMSIC node.
+ */
+ fwnode = acpi_imsic_create_fwnode((struct acpi_madt_imsic *)header);
+ if (!fwnode) {
+ pr_err("unable to create IMSIC FW node\n");
+ return -ENOMEM;
+ }
+
+ rc = imsic_early_probe(fwnode);
+ if (!rc)
+ pci_msi_register_fwnode_provider(&acpi_riscv_get_msi_fwnode);
+
+ 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-state.c b/drivers/irqchip/irq-riscv-imsic-state.c
index 412b5b919dcc..d0e09e51e8ae 100644
--- a/drivers/irqchip/irq-riscv-imsic-state.c
+++ b/drivers/irqchip/irq-riscv-imsic-state.c
@@ -225,15 +225,38 @@ static int __init imsic_get_parent_hartid(struct fwnode_handle *fwnode,
return riscv_get_intc_hartid(parent.fwnode, hartid);
}
+static int __init imsic_acpi_get_mmio_resource(struct fwnode_handle *fwnode,
+ u32 index, struct resource *res)
+{
+ int rc;
+ struct fwnode_reference_args parent;
+ u64 base;
+ u32 size;
+
+ rc = fwnode_property_get_reference_args(fwnode,
+ "interrupts-extended", NULL,
+ 0, index, &parent);
+ if (rc)
+ return rc;
+
+ rc = fwnode_property_read_u64_array(parent.fwnode, "riscv,imsic-addr",
+ &base, 1);
+ rc = fwnode_property_read_u32_array(parent.fwnode, "riscv,imsic-size",
+ &size, 1);
+ if (!rc) {
+ res->start = base;
+ res->end = res->start + size - 1;
+ }
+
+ return 0;
+}
+
static int __init imsic_get_mmio_resource(struct fwnode_handle *fwnode,
u32 index, struct resource *res)
{
- /*
- * Currently, only OF fwnode is support so extend this function
- * for other types of fwnode for ACPI support.
- */
if (!is_of_node(fwnode))
- return -EINVAL;
+ return imsic_acpi_get_mmio_resource(fwnode, index, res);
+
return of_address_to_resource(to_of_node(fwnode), index, res);
}
--
2.39.2
On Thu, Aug 03, 2023 at 11:29:03PM +0530, Sunil V L wrote:
> CBO related block size in ACPI is provided by RHCT. Add
> support to read the CMO node in RHCT to get this information.
...
> + if (!table) {
Why not positive conditional?
> + rhct = (struct acpi_table_rhct *)acpi_get_rhct();
> + if (!rhct)
> + return -ENOENT;
> + } else {
> + rhct = (struct acpi_table_rhct *)table;
> + }
...
> + end = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->header.length);
> +
Blank line here is not needed.
> + for (node = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->node_offset);
> + node < end;
> + node = ACPI_ADD_PTR(struct acpi_rhct_node_header, node, node->length)) {
> + for (int i = 0; i < hart_info->num_offsets; i++) {
> + ref_node = ACPI_ADD_PTR(struct acpi_rhct_node_header,
> + rhct, hart_info_node_offset[i]);
> + if (ref_node->type == ACPI_RHCT_NODE_TYPE_CMO) {
> + cmo_node = ACPI_ADD_PTR(struct acpi_rhct_cmo_node,
> + ref_node, size_hdr);
> + if (cbom_size)
> + *cbom_size = 1 << cmo_node->cbom_size;
> +
> + if (cboz_size)
> + *cboz_size = 1 << cmo_node->cboz_size;
> +
> + if (cbop_size)
> + *cbop_size = 1 << cmo_node->cbop_size;
BIT() in all three cases?
But how you guarantee it will not overflow? I mean who prevents cboX_size to be
bigger than 30 (note also that 31 in your case is Undefined Behaviour in
accordance with the C standard).
> + return 0;
> + }
> + }
> + }
--
With Best Regards,
Andy Shevchenko
On Thu, Aug 03, 2023 at 11:29:00PM +0530, Sunil V L wrote:
> The functions defined in arm64 for ACPI support are required
> for RISC-V also. To avoid duplication, copy these functions
> to common location.
...
> }
> +
Stray change.
> arch_initcall(acpi_pci_init);
> +
> +#if defined(CONFIG_ARM64)
...
> + 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;
Can be
cfg = pci_ecam_create(dev, &cfgres, bus_res, ecam_ops);
ret = PTR_ERR_OR_ZERO(cfg);
if (ret) {
dev_err(dev, "%04x:%pR error %d mapping ECAM\n", seg, bus_res, ret);
but as far as I understand this is in the original code like this, so consider
as a suggestion for further cleanups.
--
With Best Regards,
Andy Shevchenko
On Thu, Aug 03, 2023 at 11:29:04PM +0530, Sunil V L wrote:
> Using new interface to get the CBO block size information in
> RHCT, initialize the variables on ACPI platforms.
...
> #include <linux/of.h>
> +#include <linux/acpi.h>
Can you keep it sorted (to some extent)?
> +#include <asm/acpi.h>
What do you need this for?
> #include <asm/cacheflush.h>
--
With Best Regards,
Andy Shevchenko
On Thu, Aug 03, 2023 at 11:29:06PM +0530, Sunil V L wrote:
> From: Anup Patel <[email protected]>
>
> swnode framework can be used to create fwnode for interrupt
> controllers.
Why? What is this for?
Can you elaborate? This commit message is poorly written...
And why firmware node is not enough for ACPI case?
I assume the fwnode in DT case is already provided by OF.
> This helps in keeping the drivers same for both
> DT and ACPI. To enable this, enhance the swnode framework so
> that it can be created early during boot without dependency
> on sysfs.
...
> - swnode->kobj.kset = swnode_kset;
> + swnode->kobj.kset = (!early) ? swnode_kset : NULL;
Too many unneeded characters. Why parentheses? Why negative check?
...
> + if (early) {
> + ret = 0;
> + kobject_init(&swnode->kobj, &software_node_type_early);
> + swnode->kobj.parent = parent ? &parent->kobj : NULL;
> + if (node->name)
> + ret = kobject_set_name(&swnode->kobj,
> + "%s", node->name);
> + else
> + ret = kobject_set_name(&swnode->kobj,
> + "node%d", swnode->id);
> + if (!ret) {
> + spin_lock(&swnode_early_lock);
> + list_add_tail(&swnode->early, &swnode_early_list);
> + spin_unlock(&swnode_early_lock);
> + }
> + } else {
> + if (node->name)
> + ret = kobject_init_and_add(&swnode->kobj, &software_node_type,
> + parent ? &parent->kobj : NULL,
This looks like have a duplication.
> + "%s", node->name);
> + else
> + ret = kobject_init_and_add(&swnode->kobj, &software_node_type,
> + parent ? &parent->kobj : NULL,
> + "node%d", swnode->id);
> + }
Maybe it's possible to refactor this piece to be more compact?
...
> - return PTR_ERR_OR_ZERO(swnode_register(node, parent, 0));
> + return PTR_ERR_OR_ZERO(swnode_register(node, parent, 0, 0));
In one case you use boolean, here is unsigned int for early flag, why is the
inconsistency added?
...
> -struct fwnode_handle *
> -fwnode_create_software_node(const struct property_entry *properties,
> - const struct fwnode_handle *parent)
> +static struct fwnode_handle *
> +fwnode_create_software_node_common(const struct property_entry *properties,
> + const struct fwnode_handle *parent,
> + bool early)
Why would you need this API in early stages?
--
With Best Regards,
Andy Shevchenko
On Fri, Aug 04, 2023 at 08:53:14AM +0300, Andy Shevchenko wrote:
> On Thu, Aug 03, 2023 at 11:29:00PM +0530, Sunil V L wrote:
> > The functions defined in arm64 for ACPI support are required
> > for RISC-V also. To avoid duplication, copy these functions
> > to common location.
>
> ...
>
> > }
> > +
>
> Stray change.
>
Let me remove this in next version.
> > arch_initcall(acpi_pci_init);
> > +
> > +#if defined(CONFIG_ARM64)
>
> ...
>
> > + 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;
>
> Can be
>
> cfg = pci_ecam_create(dev, &cfgres, bus_res, ecam_ops);
> ret = PTR_ERR_OR_ZERO(cfg);
> if (ret) {
> dev_err(dev, "%04x:%pR error %d mapping ECAM\n", seg, bus_res, ret);
>
> but as far as I understand this is in the original code like this, so consider
> as a suggestion for further cleanups.
>
Good suggestion!. Sure, we can cleanup as a follow on patch.
Thanks,
Sunil
Hi Andy,
On Fri, Aug 04, 2023 at 09:09:16AM +0300, Andy Shevchenko wrote:
> On Thu, Aug 03, 2023 at 11:29:06PM +0530, Sunil V L wrote:
> > From: Anup Patel <[email protected]>
> >
> > swnode framework can be used to create fwnode for interrupt
> > controllers.
>
> Why? What is this for?
> Can you elaborate? This commit message is poorly written...
>
> And why firmware node is not enough for ACPI case?
> I assume the fwnode in DT case is already provided by OF.
>
Thanks a lot for the review!.
You are right, OF provides the fwnode for irqchip drivers. However, for
ACPI case, it is typically created using irq_domain_alloc_named_fwnode
or irq_domain_alloc_fwnode since these are not ACPI devices in the
namespace but from MADT. The fwnode created using
irq_domain_alloc_fwnode() is a simple one which doesn't support properties
similar to the one created by OF framework or software node framework.
Hence, lot of data from the MADT structures need to be cached as
separate structures in the drivers and also would need several ifdefs to
check for ACPI and some amount of code duplication is also required due
to the way DT driver gets the information vs ACPI.
The beauty of software node framework is, it supports adding properties
and also is a supported fwnode type in __irq_domain_create(). So, if we
can create the fwnode for these irqchip using software node, we can
attach the same properties and the actual irqchip driver which uses the
fwnode doesn't need to have any ACPI vs DT checks. Same driver will work
seamlessly on both DT and ACPI platforms. But the challenge is,
currently swnode expects to be created with sysfs which won't be
available during early boot when irqchip drivers need to be probed. So,
adding support to create without dependency on sysfs help us to reuse
the same framework for irqchip use case also.
Apologies for not descriptive in the commit message. Please let us know
your feedback on this approach.
> > This helps in keeping the drivers same for both
> > DT and ACPI. To enable this, enhance the swnode framework so
> > that it can be created early during boot without dependency
> > on sysfs.
>
> ...
>
> > - swnode->kobj.kset = swnode_kset;
> > + swnode->kobj.kset = (!early) ? swnode_kset : NULL;
>
> Too many unneeded characters. Why parentheses? Why negative check?
>
Sure, will update in next version.
> ...
>
> > + if (early) {
> > + ret = 0;
> > + kobject_init(&swnode->kobj, &software_node_type_early);
> > + swnode->kobj.parent = parent ? &parent->kobj : NULL;
> > + if (node->name)
> > + ret = kobject_set_name(&swnode->kobj,
> > + "%s", node->name);
> > + else
> > + ret = kobject_set_name(&swnode->kobj,
> > + "node%d", swnode->id);
> > + if (!ret) {
> > + spin_lock(&swnode_early_lock);
> > + list_add_tail(&swnode->early, &swnode_early_list);
> > + spin_unlock(&swnode_early_lock);
> > + }
> > + } else {
> > + if (node->name)
> > + ret = kobject_init_and_add(&swnode->kobj, &software_node_type,
> > + parent ? &parent->kobj : NULL,
>
> This looks like have a duplication.
>
> > + "%s", node->name);
> > + else
> > + ret = kobject_init_and_add(&swnode->kobj, &software_node_type,
> > + parent ? &parent->kobj : NULL,
> > + "node%d", swnode->id);
> > + }
>
> Maybe it's possible to refactor this piece to be more compact?
>
The issue is, kobject_init_and_add() expects sysfs. Let me try to
compact this in next version. Thanks!
> ...
>
> > - return PTR_ERR_OR_ZERO(swnode_register(node, parent, 0));
> > + return PTR_ERR_OR_ZERO(swnode_register(node, parent, 0, 0));
>
> In one case you use boolean, here is unsigned int for early flag, why is the
> inconsistency added?
>
Yeah, it should be bool. Let me fix it in next version.
> ...
>
> > -struct fwnode_handle *
> > -fwnode_create_software_node(const struct property_entry *properties,
> > - const struct fwnode_handle *parent)
> > +static struct fwnode_handle *
> > +fwnode_create_software_node_common(const struct property_entry *properties,
> > + const struct fwnode_handle *parent,
> > + bool early)
>
> Why would you need this API in early stages?
>
Hope I answered the question above.
Thanks!
Sunil
On Fri, Aug 04, 2023 at 09:00:45AM +0300, Andy Shevchenko wrote:
> On Thu, Aug 03, 2023 at 11:29:03PM +0530, Sunil V L wrote:
> > CBO related block size in ACPI is provided by RHCT. Add
> > support to read the CMO node in RHCT to get this information.
>
> ...
>
> > + if (!table) {
>
> Why not positive conditional?
>
Sure.
> > + rhct = (struct acpi_table_rhct *)acpi_get_rhct();
> > + if (!rhct)
> > + return -ENOENT;
> > + } else {
> > + rhct = (struct acpi_table_rhct *)table;
> > + }
>
> ...
>
> > + end = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->header.length);
>
> > +
>
> Blank line here is not needed.
>
Okay.
> > + for (node = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->node_offset);
> > + node < end;
> > + node = ACPI_ADD_PTR(struct acpi_rhct_node_header, node, node->length)) {
>
> > + for (int i = 0; i < hart_info->num_offsets; i++) {
> > + ref_node = ACPI_ADD_PTR(struct acpi_rhct_node_header,
> > + rhct, hart_info_node_offset[i]);
> > + if (ref_node->type == ACPI_RHCT_NODE_TYPE_CMO) {
> > + cmo_node = ACPI_ADD_PTR(struct acpi_rhct_cmo_node,
> > + ref_node, size_hdr);
> > + if (cbom_size)
> > + *cbom_size = 1 << cmo_node->cbom_size;
> > +
> > + if (cboz_size)
> > + *cboz_size = 1 << cmo_node->cboz_size;
> > +
> > + if (cbop_size)
> > + *cbop_size = 1 << cmo_node->cbop_size;
>
> BIT() in all three cases?
>
Sure.
> But how you guarantee it will not overflow? I mean who prevents cboX_size to be
> bigger than 30 (note also that 31 in your case is Undefined Behaviour in
> accordance with the C standard).
>
Good catch!. Let me add a check that the value is not more than 30.
Thanks!
Sunil
On Fri, Aug 04, 2023 at 08:56:29AM +0300, Andy Shevchenko wrote:
> On Thu, Aug 03, 2023 at 11:29:04PM +0530, Sunil V L wrote:
> > Using new interface to get the CBO block size information in
> > RHCT, initialize the variables on ACPI platforms.
>
> ...
>
> > #include <linux/of.h>
> > +#include <linux/acpi.h>
>
> Can you keep it sorted (to some extent)?
>
Sure.
> > +#include <asm/acpi.h>
>
> What do you need this for?
>
> > #include <asm/cacheflush.h>
>
When CONFIG_ACPI is disabled, this include is required to get
acpi_get_cbo_block_size().
Thanks,
Sunil
On Fri, Aug 04, 2023 at 02:50:34PM +0530, Sunil V L wrote:
> On Fri, Aug 04, 2023 at 08:56:29AM +0300, Andy Shevchenko wrote:
> > On Thu, Aug 03, 2023 at 11:29:04PM +0530, Sunil V L wrote:
> > > Using new interface to get the CBO block size information in
> > > RHCT, initialize the variables on ACPI platforms.
...
> > > +#include <asm/acpi.h>
> >
> > What do you need this for?
> >
> > > #include <asm/cacheflush.h>
> >
> When CONFIG_ACPI is disabled, this include is required to get
> acpi_get_cbo_block_size().
How is it useful without ACPI being enabled? If it's indeed
(in which I do not believe), better to make sure you have it
avaiable independently on CONFIG_ACPI. Otherwise, just put
#ifdef CONFIG_ACPI around the call.
--
With Best Regards,
Andy Shevchenko
On Fri, Aug 04, 2023 at 05:59:51PM +0300, Andy Shevchenko wrote:
> On Fri, Aug 04, 2023 at 02:50:34PM +0530, Sunil V L wrote:
> > On Fri, Aug 04, 2023 at 08:56:29AM +0300, Andy Shevchenko wrote:
> > > On Thu, Aug 03, 2023 at 11:29:04PM +0530, Sunil V L wrote:
> > > > Using new interface to get the CBO block size information in
> > > > RHCT, initialize the variables on ACPI platforms.
>
> ...
>
> > > > +#include <asm/acpi.h>
> > >
> > > What do you need this for?
> > >
> > > > #include <asm/cacheflush.h>
> > >
> > When CONFIG_ACPI is disabled, this include is required to get
> > acpi_get_cbo_block_size().
>
> How is it useful without ACPI being enabled?
It is not, as evidenced by the `return -EINVAL;`.
> If it's indeed
> (in which I do not believe), better to make sure you have it
> avaiable independently on CONFIG_ACPI. Otherwise, just put
> #ifdef CONFIG_ACPI around the call.
Let's not litter the code with ifdeffery please where it can be easily
avoided.
On Fri, Aug 04, 2023 at 07:52:56PM +0300, Andy Shevchenko wrote:
> On Fri, Aug 04, 2023 at 04:19:27PM +0100, Conor Dooley wrote:
> > On Fri, Aug 04, 2023 at 05:59:51PM +0300, Andy Shevchenko wrote:
> > > On Fri, Aug 04, 2023 at 02:50:34PM +0530, Sunil V L wrote:
> > > > On Fri, Aug 04, 2023 at 08:56:29AM +0300, Andy Shevchenko wrote:
> > > > > On Thu, Aug 03, 2023 at 11:29:04PM +0530, Sunil V L wrote:
...
> > > > > > +#include <asm/acpi.h>
> > > > >
> > > > > What do you need this for?
> > > > >
> > > > > > #include <asm/cacheflush.h>
> > > > >
> > > > When CONFIG_ACPI is disabled, this include is required to get
> > > > acpi_get_cbo_block_size().
> > >
> > > How is it useful without ACPI being enabled?
> >
> > It is not, as evidenced by the `return -EINVAL;`.
> >
> > > If it's indeed
> > > (in which I do not believe), better to make sure you have it
> > > avaiable independently on CONFIG_ACPI. Otherwise, just put
> > > #ifdef CONFIG_ACPI around the call.
> >
> > Let's not litter the code with ifdeffery please where it can be easily
> > avoided.
>
> Including asm/acpi.h looks to me as a "let's avoid it with a hack that it
> is uglier than ifdeffery". Sorry, but ifdeffery for ACPI, with all my full
> agreement with the statement that it's not good, is the correct way to fix
> this.
On the other hand this is an arch code and I see precedents of using the
headers together, alas, it seems not better to me that ugly ifdeffery.
So, I leave it to the respective maintainers to decide.
--
With Best Regards,
Andy Shevchenko
On Fri, Aug 04, 2023 at 04:19:27PM +0100, Conor Dooley wrote:
> On Fri, Aug 04, 2023 at 05:59:51PM +0300, Andy Shevchenko wrote:
> > On Fri, Aug 04, 2023 at 02:50:34PM +0530, Sunil V L wrote:
> > > On Fri, Aug 04, 2023 at 08:56:29AM +0300, Andy Shevchenko wrote:
> > > > On Thu, Aug 03, 2023 at 11:29:04PM +0530, Sunil V L wrote:
...
> > > > > +#include <asm/acpi.h>
> > > >
> > > > What do you need this for?
> > > >
> > > > > #include <asm/cacheflush.h>
> > > >
> > > When CONFIG_ACPI is disabled, this include is required to get
> > > acpi_get_cbo_block_size().
> >
> > How is it useful without ACPI being enabled?
>
> It is not, as evidenced by the `return -EINVAL;`.
>
> > If it's indeed
> > (in which I do not believe), better to make sure you have it
> > avaiable independently on CONFIG_ACPI. Otherwise, just put
> > #ifdef CONFIG_ACPI around the call.
>
> Let's not litter the code with ifdeffery please where it can be easily
> avoided.
Including asm/acpi.h looks to me as a "let's avoid it with a hack that it
is uglier than ifdeffery". Sorry, but ifdeffery for ACPI, with all my full
agreement with the statement that it's not good, is the correct way to fix
this.
--
With Best Regards,
Andy Shevchenko
On Thu, Aug 03, 2023 at 11:28:58PM +0530, Sunil V L wrote:
> acpi_os_ioremap() currently is a wrapper to memremap() on
> RISC-V. But the callers of acpi_os_ioremap() expect it to
> return __iomem address and hence sparse tool reports a new
> warning. Fix this issue by type casting to __iomem type.
>
> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Fixes: a91a9ffbd3a5 ("RISC-V: Add support to build the ACPI core")
> Signed-off-by: Sunil V L <[email protected]>
> ---
> arch/riscv/include/asm/acpi.h | 2 +-
> arch/riscv/kernel/acpi.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h
> index f71ce21ff684..d5604d2073bc 100644
> --- a/arch/riscv/include/asm/acpi.h
> +++ b/arch/riscv/include/asm/acpi.h
> @@ -19,7 +19,7 @@ typedef u64 phys_cpuid_t;
> #define PHYS_CPUID_INVALID INVALID_HARTID
>
> /* ACPI table mapping after acpi_permanent_mmap is set */
> -void *acpi_os_ioremap(acpi_physical_address phys, acpi_size size);
> +void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size);
> #define acpi_os_ioremap acpi_os_ioremap
>
> #define acpi_strict 1 /* No out-of-spec workarounds on RISC-V */
> diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c
> index 5ee03ebab80e..56cb2c986c48 100644
> --- a/arch/riscv/kernel/acpi.c
> +++ b/arch/riscv/kernel/acpi.c
> @@ -215,9 +215,9 @@ void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
> early_iounmap(map, size);
> }
>
> -void *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
> +void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
> {
> - return memremap(phys, size, MEMREMAP_WB);
> + return (void __iomem *)memremap(phys, size, MEMREMAP_WB);
> }
>
> #ifdef CONFIG_PCI
> --
> 2.39.2
>
Reviewed-by: Andrew Jones <[email protected]>
On Thu, Aug 03, 2023 at 11:29:00PM +0530, Sunil V L wrote:
> The functions defined in arm64 for ACPI support are required
> for RISC-V also. To avoid duplication, copy these functions
> to common location.
I assume this is a "move" (not a copy) and the code being moved isn't
being changed.
If so,
Acked-by: Bjorn Helgaas <[email protected]>
> Signed-off-by: Sunil V L <[email protected]>
> ---
> arch/arm64/kernel/pci.c | 193 ----------------------------------------
> drivers/pci/pci-acpi.c | 182 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 182 insertions(+), 193 deletions(-)
>
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index 2276689b5411..fd9a7bed83ce 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -6,30 +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/of_pci.h>
> -#include <linux/of_platform.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.
> @@ -63,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 a05350a4e49c..d6b2d64b8237 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>
> @@ -1517,4 +1518,185 @@ 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.39.2
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Fri, 04 Aug 2023 09:11:05 +0100,
Sunil V L <[email protected]> wrote:
>
> Hi Andy,
>
> On Fri, Aug 04, 2023 at 09:09:16AM +0300, Andy Shevchenko wrote:
> > On Thu, Aug 03, 2023 at 11:29:06PM +0530, Sunil V L wrote:
> > > From: Anup Patel <[email protected]>
> > >
> > > swnode framework can be used to create fwnode for interrupt
> > > controllers.
> >
> > Why? What is this for?
> > Can you elaborate? This commit message is poorly written...
> >
> > And why firmware node is not enough for ACPI case?
> > I assume the fwnode in DT case is already provided by OF.
> >
> Thanks a lot for the review!.
>
> You are right, OF provides the fwnode for irqchip drivers. However, for
> ACPI case, it is typically created using irq_domain_alloc_named_fwnode
> or irq_domain_alloc_fwnode since these are not ACPI devices in the
> namespace but from MADT. The fwnode created using
> irq_domain_alloc_fwnode() is a simple one which doesn't support properties
> similar to the one created by OF framework or software node framework.
> Hence, lot of data from the MADT structures need to be cached as
> separate structures in the drivers and also would need several ifdefs to
> check for ACPI and some amount of code duplication is also required due
> to the way DT driver gets the information vs ACPI.
>
> The beauty of software node framework is, it supports adding properties
> and also is a supported fwnode type in __irq_domain_create().
There is no beauty here. Only some extra bloat that we do not need.
DT and ACPI exposes very different attributes. One describe the HW,
the other one describe an OS abstraction. Pretending that you can
summon both into the same infrastructure is a fallacy. You'll just end
up with the cross product of both infrastructure, and pollute the rest
of the kernel with pointless cruft.
> So, if we
> can create the fwnode for these irqchip using software node, we can
> attach the same properties and the actual irqchip driver which uses the
> fwnode doesn't need to have any ACPI vs DT checks. Same driver will work
> seamlessly on both DT and ACPI platforms. But the challenge is,
> currently swnode expects to be created with sysfs which won't be
> available during early boot when irqchip drivers need to be probed. So,
> adding support to create without dependency on sysfs help us to reuse
> the same framework for irqchip use case also.
That's another fallacy.
Most irqchips *DO NOT* need to be probed early. Only the root
irqchip. Given that this series is about *secondary* interrupt
controllers, they absolutely don't need to be probed early.
To be clear: I do not intend to merge anything that:
- invents yet another way to "abstract" firmware interfaces
- adds more "early probe" hacks for non-primary interrupt controllers
I have already said that in response to Anup's AIA series, and this
equally applies to this series.
M.
--
Without deviation from the norm, progress is not possible.
On Mon, Aug 07, 2023 at 05:41:52PM -0500, Bjorn Helgaas wrote:
> On Thu, Aug 03, 2023 at 11:29:00PM +0530, Sunil V L wrote:
> > The functions defined in arm64 for ACPI support are required
> > for RISC-V also. To avoid duplication, copy these functions
> > to common location.
>
> I assume this is a "move" (not a copy) and the code being moved isn't
> being changed.
>
Hi Bjorn,
Thank you very much for the review!. Yes, it is a move as is. Let me
update the commit message in next version.
Thanks!
Sunil
> If so,
>
> Acked-by: Bjorn Helgaas <[email protected]>
>
> > Signed-off-by: Sunil V L <[email protected]>
> > ---
> > arch/arm64/kernel/pci.c | 193 ----------------------------------------
> > drivers/pci/pci-acpi.c | 182 +++++++++++++++++++++++++++++++++++++
> > 2 files changed, 182 insertions(+), 193 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> > index 2276689b5411..fd9a7bed83ce 100644
> > --- a/arch/arm64/kernel/pci.c
> > +++ b/arch/arm64/kernel/pci.c
> > @@ -6,30 +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/of_pci.h>
> > -#include <linux/of_platform.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.
> > @@ -63,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 a05350a4e49c..d6b2d64b8237 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>
> > @@ -1517,4 +1518,185 @@ 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.39.2
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Aug 03, 2023 at 11:29:15PM +0530, Sunil V L wrote:
> Since PLIC needs to be a platform device
For the unwashed, why does the PLCI need to be a platform device?
(And while you're at that, please try to make use of the extra ~20
characters per line that you can use here.)
> probe the
> MADT and create platform devices for each PLIC in the
> system. Use software node framework for the fwnode
> which allows to create properties and hence the
> actual irqchip driver doesn't need to do anything
> different for ACPI vs DT.
>
> Signed-off-by: Sunil V L <[email protected]>
> Co-developed-by: Haibo Xu <[email protected]>
> Signed-off-by: Haibo Xu <[email protected]>
> +static struct fwnode_handle *acpi_plic_create_fwnode(struct acpi_madt_plic *plic)
> +{
> + struct fwnode_handle *fwnode, *parent_fwnode;
> + struct riscv_irqchip_list *plic_element;
> + struct software_node_ref_args *refs;
> + struct property_entry props[8] = {};
> + int nr_contexts;
> +
> + props[0] = PROPERTY_ENTRY_U32("riscv,gsi-base", plic->gsi_base);
> + props[1] = PROPERTY_ENTRY_U32("riscv,ndev", plic->num_irqs);
> + props[2] = PROPERTY_ENTRY_U32("riscv,max_prio", plic->max_prio);
My OCD wants to know why this gets an _ but the others have a -.
> + props[3] = PROPERTY_ENTRY_U8("riscv,plic-id", plic->id);
> + props[4] = PROPERTY_ENTRY_U64("riscv,plic-base", plic->base_addr);
> + props[5] = PROPERTY_ENTRY_U32("riscv,plic-size", plic->size);
On Tue, Aug 8, 2023 at 2:12 PM Conor Dooley <[email protected]> wrote:
>
> On Thu, Aug 03, 2023 at 11:29:15PM +0530, Sunil V L wrote:
> > Since PLIC needs to be a platform device
>
> For the unwashed, why does the PLCI need to be a platform device?
> (And while you're at that, please try to make use of the extra ~20
> characters per line that you can use here.)
As suggested by Marc Z, only timers and IPIs need to be probed early.
Everything else needs to be a platform device. The devlink feature of
Linux DD framework ensures that platform devices are probed in the
right order based on the interdependency.
The PATCH5 of the v7 AIA series converts the PLIC driver into a
platform driver. This works perfectly fine.
>
> > probe the
> > MADT and create platform devices for each PLIC in the
> > system. Use software node framework for the fwnode
> > which allows to create properties and hence the
> > actual irqchip driver doesn't need to do anything
> > different for ACPI vs DT.
> >
> > Signed-off-by: Sunil V L <[email protected]>
> > Co-developed-by: Haibo Xu <[email protected]>
> > Signed-off-by: Haibo Xu <[email protected]>
>
> > +static struct fwnode_handle *acpi_plic_create_fwnode(struct acpi_madt_plic *plic)
> > +{
> > + struct fwnode_handle *fwnode, *parent_fwnode;
> > + struct riscv_irqchip_list *plic_element;
> > + struct software_node_ref_args *refs;
> > + struct property_entry props[8] = {};
> > + int nr_contexts;
> > +
> > + props[0] = PROPERTY_ENTRY_U32("riscv,gsi-base", plic->gsi_base);
> > + props[1] = PROPERTY_ENTRY_U32("riscv,ndev", plic->num_irqs);
> > + props[2] = PROPERTY_ENTRY_U32("riscv,max_prio", plic->max_prio);
>
> My OCD wants to know why this gets an _ but the others have a -.
>
> > + props[3] = PROPERTY_ENTRY_U8("riscv,plic-id", plic->id);
> > + props[4] = PROPERTY_ENTRY_U64("riscv,plic-base", plic->base_addr);
> > + props[5] = PROPERTY_ENTRY_U32("riscv,plic-size", plic->size);
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Regards,
Anup
On Tue, Aug 08, 2023 at 04:27:16PM +0530, Anup Patel wrote:
> On Tue, Aug 8, 2023 at 2:12 PM Conor Dooley <[email protected]> wrote:
> >
> > On Thu, Aug 03, 2023 at 11:29:15PM +0530, Sunil V L wrote:
> > > Since PLIC needs to be a platform device
> >
> > For the unwashed, why does the PLCI need to be a platform device?
> > (And while you're at that, please try to make use of the extra ~20
> > characters per line that you can use here.)
>
> As suggested by Marc Z, only timers and IPIs need to be probed early.
> Everything else needs to be a platform device. The devlink feature of
> Linux DD framework ensures that platform devices are probed in the
> right order based on the interdependency.
>
> The PATCH5 of the v7 AIA series converts the PLIC driver into a
> platform driver. This works perfectly fine.
To be clear, I want the explanation of why the "PLIC needs to be a
platform device" to be in the commit message.
Thanks,
Conor.
>
> >
> > > probe the
> > > MADT and create platform devices for each PLIC in the
> > > system. Use software node framework for the fwnode
> > > which allows to create properties and hence the
> > > actual irqchip driver doesn't need to do anything
> > > different for ACPI vs DT.
> > >
> > > Signed-off-by: Sunil V L <[email protected]>
> > > Co-developed-by: Haibo Xu <[email protected]>
> > > Signed-off-by: Haibo Xu <[email protected]>
> >
> > > +static struct fwnode_handle *acpi_plic_create_fwnode(struct acpi_madt_plic *plic)
> > > +{
> > > + struct fwnode_handle *fwnode, *parent_fwnode;
> > > + struct riscv_irqchip_list *plic_element;
> > > + struct software_node_ref_args *refs;
> > > + struct property_entry props[8] = {};
> > > + int nr_contexts;
> > > +
> > > + props[0] = PROPERTY_ENTRY_U32("riscv,gsi-base", plic->gsi_base);
> > > + props[1] = PROPERTY_ENTRY_U32("riscv,ndev", plic->num_irqs);
> > > + props[2] = PROPERTY_ENTRY_U32("riscv,max_prio", plic->max_prio);
> >
> > My OCD wants to know why this gets an _ but the others have a -.
> >
> > > + props[3] = PROPERTY_ENTRY_U8("riscv,plic-id", plic->id);
> > > + props[4] = PROPERTY_ENTRY_U64("riscv,plic-base", plic->base_addr);
> > > + props[5] = PROPERTY_ENTRY_U32("riscv,plic-size", plic->size);
On Tue, Aug 08, 2023 at 10:22:10AM +0530, Sunil V L wrote:
> On Mon, Aug 07, 2023 at 05:41:52PM -0500, Bjorn Helgaas wrote:
> > On Thu, Aug 03, 2023 at 11:29:00PM +0530, Sunil V L wrote:
> > > The functions defined in arm64 for ACPI support are required
> > > for RISC-V also. To avoid duplication, copy these functions
> > > to common location.
> >
> > I assume this is a "move" (not a copy) and the code being moved isn't
> > being changed.
>
> Thank you very much for the review!. Yes, it is a move as is. Let me
> update the commit message in next version.
Try to play with -M -C
--
With Best Regards,
Andy Shevchenko
On Thu, 03 Aug 2023 18:59:06 +0100,
Sunil V L <[email protected]> wrote:
>
> From: Anup Patel <[email protected]>
>
> swnode framework can be used to create fwnode for interrupt
> controllers. This helps in keeping the drivers same for both
> DT and ACPI. To enable this, enhance the swnode framework so
> that it can be created early during boot without dependency
> on sysfs.
Where is this coming from? We have had common abstractions for
irqchips for a very long time. We can create fwnodes any odd way we
want, but why oh why should we invent another method for that?
We already have multiple architectures such as arm64 and loongarch
already having both DT and ACPI. Do they need another level of
"abstraction"? No. Why is risc-v so special?
M.
--
Without deviation from the norm, progress is not possible.
On Mon, Aug 07, 2023 at 05:41:52PM -0500, Bjorn Helgaas wrote:
> On Thu, Aug 03, 2023 at 11:29:00PM +0530, Sunil V L wrote:
> > The functions defined in arm64 for ACPI support are required
> > for RISC-V also. To avoid duplication, copy these functions
> > to common location.
>
> I assume this is a "move" (not a copy) and the code being moved isn't
> being changed.
Not sure is proper -M -C will help to recognize that on the patch generation.
Maybe -M 50 (or another carefully chosen percentage) can help.
--
With Best Regards,
Andy Shevchenko
Hey Sunil,
On Thu, Aug 03, 2023 at 11:29:07PM +0530, Sunil V L wrote:
> By using swnode framework, all data from ACPI tables can
> be populated as properties of the swnode. This simplifies
> the driver code and removes the need for ACPI vs DT checks.
> Use this framework for RISC-V INTC driver.
btw, you are permitted to use more than 60 characters in a commit
message...
>
> Signed-off-by: Sunil V L <[email protected]>
> ---
> Documentation/riscv/acpi.rst | 21 +++++++++++++++
> arch/riscv/include/asm/acpi.h | 1 +
> drivers/acpi/riscv/Makefile | 2 +-
> drivers/acpi/riscv/irqchip.c | 46 ++++++++++++++++++++++++++++++++
> drivers/irqchip/irq-riscv-intc.c | 12 ++++-----
> 5 files changed, 75 insertions(+), 7 deletions(-)
> create mode 100644 drivers/acpi/riscv/irqchip.c
>
> diff --git a/Documentation/riscv/acpi.rst b/Documentation/riscv/acpi.rst
> index 9870a282815b..e2406546bc16 100644
> --- a/Documentation/riscv/acpi.rst
> +++ b/Documentation/riscv/acpi.rst
> @@ -8,3 +8,24 @@ The ISA string parsing rules for ACPI are defined by `Version ASCIIDOC
> Conversion, 12/2022 of the RISC-V specifications, as defined by tag
> "riscv-isa-release-1239329-2023-05-23" (commit 1239329
> ) <https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-isa-release-1239329-2023-05-23>`_
> +
> +Interrupt Controller Drivers
> +=======
> +
> +ACPI drivers for RISC-V interrupt controllers use software node framework to
> +create the fwnode for the interrupt controllers. Below properties are
> +additionally required for some firmware nodes apart from the properties
> +defined by the device tree bindings for these interrupt controllers. The
> +properties are created using the data in MADT table.
I don't really understand this text, specifically what you are getting
at w/ the dependency on devicetree properties. What exactly does "apart
from the properties defined by the devicetree bindings" mean?
Is there prior art for this kind of "ACPI needs swnodes that look
vaguely similar to devicetree" for other interrupt controllers?
Thanks,
Conor.
> +1) RISC-V Interrupt Controller (INTC)
> +-----------
> +``hartid`` - Hart ID of the hart this interrupt controller belongs to.
> +
> +``riscv,imsic-addr`` - Physical base address of the Incoming MSI Controller
> +(IMSIC) MMIO region of this hart.
> +
> +``riscv,imsic-size`` - Size in bytes of the IMSIC MMIO region of this hart.
> +
> +``riscv,ext-intc-id`` - The unique ID of the external interrupts connected
> +to this hart.
> diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h
> index 0c4e8b35103e..0ac2df2dd194 100644
> --- a/arch/riscv/include/asm/acpi.h
> +++ b/arch/riscv/include/asm/acpi.h
> @@ -68,6 +68,7 @@ int acpi_get_riscv_isa(struct acpi_table_header *table,
> static inline int acpi_numa_get_nid(unsigned int cpu) { return NUMA_NO_NODE; }
> int acpi_get_cbo_block_size(struct acpi_table_header *table, unsigned int cpu, u32 *cbom_size,
> u32 *cboz_size, u32 *cbop_size);
> +struct fwnode_handle *acpi_rintc_create_irqchip_fwnode(struct acpi_madt_rintc *rintc);
> #else
> static inline void acpi_init_rintc_map(void) { }
> static inline struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu)
> diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile
> index 8b3b126e0b94..8b664190d172 100644
> --- a/drivers/acpi/riscv/Makefile
> +++ b/drivers/acpi/riscv/Makefile
> @@ -1,2 +1,2 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -obj-y += rhct.o
> +obj-y += rhct.o irqchip.o
> diff --git a/drivers/acpi/riscv/irqchip.c b/drivers/acpi/riscv/irqchip.c
> new file mode 100644
> index 000000000000..36f066a2cad5
> --- /dev/null
> +++ b/drivers/acpi/riscv/irqchip.c
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2023, Ventana Micro Systems Inc
> + * Author: Sunil V L <[email protected]>
> + *
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/fwnode.h>
> +#include <linux/irqdomain.h>
> +#include <linux/list.h>
> +#include <linux/property.h>
> +
> +struct riscv_irqchip_list {
> + struct fwnode_handle *fwnode;
> + struct list_head list;
> +};
> +
> +LIST_HEAD(rintc_list);
> +
> +struct fwnode_handle *acpi_rintc_create_irqchip_fwnode(struct acpi_madt_rintc *rintc)
> +{
> + struct property_entry props[6] = {};
> + struct fwnode_handle *fwnode;
> + struct riscv_irqchip_list *rintc_element;
> +
> + props[0] = PROPERTY_ENTRY_U64("hartid", rintc->hart_id);
> + props[1] = PROPERTY_ENTRY_U32("riscv,ext-intc-id", rintc->ext_intc_id);
> + props[2] = PROPERTY_ENTRY_U64("riscv,imsic-addr", rintc->imsic_addr);
> + props[3] = PROPERTY_ENTRY_U32("riscv,imsic-size", rintc->imsic_size);
> + props[4] = PROPERTY_ENTRY_U32("#interrupt-cells", 1);
> +
> + fwnode = fwnode_create_software_node_early(props, NULL);
> + if (fwnode) {
> + rintc_element = kzalloc(sizeof(*rintc_element), GFP_KERNEL);
> + if (!rintc_element) {
> + fwnode_remove_software_node(fwnode);
> + return NULL;
> + }
> +
> + rintc_element->fwnode = fwnode;
> + list_add_tail(&rintc_element->list, &rintc_list);
> + }
> +
> + return fwnode;
> +}
> diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> index 1a0fc87152c5..1ef9cada1ed3 100644
> --- a/drivers/irqchip/irq-riscv-intc.c
> +++ b/drivers/irqchip/irq-riscv-intc.c
> @@ -203,6 +203,12 @@ static int __init riscv_intc_acpi_init(union acpi_subtable_headers *header,
>
> rintc = (struct acpi_madt_rintc *)header;
>
> + fn = acpi_rintc_create_irqchip_fwnode(rintc);
> + if (!fn) {
> + pr_err("unable to create INTC FW node\n");
> + return -ENOMEM;
> + }
> +
> /*
> * The ACPI MADT will have one INTC for each CPU (or HART)
> * so riscv_intc_acpi_init() function will be called once
> @@ -212,12 +218,6 @@ static int __init riscv_intc_acpi_init(union acpi_subtable_headers *header,
> if (riscv_hartid_to_cpuid(rintc->hart_id) != smp_processor_id())
> return 0;
>
> - fn = irq_domain_alloc_named_fwnode("RISCV-INTC");
> - if (!fn) {
> - pr_err("unable to allocate INTC FW node\n");
> - return -ENOMEM;
> - }
> -
> return riscv_intc_init_common(fn);
> }
>
> --
> 2.39.2
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, Aug 08, 2023 at 02:17:22PM +0100, Marc Zyngier wrote:
> On Fri, 04 Aug 2023 09:11:05 +0100,
> Sunil V L <[email protected]> wrote:
> >
> > Hi Andy,
> >
> > On Fri, Aug 04, 2023 at 09:09:16AM +0300, Andy Shevchenko wrote:
> > > On Thu, Aug 03, 2023 at 11:29:06PM +0530, Sunil V L wrote:
> > > > From: Anup Patel <[email protected]>
> > > >
> > > > swnode framework can be used to create fwnode for interrupt
> > > > controllers.
> > >
> > > Why? What is this for?
> > > Can you elaborate? This commit message is poorly written...
> > >
> > > And why firmware node is not enough for ACPI case?
> > > I assume the fwnode in DT case is already provided by OF.
> > >
> > Thanks a lot for the review!.
> >
> > You are right, OF provides the fwnode for irqchip drivers. However, for
> > ACPI case, it is typically created using irq_domain_alloc_named_fwnode
> > or irq_domain_alloc_fwnode since these are not ACPI devices in the
> > namespace but from MADT. The fwnode created using
> > irq_domain_alloc_fwnode() is a simple one which doesn't support properties
> > similar to the one created by OF framework or software node framework.
> > Hence, lot of data from the MADT structures need to be cached as
> > separate structures in the drivers and also would need several ifdefs to
> > check for ACPI and some amount of code duplication is also required due
> > to the way DT driver gets the information vs ACPI.
> >
> > The beauty of software node framework is, it supports adding properties
> > and also is a supported fwnode type in __irq_domain_create().
>
> There is no beauty here. Only some extra bloat that we do not need.
>
> DT and ACPI exposes very different attributes. One describe the HW,
> the other one describe an OS abstraction. Pretending that you can
> summon both into the same infrastructure is a fallacy. You'll just end
> up with the cross product of both infrastructure, and pollute the rest
> of the kernel with pointless cruft.
>
Hi Marc,
Thank you very much for the feedback!. Sure, let me revert this approach
and do as you recommended in next version.
> > So, if we
> > can create the fwnode for these irqchip using software node, we can
> > attach the same properties and the actual irqchip driver which uses the
> > fwnode doesn't need to have any ACPI vs DT checks. Same driver will work
> > seamlessly on both DT and ACPI platforms. But the challenge is,
> > currently swnode expects to be created with sysfs which won't be
> > available during early boot when irqchip drivers need to be probed. So,
> > adding support to create without dependency on sysfs help us to reuse
> > the same framework for irqchip use case also.
>
> That's another fallacy.
>
> Most irqchips *DO NOT* need to be probed early. Only the root
> irqchip. Given that this series is about *secondary* interrupt
> controllers, they absolutely don't need to be probed early.
>
Since we created swnode for root irqchip also in this approach, we had
to support early creation. With your feedback, this is no longer
required.
> To be clear: I do not intend to merge anything that:
>
> - invents yet another way to "abstract" firmware interfaces
>
> - adds more "early probe" hacks for non-primary interrupt controllers
>
> I have already said that in response to Anup's AIA series, and this
> equally applies to this series.
>
In Anup's AIA v7 series, he has made non-primary controller drivers as
platform drivers which are not probed early.
Thanks,
Sunil
On Tue, Aug 08, 2023 at 09:31:49AM +0100, Conor Dooley wrote:
> Hey Sunil,
>
> On Thu, Aug 03, 2023 at 11:29:07PM +0530, Sunil V L wrote:
> > By using swnode framework, all data from ACPI tables can
> > be populated as properties of the swnode. This simplifies
> > the driver code and removes the need for ACPI vs DT checks.
> > Use this framework for RISC-V INTC driver.
>
> btw, you are permitted to use more than 60 characters in a commit
> message...
>
Sure.
> >
> > Signed-off-by: Sunil V L <[email protected]>
> > ---
> > Documentation/riscv/acpi.rst | 21 +++++++++++++++
> > arch/riscv/include/asm/acpi.h | 1 +
> > drivers/acpi/riscv/Makefile | 2 +-
> > drivers/acpi/riscv/irqchip.c | 46 ++++++++++++++++++++++++++++++++
> > drivers/irqchip/irq-riscv-intc.c | 12 ++++-----
> > 5 files changed, 75 insertions(+), 7 deletions(-)
> > create mode 100644 drivers/acpi/riscv/irqchip.c
> >
> > diff --git a/Documentation/riscv/acpi.rst b/Documentation/riscv/acpi.rst
> > index 9870a282815b..e2406546bc16 100644
> > --- a/Documentation/riscv/acpi.rst
> > +++ b/Documentation/riscv/acpi.rst
> > @@ -8,3 +8,24 @@ The ISA string parsing rules for ACPI are defined by `Version ASCIIDOC
> > Conversion, 12/2022 of the RISC-V specifications, as defined by tag
> > "riscv-isa-release-1239329-2023-05-23" (commit 1239329
> > ) <https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-isa-release-1239329-2023-05-23>`_
> > +
> > +Interrupt Controller Drivers
> > +=======
> > +
> > +ACPI drivers for RISC-V interrupt controllers use software node framework to
> > +create the fwnode for the interrupt controllers. Below properties are
> > +additionally required for some firmware nodes apart from the properties
> > +defined by the device tree bindings for these interrupt controllers. The
> > +properties are created using the data in MADT table.
>
> I don't really understand this text, specifically what you are getting
> at w/ the dependency on devicetree properties. What exactly does "apart
> from the properties defined by the devicetree bindings" mean?
>
> Is there prior art for this kind of "ACPI needs swnodes that look
> vaguely similar to devicetree" for other interrupt controllers?
>
Never mind. This will not be required with Marc's feedback.
Thanks,
Sunil
Hi Conor,
On Tue, Aug 08, 2023 at 09:41:45AM +0100, Conor Dooley wrote:
> On Thu, Aug 03, 2023 at 11:29:15PM +0530, Sunil V L wrote:
> > Since PLIC needs to be a platform device
>
> For the unwashed, why does the PLCI need to be a platform device?
> (And while you're at that, please try to make use of the extra ~20
> characters per line that you can use here.)
>
Sure, let me add more details and use extra characters.
> > probe the
> > MADT and create platform devices for each PLIC in the
> > system. Use software node framework for the fwnode
> > which allows to create properties and hence the
> > actual irqchip driver doesn't need to do anything
> > different for ACPI vs DT.
> >
> > Signed-off-by: Sunil V L <[email protected]>
> > Co-developed-by: Haibo Xu <[email protected]>
> > Signed-off-by: Haibo Xu <[email protected]>
>
> > +static struct fwnode_handle *acpi_plic_create_fwnode(struct acpi_madt_plic *plic)
> > +{
> > + struct fwnode_handle *fwnode, *parent_fwnode;
> > + struct riscv_irqchip_list *plic_element;
> > + struct software_node_ref_args *refs;
> > + struct property_entry props[8] = {};
> > + int nr_contexts;
> > +
> > + props[0] = PROPERTY_ENTRY_U32("riscv,gsi-base", plic->gsi_base);
> > + props[1] = PROPERTY_ENTRY_U32("riscv,ndev", plic->num_irqs);
> > + props[2] = PROPERTY_ENTRY_U32("riscv,max_prio", plic->max_prio);
>
> My OCD wants to know why this gets an _ but the others have a -.
>
It should be -. But with Marc's recommendation, this is no longer
required.
Thanks!
Sunil