From: Tomasz Nowicki <tn@semihalf.com>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: helgaas@kernel.org, arnd@arndb.de, will.deacon@arm.com,
catalin.marinas@arm.com, rafael@kernel.org,
hanjun.guo@linaro.org, okaya@codeaurora.org,
jchandra@broadcom.com, robert.richter@caviumnetworks.com,
mw@semihalf.com, Liviu.Dudau@arm.com, ddaney@caviumnetworks.com,
wangyijing@huawei.com, Suravee.Suthikulpanit@amd.com,
msalter@redhat.com, linux-pci@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-acpi@vger.kernel.org,
linux-kernel@vger.kernel.org, linaro-acpi@lists.linaro.org,
jcm@redhat.com, andrea.gallo@linaro.org, dhdang@apm.com,
jeremy.linton@arm.com, liudongdong3@huawei.com,
cov@codeaurora.org
Subject: Re: [PATCH V8 9/9] pci, acpi: ARM64 support for ACPI based generic PCI host controller
Date: Thu, 2 Jun 2016 11:44:59 +0200 [thread overview]
Message-ID: <5750001B.2090309@semihalf.com> (raw)
In-Reply-To: <20160602093500.GA13962@red-moon>
On 02.06.2016 11:35, Lorenzo Pieralisi wrote:
> On Mon, May 30, 2016 at 05:14:22PM +0200, Tomasz Nowicki wrote:
>> This patch implements pci_acpi_scan_root call so that ARM64 can start
>> using ACPI to setup and enumerate PCI buses.
>>
>> The implementation of pci_acpi_scan_root() looks up config space regions
>> through MCFG interface. Then ECAM library is doing a new mapping
>> and attach generic ECAM ops which are used for accessing config space.
>>
>> On ARM64, ACPI and DT can be enabled together, and in that case
>> we need to use generic domains. In order to do that we implement
>> ARM64 specific way of retrieving domain number from pci_config_window
>> structure.
>>
>> Since we enable PCI for ACPI we need to implement raw_pci_{read|write}
>> at the same time. ARM64 provides RAW accessors as long as there is
>> correlated valid pci_bus structure, but not before.
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> Signed-off-by: Jayachandran C <jchandra@broadcom.com>
>
> [...]
>
>> +/* release_info: free resrouces allocated by init_info */
>
> Nit: s/resrouces/resources
>
>> +static void pci_acpi_generic_release_info(struct acpi_pci_root_info *ci)
>> +{
>> + struct acpi_pci_generic_root_info *ri;
>> +
>> + ri = container_of(ci, struct acpi_pci_generic_root_info, common);
>> + pci_ecam_free(ri->cfg);
>> + kfree(ri);
>> +}
>> +
>> +static struct acpi_pci_root_ops acpi_pci_root_ops = {
>> + .release_info = pci_acpi_generic_release_info,
>> +};
>> +
>> +/* Interface called from ACPI code to setup PCI host controller */
>> struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>> {
>> - /* TODO: Should be revisited when implementing PCI on ACPI */
>> - return NULL;
>> + int node = acpi_get_node(root->device->handle);
>> + struct acpi_pci_generic_root_info *ri;
>> + struct pci_bus *bus, *child;
>> + int err;
>> +
>> + ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
>> + if (!ri)
>> + return NULL;
>> +
>> + err = pci_acpi_setup_ecam_mapping(root, ri);
>> + if (err)
>> + return NULL;
> ^
> Leaking memory -> kfree(ri)
I missed that. Will fix.
>
>> + acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
>> + bus = acpi_pci_root_create(root, &acpi_pci_root_ops, &ri->common,
>> + ri->cfg);
>> + if (!bus)
>> + return NULL;
>
> Ditto.
Here kfree(ri) is not needed. __acpi_pci_root_release_info will free ri
inside of acpi_pci_root_create.
>
>> +
>> + pci_bus_size_bridges(bus);
>> + pci_bus_assign_resources(bus);
>> +
>> + list_for_each_entry(child, &bus->children, node)
>> + pcie_bus_configure_settings(child);
>> +
>> + return bus;
>> }
>>
>> void pcibios_add_bus(struct pci_bus *bus)
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 9661c85..f66d188 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -1390,7 +1390,12 @@ static inline int pci_domain_nr(struct pci_bus *bus)
>> {
>> return bus->domain_nr;
>> }
>> +/* Arch specific ACPI hook to set-up domain number */
>> +#ifdef CONFIG_ACPI
>> +int acpi_pci_bus_domain_nr(struct pci_bus *bus);
>
> Technically speaking, this should be introduced in a separate patch;
> given that I know it is a temporary plaster and no other architecture will
> have to implement it I reckon it is fine to leave it here, I will make
> it disappear as fast as I can.
>
> We can argue if it is best to move the ACPI bits into a separate file
> (acpi-pci.c), I am fine with keeping DT/ACPI in one file if that's fine
> with Catalin and Will.
>
> You can send the fix-ups above in the final pull request which I hope
> is nigh.
>
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Thanks,
Tomasz
next prev parent reply other threads:[~2016-06-02 9:45 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-30 15:14 [PATCH V8 0/9] Support for ARM64 ACPI based PCI host controller Tomasz Nowicki
2016-05-30 15:14 ` [PATCH V8 1/9] PCI: ecam: move ecam.h to linux/include/pci-ecam.h Tomasz Nowicki
2016-06-02 9:48 ` Lorenzo Pieralisi
2016-05-30 15:14 ` [PATCH V8 2/9] PCI: ecam: Add parent device field to pci_config_window Tomasz Nowicki
2016-06-02 10:13 ` Lorenzo Pieralisi
2016-05-30 15:14 ` [PATCH V8 3/9] pci: Add new function to unmap IO resources Tomasz Nowicki
2016-06-02 16:50 ` Lorenzo Pieralisi
2016-05-30 15:14 ` [PATCH V8 4/9] acpi, pci: Support IO resources when parsing PCI host bridge resources Tomasz Nowicki
2016-06-02 17:30 ` Lorenzo Pieralisi
2016-06-07 23:56 ` Bjorn Helgaas
2016-05-30 15:14 ` [PATCH V8 5/9] pci, acpi: add acpi hook to assign domain number Tomasz Nowicki
2016-06-08 0:15 ` Bjorn Helgaas
2016-06-08 10:21 ` Tomasz Nowicki
2016-06-08 13:22 ` Bjorn Helgaas
2016-06-10 15:14 ` Lorenzo Pieralisi
2016-06-10 15:49 ` Lorenzo Pieralisi
2016-06-10 16:49 ` Tomasz Nowicki
2016-06-10 18:18 ` Bjorn Helgaas
2016-06-10 18:54 ` Jon Masters
2016-05-30 15:14 ` [PATCH V8 6/9] arm64, pci, acpi: ACPI support for legacy IRQs parsing and consolidation with DT code Tomasz Nowicki
2016-05-30 15:14 ` [PATCH V8 7/9] acpi: Add generic MCFG table handling Tomasz Nowicki
2016-06-03 11:38 ` Lorenzo Pieralisi
2016-06-06 12:55 ` Tomasz Nowicki
2016-06-08 1:56 ` Bjorn Helgaas
2016-06-08 12:21 ` Tomasz Nowicki
2016-06-08 13:17 ` Bjorn Helgaas
2016-06-08 13:44 ` Tomasz Nowicki
2016-05-30 15:14 ` [PATCH V8 8/9] arm64, pci, acpi: Provide ACPI-specific prerequisites for PCI bus enumeration Tomasz Nowicki
2016-06-02 9:45 ` Lorenzo Pieralisi
2016-06-02 9:51 ` Tomasz Nowicki
2016-05-30 15:14 ` [PATCH V8 9/9] pci, acpi: ARM64 support for ACPI based generic PCI host controller Tomasz Nowicki
2016-05-30 15:38 ` Arnd Bergmann
2016-05-30 16:13 ` Jayachandran C
2016-06-02 9:35 ` Lorenzo Pieralisi
2016-06-02 9:44 ` Tomasz Nowicki [this message]
2016-06-08 2:14 ` Bjorn Helgaas
2016-06-01 7:36 ` [PATCH V8 0/9] Support for ARM64 ACPI based " Gabriele Paoloni
2016-06-02 7:31 ` Jon Masters
2016-06-02 10:06 ` Gabriele Paoloni
2016-06-02 8:52 ` Tomasz Nowicki
2016-06-02 9:58 ` Gabriele Paoloni
2016-06-02 8:48 ` Jon Masters
2016-06-07 23:13 ` Bjorn Helgaas
2016-06-08 9:20 ` Dongdong Liu
2016-06-09 16:45 ` Suravee Suthikulanit
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=5750001B.2090309@semihalf.com \
--to=tn@semihalf.com \
--cc=Liviu.Dudau@arm.com \
--cc=Suravee.Suthikulpanit@amd.com \
--cc=andrea.gallo@linaro.org \
--cc=arnd@arndb.de \
--cc=catalin.marinas@arm.com \
--cc=cov@codeaurora.org \
--cc=ddaney@caviumnetworks.com \
--cc=dhdang@apm.com \
--cc=hanjun.guo@linaro.org \
--cc=helgaas@kernel.org \
--cc=jchandra@broadcom.com \
--cc=jcm@redhat.com \
--cc=jeremy.linton@arm.com \
--cc=linaro-acpi@lists.linaro.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=liudongdong3@huawei.com \
--cc=lorenzo.pieralisi@arm.com \
--cc=msalter@redhat.com \
--cc=mw@semihalf.com \
--cc=okaya@codeaurora.org \
--cc=rafael@kernel.org \
--cc=robert.richter@caviumnetworks.com \
--cc=wangyijing@huawei.com \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).