Re: [PATCH v5 01/15] usb/gadget: f_mass_storage: create _fsg_common_free_buffers

[Michal Nazarewicz]

On Thu, Oct 03 2013, Andrzej Pietrasiewicz andrze...@samsung.com wrote:
 When configfs is in place, gadgets will have to be able to free
 fsg buffers. Add a helper function.

 Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

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

 ---
  drivers/usb/gadget/f_mass_storage.c |   24 +++-
  1 files changed, 15 insertions(+), 9 deletions(-)

 diff --git a/drivers/usb/gadget/f_mass_storage.c 
 b/drivers/usb/gadget/f_mass_storage.c
 index 32e5ab7..7d57542 100644
 --- a/drivers/usb/gadget/f_mass_storage.c
 +++ b/drivers/usb/gadget/f_mass_storage.c
 @@ -2642,6 +2642,18 @@ static inline int fsg_num_buffers_validate(unsigned 
 int fsg_num_buffers)
   return -EINVAL;
  }
  
 +static void _fsg_common_free_buffers(struct fsg_buffhd *buffhds, unsigned n)
 +{
 + if (buffhds) {
 + struct fsg_buffhd *bh = buffhds;
 + while (n--) {
 + kfree(bh-buf);
 + ++bh;
 + }
 + kfree(buffhds);
 + }
 +}
 +
  struct fsg_common *fsg_common_init(struct fsg_common *common,
  struct usb_composite_dev *cdev,
  struct fsg_config *cfg)
 @@ -2897,15 +2909,9 @@ static void fsg_common_release(struct kref *ref)
   kfree(common-luns);
   }
  
 - {
 - struct fsg_buffhd *bh = common-buffhds;
 - unsigned i = common-fsg_num_buffers;
 - do {
 - kfree(bh-buf);
 - } while (++bh, --i);
 - }
 -
 - kfree(common-buffhds);
 + if (likely(common-buffhds))

You're checking for that in the free function anyway, so just drop the
condition here.

 + _fsg_common_free_buffers(common-buffhds,
 +  common-fsg_num_buffers);
   if (common-free_storage_on_release)
   kfree(common);
  }

-- 
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] memory mapping for usbfs (v0.4)

[Ming Lei]

On Sat, Oct 5, 2013 at 4:34 AM, Alan Stern st...@rowland.harvard.edu wrote:
 On Fri, 4 Oct 2013, Markus Rechberger wrote:

 I was only testing reading the data so I didn't see any caching
 effects since I don't have a device or driver which I can send a lot
 data out.
 As far as I understand pgprot_noncached would only be required when
 sending data (writing data to the DMA'able buffer).

No, applciation still may read obsolete data from the mapped buffer even
after transfer from device is complete without any synchronization.


 This question is still open.

 The buffer should be cached.  The userspace program will have to make
 sure that it doesn't try to access the buffer while DMA is in progress.
 As long as that restriction is obeyed, the USB core will take care of
 mapping the buffer for DMA (which flushes the cache).

No, HCD mapping/unmapping can't deal with the problem since they
use the kernel direct-mapped virtual address of the buffer to flush
cache, but applications use the mapped virtual address, and CPU
can use both the two virtual addresse to cache data, so it is probable
that the transfer buffer can be cached in more than one locations by
CPU for VIVT or VIPT-alias cache.

So Markus takes the simplest way which uses nocahced mapping, but
it may degrade performance on some ARCHs since it is reported that it
is extremely slow to access non-cached memory on some ARMv7 SoCs.


  As I understand it, you wrote this in order to solve problems where
  memory couldn't be allocated for USB transfers.  If the system is
  running short on memory, won't your USBDEVFS_ALLOC_MEMORY ioctl also
  fail?
 

 No, the buffers will be kept in a list and re-used (aside of that they
 are also kept in a list in devio.c and re-used). When the buffers are
 allocated initially there's no chance to fail this process during
 run-time.

 Why not?  What if the memory is already almost full?  The call to
 alloc_pages_exact() could fail.

 I agree this is less likely than a failure with the current system, but
 it is still possible.

With CONFIG_COMPACTION enabled, it is hard to trigger the
allocation failure, and actually on some ARCHs, size of kernel stack or
page directory table for creating each process is 16KB.


Thanks,
-- 
Ming Lei
--
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 02/15] usb/gadget: f_mass_storage: make sysfs interface optional

[Michal Nazarewicz]

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 andrze...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

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

 ---
  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 +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--


signature.asc
Description: PGP signature

Re: [PATCH v5 03/15] usb/gadget: f_mass_storage: create fsg_common_setup for use in fsg_common_init

[Michal Nazarewicz]

On Thu, Oct 03 2013, Andrzej Pietrasiewicz wrote:
 fsg_common_init is a lengthy function. Factor a portion of it out.

 Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

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

 ---
  drivers/usb/gadget/f_mass_storage.c |   41 +-
  1 files changed, 25 insertions(+), 16 deletions(-)

 diff --git a/drivers/usb/gadget/f_mass_storage.c 
 b/drivers/usb/gadget/f_mass_storage.c
 index 14c9e5b..b7ed792 100644
 --- a/drivers/usb/gadget/f_mass_storage.c
 +++ b/drivers/usb/gadget/f_mass_storage.c
 @@ -2643,6 +2643,28 @@ static inline int fsg_num_buffers_validate(unsigned 
 int fsg_num_buffers)
   return -EINVAL;
  }
  
 +static struct fsg_common *fsg_common_setup(struct fsg_common *common, bool 
 zero)

I don't see the point of zero argument.

 +{
 + if (!common) {
 + common = kzalloc(sizeof(*common), GFP_KERNEL);
 + if (!common)
 + return ERR_PTR(-ENOMEM);
 + common-free_storage_on_release = 1;
 + } else {
 + if (zero)
 + memset(common, 0, sizeof(*common));
 + common-free_storage_on_release = 0;
 + }
 + init_rwsem(common-filesem);
 + spin_lock_init(common-lock);
 + kref_init(common-ref);
 + init_completion(common-thread_notifier);
 + init_waitqueue_head(common-fsg_wait);
 + common-state = FSG_STATE_TERMINATED;
 +
 + return common;
 +}
 +
  void fsg_common_set_sysfs(struct fsg_common *common, bool sysfs)
  {
   common-sysfs = sysfs;

-- 
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 v5 04/15] usb/gadget: f_mass_storage: create fsg_common_set_num_buffers for use in fsg_common_init

[Michal Nazarewicz]

On Thu, Oct 03 2013, Andrzej Pietrasiewicz wrote:
 fsg_common_init is a lengthy function. Factor a portion of it out.

 Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

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

 ---
  drivers/usb/gadget/f_mass_storage.c |   68 +-
  drivers/usb/gadget/f_mass_storage.h |2 +
  2 files changed, 44 insertions(+), 26 deletions(-)

 diff --git a/drivers/usb/gadget/f_mass_storage.c 
 b/drivers/usb/gadget/f_mass_storage.c
 index b7ed792..4833710 100644
 --- a/drivers/usb/gadget/f_mass_storage.c
 +++ b/drivers/usb/gadget/f_mass_storage.c
 @@ -2682,6 +2682,45 @@ static void _fsg_common_free_buffers(struct fsg_buffhd 
 *buffhds, unsigned n)
   }
  }
  
 +int fsg_common_set_num_buffers(struct fsg_common *common, unsigned int n)
 +{
 + struct fsg_buffhd *bh, *new_buffhds;
 + int i, rc;
 +
 + rc = fsg_num_buffers_validate(n);
 + if (rc != 0)
 + return rc;
 +
 + new_buffhds = kcalloc(n, sizeof *(new_buffhds), GFP_KERNEL);

What's with the parens around new_buffhds?  And why call it
“new_buffhds”?  What's wrong with “buffhds”?

 + if (!new_buffhds)
 + return -ENOMEM;
 +
 + /* Data buffers cyclic list */
 + bh = new_buffhds;
 + i = n;
 + goto buffhds_first_it;
 + do {
 + bh-next = bh + 1;
 + ++bh;
 +buffhds_first_it:
 + bh-buf = kmalloc(FSG_BUFLEN, GFP_KERNEL);
 + if (unlikely(!bh-buf))
 + goto error_release;
 + } while (--i);
 + bh-next = new_buffhds;
 +
 + _fsg_common_free_buffers(common-buffhds, common-fsg_num_buffers);
 + common-fsg_num_buffers = n;
 + common-buffhds = new_buffhds;
 +
 + return 0;
 +
 +error_release:
 + _fsg_common_free_buffers(new_buffhds, n - i);

Just pass n.  kfree() is a no-op for NULL, so _fsg_common_free_buffers()
should handle initialised buffhds pointers gracefully.

 +
 + return -ENOMEM;
 +}
 +
  static inline void fsg_common_remove_sysfs(struct fsg_lun *lun)
  {
   device_remove_file(lun-dev, dev_attr_nofua);

-- 
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 v5 05/15] usb/gadget: f_mass_storage: create lun handling helpers for use in fsg_common_init

[Michal Nazarewicz]

On Thu, Oct 03 2013, Andrzej Pietrasiewicz wrote:
 fsg_common_init is a lengthy function. Factor portions of it out.

 Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

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

 ---
  drivers/usb/gadget/f_mass_storage.c |   81 +++---
  drivers/usb/gadget/f_mass_storage.h |8 +++
  2 files changed, 72 insertions(+), 17 deletions(-)

 diff --git a/drivers/usb/gadget/f_mass_storage.c 
 b/drivers/usb/gadget/f_mass_storage.c
 index 4833710..16a44e1 100644
 --- a/drivers/usb/gadget/f_mass_storage.c
 +++ b/drivers/usb/gadget/f_mass_storage.c

 +void _fsg_common_remove_luns(struct fsg_common *common, int n)

Since this function is used only in this file and not exported, why not
mark it static?

 +{
 + int i;
 +
 + for (i = 0; i  n; ++i)
 + if (common-luns[i]) {
 + fsg_common_remove_lun(common-luns[i], common-sysfs);
 + common-luns[i] = NULL;
 + }
 +}


-- 
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 v5 06/15] usb/gadget: f_mass_storage: create fsg_common_set_ops/_private_data for use in fsg_common_init

[Michal Nazarewicz]

On Thu, Oct 03 2013, Andrzej Pietrasiewicz wrote:
 fsg_common_init is a lengthy function. Factor portions of it out.

 Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 ---
  drivers/usb/gadget/f_mass_storage.c |   16 ++--
  drivers/usb/gadget/f_mass_storage.h |5 +
  2 files changed, 19 insertions(+), 2 deletions(-)

 diff --git a/drivers/usb/gadget/f_mass_storage.c 
 b/drivers/usb/gadget/f_mass_storage.c
 index 16a44e1..39f7f1f 100644
 --- a/drivers/usb/gadget/f_mass_storage.c
 +++ b/drivers/usb/gadget/f_mass_storage.c
 @@ -2789,6 +2789,17 @@ int fsg_common_set_nluns(struct fsg_common *common, 
 int nluns)
   return 0;
  }
  
 +void fsg_common_set_ops(struct fsg_common *common,
 + const struct fsg_operations *ops)
 +{
 + common-ops = ops;
 +}
 +
 +void fsg_common_set_private_data(struct fsg_common *common, void *priv)
 +{
 + common-private_data = priv;
 +}
 +

This looks like over engineering to me to be honest.  What's wrong with
directly setting the fields in the structure?  At the very least, those
should be static inlines in the header file.

  #define MAX_LUN_NAME_LEN 80
  
  struct fsg_common *fsg_common_init(struct fsg_common *common,

-- 
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 v5 07/15] usb/gadget: f_mass_storage: create fsg_common_set_cdev for use in fsg_common_init

[Michal Nazarewicz]

On Thu, Oct 03 2013, Andrzej Pietrasiewicz wrote:
 fsg_common_init is a lengthy function. Factor a portion of it out.

 Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

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

 ---
  drivers/usb/gadget/f_mass_storage.c |   52 
 +--
  drivers/usb/gadget/f_mass_storage.h |3 ++
  2 files changed, 34 insertions(+), 21 deletions(-)

 diff --git a/drivers/usb/gadget/f_mass_storage.c 
 b/drivers/usb/gadget/f_mass_storage.c
 index 39f7f1f..61952b6 100644
 --- a/drivers/usb/gadget/f_mass_storage.c
 +++ b/drivers/usb/gadget/f_mass_storage.c
 @@ -2800,6 +2800,35 @@ void fsg_common_set_private_data(struct fsg_common 
 *common, void *priv)
   common-private_data = priv;
  }
  
 +int fsg_common_set_cdev(struct fsg_common *common,
 +  struct usb_composite_dev *cdev, bool can_stall)
 +{
 + struct usb_string *us;
 + int rc;

Drop rc variable.

 +
 + common-gadget = cdev-gadget;
 + common-ep0 = cdev-gadget-ep0;
 + common-ep0req = cdev-req;
 + common-cdev = cdev;
 +
 + us = usb_gstrings_attach(cdev, fsg_strings_array,
 +  ARRAY_SIZE(fsg_strings));
 + if (IS_ERR(us)) {
 + rc = PTR_ERR(us);
 + return rc;

return PTR_ERR(us);

 + }
 + fsg_intf_desc.iInterface = us[FSG_STRING_INTERFACE].id;
 +
 + /*
 +  * Some peripheral controllers are known not to be able to
 +  * halt bulk endpoints correctly.  If one of them is present,
 +  * disable stalls.
 +  */
 + common-can_stall = can_stall  !(gadget_is_at91(common-gadget));
 +
 + return 0;
 +}
 +
  #define MAX_LUN_NAME_LEN 80
  
  struct fsg_common *fsg_common_init(struct fsg_common *common,

-- 
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 v5 08/15] usb/gadget: f_mass_storage: create lun creation helpers for use in fsg_common_init

[Michal Nazarewicz]

On Thu, Oct 03 2013, Andrzej Pietrasiewicz wrote:
 fsg_common_init is a lengthy function. Factor portions of it out.

 Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

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

 ---
  drivers/usb/gadget/f_mass_storage.c |  260 
 ++-
  drivers/usb/gadget/f_mass_storage.h |6 +
  2 files changed, 169 insertions(+), 97 deletions(-)

 diff --git a/drivers/usb/gadget/f_mass_storage.c 
 b/drivers/usb/gadget/f_mass_storage.c
 index 61952b6..441bde5 100644
 --- a/drivers/usb/gadget/f_mass_storage.c
 +++ b/drivers/usb/gadget/f_mass_storage.c
 @@ -2829,18 +2829,172 @@ int fsg_common_set_cdev(struct fsg_common *common,
   return 0;
  }
  
 +static inline int fsg_common_add_sysfs(struct fsg_common *common,
 +struct fsg_lun *lun)
 +{
 + int rc;
 +
 + rc = device_register(lun-dev);
 + if (rc) {
 + put_device(lun-dev);
 + return rc;
 + }
 +
 + rc = device_create_file(lun-dev,
 + lun-cdrom
 +   ? dev_attr_ro_cdrom
 +   : dev_attr_ro);
 + if (rc)
 + goto error_cdrom;
 + rc = device_create_file(lun-dev,
 + lun-removable
 +   ? dev_attr_file
 +   : dev_attr_file_nonremovable);
 + if (rc)
 + goto error_removable;
 + rc = device_create_file(lun-dev, dev_attr_nofua);
 + if (rc)
 + goto error_nofua;
 +
 + return 0;
 +
 +error_nofua:
 + device_remove_file(lun-dev,
 +lun-removable
 +  ? dev_attr_file
 +  : dev_attr_file_nonremovable);
 +error_removable:
 + device_remove_file(lun-dev,
 +lun-cdrom
 +  ? dev_attr_ro_cdrom
 +  : dev_attr_ro);

Dose are no-ops if the file does not exist, right?  So why not just
unconditionally remove both files and then you con use the other
function you've introduced earlier to remove them.

 +error_cdrom:
 + device_unregister(lun-dev);
 + return rc;
 +}
 +
  #define MAX_LUN_NAME_LEN 80
  
 +int fsg_common_create_lun(struct fsg_common *common, struct fsg_lun_config 
 *cfg,
 +   unsigned int id, const char *name,
 +   const char **name_pfx)
 +{
 + struct fsg_lun *lun;
 + char *pathbuf;
 + int rc = -ENOMEM;
 + int name_len;
 +
 + if (!common-nluns || !common-luns)
 + return -ENODEV;
 +
 + if (common-luns[id])
 + return -EBUSY;
 +
 + name_len = strlen(name) + 1;
 + if (name_len  MAX_LUN_NAME_LEN)
 + return -ENAMETOOLONG;
 +
 + lun = kzalloc(sizeof(*lun), GFP_KERNEL);
 + if (!lun)
 + return -ENOMEM;
 +
 + lun-name = kstrndup(name, name_len, GFP_KERNEL);
 + if (!lun-name)
 + goto error_name;

Why do you care if the name is longer then MAX_LUN_NAME_LEN-1?
kstrdup() will allocate the space anyway, so why the limit at all?

And like discussed earlier, I believe that if common-sysfs, the copying
can be avoided anyway.

 + lun-name_pfx = name_pfx;
 +
 + lun-cdrom = !!cfg-cdrom;
 + lun-ro = cfg-cdrom || cfg-ro;
 + lun-initially_ro = lun-ro;
 + lun-removable = !!cfg-removable;
 +
 + common-luns[id] = lun;
 +
 + if (common-sysfs) {
 + lun-dev.release = fsg_lun_release;
 + lun-dev.parent = common-gadget-dev;
 + dev_set_drvdata(lun-dev, common-filesem);
 + dev_set_name(lun-dev, name);
 +
 + rc = fsg_common_add_sysfs(common, lun);
 + if (rc) {
 + pr_info(failed to register LUN%d: %d\n, id, rc);
 + goto error_sysfs;
 + }
 + }
 +
 + if (cfg-filename) {
 + rc = fsg_lun_open(lun, cfg-filename);
 + if (rc)
 + goto error_lun;
 + } else if (!lun-removable) {
 + pr_err(no file given for LUN%d\n, id);
 + rc = -EINVAL;
 + goto error_lun;
 + }

Actually, could !cfg-filename  !cfg-removable be checked somewhere
at the beginning of the function so that we don't waste our time
initialising stuff if we know we'll fail later on anyway?

 + pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
 + {
 + char *p = (no medium);

Just declare p alongside pathbuf at the beginning of the function and
drop the block.  It just adds indentation level for no reason.

 + if (fsg_lun_is_open(lun)) {
 + p = (error);
 + if (pathbuf) {
 + p = d_path(lun-filp-f_path,
 +pathbuf, PATH_MAX);
 + if (IS_ERR(p))
 + 

Re: [PATCH v5 09/15] usb/gadget: f_mass_storage: create fsg_common_set_inquiry_string for use in fsg_common_init

[Michal Nazarewicz]

On Thu, Oct 03 2013, Andrzej Pietrasiewicz wrote:
 fsg_common_init is a lengthy function. Factor a portion of it out.

 Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

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

 ---
  drivers/usb/gadget/f_mass_storage.c |   29 +++--
  drivers/usb/gadget/f_mass_storage.h |3 +++
  2 files changed, 22 insertions(+), 10 deletions(-)

-- 
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 v5 10/15] usb/gadget: f_mass_storage: create fsg_common_run_thread for use in fsg_common_init

[Michal Nazarewicz]

On Thu, Oct 03 2013, Andrzej Pietrasiewicz wrote:
 fsg_common_init is a lengthy function. Factor a portion of it out.

 Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

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

 ---
  drivers/usb/gadget/f_mass_storage.c |   32 +---
  drivers/usb/gadget/f_mass_storage.h |2 ++
  2 files changed, 23 insertions(+), 11 deletions(-)

-- 
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: EHCI bus glue driver works for storage, fails for a WiFi module

[Alan Stern]

On Sat, 5 Oct 2013, Arokux X wrote:

 Hi all,
 
 first of all thank you all for your help. I now have some news to
 report. Using your hint about timing I've inserted a bunch of udelays
 around the read/write functions that get called from _rtl92c_write_fw
 and got rid of the detected XactErr len 0/0 retry errors. Then I
 just tried to use the WLAN adapter and it worked. Afterwards, I've
 removed udelays and tried it again and guess what... everything still
 worked.
 
 I do not really know if I should consider these errors harmless but it
 looks as if they are. What do you think?

It's hard to tell.  But at any rate, it seems clear that your glue 
driver is working okay.

 I do however have something fishy to report, but this is probably an
 off-topic for linux-usb mailing list so I'll just mention it here
 briefly for completeness. Fist, I've compared the speed of the
 rtl8192cu vs 8192cu in 3.4 kernel. The latter one is the Realtek's
 driver. I've did many download tests of the same file and it turns out
 8192cu is 1.5 times faster. Things haven't improved since then. The
 3.12-rc1 rtl8192cu shows the same performance as in 3.4. In addition
 with 3.12-rc1 I get some strange messages in the dmesg [1].

You may want to bring this to the attention of the wireless 
maintainers.

Alan Stern

--
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 2/2] USB/host: Cleaning up the OK macro in uhci-hub.c

[Alan Stern]

On Fri, 4 Oct 2013, Deng-Cheng Zhu wrote:

 From: Deng-Cheng Zhu dengcheng@imgtec.com
 
 The logic len = (x) in OK(x) is dead. Clean it up.
 
 Reviewed-by: James Hogan james.ho...@imgtec.com
 Signed-off-by: Deng-Cheng Zhu dengcheng@imgtec.com

Getting rid of the OK macro is a good idea.  But the way you did it is 
wrong, because of a pre-existing bug in uhci_hub_control().  The 
function should return len, not retval.

Alan Stern

--
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 1/2] USB/host: Use existing macros instead of hard-coded values in uhci-debug.c

[Alan Stern]

On Fri, 4 Oct 2013, Deng-Cheng Zhu wrote:

 From: Deng-Cheng Zhu dengcheng@imgtec.com
 
 Now that UHCI IO registers have been defined in uhci-hcd.h, use them.
 
 Reviewed-by: James Hogan james.ho...@imgtec.com
 Signed-off-by: Deng-Cheng Zhu dengcheng@imgtec.com
 ---
  drivers/usb/host/uhci-debug.c |   16 
  1 files changed, 8 insertions(+), 8 deletions(-)
 
 diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c
 index 4557375..8e239cd 100644
 --- a/drivers/usb/host/uhci-debug.c
 +++ b/drivers/usb/host/uhci-debug.c
 @@ -310,14 +310,14 @@ static int uhci_show_status(struct uhci_hcd *uhci, char 
 *buf, int len)
   unsigned short portsc1, portsc2;
  
  
 - usbcmd= uhci_readw(uhci, 0);
 - usbstat   = uhci_readw(uhci, 2);
 - usbint= uhci_readw(uhci, 4);
 - usbfrnum  = uhci_readw(uhci, 6);
 - flbaseadd = uhci_readl(uhci, 8);
 - sof   = uhci_readb(uhci, 12);
 - portsc1   = uhci_readw(uhci, 16);
 - portsc2   = uhci_readw(uhci, 18);
 + usbcmd= uhci_readw(uhci, USBCMD);
 + usbstat   = uhci_readw(uhci, USBSTS);
 + usbint= uhci_readw(uhci, USBINTR);
 + usbfrnum  = uhci_readw(uhci, USBFRNUM);
 + flbaseadd = uhci_readl(uhci, USBFLBASEADD);
 + sof   = uhci_readb(uhci, USBSOF);
 + portsc1   = uhci_readw(uhci, USBPORTSC1);
 + portsc2   = uhci_readw(uhci, USBPORTSC2);
  
   out += sprintf(out,   usbcmd= %04x   %s%s%s%s%s%s%s%s\n,
   usbcmd,

Acked-by: Alan Stern st...@rowland.harvard.edu

--
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] memory mapping for usbfs (v0.4)

[Alan Stern]

On Sat, 5 Oct 2013, Ming Lei wrote:

  The buffer should be cached.  The userspace program will have to make
  sure that it doesn't try to access the buffer while DMA is in progress.
  As long as that restriction is obeyed, the USB core will take care of
  mapping the buffer for DMA (which flushes the cache).
 
 No, HCD mapping/unmapping can't deal with the problem since they
 use the kernel direct-mapped virtual address of the buffer to flush
 cache, but applications use the mapped virtual address, and CPU
 can use both the two virtual addresse to cache data, so it is probable
 that the transfer buffer can be cached in more than one locations by
 CPU for VIVT or VIPT-alias cache.
 
 So Markus takes the simplest way which uses nocahced mapping, but
 it may degrade performance on some ARCHs since it is reported that it
 is extremely slow to access non-cached memory on some ARMv7 SoCs.

Then how do you suggest the cache be flushed?

Alan Stern

--
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] memory mapping for usbfs (v0.4)

[Ming Lei]

On Sat, Oct 5, 2013 at 11:10 PM, Alan Stern st...@rowland.harvard.edu wrote:
 On Sat, 5 Oct 2013, Ming Lei wrote:

  The buffer should be cached.  The userspace program will have to make
  sure that it doesn't try to access the buffer while DMA is in progress.
  As long as that restriction is obeyed, the USB core will take care of
  mapping the buffer for DMA (which flushes the cache).

 No, HCD mapping/unmapping can't deal with the problem since they
 use the kernel direct-mapped virtual address of the buffer to flush
 cache, but applications use the mapped virtual address, and CPU
 can use both the two virtual addresse to cache data, so it is probable
 that the transfer buffer can be cached in more than one locations by
 CPU for VIVT or VIPT-alias cache.

 So Markus takes the simplest way which uses nocahced mapping, but
 it may degrade performance on some ARCHs since it is reported that it
 is extremely slow to access non-cached memory on some ARMv7 SoCs.

 Then how do you suggest the cache be flushed?

flush_cache_range() may be OK, and suggest to refer
to Documentation/cachetlb.txt first.


Thanks,
-- 
Ming Lei
--
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

[PATCH 1/5] drivers: usb: core: hcd: moved asterix to variable

[Matthias Beyer]

instead of type

Signed-off-by: Matthias Beyer m...@beyermatthias.de
---
 drivers/usb/core/hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index d6a8d23..9036794 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2818,7 +2818,7 @@ void usb_remove_hcd(struct usb_hcd *hcd)
 EXPORT_SYMBOL_GPL(usb_remove_hcd);
 
 void
-usb_hcd_platform_shutdown(struct platform_device* dev)
+usb_hcd_platform_shutdown(struct platform_device *dev)
 {
struct usb_hcd *hcd = platform_get_drvdata(dev);
 
-- 
1.8.4

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

[PATCH 3/5] drivers: usb: core: hcd: replaced C99 // comments

[Matthias Beyer]

Signed-off-by: Matthias Beyer m...@beyermatthias.de
---
 drivers/usb/core/hcd.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 0f3e5a0..3a2667c 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -428,7 +428,7 @@ rh_string(int id, struct usb_hcd const *hcd, u8 *data, 
unsigned len)
char const *s;
static char const langids[4] = {4, USB_DT_STRING, 0x09, 0x04};
 
-   // language ids
+   /* language ids */
switch (id) {
case 0:
/* Array of LANGID codes (0x0409 is MSFT-speak for en-us) */
@@ -615,7 +615,7 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb 
*urb)
case DeviceOutRequest | USB_REQ_SET_INTERFACE:
break;
case DeviceOutRequest | USB_REQ_SET_ADDRESS:
-   // wValue == urb-dev-devaddr
+   /* wValue == urb-dev-devaddr */
dev_dbg (hcd-self.controller, root hub device address %d\n,
wValue);
break;
@@ -625,7 +625,7 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb 
*urb)
/* ENDPOINT REQUESTS */
 
case EndpointRequest | USB_REQ_GET_STATUS:
-   // ENDPOINT_HALT flag
+   /* ENDPOINT_HALT flag */
tbuf[0] = 0;
tbuf[1] = 0;
len = 2;
@@ -683,7 +683,7 @@ error:
if (urb-transfer_buffer_length  len)
len = urb-transfer_buffer_length;
urb-actual_length = len;
-   // always USB_DIR_IN, toward host
+   /* always USB_DIR_IN, toward host */
memcpy (ubuf, bufp, len);
 
/* report whether RH hardware supports remote wakeup */
@@ -1134,7 +1134,7 @@ long usb_calc_bus_time (int speed, int is_input, int 
isoc, int bytecount)
return (9107L + BW_HOST_DELAY + tmp);
}
case USB_SPEED_HIGH:/* ISOC or INTR */
-   // FIXME adjust for input vs output
+   /* FIXME adjust for input vs output */
if (isoc)
tmp = HS_NSECS_ISO (bytecount);
else
-- 
1.8.4

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

[PATCH 0/5] Cleanup in drivers/usb/core/hcd.c

[Matthias Beyer]

Hi!

I did some cleanup in 

drivers/usb/core/hcd.c

of ERROR messages from scripts/checkpatch.pl --file. Not all ERRORs are fixed,
there are some remaining with assignment in if statement.

It's based on 7dee8df, which is Linus current master. I compiled it without
errors. Checkpatch script gives me some warnings, which are remaining warnings
of the file, I didn't introduce them!

Best regards!

Matthias Beyer (5):
  drivers: usb: core: hcd: moved asterix to variable
  drivers: usb: core: hcd: Whitespace fixes
  drivers: usb: core: hcd: replaced C99 // comments
  drivers: usb: core: hcd: removed braces for return statements
  drivers: usb: core: hcd: if-else-braces fixed

 drivers/usb/core/hcd.c | 80 +-
 1 file changed, 40 insertions(+), 40 deletions(-)

-- 
1.8.4

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

[PATCH 5/5] drivers: usb: core: hcd: if-else-braces fixed

[Matthias Beyer]

Put else keyword on same line as closing brace from if statement, added
{ } braces as the styleguide says.

Signed-off-by: Matthias Beyer m...@beyermatthias.de
---
 drivers/usb/core/hcd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 5d13d6c..bbd182a 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -879,9 +879,9 @@ static ssize_t authorized_default_store(struct device *dev,
if (result == 1) {
usb_hcd-authorized_default = val ? 1 : 0;
result = size;
-   }
-   else
+   } else {
result = -EINVAL;
+   }
return result;
 }
 static DEVICE_ATTR_RW(authorized_default);
-- 
1.8.4

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

[PATCH 2/5] drivers: usb: core: hcd: Whitespace fixes

[Matthias Beyer]

including
- spaces to tabs
- removing spaces before array indexing (foo [] to foo[])
- adding spaces around unary operator (foo? 1 : 0 to foo ? 1 : 0)
- removed trailing whitespace

Signed-off-by: Matthias Beyer m...@beyermatthias.de
---
 drivers/usb/core/hcd.c | 56 +-
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 9036794..0f3e5a0 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -6,7 +6,7 @@
  * (C) Copyright Deti Fliegl 1999
  * (C) Copyright Randy Dunlap 2000
  * (C) Copyright David Brownell 2000-2002
- * 
+ *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the
  * Free Software Foundation; either version 2 of the License, or (at your
@@ -93,7 +93,7 @@ EXPORT_SYMBOL_GPL (usb_bus_list);
 /* used when allocating bus numbers */
 #define USB_MAXBUS 64
 struct usb_busmap {
-   unsigned long busmap [USB_MAXBUS / (8*sizeof (unsigned long))];
+   unsigned long busmap[USB_MAXBUS / (8*sizeof (unsigned long))];
 };
 static struct usb_busmap busmap;
 
@@ -171,7 +171,7 @@ static const u8 usb25_rh_dev_descriptor[18] = {
 };
 
 /* usb 2.0 root hub device descriptor */
-static const u8 usb2_rh_dev_descriptor [18] = {
+static const u8 usb2_rh_dev_descriptor[18] = {
0x12,   /*  __u8  bLength; */
0x01,   /*  __u8  bDescriptorType; Device */
0x00, 0x02, /*  __le16 bcdUSB; v2.0 */
@@ -194,7 +194,7 @@ static const u8 usb2_rh_dev_descriptor [18] = {
 /* no usb 2.0 root hub device qualifier descriptor: one speed only */
 
 /* usb 1.1 root hub device descriptor */
-static const u8 usb11_rh_dev_descriptor [18] = {
+static const u8 usb11_rh_dev_descriptor[18] = {
0x12,   /*  __u8  bLength; */
0x01,   /*  __u8  bDescriptorType; Device */
0x10, 0x01, /*  __le16 bcdUSB; v1.1 */
@@ -219,7 +219,7 @@ static const u8 usb11_rh_dev_descriptor [18] = {
 
 /* Configuration descriptors for our root hubs */
 
-static const u8 fs_rh_config_descriptor [] = {
+static const u8 fs_rh_config_descriptor[] = {
 
/* one configuration */
0x09,   /*  __u8  bLength; */
@@ -228,13 +228,13 @@ static const u8 fs_rh_config_descriptor [] = {
0x01,   /*  __u8  bNumInterfaces; (1) */
0x01,   /*  __u8  bConfigurationValue; */
0x00,   /*  __u8  iConfiguration; */
-   0xc0,   /*  __u8  bmAttributes; 
+   0xc0,   /*  __u8  bmAttributes;
 Bit 7: must be set,
 6: Self-powered,
 5: Remote wakeup,
 4..0: resvd */
0x00,   /*  __u8  MaxPower; */
-  
+
/* USB 1.1:
 * USB 2.0, single TT organization (mandatory):
 *  one interface, protocol 0
@@ -256,17 +256,17 @@ static const u8 fs_rh_config_descriptor [] = {
0x00,   /*  __u8  if_bInterfaceSubClass; */
0x00,   /*  __u8  if_bInterfaceProtocol; [usb1.1 or single tt] */
0x00,   /*  __u8  if_iInterface; */
- 
+
/* one endpoint (status change endpoint) */
0x07,   /*  __u8  ep_bLength; */
0x05,   /*  __u8  ep_bDescriptorType; Endpoint */
0x81,   /*  __u8  ep_bEndpointAddress; IN Endpoint 1 */
-   0x03,   /*  __u8  ep_bmAttributes; Interrupt */
-   0x02, 0x00, /*  __le16 ep_wMaxPacketSize; 1 + (MAX_ROOT_PORTS / 8) */
+   0x03,   /*  __u8  ep_bmAttributes; Interrupt */
+   0x02, 0x00, /*  __le16 ep_wMaxPacketSize; 1 + (MAX_ROOT_PORTS / 8) */
0xff/*  __u8  ep_bInterval; (255ms -- usb 2.0 spec) */
 };
 
-static const u8 hs_rh_config_descriptor [] = {
+static const u8 hs_rh_config_descriptor[] = {
 
/* one configuration */
0x09,   /*  __u8  bLength; */
@@ -275,13 +275,13 @@ static const u8 hs_rh_config_descriptor [] = {
0x01,   /*  __u8  bNumInterfaces; (1) */
0x01,   /*  __u8  bConfigurationValue; */
0x00,   /*  __u8  iConfiguration; */
-   0xc0,   /*  __u8  bmAttributes; 
+   0xc0,   /*  __u8  bmAttributes;
 Bit 7: must be set,
 6: Self-powered,
 5: Remote wakeup,
 4..0: resvd */
0x00,   /*  __u8  MaxPower; */
-  
+
/* USB 1.1:
 * USB 2.0, single TT organization (mandatory):
 *  one interface, protocol 0
@@ -303,12 +303,12 @@ static const u8 hs_rh_config_descriptor [] = {
0x00,   /*  __u8  if_bInterfaceSubClass; */
0x00,   /*  __u8  if_bInterfaceProtocol; [usb1.1 or single tt] */
0x00,   /*  __u8  if_iInterface; */
- 
+
/* one endpoint (status 

Re: EHCI bus glue driver works for storage, fails for a WiFi module

[Larry Finger]

On 10/05/2013 10:01 AM, Alan Stern wrote:

On Sat, 5 Oct 2013, Arokux X wrote:


Hi all,

first of all thank you all for your help. I now have some news to
report. Using your hint about timing I've inserted a bunch of udelays
around the read/write functions that get called from _rtl92c_write_fw
and got rid of the detected XactErr len 0/0 retry errors. Then I
just tried to use the WLAN adapter and it worked. Afterwards, I've
removed udelays and tried it again and guess what... everything still
worked.

I do not really know if I should consider these errors harmless but it
looks as if they are. What do you think?


It's hard to tell.  But at any rate, it seems clear that your glue
driver is working okay.


I do however have something fishy to report, but this is probably an
off-topic for linux-usb mailing list so I'll just mention it here
briefly for completeness. Fist, I've compared the speed of the
rtl8192cu vs 8192cu in 3.4 kernel. The latter one is the Realtek's
driver. I've did many download tests of the same file and it turns out
8192cu is 1.5 times faster. Things haven't improved since then. The
3.12-rc1 rtl8192cu shows the same performance as in 3.4. In addition
with 3.12-rc1 I get some strange messages in the dmesg [1].


You may want to bring this to the attention of the wireless
maintainers.


Actually he has. From the logged messages, he must have loaded the driver with 
extra debugging enabled. (debug=2)


As for the performance, he appears to be complaining that the Linux version 
performs better than the vendor one. As I do not support the latter, there is 
nothing for me to do.


Larry


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

[GIT PATCH] USB fixes for 3.12-rc4

[Greg KH]

The following changes since commit 15c03dd4859ab16f9212238f29dd315654aa94f6:

  Linux 3.12-rc3 (2013-09-29 15:02:38 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ 
tags/usb-3.12-rc4

for you to fetch changes up to a214339d764a07b99dc0418685d6cc8a0a1970d5:

  usb: chipidea: add Intel Clovertrail pci id (2013-10-03 15:41:54 -0700)


USB fixes for 3.12-rc4

Here are 9 fixes for various USB driver problems.  The majority are
gadget/musb fixes, but there are some new device ids in here as well.

Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org


Bin Liu (2):
  usb: musb: fix otg default state
  usb: musb: gadget: fix otg active status flag

David Cohen (1):
  usb: chipidea: add Intel Clovertrail pci id

Greg Kroah-Hartman (1):
  Merge tag 'fixes-for-v3.12-rc4' of git://git.kernel.org/.../balbi/usb 
into usb-linus

Johan Hovold (2):
  usb: gadget: pxa25x_udc: fix deferred probe from __init
  usb: phy: gpio-vbus: fix deferred probe from __init

Michal Malý (1):
  USB: serial: option: Ignore card reader interface on Huawei E1750

Robert Baldyga (2):
  usb: gadget: f_fs: fix error handling
  usb: gadget: s3c-hsotg: fix can_write limit for non-periodic endpoints

Sebastian Andrzej Siewior (1):
  usb: musb: dsps: do not bind to musb-hdrc

 drivers/usb/chipidea/ci_hdrc_pci.c  |  7 ++-
 drivers/usb/gadget/f_fs.c   |  2 ++
 drivers/usb/gadget/pxa25x_udc.c |  9 +
 drivers/usb/gadget/s3c-hsotg.c  |  2 +-
 drivers/usb/musb/musb_dsps.c|  3 +++
 drivers/usb/musb/musb_gadget.c  |  5 -
 drivers/usb/phy/phy-gpio-vbus-usb.c | 11 +--
 drivers/usb/serial/option.c |  3 +++
 8 files changed, 29 insertions(+), 13 deletions(-)
--
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 2/5] drivers: usb: core: hcd: Whitespace fixes

[Joe Perches]

On Sat, 2013-10-05 at 18:02 +0200, Matthias Beyer wrote:
 - removing spaces before array indexing (foo [] to foo[])
[]
 diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
[]
 @@ -93,7 +93,7 @@ EXPORT_SYMBOL_GPL (usb_bus_list);
  /* used when allocating bus numbers */
  #define USB_MAXBUS   64
  struct usb_busmap {
 - unsigned long busmap [USB_MAXBUS / (8*sizeof (unsigned long))];
 + unsigned long busmap[USB_MAXBUS / (8*sizeof (unsigned long))];

this would be nicer as

DEFINE_BITMAP(busmap, USB_MAXBUS);


--
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 2/5] drivers: usb: core: hcd: Whitespace fixes

[Joe Perches]

On Sat, 2013-10-05 at 11:04 -0700, Joe Perches wrote:
 On Sat, 2013-10-05 at 18:02 +0200, Matthias Beyer wrote:
  - removing spaces before array indexing (foo [] to foo[])
 []
  diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
 []
  @@ -93,7 +93,7 @@ EXPORT_SYMBOL_GPL (usb_bus_list);
   /* used when allocating bus numbers */
   #define USB_MAXBUS 64
   struct usb_busmap {
  -   unsigned long busmap [USB_MAXBUS / (8*sizeof (unsigned long))];
  +   unsigned long busmap[USB_MAXBUS / (8*sizeof (unsigned long))];
 
 this would be nicer as
 
   DEFINE_BITMAP(busmap, USB_MAXBUS);
 

nack, DECLARE_BITMAP(...)


--
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: EHCI bus glue driver works for storage, fails for a WiFi module

[Alan Stern]

On Sat, 5 Oct 2013, Larry Finger wrote:

  I do however have something fishy to report, but this is probably an
  off-topic for linux-usb mailing list so I'll just mention it here
  briefly for completeness. Fist, I've compared the speed of the
  rtl8192cu vs 8192cu in 3.4 kernel. The latter one is the Realtek's
  driver. I've did many download tests of the same file and it turns out
  8192cu is 1.5 times faster. Things haven't improved since then. The
  3.12-rc1 rtl8192cu shows the same performance as in 3.4. In addition
  with 3.12-rc1 I get some strange messages in the dmesg [1].
 
  You may want to bring this to the attention of the wireless
  maintainers.
 
 Actually he has. From the logged messages, he must have loaded the driver 
 with 
 extra debugging enabled. (debug=2)
 
 As for the performance, he appears to be complaining that the Linux version 
 performs better than the vendor one.

The other way around.  rtl8192cu is the driver in the kernel, and
8192cu is RealTek's driver.  Arokux said that 8192cu is faster.  (If
you carefully parse what he wrote, you'll see it says that he did the
comparison using the 3.4 kernel, not that 8192cu is in the 3.4 kernel.)

Alan Stern

--
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: EHCI bus glue driver works for storage, fails for a WiFi module

[Larry Finger]

On 10/05/2013 02:52 PM, Alan Stern wrote:

On Sat, 5 Oct 2013, Larry Finger wrote:


I do however have something fishy to report, but this is probably an
off-topic for linux-usb mailing list so I'll just mention it here
briefly for completeness. Fist, I've compared the speed of the
rtl8192cu vs 8192cu in 3.4 kernel. The latter one is the Realtek's
driver. I've did many download tests of the same file and it turns out
8192cu is 1.5 times faster. Things haven't improved since then. The
3.12-rc1 rtl8192cu shows the same performance as in 3.4. In addition
with 3.12-rc1 I get some strange messages in the dmesg [1].


You may want to bring this to the attention of the wireless
maintainers.


Actually he has. From the logged messages, he must have loaded the driver with
extra debugging enabled. (debug=2)

As for the performance, he appears to be complaining that the Linux version
performs better than the vendor one.


The other way around.  rtl8192cu is the driver in the kernel, and
8192cu is RealTek's driver.  Arokux said that 8192cu is faster.  (If
you carefully parse what he wrote, you'll see it says that he did the
comparison using the 3.4 kernel, not that 8192cu is in the 3.4 kernel.)


Yes, I did misinterpret that. In fact, I have done a lot of work recently in 
improving the automatic gain setting code in rtl8192cu, but none of that is in 
the kernel yet.


Larry


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

[PATCH 1/1] drivers: usb: core: hcd.c: converted busmap from struct to bitmap

[Matthias Beyer]

The DECLARE_BITMAP macro should be used for declaring this bitmap.
This commit converts the busmap from a struct to a simple (static)
bitmap, using the DECLARE_BITMAP macro from linux/types.h.

Please review, as I'm new to kernel development, I don't know if this
has any hidden side effects!

Suggested by j...@perches.com

Signed-off-by: Matthias Beyer m...@beyermatthias.de
---
 drivers/usb/core/hcd.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index bbd182a..e9e1b33 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -40,6 +40,7 @@
 #include linux/platform_device.h
 #include linux/workqueue.h
 #include linux/pm_runtime.h
+#include linux/types.h
 
 #include linux/usb.h
 #include linux/usb/hcd.h
@@ -92,10 +93,7 @@ EXPORT_SYMBOL_GPL (usb_bus_list);
 
 /* used when allocating bus numbers */
 #define USB_MAXBUS 64
-struct usb_busmap {
-   unsigned long busmap[USB_MAXBUS / (8*sizeof (unsigned long))];
-};
-static struct usb_busmap busmap;
+static DECLARE_BITMAP(busmap, USB_MAXBUS);
 
 /* used when updating list of hcds */
 DEFINE_MUTEX(usb_bus_list_lock);   /* exported only for usbfs */
@@ -941,12 +939,12 @@ static int usb_register_bus(struct usb_bus *bus)
int busnum;
 
mutex_lock(usb_bus_list_lock);
-   busnum = find_next_zero_bit (busmap.busmap, USB_MAXBUS, 1);
+   busnum = find_next_zero_bit(busmap, USB_MAXBUS, 1);
if (busnum = USB_MAXBUS) {
printk (KERN_ERR %s: too many buses\n, usbcore_name);
goto error_find_busnum;
}
-   set_bit (busnum, busmap.busmap);
+   set_bit(busnum, busmap);
bus-busnum = busnum;
 
/* Add it to the local list of buses */
@@ -987,7 +985,7 @@ static void usb_deregister_bus (struct usb_bus *bus)
 
usb_notify_remove_bus(bus);
 
-   clear_bit (bus-busnum, busmap.busmap);
+   clear_bit(bus-busnum, busmap);
 }
 
 /**
-- 
1.8.4

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

[patch] USB: cyberjack: fix buggy integer overflow test

[Dan Carpenter]

old_rdtodo and size are short type.  They are type promoted to int
and the condition is never true.

Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

diff --git a/drivers/usb/serial/cyberjack.c b/drivers/usb/serial/cyberjack.c
index 7814262..6e1b69d 100644
--- a/drivers/usb/serial/cyberjack.c
+++ b/drivers/usb/serial/cyberjack.c
@@ -279,7 +279,7 @@ static void cyberjack_read_int_callback(struct urb *urb)
 
old_rdtodo = priv-rdtodo;
 
-   if (old_rdtodo + size  old_rdtodo) {
+   if (old_rdtodo  SHRT_MAX - size) {
dev_dbg(dev, To many bulk_in urbs to do.\n);
spin_unlock(priv-lock);
goto resubmit;
--
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