Hello,
This series has been extracted from another series [1] adding support
for DMA operations in a NAND driver.
The reason I decided to post those patches separately is because they
are touching core stuff, and I'd like to have feedback on these specific
aspects.
The idea is to provide a generic function creating an sg_table from
a virtual pointer and a length. This operation is complicated by
the different memory regions exposed in kernel space. For example,
you have the lowmem region, which guarantees that buffers are
physically contiguous, while the vmalloc region does not.
sg_alloc_table_from_buf() detects in which memory region your buffer
reside, and takes the appropriate precautions when creating the
sg_table. This function also takes an extract parameter, allowing
one to specify extra constraints, like the maximum DMA segment size,
the required and the preferred alignment.
Patch 1 and 2 are implementing sg_alloc_table_from_buf() (patch 1
is needed to properly detect buffers residing in the highmem/kmap
area).
Patch 3 is making use of sg_alloc_table_from_buf() in the spi_map_buf()
function (hopefully, other subsystems/drivers will be able to easily
switch to this function too).
Patch 4 is implementing what I really need: generic functions
to map/unmap a virtual buffer passed through mtd->_read/_write().
I'm not exactly a DMA or MM experts, so that would be great to have
feedbacks on this approach. That's why I added so many people in Cc
even if they're not directly impacted by those patches. Let me know if
you want me to drop/add people from/to the recipient list.
Thanks.
Best Regards,
Boris
[1]http://www.spinics.net/lists/arm-kernel/msg493552.html
Boris Brezillon (4):
mm: add is_highmem_addr() helper
scatterlist: add sg_alloc_table_from_buf() helper
spi: use sg_alloc_table_from_buf()
mtd: provide helper to prepare buffers for DMA operations
drivers/mtd/mtdcore.c | 66 ++++++++++++++++
drivers/spi/spi.c | 45 ++---------
include/linux/highmem.h | 13 ++++
include/linux/mtd/mtd.h | 25 ++++++
include/linux/scatterlist.h | 24 ++++++
lib/scatterlist.c | 183 ++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 316 insertions(+), 40 deletions(-)
--
2.5.0
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Add an helper to check if a virtual address is in the highmem region.
Signed-off-by: Boris Brezillon <[email protected]>
---
include/linux/highmem.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index bb3f329..13dff37 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -41,6 +41,14 @@ void kmap_flush_unused(void);
struct page *kmap_to_page(void *addr);
+static inline bool is_highmem_addr(const void *x)
+{
+ unsigned long vaddr = (unsigned long)x;
+
+ return vaddr >= PKMAP_BASE &&
+ vaddr < ((PKMAP_BASE + LAST_PKMAP) * PAGE_SIZE);
+}
+
#else /* CONFIG_HIGHMEM */
static inline unsigned int nr_free_highpages(void) { return 0; }
@@ -50,6 +58,11 @@ static inline struct page *kmap_to_page(void *addr)
return virt_to_page(addr);
}
+static inline bool is_highmem_addr(const void *x)
+{
+ return false;
+}
+
#define totalhigh_pages 0UL
#ifndef ARCH_HAS_KMAP
--
2.5.0
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Replace custom implementation of sg_alloc_table_from_buf() by a call to
sg_alloc_table_from_buf().
Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/spi/spi.c | 45 +++++----------------------------------------
1 file changed, 5 insertions(+), 40 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index de2f2f9..eed461d 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -705,49 +705,14 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
struct sg_table *sgt, void *buf, size_t len,
enum dma_data_direction dir)
{
- const bool vmalloced_buf = is_vmalloc_addr(buf);
- unsigned int max_seg_size = dma_get_max_seg_size(dev);
- int desc_len;
- int sgs;
- struct page *vm_page;
- void *sg_buf;
- size_t min;
- int i, ret;
-
- if (vmalloced_buf) {
- desc_len = min_t(int, max_seg_size, PAGE_SIZE);
- sgs = DIV_ROUND_UP(len + offset_in_page(buf), desc_len);
- } else {
- desc_len = min_t(int, max_seg_size, master->max_dma_len);
- sgs = DIV_ROUND_UP(len, desc_len);
- }
+ struct sg_constraints constraints = { };
+ int ret;
- ret = sg_alloc_table(sgt, sgs, GFP_KERNEL);
- if (ret != 0)
+ constraints.max_segment_size = dma_get_max_seg_size(dev);
+ ret = sg_alloc_table_from_buf(sgt, buf, len, &constraints, GFP_KERNEL);
+ if (ret)
return ret;
- for (i = 0; i < sgs; i++) {
-
- if (vmalloced_buf) {
- min = min_t(size_t,
- len, desc_len - offset_in_page(buf));
- vm_page = vmalloc_to_page(buf);
- if (!vm_page) {
- sg_free_table(sgt);
- return -ENOMEM;
- }
- sg_set_page(&sgt->sgl[i], vm_page,
- min, offset_in_page(buf));
- } else {
- min = min_t(size_t, len, desc_len);
- sg_buf = buf;
- sg_set_buf(&sgt->sgl[i], sg_buf, min);
- }
-
- buf += min;
- len -= min;
- }
-
ret = dma_map_sg(dev, sgt->sgl, sgt->nents, dir);
if (!ret)
ret = -ENOMEM;
--
2.5.0
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Some NAND controller drivers are making use of DMA to transfer data from
the controller to the buffer passed by the MTD user.
Provide a generic mtd_map/unmap_buf() implementation to avoid open coded
(and sometime erroneous) implementations.
Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/mtdcore.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/mtd/mtd.h | 25 +++++++++++++++++++
2 files changed, 91 insertions(+)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 3096251..4c20f33 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -1253,6 +1253,72 @@ void *mtd_kmalloc_up_to(const struct mtd_info *mtd, size_t *size)
}
EXPORT_SYMBOL_GPL(mtd_kmalloc_up_to);
+#ifdef CONFIG_HAS_DMA
+/**
+ * mtd_map_buf - create an SG table and prepare it for DMA operations
+ *
+ * @mtd: mtd device description object pointer
+ * @dev: device handling the DMA operation
+ * @buf: buf used to create the SG table
+ * @len: length of buf
+ * @constraints: optional constraints to take into account when creating
+ * the SG table. Can be NULL if no specific constraints
+ * are required.
+ * @dir: direction of the DMA operation
+ *
+ * This function should be used when an MTD driver wants to do DMA operations
+ * on a buffer passed by the MTD layer. This functions takes care of
+ * vmallocated buffer constraints, and return and sg_table that you can safely
+ * use.
+ */
+int mtd_map_buf(struct mtd_info *mtd, struct device *dev,
+ struct sg_table *sgt, const void *buf, size_t len,
+ const struct sg_constraints *constraints,
+ enum dma_data_direction dir)
+{
+ int ret;
+
+ ret = sg_alloc_table_from_buf(sgt, buf, len, constraints, GFP_KERNEL);
+ if (ret)
+ return ret;
+
+ ret = dma_map_sg(dev, sgt->sgl, sgt->nents, dir);
+ if (!ret)
+ ret = -ENOMEM;
+
+ if (ret < 0) {
+ sg_free_table(sgt);
+ return ret;
+ }
+
+ sgt->nents = ret;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(mtd_map_buf);
+
+/**
+ * mtd_unmap_buf - unmap an SG table and release its resources
+ *
+ * @mtd: mtd device description object pointer
+ * @dev: device handling the DMA operation
+ * @sgt: SG table
+ * @dir: direction of the DMA operation
+ *
+ * This function unmaps a previously mapped SG table and release SG table
+ * resources. Should be called when your DMA operation is done.
+ */
+void mtd_unmap_buf(struct mtd_info *mtd, struct device *dev,
+ struct sg_table *sgt, enum dma_data_direction dir)
+{
+ if (sgt->orig_nents) {
+ dma_unmap_sg(dev, sgt->sgl, sgt->orig_nents, dir);
+ sg_free_table(sgt);
+ }
+}
+EXPORT_SYMBOL_GPL(mtd_unmap_buf);
+#endif /* !CONFIG_HAS_DMA */
+
#ifdef CONFIG_PROC_FS
/*====================================================================*/
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 7712721..15cff85 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -24,6 +24,7 @@
#include <linux/uio.h>
#include <linux/notifier.h>
#include <linux/device.h>
+#include <linux/dma-mapping.h>
#include <mtd/mtd-abi.h>
@@ -410,6 +411,30 @@ extern void register_mtd_user (struct mtd_notifier *new);
extern int unregister_mtd_user (struct mtd_notifier *old);
void *mtd_kmalloc_up_to(const struct mtd_info *mtd, size_t *size);
+#ifdef CONFIG_HAS_DMA
+int mtd_map_buf(struct mtd_info *mtd, struct device *dev,
+ struct sg_table *sgt, const void *buf, size_t len,
+ const struct sg_constraints *constraints,
+ enum dma_data_direction dir);
+void mtd_unmap_buf(struct mtd_info *mtd, struct device *dev,
+ struct sg_table *sgt, enum dma_data_direction dir);
+#else
+static inline int mtd_map_buf(struct mtd_info *mtd, struct device *dev,
+ struct sg_table *sgt, const void *buf,
+ size_t len,
+ const struct sg_constraints *constraints
+ enum dma_data_direction dir)
+{
+ return -ENOTSUPP;
+}
+
+static void mtd_unmap_buf(struct mtd_info *mtd, struct device *dev,
+ struct sg_table *sgt, enum dma_data_direction dir)
+{
+ return -ENOTSUPP;
+}
+#endif
+
void mtd_erase_callback(struct erase_info *instr);
static inline int mtd_is_bitflip(int err) {
--
2.5.0
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
On Thu, Mar 31, 2016 at 02:29:43PM +0200, Boris Brezillon wrote:
> Replace custom implementation of sg_alloc_table_from_buf() by a call to
> sg_alloc_table_from_buf().
Acked-by: Mark Brown <[email protected]>
Hi Boris,
[auto build test ERROR on spi/for-next]
[also build test ERROR on v4.6-rc1 next-20160331]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Boris-Brezillon/scatterlist-sg_table-from-virtual-pointer/20160331-203118
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi for-next
config: m32r-m32104ut_defconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m32r
All error/warnings (new ones prefixed by >>):
In file included from include/linux/mtd/super.h:17:0,
from fs/romfs/storage.c:13:
>> include/linux/mtd/mtd.h:426:10: error: expected ';', ',' or ')' before 'enum'
enum dma_data_direction dir)
^
include/linux/mtd/mtd.h: In function 'mtd_unmap_buf':
>> include/linux/mtd/mtd.h:434:2: warning: 'return' with a value, in function returning void
return -ENOTSUPP;
^
fs/romfs/storage.c: At top level:
include/linux/mtd/mtd.h:431:13: warning: 'mtd_unmap_buf' defined but not used [-Wunused-function]
static void mtd_unmap_buf(struct mtd_info *mtd, struct device *dev,
^
--
In file included from include/linux/mtd/super.h:17:0,
from fs/romfs/super.c:72:
>> include/linux/mtd/mtd.h:426:10: error: expected ';', ',' or ')' before 'enum'
enum dma_data_direction dir)
^
include/linux/mtd/mtd.h: In function 'mtd_unmap_buf':
>> include/linux/mtd/mtd.h:434:2: warning: 'return' with a value, in function returning void
return -ENOTSUPP;
^
fs/romfs/super.c: At top level:
include/linux/mtd/mtd.h:431:13: warning: 'mtd_unmap_buf' defined but not used [-Wunused-function]
static void mtd_unmap_buf(struct mtd_info *mtd, struct device *dev,
^
vim +426 include/linux/mtd/mtd.h
420 struct sg_table *sgt, enum dma_data_direction dir);
421 #else
422 static inline int mtd_map_buf(struct mtd_info *mtd, struct device *dev,
423 struct sg_table *sgt, const void *buf,
424 size_t len,
425 const struct sg_constraints *constraints
> 426 enum dma_data_direction dir)
427 {
428 return -ENOTSUPP;
429 }
430
431 static void mtd_unmap_buf(struct mtd_info *mtd, struct device *dev,
432 struct sg_table *sgt, enum dma_data_direction dir)
433 {
> 434 return -ENOTSUPP;
435 }
436 #endif
437
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi,
On 03/31/2016 05:59 PM, Boris Brezillon wrote:
> Add an helper to check if a virtual address is in the highmem region.
>
> Signed-off-by: Boris Brezillon <[email protected]>
> ---
> include/linux/highmem.h | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> index bb3f329..13dff37 100644
> --- a/include/linux/highmem.h
> +++ b/include/linux/highmem.h
> @@ -41,6 +41,14 @@ void kmap_flush_unused(void);
>
> struct page *kmap_to_page(void *addr);
>
> +static inline bool is_highmem_addr(const void *x)
> +{
> + unsigned long vaddr = (unsigned long)x;
> +
> + return vaddr >= PKMAP_BASE &&
> + vaddr < ((PKMAP_BASE + LAST_PKMAP) * PAGE_SIZE);
Shouldn't this be:
vaddr < (PKMAP_BASE + (LAST_PKMAP * PAGE_SIZE)) ?
--
Regards
Vignesh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
On Mon, 4 Apr 2016 13:44:11 +0530
Vignesh R <[email protected]> wrote:
> Hi,
>
> On 03/31/2016 05:59 PM, Boris Brezillon wrote:
> > Add an helper to check if a virtual address is in the highmem region.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/[email protected]>
> > ---
> > include/linux/highmem.h | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> > index bb3f329..13dff37 100644
> > --- a/include/linux/highmem.h
> > +++ b/include/linux/highmem.h
> > @@ -41,6 +41,14 @@ void kmap_flush_unused(void);
> >
> > struct page *kmap_to_page(void *addr);
> >
> > +static inline bool is_highmem_addr(const void *x)
> > +{
> > + unsigned long vaddr = (unsigned long)x;
> > +
> > + return vaddr >= PKMAP_BASE &&
> > + vaddr < ((PKMAP_BASE + LAST_PKMAP) * PAGE_SIZE);
>
>
> Shouldn't this be:
> vaddr < (PKMAP_BASE + (LAST_PKMAP * PAGE_SIZE)) ?
Oops, yes indeed.
Anyway, given Russell's feedback I don't think I'm gonna follow up on
this series.
Sorry.
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com