RE: [PATCH 05/25] libusbg: Update strings only when writting US English strings.

[Krzysztof Opasiak]

> -Original Message-
> From: Sergei Shtylyov [mailto:sergei.shtyl...@cogentembedded.com]
> Sent: Wednesday, February 19, 2014 12:10 AM
> To: Krzysztof Opasiak; mpor...@linaro.org; linux-
> u...@vger.kernel.org
> Cc: Andrzej Pietrasiewicz; Karol Lewandowski; Stanislaw Wadas;
> Aleksander Zdyb; ty317@samsung.com; Marek Szyprowski; Robert
> Baldyga
> Subject: Re: [PATCH 05/25] libusbg: Update strings only when
> writting US English strings.
> 
> Hello.
> 
> On 02/19/2014 02:07 AM, Sergei Shtylyov wrote:
> 
> >> Strings in current verison of library are hardcoded to
> >> US English. Functions which set strings are generic and
> >> allow to set other languages, but internal library structures
> >> should be update only when setting US English strings.
> 
> >> Signed-off-by: Krzysztof Opasiak 
> [...]
> 
> > I guess you haven't run your patch via scripts/checkpatch.pl,
> otherwise
> > you would have seen it protesting against single statement *if*
> arms in {}.
> > Well, some common sense applies as well since {} are completely
> unnecessary.
> 
> Ah, I have initially overlooked that it's libusbg patch --
> libusbg may
> have its own style peculiarities.

Assuming Matt prefer kernel style comments, rest of the coding
style should be also consistent with kernel. I have been using some
framework which surrounds single if statements with {}, so I was used to
that style. I will fix this for v2.

Thank you for your remarks.

-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
k.opas...@samsung.com




--
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 net-next 12/14] r8152: replace netif_rx withnetif_receive_skb

[Francois Romieu]

hayeswang  :
>  Francois Romieu [mailto:rom...@fr.zoreil.com] 
> > Hayes Wang  :
> > > Replace netif_rx with netif_receive_skb to avoid disabling irq frequently
> > > for increasing the efficiency.
> > 
> > read_bulk_callback is issued in irq context. It could thus use plain
> > spin_lock / spin_unlock instead of the irq disabling version.
> 
> The rx_bottom() is called in tasklet, so I just think I could use
> netif_receive_skb directly. The netif_rx seems to queue the packet,
> and local_irq_disable() would be called before dequeuing the skb.

The change in rx_bottom is fine. My point is about read_bulk_callback.

rx_bottom races with read_bulk_callback. rx_bottom is issued in
tasklet (softirq) context. read_bulk_callback is issued in irq
context, with irq disabled. read_bulk_callback does not need to
disable irq itself and could go with spin_lock in place of
spin_lock_irqsave (rx_bottom can't, of course).

-- 
Ueimor
--
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 15/25] libusbg: Add getter for config name.

[Krzysztof Opasiak]

Hello,

> > +/**
> > + * @brieg Get config name
> > + * @param c Pointer to config
> > + * @param buf Buffer where name should be copied
> > + * @param len Length of given buffer
> > + * @return Pointer to destination or NULL if error occurred.
> > + */
> > +extern char *usbg_get_config_name(struct config* c, char *buf,
> size_t len);
> 
> Be consistent in your coding style: always put * next to the
> variable name
> (this is kernel style though).

True. My fault - some typo and copy pasta. Will be fixed for v2.

Thanks,
Krzysztof Opasiak


--
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: fix NULL pointer dereference

[Andrzej Pietrasiewicz]

W dniu 18.02.2014 17:40, Felipe Balbi pisze:

On Fri, Jan 17, 2014 at 05:04:55PM +0100, Michal Nazarewicz wrote:

On Thu, Jan 16 2014, Andrzej Pietrasiewicz wrote:

Fix possible NULL pointer dereference introduced in

219580e64f035bb9018dbb08d340f90b0ac50f8c
usb: f_fs: check quirk to pad epout buf size when not aligned to
maxpacketsize

after 3.13-rc1.

In cases we do wait with:

wait_event_interruptible(epfile->wait, (ep = epfile->ep));

for endpoint to be enabled, functionfs_bind() has not been called yet
and epfile->ffs->gadget is still NULL and the automatic variable 'gadget'
has been initialized with NULL at the point of its definition.
Later on it is used as a parameter to:

usb_ep_align_maybe(gadget, ep->ep, len)

which in turn dereferences it.

This patch fixes it by moving the actual assignment to the local 'gadget'
variable after the potential waiting has completed.

Signed-off-by: Andrzej Pietrasiewicz 


Acked-by: Michal Nazarewicz 

But since gadget is only used in the “if (!halt)” part of the code,
could you simply move definition of the variable inside the if?


should I wait for another revision ?



It has already been submitted:

http://www.spinics.net/lists/linux-usb/msg101199.html

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

[PATCH 1/1] usb: chipidea: need to mask when writting endptflush and endptprime

[Peter Chen]

From: Matthieu CASTET 

ENDPTFLUSH and ENDPTPRIME registers are set by software and clear
by hardware. There is a bit for each endpoint. When we are setting
a bit for an endpoint we should make sure we do not touch other
endpoint bit. There is a race condition if the hardware clear the
bit between the read and the write in hw_write.

This patch is useful for 3.11+.

Cc: stable 
Signed-off-by: Peter Chen 
Signed-off-by: Matthieu CASTET 
Tested-by: Michael Grzeschik 
---
 drivers/usb/chipidea/udc.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 80de2f8..4ab2cb6 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -105,7 +105,7 @@ static int hw_ep_flush(struct ci_hdrc *ci, int num, int dir)
 
do {
/* flush any pending transfer */
-   hw_write(ci, OP_ENDPTFLUSH, BIT(n), BIT(n));
+   hw_write(ci, OP_ENDPTFLUSH, ~0, BIT(n));
while (hw_read(ci, OP_ENDPTFLUSH, BIT(n)))
cpu_relax();
} while (hw_read(ci, OP_ENDPTSTAT, BIT(n)));
@@ -205,7 +205,7 @@ static int hw_ep_prime(struct ci_hdrc *ci, int num, int 
dir, int is_ctrl)
if (is_ctrl && dir == RX && hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
return -EAGAIN;
 
-   hw_write(ci, OP_ENDPTPRIME, BIT(n), BIT(n));
+   hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));
 
while (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
cpu_relax();
-- 
1.7.8


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

[PATCH 0/1] USB Chipidea Patches for 3.14

[Peter Chen]

Hi Greg,

Only one patch for 3.14 regression.

Peter

Matthieu CASTET (1):
  usb: chipidea: need to mask when writting endptflush and endptprime

 drivers/usb/chipidea/udc.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

-- 
1.7.8


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

[PATCH 1/1] usb: chipidea: need to mask when writting endptflush and endptprime

[Peter Chen]

From: Matthieu CASTET 

ENDPTFLUSH and ENDPTPRIME registers are set by software and clear
by hardware. There is a bit for each endpoint. When we are setting
a bit for an endpoint we should make sure we do not touch other
endpoint bit. There is a race condition if the hardware clear the
bit between the read and the write in hw_write.

This patch is useful for 3.11+.

Cc: stable 
Signed-off-by: Peter Chen 
Signed-off-by: Matthieu CASTET 
Tested-by: Michael Grzeschik 
---
 drivers/usb/chipidea/udc.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 80de2f8..4ab2cb6 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -105,7 +105,7 @@ static int hw_ep_flush(struct ci_hdrc *ci, int num, int dir)
 
do {
/* flush any pending transfer */
-   hw_write(ci, OP_ENDPTFLUSH, BIT(n), BIT(n));
+   hw_write(ci, OP_ENDPTFLUSH, ~0, BIT(n));
while (hw_read(ci, OP_ENDPTFLUSH, BIT(n)))
cpu_relax();
} while (hw_read(ci, OP_ENDPTSTAT, BIT(n)));
@@ -205,7 +205,7 @@ static int hw_ep_prime(struct ci_hdrc *ci, int num, int 
dir, int is_ctrl)
if (is_ctrl && dir == RX && hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
return -EAGAIN;
 
-   hw_write(ci, OP_ENDPTPRIME, BIT(n), BIT(n));
+   hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));
 
while (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
cpu_relax();
-- 
1.7.8


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

[PATCH 2/5] usb: chipidea: udc: refine ep operation at isr_tr_complete_handler

[Peter Chen]

- delete the warning message at interrupt handler, and adds judgement at
ep_enable, if non-ep0 requests ctrl transfer, it will indicate an error.
- delete hw_test_and_clear_setup_status which is a broken code
- Tested with g_mass_storage, g_ncm, g_ether

Cc: matthieu.cas...@parrot.com
Reported-by: Michael Grzeschik 
Acked-by: Michael Grzeschik 
Tested-by: Michael Grzeschik 
Signed-off-by: Peter Chen 
---
 drivers/usb/chipidea/udc.c |   28 
 1 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 09366b4..fe30dcc 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -178,19 +178,6 @@ static int hw_ep_get_halt(struct ci_hdrc *ci, int num, int 
dir)
 }
 
 /**
- * hw_test_and_clear_setup_status: test & clear setup status (execute without
- * interruption)
- * @n: endpoint number
- *
- * This function returns setup status
- */
-static int hw_test_and_clear_setup_status(struct ci_hdrc *ci, int n)
-{
-   n = ep_to_bit(ci, n);
-   return hw_test_and_clear(ci, OP_ENDPTSETUPSTAT, BIT(n));
-}
-
-/**
  * hw_ep_prime: primes endpoint (execute without interruption)
  * @num: endpoint number
  * @dir: endpoint direction
@@ -997,14 +984,10 @@ __acquires(ci->lock)
}
}
 
-   if (hwep->type != USB_ENDPOINT_XFER_CONTROL ||
-   !hw_test_and_clear_setup_status(ci, i))
-   continue;
-
-   if (i != 0) {
-   dev_warn(ci->dev, "ctrl traffic at endpoint %d\n", i);
+   /* Only handle setup packet below */
+   if (i != 0 ||
+   !hw_test_and_clear(ci, OP_ENDPTSETUPSTAT, BIT(0)))
continue;
-   }
 
/*
 * Flush data and handshake transactions of previous
@@ -1193,6 +1176,11 @@ static int ep_enable(struct usb_ep *ep,
 
hwep->qh.ptr->td.next |= cpu_to_le32(TD_TERMINATE);   /* needed? */
 
+   if (hwep->num != 0 && hwep->type == USB_ENDPOINT_XFER_CONTROL) {
+   dev_err(hwep->ci->dev, "Set control xfer at non-ep0\n");
+   retval = -EINVAL;
+   }
+
/*
 * Enable endpoints in the HW other than ep0 as ep0
 * is always enabled
-- 
1.7.8


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

[PATCH 0/5] USB Chipidea Patches for 3.15

[Peter Chen]

Hi Greg,

Below are chipidea patches for 3.15.
Mainly are for refining changes and forcing full-speed change.

Peter

Fabio Estevam (1):
  usb: chipidea: Propagate the real error code on platform_get_irq()
failure

Jingoo Han (1):
  usb: chipidea: use dev_get_platdata()

Michael Grzeschik (1):
  usb: chipidea: udc: add maximum-speed = full-speed option

Peter Chen (2):
  usb: chipidea: refine PHY operation
  usb: chipidea: udc: refine ep operation at isr_tr_complete_handler

 .../devicetree/bindings/usb/ci-hdrc-imx.txt|2 +
 drivers/usb/chipidea/bits.h|2 +
 drivers/usb/chipidea/ci.h  |2 -
 drivers/usb/chipidea/core.c|   87 +---
 drivers/usb/chipidea/udc.c |   34 ++--
 include/linux/usb/chipidea.h   |1 +
 6 files changed, 53 insertions(+), 75 deletions(-)

-- 
1.7.8


--
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 4/5] usb: chipidea: udc: add maximum-speed = full-speed option

[Peter Chen]

From: Michael Grzeschik 

This patch makes it possible to set the chipidea udc into full-speed only mode.
It is set by the oftree property "maximum-speed = full-speed".

Signed-off-by: Peter Chen 
Signed-off-by: Michael Grzeschik 
Signed-off-by: Marc Kleine-Budde 
---
 .../devicetree/bindings/usb/ci-hdrc-imx.txt|2 ++
 drivers/usb/chipidea/bits.h|2 ++
 drivers/usb/chipidea/core.c|   11 +++
 include/linux/usb/chipidea.h   |1 +
 4 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-imx.txt 
b/Documentation/devicetree/bindings/usb/ci-hdrc-imx.txt
index b4b5b79..a6a32cb 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-imx.txt
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-imx.txt
@@ -18,6 +18,7 @@ Optional properties:
 - vbus-supply: regulator for vbus
 - disable-over-current: disable over current detect
 - external-vbus-divider: enables off-chip resistor divider for Vbus
+- maximum-speed: limit the maximum connection speed to "full-speed".
 
 Examples:
 usb@02184000 { /* USB OTG */
@@ -28,4 +29,5 @@ usb@02184000 { /* USB OTG */
fsl,usbmisc = <&usbmisc 0>;
disable-over-current;
external-vbus-divider;
+   maximum-speed = "full-speed";
 };
diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
index a857131..83d06c1 100644
--- a/drivers/usb/chipidea/bits.h
+++ b/drivers/usb/chipidea/bits.h
@@ -50,12 +50,14 @@
 #define PORTSC_PTC(0x0FUL << 16)
 #define PORTSC_PHCD(d)   ((d) ? BIT(22) : BIT(23))
 /* PTS and PTW for non lpm version only */
+#define PORTSC_PFSC   BIT(24)
 #define PORTSC_PTS(d)  \
(u32)d) & 0x3) << 30) | (((d) & 0x4) ? BIT(25) : 0))
 #define PORTSC_PTWBIT(28)
 #define PORTSC_STSBIT(29)
 
 /* DEVLC */
+#define DEVLC_PFSCBIT(23)
 #define DEVLC_PSPD(0x03UL << 25)
 #define DEVLC_PSPD_HS (0x02UL << 25)
 #define DEVLC_PTW BIT(27)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 47b4bd8..65aeaac 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -64,6 +64,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -298,6 +299,13 @@ int hw_device_reset(struct ci_hdrc *ci, u32 mode)
if (ci->platdata->flags & CI_HDRC_DISABLE_STREAMING)
hw_write(ci, OP_USBMODE, USBMODE_CI_SDIS, USBMODE_CI_SDIS);
 
+   if (ci->platdata->flags & CI_HDRC_FORCE_FULLSPEED) {
+   if (ci->hw_bank.lpm)
+   hw_write(ci, OP_DEVLC, DEVLC_PFSC, DEVLC_PFSC);
+   else
+   hw_write(ci, OP_PORTSC, PORTSC_PFSC, PORTSC_PFSC);
+   }
+
/* USBMODE should be configured step by step */
hw_write(ci, OP_USBMODE, USBMODE_CM, USBMODE_CM_IDLE);
hw_write(ci, OP_USBMODE, USBMODE_CM, mode);
@@ -412,6 +420,9 @@ static int ci_get_platdata(struct device *dev,
}
}
 
+   if (of_usb_get_maximum_speed(dev->of_node) == USB_SPEED_FULL)
+   platdata->flags |= CI_HDRC_FORCE_FULLSPEED;
+
return 0;
 }
 
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 708bd11..bbe779f 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -25,6 +25,7 @@ struct ci_hdrc_platform_data {
 */
 #define CI_HDRC_DUAL_ROLE_NOT_OTG  BIT(4)
 #define CI_HDRC_IMX28_WRITE_FIXBIT(5)
+#define CI_HDRC_FORCE_FULLSPEEDBIT(6)
enum usb_dr_modedr_mode;
 #define CI_HDRC_CONTROLLER_RESET_EVENT 0
 #define CI_HDRC_CONTROLLER_STOPPED_EVENT   1
-- 
1.7.8


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

[PATCH 3/5] usb: chipidea: use dev_get_platdata()

[Peter Chen]

From: Jingoo Han 

Use the wrapper function for retrieving the platform data instead
of accessing dev->platform_data directly. This is a cosmetic change
to make the code simpler and enhance the readability.

Signed-off-by: Peter Chen 
Signed-off-by: Jingoo Han 
---
 drivers/usb/chipidea/core.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 75aaa9c..47b4bd8 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -505,7 +505,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
int ret;
enum usb_dr_mode dr_mode;
 
-   if (!dev->platform_data) {
+   if (!dev_get_platdata(dev)) {
dev_err(dev, "platform data missing\n");
return -ENODEV;
}
@@ -522,7 +522,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
}
 
ci->dev = dev;
-   ci->platdata = dev->platform_data;
+   ci->platdata = dev_get_platdata(dev);
ci->imx28_write_fix = !!(ci->platdata->flags &
CI_HDRC_IMX28_WRITE_FIX);
 
-- 
1.7.8


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

[PATCH 1/5] usb: chipidea: refine PHY operation

[Peter Chen]

- Delete global_phy due to we can get the phy from phy layer now
- using devm_usb_get_phy to instead of usb_get_phy
- delete the otg_set_peripheral, which should be handled by otg layer

Signed-off-by: Peter Chen 
---
 drivers/usb/chipidea/ci.h   |2 -
 drivers/usb/chipidea/core.c |   70 ---
 drivers/usb/chipidea/udc.c  |6 
 3 files changed, 26 insertions(+), 52 deletions(-)

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 88b80f7..e206406 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -196,8 +196,6 @@ struct ci_hdrc {
 
struct ci_hdrc_platform_data*platdata;
int vbus_active;
-   /* FIXME: some day, we'll not use global phy */
-   boolglobal_phy;
struct usb_phy  *transceiver;
struct usb_hcd  *hcd;
struct dentry   *debugfs;
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 33f22bc..75aaa9c 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -496,33 +496,6 @@ static void ci_get_otg_capable(struct ci_hdrc *ci)
}
 }
 
-static int ci_usb_phy_init(struct ci_hdrc *ci)
-{
-   if (ci->platdata->phy) {
-   ci->transceiver = ci->platdata->phy;
-   return usb_phy_init(ci->transceiver);
-   } else {
-   ci->global_phy = true;
-   ci->transceiver = usb_get_phy(USB_PHY_TYPE_USB2);
-   if (IS_ERR(ci->transceiver))
-   ci->transceiver = NULL;
-
-   return 0;
-   }
-}
-
-static void ci_usb_phy_destroy(struct ci_hdrc *ci)
-{
-   if (!ci->transceiver)
-   return;
-
-   otg_set_peripheral(ci->transceiver->otg, NULL);
-   if (ci->global_phy)
-   usb_put_phy(ci->transceiver);
-   else
-   usb_phy_shutdown(ci->transceiver);
-}
-
 static int ci_hdrc_probe(struct platform_device *pdev)
 {
struct device   *dev = &pdev->dev;
@@ -561,7 +534,26 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 
hw_phymode_configure(ci);
 
-   ret = ci_usb_phy_init(ci);
+   if (ci->platdata->phy)
+   ci->transceiver = ci->platdata->phy;
+   else
+   ci->transceiver = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
+
+   if (IS_ERR(ci->transceiver)) {
+   ret = PTR_ERR(ci->transceiver);
+   /*
+* if -ENXIO is returned, it means PHY layer wasn't
+* enabled, so it makes no sense to return -EPROBE_DEFER
+* in that case, since no PHY driver will ever probe.
+*/
+   if (ret == -ENXIO)
+   return ret;
+
+   dev_err(dev, "no usb2 phy configured\n");
+   return -EPROBE_DEFER;
+   }
+
+   ret = usb_phy_init(ci->transceiver);
if (ret) {
dev_err(dev, "unable to init phy: %d\n", ret);
return ret;
@@ -573,7 +565,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
if (ci->irq < 0) {
dev_err(dev, "missing IRQ\n");
ret = -ENODEV;
-   goto destroy_phy;
+   goto deinit_phy;
}
 
ci_get_otg_capable(ci);
@@ -590,23 +582,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
ret = ci_hdrc_gadget_init(ci);
if (ret)
dev_info(dev, "doesn't support gadget\n");
-   if (!ret && ci->transceiver) {
-   ret = otg_set_peripheral(ci->transceiver->otg,
-   &ci->gadget);
-   /*
-* If we implement all USB functions using chipidea 
drivers,
-* it doesn't need to call above API, meanwhile, if we 
only
-* use gadget function, calling above API is useless.
-*/
-   if (ret && ret != -ENOTSUPP)
-   goto destroy_phy;
-   }
}
 
if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
dev_err(dev, "no supported roles\n");
ret = -ENODEV;
-   goto destroy_phy;
+   goto deinit_phy;
}
 
if (ci->is_otg) {
@@ -663,8 +644,8 @@ static int ci_hdrc_probe(struct platform_device *pdev)
free_irq(ci->irq, ci);
 stop:
ci_role_destroy(ci);
-destroy_phy:
-   ci_usb_phy_destroy(ci);
+deinit_phy:
+   usb_phy_shutdown(ci->transceiver);
 
return ret;
 }
@@ -677,7 +658,8 @@ static int ci_hdrc_remove(struct platform_device *pdev)
free_irq(ci->irq, ci);
ci_role_destroy(ci);
ci_hdrc_enter_lpm(ci, true);
-   ci_usb_phy_destroy(ci);
+   usb_phy_shutdown(ci

[PATCH 5/5] usb: chipidea: Propagate the real error code on platform_get_irq() failure

[Peter Chen]

From: Fabio Estevam 

No need to return a 'fake' return value on platform_get_irq() failure.

Just return the error code itself instead.

Signed-off-by: Peter Chen 
Signed-off-by: Fabio Estevam 
---
 drivers/usb/chipidea/core.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 65aeaac..ca6831c 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -575,7 +575,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
ci->irq = platform_get_irq(pdev, 0);
if (ci->irq < 0) {
dev_err(dev, "missing IRQ\n");
-   ret = -ENODEV;
+   ret = ci->irq;
goto deinit_phy;
}
 
-- 
1.7.8


--
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] chipidea: core: Propagate the real error code on platform_get_irq() failure

[Peter Chen]

On Mon, Feb 17, 2014 at 07:12:53PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam 
> 
> No need to return a 'fake' return value on platform_get_irq() failure.
> 
> Just return the error code itself instead.
> 
> Signed-off-by: Fabio Estevam 
> ---
>  drivers/usb/chipidea/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 33f22bc..5590f08 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -572,7 +572,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>   ci->irq = platform_get_irq(pdev, 0);
>   if (ci->irq < 0) {
>   dev_err(dev, "missing IRQ\n");
> - ret = -ENODEV;
> + ret = ci->irq;
>   goto destroy_phy;
>   }
>  
> -- 

Change subject prefix to "usb: chipidea", and one conflict with
my next tree, applied, thanks.

-- 

Best Regards,
Peter Chen

--
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: phy: msm: fix possible build error

[Stephen Boyd]

On 02/18, Felipe Balbi wrote:
> This will fail builds on configs where
> CONFIG_PM_RUNTIME=y and CONFIG_PM_SLEEP=n.
> 
> Following build error will show up:
> 
>  drivers/usb/phy/phy-msm-usb.c: In function ???msm_otg_runtime_suspend???:
>  drivers/usb/phy/phy-msm-usb.c:1693:2: error: implicit declaration of \
>   function ???msm_otg_suspend??? [-Werror=implicit-function-declaration]
>return msm_otg_suspend(motg);
>^
>  drivers/usb/phy/phy-msm-usb.c: In function ???msm_otg_runtime_resume???:
>  drivers/usb/phy/phy-msm-usb.c:1701:2: error: implicit declaration of \
>   function ???msm_otg_resume??? [-Werror=implicit-function-declaration]
>return msm_otg_resume(motg);
>^
> 
> This patch fixes the error by defining msm_otg_{suspend,resume}
> whenever CONFIG_PM=y.
> 
> Signed-off-by: Felipe Balbi 

I'm lost. Didn't Josh send a patch for this to you already?

[1] https://patchwork.kernel.org/patch/3673401/

> ---
>  drivers/usb/phy/phy-msm-usb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
> index 64c9d14e..96f31aa 100644
> --- a/drivers/usb/phy/phy-msm-usb.c
> +++ b/drivers/usb/phy/phy-msm-usb.c
> @@ -159,7 +159,7 @@ put_3p3:
>   return rc;
>  }
>  
> -#ifdef CONFIG_PM_SLEEP
> +#ifdef CONFIG_PM
>  #define USB_PHY_SUSP_DIG_VOL  50
>  static int msm_hsusb_config_vddcx(int high)
>  {
> @@ -440,7 +440,7 @@ static int msm_otg_reset(struct usb_phy *phy)
>  #define PHY_SUSPEND_TIMEOUT_USEC (500 * 1000)
>  #define PHY_RESUME_TIMEOUT_USEC  (100 * 1000)
>  
> -#ifdef CONFIG_PM_SLEEP
> +#ifdef CONFIG_PM
>  static int msm_otg_suspend(struct msm_otg *motg)
>  {
>   struct usb_phy *phy = &motg->phy;
> -- 
> 1.9.0
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
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 v2 0/3] usb: chipidea: msm: Clean and fix glue layer driver

[Tim Bird]

Ivan,

I'm having tremendous problems getting this driver to initialize.  For
some reason, I can't get the driver to actually transition the
hardware into peripheral mode.  At first I was getting a lot of probe
deferrals, based on not finding the regulators early enough in the
boot, and I thought it was an issue with the gadget drivers loading
before the driver could complete its setup.  However, I switched
everything to loading via modules, and now have less probe deferrals,
but I still can't get the driver to activate.  I see zero interrupts.
In particular the routine hw_device_state (which turns on interrupts
in the controller) is never called, because I can't get
msm_otg_start_peripheral to actually kick the hardware.

I've sprinkled the code in drivers/usb/chipidea and
drivers/usb/phy/phy-msm-usb.c liberally with printks and WARNs to help
me see what's going on, but I'm having a hard time tracing it down.
I'm pretty sure I've got the DTS correct, but my USB config might not
match yours.  (Would you mind sharing your config?).

I tried configuring the qcom,otg-control for user controlled mode
setting (via debugfs), and even with doing "echo "peripheral"
>/sys/kernel/debug/msm_otg/mode, it just wouldn't start the hardware
(call hw_device_state(...1)).

Any ideas you can provide would be welcome (e.g. for things to try,
look at, etc.)

My kernel is based on an internal Sony 3.13-rc6 kernel with clock and
regulator patches applied, as well as your phy-msm-usb.c patches from
November and your chipidea patches from yesterday.

Thanks,
 -- Tim


In the printk dump below,
  UBTO=udc_bind_to_driver
  CIS=ci_udc_start
  MOSP=msm_otg_set_peripheral
  MOSW=msm_otg_sm_work

[10] platform_init()
[10] target_init()
[10] Display Init: Start
[10] display_init(),target_id=10.
[30] Config MIPI_VIDEO_PANEL.
[30] Turn on MIPI_VIDEO_PANEL.
[50] Video lane tested successfully
[50] Display Init: Done
[70] partition misc doesn't exist
[80] error in emmc_recovery_init
[80] No 'misc' partition found
[80] Error reading MISC partition
[80] failed to get ffbm cookie[90] use_signed_kernel=1, is_unlocked=0,
is_tampered=1.
[90] Loading boot image (7829504): start
[550] Loading boot image (7829504): done
[550] Found Appeneded Flattened Device tree
[550] cmdline: console=ttyMSM,115200,n8 androidboot.hardware=qcom
user_debug=31 maxcpus=2 msm_rtb.filter=0x37 ehci-hcd.park=3 earl
yprintk debug androidboot.emmc=true androidboot.serialno=40081a14
androidboot.baseband=apq
[570] Updating device tree: start
[570] Updating device tree: done
[580] booting linux @ 0x8000, ramdisk @ 0x200 (4234892),
tags/device tree @ 0x1e0
[580] Turn off MIPI_VIDEO_PANEL.
[580] Continuous splash enabled, keeping panel alive.
Uncompressing Linux... done, booting the kernel.
[0.00] Booting Linux on physical CPU 0x0
[0.00] TRB: version 8
[0.00] Linux version 3.13.0-rc6-00147-g00bb56a-dirty
(10102229@ussvlx8980) (gcc version 4.6.x-google 20120106 (prerelease)
 (GCC) ) #40 SMP PREEMPT Tue Feb 18 19:24:12 PST 2014
[0.00] CPU: ARMv7 Processor [512f06f0] revision 0 (ARMv7), cr=10c5387d
[0.00] CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache
[0.00] Machine model: Qualcomm APQ8074 Dragonboard
[0.00] bootconsole [earlycon0] enabled
[0.00] Memory policy: Data cache writealloc
[0.00] On node 0 totalpages: 524288
[0.00] free_area_init_node: node 0, pgdat c0908d80,
node_mem_map c098
[0.00]   Normal zone: 1520 pages used for memmap
[0.00]   Normal zone: 0 pages reserved
[0.00]   Normal zone: 194560 pages, LIFO batch:31
[0.00]   HighMem zone: 2576 pages used for memmap
[0.00]   HighMem zone: 329728 pages, LIFO batch:31
[0.00] PERCPU: Embedded 8 pages/cpu @c1993000 s12224 r8192 d12352 u32768
[0.00] pcpu-alloc: s12224 r8192 d12352 u32768 alloc=8*4096
[0.00] pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3
[0.00] Built 1 zonelists in Zone order, mobility grouping on.
Total pages: 522768
[0.00] Kernel command line: console=ttyMSM,115200,n8
androidboot.hardware=qcom user_debug=31 maxcpus=2 msm_rtb.filter=0x37
 ehci-hcd.park=3 earlyprintk debug androidboot.emmc=true
androidboot.serialno=40081a14 androidboot.baseband=apq
[0.00] PID hash table entries: 4096 (order: 2, 16384 bytes)
[0.00] Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
[0.00] Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
[0.00] Memory: 2067932K/2097152K available (4734K kernel code,
262K rwdata, 1912K rodata, 287K init, 446K bss, 29220K rese
rved, 1318912K highmem)
[0.00] Virtual kernel memory layout:
[0.00] vector  : 0x - 0x1000   (   4 kB)
[0.00] fixmap  : 0xfff0 - 0xfffe   ( 896 kB)
[0.00] vmalloc : 0xf000 - 0xff00   ( 240 MB)
[0.00] lowmem  : 0xc000 - 0xef80   ( 760 MB)
[0.00]

RE: [PATCH v2 1/6] usb: otg-fsm: add HNP polling operation function.

[Peter Chen]

 
> On Mon, Jan 20, 2014 at 10:00:15AM +0800, Li Jun wrote:
> > This patch adds HNP polling operation function for OTG fsm.
> >
> > Signed-off-by: Li Jun 
> > ---
> >  drivers/usb/phy/phy-fsm-usb.c |2 ++
> >  include/linux/usb/otg-fsm.h   |9 +
> >  2 files changed, 11 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/usb/phy/phy-fsm-usb.c
> > b/drivers/usb/phy/phy-fsm-usb.c index c47e5a6..ef91961 100644
> > --- a/drivers/usb/phy/phy-fsm-usb.c
> > +++ b/drivers/usb/phy/phy-fsm-usb.c
> > @@ -169,6 +169,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum
> usb_otg_state new_state)
> > otg_set_protocol(fsm, PROTO_HOST);
> > usb_bus_start_enum(fsm->otg->host,
> > fsm->otg->host->otg_port);
> > +   otg_start_hnp_polling(fsm);
> > break;
> > case OTG_STATE_A_IDLE:
> > otg_drv_vbus(fsm, 0);
> > @@ -203,6 +204,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum
> usb_otg_state new_state)
> >  */
> > if (!fsm->a_bus_req || fsm->a_suspend_req_inf)
> > otg_add_timer(fsm, A_WAIT_ENUM);
> > +   otg_start_hnp_polling(fsm);
> > break;
> > case OTG_STATE_A_SUSPEND:
> > otg_drv_vbus(fsm, 1);
> > diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
> > index b6ba1bf..79c6ee8 100644
> > --- a/include/linux/usb/otg-fsm.h
> > +++ b/include/linux/usb/otg-fsm.h
> > @@ -127,6 +127,7 @@ struct otg_fsm_ops {
> > void(*start_pulse)(struct otg_fsm *fsm);
> > void(*start_adp_prb)(struct otg_fsm *fsm);
> > void(*start_adp_sns)(struct otg_fsm *fsm);
> > +   void(*start_hnp_polling)(struct otg_fsm *fsm);
> 
> why ? HNP polling is a generic operation, is it not ? Which means you
> shouldn't need to add this function pointer here, just implement a
> generic helper function, ideally in usb-common.c
> 

Only OTG capable device will use hnp polling, why it is a generic operation?

Peter

> Also, I need to see a patch adding proper kernel doc to those structures.
> 
> cheers
> 
> --
> balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] usb: phy: msm: fix possible build error

[Peter Chen]

 
> 
> Following build error will show up:
> 
>  drivers/usb/phy/phy-msm-usb.c: In function ‘msm_otg_runtime_suspend’:
>  drivers/usb/phy/phy-msm-usb.c:1693:2: error: implicit declaration of \
>   function ‘msm_otg_suspend’ [-Werror=implicit-function-declaration]
>return msm_otg_suspend(motg);
>^
>  drivers/usb/phy/phy-msm-usb.c: In function ‘msm_otg_runtime_resume’:
>  drivers/usb/phy/phy-msm-usb.c:1701:2: error: implicit declaration of \
>   function ‘msm_otg_resume’ [-Werror=implicit-function-declaration]
>return msm_otg_resume(motg);
>^
> 
> This patch fixes the error by defining msm_otg_{suspend,resume} whenever
> CONFIG_PM=y.
> 
> Signed-off-by: Felipe Balbi 
> ---
>  drivers/usb/phy/phy-msm-usb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-
> usb.c index 64c9d14e..96f31aa 100644
> --- a/drivers/usb/phy/phy-msm-usb.c
> +++ b/drivers/usb/phy/phy-msm-usb.c
> @@ -159,7 +159,7 @@ put_3p3:
>   return rc;
>  }
> 
> -#ifdef CONFIG_PM_SLEEP
> +#ifdef CONFIG_PM
>  #define USB_PHY_SUSP_DIG_VOL  50
>  static int msm_hsusb_config_vddcx(int high)  { @@ -440,7 +440,7 @@
> static int msm_otg_reset(struct usb_phy *phy)
>  #define PHY_SUSPEND_TIMEOUT_USEC (500 * 1000)
>  #define PHY_RESUME_TIMEOUT_USEC  (100 * 1000)
> 
> -#ifdef CONFIG_PM_SLEEP
> +#ifdef CONFIG_PM
>  static int msm_otg_suspend(struct msm_otg *motg)  {
>   struct usb_phy *phy = &motg->phy;
> --
> 1.9.0

Reviewed-by: Peter Chen 

This problem is due to msm_otg_resume/msm_otg_suspend are used
at both system and runtime pm routine. So we need to use
CONFIG_PM to instead of CONFIG_PM_SLEEP.

At my phy-mxs-usb.c patch, the suspend/resume API are only used
at system suspend/resume routine.

Peter

[PATCH] usb: phy: msm: fix possible build error

[Felipe Balbi]

This will fail builds on configs where
CONFIG_PM_RUNTIME=y and CONFIG_PM_SLEEP=n.

Following build error will show up:

 drivers/usb/phy/phy-msm-usb.c: In function ‘msm_otg_runtime_suspend’:
 drivers/usb/phy/phy-msm-usb.c:1693:2: error: implicit declaration of \
function ‘msm_otg_suspend’ [-Werror=implicit-function-declaration]
   return msm_otg_suspend(motg);
   ^
 drivers/usb/phy/phy-msm-usb.c: In function ‘msm_otg_runtime_resume’:
 drivers/usb/phy/phy-msm-usb.c:1701:2: error: implicit declaration of \
function ‘msm_otg_resume’ [-Werror=implicit-function-declaration]
   return msm_otg_resume(motg);
   ^

This patch fixes the error by defining msm_otg_{suspend,resume}
whenever CONFIG_PM=y.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/phy/phy-msm-usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 64c9d14e..96f31aa 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -159,7 +159,7 @@ put_3p3:
return rc;
 }
 
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM
 #define USB_PHY_SUSP_DIG_VOL  50
 static int msm_hsusb_config_vddcx(int high)
 {
@@ -440,7 +440,7 @@ static int msm_otg_reset(struct usb_phy *phy)
 #define PHY_SUSPEND_TIMEOUT_USEC   (500 * 1000)
 #define PHY_RESUME_TIMEOUT_USEC(100 * 1000)
 
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM
 static int msm_otg_suspend(struct msm_otg *motg)
 {
struct usb_phy *phy = &motg->phy;
-- 
1.9.0

--
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 Resend 2/2] usb: gadget: s3c-hsudc: Remove unused label

[Sachin Kamat]

On 18 February 2014 20:58, Felipe Balbi  wrote:
> On Mon, Feb 03, 2014 at 01:59:39PM +0530, Sachin Kamat wrote:
>> Fixes the following compilation warning:
>> drivers/usb/gadget/s3c-hsudc.c: In function 's3c_hsudc_probe':
>> drivers/usb/gadget/s3c-hsudc.c:1347:1: warning: label 'err_add_device'
>> defined but not used [-Wunused-label]
>>
>> Signed-off-by: Sachin Kamat 
>
> What about this one ?

This patch too is needed but can go in for 3.15.

-- 
With warm regards,
Sachin
--
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 Resend 1/2] usb: gadget: s3c2410_udc: Fix build error

[Sachin Kamat]

On 18 February 2014 20:58, Felipe Balbi  wrote:
> On Mon, Feb 03, 2014 at 01:59:38PM +0530, Sachin Kamat wrote:
>> Pass value instead of address as expected by 'usb_ep_set_maxpacket_limit'.
>> Fixes the following compilation error introduced by commit e117e742d310
>> ("usb: gadget: add "maxpacket_limit" field to struct usb_ep"):
>>
>> drivers/usb/gadget/s3c2410_udc.c: In function 's3c2410_udc_reinit':
>> drivers/usb/gadget/s3c2410_udc.c:1632:3: error:
>> cannot take address of bit-field 'maxpacket'
>>usb_ep_set_maxpacket_limit(&ep->ep, &ep->ep.maxpacket);
>>
>> Signed-off-by: Sachin Kamat 
>> Reviewed-by: Robert Baldyga 
>
> is this still needed for -rc4 ?

I just verified on latest linux-next and we still get build errors
without this patch.
Hence, yes this patch is required for the current rc.

-- 
With warm regards,
Sachin
--
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 net-next 12/14] r8152: replace netif_rx withnetif_receive_skb

[hayeswang]

 Francois Romieu [mailto:rom...@fr.zoreil.com] 
> Sent: Wednesday, February 19, 2014 7:29 AM
> To: Hayes Wang
> Cc: net...@vger.kernel.org; nic_s...@realtek.com; 
> linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org
> Subject: Re: [PATCH net-next 12/14] r8152: replace netif_rx 
> withnetif_receive_skb
> 
> Hayes Wang  :
> > Replace netif_rx with netif_receive_skb to avoid disabling irq frequently
> > for increasing the efficiency.
> 
> read_bulk_callback is issued in irq context. It could thus use plain
> spin_lock / spin_unlock instead of the irq disabling version.

The rx_bottom() is called in tasklet, so I just think I could use
netif_receive_skb directly. The netif_rx seems to queue the packet,
and local_irq_disable() would be called before dequeuing the skb.
 
Best Regards,
Hayes

--
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 net-next 07/14] r8152: combine PHY reset with set_speed

[hayeswang]

 Florian Fainelli [mailto:f.faine...@gmail.com] 
> Sent: Wednesday, February 19, 2014 1:19 AM
> To: Hayes Wang
> Cc: netdev; nic_s...@realtek.com; 
> linux-ker...@vger.kernel.org; linux-usb
> Subject: Re: [PATCH net-next 07/14] r8152: combine PHY reset 
> with set_speed
[...]
> > +static void rtl_phy_reset(struct r8152 *tp)
> > +{
> > +   u16 data;
> > +   int i;
> > +
> > +   clear_bit(PHY_RESET, &tp->flags);
> > +
> > +   data = r8152_mdio_read(tp, MII_BMCR);
> > +
> > +   /* don't reset again before the previous one complete */
> > +   if (data & BMCR_RESET)
> > +   return;
> > +
> > +   data |= BMCR_RESET;
> > +   r8152_mdio_write(tp, MII_BMCR, data);
> > +
> > +   for (i = 0; i < 50; i++) {
> > +   msleep(20);
> > +   if ((r8152_mdio_read(tp, MII_BMCR) & 
> BMCR_RESET) == 0)
> > +   break;
> > +   }
> > +}
> 
> If you implemented libphy in the driver you would not have to
> duplicate that and you could use "phy_init_hw()" or
> genphy_soft_reset() to perform the BMCR-based software reset.

Thanks for you suggestion. I would study about those.
 
Best Regards,
Hayes


--
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 2/2] USB: at91: using USBA_NR_DMAS for DMA channels

[Bo Shen]

The SoCs earlier than sama5d3, they have the same number endpoints
and DMA channels. In driver code, they use the same definition
USBA_NR_ENDPOINTS for both endpoints and dma channels. However,
in sama5d3, it has different number for endpoints and DMA channels.
So, define a new micro USBA_NR_DMAs for DMA channels. And the
USBA_NR_ENDPOINS is not used anymore, remove it at the same time.

Signed-off-by: Bo Shen 
---
Changes in v2:
  - Make the commit message more clearer.

 drivers/usb/gadget/atmel_usba_udc.c | 2 +-
 drivers/usb/gadget/atmel_usba_udc.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/atmel_usba_udc.c 
b/drivers/usb/gadget/atmel_usba_udc.c
index 7e67a81..5cded1c 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1661,7 +1661,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (dma_status) {
int i;
 
-   for (i = 1; i < USBA_NR_ENDPOINTS; i++)
+   for (i = 1; i < USBA_NR_DMAS; i++)
if (dma_status & (1 << i))
usba_dma_irq(udc, &udc->usba_ep[i]);
}
diff --git a/drivers/usb/gadget/atmel_usba_udc.h 
b/drivers/usb/gadget/atmel_usba_udc.h
index 2922db5..a70706e 100644
--- a/drivers/usb/gadget/atmel_usba_udc.h
+++ b/drivers/usb/gadget/atmel_usba_udc.h
@@ -210,7 +210,7 @@
 #define USBA_FIFO_BASE(x)  ((x) << 16)
 
 /* Synth parameters */
-#define USBA_NR_ENDPOINTS  7
+#define USBA_NR_DMAS   7
 
 #define EP0_FIFO_SIZE  64
 #define EP0_EPT_SIZE   USBA_EPT_SIZE_64
-- 
1.8.5.2

--
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 1/2] USB: at91: fix the number of endpoint parameter

[Bo Shen]

In sama5d3 SoC, there are 16 endpoints, which is different with
earlier SoCs (only have 7 endpoints). The USBA_NR_ENDPOINTS micro
is not suitable for sama5d3. So, get the endpoints number through
the udc->num_ep, which get from platform data for non-dt kernel,
or parse from dt node.

Signed-off-by: Bo Shen 
---
Changes in v2:
  - Make the commit message more clearer.

 drivers/usb/gadget/atmel_usba_udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/atmel_usba_udc.c 
b/drivers/usb/gadget/atmel_usba_udc.c
index 2cb52e0..7e67a81 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1670,7 +1670,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (ep_status) {
int i;
 
-   for (i = 0; i < USBA_NR_ENDPOINTS; i++)
+   for (i = 0; i < udc->num_ep; i++)
if (ep_status & (1 << i)) {
if (ep_is_control(&udc->usba_ep[i]))
usba_control_irq(udc, &udc->usba_ep[i]);
-- 
1.8.5.2

--
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 v9 01/12] usb: doc: phy-mxs: Add more compatible strings

[Shawn Guo]

On Wed, Feb 19, 2014 at 08:44:12AM +0800, Peter Chen wrote:
> On Tue, Feb 18, 2014 at 07:24:19PM -0600, Felipe Balbi wrote:
> > On Wed, Feb 19, 2014 at 09:12:49AM +0800, Shawn Guo wrote:
> > > On Tue, Feb 18, 2014 at 10:49:24AM -0600, Felipe Balbi wrote:
> > > > On Fri, Dec 27, 2013 at 10:38:30AM +0800, Peter Chen wrote:
> > > > > Add "fsl,imx6q-usbphy" for imx6dq and imx6dl, add
> > > > > "fsl,imx6sl-usbphy" for imx6sl.
> > > > > 
> > > > > Signed-off-by: Peter Chen 
> > > > 
> > > > anybody from DT to give me an Acked-by ?
> > > 
> > > Unfortunately, the DT maintainers and list are not on copy.
> > 
> > Then we need this series to be resent. I'll drop from my queue for now.
> > 
> > -- 
> > balbi
> 
> Oh, I had thought it was ok that shawn has applied dts change.
> Do we really need dts maintainer's ack for doc if dts has already
> applied?

Yes.  If DT maintainers have objection, I will need to drop the dts
patch too.

Shawn

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

Re: [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels

[Bo Shen]

On 02/19/2014 09:22 AM, Felipe Balbi wrote:

On Wed, Feb 19, 2014 at 09:14:58AM +0800, Bo Shen wrote:

Hi Felipe Balbi,

On 02/19/2014 12:19 AM, Felipe Balbi wrote:

On Fri, Jan 17, 2014 at 10:59:25AM +0800, Bo Shen wrote:

When the SoC is earlier than sama5d3 SoC, which have the same number
endpoints and DMAs. However for sama5d3 SoC, it has different number
for endpoints and DMAs. So, define USBA_NR_DMAs for DMA channels

Signed-off-by: Bo Shen 
---

  drivers/usb/gadget/atmel_usba_udc.c | 2 +-
  drivers/usb/gadget/atmel_usba_udc.h | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/atmel_usba_udc.c 
b/drivers/usb/gadget/atmel_usba_udc.c
index 7e67a81..5cded1c 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1661,7 +1661,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (dma_status) {
int i;

-   for (i = 1; i < USBA_NR_ENDPOINTS; i++)
+   for (i = 1; i < USBA_NR_DMAS; i++)
if (dma_status & (1 << i))
usba_dma_irq(udc, &udc->usba_ep[i]);
}
diff --git a/drivers/usb/gadget/atmel_usba_udc.h 
b/drivers/usb/gadget/atmel_usba_udc.h
index 2922db5..a70706e 100644
--- a/drivers/usb/gadget/atmel_usba_udc.h
+++ b/drivers/usb/gadget/atmel_usba_udc.h
@@ -210,7 +210,7 @@
  #define USBA_FIFO_BASE(x) ((x) << 16)

  /* Synth parameters */
-#define USBA_NR_ENDPOINTS  7
+#define USBA_NR_DMAS   7


what's the difference ? You just renamed this macro. Also, please
clarify a bit your commit log.


As commit message said, the SoC before sama5d3, the endpoint number
is the same as DMA channel number, so use endpoints definition for
DMA channel number, however after sama5d3, the endpoints is not the
same as DMA channel, so use DMA micro for DMA channels.


which means you're just renaming the macro for the sake of clarity.
That's fine, just needs to be clearer in commit message.


Thanks, I will send v2 to make the commit message more clearer.

Best Regards,
Bo Shen
--
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 v9 11/12] usb: phy-mxs: Add system suspend/resume API

[Peter Chen]

On Tue, Feb 18, 2014 at 10:50:32AM -0600, Felipe Balbi wrote:
> On Fri, Dec 27, 2013 at 10:38:40AM +0800, Peter Chen wrote:
> > We need this to keep PHY's power on or off during the system
> > suspend mode. If we need to enable USB wakeup, then we
> > must keep PHY's power being on during the system suspend mode.
> > Otherwise, we need to keep PHY's power being off to save power.
> > 
> > Signed-off-by: Peter Chen 
> > ---
> >  drivers/usb/phy/phy-mxs-usb.c |   61 
> > +
> >  1 files changed, 61 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> > index 96aac05..53b8dad 100644
> > --- a/drivers/usb/phy/phy-mxs-usb.c
> > +++ b/drivers/usb/phy/phy-mxs-usb.c
> > @@ -57,6 +57,10 @@
> >  #define BM_USBPHY_DEBUG_CLKGATEBIT(30)
> >  
> >  /* Anatop Registers */
> > +#define ANADIG_ANA_MISC0   0x150
> > +#define ANADIG_ANA_MISC0_SET   0x154
> > +#define ANADIG_ANA_MISC0_CLR   0x158
> > +
> >  #define ANADIG_USB1_VBUS_DET_STAT  0x1c0
> >  #define ANADIG_USB2_VBUS_DET_STAT  0x220
> >  
> > @@ -65,6 +69,9 @@
> >  #define ANADIG_USB2_LOOPBACK_SET   0x244
> >  #define ANADIG_USB2_LOOPBACK_CLR   0x248
> >  
> > +#define BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG   BIT(12)
> > +#define BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG_SL BIT(11)
> > +
> >  #define BM_ANADIG_USB1_VBUS_DET_STAT_VBUS_VALIDBIT(3)
> >  #define BM_ANADIG_USB2_VBUS_DET_STAT_VBUS_VALIDBIT(3)
> >  
> > @@ -134,6 +141,16 @@ struct mxs_phy {
> > int port_id;
> >  };
> >  
> > +static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
> > +{
> > +   return mxs_phy->data == &imx6q_phy_data;
> > +}
> > +
> > +static inline bool is_imx6sl_phy(struct mxs_phy *mxs_phy)
> > +{
> > +   return mxs_phy->data == &imx6sl_phy_data;
> > +}
> > +
> >  static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
> >  {
> > int ret;
> > @@ -382,6 +399,8 @@ static int mxs_phy_probe(struct platform_device *pdev)
> >  
> > platform_set_drvdata(pdev, mxs_phy);
> >  
> > +   device_set_wakeup_capable(&pdev->dev, true);
> > +
> > ret = usb_add_phy_dev(&mxs_phy->phy);
> > if (ret)
> > return ret;
> > @@ -398,6 +417,47 @@ static int mxs_phy_remove(struct platform_device *pdev)
> > return 0;
> >  }
> >  
> > +#ifdef CONFIG_PM_SLEEP
> 
> please use CONFIG_PM instead.
> 

No.

See kernel/power/Kconfig

config PM_SLEEP
def_bool y
depends on SUSPEND || HIBERNATE_CALLBACKS

config PM
def_bool y
depends on PM_SLEEP || PM_RUNTIME

Here, we need suspend API for system suspend, if PM_RUNTIME
is enabled, but PM_SLEEP is disabled, we don't hope
this function is defined.

-- 

Best Regards,
Peter Chen

--
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 v9 01/12] usb: doc: phy-mxs: Add more compatible strings

[Peter Chen]

On Tue, Feb 18, 2014 at 07:24:19PM -0600, Felipe Balbi wrote:
> On Wed, Feb 19, 2014 at 09:12:49AM +0800, Shawn Guo wrote:
> > On Tue, Feb 18, 2014 at 10:49:24AM -0600, Felipe Balbi wrote:
> > > On Fri, Dec 27, 2013 at 10:38:30AM +0800, Peter Chen wrote:
> > > > Add "fsl,imx6q-usbphy" for imx6dq and imx6dl, add
> > > > "fsl,imx6sl-usbphy" for imx6sl.
> > > > 
> > > > Signed-off-by: Peter Chen 
> > > 
> > > anybody from DT to give me an Acked-by ?
> > 
> > Unfortunately, the DT maintainers and list are not on copy.
> 
> Then we need this series to be resent. I'll drop from my queue for now.
> 
> -- 
> balbi

Oh, I had thought it was ok that shawn has applied dts change.
Do we really need dts maintainer's ack for doc if dts has already
applied?

-- 

Best Regards,
Peter Chen

--
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 v4 13/14] usb: force warm reset to break resume livelock

[Dan Williams]

On Tue, Feb 18, 2014 at 3:39 PM, Julius Werner  wrote:
>>> We don't need to change hub_port_debounce() right away... that was
>>> just an additional suggestion to make things more efficient for
>>> SuperSpeed devices in general. I think for now (in order to solve
>>> Dan's problem), it would be best if he just calls hub_port_debounce()
>>> in usb_port_runtime_resume() (which should bring a connected device
>>> back in the majority of cases), and always queues up a warm reset if
>>> that fails. (This is essentially what his patch is already doing, just
>>> get rid of the extra check for USB_SS_PORT_LS_POLLING in the error
>>> path which I think might be a little too specific and overlook cases
>>> where the RxDetect/Polling cycle just happens to be at RxDetect in
>>> that moment.)
>>
>> That's the plan, and I also want to test a usb device quirk flag to
>> unconditionally warm reset on power-session loss since it does seem to
>> be device specifc in my case.
>
> For what it's worth, I think you might not need a quirk flag since you
> are not really loosing anything in that case. If the first
> hub_port_debounce() fails to reconnect, the device is either quirky or
> there is no device attached at all. In the first case we want the warm
> reset, and in the second case the warm reset is pointless but also
> doesn't cost us anything (assuming it runs totally asynchronous, which
> I think your patch guarantees).

No, it's not asynchronous.

> The only case where we really need to
> avoid wasting those 100ms is on ports which are connected and already
> usable, but those should be caught by hub_port_debounce() already.

The quirk would stop the the implementation from wasting its time on
the 2 second reconnect timeout on devices known to have problems
recovering from a logical power session loss and simply issue the
warm-reset immediately.  That said, I should also look to see how long
devices take to reconnect on average and if that is more than 120ms
maybe it would make sense to unconditionally warm reset.
--
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 v9 01/12] usb: doc: phy-mxs: Add more compatible strings

[Felipe Balbi]

On Wed, Feb 19, 2014 at 09:12:49AM +0800, Shawn Guo wrote:
> On Tue, Feb 18, 2014 at 10:49:24AM -0600, Felipe Balbi wrote:
> > On Fri, Dec 27, 2013 at 10:38:30AM +0800, Peter Chen wrote:
> > > Add "fsl,imx6q-usbphy" for imx6dq and imx6dl, add
> > > "fsl,imx6sl-usbphy" for imx6sl.
> > > 
> > > Signed-off-by: Peter Chen 
> > 
> > anybody from DT to give me an Acked-by ?
> 
> Unfortunately, the DT maintainers and list are not on copy.

Then we need this series to be resent. I'll drop from my queue for now.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels

[Felipe Balbi]

On Wed, Feb 19, 2014 at 09:14:58AM +0800, Bo Shen wrote:
> Hi Felipe Balbi,
> 
> On 02/19/2014 12:19 AM, Felipe Balbi wrote:
> >On Fri, Jan 17, 2014 at 10:59:25AM +0800, Bo Shen wrote:
> >>When the SoC is earlier than sama5d3 SoC, which have the same number
> >>endpoints and DMAs. However for sama5d3 SoC, it has different number
> >>for endpoints and DMAs. So, define USBA_NR_DMAs for DMA channels
> >>
> >>Signed-off-by: Bo Shen 
> >>---
> >>
> >>  drivers/usb/gadget/atmel_usba_udc.c | 2 +-
> >>  drivers/usb/gadget/atmel_usba_udc.h | 2 +-
> >>  2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/drivers/usb/gadget/atmel_usba_udc.c 
> >>b/drivers/usb/gadget/atmel_usba_udc.c
> >>index 7e67a81..5cded1c 100644
> >>--- a/drivers/usb/gadget/atmel_usba_udc.c
> >>+++ b/drivers/usb/gadget/atmel_usba_udc.c
> >>@@ -1661,7 +1661,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
> >>if (dma_status) {
> >>int i;
> >>
> >>-   for (i = 1; i < USBA_NR_ENDPOINTS; i++)
> >>+   for (i = 1; i < USBA_NR_DMAS; i++)
> >>if (dma_status & (1 << i))
> >>usba_dma_irq(udc, &udc->usba_ep[i]);
> >>}
> >>diff --git a/drivers/usb/gadget/atmel_usba_udc.h 
> >>b/drivers/usb/gadget/atmel_usba_udc.h
> >>index 2922db5..a70706e 100644
> >>--- a/drivers/usb/gadget/atmel_usba_udc.h
> >>+++ b/drivers/usb/gadget/atmel_usba_udc.h
> >>@@ -210,7 +210,7 @@
> >>  #define USBA_FIFO_BASE(x) ((x) << 16)
> >>
> >>  /* Synth parameters */
> >>-#define USBA_NR_ENDPOINTS  7
> >>+#define USBA_NR_DMAS   7
> >
> >what's the difference ? You just renamed this macro. Also, please
> >clarify a bit your commit log.
> 
> As commit message said, the SoC before sama5d3, the endpoint number
> is the same as DMA channel number, so use endpoints definition for
> DMA channel number, however after sama5d3, the endpoints is not the
> same as DMA channel, so use DMA micro for DMA channels.

which means you're just renaming the macro for the sake of clarity.
That's fine, just needs to be clearer in commit message.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels

[Bo Shen]

Hi Felipe Balbi,

On 02/19/2014 12:19 AM, Felipe Balbi wrote:

On Fri, Jan 17, 2014 at 10:59:25AM +0800, Bo Shen wrote:

When the SoC is earlier than sama5d3 SoC, which have the same number
endpoints and DMAs. However for sama5d3 SoC, it has different number
for endpoints and DMAs. So, define USBA_NR_DMAs for DMA channels

Signed-off-by: Bo Shen 
---

  drivers/usb/gadget/atmel_usba_udc.c | 2 +-
  drivers/usb/gadget/atmel_usba_udc.h | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/atmel_usba_udc.c 
b/drivers/usb/gadget/atmel_usba_udc.c
index 7e67a81..5cded1c 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1661,7 +1661,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (dma_status) {
int i;

-   for (i = 1; i < USBA_NR_ENDPOINTS; i++)
+   for (i = 1; i < USBA_NR_DMAS; i++)
if (dma_status & (1 << i))
usba_dma_irq(udc, &udc->usba_ep[i]);
}
diff --git a/drivers/usb/gadget/atmel_usba_udc.h 
b/drivers/usb/gadget/atmel_usba_udc.h
index 2922db5..a70706e 100644
--- a/drivers/usb/gadget/atmel_usba_udc.h
+++ b/drivers/usb/gadget/atmel_usba_udc.h
@@ -210,7 +210,7 @@
  #define USBA_FIFO_BASE(x) ((x) << 16)

  /* Synth parameters */
-#define USBA_NR_ENDPOINTS  7
+#define USBA_NR_DMAS   7


what's the difference ? You just renamed this macro. Also, please
clarify a bit your commit log.


As commit message said, the SoC before sama5d3, the endpoint number is 
the same as DMA channel number, so use endpoints definition for DMA 
channel number, however after sama5d3, the endpoints is not the same as 
DMA channel, so use DMA micro for DMA channels.


Best Regards,
Bo Shen


--
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 v9 01/12] usb: doc: phy-mxs: Add more compatible strings

[Shawn Guo]

On Tue, Feb 18, 2014 at 10:49:24AM -0600, Felipe Balbi wrote:
> On Fri, Dec 27, 2013 at 10:38:30AM +0800, Peter Chen wrote:
> > Add "fsl,imx6q-usbphy" for imx6dq and imx6dl, add
> > "fsl,imx6sl-usbphy" for imx6sl.
> > 
> > Signed-off-by: Peter Chen 
> 
> anybody from DT to give me an Acked-by ?

Unfortunately, the DT maintainers and list are not on copy.

Shawn

--
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 v4 13/14] usb: force warm reset to break resume livelock

[Julius Werner]

>> We don't need to change hub_port_debounce() right away... that was
>> just an additional suggestion to make things more efficient for
>> SuperSpeed devices in general. I think for now (in order to solve
>> Dan's problem), it would be best if he just calls hub_port_debounce()
>> in usb_port_runtime_resume() (which should bring a connected device
>> back in the majority of cases), and always queues up a warm reset if
>> that fails. (This is essentially what his patch is already doing, just
>> get rid of the extra check for USB_SS_PORT_LS_POLLING in the error
>> path which I think might be a little too specific and overlook cases
>> where the RxDetect/Polling cycle just happens to be at RxDetect in
>> that moment.)
>
> That's the plan, and I also want to test a usb device quirk flag to
> unconditionally warm reset on power-session loss since it does seem to
> be device specifc in my case.

For what it's worth, I think you might not need a quirk flag since you
are not really loosing anything in that case. If the first
hub_port_debounce() fails to reconnect, the device is either quirky or
there is no device attached at all. In the first case we want the warm
reset, and in the second case the warm reset is pointless but also
doesn't cost us anything (assuming it runs totally asynchronous, which
I think your patch guarantees). The only case where we really need to
avoid wasting those 100ms is on ports which are connected and already
usable, but those should be caught by hub_port_debounce() already.
--
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 net-next 12/14] r8152: replace netif_rx with netif_receive_skb

[Francois Romieu]

Hayes Wang  :
> Replace netif_rx with netif_receive_skb to avoid disabling irq frequently
> for increasing the efficiency.

read_bulk_callback is issued in irq context. It could thus use plain
spin_lock / spin_unlock instead of the irq disabling version.

-- 
Ueimor
--
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: mark *hci_pci irqs with IRQF_NO_THREAD

[Thomas Gleixner]

On Mon, 17 Feb 2014, Stanislaw Gruszka wrote:

> There is threadirqs kenel boot option which allow to force interrupt
> routines to be performed as thread. 
> 
> USB irq routines use spin_lock(*hci->lock) variant without disabling
> interrupts, what is perfectly fine, but that can cause deadlock when
> forced thread irqs are used. Deadlock scenario is quite reproducible for
> me, as I can not boot system with threadirqs option, when some USB
> device is connected. Patch marks USB irq routines with IRQF_NO_THREAD
> to prevent forced threading.
> 
> Signed-off-by: Stanislaw Gruszka 

NACK.

This is the wrong approach and will break RT. See the discussion about
EHCI.

Thanks,

tglx


> ---
>  drivers/usb/core/hcd-pci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
> index d59d993..b4fa1a5 100644
> --- a/drivers/usb/core/hcd-pci.c
> +++ b/drivers/usb/core/hcd-pci.c
> @@ -264,7 +264,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct 
> pci_device_id *id)
>   down_write(&companions_rwsem);
>   dev_set_drvdata(&dev->dev, hcd);
>   for_each_companion(dev, hcd, ehci_pre_add);
> - retval = usb_add_hcd(hcd, hcd_irq, IRQF_SHARED);
> + retval = usb_add_hcd(hcd, hcd_irq, IRQF_SHARED | 
> IRQF_NO_THREAD);
>   if (retval != 0)
>   dev_set_drvdata(&dev->dev, NULL);
>   for_each_companion(dev, hcd, ehci_post_add);
> @@ -272,7 +272,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct 
> pci_device_id *id)
>   } else {
>   down_read(&companions_rwsem);
>   dev_set_drvdata(&dev->dev, hcd);
> - retval = usb_add_hcd(hcd, hcd_irq, IRQF_SHARED);
> + retval = usb_add_hcd(hcd, hcd_irq, IRQF_SHARED | 
> IRQF_NO_THREAD);
>   if (retval != 0)
>   dev_set_drvdata(&dev->dev, NULL);
>   else
> -- 
> 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

Re: [PATCH v4 13/14] usb: force warm reset to break resume livelock

[Dan Williams]

On Tue, Feb 18, 2014 at 2:40 PM, Julius Werner  wrote:
>> Can you take a look at it, and see if it would address your issue?  I
>> think it will catch the case where we transition from SS.Inactive ->
>> RxDetect -> Polling.
>
> I don't think that's targeting the same problem. Hans seems to be
> describing a situation where the device connects again even though he
> doesn't want it to -- in our case, the device doesn't connect even
> though we want that. His patch shouldn't do anything for our issue
> since he checks for PORT_STAT_CONNECTION, and that bit will be unset
> when the host port is stuck in RxDetect/Polling.

Yes, agreed.

>
>> > It is a valid transitional state, unfortunately, but in a working case
>> > it should resolve itself within a few milliseconds (probably less than
>> > 10). Maybe we should try to differentiate between USB 2.0 and 3.0
>> > devices in hub_port_debounce()? I think due to the built-in link
>> > training in USB 3.0, the classic debouncing doesn't really make sense
>> > anymore (and wastes a lot of time since SuperSpeed links can train
>> > really fast when they work).
>> >
>> > As for this patch, I think the best approach would be to wait for the
>> > device to come back in usb_port_runtime_resume() (through
>> > hub_port_debounce() or something else), and if it doesn't show up
>> > always set the bit to warm reset the port (regardless of LTSSM state,
>> > since even if it says RxDetect I wouldn't be sure that there is really
>> > nothing connected). We could then also use those bits in the "lost
>> > power" case of xhci_resume() to try and work around the problems with
>> > that Synopsys controller.
>>
>> That's a lot of changes to the hub core.  Would an xHCI quirk be
>> simpler?  Is there some scenario I'm missing that the S3 resume quirk
>> wouldn't handle?
>
> We don't need to change hub_port_debounce() right away... that was
> just an additional suggestion to make things more efficient for
> SuperSpeed devices in general. I think for now (in order to solve
> Dan's problem), it would be best if he just calls hub_port_debounce()
> in usb_port_runtime_resume() (which should bring a connected device
> back in the majority of cases), and always queues up a warm reset if
> that fails. (This is essentially what his patch is already doing, just
> get rid of the extra check for USB_SS_PORT_LS_POLLING in the error
> path which I think might be a little too specific and overlook cases
> where the RxDetect/Polling cycle just happens to be at RxDetect in
> that moment.)

That's the plan, and I also want to test a usb device quirk flag to
unconditionally warm reset on power-session loss since it does seem to
be device specifc in my case.
--
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 v4 13/14] usb: force warm reset to break resume livelock

[Julius Werner]

> Julius, are you sure the Synopsys host will actually power off the
> ports?  The Intel hosts need some special ACPI methods, so I'm not sure
> if Dan's issue with ports after power on could even be seen on the
> Synopsys host.
>
> The Synopsys issue (as I remember it, please correct me if I'm wrong)
> would only be triggered if the host lost power during S3, and was halted
> and reset after a register restore failure.  I think the solution we
> agreed to was to implement a Synopsys host quirk that would warm reset
> all ports unconditionally on register restore failure.  Was that right?
>
> Then there's Dan's issue.  Dan, does the port go into SS.Inactive before
> the host starts to cycle between RxDetect and Polling and U0 for this
> case?

Sorry, I didn't mean to pull the Synopsys issue into this thread... we
should probably keep that separate in
https://lkml.org/lkml/2013/12/9/336 , or this will get too confusing.
Some aspects of that issue are definitely different, e.g. there's no
port power being turned off there (on the contrary, the problem is
that the power session stays alive during suspend but the xHC gets
reset and forgets about it). I just wanted to point out that both
issues can lead to the same state (by different means) where the port
status cycles between RxDetect and Polling, because they both reset
the host controller's port state to RxDetect in a way that the device
might not notice (or not react correctly to). I think whenever the
host port state gets forced back to RxDetect while the device is in
SS.Inactive (or anything that can lead to SS.Inactive after a few
unexpected packets) you will get into this state, and you will only
get back out of there through a warm reset (or by physically cutting
off VBUS).

> Hans also ran into this issue (or at least a variation of it), and
> proposed a patch to fix it.
>
> https://git.kernel.org/cgit/linux/kernel/git/sarah/xhci.git/commit/?h=for-usb-next-streams&id=3fd14185404e3a298a3f6b6c6f21ff8d41bb2747
>
> Can you take a look at it, and see if it would address your issue?  I
> think it will catch the case where we transition from SS.Inactive ->
> RxDetect -> Polling.

I don't think that's targeting the same problem. Hans seems to be
describing a situation where the device connects again even though he
doesn't want it to -- in our case, the device doesn't connect even
though we want that. His patch shouldn't do anything for our issue
since he checks for PORT_STAT_CONNECTION, and that bit will be unset
when the host port is stuck in RxDetect/Polling.

> > It is a valid transitional state, unfortunately, but in a working case
> > it should resolve itself within a few milliseconds (probably less than
> > 10). Maybe we should try to differentiate between USB 2.0 and 3.0
> > devices in hub_port_debounce()? I think due to the built-in link
> > training in USB 3.0, the classic debouncing doesn't really make sense
> > anymore (and wastes a lot of time since SuperSpeed links can train
> > really fast when they work).
> >
> > As for this patch, I think the best approach would be to wait for the
> > device to come back in usb_port_runtime_resume() (through
> > hub_port_debounce() or something else), and if it doesn't show up
> > always set the bit to warm reset the port (regardless of LTSSM state,
> > since even if it says RxDetect I wouldn't be sure that there is really
> > nothing connected). We could then also use those bits in the "lost
> > power" case of xhci_resume() to try and work around the problems with
> > that Synopsys controller.
>
> That's a lot of changes to the hub core.  Would an xHCI quirk be
> simpler?  Is there some scenario I'm missing that the S3 resume quirk
> wouldn't handle?

We don't need to change hub_port_debounce() right away... that was
just an additional suggestion to make things more efficient for
SuperSpeed devices in general. I think for now (in order to solve
Dan's problem), it would be best if he just calls hub_port_debounce()
in usb_port_runtime_resume() (which should bring a connected device
back in the majority of cases), and always queues up a warm reset if
that fails. (This is essentially what his patch is already doing, just
get rid of the extra check for USB_SS_PORT_LS_POLLING in the error
path which I think might be a little too specific and overlook cases
where the RxDetect/Polling cycle just happens to be at RxDetect in
that moment.)
--
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 v2 2/3] usb: chipidea: msm: Add device tree support

[Sergei Shtylyov]

Hello.

On 02/19/2014 12:34 AM, Courtney Cavin wrote:


From: "Ivan T. Ivanov" 



Allows controller to be specified via device tree.
Pass PHY phandle specified in DT to core driver.



Signed-off-by: Ivan T. Ivanov 
---
drivers/usb/chipidea/ci_hdrc_msm.c |   23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)



  You also need to describe the binding you're creating in
Documentation/devicetree/bindings/usb/.txt.



Did you check "[PATCH v2 1/3]"?



 Did you send it to 'linux-usb'? I just didn't get the patch.
(Typically, the bindings are described in the same patch the DT support is
added to the driver bu YMMV, of course.)



Although I would personally agree that this is the most logical method,
it would appear that the DT guys disagree with us [1].  Lately, they


   Thank you for the reference.


seem to have either given up or are otherwise preoccupied, so perhaps
you can still slip a few patches by them. ;)


   Yeah, I was at last able to get my Ethernet driver bindings applied. :-)


-Courtney



[1] 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/submitting-patches.txt


WBR, Sergei

--
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 15/25] libusbg: Add getter for config name.

[Sergei Shtylyov]

Hello.

On 02/17/2014 09:53 PM, Krzysztof Opasiak wrote:


Add usbg_get_config_name() and usbg_get_config_name_len()
to avoid direct config structure members access.



Signed-off-by: Krzysztof Opasiak 
---
  include/usbg/usbg.h |   16 
  src/usbg.c  |   10 ++
  2 files changed, 26 insertions(+)



diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index 0f3fe4c..ac5f730 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -480,6 +480,22 @@ extern struct config *usbg_create_config(struct gadget *g, 
char *name,
struct config_attrs *c_attrs, struct config_strs *c_strs);

  /**
+ * @brief Get config name length
+ * @param c Config which name length should be returned
+ * @return Length of name string or -1 if error occurred.
+ */
+extern size_t usbg_get_config_name_len(struct config *c);
+
+/**
+ * @brieg Get config name
+ * @param c Pointer to config
+ * @param buf Buffer where name should be copied
+ * @param len Length of given buffer
+ * @return Pointer to destination or NULL if error occurred.
+ */
+extern char *usbg_get_config_name(struct config* c, char *buf, size_t len);


   Be consistent in your coding style: always put * next to the variable name 
(this is kernel style though).



+
+/**
   * @brief Set the USB configuration attributes
   * @param c Pointer to configuration
   * @param c_attrs Configuration attributes
diff --git a/src/usbg.c b/src/usbg.c
index 073efd6..1f1e6d0 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -925,6 +925,16 @@ struct config *usbg_create_config(struct gadget *g, char 
*name,
return c;
  }

+size_t usbg_get_config_name_len(struct config *c)
+{
+   return c ? strlen(c->name): -1;
+}
+
+char *usbg_get_config_name(struct config* c, char *buf, size_t len)


   Same here.

WBR, Sergei

--
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 05/25] libusbg: Update strings only when writting US English strings.

[Sergei Shtylyov]

Hello.

On 02/19/2014 02:07 AM, Sergei Shtylyov wrote:


Strings in current verison of library are hardcoded to
US English. Functions which set strings are generic and
allow to set other languages, but internal library structures
should be update only when setting US English strings.



Signed-off-by: Krzysztof Opasiak 

[...]


I guess you haven't run your patch via scripts/checkpatch.pl, otherwise
you would have seen it protesting against single statement *if* arms in {}.
Well, some common sense applies as well since {} are completely unnecessary.


   Ah, I have initially overlooked that it's libusbg patch -- libusbg may 
have its own style peculiarities.


WBR, Sergei

--
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 05/25] libusbg: Update strings only when writting US English strings.

[Sergei Shtylyov]

Hello.

On 02/17/2014 09:53 PM, Krzysztof Opasiak wrote:


Strings in current verison of library are hardcoded to
US English. Functions which set strings are generic and
allow to set other languages, but internal library structures
should be update only when setting US English strings.



Signed-off-by: Krzysztof Opasiak 
---
  src/usbg.c |   20 
  1 file changed, 16 insertions(+), 4 deletions(-)



diff --git a/src/usbg.c b/src/usbg.c
index e62eb01..2264e7c 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -617,7 +617,10 @@ void usbg_set_gadget_serial_number(struct gadget *g, int 
lang, char *serno)

mkdir(path, S_IRWXU|S_IRWXG|S_IRWXO);

-   strcpy(g->strs.str_ser, serno);
+   /* strings in library are hardcoded to US English for now */
+   if (lang == LANG_US_ENG) {
+   strcpy(g->strs.str_ser, serno);
+   }

usbg_write_string(path, "", "serialnumber", serno);
  }
@@ -630,7 +633,10 @@ void usbg_set_gadget_manufacturer(struct gadget *g, int 
lang, char *mnf)

mkdir(path, S_IRWXU|S_IRWXG|S_IRWXO);

-   strcpy(g->strs.str_mnf, mnf);
+   /* strings in library are hardcoded to US English for now */
+   if (lang == LANG_US_ENG) {
+   strcpy(g->strs.str_mnf, mnf);
+   }

usbg_write_string(path, "", "manufacturer", mnf);
  }
@@ -643,7 +649,10 @@ void usbg_set_gadget_product(struct gadget *g, int lang, 
char *prd)

mkdir(path, S_IRWXU|S_IRWXG|S_IRWXO);

-   strcpy(g->strs.str_prd, prd);
+   /* strings in library are hardcoded to US English for now */
+   if (lang == LANG_US_ENG) {
+   strcpy(g->strs.str_prd, prd);
+   }

usbg_write_string(path, "", "product", prd);
  }
@@ -758,7 +767,10 @@ void usbg_set_config_string(struct config *c, int lang, 
char *str)

mkdir(path, S_IRWXU|S_IRWXG|S_IRWXO);

-   strcpy(c->str_cfg, str);
+   /* strings in library are hardcoded to US English for now */
+   if (lang == LANG_US_ENG) {
+   strcpy(c->str_cfg, str);
+   }


   I guess you haven't run your patch via scripts/checkpatch.pl, otherwise 
you would have seen it protesting against single statement *if* arms in {}.

Well, some common sense applies as well since {} are completely unnecessary.

WBR, Sergei

--
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 v4 13/14] usb: force warm reset to break resume livelock

[Felipe Balbi]

On Tue, Feb 18, 2014 at 09:54:59PM +, Paul Zimmerman wrote:
> > From: Felipe Balbi [mailto:ba...@ti.com]
> > Sent: Tuesday, February 18, 2014 1:05 PM
> > 
> > Hi,
> > 
> > + Paul as he might have better details about the Synopsys core host-side
> > implementation
> > 
> > On Tue, Feb 18, 2014 at 12:42:53PM -0800, Sarah Sharp wrote:
> > > On Tue, Feb 11, 2014 at 07:29:40PM -0800, Julius Werner wrote:
> > > > >> I believe I am seeing a "polling livelock" scenario as described by 
> > > > >> Julius.
> > > > >
> > > > > Julius was talking about what happens when the host controller itself
> > > > > gets reset (and therefore remembers nothing about any device) whereas
> > > > > the device still thinks it is in U3.  Is that the scenario you're
> > > > > encountering?  I thought you were working on normal runtime PM.
> > > >
> > > > When you turn the power back on for a port, it should start out in
> > > > RxDetect and switch to Polling as it detects Rx terminations. If the
> > > > downstream device is unhappy for any reason (e.g. in SS.Inactive or
> > > > still in U3) and sends no or wrong responses to the LFPS Polling, the
> > > > hub's port will either move to ComplianceMode or keep cycling back and
> > > > forth between RxDetect and Polling.
> > >
> > > > The latter is especially dangerous
> > > > because it's hard to detect (if you just sample the port status you
> > > > might see RxDetect, which would also be expected if there is nothing
> > > > connected at all), so I'm thinking an unconditional warm reset might
> > > > be unavoidable.
> > >
> > > > That is why we proposed to go that route for the
> > > > Synopsys controller, and I think the same will apply to this situation
> > > > (since I think turning off a PortPower bit in XHCI will make the
> > > > controller "forget" a previous U3 state and return to RxDetect when
> > > > you turn it back on again, even though the actual VBUS line to the
> > > > device may not have been disabled after all).
> > >
> > > Julius, are you sure the Synopsys host will actually power off the
> > > ports?  The Intel hosts need some special ACPI methods, so I'm not sure
> > > if Dan's issue with ports after power on could even be seen on the
> > > Synopsys host.
> > >
> > > The Synopsys issue (as I remember it, please correct me if I'm wrong)
> > > would only be triggered if the host lost power during S3, and was halted
> > > and reset after a register restore failure.  I think the solution we
> > > agreed to was to implement a Synopsys host quirk that would warm reset
> > > all ports unconditionally on register restore failure.  Was that right?
> > >
> > > Then there's Dan's issue.  Dan, does the port go into SS.Inactive before
> > > the host starts to cycle between RxDetect and Polling and U0 for this
> > > case?
> > >
> > > Hans also ran into this issue (or at least a variation of it), and
> > > proposed a patch to fix it.
> > >
> > > https://git.kernel.org/cgit/linux/kernel/git/sarah/xhci.git/commit/?h=for-usb-next-streams&id=3fd14185404e3a298a3f6b6c6f21ff8d41bb2747
> > >
> > > Can you take a look at it, and see if it would address your issue?  I
> > > think it will catch the case where we transition from SS.Inactive ->
> > > RxDetect -> Polling.
> > >
> > > > >> > One other thought (I don't know if it is the right thing to do) is 
> > > > >> > that
> > > > >> > we might _always_ perform a warm reset after powering-on a 
> > > > >> > SuperSpeed
> > > > >> > port, without bothering to call hub_port_debounce_be_connected.
> > > > >>
> > > > >> I'm leaning in that direction.  However, the decision comes down to
> > > > >> the relative occurrence frequency of devices that fall into this trap
> > > > >> vs those that successfully recover and would suffer the additional
> > > > >> latency of a warm reset.
> > > > >
> > > > > Is a warm reset significantly longer than an ordinary reset?  We have
> > > > > to do some kind of reset in any case.  After all, the power session
> > > > > _has_ been interrupted.  (Assuming the power switching worked...)
> > > >
> > > > USB 3.0 ports don't need to be reset on connect as a matter of course.
> > > > The should usually just start training themselves and eventually
> > > > become ready as soon as the wires touch. An extra warm reset would add
> > > > 80-120ms delay to the port resume. (In comparison, a hot reset should
> > > > not take more than 12ms, probably even much less.)
> > >
> > > I would rather avoid warm reset unconditionally on connect whenever
> > > possible, because 80-120ms is too long of a delay for some
> > > embedded/tablet systems that come into and out of S3 very often.
> > >
> > > > >> >> With this in place we may want to consider reducing the timeout 
> > > > >> >> and
> > > > >> >> relying on warm reset for recovery.
> > > > >> >
> > > > >> > Why?  I'm not familiar with the intricacies of USB-3 link state
> > > > >> > changes, but there seem to be only two possibilities:
> > > > >> >
> > > > >> > Either POR

RE: [PATCH v4 13/14] usb: force warm reset to break resume livelock

[Paul Zimmerman]

> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Tuesday, February 18, 2014 1:05 PM
> 
> Hi,
> 
> + Paul as he might have better details about the Synopsys core host-side
> implementation
> 
> On Tue, Feb 18, 2014 at 12:42:53PM -0800, Sarah Sharp wrote:
> > On Tue, Feb 11, 2014 at 07:29:40PM -0800, Julius Werner wrote:
> > > >> I believe I am seeing a "polling livelock" scenario as described by 
> > > >> Julius.
> > > >
> > > > Julius was talking about what happens when the host controller itself
> > > > gets reset (and therefore remembers nothing about any device) whereas
> > > > the device still thinks it is in U3.  Is that the scenario you're
> > > > encountering?  I thought you were working on normal runtime PM.
> > >
> > > When you turn the power back on for a port, it should start out in
> > > RxDetect and switch to Polling as it detects Rx terminations. If the
> > > downstream device is unhappy for any reason (e.g. in SS.Inactive or
> > > still in U3) and sends no or wrong responses to the LFPS Polling, the
> > > hub's port will either move to ComplianceMode or keep cycling back and
> > > forth between RxDetect and Polling.
> >
> > > The latter is especially dangerous
> > > because it's hard to detect (if you just sample the port status you
> > > might see RxDetect, which would also be expected if there is nothing
> > > connected at all), so I'm thinking an unconditional warm reset might
> > > be unavoidable.
> >
> > > That is why we proposed to go that route for the
> > > Synopsys controller, and I think the same will apply to this situation
> > > (since I think turning off a PortPower bit in XHCI will make the
> > > controller "forget" a previous U3 state and return to RxDetect when
> > > you turn it back on again, even though the actual VBUS line to the
> > > device may not have been disabled after all).
> >
> > Julius, are you sure the Synopsys host will actually power off the
> > ports?  The Intel hosts need some special ACPI methods, so I'm not sure
> > if Dan's issue with ports after power on could even be seen on the
> > Synopsys host.
> >
> > The Synopsys issue (as I remember it, please correct me if I'm wrong)
> > would only be triggered if the host lost power during S3, and was halted
> > and reset after a register restore failure.  I think the solution we
> > agreed to was to implement a Synopsys host quirk that would warm reset
> > all ports unconditionally on register restore failure.  Was that right?
> >
> > Then there's Dan's issue.  Dan, does the port go into SS.Inactive before
> > the host starts to cycle between RxDetect and Polling and U0 for this
> > case?
> >
> > Hans also ran into this issue (or at least a variation of it), and
> > proposed a patch to fix it.
> >
> > https://git.kernel.org/cgit/linux/kernel/git/sarah/xhci.git/commit/?h=for-usb-next-streams&id=3fd14185404e3a298a3f6b6c6f21ff8d41bb2747
> >
> > Can you take a look at it, and see if it would address your issue?  I
> > think it will catch the case where we transition from SS.Inactive ->
> > RxDetect -> Polling.
> >
> > > >> > One other thought (I don't know if it is the right thing to do) is 
> > > >> > that
> > > >> > we might _always_ perform a warm reset after powering-on a SuperSpeed
> > > >> > port, without bothering to call hub_port_debounce_be_connected.
> > > >>
> > > >> I'm leaning in that direction.  However, the decision comes down to
> > > >> the relative occurrence frequency of devices that fall into this trap
> > > >> vs those that successfully recover and would suffer the additional
> > > >> latency of a warm reset.
> > > >
> > > > Is a warm reset significantly longer than an ordinary reset?  We have
> > > > to do some kind of reset in any case.  After all, the power session
> > > > _has_ been interrupted.  (Assuming the power switching worked...)
> > >
> > > USB 3.0 ports don't need to be reset on connect as a matter of course.
> > > The should usually just start training themselves and eventually
> > > become ready as soon as the wires touch. An extra warm reset would add
> > > 80-120ms delay to the port resume. (In comparison, a hot reset should
> > > not take more than 12ms, probably even much less.)
> >
> > I would rather avoid warm reset unconditionally on connect whenever
> > possible, because 80-120ms is too long of a delay for some
> > embedded/tablet systems that come into and out of S3 very often.
> >
> > > >> >> With this in place we may want to consider reducing the timeout and
> > > >> >> relying on warm reset for recovery.
> > > >> >
> > > >> > Why?  I'm not familiar with the intricacies of USB-3 link state
> > > >> > changes, but there seem to be only two possibilities:
> > > >> >
> > > >> > Either PORT_LS_POLLING is a valid state to be in while
> > > >> > trying to establish a SuperSpeed connection, in which case
> > > >> > we don't want to reduce the timeout,
> > > >> >
> > > >> > Or it isn't a valid state, in which case we should abort
> > > >> >   

Re: v3.14-rc2+ WARNING: CPU: 0 PID: 24108 at fs/sysfs/group.c:216 sysfs_remove_group+0xa3/0xb0()

[Michal Šmucr]

On 18. 2. 2014 21:51, Alan Stern wrote:

On Tue, 18 Feb 2014, [windows-1252] Michal �mucr wrote:


On 17.2.2014 20:48, Alan Stern wrote:


This isn't a USB problem.


Hello,

I'm experiencing very similar messages after every disconnection of USB
soundcard (snd-usb-audio module). During searching for other similar
occurrences of message, i've found few patches. One kind basically
prevents sysfs from popping those messages at sysfs group attribute
removal - https://lkml.org/lkml/2013/11/23/102 and isn't included in
3.14-rc2, other kind of patches, if i got that correctly, fixes removal
ordering or its visibility at underlying subsystem (eg. SCSI, PCI for
Thunderbolt hotplugging) to make sysfs happy.
Do you know, that there was some general decision, to intentionally
don't modify sysfs and fix that for every subsystem, which triggers that
messages?


Yes, there was indeed such a decision.


Just asking, because then it would be probably nice to modify something
also at USB audio..

my dmesg with WARNING:
http://vpub.smucr.cz/pub/bbb/debug/dmesg-reconnections.log
and i probably not alone:
http://forums.funtoo.org/viewtopic.php?pid=10327


If you want to pursue this, you should contact the alsa-devel mailing
list and the maintainers of the Sound subsystem.

Alan Stern


Hello Alan,

thank you for information.

Best regards,

Michal Smucr

--
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 net-next 00/14] r8152: improvement and new features

[David Miller]

From: Hayes Wang 
Date: Tue, 18 Feb 2014 21:48:57 +0800

> Change some flows or behavior to improve the efficiency or make the
> code readable. Besides, support WOL and runtime suspend.

Series applied, but as Florian mentioned you should seriously consider
converting this driver to use phylib.
--
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 0/7] usb: dwc2/s3c-hsotg: Combine drivers into a single DRD

[Paul Zimmerman]

> From: Dinh Nguyen [mailto:dingu...@altera.com]
> Sent: Tuesday, February 18, 2014 1:14 PM
> 
> On Fri, 2014-02-14 at 22:50 +, Paul Zimmerman wrote:
> > > From: Jingoo Han [mailto:jg1@samsung.com]
> > > Sent: Thursday, February 13, 2014 11:46 PM
> > >
> > > On Friday, February 14, 2014 2:27 PM, Peter Chen wrote:
> > > > On Thu, Feb 13, 2014 at 03:10:40PM -0600, dingu...@altera.com wrote:
> > > > > From: Dinh Nguyen 
> > > > >
> > > > > Hello,
> > > > >
> > > > > This patch series combines the dwc2 host driver and the s3c-hsotg 
> > > > > peripheral
> > > > > driver into a single dual-roler driver similar to the dwc3.
> > > >
> > > > Does s3c-hsotg use dwc usb2 ip too? If it is, the structure should
> > > > like ip driver + glue layer. If not, why needs to put them
> > > > together, I am a little puzzled here.
> > >
> > > Yes, 's3c-hsotg' also uses DWC USB2 IP.
> > > As DWC3 USB driver (./drivers/usb/dwc3/), 'IP driver + glue layer'
> > > looks good.
> > >
> > > Best regards,
> > > Jingoo Han
> >
> > Well, DWC3 is a little different. It has a standards-based host API,
> > xHCI, that is (almost) completely independent from its non-standards-
> > based device API (I say 'almost' because of the optional OTG
> > functionality). This means the two drivers have to be separate, to
> > support other xHCI implementations that don't have the device mode
> > functionality. But it also complicates some things, like the
> > implementation of OTG support, which perhaps can be seen by the fact
> > that there is almost no OTG support yet in the DWC3 driver (except for
> > role switching).
> >
> > Whereas the DWC2 host and device modes both use a proprietary API, so
> > there is no need to have separate drivers, because there are no other
> > implementations of the core except the Synopsys one. Also, quite a few
> > of the registers are shared between host and device modes, so keeping
> > the two drivers separate would mean duplicating some code that could
> > be shared between them.
> >
> > That said, I would have no objection to keeping the two drivers
> > separate, as long as it doesn't create significant difficulties with
> > the implementation.
> >
> > Dinh, what do you think? Did you consider the possibility of keeping the
> > two drivers separate, perhaps sharing some code in a library module? For
> > example, you could have dwc2-lib.ko, dwc2.ko, and s3c-hsotg.ko.
> > dwc2-lib.ko would always be loaded, and dwc2.ko or s3c-hsotg.ko (or both)
> > would be loaded depending on which mode is selected. Each driver would
> > have its own interrupt handler, both of them sharing the common IRQ.
> > They would both have their own probe methods, too.
> >
> 
> No, I didn't think of this possibility. This initial patch was to try to
> remove some duplicating of code between dwc2/s3c-hsotg without breaking
> them too badly. But I think I failed in this aspect from the initial
> testing.
> 
> I like your suggestion of keeping the drivers separate with a library
> module for the role-switching. Do you see this driver being able to go
> full OTG in the future? And if so, will one method be more adaptable
> than the other to enable OTG?

I don't think having separate drivers should affect the ability to
implement OTG, in fact I think a lot of the OTG-capable controllers have
separate drivers for host and peripheral mode.

> > I haven't thought too deeply about this, so maybe it's a bad idea. But
> > you are the one doing the work, so I think the final decision should be
> > yours.
> >
> 
> So let try your approach. But does moving s3c-hostg into the dwc2 folder
> and sharing 1 define file still valid?

I think it would be a little weird to have the peripheral driver in a
different directory than the host, especially if they share a common
library module. So I think that is still the best approach.

-- 
Paul

Re: [PATCH v2 2/3] usb: chipidea: msm: Add device tree support

[Courtney Cavin]

On Tue, Feb 18, 2014 at 07:31:55PM +0100, Sergei Shtylyov wrote:
> On 02/18/2014 08:14 PM, Ivan T. Ivanov wrote:
> 
> >>> From: "Ivan T. Ivanov" 
> 
> >>> Allows controller to be specified via device tree.
> >>> Pass PHY phandle specified in DT to core driver.
> 
> >>> Signed-off-by: Ivan T. Ivanov 
> >>> ---
> >>>drivers/usb/chipidea/ci_hdrc_msm.c |   23 ++-
> >>>1 file changed, 22 insertions(+), 1 deletion(-)
> 
> >>  You also need to describe the binding you're creating in
> >> Documentation/devicetree/bindings/usb/.txt.
> 
> > Did you check "[PATCH v2 1/3]"?
> 
> Did you send it to 'linux-usb'? I just didn't get the patch.
> (Typically, the bindings are described in the same patch the DT support is 
> added to the driver bu YMMV, of course.)

Although I would personally agree that this is the most logical method,
it would appear that the DT guys disagree with us [1].  Lately, they
seem to have either given up or are otherwise preoccupied, so perhaps
you can still slip a few patches by them. ;)

-Courtney

[1] 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/submitting-patches.txt
--
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 0/7] usb: dwc2/s3c-hsotg: Combine drivers into a single DRD

[Felipe Balbi]

Hi,

On Tue, Feb 18, 2014 at 03:14:25PM -0600, Dinh Nguyen wrote:
> On Fri, 2014-02-14 at 22:50 +, Paul Zimmerman wrote:
> > > From: Jingoo Han [mailto:jg1@samsung.com]
> > > Sent: Thursday, February 13, 2014 11:46 PM
> > > 
> > > On Friday, February 14, 2014 2:27 PM, Peter Chen wrote:
> > > > On Thu, Feb 13, 2014 at 03:10:40PM -0600, dingu...@altera.com wrote:
> > > > > From: Dinh Nguyen 
> > > > >
> > > > > Hello,
> > > > >
> > > > > This patch series combines the dwc2 host driver and the s3c-hsotg 
> > > > > peripheral
> > > > > driver into a single dual-roler driver similar to the dwc3.
> > > >
> > > > Does s3c-hsotg use dwc usb2 ip too? If it is, the structure should
> > > > like ip driver + glue layer. If not, why needs to put them
> > > > together, I am a little puzzled here.
> > > 
> > > Yes, 's3c-hsotg' also uses DWC USB2 IP.
> > > As DWC3 USB driver (./drivers/usb/dwc3/), 'IP driver + glue layer'
> > > looks good.
> > > 
> > > Best regards,
> > > Jingoo Han
> > 
> > Well, DWC3 is a little different. It has a standards-based host API,
> > xHCI, that is (almost) completely independent from its non-standards-
> > based device API (I say 'almost' because of the optional OTG
> > functionality). This means the two drivers have to be separate, to
> > support other xHCI implementations that don't have the device mode
> > functionality. But it also complicates some things, like the
> > implementation of OTG support, which perhaps can be seen by the fact
> > that there is almost no OTG support yet in the DWC3 driver (except for
> > role switching).

I don't think having separate drivers is undermining OTG support. It's
just that nobody has put down the effort to implement it ;-)

> > Whereas the DWC2 host and device modes both use a proprietary API, so
> > there is no need to have separate drivers, because there are no other
> > implementations of the core except the Synopsys one. Also, quite a few
> > of the registers are shared between host and device modes, so keeping
> > the two drivers separate would mean duplicating some code that could
> > be shared between them.
> > 
> > That said, I would have no objection to keeping the two drivers
> > separate, as long as it doesn't create significant difficulties with
> > the implementation.
> > 
> > Dinh, what do you think? Did you consider the possibility of keeping the
> > two drivers separate, perhaps sharing some code in a library module? For
> > example, you could have dwc2-lib.ko, dwc2.ko, and s3c-hsotg.ko.
> > dwc2-lib.ko would always be loaded, and dwc2.ko or s3c-hsotg.ko (or both)
> > would be loaded depending on which mode is selected. Each driver would
> > have its own interrupt handler, both of them sharing the common IRQ.
> > They would both have their own probe methods, too.
> > 
> 
> No, I didn't think of this possibility. This initial patch was to try to
> remove some duplicating of code between dwc2/s3c-hsotg without breaking
> them too badly. But I think I failed in this aspect from the initial
> testing.
> 
> I like your suggestion of keeping the drivers separate with a library
> module for the role-switching. Do you see this driver being able to go
> full OTG in the future? And if so, will one method be more adaptable
> than the other to enable OTG?
> 
> 
> > I haven't thought too deeply about this, so maybe it's a bad idea. But
> > you are the one doing the work, so I think the final decision should be
> > yours.
> > 
> 
> So let try your approach. But does moving s3c-hostg into the dwc2 folder
> and sharing 1 define file still valid?

I would say combining those driver is the best thing you guys can do for
several reasons. It will make sure everybody uses a single generic
driver, it'll help you avoid code duplication (how many other dwc2
implementations have we seen on linux-usb alone ?), it'll focus efforts
to stabilize a single dwc2 driver etc.

In fact, I have been asking Samsung folks to cleanup s3c-hsotg for quite
a while now exactly so we could have a single dwc2 driver for host and
device.

cheers

-- 
balbi


signature.asc
Description: Digital signature

Re: [BUG] FL1009: xHCI host not responding to stop endpoint command.

[Thomas Petazzoni]

Dear Arnaud Ebalard,

On Tue, 18 Feb 2014 21:54:31 +0100, Arnaud Ebalard wrote:

> > I finally got some idea: your kernel 3.13-rc7 lacks a very important
> > fix we did in the irqchip driver MSI handling. You really need to have
> > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/irqchip/irq-armada-370-xp.c?id=c7f7bd4a136e4b02dd2a66bf95aec545bd93e8db
> > applied to get proper MSI behavior. Without this patch, there is a race
> > condition, and some MSI interrupts might be lost.
> >
> > This commit was merged in v3.14-rc2, and backported to 3.13 and
> > previous stable releases.
> >
> > Can you test after applying this commit?
> 
> Just to be sure, I compiled a 3.13 w/ PCI_MSI enabled and w/o the fix:
> it failed as usual. Then, I just applied the fix on top of it and tested
> again: I was unable to make it fail, i.e. this oneline fixes the issue.

Cool!

> Sarah, I guess this also validates the fact that FL1009 has good MSI
> support ;-)
> 
> Thanks for the time you both spent. Let's close the case.

You're welcome. Sorry for having realized so late from where the
problem could be coming from, and that it was in fact already fixed.

Thanks to you for reporting and investigating the issue in the first
place!

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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 0/7] usb: dwc2/s3c-hsotg: Combine drivers into a single DRD

[Dinh Nguyen]

On Fri, 2014-02-14 at 22:50 +, Paul Zimmerman wrote:
> > From: Jingoo Han [mailto:jg1@samsung.com]
> > Sent: Thursday, February 13, 2014 11:46 PM
> > 
> > On Friday, February 14, 2014 2:27 PM, Peter Chen wrote:
> > > On Thu, Feb 13, 2014 at 03:10:40PM -0600, dingu...@altera.com wrote:
> > > > From: Dinh Nguyen 
> > > >
> > > > Hello,
> > > >
> > > > This patch series combines the dwc2 host driver and the s3c-hsotg 
> > > > peripheral
> > > > driver into a single dual-roler driver similar to the dwc3.
> > >
> > > Does s3c-hsotg use dwc usb2 ip too? If it is, the structure should
> > > like ip driver + glue layer. If not, why needs to put them
> > > together, I am a little puzzled here.
> > 
> > Yes, 's3c-hsotg' also uses DWC USB2 IP.
> > As DWC3 USB driver (./drivers/usb/dwc3/), 'IP driver + glue layer'
> > looks good.
> > 
> > Best regards,
> > Jingoo Han
> 
> Well, DWC3 is a little different. It has a standards-based host API,
> xHCI, that is (almost) completely independent from its non-standards-
> based device API (I say 'almost' because of the optional OTG
> functionality). This means the two drivers have to be separate, to
> support other xHCI implementations that don't have the device mode
> functionality. But it also complicates some things, like the
> implementation of OTG support, which perhaps can be seen by the fact
> that there is almost no OTG support yet in the DWC3 driver (except for
> role switching).
> 
> Whereas the DWC2 host and device modes both use a proprietary API, so
> there is no need to have separate drivers, because there are no other
> implementations of the core except the Synopsys one. Also, quite a few
> of the registers are shared between host and device modes, so keeping
> the two drivers separate would mean duplicating some code that could
> be shared between them.
> 
> That said, I would have no objection to keeping the two drivers
> separate, as long as it doesn't create significant difficulties with
> the implementation.
> 
> Dinh, what do you think? Did you consider the possibility of keeping the
> two drivers separate, perhaps sharing some code in a library module? For
> example, you could have dwc2-lib.ko, dwc2.ko, and s3c-hsotg.ko.
> dwc2-lib.ko would always be loaded, and dwc2.ko or s3c-hsotg.ko (or both)
> would be loaded depending on which mode is selected. Each driver would
> have its own interrupt handler, both of them sharing the common IRQ.
> They would both have their own probe methods, too.
> 

No, I didn't think of this possibility. This initial patch was to try to
remove some duplicating of code between dwc2/s3c-hsotg without breaking
them too badly. But I think I failed in this aspect from the initial
testing.

I like your suggestion of keeping the drivers separate with a library
module for the role-switching. Do you see this driver being able to go
full OTG in the future? And if so, will one method be more adaptable
than the other to enable OTG?


> I haven't thought too deeply about this, so maybe it's a bad idea. But
> you are the one doing the work, so I think the final decision should be
> yours.
> 

So let try your approach. But does moving s3c-hostg into the dwc2 folder
and sharing 1 define file still valid?

Thanks,
Dinh


--
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 v4 13/14] usb: force warm reset to break resume livelock

[Felipe Balbi]

Hi,

+ Paul as he might have better details about the Synopsys core host-side
implementation

On Tue, Feb 18, 2014 at 12:42:53PM -0800, Sarah Sharp wrote:
> On Tue, Feb 11, 2014 at 07:29:40PM -0800, Julius Werner wrote:
> > >> I believe I am seeing a "polling livelock" scenario as described by 
> > >> Julius.
> > >
> > > Julius was talking about what happens when the host controller itself
> > > gets reset (and therefore remembers nothing about any device) whereas
> > > the device still thinks it is in U3.  Is that the scenario you're
> > > encountering?  I thought you were working on normal runtime PM.
> > 
> > When you turn the power back on for a port, it should start out in
> > RxDetect and switch to Polling as it detects Rx terminations. If the
> > downstream device is unhappy for any reason (e.g. in SS.Inactive or
> > still in U3) and sends no or wrong responses to the LFPS Polling, the
> > hub's port will either move to ComplianceMode or keep cycling back and
> > forth between RxDetect and Polling.
> 
> > The latter is especially dangerous
> > because it's hard to detect (if you just sample the port status you
> > might see RxDetect, which would also be expected if there is nothing
> > connected at all), so I'm thinking an unconditional warm reset might
> > be unavoidable.
> 
> > That is why we proposed to go that route for the
> > Synopsys controller, and I think the same will apply to this situation
> > (since I think turning off a PortPower bit in XHCI will make the
> > controller "forget" a previous U3 state and return to RxDetect when
> > you turn it back on again, even though the actual VBUS line to the
> > device may not have been disabled after all).
> 
> Julius, are you sure the Synopsys host will actually power off the
> ports?  The Intel hosts need some special ACPI methods, so I'm not sure
> if Dan's issue with ports after power on could even be seen on the
> Synopsys host.
> 
> The Synopsys issue (as I remember it, please correct me if I'm wrong)
> would only be triggered if the host lost power during S3, and was halted
> and reset after a register restore failure.  I think the solution we
> agreed to was to implement a Synopsys host quirk that would warm reset
> all ports unconditionally on register restore failure.  Was that right?
> 
> Then there's Dan's issue.  Dan, does the port go into SS.Inactive before
> the host starts to cycle between RxDetect and Polling and U0 for this
> case?
> 
> Hans also ran into this issue (or at least a variation of it), and
> proposed a patch to fix it.
> 
> https://git.kernel.org/cgit/linux/kernel/git/sarah/xhci.git/commit/?h=for-usb-next-streams&id=3fd14185404e3a298a3f6b6c6f21ff8d41bb2747
> 
> Can you take a look at it, and see if it would address your issue?  I
> think it will catch the case where we transition from SS.Inactive ->
> RxDetect -> Polling.
> 
> > >> > One other thought (I don't know if it is the right thing to do) is that
> > >> > we might _always_ perform a warm reset after powering-on a SuperSpeed
> > >> > port, without bothering to call hub_port_debounce_be_connected.
> > >>
> > >> I'm leaning in that direction.  However, the decision comes down to
> > >> the relative occurrence frequency of devices that fall into this trap
> > >> vs those that successfully recover and would suffer the additional
> > >> latency of a warm reset.
> > >
> > > Is a warm reset significantly longer than an ordinary reset?  We have
> > > to do some kind of reset in any case.  After all, the power session
> > > _has_ been interrupted.  (Assuming the power switching worked...)
> > 
> > USB 3.0 ports don't need to be reset on connect as a matter of course.
> > The should usually just start training themselves and eventually
> > become ready as soon as the wires touch. An extra warm reset would add
> > 80-120ms delay to the port resume. (In comparison, a hot reset should
> > not take more than 12ms, probably even much less.)
> 
> I would rather avoid warm reset unconditionally on connect whenever
> possible, because 80-120ms is too long of a delay for some
> embedded/tablet systems that come into and out of S3 very often.
> 
> > >> >> With this in place we may want to consider reducing the timeout and
> > >> >> relying on warm reset for recovery.
> > >> >
> > >> > Why?  I'm not familiar with the intricacies of USB-3 link state
> > >> > changes, but there seem to be only two possibilities:
> > >> >
> > >> > Either PORT_LS_POLLING is a valid state to be in while
> > >> > trying to establish a SuperSpeed connection, in which case
> > >> > we don't want to reduce the timeout,
> > >> >
> > >> > Or it isn't a valid state, in which case we should abort
> > >> > the debounce immediately.
> > 
> > It is a valid transitional state, unfortunately, but in a working case
> > it should resolve itself within a few milliseconds (probably less than
> > 10). Maybe we should try to differentiate between USB 2.0 and 3.0
> > devices in hub_

Re: [BUG] FL1009: xHCI host not responding to stop endpoint command.

[Arnaud Ebalard]

Hi Thomas,

Thomas Petazzoni  writes:

> Dear Arnaud Ebalard,
>
> On Sat, 18 Jan 2014 22:49:17 +0100, Arnaud Ebalard wrote:
>
>> I started suspecting the introduction of MSI support in Marvell PCIe
>> host controller driver (FL1009 is on the PCIe bus) and compiled a
>> a 3.13.0-rc8 w/ CONFIG_PCI_MSI disabled (it was enabled in all my
>> previous tests): I did not manage to reproduce the issue with this
>> kernel. As a side note, commits 5b4deb6526bd, 31f614edb726 and
>> 627dfcc249e2 are
>> 
>> ATM, I do not know if the problem is related to a bug in introduced MSI
>> support or some weird incompatibility of that functionality with the
>> FL1009 which would require some quirk in XHCI stack.
>> 
>> Thomas, I took a look at the changes but I am not familiar w/ how MSI
>> work. You may have an idea on what is going on here.
>
> I finally got some idea: your kernel 3.13-rc7 lacks a very important
> fix we did in the irqchip driver MSI handling. You really need to have
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/irqchip/irq-armada-370-xp.c?id=c7f7bd4a136e4b02dd2a66bf95aec545bd93e8db
> applied to get proper MSI behavior. Without this patch, there is a race
> condition, and some MSI interrupts might be lost.
>
> This commit was merged in v3.14-rc2, and backported to 3.13 and
> previous stable releases.
>
> Can you test after applying this commit?

Just to be sure, I compiled a 3.13 w/ PCI_MSI enabled and w/o the fix:
it failed as usual. Then, I just applied the fix on top of it and tested
again: I was unable to make it fail, i.e. this oneline fixes the issue.

Sarah, I guess this also validates the fact that FL1009 has good MSI
support ;-)

Thanks for the time you both spent. Let's close the case.

Cheers,

a+
--
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: mark *hci_pci irqs with IRQF_NO_THREAD

[Alan Stern]

On Tue, 18 Feb 2014, Stanislaw Gruszka wrote:

> On Mon, Feb 17, 2014 at 09:48:14AM -0500, Alan Stern wrote:
> > On Mon, 17 Feb 2014, Stanislaw Gruszka wrote:
> > 
> > > There is threadirqs kenel boot option which allow to force interrupt
> > > routines to be performed as thread. 
> > > 
> > > USB irq routines use spin_lock(*hci->lock) variant without disabling
> > > interrupts, what is perfectly fine, but that can cause deadlock when
> > > forced thread irqs are used. Deadlock scenario is quite reproducible for
> > > me, as I can not boot system with threadirqs option, when some USB
> > > device is connected. Patch marks USB irq routines with IRQF_NO_THREAD
> > > to prevent forced threading.
> > 
> > This doesn't explain the entire story.  As far as we know the deadlock 
> > affects only ehci-hcd, because only ehci-hcd uses an hrtimer callback, 
> > and hrtimer callbacks run in interrupt context even when threadirqs is 
> > specified.
> 
> Yes, you have right.
> 
> > Maybe the patch should include a comment explaining that the 
> > IRQF_NO_THREAD flag will not be needed when hrtimer callbacks are 
> > threaded.
> 
> I can add that to the patch, but perhaps better would be to just
> change ehci_irq to use spin_lock_irqsave, that will allow to have
> threaded interrupt in cost of very little penalty.

Either approach would be acceptable to me.  Using IRQF_NO_THREAD avoids
the extra overhead of spin_lock_irqsave when IRQs aren't threaded.

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: v3.14-rc2+ WARNING: CPU: 0 PID: 24108 at fs/sysfs/group.c:216 sysfs_remove_group+0xa3/0xb0()

[Alan Stern]

On Tue, 18 Feb 2014, [windows-1252] Michal �mucr wrote:

> On 17.2.2014 20:48, Alan Stern wrote:
> >
> > This isn't a USB problem.
> >
> Hello,
> 
> I'm experiencing very similar messages after every disconnection of USB 
> soundcard (snd-usb-audio module). During searching for other similar 
> occurrences of message, i've found few patches. One kind basically 
> prevents sysfs from popping those messages at sysfs group attribute 
> removal - https://lkml.org/lkml/2013/11/23/102 and isn't included in 
> 3.14-rc2, other kind of patches, if i got that correctly, fixes removal 
> ordering or its visibility at underlying subsystem (eg. SCSI, PCI for 
> Thunderbolt hotplugging) to make sysfs happy.
> Do you know, that there was some general decision, to intentionally 
> don't modify sysfs and fix that for every subsystem, which triggers that 
> messages?

Yes, there was indeed such a decision.

> Just asking, because then it would be probably nice to modify something 
> also at USB audio..
> 
> my dmesg with WARNING:
> http://vpub.smucr.cz/pub/bbb/debug/dmesg-reconnections.log
> and i probably not alone:
> http://forums.funtoo.org/viewtopic.php?pid=10327

If you want to pursue this, you should contact the alsa-devel mailing 
list and the maintainers of the Sound subsystem.

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 v4 13/14] usb: force warm reset to break resume livelock

[Sarah Sharp]

On Tue, Feb 11, 2014 at 07:29:40PM -0800, Julius Werner wrote:
> >> I believe I am seeing a "polling livelock" scenario as described by Julius.
> >
> > Julius was talking about what happens when the host controller itself
> > gets reset (and therefore remembers nothing about any device) whereas
> > the device still thinks it is in U3.  Is that the scenario you're
> > encountering?  I thought you were working on normal runtime PM.
> 
> When you turn the power back on for a port, it should start out in
> RxDetect and switch to Polling as it detects Rx terminations. If the
> downstream device is unhappy for any reason (e.g. in SS.Inactive or
> still in U3) and sends no or wrong responses to the LFPS Polling, the
> hub's port will either move to ComplianceMode or keep cycling back and
> forth between RxDetect and Polling.

> The latter is especially dangerous
> because it's hard to detect (if you just sample the port status you
> might see RxDetect, which would also be expected if there is nothing
> connected at all), so I'm thinking an unconditional warm reset might
> be unavoidable.

> That is why we proposed to go that route for the
> Synopsys controller, and I think the same will apply to this situation
> (since I think turning off a PortPower bit in XHCI will make the
> controller "forget" a previous U3 state and return to RxDetect when
> you turn it back on again, even though the actual VBUS line to the
> device may not have been disabled after all).

Julius, are you sure the Synopsys host will actually power off the
ports?  The Intel hosts need some special ACPI methods, so I'm not sure
if Dan's issue with ports after power on could even be seen on the
Synopsys host.

The Synopsys issue (as I remember it, please correct me if I'm wrong)
would only be triggered if the host lost power during S3, and was halted
and reset after a register restore failure.  I think the solution we
agreed to was to implement a Synopsys host quirk that would warm reset
all ports unconditionally on register restore failure.  Was that right?

Then there's Dan's issue.  Dan, does the port go into SS.Inactive before
the host starts to cycle between RxDetect and Polling and U0 for this
case?

Hans also ran into this issue (or at least a variation of it), and
proposed a patch to fix it.

https://git.kernel.org/cgit/linux/kernel/git/sarah/xhci.git/commit/?h=for-usb-next-streams&id=3fd14185404e3a298a3f6b6c6f21ff8d41bb2747

Can you take a look at it, and see if it would address your issue?  I
think it will catch the case where we transition from SS.Inactive ->
RxDetect -> Polling.

> >> > One other thought (I don't know if it is the right thing to do) is that
> >> > we might _always_ perform a warm reset after powering-on a SuperSpeed
> >> > port, without bothering to call hub_port_debounce_be_connected.
> >>
> >> I'm leaning in that direction.  However, the decision comes down to
> >> the relative occurrence frequency of devices that fall into this trap
> >> vs those that successfully recover and would suffer the additional
> >> latency of a warm reset.
> >
> > Is a warm reset significantly longer than an ordinary reset?  We have
> > to do some kind of reset in any case.  After all, the power session
> > _has_ been interrupted.  (Assuming the power switching worked...)
> 
> USB 3.0 ports don't need to be reset on connect as a matter of course.
> The should usually just start training themselves and eventually
> become ready as soon as the wires touch. An extra warm reset would add
> 80-120ms delay to the port resume. (In comparison, a hot reset should
> not take more than 12ms, probably even much less.)

I would rather avoid warm reset unconditionally on connect whenever
possible, because 80-120ms is too long of a delay for some
embedded/tablet systems that come into and out of S3 very often.

> >> >> With this in place we may want to consider reducing the timeout and
> >> >> relying on warm reset for recovery.
> >> >
> >> > Why?  I'm not familiar with the intricacies of USB-3 link state
> >> > changes, but there seem to be only two possibilities:
> >> >
> >> > Either PORT_LS_POLLING is a valid state to be in while
> >> > trying to establish a SuperSpeed connection, in which case
> >> > we don't want to reduce the timeout,
> >> >
> >> > Or it isn't a valid state, in which case we should abort
> >> > the debounce immediately.
> 
> It is a valid transitional state, unfortunately, but in a working case
> it should resolve itself within a few milliseconds (probably less than
> 10). Maybe we should try to differentiate between USB 2.0 and 3.0
> devices in hub_port_debounce()? I think due to the built-in link
> training in USB 3.0, the classic debouncing doesn't really make sense
> anymore (and wastes a lot of time since SuperSpeed links can train
> really fast when they work).
> 
> As for this patch, I think the best approach would be to wait for the
> device to come back in usb_port_runtime_resume()

Re: [OPW] USB subsystem questions

[Valentina Manea]

Hi,

On Sun, Feb 16, 2014 at 12:42 AM, Alan Stern  wrote:
>
> That's true when it is invoked from userspace.  It can also be invoked
> directly by a kernel driver; in that case no file is needed.
>

I managed to get it working but I think what I did is rather flawed.

The problem is where I get that struct dev_state *owner parameter
from. The closest to this I found to be the filelist member in struct
usb_device. So I got the owner as:

owner = list_entry(udev->filelist.next, struct dev_state, list);

This works but that list is supposed to be the list of open files for
that device. No files are opened in this case. Should I initialize a
struct dev_state variable similary to usbdev_open()?

Unrelated to this, USB/IP still has a problem when the device is no
longer shared. In this case, the device should be rebound to the old
drivers and be used normally. While resetting the configuration
triggers a new binding process for interface drivers, on the core
level, where usbip-host driver is, the device remains unbound.
One hack-ish way to solve this would be to manually rebind the device
to the old driver.
Is there any better way to trigger a new driver binding at core level?

Thanks,
Valentina
--
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 v2] usb/xhci: fix compilation warning when !CONFIG_PCI && !CONFIG_PM

[David Cohen]

On Tue, Feb 18, 2014 at 12:47:41PM -0600, Felipe Balbi wrote:
> On Tue, Feb 18, 2014 at 10:00:30AM -0800, David Cohen wrote:
> > Hi Sarah,
> > 
> > On Mon, Jan 06, 2014 at 07:02:19PM -0800, David Cohen wrote:
> > > When CONFIG_PCI and CONFIG_PM are not selected, xhci.c gets this
> > > warning:
> > > drivers/usb/host/xhci.c:409:13: warning: ‘xhci_msix_sync_irqs’ defined
> > > but not used [-Wunused-function]
> > > 
> > > It happens due to lack of __maybe_unused flag on xhci_msix_sync_irqs()
> > > function in case of !CONFIG_PCI.
> > > 
> > > Signed-off-by: David Cohen 
> > > ---
> > 
> > Ping :)
> > Any comments here?
> > 
> > Br, David
> > 
> > > 
> > > Change v1 -> v2:
> > >  - xhci_msix_sync_irqs() already uses __maybe_unused flag when CONFIG_PCI 
> > > is
> > >set. Proper solution is to add same flag when !CONFIG_PCI instead of 
> > > define
> > >function as inline.
> > > 
> > >  drivers/usb/host/xhci.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > > index 4265b48856f6..ed6b717b8ee1 100644
> > > --- a/drivers/usb/host/xhci.c
> > > +++ b/drivers/usb/host/xhci.c
> > > @@ -406,7 +406,7 @@ static void xhci_cleanup_msix(struct xhci_hcd *xhci)
> > >  {
> > >  }
> > >  
> > > -static void xhci_msix_sync_irqs(struct xhci_hcd *xhci)
> > > +static void __maybe_unused xhci_msix_sync_irqs(struct xhci_hcd *xhci)
> 
> bellow is likely a better fix. Usually stubs are marked inline, rather
> than getting an unused attribute:

Thanks for commenting. That would be actually the v1 of my patch :)
I changed after I see the proper function has __maybe_unused flag.

But I'm fine with Sarah picking any of the patch's versions.

Br, David

> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 3712359..8f1a6d5 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -404,16 +404,16 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
>  
>  #else
>  
> -static int xhci_try_enable_msi(struct usb_hcd *hcd)
> +static inline int xhci_try_enable_msi(struct usb_hcd *hcd)
>  {
>   return 0;
>  }
>  
> -static void xhci_cleanup_msix(struct xhci_hcd *xhci)
> +static inline void xhci_cleanup_msix(struct xhci_hcd *xhci)
>  {
>  }
>  
> -static void xhci_msix_sync_irqs(struct xhci_hcd *xhci)
> +static inline void xhci_msix_sync_irqs(struct xhci_hcd *xhci)
>  {
>  }
>  
> 
> -- 
> balbi


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

Re: [RESEND] [PATCH] xhci: Switch Intel Lynx Point ports to EHCI on shutdown.

[Sarah Sharp]

Sorry for the delay in reviewing this.  It helps me if you don't make the
patch in-reply-to a months old thread. :)  I'll take a look at this
shortly.

Sarah Sharp

On Tue, Feb 18, 2014 at 09:42:39AM +0200, Denis Turischev wrote:
> The same issue like with Panther Point chipsets. If the USB ports are
> switched to xHCI on shutdown, the xHCI host will send a spurious interrupt,
> which will wake the system. Some BIOS have work around for this, but not all.
> One example is Compulab's mini-desktop, the Intense-PC2.
> 
> The bug can be avoided if the USB ports are switched back to EHCI on
> shutdown.
> 
> Signed-off-by: Denis Turischev 
> ---
>  drivers/usb/host/xhci-pci.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 3c898c1..9233d12 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -134,6 +134,8 @@ static void xhci_pci_quirks(struct device *dev, struct 
> xhci_hcd *xhci)
>*/
>   if (pdev->subsystem_vendor == PCI_VENDOR_ID_HP)
>   xhci->quirks |= XHCI_SPURIOUS_WAKEUP;
> +
> + xhci->quirks |= XHCI_SPURIOUS_REBOOT;
>   }
>   if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
>   pdev->device == PCI_DEVICE_ID_ASROCK_P67) {
> -- 1.8.1.2
> 
> 
--
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 v2] usb/xhci: fix compilation warning when !CONFIG_PCI && !CONFIG_PM

[Felipe Balbi]

On Tue, Feb 18, 2014 at 10:00:30AM -0800, David Cohen wrote:
> Hi Sarah,
> 
> On Mon, Jan 06, 2014 at 07:02:19PM -0800, David Cohen wrote:
> > When CONFIG_PCI and CONFIG_PM are not selected, xhci.c gets this
> > warning:
> > drivers/usb/host/xhci.c:409:13: warning: ‘xhci_msix_sync_irqs’ defined
> > but not used [-Wunused-function]
> > 
> > It happens due to lack of __maybe_unused flag on xhci_msix_sync_irqs()
> > function in case of !CONFIG_PCI.
> > 
> > Signed-off-by: David Cohen 
> > ---
> 
> Ping :)
> Any comments here?
> 
> Br, David
> 
> > 
> > Change v1 -> v2:
> >  - xhci_msix_sync_irqs() already uses __maybe_unused flag when CONFIG_PCI is
> >set. Proper solution is to add same flag when !CONFIG_PCI instead of 
> > define
> >function as inline.
> > 
> >  drivers/usb/host/xhci.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > index 4265b48856f6..ed6b717b8ee1 100644
> > --- a/drivers/usb/host/xhci.c
> > +++ b/drivers/usb/host/xhci.c
> > @@ -406,7 +406,7 @@ static void xhci_cleanup_msix(struct xhci_hcd *xhci)
> >  {
> >  }
> >  
> > -static void xhci_msix_sync_irqs(struct xhci_hcd *xhci)
> > +static void __maybe_unused xhci_msix_sync_irqs(struct xhci_hcd *xhci)

bellow is likely a better fix. Usually stubs are marked inline, rather
than getting an unused attribute:

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 3712359..8f1a6d5 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -404,16 +404,16 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
 
 #else
 
-static int xhci_try_enable_msi(struct usb_hcd *hcd)
+static inline int xhci_try_enable_msi(struct usb_hcd *hcd)
 {
return 0;
 }
 
-static void xhci_cleanup_msix(struct xhci_hcd *xhci)
+static inline void xhci_cleanup_msix(struct xhci_hcd *xhci)
 {
 }
 
-static void xhci_msix_sync_irqs(struct xhci_hcd *xhci)
+static inline void xhci_msix_sync_irqs(struct xhci_hcd *xhci)
 {
 }
 

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2] usb/xhci: fix compilation warning when !CONFIG_PCI && !CONFIG_PM

[David Cohen]

Hi Sarah,

On Mon, Jan 06, 2014 at 07:02:19PM -0800, David Cohen wrote:
> When CONFIG_PCI and CONFIG_PM are not selected, xhci.c gets this
> warning:
> drivers/usb/host/xhci.c:409:13: warning: ‘xhci_msix_sync_irqs’ defined
> but not used [-Wunused-function]
> 
> It happens due to lack of __maybe_unused flag on xhci_msix_sync_irqs()
> function in case of !CONFIG_PCI.
> 
> Signed-off-by: David Cohen 
> ---

Ping :)
Any comments here?

Br, David

> 
> Change v1 -> v2:
>  - xhci_msix_sync_irqs() already uses __maybe_unused flag when CONFIG_PCI is
>set. Proper solution is to add same flag when !CONFIG_PCI instead of define
>function as inline.
> 
>  drivers/usb/host/xhci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 4265b48856f6..ed6b717b8ee1 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -406,7 +406,7 @@ static void xhci_cleanup_msix(struct xhci_hcd *xhci)
>  {
>  }
>  
> -static void xhci_msix_sync_irqs(struct xhci_hcd *xhci)
> +static void __maybe_unused xhci_msix_sync_irqs(struct xhci_hcd *xhci)
>  {
>  }
>  
> -- 
> 1.8.4.2
--
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 v4 1/2] usb: musb: dsps, debugfs files

[Felipe Balbi]

Hi,

On Tue, Feb 18, 2014 at 09:30:21AM -0800, Greg KH wrote:
> > > > > +static int dsps_musb_dbg_init(struct musb *musb, struct dsps_glue 
> > > > > *glue)
> > > > > +{
> > > > > + struct dentry *root;
> > > > > + struct dentry *file;
> > > > > + char buf[128];
> > > > > +
> > > > > + sprintf(buf, "%s.dsps", dev_name(musb->controller));
> > > > > + root = debugfs_create_dir(buf, NULL);
> > > > > + if (!root)
> > > > 
> > > > wrong, you should be using IS_ERR()
> > > 
> > > !root is fine, IS_ERR() will fail if CONFIG_DEBUGFS is not enabled.
> > 
> > in that case, files will be created on parent directory right ?
> 
> If, for some reason, creating a directory fails and then creating a file
> would succeed, yes, that will happen.
> 
> > If we pass a ERR_PTR(-ENODEV), otoh, we will try to dereference it in
> > __create_file().
> 
> No, because -ENODEV will only happen if debugfs is not enabled, so no
> dereference will ever happen.
> 
> Don't worry about checking return values from debugfs, it shouldn't be
> needed.

aha, now I see. I missed the:

348 if (error) {
349 dentry = NULL;
350 simple_release_fs(&debugfs_mount, &debugfs_mount_count);
351 }

in fs/debugfs/inode.c::__create_file(). I'll apply this patch in a few
hours (randconfig running).

cheers

-- 
balbi


signature.asc
Description: Digital signature

Re: USB 3380 using net2280 driver

[Amit Uttamchandani]

On Tue, Feb 18, 2014 at 07:55:13AM -0800, Kevin Cernekee wrote:
> On Tue, Feb 18, 2014 at 7:12 AM, Felipe Balbi  wrote:
> > On Mon, Feb 17, 2014 at 04:43:11PM -0800, Amit Uttamchandani wrote:
> >> I was looking at the changes for net2280.c and saw your name come up in
> >> a few of the more recent changes. I wanted to know, are you aware of
> >> support for PLXs USB 338o using this net2280 driver?
> >
> > no, those are two completely different controllers. Linux doesn't
> > support USB 3380 as of today.
> 
> It might be possible to use this code as a starting point:
> 
> http://patchwork.openwrt.org/patch/2889/

Thanks. Yes, I had found that patch sometime back. It actually
originates from the linux driver provided by PLX (search for Linux SDK):

http://www.plxtech.com/products/usbcontrollers/usb3380

The driver provided by PLX has quite a bit of changes to other files in
the gadget directory as well.

So I had thought if net2280 was similar enough to usb3380, that it can
be modified to support the basic functionality of usb3380. But based on
the responses, it looks like it is a completely different architecture.
--
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 v2 2/3] usb: chipidea: msm: Add device tree support

[Sergei Shtylyov]

On 02/18/2014 08:14 PM, Ivan T. Ivanov wrote:


From: "Ivan T. Ivanov" 



Allows controller to be specified via device tree.
Pass PHY phandle specified in DT to core driver.



Signed-off-by: Ivan T. Ivanov 
---
   drivers/usb/chipidea/ci_hdrc_msm.c |   23 ++-
   1 file changed, 22 insertions(+), 1 deletion(-)



 You also need to describe the binding you're creating in
Documentation/devicetree/bindings/usb/.txt.



Did you check "[PATCH v2 1/3]"?


   Did you send it to 'linux-usb'? I just didn't get the patch.
(Typically, the bindings are described in the same patch the DT support is 
added to the driver bu YMMV, of course.)



Regards,
Ivan


WBR, Sergei

--
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 v4 1/2] usb: musb: dsps, debugfs files

[Greg KH]

On Tue, Feb 18, 2014 at 11:03:35AM -0600, Felipe Balbi wrote:
> On Tue, Feb 18, 2014 at 08:59:11AM -0800, Greg KH wrote:
> > On Tue, Feb 18, 2014 at 10:20:54AM -0600, Felipe Balbi wrote:
> > > On Fri, Jan 17, 2014 at 10:22:35AM +0100, Markus Pargmann wrote:
> > > > debugfs files to show the contents of important dsps registers.
> > > > 
> > > > Signed-off-by: Markus Pargmann 
> > > > ---
> > > >  drivers/usb/musb/musb_dsps.c | 54 
> > > > 
> > > >  1 file changed, 54 insertions(+)
> > > > 
> > > > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> > > > index 593d3c9..d0a97d6 100644
> > > > --- a/drivers/usb/musb/musb_dsps.c
> > > > +++ b/drivers/usb/musb/musb_dsps.c
> > > > @@ -46,6 +46,8 @@
> > > >  #include 
> > > >  #include 
> > > >  
> > > > +#include 
> > > > +
> > > >  #include "musb_core.h"
> > > >  
> > > >  static const struct of_device_id musb_dsps_of_match[];
> > > > @@ -137,6 +139,26 @@ struct dsps_glue {
> > > > unsigned long last_timer;/* last timer data for each 
> > > > instance */
> > > >  
> > > > struct dsps_context context;
> > > > +   struct debugfs_regset32 regset;
> > > > +   struct dentry *dbgfs_root;
> > > > +};
> > > > +
> > > > +static const struct debugfs_reg32 dsps_musb_regs[] = {
> > > > +   { "revision",   0x00 },
> > > > +   { "control",0x14 },
> > > > +   { "status", 0x18 },
> > > > +   { "eoi",0x24 },
> > > > +   { "intr0_stat", 0x30 },
> > > > +   { "intr1_stat", 0x34 },
> > > > +   { "intr0_set",  0x38 },
> > > > +   { "intr1_set",  0x3c },
> > > > +   { "txmode", 0x70 },
> > > > +   { "rxmode", 0x74 },
> > > > +   { "autoreq",0xd0 },
> > > > +   { "srpfixtime", 0xd4 },
> > > > +   { "tdown",  0xd8 },
> > > > +   { "phy_utmi",   0xe0 },
> > > > +   { "mode",   0xe8 },
> > > >  };
> > > >  
> > > >  static void dsps_musb_try_idle(struct musb *musb, unsigned long 
> > > > timeout)
> > > > @@ -369,6 +391,30 @@ out:
> > > > return ret;
> > > >  }
> > > >  
> > > > +static int dsps_musb_dbg_init(struct musb *musb, struct dsps_glue 
> > > > *glue)
> > > > +{
> > > > +   struct dentry *root;
> > > > +   struct dentry *file;
> > > > +   char buf[128];
> > > > +
> > > > +   sprintf(buf, "%s.dsps", dev_name(musb->controller));
> > > > +   root = debugfs_create_dir(buf, NULL);
> > > > +   if (!root)
> > > 
> > > wrong, you should be using IS_ERR()
> > 
> > !root is fine, IS_ERR() will fail if CONFIG_DEBUGFS is not enabled.
> 
> in that case, files will be created on parent directory right ?

If, for some reason, creating a directory fails and then creating a file
would succeed, yes, that will happen.

> If we pass a ERR_PTR(-ENODEV), otoh, we will try to dereference it in
> __create_file().

No, because -ENODEV will only happen if debugfs is not enabled, so no
dereference will ever happen.

Don't worry about checking return values from debugfs, it shouldn't be
needed.

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: [PATCH] usb: musb: correct use of schedule_delayed_work()

[Felipe Balbi]

On Tue, Feb 18, 2014 at 06:22:57PM +0100, Daniel Mack wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
> 
> On 02/18/2014 05:44 PM, Felipe Balbi wrote:
> > On Tue, Feb 18, 2014 at 05:41:12PM +0100, Daniel Mack wrote:
> >> On 02/18/2014 05:33 PM, Felipe Balbi wrote:
> >>> On Wed, Feb 05, 2014 at 03:34:18PM +0100, Daniel Mack wrote:
>  schedule_delayed_work() takes the delay in jiffies, not
>  msecs.
>  
>  This bug slipped in with the recent reset logic cleanup 
>  (8ed1fb790ea: "usb: musb: finish suspend/reset work
>  independently from musb_hub_control()").
>  
>  Signed-off-by: Daniel Mack 
> >>> 
> >>> doesn't apply. Please refresh against my testing/fixes (give me
> >>> about an hour until I push the updated branch though).
> >> 
> >> Oh, I'm sorry. Greg said he would queue them up as you were on
> >> vacation, so I don't know what's the status here. Greg, can you
> >> still drop the patches so they can go through Felipe's tree?
> >> 
> >> Might be best not to cause merge trouble here ...
> > 
> > If it's already in Greg's tree, don't worry ;-) I'm just trying to
> > catch up with my inbox ;-)
> 
> Can't seem to find my patches anywhere in Greg's git.
> 
> I saw that you updated your testing/fixes branch and wanted to rebase,
> but the remaining patch ("usb: musb: correct use of
> schedule_delayed_work()") applies on top of that head[*] just fine.
> Same for you?

weird, maybe I applied another dependency which I didn't have at the
time. Will try in a bit, running my 300 randconfig build script now,
it'll take a few hours I guess :-p

cheers

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH] usb: musb: correct use of schedule_delayed_work()

[Daniel Mack]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 02/18/2014 05:44 PM, Felipe Balbi wrote:
> On Tue, Feb 18, 2014 at 05:41:12PM +0100, Daniel Mack wrote:
>> On 02/18/2014 05:33 PM, Felipe Balbi wrote:
>>> On Wed, Feb 05, 2014 at 03:34:18PM +0100, Daniel Mack wrote:
 schedule_delayed_work() takes the delay in jiffies, not
 msecs.
 
 This bug slipped in with the recent reset logic cleanup 
 (8ed1fb790ea: "usb: musb: finish suspend/reset work
 independently from musb_hub_control()").
 
 Signed-off-by: Daniel Mack 
>>> 
>>> doesn't apply. Please refresh against my testing/fixes (give me
>>> about an hour until I push the updated branch though).
>> 
>> Oh, I'm sorry. Greg said he would queue them up as you were on
>> vacation, so I don't know what's the status here. Greg, can you
>> still drop the patches so they can go through Felipe's tree?
>> 
>> Might be best not to cause merge trouble here ...
> 
> If it's already in Greg's tree, don't worry ;-) I'm just trying to
> catch up with my inbox ;-)

Can't seem to find my patches anywhere in Greg's git.

I saw that you updated your testing/fixes branch and wanted to rebase,
but the remaining patch ("usb: musb: correct use of
schedule_delayed_work()") applies on top of that head[*] just fine.
Same for you?


Thanks,
Daniel


[*]
https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/fixes&id=3823b71
-BEGIN PGP SIGNATURE-
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJTA5bxAAoJELphMRr8Y1Qkm64P/3HlRJNSfMNaSp8MIVgnjJ43
xbpeyvcLiNaInPpw+Oz0NiQ+yK6z70YTn0+hA4YJfyQbbVsRI3b4QrzSR6KzGCMV
afhJ4xyVqEshD6zWC2toiOgT02aBG4xZVfg+17m3RwcpFMAANIuHhnFSDtC5z99y
igyfoIZSb3rR3GOSyEU+dq6VcGy047IYbMBgDlmMmhA/Z6BuIKWohT6olyomWti5
ExGgv6g3H3C26nvIm1YcMo8bmn5+LAuY+lzjialcxixnahOFdP5y/o+l00dWIZcw
fGA87uWWz9yP24qFP40d1dt1av79BdsJMQNCw98FRAWdzRBNKzRiYL+laNe9wBKY
nNyghQIxfHZTaL+gJtCflxkQY51VA+30i34a5idxq3cpSFfSx9XVwGp+DEqPfccA
V4GdHxfpiJqvZ64fKJRTdKlVJ6mWkpWbEwnitOOk6x6dkYOsi0Evi6zUedbzLa5h
0gR7vZik8gJi4KzzdfPDYp7rbbvIJA7ahXNIazpy4/PchbthYhIWQcJ/UGpk/7JA
QN4Rd1SCs0ELC7EOQxDAwqudssqC3YUjj+HY/JWjMIeC/Au7Wyhk7Ty3ZHTpJq7F
rtfRtmxewUsbH1xeJd0k+rtp8dp7Wl/2Qr5oxp2qKiMeSBMEjEkJ6B7I4fTzpM7W
Wn7uhY7mAEQKhB2kH4pg
=qyOR
-END PGP SIGNATURE-
--
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 net-next 07/14] r8152: combine PHY reset with set_speed

[Florian Fainelli]

Hi Hayes,

2014-02-18 5:49 GMT-08:00 Hayes Wang :
> PHY reset is necessary after some hw settings. However, it would
> cause the linking down, and so does the set_speed function. Combine
> the PHY reset with set_speed function. That could reduce the frequency
> of linking down and accessing the PHY register.
>
> Signed-off-by: Hayes Wang 
> ---
>  drivers/net/usb/r8152.c | 57 
> ++---
>  1 file changed, 45 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index c7bae39..b3155da 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -436,6 +436,7 @@ enum rtl8152_flags {
> RTL8152_SET_RX_MODE,
> WORK_ENABLE,
> RTL8152_LINK_CHG,
> +   PHY_RESET,
>  };
>
>  /* Define these values to match your device */
> @@ -1796,6 +1797,29 @@ static void r8152_power_cut_en(struct r8152 *tp, bool 
> enable)
>
>  }
>
> +static void rtl_phy_reset(struct r8152 *tp)
> +{
> +   u16 data;
> +   int i;
> +
> +   clear_bit(PHY_RESET, &tp->flags);
> +
> +   data = r8152_mdio_read(tp, MII_BMCR);
> +
> +   /* don't reset again before the previous one complete */
> +   if (data & BMCR_RESET)
> +   return;
> +
> +   data |= BMCR_RESET;
> +   r8152_mdio_write(tp, MII_BMCR, data);
> +
> +   for (i = 0; i < 50; i++) {
> +   msleep(20);
> +   if ((r8152_mdio_read(tp, MII_BMCR) & BMCR_RESET) == 0)
> +   break;
> +   }
> +}

If you implemented libphy in the driver you would not have to
duplicate that and you could use "phy_init_hw()" or
genphy_soft_reset() to perform the BMCR-based software reset.

> +
>  static void rtl_clear_bp(struct r8152 *tp)
>  {
> ocp_write_dword(tp, MCU_TYPE_PLA, PLA_BP_0, 0);
> @@ -1854,6 +1878,7 @@ static void r8152b_hw_phy_cfg(struct r8152 *tp)
> }
>
> r8152b_disable_aldps(tp);
> +   set_bit(PHY_RESET, &tp->flags);
>  }
>
>  static void r8152b_exit_oob(struct r8152 *tp)
> @@ -2042,6 +2067,8 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
> data = sram_read(tp, SRAM_10M_AMP2);
> data |= AMP_DN;
> sram_write(tp, SRAM_10M_AMP2, data);
> +
> +   set_bit(PHY_RESET, &tp->flags);
>  }
>
>  static void r8153_u1u2en(struct r8152 *tp, bool enable)
> @@ -2295,12 +2322,26 @@ static int rtl8152_set_speed(struct r8152 *tp, u8 
> autoneg, u16 speed, u8 duplex)
> bmcr = BMCR_ANENABLE | BMCR_ANRESTART;
> }
>
> +   if (test_bit(PHY_RESET, &tp->flags))
> +   bmcr |= BMCR_RESET;
> +
> if (tp->mii.supports_gmii)
> r8152_mdio_write(tp, MII_CTRL1000, gbcr);
>
> r8152_mdio_write(tp, MII_ADVERTISE, anar);
> r8152_mdio_write(tp, MII_BMCR, bmcr);
>
> +   if (test_bit(PHY_RESET, &tp->flags)) {
> +   int i;
> +
> +   clear_bit(PHY_RESET, &tp->flags);
> +   for (i = 0; i < 50; i++) {
> +   msleep(20);
> +   if ((r8152_mdio_read(tp, MII_BMCR) & BMCR_RESET) == 0)
> +   break;
> +   }
> +   }
> +
>  out:
>
> return ret;
> @@ -2364,6 +2405,10 @@ static void rtl_work_func_t(struct work_struct *work)
> if (test_bit(RTL8152_SET_RX_MODE, &tp->flags))
> _rtl8152_set_rx_mode(tp->netdev);
>
> +
> +   if (test_bit(PHY_RESET, &tp->flags))
> +   rtl_phy_reset(tp);
> +
>  out1:
> return;
>  }
> @@ -2459,7 +2504,6 @@ static void r8152b_enable_fc(struct r8152 *tp)
>  static void r8152b_init(struct r8152 *tp)
>  {
> u32 ocp_data;
> -   int i;
>
> rtl_clear_bp(tp);
>
> @@ -2491,14 +2535,6 @@ static void r8152b_init(struct r8152 *tp)
> r8152b_enable_aldps(tp);
> r8152b_enable_fc(tp);
>
> -   r8152_mdio_write(tp, MII_BMCR, BMCR_RESET | BMCR_ANENABLE |
> -  BMCR_ANRESTART);
> -   for (i = 0; i < 100; i++) {
> -   udelay(100);
> -   if (!(r8152_mdio_read(tp, MII_BMCR) & BMCR_RESET))
> -   break;
> -   }
> -
> /* enable rx aggregation */
> ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_USB_CTRL);
> ocp_data &= ~RX_AGG_DISABLE;
> @@ -2569,9 +2605,6 @@ static void r8153_init(struct r8152 *tp)
> r8153_enable_eee(tp);
> r8153_enable_aldps(tp);
> r8152b_enable_fc(tp);
> -
> -   r8152_mdio_write(tp, MII_BMCR, BMCR_RESET | BMCR_ANENABLE |
> -  BMCR_ANRESTART);
>  }
>
>  static int rtl8152_suspend(struct usb_interface *intf, pm_message_t message)
> --
> 1.8.4.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 

Re: [PATCH v2 2/3] usb: chipidea: msm: Add device tree support

[Ivan T. Ivanov]

On Tue, 2014-02-18 at 08:08 -0600, Josh Cartwright wrote: 
> Hey Ivan-
> 
> Nit below.
> 
> On Tue, Feb 18, 2014 at 03:21:20PM +0200, Ivan T. Ivanov wrote:

> >  
> > +static struct of_device_id msm_ci_dt_match[] = {
> 
> const?
> 

Thanks, will do.

Regards,
Ivan

> > +   { .compatible = "qcom,ci-hdrc", },
> > +   { }
> > +};
> > +MODULE_DEVICE_TABLE(of, msm_ci_dt_match);
> 


--
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 v2 2/3] usb: chipidea: msm: Add device tree support

[Ivan T. Ivanov]

Hi, 

On Tue, 2014-02-18 at 20:53 +0300, Sergei Shtylyov wrote: 
> Hello.
> 
> On 02/18/2014 04:21 PM, Ivan T. Ivanov wrote:
> 
> > From: "Ivan T. Ivanov" 
> 
> > Allows controller to be specified via device tree.
> > Pass PHY phandle specified in DT to core driver.
> 
> > Signed-off-by: Ivan T. Ivanov 
> > ---
> >   drivers/usb/chipidea/ci_hdrc_msm.c |   23 ++-
> >   1 file changed, 22 insertions(+), 1 deletion(-)
> 
> You also need to describe the binding you're creating in 
> Documentation/devicetree/bindings/usb/.txt.

Did you check "[PATCH v2 1/3]"?

Regards,
Ivan

> 
> WBR, Sergei
> 


--
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 v4 1/2] usb: musb: dsps, debugfs files

[Felipe Balbi]

On Tue, Feb 18, 2014 at 08:59:11AM -0800, Greg KH wrote:
> On Tue, Feb 18, 2014 at 10:20:54AM -0600, Felipe Balbi wrote:
> > On Fri, Jan 17, 2014 at 10:22:35AM +0100, Markus Pargmann wrote:
> > > debugfs files to show the contents of important dsps registers.
> > > 
> > > Signed-off-by: Markus Pargmann 
> > > ---
> > >  drivers/usb/musb/musb_dsps.c | 54 
> > > 
> > >  1 file changed, 54 insertions(+)
> > > 
> > > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> > > index 593d3c9..d0a97d6 100644
> > > --- a/drivers/usb/musb/musb_dsps.c
> > > +++ b/drivers/usb/musb/musb_dsps.c
> > > @@ -46,6 +46,8 @@
> > >  #include 
> > >  #include 
> > >  
> > > +#include 
> > > +
> > >  #include "musb_core.h"
> > >  
> > >  static const struct of_device_id musb_dsps_of_match[];
> > > @@ -137,6 +139,26 @@ struct dsps_glue {
> > >   unsigned long last_timer;/* last timer data for each instance */
> > >  
> > >   struct dsps_context context;
> > > + struct debugfs_regset32 regset;
> > > + struct dentry *dbgfs_root;
> > > +};
> > > +
> > > +static const struct debugfs_reg32 dsps_musb_regs[] = {
> > > + { "revision",   0x00 },
> > > + { "control",0x14 },
> > > + { "status", 0x18 },
> > > + { "eoi",0x24 },
> > > + { "intr0_stat", 0x30 },
> > > + { "intr1_stat", 0x34 },
> > > + { "intr0_set",  0x38 },
> > > + { "intr1_set",  0x3c },
> > > + { "txmode", 0x70 },
> > > + { "rxmode", 0x74 },
> > > + { "autoreq",0xd0 },
> > > + { "srpfixtime", 0xd4 },
> > > + { "tdown",  0xd8 },
> > > + { "phy_utmi",   0xe0 },
> > > + { "mode",   0xe8 },
> > >  };
> > >  
> > >  static void dsps_musb_try_idle(struct musb *musb, unsigned long timeout)
> > > @@ -369,6 +391,30 @@ out:
> > >   return ret;
> > >  }
> > >  
> > > +static int dsps_musb_dbg_init(struct musb *musb, struct dsps_glue *glue)
> > > +{
> > > + struct dentry *root;
> > > + struct dentry *file;
> > > + char buf[128];
> > > +
> > > + sprintf(buf, "%s.dsps", dev_name(musb->controller));
> > > + root = debugfs_create_dir(buf, NULL);
> > > + if (!root)
> > 
> > wrong, you should be using IS_ERR()
> 
> !root is fine, IS_ERR() will fail if CONFIG_DEBUGFS is not enabled.

in that case, files will be created on parent directory right ? If we
pass a ERR_PTR(-ENODEV), otoh, we will try to dereference it in
__create_file().

-- 
balbi


signature.asc
Description: Digital signature

Re: [EHCI Debug Port] Linux fails to setup EHCI debug port

[Felipe Balbi]

On Tue, Feb 18, 2014 at 09:01:29AM -0800, Greg KH wrote:
> On Tue, Feb 18, 2014 at 02:46:00PM +0800, Lu, Baolu wrote:
> > Hi list,
> > 
> > Linux kernel (3.12.8) fails to setup EHCI debug port on my Sandy Bridge
> > server. This seems to be a normal thing as I tried on other machines and got
> > the same result.
> 
> Do you have a EHCI debug connector to attach to it?  That is required in

you can make one, if you wish:

http://www.coreboot.org/DIY_EHCI_debug_dongle

-- 
balbi


signature.asc
Description: Digital signature

Re: [EHCI Debug Port] Linux fails to setup EHCI debug port

[Greg KH]

On Tue, Feb 18, 2014 at 02:46:00PM +0800, Lu, Baolu wrote:
> Hi list,
> 
> Linux kernel (3.12.8) fails to setup EHCI debug port on my Sandy Bridge
> server. This seems to be a normal thing as I tried on other machines and got
> the same result.

Do you have a EHCI debug connector to attach to it?  That is required in
order to be able to enable this, it's a separate device you must plug
into the machine.

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: [PATCH v4 1/2] usb: musb: dsps, debugfs files

[Greg KH]

On Tue, Feb 18, 2014 at 10:20:54AM -0600, Felipe Balbi wrote:
> On Fri, Jan 17, 2014 at 10:22:35AM +0100, Markus Pargmann wrote:
> > debugfs files to show the contents of important dsps registers.
> > 
> > Signed-off-by: Markus Pargmann 
> > ---
> >  drivers/usb/musb/musb_dsps.c | 54 
> > 
> >  1 file changed, 54 insertions(+)
> > 
> > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> > index 593d3c9..d0a97d6 100644
> > --- a/drivers/usb/musb/musb_dsps.c
> > +++ b/drivers/usb/musb/musb_dsps.c
> > @@ -46,6 +46,8 @@
> >  #include 
> >  #include 
> >  
> > +#include 
> > +
> >  #include "musb_core.h"
> >  
> >  static const struct of_device_id musb_dsps_of_match[];
> > @@ -137,6 +139,26 @@ struct dsps_glue {
> > unsigned long last_timer;/* last timer data for each instance */
> >  
> > struct dsps_context context;
> > +   struct debugfs_regset32 regset;
> > +   struct dentry *dbgfs_root;
> > +};
> > +
> > +static const struct debugfs_reg32 dsps_musb_regs[] = {
> > +   { "revision",   0x00 },
> > +   { "control",0x14 },
> > +   { "status", 0x18 },
> > +   { "eoi",0x24 },
> > +   { "intr0_stat", 0x30 },
> > +   { "intr1_stat", 0x34 },
> > +   { "intr0_set",  0x38 },
> > +   { "intr1_set",  0x3c },
> > +   { "txmode", 0x70 },
> > +   { "rxmode", 0x74 },
> > +   { "autoreq",0xd0 },
> > +   { "srpfixtime", 0xd4 },
> > +   { "tdown",  0xd8 },
> > +   { "phy_utmi",   0xe0 },
> > +   { "mode",   0xe8 },
> >  };
> >  
> >  static void dsps_musb_try_idle(struct musb *musb, unsigned long timeout)
> > @@ -369,6 +391,30 @@ out:
> > return ret;
> >  }
> >  
> > +static int dsps_musb_dbg_init(struct musb *musb, struct dsps_glue *glue)
> > +{
> > +   struct dentry *root;
> > +   struct dentry *file;
> > +   char buf[128];
> > +
> > +   sprintf(buf, "%s.dsps", dev_name(musb->controller));
> > +   root = debugfs_create_dir(buf, NULL);
> > +   if (!root)
> 
> wrong, you should be using IS_ERR()

!root is fine, IS_ERR() will fail if CONFIG_DEBUGFS is not enabled.

--
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] u_ether: move receiving to RX workqueue

[Felipe Balbi]

Hi,

On Mon, Feb 17, 2014 at 02:26:54PM +0800, Clanlab (Taiwan) Linux Project wrote:
> @@ -1168,5 +1191,23 @@ void gether_disconnect(struct gether *link)
>  }
>  EXPORT_SYMBOL(gether_disconnect);
>  
> +static int __init gether_init(void)
> +{
> + gether_wq  = create_singlethread_workqueue("gether");
> + if (gether_wq == NULL) {
> + pr_err("Cannot initialize work queue\n");
> + return -ENOMEM;
> + }
> + return 0;
> +}
> +module_init(gether_init);
> +
> +static void __exit gether_exit(void)
> +{
> + destroy_workqueue(gether_wq);
> +

trailing whitespace.

> +}
> +module_exit(gether_exit);

this is actually supposed to be a library which gets linked against the
actual function module. You need to find a better way to ensure
destroy_workqueue is called.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 2/3] usb: chipidea: msm: Add device tree support

[Sergei Shtylyov]

Hello.

On 02/18/2014 04:21 PM, Ivan T. Ivanov wrote:


From: "Ivan T. Ivanov" 



Allows controller to be specified via device tree.
Pass PHY phandle specified in DT to core driver.



Signed-off-by: Ivan T. Ivanov 
---
  drivers/usb/chipidea/ci_hdrc_msm.c |   23 ++-
  1 file changed, 22 insertions(+), 1 deletion(-)


   You also need to describe the binding you're creating in 
Documentation/devicetree/bindings/usb/.txt.


WBR, Sergei

--
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 v9 11/12] usb: phy-mxs: Add system suspend/resume API

[Felipe Balbi]

On Fri, Dec 27, 2013 at 10:38:40AM +0800, Peter Chen wrote:
> We need this to keep PHY's power on or off during the system
> suspend mode. If we need to enable USB wakeup, then we
> must keep PHY's power being on during the system suspend mode.
> Otherwise, we need to keep PHY's power being off to save power.
> 
> Signed-off-by: Peter Chen 
> ---
>  drivers/usb/phy/phy-mxs-usb.c |   61 
> +
>  1 files changed, 61 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index 96aac05..53b8dad 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -57,6 +57,10 @@
>  #define BM_USBPHY_DEBUG_CLKGATE  BIT(30)
>  
>  /* Anatop Registers */
> +#define ANADIG_ANA_MISC0 0x150
> +#define ANADIG_ANA_MISC0_SET 0x154
> +#define ANADIG_ANA_MISC0_CLR 0x158
> +
>  #define ANADIG_USB1_VBUS_DET_STAT0x1c0
>  #define ANADIG_USB2_VBUS_DET_STAT0x220
>  
> @@ -65,6 +69,9 @@
>  #define ANADIG_USB2_LOOPBACK_SET 0x244
>  #define ANADIG_USB2_LOOPBACK_CLR 0x248
>  
> +#define BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG BIT(12)
> +#define BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG_SL BIT(11)
> +
>  #define BM_ANADIG_USB1_VBUS_DET_STAT_VBUS_VALID  BIT(3)
>  #define BM_ANADIG_USB2_VBUS_DET_STAT_VBUS_VALID  BIT(3)
>  
> @@ -134,6 +141,16 @@ struct mxs_phy {
>   int port_id;
>  };
>  
> +static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
> +{
> + return mxs_phy->data == &imx6q_phy_data;
> +}
> +
> +static inline bool is_imx6sl_phy(struct mxs_phy *mxs_phy)
> +{
> + return mxs_phy->data == &imx6sl_phy_data;
> +}
> +
>  static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
>  {
>   int ret;
> @@ -382,6 +399,8 @@ static int mxs_phy_probe(struct platform_device *pdev)
>  
>   platform_set_drvdata(pdev, mxs_phy);
>  
> + device_set_wakeup_capable(&pdev->dev, true);
> +
>   ret = usb_add_phy_dev(&mxs_phy->phy);
>   if (ret)
>   return ret;
> @@ -398,6 +417,47 @@ static int mxs_phy_remove(struct platform_device *pdev)
>   return 0;
>  }
>  
> +#ifdef CONFIG_PM_SLEEP

please use CONFIG_PM instead.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v9 01/12] usb: doc: phy-mxs: Add more compatible strings

[Felipe Balbi]

On Fri, Dec 27, 2013 at 10:38:30AM +0800, Peter Chen wrote:
> Add "fsl,imx6q-usbphy" for imx6dq and imx6dl, add
> "fsl,imx6sl-usbphy" for imx6sl.
> 
> Signed-off-by: Peter Chen 

anybody from DT to give me an Acked-by ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH] usb: musb: correct use of schedule_delayed_work()

[Felipe Balbi]

On Tue, Feb 18, 2014 at 05:41:12PM +0100, Daniel Mack wrote:
> On 02/18/2014 05:33 PM, Felipe Balbi wrote:
> > On Wed, Feb 05, 2014 at 03:34:18PM +0100, Daniel Mack wrote:
> >> schedule_delayed_work() takes the delay in jiffies, not msecs.
> >>
> >> This bug slipped in with the recent reset logic cleanup
> >> (8ed1fb790ea: "usb: musb: finish suspend/reset work independently from
> >> musb_hub_control()").
> >>
> >> Signed-off-by: Daniel Mack 
> > 
> > doesn't apply. Please refresh against my testing/fixes (give me about an
> > hour until I push the updated branch though).
> 
> Oh, I'm sorry. Greg said he would queue them up as you were on vacation,
> so I don't know what's the status here. Greg, can you still drop the
> patches so they can go through Felipe's tree?
> 
> Might be best not to cause merge trouble here ...

If it's already in Greg's tree, don't worry ;-) I'm just trying to catch
up with my inbox ;-)

cheers

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2] usb: phy: msm: fix compilation errors when !CONFIG_PM_SLEEP

[Felipe Balbi]

On Tue, Feb 18, 2014 at 10:33:21AM -0600, Josh Cartwright wrote:
> On Tue, Feb 18, 2014 at 10:24:16AM -0600, Felipe Balbi wrote:
> > On Fri, Jan 17, 2014 at 12:26:50PM -0600, Josh Cartwright wrote:
> > > On Fri, Jan 17, 2014 at 11:58:51AM -0600, Josh Cartwright wrote:
> > > > Both the PM_RUNTIME and PM_SLEEP callbacks call into the common
> > > > msm_otg_{suspend,resume} routines, however these routines are only being
> > > > built when CONFIG_PM_SLEEP.  In addition, msm_otg_{suspend,resume} also
> > > > depends on msm_hsusb_config_vddcx(), which is only built when
> > > > CONFIG_PM_SLEEP.
> > > > 
> > > > Fix the CONFIG_PM_RUNTIME, !CONFIG_PM_SLEEP case by changing the
> > > > preprocessor conditional, and moving msm_hsusb_config_vddcx().
> > > > 
> > > > While we're here, eliminate the CONFIG_PM conditional for setting
> > > > up the dev_pm_ops.
> > > > 
> > > > This address the following errors Russell King has hit doing randconfig
> > > > builds:
> > > > 
> > > > drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_suspend':
> > > > drivers/usb/phy/phy-msm-usb.c:1691:2: error: implicit declaration of 
> > > > function 'msm_otg_suspend'
> > > > drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_resume':
> > > > drivers/usb/phy/phy-msm-usb.c:1699:2: error: implicit declaration of 
> > > > function 'msm_otg_resume'
> > > > 
> > > > Cc: Ivan T. Ivanov 
> > > > Reported-by: Russell King 
> > > > Signed-off-by: Josh Cartwright 
> > > > ---
> > > > v1->v2: Change conditional to simply CONFIG_PM (thanks ccov and 
> > > > khilman!)
> > > > 
> > > >  drivers/usb/phy/phy-msm-usb.c | 57 
> > > > ---
> > > >  1 file changed, 26 insertions(+), 31 deletions(-)
> > > > 
> > > > diff --git a/drivers/usb/phy/phy-msm-usb.c 
> > > > b/drivers/usb/phy/phy-msm-usb.c
> > > > index 8546c8d..5b169a7 100644
> > > > --- a/drivers/usb/phy/phy-msm-usb.c
> > > > +++ b/drivers/usb/phy/phy-msm-usb.c
> > > [..]
> > > > @@ -440,7 +414,32 @@ static int msm_otg_reset(struct usb_phy *phy)
> > > >  #define PHY_SUSPEND_TIMEOUT_USEC   (500 * 1000)
> > > >  #define PHY_RESUME_TIMEOUT_USEC(100 * 1000)
> > > >  
> > > > -#ifdef CONFIG_PM_SLEEP
> > > > +#if CONFIG_PM
> > > 
> > > *sigh*.  This, of course, should have been #ifdef CONFIG_PM.  Fixed
> > > v3 below.
> > 
> > sorry, please git send-email it properly.
> 
> No problem, will do.  FWIW, it's applicable with git am --scissors.

ahaa, I didn't know about that option. Thanks :-)

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH] usb: gadget: fix NULL pointer dereference

[Felipe Balbi]

On Tue, Feb 18, 2014 at 10:40:12AM -0600, Felipe Balbi wrote:
> On Fri, Jan 17, 2014 at 05:04:55PM +0100, Michal Nazarewicz wrote:
> > On Thu, Jan 16 2014, Andrzej Pietrasiewicz wrote:
> > > Fix possible NULL pointer dereference introduced in
> > >
> > > 219580e64f035bb9018dbb08d340f90b0ac50f8c
> > > usb: f_fs: check quirk to pad epout buf size when not aligned to
> > > maxpacketsize
> > >
> > > after 3.13-rc1.
> > >
> > > In cases we do wait with:
> > >
> > > wait_event_interruptible(epfile->wait, (ep = epfile->ep));
> > >
> > > for endpoint to be enabled, functionfs_bind() has not been called yet
> > > and epfile->ffs->gadget is still NULL and the automatic variable 'gadget'
> > > has been initialized with NULL at the point of its definition.
> > > Later on it is used as a parameter to:
> > >
> > > usb_ep_align_maybe(gadget, ep->ep, len)
> > >
> > > which in turn dereferences it.
> > >
> > > This patch fixes it by moving the actual assignment to the local 'gadget'
> > > variable after the potential waiting has completed.
> > >
> > > Signed-off-by: Andrzej Pietrasiewicz 
> > 
> > Acked-by: Michal Nazarewicz 
> > 
> > But since gadget is only used in the “if (!halt)” part of the code,
> > could you simply move definition of the variable inside the if?
> 
> should I wait for another revision ?

nevermind, you already sent it ;-)

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH] usb: musb: correct use of schedule_delayed_work()

[Daniel Mack]

On 02/18/2014 05:33 PM, Felipe Balbi wrote:
> On Wed, Feb 05, 2014 at 03:34:18PM +0100, Daniel Mack wrote:
>> schedule_delayed_work() takes the delay in jiffies, not msecs.
>>
>> This bug slipped in with the recent reset logic cleanup
>> (8ed1fb790ea: "usb: musb: finish suspend/reset work independently from
>> musb_hub_control()").
>>
>> Signed-off-by: Daniel Mack 
> 
> doesn't apply. Please refresh against my testing/fixes (give me about an
> hour until I push the updated branch though).

Oh, I'm sorry. Greg said he would queue them up as you were on vacation,
so I don't know what's the status here. Greg, can you still drop the
patches so they can go through Felipe's tree?

Might be best not to cause merge trouble here ...


Thanks,
Daniel




signature.asc
Description: OpenPGP digital signature

Re: [PATCH] usb: gadget: fix NULL pointer dereference

[Felipe Balbi]

On Fri, Jan 17, 2014 at 05:04:55PM +0100, Michal Nazarewicz wrote:
> On Thu, Jan 16 2014, Andrzej Pietrasiewicz wrote:
> > Fix possible NULL pointer dereference introduced in
> >
> > 219580e64f035bb9018dbb08d340f90b0ac50f8c
> > usb: f_fs: check quirk to pad epout buf size when not aligned to
> > maxpacketsize
> >
> > after 3.13-rc1.
> >
> > In cases we do wait with:
> >
> > wait_event_interruptible(epfile->wait, (ep = epfile->ep));
> >
> > for endpoint to be enabled, functionfs_bind() has not been called yet
> > and epfile->ffs->gadget is still NULL and the automatic variable 'gadget'
> > has been initialized with NULL at the point of its definition.
> > Later on it is used as a parameter to:
> >
> > usb_ep_align_maybe(gadget, ep->ep, len)
> >
> > which in turn dereferences it.
> >
> > This patch fixes it by moving the actual assignment to the local 'gadget'
> > variable after the potential waiting has completed.
> >
> > Signed-off-by: Andrzej Pietrasiewicz 
> 
> Acked-by: Michal Nazarewicz 
> 
> But since gadget is only used in the “if (!halt)” part of the code,
> could you simply move definition of the variable inside the if?

should I wait for another revision ?

-- 
balbi


signature.asc
Description: Digital signature

[PATCH RESEND v3] usb: phy: msm: fix compilation errors when !CONFIG_PM_SLEEP

[Josh Cartwright]

Both the PM_RUNTIME and PM_SLEEP callbacks call into the common
msm_otg_{suspend,resume} routines, however these routines are only being
built when CONFIG_PM_SLEEP.  In addition, msm_otg_{suspend,resume} also
depends on msm_hsusb_config_vddcx(), which is only built when
CONFIG_PM_SLEEP.

Fix the CONFIG_PM_RUNTIME, !CONFIG_PM_SLEEP case by changing the
preprocessor conditional, and moving msm_hsusb_config_vddcx().

While we're here, eliminate the CONFIG_PM conditional for setting
up the dev_pm_ops.

This address the following errors Russell King has hit doing randconfig
builds:

drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_suspend':
drivers/usb/phy/phy-msm-usb.c:1691:2: error: implicit declaration of function 
'msm_otg_suspend'
drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_resume':
drivers/usb/phy/phy-msm-usb.c:1699:2: error: implicit declaration of function 
'msm_otg_resume'

Cc: Ivan T. Ivanov 
Reported-by: Russell King 
Signed-off-by: Josh Cartwright 
---
 drivers/usb/phy/phy-msm-usb.c | 57 ---
 1 file changed, 26 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 64c9d14e..5b37b81 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -159,32 +159,6 @@ put_3p3:
return rc;
 }
 
-#ifdef CONFIG_PM_SLEEP
-#define USB_PHY_SUSP_DIG_VOL  50
-static int msm_hsusb_config_vddcx(int high)
-{
-   int max_vol = USB_PHY_VDD_DIG_VOL_MAX;
-   int min_vol;
-   int ret;
-
-   if (high)
-   min_vol = USB_PHY_VDD_DIG_VOL_MIN;
-   else
-   min_vol = USB_PHY_SUSP_DIG_VOL;
-
-   ret = regulator_set_voltage(hsusb_vddcx, min_vol, max_vol);
-   if (ret) {
-   pr_err("%s: unable to set the voltage for regulator "
-   "HSUSB_VDDCX\n", __func__);
-   return ret;
-   }
-
-   pr_debug("%s: min_vol:%d max_vol:%d\n", __func__, min_vol, max_vol);
-
-   return ret;
-}
-#endif
-
 static int msm_hsusb_ldo_set_mode(int on)
 {
int ret = 0;
@@ -440,7 +414,32 @@ static int msm_otg_reset(struct usb_phy *phy)
 #define PHY_SUSPEND_TIMEOUT_USEC   (500 * 1000)
 #define PHY_RESUME_TIMEOUT_USEC(100 * 1000)
 
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM
+
+#define USB_PHY_SUSP_DIG_VOL  50
+static int msm_hsusb_config_vddcx(int high)
+{
+   int max_vol = USB_PHY_VDD_DIG_VOL_MAX;
+   int min_vol;
+   int ret;
+
+   if (high)
+   min_vol = USB_PHY_VDD_DIG_VOL_MIN;
+   else
+   min_vol = USB_PHY_SUSP_DIG_VOL;
+
+   ret = regulator_set_voltage(hsusb_vddcx, min_vol, max_vol);
+   if (ret) {
+   pr_err("%s: unable to set the voltage for regulator "
+   "HSUSB_VDDCX\n", __func__);
+   return ret;
+   }
+
+   pr_debug("%s: min_vol:%d max_vol:%d\n", __func__, min_vol, max_vol);
+
+   return ret;
+}
+
 static int msm_otg_suspend(struct msm_otg *motg)
 {
struct usb_phy *phy = &motg->phy;
@@ -1734,22 +1733,18 @@ static int msm_otg_pm_resume(struct device *dev)
 }
 #endif
 
-#ifdef CONFIG_PM
 static const struct dev_pm_ops msm_otg_dev_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(msm_otg_pm_suspend, msm_otg_pm_resume)
SET_RUNTIME_PM_OPS(msm_otg_runtime_suspend, msm_otg_runtime_resume,
msm_otg_runtime_idle)
 };
-#endif
 
 static struct platform_driver msm_otg_driver = {
.remove = msm_otg_remove,
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
-#ifdef CONFIG_PM
.pm = &msm_otg_dev_pm_ops,
-#endif
},
 };
 
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
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: musb: correct use of schedule_delayed_work()

[Felipe Balbi]

On Wed, Feb 05, 2014 at 03:34:18PM +0100, Daniel Mack wrote:
> schedule_delayed_work() takes the delay in jiffies, not msecs.
> 
> This bug slipped in with the recent reset logic cleanup
> (8ed1fb790ea: "usb: musb: finish suspend/reset work independently from
> musb_hub_control()").
> 
> Signed-off-by: Daniel Mack 

doesn't apply. Please refresh against my testing/fixes (give me about an
hour until I push the updated branch though).

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2] usb: phy: msm: fix compilation errors when !CONFIG_PM_SLEEP

[Josh Cartwright]

On Tue, Feb 18, 2014 at 10:24:16AM -0600, Felipe Balbi wrote:
> On Fri, Jan 17, 2014 at 12:26:50PM -0600, Josh Cartwright wrote:
> > On Fri, Jan 17, 2014 at 11:58:51AM -0600, Josh Cartwright wrote:
> > > Both the PM_RUNTIME and PM_SLEEP callbacks call into the common
> > > msm_otg_{suspend,resume} routines, however these routines are only being
> > > built when CONFIG_PM_SLEEP.  In addition, msm_otg_{suspend,resume} also
> > > depends on msm_hsusb_config_vddcx(), which is only built when
> > > CONFIG_PM_SLEEP.
> > > 
> > > Fix the CONFIG_PM_RUNTIME, !CONFIG_PM_SLEEP case by changing the
> > > preprocessor conditional, and moving msm_hsusb_config_vddcx().
> > > 
> > > While we're here, eliminate the CONFIG_PM conditional for setting
> > > up the dev_pm_ops.
> > > 
> > > This address the following errors Russell King has hit doing randconfig
> > > builds:
> > > 
> > > drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_suspend':
> > > drivers/usb/phy/phy-msm-usb.c:1691:2: error: implicit declaration of 
> > > function 'msm_otg_suspend'
> > > drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_resume':
> > > drivers/usb/phy/phy-msm-usb.c:1699:2: error: implicit declaration of 
> > > function 'msm_otg_resume'
> > > 
> > > Cc: Ivan T. Ivanov 
> > > Reported-by: Russell King 
> > > Signed-off-by: Josh Cartwright 
> > > ---
> > > v1->v2: Change conditional to simply CONFIG_PM (thanks ccov and khilman!)
> > > 
> > >  drivers/usb/phy/phy-msm-usb.c | 57 
> > > ---
> > >  1 file changed, 26 insertions(+), 31 deletions(-)
> > > 
> > > diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
> > > index 8546c8d..5b169a7 100644
> > > --- a/drivers/usb/phy/phy-msm-usb.c
> > > +++ b/drivers/usb/phy/phy-msm-usb.c
> > [..]
> > > @@ -440,7 +414,32 @@ static int msm_otg_reset(struct usb_phy *phy)
> > >  #define PHY_SUSPEND_TIMEOUT_USEC (500 * 1000)
> > >  #define PHY_RESUME_TIMEOUT_USEC  (100 * 1000)
> > >  
> > > -#ifdef CONFIG_PM_SLEEP
> > > +#if CONFIG_PM
> > 
> > *sigh*.  This, of course, should have been #ifdef CONFIG_PM.  Fixed
> > v3 below.
> 
> sorry, please git send-email it properly.

No problem, will do.  FWIW, it's applicable with git am --scissors.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
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 v2 1/6] usb: otg-fsm: add HNP polling operation function.

[Felipe Balbi]

On Mon, Jan 20, 2014 at 10:00:15AM +0800, Li Jun wrote:
> This patch adds HNP polling operation function for OTG fsm.
> 
> Signed-off-by: Li Jun 
> ---
>  drivers/usb/phy/phy-fsm-usb.c |2 ++
>  include/linux/usb/otg-fsm.h   |9 +
>  2 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-fsm-usb.c b/drivers/usb/phy/phy-fsm-usb.c
> index c47e5a6..ef91961 100644
> --- a/drivers/usb/phy/phy-fsm-usb.c
> +++ b/drivers/usb/phy/phy-fsm-usb.c
> @@ -169,6 +169,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum 
> usb_otg_state new_state)
>   otg_set_protocol(fsm, PROTO_HOST);
>   usb_bus_start_enum(fsm->otg->host,
>   fsm->otg->host->otg_port);
> + otg_start_hnp_polling(fsm);
>   break;
>   case OTG_STATE_A_IDLE:
>   otg_drv_vbus(fsm, 0);
> @@ -203,6 +204,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum 
> usb_otg_state new_state)
>*/
>   if (!fsm->a_bus_req || fsm->a_suspend_req_inf)
>   otg_add_timer(fsm, A_WAIT_ENUM);
> + otg_start_hnp_polling(fsm);
>   break;
>   case OTG_STATE_A_SUSPEND:
>   otg_drv_vbus(fsm, 1);
> diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
> index b6ba1bf..79c6ee8 100644
> --- a/include/linux/usb/otg-fsm.h
> +++ b/include/linux/usb/otg-fsm.h
> @@ -127,6 +127,7 @@ struct otg_fsm_ops {
>   void(*start_pulse)(struct otg_fsm *fsm);
>   void(*start_adp_prb)(struct otg_fsm *fsm);
>   void(*start_adp_sns)(struct otg_fsm *fsm);
> + void(*start_hnp_polling)(struct otg_fsm *fsm);

why ? HNP polling is a generic operation, is it not ? Which means you
shouldn't need to add this function pointer here, just implement a
generic helper function, ideally in usb-common.c

Also, I need to see a patch adding proper kernel doc to those
structures.

cheers

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2] usb: phy: msm: fix compilation errors when !CONFIG_PM_SLEEP

[Felipe Balbi]

On Fri, Jan 17, 2014 at 12:26:50PM -0600, Josh Cartwright wrote:
> On Fri, Jan 17, 2014 at 11:58:51AM -0600, Josh Cartwright wrote:
> > Both the PM_RUNTIME and PM_SLEEP callbacks call into the common
> > msm_otg_{suspend,resume} routines, however these routines are only being
> > built when CONFIG_PM_SLEEP.  In addition, msm_otg_{suspend,resume} also
> > depends on msm_hsusb_config_vddcx(), which is only built when
> > CONFIG_PM_SLEEP.
> > 
> > Fix the CONFIG_PM_RUNTIME, !CONFIG_PM_SLEEP case by changing the
> > preprocessor conditional, and moving msm_hsusb_config_vddcx().
> > 
> > While we're here, eliminate the CONFIG_PM conditional for setting
> > up the dev_pm_ops.
> > 
> > This address the following errors Russell King has hit doing randconfig
> > builds:
> > 
> > drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_suspend':
> > drivers/usb/phy/phy-msm-usb.c:1691:2: error: implicit declaration of 
> > function 'msm_otg_suspend'
> > drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_resume':
> > drivers/usb/phy/phy-msm-usb.c:1699:2: error: implicit declaration of 
> > function 'msm_otg_resume'
> > 
> > Cc: Ivan T. Ivanov 
> > Reported-by: Russell King 
> > Signed-off-by: Josh Cartwright 
> > ---
> > v1->v2: Change conditional to simply CONFIG_PM (thanks ccov and khilman!)
> > 
> >  drivers/usb/phy/phy-msm-usb.c | 57 
> > ---
> >  1 file changed, 26 insertions(+), 31 deletions(-)
> > 
> > diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
> > index 8546c8d..5b169a7 100644
> > --- a/drivers/usb/phy/phy-msm-usb.c
> > +++ b/drivers/usb/phy/phy-msm-usb.c
> [..]
> > @@ -440,7 +414,32 @@ static int msm_otg_reset(struct usb_phy *phy)
> >  #define PHY_SUSPEND_TIMEOUT_USEC   (500 * 1000)
> >  #define PHY_RESUME_TIMEOUT_USEC(100 * 1000)
> >  
> > -#ifdef CONFIG_PM_SLEEP
> > +#if CONFIG_PM
> 
> *sigh*.  This, of course, should have been #ifdef CONFIG_PM.  Fixed
> v3 below.

sorry, please git send-email it properly.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v4 1/2] usb: musb: dsps, debugfs files

[Felipe Balbi]

On Fri, Jan 17, 2014 at 10:22:35AM +0100, Markus Pargmann wrote:
> debugfs files to show the contents of important dsps registers.
> 
> Signed-off-by: Markus Pargmann 
> ---
>  drivers/usb/musb/musb_dsps.c | 54 
> 
>  1 file changed, 54 insertions(+)
> 
> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> index 593d3c9..d0a97d6 100644
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@ -46,6 +46,8 @@
>  #include 
>  #include 
>  
> +#include 
> +
>  #include "musb_core.h"
>  
>  static const struct of_device_id musb_dsps_of_match[];
> @@ -137,6 +139,26 @@ struct dsps_glue {
>   unsigned long last_timer;/* last timer data for each instance */
>  
>   struct dsps_context context;
> + struct debugfs_regset32 regset;
> + struct dentry *dbgfs_root;
> +};
> +
> +static const struct debugfs_reg32 dsps_musb_regs[] = {
> + { "revision",   0x00 },
> + { "control",0x14 },
> + { "status", 0x18 },
> + { "eoi",0x24 },
> + { "intr0_stat", 0x30 },
> + { "intr1_stat", 0x34 },
> + { "intr0_set",  0x38 },
> + { "intr1_set",  0x3c },
> + { "txmode", 0x70 },
> + { "rxmode", 0x74 },
> + { "autoreq",0xd0 },
> + { "srpfixtime", 0xd4 },
> + { "tdown",  0xd8 },
> + { "phy_utmi",   0xe0 },
> + { "mode",   0xe8 },
>  };
>  
>  static void dsps_musb_try_idle(struct musb *musb, unsigned long timeout)
> @@ -369,6 +391,30 @@ out:
>   return ret;
>  }
>  
> +static int dsps_musb_dbg_init(struct musb *musb, struct dsps_glue *glue)
> +{
> + struct dentry *root;
> + struct dentry *file;
> + char buf[128];
> +
> + sprintf(buf, "%s.dsps", dev_name(musb->controller));
> + root = debugfs_create_dir(buf, NULL);
> + if (!root)

wrong, you should be using IS_ERR()

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels

[Felipe Balbi]

On Fri, Jan 17, 2014 at 10:59:25AM +0800, Bo Shen wrote:
> When the SoC is earlier than sama5d3 SoC, which have the same number
> endpoints and DMAs. However for sama5d3 SoC, it has different number
> for endpoints and DMAs. So, define USBA_NR_DMAs for DMA channels
> 
> Signed-off-by: Bo Shen 
> ---
> 
>  drivers/usb/gadget/atmel_usba_udc.c | 2 +-
>  drivers/usb/gadget/atmel_usba_udc.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/atmel_usba_udc.c 
> b/drivers/usb/gadget/atmel_usba_udc.c
> index 7e67a81..5cded1c 100644
> --- a/drivers/usb/gadget/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/atmel_usba_udc.c
> @@ -1661,7 +1661,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
>   if (dma_status) {
>   int i;
>  
> - for (i = 1; i < USBA_NR_ENDPOINTS; i++)
> + for (i = 1; i < USBA_NR_DMAS; i++)
>   if (dma_status & (1 << i))
>   usba_dma_irq(udc, &udc->usba_ep[i]);
>   }
> diff --git a/drivers/usb/gadget/atmel_usba_udc.h 
> b/drivers/usb/gadget/atmel_usba_udc.h
> index 2922db5..a70706e 100644
> --- a/drivers/usb/gadget/atmel_usba_udc.h
> +++ b/drivers/usb/gadget/atmel_usba_udc.h
> @@ -210,7 +210,7 @@
>  #define USBA_FIFO_BASE(x)((x) << 16)
>  
>  /* Synth parameters */
> -#define USBA_NR_ENDPOINTS7
> +#define USBA_NR_DMAS 7

what's the difference ? You just renamed this macro. Also, please
clarify a bit your commit log.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v7 03/12] mfd: omap-usb-host: Use clock names as per function for reference clocks

[Lee Jones]

>  Use a meaningful name for the reference clocks so that it indicates the 
>  function.
> 
>  CC: Lee Jones 
>  CC: Samuel Ortiz 
>  Signed-off-by: Roger Quadros 
>  ---
>   drivers/mfd/omap-usb-host.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
>  diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
>  index 60a3bed..ce620a8 100644
>  --- a/drivers/mfd/omap-usb-host.c
>  +++ b/drivers/mfd/omap-usb-host.c
>  @@ -714,21 +714,21 @@ static int usbhs_omap_probe(struct platform_device 
>  *pdev)
>   goto err_mem;
>   }
>   
>  -omap->xclk60mhsp1_ck = devm_clk_get(dev, "xclk60mhsp1_ck");
>  +omap->xclk60mhsp1_ck = devm_clk_get(dev, "refclk_60m_ext_p1");
> >>>
> >>> You can't do that. These changes will have to be in the same patch as
> >>> the core change i.e. where they are initialised.
> >>
> >> I'm not touching them anywhere in this series. When core changes are you
> >> referring to?
> > 
> > The ones in:
> >   arch/arm/mach-omap2/cclock3xxx_data.c
> 
> OK, right. They are now no longer needed so I'll get rid of them.
> In fact that change should either be in a separate patch or combined with 
> PATCH 2
> in this series. What do you suggest?

A separate patch will do fine.

>   if (IS_ERR(omap->xclk60mhsp1_ck)) {
>   ret = PTR_ERR(omap->xclk60mhsp1_ck);
>   dev_err(dev, "xclk60mhsp1_ck failed error:%d\n", ret);
>   goto err_mem;
>   }
>   
>  -omap->xclk60mhsp2_ck = devm_clk_get(dev, "xclk60mhsp2_ck");
>  +omap->xclk60mhsp2_ck = devm_clk_get(dev, "refclk_60m_ext_p2");
>   if (IS_ERR(omap->xclk60mhsp2_ck)) {
>   ret = PTR_ERR(omap->xclk60mhsp2_ck);
>   dev_err(dev, "xclk60mhsp2_ck failed error:%d\n", ret);
>   goto err_mem;
>   }
>   
>  -omap->init_60m_fclk = devm_clk_get(dev, "init_60m_fclk");
>  +omap->init_60m_fclk = devm_clk_get(dev, "refclk_60m_int");
>   if (IS_ERR(omap->init_60m_fclk)) {
>   ret = PTR_ERR(omap->init_60m_fclk);
>   dev_err(dev, "init_60m_fclk failed error:%d\n", ret);
> >>>
> >>
> > 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | 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: USB 3380 using net2280 driver

[Kevin Cernekee]

On Tue, Feb 18, 2014 at 7:12 AM, Felipe Balbi  wrote:
> On Mon, Feb 17, 2014 at 04:43:11PM -0800, Amit Uttamchandani wrote:
>> I was looking at the changes for net2280.c and saw your name come up in
>> a few of the more recent changes. I wanted to know, are you aware of
>> support for PLXs USB 338o using this net2280 driver?
>
> no, those are two completely different controllers. Linux doesn't
> support USB 3380 as of today.

It might be possible to use this code as a starting point:

http://patchwork.openwrt.org/patch/2889/
--
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/6] usb: gadget: gr_udc: Make struct platform_device variable name clearer

[Felipe Balbi]

On Thu, Jan 09, 2014 at 11:54:13AM +0100, Andreas Larsson wrote:
> Rename struct platform_device pointers from ofdev to pdev for clarity.
> Suggested by Mark Rutland.
> 
> Signed-off-by: Andreas Larsson 
> ---
>  drivers/usb/gadget/gr_udc.c |   18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
> index 914cbd8..e66dcf0 100644
> --- a/drivers/usb/gadget/gr_udc.c
> +++ b/drivers/usb/gadget/gr_udc.c
> @@ -2071,9 +2071,9 @@ static int gr_udc_init(struct gr_udc *dev)
>   return 0;
>  }
>  
> -static int gr_remove(struct platform_device *ofdev)
> +static int gr_remove(struct platform_device *pdev)
>  {
> - struct gr_udc *dev = dev_get_drvdata(&ofdev->dev);
> + struct gr_udc *dev = dev_get_drvdata(&pdev->dev);

you can use platform_get_drvdata()

> @@ -2083,7 +2083,7 @@ static int gr_remove(struct platform_device *ofdev)
>   gr_dfs_delete(dev);
>   if (dev->desc_pool)
>   dma_pool_destroy(dev->desc_pool);
> - dev_set_drvdata(&ofdev->dev, NULL);
> + dev_set_drvdata(&pdev->dev, NULL);

and platform_set_drvdata()

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2] usb: dwc3: fix wrong bit mask in dwc3_event_devt

[Felipe Balbi]

Hi,

On Tue, Jan 07, 2014 at 05:45:50PM +0800, Huang Rui wrote:
> Per dwc3 2.70a spec in the Device-Specific Event (DEVT), the field of
> Event Information Bits(EvtInfo) uses [24:16] bits, and it has 9 bits
> not 8 bits. And the following reserved field uses [31:25] bits not
> [31:24] bits, and it has 7 bits.
> 
> So in dwc3_event_devt, the bit mask should be:
> event_info[24:16] 9 bits
> reserved31_25 [31:25] 7 bits
> 
> This patch should be backported to kernels as old as 3.2, that contain
> the commit 72246da40f3719af3bfd104a2365b32537c27d83 "usb: Introduce
> DesignWare USB3 DRD Driver".

This paragraph shouldn't be in the commit log (I'll fix it, don't
worry), also this doesn't really need to be backported all the way back
since this was changed between 2.00a and 2.30a version of the core,
which didn't even exist back then.

> Cc: 
> Signed-off-by: Huang Rui 
> ---
> 
> Changes from v1 -> v2:
> - Add CC stable mail list.
> 
>  drivers/usb/dwc3/core.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index f8af8d4..69c4583 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -815,15 +815,15 @@ struct dwc3_event_depevt {
>   *   12  - VndrDevTstRcved
>   * @reserved15_12: Reserved, not used
>   * @event_info: Information about this event
> - * @reserved31_24: Reserved, not used
> + * @reserved31_25: Reserved, not used
>   */
>  struct dwc3_event_devt {
>   u32 one_bit:1;
>   u32 device_event:7;
>   u32 type:4;
>   u32 reserved15_12:4;
> - u32 event_info:8;
> - u32 reserved31_24:8;
> + u32 event_info:9;
> + u32 reserved31_25:7;
>  } __packed;
>  
>  /**
> -- 
> 1.8.1.2
> 
> 

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH] USB: gadget: remove unused parameter from udc_stop in usb_gadget_ops

[Felipe Balbi]

On Tue, Dec 17, 2013 at 09:40:35AM +0100, Robert Baldyga wrote:
> This patch removes parameter struct usb_gadget_driver* from udc_stop() 
> function
> in struct usb_gadget_ops. This parameter is useless in udc_stop() function, 
> and
> UDC drivers can work well without it, so removeing it makes code clearer.
> 
> This patch modifies UDC drivers to make them compatible witch changed API.
> It also modifies udc-core.c, where udc_stop() function is used.
> 
> Signed-off-by: Robert Baldyga 
> Signed-off-by: Kyungmin Park 

I really need Tested-bys before I can apply this patch.

-- 
balbi


signature.asc
Description: Digital signature

Re: Remove dependency on BROKEN from drives/usb/musb/da8xx.c glue

[Felipe Balbi]

On Mon, Feb 03, 2014 at 11:57:32AM +0100, Christian Riesch wrote:
> Hi,
> 
> commit 787f5627bec80094db487bfcb401e9744f181aed
> usb: musb: make davinci and da8xx glues depend on BROKEN
> Signed-off-by: Felipe Balbi 
> 
> adds a dependency of the drivers/usb/musb/da8xx.c driver to BROKEN.
> 
> I have successfully tested this driver with kernel 3.13 on a custom
> Texas Instruments AM1808 board in gadget mode, RNDIS network gadget.
> 
> Therefore I would like to see this BROKEN dependency removed. What
> would be required for removing this dependency? Is removing the
> include of the  header sufficient? In the mailing list

yeah. Remove  and make sure it builds cleanly on other
arches.

> discussion on the patch, Sergei Shtylyov mentioned that this would
> mean duplicating the definitions. Would this be ok, just copying them
> to drivers/usb/musb/da8xx.c?

whatever is used by mach and driver should be sitting in a public header
which both include. Say something like  or
something similar.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 2/2] usb: host: xhci-plat: Fix build warning when !CONFIG_PM

[Felipe Balbi]

On Fri, Jan 31, 2014 at 02:29:53AM -0200, Fabio Estevam wrote:
> From: Fabio Estevam 
> 
> Building keystone_defconfig leads to the following build warnings:
> 
> drivers/usb/host/xhci-plat.c:203:12: warning: 'xhci_plat_suspend' defined but 
> not used [-Wunused-function]
> drivers/usb/host/xhci-plat.c:211:12: warning: 'xhci_plat_resume' defined but 
> not used [-Wunused-function]
> 
> Cc: Olof Johansson 
> Reported-by: Olof Johansson 
> Signed-off-by: Fabio Estevam 

Acked-by: Felipe Balbi 

> ---
> Build-tested only
> 
> Changes since v1:
> - none
> 
>  drivers/usb/host/xhci-plat.c | 13 -
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 9c2e583..104e48f 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -199,7 +199,7 @@ static int xhci_plat_remove(struct platform_device *dev)
>   return 0;
>  }
>  
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_PM_SLEEP
>  static int xhci_plat_suspend(struct device *dev)
>  {
>   struct usb_hcd  *hcd = dev_get_drvdata(dev);
> @@ -215,14 +215,9 @@ static int xhci_plat_resume(struct device *dev)
>  
>   return xhci_resume(xhci, 0);
>  }
> +#endif /* CONFIG_PM_SLEEP */
>  
> -static const struct dev_pm_ops xhci_plat_pm_ops = {
> - SET_SYSTEM_SLEEP_PM_OPS(xhci_plat_suspend, xhci_plat_resume)
> -};
> -#define DEV_PM_OPS   (&xhci_plat_pm_ops)
> -#else
> -#define DEV_PM_OPS   NULL
> -#endif /* CONFIG_PM */
> +static SIMPLE_DEV_PM_OPS(xhci_plat_pm_ops, xhci_plat_suspend, 
> xhci_plat_resume);
>  
>  #ifdef CONFIG_OF
>  static const struct of_device_id usb_xhci_of_match[] = {
> @@ -237,7 +232,7 @@ static struct platform_driver usb_xhci_driver = {
>   .remove = xhci_plat_remove,
>   .driver = {
>   .name = "xhci-hcd",
> - .pm = DEV_PM_OPS,
> + .pm = &xhci_plat_pm_ops,
>   .of_match_table = of_match_ptr(usb_xhci_of_match),
>   },
>  };
> -- 
> 1.8.1.2
> 

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH Resend 1/2] usb: gadget: s3c2410_udc: Fix build error

[Felipe Balbi]

On Mon, Feb 03, 2014 at 01:59:38PM +0530, Sachin Kamat wrote:
> Pass value instead of address as expected by 'usb_ep_set_maxpacket_limit'.
> Fixes the following compilation error introduced by commit e117e742d310
> ("usb: gadget: add "maxpacket_limit" field to struct usb_ep"):
> 
> drivers/usb/gadget/s3c2410_udc.c: In function ‘s3c2410_udc_reinit’:
> drivers/usb/gadget/s3c2410_udc.c:1632:3: error:
> cannot take address of bit-field ‘maxpacket’
>usb_ep_set_maxpacket_limit(&ep->ep, &ep->ep.maxpacket);
> 
> Signed-off-by: Sachin Kamat 
> Reviewed-by: Robert Baldyga 

is this still needed for -rc4 ?

> ---
>  drivers/usb/gadget/s3c2410_udc.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/s3c2410_udc.c 
> b/drivers/usb/gadget/s3c2410_udc.c
> index f04b2c3154de..dd9678f85c58 100644
> --- a/drivers/usb/gadget/s3c2410_udc.c
> +++ b/drivers/usb/gadget/s3c2410_udc.c
> @@ -1629,7 +1629,7 @@ static void s3c2410_udc_reinit(struct s3c2410_udc *dev)
>   ep->ep.desc = NULL;
>   ep->halted = 0;
>   INIT_LIST_HEAD(&ep->queue);
> - usb_ep_set_maxpacket_limit(&ep->ep, &ep->ep.maxpacket);
> + usb_ep_set_maxpacket_limit(&ep->ep, ep->ep.maxpacket);
>   }
>  }
>  
> -- 
> 1.7.9.5
> 

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH Resend 2/2] usb: gadget: s3c-hsudc: Remove unused label

[Felipe Balbi]

On Mon, Feb 03, 2014 at 01:59:39PM +0530, Sachin Kamat wrote:
> Fixes the following compilation warning:
> drivers/usb/gadget/s3c-hsudc.c: In function ‘s3c_hsudc_probe’:
> drivers/usb/gadget/s3c-hsudc.c:1347:1: warning: label ‘err_add_device’
> defined but not used [-Wunused-label]
> 
> Signed-off-by: Sachin Kamat 

What about this one ?

> ---
>  drivers/usb/gadget/s3c-hsudc.c |1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/s3c-hsudc.c b/drivers/usb/gadget/s3c-hsudc.c
> index ea4bbfe72ec0..10c6a128250c 100644
> --- a/drivers/usb/gadget/s3c-hsudc.c
> +++ b/drivers/usb/gadget/s3c-hsudc.c
> @@ -1344,7 +1344,6 @@ static int s3c_hsudc_probe(struct platform_device *pdev)
>  
>   return 0;
>  err_add_udc:
> -err_add_device:
>   clk_disable(hsudc->uclk);
>  err_res:
>   if (!IS_ERR_OR_NULL(hsudc->transceiver))
> -- 
> 1.7.9.5
> 

-- 
balbi


signature.asc
Description: Digital signature