Re: [PATCH] usb: gadget: Add per-lun inquiry string

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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 <philipp.gesang@xxxxxxxxxxxxx>
> ---
>  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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux