Re: [PATCH v4 08/12] device core: Introduce multiple dma pfn offsets
Hi Andy, On Tue, Jun 9, 2020 at 7:18 AM Andy Shevchenko wrote: > > On Mon, Jun 08, 2020 at 11:48:51AM -0400, Jim Quinlan wrote: > > On Sun, Jun 7, 2020 at 12:500f9bfe0fb8840b268af1bbcc51f1cd440514e PM > > Andy Shevchenko wrote: > > > On Fri, Jun 05, 2020 at 05:26:48PM -0400, Jim Quinlan wrote: > > ... > > > > > + *map_size = (num_ranges + 1) * sizeof(**map); > > > > + r = kzalloc(*map_size, GFP_KERNEL); > > > > > > kcalloc() > > Since I have to calculate the size anyway I thought kzalloc was fine. > > I'll switch. > > The point is to check multiplication overflow. See overflow.h for helpers. I am aware of this check and didn't think of it as applicable here, as the most dma-ranges I can envision is six. I suppose that it is possible that this may change in the future to some big number. At any rate, the next version has kcalloc(). Regards, Jim > > > > > > + if (!r) > > > > + return -ENOMEM; > > -- > With Best Regards, > Andy Shevchenko > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 08/12] device core: Introduce multiple dma pfn offsets
On Mon, Jun 08, 2020 at 11:48:51AM -0400, Jim Quinlan wrote: > On Sun, Jun 7, 2020 at 12:500f9bfe0fb8840b268af1bbcc51f1cd440514e PM > Andy Shevchenko wrote: > > On Fri, Jun 05, 2020 at 05:26:48PM -0400, Jim Quinlan wrote: ... > > > + *map_size = (num_ranges + 1) * sizeof(**map); > > > + r = kzalloc(*map_size, GFP_KERNEL); > > > > kcalloc() > Since I have to calculate the size anyway I thought kzalloc was fine. > I'll switch. The point is to check multiplication overflow. See overflow.h for helpers. > > > + if (!r) > > > + return -ENOMEM; -- With Best Regards, Andy Shevchenko ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 08/12] device core: Introduce multiple dma pfn offsets
Hi Andy, On Sun, Jun 7, 2020 at 12:500f9bfe0fb8840b268af1bbcc51f1cd440514e PM Andy Shevchenko wrote: > > On Fri, Jun 05, 2020 at 05:26:48PM -0400, Jim Quinlan wrote: > > The new field in struct device 'dma_pfn_offset_map' is used to facilitate > > the use of single or multiple pfn offsets between cpu addrs and dma addrs. > > It subsumes the role of dev->dma_pfn_offset -- a uniform offset. > > > > The function of_dma_get_range() has been modified to take two additional > > arguments: the "map", which is an array that holds the information > > regarding the pfn offset regions, and map_size, which is the size in bytes > > of the map array. > > > > of_dma_configure() is the typical manner to set pfn offsets but there are a > > number of ad hoc assignments to dev->dma_pfn_offset in the kernel driver > > code. These cases now invoke the function > > dma_attach_uniform_pfn_offset(dev, pfn_offset). > > ... > > > + int ret = dma_attach_uniform_pfn_offset > > + (dev, keystone_dma_pfn_offset); > > It's strange indentation. Have you configured your editor correctly? > Seems to me as fit on one line. I'm using emacs with the c-style set to linux. I may have some custom tweaks; I'll check into it. But I think I can fix most of your objections by using the max_line_length of 100. > > > + dev_err(dev, "set dma_pfn_offset%08lx%s\n", > > + dev->dma_pfn_offset, ret ? " failed" : ""); > > ... > > > + *map_size = (num_ranges + 1) * sizeof(**map); > > + r = kzalloc(*map_size, GFP_KERNEL); > > kcalloc() Since I have to calculate the size anyway I thought kzalloc was fine. I'll switch. > > > + if (!r) > > + return -ENOMEM; > > ... > > > + r->pfn_offset = PFN_DOWN(range.cpu_addr) > > + - PFN_DOWN(range.bus_addr); > > Ditto (indentation). > > ... > > > > + unsigned long dma_pfn_offset > > + = dma_pfn_offset_from_phys_addr(dev, paddr); > > Ditto. > > ... > > > + unsigned long dma_pfn_offset > > + = dma_pfn_offset_from_dma_addr(dev, dev_addr); > > Ditto. > > Check entire your series for a such, please! Will do, Thanks Jim Quinlan > > -- > With Best Regards, > Andy Shevchenko > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 08/12] device core: Introduce multiple dma pfn offsets
On Fri, Jun 05, 2020 at 05:26:48PM -0400, Jim Quinlan wrote: > The new field in struct device 'dma_pfn_offset_map' is used to facilitate > the use of single or multiple pfn offsets between cpu addrs and dma addrs. > It subsumes the role of dev->dma_pfn_offset -- a uniform offset. > > The function of_dma_get_range() has been modified to take two additional > arguments: the "map", which is an array that holds the information > regarding the pfn offset regions, and map_size, which is the size in bytes > of the map array. > > of_dma_configure() is the typical manner to set pfn offsets but there are a > number of ad hoc assignments to dev->dma_pfn_offset in the kernel driver > code. These cases now invoke the function > dma_attach_uniform_pfn_offset(dev, pfn_offset). ... > + int ret = dma_attach_uniform_pfn_offset > + (dev, keystone_dma_pfn_offset); It's strange indentation. Have you configured your editor correctly? Seems to me as fit on one line. > + dev_err(dev, "set dma_pfn_offset%08lx%s\n", > + dev->dma_pfn_offset, ret ? " failed" : ""); ... > + *map_size = (num_ranges + 1) * sizeof(**map); > + r = kzalloc(*map_size, GFP_KERNEL); kcalloc() > + if (!r) > + return -ENOMEM; ... > + r->pfn_offset = PFN_DOWN(range.cpu_addr) > + - PFN_DOWN(range.bus_addr); Ditto (indentation). ... > + unsigned long dma_pfn_offset > + = dma_pfn_offset_from_phys_addr(dev, paddr); Ditto. ... > + unsigned long dma_pfn_offset > + = dma_pfn_offset_from_dma_addr(dev, dev_addr); Ditto. Check entire your series for a such, please! -- With Best Regards, Andy Shevchenko ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 08/12] device core: Introduce multiple dma pfn offsets
The new field in struct device 'dma_pfn_offset_map' is used to facilitate
the use of single or multiple pfn offsets between cpu addrs and dma addrs.
It subsumes the role of dev->dma_pfn_offset -- a uniform offset.
The function of_dma_get_range() has been modified to take two additional
arguments: the "map", which is an array that holds the information
regarding the pfn offset regions, and map_size, which is the size in bytes
of the map array.
of_dma_configure() is the typical manner to set pfn offsets but there are a
number of ad hoc assignments to dev->dma_pfn_offset in the kernel driver
code. These cases now invoke the function
dma_attach_uniform_pfn_offset(dev, pfn_offset).
Signed-off-by: Jim Quinlan
---
arch/arm/include/asm/dma-mapping.h| 9 +--
arch/arm/mach-keystone/keystone.c | 9 ++-
arch/sh/drivers/pci/pcie-sh7786.c | 3 +-
arch/sh/kernel/dma-coherent.c | 14 ++--
arch/x86/pci/sta2x11-fixup.c | 7 +-
drivers/acpi/arm64/iort.c | 5 +-
drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +-
drivers/iommu/io-pgtable-arm.c| 2 +-
.../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +-
.../platform/sunxi/sun6i-csi/sun6i_csi.c | 4 +-
drivers/of/address.c | 72 ---
drivers/of/device.c | 19 +++--
drivers/of/of_private.h | 11 +--
drivers/of/unittest.c | 8 ++-
drivers/remoteproc/remoteproc_core.c | 2 +-
.../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +-
drivers/usb/core/message.c| 4 +-
drivers/usb/core/usb.c| 2 +-
include/linux/device.h| 4 +-
include/linux/dma-direct.h| 16 -
include/linux/dma-mapping.h | 38 ++
kernel/dma/coherent.c | 11 +--
kernel/dma/mapping.c | 38 ++
23 files changed, 235 insertions(+), 60 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index bdd80ddbca34..f1e72f99468b 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -35,8 +35,9 @@ static inline const struct dma_map_ops
*get_arch_dma_ops(struct bus_type *bus)
#ifndef __arch_pfn_to_dma
static inline dma_addr_t pfn_to_dma(struct device *dev, unsigned long pfn)
{
- if (dev)
- pfn -= dev->dma_pfn_offset;
+ if (dev && dev->dma_pfn_offset_map)
+ pfn -= dma_pfn_offset_from_phys_addr(dev, PFN_PHYS(pfn));
+
return (dma_addr_t)__pfn_to_bus(pfn);
}
@@ -44,8 +45,8 @@ static inline unsigned long dma_to_pfn(struct device *dev,
dma_addr_t addr)
{
unsigned long pfn = __bus_to_pfn(addr);
- if (dev)
- pfn += dev->dma_pfn_offset;
+ if (dev && dev->dma_pfn_offset_map)
+ pfn += dma_pfn_offset_from_dma_addr(dev, addr);
return pfn;
}
diff --git a/arch/arm/mach-keystone/keystone.c
b/arch/arm/mach-keystone/keystone.c
index 638808c4e122..dfb095b31534 100644
--- a/arch/arm/mach-keystone/keystone.c
+++ b/arch/arm/mach-keystone/keystone.c
@@ -8,6 +8,7 @@
*/
#include
#include
+#include
#include
#include
#include
@@ -38,9 +39,11 @@ static int keystone_platform_notifier(struct notifier_block
*nb,
return NOTIFY_BAD;
if (!dev->of_node) {
- dev->dma_pfn_offset = keystone_dma_pfn_offset;
- dev_err(dev, "set dma_pfn_offset%08lx\n",
- dev->dma_pfn_offset);
+ int ret = dma_attach_uniform_pfn_offset
+ (dev, keystone_dma_pfn_offset);
+
+ dev_err(dev, "set dma_pfn_offset%08lx%s\n",
+ dev->dma_pfn_offset, ret ? " failed" : "");
}
return NOTIFY_OK;
}
diff --git a/arch/sh/drivers/pci/pcie-sh7786.c
b/arch/sh/drivers/pci/pcie-sh7786.c
index e0b568aaa701..3e63c6b6e070 100644
--- a/arch/sh/drivers/pci/pcie-sh7786.c
+++ b/arch/sh/drivers/pci/pcie-sh7786.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -487,7 +488,7 @@ int pcibios_map_platform_irq(const struct pci_dev *pdev, u8
slot, u8 pin)
void pcibios_bus_add_device(struct pci_dev *pdev)
{
- pdev->dev.dma_pfn_offset = dma_pfn_offset;
+ dma_attach_uniform_pfn_offset(>dev, dma_pfn_offset);
}
static int __init sh7786_pcie_core_init(void)
diff --git a/arch/sh/kernel/dma-coherent.c b/arch/sh/kernel/dma-coherent.c
index d4811691b93c..f4a092e74910 100644
--- a/arch/sh/kernel/dma-coherent.c
+++ b/arch/sh/kernel/dma-coherent.c
@@ -14,6 +14,7 @@ void *arch_dma_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle,
{
void *ret, *ret_nocache;
int order = get_order(size);
+ phys_addr_t phys;
gfp |= __GFP_ZERO;
