Re: [PATCH v5] usb: gadget: f_fs: add no_disconnect mode

2015-01-13 Thread Felipe Balbi
Hi,

On Thu, Dec 18, 2014 at 09:55:10AM +0100, Robert Baldyga wrote:
 Since we can compose gadgets from many functions, there is the problem
 related to gadget breakage while FunctionFS daemon being closed. FFS
 function is userspace code so there is no way to know when it will close
 files (it doesn't matter what is the reason of this situation, it can
 be daemon logic, program breakage, process kill or any other). So when
 we have another function in gadget which, for example, sends some amount
 of data, does some software update or implements some real-time functionality,
 we may want to keep the gadget connected despite FFS function is no longer
 functional.
 
 We can't just remove one of functions from gadget since it has been
 enumerated, so the only way to keep entire gadget working is to make
 broken FFS function deactivated but still visible to host. For this
 purpose this patch introduces no_disconnect mode. It can be enabled
 by setting mount option no_disconnect=1, and results with defering
 function disconnect to the moment of reopen ep0 file or filesystem
 unmount. After closing all endpoint files, FunctionFS is set to state
 FFS_DEACTIVATED.
 
 When ffs-state == FFS_DEACTIVATED:
 - function is still bound and visible to host,
 - setup requests are automatically stalled,
 - transfers on other endpoints are refused,
 - epfiles, except ep0, are deleted from the filesystem,
 - opening ep0 causes the function to be closed, and then FunctionFS
   is ready for descriptors and string write,
 - altsetting change causes the function to be closed - we want to keep
   function alive until another functions are potentialy used, altsetting
   change means that another configuration is being selected or USB cable
   was unplugged, which indicates that we don't need to stay longer in
   FFS_DEACTIVATED state
 - unmounting of the FunctionFS instance causes the function to be closed.
 
 Signed-off-by: Robert Baldyga r.bald...@samsung.com
 
 Changelog:
 
 v5:
 - close function on altsetting change
 
 v4: https://lkml.org/lkml/2014/10/9/224
 - use ffs_data_reset() instead of ffs_data_clear() to reset ffs data
   properly after ffs-ref refcount reach 0 (or under in no_disconnect
   mode) in ffs_data_put() function
 
 v3: https://lkml.org/lkml/2014/10/9/170
 - change option name to more descriptive and less scary,
 - fix cleaning up on unmount (call ffs_data_closed() in ffs_fs_kill_sb(),
   and ffs_data_clear() in ffs_data_closed() if ffs-opened is negative).
 
 v2: https://lkml.org/lkml/2014/10/7/109
 - delete epfiles, excepting ep0, when FFS is in zombie mode,
 - add description of FFS_ZOMBIE state,
 - minor cleanups.
 
 v1: https://lkml.org/lkml/2014/10/6/128

This changelog should be after the tearline, I've manually editted it
and also applied the small modification sugested by Michal. It should
show up on my testing/next soonish.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v5] usb: gadget: f_fs: add no_disconnect mode

2014-12-24 Thread Michal Nazarewicz
On Thu, Dec 18 2014, Robert Baldyga wrote:
 Since we can compose gadgets from many functions, there is the problem
 related to gadget breakage while FunctionFS daemon being closed. FFS
 function is userspace code so there is no way to know when it will close
 files (it doesn't matter what is the reason of this situation, it can
 be daemon logic, program breakage, process kill or any other). So when
 we have another function in gadget which, for example, sends some amount
 of data, does some software update or implements some real-time functionality,
 we may want to keep the gadget connected despite FFS function is no longer
 functional.

 We can't just remove one of functions from gadget since it has been
 enumerated, so the only way to keep entire gadget working is to make
 broken FFS function deactivated but still visible to host. For this
 purpose this patch introduces no_disconnect mode. It can be enabled
 by setting mount option no_disconnect=1, and results with defering
 function disconnect to the moment of reopen ep0 file or filesystem
 unmount. After closing all endpoint files, FunctionFS is set to state
 FFS_DEACTIVATED.

 When ffs-state == FFS_DEACTIVATED:
 - function is still bound and visible to host,
 - setup requests are automatically stalled,
 - transfers on other endpoints are refused,
 - epfiles, except ep0, are deleted from the filesystem,
 - opening ep0 causes the function to be closed, and then FunctionFS
   is ready for descriptors and string write,
 - altsetting change causes the function to be closed - we want to keep
   function alive until another functions are potentialy used, altsetting
   change means that another configuration is being selected or USB cable
   was unplugged, which indicates that we don't need to stay longer in
   FFS_DEACTIVATED state
 - unmounting of the FunctionFS instance causes the function to be closed.

 Signed-off-by: Robert Baldyga r.bald...@samsung.com

Acked-by: Michal Nazarewicz min...@mina86.com

 @@ -1417,7 +1429,11 @@ static void ffs_data_opened(struct ffs_data *ffs)
   ENTER();
  
   atomic_inc(ffs-ref);
 - atomic_inc(ffs-opened);
 + if (atomic_add_return(1, ffs-opened) == 1)
 + if (ffs-state == FFS_DEACTIVATED) {
 + ffs-state = FFS_CLOSING;
 + ffs_data_reset(ffs);
 + }

Wrong indention.  Why not:

+   if (atomic_add_return(1, ffs-opened) == 1 
+   ffs-state == FFS_DEACTIVATED) {
+   ffs-state = FFS_CLOSING;
+   ffs_data_reset(ffs);
+   }

  }
  
  static void ffs_data_put(struct ffs_data *ffs)
 @@ -1439,6 +1455,21 @@ static void ffs_data_closed(struct ffs_data *ffs)

-- 
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--
--
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 v5] usb: gadget: f_fs: add no_disconnect mode

2014-12-23 Thread Felipe Balbi
On Thu, Dec 18, 2014 at 09:55:10AM +0100, Robert Baldyga wrote:
 Since we can compose gadgets from many functions, there is the problem
 related to gadget breakage while FunctionFS daemon being closed. FFS
 function is userspace code so there is no way to know when it will close
 files (it doesn't matter what is the reason of this situation, it can
 be daemon logic, program breakage, process kill or any other). So when
 we have another function in gadget which, for example, sends some amount
 of data, does some software update or implements some real-time functionality,
 we may want to keep the gadget connected despite FFS function is no longer
 functional.
 
 We can't just remove one of functions from gadget since it has been
 enumerated, so the only way to keep entire gadget working is to make
 broken FFS function deactivated but still visible to host. For this
 purpose this patch introduces no_disconnect mode. It can be enabled
 by setting mount option no_disconnect=1, and results with defering
 function disconnect to the moment of reopen ep0 file or filesystem
 unmount. After closing all endpoint files, FunctionFS is set to state
 FFS_DEACTIVATED.
 
 When ffs-state == FFS_DEACTIVATED:
 - function is still bound and visible to host,
 - setup requests are automatically stalled,
 - transfers on other endpoints are refused,
 - epfiles, except ep0, are deleted from the filesystem,
 - opening ep0 causes the function to be closed, and then FunctionFS
   is ready for descriptors and string write,
 - altsetting change causes the function to be closed - we want to keep
   function alive until another functions are potentialy used, altsetting
   change means that another configuration is being selected or USB cable
   was unplugged, which indicates that we don't need to stay longer in
   FFS_DEACTIVATED state
 - unmounting of the FunctionFS instance causes the function to be closed.
 
 Signed-off-by: Robert Baldyga r.bald...@samsung.com

David, can you test it with your setup ?

cheers

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v5] usb: gadget: f_fs: add no_disconnect mode

2014-12-23 Thread David Cohen
On Tue, Dec 23, 2014 at 12:48:58PM -0600, Felipe Balbi wrote:
 On Thu, Dec 18, 2014 at 09:55:10AM +0100, Robert Baldyga wrote:
  Since we can compose gadgets from many functions, there is the problem
  related to gadget breakage while FunctionFS daemon being closed. FFS
  function is userspace code so there is no way to know when it will close
  files (it doesn't matter what is the reason of this situation, it can
  be daemon logic, program breakage, process kill or any other). So when
  we have another function in gadget which, for example, sends some amount
  of data, does some software update or implements some real-time 
  functionality,
  we may want to keep the gadget connected despite FFS function is no longer
  functional.
  
  We can't just remove one of functions from gadget since it has been
  enumerated, so the only way to keep entire gadget working is to make
  broken FFS function deactivated but still visible to host. For this
  purpose this patch introduces no_disconnect mode. It can be enabled
  by setting mount option no_disconnect=1, and results with defering
  function disconnect to the moment of reopen ep0 file or filesystem
  unmount. After closing all endpoint files, FunctionFS is set to state
  FFS_DEACTIVATED.
  
  When ffs-state == FFS_DEACTIVATED:
  - function is still bound and visible to host,
  - setup requests are automatically stalled,
  - transfers on other endpoints are refused,
  - epfiles, except ep0, are deleted from the filesystem,
  - opening ep0 causes the function to be closed, and then FunctionFS
is ready for descriptors and string write,
  - altsetting change causes the function to be closed - we want to keep
function alive until another functions are potentialy used, altsetting
change means that another configuration is being selected or USB cable
was unplugged, which indicates that we don't need to stay longer in
FFS_DEACTIVATED state
  - unmounting of the FunctionFS instance causes the function to be closed.
  
  Signed-off-by: Robert Baldyga r.bald...@samsung.com
 
 David, can you test it with your setup ?

Works fine on my side.
Tested-by: David Cohen david.a.co...@linux.intel.com

 
 cheers
 
 -- 
 balbi


--
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 v5] usb: gadget: f_fs: add no_disconnect mode

2014-12-23 Thread Felipe Balbi
On Tue, Dec 23, 2014 at 12:47:55PM -0800, David Cohen wrote:
 On Tue, Dec 23, 2014 at 12:48:58PM -0600, Felipe Balbi wrote:
  On Thu, Dec 18, 2014 at 09:55:10AM +0100, Robert Baldyga wrote:
   Since we can compose gadgets from many functions, there is the problem
   related to gadget breakage while FunctionFS daemon being closed. FFS
   function is userspace code so there is no way to know when it will close
   files (it doesn't matter what is the reason of this situation, it can
   be daemon logic, program breakage, process kill or any other). So when
   we have another function in gadget which, for example, sends some amount
   of data, does some software update or implements some real-time 
   functionality,
   we may want to keep the gadget connected despite FFS function is no longer
   functional.
   
   We can't just remove one of functions from gadget since it has been
   enumerated, so the only way to keep entire gadget working is to make
   broken FFS function deactivated but still visible to host. For this
   purpose this patch introduces no_disconnect mode. It can be enabled
   by setting mount option no_disconnect=1, and results with defering
   function disconnect to the moment of reopen ep0 file or filesystem
   unmount. After closing all endpoint files, FunctionFS is set to state
   FFS_DEACTIVATED.
   
   When ffs-state == FFS_DEACTIVATED:
   - function is still bound and visible to host,
   - setup requests are automatically stalled,
   - transfers on other endpoints are refused,
   - epfiles, except ep0, are deleted from the filesystem,
   - opening ep0 causes the function to be closed, and then FunctionFS
 is ready for descriptors and string write,
   - altsetting change causes the function to be closed - we want to keep
 function alive until another functions are potentialy used, altsetting
 change means that another configuration is being selected or USB cable
 was unplugged, which indicates that we don't need to stay longer in
 FFS_DEACTIVATED state
   - unmounting of the FunctionFS instance causes the function to be closed.
   
   Signed-off-by: Robert Baldyga r.bald...@samsung.com
  
  David, can you test it with your setup ?
 
 Works fine on my side.
 Tested-by: David Cohen david.a.co...@linux.intel.com

just to make sure, did you try with and without the new parameter ? I
remember someone complaining about regressions when the new parameter
was used.

cheers

-- 
balbi


signature.asc
Description: Digital signature


[PATCH v5] usb: gadget: f_fs: add no_disconnect mode

2014-12-18 Thread Robert Baldyga
Since we can compose gadgets from many functions, there is the problem
related to gadget breakage while FunctionFS daemon being closed. FFS
function is userspace code so there is no way to know when it will close
files (it doesn't matter what is the reason of this situation, it can
be daemon logic, program breakage, process kill or any other). So when
we have another function in gadget which, for example, sends some amount
of data, does some software update or implements some real-time functionality,
we may want to keep the gadget connected despite FFS function is no longer
functional.

We can't just remove one of functions from gadget since it has been
enumerated, so the only way to keep entire gadget working is to make
broken FFS function deactivated but still visible to host. For this
purpose this patch introduces no_disconnect mode. It can be enabled
by setting mount option no_disconnect=1, and results with defering
function disconnect to the moment of reopen ep0 file or filesystem
unmount. After closing all endpoint files, FunctionFS is set to state
FFS_DEACTIVATED.

When ffs-state == FFS_DEACTIVATED:
- function is still bound and visible to host,
- setup requests are automatically stalled,
- transfers on other endpoints are refused,
- epfiles, except ep0, are deleted from the filesystem,
- opening ep0 causes the function to be closed, and then FunctionFS
  is ready for descriptors and string write,
- altsetting change causes the function to be closed - we want to keep
  function alive until another functions are potentialy used, altsetting
  change means that another configuration is being selected or USB cable
  was unplugged, which indicates that we don't need to stay longer in
  FFS_DEACTIVATED state
- unmounting of the FunctionFS instance causes the function to be closed.

Signed-off-by: Robert Baldyga r.bald...@samsung.com

Changelog:

v5:
- close function on altsetting change

v4: https://lkml.org/lkml/2014/10/9/224
- use ffs_data_reset() instead of ffs_data_clear() to reset ffs data
  properly after ffs-ref refcount reach 0 (or under in no_disconnect
  mode) in ffs_data_put() function

v3: https://lkml.org/lkml/2014/10/9/170
- change option name to more descriptive and less scary,
- fix cleaning up on unmount (call ffs_data_closed() in ffs_fs_kill_sb(),
  and ffs_data_clear() in ffs_data_closed() if ffs-opened is negative).

v2: https://lkml.org/lkml/2014/10/7/109
- delete epfiles, excepting ep0, when FFS is in zombie mode,
- add description of FFS_ZOMBIE state,
- minor cleanups.

v1: https://lkml.org/lkml/2014/10/6/128

---
 drivers/usb/gadget/function/f_fs.c | 56 ++
 drivers/usb/gadget/function/u_fs.h | 24 
 2 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index 63314ed..d49af15 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -606,6 +606,8 @@ static unsigned int ffs_ep0_poll(struct file *file, 
poll_table *wait)
}
case FFS_CLOSING:
break;
+   case FFS_DEACTIVATED:
+   break;
}
 
mutex_unlock(ffs-mutex);
@@ -1180,6 +1182,7 @@ struct ffs_sb_fill_data {
struct ffs_file_perms perms;
umode_t root_mode;
const char *dev_name;
+   bool no_disconnect;
struct ffs_data *ffs_data;
 };
 
@@ -1250,6 +1253,12 @@ static int ffs_fs_parse_opts(struct ffs_sb_fill_data 
*data, char *opts)
 
/* Interpret option */
switch (eq - opts) {
+   case 13:
+   if (!memcmp(opts, no_disconnect, 13))
+   data-no_disconnect = !!value;
+   else
+   goto invalid;
+   break;
case 5:
if (!memcmp(opts, rmode, 5))
data-root_mode  = (value  0555) | S_IFDIR;
@@ -1314,6 +1323,7 @@ ffs_fs_mount(struct file_system_type *t, int flags,
.gid = GLOBAL_ROOT_GID,
},
.root_mode = S_IFDIR | 0500,
+   .no_disconnect = false,
};
struct dentry *rv;
int ret;
@@ -1330,6 +1340,7 @@ ffs_fs_mount(struct file_system_type *t, int flags,
if (unlikely(!ffs))
return ERR_PTR(-ENOMEM);
ffs-file_perms = data.perms;
+   ffs-no_disconnect = data.no_disconnect;
 
ffs-dev_name = kstrdup(dev_name, GFP_KERNEL);
if (unlikely(!ffs-dev_name)) {
@@ -1361,6 +1372,7 @@ ffs_fs_kill_sb(struct super_block *sb)
kill_litter_super(sb);
if (sb-s_fs_info) {
ffs_release_dev(sb-s_fs_info);
+   ffs_data_closed(sb-s_fs_info);
ffs_data_put(sb-s_fs_info);
}
 }
@@ -1417,7 +1429,11 @@ static void ffs_data_opened(struct ffs_data *ffs)
ENTER();