RE: [PATCH v2] usb:musb:musb_host: Handle highmem in PIO mode

2012-08-08 Thread Virupax SADASHIVPETIMATH 


> -Original Message-
> From: Alan Stern [mailto:st...@rowland.harvard.edu]
> Sent: Tuesday, August 07, 2012 11:17 PM
> To: Virupax SADASHIVPETIMATH
> Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-...@vger.kernel.org; 
> linux-
> ker...@vger.kernel.org; linus.wall...@linaro.org; Praveena NADAHALLY; Rajaram
> REGUPATHY; Vikrant BAPAT
> Subject: Re: [PATCH v2] usb:musb:musb_host: Handle highmem in PIO mode
> 
> 
> > +*/
> > +   if (!urb->transfer_buffer)
> > +   use_sg = true;
> 
> Here you test urb->transfer_buffer.

I will make the test as 
if (!use_sg && !urb->transfer_buffer)
use_sg = true;

> > +   if (use_sg) {
> > +   /* sg_miter_start is already done in musb_ep_program */
> > +   if (!sg_miter_next(>sg_miter)) {
> > +   dev_err(musb->controller, "error: sg list empty\n");
> > +   sg_miter_stop(>sg_miter);
> > +   status = -EINVAL;
> > +   goto done;
> > +   }
> > +   urb->transfer_buffer = qh->sg_miter.addr;
> 
> And here you set it.  As a result, on the next iteration of this
> routine the test above won't work right.  (This function gets invoked
> once for each entry in the sg list, right?)
> 
> Is there any reason to set urb->transfer_buffer here?  You could just
> use qh->sg_miter.addr directly in the musb_write_fifo() call two lines
> below.

I will change it. 

Thanks 
Virupax S 


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

RE: [PATCH v2] usb:musb:musb_host: Handle highmem in PIO mode

2012-08-08 Thread Virupax SADASHIVPETIMATH 


 -Original Message-
 From: Alan Stern [mailto:st...@rowland.harvard.edu]
 Sent: Tuesday, August 07, 2012 11:17 PM
 To: Virupax SADASHIVPETIMATH
 Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-...@vger.kernel.org; 
 linux-
 ker...@vger.kernel.org; linus.wall...@linaro.org; Praveena NADAHALLY; Rajaram
 REGUPATHY; Vikrant BAPAT
 Subject: Re: [PATCH v2] usb:musb:musb_host: Handle highmem in PIO mode
 
 
  +*/
  +   if (!urb-transfer_buffer)
  +   use_sg = true;
 
 Here you test urb-transfer_buffer.

I will make the test as 
if (!use_sg  !urb-transfer_buffer)
use_sg = true;

  +   if (use_sg) {
  +   /* sg_miter_start is already done in musb_ep_program */
  +   if (!sg_miter_next(qh-sg_miter)) {
  +   dev_err(musb-controller, error: sg list empty\n);
  +   sg_miter_stop(qh-sg_miter);
  +   status = -EINVAL;
  +   goto done;
  +   }
  +   urb-transfer_buffer = qh-sg_miter.addr;
 
 And here you set it.  As a result, on the next iteration of this
 routine the test above won't work right.  (This function gets invoked
 once for each entry in the sg list, right?)
 
 Is there any reason to set urb-transfer_buffer here?  You could just
 use qh-sg_miter.addr directly in the musb_write_fifo() call two lines
 below.

I will change it. 

Thanks 
Virupax S 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] usb:musb:musb_host: Handle highmem in PIO mode

2012-08-07 Thread Alan Stern 
On Tue, 7 Aug 2012, Virupax Sadashivpetimath wrote:

> In case of USB bulk transfer, when himem page
> is received, the usb_sg_init function sets the
> urb transfer buffer to NULL. When such URB
> transfer is handled, kernel crashes in PIO mode.
> Handle this by mapping the highmem buffer in PIO mode.

...

> @@ -1332,9 +1353,38 @@ void musb_host_tx(struct musb *musb, u8 epnum)
>   length = qh->maxpacket;
>   /* Unmap the buffer so that CPU can use it */
>   usb_hcd_unmap_urb_for_dma(musb_to_hcd(musb), urb);
> - musb_write_fifo(hw_ep, length, urb->transfer_buffer + offset);
> +
> + /*
> +  * We need to map sg if the transfer_buffer is
> +  * NULL.
> +  */
> + if (!urb->transfer_buffer)
> + use_sg = true;

Here you test urb->transfer_buffer.

> + if (use_sg) {
> + /* sg_miter_start is already done in musb_ep_program */
> + if (!sg_miter_next(>sg_miter)) {
> + dev_err(musb->controller, "error: sg list empty\n");
> + sg_miter_stop(>sg_miter);
> + status = -EINVAL;
> + goto done;
> + }
> + urb->transfer_buffer = qh->sg_miter.addr;

And here you set it.  As a result, on the next iteration of this
routine the test above won't work right.  (This function gets invoked
once for each entry in the sg list, right?)

Is there any reason to set urb->transfer_buffer here?  You could just
use qh->sg_miter.addr directly in the musb_write_fifo() call two lines
below.

> + length = min_t(u32, length, qh->sg_miter.length);
> + musb_write_fifo(hw_ep, length, urb->transfer_buffer);
> + qh->sg_miter.consumed = length;
> + sg_miter_stop(>sg_miter);
> + } else {
> + musb_write_fifo(hw_ep, length, urb->transfer_buffer + offset);
> + }

Alan Stern

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

[PATCH v2] usb:musb:musb_host: Handle highmem in PIO mode

2012-08-07 Thread Virupax Sadashivpetimath 
In case of USB bulk transfer, when himem page
is received, the usb_sg_init function sets the
urb transfer buffer to NULL. When such URB
transfer is handled, kernel crashes in PIO mode.
Handle this by mapping the highmem buffer in PIO mode.

Signed-off-by: Virupax Sadashivpetimath 

Signed-off-by: Praveena NADAHALLY 
Acked-by: Linus Walleij 
---
 drivers/usb/musb/musb_host.c |   98 +++--
 drivers/usb/musb/musb_host.h |3 +
 2 files changed, 96 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 4bb717d..199bf1a 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -813,9 +813,28 @@ static void musb_ep_program(struct musb *musb, u8 epnum,
if (load_count) {
/* PIO to load FIFO */
qh->segsize = load_count;
-   musb_write_fifo(hw_ep, load_count, buf);
+   if (!buf) {
+   sg_miter_start(>sg_miter, urb->sg, 1,
+   SG_MITER_ATOMIC
+   | SG_MITER_FROM_SG);
+   if (!sg_miter_next(>sg_miter)) {
+   dev_err(musb->controller,
+   "error: sg"
+   "list empty\n");
+   sg_miter_stop(>sg_miter);
+   goto finish;
+   }
+   buf = qh->sg_miter.addr + urb->sg->offset +
+   urb->actual_length;
+   load_count = min_t(u32, load_count,
+   qh->sg_miter.length);
+   musb_write_fifo(hw_ep, load_count, buf);
+   qh->sg_miter.consumed = load_count;
+   sg_miter_stop(>sg_miter);
+   } else
+   musb_write_fifo(hw_ep, load_count, buf);
}
-
+finish:
/* re-enable interrupt */
musb_writew(mbase, MUSB_INTRTXE, int_txe);
 
@@ -1116,6 +1135,7 @@ void musb_host_tx(struct musb *musb, u8 epnum)
void __iomem*mbase = musb->mregs;
struct dma_channel  *dma;
booltransfer_pending = false;
+   static bool use_sg;
 
musb_ep_select(mbase, epnum);
tx_csr = musb_readw(epio, MUSB_TXCSR);
@@ -1163,6 +1183,7 @@ void musb_host_tx(struct musb *musb, u8 epnum)
return;
}
 
+done:
if (status) {
if (dma_channel_status(dma) == MUSB_DMA_STATUS_BUSY) {
dma->status = MUSB_DMA_STATUS_CORE_ABORT;
@@ -1332,9 +1353,38 @@ void musb_host_tx(struct musb *musb, u8 epnum)
length = qh->maxpacket;
/* Unmap the buffer so that CPU can use it */
usb_hcd_unmap_urb_for_dma(musb_to_hcd(musb), urb);
-   musb_write_fifo(hw_ep, length, urb->transfer_buffer + offset);
+
+   /*
+* We need to map sg if the transfer_buffer is
+* NULL.
+*/
+   if (!urb->transfer_buffer)
+   use_sg = true;
+
+   if (use_sg) {
+   /* sg_miter_start is already done in musb_ep_program */
+   if (!sg_miter_next(>sg_miter)) {
+   dev_err(musb->controller, "error: sg list empty\n");
+   sg_miter_stop(>sg_miter);
+   status = -EINVAL;
+   goto done;
+   }
+   urb->transfer_buffer = qh->sg_miter.addr;
+   length = min_t(u32, length, qh->sg_miter.length);
+   musb_write_fifo(hw_ep, length, urb->transfer_buffer);
+   qh->sg_miter.consumed = length;
+   sg_miter_stop(>sg_miter);
+   } else {
+   musb_write_fifo(hw_ep, length, urb->transfer_buffer + offset);
+   }
+
qh->segsize = length;
 
+   if (use_sg) {
+   if (offset + length >= urb->transfer_buffer_length)
+   use_sg = false;
+   }
+
musb_ep_select(mbase, epnum);
musb_writew(epio, MUSB_TXCSR,
MUSB_TXCSR_H_WZC_BITS | MUSB_TXCSR_TXPKTRDY);
@@ -1442,6 +1492,8 @@ void musb_host_rx(struct musb *musb, u8 epnum)
booldone = false;
u32 status;
struct dma_channel  *dma;
+   static bool use_sg;
+   unsigned int sg_flags = SG_MITER_ATOMIC | SG_MITER_TO_SG;
 
musb_ep_select(mbase, epnum);
 
@@ -1756,10 +1808,43 @@ void musb_host_rx(struct musb *musb, u8 epnum)
 #endif /* Mentor DMA */
 
if (!dma) {
+   unsigned int received_len;
+

[PATCH v2] usb:musb:musb_host: Handle highmem in PIO mode

2012-08-07 Thread Virupax Sadashivpetimath 
In case of USB bulk transfer, when himem page
is received, the usb_sg_init function sets the
urb transfer buffer to NULL. When such URB
transfer is handled, kernel crashes in PIO mode.
Handle this by mapping the highmem buffer in PIO mode.

Signed-off-by: Virupax Sadashivpetimath 
virupax.sadashivpetim...@stericsson.com
Signed-off-by: Praveena NADAHALLY praveen.nadaha...@stericsson.com
Acked-by: Linus Walleij linus.wall...@linaro.org
---
 drivers/usb/musb/musb_host.c |   98 +++--
 drivers/usb/musb/musb_host.h |3 +
 2 files changed, 96 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 4bb717d..199bf1a 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -813,9 +813,28 @@ static void musb_ep_program(struct musb *musb, u8 epnum,
if (load_count) {
/* PIO to load FIFO */
qh-segsize = load_count;
-   musb_write_fifo(hw_ep, load_count, buf);
+   if (!buf) {
+   sg_miter_start(qh-sg_miter, urb-sg, 1,
+   SG_MITER_ATOMIC
+   | SG_MITER_FROM_SG);
+   if (!sg_miter_next(qh-sg_miter)) {
+   dev_err(musb-controller,
+   error: sg
+   list empty\n);
+   sg_miter_stop(qh-sg_miter);
+   goto finish;
+   }
+   buf = qh-sg_miter.addr + urb-sg-offset +
+   urb-actual_length;
+   load_count = min_t(u32, load_count,
+   qh-sg_miter.length);
+   musb_write_fifo(hw_ep, load_count, buf);
+   qh-sg_miter.consumed = load_count;
+   sg_miter_stop(qh-sg_miter);
+   } else
+   musb_write_fifo(hw_ep, load_count, buf);
}
-
+finish:
/* re-enable interrupt */
musb_writew(mbase, MUSB_INTRTXE, int_txe);
 
@@ -1116,6 +1135,7 @@ void musb_host_tx(struct musb *musb, u8 epnum)
void __iomem*mbase = musb-mregs;
struct dma_channel  *dma;
booltransfer_pending = false;
+   static bool use_sg;
 
musb_ep_select(mbase, epnum);
tx_csr = musb_readw(epio, MUSB_TXCSR);
@@ -1163,6 +1183,7 @@ void musb_host_tx(struct musb *musb, u8 epnum)
return;
}
 
+done:
if (status) {
if (dma_channel_status(dma) == MUSB_DMA_STATUS_BUSY) {
dma-status = MUSB_DMA_STATUS_CORE_ABORT;
@@ -1332,9 +1353,38 @@ void musb_host_tx(struct musb *musb, u8 epnum)
length = qh-maxpacket;
/* Unmap the buffer so that CPU can use it */
usb_hcd_unmap_urb_for_dma(musb_to_hcd(musb), urb);
-   musb_write_fifo(hw_ep, length, urb-transfer_buffer + offset);
+
+   /*
+* We need to map sg if the transfer_buffer is
+* NULL.
+*/
+   if (!urb-transfer_buffer)
+   use_sg = true;
+
+   if (use_sg) {
+   /* sg_miter_start is already done in musb_ep_program */
+   if (!sg_miter_next(qh-sg_miter)) {
+   dev_err(musb-controller, error: sg list empty\n);
+   sg_miter_stop(qh-sg_miter);
+   status = -EINVAL;
+   goto done;
+   }
+   urb-transfer_buffer = qh-sg_miter.addr;
+   length = min_t(u32, length, qh-sg_miter.length);
+   musb_write_fifo(hw_ep, length, urb-transfer_buffer);
+   qh-sg_miter.consumed = length;
+   sg_miter_stop(qh-sg_miter);
+   } else {
+   musb_write_fifo(hw_ep, length, urb-transfer_buffer + offset);
+   }
+
qh-segsize = length;
 
+   if (use_sg) {
+   if (offset + length = urb-transfer_buffer_length)
+   use_sg = false;
+   }
+
musb_ep_select(mbase, epnum);
musb_writew(epio, MUSB_TXCSR,
MUSB_TXCSR_H_WZC_BITS | MUSB_TXCSR_TXPKTRDY);
@@ -1442,6 +1492,8 @@ void musb_host_rx(struct musb *musb, u8 epnum)
booldone = false;
u32 status;
struct dma_channel  *dma;
+   static bool use_sg;
+   unsigned int sg_flags = SG_MITER_ATOMIC | SG_MITER_TO_SG;
 
musb_ep_select(mbase, epnum);
 
@@ -1756,10 +1808,43 @@ void musb_host_rx(struct musb *musb, u8 epnum)
 #endif /* Mentor DMA */

Re: [PATCH v2] usb:musb:musb_host: Handle highmem in PIO mode

2012-08-07 Thread Alan Stern 
On Tue, 7 Aug 2012, Virupax Sadashivpetimath wrote:

 In case of USB bulk transfer, when himem page
 is received, the usb_sg_init function sets the
 urb transfer buffer to NULL. When such URB
 transfer is handled, kernel crashes in PIO mode.
 Handle this by mapping the highmem buffer in PIO mode.

...

 @@ -1332,9 +1353,38 @@ void musb_host_tx(struct musb *musb, u8 epnum)
   length = qh-maxpacket;
   /* Unmap the buffer so that CPU can use it */
   usb_hcd_unmap_urb_for_dma(musb_to_hcd(musb), urb);
 - musb_write_fifo(hw_ep, length, urb-transfer_buffer + offset);
 +
 + /*
 +  * We need to map sg if the transfer_buffer is
 +  * NULL.
 +  */
 + if (!urb-transfer_buffer)
 + use_sg = true;

Here you test urb-transfer_buffer.

 + if (use_sg) {
 + /* sg_miter_start is already done in musb_ep_program */
 + if (!sg_miter_next(qh-sg_miter)) {
 + dev_err(musb-controller, error: sg list empty\n);
 + sg_miter_stop(qh-sg_miter);
 + status = -EINVAL;
 + goto done;
 + }
 + urb-transfer_buffer = qh-sg_miter.addr;

And here you set it.  As a result, on the next iteration of this
routine the test above won't work right.  (This function gets invoked
once for each entry in the sg list, right?)

Is there any reason to set urb-transfer_buffer here?  You could just
use qh-sg_miter.addr directly in the musb_write_fifo() call two lines
below.

 + length = min_t(u32, length, qh-sg_miter.length);
 + musb_write_fifo(hw_ep, length, urb-transfer_buffer);
 + qh-sg_miter.consumed = length;
 + sg_miter_stop(qh-sg_miter);
 + } else {
 + musb_write_fifo(hw_ep, length, urb-transfer_buffer + offset);
 + }

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/