Message ID | 20190514143807.7745-2-laurentiu.tudor@nxp.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | prerequisites for device reserved local mem rework | expand |
> @@ -136,6 +137,10 @@ void *hcd_buffer_alloc( > if (size <= pool_max[i]) > return dma_pool_alloc(hcd->pool[i], mem_flags, dma); > } > + > + if (hcd->driver->flags & HCD_LOCAL_MEM) > + return gen_pool_dma_alloc(hcd->localmem_pool, size, dma); I think this check needs to be before the above code to use the dma pools, as we should always use the HCD local memory. Probably all the way up just below the size == 0 check, that way we can also remove the other HCD_LOCAL_MEM check. > @@ -165,5 +170,10 @@ void hcd_buffer_free( > return; > } > } > - dma_free_coherent(hcd->self.sysdev, size, addr, dma); > + > + if (hcd->driver->flags & HCD_LOCAL_MEM) > + gen_pool_free(hcd->localmem_pool, (unsigned long)addr, > + size); > + else > + dma_free_coherent(hcd->self.sysdev, size, addr, dma); Same here. > @@ -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->driver->flags & HCD_LOCAL_MEM) > + 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); I wonder if we could just use hcd_buffer_alloc/free here, althought that would require them to be exported. I'll leave that decision to the relevant maintainers, though. Except for this the series looks exactly what I had envisioned to get rid of the device local dma_declare_coherent use case, thanks!
On 14.05.2019 17:42, Christoph Hellwig wrote: >> @@ -136,6 +137,10 @@ void *hcd_buffer_alloc( >> if (size <= pool_max[i]) >> return dma_pool_alloc(hcd->pool[i], mem_flags, dma); >> } >> + >> + if (hcd->driver->flags & HCD_LOCAL_MEM) >> + return gen_pool_dma_alloc(hcd->localmem_pool, size, dma); > > I think this check needs to be before the above code to use the dma > pools, as we should always use the HCD local memory. Probably all the > way up just below the size == 0 check, that way we can also remove the > other HCD_LOCAL_MEM check. Alright. >> @@ -165,5 +170,10 @@ void hcd_buffer_free( >> return; >> } >> } >> - dma_free_coherent(hcd->self.sysdev, size, addr, dma); >> + >> + if (hcd->driver->flags & HCD_LOCAL_MEM) >> + gen_pool_free(hcd->localmem_pool, (unsigned long)addr, >> + size); >> + else >> + dma_free_coherent(hcd->self.sysdev, size, addr, dma); > > Same here. Ok. >> @@ -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->driver->flags & HCD_LOCAL_MEM) >> + 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); > > I wonder if we could just use hcd_buffer_alloc/free here, althought > that would require them to be exported. I'll leave that decision to > the relevant maintainers, though. > > Except for this the series looks exactly what I had envisioned to > get rid of the device local dma_declare_coherent use case, thanks! Glad I could help. On the remoteproc_virtio.c case, I had a cursory look and found out that the dma_declare_coherent_memory() usage was introduced quite recently, by this patch: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=086d08725d34c6b3333db710344ae9c4fdafb2d5 --- Best Regards, Laurentiu
On Wed, May 15, 2019 at 09:57:12AM +0000, Laurentiu Tudor wrote: > Glad I could help. On the remoteproc_virtio.c case, I had a cursory look > and found out that the dma_declare_coherent_memory() usage was > introduced quite recently, by this patch: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=086d08725d34c6b3333db710344ae9c4fdafb2d5 Yes. I took a look at it to, and while it isn't exactly a clean usage of the API it at least declares system memory and not a resource. So it doesn't really affect our plan.
diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c index f641342cdec0..5729801e82e0 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> @@ -136,6 +137,10 @@ void *hcd_buffer_alloc( if (size <= pool_max[i]) return dma_pool_alloc(hcd->pool[i], mem_flags, dma); } + + if (hcd->driver->flags & HCD_LOCAL_MEM) + return gen_pool_dma_alloc(hcd->localmem_pool, size, dma); + return dma_alloc_coherent(hcd->self.sysdev, size, dma, mem_flags); } @@ -165,5 +170,10 @@ void hcd_buffer_free( return; } } - dma_free_coherent(hcd->self.sysdev, size, addr, dma); + + if (hcd->driver->flags & HCD_LOCAL_MEM) + gen_pool_free(hcd->localmem_pool, (unsigned long)addr, + size); + else + dma_free_coherent(hcd->self.sysdev, size, addr, dma); } diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index 210181fd98d2..f9462c372943 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->driver->flags & HCD_LOCAL_MEM) + 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->driver->flags & HCD_LOCAL_MEM) + 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/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index 695931b03684..0fee81ef5d52 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -215,6 +215,9 @@ struct usb_hcd { #define HC_IS_RUNNING(state) ((state) & __ACTIVE) #define HC_IS_SUSPENDED(state) ((state) & __SUSPEND) + /* allocator for HCs having local memory */ + 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.