Re: [PATCH 06/24] RISC-V: ACPI: Add PCI functions to build ACPI core

[Jessica Clarke]

On 30 Jan 2023, at 18:22, Sunil V L <sunilvl@xxxxxxxxxxxxxxxx> wrote:
> 
> When CONFIG_PCI is enabled, ACPI core expects few arch
> functions related to PCI. Add those functions so that
> ACPI core gets build. These are levraged from arm64.

Presumably this is pretty generic and applies to anything without x86
weirdness. Copying all this supposedly architecture specific code
that’s really generic seems like bad practice to me as an outsider.
Should this not be unifying the two in a shared location as has been
done for other subsystems?

Jess

> Signed-off-by: Sunil V L <sunilvl@xxxxxxxxxxxxxxxx>
> ---
> arch/riscv/kernel/Makefile |   1 +
> arch/riscv/kernel/pci.c    | 173 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 174 insertions(+)
> create mode 100644 arch/riscv/kernel/pci.c
> 
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index f979dc8cf47d..e9d37639751d 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -92,3 +92,4 @@ obj-$(CONFIG_COMPAT)		+= compat_signal.o
> obj-$(CONFIG_COMPAT)		+= compat_vdso/
> 
> obj-$(CONFIG_ACPI)              += acpi.o
> +obj-$(CONFIG_PCI)               += pci.o
> diff --git a/arch/riscv/kernel/pci.c b/arch/riscv/kernel/pci.c
> new file mode 100644
> index 000000000000..3388af3a67a0
> --- /dev/null
> +++ b/arch/riscv/kernel/pci.c
> @@ -0,0 +1,173 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Code borrowed from ARM64
> + *
> + * Copyright (C) 2003 Anton Blanchard <anton@xxxxxxxxxx>, IBM
> + * Copyright (C) 2014 ARM Ltd.
> + * Copyright (C) 2022-2023 Ventana Micro System Inc.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/mm.h>
> +#include <linux/pci.h>
> +#include <linux/pci-acpi.h>
> +#include <linux/pci-ecam.h>
> +
> +#ifdef CONFIG_ACPI
> +
> +/*
> + * raw_pci_read/write - Platform-specific PCI config space access.
> + */
> +int raw_pci_read(unsigned int domain, unsigned int bus,
> +		  unsigned int devfn, int reg, int len, u32 *val)
> +{
> +	struct pci_bus *b = pci_find_bus(domain, bus);
> +
> +	if (!b)
> +		return PCIBIOS_DEVICE_NOT_FOUND;
> +	return b->ops->read(b, devfn, reg, len, val);
> +}
> +
> +int raw_pci_write(unsigned int domain, unsigned int bus,
> +		unsigned int devfn, int reg, int len, u32 val)
> +{
> +	struct pci_bus *b = pci_find_bus(domain, bus);
> +
> +	if (!b)
> +		return PCIBIOS_DEVICE_NOT_FOUND;
> +	return b->ops->write(b, devfn, reg, len, val);
> +}
> +
> +
> +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;
> +}
> +
> +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;
> +}
> +
> +#endif
> -- 
> 2.38.0
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-riscv