2010-04-21 15:38:22

by Chouteau Fabien

[permalink] [raw]
Subject: [PATCH 1/2] Composite framework: Add suspended sysfs entry

From: Fabien Chouteau <[email protected]>


Signed-off-by: Fabien Chouteau <[email protected]>
---
drivers/usb/gadget/composite.c | 21 +++++++++++++++++++++
include/linux/usb/composite.h | 1 +
include/linux/usb/gadget.h | 4 ++++
3 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 09289bb..a5352f3 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -898,6 +898,18 @@ static void composite_disconnect(struct usb_gadget *gadget)

/*-------------------------------------------------------------------------*/

+static ssize_t composite_show_suspended(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct usb_gadget *gadget = dev_to_usb_gadget(dev);
+ struct usb_composite_dev *cdev = get_gadget_data(gadget);
+
+ return sprintf(buf, "%d\n", cdev->suspended);
+}
+
+static DEVICE_ATTR(suspended, 0444, composite_show_suspended, NULL);
+
static void /* __init_or_exit */
composite_unbind(struct usb_gadget *gadget)
{
@@ -944,6 +956,7 @@ composite_unbind(struct usb_gadget *gadget)
}
kfree(cdev);
set_gadget_data(gadget, NULL);
+ device_remove_file(&gadget->dev, &dev_attr_suspended);
composite = NULL;
}

@@ -1036,6 +1049,10 @@ static int __init composite_bind(struct usb_gadget *gadget)
string_override(composite->strings,
cdev->desc.iSerialNumber, iSerialNumber);

+ status = device_create_file(&gadget->dev, &dev_attr_suspended);
+ if (status)
+ goto fail;
+
INFO(cdev, "%s ready\n", composite->name);
return 0;

@@ -1064,6 +1081,8 @@ composite_suspend(struct usb_gadget *gadget)
}
if (composite->suspend)
composite->suspend(cdev);
+
+ cdev->suspended = 1;
}

static void
@@ -1084,6 +1103,8 @@ composite_resume(struct usb_gadget *gadget)
f->resume(f);
}
}
+
+ cdev->suspended = 0;
}

/*-------------------------------------------------------------------------*/
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 738ea1a..139353e 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -326,6 +326,7 @@ struct usb_composite_dev {

/* private: */
/* internals */
+ unsigned int suspended:1;
struct usb_device_descriptor desc;
struct list_head configs;
struct usb_composite_driver *driver;
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index f4b7ca5..db6141c 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -494,6 +494,10 @@ static inline void set_gadget_data(struct usb_gadget *gadget, void *data)
{ dev_set_drvdata(&gadget->dev, data); }
static inline void *get_gadget_data(struct usb_gadget *gadget)
{ return dev_get_drvdata(&gadget->dev); }
+static inline struct usb_gadget *dev_to_usb_gadget(struct device *dev)
+{
+ return container_of(dev, struct usb_gadget, dev);
+}

/* iterates the non-control endpoints; 'tmp' is a struct usb_ep pointer */
#define gadget_for_each_ep(tmp,gadget) \
--
1.7.0.5


2010-04-21 15:38:30

by Chouteau Fabien

[permalink] [raw]
Subject: [PATCH 2/2] Mass storage gadget: Handle eject request

From: Fabien Chouteau <[email protected]>


Signed-off-by: Fabien Chouteau <[email protected]>
---
drivers/usb/gadget/f_mass_storage.c | 52 +++++++++++++++++++++++++++++++---
drivers/usb/gadget/storage_common.c | 11 +++++++
2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index f4911c0..049f01b 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -1385,12 +1385,51 @@ static int do_mode_sense(struct fsg_common *common, struct fsg_buffhd *bh)

static int do_start_stop(struct fsg_common *common)
{
- if (!common->curlun) {
+ struct fsg_lun *curlun = common->curlun;
+ int loej, start;
+
+ if (!curlun) {
return -EINVAL;
- } else if (!common->curlun->removable) {
- common->curlun->sense_data = SS_INVALID_COMMAND;
+ } else if (!curlun->removable) {
+ curlun->sense_data = SS_INVALID_COMMAND;
return -EINVAL;
}
+
+ loej = common->cmnd[4] & 0x02;
+ start = common->cmnd[4] & 0x01;
+
+ /* eject code from file_storage.c:do_start_stop() */
+
+ if ((common->cmnd[1] & ~0x01) != 0 || /* Mask away Immed */
+ (common->cmnd[4] & ~0x03) != 0) { /* Mask LoEj, Start */
+ curlun->sense_data = SS_INVALID_FIELD_IN_CDB;
+ return -EINVAL;
+ }
+
+ if (!start) {
+ /* Are we allowed to unload the media? */
+ if (curlun->prevent_medium_removal) {
+ LDBG(curlun, "unload attempt prevented\n");
+ curlun->sense_data = SS_MEDIUM_REMOVAL_PREVENTED;
+ return -EINVAL;
+ }
+ if (loej) { /* Simulate an unload/eject */
+ up_read(&common->filesem);
+ down_write(&common->filesem);
+ fsg_lun_close(curlun);
+ curlun->ejected = 1;
+ up_write(&common->filesem);
+ down_read(&common->filesem);
+ }
+ } else {
+
+ /* Our emulation doesn't support mounting; the medium is
+ * available for use as soon as it is loaded. */
+ if (!fsg_lun_is_open(curlun)) {
+ curlun->sense_data = SS_MEDIUM_NOT_PRESENT;
+ return -EINVAL;
+ }
+ }
return 0;
}

@@ -2412,7 +2451,6 @@ static void fsg_disable(struct usb_function *f)
raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE);
}

-
/*-------------------------------------------------------------------------*/

static void handle_exception(struct fsg_common *common)
@@ -2641,7 +2679,7 @@ static int fsg_main_thread(void *common_)
/* Write permission is checked per LUN in store_*() functions. */
static DEVICE_ATTR(ro, 0644, fsg_show_ro, fsg_store_ro);
static DEVICE_ATTR(file, 0644, fsg_show_file, fsg_store_file);
-
+static DEVICE_ATTR(ejected, 0444, fsg_show_ejected, NULL);

/****************************** FSG COMMON ******************************/

@@ -2747,6 +2785,9 @@ static struct fsg_common *fsg_common_init(struct fsg_common *common,
rc = device_create_file(&curlun->dev, &dev_attr_file);
if (rc)
goto error_luns;
+ rc = device_create_file(&curlun->dev, &dev_attr_ejected);
+ if (rc)
+ goto error_luns;

if (lcfg->filename) {
rc = fsg_lun_open(curlun, lcfg->filename);
@@ -2887,6 +2928,7 @@ static void fsg_common_release(struct kref *ref)
for (; i; --i, ++lun) {
device_remove_file(&lun->dev, &dev_attr_ro);
device_remove_file(&lun->dev, &dev_attr_file);
+ device_remove_file(&lun->dev, &dev_attr_ejected);
fsg_lun_close(lun);
device_unregister(&lun->dev);
}
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index 868d8ee..16ccebd 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -267,6 +267,7 @@ struct fsg_lun {
unsigned int removable:1;
unsigned int cdrom:1;
unsigned int prevent_medium_removal:1;
+ unsigned int ejected:1;
unsigned int registered:1;
unsigned int info_valid:1;

@@ -625,6 +626,7 @@ static int fsg_lun_open(struct fsg_lun *curlun, const char *filename)
curlun->filp = filp;
curlun->file_length = size;
curlun->num_sectors = num_sectors;
+ curlun->ejected = 0;
LDBG(curlun, "open backing file: %s\n", filename);
rc = 0;

@@ -776,3 +778,12 @@ static ssize_t fsg_store_file(struct device *dev, struct device_attribute *attr,
up_write(filesem);
return (rc < 0 ? rc : count);
}
+
+static ssize_t fsg_show_ejected(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+
+ return sprintf(buf, "%d\n", curlun->ejected);
+}
+
--
1.7.0.5

2010-04-21 16:43:34

by Michal Nazarewicz

[permalink] [raw]
Subject: Re: [PATCH 2/2] Mass storage gadget: Handle eject request

On Wed, 21 Apr 2010 17:38:22 +0200, <[email protected]> wrote:
> From: Fabien Chouteau <[email protected]>
>
>
> Signed-off-by: Fabien Chouteau <[email protected]>

Commit message missing, in both patches. It should contain some
details on what the patch does and how this can be used.

> @@ -2412,7 +2451,6 @@ static void fsg_disable(struct usb_function *f)
> raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE);
> }
>-

This is not important, but if you are going to resend the patch,
how about not deleting this line?

> /*-------------------------------------------------------------------------*/
> static void handle_exception(struct fsg_common *common)
> @@ -2641,7 +2679,7 @@ static int fsg_main_thread(void *common_)
> /* Write permission is checked per LUN in store_*() functions. */
> static DEVICE_ATTR(ro, 0644, fsg_show_ro, fsg_store_ro);
> static DEVICE_ATTR(file, 0644, fsg_show_file, fsg_store_file);
> -
> +static DEVICE_ATTR(ejected, 0444, fsg_show_ejected, NULL);

Same here.

Otherwise, both patches look fine to me.

--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał "mina86" Nazarewicz (o o)
ooo +---[[email protected]]---[[email protected]]---ooO--(_)--Ooo--

2010-04-21 18:15:56

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 2/2] Mass storage gadget: Handle eject request

On Wed, Apr 21, 2010 at 06:43:44PM +0200, Michał Nazarewicz wrote:
> On Wed, 21 Apr 2010 17:38:22 +0200, <[email protected]> wrote:
> >From: Fabien Chouteau <[email protected]>
> >
> >
> >Signed-off-by: Fabien Chouteau <[email protected]>
>
> Commit message missing, in both patches. It should contain some
> details on what the patch does and how this can be used.

I agree, that is a requirement. Please resubmit these with some
description of what is happening.

thanks,

greg k-h

2010-04-21 18:16:12

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 1/2] Composite framework: Add suspended sysfs entry

On Wed, Apr 21, 2010 at 05:38:21PM +0200, [email protected] wrote:
> From: Fabien Chouteau <[email protected]>
>
>
> Signed-off-by: Fabien Chouteau <[email protected]>
> ---
> drivers/usb/gadget/composite.c | 21 +++++++++++++++++++++
> include/linux/usb/composite.h | 1 +
> include/linux/usb/gadget.h | 4 ++++
> 3 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 09289bb..a5352f3 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -898,6 +898,18 @@ static void composite_disconnect(struct usb_gadget *gadget)
>
> /*-------------------------------------------------------------------------*/
>
> +static ssize_t composite_show_suspended(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)

As you added a sysfs file, it is required that you also add an entry in
Documentation/ABI/ that describes the file and what it is for. Please
respin this with that added to the patch.

thanks,

greg k-h

2010-04-22 13:01:16

by Chouteau Fabien

[permalink] [raw]
Subject: Re: [PATCH 1/2] Composite framework: Add suspended sysfs entry

On Wed, Apr 21, 2010 at 8:05 PM, Greg KH <[email protected]> wrote:
>
> On Wed, Apr 21, 2010 at 05:38:21PM +0200, [email protected] wrote:
> > From: Fabien Chouteau <[email protected]>
> >
> >
> > Signed-off-by: Fabien Chouteau <[email protected]>
> > ---
> > ?drivers/usb/gadget/composite.c | ? 21 +++++++++++++++++++++
> > ?include/linux/usb/composite.h ?| ? ?1 +
> > ?include/linux/usb/gadget.h ? ? | ? ?4 ++++
> > ?3 files changed, 26 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> > index 09289bb..a5352f3 100644
> > --- a/drivers/usb/gadget/composite.c
> > +++ b/drivers/usb/gadget/composite.c
> > @@ -898,6 +898,18 @@ static void composite_disconnect(struct usb_gadget *gadget)
> >
> > ?/*-------------------------------------------------------------------------*/
> >
> > +static ssize_t composite_show_suspended(struct device *dev,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct device_attribute *attr,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? char *buf)
>
> As you added a sysfs file, it is required that you also add an entry in
> Documentation/ABI/ that describes the file and what it is for. ?Please
> respin this with that added to the patch.

ok,
is "testing/sysfs-devices-platform-_UDC_-gadget" a good choice for
the file name?
("_UDC_" depends on the UDC driver name)

--
Fabien Chouteau
EPITA GISTR 2010