linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: msalter@redhat.com (Mark Salter)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V3 00/21] MMCONFIG refactoring and support for ARM64 PCI hostbridge init based on ACPI
Date: Tue, 19 Jan 2016 18:37:36 -0500	[thread overview]
Message-ID: <1453246656.28109.81.camel@redhat.com> (raw)
In-Reply-To: <20160119204015.GS19062@n2100.arm.linux.org.uk>

On Tue, 2016-01-19 at 20:40 +0000, Russell King - ARM Linux wrote:
> On Tue, Jan 19, 2016 at 02:25:13PM -0600, Bjorn Helgaas wrote:
> > On Mon, Jan 18, 2016 at 02:04:05PM +0000, Graeme Gregory wrote:
> > > After some private debugging with Mark it turned out that the difference
> > > between our configurations was I did not have.
> > > 
> > > CONFIG_DMA_CMA=y
> > > 
> > > With this enabled then the card works without coherent mask hack.
> > 
> > I didn't follow the whole discussion here, but is this a case where
> > the driver could fail more gracefully than it did???Can we do anything
> > to make this easier for the next person who trips over the same problem?
> 
> I've not followed the discussion at all, but reading what was in the
> quoted parts of the mail makes me somewhat suspicious.
> 
> The way PCI drivers (or in fact any driver) are supposed to work is:
> 
> - the bus code sets up a default mask (32-bit DMA in the case of PCI)
> - the driver calls dma_set_mask() or similar function with the mask
> ? the _driver_ wants to use.??Arch DMA code decides whether the mask
> ? can be supported, and if it can, it re-sets the DMA mask.??If the
> ? mask can't be supported, it returns an error.
> 
> The driver is doing things correctly:
> 
> ????????if ((sizeof(dma_addr_t) > 4) &&
> ????????????!pci_set_dma_mask(pdev, DMA_BIT_MASK(64)) && use_dac) {
> ????????????????tp->cp_cmd |= PCIDAC;
> ????????????????dev->features |= NETIF_F_HIGHDMA;
> ????????} else {
> ????????????????rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
> 
> which says: if the size of a DMA address supports 64-bit, and we can
> set a 64-bit DMA address mask (iow, the platform allows it), and
> we're permitted to use DAC, use DAC, otherwise try to set a 32-bit
> DMA mask.
> 
> It shouldn't matter one bit what the mask is before that point.
> 
> However, what the driver fails to do is to deal with the coherent
> mask - it appears to be missing a call to pci_set_consistent_dma_mask().
> That may be where the issue is.

It is.

The driver works with devicetree because of_dma_configure() sets up dma
and coherent mask for PCI devices based on root bridge info in the DT.
So the driver gets by without setting coherent mask explicitly. In the
ACPI case, the coherent mask doesn't get set because there isn't DMA
info in the ACPI tables (I think IORT table/IOMMU support will add this
in the future). Using CONFIG_DMA_CMA avoids the issue by allowing CMA
memory to be used for DMA while avoiding a check against coherent mask.

> 
> It may be worth doing what I did with the DMA API, and introducing
> pci_set_mask_and_coherent() which sets both masks together - which
> will probably allow some PCI driver code to be simplified.
> 

  reply	other threads:[~2016-01-19 23:37 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-13 13:20 [PATCH V3 00/21] MMCONFIG refactoring and support for ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
2016-01-13 13:20 ` [PATCH V3 01/21] x86, pci: Reorder logic of pci_mmconfig_insert() function Tomasz Nowicki
2016-01-13 13:20 ` [PATCH V3 02/21] x86, pci, acpi: Move arch-agnostic MMCONFIG (aka ECAM) and ACPI code out of arch/x86/ directory Tomasz Nowicki
2016-01-13 13:20 ` [PATCH V3 03/21] pci, acpi, mcfg: Provide generic implementation of MCFG code initialization Tomasz Nowicki
2016-01-13 13:20 ` [PATCH V3 04/21] x86, pci: mmconfig_{32, 64}.c code refactoring - remove code duplication Tomasz Nowicki
2016-01-13 13:20 ` [PATCH V3 05/21] x86, pci, ecam: mmconfig_64.c becomes default implementation for ECAM driver Tomasz Nowicki
2016-01-13 13:20 ` [PATCH V3 06/21] XEN / PCI: Remove the dependence on arch x86 when PCI_MMCONFIG=y Tomasz Nowicki
2016-01-13 13:20 ` [PATCH V3 07/21] pci, acpi, mcfg: Provide default RAW ACPI PCI config space accessors Tomasz Nowicki
2016-01-13 13:20 ` [PATCH V3 08/21] arm64, acpi: Use empty PCI config space accessors from mcfg.c file Tomasz Nowicki
2016-01-13 13:20 ` [PATCH V3 09/21] pci, acpi, ecam: Add flag to indicate whether ECAM region was hot added or not Tomasz Nowicki
2016-01-13 13:20 ` [PATCH V3 10/21] x86, pci: Cleanup platform specific MCFG data using previously added ECAM hot_added flag Tomasz Nowicki
2016-01-13 13:20 ` [PATCH V3 11/21] pci, acpi: Move ACPI host bridge device companion assignment to core code Tomasz Nowicki
2016-01-19 16:02   ` Lorenzo Pieralisi
2016-01-20 11:20     ` Tomasz Nowicki
2016-01-20 12:38       ` Lorenzo Pieralisi
2016-01-20 13:40         ` Tomasz Nowicki
2016-01-20 14:22           ` Lorenzo Pieralisi
2016-01-20 14:41             ` Tomasz Nowicki
2016-01-27 17:42               ` Lorenzo Pieralisi
2016-01-13 13:20 ` [PATCH V3 12/21] x86, ia64, pci: Remove ACPI companion device from platform specific data Tomasz Nowicki
2016-01-13 13:20 ` [PATCH V3 13/21] pci, acpi: Provide generic way to assign bus domain number Tomasz Nowicki
2016-01-21 18:22   ` Lorenzo Pieralisi
2016-01-21 18:38     ` Tomasz Nowicki
2016-01-22 11:25       ` Lorenzo Pieralisi
2016-01-13 13:21 ` [PATCH V3 14/21] x86, ia64: Include acpi_pci_{add|remove}_bus to the default pcibios_{add|remove}_bus implementation Tomasz Nowicki
2016-01-14 16:33   ` Lorenzo Pieralisi
2016-01-14 17:45     ` Tomasz Nowicki
2016-01-13 13:21 ` [PATCH V3 15/21] acpi, mcfg: Implement two calls that might be used to inject/remove MCFG region Tomasz Nowicki
2016-01-13 13:21 ` [PATCH V3 16/21] x86, acpi, pci: Use equivalent function introduced in previous patch Tomasz Nowicki
2016-01-13 13:21 ` [PATCH V3 17/21] acpi, mcfg: Add default PCI config accessors implementation and initial support for related quirks Tomasz Nowicki
2016-01-13 13:21 ` [PATCH V3 18/21] ACPI, PCI: Refine the way to handle translation_offset for ACPI resources Tomasz Nowicki
2016-01-14 12:13   ` Lorenzo Pieralisi
2016-01-19 12:20   ` Lorenzo Pieralisi
2016-01-25  9:52     ` Lorenzo Pieralisi
2016-01-25 16:57       ` Mark Salter
2016-01-28 10:23     ` Hanjun Guo
2016-01-13 13:21 ` [PATCH V3 19/21] pci, acpi: Support for ACPI based generic PCI host controller init Tomasz Nowicki
2016-01-15  9:57   ` Hanjun Guo
2016-01-18  9:57     ` Tomasz Nowicki
2016-01-18  9:25   ` liudongdong (C)
2016-01-18 10:34     ` Tomasz Nowicki
2016-01-19 11:58   ` Lorenzo Pieralisi
2016-01-20 15:01     ` Tomasz Nowicki
2016-01-13 13:21 ` [PATCH V3 20/21] pci, acpi: Match PCI config space accessors against platfrom specific quirks Tomasz Nowicki
2016-01-14 15:36   ` Mark Salter
2016-01-18 12:41     ` Tomasz Nowicki
2016-01-19  1:49       ` liudongdong (C)
2016-01-19  7:55         ` Tomasz Nowicki
2016-01-19  8:52           ` liudongdong (C)
2016-01-13 13:21 ` [PATCH V3 21/21] arm64, pci, acpi: Start using ACPI based PCI host bridge driver for ARM64 Tomasz Nowicki
2016-01-13 15:24 ` [PATCH V3 00/21] MMCONFIG refactoring and support for ARM64 PCI hostbridge init based on ACPI Sinan Kaya
2016-01-13 15:27   ` Tomasz Nowicki
2016-01-14 13:44 ` Graeme Gregory
2016-01-14 14:00   ` Catalin Marinas
2016-01-14 14:09     ` Mark Salter
2016-01-14 14:50       ` Catalin Marinas
2016-01-14 14:59         ` Mark Salter
2016-01-14 14:01   ` Mark Salter
2016-01-14 14:15     ` Graeme Gregory
2016-01-14 14:24       ` Mark Salter
2016-01-15 12:12         ` Graeme Gregory
2016-01-18 14:04           ` Graeme Gregory
2016-01-19 20:25             ` Bjorn Helgaas
2016-01-19 20:40               ` Russell King - ARM Linux
2016-01-19 23:37                 ` Mark Salter [this message]
2016-01-14 15:29 ` Mark Salter
2016-01-14 15:38   ` Sinan Kaya
2016-01-14 16:12     ` Lorenzo Pieralisi
2016-01-14 16:38       ` Mark Salter
2016-01-14 17:07         ` Lorenzo Pieralisi
2016-01-14 17:32           ` Mark Salter
2016-01-14 17:59             ` Lorenzo Pieralisi
2016-01-14 18:44               ` Mark Salter
2016-01-14 22:51   ` Jeremy Linton
2016-01-14 22:55 ` Jeremy Linton
2016-01-15 11:00 ` Hanjun Guo
2016-01-18 14:37   ` Hanjun Guo
2016-01-29  6:43 ` liudongdong (C)
2016-02-01 19:58 ` Duc Dang

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=1453246656.28109.81.camel@redhat.com \
    --to=msalter@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.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).