2010-03-29 00:09:29

by Ben Hutchings

[permalink] [raw]
Subject: [PATCH] rt2860sta: Fix argument to linux_pci_unmap_single()

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 <[email protected]>
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 <[email protected]>
---
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.


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2010-03-29 01:52:25

by FUJITA Tomonori

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

On Mon, 29 Mar 2010 01:24:45 +0100
Ben Hutchings <[email protected]> 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 <[email protected]>
> ---
> 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.

2010-03-29 20:41:44

by Greg KH

[permalink] [raw]
Subject: patch rt2860sta-fix-argument-to-linux_pci_unmap_single.patch added to 2.6.32-stable tree


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 [email protected] Mon Mar 29 13:35:12 2010
From: Ben Hutchings <[email protected]>
Date: Mon, 29 Mar 2010 01:09:17 +0100
Subject: rt2860sta: Fix argument to linux_pci_unmap_single()
To: [email protected]
Cc: [email protected], linux-wireless <[email protected]>, John Halton <[email protected]>, Bartlomiej Zolnierkiewicz <[email protected]>
Message-ID: <1269821357.8653.231.camel@localhost>

From: Ben Hutchings <[email protected]>

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 <[email protected]>
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 <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
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 [email protected] are


2010-03-29 01:58:56

by Ben Hutchings

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

On Mon, 2010-03-29 at 10:52 +0900, FUJITA Tomonori wrote:
> On Mon, 29 Mar 2010 01:24:45 +0100
> Ben Hutchings <[email protected]> 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 <[email protected]>
> > ---
> > 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.


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2010-03-29 00:24:53

by Ben Hutchings

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

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 <[email protected]>
---
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.


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2010-03-29 02:04:24

by FUJITA Tomonori

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

On Mon, 29 Mar 2010 02:58:48 +0100
Ben Hutchings <[email protected]> wrote:

> On Mon, 2010-03-29 at 10:52 +0900, FUJITA Tomonori wrote:
> > On Mon, 29 Mar 2010 01:24:45 +0100
> > Ben Hutchings <[email protected]> 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 <[email protected]>
> > > ---
> > > 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.

2010-04-28 17:50:05

by Greg KH

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

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 <[email protected]>

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

thanks,

greg k-h

2010-04-28 18:19:03

by drago01

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

On Mon, Mar 29, 2010 at 4:04 AM, FUJITA Tomonori
<[email protected]> wrote:
> On Mon, 29 Mar 2010 02:58:48 +0100
> Ben Hutchings <[email protected]> wrote:
>
>> On Mon, 2010-03-29 at 10:52 +0900, FUJITA Tomonori wrote:
>> > On Mon, 29 Mar 2010 01:24:45 +0100
>> > Ben Hutchings <[email protected]> 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 <[email protected]>
>> > > ---
>> > > ?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.