All of lore.kernel.org
 help / color / mirror / Atom feed
From: Samuel Dionne-Riel <samuel@dionne-riel.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] usb: Make USB_MUSB_PIO_ONLY conditional on USB_MUSB_{HOST, GADGET}
Date: Fri, 22 Nov 2019 16:14:59 -0500	[thread overview]
Message-ID: <20191122211458.15828-1-samuel@dionne-riel.com> (raw)

This ensures the USB_MUSB_PIO_ONLY config is set to an apppropriate
default values from the changes enabling USB_MUSB_GADGET does.

Namely, USB_MUSB_PIO_ONLY default to =y on USB_MUSB_SUNXI being y.
Since USB_MUSB_SUNXI is behind the conditional, the option does not
exist when using make ..._defconfig, thus the default of
USB_MUSB_PIO_ONLY is kept "# ... is not set". This, _is_
counter-intuitively a set value, meaning that the default will not be
applied once USB_MUSB_SUNXI is set to y by toggling USB_MUSB_GADGET
via menuconfig.

Signed-off-by: Samuel Dionne-Riel <samuel@dionne-riel.com>

---
Some additional context about the bug.

Using `make ..._defconfig` for a sunxi board, then using `make
menuconfig` to change the option `CONFIG_USB_MUSB_GADGET=y` causes
builds to fail. When the same option is set to `=y` via the defconfig
file, the option work. Follows the failure:

```
  CC      drivers/usb/musb-new/musb_gadget.o
drivers/usb/musb-new/musb_gadget.c: In function 'map_dma_buffer':
drivers/usb/musb-new/musb_gadget.c:103:26: warning: implicit declaration of function 'dma_map_single'; did you mean 'malloc_simple'? [-Wimplicit-function-declaration]
   request->request.dma = dma_map_single(
                          ^~~~~~~~~~~~~~
                          malloc_simple
drivers/usb/musb-new/musb_gadget.c:108:8: error: 'DMA_TO_DEVICE' undeclared (first use in this function); did you mean 'USB_DT_DEVICE'?
      ? DMA_TO_DEVICE
        ^~~~~~~~~~~~~
        USB_DT_DEVICE
drivers/usb/musb-new/musb_gadget.c:108:8: note: each undeclared identifier is reported only once for each function it appears in
drivers/usb/musb-new/musb_gadget.c:109:8: error: 'DMA_FROM_DEVICE' undeclared (first use in this function); did you mean 'U_BOOT_DEVICE'?
      : DMA_FROM_DEVICE);
        ^~~~~~~~~~~~~~~
        U_BOOT_DEVICE
drivers/usb/musb-new/musb_gadget.c:112:3: warning: implicit declaration of function 'dma_sync_single_for_device' [-Wimplicit-function-declaration]
   dma_sync_single_for_device(musb->controller,
   ^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/usb/musb-new/musb_gadget.c: In function 'unmap_dma_buffer':
drivers/usb/musb-new/musb_gadget.c:135:3: warning: implicit declaration of function 'dma_unmap_single' [-Wimplicit-function-declaration]
   dma_unmap_single(musb->controller,
   ^~~~~~~~~~~~~~~~
drivers/usb/musb-new/musb_gadget.c:139:7: error: 'DMA_TO_DEVICE' undeclared (first use in this function); did you mean 'USB_DT_DEVICE'?
     ? DMA_TO_DEVICE
       ^~~~~~~~~~~~~
       USB_DT_DEVICE
drivers/usb/musb-new/musb_gadget.c:140:7: error: 'DMA_FROM_DEVICE' undeclared (first use in this function); did you mean 'U_BOOT_DEVICE'?
     : DMA_FROM_DEVICE);
       ^~~~~~~~~~~~~~~
       U_BOOT_DEVICE
drivers/usb/musb-new/musb_gadget.c:143:3: warning: implicit declaration of function 'dma_sync_single_for_cpu' [-Wimplicit-function-declaration]
   dma_sync_single_for_cpu(musb->controller,
   ^~~~~~~~~~~~~~~~~~~~~~~
make[1]: *** [scripts/Makefile.build:279: drivers/usb/musb-new/musb_gadget.o] Error 1
```

By diffing the resulting configuration, Othe main difference found is
`CONFIG_USB_MUSB_PIO_ONLY` is not set when failing, and set to `=y`
when working. This was verified to be the missing option.

As far as I understand it in Kconfig, "is not set" basically marks the
option as using a negative "unset" value, and does not mean that it
will use the default value. This difference in meaning makes it so
that when toggling the option with `make menuconfig`, the option has
already been set to "is not set", and it does not change to its
default value.

The actual bug may be that there is a regression and the code should
work when this value is unset, in this case I do not know how to
provide the proper fix.

This fix corrects the issue by gating the option behind
`if USB_MUSB_HOST || USB_MUSB_GADGET`. To me, it looks like this was
the intention, in the initial patch, but might have been overlooked.
I assume so since the option was defined after the `if`, rather than
before, like the other options not gated behind the option.


 drivers/usb/musb-new/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb-new/Kconfig b/drivers/usb/musb-new/Kconfig
index 79ad14ef66..95499f1b17 100644
--- a/drivers/usb/musb-new/Kconfig
+++ b/drivers/usb/musb-new/Kconfig
@@ -72,11 +72,11 @@ config USB_MUSB_DISABLE_BULK_COMBINE_SPLIT
 	  support it yet. Select this option to disable the feature until the
 	  driver adds the support.
 
-endif
-
 config USB_MUSB_PIO_ONLY
 	bool "Disable DMA (always use PIO)"
 	default y if USB_MUSB_AM35X || USB_MUSB_PIC32 || USB_MUSB_OMAP2PLUS || USB_MUSB_DSPS || USB_MUSB_SUNXI
 	help
 	  All data is copied between memory and FIFO by the CPU.
 	  DMA controllers are ignored.
+
+endif
-- 
2.23.0

             reply	other threads:[~2019-11-22 21:14 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-22 21:14 Samuel Dionne-Riel [this message]
2019-11-27 19:56 ` [U-Boot] [PATCH v2] usb: Make USB_MUSB_PIO_ONLY selected by USB_MUSB_SUNXI Samuel Dionne-Riel
2021-11-15 16:41   ` Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191122211458.15828-1-samuel@dionne-riel.com \
    --to=samuel@dionne-riel.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.