Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1382924
| From | Jayachandran C <jchandra@broadcom.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API |
| Date | 2016-04-20 02:30 +0200 |
| Message-ID | <rpObL-7pt-7@gated-at.bofh.it> (permalink) |
| References | <rofpL-6tr-1@gated-at.bofh.it> <rofpM-6tr-27@gated-at.bofh.it> <rpLGV-5dm-5@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Wed, Apr 20, 2016 at 3:10 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 15 April 2016 19:06:43 Tomasz Nowicki wrote:
>> From: Jayachandran C <jchandra@broadcom.com>
>>
>> Add config option PCI_GENERIC_ECAM and file drivers/pci/ecam.c to
>> provide generic functions for accessing memory mapped PCI config space.
>>
>> The API is defined in drivers/pci/ecam.h and is written to replace the
>> API in drivers/pci/host/pci-host-common.h. The file defines a new
>> 'struct pci_config_window' to hold the information related to a PCI
>> config area and its mapping. This structure is expected to be used as
>> sysdata for controllers that have ECAM based mapping.
>>
>> Helper functions are provided to setup the mapping, free the mapping
>> and to implement the map_bus method in 'struct pci_ops'
>>
>> Signed-off-by: Jayachandran C <jchandra@broadcom.com>
>
> I've taken a fresh look now at what is going on here.
>
>> @@ -58,4 +58,9 @@ void __iomem *pci_generic_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
>> /* default ECAM ops, bus shift 20, generic read and write */
>> extern struct pci_generic_ecam_ops pci_generic_ecam_default_ops;
>>
>> +#ifdef CONFIG_PCI_HOST_GENERIC
>> +/* for DT based pci controllers that support ECAM */
>> +int pci_host_common_probe(struct platform_device *pdev,
>> + struct pci_generic_ecam_ops *ops);
>> +#endif
>> #endif
>
> This doesn't seem to belong here: just leave the declaration
> in the existing file.
This can be done, the file would just have one line so I thought
it made sense to move it to ecam.h where the struct is defined.
>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index 7a0780d..31d6eb5 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -82,6 +82,7 @@ config PCI_HOST_GENERIC
>> bool "Generic PCI host controller"
>> depends on (ARM || ARM64) && OF
>> select PCI_HOST_COMMON
>> + select PCI_GENERIC_ECAM
>> help
>> Say Y here if you want to support a simple generic PCI host
>> controller, such as the one emulated by kvmtool.
>> diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
>> index e9f850f..99d99b3 100644
>> --- a/drivers/pci/host/pci-host-common.c
>> +++ b/drivers/pci/host/pci-host-common.c
>> @@ -22,27 +22,21 @@
>> #include <linux/of_pci.h>
>> #include <linux/platform_device.h>
>>
>> -#include "pci-host-common.h"
>> +#include "../ecam.h"
>
> As mentioned, don't use headers from parent directories, anything
> that needs to be shared must go into include/linux, while the parts
> that are only needed in one directory should be declared there.
This is also ok - It can either go to pci.h or a separate pci-ecam.h
>> -static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
>> +static void gen_pci_generic_unmap_cfg(void *ptr)
>> +{
>> + pci_generic_ecam_free((struct pci_config_window *)ptr);
>> +}
>
> Why the void pointer?
devm_add_action() needs it.
>> +static struct pci_generic_ecam_ops pci_thunder_pem_ops = {
>> + .bus_shift = 24,
>> + .init = thunder_pem_init,
>> + .pci_ops = {
>> + .map_bus = pci_generic_ecam_map_bus,
>> + .read = thunder_pem_config_read,
>> + .write = thunder_pem_config_write,
>> + }
>> +};
>
> Adding the callback pointer for init here and yet another structure
> pci_config_window really seems to go too far with the number of
> abstraction levels.
The abstraction was already there in pci-host-common.h for
ops for ECAM/CAM based controllers. It made sense to move it
to ecam.h and use it for ECAM based ACPI [1].
We need to pass pci_ops, bus_shift and an additional pointer
for quirks for ECAM based host controllers. Having it as a
structure pci_generic_ecam_ops reduces the function arguments,
and also keeps most of the older API.
> I think here it makes much more sense to just implement ECAM pci_ops
> in ACPI separately, as the implementation really trivial to start with,
> and all the complexity comes just from trying to share it with other
> stuff. Doesn't ACPI already have an ECAM implementation for x86
> that you could simply use?
The implementation is extremely trivial on 64 bit, and slightly more
complex in 32bit (pci-host-common.c per bus mapping and set_pte
based mapping on x86). The generic ACPI on 64 bit is very simple
if there are no quirks,I have already posted that [2] some time back.
ACPI on x86 also has a 32 bit and a 64 bit version
(arch/x86/pci/mmconfig_{32,64}.c}. The code there is a bit messed
up and it does not make sense to share or reuse that.
There has been suggestions earlier from Bjorn on sharing the
ECAM implementation[1], which was the starting point of
doing this patch.
Overall, this patch improves config window mapping for
pci-host-common.c based drivers on 64 bit and deletes
quite a bit of duplicated code. I would argue that this makes
sense even without ACPI.
JC.
[1] https://lkml.org/lkml/2016/3/3/921
[2] http://article.gmane.org/gmane.linux.kernel.pci/47753
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH V6 00/13] Support for generic ACPI based PCI host controller Tomasz Nowicki <tn@semihalf.com> - 2016-04-15 19:10 +0200
[PATCH V6 11/13] pci, acpi: Match PCI config space accessors against platfrom specific quirks. Tomasz Nowicki <tn@semihalf.com> - 2016-04-15 19:10 +0200
Re: [PATCH V6 11/13] pci, acpi: Match PCI config space accessors against platfrom specific quirks. "liudongdong (C)" <liudongdong3@huawei.com> - 2016-04-18 14:00 +0200
Re: [PATCH V6 11/13] pci, acpi: Match PCI config space accessors against platfrom specific quirks. Tomasz Nowicki <tn@semihalf.com> - 2016-04-18 14:30 +0200
[PATCH V6 13/13] pci, pci-thunder-pem: Add ACPI support for ThunderX PEM. Tomasz Nowicki <tn@semihalf.com> - 2016-04-15 19:10 +0200
[PATCH V6 02/13] pci, acpi: Provide generic way to assign bus domain number. Tomasz Nowicki <tn@semihalf.com> - 2016-04-15 19:10 +0200
Re: [PATCH V6 02/13] pci, acpi: Provide generic way to assign bus domain number. Bjorn Helgaas <helgaas@kernel.org> - 2016-04-27 04:30 +0200
Re: [PATCH V6 02/13] pci, acpi: Provide generic way to assign bus domain number. Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-04-27 13:20 +0200
Re: [PATCH V6 02/13] pci, acpi: Provide generic way to assign bus domain number. Bjorn Helgaas <helgaas@kernel.org> - 2016-04-27 18:50 +0200
Re: [PATCH V6 02/13] pci, acpi: Provide generic way to assign bus domain number. Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-04-27 19:40 +0200
Re: [PATCH V6 02/13] pci, acpi: Provide generic way to assign bus domain number. Liviu.Dudau@arm.com - 2016-04-28 10:20 +0200
Re: [PATCH V6 02/13] pci, acpi: Provide generic way to assign bus domain number. Tomasz Nowicki <tn@semihalf.com> - 2016-04-27 14:10 +0200
[PATCH V6 12/13] pci, pci-thunder-ecam: Add ACPI support for ThunderX ECAM. Tomasz Nowicki <tn@semihalf.com> - 2016-04-15 19:10 +0200
Re: [PATCH V6 12/13] pci, pci-thunder-ecam: Add ACPI support for ThunderX ECAM. Tomasz Nowicki <tn@semihalf.com> - 2016-04-19 12:30 +0200
Re: [Linaro-acpi] [PATCH V6 12/13] pci, pci-thunder-ecam: Add ACPI support for ThunderX ECAM. G Gregory <graeme.gregory@linaro.org> - 2016-04-19 12:50 +0200
Re: [Linaro-acpi] [PATCH V6 12/13] pci, pci-thunder-ecam: Add ACPI support for ThunderX ECAM. Graeme Gregory <gg@slimlogic.co.uk> - 2016-04-19 13:20 +0200
Re: [Linaro-acpi] [PATCH V6 12/13] pci, pci-thunder-ecam: Add ACPI support for ThunderX ECAM. Tomasz Nowicki <tn@semihalf.com> - 2016-04-19 13:30 +0200
Re: [Linaro-acpi] [PATCH V6 12/13] pci, pci-thunder-ecam: Add ACPI support for ThunderX ECAM. G Gregory <graeme.gregory@linaro.org> - 2016-04-19 14:30 +0200
[PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Tomasz Nowicki <tn@semihalf.com> - 2016-04-15 19:10 +0200
Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Arnd Bergmann <arnd@arndb.de> - 2016-04-15 20:50 +0200
Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Jayachandran C <jchandra@broadcom.com> - 2016-04-16 09:30 +0200
Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Arnd Bergmann <arnd@arndb.de> - 2016-04-16 09:40 +0200
Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Jayachandran C <jchandra@broadcom.com> - 2016-04-16 16:40 +0200
Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Tomasz Nowicki <tn@semihalf.com> - 2016-04-18 15:10 +0200
Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Arnd Bergmann <arnd@arndb.de> - 2016-04-18 16:50 +0200
Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Tomasz Nowicki <tn@semihalf.com> - 2016-04-18 21:40 +0200
Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Arnd Bergmann <arnd@arndb.de> - 2016-04-19 15:10 +0200
Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Tomasz Nowicki <tn@semihalf.com> - 2016-04-21 11:30 +0200
Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Arnd Bergmann <arnd@arndb.de> - 2016-04-21 11:40 +0200
Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Tomasz Nowicki <tn@semihalf.com> - 2016-04-21 12:10 +0200
Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Jon Masters <jcm@redhat.com> - 2016-04-22 16:40 +0200
Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API David Daney <ddaney.cavm@gmail.com> - 2016-04-22 18:10 +0200
Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Arnd Bergmann <arnd@arndb.de> - 2016-04-19 23:50 +0200
Re: [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Jayachandran C <jchandra@broadcom.com> - 2016-04-20 02:30 +0200
[PATCH V6 04/13] pci, of: Move the PCI I/O space management to PCI core code. Tomasz Nowicki <tn@semihalf.com> - 2016-04-15 19:10 +0200
[PATCH V6 09/13] pci, acpi: Support for ACPI based generic PCI host controller Tomasz Nowicki <tn@semihalf.com> - 2016-04-15 19:10 +0200
Re: [PATCH V6 09/13] pci, acpi: Support for ACPI based generic PCI host controller Jayachandran C <jchandra@broadcom.com> - 2016-04-20 21:20 +0200
Re: [PATCH V6 09/13] pci, acpi: Support for ACPI based generic PCI host controller Tomasz Nowicki <tn@semihalf.com> - 2016-04-21 11:10 +0200
Re: [PATCH V6 09/13] pci, acpi: Support for ACPI based generic PCI host controller Jayachandran C <jchandra@broadcom.com> - 2016-04-22 15:00 +0200
Re: [PATCH V6 09/13] pci, acpi: Support for ACPI based generic PCI host controller Jon Masters <jcm@redhat.com> - 2016-04-22 16:50 +0200
Re: [PATCH V6 09/13] pci, acpi: Support for ACPI based generic PCI host controller Jon Masters <jcm@redhat.com> - 2016-04-23 17:30 +0200
[PATCH V6 10/13] arm64, pci, acpi: Start using ACPI based PCI host controller driver for ARM64. Tomasz Nowicki <tn@semihalf.com> - 2016-04-15 19:10 +0200
[PATCH V6 06/13] arm64, pci, acpi: ACPI support for legacy IRQs parsing and consolidation with DT code. Tomasz Nowicki <tn@semihalf.com> - 2016-04-15 19:20 +0200
Re: [PATCH V6 06/13] arm64, pci, acpi: ACPI support for legacy IRQs parsing and consolidation with DT code. Bjorn Helgaas <helgaas@kernel.org> - 2016-04-27 04:50 +0200
Re: [PATCH V6 06/13] arm64, pci, acpi: ACPI support for legacy IRQs parsing and consolidation with DT code. Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-04-27 13:50 +0200
[PATCH V6 07/13] PCI: Provide common functions for ECAM mapping Tomasz Nowicki <tn@semihalf.com> - 2016-04-15 19:20 +0200
Re: [PATCH V6 07/13] PCI: Provide common functions for ECAM mapping Arnd Bergmann <arnd@arndb.de> - 2016-04-15 20:50 +0200
[PATCH V6 01/13] pci, acpi, x86, ia64: Move ACPI host bridge device companion assignment to core code. Tomasz Nowicki <tn@semihalf.com> - 2016-04-15 19:20 +0200
Re: [PATCH V6 01/13] pci, acpi, x86, ia64: Move ACPI host bridge device companion assignment to core code. Bjorn Helgaas <helgaas@kernel.org> - 2016-04-27 00:40 +0200
Re: [PATCH V6 01/13] pci, acpi, x86, ia64: Move ACPI host bridge device companion assignment to core code. Tomasz Nowicki <tn@semihalf.com> - 2016-04-27 12:20 +0200
Re: [PATCH V6 01/13] pci, acpi, x86, ia64: Move ACPI host bridge device companion assignment to core code. Bjorn Helgaas <helgaas@kernel.org> - 2016-04-27 04:50 +0200
[PATCH V6 03/13] x86, ia64: Include acpi_pci_{add|remove}_bus to the default pcibios_{add|remove}_bus implementation. Tomasz Nowicki <tn@semihalf.com> - 2016-04-15 19:20 +0200
Re: [PATCH V6 03/13] x86, ia64: Include acpi_pci_{add|remove}_bus to the default pcibios_{add|remove}_bus implementation. Bjorn Helgaas <helgaas@kernel.org> - 2016-04-27 04:40 +0200
Re: [PATCH V6 03/13] x86, ia64: Include acpi_pci_{add|remove}_bus to the default pcibios_{add|remove}_bus implementation. Tomasz Nowicki <tn@semihalf.com> - 2016-04-27 15:30 +0200
[PATCH V6 05/13] acpi, pci: Support IO resources when parsing PCI host bridge resources. Tomasz Nowicki <tn@semihalf.com> - 2016-04-15 19:20 +0200
Re: [PATCH V6 05/13] acpi, pci: Support IO resources when parsing PCI host bridge resources. Bjorn Helgaas <helgaas@kernel.org> - 2016-04-27 04:40 +0200
Re: [PATCH V6 05/13] acpi, pci: Support IO resources when parsing PCI host bridge resources. Jon Masters <jcm@redhat.com> - 2016-04-27 07:40 +0200
Re: [PATCH V6 05/13] acpi, pci: Support IO resources when parsing PCI host bridge resources. Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-04-27 16:30 +0200
Re: [PATCH V6 05/13] acpi, pci: Support IO resources when parsing PCI host bridge resources. Liviu.Dudau@arm.com - 2016-04-27 17:20 +0200
Re: [PATCH V6 05/13] acpi, pci: Support IO resources when parsing PCI host bridge resources. Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-04-27 18:10 +0200
Re: [PATCH V6 00/13] Support for generic ACPI based PCI host controller Jon Masters <jcm@redhat.com> - 2016-04-15 20:20 +0200
Re: [PATCH V6 00/13] Support for generic ACPI based PCI host controller Jayachandran C <jchandra@broadcom.com> - 2016-04-16 17:40 +0200
Re: [PATCH V6 00/13] Support for generic ACPI based PCI host controller Tomasz Nowicki <tn@semihalf.com> - 2016-04-18 15:40 +0200
Re: [PATCH V6 00/13] Support for generic ACPI based PCI host controller Arnd Bergmann <arnd@arndb.de> - 2016-04-18 16:40 +0200
Re: [PATCH V6 00/13] Support for generic ACPI based PCI host controller Tomasz Nowicki <tn@semihalf.com> - 2016-04-18 17:30 +0200
Re: [PATCH V6 00/13] Support for generic ACPI based PCI host controller Martinez Kristofer <kristofer.s.martinez@gmail.com> - 2016-04-17 11:30 +0200
Re: [PATCH V6 00/13] Support for generic ACPI based PCI host controller Duc Dang <dhdang@apm.com> - 2016-04-16 20:40 +0200
Re: [PATCH V6 00/13] Support for generic ACPI based PCI host controller Sinan Kaya <okaya@codeaurora.org> - 2016-04-17 06:20 +0200
Re: [PATCH V6 00/13] Support for generic ACPI based PCI host controller Jeremy Linton <jeremy.linton@arm.com> - 2016-04-25 19:30 +0200
csiph-web