Hi, On Fri, Jun 19, 2015 at 01:59:40PM +0000, David Fisher wrote: > Hi all, > > I'm running 4.1-rc4 (Linaro 5.15) and trying to create a mass storage > gadget via configfs, with default 1 LUN (lun.0). As the configfs > progresses, I see "Number of LUNs=8". And when plugged in, the USB > class specific getMaxLun request returns 7. In some Windows drivers > (but not some others ?!), this MAX_LUN is taken into account and > Windows tries to load class drivers for the additional 7 duff LUNs - > resulting in "Device driver software was not correctly installed". > > With debug added to f_mass_storage.c, I get the following : > > [ 469.402527] Number of LUNs=8 (fsg_common_set_nluns), with stack : > [ 469.402611] CPU: 2 PID: 1590 Comm: mkdir Not tainted > 4.1.0-1-linaro-vexpress64 #15 > [ 469.402680] Hardware name: FVP Base (DT) > [ 469.402730] Call trace: > [ 469.402808] [<ffffffc000089a58>] dump_backtrace+0x0/0x164 > [ 469.402897] [<ffffffc000089bd8>] show_stack+0x1c/0x28 > [ 469.402975] [<ffffffc00081a908>] dump_stack+0x84/0xc8 > [ 469.403061] [<ffffffc000686938>] fsg_common_set_nluns+0x74/0xa4 > [ 469.403152] [<ffffffc00068853c>] fsg_alloc_inst+0xe4/0x218 > [ 469.403245] [<ffffffc00067f4a4>] try_get_usb_function_instance+0xb4/0xf0 > [ 469.403342] [<ffffffc00067f634>] usb_get_function_instance+0x1c/0x68 > [ 469.403425] [<ffffffc000681f2c>] function_make+0x78/0x138 > [ 469.403510] [<ffffffc000245524>] configfs_mkdir+0x110/0x38c > [ 469.403591] [<ffffffc0001d72b8>] vfs_mkdir+0xf4/0x190 > [ 469.403673] [<ffffffc0001dcc5c>] SyS_mkdirat+0xe4/0xf0 > [ 469.403755] Mass Storage Function, version: 2009/09/11 > [ 469.403823] LUN: removable file: (no medium) > [ 469.448719] Number of LUNs=8 (fsg_bind), with stack : > [ 469.448799] CPU: 1 PID: 1576 Comm: start_dwusb_dev Not tainted > 4.1.0-1-linaro-vexpress64 #15 > [ 469.448872] Hardware name: FVP Base (DT) > [ 469.448921] Call trace: > [ 469.448999] [<ffffffc000089a58>] dump_backtrace+0x0/0x164 > [ 469.449088] [<ffffffc000089bd8>] show_stack+0x1c/0x28 > [ 469.449167] [<ffffffc00081a908>] dump_stack+0x84/0xc8 > [ 469.449253] [<ffffffc000687e90>] fsg_bind+0x1a4/0x1b0 > [ 469.449340] [<ffffffc00067c4e8>] usb_add_function+0x74/0x188 > [ 469.449437] [<ffffffc0006819a4>] configfs_composite_bind+0x22c/0x33c > [ 469.449524] [<ffffffc0006835f8>] udc_bind_to_driver+0x54/0x11c > [ 469.449609] [<ffffffc00068374c>] usb_udc_attach_driver+0x8c/0xc8 > [ 469.449693] [<ffffffc00068212c>] gadget_dev_desc_UDC_store+0xc0/0x104 > [ 469.449789] [<ffffffc00067f714>] gadget_info_attr_store+0x40/0x5c > [ 469.449875] [<ffffffc0002436d8>] configfs_write_file+0xc4/0x128 > [ 469.449958] [<ffffffc0001cbbd0>] __vfs_write+0x44/0x130 > [ 469.450039] [<ffffffc0001cc44c>] vfs_write+0x98/0x178 > [ 469.450119] [<ffffffc0001ccce8>] SyS_write+0x50/0xb0 > > Configfs tree is : > > drwxr-xr-x 3 root root 0 Jun 19 11:40 > /sys/kernel/config/usb_gadget/ > drwxr-xr-x 6 root root 0 Jun 19 11:41 > /sys/kernel/config/usb_gadget/g1 > drwxr-xr-x 2 root root 0 Jun 19 11:41 > /sys/kernel/config/usb_gadget/g1/os_desc > -rw-r--r-- 1 root root 4096 Jun 19 11:42 > /sys/kernel/config/usb_gadget/g1/os_desc/qw_sign > -rw-r--r-- 1 root root 4096 Jun 19 11:42 > /sys/kernel/config/usb_gadget/g1/os_desc/b_vendor_code > -rw-r--r-- 1 root root 4096 Jun 19 11:42 > /sys/kernel/config/usb_gadget/g1/os_desc/use > drwxr-xr-x 3 root root 0 Jun 19 11:40 > /sys/kernel/config/usb_gadget/g1/strings > drwxr-xr-x 2 root root 0 Jun 19 11:41 > /sys/kernel/config/usb_gadget/g1/strings/0x409 > -rw-r--r-- 1 root root 4096 Jun 19 11:40 > /sys/kernel/config/usb_gadget/g1/strings/0x409/serialnumber > -rw-r--r-- 1 root root 4096 Jun 19 11:40 > /sys/kernel/config/usb_gadget/g1/strings/0x409/product > -rw-r--r-- 1 root root 4096 Jun 19 11:40 > /sys/kernel/config/usb_gadget/g1/strings/0x409/manufacturer > drwxr-xr-x 3 root root 0 Jun 19 11:40 > /sys/kernel/config/usb_gadget/g1/configs > drwxr-xr-x 3 root root 0 Jun 19 11:41 > /sys/kernel/config/usb_gadget/g1/configs/c.1 > lrwxrwxrwx 1 root root 0 Jun 19 11:40 > /sys/kernel/config/usb_gadget/g1/configs/c.1/mass_storage.0 -> > ../../../../usb_gadget/g1/functions/mass_storage.0 > drwxr-xr-x 3 root root 0 Jun 19 11:40 > /sys/kernel/config/usb_gadget/g1/configs/c.1/strings > drwxr-xr-x 2 root root 0 Jun 19 11:41 > /sys/kernel/config/usb_gadget/g1/configs/c.1/strings/0x409 > -rw-r--r-- 1 root root 4096 Jun 19 11:40 > /sys/kernel/config/usb_gadget/g1/configs/c.1/strings/0x409/configuration > -rw-r--r-- 1 root root 4096 Jun 19 11:42 > /sys/kernel/config/usb_gadget/g1/configs/c.1/bmAttributes > -rw-r--r-- 1 root root 4096 Jun 19 11:40 > /sys/kernel/config/usb_gadget/g1/configs/c.1/MaxPower > drwxr-xr-x 3 root root 0 Jun 19 11:40 > /sys/kernel/config/usb_gadget/g1/functions > drwxr-xr-x 3 root root 0 Jun 19 11:41 > /sys/kernel/config/usb_gadget/g1/functions/mass_storage.0 > drwxr-xr-x 2 root root 0 Jun 19 11:41 > /sys/kernel/config/usb_gadget/g1/functions/mass_storage.0/lun.0 > -rw-r--r-- 1 root root 4096 Jun 19 11:42 > /sys/kernel/config/usb_gadget/g1/functions/mass_storage.0/lun.0/nofua > -rw-r--r-- 1 root root 4096 Jun 19 11:42 > /sys/kernel/config/usb_gadget/g1/functions/mass_storage.0/lun.0/cdrom > -rw-r--r-- 1 root root 4096 Jun 19 11:42 > /sys/kernel/config/usb_gadget/g1/functions/mass_storage.0/lun.0/removable > -rw-r--r-- 1 root root 4096 Jun 19 11:42 > /sys/kernel/config/usb_gadget/g1/functions/mass_storage.0/lun.0/ro > -rw-r--r-- 1 root root 4096 Jun 19 11:40 > /sys/kernel/config/usb_gadget/g1/functions/mass_storage.0/lun.0/file > -rw-r--r-- 1 root root 4096 Jun 19 11:42 > /sys/kernel/config/usb_gadget/g1/functions/mass_storage.0/stall > -rw-r--r-- 1 root root 4096 Jun 19 11:40 > /sys/kernel/config/usb_gadget/g1/UDC > -rw-r--r-- 1 root root 4096 Jun 19 11:42 > /sys/kernel/config/usb_gadget/g1/bcdUSB > -rw-r--r-- 1 root root 4096 Jun 19 11:42 > /sys/kernel/config/usb_gadget/g1/bcdDevice > -rw-r--r-- 1 root root 4096 Jun 19 11:40 > /sys/kernel/config/usb_gadget/g1/idProduct > -rw-r--r-- 1 root root 4096 Jun 19 11:40 > /sys/kernel/config/usb_gadget/g1/idVendor > -rw-r--r-- 1 root root 4096 Jun 19 11:42 > /sys/kernel/config/usb_gadget/g1/bMaxPacketSize0 > -rw-r--r-- 1 root root 4096 Jun 19 11:42 > /sys/kernel/config/usb_gadget/g1/bDeviceProtocol > -rw-r--r-- 1 root root 4096 Jun 19 11:42 > /sys/kernel/config/usb_gadget/g1/bDeviceSubClass > -rw-r--r-- 1 root root 4096 Jun 19 11:42 > /sys/kernel/config/usb_gadget/g1/bDeviceClass > > My interpretation of this is that common->nluns is only ever set in > fsg_alloc_inst() "rc = fsg_common_set_nluns(opts->common, > FSG_MAX_LUNS);" > It doesn't take account of how many luns have been setup in the > configfs tree.
your interpretation is correct.
> I have applied a hack/fix which works for me, though it needs tweaks
> and further testing given recent f_mass_storage.c patches I've seen
> floating about. This patch updates common->nluns on each
> fsg_common_create_lun(), and only prints Number of LUNs on fsg_bind
> when the configfs tree should be all there. Also assumes contiguous
> incrementing lun ids.
this makes sense to me although we need this in a proper patch format
(have a look at Documentation/SubmittingPatches and
Documentation/SubmitChecklist)
Michal, any comments to this diff ?
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c
> b/drivers/usb/gadget/function/f_mass_storage.c
> index 3cc109f..f502f00 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -2796,8 +2796,6 @@ int fsg_common_set_nluns(struct fsg_common *common, int
> nluns)
> common->luns = curlun;
> common->nluns = nluns;
>
> - pr_info("Number of LUNs=%d\n", common->nluns);
> -
> return 0;
> }
> EXPORT_SYMBOL_GPL(fsg_common_set_nluns);
> @@ -2941,6 +2939,9 @@ int fsg_common_create_lun(struct fsg_common *common,
> struct fsg_lun_config *cfg,
> p = "(error)";
> }
> }
> +
> + common->nluns = id + 1;
> +
> pr_info("LUN: %s%s%sfile: %s\n",
> lun->removable ? "removable " : "",
> lun->ro ? "read only " : "",
> @@ -2973,8 +2974,6 @@ int fsg_common_create_luns(struct fsg_common *common,
> struct fsg_config *cfg)
> goto fail;
> }
>
> - pr_info("Number of LUNs=%d\n", common->nluns);
> -
> return 0;
>
> fail:
> @@ -3121,6 +3120,8 @@ static int fsg_bind(struct usb_configuration *c, struct
> usb_function *f)
> if (ret)
> goto autoconf_fail;
>
> + pr_info("Number of LUNs=%d\n", fsg->common->nluns);
> +
> return 0;
>
> autoconf_fail:
>
>
>
> Is this something that needs fixing or am I misunderstanding how
> configfs usb mass storage is supposed to work or be configured ?
it certainly needs to be fixed.
--
balbi
signature.asc
Description: Digital signature
