linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: USB host support should depend on HAS_DMA
@ 2013-07-10 21:18 Geert Uytterhoeven
  2013-07-10 21:31 ` Alan Stern
  0 siblings, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2013-07-10 21:18 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman
  Cc: Alan Stern, linux-input, linux-media, linux-usb, linux-kernel,
	Geert Uytterhoeven

If NO_DMA=y:

drivers/built-in.o: In function `usb_hcd_unmap_urb_setup_for_dma':
drivers/usb/core/hcd.c:1361: undefined reference to `dma_unmap_single'
drivers/built-in.o: In function `usb_hcd_unmap_urb_for_dma':
drivers/usb/core/hcd.c:1393: undefined reference to `dma_unmap_sg'
drivers/usb/core/hcd.c:1398: undefined reference to `dma_unmap_page'
drivers/usb/core/hcd.c:1403: undefined reference to `dma_unmap_single'
drivers/built-in.o: In function `usb_hcd_map_urb_for_dma':
drivers/usb/core/hcd.c:1445: undefined reference to `dma_map_single'
drivers/usb/core/hcd.c:1450: undefined reference to `dma_mapping_error'
drivers/usb/core/hcd.c:1480: undefined reference to `dma_map_sg'
drivers/usb/core/hcd.c:1495: undefined reference to `dma_map_page'
drivers/usb/core/hcd.c:1501: undefined reference to `dma_mapping_error'
drivers/usb/core/hcd.c:1507: undefined reference to `dma_map_single'
drivers/usb/core/hcd.c:1512: undefined reference to `dma_mapping_error'
drivers/built-in.o: In function `hcd_buffer_free':
drivers/usb/core/buffer.c:146: undefined reference to `dma_pool_free'
drivers/usb/core/buffer.c:150: undefined reference to `dma_free_coherent'
drivers/built-in.o: In function `hcd_buffer_destroy':
drivers/usb/core/buffer.c:90: undefined reference to `dma_pool_destroy'
drivers/built-in.o: In function `hcd_buffer_create':
drivers/usb/core/buffer.c:65: undefined reference to `dma_pool_create'
drivers/built-in.o: In function `hcd_buffer_alloc':
drivers/usb/core/buffer.c:120: undefined reference to `dma_pool_alloc'
drivers/usb/core/buffer.c:122: undefined reference to `dma_alloc_coherent'
,,,

Commit d9ea21a779278da06d0cbe989594bf542ed213d7 ("usb: host: make
USB_ARCH_HAS_?HCI obsolete") allowed to enable USB on platforms with
NO_DMA=y, and exposed several input and media USB drivers that just select
USB if USB_ARCH_HAS_HCD, without checking HAS_DMA.

Fix the former by making USB depend on HAS_DMA.

To fix the latter, instead of adding lots of "depends on HAS_DMA", make
those drivers depend on USB, instead of depending on USB_ARCH_HAS_HCD and
selecting USB.  Drivers for other busses (e.g. MOUSE_SYNAPTICS_I2C) already
handle this in a similar way.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/input/joystick/Kconfig    |    3 +--
 drivers/input/misc/Kconfig        |   15 +++++----------
 drivers/input/mouse/Kconfig       |    9 +++------
 drivers/input/tablet/Kconfig      |   15 +++++----------
 drivers/input/touchscreen/Kconfig |    3 +--
 drivers/media/rc/Kconfig          |   21 +++++++--------------
 drivers/usb/Kconfig               |    2 +-
 7 files changed, 23 insertions(+), 45 deletions(-)

diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index 56eb471..d7e36fb 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -278,8 +278,7 @@ config JOYSTICK_JOYDUMP
 
 config JOYSTICK_XPAD
 	tristate "X-Box gamepad support"
-	depends on USB_ARCH_HAS_HCD
-	select USB
+	depends on USB
 	help
 	  Say Y here if you want to use the X-Box pad with your computer.
 	  Make sure to say Y to "Joystick support" (CONFIG_INPUT_JOYDEV)
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 0b541cd..00cdecb 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -286,8 +286,7 @@ config INPUT_ATLAS_BTNS
 
 config INPUT_ATI_REMOTE2
 	tristate "ATI / Philips USB RF remote control"
-	depends on USB_ARCH_HAS_HCD
-	select USB
+	depends on USB
 	help
 	  Say Y here if you want to use an ATI or Philips USB RF remote control.
 	  These are RF remotes with USB receivers.
@@ -301,8 +300,7 @@ config INPUT_ATI_REMOTE2
 
 config INPUT_KEYSPAN_REMOTE
 	tristate "Keyspan DMR USB remote control"
-	depends on USB_ARCH_HAS_HCD
-	select USB
+	depends on USB
 	help
 	  Say Y here if you want to use a Keyspan DMR USB remote control.
 	  Currently only the UIA-11 type of receiver has been tested.  The tag
@@ -333,8 +331,7 @@ config INPUT_KXTJ9_POLLED_MODE
 
 config INPUT_POWERMATE
 	tristate "Griffin PowerMate and Contour Jog support"
-	depends on USB_ARCH_HAS_HCD
-	select USB
+	depends on USB
 	help
 	  Say Y here if you want to use Griffin PowerMate or Contour Jog devices.
 	  These are aluminum dials which can measure clockwise and anticlockwise
@@ -349,8 +346,7 @@ config INPUT_POWERMATE
 
 config INPUT_YEALINK
 	tristate "Yealink usb-p1k voip phone"
-	depends on USB_ARCH_HAS_HCD
-	select USB
+	depends on USB
 	help
 	  Say Y here if you want to enable keyboard and LCD functions of the
 	  Yealink usb-p1k usb phones. The audio part is enabled by the generic
@@ -364,8 +360,7 @@ config INPUT_YEALINK
 
 config INPUT_CM109
 	tristate "C-Media CM109 USB I/O Controller"
-	depends on USB_ARCH_HAS_HCD
-	select USB
+	depends on USB
 	help
 	  Say Y here if you want to enable keyboard and buzzer functions of the
 	  C-Media CM109 usb phones. The audio part is enabled by the generic
diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index effa9c5..90f8c0b 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -161,8 +161,7 @@ config MOUSE_SERIAL
 
 config MOUSE_APPLETOUCH
 	tristate "Apple USB Touchpad support"
-	depends on USB_ARCH_HAS_HCD
-	select USB
+	depends on USB
 	help
 	  Say Y here if you want to use an Apple USB Touchpad.
 
@@ -182,8 +181,7 @@ config MOUSE_APPLETOUCH
 
 config MOUSE_BCM5974
 	tristate "Apple USB BCM5974 Multitouch trackpad support"
-	depends on USB_ARCH_HAS_HCD
-	select USB
+	depends on USB
 	help
 	  Say Y here if you have an Apple USB BCM5974 Multitouch
 	  trackpad.
@@ -346,8 +344,7 @@ config MOUSE_SYNAPTICS_I2C
 
 config MOUSE_SYNAPTICS_USB
 	tristate "Synaptics USB device support"
-	depends on USB_ARCH_HAS_HCD
-	select USB
+	depends on USB
 	help
 	  Say Y here if you want to use a Synaptics USB touchpad or pointing
 	  stick.
diff --git a/drivers/input/tablet/Kconfig b/drivers/input/tablet/Kconfig
index bed7cbf..8e27600 100644
--- a/drivers/input/tablet/Kconfig
+++ b/drivers/input/tablet/Kconfig
@@ -13,8 +13,7 @@ if INPUT_TABLET
 
 config TABLET_USB_ACECAD
 	tristate "Acecad Flair tablet support (USB)"
-	depends on USB_ARCH_HAS_HCD
-	select USB
+	depends on USB
 	help
 	  Say Y here if you want to use the USB version of the Acecad Flair
 	  tablet.  Make sure to say Y to "Mouse support"
@@ -26,8 +25,7 @@ config TABLET_USB_ACECAD
 
 config TABLET_USB_AIPTEK
 	tristate "Aiptek 6000U/8000U and Genius G_PEN tablet support (USB)"
-	depends on USB_ARCH_HAS_HCD
-	select USB
+	depends on USB
 	help
 	  Say Y here if you want to use the USB version of the Aiptek 6000U,
 	  Aiptek 8000U or Genius G-PEN 560 tablet.  Make sure to say Y to
@@ -51,8 +49,7 @@ config TABLET_USB_GTCO
 
 config TABLET_USB_HANWANG
 	tristate "Hanwang Art Master III tablet support (USB)"
-	depends on USB_ARCH_HAS_HCD
-	select USB
+	depends on USB
 	help
 	  Say Y here if you want to use the USB version of the Hanwang Art
 	  Master III tablet.
@@ -62,8 +59,7 @@ config TABLET_USB_HANWANG
 
 config TABLET_USB_KBTAB
 	tristate "KB Gear JamStudio tablet support (USB)"
-	depends on USB_ARCH_HAS_HCD
-	select USB
+	depends on USB
 	help
 	  Say Y here if you want to use the USB version of the KB Gear
 	  JamStudio tablet.  Make sure to say Y to "Mouse support"
@@ -75,9 +71,8 @@ config TABLET_USB_KBTAB
 
 config TABLET_USB_WACOM
 	tristate "Wacom Intuos/Graphire tablet support (USB)"
-	depends on USB_ARCH_HAS_HCD
+	depends on USB
 	select POWER_SUPPLY
-	select USB
 	select NEW_LEDS
 	select LEDS_CLASS
 	help
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 3b9758b..a889c52 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -689,8 +689,7 @@ config TOUCHSCREEN_WM97XX_ZYLONITE
 
 config TOUCHSCREEN_USB_COMPOSITE
 	tristate "USB Touchscreen Driver"
-	depends on USB_ARCH_HAS_HCD
-	select USB
+	depends on USB
 	help
 	  USB Touchscreen driver for:
 	  - eGalax Touchkit USB (also includes eTurboTouch CT-410/510/700)
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 5a79c33..ee59842 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -126,9 +126,8 @@ if RC_DEVICES
 
 config RC_ATI_REMOTE
 	tristate "ATI / X10 based USB RF remote controls"
-	depends on USB_ARCH_HAS_HCD
+	depends on USB
 	depends on RC_CORE
-	select USB
 	help
 	   Say Y here if you want to use an X10 based USB remote control.
 	   These are RF remotes with USB receivers.
@@ -159,9 +158,8 @@ config IR_ENE
 
 config IR_IMON
 	tristate "SoundGraph iMON Receiver and Display"
-	depends on USB_ARCH_HAS_HCD
+	depends on USB
 	depends on RC_CORE
-	select USB
 	---help---
 	   Say Y here if you want to use a SoundGraph iMON (aka Antec Veris)
 	   IR Receiver and/or LCD/VFD/VGA display.
@@ -171,9 +169,8 @@ config IR_IMON
 
 config IR_MCEUSB
 	tristate "Windows Media Center Ed. eHome Infrared Transceiver"
-	depends on USB_ARCH_HAS_HCD
+	depends on USB
 	depends on RC_CORE
-	select USB
 	---help---
 	   Say Y here if you want to use a Windows Media Center Edition
 	   eHome Infrared Transceiver.
@@ -221,9 +218,8 @@ config IR_NUVOTON
 
 config IR_REDRAT3
 	tristate "RedRat3 IR Transceiver"
-	depends on USB_ARCH_HAS_HCD
+	depends on USB
 	depends on RC_CORE
-	select USB
 	---help---
 	   Say Y here if you want to use a RedRat3 Infrared Transceiver.
 
@@ -232,9 +228,8 @@ config IR_REDRAT3
 
 config IR_STREAMZAP
 	tristate "Streamzap PC Remote IR Receiver"
-	depends on USB_ARCH_HAS_HCD
+	depends on USB
 	depends on RC_CORE
-	select USB
 	---help---
 	   Say Y here if you want to use a Streamzap PC Remote
 	   Infrared Receiver.
@@ -261,9 +256,8 @@ config IR_WINBOND_CIR
 
 config IR_IGUANA
 	tristate "IguanaWorks USB IR Transceiver"
-	depends on USB_ARCH_HAS_HCD
+	depends on USB
 	depends on RC_CORE
-	select USB
 	---help---
 	   Say Y here if you want to use the IguanaWorks USB IR Transceiver.
 	   Both infrared receive and send are supported. If you want to
@@ -277,9 +271,8 @@ config IR_IGUANA
 
 config IR_TTUSBIR
 	tristate "TechnoTrend USB IR Receiver"
-	depends on USB_ARCH_HAS_HCD
+	depends on USB
 	depends on RC_CORE
-	select USB
 	select NEW_LEDS
 	select LEDS_CLASS
 	---help---
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 73f62ca..c530ad9 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -33,7 +33,7 @@ config USB_ARCH_HAS_HCD
 # ARM SA1111 chips have a non-PCI based "OHCI-compatible" USB host interface.
 config USB
 	tristate "Support for Host-side USB"
-	depends on USB_ARCH_HAS_HCD
+	depends on USB_ARCH_HAS_HCD && HAS_DMA
 	select NLS  # for UTF-8 strings
 	---help---
 	  Universal Serial Bus (USB) is a specification for a serial bus
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] usb: USB host support should depend on HAS_DMA
  2013-07-10 21:18 [PATCH] usb: USB host support should depend on HAS_DMA Geert Uytterhoeven
@ 2013-07-10 21:31 ` Alan Stern
  2013-07-10 23:12   ` Arnd Bergmann
  0 siblings, 1 reply; 8+ messages in thread
From: Alan Stern @ 2013-07-10 21:31 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-input, linux-media,
	linux-usb, linux-kernel

On Wed, 10 Jul 2013, Geert Uytterhoeven wrote:

> If NO_DMA=y:
> 
> drivers/built-in.o: In function `usb_hcd_unmap_urb_setup_for_dma':
> drivers/usb/core/hcd.c:1361: undefined reference to `dma_unmap_single'

> ,,,
> 
> Commit d9ea21a779278da06d0cbe989594bf542ed213d7 ("usb: host: make
> USB_ARCH_HAS_?HCI obsolete") allowed to enable USB on platforms with
> NO_DMA=y, and exposed several input and media USB drivers that just select
> USB if USB_ARCH_HAS_HCD, without checking HAS_DMA.
> 
> Fix the former by making USB depend on HAS_DMA.

This isn't right.  There are USB host controllers that use PIO, not
DMA.  The HAS_DMA dependency should go with the controller driver, not 
the USB core.

On the other hand, the USB core does call various routines like 
dma_unmap_single.  It ought to be possible to compile these calls even 
when DMA isn't enabled.  That is, they should be defined as do-nothing 
stubs.

> To fix the latter, instead of adding lots of "depends on HAS_DMA", make
> those drivers depend on USB, instead of depending on USB_ARCH_HAS_HCD and
> selecting USB.  Drivers for other busses (e.g. MOUSE_SYNAPTICS_I2C) already
> handle this in a similar way.

That seems reasonable.

Alan Stern


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] usb: USB host support should depend on HAS_DMA
  2013-07-10 21:31 ` Alan Stern
@ 2013-07-10 23:12   ` Arnd Bergmann
  2013-07-11  1:01     ` Alan Stern
  2013-08-18 21:12     ` Geert Uytterhoeven
  0 siblings, 2 replies; 8+ messages in thread
From: Arnd Bergmann @ 2013-07-10 23:12 UTC (permalink / raw)
  To: Alan Stern
  Cc: Geert Uytterhoeven, Greg Kroah-Hartman, linux-input, linux-media,
	linux-usb, linux-kernel

On Wednesday 10 July 2013, Alan Stern wrote:
> This isn't right.  There are USB host controllers that use PIO, not
> DMA.  The HAS_DMA dependency should go with the controller driver, not 
> the USB core.
> 
> On the other hand, the USB core does call various routines like 
> dma_unmap_single.  It ought to be possible to compile these calls even 
> when DMA isn't enabled.  That is, they should be defined as do-nothing 
> stubs.

The asm-generic/dma-mapping-broken.h file intentionally causes link
errors, but that could be changed.

The better approach in my mind would be to replace code like


	if (hcd->self.uses_dma)

with

	if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {

which will reliably cause that reference to be omitted from object code,
but not stop giving link errors for drivers that actually require
DMA.

	Arnd

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] usb: USB host support should depend on HAS_DMA
  2013-07-10 23:12   ` Arnd Bergmann
@ 2013-07-11  1:01     ` Alan Stern
  2013-07-11  7:49       ` Geert Uytterhoeven
  2013-08-18 21:12     ` Geert Uytterhoeven
  1 sibling, 1 reply; 8+ messages in thread
From: Alan Stern @ 2013-07-11  1:01 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Geert Uytterhoeven, Greg Kroah-Hartman, linux-input, linux-media,
	linux-usb, linux-kernel

On Thu, 11 Jul 2013, Arnd Bergmann wrote:

> On Wednesday 10 July 2013, Alan Stern wrote:
> > This isn't right.  There are USB host controllers that use PIO, not
> > DMA.  The HAS_DMA dependency should go with the controller driver, not 
> > the USB core.
> > 
> > On the other hand, the USB core does call various routines like 
> > dma_unmap_single.  It ought to be possible to compile these calls even 
> > when DMA isn't enabled.  That is, they should be defined as do-nothing 
> > stubs.
> 
> The asm-generic/dma-mapping-broken.h file intentionally causes link
> errors, but that could be changed.
> 
> The better approach in my mind would be to replace code like
> 
> 
> 	if (hcd->self.uses_dma)
> 
> with
> 
> 	if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
> 
> which will reliably cause that reference to be omitted from object code,
> but not stop giving link errors for drivers that actually require
> DMA.

How will it give link errors for drivers that require DMA?

Besides, wouldn't it be better to get an error at config time rather
than at link time?  Or even better still, not to be allowed to
configure drivers that depend on DMA if DMA isn't available?

If we add an explicit dependency for HAS_DMA to the Kconfig entries for 
these drivers, then your suggestion would be a good way to allow 
usbcore to be built independently of DMA support.

Alan Stern


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] usb: USB host support should depend on HAS_DMA
  2013-07-11  1:01     ` Alan Stern
@ 2013-07-11  7:49       ` Geert Uytterhoeven
  2013-07-11 15:03         ` Alan Stern
  0 siblings, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2013-07-11  7:49 UTC (permalink / raw)
  To: Alan Stern
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-input,
	Linux Media Mailing List, USB list, linux-kernel

On Thu, Jul 11, 2013 at 3:01 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Thu, 11 Jul 2013, Arnd Bergmann wrote:
>
>> On Wednesday 10 July 2013, Alan Stern wrote:
>> > This isn't right.  There are USB host controllers that use PIO, not
>> > DMA.  The HAS_DMA dependency should go with the controller driver, not
>> > the USB core.
>> >
>> > On the other hand, the USB core does call various routines like
>> > dma_unmap_single.  It ought to be possible to compile these calls even
>> > when DMA isn't enabled.  That is, they should be defined as do-nothing
>> > stubs.
>>
>> The asm-generic/dma-mapping-broken.h file intentionally causes link
>> errors, but that could be changed.
>>
>> The better approach in my mind would be to replace code like
>>
>>
>>       if (hcd->self.uses_dma)
>>
>> with
>>
>>       if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
>>
>> which will reliably cause that reference to be omitted from object code,
>> but not stop giving link errors for drivers that actually require
>> DMA.
>
> How will it give link errors for drivers that require DMA?

It won't. Unless the host driver itself calls into the DMA API, too
(are there any that don't?).

> Besides, wouldn't it be better to get an error at config time rather
> than at link time?  Or even better still, not to be allowed to
> configure drivers that depend on DMA if DMA isn't available?

Indeed.

> If we add an explicit dependency for HAS_DMA to the Kconfig entries for
> these drivers, then your suggestion would be a good way to allow
> usbcore to be built independently of DMA support.

However, having the link errors helps when annotating the Kconfig files
with HAS_DMA dependencies.

Unfortunately the check for "hcd->self.uses_dma" (which boils down to
"dev->dma_mask != NULL") isn't sufficient to cause breakage at compilation
time when a Kconfig entry incorrectly doesn't depend on HAS_DMA.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] usb: USB host support should depend on HAS_DMA
  2013-07-11  7:49       ` Geert Uytterhoeven
@ 2013-07-11 15:03         ` Alan Stern
  0 siblings, 0 replies; 8+ messages in thread
From: Alan Stern @ 2013-07-11 15:03 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-input,
	Linux Media Mailing List, USB list, linux-kernel

On Thu, 11 Jul 2013, Geert Uytterhoeven wrote:

> On Thu, Jul 11, 2013 at 3:01 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> > On Thu, 11 Jul 2013, Arnd Bergmann wrote:
> >
> >> On Wednesday 10 July 2013, Alan Stern wrote:
> >> > This isn't right.  There are USB host controllers that use PIO, not
> >> > DMA.  The HAS_DMA dependency should go with the controller driver, not
> >> > the USB core.
> >> >
> >> > On the other hand, the USB core does call various routines like
> >> > dma_unmap_single.  It ought to be possible to compile these calls even
> >> > when DMA isn't enabled.  That is, they should be defined as do-nothing
> >> > stubs.
> >>
> >> The asm-generic/dma-mapping-broken.h file intentionally causes link
> >> errors, but that could be changed.
> >>
> >> The better approach in my mind would be to replace code like
> >>
> >>
> >>       if (hcd->self.uses_dma)
> >>
> >> with
> >>
> >>       if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
> >>
> >> which will reliably cause that reference to be omitted from object code,
> >> but not stop giving link errors for drivers that actually require
> >> DMA.
> >
> > How will it give link errors for drivers that require DMA?
> 
> It won't. Unless the host driver itself calls into the DMA API, too
> (are there any that don't?).

To my knowledge, all the host controller drivers which use DMA _do_
call functions in the DMA API.  So they would still get link errors,
even though the USB core wouldn't.

Therefore adding the appropriate HAS_DMA dependencies should be 
straightforward: Try to build all the drivers and see which ones fail 
to link.

Alan Stern


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] usb: USB host support should depend on HAS_DMA
  2013-07-10 23:12   ` Arnd Bergmann
  2013-07-11  1:01     ` Alan Stern
@ 2013-08-18 21:12     ` Geert Uytterhoeven
  2013-08-19 15:09       ` Alan Stern
  1 sibling, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2013-08-18 21:12 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Alan Stern, Greg Kroah-Hartman, linux-input,
	Linux Media Mailing List, USB list, linux-kernel

On Thu, Jul 11, 2013 at 1:12 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 10 July 2013, Alan Stern wrote:
>> This isn't right.  There are USB host controllers that use PIO, not
>> DMA.  The HAS_DMA dependency should go with the controller driver, not
>> the USB core.
>>
>> On the other hand, the USB core does call various routines like
>> dma_unmap_single.  It ought to be possible to compile these calls even
>> when DMA isn't enabled.  That is, they should be defined as do-nothing
>> stubs.
>
> The asm-generic/dma-mapping-broken.h file intentionally causes link
> errors, but that could be changed.
>
> The better approach in my mind would be to replace code like
>
>
>         if (hcd->self.uses_dma)
>
> with
>
>         if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
>
> which will reliably cause that reference to be omitted from object code,
> but not stop giving link errors for drivers that actually require
> DMA.

This can be done for drivers/usb/core/hcd.c.

But I'm a bit puzzled by drivers/usb/core/buffer.c. E.g.

void *hcd_buffer_alloc(...)
{
        ....
        /* some USB hosts just use PIO */
        if (!bus->controller->dma_mask &&
            !(hcd->driver->flags & HCD_LOCAL_MEM)) {
                *dma = ~(dma_addr_t) 0;
                return kmalloc(size, mem_flags);
        }

        for (i = 0; i < HCD_BUFFER_POOLS; i++) {
                if (size <= pool_max[i])
                        return dma_pool_alloc(hcd->pool[i], mem_flags, dma);
        }
        return dma_alloc_coherent(hcd->self.controller, size, dma, mem_flags);
}

which is called from usb_hcd_map_urb_for_dma():

                if (hcd->self.uses_dma) {
                        ....
                } else if (hcd->driver->flags & HCD_LOCAL_MEM) {
                        ret = hcd_alloc_coherent(
                                        urb->dev->bus, mem_flags,
                                        &urb->setup_dma,
                                        (void **)&urb->setup_packet,
                                        sizeof(struct usb_ctrlrequest),
                                        DMA_TO_DEVICE);
                        ...
                }

So if DMA is not used (!hcd->self.uses_dma, i.e. bus->controller->dma_mask
is zero), and HCD_LOCAL_MEM is set, we still end up calling dma_pool_alloc()?

(Naively, I'm not so familiar with the USB code) I'd expect it to use
kmalloc() instead?

So I would change it to

        if (!IS_ENABLED(CONFIG_HAS_DMA) ||
            (!bus->controller->dma_mask &&
             !(hcd->driver->flags & HCD_LOCAL_MEM))) {
                *dma = ~(dma_addr_t) 0;
                return kmalloc(size, mem_flags);
        }

Thanks for your clarification!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] usb: USB host support should depend on HAS_DMA
  2013-08-18 21:12     ` Geert Uytterhoeven
@ 2013-08-19 15:09       ` Alan Stern
  0 siblings, 0 replies; 8+ messages in thread
From: Alan Stern @ 2013-08-19 15:09 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-input,
	Linux Media Mailing List, USB list, linux-kernel

On Sun, 18 Aug 2013, Geert Uytterhoeven wrote:

> On Thu, Jul 11, 2013 at 1:12 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday 10 July 2013, Alan Stern wrote:
> >> This isn't right.  There are USB host controllers that use PIO, not
> >> DMA.  The HAS_DMA dependency should go with the controller driver, not
> >> the USB core.
> >>
> >> On the other hand, the USB core does call various routines like
> >> dma_unmap_single.  It ought to be possible to compile these calls even
> >> when DMA isn't enabled.  That is, they should be defined as do-nothing
> >> stubs.
> >
> > The asm-generic/dma-mapping-broken.h file intentionally causes link
> > errors, but that could be changed.
> >
> > The better approach in my mind would be to replace code like
> >
> >
> >         if (hcd->self.uses_dma)
> >
> > with
> >
> >         if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
> >
> > which will reliably cause that reference to be omitted from object code,
> > but not stop giving link errors for drivers that actually require
> > DMA.
> 
> This can be done for drivers/usb/core/hcd.c.
> 
> But I'm a bit puzzled by drivers/usb/core/buffer.c. E.g.
> 
> void *hcd_buffer_alloc(...)
> {
>         ....
>         /* some USB hosts just use PIO */
>         if (!bus->controller->dma_mask &&
>             !(hcd->driver->flags & HCD_LOCAL_MEM)) {

I don't remember the full story.  You should check with the person who
added the HCD_LOCAL_MEM flag originally.

> So if DMA is not used (!hcd->self.uses_dma, i.e. bus->controller->dma_mask
> is zero), and HCD_LOCAL_MEM is set, we still end up calling dma_pool_alloc()?
> 
> (Naively, I'm not so familiar with the USB code) I'd expect it to use
> kmalloc() instead?

Maybe what happens in this case is that some sort of hook causes the 
allocation to be made from a special memory-mapped region on board the 
controller.

Alan Stern


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-08-19 15:09 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-10 21:18 [PATCH] usb: USB host support should depend on HAS_DMA Geert Uytterhoeven
2013-07-10 21:31 ` Alan Stern
2013-07-10 23:12   ` Arnd Bergmann
2013-07-11  1:01     ` Alan Stern
2013-07-11  7:49       ` Geert Uytterhoeven
2013-07-11 15:03         ` Alan Stern
2013-08-18 21:12     ` Geert Uytterhoeven
2013-08-19 15:09       ` Alan Stern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).