On Thu, Oct 03 2013, Andrzej Pietrasiewicz wrote:
> When configfs is in place, the luns will not be represented in sysfs,
> so there will be no struct device associated with a lun.
> In order to maintain compatibility and allow configfs adoption
> sysfs is made optional in this patch.
>
> As a consequence some debug macros need to be adjusted. Two new
> fields are added to struct fsg_lun: name and name_pfx.
> The "name" is for storing a string which is presented to the user
> instead of the dev_name. The "name_pfx", if non-NULL, is prepended
> to the "name" at printing time.
>
> The name_pfx is for a future lun.0, which will be a default group in
> mass_storage.<name>. By design at USB function configfs group's creation
> time its name is not known (but instead set a bit later in
> drivers/usb/gadget/configfs.c:function_make) and it is this name that
> serves the purpose of the said name prefix. So instead of copying
> a yet-unknown string a pointer to it is stored in struct fsg_lun.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
Acked-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
> ---
> drivers/usb/gadget/f_mass_storage.c | 41 ++++++++++++++++++++++++++--------
> drivers/usb/gadget/f_mass_storage.h | 2 +
> drivers/usb/gadget/storage_common.h | 20 +++++++++++++---
> 3 files changed, 49 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
> index 7d57542..14c9e5b 100644
> --- a/drivers/usb/gadget/f_mass_storage.c
> +++ b/drivers/usb/gadget/f_mass_storage.c
> @@ -2654,6 +2660,17 @@ static void _fsg_common_free_buffers(struct fsg_buffhd *buffhds, unsigned n)
> }
> }
>
> +static inline void fsg_common_remove_sysfs(struct fsg_lun *lun)
> +{
> + device_remove_file(&lun->dev, &dev_attr_nofua);
> + device_remove_file(&lun->dev, lun->cdrom
> + ? &dev_attr_ro_cdrom : &dev_attr_ro);
> + device_remove_file(&lun->dev, lun->removable
> + ? &dev_attr_file : &dev_attr_file_nonremovable);
It actually occurred to me that dose conditions should not be
necessary. Since the file is being removed, only the name is of
interest to the device_remove_file. Just a thought.
> +}
> +
> +#define MAX_LUN_NAME_LEN 80
> +
> struct fsg_common *fsg_common_init(struct fsg_common *common,
> struct usb_composite_dev *cdev,
> struct fsg_config *cfg)
> @@ -2737,6 +2756,12 @@ struct fsg_common *fsg_common_init(struct fsg_common *common,
> }
> *curlun_it = curlun;
>
> + curlun->name = kzalloc(MAX_LUN_NAME_LEN, GFP_KERNEL);
> + if (!curlun->name) {
> + rc = -ENOMEM;
> + common->nluns = i;
> + goto error_release;
> + }
> curlun->cdrom = !!lcfg->cdrom;
> curlun->ro = lcfg->cdrom || lcfg->ro;
> curlun->initially_ro = curlun->ro;
> @@ -2746,6 +2771,7 @@ struct fsg_common *fsg_common_init(struct fsg_common *common,
> /* curlun->dev.driver = &fsg_driver.driver; XXX */
> dev_set_drvdata(&curlun->dev, &common->filesem);
> dev_set_name(&curlun->dev, "lun%d", i);
> + strlcpy(curlun->name, dev_name(&curlun->dev), MAX_LUN_NAME_LEN);
Instead of creating a new buffer and copying the data, would it actually
be possible to just store a pointer to dev_name(&curlun->dev)? And with
configfs, store pointer to the directory name? Since we control when
device is unregistered, and I presume we will be notified when configfs
directory is deleted, we should be able to NULL the name when the
pointer stops being valid. This would remove a magic number and the
need to maintain the buffer.
>
> rc = device_register(&curlun->dev);
> if (rc) {
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +--<mpn@xxxxxxxxxx>--<xmpp:mina86@xxxxxxxxxx>--ooO--(_)--Ooo--
Attachment:
signature.asc
Description: PGP signature
