[v3,part1,08/11] ARM64 / ACPI: Introduce PCI functions for ACPI on ARM64

CONFIG_ACPI depends CONFIG_PCI now, and ACPI provides ACPI based
PCI hotplug and PCI host bridge hotplug, introduce some PCI
functions to make ACPI core can be compiled, some of the functions
should be revisited when implemented on ARM64.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
Signed-off-by: Al Stone <al.stone@linaro.org>
---
 arch/arm64/include/asm/pci.h |   11 +++++++++++
 arch/arm64/kernel/pci.c      |   34 ++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

On Friday 25 April 2014, Hanjun Guo wrote:
> CONFIG_ACPI depends CONFIG_PCI now, and ACPI provides ACPI based
> PCI hotplug and PCI host bridge hotplug, introduce some PCI
> functions to make ACPI core can be compiled, some of the functions
> should be revisited when implemented on ARM64.

> diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
> index d93576f..0aa3607 100644
> --- a/arch/arm64/include/asm/pci.h
> +++ b/arch/arm64/include/asm/pci.h
> @@ -21,6 +21,17 @@ struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus);
>  #define pcibios_assign_all_busses() \
>  	(pci_has_flag(PCI_REASSIGN_ALL_BUS))
>  
> +static inline void pcibios_penalize_isa_irq(int irq, int active)
> +{
> +	/* we don't do dynamic pci irq allocation */
> +}
> +
> +static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
> +{
> +	/* no legacy IRQ on arm64 */
> +	return -ENODEV;
> +}
> +

I think these would be better addressed in the caller. From what I can tell, they
are only called by the ISAPNP code path for doing IRQ resource allocation, while
the ACPIPNP code uses hardwired IRQ resources.

> @@ -171,3 +172,36 @@ unsigned long pci_ioremap_io(const struct resource *res, phys_addr_t phys_addr)
>  	/* return io_offset */
>  	return start * PAGE_SIZE - res->start;
>  }
> +
> +/*
> + * raw_pci_read - Platform-specific PCI config space access.
> + *
> + * Default empty implementation.  Replace with an architecture-specific setup
> + * routine, if necessary.
> + */
> +int raw_pci_read(unsigned int domain, unsigned int bus,
> +		  unsigned int devfn, int reg, int len, u32 *val)
> +{
> +	return -EINVAL;
> +}
> +
> +int raw_pci_write(unsigned int domain, unsigned int bus,
> +		  unsigned int devfn, int reg, int len, u32 val)
> +{
> +	return -EINVAL;
> +}
> +
> +#ifdef CONFIG_ACPI
> +/* Root bridge scanning */
> +struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
> +{
> +	/* TODO: Should be revisited when implementing PCI on ACPI */
> +	return NULL;
> +}
> +
> +void __init pci_acpi_crs_quirks(void)
> +{
> +	/* Do nothing on ARM64 */
> +	return;
> +}
> +#endif

And these probably don't need to be done at the architecture level. I expect we
will only have to worry about SBSA compliant PCI hosts, so this can be done in
the host controller driver for compliant devices, which is probably the one
that Will Deacon is working on already.

Note that we are aiming for an empty PCI implementation in ARM64, doing everything
in either the PCI core or in the individual host drivers.

For pci_acpi_crs_quirks(), it's probably enough to stub out the declaration and
to make it x86 specific. ia64 already doesn't use it, and we can hope we won't
need it for arm64 either.

For raw_pci_{read,write} we can have a trivial generic implementation in
the PCI core, like

int __weak raw_pci_read(unsigned int domain, unsigned int bus,
		  unsigned int devfn, int reg, int len, u32 *val)
{
	struct pci_bus *bus = pci_find_bus(domain, bus);
	if (!bus)
		return -ENODEV;

	return bus->ops->read(bus, devfn, reg, len, val);
}

which won't work on x86 or ia64, but should be fine everywhere else. Alternatively,
you can change the ACPI code to do the above manually and call the architecture
independent interfaces, either bus->ops->read, or pci_bus_read_config_{byte,word,dword},
which would actually simplify the ACPI code.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Arnd,

On 2014-4-28 21:49, Arnd Bergmann wrote:
> On Friday 25 April 2014, Hanjun Guo wrote:
>> CONFIG_ACPI depends CONFIG_PCI now, and ACPI provides ACPI based
>> PCI hotplug and PCI host bridge hotplug, introduce some PCI
>> functions to make ACPI core can be compiled, some of the functions
>> should be revisited when implemented on ARM64.
> 
>> diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
>> index d93576f..0aa3607 100644
>> --- a/arch/arm64/include/asm/pci.h
>> +++ b/arch/arm64/include/asm/pci.h
>> @@ -21,6 +21,17 @@ struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus);
>>  #define pcibios_assign_all_busses() \
>>  	(pci_has_flag(PCI_REASSIGN_ALL_BUS))
>>  
>> +static inline void pcibios_penalize_isa_irq(int irq, int active)
>> +{
>> +	/* we don't do dynamic pci irq allocation */
>> +}
>> +
>> +static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
>> +{
>> +	/* no legacy IRQ on arm64 */
>> +	return -ENODEV;
>> +}
>> +
> 
> I think these would be better addressed in the caller. From what I can tell, they
> are only called by the ISAPNP code path for doing IRQ resource allocation, while
> the ACPIPNP code uses hardwired IRQ resources.

I agree. pcibios_penalize_isa_irq() is only used by x86 and I will send out a
patch to make pcibios_penalize_isa_irq() as __weak in PCI core and remove
all the stub function out except x86. For pci_get_legacy_ide_irq(), I think we
can fix it in the same way, does this make sense to you?

> 
>> @@ -171,3 +172,36 @@ unsigned long pci_ioremap_io(const struct resource *res, phys_addr_t phys_addr)
>>  	/* return io_offset */
>>  	return start * PAGE_SIZE - res->start;
>>  }
>> +
>> +/*
>> + * raw_pci_read - Platform-specific PCI config space access.
>> + *
>> + * Default empty implementation.  Replace with an architecture-specific setup
>> + * routine, if necessary.
>> + */
>> +int raw_pci_read(unsigned int domain, unsigned int bus,
>> +		  unsigned int devfn, int reg, int len, u32 *val)
>> +{
>> +	return -EINVAL;
>> +}
>> +
>> +int raw_pci_write(unsigned int domain, unsigned int bus,
>> +		  unsigned int devfn, int reg, int len, u32 val)
>> +{
>> +	return -EINVAL;
>> +}
>> +
>> +#ifdef CONFIG_ACPI
>> +/* Root bridge scanning */
>> +struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>> +{
>> +	/* TODO: Should be revisited when implementing PCI on ACPI */
>> +	return NULL;
>> +}
>> +
>> +void __init pci_acpi_crs_quirks(void)
>> +{
>> +	/* Do nothing on ARM64 */
>> +	return;
>> +}
>> +#endif
> 
> And these probably don't need to be done at the architecture level. I expect we
> will only have to worry about SBSA compliant PCI hosts, so this can be done in
> the host controller driver for compliant devices, which is probably the one
> that Will Deacon is working on already.
> 
> Note that we are aiming for an empty PCI implementation in ARM64, doing everything
> in either the PCI core or in the individual host drivers.

Ok, I will review Will Deacon's patch to find out the solution for
pci_acpi_scan_root().

> 
> For pci_acpi_crs_quirks(), it's probably enough to stub out the declaration and
> to make it x86 specific. ia64 already doesn't use it, and we can hope we won't
> need it for arm64 either.

Yes, I agree, thanks a lot for the great suggestion :)
I will send out a patch to address this according to your advice.

> 
> For raw_pci_{read,write} we can have a trivial generic implementation in
> the PCI core, like
> 
> int __weak raw_pci_read(unsigned int domain, unsigned int bus,
> 		  unsigned int devfn, int reg, int len, u32 *val)
> {
> 	struct pci_bus *bus = pci_find_bus(domain, bus);
> 	if (!bus)
> 		return -ENODEV;
> 
> 	return bus->ops->read(bus, devfn, reg, len, val);
> }
> 
> which won't work on x86 or ia64, but should be fine everywhere else. Alternatively,
> you can change the ACPI code to do the above manually and call the architecture
> independent interfaces, either bus->ops->read, or pci_bus_read_config_{byte,word,dword},
> which would actually simplify the ACPI code.

This may not work. ACPI needs to be able to access PCI config space before
we've done a PCI bus scan and created pci_bus structures, so the pointer
of bus will be NULL.

Thanks
Hanjun

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html