Re: can we switch bmips to the generic dma ranges code
I'll get on it. Jim On Tue, Nov 3, 2020 at 12:42 PM Florian Fainelli wrote: > > > > On 11/3/2020 2:15 AM, Christoph Hellwig wrote: > > Hi Florian and others, > > > > now that the generic DMA ranges code landed, can we switch bmips over > > to it instead of duplicating the logic? > > This should be fine, I cannot easily test the 338x chips, however as far > as PCIe goes, I believe Jim may have some older patches he can dig up > for testing. > -- > Florian smime.p7s Description: S/MIME Cryptographic Signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Tue, Sep 8, 2020 at 5:43 AM Christoph Hellwig wrote: > > And because I like replying to myself so much, here is a link to the > version with the arm cleanup patch applied. Unlike the previous two > attempts this has at least survived very basic sanity testing: > > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-ranges.2 > Tested-by: Jim Quinlan > Note that we'll still need to sort out the arm/keystone warnings from > the original patch. Do we have anyone on the CC list who knows that > platform a little better to figure out if the ifdef solution would work? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
On Mon, Sep 7, 2020 at 5:16 AM Lorenzo Pieralisi wrote: > > On Thu, Aug 27, 2020 at 09:29:59AM -0400, Jim Quinlan wrote: > > On Thu, Aug 27, 2020 at 2:35 AM Christoph Hellwig wrote: > > > > > > On Tue, Aug 25, 2020 at 10:40:27AM -0700, Florian Fainelli wrote: > > > > Hi, > > > > > > > > On 8/24/2020 12:30 PM, Jim Quinlan wrote: > > > >> > > > >> Patchset Summary: > > > >>Enhance a PCIe host controller driver. Because of its unusual > > > >> design > > > >>we are foced to change dev->dma_pfn_offset into a more general role > > > >>allowing multiple offsets. See the 'v1' notes below for more info. > > > > > > > > We are version 11 and counting, and it is not clear to me whether there > > > > is > > > > any chance of getting these patches reviewed and hopefully merged for > > > > the > > > > 5.10 merge window. > > > > > > > > There are a lot of different files being touched, so what would be the > > > > ideal way of routing those changes towards inclusion? > > > > > > FYI, I offered to take the dma-mapping bits through the dma-mapping tree. > > > I have a bit of a backlog, but plan to review and if Jim is ok with that > > > apply the current version. > > Sounds good to me. > > Hi Jim, > > is the dependency now solved ? Should we review/take this series as > is for v5.10 through the PCI tree ? Hello Lorenzo, We are still working out a regression with the DMA offset commit on the RaspberryPi. Nicolas has found the root cause and we are now devising a solution. Thanks, Jim Quinlan Broadcom STB > > Thanks, > Lorenzo ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Mon, Sep 7, 2020 at 11:01 AM Nicolas Saenz Julienne
wrote:
>
> Hi Jim, sorry I'm a little late to the party, but was on vacation.
>
> On Thu, 2020-09-03 at 13:32 -0400, Jim Quinlan wrote:
> > On Wed, Sep 2, 2020 at 8:52 PM Nathan Chancellor
> > wrote:
> > > On Wed, Sep 02, 2020 at 05:36:29PM -0700, Florian Fainelli wrote:
> > > >
> > > > On 9/2/2020 3:38 PM, Nathan Chancellor wrote:
> > > > [snip]
> > > > > > Hello Nathan,
> > > > > >
> > > > > > Can you tell me how much memory your RPI has and if all of it is
> > > > >
> > > > > This is the 4GB version.
> > > > >
> > > > > > accessible by the PCIe device? Could you also please include the
> > > > > > DTS
> > > > > > of the PCIe node? IIRC, the RPI firmware does some mangling of the
> > > > > > PCIe DT before Linux boots -- could you describe what is going on
> > > > > > there?
> > > > >
> > > > > Unfortunately, I am not familiar with how to get this information. If
> > > > > you could provide some instructions for how to do so, I am more than
> > > > > happy to. I am not very knowleagable about the inner working of the
> > > > > Pi,
> > > > > I mainly use it as a test platform for making sure that LLVM does not
> > > > > cause problems on real devices.
> > > >
> > > > Can you bring the dtc application to your Pi root filesystem, and if
> > > > so, can
> > > > you run the following:
> > > >
> > > > dtc -I fs -O dtb /proc/device-tree -f > /tmp/device.dtb
> > >
> > > Sure, the result is attached.
> > >
> > > > or cat /sys/firmware/fdt > device.dtb
> > > >
> > > > and attach the resulting file?
> > > >
> > > > > > Finally, can you attach the text of the full boot log?
> > > > >
> > > > > I have attached a working and broken boot log. Thank you for the quick
> > > > > response!
> > > >
> > > > Is it possible for you to rebuild your kernel with CONFIG_MMC_DEBUG by
> > > > any
> > > > chance?
> > >
> > > Of course. A new log is attached with the debug output from that config.
> >
> > Hi Nicolas,
> >
> > Can you please help us out here? It appears that your commit
>
> It's dma_offset_from_dma_addr() that's causing trouble. It goes over all the
> dma regions and, if no region matches the phys address range, it returns 0.
> This isn't acceptable as is. In the past, we used to pass the offset
> nonetheless, which would make the phys address grow out of the dma memory area
> and fail the dma_capable() test.
>
> For example, RPi4 devices attached to the old interconnect see phys [0x0
> 0x3fff] at [0xc000 0x]. So say you're trying to convert
> physical address 0xfa00, you'll get 0 from dma_offset_from_phys_addr()
> (since your dma range only covers the first GB) to then test if 0xfa00 is
> dma_capable(), which it is, but for the wrong reasons. Causing DMA issues
> further down the line.
>
> I don't have a proper suggestion on how to solve this but here's the solution
> I
> used:
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 4c4646761afe..40fe3c97f2be 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -217,11 +217,19 @@ static inline u64 dma_offset_from_phys_addr(struct
> device *dev,
> {
> const struct bus_dma_region *m = dev->dma_range_map;
>
> - if (m)
> + if (m) {
> for (; m->size; m++)
> if (paddr >= m->cpu_start &&
> paddr - m->cpu_start < m->size)
> return m->offset;
> +
> + /*
> +* The physical address doesn't fit any of the DMA regions,
> +* return an impossible to fulfill offset.
> +*/
> + return -(1ULL << 46);
> + }
> +
> return 0;
> }
Hi Nicolas,
Thanks for looking into this. The concern I have with your solution
is that returning an arbitrarily large offset might overlap with an
improbable but valid usage. AFAIK there is nothing that disallows
mapping a device to anywhere within the 64bit range of PCIe space,
including up to say 0x.
As an alternative perhaps changing dma_capable() so that if the
dma_range_map is present then it validates that both ends of the
prospective DMA region get "hits" on one of the offset regions in the
map. Christoph, if you are okay with this I can quickly post a patch.
Regards,
Jim Quinlan
Broadcom STB
>
> I didn't catch this on my previous tests as I was using a newer bcm2711 SoC
> revision which expanded emmc2's DMA capabilities (can do 32 bit DMA as opposed
> to 30 bit).
>
> Regards,
> Nicolas
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Wed, Sep 2, 2020 at 8:52 PM Nathan Chancellor wrote: > > On Wed, Sep 02, 2020 at 05:36:29PM -0700, Florian Fainelli wrote: > > > > > > On 9/2/2020 3:38 PM, Nathan Chancellor wrote: > > [snip] > > > > Hello Nathan, > > > > > > > > Can you tell me how much memory your RPI has and if all of it is > > > > > > This is the 4GB version. > > > > > > > accessible by the PCIe device? Could you also please include the DTS > > > > of the PCIe node? IIRC, the RPI firmware does some mangling of the > > > > PCIe DT before Linux boots -- could you describe what is going on > > > > there? > > > > > > Unfortunately, I am not familiar with how to get this information. If > > > you could provide some instructions for how to do so, I am more than > > > happy to. I am not very knowleagable about the inner working of the Pi, > > > I mainly use it as a test platform for making sure that LLVM does not > > > cause problems on real devices. > > > > Can you bring the dtc application to your Pi root filesystem, and if so, can > > you run the following: > > > > dtc -I fs -O dtb /proc/device-tree -f > /tmp/device.dtb > > Sure, the result is attached. > > > or cat /sys/firmware/fdt > device.dtb > > > > and attach the resulting file? > > > > > > > > > Finally, can you attach the text of the full boot log? > > > > > > I have attached a working and broken boot log. Thank you for the quick > > > response! > > > > Is it possible for you to rebuild your kernel with CONFIG_MMC_DEBUG by any > > chance? > > Of course. A new log is attached with the debug output from that config. Hi Nicolas, Can you please help us out here? It appears that your commit 3d2cbb644836 "ARM: dts: bcm2711: Move emmc2 into its own bus" added the following line to the bcm2711.dtsi file: + dma-ranges = <0x0 0xc000 0x0 0x 0x4000>; and for some reason the eMMC is not operating properly w/ my commit.. Regardless, the only difference that my commit should make is to enforce the bounds of the dma_window (whereas the previous code adds/subs the pfn_offset everywhere). Thanks, Jim Quinlan PS If you would like to talk, let me know and we can make arrangements. > > > > I have a suspicion that this part of the DTS for the bcm2711.dtsi platform > > is at fault: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/bcm2711.dtsi#n264 > > > > and the resulting dma-ranges parsing is just not working for reasons to be > > determined. > > -- > > Florian > > Let me know if you need anything else out of me. > > Cheers, > Nathan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Wed, Sep 2, 2020 at 5:53 PM Nathan Chancellor wrote: > > On Mon, Aug 24, 2020 at 03:30:20PM -0400, Jim Quinlan wrote: > > The new field 'dma_range_map' in struct device is used to facilitate the > > use of single or multiple offsets between mapping regions of cpu addrs and > > dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only > > capable of holding a single uniform offset and had no region bounds > > checking. > > > > The function of_dma_get_range() has been modified so that it takes a single > > argument -- the device node -- and returns a map, NULL, or an error code. > > The map is an array that holds the information regarding the DMA regions. > > Each range entry contains the address offset, the cpu_start address, the > > dma_start address, and the size of the region. > > > > of_dma_configure() is the typical manner to set range 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_offset_range(dev, cpu_addr, dma_addr, size). > > > > Signed-off-by: Jim Quinlan > > --- > > arch/arm/include/asm/dma-mapping.h| 10 +-- > > arch/arm/mach-keystone/keystone.c | 17 +++-- > > arch/sh/drivers/pci/pcie-sh7786.c | 9 +-- > > arch/x86/pci/sta2x11-fixup.c | 7 +- > > drivers/acpi/arm64/iort.c | 5 +- > > drivers/base/core.c | 2 + > > 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 | 43 ++- > > drivers/of/of_private.h | 10 +-- > > drivers/of/unittest.c | 34 ++--- > > drivers/remoteproc/remoteproc_core.c | 8 ++- > > .../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +- > > drivers/usb/core/message.c| 9 ++- > > drivers/usb/core/usb.c| 7 +- > > include/linux/device.h| 4 +- > > include/linux/dma-direct.h| 8 +-- > > include/linux/dma-mapping.h | 36 ++ > > kernel/dma/coherent.c | 10 +-- > > kernel/dma/mapping.c | 66 + > > 23 files changed, 265 insertions(+), 115 deletions(-) > > Apologies if this has already been reported or is known but this commit > is now in next-20200902 and it causes my Raspberry Pi 4 to no longer > make it to userspace, instead spewing mmc errors: > > That commit causes my Raspberry Pi 4 to no longer make it to userspace, > instead spewing mmc errors: > > [0.00] Booting Linux on physical CPU 0x00 [0x410fd083] > [0.00] Linux version 5.9.0-rc3-4-geef520b232c6-dirty > (nathan@ubuntu-n2-xlarge-x86) (ClangBuiltLinux clang version 12.0.0 > (https://github.com/llvm/llvm-project.git > b21ddded8f04fee925bbf9e6458347104b5b99eb), LLD 12.0.0 > (https://github.com/llvm/llvm-project.git > b21ddded8f04fee925bbf9e6458347104b5b99eb)) #1 SMP PREEMPT Wed Sep 2 13:48:49 > MST 2020 > [0.00] Machine model: Raspberry Pi 4 Model B Rev 1.2 > ... > [1.459752] raspberrypi-firmware soc:firmware: Attached to firmware from > 2020-08-24T18:50:56 > [1.57] dwc2 fe98.usb: supply vusb_d not found, using dummy > regulator > [1.507454] dwc2 fe98.usb: supply vusb_a not found, using dummy > regulator > [1.615547] dwc2 fe98.usb: EPs: 8, dedicated fifos, 4080 entries in > SPRAM > [1.627537] sdhci-iproc fe30.sdhci: allocated mmc-pwrseq > [1.665497] mmc0: SDHCI controller on fe30.sdhci [fe30.sdhci] > using PIO > [1.690601] mmc0: queuing unknown CIS tuple 0x80 (2 bytes) > [1.697892] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) > [1.705173] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) > [1.713788] mmc0: queuing unknown CIS tuple 0x80 (7 bytes) > [1.721228] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) > [1.732062] mmc1: SDHCI controller on fe34.emmc2 [fe34.emmc2] > using ADMA > [1.741828] ALSA device list: > [1.744885] No soundcards found. > [1.748540] Waiting for root device PARTUUID=45a8dd8a-02... > [1.788865] random: fast init done > [1.793489] mmc1: unrecognised SCR structure version 4 > [1.798814] mmc1: error -22 whilst initialising SD card > [1.813969] mmc0: new high speed SDIO card at address 0001 > [1.883178] mmc1: unrecognised SCR structure version 2 > [1.888423] mmc1: error -22 whilst initialising SD card > [1.964069] mmc1: unrecognised SCR structure version 4 > [1.969314] mmc1: error -22 whilst initialising SD card > [2.061225] mmc1: unrecognised
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Tue, Sep 1, 2020 at 4:24 AM Christoph Hellwig wrote: > > I've applied this to the dma-mapping tree. > > I had to resolve a conflict in drivers/of/address.c with a recent > mainline commit. I also applied the minor tweaks Andy pointed out > plus a few more style changes. A real change is that I changed the > prototype for dma_copy_dma_range_map to require less boilerplate code. > > The result is here: > > > http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/eef520b232c60e74eb8b33a5a7863ad8f2b4a5c7 > > please double check that everyting works as expected. Tested-by: Jim Quinlan Thanks Christoph Jim > > I can cut a stable branch with this if you need it for other trees, but > I'd like to wait a few days to see if there is any fallout first. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
On Thu, Aug 27, 2020 at 2:35 AM Christoph Hellwig wrote: > > On Tue, Aug 25, 2020 at 10:40:27AM -0700, Florian Fainelli wrote: > > Hi, > > > > On 8/24/2020 12:30 PM, Jim Quinlan wrote: > >> > >> Patchset Summary: > >>Enhance a PCIe host controller driver. Because of its unusual design > >>we are foced to change dev->dma_pfn_offset into a more general role > >>allowing multiple offsets. See the 'v1' notes below for more info. > > > > We are version 11 and counting, and it is not clear to me whether there is > > any chance of getting these patches reviewed and hopefully merged for the > > 5.10 merge window. > > > > There are a lot of different files being touched, so what would be the > > ideal way of routing those changes towards inclusion? > > FYI, I offered to take the dma-mapping bits through the dma-mapping tree. > I have a bit of a backlog, but plan to review and if Jim is ok with that > apply the current version. Sounds good to me. Thanks, Jim ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
Hi Andy,
On Tue, Aug 25, 2020 at 5:54 AM Andy Shevchenko
wrote:
>
> On Mon, Aug 24, 2020 at 03:30:20PM -0400, Jim Quinlan wrote:
> > The new field 'dma_range_map' in struct device is used to facilitate the
> > use of single or multiple offsets between mapping regions of cpu addrs and
> > dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
> > capable of holding a single uniform offset and had no region bounds
> > checking.
> >
> > The function of_dma_get_range() has been modified so that it takes a single
> > argument -- the device node -- and returns a map, NULL, or an error code.
> > The map is an array that holds the information regarding the DMA regions.
> > Each range entry contains the address offset, the cpu_start address, the
> > dma_start address, and the size of the region.
> >
> > of_dma_configure() is the typical manner to set range 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_offset_range(dev, cpu_addr, dma_addr, size).
>
> ...
>
> > + /*
> > + * Record all info in the generic DMA ranges array for struct device.
> > + */
> > + *map = r;
> > + for_each_of_range(, ) {
> > + pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
> > + range.bus_addr, range.cpu_addr, range.size);
> > + r->cpu_start = range.cpu_addr;
> > + r->dma_start = range.bus_addr;
> > + r->size = range.size;
>
> > + r->offset = (u64)range.cpu_addr - (u64)range.bus_addr;
>
> What's the point in explicit castings to the same type?
No point. If I have to send out another version I will fix this.
>
> > + r++;
> > + }
>
> ...
>
> > + phys_addr_t paddr;
> > + dma_addr_t dma_addr;
> > + struct device dev_bogus;
>
> > unittest(paddr == expect_paddr,
> > - "of_dma_get_range wrong phys addr (%llx) on node
> > %pOF", paddr, np);
> > + "of_dma_get_range: wrong phys addr %llx (expecting
> > %llx) on node %pOF\n",
> > + (u64)paddr, expect_paddr, np);
>
> %llx -> %pap
This was intentional -- I'm aware of %pap and %pad. The problem is
that %pa[pd] print out a zero-filled 16 character number regardless
of what the number is. For example, 1 is "0x0001",
whereas using %llx yields "1".
>
> > unittest(dma_addr == expect_dma_addr,
> > - "of_dma_get_range wrong DMA addr (%llx) on node
> > %pOF", dma_addr, np);
> > + "of_dma_get_range: wrong DMA addr %llx (expecting
> > %llx) on node %pOF\n",
> > + (u64)dma_addr, expect_dma_addr, np);
>
> %llx -> %pad
>
> ...
>
> > + if (mem->use_dev_dma_pfn_offset) {
> > + u64 base_addr = PFN_PHYS((u64)mem->pfn_base);
>
> Do we need explicit casting here?
I don't think it is needed. However, the "(u64)" is useless though
since the macro recasts it to a phys_addr_t.
If there is another version of this submission I will change this.
>
> > +
> > + return base_addr - dma_offset_from_phys_addr(dev, base_addr);
> > + }
>
> ...
>
> > +int dma_set_offset_range(struct device *dev, phys_addr_t cpu_start,
> > + dma_addr_t dma_start, u64 size)
> > +{
> > + struct bus_dma_region *map;
> > + u64 offset = (u64)cpu_start - (u64)dma_start;
> > +
> > + if (dev->dma_range_map) {
> > + dev_err(dev, "attempt to add DMA range to existing map\n");
> > + return -EINVAL;
> > + }
>
> Wouldn't be better to do an assignment of offset here?
IIRC this was what Christoph requested. It has actually gone back and
forth over the versions of this submission.
>
> > + if (!offset)
> > + return 0;
> > +
> > + map = kcalloc(2, sizeof(*map), GFP_KERNEL);
> > + if (!map)
> > + return -ENOMEM;
> > + map[0].cpu_start = cpu_start;
> > + map[0].dma_start = dma_start;
> > + map[0].offset = offset;
> > + map[0].size = size;
> > + dev->dma_range_map = map;
> > +
> > + return 0;
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko
Thanks again,
Jim
>
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
The new field 'dma_range_map' in struct device is used to facilitate the
use of single or multiple offsets between mapping regions of cpu addrs and
dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
capable of holding a single uniform offset and had no region bounds
checking.
The function of_dma_get_range() has been modified so that it takes a single
argument -- the device node -- and returns a map, NULL, or an error code.
The map is an array that holds the information regarding the DMA regions.
Each range entry contains the address offset, the cpu_start address, the
dma_start address, and the size of the region.
of_dma_configure() is the typical manner to set range 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_offset_range(dev, cpu_addr, dma_addr, size).
Signed-off-by: Jim Quinlan
---
arch/arm/include/asm/dma-mapping.h| 10 +--
arch/arm/mach-keystone/keystone.c | 17 +++--
arch/sh/drivers/pci/pcie-sh7786.c | 9 +--
arch/x86/pci/sta2x11-fixup.c | 7 +-
drivers/acpi/arm64/iort.c | 5 +-
drivers/base/core.c | 2 +
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 | 43 ++-
drivers/of/of_private.h | 10 +--
drivers/of/unittest.c | 34 ++---
drivers/remoteproc/remoteproc_core.c | 8 ++-
.../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +-
drivers/usb/core/message.c| 9 ++-
drivers/usb/core/usb.c| 7 +-
include/linux/device.h| 4 +-
include/linux/dma-direct.h| 8 +--
include/linux/dma-mapping.h | 36 ++
kernel/dma/coherent.c | 10 +--
kernel/dma/mapping.c | 66 +
23 files changed, 265 insertions(+), 115 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index bdd80ddbca34..c21893f683b5 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -35,8 +35,11 @@ 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) {
+ phys_addr_t paddr = PFN_PHYS(pfn);
+
+ pfn -= PFN_DOWN(dma_offset_from_phys_addr(dev, paddr));
+ }
return (dma_addr_t)__pfn_to_bus(pfn);
}
@@ -45,8 +48,7 @@ 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;
-
+ pfn += PFN_DOWN(dma_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..78808942ad1c 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
@@ -24,8 +25,6 @@
#include "keystone.h"
-static unsigned long keystone_dma_pfn_offset __read_mostly;
-
static int keystone_platform_notifier(struct notifier_block *nb,
unsigned long event, void *data)
{
@@ -38,9 +37,12 @@ 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_set_offset_range(dev, KEYSTONE_HIGH_PHYS_START,
+ KEYSTONE_LOW_PHYS_START,
+ KEYSTONE_HIGH_PHYS_SIZE);
+ dev_err(dev, "set dma_offset%08llx%s\n",
+ KEYSTONE_HIGH_PHYS_START - KEYSTONE_LOW_PHYS_START,
+ ret ? " failed" : "");
}
return NOTIFY_OK;
}
@@ -51,11 +53,8 @@ static struct notifier_block platform_nb = {
static void __init keystone_init(void)
{
- if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START) {
- keystone_dma_pfn_offset = PFN_DOWN(KEYSTONE_HIGH_PHYS_START -
- KEYSTONE_LOW_PHYS_START);
+ if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START)
[PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
Patchset Summary: Enhance a PCIe host controller driver. Because of its unusual design we are foced to change dev->dma_pfn_offset into a more general role allowing multiple offsets. See the 'v1' notes below for more info. v11: Commit: "device-mapping: Introduce DMA range map, supplanting ..." -- Rebased to latest torvalds, Aug 20, 2020. -- Minor change in of_dma_get_range() to satisfy the kernel's robot tester. -- Use of PFN_DOWN(), PFN_PHYS() instead of explicit shifts (Andy) -- Eliminate extra return in dma_offset_from_xxx_addr() (Andy) -- Change dma_set_offset_range() to correctly handle the case of pre-existing DMA map and zero offset. v10: Commit: "device-mapping: Introduce DMA range map, supplanting ..." -- change title of commit; "bus core:" => "device-mapping:" -- instead of allocating the DMA map with devm, use kcalloc and call kfree() during device_release(). (RobH) Also, for three cases that want to use the same DMA map, copy the dma_range_map using a helper function. -- added a missing 'return = 0;' to of_dma_get_range(). (Nicolas) -- removed dma_range_overlaps(); instead return error if there is an existing DMA map. (Christoph). Commit: "PCI: brcmstb: Set additional internal memory DMA ..." -- Changed constant 1 to 1ULL. (Nicolas) Commit: "ata: ahci_brcm: Fix use of BCM7216 reset controller" This commit has been removed from this patchset and will be submitted on its own. v9: Commit: "device core: Introduce DMA range map, supplanting ..." -- A number of code improvements were implemented as suggested by ChristophH. Unfortunately, some of these changes reversed the implemented suggestions of other reviewers; for example, the new macros PFN_DMA_ADDR(), DMA_ADDR_PFN() have been pulled. v8: Commit: "device core: Introduce DMA range map, supplanting ..." -- To satisfy a specific m68 compile configuration, I moved the 'struct bus_dma_region; definition out of #ifdef CONFIG_HAS_DMA and also defined three inline functions for !CONFIG_HAS_DMA (kernel test robot). -- The sunXi drivers -- suc4i_csi, sun6i_csi, cedrus_hw -- set a pfn_offset outside of_dma_configure() but the code offers no insight on the size of the translation window. V7 had me using SIZE_MAX as the size. I have since contacted the sunXi maintainer and he said that using a size of SZ_4G would cover sunXi configurations. v7: Commit: "device core: Introduce DMA range map, supplanting ..." -- remove second kcalloc/copy in device.c (AndyS) -- use PTR_ERR_OR_ZERO() and PHYS_PFN() (AndyS) -- indentation, sizeof(struct ...) => sizeof(*r) (AndyS) -- add pfn.h definitions: PFN_DMA_ADDR(), DMA_ADDR_PFN() (AndyS) -- Fixed compile error in "sun6i_csi.c" (kernel test robot) Commit "ata: ahci_brcm: Fix use of BCM7216 reset controller" -- correct name of function in the commit msg (SergeiS) v6: Commit "device core: Introduce DMA range map": -- of_dma_get_range() now takes a single argument and returns either NULL, a valid map, or an ERR_PTR. (Robin) -- offsets are no longer a PFN value but an actual address. (Robin) -- the bus_dma_region struct stores the range size instead of the cpu_end and pci_end values. (Robin) -- devices that were setting a single offset with no boundaries have been modified to have boundaries; in a few places where this information was unavilable a /* FIXME: ... */ comment was added. (Robin) -- dma_attach_offset_range() can be called when an offset map already exists; if it's range is already present nothing is done and success is returned. (Robin) All commits: -- Man name/style/corrections/etc changed (Bjorn) -- rebase to Torvalds master v5: Commit "device core: Introduce multiple dma pfn offsets" -- in of/address.c: "map_size = 0" => "*map_size = 0" -- use kcalloc instead of kzalloc (AndyS) -- use PHYS_ADDR_MAX instead of "~(phys_addr_t)0" Commit "PCI: brcmstb: Set internal memory viewport sizes" -- now gives error on missing dma-ranges property. Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips" -- removed "Allof:" from brcm,scb-sizes definition (RobH) All Commits: -- indentation style, use max chars 100 (AndyS) -- rebased to torvalds master v4: Commit "device core: Introduce multiple dma pfn offsets" -- of_dma_get_range() does not take a dev param but instead takes two "out" params: map and map_size. We do this so that the code that parses dma-ranges is separate from the code that modifies 'dev'. (Nicolas) -- the separate case of having a single pfn offset has been removed and is now processed by going through the map array. (Nicolas) -- move attach_uniform_dma_pfn_offset() from of/address.c to dma/mapping.c so that it does not depend on CONFIG_OF. (Nicolas) -- devm_kcalloc => devm_kzalloc (DanC) -- add/fix assignment to
Re: [PATCH RESEND v10 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
Hi Anday,
On Tue, Aug 18, 2020 at 4:14 AM Andy Shevchenko
wrote:
>
> On Mon, Aug 17, 2020 at 05:53:09PM -0400, Jim Quinlan wrote:
> > The new field 'dma_range_map' in struct device is used to facilitate the
> > use of single or multiple offsets between mapping regions of cpu addrs and
> > dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
> > capable of holding a single uniform offset and had no region bounds
> > checking.
> >
> > The function of_dma_get_range() has been modified so that it takes a single
> > argument -- the device node -- and returns a map, NULL, or an error code.
> > The map is an array that holds the information regarding the DMA regions.
> > Each range entry contains the address offset, the cpu_start address, the
> > dma_start address, and the size of the region.
> >
> > of_dma_configure() is the typical manner to set range 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_offset_range(dev, cpu_addr, dma_addr, size).
>
> ...
>
> > + if (dev) {
> > + phys_addr_t paddr = PFN_PHYS(pfn);
> > +
>
> > + pfn -= (dma_offset_from_phys_addr(dev, paddr) >> PAGE_SHIFT);
>
> PFN_DOWN() ?
Yep.
>
> > + }
>
> ...
>
> > + pfn += (dma_offset_from_dma_addr(dev, addr) >> PAGE_SHIFT);
>
> Ditto.
Yep.
>
>
> ...
>
> > +static inline u64 dma_offset_from_dma_addr(struct device *dev, dma_addr_t
> > dma_addr)
> > +{
> > + const struct bus_dma_region *m = dev->dma_range_map;
> > +
> > + if (!m)
> > + return 0;
> > + for (; m->size; m++)
> > + if (dma_addr >= m->dma_start && dma_addr - m->dma_start <
> > m->size)
> > + return m->offset;
> > + return 0;
> > +}
> > +
> > +static inline u64 dma_offset_from_phys_addr(struct device *dev,
> > phys_addr_t paddr)
> > +{
> > + const struct bus_dma_region *m = dev->dma_range_map;
> > +
> > + if (!m)
> > + return 0;
> > + for (; m->size; m++)
> > + if (paddr >= m->cpu_start && paddr - m->cpu_start < m->size)
> > + return m->offset;
> > + return 0;
> > +}
>
> Perhaps for these the form with one return 0 is easier to read
>
> if (m) {
> for (; m->size; m++)
> if (paddr >= m->cpu_start && paddr - m->cpu_start <
> m->size)
> return m->offset;
> }
> return 0;
>
> ?
I see what you are saying but I don't think there is enough difference
between the two to justify changing it.
>
> ...
>
> > + if (mem->use_dev_dma_pfn_offset) {
> > + u64 base_addr = (u64)mem->pfn_base << PAGE_SHIFT;
>
> PFN_PHYS() ?
Yep.
>
> > +
> > + return base_addr - dma_offset_from_phys_addr(dev, base_addr);
> > + }
>
> ...
>
> > + * It returns -ENOMEM if out of memory, 0 otherwise.
>
> This doesn't describe cases dev->dma_range_map != NULL and offset == 0.
Okay, I'll fix this.
>
> > +int dma_set_offset_range(struct device *dev, phys_addr_t cpu_start,
> > + dma_addr_t dma_start, u64 size)
> > +{
> > + struct bus_dma_region *map;
> > + u64 offset = (u64)cpu_start - (u64)dma_start;
> > +
> > + if (!offset)
> > + return 0;
> > +
> > + if (dev->dma_range_map) {
> > + dev_err(dev, "attempt to add DMA range to existing map\n");
> > + return -EINVAL;
> > + }
> > +
> > + map = kcalloc(2, sizeof(*map), GFP_KERNEL);
> > + if (!map)
> > + return -ENOMEM;
> > + map[0].cpu_start = cpu_start;
> > + map[0].dma_start = dma_start;
> > + map[0].offset = offset;
> > + map[0].size = size;
> > + dev->dma_range_map = map;
> > +
> > + return 0;
> > +}
>
> ...
>
> > +void *dma_copy_dma_range_map(const struct bus_dma_region *map)
> > +{
> > + int num_ranges;
> > + struct bus_dma_region *new_map;
> > + const struct bus_dma_region *r = map;
> > +
> > + for (num_ranges = 0; r->size; num_ranges++)
> > + r++;
>
> > + new_map = kcalloc(num_ranges + 1, sizeof(*map), GFP_KERNEL);
> > + if (new_map)
> > + memcpy(new_map, map, sizeof(*map) * num_ranges);
>
> Looks like krealloc() on the first glance...
It's not. We are making a distinct copy of the original, not resizing it.
>
> > +
> > + return new_map;
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko
Thanks again,
Jim
>
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH RESEND v10 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
The new field 'dma_range_map' in struct device is used to facilitate the
use of single or multiple offsets between mapping regions of cpu addrs and
dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
capable of holding a single uniform offset and had no region bounds
checking.
The function of_dma_get_range() has been modified so that it takes a single
argument -- the device node -- and returns a map, NULL, or an error code.
The map is an array that holds the information regarding the DMA regions.
Each range entry contains the address offset, the cpu_start address, the
dma_start address, and the size of the region.
of_dma_configure() is the typical manner to set range 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_offset_range(dev, cpu_addr, dma_addr, size).
Signed-off-by: Jim Quinlan
---
arch/arm/include/asm/dma-mapping.h| 10 +--
arch/arm/mach-keystone/keystone.c | 17 +++--
arch/sh/drivers/pci/pcie-sh7786.c | 9 +--
arch/sh/kernel/dma-coherent.c | 15 ++--
arch/x86/pci/sta2x11-fixup.c | 7 +-
drivers/acpi/arm64/iort.c | 5 +-
drivers/base/core.c | 2 +
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 | 43 ++-
drivers/of/of_private.h | 10 +--
drivers/of/unittest.c | 31 +---
drivers/remoteproc/remoteproc_core.c | 8 ++-
.../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +-
drivers/usb/core/message.c| 9 ++-
drivers/usb/core/usb.c| 7 +-
include/linux/device.h| 4 +-
include/linux/dma-direct.h| 8 +--
include/linux/dma-mapping.h | 36 ++
kernel/dma/coherent.c | 10 +--
kernel/dma/mapping.c | 65 +
24 files changed, 269 insertions(+), 122 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index bdd80ddbca34..2405afeb7957 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -35,8 +35,11 @@ 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) {
+ phys_addr_t paddr = PFN_PHYS(pfn);
+
+ pfn -= (dma_offset_from_phys_addr(dev, paddr) >> PAGE_SHIFT);
+ }
return (dma_addr_t)__pfn_to_bus(pfn);
}
@@ -45,8 +48,7 @@ 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;
-
+ pfn += (dma_offset_from_dma_addr(dev, addr) >> PAGE_SHIFT);
return pfn;
}
diff --git a/arch/arm/mach-keystone/keystone.c
b/arch/arm/mach-keystone/keystone.c
index 638808c4e122..78808942ad1c 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
@@ -24,8 +25,6 @@
#include "keystone.h"
-static unsigned long keystone_dma_pfn_offset __read_mostly;
-
static int keystone_platform_notifier(struct notifier_block *nb,
unsigned long event, void *data)
{
@@ -38,9 +37,12 @@ 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_set_offset_range(dev, KEYSTONE_HIGH_PHYS_START,
+ KEYSTONE_LOW_PHYS_START,
+ KEYSTONE_HIGH_PHYS_SIZE);
+ dev_err(dev, "set dma_offset%08llx%s\n",
+ KEYSTONE_HIGH_PHYS_START - KEYSTONE_LOW_PHYS_START,
+ ret ? " failed" : "");
}
return NOTIFY_OK;
}
@@ -51,11 +53,8 @@ static struct notifier_block platform_nb = {
static void __init keystone_init(void)
{
- if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START) {
- keystone_dma_pfn_offset = PFN_DOWN(KEYSTONE_HIGH_PHYS_START -
- KEYSTONE_LOW_PHYS_START);
+ if
[PATCH RESEND v10 00/11] PCI: brcmstb: enable PCIe for STB chips
Patchset Summary:
Enhance a PCIe host controller driver. Because of its unusual design
we are foced to change dev->dma_pfn_offset into a more general role
allowing multiple offsets. See the 'v1' notes below for more info.
v10:
Commit: "device-mapping: Introduce DMA range map, supplanting ..."
-- change title of commit; "bus core:" => "device-mapping:"
-- instead of allocating the DMA map with devm, use kcalloc
and call kfree() during device_release(). (RobH) Also,
for three cases that want to use the same DMA map, copy
the dma_range_map using a helper function.
-- added a missing 'return = 0;' to of_dma_get_range(). (Nicolas)
-- removed dma_range_overlaps(); instead return error if there
is an existing DMA map. (Christoph).
Commit: "PCI: brcmstb: Set additional internal memory DMA ..."
-- Changed constant 1 to 1ULL. (Nicolas)
Commit: "ata: ahci_brcm: Fix use of BCM7216 reset controller"
This commit has been removed from this patchset and will be
submitted on its own.
v9:
Commit: "device core: Introduce DMA range map, supplanting ..."
-- A number of code improvements were implemented as suggested by
ChristophH. Unfortunately, some of these changes reversed the
implemented suggestions of other reviewers; for example, the new
macros PFN_DMA_ADDR(), DMA_ADDR_PFN() have been pulled.
v8:
Commit: "device core: Introduce DMA range map, supplanting ..."
-- To satisfy a specific m68 compile configuration, I moved the 'struct
bus_dma_region; definition out of #ifdef CONFIG_HAS_DMA and also defined
three inline functions for !CONFIG_HAS_DMA (kernel test robot).
-- The sunXi drivers -- suc4i_csi, sun6i_csi, cedrus_hw -- set
a pfn_offset outside of_dma_configure() but the code offers no
insight on the size of the translation window. V7 had me using
SIZE_MAX as the size. I have since contacted the sunXi maintainer and
he said that using a size of SZ_4G would cover sunXi configurations.
v7:
Commit: "device core: Introduce DMA range map, supplanting ..."
-- remove second kcalloc/copy in device.c (AndyS)
-- use PTR_ERR_OR_ZERO() and PHYS_PFN() (AndyS)
-- indentation, sizeof(struct ...) => sizeof(*r) (AndyS)
-- add pfn.h definitions: PFN_DMA_ADDR(), DMA_ADDR_PFN() (AndyS)
-- Fixed compile error in "sun6i_csi.c" (kernel test robot)
Commit "ata: ahci_brcm: Fix use of BCM7216 reset controller"
-- correct name of function in the commit msg (SergeiS)
v6:
Commit "device core: Introduce DMA range map":
-- of_dma_get_range() now takes a single argument and returns either
NULL, a valid map, or an ERR_PTR. (Robin)
-- offsets are no longer a PFN value but an actual address. (Robin)
-- the bus_dma_region struct stores the range size instead of
the cpu_end and pci_end values. (Robin)
-- devices that were setting a single offset with no boundaries
have been modified to have boundaries; in a few places
where this information was unavilable a /* FIXME: ... */
comment was added. (Robin)
-- dma_attach_offset_range() can be called when an offset
map already exists; if it's range is already present
nothing is done and success is returned. (Robin)
All commits:
-- Man name/style/corrections/etc changed (Bjorn)
-- rebase to Torvalds master
v5:
Commit "device core: Introduce multiple dma pfn offsets"
-- in of/address.c: "map_size = 0" => "*map_size = 0"
-- use kcalloc instead of kzalloc (AndyS)
-- use PHYS_ADDR_MAX instead of "~(phys_addr_t)0"
Commit "PCI: brcmstb: Set internal memory viewport sizes"
-- now gives error on missing dma-ranges property.
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- removed "Allof:" from brcm,scb-sizes definition (RobH)
All Commits:
-- indentation style, use max chars 100 (AndyS)
-- rebased to torvalds master
v4:
Commit "device core: Introduce multiple dma pfn offsets"
-- of_dma_get_range() does not take a dev param but instead
takes two "out" params: map and map_size. We do this so
that the code that parses dma-ranges is separate from
the code that modifies 'dev'. (Nicolas)
-- the separate case of having a single pfn offset has
been removed and is now processed by going through the
map array. (Nicolas)
-- move attach_uniform_dma_pfn_offset() from of/address.c to
dma/mapping.c so that it does not depend on CONFIG_OF. (Nicolas)
-- devm_kcalloc => devm_kzalloc (DanC)
-- add/fix assignment to dev->dma_pfn_offset_map for func
attach_uniform_dma_pfn_offset() (DanC, Nicolas)
-- s/struct dma_pfn_offset_region/struct bus_dma_region/ (Nicolas)
-- s/attach_uniform_dma_pfn_offset/dma_attach_uniform_pfn_offset/
-- s/attach_dma_pfn_offset_map/dma_attach_pfn_offset_map/
-- More use of PFN_{PHYS,DOWN,UP}. (AndyS)
Commit "of: Include a dev param in of_dma_get_range()"
-- this commit was sqaushed with "device core: Introduce ..."
v3:
[PATCH v10 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
The new field 'dma_range_map' in struct device is used to facilitate the
use of single or multiple offsets between mapping regions of cpu addrs and
dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
capable of holding a single uniform offset and had no region bounds
checking.
The function of_dma_get_range() has been modified so that it takes a single
argument -- the device node -- and returns a map, NULL, or an error code.
The map is an array that holds the information regarding the DMA regions.
Each range entry contains the address offset, the cpu_start address, the
dma_start address, and the size of the region.
of_dma_configure() is the typical manner to set range 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_offset_range(dev, cpu_addr, dma_addr, size).
Signed-off-by: Jim Quinlan
---
arch/arm/include/asm/dma-mapping.h| 10 +--
arch/arm/mach-keystone/keystone.c | 17 +++--
arch/sh/drivers/pci/pcie-sh7786.c | 9 +--
arch/sh/kernel/dma-coherent.c | 15 ++--
arch/x86/pci/sta2x11-fixup.c | 7 +-
drivers/acpi/arm64/iort.c | 5 +-
drivers/base/core.c | 2 +
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 | 43 ++-
drivers/of/of_private.h | 10 +--
drivers/of/unittest.c | 31 +---
drivers/remoteproc/remoteproc_core.c | 8 ++-
.../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +-
drivers/usb/core/message.c| 9 ++-
drivers/usb/core/usb.c| 7 +-
include/linux/device.h| 4 +-
include/linux/dma-direct.h| 8 +--
include/linux/dma-mapping.h | 36 ++
kernel/dma/coherent.c | 10 +--
kernel/dma/mapping.c | 65 +
24 files changed, 269 insertions(+), 122 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index bdd80ddbca34..2405afeb7957 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -35,8 +35,11 @@ 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) {
+ phys_addr_t paddr = PFN_PHYS(pfn);
+
+ pfn -= (dma_offset_from_phys_addr(dev, paddr) >> PAGE_SHIFT);
+ }
return (dma_addr_t)__pfn_to_bus(pfn);
}
@@ -45,8 +48,7 @@ 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;
-
+ pfn += (dma_offset_from_dma_addr(dev, addr) >> PAGE_SHIFT);
return pfn;
}
diff --git a/arch/arm/mach-keystone/keystone.c
b/arch/arm/mach-keystone/keystone.c
index 638808c4e122..78808942ad1c 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
@@ -24,8 +25,6 @@
#include "keystone.h"
-static unsigned long keystone_dma_pfn_offset __read_mostly;
-
static int keystone_platform_notifier(struct notifier_block *nb,
unsigned long event, void *data)
{
@@ -38,9 +37,12 @@ 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_set_offset_range(dev, KEYSTONE_HIGH_PHYS_START,
+ KEYSTONE_LOW_PHYS_START,
+ KEYSTONE_HIGH_PHYS_SIZE);
+ dev_err(dev, "set dma_offset%08llx%s\n",
+ KEYSTONE_HIGH_PHYS_START - KEYSTONE_LOW_PHYS_START,
+ ret ? " failed" : "");
}
return NOTIFY_OK;
}
@@ -51,11 +53,8 @@ static struct notifier_block platform_nb = {
static void __init keystone_init(void)
{
- if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START) {
- keystone_dma_pfn_offset = PFN_DOWN(KEYSTONE_HIGH_PHYS_START -
- KEYSTONE_LOW_PHYS_START);
+ if
[PATCH v10 00/11] PCI: brcmstb: enable PCIe for STB chips
Patchset Summary:
Enhance a PCIe host controller driver. Because of its unusual design
we are foced to change dev->dma_pfn_offset into a more general role
allowing multiple offsets. See the 'v1' notes below for more info.
v10:
Commit: "device-mapping: Introduce DMA range map, supplanting ..."
-- change title of commit; "bus core:" => "device-mapping:"
-- instead of allocating the DMA map with devm, use kcalloc
and call kfree() during device_release(). (RobH) Also,
for three cases that want to use the same DMA map, copy
the dma_range_map using a helper function.
-- added a missing 'return = 0;' to of_dma_get_range(). (Nicolas)
-- removed dma_range_overlaps(); instead return error if there
is an existing DMA map. (Christoph).
Commit: "PCI: brcmstb: Set additional internal memory DMA ..."
-- Changed constant 1 to 1ULL. (Nicolas)
Commit: "ata: ahci_brcm: Fix use of BCM7216 reset controller"
This commit has been removed from this patchset and will be
submitted on its own.
v9:
Commit: "device core: Introduce DMA range map, supplanting ..."
-- A number of code improvements were implemented as suggested by
ChristophH. Unfortunately, some of these changes reversed the
implemented suggestions of other reviewers; for example, the new
macros PFN_DMA_ADDR(), DMA_ADDR_PFN() have been pulled.
v8:
Commit: "device core: Introduce DMA range map, supplanting ..."
-- To satisfy a specific m68 compile configuration, I moved the 'struct
bus_dma_region; definition out of #ifdef CONFIG_HAS_DMA and also defined
three inline functions for !CONFIG_HAS_DMA (kernel test robot).
-- The sunXi drivers -- suc4i_csi, sun6i_csi, cedrus_hw -- set
a pfn_offset outside of_dma_configure() but the code offers no
insight on the size of the translation window. V7 had me using
SIZE_MAX as the size. I have since contacted the sunXi maintainer and
he said that using a size of SZ_4G would cover sunXi configurations.
v7:
Commit: "device core: Introduce DMA range map, supplanting ..."
-- remove second kcalloc/copy in device.c (AndyS)
-- use PTR_ERR_OR_ZERO() and PHYS_PFN() (AndyS)
-- indentation, sizeof(struct ...) => sizeof(*r) (AndyS)
-- add pfn.h definitions: PFN_DMA_ADDR(), DMA_ADDR_PFN() (AndyS)
-- Fixed compile error in "sun6i_csi.c" (kernel test robot)
Commit "ata: ahci_brcm: Fix use of BCM7216 reset controller"
-- correct name of function in the commit msg (SergeiS)
v6:
Commit "device core: Introduce DMA range map":
-- of_dma_get_range() now takes a single argument and returns either
NULL, a valid map, or an ERR_PTR. (Robin)
-- offsets are no longer a PFN value but an actual address. (Robin)
-- the bus_dma_region struct stores the range size instead of
the cpu_end and pci_end values. (Robin)
-- devices that were setting a single offset with no boundaries
have been modified to have boundaries; in a few places
where this information was unavilable a /* FIXME: ... */
comment was added. (Robin)
-- dma_attach_offset_range() can be called when an offset
map already exists; if it's range is already present
nothing is done and success is returned. (Robin)
All commits:
-- Man name/style/corrections/etc changed (Bjorn)
-- rebase to Torvalds master
v5:
Commit "device core: Introduce multiple dma pfn offsets"
-- in of/address.c: "map_size = 0" => "*map_size = 0"
-- use kcalloc instead of kzalloc (AndyS)
-- use PHYS_ADDR_MAX instead of "~(phys_addr_t)0"
Commit "PCI: brcmstb: Set internal memory viewport sizes"
-- now gives error on missing dma-ranges property.
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- removed "Allof:" from brcm,scb-sizes definition (RobH)
All Commits:
-- indentation style, use max chars 100 (AndyS)
-- rebased to torvalds master
v4:
Commit "device core: Introduce multiple dma pfn offsets"
-- of_dma_get_range() does not take a dev param but instead
takes two "out" params: map and map_size. We do this so
that the code that parses dma-ranges is separate from
the code that modifies 'dev'. (Nicolas)
-- the separate case of having a single pfn offset has
been removed and is now processed by going through the
map array. (Nicolas)
-- move attach_uniform_dma_pfn_offset() from of/address.c to
dma/mapping.c so that it does not depend on CONFIG_OF. (Nicolas)
-- devm_kcalloc => devm_kzalloc (DanC)
-- add/fix assignment to dev->dma_pfn_offset_map for func
attach_uniform_dma_pfn_offset() (DanC, Nicolas)
-- s/struct dma_pfn_offset_region/struct bus_dma_region/ (Nicolas)
-- s/attach_uniform_dma_pfn_offset/dma_attach_uniform_pfn_offset/
-- s/attach_dma_pfn_offset_map/dma_attach_pfn_offset_map/
-- More use of PFN_{PHYS,DOWN,UP}. (AndyS)
Commit "of: Include a dev param in of_dma_get_range()"
-- this commit was sqaushed with "device core: Introduce ..."
v3:
Re: [PATCH v9 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset
On Sat, Aug 1, 2020 at 1:17 PM Nicolas Saenz Julienne
wrote:
>
> Hi Jim, here's some comments after testing your series against RPi4.
>
> On Fri, 2020-07-24 at 16:33 -0400, Jim Quinlan wrote:
> > The new field 'dma_range_map' in struct device is used to facilitate the
> > use of single or multiple offsets between mapping regions of cpu addrs and
> > dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
> > capable of holding a single uniform offset and had no region bounds
> > checking.
> >
> > The function of_dma_get_range() has been modified so that it takes a single
> > argument -- the device node -- and returns a map, NULL, or an error code.
> > The map is an array that holds the information regarding the DMA regions.
> > Each range entry contains the address offset, the cpu_start address, the
> > dma_start address, and the size of the region.
> >
> > of_dma_configure() is the typical manner to set range 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_offset_range(dev, cpu_addr, dma_addr, size).
> >
> > Signed-off-by: Jim Quinlan
> > ---
>
> [...]
>
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index 8eea3f6e29a4..4b718d199efe 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -918,33 +918,33 @@ void __iomem *of_io_request_and_map(struct
> > device_node *np, int index,
> > }
> > EXPORT_SYMBOL(of_io_request_and_map);
> >
> > +#ifdef CONFIG_HAS_DMA
> > /**
> > - * of_dma_get_range - Get DMA range info
> > + * of_dma_get_range - Get DMA range info and put it into a map array
> > * @np: device node to get DMA range info
> > - * @dma_addr:pointer to store initial DMA address of DMA range
> > - * @paddr: pointer to store initial CPU address of DMA range
> > - * @size:pointer to store size of DMA range
> > + * @map: dma range structure to return
> > *
> > * Look in bottom up direction for the first "dma-ranges" property
> > - * and parse it.
> > - * dma-ranges format:
> > + * and parse it. Put the information into a DMA offset map array.
> > + *
> > + * dma-ranges format:
> > * DMA addr (dma_addr) : naddr cells
> > * CPU addr (phys_addr_t) : pna cells
> > * size: nsize cells
> > *
> > - * It returns -ENODEV if "dma-ranges" property was not found
> > - * for this device in DT.
> > + * It returns -ENODEV if "dma-ranges" property was not found for this
> > + * device in the DT.
> > */
> > -int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr,
> > u64 *size)
> > +int of_dma_get_range(struct device_node *np, const struct bus_dma_region
> > **map)
> > {
> > struct device_node *node = of_node_get(np);
> > const __be32 *ranges = NULL;
> > - int len;
> > - int ret = 0;
> > bool found_dma_ranges = false;
> > struct of_range_parser parser;
> > struct of_range range;
> > - u64 dma_start = U64_MAX, dma_end = 0, dma_offset = 0;
> > + struct bus_dma_region *r;
> > + int len, num_ranges = 0;
> > + int ret;
> >
> > while (node) {
> > ranges = of_get_property(node, "dma-ranges", );
> > @@ -970,44 +970,35 @@ int of_dma_get_range(struct device_node *np, u64
> > *dma_addr, u64 *paddr, u64 *siz
> > }
> >
> > of_dma_range_parser_init(, node);
> > + for_each_of_range(, )
> > + num_ranges++;
> > +
> > + of_dma_range_parser_init(, node);
> > +
> > + ret = -ENOMEM;
> > + r = kcalloc(num_ranges + 1, sizeof(*r), GFP_KERNEL);
> > + if (!r)
> > + goto out;
> >
> > + /*
> > + * Record all info in the generic DMA ranges array for struct device.
> > + */
> > + *map = r;
> > for_each_of_range(, ) {
> > pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
> >range.bus_addr, range.cpu_addr, range.size);
> > -
> > - if (dma_offset && range.cpu_addr - range.bus_addr !=
> > dma_offset) {
> > - pr_warn("Can't handle multiple dma-ranges with
> > different offsets on node(%pOF)\n", node);
> > - /* Don't error out as we'd break some existing DTs */
> > - continue;
> > - }
> > - dma_offset = range.cpu_addr - range.bus_addr;
> > -
> > - /* Take lower and upper limits */
> > - if (range.bus_addr < dma_start)
> > - dma_start = range.bus_addr;
> > - if (range.bus_addr + range.size > dma_end)
> > - dma_end = range.bus_addr + range.size;
> > - }
> > -
> > - if (dma_start >= dma_end) {
> > - ret = -EINVAL;
> > - pr_debug("Invalid DMA ranges configuration on node(%pOF)\n",
> > - node);
> > - goto out;
> > + r->cpu_start = range.cpu_addr;
> >
Re: [PATCH v9 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset
On Thu, Jul 30, 2020 at 1:05 PM Nicolas Saenz Julienne
wrote:
>
> Hi Jim,
>
> On Fri, 2020-07-24 at 16:33 -0400, Jim Quinlan wrote:
> > static void __init of_unittest_pci_dma_ranges(void)
> > diff --git a/drivers/pci/controller/pcie-brcmstb.c
> > b/drivers/pci/controller/pcie-brcmstb.c
> > index bfc2542d54a8..8dacb9d3b7b6 100644
> > --- a/drivers/pci/controller/pcie-brcmstb.c
> > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > @@ -1197,6 +1197,7 @@ static int brcm_pcie_probe(struct platform_device
> > *pdev)
> > ret = brcm_phy_start(pcie);
> > if (ret) {
> > dev_err(pcie->dev, "failed to start phy\n");
> > + reset_control_assert(pcie->rescal);
> > return ret;
> > }
>
> I think this sneaked in from another patch.
Thanks Nicolas. BTW, at some point will you be able to test my
patchset on the RP4 to see if I broke anything?
Thanks again,
Jim
>
> Regards,
> Nicolas
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset
On Wed, Jul 29, 2020 at 10:28 AM Rob Herring wrote: > > On Wed, Jul 29, 2020 at 12:19 AM Christoph Hellwig wrote: > > > > On Tue, Jul 28, 2020 at 02:24:51PM -0400, Jim Quinlan wrote: > > > I started using devm_kcalloc() but at least two reviewers convinced me > > > to just use kcalloc(). In addition, when I was using devm_kcalloc() > > > it was awkward because 'dev' is not available to this function. > > > > > > It comes down to whether unbind/binding the device N times is actually > > > a reasonable usage. As for my experience I've seen two cases: (1) my > > > overnight "bind/unbind the PCIe RC driver" script, and we have a > > > customer who does an unbind/bind as a hail mary to bring back life to > > > their dead EP device. If the latter case happens repeatedly, there > > > are bigger problems. > > > > We can't just leak the allocations. Do you have a pointer to the > > arguments against managed resources? I'm generally not a huge fan > > of the managed resources, but for a case like this they actually seem > > useful. If we don't use the managed resources we'll at leat need > > to explicitly free the resources when freeing the device. > > The lifetime for devm_kcalloc may not be what we want here. devm > allocs are freed on probe fail or remove, not on freeing the device > (there is a just in case free there too though). What do you suggest doing as an alternative? Jim > > > Rob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset
On Wed, Jul 29, 2020 at 2:19 AM Christoph Hellwig wrote: > > On Tue, Jul 28, 2020 at 02:24:51PM -0400, Jim Quinlan wrote: > > I started using devm_kcalloc() but at least two reviewers convinced me > > to just use kcalloc(). In addition, when I was using devm_kcalloc() > > it was awkward because 'dev' is not available to this function. > > > > It comes down to whether unbind/binding the device N times is actually > > a reasonable usage. As for my experience I've seen two cases: (1) my > > overnight "bind/unbind the PCIe RC driver" script, and we have a > > customer who does an unbind/bind as a hail mary to bring back life to > > their dead EP device. If the latter case happens repeatedly, there > > are bigger problems. > > We can't just leak the allocations. Do you have a pointer to the > arguments against managed resources? I'm generally not a huge fan > of the managed resources, but for a case like this they actually seem > useful. If we don't use the managed resources we'll at leat need > to explicitly free the resources when freeing the device. Actually, there were no arguments for using an unmanaged kcalloc, just comments [1], [2]. When it was rightly suggested to have 'dev' dropped from of_dma_range() [3], I submitted v6 to accomplish this. But now of_dma_range() would call kcalloc(), and then of_dma_configure() was required to "dup" the result, requiring a devm_kcalloc(), memcpy(), and kfree(). This was awkward, so considering [1], [2], I dropped the devm_kcalloc() and submitted v7. So I can easily revert to the awkward code of v6. But I'm hoping you have a more elegant solution :-) Thanks, Jim [1] [v6, Andy Shevchenko] > + /* We want the offset map to be device-managed, so alloc & copy > */ > + dev->dma_range_map = devm_kcalloc(dev, num_ranges + 1, > sizeof(*r), > + GFP_KERNEL); The question is how many times per device lifetime this can be called? [2] [v2, Andy Shevchenko] > + r = devm_kzalloc(dev, r_size, GFP_KERNEL); devm_?! Looking at r_size it should be rather kcalloc(). [3] [v3, Nicolas Saenz Julienne] > I agree with you. The reason I needed a "struct device *" in the call is > because I wanted to make sure the memory that is alloc'd belongs to the > device that needs it. If I do a regular kzalloc(), this memory will become > a leak once someone starts unbinding/binding their device. Also, in all > uses of of_dma_rtange() -- there is only one -- a dev is required as one > can't attach an offset map to NULL. > > I do see that there are a number of functions in drivers/of/*.c that > take 'struct device *dev' as an argument so there is precedent for > something like this. Regardless, I need an owner to the memory I > alloc(). I understand the need for dev to be around, devm_*() is key. But also it's important to keep the functions on purpose. And if of_dma_get_range() starts setting ranges it calls, for the very least, for a function rename. Although I'd rather split the parsing and setting of ranges as mentioned earlier. That said, I get that's a more drastic move. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset
Hi Christoph,
On Tue, Jul 28, 2020 at 8:33 AM Christoph Hellwig wrote:
>
> A few tiny nitpicks:
>
> The subject should have the dma-mapping prefix, this doesn't
> really touch the device core.
>
> > - rc = of_dma_get_range(np, _addr, , );
> > + rc = of_dma_get_range(np, );
> > + rc = PTR_ERR_OR_ZERO(map);
>
> I don't think you need the PTR_ERR_OR_ZERO line here, of_dma_get_range
> returns the error.
Yes, that link needs to be deleted.
>
> > +int dma_set_offset_range(struct device *dev, phys_addr_t cpu_start,
> > + dma_addr_t dma_start, u64 size)hh
> > +{
> > + struct bus_dma_region *map;
> > + u64 offset = (u64)cpu_start - (u64)dma_start;
> > +
> > + if (!dev)
> > + return -ENODEV;
>
> I don't think we need the NULL protection here, all DMA API calls
> expect a device.
Yes, your review-patch removed it but left the comment about the
function returning -ENODEV. So I wasn't sure to leave it in or not.
>
> > + if (!offset)
> > + return 0;
> > +
> > + /*
> > + * See if a map already exists and we already encompass the new range:
> > + */
> > + if (dev->dma_range_map) {
> > + if (dma_range_overlaps(dev, cpu_start, dma_start, size,
> > offset))
> > + return 0;
> > + dev_err(dev, "attempt to add conflicting DMA range to
> > existing map\n");
> > + return -EINVAL;
> > + }
>
> And here why do we need the overlap check at all? I'd be tempted to
> always return an error for this case.
I believe the overlap check was your suggestion or at least in your
review-patch? I'm fine with just returning an error.
>
> What is the plan to merge this? Do you want all this to go into one
> tree, or get as many bits into the applicable trees for 5.9 and then
> finish up for 5.10? If the former I can apply it to the dma-mapping
> tree and just fix up the nitpicks.
Whatever you think is best -- I would be quite happy if you could
accept at least the "dma_range_map" commit. Of course I'd be most
happy if the entire patchset were accepted, but perhaps you can just
apply the "dma_range_map" commit, and I will continue to bang away at
getting the N-1 PCIe-related commits ack'd and accepted.
Thanks much!
Jim Quinlan
Broadcom STB
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset
On Tue, Jul 28, 2020 at 11:05 AM Rob Herring wrote:
>
> On Fri, Jul 24, 2020 at 2:45 PM Jim Quinlan
> wrote:
> >
> > The new field 'dma_range_map' in struct device is used to facilitate the
> > use of single or multiple offsets between mapping regions of cpu addrs and
> > dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
> > capable of holding a single uniform offset and had no region bounds
> > checking.
> >
> > The function of_dma_get_range() has been modified so that it takes a single
> > argument -- the device node -- and returns a map, NULL, or an error code.
> > The map is an array that holds the information regarding the DMA regions.
> > Each range entry contains the address offset, the cpu_start address, the
> > dma_start address, and the size of the region.
> >
> > of_dma_configure() is the typical manner to set range 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_offset_range(dev, cpu_addr, dma_addr, size).
> >
> > Signed-off-by: Jim Quinlan
> > ---
>
> [...]
>
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index 8eea3f6e29a4..4b718d199efe 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -918,33 +918,33 @@ void __iomem *of_io_request_and_map(struct
> > device_node *np, int index,
> > }
> > EXPORT_SYMBOL(of_io_request_and_map);
> >
> > +#ifdef CONFIG_HAS_DMA
> > /**
> > - * of_dma_get_range - Get DMA range info
> > + * of_dma_get_range - Get DMA range info and put it into a map array
> > * @np:device node to get DMA range info
> > - * @dma_addr: pointer to store initial DMA address of DMA range
> > - * @paddr: pointer to store initial CPU address of DMA range
> > - * @size: pointer to store size of DMA range
> > + * @map: dma range structure to return
> > *
> > * Look in bottom up direction for the first "dma-ranges" property
> > - * and parse it.
> > - * dma-ranges format:
> > + * and parse it. Put the information into a DMA offset map array.
> > + *
> > + * dma-ranges format:
> > * DMA addr (dma_addr) : naddr cells
> > * CPU addr (phys_addr_t) : pna cells
> > * size: nsize cells
> > *
> > - * It returns -ENODEV if "dma-ranges" property was not found
> > - * for this device in DT.
> > + * It returns -ENODEV if "dma-ranges" property was not found for this
> > + * device in the DT.
> > */
> > -int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr,
> > u64 *size)
> > +int of_dma_get_range(struct device_node *np, const struct bus_dma_region
> > **map)
> > {
> > struct device_node *node = of_node_get(np);
> > const __be32 *ranges = NULL;
> > - int len;
> > - int ret = 0;
> > bool found_dma_ranges = false;
> > struct of_range_parser parser;
> > struct of_range range;
> > - u64 dma_start = U64_MAX, dma_end = 0, dma_offset = 0;
> > + struct bus_dma_region *r;
> > + int len, num_ranges = 0;
> > + int ret;
> >
> > while (node) {
> > ranges = of_get_property(node, "dma-ranges", );
> > @@ -970,44 +970,35 @@ int of_dma_get_range(struct device_node *np, u64
> > *dma_addr, u64 *paddr, u64 *siz
> > }
> >
> > of_dma_range_parser_init(, node);
> > + for_each_of_range(, )
> > + num_ranges++;
> > +
> > + of_dma_range_parser_init(, node);
> > +
> > + ret = -ENOMEM;
> > + r = kcalloc(num_ranges + 1, sizeof(*r), GFP_KERNEL);
> > + if (!r)
> > + goto out;
>
> AFAICT, you have the error cases covered, but you are leaking memory
> if the device is removed.
Hi Rob,
I started using devm_kcalloc() but at least two reviewers convinced me
to just use kcalloc(). In addition, when I was using devm_kcalloc()
it was awkward because 'dev' is not available to this function.
It comes down to whether unbind/binding the device N times is actually
a reasonable usage. As for my experience I've seen two cases: (1) my
overnight "bind/unbind the PCIe RC driver" script, and we have a
customer who does an unbind/bind as a hail mary to bring back life to
their dead EP device. If the latter case happens repeatedly, there
are bigger problems.
>
>
> [...]
>b
> > diff --git a/drivers/remoteproc/remoteproc_core.c
> > b/drivers/remoteproc/remoteproc_core.c
> > index 9f04c30c4aaf..49242dd6176e 100644
> > --- a/drivers/remoteproc/remoteproc_core.c
> > +++ b/drivers/remoteproc/remoteproc_core.c
> > @@ -519,7 +519,7 @@ static int rproc_handle_vdev(struct rproc *rproc,
> > struct fw_rsc_vdev *rsc,
> > /* Initialise vdev subdevice */
> > snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
> > rvdev->dev.parent = >dev;
> > - rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
> > + rvdev->dev.dma_range_map =
[PATCH v9 00/12] PCI: brcmstb: enable PCIe for STB chips
Patchset Summary:
Enhance a PCIe host controller driver. Because of its unusual design
we are foced to change dev->dma_pfn_offset into a more general role
allowing multiple offsets. See the 'v1' notes below for more info.
NOTE: ChristophH wanted the dma_set_offset_range() function
to have a range from [0...~(phys_addr_t)0], i.e. no specific
bounds. RobinM requested this function to have specific bounds,
which has been implemented since v6. If I do not hear from
Robin in the near future about this request, I will submit
v10 which will have no specific bounds.
v9:
Commit: "device core: Introduce DMA range map, supplanting ..."
-- A number of code improvements were implemented as suggested by
ChristophH. Unfortunately, some of these changes reversed the
implemented suggestions of other reviewers; for example, the new
macros PFN_DMA_ADDR(), DMA_ADDR_PFN() have been pulled.
v8:
Commit: "device core: Introduce DMA range map, supplanting ..."
-- To satisfy a specific m68 compile configuration, I moved the 'struct
bus_dma_region; definition out of #ifdef CONFIG_HAS_DMA and also defined
three inline functions for !CONFIG_HAS_DMA (kernel test robot).
-- The sunXi drivers -- suc4i_csi, sun6i_csi, cedrus_hw -- set
a pfn_offset outside of_dma_configure() but the code offers no
insight on the size of the translation window. V7 had me using
SIZE_MAX as the size. I have since contacted the sunXi maintainer and
he said that using a size of SZ_4G would cover sunXi configurations.
v7:
Commit: "device core: Introduce DMA range map, supplanting ..."
-- remove second kcalloc/copy in device.c (AndyS)
-- use PTR_ERR_OR_ZERO() and PHYS_PFN() (AndyS)
-- indentation, sizeof(struct ...) => sizeof(*r) (AndyS)
-- add pfn.h definitions: PFN_DMA_ADDR(), DMA_ADDR_PFN() (AndyS)
-- Fixed compile error in "sun6i_csi.c" (kernel test robot)
Commit "ata: ahci_brcm: Fix use of BCM7216 reset controller"
-- correct name of function in the commit msg (SergeiS)
v6:
Commit "device core: Introduce DMA range map":
-- of_dma_get_range() now takes a single argument and returns either
NULL, a valid map, or an ERR_PTR. (Robin)
-- offsets are no longer a PFN value but an actual address. (Robin)
-- the bus_dma_region struct stores the range size instead of
the cpu_end and pci_end values. (Robin)
-- devices that were setting a single offset with no boundaries
have been modified to have boundaries; in a few places
where this information was unavilable a /* FIXME: ... */
comment was added. (Robin)
-- dma_attach_offset_range() can be called when an offset
map already exists; if it's range is already present
nothing is done and success is returned. (Robin)
All commits:
-- Man name/style/corrections/etc changed (Bjorn)
-- rebase to Torvalds master
v5:
Commit "device core: Introduce multiple dma pfn offsets"
-- in of/address.c: "map_size = 0" => "*map_size = 0"
-- use kcalloc instead of kzalloc (AndyS)
-- use PHYS_ADDR_MAX instead of "~(phys_addr_t)0"
Commit "PCI: brcmstb: Set internal memory viewport sizes"
-- now gives error on missing dma-ranges property.
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- removed "Allof:" from brcm,scb-sizes definition (RobH)
All Commits:
-- indentation style, use max chars 100 (AndyS)
-- rebased to torvalds master
v4:
Commit "device core: Introduce multiple dma pfn offsets"
-- of_dma_get_range() does not take a dev param but instead
takes two "out" params: map and map_size. We do this so
that the code that parses dma-ranges is separate from
the code that modifies 'dev'. (Nicolas)
-- the separate case of having a single pfn offset has
been removed and is now processed by going through the
map array. (Nicolas)
-- move attach_uniform_dma_pfn_offset() from of/address.c to
dma/mapping.c so that it does not depend on CONFIG_OF. (Nicolas)
-- devm_kcalloc => devm_kzalloc (DanC)
-- add/fix assignment to dev->dma_pfn_offset_map for func
attach_uniform_dma_pfn_offset() (DanC, Nicolas)
-- s/struct dma_pfn_offset_region/struct bus_dma_region/ (Nicolas)
-- s/attach_uniform_dma_pfn_offset/dma_attach_uniform_pfn_offset/
-- s/attach_dma_pfn_offset_map/dma_attach_pfn_offset_map/
-- More use of PFN_{PHYS,DOWN,UP}. (AndyS)
Commit "of: Include a dev param in of_dma_get_range()"
-- this commit was sqaushed with "device core: Introduce ..."
v3:
Commit "device core: Introduce multiple dma pfn offsets"
Commit "arm: dma-mapping: Invoke dma offset func if needed"
-- The above two commits have been squashed. More importantly,
the code has been modified so that the functionality for
multiple pfn offsets subsumes the use of dev->dma_pfn_offset.
In fact, dma_pfn_offset is removed and supplanted by
dma_pfn_offset_map, which is a pointer to an array. The
[PATCH v9 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset
The new field 'dma_range_map' in struct device is used to facilitate the
use of single or multiple offsets between mapping regions of cpu addrs and
dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
capable of holding a single uniform offset and had no region bounds
checking.
The function of_dma_get_range() has been modified so that it takes a single
argument -- the device node -- and returns a map, NULL, or an error code.
The map is an array that holds the information regarding the DMA regions.
Each range entry contains the address offset, the cpu_start address, the
dma_start address, and the size of the region.
of_dma_configure() is the typical manner to set range 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_offset_range(dev, cpu_addr, dma_addr, size).
Signed-off-by: Jim Quinlan
---
arch/arm/include/asm/dma-mapping.h| 10 +--
arch/arm/mach-keystone/keystone.c | 17 +++--
arch/sh/drivers/pci/pcie-sh7786.c | 9 +--
arch/sh/kernel/dma-coherent.c | 15 ++--
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 | 71 ---
drivers/of/device.c | 43 ++-
drivers/of/of_private.h | 10 +--
drivers/of/unittest.c | 32 ++---
drivers/pci/controller/pcie-brcmstb.c | 1 +
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| 8 +--
include/linux/dma-mapping.h | 34 +
kernel/dma/coherent.c | 10 +--
kernel/dma/mapping.c | 63
24 files changed, 248 insertions(+), 122 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index bdd80ddbca34..2405afeb7957 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -35,8 +35,11 @@ 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) {
+ phys_addr_t paddr = PFN_PHYS(pfn);
+
+ pfn -= (dma_offset_from_phys_addr(dev, paddr) >> PAGE_SHIFT);
+ }
return (dma_addr_t)__pfn_to_bus(pfn);
}
@@ -45,8 +48,7 @@ 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;
-
+ pfn += (dma_offset_from_dma_addr(dev, addr) >> PAGE_SHIFT);
return pfn;
}
diff --git a/arch/arm/mach-keystone/keystone.c
b/arch/arm/mach-keystone/keystone.c
index 638808c4e122..78808942ad1c 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
@@ -24,8 +25,6 @@
#include "keystone.h"
-static unsigned long keystone_dma_pfn_offset __read_mostly;
-
static int keystone_platform_notifier(struct notifier_block *nb,
unsigned long event, void *data)
{
@@ -38,9 +37,12 @@ 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_set_offset_range(dev, KEYSTONE_HIGH_PHYS_START,
+ KEYSTONE_LOW_PHYS_START,
+ KEYSTONE_HIGH_PHYS_SIZE);
+ dev_err(dev, "set dma_offset%08llx%s\n",
+ KEYSTONE_HIGH_PHYS_START - KEYSTONE_LOW_PHYS_START,
+ ret ? " failed" : "");
}
return NOTIFY_OK;
}
@@ -51,11 +53,8 @@ static struct notifier_block platform_nb = {
static void __init keystone_init(void)
{
- if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START) {
- keystone_dma_pfn_offset = PFN_DOWN(KEYSTONE_HIGH_PHYS_START -
- KEYSTONE_LOW_PHYS_START);
+ if
Re: [PATCH v8 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset
On Tue, Jul 21, 2020 at 8:51 AM Christoph Hellwig wrote:
>
> On Wed, Jul 15, 2020 at 10:35:11AM -0400, Jim Quinlan wrote:
> > The new field 'dma_range_map' in struct device is used to facilitate the
> > use of single or multiple offsets between mapping regions of cpu addrs and
> > dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
> > capable of holding a single uniform offset and had no region bounds
> > checking.
> >
> > The function of_dma_get_range() has been modified so that it takes a single
> > argument -- the device node -- and returns a map, NULL, or an error code.
> > The map is an array that holds the information regarding the DMA regions.
> > Each range entry contains the address offset, the cpu_start address, the
> > dma_start address, and the size of the region.
> >
> > of_dma_configure() is the typical manner to set range 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_offset_range(dev, cpu_addr, dma_addr, size).
>
> So my main higher level issue here is the dma_attach_offset_range
> function. I think it should keep the old functionality and just
> set a global range from 0 to (phys_addr_t)-1, and bail out if there
> are DMA ranges already:
>
> int dma_set_global_offset(struct device *dev, u64 offset);
Hi Christoph,
I had it this way in [V1...V5] but Robin requested that for V6 I
should change this function to
o add bounds to the call
o if there is a mapping already, check if what is requested is
already covered and return success.
Can you and Robin please discuss this and let me know which way to move forward?
>
>
> otherwise there is all kinds of minor nitpicks that aren't too
> substantial, let me know what you think of something like this
> hacked up version:
Kind of hard to see what you have changed but I will diff both of our
diffs and make the changes.
Thanks,
Jim Quinlan
Broadcom STB
>
>
> diff --git a/arch/arm/include/asm/dma-mapping.h
> b/arch/arm/include/asm/dma-mapping.h
> index bdd80ddbca3451..2405afeb79573a 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -35,8 +35,11 @@ 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) {
> + phys_addr_t paddr = PFN_PHYS(pfn);
> +
> + pfn -= (dma_offset_from_phys_addr(dev, paddr) >> PAGE_SHIFT);
> + }
> return (dma_addr_t)__pfn_to_bus(pfn);
> }
>
> @@ -45,8 +48,7 @@ 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;
> -
> + pfn += (dma_offset_from_dma_addr(dev, addr) >> PAGE_SHIFT);
> return pfn;
> }
>
> diff --git a/arch/arm/mach-keystone/keystone.c
> b/arch/arm/mach-keystone/keystone.c
> index 638808c4e12247..7539679205fbf7 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
> @@ -24,8 +25,6 @@
>
> #include "keystone.h"
>
> -static unsigned long keystone_dma_pfn_offset __read_mostly;
> -
> static int keystone_platform_notifier(struct notifier_block *nb,
> unsigned long event, void *data)
> {
> @@ -38,9 +37,12 @@ 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_set_offset_range(dev, KEYSTONE_HIGH_PHYS_START,
> + KEYSTONE_LOW_PHYS_START,
> + KEYSTONE_HIGH_PHYS_SIZE);
> + dev_err(dev, "set dma_offset%08llx%s\n",
> + KEYSTONE_HIGH_PHYS_START - KEYSTONE_LOW_PHYS_START,
> + ret ? " failed" : "");
> }
> return NOTIFY_OK;
> }
> @@ -51,11 +53,8 @@ static struct notifier_block platform_nb = {
>
> static void __init keystone_init(void)
> {
> - if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START) {
> - keystone_dma_pfn_offset = PFN_DOWN(KEYSTONE_HIGH_PHYS_START -
> - KEYSTONE_LOW_PHYS_START);
> + if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START)
> bus_register_notifier(_bus_type, _nb);
> - }
> keystone_pm_runtime_init();
> }
>
> diff --git a/arch/sh/drivers/pci/pcie-sh7786.c
>
[PATCH v8 00/12] PCI: brcmstb: enable PCIe for STB chips
Patchset Summary:
Enhance a PCIe host controller driver. Because of its unusual design
we are foced to change dev->dma_pfn_offset into a more general role
allowing multiple offsets. See the 'v1' notes below for more info.
v8:
Commit: "device core: Introduce DMA range map, supplanting ..."
-- To satisfy a specific m68 compile configuration, I moved the 'struct
bus_dma_region; definition out of #ifdef CONFIG_HAS_DMA and also defined
three inline functions for !CONFIG_HAS_DMA (kernel test robot).
-- The sunXi drivers -- suc4i_csi, sun6i_csi, cedrus_hw -- set
a pfn_offset outside of_dma_configure() but the code offers no
insight on the size of the translation window. V7 had me using
SIZE_MAX as the size. I have since contacted the sunXi maintainer and
he said that using a size of SZ_4G would cover sunXi configurations.
v7:
Commit: "device core: Introduce DMA range map, supplanting ..."
-- remove second kcalloc/copy in device.c (AndyS)
-- use PTR_ERR_OR_ZERO() and PHYS_PFN() (AndyS)
-- indentation, sizeof(struct ...) => sizeof(*r) (AndyS)
-- add pfn.h definitions: PFN_DMA_ADDR(), DMA_ADDR_PFN() (AndyS)
-- Fixed compile error in "sun6i_csi.c" (kernel test robot)
Commit "ata: ahci_brcm: Fix use of BCM7216 reset controller"
-- correct name of function in the commit msg (SergeiS)
v6:
Commit "device core: Introduce DMA range map":
-- of_dma_get_range() now takes a single argument and returns either
NULL, a valid map, or an ERR_PTR. (Robin)
-- offsets are no longer a PFN value but an actual address. (Robin)
-- the bus_dma_region struct stores the range size instead of
the cpu_end and pci_end values. (Robin)
-- devices that were setting a single offset with no boundaries
have been modified to have boundaries; in a few places
where this informatino was unavilable a /* FIXME: ... */
comment was added. (Robin)
-- dma_attach_offset_range() can be called when an offset
map already exists; if it's range is already present
nothing is done and success is returned. (Robin)
All commits:
-- Man name/style/corrections/etc changed (Bjorn)
-- rebase to Torvalds master
v5:
Commit "device core: Introduce multiple dma pfn offsets"
-- in of/address.c: "map_size = 0" => "*map_size = 0"
-- use kcalloc instead of kzalloc (AndyS)
-- use PHYS_ADDR_MAX instead of "~(phys_addr_t)0"
Commit "PCI: brcmstb: Set internal memory viewport sizes"
-- now gives error on missing dma-ranges property.
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- removed "Allof:" from brcm,scb-sizes definition (RobH)
All Commits:
-- indentation style, use max chars 100 (AndyS)
-- rebased to torvalds master
v4:
Commit "device core: Introduce multiple dma pfn offsets"
-- of_dma_get_range() does not take a dev param but instead
takes two "out" params: map and map_size. We do this so
that the code that parses dma-ranges is separate from
the code that modifies 'dev'. (Nicolas)
-- the separate case of having a single pfn offset has
been removed and is now processed by going through the
map array. (Nicolas)
-- move attach_uniform_dma_pfn_offset() from of/address.c to
dma/mapping.c so that it does not depend on CONFIG_OF. (Nicolas)
-- devm_kcalloc => devm_kzalloc (DanC)
-- add/fix assignment to dev->dma_pfn_offset_map for func
attach_uniform_dma_pfn_offset() (DanC, Nicolas)
-- s/struct dma_pfn_offset_region/struct bus_dma_region/ (Nicolas)
-- s/attach_uniform_dma_pfn_offset/dma_attach_uniform_pfn_offset/
-- s/attach_dma_pfn_offset_map/dma_attach_pfn_offset_map/
-- More use of PFN_{PHYS,DOWN,UP}. (AndyS)
Commit "of: Include a dev param in of_dma_get_range()"
-- this commit was sqaushed with "device core: Introduce ..."
v3:
Commit "device core: Introduce multiple dma pfn offsets"
Commit "arm: dma-mapping: Invoke dma offset func if needed"
-- The above two commits have been squashed. More importantly,
the code has been modified so that the functionality for
multiple pfn offsets subsumes the use of dev->dma_pfn_offset.
In fact, dma_pfn_offset is removed and supplanted by
dma_pfn_offset_map, which is a pointer to an array. The
more common case of a uniform offset is now handled as
a map with a single entry, while cases requiring multiple
pfn offsets use a map with multiple entries. Code paths
that used to do this:
dev->dma_pfn_offset = mydrivers_pfn_offset;
have been changed to do this:
attach_uniform_dma_pfn_offset(dev, pfn_offset);
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- Add if/then clause for required props: resets, reset-names (RobH)
-- Change compatible list from const to enum (RobH)
-- Change list of u32-tuples to u64 (RobH)
Commit "of: Include a dev param in of_dma_get_range()"
-- modify of/unittests.c to add NULL param in
[PATCH v8 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset
The new field 'dma_range_map' in struct device is used to facilitate the
use of single or multiple offsets between mapping regions of cpu addrs and
dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
capable of holding a single uniform offset and had no region bounds
checking.
The function of_dma_get_range() has been modified so that it takes a single
argument -- the device node -- and returns a map, NULL, or an error code.
The map is an array that holds the information regarding the DMA regions.
Each range entry contains the address offset, the cpu_start address, the
dma_start address, and the size of the region.
of_dma_configure() is the typical manner to set range 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_offset_range(dev, cpu_addr, dma_addr, size).
Signed-off-by: Jim Quinlan
---
arch/arm/include/asm/dma-mapping.h| 9 +-
arch/arm/mach-keystone/keystone.c | 17 ++--
arch/sh/drivers/pci/pcie-sh7786.c | 9 +-
arch/sh/kernel/dma-coherent.c | 16 ++--
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 | 95 ++-
drivers/of/device.c | 47 +
drivers/of/of_private.h | 9 +-
drivers/of/unittest.c | 35 +--
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| 10 +-
include/linux/dma-mapping.h | 43 +
include/linux/pfn.h | 2 +
kernel/dma/coherent.c | 10 +-
kernel/dma/mapping.c | 53 +++
24 files changed, 278 insertions(+), 124 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index bdd80ddbca34..b7cdde9fb83d 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_range_map)
+ pfn -= DMA_ADDR_PFN(dma_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_range_map)
+ pfn += DMA_ADDR_PFN(dma_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..a1a19781983b 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
@@ -24,8 +25,6 @@
#include "keystone.h"
-static unsigned long keystone_dma_pfn_offset __read_mostly;
-
static int keystone_platform_notifier(struct notifier_block *nb,
unsigned long event, void *data)
{
@@ -38,9 +37,12 @@ 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_offset_range(dev, KEYSTONE_HIGH_PHYS_START,
+ KEYSTONE_LOW_PHYS_START,
+ KEYSTONE_HIGH_PHYS_SIZE);
+ dev_err(dev, "set dma_offset%08llx%s\n",
+ KEYSTONE_HIGH_PHYS_START - KEYSTONE_LOW_PHYS_START,
+ ret ? " failed" : "");
}
return NOTIFY_OK;
}
@@ -51,11 +53,8 @@ static struct notifier_block platform_nb = {
static void __init keystone_init(void)
{
- if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START) {
- keystone_dma_pfn_offset = PFN_DOWN(KEYSTONE_HIGH_PHYS_START -
- KEYSTONE_LOW_PHYS_START);
+ if
Re: [PATCH v7 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset
Hi Christoph, I'm sending all commits to since most of them are PCI related. I don't send all patches to linux-ker...@vger.kernel.org since I've read it is overused. The --cc list is generated by get_maintainer.pl. IIRC, in a previous discussion you said you preferred NOT to get the entire patchset by "--to Christoph" but instead you would pick it off from one of the "-to " kernel list sites. It appears that all commits made it to the PCI list (https://www.spinics.net/lists/linux-pci/). Let me know what you want and I'll make it so. Regards, Jim On Thu, Jul 9, 2020 at 6:31 AM Christoph Hellwig wrote: > > I someone seem to get just this patch instead of the full series for > review again.. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v7 00/12] PCI: brcmstb: enable PCIe for STB chips
Patchset Summary:
Enhance a PCIe host controller driver. Because of its unusual design
we are foced to change dev->dma_pfn_offset into a more general role
allowing multiple offsets. See the 'v1' notes below for more info.
v7:
Commit: "device core: Introduce DMA range map, supplanting ..."
-- remove second kcalloc/copy in device.c (AndyS)
-- use PTR_ERR_OR_ZERO() and PHYS_PFN() (AndyS)
-- indentation, sizeof(struct ...) => sizeof(*r) (AndyS)
-- add pfn.h definitions: PFN_DMA_ADDR(), DMA_ADDR_PFN() (AndyS)
-- Fixed compile error in "sun6i_csi.c" (kernel test robot)
Commit "ata: ahci_brcm: Fix use of BCM7216 reset controller"
-- correct name of function in the commit msg (SergeiS)
v6:
Commit "device core: Introduce DMA range map":
-- of_dma_get_range() now takes a single argument and returns either
NULL, a valid map, or an ERR_PTR. (Robin)
-- offsets are no longer a PFN value but an actual address. (Robin)
-- the bus_dma_region struct stores the range size instead of
the cpu_end and pci_end values. (Robin)
-- devices that were setting a single offset with no boundaries
have been modified to have boundaries; in a few places
where this informatino was unavilable a /* FIXME: ... */
comment was added. (Robin)
-- dma_attach_offset_range() can be called when an offset
map already exists; if it's range is already present
nothing is done and success is returned. (Robin)
All commits:
-- Man name/style/corrections/etc changed (Bjorn)
-- rebase to Torvalds master
v5:
Commit "device core: Introduce multiple dma pfn offsets"
-- in of/address.c: "map_size = 0" => "*map_size = 0"
-- use kcalloc instead of kzalloc (AndyS)
-- use PHYS_ADDR_MAX instead of "~(phys_addr_t)0"
Commit "PCI: brcmstb: Set internal memory viewport sizes"
-- now gives error on missing dma-ranges property.
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- removed "Allof:" from brcm,scb-sizes definition (RobH)
All Commits:
-- indentation style, use max chars 100 (AndyS)
-- rebased to torvalds master
v4:
Commit "device core: Introduce multiple dma pfn offsets"
-- of_dma_get_range() does not take a dev param but instead
takes two "out" params: map and map_size. We do this so
that the code that parses dma-ranges is separate from
the code that modifies 'dev'. (Nicolas)
-- the separate case of having a single pfn offset has
been removed and is now processed by going through the
map array. (Nicolas)
-- move attach_uniform_dma_pfn_offset() from of/address.c to
dma/mapping.c so that it does not depend on CONFIG_OF. (Nicolas)
-- devm_kcalloc => devm_kzalloc (DanC)
-- add/fix assignment to dev->dma_pfn_offset_map for func
attach_uniform_dma_pfn_offset() (DanC, Nicolas)
-- s/struct dma_pfn_offset_region/struct bus_dma_region/ (Nicolas)
-- s/attach_uniform_dma_pfn_offset/dma_attach_uniform_pfn_offset/
-- s/attach_dma_pfn_offset_map/dma_attach_pfn_offset_map/
-- More use of PFN_{PHYS,DOWN,UP}. (AndyS)
Commit "of: Include a dev param in of_dma_get_range()"
-- this commit was sqaushed with "device core: Introduce ..."
v3:
Commit "device core: Introduce multiple dma pfn offsets"
Commit "arm: dma-mapping: Invoke dma offset func if needed"
-- The above two commits have been squashed. More importantly,
the code has been modified so that the functionality for
multiple pfn offsets subsumes the use of dev->dma_pfn_offset.
In fact, dma_pfn_offset is removed and supplanted by
dma_pfn_offset_map, which is a pointer to an array. The
more common case of a uniform offset is now handled as
a map with a single entry, while cases requiring multiple
pfn offsets use a map with multiple entries. Code paths
that used to do this:
dev->dma_pfn_offset = mydrivers_pfn_offset;
have been changed to do this:
attach_uniform_dma_pfn_offset(dev, pfn_offset);
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- Add if/then clause for required props: resets, reset-names (RobH)
-- Change compatible list from const to enum (RobH)
-- Change list of u32-tuples to u64 (RobH)
Commit "of: Include a dev param in of_dma_get_range()"
-- modify of/unittests.c to add NULL param in of_dma_get_range() call.
Commit "device core: Add ability to handle multiple dma offsets"
-- align comment in device.h (AndyS).
-- s/cpu_beg/cpu_start/ and s/dma_beg/dma_start/ in struct
dma_pfn_offset_region (AndyS).
v2:
Commit: "device core: Add ability to handle multiple dma offsets"
o Added helper func attach_dma_pfn_offset_map() in address.c (Chistoph)
o Helpers funcs added to __phys_to_dma() & __dma_to_phys() (Christoph)
o Added warning when multiple offsets are needed and !DMA_PFN_OFFSET_MAP
o dev->dma_pfn_map => dev->dma_pfn_offset_map
o s/frm/from/ for dma_pfn_offset_frm_{phys,dma}_addr() (Christoph)
o In device.h:
[PATCH v7 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset
The new field 'dma_range_map' in struct device is used to facilitate the
use of single or multiple offsets between mapping regions of cpu addrs and
dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
capable of holding a single uniform offset and had no region bounds
checking.
The function of_dma_get_range() has been modified so that it takes a single
argument -- the device node -- and returns a map, NULL, or an error code.
The map is an array that holds the information regarding the DMA regions.
Each range entry contains the address offset, the cpu_start address, the
dma_start address, and the size of the region.
of_dma_configure() is the typical manner to set range 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_offset_range(dev, cpu_addr, dma_addr, size).
Signed-off-by: Jim Quinlan
---
arch/arm/include/asm/dma-mapping.h| 9 +-
arch/arm/mach-keystone/keystone.c | 17 ++--
arch/sh/drivers/pci/pcie-sh7786.c | 9 +-
arch/sh/kernel/dma-coherent.c | 16 ++--
arch/x86/pci/sta2x11-fixup.c | 7 +-
drivers/acpi/arm64/iort.c | 5 +-
drivers/gpu/drm/sun4i/sun4i_backend.c | 7 +-
drivers/iommu/io-pgtable-arm.c| 2 +-
.../platform/sunxi/sun4i-csi/sun4i_csi.c | 6 +-
.../platform/sunxi/sun6i-csi/sun6i_csi.c | 5 +-
drivers/of/address.c | 95 ++-
drivers/of/device.c | 47 +
drivers/of/of_private.h | 9 +-
drivers/of/unittest.c | 35 +--
drivers/remoteproc/remoteproc_core.c | 2 +-
.../staging/media/sunxi/cedrus/cedrus_hw.c| 8 +-
drivers/usb/core/message.c| 4 +-
drivers/usb/core/usb.c| 2 +-
include/linux/device.h| 4 +-
include/linux/dma-direct.h| 10 +-
include/linux/dma-mapping.h | 37
include/linux/pfn.h | 2 +
kernel/dma/coherent.c | 10 +-
kernel/dma/mapping.c | 53 +++
24 files changed, 277 insertions(+), 124 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index bdd80ddbca34..b7cdde9fb83d 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_range_map)
+ pfn -= DMA_ADDR_PFN(dma_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_range_map)
+ pfn += DMA_ADDR_PFN(dma_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..a1a19781983b 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
@@ -24,8 +25,6 @@
#include "keystone.h"
-static unsigned long keystone_dma_pfn_offset __read_mostly;
-
static int keystone_platform_notifier(struct notifier_block *nb,
unsigned long event, void *data)
{
@@ -38,9 +37,12 @@ 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_offset_range(dev, KEYSTONE_HIGH_PHYS_START,
+ KEYSTONE_LOW_PHYS_START,
+ KEYSTONE_HIGH_PHYS_SIZE);
+ dev_err(dev, "set dma_offset%08llx%s\n",
+ KEYSTONE_HIGH_PHYS_START - KEYSTONE_LOW_PHYS_START,
+ ret ? " failed" : "");
}
return NOTIFY_OK;
}
@@ -51,11 +53,8 @@ static struct notifier_block platform_nb = {
static void __init keystone_init(void)
{
- if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START) {
- keystone_dma_pfn_offset = PFN_DOWN(KEYSTONE_HIGH_PHYS_START -
- KEYSTONE_LOW_PHYS_START);
+ if
Re: [PATCH v6 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset
Hi Andy, Sorry for the delay in response. I will do what you suggest in your email. I do have one response to one of your comments below. On Thu, Jul 2, 2020 at 4:43 AM Andy Shevchenko wrote: > > On Wed, Jul 01, 2020 at 05:21:38PM -0400, Jim Quinlan wrote: > > The new field 'dma_range_map' in struct device is used to facilitate the > > use of single or multiple offsets between mapping regions of cpu addrs and > > dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only > > capable of holding a single uniform offset and had no region bounds > > checking. > > > > The function of_dma_get_range() has been modified so that it takes a single > > argument -- the device node -- and returns a map, NULL, or an error code. > > The map is an array that holds the information regarding the DMA regions. > > Each range entry contains the address offset, the cpu_start address, the > > dma_start address, and the size of the region. > > > > of_dma_configure() is the typical manner to set range 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_offset_range(dev, cpu_addr, dma_addr, size). > > ... > > > + if (dev && dev->dma_range_map) > > + pfn -= (unsigned long)PFN_DOWN(dma_offset_from_phys_addr(dev, > > PFN_PHYS(pfn))); > > Instead of casting use PHYS_PFN() and it will be consistent with latter in > the same line. > > > + if (dev && dev->dma_range_map) > > + pfn += (unsigned long)PFN_DOWN(dma_offset_from_dma_addr(dev, > > addr)); > > Ditto. > > ... > > > + dev_err(dev, "set dma_offset%08llx%s\n", > > KEYSTONE_HIGH_PHYS_START > > + - KEYSTONE_LOW_PHYS_START, ret ? " failed" : ""); > > Please, avoid such indentation. > Better split it to the three lines, argument per line (expect dev which will > go > on the first one). > > This applies to all similar places. > > ... > > > unsigned long pfn = (dma_handle >> PAGE_SHIFT); > > PHYS_PFN() / PFN_DOWN() ? > > > + if (!WARN_ON(!dev) && dev->dma_range_map) > > + pfn += (unsigned long)PFN_DOWN(dma_offset_from_dma_addr(dev, > > dma_handle)); > > PHYS_PFN() ? > > ... > > > + r = kcalloc(num_ranges + 1, sizeof(struct bus_dma_region), > > GFP_KERNEL); > > sizeof(*r) ? > > > + if (!r) > > + return ERR_PTR(-ENOMEM); > > ... > > > + ret = IS_ERR(map) ? PTR_ERR(map) : 0; > > PTR_ERR_OR_ZERO() > > ... > > > + /* We want the offset map to be device-managed, so alloc & > > copy */ > > + dev->dma_range_map = devm_kcalloc(dev, num_ranges + 1, > > sizeof(*r), > > + GFP_KERNEL); > > The question is how many times per device lifetime this can be called? Someone else questioned this. There are two cases that come to mind: our overnight test which load/unloads the RC driver over and over, and a customer that does an unbind/bind of the RC driver when their EP is hung and wants to restart. Both cases are atypical and are weak arguments to justify the second copy. I will drop the copy. Thanks, Jim Quinlan Broadcom STB > > ... > > > > + if (!dev->dma_range_map) > > + return -ENOMEM; > > + memcpy((void *)dev->dma_range_map, map, sizeof(*r) * > > num_ranges + 1); > > If it's continuous, perhaps kmemdup() ? > > ... > > > + rc = IS_ERR(map) ? PTR_ERR(map) : 0; > > PTR_ERR_OR_ZERO() > > ... > > > + = dma_offset_from_phys_addr(dev, > > PFN_PHYS(mem->pfn_base)); > > + > > + return (dma_addr_t)PFN_PHYS(mem->pfn_base) - dma_offset; > > Looking at this more, I think you need to introduce in the same header (pfn.h) > something like: > > #define PFN_DMA_ADDR() > #define DMA_ADDR_PFN() > > -- > With Best Regards, > Andy Shevchenko > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v6 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset
The new field 'dma_range_map' in struct device is used to facilitate the
use of single or multiple offsets between mapping regions of cpu addrs and
dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
capable of holding a single uniform offset and had no region bounds
checking.
The function of_dma_get_range() has been modified so that it takes a single
argument -- the device node -- and returns a map, NULL, or an error code.
The map is an array that holds the information regarding the DMA regions.
Each range entry contains the address offset, the cpu_start address, the
dma_start address, and the size of the region.
of_dma_configure() is the typical manner to set range 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_offset_range(dev, cpu_addr, dma_addr, size).
Signed-off-by: Jim Quinlan
---
arch/arm/include/asm/dma-mapping.h| 9 +-
arch/arm/mach-keystone/keystone.c | 17 ++--
arch/sh/drivers/pci/pcie-sh7786.c | 9 +-
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 | 7 +-
drivers/iommu/io-pgtable-arm.c| 2 +-
.../platform/sunxi/sun4i-csi/sun4i_csi.c | 6 +-
.../platform/sunxi/sun6i-csi/sun6i_csi.c | 5 +-
drivers/of/address.c | 95 ++-
drivers/of/device.c | 50 ++
drivers/of/of_private.h | 9 +-
drivers/of/unittest.c | 35 +--
drivers/remoteproc/remoteproc_core.c | 2 +-
.../staging/media/sunxi/cedrus/cedrus_hw.c| 8 +-
drivers/usb/core/message.c| 4 +-
drivers/usb/core/usb.c| 2 +-
include/linux/device.h| 4 +-
include/linux/dma-direct.h| 10 +-
include/linux/dma-mapping.h | 37
kernel/dma/coherent.c | 11 ++-
kernel/dma/mapping.c | 53 +++
23 files changed, 279 insertions(+), 122 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index bdd80ddbca34..b6796383f205 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_range_map)
+ pfn -= (unsigned long)PFN_DOWN(dma_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_range_map)
+ pfn += (unsigned long)PFN_DOWN(dma_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..a499ece6f054 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
@@ -24,8 +25,6 @@
#include "keystone.h"
-static unsigned long keystone_dma_pfn_offset __read_mostly;
-
static int keystone_platform_notifier(struct notifier_block *nb,
unsigned long event, void *data)
{
@@ -38,9 +37,12 @@ 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_offset_range(dev, KEYSTONE_HIGH_PHYS_START,
+ KEYSTONE_LOW_PHYS_START,
+ KEYSTONE_HIGH_PHYS_SIZE);
+ dev_err(dev, "set dma_offset%08llx%s\n",
KEYSTONE_HIGH_PHYS_START
+ - KEYSTONE_LOW_PHYS_START, ret ? " failed" : "");
+
}
return NOTIFY_OK;
}
@@ -51,11 +53,8 @@ static struct notifier_block platform_nb = {
static void __init keystone_init(void)
{
- if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START) {
- keystone_dma_pfn_offset = PFN_DOWN(KEYSTONE_HIGH_PHYS_START -
- KEYSTONE_LOW_PHYS_START);
+ if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START)
[PATCH v6 00/12] PCI: brcmstb: enable PCIe for STB chips
Patchset Summary:
Enhance a PCIe host controller driver. Because of its unusual design
we are foced to change dev->dma_pfn_offset into a more general role
allowing multiple offsets. See the 'v1' notes below for more info.
v6:
Commit "device core: Introduce DMA range map":
-- of_dma_get_range() now takes a single argument and returns either
NULL, a valid map, or an ERR_PTR. (Robin)
-- offsets are no longer a PFN value but an actual address. (Robin)
-- the bus_dma_region struct stores the range size instead of
the cpu_end and pci_end values. (Robin)
-- devices that were setting a single offset with no boundaries
have been modified to have boundaries; in a few places
where this informatino was unavilable a /* FIXME: ... */
comment was added. (Robin)
-- dma_attach_offset_range() can be called when an offset
map already exists; if it's range is already present
nothing is done and success is returned. (Robin)
All commits:
-- Man name/style/corrections/etc changed (Bjorn)
-- rebase to Torvalds master
v5:
Commit "device core: Introduce multiple dma pfn offsets"
-- in of/address.c: "map_size = 0" => "*map_size = 0"
-- use kcalloc instead of kzalloc (AndyS)
-- use PHYS_ADDR_MAX instead of "~(phys_addr_t)0"
Commit "PCI: brcmstb: Set internal memory viewport sizes"
-- now gives error on missing dma-ranges property.
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- removed "Allof:" from brcm,scb-sizes definition (RobH)
All Commits:
-- indentation style, use max chars 100 (AndyS)
-- rebased to torvalds master
v4:
Commit "device core: Introduce multiple dma pfn offsets"
-- of_dma_get_range() does not take a dev param but instead
takes two "out" params: map and map_size. We do this so
that the code that parses dma-ranges is separate from
the code that modifies 'dev'. (Nicolas)
-- the separate case of having a single pfn offset has
been removed and is now processed by going through the
map array. (Nicolas)
-- move attach_uniform_dma_pfn_offset() from of/address.c to
dma/mapping.c so that it does not depend on CONFIG_OF. (Nicolas)
-- devm_kcalloc => devm_kzalloc (DanC)
-- add/fix assignment to dev->dma_pfn_offset_map for func
attach_uniform_dma_pfn_offset() (DanC, Nicolas)
-- s/struct dma_pfn_offset_region/struct bus_dma_region/ (Nicolas)
-- s/attach_uniform_dma_pfn_offset/dma_attach_uniform_pfn_offset/
-- s/attach_dma_pfn_offset_map/dma_attach_pfn_offset_map/
-- More use of PFN_{PHYS,DOWN,UP}. (AndyS)
Commit "of: Include a dev param in of_dma_get_range()"
-- this commit was sqaushed with "device core: Introduce ..."
v3:
Commit "device core: Introduce multiple dma pfn offsets"
Commit "arm: dma-mapping: Invoke dma offset func if needed"
-- The above two commits have been squashed. More importantly,
the code has been modified so that the functionality for
multiple pfn offsets subsumes the use of dev->dma_pfn_offset.
In fact, dma_pfn_offset is removed and supplanted by
dma_pfn_offset_map, which is a pointer to an array. The
more common case of a uniform offset is now handled as
a map with a single entry, while cases requiring multiple
pfn offsets use a map with multiple entries. Code paths
that used to do this:
dev->dma_pfn_offset = mydrivers_pfn_offset;
have been changed to do this:
attach_uniform_dma_pfn_offset(dev, pfn_offset);
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- Add if/then clause for required props: resets, reset-names (RobH)
-- Change compatible list from const to enum (RobH)
-- Change list of u32-tuples to u64 (RobH)
Commit "of: Include a dev param in of_dma_get_range()"
-- modify of/unittests.c to add NULL param in of_dma_get_range() call.
Commit "device core: Add ability to handle multiple dma offsets"
-- align comment in device.h (AndyS).
-- s/cpu_beg/cpu_start/ and s/dma_beg/dma_start/ in struct
dma_pfn_offset_region (AndyS).
v2:
Commit: "device core: Add ability to handle multiple dma offsets"
o Added helper func attach_dma_pfn_offset_map() in address.c (Chistoph)
o Helpers funcs added to __phys_to_dma() & __dma_to_phys() (Christoph)
o Added warning when multiple offsets are needed and !DMA_PFN_OFFSET_MAP
o dev->dma_pfn_map => dev->dma_pfn_offset_map
o s/frm/from/ for dma_pfn_offset_frm_{phys,dma}_addr() (Christoph)
o In device.h: s/const void */const struct dma_pfn_offset_region */
o removed 'unlikely' from unlikely(dev->dma_pfn_offset_map) since
guarded by CONFIG_DMA_PFN_OFFSET_MAP (Christoph)
o Since dev->dma_pfn_offset is copied in usb/core/{usb,message}.c, now
dev->dma_pfn_offset_map is copied as well.
o Merged two of the DMA commits into one (Christoph).
Commit "arm: dma-mapping: Invoke dma offset func if needed":
o Use helper functions instead of #if CONFIG_DMA_PFN_OFFSET
Other commits'
Re: [PATCH v5 08/12] device core: Introduce multiple dma pfn offsets
On Wed, Jun 17, 2020 at 9:00 AM Robin Murphy wrote:
>
> Hi Jim,
>
> Thanks for taking this on!
Hi Robin,
>
> On 2020-06-16 21:55, 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.
>
> This isn't just about offsets - it should (eventually) subsume
> bus_dma_limit as well, so I'd be inclined to call it something like
> "dma_ranges"/"dma_range_map"/"dma_regions"/etc.
I will change my wording here.
>
> > 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).
>
> I'm also not convinced that sticking to the PFN paradigm is necessarily
> the right way to go - when there's only a single nicely-aligned offset
> to consider then an unsigned long that's immune to PAE/LPAE/etc.
> disruption is indeed the cheapest and easiest option from core code's
> PoV. However it already means that all the users have to do some degree
> of conversion back and forth between PFNs and usable addresses; once the
> core code itself also has to start bouncing back and forth between
> addresses and PFNs internally then we end up effectively just doing work
> to cancel out other work, and the whole lot would end up simpler and
> more efficient if the API worked purely in terms of addresses.
If you want me to change the paradigm to an address offset, I will.
If so please specify its type: dma_addr_t, phys_addr_t, or u64?
>
> [...]
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index 8eea3f6e29a4..767fa3b492c8 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -918,12 +918,48 @@ void __iomem *of_io_request_and_map(struct
> > device_node *np, int index,
> > }
> > EXPORT_SYMBOL(of_io_request_and_map);
> >
> > +static int dma_attach_pfn_offset_map(struct device_node *node, int
> > num_ranges,
> > + struct bus_dma_region **map, size_t
> > *map_size)
> > +{
> > + struct of_range_parser parser;
> > + struct of_range range;
> > + struct bus_dma_region *r;
> > +
> > + *map_size = (num_ranges + 1) * sizeof(**map);
> > + r = kcalloc(num_ranges + 1, sizeof(**map), GFP_KERNEL);
> > + if (!r)
> > + return -ENOMEM;
> > + *map = r;
> > +
> > + of_dma_range_parser_init(, node);
> > + /*
> > + * Record all info for DMA ranges array. We could
> > + * just use the of_range struct, but if we did that it
>
> Not making the entire DMA API depend on OF is a far better justification
> for having its own dedicated structure.
>
> > + * would require more calculations for phys_to_dma and
> > + * dma_to_phys conversions.
> > + */
>
> However that part is pretty much nonsense. Consider your "efficient"
> operation for looking up and consuming a DMA offset:
>
> API caller
> 1. load cpu_start
> 2. compare addr >= cpu_start
> 3. load cpu_end
> 4. compare addr <= cpu_end
> 5. load pfn_offset
> 6. shift pfn_offset << PAGE_SHIFT
> 7. add "offset" + addr
> 8. [use the result]
>
> versus the "more calculations" approach (once the PFN cruft is peeled away):
>
> API caller
> 1. load cpu_addr
> 2. compare addr >= cpu_addr
> 3. subtract addr - cpu_addr
> 4. load size
> 5. compare "addr_offset" < size
> 6. load dma_start
> 7. add dma_start + "addr_offset"
> 8. [use the result]
>
> Oh look, it's the exact same number of memory accesses and ALU
> operations, but with a smaller code footprint (assuming, reasonably,
> more than one caller) and less storage overhead ;)
I feel you have conflated two independent issues to get your
comparison. Issue one is having two extra fields -- cpu_end, dma_end
-- and issue two is using address verses PFN offsets. If you look at
both code sequences when using an address offset verses PFN offset,
the of_range way requires an extra computation. Do you agree?
>
> Basically, having this degree of redundancy is somewhere between silly
> and actively harmful (what if pfn_offset gets out of sync with
> cpu_start/dma_start?
How can this happen if the range map is set once and passed as a const
structure?
> What if cpu_end/dma_end don't represent equivalent lengths?)
They are set once using the same +size operation so they must be
equal. They are not changed after they are
[PATCH v5 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 | 8 ++-
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 | 4 +-
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 | 71 ---
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| 14 +++-
include/linux/dma-mapping.h | 38 ++
kernel/dma/coherent.c | 11 +--
kernel/dma/mapping.c | 39 ++
23 files changed, 231 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..5890ec90599e 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,10 @@ 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;
@@ -34,11 +35,12 @@ void
[PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips
Patchset Summary:
Enhance a PCIe host controller driver. Because of its unusual design
we are foced to change dev->dma_pfn_offset into a more general role
allowing multiple offsets.
v5:
Commit "device core: Introduce multiple dma pfn offsets"
-- in of/address.c: "map_size = 0" => "*map_size = 0"
-- use kcalloc instead of kzalloc (AndyS)
-- use PHYS_ADDR_MAX instead of "~(phys_addr_t)0"
Commit "PCI: brcmstb: Set internal memory viewport sizes"
-- now gives error on missing dma-ranges property.
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- removed "Allof:" from brcm,scb-sizes definition (RobH)
All Commits:
-- indentation style, use max chars 100 (AndyS)
-- rebased to torvalds master
v4:
Commit "device core: Introduce multiple dma pfn offsets"
-- of_dma_get_range() does not take a dev param but instead
takes two "out" params: map and map_size. We do this so
that the code that parses dma-ranges is separate from
the code that modifies 'dev'. (Nicolas)
-- the separate case of having a single pfn offset has
been removed and is now processed by going through the
map array. (Nicolas)
-- move attach_uniform_dma_pfn_offset() from of/address.c to
dma/mapping.c so that it does not depend on CONFIG_OF. (Nicolas)
-- devm_kcalloc => devm_kzalloc (DanC)
-- add/fix assignment to dev->dma_pfn_offset_map for func
attach_uniform_dma_pfn_offset() (DanC, Nicolas)
-- s/struct dma_pfn_offset_region/struct bus_dma_region/ (Nicolas)
-- s/attach_uniform_dma_pfn_offset/dma_attach_uniform_pfn_offset/
-- s/attach_dma_pfn_offset_map/dma_attach_pfn_offset_map/
-- More use of PFN_{PHYS,DOWN,UP}. (AndyS)
Commit "of: Include a dev param in of_dma_get_range()"
-- this commit was sqaushed with "device core: Introduce ..."
v3:
Commit "device core: Introduce multiple dma pfn offsets"
Commit "arm: dma-mapping: Invoke dma offset func if needed"
-- The above two commits have been squashed. More importantly,
the code has been modified so that the functionality for
multiple pfn offsets subsumes the use of dev->dma_pfn_offset.
In fact, dma_pfn_offset is removed and supplanted by
dma_pfn_offset_map, which is a pointer to an array. The
more common case of a uniform offset is now handled as
a map with a single entry, while cases requiring multiple
pfn offsets use a map with multiple entries. Code paths
that used to do this:
dev->dma_pfn_offset = mydrivers_pfn_offset;
have been changed to do this:
attach_uniform_dma_pfn_offset(dev, pfn_offset);
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- Add if/then clause for required props: resets, reset-names (RobH)
-- Change compatible list from const to enum (RobH)
-- Change list of u32-tuples to u64 (RobH)
Commit "of: Include a dev param in of_dma_get_range()"
-- modify of/unittests.c to add NULL param in of_dma_get_range() call.
Commit "device core: Add ability to handle multiple dma offsets"
-- align comment in device.h (AndyS).
-- s/cpu_beg/cpu_start/ and s/dma_beg/dma_start/ in struct
dma_pfn_offset_region (AndyS).
v2:
Commit: "device core: Add ability to handle multiple dma offsets"
o Added helper func attach_dma_pfn_offset_map() in address.c (Chistoph)
o Helpers funcs added to __phys_to_dma() & __dma_to_phys() (Christoph)
o Added warning when multiple offsets are needed and !DMA_PFN_OFFSET_MAP
o dev->dma_pfn_map => dev->dma_pfn_offset_map
o s/frm/from/ for dma_pfn_offset_frm_{phys,dma}_addr() (Christoph)
o In device.h: s/const void */const struct dma_pfn_offset_region */
o removed 'unlikely' from unlikely(dev->dma_pfn_offset_map) since
guarded by CONFIG_DMA_PFN_OFFSET_MAP (Christoph)
o Since dev->dma_pfn_offset is copied in usb/core/{usb,message}.c, now
dev->dma_pfn_offset_map is copied as well.
o Merged two of the DMA commits into one (Christoph).
Commit "arm: dma-mapping: Invoke dma offset func if needed":
o Use helper functions instead of #if CONFIG_DMA_PFN_OFFSET
Other commits' changes:
o Removed need for carrying of_id var in priv (Nicolas)
o Commit message rewordings (Bjorn)
o Commit log messages filled to 75 chars (Bjorn)
o devm_reset_control_get_shared())
=> devm_reset_control_get_optional_shared (Philipp)
o Add call to reset_control_assert() in PCIe remove routines (Philipp)
v1:
This patchset expands the usefulness of the Broadcom Settop Box PCIe
controller by building upon the PCIe driver used currently by the
Raspbery Pi. Other forms of this patchset were submitted by me years
ago and not accepted; the major sticking point was the code required
for the DMA remapping needed for the PCIe driver to work [1].
There have been many changes to the DMA and OF subsystems since that
time, making a cleaner and less intrusive patchset possible. This
patchset implements a generalization of "dev->dma_pfn_offset", except
that
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
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
[PATCH v4 00/12] PCI: brcmstb: enable PCIe for STB chips
v4:
Commit "device core: Introduce multiple dma pfn offsets"
-- of_dma_get_range() does not take a dev param but instead
takes two "out" params: map and map_size. We do this so
that the code that parses dma-ranges is separate from
the code that modifies 'dev'. (Nicolas)
-- the separate case of having a single pfn offset has
been removed and is now processed by going through the
map array. (Nicolas)
-- move attach_uniform_dma_pfn_offset() from of/address.c to
dma/mapping.c so that it does not depend on CONFIG_OF. (Nicolas)
-- devm_kcalloc => devm_kzalloc (DanC)
-- add/fix assignment to dev->dma_pfn_offset_map for func
attach_uniform_dma_pfn_offset() (DanC, Nicolas)
-- s/struct dma_pfn_offset_region/struct bus_dma_region/ (Nicolas)
-- s/attach_uniform_dma_pfn_offset/dma_attach_uniform_pfn_offset/
-- s/attach_dma_pfn_offset_map/dma_attach_pfn_offset_map/
-- More use of PFN_{PHYS,DOWN,UP}. (AndyS)
Commit "of: Include a dev param in of_dma_get_range()"
-- this commit was sqaushed with "device core: Introduce ..."
v3:
Commit "device core: Introduce multiple dma pfn offsets"
Commit "arm: dma-mapping: Invoke dma offset func if needed"
-- The above two commits have been squashed. More importantly,
the code has been modified so that the functionality for
multiple pfn offsets subsumes the use of dev->dma_pfn_offset.
In fact, dma_pfn_offset is removed and supplanted by
dma_pfn_offset_map, which is a pointer to an array. The
more common case of a uniform offset is now handled as
a map with a single entry, while cases requiring multiple
pfn offsets use a map with multiple entries. Code paths
that used to do this:
dev->dma_pfn_offset = mydrivers_pfn_offset;
have been changed to do this:
attach_uniform_dma_pfn_offset(dev, pfn_offset);
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- Add if/then clause for required props: resets, reset-names (RobH)
-- Change compatible list from const to enum (RobH)
-- Change list of u32-tuples to u64 (RobH)
Commit "of: Include a dev param in of_dma_get_range()"
-- modify of/unittests.c to add NULL param in of_dma_get_range() call.
Commit "device core: Add ability to handle multiple dma offsets"
-- align comment in device.h (AndyS).
-- s/cpu_beg/cpu_start/ and s/dma_beg/dma_start/ in struct
dma_pfn_offset_region (AndyS).
v2:
Commit: "device core: Add ability to handle multiple dma offsets"
o Added helper func attach_dma_pfn_offset_map() in address.c (Chistoph)
o Helpers funcs added to __phys_to_dma() & __dma_to_phys() (Christoph)
o Added warning when multiple offsets are needed and !DMA_PFN_OFFSET_MAP
o dev->dma_pfn_map => dev->dma_pfn_offset_map
o s/frm/from/ for dma_pfn_offset_frm_{phys,dma}_addr() (Christoph)
o In device.h: s/const void */const struct dma_pfn_offset_region */
o removed 'unlikely' from unlikely(dev->dma_pfn_offset_map) since
guarded by CONFIG_DMA_PFN_OFFSET_MAP (Christoph)
o Since dev->dma_pfn_offset is copied in usb/core/{usb,message}.c, now
dev->dma_pfn_offset_map is copied as well.
o Merged two of the DMA commits into one (Christoph).
Commit "arm: dma-mapping: Invoke dma offset func if needed":
o Use helper functions instead of #if CONFIG_DMA_PFN_OFFSET
Other commits' changes:
o Removed need for carrying of_id var in priv (Nicolas)
o Commit message rewordings (Bjorn)
o Commit log messages filled to 75 chars (Bjorn)
o devm_reset_control_get_shared())
=> devm_reset_control_get_optional_shared (Philipp)
o Add call to reset_control_assert() in PCIe remove routines (Philipp)
v1:
This patchset expands the usefulness of the Broadcom Settop Box PCIe
controller by building upon the PCIe driver used currently by the
Raspbery Pi. Other forms of this patchset were submitted by me years
ago and not accepted; the major sticking point was the code required
for the DMA remapping needed for the PCIe driver to work [1].
There have been many changes to the DMA and OF subsystems since that
time, making a cleaner and less intrusive patchset possible. This
patchset implements a generalization of "dev->dma_pfn_offset", except
that instead of a single scalar offset it provides for multiple
offsets via a function which depends upon the "dma-ranges" property of
the PCIe host controller. This is required for proper functionality
of the BrcmSTB PCIe controller and possibly some other devices.
[1]
https://lore.kernel.org/linux-arm-kernel/1516058925-46522-5-git-send-email-jim2101...@gmail.com/
Jim Quinlan (12):
PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB
ata: ahci_brcm: Fix use of BCM7216 reset controller
dt-bindings: PCI: Add bindings for more Brcmstb chips
PCI: brcmstb: Add bcm7278 register info
PCI: brcmstb: Add suspend and resume pm_ops
PCI: brcmstb: Add bcm7278 PERST support
PCI: brcmstb: Add control of rescal reset
device core:
[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;
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
On Fri, Jun 5, 2020 at 1:27 PM Nicolas Saenz Julienne
wrote:
>
> Hi Christoph,
> a question arouse, is there a real value to dealing with PFNs (as opposed to
> real addresses) in the core DMA code/structures? I see that in some cases it
> eases interacting with mm, but the overwhelming usage of say,
> dev->dma_pfn_offset, involves shifting it.
>
> Hi Jim,
> On Thu, 2020-06-04 at 14:01 -0400, Jim Quinlan wrote:
> > Hi Nicolas,
>
> [...]
>
> > > I understand the need for dev to be around, devm_*() is key. But also it's
> > > important to keep the functions on purpose. And if of_dma_get_range()
> > > starts
> > > setting ranges it calls, for the very least, for a function rename.
> > > Although
> > > I'd rather split the parsing and setting of ranges as mentioned earlier.
> > > That
> > > said, I get that's a more drastic move.
> >
> > I agree with you. I could do this from device.c:
> >
> > of_dma_get_num_ranges(..., _ranges); /* new function */
> > r = devm_kcalloc(dev, num_ranges + 1, sizeof(*r), GFP_KERNEL);
> > of_dma_get_range(np, _addr, , , r, num_ranges);
> >
> > The problem here is that there could be four ranges, all with
> > offset=0. My current code would optimize this case out but the above
> > would have us holding useless memory and looping through the four
> > ranges on every dma <=> phys conversion only to add 0.
>
> Point taken. Ultimately it's setting the device's dma ranges in
> of_dma_get_range() that was really bothering me, so if we have to pass the
> device pointer for allocations, be it.
>
> > > Talking about drastic moves. How about getting rid of the concept of
> > > dma_pfn_offset for drivers altogether. Let them provide
> > > dma_pfn_offset_regions
> > > (even when there is only one). I feel it's conceptually nicer, as you'd be
> > > dealing only in one currency, so to speak, and you'd centralize the bus
> > > DMA
> > > ranges setter function which is always easier to maintain.
> > Do you agree that we have to somehow hang this info on the struct
> > device structure? Because in the dma2phys() and phys2dma() all you
> > have is the dev parameter. I don't see how this can be done w/o
> > involving dev.
>
> Sorry I didn't make myself clear here. What bothers me is having two functions
> setting the same device parameter trough different means, I'd be happy to get
> rid of attach_uniform_dma_pfn_offset(), and always use the same function to
> set
> a device's bus dma regions. Something the likes of this comes to mind:
>
> dma_attach_pfn_offset_region(struct device *dev, struct
> dma_pfn_offset_regions *r)
>
> We could maybe use some helper macros for the linear case. But that's the gist
> of it.
>
> Also, it goes hand in hand with the comment below. Why having a special case
> for non sparse DMA offsets in struct dma_pfn_offset_regions? The way I see it,
> in this case, code simplicity is more interesting than a small optimization.
I've removed the special case and also need for 'dev' in
of_dma_get_range(). v4 is comming...
>
> > > I'd go as far as not creating a special case for uniform offsets. Let just
> > > set
> > > cpu_end and dma_end to -1 so we always get a match. It's slightly more
> > > compute
> > > heavy, but I don't think it's worth the optimization.
> > Well, there are two subcases here. One where we do know the bounds
> > and one where we do not. I suppose for the latter I could have the
> > drivers calling it with begin=0 and end=~(dma_addr_t)0. Let me give
> > this some thought...
> >
> > > Just my two cents :)
> >
> > Worth much more than $0.02 IMO :-)
>
> BTW, would you consider renaming the DMA offset struct to something simpler
> like, struct bus_dma_region? It complements 'dev->bus_dma_limit' better IMO.
Will do
Thanks,
Jim
>
> > BTW, I tried putting the "if (dev->dma_pfn_offset_map)" clause inside
> > the inline functions but the problem is that it slows the fastpath;
> > consider the following code from dma-direct.h
> >
> > if (dev->dma_pfn_offset_map) {
> > unsigned long dma_pfn_offset =
> dma_pfn_offset_from_phys_addr(dev, paddr);
> >
> > dev_addr -= ((dma_addr_t)dma_pfn_offset << PAGE_SHIFT);
> > }
> > return dev_addr;
> >
> > becomes
> >
> > unsigned long dma_pfn_offset = dma_pfn_offset_from_phys_addr(dev,
> paddr);
> >
> > dev_addr -= ((dma_addr_t)dma_pfn_offset << PAGE_SHIFT);
> > return dev_addr;
> >
> > So those configurations that have no dma_pfn_offsets are doing an
> > unnecessary shift and add.
>
> Fair enough. Still not a huge difference, but I see the value being the most
> common case.
>
> Regards,
> Nicolas
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
Hi Nicolas,
On Thu, Jun 4, 2020 at 12:52 PM Nicolas Saenz Julienne
wrote:
>
> Hi Jim,
>
> On Thu, 2020-06-04 at 10:35 -0400, Jim Quinlan wrote:
>
> [...]
>
> > > > --- a/arch/sh/kernel/dma-coherent.c
> > > > +++ b/arch/sh/kernel/dma-coherent.c
> > > > @@ -14,6 +14,8 @@ void *arch_dma_alloc(struct device *dev, size_t size,
> > > > dma_addr_t *dma_handle,
> > > > {
> > > > void *ret, *ret_nocache;
> > > > int order = get_order(size);
> > > > + unsigned long pfn;
> > > > + phys_addr_t phys;
> > > >
> > > > gfp |= __GFP_ZERO;
> > > >
> > > > @@ -34,11 +36,14 @@ void *arch_dma_alloc(struct device *dev, size_t
> > > > size,
> > > > dma_addr_t *dma_handle,
> > > > return NULL;
> > > > }
> > > >
> > > > - split_page(pfn_to_page(virt_to_phys(ret) >> PAGE_SHIFT), order);
> > > > + phys = virt_to_phys(ret);
> > > > + pfn = phys >> PAGE_SHIFT;
> > >
> > > nit: not sure it really pays off to have a pfn variable here.
> > Did it for readability; the compiler's optimization should take care
> > of any extra variables. But I can switch if you insist.
>
> No need.
>
> [...]
>
> > > > 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 = >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)
> > >
> > > As with the previous review, please take this comment with a grain of
> > > salt.
> > >
> > > I think there should be a clear split between what belongs to OF and what
> > > belongs to the core device infrastructure.
> > >
> > > OF's job should be to parse DT and provide a list/array of ranges, whereas
> > > the
> > > core device infrastructure should provide an API to assign a list of
> > > ranges/offset to a device.
> > >
> > > As a concrete example, you're forcing devices like the sta2x11 to build
> > > with
> > > OF
> > > support, which, being an Intel device, it's pretty odd. But I'm also
> > > thinking
> > > of how will all this fit once an ACPI device wants to use it.
> > To fix this I only have to move attach_uniform_dma_pfn_offset() from
> > of/address.c to say include/linux/dma-mapping.h. It has no
> > dependencies on OF. Do you agree?
>
> Yes that seems nicer. In case you didn't had it in mind already, I'd change
> the
> function name to match the naming scheme they use there.
>
> On the other hand, I'd also move the non OF parts of the non unifom dma_offset
> version of the function there.
>
> > > Expanding on this idea, once you have a split between the OF's and device
> > > core
> > > roles, it transpires that of_dma_get_range()'s job should only be to
> > > provide
> > > the ranges in a device understandable structure and of_dma_configre()'s to
> > > actually assign the device's parameters. This would obsolete patch #7.
> >
> > I think you mean patch #8.
>
> Yes, my bad.
>
> > I agree with you. The reason I needed a "struct device *" in the call is
> > because I wanted to make sure the memory that is alloc'd belongs to the
> > device that needs it. If I do a regular kzalloc(), this memory will become
> > a leak once someone starts unbinding/binding their device. Also, in all
> > uses of of_dma_rtange() -- there is only one -- a dev is required as one
> > can't attach an offset map to NULL.
> >
> > I do see that there are a number of functions in drivers/of/*.c that
> > take 'struct device *dev' as an argument so there is precedent for
> > something like this. Regardless, I need an owner to the memory I
> > alloc().
>
> I understand the need for dev to be around, devm_*() is key. But also it's
> important to keep the functions on purpose. And if of_dma_get_range() starts
> setting ranges it calls, for the very least, for a function rename. Although
> I'd rather split the parsing and setting
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
Hi Andy, On Thu, Jun 4, 2020 at 11:05 AM Andy Shevchenko wrote: > > On Thu, Jun 04, 2020 at 10:35:12AM -0400, Jim Quinlan wrote: > > On Thu, Jun 4, 2020 at 9:53 AM Nicolas Saenz Julienne > > wrote: > > > On Wed, 2020-06-03 at 15:20 -0400, Jim Quinlan wrote: > > ... > > > > > + phys = virt_to_phys(ret); > > > > + pfn = phys >> PAGE_SHIFT; > > > > > > nit: not sure it really pays off to have a pfn variable here. > > Did it for readability; the compiler's optimization should take care > > of any extra variables. But I can switch if you insist. > > One side note: please, try to get familiar with existing helpers in the > kernel. > For example, above line is like > > pfn = PFN_DOWN(phys); I just used the term in the original code; will change to PFN_DOWN(). > > ... > > > > > + if (!WARN_ON(!dev) && dev->dma_pfn_offset_map) > > > > > + *dma_handle -= PFN_PHYS( > > > > + dma_pfn_offset_from_phys_addr(dev, phys)); > > Don't do such indentation, esp. we have now 100! :-) Got it. 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 v3 09/13] device core: Introduce multiple dma pfn offsets
On Thu, Jun 4, 2020 at 10:20 AM Dan Carpenter wrote: > > On Thu, Jun 04, 2020 at 09:48:49AM -0400, Jim Quinlan wrote: > > > > + r = devm_kcalloc(dev, 1, sizeof(struct dma_pfn_offset_region), > > > > + GFP_KERNEL); > > > > > > Use:r = devm_kzalloc(dev, sizeof(*r), GFP_KERNEL); > > Will fix. > > > > > > > > > > > > + 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? > > No, the uniform pfn offset is a special case. > > Sorry, I wasn't clear. We're talking about different things. The code > does: > > r = devm_kzalloc(dev, sizeof(*r), GFP_KERNEL); > if (!r) > return -ENOMEM; > > r->uniform_offset = true; > r->pfn_offset = pfn_offset; > > return 0; > > The code allocates "r" and then doesn't save it anywhere so there is > no point. You are absolutely right, sorry I missed your point. Will fix. Thanks, Jim Quinlan > > regards, > dan carpenter > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
On Thu, Jun 4, 2020 at 9:53 AM Nicolas Saenz Julienne
wrote:
>
> Hi Jim,
>
> On Wed, 2020-06-03 at 15:20 -0400, Jim Quinlan wrote:
> > The new field in struct device 'dma_pfn_offset_map' is used to facilitate
> > the use of multiple pfn offsets between cpu addrs and dma addrs. It
> > subsumes the role of dev->dma_pfn_offset -- a uniform offset -- and
> > designates the single offset a special case.
> >
> > 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 code. These cases now invoke the function
> > attach_uniform_dma_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 | 17 ++--
> > arch/x86/pci/sta2x11-fixup.c | 7 +-
> > drivers/acpi/arm64/iort.c | 5 +-
> > drivers/gpu/drm/sun4i/sun4i_backend.c | 7 +-
> > drivers/iommu/io-pgtable-arm.c| 2 +-
> > .../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +-
> > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 5 +-
> > drivers/of/address.c | 93 +--
> > drivers/of/device.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 | 45 +
> > kernel/dma/coherent.c | 11 ++-
> > 20 files changed, 210 insertions(+), 51 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)
>
> Would it make sense to move the dev->dma_pfn_offset_map check into
> dma_pfn_offset_from_phys_addr() and return 0 if not available? Same for the
> opposite variant of the function. I think it'd make the code a little simpler
> on
> some of the use cases, and overall less error prone if anyone starts using the
> function elsewhere.
Yes it makes sense and I was debating doing it but I just wanted to
make it explicit that there was not much cost for this change for the
fastpath -- no dma_pfn_offset whatsoever -- as the cost goes from a
"pfn += dev->dma_pfn_offset" to a "if (dev->dma_pfn_offset_map)". I
will do what you suggest.
>
> > + 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..e7d3ee6e9cb5 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 = attach_uniform_dma_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..2e832a5c58c1 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
> >
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
Hi Dan,
On Thu, Jun 4, 2020 at 7:06 AM Dan Carpenter wrote:
>
> 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.
Will fix.
>
> >
> > 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
> > +#include
> > #include
> > #include
> > #include
> > @@ -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 = >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(, 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(, ) {
> > + 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.
>
[PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
The new field in struct device 'dma_pfn_offset_map' is used to facilitate
the use of multiple pfn offsets between cpu addrs and dma addrs. It
subsumes the role of dev->dma_pfn_offset -- a uniform offset -- and
designates the single offset a special case.
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 code. These cases now invoke the function
attach_uniform_dma_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 | 17 ++--
arch/x86/pci/sta2x11-fixup.c | 7 +-
drivers/acpi/arm64/iort.c | 5 +-
drivers/gpu/drm/sun4i/sun4i_backend.c | 7 +-
drivers/iommu/io-pgtable-arm.c| 2 +-
.../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +-
.../platform/sunxi/sun6i-csi/sun6i_csi.c | 5 +-
drivers/of/address.c | 93 +--
drivers/of/device.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 | 45 +
kernel/dma/coherent.c | 11 ++-
20 files changed, 210 insertions(+), 51 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..e7d3ee6e9cb5 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 = attach_uniform_dma_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..2e832a5c58c1 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;
+ attach_uniform_dma_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..5fc9e358b6c7 100644
--- a/arch/sh/kernel/dma-coherent.c
+++ b/arch/sh/kernel/dma-coherent.c
@@ -14,6 +14,8 @@ void *arch_dma_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle,
{
void *ret, *ret_nocache;
int order = get_order(size);
+ unsigned long pfn;
+ phys_addr_t phys;
gfp |= __GFP_ZERO;
@@ -34,11 +36,14 @@ void *arch_dma_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle,
return NULL;
}
- split_page(pfn_to_page(virt_to_phys(ret) >> PAGE_SHIFT), order);
+ phys = virt_to_phys(ret);
+ pfn = phys >> PAGE_SHIFT;
+ split_page(pfn_to_page(pfn), order);
- *dma_handle =
[PATCH v3 00/13] PCI: brcmstb: enable PCIe for STB chips
v3:
Commit "device core: Introduce multiple dma pfn offsets"
Commit "arm: dma-mapping: Invoke dma offset func if needed"
-- The above two commits have been squashed. More importantly,
the code has been modified so that the functionality for
multiple pfn offsets subsumes the use of dev->dma_pfn_offset.
In fact, dma_pfn_offset is removed and supplanted by
dma_pfn_offset_map, which is a pointer to an array. The
more common case of a uniform offset is now handled as
a map with a single entry, while cases requiring multiple
pfn offsets use a map with multiple entries. Code paths
that used to do this:
dev->dma_pfn_offset = mydrivers_pfn_offset;
have been changed to do this:
attach_uniform_dma_pfn_offset(dev, pfn_offset);
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- Add if/then clause for required props: resets, reset-names (RobH)
-- Change compatible list from const to enum (RobH)
-- Change list of u32-tuples to u64 (RobH)
Commit "of: Include a dev param in of_dma_get_range()"
-- modify of/unittests.c to add NULL param in of_dma_get_range() call.
Commit "device core: Add ability to handle multiple dma offsets"
-- align comment in device.h (AndyS).
-- s/cpu_beg/cpu_start/ and s/dma_beg/dma_start/ in struct
dma_pfn_offset_region (AndyS).
v2:
Commit: "device core: Add ability to handle multiple dma offsets"
o Added helper func attach_dma_pfn_offset_map() in address.c (Chistoph)
o Helpers funcs added to __phys_to_dma() & __dma_to_phys() (Christoph)
o Added warning when multiple offsets are needed and !DMA_PFN_OFFSET_MAP
o dev->dma_pfn_map => dev->dma_pfn_offset_map
o s/frm/from/ for dma_pfn_offset_frm_{phys,dma}_addr() (Christoph)
o In device.h: s/const void */const struct dma_pfn_offset_region */
o removed 'unlikely' from unlikely(dev->dma_pfn_offset_map) since
guarded by CONFIG_DMA_PFN_OFFSET_MAP (Christoph)
o Since dev->dma_pfn_offset is copied in usb/core/{usb,message}.c, now
dev->dma_pfn_offset_map is copied as well.
o Merged two of the DMA commits into one (Christoph).
Commit "arm: dma-mapping: Invoke dma offset func if needed":
o Use helper functions instead of #if CONFIG_DMA_PFN_OFFSET
Other commits' changes:
o Removed need for carrying of_id var in priv (Nicolas)
o Commit message rewordings (Bjorn)
o Commit log messages filled to 75 chars (Bjorn)
o devm_reset_control_get_shared())
=> devm_reset_control_get_optional_shared (Philipp)
o Add call to reset_control_assert() in PCIe remove routines (Philipp)
v1:
This patchset expands the usefulness of the Broadcom Settop Box PCIe
controller by building upon the PCIe driver used currently by the
Raspbery Pi. Other forms of this patchset were submitted by me years
ago and not accepted; the major sticking point was the code required
for the DMA remapping needed for the PCIe driver to work [1].
There have been many changes to the DMA and OF subsystems since that
time, making a cleaner and less intrusive patchset possible. This
patchset implements a generalization of "dev->dma_pfn_offset", except
that instead of a single scalar offset it provides for multiple
offsets via a function which depends upon the "dma-ranges" property of
the PCIe host controller. This is required for proper functionality
of the BrcmSTB PCIe controller and possibly some other devices.
[1]
https://lore.kernel.org/linux-arm-kernel/1516058925-46522-5-git-send-email-jim2101...@gmail.com/
Jim Quinlan (13):
PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB
ata: ahci_brcm: Fix use of BCM7216 reset controller
dt-bindings: PCI: Add bindings for more Brcmstb chips
PCI: brcmstb: Add bcm7278 reigister info
PCI: brcmstb: Add suspend and resume pm_ops
PCI: brcmstb: Add bcm7278 PERST support
PCI: brcmstb: Add control of rescal reset
of: Include a dev param in of_dma_get_range()
device core: Introduce multiple dma pfn offsets
PCI: brcmstb: Set internal memory viewport sizes
PCI: brcmstb: Accommodate MSI for older chips
PCI: brcmstb: Set bus max burst size by chip type
PCI: brcmstb: Add bcm7211, bcm7216, bcm7445, bcm7278 to match list
.../bindings/pci/brcm,stb-pcie.yaml | 58 ++-
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 | 17 +-
arch/x86/pci/sta2x11-fixup.c | 7 +-
drivers/acpi/arm64/iort.c | 5 +-
drivers/ata/ahci_brcm.c | 14 +-
drivers/gpu/drm/sun4i/sun4i_backend.c | 7 +-
drivers/iommu/io-pgtable-arm.c| 2 +-
.../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +-
.../platform/sunxi/sun6i-csi/sun6i_csi.c | 5 +-
drivers/of/address.c | 97 -
drivers/of/device.c | 10 +-
Re: [PATCH v2 00/14] PCI: brcmstb: enable PCIe for STB chips
On Fri, May 29, 2020 at 1:49 PM Rob Herring wrote:
>
> On Tue, May 26, 2020 at 03:12:39PM -0400, Jim Quinlan wrote:
> > v2:
> > Commit: "device core: Add ability to handle multiple dma offsets"
> > o Added helper func attach_dma_pfn_offset_map() in address.c (Chistoph)
> > o Helpers funcs added to __phys_to_dma() & __dma_to_phys() (Christoph)
> > o Added warning when multiple offsets are needed and !DMA_PFN_OFFSET_MAP
> > o dev->dma_pfn_map => dev->dma_pfn_offset_map
> > o s/frm/from/ for dma_pfn_offset_frm_{phys,dma}_addr() (Christoph)
> > o In device.h: s/const void */const struct dma_pfn_offset_region */
> > o removed 'unlikely' from unlikely(dev->dma_pfn_offset_map) since
> > guarded by CONFIG_DMA_PFN_OFFSET_MAP (Christoph)
> > o Since dev->dma_pfn_offset is copied in usb/core/{usb,message}.c, now
> > dev->dma_pfn_offset_map is copied as well.
> > o Merged two of the DMA commits into one (Christoph).
> >
> > Commit "arm: dma-mapping: Invoke dma offset func if needed":
> > o Use helper functions instead of #if CONFIG_DMA_PFN_OFFSET
> >
> > Other commits' changes:
> > o Removed need for carrying of_id var in priv (Nicolas)
> > o Commit message rewordings (Bjorn)
> > o Commit log messages filled to 75 chars (Bjorn)
> > o devm_reset_control_get_shared())
> > => devm_reset_control_get_optional_shared (Philipp)
> > o Add call to reset_control_assert() in PCIe remove routines (Philipp)
> >
> > v1:
> > This patchset expands the usefulness of the Broadcom Settop Box PCIe
> > controller by building upon the PCIe driver used currently by the
> > Raspbery Pi. Other forms of this patchset were submitted by me years
> > ago and not accepted; the major sticking point was the code required
> > for the DMA remapping needed for the PCIe driver to work [1].
> >
> > There have been many changes to the DMA and OF subsystems since that
> > time, making a cleaner and less intrusive patchset possible. This
> > patchset implements a generalization of "dev->dma_pfn_offset", except
> > that instead of a single scalar offset it provides for multiple
> > offsets via a function which depends upon the "dma-ranges" property of
> > the PCIe host controller. This is required for proper functionality
> > of the BrcmSTB PCIe controller and possibly some other devices.
>
> If you can enable the h/w support without the multiple offset support,
> then I'd split up this series. The latter part might take a bit more
> time.
>
> Rob
Unfortunately, the STB PCIe controller depends on the multiple PFN
offset functionality.
Thanks,
Jim
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 09/14] device core: Add ability to handle multiple dma offsets
On Fri, May 29, 2020 at 1:35 PM Rob Herring wrote:
>
> On Wed, May 27, 2020 at 9:43 AM Jim Quinlan
> wrote:
> >
> > Hi Nicolas,
> >
> > On Wed, May 27, 2020 at 11:00 AM Nicolas Saenz Julienne
> > wrote:
> > >
> > > Hi Jim,
> > > one thing comes to mind, there is a small test suite in
> > > drivers/of/unittest.c
> > > (specifically of_unittest_pci_dma_ranges()) you could extend it to
> > > include your
> > > use cases.
> > Sure, will check out.
> > >
> > > On Tue, 2020-05-26 at 15:12 -0400, Jim Quinlan wrote:
> > > > The new field in struct device 'dma_pfn_offset_map' is used to
> > > > facilitate
> > > > the use of multiple pfn offsets between cpu addrs and dma addrs. It is
> > > > similar to 'dma_pfn_offset' except that the offset chosen depends on the
> > > > cpu or dma address involved.
> > > >
> > > > Signed-off-by: Jim Quinlan
> > > > ---
> > > > drivers/of/address.c| 65 +++--
> > > > drivers/usb/core/message.c | 3 ++
> > > > drivers/usb/core/usb.c | 3 ++
> > > > include/linux/device.h | 10 +-
> > > > include/linux/dma-direct.h | 10 --
> > > > include/linux/dma-mapping.h | 46 ++
> > > > kernel/dma/Kconfig | 13
> > > > 7 files changed, 144 insertions(+), 6 deletions(-)
> > > >
> > >
> > > [...]
> > >
> > > > @@ -977,10 +1020,19 @@ int of_dma_get_range(struct device *dev, struct
> > > > device_node *np, u64 *dma_addr,
> > > > pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
> > > >range.bus_addr, range.cpu_addr, range.size);
> > > >
> > > > + num_ranges++;
> > > > if (dma_offset && range.cpu_addr - range.bus_addr !=
> > > > dma_offset)
> > > > {
> > > > - pr_warn("Can't handle multiple dma-ranges with
> > > > different
> > > > offsets on node(%pOF)\n", node);
> > > > - /* Don't error out as we'd break some existing
> > > > DTs */
> > > > - continue;
> > > > + if (!IS_ENABLED(CONFIG_DMA_PFN_OFFSET_MAP)) {
> > > > + pr_warn("Can't handle multiple dma-ranges
> > > > with
> > > > different offsets on node(%pOF)\n", node);
> > > > + pr_warn("Perhaps set
> > > > DMA_PFN_OFFSET_MAP=y?\n");
> > > > + /*
> > > > + * Don't error out as we'd break some
> > > > existing
> > > > + * DTs that are using configs w/o
> > > > + * CONFIG_DMA_PFN_OFFSET_MAP set.
> > > > + */
> > > > + continue;
> > >
> > > dev->bus_dma_limit is set in of_dma_configure(), this function's caller,
> > > based
> > > on dma_start's value (set after this continue). So you'd be effectively
> > > setting
> > > the dev->bus_dma_limit to whatever we get from the first dma-range.
> > I'm not seeing that at all. On the evaluation of each dma-range,
> > dma_start and dma_end are re-evaluated to be the lowest and highest
> > bus values of the dma-ranges seen so far. After all dma-ranges are
> > examined, dev->bus_dma_limit being set to the highest. In fact, the
> > current code -- ie before my commits -- already does this for multiple
> > dma-ranges as long as the cpu-bus offset is the same in the
> > dma-ranges.
> > >
> > > This can be troublesome depending on how the dma-ranges are setup, for
> > > example
> > > if the first dma-range doesn't include the CMA area, in arm64 generally
> > > set as
> > > high as possible in ZONE_DMA32, that would render it useless for
> > > dma/{direct/swiotlb}. Again depending on the bus_dma_limit value, if
> > > smaller
> > > than ZONE_DMA you'd be unable to allocate any DMA memory.
> > >
> > > IMO, a solution to this calls for a revamp of dma-direct's dma_capable():
> > > match
> > > the target DMA memory area with each dma-range we have to see if it fits.
> > >
> > > > + }
> > > > + dma_multi_pfn_offset = true;
> > > > }
> > > > dma_offset = range.cpu_addr - range.bus_addr;
> > > >
> > > > @@ -991,6 +1043,13 @@ int of_dma_get_range(struct device *dev, struct
> > > > device_node *np, u64 *dma_addr,
> > > > dma_end = range.bus_addr + range.size;
> > > > }
> > > >
> > > > + if (dma_multi_pfn_offset) {
> > > > + dma_offset = 0;
> > > > + ret = attach_dma_pfn_offset_map(dev, node, num_ranges);
> > > > + if (ret)
> > > > + return ret;
> > > > + }
> > > > +
> > > > if (dma_start >= dma_end) {
> > > > ret = -EINVAL;
> > > > pr_debug("Invalid DMA ranges configuration on
> > > > node(%pOF)\n",
> > > > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> > > > index 6197938dcc2d..aaa3e58f5eb4 100644
> > > > ---
Re: [PATCH v2 09/14] device core: Add ability to handle multiple dma offsets
Hi Nicolas,
On Wed, May 27, 2020 at 11:00 AM Nicolas Saenz Julienne
wrote:
>
> Hi Jim,
> one thing comes to mind, there is a small test suite in drivers/of/unittest.c
> (specifically of_unittest_pci_dma_ranges()) you could extend it to include
> your
> use cases.
Sure, will check out.
>
> On Tue, 2020-05-26 at 15:12 -0400, Jim Quinlan wrote:
> > The new field in struct device 'dma_pfn_offset_map' is used to facilitate
> > the use of multiple pfn offsets between cpu addrs and dma addrs. It is
> > similar to 'dma_pfn_offset' except that the offset chosen depends on the
> > cpu or dma address involved.
> >
> > Signed-off-by: Jim Quinlan
> > ---
> > drivers/of/address.c| 65 +++--
> > drivers/usb/core/message.c | 3 ++
> > drivers/usb/core/usb.c | 3 ++
> > include/linux/device.h | 10 +-
> > include/linux/dma-direct.h | 10 --
> > include/linux/dma-mapping.h | 46 ++
> > kernel/dma/Kconfig | 13
> > 7 files changed, 144 insertions(+), 6 deletions(-)
> >
>
> [...]
>
> > @@ -977,10 +1020,19 @@ int of_dma_get_range(struct device *dev, struct
> > device_node *np, u64 *dma_addr,
> > pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
> >range.bus_addr, range.cpu_addr, range.size);
> >
> > + num_ranges++;
> > if (dma_offset && range.cpu_addr - range.bus_addr !=
> > dma_offset)
> > {
> > - pr_warn("Can't handle multiple dma-ranges with
> > different
> > offsets on node(%pOF)\n", node);
> > - /* Don't error out as we'd break some existing DTs */
> > - continue;
> > + if (!IS_ENABLED(CONFIG_DMA_PFN_OFFSET_MAP)) {
> > + pr_warn("Can't handle multiple dma-ranges with
> > different offsets on node(%pOF)\n", node);
> > + pr_warn("Perhaps set
> > DMA_PFN_OFFSET_MAP=y?\n");
> > + /*
> > + * Don't error out as we'd break some existing
> > + * DTs that are using configs w/o
> > + * CONFIG_DMA_PFN_OFFSET_MAP set.
> > + */
> > + continue;
>
> dev->bus_dma_limit is set in of_dma_configure(), this function's caller, based
> on dma_start's value (set after this continue). So you'd be effectively
> setting
> the dev->bus_dma_limit to whatever we get from the first dma-range.
I'm not seeing that at all. On the evaluation of each dma-range,
dma_start and dma_end are re-evaluated to be the lowest and highest
bus values of the dma-ranges seen so far. After all dma-ranges are
examined, dev->bus_dma_limit being set to the highest. In fact, the
current code -- ie before my commits -- already does this for multiple
dma-ranges as long as the cpu-bus offset is the same in the
dma-ranges.
>
> This can be troublesome depending on how the dma-ranges are setup, for example
> if the first dma-range doesn't include the CMA area, in arm64 generally set as
> high as possible in ZONE_DMA32, that would render it useless for
> dma/{direct/swiotlb}. Again depending on the bus_dma_limit value, if smaller
> than ZONE_DMA you'd be unable to allocate any DMA memory.
>
> IMO, a solution to this calls for a revamp of dma-direct's dma_capable():
> match
> the target DMA memory area with each dma-range we have to see if it fits.
>
> > + }
> > + dma_multi_pfn_offset = true;
> > }
> > dma_offset = range.cpu_addr - range.bus_addr;
> >
> > @@ -991,6 +1043,13 @@ int of_dma_get_range(struct device *dev, struct
> > device_node *np, u64 *dma_addr,
> > dma_end = range.bus_addr + range.size;
> > }
> >
> > + if (dma_multi_pfn_offset) {
> > + dma_offset = 0;
> > + ret = attach_dma_pfn_offset_map(dev, node, num_ranges);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > if (dma_start >= dma_end) {
> > ret = -EINVAL;
> > pr_debug("Invalid DMA ranges configuration on node(%pOF)\n",
> > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> > index 6197938dcc2d..aaa3e58f5eb4 100644
> > --- a/drivers/usb/core/message.c
> > +++ b/drivers/usb/core/message.c
> > @@ -1960,6 +1960,9 @@ int usb_set_configuration(struct usb_device *dev, int
> > configuration)
> >*/
> > intf->dev.dma_mask = dev->dev.dma_mask;
> > intf->dev.dma_pfn_offset = dev->dev.dma_pfn_offset;
> > +#ifdef CONFIG_DMA_PFN_OFFSET_MAP
> > + intf->dev.dma_pfn_offset_map = dev->dev.dma_pfn_offset_map;
> > +#endif
>
> Thanks for looking at this, that said, I see more instances of drivers
> changing
> dma_pfn_offset outside of the core code. Why not doing this there too?
>
Re: [PATCH v2 09/14] device core: Add ability to handle multiple dma offsets
Hello Andy,
On Tue, May 26, 2020 at 4:54 PM Andy Shevchenko
wrote:
>
> On Tue, May 26, 2020 at 03:12:48PM -0400, Jim Quinlan wrote:
> > The new field in struct device 'dma_pfn_offset_map' is used to facilitate
> > the use of multiple pfn offsets between cpu addrs and dma addrs. It is
> > similar to 'dma_pfn_offset' except that the offset chosen depends on the
> > cpu or dma address involved.
> >
> > Signed-off-by: Jim Quinlan
> > ---
> > drivers/of/address.c| 65 +++--
> > drivers/usb/core/message.c | 3 ++
> > drivers/usb/core/usb.c | 3 ++
> > include/linux/device.h | 10 +-
> > include/linux/dma-direct.h | 10 --
> > include/linux/dma-mapping.h | 46 ++
> > kernel/dma/Kconfig | 13
> > 7 files changed, 144 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index 96d8cfb14a60..a01afffcde7d 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -918,6 +918,47 @@ void __iomem *of_io_request_and_map(struct device_node
> > *np, int index,
> > }
> > EXPORT_SYMBOL(of_io_request_and_map);
> >
> > +#ifdef CONFIG_DMA_PFN_OFFSET_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;
> > + size_t r_size = (num_ranges + 1)
> > + * sizeof(struct dma_pfn_offset_region);
> > + struct dma_pfn_offset_region *r;
> > +
>
> > + r = devm_kzalloc(dev, r_size, GFP_KERNEL);
>
> devm_?!
Yes, otherwise if the device gets unbound/bound repeatedly then there
would be a memory leak.
>
>
> Looking at r_size it should be rather kcalloc().
Yep.
>
>
> > + if (!r)
> > + return -ENOMEM;
> > + dev->dma_pfn_offset_map = r;
> > + of_dma_range_parser_init(, 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(, ) {
> > + r->cpu_beg = range.cpu_addr;
> > + r->cpu_end = r->cpu_beg + range.size;
> > + r->dma_beg = range.bus_addr;
> > + r->dma_end = r->dma_beg + range.size;
> > + r->pfn_offset = PFN_DOWN(range.cpu_addr)
> > + - PFN_DOWN(range.bus_addr);
> > + r++;
> > + }
> > + return 0;
> > +}
> > +#else
> > +static int attach_dma_pfn_offset_map(struct device *dev,
> > + struct device_node *node, int num_ranges)
> > +{
> > + return 0;
> > +}
> > +#endif
> > +
> > /**
> > * of_dma_get_range - Get DMA range info
> > * @dev: device pointer; only needed for a corner case.
> > @@ -947,6 +988,8 @@ int of_dma_get_range(struct device *dev, struct
> > device_node *np, u64 *dma_addr,
> > struct of_range_parser parser;
> > struct of_range range;
> > u64 dma_start = U64_MAX, dma_end = 0, dma_offset = 0;
> > + bool dma_multi_pfn_offset = false;
> > + int num_ranges = 0;
> >
> > while (node) {
> > ranges = of_get_property(node, "dma-ranges", );
> > @@ -977,10 +1020,19 @@ int of_dma_get_range(struct device *dev, struct
> > device_node *np, u64 *dma_addr,
> > pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
> >range.bus_addr, range.cpu_addr, range.size);
> >
> > + num_ranges++;
> > if (dma_offset && range.cpu_addr - range.bus_addr !=
> > dma_offset) {
> > - pr_warn("Can't handle multiple dma-ranges with
> > different offsets on node(%pOF)\n", node);
> > - /* Don't error out as we'd break some existing DTs */
> > - continue;
> > + if (!IS_ENABLED(CONFIG_DMA_PFN_OFFSET_MAP)) {
> > + pr_warn("Can't handle multiple dma-ranges
> > with different offsets on node(%pOF)\n", node);
> > + pr_warn("Perhaps set
> > DMA_PFN_OFFSET_MAP=y?\n");
> > + /*
> > + * Don't error out as we'd break some existing
> > + * DTs that are using configs w/o
> > + * CONFIG_DMA_PFN_OFFSET_MAP set.
> > + */
> > + continue;
> > + }
> > + dma_multi_pfn_offset = true;
> > }
> > dma_offset = range.cpu_addr - range.bus_addr;
> >
> > @@ -991,6 +1043,13 @@ int of_dma_get_range(struct device *dev, struct
> > device_node *np, u64 *dma_addr,
> > dma_end = range.bus_addr + range.size;
> > }
> >
> > + if
[PATCH v2 09/14] device core: Add ability to handle multiple dma offsets
The new field in struct device 'dma_pfn_offset_map' is used to facilitate
the use of multiple pfn offsets between cpu addrs and dma addrs. It is
similar to 'dma_pfn_offset' except that the offset chosen depends on the
cpu or dma address involved.
Signed-off-by: Jim Quinlan
---
drivers/of/address.c| 65 +++--
drivers/usb/core/message.c | 3 ++
drivers/usb/core/usb.c | 3 ++
include/linux/device.h | 10 +-
include/linux/dma-direct.h | 10 --
include/linux/dma-mapping.h | 46 ++
kernel/dma/Kconfig | 13
7 files changed, 144 insertions(+), 6 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 96d8cfb14a60..a01afffcde7d 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -918,6 +918,47 @@ void __iomem *of_io_request_and_map(struct device_node
*np, int index,
}
EXPORT_SYMBOL(of_io_request_and_map);
+#ifdef CONFIG_DMA_PFN_OFFSET_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;
+ size_t r_size = (num_ranges + 1)
+ * sizeof(struct dma_pfn_offset_region);
+ struct dma_pfn_offset_region *r;
+
+ r = devm_kzalloc(dev, r_size, GFP_KERNEL);
+ if (!r)
+ return -ENOMEM;
+ dev->dma_pfn_offset_map = r;
+ of_dma_range_parser_init(, 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(, ) {
+ r->cpu_beg = range.cpu_addr;
+ r->cpu_end = r->cpu_beg + range.size;
+ r->dma_beg = range.bus_addr;
+ r->dma_end = r->dma_beg + range.size;
+ r->pfn_offset = PFN_DOWN(range.cpu_addr)
+ - PFN_DOWN(range.bus_addr);
+ r++;
+ }
+ return 0;
+}
+#else
+static int attach_dma_pfn_offset_map(struct device *dev,
+struct device_node *node, int num_ranges)
+{
+ return 0;
+}
+#endif
+
/**
* of_dma_get_range - Get DMA range info
* @dev: device pointer; only needed for a corner case.
@@ -947,6 +988,8 @@ int of_dma_get_range(struct device *dev, struct device_node
*np, u64 *dma_addr,
struct of_range_parser parser;
struct of_range range;
u64 dma_start = U64_MAX, dma_end = 0, dma_offset = 0;
+ bool dma_multi_pfn_offset = false;
+ int num_ranges = 0;
while (node) {
ranges = of_get_property(node, "dma-ranges", );
@@ -977,10 +1020,19 @@ int of_dma_get_range(struct device *dev, struct
device_node *np, u64 *dma_addr,
pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
range.bus_addr, range.cpu_addr, range.size);
+ num_ranges++;
if (dma_offset && range.cpu_addr - range.bus_addr !=
dma_offset) {
- pr_warn("Can't handle multiple dma-ranges with
different offsets on node(%pOF)\n", node);
- /* Don't error out as we'd break some existing DTs */
- continue;
+ if (!IS_ENABLED(CONFIG_DMA_PFN_OFFSET_MAP)) {
+ pr_warn("Can't handle multiple dma-ranges with
different offsets on node(%pOF)\n", node);
+ pr_warn("Perhaps set DMA_PFN_OFFSET_MAP=y?\n");
+ /*
+* Don't error out as we'd break some existing
+* DTs that are using configs w/o
+* CONFIG_DMA_PFN_OFFSET_MAP set.
+*/
+ continue;
+ }
+ dma_multi_pfn_offset = true;
}
dma_offset = range.cpu_addr - range.bus_addr;
@@ -991,6 +1043,13 @@ int of_dma_get_range(struct device *dev, struct
device_node *np, u64 *dma_addr,
dma_end = range.bus_addr + range.size;
}
+ if (dma_multi_pfn_offset) {
+ dma_offset = 0;
+ ret = attach_dma_pfn_offset_map(dev, node, num_ranges);
+ if (ret)
+ return ret;
+ }
+
if (dma_start >= dma_end) {
ret = -EINVAL;
pr_debug("Invalid DMA ranges configuration on node(%pOF)\n",
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 6197938dcc2d..aaa3e58f5eb4 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1960,6 +1960,9 @@ int usb_set_configuration(struct usb_device *dev, int
configuration)
[PATCH v2 00/14] PCI: brcmstb: enable PCIe for STB chips
v2:
Commit: "device core: Add ability to handle multiple dma offsets"
o Added helper func attach_dma_pfn_offset_map() in address.c (Chistoph)
o Helpers funcs added to __phys_to_dma() & __dma_to_phys() (Christoph)
o Added warning when multiple offsets are needed and !DMA_PFN_OFFSET_MAP
o dev->dma_pfn_map => dev->dma_pfn_offset_map
o s/frm/from/ for dma_pfn_offset_frm_{phys,dma}_addr() (Christoph)
o In device.h: s/const void */const struct dma_pfn_offset_region */
o removed 'unlikely' from unlikely(dev->dma_pfn_offset_map) since
guarded by CONFIG_DMA_PFN_OFFSET_MAP (Christoph)
o Since dev->dma_pfn_offset is copied in usb/core/{usb,message}.c, now
dev->dma_pfn_offset_map is copied as well.
o Merged two of the DMA commits into one (Christoph).
Commit "arm: dma-mapping: Invoke dma offset func if needed":
o Use helper functions instead of #if CONFIG_DMA_PFN_OFFSET
Other commits' changes:
o Removed need for carrying of_id var in priv (Nicolas)
o Commit message rewordings (Bjorn)
o Commit log messages filled to 75 chars (Bjorn)
o devm_reset_control_get_shared())
=> devm_reset_control_get_optional_shared (Philipp)
o Add call to reset_control_assert() in PCIe remove routines (Philipp)
v1:
This patchset expands the usefulness of the Broadcom Settop Box PCIe
controller by building upon the PCIe driver used currently by the
Raspbery Pi. Other forms of this patchset were submitted by me years
ago and not accepted; the major sticking point was the code required
for the DMA remapping needed for the PCIe driver to work [1].
There have been many changes to the DMA and OF subsystems since that
time, making a cleaner and less intrusive patchset possible. This
patchset implements a generalization of "dev->dma_pfn_offset", except
that instead of a single scalar offset it provides for multiple
offsets via a function which depends upon the "dma-ranges" property of
the PCIe host controller. This is required for proper functionality
of the BrcmSTB PCIe controller and possibly some other devices.
[1]
https://lore.kernel.org/linux-arm-kernel/1516058925-46522-5-git-send-email-jim2101...@gmail.com/
Jim Quinlan (14):
PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB
ata: ahci_brcm: Fix use of BCM7216 reset controller
dt-bindings: PCI: Add bindings for more Brcmstb chips
PCI: brcmstb: Add bcm7278 reigister info
PCI: brcmstb: Add suspend and resume pm_ops
PCI: brcmstb: Add bcm7278 PERST support
PCI: brcmstb: Add control of rescal reset
of: Include a dev param in of_dma_get_range()
device core: Add ability to handle multiple dma offsets
arm: dma-mapping: Invoke dma offset func if needed
PCI: brcmstb: Set internal memory viewport sizes
PCI: brcmstb: Accommodate MSI for older chips
PCI: brcmstb: Set bus max burst size by chip type
PCI: brcmstb: Add bcm7211, bcm7216, bcm7445, bcm7278 to match list
.../bindings/pci/brcm,stb-pcie.yaml | 40 +-
arch/arm/include/asm/dma-mapping.h| 13 +-
drivers/ata/ahci_brcm.c | 14 +-
drivers/of/address.c | 69 ++-
drivers/of/device.c | 2 +-
drivers/of/of_private.h | 8 +-
drivers/pci/controller/Kconfig| 3 +-
drivers/pci/controller/pcie-brcmstb.c | 408 +++---
drivers/usb/core/message.c| 3 +
drivers/usb/core/usb.c| 3 +
include/linux/device.h| 10 +-
include/linux/dma-direct.h| 10 +-
include/linux/dma-mapping.h | 46 ++
kernel/dma/Kconfig| 13 +
14 files changed, 559 insertions(+), 83 deletions(-)
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 09/15] device core: Add ability to handle multiple dma offsets
Hi Nicolas,
On Wed, May 20, 2020 at 7:28 AM Nicolas Saenz Julienne
wrote:
>
> Hi Jim,
> thanks for having a go at this! My two cents.
>
> On Tue, 2020-05-19 at 16:34 -0400, Jim Quinlan wrote:
> > The device variable 'dma_pfn_offset' is used to do a single
> > linear map between cpu addrs and dma addrs. The variable
> > 'dma_map' is added to struct device to point to an array
> > of multiple offsets which is required for some devices.
> >
> > Signed-off-by: Jim Quinlan
> > ---
>
> [...]
>
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -493,6 +493,8 @@ struct dev_links_info {
> > * @bus_dma_limit: Limit of an upstream bridge or bus which imposes a
> > smaller
> > * DMA limit than the device itself supports.
> > * @dma_pfn_offset: offset of DMA memory range relatively of RAM
> > + * @dma_map: Like dma_pfn_offset but used when there are multiple
> > + * pfn offsets for multiple dma-ranges.
> > * @dma_parms: A low level driver may set these to teach IOMMU code
> > about
> > * segment limitations.
> > * @dma_pools: Dma pools (if dma'ble device).
> > @@ -578,7 +580,12 @@ struct device {
> >allocations such descriptors. */
> > u64 bus_dma_limit; /* upstream dma constraint */
> > unsigned long dma_pfn_offset;
> > -
> > +#ifdef CONFIG_DMA_PFN_OFFSET_MAP
> > + const void *dma_offset_map; /* Like dma_pfn_offset, but for
> > + * the unlikely case of multiple
> > + * offsets. If non-null,
> > dma_pfn_offset
> > + * will be 0. */
>
> I get a bad feeling about separating the DMA offset handling into two distinct
> variables. Albeit generally frowned upon, there is a fair amount of trickery
> around dev->dma_pfn_offset all over the kernel. usb_alloc_dev() comes to mind
> for example. And this obviously doesn't play well with it.
The trickery should only be present when
CONFIG_DMA_PFN_OFFSET_MAP=y**. Otherwise it does no harm. Also, I
feel that if dev-dma_pfn_offset is valid then so is
dev->dma_pfn_offset_map -- they both use the same mechanism in the
same places. I am merely
extending something that has been in Linux for a long time..
Further, I could have had dma_pfn_offset_map subsume dma_pfn_offset
but I wanted to leave it alone since folks would complain that it
would go from an addition to an if-clause and an inline function. But
if I did go that way there would only be one mechanism that would
cover both cases.
> I feel a potential
> solution to multiple DMA ranges should completely integrate with the current
> device DMA handling code, without special cases, on top of that, be
> transparent
> to the user.
Having dma_pfn_offset_map subsume dma_pfn_offset would integrate the
current code too. And I am not sure what you mean by being
"transparent to the user" -- the writer of the PCIe endpoint driver is
going to do some DMA calls and they have no idea if this mechanism is
in play or not.
>
> In more concrete terms, I'd repackage dev->bus_dma_limit and
> dev->dma_pfn_offset into a list/array of DMA range structures
This is sort of what I am doing except I defined my own structure.
Using the of_range structure would require one to do the same extra
calculations over and over for a DMA call; this is why I defined my
structure that has all of the needed precomputed variables.
> and adapt/create
> the relevant getter/setter functions so as for DMA users not to have to worry
> about the specifics of a device's DMA constraints.
I'm not sure I understand where these getter/setter functions would
exist or what they would do.
> editing dev->dma_pfn_offset, you'd be passing a DMA range structure to the
> device core, and let it take the relevant decisions on how to handle it
How and where would the device core operate for these getter/setters?
In how many places in the code? The way I see it, any solution has to
adjust the value when doing dma2phys and phys2dma conversions, and the
most efficient place to do that is in the two DMA header files (the
second one is for ARM).
> internally (overwrite, add a new entry, merge them, etc...).
I'm concerned that this would be overkill; I am just trying to get a
driver upstream for some baroque PCIe RC HW I'm not sure if we should
set up something elaborate when the demand is not there.
I'll be posting a v2. ChrisophH has sent me some personal feedback
which I am incorporating; so feel free to discuss your ideas with him
as well because I really want consensus on any large changes in
direction.
Thanks,
Jim
** CONFIG_DMA_OF_PFN_OFFSET_MAP=y only occurs when building for
ARCH_BRCMSTB. However, ARCH_BRCMSTB is set by the ARM64 defconfig and
the ARM multi_v7_defconfig, so it would be activated for those
defconfigs. This may(a) get us kicked out of those defconfigs or
(b) we may have to
Re: [PATCH 09/15] device core: Add ability to handle multiple dma offsets
Sorry, I meant to put you on the to-list for all patches. The last time I sent out this many patches using a collective cc-list for all patches I was told to reduce my cc-list. Jim On Wed, May 20, 2020 at 1:42 PM Christoph Hellwig wrote: > > If you don't Cc me on the whole series I have absolutely no way to > review it. Don't ever do these stupid partial Ccs as they cause nothing > but pain. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 09/15] device core: Add ability to handle multiple dma offsets
On Wed, May 20, 2020 at 10:03 AM Greg Kroah-Hartman <
gre...@linuxfoundation.org> wrote:
> On Wed, May 20, 2020 at 09:50:36AM -0400, Jim Quinlan wrote:
> > On Wed, May 20, 2020 at 1:43 AM Greg Kroah-Hartman
> > wrote:
> > >
> > > On Tue, May 19, 2020 at 04:34:07PM -0400, Jim Quinlan wrote:
> > > > diff --git a/include/linux/device.h b/include/linux/device.h
> > > > index ac8e37cd716a..6cd916860b5f 100644
> > > > --- a/include/linux/device.h
> > > > +++ b/include/linux/device.h
> > > > @@ -493,6 +493,8 @@ struct dev_links_info {
> > > > * @bus_dma_limit: Limit of an upstream bridge or bus which imposes
> a smaller
> > > > * DMA limit than the device itself supports.
> > > > * @dma_pfn_offset: offset of DMA memory range relatively of RAM
> > > > + * @dma_map: Like dma_pfn_offset but used when there are multiple
> > > > + * pfn offsets for multiple dma-ranges.
> > > > * @dma_parms: A low level driver may set these to teach
> IOMMU code about
> > > > * segment limitations.
> > > > * @dma_pools: Dma pools (if dma'ble device).
> > > > @@ -578,7 +580,12 @@ struct device {
> > > >allocations such
> descriptors. */
> > > > u64 bus_dma_limit; /* upstream dma constraint */
> > > > unsigned long dma_pfn_offset;
> > > > -
> > > > +#ifdef CONFIG_DMA_PFN_OFFSET_MAP
> > > > + const void *dma_offset_map; /* Like dma_pfn_offset, but for
> > > > + * the unlikely case of
> multiple
> > > > + * offsets. If non-null,
> dma_pfn_offset
> > > > + * will be 0. */
> > > > +#endif
> > > > struct device_dma_parameters *dma_parms;
> > > >
> > > > struct list_headdma_pools; /* dma pools (if
> dma'ble) */
> > >
> > > I'll defer to Christoph here, but I thought we were trying to get rid
> of
> > > stuff like this from struct device, not add new things to it for dma
> > Hi Greg,
> >
> > I wasn't aware of this policy. I put it in 'struct device' because
> > just like the existing dma_pfn_offset; it seemed to be the only way to
> > pull this off. I'll certainly follow any ideas on alternative
> > strategies from Christoph et al.
> >
> > > apis. And why is it a void *?
> > Just wanted to minimize the number of lines I've added to device.h, no
> > other reason.
>
> How would using a real type make this more lines? Never use a void *
> unless you have to, we want the compiler to check our errors for us :)
>
Got it; I did not need to define the struct in device.h. Will fix if this
code makes it past v1 :-)
Jim
> thanks,
>
> greg k-h
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 09/15] device core: Add ability to handle multiple dma offsets
On Wed, May 20, 2020 at 1:43 AM Greg Kroah-Hartman
wrote:
>
> On Tue, May 19, 2020 at 04:34:07PM -0400, Jim Quinlan wrote:
> > diff --git a/include/linux/device.h b/include/linux/device.h
> > index ac8e37cd716a..6cd916860b5f 100644
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -493,6 +493,8 @@ struct dev_links_info {
> > * @bus_dma_limit: Limit of an upstream bridge or bus which imposes a
> > smaller
> > * DMA limit than the device itself supports.
> > * @dma_pfn_offset: offset of DMA memory range relatively of RAM
> > + * @dma_map: Like dma_pfn_offset but used when there are multiple
> > + * pfn offsets for multiple dma-ranges.
> > * @dma_parms: A low level driver may set these to teach IOMMU code
> > about
> > * segment limitations.
> > * @dma_pools: Dma pools (if dma'ble device).
> > @@ -578,7 +580,12 @@ struct device {
> >allocations such descriptors. */
> > u64 bus_dma_limit; /* upstream dma constraint */
> > unsigned long dma_pfn_offset;
> > -
> > +#ifdef CONFIG_DMA_PFN_OFFSET_MAP
> > + const void *dma_offset_map; /* Like dma_pfn_offset, but for
> > + * the unlikely case of multiple
> > + * offsets. If non-null,
> > dma_pfn_offset
> > + * will be 0. */
> > +#endif
> > struct device_dma_parameters *dma_parms;
> >
> > struct list_headdma_pools; /* dma pools (if dma'ble) */
>
> I'll defer to Christoph here, but I thought we were trying to get rid of
> stuff like this from struct device, not add new things to it for dma
Hi Greg,
I wasn't aware of this policy. I put it in 'struct device' because
just like the existing dma_pfn_offset; it seemed to be the only way to
pull this off. I'll certainly follow any ideas on alternative
strategies from Christoph et al.
> apis. And why is it a void *?
Just wanted to minimize the number of lines I've added to device.h, no
other reason.
Thanks,
Jim
>
> thanks,
>
> greg k-h
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 09/15] device core: Add ability to handle multiple dma offsets
The device variable 'dma_pfn_offset' is used to do a single
linear map between cpu addrs and dma addrs. The variable
'dma_map' is added to struct device to point to an array
of multiple offsets which is required for some devices.
Signed-off-by: Jim Quinlan
---
drivers/of/address.c| 50 ++---
include/linux/device.h | 9 ++-
include/linux/dma-mapping.h | 44
kernel/dma/Kconfig | 12 +
4 files changed, 111 insertions(+), 4 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 96d8cfb14a60..7dfff618af6a 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -947,6 +947,8 @@ int of_dma_get_range(struct device *dev, struct device_node
*np, u64 *dma_addr,
struct of_range_parser parser;
struct of_range range;
u64 dma_start = U64_MAX, dma_end = 0, dma_offset = 0;
+ bool dma_multi_pfn_offset = false;
+ int num_ranges = 0;
while (node) {
ranges = of_get_property(node, "dma-ranges", );
@@ -977,10 +979,18 @@ int of_dma_get_range(struct device *dev, struct
device_node *np, u64 *dma_addr,
pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
range.bus_addr, range.cpu_addr, range.size);
+ num_ranges++;
if (dma_offset && range.cpu_addr - range.bus_addr !=
dma_offset) {
- pr_warn("Can't handle multiple dma-ranges with
different offsets on node(%pOF)\n", node);
- /* Don't error out as we'd break some existing DTs */
- continue;
+ dma_multi_pfn_offset = true;
+ if (!IS_ENABLED(CONFIG_DMA_PFN_OFFSET_MAP)) {
+ pr_warn("Can't handle multiple dma-ranges with
different offsets on node(%pOF)\n", node);
+ /*
+* Don't error out as we'd break some
+* existing DTs that are using configs
+* w/o CONFIG_DMA_PFN_OFFSET_MAP set.
+*/
+ continue;
+ }
}
dma_offset = range.cpu_addr - range.bus_addr;
@@ -991,6 +1001,40 @@ int of_dma_get_range(struct device *dev, struct
device_node *np, u64 *dma_addr,
dma_end = range.bus_addr + range.size;
}
+#ifdef CONFIG_DMA_PFN_OFFSET_MAP
+ if (dma_multi_pfn_offset) {
+ size_t r_size = (num_ranges + 1)
+ * sizeof(struct dma_pfn_offset_region);
+ struct dma_pfn_offset_region *r;
+
+ if (!dev)
+ return -EINVAL;
+
+ dma_offset = 0;
+ r = devm_kcalloc(dev, 1, r_size, GFP_KERNEL);
+ if (!r)
+ return -ENOMEM;
+ dev->dma_offset_map = (const void *) r;
+ of_dma_range_parser_init(, 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(, ) {
+ r->cpu_beg = range.cpu_addr;
+ r->cpu_end = r->cpu_beg + range.size;
+ r->dma_beg = range.bus_addr;
+ r->dma_end = r->dma_beg + range.size;
+ r->pfn_offset = PFN_DOWN(range.cpu_addr)
+ - PFN_DOWN(range.bus_addr);
+ r++;
+ }
+ }
+#endif
+
if (dma_start >= dma_end) {
ret = -EINVAL;
pr_debug("Invalid DMA ranges configuration on node(%pOF)\n",
diff --git a/include/linux/device.h b/include/linux/device.h
index ac8e37cd716a..6cd916860b5f 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -493,6 +493,8 @@ struct dev_links_info {
* @bus_dma_limit: Limit of an upstream bridge or bus which imposes a smaller
* DMA limit than the device itself supports.
* @dma_pfn_offset: offset of DMA memory range relatively of RAM
+ * @dma_map: Like dma_pfn_offset but used when there are multiple
+ * pfn offsets for multiple dma-ranges.
* @dma_parms: A low level driver may set these to teach IOMMU code about
* segment limitations.
* @dma_pools: Dma pools (if dma'ble device).
@@ -578,7 +580,12 @@ struct device {
allocations such descriptors. */
u64 bus_dma_limit; /* upstream dma constraint */
unsigned long dma_pfn_offset;
-
+#ifdef CONFIG_DMA_PFN_OFFSET_MAP
+ const void *dma_offset_map; /* Like
[PATCH 00/15] PCI: brcmstb: enable PCIe for STB chips
This patchset expands the usefulness of the Broadcom Settop Box PCIe controller by building upon the PCIe driver used currently by the Raspbery Pi. Other forms of this patchset were submitted by me years ago and not accepted; the major sticking point was the code required for the DMA remapping needed for the PCIe driver to work [1]. There have been many changes to the DMA and OF subsystems since that time, making a cleaner and less intrusive patchset possible. This patchset implements a generalization of "dev->dma_pfn_offset", except that instead of a single scalar offset it provides for multiple offsets via a function which depends upon the "dma-ranges" property of the PCIe host controller. This is required for proper functionality of the BrcmSTB PCIe controller and possibly some other devices. [1] https://lore.kernel.org/linux-arm-kernel/1516058925-46522-5-git-send-email-jim2101...@gmail.com/ Jim Quinlan (15): PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB ahci_brcm: fix use of BCM7216 reset controller dt-bindings: PCI: Add bindings for more Brcmstb chips PCI: brcmstb: Add compatibily of other chips PCI: brcmstb: Add suspend and resume pm_ops PCI: brcmstb: Asserting PERST is different for 7278 PCI: brcmstb: Add control of rescal reset of: Include a dev param in of_dma_get_range() device core: Add ability to handle multiple dma offsets dma-direct: Invoke dma offset func if needed arm: dma-mapping: Invoke dma offset func if needed PCI: brcmstb: Set internal memory viewport sizes PCI: brcmstb: Accommodate MSI for older chips PCI: brcmstb: Set bus max burst side by chip type PCI: brcmstb: add compatilbe chips to match list .../bindings/pci/brcm,stb-pcie.yaml | 40 +- arch/arm/include/asm/dma-mapping.h| 17 +- drivers/ata/ahci_brcm.c | 14 +- drivers/of/address.c | 54 ++- drivers/of/device.c | 2 +- drivers/of/of_private.h | 8 +- drivers/pci/controller/Kconfig| 4 +- drivers/pci/controller/pcie-brcmstb.c | 403 +++--- include/linux/device.h| 9 +- include/linux/dma-direct.h| 16 + include/linux/dma-mapping.h | 44 ++ kernel/dma/Kconfig| 12 + 12 files changed, 542 insertions(+), 81 deletions(-) -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 10/15] dma-direct: Invoke dma offset func if needed
Just like dma_pfn_offset, another offset is added to
the dma/phys translation if there happen to be multiple
regions that have different mapping offsets.
Signed-off-by: Jim Quinlan
---
include/linux/dma-direct.h | 16
1 file changed, 16 insertions(+)
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index 24b8684aa21d..825a773dbbc3 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -15,6 +15,14 @@ static inline dma_addr_t __phys_to_dma(struct device *dev,
phys_addr_t paddr)
{
dma_addr_t dev_addr = (dma_addr_t)paddr;
+#ifdef CONFIG_DMA_PFN_OFFSET_MAP
+ if (unlikely(dev->dma_offset_map)) {
+ unsigned long dma_pfn_offset = dma_pfn_offset_frm_phys_addr(
+ dev->dma_offset_map, paddr);
+
+ return dev_addr - ((dma_addr_t)dma_pfn_offset << PAGE_SHIFT);
+ }
+#endif
return dev_addr - ((dma_addr_t)dev->dma_pfn_offset << PAGE_SHIFT);
}
@@ -22,6 +30,14 @@ static inline phys_addr_t __dma_to_phys(struct device *dev,
dma_addr_t dev_addr)
{
phys_addr_t paddr = (phys_addr_t)dev_addr;
+#ifdef CONFIG_DMA_PFN_OFFSET_MAP
+ if (unlikely(dev->dma_offset_map)) {
+ unsigned long dma_pfn_offset = dma_pfn_offset_frm_dma_addr(
+ dev->dma_offset_map, dev_addr);
+
+ return paddr + ((phys_addr_t)dma_pfn_offset << PAGE_SHIFT);
+ }
+#endif
return paddr + ((phys_addr_t)dev->dma_pfn_offset << PAGE_SHIFT);
}
#endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
