Re: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Susmita/Rajib Bandopadhyay
On 04/05/2016, Alan Stern  wrote:
> There is a program called monosim that might do
> what you want.

Okay, Sir. I will look at it. It has a GUI built with GTK and QT.
Regards
--
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] drivers: usb: dwc3 : Configure DMA properties and ops from DT

2016-05-03 Thread Rajesh Bhagat
On certain platforms (e.g. ARM64) the dma_ops needs to be explicitly set
to be able to do DMA allocations, so use the of_dma_configure() helper
to populate the dma properties and assign an appropriate dma_ops.

Signed-off-by: Rajesh Bhagat 
Reviewed-by: Yang-Leo Li 
---
 drivers/usb/dwc3/host.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index c679f63..4d5b783 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -17,6 +17,7 @@
 
 #include 
 #include 
+#include 
 
 #include "core.h"
 
@@ -32,6 +33,9 @@ int dwc3_host_init(struct dwc3 *dwc)
return -ENOMEM;
}
 
+   if (IS_ENABLED(CONFIG_OF) && dwc->dev->of_node)
+   of_dma_configure(>dev, dwc->dev->of_node);
+
dma_set_coherent_mask(>dev, dwc->dev->coherent_dma_mask);
 
xhci->dev.parent= dwc->dev;
-- 
2.6.2.198.g614a2ac

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


Re: [PATCH v6 09/12] usb: gadget: udc: adapt to OTG core

2016-05-03 Thread Peter Chen
On Tue, May 03, 2016 at 06:44:46PM +0300, Roger Quadros wrote:
> Hi,
> 
> On 03/05/16 10:06, Jun Li wrote:
> > Hi
> > 
> >  /**
> > + * usb_gadget_start - start the usb gadget controller and
> > +connect to bus
> > + * @gadget: the gadget device to start
> > + *
> > + * This is external API for use by OTG core.
> > + *
> > + * Start the usb device controller and connect to bus (enable
> >> pull).
> > + */
> > +static int usb_gadget_start(struct usb_gadget *gadget) {
> > +   int ret;
> > +   struct usb_udc *udc = NULL;
> > +
> > +   dev_dbg(>dev, "%s\n", __func__);
> > +   mutex_lock(_lock);
> > +   list_for_each_entry(udc, _list, list)
> > +   if (udc->gadget == gadget)
> > +   goto found;
> > +
> > +   dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
> > +   __func__);
> > +   mutex_unlock(_lock);
> > +   return -EINVAL;
> > +
> > +found:
> > +   ret = usb_gadget_udc_start(udc);
> > +   if (ret)
> > +   dev_err(>dev, "USB Device Controller didn't
>  start: %d\n",
> > +   ret);
> > +   else
> > +   usb_udc_connect_control(udc);
> 
>  For drd, it's fine, but for real otg, gadget connect should be
>  done by
>  loc_conn() instead of gadget start.
> >>>
> >>> It is upto the OTG state machine to call gadget_start() when it
> >>> needs to connect to the bus (i.e. loc_conn()). I see no point in
> >>> calling gadget start before.
> >>>
> >>> Do you see any issue in doing so?
> >>
> >> This is what OTG state machine does:
> >> case OTG_STATE_B_PERIPHERAL:
> >>  otg_chrg_vbus(otg, 0);
> >>  otg_loc_sof(otg, 0);
> >>  otg_set_protocol(fsm, PROTO_GADGET);
> >>  otg_loc_conn(otg, 1);
> >>  break;
> 
>  On second thoughts, after seen the OTG state machine.
>  otg_set_protocol(fsm, PROTO_GADGET); is always followed by
>  otg_loc_conn(otg, 1); And whenever protocol changes to anything other
>  the PROTO_GADGET, we use otg_loc_conn(otg, 0);
> 
>  So otg_loc_conn seems redundant. Can we just get rid of it?
> 
>  usb_gadget_start() implies that gadget controller starts up and
>  enables pull.
>  usb_gadget_stop() implies that gadget controller disables pull and
> >> stops.
> 
> 
>  Can you please explain why just these 2 APIs are not sufficient for
>  full OTG?
> 
>  Do we want anything to happen between gadget controller start/stop
>  and pull on/off?
> >>>
> >>> "loc_conn" is a standard output parameter in OTG spec, it deserves a
> >>> separate api, yes, current implementation of OTG state machine code
> >>> seems allow you to combine the 2 things into one, but don't do that,
> >>> because they do not always happen together, e.g. for peripheral only B
> >>> device (also a part OTG spec: section 7.3), will be fixed in gadget
> >>> mode, but it will do gadget connect and disconnect in its diff states,
> >>> so, to make the framework common, let's keep them separated.
> >>
> >> I'm sorry but I didn't understand your comment about "it will do gadget
> >> connect and disconnect in its diff states"
> > 
> > Gadget connect means loc_conn(1).
> > 
> >>
> >> I am reading the OTG v2.0 specification and loc_conn is always true when
> >> b_peripheral or a_peripheral is true and false otherwise.
> > 
> > If you only talk about these 2 states, yes, loc_conn is ture.
> > 
> >>
> >> loc_conn is just an internal state variable and it corresponds to our
> >> gadget_start/stop() state.
> > 
> > It's not an internal variable, there are OTG state machine
> > parameters tables(table 7-x) in OTG spec which have clear lists
> > which are "internal variable", which are "input", which are "output"...
> > 
> > Those APIs are driven directly from OTG spec, easily understood so
> > code reader can know what's those APIs for. For real OTG, I don't
> > see the benefit if get rid of it.
> 
> OK, no issues if we don't get rid of it. But I am still in favor of
> doing a connect in usb_gadget_start(), because
> 
> 1) If we split connect/disconnect() and usb_gadget_start/stop() then there is
> additional overhead of keeping track whether connect was called or not during
> usb_gadget_stop(). Plus we need to take care that users don't call 
> connect/disconnect
> outside of start/stop. It is just complicating things.
> 
> 2) for many controllers there is no difference between run/stop and
> connect/disconnect. i.e. a single register bit controls both.
> 
> 3) it fits well with the OTG specification. OTG specification says
> that loc_conn *variable* must be true *after* the device has signalled a 
> connect.
> So OTG state machine can safely set loc_conn 

Re: [RESEND PATCH v10 4/4] power: wm831x_power: Support USB charger current limit management

2016-05-03 Thread Manish Badarkhe
Hi Mark

>> > +static const unsigned int wm831x_usb_limits[] = {
>> > +   0,
>> > +   2,
>> > +   100,
>> > +   500,
>> > +   900,
>> > +   1500,
>> > +   1800,
>> > +   550,
>> > +};
>
>> Just for curiosity, How these current limits are getting decided?
>> Can we have some proper defines over here so that it can be grasped easily?
>
> They're in the silicon, it's just a table of values that were put into
> the silicon at design time.  The defines would just be TABLE_ENTRY_1 or
> whatever.

Thanks for the clarification, In that case, comments/documentation
will work instead of making any defines.

Regards
Manish Badarkhe
--
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: hub: fix panic caused by NULL bos pointer during reset device

2016-05-03 Thread Du, Changbin
> > I think Greg is referring to commit 464ad8c43a9e ("usb: core : hub: Fix
> > BOS 'NULL pointer' kernel panic"), which has already been applied
> > upstream.  It looks to me like that patch might have fixed the same
> > problem in a different way, in which case Changbin's patch is not
> > needed.  But I haven't been involved in developing or testing that
> > patch, so I can't say for sure.  At the very least, 464ad8c43a9e
> > conflicts with Changbin's patch.
> >
> > Changbin, can you take a look at 464ad8c43a9e and see if that fixes the
> > same problem that your patch did?
> 
> Given the lack of response here, I've dropped this from my queue.  If
> it's still needed, someone must resend it.
> 
> thanks,
> 
> greg k-h

Hi,
I missed this email because it was junked by my email client. Just checked
patch 464ad8c43a9e, it fix the same issue. So my patch no longer need now.
Thanks for the patch.

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


Re: [PATCH v6 09/12] usb: gadget: udc: adapt to OTG core

2016-05-03 Thread Peter Chen
On Tue, May 03, 2016 at 06:44:46PM +0300, Roger Quadros wrote:
> Hi,
> 
> On 03/05/16 10:06, Jun Li wrote:
> > Hi
> > 
> >  /**
> > + * usb_gadget_start - start the usb gadget controller and
> > +connect to bus
> > + * @gadget: the gadget device to start
> > + *
> > + * This is external API for use by OTG core.
> > + *
> > + * Start the usb device controller and connect to bus (enable
> >> pull).
> > + */
> > +static int usb_gadget_start(struct usb_gadget *gadget) {
> > +   int ret;
> > +   struct usb_udc *udc = NULL;
> > +
> > +   dev_dbg(>dev, "%s\n", __func__);
> > +   mutex_lock(_lock);
> > +   list_for_each_entry(udc, _list, list)
> > +   if (udc->gadget == gadget)
> > +   goto found;
> > +
> > +   dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
> > +   __func__);
> > +   mutex_unlock(_lock);
> > +   return -EINVAL;
> > +
> > +found:
> > +   ret = usb_gadget_udc_start(udc);
> > +   if (ret)
> > +   dev_err(>dev, "USB Device Controller didn't
>  start: %d\n",
> > +   ret);
> > +   else
> > +   usb_udc_connect_control(udc);
> 
>  For drd, it's fine, but for real otg, gadget connect should be
>  done by
>  loc_conn() instead of gadget start.
> >>>
> >>> It is upto the OTG state machine to call gadget_start() when it
> >>> needs to connect to the bus (i.e. loc_conn()). I see no point in
> >>> calling gadget start before.
> >>>
> >>> Do you see any issue in doing so?
> >>
> >> This is what OTG state machine does:
> >> case OTG_STATE_B_PERIPHERAL:
> >>  otg_chrg_vbus(otg, 0);
> >>  otg_loc_sof(otg, 0);
> >>  otg_set_protocol(fsm, PROTO_GADGET);
> >>  otg_loc_conn(otg, 1);
> >>  break;
> 
>  On second thoughts, after seen the OTG state machine.
>  otg_set_protocol(fsm, PROTO_GADGET); is always followed by
>  otg_loc_conn(otg, 1); And whenever protocol changes to anything other
>  the PROTO_GADGET, we use otg_loc_conn(otg, 0);
> 
>  So otg_loc_conn seems redundant. Can we just get rid of it?
> 
>  usb_gadget_start() implies that gadget controller starts up and
>  enables pull.
>  usb_gadget_stop() implies that gadget controller disables pull and
> >> stops.
> 
> 
>  Can you please explain why just these 2 APIs are not sufficient for
>  full OTG?
> 
>  Do we want anything to happen between gadget controller start/stop
>  and pull on/off?
> >>>
> >>> "loc_conn" is a standard output parameter in OTG spec, it deserves a
> >>> separate api, yes, current implementation of OTG state machine code
> >>> seems allow you to combine the 2 things into one, but don't do that,
> >>> because they do not always happen together, e.g. for peripheral only B
> >>> device (also a part OTG spec: section 7.3), will be fixed in gadget
> >>> mode, but it will do gadget connect and disconnect in its diff states,
> >>> so, to make the framework common, let's keep them separated.
> >>
> >> I'm sorry but I didn't understand your comment about "it will do gadget
> >> connect and disconnect in its diff states"
> > 
> > Gadget connect means loc_conn(1).
> > 
> >>
> >> I am reading the OTG v2.0 specification and loc_conn is always true when
> >> b_peripheral or a_peripheral is true and false otherwise.
> > 
> > If you only talk about these 2 states, yes, loc_conn is ture.
> > 
> >>
> >> loc_conn is just an internal state variable and it corresponds to our
> >> gadget_start/stop() state.
> > 
> > It's not an internal variable, there are OTG state machine
> > parameters tables(table 7-x) in OTG spec which have clear lists
> > which are "internal variable", which are "input", which are "output"...
> > 
> > Those APIs are driven directly from OTG spec, easily understood so
> > code reader can know what's those APIs for. For real OTG, I don't
> > see the benefit if get rid of it.
> 
> OK, no issues if we don't get rid of it. But I am still in favor of
> doing a connect in usb_gadget_start(), because
> 
> 1) If we split connect/disconnect() and usb_gadget_start/stop() then there is
> additional overhead of keeping track whether connect was called or not during
> usb_gadget_stop(). Plus we need to take care that users don't call 
> connect/disconnect
> outside of start/stop. It is just complicating things.
> 
> 2) for many controllers there is no difference between run/stop and
> connect/disconnect. i.e. a single register bit controls both.
> 
> 3) it fits well with the OTG specification. OTG specification says
> that loc_conn *variable* must be true *after* the device has signalled a 
> connect.
> So OTG state machine can safely set loc_conn 

Re: [PATCH] usb: xhci-mtk: fixup mouse wakeup failure during system suspend

2016-05-03 Thread chunfeng yun
On Tue, 2016-05-03 at 12:33 +0300, Felipe Balbi wrote:
> Hi,
> 
> chunfeng yun  writes:
> >> chunfeng yun  writes:
> >> > On Thu, 2016-04-21 at 10:04 +0800, Chunfeng Yun wrote:
> >> >> Click mouse after xhci suspend completion but before system suspend
> >> >> completion, system will not be waken up by mouse if the duration of
> >> >> them is larger than 20ms which is the device UFP's resume signaling
> >> 
> >> what is "them" here ? The duration of what is longer than 20ms ?
> > They are "xhci suspend completion" and "system suspend completion";
> >
> > It's time duration
> 
> okay. So if xhci suspend takes longer than 20ms your SPM doesn't see a
> wakeup ?
It is not the time of xhci suspend consumed, but is the time of duration
from xhci suspend completion to system suspend completion(after BOOT-CPU
is turned off, SPM will be enabled to receive wakeup event)

> 
> >> >> lasted. Another reason is that the SPM is not enabled before system
> >> 
> >> what's SPM ?
> > It is System Power Management which is powered off when system is
> > running in normal mode, and is powered on when system enters suspend
> > mode. It is used to wakeup system when some wakeup sources, such as
> > bluetooth or powerkey etc, tigger wakeup event.
> 
> okay, thanks
> 
> >> >> suspend compeltion, this causes SPM also not notice the resume signal.
> >>^^
> >>completion
> >> 
> >> >> So in order to reduce the duration less than 20ms, make use of
> >> >> syscore's suspend/resume interface.
> >> 
> >> no, this is the wrong approach
> > But it seems only one workable approach from software side
> 
> I wouldn't say that. It seems to me SPM should be enabled earlier.
Yes, but normally SPM should be enabled after all CPUS are turned off,
so it's difficult to do that, I mean enable SPM before turning off CPUS
> 
> >> >> Because the syscore runs on irq disabled context, and xhci's
> >> >> suspend/resume calls some sleeping functions, enable local irq
> >> >> and then disable it during suspend/resume. This may be not a problem,
> >> >> since only boot CPU is runing.
> >> 
> >> another problem :) calling local_irq_{enable,disable}() is an indication
> >> that something's wrong.
> > Oh!
> >
> > BTW: There will be warning logs if they are not called.
> 
> yeah, I got that :-) But it's still wrong to use
> local_irq_{enable,disable}() the way you're using them :-)
Yes

Thank you very much.
> 


--
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 v4 1/3] USB: serial: cp210x: Fixed RTS mode setting by the CRTSCTS flag.

2016-05-03 Thread Konstantin Shkolnyy
A bug in the CRTSCTS handling caused RTS to alternate between
CRTSCTS=0 => "RTS transmits active signal" and
CRTSCTS=1 => "RTS receives flow control"
instead of
CRTSCTS=0 => "RTS is statically active" and
CRTSCTS=1 => "RTS receives flow control"
This only happened after first having enabled CRTSCTS.

Signed-off-by: Konstantin Shkolnyy 
---
v4:
Same series of patches, fixed names and defines by feedback.
v3:
Regenerated the patches correctly against the latest usb-next branch.
v2
Improved CRTSCTS fix by feedback. Dropped get_termios error handling fix.

 drivers/usb/serial/cp210x.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index dd47823..fef7a51 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -967,8 +967,7 @@ static void cp210x_set_termios(struct tty_struct *tty,
} else {
modem_ctl[0] &= ~0x7B;
modem_ctl[0] |= 0x01;
-   /* FIXME - OR here instead of assignment looks wrong */
-   modem_ctl[4] |= 0x40;
+   modem_ctl[4] = 0x40;
dev_dbg(dev, "%s - flow control = NONE\n", __func__);
}
 
-- 
1.8.4.5

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


[PATCH v4 3/3] USB: serial: cp210x: Cleaned up CRTSCTS flag code.

2016-05-03 Thread Konstantin Shkolnyy
The CRTSCTS flag code cleared (and inconsistently) bits unrelated to
CRTSCTS functionality. It was also harder than necessary to read.

Signed-off-by: Konstantin Shkolnyy 
---
v4:
Same series of patches, fixed names and defines by feedback.
v3:
Regenerated the patches correctly against the latest usb-next branch.
v2
Improved CRTSCTS fix by feedback. Dropped get_termios error handling fix.

 drivers/usb/serial/cp210x.c | 32 ++--
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index 9857d0c..2d43716 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -995,34 +995,22 @@ static void cp210x_set_termios(struct tty_struct *tty,
dev_dbg(dev, "%s - read ulControlHandshake=0x%08x, 
ulFlowReplace=0x%08x\n",
__func__, ctl_hs, flow_repl);
 
+   ctl_hs &= ~CP210X_SERIAL_DSR_HANDSHAKE;
+   ctl_hs &= ~CP210X_SERIAL_DCD_HANDSHAKE;
+   ctl_hs &= ~CP210X_SERIAL_DSR_SENSITIVITY;
+   ctl_hs &= ~CP210X_SERIAL_DTR_MASK;
+   ctl_hs |= CP210X_SERIAL_DTR_SHIFT(CP210X_SERIAL_DTR_ACTIVE);
if (cflag & CRTSCTS) {
-   ctl_hs &= ~(CP210X_SERIAL_DTR_MASK |
-   CP210X_SERIAL_CTS_HANDSHAKE |
-   CP210X_SERIAL_DSR_HANDSHAKE |
-   CP210X_SERIAL_DCD_HANDSHAKE |
-   CP210X_SERIAL_DSR_SENSITIVITY);
-   ctl_hs |= CP210X_SERIAL_DTR_SHIFT(
-   CP210X_SERIAL_DTR_ACTIVE);
ctl_hs |= CP210X_SERIAL_CTS_HANDSHAKE;
-   /*
-* FIXME: Why clear bits unrelated to flow control.
-* Why clear CP210X_SERIAL_XOFF_CONTINUE which is
-* never set
-*/
-   flow_repl = 0;
+
+   flow_repl &= ~CP210X_SERIAL_RTS_MASK;
flow_repl |= CP210X_SERIAL_RTS_SHIFT(
CP210X_SERIAL_RTS_FLOW_CTL);
dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__);
} else {
-   ctl_hs &= ~(CP210X_SERIAL_DTR_MASK |
-   CP210X_SERIAL_CTS_HANDSHAKE |
-   CP210X_SERIAL_DSR_HANDSHAKE |
-   CP210X_SERIAL_DCD_HANDSHAKE |
-   CP210X_SERIAL_DSR_SENSITIVITY);
-   ctl_hs |= CP210X_SERIAL_DTR_SHIFT(
-   CP210X_SERIAL_DTR_ACTIVE;
-   /* FIXME: Why clear bits unrelated to flow control */
-   ((u8)flow_repl) = 0;
+   ctl_hs &= ~CP210X_SERIAL_CTS_HANDSHAKE;
+
+   flow_repl &= ~CP210X_SERIAL_RTS_MASK;
flow_repl |= CP210X_SERIAL_RTS_SHIFT(
CP210X_SERIAL_RTS_ACTIVE);
dev_dbg(dev, "%s - flow control = NONE\n", __func__);
-- 
1.8.4.5

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


[PATCH v4 2/3] USB: serial: cp210x: Got rid of magic numbers in CRTSCTS flag code.

2016-05-03 Thread Konstantin Shkolnyy
Replaced magic numbers used in the CRTSCTS flag code with symbolic names
from the chip specification.

Signed-off-by: Konstantin Shkolnyy 
---
v4:
Same series of patches, fixed names and defines by feedback.
v3:
Regenerated the patches correctly against the latest usb-next branch.
v2
Improved CRTSCTS fix by feedback. Dropped get_termios error handling fix.

 drivers/usb/serial/cp210x.c | 109 ++--
 1 file changed, 84 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index fef7a51..9857d0c 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -327,6 +327,42 @@ struct cp210x_comm_status {
  */
 #define PURGE_ALL  0x000f
 
+/* CP210X_GET_FLOW/CP210X_SET_FLOW read/write these 0x10 bytes */
+struct cp210x_flow_ctl {
+   __le32  ulControlHandshake;
+   __le32  ulFlowReplace;
+   __le32  ulXonLimit;
+   __le32  ulXoffLimit;
+} __packed;
+
+/* cp210x_flow_ctl::ulControlHandshake */
+#define CP210X_SERIAL_DTR_MASK GENMASK(1, 0)
+#define CP210X_SERIAL_DTR_SHIFT(_mode) (_mode)
+#define CP210X_SERIAL_CTS_HANDSHAKEBIT(3)
+#define CP210X_SERIAL_DSR_HANDSHAKEBIT(4)
+#define CP210X_SERIAL_DCD_HANDSHAKEBIT(5)
+#define CP210X_SERIAL_DSR_SENSITIVITY  BIT(6)
+
+/* values for cp210x_flow_ctl::ulControlHandshake::CP210X_SERIAL_DTR_MASK */
+#define CP210X_SERIAL_DTR_INACTIVE 0
+#define CP210X_SERIAL_DTR_ACTIVE   1
+#define CP210X_SERIAL_DTR_FLOW_CTL 2
+
+/* cp210x_flow_ctl::ulFlowReplace */
+#define CP210X_SERIAL_AUTO_TRANSMITBIT(0)
+#define CP210X_SERIAL_AUTO_RECEIVE BIT(1)
+#define CP210X_SERIAL_ERROR_CHAR   BIT(2)
+#define CP210X_SERIAL_NULL_STRIPPING   BIT(3)
+#define CP210X_SERIAL_BREAK_CHAR   BIT(4)
+#define CP210X_SERIAL_RTS_MASK GENMASK(7, 6)
+#define CP210X_SERIAL_RTS_SHIFT(_mode) (_mode << 6)
+#define CP210X_SERIAL_XOFF_CONTINUEBIT(31)
+
+/* values for cp210x_flow_ctl::ulFlowReplace::CP210X_SERIAL_RTS_MASK */
+#define CP210X_SERIAL_RTS_INACTIVE 0
+#define CP210X_SERIAL_RTS_ACTIVE   1
+#define CP210X_SERIAL_RTS_FLOW_CTL 2
+
 /*
  * Reads a variable-sized block of CP210X_ registers, identified by req.
  * Returns data into buf in native USB byte order.
@@ -694,9 +730,10 @@ static void cp210x_get_termios_port(struct usb_serial_port 
*port,
 {
struct device *dev = >dev;
unsigned int cflag;
-   u8 modem_ctl[16];
+   struct cp210x_flow_ctl flow_ctl;
u32 baud;
u16 bits;
+   u32 ctl_hs;
 
cp210x_read_u32_reg(port, CP210X_GET_BAUDRATE, );
 
@@ -792,9 +829,10 @@ static void cp210x_get_termios_port(struct usb_serial_port 
*port,
break;
}
 
-   cp210x_read_reg_block(port, CP210X_GET_FLOW, modem_ctl,
-   sizeof(modem_ctl));
-   if (modem_ctl[0] & 0x08) {
+   cp210x_read_reg_block(port, CP210X_GET_FLOW, _ctl,
+   sizeof(flow_ctl));
+   ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake);
+   if (ctl_hs & CP210X_SERIAL_CTS_HANDSHAKE) {
dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__);
cflag |= CRTSCTS;
} else {
@@ -863,7 +901,6 @@ static void cp210x_set_termios(struct tty_struct *tty,
struct device *dev = >dev;
unsigned int cflag, old_cflag;
u16 bits;
-   u8 modem_ctl[16];
 
cflag = tty->termios.c_cflag;
old_cflag = old_termios->c_cflag;
@@ -947,34 +984,56 @@ static void cp210x_set_termios(struct tty_struct *tty,
}
 
if ((cflag & CRTSCTS) != (old_cflag & CRTSCTS)) {
+   struct cp210x_flow_ctl flow_ctl;
+   u32 ctl_hs;
+   u32 flow_repl;
 
-   /* Only bytes 0, 4 and 7 out of first 8 have functional bits */
-
-   cp210x_read_reg_block(port, CP210X_GET_FLOW, modem_ctl,
-   sizeof(modem_ctl));
-   dev_dbg(dev, "%s - read modem controls = %02x .. .. .. %02x .. 
.. %02x\n",
-   __func__, modem_ctl[0], modem_ctl[4], modem_ctl[7]);
+   cp210x_read_reg_block(port, CP210X_GET_FLOW, _ctl,
+   sizeof(flow_ctl));
+   ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake);
+   flow_repl  = le32_to_cpu(flow_ctl.ulFlowReplace);
+   dev_dbg(dev, "%s - read ulControlHandshake=0x%08x, 
ulFlowReplace=0x%08x\n",
+   __func__, ctl_hs, flow_repl);
 
if (cflag & CRTSCTS) {
-   modem_ctl[0] &= ~0x7B;
-   modem_ctl[0] |= 0x09;
-   modem_ctl[4] = 0x80;
-   /* FIXME - why clear reserved bits just read? */
-   modem_ctl[5] = 0;
-   modem_ctl[6] = 0;
-   modem_ctl[7] = 0;
+   ctl_hs &= 

Re: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions

2016-05-03 Thread David B. Robins

On 2016-05-03 17:16, Dean Jenkins wrote:

On 03/05/16 15:42, David B. Robins wrote:


I don't think the first one is giving you problems (except as 
triggered by the second) but I had concerns about the second myself 
(and emailed the author off-list, but received no reply), and we did 
not take that commit for our own product.



Sorry, I might have missed your original E-mail.

Specifically, the second change, 3f30... (original patch: 
https://www.mail-archive.com/netdev@vger.kernel.org/msg80720.html) (1) 
appears to do the exact opposite of what it claims, i.e., instead of 
"resync if this looks like a header", it does "resync if this does NOT 
look like a (packet) header", where "looks like a header" means "bits 
0-10 (size) are equal to the bitwise-NOT of bits 16-26", and (2) can 
happen by coincidence for 1/2048 32-bit values starting a continuation 
URB (easy to hit dealing with large volumes of video data as we were). 
It appears to expect the header for every URB whereas the rest of the 
code at least expects it only once per network packet (look at 
following code that only reads it for remaining == 0).


David, I think that your interpretation is incorrect. Please see below.

Here is the code snippet from the patch with my annotations between #
#, I will try to explain my intentions. Feel free to point out any
flaws:

if (rx->remaining && (rx->remaining + sizeof(u32) <= skb->len)) {
# Only runs when rx->remaining !=0 and the end of the Ethernet
frame + next 32-bit header word is within the URB buffer. #
# Therefore, this code does not run when the end of an
Ethernet frame has been reached in the previous URB #
# or when the end of the Ethernet frame + next 32-bit header
word will be in a later URB buffer #


It may well be. I don't have the setup with me now, but I can try 
tomorrow to reproduce an environment where I can add some more detailed 
logging.


Since the URB length has to be >= than the remaining data plus a u32, 
the devices that John Stultz and I are using (AX88772B in my case) may 
be adding some additional data/padding after an Ethernet frame, 
expecting it to be discarded, and running into this check and its 
consequences. This may mean the device is badly behaved, if it is 
specified not to send anything extra; in any case, a well-intentioned 
error correction has gone badly, but I better understand the intent now. 
I am curious to know how often the device you are using benefits from 
this block of code.



Regards,
Dean


David
--
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 v7 1/7] regulator: fixed: add support for ACPI interface

2016-05-03 Thread Lu Baolu
Hi,

On 05/03/2016 07:49 PM, Mark Brown wrote:
> On Tue, May 03, 2016 at 09:43:58AM +0800, Lu Baolu wrote:
>> On 05/02/2016 07:00 PM, Mark Brown wrote:
>>> On Fri, Apr 29, 2016 at 02:26:32PM +0800, Lu Baolu wrote:
 +  gpiod = gpiod_get(dev, "vbus_en", GPIOD_ASIS);
 +  if (IS_ERR(gpiod))
 +  return PTR_ERR(gpiod);
>>> This is clearly an inappropriate name for the signal in generic code,
>>> it's specific to your use case.
>> I will change the gpio name to "gpio". Is that okay?
> Yes, that looks good (and lines up with DT so hopefully the code can be
> shared).

Fair enough. Thanks.

Best regards,
Lu Baolu
--
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/2] musb_host: make musb_tx_dma_set_mode_*() *void*

2016-05-03 Thread Sergei Shtylyov
Now that the DMA engine check was moved to musb_tx_dma_porgram(), both
musb_tx_dma_set_mode_cppi_tusb() and musb_tx_dma_set_mode_mentor() always
return 0, so we can  make both these functions *void*.

Signed-off-by: Sergei Shtylyov 

---
 drivers/usb/musb/musb_host.c |   31 ---
 1 file changed, 12 insertions(+), 19 deletions(-)

Index: usb/drivers/usb/musb/musb_host.c
===
--- usb.orig/drivers/usb/musb/musb_host.c
+++ usb/drivers/usb/musb/musb_host.c
@@ -627,7 +627,7 @@ musb_rx_reinit(struct musb *musb, struct
ep->rx_reinit = 0;
 }
 
-static int musb_tx_dma_set_mode_mentor(struct dma_controller *dma,
+static void musb_tx_dma_set_mode_mentor(struct dma_controller *dma,
struct musb_hw_ep *hw_ep, struct musb_qh *qh,
struct urb *urb, u32 offset,
u32 *length, u8 *mode)
@@ -664,17 +664,15 @@ static int musb_tx_dma_set_mode_mentor(s
}
channel->desired_mode = *mode;
musb_writew(epio, MUSB_TXCSR, csr);
-
-   return 0;
 }
 
-static int musb_tx_dma_set_mode_cppi_tusb(struct dma_controller *dma,
- struct musb_hw_ep *hw_ep,
- struct musb_qh *qh,
- struct urb *urb,
- u32 offset,
- u32 *length,
- u8 *mode)
+static void musb_tx_dma_set_mode_cppi_tusb(struct dma_controller *dma,
+  struct musb_hw_ep *hw_ep,
+  struct musb_qh *qh,
+  struct urb *urb,
+  u32 offset,
+  u32 *length,
+  u8 *mode)
 {
struct dma_channel *channel = hw_ep->tx_channel;
 
@@ -685,8 +683,6 @@ static int musb_tx_dma_set_mode_cppi_tus
 * to identify the zero-length-final-packet case.
 */
*mode = (urb->transfer_flags & URB_ZERO_PACKET) ? 1 : 0;
-
-   return 0;
 }
 
 static bool musb_tx_dma_program(struct dma_controller *dma,
@@ -696,18 +692,15 @@ static bool musb_tx_dma_program(struct d
struct dma_channel  *channel = hw_ep->tx_channel;
u16 pkt_size = qh->maxpacket;
u8  mode;
-   int res;
 
if (musb_dma_inventra(hw_ep->musb) || musb_dma_ux500(hw_ep->musb))
-   res = musb_tx_dma_set_mode_mentor(dma, hw_ep, qh, urb,
-offset, , );
+   musb_tx_dma_set_mode_mentor(dma, hw_ep, qh, urb, offset,
+   , );
else if (is_cppi_enabled(hw_ep->musb) || tusb_dma_omap(hw_ep->musb))
-   res = musb_tx_dma_set_mode_cppi_tusb(dma, hw_ep, qh, urb,
-offset, , );
+   musb_tx_dma_set_mode_cppi_tusb(dma, hw_ep, qh, urb, offset,
+  , );
else
return false;
-   if (res)
-   return false;
 
qh->segsize = length;
 

--
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/2] musb_host: move DMA engine check from musb_tx_dma_set_mode_cppi_tusb() to its caller

2016-05-03 Thread Sergei Shtylyov
Commit 754fe4a92c07 ("usb: musb: Remove ifdefs for TX DMA for musb_host.c")
looks incomplete: the DMA engine checks are  done outside the Mentor/UX500
handler  but inside the CPPI/TUSB handler. Move the checks out of the CPPI/
TUSB handler into its caller, musb_tx_dma_program().

Signed-off-by: Sergei Shtylyov 

---
 drivers/usb/musb/musb_host.c |7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

Index: usb/drivers/usb/musb/musb_host.c
===
--- usb.orig/drivers/usb/musb/musb_host.c
+++ usb/drivers/usb/musb/musb_host.c
@@ -678,9 +678,6 @@ static int musb_tx_dma_set_mode_cppi_tus
 {
struct dma_channel *channel = hw_ep->tx_channel;
 
-   if (!is_cppi_enabled(hw_ep->musb) && !tusb_dma_omap(hw_ep->musb))
-   return -ENODEV;
-
channel->actual_len = 0;
 
/*
@@ -704,9 +701,11 @@ static bool musb_tx_dma_program(struct d
if (musb_dma_inventra(hw_ep->musb) || musb_dma_ux500(hw_ep->musb))
res = musb_tx_dma_set_mode_mentor(dma, hw_ep, qh, urb,
 offset, , );
-   else
+   else if (is_cppi_enabled(hw_ep->musb) || tusb_dma_omap(hw_ep->musb))
res = musb_tx_dma_set_mode_cppi_tusb(dma, hw_ep, qh, urb,
 offset, , );
+   else
+   return false;
if (res)
return false;
 

--
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/2] musb_host: TX DMA cleanup after Tony's patches to the MUSB host driver

2016-05-03 Thread Sergei Shtylyov
Hello.

   Here's 2 patches against the 'next' branch of Felipe's 'usb.git' repo. Tony
Lindgren's patches from the last year didn't seem complete, so trying to clean
up the TX DMA patch...

[1/2] musb_host: move DMA engine check from musb_tx_dma_set_mode_cppi_tusb() to 
its caller
[2/2] musb_host: make musb_tx_dma_set_mode_*() *void*

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


[PATCH] USB: serial: option: add more ZTE device ids

2016-05-03 Thread gre...@linuxfoundation.org
From: lei liu 

More ZTE device ids.

Signed-off-by: lei liu 
Cc: stable 
[properly sort them - gregkh]
Signed-off-by: Greg Kroah-Hartman 

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index c6f497f16526..c25ca7f77c42 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -1602,7 +1602,79 @@ static const struct usb_device_id option_ids[] = {
.driver_info = (kernel_ulong_t)_intf3_blacklist },
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x0178, 0xff, 0xff, 
0xff),
.driver_info = (kernel_ulong_t)_intf3_blacklist },
-   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xffe9, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff42, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff43, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff44, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff45, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff46, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff47, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff48, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff49, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff4a, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff4b, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff4c, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff4d, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff4e, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff4f, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff50, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff51, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff52, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff53, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff54, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff55, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff56, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff57, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff58, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff59, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff5a, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff5b, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff5c, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff5d, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff5e, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff5f, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff60, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff61, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff62, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff63, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff64, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff65, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff66, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff67, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff68, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff69, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff6a, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff6b, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff6c, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff6d, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff6e, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff6f, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff70, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff71, 0xff, 0xff, 
0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff72, 0xff, 0xff, 
0xff) },
+   { 

Re: [patch] usb: dwc3: gadget: fix mask and shift order in DWC3_DCFG_NUMP()

2016-05-03 Thread Greg Kroah-Hartman
On Tue, May 03, 2016 at 10:53:05AM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Dan Carpenter  writes:
> > In the original DWC3_DCFG_NUMP() was always zero.  It looks like the
> > intent was to shift first and then do the mask.
> >
> > Fixes: 2a58f9c12bb3 ('usb: dwc3: gadget: disable automatic calculation of 
> > ACK TP NUMP')
> > Signed-off-by: Dan Carpenter 
> 
> Thanks for this fix, I completely missed it :-) Greg, if you want to
> take this as a patch, that's fine by me. Otherwise I can queue it for
> -rc1. In any case, S-o-B below:
> 
> Signed-off-by: Felipe Balbi 

I'll take it, 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: add device id to linux kernel

2016-05-03 Thread gre...@linuxfoundation.org
On Tue, May 03, 2016 at 07:56:54AM +0700, Lars Melin wrote:
> On 2016-05-02 20:43, gre...@linuxfoundation.org wrote:
> > On Mon, May 02, 2016 at 09:22:57AM +0300, Felipe Balbi wrote:
> > > 
> > > Hi,
> > > 
> > > 刘磊  writes:
> > > > dear linuxfoundation:
> > > >  I am liulei come from ZTEMT.  We need add some device ID to linux 
> > > > kernel, We need some help from yours.
> > > > Looking forward to you reply, thanks!
> > > > 
> > > > 
> > > > Signed-off-by:lei liu
> > 
> > 
> > 
> > > Do all these devices really exist ? If they do, then please just send
> > > this as a proper patch and it should be simple to get it merged. See
> > > Documentation/SubmittingPatches for some hints as to how to write proper
> > > patches, but basically, your patch needs a commit log with a
> > > Signed-off-by line.
> > 
> > There is a signed-off-by line :)
> > 
> > --
> The patch is anyway wrong since it is inserted in the wrong place and the
> id's are reverse sorted.

I'll go fix it up...

--
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 v1 1/1] usbip: adding names db to port operation

2016-05-03 Thread Greg KH
On Mon, May 02, 2016 at 12:02:08AM +, fx IWATA NOBUO wrote:
> > Does nothing need to be changed in the usbip code to add this new feature?
> 
> Yes, nothing else is needed.
> 
> Name conversion itself is already done in
> libsrc/vhci_driver.c:usbip_vhci_imported_device_dump().
> 
> For 'list' operation, it works because name DB is initialized.
> But, for 'port' operation, it doesn't work because it's not initialized.
> 
> This patch just add name DB initialization which activates name conversion to 
> 'port'.
> 
> > Is this a stand-alone patch I can take without anything else?
> 
> Yes, this is a stand-alone patch.

Thanks for the information.

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: hub: fix panic caused by NULL bos pointer during reset device

2016-05-03 Thread Greg KH
On Wed, Apr 27, 2016 at 09:35:57AM -0400, Tony Battersby wrote:
> On 04/26/2016 10:53 PM, Du, Changbin wrote:
> >> On Tue, Mar 08, 2016 at 05:15:17PM +0800, changbin...@intel.com wrote:
> >>> From: "Du, Changbin" 
> >>>
> >>> This is a reworked patch based on reverted commit d8f00cd685f5 ("usb:
> >>> hub: do not clear BOS field during reset device").
> >>>
> >>> The privious one caused double mem-free if run to re_enumerate label.
> >>> New patch title changed to distinguish from old one. And I have tested
> >>> it with memory debugging options.
> >>>
> >>> In function usb_reset_and_verify_device, the old BOS descriptor may
> >>> still be used before allocating a new one. (usb_disable_lpm function
> >>> uses it under the situation that it fails at usb_disable_link_state.)
> >>> So we cannot set the udev->bos to NULL before that, just keep what it
> >>> was. It will be overwrite when allocating a new one.
> >>>
> >>> How to reproduce:
> >>> 1. connect one usb3 hub to xhci port.
> >>> 2. connect several lpm-capable super-speed usb disk to the hub.
> >>> 3. copy big files to the usb disks.
> >>> 4. disconnect the hub and repeat step 1-4.
> >>>
> >>> Crash log:
> >>> BUG: unable to handle kernel NULL pointer dereference at
> >>> 0010
> >>> IP: [] usb_enable_link_state+0x2d/0x2f0
> >>> Call Trace:
> >>> [] ? usb_set_lpm_timeout+0x12b/0x140
> >>> [] usb_enable_lpm+0x81/0xa0
> >>> [] usb_disable_lpm+0xa8/0xc0
> >>> [] usb_unlocked_disable_lpm+0x2c/0x50
> >>> [] usb_reset_and_verify_device+0xc3/0x710
> >>> [] ? usb_sg_wait+0x13d/0x190
> >>> [] usb_reset_device+0x133/0x280
> >>> [] usb_stor_port_reset+0x61/0x70
> >>> [] usb_stor_invoke_transport+0x88/0x520
> >>>
> >>> Signed-off-by: Du, Changbin 
> >>> ---
> >>>  drivers/usb/core/hub.c | 14 +-
> >>>  1 file changed, 9 insertions(+), 5 deletions(-)
> >> Is this patch still needed?  I thought we had some other fix in this
> >> area...
> >>
> >> confused,
> >>
> >> greg k-h
> >>
> > Hi, Greg k-h,
> > Sorry for it confused you. This patch still need. This is same fix with
> > previous commit d8f00cd685f5 ("usb: hub: do not clear BOS field
> > during reset device"). But d8f00cd685f5 is buggy and reverted. This
> > new patch should be the final fix.
> >
> > Best Regards,
> > Du, Changbin
> >
> 
> I think Greg is referring to commit 464ad8c43a9e ("usb: core : hub: Fix
> BOS 'NULL pointer' kernel panic"), which has already been applied
> upstream.  It looks to me like that patch might have fixed the same
> problem in a different way, in which case Changbin's patch is not
> needed.  But I haven't been involved in developing or testing that
> patch, so I can't say for sure.  At the very least, 464ad8c43a9e
> conflicts with Changbin's patch.
> 
> Changbin, can you take a look at 464ad8c43a9e and see if that fixes the
> same problem that your patch did?

Given the lack of response here, I've dropped this from my queue.  If
it's still needed, someone must resend it.

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: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions

2016-05-03 Thread Dean Jenkins

On 03/05/16 15:42, David B. Robins wrote:


I don't think the first one is giving you problems (except as 
triggered by the second) but I had concerns about the second myself 
(and emailed the author off-list, but received no reply), and we did 
not take that commit for our own product.



Sorry, I might have missed your original E-mail.

Specifically, the second change, 3f30... (original patch: 
https://www.mail-archive.com/netdev@vger.kernel.org/msg80720.html) (1) 
appears to do the exact opposite of what it claims, i.e., instead of 
"resync if this looks like a header", it does "resync if this does NOT 
look like a (packet) header", where "looks like a header" means "bits 
0-10 (size) are equal to the bitwise-NOT of bits 16-26", and (2) can 
happen by coincidence for 1/2048 32-bit values starting a continuation 
URB (easy to hit dealing with large volumes of video data as we were). 
It appears to expect the header for every URB whereas the rest of the 
code at least expects it only once per network packet (look at 
following code that only reads it for remaining == 0).


David, I think that your interpretation is incorrect. Please see below.

Here is the code snippet from the patch with my annotations between # #, 
I will try to explain my intentions. Feel free to point out any flaws:


if (rx->remaining && (rx->remaining + sizeof(u32) <= skb->len)) {
# Only runs when rx->remaining !=0 and the end of the Ethernet 
frame + next 32-bit header word is within the URB buffer. #
# Therefore, this code does not run when the end of an Ethernet 
frame has been reached in the previous URB #
# or when the end of the Ethernet frame + next 32-bit header 
word will be in a later URB buffer #


# offset is an index to the expected next 32-bit header word 
after the end of the Ethernet frame #

offset = ((rx->remaining + 1) & 0xfffe) + sizeof(u32);

# rx->header contains the expected 32-bit header value 
corrected for Endianness and alignment #

rx->header = get_unaligned_le32(skb->data + offset);
offset = 0;

# check the data integrity of the size value from the header word #
size = (u16)(rx->header & 0x7ff);
# if the size value fails the integrity check then we are not 
looking at a valid header word so #

# synchronisation has been lost #
if (size != ((~rx->header >> 16) & 0x7ff)) {
netdev_err(dev->net, "asix_rx_fixup() Data Header 
synchronisation was lost, remaining %d\n",

   rx->remaining);
if (rx->ax_skb) {
kfree_skb(rx->ax_skb);
rx->ax_skb = NULL;
/* Discard the incomplete netdev Ethernet frame
 * and assume the Data header is at the start of
 * the current URB socket buffer.
 */
}
rx->remaining = 0;
}
}




So that change made no sense to me, but I don't have significant 
kernel dev experience. Effectively it will drop/truncate every 
(2047/2048) split (longer than an URB) packet, and report an error for 
the second URB and then again for treating said second URB as a first 
URB for a packet. I would expect your problems will go away just 
removing the second change. You could also change the != to == in "if 
(size != ...)" but then you'd still have 1/2048 (depending on data 
patterns) false positives.
The code only runs when the Ethernet frame spans across URBs and is 
checking that the next 32-bit header word is present and valid.


Upon loss of synchronisation, the strategy is to assume that the 32-bit 
header is at the start of the URB buffer. Obviously, that might not be 
true every time but it is the most likely location especially when 
Ethernet frames are not spanning URBs at that point at time.


Looking at the error messages:


[  239.037310] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0x54ebb5ec, offset 4 


The offset 4 means that the 32-bit header word was invalid at the start 
of the URB buffer. This could be a consequence of data synchronisation 
being lost however, we would expect the timestamps between the error 
messages of "synchronisation was lost" and "Bad Header Length" to very 
close as they would be consecutive URBs. The evidence is showing 10ms 
gaps which does not suggest consecutive URBs. In other words, an 
Ethernet frame should not be spanned over a time gap of 10ms as that 
would be very inefficient. If that were true then there would be USB 
communications problem with the USB to Ethernet adaptor which I hope is 
not true.


[  239.027993] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988

This error message consistently shows the remaining value to be 988, at 
least for the 3 examples provided by John. This does not suggest a 
random failure unless there are other examples of a non 988 remaining 
value error message. 988 is well within a Ethernet frame 

[PATCH] fix infoleak in devio

2016-05-03 Thread Kangjie Lu
The stack object “ci” has a total size of 8 bytes. Its last 3 bytes
are padding bytes which are not initialized and leaked to userland
via “copy_to_user”.

Signed-off-by: Kangjie Lu 
---
 drivers/usb/core/devio.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 52c4461..9b7f1f7 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1316,10 +1316,11 @@ static int proc_getdriver(struct usb_dev_state *ps, 
void __user *arg)
 
 static int proc_connectinfo(struct usb_dev_state *ps, void __user *arg)
 {
-   struct usbdevfs_connectinfo ci = {
-   .devnum = ps->dev->devnum,
-   .slow = ps->dev->speed == USB_SPEED_LOW
-   };
+   struct usbdevfs_connectinfo ci;
+
+   memset(, 0, sizeof(ci));
+   ci.devnum = ps->dev->devnum;
+   ci.slow = ps->dev->speed == USB_SPEED_LOW;
 
if (copy_to_user(arg, , sizeof(ci)))
return -EFAULT;
-- 
1.9.1

--
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 3/3] rtl8152: correct speed testing

2016-05-03 Thread David Miller
From: Oliver Neukum 
Date: Mon,  2 May 2016 13:06:14 +0200

> Allow for SS+ USB
> 
> Signed-off-by: Oliver Neukum 

Applied.
--
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/3] usbnet: correct speed testing

2016-05-03 Thread David Miller
From: Oliver Neukum 
Date: Mon,  2 May 2016 13:06:13 +0200

> Allow for SS+ USB
> 
> Signed-off-by: Oliver Neukum 

Applied.
--
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/3] brcm80211: correct speed testing

2016-05-03 Thread David Miller
From: Oliver Neukum 
Date: Mon,  2 May 2016 13:06:12 +0200

> Allow for SS+ USB
> 
> Signed-off-by: Oliver Neukum 

Applied.
--
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: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Alan Stern
On Tue, 3 May 2016, Susmita/Rajib Bandopadhyay wrote:

> Dear Sir,
> 
> To me you appear like Out Of The World! Such understanding! I was
> unfortunate not to have been born to be near you. I would have learnt
> a lot, then.
> 
> People I have come across are petty, and pale in your comparison! I
> would have given up my right to be alive to have learnt computing from
> people like you by asking you questions.
> 
> The driver provided by the seller in a miniCD is on the google drive.
> The link is here:
> https://drive.google.com/open?id=0B0qPAe-7HylCV0VRM01sQkppR3M
> 
> Writing a Linux driver is way beyond my acumen, Sir! I never had an
> opportunity to be with and learn from people like you. I only know to
> identify great analytical abilites when I come across such people, and
> I thank nature for bringing me to their viccinity. The thought of
> using people like you for my self-interests doesn't even occur to me.
> It is a pity that the world doesn't really realise your value. You all
> are God's miracle upon the earth!
> 
> So if you suggest a device that is Linux compliant and could read a
> SIM card with existing Linux tools, I would search for it in India and
> buy it.

Maybe your device can be used with existing Linux tools.  Have you
tried asking Google?  There is a program called monosim that might do
what you want.

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] usb: host: ehci-tegra: Avoid getting the same reset twice

2016-05-03 Thread Alan Stern
On Tue, 3 May 2016, Thierry Reding wrote:

> From: Thierry Reding 
> 
> Starting with commit 0b52297f2288 ("reset: Add support for shared reset
> controls") there is a reference count for reset control assertions. The
> goal is to allow resets to be shared by multiple devices and an assert
> will take effect only when all instances have asserted the reset.
> 
> In order to preserve backwards-compatibility, all reset controls become
> exclusive by default. This is to ensure that reset_control_assert() can
> immediately assert in hardware.
> 
> However, this new behaviour triggers the following warning in the EHCI
> driver for Tegra:

...

> The reason is that the EHCI implements three ports, each with a separate
> reset line. However the first port's reset also serves as a means to
> reset the UTMI pad for all ports. There is special code in the driver to
> assert and deassert this shared reset at probe time. It needs to do this
> regardless of which port is probed first. Unfortunately this means that
> if the first port is probed first, it will request its own reset line
> and subsequently request the same reset line again (temporarily) to
> perform the reset. This used to work fine before the above-mentioned
> commit, but now triggers the new WARN.
> 
> Work around this by making sure we reuse the port's reset if it happens
> to be the same as the UTMI pad reset.
> 
> Cc: Philipp Zabel 
> Cc: Hans de Goede 
> Signed-off-by: Thierry Reding 

Possibly related to the problems you are still seeing, I noticed 
something odd about the patch.

> --- a/drivers/usb/host/ehci-tegra.c
> +++ b/drivers/usb/host/ehci-tegra.c
> @@ -81,15 +81,23 @@ static int tegra_reset_usb_controller(struct 
> platform_device *pdev)
>   struct usb_hcd *hcd = platform_get_drvdata(pdev);
>   struct tegra_ehci_hcd *tegra =
>   (struct tegra_ehci_hcd *)hcd_to_ehci(hcd)->priv;
> + bool has_utmi_pad_registers = false;
>  
>   phy_np = of_parse_phandle(pdev->dev.of_node, "nvidia,phy", 0);
>   if (!phy_np)
>   return -ENOENT;
>  
> + if (of_property_read_bool(phy_np, "nvidia,has-utmi-pad-registers"))
> + has_utmi_pad_registers = true;

I would have writte simply:

has_utmi_pad_registers = of_property_read_bool(phy_np, 
"nvidia,has-utmi-pad-registers");

Regardless, notice that has_utmi_pad_registers gets set to true if the 
OF property value is true.

...

> - if (!of_property_read_bool(phy_np, "nvidia,has-utmi-pad-registers")) {
> + if (has_utmi_pad_registers) {

But down here the sense of the test is reversed.  Now the conditional 
block gets executed if the property is true, whereas the original code 
executed it if the property was false.

>   reset_control_assert(tegra->rst);
>   udelay(1);
>   reset_control_deassert(tegra->rst);

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] usb: host: ehci-tegra: Avoid getting the same reset twice

2016-05-03 Thread Thierry Reding
On Tue, May 03, 2016 at 08:05:55PM +0200, Thierry Reding wrote:
> From: Thierry Reding <tred...@nvidia.com>
> 
> Starting with commit 0b52297f2288 ("reset: Add support for shared reset
> controls") there is a reference count for reset control assertions. The
> goal is to allow resets to be shared by multiple devices and an assert
> will take effect only when all instances have asserted the reset.
> 
> In order to preserve backwards-compatibility, all reset controls become
> exclusive by default. This is to ensure that reset_control_assert() can
> immediately assert in hardware.
> 
> However, this new behaviour triggers the following warning in the EHCI
> driver for Tegra:
> 
> [3.365019] [ cut here ]
> [3.369639] WARNING: CPU: 0 PID: 1 at drivers/reset/core.c:187 
> __of_reset_control_get+0x16c/0x23c
> [3.382151] Modules linked in:
> [    3.385214] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
> 4.6.0-rc6-next-20160503 #140
> [3.392769] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> [3.399046] [] (unwind_backtrace) from [] 
> (show_stack+0x10/0x14)
> [3.406787] [] (show_stack) from [] 
> (dump_stack+0x90/0xa4)
> [3.414007] [] (dump_stack) from [] (__warn+0xe8/0x100)
> [3.420964] [] (__warn) from [] 
> (warn_slowpath_null+0x20/0x28)
> [3.428525] [] (warn_slowpath_null) from [] 
> (__of_reset_control_get+0x16c/0x23c)
> [3.437648] [] (__of_reset_control_get) from [] 
> (tegra_ehci_probe+0x394/0x518)
> [3.446600] [] (tegra_ehci_probe) from [] 
> (platform_drv_probe+0x4c/0xb0)
> [3.455029] [] (platform_drv_probe) from [] 
> (driver_probe_device+0x1ec/0x330)
> [3.463892] [] (driver_probe_device) from [] 
> (__driver_attach+0xb8/0xbc)
> [3.472320] [] (__driver_attach) from [] 
> (bus_for_each_dev+0x68/0x9c)
> [3.480489] [] (bus_for_each_dev) from [] 
> (bus_add_driver+0x1a0/0x218)
> [3.488743] [] (bus_add_driver) from [] 
> (driver_register+0x78/0xf8)
> [3.496738] [] (driver_register) from [] 
> (do_one_initcall+0x40/0x170)
> [3.504909] [] (do_one_initcall) from [] 
> (kernel_init_freeable+0x158/0x1f8)
> [3.513600] [] (kernel_init_freeable) from [] 
> (kernel_init+0x8/0x114)
> [3.521770] [] (kernel_init) from [] 
> (ret_from_fork+0x14/0x3c)
> [3.529361] ---[ end trace 4bda87dbe4ecef8a ]---
> 
> The reason is that the EHCI implements three ports, each with a separate
> reset line. However the first port's reset also serves as a means to
> reset the UTMI pad for all ports. There is special code in the driver to
> assert and deassert this shared reset at probe time. It needs to do this
> regardless of which port is probed first. Unfortunately this means that
> if the first port is probed first, it will request its own reset line
> and subsequently request the same reset line again (temporarily) to
> perform the reset. This used to work fine before the above-mentioned
> commit, but now triggers the new WARN.
> 
> Work around this by making sure we reuse the port's reset if it happens
> to be the same as the UTMI pad reset.
> 
> Cc: Philipp Zabel <p.za...@pengutronix.de>
> Cc: Hans de Goede <hdego...@redhat.com>
> Signed-off-by: Thierry Reding <tred...@nvidia.com>
> ---
>  drivers/usb/host/ehci-tegra.c | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)

Looks like I sent this too quickly. It doesn't seem to get rid of the
warning in all cases, so I'll need to investigate further.

Sorry for the noise.

Thierry


signature.asc
Description: PGP signature


[PATCH] usb: host: ehci-tegra: Avoid getting the same reset twice

2016-05-03 Thread Thierry Reding
From: Thierry Reding <tred...@nvidia.com>

Starting with commit 0b52297f2288 ("reset: Add support for shared reset
controls") there is a reference count for reset control assertions. The
goal is to allow resets to be shared by multiple devices and an assert
will take effect only when all instances have asserted the reset.

In order to preserve backwards-compatibility, all reset controls become
exclusive by default. This is to ensure that reset_control_assert() can
immediately assert in hardware.

However, this new behaviour triggers the following warning in the EHCI
driver for Tegra:

[3.365019] [ cut here ]
[3.369639] WARNING: CPU: 0 PID: 1 at drivers/reset/core.c:187 
__of_reset_control_get+0x16c/0x23c
[3.382151] Modules linked in:
[3.385214] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
4.6.0-rc6-next-20160503 #140
[3.392769] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[3.399046] [] (unwind_backtrace) from [] 
(show_stack+0x10/0x14)
[3.406787] [] (show_stack) from [] 
(dump_stack+0x90/0xa4)
[3.414007] [] (dump_stack) from [] (__warn+0xe8/0x100)
[3.420964] [] (__warn) from [] 
(warn_slowpath_null+0x20/0x28)
[3.428525] [] (warn_slowpath_null) from [] 
(__of_reset_control_get+0x16c/0x23c)
[3.437648] [] (__of_reset_control_get) from [] 
(tegra_ehci_probe+0x394/0x518)
[3.446600] [] (tegra_ehci_probe) from [] 
(platform_drv_probe+0x4c/0xb0)
[3.455029] [] (platform_drv_probe) from [] 
(driver_probe_device+0x1ec/0x330)
[3.463892] [] (driver_probe_device) from [] 
(__driver_attach+0xb8/0xbc)
[3.472320] [] (__driver_attach) from [] 
(bus_for_each_dev+0x68/0x9c)
[3.480489] [] (bus_for_each_dev) from [] 
(bus_add_driver+0x1a0/0x218)
[3.488743] [] (bus_add_driver) from [] 
(driver_register+0x78/0xf8)
[3.496738] [] (driver_register) from [] 
(do_one_initcall+0x40/0x170)
[3.504909] [] (do_one_initcall) from [] 
(kernel_init_freeable+0x158/0x1f8)
[3.513600] [] (kernel_init_freeable) from [] 
(kernel_init+0x8/0x114)
[3.521770] [] (kernel_init) from [] 
(ret_from_fork+0x14/0x3c)
[3.529361] ---[ end trace 4bda87dbe4ecef8a ]---

The reason is that the EHCI implements three ports, each with a separate
reset line. However the first port's reset also serves as a means to
reset the UTMI pad for all ports. There is special code in the driver to
assert and deassert this shared reset at probe time. It needs to do this
regardless of which port is probed first. Unfortunately this means that
if the first port is probed first, it will request its own reset line
and subsequently request the same reset line again (temporarily) to
perform the reset. This used to work fine before the above-mentioned
commit, but now triggers the new WARN.

Work around this by making sure we reuse the port's reset if it happens
to be the same as the UTMI pad reset.

Cc: Philipp Zabel <p.za...@pengutronix.de>
Cc: Hans de Goede <hdego...@redhat.com>
Signed-off-by: Thierry Reding <tred...@nvidia.com>
---
 drivers/usb/host/ehci-tegra.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 4031b372008e..c8fe0fe9de68 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -81,15 +81,23 @@ static int tegra_reset_usb_controller(struct 
platform_device *pdev)
struct usb_hcd *hcd = platform_get_drvdata(pdev);
struct tegra_ehci_hcd *tegra =
(struct tegra_ehci_hcd *)hcd_to_ehci(hcd)->priv;
+   bool has_utmi_pad_registers = false;
 
phy_np = of_parse_phandle(pdev->dev.of_node, "nvidia,phy", 0);
if (!phy_np)
return -ENOENT;
 
+   if (of_property_read_bool(phy_np, "nvidia,has-utmi-pad-registers"))
+   has_utmi_pad_registers = true;
+
if (!usb1_reset_attempted) {
struct reset_control *usb1_reset;
 
-   usb1_reset = of_reset_control_get(phy_np, "usb");
+   if (!has_utmi_pad_registers)
+   usb1_reset = of_reset_control_get(phy_np, "usb");
+   else
+   usb1_reset = tegra->rst;
+
if (IS_ERR(usb1_reset)) {
dev_warn(>dev,
 "can't get utmi-pads reset from the PHY\n");
@@ -101,11 +109,13 @@ static int tegra_reset_usb_controller(struct 
platform_device *pdev)
reset_control_deassert(usb1_reset);
}
 
-   reset_control_put(usb1_reset);
+   if (!has_utmi_pad_registers)
+   reset_control_put(usb1_reset);
+
usb1_reset_attempted = true;
}
 
-   if (!of_property_read_bool(phy_np, "nvidia,has-utmi-pad-registers")) {
+   if (has_utmi_pad_registers) {
rese

Re: [RFT PATCH 1/3] usb: misc: usb3503: Fix HUB mode after bootloader initialization

2016-05-03 Thread Rob Herring
On Mon, May 02, 2016 at 11:55:01AM +0100, Mark Brown wrote:
> On Mon, May 02, 2016 at 11:49:12AM +0200, Krzysztof Kozlowski wrote:
> 
> > This VDD regulator supply actually is not a usb3503 USB HUB regulator
> > supply... but a supply to the LAN attached to this HUB. Regulator off/on
> > is needed for LAN to show up. The hub will show up with typical reset
> > (which is also missing before my patchset btw).
> 
> > The LAN, as a USB device, is auto-probed so it cannot take the regulator
> > and play with it. The simplest idea I have is to add it as "external
> > supply"  to the parent: usb3503.
> 
> This is common enough that that just isn't going to scale well I fear
> without some generic handling, either walking child devices at the bus
> level or at the device level with a pre-probe() callback to get the
> device to power on.  The latter is more appropriate to things like
> Slimbus where the device is more likely to do active management at
> runtime, it's not clear people are building USB devices like that.

There's a new binding and support in -next (.../usb/usb-device.txt) for 
USB devices that should help here. Though, how to handle a hub on USB 
and I2C buses would need to be worked out.

Rob
--
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: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Susmita/Rajib Bandopadhyay
Dear Sir,

To me you appear like Out Of The World! Such understanding! I was
unfortunate not to have been born to be near you. I would have learnt
a lot, then.

People I have come across are petty, and pale in your comparison! I
would have given up my right to be alive to have learnt computing from
people like you by asking you questions.

The driver provided by the seller in a miniCD is on the google drive.
The link is here:
https://drive.google.com/open?id=0B0qPAe-7HylCV0VRM01sQkppR3M

Writing a Linux driver is way beyond my acumen, Sir! I never had an
opportunity to be with and learn from people like you. I only know to
identify great analytical abilites when I come across such people, and
I thank nature for bringing me to their viccinity. The thought of
using people like you for my self-interests doesn't even occur to me.
It is a pity that the world doesn't really realise your value. You all
are God's miracle upon the earth!

So if you suggest a device that is Linux compliant and could read a
SIM card with existing Linux tools, I would search for it in India and
buy it.

My greatest regards to you, Sir!
--
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 00/10] Get MUSB PM runtime working again

2016-05-03 Thread Ivaylo Dimitrov

Hi,

On  3.05.2016 19:25, Tony Lindgren wrote:

* Ivaylo Dimitrov  [160430 23:34]:


Unfortunately that makes my n900 unbootable with USB cable detached, see the
attached file (at [  152.121673]) when there is USB cable. I guess we're
missing a couple of pm_runtime_get()/put() calls.


OK so it seems. I did some brief testing with n900 too, but never
hit this so far. Care to email me the kernel .config you're using
so I can try to reproduce it here?



.config is attached. There is stuff in it still to be upstreamed, just 
ignore it :). Or, you may pull 
https://github.com/freemangordon/linux-n900/commits/v4.6-rc4-n900-camera, apply 
the $subject series on top of it and build rx51_defconfig. There 
shouldn't be any patches touching USB.


Does bqXXX and isp1707_charger modules got loaded when you tested it? Or 
those were built-in? or not built at all? As it seems the problem arises 
when those try to touch USB ULPI.



[  152.121673] Unhandled fault: external abort on non-linefetch (0x1028) at 
0xfa0ab001

...


[  152.403594] [] (musb_default_readb) from [] 
(musb_ulpi_write+0x34/0xf8)
[  152.409454] [] (musb_ulpi_write) from [] 
(isp1704_charger_probe+0x15c/0x4e0 [isp1704_charger])
[  152.421325] [] (isp1704_charger_probe [isp1704_charger]) from 
[] (platform_drv_probe+0x58/0xa0)
[  152.433624] [] (platform_drv_probe) from [] 
(driver_probe_device+0x120/0x2b0)
[  152.440093] [] (driver_probe_device) from [] 
(__driver_attach+0x80/0xa4)
[  152.446624] [] (__driver_attach) from [] 
(bus_for_each_dev+0x50/0x88)
[  152.453155] [] (bus_for_each_dev) from [] 
(bus_add_driver+0xc4/0x1e4)
[  152.459716] [] (bus_add_driver) from [] 
(driver_register+0x9c/0xe0)
[  152.466247] [] (driver_register) from [] 
(do_one_initcall+0xf8/0x1c4)
[  152.472839] [] (do_one_initcall) from [] 
(do_init_module+0x54/0x1ac)
[  152.479400] [] (do_init_module) from [] 
(load_module+0x185c/0x1b14)
[  152.485961] [] (load_module) from [] 
(SyS_init_module+0x140/0x158)
[  152.492553] [] (SyS_init_module) from [] 
(ret_fast_syscall+0x0/0x3c)


Yeah looks like a case of missing PM runtime get/put.

Also, I'm aware of some remaining phy-twl4030-usb issues with
having cable plugged in on the boot. Will send patches for those
after the MUSB core issues are out of the way.



Hmm, maybe I was unclear - with no cable attached I am unable to boot 
the device at all, is it goes in an oops loop. Unfortunately I can't 
provide logs as I don't have serial port cable attached to my device(it 
is production device). The logs I provided are taken with USB cable 
attached at boot.


Thanks,
Ivo
#
# Automatically generated file; DO NOT EDIT.
# Linux/arm 4.6.0-rc4 Kernel Configuration
#
CONFIG_ARM=y
CONFIG_ARM_HAS_SG_CHAIN=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_ARM_DMA_USE_IOMMU=y
CONFIG_ARM_DMA_IOMMU_ALIGNMENT=8
CONFIG_MIGHT_HAVE_PCI=y
CONFIG_SYS_SUPPORTS_APM_EMULATION=y
CONFIG_HAVE_PROC_CPU=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_TRACE_IRQFLAGS_SUPPORT=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_ARCH_HAS_BANDGAP=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_VECTORS_BASE=0x
CONFIG_ARM_PATCH_PHYS_VIRT=y
CONFIG_GENERIC_BUG=y
CONFIG_PGTABLE_LEVELS=2
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y

#
# General setup
#
CONFIG_BROKEN_ON_SMP=y
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION=""
# CONFIG_LOCALVERSION_AUTO is not set
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_HAVE_KERNEL_LZ4=y
CONFIG_KERNEL_GZIP=y
# CONFIG_KERNEL_LZMA is not set
# CONFIG_KERNEL_XZ is not set
# CONFIG_KERNEL_LZO is not set
# CONFIG_KERNEL_LZ4 is not set
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_CROSS_MEMORY_ATTACH=y
# CONFIG_FHANDLE is not set
CONFIG_USELIB=y
# CONFIG_AUDIT is not set
CONFIG_HAVE_ARCH_AUDITSYSCALL=y

#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_IRQ_SHOW_LEVEL=y
CONFIG_HARDIRQS_SW_RESEND=y
CONFIG_GENERIC_IRQ_CHIP=y
CONFIG_IRQ_DOMAIN=y
CONFIG_HANDLE_DOMAIN_IRQ=y
# CONFIG_IRQ_DOMAIN_DEBUG is not set
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_GENERIC_CLOCKEVENTS=y

#
# Timers subsystem
#
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ_COMMON=y
# CONFIG_HZ_PERIODIC is not set
CONFIG_NO_HZ_IDLE=y
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y

#
# CPU/Task time and stats accounting
#
CONFIG_TICK_CPU_ACCOUNTING=y
# CONFIG_VIRT_CPU_ACCOUNTING_GEN is not set
# CONFIG_IRQ_TIME_ACCOUNTING is not set
CONFIG_BSD_PROCESS_ACCT=y
# CONFIG_BSD_PROCESS_ACCT_V3 is not set
CONFIG_TASKSTATS=y
CONFIG_TASK_DELAY_ACCT=y
CONFIG_TASK_XACCT=y
CONFIG_TASK_IO_ACCOUNTING=y

#
# RCU Subsystem
#
CONFIG_PREEMPT_RCU=y

Re: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Alan Stern
On Tue, 3 May 2016, Susmita/Rajib Bandopadhyay wrote:

> Great, Sir!
> Here is the file.

This pattern repeats over and over in the file:

88009ff42c00 2926401044 S Bo:1:003:2 -115 31 = 55534243 0600  
0600    00
88009ff42c00 2926401182 C Bo:1:003:2 0 31 >
88009ff42c00 2926401197 S Bi:1:003:1 -115 13 <
88009ff42c00 2926401299 C Bi:1:003:1 0 13 = 55534253 0600  01
88009ff42c00 2926401308 S Bo:1:003:2 -115 31 = 55534243 0700 1200 
8603 0012   00
88009ff42c00 2926401424 C Bo:1:003:2 0 31 >
88009c4f7240 2926401437 S Bi:1:003:1 -115 18 <
88009c4f7240 2926401673 C Bi:1:003:1 0 18 = 7200 000a  
3a00 

This shows the computer asking the card reader for its status, and the 
card reader returning an error code that means "Medium Not Present".  
In other words, the reader thinks there is no memory card present.

The kernel uses a standard protocol (Bulk-Only Mass Storage) to 
communicate with the reader, because that's what the reader says it 
will accept, according to the lsusb output.  Probably the protocol 
would work fine with an SDHC card, but it doesn't work with a SIM card.

That leaves two possibilities:

The reader doesn't work with SIM cards at all.  This may seem 
unlikely, but it's possible -- the eBay information could
simply be wrong.

It does work with SIM cards, but it needs to use a non-standard
protocol.

I guess that protocol is provided by the driver program in the software
that came with the reader.  In that case, to get it to work under Linux
you would have to learn what the protocol is and write a Linux driver
for it.

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 00/10] Get MUSB PM runtime working again

2016-05-03 Thread Tony Lindgren
* Ivaylo Dimitrov  [160430 23:34]:
> 
> Unfortunately that makes my n900 unbootable with USB cable detached, see the
> attached file (at [  152.121673]) when there is USB cable. I guess we're
> missing a couple of pm_runtime_get()/put() calls.

OK so it seems. I did some brief testing with n900 too, but never
hit this so far. Care to email me the kernel .config you're using
so I can try to reproduce it here?

> [  152.121673] Unhandled fault: external abort on non-linefetch (0x1028) at 
> 0xfa0ab001
...

> [  152.403594] [] (musb_default_readb) from [] 
> (musb_ulpi_write+0x34/0xf8)
> [  152.409454] [] (musb_ulpi_write) from [] 
> (isp1704_charger_probe+0x15c/0x4e0 [isp1704_charger])
> [  152.421325] [] (isp1704_charger_probe [isp1704_charger]) from 
> [] (platform_drv_probe+0x58/0xa0)
> [  152.433624] [] (platform_drv_probe) from [] 
> (driver_probe_device+0x120/0x2b0)
> [  152.440093] [] (driver_probe_device) from [] 
> (__driver_attach+0x80/0xa4)
> [  152.446624] [] (__driver_attach) from [] 
> (bus_for_each_dev+0x50/0x88)
> [  152.453155] [] (bus_for_each_dev) from [] 
> (bus_add_driver+0xc4/0x1e4)
> [  152.459716] [] (bus_add_driver) from [] 
> (driver_register+0x9c/0xe0)
> [  152.466247] [] (driver_register) from [] 
> (do_one_initcall+0xf8/0x1c4)
> [  152.472839] [] (do_one_initcall) from [] 
> (do_init_module+0x54/0x1ac)
> [  152.479400] [] (do_init_module) from [] 
> (load_module+0x185c/0x1b14)
> [  152.485961] [] (load_module) from [] 
> (SyS_init_module+0x140/0x158)
> [  152.492553] [] (SyS_init_module) from [] 
> (ret_fast_syscall+0x0/0x3c)

Yeah looks like a case of missing PM runtime get/put.

Also, I'm aware of some remaining phy-twl4030-usb issues with
having cable plugged in on the boot. Will send patches for those
after the MUSB core issues are out of the way.

Regards,

Tony
--
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: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Alan Stern
On Tue, 3 May 2016, Susmita/Rajib Bandopadhyay wrote:

> Sir,
> The same goes for Debian Squeeze, no directories /usb/usbmon/1u under
> /sys/kernel/debug
> debug directory is empty, without hidden files.

Ah, okay.  This means before running the test, you have to do this:

sudo mount -t debugfs none /sys/kernel/debug

I thought Knoppix and Debian did this for you automatically.

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: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Susmita/Rajib Bandopadhyay
Sir,
The same goes for Debian Squeeze, no directories /usb/usbmon/1u under
/sys/kernel/debug
debug directory is empty, without hidden files.
Regards
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 09/12] usb: gadget: udc: adapt to OTG core

2016-05-03 Thread Roger Quadros
Hi,

On 03/05/16 10:06, Jun Li wrote:
> Hi
> 
>  /**
> + * usb_gadget_start - start the usb gadget controller and
> +connect to bus
> + * @gadget: the gadget device to start
> + *
> + * This is external API for use by OTG core.
> + *
> + * Start the usb device controller and connect to bus (enable
>> pull).
> + */
> +static int usb_gadget_start(struct usb_gadget *gadget) {
> + int ret;
> + struct usb_udc *udc = NULL;
> +
> + dev_dbg(>dev, "%s\n", __func__);
> + mutex_lock(_lock);
> + list_for_each_entry(udc, _list, list)
> + if (udc->gadget == gadget)
> + goto found;
> +
> + dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
> + __func__);
> + mutex_unlock(_lock);
> + return -EINVAL;
> +
> +found:
> + ret = usb_gadget_udc_start(udc);
> + if (ret)
> + dev_err(>dev, "USB Device Controller didn't
 start: %d\n",
> + ret);
> + else
> + usb_udc_connect_control(udc);

 For drd, it's fine, but for real otg, gadget connect should be
 done by
 loc_conn() instead of gadget start.
>>>
>>> It is upto the OTG state machine to call gadget_start() when it
>>> needs to connect to the bus (i.e. loc_conn()). I see no point in
>>> calling gadget start before.
>>>
>>> Do you see any issue in doing so?
>>
>> This is what OTG state machine does:
>> case OTG_STATE_B_PERIPHERAL:
>>  otg_chrg_vbus(otg, 0);
>>  otg_loc_sof(otg, 0);
>>  otg_set_protocol(fsm, PROTO_GADGET);
>>  otg_loc_conn(otg, 1);
>>  break;

 On second thoughts, after seen the OTG state machine.
 otg_set_protocol(fsm, PROTO_GADGET); is always followed by
 otg_loc_conn(otg, 1); And whenever protocol changes to anything other
 the PROTO_GADGET, we use otg_loc_conn(otg, 0);

 So otg_loc_conn seems redundant. Can we just get rid of it?

 usb_gadget_start() implies that gadget controller starts up and
 enables pull.
 usb_gadget_stop() implies that gadget controller disables pull and
>> stops.


 Can you please explain why just these 2 APIs are not sufficient for
 full OTG?

 Do we want anything to happen between gadget controller start/stop
 and pull on/off?
>>>
>>> "loc_conn" is a standard output parameter in OTG spec, it deserves a
>>> separate api, yes, current implementation of OTG state machine code
>>> seems allow you to combine the 2 things into one, but don't do that,
>>> because they do not always happen together, e.g. for peripheral only B
>>> device (also a part OTG spec: section 7.3), will be fixed in gadget
>>> mode, but it will do gadget connect and disconnect in its diff states,
>>> so, to make the framework common, let's keep them separated.
>>
>> I'm sorry but I didn't understand your comment about "it will do gadget
>> connect and disconnect in its diff states"
> 
> Gadget connect means loc_conn(1).
> 
>>
>> I am reading the OTG v2.0 specification and loc_conn is always true when
>> b_peripheral or a_peripheral is true and false otherwise.
> 
> If you only talk about these 2 states, yes, loc_conn is ture.
> 
>>
>> loc_conn is just an internal state variable and it corresponds to our
>> gadget_start/stop() state.
> 
> It's not an internal variable, there are OTG state machine
> parameters tables(table 7-x) in OTG spec which have clear lists
> which are "internal variable", which are "input", which are "output"...
> 
> Those APIs are driven directly from OTG spec, easily understood so
> code reader can know what's those APIs for. For real OTG, I don't
> see the benefit if get rid of it.

OK, no issues if we don't get rid of it. But I am still in favor of
doing a connect in usb_gadget_start(), because

1) If we split connect/disconnect() and usb_gadget_start/stop() then there is
additional overhead of keeping track whether connect was called or not during
usb_gadget_stop(). Plus we need to take care that users don't call 
connect/disconnect
outside of start/stop. It is just complicating things.

2) for many controllers there is no difference between run/stop and
connect/disconnect. i.e. a single register bit controls both.

3) it fits well with the OTG specification. OTG specification says
that loc_conn *variable* must be true *after* the device has signalled a 
connect.
So OTG state machine can safely set loc_conn variable to true after doing
otg_set_protocol(fsm, PROTO_GADGET); and set it to false otherwise.

Note, OTG specification does not say to take any action based on loc_conn.
It is just a connect indicator variable. So we might have to fix this in the
OTG state machine.

My 

Re: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions

2016-05-03 Thread John Stultz
On Tue, May 3, 2016 at 7:42 AM, David B. Robins  wrote:
> On 2016-05-03 00:55, John Stultz wrote:
>>
>> Looking through the commits since the v4.1 kernel where we didn't see
>> this, I narrowed the regression down, and reverting the following two
>> commits seems to avoid the problem:
>>
>> 6a570814cd430fa5ef4f278e8046dcf12ee63f13 asix: Continue processing URB
>> if no RX netdev buffer
>> 3f30b158eba5c604b6e0870027eef5d19fc9271d asix: On RX avoid creating
>> bad Ethernet frames
>>
>
> I don't think the first one is giving you problems (except as triggered by
> the second) but I had concerns about the second myself (and emailed the
> author off-list, but received no reply), and we did not take that commit for
> our own product.

Yes, the first/later commit is being reverted as it modifies code that
was also changed by the second/earlier one. So the 3f30 patch doesn't
revert cleanly by itself, but I have tested by just removing the (now
modified by 6a57) chunk of code it adds does seem to avoid the problem
as well. Though I wasn't able to review things closely enough to be
confident that didn't introduce any subtle bugs in the remaining logic
in the 6a57 patch.

> Specifically, the second change, 3f30... (original patch:
> https://www.mail-archive.com/netdev@vger.kernel.org/msg80720.html) (1)
> appears to do the exact opposite of what it claims, i.e., instead of "resync
> if this looks like a header", it does "resync if this does NOT look like a
> (packet) header", where "looks like a header" means "bits 0-10 (size) are
> equal to the bitwise-NOT of bits 16-26", and (2) can happen by coincidence
> for 1/2048 32-bit values starting a continuation URB (easy to hit dealing
> with large volumes of video data as we were). It appears to expect the
> header for every URB whereas the rest of the code at least expects it only
> once per network packet (look at following code that only reads it for
> remaining == 0).
>
> So that change made no sense to me, but I don't have significant kernel dev
> experience. Effectively it will drop/truncate every (2047/2048) split
> (longer than an URB) packet, and report an error for the second URB and then
> again for treating said second URB as a first URB for a packet. I would
> expect your problems will go away just removing the second change. You could
> also change the != to == in "if (size != ...)" but then you'd still have
> 1/2048 (depending on data patterns) false positives.

I'll have to look into this more. I'm not super familiar with usb or
networking, so I'm not sure I can judge the better approach.

thanks
-john
--
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 net-next 4/5] treewide: replace dev->trans_start update with helper

2016-05-03 Thread Florian Westphal
Replace all trans_start updates with netif_trans_update helper.
change was done via spatch:

struct net_device *d;
@@
- d->trans_start = jiffies
+ netif_trans_update(d)

Compile tested only.

Cc: user-mode-linux-de...@lists.sourceforge.net
Cc: linux-xte...@linux-xtensa.org
Cc: linux1394-de...@lists.sourceforge.net
Cc: linux-r...@vger.kernel.org
Cc: net...@vger.kernel.org
Cc: mpt-fusionlinux@broadcom.com
Cc: linux-s...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: linux-par...@vger.kernel.org
Cc: linux-o...@vger.kernel.org
Cc: linux-h...@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: linux-wirel...@vger.kernel.org
Cc: linux-s...@vger.kernel.org
Cc: de...@driverdev.osuosl.org
Cc: b.a.t.m@lists.open-mesh.org
Cc: linux-blueto...@vger.kernel.org
Signed-off-by: Florian Westphal 
---
 Checkpatch complains about whitespace damage, but
 this extra whitespace already exists before this patch.

 arch/um/drivers/net_kern.c | 4 ++--
 arch/xtensa/platforms/iss/network.c| 2 +-
 drivers/char/pcmcia/synclink_cs.c  | 4 ++--
 drivers/firewire/net.c | 2 +-
 drivers/infiniband/hw/nes/nes_nic.c| 2 +-
 drivers/infiniband/ulp/ipoib/ipoib_cm.c| 2 +-
 drivers/infiniband/ulp/ipoib/ipoib_ib.c| 2 +-
 drivers/isdn/hysdn/hysdn_net.c | 2 +-
 drivers/isdn/i4l/isdn_net.c| 4 ++--
 drivers/isdn/i4l/isdn_x25iface.c   | 2 +-
 drivers/message/fusion/mptlan.c| 2 +-
 drivers/net/appletalk/cops.c   | 2 +-
 drivers/net/can/mscan/mscan.c  | 4 ++--
 drivers/net/can/usb/ems_usb.c  | 4 ++--
 drivers/net/can/usb/esd_usb2.c | 4 ++--
 drivers/net/can/usb/peak_usb/pcan_usb_core.c   | 4 ++--
 drivers/net/cris/eth_v10.c | 2 +-
 drivers/net/ethernet/3com/3c509.c  | 2 +-
 drivers/net/ethernet/3com/3c515.c  | 2 +-
 drivers/net/ethernet/3com/3c574_cs.c   | 2 +-
 drivers/net/ethernet/3com/3c589_cs.c   | 2 +-
 drivers/net/ethernet/3com/3c59x.c  | 2 +-
 drivers/net/ethernet/8390/axnet_cs.c   | 6 +++---
 drivers/net/ethernet/8390/lib8390.c| 4 ++--
 drivers/net/ethernet/adaptec/starfire.c| 2 +-
 drivers/net/ethernet/adi/bfin_mac.c| 2 +-
 drivers/net/ethernet/agere/et131x.c| 4 ++--
 drivers/net/ethernet/allwinner/sun4i-emac.c| 6 +++---
 drivers/net/ethernet/amd/7990.c| 4 ++--
 drivers/net/ethernet/amd/a2065.c   | 2 +-
 drivers/net/ethernet/amd/atarilance.c  | 2 +-
 drivers/net/ethernet/amd/au1000_eth.c  | 2 +-
 drivers/net/ethernet/amd/declance.c| 2 +-
 drivers/net/ethernet/amd/lance.c   | 2 +-
 drivers/net/ethernet/amd/ni65.c| 4 ++--
 drivers/net/ethernet/amd/nmclan_cs.c   | 2 +-
 drivers/net/ethernet/amd/pcnet32.c | 4 ++--
 drivers/net/ethernet/amd/sunlance.c| 2 +-
 drivers/net/ethernet/atheros/alx/main.c| 2 +-
 drivers/net/ethernet/broadcom/bcmsysport.c | 2 +-
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 +-
 drivers/net/ethernet/broadcom/sb1250-mac.c | 2 +-
 drivers/net/ethernet/broadcom/tg3.c| 2 +-
 drivers/net/ethernet/cavium/liquidio/lio_main.c| 4 ++--
 drivers/net/ethernet/cavium/octeon/octeon_mgmt.c   | 2 +-
 drivers/net/ethernet/cavium/thunder/nicvf_main.c   | 2 +-
 drivers/net/ethernet/chelsio/cxgb4vf/sge.c | 2 +-
 drivers/net/ethernet/davicom/dm9000.c  | 4 ++--
 drivers/net/ethernet/dec/tulip/de4x5.c | 4 ++--
 drivers/net/ethernet/dec/tulip/dmfe.c  | 6 +++---
 drivers/net/ethernet/dec/tulip/pnic.c  | 6 +++---
 drivers/net/ethernet/dec/tulip/tulip_core.c| 2 +-
 drivers/net/ethernet/dec/tulip/uli526x.c   | 4 ++--
 drivers/net/ethernet/dec/tulip/winbond-840.c   | 2 +-
 drivers/net/ethernet/dlink/dl2k.c  | 2 +-
 drivers/net/ethernet/dlink/sundance.c  | 2 +-
 drivers/net/ethernet/fealnx.c  | 2 +-
 drivers/net/ethernet/freescale/gianfar.c   | 2 +-
 drivers/net/ethernet/fujitsu/fmvj18x_cs.c  | 2 +-
 drivers/net/ethernet/hisilicon/hix5hd2_gmac.c  | 2 +-
 drivers/net/ethernet/hisilicon/hns/hns_enet.c  | 6 +++---
 drivers/net/ethernet/hp/hp100.c| 2 +-
 drivers/net/ethernet/i825xx/82596.c| 2 +-
 drivers/net/ethernet/i825xx/lib82596.c   

RE: [PATCH] usb: misc: usbtest: fix pattern tests for scatterlists.

2016-05-03 Thread David Laight
> interesting. We actually found a similar issue with XHCI. scatterlist
> has to be aligned to wMaxPacketSize but only before a link TRB. Mathias
> has been working on a solution which involves memcpy()ing enough bytes
> to align to wMaxPacketSize before the link TRB (it's very infrequent as
> we have 256 TRBs per segment on XHCI), but if you know of a nicer way,
> we're all ears :-)

That is a restriction of the xhci interface.
Link TRBs are only valid on USB buffer boundaries.
There is also the limitation that each TRB buffer can't
cross a 64k boundary.
Both require the TRB list be constructed very carefully.
(Having read the spec very carefully first.)

Possibly requiring backtracking (even aborting the URB itself)
since it is hard to work out beforehand how many TRB are needed.

If a link TRB would cross a USB packet boundary you could go back
to the previous boundary, cut the buffer and add the link TRB there.

Using 8 bytes of immediate data (instead of a pointer) in the TRB
may be a way out of the worst case scenario - it allows almost 2k
of data in 256 TRBs.

David

--
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: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Susmita/Rajib Bandopadhyay
Sir,
I have no directories /usb/usbmon/1u under /sys/kernel/debug in knoppix.
I will look into Debian and reply back.
Please give me some time.
Regards
--
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: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions

2016-05-03 Thread David B. Robins

On 2016-05-03 00:55, John Stultz wrote:

In testing with HiKey, we found that since commit 3f30b158eba5c60
(asix: On RX avoid creating bad Ethernet frames), we're seeing lots of
noise during network transfers:

[  239.027993] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  239.037310] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0x54ebb5ec, offset 4
[  239.045519] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0xcdffe7a2, offset 4
[  239.275044] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  239.284355] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0x1d36f59d, offset 4
[  239.292541] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0xaef3c1e9, offset 4
[  239.518996] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  239.528300] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0x2881912, offset 4
[  239.536413] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0x5638f7e2, offset 4

And network throughput ends up being pretty bursty and slow with a
overall throughput of at best ~30kB/s.

Looking through the commits since the v4.1 kernel where we didn't see
this, I narrowed the regression down, and reverting the following two
commits seems to avoid the problem:

6a570814cd430fa5ef4f278e8046dcf12ee63f13 asix: Continue processing URB
if no RX netdev buffer
3f30b158eba5c604b6e0870027eef5d19fc9271d asix: On RX avoid creating
bad Ethernet frames

With these reverted, we don't see all the error messages, and we see
better ~1.1MB/s throughput (I've got a mouse plugged in, so I think
the usb host is only running at "full-speed" mode here).


I don't think the first one is giving you problems (except as triggered 
by the second) but I had concerns about the second myself (and emailed 
the author off-list, but received no reply), and we did not take that 
commit for our own product.


Specifically, the second change, 3f30... (original patch: 
https://www.mail-archive.com/netdev@vger.kernel.org/msg80720.html) (1) 
appears to do the exact opposite of what it claims, i.e., instead of 
"resync if this looks like a header", it does "resync if this does NOT 
look like a (packet) header", where "looks like a header" means "bits 
0-10 (size) are equal to the bitwise-NOT of bits 16-26", and (2) can 
happen by coincidence for 1/2048 32-bit values starting a continuation 
URB (easy to hit dealing with large volumes of video data as we were). 
It appears to expect the header for every URB whereas the rest of the 
code at least expects it only once per network packet (look at following 
code that only reads it for remaining == 0).


So that change made no sense to me, but I don't have significant kernel 
dev experience. Effectively it will drop/truncate every (2047/2048) 
split (longer than an URB) packet, and report an error for the second 
URB and then again for treating said second URB as a first URB for a 
packet. I would expect your problems will go away just removing the 
second change. You could also change the != to == in "if (size != ...)" 
but then you'd still have 1/2048 (depending on data patterns) false 
positives.



This worries me some, as the patches seem to describe trying to fix
the issue they seem to cause, so I suspect a revert isn't the correct
solution, but am not sure why we're having such trouble and the patch
authors did not.  I'd be happy to do further testing of patches if
folks have any ideas.

Originally Reported-by: Yongqin Liu 

thanks
-john


David

--
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: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Alan Stern
On Tue, 3 May 2016, Susmita/Rajib Bandopadhyay wrote:

> Dear Sirs,
> I had already tried
> $ mkdir /media/sim
> $ sudo mount /dev/sdc /media/sim
> But this doesn't work, as I don't know the filesystem of the card.
> Sirs, I took up the matter with the Debian & the Knoppix communities,
> as mentioned below:
>  href='http://forums.debian.net/viewtopic.php?f=7=128156=30=c650525e3dc6562592e29ab20c442e0d'>How
> to back up data of SIM Card using its Reader?
>  href='http://knoppix.net/forum/threads/31824-How-to-back-up-SIM-Card-data-with-its-Reader?p=133625#post133625'>How
> to back up SIM Card & data with its Reader?

We need more information about what happens when you plug in the card 
reader.  Please try this experiment:

Before plugging the card reader into the USB port:

sudo modprobe usbmon
sudo cat /sys/kernel/debug/usb/usbmon/1u >/tmp/usbmon.out

Then insert the card into the reader and plug the reader in.  Wait 2
minutes and then type ^C to kill the "cat" program.

Post the contents of /tmp/usbmon.out so we can see what happened.

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: [PATCHv2] musb_host: fix lockup on rxcsr_h_error

2016-05-03 Thread Bin Liu
Hi,

On Tue, May 03, 2016 at 04:25:58PM +0200, Yegor Yefremov wrote:
> On Tue, May 3, 2016 at 3:48 PM, Bin Liu  wrote:
> > Hi,
> >
> > On Tue, May 03, 2016 at 12:03:52PM +0200, Yegor Yefremov wrote:
> >> On Thu, Apr 28, 2016 at 4:37 PM, Bin Liu  wrote:
> >> > Hi,
> >> >
> >> > On Thu, Apr 28, 2016 at 09:51:37AM +0300, Maxim Uvarov wrote:
> >> >
> >> > [snip]
> >> >
> >> >> Hello Bin,
> >> >>
> >> >> yes, it also works with that reset and go to finish:
> >> >>
> >> >> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> >> >> index c3d5fc9..8cd98e7 100644
> >> >> --- a/drivers/usb/musb/musb_host.c
> >> >> +++ b/drivers/usb/musb/musb_host.c
> >> >> @@ -1599,6 +1599,10 @@ void musb_host_rx(struct musb *musb, u8 epnum)
> >> >> status = -EPROTO;
> >> >> musb_writeb(epio, MUSB_RXINTERVAL, 0);
> >> >>
> >> >> +   rx_csr &= ~MUSB_RXCSR_H_ERROR;
> >> >> +   musb_writew(epio, MUSB_RXCSR, rx_csr);
> >> >> +
> >> >> +   goto finish;
> >> >> } else if (rx_csr & MUSB_RXCSR_DATAERROR) {
> >> >>
> >> >> if (USB_ENDPOINT_XFER_ISOC != qh->type) {
> >> >>
> >> >
> >> > Thanks for testing it.
> >>
> >> Have tested your patch and now both FT4232 and Huawei don't freeze on 
> >> removal.
> >>
> >> Bin, Max thanks for fixing this issue.
> >>
> >> Tested-by: Yegor Yefremov 
> >
> > Thanks for testing.
> >
> > Can you please test the patch [1] instead? I'd like to use it as the
> > fix.
> >
> > [1] http://marc.info/?l=linux-usb=146222355213935=2
> 
> The patch behaves the same as the previous one.
> 
> Kernel: 4.6-rc6

Thanks for testing. I will add your Tested-by.

Regards,
-Bin.
--
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: misc: usbtest: fix pattern tests for scatterlists.

2016-05-03 Thread Alan Stern
On Tue, 3 May 2016, Felipe Balbi wrote:

> Hi,
> 
> Alan Stern  writes:
> > On Mon, 2 May 2016, Mathias Nyman wrote:
> >
> >> The current implemenentation restart the sent pattern for each entry in
> >> the sg list. The receiving end expects a continuous pattern, and test
> >> will fail unless scatterilst entries happen to be aligned with the
> >> pattern
> >
> > Ah.  We didn't spot this earlier because for non-xHCI controllers, the 
> > scatterlist entries _must_ be aligned with the maxpacket size and 
> > therefore with the pattern.
> 
> interesting. We actually found a similar issue with XHCI. scatterlist
> has to be aligned to wMaxPacketSize but only before a link TRB. Mathias
> has been working on a solution which involves memcpy()ing enough bytes
> to align to wMaxPacketSize before the link TRB (it's very infrequent as
> we have 256 TRBs per segment on XHCI), but if you know of a nicer way,
> we're all ears :-)

You should be able to handle this without memcpy'ing anything.

If the individual scatterlist entries are large enough, you can simply
end the ring segment early.  Either put a link TRB before the physical
segment end, at the last point where the cumulative transfer size is
divisible by maxpacket, or else fill out from there to the end of the
segment with no-op TRBs.  Common case: Each scatterlist entry is a
multiple of 512 bytes and the maxpacket size is 1024.  Then you either
have to split the last entry in two or move it completely into the next
ring segment.

This approach doesn't work quite as well if the scatterlist entries are
very small.  For instance, if they are 8 bytes or smaller then you
might have to fill out the segment with 128 or more no-op TRBs, which
is not so good if the segment can hold only 256 TRBs.  I suppose we
could simply rule this out by requiring SG transfers to have a minimum
entry size of 128 bytes, or something like that.

> /me goes dig EHCI

Not sure that will help.  The same issue could arise there, if the 
scatterlist entries were smaller than 512 bytes.  The driver doesn't 
handle this case properly, but it works out okay because the case never 
comes up.

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: [PATCHv2] musb_host: fix lockup on rxcsr_h_error

2016-05-03 Thread Yegor Yefremov
On Tue, May 3, 2016 at 3:48 PM, Bin Liu  wrote:
> Hi,
>
> On Tue, May 03, 2016 at 12:03:52PM +0200, Yegor Yefremov wrote:
>> On Thu, Apr 28, 2016 at 4:37 PM, Bin Liu  wrote:
>> > Hi,
>> >
>> > On Thu, Apr 28, 2016 at 09:51:37AM +0300, Maxim Uvarov wrote:
>> >
>> > [snip]
>> >
>> >> Hello Bin,
>> >>
>> >> yes, it also works with that reset and go to finish:
>> >>
>> >> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
>> >> index c3d5fc9..8cd98e7 100644
>> >> --- a/drivers/usb/musb/musb_host.c
>> >> +++ b/drivers/usb/musb/musb_host.c
>> >> @@ -1599,6 +1599,10 @@ void musb_host_rx(struct musb *musb, u8 epnum)
>> >> status = -EPROTO;
>> >> musb_writeb(epio, MUSB_RXINTERVAL, 0);
>> >>
>> >> +   rx_csr &= ~MUSB_RXCSR_H_ERROR;
>> >> +   musb_writew(epio, MUSB_RXCSR, rx_csr);
>> >> +
>> >> +   goto finish;
>> >> } else if (rx_csr & MUSB_RXCSR_DATAERROR) {
>> >>
>> >> if (USB_ENDPOINT_XFER_ISOC != qh->type) {
>> >>
>> >
>> > Thanks for testing it.
>>
>> Have tested your patch and now both FT4232 and Huawei don't freeze on 
>> removal.
>>
>> Bin, Max thanks for fixing this issue.
>>
>> Tested-by: Yegor Yefremov 
>
> Thanks for testing.
>
> Can you please test the patch [1] instead? I'd like to use it as the
> fix.
>
> [1] http://marc.info/?l=linux-usb=146222355213935=2

The patch behaves the same as the previous one.

Kernel: 4.6-rc6

Yegor
--
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: [PATCHv2] musb_host: fix lockup on rxcsr_h_error

2016-05-03 Thread Bin Liu
Hi,

On Tue, May 03, 2016 at 12:03:52PM +0200, Yegor Yefremov wrote:
> On Thu, Apr 28, 2016 at 4:37 PM, Bin Liu  wrote:
> > Hi,
> >
> > On Thu, Apr 28, 2016 at 09:51:37AM +0300, Maxim Uvarov wrote:
> >
> > [snip]
> >
> >> Hello Bin,
> >>
> >> yes, it also works with that reset and go to finish:
> >>
> >> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> >> index c3d5fc9..8cd98e7 100644
> >> --- a/drivers/usb/musb/musb_host.c
> >> +++ b/drivers/usb/musb/musb_host.c
> >> @@ -1599,6 +1599,10 @@ void musb_host_rx(struct musb *musb, u8 epnum)
> >> status = -EPROTO;
> >> musb_writeb(epio, MUSB_RXINTERVAL, 0);
> >>
> >> +   rx_csr &= ~MUSB_RXCSR_H_ERROR;
> >> +   musb_writew(epio, MUSB_RXCSR, rx_csr);
> >> +
> >> +   goto finish;
> >> } else if (rx_csr & MUSB_RXCSR_DATAERROR) {
> >>
> >> if (USB_ENDPOINT_XFER_ISOC != qh->type) {
> >>
> >
> > Thanks for testing it.
> 
> Have tested your patch and now both FT4232 and Huawei don't freeze on removal.
> 
> Bin, Max thanks for fixing this issue.
> 
> Tested-by: Yegor Yefremov 

Thanks for testing.

Can you please test the patch [1] instead? I'd like to use it as the
fix.

Regards,
-Bin.

[1] http://marc.info/?l=linux-usb=146222355213935=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] usb: xhci: ring: fix off-by-one error

2016-05-03 Thread Felipe Balbi

Hi,

Mathias Nyman  writes:
>> Mathias Nyman  writes:
>>> On 03.05.2016 13:30, Felipe Balbi wrote:
 When trying to access our last TRB, XHCI was
 actually reading memory outside of the TRB
 array/ring due to an off-by-one error.

 This patch fixes that error and has the side effect
 of also fixing some rare situations where long mass
 storage transfers would timeout and XHCI would reset
 the mass storage device before continuing.

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

 diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
 index 6c41dbbf9f2f..ba610a72c396 100644
 --- a/drivers/usb/host/xhci-ring.c
 +++ b/drivers/usb/host/xhci-ring.c
 @@ -95,7 +95,7 @@ static bool last_trb_on_last_seg(struct xhci_hcd *xhci, 
 struct xhci_ring *ring,
struct xhci_segment *seg, union xhci_trb *trb)
{
if (ring == xhci->event_ring)
 -  return (trb == >trbs[TRBS_PER_SEGMENT]) &&
 +  return (trb == >trbs[TRBS_PER_SEGMENT - 1]) &&
(seg->next == xhci->event_ring->first_seg);
else
return le32_to_cpu(trb->link.control) & LINK_TOGGLE;
 @@ -109,7 +109,7 @@ static int last_trb(struct xhci_hcd *xhci, struct 
 xhci_ring *ring,
struct xhci_segment *seg, union xhci_trb *trb)
{
if (ring == xhci->event_ring)
 -  return trb == >trbs[TRBS_PER_SEGMENT];
 +  return trb == >trbs[TRBS_PER_SEGMENT - 1];
else
return TRB_TYPE_LINK_LE32(trb->link.control);
}

>>>
>>> Thanks, this needs to be fixed, but there are some changes needed to 
>>> inc_enq()
>>> as well together with this fix.
>>> Otherwise the last TRB of a event ring won't be used
>>
>> Any idea how a problem due to this could be triggered ? So far I can't
>> get any failures. BTW, the last TRB *will* be used by the HW anyway, no?
>> A truer argument would be that Linux XHCI driver might miss handling
>> that event. I still can't get this to fail. It seems to me that it's
>> pretty much impossible for us to miss an event.
>>
>> If we consider what happens from the time an IRQ fires, we will have the
>> following sequence of events:
>>
>> MSI/MSI-X/normal IRQ asserted
>>   xhci_irq()
>>while(xhci_handle_event(xhci) > 0);
>> event = xhci->event_ring->dequeue;
>>  /* handle the event given its type */
>>   if (update_ptrs) inc_deq();
>>ring->deq_updates++;
>>do {
>> if (last_trb(xhci, ring, ring->deq_seg, ring->dequeue))
>>  if (ring->type == EVENT && last_trb_on_last_seg())
>>ring-cycle_state ^= 1;
>>   ring->deq_seg = ring->deq_seg->next;
>>   ring->dequeue = ring->deq_seg->trbs;
>>  else
>>   ring->dequeue++
>>} while (last_trb())
>>
>
> event ring:
> trb[0]
> ...
> trb[254]
> trb[255] = the last trb on the ring, still used for events as event
> ring has no link trb.
>
> One scenario where the dequeue pointer, both the actual hw one, and
> the sw one is at trb[254]
>
> xhci hw writes an event at trb[254], event ring dequeue is at trb[254]
>   - interrupt, we handle the event at dequeue trb[254]
>   - call inc_deq(), last_trb(trb[254]) check is false so instead we increase 
> the dequeue.
>   - dequeue is now at trb[255], while(last_trb(trb[255]) will now be true, so 
> go back to "do{".
>   - last_trb(trb[255]) check is now true, so we jump to next segment.
>
> so now the driver thinks the dequeue pointer is at the next segment,
> but hw is going to write the next event at trb[255] which the driver
> is not going to notice -> drive misses one event.

Good point, but it seems to me the problem is that do {} while() loop on
inc_deq(), how can we have more than one last TRB ? Granted, that check
is different for non-EVENT rings (which checks for a link TRB instead -
also wrong check as Link might not be last).

Also, I haven't managed to trigger any failures from this but I have to
fix my commit log as the lock-up -> timeout -> Reset on the mass storage
device still happens even with $subject, albeit not as frequently.

Anyway, seems like below is also needed (maybe as patch 1 on a series of
two patches ?)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ba610a72c396..6f991b7c4249 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -153,24 +153,21 @@ static void inc_deq(struct xhci_hcd *xhci, struct 
xhci_ring *ring)
!last_trb(xhci, ring, ring->deq_seg, ring->dequeue))
ring->num_trbs_free++;
 
-   do {
+   ring->dequeue++;
+
+   if 

Re: [PATCH] usb: xhci: ring: fix off-by-one error

2016-05-03 Thread Mathias Nyman

On 03.05.2016 14:55, Felipe Balbi wrote:


Hi,

Mathias Nyman  writes:

On 03.05.2016 13:30, Felipe Balbi wrote:

When trying to access our last TRB, XHCI was
actually reading memory outside of the TRB
array/ring due to an off-by-one error.

This patch fixes that error and has the side effect
of also fixing some rare situations where long mass
storage transfers would timeout and XHCI would reset
the mass storage device before continuing.

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

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 6c41dbbf9f2f..ba610a72c396 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -95,7 +95,7 @@ static bool last_trb_on_last_seg(struct xhci_hcd *xhci, 
struct xhci_ring *ring,
struct xhci_segment *seg, union xhci_trb *trb)
   {
if (ring == xhci->event_ring)
-   return (trb == >trbs[TRBS_PER_SEGMENT]) &&
+   return (trb == >trbs[TRBS_PER_SEGMENT - 1]) &&
(seg->next == xhci->event_ring->first_seg);
else
return le32_to_cpu(trb->link.control) & LINK_TOGGLE;
@@ -109,7 +109,7 @@ static int last_trb(struct xhci_hcd *xhci, struct xhci_ring 
*ring,
struct xhci_segment *seg, union xhci_trb *trb)
   {
if (ring == xhci->event_ring)
-   return trb == >trbs[TRBS_PER_SEGMENT];
+   return trb == >trbs[TRBS_PER_SEGMENT - 1];
else
return TRB_TYPE_LINK_LE32(trb->link.control);
   }



Thanks, this needs to be fixed, but there are some changes needed to inc_enq()
as well together with this fix.
Otherwise the last TRB of a event ring won't be used


Any idea how a problem due to this could be triggered ? So far I can't
get any failures. BTW, the last TRB *will* be used by the HW anyway, no?
A truer argument would be that Linux XHCI driver might miss handling
that event. I still can't get this to fail. It seems to me that it's
pretty much impossible for us to miss an event.

If we consider what happens from the time an IRQ fires, we will have the
following sequence of events:

MSI/MSI-X/normal IRQ asserted
  xhci_irq()
   while(xhci_handle_event(xhci) > 0);
event = xhci->event_ring->dequeue;
 /* handle the event given its type */
  if (update_ptrs) inc_deq();
   ring->deq_updates++;
   do {
if (last_trb(xhci, ring, ring->deq_seg, ring->dequeue))
 if (ring->type == EVENT && last_trb_on_last_seg())
   ring-cycle_state ^= 1;
  ring->deq_seg = ring->deq_seg->next;
  ring->dequeue = ring->deq_seg->trbs;
 else
  ring->dequeue++
   } while (last_trb())



event ring:
trb[0]
...
trb[254]
trb[255] = the last trb on the ring, still used for events as event ring has no 
link trb.

One scenario where the dequeue pointer, both the actual hw one, and the sw one 
is at trb[254]

xhci hw writes an event at trb[254], event ring dequeue is at trb[254]
 - interrupt, we handle the event at dequeue trb[254]
 - call inc_deq(), last_trb(trb[254]) check is false so instead we increase the 
dequeue.
 - dequeue is now at trb[255], while(last_trb(trb[255]) will now be true, so go back to 
"do{".
 - last_trb(trb[255]) check is now true, so we jump to next segment.

so now the driver thinks the dequeue pointer is at the next segment, but hw is 
going to write
the next event at trb[255] which the driver is not going to notice -> drive 
misses one event.


With the patch above, here's (again, to make it easier to comment) what
last_trb() and last_trb_on_last_seg() look like:


static bool last_trb_on_last_seg(struct xhci_hcd *xhci, struct xhci_ring *ring,
struct xhci_segment *seg, union xhci_trb *trb)
{
if (ring == xhci->event_ring)
return (trb == >trbs[TRBS_PER_SEGMENT - 1]) &&
(seg->next == xhci->event_ring->first_seg);
else
return le32_to_cpu(trb->link.control) & LINK_TOGGLE;
}


for a ring segment this evaluates to:

return trb == last_trb && seg->next == xhci->event_ring->first_seg;

IOW:

 if this is the last_trb on current segment and is also the last
 trb on the last segment, return true.

in fact, trb == >trbs[TRBS_PER_SEGMENT - 1] is a redundant check.


static int last_trb(struct xhci_hcd *xhci, struct xhci_ring *ring,
struct xhci_segment *seg, union xhci_trb *trb)
{
if (ring == xhci->event_ring)
return trb == >trbs[TRBS_PER_SEGMENT - 1];
else
return TRB_TYPE_LINK_LE32(trb->link.control);
}


likewise, for an event_ring, this turns out to be:

return trb == last_trb;

IOW, before this patch, we were potentially checking memory outside of
the TRB ring which tells me that, unless we were lucky, 

Re: [EXT] RE: [PATCH v2 2/3] USB: serial: cp210x: Added comments to CRTSCTS flag code.

2016-05-03 Thread Johan Hovold
On Tue, May 03, 2016 at 12:11:53PM +, Konstantin Shkolnyy wrote:
> > -Original Message-
> > From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> > ow...@vger.kernel.org] On Behalf Of David Laight
> > Sent: Tuesday, May 03, 2016 04:44
> > To: 'Konstantin Shkolnyy'; jo...@kernel.org
> > Cc: linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org
> > Subject: [EXT] RE: [PATCH v2 2/3] USB: serial: cp210x: Added comments to
> > CRTSCTS flag code.
> > 
> > From: Konstantin Shkolnyy
> > > Sent: 30 April 2016 03:22
> > > Replaced magic numbers used in the CRTSCTS flag code with symbolic
> > names
> > > from the chip specification.
> > >
> > > Signed-off-by: Konstantin Shkolnyy 
> > > ---
> > > Changes in v2:
> > > Improved CRTSCTS fix based on feedback. Dropped get_termios error
> > handling.
> > >
> > >  drivers/usb/serial/cp210x.c | 93
> > +
> > >  1 file changed, 69 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> > ...
> > > +/* cp210x_flow_ctl::ulControlHandshake */
> > > +#define SERIAL_DTR_MASK  0x0003
> > > +#define SERIAL_CTS_HANDSHAKE 0x0008
> > > +#define SERIAL_DSR_HANDSHAKE 0x0010
> > > +#define SERIAL_DCD_HANDSHAKE 0x0020
> > > +#define SERIAL_DSR_SENSITIVITY   0x0040
> > ...
> > 
> > I'd have thought the names ought to start CP210X_
> 
> These names are inherited from the Labs chip spec.

Yes, but it's still a good idea to add a CP210X_ prefix to avoid any
confusion with the serial-core defines.

Thanks,
Johan
--
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: [EXT] RE: [PATCH v2 2/3] USB: serial: cp210x: Added comments to CRTSCTS flag code.

2016-05-03 Thread Konstantin Shkolnyy
> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of David Laight
> Sent: Tuesday, May 03, 2016 04:44
> To: 'Konstantin Shkolnyy'; jo...@kernel.org
> Cc: linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: [EXT] RE: [PATCH v2 2/3] USB: serial: cp210x: Added comments to
> CRTSCTS flag code.
> 
> From: Konstantin Shkolnyy
> > Sent: 30 April 2016 03:22
> > Replaced magic numbers used in the CRTSCTS flag code with symbolic
> names
> > from the chip specification.
> >
> > Signed-off-by: Konstantin Shkolnyy 
> > ---
> > Changes in v2:
> > Improved CRTSCTS fix based on feedback. Dropped get_termios error
> handling.
> >
> >  drivers/usb/serial/cp210x.c | 93
> +
> >  1 file changed, 69 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> ...
> > +/* cp210x_flow_ctl::ulControlHandshake */
> > +#define SERIAL_DTR_MASK0x0003
> > +#define SERIAL_CTS_HANDSHAKE   0x0008
> > +#define SERIAL_DSR_HANDSHAKE   0x0010
> > +#define SERIAL_DCD_HANDSHAKE   0x0020
> > +#define SERIAL_DSR_SENSITIVITY 0x0040
> ...
> 
> I'd have thought the names ought to start CP210X_
> 

These names are inherited from the Labs chip spec.
--
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: xhci: ring: fix off-by-one error

2016-05-03 Thread Felipe Balbi

Hi,

Mathias Nyman  writes:
> On 03.05.2016 13:30, Felipe Balbi wrote:
>> When trying to access our last TRB, XHCI was
>> actually reading memory outside of the TRB
>> array/ring due to an off-by-one error.
>>
>> This patch fixes that error and has the side effect
>> of also fixing some rare situations where long mass
>> storage transfers would timeout and XHCI would reset
>> the mass storage device before continuing.
>>
>> Signed-off-by: Felipe Balbi 
>> ---
>>   drivers/usb/host/xhci-ring.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>> index 6c41dbbf9f2f..ba610a72c396 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -95,7 +95,7 @@ static bool last_trb_on_last_seg(struct xhci_hcd *xhci, 
>> struct xhci_ring *ring,
>>  struct xhci_segment *seg, union xhci_trb *trb)
>>   {
>>  if (ring == xhci->event_ring)
>> -return (trb == >trbs[TRBS_PER_SEGMENT]) &&
>> +return (trb == >trbs[TRBS_PER_SEGMENT - 1]) &&
>>  (seg->next == xhci->event_ring->first_seg);
>>  else
>>  return le32_to_cpu(trb->link.control) & LINK_TOGGLE;
>> @@ -109,7 +109,7 @@ static int last_trb(struct xhci_hcd *xhci, struct 
>> xhci_ring *ring,
>>  struct xhci_segment *seg, union xhci_trb *trb)
>>   {
>>  if (ring == xhci->event_ring)
>> -return trb == >trbs[TRBS_PER_SEGMENT];
>> +return trb == >trbs[TRBS_PER_SEGMENT - 1];
>>  else
>>  return TRB_TYPE_LINK_LE32(trb->link.control);
>>   }
>>
>
> Thanks, this needs to be fixed, but there are some changes needed to inc_enq()
> as well together with this fix.
> Otherwise the last TRB of a event ring won't be used

Any idea how a problem due to this could be triggered ? So far I can't
get any failures. BTW, the last TRB *will* be used by the HW anyway, no?
A truer argument would be that Linux XHCI driver might miss handling
that event. I still can't get this to fail. It seems to me that it's
pretty much impossible for us to miss an event.

If we consider what happens from the time an IRQ fires, we will have the
following sequence of events:

MSI/MSI-X/normal IRQ asserted
 xhci_irq()
  while(xhci_handle_event(xhci) > 0);
   event = xhci->event_ring->dequeue;
/* handle the event given its type */
 if (update_ptrs) inc_deq();
  ring->deq_updates++;
  do {
   if (last_trb(xhci, ring, ring->deq_seg, ring->dequeue))
if (ring->type == EVENT && last_trb_on_last_seg())
  ring-cycle_state ^= 1;
 ring->deq_seg = ring->deq_seg->next;
 ring->dequeue = ring->deq_seg->trbs;
else
 ring->dequeue++
  } while (last_trb())
 
With the patch above, here's (again, to make it easier to comment) what
last_trb() and last_trb_on_last_seg() look like:

> static bool last_trb_on_last_seg(struct xhci_hcd *xhci, struct xhci_ring 
> *ring,
>   struct xhci_segment *seg, union xhci_trb *trb)
> {
>   if (ring == xhci->event_ring)
>   return (trb == >trbs[TRBS_PER_SEGMENT - 1]) &&
>   (seg->next == xhci->event_ring->first_seg);
>   else
>   return le32_to_cpu(trb->link.control) & LINK_TOGGLE;
> }

for a ring segment this evaluates to:

return trb == last_trb && seg->next == xhci->event_ring->first_seg;

IOW:

if this is the last_trb on current segment and is also the last
trb on the last segment, return true.

in fact, trb == >trbs[TRBS_PER_SEGMENT - 1] is a redundant check.

> static int last_trb(struct xhci_hcd *xhci, struct xhci_ring *ring,
>   struct xhci_segment *seg, union xhci_trb *trb)
> {
>   if (ring == xhci->event_ring)
>   return trb == >trbs[TRBS_PER_SEGMENT - 1];
>   else
>   return TRB_TYPE_LINK_LE32(trb->link.control);
> }

likewise, for an event_ring, this turns out to be:

return trb == last_trb;

IOW, before this patch, we were potentially checking memory outside of
the TRB ring which tells me that, unless we were lucky, last_trb() and
last_trb_on_last_seg() would always evaluate to false.

Oh; and, BTW, a further cleanup would be:

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ba610a72c396..e3a647c73323 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -88,19 +88,6 @@ dma_addr_t xhci_trb_virt_to_dma(struct xhci_segment *seg,
return seg->dma + (segment_offset * sizeof(*trb));
 }
 
-/* Does this link TRB point to the first segment in a ring,
- * or was the previous TRB the last TRB on the last segment in the ERST?
- */
-static bool last_trb_on_last_seg(struct xhci_hcd *xhci, struct xhci_ring *ring,
-   struct xhci_segment *seg, union xhci_trb *trb)
-{
-   if (ring == xhci->event_ring)
- 

Re: [PATCH v7 1/7] regulator: fixed: add support for ACPI interface

2016-05-03 Thread Mark Brown
On Tue, May 03, 2016 at 09:43:58AM +0800, Lu Baolu wrote:
> On 05/02/2016 07:00 PM, Mark Brown wrote:
> > On Fri, Apr 29, 2016 at 02:26:32PM +0800, Lu Baolu wrote:

> >> +  gpiod = gpiod_get(dev, "vbus_en", GPIOD_ASIS);
> >> +  if (IS_ERR(gpiod))
> >> +  return PTR_ERR(gpiod);

> > This is clearly an inappropriate name for the signal in generic code,
> > it's specific to your use case.

> I will change the gpio name to "gpio". Is that okay?

Yes, that looks good (and lines up with DT so hopefully the code can be
shared).


signature.asc
Description: PGP signature


Re: [PATCH] usb: serial: ti_usb_3410_5052: add MOXA UPORT 11x0 support

2016-05-03 Thread Mathieu OTHACEHE

> No, I was trying to say that the we should not attempt to load a
> firmware on the "ti_usb-v%04x-p%04x.fw" format before loading the moxa
> firmware.

For MTS devices (mts_*.fw) and for devices using generic firmware (ti_3410.fw
and ti_5052.fw), ti_usb-v%04x-p%04x.fw loading is already failing.

So, I can patch the driver to request firmwares in this order :

1. VID dependant (MTS and MOXA now)
2. ti_usb-v%04x-p%04x.fw format
3. Generic firmware

But, for generic firmware users, ti_usb-v%04x-p%04x.fw loading will
still always fail ...

Or we can get rid of ti_usb-v%04x-p%04x.fw loading because no one has
defined a firmware with this format in linux-firmware repository ?

> No, that's just integer-division with rounding. Remember that the
> divisions above are integer divisions so the results are actually 96 and
> 8 as expected.

You're right, sorry !

Thank you,

Mathieu
--
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: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Susmita/Rajib Bandopadhyay
On 03/05/2016, Lars Melin  wrote:
> You are jumping to big conclusions by assuming that your device presents
> a mountable file system. ...

Sir, I don't know really, but only because I know the community shall
guide me, and
(1)  I have my faith firmly upon the FOSS community, that overcame the
very restrictions which I too continued to face till the present (and
in future too) and made available drivers for us, when they were/are
not provided by the OEMs; &
(2)  The SIM card is read by my cellular phone, and the OEM provides a
software to read the data stored.
Regards, Sir!
--
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: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Lars Melin

On 2016-05-03 18:06, Susmita/Rajib Bandopadhyay wrote:



Since I don't have a winDoze system I can't use the software.



This list handles linux usb drivers, ie internal linux software.
Userland software for talking to your device should be provided by the mfgr.
You are jumping to big conclusions by assuming that your device presents 
a mountable file system.


/Lars

--
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: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Susmita/Rajib Bandopadhyay
From: Bjørn Mork 
Date: Tue, 03 May 2016 13:17:11 +0200
Subject: Re: How to read & back up data of SIM Card using its Reader?
To: Susmita/Rajib Bandopadhyay 
Cc: Lars Melin , linux-usb@vger.kernel.org

"Susmita/Rajib Bandopadhyay"  writes:
> I again reiterate the SIM Card reader's Specifications in ebay.in
OK, so let's play that game them.

I am not here to play the "Winning the argument" game with a young man
maybe of my kid's age hypercharged to prove his arguments.  I won't
choose to participate in this ego battle. For me, it was just clearing
a confusion.

I am here to find a solution: to have the SIM Card Reader used to read
and back up my SIM card.

It is true that the ebay.in seller won't support Linux. There is no
card reader in India that is supported by Linux. But I have faith in
my competent FOSS community members in overcoming restrictions.

I would be ready to cheer you in helping us overcome this restriction.
I will be ready to pay for this FOSS endeavour.

Rest is just ignored.

Regards
--
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: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Bjørn Mork
"Susmita/Rajib Bandopadhyay"  writes:

> I again reiterate the SIM Card reader's Specifications in ebay.in

OK, so let's play that game them.

> OS System-Windows 7/ 98SE / Me / 2000 / XP / Vista

No Linux support. The eBay seller said so.  You should believe him.
eBay sellers can always be trusted.


Bjørn
--
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: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Susmita/Rajib Bandopadhyay
I again reiterate the SIM Card reader's Specifications in ebay.in
because of the confusion created by Prof.Bjørn Mork's opinion.

The product is this:
---
http://www.ebay.in/itm/231841478579?euid=063123e1b1d04e44b73872eb30670836=1
(Copied from ebay.in) Item Specifications

Model-MG-008
Interface-USB 2.0
Supports Card Type-SIM Card
Supports Max. Capacity-n/a
Transmission Rate-n/a
OS System-Windows 7/ 98SE / Me / 2000 / XP / Vista
Dimensions-2.28 in x 0.79 in x 0.39 in (5.8 cm x 2.0 cm x 1.0 cm)
Weight-0.25 oz (7 g)
Speed up to 480 Mbps
480 MBPS/SDHC Ready/Windows XP, 7.0,ME,2K,OS X Supported
Material-Plastic
Quantity-1
Colour-White/Black/Green(Choice of Colour on Stock Availbility)
Package Contents -1 x Card reader & 1 x Software CD

The card Reader is accompanied by a software miniCD which helps in reading
the card. The link to the software is here:
https://drive.google.com/open?id=0B0qPAe-7HylCV0VRM01sQkppR3M

The seller adds a troubleshooting notice:
Troubleshooting : If you have any issues with the Compatibility with
any version of Windows, please contact us and we will assist you

Since I don't have a winDoze system I can't use the software.

I hope this helps.
Regards
--
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: xhci: ring: fix off-by-one error

2016-05-03 Thread Mathias Nyman

Hi

On 03.05.2016 13:30, Felipe Balbi wrote:

When trying to access our last TRB, XHCI was
actually reading memory outside of the TRB
array/ring due to an off-by-one error.

This patch fixes that error and has the side effect
of also fixing some rare situations where long mass
storage transfers would timeout and XHCI would reset
the mass storage device before continuing.

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

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 6c41dbbf9f2f..ba610a72c396 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -95,7 +95,7 @@ static bool last_trb_on_last_seg(struct xhci_hcd *xhci, 
struct xhci_ring *ring,
struct xhci_segment *seg, union xhci_trb *trb)
  {
if (ring == xhci->event_ring)
-   return (trb == >trbs[TRBS_PER_SEGMENT]) &&
+   return (trb == >trbs[TRBS_PER_SEGMENT - 1]) &&
(seg->next == xhci->event_ring->first_seg);
else
return le32_to_cpu(trb->link.control) & LINK_TOGGLE;
@@ -109,7 +109,7 @@ static int last_trb(struct xhci_hcd *xhci, struct xhci_ring 
*ring,
struct xhci_segment *seg, union xhci_trb *trb)
  {
if (ring == xhci->event_ring)
-   return trb == >trbs[TRBS_PER_SEGMENT];
+   return trb == >trbs[TRBS_PER_SEGMENT - 1];
else
return TRB_TYPE_LINK_LE32(trb->link.control);
  }



Thanks, this needs to be fixed, but there are some changes needed to inc_enq()
as well together with this fix.
Otherwise the last TRB of a event ring won't be used

-Mathias  


--
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: leave LPM alone if possible when binding/unbinding interface drivers

2016-05-03 Thread Mathias Nyman

On 02.05.2016 18:18, Alan Stern wrote:

On Mon, 2 May 2016, Mathias Nyman wrote:


On 29.04.2016 22:25, Alan Stern wrote:

When a USB driver is bound to an interface (either through probing or
by claiming it) or is unbound from an interface, the USB core always
disables Link Power Management during the transition and then
re-enables it afterward.  The reason is because the driver might want
to prevent hub-initiated link power transitions, in which case the HCD
would have to recalculate the various LPM parameters.  This
recalculation takes place when LPM is re-enabled and the new
parameters are sent to the device and its parent hub.

However, if the driver does not want to prevent hub-initiated link
power transitions then none of this work is necessary.  The parameters
don't need to be recalculated, and LPM doesn't need to be disabled and
re-enabled.

It turns out that disabling and enabling LPM can be time-consuming,
enough so that it interferes with user programs that want to claim and
release interfaces rapidly via usbfs.  Since the usbfs kernel driver
doesn't set the disable_hub_initiated_lpm flag, we can speed things up
and get the user programs to work by leaving LPM alone whenever the
flag isn't set.

And while we're improving the way disable_hub_initiated_lpm gets used,
let's also fix its kerneldoc.

Signed-off-by: Alan Stern 
Tested-by: Matthew Giassa 
CC: Mathias Nyman 
CC: 

---

Mathias, can you check that this is right?  If the driver's
disable_hub_initiated_lpm flag isn't set then binding or unbinding it
shouldn't require any changes to the LPM parameters.  The flag gets
used in only one place, in xhci_calculate_lpm_timeout(), and if it
isn't set then then result would be the same as if the driver weren't
bound.

Thanks.


It looks like xhci driver calculates the u1 and u2 timeout values for
ports based on udev->u1_params.sel * x (where x depends on endpoint type).

It loops through all active interfaces all endpoints, and picks the worst
(longest) timeout as the timeout value.

In this sense if a interface drive disappears or appears it could be possible
that the timeout values change, even if the xhci_calculate_lpm_timeout() is
not set.


I don't understand.  The calculation below doesn't care whether the
interface is bound to a driver, does it?  It looks at all the endpoints
in all the current altsettings, whether they are bound or not.

Or have I misunderstood something?



No, you're right.
checking for a driver (udev->actconfig->interface[i]->dev.driver) while looping
through the interfaces is only done for the disable_hub_initiated_lpm flag.

The rest of the calculations don't care if a driver is bound of not.

So looks good to me.

-Mathias
--
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 v10 4/4] power: wm831x_power: Support USB charger current limit management

2016-05-03 Thread Mark Brown
On Tue, May 03, 2016 at 09:30:48AM +0530, Manish Badarkhe wrote:
> On Tue, May 3, 2016 at 9:00 AM, Baolin Wang  wrote:

> > +static const unsigned int wm831x_usb_limits[] = {
> > +   0,
> > +   2,
> > +   100,
> > +   500,
> > +   900,
> > +   1500,
> > +   1800,
> > +   550,
> > +};

> Just for curiosity, How these current limits are getting decided?
> Can we have some proper defines over here so that it can be grasped easily?

They're in the silicon, it's just a table of values that were put into
the silicon at design time.  The defines would just be TABLE_ENTRY_1 or
whatever.


signature.asc
Description: PGP signature


Re: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions

2016-05-03 Thread Dean Jenkins

On 03/05/16 11:04, Guodong Xu wrote:

On 3 May 2016 at 17:23, Dean Jenkins  wrote:

On 03/05/16 05:55, John Stultz wrote:

In testing with HiKey, we found that since commit 3f30b158eba5c60
(asix: On RX avoid creating bad Ethernet frames), we're seeing lots of
noise during network transfers:

[  239.027993] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  239.037310] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0x54ebb5ec, offset 4
[  239.045519] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0xcdffe7a2, offset 4
[  239.275044] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  239.284355] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0x1d36f59d, offset 4
[  239.292541] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0xaef3c1e9, offset 4
[  239.518996] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  239.528300] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0x2881912, offset 4
[  239.536413] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0x5638f7e2, offset 4


And network throughput ends up being pretty bursty and slow with a
overall throughput of at best ~30kB/s.

Looking through the commits since the v4.1 kernel where we didn't see
this, I narrowed the regression down, and reverting the following two
commits seems to avoid the problem:

6a570814cd430fa5ef4f278e8046dcf12ee63f13 asix: Continue processing URB
if no RX netdev buffer
3f30b158eba5c604b6e0870027eef5d19fc9271d asix: On RX avoid creating
bad Ethernet frames

With these reverted, we don't see all the error messages, and we see
better ~1.1MB/s throughput (I've got a mouse plugged in, so I think
the usb host is only running at "full-speed" mode here).

This worries me some, as the patches seem to describe trying to fix
the issue they seem to cause, so I suspect a revert isn't the correct
solution, but am not sure why we're having such trouble and the patch
authors did not.  I'd be happy to do further testing of patches if
folks have any ideas.

Originally Reported-by: Yongqin Liu 

thanks
-john

Hi John,

Some ASIX chipsets span the Ethernet frame over consecutive URBs which
requires successful transfer of 2 URBs.

This means states of a previous URB influences the processing of the next
URB including a dropped URB (causes a discontinuity in the data stream). In
other words synchronisation of the in-band 32-bit header word needs to be
tracked between URBs. Some ASIX chipsets allow the in-band 32-bit header
word to be no longer fixed to the start of the URB buffer so it moves to any
position within the URB buffer.

I understand your point of suggesting it is a "regression" for your device
but the driver was broken for DUB-E100 C1 (small black USB device). So you
cannot revert the commits as this would break DUB-E100 C1 (small black USB
device).


6a570814cd430fa5ef4f278e8046dcf12ee63f13 asix: Continue processing URB
if no RX netdev buffer

This commit is necessary because it avoids a crash when netdev buffer failed
to be allocated for the 1st URB and the 2nd URB containing a spanned
Ethernet frame is processed. The crash happens because the 2nd URB assumed
that the netdev buffer had been allocated.


3f30b158eba5c604b6e0870027eef5d19fc9271d asix: On RX avoid creating
bad Ethernet frames

This commit is necessary to avoid sending bad Ethernet frames into the IP
stack during loss of synchronisation and to dropping good Ethernet frames.
This commit improves the synchronisation recovery mechanism of the in-band
32-bit header word.

The ASIX USB to Ethernet devices these commits were tested on where DUB-E100
C1 (small black USB device). Embedded ARM based systems were used where
memory resources can run out.

I don't have the chance to look into detail yet. But just a caution,
did you test on ARM 64-bit system or ARM 32-bit? I ask because HiKey
is an ARM 64-bit system. I suggest we should be careful on that. I saw
similar issues when transferring to a 64-bit system in other net
drivers.
We used 32-bit ARM and never tested on 64-bit ARM so I suggest that the 
commits need to be reviewed with 64-bit OS in mind.


Do you have any suggestion on this regard?
Try testing on a Linux PC x86 32-bit OS which has has a kernel 
containing my ASIX commits. This will help to confirm whether the 
failure is related to 32-bit or 64-bit OS. Then try with Linux PC x86 
64-bit OS, this should fail otherwise it points to something specific in 
your ARM 64-bit platform.





It could be that for your USB to Ethernet device that the wrong
configuration settings have been used. In other words the ASIX driver is
flexible to support various variants of the ASIX chipsets. For example, does
your device support Ethernet frames spanning multiple URBs (multiple USB
transfers) ?

Would you please suggest how to find out this information? How can I
change my 

Re: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Susmita/Rajib Bandopadhyay
Sir,
Please also choose to read this line of mine:
>> Since I don't have a winDoze system I can't use the software.

I differ from your opinion. The problem is a linux-related problem
because Doze can do it, but Linux can't.

Just because some inventor develops this device and its driver
software, usually via the open source platform, and then forgets it,
it is usurped by businesses, which then makes the software
proprietary. And we, the FOSS community don't even protect or further
develop the product one of us developed.

I am surprised that you chose to ignore my earlier email's ebay.in
link and the software file that is in the google drive, and pass an
inappropriate judgement : "... What you have is *not* a sim card
reader. This is not a Linux relatedn problem and IMHO highly
inappropriate for this list..."

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


[PATCH] usb: xhci: ring: fix off-by-one error

2016-05-03 Thread Felipe Balbi
When trying to access our last TRB, XHCI was
actually reading memory outside of the TRB
array/ring due to an off-by-one error.

This patch fixes that error and has the side effect
of also fixing some rare situations where long mass
storage transfers would timeout and XHCI would reset
the mass storage device before continuing.

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

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 6c41dbbf9f2f..ba610a72c396 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -95,7 +95,7 @@ static bool last_trb_on_last_seg(struct xhci_hcd *xhci, 
struct xhci_ring *ring,
struct xhci_segment *seg, union xhci_trb *trb)
 {
if (ring == xhci->event_ring)
-   return (trb == >trbs[TRBS_PER_SEGMENT]) &&
+   return (trb == >trbs[TRBS_PER_SEGMENT - 1]) &&
(seg->next == xhci->event_ring->first_seg);
else
return le32_to_cpu(trb->link.control) & LINK_TOGGLE;
@@ -109,7 +109,7 @@ static int last_trb(struct xhci_hcd *xhci, struct xhci_ring 
*ring,
struct xhci_segment *seg, union xhci_trb *trb)
 {
if (ring == xhci->event_ring)
-   return trb == >trbs[TRBS_PER_SEGMENT];
+   return trb == >trbs[TRBS_PER_SEGMENT - 1];
else
return TRB_TYPE_LINK_LE32(trb->link.control);
 }
-- 
2.8.1

--
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: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Bjørn Mork
"Susmita/Rajib Bandopadhyay"  writes:

> The card is accompanied by a software miniCD which helps in reading
> the card.

Please test with that software and then take up on this offer when you
have verified that it doesn't work:

> Troubleshooting : If you have any issues with the Compatibility with
> any version of Windows, please contact us and we will assist you

What you have is *not* a sim card reader. This is not a Linux related
problem and IMHO highly inappropriate for this list.


Bjørn
--
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: misc: usbtest: fix pattern tests for scatterlists.

2016-05-03 Thread Felipe Balbi

Hi

Peter Chen  writes:
> On Tue, May 03, 2016 at 12:31:21PM +0300, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Peter Chen  writes:
>> > On Mon, May 02, 2016 at 11:39:03AM +0300, Mathias Nyman wrote:
>> >> The current implemenentation restart the sent pattern for each entry in
>> >
>> > %s/implemenentation/implementation
>> >
>> >> the sg list. The receiving end expects a continuous pattern, and test
>> >
>> > The f_sourcesink may not expect that, have you tried it?
>> 
>> Use the sources, Luke.
>> 
>
> Sorry, what do you mean ? is it a special device?
>
> At Documentation/usb/gadget-testing.txt, sourcesink uses usbtest to
> test.

Well, f_sourcesink implements the same protocol that an old NEC test
device implemented (IIRC), but anyway, both f_sourcesink and usbtest are
open source and shipped with the kernel. You can check all assumptions
made by both sides.

In any case, the gadget driver *has* to assume a continuous pattern
without any breaks otherwise it needs to agree with the host what the
pattern will be before doing any transactions.

To make the problem slightly clearer, when we pass pattern=1 to *both*
g_zero and usbtest, we're using a mod63 pattern. Buffers should be
initialized like so:

for_each_byte_in_buffer(i)
buf[i] = (i % wMaxPacketSize) % 63;

When using sg lists, the initialization is slightly different:

for_each_sg(sglist)
for_each_byte_in_sg_buffer(i)
buf[i] = (i % wMaxPacketSize) % 63;

From this, it's clear to see that pattern would restart from 0 at every
sg entry. This means that gadget side's checks (which, at least as of
now, always uses linear buffer), would fail.

That's why we need to keep track of
total_size_of_all_previously_initialized_sg_entries, so that we can
continue the pattern where it stopped on previous entry, rather than
restarting from 0 every time.

All this to say that your previous argument:

>> > The f_sourcesink may not expect that, have you tried it?

Is wrong in two details:

a) we have certainly tested. How else would we find the problem ?
b) f_sourcesink *must* make the assumption of a continuous pattern with
   no breaks and/or cuts anywhere.

hope this answers your question.

cheers

-- 
balbi


signature.asc
Description: PGP signature


Re: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Susmita/Rajib Bandopadhyay
Dear Sir,
The card is accompanied by a software miniCD which helps in reading
the card. The link to the software is here:
https://drive.google.com/open?id=0B0qPAe-7HylCV0VRM01sQkppR3M
Since I don't have a winDoze system I can't use the software.
The product is this:
http://www.ebay.in/itm/231841478579?euid=063123e1b1d04e44b73872eb30670836=1

(Copied from ebay.in) Item Specifications

Model-MG-008

Interface-USB 2.0

Supports Card Type-SIM Card

Supports Max. Capacity-n/a

Transmission Rate-n/a

OS System-Windows 7/ 98SE / Me / 2000 / XP / Vista

Dimensions-2.28 in x 0.79 in x 0.39 in (5.8 cm x 2.0 cm x 1.0 cm)

Weight-0.25 oz (7 g)

Speed up to 480 Mbps

480 MBPS/SDHC Ready/Windows XP, 7.0,ME,2K,OS X Supported

Material-Plastic

Quantity-1

Colour-White/Black/Green(Choice of Colour on Stock Availbility)

Package Contents -1 x Card reader & 1 x Software CD

Troubleshooting : If you have any issues with the Compatibility with
any version of Windows, please contact us and we will assist you
-
I hope this helps.
Regards
--
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: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions

2016-05-03 Thread Guodong Xu
On 3 May 2016 at 17:23, Dean Jenkins  wrote:
> On 03/05/16 05:55, John Stultz wrote:
>>
>> In testing with HiKey, we found that since commit 3f30b158eba5c60
>> (asix: On RX avoid creating bad Ethernet frames), we're seeing lots of
>> noise during network transfers:
>>
>> [  239.027993] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header
>> synchronisation was lost, remaining 988
>> [  239.037310] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
>> 0x54ebb5ec, offset 4
>> [  239.045519] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
>> 0xcdffe7a2, offset 4
>> [  239.275044] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header
>> synchronisation was lost, remaining 988
>> [  239.284355] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
>> 0x1d36f59d, offset 4
>> [  239.292541] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
>> 0xaef3c1e9, offset 4
>> [  239.518996] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header
>> synchronisation was lost, remaining 988
>> [  239.528300] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
>> 0x2881912, offset 4
>> [  239.536413] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
>> 0x5638f7e2, offset 4
>>
>>
>> And network throughput ends up being pretty bursty and slow with a
>> overall throughput of at best ~30kB/s.
>>
>> Looking through the commits since the v4.1 kernel where we didn't see
>> this, I narrowed the regression down, and reverting the following two
>> commits seems to avoid the problem:
>>
>> 6a570814cd430fa5ef4f278e8046dcf12ee63f13 asix: Continue processing URB
>> if no RX netdev buffer
>> 3f30b158eba5c604b6e0870027eef5d19fc9271d asix: On RX avoid creating
>> bad Ethernet frames
>>
>> With these reverted, we don't see all the error messages, and we see
>> better ~1.1MB/s throughput (I've got a mouse plugged in, so I think
>> the usb host is only running at "full-speed" mode here).
>>
>> This worries me some, as the patches seem to describe trying to fix
>> the issue they seem to cause, so I suspect a revert isn't the correct
>> solution, but am not sure why we're having such trouble and the patch
>> authors did not.  I'd be happy to do further testing of patches if
>> folks have any ideas.
>>
>> Originally Reported-by: Yongqin Liu 
>>
>> thanks
>> -john
>
> Hi John,
>
> Some ASIX chipsets span the Ethernet frame over consecutive URBs which
> requires successful transfer of 2 URBs.
>
> This means states of a previous URB influences the processing of the next
> URB including a dropped URB (causes a discontinuity in the data stream). In
> other words synchronisation of the in-band 32-bit header word needs to be
> tracked between URBs. Some ASIX chipsets allow the in-band 32-bit header
> word to be no longer fixed to the start of the URB buffer so it moves to any
> position within the URB buffer.
>
> I understand your point of suggesting it is a "regression" for your device
> but the driver was broken for DUB-E100 C1 (small black USB device). So you
> cannot revert the commits as this would break DUB-E100 C1 (small black USB
> device).
>
>> 6a570814cd430fa5ef4f278e8046dcf12ee63f13 asix: Continue processing URB
>> if no RX netdev buffer
>
> This commit is necessary because it avoids a crash when netdev buffer failed
> to be allocated for the 1st URB and the 2nd URB containing a spanned
> Ethernet frame is processed. The crash happens because the 2nd URB assumed
> that the netdev buffer had been allocated.
>
>> 3f30b158eba5c604b6e0870027eef5d19fc9271d asix: On RX avoid creating
>> bad Ethernet frames
>
> This commit is necessary to avoid sending bad Ethernet frames into the IP
> stack during loss of synchronisation and to dropping good Ethernet frames.
> This commit improves the synchronisation recovery mechanism of the in-band
> 32-bit header word.
>
> The ASIX USB to Ethernet devices these commits were tested on where DUB-E100
> C1 (small black USB device). Embedded ARM based systems were used where
> memory resources can run out.

I don't have the chance to look into detail yet. But just a caution,
did you test on ARM 64-bit system or ARM 32-bit? I ask because HiKey
is an ARM 64-bit system. I suggest we should be careful on that. I saw
similar issues when transferring to a 64-bit system in other net
drivers.

Do you have any suggestion on this regard?

>
> It could be that for your USB to Ethernet device that the wrong
> configuration settings have been used. In other words the ASIX driver is
> flexible to support various variants of the ASIX chipsets. For example, does
> your device support Ethernet frames spanning multiple URBs (multiple USB
> transfers) ?

Would you please suggest how to find out this information? How can I
change my device's configuration settings to support spanning multiple
URBs?

>
> So I doubt my commits are "broken" because we don't see your failures (not
> tested your device). It is more likely that your ASIX device needs to be
> properly identified and 

Re: [PATCHv2] musb_host: fix lockup on rxcsr_h_error

2016-05-03 Thread Yegor Yefremov
On Thu, Apr 28, 2016 at 4:37 PM, Bin Liu  wrote:
> Hi,
>
> On Thu, Apr 28, 2016 at 09:51:37AM +0300, Maxim Uvarov wrote:
>
> [snip]
>
>> Hello Bin,
>>
>> yes, it also works with that reset and go to finish:
>>
>> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
>> index c3d5fc9..8cd98e7 100644
>> --- a/drivers/usb/musb/musb_host.c
>> +++ b/drivers/usb/musb/musb_host.c
>> @@ -1599,6 +1599,10 @@ void musb_host_rx(struct musb *musb, u8 epnum)
>> status = -EPROTO;
>> musb_writeb(epio, MUSB_RXINTERVAL, 0);
>>
>> +   rx_csr &= ~MUSB_RXCSR_H_ERROR;
>> +   musb_writew(epio, MUSB_RXCSR, rx_csr);
>> +
>> +   goto finish;
>> } else if (rx_csr & MUSB_RXCSR_DATAERROR) {
>>
>> if (USB_ENDPOINT_XFER_ISOC != qh->type) {
>>
>
> Thanks for testing it.

Have tested your patch and now both FT4232 and Huawei don't freeze on removal.

Bin, Max thanks for fixing this issue.

Tested-by: Yegor Yefremov 

>> That I think a key thing, which is done in other error. If that change
>> is good for you than I'm also happy with it.
>
> We need to understand why the controller keeps generating the same
> interrupt to come out a proper fix.
>
> I will take a look. But I can only use my spare time on this, so be
> patient.
>
>>
>> I also not sure if musb_writeb(epio, MUSB_RXINTERVAL, 0); is needed.
>> In my case it's the same result with it and without it.
>> In other scenarios might be reasonable...
>
> It disables NAK timeout.
>
>>
>>
>> > First of all, I don't like the idea of merging the two branches, it
>> > makes the code ugly.
>>
>> Yes, I don't like that function at all, it's too long and difficult to
>> read if you first look on it first time. It will be good to split it
>> on 3 small functions for each big if.
>
> This particular function is not that hard to understand, but the driver
> in general is messy. But I am not sure if anyone in the community can
> refactory this driver. The community had some effort in the past to
> clean up this driver, but it always broke usecases on different
> platforms.
>
> Regards,
> -Bin.
--
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: misc: usbtest: fix pattern tests for scatterlists.

2016-05-03 Thread Peter Chen
On Tue, May 03, 2016 at 12:31:21PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Peter Chen  writes:
> > On Mon, May 02, 2016 at 11:39:03AM +0300, Mathias Nyman wrote:
> >> The current implemenentation restart the sent pattern for each entry in
> >
> > %s/implemenentation/implementation
> >
> >> the sg list. The receiving end expects a continuous pattern, and test
> >
> > The f_sourcesink may not expect that, have you tried it?
> 
> Use the sources, Luke.
> 

Sorry, what do you mean ? is it a special device?

At Documentation/usb/gadget-testing.txt, sourcesink uses usbtest to
test.

-- 

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 v2 2/3] USB: serial: cp210x: Added comments to CRTSCTS flag code.

2016-05-03 Thread David Laight
From: Konstantin Shkolnyy
> Sent: 30 April 2016 03:22
> Replaced magic numbers used in the CRTSCTS flag code with symbolic names
> from the chip specification.
> 
> Signed-off-by: Konstantin Shkolnyy 
> ---
> Changes in v2:
> Improved CRTSCTS fix based on feedback. Dropped get_termios error handling.
> 
>  drivers/usb/serial/cp210x.c | 93 
> +
>  1 file changed, 69 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
...
> +/* cp210x_flow_ctl::ulControlHandshake */
> +#define SERIAL_DTR_MASK  0x0003
> +#define SERIAL_CTS_HANDSHAKE 0x0008
> +#define SERIAL_DSR_HANDSHAKE 0x0010
> +#define SERIAL_DCD_HANDSHAKE 0x0020
> +#define SERIAL_DSR_SENSITIVITY   0x0040
...

I'd have thought the names ought to start CP210X_

David

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


Re: [PATCH v3 2/3] USB: serial: cp210x: Got rid of magic numbers in CRTSCTS flag code.

2016-05-03 Thread Johan Hovold
On Sat, Apr 30, 2016 at 09:49:38AM -0500, Konstantin Shkolnyy wrote:
> Replaced magic numbers used in the CRTSCTS flag code with symbolic names
> from the chip specification.
> 
> Signed-off-by: Konstantin Shkolnyy 
> ---
> v3:
> Regenerated the patches correctly against the latest usb-next branch.
> v2
> Improved CRTSCTS fix by feedback. Dropped get_termios error handling fix.
> 
>  drivers/usb/serial/cp210x.c | 93 
> +
>  1 file changed, 69 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index fef7a51..24955a7 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -327,6 +327,40 @@ struct cp210x_comm_status {
>   */
>  #define PURGE_ALL0x000f
>  
> +/* CP210X_GET_FLOW/CP210X_SET_FLOW read/write these 0x10 bytes */
> +struct cp210x_flow_ctl {
> + __le32  ulControlHandshake;
> + __le32  ulFlowReplace;
> + __le32  ulXonLimit;
> + __le32  ulXoffLimit;
> +} __packed;
> +
> +/* cp210x_flow_ctl::ulControlHandshake */
> +#define SERIAL_DTR_MASK  0x0003
> +#define SERIAL_CTS_HANDSHAKE 0x0008
> +#define SERIAL_DSR_HANDSHAKE 0x0010
> +#define SERIAL_DCD_HANDSHAKE 0x0020
> +#define SERIAL_DSR_SENSITIVITY   0x0040
> +
> +/* values for cp210x_flow_ctl::ulControlHandshake::SERIAL_DTR_MASK */
> +#define SERIAL_DTR_INACTIVE  0x
> +#define SERIAL_DTR_ACTIVE0x0001
> +#define SERIAL_DTR_FLOW_CTL  0x0002
> +
> +/* cp210x_flow_ctl::ulFlowReplace */
> +#define SERIAL_AUTO_TRANSMIT 0x0001
> +#define SERIAL_AUTO_RECEIVE  0x0002
> +#define SERIAL_ERROR_CHAR0x0004
> +#define SERIAL_NULL_STRIPPING0x0008
> +#define SERIAL_BREAK_CHAR0x0010
> +#define SERIAL_RTS_MASK  0x00c0
> +#define SERIAL_XOFF_CONTINUE 0x8000
> +
> +/* values for cp210x_flow_ctl::ulFlowReplace::SERIAL_RTS_MASK */
> +#define SERIAL_RTS_INACTIVE  0x
> +#define SERIAL_RTS_ACTIVE0x0040
> +#define SERIAL_RTS_FLOW_CTL  0x0080

Please use the BIT and GENMASK macros for the above. Also add shift
defines for the DTR and RTS fields instead of including it in the
values.

> +
>  /*
>   * Reads a variable-sized block of CP210X_ registers, identified by req.
>   * Returns data into buf in native USB byte order.
> @@ -694,7 +728,7 @@ static void cp210x_get_termios_port(struct 
> usb_serial_port *port,
>  {
>   struct device *dev = >dev;
>   unsigned int cflag;
> - u8 modem_ctl[16];
> + struct cp210x_flow_ctl flow_ctl;
>   u32 baud;
>   u16 bits;
>  
> @@ -792,9 +826,9 @@ static void cp210x_get_termios_port(struct 
> usb_serial_port *port,
>   break;
>   }
>  
> - cp210x_read_reg_block(port, CP210X_GET_FLOW, modem_ctl,
> - sizeof(modem_ctl));
> - if (modem_ctl[0] & 0x08) {
> + cp210x_read_reg_block(port, CP210X_GET_FLOW, _ctl,
> + sizeof(flow_ctl));
> + if (le32_to_cpu(flow_ctl.ulControlHandshake) & SERIAL_CTS_HANDSHAKE) {

Use a temporary for the control-handshake field to make this a bit more
readable.

>   dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__);
>   cflag |= CRTSCTS;
>   } else {
> @@ -863,7 +897,6 @@ static void cp210x_set_termios(struct tty_struct *tty,
>   struct device *dev = >dev;
>   unsigned int cflag, old_cflag;
>   u16 bits;
> - u8 modem_ctl[16];
>  
>   cflag = tty->termios.c_cflag;
>   old_cflag = old_termios->c_cflag;
> @@ -948,33 +981,45 @@ static void cp210x_set_termios(struct tty_struct *tty,
>  
>   if ((cflag & CRTSCTS) != (old_cflag & CRTSCTS)) {
>  

You can remove this stray newline as well.

> - /* Only bytes 0, 4 and 7 out of first 8 have functional bits */
> + struct cp210x_flow_ctl flow_ctl;
> + u32 ControlHandshake;
> + u32 FlowReplace;

Don't use CamelCase. The only exception would be for message structures
going out over the wire were we allow using the specification field
names.

Use something short as ctrl and flow for these temporaries.

> - cp210x_read_reg_block(port, CP210X_GET_FLOW, modem_ctl,
> - sizeof(modem_ctl));
> - dev_dbg(dev, "%s - read modem controls = %02x .. .. .. %02x .. 
> .. %02x\n",
> - __func__, modem_ctl[0], modem_ctl[4], modem_ctl[7]);
> + cp210x_read_reg_block(port, CP210X_GET_FLOW, _ctl,
> + sizeof(flow_ctl));
> + ControlHandshake = le32_to_cpu(flow_ctl.ulControlHandshake);
> + FlowReplace  = le32_to_cpu(flow_ctl.ulFlowReplace);
> + dev_dbg(dev, "%s - read ulControlHandshake=%08x 
> ulFlowReplace=%08x\n",
> + __func__, ControlHandshake, FlowReplace);

Please add a 0x prefix after the equal signs. Add a comma after the

Re: [PATCH] usb: xhci-mtk: fixup mouse wakeup failure during system suspend

2016-05-03 Thread Felipe Balbi

Hi,

chunfeng yun  writes:
>> chunfeng yun  writes:
>> > On Thu, 2016-04-21 at 10:04 +0800, Chunfeng Yun wrote:
>> >> Click mouse after xhci suspend completion but before system suspend
>> >> completion, system will not be waken up by mouse if the duration of
>> >> them is larger than 20ms which is the device UFP's resume signaling
>> 
>> what is "them" here ? The duration of what is longer than 20ms ?
> They are "xhci suspend completion" and "system suspend completion";
>
> It's time duration

okay. So if xhci suspend takes longer than 20ms your SPM doesn't see a
wakeup ?

>> >> lasted. Another reason is that the SPM is not enabled before system
>> 
>> what's SPM ?
> It is System Power Management which is powered off when system is
> running in normal mode, and is powered on when system enters suspend
> mode. It is used to wakeup system when some wakeup sources, such as
> bluetooth or powerkey etc, tigger wakeup event.

okay, thanks

>> >> suspend compeltion, this causes SPM also not notice the resume signal.
>>^^
>>completion
>> 
>> >> So in order to reduce the duration less than 20ms, make use of
>> >> syscore's suspend/resume interface.
>> 
>> no, this is the wrong approach
> But it seems only one workable approach from software side

I wouldn't say that. It seems to me SPM should be enabled earlier.

>> >> Because the syscore runs on irq disabled context, and xhci's
>> >> suspend/resume calls some sleeping functions, enable local irq
>> >> and then disable it during suspend/resume. This may be not a problem,
>> >> since only boot CPU is runing.
>> 
>> another problem :) calling local_irq_{enable,disable}() is an indication
>> that something's wrong.
> Oh!
>
> BTW: There will be warning logs if they are not called.

yeah, I got that :-) But it's still wrong to use
local_irq_{enable,disable}() the way you're using them :-)

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH] usb: misc: usbtest: fix pattern tests for scatterlists.

2016-05-03 Thread Felipe Balbi

Hi,

Peter Chen  writes:
> On Mon, May 02, 2016 at 11:39:03AM +0300, Mathias Nyman wrote:
>> The current implemenentation restart the sent pattern for each entry in
>
> %s/implemenentation/implementation
>
>> the sg list. The receiving end expects a continuous pattern, and test
>
> The f_sourcesink may not expect that, have you tried it?

Use the sources, Luke.

-- 
balbi


signature.asc
Description: PGP signature


Re: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions

2016-05-03 Thread Dean Jenkins

On 03/05/16 05:55, John Stultz wrote:

In testing with HiKey, we found that since commit 3f30b158eba5c60
(asix: On RX avoid creating bad Ethernet frames), we're seeing lots of
noise during network transfers:

[  239.027993] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  239.037310] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0x54ebb5ec, offset 4
[  239.045519] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0xcdffe7a2, offset 4
[  239.275044] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  239.284355] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0x1d36f59d, offset 4
[  239.292541] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0xaef3c1e9, offset 4
[  239.518996] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  239.528300] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0x2881912, offset 4
[  239.536413] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length
0x5638f7e2, offset 4


And network throughput ends up being pretty bursty and slow with a
overall throughput of at best ~30kB/s.

Looking through the commits since the v4.1 kernel where we didn't see
this, I narrowed the regression down, and reverting the following two
commits seems to avoid the problem:

6a570814cd430fa5ef4f278e8046dcf12ee63f13 asix: Continue processing URB
if no RX netdev buffer
3f30b158eba5c604b6e0870027eef5d19fc9271d asix: On RX avoid creating
bad Ethernet frames

With these reverted, we don't see all the error messages, and we see
better ~1.1MB/s throughput (I've got a mouse plugged in, so I think
the usb host is only running at "full-speed" mode here).

This worries me some, as the patches seem to describe trying to fix
the issue they seem to cause, so I suspect a revert isn't the correct
solution, but am not sure why we're having such trouble and the patch
authors did not.  I'd be happy to do further testing of patches if
folks have any ideas.

Originally Reported-by: Yongqin Liu 

thanks
-john

Hi John,

Some ASIX chipsets span the Ethernet frame over consecutive URBs which 
requires successful transfer of 2 URBs.


This means states of a previous URB influences the processing of the 
next URB including a dropped URB (causes a discontinuity in the data 
stream). In other words synchronisation of the in-band 32-bit header 
word needs to be tracked between URBs. Some ASIX chipsets allow the 
in-band 32-bit header word to be no longer fixed to the start of the URB 
buffer so it moves to any position within the URB buffer.


I understand your point of suggesting it is a "regression" for your 
device but the driver was broken for DUB-E100 C1 (small black USB 
device). So you cannot revert the commits as this would break DUB-E100 
C1 (small black USB device).



6a570814cd430fa5ef4f278e8046dcf12ee63f13 asix: Continue processing URB
if no RX netdev buffer
This commit is necessary because it avoids a crash when netdev buffer 
failed to be allocated for the 1st URB and the 2nd URB containing a 
spanned Ethernet frame is processed. The crash happens because the 2nd 
URB assumed that the netdev buffer had been allocated.



3f30b158eba5c604b6e0870027eef5d19fc9271d asix: On RX avoid creating
bad Ethernet frames
This commit is necessary to avoid sending bad Ethernet frames into the 
IP stack during loss of synchronisation and to dropping good Ethernet 
frames. This commit improves the synchronisation recovery mechanism of 
the in-band 32-bit header word.


The ASIX USB to Ethernet devices these commits were tested on where 
DUB-E100 C1 (small black USB device). Embedded ARM based systems were 
used where memory resources can run out.


It could be that for your USB to Ethernet device that the wrong 
configuration settings have been used. In other words the ASIX driver is 
flexible to support various variants of the ASIX chipsets. For example, 
does your device support Ethernet frames spanning multiple URBs 
(multiple USB transfers) ?


So I doubt my commits are "broken" because we don't see your failures 
(not tested your device). It is more likely that your ASIX device needs 
to be properly identified and configured to be compatible with the ASIX 
driver. At least, I suggest that is the best place to start your 
investigation.


Of course, your ASIX chipset might have a different behaviour for how 
the in-band 32-bit header word operates so perhaps special treatment is 
needed for your chipset ?


Please send to the mailing list the output of lsusb for your device so 
that people can know the USB product ID and vendor ID for your device. 
This is allows people to assist with the investigation. Do you have any 
links to websites that sell your device ?


Are you using UDP or TCP connections ?

Regards,
Dean

--
Dean Jenkins
Embedded Software Engineer
Linux Transportation Solutions
Mentor Embedded Software Division
Mentor Graphics (UK) Ltd.

--

Re: [PATCH] usb: misc: usbtest: fix pattern tests for scatterlists.

2016-05-03 Thread Peter Chen
On Mon, May 02, 2016 at 11:39:03AM +0300, Mathias Nyman wrote:
> The current implemenentation restart the sent pattern for each entry in

%s/implemenentation/implementation

> the sg list. The receiving end expects a continuous pattern, and test

The f_sourcesink may not expect that, have you tried it?

> will fail unless scatterilst entries happen to be aligned with the
> pattern
> 
> Fix this by calculating the pattern byte based on total sent size
> instead of just the current sg entry.
> 
> Signed-off-by: Mathias Nyman 
> ---
>  drivers/usb/misc/usbtest.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
> index 92fdb6e..c78ff95 100644
> --- a/drivers/usb/misc/usbtest.c
> +++ b/drivers/usb/misc/usbtest.c
> @@ -529,6 +529,7 @@ static struct scatterlist *
>  alloc_sglist(int nents, int max, int vary, struct usbtest_dev *dev, int pipe)
>  {
>   struct scatterlist  *sg;
> + unsigned intn_size = 0;
>   unsignedi;
>   unsignedsize = max;
>   unsignedmaxpacket =
> @@ -561,7 +562,8 @@ alloc_sglist(int nents, int max, int vary, struct 
> usbtest_dev *dev, int pipe)
>   break;
>   case 1:
>   for (j = 0; j < size; j++)
> - *buf++ = (u8) ((j % maxpacket) % 63);
> + *buf++ = (u8) (((j + n_size) % maxpacket) % 63);
> + n_size += size;
>   break;
>   }
>  
> -- 
> 1.9.1
> 
> --
> 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

-- 

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: xhci-mtk: fixup mouse wakeup failure during system suspend

2016-05-03 Thread chunfeng yun
On Tue, 2016-05-03 at 10:51 +0300, Felipe Balbi wrote:
> Hi,
> 
> chunfeng yun  writes:
> > On Thu, 2016-04-21 at 10:04 +0800, Chunfeng Yun wrote:
> >> Click mouse after xhci suspend completion but before system suspend
> >> completion, system will not be waken up by mouse if the duration of
> >> them is larger than 20ms which is the device UFP's resume signaling
> 
> what is "them" here ? The duration of what is longer than 20ms ?
They are "xhci suspend completion" and "system suspend completion";

It's time duration

> 
> >> lasted. Another reason is that the SPM is not enabled before system
> 
> what's SPM ?
It is System Power Management which is powered off when system is
running in normal mode, and is powered on when system enters suspend
mode. It is used to wakeup system when some wakeup sources, such as
bluetooth or powerkey etc, tigger wakeup event.

> 
> >> suspend compeltion, this causes SPM also not notice the resume signal.
>^^
>completion
> 
> >> So in order to reduce the duration less than 20ms, make use of
> >> syscore's suspend/resume interface.
> 
> no, this is the wrong approach
But it seems only one workable approach from software side

> 
> >> In fact it is a work around solution which only reduces the
> >> probability of failure, because we can't ensure that the duration from
> >> syscore's suspend completion to SPM working is always less than 20ms.
> 
> right, which means you're not really fixing anything. Morevoer, you make
> it so that this won't work with multiple instances of the XHCI IP in
> your SoC.
> 
It can be easily fixed up from hardware side by latching resume signal
until SPM is powered on.

I forgot to take into account of multiple instances
 
> >> Because the syscore runs on irq disabled context, and xhci's
> >> suspend/resume calls some sleeping functions, enable local irq
> >> and then disable it during suspend/resume. This may be not a problem,
> >> since only boot CPU is runing.
> 
> another problem :) calling local_irq_{enable,disable}() is an indication
> that something's wrong.
Oh!

BTW: There will be warning logs if they are not called.

Thanks a lot
> 
> >> Signed-off-by: Chunfeng Yun 
> >> ---
> >>  drivers/usb/host/xhci-mtk.c |   31 ++-
> >>  1 file changed, 30 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> >> index 79959f1..8277f02 100644
> >> --- a/drivers/usb/host/xhci-mtk.c
> >> +++ b/drivers/usb/host/xhci-mtk.c
> >> @@ -28,6 +28,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>  
> >>  #include "xhci.h"
> >>  #include "xhci-mtk.h"
> >> @@ -490,6 +491,8 @@ static int xhci_mtk_setup(struct usb_hcd *hcd)
> >>return xhci_gen_setup(hcd, xhci_mtk_quirks);
> >>  }
> >>  
> >> +static struct device *xhci_mtk_syscore_dev;
> 
> now, what happens when you have more than one XHCI instance ?
> 


--
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: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Lars Melin

On 2016-05-03 14:48, Susmita/Rajib Bandopadhyay wrote:

Dear Sirs,
I had already tried
$ mkdir /media/sim
$ sudo mount /dev/sdc /media/sim
But this doesn't work, as I don't know the filesystem of the card.


Your reader is a TF/SD/MMC card reader from what I can see, it reads 
various types of memory cards.
A SIM card is a smart card and not a memory card/mass storage device and 
smart card are accessed serially using the ISO7816 communication protocol.
The internal file system is not accessible for mounting, instead there 
are serial 5 byte commands for selecting a file and commands for 
reading/erasing/writing the file you have selected.


/Lars



--
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] option.c: Support for Gemalto's Cinterion PH8 and AHxx products added

2016-05-03 Thread Johan Hovold
On Fri, Apr 29, 2016 at 08:51:06AM +, Schemmel Hans-Christoph wrote:
> Added support for Gemalto's Cinterion PH8 and AHxx products
> with 2 RmNet Interfaces and products with 1 RmNet + 1 USB Audio interface.
> 
> In addition some minor renaming and formatting.
> 
> Signed-off-by: Hans-Christoph Schemmel 

Now applied, thanks.

Johan
--
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/2] usb: xhci: mem: convert to a switch statement

2016-05-03 Thread Felipe Balbi
when getting endpoint type a switch statement looks
better than a series of if () branches. There are no
functional changes with this patch, cleanup only.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/host/xhci-mem.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 0f2ca6e85bc0..5949c7962aec 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1364,14 +1364,16 @@ static u32 xhci_get_endpoint_type(struct 
usb_host_endpoint *ep)
 
in = usb_endpoint_dir_in(>desc);
 
-   if (usb_endpoint_xfer_control(>desc))
+   switch (usb_endpoint_type(>desc)) {
+   case USB_ENDPOINT_XFER_CONTROL:
return CTRL_EP;
-   if (usb_endpoint_xfer_bulk(>desc))
+   case USB_ENDPOINT_XFER_BULK:
return in ? BULK_IN_EP : BULK_OUT_EP;
-   if (usb_endpoint_xfer_isoc(>desc))
+   case USB_ENDPOINT_XFER_ISOC:
return in ? ISOC_IN_EP : ISOC_OUT_EP;
-   if (usb_endpoint_xfer_int(>desc))
+   case USB_ENDPOINT_XFER_INT:
return in ? INT_IN_EP : INT_OUT_EP;
+   }
return 0;
 }
 
-- 
2.8.1

--
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/2] usb: xhci: switch to running avg trb length

2016-05-03 Thread Felipe Balbi
It's unlikely that we will ever know the avg so
instead of assuming it'll be something really large,
we will calculate the avg as we go as mentioned in
XHCI specification section 4.14.1.1.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/host/xhci-mem.c  | 34 --
 drivers/usb/host/xhci-ring.c | 20 
 drivers/usb/host/xhci.h  |  3 ++-
 3 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index bad0d1f9a41d..0f2ca6e85bc0 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1451,18 +1451,34 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
virt_dev->eps[ep_index].skip = false;
ep_ring = virt_dev->eps[ep_index].new_ring;
 
-   /*
-* Get values to fill the endpoint context, mostly from ep descriptor.
-* The average TRB buffer lengt for bulk endpoints is unclear as we
-* have no clue on scatter gather list entry size. For Isoc and Int,
-* set it to max available. See xHCI 1.1 spec 4.14.1.1 for details.
-*/
+   /* Get values to fill the endpoint context, mostly from ep descriptor. 
*/
max_esit_payload = xhci_get_max_esit_payload(udev, ep);
interval = xhci_get_endpoint_interval(udev, ep);
mult = xhci_get_endpoint_mult(udev, ep);
max_packet = GET_MAX_PACKET(usb_endpoint_maxp(>desc));
max_burst = xhci_get_endpoint_max_burst(udev, ep);
-   avg_trb_len = max_esit_payload;
+
+   /*
+* We are using a running avg for our endpoint's avg_trb_len. The reason
+* is that we have no clue about average transfer sizes for any
+* endpoints because the HCD does not know which USB Class is running on
+* the other end.
+*
+* See xHCI 1.1 spec 4.14.1.1 for details about initial avg_trb_len
+* setting.
+*/
+   switch (usb_endpoint_type(>desc)) {
+   case USB_ENDPOINT_XFER_CONTROL:
+   avg_trb_len = 8;
+   break;
+   case USB_ENDPOINT_XFER_INT:
+   avg_trb_len = 1024;
+   break;
+   case USB_ENDPOINT_XFER_ISOC:
+   case USB_ENDPOINT_XFER_BULK:
+   avg_trb_len = 3072;
+   break;
+   }
 
/* FIXME dig Mult and streams info out of ep companion desc */
 
@@ -1472,9 +1488,7 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
/* Some devices get this wrong */
if (usb_endpoint_xfer_bulk(>desc) && udev->speed == USB_SPEED_HIGH)
max_packet = 512;
-   /* xHCI 1.0 and 1.1 indicates that ctrl ep avg TRB Length should be 8 */
-   if (usb_endpoint_xfer_control(>desc) && xhci->hci_version >= 0x100)
-   avg_trb_len = 8;
+
/* xhci 1.1 with LEC support doesn't use mult field, use RsvdZ */
if ((xhci->hci_version > 0x100) && HCC2_LEC(xhci->hcc_params2))
mult = 0;
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 99b4ff42f7a0..6c41dbbf9f2f 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2901,6 +2901,8 @@ static int prepare_transfer(struct xhci_hcd *xhci,
struct xhci_td  *td;
struct xhci_ring *ep_ring;
struct xhci_ep_ctx *ep_ctx = xhci_get_ep_ctx(xhci, xdev->out_ctx, 
ep_index);
+   unsigned int avg_trb_len;
+   unsigned int tx_info;
 
ep_ring = xhci_stream_id_to_ring(xdev, ep_index, stream_id);
if (!ep_ring) {
@@ -2909,6 +2911,24 @@ static int prepare_transfer(struct xhci_hcd *xhci,
return -EINVAL;
}
 
+   /*
+* Here we update avg_trb_len so that, over time, we get a better
+* representation of what the actual average length for this endpoint's
+* TRBs are going to be.
+*/
+   if (urb->transfer_buffer_length > 0) {
+   tx_info = le32_to_cpu(ep_ctx->tx_info);
+
+   avg_trb_len = EP_AVG_TRB_LENGTH(tx_info);
+   avg_trb_len += urb->transfer_buffer_length;
+   avg_trb_len /= 2;
+
+   tx_info &= ~EP_AVG_TRB_LENGTH_MASK;
+   tx_info |= EP_AVG_TRB_LENGTH(avg_trb_len);
+
+   ep_ctx->tx_info = cpu_to_le32(tx_info);
+   }
+
ret = prepare_ring(xhci, ep_ring,
   le32_to_cpu(ep_ctx->ep_info) & EP_STATE_MASK,
   num_trbs, mem_flags);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 6c629c97f8ad..9f1e9be0afcc 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -751,7 +751,8 @@ struct xhci_ep_ctx {
 #define GET_MAX_PACKET(p)  ((p) & 0x7ff)
 
 /* tx_info bitmasks */
-#define EP_AVG_TRB_LENGTH(p)   ((p) & 0x)
+#define EP_AVG_TRB_LENGTH_MASK 0x
+#define EP_AVG_TRB_LENGTH(p)   ((p) & EP_AVG_TRB_LENGTH_MASK)
 #define EP_MAX_ESIT_PAYLOAD_LO(p)  (((p) & 0x) 

Re: [PATCH] usb: serial: ti_usb_3410_5052: add MOXA UPORT 11x0 support

2016-05-03 Thread Johan Hovold
On Mon, May 02, 2016 at 08:37:15PM +0200, Mathieu OTHACEHE wrote:
> Hi Johan,
> 
> Thanks for your review.
> 
> > Looks like this code could use a few vid/pid temporaries.
> 
> > I'm not sure it makes sense to try to load a "ti_usb-v110a-p1150.fw"
> > firmware before requesting the moxa firmware. Avoids a confusing:
> 
> > usb 1-2.2: Direct firmware load for ti_usb-v110a-p1150.fw failed with error 
> > -2
> 
> > message too.
> 
> I'm not sure to get your point here, shall I rename moxa firmwares in
> linux-firmware repo to be compliant with ti_usb-v%04x-p%04x.fw format ?

No, I was trying to say that the we should not attempt to load a
firmware on the "ti_usb-v%04x-p%04x.fw" format before loading the moxa
firmware.

I guess the moxa firmware names have been chosen by Moxa and it might be
confusing if we renamed them, but that could be an option too.

> > I did a quick test of the patch using a Moxa 1150-device. Works at
> > 115200, but communication appeared broken at 9600. Looks like the baud
> > rate calculations are similar but not identical to what the Moxa driver
> > does. Is this something you have looked into?
> 
> Well, on my moxa 1110 communication is working at 9600, 115200 and
> other baud rates.

I must have messed something up in my test, as now 9600 seems to work.

> However, I think baud rate calculation may be wrong for TI3410 chips.
> 
> According to table 5-13 in datasheet 
> http://www.ti.com/lit/ds/symlink/tusb3410.pdf
> the baud rate calculation formula, is :
> 
> baud_rate = 923077 / (desired_baud_rate)
> 
> So, we get :
> 
> desired_baud = 9600   -> baud_rate = 923077 / 9600 = 96
> desired_baud = 115200 -> baud_rate = 923077 / 115200 = 8
> ...
> 
> In ti_usb_3410_5052 driver, the formula used for 3410 is :
> 
> baud_rate = (923077 + desired_baud_rate/2) / desired_baud_rate
> 
> so,
> 
> desired_baud = 9600   -> baud_rate = (923077 + 9600/2) / 9600 = 97 (!= 96)
> desired_baud = 115200 -> baud_rate = (923077 + 115200/2) / 115200 = 9 (!= 8)
> 
> It seems the formula is wrong but some firmware deal with it anyway.
> Should I correct the formula or use a quirk for moxa devices ?

No, that's just integer-division with rounding. Remember that the
divisions above are integer divisions so the results are actually 96 and
8 as expected.

Thanks,
Johan
--
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: dwc3: gadget: fix mask and shift order in DWC3_DCFG_NUMP()

2016-05-03 Thread Felipe Balbi

Hi,

Dan Carpenter  writes:
> In the original DWC3_DCFG_NUMP() was always zero.  It looks like the
> intent was to shift first and then do the mask.
>
> Fixes: 2a58f9c12bb3 ('usb: dwc3: gadget: disable automatic calculation of ACK 
> TP NUMP')
> Signed-off-by: Dan Carpenter 

Thanks for this fix, I completely missed it :-) Greg, if you want to
take this as a patch, that's fine by me. Otherwise I can queue it for
-rc1. In any case, S-o-B below:

Signed-off-by: Felipe Balbi 

> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 186a886..7ddf944 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -277,7 +277,7 @@
>  #define DWC3_DCFG_FULLSPEED1 (3 << 0)
>  
>  #define DWC3_DCFG_NUMP_SHIFT 17
> -#define DWC3_DCFG_NUMP(n)(((n) & 0x1f) >> DWC3_DCFG_NUMP_SHIFT)
> +#define DWC3_DCFG_NUMP(n)(((n) >> DWC3_DCFG_NUMP_SHIFT) & 0x1f)
>  #define DWC3_DCFG_NUMP_MASK  (0x1f << DWC3_DCFG_NUMP_SHIFT)
>  #define DWC3_DCFG_LPM_CAP(1 << 22)
>  
> --
> 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

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH] usb: xhci-mtk: fixup mouse wakeup failure during system suspend

2016-05-03 Thread Felipe Balbi

Hi,

chunfeng yun  writes:
> On Thu, 2016-04-21 at 10:04 +0800, Chunfeng Yun wrote:
>> Click mouse after xhci suspend completion but before system suspend
>> completion, system will not be waken up by mouse if the duration of
>> them is larger than 20ms which is the device UFP's resume signaling

what is "them" here ? The duration of what is longer than 20ms ?

>> lasted. Another reason is that the SPM is not enabled before system

what's SPM ?

>> suspend compeltion, this causes SPM also not notice the resume signal.
   ^^
   completion

>> So in order to reduce the duration less than 20ms, make use of
>> syscore's suspend/resume interface.

no, this is the wrong approach

>> In fact it is a work around solution which only reduces the
>> probability of failure, because we can't ensure that the duration from
>> syscore's suspend completion to SPM working is always less than 20ms.

right, which means you're not really fixing anything. Morevoer, you make
it so that this won't work with multiple instances of the XHCI IP in
your SoC.

>> Because the syscore runs on irq disabled context, and xhci's
>> suspend/resume calls some sleeping functions, enable local irq
>> and then disable it during suspend/resume. This may be not a problem,
>> since only boot CPU is runing.

another problem :) calling local_irq_{enable,disable}() is an indication
that something's wrong.

>> Signed-off-by: Chunfeng Yun 
>> ---
>>  drivers/usb/host/xhci-mtk.c |   31 ++-
>>  1 file changed, 30 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
>> index 79959f1..8277f02 100644
>> --- a/drivers/usb/host/xhci-mtk.c
>> +++ b/drivers/usb/host/xhci-mtk.c
>> @@ -28,6 +28,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  
>>  #include "xhci.h"
>>  #include "xhci-mtk.h"
>> @@ -490,6 +491,8 @@ static int xhci_mtk_setup(struct usb_hcd *hcd)
>>  return xhci_gen_setup(hcd, xhci_mtk_quirks);
>>  }
>>  
>> +static struct device *xhci_mtk_syscore_dev;

now, what happens when you have more than one XHCI instance ?

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH] Revert "USB / PM: Allow USB devices to remain runtime-suspended when sleeping"

2016-05-03 Thread Johan Hovold
On Mon, May 02, 2016 at 11:13:10AM -0400, Alan Stern wrote:
> On Mon, 2 May 2016, Johan Hovold wrote:
> 
> > This reverts commit e3345db85068ddb937fc0ba40dfc39c293dad977, which
> > broke system resume for a large class of devices.
> > 
> > Devices that after having been reset during resume need to be rebound
> > due to a missing reset_resume callback, are now left in a suspended
> > state. This specifically broke resume of common USB-serial devices,
> > which are now unusable after system suspend (until disconnected and
> > reconnected) when USB persist is enabled.
> > 
> > During resume, usb_resume_interface will set the needs_binding flag for
> > such interfaces, but unlike system resume, run-time resume does not
> > honour it.
> > 
> > Cc: stable  # 4.5
> > Signed-off-by: Johan Hovold 
> > ---
> > 
> > Greg, Alan,
> > 
> > This patch for v4.6-rc7 fixes a 4.5-regression that broke system suspend
> > for a large class of devices, including USB-serial devices, for example
> > when USB persist is enabled.
> > 
> > We may be able to find a way around this, but since it's a user-visible
> > regression and late in the rc-cycle, I believe reverting the offending
> > commit is the right thing to do.
> 
> The description of the problem doesn't sound right to me.  For 
> instance, would it help if usb_runtime_resume() did honor the 
> needs_binding flag?  I doubt it.  Things like the wakeup setting would 
> still be lost before the runtime resume occurred.
>
> I suspect the right answer is always to resume a USB device if it needs 
> a reset-resume, but otherwise allow it to remain in runtime suspend.

I was trying to describe how things broke but not necessarily to suggest
how this should be fixed. Could have put the last paragraph below the
cut-off line I guess.

> Reverting the patch for now is okay with me.  Tomeu may want to work on
> a better solution.  Part of the difficulty is that the PM core wants to
> know before suspending whether skipping resume will be okay, but the
> USB stack doesn't know until after the host controller has been 
> resumed.
> 
> In the end, we'll probably have the PM core call usb_resume all the 
> time, but usb_resume will leave the device in runtime suspend if it 
> can.  This isn't ideal but it may be the best we can do.

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


[patch] usb: dwc3: gadget: fix mask and shift order in DWC3_DCFG_NUMP()

2016-05-03 Thread Dan Carpenter
In the original DWC3_DCFG_NUMP() was always zero.  It looks like the
intent was to shift first and then do the mask.

Fixes: 2a58f9c12bb3 ('usb: dwc3: gadget: disable automatic calculation of ACK 
TP NUMP')
Signed-off-by: Dan Carpenter 

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 186a886..7ddf944 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -277,7 +277,7 @@
 #define DWC3_DCFG_FULLSPEED1   (3 << 0)
 
 #define DWC3_DCFG_NUMP_SHIFT   17
-#define DWC3_DCFG_NUMP(n)  (((n) & 0x1f) >> DWC3_DCFG_NUMP_SHIFT)
+#define DWC3_DCFG_NUMP(n)  (((n) >> DWC3_DCFG_NUMP_SHIFT) & 0x1f)
 #define DWC3_DCFG_NUMP_MASK(0x1f << DWC3_DCFG_NUMP_SHIFT)
 #define DWC3_DCFG_LPM_CAP  (1 << 22)
 
--
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: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Susmita/Rajib Bandopadhyay
Dear Sirs,
I had already tried
$ mkdir /media/sim
$ sudo mount /dev/sdc /media/sim
But this doesn't work, as I don't know the filesystem of the card.
Sirs, I took up the matter with the Debian & the Knoppix communities,
as mentioned below:
How
to back up data of SIM Card using its Reader?
How
to back up SIM Card & data with its Reader?

The Debian Forum members' inputs can be followed from the link.

Knoppix forum is silent.

Do I need to install some packages to read the fs of the card?

Regards
--
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: xhci-mtk: fixup mouse wakeup failure during system suspend

2016-05-03 Thread chunfeng yun
Hi Mathias,

On Thu, 2016-04-21 at 10:04 +0800, Chunfeng Yun wrote:
> Click mouse after xhci suspend completion but before system suspend
> completion, system will not be waken up by mouse if the duration of
> them is larger than 20ms which is the device UFP's resume signaling
> lasted. Another reason is that the SPM is not enabled before system
> suspend compeltion, this causes SPM also not notice the resume signal.
> So in order to reduce the duration less than 20ms, make use of
> syscore's suspend/resume interface.
> In fact it is a work around solution which only reduces the
> probability of failure, because we can't ensure that the duration from
> syscore's suspend completion to SPM working is always less than 20ms.
> 
> Because the syscore runs on irq disabled context, and xhci's
> suspend/resume calls some sleeping functions, enable local irq
> and then disable it during suspend/resume. This may be not a problem,
> since only boot CPU is runing.
> 
> Signed-off-by: Chunfeng Yun 
> ---
>  drivers/usb/host/xhci-mtk.c |   31 ++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index 79959f1..8277f02 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -28,6 +28,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "xhci.h"
>  #include "xhci-mtk.h"
> @@ -490,6 +491,8 @@ static int xhci_mtk_setup(struct usb_hcd *hcd)
>   return xhci_gen_setup(hcd, xhci_mtk_quirks);
>  }
>  
> +static struct device *xhci_mtk_syscore_dev;
> +
>  static int xhci_mtk_probe(struct platform_device *pdev)
>  {
>   struct device *dev = >dev;
> @@ -512,6 +515,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>   if (!mtk)
>   return -ENOMEM;
>  
> + xhci_mtk_syscore_dev = dev;
>   mtk->dev = dev;
>   mtk->vbus = devm_regulator_get(dev, "vbus");
>   if (IS_ERR(mtk->vbus)) {
> @@ -740,6 +744,29 @@ static int __maybe_unused xhci_mtk_resume(struct device 
> *dev)
>   return 0;
>  }
>  
> +static int xhci_mtk_syscore_suspend(void)
> +{
> + local_irq_enable();
> + xhci_mtk_suspend(xhci_mtk_syscore_dev);
> + local_irq_disable();
> +
> + return 0;
> +}
> +
> +static void xhci_mtk_syscore_resume(void)
> +{
> + local_irq_enable();
> + xhci_mtk_resume(xhci_mtk_syscore_dev);
> + local_irq_disable();
> +
> + return;
> +}
> +
> +static struct syscore_ops xhci_mtk_syscore_ops = {
> + .suspend = xhci_mtk_syscore_suspend,
> + .resume = xhci_mtk_syscore_resume,
> +};
> +
>  static const struct dev_pm_ops xhci_mtk_pm_ops = {
>   SET_SYSTEM_SLEEP_PM_OPS(xhci_mtk_suspend, xhci_mtk_resume)
>  };
> @@ -758,7 +785,7 @@ static struct platform_driver mtk_xhci_driver = {
>   .remove = xhci_mtk_remove,
>   .driver = {
>   .name = "xhci-mtk",
> - .pm = DEV_PM_OPS,
> + /*.pm = DEV_PM_OPS,*/
>   .of_match_table = of_match_ptr(mtk_xhci_of_match),
>   },
>  };
> @@ -766,6 +793,7 @@ MODULE_ALIAS("platform:xhci-mtk");
>  
>  static int __init xhci_mtk_init(void)
>  {
> + register_syscore_ops(_mtk_syscore_ops);
>   xhci_init_driver(_mtk_hc_driver, _mtk_overrides);
>   return platform_driver_register(_xhci_driver);
>  }
> @@ -774,6 +802,7 @@ module_init(xhci_mtk_init);
>  static void __exit xhci_mtk_exit(void)
>  {
>   platform_driver_unregister(_xhci_driver);
> + unregister_syscore_ops(_mtk_syscore_ops);
>  }
>  module_exit(xhci_mtk_exit);

Do you have any suggestions about the patch?

Thanks
>  


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


RE: [PATCH v6 09/12] usb: gadget: udc: adapt to OTG core

2016-05-03 Thread Jun Li
Hi

> >>>  /**
> >>> + * usb_gadget_start - start the usb gadget controller and
> >>> +connect to bus
> >>> + * @gadget: the gadget device to start
> >>> + *
> >>> + * This is external API for use by OTG core.
> >>> + *
> >>> + * Start the usb device controller and connect to bus (enable
> pull).
> >>> + */
> >>> +static int usb_gadget_start(struct usb_gadget *gadget) {
> >>> + int ret;
> >>> + struct usb_udc *udc = NULL;
> >>> +
> >>> + dev_dbg(>dev, "%s\n", __func__);
> >>> + mutex_lock(_lock);
> >>> + list_for_each_entry(udc, _list, list)
> >>> + if (udc->gadget == gadget)
> >>> + goto found;
> >>> +
> >>> + dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
> >>> + __func__);
> >>> + mutex_unlock(_lock);
> >>> + return -EINVAL;
> >>> +
> >>> +found:
> >>> + ret = usb_gadget_udc_start(udc);
> >>> + if (ret)
> >>> + dev_err(>dev, "USB Device Controller didn't
> >> start: %d\n",
> >>> + ret);
> >>> + else
> >>> + usb_udc_connect_control(udc);
> >>
> >> For drd, it's fine, but for real otg, gadget connect should be
> >> done by
> >> loc_conn() instead of gadget start.
> >
> > It is upto the OTG state machine to call gadget_start() when it
> > needs to connect to the bus (i.e. loc_conn()). I see no point in
> > calling gadget start before.
> >
> > Do you see any issue in doing so?
> 
>  This is what OTG state machine does:
>  case OTG_STATE_B_PERIPHERAL:
>   otg_chrg_vbus(otg, 0);
>   otg_loc_sof(otg, 0);
>   otg_set_protocol(fsm, PROTO_GADGET);
>   otg_loc_conn(otg, 1);
>   break;
> >>
> >> On second thoughts, after seen the OTG state machine.
> >> otg_set_protocol(fsm, PROTO_GADGET); is always followed by
> >> otg_loc_conn(otg, 1); And whenever protocol changes to anything other
> >> the PROTO_GADGET, we use otg_loc_conn(otg, 0);
> >>
> >> So otg_loc_conn seems redundant. Can we just get rid of it?
> >>
> >> usb_gadget_start() implies that gadget controller starts up and
> >> enables pull.
> >> usb_gadget_stop() implies that gadget controller disables pull and
> stops.
> >>
> >>
> >> Can you please explain why just these 2 APIs are not sufficient for
> >> full OTG?
> >>
> >> Do we want anything to happen between gadget controller start/stop
> >> and pull on/off?
> >
> > "loc_conn" is a standard output parameter in OTG spec, it deserves a
> > separate api, yes, current implementation of OTG state machine code
> > seems allow you to combine the 2 things into one, but don't do that,
> > because they do not always happen together, e.g. for peripheral only B
> > device (also a part OTG spec: section 7.3), will be fixed in gadget
> > mode, but it will do gadget connect and disconnect in its diff states,
> > so, to make the framework common, let's keep them separated.
> 
> I'm sorry but I didn't understand your comment about "it will do gadget
> connect and disconnect in its diff states"

Gadget connect means loc_conn(1).

> 
> I am reading the OTG v2.0 specification and loc_conn is always true when
> b_peripheral or a_peripheral is true and false otherwise.

If you only talk about these 2 states, yes, loc_conn is ture.

> 
> loc_conn is just an internal state variable and it corresponds to our
> gadget_start/stop() state.

It's not an internal variable, there are OTG state machine
parameters tables(table 7-x) in OTG spec which have clear lists
which are "internal variable", which are "input", which are "output"...

Those APIs are driven directly from OTG spec, easily understood so
code reader can know what's those APIs for. For real OTG, I don't
see the benefit if get rid of it.

> 
> As per 7.4.2.3
> "loc_conn
> The "local connect" (loc_conn) variable is TRUE when the local device has
> signaled that it is connected to the bus. This variable is FALSE when the
> local device has signaled that it is disconnected from the bus"
> 
> Can you please point me in the specification if there is any place where
> loc_conn is false and b_peripheral/a_peripheral is true?
> 
> cheers,
> -roger
--
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] usb: ehci-platform: add reset controller number in struct ehci_platform_priv

2016-05-03 Thread Jiancheng Xue
Some ehci compatible controllers have more than one reset signal lines,
e.g., Synopsys DWC USB2.0 Host-AHB Controller has two resets hreset_i_n
and phy_rst_i_n. Two more resets are added in this patch in order for
this kind of controller to use this driver directly.

Signed-off-by: Jiancheng Xue 
---
Change Log:
v2:
-Fixed a bug pointed by Alan Stern.
 Called reset_control_put() for the offending reset line.
-Fixed a compiling error.

 drivers/usb/host/ehci-platform.c | 45 +---
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index 1757ebb..bc33f45 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -39,11 +39,12 @@
 
 #define DRIVER_DESC "EHCI generic platform driver"
 #define EHCI_MAX_CLKS 3
+#define EHCI_MAX_RSTS 3
 #define hcd_to_ehci_priv(h) ((struct ehci_platform_priv *)hcd_to_ehci(h)->priv)
 
 struct ehci_platform_priv {
struct clk *clks[EHCI_MAX_CLKS];
-   struct reset_control *rst;
+   struct reset_control *rsts[EHCI_MAX_RSTS];
struct phy **phys;
int num_phys;
bool reset_on_resume;
@@ -149,7 +150,7 @@ static int ehci_platform_probe(struct platform_device *dev)
struct usb_ehci_pdata *pdata = dev_get_platdata(>dev);
struct ehci_platform_priv *priv;
struct ehci_hcd *ehci;
-   int err, irq, phy_num, clk = 0;
+   int err, irq, phy_num, clk = 0, rst;
 
if (usb_disabled())
return -ENODEV;
@@ -234,16 +235,22 @@ static int ehci_platform_probe(struct platform_device 
*dev)
}
}
 
-   priv->rst = devm_reset_control_get_optional(>dev, NULL);
-   if (IS_ERR(priv->rst)) {
-   err = PTR_ERR(priv->rst);
-   if (err == -EPROBE_DEFER)
-   goto err_put_clks;
-   priv->rst = NULL;
-   } else {
-   err = reset_control_deassert(priv->rst);
-   if (err)
-   goto err_put_clks;
+   for (rst = 0; rst < EHCI_MAX_RSTS; rst++) {
+   priv->rsts[rst] = of_reset_control_get_by_index(
+   dev->dev.of_node, rst);
+   if (IS_ERR(priv->rsts[rst])) {
+   err = PTR_ERR(priv->rsts[rst]);
+   if (err == -EPROBE_DEFER)
+   goto err_reset;
+   priv->rsts[rst] = NULL;
+   break;
+   }
+
+   err = reset_control_deassert(priv->rsts[rst]);
+   if (err) {
+   reset_control_put(priv->rsts[rst]);
+   goto err_reset;
+   }
}
 
if (pdata->big_endian_desc)
@@ -300,8 +307,10 @@ err_power:
if (pdata->power_off)
pdata->power_off(dev);
 err_reset:
-   if (priv->rst)
-   reset_control_assert(priv->rst);
+   while (--rst >= 0) {
+   reset_control_assert(priv->rsts[rst]);
+   reset_control_put(priv->rsts[rst]);
+   }
 err_put_clks:
while (--clk >= 0)
clk_put(priv->clks[clk]);
@@ -319,15 +328,17 @@ static int ehci_platform_remove(struct platform_device 
*dev)
struct usb_hcd *hcd = platform_get_drvdata(dev);
struct usb_ehci_pdata *pdata = dev_get_platdata(>dev);
struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
-   int clk;
+   int clk, rst;
 
usb_remove_hcd(hcd);
 
if (pdata->power_off)
pdata->power_off(dev);
 
-   if (priv->rst)
-   reset_control_assert(priv->rst);
+   for (rst = 0; rst < EHCI_MAX_RSTS && priv->rsts[rst]; rst++) {
+   reset_control_assert(priv->rsts[rst]);
+   reset_control_put(priv->rsts[rst]);
+   }
 
for (clk = 0; clk < EHCI_MAX_CLKS && priv->clks[clk]; clk++)
clk_put(priv->clks[clk]);
-- 
1.9.1

--
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: misc: usbtest: fix pattern tests for scatterlists.

2016-05-03 Thread Felipe Balbi

Hi,

Alan Stern  writes:
> On Mon, 2 May 2016, Mathias Nyman wrote:
>
>> The current implemenentation restart the sent pattern for each entry in
>> the sg list. The receiving end expects a continuous pattern, and test
>> will fail unless scatterilst entries happen to be aligned with the
>> pattern
>
> Ah.  We didn't spot this earlier because for non-xHCI controllers, the 
> scatterlist entries _must_ be aligned with the maxpacket size and 
> therefore with the pattern.

interesting. We actually found a similar issue with XHCI. scatterlist
has to be aligned to wMaxPacketSize but only before a link TRB. Mathias
has been working on a solution which involves memcpy()ing enough bytes
to align to wMaxPacketSize before the link TRB (it's very infrequent as
we have 256 TRBs per segment on XHCI), but if you know of a nicer way,
we're all ears :-)

/me goes dig EHCI

-- 
balbi


signature.asc
Description: PGP signature


Re: add device id to linux kernel

2016-05-03 Thread Felipe Balbi

Hi,

"gre...@linuxfoundation.org"  writes:
> On Mon, May 02, 2016 at 09:22:57AM +0300, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> 刘磊  writes:
>> > dear linuxfoundation:
>> > I am liulei come from ZTEMT.  We need add some device ID to linux 
>> > kernel, We need some help from yours. 
>> > Looking forward to you reply, thanks!
>> >
>> >
>> > Signed-off-by:lei liu
>
> 
>
>> Do all these devices really exist ? If they do, then please just send
>> this as a proper patch and it should be simple to get it merged. See
>> Documentation/SubmittingPatches for some hints as to how to write proper
>> patches, but basically, your patch needs a commit log with a
>> Signed-off-by line.
>
> There is a signed-off-by line :)

heh, totally missed it without the tearline ;)

-- 
balbi


signature.asc
Description: PGP signature


Re: How to read & back up data of SIM Card using its Reader?

2016-05-03 Thread Felipe Balbi

Hi,

"Susmita/Rajib Bandopadhyay"  writes:
> knoppix@Microknoppix:~$ sudo dmesg -c
> [  421.080059] usb 1-6: new high-speed USB device number 3 using ehci-pci
> [  421.205730] usb 1-6: New USB device found, idVendor=14cd, idProduct=6700
> [  421.205744] usb 1-6: New USB device strings: Mfr=1, Product=3, 
> SerialNumber=2
> [  421.205752] usb 1-6: Product: USB 2.0  SD/MMC READER
> [  421.205759] usb 1-6: Manufacturer: SDMMC M121
> [  421.205765] usb 1-6: SerialNumber: 81282789
> [  421.210050] scsi4 : usb-storage 1-6:1.0
> knoppix@Microknoppix:~$ sudo dmesg -c
> [  422.214410] scsi 4:0:0:0: Direct-Access USB 2.0  SD/MMC Reader
>PQ: 0 ANSI: 0 CCS
> [  422.214781] sd 4:0:0:0: Attached scsi generic sg3 type 0
> [  422.218262] sd 4:0:0:0: [sdc] Attached SCSI removable disk
  ^^^
  see ?

Just like Greg wrote, all you have to do is:

$ mkdir /tmp/x
$ sudo mount /dev/sdc /tmp/x

And it should work. Try that and tell us if you have any problems.

-- 
balbi


signature.asc
Description: PGP signature