Re: [PATCH] usb: USB host support should depend on HAS_DMA

[Geert Uytterhoeven]

On Thu, Jul 11, 2013 at 1:12 AM, Arnd Bergmann <arnd@xxxxxxxx> 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@xxxxxxxxxxxxxx

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/