On 06/22/2015 04:24 PM, Michal Nazarewicz wrote:
On Mon, Jun 22 2015, Krzysztof Opasiak wrote:
On 06/20/2015 12:18 AM, Michal Nazarewicz wrote:
On Fri, Jun 19 2015, Felipe Balbi wrote:
the fact is that this needs to be configurable from configfs. If user
sets up a single lun, then this should be as you said, however, if 2
luns are configured, I still want to have those 2 working.
From a user perspective, configfs should support N luns just fine as
long as we have enough memory available.
Yes, sorry, you’re right. I didn’t fallow closely migration to configfs
based configuration and didn’t have the full picture.
Well, not exactly. MS spec says that there should be no more that 16 LUNs.
Yeah. fsg_lun_make limits LUN to < FSG_MAX_LUNS which is eight so we’re
fine.
After taking another look, I think the best solution is to have a loop
in fsg_alloc, something like the following (beware that this has not
been tested in any way):
>From 19ec2796009f8650c7db4358f7e16931ba96e4ee Mon Sep 17 00:00:00 2001
From: Michal Nazarewicz <mina86@xxxxxxxxxx>
Date: Fri, 19 Jun 2015 23:56:34 +0200
Subject: [PATCH] usb: f_mass_storage: limit number of reported LUNs
Mass storage function created via configfs always reports eight LUNs
to the hosts even if only one LUN has been configured. Adjust the
number when the USB function is allocated based on LUNs that user
has created.
Signed-off-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
---
drivers/usb/gadget/function/f_mass_storage.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index 3cc109f..8e26a12 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -3563,14 +3563,28 @@ static struct usb_function *fsg_alloc(struct usb_function_instance *fi)
struct fsg_opts *opts = fsg_opts_from_func_inst(fi);
struct fsg_common *common = opts->common;
struct fsg_dev *fsg;
+ unsigned nluns, i;
fsg = kzalloc(sizeof(*fsg), GFP_KERNEL);
if (unlikely(!fsg))
return ERR_PTR(-ENOMEM);
mutex_lock(&opts->lock);
+ if (!opts->refcnt) {
+ for (nluns = i = 0; i < FSG_MAX_LUNS; ++i)
+ if (common->luns[i])
+ nluns = i + 1;
+ if (!nluns) {
+ pr_warn("No LUNS defined, continuing anyway\n");
+ } else if (nluns != common->nluns) {
+ pr_info("Adjusting number of LUNs=%u (was %u)\n",
+ nluns, common->nluns);
+ common->nluns = nluns;
+ }
+ }
opts->refcnt++;
mutex_unlock(&opts->lock);
+
fsg->function.name = FSG_DRIVER_DESC;
fsg->function.bind = fsg_bind;
fsg->function.unbind = fsg_unbind;
Looks simple but we will still get comment "number of LUNs=8". Moreover
it may cause some trouble if we set nluns == 0,
Which is why the code leaves common->nluns == 8 if nluns == 0.
because in create lun we check whether nluns > 0. In addition if we
use for example fsg_common_set_nluns(1) or anything other what <
FSG_MAX_LUNS this loop will read past allocated buffer.
common->nluns is set to FSG_MAX_LUNS in fsg_alloc_inst (which is called
prior to fsg_alloc) so this is not an issue.
True, but all legacy gadgets which use mass storage call
fsg_common_set_nluns() after reading module parameters and after
allocating function instance. As argument they pass number luns received
in module params.
BR's
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
