2003-09-05 19:41:49

by Daniel Ritz

[permalink] [raw]
Subject: Re: Problems with PCMCIA (Texas Instruments PCI1450)

On Fri September 5 2003 20:38, Russell King wrote:
> On Fri, Sep 05, 2003 at 08:19:28PM +0200, Daniel Ritz wrote:
> > ok, now i can reproduce the problem on my ti1410 too. on boot detection
> > works fine with an UP kernel and fails with an SMP kernel. thanx for the
> > hint.
> >
> > i go to look at the csets a bit and try to find out more....
> > (i think i know which change...)
>
> Care to provide a hint?

yes. just tested. patch below makes on boot detection with a SMP kernel
working again (for me). which is nice, but i don't see why it is better
that way...


===== cs.c 1.56 vs edited =====
--- 1.56/drivers/pcmcia/cs.c Sun Aug 3 14:48:43 2003
+++ edited/cs.c Fri Sep 5 21:42:09 2003
@@ -316,7 +316,6 @@

wait_for_completion(&socket->thread_done);
BUG_ON(!socket->thread);
- pcmcia_parse_events(socket, SS_DETECT);

return 0;
}
@@ -1524,6 +1523,9 @@
if (client == NULL)
return CS_OUT_OF_RESOURCE;

+ if (++s->real_clients == 1)
+ pcmcia_parse_events(s, SS_DETECT);
+
*handle = client;
client->state &= ~CLIENT_UNBOUND;
client->Socket = s;


2003-09-05 19:54:42

by Russell King

[permalink] [raw]
Subject: Re: Problems with PCMCIA (Texas Instruments PCI1450)

On Fri, Sep 05, 2003 at 09:40:27PM +0200, Daniel Ritz wrote:
> On Fri September 5 2003 20:38, Russell King wrote:
> > On Fri, Sep 05, 2003 at 08:19:28PM +0200, Daniel Ritz wrote:
> > > ok, now i can reproduce the problem on my ti1410 too. on boot detection
> > > works fine with an UP kernel and fails with an SMP kernel. thanx for the
> > > hint.
> > >
> > > i go to look at the csets a bit and try to find out more....
> > > (i think i know which change...)
> >
> > Care to provide a hint?
>
> yes. just tested. patch below makes on boot detection with a SMP kernel
> working again (for me). which is nice, but i don't see why it is better
> that way...

Ok, now I need to hear from Sven (and others) to see if this patch fixes
their problems. Also, are these other people running a SMP kernel as
well?

Meanwhile, I'm wondering if we have a timing problem here. Can you check
whether adding a mdelay(1) just after the BUG_ON in the original code
fixes the problem?

> ===== cs.c 1.56 vs edited =====
> --- 1.56/drivers/pcmcia/cs.c Sun Aug 3 14:48:43 2003
> +++ edited/cs.c Fri Sep 5 21:42:09 2003
> @@ -316,7 +316,6 @@
>
> wait_for_completion(&socket->thread_done);
> BUG_ON(!socket->thread);
> - pcmcia_parse_events(socket, SS_DETECT);
>
> return 0;
> }
> @@ -1524,6 +1523,9 @@
> if (client == NULL)
> return CS_OUT_OF_RESOURCE;
>
> + if (++s->real_clients == 1)
> + pcmcia_parse_events(s, SS_DETECT);
> +
> *handle = client;
> client->state &= ~CLIENT_UNBOUND;
> client->Socket = s;
>

--
Russell King ([email protected]) http://www.arm.linux.org.uk/personal/
Linux kernel maintainer of:
2.6 ARM Linux - http://www.arm.linux.org.uk/
2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2003-09-05 20:20:34

by Daniel Ritz

[permalink] [raw]
Subject: Re: Problems with PCMCIA (Texas Instruments PCI1450)

On Fri September 5 2003 21:54, Russell King wrote:
> On Fri, Sep 05, 2003 at 09:40:27PM +0200, Daniel Ritz wrote:
> > On Fri September 5 2003 20:38, Russell King wrote:
> > > On Fri, Sep 05, 2003 at 08:19:28PM +0200, Daniel Ritz wrote:
> > > > ok, now i can reproduce the problem on my ti1410 too. on boot detection
> > > > works fine with an UP kernel and fails with an SMP kernel. thanx for the
> > > > hint.
> > > >
> > > > i go to look at the csets a bit and try to find out more....
> > > > (i think i know which change...)
> > >
> > > Care to provide a hint?
> >
> > yes. just tested. patch below makes on boot detection with a SMP kernel
> > working again (for me). which is nice, but i don't see why it is better
> > that way...
>
> Ok, now I need to hear from Sven (and others) to see if this patch fixes
> their problems. Also, are these other people running a SMP kernel as
> well?
>
> Meanwhile, I'm wondering if we have a timing problem here. Can you check
> whether adding a mdelay(1) just after the BUG_ON in the original code
> fixes the problem?

no effect. i tried that before with loooong sleeps (1 second) w/o any effect...

>
> > ===== cs.c 1.56 vs edited =====
> > --- 1.56/drivers/pcmcia/cs.c Sun Aug 3 14:48:43 2003
> > +++ edited/cs.c Fri Sep 5 21:42:09 2003
> > @@ -316,7 +316,6 @@
> >
> > wait_for_completion(&socket->thread_done);
> > BUG_ON(!socket->thread);
> > - pcmcia_parse_events(socket, SS_DETECT);
> >
> > return 0;
> > }
> > @@ -1524,6 +1523,9 @@
> > if (client == NULL)
> > return CS_OUT_OF_RESOURCE;
> >
> > + if (++s->real_clients == 1)
> > + pcmcia_parse_events(s, SS_DETECT);
> > +
> > *handle = client;
> > client->state &= ~CLIENT_UNBOUND;
> > client->Socket = s;
> >
>
> --
> Russell King ([email protected]) http://www.arm.linux.org.uk/personal/
> Linux kernel maintainer of:
> 2.6 ARM Linux - http://www.arm.linux.org.uk/
> 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
> 2.6 Serial core
>
>

2003-09-06 16:41:49

by Russell King

[permalink] [raw]
Subject: [PATCH] Re: Problems with PCMCIA (Texas Instruments PCI1450)

On Fri, Sep 05, 2003 at 08:54:29PM +0100, Russell King wrote:
> On Fri, Sep 05, 2003 at 09:40:27PM +0200, Daniel Ritz wrote:
> > On Fri September 5 2003 20:38, Russell King wrote:
> > > On Fri, Sep 05, 2003 at 08:19:28PM +0200, Daniel Ritz wrote:
> > > > ok, now i can reproduce the problem on my ti1410 too. on boot detection
> > > > works fine with an UP kernel and fails with an SMP kernel. thanx for the
> > > > hint.
> > > >
> > > > i go to look at the csets a bit and try to find out more....
> > > > (i think i know which change...)
> > >
> > > Care to provide a hint?
> >
> > yes. just tested. patch below makes on boot detection with a SMP kernel
> > working again (for me). which is nice, but i don't see why it is better
> > that way...
>
> Ok, now I need to hear from Sven (and others) to see if this patch fixes
> their problems. Also, are these other people running a SMP kernel as
> well?

Ok, I've updated pcmcia.arm.linux.org.uk with a couple of patches which
should solve the real cause of problem - a nice race condition. I'm
including the patch here - can people which this problem check whether
it solves the problem on their hardware?

I'd like to hear back from people who have been affected by this bug
before I push this patch to Linus.

Thanks.

diff -ur ref/drivers/pcmcia/cs.c linux/drivers/pcmcia/cs.c
--- ref/drivers/pcmcia/cs.c Tue Aug 5 11:19:39 2003
+++ linux/drivers/pcmcia/cs.c Sat Sep 6 15:07:25 2003
@@ -474,7 +474,7 @@
DEBUG(1, "cs: shutdown_socket(%p)\n", s);

/* Blank out the socket state */
- s->state &= SOCKET_PRESENT|SOCKET_SETUP_PENDING;
+ s->state &= SOCKET_PRESENT|SOCKET_INUSE;
s->socket = dead_socket;
s->ops->init(s);
s->irq.AssignedIRQ = s->irq.Config = 0;
@@ -657,7 +657,6 @@
pcmcia_error(skt, "unsupported voltage key.\n");
return CS_BAD_TYPE;
}
- skt->state |= SOCKET_PRESENT;
skt->socket.flags = SS_DEBOUNCED;
skt->ops->set_socket(skt, &skt->socket);

@@ -678,22 +677,23 @@
{
int ret;

- if (!try_module_get(skt->owner))
+ if (!cs_socket_get(skt))
return CS_NO_CARD;

ret = socket_setup(skt, setup_delay);
if (ret == CS_SUCCESS) {
+ skt->state |= SOCKET_PRESENT;
#ifdef CONFIG_CARDBUS
if (skt->state & SOCKET_CARDBUS) {
cb_alloc(skt);
skt->state |= SOCKET_CARDBUS_CONFIG;
}
#endif
send_event(skt, CS_EVENT_CARD_INSERTION, CS_EVENT_PRI_LOW);
skt->socket.flags &= ~SS_DEBOUNCED;
} else {
socket_shutdown(skt);
- module_put(skt->owner);
+ cs_socket_put(skt);
}

return ret;
@@ -741,10 +741,8 @@
}
skt->socket.flags &= ~SS_DEBOUNCED;
} else {
- unsigned int old_state = skt->state;
socket_shutdown(skt);
- if (old_state & SOCKET_PRESENT)
- module_put(skt->owner);
+ cs_socket_put(skt);
}

skt->state &= ~SOCKET_SUSPEND;
@@ -755,7 +753,7 @@
static void socket_remove(struct pcmcia_socket *skt)
{
socket_shutdown(skt);
- module_put(skt->owner);
+ cs_socket_put(skt);
}

/*
@@ -1346,8 +1344,6 @@
status->CardState |= CS_EVENT_PM_SUSPEND;
if (!(s->state & SOCKET_PRESENT))
return CS_NO_CARD;
- if (s->state & SOCKET_SETUP_PENDING)
- status->CardState |= CS_EVENT_CARD_INSERTION;

/* Get info from the PRR, if necessary */
if (handle->Function == BIND_FN_ALL) {
@@ -1524,6 +1520,10 @@
if (client == NULL)
return CS_OUT_OF_RESOURCE;

+ /*
+ * Prevent this racing with a card insertion.
+ */
+ down(&s->skt_sem);
*handle = client;
client->state &= ~CLIENT_UNBOUND;
client->Socket = s;
@@ -1555,13 +1555,15 @@
client, client->Socket, client->dev_info);
if (client->EventMask & CS_EVENT_REGISTRATION_COMPLETE)
EVENT(client, CS_EVENT_REGISTRATION_COMPLETE, CS_EVENT_PRI_LOW);
- if ((s->state & SOCKET_PRESENT) &&
- !(s->state & SOCKET_SETUP_PENDING)) {
+
+ if ((s->state & (SOCKET_PRESENT|SOCKET_CARDBUS)) == SOCKET_PRESENT) {
if (client->EventMask & CS_EVENT_CARD_INSERTION)
EVENT(client, CS_EVENT_CARD_INSERTION, CS_EVENT_PRI_LOW);
else
client->PendingEvents |= CS_EVENT_CARD_INSERTION;
}
+
+ up(&s->skt_sem);
return CS_SUCCESS;
} /* register_client */

diff -ur ref/drivers/pcmcia/cs_internal.h linux/drivers/pcmcia/cs_internal.h
--- ref/drivers/pcmcia/cs_internal.h Tue Aug 5 11:19:39 2003
+++ linux/drivers/pcmcia/cs_internal.h Sat Sep 6 14:41:19 2003
@@ -99,7 +99,7 @@

/* Flags in socket state */
#define SOCKET_PRESENT 0x0008
-#define SOCKET_SETUP_PENDING 0x0010
+#define SOCKET_INUSE 0x0010
#define SOCKET_SHUTDOWN_PENDING 0x0020
#define SOCKET_RESET_PENDING 0x0040
#define SOCKET_SUSPEND 0x0080
@@ -109,6 +109,26 @@
#define SOCKET_CARDBUS 0x8000
#define SOCKET_CARDBUS_CONFIG 0x10000

+static inline int cs_socket_get(struct pcmcia_socket *skt)
+{
+ int ret;
+
+ WARN_ON(skt->state & SOCKET_INUSE);
+
+ ret = try_module_get(skt->owner);
+ if (ret)
+ skt->state |= SOCKET_INUSE;
+ return ret;
+}
+
+static inline void cs_socket_put(struct pcmcia_socket *skt)
+{
+ if (skt->state & SOCKET_INUSE) {
+ skt->state &= ~SOCKET_INUSE;
+ module_put(skt->owner);
+ }
+}
+
#define CHECK_HANDLE(h) \
(((h) == NULL) || ((h)->client_magic != CLIENT_MAGIC))



--
Russell King ([email protected]) http://www.arm.linux.org.uk/personal/
Linux kernel maintainer of:
2.6 ARM Linux - http://www.arm.linux.org.uk/
2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2003-09-07 09:38:39

by Sven Dowideit

[permalink] [raw]
Subject: Re: [PATCH] Re: Problems with PCMCIA (Texas Instruments PCI1450)

On Sun, 2003-09-07 at 02:41, Russell King wrote:
> I'd like to hear back from people who have been affected by this bug
> before I push this patch to Linus.
it fixes my pcmcia problem at startup :)

and for bonus points, it also when plugged into the docking station
pcmcia cards (which is one up on the last 2.4 i used)

when i get a chance I will put the pc-card bridge back into my dual
processor piii, and see how that goes too.. is the PC-Card code supposed
to work on SMP?

> Thanks.
no, thank you!

in the process of playing around with cardctl eject, insert and pulling
out the card without warning i have gotten the following..

airo: Doing fast bap_reads
airo: MAC enabled eth1 0:40:96:33:e:a4
eth1: index 0x05: Vcc 5.0, Vpp 5.0, irq 3, io 0x0100-0x013f
nfs warning: mount version older than kernel
nfs: server 10.10.10.10 not responding, still trying
nfs: server 10.10.10.10 OK
nfs: server 10.10.10.10 not responding, still trying
RPC: sendmsg returned error 101
nfs: RPC call returned error 101
RPC: sendmsg returned error 101
nfs: RPC call returned error 101
RPC: sendmsg returned error 101
nfs: RPC call returned error 101
airo: Probing for PCI adapters
kobject_register failed for airo (-17)
Call Trace:
[<c0214da9>] kobject_register+0x59/0x60
[<c023c1e2>] bus_add_driver+0x52/0xb0
[<c021ef7e>] pci_register_driver+0x6e/0xa0
[<e08670e8>] airo_init_module+0xe8/0x10d [airo]
[<c0139f0f>] sys_init_module+0x12f/0x260
[<c010b0db>] syscall_call+0x7/0xb

airo: Finished probing for PCI adapters
airo: Doing fast bap_reads
airo: MAC enabled eth1 0:40:96:33:e:a4
eth1: index 0x05: Vcc 5.0, Vpp 5.0, irq 3, io 0x0100-0x013f
airo: Probing for PCI adapters
kobject_register failed for airo (-17)
Call Trace:
[<c0214da9>] kobject_register+0x59/0x60
[<c023c1e2>] bus_add_driver+0x52/0xb0
[<c021ef7e>] pci_register_driver+0x6e/0xa0
[<e08670e8>] airo_init_module+0xe8/0x10d [airo]
[<c0139f0f>] sys_init_module+0x12f/0x260
[<c010b0db>] syscall_call+0x7/0xb

airo: Finished probing for PCI adapters
airo: Doing fast bap_reads
airo: MAC enabled eth1 0:40:96:33:e:a4
eth1: index 0x05: Vcc 5.0, Vpp 5.0, irq 3, io 0x0100-0x013f
irq 9: nobody cared!
Call Trace:
[<c010d45a>] __report_bad_irq+0x2a/0x90
[<c010d550>] note_interrupt+0x70/0xb0
[<c010d890>] do_IRQ+0x160/0x1a0
[<c010ba48>] common_interrupt+0x18/0x20
[<e087007b>] yenta_get_status+0x4b/0x110 [yenta_socket]
[<c010d3e0>] handle_IRQ_event+0x30/0x80
[<c010d7e7>] do_IRQ+0xb7/0x1a0
[<c010ba48>] common_interrupt+0x18/0x20
[<e0828253>] apm_bios_call_simple+0x83/0x100 [apm]
[<e0828437>] apm_do_idle+0x27/0x80 [apm]
[<e0828572>] apm_cpu_idle+0xa2/0x140 [apm]
[<c0105000>] _stext+0x0/0x70
[<c0108c8b>] cpu_idle+0x3b/0x50
[<c03ea919>] start_kernel+0x199/0x1d0
[<c03ea490>] unknown_bootoption+0x0/0x120

handlers:
[<c0260f90>] (ide_intr+0x0/0x1e0)
[<e08708b0>] (yenta_interrupt+0x0/0x40 [yenta_socket])
[<e08708b0>] (yenta_interrupt+0x0/0x40 [yenta_socket])
Disabling IRQ #9

> diff -ur ref/drivers/pcmcia/cs.c linux/drivers/pcmcia/cs.c
> -- ref/drivers/pcmcia/cs.c Tue Aug 5 11:19:39 2003
> +++ linux/drivers/pcmcia/cs.c Sat Sep 6 15:07:25 2003


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

2003-09-08 22:32:53

by Tom Marshall

[permalink] [raw]
Subject: Re: [PATCH] Re: Problems with PCMCIA (Texas Instruments PCI1450)

> > Ok, now I need to hear from Sven (and others) to see if this patch fixes
> > their problems. Also, are these other people running a SMP kernel as
> > well?
>
> Ok, I've updated pcmcia.arm.linux.org.uk with a couple of patches which
> should solve the real cause of problem - a nice race condition. I'm
> including the patch here - can people which this problem check whether
> it solves the problem on their hardware?
>
> I'd like to hear back from people who have been affected by this bug
> before I push this patch to Linus.

Seems to work for me. Thanks! :-)

--
If voting could change the system, it would be illegal. If not voting
could change the system, it would be illegal.


Attachments:
(No filename) (695.00 B)
(No filename) (240.00 B)
Download all attachments