* stuff-up in pcmcia/cardbus stuff
@ 2003-02-18 1:50 Paul Mackerras
2003-02-18 8:15 ` [PATCH] " Dominik Brodowski
0 siblings, 1 reply; 5+ messages in thread
From: Paul Mackerras @ 2003-02-18 1:50 UTC (permalink / raw)
To: linux-kernel, linux
Recent 2.5 kernels will crash with a null pointer dereference on my
powerbook (PowerPC laptop) when I try to suspend. I tracked it down
to cardbus_suspend() in drivers/pcmcia/pci-socket.c calling
pcmcia_suspend_socket() with a NULL argument. It turns out that
socket->pcmcia_socket is never set in the current code.
This changeset seems to be the culprit:
ChangeSet 1.1017 2003/02/15 12:42:51 linux@brodo.de
[PATCH] pcmcia: use device_class->add_device/remove_device
This patch removes the {un}register_ss_entry/pcmcia_{un}register_socket
calls, and replaces them with generic driver-model-compatible functions.
Also, update the CodingStyle of these cs.c functions to what's recommended
in the Linux kernel.
Paul.
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] Re: stuff-up in pcmcia/cardbus stuff 2003-02-18 1:50 stuff-up in pcmcia/cardbus stuff Paul Mackerras @ 2003-02-18 8:15 ` Dominik Brodowski 2003-02-18 9:23 ` Jeff Garzik 0 siblings, 1 reply; 5+ messages in thread From: Dominik Brodowski @ 2003-02-18 8:15 UTC (permalink / raw) To: Paul Mackerras; +Cc: linux-kernel On Tue, Feb 18, 2003 at 12:50:52PM +1100, Paul Mackerras wrote: > Recent 2.5 kernels will crash with a null pointer dereference on my > powerbook (PowerPC laptop) when I try to suspend. I tracked it down > to cardbus_suspend() in drivers/pcmcia/pci-socket.c calling > pcmcia_suspend_socket() with a NULL argument. It turns out that > socket->pcmcia_socket is never set in the current code. Indeed. socket->pcmcia_socket (old) == socket->cls_d.s_info[0] (new) Could you please check whether this patch helps? Dominik diff -ruN linux-original/drivers/pcmcia/pci_socket.c linux/drivers/pcmcia/pci_socket.c --- linux-original/drivers/pcmcia/pci_socket.c 2003-02-18 09:08:00.000000000 +0100 +++ linux/drivers/pcmcia/pci_socket.c 2003-02-18 09:12:02.000000000 +0100 @@ -230,14 +230,16 @@ static int cardbus_suspend (struct pci_dev *dev, u32 state) { pci_socket_t *socket = pci_get_drvdata(dev); - pcmcia_suspend_socket (socket->pcmcia_socket); + if (socket && socket->cls_d.s_info[0]) + pcmcia_suspend_socket (socket->cls_d.s_info[0]); return 0; } static int cardbus_resume (struct pci_dev *dev) { pci_socket_t *socket = pci_get_drvdata(dev); - pcmcia_resume_socket (socket->pcmcia_socket); + if (socket && socket->cls_d.s_info[0]) + pcmcia_resume_socket (socket->cls_d.s_info[0]); return 0; } diff -ruN linux-original/drivers/pcmcia/pci_socket.h linux/drivers/pcmcia/pci_socket.h --- linux-original/drivers/pcmcia/pci_socket.h 2003-02-18 09:08:00.000000000 +0100 +++ linux/drivers/pcmcia/pci_socket.h 2003-02-18 09:10:29.000000000 +0100 @@ -20,7 +20,6 @@ socket_cap_t cap; spinlock_t event_lock; unsigned int events; - struct socket_info_t *pcmcia_socket; struct work_struct tq_task; struct timer_list poll_timer; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Re: stuff-up in pcmcia/cardbus stuff 2003-02-18 8:15 ` [PATCH] " Dominik Brodowski @ 2003-02-18 9:23 ` Jeff Garzik 2003-02-18 14:55 ` Dominik Brodowski 2003-02-18 22:14 ` [PATCH, updated] " Dominik Brodowski 0 siblings, 2 replies; 5+ messages in thread From: Jeff Garzik @ 2003-02-18 9:23 UTC (permalink / raw) To: Dominik Brodowski; +Cc: Paul Mackerras, linux-kernel Dominik Brodowski wrote: > Indeed. socket->pcmcia_socket (old) == socket->cls_d.s_info[0] (new) If this is true... > @@ -230,14 +230,16 @@ > static int cardbus_suspend (struct pci_dev *dev, u32 state) > { > pci_socket_t *socket = pci_get_drvdata(dev); > - pcmcia_suspend_socket (socket->pcmcia_socket); > + if (socket && socket->cls_d.s_info[0]) > + pcmcia_suspend_socket (socket->cls_d.s_info[0]); > return 0; > } > > static int cardbus_resume (struct pci_dev *dev) > { > pci_socket_t *socket = pci_get_drvdata(dev); > - pcmcia_resume_socket (socket->pcmcia_socket); > + if (socket && socket->cls_d.s_info[0]) > + pcmcia_resume_socket (socket->cls_d.s_info[0]); > return 0; > } 1) ...why do you bother checking for NULL? Isn't NULL indicative of a BUG(), instead? 2) why are multiple s_info records allocated, when you hardcode use of record #0 ? Jeff ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Re: stuff-up in pcmcia/cardbus stuff 2003-02-18 9:23 ` Jeff Garzik @ 2003-02-18 14:55 ` Dominik Brodowski 2003-02-18 22:14 ` [PATCH, updated] " Dominik Brodowski 1 sibling, 0 replies; 5+ messages in thread From: Dominik Brodowski @ 2003-02-18 14:55 UTC (permalink / raw) To: Jeff Garzik; +Cc: Paul Mackerras, linux-kernel On Tue, Feb 18, 2003 at 04:23:45AM -0500, Jeff Garzik wrote: > Dominik Brodowski wrote: > >Indeed. socket->pcmcia_socket (old) == socket->cls_d.s_info[0] (new) > > If this is true... > > >@@ -230,14 +230,16 @@ > > static int cardbus_suspend (struct pci_dev *dev, u32 state) > > { > > pci_socket_t *socket = pci_get_drvdata(dev); > >- pcmcia_suspend_socket (socket->pcmcia_socket); > >+ if (socket && socket->cls_d.s_info[0]) > >+ pcmcia_suspend_socket (socket->cls_d.s_info[0]); > > return 0; > > } > > > > static int cardbus_resume (struct pci_dev *dev) > > { > > pci_socket_t *socket = pci_get_drvdata(dev); > >- pcmcia_resume_socket (socket->pcmcia_socket); > >+ if (socket && socket->cls_d.s_info[0]) > >+ pcmcia_resume_socket (socket->cls_d.s_info[0]); > > return 0; > > } > > > 1) ...why do you bother checking for NULL? Isn't NULL indicative of a > BUG(), instead? Well, it's only a safeguard against suspending / resuming combined with probing or removing the device. Else it's a BUG indeed... > 2) why are multiple s_info records allocated, when you hardcode use of > record #0 ? Only one s_info is actually allocated (in cs.c::pcmcia_register_socket) as only one pcmcia/cardbus socket is attached to one pci_dev for yenta-style devices. There are up to four pcmcia sockets to one pci_dev for i82092 devices, though. And so s_info[3] might be perfectly valid within the i82092 driver. Dominik ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH, updated] Re: stuff-up in pcmcia/cardbus stuff 2003-02-18 9:23 ` Jeff Garzik 2003-02-18 14:55 ` Dominik Brodowski @ 2003-02-18 22:14 ` Dominik Brodowski 1 sibling, 0 replies; 5+ messages in thread From: Dominik Brodowski @ 2003-02-18 22:14 UTC (permalink / raw) To: Jeff Garzik; +Cc: Paul Mackerras, linux-kernel On Tue, Feb 18, 2003 at 04:23:45AM -0500, Jeff Garzik wrote: > Dominik Brodowski wrote: > >Indeed. socket->pcmcia_socket (old) == socket->cls_d.s_info[0] (new) > > If this is true... <snip> > > 2) why are multiple s_info records allocated, when you hardcode use of > record #0 ? Indeed, the allocation of MAX_SOCK_PER_DEV s_info[] pointers is pointless. Dominik diff -ruN linux-original/drivers/pcmcia/cs.c linux-pcmcia/drivers/pcmcia/cs.c --- linux-original/drivers/pcmcia/cs.c 2003-02-18 09:08:00.000000000 +0100 +++ linux-pcmcia/drivers/pcmcia/cs.c 2003-02-18 23:10:56.000000000 +0100 @@ -330,11 +330,12 @@ return -ENOMEM; memset(s_info, 0, cls_d->nsock * sizeof(socket_info_t)); + cls_d->s_info = s_info; + /* socket initialization */ for (i = 0; i < cls_d->nsock; i++) { socket_info_t *s = &s_info[i]; - cls_d->s_info[i] = s; s->ss_entry = cls_d->ops; s->sock = i; diff -ruN linux-original/drivers/pcmcia/pci_socket.c linux-pcmcia/drivers/pcmcia/pci_socket.c --- linux-original/drivers/pcmcia/pci_socket.c 2003-02-18 22:54:34.000000000 +0100 +++ linux-pcmcia/drivers/pcmcia/pci_socket.c 2003-02-18 23:10:10.000000000 +0100 @@ -230,14 +230,16 @@ static int cardbus_suspend (struct pci_dev *dev, u32 state) { pci_socket_t *socket = pci_get_drvdata(dev); - pcmcia_suspend_socket (socket->pcmcia_socket); + if (socket && socket->cls_d.s_info) + pcmcia_suspend_socket (socket->cls_d.s_info); return 0; } static int cardbus_resume (struct pci_dev *dev) { pci_socket_t *socket = pci_get_drvdata(dev); - pcmcia_resume_socket (socket->pcmcia_socket); + if (socket && socket->cls_d.s_info) + pcmcia_resume_socket (socket->cls_d.s_info); return 0; } diff -ruN linux-original/drivers/pcmcia/pci_socket.h linux-pcmcia/drivers/pcmcia/pci_socket.h --- linux-original/drivers/pcmcia/pci_socket.h 2003-02-18 22:54:34.000000000 +0100 +++ linux-pcmcia/drivers/pcmcia/pci_socket.h 2003-02-18 23:10:42.000000000 +0100 @@ -20,7 +20,6 @@ socket_cap_t cap; spinlock_t event_lock; unsigned int events; - struct socket_info_t *pcmcia_socket; struct work_struct tq_task; struct timer_list poll_timer; diff -ruN linux-original/include/pcmcia/ss.h linux-pcmcia/include/pcmcia/ss.h --- linux-original/include/pcmcia/ss.h 2003-02-18 09:08:02.000000000 +0100 +++ linux-pcmcia/include/pcmcia/ss.h 2003-02-18 23:09:55.000000000 +0100 @@ -144,12 +144,10 @@ * Calls to set up low-level "Socket Services" drivers */ -#define MAX_SOCKETS_PER_DEV 8 - struct pcmcia_socket_class_data { unsigned int nsock; /* number of sockets */ struct pccard_operations *ops; /* see above */ - void *s_info[MAX_SOCKETS_PER_DEV]; /* socket_info_t */ + void *s_info; /* socket_info_t */ unsigned int use_bus_pm; }; ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2003-02-18 22:05 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2003-02-18 1:50 stuff-up in pcmcia/cardbus stuff Paul Mackerras 2003-02-18 8:15 ` [PATCH] " Dominik Brodowski 2003-02-18 9:23 ` Jeff Garzik 2003-02-18 14:55 ` Dominik Brodowski 2003-02-18 22:14 ` [PATCH, updated] " Dominik Brodowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox