Re: [PATCH] usb-storage: Revert commit 747668dbc061 ("usb-storage: Set virt_boundary_mask to avoid SG overflows")
On Mon, Oct 21, 2019 at 11:48:06AM -0400, Alan Stern wrote:
> There is no longer any reason to keep the virt_boundary_mask setting
> for usb-storage. It was needed in the first place only for handling
> devices with a block size smaller than the maxpacket size and where
> the host controller was not capable of fully general scatter-gather
> operation (that is, able to merge two SG segments into a single USB
> packet). But:
>
> High-speed or slower connections never use a bulk maxpacket
> value larger than 512;
>
> The SCSI layer does not handle block devices with a block size
> smaller than 512 bytes;
>
> All the host controllers capable of SuperSpeed operation can
> handle fully general SG;
>
> Since commit ea44d190764b ("usbip: Implement SG support to
> vhci-hcd and stub driver") was merged, the USB/IP driver can
> also handle SG.
>
> Therefore all supported device/controller combinations should be okay
> with no need for any special virt_boundary_mask. So in order to fix
> the swiotlb problem, this patch reverts commit 747668dbc061.
That's great to know. The same should also apply to uas, shouldn't
it?
Otherwise:
Acked-by: Christoph Hellwig
Re: [PATCH v3 10/11] RFC: usb-storage: export symbols in USB_STORAGE namespace
On Wed, Aug 21, 2019 at 12:49:25PM +0100, Matthias Maennich wrote: > Modules using these symbols are required to explicitly import the > namespace. This patch was generated with the following steps and serves > as a reference to use the symbol namespace feature: > > 1) Define DEFAULT_SYMBOL_NAMESPACE in the corresponding Makefile > 2) make (see warnings during modpost about missing imports) > 3) make nsdeps > > Instead of a DEFAULT_SYMBOL_NAMESPACE definition, the EXPORT_SYMBOL_NS > variants can be used to explicitly specify the namespace. The advantage > of the method used here is that newly added symbols are automatically > exported and existing ones are exported without touching their > respective EXPORT_SYMBOL macro expansion. So what is USB_STORAGE here? It isn't a C string, so where does it come from? To me using a C string would seem like the nicer interface vs a random cpp symbol that gets injected somewhere.
Re: reeze while write on external usb 3.0 hard disk [Bug 204095]
On Mon, Aug 19, 2019 at 10:14:25AM -0400, Alan Stern wrote:
> Let's bring this to the attention of some more people.
>
> It looks like the bug that was supposed to be fixed by commit
> d74ffae8b8dd ("usb-storage: Add a limitation for
> blk_queue_max_hw_sectors()"), which is part of 5.2.5, but apparently
> the bug still occurs.
Piergiorgio,
can you dump the content of max_hw_sectors_kb file for your USB storage
device and send that to this thread?
[PATCH 5/6] dma-mapping: remove is_device_dma_capable
No users left.
Signed-off-by: Christoph Hellwig
---
include/linux/dma-mapping.h | 5 -
1 file changed, 5 deletions(-)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index f7d1eea32c78..14702e2d6fa8 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -149,11 +149,6 @@ static inline int valid_dma_direction(int dma_direction)
(dma_direction == DMA_FROM_DEVICE));
}
-static inline int is_device_dma_capable(struct device *dev)
-{
- return dev->dma_mask != NULL && *dev->dma_mask != DMA_MASK_NONE;
-}
-
#ifdef CONFIG_DMA_DECLARE_COHERENT
/*
* These three functions are only for dma allocator.
--
2.20.1
[PATCH 4/6] usb/max3421: remove the dummy {un,}map_urb_for_dma methods
Now that we have an explicit HCD_DMA flag, there is not need to override
these methods.
Signed-off-by: Christoph Hellwig
---
drivers/usb/host/max3421-hcd.c | 17 -
1 file changed, 17 deletions(-)
diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index afa321ab55fc..8819f502b6a6 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -1800,21 +1800,6 @@ max3421_bus_resume(struct usb_hcd *hcd)
return -1;
}
-/*
- * The SPI driver already takes care of DMA-mapping/unmapping, so no
- * reason to do it twice.
- */
-static int
-max3421_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags)
-{
- return 0;
-}
-
-static void
-max3421_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
-{
-}
-
static const struct hc_driver max3421_hcd_desc = {
.description = "max3421",
.product_desc = DRIVER_DESC,
@@ -1826,8 +1811,6 @@ static const struct hc_driver max3421_hcd_desc = {
.get_frame_number = max3421_get_frame_number,
.urb_enqueue = max3421_urb_enqueue,
.urb_dequeue = max3421_urb_dequeue,
- .map_urb_for_dma = max3421_map_urb_for_dma,
- .unmap_urb_for_dma =max3421_unmap_urb_for_dma,
.endpoint_disable = max3421_endpoint_disable,
.hub_status_data = max3421_hub_status_data,
.hub_control = max3421_hub_control,
--
2.20.1
[PATCH 3/6] usb: add a HCD_DMA flag instead of guestimating DMA capabilities
The usb core is the only major place in the kernel that checks for
a non-NULL device dma_mask to see if a device is DMA capable. This
is generally a bad idea, as all major busses always set up a DMA mask,
even if the device is not DMA capable - in fact bus layers like PCI
can't even know if a device is DMA capable at enumeration time. This
leads to lots of workaround in HCD drivers, and also prevented us from
setting up a DMA mask for platform devices by default last time we
tried.
Replace this guess with an explicit HCD_DMA that is set by drivers that
appear to have DMA support.
Signed-off-by: Christoph Hellwig
---
drivers/staging/octeon-usb/octeon-hcd.c | 2 +-
drivers/usb/core/hcd.c | 1 -
drivers/usb/dwc2/hcd.c | 6 +++---
drivers/usb/host/ehci-grlib.c | 2 +-
drivers/usb/host/ehci-hcd.c | 2 +-
drivers/usb/host/ehci-pmcmsp.c | 2 +-
drivers/usb/host/ehci-ppc-of.c | 2 +-
drivers/usb/host/ehci-ps3.c | 2 +-
drivers/usb/host/ehci-sh.c | 2 +-
drivers/usb/host/ehci-xilinx-of.c | 2 +-
drivers/usb/host/fhci-hcd.c | 2 +-
drivers/usb/host/fotg210-hcd.c | 2 +-
drivers/usb/host/imx21-hcd.c| 2 +-
drivers/usb/host/isp116x-hcd.c | 6 --
drivers/usb/host/isp1362-hcd.c | 5 -
drivers/usb/host/ohci-hcd.c | 2 +-
drivers/usb/host/ohci-ppc-of.c | 2 +-
drivers/usb/host/ohci-ps3.c | 2 +-
drivers/usb/host/ohci-sa.c | 2 +-
drivers/usb/host/ohci-sm501.c | 2 +-
drivers/usb/host/ohci-tmio.c| 2 +-
drivers/usb/host/oxu210hp-hcd.c | 3 ---
drivers/usb/host/r8a66597-hcd.c | 6 --
drivers/usb/host/sl811-hcd.c| 6 --
drivers/usb/host/u132-hcd.c | 2 --
drivers/usb/host/uhci-grlib.c | 2 +-
drivers/usb/host/uhci-pci.c | 2 +-
drivers/usb/host/uhci-platform.c| 2 +-
drivers/usb/host/xhci.c | 2 +-
drivers/usb/isp1760/isp1760-core.c | 3 ---
drivers/usb/isp1760/isp1760-if.c| 1 -
drivers/usb/musb/musb_host.c| 2 +-
drivers/usb/renesas_usbhs/mod_host.c| 2 +-
include/linux/usb.h | 1 -
include/linux/usb/hcd.h | 7 +--
35 files changed, 31 insertions(+), 62 deletions(-)
diff --git a/drivers/staging/octeon-usb/octeon-hcd.c
b/drivers/staging/octeon-usb/octeon-hcd.c
index cd2b777073c4..a5321cc692c5 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -3512,7 +3512,7 @@ static const struct hc_driver octeon_hc_driver = {
.product_desc = "Octeon Host Controller",
.hcd_priv_size = sizeof(struct octeon_hcd),
.irq= octeon_usb_irq,
- .flags = HCD_MEMORY | HCD_USB2,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB2,
.start = octeon_usb_start,
.stop = octeon_usb_stop,
.urb_enqueue= octeon_usb_urb_enqueue,
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 8592c0344fe8..add2af4af766 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2454,7 +2454,6 @@ struct usb_hcd *__usb_create_hcd(const struct hc_driver
*driver,
hcd->self.controller = dev;
hcd->self.sysdev = sysdev;
hcd->self.bus_name = bus_name;
- hcd->self.uses_dma = (sysdev->dma_mask != NULL);
timer_setup(&hcd->rh_timer, rh_timer_func, 0);
#ifdef CONFIG_PM
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 111787a137ee..81afe553aa66 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -5062,13 +5062,13 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg)
dwc2_hc_driver.reset_device = dwc2_reset_device;
}
+ if (hsotg->params.host_dma)
+ dwc2_hc_driver.flags |= HCD_DMA;
+
hcd = usb_create_hcd(&dwc2_hc_driver, hsotg->dev, dev_name(hsotg->dev));
if (!hcd)
goto error1;
- if (!hsotg->params.host_dma)
- hcd->self.uses_dma = 0;
-
hcd->has_tt = 1;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
diff --git a/drivers/usb/host/ehci-grlib.c b/drivers/usb/host/ehci-grlib.c
index 656b8c08efc8..a2c3b4ec8a8b 100644
--- a/drivers/usb/host/ehci-grlib.c
+++ b/drivers/usb/host/ehci-grlib.c
@@ -30,7 +30,7 @@ static const struct hc_driver ehci_grlib_hc_driver = {
* generic hardware linkage
*/
.irq= ehci_irq,
- .flags = HCD_MEMORY | HCD_USB2 | HCD_BH,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB2 | HCD_BH,
/*
* basic lifecycle operations
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.
[PATCH 2/6] usb: add a hcd_uses_dma helper
The USB buffer allocation code is the only place in the usb core (and in
fact the whole kernel) that uses is_device_dma_capable, while the URB
mapping code uses the uses_dma flag in struct usb_bus. Switch the buffer
allocation to use the uses_dma flag used by the rest of the USB code,
and create a helper in hcd.h that checks this flag as well as the
CONFIG_HAS_DMA to simplify the caller a bit.
Signed-off-by: Christoph Hellwig
---
drivers/usb/core/buffer.c | 10 +++---
drivers/usb/core/hcd.c| 4 ++--
drivers/usb/dwc2/hcd.c| 2 +-
include/linux/usb.h | 2 +-
include/linux/usb/hcd.h | 3 +++
5 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index 1a5b3dcae930..6cf22c27f2d2 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -66,9 +66,7 @@ int hcd_buffer_create(struct usb_hcd *hcd)
charname[16];
int i, size;
- if (hcd->localmem_pool ||
- !IS_ENABLED(CONFIG_HAS_DMA) ||
- !is_device_dma_capable(hcd->self.sysdev))
+ if (hcd->localmem_pool || !hcd_uses_dma(hcd))
return 0;
for (i = 0; i < HCD_BUFFER_POOLS; i++) {
@@ -129,8 +127,7 @@ void *hcd_buffer_alloc(
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)) {
+ if (!hcd_uses_dma(hcd)) {
*dma = ~(dma_addr_t) 0;
return kmalloc(size, mem_flags);
}
@@ -160,8 +157,7 @@ void hcd_buffer_free(
return;
}
- if (!IS_ENABLED(CONFIG_HAS_DMA) ||
- !is_device_dma_capable(bus->sysdev)) {
+ if (!hcd_uses_dma(hcd)) {
kfree(addr);
return;
}
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 2ccbc2f83570..8592c0344fe8 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1412,7 +1412,7 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct
urb *urb,
if (usb_endpoint_xfer_control(&urb->ep->desc)) {
if (hcd->self.uses_pio_for_control)
return ret;
- if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
+ if (hcd_uses_dma(hcd)) {
if (is_vmalloc_addr(urb->setup_packet)) {
WARN_ONCE(1, "setup packet is not dma
capable\n");
return -EAGAIN;
@@ -1446,7 +1446,7 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct
urb *urb,
dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
if (urb->transfer_buffer_length != 0
&& !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
- if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
+ if (hcd_uses_dma(hcd)) {
if (urb->num_sgs) {
int n;
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index ee144ff8af5b..111787a137ee 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -4608,7 +4608,7 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd,
struct urb *urb,
buf = urb->transfer_buffer;
- if (hcd->self.uses_dma) {
+ if (hcd_uses_dma(hcd)) {
if (!buf && (urb->transfer_dma & 3)) {
dev_err(hsotg->dev,
"%s: unaligned transfer with no
transfer_buffer",
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 83d35d993e8c..e87826e23d59 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1457,7 +1457,7 @@ typedef void (*usb_complete_t)(struct urb *);
* field rather than determining a dma address themselves.
*
* Note that transfer_buffer must still be set if the controller
- * does not support DMA (as indicated by bus.uses_dma) and when talking
+ * does not support DMA (as indicated by hcd_uses_dma()) and when talking
* to root hub. If you have to trasfer between highmem zone and the device
* on such controller, create a bounce buffer or bail out with an error.
* If transfer_buffer cannot be set (is in highmem) and the controller is DMA
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index bab27ccc8ff5..a20e7815d814 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -422,6 +422,9 @@ static inline bool
hcd_periodic_completion_in_progress(struct usb_hcd *hcd,
return hcd->high_prio_bh.completing_ep == ep;
}
+#define hcd_uses_dma(hcd) \
+ (IS_ENABLED(CONFIG_HAS_DMA) && (hcd)->self.uses_dma)
+
extern int usb_hcd_link_urb_to_ep(struct usb_hcd *hcd, struct urb *urb);
extern int usb_hcd_check_unlink_urb(struct usb_hcd *hcd, struct urb *urb,
int status);
--
2.20.1
[PATCH 1/6] usb: don't create dma pools for HCDs with a localmem_pool
If the HCD provides a localmem pool we will never use the DMA pools, so
don't create them.
Fixes: b0310c2f09bb ("USB: use genalloc for USB HCs with local memory")
Signed-off-by: Christoph Hellwig
---
drivers/usb/core/buffer.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index 1359b78a624e..1a5b3dcae930 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -66,9 +66,9 @@ int hcd_buffer_create(struct usb_hcd *hcd)
charname[16];
int i, size;
- if (!IS_ENABLED(CONFIG_HAS_DMA) ||
- (!is_device_dma_capable(hcd->self.sysdev) &&
-!hcd->localmem_pool))
+ if (hcd->localmem_pool ||
+ !IS_ENABLED(CONFIG_HAS_DMA) ||
+ !is_device_dma_capable(hcd->self.sysdev))
return 0;
for (i = 0; i < HCD_BUFFER_POOLS; i++) {
--
2.20.1
[PATCH 6/6] driver core: initialize a default DMA mask for platform device
We still treat devices without a DMA mask as defaulting to 32-bits for
both mask, but a few releases ago we've started warning about such
cases, as they require special cases to work around this sloppyness.
Add a dma_mask field to struct platform_device so that we can initialize
the dma_mask pointer in struct device and initialize both masks to
32-bits by default, replacing similar functionality in m68k and
powerpc. The arch_setup_pdev_archdata hooks is now unused and removed.
Note that the code looks a little odd with the various conditionals
because we have to support platform_device structures that are
statically allocated.
Signed-off-by: Christoph Hellwig
---
arch/m68k/kernel/dma.c | 9 ---
arch/powerpc/kernel/setup-common.c | 6 -
arch/sh/boards/mach-ap325rxa/setup.c | 1 -
arch/sh/boards/mach-ecovec24/setup.c | 2 --
arch/sh/boards/mach-kfr2r09/setup.c | 1 -
arch/sh/boards/mach-migor/setup.c| 1 -
arch/sh/boards/mach-se/7724/setup.c | 2 --
drivers/base/platform.c | 37
include/linux/platform_device.h | 2 +-
9 files changed, 17 insertions(+), 44 deletions(-)
diff --git a/arch/m68k/kernel/dma.c b/arch/m68k/kernel/dma.c
index 30cd59caf037..447849d1d645 100644
--- a/arch/m68k/kernel/dma.c
+++ b/arch/m68k/kernel/dma.c
@@ -79,12 +79,3 @@ void arch_sync_dma_for_device(struct device *dev,
phys_addr_t handle,
break;
}
}
-
-void arch_setup_pdev_archdata(struct platform_device *pdev)
-{
- if (pdev->dev.coherent_dma_mask == DMA_MASK_NONE &&
- pdev->dev.dma_mask == NULL) {
- pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
- pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
- }
-}
diff --git a/arch/powerpc/kernel/setup-common.c
b/arch/powerpc/kernel/setup-common.c
index 1f8db666468d..5e6543aba1b3 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -778,12 +778,6 @@ void ppc_printk_progress(char *s, unsigned short hex)
pr_info("%s\n", s);
}
-void arch_setup_pdev_archdata(struct platform_device *pdev)
-{
- pdev->archdata.dma_mask = DMA_BIT_MASK(32);
- pdev->dev.dma_mask = &pdev->archdata.dma_mask;
-}
-
static __init void print_system_info(void)
{
pr_info("-\n");
diff --git a/arch/sh/boards/mach-ap325rxa/setup.c
b/arch/sh/boards/mach-ap325rxa/setup.c
index 8301a4378f50..665cad452798 100644
--- a/arch/sh/boards/mach-ap325rxa/setup.c
+++ b/arch/sh/boards/mach-ap325rxa/setup.c
@@ -527,7 +527,6 @@ static int __init ap325rxa_devices_setup(void)
/* Initialize CEU platform device separately to map memory first */
device_initialize(&ap325rxa_ceu_device.dev);
- arch_setup_pdev_archdata(&ap325rxa_ceu_device);
dma_declare_coherent_memory(&ap325rxa_ceu_device.dev,
ceu_dma_membase, ceu_dma_membase,
ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
diff --git a/arch/sh/boards/mach-ecovec24/setup.c
b/arch/sh/boards/mach-ecovec24/setup.c
index f402aa741bf3..acaa97459531 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -1440,7 +1440,6 @@ static int __init arch_setup(void)
/* Initialize CEU platform devices separately to map memory first */
device_initialize(&ecovec_ceu_devices[0]->dev);
- arch_setup_pdev_archdata(ecovec_ceu_devices[0]);
dma_declare_coherent_memory(&ecovec_ceu_devices[0]->dev,
ceu0_dma_membase, ceu0_dma_membase,
ceu0_dma_membase +
@@ -1448,7 +1447,6 @@ static int __init arch_setup(void)
platform_device_add(ecovec_ceu_devices[0]);
device_initialize(&ecovec_ceu_devices[1]->dev);
- arch_setup_pdev_archdata(ecovec_ceu_devices[1]);
dma_declare_coherent_memory(&ecovec_ceu_devices[1]->dev,
ceu1_dma_membase, ceu1_dma_membase,
ceu1_dma_membase +
diff --git a/arch/sh/boards/mach-kfr2r09/setup.c
b/arch/sh/boards/mach-kfr2r09/setup.c
index 1cf9a47ac90e..96538ba3aa32 100644
--- a/arch/sh/boards/mach-kfr2r09/setup.c
+++ b/arch/sh/boards/mach-kfr2r09/setup.c
@@ -601,7 +601,6 @@ static int __init kfr2r09_devices_setup(void)
/* Initialize CEU platform device separately to map memory first */
device_initialize(&kfr2r09_ceu_device.dev);
- arch_setup_pdev_archdata(&kfr2r09_ceu_device);
dma_declare_coherent_memory(&kfr2r09_ceu_device.dev,
ceu_dma_membase, ceu_dma_membase,
ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
diff --git a/arch/sh/boards/mach-migor/setup.c
b/arch/sh/boards/mach-migor/setup.c
index 90702740f207.
next take at setting up a dma mask by default for platform devices v2
Hi all, this is another attempt to make sure the dma_mask pointer is always initialized for platform devices. Not doing so lead to lots of boilerplate code, and makes platform devices different from all our major busses like PCI where we always set up a dma_mask. In the long run this should also help to eventually make dma_mask a scalar value instead of a pointer and remove even more cruft. The bigger blocker for this last time was the fact that the usb subsystem uses the presence or lack of a dma_mask to check if the core should do dma mapping for the driver, which is highly unusual. So we fix this first. Note that this has some overlap with the pending desire to use the proper dma_mmap_coherent helper for mapping usb buffers. The first two patches have already been queued up by Greg and are only included for completeness. Changes since v1: - fix a compile error in the ppc of ohci driver - revamp the last patch to get rid of the archdata callout entirely.
Re: [PATCH 6/6] driver core: initialize a default DMA mask for platform device
On Thu, Aug 15, 2019 at 03:03:25PM +0200, Greg Kroah-Hartman wrote:
> > --- a/include/linux/platform_device.h
> > +++ b/include/linux/platform_device.h
> > @@ -24,6 +24,7 @@ struct platform_device {
> > int id;
> > boolid_auto;
> > struct device dev;
> > + u64 dma_mask;
>
> Why is the dma_mask in 'struct device' which is part of this structure,
> not sufficient here? Shouldn't the "platform" be setting that up
> correctly already in the "archdata" type callback?
Becaus the dma_mask in struct device is a pointer that needs to point
to something, and this is the best space we can allocate for 'something'.
m68k and powerpc currently do something roughly equivalent at the moment,
while everyone else just has horrible, horrible hacks. As mentioned in
the changelog the intent of this patch is that we treat platform devices
like any other bus, where the bus allocates the space for the dma_mask.
The long term plan is to eventually kill that weird pointer indirection
that doesn't help anyone, but for that we need to sort out the basics
first.
Re: [PATCH 6/6] driver core: initialize a default DMA mask for platform device
On Wed, Aug 14, 2019 at 04:49:13PM +0100, Robin Murphy wrote:
>> because we have to support platform_device structures that are
>> statically allocated.
>
> This would be a good point to also get rid of the long-standing bodge in
> platform_device_register_full().
platform_device_register_full looks odd to start with, especially
as the coumentation is rather lacking..
>> +static void setup_pdev_archdata(struct platform_device *pdev)
>
> Bikeshed: painting the generic DMA API properties as "archdata" feels a bit
> off-target :/
>
>> +{
>> +if (!pdev->dev.coherent_dma_mask)
>> +pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>> +if (!pdev->dma_mask)
>> +pdev->dma_mask = DMA_BIT_MASK(32);
>> +if (!pdev->dev.dma_mask)
>> +pdev->dev.dma_mask = &pdev->dma_mask;
>> +arch_setup_pdev_archdata(pdev);
>
> AFAICS m68k's implementation of that arch hook becomes entirely redundant
> after this change, so may as well go. That would just leave powerpc's
> actual archdata, which at a glance looks like it could probably be cleaned
> up with not *too* much trouble.
Actually I think we can just kill both off. At the point archdata
is indeed entirely misnamed.
Re: next take at setting up a dma mask by default for platform devices
On Thu, Aug 15, 2019 at 03:23:18PM +0200, Greg Kroah-Hartman wrote: > I've taken the first 2 patches for 5.3-final. Given that patch 3 needs > to be fixed, I'll wait for a respin of these before considering them. I have a respun version ready, but I'd really like to hear some comments from usb developers about the approach before spamming everyone again..
Re: [PATCH v5 0/2] usbip: Implement SG support
FYI, I think my "usb: add a HCD_DMA flag instead of guestimating DMA capabilities" is the proper core fix for what your patch 1 works around, as the USB core should not assume DMA capabilities based on the presence of a DMA mask.
Re: [PATCH 3/6] usb: add a HCD_DMA flag instead of guestimating DMA capabilities
> diff --git a/drivers/usb/host/ehci-ppc-of.c b/drivers/usb/host/ehci-ppc-of.c
> index 576f7d79ad4e..9d17e0695e35 100644
> --- a/drivers/usb/host/ehci-ppc-of.c
> +++ b/drivers/usb/host/ehci-ppc-of.c
> @@ -31,7 +31,7 @@ static const struct hc_driver ehci_ppc_of_hc_driver = {
>* generic hardware linkage
>*/
> .irq= ehci_irq,
> - .flags = HCD_MEMORY | HCD_USB2 | HCD_BH,
> + .flags = HCD_MEMORY | HC_DMA | HCD_USB2 | HCD_BH,
FYI, the kbuild bot found a little typo here, so even for the unlikely
case that the series is otherwise perfect I'll have to resend it at
least once.
[PATCH 3/6] usb: add a HCD_DMA flag instead of guestimating DMA capabilities
The usb core is the only major place in the kernel that checks for
a non-NULL device dma_mask to see if a device is DMA capable. This
is generally a bad idea, as all major busses always set up a DMA mask,
even if the device is not DMA capable - in fact bus layers like PCI
can't even know if a device is DMA capable at enumeration time. This
leads to lots of workaround in HCD drivers, and also prevented us from
setting up a DMA mask for platform devices by default last time we
tried.
Replace this guess with an explicit HCD_DMA that is set by drivers that
appear to have DMA support.
Signed-off-by: Christoph Hellwig
---
drivers/staging/octeon-usb/octeon-hcd.c | 2 +-
drivers/usb/core/hcd.c | 1 -
drivers/usb/dwc2/hcd.c | 6 +++---
drivers/usb/host/ehci-grlib.c | 2 +-
drivers/usb/host/ehci-hcd.c | 2 +-
drivers/usb/host/ehci-pmcmsp.c | 2 +-
drivers/usb/host/ehci-ppc-of.c | 2 +-
drivers/usb/host/ehci-ps3.c | 2 +-
drivers/usb/host/ehci-sh.c | 2 +-
drivers/usb/host/ehci-xilinx-of.c | 2 +-
drivers/usb/host/fhci-hcd.c | 2 +-
drivers/usb/host/fotg210-hcd.c | 2 +-
drivers/usb/host/imx21-hcd.c| 2 +-
drivers/usb/host/isp116x-hcd.c | 6 --
drivers/usb/host/isp1362-hcd.c | 5 -
drivers/usb/host/ohci-hcd.c | 2 +-
drivers/usb/host/ohci-ppc-of.c | 2 +-
drivers/usb/host/ohci-ps3.c | 2 +-
drivers/usb/host/ohci-sa.c | 2 +-
drivers/usb/host/ohci-sm501.c | 2 +-
drivers/usb/host/ohci-tmio.c| 2 +-
drivers/usb/host/oxu210hp-hcd.c | 3 ---
drivers/usb/host/r8a66597-hcd.c | 6 --
drivers/usb/host/sl811-hcd.c| 6 --
drivers/usb/host/u132-hcd.c | 2 --
drivers/usb/host/uhci-grlib.c | 2 +-
drivers/usb/host/uhci-pci.c | 2 +-
drivers/usb/host/uhci-platform.c| 2 +-
drivers/usb/host/xhci.c | 2 +-
drivers/usb/isp1760/isp1760-core.c | 3 ---
drivers/usb/isp1760/isp1760-if.c| 1 -
drivers/usb/musb/musb_host.c| 2 +-
drivers/usb/renesas_usbhs/mod_host.c| 2 +-
include/linux/usb.h | 1 -
include/linux/usb/hcd.h | 7 +--
35 files changed, 31 insertions(+), 62 deletions(-)
diff --git a/drivers/staging/octeon-usb/octeon-hcd.c
b/drivers/staging/octeon-usb/octeon-hcd.c
index cd2b777073c4..a5321cc692c5 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -3512,7 +3512,7 @@ static const struct hc_driver octeon_hc_driver = {
.product_desc = "Octeon Host Controller",
.hcd_priv_size = sizeof(struct octeon_hcd),
.irq= octeon_usb_irq,
- .flags = HCD_MEMORY | HCD_USB2,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB2,
.start = octeon_usb_start,
.stop = octeon_usb_stop,
.urb_enqueue= octeon_usb_urb_enqueue,
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 8592c0344fe8..add2af4af766 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2454,7 +2454,6 @@ struct usb_hcd *__usb_create_hcd(const struct hc_driver
*driver,
hcd->self.controller = dev;
hcd->self.sysdev = sysdev;
hcd->self.bus_name = bus_name;
- hcd->self.uses_dma = (sysdev->dma_mask != NULL);
timer_setup(&hcd->rh_timer, rh_timer_func, 0);
#ifdef CONFIG_PM
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 111787a137ee..81afe553aa66 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -5062,13 +5062,13 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg)
dwc2_hc_driver.reset_device = dwc2_reset_device;
}
+ if (hsotg->params.host_dma)
+ dwc2_hc_driver.flags |= HCD_DMA;
+
hcd = usb_create_hcd(&dwc2_hc_driver, hsotg->dev, dev_name(hsotg->dev));
if (!hcd)
goto error1;
- if (!hsotg->params.host_dma)
- hcd->self.uses_dma = 0;
-
hcd->has_tt = 1;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
diff --git a/drivers/usb/host/ehci-grlib.c b/drivers/usb/host/ehci-grlib.c
index 656b8c08efc8..a2c3b4ec8a8b 100644
--- a/drivers/usb/host/ehci-grlib.c
+++ b/drivers/usb/host/ehci-grlib.c
@@ -30,7 +30,7 @@ static const struct hc_driver ehci_grlib_hc_driver = {
* generic hardware linkage
*/
.irq= ehci_irq,
- .flags = HCD_MEMORY | HCD_USB2 | HCD_BH,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB2 | HCD_BH,
/*
* basic lifecycle operations
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.
[PATCH 5/6] dma-mapping: remove is_device_dma_capable
No users left.
Signed-off-by: Christoph Hellwig
---
include/linux/dma-mapping.h | 5 -
1 file changed, 5 deletions(-)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index f7d1eea32c78..14702e2d6fa8 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -149,11 +149,6 @@ static inline int valid_dma_direction(int dma_direction)
(dma_direction == DMA_FROM_DEVICE));
}
-static inline int is_device_dma_capable(struct device *dev)
-{
- return dev->dma_mask != NULL && *dev->dma_mask != DMA_MASK_NONE;
-}
-
#ifdef CONFIG_DMA_DECLARE_COHERENT
/*
* These three functions are only for dma allocator.
--
2.20.1
[PATCH 4/6] usb/max3421: remove the dummy {un,}map_urb_for_dma methods
Now that we have an explicit HCD_DMA flag, there is not need to override
these methods.
Signed-off-by: Christoph Hellwig
---
drivers/usb/host/max3421-hcd.c | 17 -
1 file changed, 17 deletions(-)
diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index afa321ab55fc..8819f502b6a6 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -1800,21 +1800,6 @@ max3421_bus_resume(struct usb_hcd *hcd)
return -1;
}
-/*
- * The SPI driver already takes care of DMA-mapping/unmapping, so no
- * reason to do it twice.
- */
-static int
-max3421_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags)
-{
- return 0;
-}
-
-static void
-max3421_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
-{
-}
-
static const struct hc_driver max3421_hcd_desc = {
.description = "max3421",
.product_desc = DRIVER_DESC,
@@ -1826,8 +1811,6 @@ static const struct hc_driver max3421_hcd_desc = {
.get_frame_number = max3421_get_frame_number,
.urb_enqueue = max3421_urb_enqueue,
.urb_dequeue = max3421_urb_dequeue,
- .map_urb_for_dma = max3421_map_urb_for_dma,
- .unmap_urb_for_dma =max3421_unmap_urb_for_dma,
.endpoint_disable = max3421_endpoint_disable,
.hub_status_data = max3421_hub_status_data,
.hub_control = max3421_hub_control,
--
2.20.1
[PATCH 2/6] usb: add a hcd_uses_dma helper
The USB buffer allocation code is the only place in the usb core (and in
fact the whole kernel) that uses is_device_dma_capable, while the URB
mapping code uses the uses_dma flag in struct usb_bus. Switch the buffer
allocation to use the uses_dma flag used by the rest of the USB code,
and create a helper in hcd.h that checks this flag as well as the
CONFIG_HAS_DMA to simplify the caller a bit.
Signed-off-by: Christoph Hellwig
---
drivers/usb/core/buffer.c | 10 +++---
drivers/usb/core/hcd.c| 4 ++--
drivers/usb/dwc2/hcd.c| 2 +-
include/linux/usb.h | 2 +-
include/linux/usb/hcd.h | 3 +++
5 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index 1a5b3dcae930..6cf22c27f2d2 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -66,9 +66,7 @@ int hcd_buffer_create(struct usb_hcd *hcd)
charname[16];
int i, size;
- if (hcd->localmem_pool ||
- !IS_ENABLED(CONFIG_HAS_DMA) ||
- !is_device_dma_capable(hcd->self.sysdev))
+ if (hcd->localmem_pool || !hcd_uses_dma(hcd))
return 0;
for (i = 0; i < HCD_BUFFER_POOLS; i++) {
@@ -129,8 +127,7 @@ void *hcd_buffer_alloc(
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)) {
+ if (!hcd_uses_dma(hcd)) {
*dma = ~(dma_addr_t) 0;
return kmalloc(size, mem_flags);
}
@@ -160,8 +157,7 @@ void hcd_buffer_free(
return;
}
- if (!IS_ENABLED(CONFIG_HAS_DMA) ||
- !is_device_dma_capable(bus->sysdev)) {
+ if (!hcd_uses_dma(hcd)) {
kfree(addr);
return;
}
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 2ccbc2f83570..8592c0344fe8 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1412,7 +1412,7 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct
urb *urb,
if (usb_endpoint_xfer_control(&urb->ep->desc)) {
if (hcd->self.uses_pio_for_control)
return ret;
- if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
+ if (hcd_uses_dma(hcd)) {
if (is_vmalloc_addr(urb->setup_packet)) {
WARN_ONCE(1, "setup packet is not dma
capable\n");
return -EAGAIN;
@@ -1446,7 +1446,7 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct
urb *urb,
dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
if (urb->transfer_buffer_length != 0
&& !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
- if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
+ if (hcd_uses_dma(hcd)) {
if (urb->num_sgs) {
int n;
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index ee144ff8af5b..111787a137ee 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -4608,7 +4608,7 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd,
struct urb *urb,
buf = urb->transfer_buffer;
- if (hcd->self.uses_dma) {
+ if (hcd_uses_dma(hcd)) {
if (!buf && (urb->transfer_dma & 3)) {
dev_err(hsotg->dev,
"%s: unaligned transfer with no
transfer_buffer",
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 83d35d993e8c..e87826e23d59 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1457,7 +1457,7 @@ typedef void (*usb_complete_t)(struct urb *);
* field rather than determining a dma address themselves.
*
* Note that transfer_buffer must still be set if the controller
- * does not support DMA (as indicated by bus.uses_dma) and when talking
+ * does not support DMA (as indicated by hcd_uses_dma()) and when talking
* to root hub. If you have to trasfer between highmem zone and the device
* on such controller, create a bounce buffer or bail out with an error.
* If transfer_buffer cannot be set (is in highmem) and the controller is DMA
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index bab27ccc8ff5..a20e7815d814 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -422,6 +422,9 @@ static inline bool
hcd_periodic_completion_in_progress(struct usb_hcd *hcd,
return hcd->high_prio_bh.completing_ep == ep;
}
+#define hcd_uses_dma(hcd) \
+ (IS_ENABLED(CONFIG_HAS_DMA) && (hcd)->self.uses_dma)
+
extern int usb_hcd_link_urb_to_ep(struct usb_hcd *hcd, struct urb *urb);
extern int usb_hcd_check_unlink_urb(struct usb_hcd *hcd, struct urb *urb,
int status);
--
2.20.1
next take at setting up a dma mask by default for platform devices
Hi all, this is another attempt to make sure the dma_mask pointer is always initialized for platform devices. Not doing so lead to lots of boilerplate code, and makes platform devices different from all our major busses like PCI where we always set up a dma_mask. In the long run this should also help to eventually make dma_mask a scalar value instead of a pointer and remove even more cruft. The bigger blocker for this last time was the fact that the usb subsystem uses the presence or lack of a dma_mask to check if the core should do dma mapping for the driver, which is highly unusual. So we fix this first. Note that this has some overlap with the pending desire to use the proper dma_mmap_coherent helper for mapping usb buffers. The first two patches from this series should probably go into 5.3 and then uses as the basis for the decision to use dma_mmap_coherent.
[PATCH 6/6] driver core: initialize a default DMA mask for platform device
We still treat devices without a DMA mask as defaulting to 32-bits for
both mask, but a few releases ago we've started warning about such
cases, as they require special cases to work around this sloppyness.
Add a dma_mask field to struct platform_object so that we can initialize
the dma_mask pointer in struct device and initialize both masks to
32-bits by default. Architectures can still override this in
arch_setup_pdev_archdata if needed.
Note that the code looks a little odd with the various conditionals
because we have to support platform_device structures that are
statically allocated.
Signed-off-by: Christoph Hellwig
---
drivers/base/platform.c | 15 +--
include/linux/platform_device.h | 1 +
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index ec974ba9c0c4..b216fcb0a8af 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -264,6 +264,17 @@ struct platform_object {
char name[];
};
+static void setup_pdev_archdata(struct platform_device *pdev)
+{
+ if (!pdev->dev.coherent_dma_mask)
+ pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+ if (!pdev->dma_mask)
+ pdev->dma_mask = DMA_BIT_MASK(32);
+ if (!pdev->dev.dma_mask)
+ pdev->dev.dma_mask = &pdev->dma_mask;
+ arch_setup_pdev_archdata(pdev);
+};
+
/**
* platform_device_put - destroy a platform device
* @pdev: platform device to free
@@ -310,7 +321,7 @@ struct platform_device *platform_device_alloc(const char
*name, int id)
pa->pdev.id = id;
device_initialize(&pa->pdev.dev);
pa->pdev.dev.release = platform_device_release;
- arch_setup_pdev_archdata(&pa->pdev);
+ setup_pdev_archdata(&pa->pdev);
}
return pa ? &pa->pdev : NULL;
@@ -512,7 +523,7 @@ EXPORT_SYMBOL_GPL(platform_device_del);
int platform_device_register(struct platform_device *pdev)
{
device_initialize(&pdev->dev);
- arch_setup_pdev_archdata(pdev);
+ setup_pdev_archdata(pdev);
return platform_device_add(pdev);
}
EXPORT_SYMBOL_GPL(platform_device_register);
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 9bc36b589827..a2abde2aef25 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -24,6 +24,7 @@ struct platform_device {
int id;
boolid_auto;
struct device dev;
+ u64 dma_mask;
u32 num_resources;
struct resource *resource;
--
2.20.1
[PATCH 1/6] usb: don't create dma pools for HCDs with a localmem_pool
If the HCD provides a localmem pool we will never use the DMA pools, so
don't create them.
Fixes: b0310c2f09bb ("USB: use genalloc for USB HCs with local memory")
Signed-off-by: Christoph Hellwig
---
drivers/usb/core/buffer.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index 1359b78a624e..1a5b3dcae930 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -66,9 +66,9 @@ int hcd_buffer_create(struct usb_hcd *hcd)
charname[16];
int i, size;
- if (!IS_ENABLED(CONFIG_HAS_DMA) ||
- (!is_device_dma_capable(hcd->self.sysdev) &&
-!hcd->localmem_pool))
+ if (hcd->localmem_pool ||
+ !IS_ENABLED(CONFIG_HAS_DMA) ||
+ !is_device_dma_capable(hcd->self.sysdev))
return 0;
for (i = 0; i < HCD_BUFFER_POOLS; i++) {
--
2.20.1
Re: [PATCH 03/22] ARM: omap1: move omap15xx local bus handling to usb.c
Thanks for doing this! The odd platforms have always been very
confusing.
> diff --git a/arch/arm/mach-omap1/include/mach/omap1510.h
> b/arch/arm/mach-omap1/include/mach/omap1510.h
> index 3d235244bf5c..7af9c0c7c5ab 100644
> --- a/arch/arm/mach-omap1/include/mach/omap1510.h
> +++ b/arch/arm/mach-omap1/include/mach/omap1510.h
> @@ -159,4 +159,3 @@
> #define OMAP1510_INT_FPGA23 (OMAP_FPGA_IRQ_BASE + 23)
>
> #endif /* __ASM_ARCH_OMAP15XX_H */
> -
Spurious whitespace change?
> diff --git a/arch/arm/mach-omap1/usb.c b/arch/arm/mach-omap1/usb.c
> index d8e9bbda8f7b..740c876ae46b 100644
> --- a/arch/arm/mach-omap1/usb.c
> +++ b/arch/arm/mach-omap1/usb.c
> @@ -10,6 +10,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -127,6 +128,7 @@ omap_otg_init(struct omap_usb_config *config)
>
> syscon &= ~HST_IDLE_EN;
> ohci_device->dev.platform_data = config;
> +
> status = platform_device_register(ohci_device);
Same here.
> +#define OMAP1510_LB_OFFSET UL(0x3000)
> +#define OMAP1510_LB_DMA_PFN_OFFSET ((OMAP1510_LB_OFFSET - PAGE_OFFSET) >>
> PAGE_SHIFT)
Overly long line.
> +/*
> + * OMAP-1510 specific Local Bus clock on/off
> + */
> +static int omap_1510_local_bus_power(int on)
> +{
> + if (on) {
> + omap_writel((1 << 1) | (1 << 0), OMAP1510_LB_MMU_CTL);
> + udelay(200);
> + } else {
> + omap_writel(0, OMAP1510_LB_MMU_CTL);
> + }
> +
> + return 0;
> +}
The caller never checks the const return value, and on is always true as
well. In fact it seems like omap_1510_local_bus_power and
omap_1510_local_bus_init could probably just be merged into the caller.
> +
> +/*
> + * OMAP-1510 specific Local Bus initialization
> + * NOTE: This assumes 32MB memory size in OMAP1510LB_MEMSIZE.
> + * See also arch/mach-omap/memory.h for __virt_to_dma() and
> + * __dma_to_virt() which need to match with the physical
> + * Local Bus address below.
I think that NOTE is out of date, as __virt_to_dma and __dma_to_virt
don't exist anymore.
> +static int omap_1510_local_bus_init(void)
> +{
> + unsigned int tlb;
> + unsigned long lbaddr, physaddr;
> +
> + omap_writel((omap_readl(OMAP1510_LB_CLOCK_DIV) & 0xfff8) | 0x4,
> +OMAP1510_LB_CLOCK_DIV);
> +
> + /* Configure the Local Bus MMU table */
> + for (tlb = 0; tlb < OMAP1510_LB_MEMSIZE; tlb++) {
> + lbaddr = tlb * 0x0010 + OMAP1510_LB_OFFSET;
> + physaddr = tlb * 0x0010 + PHYS_OFFSET;
> + omap_writel((lbaddr & 0x0fff) >> 22, OMAP1510_LB_MMU_CAM_H);
> + omap_writel(((lbaddr & 0x003ffc00) >> 6) | 0xc,
> +OMAP1510_LB_MMU_CAM_L);
> + omap_writel(physaddr >> 16, OMAP1510_LB_MMU_RAM_H);
> + omap_writel((physaddr & 0xfc00) | 0x300,
> OMAP1510_LB_MMU_RAM_L);
Another > 80 chars line.
> + omap_writel(tlb << 4, OMAP1510_LB_MMU_LCK);
> + omap_writel(0x1, OMAP1510_LB_MMU_LD_TLB);
> + }
> +
> + /* Enable the walking table */
> + omap_writel(omap_readl(OMAP1510_LB_MMU_CTL) | (1 << 3),
> OMAP1510_LB_MMU_CTL);
One more.
> + udelay(200);
> +
> + return 0;
The return value is ignored.
Re: usb zero copy dma handling
On Thu, Aug 08, 2019 at 09:10:15AM -0700, Christoph Hellwig wrote:
> On Thu, Aug 08, 2019 at 10:46:36AM +0200, yvahkhfo.1df7f...@hashmail.org
> wrote:
> > --- a/drivers/usb/core/devio.c
> > +++ b/drivers/usb/core/devio.c
> > @@ -238,9 +238,14 @@ static int usbdev_mmap(struct file *file, struct
> > vm_area_struct *vma)
> > usbm->vma_use_count = 1;
> > INIT_LIST_HEAD(&usbm->memlist);
> >
> > +#ifdef CONFIG_X86
> > if (remap_pfn_range(vma, vma->vm_start,
> > virt_to_phys(usbm->mem) >> PAGE_SHIFT,
> > size, vma->vm_page_prot) < 0) {
> > +#else /* !CONFIG_X86 */
> > + if (dma_mmap_coherent(ps->dev->bus->sysdev,
> > + vma, mem, dma_handle, size) < 0) {
> > +#endif /* !CONFIG_X86 */
>
> Doing the dma_mmap_coherent unconditionally is the right thing here.
> Gavin who is on Cc has been looking into that.
Ok, tht is assuming it always is dma_alloc_* memory which apparently
it isn't. But the arch ifdef for sure is wrong.
Re: usb zero copy dma handling
On Thu, Aug 08, 2019 at 10:46:36AM +0200, yvahkhfo.1df7f...@hashmail.org wrote:
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -238,9 +238,14 @@ static int usbdev_mmap(struct file *file, struct
> vm_area_struct *vma)
> usbm->vma_use_count = 1;
> INIT_LIST_HEAD(&usbm->memlist);
>
> +#ifdef CONFIG_X86
> if (remap_pfn_range(vma, vma->vm_start,
> virt_to_phys(usbm->mem) >> PAGE_SHIFT,
> size, vma->vm_page_prot) < 0) {
> +#else /* !CONFIG_X86 */
> + if (dma_mmap_coherent(ps->dev->bus->sysdev,
> + vma, mem, dma_handle, size) < 0) {
> +#endif /* !CONFIG_X86 */
Doing the dma_mmap_coherent unconditionally is the right thing here.
Gavin who is on Cc has been looking into that.
Note that you'll also need this patch which I'm going to send to Linus
this week before it properly works on x86:
http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/197b3e665b82c6027be5c68a143233df7ce5224f
Re: [PATCH v3] usb-storage: Add a limitation for blk_queue_max_hw_sectors()
Looks good: Reviewed-by: Christoph Hellwig
Re: [RFC PATCH v2 1/3] USB: use genalloc for USB HCs with local memory
On Wed, May 15, 2019 at 09:57:12AM +, 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=086d08725d34c6bdb710344ae9c4fdafb2d5 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.
Re: [RFC PATCH v2 1/3] USB: use genalloc for USB HCs with local memory
> @@ -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!
Re: [PATCH 01/18] MIPS: lantiq: pass struct device to DMA API functions
On Thu, Feb 07, 2019 at 11:29:14PM +, Paul Burton wrote: > Would you like this to go through the MIPS tree or elsewhere? If the > latter: > > Acked-by: Paul Burton Please pick it up through the mips tree!
Re: [PATCH 12/18] fotg210-udc: remove a bogus dma_sync_single_for_device call
On Fri, Feb 01, 2019 at 05:10:26PM +0100, Christoph Hellwig wrote: > On Fri, Feb 01, 2019 at 03:19:41PM +0200, Felipe Balbi wrote: > > Christoph Hellwig writes: > > > > > dma_map_single already transfers ownership to the device. > > > > > > Signed-off-by: Christoph Hellwig > > > > Do you want me to take the USB bits or will you take the entire series? > > In case you're taking the entire series: > > If you want to take the USB feel free. I just want most of this in > this merge window if possible. I didn't see in the USB tree yet, so please let me know if you want to take it.
Re: [PATCH 02/18] dmaengine: imx-sdma: pass struct device to DMA API functions
On Sat, Feb 02, 2019 at 03:41:21PM +0530, Vinod Koul wrote: > On 01-02-19, 09:47, Christoph Hellwig wrote: > > The DMA API generally relies on a struct device to work properly, and > > only barely works without one for legacy reasons. Pass the easily > > available struct device from the platform_device to remedy this. > > This looks good to me but fails to apply. Can you please base it on > dmaengine-next or linux-next please and resend commit ceaf52265148d3a5ca24237fd1c709caa5f46184 Author: Andy Duan Date: Fri Jan 11 14:29:49 2019 + dmaengine: imx-sdma: pass ->dev to dma_alloc_coherent() API in linux-next actually is equivalent to this patch, so we can drop this one. > > Thanks > -- > ~Vinod ---end quoted text---
Re: [PATCH 10/18] smc911x: pass struct device to DMA API functions
On Fri, Feb 01, 2019 at 02:14:34PM +, Robin Murphy wrote: > And equivalently for rxdma here. However, given that this all seems only > relevant to antique ARCH_PXA platforms which are presumably managing to > work as-is, it's probably not worth tinkering too much. I'd just stick a > note in the commit message that we're still only making these > self-consistent with the existing dma_map_single() calls rather than > necessarily correct. Sounds good.
Re: [PATCH 03/18] net: caif: pass struct device to DMA API functions
On Fri, Feb 01, 2019 at 01:53:09PM +, Robin Murphy wrote: >> #define LOW_WATER_MARK 100 >> #define HIGH_WATER_MARK (LOW_WATER_MARK*5) >> -#ifdef CONFIG_UML >> +#ifdef CONFIG_HAS_DMA > > #ifndef, surely? Indeed.
Re: [alsa-devel] don't pass a NULL struct device to DMA API functions
On Fri, Feb 01, 2019 at 02:16:08PM +0100, Takashi Iwai wrote: > Actually there are a bunch of ISA sound drivers that still call > allocators with NULL device. > > The patch below should address it, although it's only compile-tested. Oh, I missed these "indirect" calls. This looks good to me: Reviewed-by: Christoph Hellwig
Re: [PATCH 12/18] fotg210-udc: remove a bogus dma_sync_single_for_device call
On Fri, Feb 01, 2019 at 03:19:41PM +0200, Felipe Balbi wrote: > Christoph Hellwig writes: > > > dma_map_single already transfers ownership to the device. > > > > Signed-off-by: Christoph Hellwig > > Do you want me to take the USB bits or will you take the entire series? > In case you're taking the entire series: If you want to take the USB feel free. I just want most of this in this merge window if possible.
Re: [alsa-devel] [PATCH 17/18] ALSA: hal2: pass struct device to DMA API functions
On Fri, Feb 01, 2019 at 02:12:45PM +0100, Takashi Iwai wrote: > Shall I take this one through sound git tree or all through yours? Feel free to merge the sound bits through your tree!
[PATCH 05/18] macb_main: pass struct device to DMA API functions
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Signed-off-by: Christoph Hellwig
---
drivers/net/ethernet/cadence/macb_main.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c
b/drivers/net/ethernet/cadence/macb_main.c
index 2b2882615e8b..61a27963f1d1 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -3673,9 +3673,9 @@ static netdev_tx_t at91ether_start_xmit(struct sk_buff
*skb,
/* Store packet information (to free when Tx completed) */
lp->skb = skb;
lp->skb_length = skb->len;
- lp->skb_physaddr = dma_map_single(NULL, skb->data, skb->len,
- DMA_TO_DEVICE);
- if (dma_mapping_error(NULL, lp->skb_physaddr)) {
+ lp->skb_physaddr = dma_map_single(&lp->pdev->dev, skb->data,
+ skb->len, DMA_TO_DEVICE);
+ if (dma_mapping_error(&lp->pdev->dev, lp->skb_physaddr)) {
dev_kfree_skb_any(skb);
dev->stats.tx_dropped++;
netdev_err(dev, "%s: DMA mapping error\n", __func__);
@@ -3765,7 +3765,7 @@ static irqreturn_t at91ether_interrupt(int irq, void
*dev_id)
if (lp->skb) {
dev_kfree_skb_irq(lp->skb);
lp->skb = NULL;
- dma_unmap_single(NULL, lp->skb_physaddr,
+ dma_unmap_single(&lp->pdev->dev, lp->skb_physaddr,
lp->skb_length, DMA_TO_DEVICE);
dev->stats.tx_packets++;
dev->stats.tx_bytes += lp->skb_length;
--
2.20.1
[PATCH 15/18] gbefb: switch to managed version of the DMA allocator
gbefb uses managed resources, so it should do the same for DMA
allocations.
Signed-off-by: Christoph Hellwig
---
drivers/video/fbdev/gbefb.c | 24
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/drivers/video/fbdev/gbefb.c b/drivers/video/fbdev/gbefb.c
index 1a242b1338e9..3fcb33232ba3 100644
--- a/drivers/video/fbdev/gbefb.c
+++ b/drivers/video/fbdev/gbefb.c
@@ -1162,9 +1162,9 @@ static int gbefb_probe(struct platform_device *p_dev)
}
gbe_revision = gbe->ctrlstat & 15;
- gbe_tiles.cpu =
- dma_alloc_coherent(NULL, GBE_TLB_SIZE * sizeof(uint16_t),
- &gbe_tiles.dma, GFP_KERNEL);
+ gbe_tiles.cpu = dmam_alloc_coherent(&p_dev->dev,
+ GBE_TLB_SIZE * sizeof(uint16_t),
+ &gbe_tiles.dma, GFP_KERNEL);
if (!gbe_tiles.cpu) {
printk(KERN_ERR "gbefb: couldn't allocate tiles table\n");
ret = -ENOMEM;
@@ -1178,19 +1178,20 @@ static int gbefb_probe(struct platform_device *p_dev)
if (!gbe_mem) {
printk(KERN_ERR "gbefb: couldn't map framebuffer\n");
ret = -ENOMEM;
- goto out_tiles_free;
+ goto out_release_mem_region;
}
gbe_dma_addr = 0;
} else {
/* try to allocate memory with the classical allocator
* this has high chance to fail on low memory machines */
- gbe_mem = dma_alloc_wc(NULL, gbe_mem_size, &gbe_dma_addr,
- GFP_KERNEL);
+ gbe_mem = dmam_alloc_attrs(&p_dev->dev, gbe_mem_size,
+ &gbe_dma_addr, GFP_KERNEL,
+ DMA_ATTR_WRITE_COMBINE);
if (!gbe_mem) {
printk(KERN_ERR "gbefb: couldn't allocate framebuffer
memory\n");
ret = -ENOMEM;
- goto out_tiles_free;
+ goto out_release_mem_region;
}
gbe_mem_phys = (unsigned long) gbe_dma_addr;
@@ -1237,11 +1238,6 @@ static int gbefb_probe(struct platform_device *p_dev)
out_gbe_unmap:
arch_phys_wc_del(par->wc_cookie);
- if (gbe_dma_addr)
- dma_free_wc(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
-out_tiles_free:
- dma_free_coherent(NULL, GBE_TLB_SIZE * sizeof(uint16_t),
- (void *)gbe_tiles.cpu, gbe_tiles.dma);
out_release_mem_region:
release_mem_region(GBE_BASE, sizeof(struct sgi_gbe));
out_release_framebuffer:
@@ -1258,10 +1254,6 @@ static int gbefb_remove(struct platform_device* p_dev)
unregister_framebuffer(info);
gbe_turn_off();
arch_phys_wc_del(par->wc_cookie);
- if (gbe_dma_addr)
- dma_free_wc(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
- dma_free_coherent(NULL, GBE_TLB_SIZE * sizeof(uint16_t),
- (void *)gbe_tiles.cpu, gbe_tiles.dma);
release_mem_region(GBE_BASE, sizeof(struct sgi_gbe));
gbefb_remove_sysfs(&p_dev->dev);
framebuffer_release(info);
--
2.20.1
[PATCH 01/18] MIPS: lantiq: pass struct device to DMA API functions
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Also use GFP_KERNEL instead of GFP_ATOMIC as the gfp_t for the memory
allocation, as we aren't in interrupt context or under a lock.
Note that this whole function looks somewhat bogus given that we never
even look at the returned dma address, and the CPHYSADDR magic on
a returned noncached mapping looks "interesting". But I'll leave
that to people more familiar with the code to sort out.
Signed-off-by: Christoph Hellwig
---
arch/mips/lantiq/xway/vmmc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/mips/lantiq/xway/vmmc.c b/arch/mips/lantiq/xway/vmmc.c
index 577ec81b557d..3deab9a77718 100644
--- a/arch/mips/lantiq/xway/vmmc.c
+++ b/arch/mips/lantiq/xway/vmmc.c
@@ -31,8 +31,8 @@ static int vmmc_probe(struct platform_device *pdev)
dma_addr_t dma;
cp1_base =
- (void *) CPHYSADDR(dma_alloc_coherent(NULL, CP1_SIZE,
- &dma, GFP_ATOMIC));
+ (void *) CPHYSADDR(dma_alloc_coherent(&pdev->dev, CP1_SIZE,
+ &dma, GFP_KERNEL));
gpio_count = of_gpio_count(pdev->dev.of_node);
while (gpio_count > 0) {
--
2.20.1
[PATCH 04/18] au1000_eth: pass struct device to DMA API functions
The DMA API generally relies on a struct device to work properly, and only barely works without one for legacy reasons. Pass the easily available struct device from the platform_device to remedy this. Signed-off-by: Christoph Hellwig --- drivers/net/ethernet/amd/au1000_eth.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/amd/au1000_eth.c b/drivers/net/ethernet/amd/au1000_eth.c index e833d1b3fe18..e5073aeea06a 100644 --- a/drivers/net/ethernet/amd/au1000_eth.c +++ b/drivers/net/ethernet/amd/au1000_eth.c @@ -1167,7 +1167,7 @@ static int au1000_probe(struct platform_device *pdev) /* Allocate the data buffers * Snooping works fine with eth on all au1xxx */ - aup->vaddr = (u32)dma_alloc_attrs(NULL, MAX_BUF_SIZE * + aup->vaddr = (u32)dma_alloc_attrs(&pdev->dev, MAX_BUF_SIZE * (NUM_TX_BUFFS + NUM_RX_BUFFS), &aup->dma_addr, 0, DMA_ATTR_NON_CONSISTENT); @@ -1349,7 +1349,7 @@ static int au1000_probe(struct platform_device *pdev) err_remap2: iounmap(aup->mac); err_remap1: - dma_free_attrs(NULL, MAX_BUF_SIZE * (NUM_TX_BUFFS + NUM_RX_BUFFS), + dma_free_attrs(&pdev->dev, MAX_BUF_SIZE * (NUM_TX_BUFFS + NUM_RX_BUFFS), (void *)aup->vaddr, aup->dma_addr, DMA_ATTR_NON_CONSISTENT); err_vaddr: @@ -1383,7 +1383,7 @@ static int au1000_remove(struct platform_device *pdev) if (aup->tx_db_inuse[i]) au1000_ReleaseDB(aup, aup->tx_db_inuse[i]); - dma_free_attrs(NULL, MAX_BUF_SIZE * (NUM_TX_BUFFS + NUM_RX_BUFFS), + dma_free_attrs(&pdev->dev, MAX_BUF_SIZE * (NUM_TX_BUFFS + NUM_RX_BUFFS), (void *)aup->vaddr, aup->dma_addr, DMA_ATTR_NON_CONSISTENT); -- 2.20.1
[PATCH 18/18] ALSA: pass struct device to DMA API functions
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Also use GFP_KERNEL instead of GFP_USER as the gfp_t for the memory
allocation, as we should treat this allocation as a normal kernel one.
Signed-off-by: Christoph Hellwig
---
sound/mips/sgio2audio.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/mips/sgio2audio.c b/sound/mips/sgio2audio.c
index 3ec9391a4736..53a4ee01c522 100644
--- a/sound/mips/sgio2audio.c
+++ b/sound/mips/sgio2audio.c
@@ -805,7 +805,7 @@ static int snd_sgio2audio_free(struct snd_sgio2audio *chip)
free_irq(snd_sgio2_isr_table[i].irq,
&chip->channel[snd_sgio2_isr_table[i].idx]);
- dma_free_coherent(NULL, MACEISA_RINGBUFFERS_SIZE,
+ dma_free_coherent(chip->card->dev, MACEISA_RINGBUFFERS_SIZE,
chip->ring_base, chip->ring_base_dma);
/* release card data */
@@ -843,8 +843,9 @@ static int snd_sgio2audio_create(struct snd_card *card,
chip->card = card;
- chip->ring_base = dma_alloc_coherent(NULL, MACEISA_RINGBUFFERS_SIZE,
-&chip->ring_base_dma, GFP_USER);
+ chip->ring_base = dma_alloc_coherent(card->dev,
+MACEISA_RINGBUFFERS_SIZE,
+&chip->ring_base_dma, GFP_KERNEL);
if (chip->ring_base == NULL) {
printk(KERN_ERR
"sgio2audio: could not allocate ring buffers\n");
--
2.20.1
[PATCH 08/18] moxart_ether: pass struct device to DMA API functions
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Signed-off-by: Christoph Hellwig
---
drivers/net/ethernet/moxa/moxart_ether.c | 11 +++
drivers/net/ethernet/moxa/moxart_ether.h | 1 +
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/moxa/moxart_ether.c
b/drivers/net/ethernet/moxa/moxart_ether.c
index b34055ac476f..00dec0ffb11b 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -81,11 +81,13 @@ static void moxart_mac_free_memory(struct net_device *ndev)
priv->rx_buf_size, DMA_FROM_DEVICE);
if (priv->tx_desc_base)
- dma_free_coherent(NULL, TX_REG_DESC_SIZE * TX_DESC_NUM,
+ dma_free_coherent(&priv->pdev->dev,
+ TX_REG_DESC_SIZE * TX_DESC_NUM,
priv->tx_desc_base, priv->tx_base);
if (priv->rx_desc_base)
- dma_free_coherent(NULL, RX_REG_DESC_SIZE * RX_DESC_NUM,
+ dma_free_coherent(&priv->pdev->dev,
+ RX_REG_DESC_SIZE * RX_DESC_NUM,
priv->rx_desc_base, priv->rx_base);
kfree(priv->tx_buf_base);
@@ -476,6 +478,7 @@ static int moxart_mac_probe(struct platform_device *pdev)
priv = netdev_priv(ndev);
priv->ndev = ndev;
+ priv->pdev = pdev;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
ndev->base_addr = res->start;
@@ -491,7 +494,7 @@ static int moxart_mac_probe(struct platform_device *pdev)
priv->tx_buf_size = TX_BUF_SIZE;
priv->rx_buf_size = RX_BUF_SIZE;
- priv->tx_desc_base = dma_alloc_coherent(NULL, TX_REG_DESC_SIZE *
+ priv->tx_desc_base = dma_alloc_coherent(&pdev->dev, TX_REG_DESC_SIZE *
TX_DESC_NUM, &priv->tx_base,
GFP_DMA | GFP_KERNEL);
if (!priv->tx_desc_base) {
@@ -499,7 +502,7 @@ static int moxart_mac_probe(struct platform_device *pdev)
goto init_fail;
}
- priv->rx_desc_base = dma_alloc_coherent(NULL, RX_REG_DESC_SIZE *
+ priv->rx_desc_base = dma_alloc_coherent(&pdev->dev, RX_REG_DESC_SIZE *
RX_DESC_NUM, &priv->rx_base,
GFP_DMA | GFP_KERNEL);
if (!priv->rx_desc_base) {
diff --git a/drivers/net/ethernet/moxa/moxart_ether.h
b/drivers/net/ethernet/moxa/moxart_ether.h
index bee608b547d1..bf4c3029cd0c 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.h
+++ b/drivers/net/ethernet/moxa/moxart_ether.h
@@ -292,6 +292,7 @@
#define LINK_STATUS0x4
struct moxart_mac_priv_t {
+ struct platform_device *pdev;
void __iomem *base;
unsigned int reg_maccr;
unsigned int reg_imr;
--
2.20.1
[PATCH 16/18] pxa3xx-gcu: pass struct device to dma_mmap_coherent
Just like we do for all other DMA operations.
Signed-off-by: Christoph Hellwig
---
drivers/video/fbdev/pxa3xx-gcu.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/pxa3xx-gcu.c b/drivers/video/fbdev/pxa3xx-gcu.c
index 69cfb337c857..047a2fa4b87e 100644
--- a/drivers/video/fbdev/pxa3xx-gcu.c
+++ b/drivers/video/fbdev/pxa3xx-gcu.c
@@ -96,6 +96,7 @@ struct pxa3xx_gcu_batch {
};
struct pxa3xx_gcu_priv {
+ struct device*dev;
void __iomem *mmio_base;
struct clk *clk;
struct pxa3xx_gcu_shared *shared;
@@ -493,7 +494,7 @@ pxa3xx_gcu_mmap(struct file *file, struct vm_area_struct
*vma)
if (size != SHARED_SIZE)
return -EINVAL;
- return dma_mmap_coherent(NULL, vma,
+ return dma_mmap_coherent(priv->dev, vma,
priv->shared, priv->shared_phys, size);
case SHARED_SIZE >> PAGE_SHIFT:
@@ -670,6 +671,7 @@ static int pxa3xx_gcu_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, priv);
priv->resource_mem = r;
+ priv->dev = dev;
pxa3xx_gcu_reset(priv);
pxa3xx_gcu_init_debug_timer(priv);
--
2.20.1
[PATCH 14/18] da8xx-fb: pass struct device to DMA API functions
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Signed-off-by: Christoph Hellwig
---
drivers/video/fbdev/da8xx-fb.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/video/fbdev/da8xx-fb.c b/drivers/video/fbdev/da8xx-fb.c
index 43f2a4816860..ec62274b914b 100644
--- a/drivers/video/fbdev/da8xx-fb.c
+++ b/drivers/video/fbdev/da8xx-fb.c
@@ -1097,9 +1097,9 @@ static int fb_remove(struct platform_device *dev)
unregister_framebuffer(info);
fb_dealloc_cmap(&info->cmap);
- dma_free_coherent(NULL, PALETTE_SIZE, par->v_palette_base,
+ dma_free_coherent(par->dev, PALETTE_SIZE, par->v_palette_base,
par->p_palette_base);
- dma_free_coherent(NULL, par->vram_size, par->vram_virt,
+ dma_free_coherent(par->dev, par->vram_size, par->vram_virt,
par->vram_phys);
pm_runtime_put_sync(&dev->dev);
pm_runtime_disable(&dev->dev);
@@ -1425,7 +1425,7 @@ static int fb_probe(struct platform_device *device)
par->vram_size = roundup(par->vram_size/8, ulcm);
par->vram_size = par->vram_size * LCD_NUM_BUFFERS;
- par->vram_virt = dma_alloc_coherent(NULL,
+ par->vram_virt = dma_alloc_coherent(par->dev,
par->vram_size,
&par->vram_phys,
GFP_KERNEL | GFP_DMA);
@@ -1446,7 +1446,7 @@ static int fb_probe(struct platform_device *device)
da8xx_fb_fix.line_length - 1;
/* allocate palette buffer */
- par->v_palette_base = dma_alloc_coherent(NULL, PALETTE_SIZE,
+ par->v_palette_base = dma_alloc_coherent(par->dev, PALETTE_SIZE,
&par->p_palette_base,
GFP_KERNEL | GFP_DMA);
if (!par->v_palette_base) {
@@ -1532,11 +1532,12 @@ static int fb_probe(struct platform_device *device)
fb_dealloc_cmap(&da8xx_fb_info->cmap);
err_release_pl_mem:
- dma_free_coherent(NULL, PALETTE_SIZE, par->v_palette_base,
+ dma_free_coherent(par->dev, PALETTE_SIZE, par->v_palette_base,
par->p_palette_base);
err_release_fb_mem:
- dma_free_coherent(NULL, par->vram_size, par->vram_virt, par->vram_phys);
+ dma_free_coherent(par->dev, par->vram_size, par->vram_virt,
+ par->vram_phys);
err_release_fb:
framebuffer_release(da8xx_fb_info);
--
2.20.1
[PATCH 02/18] dmaengine: imx-sdma: pass struct device to DMA API functions
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Signed-off-by: Christoph Hellwig
---
drivers/dma/imx-sdma.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 86708fb9bda1..0b6bba0b9f38 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -677,7 +677,7 @@ static int sdma_load_script(struct sdma_engine *sdma, void
*buf, int size,
int ret;
unsigned long flags;
- buf_virt = dma_alloc_coherent(NULL, size, &buf_phys, GFP_KERNEL);
+ buf_virt = dma_alloc_coherent(sdma->dev, size, &buf_phys, GFP_KERNEL);
if (!buf_virt) {
return -ENOMEM;
}
@@ -696,7 +696,7 @@ static int sdma_load_script(struct sdma_engine *sdma, void
*buf, int size,
spin_unlock_irqrestore(&sdma->channel_0_lock, flags);
- dma_free_coherent(NULL, size, buf_virt, buf_phys);
+ dma_free_coherent(sdma->dev, size, buf_virt, buf_phys);
return ret;
}
@@ -1182,8 +1182,8 @@ static int sdma_request_channel0(struct sdma_engine *sdma)
{
int ret = -EBUSY;
- sdma->bd0 = dma_alloc_coherent(NULL, PAGE_SIZE, &sdma->bd0_phys,
- GFP_NOWAIT);
+ sdma->bd0 = dma_alloc_coherent(sdma->dev, PAGE_SIZE, &sdma->bd0_phys,
+ GFP_NOWAIT);
if (!sdma->bd0) {
ret = -ENOMEM;
goto out;
@@ -1842,7 +1842,7 @@ static int sdma_init(struct sdma_engine *sdma)
/* Be sure SDMA has not started yet */
writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
- sdma->channel_control = dma_alloc_coherent(NULL,
+ sdma->channel_control = dma_alloc_coherent(sdma->dev,
MAX_DMA_CHANNELS * sizeof (struct sdma_channel_control)
+
sizeof(struct sdma_context_data),
&ccb_phys, GFP_KERNEL);
--
2.20.1
don't pass a NULL struct device to DMA API functions
We still have a few drivers which pass a NULL struct device pointer to DMA API functions, which generally is a bad idea as the API implementations rely on the device not only for ops selection, but also the dma mask and various other attributes. This series contains all easy conversions to pass a struct device, besides that there also is some arch code that needs separate handling, a driver that should not use the DMA API at all, and one that is a complete basket case to be deal with separately.
[PATCH 03/18] net: caif: pass struct device to DMA API functions
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Also use the proper Kconfig symbol to check for DMA API availability.
Signed-off-by: Christoph Hellwig
---
drivers/net/caif/caif_spi.c | 30 --
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/net/caif/caif_spi.c b/drivers/net/caif/caif_spi.c
index d28a1398c091..b7f3e263b57c 100644
--- a/drivers/net/caif/caif_spi.c
+++ b/drivers/net/caif/caif_spi.c
@@ -73,35 +73,37 @@ MODULE_PARM_DESC(spi_down_tail_align, "SPI downlink tail
alignment.");
#define LOW_WATER_MARK 100
#define HIGH_WATER_MARK (LOW_WATER_MARK*5)
-#ifdef CONFIG_UML
+#ifdef CONFIG_HAS_DMA
/*
* We sometimes use UML for debugging, but it cannot handle
* dma_alloc_coherent so we have to wrap it.
*/
-static inline void *dma_alloc(dma_addr_t *daddr)
+static inline void *dma_alloc(struct cfspi *cfspi, dma_addr_t *daddr)
{
return kmalloc(SPI_DMA_BUF_LEN, GFP_KERNEL);
}
-static inline void dma_free(void *cpu_addr, dma_addr_t handle)
+static inline void dma_free(struct cfspi *cfspi, void *cpu_addr,
+ dma_addr_t handle)
{
kfree(cpu_addr);
}
#else
-static inline void *dma_alloc(dma_addr_t *daddr)
+static inline void *dma_alloc(struct cfspi *cfspi, dma_addr_t *daddr)
{
- return dma_alloc_coherent(NULL, SPI_DMA_BUF_LEN, daddr,
+ return dma_alloc_coherent(&cfspi->pdev->dev, SPI_DMA_BUF_LEN, daddr,
GFP_KERNEL);
}
-static inline void dma_free(void *cpu_addr, dma_addr_t handle)
+static inline void dma_free(struct cfspi *cfspi, void *cpu_addr,
+ dma_addr_t handle)
{
- dma_free_coherent(NULL, SPI_DMA_BUF_LEN, cpu_addr, handle);
+ dma_free_coherent(&cfspi->pdev->dev, SPI_DMA_BUF_LEN, cpu_addr, handle);
}
-#endif /* CONFIG_UML */
+#endif /* CONFIG_HAS_DMA */
#ifdef CONFIG_DEBUG_FS
@@ -610,13 +612,13 @@ static int cfspi_init(struct net_device *dev)
}
/* Allocate DMA buffers. */
- cfspi->xfer.va_tx[0] = dma_alloc(&cfspi->xfer.pa_tx[0]);
+ cfspi->xfer.va_tx[0] = dma_alloc(cfspi, &cfspi->xfer.pa_tx[0]);
if (!cfspi->xfer.va_tx[0]) {
res = -ENODEV;
goto err_dma_alloc_tx_0;
}
- cfspi->xfer.va_rx = dma_alloc(&cfspi->xfer.pa_rx);
+ cfspi->xfer.va_rx = dma_alloc(cfspi, &cfspi->xfer.pa_rx);
if (!cfspi->xfer.va_rx) {
res = -ENODEV;
@@ -665,9 +667,9 @@ static int cfspi_init(struct net_device *dev)
return 0;
err_create_wq:
- dma_free(cfspi->xfer.va_rx, cfspi->xfer.pa_rx);
+ dma_free(cfspi, cfspi->xfer.va_rx, cfspi->xfer.pa_rx);
err_dma_alloc_rx:
- dma_free(cfspi->xfer.va_tx[0], cfspi->xfer.pa_tx[0]);
+ dma_free(cfspi, cfspi->xfer.va_tx[0], cfspi->xfer.pa_tx[0]);
err_dma_alloc_tx_0:
return res;
}
@@ -683,8 +685,8 @@ static void cfspi_uninit(struct net_device *dev)
cfspi->ndev = NULL;
/* Free DMA buffers. */
- dma_free(cfspi->xfer.va_rx, cfspi->xfer.pa_rx);
- dma_free(cfspi->xfer.va_tx[0], cfspi->xfer.pa_tx[0]);
+ dma_free(cfspi, cfspi->xfer.va_rx, cfspi->xfer.pa_rx);
+ dma_free(cfspi, cfspi->xfer.va_tx[0], cfspi->xfer.pa_tx[0]);
set_bit(SPI_TERMINATE, &cfspi->state);
wake_up_interruptible(&cfspi->wait);
destroy_workqueue(cfspi->wq);
--
2.20.1
[PATCH 10/18] smc911x: pass struct device to DMA API functions
The DMA API generally relies on a struct device to work properly, and only barely works without one for legacy reasons. Pass the easily available struct device from the platform_device to remedy this. Signed-off-by: Christoph Hellwig --- drivers/net/ethernet/smsc/smc911x.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/smsc/smc911x.c b/drivers/net/ethernet/smsc/smc911x.c index 8355dfbb8ec3..b550e624500d 100644 --- a/drivers/net/ethernet/smsc/smc911x.c +++ b/drivers/net/ethernet/smsc/smc911x.c @@ -1188,7 +1188,7 @@ smc911x_tx_dma_irq(void *data) DBG(SMC_DEBUG_TX | SMC_DEBUG_DMA, dev, "TX DMA irq handler\n"); BUG_ON(skb == NULL); - dma_unmap_single(NULL, tx_dmabuf, tx_dmalen, DMA_TO_DEVICE); + dma_unmap_single(lp->dev, tx_dmabuf, tx_dmalen, DMA_TO_DEVICE); netif_trans_update(dev); dev_kfree_skb_irq(skb); lp->current_tx_skb = NULL; @@ -1219,7 +1219,7 @@ smc911x_rx_dma_irq(void *data) DBG(SMC_DEBUG_FUNC, dev, "--> %s\n", __func__); DBG(SMC_DEBUG_RX | SMC_DEBUG_DMA, dev, "RX DMA irq handler\n"); - dma_unmap_single(NULL, rx_dmabuf, rx_dmalen, DMA_FROM_DEVICE); + dma_unmap_single(lp->dev, rx_dmabuf, rx_dmalen, DMA_FROM_DEVICE); BUG_ON(skb == NULL); lp->current_rx_skb = NULL; PRINT_PKT(skb->data, skb->len); -- 2.20.1
[PATCH 07/18] pxa168_eth: pass struct device to DMA API functions
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Note that this driver seems to entirely lack dma_map_single error
handling, but that is left for another time.
Signed-off-by: Christoph Hellwig
---
drivers/net/ethernet/marvell/pxa168_eth.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c
b/drivers/net/ethernet/marvell/pxa168_eth.c
index f8a6d6e3cb7a..35f2142aac5e 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -201,6 +201,7 @@ struct tx_desc {
};
struct pxa168_eth_private {
+ struct platform_device *pdev;
int port_num; /* User Ethernet port number*/
int phy_addr;
int phy_speed;
@@ -331,7 +332,7 @@ static void rxq_refill(struct net_device *dev)
used_rx_desc = pep->rx_used_desc_q;
p_used_rx_desc = &pep->p_rx_desc_area[used_rx_desc];
size = skb_end_pointer(skb) - skb->data;
- p_used_rx_desc->buf_ptr = dma_map_single(NULL,
+ p_used_rx_desc->buf_ptr = dma_map_single(&pep->pdev->dev,
skb->data,
size,
DMA_FROM_DEVICE);
@@ -743,7 +744,7 @@ static int txq_reclaim(struct net_device *dev, int force)
netdev_err(dev, "Error in TX\n");
dev->stats.tx_errors++;
}
- dma_unmap_single(NULL, addr, count, DMA_TO_DEVICE);
+ dma_unmap_single(&pep->pdev->dev, addr, count, DMA_TO_DEVICE);
if (skb)
dev_kfree_skb_irq(skb);
released++;
@@ -805,7 +806,7 @@ static int rxq_process(struct net_device *dev, int budget)
if (rx_next_curr_desc == rx_used_desc)
pep->rx_resource_err = 1;
pep->rx_desc_count--;
- dma_unmap_single(NULL, rx_desc->buf_ptr,
+ dma_unmap_single(&pep->pdev->dev, rx_desc->buf_ptr,
rx_desc->buf_size,
DMA_FROM_DEVICE);
received_packets++;
@@ -1274,7 +1275,8 @@ pxa168_eth_start_xmit(struct sk_buff *skb, struct
net_device *dev)
length = skb->len;
pep->tx_skb[tx_index] = skb;
desc->byte_cnt = length;
- desc->buf_ptr = dma_map_single(NULL, skb->data, length, DMA_TO_DEVICE);
+ desc->buf_ptr = dma_map_single(&pep->pdev->dev, skb->data, length,
+ DMA_TO_DEVICE);
skb_tx_timestamp(skb);
@@ -1528,6 +1530,7 @@ static int pxa168_eth_probe(struct platform_device *pdev)
if (err)
goto err_free_mdio;
+ pep->pdev = pdev;
SET_NETDEV_DEV(dev, &pdev->dev);
pxa168_init_hw(pep);
err = register_netdev(dev);
--
2.20.1
[PATCH 06/18] lantiq_etop: pass struct device to DMA API functions
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Note this driver seems to lack dma_unmap_* calls entirely, but fixing
that is left for another time.
Signed-off-by: Christoph Hellwig
---
drivers/net/ethernet/lantiq_etop.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/lantiq_etop.c
b/drivers/net/ethernet/lantiq_etop.c
index 32ac9045cdae..f9bb890733b5 100644
--- a/drivers/net/ethernet/lantiq_etop.c
+++ b/drivers/net/ethernet/lantiq_etop.c
@@ -112,10 +112,12 @@ struct ltq_etop_priv {
static int
ltq_etop_alloc_skb(struct ltq_etop_chan *ch)
{
+ struct ltq_etop_priv *priv = netdev_priv(ch->netdev);
+
ch->skb[ch->dma.desc] = netdev_alloc_skb(ch->netdev, MAX_DMA_DATA_LEN);
if (!ch->skb[ch->dma.desc])
return -ENOMEM;
- ch->dma.desc_base[ch->dma.desc].addr = dma_map_single(NULL,
+ ch->dma.desc_base[ch->dma.desc].addr = dma_map_single(&priv->pdev->dev,
ch->skb[ch->dma.desc]->data, MAX_DMA_DATA_LEN,
DMA_FROM_DEVICE);
ch->dma.desc_base[ch->dma.desc].addr =
@@ -487,7 +489,7 @@ ltq_etop_tx(struct sk_buff *skb, struct net_device *dev)
netif_trans_update(dev);
spin_lock_irqsave(&priv->lock, flags);
- desc->addr = ((unsigned int) dma_map_single(NULL, skb->data, len,
+ desc->addr = ((unsigned int) dma_map_single(&priv->pdev->dev,
skb->data, len,
DMA_TO_DEVICE)) - byte_offset;
wmb();
desc->ctl = LTQ_DMA_OWN | LTQ_DMA_SOP | LTQ_DMA_EOP |
--
2.20.1
[PATCH 11/18] parport_ip32: pass struct device to DMA API functions
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Signed-off-by: Christoph Hellwig
---
drivers/parport/parport_ip32.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/parport/parport_ip32.c b/drivers/parport/parport_ip32.c
index 62873070f988..b7a892791c3e 100644
--- a/drivers/parport/parport_ip32.c
+++ b/drivers/parport/parport_ip32.c
@@ -568,6 +568,7 @@ static irqreturn_t parport_ip32_merr_interrupt(int irq,
void *dev_id)
/**
* parport_ip32_dma_start - begins a DMA transfer
+ * @p: partport to work on
* @dir: DMA direction: DMA_TO_DEVICE or DMA_FROM_DEVICE
* @addr: pointer to data buffer
* @count: buffer size
@@ -575,8 +576,8 @@ static irqreturn_t parport_ip32_merr_interrupt(int irq,
void *dev_id)
* Calls to parport_ip32_dma_start() and parport_ip32_dma_stop() must be
* correctly balanced.
*/
-static int parport_ip32_dma_start(enum dma_data_direction dir,
- void *addr, size_t count)
+static int parport_ip32_dma_start(struct parport *p,
+ enum dma_data_direction dir, void *addr, size_t count)
{
unsigned int limit;
u64 ctrl;
@@ -601,7 +602,7 @@ static int parport_ip32_dma_start(enum dma_data_direction
dir,
/* Prepare DMA pointers */
parport_ip32_dma.dir = dir;
- parport_ip32_dma.buf = dma_map_single(NULL, addr, count, dir);
+ parport_ip32_dma.buf = dma_map_single(&p->bus_dev, addr, count, dir);
parport_ip32_dma.len = count;
parport_ip32_dma.next = parport_ip32_dma.buf;
parport_ip32_dma.left = parport_ip32_dma.len;
@@ -625,11 +626,12 @@ static int parport_ip32_dma_start(enum dma_data_direction
dir,
/**
* parport_ip32_dma_stop - ends a running DMA transfer
+ * @p: partport to work on
*
* Calls to parport_ip32_dma_start() and parport_ip32_dma_stop() must be
* correctly balanced.
*/
-static void parport_ip32_dma_stop(void)
+static void parport_ip32_dma_stop(struct parport *p)
{
u64 ctx_a;
u64 ctx_b;
@@ -685,8 +687,8 @@ static void parport_ip32_dma_stop(void)
enable_irq(MACEISA_PAR_CTXB_IRQ);
parport_ip32_dma.irq_on = 1;
- dma_unmap_single(NULL, parport_ip32_dma.buf, parport_ip32_dma.len,
-parport_ip32_dma.dir);
+ dma_unmap_single(&p->bus_dev, parport_ip32_dma.buf,
+parport_ip32_dma.len, parport_ip32_dma.dir);
}
/**
@@ -1445,7 +1447,7 @@ static size_t parport_ip32_fifo_write_block_dma(struct
parport *p,
priv->irq_mode = PARPORT_IP32_IRQ_HERE;
- parport_ip32_dma_start(DMA_TO_DEVICE, (void *)buf, len);
+ parport_ip32_dma_start(p, DMA_TO_DEVICE, (void *)buf, len);
reinit_completion(&priv->irq_complete);
parport_ip32_frob_econtrol(p, ECR_DMAEN | ECR_SERVINTR, ECR_DMAEN);
@@ -1461,7 +1463,7 @@ static size_t parport_ip32_fifo_write_block_dma(struct
parport *p,
if (ecr & ECR_SERVINTR)
break; /* DMA transfer just finished */
}
- parport_ip32_dma_stop();
+ parport_ip32_dma_stop(p);
written = len - parport_ip32_dma_get_residue();
priv->irq_mode = PARPORT_IP32_IRQ_FWD;
--
2.20.1
[PATCH 12/18] fotg210-udc: remove a bogus dma_sync_single_for_device call
dma_map_single already transfers ownership to the device. Signed-off-by: Christoph Hellwig --- drivers/usb/gadget/udc/fotg210-udc.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/usb/gadget/udc/fotg210-udc.c b/drivers/usb/gadget/udc/fotg210-udc.c index bc6abaea907d..fe9cf415f2f1 100644 --- a/drivers/usb/gadget/udc/fotg210-udc.c +++ b/drivers/usb/gadget/udc/fotg210-udc.c @@ -356,10 +356,6 @@ static void fotg210_start_dma(struct fotg210_ep *ep, return; } - dma_sync_single_for_device(NULL, d, length, - ep->dir_in ? DMA_TO_DEVICE : - DMA_FROM_DEVICE); - fotg210_enable_dma(ep, d, length); /* check if dma is done */ -- 2.20.1
[PATCH 13/18] fotg210-udc: pass struct device to DMA API functions
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Signed-off-by: Christoph Hellwig
---
drivers/usb/gadget/udc/fotg210-udc.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/fotg210-udc.c
b/drivers/usb/gadget/udc/fotg210-udc.c
index fe9cf415f2f1..cec49294bac6 100644
--- a/drivers/usb/gadget/udc/fotg210-udc.c
+++ b/drivers/usb/gadget/udc/fotg210-udc.c
@@ -326,6 +326,7 @@ static void fotg210_wait_dma_done(struct fotg210_ep *ep)
static void fotg210_start_dma(struct fotg210_ep *ep,
struct fotg210_request *req)
{
+ struct device *dev = &ep->fotg210->gadget.dev;
dma_addr_t d;
u8 *buffer;
u32 length;
@@ -348,10 +349,10 @@ static void fotg210_start_dma(struct fotg210_ep *ep,
length = req->req.length;
}
- d = dma_map_single(NULL, buffer, length,
+ d = dma_map_single(dev, buffer, length,
ep->dir_in ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
- if (dma_mapping_error(NULL, d)) {
+ if (dma_mapping_error(dev, d)) {
pr_err("dma_mapping_error\n");
return;
}
@@ -366,7 +367,7 @@ static void fotg210_start_dma(struct fotg210_ep *ep,
/* update actual transfer length */
req->req.actual += length;
- dma_unmap_single(NULL, d, length, DMA_TO_DEVICE);
+ dma_unmap_single(dev, d, length, DMA_TO_DEVICE);
}
static void fotg210_ep0_queue(struct fotg210_ep *ep,
--
2.20.1
[PATCH 17/18] ALSA: hal2: pass struct device to DMA API functions
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Signed-off-by: Christoph Hellwig
---
sound/mips/hal2.c | 31 +--
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/sound/mips/hal2.c b/sound/mips/hal2.c
index a4ed54aeaf1d..d63e1565b62b 100644
--- a/sound/mips/hal2.c
+++ b/sound/mips/hal2.c
@@ -454,21 +454,22 @@ static inline void hal2_stop_adc(struct snd_hal2 *hal2)
hal2->adc.pbus.pbus->pbdma_ctrl = HPC3_PDMACTRL_LD;
}
-static int hal2_alloc_dmabuf(struct hal2_codec *codec)
+static int hal2_alloc_dmabuf(struct snd_hal2 *hal2, struct hal2_codec *codec)
{
+ struct device *dev = hal2->card->dev;
struct hal2_desc *desc;
dma_addr_t desc_dma, buffer_dma;
int count = H2_BUF_SIZE / H2_BLOCK_SIZE;
int i;
- codec->buffer = dma_alloc_attrs(NULL, H2_BUF_SIZE, &buffer_dma,
+ codec->buffer = dma_alloc_attrs(dev, H2_BUF_SIZE, &buffer_dma,
GFP_KERNEL, DMA_ATTR_NON_CONSISTENT);
if (!codec->buffer)
return -ENOMEM;
- desc = dma_alloc_attrs(NULL, count * sizeof(struct hal2_desc),
+ desc = dma_alloc_attrs(dev, count * sizeof(struct hal2_desc),
&desc_dma, GFP_KERNEL, DMA_ATTR_NON_CONSISTENT);
if (!desc) {
- dma_free_attrs(NULL, H2_BUF_SIZE, codec->buffer, buffer_dma,
+ dma_free_attrs(dev, H2_BUF_SIZE, codec->buffer, buffer_dma,
DMA_ATTR_NON_CONSISTENT);
return -ENOMEM;
}
@@ -482,17 +483,19 @@ static int hal2_alloc_dmabuf(struct hal2_codec *codec)
desc_dma : desc_dma + (i + 1) * sizeof(struct hal2_desc);
desc++;
}
- dma_cache_sync(NULL, codec->desc, count * sizeof(struct hal2_desc),
+ dma_cache_sync(dev, codec->desc, count * sizeof(struct hal2_desc),
DMA_TO_DEVICE);
codec->desc_count = count;
return 0;
}
-static void hal2_free_dmabuf(struct hal2_codec *codec)
+static void hal2_free_dmabuf(struct snd_hal2 *hal2, struct hal2_codec *codec)
{
- dma_free_attrs(NULL, codec->desc_count * sizeof(struct hal2_desc),
+ struct device *dev = hal2->card->dev;
+
+ dma_free_attrs(dev, codec->desc_count * sizeof(struct hal2_desc),
codec->desc, codec->desc_dma, DMA_ATTR_NON_CONSISTENT);
- dma_free_attrs(NULL, H2_BUF_SIZE, codec->buffer, codec->buffer_dma,
+ dma_free_attrs(dev, H2_BUF_SIZE, codec->buffer, codec->buffer_dma,
DMA_ATTR_NON_CONSISTENT);
}
@@ -540,7 +543,7 @@ static int hal2_playback_open(struct snd_pcm_substream
*substream)
runtime->hw = hal2_pcm_hw;
- err = hal2_alloc_dmabuf(&hal2->dac);
+ err = hal2_alloc_dmabuf(hal2, &hal2->dac);
if (err)
return err;
return 0;
@@ -550,7 +553,7 @@ static int hal2_playback_close(struct snd_pcm_substream
*substream)
{
struct snd_hal2 *hal2 = snd_pcm_substream_chip(substream);
- hal2_free_dmabuf(&hal2->dac);
+ hal2_free_dmabuf(hal2, &hal2->dac);
return 0;
}
@@ -606,7 +609,7 @@ static void hal2_playback_transfer(struct snd_pcm_substream
*substream,
unsigned char *buf = hal2->dac.buffer + rec->hw_data;
memcpy(buf, substream->runtime->dma_area + rec->sw_data, bytes);
- dma_cache_sync(NULL, buf, bytes, DMA_TO_DEVICE);
+ dma_cache_sync(hal2->card->dev, buf, bytes, DMA_TO_DEVICE);
}
@@ -629,7 +632,7 @@ static int hal2_capture_open(struct snd_pcm_substream
*substream)
runtime->hw = hal2_pcm_hw;
- err = hal2_alloc_dmabuf(adc);
+ err = hal2_alloc_dmabuf(hal2, adc);
if (err)
return err;
return 0;
@@ -639,7 +642,7 @@ static int hal2_capture_close(struct snd_pcm_substream
*substream)
{
struct snd_hal2 *hal2 = snd_pcm_substream_chip(substream);
- hal2_free_dmabuf(&hal2->adc);
+ hal2_free_dmabuf(hal2, &hal2->adc);
return 0;
}
@@ -694,7 +697,7 @@ static void hal2_capture_transfer(struct snd_pcm_substream
*substream,
struct snd_hal2 *hal2 = snd_pcm_substream_chip(substream);
unsigned char *buf = hal2->adc.buffer + rec->hw_data;
- dma_cache_sync(NULL, buf, bytes, DMA_FROM_DEVICE);
+ dma_cache_sync(hal2->card->dev, buf, bytes, DMA_FROM_DEVICE);
memcpy(substream->runtime->dma_area + rec->sw_data, buf, bytes);
}
--
2.20.1
[PATCH 09/18] meth: pass struct device to DMA API functions
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Also use GFP_KERNEL instead of GFP_ATOMIC as the gfp_t for the memory
allocation, as we aren't in interrupt context or under a lock.
Signed-off-by: Christoph Hellwig
---
drivers/net/ethernet/sgi/meth.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/sgi/meth.c b/drivers/net/ethernet/sgi/meth.c
index 0e1b7e960b98..67954a9e3675 100644
--- a/drivers/net/ethernet/sgi/meth.c
+++ b/drivers/net/ethernet/sgi/meth.c
@@ -68,6 +68,8 @@ module_param(timeout, int, 0);
* packets in and out, so there is place for a packet
*/
struct meth_private {
+ struct platform_device *pdev;
+
/* in-memory copy of MAC Control register */
u64 mac_ctrl;
@@ -211,8 +213,8 @@ static void meth_check_link(struct net_device *dev)
static int meth_init_tx_ring(struct meth_private *priv)
{
/* Init TX ring */
- priv->tx_ring = dma_alloc_coherent(NULL, TX_RING_BUFFER_SIZE,
- &priv->tx_ring_dma, GFP_ATOMIC);
+ priv->tx_ring = dma_alloc_coherent(&priv->pdev->dev,
+ TX_RING_BUFFER_SIZE, &priv->tx_ring_dma, GFP_KERNEL);
if (!priv->tx_ring)
return -ENOMEM;
@@ -236,7 +238,7 @@ static int meth_init_rx_ring(struct meth_private *priv)
priv->rx_ring[i]=(rx_packet*)(priv->rx_skbs[i]->head);
/* I'll need to re-sync it after each RX */
priv->rx_ring_dmas[i] =
- dma_map_single(NULL, priv->rx_ring[i],
+ dma_map_single(&priv->pdev->dev, priv->rx_ring[i],
METH_RX_BUFF_SIZE, DMA_FROM_DEVICE);
mace->eth.rx_fifo = priv->rx_ring_dmas[i];
}
@@ -253,7 +255,7 @@ static void meth_free_tx_ring(struct meth_private *priv)
dev_kfree_skb(priv->tx_skbs[i]);
priv->tx_skbs[i] = NULL;
}
- dma_free_coherent(NULL, TX_RING_BUFFER_SIZE, priv->tx_ring,
+ dma_free_coherent(&priv->pdev->dev, TX_RING_BUFFER_SIZE, priv->tx_ring,
priv->tx_ring_dma);
}
@@ -263,7 +265,7 @@ static void meth_free_rx_ring(struct meth_private *priv)
int i;
for (i = 0; i < RX_RING_ENTRIES; i++) {
- dma_unmap_single(NULL, priv->rx_ring_dmas[i],
+ dma_unmap_single(&priv->pdev->dev, priv->rx_ring_dmas[i],
METH_RX_BUFF_SIZE, DMA_FROM_DEVICE);
priv->rx_ring[i] = 0;
priv->rx_ring_dmas[i] = 0;
@@ -393,7 +395,8 @@ static void meth_rx(struct net_device* dev, unsigned long
int_status)
fifo_rptr = (fifo_rptr - 1) & 0x0f;
}
while (priv->rx_write != fifo_rptr) {
- dma_unmap_single(NULL, priv->rx_ring_dmas[priv->rx_write],
+ dma_unmap_single(&priv->pdev->dev,
+priv->rx_ring_dmas[priv->rx_write],
METH_RX_BUFF_SIZE, DMA_FROM_DEVICE);
status = priv->rx_ring[priv->rx_write]->status.raw;
#if MFE_DEBUG
@@ -454,7 +457,8 @@ static void meth_rx(struct net_device* dev, unsigned long
int_status)
priv->rx_ring[priv->rx_write] = (rx_packet*)skb->head;
priv->rx_ring[priv->rx_write]->status.raw = 0;
priv->rx_ring_dmas[priv->rx_write] =
- dma_map_single(NULL, priv->rx_ring[priv->rx_write],
+ dma_map_single(&priv->pdev->dev,
+ priv->rx_ring[priv->rx_write],
METH_RX_BUFF_SIZE, DMA_FROM_DEVICE);
mace->eth.rx_fifo = priv->rx_ring_dmas[priv->rx_write];
ADVANCE_RX_PTR(priv->rx_write);
@@ -637,7 +641,7 @@ static void meth_tx_1page_prepare(struct meth_private *priv,
}
/* first page */
- catbuf = dma_map_single(NULL, buffer_data, buffer_len,
+ catbuf = dma_map_single(&priv->pdev->dev, buffer_data, buffer_len,
DMA_TO_DEVICE);
desc->data.cat_buf[0].form.start_addr = catbuf >> 3;
desc->data.cat_buf[0].form.len = buffer_len - 1;
@@ -663,12 +667,12 @@ static void meth_tx_2page_prepare(struct meth_private
*priv,
}
/* first page */
- catbuf1 = dma_map_single(NULL, buffer1_data, buffer1_len,
+ catbuf1 = dma_map_single(&priv->pdev->dev, buffer1_data, buffer1_len,
DMA_TO_DEVI
Re: [RFC PATCH] USB: PCI: set 32bit DMA mask for PCI based USB controllers
On Wed, Jan 30, 2019 at 03:01:54PM +0800, Hanjun Guo wrote: > This is the RFC version, I'm not sure this is the best solution, > comments are warmly welcomed. > > Thanks > Hanjun > > drivers/usb/core/hcd-pci.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c > index 0343246..a9c33e6 100644 > --- a/drivers/usb/core/hcd-pci.c > +++ b/drivers/usb/core/hcd-pci.c > @@ -188,6 +188,10 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct > pci_device_id *id) > if (pci_enable_device(dev) < 0) > return -ENODEV; > > + retval = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32)); > + if (retval) > + return retval; dma_coerce_mask_and_coherent is only for platform devices (and I'm not sure it is a good idea to start with, but that is a different story). PCI device should have the dma_mask pointer set already, so you should use dma_set_mask_and_coherent here.
Re: [PATCH v2 7/7] Move the resid member from struct scsi_data_buffer into struct scsi_cmnd
On Wed, Jan 23, 2019 at 11:10:13AM -0800, Bart Van Assche wrote: > This patch does not change any functionality but reduces the size of > struct scsi_cmnd. This looks sensible, but please do it without adding more pointless wrappers first, just use the field directly.
[PATCH] USB: remove the unused struct hcd_timeout definition
No users of this type anywhere in the tree.
Signed-off-by: Christoph Hellwig
---
include/linux/usb/hcd.h | 5 -
1 file changed, 5 deletions(-)
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 97e2ddec18b1..7dc3a411bece 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -235,11 +235,6 @@ static inline struct usb_hcd *bus_to_hcd(struct usb_bus
*bus)
return container_of(bus, struct usb_hcd, self);
}
-struct hcd_timeout { /* timeouts we allocate */
- struct list_headtimeout_list;
- struct timer_list timer;
-};
-
/*-*/
--
2.19.1
Re: [RFC 2/3] USB: core: Add non-coherent buffer allocation helpers
On Thu, Aug 30, 2018 at 07:11:35PM -0300, Ezequiel Garcia wrote: > On Thu, 2018-08-30 at 10:58 -0700, Christoph Hellwig wrote: > > Please don't introduce new DMA_ATTR_NON_CONSISTENT users, it is > > a rather horrible interface, and I plan to kill it off rather sooner > > than later. I plan to post some patches for a better interface > > that can reuse the normal dma_sync_single_* interfaces for ownership > > transfers. I can happily include usb in that initial patch set based > > on your work here if that helps. > > Please do. Until we have proper allocators that go thru the DMA API, > drivers will have to kmalloc the USB transfer buffers, and have > streaming mappings. Which in turns mean not using IOMMU or CMA. dma_map_page will of course use the iommu.
Re: [RFC 3/3] stk1160: Use non-coherent buffers for USB transfers
> + dma_sync_single_for_cpu(&urb->dev->dev, urb->transfer_dma, > + urb->transfer_buffer_length, DMA_FROM_DEVICE); You can't ue dma_sync_single_for_cpu on non-coherent dma buffers, which is one of the major issues with them.
Re: [RFC 2/3] USB: core: Add non-coherent buffer allocation helpers
Please don't introduce new DMA_ATTR_NON_CONSISTENT users, it is a rather horrible interface, and I plan to kill it off rather sooner than later. I plan to post some patches for a better interface that can reuse the normal dma_sync_single_* interfaces for ownership transfers. I can happily include usb in that initial patch set based on your work here if that helps.
Re: [PATCH] usb-storage: stop using block layer bounce buffers
On Fri, Apr 27, 2018 at 10:09:17AM -0400, Alan Stern wrote: > On Fri, 27 Apr 2018, Christoph Hellwig wrote: > > > On Sun, Apr 15, 2018 at 11:24:11AM -0400, Alan Stern wrote: > > > On Sun, 15 Apr 2018, Christoph Hellwig wrote: > > > > > > > USB host controllers now must handle highmem, so we can get rid of > > > > bounce > > > > buffering highmem pages in the block layer. > > > > > > Sorry, I don't quite understand what you are saying. Do you mean that > > > all USB host controllers now magically _do_ handle highmem? Or do you > > > mean that if they _don't_ handle highmem, we will not support them any > > > more? > > > > USB controller themselves never cared about highmem, drivers did. For > > PIO based controllers they'd have to kmap any address no in the kernel > > drirect mapping. > > > > Nothing in drivers/usb/host or the other diretories related to host > > drivers calls page_address (only used in a single gadget) or sg_virt > > (only used in a few upper level drivers), which makes me assume > > semi-confidently that no USB host driver is not highmem aware these > > days. > > sg_virt is called in drivers/usb/core/message.c. (Maybe that's what > you meant by "upper level drivers".) I'm not sure just how important > that usage is. I don't really know either. I'll need some guidance from the usb maintainers on: - when drivers can submit urbs with a scatterlist - if there are any drivers that do not want to take highmem Unfortunately the way dma mapping works in usb is just so deeply convoluted that I have a hard time following it, and often wonder what is intentional and what is accidental in it. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb-storage: stop using block layer bounce buffers
On Sun, Apr 15, 2018 at 11:24:11AM -0400, Alan Stern wrote: > On Sun, 15 Apr 2018, Christoph Hellwig wrote: > > > USB host controllers now must handle highmem, so we can get rid of bounce > > buffering highmem pages in the block layer. > > Sorry, I don't quite understand what you are saying. Do you mean that > all USB host controllers now magically _do_ handle highmem? Or do you > mean that if they _don't_ handle highmem, we will not support them any > more? USB controller themselves never cared about highmem, drivers did. For PIO based controllers they'd have to kmap any address no in the kernel drirect mapping. Nothing in drivers/usb/host or the other diretories related to host drivers calls page_address (only used in a single gadget) or sg_virt (only used in a few upper level drivers), which makes me assume semi-confidently that no USB host driver is not highmem aware these days. Greg, does this match your observation as the USB maintainer? -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb-storage: stop using block layer bounce buffers
USB host controllers now must handle highmem, so we can get rid of bounce buffering highmem pages in the block layer. Signed-off-by: Christoph Hellwig --- drivers/usb/storage/scsiglue.c | 9 - 1 file changed, 9 deletions(-) diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c index c267f2812a04..4e453d9d45d5 100644 --- a/drivers/usb/storage/scsiglue.c +++ b/drivers/usb/storage/scsiglue.c @@ -130,15 +130,6 @@ static int slave_configure(struct scsi_device *sdev) blk_queue_max_hw_sectors(sdev->request_queue, 2048); } - /* -* Some USB host controllers can't do DMA; they have to use PIO. -* They indicate this by setting their dma_mask to NULL. For -* such controllers we need to make sure the block layer sets -* up bounce buffers in addressable memory. -*/ - if (!us->pusb_dev->bus->controller->dma_mask) - blk_queue_bounce_limit(sdev->request_queue, BLK_BOUNCE_HIGH); - /* * We can't put these settings in slave_alloc() because that gets * called before the device type is known. Consequently these -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] USB/PHY driver patches for 4.17-rc1
On Thu, Apr 05, 2018 at 09:19:28AM +0200, Lars-Peter Clausen wrote: > On 04/05/2018 08:31 AM, Kees Cook wrote: > > On Wed, Apr 4, 2018 at 3:31 AM, Greg KH wrote: > >> Lars-Peter Clausen (2): > >> usb: gadget: ffs: Execute copy_to_user() with USER_DS set > > > > https://git.kernel.org/linus/4058ebf33cb0be88ca516f968eda24ab7b6b93e4 > > > > Isn't there a better way to do this without the set_fs() usage? We've > > been try to eliminate it in the kernel. I thought there was a safer > > way to use iters now? > > The problem is use_mm(). It needs to be accompanied with set_fs(DS_USER) to > work reliably. This has simply been missing for this particular instance of > use_mm(). To me it seems like use_mm() should do set_fs(USER_DS) and unuse_mm() should do set_fs(KERNEL_DS) to get drivers outo of this mess. I'll see if I can come up with patches for the next merge window. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/5] Allow compile-testing NO_DMA (core)
Thanks Geert, applied to the dma-mapping tree for 4.17. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/16] remove eight obsolete architectures
On Thu, Mar 15, 2018 at 11:42:25AM +0100, Arnd Bergmann wrote: > Is anyone producing a chip that includes enough of the Privileged ISA spec > to have things like system calls, but not the MMU parts? Various SiFive SOCs seem to support M and U mode, but no S mode or iommu. That should be enough for nommu Linux running in M mode if someone cares enough to actually port it. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WARN_ON(irqs_disabled()) in dma_free_attrs?
On Wed, Mar 14, 2018 at 05:43:46PM +, Robin Murphy wrote: >> Looking back I don't really understand why we even indirect the "classic" >> per-device dma_declare_coherent_memory use case through the DMA API. > > It certainly makes sense for devices which can exist in both shared-memory > and device-local-memory configurations, so the driver doesn't have to care > about the details (particularly on mobile SoCs where the 'local' memory > might just be a chunk of system RAM reserved by the bootloader, and it's > just a matter of different use-cases on identical hardware). Well, the "classic" case for me is memory buffers in the device. Setting some memory aside, either in a global pool as now done for arm-nommu or even per-device as on some ARM SOCs is different indeed. As far as I can tell the few devices that use 'local' memory always use that. >> It seems like a pretty different use case to me. In the USB case we >> also have the following additional twist in that it doesn't even need >> the mapping to be coherent. > > I'm pretty sure it does (in the sense that it needs to ensure the arch code > makes the mapping non-cacheable), otherwise I can't see how the bouncing > could work properly. I think the last bit of the comment above > hcd_alloc_coherent() is a bit misleading. Well, if it isn't marked non-cacheable we'd have to do dma_cache_sync operations for it. Which would probably still be faster than non-cacheable mappings. >> So maybe for now the quick fix is to move the sleep check as suggested >> earlier in this thread, but in the long run we probably need to do some >> major rework of how dma_declare_coherent_memory and friends work. > > Maybe; I do think the specific hcd_alloc_coherent() case could still be > fixed within the scope of the existing code, but it's not quite as clean > and straightforward as I first thought, and the practical impact of > tweaking the WARN should be effectively zero despite the theoretical edge > cases it opens up. Do you want me to write it up as a proper patch? Yes. Including a proper comment on why the might_sleep is placed there. My mid-term plan was to actually remove the gfp flags argument from the dma alloc routines as is creates more confusion than it helps. I guess this means we'll at least need to introduce a DMA_ATTR_NON_BLOCK or similar flag instead then unfortunately. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WARN_ON(irqs_disabled()) in dma_free_attrs?
On Tue, Mar 13, 2018 at 12:11:49PM +, Robin Murphy wrote:
> Taking a step back, though, provided the original rationale about
> dma_declare_coherent_memory() is still valid, I wonder if we should simply
> permit the USB code to call dma_{alloc,free}_from_dev_coherent() directly
> here instead of being "good" and indirecting through the top-level DMA API
> (which is the part which leads to problems). Given that it's a specific DMA
> bounce buffer implementation within a core API, not just any old driver
> code, I personally would consider that reasonable.
Looking back I don't really understand why we even indirect the "classic"
per-device dma_declare_coherent_memory use case through the DMA API.
It seems like a pretty different use case to me. In the USB case we
also have the following additional twist in that it doesn't even need
the mapping to be coherent.
So maybe for now the quick fix is to move the sleep check as suggested
earlier in this thread, but in the long run we probably need to do some
major rework of how dma_declare_coherent_memory and friends work.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WARN_ON(irqs_disabled()) in dma_free_attrs?
On Sat, Mar 03, 2018 at 07:19:06PM +0100, Fredrik Noring wrote:
> Christoph, Alan,
>
> > If it is allocating / freeing this memory all the time in the hot path
> > it should really use a dma pool (see include/ilinux/dmapool.h).
> > The dma coherent APIs aren't really built for being called in the
> > hot path.
>
> hcd_buffer_free uses a combination of dma pools and dma coherent APIs:
>
> ...
> for (i = 0; i < HCD_BUFFER_POOLS; i++) {
> if (size <= pool_max[i]) {
> dma_pool_free(hcd->pool[i], addr, dma);
> return;
> }
> }
> dma_free_coherent(hcd->self.sysdev, size, addr, dma);
>
> Alan, can dma_free_coherent be delayed to a point when IRQs are enabled?
The point is that you should always use a pool, period.
dma_alloc*/dma_free* are fundamentally expensive operations on my
architectures, so if you call them from a fast path you are doing
something wrong.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/15] usb: gadget: make config_item_type structures const
> > Now we have 9 const instances of the config_item_type structure that are > identical, with only the .ct_owner field set. Should they be all merged into > a > single structure ? I think that's a good idea. But I'm about to slurp up this whole series into my tree, how about making that an incremental patch? -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] dma-mapping: skip USB devices when configuring DMA during probe
I think the root problem is that the code added by " of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices" is completely bogus and needs to be reverted. We can't simply iterate over all devices in the system and set up dma for them. We'll need to ask the firmware / OF what root port this applies to and only apply it to those buses. Note that the current code checks for a OF or ACPI node already, so we should start by refining these checks. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: CPU lock-ups with 4.12.0+ kernels related to usb_storage
On Thu, Jul 13, 2017 at 01:00:29PM -0400, Alan Stern wrote: > All right. In the meantime, changing usb-storage won't hurt. > Arthur, can you test the patch below? Do you plan to send this fix to Greg for 4.13-rc? -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: CPU lock-ups with 4.12.0+ kernels related to usb_storage
On Wed, Jul 12, 2017 at 12:10:02PM -0400, Alan Stern wrote: > This is pretty conclusive. The problem comes about because > usb_stor_control_thread() calls scsi_mq_done() while holding > shost->host_lock, and then scsi_eh_scmd_add() tries to acquire that > same lock. > > I don't know why this didn't show up in earlier kernels. I guess some > element of the call chain listed above must be new in 4.12. > > Christoph, what's the best way to fix this? Should usb-storage release > the host lock before issuing the ->scsi_done callback? If so, does > that change need to be applied to any kernels before 4.12? 4.12 switched to blk-mq by default, and while the old code used a softirq for completions, which is always a difference context the blk-mq code might execute in the same context it's called in. So yes, for that we'd need to drop host_lock. But I wonder how many more of these are lingering somewhere and if we can find another workaround. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REGRESSION] Failed network caused by: xhci: switch to pci_alloc_irq_vectors
On Sat, May 20, 2017 at 09:49:56AM -0700, Linus Torvalds wrote:
> Side note: why is it doing that " > 1" check, when any value _other_
> than 1 is wrong?
It's the same effect, so either one is fine with me.
> Also, to match the non-MSI implementation, wouldn't it be nicer to
> just write it that same way (and also verify "dev->irq"):
>
> if (flags & PCI_IRQ_LEGACY) {
> if (min_vecs == 1 && dev->irq)
> return 1;
> }
> return -ENOSPC;
>
> (the exact error value probably doesn't matter in practice, but the
> CONFIG_MSI case returns ENOSPC by default and that's what
> Documentation/PCI/MSI-HOWTO.txt says too).
Sure. Just sent the previous version to Bjorn so that he could maybe
make it for -rc2, but I'll respin it.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REGRESSION] Failed network caused by: xhci: switch to pci_alloc_irq_vectors
On Fri, May 19, 2017 at 08:37:21AM -0400, Steven Rostedt wrote:
> ktest config bisect ended with:
>
> ***
> Found bad config: CONFIG_PCI_MSI
> ***
Oh, that's interesting. I think there's been a bug in the !CONFIG_PCI_MSI
fallback for pci_alloc_irq_vectors since the very beginning. And it
didn't matter for any driver so far, but xhci has a very odd way
to set MSI(-X) vs legacy interrupts.
Can you try the patch below?
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 33c2b0b77429..5a7fd3b6a7b9 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1342,7 +1342,7 @@ pci_alloc_irq_vectors_affinity(struct pci_dev *dev,
unsigned int min_vecs,
unsigned int max_vecs, unsigned int flags,
const struct irq_affinity *aff_desc)
{
- if (min_vecs > 1)
+ if (min_vecs > 1 || !(flags & PCI_IRQ_LEGACY))
return -EINVAL;
return 1;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] ACPI: Switch to use generic UUID API
On Fri, May 05, 2017 at 12:50:31PM +0300, Amir Goldstein wrote: > To complete the picture for folks not cc'ed on my patches, > xfs use case suggests there is also justification for the additional helpers: > > uuid_is_null() / uuid_equal() > guid_is_null() / guid_equal() The is_null is useful and I bet we'll find other instances. I'm not sure _equals really adds much value over the existing _cmp helpers, but on the other hand they are so trivial that we might as well add them. The other thing XFS has is uuid_copy. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] ACPI: Switch to use generic UUID API
On Fri, May 05, 2017 at 10:06:06AM +0300, Amir Goldstein wrote: > I much rather that you sort out uuid helpers in a way that will > satisfy the filesystem > needs (just provide the helpers don't need to convert filesystems code). Yeah. > IMO, you should acknowledge that the common use case for filesystems is > to handle an opaque char[16] which most likely holds a uuid_be and you > should provide 'neutral' helpers to satisfy this use case. > > The simplest would be to typedef uuid_t to struct uuid_be and to name > 'neutral' > helpers' uuid_cmp/uuid_copy(uuid_t *, uuid_t *), similar to my proposal. It's not jut neutral, it's the right thing to to. The Apollo / DCE uuids (later also specified in RFC 4122) are big endian, so we should use the name there. > Christoph also suggested a similar treatment to typedef guid_t to > struct uuid_le. Exactly. The whole idea of "little endian UUIDs" comes from the Wintel world, and if you look at the relevant specs they are almost exclusively referred to as GUIDs. The magic XFS and AFS types for specific interpretations of one of the RFC4122 formats don't really help, but I'll just send a patch to kill them off for XFS ASAP to at least get that out, and we probably should revert at least "afs: Move UUID struct to linux/uuid.h" That moved the AFS mess to common code as a start, and then also clean up the way we generate random UUIDs, where we currently have le helper, a be helper and then also generate_random_uuid just to confuse the heck out of people. With no description of either of them. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] xhci: switch to pci_alloc_irq_vectors
Use the modern API to request MSI or MSI-X interrupts, which allows us to
get rid of the msix_entries array, as well as cleaning up the cleanup
code.
Signed-off-by: Christoph Hellwig
---
drivers/usb/host/xhci.c | 99 ++---
drivers/usb/host/xhci.h | 1 -
2 files changed, 28 insertions(+), 72 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 953fd8f62df0..5498271f4af6 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -217,20 +217,6 @@ int xhci_reset(struct xhci_hcd *xhci)
}
#ifdef CONFIG_PCI
-static int xhci_free_msi(struct xhci_hcd *xhci)
-{
- int i;
-
- if (!xhci->msix_entries)
- return -EINVAL;
-
- for (i = 0; i < xhci->msix_count; i++)
- if (xhci->msix_entries[i].vector)
- free_irq(xhci->msix_entries[i].vector,
- xhci_to_hcd(xhci));
- return 0;
-}
-
/*
* Set up MSI
*/
@@ -239,8 +225,8 @@ static int xhci_setup_msi(struct xhci_hcd *xhci)
int ret;
struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
- ret = pci_enable_msi(pdev);
- if (ret) {
+ ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI);
+ if (ret < 0) {
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"failed to allocate MSI entry");
return ret;
@@ -251,35 +237,13 @@ static int xhci_setup_msi(struct xhci_hcd *xhci)
if (ret) {
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"disable MSI interrupt");
- pci_disable_msi(pdev);
+ pci_free_irq_vectors(pdev);
}
return ret;
}
/*
- * Free IRQs
- * free all IRQs request
- */
-static void xhci_free_irq(struct xhci_hcd *xhci)
-{
- struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
- int ret;
-
- /* return if using legacy interrupt */
- if (xhci_to_hcd(xhci)->irq > 0)
- return;
-
- ret = xhci_free_msi(xhci);
- if (!ret)
- return;
- if (pdev->irq > 0)
- free_irq(pdev->irq, xhci_to_hcd(xhci));
-
- return;
-}
-
-/*
* Set up MSI-X
*/
static int xhci_setup_msix(struct xhci_hcd *xhci)
@@ -298,28 +262,17 @@ static int xhci_setup_msix(struct xhci_hcd *xhci)
xhci->msix_count = min(num_online_cpus() + 1,
HCS_MAX_INTRS(xhci->hcs_params1));
- xhci->msix_entries =
- kmalloc((sizeof(struct msix_entry))*xhci->msix_count,
- GFP_KERNEL);
- if (!xhci->msix_entries)
- return -ENOMEM;
-
- for (i = 0; i < xhci->msix_count; i++) {
- xhci->msix_entries[i].entry = i;
- xhci->msix_entries[i].vector = 0;
- }
-
- ret = pci_enable_msix_exact(pdev, xhci->msix_entries, xhci->msix_count);
- if (ret) {
+ ret = pci_alloc_irq_vectors(pdev, xhci->msix_count, xhci->msix_count,
+ PCI_IRQ_MSIX);
+ if (ret < 0) {
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"Failed to enable MSI-X");
- goto free_entries;
+ return ret;
}
for (i = 0; i < xhci->msix_count; i++) {
- ret = request_irq(xhci->msix_entries[i].vector,
- xhci_msi_irq,
- 0, "xhci_hcd", xhci_to_hcd(xhci));
+ ret = request_irq(pci_irq_vector(pdev, i), xhci_msi_irq, 0,
+ "xhci_hcd", xhci_to_hcd(xhci));
if (ret)
goto disable_msix;
}
@@ -329,11 +282,9 @@ static int xhci_setup_msix(struct xhci_hcd *xhci)
disable_msix:
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "disable MSI-X interrupt");
- xhci_free_irq(xhci);
- pci_disable_msix(pdev);
-free_entries:
- kfree(xhci->msix_entries);
- xhci->msix_entries = NULL;
+ while (--i >= 0)
+ free_irq(pci_irq_vector(pdev, i), xhci_to_hcd(xhci));
+ pci_free_irq_vectors(pdev);
return ret;
}
@@ -346,27 +297,33 @@ static void xhci_cleanup_msix(struct xhci_hcd *xhci)
if (xhci->quirks & XHCI_PLAT)
return;
- xhci_free_irq(xhci);
+ /* return if using legacy interrupt */
+ if (hcd->irq > 0)
+ return;
+
+ if (hcd->msix_enabled) {
+ int i;
- if (xhci->msix_entries) {
- pci_disable_msix(pdev);
- kfree(xhci->msix_entries);
- xhci->msix_entries = NULL;
+ for (i = 0; i < xhci->msix_cou
Re: [PATCH] fs: configfs: don't return anything from drop_link
Thanks a lot Andrzej! I've applied the patch, but I undid the reformatting of the nvmet code to keep the patch as simple as possible. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: uvc: fix returnvar.cocci warnings
On Wed, Nov 23, 2016 at 09:35:36AM +0100, Andrzej Pietrasiewicz wrote: > The ->drop_item() is indeed a void function, the ->drop_link() is > actually not. This, together with the fact that the value of ->drop_link() > is silently ignored suggests, that it is the ->drop_link() return > type that should be corrected and changed to void. Please send a patch to change it. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] uas: Limit qdepth at the scsi-host level
On Sat, Mar 19, 2016 at 10:06:16AM -0700, James Bottomley wrote: > Help me understand this bug a bit more. Are you saying that the commit > you identify is causing the block layer to queue more commands than > you've set the per-lun limit to? In which case we have a serious > problem for more than just UAS. Or are you saying that UAS always had > a global command limit, but it just didn't get set correctly; however, > it mostly worked until the above commit exposed the problem? I did mishandle uas in my patch to make the host wide tags the default and didn't notice that uas set a different limit for the host wide tag limit and ->can_queue. With the old code we'd apply the lower of the two limits, which was the host wide tag limit, but my patch removed that one and now purely relies on can_queue. Now most uas devices only have a single lun, and the per-lun limit was set to what the global limit should be as well, so most users didn't really notice this. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] uas: Limit qdepth at the scsi-host level
On Sat, Mar 19, 2016 at 09:59:12AM +0100, Hans de Goede wrote:
> Commit 64d513ac31bd ("scsi: use host wide tags by default") causes
> the scsi-core to queue more cmnds then we can handle on devices with
> multiple LUNs, limit the qdepth at the scsi-host level instead of
> per slave to fix this.
>
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1315013
> Cc: sta...@vger.kernel.org # 4.4.x and 4.5.x
> Signed-off-by: Hans de Goede
Looks good,
Reviewed-by: Christoph Hellwig
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: f_acm: Fix configfs attr name
On Tue, Mar 01, 2016 at 12:47:11PM +0100, Krzysztof Opasiak wrote:
> Correct attribute name is port_num not num.
>
> Signed-off-by: Krzysztof Opasiak
> Fixes: ea6bd6b ("usb-gadget/f_acm: use per-attribute show and store methods")
My fault, sorry.
Reviewed-by: Christoph Hellwig
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] configfs: switch ->default groups to a linked list
On Sun, Feb 28, 2016 at 12:58:42PM +0200, Sagi Grimberg wrote: >> -As a consequence of this, default_groups cannot be removed directly via >> +As a consequence of this, default groups cannot be removed directly via >> rmdir(2). They also are not considered when rmdir(2) on the parent >> group is checking for children. >> > > What's changed here? I removed the underscore in default_groups - while we still have a member of that name it's not exposed directly anymore, so I'd rather not have the documentation refer to it. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] configfs: switch ->default groups to a linked list
Replace the current NULL-terminated array of default groups with a linked
list. This gets rid of lots of nasty code to size and/or dynamically
allocate the array.
While we're at it also provide a conveniant helper to remove the default
groups.
Signed-off-by: Christoph Hellwig
---
Documentation/filesystems/configfs/configfs.txt | 11 +-
drivers/infiniband/core/cma_configfs.c | 31 ++--
drivers/target/iscsi/iscsi_target_configfs.c| 75 +++--
drivers/target/target_core_configfs.c | 203 +---
drivers/target/target_core_fabric_configfs.c| 194 ++
drivers/target/target_core_internal.h | 1 -
drivers/target/target_core_stat.c | 41 ++---
drivers/usb/gadget/configfs.c | 36 ++---
drivers/usb/gadget/function/f_mass_storage.c| 6 +-
drivers/usb/gadget/function/f_rndis.c | 1 -
drivers/usb/gadget/function/uvc_configfs.c | 198 +--
fs/configfs/dir.c | 44 +++--
fs/configfs/item.c | 1 +
fs/dlm/config.c | 38 +
fs/ocfs2/cluster/nodemanager.c | 22 +--
include/linux/configfs.h| 11 +-
include/target/target_core_base.h | 3 -
17 files changed, 284 insertions(+), 632 deletions(-)
diff --git a/Documentation/filesystems/configfs/configfs.txt
b/Documentation/filesystems/configfs/configfs.txt
index e5fe521..8ec9136 100644
--- a/Documentation/filesystems/configfs/configfs.txt
+++ b/Documentation/filesystems/configfs/configfs.txt
@@ -250,7 +250,8 @@ child item.
struct config_item cg_item;
struct list_headcg_children;
struct configfs_subsystem *cg_subsys;
- struct config_group **default_groups;
+ struct list_headdefault_groups;
+ struct list_headgroup_entry;
};
void config_group_init(struct config_group *group);
@@ -420,15 +421,15 @@ These automatic subgroups, or default groups, do not
preclude other
children of the parent group. If ct_group_ops->make_group() exists,
other child groups can be created on the parent group directly.
-A configfs subsystem specifies default groups by filling in the
-NULL-terminated array default_groups on the config_group structure.
-Each group in that array is populated in the configfs tree at the same
+A configfs subsystem specifies default groups by adding them using the
+configfs_add_default_group() function to the parent config_group
+structure. Each added group is populated in the configfs tree at the same
time as the parent group. Similarly, they are removed at the same time
as the parent. No extra notification is provided. When a ->drop_item()
method call notifies the subsystem the parent group is going away, it
also means every default group child associated with that parent group.
-As a consequence of this, default_groups cannot be removed directly via
+As a consequence of this, default groups cannot be removed directly via
rmdir(2). They also are not considered when rmdir(2) on the parent
group is checking for children.
diff --git a/drivers/infiniband/core/cma_configfs.c
b/drivers/infiniband/core/cma_configfs.c
index 18b112a..41573df 100644
--- a/drivers/infiniband/core/cma_configfs.c
+++ b/drivers/infiniband/core/cma_configfs.c
@@ -49,8 +49,6 @@ struct cma_dev_group {
charname[IB_DEVICE_NAME_MAX];
struct config_group device_group;
struct config_group ports_group;
- struct config_group *default_dev_group[2];
- struct config_group **default_ports_group;
struct cma_dev_port_group *ports;
};
@@ -158,7 +156,6 @@ static int make_cma_ports(struct cma_dev_group
*cma_dev_group,
unsigned int i;
unsigned int ports_num;
struct cma_dev_port_group *ports;
- struct config_group **ports_group;
int err;
ibdev = cma_get_ib_dev(cma_dev);
@@ -169,9 +166,8 @@ static int make_cma_ports(struct cma_dev_group
*cma_dev_group,
ports_num = ibdev->phys_port_cnt;
ports = kcalloc(ports_num, sizeof(*cma_dev_group->ports),
GFP_KERNEL);
- ports_group = kcalloc(ports_num + 1, sizeof(*ports_group), GFP_KERNEL);
- if (!ports || !ports_group) {
+ if (!ports) {
err = -ENOMEM;
goto free;
}
@@ -185,18 +181,16 @@ static int make_cma_ports(struct cma_dev_group
*cma_dev_group,
config_group_init_type_name(&ports[i].group,
port_str,
&cma_port_group_type);
- ports_g
Re: [PATCH] Add support for usbfs zerocopy.
This is a completely broken usage of the mmap interface. if you use mmap on a device file you must use the actual mmap for the data transfer. Our interface for zero copy reads/writes is O_DIRECT, and that requires not special memory allocation, just proper alignment. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: uvc: fix permissions of configfs attributes
Hi Mian, sorry for the bug, the fix looks correct. Reviewed-by: Christoph Hellwig -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Infrastructure for zerocopy I/O
On Tue, Nov 17, 2015 at 02:07:58PM -0500, Alan Stern wrote: > If we really want to do zerocopy I/O then we should not use a bounce > buffer. But the only way to avoid bounce buffers may be to give user > programs a way of allocating memory pages below 4 GB, because lots of > USB hardware can only do 32-bit DMA. But any system worth it's money these days has an IOMMU. > Is there an API for allocating user memory below 4 GB? Would a new > MMAP flag be acceptable? You'll have to ask the MM folks. But I doubt they will be excited about it. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Infrastructure for zerocopy I/O
On Mon, Nov 16, 2015 at 03:22:06PM -0500, Alan Stern wrote: > In other words, you're suggesting we do this: > > Check that userspace requested zerocopy (otherwise the user > program might try to access other data stored in the same cache > lines as the buffer while the I/O is in progres); > > Call get_user_pages (or get_user_pages_fast? -- it's not clear > which should be used) for this buffer; > > Use the array of pages returned by that routine to populate > a scatter-gather list (sg_alloc_table_from_pages); > > Pass that list to dma_map_sg. > > Is that right? Yes. > Does dma_map_sg check the page addresses against the DMA mask and > automatically create a bounce buffer, or do we have to do that > manually? Documentation/DMA-API-HOWTO.txt doesn't discuss this. You need to do this manually. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Infrastructure for zerocopy I/O
On Mon, Nov 16, 2015 at 08:03:12PM +0100, Steinar H. Gunderson wrote: > The original use case: DVB capture on embedded devices. > > My use case: USB3 uncompressed HD video capture (from multiple cards). > > Both of these hit (beyond the speed boost from zerocopy) the problem that > as time goes by, memory gets more fragmented and usbfs fails allocation. > Allocating memory up-front solves that. As said I think you should just use get_user_pages() on user memory, and bounce buffer if it doesn't fit the DMA mask. Thіs also allows the user to use hugetlbs if they need large contiguous allocations for performance reasons. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Infrastructure for zerocopy I/O
On Mon, Nov 16, 2015 at 01:29:58PM -0500, Alan Stern wrote: > Allocating memory pages that match the device's DMA mask, for > use as I/O buffers, and locking them so their physical > addresses don't change (and they don't get paged out); > > Mapping those pages into a user process; > > Constructing scatter-gather lists to describe the I/O if the > pages aren't contiguous. > > I don't have much experience in this area. Can you point to good > examples where these things are done or documents describing what is > involved? For example, are the allocating and mapping steps normally > done separately or are they normally combined in an mmap(2) system > call? The answer is probably not what you want to here, but the recommendation is to not bother. Just use the iommu for this one reasonable hardware, or swiotlb for bounce buffering if your systems has more addressable memory than what the hardware can address. This is what we've been doing for block I/O since day 1. > A proposed patch has been posted > (http://marc.info/?l=linux-usb&m=144763502829452&w=2), but I'm not > convinced that it is the best approach. For instance, it always tries > to get contiguous pages and so is vulnerable to memory fragmentation. This looks totally crazy to me. What's the use case for it? -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/21] usb/gadget: Remove set-but-not-used variables
On Thu, Oct 22, 2015 at 03:55:45PM -0700, Bart Van Assche wrote: > Avoid that building with W=1 triggers compiler warnings about > set-but-not-used variables. Looks good, Reviewed-by: Christoph Hellwig -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 01/15] fs: configfs: Add unlocked version of configfs_depend_item()
Hi Andrzej, please find a way to share code between the two depend function. And also drop the duplicate undepend version and just stop passing the unused subsystem argument. Not only do we not keep unused argument in Linux in general, but in this case it's also really useful for the new API. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] fs: configfs: Add unlocked version of configfs_depend_item()
On Tue, Oct 20, 2015 at 02:32:32PM +0200, Andrzej Pietrasiewicz wrote:
> From: Krzysztof Opasiak
>
> This change is necessary for the SCSI target usb gadget composed with
> configfs. In this case configfs will be used for two different purposes:
> to compose a usb gadget and to configure the target part. If an instance
> of tcm function is created in $CONFIGFS_ROOT/usb_gadget//functions
> a tpg can be created in $CONFIGFS_ROOT/target/usb_gadget//, but after
> a tpg is created the tcm function must not be removed until its
> corresponding tpg is gone. While the configfs_depend/undepend_item() are
> meant exactly for creating this kind of dependencies, they are not suitable
> if the other kernel subsystem happens to be another subsystem in configfs,
> so this patch adds unlocked versions meant for configfs callbacks.
configfs_depend_item is a lot of deep magic (which I haven't even tried
to fully understand yet). Can you explain why your versions is safe
without all that? Can also please document the precoditions required
for the function in comments, or wherever possible asserts?
Also needs a kerneldoc comment explaining the exact semantics.
> +void configfs_undepend_item_unlocked(struct config_item *target)
> +{
> + configfs_undepend_item(NULL, target);
> +}
> +EXPORT_SYMBOL(configfs_undepend_item_unlocked);
And given that configfs_undepend_item ignores the subsys argument this
wrapper clearl isn't needed. Just send a separate patch to drop the
subsys argument to configfs_undepend_item instead.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: simplify configfs attributes V2
On Mon, Oct 05, 2015 at 02:37:05PM -0700, Andrew Morton wrote: > > This series consolidates the code to implement configfs attributes > > by providing the ->show and ->store method in common code and using > > container_of in the methods to access the containing structure. > > > > This reduces source and binary size of configfs consumers a lot. > > There's a version of this patch series (I assume V1) in linux-next via > Nicholas's tree so my hands are tied. I trust that Nicholas will > update things. > > Or maybe it was a slipup - modifying usb, ocfs2 etc from the iscsi tree > is innovative ;) Nic wanted this as the target code is the biggest user of configfs. I don't really care where it goes as long as we get in the current version into a stable not to be rebased branch somehwere so that all consumers can use that as a base point. Nic, can you do that? A lot of previous work went in through Andrew, but I think -mm is still a quilt stack so the stable base branch wouldn't be possible that way. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: simplify configfs attributes V2
On Fri, Oct 09, 2015 at 04:37:51PM -0500, Felipe Balbi wrote: > For reference, I'm fine if you guys take all patches through FS > tree. Another option is waiting for dependencies to be merged in v4.4, > and the gadget changes merge in v4.5, whatever works. I'd prefer to take them all in one go. If we really have to I could drop the last patch for now and clean up later, but I'd prefer not to. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/23] spear13xx_pcie_gadget: use per-attribute show and store methods
On Fri, Oct 09, 2015 at 04:05:17PM -0500, Felipe Balbi wrote: > Pratyush Anand writes: > > > On Sat, Oct 3, 2015 at 7:02 PM, Christoph Hellwig wrote: > >> Signed-off-by: Christoph Hellwig > > > > Acked-by: Pratyush Anand > > I don't seem to have the actual patch, care to resend? The whole series was sent to all the receipients in the To and Cc lists, so check your spam folder or one of the list archives. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/23] usb-gadget: use per-attribute show and store methods
On Fri, Oct 09, 2015 at 04:19:57PM -0500, Felipe Balbi wrote: > > Signed-off-by: Christoph Hellwig > > Reviewed-by: Andrzej Pietrasiewicz > > I suppose this depends on other fs/configfs changes ? The whole series should be applied in order, but doesn't have any other dependencies. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/15] pcnet32: use pci_set_dma_mask insted of pci_dma_supported
This ensures the dma mask that is supported by the driver is recorded
in the device structure.
Signed-off-by: Christoph Hellwig
---
drivers/net/ethernet/amd/pcnet32.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/amd/pcnet32.c
b/drivers/net/ethernet/amd/pcnet32.c
index bc8b04f..e2afabf 100644
--- a/drivers/net/ethernet/amd/pcnet32.c
+++ b/drivers/net/ethernet/amd/pcnet32.c
@@ -1500,7 +1500,7 @@ pcnet32_probe_pci(struct pci_dev *pdev, const struct
pci_device_id *ent)
return -ENODEV;
}
- if (!pci_dma_supported(pdev, PCNET32_DMA_MASK)) {
+ if (!pci_set_dma_mask(pdev, PCNET32_DMA_MASK)) {
if (pcnet32_debug & NETIF_MSG_PROBE)
pr_err("architecture does not support 32bit PCI
busmaster DMA\n");
return -ENODEV;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
