Re: [PATCH V2] PCI: tegra: Use the DMA-API to get the MSI address

[Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>]

On Fri, Apr 05, 2019 at 03:00:36AM +0530, Vidya Sagar wrote:
> On 4/1/2019 11:13 AM, Vidya Sagar wrote:
> 
> Hi Bjorn / Lorenzo,
> Apologies for reminding you again. Could you please review this patch?

It is in the patchwork queue - I will get to it, thank you for your
patience.

Lorenzo

> Thanks,
> Vidya Sagar
> 
> >Hi Bjorn / Lorenzo,
> >Can you please review this patch?
> >
> >Thanks,
> >Vidya Sagar
> >
> >On 3/27/2019 4:29 PM, vidya sagar wrote:
> >>Hi Bjorn/Lorenzo,
> >>Can you please review this patch?
> >>Thierry has reviewed it and I already took care of his comments.
> >>
> >>Thanks,
> >>Vidya Sagar
> >>
> >>On Tue, Mar 19, 2019 at 7:33 PM Vidya Sagar <vidyas@nvidia.com <mailto:vidyas@nvidia.com>> wrote:
> >>
> >>    Since the upstream MSI memory writes are generated by downstream devices,
> >>    it is logically correct to have MSI target memory coming from the DMA pool
> >>    reserved for PCIe than from the general memory pool reserved for CPU
> >>    access. This avoids PCIe DMA addresses coinciding with MSI target address
> >>    thereby raising unwanted MSI interrupts. This patch also enforces to limit
> >>    the MSI target address to 32-bits to make it work for PCIe endponits that
> >>    support only 32-bit MSI target address and those endpoints that support
> >>    64-bit MSI target address anyway work with 32-bit MSI target address.
> >>
> >>    Signed-off-by: Vidya Sagar <vidyas@nvidia.com <mailto:vidyas@nvidia.com>>
> >>    Reviewed-by: Thierry Reding <treding@nvidia.com <mailto:treding@nvidia.com>>
> >>    Acked-by: Thierry Reding <treding@nvidia.com <mailto:treding@nvidia.com>>
> >>    ---
> >>    v2:
> >>    * changed 'phys' type to 'dma_addr_t' from 'u64'
> >>    * added a comment on why DMA mask is set to 32-bit
> >>    * replaced 'dma' with 'DMA'
> >>
> >>      drivers/pci/controller/pci-tegra.c | 35 ++++++++++++++++++++++++++---------
> >>      1 file changed, 26 insertions(+), 9 deletions(-)
> >>
> >>    diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> >>    index f4f53d092e00..f8173a5e352d 100644
> >>    --- a/drivers/pci/controller/pci-tegra.c
> >>    +++ b/drivers/pci/controller/pci-tegra.c
> >>    @@ -231,9 +231,9 @@ struct tegra_msi {
> >>             struct msi_controller chip;
> >>             DECLARE_BITMAP(used, INT_PCI_MSI_NR);
> >>             struct irq_domain *domain;
> >>    -       unsigned long pages;
> >>             struct mutex lock;
> >>    -       u64 phys;
> >>    +       void *virt;
> >>    +       dma_addr_t phys;
> >>             int irq;
> >>      };
> >>
> >>    @@ -1536,7 +1536,7 @@ static int tegra_pcie_msi_setup(struct tegra_pcie *pcie)
> >>             err = platform_get_irq_byname(pdev, "msi");
> >>             if (err < 0) {
> >>                     dev_err(dev, "failed to get IRQ: %d\n", err);
> >>    -               goto err;
> >>    +               goto free_irq_domain;
> >>             }
> >>
> >>             msi->irq = err;
> >>    @@ -1545,17 +1545,34 @@ static int tegra_pcie_msi_setup(struct tegra_pcie *pcie)
> >>    tegra_msi_irq_chip.name <http://tegra_msi_irq_chip.name>, pcie);
> >>             if (err < 0) {
> >>                     dev_err(dev, "failed to request IRQ: %d\n", err);
> >>    -               goto err;
> >>    +               goto free_irq_domain;
> >>    +       }
> >>    +
> >>    +       /* Though the PCIe controller can address >32-bit address space, to
> >>    +        * facilitate endpoints that support only 32-bit MSI target address,
> >>    +        * the mask is set to 32-bit to make sure that MSI target address is
> >>    +        * always a 32-bit address
> >>    +        */
> >>    +       err = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> >>    +       if (err < 0) {
> >>    +               dev_err(dev, "failed to set DMA coherent mask: %d\n", err);
> >>    +               goto free_irq;
> >>    +       }
> >>    +
> >>    +       msi->virt = dma_alloc_coherent(dev, PAGE_SIZE, &msi->phys, GFP_KERNEL);
> >>    +       if (!msi->virt) {
> >>    +               dev_err(dev, "failed to allocate DMA memory for MSI\n");
> >>    +               err = -ENOMEM;
> >>    +               goto free_irq;
> >>             }
> >>
> >>    -       /* setup AFI/FPCI range */
> >>    -       msi->pages = __get_free_pages(GFP_KERNEL, 0);
> >>    -       msi->phys = virt_to_phys((void *)msi->pages);
> >>             host->msi = &msi->chip;
> >>
> >>             return 0;
> >>
> >>    -err:
> >>    +free_irq:
> >>    +       free_irq(msi->irq, pcie);
> >>    +free_irq_domain:
> >>             irq_domain_remove(msi->domain);
> >>             return err;
> >>      }
> >>    @@ -1592,7 +1609,7 @@ static void tegra_pcie_msi_teardown(struct tegra_pcie *pcie)
> >>             struct tegra_msi *msi = &pcie->msi;
> >>             unsigned int i, irq;
> >>
> >>    -       free_pages(msi->pages, 0);
> >>    +       dma_free_coherent(pcie->dev, PAGE_SIZE, msi->virt, msi->phys);
> >>
> >>             if (msi->irq > 0)
> >>                     free_irq(msi->irq, pcie);
> >>    --     2.7.4
> >>
> >
>