Re: [PATCH v6] can: kvaser_usb: Add support for Kvaser CAN/USB devices

2012-11-23 Thread Marc Kleine-Budde
On 11/22/2012 10:30 PM, Greg KH wrote:
> On Thu, Nov 22, 2012 at 04:01:49PM +0100, Olivier Sobrie wrote:
>> Hi linux-usb folks,
>>
>> Is there someone who can help me to fix the following errors?
>>
>> smatch warnings:
>>
>> + drivers/net/can/usb/kvaser_usb.c:431 kvaser_usb_send_simple_msg() error: 
>> doing
>> +dma on the stack ((null))
>> + drivers/net/can/usb/kvaser_usb.c:1073 kvaser_usb_set_opt_mode() error: 
>> doing
>> +dma on the stack ((null))
>> + drivers/net/can/usb/kvaser_usb.c:1174 kvaser_usb_flush_queue() error: doing
>> +dma on the stack ((null))
>> + drivers/net/can/usb/kvaser_usb.c:1384 kvaser_usb_set_bittiming() error: 
>> doing
>> +dma on the stack ((null))
>>
>> I assume it's due to the buffer I pass to the function usb_bulk_msg()
>> which is on the stack and can't be.
>> Do I just have to kmalloc a buffer and give it to the usb_bulk_msg()
>> function? That's what I understood by reading
>> "Documentation/DMA-API-HOWTO.txt" section "What memory is DMA'able?"...
>> and from commit
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=32ec4576c3fb37316b1d11a04b220527822f3f0d
> 
> Yes, that is all that is needed.

Thanks Greg. Olivier, you can post an incremental patch, I'll squash it
before sending the patches upstream.

regards,
Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature


Re: [PATCH v3 3/7] usb: chipidea: usbmisc: fix a potential race condition

2012-11-23 Thread Alexander Shishkin
Sascha Hauer  writes:

> On Fri, Nov 23, 2012 at 01:36:36PM +0800, Peter Chen wrote:
>> On Wed, Nov 21, 2012 at 03:06:29PM +0100, Michael Grzeschik wrote:
>> > From: Marc Kleine-Budde 
>> > 
>> > This fixes a potential race condition where the ci13xxx_imx glue code
>> > could be fast enough to call one of the usbmisc_ops before he got a
>> > valid value on the static usbmisc pointer. To fix that we first set
>> > usbmisc, then call usbmisc_set_ops().
>> 
>> usbmisc is subsys_initcall, and cil13xxx_imx is module_init. Any
>> potential situation that the ci13xxx_imx's probe is ran before the
>> usbmisc's probe is completed?
>
> Not having looked at the code you are referring to at all I just want
> to say that: drivers can be modules (don't know if that's true for
> chipidea) and sooner or later we'll probably get devicetree overlays,

ChipIdea can be not even one, but two modules (ci_hdrc, the actual
controller driver, always a platform_driver) and platform bindings like
ci13xxx_imx, ci13xxx_pci, ci13xxx_msm, which can be platform or pci or
whatever else drivers.

> so the devicetree nodes might just appear during runtime. Depending on
> initcall order is generally not a good idea.

That's very true.

Regards,
--
Alex
--
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: [RFCv4 PATCH 00/13] Configfs integration

2012-11-23 Thread Andrzej Pietrasiewicz
On Thursday, November 22, 2012 3:37 PM Sebastian Andrzej Siewior wrote:
> * Andrzej Pietrasiewicz | 2012-11-22 13:06:54 [+0100]:
> 
> >My intention is not to add new gadgets, but to promote the use of
> >usb function modules, e.g. f_mass_storage.ko.
> finally.
> 
> >I don't use the usbf_option stuff, though, for the reasons I would
> >like to state below.
> Good. In the meantime we decided to drop usbf_option.
> 
> >The mass storage function requires a hierarchy of subdirectories,
> >not just one directory in configfs. With usbf_opt there is no
> Yes, we will have native configfs interface. The tcm gadget uses
> configfs already for its lun setup.
> 
> >During its lifetime, the mass storage can require creating
> >lunX directories for its luns. And again, with usbf_option
> You have first to detach the gadget because you can't update while it is
> in progress connected / working. If you add an additional LUN then there
> is no way to notify the host side about this.
> 
> >All in all, I think it is better not to force complete separation
> >of functions from their configfs parts. Instead, I propose
> >to extend the struct usb_function with 2 operations and extend
> >the functions with a configfs part dedicated to them.
> Yes, good.
> 
> >I also show how to write an adapter module, which provides
> >the old sysfs-based interface to mass storage with module parameters,
> >but internally operates on configfs.
> There is no sysfs interface, is there? We have only modprobe interface.
> 
> >The patches 03 throuth 12 are not squashed in order to show the
> >steps involved in porting mass storage to the new framework,
> >but, ATTENTION, this means that if only some of them are applied,
> >the code may or may not compile. The patches may need checkpach.
> k.
> 

@Sebastian, @Michał: Thank you for your reviews. Please see the
responses to particular posts which will appear as I go through
your comments.

Andrzej



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


[RFC PATCH 0/2] staging: usbip: refine the spinlock

2012-11-23 Thread Harvey Yang
This patchset refine some spinlocks which I think not used properly. Maybe my 
'refine' make the code  broken, so any comments will be appreciated. 

patch1: The function 'usbip_event_add()' may be called in interrupt context on 
the stub side: 
'stub_complete'->'stub_enqueue_ret_unlink'->'usbip_event_add'.
In this function it tries to get the lock 'ud->lock', so we shoud disable irq 
when we get this lock in process context.

patch2: On the client side, we have a virtual hcd driver so there actually no 
hardware interrupts. Maybe to achieve a good performance there is no need to 
use the interrupt safe spinlock. Just replace them with a non interrupt safe 
version.


Harvey Yang (2):
  staging: usbip: use interrupt safe spinlock to avoid potential
deadlock.
  staging: usbip: replace the interrupt safe spinlock because no
hardware interrupt exists.

 drivers/staging/usbip/stub_dev.c|   34 
 drivers/staging/usbip/stub_rx.c |4 +-
 drivers/staging/usbip/usbip_event.c |6 ++-
 drivers/staging/usbip/vhci_hcd.c|   75 +++
 drivers/staging/usbip/vhci_rx.c |   10 ++---
 drivers/staging/usbip/vhci_tx.c |   14 +++
 6 files changed, 65 insertions(+), 78 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: [RFCv4 PATCH 00/13] Configfs integration

2012-11-23 Thread Sebastian Andrzej Siewior

On 11/22/2012 09:09 PM, Michal Nazarewicz wrote:

* Andrzej Pietrasiewicz | 2012-11-22 13:06:54 [+0100]:

During its lifetime, the mass storage can require creating
lunX directories for its luns. And again, with usbf_option


On Thu, Nov 22 2012, Sebastian Andrzej Siewior wrote:

You have first to detach the gadget because you can't update while it
is in progress connected / working. If you add an additional LUN then
there is no way to notify the host side about this.


I believe the point was that this happens during configuration.  The way
I see it is that user writes number of luns it wants the gadget to
have into a configfs file at which point the function creates lun#
directories so that each lun can be configured separately.


That is okay then. On tcm you simply mkdir lun_directory.


I also show how to write an adapter module, which provides
the old sysfs-based interface to mass storage with module parameters,
but internally operates on configfs.



There is no sysfs interface, is there? We have only modprobe
interface.


f_mass_storage creates a device and thus a sysfs entry for each LUN.
This, among other things, allows setting up of the backing file if the
LUN is removable.


I see. But there is no way to notify the host that you removed the disk
or inserted it back, right?

Sebastian
--
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: [RFCv4 PATCH 02/13] usb: gadget: Add USB Functions Gadget

2012-11-23 Thread Sebastian Andrzej Siewior

On 11/22/2012 09:25 PM, Michal Nazarewicz wrote:

echo 1>  /cfg/usb-function-gadget/G1/ready


On Thu, Nov 22 2012, Sebastian Andrzej Siewior wrote:

One thing you miss: Lets say you have C1 and C2. How do you configure
the same F1 in C1 and C2 _and_ how do you configure a different F1 in
F1 and F2 in C2. I guess the latter will work just now but the former
example won't. The former example is used by the nokia gadget, the
latter by g_serial.


Yeah.  I would propose having a separate directory for functions, ie.:

cd /cfg/usb-function-gadget/gadget1
mkdir functions/mass_storage

would load the function and allocate usb_function structure for it (or
whatever) as well as create all the interesting attributes inside the
directory, and later on:

mkdir configs/config0
mkdir configs/config0/func0
ln functions/mass_storage configs/config0/func0/function

or something similar.


Cute from the interface point of view. So we need one struct
usb_function for each gadget and something which holds the private of
the function pointing to the disk_data/netdev/uart_port/…

Sebastian
--
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: [RFCv4 PATCH 08/13] usb: gadget: example port of mass storage to UFG: f_mass_storage: remove unused operations

2012-11-23 Thread Sebastian Andrzej Siewior

On 11/22/2012 09:30 PM, Michal Nazarewicz wrote:

On Thu, Nov 22 2012, Andrzej Pietrasiewicz wrote:

pre_eject and post_eject are not used by anyone. Removing them.

Signed-off-by: Andrzej Pietrasiewicz
Signed-off-by: Kyungmin Park


Acked-by: Michal Nazarewicz

As far as I'm concerned, this can get merged regardless of the other
patches.


Sounds nice. A re-post without the RFC and the series so it can be
picked up?

Sebastian
--
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 v3 3/7] usb: chipidea: usbmisc: fix a potential race condition

2012-11-23 Thread Marc Kleine-Budde
On 11/23/2012 06:36 AM, Peter Chen wrote:
> On Wed, Nov 21, 2012 at 03:06:29PM +0100, Michael Grzeschik wrote:
>> From: Marc Kleine-Budde 
>>
>> This fixes a potential race condition where the ci13xxx_imx glue code
>> could be fast enough to call one of the usbmisc_ops before he got a
>> valid value on the static usbmisc pointer. To fix that we first set
>> usbmisc, then call usbmisc_set_ops().
> 
> usbmisc is subsys_initcall, and cil13xxx_imx is module_init. Any
> potential situation that the ci13xxx_imx's probe is ran before the
> usbmisc's probe is completed? Besides, there is usbmisc_ops value
> check at the beginning of cil13xxx_imx's probe.

It's bad practice to rely your code on some external ordering mechanism,
even more if the correct solution is so simple. And as Sascha pointed
out everything might be build as modules and loaded individually.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature


Re: [PATCH v3 6/7] usb: chipidea: usbmisc: add mx53 support

2012-11-23 Thread Marc Kleine-Budde
On 11/23/2012 07:53 AM, Peter Chen wrote:
> On Wed, Nov 21, 2012 at 03:06:32PM +0100, Michael Grzeschik wrote:
>> This adds mx53 as the next user of the usbmisc driver and makes it
>> possible to disable the overcurrent-detection of the internal phy.
>>
>> Signed-off-by: Michael Grzeschik 
>> Signed-off-by: Marc Kleine-Budde 
>> ---
>> Changes since v2:
>> * added defines for register offsets and bitmasks
>> * fixed disable_oc option for all ports
>>
>>  drivers/usb/chipidea/usbmisc_imx.c |   51 
>> 
>>  1 file changed, 51 insertions(+)
>>
>> diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
>> b/drivers/usb/chipidea/usbmisc_imx.c
>> index e090e0f..b1811df 100644
>> --- a/drivers/usb/chipidea/usbmisc_imx.c
>> +++ b/drivers/usb/chipidea/usbmisc_imx.c
>> @@ -19,6 +19,13 @@
>>  
>>  #define USB_DEV_MAX 4
>>  
>> +#define MX53_USB_OTG_PHY_CTRL_0_OFFSET  0x08;
>> +#define MX53_USB_UH2_CTRL_OFFSET0x14;
>> +#define MX53_USB_UH3_CTRL_OFFSET0x18;
>> +#define MX53_BM_OVER_CUR_DIS_H1 BIT(5)
>> +#define MX53_BM_OVER_CUR_DIS_OTGBIT(8)
>> +#define MX53_BM_OVER_CUR_DIS_UHxBIT(30)
>> +
>>  #define MX6_BM_OVER_CUR_DIS BIT(7)
> 
> As every SoC platform's non-core register mapping is different,
> it is better to use .h to put register bit definition

AFAIK in the kernel we don't make extra .h files any more, if there is
only a single user of the contents.

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature


RE: [RFCv4 PATCH 01/13] usb: composite: add make_group and add_function operations

2012-11-23 Thread Andrzej Pietrasiewicz
On Thursday, November 22, 2012 3:55 PM Sebastian Andrzej Siewior wrote:
> * Andrzej Pietrasiewicz | 2012-11-22 13:06:55 [+0100]:
> 



> >diff --git a/include/linux/usb/composite.h
> b/include/linux/usb/composite.h
> >index e84c754..5bac1f8 100644
> >--- a/include/linux/usb/composite.h
> >+++ b/include/linux/usb/composite.h
> >@@ -39,6 +39,7 @@
> > #include 
> > #include 
> > #include 
> >+#include 
> >
> > /*
> >  * USB function drivers should return USB_GADGET_DELAYED_STATUS if they
> >@@ -146,6 +147,11 @@ struct usb_function {
> >  * we can't restructure things to avoid mismatching.
> >  * Related:  unbind() may kfree() but bind() won't...
> >  */
> >+
> >+struct config_group *(*make_group)(struct config_group *parent,
> >+   const char *name);
> >+int (*add_function)(struct usb_configuration *c, struct usb_function
> *f,
> >+struct config_item *item, void *data);
> 
> a wrapper? I though you pass the configfs root node and the function
> will add all files / subdirectories. Why do we have two callbacks?
> 

make_group explanation:
In configfs the directory needs to know how to create its subdirectories.
In order to do it there are make_group and make_item methods in
struct configfs_group_operations (make_group and make_item are mutually
exclusive, i.e. only one of them can be specified). In the following
UFG implementation the function-dependent directories subtree is
rooted at some configfs directory which is of a generic type. So there
needs to be a way to pass this function-specific method to the generic
code. make_group here serves this purpose. It is used during gadget
setup, that is, when the configfs directories are created but prior
to actual binding of the gadget. Since creating the directories
does not equal gadget binding, it needs to be a separate function.

add_function explanation:
In order to use your framework for registering the usb functions,
the usb_get_function should be called from the generic code (otherwise
it leads to a situation where one needs to have a function's module
loaded in order to request this module... not good :O ). Once the
generic code has the function, it can call usb_add_funciton.
But some functions (as is the case with mass storage)
seem to need some additional function-specific activities together
with usb_add_function. So I decided to collect them in one
add_function operation with the semantics: do usb_add_function
plus any necessary function-specific things related to adding
this function.

Andrzej




--
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: [RFCv4 PATCH 01/13] usb: composite: add make_group and add_function operations

2012-11-23 Thread Sebastian Andrzej Siewior

On 11/22/2012 09:48 PM, Michal Nazarewicz wrote:

I think neither is correct.  The reviewed-by tag implies that the person
did a careful review of the code as per “Reviewer's statement of
oversight” (see Documentation/SubmittingPatches).

What actually happens is Kyungmin giving a green light to shipping the
patch from copyright stand-point since Samsung is copyright holder and
Andrzej has no power to say weather he can or cannot release the code.

So logical path the code took was:

Andrzej ->  Kyungmin ->  Andrzej ->  linux-usb


Aha. So is Kyungmin a lawyer and not a hacker as I assumed in the first
place.


If you look at other patches coming from SPRC (including mine while
I was working for Samsung) they all have the same Signed-off schema
where the first line is of the author and second is of Kyungmin.


This together with the statement above explains a lot to me. I always
saw that and wondered how much code he can write. I assumed that
Kyungmin was some kind of kick-ass hacker that knows all the chips
very well and therefore writes all of the Samsung code ahead of HW and
then is too busy with other stuff and so other people in his team push
his patches mainline and deal with the review.
I know that other companies work like that, where a small group of
people does the bring-up and then others take their code and try to
merge upstream. And this impressed me because Kyungmin is a person and
not a small group.

Anyway.
Signed-off indicates that he was involved in code development but he
was not. As it seems it me, his OKAY is very important why not add him
as

   Acked-By: ... [copyright]

I added the [copyright] as the subsystem since he did Ack only a part
of the patch, not the functionality etc. I know that (now) but others
might not.

Sebastian
--
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 v9 1/2] usb: phy: samsung: Introducing usb phy driver for hsotg

2012-11-23 Thread Praveen Paneri
This driver uses usb_phy interface to interact with s3c-hsotg. Supports
phy_init and phy_shutdown functions to enable/disable usb phy. Support
will be extended to host controllers and more Samsung SoCs.

Signed-off-by: Praveen Paneri 
Acked-by: Heiko Stuebner 
Acked-by: Kyungmin Park 
---
 .../devicetree/bindings/usb/samsung-usbphy.txt |   11 +
 drivers/usb/phy/Kconfig|8 +
 drivers/usb/phy/Makefile   |1 +
 drivers/usb/phy/samsung-usbphy.c   |  354 
 include/linux/platform_data/samsung-usbphy.h   |   27 ++
 5 files changed, 401 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/samsung-usbphy.txt
 create mode 100644 drivers/usb/phy/samsung-usbphy.c
 create mode 100644 include/linux/platform_data/samsung-usbphy.h

diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt 
b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
new file mode 100644
index 000..7b26e2d
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
@@ -0,0 +1,11 @@
+* Samsung's usb phy transceiver
+
+The Samsung's phy transceiver is used for controlling usb otg phy for
+s3c-hsotg usb device controller.
+TODO: Adding the PHY binding with controller(s) according to the under
+developement generic PHY driver.
+
+Required properties:
+- compatible : should be "samsung,exynos4210-usbphy"
+- reg : base physical address of the phy registers and length of memory mapped
+   region.
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 7eb73c5..17ad743 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -44,3 +44,11 @@ config USB_RCAR_PHY
 
  To compile this driver as a module, choose M here: the
  module will be called rcar-phy.
+
+config SAMSUNG_USBPHY
+   bool "Samsung USB PHY controller Driver"
+   depends on USB_S3C_HSOTG
+   select USB_OTG_UTILS
+   help
+ Enable this to support Samsung USB phy controller for samsung
+ SoCs.
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 1a579a8..ec304f6 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_USB_ISP1301)   += isp1301.o
 obj-$(CONFIG_MV_U3D_PHY)   += mv_u3d_phy.o
 obj-$(CONFIG_USB_EHCI_TEGRA)   += tegra_usb_phy.o
 obj-$(CONFIG_USB_RCAR_PHY) += rcar-phy.o
+obj-$(CONFIG_SAMSUNG_USBPHY)   += samsung-usbphy.o
diff --git a/drivers/usb/phy/samsung-usbphy.c b/drivers/usb/phy/samsung-usbphy.c
new file mode 100644
index 000..5c5e1bb
--- /dev/null
+++ b/drivers/usb/phy/samsung-usbphy.c
@@ -0,0 +1,354 @@
+/* linux/drivers/usb/phy/samsung-usbphy.c
+ *
+ * Copyright (c) 2012 Samsung Electronics Co., Ltd.
+ *  http://www.samsung.com
+ *
+ * Author: Praveen Paneri 
+ *
+ * Samsung USB2.0 High-speed OTG transceiver, talks to S3C HS OTG controller
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Register definitions */
+
+#define SAMSUNG_PHYPWR (0x00)
+
+#define PHYPWR_NORMAL_MASK (0x19 << 0)
+#define PHYPWR_OTG_DISABLE (0x1 << 4)
+#define PHYPWR_ANALOG_POWERDOWN(0x1 << 3)
+#define PHYPWR_FORCE_SUSPEND   (0x1 << 1)
+/* For Exynos4 */
+#define PHYPWR_NORMAL_MASK_PHY0(0x39 << 0)
+#define PHYPWR_SLEEP_PHY0  (0x1 << 5)
+
+#define SAMSUNG_PHYCLK (0x04)
+
+#define PHYCLK_MODE_USB11  (0x1 << 6)
+#define PHYCLK_EXT_OSC (0x1 << 5)
+#define PHYCLK_COMMON_ON_N (0x1 << 4)
+#define PHYCLK_ID_PULL (0x1 << 2)
+#define PHYCLK_CLKSEL_MASK (0x3 << 0)
+#define PHYCLK_CLKSEL_48M  (0x0 << 0)
+#define PHYCLK_CLKSEL_12M  (0x2 << 0)
+#define PHYCLK_CLKSEL_24M  (0x3 << 0)
+
+#define SAMSUNG_RSTCON (0x08)
+
+#define RSTCON_PHYLINK_SWRST   (0x1 << 2)
+#define RSTCON_HLINK_SWRST (0x1 << 1)
+#define RSTCON_SWRST   (0x1 << 0)
+
+#ifndef MHZ
+#define MHZ (1000*1000)
+#endif
+
+enum samsung_cpu_type {
+   TYPE_S3C64XX,
+   TYPE_EXYNOS4210,
+};
+
+/*
+ * struct samsung_usbphy - transceiver driver state
+ * @phy: transceiver struc

Re: [PATCH 16/16] ARM: OMAP: omap4panda: Power down the USB PHY and ETH when not in use

2012-11-23 Thread Roger Quadros
On 11/22/2012 06:12 PM, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Nov 22, 2012 at 05:00:45PM +0200, Roger Quadros wrote:
>> On 11/22/2012 03:56 PM, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Thu, Nov 22, 2012 at 09:49:05PM +0800, Andy Green wrote:
> Again it sounds like something that should be done at the hub driver
> level. I mean using the GPIO (for reset) and Power Regulator.
>
> not implementing the regulator part itself, but using it.

 If you mean change the regulator managing code from living in
 omap-hsusb to ehci-hcd, it sounds like a good idea.
>>>
>>> I mean that drivers/usb/core/hub.c should manage it, since that's what
>>> actually manages the HUB entity.
>>
>> Yes. I agree that powering up the on-board hubs should be done
>> generically and not in ehci-omap.c. I'm not sure how it can be done in
>> hub.c.
>>
>> I'm assuming that such problem is limited to root hub ports where system
> 
> an external LAN95xx HUB is not the roothub. LAN95xx is connected to the
> roothub.
> 

I didn't say LAN95xx is the root hub. It is connected to the root hub.
So it is in theory, the root hub port's responsibility to power the LAN95xx.

>> designers have the flexibility to provide power to the peripherals
>> outside the USB Hub specification.
>>
>> I can think of two solutions
>>
>> 1) Associate the regulators with the root hub _ports_ and enable them as
>> part of port's power-up routine.
> 
> doesn't make sense. We need to figure out a way to attach the regulator
> to the ports which actually have them. In this case it's the external
> LAN95xx, right ?

I think you are confused here. The LAN95xx's ports are compatible with
USB hub specification and they work using the in-band set_port_feature
mechanism already. We have a problem powering the LAN95xx itself which
ideally should have been powered with set_port_feature on the root hub.
(or ehci_port_power() in this case).

> 
>> 2) Have a global list of regulators that are registered by platform code
>> and enabled them all at once on hcd init.
> 
> also wrong as it might cause increased power consumption even when only
> a single USB port is currently in use.

Yes that is true. I'm not for (2) certainly.
> 
> The patch below is *CLEARLY WRONG* it's just to illustrate how this
> could be started:
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 1af04bd..662d4cf 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -26,6 +26,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -44,6 +45,7 @@ struct usb_port {
>   struct device dev;
>   struct dev_state *port_owner;
>   enum usb_port_connect_type connect_type;
> + struct regulator *port_regulator;
>  };
>  
>  struct usb_hub {
> @@ -847,8 +849,41 @@ static unsigned hub_power_on(struct usb_hub *hub, bool 
> do_delay)
>   else
>   dev_dbg(hub->intfdev, "trying to enable port power on "
>   "non-switchable hub\n");
> - for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++)
> + for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++) {
> + regulator_enable(hub->ports[port1]->port_regulator);
>   set_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER);
> + }
> +
> + /* Wait at least 100 msec for power to become stable */
> + delay = max(pgood_delay, (unsigned) 100);
> + if (do_delay)
> + msleep(delay);
> + return delay;
> +}
> +
> +static unsigned hub_power_off(struct usb_hub *hub, bool do_delay)
> +{
> + int port1;
> + unsigned pgood_delay = hub->descriptor->bPwrOn2PwrGood * 2;
> + unsigned delay;
> + u16 wHubCharacteristics =
> + le16_to_cpu(hub->descriptor->wHubCharacteristics);
> +
> + /* Disable power on each port.  Some hubs have reserved values
> +  * of LPSM (> 2) in their descriptors, even though they are
> +  * USB 2.0 hubs.  Some hubs do not implement port-power switching
> +  * but only emulate it.  In all cases, the ports won't work
> +  * unless we send these messages to the hub.
> +  */
> + if ((wHubCharacteristics & HUB_CHAR_LPSM) < 2)
> + dev_dbg(hub->intfdev, "disabling power on all ports\n");
> + else
> + dev_dbg(hub->intfdev, "trying to disable port power on "
> + "non-switchable hub\n");
> + for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++) {
> + regulator_disable(hub->ports[port1]->port_regulator);
> + clear_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER);
> + }
>  
>   /* Wait at least 100 msec for power to become stable */
>   delay = max(pgood_delay, (unsigned) 100);
> @@ -1336,6 +1371,9 @@ static int hub_configure(struct usb_hub *hub,
>   goto fail;
>   }
>  
> + if (hub->has_external_port_regulator) /* maybe implement with a quirk 
> flag ??? */
> + r

Re: [PATCH 16/16] ARM: OMAP: omap4panda: Power down the USB PHY and ETH when not in use

2012-11-23 Thread Felipe Balbi
Hi,

On Thu, Nov 22, 2012 at 09:35:06PM -0500, Alan Stern wrote:
> On Thu, 22 Nov 2012, Felipe Balbi wrote:
> 
> > > The latter, more or less.  For example, maybe we can tell usbcore
> > > somehow that regulator X is in control of a device attached to host
> > > controller Y (not sure how we would express X and Y though).  Then when
> > > usb_add_hcd() sees that the host controller being added is Y, it will
> > > know to turn on regulator X.  Similarly for usb_remove_hcd().
> > 
> > that'd look very nice indeed. Perhaps we could even take care of such
> > details for the roothub, even. Maybe some systems might show up where
> > roothub need external regulators provided by e.g. PMIC ?!?
> 
> As far as I can see, that ought to work provided the controller's 
> platform driver is careful not to access the controller hardware before 
> calling usb_add_hcd().
> 
> And maybe the same sort of scheme could be used for clocks, although I 
> don't know how to do it in a generic way that will work on all 
> platforms.

perhaps making use of pm_clk_add() and letting PM layer do the rest for
us ? If that doesn't work then it means PM layer's clk handling could be
improved, I suppose.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 16/16] ARM: OMAP: omap4panda: Power down the USB PHY and ETH when not in use

2012-11-23 Thread Felipe Balbi
On Fri, Nov 23, 2012 at 12:23:52PM +0200, Roger Quadros wrote:
> On 11/22/2012 06:12 PM, Felipe Balbi wrote:
> > Hi,
> > 
> > On Thu, Nov 22, 2012 at 05:00:45PM +0200, Roger Quadros wrote:
> >> On 11/22/2012 03:56 PM, Felipe Balbi wrote:
> >>> Hi,
> >>>
> >>> On Thu, Nov 22, 2012 at 09:49:05PM +0800, Andy Green wrote:
> > Again it sounds like something that should be done at the hub driver
> > level. I mean using the GPIO (for reset) and Power Regulator.
> >
> > not implementing the regulator part itself, but using it.
> 
>  If you mean change the regulator managing code from living in
>  omap-hsusb to ehci-hcd, it sounds like a good idea.
> >>>
> >>> I mean that drivers/usb/core/hub.c should manage it, since that's what
> >>> actually manages the HUB entity.
> >>
> >> Yes. I agree that powering up the on-board hubs should be done
> >> generically and not in ehci-omap.c. I'm not sure how it can be done in
> >> hub.c.
> >>
> >> I'm assuming that such problem is limited to root hub ports where system
> > 
> > an external LAN95xx HUB is not the roothub. LAN95xx is connected to the
> > roothub.
> > 
> 
> I didn't say LAN95xx is the root hub. It is connected to the root hub.
> So it is in theory, the root hub port's responsibility to power the LAN95xx.

no, it's LAN95xx's responsibility to power itself up. What if you had
multiple tiers of LAN95xx ?

> >> designers have the flexibility to provide power to the peripherals
> >> outside the USB Hub specification.
> >>
> >> I can think of two solutions
> >>
> >> 1) Associate the regulators with the root hub _ports_ and enable them as
> >> part of port's power-up routine.
> > 
> > doesn't make sense. We need to figure out a way to attach the regulator
> > to the ports which actually have them. In this case it's the external
> > LAN95xx, right ?
> 
> I think you are confused here. The LAN95xx's ports are compatible with
> USB hub specification and they work using the in-band set_port_feature
> mechanism already. We have a problem powering the LAN95xx itself which
> ideally should have been powered with set_port_feature on the root hub.
> (or ehci_port_power() in this case).

I don't set_port_feature() has anything to do with the problem here.
It's not working because the controller's supply isn't turned on. How
can any port be turned on if the supply isn't turned on ?

I still think, however, the hub needs to know how to power itself up.

> > The patch below is *CLEARLY WRONG* it's just to illustrate how this
> > could be started:
> > 
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 1af04bd..662d4cf 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -26,6 +26,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  
> >  #include 
> >  #include 
> > @@ -44,6 +45,7 @@ struct usb_port {
> > struct device dev;
> > struct dev_state *port_owner;
> > enum usb_port_connect_type connect_type;
> > +   struct regulator *port_regulator;
> >  };
> >  
> >  struct usb_hub {
> > @@ -847,8 +849,41 @@ static unsigned hub_power_on(struct usb_hub *hub, bool 
> > do_delay)
> > else
> > dev_dbg(hub->intfdev, "trying to enable port power on "
> > "non-switchable hub\n");
> > -   for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++)
> > +   for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++) {
> > +   regulator_enable(hub->ports[port1]->port_regulator);
> > set_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER);
> > +   }
> > +
> > +   /* Wait at least 100 msec for power to become stable */
> > +   delay = max(pgood_delay, (unsigned) 100);
> > +   if (do_delay)
> > +   msleep(delay);
> > +   return delay;
> > +}
> > +
> > +static unsigned hub_power_off(struct usb_hub *hub, bool do_delay)
> > +{
> > +   int port1;
> > +   unsigned pgood_delay = hub->descriptor->bPwrOn2PwrGood * 2;
> > +   unsigned delay;
> > +   u16 wHubCharacteristics =
> > +   le16_to_cpu(hub->descriptor->wHubCharacteristics);
> > +
> > +   /* Disable power on each port.  Some hubs have reserved values
> > +* of LPSM (> 2) in their descriptors, even though they are
> > +* USB 2.0 hubs.  Some hubs do not implement port-power switching
> > +* but only emulate it.  In all cases, the ports won't work
> > +* unless we send these messages to the hub.
> > +*/
> > +   if ((wHubCharacteristics & HUB_CHAR_LPSM) < 2)
> > +   dev_dbg(hub->intfdev, "disabling power on all ports\n");
> > +   else
> > +   dev_dbg(hub->intfdev, "trying to disable port power on "
> > +   "non-switchable hub\n");
> > +   for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++) {
> > +   regulator_disable(hub->ports[port1]->port_regulator);
> > +   clear_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER);
> > +   }
> >  
> > /* Wait at least 100 msec for power

RE: [RFCv4 PATCH 01/13] usb: composite: add make_group and add_function operations

2012-11-23 Thread Kyungmin Park
Hi,

> -Original Message-
> From: Sebastian Andrzej Siewior [mailto:bige...@linutronix.de]
> Sent: Friday, November 23, 2012 7:17 PM
> To: Michal Nazarewicz
> Cc: Andrzej Pietrasiewicz; linux-usb@vger.kernel.org; 'Kyungmin Park';
> 'Felipe Balbi'; 'Greg Kroah-Hartman'; 'Joel Becker'; Marek Szyprowski
> Subject: Re: [RFCv4 PATCH 01/13] usb: composite: add make_group and
> add_function operations
> 
> On 11/22/2012 09:48 PM, Michal Nazarewicz wrote:
> > I think neither is correct.  The reviewed-by tag implies that the
> > person did a careful review of the code as per “Reviewer's statement
> > of oversight” (see Documentation/SubmittingPatches).
> >
> > What actually happens is Kyungmin giving a green light to shipping the
> > patch from copyright stand-point since Samsung is copyright holder and
> > Andrzej has no power to say weather he can or cannot release the code.
> >
> > So logical path the code took was:
> >
> > Andrzej ->  Kyungmin ->  Andrzej ->  linux-usb
> 
> Aha. So is Kyungmin a lawyer and not a hacker as I assumed in the first
> place.
> 
> > If you look at other patches coming from SPRC (including mine while I
> > was working for Samsung) they all have the same Signed-off schema
> > where the first line is of the author and second is of Kyungmin.
> 
> This together with the statement above explains a lot to me. I always saw
> that and wondered how much code he can write. I assumed that Kyungmin was
> some kind of kick-ass hacker that knows all the chips very well and
> therefore writes all of the Samsung code ahead of HW and then is too busy
> with other stuff and so other people in his team push his patches mainline
> and deal with the review.
> I know that other companies work like that, where a small group of people
> does the bring-up and then others take their code and try to merge
> upstream. And this impressed me because Kyungmin is a person and not a
> small group.
> 
> Anyway.
> Signed-off indicates that he was involved in code development but he was
> not. As it seems it me, his OKAY is very important why not add him as
> 
> Acked-By: ... [copyright]
> 
> I added the [copyright] as the subsystem since he did Ack only a part of
> the patch, not the functionality etc. I know that (now) but others might
> not.

Even though all codes are not tested at internal tree, but most codes are 
tested internal tree. And these internal tree is managed by me.
That's reason to add Signed-off as internal tree maintainer. 
And most of codes from us, I checked it by internal approval process. If you 
don't feel it's not correct Signed-off scheme.
No problem to replace it with Reviewed-by or Acked-by.

Thank you,
Kyungmin Park

> 
> Sebastian

--
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: [RFCv4 PATCH 01/13] usb: composite: add make_group and add_function operations

2012-11-23 Thread Kyungmin Park
One more,
I checked it again "Documentation/SubmittingPatches"

In the 12) Sign your work

You can find the following paragraphs. 
"Some people also put extra tags at the end.  They'll just be ignored for
now, but you can do this to mark internal company procedures or just
point out some special detail about the sign-off."

And 13) When to use Acked-by: and Cc:

"The Signed-off-by: tag indicates that the signer was involved in the
development of the patch, or that he/she was in the patch's delivery path."

I used Signed-off as our contribution to open source from our teams.

Of course some codes are written by me with correct Signed-off

Thank you,
Kyungmin Park

> -Original Message-
> From: Kyungmin Park [mailto:kyungmin.p...@samsung.com]
> Sent: Friday, November 23, 2012 8:05 PM
> To: 'Sebastian Andrzej Siewior'; 'Michal Nazarewicz'
> Cc: 'Andrzej Pietrasiewicz'; 'linux-usb@vger.kernel.org'; 'Felipe Balbi';
> 'Greg Kroah-Hartman'; 'Joel Becker'; 'Marek Szyprowski'
> Subject: RE: [RFCv4 PATCH 01/13] usb: composite: add make_group and
> add_function operations
> 
> Hi,
> 
> > -Original Message-
> > From: Sebastian Andrzej Siewior [mailto:bige...@linutronix.de]
> > Sent: Friday, November 23, 2012 7:17 PM
> > To: Michal Nazarewicz
> > Cc: Andrzej Pietrasiewicz; linux-usb@vger.kernel.org; 'Kyungmin Park';
> > 'Felipe Balbi'; 'Greg Kroah-Hartman'; 'Joel Becker'; Marek Szyprowski
> > Subject: Re: [RFCv4 PATCH 01/13] usb: composite: add make_group and
> > add_function operations
> >
> > On 11/22/2012 09:48 PM, Michal Nazarewicz wrote:
> > > I think neither is correct.  The reviewed-by tag implies that the
> > > person did a careful review of the code as per “Reviewer's statement
> > > of oversight” (see Documentation/SubmittingPatches).
> > >
> > > What actually happens is Kyungmin giving a green light to shipping the
> > > patch from copyright stand-point since Samsung is copyright holder and
> > > Andrzej has no power to say weather he can or cannot release the code.
> > >
> > > So logical path the code took was:
> > >
> > >   Andrzej ->  Kyungmin ->  Andrzej ->  linux-usb
> >
> > Aha. So is Kyungmin a lawyer and not a hacker as I assumed in the first
> > place.
> >
> > > If you look at other patches coming from SPRC (including mine while I
> > > was working for Samsung) they all have the same Signed-off schema
> > > where the first line is of the author and second is of Kyungmin.
> >
> > This together with the statement above explains a lot to me. I always
> saw
> > that and wondered how much code he can write. I assumed that Kyungmin
> was
> > some kind of kick-ass hacker that knows all the chips very well and
> > therefore writes all of the Samsung code ahead of HW and then is too
> busy
> > with other stuff and so other people in his team push his patches
> mainline
> > and deal with the review.
> > I know that other companies work like that, where a small group of
> people
> > does the bring-up and then others take their code and try to merge
> > upstream. And this impressed me because Kyungmin is a person and not a
> > small group.
> >
> > Anyway.
> > Signed-off indicates that he was involved in code development but he was
> > not. As it seems it me, his OKAY is very important why not add him as
> >
> > Acked-By: ... [copyright]
> >
> > I added the [copyright] as the subsystem since he did Ack only a part of
> > the patch, not the functionality etc. I know that (now) but others might
> > not.
> 
> Even though all codes are not tested at internal tree, but most codes are
> tested internal tree. And these internal tree is managed by me.
> That's reason to add Signed-off as internal tree maintainer.
> And most of codes from us, I checked it by internal approval process. If
> you don't feel it's not correct Signed-off scheme.
> No problem to replace it with Reviewed-by or Acked-by.
> 
> Thank you,
> Kyungmin Park
> 
> >
> > Sebastian

--
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: [RFCv4 PATCH 02/13] usb: gadget: Add USB Functions Gadget

2012-11-23 Thread Andrzej Pietrasiewicz
Hello Sebastian,

As far as many of your comments are concerned, I agree and do not reply
to them. For others I agree and reply, and for yet others I reply ;)
Please see inline.

On Thursday, November 22, 2012 5:57 PM Sebastian Andrzej Siewior wrote:
> * Andrzej Pietrasiewicz | 2012-11-22 13:06:56 [+0100]:
> 



> >echo .img >
> >/cfg/usb-function-gadget/G1/C1/F1/MassStorage/lun0/file
> >
> >Do the similar thing to other functions, then
> >
> >echo 1 > /cfg/usb-function-gadget/G1/ready
> 
> The upper part (left out) sounds okay. I don't comment on the names. One
> thing you miss: Lets say you have C1 and C2. How do you configure the same
> F1 in C1 and C2 _and_ how do you configure a different F1 in F1 and
> F2 in C2. I guess the latter will work just now but the former example
> won't. The former example is used by the nokia gadget, the latter by
> g_serial.

The general idea is that:

echo "some_name" > /cfg/usb-function-gadget/G1/C1/F1/name
echo "some_name" > /cfg/usb-function-gadget/G1/C2/F1/name

creates two separate instances of F1 in C1 and C2. Did you mean
the same instance of F1 used both in C1 and C2?

Anyway, since Michał proposed some alternative this scheme is
likely to be revised.

> 
> >to actually probe and bind the gadget.
> >
> >In this case, under F1 there will be automatically created interface00
> >directory with the contents similar to this:
> >
> >/cfg/usb-function-gadget/G1/C1/F1/interface00
> >/cfg/usb-function-gadget/G1/C1/F1/interface00/altsetting
> >/cfg/usb-function-gadget/G1/C1/F1/interface00/interface_class
> >/cfg/usb-function-gadget/G1/C1/F1/interface00/interface_number
> >/cfg/usb-function-gadget/G1/C1/F1/interface00/interface_protocol
> >/cfg/usb-function-gadget/G1/C1/F1/interface00/interface_subclass
> >/cfg/usb-function-gadget/G1/C1/F1/interface00/n_endpoints
> >/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint02
> >/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint02/attributes
> >/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint02/endpoint_addre
> >ss /cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint02/interval
> >/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint02/max_packet_siz
> >e
> >/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint81
> >/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint81/attributes
> >/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint81/endpoint_addre
> >ss /cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint81/interval
> >/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint81/max_packet_siz
> >e
> 
> Interresting. I have no idea if I do like or I don't. Some minor things:
> You do have endpoint02 and you have endpoint_address. I guess
> endpoint_address here reads 2. So I think this attribute is read only.
> Same goes for max_packet_size since this is usually protocol specific.
> 

Once upon a time Felipe expressed interest in having all these accessible
from userspace (e.g. for debugging purposes) so here it is.
Meant to be read-only. 



> >
> >+config USB_FG
> >+tristate "USB Functions Gadget (EXPERIMENTAL)"
> >+select USB_LIBCOMPOSITE
> >+depends on EXPERIMENTAL && CONFIGFS_FS
> 
> EXPERIMENTAL is going away. It is no longer under drivers/usb.
> 

Anything else instead perhaps?

> >+help
> >+  USB Functions Gadget is a device which aggregates a number of
> >+  USB functions. The gadget is composed by userspace through a
> >+  configfs interface, which enables specifying what USB
> >+  configurations the gadget is composed of, what USB functions
> >+  a USB configuration is composed of and enabling/disabling
> >+  the gadget.
> 
> It would be nice to point to Documentation/usb/file-with-examples.txt

You can expect it in the next version.

> 
> >diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> >index fef41f5..09faa34 100644
> >--- a/drivers/usb/gadget/Makefile
> >+++ b/drivers/usb/gadget/Makefile
> >@@ -38,6 +38,7 @@ obj-$(CONFIG_USB_MV_U3D)   += mv_u3d_core.o
> > #
> > # USB gadget drivers
> > #
> >+g_usb_functions-y   := usb_functions.o
> > g_zero-y:= zero.o
> > g_audio-y   := audio.o
> > g_ether-y   := ether.o
> >@@ -56,6 +57,7 @@ g_ncm-y:= ncm.o
> > g_acm_ms-y  := acm_ms.o
> > g_tcm_usb_gadget-y  := tcm_usb_gadget.o
> >
> >+obj-$(CONFIG_USB_FG)+= g_usb_functions.o
> 
> Don't use this indirection and I don't see a reason why you shouldn't drop
> the g_ at the beginning. After all this will not be a gadget but an
> interface to create gadgets.
> 

This calls for a discussion: where actually this kind of code should be
located? usb_functions.c or similar? composite.c?



> 
> >+
> >+USB_GADGET_COMPOSITE_OPTIONS();
> No, no modules parameters please. This will be configsfs only.

You are right.



> >+static struct config_group *ufg_make_function_subgroup(
> >+struct config_group *group, const char *name) {
>

RE: [RFCv4 PATCH 00/13] Configfs integration

2012-11-23 Thread Andrzej Pietrasiewicz
On Thursday, November 22, 2012 9:09 PM Michal Nazarewicz wrote:

> > * Andrzej Pietrasiewicz | 2012-11-22 13:06:54 [+0100]:
> >>During its lifetime, the mass storage can require creating lunX
> >>directories for its luns. And again, with usbf_option
> 
> On Thu, Nov 22 2012, Sebastian Andrzej Siewior wrote:
> > You have first to detach the gadget because you can't update while it
> > is in progress connected / working. If you add an additional LUN then
> > there is no way to notify the host side about this.
> 
> I believe the point was that this happens during configuration.  The way I
> see it is that user writes number of luns it wants the gadget to have into
> a configfs file at which point the function creates lun# directories so
> that each lun can be configured separately.

@Michał, yes this is what I meant. Thanks for explaining.


Andrzej


--
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: [RFCv4 PATCH 02/13] usb: gadget: Add USB Functions Gadget

2012-11-23 Thread Andrzej Pietrasiewicz
Hello Michał,

I generally agree. Specifically, please see inline.

On Thursday, November 22, 2012 9:25 PM Michal Nazarewicz wrote:


 
> On Thu, Nov 22 2012, Sebastian Andrzej Siewior wrote:
> > One thing you miss: Lets say you have C1 and C2. How do you configure
> > the same F1 in C1 and C2 _and_ how do you configure a different F1 in
> > F1 and F2 in C2. I guess the latter will work just now but the former
> > example won't. The former example is used by the nokia gadget, the
> > latter by g_serial.
> 
> Yeah.  I would propose having a separate directory for functions, ie.:
> 
>   cd /cfg/usb-function-gadget/gadget1
>   mkdir functions/mass_storage
> 
> would load the function and allocate usb_function structure for it (or
> whatever) as well as create all the interesting attributes inside the
> directory, and later on:
> 
>   mkdir configs/config0
>   mkdir configs/config0/func0
>   ln functions/mass_storage configs/config0/func0/function
> 
> or something similar.
> 

I think I like it and would give it a try.

Andrzej


--
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: [RFCv4 PATCH 02/13] usb: gadget: Add USB Functions Gadget

2012-11-23 Thread Andrzej Pietrasiewicz
Hello Michał,

Thanks for comments, I generally agree.

On Thursday, November 22, 2012 10:00 PM Michal Nazarewicz wrote:



> >
> > Then specific functions to be run follow, e.g.:
> >
> > echo MassStorage > /cfg/usb-function-gadget/G1/C1/F1/name
> 
> Why not
> 
>   mkdir /cfg/usb-function-gadget/G1/C1/F1/MassStorage
> 
> ?
>

In fact, if the directories layout you proposed is used, the
mkdir will be used and in a different directory.

 


> >
> > echo 1 > /cfg/usb-function-gadget/G1/ready
> 
> This is temporary though, right?  In the future, we want to have UDCs in
> configfs as well and link the two together?

Right. I should have said it explicitly in the cover letter.

> Would it be possible to let the user just do:
> 
>   rmdir /cfg/usb-function-gadget/G1
> 
> and make the module unbind the gadget and recursively remove all the
> directories?  Specifically the requirement of:
> 
>   echo > /cfg/usb-function-gadget/G1/C1/F1/MassStorage/lun0/file
> 
> seems shady to me because this makes it impossible to have a generic way
> of unloading gadgets.  Ie. if I don't know that this gadget has a mass
> storage function which requires empty string to be written to LUN's file,
> than I won't be able to unload it.

Yeah, would be nice from the user's point of view.

Andrzej


--
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: [RFCv4 PATCH 04/13] usb: gadget: example port of mass storage to UFG: storage common: change struct device to configfs entities

2012-11-23 Thread Andrzej Pietrasiewicz
On Thursday, November 22, 2012 10:07 PM Michal Nazarewicz wrote:
> On Thu, Nov 22 2012, Andrzej Pietrasiewicz wrote:
> > @@ -45,10 +46,9 @@
> >  #define VLDBG(lun, fmt, args...) do { } while (0)  #endif /*
> > VERBOSE_DEBUG */
> >
> > -#define LDBG(lun, fmt, args...)   dev_dbg (&(lun)->dev, fmt, ## args)
> > -#define LERROR(lun, fmt, args...) dev_err (&(lun)->dev, fmt, ## args)
> > -#define LWARN(lun, fmt, args...)  dev_warn(&(lun)->dev, fmt, ## args)
> > -#define LINFO(lun, fmt, args...)  dev_info(&(lun)->dev, fmt, ## args)
> > +#define LERROR(lun, fmt, args...)  pr_err(fmt, ## args)
> > +#define LDBG(lun, fmt, args...)   pr_debug(fmt, ## args)
> > +#define LINFO(lun, fmt, args...)  pr_info(fmt, ## args)
> 
> Why is it changed in this patch?
> 

The patches were structured in order to show the steps required to convert
mass storage to the new framework. This patch removes struct devices
and code related to struct devices. That's what I meant.

Andrzej



--
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: [RFCv4 PATCH 13/13] usb: gadget: ufg: add Mass Storage Gadget adapter to UFG

2012-11-23 Thread Andrzej Pietrasiewicz
On Thursday, November 22, 2012 11:00 PM Michal Nazarewicz wrote:



> 
> > +
> > +#define UFG_MODULE (UFG_SUBSYSTEM->subsys.su_group.cg_item.ci_type-
> >ct_owner)
> 
> I cannot seem to find UFG_SUBSYSTEM anywhere.
> 

Because it is not here... It is exported from the usb_functions.c.
That's also why it is written in capital letters - to indicate
that there is something unusual about it. Maybe I could use

__ufg_subsytem instead?

Maybe not so elegant, but in order to programmatically operate on
configfs I need to have a configfs subsystem accessible in the
adapter modules. This way seemed the easiest. Providing some API
in usb_functions.c for the sake of adapters is probably not good
since the new gadget framework should not be aware that there are any
adapters at all. Even though in fact this _is_ a kind of API,
it is easily removed when time comes to remove the adapters,
which is the ultimate goal.



> > +
> > +/*
> > + * ATTENTION:
> > + *
> > + * struct configfs_dirent is "borrowed" from
> fs/configfs/configfs_internal.h.
> 
> Could we just include it?  It sounds like a better idea to me.

I could have done it. Why not? I thought that using something
"internal" is not good. On the other hand in fact I do use it
and there is the "Stable internal API nonsense" plus the
adapters are temporary by their nature. In the next version
I can include it just as well.


Andrzej


--
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: gadget: f_mass_storage: remove unused operations

2012-11-23 Thread Andrzej Pietrasiewicz
pre_eject and post_eject are not used by anyone. Removing them.

Signed-off-by: Andrzej Pietrasiewicz 
Signed-off-by: Kyungmin Park 
Acked-by: Michal Nazarewicz 

---
 drivers/usb/gadget/f_mass_storage.c |   15 +--
 drivers/usb/gadget/f_mass_storage.h |   14 --
 2 files changed, 1 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/gadget/f_mass_storage.c 
b/drivers/usb/gadget/f_mass_storage.c
index 62687df..a6b3a94 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -1259,26 +1259,13 @@ static int do_start_stop(struct fsg_common *common)
if (!loej)
return 0;
 
-   /* Simulate an unload/eject */
-   if (common->ops && common->ops->pre_eject) {
-   int r = common->ops->pre_eject(common, curlun,
-  curlun - common->luns);
-   if (unlikely(r < 0))
-   return r;
-   else if (r)
-   return 0;
-   }
-
up_read(&common->filesem);
down_write(&common->filesem);
fsg_lun_close(curlun);
up_write(&common->filesem);
down_read(&common->filesem);
 
-   return common->ops && common->ops->post_eject
-   ? min(0, common->ops->post_eject(common, curlun,
-curlun - common->luns))
-   : 0;
+   return 0;
 }
 
 static int do_prevent_allow(struct fsg_common *common)
diff --git a/drivers/usb/gadget/f_mass_storage.h 
b/drivers/usb/gadget/f_mass_storage.h
index 9f8d47e..df313e3 100644
--- a/drivers/usb/gadget/f_mass_storage.h
+++ b/drivers/usb/gadget/f_mass_storage.h
@@ -19,20 +19,6 @@ struct fsg_operations {
 * set).
 */
int (*thread_exits)(struct fsg_common *common);
-
-   /*
-* Called prior to ejection.  Negative return means error,
-* zero means to continue with ejection, positive means not to
-* eject.
-*/
-   int (*pre_eject)(struct fsg_common *common,
-struct fsg_lun *lun, int num);
-   /*
-* Called after ejection.  Negative return means error, zero
-* or positive is just a success.
-*/
-   int (*post_eject)(struct fsg_common *common,
- struct fsg_lun *lun, int num);
 };
 
 /* Data shared by all the FSG instances. */
-- 
1.7.0.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


Re: [PATCH] usb: gadget: f_mass_storage: remove unused operations

2012-11-23 Thread Sebastian Andrzej Siewior

On 11/23/2012 02:22 PM, Andrzej Pietrasiewicz wrote:


--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -1259,26 +1259,13 @@ static int do_start_stop(struct fsg_common *common)
if (!loej)


I don't know what loej does but it is unused now, isn't it?


return 0;

-   /* Simulate an unload/eject */
-   if (common->ops&&  common->ops->pre_eject) {
-   int r = common->ops->pre_eject(common, curlun,
-  curlun - common->luns);
-   if (unlikely(r<  0))
-   return r;
-   else if (r)
-   return 0;
-   }
-
up_read(&common->filesem);
down_write(&common->filesem);
fsg_lun_close(curlun);
up_write(&common->filesem);
down_read(&common->filesem);

-   return common->ops&&  common->ops->post_eject
-   ? min(0, common->ops->post_eject(common, curlun,
-curlun - common->luns))
-   : 0;
+   return 0;
  }

  static int do_prevent_allow(struct fsg_common *common)


Sebastian
--
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] usb: gadget: f_mass_storage: remove unused operations

2012-11-23 Thread Sebastian Andrzej Siewior
On Fri, Nov 23, 2012 at 02:25:44PM +0100, Sebastian Andrzej Siewior wrote:
> On 11/23/2012 02:22 PM, Andrzej Pietrasiewicz wrote:
> 
> >--- a/drivers/usb/gadget/f_mass_storage.c
> >+++ b/drivers/usb/gadget/f_mass_storage.c
> >@@ -1259,26 +1259,13 @@ static int do_start_stop(struct fsg_common *common)
> > if (!loej)
> 
> I don't know what loej does but it is unused now, isn't it?

Forget what I just wrote, it looks okay.

Sebastian
--
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] kvaser_usb: fix "dma on the stack" errors

2012-11-23 Thread Olivier Sobrie
The dma buffer given to usb_bulk_msg() must be allocated and not on
the stack.
See Documentation/DMA-API-HOWTO.txt section "What memory is DMA'able?"

Signed-off-by: Olivier Sobrie 
---
Here is the incremental patch.
Thank you Greg !

Olivier

 drivers/net/can/usb/kvaser_usb.c |  110 --
 1 file changed, 69 insertions(+), 41 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 8807bf8..7ac6e82 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -421,14 +421,21 @@ end:
 static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev,
  u8 msg_id, int channel)
 {
-   struct kvaser_msg msg = {
-   .len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple),
-   .id = msg_id,
-   .u.simple.channel = channel,
-   .u.simple.tid = 0xff,
-   };
-
-   return kvaser_usb_send_msg(dev, &msg);
+   struct kvaser_msg *msg;
+   int rc;
+
+   msg = kmalloc(sizeof(*msg), GFP_KERNEL);
+   if (!msg)
+   return -ENOMEM;
+
+   msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple);
+   msg->u.simple.channel = channel;
+   msg->u.simple.tid = 0xff;
+
+   rc = kvaser_usb_send_msg(dev, msg);
+
+   kfree(msg);
+   return rc;
 }
 
 static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
@@ -1057,20 +1064,27 @@ static int kvaser_usb_setup_rx_urbs(struct kvaser_usb 
*dev)
 
 static int kvaser_usb_set_opt_mode(const struct kvaser_usb_net_priv *priv)
 {
-   struct kvaser_msg msg = {
-   .id = CMD_SET_CTRL_MODE,
-   .len = MSG_HEADER_LEN +
-  sizeof(struct kvaser_msg_ctrl_mode),
-   .u.ctrl_mode.tid = 0xff,
-   .u.ctrl_mode.channel = priv->channel,
-   };
+   struct kvaser_msg *msg;
+   int rc;
+
+   msg = kmalloc(sizeof(*msg), GFP_KERNEL);
+   if (!msg)
+   return -ENOMEM;
+
+   msg->id = CMD_SET_CTRL_MODE;
+   msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_ctrl_mode);
+   msg->u.ctrl_mode.tid = 0xff;
+   msg->u.ctrl_mode.channel = priv->channel;
 
if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
-   msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT;
+   msg->u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT;
else
-   msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL;
+   msg->u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL;
+
+   rc = kvaser_usb_send_msg(priv->dev, msg);
 
-   return kvaser_usb_send_msg(priv->dev, &msg);
+   kfree(msg);
+   return rc;
 }
 
 static int kvaser_usb_start_chip(struct kvaser_usb_net_priv *priv)
@@ -1163,15 +1177,22 @@ static int kvaser_usb_stop_chip(struct 
kvaser_usb_net_priv *priv)
 
 static int kvaser_usb_flush_queue(struct kvaser_usb_net_priv *priv)
 {
-   struct kvaser_msg msg = {
-   .id = CMD_FLUSH_QUEUE,
-   .len = MSG_HEADER_LEN +
-  sizeof(struct kvaser_msg_flush_queue),
-   .u.flush_queue.channel = priv->channel,
-   .u.flush_queue.flags = 0x00,
-   };
-
-   return kvaser_usb_send_msg(priv->dev, &msg);
+   struct kvaser_msg *msg;
+   int rc;
+
+   msg = kmalloc(sizeof(*msg), GFP_KERNEL);
+   if (!msg)
+   return -ENOMEM;
+
+   msg->id = CMD_FLUSH_QUEUE;
+   msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_flush_queue);
+   msg->u.flush_queue.channel = priv->channel;
+   msg->u.flush_queue.flags = 0x00;
+
+   rc = kvaser_usb_send_msg(priv->dev, msg);
+
+   kfree(msg);
+   return rc;
 }
 
 static int kvaser_usb_close(struct net_device *netdev)
@@ -1364,24 +1385,31 @@ static int kvaser_usb_set_bittiming(struct net_device 
*netdev)
struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
struct can_bittiming *bt = &priv->can.bittiming;
struct kvaser_usb *dev = priv->dev;
-   struct kvaser_msg msg = {
-   .id = CMD_SET_BUS_PARAMS,
-   .len = MSG_HEADER_LEN +
-  sizeof(struct kvaser_msg_busparams),
-   .u.busparams.channel = priv->channel,
-   .u.busparams.tid = 0xff,
-   .u.busparams.bitrate = cpu_to_le32(bt->bitrate),
-   .u.busparams.sjw = bt->sjw,
-   .u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1,
-   .u.busparams.tseg2 = bt->phase_seg2,
-   };
+   struct kvaser_msg *msg;
+   int rc;
+
+   msg = kmalloc(sizeof(*msg), GFP_KERNEL);
+   if (!msg)
+   return -ENOMEM;
+
+   msg->id = CMD_SET_BUS_PARAMS;
+   msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_busparams);
+   msg->u.busparams.channel = priv->channel;
+   msg->u.busparams.tid = 0xff;
+   msg->u.busparams.bitrate = cpu_to_le3

Re: [PATCH] kvaser_usb: fix "dma on the stack" errors

2012-11-23 Thread Olivier Sobrie
On Fri, Nov 23, 2012 at 02:30:28PM +0100, Olivier Sobrie wrote:
> The dma buffer given to usb_bulk_msg() must be allocated and not on
> the stack.
> See Documentation/DMA-API-HOWTO.txt section "What memory is DMA'able?"
> 
> Signed-off-by: Olivier Sobrie 
> ---
> Here is the incremental patch.
> Thank you Greg !
> 
> Olivier
> 
>  drivers/net/can/usb/kvaser_usb.c |  110 
> --
>  1 file changed, 69 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/net/can/usb/kvaser_usb.c 
> b/drivers/net/can/usb/kvaser_usb.c
> index 8807bf8..7ac6e82 100644
> --- a/drivers/net/can/usb/kvaser_usb.c
> +++ b/drivers/net/can/usb/kvaser_usb.c
> @@ -421,14 +421,21 @@ end:
>  static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev,
> u8 msg_id, int channel)
>  {
> - struct kvaser_msg msg = {
> - .len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple),
> - .id = msg_id,
> - .u.simple.channel = channel,
> - .u.simple.tid = 0xff,
> - };
> -
> - return kvaser_usb_send_msg(dev, &msg);
> + struct kvaser_msg *msg;
> + int rc;
> +
> + msg = kmalloc(sizeof(*msg), GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +

Doh! I removed by mistake the line "msg->id = msg_id"... grr

> + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple);
> + msg->u.simple.channel = channel;
> + msg->u.simple.tid = 0xff;
> +
> + rc = kvaser_usb_send_msg(dev, msg);
> +
> + kfree(msg);
> + return rc;
>  }
>  
>  static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
> @@ -1057,20 +1064,27 @@ static int kvaser_usb_setup_rx_urbs(struct kvaser_usb 
> *dev)
>  
>  static int kvaser_usb_set_opt_mode(const struct kvaser_usb_net_priv *priv)
>  {
> - struct kvaser_msg msg = {
> - .id = CMD_SET_CTRL_MODE,
> - .len = MSG_HEADER_LEN +
> -sizeof(struct kvaser_msg_ctrl_mode),
> - .u.ctrl_mode.tid = 0xff,
> - .u.ctrl_mode.channel = priv->channel,
> - };
> + struct kvaser_msg *msg;
> + int rc;
> +
> + msg = kmalloc(sizeof(*msg), GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> + msg->id = CMD_SET_CTRL_MODE;
> + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_ctrl_mode);
> + msg->u.ctrl_mode.tid = 0xff;
> + msg->u.ctrl_mode.channel = priv->channel;
>  
>   if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
> - msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT;
> + msg->u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT;
>   else
> - msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL;
> + msg->u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL;
> +
> + rc = kvaser_usb_send_msg(priv->dev, msg);
>  
> - return kvaser_usb_send_msg(priv->dev, &msg);
> + kfree(msg);
> + return rc;
>  }
>  
>  static int kvaser_usb_start_chip(struct kvaser_usb_net_priv *priv)
> @@ -1163,15 +1177,22 @@ static int kvaser_usb_stop_chip(struct 
> kvaser_usb_net_priv *priv)
>  
>  static int kvaser_usb_flush_queue(struct kvaser_usb_net_priv *priv)
>  {
> - struct kvaser_msg msg = {
> - .id = CMD_FLUSH_QUEUE,
> - .len = MSG_HEADER_LEN +
> -sizeof(struct kvaser_msg_flush_queue),
> - .u.flush_queue.channel = priv->channel,
> - .u.flush_queue.flags = 0x00,
> - };
> -
> - return kvaser_usb_send_msg(priv->dev, &msg);
> + struct kvaser_msg *msg;
> + int rc;
> +
> + msg = kmalloc(sizeof(*msg), GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> + msg->id = CMD_FLUSH_QUEUE;
> + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_flush_queue);
> + msg->u.flush_queue.channel = priv->channel;
> + msg->u.flush_queue.flags = 0x00;
> +
> + rc = kvaser_usb_send_msg(priv->dev, msg);
> +
> + kfree(msg);
> + return rc;
>  }
>  
>  static int kvaser_usb_close(struct net_device *netdev)
> @@ -1364,24 +1385,31 @@ static int kvaser_usb_set_bittiming(struct net_device 
> *netdev)
>   struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
>   struct can_bittiming *bt = &priv->can.bittiming;
>   struct kvaser_usb *dev = priv->dev;
> - struct kvaser_msg msg = {
> - .id = CMD_SET_BUS_PARAMS,
> - .len = MSG_HEADER_LEN +
> -sizeof(struct kvaser_msg_busparams),
> - .u.busparams.channel = priv->channel,
> - .u.busparams.tid = 0xff,
> - .u.busparams.bitrate = cpu_to_le32(bt->bitrate),
> - .u.busparams.sjw = bt->sjw,
> - .u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1,
> - .u.busparams.tseg2 = bt->phase_seg2,
> - };
> + struct kvaser_msg *msg;
> + int rc;
> +
> + msg = kmalloc(sizeof(*msg), GFP_KERNEL);
> + if (!msg)
> + return -ENOM

Cannot open two tyyACMs at a time when using 2514b USB hub

2012-11-23 Thread Jan Pohanka

Hello,

I have a strange problem with embedded system I'm developing on. It is a  
board with TI's DM365 SoC. It has 2514b 4-port hub connected to its usb  
interface. It is running Linux in version 2.6.37 with TI patches, but I  
tried also vanilla and another older or recent versions and the situation  
is the same.


An USB GSM model (Telit HE910) is one of the devices connected to the hub,  
cdc_acm driver automatically detects it and ttyACM0 - ttyACM6 are created  
in dev directory. ttyACM0 and ttyACM3 can be used to send and read AT  
commands and both of them alone works as expected. The problem arises when  
I try to use them at once.


eg.
cat /dev/ttyACM0 &
cat /dev/ttyACM3

Unfortunately the second command ends with following error:
tty_port_close_start: tty->count = 1 port count = 0.
cat: can't open '/dev/ttyACM3': Input/output error

I have tried to reproduce this behavior directly on my PC, but there it  
works like a charm - at first I thought that the problem lies in cdc_acm  
driver, but today I tried to desolder the hub and connect the USB modem  
directly to the SoC. And voila it works too and I can use both ACM  
interfaces at time (one for ppp connection and second for maintenance and  
voice commands). But I definetely need a hub there...


Could please someone give me some explanation how and why the 2414b USB  
HUB can do such problems and if there is any way how to fix it?


with best regards
Jan

--
Tato zpráva byla vytvořena převratným poštovním klientem Opery:  
http://www.opera.com/mail/

--
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] kvaser_usb: fix "dma on the stack" errors

2012-11-23 Thread Marc Kleine-Budde
On 11/23/2012 02:40 PM, Olivier Sobrie wrote:
> On Fri, Nov 23, 2012 at 02:30:28PM +0100, Olivier Sobrie wrote:
>> The dma buffer given to usb_bulk_msg() must be allocated and not on
>> the stack.
>> See Documentation/DMA-API-HOWTO.txt section "What memory is DMA'able?"
>>
>> Signed-off-by: Olivier Sobrie 
>> ---
>> Here is the incremental patch.
>> Thank you Greg !
>>
>> Olivier
>>
>>  drivers/net/can/usb/kvaser_usb.c |  110 
>> --
>>  1 file changed, 69 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/net/can/usb/kvaser_usb.c 
>> b/drivers/net/can/usb/kvaser_usb.c
>> index 8807bf8..7ac6e82 100644
>> --- a/drivers/net/can/usb/kvaser_usb.c
>> +++ b/drivers/net/can/usb/kvaser_usb.c
>> @@ -421,14 +421,21 @@ end:
>>  static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev,
>>u8 msg_id, int channel)
>>  {
>> -struct kvaser_msg msg = {
>> -.len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple),
>> -.id = msg_id,
>> -.u.simple.channel = channel,
>> -.u.simple.tid = 0xff,
>> -};
>> -
>> -return kvaser_usb_send_msg(dev, &msg);
>> +struct kvaser_msg *msg;
>> +int rc;
>> +
>> +msg = kmalloc(sizeof(*msg), GFP_KERNEL);
>> +if (!msg)
>> +return -ENOMEM;
>> +
> 
> Doh! I removed by mistake the line "msg->id = msg_id"... grr

Please send a v2 version of this patch with this problem fixed.

MarcMarc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature


Re: Cannot open two tyyACMs at a time when using 2514b USB hub

2012-11-23 Thread Oliver Neukum
On Friday 23 November 2012 14:40:37 Jan Pohanka wrote:
> I have tried to reproduce this behavior directly on my PC, but there it  
> works like a charm - at first I thought that the problem lies in cdc_acm  
> driver, but today I tried to desolder the hub and connect the USB modem  
> directly to the SoC. And voila it works too and I can use both ACM  
> interfaces at time (one for ppp connection and second for maintenance and  
> voice commands). But I definetely need a hub there...
> 
> Could please someone give me some explanation how and why the 2414b USB  
> HUB can do such problems and if there is any way how to fix it?

A trace with usbmon as described in Documentation/usb should show what IO
goes wrong.

Regards
Oliver

--
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 v2] kvaser_usb: fix "dma on the stack" errors

2012-11-23 Thread Olivier Sobrie
The dma buffer given to usb_bulk_msg() must be allocated and not on
the stack.
See Documentation/DMA-API-HOWTO.txt section "What memory is DMA'able?"

Signed-off-by: Olivier Sobrie 
---
 drivers/net/can/usb/kvaser_usb.c |  111 --
 1 file changed, 70 insertions(+), 41 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 8807bf8..1b159e7 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -421,14 +421,22 @@ end:
 static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev,
  u8 msg_id, int channel)
 {
-   struct kvaser_msg msg = {
-   .len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple),
-   .id = msg_id,
-   .u.simple.channel = channel,
-   .u.simple.tid = 0xff,
-   };
-
-   return kvaser_usb_send_msg(dev, &msg);
+   struct kvaser_msg *msg;
+   int rc;
+
+   msg = kmalloc(sizeof(*msg), GFP_KERNEL);
+   if (!msg)
+   return -ENOMEM;
+
+   msg->id = msg_id;
+   msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple);
+   msg->u.simple.channel = channel;
+   msg->u.simple.tid = 0xff;
+
+   rc = kvaser_usb_send_msg(dev, msg);
+
+   kfree(msg);
+   return rc;
 }
 
 static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
@@ -1057,20 +1065,27 @@ static int kvaser_usb_setup_rx_urbs(struct kvaser_usb 
*dev)
 
 static int kvaser_usb_set_opt_mode(const struct kvaser_usb_net_priv *priv)
 {
-   struct kvaser_msg msg = {
-   .id = CMD_SET_CTRL_MODE,
-   .len = MSG_HEADER_LEN +
-  sizeof(struct kvaser_msg_ctrl_mode),
-   .u.ctrl_mode.tid = 0xff,
-   .u.ctrl_mode.channel = priv->channel,
-   };
+   struct kvaser_msg *msg;
+   int rc;
+
+   msg = kmalloc(sizeof(*msg), GFP_KERNEL);
+   if (!msg)
+   return -ENOMEM;
+
+   msg->id = CMD_SET_CTRL_MODE;
+   msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_ctrl_mode);
+   msg->u.ctrl_mode.tid = 0xff;
+   msg->u.ctrl_mode.channel = priv->channel;
 
if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
-   msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT;
+   msg->u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT;
else
-   msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL;
+   msg->u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL;
 
-   return kvaser_usb_send_msg(priv->dev, &msg);
+   rc = kvaser_usb_send_msg(priv->dev, msg);
+
+   kfree(msg);
+   return rc;
 }
 
 static int kvaser_usb_start_chip(struct kvaser_usb_net_priv *priv)
@@ -1163,15 +1178,22 @@ static int kvaser_usb_stop_chip(struct 
kvaser_usb_net_priv *priv)
 
 static int kvaser_usb_flush_queue(struct kvaser_usb_net_priv *priv)
 {
-   struct kvaser_msg msg = {
-   .id = CMD_FLUSH_QUEUE,
-   .len = MSG_HEADER_LEN +
-  sizeof(struct kvaser_msg_flush_queue),
-   .u.flush_queue.channel = priv->channel,
-   .u.flush_queue.flags = 0x00,
-   };
-
-   return kvaser_usb_send_msg(priv->dev, &msg);
+   struct kvaser_msg *msg;
+   int rc;
+
+   msg = kmalloc(sizeof(*msg), GFP_KERNEL);
+   if (!msg)
+   return -ENOMEM;
+
+   msg->id = CMD_FLUSH_QUEUE;
+   msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_flush_queue);
+   msg->u.flush_queue.channel = priv->channel;
+   msg->u.flush_queue.flags = 0x00;
+
+   rc = kvaser_usb_send_msg(priv->dev, msg);
+
+   kfree(msg);
+   return rc;
 }
 
 static int kvaser_usb_close(struct net_device *netdev)
@@ -1364,24 +1386,31 @@ static int kvaser_usb_set_bittiming(struct net_device 
*netdev)
struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
struct can_bittiming *bt = &priv->can.bittiming;
struct kvaser_usb *dev = priv->dev;
-   struct kvaser_msg msg = {
-   .id = CMD_SET_BUS_PARAMS,
-   .len = MSG_HEADER_LEN +
-  sizeof(struct kvaser_msg_busparams),
-   .u.busparams.channel = priv->channel,
-   .u.busparams.tid = 0xff,
-   .u.busparams.bitrate = cpu_to_le32(bt->bitrate),
-   .u.busparams.sjw = bt->sjw,
-   .u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1,
-   .u.busparams.tseg2 = bt->phase_seg2,
-   };
+   struct kvaser_msg *msg;
+   int rc;
+
+   msg = kmalloc(sizeof(*msg), GFP_KERNEL);
+   if (!msg)
+   return -ENOMEM;
+
+   msg->id = CMD_SET_BUS_PARAMS;
+   msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_busparams);
+   msg->u.busparams.channel = priv->channel;
+   msg->u.busparams.tid = 0xff;
+   msg->u.busparams.bitrate = cpu_to_le32(bt->bitrate);
+   msg->u.b

Re: [PATCH v2] kvaser_usb: fix "dma on the stack" errors

2012-11-23 Thread Marc Kleine-Budde
On 11/23/2012 02:54 PM, Olivier Sobrie wrote:
> The dma buffer given to usb_bulk_msg() must be allocated and not on
> the stack.
> See Documentation/DMA-API-HOWTO.txt section "What memory is DMA'able?"
> 
> Signed-off-by: Olivier Sobrie 

Thanks, I've squashed it into the original patch and pushed to
linux-can-next/for-davem

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature


Re: [PATCH v2] kvaser_usb: fix "dma on the stack" errors

2012-11-23 Thread Olivier Sobrie
On Fri, Nov 23, 2012 at 03:08:08PM +0100, Marc Kleine-Budde wrote:
> On 11/23/2012 02:54 PM, Olivier Sobrie wrote:
> > The dma buffer given to usb_bulk_msg() must be allocated and not on
> > the stack.
> > See Documentation/DMA-API-HOWTO.txt section "What memory is DMA'able?"
> > 
> > Signed-off-by: Olivier Sobrie 
> 
> Thanks, I've squashed it into the original patch and pushed to
> linux-can-next/for-davem

Ok thanks.
Have a good week-end,

Olivier

> 
> Marc
> 
> -- 
> Pengutronix e.K.  | Marc Kleine-Budde   |
> Industrial Linux Solutions| Phone: +49-231-2826-924 |
> Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
> 



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


Any followups concerning: cannot submit urb 0, error -22: internal error followed by USB hung tasks

2012-11-23 Thread Oliver Neukum
Hi Alan,

did you ever recieve any feedback concerning the patch you wrote
the patch in the message mentioned above for? I am getting reports
from customers that it fixes issues with NVIDIA chipsets.

Regards
Oliver

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


doubt about usb_cancel_queued_reset()

2012-11-23 Thread Oliver Neukum
Hi,

in usb_unbind_interface() we call usb_cancel_queued_reset() before
restoring altsetting 0. This seems wrong to me. If a driver found it
necessary to reset a device we cannot simply ignore that. I'd say
that we should wait for the work to finish, not cancel it.
What do you say?

Regards
Oliver

--
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: storage: remove redundant memset() in usb_probe_stor1()

2012-11-23 Thread Yan Hong
scsi_host_alloc() will zero our private data, no need to memset it.

Signed-off-by: Yan Hong 
---
 drivers/usb/storage/usb.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 12aa726..31b3e1a 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -925,7 +925,6 @@ int usb_stor_probe1(struct us_data **pus,
host->max_cmd_len = 16;
host->sg_tablesize = usb_stor_sg_tablesize(intf);
*pus = us = host_to_us(host);
-   memset(us, 0, sizeof(struct us_data));
mutex_init(&(us->dev_mutex));
us_set_lock_class(&us->dev_mutex, intf);
init_completion(&us->cmnd_ready);
-- 
1.7.9.5

--
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: USB issue with kernel 3.6

2012-11-23 Thread Alan Stern
On Thu, 22 Nov 2012, Piergiorgio Sartor wrote:

> OK, I got a log with working and then non working system,
> so there should be a transition,
> Problem is, the file is 1.2MB, bizp2 reduces it to 200K.
> How do I pass it to you?

You could post it on a web site, or put it on pastebin.com.  Or you 
could attach it to a bug report on bugzilla.kernel.org.

> Furthermore, this time it was quite hard to get the error.
> The system was working several minutes before it happened.
> 
> I noticed, maybe unrelated, that when working, the CPU
> clock was at max (2.6GHz), just before stopping to work,
> the CPU clock was at min (800MHz).

It's hard to know whether that is connected with the problem.

> Another, possibly related item, in this test run, the
> transfer speed was always at max, monitoring the USB
> was not slowing it down as seen before (same kernel).
> 
> I might think that the clock at max speed was responsible.

Okay, could be.

> > Also, after the problem occurs, you should go into the 
> > /sys/kernel/debug/usb/ehci/ directory and find the subdirectory 
> > corresponding to the controller for bus 1.  Let's see what the files in 
> > that subdirectory say.
> 
> Of the 4 files I found there, 2 were empty, the others were
> "periodic" and "registers", with following content:
> 
> size = 512
>1:  qh256-0001/88014a55a5a0 (h4 ep1in [1/0] q1 p1)
>  257:  qh256-0001/88014a55a5a0
> 
> bus pci, device :00:0b.1
> EHCI Host Controller
> EHCI 1.00, rh state running
> ownership 0001
> SMI sts/enable 0xc008
> structural params 0x00101888
> capability params 0xa086
> status c008 Async Periodic FLR
> command 0010015 (park)=0 ithresh=1 Periodic period=512 RUN
> intrenable 37 IAA FATAL PCD ERR INT
> uframe 0deb
> port:1 status 003400 0  ACK POWER OWNER sig=k
> port:2 status 003400 0  ACK POWER OWNER sig=k
> port:3 status 001005 0  ACK POWER sig=se0 PE CONNECT
> port:4 status 001000 0  ACK POWER sig=se0
> port:5 status 001000 0  ACK POWER sig=se0
> port:6 status 001000 0  ACK POWER sig=se0
> port:7 status 003400 0  ACK POWER OWNER sig=k
> port:8 status 003400 0  ACK POWER OWNER sig=k
> irq normal 523806 err 103 iaa 29127 (lost 0)
> complete 1188957 unlink 51
> 
> There were both taken after the error and after disconnecting
> the HUBs, I'm not sure if there're meaningful to you.

I'm not sure either.

> In case not, please let me know what should be the exact
> procedure you need.

It would be good to have another copy of all those files, this time 
taken during the test _before_ the problem occurs.  For comparison.  
Also, it would be good to get a copy of /sys/kernel/debug/usb/devices 
also taken before the problem.

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 06/29] usb: host: ehci-mv: fix clk APIs

2012-11-23 Thread Alan Stern
On Wed, 21 Nov 2012, Chao Xie wrote:

> From: Chao Xie 
> 
> Signed-off-by: Chao Xie 

I don't like patches with no patch description.  You should write 
something here, explaining why the change is needed.  What's wrong with 
the clk APIs?

The same hold for your other patches, except perhaps the trivial one 
that removes an unused local variable definition.

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: doubt about usb_cancel_queued_reset()

2012-11-23 Thread Ming Lei
On Fri, Nov 23, 2012 at 10:39 PM, Oliver Neukum  wrote:
> Hi,
>
> in usb_unbind_interface() we call usb_cancel_queued_reset() before
> restoring altsetting 0. This seems wrong to me. If a driver found it
> necessary to reset a device we cannot simply ignore that. I'd say
> that we should wait for the work to finish, not cancel it.
> What do you say?

Part of doing so may be related with below:

   The lifecycle of the work_struct is tied to the usb_interface.

On the other side, it is still reasonable to cancel the reset because
the interface driver which triggers the reset is to be unbound.


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 v3 5/7] usb: chipidea: usbmisc: add support for ahb, ipg and per clock

2012-11-23 Thread Michael Grzeschik
On Fri, Nov 23, 2012 at 02:51:16PM +0800, Peter Chen wrote:
> On Wed, Nov 21, 2012 at 03:06:31PM +0100, Michael Grzeschik wrote:
> > From: Marc Kleine-Budde 
> > 
> > This patch adds support for a second and third clock to the usbmisc driver. 
> > On
> > modern freescale ARM cores like the imx51, imx53 and imx6q three clocks 
> > ("ahb",
> > "ipg" and "per") must be enabled in order to access the USB core.
> 
> The imx6q usb's clock structure is different with old mxc serials (mx3x, mx5x)
> due to different PHY. This patch will cause mx6q's probe fail, the message
> like below:
> 
> imx_usb 2184000.usb: Failed to get clock, err=-2
> imx_usb: probe of 2184000.usb failed with error -2
> 
> The mx5x (50, 51, 53) and mx6q's clock structure like below:
> 
> mx5x:
> - usboh3_ipg_ahb: CCGR2(index13), used to access register
> (like your ahb)

That is probably the one, which already gets enabled inside the
ci13xxx_imx code.

> - usboh3_60M: CCGR2(index14), used to access register at serial phy mode
> as the phy is at serial mode, we need to use it at the first before the phy
> goes to other modes.
> (like your ipg)



> usbphy: used to transfer data
> (like your per)

The "per" clk is a different clk than the usbphy. We have an usbphy clk
aswell on the mx5x. As discussed before [1], we need a solution to enable
the usbphy clk. The current approach is to let the nop-xceiver enable
this.

[1] http://www.spinics.net/lists/linux-usb/msg74489.html

> mx6q:
> - usboh3(Index 162 at DT), used to access register
> - usbphy(Index 182 for otg port at DT), at phy controller

> So, we may consider a way to consolidate both mx6q (mx28) and mxc (mx5x, mx3x)
> platforms.

Yes, we will discuss a real clktree solution in the usbmisc driver for
the different cores. The first thoughts are, that the clk configuration
has completely to be done inside the usbmisc driver. For this, the
usbmisc needs to become a hard dependency for the ci13xxx_imx glue code.

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
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 16/16] ARM: OMAP: omap4panda: Power down the USB PHY and ETH when not in use

2012-11-23 Thread Alan Stern
On Fri, 23 Nov 2012, Felipe Balbi wrote:

> > Thanks for the example. The only problem is, how do we associate a
> > regulator to a specific port in the system.
> 
> heh, that's the 1 million dollar question, isn't it ? :-)
> 
> that's what we need to figure out now. Luckily we have Alan Stern
> helping us out ;-)

Some people might think having me around to interfere was not so lucky
after all...  :-)

First question: Should we worry about individual ports?  Ordinarily I'd
say No, because hubs always power up all of their ports.  But with Lan
Tianyu's recent work, that won't be true any more.  In this case, it's
conceivable that the user might want to power-off the LAN95xx in order
to save energy, even though ehci-hcd is still loaded and managing other
USB devices.

Either way, the regulator has to be associated with _something_: either
a root-hub port or the host controller itself.  And this will most
likely have to be done before the port's or the controller's struct
device exists.

Something like Andy's scheme might work.  It would require the kernel
to parse name strings in various formats, which could get complicated.  
It would be great if the difficult parts could be stuck in the PM core
somewhere.

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 16/16] ARM: OMAP: omap4panda: Power down the USB PHY and ETH when not in use

2012-11-23 Thread Alan Stern
On Fri, 23 Nov 2012, Felipe Balbi wrote:

> > And maybe the same sort of scheme could be used for clocks, although I 
> > don't know how to do it in a generic way that will work on all 
> > platforms.
> 
> perhaps making use of pm_clk_add() and letting PM layer do the rest for
> us ? If that doesn't work then it means PM layer's clk handling could be
> improved, I suppose.

Is the clock framework sufficiently generic at this point that it can
be used by core code (like the PM layer)?

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: [RFCv4 PATCH 01/13] usb: composite: add make_group and add_function operations

2012-11-23 Thread 'Greg Kroah-Hartman'
On Fri, Nov 23, 2012 at 08:05:29PM +0900, Kyungmin Park wrote:
> Hi,
> 
> > -Original Message-
> > From: Sebastian Andrzej Siewior [mailto:bige...@linutronix.de]
> > Sent: Friday, November 23, 2012 7:17 PM
> > To: Michal Nazarewicz
> > Cc: Andrzej Pietrasiewicz; linux-usb@vger.kernel.org; 'Kyungmin Park';
> > 'Felipe Balbi'; 'Greg Kroah-Hartman'; 'Joel Becker'; Marek Szyprowski
> > Subject: Re: [RFCv4 PATCH 01/13] usb: composite: add make_group and
> > add_function operations
> > 
> > On 11/22/2012 09:48 PM, Michal Nazarewicz wrote:
> > > I think neither is correct.  The reviewed-by tag implies that the
> > > person did a careful review of the code as per “Reviewer's statement
> > > of oversight” (see Documentation/SubmittingPatches).
> > >
> > > What actually happens is Kyungmin giving a green light to shipping the
> > > patch from copyright stand-point since Samsung is copyright holder and
> > > Andrzej has no power to say weather he can or cannot release the code.
> > >
> > > So logical path the code took was:
> > >
> > >   Andrzej ->  Kyungmin ->  Andrzej ->  linux-usb
> > 
> > Aha. So is Kyungmin a lawyer and not a hacker as I assumed in the first
> > place.
> > 
> > > If you look at other patches coming from SPRC (including mine while I
> > > was working for Samsung) they all have the same Signed-off schema
> > > where the first line is of the author and second is of Kyungmin.
> > 
> > This together with the statement above explains a lot to me. I always saw
> > that and wondered how much code he can write. I assumed that Kyungmin was
> > some kind of kick-ass hacker that knows all the chips very well and
> > therefore writes all of the Samsung code ahead of HW and then is too busy
> > with other stuff and so other people in his team push his patches mainline
> > and deal with the review.
> > I know that other companies work like that, where a small group of people
> > does the bring-up and then others take their code and try to merge
> > upstream. And this impressed me because Kyungmin is a person and not a
> > small group.
> > 
> > Anyway.
> > Signed-off indicates that he was involved in code development but he was
> > not. As it seems it me, his OKAY is very important why not add him as
> > 
> > Acked-By: ... [copyright]
> > 
> > I added the [copyright] as the subsystem since he did Ack only a part of
> > the patch, not the functionality etc. I know that (now) but others might
> > not.
> 
> Even though all codes are not tested at internal tree, but most codes are 
> tested internal tree. And these internal tree is managed by me.
> That's reason to add Signed-off as internal tree maintainer. 
> And most of codes from us, I checked it by internal approval process. If you 
> don't feel it's not correct Signed-off scheme.
> No problem to replace it with Reviewed-by or Acked-by.

No, it is correct that you used Signed-off-by: here.

I know lots of managers that have added Signed-off-by: lines to patches,
it is not necessary to be a developer of the patch to do this,
Signed-off-by: means what it says in the Documentation/SubmittingPatches
file.

thanks,

greg k-h
--
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: doubt about usb_cancel_queued_reset()

2012-11-23 Thread Alan Stern
On Fri, 23 Nov 2012, Oliver Neukum wrote:

> Hi,
> 
> in usb_unbind_interface() we call usb_cancel_queued_reset() before
> restoring altsetting 0. This seems wrong to me. If a driver found it
> necessary to reset a device we cannot simply ignore that. I'd say
> that we should wait for the work to finish, not cancel it.
> What do you say?

Since the driver is being unbound from the interface, it no longer has
any control over what happens to that interface (or the device).  In
particular, it no longer has any right to ask for a reset.

Furthermore, if the queued reset is attempted after the driver has been
unbound, it will fail.  That's because usb_lock_device_for_reset() will
return an error if the interface's condition is UNBINDING or UNBOUND.

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: Any followups concerning: cannot submit urb 0, error -22: internal error followed by USB hung tasks

2012-11-23 Thread Alan Stern
On Fri, 23 Nov 2012, Oliver Neukum wrote:

> Hi Alan,
> 
> did you ever recieve any feedback concerning the patch you wrote
> the patch in the message mentioned above for? I am getting reports
> from customers that it fixes issues with NVIDIA chipsets.

You mean the patch I posted on Oct. 9?  No, no feedback.  If people say 
that it fixes problems, I'm willing to submit it to Greg.  Can we get 
any Tested-by's?

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: Mouse and keyboard freeze Ivy-bridge

2012-11-23 Thread Alan Stern
On Thu, 22 Nov 2012, Chris Holland wrote:

> https://bugzilla.kernel.org/show_bug.cgi?id=50821
> 
> 
> Summary: Mouse and keyboard freeze Ivy-bridge
> I have been having the problem of the mouse and keyboard freezing since
> upgrading my system in May 2012
> 
> GIGABYTE GA-Z77X-UD5H LGA 1155 Intel Z77 HDMI SATA
> Intel Core i5-3550 Ivy Bridge
> 
> 
> Mouse and keyboard stop responding randomly. By using only the usb connections
> on the motherboard it requires logging in by ssh to restart the computer or
> hard resetting. Unplugging and plugging the mouse or keyboard in has no effect
> on the issue.

Which USB controller were the mouse and keyboard plugged into?

> I added a pci usb controller and this stops working as well but you can unplug
> and plug the devices back in and continue to use the computer.

In the logs you attached to the bugzilla.kernel.org report, the
keyboard and mouse were attached to the OHCI controller at
:06:00.0.  Is that the PCI controller?

If you collect a usbmon trace for that bus (bus 4 in the attached 
logs), what does it show when the problem occurs?

If you run a kernel that has CONFIG_USB_DEBUG enabled, what shows up in 
the dmesg log when the problem occurs?

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: [RFC 2/8] USB: Ignore xHCI Reset Device status.

2012-11-23 Thread Alan Stern
On Wed, 21 Nov 2012, Sarah Sharp wrote:

> A device may need to be reset several times during enumeration, and this
> causes a disconnect between the USB core's device state and the xHC's
> device state.
> 
> For example, usb_reset_and_verify_device calls into hub_port_init in a
> loop.  Say that on the first call into hub_port_init, the device is
> successfully reset, but doesn't respond to several set address control
> transfers.  Then the port will be disabled, but the udev will remain in
> tact.  usb_reset_and_verify_device will call into hub_port_init again.
> 
> The problem is that the xHC was sent a Reset Device command after the
> first port reset.  At that point, it thinks the device is in the Default
> state.  The failed Set Address commands do not move the device from the
> Default state.  On the second port reset, the Reset Device command is
> issued again.  Since the xHC thinks device is already in the Default
> state, it will reject the second Reset Device command.

That's kind of strange.  Why shouldn't the computer be allowed to reset
a device twice in a row?  The hardware's "xHC knows best" attitude is
a little annoying...

>  This will cause
> the port reset to fail until the device is logically disconnected.
> 
> Fix this by ignoring the return value of the HCD reset_device callback.

Does this really fix anything?  Since the device can't be reset after
the first attempt, the end result is bound to be a failure anyway.  
Would it be simpler to just forget about the loop (set the number of
tries to 1) in usb_reset_and_verify_device for SuperSpeed devices?

> This commit should be backported to kernels as old as 3.2, that contain
> the commit 75d7cf72ab9fa01dc70877aa5c68e8ef477229dc "usbcore: refine
> warm reset logic".
> 
> Signed-off-by: Sarah Sharp 
> Cc: sta...@vger.kernel.org
> ---
>  drivers/usb/core/hub.c |   13 +
>  1 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 7f8f10e..3bc50fc 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2565,14 +2565,11 @@ static void hub_port_finish_reset(struct usb_hub 
> *hub, int port1,
>   msleep(10 + 40);
>   update_devnum(udev, 0);
>   hcd = bus_to_hcd(udev->bus);
> - if (hcd->driver->reset_device) {
> - *status = hcd->driver->reset_device(hcd, udev);
> - if (*status < 0) {
> - dev_err(&udev->dev, "Cannot reset "
> - "HCD device state\n");
> - break;
> - }
> - }
> + /* The xHC may think the device is already reset,
> +  * so ignore the status.
> +  */

When adding new comments, stick to the recommended format:

/*
 * Comment
 * more comment
 */

Don't worry if this clashes with comments that are already present.

Also, it would be best not to mention xHC here.  In theory, other 
controller types might implement a reset_device method.  "The HC ..." 
would be better.

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: [RFC 1/8] USB: Handle auto-transition from hot to warm reset.

2012-11-23 Thread Alan Stern
On Wed, 21 Nov 2012, Sarah Sharp wrote:

> USB 3.0 hubs and roothubs will automatically transition a failed hot
> reset to a warm (BH) reset.  In that case, the warm reset change bit
> will be set, and the link state change bit may also be set.  Change
> hub_port_finish_reset to unconditionally clear those change bits for USB
> 3.0 hubs.  If these bits are not cleared, we may lose port change events
> from the roothub.
> 
> This commit should be backported to kernels as old as 3.2, that contain
> the commit 75d7cf72ab9fa01dc70877aa5c68e8ef477229dc "usbcore: refine
> warm reset logic".
> 
> Signed-off-by: Sarah Sharp 
> Cc: sta...@vger.kernel.org
> ---
>  drivers/usb/core/hub.c |6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index a815fd2..7f8f10e 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2580,16 +2580,16 @@ static void hub_port_finish_reset(struct usb_hub 
> *hub, int port1,
>   clear_port_feature(hub->hdev,
>   port1, USB_PORT_FEAT_C_RESET);
>   /* FIXME need disconnect() for NOTATTACHED device */
> - if (warm) {
> + if (hub_is_superspeed(hub->hdev)) {
>   clear_port_feature(hub->hdev, port1,
>   USB_PORT_FEAT_C_BH_PORT_RESET);
>   clear_port_feature(hub->hdev, port1,
>   USB_PORT_FEAT_C_PORT_LINK_STATE);

It would be nice to avoid sending these requests (and the one above) if 
the feature bits are already clear.  Unfortunately that's a little 
difficult to achieve the way the code is structured now.  Some 
refactoring may be in order.

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: [RFC 3/8] xHCI: Clear all USB 2.0 change bits on port disable.

2012-11-23 Thread Alan Stern
On Wed, 21 Nov 2012, Sarah Sharp wrote:

> The USB core disables the USB 2.0 ports in several different error
> cases.  It expects that all USB hubs will act like external hubs, and
> always return an interrupt event if a port change bit is set.  External
> USB 2.0 and USB 3.0 hubs are level-triggered, so this works for them.
> 
> However, xHCI roothubs are edge-triggered.  If no port status change
> bits are set, and a new change bit is set, the xHCI host sends an
> interrupt and a Port Status Change Event.  It will not generate another
> port event until all change bits are cleared and a new one is set.
> 
> This opens up issues with the USB core port disabling code.  Right now,
> it assumes that it can simply clear the port enable bit, and wait for
> the hub to send an interrupt when a change bit is set.  If previous
> change bits weren't cleared, it's assumed to be fine because the hub
> will continue to remind the USB core about them through the interrupt
> endpoint.

Is port-disabling the only place where this problem occurs?

A more defensive approach would be to copy what ohci-hcd does.  When a
port-change interrupt occurs, the driver switches over to polling for
root-hub status changes.  It doesn't switch back to interrupt-driven
operation until the hub_status_data routine sees that none of the ports
have any change bits set.

> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index a686cf4..7d9dcd6 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -340,6 +340,10 @@ static void xhci_disable_port(struct usb_hcd *hcd, 
> struct xhci_hcd *xhci,
>   return;
>   }
>  
> + /* Clear all change bits, so that we get a device connect event. */
> + port_status |= PORT_CSC | PORT_PEC | PORT_WRC |
> + PORT_OCC | PORT_RC | PORT_PLC |
> + PORT_CEC;

What if a connect-change occurred just before this?  Would it get lost?

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: [RFC 6/8] USB: Ignore USB 3.0 port state until reset completes.

2012-11-23 Thread Alan Stern
On Wed, 21 Nov 2012, Sarah Sharp wrote:

> The port reset code bails out early if the current connect status is
> cleared (device disconnected).  If we're issuing a hot reset, it may
> also look at the link state before the reset is finished.
> 
> Section 10.14.2.6 of the USB 3.0 spec says that when a port enters the
> Error state or Resetting state, the port connection bit retains the
> value from the previous state.  Therefore we can't trust it until the
> reset finishes.  Also, the xHCI spec section 4.19.1.2.5 says software
> shall ignore the link state while the port is resetting, as it can be in
> an unknown state.

In fact this is true for USB-2 hubs as well.  The hub sends a reset 
signal by driving the bus into an SE0 state.  This overwhelms the 
"connect" signal from the device, so the port can't tell whether 
anything is connected or not.

> Fix the port reset code to ignore the port link state and current
> connect bit until the reset finishes for all USB 3.0 hubs.

We might as well ignore the current-connect and connect-change bits 
until the reset has finished for all hubs, USB-3 or not.

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: [RFC 0/8] USB core reset changes

2012-11-23 Thread Alan Stern
On Wed, 21 Nov 2012, Sarah Sharp wrote:

> Hi Alan,
> 
> Can you take a look over this patchset to improve reset support under
> xHCI roothubs?
> 
> There's a lot of changes here, but I tried to break it up into easily
> verifiable chunks.  I'd like to backport all the patches except the last
> one to stable trees, so I would really appreciate your review.

Mostly they look okay.  I posted some comments on a few of the patches; 
you don't have to do anything about them right away if you prefer not 
to.  I haven't looked at them in enough detail yet to make sure that 
they don't change anything for USB-2 devices.  Assuming they don't, I 
won't have any objections to the first 7 patches.

The last one is the trickiest.  Can you at least break it up into two 
pieces: one to get rid of the recursion in the resetting code and the 
other for everything else?

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/1 v2] asix: use ramdom hw addr if the one read is not valid

2012-11-23 Thread David Miller
From: Jean-Christophe PLAGNIOL-VILLARD 
Date: Thu, 22 Nov 2012 08:35:17 +0100

> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD 

Applied to net-next.

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


gadget: provide function registration interface, v4

2012-11-23 Thread Sebastian Andrzej Siewior
this is v4 of the gadget interface.
v1. first version
v2. converted all ACM users
v3. removed the "generic configuration". This will be added once we have
configfs.
v4. changes based on feedback I received:
  - renamed usb_d_function to usb_function_driver
  - removed the typedef of function. I had only one user
  - added two comment for the two new members in struct usb_function

Comments are greatly appreciated.

--
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 01/15] usb/gadget: composite: don't call driver's unbind() if bind() failed

2012-11-23 Thread Sebastian Andrzej Siewior
Lets assume nokia_bind() starts with "return -EINVAL". After loading the
gadget we end up with:

|udc dummy_udc.0: registering UDC driver [g_nokia]
|BUG: unable to handle kernel NULL pointer dereference at 0040
|IP: [] __list_add+0x25/0xf0
|Call Trace:
| [] rollback_registered+0x21/0x40
| [] unregister_netdevice_queue+0x4f/0xa0
| [] unregister_netdev+0x19/0x30
| [] gphonet_cleanup+0x32/0x50 [g_nokia]
| [] nokia_unbind+0x1c/0x2a [g_nokia]
| [] __composite_unbind.constprop.10+0x4f/0xb0 [libcomposite]
| [] composite_bind+0x1ae/0x230 [libcomposite]
| [] usb_gadget_probe_driver+0xc6/0x1b0
| [] usb_composite_probe+0x7a/0xa0 [libcomposite]

That is crash from nokia_unbind() invoked via nokia_bind(). This crash
will look different we if make it until usb_string_ids_tab() before we
enter an error condition in the probe function.
nokia_bind_config() tries to clean up which is IMHO the right thing to
do. Leaving things as-is and hoping that its unbind() will clean it up
is kinda backwards. Especially since the bind function never succeeded so
it can't know how much it needs to clean up.
This fixes the behaviour by not calling the driver's unbind function if
its bind function failed.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/gadget/composite.c |   12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 2a6bfe7..4eb07c7 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1330,8 +1330,7 @@ static ssize_t composite_show_suspended(struct device 
*dev,
 
 static DEVICE_ATTR(suspended, 0444, composite_show_suspended, NULL);
 
-static void
-composite_unbind(struct usb_gadget *gadget)
+static void __composite_unbind(struct usb_gadget *gadget, bool unbind_driver)
 {
struct usb_composite_dev*cdev = get_gadget_data(gadget);
 
@@ -1348,7 +1347,7 @@ composite_unbind(struct usb_gadget *gadget)
struct usb_configuration, list);
remove_config(cdev, c);
}
-   if (cdev->driver->unbind)
+   if (cdev->driver->unbind && unbind_driver)
cdev->driver->unbind(cdev);
 
if (cdev->req) {
@@ -1361,6 +1360,11 @@ composite_unbind(struct usb_gadget *gadget)
set_gadget_data(gadget, NULL);
 }
 
+static void composite_unbind(struct usb_gadget *gadget)
+{
+   __composite_unbind(gadget, true);
+}
+
 static void update_unchanged_dev_desc(struct usb_device_descriptor *new,
const struct usb_device_descriptor *old)
 {
@@ -1469,7 +1473,7 @@ static int composite_bind(struct usb_gadget *gadget,
return 0;
 
 fail:
-   composite_unbind(gadget);
+   __composite_unbind(gadget, false);
return status;
 }
 
-- 
1.7.10.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 04/15] usb/gadget: move loopback's config descriptor out of f_loopback

2012-11-23 Thread Sebastian Andrzej Siewior
f_loopback should only include the bare function but it also includes
the config descriptor. This patch moves the config descriptor into
zero.c, the only user of this function.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/gadget/f_loopback.c |   44 ++-
 drivers/usb/gadget/g_zero.h |3 ---
 drivers/usb/gadget/zero.c   |   24 ++---
 3 files changed, 27 insertions(+), 44 deletions(-)

diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c
index bb39cb2..3d103a2 100644
--- a/drivers/usb/gadget/f_loopback.c
+++ b/drivers/usb/gadget/f_loopback.c
@@ -185,6 +185,12 @@ loopback_bind(struct usb_configuration *c, struct 
usb_function *f)
return id;
loopback_intf.bInterfaceNumber = id;
 
+   id = usb_string_id(cdev);
+   if (id < 0)
+   return id;
+   strings_loopback[0].id = id;
+   loopback_intf.iInterface = id;
+
/* allocate endpoints */
 
loop->in_ep = usb_ep_autoconfig(cdev->gadget, &fs_loop_source_desc);
@@ -388,41 +394,3 @@ static int __init loopback_bind_config(struct 
usb_configuration *c)
kfree(loop);
return status;
 }
-
-static struct usb_configuration loopback_driver = {
-   .label  = "loopback",
-   .strings= loopback_strings,
-   .bConfigurationValue = 2,
-   .bmAttributes   = USB_CONFIG_ATT_SELFPOWER,
-   /* .iConfiguration = DYNAMIC */
-};
-
-/**
- * loopback_add - add a loopback testing configuration to a device
- * @cdev: the device to support the loopback configuration
- */
-int __init loopback_add(struct usb_composite_dev *cdev, bool autoresume)
-{
-   int id;
-
-   /* allocate string ID(s) */
-   id = usb_string_id(cdev);
-   if (id < 0)
-   return id;
-   strings_loopback[0].id = id;
-
-   loopback_intf.iInterface = id;
-   loopback_driver.iConfiguration = id;
-
-   /* support autoresume for remote wakeup testing */
-   if (autoresume)
-   loopback_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
-
-   /* support OTG systems */
-   if (gadget_is_otg(cdev->gadget)) {
-   loopback_driver.descriptors = otg_desc;
-   loopback_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
-   }
-
-   return usb_add_config(cdev, &loopback_driver, loopback_bind_config);
-}
diff --git a/drivers/usb/gadget/g_zero.h b/drivers/usb/gadget/g_zero.h
index 919eaa9..281239c 100644
--- a/drivers/usb/gadget/g_zero.h
+++ b/drivers/usb/gadget/g_zero.h
@@ -19,7 +19,4 @@ void disable_endpoints(struct usb_composite_dev *cdev,
struct usb_ep *in, struct usb_ep *out,
struct usb_ep *iso_in, struct usb_ep *iso_out);
 
-/* configuration-specific linkup */
-int loopback_add(struct usb_composite_dev *cdev, bool autoresume);
-
 #endif /* __G_ZERO_H */
diff --git a/drivers/usb/gadget/zero.c b/drivers/usb/gadget/zero.c
index ddf37cf..8ba0bee 100644
--- a/drivers/usb/gadget/zero.c
+++ b/drivers/usb/gadget/zero.c
@@ -140,12 +140,14 @@ const struct usb_descriptor_header *otg_desc[] = {
 static char serial[] = "0123456789.0123456789.0123456789";
 
 #define USB_GZERO_SS_DESC  (USB_GADGET_FIRST_AVAIL_IDX + 0)
+#define USB_GZERO_LB_DESC  (USB_GADGET_FIRST_AVAIL_IDX + 1)
 
 static struct usb_string strings_dev[] = {
[USB_GADGET_MANUFACTURER_IDX].s = "",
[USB_GADGET_PRODUCT_IDX].s = longname,
[USB_GADGET_SERIAL_IDX].s = serial,
[USB_GZERO_SS_DESC].s   = "source and sink data",
+   [USB_GZERO_LB_DESC].s   = "loop input to output",
{  }/* end of list */
 };
 
@@ -254,6 +256,14 @@ static void zero_resume(struct usb_composite_dev *cdev)
 
 /*-*/
 
+static struct usb_configuration loopback_driver = {
+   .label  = "loopback",
+   .strings= loopback_strings,
+   .bConfigurationValue = 2,
+   .bmAttributes   = USB_CONFIG_ATT_SELFPOWER,
+   /* .iConfiguration = DYNAMIC */
+};
+
 static struct usb_configuration sourcesink_driver = {
.label  = "source/sink",
.strings= sourcesink_strings,
@@ -281,29 +291,37 @@ static int __init zero_bind(struct usb_composite_dev 
*cdev)
setup_timer(&autoresume_timer, zero_autoresume, (unsigned long) cdev);
 
sourcesink_driver.iConfiguration = strings_dev[USB_GZERO_SS_DESC].id;
+   loopback_driver.iConfiguration = strings_dev[USB_GZERO_LB_DESC].id;
+
/* support autoresume for remote wakeup testing */
sourcesink_driver.bmAttributes &= ~USB_CONFIG_ATT_WAKEUP;
+   loopback_driver.bmAttributes &= ~USB_CONFIG_ATT_WAKEUP;
sourcesink_driver.descriptors = NULL;
-   if (autoresume)
+   loopback_driver.descriptors = NULL;
+   if (autoresume) {
sourcesink_driver.bmAttributes |= USB_CONFIG_ATT_W

[PATCH 05/15] usb/gadget: add some infracture to register/unregister functions

2012-11-23 Thread Sebastian Andrzej Siewior
This patch provides an infrastructure to register & unregister a USB
function. This allows to turn a function into a module and avoid the
'#include "f_.*.c"' magic and we get a clear API / cut between the bare
gadget and its functions.
The concept is simple:
Each function defines the DECLARE_USB_FUNCTION_INIT macro whith an unique
name of the function and an allocation function. The name is used for
automaticaly loading the module if it is not yet present and request the
function from the gadget because we don't include the functions anymore.
The allocate function is mostly the "old" bind-callback which was passed
to usb_add_config() with a minor change:
- a function de-allocate function
  This is mostly the same thing that is done by the unbind function. It
  is called from within the function on "free" instead from the unbind
  path on gadget removal.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/gadget/Makefile|2 +-
 drivers/usb/gadget/composite.c |   53 +-
 drivers/usb/gadget/functions.c |   97 
 include/linux/usb/composite.h  |   44 ++
 4 files changed, 175 insertions(+), 21 deletions(-)
 create mode 100644 drivers/usb/gadget/functions.c

diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index 8b4acfd..fef41f5 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -6,7 +6,7 @@ ccflags-$(CONFIG_USB_GADGET_DEBUG) := -DDEBUG
 obj-$(CONFIG_USB_GADGET)   += udc-core.o
 obj-$(CONFIG_USB_LIBCOMPOSITE) += libcomposite.o
 libcomposite-y := usbstring.o config.o epautoconf.o
-libcomposite-y += composite.o
+libcomposite-y += composite.o functions.o
 obj-$(CONFIG_USB_DUMMY_HCD)+= dummy_hcd.o
 obj-$(CONFIG_USB_NET2272)  += net2272.o
 obj-$(CONFIG_USB_NET2280)  += net2280.o
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 4eb07c7..c46ae24 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -664,6 +664,35 @@ static int set_config(struct usb_composite_dev *cdev,
return result;
 }
 
+int usb_add_config_only(struct usb_composite_dev *cdev,
+   struct usb_configuration *config)
+{
+   struct usb_configuration *c;
+
+   DBG(cdev, "adding config #%u '%s'/%p\n",
+   config->bConfigurationValue,
+   config->label, config);
+
+   if (!config->bConfigurationValue)
+   return -EINVAL;
+
+   /* Prevent duplicate configuration identifiers */
+   list_for_each_entry(c, &cdev->configs, list) {
+   if (c->bConfigurationValue == config->bConfigurationValue)
+   return -EBUSY;
+   }
+
+   config->cdev = cdev;
+   list_add_tail(&config->list, &cdev->configs);
+
+   INIT_LIST_HEAD(&config->functions);
+   config->next_interface_id = 0;
+   memset(config->interface, 0, sizeof(config->interface));
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(usb_add_config_only);
+
 /**
  * usb_add_config() - add a configuration to a device.
  * @cdev: wraps the USB gadget
@@ -684,29 +713,13 @@ int usb_add_config(struct usb_composite_dev *cdev,
int (*bind)(struct usb_configuration *))
 {
int status = -EINVAL;
-   struct usb_configuration*c;
-
-   DBG(cdev, "adding config #%u '%s'/%p\n",
-   config->bConfigurationValue,
-   config->label, config);
 
-   if (!config->bConfigurationValue || !bind)
+   if (!bind)
goto done;
 
-   /* Prevent duplicate configuration identifiers */
-   list_for_each_entry(c, &cdev->configs, list) {
-   if (c->bConfigurationValue == config->bConfigurationValue) {
-   status = -EBUSY;
-   goto done;
-   }
-   }
-
-   config->cdev = cdev;
-   list_add_tail(&config->list, &cdev->configs);
-
-   INIT_LIST_HEAD(&config->functions);
-   config->next_interface_id = 0;
-   memset(config->interface, 0, sizeof(config->interface));
+   status = usb_add_config_only(cdev, config);
+   if (status)
+   goto done;
 
status = bind(config);
if (status < 0) {
diff --git a/drivers/usb/gadget/functions.c b/drivers/usb/gadget/functions.c
new file mode 100644
index 000..b7a36c3
--- /dev/null
+++ b/drivers/usb/gadget/functions.c
@@ -0,0 +1,97 @@
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+static LIST_HEAD(func_list);
+static DEFINE_MUTEX(func_lock);
+
+static struct usb_function *try_get_usb_function(const char *name)
+{
+   struct usb_function_driver *fd;
+   struct usb_function *f;
+
+   f = ERR_PTR(-ENOENT);
+   mutex_lock(&func_lock);
+   list_for_each_entry(fd, &func_list, list) {
+
+   if (strcmp(name, fd->name))
+  

[PATCH 07/15] usb/gadget: remove empty function in f_acm

2012-11-23 Thread Sebastian Andrzej Siewior
The significant part of this function was removed in 90f7976 ("USB:
Remove unsupported usb gadget drivers"). I would move this to function
bind time but I don't see the point in moving an empty function.
Therefore bye bye.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/gadget/f_acm.c |   10 --
 1 file changed, 10 deletions(-)

diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c
index 5491744..d4a7c19 100644
--- a/drivers/usb/gadget/f_acm.c
+++ b/drivers/usb/gadget/f_acm.c
@@ -711,13 +711,6 @@ acm_unbind(struct usb_configuration *c, struct 
usb_function *f)
kfree(acm);
 }
 
-/* Some controllers can't support CDC ACM ... */
-static inline bool can_support_cdc(struct usb_configuration *c)
-{
-   /* everything else is *probably* fine ... */
-   return true;
-}
-
 /**
  * acm_bind_config - add a CDC ACM function to a configuration
  * @c: the configuration to support the CDC ACM instance
@@ -735,9 +728,6 @@ int acm_bind_config(struct usb_configuration *c, u8 
port_num)
struct f_acm*acm;
int status;
 
-   if (!can_support_cdc(c))
-   return -EINVAL;
-
/* REVISIT might want instance-specific strings to help
 * distinguish instances ...
 */
-- 
1.7.10.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 08/15] usb/gadget: split the three possible function in g_serial into three bind functions

2012-11-23 Thread Sebastian Andrzej Siewior
This patch factors out the three possible functions into three possible
bind functions which are passed as an argument to usb_add_config(). This
will ease the step by step converting of the individual functions to the
new function registration method.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/gadget/serial.c |   43 ---
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/gadget/serial.c b/drivers/usb/gadget/serial.c
index 44752f5..bc23905 100644
--- a/drivers/usb/gadget/serial.c
+++ b/drivers/usb/gadget/serial.c
@@ -129,19 +129,33 @@ MODULE_PARM_DESC(n_ports, "number of ports to create, 
default=1");
 
 /*-*/
 
-static int __init serial_bind_config(struct usb_configuration *c)
+static int __init serial_bind_acm_config(struct usb_configuration *c)
 {
unsigned i;
int status = 0;
 
-   for (i = 0; i < n_ports && status == 0; i++) {
-   if (use_acm)
-   status = acm_bind_config(c, i);
-   else if (use_obex)
-   status = obex_bind_config(c, i);
-   else
-   status = gser_bind_config(c, i);
-   }
+   for (i = 0; i < n_ports && status == 0; i++)
+   status = acm_bind_config(c, i);
+   return status;
+}
+
+static int __init serial_bind_obex_config(struct usb_configuration *c)
+{
+   unsigned i;
+   int status = 0;
+
+   for (i = 0; i < n_ports && status == 0; i++)
+   status = obex_bind_config(c, i);
+   return status;
+}
+
+static int __init serial_bind_gser_config(struct usb_configuration *c)
+{
+   unsigned i;
+   int status = 0;
+
+   for (i = 0; i < n_ports && status == 0; i++)
+   status = gser_bind_config(c, i);
return status;
 }
 
@@ -178,8 +192,15 @@ static int __init gs_bind(struct usb_composite_dev *cdev)
}
 
/* register our configuration */
-   status = usb_add_config(cdev, &serial_config_driver,
-   serial_bind_config);
+   if (use_acm)
+   status = usb_add_config(cdev, &serial_config_driver,
+   serial_bind_acm_config);
+   else if (use_obex)
+   status = usb_add_config(cdev, &serial_config_driver,
+   serial_bind_obex_config);
+   else
+   status = usb_add_config(cdev, &serial_config_driver,
+   serial_bind_gser_config);
if (status < 0)
goto fail;
 
-- 
1.7.10.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 06/15] usb/gadget: convert source sink and loopback to new function interface

2012-11-23 Thread Sebastian Andrzej Siewior
This patch converts the f_sourcesink and f_loopback file to the USB-function
module. Both functions shares a few common utility functions which are
currently implemented in g_zero.c itself. This patch moves the common
code into the sourcesink file and creates one module out of the the two
functions (source sink and loop back).
The g_zero gadget is function specific to source sink and loop back to
set a few options. This Symbol dependency enforces a modul load right
now.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/gadget/Kconfig|4 +
 drivers/usb/gadget/Makefile   |4 +
 drivers/usb/gadget/f_loopback.c   |   59 -
 drivers/usb/gadget/f_sourcesink.c |  148 
 drivers/usb/gadget/g_zero.h   |   18 +++-
 drivers/usb/gadget/zero.c |  168 +++--
 6 files changed, 246 insertions(+), 155 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 14625fd..0ef5549 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -500,6 +500,9 @@ config USB_LIBCOMPOSITE
tristate
depends on USB_GADGET
 
+config USB_F_SS_LB
+   tristate
+
 choice
tristate "USB Gadget Drivers"
default USB_ETH
@@ -524,6 +527,7 @@ choice
 config USB_ZERO
tristate "Gadget Zero (DEVELOPMENT)"
select USB_LIBCOMPOSITE
+   select USB_F_SS_LB
help
  Gadget Zero is a two-configuration device.  It either sinks and
  sources bulk data; or it loops back a configurable number of
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index fef41f5..e412bef 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -74,3 +74,7 @@ obj-$(CONFIG_USB_G_WEBCAM)+= g_webcam.o
 obj-$(CONFIG_USB_G_NCM)+= g_ncm.o
 obj-$(CONFIG_USB_G_ACM_MS) += g_acm_ms.o
 obj-$(CONFIG_USB_GADGET_TARGET)+= tcm_usb_gadget.o
+
+# USB Functions
+f_ss_lb-y  := f_loopback.o f_sourcesink.o
+obj-$(CONFIG_USB_F_SS_LB)  += f_ss_lb.o
diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c
index 3d103a2..2d5aade 100644
--- a/drivers/usb/gadget/f_loopback.c
+++ b/drivers/usb/gadget/f_loopback.c
@@ -15,10 +15,11 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 #include "g_zero.h"
-#include "gadget_chips.h"
-
 
 /*
  * LOOPBACK FUNCTION ... a testing vehicle for USB peripherals,
@@ -44,9 +45,17 @@ static inline struct f_loopback *func_to_loop(struct 
usb_function *f)
return container_of(f, struct f_loopback, function);
 }
 
-static unsigned qlen = 32;
-module_param(qlen, uint, 0);
-MODULE_PARM_DESC(qlenn, "depth of loopback queue");
+static unsigned qlen;
+static unsigned buflen;
+
+void lb_set_options(struct usb_function *f, struct usb_zero_options *lb_opt)
+{
+   buflen = lb_opt->bulk_buflen;
+   qlen = lb_opt->qlen;
+   if (!qlen)
+   qlen = 32;
+}
+EXPORT_SYMBOL_GPL(lb_set_options);
 
 /*-*/
 
@@ -171,8 +180,7 @@ static struct usb_gadget_strings *loopback_strings[] = {
 
 /*-*/
 
-static int __init
-loopback_bind(struct usb_configuration *c, struct usb_function *f)
+static int loopback_bind(struct usb_configuration *c, struct usb_function *f)
 {
struct usb_composite_dev *cdev = c->cdev;
struct f_loopback   *loop = func_to_loop(f);
@@ -229,8 +237,7 @@ loopback_bind(struct usb_configuration *c, struct 
usb_function *f)
return 0;
 }
 
-static void
-loopback_unbind(struct usb_configuration *c, struct usb_function *f)
+static void lb_free_func(struct usb_function *f)
 {
usb_free_all_descriptors(f);
kfree(func_to_loop(f));
@@ -372,25 +379,39 @@ static void loopback_disable(struct usb_function *f)
disable_loopback(loop);
 }
 
-/*-*/
-
-static int __init loopback_bind_config(struct usb_configuration *c)
+static struct usb_function *loopback_alloc(void)
 {
struct f_loopback   *loop;
-   int status;
 
loop = kzalloc(sizeof *loop, GFP_KERNEL);
if (!loop)
-   return -ENOMEM;
+   return ERR_PTR(-ENOMEM);
 
loop->function.name = "loopback";
loop->function.bind = loopback_bind;
-   loop->function.unbind = loopback_unbind;
loop->function.set_alt = loopback_set_alt;
loop->function.disable = loopback_disable;
+   loop->function.strings = loopback_strings;
+
+   loop->function.free_func = lb_free_func;
+
+   return &loop->function;
+}
 
-   status = usb_add_function(c, &loop->function);
-   if (status)
-   kfree(loop);
-   return status;
+DECLARE_USB_FUNCTION(Loopback, loopback_alloc);
+
+int __init 

[PATCH 09/15] usb/gadget: convert u_serial into a module

2012-11-23 Thread Sebastian Andrzej Siewior
Every user of u_serial has now to select the U_SERIAL symbol instead of
including the file.
There is one limition with this: ports and and gs_tty_driver are global
variables in u_serial. Since all users share them, there can be only one
user loaded at a time i.e. either g_serial or g_nokia.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/gadget/Kconfig|9 +
 drivers/usb/gadget/Makefile   |1 +
 drivers/usb/gadget/acm_ms.c   |1 -
 drivers/usb/gadget/cdc2.c |1 -
 drivers/usb/gadget/dbgp.c |4 +---
 drivers/usb/gadget/multi.c|1 -
 drivers/usb/gadget/nokia.c|1 -
 drivers/usb/gadget/serial.c   |1 -
 drivers/usb/gadget/u_serial.c |   13 -
 9 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 0ef5549..8aefbbd 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -503,6 +503,9 @@ config USB_LIBCOMPOSITE
 config USB_F_SS_LB
tristate
 
+config USB_U_SERIAL
+   tristate
+
 choice
tristate "USB Gadget Drivers"
default USB_ETH
@@ -754,6 +757,7 @@ config USB_GADGET_TARGET
 
 config USB_G_SERIAL
tristate "Serial Gadget (with CDC ACM and CDC OBEX support)"
+   select USB_U_SERIAL
select USB_LIBCOMPOSITE
help
  The Serial Gadget talks to the Linux-USB generic serial driver.
@@ -807,6 +811,7 @@ config USB_CDC_COMPOSITE
tristate "CDC Composite Device (Ethernet and ACM)"
depends on NET
select USB_LIBCOMPOSITE
+   select USB_U_SERIAL
help
  This driver provides two functions in one configuration:
  a CDC Ethernet (ECM) link, and a CDC ACM (serial port) link.
@@ -822,6 +827,7 @@ config USB_G_NOKIA
tristate "Nokia composite gadget"
depends on PHONET
select USB_LIBCOMPOSITE
+   select USB_U_SERIAL
help
  The Nokia composite gadget provides support for acm, obex
  and phonet in only one composite gadget driver.
@@ -833,6 +839,7 @@ config USB_G_ACM_MS
tristate "CDC Composite Device (ACM and mass storage)"
depends on BLOCK
select USB_LIBCOMPOSITE
+   select USB_U_SERIAL
help
  This driver provides two functions in one configuration:
  a mass storage, and a CDC ACM (serial port) link.
@@ -845,6 +852,7 @@ config USB_G_MULTI
depends on BLOCK && NET
select USB_G_MULTI_CDC if !USB_G_MULTI_RNDIS
select USB_LIBCOMPOSITE
+   select USB_U_SERIAL
help
  The Multifunction Composite Gadget provides Ethernet (RNDIS
  and/or CDC Ethernet), mass storage and ACM serial link
@@ -920,6 +928,7 @@ config USB_G_DBGP_PRINTK
 
 config USB_G_DBGP_SERIAL
depends on USB_G_DBGP
+   select USB_U_SERIAL
bool "serial"
help
  Userland can interact using /dev/ttyGSxxx.
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index e412bef..b88ee77 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -78,3 +78,4 @@ obj-$(CONFIG_USB_GADGET_TARGET)   += tcm_usb_gadget.o
 # USB Functions
 f_ss_lb-y  := f_loopback.o f_sourcesink.o
 obj-$(CONFIG_USB_F_SS_LB)  += f_ss_lb.o
+obj-$(CONFIG_USB_U_SERIAL) += u_serial.o
diff --git a/drivers/usb/gadget/acm_ms.c b/drivers/usb/gadget/acm_ms.c
index 5a7f289..35cbe72 100644
--- a/drivers/usb/gadget/acm_ms.c
+++ b/drivers/usb/gadget/acm_ms.c
@@ -41,7 +41,6 @@
  * a "gcc --combine ... part1.c part2.c part3.c ... " build would.
  */
 
-#include "u_serial.c"
 #include "f_acm.c"
 #include "f_mass_storage.c"
 
diff --git a/drivers/usb/gadget/cdc2.c b/drivers/usb/gadget/cdc2.c
index 1e4bb77..379df67 100644
--- a/drivers/usb/gadget/cdc2.c
+++ b/drivers/usb/gadget/cdc2.c
@@ -43,7 +43,6 @@ USB_GADGET_COMPOSITE_OPTIONS();
  * a "gcc --combine ... part1.c part2.c part3.c ... " build would.
  */
 
-#include "u_serial.c"
 #include "f_acm.c"
 #include "f_ecm.c"
 #include "u_ether.c"
diff --git a/drivers/usb/gadget/dbgp.c b/drivers/usb/gadget/dbgp.c
index 87d1650..41eb98d 100644
--- a/drivers/usb/gadget/dbgp.c
+++ b/drivers/usb/gadget/dbgp.c
@@ -13,9 +13,7 @@
 #include 
 #include 
 
-#ifdef CONFIG_USB_G_DBGP_SERIAL
-#include "u_serial.c"
-#endif
+#include "u_serial.h"
 
 #define DRIVER_VENDOR_ID   0x0525 /* NetChip */
 #define DRIVER_PRODUCT_ID  0xc0de /* undefined */
diff --git a/drivers/usb/gadget/multi.c b/drivers/usb/gadget/multi.c
index 88472bf..0066791 100644
--- a/drivers/usb/gadget/multi.c
+++ b/drivers/usb/gadget/multi.c
@@ -42,7 +42,6 @@ MODULE_LICENSE("GPL");
  */
 #include "f_mass_storage.c"
 
-#include "u_serial.c"
 #include "f_acm.c"
 
 #include "f_ecm.c"
diff --git a/drivers/usb/gadget/nokia.c b/drivers/usb/gadget/nokia.c
index 661600a..60937d0 100644
--- a/drivers/usb/gadget/nokia.c
+++ b/drivers/usb/gadget/nokia.c
@@ -37,7 +37,6 @@
  * the runtime footprint, and giving us at lea

[PATCH 03/15] usb/gadget: move source sink's config descriptor out of f_sourcesink

2012-11-23 Thread Sebastian Andrzej Siewior
f_sourcesink should only include the bare function but it also includes
the config descriptor. This patch moves the config descriptor into
zero.c, the only user of this function.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/gadget/f_sourcesink.c |   48 +++--
 drivers/usb/gadget/g_zero.h   |1 -
 drivers/usb/gadget/zero.c |   31 ++--
 3 files changed, 37 insertions(+), 43 deletions(-)

diff --git a/drivers/usb/gadget/f_sourcesink.c 
b/drivers/usb/gadget/f_sourcesink.c
index 4cf52fb..1afe562 100644
--- a/drivers/usb/gadget/f_sourcesink.c
+++ b/drivers/usb/gadget/f_sourcesink.c
@@ -328,6 +328,14 @@ sourcesink_bind(struct usb_configuration *c, struct 
usb_function *f)
source_sink_intf_alt0.bInterfaceNumber = id;
source_sink_intf_alt1.bInterfaceNumber = id;
 
+   /* allocate string ID(s) */
+   id = usb_string_id(cdev);
+   if (id < 0)
+   return id;
+   strings_sourcesink[0].id = id;
+   source_sink_intf_alt0.iInterface = id;
+   source_sink_intf_alt1.iInterface = id;
+
/* allocate bulk endpoints */
ss->in_ep = usb_ep_autoconfig(cdev->gadget, &fs_source_desc);
if (!ss->in_ep) {
@@ -870,43 +878,3 @@ static int ss_config_setup(struct usb_configuration *c,
return -EOPNOTSUPP;
}
 }
-
-static struct usb_configuration sourcesink_driver = {
-   .label  = "source/sink",
-   .strings= sourcesink_strings,
-   .setup  = ss_config_setup,
-   .bConfigurationValue= 3,
-   .bmAttributes   = USB_CONFIG_ATT_SELFPOWER,
-   /* .iConfiguration  = DYNAMIC */
-};
-
-/**
- * sourcesink_add - add a source/sink testing configuration to a device
- * @cdev: the device to support the configuration
- */
-int __init sourcesink_add(struct usb_composite_dev *cdev, bool autoresume)
-{
-   int id;
-
-   /* allocate string ID(s) */
-   id = usb_string_id(cdev);
-   if (id < 0)
-   return id;
-   strings_sourcesink[0].id = id;
-
-   source_sink_intf_alt0.iInterface = id;
-   source_sink_intf_alt1.iInterface = id;
-   sourcesink_driver.iConfiguration = id;
-
-   /* support autoresume for remote wakeup testing */
-   if (autoresume)
-   sourcesink_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
-
-   /* support OTG systems */
-   if (gadget_is_otg(cdev->gadget)) {
-   sourcesink_driver.descriptors = otg_desc;
-   sourcesink_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
-   }
-
-   return usb_add_config(cdev, &sourcesink_driver, sourcesink_bind_config);
-}
diff --git a/drivers/usb/gadget/g_zero.h b/drivers/usb/gadget/g_zero.h
index 71ca193..919eaa9 100644
--- a/drivers/usb/gadget/g_zero.h
+++ b/drivers/usb/gadget/g_zero.h
@@ -20,7 +20,6 @@ void disable_endpoints(struct usb_composite_dev *cdev,
struct usb_ep *iso_in, struct usb_ep *iso_out);
 
 /* configuration-specific linkup */
-int sourcesink_add(struct usb_composite_dev *cdev, bool autoresume);
 int loopback_add(struct usb_composite_dev *cdev, bool autoresume);
 
 #endif /* __G_ZERO_H */
diff --git a/drivers/usb/gadget/zero.c b/drivers/usb/gadget/zero.c
index 6bf4c06..ddf37cf 100644
--- a/drivers/usb/gadget/zero.c
+++ b/drivers/usb/gadget/zero.c
@@ -139,10 +139,13 @@ const struct usb_descriptor_header *otg_desc[] = {
 /* default serial number takes at least two packets */
 static char serial[] = "0123456789.0123456789.0123456789";
 
+#define USB_GZERO_SS_DESC  (USB_GADGET_FIRST_AVAIL_IDX + 0)
+
 static struct usb_string strings_dev[] = {
[USB_GADGET_MANUFACTURER_IDX].s = "",
[USB_GADGET_PRODUCT_IDX].s = longname,
[USB_GADGET_SERIAL_IDX].s = serial,
+   [USB_GZERO_SS_DESC].s   = "source and sink data",
{  }/* end of list */
 };
 
@@ -251,6 +254,15 @@ static void zero_resume(struct usb_composite_dev *cdev)
 
 /*-*/
 
+static struct usb_configuration sourcesink_driver = {
+   .label  = "source/sink",
+   .strings= sourcesink_strings,
+   .setup  = ss_config_setup,
+   .bConfigurationValue= 3,
+   .bmAttributes   = USB_CONFIG_ATT_SELFPOWER,
+   /* .iConfiguration  = DYNAMIC */
+};
+
 static int __init zero_bind(struct usb_composite_dev *cdev)
 {
int status;
@@ -268,14 +280,29 @@ static int __init zero_bind(struct usb_composite_dev 
*cdev)
 
setup_timer(&autoresume_timer, zero_autoresume, (unsigned long) cdev);
 
+   sourcesink_driver.iConfiguration = strings_dev[USB_GZERO_SS_DESC].id;
+   /* support autoresume for remote wakeup testing */
+   sourcesink_driver.bmAttributes &= ~USB_CONFIG_ATT_WAKEUP;
+   sourcesink_driver.descriptors = NULL;
+   if 

[PATCH 02/15] usb/gadget: provide a wrapper around SourceSink's setup function

2012-11-23 Thread Sebastian Andrzej Siewior
The setup request can be sent to an interface/endpoint or to the device
itself. If it is sent to an interface / endpoint then we forward it to
the function that is mapped to that interface / endpoint.
If the device is the target of the setup request then we forward it to the
->setup() callback of the currently active configuration.
In case of the sourcesink function the requests are function specific
but are sent to the device.
This patch introduces a setup wrapper at configuration level which
forwards the request to the function. By using this wrapper we can keep
the function specific code within the function file and we need just a
hint at config level to forward the request.
The here introduced global variable will be moved into the gadget (which
combines the two functions) in a later patch.
SourceSink is currently the only function using ->setup() at config level.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/gadget/f_sourcesink.c |   70 -
 1 file changed, 45 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/gadget/f_sourcesink.c 
b/drivers/usb/gadget/f_sourcesink.c
index 102d49b..4cf52fb 100644
--- a/drivers/usb/gadget/f_sourcesink.c
+++ b/drivers/usb/gadget/f_sourcesink.c
@@ -449,11 +449,14 @@ sourcesink_bind(struct usb_configuration *c, struct 
usb_function *f)
return 0;
 }
 
+static struct usb_function *global_ss_func;
+
 static void
 sourcesink_unbind(struct usb_configuration *c, struct usb_function *f)
 {
usb_free_all_descriptors(f);
kfree(func_to_ss(f));
+   global_ss_func = NULL;
 }
 
 /* optionally require specific source/sink data patterns  */
@@ -757,32 +760,10 @@ static void sourcesink_disable(struct usb_function *f)
 }
 
 /*-*/
-
-static int __init sourcesink_bind_config(struct usb_configuration *c)
-{
-   struct f_sourcesink *ss;
-   int status;
-
-   ss = kzalloc(sizeof *ss, GFP_KERNEL);
-   if (!ss)
-   return -ENOMEM;
-
-   ss->function.name = "source/sink";
-   ss->function.bind = sourcesink_bind;
-   ss->function.unbind = sourcesink_unbind;
-   ss->function.set_alt = sourcesink_set_alt;
-   ss->function.get_alt = sourcesink_get_alt;
-   ss->function.disable = sourcesink_disable;
-
-   status = usb_add_function(c, &ss->function);
-   if (status)
-   kfree(ss);
-   return status;
-}
-
-static int sourcesink_setup(struct usb_configuration *c,
+static int sourcesink_setup(struct usb_function *f,
const struct usb_ctrlrequest *ctrl)
 {
+   struct usb_configuration*c = f->config;
struct usb_request  *req = c->cdev->req;
int value = -EOPNOTSUPP;
u16 w_index = le16_to_cpu(ctrl->wIndex);
@@ -851,10 +832,49 @@ static int sourcesink_setup(struct usb_configuration *c,
return value;
 }
 
+static int __init sourcesink_bind_config(struct usb_configuration *c)
+{
+   struct f_sourcesink *ss;
+   int status;
+
+   ss = kzalloc(sizeof(*ss), GFP_KERNEL);
+   if (!ss)
+   return -ENOMEM;
+
+   global_ss_func = &ss->function;
+
+   ss->function.name = "source/sink";
+   ss->function.bind = sourcesink_bind;
+   ss->function.unbind = sourcesink_unbind;
+   ss->function.set_alt = sourcesink_set_alt;
+   ss->function.get_alt = sourcesink_get_alt;
+   ss->function.disable = sourcesink_disable;
+   ss->function.setup = sourcesink_setup;
+
+   status = usb_add_function(c, &ss->function);
+   if (status)
+   kfree(ss);
+   return status;
+}
+
+static int ss_config_setup(struct usb_configuration *c,
+   const struct usb_ctrlrequest *ctrl)
+{
+   if (!global_ss_func)
+   return -EOPNOTSUPP;
+   switch (ctrl->bRequest) {
+   case 0x5b:
+   case 0x5c:
+   return global_ss_func->setup(global_ss_func, ctrl);
+   default:
+   return -EOPNOTSUPP;
+   }
+}
+
 static struct usb_configuration sourcesink_driver = {
.label  = "source/sink",
.strings= sourcesink_strings,
-   .setup  = sourcesink_setup,
+   .setup  = ss_config_setup,
.bConfigurationValue= 3,
.bmAttributes   = USB_CONFIG_ATT_SELFPOWER,
/* .iConfiguration  = DYNAMIC */
-- 
1.7.10.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 10/15] usb/gadget: add usb_remove_function()

2012-11-23 Thread Sebastian Andrzej Siewior
This will be used to remove a single function from a given config. Right
now "ignore" that an error at ->bind() time and cleanup later during
composite_unbind() / remove_config().

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/gadget/composite.c |   12 
 include/linux/usb/composite.h  |1 +
 2 files changed, 13 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index c46ae24..682ae15 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -215,6 +215,18 @@ int usb_add_function(struct usb_configuration *config,
 }
 EXPORT_SYMBOL_GPL(usb_add_function);
 
+void usb_remove_function(struct usb_configuration *c, struct usb_function *f)
+{
+   if (f->disable)
+   f->disable(f);
+
+   bitmap_zero(f->endpoints, 32);
+   list_del(&f->list);
+   if (f->unbind)
+   f->unbind(c, f);
+}
+EXPORT_SYMBOL_GPL(usb_remove_function);
+
 /**
  * usb_function_deactivate - prevent function and gadget enumeration
  * @function: the function that isn't yet ready to respond
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 1fa9941..b52211a 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -453,6 +453,7 @@ struct usb_configuration *usb_get_config(struct 
usb_composite_dev *cdev,
int val);
 int usb_add_config_only(struct usb_composite_dev *cdev,
struct usb_configuration *config);
+void usb_remove_function(struct usb_configuration *c, struct usb_function *f);
 
 #define DECLARE_USB_FUNCTION(_name, _alloc)\
static struct usb_function_driver _name ## usb_func = { \
-- 
1.7.10.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 11/15] usb/gadget: convert f_acm to new function interface with backwards compatibility

2012-11-23 Thread Sebastian Andrzej Siewior
This patch converts f_acm into a module which uses the new function
interface. It also converts one of its users that is g_serial to make
use of it. The other users of it (g_nokia for instance) are still using
the old include file system and should not notice the change at all. So
they can be converter later independently.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/gadget/Kconfig|4 ++
 drivers/usb/gadget/Makefile   |1 +
 drivers/usb/gadget/acm_ms.c   |2 +-
 drivers/usb/gadget/cdc2.c |2 +-
 drivers/usb/gadget/f_acm.c|  110 +
 drivers/usb/gadget/multi.c|2 +-
 drivers/usb/gadget/nokia.c|1 +
 drivers/usb/gadget/serial.c   |   81 --
 drivers/usb/gadget/u_serial.h |2 +
 9 files changed, 156 insertions(+), 49 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 8aefbbd..b61c72f 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -500,6 +500,9 @@ config USB_LIBCOMPOSITE
tristate
depends on USB_GADGET
 
+config USB_F_ACM
+   tristate
+
 config USB_F_SS_LB
tristate
 
@@ -758,6 +761,7 @@ config USB_GADGET_TARGET
 config USB_G_SERIAL
tristate "Serial Gadget (with CDC ACM and CDC OBEX support)"
select USB_U_SERIAL
+   select USB_F_ACM
select USB_LIBCOMPOSITE
help
  The Serial Gadget talks to the Linux-USB generic serial driver.
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index b88ee77..97a13c3 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_USB_G_ACM_MS)+= g_acm_ms.o
 obj-$(CONFIG_USB_GADGET_TARGET)+= tcm_usb_gadget.o
 
 # USB Functions
+obj-$(CONFIG_USB_F_ACM)+= f_acm.o
 f_ss_lb-y  := f_loopback.o f_sourcesink.o
 obj-$(CONFIG_USB_F_SS_LB)  += f_ss_lb.o
 obj-$(CONFIG_USB_U_SERIAL) += u_serial.o
diff --git a/drivers/usb/gadget/acm_ms.c b/drivers/usb/gadget/acm_ms.c
index 35cbe72..4c5f116 100644
--- a/drivers/usb/gadget/acm_ms.c
+++ b/drivers/usb/gadget/acm_ms.c
@@ -40,7 +40,7 @@
  * the runtime footprint, and giving us at least some parts of what
  * a "gcc --combine ... part1.c part2.c part3.c ... " build would.
  */
-
+#define USB_FACM_INCLUDED
 #include "f_acm.c"
 #include "f_mass_storage.c"
 
diff --git a/drivers/usb/gadget/cdc2.c b/drivers/usb/gadget/cdc2.c
index 379df67..fa7a26c 100644
--- a/drivers/usb/gadget/cdc2.c
+++ b/drivers/usb/gadget/cdc2.c
@@ -42,7 +42,7 @@ USB_GADGET_COMPOSITE_OPTIONS();
  * the runtime footprint, and giving us at least some parts of what
  * a "gcc --combine ... part1.c part2.c part3.c ... " build would.
  */
-
+#define USB_FACM_INCLUDED
 #include "f_acm.c"
 #include "f_ecm.c"
 #include "u_ether.c"
diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c
index d4a7c19..3aba07dc 100644
--- a/drivers/usb/gadget/f_acm.c
+++ b/drivers/usb/gadget/f_acm.c
@@ -16,7 +16,9 @@
 
 #include 
 #include 
+#include 
 #include 
+#include 
 
 #include "u_serial.h"
 #include "gadget_chips.h"
@@ -608,6 +610,22 @@ acm_bind(struct usb_configuration *c, struct usb_function 
*f)
int status;
struct usb_ep   *ep;
 
+   /* REVISIT might want instance-specific strings to help
+* distinguish instances ...
+*/
+
+   /* maybe allocate device-global string IDs, and patch descriptors */
+   if (acm_string_defs[0].id == 0) {
+   status = usb_string_ids_tab(c->cdev, acm_string_defs);
+   if (status < 0)
+   return status;
+   acm_control_interface_desc.iInterface =
+   acm_string_defs[ACM_CTRL_IDX].id;
+   acm_data_interface_desc.iInterface =
+   acm_string_defs[ACM_DATA_IDX].id;
+   acm_iad_descriptor.iFunction = acm_string_defs[ACM_IAD_IDX].id;
+   }
+
/* allocate instance-specific interface IDs, and patch descriptors */
status = usb_interface_id(c, f);
if (status < 0)
@@ -707,10 +725,37 @@ acm_unbind(struct usb_configuration *c, struct 
usb_function *f)
 
acm_string_defs[0].id = 0;
usb_free_all_descriptors(f);
-   gs_free_req(acm->notify, acm->notify_req);
+   if (acm->notify_req)
+   gs_free_req(acm->notify, acm->notify_req);
kfree(acm);
 }
 
+static struct f_acm *acm_alloc_basic_func(void)
+{
+   struct f_acm*acm;
+
+   acm = kzalloc(sizeof(*acm), GFP_KERNEL);
+   if (!acm)
+   return NULL;
+
+   spin_lock_init(&acm->lock);
+
+   acm->port.connect = acm_connect;
+   acm->port.disconnect = acm_disconnect;
+   acm->port.send_break = acm_send_break;
+
+   acm->port.func.name = "acm";
+   acm->port.func.strings = acm_strings;
+   /* descriptors are per-instance copies */
+   acm->port.func.b

[PATCH 14/15] usb/gadget: multi: use function framework for ACM

2012-11-23 Thread Sebastian Andrzej Siewior
This patch converts the acm_ms gadget to make use of the function
framework to request the ACM function.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/gadget/Kconfig |1 +
 drivers/usb/gadget/multi.c |   47 ++--
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index dfe3e2d..6665d25 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -859,6 +859,7 @@ config USB_G_MULTI
select USB_G_MULTI_CDC if !USB_G_MULTI_RNDIS
select USB_LIBCOMPOSITE
select USB_U_SERIAL
+   select USB_F_ACM
help
  The Multifunction Composite Gadget provides Ethernet (RNDIS
  and/or CDC Ethernet), mass storage and ACM serial link
diff --git a/drivers/usb/gadget/multi.c b/drivers/usb/gadget/multi.c
index 78b26ed..0b871b3 100644
--- a/drivers/usb/gadget/multi.c
+++ b/drivers/usb/gadget/multi.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 
+#include "u_serial.h"
 #if defined USB_ETH_RNDIS
 #  undef USB_ETH_RNDIS
 #endif
@@ -41,8 +42,6 @@ MODULE_LICENSE("GPL");
  * a "gcc --combine ... part1.c part2.c part3.c ... " build would.
  */
 #include "f_mass_storage.c"
-#define USB_FACM_INCLUDED
-#include "f_acm.c"
 
 #include "f_ecm.c"
 #include "f_subset.c"
@@ -140,6 +139,7 @@ static u8 hostaddr[ETH_ALEN];
 /** RNDIS **/
 
 #ifdef USB_ETH_RNDIS
+static struct usb_function *f_acm_rndis;
 
 static __init int rndis_do_config(struct usb_configuration *c)
 {
@@ -154,15 +154,25 @@ static __init int rndis_do_config(struct 
usb_configuration *c)
if (ret < 0)
return ret;
 
-   ret = acm_bind_config(c, 0);
-   if (ret < 0)
-   return ret;
+   f_acm_rndis = usb_get_function("acm");
+   if (IS_ERR(f_acm_rndis))
+   return PTR_ERR(f_acm_rndis);
+
+   facm_configure(f_acm_rndis, 0);
+   ret = usb_add_function(c, f_acm_rndis);
+   if (ret)
+   goto err_conf;
 
ret = fsg_bind_config(c->cdev, c, &fsg_common);
if (ret < 0)
-   return ret;
+   goto err_fsg;
 
return 0;
+err_fsg:
+   usb_remove_function(c, f_acm_rndis);
+err_conf:
+   usb_put_function(f_acm_rndis);
+   return ret;
 }
 
 static int rndis_config_register(struct usb_composite_dev *cdev)
@@ -191,6 +201,7 @@ static int rndis_config_register(struct usb_composite_dev 
*cdev)
 /** CDC ECM **/
 
 #ifdef CONFIG_USB_G_MULTI_CDC
+static struct usb_function *f_acm_multi;
 
 static __init int cdc_do_config(struct usb_configuration *c)
 {
@@ -205,15 +216,25 @@ static __init int cdc_do_config(struct usb_configuration 
*c)
if (ret < 0)
return ret;
 
-   ret = acm_bind_config(c, 0);
-   if (ret < 0)
-   return ret;
+   f_acm_multi = usb_get_function("acm");
+   if (IS_ERR(f_acm_multi))
+   return PTR_ERR(f_acm_multi);
+
+   facm_configure(f_acm_multi, 0);
+   ret = usb_add_function(c, f_acm_multi);
+   if (ret)
+   goto err_conf;
 
ret = fsg_bind_config(c->cdev, c, &fsg_common);
if (ret < 0)
-   return ret;
+   goto err_fsg;
 
return 0;
+err_fsg:
+   usb_remove_function(c, f_acm_multi);
+err_conf:
+   usb_put_function(f_acm_multi);
+   return ret;
 }
 
 static int cdc_config_register(struct usb_composite_dev *cdev)
@@ -308,6 +329,12 @@ static int __ref multi_bind(struct usb_composite_dev *cdev)
 
 static int __exit multi_unbind(struct usb_composite_dev *cdev)
 {
+#ifdef CONFIG_USB_G_MULTI_CDC
+   usb_put_function(f_acm_multi);
+#endif
+#ifdef USB_ETH_RNDIS
+   usb_put_function(f_acm_rndis);
+#endif
gserial_cleanup();
gether_cleanup();
return 0;
-- 
1.7.10.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 12/15] usb/gadget: acm_ms: use function framework for ACM

2012-11-23 Thread Sebastian Andrzej Siewior
This patch converts the acm_ms gadget to make use of the function
framework to request the ACM function.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/gadget/Kconfig  |1 +
 drivers/usb/gadget/acm_ms.c |   21 ++---
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index b61c72f..8dad2ce 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -844,6 +844,7 @@ config USB_G_ACM_MS
depends on BLOCK
select USB_LIBCOMPOSITE
select USB_U_SERIAL
+   select USB_F_ACM
help
  This driver provides two functions in one configuration:
  a mass storage, and a CDC ACM (serial port) link.
diff --git a/drivers/usb/gadget/acm_ms.c b/drivers/usb/gadget/acm_ms.c
index 4c5f116..c570aa2 100644
--- a/drivers/usb/gadget/acm_ms.c
+++ b/drivers/usb/gadget/acm_ms.c
@@ -40,8 +40,6 @@
  * the runtime footprint, and giving us at least some parts of what
  * a "gcc --combine ... part1.c part2.c part3.c ... " build would.
  */
-#define USB_FACM_INCLUDED
-#include "f_acm.c"
 #include "f_mass_storage.c"
 
 /*-*/
@@ -111,7 +109,7 @@ FSG_MODULE_PARAMETERS(/* no prefix */, fsg_mod_data);
 static struct fsg_common fsg_common;
 
 /*-*/
-
+static struct usb_function *f_acm;
 /*
  * We _always_ have both ACM and mass storage functions.
  */
@@ -124,16 +122,25 @@ static int __init acm_ms_do_config(struct 
usb_configuration *c)
c->bmAttributes |= USB_CONFIG_ATT_WAKEUP;
}
 
+   f_acm = usb_get_function("acm");
+   if (IS_ERR(f_acm))
+   return PTR_ERR(f_acm);
 
-   status = acm_bind_config(c, 0);
+   facm_configure(f_acm, 0);
+   status = usb_add_function(c, f_acm);
if (status < 0)
-   return status;
+   goto err_conf;
 
status = fsg_bind_config(c->cdev, c, &fsg_common);
if (status < 0)
-   return status;
+   goto err_fsg;
 
return 0;
+err_fsg:
+   usb_remove_function(c, f_acm);
+err_conf:
+   usb_put_function(f_acm);
+   return status;
 }
 
 static struct usb_configuration acm_ms_config_driver = {
@@ -194,8 +201,8 @@ static int __init acm_ms_bind(struct usb_composite_dev 
*cdev)
 
 static int __exit acm_ms_unbind(struct usb_composite_dev *cdev)
 {
+   usb_put_function(f_acm);
gserial_cleanup();
-
return 0;
 }
 
-- 
1.7.10.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 15/15] usb/gadget: nokia: use function framework for ACM

2012-11-23 Thread Sebastian Andrzej Siewior
This patch converts the acm_ms gadget to make use of the function
framework to request the ACM function.

With nokia beeing the last user of the include interface, it is gone
now.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/gadget/Kconfig|1 +
 drivers/usb/gadget/f_acm.c|   36 ---
 drivers/usb/gadget/nokia.c|   64 ++---
 drivers/usb/gadget/u_serial.h |1 -
 4 files changed, 42 insertions(+), 60 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 6665d25..afb78f6 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -833,6 +833,7 @@ config USB_G_NOKIA
depends on PHONET
select USB_LIBCOMPOSITE
select USB_U_SERIAL
+   select USB_F_ACM
help
  The Nokia composite gadget provides support for acm, obex
  and phonet in only one composite gadget driver.
diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c
index 3aba07dc..64cea16 100644
--- a/drivers/usb/gadget/f_acm.c
+++ b/drivers/usb/gadget/f_acm.c
@@ -755,41 +755,6 @@ static struct f_acm *acm_alloc_basic_func(void)
return acm;
 }
 
-#ifdef USB_FACM_INCLUDED
-/**
- * acm_bind_config - add a CDC ACM function to a configuration
- * @c: the configuration to support the CDC ACM instance
- * @port_num: /dev/ttyGS* port this interface will use
- * Context: single threaded during gadget setup
- *
- * Returns zero on success, else negative errno.
- *
- * Caller must have called @gserial_setup() with enough ports to
- * handle all the ones it binds.  Caller is also responsible
- * for calling @gserial_cleanup() before module unload.
- */
-int acm_bind_config(struct usb_configuration *c, u8 port_num)
-{
-   struct f_acm*acm;
-   int status;
-
-   /* allocate and initialize one new instance */
-   acm = acm_alloc_basic_func();
-   if (!acm)
-   return -ENOMEM;
-
-   acm->port_num = port_num;
-
-   acm->port.func.unbind = acm_unbind;
-
-   status = usb_add_function(c, &acm->port.func);
-   if (status)
-   kfree(acm);
-   return status;
-}
-
-#else
-
 static void acm_free_func(struct usb_function *f)
 {
acm_unbind(NULL, f);
@@ -818,4 +783,3 @@ static struct usb_function *acm_alloc_func(void)
 }
 DECLARE_USB_FUNCTION_INIT(acm, acm_alloc_func);
 MODULE_LICENSE("GPL");
-#endif
diff --git a/drivers/usb/gadget/nokia.c b/drivers/usb/gadget/nokia.c
index a000fb7..32ba900 100644
--- a/drivers/usb/gadget/nokia.c
+++ b/drivers/usb/gadget/nokia.c
@@ -37,8 +37,6 @@
  * the runtime footprint, and giving us at least some parts of what
  * a "gcc --combine ... part1.c part2.c part3.c ... " build would.
  */
-#define USB_FACM_INCLUDED
-#include "f_acm.c"
 #include "f_ecm.c"
 #include "f_obex.c"
 #include "f_serial.c"
@@ -98,11 +96,29 @@ MODULE_AUTHOR("Felipe Balbi");
 MODULE_LICENSE("GPL");
 
 /*-*/
-
+static struct usb_function *f_acm_cfg1;
+static struct usb_function *f_acm_cfg2;
 static u8 hostaddr[ETH_ALEN];
 
+static struct usb_configuration nokia_config_500ma_driver = {
+   .label  = "Bus Powered",
+   .bConfigurationValue = 1,
+   /* .iConfiguration = DYNAMIC */
+   .bmAttributes   = USB_CONFIG_ATT_ONE,
+   .bMaxPower  = 250, /* 500mA */
+};
+
+static struct usb_configuration nokia_config_100ma_driver = {
+   .label  = "Self Powered",
+   .bConfigurationValue = 2,
+   /* .iConfiguration = DYNAMIC */
+   .bmAttributes   = USB_CONFIG_ATT_ONE | USB_CONFIG_ATT_SELFPOWER,
+   .bMaxPower  = 50, /* 100 mA */
+};
+
 static int __init nokia_bind_config(struct usb_configuration *c)
 {
+   struct usb_function *f_acm;
int status = 0;
 
status = phonet_bind_config(c);
@@ -117,33 +133,33 @@ static int __init nokia_bind_config(struct 
usb_configuration *c)
if (status)
printk(KERN_DEBUG "could not bind obex config %d\n", 0);
 
-   status = acm_bind_config(c, 2);
+   f_acm = usb_get_function("acm");
+   if (IS_ERR(f_acm))
+   return PTR_ERR(f_acm);
+
+   facm_configure(f_acm, 2);
+   status = usb_add_function(c, f_acm);
if (status)
-   printk(KERN_DEBUG "could not bind acm config\n");
+   goto err_conf;
 
status = ecm_bind_config(c, hostaddr);
-   if (status)
-   printk(KERN_DEBUG "could not bind ecm config\n");
+   if (status) {
+   pr_debug("could not bind ecm config %d\n", status);
+   goto err_ecm;
+   }
+   if (c == &nokia_config_500ma_driver)
+   f_acm_cfg1 = f_acm;
+   else
+   f_acm_cfg2 = f_acm;
 
return status;
+err_ecm:
+   usb_remove_function(c, f_acm);
+err_conf:
+   usb_put_function(f_acm);
+   return status;
 }
 
-static struct usb_co

[PATCH 13/15] usb/gadget: cdc2: use function framework for ACM

2012-11-23 Thread Sebastian Andrzej Siewior
This patch converts the acm_ms gadget to make use of the function
framework to request the ACM function.

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/gadget/Kconfig |1 +
 drivers/usb/gadget/cdc2.c  |   17 -
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 8dad2ce..dfe3e2d 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -816,6 +816,7 @@ config USB_CDC_COMPOSITE
depends on NET
select USB_LIBCOMPOSITE
select USB_U_SERIAL
+   select USB_F_ACM
help
  This driver provides two functions in one configuration:
  a CDC Ethernet (ECM) link, and a CDC ACM (serial port) link.
diff --git a/drivers/usb/gadget/cdc2.c b/drivers/usb/gadget/cdc2.c
index fa7a26c..d6e2675 100644
--- a/drivers/usb/gadget/cdc2.c
+++ b/drivers/usb/gadget/cdc2.c
@@ -42,8 +42,6 @@ USB_GADGET_COMPOSITE_OPTIONS();
  * the runtime footprint, and giving us at least some parts of what
  * a "gcc --combine ... part1.c part2.c part3.c ... " build would.
  */
-#define USB_FACM_INCLUDED
-#include "f_acm.c"
 #include "f_ecm.c"
 #include "u_ether.c"
 
@@ -107,6 +105,7 @@ static struct usb_gadget_strings *dev_strings[] = {
 static u8 hostaddr[ETH_ALEN];
 
 /*-*/
+static struct usb_function *f_acm;
 
 /*
  * We _always_ have both CDC ECM and CDC ACM functions.
@@ -124,11 +123,18 @@ static int __init cdc_do_config(struct usb_configuration 
*c)
if (status < 0)
return status;
 
-   status = acm_bind_config(c, 0);
-   if (status < 0)
-   return status;
+   f_acm = usb_get_function("acm");
+   if (IS_ERR(f_acm))
+   return PTR_ERR(f_acm);
 
+   facm_configure(f_acm, 0);
+   status = usb_add_function(c, f_acm);
+   if (status)
+   goto err_conf;
return 0;
+err_conf:
+   usb_put_function(f_acm);
+   return status;
 }
 
 static struct usb_configuration cdc_config_driver = {
@@ -191,6 +197,7 @@ static int __init cdc_bind(struct usb_composite_dev *cdev)
 
 static int __exit cdc_unbind(struct usb_composite_dev *cdev)
 {
+   usb_put_function(f_acm);
gserial_cleanup();
gether_cleanup();
return 0;
-- 
1.7.10.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


Re: [PATCH 16/16] ARM: OMAP: omap4panda: Power down the USB PHY and ETH when not in use

2012-11-23 Thread Andy Green

On 11/24/12 00:25, the mail apparently from Alan Stern included:

On Fri, 23 Nov 2012, Felipe Balbi wrote:


Thanks for the example. The only problem is, how do we associate a
regulator to a specific port in the system.


heh, that's the 1 million dollar question, isn't it ? :-)

that's what we need to figure out now. Luckily we have Alan Stern
helping us out ;-)


Some people might think having me around to interfere was not so lucky
after all...  :-)

First question: Should we worry about individual ports?  Ordinarily I'd
say No, because hubs always power up all of their ports.  But with Lan
Tianyu's recent work, that won't be true any more.  In this case, it's
conceivable that the user might want to power-off the LAN95xx in order
to save energy, even though ehci-hcd is still loaded and managing other
USB devices.

Either way, the regulator has to be associated with _something_: either
a root-hub port or the host controller itself.  And this will most
likely have to be done before the port's or the controller's struct
device exists.

Something like Andy's scheme might work.  It would require the kernel
to parse name strings in various formats, which could get complicated.
It would be great if the difficult parts could be stuck in the PM core
somewhere.


If we're just looking at fixing the current "magic regulator name" 
scheme of "hsusb0" so that it can work with abstract devices like any 
hub / port, we could invert what my original "device path" scheme did.


So instead of having a parser (which boiled down quite small, but is 
complicated by usb%d being the same for different usb drivers), we could 
just add a helper function that walks the device parents to generate a 
string representing the device instance.  Like


int device_path_generate(struct device *device, char *name, int len);

if you called it from the hub driver's probe function (or anything 
else's probe function) with the new hub device pointer, it might fill 
name with "ehci1/usbhub1-1/1-1.1" or somesuch.


That is then used in the hub probe function as the magic regulator name, 
if a regulator exists of that name it gets managed according to logical 
hub device lifecycle, same as the omap bit does at the moment.


That way it'll work with DT alright (you just arrange the regulator name 
to be "ehci1/usbhub1-1/1-1.1" or whatever) and you're just trying to 
sell device_path_generate() to someone else, which should be pretty small.


-Andy

--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106  - 
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog

--
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: Mouse and keyboard freeze Ivy-bridge

2012-11-23 Thread Chris Holland
On Fri, Nov 23, 2012 at 11:03 AM, Alan Stern  wrote:
> On Thu, 22 Nov 2012, Chris Holland wrote:
>
>> https://bugzilla.kernel.org/show_bug.cgi?id=50821
>>
>>
>> Summary: Mouse and keyboard freeze Ivy-bridge
>> I have been having the problem of the mouse and keyboard freezing since
>> upgrading my system in May 2012
>>
>> GIGABYTE GA-Z77X-UD5H LGA 1155 Intel Z77 HDMI SATA
>> Intel Core i5-3550 Ivy Bridge
>>
>>
>> Mouse and keyboard stop responding randomly. By using only the usb 
>> connections
>> on the motherboard it requires logging in by ssh to restart the computer or
>> hard resetting. Unplugging and plugging the mouse or keyboard in has no 
>> effect
>> on the issue.
>
> Which USB controller were the mouse and keyboard plugged into?

They were plugged into the added pci controller. I have 2 sets of mice
and keyboards plugged in 1 set to the pci card and 1 set to
the motherboard.

It does not matter if its usb2 or usb3 ports you have them plugged
into the motherboard when this happens they stop responding.
And unplugging replugging has no effect. And running modprobe -r
ehci_hcd && modprobe ehci_hcd results in
FATAL: Module ehci_hcd is builtin


>
>> I added a pci usb controller and this stops working as well but you can 
>> unplug
>> and plug the devices back in and continue to use the computer.
>
> In the logs you attached to the bugzilla.kernel.org report, the
> keyboard and mouse were attached to the OHCI controller at
> :06:00.0.  Is that the PCI controller?

Yes. Attached lspci

>
> If you collect a usbmon trace for that bus (bus 4 in the attached
> logs), what does it show when the problem occurs?

Dont have a clue but I will into it.

>
> If you run a kernel that has CONFIG_USB_DEBUG enabled, what shows up in
> the dmesg log when the problem occurs?

I dont have a debug kernel but I will try and build one. I am not the
most advanced Linux user around.

>
> Alan Stern
>

Chris Holland


lspci
Description: Binary data


[PATCH 24/24] MAINTAINERS: remove drivers/block/ub.c

2012-11-23 Thread Cesar Eduardo Barros
This driver was removed by commit 68a5059 (block: remove the deprecated
ub driver).

Cc: Pete Zaitcev 
Cc: linux-usb@vger.kernel.org
Signed-off-by: Cesar Eduardo Barros 
---
 MAINTAINERS | 6 --
 1 file changed, 6 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index e8990d2..f4934bd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7624,12 +7624,6 @@ L:   linux-s...@vger.kernel.org
 S: Supported
 F: drivers/usb/storage/uas.c
 
-USB BLOCK DRIVER (UB ub)
-M: Pete Zaitcev 
-L: linux-usb@vger.kernel.org
-S: Supported
-F: drivers/block/ub.c
-
 USB CDC ETHERNET DRIVER
 M: Oliver Neukum 
 L: linux-usb@vger.kernel.org
-- 
1.7.11.7

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