[PATCH] Revert "usb: gadget: allow to enable legacy drivers without USB_ETH"
Romain Izard reported the following about commit 7a9618a22aad: As it reached Linus' tree with v4.15-rc3, I recently noticed the following commit that triggered a Kconfig request. I believe that this change does not make sense. 7a9618a22aa usb: gadget: allow to enable legacy drivers without USB_ETH USB_ETH was not a dependency, but a default value for the choice. As the choice was marked as "optional", it was possible to remove this value when building. After this modification, the Kconfig choice option does not contain anything anymore, so it is useless. It is also possible to select multiple built-in legacy drivers. This builds, but will not work as expected as only one legacy driver can be bound to an USB device controller at a time. Hence revert commit 7a9618a22aad. Signed-off-by: Bart Van Assche <bart.vanass...@wdc.com> Cc: Romain Izard <romain.izard@gmail.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Stephen Rothwell <s...@canb.auug.org.au> Cc: Masahiro Yamada <yamada.masah...@socionext.com> Cc: Hannes Reinecke <h...@suse.com> Cc: Nicholas Bellinger <n...@linux-iscsi.org> Cc: Andrzej Pietrasiewicz <andrze...@samsung.com> Cc: linux-usb@vger.kernel.org Cc: Felipe Balbi <felipe.ba...@linux.intel.com> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 31cce7805eb2..0a19a76645ad 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -508,8 +508,8 @@ choice controller, and the relevant drivers for each function declared by the device. -source "drivers/usb/gadget/legacy/Kconfig" - endchoice +source "drivers/usb/gadget/legacy/Kconfig" + endif # USB_GADGET diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig index a12fb459dbd9..9570bbeced4f 100644 --- a/drivers/usb/gadget/legacy/Kconfig +++ b/drivers/usb/gadget/legacy/Kconfig @@ -13,6 +13,14 @@ # both kinds of controller can also support "USB On-the-Go" (CONFIG_USB_OTG). # +menuconfig USB_GADGET_LEGACY + bool "Legacy USB Gadget Support" + help + Legacy USB gadgets are USB gadgets that do not use the USB gadget + configfs interface. + +if USB_GADGET_LEGACY + config USB_ZERO tristate "Gadget Zero (DEVELOPMENT)" select USB_LIBCOMPOSITE @@ -490,3 +498,5 @@ config USB_G_WEBCAM Say "y" to link the driver statically, or "m" to build a dynamically linked module called "g_webcam". + +endif : --- drivers/usb/gadget/Kconfig| 4 ++-- drivers/usb/gadget/legacy/Kconfig | 10 -- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 0a19a76645ad..31cce7805eb2 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -508,8 +508,8 @@ choice controller, and the relevant drivers for each function declared by the device. -endchoice - source "drivers/usb/gadget/legacy/Kconfig" +endchoice + endif # USB_GADGET diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig index 9570bbeced4f..a12fb459dbd9 100644 --- a/drivers/usb/gadget/legacy/Kconfig +++ b/drivers/usb/gadget/legacy/Kconfig @@ -13,14 +13,6 @@ # both kinds of controller can also support "USB On-the-Go" (CONFIG_USB_OTG). # -menuconfig USB_GADGET_LEGACY - bool "Legacy USB Gadget Support" - help - Legacy USB gadgets are USB gadgets that do not use the USB gadget - configfs interface. - -if USB_GADGET_LEGACY - config USB_ZERO tristate "Gadget Zero (DEVELOPMENT)" select USB_LIBCOMPOSITE @@ -498,5 +490,3 @@ config USB_G_WEBCAM Say "y" to link the driver statically, or "m" to build a dynamically linked module called "g_webcam". - -endif -- 2.15.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 1/2] usb: gadget: restore tristate-choice for legacy gadgets
On Mon, 2017-12-11 at 12:30 +0100, Arnd Bergmann wrote:
> One patch that was meant as a cleanup apparently did more than it intended,
> allowing all combinations of legacy gadget drivers to be built into the
> kernel, and leaving an empty 'choice' statement behind:
>
> drivers/usb/gadget/Kconfig:487:warning: choice default symbol 'USB_ETH' is
> not contained in the choice
>
> The description of commit 7a9618a22aad ("usb: gadget: allow to enable legacy
> drivers without USB_ETH") was a bit cryptic, as it did not change the
> behavior of USB_ETH other than allowing it to be built into the kernel
> alongside other legacy gadgets, which is not a valid configuration.
>
> As Felipe explained in the description for commit bc49d1d17dcf ("usb:
> gadget: don't couple configfs to legacy gadgets"), the configfs based
> gadgets can be freely configured as loadable modules or built-in
> drivers, but the legacy gadgets can only be modules if there is more
> than one of them, so we require the 'choice' statement here.
>
> This leaves the added USB_GADGET_LEGACY menuconfig symbol in place,
> but then restores the 'choice' below it, so we can enforce the
> single-legacy-gadget rule as before.
Hello Arnd,
A discussion is ongoing about whether or not commit 7a9618a22aad should be
reverted.
Please drop this patch until a conclusion has been reached.
Thanks,
Bart.
Re: [PATCH 11/11] usb/gadget: Make it again possible to enable the legacy drivers without enabling USB_ETH
On Tue, 2017-10-31 at 11:03 -0700, Bart Van Assche wrote:
> Considerable time ago the legacy gadget menu was added inside the
> USB_ETH choice. I think this was a mistake and that the legacy
> gadget menu should have been added after "endchoice" instead of
> before. Hence this patch.
>
> Fixes: commit 8443f2d2b778 ("usb: gadget: Gadget directory cleanup - group
> legacy gadgets")
> Signed-off-by: Bart Van Assche <bart.vanass...@wdc.com>
> Cc: Nicholas Bellinger <n...@linux-iscsi.org>
> Cc: Andrzej Pietrasiewicz <andrze...@samsung.com>
> Cc: Felipe Balbi <ba...@ti.com>
> Cc: linux-usb@vger.kernel.org
Hello Andrzej and Felipe,
Can one or both of you have a look at this patch?
Thanks,
Bart.N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&�� �G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
Re: Seagate External SMR drive USB resets... why? / USB storage debugging
On Wed, 2017-11-15 at 18:27 -0500, Jérôme Carretero wrote: > OK but I find that a "reset" message without any reason is not > as helpful as it could have been. At the minimum I'll try to scratch my > own itch and see if I can go at the bottom of my issue. If you want more information about SCSI error handler decisions, observing the output sent by the following command to the system log will probably help: echo 63 > /sys/module/scsi_mod/parameters/scsi_logging_level See also https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/scsi/scsi_logging.h Bart.
[PATCH 11/11] usb/gadget: Make it again possible to enable the legacy drivers without enabling USB_ETH
Considerable time ago the legacy gadget menu was added inside the
USB_ETH choice. I think this was a mistake and that the legacy
gadget menu should have been added after "endchoice" instead of
before. Hence this patch.
Fixes: commit 8443f2d2b778 ("usb: gadget: Gadget directory cleanup - group
legacy gadgets")
Signed-off-by: Bart Van Assche <bart.vanass...@wdc.com>
Cc: Nicholas Bellinger <n...@linux-iscsi.org>
Cc: Andrzej Pietrasiewicz <andrze...@samsung.com>
Cc: Felipe Balbi <ba...@ti.com>
Cc: linux-usb@vger.kernel.org
---
drivers/usb/gadget/Kconfig| 4 ++--
drivers/usb/gadget/legacy/Kconfig | 10 ++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 31cce7805eb2..0a19a76645ad 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -508,8 +508,8 @@ choice
controller, and the relevant drivers for each function declared
by the device.
-source "drivers/usb/gadget/legacy/Kconfig"
-
endchoice
+source "drivers/usb/gadget/legacy/Kconfig"
+
endif # USB_GADGET
diff --git a/drivers/usb/gadget/legacy/Kconfig
b/drivers/usb/gadget/legacy/Kconfig
index a12fb459dbd9..9570bbeced4f 100644
--- a/drivers/usb/gadget/legacy/Kconfig
+++ b/drivers/usb/gadget/legacy/Kconfig
@@ -13,6 +13,14 @@
# both kinds of controller can also support "USB On-the-Go" (CONFIG_USB_OTG).
#
+menuconfig USB_GADGET_LEGACY
+ bool "Legacy USB Gadget Support"
+ help
+ Legacy USB gadgets are USB gadgets that do not use the USB gadget
+ configfs interface.
+
+if USB_GADGET_LEGACY
+
config USB_ZERO
tristate "Gadget Zero (DEVELOPMENT)"
select USB_LIBCOMPOSITE
@@ -490,3 +498,5 @@ config USB_G_WEBCAM
Say "y" to link the driver statically, or "m" to build a
dynamically linked module called "g_webcam".
+
+endif
--
2.14.2
--
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: USB-related lock inversion complaint
On Mon, 2017-05-01 at 13:39 -0400, Alan Stern wrote: > On Fri, 28 Apr 2017, Alan Stern wrote: > > On Fri, 28 Apr 2017, Bart Van Assche wrote: > He wrote: > > > There, that's where we hold the lock with interrupts enabled. > > > > So someone forced interrupt threading and the code assumes the interrupt > > code has IRQs disabled or something along those lines. > > So we should be able to fix the problem by changing spin_lock/unlock in > xhci_irq() to spin_lock_irqsave/restore. Bart, please try out the > patch below. Hello Alan, Thanks for getting back to me and sorry that I forgot to mention that I had enabled interrupt threading. Is the patch you posted against a USB tree that is not yet upstream? It did not apply cleanly against kernel v4.11. But a backport of that patch to kernel v4.11 made the lock inversion complaints disappear so I think you can add: Tested-by: Bart Van Assche <bart.vanass...@sandisk.com>-- 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
USB-related lock inversion complaint
Hello,
Every time I boot a particular development server the complaint shown
below appears in the system log. I don't know when this behavior has
been introduced. But I noticed that I can reproduce this with older
kernel versions, e.g. 4.4.63. Does anyone know what is going on or how
to fix this?
Thanks,
Bart.
usb 1-8: new low-speed USB device number 2 using xhci_hcd
usb 3-1: new high-speed USB device number 2 using ehci-pci
=
[ INFO: possible irq lock inversion dependency detected ]
4.11.0-rc8-dbg+ #1 Not tainted
-
swapper/7/0 just changed the state of lock:
(&(>lock)->rlock){-.-...}, at: []
ehci_hrtimer_func+0x29/0xc0 [ehci_hcd]
but this lock took another, HARDIRQ-unsafe lock in the past:
(hcd_urb_list_lock){+.}
and interrupts could create inverse lock ordering between them.
other info that might help us debug this:
Possible interrupt unsafe locking scenario:
CPU0CPU1
lock(hcd_urb_list_lock);
local_irq_disable();
lock(&(>lock)->rlock);
lock(hcd_urb_list_lock);
lock(&(>lock)->rlock);
*** DEADLOCK ***
no locks held by swapper/7/0.
the shortest dependencies between 2nd lock and 1st lock:
-> (hcd_urb_list_lock){+.} ops: 252 {
HARDIRQ-ON-W at:
__lock_acquire+0x602/0x1280
lock_acquire+0xd5/0x1c0
_raw_spin_lock+0x2f/0x40
usb_hcd_unlink_urb_from_ep+0x1b/0x60 [usbcore]
xhci_giveback_urb_in_irq.isra.45+0x70/0x1b0 [xhci_hcd]
finish_td.constprop.60+0x1d8/0x2e0 [xhci_hcd]
xhci_irq+0xdd6/0x1fa0 [xhci_hcd]
usb_hcd_irq+0x26/0x40 [usbcore]
irq_forced_thread_fn+0x2f/0x70
irq_thread+0x149/0x1d0
kthread+0x113/0x150
ret_from_fork+0x2e/0x40
INITIAL USE at:
__lock_acquire+0x343/0x1280
lock_acquire+0xd5/0x1c0
_raw_spin_lock+0x2f/0x40
usb_hcd_link_urb_to_ep+0x2e/0xd0 [usbcore]
usb_hcd_submit_urb+0x117/0xb40 [usbcore]
usb_submit_urb+0x2f4/0x560 [usbcore]
usb_start_wait_urb+0x5f/0x150 [usbcore]
usb_control_msg+0xc1/0xf0 [usbcore]
usb_get_descriptor+0x79/0xc0 [usbcore]
usb_get_device_descriptor+0x57/0x100 [usbcore]
usb_add_hcd+0x329/0x8a0 [usbcore]
usb_hcd_pci_probe+0x1af/0x450 [usbcore]
xhci_pci_probe+0x1c/0x180 [xhci_pci]
local_pci_probe+0x24/0x60
pci_device_probe+0xd8/0x130
driver_probe_device+0x26a/0x410
__driver_attach+0xe3/0xf0
bus_for_each_dev+0x62/0xa0
driver_attach+0x1e/0x20
bus_add_driver+0x173/0x270
driver_register+0x60/0xe0
__pci_register_driver+0x5d/0x60
0xa0147031
do_one_initcall+0x43/0x170
do_init_module+0x5f/0x1fa
load_module+0x24ab/0x2c00
SYSC_finit_module+0xbc/0xf0
SyS_finit_module+0xe/0x10
do_syscall_64+0x5c/0x140
return_from_SYSCALL_64+0x0/0x7a
}
... key at: []
hcd_urb_list_lock+0x18/0x7100 [usbcore]
... acquired at:
lock_acquire+0xd5/0x1c0
_raw_spin_lock+0x2f/0x40
usb_hcd_link_urb_to_ep+0x2e/0xd0 [usbcore]
ehci_urb_enqueue+0xab/0x107f [ehci_hcd]
usb_hcd_submit_urb+0x8f/0xb40 [usbcore]
usb_submit_urb+0x2f4/0x560 [usbcore]
usb_start_wait_urb+0x5f/0x150 [usbcore]
usb_control_msg+0xc1/0xf0 [usbcore]
hub_port_init+0x305/0xb90 [usbcore]
hub_event+0x690/0x1250 [usbcore]
process_one_work+0x20b/0x6a0
worker_thread+0x4e/0x4a0
kthread+0x113/0x150
ret_from_fork+0x2e/0x40
-> (&(>lock)->rlock){-.-...} ops: 38 {
IN-HARDIRQ-W at:
__lock_acquire+0x67d/0x1280
lock_acquire+0xd5/0x1c0
_raw_spin_lock_irqsave+0x3a/0x50
ehci_hrtimer_func+0x29/0xc0 [ehci_hcd]
__hrtimer_run_queues+0xde/0x4c0
hrtimer_interrupt+0xaa/0x200
local_apic_timer_interrupt+0x38/0x60
smp_apic_timer_interrupt+0x38/0x50
apic_timer_interrupt+0x90/0xa0
cpuidle_enter_state+0x135/0x380
cpuidle_enter+0x17/0x20
call_cpuidle+0x23/0x40
do_idle+0xe8/0x1c0
Re: [PATCH 4.10-rc3 09/13] iscsi: fix build errors when linux/phy*.h is removed from net/dsa.h
On Tue, 2017-01-31 at 19:19 +, Russell King wrote: > drivers/target/iscsi/iscsi_target_login.c:1135:7: error: implicit declaration > of function 'try_module_get' [-Werror=implicit-function-declaration] > > Add linux/module.h to iscsi_target_login.c. > > Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk> > --- > drivers/target/iscsi/iscsi_target_login.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/target/iscsi/iscsi_target_login.c > b/drivers/target/iscsi/iscsi_target_login.c > index 450f51deb2a2..eab274d17b5c 100644 > --- a/drivers/target/iscsi/iscsi_target_login.c > +++ b/drivers/target/iscsi/iscsi_target_login.c > @@ -17,6 +17,7 @@ > > **/ > > #include > +#include > #include > #include > #include Reviewed-by: Bart Van Assche <bart.vanass...@sandisk.com>-- 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 07/21] usb/gadget: Remove set-but-not-used variables
Avoid that building with W=1 triggers compiler warnings about
set-but-not-used variables.
Signed-off-by: Bart Van Assche <bart.vanass...@sandisk.com>
Acked-by: Felipe Balbi <ba...@ti.com>
Reviewed-by: Andy Grover <agro...@redhat.com>
Cc: Christoph Hellwig <h...@lst.de>
Cc: Hannes Reinecke <h...@suse.de>
---
drivers/usb/gadget/legacy/tcm_usb_gadget.c | 13 -
1 file changed, 13 deletions(-)
diff --git a/drivers/usb/gadget/legacy/tcm_usb_gadget.c
b/drivers/usb/gadget/legacy/tcm_usb_gadget.c
index 33833fe..ddef0c5 100644
--- a/drivers/usb/gadget/legacy/tcm_usb_gadget.c
+++ b/drivers/usb/gadget/legacy/tcm_usb_gadget.c
@@ -71,18 +71,9 @@ static void bot_enqueue_sense_code(struct f_uas *fu, struct
usbg_cmd *cmd)
{
struct bulk_cs_wrap *csw = >bot_status.csw;
int ret;
- u8 *sense;
unsigned int csw_stat;
csw_stat = cmd->csw_code;
-
- /*
-* We can't send SENSE as a response. So we take ASC & ASCQ from our
-* sense buffer and queue it and hope the host sends a REQUEST_SENSE
-* command where it learns why we failed.
-*/
- sense = cmd->sense_iu.sense;
-
csw->Tag = cmd->bot_tag;
csw->Status = csw_stat;
fu->bot_status.req->context = cmd;
@@ -1078,7 +1069,6 @@ static int usbg_submit_command(struct f_uas *fu,
struct command_iu *cmd_iu = cmdbuf;
struct usbg_cmd *cmd;
struct usbg_tpg *tpg;
- struct se_cmd *se_cmd;
struct tcm_usbg_nexus *tv_nexus;
u32 cmd_len;
int ret;
@@ -1142,7 +1132,6 @@ static int usbg_submit_command(struct f_uas *fu,
break;
}
- se_cmd = >se_cmd;
cmd->unpacked_lun = scsilun_to_int(_iu->lun);
INIT_WORK(>work, usbg_cmd_work);
@@ -1195,7 +1184,6 @@ static int bot_submit_command(struct f_uas *fu,
struct bulk_cb_wrap *cbw = cmdbuf;
struct usbg_cmd *cmd;
struct usbg_tpg *tpg;
- struct se_cmd *se_cmd;
struct tcm_usbg_nexus *tv_nexus;
u32 cmd_len;
int ret;
@@ -1236,7 +1224,6 @@ static int bot_submit_command(struct f_uas *fu,
}
cmd->prio_attr = TCM_SIMPLE_TAG;
- se_cmd = >se_cmd;
cmd->unpacked_lun = cbw->Lun;
cmd->is_read = cbw->Flags & US_BULK_FLAG_IN ? 1 : 0;
cmd->data_len = le32_to_cpu(cbw->DataTransferLength);
--
2.1.4
--
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: Kernel crash on usb-storage disconnect, multiqueue in use
On 02/11/15 22:25, Bruno Prémont wrote: Since 3.19-rcX kernel crashes when I disconnect an external HDD drive (not sure since which kernel revision tough early 3.19-rcs possibly are not affected). It looks like this crash is related to the fact that I enable multiqueue (CONFIG_SCSI_MQ_DEFAULT=y). Can you repeat your test with kernel 3.19, which includes the pull request https://lkml.org/lkml/2015/2/3/606 ? Bart. -- 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
