[PATCH] rt2860sta: Fix argument to linux_pci_unmap_single()

[PATCH] rt2860sta: Fix argument to linux_pci_unmap_single()

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]

John Halton wrote in <http://bugs.debian.org/575726>:
> Whenever wpa_supplicant is deactivated (whether by killing the process or 
> during a normal shutdown) I am getting a kerneloops that prevents the 
> computer from completing shutdown. Here is the relevant syslog output:

The backtrace points to an incorrect call from RTMPFreeTxRxRingMemory()
into linux_pci_unmap_single().  This appears to have been fixed in Linux
2.6.33 by this change:

commit ca97b8388838ee9ea4b4bad04948f8f7f8a607a3
Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Date:   Tue Sep 22 20:44:07 2009 +0200

    Staging: rt28x0: updates from vendor's V2.1.0.0 drivers

For stable-2.6.32, just fix this one function call.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/staging/rt2860/common/2860_rtmp_init.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/rt2860/common/2860_rtmp_init.c b/drivers/staging/rt2860/common/2860_rtmp_init.c
index 0bc0fb9..98b0f8e 100644
--- a/drivers/staging/rt2860/common/2860_rtmp_init.c
+++ b/drivers/staging/rt2860/common/2860_rtmp_init.c
@@ -716,7 +716,7 @@ VOID RTMPFreeTxRxRingMemory(
 	{
 		if ((pAd->RxRing.Cell[index].DmaBuf.AllocVa) && (pAd->RxRing.Cell[index].pNdisPacket))
 		{
-			PCI_UNMAP_SINGLE(pObj->pci_dev, pAd->RxRing.Cell[index].DmaBuf.AllocPa, pAd->RxRing.Cell[index].DmaBuf.AllocSize, PCI_DMA_FROMDEVICE);
+			PCI_UNMAP_SINGLE(pAd, pAd->RxRing.Cell[index].DmaBuf.AllocPa, pAd->RxRing.Cell[index].DmaBuf.AllocSize, PCI_DMA_FROMDEVICE);
 			RELEASE_NDIS_PACKET(pAd, pAd->RxRing.Cell[index].pNdisPacket, NDIS_STATUS_SUCCESS);
 		}
 	}
-- 
1.7.0.3


-- 
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH] rt28xx: Make PCI_{MAP,UNMAP}_SINGLE type-safe

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 3604 bytes --]

To avoid recurrence of bugs such as <http://bugs.debian.org/575726>,
change the type of the first parameter to linux_pci_{map,unmap}_single()
from void * to struct rt_rtmp_adapter *.  Also do not define the macros
PCI_{MAP,UNMAP}_SINGLE() when building the rt2870sta driver; they are
not used and if they were that would be a bug.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk> 
---
 drivers/staging/rt2860/rt_linux.h    |   14 +++++---------
 drivers/staging/rt2860/rt_pci_rbus.c |   12 ++++--------
 2 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/rt2860/rt_linux.h b/drivers/staging/rt2860/rt_linux.h
index a7c540f..b370fb2 100644
--- a/drivers/staging/rt2860/rt_linux.h
+++ b/drivers/staging/rt2860/rt_linux.h
@@ -455,10 +455,11 @@ void hex_dump(char *str, unsigned char *pSrcBufVA, unsigned int SrcBufLen);
  * Device DMA Access related definitions and data structures.
  **********************************************************************************/
 #ifdef RTMP_MAC_PCI
-dma_addr_t linux_pci_map_single(void *handle, void *ptr, size_t size,
-				int sd_idx, int direction);
-void linux_pci_unmap_single(void *handle, dma_addr_t dma_addr, size_t size,
-			    int direction);
+struct rt_rtmp_adapter;
+dma_addr_t linux_pci_map_single(struct rt_rtmp_adapter *pAd, void *ptr,
+				size_t size, int sd_idx, int direction);
+void linux_pci_unmap_single(struct rt_rtmp_adapter *pAd, dma_addr_t dma_addr,
+			    size_t size, int direction);
 
 #define PCI_MAP_SINGLE(_handle, _ptr, _size, _sd_idx, _dir) \
 	linux_pci_map_single(_handle, _ptr, _size, _sd_idx, _dir)
@@ -475,11 +476,6 @@ void linux_pci_unmap_single(void *handle, dma_addr_t dma_addr, size_t size,
 #define DEV_ALLOC_SKB(_length) \
 	dev_alloc_skb(_length)
 #endif /* RTMP_MAC_PCI // */
-#ifdef RTMP_MAC_USB
-#define PCI_MAP_SINGLE(_handle, _ptr, _size, _dir) (unsigned long)0
-
-#define PCI_UNMAP_SINGLE(_handle, _ptr, _size, _dir)
-#endif /* RTMP_MAC_USB // */
 
 /*
  * unsigned long
diff --git a/drivers/staging/rt2860/rt_pci_rbus.c b/drivers/staging/rt2860/rt_pci_rbus.c
index e0a0aee..acdf09f 100644
--- a/drivers/staging/rt2860/rt_pci_rbus.c
+++ b/drivers/staging/rt2860/rt_pci_rbus.c
@@ -790,10 +790,9 @@ IRQ_HANDLE_TYPE rt2860_interrupt(int irq, void *dev_instance)
  * invaild or writeback cache
  * and convert virtual address to physical address
  */
-dma_addr_t linux_pci_map_single(void *handle, void *ptr, size_t size,
-				int sd_idx, int direction)
+dma_addr_t linux_pci_map_single(struct rt_rtmp_adapter *pAd, void *ptr,
+				size_t size, int sd_idx, int direction)
 {
-	struct rt_rtmp_adapter *pAd;
 	struct os_cookie *pObj;
 
 	/*
@@ -812,7 +811,6 @@ dma_addr_t linux_pci_map_single(void *handle, void *ptr, size_t size,
 	   sd_idx = -1
 	 */
 
-	pAd = (struct rt_rtmp_adapter *)handle;
 	pObj = (struct os_cookie *)pAd->OS_Cookie;
 
 	if (sd_idx == 1) {
@@ -826,13 +824,11 @@ dma_addr_t linux_pci_map_single(void *handle, void *ptr, size_t size,
 
 }
 
-void linux_pci_unmap_single(void *handle, dma_addr_t dma_addr, size_t size,
-			    int direction)
+void linux_pci_unmap_single(struct rt_rtmp_adapter *pAd, dma_addr_t dma_addr,
+			    size_t size, int direction)
 {
-	struct rt_rtmp_adapter *pAd;
 	struct os_cookie *pObj;
 
-	pAd = (struct rt_rtmp_adapter *)handle;
 	pObj = (struct os_cookie *)pAd->OS_Cookie;
 
 	pci_unmap_single(pObj->pci_dev, dma_addr, size, direction);
-- 
1.7.0.3


-- 
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] rt28xx: Make PCI_{MAP,UNMAP}_SINGLE type-safe

[FUJITA Tomonori]

On Mon, 29 Mar 2010 01:24:45 +0100
Ben Hutchings <ben@decadent.org.uk> wrote:

> To avoid recurrence of bugs such as <http://bugs.debian.org/575726>,
> change the type of the first parameter to linux_pci_{map,unmap}_single()
> from void * to struct rt_rtmp_adapter *.  Also do not define the macros
> PCI_{MAP,UNMAP}_SINGLE() when building the rt2870sta driver; they are
> not used and if they were that would be a bug.
> 
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk> 
> ---
>  drivers/staging/rt2860/rt_linux.h    |   14 +++++---------
>  drivers/staging/rt2860/rt_pci_rbus.c |   12 ++++--------
>  2 files changed, 9 insertions(+), 17 deletions(-)

I think using dma_map_single directly instead of inventing a wrapper
function make the code more readable.

Re: [PATCH] rt28xx: Make PCI_{MAP,UNMAP}_SINGLE type-safe

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1105 bytes --]

On Mon, 2010-03-29 at 10:52 +0900, FUJITA Tomonori wrote:
> On Mon, 29 Mar 2010 01:24:45 +0100
> Ben Hutchings <ben@decadent.org.uk> wrote:
> 
> > To avoid recurrence of bugs such as <http://bugs.debian.org/575726>,
> > change the type of the first parameter to linux_pci_{map,unmap}_single()
> > from void * to struct rt_rtmp_adapter *.  Also do not define the macros
> > PCI_{MAP,UNMAP}_SINGLE() when building the rt2870sta driver; they are
> > not used and if they were that would be a bug.
> > 
> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk> 
> > ---
> >  drivers/staging/rt2860/rt_linux.h    |   14 +++++---------
> >  drivers/staging/rt2860/rt_pci_rbus.c |   12 ++++--------
> >  2 files changed, 9 insertions(+), 17 deletions(-)
> 
> I think using dma_map_single directly instead of inventing a wrapper
> function make the code more readable.

These functions are not quite simple wrappers, unfortunately.  So while
I think that is worth doing it is a separate change.

Ben.

-- 
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] rt28xx: Make PCI_{MAP,UNMAP}_SINGLE type-safe

[FUJITA Tomonori]

On Mon, 29 Mar 2010 02:58:48 +0100
Ben Hutchings <ben@decadent.org.uk> wrote:

> On Mon, 2010-03-29 at 10:52 +0900, FUJITA Tomonori wrote:
> > On Mon, 29 Mar 2010 01:24:45 +0100
> > Ben Hutchings <ben@decadent.org.uk> wrote:
> > 
> > > To avoid recurrence of bugs such as <http://bugs.debian.org/575726>,
> > > change the type of the first parameter to linux_pci_{map,unmap}_single()
> > > from void * to struct rt_rtmp_adapter *.  Also do not define the macros
> > > PCI_{MAP,UNMAP}_SINGLE() when building the rt2870sta driver; they are
> > > not used and if they were that would be a bug.
> > > 
> > > Signed-off-by: Ben Hutchings <ben@decadent.org.uk> 
> > > ---
> > >  drivers/staging/rt2860/rt_linux.h    |   14 +++++---------
> > >  drivers/staging/rt2860/rt_pci_rbus.c |   12 ++++--------
> > >  2 files changed, 9 insertions(+), 17 deletions(-)
> > 
> > I think using dma_map_single directly instead of inventing a wrapper
> > function make the code more readable.
> 
> These functions are not quite simple wrappers, unfortunately.  So while
> I think that is worth doing it is a separate change.

Well, the current wrapper functions looks terrible. Needs to fix them
before moving the driver out of the staging anyway, I think.

patch rt2860sta-fix-argument-to-linux_pci_unmap_single.patch added to 2.6.32-stable tree

[gregkh]

This is a note to let you know that we have just queued up the patch titled

    Subject: rt2860sta: Fix argument to linux_pci_unmap_single()

to the 2.6.32-stable tree.  Its filename is

    rt2860sta-fix-argument-to-linux_pci_unmap_single.patch

A git repo of this tree can be found at 
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary


>From ben@decadent.org.uk  Mon Mar 29 13:35:12 2010
From: Ben Hutchings <ben@decadent.org.uk>
Date: Mon, 29 Mar 2010 01:09:17 +0100
Subject: rt2860sta: Fix argument to linux_pci_unmap_single()
To: stable@kernel.org
Cc: 575726@bugs.debian.org, linux-wireless <linux-wireless@vger.kernel.org>, John Halton <johnhalton@gmail.com>, Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Message-ID: <1269821357.8653.231.camel@localhost>

From: Ben Hutchings <ben@decadent.org.uk>

John Halton wrote in <http://bugs.debian.org/575726>:
> Whenever wpa_supplicant is deactivated (whether by killing the process or
> during a normal shutdown) I am getting a kerneloops that prevents the
> computer from completing shutdown. Here is the relevant syslog output:

The backtrace points to an incorrect call from RTMPFreeTxRxRingMemory()
into linux_pci_unmap_single().  This appears to have been fixed in Linux
2.6.33 by this change:

commit ca97b8388838ee9ea4b4bad04948f8f7f8a607a3
Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Date:   Tue Sep 22 20:44:07 2009 +0200

    Staging: rt28x0: updates from vendor's V2.1.0.0 drivers

For stable-2.6.32, just fix this one function call.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/staging/rt2860/common/2860_rtmp_init.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/staging/rt2860/common/2860_rtmp_init.c
+++ b/drivers/staging/rt2860/common/2860_rtmp_init.c
@@ -716,7 +716,7 @@ VOID RTMPFreeTxRxRingMemory(
 	{
 		if ((pAd->RxRing.Cell[index].DmaBuf.AllocVa) && (pAd->RxRing.Cell[index].pNdisPacket))
 		{
-			PCI_UNMAP_SINGLE(pObj->pci_dev, pAd->RxRing.Cell[index].DmaBuf.AllocPa, pAd->RxRing.Cell[index].DmaBuf.AllocSize, PCI_DMA_FROMDEVICE);
+			PCI_UNMAP_SINGLE(pAd, pAd->RxRing.Cell[index].DmaBuf.AllocPa, pAd->RxRing.Cell[index].DmaBuf.AllocSize, PCI_DMA_FROMDEVICE);
 			RELEASE_NDIS_PACKET(pAd, pAd->RxRing.Cell[index].pNdisPacket, NDIS_STATUS_SUCCESS);
 		}
 	}


Patches currently in stable-queue which might be from ben@decadent.org.uk are

Re: [PATCH] rt28xx: Make PCI_{MAP,UNMAP}_SINGLE type-safe

[Greg KH]

On Mon, Mar 29, 2010 at 01:24:45AM +0100, Ben Hutchings wrote:
> To avoid recurrence of bugs such as <http://bugs.debian.org/575726>,
> change the type of the first parameter to linux_pci_{map,unmap}_single()
> from void * to struct rt_rtmp_adapter *.  Also do not define the macros
> PCI_{MAP,UNMAP}_SINGLE() when building the rt2870sta driver; they are
> not used and if they were that would be a bug.
> 
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk> 

In the future, please do not sign your emails, it just makes it harder
to apply them :(

thanks,

greg k-h

Re: [PATCH] rt28xx: Make PCI_{MAP,UNMAP}_SINGLE type-safe

[drago01]

On Mon, Mar 29, 2010 at 4:04 AM, FUJITA Tomonori
<fujita.tomonori@lab.ntt.co.jp> wrote:
> On Mon, 29 Mar 2010 02:58:48 +0100
> Ben Hutchings <ben@decadent.org.uk> wrote:
>
>> On Mon, 2010-03-29 at 10:52 +0900, FUJITA Tomonori wrote:
>> > On Mon, 29 Mar 2010 01:24:45 +0100
>> > Ben Hutchings <ben@decadent.org.uk> wrote:
>> >
>> > > To avoid recurrence of bugs such as <http://bugs.debian.org/575726>,
>> > > change the type of the first parameter to linux_pci_{map,unmap}_single()
>> > > from void * to struct rt_rtmp_adapter *.  Also do not define the macros
>> > > PCI_{MAP,UNMAP}_SINGLE() when building the rt2870sta driver; they are
>> > > not used and if they were that would be a bug.
>> > >
>> > > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
>> > > ---
>> > >  drivers/staging/rt2860/rt_linux.h    |   14 +++++---------
>> > >  drivers/staging/rt2860/rt_pci_rbus.c |   12 ++++--------
>> > >  2 files changed, 9 insertions(+), 17 deletions(-)
>> >
>> > I think using dma_map_single directly instead of inventing a wrapper
>> > function make the code more readable.
>>
>> These functions are not quite simple wrappers, unfortunately.  So while
>> I think that is worth doing it is a separate change.
>
> Well, the current wrapper functions looks terrible. Needs to fix them
> before moving the driver out of the staging anyway, I think.

The driver is not expected to make it ever out of staging ... unless
someone ports it to mac80211 ... the in kernel rt2x00 drivers are the
longer term solutions for supporting this hardware.