[PATCH] USB: gadget: s3c-hsotg: add isochronous transfers support
This patch adds isochronous transfer support. It adds few modifications:
- Modify s3c_hsotg_write_fifo() function. It actually calculates transfer
size, taking into account Multi Count value, which indicates number of
transactions per microframe.
- Fix s3c_hsotg_start_req() function by setting number of packets to Multi
Count field in DIEPTSIZ register for isochronous endpoints.
- Fix s3c_hsotg_set_ep_maxpacket() function. Field wMaxPacketSize of endpoint
descriptor is now splitted into maximum packet size value and number of
additional transaction per microframe.
- Modify s3c_hsotg_epint() function. Some interrupts are ignored for
isochronous endpoints, (e.g. INTknTXFEmpMsk) becouse isochronous request is
always transfered in single transaction, which ends with XferCompl interrupt.
Add Odd/Even microframe toggle to allow data transfering in each microframe.
- Fix s3c_hsotg_ep_enable() function by supporting isochronous endpoint type.
Signed-off-by: Robert Baldyga
Signed-off-by: Kyungmin Park
---
drivers/usb/gadget/s3c-hsotg.c | 74 +++-
1 file changed, 57 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index d5d951d..d737452 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -82,9 +82,12 @@ struct s3c_hsotg_req;
* @dir_in: Set to true if this endpoint is of the IN direction, which
* means that it is sending data to the Host.
* @index: The index for the endpoint registers.
+ * @mc: Multi Count - number of transactions per microframe
+ * @interval - Interval for periodic endpoints
* @name: The name array passed to the USB core.
* @halted: Set if the endpoint has been halted.
* @periodic: Set if this is a periodic ep, such as Interrupt
+ * @insochronous: Set if this is a isochronous ep
* @sent_zlp: Set if we've sent a zero-length packet.
* @total_data: The total number of data bytes done.
* @fifo_size: The size of the FIFO (for periodic IN endpoints)
@@ -120,9 +123,12 @@ struct s3c_hsotg_ep {
unsigned char dir_in;
unsigned char index;
+ unsigned char mc;
+ unsigned char interval;
unsigned inthalted:1;
unsigned intperiodic:1;
+ unsigned intisochronous:1;
unsigned intsent_zlp:1;
charname[10];
@@ -467,6 +473,7 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
void *data;
int can_write;
int pkt_round;
+ int max_transfer;
to_write -= (buf_pos - hs_ep->last_load);
@@ -534,15 +541,17 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
can_write *= 4; /* fifo size is in 32bit quantities. */
}
- dev_dbg(hsotg->dev, "%s: GNPTXSTS=%08x, can=%d, to=%d, mps %d\n",
-__func__, gnptxsts, can_write, to_write, hs_ep->ep.maxpacket);
+ max_transfer = hs_ep->ep.maxpacket * hs_ep->mc;
+
+ dev_dbg(hsotg->dev, "%s: GNPTXSTS=%08x, can=%d, to=%d, max_transfer
%d\n",
+__func__, gnptxsts, can_write, to_write, max_transfer);
/*
* limit to 512 bytes of data, it seems at least on the non-periodic
* FIFO, requests of >512 cause the endpoint to get stuck with a
* fragment of the end of the transfer in it.
*/
- if (can_write > 512)
+ if (can_write > 512 && !periodic)
can_write = 512;
/*
@@ -550,8 +559,8 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
* the transfer to return that it did not run out of fifo space
* doing it.
*/
- if (to_write > hs_ep->ep.maxpacket) {
- to_write = hs_ep->ep.maxpacket;
+ if (to_write > max_transfer) {
+ to_write = max_transfer;
/* it's needed only when we do not use dedicated fifos */
if (!hsotg->dedicated_fifos)
@@ -564,7 +573,7 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
if (to_write > can_write) {
to_write = can_write;
- pkt_round = to_write % hs_ep->ep.maxpacket;
+ pkt_round = to_write % max_transfer;
/*
* Round the write down to an
@@ -730,8 +739,16 @@ static void s3c_hsotg_start_req(struct s3c_hsotg *hsotg,
else
packets = 1;/* send one packet if length is zero. */
+ if (length > (hs_ep->mc * hs_ep->ep.maxpacket) && hs_ep->isochronous) {
+ dev_err(hsotg->dev, "req length > maxpacket*mc\n");
+ return;
+ }
+
if (dir_in && index != 0)
- epsize = DxEPTSIZ_MC(1);
+ if (hs_ep->isochronous)
+ epsize = DxEPTSIZ_MC(packets);
+ else
+ epsize = DxEPTSIZ_MC(1);
else
[PATCH] usb: s3c-hsotg: add isochronous transfers support
Hello, There is my initial proposal for isochronous transfers support in s3c-hsotg driver. This patch does few modifications: - Fix few functions to make them usable in isochronous transfers handling. - Add few fields to s3c_hsotg_ep structure, used for isochronous ep handling. - Add isochronous specific endpoint descriptor fields handling. - Add high-speed high-bandwidth trensfers support by correct Multi Count handling and Odd/Even frame toggle for interval=1. - Improve endpoint interrupt handling by ignoring unneded interrupts for isochronous endpoints. Best regards Robert Baldyga Samsung R&D Institute Poland Robert Baldyga (1): USB: gadget: s3c-hsotg: add isochronous transfers support drivers/usb/gadget/s3c-hsotg.c | 74 +++- 1 file changed, 57 insertions(+), 17 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Cannot shutdown power use from built in webcam in thinkpad T530 questions]
Am 22.09.2013 22:36, schrieb Marc MERLIN: > On Sun, Sep 22, 2013 at 04:32:08PM -0400, Alan Stern wrote: >>> I'm somehow thinking there is a driver or hardware problem when the device >>> does get stuck in a mode where it won't sleep properly again until the next >>> reboot (just unloading/loading the driver does not fix this). >> >> That's quite possible. But if it is a driver problem, wouldn't >> unloading and reloading the driver fix it? > > You'd think that but it hasn't so far with this one device. > You might get more information from a kernel with CONFIG_USB_DEBUG enabled. Especially if you add #define VERBOSE_DEBUG to drivers/usb/core/driver.c before the first #include line. >>> >>> Do you think thaty would help debug the problem above, or not really? I'm >> >> There's no way to know in advance. >> >>> starting to think that the USB layer is not at fault, although I could be >>> wrong I suppose. >> >> You asked for advice on things to try, and I suggested something. >> That's the best I can do. > > Understood, just making sure this was still potentially useful considering > what I found out since my last message. Which version of powertop are you actually using? None of current versions would show you Watt usage for devices. you can use "watch grep . *" instead and check fallowing fields: runtime_suspended_time - suspend time. If it is growing device is suspended runtime_active_kids - if not zero, some program use it control - if "on" then autosuspend is disabled. -- Regards, Oleksij -- To unsubscribe from this list: send the line "unsubscribe 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: No keyboard and mouse usb detected at startup.
Hi, On 09/23/2013 07:26 AM, Kumar Gaurav wrote: > Hi Alan, > I have one question just for knowledge. I was looking for the reason of this > bug but wasn't even able to identify which driver (ehci,xhci etc) these > devices are using. I tried inspecting with my USB mouse and found out it was > ehci (i believe it depends on devices). >>> My configuration files are almost similar between 3.10 and 3.11... >>> Attached 2 "lsusb -v" files, 1 that works, the other doesn't. >>> >>> I use debian wheezy with LUKS. >>> >>> Thank you, best regards >>> >>> Note: Same problem with the kernel 3.12rc1. >> It looks like your system did not load the ohci-pci driver. Is >> CONFIG_USB_OHCI_HCD_PCI enabled? > how did you identified it was using ohci? > Please reply. Because almost all keyboards/mice are full speed devices and will need the OHCI controller to handle them, unless they are connected via an external high speed hub that does Transaction Translation (TT). 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 5/7] staging: usbip: Add encryption support to kernel
On Thu, Sep 19, 2013 at 04:11:57PM +0200, Dominik Paulus wrote:
> +int usbip_init_crypto(struct usbip_device *ud, unsigned char *sendkey,
> unsigned
> + char *recvkey)
> +{
> + int ret;
> +
> + ud->use_crypto = 1;
> +
> + ud->tfm_recv = crypto_alloc_aead("gcm(aes)", 0, 0);
> + if (IS_ERR(ud->tfm_recv))
> + return -PTR_ERR(ud->tfm_recv);
PTR_ERR() already returns a negative.
> + ud->tfm_send = crypto_alloc_aead("gcm(aes)", 0, 0);
> + if (IS_ERR(ud->tfm_send)) {
> + crypto_free_aead(ud->tfm_recv);
> + return -PTR_ERR(ud->tfm_send);
Again. All the uses of PTR_ERR() in this patch have the same problem.
> + plainbuf = kmalloc(USBIP_PACKETSIZE, GFP_KERNEL);
> + if (IS_ERR(plainbuf))
> + return -PTR_ERR(plainbuf);
kmalloc() returns NULL on error and not an ERR_PTR. All the calls to
kmalloc() have this problem.
> + cipherbuf = kmalloc(USBIP_PACKETSIZE, GFP_KERNEL);
> + if (IS_ERR(cipherbuf)) {
> + kfree(plainbuf);
> + return -PTR_ERR(cipherbuf);
> + }
> +
> + while (total < size) {
> + uint32_t packetsize;
> + struct kvec recvvec;
> +
> + /*
> + * We use a global kfifo to buffer unrequested plaintext bytes.
> + * Flush this buffer first before receiving new data.
> + */
> + if (kfifo_len(&ud->recv_queue)) {
> + size_t next = min_t(size_t, kfifo_len(&ud->recv_queue),
> + size - total);
> + /* No error checking necessary - see previous line */
> + ret = kfifo_out(&ud->recv_queue, ((char *)
> + vec[0].iov_base)+total, next);
The comment assume there is only one reader and one writer at a time,
yes? The casting is not needed:
ret = kfifo_out(&ud->recv_queue,
vec[0].iov_base + total, next);
v> +total += next;
> + continue;
> + }
> +
> + /* See usbip_sendmsg() for the format of one encrypted packet */
> +
> + /*
> + * Receive size of next crypto packet
> + */
> + recvvec.iov_base = &packetsize;
> + recvvec.iov_len = sizeof(packetsize);
> +
> + ret = kernel_recvmsg(ud->tcp_socket, msg, &recvvec, 1,
> + sizeof(packetsize), flags);
> + packetsize = be32_to_cpu(packetsize);
> + if (ret <= 0) {
I think a return of zero should mean total = -EBADMSG;. In other words
this check should be "if (ret < 0) {" and we hit the next else if.
Same below again.
> + total = ret;
> + goto err;
> + } else if (ret != sizeof(packetsize)) {
> + total = -EBADMSG;
> + goto err;
> + }
> +
> + if (packetsize > USBIP_PACKETSIZE) {
> + total = -EBADMSG;
> + goto err;
> + }
> +
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe 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 5/7] staging: usbip: Add encryption support to kernel
On Thu, Sep 19, 2013 at 04:11:57PM +0200, Dominik Paulus wrote:
> +int usbip_init_crypto(struct usbip_device *ud, unsigned char *sendkey,
> unsigned
> + char *recvkey)
> +{
> + int ret;
> +
> + ud->use_crypto = 1;
> +
> + ud->tfm_recv = crypto_alloc_aead("gcm(aes)", 0, 0);
> + if (IS_ERR(ud->tfm_recv))
> + return -PTR_ERR(ud->tfm_recv);
> + ud->tfm_send = crypto_alloc_aead("gcm(aes)", 0, 0);
> + if (IS_ERR(ud->tfm_send)) {
> + crypto_free_aead(ud->tfm_recv);
> + return -PTR_ERR(ud->tfm_send);
> + }
> + ret = kfifo_alloc(&ud->recv_queue, RECVQ_SIZE, GFP_KERNEL);
> + if (ret) {
> + crypto_free_aead(ud->tfm_recv);
> + crypto_free_aead(ud->tfm_send);
> + return ret;
> + }
> +
> + if (crypto_aead_setkey(ud->tfm_send, sendkey, USBIP_KEYSIZE) != 0 ||
> + crypto_aead_setkey(ud->tfm_recv, recvkey,
> + USBIP_KEYSIZE) != 0 ||
> + crypto_aead_setauthsize(ud->tfm_send,
> + USBIP_AUTHSIZE) != 0 ||
> + crypto_aead_setauthsize(ud->tfm_recv,
> + USBIP_AUTHSIZE)) {
> + crypto_free_aead(ud->tfm_recv);
> + crypto_free_aead(ud->tfm_send);
> + kfifo_free(&ud->recv_queue);
> + }
This returns success on error instead of failure.
The indenting is messed up. There are three places which check " != 0"
and doesn't. Please leave off the "!= 0" throughout the whole patch.
It should look like:
if (crypto_aead_setkey(ud->tfm_send, sendkey, USBIP_KEYSIZE) ||
crypto_aead_setkey(ud->tfm_recv, recvkey, USBIP_KEYSIZE) ||
crypto_aead_setauthsize(ud->tfm_send, USBIP_AUTHSIZE) ||
crypto_aead_setauthsize(ud->tfm_recv, USBIP_AUTHSIZE)) {
ret = -EINVAL;
goto err_free_fifo;
}
Notice how the label name is chosen based on the label location and not
the goto location.
The end of the function should look like:
return 0;
err_free_fifo:
kfifo_free(&ud->recv_queue);
err_free_send:
crypto_free_aead(ud->tfm_send);
err_free_recv:
crypto_free_aead(ud->tfm_recv);
return ret;
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 5/5] dma: cppi41: add support for suspend and resume
On Mon, Sep 23, 2013 at 07:53:11AM +0200, Daniel Mack wrote:
> On 23.09.2013 06:09, Vinod Koul wrote:
> > On Sun, Sep 22, 2013 at 04:50:04PM +0200, Daniel Mack wrote:
>
> >> +#ifdef CONFIG_PM_SLEEP
> > a
> >
> >> +static int cppi41_suspend(struct device *dev)
> >> +{
> >> + struct cppi41_dd *cdd = dev_get_drvdata(dev);
> >> +
> >> + cppi_writel(0, cdd->usbss_mem + USBSS_IRQ_CLEARR);
> >> + disable_sched(cdd);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int cppi41_resume(struct device *dev)
> >> +{
> >> + struct cppi41_dd *cdd = dev_get_drvdata(dev);
> >> + int i;
> >> +
> >> + for (i = 0; i < DESCS_AREAS; i++)
> >> + cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
> >> +
> >> + init_sched(cdd);
> >> + cppi_writel(USBSS_IRQ_PD_COMP, cdd->usbss_mem + USBSS_IRQ_ENABLER);
> >> +
> >> + return 0;
> >> +}
> >> +#endif
> >> +
> >> +static SIMPLE_DEV_PM_OPS(cppi41_pm_ops, cppi41_suspend, cppi41_resume);
> > Here is the macro in pm.h
>
> [...]
>
> > Now since you are using the macro there should be no need to wrap ifdef
> > around
> > your code, the macro will take care of it.
>
> Well yes, which is why I put the macro itself *outside* of the #ifdef
> block. Without that #ifdef, however, and with CONFIG_PM_SLEEP unset, I get:
>
> drivers/dma/cppi41.c:1043:12: warning: ‘cppi41_suspend’ defined but not
> used [-Wunused-function]
> static int cppi41_suspend(struct device *dev)
> ^
> drivers/dma/cppi41.c:1053:12: warning: ‘cppi41_resume’ defined but not
> used [-Wunused-function]
> static int cppi41_resume(struct device *dev)
> ^
>
> ... which doesn't surprise me much. Or do I still not get your point?
And this is what i had expected... I was thinking we should ignore this, but
this is better too, so I will try to apply this now
~Vinod
--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 5/5] dma: cppi41: add support for suspend and resume
On Sun, Sep 22, 2013 at 04:50:04PM +0200, Daniel Mack wrote:
> This patch adds support for suspend/resume functionality to the cppi41
> DMA driver. The steps necessary to make the system resume properly were
> figured out by trial-and-error. The code as it stands now is the
> minimum that has to be done to put the musb host system on an AM33xx
> system into an operable state after resume.
Applied, thanks
~Vinod
>
> Signed-off-by: Daniel Mack
> ---
> drivers/dma/cppi41.c | 29 +
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
> index 3347321..89decc9 100644
> --- a/drivers/dma/cppi41.c
> +++ b/drivers/dma/cppi41.c
> @@ -1040,12 +1040,41 @@ static int cppi41_dma_remove(struct platform_device
> *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_PM_SLEEP
> +static int cppi41_suspend(struct device *dev)
> +{
> + struct cppi41_dd *cdd = dev_get_drvdata(dev);
> +
> + cppi_writel(0, cdd->usbss_mem + USBSS_IRQ_CLEARR);
> + disable_sched(cdd);
> +
> + return 0;
> +}
> +
> +static int cppi41_resume(struct device *dev)
> +{
> + struct cppi41_dd *cdd = dev_get_drvdata(dev);
> + int i;
> +
> + for (i = 0; i < DESCS_AREAS; i++)
> + cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
> +
> + init_sched(cdd);
> + cppi_writel(USBSS_IRQ_PD_COMP, cdd->usbss_mem + USBSS_IRQ_ENABLER);
> +
> + return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(cppi41_pm_ops, cppi41_suspend, cppi41_resume);
> +
> static struct platform_driver cpp41_dma_driver = {
> .probe = cppi41_dma_probe,
> .remove = cppi41_dma_remove,
> .driver = {
> .name = "cppi41-dma-engine",
> .owner = THIS_MODULE,
> + .pm = &cppi41_pm_ops,
> .of_match_table = of_match_ptr(cppi41_dma_ids),
> },
> };
> --
> 1.8.3.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 5/7] staging: usbip: Add encryption support to kernel
On Thu, Sep 19, 2013 at 04:11:57PM +0200, Dominik Paulus wrote:
> +/*
> + * Perform encryption/decryption on one chunk of data.
> + * Uses global crypto state stored in usbip_device.
> + * Parameters:
> + * encrypt: 1 to perform encryption, 0 to perform decryption operation
Make this a define:
#define USBIP_ENCRYPT 1
#define USBIP_ENCRYPT 0
> + * packetsize: Size of the encrypted packet, including the authentication
> tag,
> + * not including the associated data (length field).
> + * plaintext and ciphertext have to be appropiately managed by the caller
> + * (i.e. they must be at least packetsize bytes long).
> + * Returns: 0 on success
> + */
> +static int usbip_crypt(struct usbip_device *ud, int encrypt, uint32_t
> + packetsize, unsigned char *plaintext, unsigned char
> + *ciphertext)
Don't break put line breaks between the type and the variable name. It
should be:
static int usbip_crypt(struct usbip_device *ud, int encrypt,
uint32_t packetsize, unsigned char *plaintext,
unsigned char *ciphertext)
This applies to earlier patches in this series as well.
> +{
> + struct crypto_aead *tfm;
> + struct aead_request *req;
> + struct tcrypt_result result;
> + struct scatterlist plain, cipher, assoc;
> + char iv[16];
> + u64 *iv_num;
> + u64 iv_net;
> + const int plainsize = packetsize - USBIP_AUTHSIZE;
Is it possible that packetsize is less than USBIP_AUTHSIZE?
> + int ret;
> +
> + memset(iv, 0, sizeof(iv));
> + if (encrypt) {
> + tfm = ud->tfm_send;
> + iv_num = &ud->ctr_send;
> + } else {
> + tfm = ud->tfm_recv;
> + iv_num = &ud->ctr_recv;
> + }
> + iv_net = cpu_to_be64(*iv_num);
> + memcpy(iv, &iv_net, sizeof(iv_net));
> +
> + req = aead_request_alloc(tfm, GFP_KERNEL);
> + if (IS_ERR(req))
> + return -PTR_ERR(req);
> +
> + init_completion(&result.completion);
> + aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> + tcrypt_complete, &result);
Align this up like:
aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
tcrypt_complete, &result);
> +
> + sg_init_one(&cipher, ciphertext, packetsize);
> + sg_init_one(&plain, plaintext, plainsize);
> + crypto_aead_clear_flags(tfm, ~0);
> +
> + if (encrypt)
> + aead_request_set_crypt(req, &plain, &cipher, plainsize, iv);
> + else
> + aead_request_set_crypt(req, &cipher, &plain, packetsize, iv);
> + packetsize = cpu_to_be32(packetsize);
> + sg_init_one(&assoc, &packetsize, sizeof(packetsize));
> + /* Associated data: Unencrypted length tag */
> + aead_request_set_assoc(req, &assoc, sizeof(packetsize));
> +
> + if (encrypt)
> + ret = crypto_aead_encrypt(req);
> + else
> + ret = crypto_aead_decrypt(req);
> +
Good on you for figuring out what crypto_aead_en/decrypt() returns.
Where are these functions documented?
> + switch (ret) {
> + case 0: /* Success */
> + break;
> + case -EINPROGRESS:
> + case -EBUSY:
> + wait_for_completion(&result.completion);
> + break;
> + default:
> + aead_request_free(req);
> + return ret;
> + }
> +
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe 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: [alsa-devel] [PATCH 23/51] DMA-API: dma: pl08x: add dma_set_mask_and_coherent() call
On Thu, Sep 19, 2013 at 10:48:01PM +0100, Russell King wrote:
> The DMA API requires drivers to call the appropriate dma_set_mask()
> functions before doing any DMA mapping. Add this required call to
> the AMBA PL08x driver.
>
> Signed-off-by: Russell King
Acked-by: Vinod Koul
~Vinod
> ---
> drivers/dma/amba-pl08x.c |5 +
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
> index fce46c5..e51a983 100644
> --- a/drivers/dma/amba-pl08x.c
> +++ b/drivers/dma/amba-pl08x.c
> @@ -2055,6 +2055,11 @@ static int pl08x_probe(struct amba_device *adev, const
> struct amba_id *id)
> if (ret)
> return ret;
>
> + /* Ensure that we can do DMA */
> + ret = dma_set_mask_and_coherent(&adev->dev, DMA_BIT_MASK(32));
> + if (ret)
> + goto out_no_pl08x;
> +
> /* Create the driver state holder */
> pl08x = kzalloc(sizeof(*pl08x), GFP_KERNEL);
> if (!pl08x) {
> --
> 1.7.4.4
>
> ___
> Alsa-devel mailing list
> alsa-de...@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
--
--
To unsubscribe from this list: send the line "unsubscribe 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: [alsa-devel] [PATCH 43/51] DMA-API: dma: edma.c: no need to explicitly initialize DMA masks
On Fri, Sep 20, 2013 at 12:15:39AM +0100, Russell King wrote:
> register_platform_device_full() can setup the DMA mask provided the
> appropriate member is set in struct platform_device_info. So lets
> make that be the case. This avoids a direct reference to the DMA
> masks by this driver.
>
> Signed-off-by: Russell King
Acked-by: Vinod Koul
This also brings me question that should we force the driver to use the
dma_set_mask_and_coherent() API or they have below flexiblity too?
~Vinod
> ---
> drivers/dma/edma.c |6 ++
> 1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index ff50ff4..7f9fe30 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -702,11 +702,13 @@ static struct platform_device *pdev0, *pdev1;
> static const struct platform_device_info edma_dev_info0 = {
> .name = "edma-dma-engine",
> .id = 0,
> + .dma_mask = DMA_BIT_MASK(32),
> };
>
> static const struct platform_device_info edma_dev_info1 = {
> .name = "edma-dma-engine",
> .id = 1,
> + .dma_mask = DMA_BIT_MASK(32),
> };
>
> static int edma_init(void)
> @@ -720,8 +722,6 @@ static int edma_init(void)
> ret = PTR_ERR(pdev0);
> goto out;
> }
> - pdev0->dev.dma_mask = &pdev0->dev.coherent_dma_mask;
> - pdev0->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> }
>
> if (EDMA_CTLRS == 2) {
> @@ -731,8 +731,6 @@ static int edma_init(void)
> platform_device_unregister(pdev0);
> ret = PTR_ERR(pdev1);
> }
> - pdev1->dev.dma_mask = &pdev1->dev.coherent_dma_mask;
> - pdev1->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> }
>
> out:
> --
> 1.7.4.4
>
> ___
> Alsa-devel mailing list
> alsa-de...@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
--
--
To unsubscribe from this list: send the line "unsubscribe 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: [alsa-devel] [PATCH 24/51] DMA-API: dma: pl330: add dma_set_mask_and_coherent() call
On Sat, Sep 21, 2013 at 09:00:00PM +0100, Russell King - ARM Linux wrote: > On Fri, Sep 20, 2013 at 07:26:27PM +0200, Heiko Stübner wrote: > > Am Donnerstag, 19. September 2013, 23:49:01 schrieb Russell King: > > > The DMA API requires drivers to call the appropriate dma_set_mask() > > > functions before doing any DMA mapping. Add this required call to > > > the AMBA PL08x driver. > > ^--- copy and paste error - should of course be PL330 > > Fixed, thanks. with fixed changelog... Acked-by: Vinod Koul ~Vinod -- -- To unsubscribe from this list: send the line "unsubscribe 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: [alsa-devel] [PATCH 43/51] DMA-API: dma: edma.c: no need to explicitly initialize DMA masks
On Mon, Sep 23, 2013 at 03:55:33PM +0530, Vinod Koul wrote:
> On Fri, Sep 20, 2013 at 12:15:39AM +0100, Russell King wrote:
> > register_platform_device_full() can setup the DMA mask provided the
> > appropriate member is set in struct platform_device_info. So lets
> > make that be the case. This avoids a direct reference to the DMA
> > masks by this driver.
> >
> > Signed-off-by: Russell King
> Acked-by: Vinod Koul
>
> This also brings me question that should we force the driver to use the
> dma_set_mask_and_coherent() API or they have below flexiblity too?
There's two issues here:
1. dma_set_mask_and_coherent() will only work if dev->dma_mask points at
some storage for the mask. This needs to have .dma_mask in the
platform_device_info initialised.
2. Yes, this driver should also be calling the appropriate DMA mask setting
functions in addition to having the mask initialized at device creation
time.
Here's a replacement patch, though maybe it would be better to roll all
the additions of dma_set_mask_and_coherent() in drivers/dma into one
patch? In other words, combine the addition of this with these two
patches:
dma: pl330: add dma_set_mask_and_coherent() call
dma: pl08x: add dma_set_mask_and_coherent() call
8<=
From: Russell King
Subject: [PATCH] DMA-API: dma: edma.c: no need to explicitly initialize DMA
masks
register_platform_device_full() can setup the DMA mask provided the
appropriate member is set in struct platform_device_info. So lets
make that be the case. This avoids a direct reference to the DMA
masks by this driver.
While here, add the dma_set_mask_and_coherent() call which the DMA API
requires DMA-using drivers to call.
Signed-off-by: Russell King
---
drivers/dma/edma.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index ff50ff4..fd5e48c 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -631,6 +631,10 @@ static int edma_probe(struct platform_device *pdev)
struct edma_cc *ecc;
int ret;
+ ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+ if (ret)
+ return ret;
+
ecc = devm_kzalloc(&pdev->dev, sizeof(*ecc), GFP_KERNEL);
if (!ecc) {
dev_err(&pdev->dev, "Can't allocate controller\n");
@@ -702,11 +706,13 @@ static struct platform_device *pdev0, *pdev1;
static const struct platform_device_info edma_dev_info0 = {
.name = "edma-dma-engine",
.id = 0,
+ .dma_mask = DMA_BIT_MASK(32),
};
static const struct platform_device_info edma_dev_info1 = {
.name = "edma-dma-engine",
.id = 1,
+ .dma_mask = DMA_BIT_MASK(32),
};
static int edma_init(void)
@@ -720,8 +726,6 @@ static int edma_init(void)
ret = PTR_ERR(pdev0);
goto out;
}
- pdev0->dev.dma_mask = &pdev0->dev.coherent_dma_mask;
- pdev0->dev.coherent_dma_mask = DMA_BIT_MASK(32);
}
if (EDMA_CTLRS == 2) {
@@ -731,8 +735,6 @@ static int edma_init(void)
platform_device_unregister(pdev0);
ret = PTR_ERR(pdev1);
}
- pdev1->dev.dma_mask = &pdev1->dev.coherent_dma_mask;
- pdev1->dev.coherent_dma_mask = DMA_BIT_MASK(32);
}
out:
--
1.7.4.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5 5/9] USB: OHCI: make ohci-at91 a separate driver
On 21/09/2013 13:08, Manjunath Goudar :
Separate the TI OHCI Atmel host controller driver from ohci-hcd
Why "TI" here? ?
host code so that it can be built as a separate driver module.
This work is part of enabling multi-platform kernels on ARM.
Signed-off-by: Manjunath Goudar
Signed-off-by: Deepak Saxena
Acked-by: Alan Stern
Otherwise, seems correct.
Acked-by: Nicolas Ferre
Cc: Arnd Bergmann
Cc: Greg KH
Cc: linux-usb@vger.kernel.org
V3->V4:
- Removed extra space after "tristate".
- Removed extra space between function name and '(' characters.
- MODULE_ALIAS line moved to last statement of ohci-at91 file.
V2->V3:
-The ohci_restart() function is not required in current scenario,
only discarding connection state of integrated transceivers is sufficient,
for this directly handling ohci->hc_control.
V1->V2:
-Set non-standard fields in ohci_at91_hc_driver manually, rather than
relying on an expanded struct ohci_driver_overrides.
-Save orig_ohci_hub_control and orig_ohci_hub_status_data rather than
relying on ohci_hub_control and hub_status_data being exported.
-ohci_setup() has been removed because it is called in .reset member
of the ohci_hc_driver structure.
---
drivers/usb/host/Kconfig |8 +++
drivers/usb/host/Makefile|1 +
drivers/usb/host/ohci-at91.c | 156 +++---
drivers/usb/host/ohci-hcd.c | 18 -
4 files changed, 79 insertions(+), 104 deletions(-)
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 00d22f5..6900b72 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -382,6 +382,14 @@ config USB_OHCI_HCD_SPEAR
Enables support for the on-chip OHCI controller on
ST SPEAr chips.
+config USB_OHCI_HCD_AT91
+tristate "Support for Atmel on-chip OHCI USB controller"
+depends on USB_OHCI_HCD && ARCH_AT91
+default y
+---help---
+ Enables support for the on-chip OHCI controller on
+ Atmel chips.
+
config USB_OHCI_HCD_OMAP3
tristate "OHCI support for OMAP3 and later chips"
depends on (ARCH_OMAP3 || ARCH_OMAP4)
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 9efcb7c..0d7a81a 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_USB_OHCI_EXYNOS) += ohci-exynos.o
obj-$(CONFIG_USB_OHCI_HCD_OMAP1) += ohci-omap.o
obj-$(CONFIG_USB_OHCI_HCD_OMAP3) += ohci-omap3.o
obj-$(CONFIG_USB_OHCI_HCD_SPEAR) += ohci-spear.o
+obj-$(CONFIG_USB_OHCI_HCD_AT91)+= ohci-at91.o
obj-$(CONFIG_USB_UHCI_HCD)+= uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD)+= fhci.o
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index caa3764..476b5a5 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -13,19 +13,24 @@
*/
#include
-#include
+#include
#include
#include
+#include
#include
+#include
+#include
+#include
+#include
+#include
#include
#include
#include
-#ifndef CONFIG_ARCH_AT91
-#error "CONFIG_ARCH_AT91 must be defined."
-#endif
+
+#include "ohci.h"
#define valid_port(index) ((index) >= 0 && (index) < AT91_MAX_USBH_PORTS)
#define at91_for_each_port(index) \
@@ -33,7 +38,17 @@
/* interface, function and usb clocks; sometimes also an AHB clock */
static struct clk *iclk, *fclk, *uclk, *hclk;
+/* interface and function clocks; sometimes also an AHB clock */
+
+#define DRIVER_DESC "OHCI Atmel driver"
+
+static const char hcd_name[] = "ohci-atmel";
+
+static struct hc_driver __read_mostly ohci_at91_hc_driver;
static int clocked;
+static int (*orig_ohci_hub_control)(struct usb_hcd *hcd, u16 typeReq,
+ u16 wValue, u16 wIndex, char *buf, u16 wLength);
+static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
extern int usb_disabled(void);
@@ -117,6 +132,8 @@ static void usb_hcd_at91_remove (struct usb_hcd *, struct
platform_device *);
static int usb_hcd_at91_probe(const struct hc_driver *driver,
struct platform_device *pdev)
{
+ struct at91_usbh_data *board;
+ struct ohci_hcd *ohci;
int retval;
struct usb_hcd *hcd = NULL;
@@ -177,8 +194,10 @@ static int usb_hcd_at91_probe(const struct hc_driver
*driver,
}
}
+ board = hcd->self.controller->platform_data;
+ ohci = hcd_to_ohci(hcd);
+ ohci->num_ports = board->ports;
at91_start_hc(pdev);
- ohci_hcd_init(hcd_to_ohci(hcd));
retval = usb_add_hcd(hcd, pdev->resource[1].start, IRQF_SHARED);
if (retval == 0)
@@ -238,36 +257,6 @@ static void usb_hcd_at91_remove(struct usb_hcd *hcd,
}
/*-*/
-
-static int
-ohci_at91_reset (struct usb_hcd *hcd)
-{
- struct at91_usbh_data *board = dev_get_platdata(hcd->self.contro
Re: [PATCH 36/51] DMA-API: usb: use dma_set_coherent_mask()
On 20/09/2013 00:01, Russell King :
The correct way for a driver to specify the coherent DMA mask is
not to directly access the field in the struct device, but to use
dma_set_coherent_mask(). Only arch and bus code should access this
member directly.
Convert all direct write accesses to using the correct API.
Signed-off-by: Russell King
---
drivers/usb/chipidea/ci_hdrc_imx.c |5 +++--
drivers/usb/dwc3/dwc3-exynos.c |5 +++--
drivers/usb/gadget/lpc32xx_udc.c |4 +++-
drivers/usb/host/ehci-atmel.c |5 +++--
For Atmel driver:
Acked-by: Nicolas Ferre
[..]
diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
index 3b645ff..5831a88 100644
--- a/drivers/usb/host/ehci-atmel.c
+++ b/drivers/usb/host/ehci-atmel.c
@@ -92,8 +92,9 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
*/
if (!pdev->dev.dma_mask)
pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
- if (!pdev->dev.coherent_dma_mask)
- pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+ retval = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
+ if (retval)
+ goto fail_create_hcd;
hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
if (!hcd) {
[..]
Thanks,
--
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe 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 37/51] DMA-API: usb: use new dma_coerce_mask_and_coherent()
On 20/09/2013 00:02, Russell King : Signed-off-by: Russell King --- drivers/usb/chipidea/ci_hdrc_imx.c |4 +--- drivers/usb/dwc3/dwc3-exynos.c |4 +--- drivers/usb/host/ehci-atmel.c |4 +--- For Atmel driver: Acked-by: Nicolas Ferre [..] diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c index 5831a88..8e7323e 100644 --- a/drivers/usb/host/ehci-atmel.c +++ b/drivers/usb/host/ehci-atmel.c @@ -90,9 +90,7 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev) * Since shared usb code relies on it, set it here for now. * Once we have dma capability bindings this can go away. */ - if (!pdev->dev.dma_mask) - pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask; - retval = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32)); + retval = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); if (retval) goto fail_create_hcd; [..] Thanks Russell, -- Nicolas Ferre -- To unsubscribe from this list: send the line "unsubscribe 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/core/devio.c:tolerate wrong direction flag in control endpoints
Hi Alan, On 09/23/2013 04:28 AM, Alan Stern wrote: > On Sun, 22 Sep 2013, Kurt Garloff wrote: > >> Well, this seems to be a question of terminology, no? >> I saw the endpoint byte as consisting of endpoint index plus the direction >> bit. > See the entry for "Endpoint Address" in Chapter 2 (Terms and > Abbreviations) of the USB 2.0 specification. The definition says: > > The combination of an endpoint number and an endpoint direction > on a USB device. Each endpoint address supports data transfer > in one direction. OK. This definitely helps me to use the correct terminology. >> OK, you certainly know the USB specs better than I do. >> >> If the message is not according to spec because the windex byte (which >> should reference the endpoint) has the endpoint direction flag wrong, >> then the question may become whether the kernel should reject it or >> leave that to the device. >> Rejecting by the kernel has the risk that somewhat non-compliant devices >> with somewhat non-compliant (userspace) drivers are prevented from working. >> While not rejecting has the risk that overly sensitive devices might freak >> out >> on receiving a non-compliant transfer. The fact that Win does not not seem to >> filter here however might make that risk rather small. >> (Long years have taught us that companies don't test against the spec but >> this >> "OS" from Redmond.) > This is a good explanation of why the patch should be accepted. OK, I added it into the patch header. >> It seems to interpret wIndex differently indeed. Not sure whether >> that qualifies as a bug or not. Maybe it should not claim to be a >> HID device then? > Maybe not. This particular combination of bRequestType and bRequest > values (0x22, 0x09) is not defined in the HID 1.11 spec. Do you know > if it is defined somewhere else? These are custom commands, somewhat described at http://pegatech.com/_Uploads/Downloads/Documents/Protocol_Definition_Rev_1.12.pdf >> Hmmm, none of the devices I have show this. >> My impression was that the EP byte is composed of >> ep = epindex | (dir==in? 0x80: 0) >> and that index alone must be unique already. But then again, I'm in no >> way an expert in USB specs and may just have jumped to conclusions >> from the wording. > The spec is pretty clear about this. For example, it says that in > addition to endpoint 0, a device can have up to 15 input endpoints and > up to 15 output endpoints (section 5.3.1.2). > >> (And again the behavior might not be enforced by the spec, but maybe >> by Windows?) > More likely the behavior isn't enforced at all. The device may simply > be buggy. With behavior here I referred to the fact that I have not yet seen a USB device that has two endpoints with the same endpoint number (but different direction). Anyway, that's not relevant to the patch ... We don't change the value of wIndex, we just decide to let the transfer through and let the device deal with it. >> Based on the observation that uurb.endpoint = 0 (see above), I have >> changed my code in the Linux program (at [2]) to use 0x00 as wIndex >> instead of 0x81 (or formerly 0x01). It still worked. >> So it's questionable, whether wIndex should even point to an EP ... >> and the hardware might just ignore it. > It looks that way. The request claims to be class-specific, so it > would be nice to know which class document defines the request's > format. I guess none ... I just dropped the (or 00), as it's not reflected in the code ... >> Thanks for the review! I will submit a new patch. > Good. Find it here. (Let me know if it should be sent again separately for some reason.) Let me try inline insert (by c'n'p: I switched from mutt to Thunderbird recently and lack experience whether this breaks formatting or so ...) 8< From: Kurt Garloff Date: Mon, 23 Sep 2013 14:19:02 +0200 Subject: Tolerate wrong direction bit in endpoint address for control messages Trying to read data from the Pegasus Technologies NoteTaker (0e20:0101) [1] with the Windows App (EasyNote) works natively but fails when Windows is running under KVM (and the USB device handed to KVM). The reason is a USB control message usb 4-2.2: control urb: bRequestType=22 bRequest=09 wValue=0200 wIndex=0001 wLength=0008 This goes to endpoint address 0x01 (wIndex); however, endpoint number 1 is an input endpoint and thus has endpoint address 0x81. The kernel thus rejects the IO and thus we see the failure. Apparently, Linux is more strict here than Windows ... we can't change the Win app easily, so that's a problem. It seems that the Win app/driver is buggy here and the driver does not behave fully according to the USB HID class that it claims to belong to. The device seems to happily deal with that though (and seems to not really care about this value much). So the question is whether the Linux kernel should filter here. Rejecting has the risk that somewhat non-compliant userspace apps/ drivers (most likely in
[PATCH v1] USBNET: fix handling padding packet
Commit 638c5115a7949(USBNET: support DMA SG) introduces DMA SG
if the usb host controller is capable of building packet from
discontinuous buffers, but missed handling padding packet when
building DMA SG.
This patch attachs the pre-allocated padding packet at the
end of the sg list, so padding packet can be sent to device
if drivers require that.
Reported-by: David Laight
Acked-by: Oliver Neukum
Signed-off-by: Ming Lei
---
v1:
- fix bug in case of dev->can_dma_sg and !urb->num_sgs
---
drivers/net/usb/usbnet.c | 27 +--
include/linux/usb/usbnet.h |1 +
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 7b331e6..bf94e10 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1241,7 +1241,9 @@ static int build_dma_sg(const struct sk_buff *skb, struct
urb *urb)
if (num_sgs == 1)
return 0;
- urb->sg = kmalloc(num_sgs * sizeof(struct scatterlist), GFP_ATOMIC);
+ /* reserve one for zero packet */
+ urb->sg = kmalloc((num_sgs + 1) * sizeof(struct scatterlist),
+ GFP_ATOMIC);
if (!urb->sg)
return -ENOMEM;
@@ -1305,7 +1307,7 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
if (build_dma_sg(skb, urb) < 0)
goto drop;
}
- entry->length = length = urb->transfer_buffer_length;
+ length = urb->transfer_buffer_length;
/* don't assume the hardware handles USB_ZERO_PACKET
* NOTE: strictly conforming cdc-ether devices should expect
@@ -1317,15 +1319,18 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
if (length % dev->maxpacket == 0) {
if (!(info->flags & FLAG_SEND_ZLP)) {
if (!(info->flags & FLAG_MULTI_PACKET)) {
- urb->transfer_buffer_length++;
- if (skb_tailroom(skb)) {
+ length++;
+ if (skb_tailroom(skb) && !urb->num_sgs) {
skb->data[skb->len] = 0;
__skb_put(skb, 1);
- }
+ } else if (urb->num_sgs)
+ sg_set_buf(&urb->sg[urb->num_sgs++],
+ dev->padding_pkt, 1);
}
} else
urb->transfer_flags |= URB_ZERO_PACKET;
}
+ entry->length = urb->transfer_buffer_length = length;
spin_lock_irqsave(&dev->txq.lock, flags);
retval = usb_autopm_get_interface_async(dev->intf);
@@ -1509,6 +1514,7 @@ void usbnet_disconnect (struct usb_interface *intf)
usb_kill_urb(dev->interrupt);
usb_free_urb(dev->interrupt);
+ kfree(dev->padding_pkt);
free_netdev(net);
}
@@ -1679,9 +1685,16 @@ usbnet_probe (struct usb_interface *udev, const struct
usb_device_id *prod)
/* initialize max rx_qlen and tx_qlen */
usbnet_update_max_qlen(dev);
+ if (dev->can_dma_sg && !(info->flags & FLAG_SEND_ZLP) &&
+ !(info->flags & FLAG_MULTI_PACKET)) {
+ dev->padding_pkt = kzalloc(1, GFP_KERNEL);
+ if (!dev->padding_pkt)
+ goto out4;
+ }
+
status = register_netdev (net);
if (status)
- goto out4;
+ goto out5;
netif_info(dev, probe, dev->net,
"register '%s' at usb-%s-%s, %s, %pM\n",
udev->dev.driver->name,
@@ -1699,6 +1712,8 @@ usbnet_probe (struct usb_interface *udev, const struct
usb_device_id *prod)
return 0;
+out5:
+ kfree(dev->padding_pkt);
out4:
usb_free_urb(dev->interrupt);
out3:
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 9cb2fe8..e303eef 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -42,6 +42,7 @@ struct usbnet {
struct usb_host_endpoint *status;
unsignedmaxpacket;
struct timer_list delay;
+ const char *padding_pkt;
/* protocol/interface state */
struct net_device *net;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 09/10] usb: dwc3: omap: manage "usb_otg_ss_refclk960m" clock
Hi Felipe,
On 09/18/2013 03:49 PM, Roger Quadros wrote:
> "usb_otg_ss_refclk960m" is an optional functional clock to the
> UBS_OTG_SS module. So manage it in the driver.
>
Just realized that "usb_otg_ss_refclk960m" is in fact functional clock to the
PHY and not USB_OTG_SS module. The name is misleading.
So please ignore patch 9 and 10.
cheers,
-roger
> Also update device tree binding information.
>
> Signed-off-by: Roger Quadros
> ---
> Documentation/devicetree/bindings/usb/omap-usb.txt |4
> drivers/usb/dwc3/dwc3-omap.c | 13 +
> 2 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt
> b/Documentation/devicetree/bindings/usb/omap-usb.txt
> index f67573c..47c8530 100644
> --- a/Documentation/devicetree/bindings/usb/omap-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
> @@ -47,6 +47,8 @@ OMAP DWC3 GLUE
> - #address-cells, #size-cells : Must be present if the device has sub-nodes
> - utmi-mode : controls the source of UTMI/PIPE status for VBUS and OTG ID.
> It should be set to "1" for HW mode and "2" for SW mode.
> + - clock : should refer to the clock node that provides 960MHz functional
> clock.
> + - clock-names : should be "usb_otg_ss_refclk960m"
> - ranges: the child address space are mapped 1:1 onto the parent address
> space
>
> Optional Properties:
> @@ -68,6 +70,8 @@ omap_dwc3 {
> #address-cells = <1>;
> #size-cells = <1>;
> utmi-mode = <2>;
> + clocks = <&usb_otg_ss1_refclk960m>;
> + clock-names = "usb_otg_ss_refclk960m";
> ranges;
> };
>
> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
> index 7f7ea62..c33b26c 100644
> --- a/drivers/usb/dwc3/dwc3-omap.c
> +++ b/drivers/usb/dwc3/dwc3-omap.c
> @@ -32,6 +32,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -119,6 +120,8 @@
> #define USBOTGSS_UTMI_OTG_STATUS_SESSVALID (1 << 2)
> #define USBOTGSS_UTMI_OTG_STATUS_VBUSVALID (1 << 1)
>
> +#define USBOTGSS_REFCLK "usb_otg_ss_refclk960m"
> +
> struct dwc3_omap {
> /* device lock */
> spinlock_t lock;
> @@ -144,6 +147,7 @@ struct dwc3_omap {
> struct notifier_block id_nb;
>
> struct regulator*vbus_reg;
> + struct clk *refclk;
> };
>
> enum omap_dwc3_vbus_id_status {
> @@ -449,6 +453,12 @@ static int dwc3_omap_probe(struct platform_device *pdev)
> }
> }
>
> + omap->refclk = devm_clk_get(dev, USBOTGSS_REFCLK);
> + if (IS_ERR(omap->refclk)) {
> + dev_err(dev, "couldn't get %s\n", USBOTGSS_REFCLK);
> + return PTR_ERR(omap->refclk);
> + }
> +
> spin_lock_init(&omap->lock);
>
> omap->dev = dev;
> @@ -464,6 +474,8 @@ static int dwc3_omap_probe(struct platform_device *pdev)
> goto err0;
> }
>
> + clk_prepare_enable(omap->refclk);
> +
> reg = dwc3_omap_readl(omap->base, USBOTGSS_REVISION);
> omap->revision = reg;
> x_major = USBOTGSS_REVISION_XMAJOR(reg);
> @@ -593,6 +605,7 @@ static int dwc3_omap_remove(struct platform_device *pdev)
> extcon_unregister_interest(&omap->extcon_id_dev);
> dwc3_omap_disable_irqs(omap);
> pm_runtime_put_sync(&pdev->dev);
> + clk_disable_unprepare(omap->refclk);
> pm_runtime_disable(&pdev->dev);
> device_for_each_child(&pdev->dev, NULL, dwc3_omap_remove_core);
>
>
--
To unsubscribe from this list: send the line "unsubscribe 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: usbtest: bmAttributes would better be masked
When transfer type is isochronous, the other bits (bits 5..2) of bmAttributes in
endpoint descriptor might not be set zero. So it's better to mask bmAttributes
with USB_ENDPOINT_XFERTYPE_MASK to judge the transfter type later.
Signed-off-by: Huang Rui
---
drivers/usb/misc/usbtest.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index aa28ac8..c69f199 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -120,7 +120,8 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface
*intf)
struct usb_host_endpoint*e;
e = alt->endpoint + ep;
- switch (e->desc.bmAttributes) {
+ switch (e->desc.bmAttributes &
+ USB_ENDPOINT_XFERTYPE_MASK) {
case USB_ENDPOINT_XFER_BULK:
break;
case USB_ENDPOINT_XFER_ISOC:
--
1.7.11.7
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: gadget: s3c-hsotg: add isochronous transfers support
Hi Robert,
On Monday, September 23, 2013 10:07:12 AM Robert Baldyga wrote:
> This patch adds isochronous transfer support. It adds few modifications:
> - Modify s3c_hsotg_write_fifo() function. It actually calculates transfer
> size, taking into account Multi Count value, which indicates number of
> transactions per microframe.
> - Fix s3c_hsotg_start_req() function by setting number of packets to Multi
> Count field in DIEPTSIZ register for isochronous endpoints.
> - Fix s3c_hsotg_set_ep_maxpacket() function. Field wMaxPacketSize of endpoint
> descriptor is now splitted into maximum packet size value and number of
> additional transaction per microframe.
> - Modify s3c_hsotg_epint() function. Some interrupts are ignored for
> isochronous endpoints, (e.g. INTknTXFEmpMsk) becouse isochronous request is
> always transfered in single transaction, which ends with XferCompl
> interrupt.
> Add Odd/Even microframe toggle to allow data transfering in each microframe.
> - Fix s3c_hsotg_ep_enable() function by supporting isochronous endpoint type.
>
> Signed-off-by: Robert Baldyga
> Signed-off-by: Kyungmin Park
> ---
> drivers/usb/gadget/s3c-hsotg.c | 74
> +++-
> 1 file changed, 57 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
> index d5d951d..d737452 100644
> --- a/drivers/usb/gadget/s3c-hsotg.c
> +++ b/drivers/usb/gadget/s3c-hsotg.c
> @@ -82,9 +82,12 @@ struct s3c_hsotg_req;
> * @dir_in: Set to true if this endpoint is of the IN direction, which
> * means that it is sending data to the Host.
> * @index: The index for the endpoint registers.
> + * @mc: Multi Count - number of transactions per microframe
> + * @interval - Interval for periodic endpoints
> * @name: The name array passed to the USB core.
> * @halted: Set if the endpoint has been halted.
> * @periodic: Set if this is a periodic ep, such as Interrupt
> + * @insochronous: Set if this is a isochronous ep
s/insochronous/isochronous/
> * @sent_zlp: Set if we've sent a zero-length packet.
> * @total_data: The total number of data bytes done.
> * @fifo_size: The size of the FIFO (for periodic IN endpoints)
> @@ -120,9 +123,12 @@ struct s3c_hsotg_ep {
>
> unsigned char dir_in;
> unsigned char index;
> + unsigned char mc;
> + unsigned char interval;
>
> unsigned inthalted:1;
> unsigned intperiodic:1;
> + unsigned intisochronous:1;
> unsigned intsent_zlp:1;
>
> charname[10];
> @@ -467,6 +473,7 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
> void *data;
> int can_write;
> int pkt_round;
> + int max_transfer;
>
> to_write -= (buf_pos - hs_ep->last_load);
>
> @@ -534,15 +541,17 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
> can_write *= 4; /* fifo size is in 32bit quantities. */
> }
>
> - dev_dbg(hsotg->dev, "%s: GNPTXSTS=%08x, can=%d, to=%d, mps %d\n",
> - __func__, gnptxsts, can_write, to_write, hs_ep->ep.maxpacket);
> + max_transfer = hs_ep->ep.maxpacket * hs_ep->mc;
> +
> + dev_dbg(hsotg->dev, "%s: GNPTXSTS=%08x, can=%d, to=%d, max_transfer
> %d\n",
> + __func__, gnptxsts, can_write, to_write, max_transfer);
>
> /*
>* limit to 512 bytes of data, it seems at least on the non-periodic
>* FIFO, requests of >512 cause the endpoint to get stuck with a
>* fragment of the end of the transfer in it.
>*/
> - if (can_write > 512)
> + if (can_write > 512 && !periodic)
> can_write = 512;
Doesn't it also affect non-isochronous transfers?
If so this change should be in a separate preparatory patch.
> /*
> @@ -550,8 +559,8 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
>* the transfer to return that it did not run out of fifo space
>* doing it.
>*/
> - if (to_write > hs_ep->ep.maxpacket) {
> - to_write = hs_ep->ep.maxpacket;
> + if (to_write > max_transfer) {
> + to_write = max_transfer;
>
> /* it's needed only when we do not use dedicated fifos */
> if (!hsotg->dedicated_fifos)
> @@ -564,7 +573,7 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
>
> if (to_write > can_write) {
> to_write = can_write;
> - pkt_round = to_write % hs_ep->ep.maxpacket;
> + pkt_round = to_write % max_transfer;
>
> /*
>* Round the write down to an
> @@ -730,8 +739,16 @@ static void s3c_hsotg_start_req(struct s3c_hsotg *hsotg,
> else
> packets = 1;/* send one packet if length is zero. */
>
> + if (length > (hs_ep->mc * hs_ep->ep.maxpacket) && hs_ep->isochronous) {
Wouldn't checking hs
Re: No keyboard and mouse usb detected at startup.
On Mon, 23 Sep 2013, Kumar Gaurav wrote: > Hi Alan, > I have one question just for knowledge. I was looking for the reason of > this bug but wasn't even able to identify which driver (ehci,xhci etc) > these devices are using. I tried inspecting with my USB mouse and found > out it was ehci (i believe it depends on devices). > >> My configuration files are almost similar between 3.10 and 3.11... > >> Attached 2 "lsusb -v" files, 1 that works, the other doesn't. > >> > >> I use debian wheezy with LUKS. > >> > >> Thank you, best regards > >> > >> Note: Same problem with the kernel 3.12rc1. > > It looks like your system did not load the ohci-pci driver. Is > > CONFIG_USB_OHCI_HCD_PCI enabled? > how did you identified it was using ohci? You can tell by looking at the output from lsusb -v. Here are the important lines: > Bus 004 Device 002: ID 04d9:1603 Holtek Semiconductor, Inc. Keyboard > Bus 005 Device 002: ID 1bcf:0005 Sunplus Innovation Technology Inc. These lines say that the Holtek keyboard is on bus 4 and the Sunplus mouse is on bus 5. > Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub > Device Descriptor: > bLength18 > bDescriptorType 1 > bcdUSB 1.10 > bDeviceClass9 Hub > bDeviceSubClass 0 Unused > bDeviceProtocol 0 Full speed (or root) hub > bMaxPacketSize064 > idVendor 0x1d6b Linux Foundation > idProduct 0x0001 1.1 root hub > bcdDevice3.10 > iManufacturer 3 Linux 3.10.11-gnu-hardened ohci_hcd > iProduct2 OHCI Host Controller > iSerial 1 :00:12.0 > Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub > Device Descriptor: > bLength18 > bDescriptorType 1 > bcdUSB 1.10 > bDeviceClass9 Hub > bDeviceSubClass 0 Unused > bDeviceProtocol 0 Full speed (or root) hub > bMaxPacketSize064 > idVendor 0x1d6b Linux Foundation > idProduct 0x0001 1.1 root hub > bcdDevice3.10 > iManufacturer 3 Linux 3.10.11-gnu-hardened ohci_hcd > iProduct2 OHCI Host Controller > iSerial 1 :00:13.0 These lines say that the host controllers for buses 4 and 5 are both OHCI (see the iProduct entries). Also, the iSerial values give the controllers' PCI addresses. 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 3/7] mtd: atmel_nand: fix deferred probe from __init
Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.
Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.
Cc: David Woodhouse
Cc: Josh Wu
Signed-off-by: Johan Hovold
---
drivers/mtd/nand/atmel_nand.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 060feea..bd1ce7d 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -1139,7 +1139,7 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host,
return 0;
}
-static int __init atmel_pmecc_nand_init_params(struct platform_device *pdev,
+static int atmel_pmecc_nand_init_params(struct platform_device *pdev,
struct atmel_nand_host *host)
{
struct mtd_info *mtd = &host->mtd;
@@ -1548,7 +1548,7 @@ static int atmel_of_init_port(struct atmel_nand_host
*host,
}
#endif
-static int __init atmel_hw_nand_init_params(struct platform_device *pdev,
+static int atmel_hw_nand_init_params(struct platform_device *pdev,
struct atmel_nand_host *host)
{
struct mtd_info *mtd = &host->mtd;
@@ -1987,7 +1987,7 @@ static struct platform_driver atmel_nand_nfc_driver;
/*
* Probe for the NAND device.
*/
-static int __init atmel_nand_probe(struct platform_device *pdev)
+static int atmel_nand_probe(struct platform_device *pdev)
{
struct atmel_nand_host *host;
struct mtd_info *mtd;
@@ -2184,7 +2184,7 @@ err_nand_ioremap:
/*
* Remove a NAND device.
*/
-static int __exit atmel_nand_remove(struct platform_device *pdev)
+static int atmel_nand_remove(struct platform_device *pdev)
{
struct atmel_nand_host *host = platform_get_drvdata(pdev);
struct mtd_info *mtd = &host->mtd;
@@ -2270,7 +2270,8 @@ static struct platform_driver atmel_nand_nfc_driver = {
};
static struct platform_driver atmel_nand_driver = {
- .remove = __exit_p(atmel_nand_remove),
+ .probe = atmel_nand_probe,
+ .remove = atmel_nand_remove,
.driver = {
.name = "atmel_nand",
.owner = THIS_MODULE,
@@ -2278,7 +2279,7 @@ static struct platform_driver atmel_nand_driver = {
},
};
-module_platform_driver_probe(atmel_nand_driver, atmel_nand_probe);
+module_platform_driver(atmel_nand_driver);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Rick Bronson");
--
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
[PATCH 0/7] driver core: prevent deferred probe with platform_driver_probe
Deferred probing cannot be used with platform_driver_probe as by the
time probing is retried either the driver has been unregistered or its
probe function has been set to platform_drv_probe_fail.
With commit e9354576 ("gpiolib: Defer failed gpio requests by default")
the gpio subsystem started returning -EPROBE_DEFER, which in turn
several platform drivers using platform_driver_probe return to driver
core. Other subsystems (e.g. regulator) has since started doing the
same.
The first patch in this series prevents platform drivers using
platform_driver_probe from requesting probe deferral while warning that
it is not supported.
The remaining patches move six platform-driver probe functions that rely
on gpio_request out of __init. There are likely other probe functions
that might return -EPROBE_DEFER and should be moved out of __init as
well.
Note that the mvsdio, at91_cf and pxa25x_udc patches are completely
untested.
Johan
Johan Hovold (7):
driver core: prevent deferred probe with platform_driver_probe
mmc: mvsdio: fix deferred probe from __init
mtd: atmel_nand: fix deferred probe from __init
pcmcia: at91_cf: fix deferred probe from __init
usb: gadget: pxa25x_udc: fix deferred probe from __init
usb: phy: gpio-vbus: fix deferred probe from __init
backlight: atmel-pwm-bl: fix deferred probe from __init
drivers/base/platform.c| 17 +
drivers/mmc/host/mvsdio.c | 11 ++-
drivers/mtd/nand/atmel_nand.c | 13 +++--
drivers/pcmcia/at91_cf.c | 11 +--
drivers/usb/gadget/pxa25x_udc.c| 9 +
drivers/usb/phy/phy-gpio-vbus-usb.c| 11 +--
drivers/video/backlight/atmel-pwm-bl.c | 9 +
include/linux/platform_device.h| 1 +
8 files changed, 47 insertions(+), 35 deletions(-)
--
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/core/devio.c:tolerate wrong direction flag in control endpoints
On Mon, 23 Sep 2013, Kurt Garloff wrote:
> >> that qualifies as a bug or not. Maybe it should not claim to be a
> >> HID device then?
> > Maybe not. This particular combination of bRequestType and bRequest
> > values (0x22, 0x09) is not defined in the HID 1.11 spec. Do you know
> > if it is defined somewhere else?
> These are custom commands, somewhat described at
> http://pegatech.com/_Uploads/Downloads/Documents/Protocol_Definition_Rev_1.12.pdf
That document describes a UART protocol with no mention of USB at all.
> >> (And again the behavior might not be enforced by the spec, but maybe
> >> by Windows?)
> > More likely the behavior isn't enforced at all. The device may simply
> > be buggy.
> With behavior here I referred to the fact that I have not yet seen a USB
> device that
> has two endpoints with the same endpoint number (but different direction).
I have. They aren't very common but they do exist.
> Let me try inline insert (by c'n'p: I switched from mutt to Thunderbird
> recently and lack
> experience whether this breaks formatting or so ...)
It did mangle the whitespace characters. That doesn't matter for
reviewing, but it is important when you submit the patch. Take a look
at Documentation/email-clients.txt for some suggestions.
> 8<
>
> From: Kurt Garloff
> Date: Mon, 23 Sep 2013 14:19:02 +0200
> Subject: Tolerate wrong direction bit in endpoint address for control
> messages
>
> Trying to read data from the Pegasus Technologies NoteTaker (0e20:0101)
> [1] with the Windows App (EasyNote) works natively but fails when
> Windows is running under KVM (and the USB device handed to KVM).
>
> The reason is a USB control message
> usb 4-2.2: control urb: bRequestType=22 bRequest=09 wValue=0200
> wIndex=0001 wLength=0008
> This goes to endpoint address 0x01 (wIndex); however, endpoint number 1
> is an input endpoint and thus has endpoint address 0x81.
You should say something like:
however, endpoint 0x01 doesn't exist. There is an endpoint
0x81, though; perhaps the app meant that endpoint instead.
> The kernel thus rejects the IO and thus we see the failure.
>
> Apparently, Linux is more strict here than Windows ... we can't change
> the Win app easily, so that's a problem.
>
> It seems that the Win app/driver is buggy here and the driver does not
> behave fully according to the USB HID class that it claims to belong to.
> The device seems to happily deal with that though (and seems to not
> really care about this value much).
>
> So the question is whether the Linux kernel should filter here.
> Rejecting has the risk that somewhat non-compliant userspace apps/
> drivers (most likely in a virtual machine) are prevented from working.
> Not rejecting has the risk of confusing an overly sensitive device with
> such a transfer. Given the fact that Windows does not filter it makes
> this risk rather small though.
>
> The patch makes the kernel more tolerant: If the endpoint address in
> wIndex does not exist, but an endpoint with toggled direction bit does,
> it will let the transfer through. (It does NOT change the message.)
>
> With attached patch, the app in Windows in KVM works.
> usb 4-2.2: check_ctrlrecip: process 13073 (qemu-kvm) requesting ep 01
> but needs 81 (or 00)
You need to remove the "(or 00)" here.
> I suspect this will mostly affect apps in virtual environments; as on
> Linux the apps would have been adapted to the stricter handling of the
> kernel. I have done that for mine[2].
>
> [1] http://www.pegatech.com/
> [2] https://sourceforge.net/projects/notetakerpen/
>
> Signed-off-by: Kurt Garloff
> Cc: sta...@vger.kernel.or
Fix the spelling (.org).
> ---
> drivers/usb/core/devio.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index 737e3c1..4ff61f9 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -742,6 +742,22 @@ static int check_ctrlrecip(struct dev_state *ps,
> unsigned int requesttype,
> if ((index & ~USB_DIR_IN) == 0)
> return 0;
> ret = findintfep(ps->dev, index);
> +if (ret < 0) {
> +/*
> + * Some not fully compliant Win apps seem to get
> + * ndex wrong and have the endpoint number here
s/ndex/index/
> + * rather than the endpoint address (with the
> + * correct direction). Win does let this through,
> + * so we'll give it a second try as well (to not
> + * break KVM) -- but warn.
> + */
> +ret = findintfep(ps->dev, index ^ 0x80);
> +if (ret >= 0)
> +dev_info(&ps->dev->dev ,
> +"%s: process %i (%s) requesting ep %02x but needs
> %02x\n",
> +__func__, task_pid_nr(current),
> +current->comm, index, index ^ 0x80);
> +}
> if (ret >= 0)
> re
[PATCH 2/7] mmc: mvsdio: fix deferred probe from __init
Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.
Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if the mmc_gpio_request_cd fails.
Cc: Nicolas Pitre
Cc: Chris Ball
Signed-off-by: Johan Hovold
---
drivers/mmc/host/mvsdio.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index 06c5b0b..deecee0 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -655,7 +655,7 @@ static const struct mmc_host_ops mvsd_ops = {
.enable_sdio_irq= mvsd_enable_sdio_irq,
};
-static void __init
+static void
mv_conf_mbus_windows(struct mvsd_host *host,
const struct mbus_dram_target_info *dram)
{
@@ -677,7 +677,7 @@ mv_conf_mbus_windows(struct mvsd_host *host,
}
}
-static int __init mvsd_probe(struct platform_device *pdev)
+static int mvsd_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct mmc_host *mmc = NULL;
@@ -819,7 +819,7 @@ out:
return ret;
}
-static int __exit mvsd_remove(struct platform_device *pdev)
+static int mvsd_remove(struct platform_device *pdev)
{
struct mmc_host *mmc = platform_get_drvdata(pdev);
@@ -872,7 +872,8 @@ static const struct of_device_id mvsdio_dt_ids[] = {
MODULE_DEVICE_TABLE(of, mvsdio_dt_ids);
static struct platform_driver mvsd_driver = {
- .remove = __exit_p(mvsd_remove),
+ .probe = mvsd_probe,
+ .remove = mvsd_remove,
.suspend= mvsd_suspend,
.resume = mvsd_resume,
.driver = {
@@ -881,7 +882,7 @@ static struct platform_driver mvsd_driver = {
},
};
-module_platform_driver_probe(mvsd_driver, mvsd_probe);
+module_platform_driver(mvsd_driver);
/* maximum card clock frequency (default 50MHz) */
module_param(maxfreq, int, 0);
--
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
[PATCH 6/7] usb: phy: gpio-vbus: fix deferred probe from __init
Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.
Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
and 04bf3011 ("regulator: Support driver probe deferral") this driver
might return -EPROBE_DEFER if a gpio_request or regulator_get fails.
Cc: Felipe Balbi
Signed-off-by: Johan Hovold
---
drivers/usb/phy/phy-gpio-vbus-usb.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/phy/phy-gpio-vbus-usb.c
b/drivers/usb/phy/phy-gpio-vbus-usb.c
index b2f29c9..02799a5 100644
--- a/drivers/usb/phy/phy-gpio-vbus-usb.c
+++ b/drivers/usb/phy/phy-gpio-vbus-usb.c
@@ -241,7 +241,7 @@ static int gpio_vbus_set_suspend(struct usb_phy *phy, int
suspend)
/* platform driver interface */
-static int __init gpio_vbus_probe(struct platform_device *pdev)
+static int gpio_vbus_probe(struct platform_device *pdev)
{
struct gpio_vbus_mach_info *pdata = dev_get_platdata(&pdev->dev);
struct gpio_vbus_data *gpio_vbus;
@@ -349,7 +349,7 @@ err_gpio:
return err;
}
-static int __exit gpio_vbus_remove(struct platform_device *pdev)
+static int gpio_vbus_remove(struct platform_device *pdev)
{
struct gpio_vbus_data *gpio_vbus = platform_get_drvdata(pdev);
struct gpio_vbus_mach_info *pdata = dev_get_platdata(&pdev->dev);
@@ -398,8 +398,6 @@ static const struct dev_pm_ops gpio_vbus_dev_pm_ops = {
};
#endif
-/* NOTE: the gpio-vbus device may *NOT* be hotplugged */
-
MODULE_ALIAS("platform:gpio-vbus");
static struct platform_driver gpio_vbus_driver = {
@@ -410,10 +408,11 @@ static struct platform_driver gpio_vbus_driver = {
.pm = &gpio_vbus_dev_pm_ops,
#endif
},
- .remove = __exit_p(gpio_vbus_remove),
+ .probe = gpio_vbus_probe,
+ .remove = gpio_vbus_remove,
};
-module_platform_driver_probe(gpio_vbus_driver, gpio_vbus_probe);
+module_platform_driver(gpio_vbus_driver);
MODULE_DESCRIPTION("simple GPIO controlled OTG transceiver driver");
MODULE_AUTHOR("Philipp Zabel");
--
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
[PATCH 4/7] pcmcia: at91_cf: fix deferred probe from __init
Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.
Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.
Cc: Jean-Christophe PLAGNIOL-VILLARD
Cc: Nicolas Ferre
Signed-off-by: Johan Hovold
---
drivers/pcmcia/at91_cf.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/pcmcia/at91_cf.c b/drivers/pcmcia/at91_cf.c
index b8f5acf..de24232 100644
--- a/drivers/pcmcia/at91_cf.c
+++ b/drivers/pcmcia/at91_cf.c
@@ -245,7 +245,7 @@ static int at91_cf_dt_init(struct platform_device *pdev)
}
#endif
-static int __init at91_cf_probe(struct platform_device *pdev)
+static int at91_cf_probe(struct platform_device *pdev)
{
struct at91_cf_socket *cf;
struct at91_cf_data *board = pdev->dev.platform_data;
@@ -354,7 +354,7 @@ fail0a:
return status;
}
-static int __exit at91_cf_remove(struct platform_device *pdev)
+static int at91_cf_remove(struct platform_device *pdev)
{
struct at91_cf_socket *cf = platform_get_drvdata(pdev);
@@ -404,14 +404,13 @@ static struct platform_driver at91_cf_driver = {
.owner = THIS_MODULE,
.of_match_table = of_match_ptr(at91_cf_dt_ids),
},
- .remove = __exit_p(at91_cf_remove),
+ .probe = at91_cf_probe,
+ .remove = at91_cf_remove,
.suspend= at91_cf_suspend,
.resume = at91_cf_resume,
};
-/*--*/
-
-module_platform_driver_probe(at91_cf_driver, at91_cf_probe);
+module_platform_driver(at91_cf_driver);
MODULE_DESCRIPTION("AT91 Compact Flash Driver");
MODULE_AUTHOR("David Brownell");
--
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
[PATCH 5/7] usb: gadget: pxa25x_udc: fix deferred probe from __init
Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.
Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.
Cc: Eric Miao
Cc: Russell King
Cc: Haojian Zhuang
Cc: Felipe Balbi
Signed-off-by: Johan Hovold
---
drivers/usb/gadget/pxa25x_udc.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/pxa25x_udc.c b/drivers/usb/gadget/pxa25x_udc.c
index cc92074..0ac6064 100644
--- a/drivers/usb/gadget/pxa25x_udc.c
+++ b/drivers/usb/gadget/pxa25x_udc.c
@@ -2054,7 +2054,7 @@ static struct pxa25x_udc memory = {
/*
* probe - binds to the platform device
*/
-static int __init pxa25x_udc_probe(struct platform_device *pdev)
+static int pxa25x_udc_probe(struct platform_device *pdev)
{
struct pxa25x_udc *dev = &memory;
int retval, irq;
@@ -2203,7 +2203,7 @@ static void pxa25x_udc_shutdown(struct platform_device
*_dev)
pullup_off();
}
-static int __exit pxa25x_udc_remove(struct platform_device *pdev)
+static int pxa25x_udc_remove(struct platform_device *pdev)
{
struct pxa25x_udc *dev = platform_get_drvdata(pdev);
@@ -2294,7 +2294,8 @@ static int pxa25x_udc_resume(struct platform_device *dev)
static struct platform_driver udc_driver = {
.shutdown = pxa25x_udc_shutdown,
- .remove = __exit_p(pxa25x_udc_remove),
+ .probe = pxa25x_udc_probe,
+ .remove = pxa25x_udc_remove,
.suspend= pxa25x_udc_suspend,
.resume = pxa25x_udc_resume,
.driver = {
@@ -2303,7 +2304,7 @@ static struct platform_driver udc_driver = {
},
};
-module_platform_driver_probe(udc_driver, pxa25x_udc_probe);
+module_platform_driver(udc_driver);
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_AUTHOR("Frank Becker, Robert Schwebel, David Brownell");
--
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
[PATCH 7/7] backlight: atmel-pwm-bl: fix deferred probe from __init
Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.
Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.
Cc: Richard Purdie
Cc: Jingoo Han
Cc: Jean-Christophe Plagniol-Villard
Signed-off-by: Johan Hovold
---
drivers/video/backlight/atmel-pwm-bl.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/atmel-pwm-bl.c
b/drivers/video/backlight/atmel-pwm-bl.c
index 0393d82..f7447f7 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -118,7 +118,7 @@ static const struct backlight_ops atmel_pwm_bl_ops = {
.update_status = atmel_pwm_bl_set_intensity,
};
-static int __init atmel_pwm_bl_probe(struct platform_device *pdev)
+static int atmel_pwm_bl_probe(struct platform_device *pdev)
{
struct backlight_properties props;
const struct atmel_pwm_bl_platform_data *pdata;
@@ -202,7 +202,7 @@ err_free_mem:
return retval;
}
-static int __exit atmel_pwm_bl_remove(struct platform_device *pdev)
+static int atmel_pwm_bl_remove(struct platform_device *pdev)
{
struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
@@ -220,10 +220,11 @@ static struct platform_driver atmel_pwm_bl_driver = {
.name = "atmel-pwm-bl",
},
/* REVISIT add suspend() and resume() */
- .remove = __exit_p(atmel_pwm_bl_remove),
+ .probe = atmel_pwm_bl_probe,
+ .remove = atmel_pwm_bl_remove,
};
-module_platform_driver_probe(atmel_pwm_bl_driver, atmel_pwm_bl_probe);
+module_platform_driver(atmel_pwm_bl_driver);
MODULE_AUTHOR("Hans-Christian egtvedt ");
MODULE_DESCRIPTION("Atmel PWM backlight driver");
--
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
[PATCH 1/7] driver core: prevent deferred probe with platform_driver_probe
Prevent drivers relying on platform_driver_probe from requesting
deferred probing in order to avoid further futile probe attempts (either
the driver has been unregistered or its probe function has been set to
platform_drv_probe_fail when probing is retried).
Note that several platform drivers currently return subsystem errors
from probe and that these can include -EPROBE_DEFER (e.g. if a gpio
request fails).
Add a warning to platform_drv_probe that can be used to catch drivers
that inadvertently request probe deferral while using
platform_driver_probe.
Signed-off-by: Johan Hovold
---
drivers/base/platform.c | 17 +
include/linux/platform_device.h | 1 +
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..47051cd 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -488,6 +488,11 @@ static int platform_drv_probe(struct device *_dev)
if (ret && ACPI_HANDLE(_dev))
acpi_dev_pm_detach(_dev, true);
+ if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
+ dev_warn(_dev, "probe deferral not supported\n");
+ ret = -ENXIO;
+ }
+
return ret;
}
@@ -553,8 +558,7 @@ EXPORT_SYMBOL_GPL(platform_driver_unregister);
/**
* platform_driver_probe - register driver for non-hotpluggable device
* @drv: platform driver structure
- * @probe: the driver probe routine, probably from an __init section,
- * must not return -EPROBE_DEFER.
+ * @probe: the driver probe routine, probably from an __init section
*
* Use this instead of platform_driver_register() when you know the device
* is not hotpluggable and has already been registered, and you want to
@@ -565,8 +569,7 @@ EXPORT_SYMBOL_GPL(platform_driver_unregister);
* into system-on-chip processors, where the controller devices have been
* configured as part of board setup.
*
- * This is incompatible with deferred probing so probe() must not
- * return -EPROBE_DEFER.
+ * Note that this is incompatible with deferred probing.
*
* Returns zero if the driver registered and bound to a device, else returns
* a negative error code and with the driver not registered.
@@ -576,6 +579,12 @@ int __init_or_module platform_driver_probe(struct
platform_driver *drv,
{
int retval, code;
+ /*
+* Prevent driver from requesting probe deferral to avoid further
+* futile probe attempts.
+*/
+ drv->prevent_deferred_probe = true;
+
/* make sure driver won't have bind/unbind attributes */
drv->driver.suppress_bind_attrs = true;
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index ce8e4ff..16f6654 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -178,6 +178,7 @@ struct platform_driver {
int (*resume)(struct platform_device *);
struct device_driver driver;
const struct platform_device_id *id_table;
+ bool prevent_deferred_probe;
};
#define to_platform_driver(drv)(container_of((drv), struct
platform_driver, \
--
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: usbtest: bmAttributes would better be masked
On Mon, 23 Sep 2013, Huang Rui wrote:
> When transfer type is isochronous, the other bits (bits 5..2) of bmAttributes
> in
> endpoint descriptor might not be set zero. So it's better to mask bmAttributes
> with USB_ENDPOINT_XFERTYPE_MASK to judge the transfter type later.
>
> Signed-off-by: Huang Rui
> ---
> drivers/usb/misc/usbtest.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
> index aa28ac8..c69f199 100644
> --- a/drivers/usb/misc/usbtest.c
> +++ b/drivers/usb/misc/usbtest.c
> @@ -120,7 +120,8 @@ get_endpoints(struct usbtest_dev *dev, struct
> usb_interface *intf)
> struct usb_host_endpoint*e;
>
> e = alt->endpoint + ep;
> - switch (e->desc.bmAttributes) {
> + switch (e->desc.bmAttributes &
> + USB_ENDPOINT_XFERTYPE_MASK) {
> case USB_ENDPOINT_XFER_BULK:
> break;
> case USB_ENDPOINT_XFER_ISOC:
Use usb_endpoint_type() instead of doing this by hand.
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 4/5] dma: cppi41: only allocate descriptor memory once
On 09/23/2013 06:17 AM, Vinod Koul wrote: > Looks fine, Sebastian cna you test it pls Just noticed that you already applied some of them. I just got back after a few weeks of. Will review & test as soon as I get to it. > > ~Vinod > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] usb: musb: am335x: Do not remove the session bit HOST-only mode
On 09/20/2013 05:45 PM, Felipe Balbi wrote: > > Acked-by: Felipe Balbi > Those four patches went already in, for instance: commit 781f17983015dae33324e34d1bb831e715fa04d4 Author: Sebastian Andrzej Siewior AuthorDate: Tue Aug 20 18:35:49 2013 +0200 Commit: Felipe Balbi CommitDate: Tue Aug 27 14:18:41 2013 -0500 usb: musb: am335x-evm: Do not remove the session bit HOST-only mode Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH]fsl/usb: Workarourd for USB erratum-A005697
On Fri, 20 Sep 2013, Mehresh Ramneek-B31383 wrote:
> From: Alan Stern [mailto:st...@rowland.harvard.edu]
> Sent: Thursday, September 19, 2013 7:03 PM
> To: Mehresh Ramneek-B31383
> Cc: linux-usb@vger.kernel.org
> Subject: Re: [PATCH]fsl/usb: Workarourd for USB erratum-A005697
>
> On Thu, 19 Sep 2013, Ramneek Mehresh wrote:
>
> > As per USB specification, in Suspend state the status bit does not
> > change until the port is suspended. However, there may be a delay in
> > suspending a port if there is a transaction currently in progress on
> > the bus.
> >
> > In the USBDR controller, the PORTSCx[SUSP] bit changes immediately
> > when the application sets it and not when the port is actually
> > suspended
> >
> > Workaround for this issue involves waiting for a minimum of 10ms to
> > allow the controller to go into SUSPEND state before proceeding ahead
>
> Why is this workaround needed? Does anything go wrong if it isn't applied?
> [Ramneek]: This workaround is needed because we have this issue with the
> controller.
> In some protocols like HNP, a notification needs to be sent to another OTG
> device
> as soon as current controller port goes into SUSPEND state. However, this
> notification could be false if the port is still transmitting some data and
> the
> controller informs system s/w that the port is already in suspend state.
Which subroutine in which source file sends the HNP notification?
> > --- a/drivers/usb/host/ehci-hub.c
> > +++ b/drivers/usb/host/ehci-hub.c
> > @@ -284,6 +284,8 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
> > else if ((t1 & PORT_PE) && !(t1 & PORT_SUSPEND)) {
> > t2 |= PORT_SUSPEND;
> > set_bit(port, &ehci->bus_suspended);
> > + if (ehci_has_fsl_susp_errata(ehci))
> > + usleep_range(1, 2);
> > }
This is the wrong place to add a delay. For one thing, this isn't
where the port gets put into suspend -- that occurs about 17 lines
later.
For another, ehci_bus_suspend() gets called when the root hub is
suspended. But you want to implement this delay when the port gets
suspended, which is in ehci_hub_control().
Also, you shouldn't use usleep_range(). Call ehci_handshake() instead.
And be certain to drop ehci->lock while you are waiting for the port to
go into suspend. 10 ms is too long to hold a spinlock.
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 0/7] driver core: prevent deferred probe with platform_driver_probe
On Mon, Sep 23, 2013 at 04:27:25PM +0200, Johan Hovold wrote:
> Deferred probing cannot be used with platform_driver_probe as by the
> time probing is retried either the driver has been unregistered or its
> probe function has been set to platform_drv_probe_fail.
>
> With commit e9354576 ("gpiolib: Defer failed gpio requests by default")
> the gpio subsystem started returning -EPROBE_DEFER, which in turn
> several platform drivers using platform_driver_probe return to driver
> core. Other subsystems (e.g. regulator) has since started doing the
> same.
>
> The first patch in this series prevents platform drivers using
> platform_driver_probe from requesting probe deferral while warning that
> it is not supported.
>
> The remaining patches move six platform-driver probe functions that rely
> on gpio_request out of __init. There are likely other probe functions
> that might return -EPROBE_DEFER and should be moved out of __init as
> well.
As usually when I read this I wonder why platform_driver_probe exists
anyway. The only advantage I can think off is that the probe functions
are in __init and thus can be disposed of later. Now you remove the
__init annotations from these probe functions. Wouldn't it be better to
convert the drivers to regular platform_driver_register instead?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: usbtest: bmAttributes would better be masked
On Mon, Sep 23, 2013 at 10:32:30AM -0400, Alan Stern wrote:
> On Mon, 23 Sep 2013, Huang Rui wrote:
>
> > When transfer type is isochronous, the other bits (bits 5..2) of
> > bmAttributes in
> > endpoint descriptor might not be set zero. So it's better to mask
> > bmAttributes
> > with USB_ENDPOINT_XFERTYPE_MASK to judge the transfter type later.
> >
> > Signed-off-by: Huang Rui
> > ---
> > drivers/usb/misc/usbtest.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
> > index aa28ac8..c69f199 100644
> > --- a/drivers/usb/misc/usbtest.c
> > +++ b/drivers/usb/misc/usbtest.c
> > @@ -120,7 +120,8 @@ get_endpoints(struct usbtest_dev *dev, struct
> > usb_interface *intf)
> > struct usb_host_endpoint*e;
> >
> > e = alt->endpoint + ep;
> > - switch (e->desc.bmAttributes) {
> > + switch (e->desc.bmAttributes &
> > + USB_ENDPOINT_XFERTYPE_MASK) {
> > case USB_ENDPOINT_XFER_BULK:
> > break;
> > case USB_ENDPOINT_XFER_ISOC:
>
> Use usb_endpoint_type() instead of doing this by hand.
>
Got it, will update soon.
Thanks,
Rui
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] driver core: prevent deferred probe with platform_driver_probe
On Mon, Sep 23, 2013 at 04:50:29PM +0200, Sascha Hauer wrote:
> On Mon, Sep 23, 2013 at 04:27:25PM +0200, Johan Hovold wrote:
> > Deferred probing cannot be used with platform_driver_probe as by the
> > time probing is retried either the driver has been unregistered or its
> > probe function has been set to platform_drv_probe_fail.
> >
> > With commit e9354576 ("gpiolib: Defer failed gpio requests by default")
> > the gpio subsystem started returning -EPROBE_DEFER, which in turn
> > several platform drivers using platform_driver_probe return to driver
> > core. Other subsystems (e.g. regulator) has since started doing the
> > same.
> >
> > The first patch in this series prevents platform drivers using
> > platform_driver_probe from requesting probe deferral while warning that
> > it is not supported.
> >
> > The remaining patches move six platform-driver probe functions that rely
> > on gpio_request out of __init. There are likely other probe functions
> > that might return -EPROBE_DEFER and should be moved out of __init as
> > well.
>
> As usually when I read this I wonder why platform_driver_probe exists
> anyway. The only advantage I can think off is that the probe functions
> are in __init and thus can be disposed of later. Now you remove the
> __init annotations from these probe functions. Wouldn't it be better to
> convert the drivers to regular platform_driver_register instead?
Perhaps that paragraph was a bit unclear: I move them out of __init
_and_ use platform_driver_register instead of platform_driver_probe as
well.
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] driver core: prevent deferred probe with platform_driver_probe
On Mon, Sep 23, 2013 at 05:20:18PM +0200, Johan Hovold wrote:
> On Mon, Sep 23, 2013 at 04:50:29PM +0200, Sascha Hauer wrote:
> > On Mon, Sep 23, 2013 at 04:27:25PM +0200, Johan Hovold wrote:
> > > Deferred probing cannot be used with platform_driver_probe as by the
> > > time probing is retried either the driver has been unregistered or its
> > > probe function has been set to platform_drv_probe_fail.
> > >
> > > With commit e9354576 ("gpiolib: Defer failed gpio requests by default")
> > > the gpio subsystem started returning -EPROBE_DEFER, which in turn
> > > several platform drivers using platform_driver_probe return to driver
> > > core. Other subsystems (e.g. regulator) has since started doing the
> > > same.
> > >
> > > The first patch in this series prevents platform drivers using
> > > platform_driver_probe from requesting probe deferral while warning that
> > > it is not supported.
> > >
> > > The remaining patches move six platform-driver probe functions that rely
> > > on gpio_request out of __init. There are likely other probe functions
> > > that might return -EPROBE_DEFER and should be moved out of __init as
> > > well.
> >
> > As usually when I read this I wonder why platform_driver_probe exists
> > anyway. The only advantage I can think off is that the probe functions
> > are in __init and thus can be disposed of later. Now you remove the
> > __init annotations from these probe functions. Wouldn't it be better to
> > convert the drivers to regular platform_driver_register instead?
>
> Perhaps that paragraph was a bit unclear: I move them out of __init
> _and_ use platform_driver_register instead of platform_driver_probe as
> well.
Oh yes, I should have looked closer. Sorry for the noise.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/5] dma: cppi41: only allocate descriptor memory once
On Mon, Sep 23, 2013 at 04:51:06PM +0200, Sebastian Andrzej Siewior wrote: > On 09/23/2013 06:17 AM, Vinod Koul wrote: > > Looks fine, Sebastian cna you test it pls > > Just noticed that you already applied some of them. I just got back > after a few weeks of. Will review & test as soon as I get to it. Yes, the first three were trvial, 5th one looked fine to me ! ~Vinod -- -- To unsubscribe from this list: send the line "unsubscribe 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/core/devio.c:tolerate wrong direction flag in control endpoints
Hi Alan,
Alan Stern schrieb:
> On Mon, 23 Sep 2013, Kurt Garloff wrote:
>
> > >> that qualifies as a bug or not. Maybe it should not claim to be a
> > >> HID device then?
> > > Maybe not. This particular combination of bRequestType and
> bRequest
> > > values (0x22, 0x09) is not defined in the HID 1.11 spec. Do you
> know
> > > if it is defined somewhere else?
> > These are custom commands, somewhat described at
> >
> http://pegatech.com/_Uploads/Downloads/Documents/Protocol_Definition_Rev_1.12.pdf
>
> That document describes a UART protocol with no mention of USB at all.
Yep, probably they just took the serial protocol to USB...
Beyond the spec, I did watch the Win app a bit when implementing [2] ...
> > With behavior here I referred to the fact that I have not yet seen a
> USB
> > device that
> > has two endpoints with the same endpoint number (but different direction).
>
> I have. They aren't very common but they do exist.
>
> > Let me try inline insert (by c'n'p: I switched from mutt to Thunderbird
> > recently and lack
> > experience whether this breaks formatting or so ...)
>
> It did mangle the whitespace characters.
Cr*p.
> That doesn't matter for
> reviewing, but it is important when you submit the patch. Take a look
>
> at Documentation/email-clients.txt for some suggestions.
I'll go back to mutt then, I guess - used it for >10 years...
> > 8<
> >
> > From: Kurt Garloff
> > Date: Mon, 23 Sep 2013 14:19:02 +0200
> > Subject: Tolerate wrong direction bit in endpoint address for control
> > messages
> >
> > Trying to read data from the Pegasus Technologies NoteTaker (0e20:0101)
> > [1] with the Windows App (EasyNote) works natively but fails when
> > Windows is running under KVM (and the USB device handed to KVM).
> >
> > The reason is a USB control message
> > usb 4-2.2: control urb: bRequestType=22 bRequest=09 wValue=0200
> > wIndex=0001 wLength=0008
> > This goes to endpoint address 0x01 (wIndex); however, endpoint number 1
> > is an input endpoint and thus has endpoint address 0x81.
>
> You should say something like:
>
> however, endpoint 0x01 doesn't exist. There is an endpoint
> 0x81, though; perhaps the app meant that endpoint instead.
OK.
> > The kernel thus rejects the IO and thus we see the failure.
> >
> > Apparently, Linux is more strict here than Windows ... we can't change
> > the Win app easily, so that's a problem.
> >
> > It seems that the Win app/driver is buggy here and the driver does not
> > behave fully according to the USB HID class that it claims to belong to.
> > The device seems to happily deal with that though (and seems to not
> > really care about this value much).
> >
> > So the question is whether the Linux kernel should filter here.
> > Rejecting has the risk that somewhat non-compliant userspace apps/
> > drivers (most likely in a virtual machine) are prevented from
> working.
> > Not rejecting has the risk of confusing an overly sensitive device with
> > such a transfer. Given the fact that Windows does not filter it makes
> > this risk rather small though.
> >
> > The patch makes the kernel more tolerant: If the endpoint address in
> > wIndex does not exist, but an endpoint with toggled direction bit does,
> > it will let the transfer through. (It does NOT change the message.)
> >
> > With attached patch, the app in Windows in KVM works.
> > usb 4-2.2: check_ctrlrecip: process 13073 (qemu-kvm) requesting ep
> 01
> > but needs 81 (or 00)
>
> You need to remove the "(or 00)" here.
Good catch! I changed the code but not the log...
> > I suspect this will mostly affect apps in virtual environments; as on
> > Linux the apps would have been adapted to the stricter handling of the
> > kernel. I have done that for mine[2].
> >
> > [1] http://www.pegatech.com/
> > [2] https://sourceforge.net/projects/notetakerpen/
> >
> > Signed-off-by: Kurt Garloff
> > Cc: sta...@vger.kernel.or
>
> Fix the spelling (.org).
The second time in two days, my fingers are getting too slow...
> > ---
> > drivers/usb/core/devio.c | 16
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> > index 737e3c1..4ff61f9 100644
> > --- a/drivers/usb/core/devio.c
> > +++ b/drivers/usb/core/devio.c
> > @@ -742,6 +742,22 @@ static int check_ctrlrecip(struct dev_state *ps,
> > unsigned int requesttype,
> > if ((index & ~USB_DIR_IN) == 0)
> > return 0;
> > ret = findintfep(ps->dev, index);
> > +if (ret < 0) {
> > +/*
> > + * Some not fully compliant Win apps seem to get
> > + * ndex wrong and have the endpoint number here
>
> s/ndex/index/
Thx.
>
> > + * rather than the endpoint address (with the
> > + * correct direction). Win does let this through,
> > + * so we'll give it a second try as well (to not
> > + * break KVM) -
[PATCH v2] usb: usbtest: bmAttributes would better be masked
When transfer type is isochronous, the other bits (bits 5..2) of
bmAttributes in endpoint descriptor might not be set zero. So it's better
to use usb_endpoint_type routine to mask bmAttributes with
USB_ENDPOINT_XFERTYPE_MASK to judge the transfter type later.
Signed-off-by: Huang Rui
---
Changes from v1 -> v2:
- Use usb_endpoint_type routine instead of doing by hand.
drivers/usb/misc/usbtest.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index aa28ac8..3e91d3e9 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -120,7 +120,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface
*intf)
struct usb_host_endpoint*e;
e = alt->endpoint + ep;
- switch (e->desc.bmAttributes) {
+ switch (usb_endpoint_type(&e->desc)) {
case USB_ENDPOINT_XFER_BULK:
break;
case USB_ENDPOINT_XFER_ISOC:
--
1.7.11.7
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] pcmcia: at91_cf: fix deferred probe from __init
On 23/09/2013 16:27, Johan Hovold :
Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.
Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.
Cc: Jean-Christophe PLAGNIOL-VILLARD
Cc: Nicolas Ferre
Acked-by: Nicolas Ferre
Thanks Johan.
Signed-off-by: Johan Hovold
---
drivers/pcmcia/at91_cf.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/pcmcia/at91_cf.c b/drivers/pcmcia/at91_cf.c
index b8f5acf..de24232 100644
--- a/drivers/pcmcia/at91_cf.c
+++ b/drivers/pcmcia/at91_cf.c
@@ -245,7 +245,7 @@ static int at91_cf_dt_init(struct platform_device *pdev)
}
#endif
-static int __init at91_cf_probe(struct platform_device *pdev)
+static int at91_cf_probe(struct platform_device *pdev)
{
struct at91_cf_socket *cf;
struct at91_cf_data *board = pdev->dev.platform_data;
@@ -354,7 +354,7 @@ fail0a:
return status;
}
-static int __exit at91_cf_remove(struct platform_device *pdev)
+static int at91_cf_remove(struct platform_device *pdev)
{
struct at91_cf_socket *cf = platform_get_drvdata(pdev);
@@ -404,14 +404,13 @@ static struct platform_driver at91_cf_driver = {
.owner = THIS_MODULE,
.of_match_table = of_match_ptr(at91_cf_dt_ids),
},
- .remove = __exit_p(at91_cf_remove),
+ .probe = at91_cf_probe,
+ .remove = at91_cf_remove,
.suspend= at91_cf_suspend,
.resume = at91_cf_resume,
};
-/*--*/
-
-module_platform_driver_probe(at91_cf_driver, at91_cf_probe);
+module_platform_driver(at91_cf_driver);
MODULE_DESCRIPTION("AT91 Compact Flash Driver");
MODULE_AUTHOR("David Brownell");
--
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[resolved] No keyboard and mouse usb detected at startup.
It looks like your system did not load the ohci-pci driver. Is CONFIG_USB_OHCI_HCD_PCI enabled? Thanks my problem was there. As I do not have PCI card (Peripheral Component Interconnect) I had not enabled this option thinking I didn't need that... Thank you very much for your help and for your work. -- Best regards, HacKurx www.hackurx.info -- To unsubscribe from this list: send the line "unsubscribe 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: Fix race between ep halt and URB cancellation
Thanks Florian. I was waiting for after the 3.12 merge window to close
to send Greg bug fixes. I'm queuing this up for test today, and will
send it out once it passes.
Sarah Sharp
On Sun, Sep 22, 2013 at 09:09:54PM +0200, Florian Wolter wrote:
> Hi Sarah,
>
> the described Problem also exisits on actual Linux Kernel 3.12-rc so I
> rebased my patch to this.
> --
>
>
> The halted state of a endpoint cannot be cleared over CLEAR_HALT from a
> user process, because the stopped_td variable was overwritten in the
> handle_stopped_endpoint() function. So the xhci_endpoint_reset() function
> will
> refuse the reset and communication with device can not run over this endpoint.
> https://bugzilla.kernel.org/show_bug.cgi?id=60699
>
>
> Signed-off-by: Florian Wolter
> ---
> drivers/usb/host/xhci-ring.c |8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index c47f90e..411da1f 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -859,8 +859,12 @@ remove_finished_td:
> /* Otherwise ring the doorbell(s) to restart queued transfers */
> ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
> }
> - ep->stopped_td = NULL;
> - ep->stopped_trb = NULL;
> +
> + /* Clear stopped_td and stopped_trb if endpoint is not halted */
> + if (!(ep->ep_state & EP_HALTED)) {
> + ep->stopped_td = NULL;
> + ep->stopped_trb = NULL;
> + }
>
> /*
>* Drop the lock and complete the URBs in the cancelled TD list.
> --
> 1.7.10.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: usbtest: bmAttributes would better be masked
On Tue, 24 Sep 2013, Huang Rui wrote:
> When transfer type is isochronous, the other bits (bits 5..2) of
> bmAttributes in endpoint descriptor might not be set zero. So it's better
> to use usb_endpoint_type routine to mask bmAttributes with
> USB_ENDPOINT_XFERTYPE_MASK to judge the transfter type later.
>
> Signed-off-by: Huang Rui
> ---
>
> Changes from v1 -> v2:
> - Use usb_endpoint_type routine instead of doing by hand.
>
> drivers/usb/misc/usbtest.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
> index aa28ac8..3e91d3e9 100644
> --- a/drivers/usb/misc/usbtest.c
> +++ b/drivers/usb/misc/usbtest.c
> @@ -120,7 +120,7 @@ get_endpoints(struct usbtest_dev *dev, struct
> usb_interface *intf)
> struct usb_host_endpoint*e;
>
> e = alt->endpoint + ep;
> - switch (e->desc.bmAttributes) {
> + switch (usb_endpoint_type(&e->desc)) {
> case USB_ENDPOINT_XFER_BULK:
> break;
> case USB_ENDPOINT_XFER_ISOC:
Acked-by: 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
[GIT PATCH] USB fixes for 3.12-rc2
The following changes since commit 272b98c6455f00884f0350f775c5342358ebb73f: Linux 3.12-rc1 (2013-09-16 16:17:51 -0400) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ tags/usb-3.12-rc2 for you to fetch changes up to 42f4891ca29a3c1535692a24acefb7015bbbc077: Merge tag 'fixes-for-v3.12-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-linus (2013-09-17 13:00:43 -0700) USB fixes for 3.12-rc2 Here are a number of small USB fixes for 3.12-rc2. One is a revert of a EHCI change that isn't quite ready for 3.12. Others are minor things, gadget fixes, Kconfig fixes, and some quirks and documentation updates. All have been in linux-next for a bit. Signed-off-by: Greg Kroah-Hartman Alan Stern (1): usb: gadget: fix a bug and a WARN_ON in dummy-hcd Alexey Khoroshilov (1): usb: gadget: mv_u3d_core: fix violation of locking discipline in mv_u3d_ep_disable() Andrzej Pietrasiewicz (1): usb: gadget: cdc2: fix conversion to new interface of f_ecm Chanho Park (1): usb: s3c-hsotg: do not disconnect gadget when receiving ErlySusp intr Chen Gang (1): usb: gadget: add '__ref' for rndis_config_register() and cdc_config_register() Dave Jones (2): USB: fix typo in usb serial simple driver Kconfig USB: Faraday fotg210: fix email addresses David Cohen (1): usb: dwc3: gadget: avoid memory leak when failing to allocate all eps Frank Schäfer (1): USB: pl2303: distinguish between original and cloned HX chips Greg Kroah-Hartman (2): Revert "USB: EHCI: support running URB giveback in tasklet context" Merge tag 'fixes-for-v3.12-rc2' of git://git.kernel.org/.../balbi/usb into usb-linus Heikki Krogerus (2): usb: dwc3: pci: add support for BayTrail usb: dwc3: remove extcon dependency Marek Szyprowski (1): usb: s3c-hsotg: fix unregistration function Peter Oh (1): usb: gadget: f_mass_storage: reset endpoint driver data when disabled Sachin Kamat (4): usb: phy: omap-usb3: Fix return value usb: gadget: f_ecm: Staticize ecm_alloc usb: gadget: f_eem: Staticize eem_alloc usb: host: fsl-mph-dr-of: Staticize local symbols drivers/usb/dwc3/Kconfig| 1 - drivers/usb/dwc3/dwc3-pci.c | 2 ++ drivers/usb/dwc3/gadget.c | 6 ++ drivers/usb/gadget/cdc2.c | 19 +--- drivers/usb/gadget/dummy_hcd.c | 7 +++--- drivers/usb/gadget/f_ecm.c | 2 +- drivers/usb/gadget/f_eem.c | 2 +- drivers/usb/gadget/f_mass_storage.c | 2 ++ drivers/usb/gadget/fotg210-udc.c| 2 +- drivers/usb/gadget/fusb300_udc.c| 2 +- drivers/usb/gadget/multi.c | 8 +++ drivers/usb/gadget/mv_u3d_core.c| 3 +++ drivers/usb/gadget/s3c-hsotg.c | 13 --- drivers/usb/host/ehci-fsl.c | 2 +- drivers/usb/host/ehci-grlib.c | 2 +- drivers/usb/host/ehci-hcd.c | 2 +- drivers/usb/host/ehci-mv.c | 2 +- drivers/usb/host/ehci-octeon.c | 2 +- drivers/usb/host/ehci-pmcmsp.c | 2 +- drivers/usb/host/ehci-ppc-of.c | 2 +- drivers/usb/host/ehci-ps3.c | 2 +- drivers/usb/host/ehci-q.c | 5 + drivers/usb/host/ehci-sead3.c | 2 +- drivers/usb/host/ehci-sh.c | 2 +- drivers/usb/host/ehci-tilegx.c | 2 +- drivers/usb/host/ehci-w90x900.c | 2 +- drivers/usb/host/ehci-xilinx-of.c | 2 +- drivers/usb/host/fsl-mph-dr-of.c| 6 +++--- drivers/usb/phy/phy-omap-usb3.c | 2 +- drivers/usb/serial/Kconfig | 2 +- drivers/usb/serial/pl2303.c | 43 +++-- 31 files changed, 81 insertions(+), 72 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: fix write to USB3_PSSEN and XUSB2PRM pci config registers
On Fri, Sep 20, 2013 at 07:45:53PM +0300, Xenia Ragiadakou wrote:
> The function pci_write_config_dword() sets the appropriate byteordering
> internally so the value argument should not be converted to little-endian.
> This bug was found by sparse.
Can you give the exact error or warning message that sparse gave?
I ask because this description sounded odd to Greg and I when we met
last week at LinuxCon North America. I've tried to track this down to
see where the code might be converting the value from CPU format to
little endian, and I don't see it.
AFAICT, pci_write_config_dword() is defined in include/linux/pci.h, and
calls pci_bus_write_config_dword():
static inline int pci_write_config_dword(const struct pci_dev *dev, int where,
u32 val)
{
return pci_bus_write_config_dword(dev->bus, dev->devfn, where, val);
}
pci_bus_write_config_dword is defined as a macro in drivers/pci/access.h:
#define PCI_OP_WRITE(size,type,len) \
int pci_bus_write_config_##size \
(struct pci_bus *bus, unsigned int devfn, int pos, type value) \
{ \
int res;\
unsigned long flags;\
if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
raw_spin_lock_irqsave(&pci_lock, flags);\
res = bus->ops->write(bus, devfn, pos, len, value); \
raw_spin_unlock_irqrestore(&pci_lock, flags); \
return res; \
}
That macro simply calls the write function for whatever PCI bus driver
is installed. Note that bus driver can be different than the standard
bus driver. I don't see any conversion to little endian here, so that
means each bus driver would have to convert it.
I can dig deeper into each .write function, but if the conversion isn't
done at the upper layers, it's possible someone will create a .write
function without the conversion to little endian.
Am I missing something?
>
> Signed-off-by: Xenia Ragiadakou
> ---
> drivers/usb/host/pci-quirks.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 2c76ef1..08ef282 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -799,7 +799,7 @@ void usb_enable_intel_xhci_ports(struct pci_dev
> *xhci_pdev)
>* switchable ports.
>*/
> pci_write_config_dword(xhci_pdev, USB_INTEL_USB3_PSSEN,
> - cpu_to_le32(ports_available));
> + ports_available);
>
> pci_read_config_dword(xhci_pdev, USB_INTEL_USB3_PSSEN,
> &ports_available);
> @@ -821,7 +821,7 @@ void usb_enable_intel_xhci_ports(struct pci_dev
> *xhci_pdev)
>* host.
>*/
> pci_write_config_dword(xhci_pdev, USB_INTEL_XUSB2PR,
> - cpu_to_le32(ports_available));
> + ports_available);
>
> pci_read_config_dword(xhci_pdev, USB_INTEL_XUSB2PR,
> &ports_available);
> --
> 1.8.3.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe 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 0/2] usb: implement AMD remote wakeup quirk
On Mon, 16 Sep 2013, Huang Rui wrote: > Hi Greg, Sarah, Alan, > > The following patches are required to resolve remote wake issues with > certain devices. > > Issue description: > If the remote wake is issued from the device in a specific timing > condition while the system is entering sleep state then it may cause > system to auto wake on subsequent sleep cycle. > > Root cause: > Host controller rebroadcasts the Resume signal > 100 µseconds after > receiving the original resume event from the device. For proper > function, some devices may require the rebroadcast of resume event > within the USB spec of 100µS. > > Reproduce steps: > 1. Enable remote wakeup for usb mouse. > 2. Execute S3. > 3. Click mouse _intensely_ (more than 10 times) to wake the system up. > 4. Then execute S3 again. > 5. Observe that the system auto wake up. > > [Q] Why the special devices are only mice? Would high speed devices > such as 3G modem or USB Bluetooth adapter trigger this issue? > - Current this sensitivity is only confined to devices that use Pixart > controllers. This controller is designed for use with LS mouse > devices only. We have not observed any other devices failing. There > may be a small risk for other devices also but this patch (reset > device in resume phase) will cover the cases if required. > > [Q] Shouldn’t the resume signal be sent within 100 us for every > device? > - The Host controller may not send the resume signal within 100us, > this our host controller specification change. This is why we > require the patch to prevent side effects on certain known devices. > > [Q] Why would clicking mouse INTENSELY to wake the system up trigger > this issue? > - This behavior is specific to the devices that use Pixart controller. > It is timing dependent on when the resume event is triggered during > the sleep state. > > [Q] Is it a host controller issue or mouse? > - It is the host controller behavior during resume that triggers the > device incorrect behavior on the next resume. > > - Patch 1 refactors AMD quirk to abstract chipset types. > - Patch 2 implements AMD remote wakeup quirk. > They are generated on gregkh/usb usb-next. > > Changes from v1 -> v2: > - Add reproduce steps. > - Add a patch to put the changes of core/hub.c and pci-quirks into one > patch. > - Remove all the split strings. > - Refactor usb_amd_remote_wakeup_quirk to save a variable and less > codes. > - Remove 10ms delay in spinlock during xhci reset port by mouse. > - Add an extra tab to algin to all quirks macros. > - Remove backport request. > - Add descriptions of the fixes and how they work. > - Summarized questions from Sarah and Alan. > > Changes from v2 -> v3: > - Split first patch into two, the one add all the issue USB mice with > vendor id and product id in core/quirk.c, the other one is for filtering > the special AMD platforms. > - Remove is_usb_mouse function, as Greg required that don't use device type > for filtering. > > Changes from v3 -> v4: > - Refactor AMD pci quirk to abstract out chipset types to list generation > with a enumeration type and revision as Greg's suggestion. > - Remove xhci and ohci level codes. > - Implement remote wakeup quirk at core level as Alan's suggestion. This version of the patch set looks good. 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: fix write to USB3_PSSEN and XUSB2PRM pci config registers
On 09/23/2013 07:45 PM, Sarah Sharp wrote:
On Fri, Sep 20, 2013 at 07:45:53PM +0300, Xenia Ragiadakou wrote:
The function pci_write_config_dword() sets the appropriate byteordering
internally so the value argument should not be converted to little-endian.
This bug was found by sparse.
Can you give the exact error or warning message that sparse gave?
Yes, sure.
drivers/usb/host/pci-quirks.c:802:25: warning: incorrect type in
argument 3 (different base types)
drivers/usb/host/pci-quirks.c:802:25:expected unsigned int
[unsigned] [usertype] val
drivers/usb/host/pci-quirks.c:802:25:got restricted __le32
[usertype]
drivers/usb/host/pci-quirks.c:824:25: warning: incorrect type in
argument 3 (different base types)
drivers/usb/host/pci-quirks.c:824:25:expected unsigned int
[unsigned] [usertype] val
drivers/usb/host/pci-quirks.c:824:25:got restricted __le32
[usertype]
I ask because this description sounded odd to Greg and I when we met
last week at LinuxCon North America. I've tried to track this down to
see where the code might be converting the value from CPU format to
little endian, and I don't see it.
AFAICT, pci_write_config_dword() is defined in include/linux/pci.h, and
calls pci_bus_write_config_dword():
static inline int pci_write_config_dword(const struct pci_dev *dev, int where,
u32 val)
{
return pci_bus_write_config_dword(dev->bus, dev->devfn, where, val);
}
pci_bus_write_config_dword is defined as a macro in drivers/pci/access.h:
#define PCI_OP_WRITE(size,type,len) \
int pci_bus_write_config_##size \
(struct pci_bus *bus, unsigned int devfn, int pos, type value) \
{ \
int res;\
unsigned long flags;\
if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
raw_spin_lock_irqsave(&pci_lock, flags);\
res = bus->ops->write(bus, devfn, pos, len, value); \
raw_spin_unlock_irqrestore(&pci_lock, flags); \
return res; \
}
That macro simply calls the write function for whatever PCI bus driver
is installed. Note that bus driver can be different than the standard
bus driver. I don't see any conversion to little endian here, so that
means each bus driver would have to convert it.
I can dig deeper into each .write function, but if the conversion isn't
done at the upper layers, it's possible someone will create a .write
function without the conversion to little endian.
Am I missing something?
I had in mind that the pci_ops .read and .write defined by the PCI
driver will take care of consistent byteorder access to the
configuration registers. At least, that was what i understood after
reading the
chapter on PCI of Linux Device Drivers (more specifically for
pci_write_config_* functions, it states that "The word and dword
functions convert the value to little-endian before writing to the
peripheral device.").
regards,
ksenia
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/pci-quirks.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 2c76ef1..08ef282 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -799,7 +799,7 @@ void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev)
* switchable ports.
*/
pci_write_config_dword(xhci_pdev, USB_INTEL_USB3_PSSEN,
- cpu_to_le32(ports_available));
+ ports_available);
pci_read_config_dword(xhci_pdev, USB_INTEL_USB3_PSSEN,
&ports_available);
@@ -821,7 +821,7 @@ void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev)
* host.
*/
pci_write_config_dword(xhci_pdev, USB_INTEL_XUSB2PR,
- cpu_to_le32(ports_available));
+ ports_available);
pci_read_config_dword(xhci_pdev, USB_INTEL_XUSB2PR,
&ports_available);
--
1.8.3.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe 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] usb: phy: nop: Don't use regulator framework for RESET line
Hi Felipe,
There is an issue with this patch and is shown below.
I will send a v3 for this.
On 08/15/2013 01:18 PM, Roger Quadros wrote:
> Modelling the RESET line as a regulator supply wasn't a good idea
> as it kind of abuses the regulator framework and also makes adaptation
> code more complex.
>
> Instead, manage the RESET gpio line directly in the driver. Update
> the device tree binding information.
>
> This also makes us easy to migrate to a dedicated GPIO RESET controller
> whenever it becomes available.
>
> Signed-off-by: Roger Quadros
> ---
> .../devicetree/bindings/usb/usb-nop-xceiv.txt |7 +-
> drivers/usb/phy/phy-nop.c | 71 ++-
> 2 files changed, 55 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> index d7e2726..1bd37fa 100644
> --- a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> @@ -15,7 +15,7 @@ Optional properties:
>
> - vcc-supply: phandle to the regulator that provides RESET to the PHY.
>
> -- reset-supply: phandle to the regulator that provides power to the PHY.
> +- reset-gpios: Should specify the GPIO for reset.
>
> Example:
>
> @@ -25,10 +25,9 @@ Example:
> clocks = <&osc 0>;
> clock-names = "main_clk";
> vcc-supply = <&hsusb1_vcc_regulator>;
> - reset-supply = <&hsusb1_reset_regulator>;
> + reset-gpios = <&gpio1 7 GPIO_ACTIVE_LOW>;
> };
>
> hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator
> and expects that clock to be configured to 19.2MHz by the NOP PHY driver.
> -hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator
> -controls RESET.
> +hsusb1_vcc_regulator provides power to the PHY and GPIO 7 controls RESET.
> diff --git a/drivers/usb/phy/phy-nop.c b/drivers/usb/phy/phy-nop.c
> index 55445e5d..1f88c32 100644
> --- a/drivers/usb/phy/phy-nop.c
> +++ b/drivers/usb/phy/phy-nop.c
> @@ -35,6 +35,9 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
>
> struct nop_usb_xceiv {
> struct usb_phy phy;
> @@ -42,6 +45,8 @@ struct nop_usb_xceiv {
> struct clk *clk;
> struct regulator *vcc;
> struct regulator *reset;
> + int gpio_reset;
> + bool reset_active_low;
> };
>
> static struct platform_device *pd;
> @@ -70,6 +75,23 @@ static int nop_set_suspend(struct usb_phy *x, int suspend)
> return 0;
> }
>
> +static void nop_reset_set(struct nop_usb_xceiv *nop, int asserted)
> +{
> + int value;
> +
> + if (!gpio_is_valid(nop->gpio_reset))
> + return;
> +
> + value = asserted;
> + if (nop->reset_active_low)
> + value = !value;
> +
> + gpio_set_value_cansleep(nop->gpio_reset, value);
> +
> + if (!asserted)
> + usleep_range(1, 2);
> +}
> +
> static int nop_init(struct usb_phy *phy)
> {
> struct nop_usb_xceiv *nop = dev_get_drvdata(phy->dev);
> @@ -82,11 +104,8 @@ static int nop_init(struct usb_phy *phy)
> if (!IS_ERR(nop->clk))
> clk_enable(nop->clk);
>
> - if (!IS_ERR(nop->reset)) {
> - /* De-assert RESET */
> - if (regulator_enable(nop->reset))
> - dev_err(phy->dev, "Failed to de-assert reset\n");
> - }
> + /* De-assert RESET */
> + nop_reset_set(nop, 0);
>
> return 0;
> }
> @@ -95,11 +114,8 @@ static void nop_shutdown(struct usb_phy *phy)
> {
> struct nop_usb_xceiv *nop = dev_get_drvdata(phy->dev);
>
> - if (!IS_ERR(nop->reset)) {
> - /* Assert RESET */
> - if (regulator_disable(nop->reset))
> - dev_err(phy->dev, "Failed to assert reset\n");
> - }
> + /* Assert RESET */
> + nop_reset_set(nop, 1);
>
> if (!IS_ERR(nop->clk))
> clk_disable(nop->clk);
> @@ -148,7 +164,6 @@ static int nop_usb_xceiv_probe(struct platform_device
> *pdev)
> int err;
> u32 clk_rate = 0;
> bool needs_vcc = false;
> - bool needs_reset = false;
>
> nop = devm_kzalloc(&pdev->dev, sizeof(*nop), GFP_KERNEL);
> if (!nop)
> @@ -159,20 +174,28 @@ static int nop_usb_xceiv_probe(struct platform_device
> *pdev)
> if (!nop->phy.otg)
> return -ENOMEM;
>
> + nop->reset_active_low = true; /* default behaviour */
> +
> if (dev->of_node) {
> struct device_node *node = dev->of_node;
> + enum of_gpio_flags flags;
>
> if (of_property_read_u32(node, "clock-frequency", &clk_rate))
> clk_rate = 0;
>
> needs_vcc = of_property_read_bool(node, "vcc-supply");
> - needs_reset = of_property_read_bool(node, "reset-supply");
> + nop->gpio_reset = of_get_named_gpio_flags(node
Re: [PATCH 4/4] RX-51: Add platform function and data for bq24150a charger
* Pali Rohár [130920 12:29]:
> On Sunday 08 September 2013 10:50:39 Pali Rohár wrote:
> > This patch will register bq24150a charger in RX-51 board data.
> > Patch also adding platform function between isp1704 and
> > bq2415x drivers for detecting charger type.
> >
> > So finally charging battery on Nokia N900 (RX-51) working
> > automatically without any proprietary Nokia bits in userspace.
...
> > @@ -277,6 +316,7 @@ static void rx51_charger_set_power(bool
> > on)
> >
> > static struct isp1704_charger_data rx51_charger_data = {
> > .set_power = rx51_charger_set_power,
> > + .set_current= rx51_charger_set_current,
> > };
We want to get rid of the platform data callbacks here,
there no longer any need to keep these under arch/arm.
> Tony, can you look and review this board patch?
Yes, looks like this can all be done in the driver nowadays.
You can use drivers/reset for the set_power. Or if it's really
controlling the regulator, then the regulator framework. The
info can be passed in a .dts file for those.
The .set_current you can do in the driver based on the
compatible flag.
> I think that this patch series it the most important for Nokia
> N900, because it finally bringing charging support. And without
> charging battery it very hard to use phone which has power supply
> only from battery.
Right, let's get this driver updated to use the device tree
based init and that way this file is no longer needed.
I would like to $ grep -i grandmom ~/.phonebook | call too :)
I forgot how this charger is wired up, but maybe take a
look at commit d7bf353f (bq24190_charger: Add support for TI
BQ24190 Battery Charger) for the DT parts.
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
ehci-pci D3cold logspam
I've been getting this on several recent kernel revisions. Is it interesting? If so, I'm happy to help diagnose it. If not, can the message be killed or severely ratelimited? I'm getting so much of this that it tends to overflow the log ring. [ 287.344991] ehci-pci :00:1d.0: power state changed by ACPI to D0 [ 287.445433] ehci-pci :00:1d.0: setting latency timer to 64 [ 287.446255] ehci-pci :00:1a.0: power state changed by ACPI to D0 [ 287.456094] ehci-pci :00:1d.0: power state changed by ACPI to D3cold [ 287.547205] ehci-pci :00:1a.0: setting latency timer to 64 [ 287.557890] ehci-pci :00:1a.0: power state changed by ACPI to D3cold [ 290.126910] ehci-pci :00:1d.0: power state changed by ACPI to D0 [ 290.227958] ehci-pci :00:1d.0: setting latency timer to 64 [ 290.228416] ehci-pci :00:1a.0: power state changed by ACPI to D0 [ 290.238749] ehci-pci :00:1d.0: power state changed by ACPI to D3cold [ 290.328904] ehci-pci :00:1a.0: setting latency timer to 64 [ 290.339565] ehci-pci :00:1a.0: power state changed by ACPI to D3cold [ 292.214834] ehci-pci :00:1d.0: power state changed by ACPI to D0 [ 292.315458] ehci-pci :00:1d.0: setting latency timer to 64 [ 292.315908] ehci-pci :00:1a.0: power state changed by ACPI to D0 [ 292.326262] ehci-pci :00:1d.0: power state changed by ACPI to D3cold [ 292.416487] ehci-pci :00:1a.0: setting latency timer to 64 [ 292.431075] ehci-pci :00:1a.0: power state changed by ACPI to D3cold [ 295.458048] ehci-pci :00:1d.0: power state changed by ACPI to D0 [ 295.558613] ehci-pci :00:1d.0: setting latency timer to 64 -- To unsubscribe from this list: send the line "unsubscribe 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 36/51] DMA-API: usb: use dma_set_coherent_mask()
On Mon, Sep 23, 2013 at 02:27:39PM -0400, Alan Stern wrote: > On Thu, 19 Sep 2013, Russell King wrote: > > > The correct way for a driver to specify the coherent DMA mask is > > not to directly access the field in the struct device, but to use > > dma_set_coherent_mask(). Only arch and bus code should access this > > member directly. > > > > Convert all direct write accesses to using the correct API. > > > > Signed-off-by: Russell King > > > diff --git a/drivers/usb/host/ehci-platform.c > > b/drivers/usb/host/ehci-platform.c > > index f6b790c..5b0cd2d 100644 > > --- a/drivers/usb/host/ehci-platform.c > > +++ b/drivers/usb/host/ehci-platform.c > > > @@ -91,8 +91,9 @@ static int ehci_platform_probe(struct platform_device > > *dev) > > dev->dev.platform_data = &ehci_platform_defaults; > > if (!dev->dev.dma_mask) > > dev->dev.dma_mask = &dev->dev.coherent_dma_mask; > > - if (!dev->dev.coherent_dma_mask) > > - dev->dev.coherent_dma_mask = DMA_BIT_MASK(32); > > + err = dma_set_coherent_mask(&dev->dev, DMA_BIT_MASK(32)); > > + if (err) > > + return err; > > > > pdata = dev_get_platdata(&dev->dev); > > ehci-platform.c is a generic file, intended for use by multiple > platforms. Is it not possible that on some platforms, the arch or bus > code may already have initialized the DMA masks? Isn't something like > this (i.e., DMA bindings) planned for Device Tree? > > By eliminating the tests above and calling dma_set_coherent_mask or > dma_coerce_mask_and_coherent unconditionally, this patch (and the next) > would overwrite those initial settings. > > I don't know to what extent the same may be true for the other, > platform-specific, drivers changed by this patch. But it's something > to be aware of. Please check the DMA API documentation: = For correct operation, you must interrogate the kernel in your device probe routine to see if the DMA controller on the machine can properly support the DMA addressing limitation your device has. It is good style to do this even if your device holds the default setting, because this shows that you did think about these issues wrt. your device. The query is performed via a call to dma_set_mask(): int dma_set_mask(struct device *dev, u64 mask); The query for consistent allocations is performed via a call to dma_set_coherent_mask(): int dma_set_coherent_mask(struct device *dev, u64 mask); = So, All drivers which use DMA are supposed to issue the appropriate calls to check the DMA masks before they perform DMA, even if the "default" is correct. And yes, this is all part of sorting out the DT mess - we should start not from the current mess (which is really totally haphazard) but from the well established position of how the DMA API _should_ be used. What that means is that all drivers should be issuing these calls as appropriate today. The default mask setup when the device is created is just that - it's a default mask, and it should not be used to decide anything about the device. That's something which the driver should compute on its own accord and then inform the various other layers via the appropriate call. Remember, on PCI, even when we have 64-bit, we do not declare PCI devices with a 64-bit DMA mask: that's up to PCI device drivers to _try_ to set a 64-bit DMA mask, which when successful _allows_ them to use 64-bit DMA. If it fails, they have to only use 32-bit. If they want a smaller mask, the _driver_ has to set the smaller mask, not the device creating code. The reason we're into this (particularly on ARM) is that we got lazy because we could get by with declaring a DMA mask at device creation time, since all devices were statically declared. Now it's time to get rid of those old lazy ways and start doing things correctly and to the requirements of the APIs. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ehci-pci D3cold logspam
On Mon, 23 Sep 2013, Andy Lutomirski wrote: > I've been getting this on several recent kernel revisions. Is it > interesting? If so, I'm happy to help diagnose it. If not, can the > message be killed or severely ratelimited? I'm getting so much of > this that it tends to overflow the log ring. It's interesting only if you care about when your EHCI controllers get resumed and suspended. In this case, it's not clear why the transitions happen so rapidly. It looks like some sort of polling is going on at roughly 2-second intervals. > [ 287.344991] ehci-pci :00:1d.0: power state changed by ACPI to D0 > [ 287.445433] ehci-pci :00:1d.0: setting latency timer to 64 > [ 287.446255] ehci-pci :00:1a.0: power state changed by ACPI to D0 > [ 287.456094] ehci-pci :00:1d.0: power state changed by ACPI to D3cold > [ 287.547205] ehci-pci :00:1a.0: setting latency timer to 64 > [ 287.557890] ehci-pci :00:1a.0: power state changed by ACPI to D3cold > [ 290.126910] ehci-pci :00:1d.0: power state changed by ACPI to D0 > [ 290.227958] ehci-pci :00:1d.0: setting latency timer to 64 > [ 290.228416] ehci-pci :00:1a.0: power state changed by ACPI to D0 > [ 290.238749] ehci-pci :00:1d.0: power state changed by ACPI to D3cold > [ 290.328904] ehci-pci :00:1a.0: setting latency timer to 64 > [ 290.339565] ehci-pci :00:1a.0: power state changed by ACPI to D3cold > [ 292.214834] ehci-pci :00:1d.0: power state changed by ACPI to D0 > [ 292.315458] ehci-pci :00:1d.0: setting latency timer to 64 > [ 292.315908] ehci-pci :00:1a.0: power state changed by ACPI to D0 > [ 292.326262] ehci-pci :00:1d.0: power state changed by ACPI to D3cold > [ 292.416487] ehci-pci :00:1a.0: setting latency timer to 64 > [ 292.431075] ehci-pci :00:1a.0: power state changed by ACPI to D3cold > [ 295.458048] ehci-pci :00:1d.0: power state changed by ACPI to D0 > [ 295.558613] ehci-pci :00:1d.0: setting latency timer to 64 This question should be addressed to the PCI mailing list (cc'ed), as those two messages are generated by drivers/pci/pci-acpi.c:acpi_pci_set_power_state() and drivers/pci/pci.c:pcibios_set_master() respectively. 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
Mx28 USB workaround for errata
Hi,
I had a lot of USB errors on i.mx28. I could reproduce it most easily
with USB-serial adapters, and USB modems (CDC and WWAN drivers). If I
sent some data to the TTY port with the echo command in a loop (so
basically open the port, send data, and close the port), all of the
devices were dropped down from the root hub. (Later I found, it is
enough to just periodically open and close the TTY port.)
The reason was an mx28 errata: "ENGR119653 USB: ARM to USB register
error issue"
Freescale issued a patch for 2.6.35 to workaround this problem last
year. I ported this patch. However, it is not totally "device tree
compatible". I mean, this patch is only needed for mx28, not for the
other SOCs using chipidea, and I used ifdefs. So you can't compile a
kernel both for mx28 and e.g. for mx23. If you check the patch, you see,
that it is modifying the lowest layer, the writel() function; it is
changing it to an SWP instruction. According the errata, I need to use
SWP instructions to write USB registers.
So I'm curious, what other possibilities do I have to make this patch
device tree compatible? I mean, I can't check the machine's type on each
USB register writing. Then I could use function pointers for register
writing function, or weak aliases somehow. But that would causes an
overhead because of the additional function calling, instead of simple
inline assembly codes.
What's the standard way in this case? Or should I just leave the patch
as it is?
Robert Hodaszi
From de71990cd37a973a598ba8924d415696ea5dabdf Mon Sep 17 00:00:00 2001
From: Robert Hodaszi
Date: Mon, 23 Sep 2013 18:07:18 +0200
Subject: [PATCH] usb: chipidea: implement workaround for i.mx28 errata
ENGR119653: USB: ARM to USB register error issue
From the i.mx28 Errata:
ENGR119653 USB: ARM to USB register error issue
Description:
The ARM writes a data error to the USB core register unless SRM SWP
instruction is used.
The issue occurs when all of the following conditions are met:
1. Last AHB access is to the non-USB AHB slave
2. Current AHB access is to the USB
3. These two accesses are back-to-back
4. The last data phase of the last AHB access has a wait state
5. Only happens when D-cache is enabled
Projected Impact:
The USB register does not get correct data when writing to the USB slave
through the AHB bus
when D-cache is enabled.
Workaround:
All USB register write operations must use the ARM SWP instruction.
Projected Solution:
No fix scheduled.
Signed-off-by: Robert Hodaszi
---
drivers/usb/chipidea/ci.h | 13 +
drivers/usb/host/ehci.h |9 +
2 files changed, 22 insertions(+)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 1c94fc5..1ba7428 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -25,6 +25,19 @@
#define CI_HDRC_PAGE_SIZE 4096ul /* page size for TD's */
#define ENDPT_MAX 32
+#ifdef CONFIG_SOC_IMX28
+/* Fix errata ENGR119653 for i.mx28 */
+static inline void safe_writel(u32 val32, volatile u32 *addr)
+{
+ __asm__ ("swp %0, %0, [%1]" : : "r"(val32), "r"(addr));
+}
+
+#undef iowrite32
+#undef writel
+#define iowrite32(v, addr) safe_writel(v, addr)
+#define writel(v, addr) safe_writel(v, addr)
+#endif
+
/**
* STRUCTURES
*/
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 7c978b2..34dbec0 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -675,6 +675,13 @@ static inline unsigned int ehci_readl(const struct
ehci_hcd *ehci,
#endif
}
+#ifdef CONFIG_SOC_IMX28
+static inline void fsl_safe_writel(u32 val32, volatile u32 *addr)
+{
+ __asm__ ("swp %0, %0, [%1]" : : "r"(val32), "r"(addr));
+}
+#endif
+
static inline void ehci_writel(const struct ehci_hcd *ehci,
const unsigned int val, __u32 __iomem *regs)
{
@@ -682,6 +689,8 @@ static inline void ehci_writel(const struct ehci_hcd
*ehci,
ehci_big_endian_mmio(ehci) ?
writel_be(val, regs) :
writel(val, regs);
+#elif defined(CONFIG_SOC_IMX28)
+ fsl_safe_writel(val, regs);
#else
writel(val, regs);
#endif
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc3: gadget: don't request IRQs in atomic
mainline commit b0d7ffd44ba9cd2dfbf299674418193a5f9ed21a
We cannot request an IRQ with spinlocks held
as that would trigger a sleeping inside
spinlock warning.
Cc: # v3.10 v3.11
Reported-by: Stephen Boyd
Signed-off-by: Felipe Balbi
---
Paul reported that this patch wasn't on v3.11 so I cherry-picked
on top of v3.11.1 and it applies cleanly.
Let me know if you have issues applying to that tree and
I'll fix it up.
drivers/usb/dwc3/gadget.c | 39 ++-
1 file changed, 22 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index f77083f..14d28d6 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1508,6 +1508,15 @@ static int dwc3_gadget_start(struct usb_gadget *g,
int irq;
u32 reg;
+ irq = platform_get_irq(to_platform_device(dwc->dev), 0);
+ ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt,
+ IRQF_SHARED | IRQF_ONESHOT, "dwc3", dwc);
+ if (ret) {
+ dev_err(dwc->dev, "failed to request irq #%d --> %d\n",
+ irq, ret);
+ goto err0;
+ }
+
spin_lock_irqsave(&dwc->lock, flags);
if (dwc->gadget_driver) {
@@ -1515,7 +1524,7 @@ static int dwc3_gadget_start(struct usb_gadget *g,
dwc->gadget.name,
dwc->gadget_driver->driver.name);
ret = -EBUSY;
- goto err0;
+ goto err1;
}
dwc->gadget_driver = driver;
@@ -1551,42 +1560,38 @@ static int dwc3_gadget_start(struct usb_gadget *g,
ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL, false);
if (ret) {
dev_err(dwc->dev, "failed to enable %s\n", dep->name);
- goto err0;
+ goto err2;
}
dep = dwc->eps[1];
ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL, false);
if (ret) {
dev_err(dwc->dev, "failed to enable %s\n", dep->name);
- goto err1;
+ goto err3;
}
/* begin to receive SETUP packets */
dwc->ep0state = EP0_SETUP_PHASE;
dwc3_ep0_out_start(dwc);
- irq = platform_get_irq(to_platform_device(dwc->dev), 0);
- ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt,
- IRQF_SHARED | IRQF_ONESHOT, "dwc3", dwc);
- if (ret) {
- dev_err(dwc->dev, "failed to request irq #%d --> %d\n",
- irq, ret);
- goto err1;
- }
-
dwc3_gadget_enable_irq(dwc);
spin_unlock_irqrestore(&dwc->lock, flags);
return 0;
-err1:
+err3:
__dwc3_gadget_ep_disable(dwc->eps[0]);
-err0:
+err2:
dwc->gadget_driver = NULL;
+
+err1:
spin_unlock_irqrestore(&dwc->lock, flags);
+ free_irq(irq, dwc);
+
+err0:
return ret;
}
@@ -1600,9 +1605,6 @@ static int dwc3_gadget_stop(struct usb_gadget *g,
spin_lock_irqsave(&dwc->lock, flags);
dwc3_gadget_disable_irq(dwc);
- irq = platform_get_irq(to_platform_device(dwc->dev), 0);
- free_irq(irq, dwc);
-
__dwc3_gadget_ep_disable(dwc->eps[0]);
__dwc3_gadget_ep_disable(dwc->eps[1]);
@@ -1610,6 +1612,9 @@ static int dwc3_gadget_stop(struct usb_gadget *g,
spin_unlock_irqrestore(&dwc->lock, flags);
+ irq = platform_get_irq(to_platform_device(dwc->dev), 0);
+ free_irq(irq, dwc);
+
return 0;
}
--
1.8.3.4.840.g6a90778
--
To unsubscribe from this list: send the line "unsubscribe 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 LLVMLinux: Removing the use of VLAIS from the gadget driver
Hi, On Thu, Sep 05, 2013 at 08:07:11PM -0400, Behan Webster wrote: > Replying to my patch email just in case it was missed before. I remember there was a discussion of not dropping variable length array support, wasn't there ? -- balbi signature.asc Description: Digital signature
Re: [PATCH v4 1/3] usb: dwc3: msm: Add device tree binding information
Hi,
On Tue, Aug 20, 2013 at 12:56:03PM +0300, Ivan T. Ivanov wrote:
> From: "Ivan T. Ivanov"
>
> MSM USB3.0 core wrapper consist of USB3.0 IP from Synopsys
> (SNPS) and HS, SS PHY's control and configuration registers.
>
> It could operate in device mode (SS, HS, FS) and host
> mode (SS, HS, FS, LS).
>
> Signed-off-by: Ivan T. Ivanov
Any acks for the DT part ? This patch has been pending forever.
> ---
> .../devicetree/bindings/usb/msm-ssusb.txt | 104
>
> 1 file changed, 104 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/msm-ssusb.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/msm-ssusb.txt
> b/Documentation/devicetree/bindings/usb/msm-ssusb.txt
> new file mode 100644
> index 000..cacbd3b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/msm-ssusb.txt
> @@ -0,0 +1,104 @@
> +MSM SuperSpeed DWC3 USB SoC controller
> +
> +
> +DWC3 Highspeed USB PHY
> +==
> +Required properities :
> +- compatible : sould be "qcom,dwc3-hsphy";
> +- reg : offset and length of the register set in the memory map
> +- clocks : phandles to clock instances of the device tree nodes
> +- clock-names :
> + "xo" : External reference clock 19 MHz
> + "sleep_a" : Sleep clock, used when USB3 core goes into low
> + power mode (U3).
> +-supply : phandle to the regulator device tree node
> +Required "supply-name" are:
> + "v1p8" : 1.8v supply for HSPHY
> + "v3p3" : 3.3v supply for HSPHY
> + "vbus" : vbus supply for host mode
> + "vddcx" : vdd supply for HS-PHY digital circuit operation
> +
> +DWC3 Superspeed USB PHY
> +===
> +Required properities :
> +- compatible : sould be "qcom,dwc3-ssphy";
> +- reg : offset and length of the register set in the memory map
> +- clocks : phandles to clock instances of the device tree nodes
> +- clock-names :
> + "xo" : External reference clock 19 MHz
> + "ref" : Reference clock - used in host mode.
> +-supply : phandle to the regulator device tree node
> +Required "supply-name" are:
> + "v1p8" : 1.8v supply for SS-PHY
> + "vddcx" : vdd supply for SS-PHY digital circuit operation
> +
> +DWC3 controller wrapper
> +===
> +Required properties :
> +- compatible : should be "qcom,dwc3"
> +- reg : offset and length of the register set in the memory map
> + offset and length of the TCSR register for routing USB
> + signals to either picoPHY0 or picoPHY1.
> +- clocks : phandles to clock instances of the device tree nodes
> +- clock-names :
> + "core" : Master/Core clock, have to be >= 125 MHz for SS
> + operation and >= 60MHz for HS operation
> + "iface" : System bus AXI clock
> + "sleep" : Sleep clock, used when USB3 core goes into low
> + power mode (U3).
> + "utmi" : Generated by HS-PHY. Used to clock the low power
> + parts of thr HS Link layer.
> +Optional properties :
> +- gdsc-supply : phandle to the globally distributed switch controller
> + regulator node to the USB controller.
> +Required child node:
> +A child node must exist to represent the core DWC3 IP block. The name of
> +the node is not important. The content of the node is defined in dwc3.txt.
> +
> +Example device nodes:
> +
> + dwc3_hsphy: phy@f92f8800 {
> + compatible = "qcom,dwc3-hsphy";
> + reg = <0xf92f8800 0x30>;
> +
> + clocks = <&cxo>, <&usb2a_phy_sleep_cxc>;
> + clock-names = "xo", "sleep_a";
> +
> + vbus-supply = <&supply>;
> + vddcx-supply = <&supply>;
> + v1p8-supply = <&supply>;
> + v3p3-supply = <&supply>;
> + };
> +
> + dwc3_ssphy: phy@f92f8830 {
> + compatible = "qcom,dwc3-ssphy";
> + reg = <0xf92f8830 0x30>;
> +
> + clocks = <&cxo>, <&usb30_mock_utmi_cxc>;
> + clock-names = "xo", "ref";
> +
> + vddcx-supply = <&supply>;
> + v1p8-supply = <&supply>;
> + };
> +
> + usb@fd4ab000 {
> + compatible = "qcom,dwc3";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + reg = <0xfd4ab000 0x4>;
> +
> + clocks = <&usb30_master_cxc>, <&sys_noc_usb3_axi_cxc>,
> + <&usb30_sleep_cxc>, <&usb30_mock_utmi_cxc>;
> + clock-names = "core", "iface", "sleep", "utmi";
> +
> + gdsc-supply = <&supply>;
> +
> + ranges;
> + dwc3@f920 {
> + compatible = "snps,dwc3";
> + reg = <0xf920 0xcd00>;
> + interrupts = <0 131 0>;
> + usb-phy = <&dwc3_hsphy>, <&dwc3_ssphy>;
> + tx-fifo-resize;
> + };
> + };
> --
> 1.7.9.5
>
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v5 2/3] usb: phy: Add Qualcomm SS-USB and HS-USB drivers for DW PHY's
Hi, On Wed, Aug 21, 2013 at 04:29:45PM +0300, Ivan T. Ivanov wrote: > From: "Ivan T. Ivanov" > > These drivers handles control and configuration of the HS > and SS USB PHY transceivers. They are part of the driver > which manage Synopsys DesignWare USB3 controller stack > inside Qualcomm SoC's. > > Signed-off-by: Ivan T. Ivanov I can take this one if DT folks agree with bindings proposed on previous patch. -- balbi signature.asc Description: Digital signature
Re: [PATCH v5 1/3] usb: dwc3: msm: Add device tree binding information
Hi,
On Wed, Aug 21, 2013 at 04:29:44PM +0300, Ivan T. Ivanov wrote:
> From: "Ivan T. Ivanov"
>
> MSM USB3.0 core wrapper consist of USB3.0 IP from Synopsys
> (SNPS) and HS, SS PHY's control and configuration registers.
>
> It could operate in device mode (SS, HS, FS) and host
> mode (SS, HS, FS, LS).
>
> Signed-off-by: Ivan T. Ivanov
and here's a new version from same patch
> ---
> .../devicetree/bindings/usb/msm-ssusb.txt | 104
>
> 1 file changed, 104 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/msm-ssusb.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/msm-ssusb.txt
> b/Documentation/devicetree/bindings/usb/msm-ssusb.txt
> new file mode 100644
> index 000..f57ba8d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/msm-ssusb.txt
> @@ -0,0 +1,104 @@
> +MSM SuperSpeed DWC3 USB SoC controller
> +
> +
> +MSM DW Highspeed USB PHY
> +
> +Required properities :
> +- compatible : sould be "qcom,dw-hsphy";
> +- reg : offset and length of the register set in the memory map
> +- clocks : phandles to clock instances of the device tree nodes
> +- clock-names :
> + "xo" : External reference clock 19 MHz
> + "sleep_a" : Sleep clock, used when USB3 core goes into low
> + power mode (U3).
> +-supply : phandle to the regulator device tree node
> +Required "supply-name" are:
> + "v1p8" : 1.8v supply for HSPHY
> + "v3p3" : 3.3v supply for HSPHY
> + "vbus" : vbus supply for host mode
> + "vddcx" : vdd supply for HS-PHY digital circuit operation
> +
> +MSM DW Superspeed USB PHY
> +=
> +Required properities :
> +- compatible : sould be "qcom,dw-ssphy";
> +- reg : offset and length of the register set in the memory map
> +- clocks : phandles to clock instances of the device tree nodes
> +- clock-names :
> + "xo" : External reference clock 19 MHz
> + "ref" : Reference clock - used in host mode.
> +-supply : phandle to the regulator device tree node
> +Required "supply-name" are:
> + "v1p8" : 1.8v supply for SS-PHY
> + "vddcx" : vdd supply for SS-PHY digital circuit operation
> +
> +MSM DWC3 controller wrapper
> +===
> +Required properties :
> +- compatible : should be "qcom,dwc3"
> +- reg : offset and length of the register set in the memory map
> + offset and length of the TCSR register for routing USB
> + signals to either picoPHY0 or picoPHY1.
> +- clocks : phandles to clock instances of the device tree nodes
> +- clock-names :
> + "core" : Master/Core clock, have to be >= 125 MHz for SS
> + operation and >= 60MHz for HS operation
> + "iface" : System bus AXI clock
> + "sleep" : Sleep clock, used when USB3 core goes into low
> + power mode (U3).
> + "utmi" : Generated by HS-PHY. Used to clock the low power
> + parts of thr HS Link layer.
> +Optional properties :
> +- gdsc-supply : phandle to the globally distributed switch controller
> + regulator node to the USB controller.
> +Required child node:
> +A child node must exist to represent the core DWC3 IP block. The name of
> +the node is not important. The content of the node is defined in dwc3.txt.
> +
> +Example device nodes:
> +
> + dw_hsphy: phy@f92f8800 {
> + compatible = "qcom,dw-hsphy";
> + reg = <0xf92f8800 0x30>;
> +
> + clocks = <&cxo>, <&usb2a_phy_sleep_cxc>;
> + clock-names = "xo", "sleep_a";
> +
> + vbus-supply = <&supply>;
> + vddcx-supply = <&supply>;
> + v1p8-supply = <&supply>;
> + v3p3-supply = <&supply>;
> + };
> +
> + dw_ssphy: phy@f92f8830 {
> + compatible = "qcom,dw-ssphy";
> + reg = <0xf92f8830 0x30>;
> +
> + clocks = <&cxo>, <&usb30_mock_utmi_cxc>;
> + clock-names = "xo", "ref";
> +
> + vddcx-supply = <&supply>;
> + v1p8-supply = <&supply>;
> + };
> +
> + usb@fd4ab000 {
> + compatible = "qcom,dwc3";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + reg = <0xfd4ab000 0x4>;
> +
> + clocks = <&usb30_master_cxc>, <&sys_noc_usb3_axi_cxc>,
> + <&usb30_sleep_cxc>, <&usb30_mock_utmi_cxc>;
> + clock-names = "core", "iface", "sleep", "utmi";
> +
> + gdsc-supply = <&supply>;
> +
> + ranges;
> + dwc3@f920 {
> + compatible = "snps,dwc3";
> + reg = <0xf920 0xcd00>;
> + interrupts = <0 131 0>;
> + usb-phy = <&dw_hsphy>, <&dw_ssphy>;
> + tx-fifo-resize;
> + };
> + };
> --
> 1.7.9.5
>
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v6 8/8] ARM: dts: omap5: update omap-control-usb node
On Mon, Aug 26, 2013 at 06:08:33PM +0300, Roger Quadros wrote: > Split USB2 PHY and USB3 PHY into separate omap-control-usb > nodes. Get rid of "ti,type" property. > > CC: Benoit Cousson > Signed-off-by: Roger Quadros Benoit ? ping here too... -- balbi signature.asc Description: Digital signature
Re: Mx28 USB workaround for errata
On Mon, 23 Sep 2013, Robert Hodaszi wrote: > Hi, > > I had a lot of USB errors on i.mx28. I could reproduce it most easily > with USB-serial adapters, and USB modems (CDC and WWAN drivers). If I > sent some data to the TTY port with the echo command in a loop (so > basically open the port, send data, and close the port), all of the > devices were dropped down from the root hub. (Later I found, it is > enough to just periodically open and close the TTY port.) > > The reason was an mx28 errata: "ENGR119653 USB: ARM to USB register > error issue" > > Freescale issued a patch for 2.6.35 to workaround this problem last > year. I ported this patch. However, it is not totally "device tree > compatible". I mean, this patch is only needed for mx28, not for the > other SOCs using chipidea, and I used ifdefs. So you can't compile a > kernel both for mx28 and e.g. for mx23. If you check the patch, you see, > that it is modifying the lowest layer, the writel() function; it is > changing it to an SWP instruction. According the errata, I need to use > SWP instructions to write USB registers. Presumably you _could_ use the SWP instructions for other SOCs, but they would slow down the driver. Right? > So I'm curious, what other possibilities do I have to make this patch > device tree compatible? I mean, I can't check the machine's type on each > USB register writing. Actually you can. See the definition of ehci_writel for an example. > Then I could use function pointers for register > writing function, or weak aliases somehow. But that would causes an > overhead because of the additional function calling, instead of simple > inline assembly codes. > > What's the standard way in this case? Or should I just leave the patch > as it is? Let's see what the Freescale and ChipIdea maintainers suggest. 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 LLVMLinux: Removing the use of VLAIS from the gadget driver
On 09/23/13 14:30, Felipe Balbi wrote: Hi, On Thu, Sep 05, 2013 at 08:07:11PM -0400, Behan Webster wrote: Replying to my patch email just in case it was missed before. I remember there was a discussion of not dropping variable length array support, wasn't there ? There was mostly an objection to the implementation of that patch. The new patch follows the advice given by the linux-usb list members at that time. There is increasing interest from various kernel devs (including maintainers and above) to be able to use clang to compile the Linux kernel. The removal of the use of VLAIS in the kernel is a step in allowing that to happen. Andrzej Pietrasiewicz attended the LLVM microconference at Plumbers last week and accepted the USB gadget patch. Thanks, Behan -- Behan Webster beh...@converseincode.com -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget LLVMLinux: Removing the use of VLAIS from the gadget driver
On Mon, Sep 23, 2013 at 12:30 PM, Felipe Balbi wrote: > > I remember there was a discussion of not dropping variable length array > support, wasn't there ? We should definitely drop it. The feature is an abomination. I thought gcc only allowed them at the end of structs, in the middle of a struct it's just f*cking insane beyond belief. That said, for *this* particular case, that USB widget driver already does a ton of small kmalloc's and then remembers the addresses independently. People may not care about performance, but it *might* be a good idea to just do one kmalloc()/kfree(), and then still have those pointer variables, but just be offsets within that one allocation. That's what gcc has to basically do for that thing internally *anyway*, just hidden behind a horrible construct that should never have existed. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 7/8] ARM: dts: omap4: update omap-control-usb nodes
Hi, On Mon, Aug 26, 2013 at 06:08:32PM +0300, Roger Quadros wrote: > Split otghs_ctrl and USB2 PHY power down into separate > omap-control-usb nodes. Get rid of "ti,type" property. > > Also get rid of "ti,has-mailbox" property from usb_otg_hs > node and provide the ctrl-module phandle. > > CC: Benoit Cousson > Signed-off-by: Roger Quadros Benoit ? Any comments here ? -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: gadget LLVMLinux: Removing the use of VLAIS from the gadget driver
On 09/23/13 15:08, Linus Torvalds wrote: On Mon, Sep 23, 2013 at 12:30 PM, Felipe Balbi wrote: I remember there was a discussion of not dropping variable length array support, wasn't there ? We should definitely drop it. The feature is an abomination. I thought gcc only allowed them at the end of structs, in the middle of a struct it's just f*cking insane beyond belief. That said, for *this* particular case, that USB widget driver already does a ton of small kmalloc's and then remembers the addresses independently. People may not care about performance, but it *might* be a good idea to just do one kmalloc()/kfree(), and then still have those pointer variables, but just be offsets within that one allocation. That's what gcc has to basically do for that thing internally *anyway*, just hidden behind a horrible construct that should never have existed. We can certainly do that instead. I believe I already have a version of the patch which does just that (without using macros). I will post it for comment. Thanks, Behan -- Behan Webster beh...@converseincode.com -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: Removing the use of VLAIS from the gadget driver
Hi,
On Thu, Aug 01, 2013 at 09:35:44PM -0400, beh...@converseincode.com wrote:
> From: Behan Webster
>
> The use of variable length arrays in structs (VLAIS) in the Linux Kernel code
> precludes the use of compilers which don't implement VLAIS (for instance the
> Clang compiler). This patch removes the use of VLAIS in the gadget driver.
>
> Signed-off-by: Mark Charlebois
> Signed-off-by: Behan Webster
> ---
> drivers/usb/gadget/f_fs.c | 128
> +++---
> 1 file changed, 76 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
> index f394f29..4b872c4 100644
> --- a/drivers/usb/gadget/f_fs.c
> +++ b/drivers/usb/gadget/f_fs.c
> @@ -30,7 +30,6 @@
>
> #define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */
>
> -
> /* Debugging
> /
>
> #ifdef VERBOSE_DEBUG
> @@ -214,6 +213,8 @@ struct ffs_data {
> /* ids in stringtabs are set in functionfs_bind() */
> const void *raw_strings;
> struct usb_gadget_strings **stringtabs;
> + struct usb_gadget_strings *stringtab;
> + struct usb_string *strings;
>
> /*
>* File system's super block, write once when file system is
> @@ -263,7 +264,10 @@ struct ffs_function {
>
> struct ffs_ep *eps;
> u8 eps_revmap[16];
> + struct usb_descriptor_header**fs_descs;
> + struct usb_descriptor_header**hs_descs;
> short *interfaces_nums;
> + char*raw_descs;
>
> struct usb_function function;
> };
> @@ -1345,6 +1349,8 @@ static void ffs_data_clear(struct ffs_data *ffs)
> kfree(ffs->raw_descs);
> kfree(ffs->raw_strings);
> kfree(ffs->stringtabs);
> + kfree(ffs->stringtab);
> + kfree(ffs->strings);
> }
>
> static void ffs_data_reset(struct ffs_data *ffs)
> @@ -1357,6 +1363,8 @@ static void ffs_data_reset(struct ffs_data *ffs)
> ffs->raw_descs = NULL;
> ffs->raw_strings = NULL;
> ffs->stringtabs = NULL;
> + ffs->stringtab = NULL;
> + ffs->strings = NULL;
>
> ffs->raw_descs_length = 0;
> ffs->raw_fs_descs_length = 0;
> @@ -1528,12 +1536,10 @@ static void ffs_func_free(struct ffs_function *func)
> ffs_data_put(func->ffs);
>
> kfree(func->eps);
> - /*
> - * eps and interfaces_nums are allocated in the same chunk so
> - * only one free is required. Descriptors are also allocated
> - * in the same chunk.
> - */
> -
> + kfree(func->fs_descs);
> + kfree(func->hs_descs);
> + kfree(func->interfaces_nums);
> + kfree(func->raw_descs);
> kfree(func);
> }
>
> @@ -1907,33 +1913,35 @@ static int __ffs_data_got_strings(struct ffs_data
> *ffs,
> return 0;
> }
>
> - /* Allocate everything in one chunk so there's less maintenance. */
> {
> - struct {
> - struct usb_gadget_strings *stringtabs[lang_count + 1];
> - struct usb_gadget_strings stringtab[lang_count];
> - struct usb_string strings[lang_count*(needed_count+1)];
> - } *d;
> unsigned i = 0;
> -
> - d = kmalloc(sizeof *d, GFP_KERNEL);
> - if (unlikely(!d)) {
> + usb_gadget_strings **stringtabs = NULL;
> + usb_gadget_strings *stringtab = NULL;
> + usb_string *strings = NULL;
did you compile this patch ?
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 1/1] usb: gadget: zero: Add flexible auto remote wakeup test method
On Thu, Sep 19, 2013 at 08:38:23PM +0800, Peter Chen wrote: > On Thu, Sep 19, 2013 at 09:24:15AM +0200, Daniele Forsi wrote: > > 2013/9/19 Peter Chen: > > > > > + "milliseconds to increase successive wakup delays"); > > > > there's a typo: s/wakup/wakeup/ > > > > Thanks, I think I need check more carefully for typo error I fixed this one when applying. Thanks -- balbi signature.asc Description: Digital signature
Re: [PATCH v3] davinci: fix resources passed to MUSB driver for DM6467
Hi,
On Sun, Sep 22, 2013 at 01:43:58AM +0400, Sergei Shtylyov wrote:
> After commit 09fc7d22b024692b2fe8a943b246de1af307132b (usb: musb: fix
> incorrect
> usage of resource pointer), CPPI DMA driver on DaVinci DM6467 can't detect
> its
> dedicated IRQ and so the MUSB IRQ is erroneously used instead. This is
> because
> only 2 resources are passed to the MUSB driver from the DaVinci glue layer,
> so
> fix this by always copying 3 resources (it's safe since a placeholder for
> the
> 3rd resource is always there) and passing 'pdev->num_resources' instead of
> the
> size of musb_resources[] to platform_device_add_resources().
>
> Signed-off-by: Sergei Shtylyov
> Cc: sta...@vger.kernel.org # 3.11+
>
> ---
> The patch is against the 'fixes' branch of Felipe Balbi's 'usb.git' repo.
>
> Changes in version 3:
> - fixed the size of musb_resources[] in davinci_probe().
>
> Changes in version 2:
> - resolved reject.
>
> drivers/usb/musb/davinci.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> Index: usb/drivers/usb/musb/davinci.c
> ===
> --- usb.orig/drivers/usb/musb/davinci.c
> +++ usb/drivers/usb/musb/davinci.c
> @@ -509,7 +509,7 @@ static u64 davinci_dmamask = DMA_BIT_MAS
>
> static int davinci_probe(struct platform_device *pdev)
> {
> - struct resource musb_resources[2];
> + struct resource musb_resources[3];
why don't you kcalloc this array based on pdev->num_resources ?
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] xhci: fix write to USB3_PSSEN and XUSB2PRM pci config registers
On Mon, Sep 23, 2013 at 09:06:54PM +0300, Xenia Ragiadakou wrote:
> On 09/23/2013 07:45 PM, Sarah Sharp wrote:
> >On Fri, Sep 20, 2013 at 07:45:53PM +0300, Xenia Ragiadakou wrote:
> >>The function pci_write_config_dword() sets the appropriate byteordering
> >>internally so the value argument should not be converted to little-endian.
> >>This bug was found by sparse.
> >Can you give the exact error or warning message that sparse gave?
>
> Yes, sure.
>
> drivers/usb/host/pci-quirks.c:802:25: warning: incorrect type in
> argument 3 (different base types)
> drivers/usb/host/pci-quirks.c:802:25:expected unsigned int
> [unsigned] [usertype] val
> drivers/usb/host/pci-quirks.c:802:25:got restricted __le32
> [usertype]
> drivers/usb/host/pci-quirks.c:824:25: warning: incorrect type in
> argument 3 (different base types)
> drivers/usb/host/pci-quirks.c:824:25:expected unsigned int
> [unsigned] [usertype] val
> drivers/usb/host/pci-quirks.c:824:25:got restricted __le32
> [usertype]
>
> >
> >I ask because this description sounded odd to Greg and I when we met
> >last week at LinuxCon North America. I've tried to track this down to
> >see where the code might be converting the value from CPU format to
> >little endian, and I don't see it.
> >
> >AFAICT, pci_write_config_dword() is defined in include/linux/pci.h, and
> >calls pci_bus_write_config_dword():
> >
> >static inline int pci_write_config_dword(const struct pci_dev *dev, int
> >where,
> > u32 val)
> >{
> > return pci_bus_write_config_dword(dev->bus, dev->devfn, where, val);
> >}
> >
> >pci_bus_write_config_dword is defined as a macro in drivers/pci/access.h:
> >
> >#define PCI_OP_WRITE(size,type,len) \
> >int pci_bus_write_config_##size \
> > (struct pci_bus *bus, unsigned int devfn, int pos, type value) \
> >{ \
> > int res;\
> > unsigned long flags;\
> > if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
> > raw_spin_lock_irqsave(&pci_lock, flags);\
> > res = bus->ops->write(bus, devfn, pos, len, value); \
> > raw_spin_unlock_irqrestore(&pci_lock, flags); \
> > return res; \
> >}
> >
> >That macro simply calls the write function for whatever PCI bus driver
> >is installed. Note that bus driver can be different than the standard
> >bus driver. I don't see any conversion to little endian here, so that
> >means each bus driver would have to convert it.
> >
> >I can dig deeper into each .write function, but if the conversion isn't
> >done at the upper layers, it's possible someone will create a .write
> >function without the conversion to little endian.
> >
> >Am I missing something?
>
> I had in mind that the pci_ops .read and .write defined by the PCI
> driver will take care of consistent byteorder access to the
> configuration registers. At least, that was what i understood after
> reading the
> chapter on PCI of Linux Device Drivers (more specifically for
> pci_write_config_* functions, it states that "The word and dword
> functions convert the value to little-endian before writing to the
> peripheral device.").
Hm, I wrote that paragraph (or at least I think I did), but I sure
didn't remember this at all...
Hm, wait, I do see this happening for the PowerPC cell PCI code, so it
might happen somewhere burried in the platform-specific code for
different arches. You will not see it happen on x86 as there's no need
to swap any bytes around.
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 v3] davinci: fix resources passed to MUSB driver for DM6467
Hello.
On 09/24/2013 12:42 AM, Felipe Balbi wrote:
After commit 09fc7d22b024692b2fe8a943b246de1af307132b (usb: musb: fix incorrect
usage of resource pointer), CPPI DMA driver on DaVinci DM6467 can't detect its
dedicated IRQ and so the MUSB IRQ is erroneously used instead. This is because
only 2 resources are passed to the MUSB driver from the DaVinci glue layer, so
fix this by always copying 3 resources (it's safe since a placeholder for the
3rd resource is always there) and passing 'pdev->num_resources' instead of the
size of musb_resources[] to platform_device_add_resources().
Signed-off-by: Sergei Shtylyov
Cc: sta...@vger.kernel.org # 3.11+
---
The patch is against the 'fixes' branch of Felipe Balbi's 'usb.git' repo.
Changes in version 3:
- fixed the size of musb_resources[] in davinci_probe().
Changes in version 2:
- resolved reject.
drivers/usb/musb/davinci.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
Index: usb/drivers/usb/musb/davinci.c
===
--- usb.orig/drivers/usb/musb/davinci.c
+++ usb/drivers/usb/musb/davinci.c
@@ -509,7 +509,7 @@ static u64 davinci_dmamask = DMA_BIT_MAS
static int davinci_probe(struct platform_device *pdev)
{
- struct resource musb_resources[2];
+ struct resource musb_resources[3];
why don't you kcalloc this array based on pdev->num_resources ?
I just followed your approach. And I don't see why abusing the heap is any
better (the resources will be kmemdup()ed anyway upon adding them to device)
when I know there's no more than 3 resources...
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc3: gadget: don't request IRQs in atomic
On Mon, Sep 23, 2013 at 02:26:29PM -0500, Felipe Balbi wrote: > mainline commit b0d7ffd44ba9cd2dfbf299674418193a5f9ed21a > > We cannot request an IRQ with spinlocks held > as that would trigger a sleeping inside > spinlock warning. > > Cc: # v3.10 v3.11 > Reported-by: Stephen Boyd > Signed-off-by: Felipe Balbi > --- > > Paul reported that this patch wasn't on v3.11 so I cherry-picked > on top of v3.11.1 and it applies cleanly. It wasn't there yet as it was still in my queue, I've now applied 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
[PATCH v3 0/2] staging: dwc2: add microframe scheduler
Here is the third version of the microframe scheduler patch. This version removes the NAK holdoff patch from the series, since it was effectively a no-op as pointed out by Matthijs. It also splits out into a separate patch one hunk which was not part of the downstream patch. It also acknowledges denx.de as the original source of the patch. There were some style-related comments from Dan Carpenter, but I would prefer to have those as an additional patch, once we have this known-good code added to the driver. v2: Split unrelated changes into separate patches per Matthijs Kooijman. v1: Original submission. Paul Zimmerman (2): staging: dwc2: validate urb->actual_length for OUT endpoints staging: dwc2: add microframe scheduler from downstream Pi kernel drivers/staging/dwc2/core.c | 21 + drivers/staging/dwc2/core.h | 7 ++ drivers/staging/dwc2/hcd.c | 54 --- drivers/staging/dwc2/hcd.h | 3 + drivers/staging/dwc2/hcd_ddma.c | 13 ++- drivers/staging/dwc2/hcd_intr.c | 29 +++--- drivers/staging/dwc2/hcd_queue.c | 199 --- drivers/staging/dwc2/pci.c | 1 + 8 files changed, 285 insertions(+), 42 deletions(-) -- 1.8.2.rc0.16.g20a599e -- To unsubscribe from this list: send the line "unsubscribe 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] staging: dwc2: add microframe scheduler from downstream Pi kernel
From: Dom Cobley
The transfer scheduler in the dwc2 driver is pretty basic, not to
mention buggy. It works fairly well with just a couple of devices
plugged in, but if you add, say, multiple devices with periodic
endpoints, the scheduler breaks down and can't even enumerate all
the devices.
To improve this, import the "microframe scheduler" patch from the
driver in the downstream Raspberry Pi kernel, which is based on
the Synopsys vendor driver. The original patch came from Denx
(http://git.denx.de/?p=linux-denx.git) and was commited to the
raspberrypi.org git tree by "popcornmix" (Dom Cobley).
I have added a driver parameter for this, enabled by default, in
case anyone has problems with it and needs to disable it. I don't
think we should add a DT binding for that, though, since I plan
to remove the option once any bugs are fixed.
[raspberrypi.org patch from Dom Cobley]
Signed-off-by: Dom Cobley
[adapted to dwc2 driver by Paul Zimmerman]
Signed-off-by: Paul Zimmerman
---
drivers/staging/dwc2/core.c | 21 +
drivers/staging/dwc2/core.h | 7 ++
drivers/staging/dwc2/hcd.c | 50 +++---
drivers/staging/dwc2/hcd.h | 3 +
drivers/staging/dwc2/hcd_ddma.c | 13 ++-
drivers/staging/dwc2/hcd_intr.c | 29 +++---
drivers/staging/dwc2/hcd_queue.c | 199 ---
drivers/staging/dwc2/pci.c | 1 +
8 files changed, 281 insertions(+), 42 deletions(-)
diff --git a/drivers/staging/dwc2/core.c b/drivers/staging/dwc2/core.c
index 8dbd174..9140f5e 100644
--- a/drivers/staging/dwc2/core.c
+++ b/drivers/staging/dwc2/core.c
@@ -2736,6 +2736,26 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
return 0;
}
+int dwc2_set_param_uframe_sched(struct dwc2_hsotg *hsotg, int val)
+{
+ int retval = 0;
+
+ if (DWC2_PARAM_TEST(val, 0, 1)) {
+ if (val >= 0) {
+ dev_err(hsotg->dev,
+ "'%d' invalid for parameter uframe_sched\n",
+ val);
+ dev_err(hsotg->dev, "uframe_sched must be 0 or 1\n");
+ }
+ val = 1;
+ dev_dbg(hsotg->dev, "Setting uframe_sched to %d\n", val);
+ retval = -EINVAL;
+ }
+
+ hsotg->core_params->uframe_sched = val;
+ return retval;
+}
+
/*
* This function is called during module intialization to pass module
parameters
* for the DWC_otg core. It returns non-0 if any parameters are invalid.
@@ -2782,6 +2802,7 @@ int dwc2_set_parameters(struct dwc2_hsotg *hsotg,
retval |= dwc2_set_param_reload_ctl(hsotg, params->reload_ctl);
retval |= dwc2_set_param_ahbcfg(hsotg, params->ahbcfg);
retval |= dwc2_set_param_otg_ver(hsotg, params->otg_ver);
+ retval |= dwc2_set_param_uframe_sched(hsotg, params->uframe_sched);
return retval;
}
diff --git a/drivers/staging/dwc2/core.h b/drivers/staging/dwc2/core.h
index 9102f66..f7ba34b 100644
--- a/drivers/staging/dwc2/core.h
+++ b/drivers/staging/dwc2/core.h
@@ -188,6 +188,7 @@ enum dwc2_lx_state {
* bits defined by GAHBCFG_CTRL_MASK are controlled
* by the driver and are ignored in this
* configuration value.
+ * @uframe_sched: True to enable the microframe scheduler
*
* The following parameters may be specified when starting the module. These
* parameters define how the DWC_otg controller should be configured. A
@@ -224,6 +225,7 @@ struct dwc2_core_params {
int ts_dline;
int reload_ctl;
int ahbcfg;
+ int uframe_sched;
};
/**
@@ -370,6 +372,7 @@ struct dwc2_hw_params {
* This value is in microseconds per (micro)frame. The
* assumption is that all periodic transfers may occur in
* the same (micro)frame.
+ * @frame_usecs:Internal variable used by the microframe scheduler
* @frame_number: Frame number read from the core at SOF. The value
ranges
* from 0 to HFNUM_MAX_FRNUM.
* @periodic_qh_count: Count of periodic QHs, if using several eps. Used for
@@ -382,6 +385,8 @@ struct dwc2_hw_params {
* host channel is available for non-periodic
transactions.
* @non_periodic_channels: Number of host channels assigned to non-periodic
* transfers
+ * @available_host_channels Number of host channels available for the
microframe
+ * scheduler to use
* @hc_ptr_array: Array of pointers to the host channel descriptors.
* Allows accessing a host channel descriptor given the
* host channel number. This is useful in interrupt
@@ -436,6 +441,7 @@ struct dwc2_hsotg {
struct list_head periodic_sched_assigned;
struct list_head periodic_sched_queued;
u16 periodic_usecs;
+ u16 frame_usecs[8];
u16 frame_num
[PATCH v3 1/2] staging: dwc2: validate urb->actual_length for OUT endpoints
In dwc2_assign_and_init_hc(), validate urb->actual_length for OUT
endpoints before using the value. This fix is from the Synopsys
vendor driver.
Signed-off-by: Paul Zimmerman
---
drivers/staging/dwc2/hcd.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/staging/dwc2/hcd.c b/drivers/staging/dwc2/hcd.c
index da0d35c..b245405 100644
--- a/drivers/staging/dwc2/hcd.c
+++ b/drivers/staging/dwc2/hcd.c
@@ -780,6 +780,10 @@ static void dwc2_assign_and_init_hc(struct dwc2_hsotg
*hsotg,
chan->data_pid_start = qh->data_toggle;
chan->multi_count = 1;
+ if ((urb->actual_length < 0 || urb->actual_length > urb->length) &&
+ !dwc2_hcd_is_pipe_in(&urb->pipe_info))
+ urb->actual_length = urb->length;
+
if (hsotg->core_params->dma_enable > 0) {
chan->xfer_dma = urb->dma + urb->actual_length;
--
1.8.2.rc0.16.g20a599e
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
EHCI bus glue driver works for storage, fails for a WiFi module
Hi, recently I was working on porting EHCI HCD bus glue driver from the vendors kernel tree to the mainline [1]. I've got the storage (USB stick and USB external hard drive) working and was happy. However it does not work completely. Specifically something goes wrong if WiFi module is talked to. This WiFi module is on-board and connected through USB. The WiFi module is correctly identified and the rtl8192cu driver manages to output something, but issuing ip link set wlan0 up will cause an error, see the output of the dmesg [2]. Note, the same hardware works with vendor's tree EHCI bus glue driver and rtl8192cu, so hardware is ok. Maybe using a fact that my driver works for the storage but fails for the WiFi module you could help me identify the problem in it? By the way, the output of the lsusb - looks identical in both cases (with my driver and vendor's) [3]. Thank you, Arokux [1] https://github.com/arokux/linux/commit/da09a5739d610d10aeabc0ac852ecb5b8eaee8d9 [2] http://sprunge.us/Qihe [3] http://sprunge.us/NKXc -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ehci-pci D3cold logspam
[+cc Rafael, linux-pm]
On Mon, Sep 23, 2013 at 1:36 PM, Alan Stern wrote:
> On Mon, 23 Sep 2013, Andy Lutomirski wrote:
>
>> I've been getting this on several recent kernel revisions. Is it
>> interesting? If so, I'm happy to help diagnose it. If not, can the
>> message be killed or severely ratelimited? I'm getting so much of
>> this that it tends to overflow the log ring.
>
> It's interesting only if you care about when your EHCI controllers get
> resumed and suspended. In this case, it's not clear why the
> transitions happen so rapidly. It looks like some sort of polling
> is going on at roughly 2-second intervals.
>
>> [ 287.344991] ehci-pci :00:1d.0: power state changed by ACPI to D0
>> [ 287.445433] ehci-pci :00:1d.0: setting latency timer to 64
>> [ 287.446255] ehci-pci :00:1a.0: power state changed by ACPI to D0
>> [ 287.456094] ehci-pci :00:1d.0: power state changed by ACPI to D3cold
>> [ 287.547205] ehci-pci :00:1a.0: setting latency timer to 64
>> [ 287.557890] ehci-pci :00:1a.0: power state changed by ACPI to D3cold
>> [ 290.126910] ehci-pci :00:1d.0: power state changed by ACPI to D0
>> [ 290.227958] ehci-pci :00:1d.0: setting latency timer to 64
>> [ 290.228416] ehci-pci :00:1a.0: power state changed by ACPI to D0
>> [ 290.238749] ehci-pci :00:1d.0: power state changed by ACPI to D3cold
>> [ 290.328904] ehci-pci :00:1a.0: setting latency timer to 64
>> [ 290.339565] ehci-pci :00:1a.0: power state changed by ACPI to D3cold
>> [ 292.214834] ehci-pci :00:1d.0: power state changed by ACPI to D0
>> [ 292.315458] ehci-pci :00:1d.0: setting latency timer to 64
>> [ 292.315908] ehci-pci :00:1a.0: power state changed by ACPI to D0
>> [ 292.326262] ehci-pci :00:1d.0: power state changed by ACPI to D3cold
>> [ 292.416487] ehci-pci :00:1a.0: setting latency timer to 64
>> [ 292.431075] ehci-pci :00:1a.0: power state changed by ACPI to D3cold
>> [ 295.458048] ehci-pci :00:1d.0: power state changed by ACPI to D0
>> [ 295.558613] ehci-pci :00:1d.0: setting latency timer to 64
>
> This question should be addressed to the PCI mailing list (cc'ed), as
> those two messages are generated by
> drivers/pci/pci-acpi.c:acpi_pci_set_power_state() and
> drivers/pci/pci.c:pcibios_set_master() respectively.
d010e5769 ("PCI / ACPI: Use dev_dbg() instead of dev_info() in
acpi_pci_set_power_state()") might be part of the solution. That was
done in response to https://bugzilla.kernel.org/show_bug.cgi?id=60636,
which looks basically the same as your complaint.
But if we are indeed polling every two seconds, even a dev_dbg() seems
like overkill to me. Rafael or Lan can probably provide a better
answer here.
As for the "setting latency timer" messages, I really doubt those are
useful to anybody. If nobody objects, I'll just drop it, e.g.:
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b821a62..55a947b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2854,7 +2854,7 @@ void __weak pcibios_set_master(struct pci_dev *dev)
lat = pcibios_max_latency;
else
return;
- dev_printk(KERN_DEBUG, &dev->dev, "setting latency timer to %d\n", lat);
+
pci_write_config_byte(dev, PCI_LATENCY_TIMER, lat);
}
--
To unsubscribe from this list: send the line "unsubscribe 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: fix write to USB3_PSSEN and XUSB2PRM pci config registers
On Mon, Sep 23, 2013 at 3:49 PM, Greg KH wrote:
> On Mon, Sep 23, 2013 at 09:06:54PM +0300, Xenia Ragiadakou wrote:
>> On 09/23/2013 07:45 PM, Sarah Sharp wrote:
>> >On Fri, Sep 20, 2013 at 07:45:53PM +0300, Xenia Ragiadakou wrote:
>> >>The function pci_write_config_dword() sets the appropriate byteordering
>> >>internally so the value argument should not be converted to little-endian.
>> >>This bug was found by sparse.
>> >Can you give the exact error or warning message that sparse gave?
>>
>> Yes, sure.
>>
>> drivers/usb/host/pci-quirks.c:802:25: warning: incorrect type in
>> argument 3 (different base types)
>> drivers/usb/host/pci-quirks.c:802:25:expected unsigned int
>> [unsigned] [usertype] val
>> drivers/usb/host/pci-quirks.c:802:25:got restricted __le32
>> [usertype]
>> drivers/usb/host/pci-quirks.c:824:25: warning: incorrect type in
>> argument 3 (different base types)
>> drivers/usb/host/pci-quirks.c:824:25:expected unsigned int
>> [unsigned] [usertype] val
>> drivers/usb/host/pci-quirks.c:824:25:got restricted __le32
>> [usertype]
>>
>> >
>> >I ask because this description sounded odd to Greg and I when we met
>> >last week at LinuxCon North America. I've tried to track this down to
>> >see where the code might be converting the value from CPU format to
>> >little endian, and I don't see it.
>> >
>> >AFAICT, pci_write_config_dword() is defined in include/linux/pci.h, and
>> >calls pci_bus_write_config_dword():
>> >
>> >static inline int pci_write_config_dword(const struct pci_dev *dev, int
>> >where,
>> > u32 val)
>> >{
>> > return pci_bus_write_config_dword(dev->bus, dev->devfn, where,
>> > val);
>> >}
>> >
>> >pci_bus_write_config_dword is defined as a macro in drivers/pci/access.h:
>> >
>> >#define PCI_OP_WRITE(size,type,len) \
>> >int pci_bus_write_config_##size \
>> > (struct pci_bus *bus, unsigned int devfn, int pos, type value) \
>> >{ \
>> > int res;\
>> > unsigned long flags;\
>> > if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
>> > raw_spin_lock_irqsave(&pci_lock, flags);\
>> > res = bus->ops->write(bus, devfn, pos, len, value); \
>> > raw_spin_unlock_irqrestore(&pci_lock, flags); \
>> > return res; \
>> >}
>> >
>> >That macro simply calls the write function for whatever PCI bus driver
>> >is installed. Note that bus driver can be different than the standard
>> >bus driver. I don't see any conversion to little endian here, so that
>> >means each bus driver would have to convert it.
>> >
>> >I can dig deeper into each .write function, but if the conversion isn't
>> >done at the upper layers, it's possible someone will create a .write
>> >function without the conversion to little endian.
>> >
>> >Am I missing something?
>>
>> I had in mind that the pci_ops .read and .write defined by the PCI
>> driver will take care of consistent byteorder access to the
>> configuration registers. At least, that was what i understood after
>> reading the
>> chapter on PCI of Linux Device Drivers (more specifically for
>> pci_write_config_* functions, it states that "The word and dword
>> functions convert the value to little-endian before writing to the
>> peripheral device.").
>
> Hm, I wrote that paragraph (or at least I think I did), but I sure
> didn't remember this at all...
>
> Hm, wait, I do see this happening for the PowerPC cell PCI code, so it
> might happen somewhere burried in the platform-specific code for
> different arches. You will not see it happen on x86 as there's no need
> to swap any bytes around.
Greg, with regard to Xenia's patch, is this an ack or a nack? Since
you didn't include an "Acked-by" line, I assume you think Xenia's
patch is unnecessary. In that case, is there any way to shut sparse
up so it doesn't complain about this?
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc3: gadget: don't request IRQs in atomic
Hi, On Mon, Sep 23, 2013 at 02:06:12PM -0700, Greg KH wrote: > On Mon, Sep 23, 2013 at 02:26:29PM -0500, Felipe Balbi wrote: > > mainline commit b0d7ffd44ba9cd2dfbf299674418193a5f9ed21a > > > > We cannot request an IRQ with spinlocks held > > as that would trigger a sleeping inside > > spinlock warning. > > > > Cc: # v3.10 v3.11 > > Reported-by: Stephen Boyd > > Signed-off-by: Felipe Balbi > > --- > > > > Paul reported that this patch wasn't on v3.11 so I cherry-picked > > on top of v3.11.1 and it applies cleanly. > > It wasn't there yet as it was still in my queue, I've now applied it, > thanks. Alright, thanks... sorry to bother :-) -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 1/2] staging: dwc2: validate urb->actual_length for OUT endpoints
Hi, On Mon, Sep 23, 2013 at 02:23:33PM -0700, Paul Zimmerman wrote: > In dwc2_assign_and_init_hc(), validate urb->actual_length for OUT > endpoints before using the value. This fix is from the Synopsys > vendor driver. > > Signed-off-by: Paul Zimmerman > --- > drivers/staging/dwc2/hcd.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/staging/dwc2/hcd.c b/drivers/staging/dwc2/hcd.c > index da0d35c..b245405 100644 > --- a/drivers/staging/dwc2/hcd.c > +++ b/drivers/staging/dwc2/hcd.c > @@ -780,6 +780,10 @@ static void dwc2_assign_and_init_hc(struct dwc2_hsotg > *hsotg, > chan->data_pid_start = qh->data_toggle; > chan->multi_count = 1; > > + if ((urb->actual_length < 0 || urb->actual_length > urb->length) && > + !dwc2_hcd_is_pipe_in(&urb->pipe_info)) > + urb->actual_length = urb->length; weird, why would actual_length be less than zero or greather than urb->length ? I guess you need some more "meat" in your commit log. -- balbi signature.asc Description: Digital signature
Re: [PATCH v3] davinci: fix resources passed to MUSB driver for DM6467
On Tue, Sep 24, 2013 at 12:50:12AM +0400, Sergei Shtylyov wrote:
> Hello.
>
> On 09/24/2013 12:42 AM, Felipe Balbi wrote:
>
> >>After commit 09fc7d22b024692b2fe8a943b246de1af307132b (usb: musb: fix
> >>incorrect
> >>usage of resource pointer), CPPI DMA driver on DaVinci DM6467 can't detect
> >>its
> >>dedicated IRQ and so the MUSB IRQ is erroneously used instead. This is
> >>because
> >>only 2 resources are passed to the MUSB driver from the DaVinci glue layer,
> >> so
> >>fix this by always copying 3 resources (it's safe since a placeholder for
> >>the
> >>3rd resource is always there) and passing 'pdev->num_resources' instead of
> >>the
> >>size of musb_resources[] to platform_device_add_resources().
>
> >>Signed-off-by: Sergei Shtylyov
> >>Cc: sta...@vger.kernel.org # 3.11+
>
> >>---
> >>The patch is against the 'fixes' branch of Felipe Balbi's 'usb.git' repo.
>
> >>Changes in version 3:
> >>- fixed the size of musb_resources[] in davinci_probe().
>
> >>Changes in version 2:
> >>- resolved reject.
>
> >> drivers/usb/musb/davinci.c | 13 +++--
> >> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> >>Index: usb/drivers/usb/musb/davinci.c
> >>===
> >>--- usb.orig/drivers/usb/musb/davinci.c
> >>+++ usb/drivers/usb/musb/davinci.c
> >>@@ -509,7 +509,7 @@ static u64 davinci_dmamask = DMA_BIT_MAS
> >>
> >> static int davinci_probe(struct platform_device *pdev)
> >> {
> >>- struct resource musb_resources[2];
> >>+ struct resource musb_resources[3];
>
> >why don't you kcalloc this array based on pdev->num_resources ?
>
>I just followed your approach. And I don't see why abusing the
> heap is any better (the resources will be kmemdup()ed anyway upon
> adding them to device) when I know there's no more than 3
> resources...
well, fair enough... at least on OMAP-based devices we could have 2 or 3
resources, so it makes more sense there, I guess.
cheers
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] xhci: fix write to USB3_PSSEN and XUSB2PRM pci config registers
On Mon, Sep 23, 2013 at 04:38:22PM -0500, Bjorn Helgaas wrote:
> On Mon, Sep 23, 2013 at 3:49 PM, Greg KH wrote:
> > On Mon, Sep 23, 2013 at 09:06:54PM +0300, Xenia Ragiadakou wrote:
> >> On 09/23/2013 07:45 PM, Sarah Sharp wrote:
> >> >On Fri, Sep 20, 2013 at 07:45:53PM +0300, Xenia Ragiadakou wrote:
> >> >>The function pci_write_config_dword() sets the appropriate byteordering
> >> >>internally so the value argument should not be converted to
> >> >>little-endian.
> >> >>This bug was found by sparse.
> >> >Can you give the exact error or warning message that sparse gave?
> >>
> >> Yes, sure.
> >>
> >> drivers/usb/host/pci-quirks.c:802:25: warning: incorrect type in
> >> argument 3 (different base types)
> >> drivers/usb/host/pci-quirks.c:802:25:expected unsigned int
> >> [unsigned] [usertype] val
> >> drivers/usb/host/pci-quirks.c:802:25:got restricted __le32
> >> [usertype]
> >> drivers/usb/host/pci-quirks.c:824:25: warning: incorrect type in
> >> argument 3 (different base types)
> >> drivers/usb/host/pci-quirks.c:824:25:expected unsigned int
> >> [unsigned] [usertype] val
> >> drivers/usb/host/pci-quirks.c:824:25:got restricted __le32
> >> [usertype]
> >>
> >> >
> >> >I ask because this description sounded odd to Greg and I when we met
> >> >last week at LinuxCon North America. I've tried to track this down to
> >> >see where the code might be converting the value from CPU format to
> >> >little endian, and I don't see it.
> >> >
> >> >AFAICT, pci_write_config_dword() is defined in include/linux/pci.h, and
> >> >calls pci_bus_write_config_dword():
> >> >
> >> >static inline int pci_write_config_dword(const struct pci_dev *dev, int
> >> >where,
> >> > u32 val)
> >> >{
> >> > return pci_bus_write_config_dword(dev->bus, dev->devfn, where,
> >> > val);
> >> >}
> >> >
> >> >pci_bus_write_config_dword is defined as a macro in drivers/pci/access.h:
> >> >
> >> >#define PCI_OP_WRITE(size,type,len) \
> >> >int pci_bus_write_config_##size \
> >> > (struct pci_bus *bus, unsigned int devfn, int pos, type value) \
> >> >{ \
> >> > int res;\
> >> > unsigned long flags;\
> >> > if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
> >> > raw_spin_lock_irqsave(&pci_lock, flags);\
> >> > res = bus->ops->write(bus, devfn, pos, len, value); \
> >> > raw_spin_unlock_irqrestore(&pci_lock, flags); \
> >> > return res; \
> >> >}
> >> >
> >> >That macro simply calls the write function for whatever PCI bus driver
> >> >is installed. Note that bus driver can be different than the standard
> >> >bus driver. I don't see any conversion to little endian here, so that
> >> >means each bus driver would have to convert it.
> >> >
> >> >I can dig deeper into each .write function, but if the conversion isn't
> >> >done at the upper layers, it's possible someone will create a .write
> >> >function without the conversion to little endian.
> >> >
> >> >Am I missing something?
> >>
> >> I had in mind that the pci_ops .read and .write defined by the PCI
> >> driver will take care of consistent byteorder access to the
> >> configuration registers. At least, that was what i understood after
> >> reading the
> >> chapter on PCI of Linux Device Drivers (more specifically for
> >> pci_write_config_* functions, it states that "The word and dword
> >> functions convert the value to little-endian before writing to the
> >> peripheral device.").
> >
> > Hm, I wrote that paragraph (or at least I think I did), but I sure
> > didn't remember this at all...
> >
> > Hm, wait, I do see this happening for the PowerPC cell PCI code, so it
> > might happen somewhere burried in the platform-specific code for
> > different arches. You will not see it happen on x86 as there's no need
> > to swap any bytes around.
>
> Greg, with regard to Xenia's patch, is this an ack or a nack? Since
> you didn't include an "Acked-by" line, I assume you think Xenia's
> patch is unnecessary. In that case, is there any way to shut sparse
> up so it doesn't complain about this?
At this point in time, I don't remember what the original patch looked
like, and as it's an xhci patch, Sarah needs to ack it, not me :)
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 v3 1/2] staging: dwc2: validate urb->actual_length for OUT endpoints
> From: Felipe Balbi [mailto:ba...@ti.com] > Sent: Monday, September 23, 2013 2:45 PM > > On Mon, Sep 23, 2013 at 02:23:33PM -0700, Paul Zimmerman wrote: > > In dwc2_assign_and_init_hc(), validate urb->actual_length for OUT > > endpoints before using the value. This fix is from the Synopsys > > vendor driver. > > > > Signed-off-by: Paul Zimmerman > > --- > > drivers/staging/dwc2/hcd.c | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/staging/dwc2/hcd.c b/drivers/staging/dwc2/hcd.c > > index da0d35c..b245405 100644 > > --- a/drivers/staging/dwc2/hcd.c > > +++ b/drivers/staging/dwc2/hcd.c > > @@ -780,6 +780,10 @@ static void dwc2_assign_and_init_hc(struct dwc2_hsotg > > *hsotg, > > chan->data_pid_start = qh->data_toggle; > > chan->multi_count = 1; > > > > + if ((urb->actual_length < 0 || urb->actual_length > urb->length) && > > + !dwc2_hcd_is_pipe_in(&urb->pipe_info)) > > + urb->actual_length = urb->length; > > weird, why would actual_length be less than zero or greather than > urb->length ? I guess you need some more "meat" in your commit log. I assume it was to fix some issue seen by our developers. We don't have detailed commit logs for that driver, so I am unable to say for certain. But since it is part of the downstream RaspberryPi driver, which has seen a lot of testing, I thought it best to include it. -- Paul -- To unsubscribe from this list: send the line "unsubscribe 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 1/3] usb: dwc3: msm: Add device tree binding information
On 09/23/2013 01:32 PM, Felipe Balbi wrote: > Hi, > > On Wed, Aug 21, 2013 at 04:29:44PM +0300, Ivan T. Ivanov wrote: >> From: "Ivan T. Ivanov" >> >> MSM USB3.0 core wrapper consist of USB3.0 IP from Synopsys (SNPS) >> and HS, SS PHY's control and configuration registers. >> >> It could operate in device mode (SS, HS, FS) and host mode (SS, >> HS, FS, LS). >> >> Signed-off-by: Ivan T. Ivanov > > and here's a new version from same patch The binding looks pretty simple, so I don't think it's too contentious. >> diff --git a/Documentation/devicetree/bindings/usb/msm-ssusb.txt >> b/Documentation/devicetree/bindings/usb/msm-ssusb.txt >> +MSM DWC3 controller wrapper >> +Optional properties : +- gdsc-supply : phandle to the globally >> distributed switch controller + regulator node to the USB >> controller. If that's a regulator node, why not use xxx-supply properties to interface with it? Aside from that, the binding, Acked-by: Stephen Warren -- To unsubscribe from this list: send the line "unsubscribe 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: Removing the use of VLAIS from the gadget driver
On 09/23/13 16:24, Felipe Balbi wrote:
Hi,
On Thu, Aug 01, 2013 at 09:35:44PM -0400, beh...@converseincode.com wrote:
From: Behan Webster
The use of variable length arrays in structs (VLAIS) in the Linux Kernel code
precludes the use of compilers which don't implement VLAIS (for instance the
Clang compiler). This patch removes the use of VLAIS in the gadget driver.
Signed-off-by: Mark Charlebois
Signed-off-by: Behan Webster
---
drivers/usb/gadget/f_fs.c | 128 +++---
1 file changed, 76 insertions(+), 52 deletions(-)
diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index f394f29..4b872c4 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -30,7 +30,6 @@
#define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */
-
/* Debugging /
#ifdef VERBOSE_DEBUG
@@ -214,6 +213,8 @@ struct ffs_data {
/* ids in stringtabs are set in functionfs_bind() */
const void *raw_strings;
struct usb_gadget_strings **stringtabs;
+ struct usb_gadget_strings *stringtab;
+ struct usb_string *strings;
/*
* File system's super block, write once when file system is
@@ -263,7 +264,10 @@ struct ffs_function {
struct ffs_ep *eps;
u8 eps_revmap[16];
+ struct usb_descriptor_header**fs_descs;
+ struct usb_descriptor_header**hs_descs;
short *interfaces_nums;
+ char*raw_descs;
struct usb_function function;
};
@@ -1345,6 +1349,8 @@ static void ffs_data_clear(struct ffs_data *ffs)
kfree(ffs->raw_descs);
kfree(ffs->raw_strings);
kfree(ffs->stringtabs);
+ kfree(ffs->stringtab);
+ kfree(ffs->strings);
}
static void ffs_data_reset(struct ffs_data *ffs)
@@ -1357,6 +1363,8 @@ static void ffs_data_reset(struct ffs_data *ffs)
ffs->raw_descs = NULL;
ffs->raw_strings = NULL;
ffs->stringtabs = NULL;
+ ffs->stringtab = NULL;
+ ffs->strings = NULL;
ffs->raw_descs_length = 0;
ffs->raw_fs_descs_length = 0;
@@ -1528,12 +1536,10 @@ static void ffs_func_free(struct ffs_function *func)
ffs_data_put(func->ffs);
kfree(func->eps);
- /*
-* eps and interfaces_nums are allocated in the same chunk so
-* only one free is required. Descriptors are also allocated
-* in the same chunk.
-*/
-
+ kfree(func->fs_descs);
+ kfree(func->hs_descs);
+ kfree(func->interfaces_nums);
+ kfree(func->raw_descs);
kfree(func);
}
@@ -1907,33 +1913,35 @@ static int __ffs_data_got_strings(struct ffs_data *ffs,
return 0;
}
- /* Allocate everything in one chunk so there's less maintenance. */
{
- struct {
- struct usb_gadget_strings *stringtabs[lang_count + 1];
- struct usb_gadget_strings stringtab[lang_count];
- struct usb_string strings[lang_count*(needed_count+1)];
- } *d;
unsigned i = 0;
-
- d = kmalloc(sizeof *d, GFP_KERNEL);
- if (unlikely(!d)) {
+ usb_gadget_strings **stringtabs = NULL;
+ usb_gadget_strings *stringtab = NULL;
+ usb_string *strings = NULL;
did you compile this patch ?
Appologies. The patch as posted has a bug which was fixed after sending
it to the list.
Andrzej Pietrasiewicz has the fixed one. Will
send the fixed one to the list too.
Behan
--
Behan Webster
beh...@converseincode.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: LLVMLinux: Remove VLAIS from USB gadget
From: Mark Charlebois
The use of variable length arrays in structs (VLAIS) in the Linux Kernel code
precludes the use of compilers which don't implement VLAIS (for instance the
Clang compiler). This patch removes the use of VLAIS in the gadget driver.
This version has been tested to compile cleanly.
Signed-off-by: Mark Charlebois
Signed-off-by: Behan Webster
---
drivers/usb/gadget/f_fs.c | 128 +++---
1 file changed, 76 insertions(+), 52 deletions(-)
--- linux.orig/drivers/usb/gadget/f_fs.c
+++ linux/drivers/usb/gadget/f_fs.c
@@ -30,7 +30,6 @@
#define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */
-
/* Debugging /
#ifdef VERBOSE_DEBUG
@@ -214,6 +213,8 @@
/* ids in stringtabs are set in functionfs_bind() */
const void *raw_strings;
struct usb_gadget_strings **stringtabs;
+ struct usb_gadget_strings *stringtab;
+ struct usb_string *strings;
/*
* File system's super block, write once when file system is
@@ -263,7 +264,10 @@
struct ffs_ep *eps;
u8 eps_revmap[16];
+ struct usb_descriptor_header**fs_descs;
+ struct usb_descriptor_header**hs_descs;
short *interfaces_nums;
+ char*raw_descs;
struct usb_function function;
};
@@ -1345,6 +1349,8 @@
kfree(ffs->raw_descs);
kfree(ffs->raw_strings);
kfree(ffs->stringtabs);
+ kfree(ffs->stringtab);
+ kfree(ffs->strings);
}
static void ffs_data_reset(struct ffs_data *ffs)
@@ -1357,6 +1363,8 @@
ffs->raw_descs = NULL;
ffs->raw_strings = NULL;
ffs->stringtabs = NULL;
+ ffs->stringtab = NULL;
+ ffs->strings = NULL;
ffs->raw_descs_length = 0;
ffs->raw_fs_descs_length = 0;
@@ -1528,12 +1536,10 @@
ffs_data_put(func->ffs);
kfree(func->eps);
- /*
-* eps and interfaces_nums are allocated in the same chunk so
-* only one free is required. Descriptors are also allocated
-* in the same chunk.
-*/
-
+ kfree(func->fs_descs);
+ kfree(func->hs_descs);
+ kfree(func->interfaces_nums);
+ kfree(func->raw_descs);
kfree(func);
}
@@ -1877,8 +1883,9 @@
char *const _data, size_t len)
{
u32 str_count, needed_count, lang_count;
- struct usb_gadget_strings **stringtabs, *t;
- struct usb_string *strings, *s;
+ struct usb_gadget_strings *t, **stringtabs = NULL;
+ struct usb_gadget_strings *stringtab = NULL;
+ struct usb_string *s, *strings = NULL;
const char *data = _data;
ENTER();
@@ -1907,33 +1914,33 @@
return 0;
}
- /* Allocate everything in one chunk so there's less maintenance. */
{
- struct {
- struct usb_gadget_strings *stringtabs[lang_count + 1];
- struct usb_gadget_strings stringtab[lang_count];
- struct usb_string strings[lang_count*(needed_count+1)];
- } *d;
unsigned i = 0;
-
- d = kmalloc(sizeof *d, GFP_KERNEL);
- if (unlikely(!d)) {
+ struct usb_gadget_strings **b;
+
+ stringtabs = kmalloc(sizeof(*stringtabs)*(lang_count + 1),
+ GFP_KERNEL);
+ stringtab = kmalloc(sizeof(*stringtab)*(lang_count),
+ GFP_KERNEL);
+ strings = kmalloc(sizeof(*strings)
+ * (lang_count * (needed_count + 1)), GFP_KERNEL);
+ if (unlikely(!stringtabs || !stringtab || !strings)) {
+ kfree(stringtabs);
+ kfree(stringtab);
+ kfree(strings);
kfree(_data);
return -ENOMEM;
}
-
- stringtabs = d->stringtabs;
- t = d->stringtab;
+ b = stringtabs;
+ t = stringtab;
i = lang_count;
do {
- *stringtabs++ = t++;
+ *b++ = t++;
} while (--i);
- *stringtabs = NULL;
+ *b = NULL;
- stringtabs = d->stringtabs;
- t = d->stringtab;
- s = d->strings;
- strings = s;
+ t = stringtab;
+ s = strings;
}
/* For each language */
@@ -1991,12 +1998,16 @@
/* Done! */
ffs->stringtabs = stringtabs;
+ ffs->stringtab = stringtab;
+ ffs->strings = strings;
ffs->raw_strings = _data;
return 0;
error_free:
Re: ehci-pci D3cold logspam
On Monday, September 23, 2013 04:28:49 PM Bjorn Helgaas wrote:
> [+cc Rafael, linux-pm]
>
> On Mon, Sep 23, 2013 at 1:36 PM, Alan Stern wrote:
> > On Mon, 23 Sep 2013, Andy Lutomirski wrote:
> >
> >> I've been getting this on several recent kernel revisions. Is it
> >> interesting? If so, I'm happy to help diagnose it. If not, can the
> >> message be killed or severely ratelimited? I'm getting so much of
> >> this that it tends to overflow the log ring.
> >
> > It's interesting only if you care about when your EHCI controllers get
> > resumed and suspended. In this case, it's not clear why the
> > transitions happen so rapidly. It looks like some sort of polling
> > is going on at roughly 2-second intervals.
> >
> >> [ 287.344991] ehci-pci :00:1d.0: power state changed by ACPI to D0
> >> [ 287.445433] ehci-pci :00:1d.0: setting latency timer to 64
> >> [ 287.446255] ehci-pci :00:1a.0: power state changed by ACPI to D0
> >> [ 287.456094] ehci-pci :00:1d.0: power state changed by ACPI to D3cold
> >> [ 287.547205] ehci-pci :00:1a.0: setting latency timer to 64
> >> [ 287.557890] ehci-pci :00:1a.0: power state changed by ACPI to D3cold
> >> [ 290.126910] ehci-pci :00:1d.0: power state changed by ACPI to D0
> >> [ 290.227958] ehci-pci :00:1d.0: setting latency timer to 64
> >> [ 290.228416] ehci-pci :00:1a.0: power state changed by ACPI to D0
> >> [ 290.238749] ehci-pci :00:1d.0: power state changed by ACPI to D3cold
> >> [ 290.328904] ehci-pci :00:1a.0: setting latency timer to 64
> >> [ 290.339565] ehci-pci :00:1a.0: power state changed by ACPI to D3cold
> >> [ 292.214834] ehci-pci :00:1d.0: power state changed by ACPI to D0
> >> [ 292.315458] ehci-pci :00:1d.0: setting latency timer to 64
> >> [ 292.315908] ehci-pci :00:1a.0: power state changed by ACPI to D0
> >> [ 292.326262] ehci-pci :00:1d.0: power state changed by ACPI to D3cold
> >> [ 292.416487] ehci-pci :00:1a.0: setting latency timer to 64
> >> [ 292.431075] ehci-pci :00:1a.0: power state changed by ACPI to D3cold
> >> [ 295.458048] ehci-pci :00:1d.0: power state changed by ACPI to D0
> >> [ 295.558613] ehci-pci :00:1d.0: setting latency timer to 64
> >
> > This question should be addressed to the PCI mailing list (cc'ed), as
> > those two messages are generated by
> > drivers/pci/pci-acpi.c:acpi_pci_set_power_state() and
> > drivers/pci/pci.c:pcibios_set_master() respectively.
>
> d010e5769 ("PCI / ACPI: Use dev_dbg() instead of dev_info() in
> acpi_pci_set_power_state()") might be part of the solution. That was
> done in response to https://bugzilla.kernel.org/show_bug.cgi?id=60636,
> which looks basically the same as your complaint.
>
> But if we are indeed polling every two seconds, even a dev_dbg() seems
> like overkill to me. Rafael or Lan can probably provide a better
> answer here.
I'd like to be able to see if ACPI PM is called for devices at least in
debug mode. Otherwise it's difficult to say why things don't work sometimes.
Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe 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: LLVMLinux: Remove VLAIS from USB gadget
On 09/23/13 18:53, charl...@gmail.com wrote: From: Mark Charlebois The use of variable length arrays in structs (VLAIS) in the Linux Kernel code precludes the use of compilers which don't implement VLAIS (for instance the Clang compiler). This patch removes the use of VLAIS in the gadget driver. This version has been tested to compile cleanly. Signed-off-by: Mark Charlebois Signed-off-by: Behan Webster This is the fixed patch which was sent to Andrzej Pietrasiewicz. Behan -- Behan Webster beh...@converseincode.com -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] usb: xhci: change default enumeration scheme and trivial cleanups
Patch 4 is the only interesting one and adds support for the default enumeration scheme used by legacy hosts, details in the changelog. The others are just random cleanup items. The original discussion for patch 4 can be found here: http://marc.info/?l=linux-usb&m=135906806229309&w=2 This is all 3.13 material. We may want the enumeration change in -stable at some point, but for now it needs more exposure. Comments welcome. -- Dan --- Dan Williams (4): usb: hub_activate kill an 'else' usb: xhci: kill a conditional when toggling cycle usb: xhci: remove the unused ->address field usb: xhci: change enumeration scheme to 'new scheme' by default drivers/usb/core/hub.c | 33 - drivers/usb/host/xhci-pci.c |1 + drivers/usb/host/xhci-plat.c |1 + drivers/usb/host/xhci-ring.c |8 drivers/usb/host/xhci.c | 29 + drivers/usb/host/xhci.h | 13 ++--- include/linux/usb/hcd.h |2 ++ 7 files changed, 59 insertions(+), 28 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] usb: hub_activate kill an 'else'
Remove a few extra lines and make it clear that all implementations
disable the port by sharing the same line of code.
Cc: Alan Stern
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 11 ---
1 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5d859fc..69bbb51 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1107,16 +1107,13 @@ static void hub_activate(struct usb_hub *hub, enum
hub_activation_type type)
/*
* USB3 protocol ports will automatically transition
* to Enabled state when detect an USB3.0 device attach.
-* Do not disable USB3 protocol ports.
+* Do not disable USB3 protocol ports, just pretend
+* power was lost
*/
- if (!hub_is_superspeed(hdev)) {
+ portstatus &= ~USB_PORT_STAT_ENABLE;
+ if (!hub_is_superspeed(hdev))
usb_clear_port_feature(hdev, port1,
USB_PORT_FEAT_ENABLE);
- portstatus &= ~USB_PORT_STAT_ENABLE;
- } else {
- /* Pretend that power was lost for USB3 devs */
- portstatus &= ~USB_PORT_STAT_ENABLE;
- }
}
/* Clear status-change flags; we'll debounce later */
--
To unsubscribe from this list: send the line "unsubscribe 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/4] usb: xhci: kill a conditional when toggling cycle
Perform an unconditional toggle of the cycle bit with 'xor'.
Signed-off-by: Dan Williams
---
drivers/usb/host/xhci-ring.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 411da1f..7c043ec 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -168,7 +168,7 @@ static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring
*ring)
if (ring->type == TYPE_EVENT &&
last_trb_on_last_seg(xhci, ring,
ring->deq_seg, ring->dequeue)) {
- ring->cycle_state = (ring->cycle_state ? 0 : 1);
+ ring->cycle_state ^= 1;
}
ring->deq_seg = ring->deq_seg->next;
ring->dequeue = ring->deq_seg->trbs;
--
To unsubscribe from this list: send the line "unsubscribe 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/4] usb: xhci: remove the unused ->address field
Only used for debug output, so we don't need to save it.
Signed-off-by: Dan Williams
---
drivers/usb/host/xhci.c | 10 ++
drivers/usb/host/xhci.h |2 --
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 49b6edb..763d0a5 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3724,9 +3724,6 @@ disable_slot:
* the device).
* We should be protected by the usb_address0_mutex in khubd's hub_port_init,
so
* we should only issue and wait on one address command at the same time.
- *
- * We add one to the device address issued by the hardware because the USB core
- * uses address 1 for the root hubs (even though they're not really devices).
*/
int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev)
{
@@ -3871,16 +3868,13 @@ int xhci_address_device(struct usb_hcd *hcd, struct
usb_device *udev)
slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->out_ctx);
trace_xhci_address_ctx(xhci, virt_dev->out_ctx,
slot_ctx->dev_info >> 27);
- /* Use kernel assigned address for devices; store xHC assigned
-* address locally. */
- virt_dev->address = (le32_to_cpu(slot_ctx->dev_state) & DEV_ADDR_MASK)
- + 1;
/* Zero the input context control for later use */
ctrl_ctx->add_flags = 0;
ctrl_ctx->drop_flags = 0;
xhci_dbg_trace(xhci, trace_xhci_dbg_address,
- "Internal device address = %d", virt_dev->address);
+ "Internal device address = %d",
+ le32_to_cpu(slot_ctx->dev_state) & DEV_ADDR_MASK);
return 0;
}
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 46aa148..6c7aa90 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -934,8 +934,6 @@ struct xhci_virt_device {
/* Rings saved to ensure old alt settings can be re-instated */
struct xhci_ring**ring_cache;
int num_rings_cached;
- /* Store xHC assigned device address */
- int address;
#defineXHCI_MAX_RINGS_CACHED 31
struct xhci_virt_ep eps[31];
struct completion cmd_completion;
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] usb: xhci: change enumeration scheme to 'new scheme' by default
Change the enumeration scheme for xhci attached devices from:
SetAddress
GetDescriptor(8)
GetDescriptor(18)
...to:
GetDescriptor(64)
SetAddress
GetDescriptor(18)
...as some devices misbehave when encountering a SetAddress command
prior to GetDescriptor. There are known devices that require this
enumeration scheme but is unknown how much, if any, regression there
will be of xhci-attached devices that can not tolerate the change. For
now, follow the ehci case and enable 'new scheme' by default. Of course
the existing 'old_scheme_first' and 'use_both_schemes' usbcore module
parameters are available to debug / workaround regressions.
To support this enumeration scheme on xhci the AddressDevice operation
needs to be performed twice. The first instance of the command enables
the HC's device and slot context info for the device, but omits sending
the device a SetAddress command (BSR == block set address request).
Then, after GetDescriptor completes, follow up with the full
AddressDevice+SetAddress operation.
Reported-by: David Moore
Suggested-by: Sarah Sharp
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 22 --
drivers/usb/host/xhci-pci.c |1 +
drivers/usb/host/xhci-plat.c |1 +
drivers/usb/host/xhci-ring.c |6 +++---
drivers/usb/host/xhci.c | 19 +++
drivers/usb/host/xhci.h | 11 ++-
include/linux/usb/hcd.h |2 ++
7 files changed, 52 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 69bbb51..355a09d 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3943,6 +3943,20 @@ static int hub_set_address(struct usb_device *udev, int
devnum)
return retval;
}
+static int hub_enable_device(struct usb_device *udev)
+{
+ struct usb_hcd *hcd = bus_to_hcd(udev->bus);
+
+ if (!hcd->driver->enable_device)
+ return 0;
+ if (udev->state == USB_STATE_ADDRESS)
+ return 0;
+ if (udev->state != USB_STATE_DEFAULT)
+ return -EINVAL;
+
+ return hcd->driver->enable_device(hcd, udev);
+}
+
/* Reset device, (re)assign address, get device descriptor.
* Device connection must be stable, no more debouncing needed.
* Returns device in USB_STATE_ADDRESS, except on error.
@@ -4063,7 +4077,7 @@ hub_port_init (struct usb_hub *hub, struct usb_device
*udev, int port1,
* value.
*/
for (i = 0; i < GET_DESCRIPTOR_TRIES; (++i, msleep(100))) {
- if (USE_NEW_SCHEME(retry_counter) && !(hcd->driver->flags &
HCD_USB3)) {
+ if (USE_NEW_SCHEME(retry_counter)) {
struct usb_device_descriptor *buf;
int r = 0;
@@ -4074,6 +4088,10 @@ hub_port_init (struct usb_hub *hub, struct usb_device
*udev, int port1,
continue;
}
+ retval = hub_enable_device(udev);
+ if (retval < 0)
+ goto fail;
+
/* Retry on all errors; some devices are flakey.
* 255 is for WUSB devices, we actually need to use
* 512 (WUSB1.0[4.8.1]).
@@ -4155,7 +4173,7 @@ hub_port_init (struct usb_hub *hub, struct usb_device
*udev, int port1,
* - read ep0 maxpacket even for high and low speed,
*/
msleep(10);
- if (USE_NEW_SCHEME(retry_counter) &&
!(hcd->driver->flags & HCD_USB3))
+ if (USE_NEW_SCHEME(retry_counter))
break;
}
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index c2d4950..df564d1 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -306,6 +306,7 @@ static const struct hc_driver xhci_pci_hc_driver = {
.check_bandwidth = xhci_check_bandwidth,
.reset_bandwidth = xhci_reset_bandwidth,
.address_device = xhci_address_device,
+ .enable_device =xhci_enable_device,
.update_hub_device =xhci_update_hub_device,
.reset_device = xhci_discover_or_reset_device,
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index d9c169f..5413861 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -69,6 +69,7 @@ static const struct hc_driver xhci_plat_xhci_driver = {
.check_bandwidth = xhci_check_bandwidth,
.reset_bandwidth = xhci_reset_bandwidth,
.address_device = xhci_address_device,
+ .enable_device =xhci_enable_device,
.update_hub_device =xhci_update_hub_device,
.reset_device = xhci_discover_or_reset_device,
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7c043ec..644e2c6 1006
Re: [PATCH v3 0/2] staging: dwc2: add microframe scheduler
On Mon, Sep 23, 2013 at 02:23:32PM -0700, Paul Zimmerman wrote: > There were some style-related comments from Dan Carpenter, but I > would prefer to have those as an additional patch, once we have > this known-good code added to the driver. > Sure. Just add a link to the TODO list so we don't forget. http://www.mail-archive.com/linux-usb@vger.kernel.org/msg26650.html regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe 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 0/2] usb: implement AMD remote wakeup quirk
On Mon, Sep 23, 2013 at 12:43:15PM -0400, Alan Stern wrote: > This version of the patch set looks good. > > Alan Stern Looks fine to me as well. Acked-by: Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe 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] staging: dwc2: add TODO file
Add TODO file for DWC2 driver Signed-off-by: Paul Zimmerman --- Greg, this should be applied after the microframe scheduler patch, since it assumes that one has already been applied. drivers/staging/dwc2/TODO | 33 + 1 file changed, 33 insertions(+) create mode 100644 drivers/staging/dwc2/TODO diff --git a/drivers/staging/dwc2/TODO b/drivers/staging/dwc2/TODO new file mode 100644 index 000..282470d --- /dev/null +++ b/drivers/staging/dwc2/TODO @@ -0,0 +1,33 @@ +TODO: + - Dan Carpenter would like to see some cleanups to the microframe + scheduler code: + http://www.mail-archive.com/linux-usb@vger.kernel.org/msg26650.html + + - Should merge the NAK holdoff patch from Raspberry Pi + (http://marc.info/?l=linux-usb&m=137625067103833). But as it stands + that patch is incomplete, it needs more investigation to see if it + can be made to work for non-Raspberry Pi platforms that lack the + special FIQ interrupt that the Pi has. Without this patch, the driver + has a high interrupt rate (8K/sec). + + - The Raspberry Pi platform needs to have support for its FIQ interrupt + added, to get the same level of functionality as the downstream + driver. The raspberrypi.org developers have indicated they are + willing to help with that. + + - Some of the default driver parameters (see 'struct dwc2_core_params' + in core.h) won't work for many platforms. So DT attributes will need + to be added for some of these. But that can be done as-needed as new + platforms are added. + + - Eventually the driver should be merged with the s3c-hsotg peripheral + mode driver, so that both modes of operation can be supported with a + single driver. But I think that can wait till after the driver has + been moved to mainline. + + - After that, OTG support can be added. I'm not sure how much demand + there is for that, though, so I have that as a low priority. + +Please send any patches for this driver to Paul Zimmerman +and Greg Kroah-Hartman . And please CC linux-usb + too. -- 1.8.2.rc0.16.g20a599e -- To unsubscribe from this list: send the line "unsubscribe 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: fix write to USB3_PSSEN and XUSB2PRM pci config registers
On Mon, Sep 23, 2013 at 02:49:07PM -0700, Greg KH wrote: > On Mon, Sep 23, 2013 at 04:38:22PM -0500, Bjorn Helgaas wrote: > > On Mon, Sep 23, 2013 at 3:49 PM, Greg KH wrote: > > > On Mon, Sep 23, 2013 at 09:06:54PM +0300, Xenia Ragiadakou wrote: > > >> I had in mind that the pci_ops .read and .write defined by the PCI > > >> driver will take care of consistent byteorder access to the > > >> configuration registers. At least, that was what i understood after > > >> reading the > > >> chapter on PCI of Linux Device Drivers (more specifically for > > >> pci_write_config_* functions, it states that "The word and dword > > >> functions convert the value to little-endian before writing to the > > >> peripheral device."). > > > > > > Hm, I wrote that paragraph (or at least I think I did), but I sure > > > didn't remember this at all... > > > > > > Hm, wait, I do see this happening for the PowerPC cell PCI code, so it > > > might happen somewhere burried in the platform-specific code for > > > different arches. You will not see it happen on x86 as there's no need > > > to swap any bytes around. > > > > Greg, with regard to Xenia's patch, is this an ack or a nack? Since > > you didn't include an "Acked-by" line, I assume you think Xenia's > > patch is unnecessary. In that case, is there any way to shut sparse > > up so it doesn't complain about this? > > At this point in time, I don't remember what the original patch looked > like, and as it's an xhci patch, Sarah needs to ack it, not me :) Greg: So you're saying that there isn't a need to convert values from CPU byte-ordering to little endian byte-ordering before passing them on to pci_write_config_*? If so, then yes, Xenia's patch is fine and I'll pull that into my xHCI tree. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe 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] usbcore: check usb device's state before sending a Set SEL control transfer
From: Xenia Ragiadakou Set SEL control urbs cannot be sent to a device in unconfigured state. This patch adds a check in usb_req_set_sel() to ensure the usb device's state is USB_STATE_CONFIGURED. Signed-off-by: Xenia Ragiadakou Reported-by: Martin MOKREJS Suggested-by: Sarah Sharp Signed-off-by: Sarah Sharp --- drivers/usb/core/hub.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index dde4c83..e6b682c 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -3426,6 +3426,9 @@ static int usb_req_set_sel(struct usb_device *udev, enum usb3_link_state state) unsigned long long u2_pel; int ret; + if (udev->state != USB_STATE_CONFIGURED) + return 0; + /* Convert SEL and PEL stored in ns to us */ u1_sel = DIV_ROUND_UP(udev->u1_params.sel, 1000); u1_pel = DIV_ROUND_UP(udev->u1_params.pel, 1000); -- 1.8.3.3 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] usb: Fix xHCI host issues on remote wakeup.
When a device signals remote wakeup on a roothub, and the suspend change
bit is set, the host controller driver must not give control back to the
USB core until the port goes back into the active state.
EHCI accomplishes this by waiting in the get port status function until
the PORT_RESUME bit is cleared:
/* stop resume signaling */
temp &= ~(PORT_RWC_BITS | PORT_SUSPEND | PORT_RESUME);
ehci_writel(ehci, temp, status_reg);
clear_bit(wIndex, &ehci->resuming_ports);
retval = ehci_handshake(ehci, status_reg,
PORT_RESUME, 0, 2000 /* 2msec */);
Similarly, the xHCI host should wait until the port goes into U0, before
passing control up to the USB core. When the port transitions from the
RExit state to U0, the xHCI driver will get a port status change event.
We need to wait for that event before passing control up to the USB
core.
After the port transitions to the active state, the USB core should time
a recovery interval before it talks to the device. The length of that
recovery interval is TRSMRCY, 10 ms, mentioned in the USB 2.0 spec,
section 7.1.7.7. The previous xHCI code (which did not wait for the
port to go into U0) would cause the USB core to violate that recovery
interval.
This bug caused numerous USB device disconnects on remote wakeup under
ChromeOS and a Lynx Point LP xHCI host that takes up to 20 ms to move
from RExit to U0. ChromeOS is very aggressive about power savings, and
sets the autosuspend_delay to 100 ms, and disables USB persist.
I attempted to replicate this bug with Ubuntu 12.04, but could not. I
used Ubuntu 12.04 on the same platform, with the same BIOS that the bug
was triggered on ChromeOS with. I also changed the USB sysfs settings
as described above, but still could not reproduce the bug under Ubuntu.
It may be that ChromeOS userspace triggers this bug through additional
settings.
Signed-off-by: Sarah Sharp
---
drivers/usb/host/xhci-hub.c | 45 ++--
drivers/usb/host/xhci-mem.c | 2 ++
drivers/usb/host/xhci-ring.c | 13 +
drivers/usb/host/xhci.h | 10 ++
4 files changed, 60 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index ccf0a06..773a6b28 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -552,11 +552,15 @@ void xhci_del_comp_mod_timer(struct xhci_hcd *xhci, u32
status, u16 wIndex)
* - Mark a port as being done with device resume,
*and ring the endpoint doorbells.
* - Stop the Synopsys redriver Compliance Mode polling.
+ * - Drop and reacquire the xHCI lock, in order to wait for port resume.
*/
static u32 xhci_get_port_status(struct usb_hcd *hcd,
struct xhci_bus_state *bus_state,
__le32 __iomem **port_array,
- u16 wIndex, u32 raw_port_status)
+ u16 wIndex, u32 raw_port_status,
+ unsigned long flags)
+ __releases(&xhci->lock)
+ __acquires(&xhci->lock)
{
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
u32 status = 0;
@@ -591,21 +595,42 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
return 0x;
if (time_after_eq(jiffies,
bus_state->resume_done[wIndex])) {
+ int time_left;
+
xhci_dbg(xhci, "Resume USB2 port %d\n",
wIndex + 1);
bus_state->resume_done[wIndex] = 0;
clear_bit(wIndex, &bus_state->resuming_ports);
+
+ set_bit(wIndex, &bus_state->rexit_ports);
xhci_set_link_state(xhci, port_array, wIndex,
XDEV_U0);
- xhci_dbg(xhci, "set port %d resume\n",
- wIndex + 1);
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (!slot_id) {
- xhci_dbg(xhci, "slot_id is zero\n");
- return 0x;
+
+ spin_unlock_irqrestore(&xhci->lock, flags);
+ time_left = wait_for_completion_timeout(
+ &bus_state->rexit_done[wIndex],
+ msecs_to_jiffies(
+ XHCI_MAX_REXIT_TIMEOUT));
+ spin_lock_irqsave(&xhci->lock, flags);
+
+ if (time_left) {
+ slot_id = xhci_find_slot_id_by_port(hcd,
+ xhci, wIndex + 1);
+ if (!slot_id) {
+
