public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: tn@semihalf•com (Tomasz Nowicki)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH V6 01/13] pci, acpi, x86, ia64: Move ACPI host bridge device companion assignment to core code.
Date: Wed, 4 May 2016 10:10:58 +0200	[thread overview]
Message-ID: <5729AE92.2090106@semihalf.com> (raw)
In-Reply-To: <20160427024537.GH6789@localhost>

On 27.04.2016 04:45, Bjorn Helgaas wrote:
> [question for Rafael below]
>
> On Fri, Apr 15, 2016 at 07:06:36PM +0200, Tomasz Nowicki wrote:
>> Currently we have two platforms (x86 & ia64) capable of PCI ACPI host
>> bridge initialization. They both use arch-specific sysdata to pass down
>> parent device reference and both rely on NULL parent in pci_create_root_bus()
>> to validate sysdata content.
>>
>> It looks hacky and prevents us from getting some firmware specific
>> info for PCI host controller based on its acpi_device structure
>> in generic pci_create_root_bus() function. However, we overcome that
>> blocker by passing down parent device via pci_create_root_bus parameter
>> (as the ACPI device type). Then we use ACPI_COMPANION_SET in core code
>> for ACPI boot method only. ACPI_COMPANION_SET is safe to run for all
>> cases DT, ACPI and DT&ACPI.
>>
>> Since now PCI core code is setting ACPI companion device for us,
>> x86 & ia64 specific ACPI companion device setting turns out to be dead now.
>> We can get rid of it, including related companion reference from
>> PCI sysdata structure. Aslo, PCI_CONTROLLER macro cannot return valid
>> companion device anymore. Therefore we need to convert its usage to
>> ACPI_COMPANION.
>>
>> Suggested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm•com>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf•com>
>> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm•com>
>> Tested-by: Duc Dang <dhdang@apm•com>
>> Tested-by: Dongdong Liu <liudongdong3@huawei•com>
>> Tested-by: Hanjun Guo <hanjun.guo@linaro•org>
>> Tested-by: Graeme Gregory <graeme.gregory@linaro•org>
>> Tested-by: Sinan Kaya <okaya@codeaurora•org>
>> ---
>>   arch/ia64/hp/common/sba_iommu.c    |  2 +-
>>   arch/ia64/include/asm/pci.h        |  1 -
>>   arch/ia64/pci/pci.c                | 16 ----------------
>>   arch/ia64/sn/kernel/io_acpi_init.c |  4 ++--
>>   arch/x86/include/asm/pci.h         |  3 ---
>>   arch/x86/pci/acpi.c                | 17 -----------------
>>   drivers/acpi/pci_root.c            |  7 ++++++-
>>   drivers/pci/probe.c                |  2 ++
>>   8 files changed, 11 insertions(+), 41 deletions(-)
>>
>> diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
>> index a6d6190..78e4444 100644
>> --- a/arch/ia64/hp/common/sba_iommu.c
>> +++ b/arch/ia64/hp/common/sba_iommu.c
>> @@ -1981,7 +1981,7 @@ sba_connect_bus(struct pci_bus *bus)
>>   	if (PCI_CONTROLLER(bus)->iommu)
>>   		return;
>>
>> -	handle = acpi_device_handle(PCI_CONTROLLER(bus)->companion);
>> +	handle = acpi_device_handle(ACPI_COMPANION(bus->bridge));
>>   	if (!handle)
>>   		return;
>>
>> diff --git a/arch/ia64/include/asm/pci.h b/arch/ia64/include/asm/pci.h
>> index c0835b0..12423f4 100644
>> --- a/arch/ia64/include/asm/pci.h
>> +++ b/arch/ia64/include/asm/pci.h
>> @@ -63,7 +63,6 @@ extern int pci_mmap_legacy_page_range(struct pci_bus *bus,
>>   #define pci_legacy_write platform_pci_legacy_write
>>
>>   struct pci_controller {
>> -	struct acpi_device *companion;
>>   	void *iommu;
>>   	int segment;
>>   	int node;		/* nearest node with memory or NUMA_NO_NODE for global allocation */
>> diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
>> index 8f6ac2f..978d6af 100644
>> --- a/arch/ia64/pci/pci.c
>> +++ b/arch/ia64/pci/pci.c
>> @@ -301,28 +301,12 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>>   	}
>>
>>   	info->controller.segment = root->segment;
>> -	info->controller.companion = device;
>>   	info->controller.node = acpi_get_node(device->handle);
>>   	INIT_LIST_HEAD(&info->io_resources);
>>   	return acpi_pci_root_create(root, &pci_acpi_root_ops,
>>   				    &info->common, &info->controller);
>>   }
>>
>> -int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
>> -{
>> -	/*
>> -	 * We pass NULL as parent to pci_create_root_bus(), so if it is not NULL
>> -	 * here, pci_create_root_bus() has been called by someone else and
>> -	 * sysdata is likely to be different from what we expect.  Let it go in
>> -	 * that case.
>> -	 */
>> -	if (!bridge->dev.parent) {
>> -		struct pci_controller *controller = bridge->bus->sysdata;
>> -		ACPI_COMPANION_SET(&bridge->dev, controller->companion);
>> -	}
>> -	return 0;
>> -}
>> -
>>   void pcibios_fixup_device_resources(struct pci_dev *dev)
>>   {
>>   	int idx;
>> diff --git a/arch/ia64/sn/kernel/io_acpi_init.c b/arch/ia64/sn/kernel/io_acpi_init.c
>> index 231234c..e454492 100644
>> --- a/arch/ia64/sn/kernel/io_acpi_init.c
>> +++ b/arch/ia64/sn/kernel/io_acpi_init.c
>> @@ -132,7 +132,7 @@ sn_get_bussoft_ptr(struct pci_bus *bus)
>>   	struct acpi_resource_vendor_typed *vendor;
>>
>>
>> -	handle = acpi_device_handle(PCI_CONTROLLER(bus)->companion);
>> +	handle = acpi_device_handle(ACPI_COMPANION(bus->bridge));
>>   	status = acpi_get_vendor_resource(handle, METHOD_NAME__CRS,
>>   					  &sn_uuid, &buffer);
>>   	if (ACPI_FAILURE(status)) {
>> @@ -360,7 +360,7 @@ sn_acpi_get_pcidev_info(struct pci_dev *dev, struct pcidev_info **pcidev_info,
>>   	acpi_status status;
>>   	struct acpi_buffer name_buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>>
>> -	rootbus_handle = acpi_device_handle(PCI_CONTROLLER(dev)->companion);
>> +	rootbus_handle = acpi_device_handle(ACPI_COMPANION(dev->bus->bridge));
>>           status = acpi_evaluate_integer(rootbus_handle, METHOD_NAME__SEG, NULL,
>>                                          &segment);
>>           if (ACPI_SUCCESS(status)) {
>> diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
>> index 9ab7507..24de07d 100644
>> --- a/arch/x86/include/asm/pci.h
>> +++ b/arch/x86/include/asm/pci.h
>> @@ -14,9 +14,6 @@
>>   struct pci_sysdata {
>>   	int		domain;		/* PCI domain */
>>   	int		node;		/* NUMA node */
>> -#ifdef CONFIG_ACPI
>> -	struct acpi_device *companion;	/* ACPI companion device */
>> -#endif
>>   #ifdef CONFIG_X86_64
>>   	void		*iommu;		/* IOMMU private data */
>>   #endif
>> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
>> index 3cd6983..f4ca17a 100644
>> --- a/arch/x86/pci/acpi.c
>> +++ b/arch/x86/pci/acpi.c
>> @@ -340,7 +340,6 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>>   		struct pci_sysdata sd = {
>>   			.domain = domain,
>>   			.node = node,
>> -			.companion = root->device
>>   		};
>>
>>   		memcpy(bus->sysdata, &sd, sizeof(sd));
>> @@ -355,7 +354,6 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>>   		else {
>>   			info->sd.domain = domain;
>>   			info->sd.node = node;
>> -			info->sd.companion = root->device;
>>   			bus = acpi_pci_root_create(root, &acpi_pci_root_ops,
>>   						   &info->common, &info->sd);
>>   		}
>> @@ -373,21 +371,6 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>>   	return bus;
>>   }
>>
>> -int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
>> -{
>> -	/*
>> -	 * We pass NULL as parent to pci_create_root_bus(), so if it is not NULL
>> -	 * here, pci_create_root_bus() has been called by someone else and
>> -	 * sysdata is likely to be different from what we expect.  Let it go in
>> -	 * that case.
>> -	 */
>> -	if (!bridge->dev.parent) {
>> -		struct pci_sysdata *sd = bridge->bus->sysdata;
>> -		ACPI_COMPANION_SET(&bridge->dev, sd->companion);
>> -	}
>> -	return 0;
>> -}
>> -
>>   int __init pci_acpi_init(void)
>>   {
>>   	struct pci_dev *dev = NULL;
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index ae3fe4e..4581e0e 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
>> @@ -564,6 +564,11 @@ static int acpi_pci_root_add(struct acpi_device *device,
>>   		}
>>   	}
>>
>> +	/*
>> +	 * pci_create_root_bus() needs to detect the parent device type,
>> +	 * so initialize its companion data accordingly.
>> +	 */
>> +	ACPI_COMPANION_SET(&device->dev, device);
>>   	root->device = device;
>>   	root->segment = segment & 0xFFFF;
>>   	strcpy(acpi_device_name(device), ACPI_PCI_ROOT_DEVICE_NAME);
>> @@ -846,7 +851,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>>
>>   	pci_acpi_root_add_resources(info);
>>   	pci_add_resource(&info->resources, &root->secondary);
>> -	bus = pci_create_root_bus(NULL, busnum, ops->pci_ops,
>> +	bus = pci_create_root_bus(&device->dev, busnum, ops->pci_ops,
>>   				  sysdata, &info->resources);
>
> "device" here is a struct acpi_device *.  Rafael, is that the right
> thing to do?  I dimly recall proposing something similar long ago and
> that it turned out to be a bad idea.
>

Joining Bjorn's question. Rafael, do you recall what was the issue here 
from the past?

Thanks,
Tomasz

  reply	other threads:[~2016-05-04  8:10 UTC|newest]

Thread overview: 105+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-15 17:06 [PATCH V6 00/13] Support for generic ACPI based PCI host controller Tomasz Nowicki
2016-04-15 17:06 ` [PATCH V6 01/13] pci, acpi, x86, ia64: Move ACPI host bridge device companion assignment to core code Tomasz Nowicki
2016-04-15 20:41   ` kbuild test robot
2016-04-26 22:36     ` Bjorn Helgaas
2016-04-27 10:12       ` Tomasz Nowicki
2016-04-27  2:45   ` Bjorn Helgaas
2016-05-04  8:10     ` Tomasz Nowicki [this message]
2016-05-09 22:18       ` Rafael J. Wysocki
2016-05-10 10:27         ` Lorenzo Pieralisi
2016-05-09 22:56   ` Rafael J. Wysocki
2016-05-10  1:53     ` Bjorn Helgaas
2016-05-10 10:07       ` Lorenzo Pieralisi
2016-04-15 17:06 ` [PATCH V6 02/13] pci, acpi: Provide generic way to assign bus domain number Tomasz Nowicki
2016-04-27  2:26   ` Bjorn Helgaas
2016-04-27 11:17     ` Lorenzo Pieralisi
2016-04-27 16:44       ` Bjorn Helgaas
2016-04-27 17:31         ` Lorenzo Pieralisi
2016-04-28  8:13           ` Liviu.Dudau at arm.com
2016-04-28 15:12           ` Bjorn Helgaas
2016-04-28 15:34             ` Arnd Bergmann
2016-04-29 22:50               ` Arnd Bergmann
2016-05-02 12:43       ` Tomasz Nowicki
2016-05-02 13:26         ` Jayachandran C
2016-05-03 11:02           ` Lorenzo Pieralisi
2016-05-03 14:22             ` Jayachandran C
2016-05-03 14:55               ` Lorenzo Pieralisi
2016-04-27 11:59     ` Tomasz Nowicki
2016-04-15 17:06 ` [PATCH V6 03/13] x86, ia64: Include acpi_pci_{add|remove}_bus to the default pcibios_{add|remove}_bus implementation Tomasz Nowicki
2016-04-27  2:34   ` Bjorn Helgaas
2016-04-27 13:19     ` Tomasz Nowicki
2016-04-15 17:06 ` [PATCH V6 04/13] pci, of: Move the PCI I/O space management to PCI core code Tomasz Nowicki
2016-04-15 17:06 ` [PATCH V6 05/13] acpi, pci: Support IO resources when parsing PCI host bridge resources Tomasz Nowicki
2016-04-27  2:39   ` Bjorn Helgaas
2016-04-27  5:36     ` Jon Masters
2016-04-28 21:53       ` Jon Masters
2016-04-27 14:26     ` Lorenzo Pieralisi
2016-04-27 15:10       ` Liviu.Dudau at arm.com
2016-04-27 16:09         ` Lorenzo Pieralisi
2016-04-28 15:45       ` Bjorn Helgaas
2016-04-15 17:06 ` [PATCH V6 06/13] arm64, pci, acpi: ACPI support for legacy IRQs parsing and consolidation with DT code Tomasz Nowicki
2016-04-27  2:44   ` Bjorn Helgaas
2016-04-27 11:46     ` Lorenzo Pieralisi
2016-04-15 17:06 ` [PATCH V6 07/13] PCI: Provide common functions for ECAM mapping Tomasz Nowicki
2016-04-15 18:41   ` Arnd Bergmann
2016-04-28 21:47   ` Bjorn Helgaas
2016-04-29  8:01     ` Jayachandran C
2016-05-05  9:24       ` Jayachandran C
2016-05-05 10:38         ` Tomasz Nowicki
2016-04-15 17:06 ` [PATCH V6 08/13] PCI: generic, thunder: update to use generic ECAM API Tomasz Nowicki
2016-04-15 18:39   ` Arnd Bergmann
2016-04-16  7:20     ` Jayachandran C
2016-04-16  7:31       ` Arnd Bergmann
2016-04-16 14:36         ` Jayachandran C
2016-04-18 13:03           ` Tomasz Nowicki
2016-04-18 14:44             ` Arnd Bergmann
2016-04-18 19:31               ` Tomasz Nowicki
2016-04-19 13:06                 ` Arnd Bergmann
2016-04-21  9:28                   ` Tomasz Nowicki
2016-04-21  9:36                     ` Arnd Bergmann
2016-04-21 10:08                       ` Tomasz Nowicki
2016-04-22 14:30                         ` Jon Masters
2016-04-22 16:00                           ` David Daney
2016-04-28 20:14                       ` Bjorn Helgaas
2016-04-28 20:40                         ` Arnd Bergmann
2016-04-28 21:18                           ` Bjorn Helgaas
2016-04-28 21:47                             ` Jon Masters
2016-04-29  9:41                               ` Lorenzo Pieralisi
2016-04-19 21:40   ` Arnd Bergmann
2016-04-20  0:22     ` Jayachandran C
2016-04-15 17:06 ` [PATCH V6 09/13] pci, acpi: Support for ACPI based generic PCI host controller Tomasz Nowicki
2016-04-20 19:12   ` Jayachandran C
2016-04-21  9:06     ` Tomasz Nowicki
2016-04-22 12:49       ` Jayachandran C
2016-04-22 14:40       ` Jon Masters
2016-04-23 15:23         ` Jon Masters
2016-04-28 21:48   ` Bjorn Helgaas
2016-04-29  8:37     ` Lorenzo Pieralisi
2016-04-29 17:35       ` Jayachandran C
2016-05-02 11:31         ` Tomasz Nowicki
2016-05-03  8:46         ` Lorenzo Pieralisi
2016-05-02 11:03     ` Tomasz Nowicki
2016-04-15 17:06 ` [PATCH V6 10/13] arm64, pci, acpi: Start using ACPI based PCI host controller driver for ARM64 Tomasz Nowicki
2016-04-15 17:06 ` [PATCH V6 11/13] pci, acpi: Match PCI config space accessors against platfrom specific quirks Tomasz Nowicki
2016-04-18 11:37   ` liudongdong (C)
2016-04-18 12:21     ` Tomasz Nowicki
2016-04-15 17:06 ` [PATCH V6 12/13] pci, pci-thunder-ecam: Add ACPI support for ThunderX ECAM Tomasz Nowicki
2016-04-19 10:26   ` Tomasz Nowicki
2016-04-19 10:41     ` [Linaro-acpi] " G Gregory
2016-04-19 11:12       ` Graeme Gregory
2016-04-19 11:22         ` Tomasz Nowicki
2016-04-19 12:29           ` G Gregory
2016-04-15 17:06 ` [PATCH V6 13/13] pci, pci-thunder-pem: Add ACPI support for ThunderX PEM Tomasz Nowicki
2016-04-15 18:19 ` [PATCH V6 00/13] Support for generic ACPI based PCI host controller Jon Masters
2016-04-16 15:31   ` Jayachandran C
2016-04-18 13:33     ` Tomasz Nowicki
2016-04-18 14:38       ` Arnd Bergmann
2016-04-18 15:26         ` Tomasz Nowicki
2016-04-18 16:14           ` Mark Langsdorf
2016-04-17  9:23   ` Martinez Kristofer
2016-04-16 18:30 ` Duc Dang
2016-04-17  4:18 ` Sinan Kaya
2016-04-22 16:08 ` Robert Richter
2016-04-22 20:46 ` Suravee Suthikulpanit
2016-04-25 17:23 ` Jeremy Linton
2016-04-26  9:07 ` liudongdong (C)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5729AE92.2090106@semihalf.com \
    --to=tn@semihalf$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox