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? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
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
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
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. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
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
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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
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
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
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 (11):
PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB
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-mapping: Introduce DMA range map, supplanting dma_pfn_offset
PCI: brcmstb: Set additional internal memory DMA 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
[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 -
-
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
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RESEND v10 00/11] PCI: brcmstb: enable PCIe for STB chips
ch_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 (11):
PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB
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-mapping: Introduce DMA range map, supplanting dma_pfn_offset
PCI: brcmstb: Set additional internal memory DMA 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 | 56 ++-
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
[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_
[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_
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
[PATCH v10 00/11] PCI: brcmstb: enable PCIe for STB chips
ch_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 (11):
PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB
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-mapping: Introduce DMA range map, supplanting dma_pfn_offset
PCI: brcmstb: Set additional internal memory DMA 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 | 56 ++-
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
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
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
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. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
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.
>
>
>
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 00/12] PCI: brcmstb: enable PCIe for STB chips
at 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: Introduce DMA range map, supplanting dma_pfn_offset
PCI: brcmstb: Set additional internal memory DMA 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 | 56 ++-
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/ata/ahci_brcm.c | 11 +-
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/Kconfig
[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_
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 __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_off
[PATCH v8 00/12] PCI: brcmstb: enable PCIe for STB chips
e 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: Introduce DMA range map, supplanting dma_pfn_offset
PCI: brcmstb: Set additional internal memory DMA 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 | 56 ++-
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/ata/ahci_brcm.c | 11 +-
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/pci/controller/Kconfig| 3 +-
drivers/pci/controller/pcie-brcmstb.c | 408 +++---
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 +++
28 files changed, 683 insertions(+), 197 deletions(-)
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
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.. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 00/12] PCI: brcmstb: enable PCIe for STB chips
ma_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: Introduce DMA range map, supplanting dma_pfn_offset
PCI: brcmstb: Set additional internal memory DMA 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 | 56 ++-
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/ata/ahci_brcm.c | 11 +-
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/pci/controller/Kconfig| 3 +-
drivers/pci/controller/pcie-brcmstb.c | 408 +++---
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 +++
28 files changed, 682 insertions(+), 197 deletions(-)
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[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_off
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 > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[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 -
-
[PATCH v6 00/12] PCI: brcmstb: enable PCIe for STB chips
et_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: Introduce DMA range map, supplanting dma_pfn_offset PCI: brcmstb: Set additional internal memory DMA 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 | 56 ++- 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/ata/ahci_brcm.c | 11 +- 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/pci/controller/Kconfig| 3 +- drivers/pci/controller/pcie-brcmstb.c | 408 +++--- 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 +++ 27 files changed, 684 insertions(+), 195 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
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 s
[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 orde
[PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips
ver 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: 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 | 56 ++- 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/ata/ahci_brcm.c | 11 +- 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/pci/controller/Kconfig| 3 +- drivers/pci/controller/pcie-brcmstb.c | 403 +++--- 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 ++ 27 files changed, 632 insertions(+), 132 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
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 > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
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 > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[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: A
[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, *r
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
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
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 > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
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
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-
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.
> >
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 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[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
[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);
+
