linux-kernel - Re: [RFC part1 PATCH 1/7] ACPI: Make ACPI core running without PCI on ARM64

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20131203164748.0c19d346@alan.etchedpixels.co.uk>
Date:	Tue, 3 Dec 2013 16:47:48 +0000
From:	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>
To:	Hanjun Guo <hanjun.guo@...aro.org>
Cc:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	Russell King - ARM Linux <linux@....linux.org.uk>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	linux-acpi@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	Grant Likely <grant.likely@...aro.org>,
	Matthew Garrett <mjg59@...f.ucam.org>,
	Olof Johansson <olof@...om.net>,
	Linus Walleij <linus.walleij@...aro.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Rob Herring <rob.herring@...xeda.com>,
	Mark Rutland <mark.rutland@....com>,
	Jon Masters <jonathan@...masters.org>, patches@...aro.org,
	linux-kernel@...r.kernel.org, linaro-kernel@...ts.linaro.org,
	linaro-acpi@...ts.linaro.org,
	Graeme Gregory <graeme.gregory@...aro.org>,
	Al Stone <al.stone@...aro.org>
Subject: Re: [RFC part1 PATCH 1/7] ACPI: Make ACPI core running without PCI
 on ARM64


> diff --git a/drivers/acpi/reboot.c b/drivers/acpi/reboot.c
> index a6c77e8b..89a181f 100644
> --- a/drivers/acpi/reboot.c
> +++ b/drivers/acpi/reboot.c
> @@ -3,12 +3,43 @@
>  #include <linux/acpi.h>
>  #include <acpi/reboot.h>
>  
> +/*
> + * There are some rare cases in the ARM world with PCI is not one
> + * of the buses available to us, even though we use ACPI.

Can we have a comment that is easier to understand here and perhaps a
better function name ?

> + */
> +#ifdef CONFIG_PCI
> +static void acpi_reset_with_writing_pci_config(u64 address, u8 reset_value)
> +{
> +	struct pci_bus *bus0;
> +	unsigned int devfn;
> +
> +	/* The reset register can only live on bus 0. */
> +	bus0 = pci_find_bus(0, 0);
> +	if (!bus0)
> +		return;

So if you can't find the PCI eg because we have no PCI on the device you
return silently, but


> +static void acpi_reset_with_writing_pci_config(u64 address, u8 reset_value)
> +{
> +	pr_warn("Resetting with ACPI PCI RESET_REG failed, PCI is disabled\n");
> +	return;
> +}

the same system without CONFIG_PCI makes a noise.

What happens when you want to build a single kernel which works on both
PCI and non PCI systems. Surely the behaviour should be the same.

The other question I'd ask is given the nature of some of these bits
would it be better to have an acpi/pci.c which holds the PCI bits ?

> +		acpi_reset_with_writing_pci_config(rr->address, reset_value);
>  		break;
>  
>  	case ACPI_ADR_SPACE_SYSTEM_MEMORY:
> diff --git a/drivers/pnp/pnpacpi/rsparser.c b/drivers/pnp/pnpacpi/rsparser.c
> index 167f3d0..5804e77 100644
> --- a/drivers/pnp/pnpacpi/rsparser.c
> +++ b/drivers/pnp/pnpacpi/rsparser.c
> @@ -113,8 +113,10 @@ static int dma_flags(struct pnp_dev *dev, int type, int bus_master,
>  
>  static void pnpacpi_add_irqresource(struct pnp_dev *dev, struct resource *r)
>  {
> +#ifdef CONFIG_PCI
>  	if (!(r->flags & IORESOURCE_DISABLED))
>  		pcibios_penalize_isa_irq(r->start, 1);

Probably better avoid PCI ifdefs all over the place. Any reason the
includes for the PCI layer can't provide this as a dummy on a non-PCI
system ?

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/