2019-05-29 10:31:36

by Laurentiu Tudor

[permalink] [raw]
Subject: [PATCH v7 0/5] prerequisites for device reserved local mem rework

From: Laurentiu Tudor <[email protected]>

For HCs that have local memory, replace the current DMA API usage
with a genalloc generic allocator to manage the mappings for these
devices.
This is in preparation for dropping the existing "coherent" dma
mem declaration APIs. Current implementation was relying on a short
circuit in the DMA API that in the end, was acting as an allocator
for these type of devices.

Only compiled tested, so any volunteers willing to test are most welcome.

Thank you!

For context, see thread here: https://lkml.org/lkml/2019/4/22/357

Changes in v7:
- drop useless __iomem annotation to fix sparse warning
- select GENERIC_ALLOCATOR to fix compilation on sh arch

Changes in v6:
- drop some unneeded initializations (Alan)
- use device name for genpool instead of misleading "ohci-hcd" (Alan)
- updated some comments (Alan, Fredrik)
- added Tested-By tags

Changes in v5:
- updated first patch to preserve bisectability (Christoph, Greg)
- fixed a few more places where dma api was still being
used (e.g. td_alloc, ed_alloc) (Fredrik)
- included patch from Fredrik adding gen_pool_dma_zalloc() api
- added patch that drops HCD_LOCAL_MEM altogether (Greg)
- set td_cache / ed_cache to null for devices with local mem (Fredrik)
- introduce usb_hcd_setup_local_mem() that sets up the genalloc
pool for drivers and updated drivers to use it

Changes in v4:
- created mapping for local mem
- fix genalloc misuse

Changes in v3:
- rearranged calls into genalloc simplifying the code a bit (Christoph)
- dropped RFC prefix

Changes in v2:
- use genalloc also in core usb (based on comment from Robin)
- moved genpool decl to usb_hcd to be accesible from core usb

Fredrik Noring (1):
lib/genalloc.c: Add gen_pool_dma_zalloc() for zeroed DMA allocations

Laurentiu Tudor (4):
USB: use genalloc for USB HCs with local memory
usb: host: ohci-sm501: init genalloc for local memory
usb: host: ohci-tmio: init genalloc for local memory
USB: drop HCD_LOCAL_MEM flag

drivers/usb/Kconfig | 1 +
drivers/usb/core/buffer.c | 17 ++++++++----
drivers/usb/core/hcd.c | 51 ++++++++++++++++++++++++++++------
drivers/usb/host/ehci-hcd.c | 2 +-
drivers/usb/host/fotg210-hcd.c | 2 +-
drivers/usb/host/ohci-hcd.c | 25 +++++++++++++----
drivers/usb/host/ohci-mem.c | 35 ++++++++++++++++++++---
drivers/usb/host/ohci-sm501.c | 50 +++++++++++++++------------------
drivers/usb/host/ohci-tmio.c | 15 ++++------
drivers/usb/host/ohci.h | 2 ++
drivers/usb/host/uhci-hcd.c | 2 +-
include/linux/genalloc.h | 1 +
include/linux/usb/hcd.h | 6 +++-
lib/genalloc.c | 29 ++++++++++++++++++-
14 files changed, 172 insertions(+), 66 deletions(-)

--
2.17.1


2019-05-29 10:31:38

by Laurentiu Tudor

[permalink] [raw]
Subject: [PATCH v7 5/5] USB: drop HCD_LOCAL_MEM flag

From: Laurentiu Tudor <[email protected]>

With the addition of the local memory allocator, the HCD_LOCAL_MEM
flag can be dropped and the checks against it replaced with a check
for the localmem_pool ptr being initialized.

Signed-off-by: Laurentiu Tudor <[email protected]>
Tested-by: Fredrik Noring <[email protected]>
---
drivers/usb/core/buffer.c | 8 +++-----
drivers/usb/core/hcd.c | 15 ++++++---------
drivers/usb/host/ehci-hcd.c | 2 +-
drivers/usb/host/fotg210-hcd.c | 2 +-
drivers/usb/host/ohci-hcd.c | 2 +-
drivers/usb/host/ohci-sm501.c | 5 +++--
drivers/usb/host/ohci-tmio.c | 2 +-
drivers/usb/host/uhci-hcd.c | 2 +-
include/linux/usb/hcd.h | 1 -
9 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index d2064ad7ad14..1359b78a624e 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -68,7 +68,7 @@ int hcd_buffer_create(struct usb_hcd *hcd)

if (!IS_ENABLED(CONFIG_HAS_DMA) ||
(!is_device_dma_capable(hcd->self.sysdev) &&
- !(hcd->driver->flags & HCD_LOCAL_MEM)))
+ !hcd->localmem_pool))
return 0;

for (i = 0; i < HCD_BUFFER_POOLS; i++) {
@@ -130,8 +130,7 @@ void *hcd_buffer_alloc(

/* some USB hosts just use PIO */
if (!IS_ENABLED(CONFIG_HAS_DMA) ||
- (!is_device_dma_capable(bus->sysdev) &&
- !(hcd->driver->flags & HCD_LOCAL_MEM))) {
+ !is_device_dma_capable(bus->sysdev)) {
*dma = ~(dma_addr_t) 0;
return kmalloc(size, mem_flags);
}
@@ -162,8 +161,7 @@ void hcd_buffer_free(
}

if (!IS_ENABLED(CONFIG_HAS_DMA) ||
- (!is_device_dma_capable(bus->sysdev) &&
- !(hcd->driver->flags & HCD_LOCAL_MEM))) {
+ !is_device_dma_capable(bus->sysdev)) {
kfree(addr);
return;
}
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 29b96e5e8621..fe631d18c1ed 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1347,14 +1347,14 @@ EXPORT_SYMBOL_GPL(usb_hcd_unlink_urb_from_ep);
* using regular system memory - like pci devices doing bus mastering.
*
* To support host controllers with limited dma capabilities we provide dma
- * bounce buffers. This feature can be enabled using the HCD_LOCAL_MEM flag.
+ * bounce buffers. This feature can be enabled by initializing
+ * hcd->localmem_pool using usb_hcd_setup_local_mem().
* For this to work properly the host controller code must first use the
* function dma_declare_coherent_memory() to point out which memory area
* that should be used for dma allocations.
*
- * The HCD_LOCAL_MEM flag then tells the usb code to allocate all data for
- * dma using dma_alloc_coherent() which in turn allocates from the memory
- * area pointed out with dma_declare_coherent_memory().
+ * The initialized hcd->localmem_pool then tells the usb code to allocate all
+ * data for dma using the genalloc API.
*
* So, to summarize...
*
@@ -1364,9 +1364,6 @@ EXPORT_SYMBOL_GPL(usb_hcd_unlink_urb_from_ep);
* (a) "normal" kernel memory is no good, and
* (b) there's not enough to share
*
- * - The only *portable* hook for such stuff in the
- * DMA framework is dma_declare_coherent_memory()
- *
* - So we use that, even though the primary requirement
* is that the memory be "local" (hence addressable
* by that device), not "coherent".
@@ -1533,7 +1530,7 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
urb->setup_dma))
return -EAGAIN;
urb->transfer_flags |= URB_SETUP_MAP_SINGLE;
- } else if (hcd->driver->flags & HCD_LOCAL_MEM) {
+ } else if (hcd->localmem_pool) {
ret = hcd_alloc_coherent(
urb->dev->bus, mem_flags,
&urb->setup_dma,
@@ -1603,7 +1600,7 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
else
urb->transfer_flags |= URB_DMA_MAP_SINGLE;
}
- } else if (hcd->driver->flags & HCD_LOCAL_MEM) {
+ } else if (hcd->localmem_pool) {
ret = hcd_alloc_coherent(
urb->dev->bus, mem_flags,
&urb->transfer_dma,
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index cdafa97f632d..9da7e22848c9 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -559,7 +559,7 @@ static int ehci_init(struct usb_hcd *hcd)
ehci->command = temp;

/* Accept arbitrarily long scatter-gather lists */
- if (!(hcd->driver->flags & HCD_LOCAL_MEM))
+ if (!hcd->localmem_pool)
hcd->self.sg_tablesize = ~0;

/* Prepare for unlinking active QHs */
diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
index 0da68df259c8..5d74ff61fa4c 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -4995,7 +4995,7 @@ static int hcd_fotg210_init(struct usb_hcd *hcd)
fotg210->command = temp;

/* Accept arbitrarily long scatter-gather lists */
- if (!(hcd->driver->flags & HCD_LOCAL_MEM))
+ if (!hcd->localmem_pool)
hcd->self.sg_tablesize = ~0;
return 0;
}
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index b200b19b44fa..5801858d867e 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -448,7 +448,7 @@ static int ohci_init (struct ohci_hcd *ohci)
struct usb_hcd *hcd = ohci_to_hcd(ohci);

/* Accept arbitrarily long scatter-gather lists */
- if (!(hcd->driver->flags & HCD_LOCAL_MEM))
+ if (!hcd->localmem_pool)
hcd->self.sg_tablesize = ~0;

if (distrust_firmware)
diff --git a/drivers/usb/host/ohci-sm501.c b/drivers/usb/host/ohci-sm501.c
index b710e100aec9..c158cda9e4b9 100644
--- a/drivers/usb/host/ohci-sm501.c
+++ b/drivers/usb/host/ohci-sm501.c
@@ -49,7 +49,7 @@ static const struct hc_driver ohci_sm501_hc_driver = {
* generic hardware linkage
*/
.irq = ohci_irq,
- .flags = HCD_USB11 | HCD_MEMORY | HCD_LOCAL_MEM,
+ .flags = HCD_USB11 | HCD_MEMORY,

/*
* basic lifecycle operations
@@ -153,7 +153,8 @@ static int ohci_hcd_sm501_drv_probe(struct platform_device *pdev)
* fine. This is however not always the case - buffers may be allocated
* using kmalloc() - so the usb core needs to be told that it must copy
* data into our local memory if the buffers happen to be placed in
- * regular memory. The HCD_LOCAL_MEM flag does just that.
+ * regular memory. A non-null hcd->localmem_pool initialized by the
+ * the call to usb_hcd_setup_local_mem() below does just that.
*/

if (usb_hcd_setup_local_mem(hcd, mem->start,
diff --git a/drivers/usb/host/ohci-tmio.c b/drivers/usb/host/ohci-tmio.c
index 3b84ce0c3f29..d5a293a707b6 100644
--- a/drivers/usb/host/ohci-tmio.c
+++ b/drivers/usb/host/ohci-tmio.c
@@ -153,7 +153,7 @@ static const struct hc_driver ohci_tmio_hc_driver = {

/* generic hardware linkage */
.irq = ohci_irq,
- .flags = HCD_USB11 | HCD_MEMORY | HCD_LOCAL_MEM,
+ .flags = HCD_USB11 | HCD_MEMORY,

/* basic lifecycle operations */
.start = ohci_tmio_start,
diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
index 98deb5f64268..03bc59755123 100644
--- a/drivers/usb/host/uhci-hcd.c
+++ b/drivers/usb/host/uhci-hcd.c
@@ -581,7 +581,7 @@ static int uhci_start(struct usb_hcd *hcd)

hcd->uses_new_polling = 1;
/* Accept arbitrarily long scatter-gather lists */
- if (!(hcd->driver->flags & HCD_LOCAL_MEM))
+ if (!hcd->localmem_pool)
hcd->self.sg_tablesize = ~0;

spin_lock_init(&uhci->lock);
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 127560a4bfa0..bab27ccc8ff5 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -256,7 +256,6 @@ struct hc_driver {

int flags;
#define HCD_MEMORY 0x0001 /* HC regs use memory (else I/O) */
-#define HCD_LOCAL_MEM 0x0002 /* HC needs local memory */
#define HCD_SHARED 0x0004 /* Two (or more) usb_hcds share HW */
#define HCD_USB11 0x0010 /* USB 1.1 */
#define HCD_USB2 0x0020 /* USB 2.0 */
--
2.17.1

2019-05-29 10:31:52

by Laurentiu Tudor

[permalink] [raw]
Subject: [PATCH v7 1/5] lib/genalloc.c: Add gen_pool_dma_zalloc() for zeroed DMA allocations

From: Fredrik Noring <[email protected]>

gen_pool_dma_zalloc() is a zeroed memory variant of gen_pool_dma_alloc().
Document return values of both, and indicate NULL as a "%NULL" constant.

Signed-off-by: Fredrik Noring <[email protected]>
Tested-by: Fredrik Noring <[email protected]>
---
include/linux/genalloc.h | 1 +
lib/genalloc.c | 29 ++++++++++++++++++++++++++++-
2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
index dd0a452373e7..6c62eeca754f 100644
--- a/include/linux/genalloc.h
+++ b/include/linux/genalloc.h
@@ -121,6 +121,7 @@ extern unsigned long gen_pool_alloc_algo(struct gen_pool *, size_t,
genpool_algo_t algo, void *data);
extern void *gen_pool_dma_alloc(struct gen_pool *pool, size_t size,
dma_addr_t *dma);
+void *gen_pool_dma_zalloc(struct gen_pool *pool, size_t size, dma_addr_t *dma);
extern void gen_pool_free(struct gen_pool *, unsigned long, size_t);
extern void gen_pool_for_each_chunk(struct gen_pool *,
void (*)(struct gen_pool *, struct gen_pool_chunk *, void *), void *);
diff --git a/lib/genalloc.c b/lib/genalloc.c
index 7e85d1e37a6e..5db43476a19b 100644
--- a/lib/genalloc.c
+++ b/lib/genalloc.c
@@ -337,12 +337,14 @@ EXPORT_SYMBOL(gen_pool_alloc_algo);
* gen_pool_dma_alloc - allocate special memory from the pool for DMA usage
* @pool: pool to allocate from
* @size: number of bytes to allocate from the pool
- * @dma: dma-view physical address return value. Use NULL if unneeded.
+ * @dma: dma-view physical address return value. Use %NULL if unneeded.
*
* Allocate the requested number of bytes from the specified pool.
* Uses the pool allocation function (with first-fit algorithm by default).
* Can not be used in NMI handler on architectures without
* NMI-safe cmpxchg implementation.
+ *
+ * Return: virtual address of the allocated memory, or %NULL on failure
*/
void *gen_pool_dma_alloc(struct gen_pool *pool, size_t size, dma_addr_t *dma)
{
@@ -362,6 +364,31 @@ void *gen_pool_dma_alloc(struct gen_pool *pool, size_t size, dma_addr_t *dma)
}
EXPORT_SYMBOL(gen_pool_dma_alloc);

+/**
+ * gen_pool_dma_zalloc - allocate special zeroed memory from the pool for
+ * DMA usage
+ * @pool: pool to allocate from
+ * @size: number of bytes to allocate from the pool
+ * @dma: dma-view physical address return value. Use %NULL if unneeded.
+ *
+ * Allocate the requested number of zeroed bytes from the specified pool.
+ * Uses the pool allocation function (with first-fit algorithm by default).
+ * Can not be used in NMI handler on architectures without
+ * NMI-safe cmpxchg implementation.
+ *
+ * Return: virtual address of the allocated zeroed memory, or %NULL on failure
+ */
+void *gen_pool_dma_zalloc(struct gen_pool *pool, size_t size, dma_addr_t *dma)
+{
+ void *vaddr = gen_pool_dma_alloc(pool, size, dma);
+
+ if (vaddr)
+ memset(vaddr, 0, size);
+
+ return vaddr;
+}
+EXPORT_SYMBOL(gen_pool_dma_zalloc);
+
/**
* gen_pool_free - free allocated special memory back to the pool
* @pool: pool to free to
--
2.17.1

2019-05-29 10:32:02

by Laurentiu Tudor

[permalink] [raw]
Subject: [PATCH v7 2/5] USB: use genalloc for USB HCs with local memory

From: Laurentiu Tudor <[email protected]>

For HCs that have local memory, replace the current DMA API usage
with a genalloc generic allocator to manage the mappings for these
devices. To help users, introduce a new HCD API,
usb_hcd_setup_local_mem() that will setup up the genalloc backing
up the device local memory. It will be used in subsequent patches.
This is in preparation for dropping the existing "coherent" dma
mem declaration APIs. Current implementation was relying on a short
circuit in the DMA API that in the end, was acting as an allocator
for these type of devices.
For sh arch to compile GENERIC_ALLOCATOR needs to be selected in
Kconfig.

For context, see thread here: https://lkml.org/lkml/2019/4/22/357

Signed-off-by: Laurentiu Tudor <[email protected]>
Signed-off-by: Fredrik Noring <[email protected]>
Tested-by: Fredrik Noring <[email protected]>
Reported-by: kbuild test robot <[email protected]>
---
drivers/usb/Kconfig | 1 +
drivers/usb/core/buffer.c | 9 +++++++++
drivers/usb/core/hcd.c | 36 ++++++++++++++++++++++++++++++++++++
drivers/usb/host/ohci-hcd.c | 23 ++++++++++++++++++-----
drivers/usb/host/ohci-mem.c | 35 +++++++++++++++++++++++++++++++----
drivers/usb/host/ohci.h | 2 ++
include/linux/usb/hcd.h | 5 +++++
7 files changed, 102 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index e4b27413f528..389c57d8eba7 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -45,6 +45,7 @@ config USB_ARCH_HAS_HCD
config USB
tristate "Support for Host-side USB"
depends on USB_ARCH_HAS_HCD
+ select GENERIC_ALLOCATOR
select USB_COMMON
select NLS # for UTF-8 strings
---help---
diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index f641342cdec0..d2064ad7ad14 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -16,6 +16,7 @@
#include <linux/io.h>
#include <linux/dma-mapping.h>
#include <linux/dmapool.h>
+#include <linux/genalloc.h>
#include <linux/usb.h>
#include <linux/usb/hcd.h>

@@ -124,6 +125,9 @@ void *hcd_buffer_alloc(
if (size == 0)
return NULL;

+ if (hcd->localmem_pool)
+ return gen_pool_dma_alloc(hcd->localmem_pool, size, dma);
+
/* some USB hosts just use PIO */
if (!IS_ENABLED(CONFIG_HAS_DMA) ||
(!is_device_dma_capable(bus->sysdev) &&
@@ -152,6 +156,11 @@ void hcd_buffer_free(
if (!addr)
return;

+ if (hcd->localmem_pool) {
+ gen_pool_free(hcd->localmem_pool, (unsigned long)addr, size);
+ return;
+ }
+
if (!IS_ENABLED(CONFIG_HAS_DMA) ||
(!is_device_dma_capable(bus->sysdev) &&
!(hcd->driver->flags & HCD_LOCAL_MEM))) {
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 94d22551fc1b..29b96e5e8621 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -29,6 +29,8 @@
#include <linux/workqueue.h>
#include <linux/pm_runtime.h>
#include <linux/types.h>
+#include <linux/genalloc.h>
+#include <linux/io.h>

#include <linux/phy/phy.h>
#include <linux/usb.h>
@@ -3039,6 +3041,40 @@ usb_hcd_platform_shutdown(struct platform_device *dev)
}
EXPORT_SYMBOL_GPL(usb_hcd_platform_shutdown);

+int usb_hcd_setup_local_mem(struct usb_hcd *hcd, phys_addr_t phys_addr,
+ dma_addr_t dma, size_t size)
+{
+ int err;
+ void *local_mem;
+
+ hcd->localmem_pool = devm_gen_pool_create(hcd->self.sysdev, PAGE_SHIFT,
+ dev_to_node(hcd->self.sysdev),
+ dev_name(hcd->self.sysdev));
+ if (IS_ERR(hcd->localmem_pool))
+ return PTR_ERR(hcd->localmem_pool);
+
+ local_mem = devm_memremap(hcd->self.sysdev, phys_addr,
+ size, MEMREMAP_WC);
+ if (!local_mem)
+ return -ENOMEM;
+
+ /*
+ * Here we pass a dma_addr_t but the arg type is a phys_addr_t.
+ * It's not backed by system memory and thus there's no kernel mapping
+ * for it.
+ */
+ err = gen_pool_add_virt(hcd->localmem_pool, (unsigned long)local_mem,
+ dma, size, dev_to_node(hcd->self.sysdev));
+ if (err < 0) {
+ dev_err(hcd->self.sysdev, "gen_pool_add_virt failed with %d\n",
+ err);
+ return err;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(usb_hcd_setup_local_mem);
+
/*-------------------------------------------------------------------------*/

#if IS_ENABLED(CONFIG_USB_MON)
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 210181fd98d2..b200b19b44fa 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -40,6 +40,7 @@
#include <linux/dmapool.h>
#include <linux/workqueue.h>
#include <linux/debugfs.h>
+#include <linux/genalloc.h>

#include <asm/io.h>
#include <asm/irq.h>
@@ -505,8 +506,15 @@ static int ohci_init (struct ohci_hcd *ohci)
timer_setup(&ohci->io_watchdog, io_watchdog_func, 0);
ohci->prev_frame_no = IO_WATCHDOG_OFF;

- ohci->hcca = dma_alloc_coherent (hcd->self.controller,
- sizeof(*ohci->hcca), &ohci->hcca_dma, GFP_KERNEL);
+ if (hcd->localmem_pool)
+ ohci->hcca = gen_pool_dma_alloc(hcd->localmem_pool,
+ sizeof(*ohci->hcca),
+ &ohci->hcca_dma);
+ else
+ ohci->hcca = dma_alloc_coherent(hcd->self.controller,
+ sizeof(*ohci->hcca),
+ &ohci->hcca_dma,
+ GFP_KERNEL);
if (!ohci->hcca)
return -ENOMEM;

@@ -990,9 +998,14 @@ static void ohci_stop (struct usb_hcd *hcd)
remove_debug_files (ohci);
ohci_mem_cleanup (ohci);
if (ohci->hcca) {
- dma_free_coherent (hcd->self.controller,
- sizeof *ohci->hcca,
- ohci->hcca, ohci->hcca_dma);
+ if (hcd->localmem_pool)
+ gen_pool_free(hcd->localmem_pool,
+ (unsigned long)ohci->hcca,
+ sizeof(*ohci->hcca));
+ else
+ dma_free_coherent(hcd->self.controller,
+ sizeof(*ohci->hcca),
+ ohci->hcca, ohci->hcca_dma);
ohci->hcca = NULL;
ohci->hcca_dma = 0;
}
diff --git a/drivers/usb/host/ohci-mem.c b/drivers/usb/host/ohci-mem.c
index 3965ac0341eb..4afe27cc7e46 100644
--- a/drivers/usb/host/ohci-mem.c
+++ b/drivers/usb/host/ohci-mem.c
@@ -36,6 +36,13 @@ static void ohci_hcd_init (struct ohci_hcd *ohci)

static int ohci_mem_init (struct ohci_hcd *ohci)
{
+ /*
+ * HCs with local memory allocate from localmem_pool so there's
+ * no need to create the below dma pools.
+ */
+ if (ohci_to_hcd(ohci)->localmem_pool)
+ return 0;
+
ohci->td_cache = dma_pool_create ("ohci_td",
ohci_to_hcd(ohci)->self.controller,
sizeof (struct td),
@@ -84,8 +91,12 @@ td_alloc (struct ohci_hcd *hc, gfp_t mem_flags)
{
dma_addr_t dma;
struct td *td;
+ struct usb_hcd *hcd = ohci_to_hcd(hc);

- td = dma_pool_zalloc (hc->td_cache, mem_flags, &dma);
+ if (hcd->localmem_pool)
+ td = gen_pool_dma_zalloc(hcd->localmem_pool, sizeof(*td), &dma);
+ else
+ td = dma_pool_zalloc(hc->td_cache, mem_flags, &dma);
if (td) {
/* in case hc fetches it, make it look dead */
td->hwNextTD = cpu_to_hc32 (hc, dma);
@@ -99,6 +110,7 @@ static void
td_free (struct ohci_hcd *hc, struct td *td)
{
struct td **prev = &hc->td_hash [TD_HASH_FUNC (td->td_dma)];
+ struct usb_hcd *hcd = ohci_to_hcd(hc);

while (*prev && *prev != td)
prev = &(*prev)->td_hash;
@@ -106,7 +118,12 @@ td_free (struct ohci_hcd *hc, struct td *td)
*prev = td->td_hash;
else if ((td->hwINFO & cpu_to_hc32(hc, TD_DONE)) != 0)
ohci_dbg (hc, "no hash for td %p\n", td);
- dma_pool_free (hc->td_cache, td, td->td_dma);
+
+ if (hcd->localmem_pool)
+ gen_pool_free(hcd->localmem_pool, (unsigned long)td,
+ sizeof(*td));
+ else
+ dma_pool_free(hc->td_cache, td, td->td_dma);
}

/*-------------------------------------------------------------------------*/
@@ -117,8 +134,12 @@ ed_alloc (struct ohci_hcd *hc, gfp_t mem_flags)
{
dma_addr_t dma;
struct ed *ed;
+ struct usb_hcd *hcd = ohci_to_hcd(hc);

- ed = dma_pool_zalloc (hc->ed_cache, mem_flags, &dma);
+ if (hcd->localmem_pool)
+ ed = gen_pool_dma_zalloc(hcd->localmem_pool, sizeof(*ed), &dma);
+ else
+ ed = dma_pool_zalloc(hc->ed_cache, mem_flags, &dma);
if (ed) {
INIT_LIST_HEAD (&ed->td_list);
ed->dma = dma;
@@ -129,6 +150,12 @@ ed_alloc (struct ohci_hcd *hc, gfp_t mem_flags)
static void
ed_free (struct ohci_hcd *hc, struct ed *ed)
{
- dma_pool_free (hc->ed_cache, ed, ed->dma);
+ struct usb_hcd *hcd = ohci_to_hcd(hc);
+
+ if (hcd->localmem_pool)
+ gen_pool_free(hcd->localmem_pool, (unsigned long)ed,
+ sizeof(*ed));
+ else
+ dma_pool_free(hc->ed_cache, ed, ed->dma);
}

diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
index ef4813bfc5bf..b015b00774b2 100644
--- a/drivers/usb/host/ohci.h
+++ b/drivers/usb/host/ohci.h
@@ -385,6 +385,8 @@ struct ohci_hcd {

/*
* memory management for queue data structures
+ *
+ * @td_cache and @ed_cache are %NULL if &usb_hcd.localmem_pool is used.
*/
struct dma_pool *td_cache;
struct dma_pool *ed_cache;
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index bb57b5af4700..127560a4bfa0 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -216,6 +216,9 @@ struct usb_hcd {
#define HC_IS_RUNNING(state) ((state) & __ACTIVE)
#define HC_IS_SUSPENDED(state) ((state) & __SUSPEND)

+ /* memory pool for HCs having local memory, or %NULL */
+ struct gen_pool *localmem_pool;
+
/* more shared queuing code would be good; it should support
* smarter scheduling, handle transaction translators, etc;
* input size of periodic table to an interrupt scheduler.
@@ -461,6 +464,8 @@ extern int usb_add_hcd(struct usb_hcd *hcd,
unsigned int irqnum, unsigned long irqflags);
extern void usb_remove_hcd(struct usb_hcd *hcd);
extern int usb_hcd_find_raw_port_number(struct usb_hcd *hcd, int port1);
+int usb_hcd_setup_local_mem(struct usb_hcd *hcd, phys_addr_t phys_addr,
+ dma_addr_t dma, size_t size);

struct platform_device;
extern void usb_hcd_platform_shutdown(struct platform_device *dev);
--
2.17.1

2019-05-29 10:41:09

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v7 2/5] USB: use genalloc for USB HCs with local memory

On Wed, May 29, 2019 at 01:28:40PM +0300, [email protected] wrote:
> From: Laurentiu Tudor <[email protected]>
>
> For HCs that have local memory, replace the current DMA API usage
> with a genalloc generic allocator to manage the mappings for these
> devices. To help users, introduce a new HCD API,
> usb_hcd_setup_local_mem() that will setup up the genalloc backing
> up the device local memory. It will be used in subsequent patches.
> This is in preparation for dropping the existing "coherent" dma
> mem declaration APIs. Current implementation was relying on a short
> circuit in the DMA API that in the end, was acting as an allocator
> for these type of devices.
> For sh arch to compile GENERIC_ALLOCATOR needs to be selected in
> Kconfig.
>
> For context, see thread here: https://lkml.org/lkml/2019/4/22/357
>
> Signed-off-by: Laurentiu Tudor <[email protected]>
> Signed-off-by: Fredrik Noring <[email protected]>
> Tested-by: Fredrik Noring <[email protected]>
> Reported-by: kbuild test robot <[email protected]>
> ---
> drivers/usb/Kconfig | 1 +
> drivers/usb/core/buffer.c | 9 +++++++++
> drivers/usb/core/hcd.c | 36 ++++++++++++++++++++++++++++++++++++
> drivers/usb/host/ohci-hcd.c | 23 ++++++++++++++++++-----
> drivers/usb/host/ohci-mem.c | 35 +++++++++++++++++++++++++++++++----
> drivers/usb/host/ohci.h | 2 ++
> include/linux/usb/hcd.h | 5 +++++
> 7 files changed, 102 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index e4b27413f528..389c57d8eba7 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -45,6 +45,7 @@ config USB_ARCH_HAS_HCD
> config USB
> tristate "Support for Host-side USB"
> depends on USB_ARCH_HAS_HCD
> + select GENERIC_ALLOCATOR

Are there any arches that does not have GENERIC_ALLOCATOR? I don't want
to suddenly cut off a bunch of working systems from having USB support.

thanks,

greg k-h

2019-05-29 11:17:44

by Laurentiu Tudor

[permalink] [raw]
Subject: Re: [PATCH v7 2/5] USB: use genalloc for USB HCs with local memory



On 29.05.2019 13:38, Greg KH wrote:
> On Wed, May 29, 2019 at 01:28:40PM +0300, [email protected] wrote:
>> From: Laurentiu Tudor <[email protected]>
>>
>> For HCs that have local memory, replace the current DMA API usage
>> with a genalloc generic allocator to manage the mappings for these
>> devices. To help users, introduce a new HCD API,
>> usb_hcd_setup_local_mem() that will setup up the genalloc backing
>> up the device local memory. It will be used in subsequent patches.
>> This is in preparation for dropping the existing "coherent" dma
>> mem declaration APIs. Current implementation was relying on a short
>> circuit in the DMA API that in the end, was acting as an allocator
>> for these type of devices.
>> For sh arch to compile GENERIC_ALLOCATOR needs to be selected in
>> Kconfig.
>>
>> For context, see thread here: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2019%2F4%2F22%2F357&amp;data=02%7C01%7Claurentiu.tudor%40nxp.com%7Cade28e1f322c4502cd4808d6e421d0ba%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636947231220264072&amp;sdata=xvmzDztMbeD9GwlcAfx7bBoxhARWgB3vmQkqiE81Lbg%3D&amp;reserved=0
>>
>> Signed-off-by: Laurentiu Tudor <[email protected]>
>> Signed-off-by: Fredrik Noring <[email protected]>
>> Tested-by: Fredrik Noring <[email protected]>
>> Reported-by: kbuild test robot <[email protected]>
>> ---
>> drivers/usb/Kconfig | 1 +
>> drivers/usb/core/buffer.c | 9 +++++++++
>> drivers/usb/core/hcd.c | 36 ++++++++++++++++++++++++++++++++++++
>> drivers/usb/host/ohci-hcd.c | 23 ++++++++++++++++++-----
>> drivers/usb/host/ohci-mem.c | 35 +++++++++++++++++++++++++++++++----
>> drivers/usb/host/ohci.h | 2 ++
>> include/linux/usb/hcd.h | 5 +++++
>> 7 files changed, 102 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
>> index e4b27413f528..389c57d8eba7 100644
>> --- a/drivers/usb/Kconfig
>> +++ b/drivers/usb/Kconfig
>> @@ -45,6 +45,7 @@ config USB_ARCH_HAS_HCD
>> config USB
>> tristate "Support for Host-side USB"
>> depends on USB_ARCH_HAS_HCD
>> + select GENERIC_ALLOCATOR
>
> Are there any arches that does not have GENERIC_ALLOCATOR? I don't want
> to suddenly cut off a bunch of working systems from having USB support.
>

lkp report mentions only sh, but even if there are others, I think that
now having the explicit select should cover them, right?

---
Best Regards, Laurentiu

2019-05-29 11:25:04

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v7 2/5] USB: use genalloc for USB HCs with local memory

On Wed, May 29, 2019 at 11:15:54AM +0000, Laurentiu Tudor wrote:
>
>
> On 29.05.2019 13:38, Greg KH wrote:
> > On Wed, May 29, 2019 at 01:28:40PM +0300, [email protected] wrote:
> >> From: Laurentiu Tudor <[email protected]>
> >>
> >> For HCs that have local memory, replace the current DMA API usage
> >> with a genalloc generic allocator to manage the mappings for these
> >> devices. To help users, introduce a new HCD API,
> >> usb_hcd_setup_local_mem() that will setup up the genalloc backing
> >> up the device local memory. It will be used in subsequent patches.
> >> This is in preparation for dropping the existing "coherent" dma
> >> mem declaration APIs. Current implementation was relying on a short
> >> circuit in the DMA API that in the end, was acting as an allocator
> >> for these type of devices.
> >> For sh arch to compile GENERIC_ALLOCATOR needs to be selected in
> >> Kconfig.
> >>
> >> For context, see thread here: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2019%2F4%2F22%2F357&amp;data=02%7C01%7Claurentiu.tudor%40nxp.com%7Cade28e1f322c4502cd4808d6e421d0ba%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636947231220264072&amp;sdata=xvmzDztMbeD9GwlcAfx7bBoxhARWgB3vmQkqiE81Lbg%3D&amp;reserved=0
> >>
> >> Signed-off-by: Laurentiu Tudor <[email protected]>
> >> Signed-off-by: Fredrik Noring <[email protected]>
> >> Tested-by: Fredrik Noring <[email protected]>
> >> Reported-by: kbuild test robot <[email protected]>
> >> ---
> >> drivers/usb/Kconfig | 1 +
> >> drivers/usb/core/buffer.c | 9 +++++++++
> >> drivers/usb/core/hcd.c | 36 ++++++++++++++++++++++++++++++++++++
> >> drivers/usb/host/ohci-hcd.c | 23 ++++++++++++++++++-----
> >> drivers/usb/host/ohci-mem.c | 35 +++++++++++++++++++++++++++++++----
> >> drivers/usb/host/ohci.h | 2 ++
> >> include/linux/usb/hcd.h | 5 +++++
> >> 7 files changed, 102 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> >> index e4b27413f528..389c57d8eba7 100644
> >> --- a/drivers/usb/Kconfig
> >> +++ b/drivers/usb/Kconfig
> >> @@ -45,6 +45,7 @@ config USB_ARCH_HAS_HCD
> >> config USB
> >> tristate "Support for Host-side USB"
> >> depends on USB_ARCH_HAS_HCD
> >> + select GENERIC_ALLOCATOR
> >
> > Are there any arches that does not have GENERIC_ALLOCATOR? I don't want
> > to suddenly cut off a bunch of working systems from having USB support.
> >
>
> lkp report mentions only sh, but even if there are others, I think that
> now having the explicit select should cover them, right?

As long as GENERIC_ALLOCATOR works on all arches, yes, that's fine, but
please verify that this is the case.

thanks,

greg k-h

2019-05-29 11:27:16

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v7 2/5] USB: use genalloc for USB HCs with local memory

On Wed, May 29, 2019 at 04:23:34AM -0700, Greg KH wrote:
> As long as GENERIC_ALLOCATOR works on all arches, yes, that's fine, but
> please verify that this is the case.

It works everywhere. The issue here is just that it get pulled in
by default on most architetures, but not on all.

2019-05-29 11:34:14

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v7 2/5] USB: use genalloc for USB HCs with local memory

On Wed, May 29, 2019 at 01:25:15PM +0200, [email protected] wrote:
> On Wed, May 29, 2019 at 04:23:34AM -0700, Greg KH wrote:
> > As long as GENERIC_ALLOCATOR works on all arches, yes, that's fine, but
> > please verify that this is the case.
>
> It works everywhere. The issue here is just that it get pulled in
> by default on most architetures, but not on all.

Ah, ok, that's fine then, no objection from me, let me go review the
patches again...

thanks,

greg k-h

2019-05-29 11:35:58

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v7 0/5] prerequisites for device reserved local mem rework

On Wed, May 29, 2019 at 01:28:38PM +0300, [email protected] wrote:
> From: Laurentiu Tudor <[email protected]>
>
> For HCs that have local memory, replace the current DMA API usage
> with a genalloc generic allocator to manage the mappings for these
> devices.
> This is in preparation for dropping the existing "coherent" dma
> mem declaration APIs. Current implementation was relying on a short
> circuit in the DMA API that in the end, was acting as an allocator
> for these type of devices.
>
> Only compiled tested, so any volunteers willing to test are most welcome.
>
> Thank you!
>
> For context, see thread here: https://lkml.org/lkml/2019/4/22/357

All look good to me:

Reviewed-by: Greg Kroah-Hartman <[email protected]>

Christoph, this is going through your tree, right?

thanks,

greg k-h

2019-05-29 11:40:47

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v7 0/5] prerequisites for device reserved local mem rework

On Wed, May 29, 2019 at 04:34:27AM -0700, Greg KH wrote:
> Reviewed-by: Greg Kroah-Hartman <[email protected]>
>
> Christoph, this is going through your tree, right?

Yes, I'll pіck it up.

2019-05-29 14:09:00

by Laurentiu Tudor

[permalink] [raw]
Subject: Re: [PATCH v7 0/5] prerequisites for device reserved local mem rework

Hi Christoph,

On 29.05.2019 14:37, Christoph Hellwig wrote:
> On Wed, May 29, 2019 at 04:34:27AM -0700, Greg KH wrote:
>> Reviewed-by: Greg Kroah-Hartman <[email protected]>
>>
>> Christoph, this is going through your tree, right?
>
> Yes, I'll pіck it up.
>

Thanks, hope this time everything is fine.
When you get the time, please let me know your ideas on the next steps.

---
Best Regards, Laurentiu

2019-05-31 16:45:29

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v7 0/5] prerequisites for device reserved local mem rework

On Wed, May 29, 2019 at 02:06:12PM +0000, Laurentiu Tudor wrote:
> Thanks, hope this time everything is fine.

I've applied it to the dma-mapping tree now.

> When you get the time, please let me know your ideas on the next steps.

I think the next step is to move the call to
dma_alloc_from_dev_coherent from dma_alloc_attrs into the ->alloc
instances. The only onces that really need it for now are the
generic and legacy arm dma-direct code, and drivers/iommu/dma-iommu.c
as well as the ARM DMA API code, as those are the ones use for
architectures that declare coherent regions. The other iommus are
not used on OF platforms (at least that's what my analysis said a while
ago, feel free to double check it)

2019-05-31 17:07:59

by Laurentiu Tudor

[permalink] [raw]
Subject: RE: [PATCH v7 0/5] prerequisites for device reserved local mem rework

> -----Original Message-----
> From: Christoph Hellwig <[email protected]>
> Sent: Friday, May 31, 2019 7:43 PM
>
> On Wed, May 29, 2019 at 02:06:12PM +0000, Laurentiu Tudor wrote:
> > Thanks, hope this time everything is fine.
>
> I've applied it to the dma-mapping tree now.
>
> > When you get the time, please let me know your ideas on the next steps.
>
> I think the next step is to move the call to
> dma_alloc_from_dev_coherent from dma_alloc_attrs into the ->alloc
> instances. The only onces that really need it for now are the
> generic and legacy arm dma-direct code, and drivers/iommu/dma-iommu.c
> as well as the ARM DMA API code, as those are the ones use for
> architectures that declare coherent regions. The other iommus are
> not used on OF platforms (at least that's what my analysis said a while
> ago, feel free to double check it)

Thanks. I'll start looking into it starting next week. Keep in touch.

---
Best Regards, Laurentiu

2019-06-04 14:19:35

by Laurentiu Tudor

[permalink] [raw]
Subject: Re: [PATCH v7 0/5] prerequisites for device reserved local mem rework

On 31.05.2019 19:43, Christoph Hellwig wrote:
> On Wed, May 29, 2019 at 02:06:12PM +0000, Laurentiu Tudor wrote:
>> Thanks, hope this time everything is fine.
>
> I've applied it to the dma-mapping tree now.
>
>> When you get the time, please let me know your ideas on the next steps.
>
> I think the next step is to move the call to
> dma_alloc_from_dev_coherent from dma_alloc_attrs into the ->alloc
> instances. The only onces that really need it for now are the
> generic and legacy arm dma-direct code, and drivers/iommu/dma-iommu.c
> as well as the ARM DMA API code, as those are the ones use for
> architectures that declare coherent regions. The other iommus are
> not used on OF platforms (at least that's what my analysis said a while
> ago, feel free to double check it)

Thanks for the details. I'll be OOO next week so probably I'll start
looking into it next next week.

---
Best Regards, Laurentiu