Re: [PATCH v6 13/14] usb/gadget: storage_common: add methods to show/store 'cdrom' and 'removable'

2013-10-11 Thread Andrzej Pietrasiewicz

W dniu 10.10.2013 20:05, Michal Nazarewicz pisze:

On Wed, Oct 09 2013, Andrzej Pietrasiewicz wrote:

This will be required by configfs integration.

Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungin Park kyungmin.p...@samsung.com
---
  drivers/usb/gadget/storage_common.c |   42 +++
  drivers/usb/gadget/storage_common.h |5 
  2 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/storage_common.c 
b/drivers/usb/gadget/storage_common.c
index 969948d..c7b78a1 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -439,4 +450,35 @@ ssize_t fsg_store_file(struct fsg_lun *curlun, struct 
rw_semaphore *filesem,
  }
  EXPORT_SYMBOL(fsg_store_file);

+ssize_t fsg_store_cdrom(struct fsg_lun *curlun, const char *buf, size_t count)
+{
+   unsignedcdrom;
+   int ret;
+
+   ret = kstrtouint(buf, 2, cdrom);


Base 2?  That's rather strange.  Just keep it base 10 or use strtobool().


I vote for strtobool().




+   if (ret)
+   return ret;
+
+   curlun-cdrom = cdrom;
+
+   return count;
+}
+EXPORT_SYMBOL(fsg_store_cdrom);


Loading the module with cdrom set, implies read-only flag, but with this
function, it will become possible to set cdrom without setting read-only
flag.  Some care is needed to make it work properly.


Is it enough if I call fsg_store_ro() before actually assigning
curlun-crdrom = cdrom?

Of course some adjustments are needed; the essential part of
fsg_store_ro() factored out to e.g. _fsg_store_ro()
(assumes locks taken), fsg_store_cdrom() accepting the filesem and
taking it before calling _fsg_store_ro(); appropriate error handling in
case _fsg_store_ro() fails.

AP


--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 13/14] usb/gadget: storage_common: add methods to show/store 'cdrom' and 'removable'

2013-10-11 Thread Michal Nazarewicz
On Fri, Oct 11 2013, Andrzej Pietrasiewicz wrote:
 W dniu 10.10.2013 20:05, Michal Nazarewicz pisze:
 On Wed, Oct 09 2013, Andrzej Pietrasiewicz wrote:
 This will be required by configfs integration.

 Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
 Signed-off-by: Kyungin Park kyungmin.p...@samsung.com
 ---
   drivers/usb/gadget/storage_common.c |   42 
 +++
   drivers/usb/gadget/storage_common.h |5 
   2 files changed, 47 insertions(+), 0 deletions(-)

 diff --git a/drivers/usb/gadget/storage_common.c 
 b/drivers/usb/gadget/storage_common.c
 index 969948d..c7b78a1 100644
 --- a/drivers/usb/gadget/storage_common.c
 +++ b/drivers/usb/gadget/storage_common.c
 @@ -439,4 +450,35 @@ ssize_t fsg_store_file(struct fsg_lun *curlun, struct 
 rw_semaphore *filesem,
   }
   EXPORT_SYMBOL(fsg_store_file);

 +ssize_t fsg_store_cdrom(struct fsg_lun *curlun, const char *buf, size_t 
 count)
 +{
 +   unsignedcdrom;
 +   int ret;
 +
 +   ret = kstrtouint(buf, 2, cdrom);

 Base 2?  That's rather strange.  Just keep it base 10 or use strtobool().

 I vote for strtobool().

One thing to keep in mind though, is that wit strtobool() the interface
for those files would be different from the interface for the other
files.  Namely, cdrom would accept, 0, 1, n, N, y and Y, whereas the
other files would accept numbers and treat non-zero as true.

 +   if (ret)
 +   return ret;
 +
 +   curlun-cdrom = cdrom;
 +
 +   return count;
 +}
 +EXPORT_SYMBOL(fsg_store_cdrom);

 Loading the module with cdrom set, implies read-only flag, but with this
 function, it will become possible to set cdrom without setting read-only
 flag.  Some care is needed to make it work properly.

 Is it enough if I call fsg_store_ro() before actually assigning
 curlun-crdrom = cdrom?

I don't think you need to call fsg_store_ro, just set the ro flag if
cdrom flog is being set.

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


signature.asc
Description: PGP signature


Re: [PATCH v6 13/14] usb/gadget: storage_common: add methods to show/store 'cdrom' and 'removable'

2013-10-10 Thread Michal Nazarewicz
On Wed, Oct 09 2013, Andrzej Pietrasiewicz wrote:
 This will be required by configfs integration.

 Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
 Signed-off-by: Kyungin Park kyungmin.p...@samsung.com
 ---
  drivers/usb/gadget/storage_common.c |   42 
 +++
  drivers/usb/gadget/storage_common.h |5 
  2 files changed, 47 insertions(+), 0 deletions(-)

 diff --git a/drivers/usb/gadget/storage_common.c 
 b/drivers/usb/gadget/storage_common.c
 index 969948d..c7b78a1 100644
 --- a/drivers/usb/gadget/storage_common.c
 +++ b/drivers/usb/gadget/storage_common.c
 @@ -439,4 +450,35 @@ ssize_t fsg_store_file(struct fsg_lun *curlun, struct 
 rw_semaphore *filesem,
  }
  EXPORT_SYMBOL(fsg_store_file);
  
 +ssize_t fsg_store_cdrom(struct fsg_lun *curlun, const char *buf, size_t 
 count)
 +{
 + unsignedcdrom;
 + int ret;
 +
 + ret = kstrtouint(buf, 2, cdrom);

Base 2?  That's rather strange.  Just keep it base 10 or use strtobool().

 + if (ret)
 + return ret;
 +
 + curlun-cdrom = cdrom;
 +
 + return count;
 +}
 +EXPORT_SYMBOL(fsg_store_cdrom);

Loading the module with cdrom set, implies read-only flag, but with this
function, it will become possible to set cdrom without setting read-only
flag.  Some care is needed to make it work properly.

 +
 +ssize_t fsg_store_removable(struct fsg_lun *curlun, const char *buf,
 + size_t count)
 +{
 + unsignedremovable;
 + int ret;
 +
 + ret = kstrtouint(buf, 2, removable);
 + if (ret)
 + return ret;
 +
 + curlun-removable = removable;
 +
 + return count;
 +}
 +EXPORT_SYMBOL(fsg_store_removable);
 +
  MODULE_LICENSE(GPL);

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


signature.asc
Description: PGP signature