From: Thierry Reding <thierry.reding@gmail.com>
To: John Garry <john.garry@huawei.com>
Cc: mika.westerberg@linux.intel.com, rafael@kernel.org,
lorenzo.pieralisi@arm.com, rjw@rjwysocki.net,
hanjun.guo@linaro.org, robh+dt@kernel.org, bhelgaas@google.com,
arnd@arndb.de, mark.rutland@arm.com, olof@lixom.net,
dann.frazier@canonical.com, andy.shevchenko@gmail.com,
robh@kernel.org, andriy.shevchenko@linux.intel.com,
joe@perches.com, benh@kernel.crashing.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-acpi@vger.kernel.org, linuxarm@huawei.com, minyard@acm.org,
devicetree@vger.kernel.org, linux-arch@vger.kernel.org,
rdunlap@infradead.org, gregkh@linuxfoundation.org,
akpm@linux-foundation.org, frowand.list@gmail.com, agraf@suse.de
Subject: Re: [PATCH v17 04/10] PCI: Apply the new generic I/O management on PCI IO hosts
Date: Tue, 3 Apr 2018 15:45:31 +0200 [thread overview]
Message-ID: <20180403134531.GD27789@ulmo> (raw)
In-Reply-To: <1521051359-34473-5-git-send-email-john.garry@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 3874 bytes --]
On Thu, Mar 15, 2018 at 02:15:53AM +0800, John Garry wrote:
> From: Zhichang Yuan <yuanzhichang@hisilicon.com>
>
> After introducing the new generic I/O space management(Logical PIO), the
> original PCI MMIO relevant helpers need to be updated based on the new
> interfaces defined in logical PIO.
> This patch adapts the corresponding code to match the changes introduced
> by logical PIO.
>
> Signed-off-by: Zhichang Yuan <yuanzhichang@hisilicon.com>
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de> #earlier draft
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Tested-by: dann frazier <dann.frazier@canonical.com>
> ---
> drivers/pci/pci.c | 92 +++++++++---------------------------------------
> include/asm-generic/io.h | 2 +-
> 2 files changed, 18 insertions(+), 76 deletions(-)
Today's linux-next regresses on NFS boot for Jetson TK1. I was able to
bisect it to this commit. I'll comment below for where I think this is
buggy.
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 3f30b7d..09c2490 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -22,6 +22,7 @@
> #include <linux/spinlock.h>
> #include <linux/string.h>
> #include <linux/log2.h>
> +#include <linux/logic_pio.h>
> #include <linux/pci-aspm.h>
> #include <linux/pm_wakeup.h>
> #include <linux/interrupt.h>
> @@ -3436,17 +3437,6 @@ int pci_request_regions_exclusive(struct pci_dev *pdev, const char *res_name)
> }
> EXPORT_SYMBOL(pci_request_regions_exclusive);
>
> -#ifdef PCI_IOBASE
> -struct io_range {
> - struct list_head list;
> - phys_addr_t start;
> - resource_size_t size;
> -};
> -
> -static LIST_HEAD(io_range_list);
> -static DEFINE_SPINLOCK(io_range_lock);
> -#endif
> -
> /*
> * Record the PCI IO range (expressed as CPU physical address + size).
> * Return a negative value if an error has occured, zero otherwise
> @@ -3454,51 +3444,28 @@ struct io_range {
> int pci_register_io_range(struct fwnode_handle *fwnode, phys_addr_t addr,
> resource_size_t size)
> {
> - int err = 0;
> -
> + int ret = 0;
> #ifdef PCI_IOBASE
> - struct io_range *range;
> - resource_size_t allocated_size = 0;
> -
> - /* check if the range hasn't been previously recorded */
> - spin_lock(&io_range_lock);
> - list_for_each_entry(range, &io_range_list, list) {
> - if (addr >= range->start && addr + size <= range->start + size) {
> - /* range already registered, bail out */
> - goto end_register;
> - }
> - allocated_size += range->size;
> - }
> + struct logic_pio_hwaddr *range;
>
> - /* range not registed yet, check for available space */
> - if (allocated_size + size - 1 > IO_SPACE_LIMIT) {
> - /* if it's too big check if 64K space can be reserved */
> - if (allocated_size + SZ_64K - 1 > IO_SPACE_LIMIT) {
> - err = -E2BIG;
> - goto end_register;
> - }
> -
> - size = SZ_64K;
> - pr_warn("Requested IO range too big, new size set to 64K\n");
> - }
> + if (!size || addr + size < addr)
> + return -EINVAL;
>
> - /* add the range to the list */
> range = kzalloc(sizeof(*range), GFP_ATOMIC);
> - if (!range) {
> - err = -ENOMEM;
> - goto end_register;
> - }
> + if (!range)
> + return -ENOMEM;
>
> - range->start = addr;
> + range->fwnode = fwnode;
> range->size = size;
> + range->hw_start = addr;
> + range->flags = LOGIC_PIO_CPU_MMIO;
>
> - list_add_tail(&range->list, &io_range_list);
> -
> -end_register:
> - spin_unlock(&io_range_lock);
> + ret = logic_pio_register_range(range);
This ends up returning -EFAULT at some point, causing the driver's
(pci-tegra.c) ->probe() to fail.
Let me comment on a prior patch to pinpoint what exactly goes wrong.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2018-04-03 13:45 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-14 18:15 [PATCH v17 00/10] LPC: legacy ISA I/O support John Garry
2018-03-14 18:15 ` [PATCH v17 01/10] LIB: Introduce a generic PIO mapping method John Garry
2018-04-03 14:04 ` Thierry Reding
2018-04-03 14:39 ` Thierry Reding
2018-04-03 16:01 ` John Garry
2018-04-03 16:37 ` Thierry Reding
2018-04-03 17:02 ` John Garry
2018-04-03 17:53 ` Bjorn Helgaas
2018-04-03 18:24 ` John Garry
2018-03-14 18:15 ` [PATCH v17 02/10] PCI: Remove unused __weak attribute in pci_register_io_range() John Garry
2018-03-14 18:15 ` [PATCH v17 03/10] PCI: Add fwnode handler as input param of pci_register_io_range() John Garry
2018-03-14 18:15 ` [PATCH v17 04/10] PCI: Apply the new generic I/O management on PCI IO hosts John Garry
2018-04-03 13:45 ` Thierry Reding [this message]
2018-04-03 14:02 ` John Garry
2018-03-14 18:15 ` [PATCH v17 05/10] OF: Add missing I/O range exception for indirect-IO devices John Garry
2018-03-14 18:15 ` [PATCH v17 06/10] HISI LPC: Support the LPC host on Hip06/Hip07 with DT bindings John Garry
2018-03-14 18:15 ` [PATCH v17 07/10] ACPI / scan: rename acpi_is_serial_bus_slave() to widen use John Garry
2018-03-19 10:27 ` Rafael J. Wysocki
2018-03-14 18:15 ` [PATCH v17 08/10] ACPI / scan: do not enumerate Indirect IO host children John Garry
2018-03-19 10:30 ` Rafael J. Wysocki
2018-03-19 10:48 ` John Garry
2018-03-19 10:57 ` Rafael J. Wysocki
2018-03-19 11:13 ` John Garry
2018-03-14 18:15 ` [PATCH v17 09/10] HISI LPC: Add ACPI support John Garry
2018-03-14 18:15 ` [PATCH v17 10/10] MAINTAINERS: Add maintainer for HiSilicon LPC driver John Garry
2018-03-21 23:39 ` [PATCH v17 00/10] LPC: legacy ISA I/O support Bjorn Helgaas
2018-03-22 10:38 ` John Garry
2018-03-22 13:35 ` Bjorn Helgaas
2018-03-22 14:18 ` John Garry
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=20180403134531.GD27789@ulmo \
--to=thierry.reding@gmail.com \
--cc=agraf@suse.de \
--cc=akpm@linux-foundation.org \
--cc=andriy.shevchenko@linux.intel.com \
--cc=andy.shevchenko@gmail.com \
--cc=arnd@arndb.de \
--cc=benh@kernel.crashing.org \
--cc=bhelgaas@google.com \
--cc=dann.frazier@canonical.com \
--cc=devicetree@vger.kernel.org \
--cc=frowand.list@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=hanjun.guo@linaro.org \
--cc=joe@perches.com \
--cc=john.garry@huawei.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linuxarm@huawei.com \
--cc=lorenzo.pieralisi@arm.com \
--cc=mark.rutland@arm.com \
--cc=mika.westerberg@linux.intel.com \
--cc=minyard@acm.org \
--cc=olof@lixom.net \
--cc=rafael@kernel.org \
--cc=rdunlap@infradead.org \
--cc=rjw@rjwysocki.net \
--cc=robh+dt@kernel.org \
--cc=robh@kernel.org \
/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).