Re: [RFC 4/4] xhci: remove conversion from generic to pci device in xhci_mem.c

2013-11-14 Thread Sarah Sharp
On Thu, Nov 14, 2013 at 12:31:16PM -0800, Sarah Sharp wrote:
> Hi Xenia,
> 
> Sorry for the really late review on this.
> 
> On Mon, Aug 26, 2013 at 11:29:49PM +0300, Xenia Ragiadakou wrote:
> > This patch removes the to_pci_dev() conversion performed to generic struct
> > device since it is not actually useful (the pointer to the generic device
> > can be used directly rather through a conversion to pci_dev) and it is pci
> > bus specific.
> 
> Good catch!  I think the code only works on non-PCI hosts because it's
> just pointer math that takes the address of the pci_dev and then uses it
> to compute the underlying device pointer, and the code doesn't
> dereference the pci_dev structure pointer.
> 
> The patch shouldn't change the current behavior, so I'll queue it for
> usb-next and 3.14 instead of 3.13.

Hmm, this doesn't apply to my for-usb-next-queue branch anymore, so I
will need you resend this.

Thanks,
Sarah Sharp

> > Signed-off-by: Xenia Ragiadakou 
> > ---
> >  drivers/usb/host/xhci-mem.c | 20 ++--
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> > index 6dc5c8b..f201990 100644
> > --- a/drivers/usb/host/xhci-mem.c
> > +++ b/drivers/usb/host/xhci-mem.c
> > @@ -432,10 +432,10 @@ static void xhci_free_stream_ctx(struct xhci_hcd 
> > *xhci,
> > unsigned int num_stream_ctxs,
> > struct xhci_stream_ctx *stream_ctx, dma_addr_t dma)
> >  {
> > -   struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
> > +   struct device *dev = xhci_to_hcd(xhci)->self.controller;
> >  
> > if (num_stream_ctxs > MEDIUM_STREAM_ARRAY_SIZE)
> > -   dma_free_coherent(&pdev->dev,
> > +   dma_free_coherent(dev,
> > sizeof(struct xhci_stream_ctx)*num_stream_ctxs,
> > stream_ctx, dma);
> > else if (num_stream_ctxs <= SMALL_STREAM_ARRAY_SIZE)
> > @@ -460,10 +460,10 @@ static struct xhci_stream_ctx 
> > *xhci_alloc_stream_ctx(struct xhci_hcd *xhci,
> > unsigned int num_stream_ctxs, dma_addr_t *dma,
> > gfp_t mem_flags)
> >  {
> > -   struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
> > +   struct device *dev = xhci_to_hcd(xhci)->self.controller;
> >  
> > if (num_stream_ctxs > MEDIUM_STREAM_ARRAY_SIZE)
> > -   return dma_alloc_coherent(&pdev->dev,
> > +   return dma_alloc_coherent(dev,
> > sizeof(struct xhci_stream_ctx)*num_stream_ctxs,
> > dma, mem_flags);
> > else if (num_stream_ctxs <= SMALL_STREAM_ARRAY_SIZE)
> > @@ -1615,7 +1615,7 @@ static void scratchpad_free(struct xhci_hcd *xhci)
> >  {
> > int num_sp;
> > int i;
> > -   struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
> > +   struct device *dev = xhci_to_hcd(xhci)->self.controller;
> >  
> > if (!xhci->scratchpad)
> > return;
> > @@ -1623,13 +1623,13 @@ static void scratchpad_free(struct xhci_hcd *xhci)
> > num_sp = HCS_MAX_SCRATCHPAD(xhci->hcs_params2);
> >  
> > for (i = 0; i < num_sp; i++) {
> > -   dma_free_coherent(&pdev->dev, xhci->page_size,
> > +   dma_free_coherent(dev, xhci->page_size,
> > xhci->scratchpad->sp_buffers[i],
> > xhci->scratchpad->sp_dma_buffers[i]);
> > }
> > kfree(xhci->scratchpad->sp_dma_buffers);
> > kfree(xhci->scratchpad->sp_buffers);
> > -   dma_free_coherent(&pdev->dev, num_sp * sizeof(u64),
> > +   dma_free_coherent(dev, num_sp * sizeof(u64),
> > xhci->scratchpad->sp_array,
> > xhci->scratchpad->sp_dma);
> > kfree(xhci->scratchpad);
> > @@ -1691,7 +1691,7 @@ void xhci_free_command(struct xhci_hcd *xhci,
> >  
> >  void xhci_mem_cleanup(struct xhci_hcd *xhci)
> >  {
> > -   struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
> > +   struct device   *dev = xhci_to_hcd(xhci)->self.controller;
> > struct dev_info *dev_info, *next;
> > struct xhci_cd  *cur_cd, *next_cd;
> > unsigned long   flags;
> > @@ -1701,7 +1701,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
> > /* Free the Event Ring Segment Table and the actual Event Ring */
> > size = sizeof(struct xhci_erst_entry)*(xhci->erst.num_entries);
> > if (xhci->erst.entries)
> > -   dma_free_coherent(&pdev->dev, size,
> > +   dma_free_coherent(dev, size,
> > xhci->erst.entries, xhci->erst.erst_dma_addr);
> > xhci->erst.entries = NULL;
> > xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed ERST");
> > @@ -1749,7 +1749,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
> > "Freed medium stream array pool");
> >  
> > if (xhci->dcbaa)
> > -   dma_free_coherent(&pdev->dev, sizeof(*xhci->dcbaa),
> > +   dma_free_coherent(d

Re: [RFC 4/4] xhci: remove conversion from generic to pci device in xhci_mem.c

2013-11-14 Thread Sarah Sharp
Hi Xenia,

Sorry for the really late review on this.

On Mon, Aug 26, 2013 at 11:29:49PM +0300, Xenia Ragiadakou wrote:
> This patch removes the to_pci_dev() conversion performed to generic struct
> device since it is not actually useful (the pointer to the generic device
> can be used directly rather through a conversion to pci_dev) and it is pci
> bus specific.

Good catch!  I think the code only works on non-PCI hosts because it's
just pointer math that takes the address of the pci_dev and then uses it
to compute the underlying device pointer, and the code doesn't
dereference the pci_dev structure pointer.

The patch shouldn't change the current behavior, so I'll queue it for
usb-next and 3.14 instead of 3.13.

Sarah Sharp

> Signed-off-by: Xenia Ragiadakou 
> ---
>  drivers/usb/host/xhci-mem.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 6dc5c8b..f201990 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -432,10 +432,10 @@ static void xhci_free_stream_ctx(struct xhci_hcd *xhci,
>   unsigned int num_stream_ctxs,
>   struct xhci_stream_ctx *stream_ctx, dma_addr_t dma)
>  {
> - struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
> + struct device *dev = xhci_to_hcd(xhci)->self.controller;
>  
>   if (num_stream_ctxs > MEDIUM_STREAM_ARRAY_SIZE)
> - dma_free_coherent(&pdev->dev,
> + dma_free_coherent(dev,
>   sizeof(struct xhci_stream_ctx)*num_stream_ctxs,
>   stream_ctx, dma);
>   else if (num_stream_ctxs <= SMALL_STREAM_ARRAY_SIZE)
> @@ -460,10 +460,10 @@ static struct xhci_stream_ctx 
> *xhci_alloc_stream_ctx(struct xhci_hcd *xhci,
>   unsigned int num_stream_ctxs, dma_addr_t *dma,
>   gfp_t mem_flags)
>  {
> - struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
> + struct device *dev = xhci_to_hcd(xhci)->self.controller;
>  
>   if (num_stream_ctxs > MEDIUM_STREAM_ARRAY_SIZE)
> - return dma_alloc_coherent(&pdev->dev,
> + return dma_alloc_coherent(dev,
>   sizeof(struct xhci_stream_ctx)*num_stream_ctxs,
>   dma, mem_flags);
>   else if (num_stream_ctxs <= SMALL_STREAM_ARRAY_SIZE)
> @@ -1615,7 +1615,7 @@ static void scratchpad_free(struct xhci_hcd *xhci)
>  {
>   int num_sp;
>   int i;
> - struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
> + struct device *dev = xhci_to_hcd(xhci)->self.controller;
>  
>   if (!xhci->scratchpad)
>   return;
> @@ -1623,13 +1623,13 @@ static void scratchpad_free(struct xhci_hcd *xhci)
>   num_sp = HCS_MAX_SCRATCHPAD(xhci->hcs_params2);
>  
>   for (i = 0; i < num_sp; i++) {
> - dma_free_coherent(&pdev->dev, xhci->page_size,
> + dma_free_coherent(dev, xhci->page_size,
>   xhci->scratchpad->sp_buffers[i],
>   xhci->scratchpad->sp_dma_buffers[i]);
>   }
>   kfree(xhci->scratchpad->sp_dma_buffers);
>   kfree(xhci->scratchpad->sp_buffers);
> - dma_free_coherent(&pdev->dev, num_sp * sizeof(u64),
> + dma_free_coherent(dev, num_sp * sizeof(u64),
>   xhci->scratchpad->sp_array,
>   xhci->scratchpad->sp_dma);
>   kfree(xhci->scratchpad);
> @@ -1691,7 +1691,7 @@ void xhci_free_command(struct xhci_hcd *xhci,
>  
>  void xhci_mem_cleanup(struct xhci_hcd *xhci)
>  {
> - struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
> + struct device   *dev = xhci_to_hcd(xhci)->self.controller;
>   struct dev_info *dev_info, *next;
>   struct xhci_cd  *cur_cd, *next_cd;
>   unsigned long   flags;
> @@ -1701,7 +1701,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
>   /* Free the Event Ring Segment Table and the actual Event Ring */
>   size = sizeof(struct xhci_erst_entry)*(xhci->erst.num_entries);
>   if (xhci->erst.entries)
> - dma_free_coherent(&pdev->dev, size,
> + dma_free_coherent(dev, size,
>   xhci->erst.entries, xhci->erst.erst_dma_addr);
>   xhci->erst.entries = NULL;
>   xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed ERST");
> @@ -1749,7 +1749,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
>   "Freed medium stream array pool");
>  
>   if (xhci->dcbaa)
> - dma_free_coherent(&pdev->dev, sizeof(*xhci->dcbaa),
> + dma_free_coherent(dev, sizeof(*xhci->dcbaa),
>   xhci->dcbaa, xhci->dcbaa->dma);
>   xhci->dcbaa = NULL;
>  
> -- 
> 1.8.3.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.

[RFC 4/4] xhci: remove conversion from generic to pci device in xhci_mem.c

2013-08-26 Thread Xenia Ragiadakou
This patch removes the to_pci_dev() conversion performed to generic struct
device since it is not actually useful (the pointer to the generic device
can be used directly rather through a conversion to pci_dev) and it is pci
bus specific.

Signed-off-by: Xenia Ragiadakou 
---
 drivers/usb/host/xhci-mem.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 6dc5c8b..f201990 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -432,10 +432,10 @@ static void xhci_free_stream_ctx(struct xhci_hcd *xhci,
unsigned int num_stream_ctxs,
struct xhci_stream_ctx *stream_ctx, dma_addr_t dma)
 {
-   struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
+   struct device *dev = xhci_to_hcd(xhci)->self.controller;
 
if (num_stream_ctxs > MEDIUM_STREAM_ARRAY_SIZE)
-   dma_free_coherent(&pdev->dev,
+   dma_free_coherent(dev,
sizeof(struct xhci_stream_ctx)*num_stream_ctxs,
stream_ctx, dma);
else if (num_stream_ctxs <= SMALL_STREAM_ARRAY_SIZE)
@@ -460,10 +460,10 @@ static struct xhci_stream_ctx 
*xhci_alloc_stream_ctx(struct xhci_hcd *xhci,
unsigned int num_stream_ctxs, dma_addr_t *dma,
gfp_t mem_flags)
 {
-   struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
+   struct device *dev = xhci_to_hcd(xhci)->self.controller;
 
if (num_stream_ctxs > MEDIUM_STREAM_ARRAY_SIZE)
-   return dma_alloc_coherent(&pdev->dev,
+   return dma_alloc_coherent(dev,
sizeof(struct xhci_stream_ctx)*num_stream_ctxs,
dma, mem_flags);
else if (num_stream_ctxs <= SMALL_STREAM_ARRAY_SIZE)
@@ -1615,7 +1615,7 @@ static void scratchpad_free(struct xhci_hcd *xhci)
 {
int num_sp;
int i;
-   struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
+   struct device *dev = xhci_to_hcd(xhci)->self.controller;
 
if (!xhci->scratchpad)
return;
@@ -1623,13 +1623,13 @@ static void scratchpad_free(struct xhci_hcd *xhci)
num_sp = HCS_MAX_SCRATCHPAD(xhci->hcs_params2);
 
for (i = 0; i < num_sp; i++) {
-   dma_free_coherent(&pdev->dev, xhci->page_size,
+   dma_free_coherent(dev, xhci->page_size,
xhci->scratchpad->sp_buffers[i],
xhci->scratchpad->sp_dma_buffers[i]);
}
kfree(xhci->scratchpad->sp_dma_buffers);
kfree(xhci->scratchpad->sp_buffers);
-   dma_free_coherent(&pdev->dev, num_sp * sizeof(u64),
+   dma_free_coherent(dev, num_sp * sizeof(u64),
xhci->scratchpad->sp_array,
xhci->scratchpad->sp_dma);
kfree(xhci->scratchpad);
@@ -1691,7 +1691,7 @@ void xhci_free_command(struct xhci_hcd *xhci,
 
 void xhci_mem_cleanup(struct xhci_hcd *xhci)
 {
-   struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
+   struct device   *dev = xhci_to_hcd(xhci)->self.controller;
struct dev_info *dev_info, *next;
struct xhci_cd  *cur_cd, *next_cd;
unsigned long   flags;
@@ -1701,7 +1701,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
/* Free the Event Ring Segment Table and the actual Event Ring */
size = sizeof(struct xhci_erst_entry)*(xhci->erst.num_entries);
if (xhci->erst.entries)
-   dma_free_coherent(&pdev->dev, size,
+   dma_free_coherent(dev, size,
xhci->erst.entries, xhci->erst.erst_dma_addr);
xhci->erst.entries = NULL;
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed ERST");
@@ -1749,7 +1749,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
"Freed medium stream array pool");
 
if (xhci->dcbaa)
-   dma_free_coherent(&pdev->dev, sizeof(*xhci->dcbaa),
+   dma_free_coherent(dev, sizeof(*xhci->dcbaa),
xhci->dcbaa, xhci->dcbaa->dma);
xhci->dcbaa = NULL;
 
-- 
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