* [U-Boot] [PATCH] usb: Make USB_MUSB_PIO_ONLY conditional on USB_MUSB_{HOST, GADGET}
@ 2019-11-22 21:14 Samuel Dionne-Riel
2019-11-27 19:56 ` [U-Boot] [PATCH v2] usb: Make USB_MUSB_PIO_ONLY selected by USB_MUSB_SUNXI Samuel Dionne-Riel
0 siblings, 1 reply; 3+ messages in thread
From: Samuel Dionne-Riel @ 2019-11-22 21:14 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [U-Boot] [PATCH v2] usb: Make USB_MUSB_PIO_ONLY selected by USB_MUSB_SUNXI
2019-11-22 21:14 [U-Boot] [PATCH] usb: Make USB_MUSB_PIO_ONLY conditional on USB_MUSB_{HOST, GADGET} Samuel Dionne-Riel
@ 2019-11-27 19:56 ` Samuel Dionne-Riel
2021-11-15 16:41 ` Tom Rini
0 siblings, 1 reply; 3+ messages in thread
From: Samuel Dionne-Riel @ 2019-11-27 19:56 UTC (permalink / raw)
To: u-boot
This ensures the USB_MUSB_PIO_ONLY config is set to an apppropriate
value from the changes enabling USB_MUSB_GADGET does.
Namely, USB_MUSB_PIO_ONLY default to =y on USB_MUSB_SUNXI being y.
Signed-off-by: Samuel Dionne-Riel <samuel@dionne-riel.com>
---
Changes in v2:
- Use select as a reverse-dependency
As discussed on IRC with Marex,
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 | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/musb-new/Kconfig b/drivers/usb/musb-new/Kconfig
index 79ad14ef66..d7754014e3 100644
--- a/drivers/usb/musb-new/Kconfig
+++ b/drivers/usb/musb-new/Kconfig
@@ -58,6 +58,7 @@ config USB_MUSB_PIC32
config USB_MUSB_SUNXI
bool "Enable sunxi OTG / DRC USB controller"
depends on ARCH_SUNXI
+ select USB_MUSB_PIO_ONLY
default y
---help---
Say y here to enable support for the sunxi OTG / DRC USB controller
--
2.23.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [U-Boot] [PATCH v2] usb: Make USB_MUSB_PIO_ONLY selected by USB_MUSB_SUNXI
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
0 siblings, 0 replies; 3+ messages in thread
From: Tom Rini @ 2021-11-15 16:41 UTC (permalink / raw)
To: Samuel Dionne-Riel; +Cc: u-boot, Marek Vasut, Hannes Schmelzer
[-- Attachment #1: Type: text/plain, Size: 388 bytes --]
On Wed, Nov 27, 2019 at 02:56:02PM -0500, Samuel Dionne-Riel wrote:
> This ensures the USB_MUSB_PIO_ONLY config is set to an apppropriate
> value from the changes enabling USB_MUSB_GADGET does.
>
> Namely, USB_MUSB_PIO_ONLY default to =y on USB_MUSB_SUNXI being y.
>
> Signed-off-by: Samuel Dionne-Riel <samuel@dionne-riel.com>
Applied to u-boot/master, thanks!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-15 16:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-22 21:14 [U-Boot] [PATCH] usb: Make USB_MUSB_PIO_ONLY conditional on USB_MUSB_{HOST, GADGET} Samuel Dionne-Riel
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
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.