> On Fri, Jun 19, 2015 at 01:59:40PM +0000, David Fisher wrote:
>> 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.

On Fri, Jun 19 2015, Felipe Balbi wrote:
> 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;
>> +

This breaks fsg_common_create_luns.  If configfs interface allows for
a single LUN only (which I think is the case), just change

        rc = fsg_common_set_nluns(opts->common, FSG_MAX_LUNS);

to

        /* Only one LUN is supported via configfs */
        rc = fsg_common_set_nluns(opts->common, 1);

in fsg_alloc_inst.


>>      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

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<m...@google.com>--<xmpp:min...@jabber.org>--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in

Reply via email to