On 07/29/2016 01:45 PM, Philipp Gesang wrote:
> Introduce an attribute "inquiry_string" to the lun.
>
> In some environments, e. g. BIOS boot menus, the inquiry string
> is the only information about devices presented to the user. The
> default string depends on the "cdrom" bit of the first lun as
> well as the kernel version and allows no further customization.
> So without access to the client it is not obvious which gadget is
> active at a given point and what any of the available luns might
> contain.
>
> If "inquiry_string" is ignored or set to the empty string, the
> old behavior is preserved.
>
> Signed-off-by: Philipp Gesang <[email protected]>
> ---
> drivers/usb/gadget/function/f_mass_storage.c | 22 +++++++++++++++++++++-
> drivers/usb/gadget/function/f_mass_storage.h | 1 +
> drivers/usb/gadget/function/storage_common.c | 24 ++++++++++++++++++++++++
> drivers/usb/gadget/function/storage_common.h | 4 ++++
> 4 files changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c
> b/drivers/usb/gadget/function/f_mass_storage.c
> index 2505117..47d8248 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -1107,7 +1107,12 @@ static int do_inquiry(struct fsg_common *common,
> struct fsg_buffhd *bh)
> buf[5] = 0; /* No special options */
> buf[6] = 0;
> buf[7] = 0;
> - memcpy(buf + 8, common->inquiry_string, sizeof common->inquiry_string);
> + if (curlun->inquiry_string[0])
> + memcpy(buf + 8, curlun->inquiry_string,
> + sizeof(curlun->inquiry_string));
> + else
> + memcpy(buf + 8, common->inquiry_string,
> + sizeof(common->inquiry_string));
> return 36;
> }
>
> @@ -3209,12 +3214,27 @@ static ssize_t fsg_lun_opts_nofua_store(struct
> config_item *item,
>
> CONFIGFS_ATTR(fsg_lun_opts_, nofua);
>
> +static ssize_t fsg_lun_opts_inquiry_string_show(struct config_item *item,
> + char *page)
> +{
> + return fsg_show_inquiry_string(to_fsg_lun_opts(item)->lun, page);
> +}
> +
> +static ssize_t fsg_lun_opts_inquiry_string_store(struct config_item *item,
> + const char *page, size_t len)
> +{
> + return fsg_store_inquiry_string(to_fsg_lun_opts(item)->lun, page, len);
> +}
> +
> +CONFIGFS_ATTR(fsg_lun_opts_, inquiry_string);
> +
> static struct configfs_attribute *fsg_lun_attrs[] = {
> &fsg_lun_opts_attr_file,
> &fsg_lun_opts_attr_ro,
> &fsg_lun_opts_attr_removable,
> &fsg_lun_opts_attr_cdrom,
> &fsg_lun_opts_attr_nofua,
> + &fsg_lun_opts_attr_inquiry_string,
> NULL,
> };
>
> diff --git a/drivers/usb/gadget/function/f_mass_storage.h
> b/drivers/usb/gadget/function/f_mass_storage.h
> index b6a9918..99ac7ad 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.h
> +++ b/drivers/usb/gadget/function/f_mass_storage.h
> @@ -100,6 +100,7 @@ struct fsg_lun_config {
> char removable;
> char cdrom;
> char nofua;
> + char inquiry_string[8 + 16 + 4 + 1];
Could you please make those calculations as some define (as they exist
in at least two places) and add some comments about meaning of those
magic numbers?
> };
>
> struct fsg_config {
> diff --git a/drivers/usb/gadget/function/storage_common.c
> b/drivers/usb/gadget/function/storage_common.c
> index 990df22..9b56859 100644
> --- a/drivers/usb/gadget/function/storage_common.c
> +++ b/drivers/usb/gadget/function/storage_common.c
> @@ -369,6 +369,12 @@ ssize_t fsg_show_removable(struct fsg_lun *curlun, char
> *buf)
> }
> EXPORT_SYMBOL_GPL(fsg_show_removable);
>
> +ssize_t fsg_show_inquiry_string(struct fsg_lun *curlun, char *buf)
> +{
> + return sprintf(buf, "%s\n", curlun->inquiry_string);
> +}
> +EXPORT_SYMBOL_GPL(fsg_show_inquiry_string);
> +
> /*
> * The caller must hold fsg->filesem for reading when calling this function.
> */
> @@ -499,4 +505,22 @@ ssize_t fsg_store_removable(struct fsg_lun *curlun,
> const char *buf,
> }
> EXPORT_SYMBOL_GPL(fsg_store_removable);
>
> +ssize_t fsg_store_inquiry_string(struct fsg_lun *curlun, const char *buf,
> + size_t count)
> +{
> + const size_t len = min(count, sizeof(curlun->inquiry_string));
> +
> + if (len == 0 || buf[0] == '\n')
> + curlun->inquiry_string[0] = 0;
> + else {
> + snprintf(curlun->inquiry_string,
> + sizeof(curlun->inquiry_string), "%-28s", buf);
> + if (curlun->inquiry_string[len-1] == '\n')
> + curlun->inquiry_string[len-1] = ' ';
> + }
According to codding style you should probably do:
if (...) {
instruction;
} else {
instruction;
instruction;
/* ... */
}
in this case.
> +
> + return count;
> +}
> +EXPORT_SYMBOL_GPL(fsg_store_inquiry_string);
> +
> MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/gadget/function/storage_common.h
> b/drivers/usb/gadget/function/storage_common.h
> index c3544e6..af74044 100644
> --- a/drivers/usb/gadget/function/storage_common.h
> +++ b/drivers/usb/gadget/function/storage_common.h
> @@ -112,6 +112,7 @@ struct fsg_lun {
> struct device dev;
> const char *name; /* "lun.name" */
> const char **name_pfx; /* "function.name" */
> + char inquiry_string[8 + 16 + 4 + 1];
Just like I wrote above. Please make define for size of this array.
Best regards,
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html