Re: [PATCH v4 5/5] usb: gadget: f_uac2: send reasonably sized packets
On Wed, Aug 27, 2014 at 12:20 PM, Daniel Mack wrote:
> On 08/27/2014 06:08 AM, Jassi Brar wrote:
>> On Wed, Aug 27, 2014 at 3:23 AM, Daniel Mack wrote:
>
>>> + uac2->p_interval = (1 << (ep_desc->bInterval - 1)) * factor;
>>> + req_len = rate / uac2->p_interval;
>>>
>> + if (opts->p_srate % uac2->p_interval)
>> + req_len += fsz;
>> .
>>
>>> + uac2->p_residue = 0;
>>> } else {
>>> dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
>>> return -EINVAL;
>>> @@ -1128,7 +1188,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
>>> unsigned alt)
>>>
>>> req->zero = 0;
>>> req->context = &prm->ureq[i];
>>> - req->length = prm->max_psize;
>>> + req->length = req_len;
>>> req->complete = agdev_iso_complete;
>>> req->buf = prm->rbuf + i * req->length;
>>>
>> otherwise req[0]->buf might overlap req[1]->buf's first frame
>> for when we need to send an extra frame.
>
> Hmm? The first USB_XFERS packets will only contain zeros, and we're only
> preparing those here. For every successive packet, the length is
> recalculated and the audio material is copied in accordingly before the
> requets is requeued. What buffers should overlap here?
>
For 44100/2/S16, req_len is 176 or 44 frames. But we need to send 45
frames in a packet occasionally.
req[0]->buf = rbuf + 0 and req[1]->buf = rbuf + 176.
But what if we req[0] needed to carry the packet with 45 frames?
-jassi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 5/5] usb: gadget: f_uac2: send reasonably sized packets
On 08/27/2014 09:07 AM, Jassi Brar wrote: > On Wed, Aug 27, 2014 at 12:20 PM, Daniel Mack wrote: >> Hmm? The first USB_XFERS packets will only contain zeros, and we're only >> preparing those here. For every successive packet, the length is >> recalculated and the audio material is copied in accordingly before the >> requets is requeued. What buffers should overlap here? >> > For 44100/2/S16, req_len is 176 or 44 frames. But we need to send 45 > frames in a packet occasionally. > > req[0]->buf = rbuf + 0 and req[1]->buf = rbuf + 176. No. req[0]->buf = rbuf + 0 and req[1]->buf = rbuf + max_psize. prm->max_psize is still set to wMaxPacketSize of the endpoint. We just decide how much of it to really use dynamically, each time before a packet with real payload is commited. > But what if we req[0] needed to carry the packet with 45 frames? Then we do so in iso_complete(). This is unrelated to the first USB_XFERS dummy packets. I just wanted to avoid sending queuing them with max_psize length. Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 5/5] usb: gadget: f_uac2: send reasonably sized packets
On 08/27/2014 09:15 AM, Daniel Mack wrote: > On 08/27/2014 09:07 AM, Jassi Brar wrote: >> On Wed, Aug 27, 2014 at 12:20 PM, Daniel Mack wrote: > >>> Hmm? The first USB_XFERS packets will only contain zeros, and we're only >>> preparing those here. For every successive packet, the length is >>> recalculated and the audio material is copied in accordingly before the >>> requets is requeued. What buffers should overlap here? >>> >> For 44100/2/S16, req_len is 176 or 44 frames. But we need to send 45 >> frames in a packet occasionally. >> >> req[0]->buf = rbuf + 0 and req[1]->buf = rbuf + 176. > > No. req[0]->buf = rbuf + 0 and req[1]->buf = rbuf + max_psize. > > prm->max_psize is still set to wMaxPacketSize of the endpoint. We just > decide how much of it to really use dynamically, each time before a > packet with real payload is commited. As I said earlier, I decided to do it like this so that we can eventually implement sample rate and format switches during runtime. We wouldn't want to touch the size of allocated buffers when this happens, so let's keep them at the largest packet size that might ever occur on the endpoint. The *actual* size of the each request's buffer is a different thing. Thanks, Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 5/5] usb: gadget: f_uac2: send reasonably sized packets
On Wed, Aug 27, 2014 at 12:45 PM, Daniel Mack wrote: > On 08/27/2014 09:07 AM, Jassi Brar wrote: >> On Wed, Aug 27, 2014 at 12:20 PM, Daniel Mack wrote: > >>> Hmm? The first USB_XFERS packets will only contain zeros, and we're only >>> preparing those here. For every successive packet, the length is >>> recalculated and the audio material is copied in accordingly before the >>> requets is requeued. What buffers should overlap here? >>> >> For 44100/2/S16, req_len is 176 or 44 frames. But we need to send 45 >> frames in a packet occasionally. >> >> req[0]->buf = rbuf + 0 and req[1]->buf = rbuf + 176. > > No. req[0]->buf = rbuf + 0 and req[1]->buf = rbuf + max_psize. > You patch does - req->length = prm->max_psize; + req->length = req_len; Or did you send the wrong version of patch? where req_len is calculated as req_len = rate / uac2->p_interval; Clearly for 44.1/2/S16, req_len evaluates to 176. -jassi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 5/5] usb: gadget: f_uac2: send reasonably sized packets
On Wed, Aug 27, 2014 at 3:23 AM, Daniel Mack wrote:
> @@ -1099,11 +1139,31 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
> unsigned alt)
> prm = &uac2->c_prm;
> config_ep_by_speed(gadget, fn, ep);
> agdev->as_out_alt = alt;
> + req_len = prm->max_psize;
> } else if (intf == agdev->as_in_intf) {
> + struct f_uac2_opts *opts = agdev_to_uac2_opts(agdev);
> + unsigned int rate = opts->p_srate * opts->p_ssize *
> + num_channels(opts->p_chmask);
> + struct usb_endpoint_descriptor *ep_desc;
> + unsigned int factor;
> +
> ep = agdev->in_ep;
> prm = &uac2->p_prm;
> config_ep_by_speed(gadget, fn, ep);
> agdev->as_in_alt = alt;
> +
> + /* pre-calculate the playback endpoint's interval */
> + if (gadget->speed == USB_SPEED_FULL) {
> + ep_desc = &fs_epin_desc;
> + factor = 1000;
> + } else {
> + ep_desc = &hs_epin_desc;
> + factor = 125;
> + }
> +
> + uac2->p_interval = (1 << (ep_desc->bInterval - 1)) * factor;
> + req_len = rate / uac2->p_interval;
>
(a)For 44.1/2/S16, req_len = 176
> + uac2->p_residue = 0;
> } else {
> dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> return -EINVAL;
> @@ -1128,7 +1188,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
> unsigned alt)
>
> req->zero = 0;
> req->context = &prm->ureq[i];
> - req->length = prm->max_psize;
> + req->length = req_len;
(b)req->length = req_len or 176
> req->complete = agdev_iso_complete;
> req->buf = prm->rbuf + i * req->length;
>
Here req[0]->buf is req->length, which is 176 bytes from (b).
I hope this makes it clear.
-jassi
> }
> --
> 2.1.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 5/5] usb: gadget: f_uac2: send reasonably sized packets
On 08/27/2014 09:28 AM, Jassi Brar wrote: > On Wed, Aug 27, 2014 at 12:45 PM, Daniel Mack wrote: >> On 08/27/2014 09:07 AM, Jassi Brar wrote: >>> On Wed, Aug 27, 2014 at 12:20 PM, Daniel Mack wrote: >> Hmm? The first USB_XFERS packets will only contain zeros, and we're only preparing those here. For every successive packet, the length is recalculated and the audio material is copied in accordingly before the requets is requeued. What buffers should overlap here? >>> For 44100/2/S16, req_len is 176 or 44 frames. But we need to send 45 >>> frames in a packet occasionally. >>> >>> req[0]->buf = rbuf + 0 and req[1]->buf = rbuf + 176. >> >> No. req[0]->buf = rbuf + 0 and req[1]->buf = rbuf + max_psize. Note that you are referring to the buffer pointer here. > You patch does > - req->length = prm->max_psize; > + req->length = req_len; Yes, because we tell the udc driver we want to commit req_len bytes in this request. req->buf, however, it set to a buffer that can accommodate wMaxPacketSize, so when the packet completes, we can stuff more bytes into it and modify req->length again. req->length it the request length, not the max size of the buffer we point to. > Clearly for 44.1/2/S16, req_len evaluates to 176. Yes, that's the start condition. Once both the USB and the ALSA side are running, the req->length calculation in iso_complete() kicks in and tweaks the buffer sizes. Does it make sense now? Or am I missing anything? Thanks, Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 5/5] usb: gadget: f_uac2: send reasonably sized packets
On 08/27/2014 09:35 AM, Jassi Brar wrote:
> On Wed, Aug 27, 2014 at 3:23 AM, Daniel Mack wrote:
>> + uac2->p_residue = 0;
>> } else {
>> dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
>> return -EINVAL;
>> @@ -1128,7 +1188,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
>> unsigned alt)
>>
>> req->zero = 0;
>> req->context = &prm->ureq[i];
>> - req->length = prm->max_psize;
>> + req->length = req_len;
> (b)req->length = req_len or 176
>
>> req->complete = agdev_iso_complete;
>> req->buf = prm->rbuf + i * req->length;
>>
> Here req[0]->buf is req->length, which is 176 bytes from (b).
Ah. Sorry, now I see what you mean. Yes, that should be
req->buf = prm->rbuf + i * p_maxsize
Alright, let me spin v5 later.
Thanks,
Daniel
--
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 v5 0/5] usb: gadget: f_uac2: cleanups and capture timing
Hi, this is v4 of the f_uac2 timing fixup series. Changes from v4: * Fix buffer setup in afunc_set_alt() Changes from v3: * add another patch (3/5) to introduce agdev_to_uac2_opts() which is also needed in 5/5 * patch 5/5 only: move from a pre-calculated sequence of packet lengths to an accumulator that sums up the residual values from the packet size calculation and adds an extra sample frame now and then when the accumulator has gathered enough bytes. Changes from v2: * swap path 3 and 4, so that the ALSA buffer wrap around fix comes in first. It's not actually a bug fix for the current code, but more a preparation to allow for smaller packets. * use the p_ssize, p_chmask and p_srate for the length calculations * prepare a sequence of packet sizes to send, and alternate over them when sending the the IN packets. The actual commit log and the comments yield some more details on that. Changes from v1: * drop UAC_EP_CS_ATTR_FILL_MAX approach and rather size the packets correctly * add a patch to fix buffer wrap problems in the ALSA buffer logic, which wasn't needed before The first two patches are just cleanups. Thanks to Alan Stern and Jassi Brar for the feedback! Daniel Daniel Mack (5): usb: gadget: f_uac2: restructure some code in afunc_set_alt() usb: gadget: f_uac2: add short-hand for 'dev' usb: gadget: f_uac2: introduce agdev_to_uac2_opts usb: gadget: f_uac2: handle partial dma area wrap usb: gadget: f_uac2: send reasonably sized packets drivers/usb/gadget/function/f_uac2.c | 164 --- 1 file changed, 115 insertions(+), 49 deletions(-) -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/5] usb: gadget: f_uac2: restructure some code in afunc_set_alt()
Restructure some code to make it easier to read.
While at it, return -ENOMEM instead of -EINVAL if
usb_ep_alloc_request() fails, and omit the logging in such cases
(the mm core will complain loud enough).
Signed-off-by: Daniel Mack
---
drivers/usb/gadget/function/f_uac2.c | 39 +++-
1 file changed, 16 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 0d65e7c..ab4652e 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -1104,31 +1104,24 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
usb_ep_enable(ep);
for (i = 0; i < USB_XFERS; i++) {
- if (prm->ureq[i].req) {
- if (usb_ep_queue(ep, prm->ureq[i].req, GFP_ATOMIC))
- dev_err(&uac2->pdev.dev, "%d Error!\n",
- __LINE__);
- continue;
- }
-
- req = usb_ep_alloc_request(ep, GFP_ATOMIC);
- if (req == NULL) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
- return -EINVAL;
+ if (!prm->ureq[i].req) {
+ req = usb_ep_alloc_request(ep, GFP_ATOMIC);
+ if (req == NULL)
+ return -ENOMEM;
+
+ prm->ureq[i].req = req;
+ prm->ureq[i].pp = prm;
+
+ req->zero = 0;
+ req->context = &prm->ureq[i];
+ req->length = prm->max_psize;
+ req->complete = agdev_iso_complete;
+ req->buf = prm->rbuf + i * req->length;
}
- prm->ureq[i].req = req;
- prm->ureq[i].pp = prm;
-
- req->zero = 0;
- req->context = &prm->ureq[i];
- req->length = prm->max_psize;
- req->complete = agdev_iso_complete;
- req->buf = prm->rbuf + i * req->length;
-
- if (usb_ep_queue(ep, req, GFP_ATOMIC))
- dev_err(&uac2->pdev.dev, "%d Error!\n", __LINE__);
+ if (usb_ep_queue(ep, prm->ureq[i].req, GFP_ATOMIC))
+ dev_err(&uac2->pdev.dev, "%s:%d Error!\n",
+ __func__, __LINE__);
}
return 0;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 2/5] usb: gadget: f_uac2: add short-hand for 'dev'
In afunc_bind() and afunc_set_alt(), &uac2->pdev.dev are used multiple
times. Adding a short-hand for them makes lines shorter so we can
remove some line wraps.
No functional change.
Signed-off-by: Daniel Mack
---
drivers/usb/gadget/function/f_uac2.c | 29 +++--
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index ab4652e..efe8add 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -918,6 +918,7 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
struct snd_uac2_chip *uac2 = &agdev->uac2;
struct usb_composite_dev *cdev = cfg->cdev;
struct usb_gadget *gadget = cdev->gadget;
+ struct device *dev = &uac2->pdev.dev;
struct uac2_rtd_params *prm;
struct f_uac2_opts *uac2_opts;
struct usb_string *us;
@@ -961,8 +962,7 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
ret = usb_interface_id(cfg, fn);
if (ret < 0) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return ret;
}
std_ac_if_desc.bInterfaceNumber = ret;
@@ -971,8 +971,7 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
ret = usb_interface_id(cfg, fn);
if (ret < 0) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return ret;
}
std_as_out_if0_desc.bInterfaceNumber = ret;
@@ -982,8 +981,7 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
ret = usb_interface_id(cfg, fn);
if (ret < 0) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return ret;
}
std_as_in_if0_desc.bInterfaceNumber = ret;
@@ -993,16 +991,14 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
agdev->out_ep = usb_ep_autoconfig(gadget, &fs_epout_desc);
if (!agdev->out_ep) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
goto err;
}
agdev->out_ep->driver_data = agdev;
agdev->in_ep = usb_ep_autoconfig(gadget, &fs_epin_desc);
if (!agdev->in_ep) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
goto err;
}
agdev->in_ep->driver_data = agdev;
@@ -1057,6 +1053,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
struct audio_dev *agdev = func_to_agdev(fn);
struct snd_uac2_chip *uac2 = &agdev->uac2;
struct usb_gadget *gadget = cdev->gadget;
+ struct device *dev = &uac2->pdev.dev;
struct usb_request *req;
struct usb_ep *ep;
struct uac2_rtd_params *prm;
@@ -1064,16 +1061,14 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
/* No i/f has more than 2 alt settings */
if (alt > 1) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return -EINVAL;
}
if (intf == agdev->ac_intf) {
/* Control I/f has only 1 AltSetting - 0 */
if (alt) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return -EINVAL;
}
return 0;
@@ -1090,8 +1085,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
config_ep_by_speed(gadget, fn, ep);
agdev->as_in_alt = alt;
} else {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return -EINVAL;
}
@@ -1120,8 +1114,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
}
if (usb_ep_queue(ep, prm->ureq[i].req, GFP_ATOMIC))
- dev_err(&uac2->pdev.dev, "%s:%d Error!\n",
- __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
}
return 0;
--
2.1.0
--
To unsubscribe
[PATCH v5 3/5] usb: gadget: f_uac2: introduce agdev_to_uac2_opts
Add a simple container_of() wrapper to get a struct f_uac2_opts from a
struct struct audio_dev. Use it in two places where it is currently
open-coded.
Signed-off-by: Daniel Mack
---
drivers/usb/gadget/function/f_uac2.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index efe8add..9c8831d 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -140,6 +140,12 @@ struct snd_uac2_chip *pdev_to_uac2(struct platform_device
*p)
}
static inline
+struct f_uac2_opts *agdev_to_uac2_opts(struct audio_dev *agdev)
+{
+ return container_of(agdev->func.fi, struct f_uac2_opts, func_inst);
+}
+
+static inline
uint num_channels(uint chanmask)
{
uint num = 0;
@@ -1168,7 +1174,7 @@ in_rq_cur(struct usb_function *fn, const struct
usb_ctrlrequest *cr)
int value = -EOPNOTSUPP;
int p_srate, c_srate;
- opts = container_of(agdev->func.fi, struct f_uac2_opts, func_inst);
+ opts = agdev_to_uac2_opts(agdev);
p_srate = opts->p_srate;
c_srate = opts->c_srate;
@@ -1210,7 +1216,7 @@ in_rq_range(struct usb_function *fn, const struct
usb_ctrlrequest *cr)
int value = -EOPNOTSUPP;
int p_srate, c_srate;
- opts = container_of(agdev->func.fi, struct f_uac2_opts, func_inst);
+ opts = agdev_to_uac2_opts(agdev);
p_srate = opts->p_srate;
c_srate = opts->c_srate;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 5/5] usb: gadget: f_uac2: send reasonably sized packets
The UAC2 function driver currently responds to all packets at all times
with wMaxPacketSize packets. That results in way too fast audio
playback as the function driver (which is in fact supposed to define
the audio stream pace) delivers as fast as it can.
Fix this by sizing each packet correctly with the following steps:
a) Set the packet's size by dividing the nominal data rate by the
playback endpoint's interval.q
b) If there is a residual value from the calculation in a), add
it to a accumulator to keep track of it across packets.
c) If the accumulator has gathered at least the number of bytes
that are needed for one sample frame, increase the packet size.
This way, the packet size calculation will get rid of any kind of
imprecision that would otherwise occur with a simple division over
time.
Signed-off-by: Daniel Mack
---
drivers/usb/gadget/function/f_uac2.c | 68 +---
1 file changed, 64 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 246a778..f42c2ad 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -92,6 +92,10 @@ struct snd_uac2_chip {
struct snd_card *card;
struct snd_pcm *pcm;
+
+ /* timekeeping for the playback endpoint */
+ unsigned int p_interval;
+ unsigned int p_residue;
};
#define BUFF_SIZE_MAX (PAGE_SIZE * 16)
@@ -191,8 +195,43 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request
*req)
spin_lock_irqsave(&prm->lock, flags);
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+ struct audio_dev *agdev = uac2_to_agdev(uac2);
+ struct f_uac2_opts *opts = agdev_to_uac2_opts(agdev);
+ unsigned int fsz = opts->p_ssize * num_channels(opts->p_chmask);
+ unsigned int rate = opts->p_srate * fsz;
+
+ /*
+* For each IN packet, calculate the minimum packet size by
+* dividing the nominal bytes per second as required by the
+* current audio format by the endpoint's interval.
+*/
+ req->length = min_t(unsigned int,
+ rate / uac2->p_interval, prm->max_psize);
+
+ /*
+* If there is a residual value from the division, add it to
+* the residue accumulator.
+*/
+ uac2->p_residue += rate % uac2->p_interval;
+
+ /*
+* Whenever there are more bytes in the accumulator than we
+* need to add one more sample frame, increase this packet's
+* size and decrease the accumulator.
+*/
+ if (uac2->p_residue / uac2->p_interval >= fsz) {
+ req->length += fsz;
+ uac2->p_residue -= fsz * uac2->p_interval;
+ }
+
+ /*
+* req->actual is used as copy length below. We can safely
+* override it here as it is not looked at when the packet
+* is resubmitted on an IN endpoint.
+*/
req->actual = req->length;
+ }
pending = prm->hw_ptr % prm->period_size;
pending += req->actual;
@@ -346,6 +385,7 @@ static int uac2_pcm_open(struct snd_pcm_substream
*substream)
c_srate = opts->c_srate;
p_chmask = opts->p_chmask;
c_chmask = opts->c_chmask;
+ uac2->p_residue = 0;
runtime->hw = uac2_pcm_hardware;
@@ -1077,7 +1117,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
struct usb_request *req;
struct usb_ep *ep;
struct uac2_rtd_params *prm;
- int i;
+ int req_len, i;
/* No i/f has more than 2 alt settings */
if (alt > 1) {
@@ -1099,11 +1139,31 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
prm = &uac2->c_prm;
config_ep_by_speed(gadget, fn, ep);
agdev->as_out_alt = alt;
+ req_len = prm->max_psize;
} else if (intf == agdev->as_in_intf) {
+ struct f_uac2_opts *opts = agdev_to_uac2_opts(agdev);
+ unsigned int rate = opts->p_srate * opts->p_ssize *
+ num_channels(opts->p_chmask);
+ struct usb_endpoint_descriptor *ep_desc;
+ unsigned int factor;
+
ep = agdev->in_ep;
prm = &uac2->p_prm;
config_ep_by_speed(gadget, fn, ep);
agdev->as_in_alt = alt;
+
+ /* pre-calculate the playback endpoint's interval */
+ if (gadget->speed == USB_SPEED_FULL) {
+ ep_desc = &fs_epin_desc;
+ factor = 1000;
+ } e
[PATCH v5 4/5] usb: gadget: f_uac2: handle partial dma area wrap
With packet sizes other than 512, payloads in the packets may wrap
around the ALSA dma buffer partially, which leads to memory corruption
and audible clicks and pops in the audio stream at the moment, because
there is no boundary check before the memcpy().
In preparation to an implementation for smaller and dynamically sized
packets, we have to address such cases, and copy the payload in two
steps conditionally.
The 'src' and 'dst' approach doesn't work here anymore, as different
behavior is necessary in playback and capture cases. Thus, this patch
open-codes the routine now.
Signed-off-by: Daniel Mack
---
drivers/usb/gadget/function/f_uac2.c | 32 +++-
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 9c8831d..246a778 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -163,8 +163,8 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request
*req)
{
unsigned pending;
unsigned long flags;
+ unsigned int hw_ptr;
bool update_alsa = false;
- unsigned char *src, *dst;
int status = req->status;
struct uac2_req *ur = req->context;
struct snd_pcm_substream *substream;
@@ -191,26 +191,40 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request
*req)
spin_lock_irqsave(&prm->lock, flags);
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
- src = prm->dma_area + prm->hw_ptr;
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
req->actual = req->length;
- dst = req->buf;
- } else {
- dst = prm->dma_area + prm->hw_ptr;
- src = req->buf;
- }
pending = prm->hw_ptr % prm->period_size;
pending += req->actual;
if (pending >= prm->period_size)
update_alsa = true;
+ hw_ptr = prm->hw_ptr;
prm->hw_ptr = (prm->hw_ptr + req->actual) % prm->dma_bytes;
spin_unlock_irqrestore(&prm->lock, flags);
/* Pack USB load in ALSA ring buffer */
- memcpy(dst, src, req->actual);
+ pending = prm->dma_bytes - hw_ptr;
+
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+ if (unlikely(pending < req->actual)) {
+ memcpy(req->buf, prm->dma_area + hw_ptr, pending);
+ memcpy(req->buf + pending, prm->dma_area,
+ req->actual - pending);
+ } else {
+ memcpy(req->buf, prm->dma_area + hw_ptr, req->actual);
+ }
+ } else {
+ if (unlikely(pending < req->actual)) {
+ memcpy(prm->dma_area + hw_ptr, req->buf, pending);
+ memcpy(prm->dma_area, req->buf + pending,
+ req->actual - pending);
+ } else {
+ memcpy(prm->dma_area + hw_ptr, req->buf, req->actual);
+ }
+ }
+
exit:
if (usb_ep_queue(ep, req, GFP_ATOMIC))
dev_err(&uac2->pdev.dev, "%d Error!\n", __LINE__);
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RES: RES: AS2105-based enclosure size issues with >2TB HDDs
On Tue, 2014-08-26 at 10:47 -0400, Alan Stern wrote: > On Mon, 25 Aug 2014, Oliver Neukum wrote: > > Just set US_FL_NEEDS_CAP16. If READ CAPACITY(16) fails in that case, > > it is clear that something is wrong. It must be set or READ CAPACITY(10) > > alone would be taken as giving a valid answer. > > You have committed a mental layering violation. :-) Indeed. > US_FL_NEEDS_CAP16 is a usb-storage flag. But the capacity > determination is made by the sd driver, which relies on the SCSI > try_rc_10_first flag. If that flag isn't set, sd tries READ > CAPACITY(16) and then falls back to READ CAPACITY(10) if an error > occurs. That's what happened here. > > I don't think we want to add another SCSI flag to say that READ > CAPACITY(10) is unreliable. Why not? It would only be friendly to tell the upper layer of a malfunction if we know about it. > > At that time we are sure that the drive will be unusable unless we > > determine the correct capacity, so we don't make matters worse if we > > crash it. > > Given the difficulty of determining the true capacity, I see two > alternatives. We could set the capacity to a ridiculously large value > (like 1 billion TB), or we could leave the capacity set to the low > value and disable the "block within bounds" checks. Neither of these > is attractive and they both have issues of their own -- although the > second is close to what Windows does. That seems to be the most attractive solution to me. > (For example, udev often tries to read the last sectors of a new drive, > looking for a GPT or RAID signature. That won't work if the capacity > is set to some random value.) Yes, but clipping has its own dangers. Suppose you use the medium without a partition table. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc3: exynos: remove usb_phy_generic support
dwc3 driver is using the new Exynos5 SoC series USB DRD PHY driver
(PHY_EXYNOS5_USBDRD which selects GENERIC_PHY) as can be seen by
looking at the following commits:
7a4cf0fde054 ("ARM: dts: Update DWC3 usb controller to use new
phy driver for exynos5250")
f070267b5fc1 ("ARM: dts: Enable support for DWC3 controller for
exynos5420")
Thus remove unused usb_phy_generic support from dwc3 Exynos glue
layer.
[ The code that is being removed is harmful in the context of
multi_v7_defconfig and enabling "EHCI support for Samsung
S5P/EXYNOS SoC Series" (USB_EHCI_EXYNOS) + "OHCI support for
Samsung S5P/EXYNOS SoC Series" (USB_OHCI_EXYNOS) because "EHCI
support for OMAP3 and later chips" (USB_EHCI_HCD_OMAP) selects
"NOP USB Transceiver Driver" (NOP_USB_XCEIV). NOP USB driver
attaches itself to usb_phy_generic platform devices created by
dwc3 Exynos glue layer and later causes Exynos EHCI driver to
fail probe and Exynos OHCI driver to hang on probe (as observed
on Exynos5250 Arndale board). ]
Cc: Olof Johansson
Cc: Kukjin Kim
Cc: Vivek Gautam
Acked-by: Kyungmin Park
Signed-off-by: Bartlomiej Zolnierkiewicz
---
drivers/usb/dwc3/dwc3-exynos.c | 68 -
1 file changed, 1 insertion(+), 67 deletions(-)
Index: b/drivers/usb/dwc3/dwc3-exynos.c
===
--- a/drivers/usb/dwc3/dwc3-exynos.c2014-08-25 14:57:04.991781925 +0200
+++ b/drivers/usb/dwc3/dwc3-exynos.c2014-08-27 09:16:38.312617727 +0200
@@ -23,15 +23,12 @@
#include
#include
#include
-#include
-#include
+#include
#include
#include
#include
struct dwc3_exynos {
- struct platform_device *usb2_phy;
- struct platform_device *usb3_phy;
struct device *dev;
struct clk *clk;
@@ -39,61 +36,6 @@ struct dwc3_exynos {
struct regulator*vdd10;
};
-static int dwc3_exynos_register_phys(struct dwc3_exynos *exynos)
-{
- struct usb_phy_generic_platform_data pdata;
- struct platform_device *pdev;
- int ret;
-
- memset(&pdata, 0x00, sizeof(pdata));
-
- pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO);
- if (!pdev)
- return -ENOMEM;
-
- exynos->usb2_phy = pdev;
- pdata.type = USB_PHY_TYPE_USB2;
- pdata.gpio_reset = -1;
-
- ret = platform_device_add_data(exynos->usb2_phy, &pdata, sizeof(pdata));
- if (ret)
- goto err1;
-
- pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO);
- if (!pdev) {
- ret = -ENOMEM;
- goto err1;
- }
-
- exynos->usb3_phy = pdev;
- pdata.type = USB_PHY_TYPE_USB3;
-
- ret = platform_device_add_data(exynos->usb3_phy, &pdata, sizeof(pdata));
- if (ret)
- goto err2;
-
- ret = platform_device_add(exynos->usb2_phy);
- if (ret)
- goto err2;
-
- ret = platform_device_add(exynos->usb3_phy);
- if (ret)
- goto err3;
-
- return 0;
-
-err3:
- platform_device_del(exynos->usb2_phy);
-
-err2:
- platform_device_put(exynos->usb3_phy);
-
-err1:
- platform_device_put(exynos->usb2_phy);
-
- return ret;
-}
-
static int dwc3_exynos_remove_child(struct device *dev, void *unused)
{
struct platform_device *pdev = to_platform_device(dev);
@@ -127,12 +69,6 @@ static int dwc3_exynos_probe(struct plat
platform_set_drvdata(pdev, exynos);
- ret = dwc3_exynos_register_phys(exynos);
- if (ret) {
- dev_err(dev, "couldn't register PHYs\n");
- return ret;
- }
-
clk = devm_clk_get(dev, "usbdrd30");
if (IS_ERR(clk)) {
dev_err(dev, "couldn't get clock\n");
@@ -194,8 +130,6 @@ static int dwc3_exynos_remove(struct pla
struct dwc3_exynos *exynos = platform_get_drvdata(pdev);
device_for_each_child(&pdev->dev, NULL, dwc3_exynos_remove_child);
- platform_device_unregister(exynos->usb2_phy);
- platform_device_unregister(exynos->usb3_phy);
clk_disable_unprepare(exynos->clk);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 0/5] usb: gadget: f_uac2: cleanups and capture timing
On Wed, Aug 27, 2014 at 1:15 PM, Daniel Mack wrote: > Hi, > > this is v4 of the f_uac2 timing fixup series. > > Changes from v4: > > * Fix buffer setup in afunc_set_alt() > > Changes from v3: > > * add another patch (3/5) to introduce agdev_to_uac2_opts() > which is also needed in 5/5 > * patch 5/5 only: > move from a pre-calculated sequence of packet lengths to > an accumulator that sums up the residual values from the > packet size calculation and adds an extra sample frame > now and then when the accumulator has gathered enough bytes. > > Changes from v2: > > * swap path 3 and 4, so that the ALSA buffer wrap around fix > comes in first. It's not actually a bug fix for the current > code, but more a preparation to allow for smaller packets. > * use the p_ssize, p_chmask and p_srate for the length > calculations > * prepare a sequence of packet sizes to send, and alternate > over them when sending the the IN packets. The actual commit > log and the comments yield some more details on that. > > Changes from v1: > > * drop UAC_EP_CS_ATTR_FILL_MAX approach and rather size the > packets correctly > * add a patch to fix buffer wrap problems in the ALSA buffer > logic, which wasn't needed before > > The first two patches are just cleanups. > > Thanks to Alan Stern and Jassi Brar for the feedback! > > > Daniel > > Daniel Mack (5): > usb: gadget: f_uac2: restructure some code in afunc_set_alt() > usb: gadget: f_uac2: add short-hand for 'dev' > usb: gadget: f_uac2: introduce agdev_to_uac2_opts > usb: gadget: f_uac2: handle partial dma area wrap > usb: gadget: f_uac2: send reasonably sized packets > Acked-by: Jassi Brar BTW, against which tree is this patchset based? Thanks Jassi -- 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: Buffer I/O error after s2ram with usb storage
Ping
I have got also a problem with a usb sdcard reader (without power cut
during suspend)
[ 1073.606668] PM: Entering mem sleep
[ 1073.606742] Suspending console(s) (use no_console_suspend to debug)
[ 1073.607146] sd 1:0:0:0: [sda] Synchronizing SCSI cache
[ 1073.639076] sd 1:0:0:0: [sda] Stopping disk
[ 1074.186688] PM: suspend of devices complete after 580.127 msecs
[...]
[ 1075.265183] PM: resume of devices complete after 615.990 msecs
[ 1075.265627] PM: Finishing wakeup.
[ 1075.265630] Restarting tasks ...
[...]
[ 1203.404593] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
6, 29065 clusters in bitmap, 32768 in gd; block bitmap corrupt.
[ 1203.404628] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
5, 1601 clusters in bitmap, 32321 in gd; block bitmap corrupt.
[ 1203.404648] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
4, 0 clusters in bitmap, 32768 in gd; block bitmap corrupt.
[ 1203.404667] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
3, 1601 clusters in bitmap, 32321 in gd; block bitmap corrupt.
[ 1203.404686] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
2, 0 clusters in bitmap, 32768 in gd; block bitmap corrupt.
[ 1203.404705] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
1, 1600 clusters in bitmap, 32321 in gd; block bitmap corrupt.
[ 1203.404726] JBD2: Spotted dirty metadata buffer (dev = sdb6, blocknr = 0).
There's a risk of filesystem corruption in case of system crash.
[ 1204.141482] sd 8:0:0:0: [sdb] Media Changed
[ 1204.141490] sd 8:0:0:0: [sdb]
[ 1204.141494] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
[ 1204.141497] sd 8:0:0:0: [sdb]
[ 1204.141499] Sense Key : Unit Attention [current]
[ 1204.141504] Info fld=0x0
[ 1204.141506] sd 8:0:0:0: [sdb]
[ 1204.141510] Add. Sense: Not ready to ready change, medium may have changed
[ 1204.141514] sd 8:0:0:0: [sdb] CDB:
[ 1204.141516] Read(10): 28 00 00 0a 75 f8 00 00 08 00
[ 1204.141526] end_request: I/O error, dev sdb, sector 685560
Le Mon, 28 Apr 2014 15:01:39 +0200,
Matthieu CASTET a écrit :
> Hi,
>
> any news on this.
>
> Matthieu CASTET
>
> Le Tue, 22 Apr 2014 16:01:15 +0200,
> Matthieu CASTET a écrit :
>
> > Hi,
> >
> > while playing with suspend to ram I found a strange behavior with usb
> > key.
> >
> > This can be easily reproduced by doing :
> > - plug a usb key
> > - start to read the usb key : "cat /dev/sdx > /dev/null"
> > - go to suspend : "echo mem > /sys/power/state"
> > - while in suspend, unplug and replug the usb key (simulate usb power
> > loss for computer that keep power)
> > - exit suspend
> > - there is read error on the usb key
> >
> >
> > Because the power was cut during s2ram, the usb stack reset the device
> > <1>.
> > When new data transfer are done, we got a UNIT_ATTENTION from the
> > device <2> and IO error are returned to user space application.
> >
> > After some investigation it seems some (all on the 3 I tested) usb key
> > report as removable, and scsi layer abort the transfer in case of
> > UNIT_ATTENTION <3>.
> >
> > The usb storage driver call scsi_report_bus_reset after device reset,
> > but because of commit dfcf7775 <4>, we don't ignore unit attention if
> > "sshdr.asc == 0x28 && sshdr.ascq == 0x00" ("Not-ready to ready").
> >
> > If dfcf7775 is reverted there is no more Buffer I/O error.
> >
> > Is that possible to find a way to restore the behavior before dfcf7775
> > commit (no Buffer I/O error after device reset) after a suspend to ram ?
> >
> >
> > Matthieu CASTET
> >
> > PS : the same error happen if sg_reset -b /dev/sdx is used on the
> > device.
> >
> >
> > <1>
> > [ 117.070255] usb 2-1.4: reset high-speed USB device number 3 using
> > ehci-pci [...]
> > [ 117.543922] Restarting tasks ... done.
> > [ 117.548390] video LNXVIDEO:01: Restoring backlight state
> > <2>
> > [ 117.549179] sd 6:0:0:0: [sdb] Media Changed
> > [ 117.549184] sd 6:0:0:0: [sdb]
> > [ 117.549187] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
> > [ 117.549189] sd 6:0:0:0: [sdb]
> > [ 117.549191] Sense Key : Unit Attention [current]
> > [ 117.549195] Info fld=0x0
> > [ 117.549197] sd 6:0:0:0: [sdb]
> > [ 117.549201] Add. Sense: Not ready to ready change, medium may have
> > changed [ 117.549203] sd 6:0:0:0: [sdb] CDB:
> > [ 117.549205] Read(10): 28 00 00 02 c9 00 00 00 f0 00
> > [ 117.549212] end_request: I/O error, dev sdb, sector 182528
> > [ 117.549218] Buffer I/O error on device sdb1, logical block 22560
> > [ 117.549225] Buffer I/O error on device sdb1, logical block 22561
> > [ 117.549228] Buffer I/O error on device sdb1, logical block 22562
> > [ 117.549231] Buffer I/O error on device sdb1, logical block 22563
> > [ 117.549233] Buffer I/O error on device sdb1, logical block 22564
> > [ 117.549235] Buffer I/O error on device sdb1, logical block 22565
> > [ 117.549238] Buffer I/O error on device sdb1, logical block 22566
> > [ 117.549240] Buffer I/O error on
Re: [PATCH v5 0/5] usb: gadget: f_uac2: cleanups and capture timing
On 08/27/2014 10:38 AM, Jassi Brar wrote: > On Wed, Aug 27, 2014 at 1:15 PM, Daniel Mack wrote: >> Daniel Mack (5): >> usb: gadget: f_uac2: restructure some code in afunc_set_alt() >> usb: gadget: f_uac2: add short-hand for 'dev' >> usb: gadget: f_uac2: introduce agdev_to_uac2_opts >> usb: gadget: f_uac2: handle partial dma area wrap >> usb: gadget: f_uac2: send reasonably sized packets >> > Acked-by: Jassi Brar Thanks for your continuous review and your feedback! > BTW, against which tree is this patchset based? Felipe's usb-next. Best regards, Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: use $(srctree) instead of $(PWD) for includes
From: Yegor Yefremov Using $(PWD) breaks builds when make was invoked from outside of the kernel tree. Signed-off-by: Yegor Yefremov --- drivers/usb/gadget/Makefile |2 +- drivers/usb/gadget/function/Makefile |4 ++-- drivers/usb/gadget/legacy/Makefile |6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile index a186afe..598a67d 100644 --- a/drivers/usb/gadget/Makefile +++ b/drivers/usb/gadget/Makefile @@ -3,7 +3,7 @@ # subdir-ccflags-$(CONFIG_USB_GADGET_DEBUG) := -DDEBUG subdir-ccflags-$(CONFIG_USB_GADGET_VERBOSE)+= -DVERBOSE_DEBUG -ccflags-y += -I$(PWD)/drivers/usb/gadget/udc +ccflags-y += -I$(srctree)/drivers/usb/gadget/udc obj-$(CONFIG_USB_LIBCOMPOSITE) += libcomposite.o libcomposite-y := usbstring.o config.o epautoconf.o diff --git a/drivers/usb/gadget/function/Makefile b/drivers/usb/gadget/function/Makefile index 6d91f21..61daac6 100644 --- a/drivers/usb/gadget/function/Makefile +++ b/drivers/usb/gadget/function/Makefile @@ -2,8 +2,8 @@ # USB peripheral controller drivers # -ccflags-y := -I$(PWD)/drivers/usb/gadget/ -ccflags-y += -I$(PWD)/drivers/usb/gadget/udc/ +ccflags-y := -I$(srctree)/drivers/usb/gadget/ +ccflags-y += -I$(srctree)/drivers/usb/gadget/udc/ # USB Functions usb_f_acm-y:= f_acm.o diff --git a/drivers/usb/gadget/legacy/Makefile b/drivers/usb/gadget/legacy/Makefile index a11aad5..7f485f2 100644 --- a/drivers/usb/gadget/legacy/Makefile +++ b/drivers/usb/gadget/legacy/Makefile @@ -2,9 +2,9 @@ # USB gadget drivers # -ccflags-y := -I$(PWD)/drivers/usb/gadget/ -ccflags-y += -I$(PWD)/drivers/usb/gadget/udc/ -ccflags-y += -I$(PWD)/drivers/usb/gadget/function/ +ccflags-y := -I$(srctree)/drivers/usb/gadget/ +ccflags-y += -I$(srctree)/drivers/usb/gadget/udc/ +ccflags-y += -I$(srctree)/drivers/usb/gadget/function/ g_zero-y := zero.o g_audio-y := audio.o -- 1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: hub: Prevent hub autosuspend if usbcore.autosuspend is -1
On 08/04/2014 05:07 PM, Alan Stern wrote: > On Mon, 4 Aug 2014, Roger Quadros wrote: > >> If user specifies that USB autosuspend must be disabled by module >> parameter "usbcore.autosuspend=-1" then we must prevent >> autosuspend of USB hub devices as well. >> >> commit 596d789a211d introduced in v3.8 changed the original behaivour >> and stopped respecting the usbcore.autosuspend parameter for hubs. >> >> Fixes: 596d789a211d "USB: set hub's default autosuspend delay as 0" >> >> Cc: [3.8+] >> Signed-off-by: Roger Quadros > > Acked-by: Alan Stern > Just noticed the below error by the Intel's build script > tree: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-linus > head: 039368901ad0a6476c7ecf0cfe4f84d735e30135 > commit: bdd405d2a5287bdb9b04670ea255e1f122138e66 [19/20] usb: hub: Prevent > hub autosuspend if usbcore.autosuspend is -1 > config: make ARCH=sparc64 defconfig > > All error/warnings: > >drivers/usb/core/hub.c: In function 'hub_probe': >>> drivers/usb/core/hub.c:1735:21: error: 'struct dev_pm_info' has no member >>> named 'autosuspend_delay' > if (hdev->dev.power.autosuspend_delay >= 0) > ^ Seems like the "if" has to be placed within a #ifdef CONFIG_PM_RUNTIME. Greg, Should I send a v2 of this patch or a fix on top of your usb-linus branch? 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
Re: [PATCH] xhci: Disable streams on Via XHCI with device-id 0x3432
On 08/26/2014 09:55 PM, Greg Kroah-Hartman wrote: > On Tue, Aug 26, 2014 at 09:35:32AM +0200, Hans de Goede wrote: >> Hi, >> >> On 08/25/2014 08:14 PM, Greg Kroah-Hartman wrote: >>> On Mon, Aug 25, 2014 at 12:21:56PM +0200, Hans de Goede wrote: This is a bit bigger hammer then I would like to use for this, but for now it will have to make do. I'm working on getting my hands on one of these so that I can try to get streams to work (with a quirk flag if necessary) and then we can re-enable them. For now this at least makes uas capable disk enclosures work again by forcing fallback to the usb-storage driver. https://bugzilla.kernel.org/show_bug.cgi?id=79511 Cc: sta...@vger.kernel.org # 3.15 Signed-off-by: Hans de Goede --- drivers/usb/host/xhci-pci.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 687d366..d973682 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -151,6 +151,11 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) if (pdev->vendor == PCI_VENDOR_ID_VIA) xhci->quirks |= XHCI_RESET_ON_RESUME; + /* See https://bugzilla.kernel.org/show_bug.cgi?id=79511 */ + if (pdev->vendor == PCI_VENDOR_ID_VIA && + pdev->device == 0x3432) + xhci->quirks |= XHCI_BROKEN_STREAMS; + if (xhci->quirks & XHCI_RESET_ON_RESUME) xhci_dbg_trace(xhci, trace_xhci_dbg_quirks, "QUIRK: Resetting on resume"); >>> >>> That's harsh :) >>> >>> Do you want this in 3.17-final? >> >> If possible, then yes please. > > Ok. Mathias, can you ack this so I can apply it now? > Acked-by: Mathias Nyman -- 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] xhci: rework cycle bit checking for new dequeue pointers
On 08/21/2014 01:06 AM, Joseph Salisbury wrote:
> On 08/19/2014 08:17 AM, Mathias Nyman wrote:
>> When we manually need to move the TR dequeue pointer we need to set the
>> correct cycle bit as well. Previously we used the trb pointer from the
>> last event received as a base, but this was changed in
>> commit 1f81b6d22a59 ("usb: xhci: Prefer endpoint context dequeue pointer")
>> to use the dequeue pointer from the endpoint context instead
>>
>> It turns out some Asmedia controllers advance the dequeue pointer
>> stored in the endpoint context past the event triggering TRB, and
>> this messed up the way the cycle bit was calculated.
>>
>> Instead of adding a quirk or complicating the already hard to follow cycle
>> bit
>> code, the whole cycle bit calculation is now simplified and adapted to handle
>> event and endpoint context dequeue pointer differences.
>>
>> Fixes: 1f81b6d22a59 ("usb: xhci: Prefer endpoint context dequeue pointer")
>> Reported-by: Maciej Puzio
>> Reported-by: Evan Langlois
>> Reviewed-by: Julius Werner
>> Tested-by: Maciej Puzio
>> Tested-by: Evan Langlois
>> Signed-off-by: Mathias Nyman
>> Cc: sta...@vger.kernel.org
>> ---
>> drivers/usb/host/xhci-ring.c | 101
>> +--
>> drivers/usb/host/xhci.c | 3 ++
>> 2 files changed, 42 insertions(+), 62 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>> index ac8cf23..abed30b 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -364,32 +364,6 @@ static void ring_doorbell_for_active_rings(struct
>> xhci_hcd *xhci,
>> }
>> }
>>
>> -/*
>> - * Find the segment that trb is in. Start searching in start_seg.
>> - * If we must move past a segment that has a link TRB with a toggle cycle
>> state
>> - * bit set, then we will toggle the value pointed at by cycle_state.
>> - */
>> -static struct xhci_segment *find_trb_seg(
>> -struct xhci_segment *start_seg,
>> -union xhci_trb *trb, int *cycle_state)
>> -{
>> -struct xhci_segment *cur_seg = start_seg;
>> -struct xhci_generic_trb *generic_trb;
>> -
>> -while (cur_seg->trbs > trb ||
>> -&cur_seg->trbs[TRBS_PER_SEGMENT - 1] < trb) {
>> -generic_trb = &cur_seg->trbs[TRBS_PER_SEGMENT - 1].generic;
>> -if (generic_trb->field[3] & cpu_to_le32(LINK_TOGGLE))
>> -*cycle_state ^= 0x1;
>> -cur_seg = cur_seg->next;
>> -if (cur_seg == start_seg)
>> -/* Looped over the entire list. Oops! */
>> -return NULL;
>> -}
>> -return cur_seg;
>> -}
>> -
>> -
>> static struct xhci_ring *xhci_triad_to_transfer_ring(struct xhci_hcd *xhci,
>> unsigned int slot_id, unsigned int ep_index,
>> unsigned int stream_id)
>> @@ -459,9 +433,12 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
>> struct xhci_virt_device *dev = xhci->devs[slot_id];
>> struct xhci_virt_ep *ep = &dev->eps[ep_index];
>> struct xhci_ring *ep_ring;
>> -struct xhci_generic_trb *trb;
>> +struct xhci_segment *new_seg;
>> +union xhci_trb *new_deq;
>> dma_addr_t addr;
>> u64 hw_dequeue;
>> +bool cycle_found = false;
>> +bool td_last_trb_found = false;
>>
>> ep_ring = xhci_triad_to_transfer_ring(xhci, slot_id,
>> ep_index, stream_id);
>> @@ -486,45 +463,45 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
>> hw_dequeue = le64_to_cpu(ep_ctx->deq);
>> }
>>
>> -/* Find virtual address and segment of hardware dequeue pointer */
>> -state->new_deq_seg = ep_ring->deq_seg;
>> -state->new_deq_ptr = ep_ring->dequeue;
>> -while (xhci_trb_virt_to_dma(state->new_deq_seg, state->new_deq_ptr)
>> -!= (dma_addr_t)(hw_dequeue & ~0xf)) {
>> -next_trb(xhci, ep_ring, &state->new_deq_seg,
>> -&state->new_deq_ptr);
>> -if (state->new_deq_ptr == ep_ring->dequeue) {
>> -WARN_ON(1);
>> -return;
>> -}
>> -}
>> +new_seg = ep_ring->deq_seg;
>> +new_deq = ep_ring->dequeue;
>> +state->new_cycle_state = hw_dequeue & 0x1;
>> +
>> /*
>> - * Find cycle state for last_trb, starting at old cycle state of
>> - * hw_dequeue. If there is only one segment ring, find_trb_seg() will
>> - * return immediately and cannot toggle the cycle state if this search
>> - * wraps around, so add one more toggle manually in that case.
>> + * We want to find the pointer, segment and cycle state of the new trb
>> + * (the one after current TD's last_trb). We know the cycle state at
>> + * hw_dequeue, so walk the ring until both hw_dequeue and last_trb are
>> + * found.
>> */
>> -state->new_cycle_state = hw_dequeue & 0x1;
>> -if (ep_ring->first_seg == ep_ring->first_seg-
[PATCH v2] usb: hub: Prevent hub autosuspend if usbcore.autosuspend is -1
If user specifies that USB autosuspend must be disabled by module parameter "usbcore.autosuspend=-1" then we must prevent autosuspend of USB hub devices as well. commit 596d789a211d introduced in v3.8 changed the original behaivour and stopped respecting the usbcore.autosuspend parameter for hubs. Fixes: 596d789a211d "USB: set hub's default autosuspend delay as 0" Cc: [3.8+] Signed-off-by: Roger Quadros --- drivers/usb/core/hub.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 8a4dcbc..59b599d 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -1728,8 +1728,14 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id) * - Change autosuspend delay of hub can avoid unnecessary auto * suspend timer for hub, also may decrease power consumption * of USB bus. +* +* - If user has indicated to prevent autosuspend by passing +* usbcore.autosuspend = -1 then keep autosuspend disabled. */ - pm_runtime_set_autosuspend_delay(&hdev->dev, 0); +#ifdef CONFIG_PM_RUNTIME + if (hdev->dev.power.autosuspend_delay >= 0) + pm_runtime_set_autosuspend_delay(&hdev->dev, 0); +#endif /* * Hubs have proper suspend/resume support, except for root hubs -- 1.8.3.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: dwc3: exynos: remove usb_phy_generic support
On Wednesday, August 27, 2014 4:53 PM, Bartlomiej Zolnierkiewicz wrote:
>
> dwc3 driver is using the new Exynos5 SoC series USB DRD PHY driver
> (PHY_EXYNOS5_USBDRD which selects GENERIC_PHY) as can be seen by
> looking at the following commits:
>
> 7a4cf0fde054 ("ARM: dts: Update DWC3 usb controller to use new
> phy driver for exynos5250")
>
> f070267b5fc1 ("ARM: dts: Enable support for DWC3 controller for
> exynos5420")
>
> Thus remove unused usb_phy_generic support from dwc3 Exynos glue
> layer.
>
> [ The code that is being removed is harmful in the context of
> multi_v7_defconfig and enabling "EHCI support for Samsung
> S5P/EXYNOS SoC Series" (USB_EHCI_EXYNOS) + "OHCI support for
> Samsung S5P/EXYNOS SoC Series" (USB_OHCI_EXYNOS) because "EHCI
> support for OMAP3 and later chips" (USB_EHCI_HCD_OMAP) selects
> "NOP USB Transceiver Driver" (NOP_USB_XCEIV). NOP USB driver
> attaches itself to usb_phy_generic platform devices created by
> dwc3 Exynos glue layer and later causes Exynos EHCI driver to
> fail probe and Exynos OHCI driver to hang on probe (as observed
> on Exynos5250 Arndale board). ]
I also agree with this patch, because dwc3 IPs of all Exynos SoCs
do not use "NOP USB Transceiver Driver". So, "usb_phy_generic" can be
removed from Exynos dwc3 driver.
Is there any reason to support 'usb_phy_generic' for Exynos dwc3?
If so, please let me know. Thank you.
Best regards,
Jingoo Han
>
> Cc: Olof Johansson
> Cc: Kukjin Kim
> Cc: Vivek Gautam
> Acked-by: Kyungmin Park
> Signed-off-by: Bartlomiej Zolnierkiewicz
> ---
> drivers/usb/dwc3/dwc3-exynos.c | 68
> -
> 1 file changed, 1 insertion(+), 67 deletions(-)
>
> Index: b/drivers/usb/dwc3/dwc3-exynos.c
> ===
> --- a/drivers/usb/dwc3/dwc3-exynos.c 2014-08-25 14:57:04.991781925 +0200
> +++ b/drivers/usb/dwc3/dwc3-exynos.c 2014-08-27 09:16:38.312617727 +0200
> @@ -23,15 +23,12 @@
> #include
> #include
> #include
> -#include
> -#include
> +#include
> #include
> #include
> #include
>
> struct dwc3_exynos {
> - struct platform_device *usb2_phy;
> - struct platform_device *usb3_phy;
> struct device *dev;
>
> struct clk *clk;
> @@ -39,61 +36,6 @@ struct dwc3_exynos {
> struct regulator*vdd10;
> };
>
> -static int dwc3_exynos_register_phys(struct dwc3_exynos *exynos)
> -{
> - struct usb_phy_generic_platform_data pdata;
> - struct platform_device *pdev;
> - int ret;
> -
> - memset(&pdata, 0x00, sizeof(pdata));
> -
> - pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO);
> - if (!pdev)
> - return -ENOMEM;
> -
> - exynos->usb2_phy = pdev;
> - pdata.type = USB_PHY_TYPE_USB2;
> - pdata.gpio_reset = -1;
> -
> - ret = platform_device_add_data(exynos->usb2_phy, &pdata, sizeof(pdata));
> - if (ret)
> - goto err1;
> -
> - pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO);
> - if (!pdev) {
> - ret = -ENOMEM;
> - goto err1;
> - }
> -
> - exynos->usb3_phy = pdev;
> - pdata.type = USB_PHY_TYPE_USB3;
> -
> - ret = platform_device_add_data(exynos->usb3_phy, &pdata, sizeof(pdata));
> - if (ret)
> - goto err2;
> -
> - ret = platform_device_add(exynos->usb2_phy);
> - if (ret)
> - goto err2;
> -
> - ret = platform_device_add(exynos->usb3_phy);
> - if (ret)
> - goto err3;
> -
> - return 0;
> -
> -err3:
> - platform_device_del(exynos->usb2_phy);
> -
> -err2:
> - platform_device_put(exynos->usb3_phy);
> -
> -err1:
> - platform_device_put(exynos->usb2_phy);
> -
> - return ret;
> -}
> -
> static int dwc3_exynos_remove_child(struct device *dev, void *unused)
> {
> struct platform_device *pdev = to_platform_device(dev);
> @@ -127,12 +69,6 @@ static int dwc3_exynos_probe(struct plat
>
> platform_set_drvdata(pdev, exynos);
>
> - ret = dwc3_exynos_register_phys(exynos);
> - if (ret) {
> - dev_err(dev, "couldn't register PHYs\n");
> - return ret;
> - }
> -
> clk = devm_clk_get(dev, "usbdrd30");
> if (IS_ERR(clk)) {
> dev_err(dev, "couldn't get clock\n");
> @@ -194,8 +130,6 @@ static int dwc3_exynos_remove(struct pla
> struct dwc3_exynos *exynos = platform_get_drvdata(pdev);
>
> device_for_each_child(&pdev->dev, NULL, dwc3_exynos_remove_child);
> - platform_device_unregister(exynos->usb2_phy);
> - platform_device_unregister(exynos->usb3_phy);
>
> clk_disable_unprepare(exynos->clk);
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: net2280: Remove pci_class from PCI_TABLE
Defining the vendor and the product id should be enough to discriminate
the device.
The reason for this patch is that there is a missmatch betweed the
modalias showed by sysfs and the modalias generated by file2alias.
One expects the programming interface in uppercase and the other
generates it in lowercase.
This means that some implementations modprobe will fail to load the
driver.
Signed-off-by: Ricardo Ribalda Delgado
---
drivers/usb/gadget/udc/net2280.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
index f4eac11..542ab89 100644
--- a/drivers/usb/gadget/udc/net2280.c
+++ b/drivers/usb/gadget/udc/net2280.c
@@ -3770,16 +3770,12 @@ static void net2280_shutdown(struct pci_dev *pdev)
/*-*/
static const struct pci_device_id pci_ids[] = { {
- .class =((PCI_CLASS_SERIAL_USB << 8) | 0xfe),
- .class_mask = ~0,
.vendor = PCI_VENDOR_ID_PLX_LEGACY,
.device = 0x2280,
.subvendor =PCI_ANY_ID,
.subdevice =PCI_ANY_ID,
.driver_data = PLX_LEGACY | PLX_2280,
}, {
- .class =((PCI_CLASS_SERIAL_USB << 8) | 0xfe),
- .class_mask = ~0,
.vendor = PCI_VENDOR_ID_PLX_LEGACY,
.device = 0x2282,
.subvendor =PCI_ANY_ID,
@@ -3787,8 +3783,6 @@ static const struct pci_device_id pci_ids[] = { {
.driver_data = PLX_LEGACY,
},
{
- .class =((PCI_CLASS_SERIAL_USB << 8) | 0xfe),
- .class_mask = ~0,
.vendor = PCI_VENDOR_ID_PLX,
.device = 0x3380,
.subvendor =PCI_ANY_ID,
@@ -3796,8 +3790,6 @@ static const struct pci_device_id pci_ids[] = { {
.driver_data = PLX_SUPERSPEED,
},
{
- .class =((PCI_CLASS_SERIAL_USB << 8) | 0xfe),
- .class_mask = ~0,
.vendor = PCI_VENDOR_ID_PLX,
.device = 0x3382,
.subvendor =PCI_ANY_ID,
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/2] usb: gadget: Refactor request completion
All USB peripheral controller drivers called completion routines
directly. This patch moves the completion call from drivers to
usb_gadget_giveback_request(), in order to have a place where common
functionality can be added.
All places in drivers/usb/ matching "[-.]complete(" were replaced with a
call to usb_gadget_giveback_request(). This was compile-tested with all
ARM drivers enabled and runtime-tested for musb.
Signed-off-by: Michal Sojka
---
drivers/usb/chipidea/udc.c | 6 +++---
drivers/usb/dwc2/gadget.c | 6 +++---
drivers/usb/dwc3/gadget.c | 2 +-
drivers/usb/gadget/udc/amd5536udc.c | 2 +-
drivers/usb/gadget/udc/at91_udc.c | 2 +-
drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++--
drivers/usb/gadget/udc/bcm63xx_udc.c| 2 +-
drivers/usb/gadget/udc/dummy_hcd.c | 10 +-
drivers/usb/gadget/udc/fotg210-udc.c| 2 +-
drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +-
drivers/usb/gadget/udc/fsl_udc_core.c | 6 ++
drivers/usb/gadget/udc/fusb300_udc.c| 2 +-
drivers/usb/gadget/udc/goku_udc.c | 2 +-
drivers/usb/gadget/udc/gr_udc.c | 2 +-
drivers/usb/gadget/udc/lpc32xx_udc.c| 2 +-
drivers/usb/gadget/udc/m66592-udc.c | 2 +-
drivers/usb/gadget/udc/mv_u3d_core.c| 8 ++--
drivers/usb/gadget/udc/mv_udc_core.c| 8 ++--
drivers/usb/gadget/udc/net2272.c| 2 +-
drivers/usb/gadget/udc/net2280.c| 2 +-
drivers/usb/gadget/udc/omap_udc.c | 2 +-
drivers/usb/gadget/udc/pch_udc.c| 2 +-
drivers/usb/gadget/udc/pxa25x_udc.c | 2 +-
drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
drivers/usb/gadget/udc/r8a66597-udc.c | 2 +-
drivers/usb/gadget/udc/s3c-hsudc.c | 3 +--
drivers/usb/gadget/udc/s3c2410_udc.c| 2 +-
drivers/usb/gadget/udc/udc-core.c | 9 +
drivers/usb/musb/musb_gadget.c | 2 +-
drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
include/linux/usb/gadget.h | 8
31 files changed, 58 insertions(+), 56 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index b8125aa..0444d3f 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -627,7 +627,7 @@ __acquires(hwep->lock)
if (hwreq->req.complete != NULL) {
spin_unlock(hwep->lock);
- hwreq->req.complete(&hwep->ep, &hwreq->req);
+ usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
spin_lock(hwep->lock);
}
}
@@ -922,7 +922,7 @@ __acquires(hwep->lock)
if ((hwep->type == USB_ENDPOINT_XFER_CONTROL) &&
hwreq->req.length)
hweptemp = hwep->ci->ep0in;
- hwreq->req.complete(&hweptemp->ep, &hwreq->req);
+ usb_gadget_giveback_request(&hweptemp->ep, &hwreq->req);
spin_lock(hwep->lock);
}
}
@@ -1347,7 +1347,7 @@ static int ep_dequeue(struct usb_ep *ep, struct
usb_request *req)
if (hwreq->req.complete != NULL) {
spin_unlock(hwep->lock);
- hwreq->req.complete(&hwep->ep, &hwreq->req);
+ usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
spin_lock(hwep->lock);
}
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0ba9c33..5a524a6 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -987,8 +987,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg
*hsotg,
hs_req = ep->req;
ep->req = NULL;
list_del_init(&hs_req->queue);
- hs_req->req.complete(&ep->ep,
-&hs_req->req);
+ usb_gadget_giveback_request(&ep->ep,
+
&hs_req->req);
}
/* If we have pending request, then start it */
@@ -1245,7 +1245,7 @@ static void s3c_hsotg_complete_request(struct s3c_hsotg
*hsotg,
if (hs_req->req.complete) {
spin_unlock(&hsotg->lock);
- hs_req->req.complete(&hs_ep->ep, &hs_req->req);
+ usb_gadget_giveback_request(&hs_ep->ep, &hs_req->req);
spin_lock(&hsotg->lock);
}
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 349cacc..b4b7a6b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -268,7 +268,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct
dwc3_request *req,
req->request.length, status);
spin_unlock(&dwc->lock);
- req->
[PATCH v3 0/2] LED triggers for USB host and device
This adds LED triggers for USB host and device. First patch refactors UDC drivers as requested by Felipe Balbi, second patch adds the LED triggers. Changes from v2: - host/gadget triggers merged to a single file in usb/common/ - udc drivers refactored so that LED trigger works for all of them Changes from v1: - Moved from drivers/leds/ to drivers/usb/ - Improved Kconfig help - Linked with other modules rather than being standalone modules Michal Sojka (2): usb: gadget: Refactor request completion usb: Add LED triggers for USB activity drivers/usb/Kconfig | 11 +++ drivers/usb/chipidea/udc.c | 6 +- drivers/usb/common/Makefile | 5 +- drivers/usb/common/common.c | 144 drivers/usb/common/led.c| 59 + drivers/usb/common/usb-common.c | 144 drivers/usb/core/hcd.c | 2 + drivers/usb/dwc2/gadget.c | 6 +- drivers/usb/dwc3/gadget.c | 2 +- drivers/usb/gadget/udc/amd5536udc.c | 2 +- drivers/usb/gadget/udc/at91_udc.c | 2 +- drivers/usb/gadget/udc/atmel_usba_udc.c | 4 +- drivers/usb/gadget/udc/bcm63xx_udc.c| 2 +- drivers/usb/gadget/udc/dummy_hcd.c | 10 +-- drivers/usb/gadget/udc/fotg210-udc.c| 2 +- drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +- drivers/usb/gadget/udc/fsl_udc_core.c | 6 +- drivers/usb/gadget/udc/fusb300_udc.c| 2 +- drivers/usb/gadget/udc/goku_udc.c | 2 +- drivers/usb/gadget/udc/gr_udc.c | 2 +- drivers/usb/gadget/udc/lpc32xx_udc.c| 2 +- drivers/usb/gadget/udc/m66592-udc.c | 2 +- drivers/usb/gadget/udc/mv_u3d_core.c| 8 +- drivers/usb/gadget/udc/mv_udc_core.c| 8 +- drivers/usb/gadget/udc/net2272.c| 2 +- drivers/usb/gadget/udc/net2280.c| 2 +- drivers/usb/gadget/udc/omap_udc.c | 2 +- drivers/usb/gadget/udc/pch_udc.c| 2 +- drivers/usb/gadget/udc/pxa25x_udc.c | 2 +- drivers/usb/gadget/udc/pxa27x_udc.c | 2 +- drivers/usb/gadget/udc/r8a66597-udc.c | 2 +- drivers/usb/gadget/udc/s3c-hsudc.c | 3 +- drivers/usb/gadget/udc/s3c2410_udc.c| 2 +- drivers/usb/gadget/udc/udc-core.c | 13 +++ drivers/usb/musb/musb_gadget.c | 2 +- drivers/usb/renesas_usbhs/mod_gadget.c | 2 +- include/linux/usb.h | 12 +++ include/linux/usb/gadget.h | 8 ++ 38 files changed, 294 insertions(+), 201 deletions(-) create mode 100644 drivers/usb/common/common.c create mode 100644 drivers/usb/common/led.c delete mode 100644 drivers/usb/common/usb-common.c -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] usb: Add LED triggers for USB activity
With this patch, USB activity can be signaled by blinking a LED. There
are two triggers, one for activity on USB host and one for USB gadget.
Both trigger should work with all host/device controllers. Tested only
with musb.
Signed-off-by: Michal Sojka
---
drivers/usb/Kconfig | 11 ++
drivers/usb/common/Makefile | 5 ++-
drivers/usb/common/{usb-common.c => common.c} | 0
drivers/usb/common/led.c | 56 +++
drivers/usb/core/hcd.c| 2 +
drivers/usb/gadget/udc/udc-core.c | 4 ++
include/linux/usb.h | 12 ++
7 files changed, 89 insertions(+), 1 deletion(-)
rename drivers/usb/common/{usb-common.c => common.c} (100%)
create mode 100644 drivers/usb/common/led.c
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index e0cad441..fc90cda 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -147,4 +147,15 @@ source "drivers/usb/phy/Kconfig"
source "drivers/usb/gadget/Kconfig"
+config USB_LED_TRIG
+ bool "USB LED Triggers"
+ depends on LEDS_CLASS && USB_COMMON
+ select LEDS_TRIGGERS
+ help
+ This option adds LED triggers for USB host and/or gadget activity.
+
+ Say Y here if you are working on a system with led-class supported
+ LEDs and you want to use them as activity indicators for USB host or
+ gadget.
+
endif # USB_SUPPORT
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 7526461..ca2f8bd 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -2,5 +2,8 @@
# Makefile for the usb common parts.
#
-obj-$(CONFIG_USB_COMMON) += usb-common.o
+obj-$(CONFIG_USB_COMMON) += usb-common.o
+usb-common-y += common.o
+usb-common-$(CONFIG_USB_LED_TRIG) += led.o
+
obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
diff --git a/drivers/usb/common/usb-common.c b/drivers/usb/common/common.c
similarity index 100%
rename from drivers/usb/common/usb-common.c
rename to drivers/usb/common/common.c
diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
new file mode 100644
index 000..af3ce2c
--- /dev/null
+++ b/drivers/usb/common/led.c
@@ -0,0 +1,56 @@
+/*
+ * LED Triggers for USB Activity
+ *
+ * Copyright 2014 Michal Sojka
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#define BLINK_DELAY 30
+
+static unsigned long usb_blink_delay = BLINK_DELAY;
+
+DEFINE_LED_TRIGGER(ledtrig_usb_gadget);
+DEFINE_LED_TRIGGER(ledtrig_usb_host);
+
+void usb_led_activity(enum usb_led_event ev)
+{
+ struct led_trigger *trig = NULL;
+
+ switch (ev) {
+ case USB_LED_EVENT_GADGET: trig = ledtrig_usb_gadget; break;
+ case USB_LED_EVENT_HOST: trig = ledtrig_usb_host;break;
+ }
+ led_trigger_blink_oneshot(trig, &usb_blink_delay, &usb_blink_delay, 0);
+}
+EXPORT_SYMBOL(usb_led_activity);
+
+
+static int __init ledtrig_usb_init(void)
+{
+#ifdef CONFIG_USB_GADGET
+ led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
+#endif
+#ifdef CONFIG_USB
+ led_trigger_register_simple("usb-host", &ledtrig_usb_host);
+#endif
+ return 0;
+}
+
+static void __exit ledtrig_usb_exit(void)
+{
+ led_trigger_unregister_simple(ledtrig_usb_gadget);
+ led_trigger_unregister_simple(ledtrig_usb_host);
+}
+
+module_init(ledtrig_usb_init);
+module_exit(ledtrig_usb_exit);
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 487abcf..409cb95 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
usbmon_urb_complete(&hcd->self, urb, status);
usb_anchor_suspend_wakeups(anchor);
usb_unanchor_urb(urb);
+ if (likely(status == 0))
+ usb_led_activity(USB_LED_EVENT_HOST);
/* pass ownership to the completion handler */
urb->status = status;
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index 2eb0ae4..2ce42f9 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -27,6 +27,7 @@
#include
#include
+#include
/**
* struct usb_udc - describes one usb device controller
@@ -105,6 +106,9 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
void usb_gadget_giveback_request(struct usb_ep *ep,
struct usb_request *req)
{
+ if (likely(req->status == 0))
+ usb_led_activity(USB_LED_EVENT_GADGET);
+
/* complete() is from gadget layer,
* eg fsg->bulk_in_complete() */
if (req->complete)
diff --git a/include/linux/usb.h b/include/linux/usb.h
index d2465bc..447a7e2 100644
--- a/include/linux/usb.h
+++ b/includ
Re: Buffer I/O error after s2ram with usb storage
On 14-08-27 04:40 AM, Matthieu CASTET wrote:
Ping
I have got also a problem with a usb sdcard reader (without power cut
during suspend)
[ 1073.606668] PM: Entering mem sleep
[ 1073.606742] Suspending console(s) (use no_console_suspend to debug)
[ 1073.607146] sd 1:0:0:0: [sda] Synchronizing SCSI cache
[ 1073.639076] sd 1:0:0:0: [sda] Stopping disk
[ 1074.186688] PM: suspend of devices complete after 580.127 msecs
[...]
[ 1075.265183] PM: resume of devices complete after 615.990 msecs
[ 1075.265627] PM: Finishing wakeup.
[ 1075.265630] Restarting tasks ...
[...]
[ 1203.404593] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
6, 29065 clusters in bitmap, 32768 in gd; block bitmap corrupt.
[ 1203.404628] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
5, 1601 clusters in bitmap, 32321 in gd; block bitmap corrupt.
[ 1203.404648] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
4, 0 clusters in bitmap, 32768 in gd; block bitmap corrupt.
[ 1203.404667] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
3, 1601 clusters in bitmap, 32321 in gd; block bitmap corrupt.
[ 1203.404686] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
2, 0 clusters in bitmap, 32768 in gd; block bitmap corrupt.
[ 1203.404705] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
1, 1600 clusters in bitmap, 32321 in gd; block bitmap corrupt.
[ 1203.404726] JBD2: Spotted dirty metadata buffer (dev = sdb6, blocknr = 0).
There's a risk of filesystem corruption in case of system crash.
[ 1204.141482] sd 8:0:0:0: [sdb] Media Changed
[ 1204.141490] sd 8:0:0:0: [sdb]
[ 1204.141494] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
[ 1204.141497] sd 8:0:0:0: [sdb]
[ 1204.141499] Sense Key : Unit Attention [current]
[ 1204.141504] Info fld=0x0
[ 1204.141506] sd 8:0:0:0: [sdb]
[ 1204.141510] Add. Sense: Not ready to ready change, medium may have changed
The unit attention doesn't look like a problem, it
looks correct. If the system is unable to detect
removable media being changed while the system is
suspended, then
If the media has a unique identifier, then this unit
attention at wakeup should trigger sd to make sure
that unique identifier has not changed.
Not sure why ext4 starts looking at sdb6 _before_ the
sd driver processes that unit attention. Perhaps a
TEST UNIT READY should be done earlier in the wake-up
sequence to flush out (and process) unit attentions.
There is also the case in which the removable media is
no longer present; and that should change EXT4-fs
processing to a surprise removal.
Doug Gilbert
[ 1204.141514] sd 8:0:0:0: [sdb] CDB:
[ 1204.141516] Read(10): 28 00 00 0a 75 f8 00 00 08 00
[ 1204.141526] end_request: I/O error, dev sdb, sector 685560
Le Mon, 28 Apr 2014 15:01:39 +0200,
Matthieu CASTET a écrit :
Hi,
any news on this.
Matthieu CASTET
Le Tue, 22 Apr 2014 16:01:15 +0200,
Matthieu CASTET a écrit :
Hi,
while playing with suspend to ram I found a strange behavior with usb
key.
This can be easily reproduced by doing :
- plug a usb key
- start to read the usb key : "cat /dev/sdx > /dev/null"
- go to suspend : "echo mem > /sys/power/state"
- while in suspend, unplug and replug the usb key (simulate usb power
loss for computer that keep power)
- exit suspend
- there is read error on the usb key
Because the power was cut during s2ram, the usb stack reset the device
<1>.
When new data transfer are done, we got a UNIT_ATTENTION from the
device <2> and IO error are returned to user space application.
After some investigation it seems some (all on the 3 I tested) usb key
report as removable, and scsi layer abort the transfer in case of
UNIT_ATTENTION <3>.
The usb storage driver call scsi_report_bus_reset after device reset,
but because of commit dfcf7775 <4>, we don't ignore unit attention if
"sshdr.asc == 0x28 && sshdr.ascq == 0x00" ("Not-ready to ready").
If dfcf7775 is reverted there is no more Buffer I/O error.
Is that possible to find a way to restore the behavior before dfcf7775
commit (no Buffer I/O error after device reset) after a suspend to ram ?
Matthieu CASTET
PS : the same error happen if sg_reset -b /dev/sdx is used on the
device.
<1>
[ 117.070255] usb 2-1.4: reset high-speed USB device number 3 using
ehci-pci [...]
[ 117.543922] Restarting tasks ... done.
[ 117.548390] video LNXVIDEO:01: Restoring backlight state
<2>
[ 117.549179] sd 6:0:0:0: [sdb] Media Changed
[ 117.549184] sd 6:0:0:0: [sdb]
[ 117.549187] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
[ 117.549189] sd 6:0:0:0: [sdb]
[ 117.549191] Sense Key : Unit Attention [current]
[ 117.549195] Info fld=0x0
[ 117.549197] sd 6:0:0:0: [sdb]
[ 117.549201] Add. Sense: Not ready to ready change, medium may have
changed [ 117.549203] sd 6:0:0:0: [sdb] CDB:
[ 117.549205] Read(10): 28 00 00 02 c9 00 00 00 f0 00
[ 117.549212] end_request: I/O error, dev sdb, sector 182528
[ 117.549218] Buffer I/O error on device sdb1, logical block 22
Re: [PATCH] xhci-ring: Fix Null pointer dereference
On 08/26/2014 06:47 PM, Ricardo Ribalda Delgado wrote: > While testing a usb gadget I managed to crash completely the host > computer. This was due to a NULL pointer derefence. > > This patch avoids the crash although the kernel still outputs some > warnings. > > Without this patch, kernels from (at least) 3.14 can be crashed with > mass storage gadgets. > > Affected host: NEC Corporation uPD720200 USB 3.0 > This should not be necessary anymore after commit 365038d83313951d6ace15342eb24624bbef1666 xhci: rework cycle bit checking for new dequeue pointers http://marc.info/?l=linux-usb&m=140844993115671&w=2 Which was just added to Greg's usb-linus branch. It checks that the new_deq_ptr and new_deq_seg are valid before calling xhci_queue_new_dequeue_state() -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] xhci-ring: Fix Null pointer dereference
At least I have seen the issue on Debian 3.14 and 3.16. Is your patch going to be backported to linux-stable? The computer crashes very very badly On Wed, Aug 27, 2014 at 4:25 PM, Mathias Nyman wrote: > On 08/26/2014 06:47 PM, Ricardo Ribalda Delgado wrote: >> While testing a usb gadget I managed to crash completely the host >> computer. This was due to a NULL pointer derefence. >> >> This patch avoids the crash although the kernel still outputs some >> warnings. >> >> Without this patch, kernels from (at least) 3.14 can be crashed with >> mass storage gadgets. >> >> Affected host: NEC Corporation uPD720200 USB 3.0 >> > > > This should not be necessary anymore after > commit 365038d83313951d6ace15342eb24624bbef1666 > xhci: rework cycle bit checking for new dequeue pointers > > http://marc.info/?l=linux-usb&m=140844993115671&w=2 > > Which was just added to Greg's usb-linus branch. > It checks that the new_deq_ptr and new_deq_seg are valid before calling > xhci_queue_new_dequeue_state() > > -Mathias > > > > > -- Ricardo Ribalda -- 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] xhci: rework cycle bit checking for new dequeue pointers
On 08/27/2014 07:07 AM, Mathias Nyman wrote:
> On 08/21/2014 01:06 AM, Joseph Salisbury wrote:
>> On 08/19/2014 08:17 AM, Mathias Nyman wrote:
>>> When we manually need to move the TR dequeue pointer we need to set the
>>> correct cycle bit as well. Previously we used the trb pointer from the
>>> last event received as a base, but this was changed in
>>> commit 1f81b6d22a59 ("usb: xhci: Prefer endpoint context dequeue pointer")
>>> to use the dequeue pointer from the endpoint context instead
>>>
>>> It turns out some Asmedia controllers advance the dequeue pointer
>>> stored in the endpoint context past the event triggering TRB, and
>>> this messed up the way the cycle bit was calculated.
>>>
>>> Instead of adding a quirk or complicating the already hard to follow cycle
>>> bit
>>> code, the whole cycle bit calculation is now simplified and adapted to
>>> handle
>>> event and endpoint context dequeue pointer differences.
>>>
>>> Fixes: 1f81b6d22a59 ("usb: xhci: Prefer endpoint context dequeue pointer")
>>> Reported-by: Maciej Puzio
>>> Reported-by: Evan Langlois
>>> Reviewed-by: Julius Werner
>>> Tested-by: Maciej Puzio
>>> Tested-by: Evan Langlois
>>> Signed-off-by: Mathias Nyman
>>> Cc: sta...@vger.kernel.org
>>> ---
>>> drivers/usb/host/xhci-ring.c | 101
>>> +--
>>> drivers/usb/host/xhci.c | 3 ++
>>> 2 files changed, 42 insertions(+), 62 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>>> index ac8cf23..abed30b 100644
>>> --- a/drivers/usb/host/xhci-ring.c
>>> +++ b/drivers/usb/host/xhci-ring.c
>>> @@ -364,32 +364,6 @@ static void ring_doorbell_for_active_rings(struct
>>> xhci_hcd *xhci,
>>> }
>>> }
>>>
>>> -/*
>>> - * Find the segment that trb is in. Start searching in start_seg.
>>> - * If we must move past a segment that has a link TRB with a toggle cycle
>>> state
>>> - * bit set, then we will toggle the value pointed at by cycle_state.
>>> - */
>>> -static struct xhci_segment *find_trb_seg(
>>> - struct xhci_segment *start_seg,
>>> - union xhci_trb *trb, int *cycle_state)
>>> -{
>>> - struct xhci_segment *cur_seg = start_seg;
>>> - struct xhci_generic_trb *generic_trb;
>>> -
>>> - while (cur_seg->trbs > trb ||
>>> - &cur_seg->trbs[TRBS_PER_SEGMENT - 1] < trb) {
>>> - generic_trb = &cur_seg->trbs[TRBS_PER_SEGMENT - 1].generic;
>>> - if (generic_trb->field[3] & cpu_to_le32(LINK_TOGGLE))
>>> - *cycle_state ^= 0x1;
>>> - cur_seg = cur_seg->next;
>>> - if (cur_seg == start_seg)
>>> - /* Looped over the entire list. Oops! */
>>> - return NULL;
>>> - }
>>> - return cur_seg;
>>> -}
>>> -
>>> -
>>> static struct xhci_ring *xhci_triad_to_transfer_ring(struct xhci_hcd *xhci,
>>> unsigned int slot_id, unsigned int ep_index,
>>> unsigned int stream_id)
>>> @@ -459,9 +433,12 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
>>> struct xhci_virt_device *dev = xhci->devs[slot_id];
>>> struct xhci_virt_ep *ep = &dev->eps[ep_index];
>>> struct xhci_ring *ep_ring;
>>> - struct xhci_generic_trb *trb;
>>> + struct xhci_segment *new_seg;
>>> + union xhci_trb *new_deq;
>>> dma_addr_t addr;
>>> u64 hw_dequeue;
>>> + bool cycle_found = false;
>>> + bool td_last_trb_found = false;
>>>
>>> ep_ring = xhci_triad_to_transfer_ring(xhci, slot_id,
>>> ep_index, stream_id);
>>> @@ -486,45 +463,45 @@ void xhci_find_new_dequeue_state(struct xhci_hcd
>>> *xhci,
>>> hw_dequeue = le64_to_cpu(ep_ctx->deq);
>>> }
>>>
>>> - /* Find virtual address and segment of hardware dequeue pointer */
>>> - state->new_deq_seg = ep_ring->deq_seg;
>>> - state->new_deq_ptr = ep_ring->dequeue;
>>> - while (xhci_trb_virt_to_dma(state->new_deq_seg, state->new_deq_ptr)
>>> - != (dma_addr_t)(hw_dequeue & ~0xf)) {
>>> - next_trb(xhci, ep_ring, &state->new_deq_seg,
>>> - &state->new_deq_ptr);
>>> - if (state->new_deq_ptr == ep_ring->dequeue) {
>>> - WARN_ON(1);
>>> - return;
>>> - }
>>> - }
>>> + new_seg = ep_ring->deq_seg;
>>> + new_deq = ep_ring->dequeue;
>>> + state->new_cycle_state = hw_dequeue & 0x1;
>>> +
>>> /*
>>> -* Find cycle state for last_trb, starting at old cycle state of
>>> -* hw_dequeue. If there is only one segment ring, find_trb_seg() will
>>> -* return immediately and cannot toggle the cycle state if this search
>>> -* wraps around, so add one more toggle manually in that case.
>>> +* We want to find the pointer, segment and cycle state of the new trb
>>> +* (the one after current TD's last_trb). We know the cycle state at
>>> +* hw_dequeue, so walk the ring until both hw_dequeue and last_trb are
>>> +* found.
>>>
Re: [PATCH v5 5/5] usb: gadget: f_uac2: send reasonably sized packets
On Wed, 27 Aug 2014, Daniel Mack wrote:
> The UAC2 function driver currently responds to all packets at all times
> with wMaxPacketSize packets. That results in way too fast audio
> playback as the function driver (which is in fact supposed to define
> the audio stream pace) delivers as fast as it can.
>
> Fix this by sizing each packet correctly with the following steps:
>
> a) Set the packet's size by dividing the nominal data rate by the
> playback endpoint's interval.q
>
> b) If there is a residual value from the calculation in a), add
> it to a accumulator to keep track of it across packets.
>
> c) If the accumulator has gathered at least the number of bytes
> that are needed for one sample frame, increase the packet size.
>
> This way, the packet size calculation will get rid of any kind of
> imprecision that would otherwise occur with a simple division over
> time.
>
> Signed-off-by: Daniel Mack
> @@ -191,8 +195,43 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request
> *req)
>
> spin_lock_irqsave(&prm->lock, flags);
>
> - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> + struct audio_dev *agdev = uac2_to_agdev(uac2);
> + struct f_uac2_opts *opts = agdev_to_uac2_opts(agdev);
> + unsigned int fsz = opts->p_ssize * num_channels(opts->p_chmask);
> + unsigned int rate = opts->p_srate * fsz;
> +
> + /*
> + * For each IN packet, calculate the minimum packet size by
> + * dividing the nominal bytes per second as required by the
> + * current audio format by the endpoint's interval.
> + */
> + req->length = min_t(unsigned int,
> + rate / uac2->p_interval, prm->max_psize);
> +
> + /*
> + * If there is a residual value from the division, add it to
> + * the residue accumulator.
> + */
> + uac2->p_residue += rate % uac2->p_interval;
> +
> + /*
> + * Whenever there are more bytes in the accumulator than we
> + * need to add one more sample frame, increase this packet's
> + * size and decrease the accumulator.
> + */
> + if (uac2->p_residue / uac2->p_interval >= fsz) {
> + req->length += fsz;
> + uac2->p_residue -= fsz * uac2->p_interval;
> + }
One thing I didn't mention about the algorithm I posted earlier: The
individual packet calculations don't contain any multiplications or
divisions.
The code here is in the hottest part of the driver, so you might want
to optimize it a little. For instance, several of the operations could
be precomputed and stored for later use.
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: About the reboot hang issue with EHCI driver on the Baytrail platform
On Wed, 27 Aug 2014, Gavin Guo wrote: > > Still, why not use the BIOS setting to disable EHCI? > > It's hardware enablement policy. By default, we can't change the BIOS > setting to make the bug disappear until we find out the bug is due to > BIOS's bug. Add kernel parameters is the temporary solution for the > platform can ship. And if the bug is from kernel, we need to send > patches to upstream and pull back to Ubuntu kernel or send to Ubuntu > Kernel directly if it's a trivial solution like adding device id. > However, as you said, in this case about the bug the root may be from > BIOS code. So, we can coordinate with the OEM BIOS team to modify the > default BIOS setting. There is no doubt that the BIOS is buggy. Your log contains this line: [2.804205] pci :00:1d.0: EHCI: BIOS handoff failed (BIOS bug?) 01010001 The only way that can happen is if the BIOS isn't working right. However, this may be a different bug from the crashes you've been seeing. 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: RES: RES: AS2105-based enclosure size issues with >2TB HDDs
On Wed, 27 Aug 2014, Oliver Neukum wrote: > > I don't think we want to add another SCSI flag to say that READ > > CAPACITY(10) is unreliable. > > Why not? It would only be friendly to tell the upper layer > of a malfunction if we know about it. To what end? What will the upper layer do with this information? > > Given the difficulty of determining the true capacity, I see two > > alternatives. We could set the capacity to a ridiculously large value > > (like 1 billion TB), or we could leave the capacity set to the low > > value and disable the "block within bounds" checks. Neither of these > > is attractive and they both have issues of their own -- although the > > second is close to what Windows does. > > That seems to be the most attractive solution to me. I'm skeptical that you can convince the SCSI and block-layer developers about this. Maybe they'll accept it if it is applied only to USB transports... > > (For example, udev often tries to read the last sectors of a new drive, > > looking for a GPT or RAID signature. That won't work if the capacity > > is set to some random value.) > > Yes, but clipping has its own dangers. Suppose you use the medium > without a partition table. What would Windows do? In the absence of a partition table, it would believe the value from READ CAPACITY, right? Isn't that the same as clipping? 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: Buffer I/O error after s2ram with usb storage
On Wed, 27 Aug 2014, Matthieu CASTET wrote:
> Ping
>
> I have got also a problem with a usb sdcard reader (without power cut
> during suspend)
> > > The usb storage driver call scsi_report_bus_reset after device reset,
> > > but because of commit dfcf7775 <4>, we don't ignore unit attention if
> > > "sshdr.asc == 0x28 && sshdr.ascq == 0x00" ("Not-ready to ready").
> > >
> > > If dfcf7775 is reverted there is no more Buffer I/O error.
> > >
> > > Is that possible to find a way to restore the behavior before dfcf7775
> > > commit (no Buffer I/O error after device reset) after a suspend to ram ?
Since that commit was written to fix a problem with certain cdrom
drives, maybe we would work around the issue by modifying the commit.
Have it go back to the original behavior if the device isn't a cdrom
drive.
That's not a complete fix (it won't help when a CD drive is attached
via USB), but maybe it's better than nothing.
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: Buffer I/O error after s2ram with usb storage
On Wed, 27 Aug 2014, Douglas Gilbert wrote: > The unit attention doesn't look like a problem, it > looks correct. If the system is unable to detect > removable media being changed while the system is > suspended, then > > If the media has a unique identifier, then this unit > attention at wakeup should trigger sd to make sure > that unique identifier has not changed. Does sd have any code to do this? I'm not aware of any, but there ought to be some. Otherwise there's no way to tell when a so-called media change was just the old media being taken out and put back in. Or maybe this functionality belongs in the block layer rather than in sd. 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 v3 1/2] usb: gadget: Refactor request completion
On Wed, 27 Aug 2014, Michal Sojka wrote:
> All USB peripheral controller drivers called completion routines
> directly. This patch moves the completion call from drivers to
> usb_gadget_giveback_request(), in order to have a place where common
> functionality can be added.
>
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -102,6 +102,15 @@ void usb_gadget_unmap_request(struct usb_gadget *gadget,
> }
> EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>
> +void usb_gadget_giveback_request(struct usb_ep *ep,
> + struct usb_request *req)
> +{
> + /* complete() is from gadget layer,
> + * eg fsg->bulk_in_complete() */
Wrong format for a multi-line comment. It should look like this:
/*
* Blah, blah, blah
* blah, blah, blah
*/
Also, the reference to fsg is obsolete; the File-backed Storage Gadget
is no longer in the kernel. You might as well leave the comment out
entirely.
> + if (req->complete)
This test shouldn't be here. The UDC drivers don't do it, but they
probably do check the req->complete isn't NULL when the request is
submitted. In any case, if req->complete is NULL then we want to know
about it because it is a bug. It should not be covered up silently.
> + req->complete(ep, req);
> +}
> +EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
> #endif /* CONFIG_HAS_DMA */
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] xhci-ring: Fix Null pointer dereference
On 08/27/2014 05:14 PM, Ricardo Ribalda Delgado wrote: > At least I have seen the issue on Debian 3.14 and 3.16. Is your patch > going to be backported to linux-stable? The computer crashes very very > badly > Yes, it is, but it might need some additional work as it won't apply cleanly on older versions http://marc.info/?l=linux-usb&m=140913688327011&w=2 -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
[PATCH] usb: gadget: f_uvc fix transition to video_ioctl2
UVC video node is a TX device from the point of view of the gadget, so we cannot rely on the video struct being filled with zeros, because VFL_DIR_TX is actually 1. Suggested-by: Sylwester Nawrocki Signed-off-by: Andrzej Pietrasiewicz --- drivers/usb/gadget/function/f_uvc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index 5209105..95dc1c6 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -411,6 +411,7 @@ uvc_register_video(struct uvc_device *uvc) video->fops = &uvc_v4l2_fops; video->ioctl_ops = &uvc_v4l2_ioctl_ops; video->release = video_device_release; + video->vfl_dir = VFL_DIR_TX; strlcpy(video->name, cdev->gadget->name, sizeof(video->name)); uvc->vdev = video; -- 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 v3 0/4] usb: gadget: f_uac2: cleanups and capture timing
Hi Xuebing, > Would you mind to share how did you run the test for both > Playback/Capture? What host version are you using, as different Host > enumerates (sends sequences of USB_REQ_SET_INTERFACE request differently)? sure, on client side (BBB) I have a jackd on top alsa running and connected capture and playback ports together. On host side I use a Linux Kernel 3.16.1 and forward the gadget usb with alsa_in and alsa_out to a jackd pipe with another usb interface (focusrite UAC2). regards, Sebastian signature.asc Description: This is a digitally signed message part
Re: [PATCH 0/7] g_webcam: Preparation for configfs
W dniu 18.08.2014 o 21:45, Laurent Pinchart pisze: Hi Andrzej, Thank you for the patches. The series looks good, I only had a few comments. I have rebased the patches on top of my UVC gadget branch, addressed my comments (the modified patches are marked as such in the commit message) and pushed the result to git://linuxtv.org/pinchartl/media.git uvc/gadget Would you be able to test the result and hopefully ack it ? If everything is fine with you there's no need to submit a new version. It seems that the f_uvc transition to video_ioctl2 is missing vfl_dir flag; I posted a patch fixing it. After the fix is applied I was able to test on real hardware, so as far as your changes to my patches are concerned, this is Acked-by: Andrzej Pietrasiewicz AP -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Status of g_webcam uvc-gadget
Hello Is somebody using/supporting g_webcam? The only reference userland server is uvc-gadget from http://git.ideasonboard.org/?p=uvc-gadget.git;a=summary ? I have an industrial fpga camera that speaks v4l2, my plan is to export it as an uvc camera via usb3380 as a debug interface. Thanks! -- Ricardo Ribalda -- 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] xhci-ring: Fix Null pointer dereference
Perhaps we could apply both patches to current tree and backport mine to older kernels? On Wed, Aug 27, 2014 at 5:27 PM, Mathias Nyman wrote: > On 08/27/2014 05:14 PM, Ricardo Ribalda Delgado wrote: >> At least I have seen the issue on Debian 3.14 and 3.16. Is your patch >> going to be backported to linux-stable? The computer crashes very very >> badly >> > > Yes, it is, but it might need some additional work as it won't apply cleanly > on older versions > > http://marc.info/?l=linux-usb&m=140913688327011&w=2 > > -Mathias -- Ricardo Ribalda -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/9] of: Add NVIDIA Tegra XUSB mailbox binding
On Mon, Aug 25, 2014 at 11:48 AM, Stephen Warren wrote: > On 08/18/2014 11:08 AM, Andrew Bresticker wrote: > >> diff --git a/include/dt-bindings/mailbox/tegra-xusb-mailbox.h >> b/include/dt-bindings/mailbox/tegra-xusb-mailbox.h > > >> +#define TEGRA_XUSB_MBOX_CHAN_HOST 0 >> +#define TEGRA_XUSB_MBOX_CHAN_PHY 1 > > > I can't work out how these values relate to hardware at all. Are they in > fact properties of the particular firmware that's loaded into the XUSB > module? If so, I don't think the DT should contain these values at all. Yes this is rather ugly... they're used by software for the demuxing of messages and don't correspond to actual hardware. > I wonder if the individual MBOX_CMD_* values from patch 2 are any > better, although I think those are also defined by the firmware, not the > hardware? They are indeed defined by the firmware, so this would become an issue if the firmware API ever changed. -- 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 3/9] of: Update Tegra XUSB pad controller binding for USB
On 08/27/2014 10:36 AM, Andrew Bresticker wrote:
On Mon, Aug 25, 2014 at 12:12 PM, Stephen Warren wrote:
On 08/18/2014 11:08 AM, Andrew Bresticker wrote:
- #phy-cells: Should be 1. The specifier is the index of the PHY to
reference.
See for the list of valid
values.
+- mboxes: Must contain an entry for the XUSB PHY mailbox channel.
+ See ../mailbox/mailbox.txt for details.
Can we require the mbox-names property here, so that everything is looked up
by names. I know that the proposed mbox binding states that using indexes is
preferred over names, but that's just silly considering that names are
widely used in most other similar bindings, and are much easier to extend in
a backwards compatible fashion in the face of optional entries. As such, I'd
prefer that all Tegra bindings use foo-names properties where they exist.
Sure, will do.
+Optional properties:
+---
+- vbus-otg-{0,1,2}-supply: VBUS regulator for the corresponding UTMI pad.
Why "-otg"? It's quite possible to have a regulator for VBUS even on systems
that don't support OTG, but rather simply have the ability to turn VBUS off.
Because they're the VBUS supplies for the OTG 'lanes'. It doesn't
really add anything, so I'll omit the "-otg".
Ah right. In that case, if the lanes are named "OTG" lanes in the HW
docs, I'm happy either way. If you did decide to keep the "-otg",
rewording as "VBUS regulator for the corresponding OTG UTMI pad" would
make the meaning clearer.
--
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 4/9] pinctrl: tegra-xusb: Add USB PHY support
On Mon, Aug 25, 2014 at 12:22 PM, Stephen Warren wrote:
> On 08/18/2014 11:08 AM, Andrew Bresticker wrote:
>>
>> In addition to the PCIe and SATA PHYs, the XUSB pad controller also
>> supports 3 UTMI, 2 HSIC, and 2 USB3 PHYs. Each USB3 PHY uses a single
>> PCIe or SATA lane and is mapped to one of the three UTMI ports.
>>
>> The xHCI controller will also send messages intended for the PHY driver,
>> so request and listen for messages on the mailbox's PHY channel.
>
>
> I'd like a review from Thierry here as the HW expert.
>
> I need an ack from LinusW in order to take this pinctrl patch through the
> Tegra tree.
>
>> diff --git a/drivers/pinctrl/pinctrl-tegra-xusb.c
>> b/drivers/pinctrl/pinctrl-tegra-xusb.c
>
>
>> +static int usb3_phy_power_on(struct phy *phy)
>> +{
>> + struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
>> + int port = usb3_phy_to_port(phy);
>> + int lane = padctl->usb3_ports[port].lane;
>> + u32 value, offset;
>> +
>> + if (!is_pcie_or_sata_lane(lane)) {
>> + dev_err(padctl->dev, "USB3 PHY %d mapped to invalid lane:
>> %d\n",
>> + port, lane);
>> + return -EINVAL;
>> + }
>
>
> An aside: This implies that the SATA driver should be talking to this
> pinctrl driver and explicitly powering on the XUSB pins. However, the SATA
> driver doesn't depend on this series. I'm a bit confused how that works.
> Perhaps it's just by accident? Mikko, can you comment?
As Mikko mentioned, the enabling of the SATA lane in
usb3_phy_power_on() is for when the SATA lane is being used for USB3.
>> +static int utmi_phy_to_port(struct phy *phy)
>> +{
>> + struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
>> + int i;
>> +
>> + for (i = 0; i < TEGRA_XUSB_UTMI_PHYS; i++) {
>> + if (phy == padctl->phys[TEGRA_XUSB_PADCTL_UTMI_P0 + i])
>> + break;
>> + }
>> + BUG_ON(i == TEGRA_XUSB_UTMI_PHYS);
>
>
> Can this be triggered by e.g. bad DT content? If so, returning an error
> would be nicer. The comment applies to other xxx_to_port() functions.
No, it cannot. The struct phy that's passed in here comes from the
PHY core and must be a PHY that we registered earlier in probe().
--
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 3/9] of: Update Tegra XUSB pad controller binding for USB
On Mon, Aug 25, 2014 at 12:12 PM, Stephen Warren wrote:
> On 08/18/2014 11:08 AM, Andrew Bresticker wrote:
>> - #phy-cells: Should be 1. The specifier is the index of the PHY to
>> reference.
>> See for the list of valid
>> values.
>> +- mboxes: Must contain an entry for the XUSB PHY mailbox channel.
>> + See ../mailbox/mailbox.txt for details.
>
>
> Can we require the mbox-names property here, so that everything is looked up
> by names. I know that the proposed mbox binding states that using indexes is
> preferred over names, but that's just silly considering that names are
> widely used in most other similar bindings, and are much easier to extend in
> a backwards compatible fashion in the face of optional entries. As such, I'd
> prefer that all Tegra bindings use foo-names properties where they exist.
Sure, will do.
>> +Optional properties:
>> +---
>> +- vbus-otg-{0,1,2}-supply: VBUS regulator for the corresponding UTMI pad.
>
>
> Why "-otg"? It's quite possible to have a regulator for VBUS even on systems
> that don't support OTG, but rather simply have the ability to turn VBUS off.
Because they're the VBUS supplies for the OTG 'lanes'. It doesn't
really add anything, so I'll omit the "-otg".
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 5/5] usb: gadget: f_uac2: send reasonably sized packets
On 08/27/2014 04:18 PM, Alan Stern wrote:
> On Wed, 27 Aug 2014, Daniel Mack wrote:
>> +/*
>> + * Whenever there are more bytes in the accumulator than we
>> + * need to add one more sample frame, increase this packet's
>> + * size and decrease the accumulator.
>> + */
>> +if (uac2->p_residue / uac2->p_interval >= fsz) {
>> +req->length += fsz;
>> +uac2->p_residue -= fsz * uac2->p_interval;
>> +}
>
> One thing I didn't mention about the algorithm I posted earlier: The
> individual packet calculations don't contain any multiplications or
> divisions.
>
> The code here is in the hottest part of the driver, so you might want
> to optimize it a little. For instance, several of the operations could
> be precomputed and stored for later use.
Yeah, well. I did tests with that, and at least the difference in load
is not measureable on BBB. But you're right of course, and it even leads
to nicer code. Will send that as v6.
Thanks,
Daniel
--
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: About the reboot hang issue with EHCI driver on the Baytrail platform
Hi Alan, On Wed, Aug 27, 2014 at 10:38 PM, Alan Stern wrote: > On Wed, 27 Aug 2014, Gavin Guo wrote: > >> > Still, why not use the BIOS setting to disable EHCI? >> >> It's hardware enablement policy. By default, we can't change the BIOS >> setting to make the bug disappear until we find out the bug is due to >> BIOS's bug. Add kernel parameters is the temporary solution for the >> platform can ship. And if the bug is from kernel, we need to send >> patches to upstream and pull back to Ubuntu kernel or send to Ubuntu >> Kernel directly if it's a trivial solution like adding device id. >> However, as you said, in this case about the bug the root may be from >> BIOS code. So, we can coordinate with the OEM BIOS team to modify the >> default BIOS setting. > > There is no doubt that the BIOS is buggy. Your log contains this line: > > [2.804205] pci :00:1d.0: EHCI: BIOS handoff failed (BIOS bug?) > 01010001 > > The only way that can happen is if the BIOS isn't working right. > However, this may be a different bug from the crashes you've been > seeing. Thanks for your comments. I really appreciate your support. Gavin Guo > > Alan Stern > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- 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 v6 0/5] usb: gadget: f_uac2: cleanups and capture timing
Hi, this is v6 of the f_uac2 timing fixup series. Changes from v5: * Pre-calculate some more variables so we have to do their computation at runtime for each frame Changes from v4: * Fix buffer setup in afunc_set_alt() Changes from v3: * add another patch (3/5) to introduce agdev_to_uac2_opts() which is also needed in 5/5 * patch 5/5 only: move from a pre-calculated sequence of packet lengths to an accumulator that sums up the residual values from the packet size calculation and adds an extra sample frame now and then when the accumulator has gathered enough bytes. Changes from v2: * swap path 3 and 4, so that the ALSA buffer wrap around fix comes in first. It's not actually a bug fix for the current code, but more a preparation to allow for smaller packets. * use the p_ssize, p_chmask and p_srate for the length calculations * prepare a sequence of packet sizes to send, and alternate over them when sending the the IN packets. The actual commit log and the comments yield some more details on that. Changes from v1: * drop UAC_EP_CS_ATTR_FILL_MAX approach and rather size the packets correctly * add a patch to fix buffer wrap problems in the ALSA buffer logic, which wasn't needed before The first two patches are just cleanups. Thanks to Alan Stern and Jassi Brar for the feedback! Daniel Subject: [PATCH v6 0/5] *** SUBJECT HERE *** *** BLURB HERE *** Daniel Mack (5): usb: gadget: f_uac2: restructure some code in afunc_set_alt() usb: gadget: f_uac2: add short-hand for 'dev' usb: gadget: f_uac2: introduce agdev_to_uac2_opts usb: gadget: f_uac2: handle partial dma area wrap usb: gadget: f_uac2: send reasonably sized packets drivers/usb/gadget/function/f_uac2.c | 167 +-- 1 file changed, 118 insertions(+), 49 deletions(-) -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 5/5] usb: gadget: f_uac2: send reasonably sized packets
The UAC2 function driver currently responds to all packets at all times
with wMaxPacketSize packets. That results in way too fast audio
playback as the function driver (which is in fact supposed to define
the audio stream pace) delivers as fast as it can.
Fix this by sizing each packet correctly with the following steps:
a) Set the packet's size by dividing the nominal data rate by the
playback endpoint's interval.
b) If there is a residual value from the calculation in a), add
it to a accumulator to keep track of it across packets.
c) If the accumulator has gathered at least the number of bytes
that are needed for one sample frame, increase the packet size.
This way, the packet size calculation will get rid of any kind of
imprecision that would otherwise occur with a simple division over
time.
Some of the variables that are needed while processing each packet
are pre-computed for performance reasons.
Signed-off-by: Daniel Mack
---
drivers/usb/gadget/function/f_uac2.c | 69 +---
1 file changed, 65 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 246a778..a5a27a5 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -92,6 +92,15 @@ struct snd_uac2_chip {
struct snd_card *card;
struct snd_pcm *pcm;
+
+ /* timekeeping for the playback endpoint */
+ unsigned int p_interval;
+ unsigned int p_residue;
+
+ /* pre-calculated values for playback iso completion */
+ unsigned int p_pktsize;
+ unsigned int p_pktsize_residue;
+ unsigned int p_framesize;
};
#define BUFF_SIZE_MAX (PAGE_SIZE * 16)
@@ -191,8 +200,29 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request
*req)
spin_lock_irqsave(&prm->lock, flags);
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+ /*
+* For each IN packet, take the quotient of the current data
+* rate and the endpoint's interval as the base packet size.
+* If there is a residue from this division, add it to the
+* residue accumulator.
+*/
+ req->length = uac2->p_pktsize;
+ uac2->p_residue += uac2->p_pktsize_residue;
+
+ /*
+* Whenever there are more bytes in the accumulator than we
+* need to add one more sample frame, increase this packet's
+* size and decrease the accumulator.
+*/
+ if (uac2->p_residue / uac2->p_interval >= uac2->p_framesize) {
+ req->length += uac2->p_framesize;
+ uac2->p_residue -= uac2->p_framesize *
+ uac2->p_interval;
+ }
+
req->actual = req->length;
+ }
pending = prm->hw_ptr % prm->period_size;
pending += req->actual;
@@ -346,6 +376,7 @@ static int uac2_pcm_open(struct snd_pcm_substream
*substream)
c_srate = opts->c_srate;
p_chmask = opts->p_chmask;
c_chmask = opts->c_chmask;
+ uac2->p_residue = 0;
runtime->hw = uac2_pcm_hardware;
@@ -1077,7 +1108,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
struct usb_request *req;
struct usb_ep *ep;
struct uac2_rtd_params *prm;
- int i;
+ int req_len, i;
/* No i/f has more than 2 alt settings */
if (alt > 1) {
@@ -1099,11 +1130,41 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
prm = &uac2->c_prm;
config_ep_by_speed(gadget, fn, ep);
agdev->as_out_alt = alt;
+ req_len = prm->max_psize;
} else if (intf == agdev->as_in_intf) {
+ struct f_uac2_opts *opts = agdev_to_uac2_opts(agdev);
+ unsigned int factor, rate;
+ struct usb_endpoint_descriptor *ep_desc;
+
ep = agdev->in_ep;
prm = &uac2->p_prm;
config_ep_by_speed(gadget, fn, ep);
agdev->as_in_alt = alt;
+
+ /* pre-calculate the playback endpoint's interval */
+ if (gadget->speed == USB_SPEED_FULL) {
+ ep_desc = &fs_epin_desc;
+ factor = 1000;
+ } else {
+ ep_desc = &hs_epin_desc;
+ factor = 125;
+ }
+
+ /* pre-compute some values for iso_complete() */
+ uac2->p_framesize = opts->p_ssize *
+ num_channels(opts->p_chmask);
+ rate = opts->p_srate * uac2->p_framesize;
+ uac2->p_interval = (1 << (ep_desc->bInterval - 1)) * factor;
+ uac2->p_pktsize = min_t(
[PATCH v6 2/5] usb: gadget: f_uac2: add short-hand for 'dev'
In afunc_bind() and afunc_set_alt(), &uac2->pdev.dev are used multiple
times. Adding a short-hand for them makes lines shorter so we can
remove some line wraps.
No functional change.
Signed-off-by: Daniel Mack
---
drivers/usb/gadget/function/f_uac2.c | 29 +++--
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index ab4652e..efe8add 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -918,6 +918,7 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
struct snd_uac2_chip *uac2 = &agdev->uac2;
struct usb_composite_dev *cdev = cfg->cdev;
struct usb_gadget *gadget = cdev->gadget;
+ struct device *dev = &uac2->pdev.dev;
struct uac2_rtd_params *prm;
struct f_uac2_opts *uac2_opts;
struct usb_string *us;
@@ -961,8 +962,7 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
ret = usb_interface_id(cfg, fn);
if (ret < 0) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return ret;
}
std_ac_if_desc.bInterfaceNumber = ret;
@@ -971,8 +971,7 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
ret = usb_interface_id(cfg, fn);
if (ret < 0) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return ret;
}
std_as_out_if0_desc.bInterfaceNumber = ret;
@@ -982,8 +981,7 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
ret = usb_interface_id(cfg, fn);
if (ret < 0) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return ret;
}
std_as_in_if0_desc.bInterfaceNumber = ret;
@@ -993,16 +991,14 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
agdev->out_ep = usb_ep_autoconfig(gadget, &fs_epout_desc);
if (!agdev->out_ep) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
goto err;
}
agdev->out_ep->driver_data = agdev;
agdev->in_ep = usb_ep_autoconfig(gadget, &fs_epin_desc);
if (!agdev->in_ep) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
goto err;
}
agdev->in_ep->driver_data = agdev;
@@ -1057,6 +1053,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
struct audio_dev *agdev = func_to_agdev(fn);
struct snd_uac2_chip *uac2 = &agdev->uac2;
struct usb_gadget *gadget = cdev->gadget;
+ struct device *dev = &uac2->pdev.dev;
struct usb_request *req;
struct usb_ep *ep;
struct uac2_rtd_params *prm;
@@ -1064,16 +1061,14 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
/* No i/f has more than 2 alt settings */
if (alt > 1) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return -EINVAL;
}
if (intf == agdev->ac_intf) {
/* Control I/f has only 1 AltSetting - 0 */
if (alt) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return -EINVAL;
}
return 0;
@@ -1090,8 +1085,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
config_ep_by_speed(gadget, fn, ep);
agdev->as_in_alt = alt;
} else {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return -EINVAL;
}
@@ -1120,8 +1114,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
}
if (usb_ep_queue(ep, prm->ureq[i].req, GFP_ATOMIC))
- dev_err(&uac2->pdev.dev, "%s:%d Error!\n",
- __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
}
return 0;
--
2.1.0
--
To unsubscribe
[PATCH v6 4/5] usb: gadget: f_uac2: handle partial dma area wrap
With packet sizes other than 512, payloads in the packets may wrap
around the ALSA dma buffer partially, which leads to memory corruption
and audible clicks and pops in the audio stream at the moment, because
there is no boundary check before the memcpy().
In preparation to an implementation for smaller and dynamically sized
packets, we have to address such cases, and copy the payload in two
steps conditionally.
The 'src' and 'dst' approach doesn't work here anymore, as different
behavior is necessary in playback and capture cases. Thus, this patch
open-codes the routine now.
Signed-off-by: Daniel Mack
---
drivers/usb/gadget/function/f_uac2.c | 32 +++-
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 9c8831d..246a778 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -163,8 +163,8 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request
*req)
{
unsigned pending;
unsigned long flags;
+ unsigned int hw_ptr;
bool update_alsa = false;
- unsigned char *src, *dst;
int status = req->status;
struct uac2_req *ur = req->context;
struct snd_pcm_substream *substream;
@@ -191,26 +191,40 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request
*req)
spin_lock_irqsave(&prm->lock, flags);
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
- src = prm->dma_area + prm->hw_ptr;
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
req->actual = req->length;
- dst = req->buf;
- } else {
- dst = prm->dma_area + prm->hw_ptr;
- src = req->buf;
- }
pending = prm->hw_ptr % prm->period_size;
pending += req->actual;
if (pending >= prm->period_size)
update_alsa = true;
+ hw_ptr = prm->hw_ptr;
prm->hw_ptr = (prm->hw_ptr + req->actual) % prm->dma_bytes;
spin_unlock_irqrestore(&prm->lock, flags);
/* Pack USB load in ALSA ring buffer */
- memcpy(dst, src, req->actual);
+ pending = prm->dma_bytes - hw_ptr;
+
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+ if (unlikely(pending < req->actual)) {
+ memcpy(req->buf, prm->dma_area + hw_ptr, pending);
+ memcpy(req->buf + pending, prm->dma_area,
+ req->actual - pending);
+ } else {
+ memcpy(req->buf, prm->dma_area + hw_ptr, req->actual);
+ }
+ } else {
+ if (unlikely(pending < req->actual)) {
+ memcpy(prm->dma_area + hw_ptr, req->buf, pending);
+ memcpy(prm->dma_area, req->buf + pending,
+ req->actual - pending);
+ } else {
+ memcpy(prm->dma_area + hw_ptr, req->buf, req->actual);
+ }
+ }
+
exit:
if (usb_ep_queue(ep, req, GFP_ATOMIC))
dev_err(&uac2->pdev.dev, "%d Error!\n", __LINE__);
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 3/5] usb: gadget: f_uac2: introduce agdev_to_uac2_opts
Add a simple container_of() wrapper to get a struct f_uac2_opts from a
struct struct audio_dev. Use it in two places where it is currently
open-coded.
Signed-off-by: Daniel Mack
---
drivers/usb/gadget/function/f_uac2.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index efe8add..9c8831d 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -140,6 +140,12 @@ struct snd_uac2_chip *pdev_to_uac2(struct platform_device
*p)
}
static inline
+struct f_uac2_opts *agdev_to_uac2_opts(struct audio_dev *agdev)
+{
+ return container_of(agdev->func.fi, struct f_uac2_opts, func_inst);
+}
+
+static inline
uint num_channels(uint chanmask)
{
uint num = 0;
@@ -1168,7 +1174,7 @@ in_rq_cur(struct usb_function *fn, const struct
usb_ctrlrequest *cr)
int value = -EOPNOTSUPP;
int p_srate, c_srate;
- opts = container_of(agdev->func.fi, struct f_uac2_opts, func_inst);
+ opts = agdev_to_uac2_opts(agdev);
p_srate = opts->p_srate;
c_srate = opts->c_srate;
@@ -1210,7 +1216,7 @@ in_rq_range(struct usb_function *fn, const struct
usb_ctrlrequest *cr)
int value = -EOPNOTSUPP;
int p_srate, c_srate;
- opts = container_of(agdev->func.fi, struct f_uac2_opts, func_inst);
+ opts = agdev_to_uac2_opts(agdev);
p_srate = opts->p_srate;
c_srate = opts->c_srate;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 1/5] usb: gadget: f_uac2: restructure some code in afunc_set_alt()
Restructure some code to make it easier to read.
While at it, return -ENOMEM instead of -EINVAL if
usb_ep_alloc_request() fails, and omit the logging in such cases
(the mm core will complain loud enough).
Signed-off-by: Daniel Mack
---
drivers/usb/gadget/function/f_uac2.c | 39 +++-
1 file changed, 16 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 0d65e7c..ab4652e 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -1104,31 +1104,24 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
usb_ep_enable(ep);
for (i = 0; i < USB_XFERS; i++) {
- if (prm->ureq[i].req) {
- if (usb_ep_queue(ep, prm->ureq[i].req, GFP_ATOMIC))
- dev_err(&uac2->pdev.dev, "%d Error!\n",
- __LINE__);
- continue;
- }
-
- req = usb_ep_alloc_request(ep, GFP_ATOMIC);
- if (req == NULL) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
- return -EINVAL;
+ if (!prm->ureq[i].req) {
+ req = usb_ep_alloc_request(ep, GFP_ATOMIC);
+ if (req == NULL)
+ return -ENOMEM;
+
+ prm->ureq[i].req = req;
+ prm->ureq[i].pp = prm;
+
+ req->zero = 0;
+ req->context = &prm->ureq[i];
+ req->length = prm->max_psize;
+ req->complete = agdev_iso_complete;
+ req->buf = prm->rbuf + i * req->length;
}
- prm->ureq[i].req = req;
- prm->ureq[i].pp = prm;
-
- req->zero = 0;
- req->context = &prm->ureq[i];
- req->length = prm->max_psize;
- req->complete = agdev_iso_complete;
- req->buf = prm->rbuf + i * req->length;
-
- if (usb_ep_queue(ep, req, GFP_ATOMIC))
- dev_err(&uac2->pdev.dev, "%d Error!\n", __LINE__);
+ if (usb_ep_queue(ep, prm->ureq[i].req, GFP_ATOMIC))
+ dev_err(&uac2->pdev.dev, "%s:%d Error!\n",
+ __func__, __LINE__);
}
return 0;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [xhci] BUG: unable to handle kernel NULL pointer dereference at (null)
> [ 12.410950] [] ? setup_test_wrap64+0x320/0x320
> [ 12.410950] [] ? setup_test_dont_trim+0x2f0/0x2f0
> [ 12.410950] [] ? xhci_ring_free+0x1d0/0x1d0
> [ 12.410950] [] ? ohci_platform_init+0x60/0x60
> [ 12.410950] [] do_one_initcall+0x143/0x24d
> [ 12.410950] [] ? parse_args+0x2fb/0x530
> [ 12.410950] [] kernel_init_freeable+0x1dc/0x2aa
> [ 12.410950] [] ? do_early_param+0xc3/0xc3
> [ 12.410950] [] ? rest_init+0xd0/0xd0
> [ 12.410950] [] kernel_init+0xe/0x160
> [ 12.410950] [] ret_from_fork+0x7c/0xb0
> [ 12.410950] [] ? rest_init+0xd0/0xd0
> [ 12.410950] Code: 48 85 ff 40 0f 94 c6 44 0f b6 ce 49 83 c1 02 4a 83 04 cd
> a0 e9 b3 82 01 45 31 c9 40 84 f6 75 0b 45 0f b6 ca 49 c1 e1 04 49 01 f9 <49>
> 8b 39 48 8b 30 48 c1 e1 06 4c 89 78 10 44 89 40 08 01 d3 89
> [ 12.410950] RIP [] setup_test_skip64+0x183/0x270
> [ 12.410950] RSP
> [ 12.410950] CR2:
> [ 12.410950] ---[ end trace 3157077290b0c2c1 ]---
> [ 12.410950] Kernel panic - not syncing: Fatal exception
>
> git bisect start 66e8dfa4e0d9600dedc08adcaac83c378b65351b
> 52addcf9d6669fa439387610bc65c92fa0980cef --
> git bisect good 511b6daa3a596ab5c54bee5dab56ed4f77337a40 # 22:39 20+
> 0 Merge 'ipvs-next/master' into devel-hourly-2014082722
> git bisect bad 73e9ac542728ea03b8796cf9818950dc9e05d534 # 22:49 0-
> 20 Merge 'hid/for-3.18/upstream' into devel-hourly-2014082722
> git bisect good 513dd18bd1b397935660c01daa14e53e819b9270 # 23:00 20+
> 0 Merge 'netdev-next/master' into devel-hourly-2014082722
> git bisect good a617416625136eec767df79308544cbb46fe0311 # 23:03 20+
> 0 Merge 'kvm-ppc/kvm-ppc-queue' into devel-hourly-2014082722
> git bisect good 858bf88bf6175c80920daa8c9210b0209443b7e1 # 23:06 20+
> 0 Merge 'spi/for-next' into devel-hourly-2014082722
> git bisect good cdb03bc488490bb364fa29ec292ecd3291ca5770 # 23:10 20+
> 0 Merge 'regulator/for-next' into devel-hourly-2014082722
> git bisect bad 8f5a71eb299401d62562e7ab634665ff98850e8f # 23:13 0-
> 20 Merge 'djbw-usb/td-fragments-v1' into devel-hourly-2014082722
> git bisect good a75ef911cf100b8cf7d25baf6dac8052328a96e7 # 23:22 20+
> 0 xhci: clarify "ring valid" checks
> git bisect good 652b7ee36207f186f3d701675483df43b4845c5c # 23:26 20+
> 0 xhci: kill ->num_trbs_free_temp in struct xhci_ring
> git bisect good 1c11eb8545a3321e7ca27fc7ba8c56b6e6df2b57 # 23:31 20+
> 0 xhci: add xhci_ring_reap_td() helper
> git bisect bad e65e21a542cab81d794db4e5fe919c4e1d624ea7 # 23:54 0-
> 20 xhci: unit test ring enqueue/dequeue routines
> git bisect good fb6fa3e625e1e453aea9eeb97d58bee30e1c0781 # 23:58 20+
> 0 xhci: v1.0 scatterlist enqueue support (td-fragment rework)
> # first bad commit: [e65e21a542cab81d794db4e5fe919c4e1d624ea7] xhci: unit
> test ring enqueue/dequeue routines
> git bisect good fb6fa3e625e1e453aea9eeb97d58bee30e1c0781 # 00:00 60+
> 0 xhci: v1.0 scatterlist enqueue support (td-fragment rework)
> git bisect bad 66e8dfa4e0d9600dedc08adcaac83c378b65351b # 00:00 0-
> 11 0day head guard for 'devel-hourly-2014082722'
> git bisect good 68e370289c29e3beac99d59c6d840d470af9dfcf # 00:19 60+
> 2 Merge branch 'for-linus' of
> git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
> git bisect good d05446ae2128064a4bb8f74c84f6901ffb5c94bc # 00:33 60+
> 1 Add linux-next specific files for 20140827
>
>
> This script may reproduce the error.
>
>
> #!/bin/bash
>
> kernel=$1
> initrd=quantal-core-x86_64.cgz
>
> wget --no-clobber
> https://github.com/fengguang/reproduce-kernel-bug/raw/master/initrd/$initrd
>
> kvm=(
> qemu-system-x86_64
> -cpu kvm64
> -enable-kvm
> -kernel $kernel
> -initrd $initrd
> -m 320
> -smp 2
> -net nic,vlan=1,model=e1000
> -net user,vlan=1
> -boot order=nc
> -no-reboot
> -watchdog i6300esb
> -rtc base=localtime
> -serial stdio
> -display none
> -monitor null
> )
>
> append=(
> hung_task_panic=1
> earlyprintk=ttyS0,115200
> debug
> apic=debug
> sysrq_always_enabled
> rcupdate.rcu_cpu_stall_timeout=100
> panic=-1
> softlockup_panic=1
> nmi_watchdog=panic
> oops=panic
> load_ramdisk=2
> prompt_ramdisk=0
> console=ttyS0,115200
> console=tty0
> vga=normal
> root=/dev/ram0
> rw
> drbd.minor_count=8
> )
>
> "${kvm[@]}" --append "${append[*]}"
>
>
> Thanks,
> Fengguang
--
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/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver
On Mon, Aug 25, 2014 at 12:01 PM, Stephen Warren wrote:
> On 08/18/2014 11:08 AM, Andrew Bresticker wrote:
>>
>> The Tegra xHCI controller's firmware communicates requests to the host
>> processor through a mailbox interface. While there is only a single
>> communication channel, messages sent by the controller can be divided
>> into two groups: those intended for the PHY driver and those intended
>> for the host-controller driver. This mailbox driver exposes the two
>> channels and routes incoming messages to the appropriate channel based
>> on the command encoded in the message.
>
>
>> diff --git a/drivers/mailbox/tegra-xusb-mailbox.c
>> b/drivers/mailbox/tegra-xusb-mailbox.c
>
>
>> +#define XUSB_CFG_ARU_MBOX_CMD 0xe4
>> +#define MBOX_FALC_INT_EN BIT(27)
>> +#define MBOX_PME_INT_EN BIT(28)
>> +#define MBOX_SMI_INT_EN BIT(29)
>> +#define MBOX_XHCI_INT_EN BIT(30)
>> +#define MBOX_INT_EN BIT(31)
>
>
> Those field names don't match the documentation in the TRM; they're called
> DEST_xxx rather than xxx_INT_EN. I'm not sure what that disconnect means
> (i.e. whether it's just a different naming choice, or there's some practical
> disconnect that will cause issues.)
Hmm... interestingly *_INT_EN is the convention the downstream kernels
used. DEST_* is definitely more accurate as I'm pretty sure these
bits select the destination for the interrupt.
>> +static struct mbox_chan *mbox_cmd_to_chan(struct tegra_xusb_mbox *mbox,
>> u32 cmd)
>> +{
>> + switch (cmd) {
>> + case MBOX_CMD_INC_FALC_CLOCK:
>> + case MBOX_CMD_DEC_FALC_CLOCK:
>> + case MBOX_CMD_INC_SSPI_CLOCK:
>> + case MBOX_CMD_DEC_SSPI_CLOCK:
>> + case MBOX_CMD_SET_BW:
>> + return &mbox->mbox.chans[TEGRA_XUSB_MBOX_CHAN_HOST];
>> + case MBOX_CMD_SAVE_DFE_CTLE_CTX:
>> + case MBOX_CMD_START_HSIC_IDLE:
>> + case MBOX_CMD_STOP_HSIC_IDLE:
>> + return &mbox->mbox.chans[TEGRA_XUSB_MBOX_CHAN_PHY];
>> + default:
>> + return NULL;
>> + }
>> +}
>
>
> This makes me think that the CHAN_HOST/CHAN_PHY values are purely a facet of
> the Linux driver's message de-multiplexing, rather than anything to do with
> the HW.
Yup, they are...
> I'm not even sure if it's appropriate for the low-level mailbox driver to
> know about the semantics of the message, rather than simply sending them on
> to the client driver? Perhaps when drivers register(?) for callbacks(?) for
> messages, they should state which types of messages they want to listen to?
So there's not really a way for the client driver to tell the mailbox
driver which types of messages it wants to listen to on a particular
channel with the mailbox framework - it simply provides a way for
clients to bind with channels. I think there are a couple of options
here, either: a) have a channel per message (as you mentioned in the
previous patch), which allows the client to only register for messages
(channels) it wants to handle, or b) extend the mailbox framework to
allow shared channels so that both clients can receive messages on the
single channel and handle messages appropriately. The disadvantage
of (a) is that the commands are firmware defined and could
theoretically change between releases of the firmware, though I'm not
sure how common that is in practice. So that leaves (b) - Jassi, what
do you think about having shared (non-exclusive) channels?
>> +static irqreturn_t tegra_xusb_mbox_irq(int irq, void *p)
>
>
>> + /* Clear mbox interrupts */
>>
>> + reg = mbox_readl(mbox, XUSB_CFG_ARU_SMI_INTR);
>> + if (reg & MBOX_SMI_INTR_FW_HANG)
>> + dev_err(mbox->mbox.dev, "Controller firmware hang\n");
>> + mbox_writel(mbox, reg, XUSB_CFG_ARU_SMI_INTR);
>
>
>> + /*
>>
>> +* Set the mailbox back to idle. The recipient of the message is
>> +* responsible for sending an ACK/NAK, if necessary.
>> +*/
>> + reg = mbox_readl(mbox, XUSB_CFG_ARU_MBOX_CMD);
>> + reg &= ~MBOX_SMI_INT_EN;
>> + mbox_writel(mbox, reg, XUSB_CFG_ARU_MBOX_CMD);
>
>
> Does the protocol not allow the remote firmware to send another message
> until the host has ack'd/nak'd the message; the code above turns off the IRQ
> that indicated to the host that a message was sent to it...
While the firmware generally will not send another message until the
previous one is ACK'd/NAK'd (with the exception of the SET_BW
command), the above does not prevent it from doing so. I believe the
controller sets up the DEST_* bits properly before sending another
message.
>> +static int tegra_xusb_mbox_probe(struct platform_device *pdev)
>
>
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>
>> + if (!res)
>> + return -ENODEV;
>
>
> Should devm_request_mem_region() be called here to claim the region?
No, the xHCI
Re: [PATCH v2 2/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver
On 08/27/2014 11:38 AM, Andrew Bresticker wrote:
On Mon, Aug 25, 2014 at 12:01 PM, Stephen Warren wrote:
On 08/18/2014 11:08 AM, Andrew Bresticker wrote:
The Tegra xHCI controller's firmware communicates requests to the host
processor through a mailbox interface. While there is only a single
communication channel, messages sent by the controller can be divided
into two groups: those intended for the PHY driver and those intended
for the host-controller driver. This mailbox driver exposes the two
channels and routes incoming messages to the appropriate channel based
on the command encoded in the message.
diff --git a/drivers/mailbox/tegra-xusb-mailbox.c
b/drivers/mailbox/tegra-xusb-mailbox.c
+static struct mbox_chan *mbox_cmd_to_chan(struct tegra_xusb_mbox *mbox,
u32 cmd)
+{
+ switch (cmd) {
+ case MBOX_CMD_INC_FALC_CLOCK:
+ case MBOX_CMD_DEC_FALC_CLOCK:
+ case MBOX_CMD_INC_SSPI_CLOCK:
+ case MBOX_CMD_DEC_SSPI_CLOCK:
+ case MBOX_CMD_SET_BW:
+ return &mbox->mbox.chans[TEGRA_XUSB_MBOX_CHAN_HOST];
+ case MBOX_CMD_SAVE_DFE_CTLE_CTX:
+ case MBOX_CMD_START_HSIC_IDLE:
+ case MBOX_CMD_STOP_HSIC_IDLE:
+ return &mbox->mbox.chans[TEGRA_XUSB_MBOX_CHAN_PHY];
+ default:
+ return NULL;
+ }
+}
This makes me think that the CHAN_HOST/CHAN_PHY values are purely a facet of
the Linux driver's message de-multiplexing, rather than anything to do with
the HW.
Yup, they are...
I'm not even sure if it's appropriate for the low-level mailbox driver to
know about the semantics of the message, rather than simply sending them on
to the client driver? Perhaps when drivers register(?) for callbacks(?) for
messages, they should state which types of messages they want to listen to?
So there's not really a way for the client driver to tell the mailbox
driver which types of messages it wants to listen to on a particular
channel with the mailbox framework - it simply provides a way for
clients to bind with channels. I think there are a couple of options
here, either: a) have a channel per message (as you mentioned in the
previous patch), which allows the client to only register for messages
(channels) it wants to handle, or b) extend the mailbox framework to
allow shared channels so that both clients can receive messages on the
single channel and handle messages appropriately. The disadvantage
of (a) is that the commands are firmware defined and could
theoretically change between releases of the firmware, though I'm not
sure how common that is in practice. So that leaves (b) - Jassi, what
do you think about having shared (non-exclusive) channels?
Another alternative might be for each client driver to hard-code a
unique dummy channel ID so that each client still gets a separate
exclusive channel, but then have the mbox driver broadcast each message
to each of those channels. I'm not sure that would be any better though;
adding (b) as an explicit option to the mbox subsystem would almost
certainly be cleaner.
+static int tegra_xusb_mbox_probe(struct platform_device *pdev)
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;
Should devm_request_mem_region() be called here to claim the region?
No, the xHCI host driver also needs to map these registers, so they
cannot be mapped exclusively here.
That's unfortunate. Having multiple drivers with overlapping register
regions is not a good idea. Can we instead have a top-level driver map
all the IO regions, then instantiate the various different
sub-components internally, and divide up the address space. Probably via
MFD or similar. That would prevent multiple drivers from touching the
same register region.
--
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/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver
On Wed, Aug 27, 2014 at 10:50 AM, Stephen Warren wrote: > On 08/27/2014 11:38 AM, Andrew Bresticker wrote: >> >> On Mon, Aug 25, 2014 at 12:01 PM, Stephen Warren >> wrote: >>> >>> On 08/18/2014 11:08 AM, Andrew Bresticker wrote: +static int tegra_xusb_mbox_probe(struct platform_device *pdev) >>> >>> >>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -ENODEV; >>> >>> >>> >>> Should devm_request_mem_region() be called here to claim the region? >> >> >> No, the xHCI host driver also needs to map these registers, so they >> cannot be mapped exclusively here. > > > That's unfortunate. Having multiple drivers with overlapping register > regions is not a good idea. Can we instead have a top-level driver map all > the IO regions, then instantiate the various different sub-components > internally, and divide up the address space. Probably via MFD or similar. > That would prevent multiple drivers from touching the same register region. Perhaps I'm misunderstanding, but I don't see how MFD would prevent us from having to map this register space in two different locations - the XUSB FPCI address space cannot be divided cleanly between host and mailbox registers. Or are you saying that there should be a separate device driver that exposes an API for accessing this register space, like the Tegra fuse or PMC drivers? -- 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: exynos: remove usb_phy_generic support
Hi Baltlomiej,
On Wed, Aug 27, 2014 at 1:22 PM, Bartlomiej Zolnierkiewicz
wrote:
> dwc3 driver is using the new Exynos5 SoC series USB DRD PHY driver
> (PHY_EXYNOS5_USBDRD which selects GENERIC_PHY) as can be seen by
> looking at the following commits:
>
> 7a4cf0fde054 ("ARM: dts: Update DWC3 usb controller to use new
> phy driver for exynos5250")
>
> f070267b5fc1 ("ARM: dts: Enable support for DWC3 controller for
> exynos5420")
>
> Thus remove unused usb_phy_generic support from dwc3 Exynos glue
> layer.
>
> [ The code that is being removed is harmful in the context of
> multi_v7_defconfig and enabling "EHCI support for Samsung
> S5P/EXYNOS SoC Series" (USB_EHCI_EXYNOS) + "OHCI support for
> Samsung S5P/EXYNOS SoC Series" (USB_OHCI_EXYNOS) because "EHCI
> support for OMAP3 and later chips" (USB_EHCI_HCD_OMAP) selects
> "NOP USB Transceiver Driver" (NOP_USB_XCEIV). NOP USB driver
> attaches itself to usb_phy_generic platform devices created by
> dwc3 Exynos glue layer and later causes Exynos EHCI driver to
> fail probe and Exynos OHCI driver to hang on probe (as observed
> on Exynos5250 Arndale board). ]
The issue with EHCI and OHCI on exynos platforms is that until now
they also request
usb-phy and only later if they don't find one, they go ahead and get a
'phy' generic.
Fortunately we missed this issue with exynos_defconfig, and as you rightly
mentioned with multi_v7_defconfig, the NOP_USB_XCEIV gets enabled and
EHCI and OHCI exynos get no-op usb-phy, which actually blocks EHCI/OHCI from
initializing the real PHYs.
This issue is resolved with patches:
[PATCH v2 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support
[PATCH v2 2/2] usb: host: ohci-exynos: Remove unnecessary usb-phy support
wherein we are completely removing the usb-phy support from ehci/ohci-exynos.
So with these patches we should not see the issue mentioned by you.
Now for the DWC3 part, the usb-phy part was added to use register two
separate no-op-xceivers of USB2_PHY type and USB3_PHY type,
so that platforms with no separate PHY can still be able to use dwc3.
>
> Cc: Olof Johansson
> Cc: Kukjin Kim
> Cc: Vivek Gautam
> Acked-by: Kyungmin Park
> Signed-off-by: Bartlomiej Zolnierkiewicz
> ---
> drivers/usb/dwc3/dwc3-exynos.c | 68
> -
> 1 file changed, 1 insertion(+), 67 deletions(-)
>
> Index: b/drivers/usb/dwc3/dwc3-exynos.c
> ===
> --- a/drivers/usb/dwc3/dwc3-exynos.c2014-08-25 14:57:04.991781925 +0200
> +++ b/drivers/usb/dwc3/dwc3-exynos.c2014-08-27 09:16:38.312617727 +0200
> @@ -23,15 +23,12 @@
> #include
> #include
> #include
> -#include
> -#include
> +#include
> #include
> #include
> #include
>
> struct dwc3_exynos {
> - struct platform_device *usb2_phy;
> - struct platform_device *usb3_phy;
> struct device *dev;
>
> struct clk *clk;
> @@ -39,61 +36,6 @@ struct dwc3_exynos {
> struct regulator*vdd10;
> };
>
> -static int dwc3_exynos_register_phys(struct dwc3_exynos *exynos)
> -{
> - struct usb_phy_generic_platform_data pdata;
> - struct platform_device *pdev;
> - int ret;
> -
> - memset(&pdata, 0x00, sizeof(pdata));
> -
> - pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO);
> - if (!pdev)
> - return -ENOMEM;
> -
> - exynos->usb2_phy = pdev;
> - pdata.type = USB_PHY_TYPE_USB2;
> - pdata.gpio_reset = -1;
> -
> - ret = platform_device_add_data(exynos->usb2_phy, &pdata,
> sizeof(pdata));
> - if (ret)
> - goto err1;
> -
> - pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO);
> - if (!pdev) {
> - ret = -ENOMEM;
> - goto err1;
> - }
> -
> - exynos->usb3_phy = pdev;
> - pdata.type = USB_PHY_TYPE_USB3;
> -
> - ret = platform_device_add_data(exynos->usb3_phy, &pdata,
> sizeof(pdata));
> - if (ret)
> - goto err2;
> -
> - ret = platform_device_add(exynos->usb2_phy);
> - if (ret)
> - goto err2;
> -
> - ret = platform_device_add(exynos->usb3_phy);
> - if (ret)
> - goto err3;
> -
> - return 0;
> -
> -err3:
> - platform_device_del(exynos->usb2_phy);
> -
> -err2:
> - platform_device_put(exynos->usb3_phy);
> -
> -err1:
> - platform_device_put(exynos->usb2_phy);
> -
> - return ret;
> -}
> -
> static int dwc3_exynos_remove_child(struct device *dev, void *unused)
> {
> struct platform_device *pdev = to_platform_device(dev);
> @@ -127,12 +69,6 @@ static int dwc3_exynos_probe(struct plat
>
> platform_set_drvdata(pdev, exynos);
>
> - ret = dwc3_exynos_register_phys(exynos);
> - if (ret) {
> - dev_err(dev, "couldn't register PHYs\n");
> -
Re: [PATCH v2 2/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver
On 08/27/2014 12:13 PM, Andrew Bresticker wrote: On Wed, Aug 27, 2014 at 10:50 AM, Stephen Warren wrote: On 08/27/2014 11:38 AM, Andrew Bresticker wrote: On Mon, Aug 25, 2014 at 12:01 PM, Stephen Warren wrote: On 08/18/2014 11:08 AM, Andrew Bresticker wrote: +static int tegra_xusb_mbox_probe(struct platform_device *pdev) + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -ENODEV; Should devm_request_mem_region() be called here to claim the region? No, the xHCI host driver also needs to map these registers, so they cannot be mapped exclusively here. That's unfortunate. Having multiple drivers with overlapping register regions is not a good idea. Can we instead have a top-level driver map all the IO regions, then instantiate the various different sub-components internally, and divide up the address space. Probably via MFD or similar. That would prevent multiple drivers from touching the same register region. Perhaps I'm misunderstanding, but I don't see how MFD would prevent us from having to map this register space in two different locations - the XUSB FPCI address space cannot be divided cleanly between host and mailbox registers. Or are you saying that there should be a separate device driver that exposes an API for accessing this register space, like the Tegra fuse or PMC drivers? With MFD, there's typically a top-level driver for the HW module (or register space) that gets instantiated by the DT node. This driver then instantiates all the different sub-drivers that use that register space, and provides APIs for the sub-drivers to access the registers (either custom APIs or more recently by passing a regmap object down to the sub-drivers). This top-level driver is the only driver that maps the space, and can manage sharing the space between the various sub-drivers. That said, I haven't noticed many MFD drivers for MMIO devices. I certainly have seen multiple different drivers just re-mapping shared registers for themselves. It is simpler and does work. However, people usually mutter about it when it happens, since it's not clear which drivers are using what from the IO mapping registry. Using MFD or similar would allow the sharing to work in a clean fashion. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: composite: dequeue cdev->req before free its buffer
Hi, On Mon, Aug 25, 2014 at 04:49:12PM +0800, Li Jun wrote: > As Felipe suggested, dequeue the cdev->req before free its buffer. > > Suggested-by: Felipe Balbi > Signed-off-by: Li Jun please read Documentation/SubmittingPatches. Your commit log does not make sense. You need to explain why the change is necessary and what is it fixing. -- balbi signature.asc Description: Digital signature
Re: [PATCH 2/2] doc: dt: mxs-phy: add compatible string for imx6sx-usbphy
On Tue, Aug 26, 2014 at 10:55:18AM +0800, Peter Chen wrote: > Add compatible string for imx6sx-usbphy. > > Signed-off-by: Peter Chen do I need to take this one as well or will DT folks take care of patch 2 ? -- balbi signature.asc Description: Digital signature
Re: [RFC 2/2] usb: otg: Temporarily hold wakeupsource on charger and disconnect events
On Fri, Aug 22, 2014 at 03:20:00PM +0530, Kiran Kumar Raparthy wrote: > From: Todd Poynor > > usb: otg: Temporarily hold wakeupsource on charger and disconnect events > > Allow other parts of the system to react to the charger connect/disconnect > event without allowing the system to suspend before the other parts can > process > the event. This wakeup_source times out after 2 seconds; if nobody else holds > a > wakeup_source by that time then the device can sleep. > > This patch also refactoras the logic of wakeupsource_init,otg_notifications > and > handle event funtions refactoring should be in another patch. > This is one of the number of patches from the Android AOSP tegra.git tree, > which is used on Android devices. so I wanted to submit it for > review to see if it should go upstream. this paragraph doesn't need to be in commit log. -- balbi signature.asc Description: Digital signature
Re: [PATCH v6 2/3] usb: gadget: f_fs: add ioctl returning ep descriptor
Hi,
On Mon, Aug 25, 2014 at 11:16:28AM +0200, Robert Baldyga wrote:
> This patch introduces ioctl named FUNCTIONFS_ENDPOINT_DESC, which
> returns endpoint descriptor to userspace. It works only if function
> is active.
>
> Signed-off-by: Robert Baldyga
> Acked-by: Michal Nazarewicz
> ---
> drivers/usb/gadget/function/f_fs.c | 23 +++
> include/uapi/linux/usb/functionfs.h | 6 ++
> 2 files changed, 29 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c
> b/drivers/usb/gadget/function/f_fs.c
> index 0dc3552d..6edf7e4 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -1032,6 +1032,29 @@ static long ffs_epfile_ioctl(struct file *file,
> unsigned code,
> case FUNCTIONFS_ENDPOINT_REVMAP:
> ret = epfile->ep->num;
> break;
> + case FUNCTIONFS_ENDPOINT_DESC:
> + {
> + int desc_idx;
> + struct usb_endpoint_descriptor *desc;
> +
> + switch (epfile->ffs->gadget->speed) {
> + case USB_SPEED_SUPER:
> + desc_idx = 2;
> + break;
> + case USB_SPEED_HIGH:
> + desc_idx = 1;
> + break;
> + default:
> + desc_idx = 0;
> + }
> + desc = epfile->ep->descs[desc_idx];
> +
> + spin_unlock_irq(&epfile->ffs->eps_lock);
> + ret = copy_to_user((void *)value, desc, sizeof(*desc));
> + if (ret)
> + ret = -EFAULT;
> + return;
should return a value here. Have you tested this at all ?
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] usb: hub: Prevent hub autosuspend if usbcore.autosuspend is -1
On Wed, Aug 27, 2014 at 01:31:24PM +0300, Roger Quadros wrote: > On 08/04/2014 05:07 PM, Alan Stern wrote: > > On Mon, 4 Aug 2014, Roger Quadros wrote: > > > >> If user specifies that USB autosuspend must be disabled by module > >> parameter "usbcore.autosuspend=-1" then we must prevent > >> autosuspend of USB hub devices as well. > >> > >> commit 596d789a211d introduced in v3.8 changed the original behaivour > >> and stopped respecting the usbcore.autosuspend parameter for hubs. > >> > >> Fixes: 596d789a211d "USB: set hub's default autosuspend delay as 0" > >> > >> Cc: [3.8+] > >> Signed-off-by: Roger Quadros > > > > Acked-by: Alan Stern > > > > Just noticed the below error by the Intel's build script > > > tree: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git > > usb-linus > > head: 039368901ad0a6476c7ecf0cfe4f84d735e30135 > > commit: bdd405d2a5287bdb9b04670ea255e1f122138e66 [19/20] usb: hub: Prevent > > hub autosuspend if usbcore.autosuspend is -1 > > config: make ARCH=sparc64 defconfig > > > > All error/warnings: > > > >drivers/usb/core/hub.c: In function 'hub_probe': > >>> drivers/usb/core/hub.c:1735:21: error: 'struct dev_pm_info' has no member > >>> named 'autosuspend_delay' > > if (hdev->dev.power.autosuspend_delay >= 0) > > ^ > > Seems like the "if" has to be placed within a #ifdef CONFIG_PM_RUNTIME. > > Greg, > > Should I send a v2 of this patch or a fix on top of your usb-linus branch? Please send a fix-on-top, as I already have your patch applied, and can't remove 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: Problem with USB-to-SATA adapters (was: AS2105-based enclosure size issues with >2TB HDDs)
> From: James Bottomley > Did you try read capacity 16 on it? What happened? (the AS2105 rejects > read capacity 16, so there's no reliable way to deduce the capacity of > drives over 2TB). OK, I had to track down which package contains sg_readcap. The adapter that fails gives this output: # sg_readcap --16 /dev/sdb bad field in READ CAPACITY (16) cdb including unsupported service action The adapter that succeeds gives this output: # sg_readcap --16 /dev/sdc Read Capacity results: Protection: prot_en=0, p_type=0, p_i_exponent=0 Logical block provisioning: lbpme=0, lbprz=0 Last logical block address=5860533167 (0x15d50a3af), Number of logical blocks=5860533168 Logical block length=512 bytes Logical blocks per physical block exponent=0 Lowest aligned logical block address=0 Hence: Device size: 3000592982016 bytes, 2861588.5 MiB, 3000.59 GB Dale -- 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 02/13] usb: udc: set the udc is ready to pullup dp when it needs
On Wed, Aug 20, 2014 at 01:30:40PM +0800, Peter Chen wrote: > On Tue, Aug 19, 2014 at 09:02:54PM +, Paul Zimmerman wrote: > > > From: linux-usb-ow...@vger.kernel.org > > > [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Peter Chen > > > Sent: Tuesday, August 19, 2014 7:26 AM > > > > > > On Tue, Aug 19, 2014 at 01:46:17AM +, Paul Zimmerman wrote: > > > > > From: Peter Chen [mailto:peter.c...@freescale.com] > > > > > Sent: Sunday, August 17, 2014 9:14 PM > > > > > > > > > > Except for chipidea driver, all other udc drivers will tell the > > > > > gadget driver that they are ready to pullup dp at udc_start, it > > > > > is the default behaviour. > > > > > > > > > > The chipidea driver is ready to pullup dp when the vbus is there, > > > > > and isn't ready to pullup dp when the vbus is not there. Other > > > > > udc drivers which should not pull up when vbus is not there should > > > > > change like chipidea does to pass the Back-Voltage Testing at > > > > > www.usb.org/developers/docs/USB-IFTestProc1_3.pdf. > > > > > > > > > > Signed-off-by: Peter Chen > > > > > --- > > > > > drivers/usb/chipidea/udc.c |9 - > > > > > drivers/usb/dwc2/gadget.c |2 ++ > > > > > drivers/usb/dwc3/gadget.c |2 ++ > > > > > drivers/usb/gadget/udc/amd5536udc.c |1 + > > > > > drivers/usb/gadget/udc/atmel_usba_udc.c |2 ++ > > > > > drivers/usb/gadget/udc/bcm63xx_udc.c|2 ++ > > > > > drivers/usb/gadget/udc/dummy_hcd.c |1 + > > > > > drivers/usb/gadget/udc/fotg210-udc.c|1 + > > > > > drivers/usb/gadget/udc/fsl_qe_udc.c |1 + > > > > > drivers/usb/gadget/udc/fsl_udc_core.c |2 ++ > > > > > drivers/usb/gadget/udc/fusb300_udc.c|1 + > > > > > drivers/usb/gadget/udc/gr_udc.c |2 ++ > > > > > drivers/usb/gadget/udc/lpc32xx_udc.c|2 ++ > > > > > drivers/usb/gadget/udc/m66592-udc.c |2 ++ > > > > > drivers/usb/gadget/udc/mv_u3d_core.c|1 + > > > > > drivers/usb/gadget/udc/mv_udc_core.c|2 ++ > > > > > drivers/usb/gadget/udc/net2272.c|1 + > > > > > drivers/usb/gadget/udc/net2280.c|1 + > > > > > drivers/usb/gadget/udc/omap_udc.c |1 + > > > > > drivers/usb/gadget/udc/pch_udc.c|1 + > > > > > drivers/usb/gadget/udc/pxa25x_udc.c |1 + > > > > > drivers/usb/gadget/udc/pxa27x_udc.c |1 + > > > > > drivers/usb/gadget/udc/r8a66597-udc.c |1 + > > > > > drivers/usb/gadget/udc/s3c-hsudc.c |1 + > > > > > drivers/usb/gadget/udc/s3c2410_udc.c|1 + > > > > > drivers/usb/musb/musb_gadget.c |2 ++ > > > > > drivers/usb/renesas_usbhs/mod_gadget.c |7 ++- > > > > > 27 files changed, 45 insertions(+), 6 deletions(-) > > > > > > > > Instead of modifying all of the UDC drivers, how about adding a flag to > > > > 'struct usb_gadget' named 'needs_ready' or similar? If the UDC doesn't > > > > set the flag, udc-core would call usb_udc_ready_to_connect() on behalf > > > > of the UDC. If the UDC sets the flag (chipidea only?) then the UDC would > > > > be responsible for calling usb_udc_ready_to_connect(). > > > > > > > > > > USB spec requires dp is not pulled up when the vbus is not there, the > > > dwc3 is > > > the newest core, I don't think other older udc cores all has similar > > > capability > > > that does don't draw back voltage if software pullup bit is set and vbus > > > is > > > not there. > > > > > > This patchset will delete the unconditional pullup dp operation at > > > udc-core, > > > and we need to pullup dp at the end of hardware initialization (not > > > consider > > > vbus case), then the end of .udc_start at udc driver is the old place. > > > > I think you misunderstood my suggestion. Since you are adding a call > > to usb_udc_ready_to_connect() at the end of almost every .udc_start > > function, why not have udc-core do it instead, immediately after the > > call to .udc_start? Unless the 'needs_ready' flag is set, which would > > only be set by the udc drivers for those controllers that need it. > > > > Thanks. > > Yes, we can do that, my original plan is > usb_gadget_connect/usb_gadget_disconnect > are only called by gadget driver. If Felipe has no comment for it, I will not directly though. At least not for those using the composite layer. All functions using composite layer should use activate/deactivate. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 0/5] usb: phy: samsung: remove old USB PHY code
Hi, On Fri, Aug 22, 2014 at 06:59:00PM +0200, Bartlomiej Zolnierkiewicz wrote: > Hi, > > This patch series removes the old Samsung USB PHY drivers that > got replaced by the new ones using the generic PHY layer. > > Depends on: > - v3.17-rc1 branch of Linus' kernel > > Changes since v1 (https://lkml.org/lkml/2014/8/14/241): > - rebased on v3.17-rc1 kernel > - added Acked-by-s from Kishon Vijay Abraham I > - added Reviewed-by-s from Vivek Gautam and Jingoo Han So, everybody agrees that there will be no regressions, right ? I'll merge this on my testing/next. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2] usb: hub: Prevent hub autosuspend if usbcore.autosuspend is -1
On Wed, Aug 27, 2014 at 03:11:10PM +0300, Roger Quadros wrote: > If user specifies that USB autosuspend must be disabled by module > parameter "usbcore.autosuspend=-1" then we must prevent > autosuspend of USB hub devices as well. > > commit 596d789a211d introduced in v3.8 changed the original behaivour > and stopped respecting the usbcore.autosuspend parameter for hubs. > > Fixes: 596d789a211d "USB: set hub's default autosuspend delay as 0" > > Cc: [3.8+] > Signed-off-by: Roger Quadros > --- > drivers/usb/core/hub.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index 8a4dcbc..59b599d 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -1728,8 +1728,14 @@ static int hub_probe(struct usb_interface *intf, const > struct usb_device_id *id) >* - Change autosuspend delay of hub can avoid unnecessary auto >* suspend timer for hub, also may decrease power consumption >* of USB bus. > + * > + * - If user has indicated to prevent autosuspend by passing > + * usbcore.autosuspend = -1 then keep autosuspend disabled. >*/ > - pm_runtime_set_autosuspend_delay(&hdev->dev, 0); > +#ifdef CONFIG_PM_RUNTIME > + if (hdev->dev.power.autosuspend_delay >= 0) > + pm_runtime_set_autosuspend_delay(&hdev->dev, 0); > +#endif > > /* >* Hubs have proper suspend/resume support, except for root hubs Sorry, but can I have a patch that just adds the #ifdef as I already have your original one in my tree. 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: Problem with USB-to-SATA adapters (was: AS2105-based enclosure size issues with >2TB HDDs)
What I find interesting is that Windows (at least, Windows 7 Professional) seems to be able to handle the deficient adapter. What I'd like to do is log the disk commands during the mounting sequence, preferably at both the SCSI and USB layers. Then at least we'll know exactly what the driver is doing. Are there kernel options to do this? Unfortunately, I don't know any way to log what Windows is doing with the drive. Dale -- 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 5/5] usb: phy: samsung: remove old common USB PHY code
Hi, On Fri, Aug 22, 2014 at 06:59:05PM +0200, Bartlomiej Zolnierkiewicz wrote: > drivers/usb/phy/phy-samsung-usb[2,3] drivers got replaced by > drivers/phy/phy-samsung-usb[2,3] ones and the old common Samsung > USB PHY code is no longer used. > > Signed-off-by: Bartlomiej Zolnierkiewicz > Acked-by: Kyungmin Park > Reviewed-by: Vivek Gautam > Reviewed-by: Jingoo Han > Acked-by: Kishon Vijay Abraham I > Cc: Kamil Debski this patch doesn't apply. Please rebase on my testing/next -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: net2280: Remove pci_class from PCI_TABLE
On Wed, Aug 27, 2014 at 03:00:29PM +0200, Ricardo Ribalda Delgado wrote: > Defining the vendor and the product id should be enough to discriminate > the device. > > The reason for this patch is that there is a missmatch betweed the > modalias showed by sysfs and the modalias generated by file2alias. > > One expects the programming interface in uppercase and the other > generates it in lowercase. I don't understand, what is wrong here? Who does it in uppercase and who in lower? And does it matter? It's just a numeric value that should not be used as a string compare. > This means that some implementations modprobe will fail to load the > driver. What implementations fail to work? Shouldn't we fix the root of the problem and not just patch up all drivers to display incorrect data? And I mean incorrect, as you are changing the values here from being very specific, to being much broader. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver
On 27 August 2014 23:08, Andrew Bresticker wrote: > On Mon, Aug 25, 2014 at 12:01 PM, Stephen Warren > wrote: >> I'm not even sure if it's appropriate for the low-level mailbox driver to >> know about the semantics of the message, rather than simply sending them on >> to the client driver? Perhaps when drivers register(?) for callbacks(?) for >> messages, they should state which types of messages they want to listen to? > > So there's not really a way for the client driver to tell the mailbox > driver which types of messages it wants to listen to on a particular > channel with the mailbox framework - it simply provides a way for > clients to bind with channels. I think there are a couple of options > here, either: a) have a channel per message (as you mentioned in the > previous patch), which allows the client to only register for messages > (channels) it wants to handle, or b) extend the mailbox framework to > allow shared channels so that both clients can receive messages on the > single channel and handle messages appropriately. The disadvantage > of (a) is that the commands are firmware defined and could > theoretically change between releases of the firmware, though I'm not > sure how common that is in practice. So that leaves (b) - Jassi, what > do you think about having shared (non-exclusive) channels? > n++ ... 'mailbox' is one such device that imbibes properties of local controller hardware and the remote firmware. Change in remote f/w's behavior might require the controller driver to change besides our client driver. A typical example is format of payloads (if involved) a client and mailbox controller driver have direct understanding of the packet format ... which is likely to change with remote f/w. So if the original concern "why are we doing s/w protocol stuff in controller driver?" won't go away by providing for shared channels (which would have its own tradeoffs). BTW, on DMAEngine we are moving towards virtual channels backed by limited physical ones ... your setup looks quite similar. So your demuxer doesn't hurt my eyes at all. -jassi -- 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/2] usb: Add LED triggers for USB activity
On Wed, Aug 27, 2014 at 03:03:45PM +0200, Michal Sojka wrote:
> With this patch, USB activity can be signaled by blinking a LED. There
> are two triggers, one for activity on USB host and one for USB gadget.
>
> Both trigger should work with all host/device controllers. Tested only
> with musb.
>
> Signed-off-by: Michal Sojka
> ---
> drivers/usb/Kconfig | 11 ++
> drivers/usb/common/Makefile | 5 ++-
> drivers/usb/common/{usb-common.c => common.c} | 0
I don't mind renaming files, but don't "hide" it in this patch, do it in
a separate patch please, and then base your next patch on this one.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc3: exynos: remove usb_phy_generic support
Hi,
On Wed, Aug 27, 2014 at 11:42:25PM +0530, Vivek Gautam wrote:
> On Wed, Aug 27, 2014 at 1:22 PM, Bartlomiej Zolnierkiewicz
> wrote:
> > dwc3 driver is using the new Exynos5 SoC series USB DRD PHY driver
> > (PHY_EXYNOS5_USBDRD which selects GENERIC_PHY) as can be seen by
> > looking at the following commits:
> >
> > 7a4cf0fde054 ("ARM: dts: Update DWC3 usb controller to use new
> > phy driver for exynos5250")
> >
> > f070267b5fc1 ("ARM: dts: Enable support for DWC3 controller for
> > exynos5420")
> >
> > Thus remove unused usb_phy_generic support from dwc3 Exynos glue
> > layer.
> >
> > [ The code that is being removed is harmful in the context of
> > multi_v7_defconfig and enabling "EHCI support for Samsung
> > S5P/EXYNOS SoC Series" (USB_EHCI_EXYNOS) + "OHCI support for
> > Samsung S5P/EXYNOS SoC Series" (USB_OHCI_EXYNOS) because "EHCI
> > support for OMAP3 and later chips" (USB_EHCI_HCD_OMAP) selects
> > "NOP USB Transceiver Driver" (NOP_USB_XCEIV). NOP USB driver
> > attaches itself to usb_phy_generic platform devices created by
> > dwc3 Exynos glue layer and later causes Exynos EHCI driver to
> > fail probe and Exynos OHCI driver to hang on probe (as observed
> > on Exynos5250 Arndale board). ]
>
> The issue with EHCI and OHCI on exynos platforms is that until now
> they also request
> usb-phy and only later if they don't find one, they go ahead and get a
> 'phy' generic.
>
> Fortunately we missed this issue with exynos_defconfig, and as you rightly
> mentioned with multi_v7_defconfig, the NOP_USB_XCEIV gets enabled and
> EHCI and OHCI exynos get no-op usb-phy, which actually blocks EHCI/OHCI from
> initializing the real PHYs.
right, when I added PHY support to dwc3 I expected people to remote NOP
transceivers from their glue layers after they had proper PHY drivers
available.
> This issue is resolved with patches:
> [PATCH v2 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support
> [PATCH v2 2/2] usb: host: ohci-exynos: Remove unnecessary usb-phy support
> wherein we are completely removing the usb-phy support from ehci/ohci-exynos.
> So with these patches we should not see the issue mentioned by you.
>
> Now for the DWC3 part, the usb-phy part was added to use register two
> separate no-op-xceivers of USB2_PHY type and USB3_PHY type,
> so that platforms with no separate PHY can still be able to use dwc3.
correct.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 2/2] usb: Add LED triggers for USB activity
On Wed, Aug 27, 2014 at 12:27:51PM -0700, Greg Kroah-Hartman wrote:
> On Wed, Aug 27, 2014 at 03:03:45PM +0200, Michal Sojka wrote:
> > With this patch, USB activity can be signaled by blinking a LED. There
> > are two triggers, one for activity on USB host and one for USB gadget.
> >
> > Both trigger should work with all host/device controllers. Tested only
> > with musb.
> >
> > Signed-off-by: Michal Sojka
> > ---
> > drivers/usb/Kconfig | 11 ++
> > drivers/usb/common/Makefile | 5 ++-
> > drivers/usb/common/{usb-common.c => common.c} | 0
>
> I don't mind renaming files, but don't "hide" it in this patch, do it in
> a separate patch please, and then base your next patch on this one.
+1
yeah, this prevents us from reviewing the actual changes
--
balbi
signature.asc
Description: Digital signature
Re: [RFC PATCH 0/7] usb: gadget: add reset API at usb_gadget_driver
Hi, On Tue, Aug 26, 2014 at 03:30:32PM +0800, Peter Chen wrote: > On Mon, Aug 25, 2014 at 11:27:47AM -0400, Alan Stern wrote: > > On Mon, 25 Aug 2014, Peter Chen wrote: > > > > > Hi Felipe & Alan, > > > > > > It is the follow-up for: > > > http://www.spinics.net/lists/linux-usb/msg112193.html > > > > > > This patchset adds reset API at usb_gadget_driver, the UDC driver > > > can call it at bus_reset handler instead of calling disconnect. > > > The benefits of this patchset are: > > > - We can let the gadget driver do different things for bus reset. > > > and host is disconnected, eg, whether pull down dp or not. > > > - Match kernel doc for disconnect API, it says it is invoked > > > "when the host is disconnected". > > > - Will be more match for the real things the gadget driver > > > does for connect and disconnect, when we introduce connect API later. > > > > > > The reason for I mark RFC is I don't add the modification for mass > > > UDC driver changes, if you consider this patchset is ok, I will > > > add them without RFC later. > > > > This looks good. > > > > In patches 2, 4, and 5, shouldn't you call usb_gadget_disconnect() > > _before_ the gadget driver's original disconnect handler, instead of > > _after_? That's how we do it now. > > > > Hi Alan & Felipe, > > During the changing UDC drivers process, I find we can't simply move > usb_gadget_disconnect to usb_gadget_driver.disconnect, some UDC > drivers (like dwc3) will be no chance to set software pullup bit > (dp control always means software dp pullup bit if no specific at below) > again between disconnection and re-connection. sorry, can you rephrase this a bit, I really can't get what you mean. DWC3 doesn't really have a pullup bit. It's got a RUN/STOP bit. Sure, it controls the pullups but it also kills the internal DMA and quite a bit of other internal blocks. The way the code is today, we will have a pullup connected when a gadget driver is loaded and not before that. > There are two kinds of UDCs, dp is always pullup(UDC-A), pullup dp relies > on vbus (UDC-B), so we may need to introduce a flag like pullup_on_vbus > at struct usb_gadget, UDC-B needs to set this flag at .udc_start. > > For the whole gadget life cycle, the dp change for the two kinds of > UDC like below: > Process dp at UDC-A dp at UDC-B > insmod1 0 > connect 1 1 > bus_reset 1 1 > disconnect1 0 > rmmod 0 0 > > For insmod/rmmod gadget driver, we can control dp at udc-core relies > on flag pullup_on_vbus. > > For other three stages (no need to control at bus_reset), we can control dp > at two gadget driver's API relies on flag pullup_on_vbus. I also can't get what you mean here. > Do you agree above? > > If you agree, the first patchset for adding reset API at usb_gadget_driver may > do less thing, and the reset API implementation at each gadget driver is the > same with disconnect. that's why we never had a ->reset() callback so far. From a gadget driver perspective, it's the same as disconnect followed by a connect. > At the second patchset, we introduce the flag pullup_on_vbus, connect API > at usb_gadget_driver, change the disconnect implementation at > each gadget driver, and add control dp through function driver. IIRC, only mass storage gadget was showing a need for a ->reset() callback, why would you need to modify every gadget driver ? -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: net2280: Remove pci_class from PCI_TABLE
Hello Greg
On Wed, Aug 27, 2014 at 9:25 PM, Greg Kroah-Hartman
wrote:
> On Wed, Aug 27, 2014 at 03:00:29PM +0200, Ricardo Ribalda Delgado wrote:
>> Defining the vendor and the product id should be enough to discriminate
>> the device.
>>
>> The reason for this patch is that there is a missmatch betweed the
>> modalias showed by sysfs and the modalias generated by file2alias.
>>
>> One expects the programming interface in uppercase and the other
>> generates it in lowercase.
>
> I don't understand, what is wrong here? Who does it in uppercase and
> who in lower? And does it matter? It's just a numeric value that
> should not be used as a string compare.
>
In pci-sysfs:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/pci-sysfs.c#n175
static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
pci_dev->vendor, pci_dev->device,
pci_dev->subsystem_vendor, pci_dev->subsystem_device,
(u8)(pci_dev->class >> 16), (u8)(pci_dev->class >> 8),
(u8)(pci_dev->class));
}
In file2alias:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/scripts/mod/file2alias.c
#define ADD(str, sep, cond, field) \
do {\
strcat(str, sep); \
if (cond) \
sprintf(str + strlen(str), \
sizeof(field) == 1 ? "%02X" : \
sizeof(field) == 2 ? "%04X" : \
sizeof(field) == 4 ? "%08X" : "", \
field); \
else\
sprintf(str + strlen(str), "*");\
} while(0)
ADD(alias, "bc", baseclass_mask == 0xFF, baseclass);
ADD(alias, "sc", subclass_mask == 0xFF, subclass);
ADD(alias, "i", interface_mask == 0xFF, interface);
>> This means that some implementations modprobe will fail to load the
>> driver.
>
> What implementations fail to work? Shouldn't we fix the root of the
> problem and not just patch up all drivers to display incorrect data?
At least the implementation of kmod in yocproject does a case sensitive match.
I have already sent a patch to fix what I consider the root of the problem
https://lkml.org/lkml/2014/8/27/242
>
> And I mean incorrect, as you are changing the values here from being
> very specific, to being much broader.
>
Not many drivers define the pci interface and there is no other driver
that has the same vendor and product id. Therefore I see no hurt in
adding both patches, one to make the driver broader, and another to
fix pci-sysfs.
Also, the change on pci-sysfs might affect more stuff and therefore
take longer to be applied.
> thanks,
Thank you :)
>
> greg k-h
--
Ricardo Ribalda
--
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: Problem with USB-to-SATA adapters (was: AS2105-based enclosure size issues with >2TB HDDs)
On Wed, 2014-08-27 at 15:21 -0400, Dale R. Worley wrote: > > From: James Bottomley > > > Did you try read capacity 16 on it? What happened? (the AS2105 rejects > > read capacity 16, so there's no reliable way to deduce the capacity of > > drives over 2TB). > > OK, I had to track down which package contains sg_readcap. > > The adapter that fails gives this output: > > # sg_readcap --16 /dev/sdb > bad field in READ CAPACITY (16) cdb including unsupported service action OK, so that's definitely the tame taxonomy. James -- 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: Problem with USB-to-SATA adapters (was: AS2105-based enclosure size issues with >2TB HDDs)
On Wed, 27 Aug 2014, Dale R. Worley wrote: > What I find interesting is that Windows (at least, Windows 7 > Professional) seems to be able to handle the deficient adapter. So does Linux. The difference is that Windows believes the values in the partition table in preference to what the hardware says, whereas Linux believes the hardware in preference to the partition table. Thus, if the hardware says the disk contains 0.8 TB and the partition table says the first partition contains 2.8 TB, Windows will try to access all 2.8 TB but Linux will complain that the partition entry is invalid (because the partition extends beyond the end of the disk). If you try to repartition the drive under Windows using the deficient adapter, you'll see that the problem still exists. It just doesn't show up during normal use. > What > I'd like to do is log the disk commands during the mounting sequence, > preferably at both the SCSI and USB layers. Then at least we'll know > exactly what the driver is doing. Are there kernel options to do > this? You can record the USB transfers by using usbmon (see Documentation/usb/usbmon.txt). The trace will include all the important SCSI data, so you don't need to record anything at the SCSI layer. This isn't really necessary, though. We already know what the driver is doing. > Unfortunately, I don't know any way to log what Windows is doing with > the drive. My experiments with Windows show that it does essentially the same things as Linux does. The important difference is not what commands and data get sent, but whether the OS pays attention to the result. 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 v3 1/2] usb: gadget: Refactor request completion
On Wed, Aug 27 2014, Michal Sojka wrote:
> All USB peripheral controller drivers called completion routines
> directly. This patch moves the completion call from drivers to
> usb_gadget_giveback_request(), in order to have a place where common
> functionality can be added.
>
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.
[...]
> diff --git a/drivers/usb/gadget/udc/udc-core.c
> b/drivers/usb/gadget/udc/udc-core.c
> index b0d9817..2eb0ae4 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -102,6 +102,15 @@ void usb_gadget_unmap_request(struct usb_gadget *gadget,
> }
> EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>
> +void usb_gadget_giveback_request(struct usb_ep *ep,
> + struct usb_request *req)
> +{
> + /* complete() is from gadget layer,
> + * eg fsg->bulk_in_complete() */
> + if (req->complete)
> + req->complete(ep, req);
> +}
> +EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
> #endif /* CONFIG_HAS_DMA */
#endif should be before usb_gadget_giveback_request(). I'll fix this in
v4.
-Michal
--
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 1/2] usb: gadget: Refactor request completion
On Wed, Aug 27, 2014 at 03:03:44PM +0200, Michal Sojka wrote:
> All USB peripheral controller drivers called completion routines
> directly. This patch moves the completion call from drivers to
> usb_gadget_giveback_request(), in order to have a place where common
> functionality can be added.
>
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.
>
> Signed-off-by: Michal Sojka
> ---
> drivers/usb/chipidea/udc.c | 6 +++---
> drivers/usb/dwc2/gadget.c | 6 +++---
> drivers/usb/dwc3/gadget.c | 2 +-
> drivers/usb/gadget/udc/amd5536udc.c | 2 +-
> drivers/usb/gadget/udc/at91_udc.c | 2 +-
> drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++--
> drivers/usb/gadget/udc/bcm63xx_udc.c| 2 +-
> drivers/usb/gadget/udc/dummy_hcd.c | 10 +-
> drivers/usb/gadget/udc/fotg210-udc.c| 2 +-
> drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +-
> drivers/usb/gadget/udc/fsl_udc_core.c | 6 ++
> drivers/usb/gadget/udc/fusb300_udc.c| 2 +-
> drivers/usb/gadget/udc/goku_udc.c | 2 +-
> drivers/usb/gadget/udc/gr_udc.c | 2 +-
> drivers/usb/gadget/udc/lpc32xx_udc.c| 2 +-
> drivers/usb/gadget/udc/m66592-udc.c | 2 +-
> drivers/usb/gadget/udc/mv_u3d_core.c| 8 ++--
> drivers/usb/gadget/udc/mv_udc_core.c| 8 ++--
> drivers/usb/gadget/udc/net2272.c| 2 +-
> drivers/usb/gadget/udc/net2280.c| 2 +-
> drivers/usb/gadget/udc/omap_udc.c | 2 +-
> drivers/usb/gadget/udc/pch_udc.c| 2 +-
> drivers/usb/gadget/udc/pxa25x_udc.c | 2 +-
> drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
> drivers/usb/gadget/udc/r8a66597-udc.c | 2 +-
> drivers/usb/gadget/udc/s3c-hsudc.c | 3 +--
> drivers/usb/gadget/udc/s3c2410_udc.c| 2 +-
> drivers/usb/gadget/udc/udc-core.c | 9 +
> drivers/usb/musb/musb_gadget.c | 2 +-
> drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
> include/linux/usb/gadget.h | 8
> 31 files changed, 58 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index b8125aa..0444d3f 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -627,7 +627,7 @@ __acquires(hwep->lock)
>
> if (hwreq->req.complete != NULL) {
> spin_unlock(hwep->lock);
> - hwreq->req.complete(&hwep->ep, &hwreq->req);
> + usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
> spin_lock(hwep->lock);
> }
> }
> @@ -922,7 +922,7 @@ __acquires(hwep->lock)
> if ((hwep->type == USB_ENDPOINT_XFER_CONTROL) &&
> hwreq->req.length)
> hweptemp = hwep->ci->ep0in;
> - hwreq->req.complete(&hweptemp->ep, &hwreq->req);
> + usb_gadget_giveback_request(&hweptemp->ep, &hwreq->req);
> spin_lock(hwep->lock);
> }
> }
> @@ -1347,7 +1347,7 @@ static int ep_dequeue(struct usb_ep *ep, struct
> usb_request *req)
>
> if (hwreq->req.complete != NULL) {
> spin_unlock(hwep->lock);
> - hwreq->req.complete(&hwep->ep, &hwreq->req);
> + usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
> spin_lock(hwep->lock);
> }
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 0ba9c33..5a524a6 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -987,8 +987,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg
> *hsotg,
> hs_req = ep->req;
> ep->req = NULL;
> list_del_init(&hs_req->queue);
> - hs_req->req.complete(&ep->ep,
> - &hs_req->req);
> + usb_gadget_giveback_request(&ep->ep,
> +
> &hs_req->req);
> }
>
> /* If we have pending request, then start it */
> @@ -1245,7 +1245,7 @@ static void s3c_hsotg_complete_request(struct s3c_hsotg
> *hsotg,
>
> if (hs_req->req.complete) {
> spin_unlock(&hsotg->lock);
> - hs_req->req.complete(&hs_ep->ep, &hs_req->req);
> + usb_gadget_giveback_request(&hs_ep->ep, &hs_req->req);
> spin_lock(&hsotg->lock);
> }
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 349cacc..b4b7a6b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/g
Re: [RFC PATCH 0/7] usb: gadget: add reset API at usb_gadget_driver
On Wed, 27 Aug 2014, Felipe Balbi wrote: > > Hi Alan & Felipe, > > > > During the changing UDC drivers process, I find we can't simply move > > usb_gadget_disconnect to usb_gadget_driver.disconnect, some UDC > > drivers (like dwc3) will be no chance to set software pullup bit > > (dp control always means software dp pullup bit if no specific at below) > > again between disconnection and re-connection. > > sorry, can you rephrase this a bit, I really can't get what you mean. > > DWC3 doesn't really have a pullup bit. It's got a RUN/STOP bit. Sure, it > controls the pullups but it also kills the internal DMA and quite a bit > of other internal blocks. > > The way the code is today, we will have a pullup connected when a gadget > driver is loaded and not before that. > > > There are two kinds of UDCs, dp is always pullup(UDC-A), pullup dp relies > > on vbus (UDC-B), so we may need to introduce a flag like pullup_on_vbus > > at struct usb_gadget, UDC-B needs to set this flag at .udc_start. > > > > For the whole gadget life cycle, the dp change for the two kinds of > > UDC like below: > > Process dp at UDC-A dp at UDC-B > > insmod 1 0 > > connect 1 1 > > bus_reset 1 1 > > disconnect 1 0 > > rmmod 0 0 > > > > For insmod/rmmod gadget driver, we can control dp at udc-core relies > > on flag pullup_on_vbus. > > > > For other three stages (no need to control at bus_reset), we can control dp > > at two gadget driver's API relies on flag pullup_on_vbus. > > I also can't get what you mean here. I think Peter is saying that some UDC hardware (which he calls UDC-A) automatically turns off the pullup when Vbus is absent, whereas other hardware (UDC-B) relies on software to do this. I'm not sure why this matters, however. Does anything go wrong if the driver tells UDC-A hardware to turn off the pullup when the cable is unplugged and Vbus goes away? > > Do you agree above? > > > > If you agree, the first patchset for adding reset API at usb_gadget_driver > > may > > do less thing, and the reset API implementation at each gadget driver is the > > same with disconnect. > > that's why we never had a ->reset() callback so far. From a gadget > driver perspective, it's the same as disconnect followed by a connect. > > > At the second patchset, we introduce the flag pullup_on_vbus, connect API > > at usb_gadget_driver, change the disconnect implementation at > > each gadget driver, and add control dp through function driver. > > IIRC, only mass storage gadget was showing a need for a ->reset() > callback, why would you need to modify every gadget driver ? The mass-storage gadget is a function driver, not a gadget driver. Even g_mass_storage.c is simply a single-function wrapper; it still relies on the composite layer. There are only four gadget drivers in the tree: composite, configfs, gadgetfs, and dbgp. 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: gadget: net2280: Remove pci_class from PCI_TABLE
On Wed, Aug 27, 2014 at 09:39:43PM +0200, Ricardo Ribalda Delgado wrote:
> Hello Greg
>
>
>
>
>
> On Wed, Aug 27, 2014 at 9:25 PM, Greg Kroah-Hartman
> wrote:
> > On Wed, Aug 27, 2014 at 03:00:29PM +0200, Ricardo Ribalda Delgado wrote:
> >> Defining the vendor and the product id should be enough to discriminate
> >> the device.
> >>
> >> The reason for this patch is that there is a missmatch betweed the
> >> modalias showed by sysfs and the modalias generated by file2alias.
> >>
> >> One expects the programming interface in uppercase and the other
> >> generates it in lowercase.
> >
> > I don't understand, what is wrong here? Who does it in uppercase and
> > who in lower? And does it matter? It's just a numeric value that
> > should not be used as a string compare.
> >
>
> In pci-sysfs:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/pci-sysfs.c#n175
>
> static ssize_t modalias_show(struct device *dev, struct device_attribute
> *attr,
> char *buf)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
>
> return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
> pci_dev->vendor, pci_dev->device,
> pci_dev->subsystem_vendor, pci_dev->subsystem_device,
> (u8)(pci_dev->class >> 16), (u8)(pci_dev->class >> 8),
> (u8)(pci_dev->class));
> }
>
>
> In file2alias:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/scripts/mod/file2alias.c
>
> #define ADD(str, sep, cond, field) \
> do {\
> strcat(str, sep); \
> if (cond) \
> sprintf(str + strlen(str), \
> sizeof(field) == 1 ? "%02X" : \
> sizeof(field) == 2 ? "%04X" : \
> sizeof(field) == 4 ? "%08X" : "", \
> field); \
> else\
> sprintf(str + strlen(str), "*");\
> } while(0)
>
>
> ADD(alias, "bc", baseclass_mask == 0xFF, baseclass);
> ADD(alias, "sc", subclass_mask == 0xFF, subclass);
> ADD(alias, "i", interface_mask == 0xFF, interface);
>
>
>
> >> This means that some implementations modprobe will fail to load the
> >> driver.
> >
> > What implementations fail to work? Shouldn't we fix the root of the
> > problem and not just patch up all drivers to display incorrect data?
>
> At least the implementation of kmod in yocproject does a case sensitive match.
>
>
> I have already sent a patch to fix what I consider the root of the problem
>
> https://lkml.org/lkml/2014/8/27/242
No, the root cause of the problem is a userspace tool looking at a hex
value as a string and not a number. It doesn't matter if we print it in
upper or lower case, it's a digit, not a string. Do the numeric
compare, not a string compare.
> > And I mean incorrect, as you are changing the values here from being
> > very specific, to being much broader.
> >
>
> Not many drivers define the pci interface and there is no other driver
> that has the same vendor and product id. Therefore I see no hurt in
> adding both patches, one to make the driver broader, and another to
> fix pci-sysfs.
>
> Also, the change on pci-sysfs might affect more stuff and therefore
> take longer to be applied.
As we have been printing the value to userspace in this way for well
over a decade now, and nothing has changed, I say it's a userspace bug
that you should fix instead. Don't work around broken user programs in
the kernel by changing something that has been stable for 10+ years.
Ok, sorry, not 10+ years, the commit was written May of 2005, so 9
years.
Fix your module loading code please.
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: [RFC PATCH 0/7] usb: gadget: add reset API at usb_gadget_driver
On Wed, Aug 27, 2014 at 04:20:25PM -0400, Alan Stern wrote: > On Wed, 27 Aug 2014, Felipe Balbi wrote: > > > > Hi Alan & Felipe, > > > > > > During the changing UDC drivers process, I find we can't simply move > > > usb_gadget_disconnect to usb_gadget_driver.disconnect, some UDC > > > drivers (like dwc3) will be no chance to set software pullup bit > > > (dp control always means software dp pullup bit if no specific at below) > > > again between disconnection and re-connection. > > > > sorry, can you rephrase this a bit, I really can't get what you mean. > > > > DWC3 doesn't really have a pullup bit. It's got a RUN/STOP bit. Sure, it > > controls the pullups but it also kills the internal DMA and quite a bit > > of other internal blocks. > > > > The way the code is today, we will have a pullup connected when a gadget > > driver is loaded and not before that. > > > > > There are two kinds of UDCs, dp is always pullup(UDC-A), pullup dp relies > > > on vbus (UDC-B), so we may need to introduce a flag like pullup_on_vbus > > > at struct usb_gadget, UDC-B needs to set this flag at .udc_start. > > > > > > For the whole gadget life cycle, the dp change for the two kinds of > > > UDC like below: > > > Process dp at UDC-A dp at UDC-B > > > insmod1 0 > > > connect 1 1 > > > bus_reset 1 1 > > > disconnect1 0 > > > rmmod 0 0 > > > > > > For insmod/rmmod gadget driver, we can control dp at udc-core relies > > > on flag pullup_on_vbus. > > > > > > For other three stages (no need to control at bus_reset), we can control > > > dp > > > at two gadget driver's API relies on flag pullup_on_vbus. > > > > I also can't get what you mean here. > > I think Peter is saying that some UDC hardware (which he calls UDC-A) > automatically turns off the pullup when Vbus is absent, whereas other > hardware (UDC-B) relies on software to do this. > > I'm not sure why this matters, however. Does anything go wrong if the > driver tells UDC-A hardware to turn off the pullup when the cable is > unplugged and Vbus goes away? nope, nothing bad should happen. > > > Do you agree above? > > > > > > If you agree, the first patchset for adding reset API at > > > usb_gadget_driver may > > > do less thing, and the reset API implementation at each gadget driver is > > > the > > > same with disconnect. > > > > that's why we never had a ->reset() callback so far. From a gadget > > driver perspective, it's the same as disconnect followed by a connect. > > > > > At the second patchset, we introduce the flag pullup_on_vbus, connect API > > > at usb_gadget_driver, change the disconnect implementation at > > > each gadget driver, and add control dp through function driver. > > > > IIRC, only mass storage gadget was showing a need for a ->reset() > > callback, why would you need to modify every gadget driver ? > > The mass-storage gadget is a function driver, not a gadget driver. > Even g_mass_storage.c is simply a single-function wrapper; it still > relies on the composite layer. > > There are only four gadget drivers in the tree: composite, configfs, > gadgetfs, and dbgp. right, but those are the only ones which should be modified. For all gadget drivers which are composed of function drivers (even single function drivers) they should rely on the function knowing what to do, otherwise we might still mistakenly connect to the host when some userspace daemon isn't ready yet. cheers -- balbi signature.asc Description: Digital signature
[PATCH v4 2/3] usb: Rename usb-common.c
In the next commit, we will want the usb-common module to be composed of
two object files. Since Kbuild cannot "append" another object to an
existing one, we need to rename usb-common.c to something
else (common.c) and create usb-common.o by linking the wanted objects
together. Currently, usb-common.o comprises only common.o.
Signed-off-by: Michal Sojka
---
drivers/usb/common/Makefile | 4 +++-
drivers/usb/common/{usb-common.c => common.c} | 0
2 files changed, 3 insertions(+), 1 deletion(-)
rename drivers/usb/common/{usb-common.c => common.c} (100%)
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 7526461..052c120 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -2,5 +2,7 @@
# Makefile for the usb common parts.
#
-obj-$(CONFIG_USB_COMMON) += usb-common.o
+obj-$(CONFIG_USB_COMMON) += usb-common.o
+usb-common-y += common.o
+
obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
diff --git a/drivers/usb/common/usb-common.c b/drivers/usb/common/common.c
similarity index 100%
rename from drivers/usb/common/usb-common.c
rename to drivers/usb/common/common.c
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 3/3] usb: Add LED triggers for USB activity
With this patch, USB activity can be signaled by blinking a LED. There
are two triggers, one for activity on USB host and one for USB gadget.
Both trigger should work with all host/device controllers. Tested only
with musb.
Signed-off-by: Michal Sojka
---
drivers/usb/Kconfig | 11
drivers/usb/common/Makefile | 1 +
drivers/usb/common/led.c | 56 +++
drivers/usb/core/hcd.c| 2 ++
drivers/usb/gadget/udc/udc-core.c | 4 +++
include/linux/usb.h | 12 +
6 files changed, 86 insertions(+)
create mode 100644 drivers/usb/common/led.c
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index e0cad441..fc90cda 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -147,4 +147,15 @@ source "drivers/usb/phy/Kconfig"
source "drivers/usb/gadget/Kconfig"
+config USB_LED_TRIG
+ bool "USB LED Triggers"
+ depends on LEDS_CLASS && USB_COMMON
+ select LEDS_TRIGGERS
+ help
+ This option adds LED triggers for USB host and/or gadget activity.
+
+ Say Y here if you are working on a system with led-class supported
+ LEDs and you want to use them as activity indicators for USB host or
+ gadget.
+
endif # USB_SUPPORT
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 052c120..ca2f8bd 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -4,5 +4,6 @@
obj-$(CONFIG_USB_COMMON) += usb-common.o
usb-common-y += common.o
+usb-common-$(CONFIG_USB_LED_TRIG) += led.o
obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
new file mode 100644
index 000..af3ce2c
--- /dev/null
+++ b/drivers/usb/common/led.c
@@ -0,0 +1,56 @@
+/*
+ * LED Triggers for USB Activity
+ *
+ * Copyright 2014 Michal Sojka
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#define BLINK_DELAY 30
+
+static unsigned long usb_blink_delay = BLINK_DELAY;
+
+DEFINE_LED_TRIGGER(ledtrig_usb_gadget);
+DEFINE_LED_TRIGGER(ledtrig_usb_host);
+
+void usb_led_activity(enum usb_led_event ev)
+{
+ struct led_trigger *trig = NULL;
+
+ switch (ev) {
+ case USB_LED_EVENT_GADGET: trig = ledtrig_usb_gadget; break;
+ case USB_LED_EVENT_HOST: trig = ledtrig_usb_host;break;
+ }
+ led_trigger_blink_oneshot(trig, &usb_blink_delay, &usb_blink_delay, 0);
+}
+EXPORT_SYMBOL(usb_led_activity);
+
+
+static int __init ledtrig_usb_init(void)
+{
+#ifdef CONFIG_USB_GADGET
+ led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
+#endif
+#ifdef CONFIG_USB
+ led_trigger_register_simple("usb-host", &ledtrig_usb_host);
+#endif
+ return 0;
+}
+
+static void __exit ledtrig_usb_exit(void)
+{
+ led_trigger_unregister_simple(ledtrig_usb_gadget);
+ led_trigger_unregister_simple(ledtrig_usb_host);
+}
+
+module_init(ledtrig_usb_init);
+module_exit(ledtrig_usb_exit);
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 487abcf..409cb95 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
usbmon_urb_complete(&hcd->self, urb, status);
usb_anchor_suspend_wakeups(anchor);
usb_unanchor_urb(urb);
+ if (likely(status == 0))
+ usb_led_activity(USB_LED_EVENT_HOST);
/* pass ownership to the completion handler */
urb->status = status;
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index c1df19b..9fc4a36 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -27,6 +27,7 @@
#include
#include
+#include
/**
* struct usb_udc - describes one usb device controller
@@ -116,6 +117,9 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
void usb_gadget_giveback_request(struct usb_ep *ep,
struct usb_request *req)
{
+ if (likely(req->status == 0))
+ usb_led_activity(USB_LED_EVENT_GADGET);
+
BUG_ON(req->complete == NULL);
req->complete(ep, req);
}
diff --git a/include/linux/usb.h b/include/linux/usb.h
index d2465bc..447a7e2 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1862,6 +1862,18 @@ extern void usb_unregister_notify(struct notifier_block
*nb);
/* debugfs stuff */
extern struct dentry *usb_debug_root;
+/* LED triggers */
+enum usb_led_event {
+ USB_LED_EVENT_HOST = 0,
+ USB_LED_EVENT_GADGET = 1,
+};
+
+#ifdef CONFIG_USB_LED_TRIG
+extern void usb_led_activity(enum usb_led_event ev);
+#else
+static inline void usb_led_activity(enum usb_led_event ev) {}
+#endif
+
#endif /* __KERNEL__ */
#endif
--
2.1.0
[PATCH v4 1/3] usb: gadget: Refactor request completion
All USB peripheral controller drivers called completion routines
directly. This patch moves the completion call from drivers to
usb_gadget_giveback_request(), in order to have a place where common
functionality can be added.
All places in drivers/usb/ matching "[-.]complete(" were replaced with a
call to usb_gadget_giveback_request(). This was compile-tested with all
ARM drivers enabled and runtime-tested for musb.
Signed-off-by: Michal Sojka
---
drivers/usb/chipidea/udc.c | 6 +++---
drivers/usb/dwc2/gadget.c | 6 +++---
drivers/usb/dwc3/gadget.c | 2 +-
drivers/usb/gadget/udc/amd5536udc.c | 2 +-
drivers/usb/gadget/udc/at91_udc.c | 2 +-
drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++--
drivers/usb/gadget/udc/bcm63xx_udc.c| 2 +-
drivers/usb/gadget/udc/dummy_hcd.c | 10 +-
drivers/usb/gadget/udc/fotg210-udc.c| 2 +-
drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +-
drivers/usb/gadget/udc/fsl_udc_core.c | 6 ++
drivers/usb/gadget/udc/fusb300_udc.c| 2 +-
drivers/usb/gadget/udc/goku_udc.c | 2 +-
drivers/usb/gadget/udc/gr_udc.c | 2 +-
drivers/usb/gadget/udc/lpc32xx_udc.c| 2 +-
drivers/usb/gadget/udc/m66592-udc.c | 2 +-
drivers/usb/gadget/udc/mv_u3d_core.c| 8 ++--
drivers/usb/gadget/udc/mv_udc_core.c| 8 ++--
drivers/usb/gadget/udc/net2272.c| 2 +-
drivers/usb/gadget/udc/net2280.c| 2 +-
drivers/usb/gadget/udc/omap_udc.c | 2 +-
drivers/usb/gadget/udc/pch_udc.c| 2 +-
drivers/usb/gadget/udc/pxa25x_udc.c | 2 +-
drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
drivers/usb/gadget/udc/r8a66597-udc.c | 2 +-
drivers/usb/gadget/udc/s3c-hsudc.c | 3 +--
drivers/usb/gadget/udc/s3c2410_udc.c| 2 +-
drivers/usb/gadget/udc/udc-core.c | 17 +
drivers/usb/musb/musb_gadget.c | 2 +-
drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
include/linux/usb/gadget.h | 8
31 files changed, 66 insertions(+), 56 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index b8125aa..0444d3f 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -627,7 +627,7 @@ __acquires(hwep->lock)
if (hwreq->req.complete != NULL) {
spin_unlock(hwep->lock);
- hwreq->req.complete(&hwep->ep, &hwreq->req);
+ usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
spin_lock(hwep->lock);
}
}
@@ -922,7 +922,7 @@ __acquires(hwep->lock)
if ((hwep->type == USB_ENDPOINT_XFER_CONTROL) &&
hwreq->req.length)
hweptemp = hwep->ci->ep0in;
- hwreq->req.complete(&hweptemp->ep, &hwreq->req);
+ usb_gadget_giveback_request(&hweptemp->ep, &hwreq->req);
spin_lock(hwep->lock);
}
}
@@ -1347,7 +1347,7 @@ static int ep_dequeue(struct usb_ep *ep, struct
usb_request *req)
if (hwreq->req.complete != NULL) {
spin_unlock(hwep->lock);
- hwreq->req.complete(&hwep->ep, &hwreq->req);
+ usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
spin_lock(hwep->lock);
}
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0ba9c33..5a524a6 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -987,8 +987,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg
*hsotg,
hs_req = ep->req;
ep->req = NULL;
list_del_init(&hs_req->queue);
- hs_req->req.complete(&ep->ep,
-&hs_req->req);
+ usb_gadget_giveback_request(&ep->ep,
+
&hs_req->req);
}
/* If we have pending request, then start it */
@@ -1245,7 +1245,7 @@ static void s3c_hsotg_complete_request(struct s3c_hsotg
*hsotg,
if (hs_req->req.complete) {
spin_unlock(&hsotg->lock);
- hs_req->req.complete(&hs_ep->ep, &hs_req->req);
+ usb_gadget_giveback_request(&hs_ep->ep, &hs_req->req);
spin_lock(&hsotg->lock);
}
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 349cacc..b4b7a6b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -268,7 +268,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct
dwc3_request *req,
req->request.length, status);
spin_unlock(&dwc->lock);
-
[PATCH v4 0/3] LED triggers for USB host and device
This adds LED triggers for USB host and device. First patch refactors UDC drivers as requested by Felipe Balbi, second is a preparation for the third, which adds the LED triggers. Changes from v3: - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA conditioned block. - Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi). - Removed outdated comment (Alan Stern). - req->complete == NULL is now a bug. Previously, this was ignored (Alan Stern). - File rename moved to a separate commit (greg k-h). Changes from v2: - Host/gadget triggers merged to a single file in usb/common/ (Felipe Balbi). - UDC drivers refactored so that LED trigger works for all of them. Changes from v1: - Moved from drivers/leds/ to drivers/usb/. - Improved Kconfig help. - Linked with other modules rather than being standalone modules. Michal Sojka (2): usb: gadget: Refactor request completion usb: Add LED triggers for USB activity drivers/usb/Kconfig | 11 +++ drivers/usb/chipidea/udc.c | 6 +- drivers/usb/common/Makefile | 5 +- drivers/usb/common/common.c | 144 drivers/usb/common/led.c| 59 + drivers/usb/common/usb-common.c | 144 drivers/usb/core/hcd.c | 2 + drivers/usb/dwc2/gadget.c | 6 +- drivers/usb/dwc3/gadget.c | 2 +- drivers/usb/gadget/udc/amd5536udc.c | 2 +- drivers/usb/gadget/udc/at91_udc.c | 2 +- drivers/usb/gadget/udc/atmel_usba_udc.c | 4 +- drivers/usb/gadget/udc/bcm63xx_udc.c| 2 +- drivers/usb/gadget/udc/dummy_hcd.c | 10 +-- drivers/usb/gadget/udc/fotg210-udc.c| 2 +- drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +- drivers/usb/gadget/udc/fsl_udc_core.c | 6 +- drivers/usb/gadget/udc/fusb300_udc.c| 2 +- drivers/usb/gadget/udc/goku_udc.c | 2 +- drivers/usb/gadget/udc/gr_udc.c | 2 +- drivers/usb/gadget/udc/lpc32xx_udc.c| 2 +- drivers/usb/gadget/udc/m66592-udc.c | 2 +- drivers/usb/gadget/udc/mv_u3d_core.c| 8 +- drivers/usb/gadget/udc/mv_udc_core.c| 8 +- drivers/usb/gadget/udc/net2272.c| 2 +- drivers/usb/gadget/udc/net2280.c| 2 +- drivers/usb/gadget/udc/omap_udc.c | 2 +- drivers/usb/gadget/udc/pch_udc.c| 2 +- drivers/usb/gadget/udc/pxa25x_udc.c | 2 +- drivers/usb/gadget/udc/pxa27x_udc.c | 2 +- drivers/usb/gadget/udc/r8a66597-udc.c | 2 +- drivers/usb/gadget/udc/s3c-hsudc.c | 3 +- drivers/usb/gadget/udc/s3c2410_udc.c| 2 +- drivers/usb/gadget/udc/udc-core.c | 13 +++ drivers/usb/musb/musb_gadget.c | 2 +- drivers/usb/renesas_usbhs/mod_gadget.c | 2 +- include/linux/usb.h | 12 +++ include/linux/usb/gadget.h | 8 ++ 38 files changed, 294 insertions(+), 201 deletions(-) create mode 100644 drivers/usb/common/common.c create mode 100644 drivers/usb/common/led.c delete mode 100644 drivers/usb/common/usb-common.c -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: hub: Prevent hub autosuspend if usbcore.autosuspend is -1
On Wed, Aug 27, 2014 at 12:23:39PM -0700, Greg KH wrote: > On Wed, Aug 27, 2014 at 03:11:10PM +0300, Roger Quadros wrote: > > If user specifies that USB autosuspend must be disabled by module > > parameter "usbcore.autosuspend=-1" then we must prevent > > autosuspend of USB hub devices as well. > > > > commit 596d789a211d introduced in v3.8 changed the original behaivour > > and stopped respecting the usbcore.autosuspend parameter for hubs. > > > > Fixes: 596d789a211d "USB: set hub's default autosuspend delay as 0" > > > > Cc: [3.8+] > > Signed-off-by: Roger Quadros > > --- > > drivers/usb/core/hub.c | 8 +++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > > index 8a4dcbc..59b599d 100644 > > --- a/drivers/usb/core/hub.c > > +++ b/drivers/usb/core/hub.c > > @@ -1728,8 +1728,14 @@ static int hub_probe(struct usb_interface *intf, > > const struct usb_device_id *id) > > * - Change autosuspend delay of hub can avoid unnecessary auto > > * suspend timer for hub, also may decrease power consumption > > * of USB bus. > > +* > > +* - If user has indicated to prevent autosuspend by passing > > +* usbcore.autosuspend = -1 then keep autosuspend disabled. > > */ > > - pm_runtime_set_autosuspend_delay(&hdev->dev, 0); > > +#ifdef CONFIG_PM_RUNTIME > > + if (hdev->dev.power.autosuspend_delay >= 0) > > + pm_runtime_set_autosuspend_delay(&hdev->dev, 0); > > +#endif > > > > /* > > * Hubs have proper suspend/resume support, except for root hubs > > Sorry, but can I have a patch that just adds the #ifdef as I already > have your original one in my tree. This should be all that is needed, right? - diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 003cb6b1a6bf..46f5161c7891 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -1732,8 +1732,10 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id) * - If user has indicated to prevent autosuspend by passing * usbcore.autosuspend = -1 then keep autosuspend disabled. */ +#ifdef CONFIG_PM_RUNTIME if (hdev->dev.power.autosuspend_delay >= 0) pm_runtime_set_autosuspend_delay(&hdev->dev, 0); +#endif /* * Hubs have proper suspend/resume support, except for root hubs -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: net2280: Remove pci_class from PCI_TABLE
Hello Greg >> >> Not many drivers define the pci interface and there is no other driver >> that has the same vendor and product id. Therefore I see no hurt in >> adding both patches, one to make the driver broader, and another to >> fix pci-sysfs. >> >> Also, the change on pci-sysfs might affect more stuff and therefore >> take longer to be applied. > > As we have been printing the value to userspace in this way for well > over a decade now, and nothing has changed, I say it's a userspace bug > that you should fix instead. Don't work around broken user programs in > the kernel by changing something that has been stable for 10+ years. > > Ok, sorry, not 10+ years, the commit was written May of 2005, so 9 > years. > > Fix your module loading code please. On the other thread ( https://lkml.org/lkml/2014/8/27/242 ) we have agreed about fixing this thing on pci-sysfs.c . Still I think that there is no good reason to add the pci interface to the pci_table on this driver. Therefore I consider that this patch is still valid. What do you think. This patch is NACK? Thanks! > > thanks, > > greg k-h -- Ricardo Ribalda -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/3] usb: gadget: Refactor request completion
On Wed, 27 Aug 2014, Michal Sojka wrote:
> +/**
> + * usb_gadget_giveback_request - give the request back to the gadget layer
> + * Context: in_interrupt()
> + *
> + * This is called by device controller drivers in order to return the
> + * completed request back to the gadget layer.
> + */
> +void usb_gadget_giveback_request(struct usb_ep *ep,
> + struct usb_request *req)
> +{
> + BUG_ON(req->complete == NULL);
> + req->complete(ep, req);
> +}
I guess it doesn't hurt to have the BUG_ON, but it doesn't help either.
Think about what would happen if req->complete was NULL and the BUG_ON
wasn't present.
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
[PATCH 2/5] tools: ffs-test: convert to new descriptor format
Since commit [ac8dde11: “Add flags to descriptors block”] functionfs
supports a new, more powerful and extensible, descriptor format.
Since ffs-test is probably the first thing users of the functionfs
interface see when they start writing functionfs user space daemons,
convert it to use the new format thus promoting it.
Signed-off-by: Michal Nazarewicz
---
include/uapi/linux/usb/functionfs.h | 2 +-
tools/usb/ffs-test.c| 14 +-
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/usb/functionfs.h
b/include/uapi/linux/usb/functionfs.h
index 3ca03de..6d2a16b 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -102,7 +102,7 @@ struct usb_ext_prop_desc {
* structure. Any flags that are not recognised cause the whole block to be
* rejected with -ENOSYS.
*
- * Legacy descriptors format:
+ * Legacy descriptors format (deprecated as of 3.14):
*
* | off | name | type | description |
* |-+---+--+--|
diff --git a/tools/usb/ffs-test.c b/tools/usb/ffs-test.c
index a87e99f..708d317 100644
--- a/tools/usb/ffs-test.c
+++ b/tools/usb/ffs-test.c
@@ -1,5 +1,5 @@
/*
- * ffs-test.c.c -- user mode filesystem api for usb composite function
+ * ffs-test.c -- user mode filesystem api for usb composite function
*
* Copyright (C) 2010 Samsung Electronics
*Author: Michal Nazarewicz
@@ -106,7 +106,9 @@ static void _msg(unsigned level, const char *fmt, ...)
/ Descriptors and Strings ***/
static const struct {
- struct usb_functionfs_descs_head header;
+ struct usb_functionfs_descs_head_v2 header;
+ __le32 fs_count;
+ __le32 hs_count;
struct {
struct usb_interface_descriptor intf;
struct usb_endpoint_descriptor_no_audio sink;
@@ -114,11 +116,12 @@ static const struct {
} __attribute__((packed)) fs_descs, hs_descs;
} __attribute__((packed)) descriptors = {
.header = {
- .magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC),
+ .magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC_V2),
+ .flags = cpu_to_le32(FUNCTIONFS_HAS_FS_DESC |
+FUNCTIONFS_HAS_HS_DESC),
.length = cpu_to_le32(sizeof descriptors),
- .fs_count = cpu_to_le32(3),
- .hs_count = cpu_to_le32(3),
},
+ .fs_count = cpu_to_le32(3),
.fs_descs = {
.intf = {
.bLength = sizeof descriptors.fs_descs.intf,
@@ -142,6 +145,7 @@ static const struct {
/* .wMaxPacketSize = autoconfiguration (kernel) */
},
},
+ .hs_count = cpu_to_le32(3),
.hs_descs = {
.intf = {
.bLength = sizeof descriptors.fs_descs.intf,
--
2.1.0.rc2.206.gedb03e5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] USB:gadget: Fix a warning while loading g_mass_storage
From: Yang Wei While loading g_mass_storage module, the following warning is triggered. WARNING: at drivers/usb/gadget/composite.c: usb_composite_setup_continue: Unexpected call Modules linked in: fat vfat minix nls_cp437 nls_iso8859_1 g_mass_storage [<800179cc>] (unwind_backtrace+0x0/0x104) from [<80619608>] (dump_stack+0x20/0x24) [<80619608>] (dump_stack+0x20/0x24) from [<80025100>] (warn_slowpath_common+0x64/0x74) [<80025100>] (warn_slowpath_common+0x64/0x74) from [<800251cc>] (warn_slowpath_fmt+0x40/0x48) [<800251cc>] (warn_slowpath_fmt+0x40/0x48) from [<7f047774>] (usb_composite_setup_continue+0xb4/0xbc [g_mass_storage]) [<7f047774>] (usb_composite_setup_continue+0xb4/0xbc [g_mass_storage]) from [<7f047ad4>] (handle_exception+0x358/0x3e4 [g_mass_storage]) [<7f047ad4>] (handle_exception+0x358/0x3e4 [g_mass_storage]) from [<7f048080>] (fsg_main_thread+0x520/0x157c [g_mass_storage]) [<7f048080>] (fsg_main_thread+0x520/0x157c [g_mass_storage]) from [<8004bc90>] (kthread+0x98/0x9c) [<8004bc90>] (kthread+0x98/0x9c) from [<8000faec>] (kernel_thread_exit+0x0/0x8) The root cause is that the existing code fails to take into account the possibility that common->new_fsg can change while do_set_interface() is running, so the value of common->new_fsg that gets tested after do_set_interface returns needs to be the same as the value used by do_set_interface. Signed-off-by: Yang Wei Signed-off-by: Michal Nazarewicz Acked-by: Alan Stern --- drivers/usb/gadget/function/f_mass_storage.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 811929c..cf4b202 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -2342,6 +2342,7 @@ static void handle_exception(struct fsg_common *common) struct fsg_buffhd *bh; enum fsg_state old_state; struct fsg_lun *curlun; + struct fsg_dev *new_fsg; unsigned intexception_req_tag; /* @@ -2460,8 +2461,9 @@ static void handle_exception(struct fsg_common *common) break; case FSG_STATE_CONFIG_CHANGE: - do_set_interface(common, common->new_fsg); - if (common->new_fsg) + new_fsg = common->new_fsg; + do_set_interface(common, new_fsg); + if (new_fsg) usb_composite_setup_continue(common->cdev); break; -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] usb: gadget: f_fs: add usb_functionfs_descs_head_v2 structure
The structure can be used with user space tools that use the new
functionfs description format, for example as follows:
static const struct {
struct usb_functionfs_descs_head_v2 header;
__le32 fs_count;
__le32 hs_count;
struct {
…
} fs_desc;
struct {
…
} hs_desc;
} descriptors = {
.header = {
.magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC_V2),
.length = cpu_to_le32(sizeof(descriptors)),
.flags = cpu_to_le32(FUNCTIONFS_HAS_FS_DESC |
FUNCTIONFS_HAS_HS_DESC)
},
.fs_count = cpu_to_le32(X),
.fs_desc = {
…
},
.hs_count = cpu_to_le32(Y),
.hs_desc = {
…
}
};
Signed-off-by: Michal Nazarewicz
---
include/uapi/linux/usb/functionfs.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/include/uapi/linux/usb/functionfs.h
b/include/uapi/linux/usb/functionfs.h
index 0154b28..3ca03de 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -32,6 +32,16 @@ struct usb_endpoint_descriptor_no_audio {
__u8 bInterval;
} __attribute__((packed));
+struct usb_functionfs_descs_head_v2 {
+ __le32 magic;
+ __le32 length;
+ __le32 flags;
+ /*
+* __le32 fs_count, hs_count, fs_count; must be included manually in
+* the structure taking flags into consideration.
+*/
+} __attribute__((packed));
+
/* Legacy format, deprecated as of 3.14. */
struct usb_functionfs_descs_head {
__le32 magic;
--
2.1.0.rc2.206.gedb03e5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] Small USB fixes/improvements
Apparently I've slept over 3.17 merge window, so I guess this is for 3.18. Rebased on top of balbi/usb.git next. Michal Nazarewicz (4): usb: gadget: f_fs: add usb_functionfs_descs_head_v2 structure tools: ffs-test: convert to new descriptor format tools: ffs-test: add compatibility code for old kernels usb: gadget: f_mass_storage: simplify start_transfer slightly Yang Wei (1): USB:gadget: Fix a warning while loading g_mass_storage drivers/usb/gadget/function/f_mass_storage.c | 33 +++ include/uapi/linux/usb/functionfs.h | 12 ++- tools/usb/ffs-test.c | 126 --- 3 files changed, 144 insertions(+), 27 deletions(-) -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] tools: ffs-test: add compatibility code for old kernels
If ffs-test is used with a kernel prior to 3.14, which do not
support the new descriptors format, it will fail when trying to
write the descriptors. Add a function that converts the new
descriptors to the legacy ones and use it to retry writing the
descriptors using the legacy format.
Also add “-l” flag to ffs-test which will cause the tool to
never try the new format and instead immediatelly try the
legacy one. This should be useful to test whether parsing
of the old format still works on given 3.14+ kernel.
Signed-off-by: Michal Nazarewicz
---
tools/usb/ffs-test.c | 112 ---
1 file changed, 107 insertions(+), 5 deletions(-)
diff --git a/tools/usb/ffs-test.c b/tools/usb/ffs-test.c
index 708d317..88d5e71 100644
--- a/tools/usb/ffs-test.c
+++ b/tools/usb/ffs-test.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -172,6 +173,89 @@ static const struct {
},
};
+static size_t descs_to_legacy(void **legacy, const void *descriptors_v2)
+{
+ const unsigned char *descs_end, *descs_start;
+ __u32 length, fs_count = 0, hs_count = 0, count;
+
+ /* Read v2 header */
+ {
+ const struct {
+ const struct usb_functionfs_descs_head_v2 header;
+ const __le32 counts[];
+ } __attribute__((packed)) *const in = descriptors_v2;
+ const __le32 *counts = in->counts;
+ __u32 flags;
+
+ if (le32_to_cpu(in->header.magic) !=
+ FUNCTIONFS_DESCRIPTORS_MAGIC_V2)
+ return 0;
+ length = le32_to_cpu(in->header.length);
+ if (length <= sizeof in->header)
+ return 0;
+ length -= sizeof in->header;
+ flags = le32_to_cpu(in->header.flags);
+ if (flags & ~(FUNCTIONFS_HAS_FS_DESC | FUNCTIONFS_HAS_HS_DESC |
+ FUNCTIONFS_HAS_SS_DESC))
+ return 0;
+
+#define GET_NEXT_COUNT_IF_FLAG(ret, flg) do { \
+ if (!(flags & (flg))) \
+ break; \
+ if (length < 4) \
+ return 0; \
+ ret = le32_to_cpu(*counts); \
+ length -= 4;\
+ ++counts; \
+ } while (0)
+
+ GET_NEXT_COUNT_IF_FLAG(fs_count, FUNCTIONFS_HAS_FS_DESC);
+ GET_NEXT_COUNT_IF_FLAG(hs_count, FUNCTIONFS_HAS_HS_DESC);
+ GET_NEXT_COUNT_IF_FLAG(count, FUNCTIONFS_HAS_SS_DESC);
+
+ count = fs_count + hs_count;
+ if (!count)
+ return 0;
+ descs_start = (const void *)counts;
+
+#undef GET_NEXT_COUNT_IF_FLAG
+ }
+
+ /*
+* Find the end of FS and HS USB descriptors. SS descriptors
+* are ignored since legacy format does not support them.
+*/
+ descs_end = descs_start;
+ do {
+ if (length < *descs_end)
+ return 0;
+ length -= *descs_end;
+ descs_end += *descs_end;
+ } while (--count);
+
+ /* Allocate legacy descriptors and copy the data. */
+ {
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+ struct {
+ struct usb_functionfs_descs_head header;
+ __u8 descriptors[];
+ } __attribute__((packed)) *out;
+#pragma GCC diagnostic pop
+
+ length = sizeof out->header + (descs_end - descs_start);
+ out = malloc(length);
+ out->header.magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC);
+ out->header.length = cpu_to_le32(length);
+ out->header.fs_count = cpu_to_le32(fs_count);
+ out->header.hs_count = cpu_to_le32(hs_count);
+ memcpy(out->descriptors, descs_start, descs_end - descs_start);
+ *legacy = out;
+ }
+
+ return length;
+}
+
#define STR_INTERFACE_ "Source/Sink"
@@ -491,12 +575,29 @@ ep0_consume(struct thread *ignore, const void *buf,
size_t nbytes)
return nbytes;
}
-static void ep0_init(struct thread *t)
+static void ep0_init(struct thread *t, bool legacy_descriptors)
{
+ void *legacy;
ssize_t ret;
+ size_t len;
+
+ if (legacy_descriptors) {
+ info("%s: writing descriptors\n", t->filename);
+ goto legacy;
+ }
- info("%s: writing descriptors\n", t->filename);
+ info("%s: writing descriptors (in v2 format)\n", t->filename);
ret = write(t->fd, &descriptors, sizeof descriptors);
+
+ if (ret < 0 && errno == EINVAL) {
+ warn("%
[PATCH 4/5] usb: gadget: f_mass_storage: simplify start_transfer slightly
Flatten the start_transfer function by reversing the if condition and
returning early out of the function if everything went fine. It makes
the function look less complicated, at least to me, and easier to
understand.
Signed-off-by: Michal Nazarewicz
---
drivers/usb/gadget/function/f_mass_storage.c | 27 +--
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index b963939..811929c 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -566,22 +566,22 @@ static void start_transfer(struct fsg_dev *fsg, struct
usb_ep *ep,
*pbusy = 1;
*state = BUF_STATE_BUSY;
spin_unlock_irq(&fsg->common->lock);
+
rc = usb_ep_queue(ep, req, GFP_KERNEL);
- if (rc != 0) {
- *pbusy = 0;
- *state = BUF_STATE_EMPTY;
+ if (rc == 0)
+ return; /* All good, we're done */
- /* We can't do much more than wait for a reset */
+ *pbusy = 0;
+ *state = BUF_STATE_EMPTY;
- /*
-* Note: currently the net2280 driver fails zero-length
-* submissions if DMA is enabled.
-*/
- if (rc != -ESHUTDOWN &&
- !(rc == -EOPNOTSUPP && req->length == 0))
- WARNING(fsg, "error in submission: %s --> %d\n",
- ep->name, rc);
- }
+ /* We can't do much more than wait for a reset */
+
+ /*
+* Note: currently the net2280 driver fails zero-length
+* submissions if DMA is enabled.
+*/
+ if (rc != -ESHUTDOWN && !(rc == -EOPNOTSUPP && req->length == 0))
+ WARNING(fsg, "error in submission: %s --> %d\n", ep->name, rc);
}
static bool start_in_transfer(struct fsg_common *common, struct fsg_buffhd *bh)
@@ -3665,4 +3665,3 @@ void fsg_config_from_params(struct fsg_config *cfg,
cfg->fsg_num_buffers = fsg_num_buffers;
}
EXPORT_SYMBOL_GPL(fsg_config_from_params);
-
--
2.1.0.rc2.206.gedb03e5
--
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
