Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets

On Wed, Jun 03, 2020 at 03:20:41PM -0400, Jim Quinlan wrote:

@@ -786,7 +787,7 @@ static int sun4i_backend_bind(struct device *dev, struct device *master, const struct sun4i_backend_quirks *quirks; struct resource *res; void __iomem *regs;

• int i, ret;

• int i, ret = 0;

No need for this.

backend = devm_kzalloc(dev, sizeof(*backend), GFP_KERNEL); if (!backend) @@ -812,7 +813,9 @@ static int sun4i_backend_bind(struct device *dev, struct device *master, * on our device since the RAM mapping is at 0 for the DMA bus, * unlike the CPU. */

• drm->dev->dma_pfn_offset = PHYS_PFN_OFFSET;
  

• ret = attach_uniform_dma_pfn_offset(dev, PHYS_PFN_OFFSET);
  

• if (ret)
  

• 	return ret;
  

  }

  backend->engine.node = dev->of_node;

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c index 04fbd4bf0ff9..e9cc1c2d47cd 100644 --- a/drivers/iommu/io-pgtable-arm.c +++ b/drivers/iommu/io-pgtable-arm.c @@ -754,7 +754,7 @@ arm_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg) if (cfg->oas > ARM_LPAE_MAX_ADDR_BITS) return NULL;

• if (!selftest_running && cfg->iommu_dev->dma_pfn_offset) {

• if (!selftest_running && cfg->iommu_dev->dma_pfn_offset_map) { dev_err(cfg->iommu_dev, "Cannot accommodate DMA offset for IOMMU page tables\n"); return NULL; }

diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c index eff34ded6305..7212da5e1076 100644 --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c @@ -7,6 +7,7 @@ */

#include <linux/clk.h> +#include <linux/dma-mapping.h> #include <linux/interrupt.h> #include <linux/module.h> #include <linux/mutex.h> @@ -183,7 +184,9 @@ static int sun4i_csi_probe(struct platform_device *pdev) return ret; } else { #ifdef PHYS_PFN_OFFSET

• csi->dev->dma_pfn_offset = PHYS_PFN_OFFSET;
  

• ret = attach_uniform_dma_pfn_offset(dev, PHYS_PFN_OFFSET);
  

• if (ret)
  

• 	return ret;
  

#endif }

diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c index 055eb0b8e396..2d66d415b6c3 100644 --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c @@ -898,7 +898,10 @@ static int sun6i_csi_probe(struct platform_device *pdev)

sdev->dev = &pdev->dev; /* The DMA bus has the memory mapped at 0 */

• sdev->dev->dma_pfn_offset = PHYS_OFFSET >> PAGE_SHIFT;

• ret = attach_uniform_dma_pfn_offset(sdev->dev,

• 			    PHYS_OFFSET >> PAGE_SHIFT);
  

• if (ret)

• return ret;
  
  

  ret = sun6i_csi_resource_request(sdev, pdev); if (ret)

diff --git a/drivers/of/address.c b/drivers/of/address.c index 96d8cfb14a60..c89333b0a5fb 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -918,6 +918,70 @@ void __iomem *of_io_request_and_map(struct device_node *np, int index, } EXPORT_SYMBOL(of_io_request_and_map);

+static int attach_dma_pfn_offset_map(struct device *dev,

• 		     struct device_node *node, int num_ranges)
  

+{

• struct of_range_parser parser;
• struct of_range range;
• struct dma_pfn_offset_region *r;
• r = devm_kcalloc(dev, num_ranges + 1,

• 	 sizeof(struct dma_pfn_offset_region), GFP_KERNEL);
  

• if (!r)

• return -ENOMEM;
  

• dev->dma_pfn_offset_map = r;
• of_dma_range_parser_init(&parser, node);
• /*

• * Record all info for DMA ranges array.  We could
  

• * just use the of_range struct, but if we did that it
  

• * would require more calculations for phys_to_dma and
  

• * dma_to_phys conversions.
  

• */
  

• for_each_of_range(&parser, &range) {

• r->cpu_start = range.cpu_addr;
  

• r->cpu_end = r->cpu_start + range.size - 1;
  

• r->dma_start = range.bus_addr;
  

• r->dma_end = r->dma_start + range.size - 1;
  

• r->pfn_offset = PFN_DOWN(range.cpu_addr)
  

• 	- PFN_DOWN(range.bus_addr);
  

• r++;
  

• }
• return 0;

+}

+/**

• • attach_dma_pfn_offset - Assign scalar offset for all addresses.
• • @dev: device pointer; only needed for a corner case.
• • @dma_pfn_offset: offset to apply when converting from phys addr

^^^^^^^^^^^^^^^ This parameter name does not match.

• • 	to dma addr and vice versa.
    

• • It returns -ENOMEM if out of memory, otherwise 0.

It can also return -ENODEV. Why are we passing NULL dev pointers to all these functions anyway?

• */

+int attach_uniform_dma_pfn_offset(struct device *dev, unsigned long pfn_offset) +{

• struct dma_pfn_offset_region *r;
• if (!dev)

• return -ENODEV;
  

• if (!pfn_offset)

• return 0;
  

• r = devm_kcalloc(dev, 1, sizeof(struct dma_pfn_offset_region),

• 	 GFP_KERNEL);
  

Use: r = devm_kzalloc(dev, sizeof(*r), GFP_KERNEL);

• if (!r)

• return -ENOMEM;
  

• r->uniform_offset = true;
• r->pfn_offset = pfn_offset;
• return 0;

+}

This function doesn't seem to do anything useful. Is part of it missing?

+EXPORT_SYMBOL_GPL(attach_uniform_dma_pfn_offset);

regards, dan carpenter