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: <529F38FA.7070505@linaro.org>
Date:	Wed, 04 Dec 2013 22:15:22 +0800
From:	Hanjun Guo <hanjun.guo@...aro.org>
To:	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>
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

On 2013年12月04日 00:47, One Thousand Gnomes wrote:
>> 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 ?

ok, how about "Not all the ARM/ARM64 platforms with CONFIG_PCI enabled, introduce
stub function here in case of !CONFIG_PCI when using ACPI" ?

I will discuss with Graeme for 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.

Good point, thanks for the guidance, will update in next version.


>
> 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 ?

Sorry, I'm confused here, which 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 ?

Agreed, I will introduce arch\arm64\include\asm\pci.h to cover pcibios_penalize_isa_irq()
as ARM did, then #ifdef here can be removed.

Thanks
Hanjun

--
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/