Re: [PATCH V2] mmc: sdhci-acpi: Add 64-bit DMA support

[Arnd Bergmann <arnd@arndb.de>]

On Tuesday 28 October 2014 12:05:30 Adrian Hunter wrote:
> On 28/10/14 11:43, Arnd Bergmann wrote:
> > On Tuesday 28 October 2014 10:37:20 Adrian Hunter wrote:
> >>  static int sdhci_acpi_enable_dma(struct sdhci_host *host)
> >>  {
> >> -       return 0;
> >> +       struct sdhci_acpi_host *c = sdhci_priv(host);
> >> +       struct device *dev = &c->pdev->dev;
> >> +       int err = -1;
> >> +
> >> +       if (c->dma_setup)
> >> +               return 0;
> >> +
> >> +       if (host->flags & SDHCI_USE_64_BIT_DMA) {
> >> +               if (host->quirks2 & SDHCI_QUIRK2_BROKEN_64_BIT_DMA) {
> >> +                       host->flags &= ~SDHCI_USE_64_BIT_DMA;
> >> +               } else {
> >> +                       err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> >> +                       if (err)
> >> +                               dev_warn(dev, "Failed to set 64-bit DMA mask\n");
> >> +               }
> >> +       }
> >> +
> >> +       if (err)
> >> +               err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> >> +
> >> +       c->dma_setup = !err;
> >> +
> >> +       return err;
> >>  }
> >>
> > 
> > I don't think it's worth a dev_warn() message (maybe dev_info), there is nothing
> 
> It is worth a dev_warn because 32-bit DMA can allocate memory for bounce
> buffers which jeopardizes memory reclaim.

Then you should also warn if SDHCI_USE_64_BIT_DMA isn't or if
SDHCI_QUIRK2_BROKEN_64_BIT_DMA is set I guess.

> > wrong in this case, but you probably have to clear the SDHCI_USE_64_BIT_DMA to
> > ensure that the driver won't try to to use the 64-bit DMA if the mask is not set.
> 
> 64-bit DMA will work with a 32-bit DMA mask so there is no need.

Wouldn't that at least be (slightly) less efficient? You end up having to do
two expensive MMIO register accesses each time you transfer an address.

	Arnd