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
Attachment:
signature.asc
Description: Digital signature
