LinuxLists.cc - [PATCH] pcmcia event thread. (fwd)

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

David Woodhouse wrote:
> I'm not sure why we changed from the existing state machine / timer setup to
> sleeping in the PCMCIA parse_events() routine,

I don't remember the exact reason, but Linus changed it for yenta...
maybe because CardBus Watcher always called the state machine from user
context...

> +static int pcmcia_event_thread(void * dummy)
> +{
> + DECLARE_WAITQUEUE(wait, current);
> +
> + current->session = 1;
> + current->pgrp = 1;
> + strcpy(current->comm, "kpcmciad");
> + current->tty = NULL;
> + spin_lock_irq(&current->sigmask_lock);
> + sigfillset(&current->blocked);
> + recalc_sigpending(current);
> + spin_unlock_irq(&current->sigmask_lock);
> + exit_mm(current);
> + exit_files(current);
> + exit_sighand(current);
> + exit_fs(current);

Replace most if not all of this crap w/ daemonize()

> +
> + while(!event_thread_leaving) {
> + void *active;
> +
> + set_current_state(TASK_INTERRUPTIBLE);
> + add_wait_queue(&event_thread_wq, &wait);
> +
> + /* Don't really need locking. But the implied mb() */
> + spin_lock(&tqueue_lock);
> + active = tq_pcmcia;
> + spin_unlock(&tqueue_lock);
> +
> + if (!active)
> + schedule();
> +
> + set_current_state(TASK_RUNNING);
> + remove_wait_queue(&event_thread_wq, &wait);
> +
> + run_task_queue(&tq_pcmcia);
> + }

it looks like the loop can be simplified to

while (1) {
mb();
active = tq_pcmcia;
if (!active)
interruptible_sleep_on(&event_thread_wq);
if (signal_pending(current)
break;
run_task_queue(&tq_pcmcia);
}

> + /* Need up_and_exit() */
> + up(&event_thread_exit_sem);

Racy. Use waitpid() in the thread killer instead. You also need to
reap the process, just like in userland...

> + /* Start the thread for handling queued events for socket drivers */
> + kernel_thread (pcmcia_event_thread, NULL,
> + CLONE_FS | CLONE_FILES | CLONE_SIGHAND);

Why are you cloning _FS, _FILES, and _SIGHAND? I don't see why the
third arg should not be zero. man clone...

--
Jeff Garzik |
Building 1024 | The chief enemy of creativity is "good" sense
MandrakeSoft | -- Picasso

2000-11-13 14:38:32

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

[email protected] said:
> Replace most if not all of this crap w/ daemonize()

OK.

> it looks like the loop can be simplified to

Nope. sleep_on is bad. We have to set_current_state(TASK_INTERRUPTIBLE)
before checking tq_pcmcia.

> while (1) {
> mb();
> active = tq_pcmcia;

/* EVENT ARRIVES NOW */

> if (!active)
> interruptible_sleep_on(&event_thread_wq);

/* WE SLEEP ANYWAY */

> if (signal_pending(current)
> break;
> run_task_queue(&tq_pcmcia); }

[email protected] said:
> Racy. Use waitpid() in the thread killer instead.

Doesn't waitpid() require the thread we're waiting for to be child of the
rmmod process? I suppose we could arrange that, but it's not particularly
clean.

I prefer the tiny race and the campaign for up_and_exit() :)

> You also need to reap the process, just like in userland...

Gratuitous cruft. Just let init(8) do it.

[email protected] said:
> Why are you cloning _FS, _FILES, and _SIGHAND? I don't see why the
> third arg should not be zero. man clone...

If we don't specify CLONE_FS | CLONE_FILES | CLONE_SIGHAND then new ones
get allocated just for us to free them again immediately. If we clone them,
then we just increase and decrease the use counts of the parent's ones. The
latter is slightly more efficient, and I don't think it really matters. If
you really care, that can be changed. I've dropped CLONE_SIGHAND because
daemonize() doesn't free that, but left CLONE_FS and CLONE_FILES.

Revised patch:

Index: drivers/pcmcia/cs.c
===================================================================
RCS file: /inst/cvs/linux/drivers/pcmcia/Attic/cs.c,v
retrieving revision 1.1.2.28
diff -u -r1.1.2.28 cs.c
--- drivers/pcmcia/cs.c 2000/11/10 14:56:32 1.1.2.28
+++ drivers/pcmcia/cs.c 2000/11/13 14:35:13
@@ -2333,6 +2333,58 @@

/*======================================================================

+ Kernel thread for submitting events on behalf of interrupt handlers
+
+======================================================================*/
+static int event_thread_leaving = 0;
+static DECLARE_TASK_QUEUE(tq_pcmcia);
+static DECLARE_WAIT_QUEUE_HEAD(event_thread_wq);
+static DECLARE_MUTEX_LOCKED(event_thread_exit_sem);
+static int event_thread_pid;
+
+static int pcmcia_event_thread(void * dummy)
+{
+ DECLARE_WAITQUEUE(wait, current);
+
+ daemonize();
+ strcpy(current->comm, "kpcmciad");
+ spin_lock_irq(&current->sigmask_lock);
+ sigfillset(&current->blocked);
+ recalc_sigpending(current);
+ spin_unlock_irq(&current->sigmask_lock);
+
+ while(!event_thread_leaving) {
+ void *active;
+
+ set_current_state(TASK_INTERRUPTIBLE);
+ add_wait_queue(&event_thread_wq, &wait);
+
+ spin_lock(&tqueue_lock);
+ active = tq_pcmcia;
+ spin_unlock(&tqueue_lock);
+
+ if (!active)
+ schedule();
+
+ set_current_state(TASK_RUNNING);
+ remove_wait_queue(&event_thread_wq, &wait);
+
+ run_task_queue(&tq_pcmcia);
+ }
+
+ /* Need up_and_exit() */
+ up(&event_thread_exit_sem);
+ return 0;
+}
+
+void pcmcia_queue_task(struct tq_struct *task)
+{
+ queue_task(task, &tq_pcmcia);
+ wake_up(&event_thread_wq);
+}
+
+/*======================================================================
+
OS-specific module glue goes here

======================================================================*/
@@ -2366,6 +2418,7 @@
EXPORT_SYMBOL(pcmcia_modify_window);
EXPORT_SYMBOL(pcmcia_open_memory);
EXPORT_SYMBOL(pcmcia_parse_tuple);
+EXPORT_SYMBOL(pcmcia_queue_task);
EXPORT_SYMBOL(pcmcia_read_memory);
EXPORT_SYMBOL(pcmcia_register_client);
EXPORT_SYMBOL(pcmcia_register_erase_queue);
@@ -2406,6 +2459,15 @@
printk(KERN_INFO "%s\n", release);
printk(KERN_INFO " %s\n", options);
DEBUG(0, "%s\n", version);
+
+ /* Start the thread for handling queued events for socket drivers */
+ event_thread_pid = kernel_thread (pcmcia_event_thread, NULL, CLONE_FS | CLONE_FILES);
+
+ if (event_thread_pid < 0) {
+ printk(KERN_ERR "init_pcmcia_cs: fork failed: errno %d\n", -event_thread_pid);
+ return event_thread_pid;
+ }
+
if (do_apm)
pm_register(PM_SYS_DEV, PM_SYS_PCMCIA, handle_pm_event);
#ifdef CONFIG_PROC_FS
@@ -2417,6 +2479,14 @@
static void __exit exit_pcmcia_cs(void)
{
printk(KERN_INFO "unloading PCMCIA Card Services\n");
+
+ /* Tell the event thread to die */
+ event_thread_leaving = 1;
+ wake_up(&event_thread_wq);
+
+ /* Wait for it... */
+ down(&event_thread_exit_sem);
+
#ifdef CONFIG_PROC_FS
if (proc_pccard) {
remove_proc_entry("pccard", proc_bus);
Index: include/pcmcia/cs.h
===================================================================
RCS file: /inst/cvs/linux/include/pcmcia/Attic/cs.h,v
retrieving revision 1.1.2.8
diff -u -r1.1.2.8 cs.h
--- include/pcmcia/cs.h 2000/09/07 08:26:16 1.1.2.8
+++ include/pcmcia/cs.h 2000/11/13 14:35:13
@@ -443,6 +443,7 @@
int pcmcia_map_mem_page(window_handle_t win, memreq_t *req);
int pcmcia_modify_configuration(client_handle_t handle, modconf_t *mod);
int pcmcia_modify_window(window_handle_t win, modwin_t *req);
+void pcmcia_queue_task(struct tq_struct *task);
int pcmcia_register_client(client_handle_t *handle, client_reg_t *req);
int pcmcia_release_configuration(client_handle_t handle);
int pcmcia_release_io(client_handle_t handle, io_req_t *req);

--
dwmw2

2000-11-13 15:06:35

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

David Woodhouse wrote:
> [email protected] said:
> > Racy. Use waitpid() in the thread killer instead.
>
> Doesn't waitpid() require the thread we're waiting for to be child of the
> rmmod process? I suppose we could arrange that, but it's not particularly
> clean.

Doh, totally forgot about that. Will this work:

save_current = current;
current = find_task_by_pid(1);
waitpid(...);
current = save_current;

In any case, we -really- need a wait_for_kernel_thread_to_die()
function.

> [email protected] said:
> > Why are you cloning _FS, _FILES, and _SIGHAND? I don't see why the
> > third arg should not be zero. man clone...
>
> If we don't specify CLONE_FS | CLONE_FILES | CLONE_SIGHAND then new ones
> get allocated just for us to free them again immediately. If we clone them,
> then we just increase and decrease the use counts of the parent's ones. The
> latter is slightly more efficient, and I don't think it really matters. If
> you really care, that can be changed. I've dropped CLONE_SIGHAND because
> daemonize() doesn't free that, but left CLONE_FS and CLONE_FILES.

Cool tip, thanks.

> + spin_lock_irq(&current->sigmask_lock);
> + sigfillset(&current->blocked);
> + recalc_sigpending(current);
> + spin_unlock_irq(&current->sigmask_lock);

what does this do?

> + /* Start the thread for handling queued events for socket drivers */
> + event_thread_pid = kernel_thread (pcmcia_event_thread, NULL, CLONE_FS | CLONE_FILES);
> +
> + if (event_thread_pid < 0) {
> + printk(KERN_ERR "init_pcmcia_cs: fork failed: errno %d\n", -event_thread_pid);
> + return event_thread_pid;
> + }

why do you store the event_thread_pid but never use it?

regards,

Jeff

--
Jeff Garzik |
Building 1024 | The chief enemy of creativity is "good" sense
MandrakeSoft | -- Picasso

2000-11-13 15:14:36

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

[email protected] said:
> Doh, totally forgot about that. Will this work:
> save_current = current;
> current = find_task_by_pid(1);
> waitpid(...);
> current = save_current;

Changing the thread's parent to current->pid would be better than modifying
current. Still sucks though....

> In any case, we -really- need a wait_for_kernel_thread_to_die()
> function.

up_and_exit() would do it. Or preferably passing the address of the
semaphore as an extra argument to kernel_thread() in the first place.

In the meantime, enough other places in the kernel have this same race that
I'm not too concerned about it. Hopefully, we'll get the extra arg to
kernel_thread before the final cut of 2.4, and this code can get converted
along with everything else.

> > + spin_lock_irq(&current->sigmask_lock);
> > + sigfillset(&current->blocked);
> > + recalc_sigpending(current);
> > + spin_unlock_irq(&current->sigmask_lock);

> what does this do?

Well, I _hope_ it blocks all signals, so that we can sleep in
TASK_INTERRUPTIBLE without adding 1 to the load average, and without having
to worry about the pesky things actually interrupting us.

[email protected] said:
> why do you store the event_thread_pid but never use it?

Because I got half way through doing waitpid() before I realised it was
sucky. We store it short-term so we can print it. No reason why we can't
shift the static declaration into init_pcmcia_cs() though.

--
dwmw2

2000-11-13 15:34:49

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

David Woodhouse wrote:
> [email protected] said:
> > In any case, we -really- need a wait_for_kernel_thread_to_die()
> > function.
>
> up_and_exit() would do it. Or preferably passing the address of the
> semaphore as an extra argument to kernel_thread() in the first place.

Any solution which involves a function being called from a kernel
thread, and/or passing another arg to a kernel thread, is a non-starter
... See the discussion about timer_exit() a while ago. This sort of
thing should be done at a higher level. Not only does that eliminate
any possibility of a race between thread function calling
MOD_DEC_USE_COUNT/up_and_exit and module unload, but it also allows us
to more easily implement wait_for_kernel_thread_to_die() without having
to do silly stuff like pass extra args to threads (...modifying the
thread API in the process).

IMHO it is possible to solve this in a generic way without having to
change the code for every single kernel thread.

Thanks for all the explanation, I think I now understand all the stuff
your kernel thread is doing. Your solution sounds like a decent
solution for the problem described. I have not looked at the socket
driver code observe parse_events() usage, so I cannot say whether your
problem description is accurate however :)

Jeff

--
Jeff Garzik |
Building 1024 | The chief enemy of creativity is "good" sense
MandrakeSoft | -- Picasso

2000-11-13 15:43:00

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

[email protected] said:
> Thanks for all the explanation, I think I now understand all the
> stuff your kernel thread is doing. Your solution sounds like a decent
> solution for the problem described. I have not looked at the socket
> driver code observe parse_events() usage, so I cannot say whether your
> problem description is accurate however :)

It's the socket drivers which _aren't_ in the kernel source which are most
likely to exhibit this problem. Anything in the kernel tree was probably
converted by Linus, and hence done right. As there are so few socket drivers
in his tree, the amount of code duplication wasn't immediately obvious
either.

The offenders are the multitude of PCMCIA and CF socket drivers for
embedded boards which are floating around. They aren't likely to get merged
into 2.4, unfortunately - but I think we should at least make this
available in order for them to work correctly.

--
dwmw2

2000-11-13 18:59:05

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

On Mon, Nov 13, 2000 at 03:42:34PM +0000, David Woodhouse wrote:

> It's the socket drivers which _aren't_ in the kernel source which are most
> likely to exhibit this problem. Anything in the kernel tree was probably
> converted by Linus, and hence done right. As there are so few socket drivers
> in his tree, the amount of code duplication wasn't immediately obvious
> either.

The i82365 and tcic drivers in the 2.4 tree have not been converted to
use the thread stuff; as far as I know, the yenta driver is the only
socket driver that works at all in 2.4.

-- Dave

2000-11-13 22:27:05

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

On Mon, 13 Nov 2000, David Hinds wrote:

> The i82365 and tcic drivers in the 2.4 tree have not been converted to
> use the thread stuff; as far as I know, the yenta driver is the only
> socket driver that works at all in 2.4.

OK. I take it you support my proposed change?

Can you review this patch for i82365.c?

Index: i82365.c
===================================================================
RCS file: /net/passion/inst/cvs/linux/drivers/pcmcia/Attic/i82365.c,v
retrieving revision 1.1.2.14
diff -u -r1.1.2.14 i82365.c
--- i82365.c 2000/06/07 14:48:18 1.1.2.14
+++ i82365.c 2000/11/13 21:49:29
@@ -859,6 +859,26 @@

/*====================================================================*/

+static u_int pending_events[8] = {0,0,0,0,0,0,0,0};
+static spinlock_t pending_event_lock = SPIN_LOCK_UNLOCKED;
+
+static void pcic_bh(void *dummy)
+{
+ u_int events;
+ int i;
+
+ for (i=0; i < sockets; i++) {
+ spin_lock_irq(&pending_event_lock);
+ events = pending_events[i];
+ pending_events[i] = 0;
+ spin_unlock_irq(&pending_event_lock);
+ if (socket[i].handler)
+ socket[i].handler(socket[i].info, events);
+ }
+}
+
+static struct tq_struct pcic_task = {0, 0, &pcic_bh, NULL};
+
static void pcic_interrupt(int irq, void *dev,
struct pt_regs *regs)
{
@@ -893,8 +913,13 @@
}
ISA_UNLOCK(i, flags);
DEBUG(2, "i82365: socket %d event 0x%02x\n", i, events);
- if (events)
- socket[i].handler(socket[i].info, events);
+
+ if (events) {
+ spin_lock(&pending_event_lock);
+ pending_events[i] |= events;
+ spin_unlock(&pending_event_lock);
+ pcmcia_queue_task(&pcic_task);
+ }
active |= events;
}
if (!active) break;

--
dwmw2

2000-11-13 22:27:55

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

On Mon, Nov 13, 2000 at 09:52:30PM +0000, David Woodhouse wrote:
>
> OK. I take it you support my proposed change?
>
> Can you review this patch for i82365.c?

It looks reasonable and straighforward to me.

-- Dave

2000-11-13 23:01:39

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

On Mon, 13 Nov 2000, David Hinds wrote:

> The i82365 and tcic drivers in the 2.4 tree have not been converted to
> use the thread stuff; as far as I know, the yenta driver is the only
> socket driver that works at all in 2.4.
>
> On Mon, Nov 13, 2000 at 09:52:30PM +0000, David Woodhouse wrote:
> > OK. I take it you support my proposed change?
> > Can you review this patch for i82365.c?
>
> It looks reasonable and straighforward to me.

Cool. Linus, please could you apply this patch. If the fact that i82365
and tcic are broken in 2.4 isn't on Ted's critical list, then I think it
probably ought to have been - and this should fix it.

As before, this causes the PCMCIA core code to start a kernel thread
solely for the purpose of running a queue of tasks submitted by random
drivers. It also now converts the tcic and i82365 driver to use it, rather
than just causing the machine to panic by calling parse_events() from
interrupt context.

There's still a small amount of duplicated code, but far less than before,
and I think the flexibility of making a task queue available rather than
just accepting event submission asynchronously is probably going to be
worth it.

Index: include/pcmcia/cs.h
===================================================================
RCS file: /net/passion/inst/cvs/linux/include/pcmcia/Attic/cs.h,v
retrieving revision 1.1.2.8
diff -u -r1.1.2.8 cs.h
--- include/pcmcia/cs.h 2000/09/07 08:26:16 1.1.2.8
+++ include/pcmcia/cs.h 2000/11/13 22:21:39
@@ -443,6 +443,7 @@
int pcmcia_map_mem_page(window_handle_t win, memreq_t *req);
int pcmcia_modify_configuration(client_handle_t handle, modconf_t *mod);
int pcmcia_modify_window(window_handle_t win, modwin_t *req);
+void pcmcia_queue_task(struct tq_struct *task);
int pcmcia_register_client(client_handle_t *handle, client_reg_t *req);
int pcmcia_release_configuration(client_handle_t handle);
int pcmcia_release_io(client_handle_t handle, io_req_t *req);
Index: drivers/pcmcia/cs.c
===================================================================
RCS file: /net/passion/inst/cvs/linux/drivers/pcmcia/Attic/cs.c,v
retrieving revision 1.1.2.28
diff -u -r1.1.2.28 cs.c
--- drivers/pcmcia/cs.c 2000/11/10 14:56:32 1.1.2.28
+++ drivers/pcmcia/cs.c 2000/11/13 22:21:41
@@ -2333,6 +2333,60 @@

/*======================================================================

+ Kernel thread for submitting events on behalf of interrupt handlers
+
+======================================================================*/
+static int event_thread_leaving = 0;
+static DECLARE_TASK_QUEUE(tq_pcmcia);
+static DECLARE_WAIT_QUEUE_HEAD(event_thread_wq);
+static DECLARE_MUTEX_LOCKED(event_thread_exit_sem);
+
+static int pcmcia_event_thread(void * dummy)
+{
+ DECLARE_WAITQUEUE(wait, current);
+
+ daemonize();
+ strcpy(current->comm, "kpcmciad");
+ spin_lock_irq(&current->sigmask_lock);
+ sigfillset(&current->blocked);
+ recalc_sigpending(current);
+ spin_unlock_irq(&current->sigmask_lock);
+
+ while(!event_thread_leaving) {
+ void *active;
+
+ set_current_state(TASK_INTERRUPTIBLE);
+ add_wait_queue(&event_thread_wq, &wait);
+
+ /* We don't really need locking because it'll be
+ an atomic copy - but we do need the implied mb()
+ */
+ spin_lock(&tqueue_lock);
+ active = tq_pcmcia;
+ spin_unlock(&tqueue_lock);
+
+ if (!active)
+ schedule();
+
+ set_current_state(TASK_RUNNING);
+ remove_wait_queue(&event_thread_wq, &wait);
+
+ run_task_queue(&tq_pcmcia);
+ }
+
+ /* Need up_and_exit() */
+ up(&event_thread_exit_sem);
+ return 0;
+}
+
+void pcmcia_queue_task(struct tq_struct *task)
+{
+ queue_task(task, &tq_pcmcia);
+ wake_up(&event_thread_wq);
+}
+
+/*======================================================================
+
OS-specific module glue goes here

======================================================================*/
@@ -2366,6 +2420,7 @@
EXPORT_SYMBOL(pcmcia_modify_window);
EXPORT_SYMBOL(pcmcia_open_memory);
EXPORT_SYMBOL(pcmcia_parse_tuple);
+EXPORT_SYMBOL(pcmcia_queue_task);
EXPORT_SYMBOL(pcmcia_read_memory);
EXPORT_SYMBOL(pcmcia_register_client);
EXPORT_SYMBOL(pcmcia_register_erase_queue);
@@ -2403,9 +2458,19 @@

static int __init init_pcmcia_cs(void)
{
+ int pid;
printk(KERN_INFO "%s\n", release);
printk(KERN_INFO " %s\n", options);
DEBUG(0, "%s\n", version);
+
+ /* Start the thread for handling queued events for socket drivers */
+ pid = kernel_thread (pcmcia_event_thread, NULL, CLONE_FS | CLONE_FILES);
+
+ if (pid < 0) {
+ printk(KERN_ERR "init_pcmcia_cs: fork failed: errno %d\n", -pid);
+ return pid;
+ }
+
if (do_apm)
pm_register(PM_SYS_DEV, PM_SYS_PCMCIA, handle_pm_event);
#ifdef CONFIG_PROC_FS
@@ -2417,6 +2482,14 @@
static void __exit exit_pcmcia_cs(void)
{
printk(KERN_INFO "unloading PCMCIA Card Services\n");
+
+ /* Tell the event thread to die */
+ event_thread_leaving = 1;
+ wake_up(&event_thread_wq);
+
+ /* Wait for it... */
+ down(&event_thread_exit_sem);
+
#ifdef CONFIG_PROC_FS
if (proc_pccard) {
remove_proc_entry("pccard", proc_bus);
Index: drivers/pcmcia/i82365.c
===================================================================
RCS file: /net/passion/inst/cvs/linux/drivers/pcmcia/Attic/i82365.c,v
retrieving revision 1.1.2.14
diff -u -r1.1.2.14 i82365.c
--- drivers/pcmcia/i82365.c 2000/06/07 14:48:18 1.1.2.14
+++ drivers/pcmcia/i82365.c 2000/11/13 22:21:43
@@ -859,6 +859,26 @@

/*====================================================================*/

+static u_int pending_events[8] = {0,0,0,0,0,0,0,0};
+static spinlock_t pending_event_lock = SPIN_LOCK_UNLOCKED;
+
+static void pcic_bh(void *dummy)
+{
+ u_int events;
+ int i;
+
+ for (i=0; i < sockets; i++) {
+ spin_lock_irq(&pending_event_lock);
+ events = pending_events[i];
+ pending_events[i] = 0;
+ spin_unlock_irq(&pending_event_lock);
+ if (socket[i].handler)
+ socket[i].handler(socket[i].info, events);
+ }
+}
+
+static struct tq_struct pcic_task = {0, 0, &pcic_bh, NULL};
+
static void pcic_interrupt(int irq, void *dev,
struct pt_regs *regs)
{
@@ -893,8 +913,13 @@
}
ISA_UNLOCK(i, flags);
DEBUG(2, "i82365: socket %d event 0x%02x\n", i, events);
- if (events)
- socket[i].handler(socket[i].info, events);
+
+ if (events) {
+ spin_lock(&pending_event_lock);
+ pending_events[i] |= events;
+ spin_unlock(&pending_event_lock);
+ pcmcia_queue_task(&pcic_task);
+ }
active |= events;
}
if (!active) break;
Index: drivers/pcmcia/tcic.c
===================================================================
RCS file: /net/passion/inst/cvs/linux/drivers/pcmcia/Attic/tcic.c,v
retrieving revision 1.1.2.9
diff -u -r1.1.2.9 tcic.c
--- drivers/pcmcia/tcic.c 2000/06/07 14:48:18 1.1.2.9
+++ drivers/pcmcia/tcic.c 2000/11/13 22:21:44
@@ -530,6 +530,26 @@

/*====================================================================*/

+static u_int pending_events[2] = {0,0};
+static spinlock_t pending_event_lock = SPIN_LOCK_UNLOCKED;
+
+static void tcic_bh(void *dummy)
+{
+ u_int events;
+ int i;
+
+ for (i=0; i < sockets; i++) {
+ spin_lock_irq(&pending_event_lock);
+ events = pending_events[i];
+ pending_events[i] = 0;
+ spin_unlock_irq(&pending_event_lock);
+ if (socket_table[i].handler)
+ socket_table[i].handler(socket_table[i].info, events);
+ }
+}
+
+static struct tq_struct tcic_task = {0, 0, &tcic_bh, NULL};
+
static void tcic_interrupt(int irq, void *dev, struct pt_regs *regs)
{
int i, quick = 0;
@@ -567,9 +587,13 @@
events |= (latch & TCIC_SSTAT_RDY) ? SS_READY : 0;
events |= (latch & TCIC_SSTAT_LBAT1) ? SS_BATDEAD : 0;
events |= (latch & TCIC_SSTAT_LBAT2) ? SS_BATWARN : 0;
+ }
+ if (events) {
+ spin_lock(&pending_event_lock);
+ pending_events[i] |= events;
+ spin_unlock(&pending_event_lock);
+ pcmcia_queue_task(&tcic_task);
}
- if (events)
- socket_table[i].handler(socket_table[i].info, events);
}

/* Schedule next poll, if needed */

--
dwmw2

2000-11-13 23:18:31

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

David Woodhouse wrote:
>
> On Mon, 13 Nov 2000, David Hinds wrote:
>
> > The i82365 and tcic drivers in the 2.4 tree have not been converted to
> > use the thread stuff; as far as I know, the yenta driver is the only
> > socket driver that works at all in 2.4.
> >
> > On Mon, Nov 13, 2000 at 09:52:30PM +0000, David Woodhouse wrote:
> > > OK. I take it you support my proposed change?
> > > Can you review this patch for i82365.c?
> >
> > It looks reasonable and straighforward to me.
>
> Cool. Linus, please could you apply this patch. If the fact that i82365
> and tcic are broken in 2.4 isn't on Ted's critical list, then I think it
> probably ought to have been - and this should fix it.

It's purposefully not on Ted's critical list, the official line is "use
pcmcia_cs external package" if you need i82365 or tcic instead of yenta
AFAIK. However... fixing things and being able to support all pcmcia
and cardbus adapters would be wonderful.

drivers/pcmcia/Config.in:
> #tristate 'PCMCIA/CardBus support' CONFIG_PCMCIA
> #if [ "$CONFIG_PCMCIA" != "n" ]; then
> # if [ "$CONFIG_PCI" != "n" ]; then
> # bool ' CardBus support' CONFIG_CARDBUS
> # fi
> # bool ' i82365 compatible bridge support' CONFIG_I82365
> # bool ' Databook TCIC host bridge support' CONFIG_TCIC
> #fi

--
Jeff Garzik |
Building 1024 | The chief enemy of creativity is "good" sense
MandrakeSoft | -- Picasso

2000-11-13 23:35:39

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

On Mon, 13 Nov 2000, Jeff Garzik wrote:

> It's purposefully not on Ted's critical list, the official line is "use
> pcmcia_cs external package" if you need i82365 or tcic instead of yenta
> AFAIK. However... fixing things and being able to support all pcmcia
> and cardbus adapters would be wonderful.
>
> drivers/pcmcia/Config.in:
> > #tristate 'PCMCIA/CardBus support' CONFIG_PCMCIA
> > #if [ "$CONFIG_PCMCIA" != "n" ]; then
> > # if [ "$CONFIG_PCI" != "n" ]; then
> > # bool ' CardBus support' CONFIG_CARDBUS
> > # fi
> > # bool ' i82365 compatible bridge support' CONFIG_I82365
> > # bool ' Databook TCIC host bridge support' CONFIG_TCIC
> > #fi

Ok, well the patch I've just submitted _ought_ to fix stuff, and certainly
works with my own socket drivers for an embedded board I'm working on.

I have an i82365 eval board kicking around somewhere. I'll see if I can
dig it out tomorrow and verify that it now works, and then perhaps we can
consider re-enabling the config options.

I may add code to handle non-CardBus PCI->PCMCIA i82365 devices again
while I'm at it - because I disapprove of having to specify
"i365_base=0xc800 cs_irq=17" for a PCI card.

--
dwmw2

2000-11-15 00:42:14

by Russell King

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

David Woodhouse writes:
> If we don't specify CLONE_FS | CLONE_FILES | CLONE_SIGHAND then new ones
> get allocated just for us to free them again immediately. If we clone them,
> then we just increase and decrease the use counts of the parent's ones. The
> latter is slightly more efficient, and I don't think it really matters. If
> you really care, that can be changed. I've dropped CLONE_SIGHAND because
> daemonize() doesn't free that, but left CLONE_FS and CLONE_FILES.

Small suggestion - when your thread is created, make sure that all /proc
accesses to stuff relating to this thread doesn't cause the kernel to panic.

I used to create some processes with '0' as the third arg until Debian's
start-stop-daemon script started killing peoples machines with a kernel
oops. Now I always use CLONE_FS | CLONE_FILES | CLONE_SIGHAND as per
fs/buffer.c (which I used as the reference for creating kernel threads).
_____
|_____| ------------------------------------------------- ---+---+-
| | Russell King [email protected] --- ---
| | | | http://www.arm.linux.org.uk/personal/aboutme.html / / |
| +-+-+ --- -+-
/ | THE developer of ARM Linux |+| /|\
/ | | | --- |
+-+-+ ------------------------------------------------- /\\\ |

2000-11-16 13:58:42

by Alan

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

> it looks like the loop can be simplified to
>
> while (1) {
> mb();
> active = tq_pcmcia;
> if (!active)
> interruptible_sleep_on(&event_thread_wq);
> if (signal_pending(current)
> break;
> run_task_queue(&tq_pcmcia);
> }

Not if you want it to work

Alan

2000-11-16 14:10:14

by Alan

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

> > up_and_exit() would do it. Or preferably passing the address of the
> > semaphore as an extra argument to kernel_thread() in the first place.
>
> Any solution which involves a function being called from a kernel
> thread, and/or passing another arg to a kernel thread, is a non-starter
> ... See the discussion about timer_exit() a while ago. This sort of

timer_exit is a completely unrelated situation

> IMHO it is possible to solve this in a generic way without having to
> change the code for every single kernel thread.

It comes entirely down to one thing. The termination function of the thread
has to be code in the kernel code not a module. Thats it, nothing else. So
you just need some kind of sleep/wakeup proceedure for it.

As far as I can see all you need is

wait_and_die(sem)
{
up(&sem); /* In the kernel so can unload */
sys_exit(0); /* Never return to the unloaded module code */
}

in the main kernel. In fact if you were bored you could build this on the kernel
stack ;)

2000-11-16 14:10:54

by Alan

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

> It's the socket drivers which _aren't_ in the kernel source which are most
> likely to exhibit this problem. Anything in the kernel tree was probably
> converted by Linus, and hence done right. As there are so few socket drivers

Umm.. Linus drivers dont appear to be SMP safe on unload

2000-11-16 14:45:29

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

[email protected] said:
> Umm.. Linus drivers dont appear to be SMP safe on unload

AFAIK, no kernel threads are currently SMP safe on unload. However,
the PCMCIA thread would be safe with the patch below, and we could fairly
easily convert the others to use up_and_exit() once it's available.

Anyone using PCMCIA or CardBus with 2.4, even if you have a non-CardBus
i82365 or TCIC controller for which the driver was disabled in test11-pre5,
please could you test this? Especially if you have TCIC, in fact, because
it's already been tested successfully on yenta and i82365.

(pcmcia-dif-7). Linus, this is the full patch against pre5, including the
incremental part I sent this morning.

Index: drivers/pcmcia/Config.in
===================================================================
RCS file: /inst/cvs/linux/drivers/pcmcia/Attic/Config.in,v
retrieving revision 1.1.2.7
diff -u -r1.1.2.7 Config.in
--- drivers/pcmcia/Config.in 2000/08/01 14:18:00 1.1.2.7
+++ drivers/pcmcia/Config.in 2000/11/16 11:52:11
@@ -7,18 +7,18 @@
mainmenu_option next_comment
comment 'PCMCIA/CardBus support'

-dep_tristate 'CardBus support' CONFIG_PCMCIA $CONFIG_PCI
-if [ "$CONFIG_PCMCIA" != "n" ]; then
- define_bool CONFIG_CARDBUS y
-fi
-
-#tristate 'PCMCIA/CardBus support' CONFIG_PCMCIA
+#dep_tristate 'CardBus support' CONFIG_PCMCIA $CONFIG_PCI
#if [ "$CONFIG_PCMCIA" != "n" ]; then
-# if [ "$CONFIG_PCI" != "n" ]; then
-# bool ' CardBus support' CONFIG_CARDBUS
-# fi
-# bool ' i82365 compatible bridge support' CONFIG_I82365
-# bool ' Databook TCIC host bridge support' CONFIG_TCIC
+# define_bool CONFIG_CARDBUS y
#fi
+
+tristate 'PCMCIA/CardBus support' CONFIG_PCMCIA
+if [ "$CONFIG_PCMCIA" != "n" ]; then
+ if [ "$CONFIG_PCI" != "n" ]; then
+ bool ' CardBus support' CONFIG_CARDBUS
+ fi
+ bool ' i82365 compatible bridge support' CONFIG_I82365
+ bool ' Databook TCIC host bridge support' CONFIG_TCIC
+fi

endmenu
Index: drivers/pcmcia/cs.c
===================================================================
RCS file: /inst/cvs/linux/drivers/pcmcia/Attic/cs.c,v
retrieving revision 1.1.2.28
diff -u -r1.1.2.28 cs.c
--- drivers/pcmcia/cs.c 2000/11/10 14:56:32 1.1.2.28
+++ drivers/pcmcia/cs.c 2000/11/16 11:52:11
@@ -2333,6 +2333,58 @@

/*======================================================================

+ Kernel thread for doing stuff(tm) on behalf of interrupt handlers
+
+======================================================================*/
+static int event_thread_leaving = 0;
+static DECLARE_TASK_QUEUE(tq_pcmcia);
+static DECLARE_WAIT_QUEUE_HEAD(event_thread_wq);
+static DECLARE_MUTEX_LOCKED(event_thread_exit_sem);
+
+static int pcmcia_event_thread(void * dummy)
+{
+ DECLARE_WAITQUEUE(wait, current);
+
+ daemonize();
+ strcpy(current->comm, "kpcmciad");
+ spin_lock_irq(&current->sigmask_lock);
+ sigfillset(&current->blocked);
+ recalc_sigpending(current);
+ spin_unlock_irq(&current->sigmask_lock);
+
+ while(!event_thread_leaving) {
+ void *active;
+
+ set_current_state(TASK_INTERRUPTIBLE);
+ add_wait_queue(&event_thread_wq, &wait);
+
+ /* We don't really need locking because it'll be
+ an atomic copy - but we do need the implied mb()
+ */
+ spin_lock(&tqueue_lock);
+ active = tq_pcmcia;
+ spin_unlock(&tqueue_lock);
+
+ if (!active)
+ schedule();
+
+ set_current_state(TASK_RUNNING);
+ remove_wait_queue(&event_thread_wq, &wait);
+
+ run_task_queue(&tq_pcmcia);
+ }
+
+ up_and_exit(&event_thread_exit_sem,0);
+}
+
+void pcmcia_queue_task(struct tq_struct *task)
+{
+ queue_task(task, &tq_pcmcia);
+ wake_up(&event_thread_wq);
+}
+
+/*======================================================================
+
OS-specific module glue goes here

======================================================================*/
@@ -2366,6 +2418,7 @@
EXPORT_SYMBOL(pcmcia_modify_window);
EXPORT_SYMBOL(pcmcia_open_memory);
EXPORT_SYMBOL(pcmcia_parse_tuple);
+EXPORT_SYMBOL(pcmcia_queue_task);
EXPORT_SYMBOL(pcmcia_read_memory);
EXPORT_SYMBOL(pcmcia_register_client);
EXPORT_SYMBOL(pcmcia_register_erase_queue);
@@ -2403,9 +2456,19 @@

static int __init init_pcmcia_cs(void)
{
+ pid_t pid;
printk(KERN_INFO "%s\n", release);
printk(KERN_INFO " %s\n", options);
DEBUG(0, "%s\n", version);
+
+ /* Start the thread for handling queued events for socket drivers */
+ pid = kernel_thread (pcmcia_event_thread, NULL, CLONE_FS | CLONE_FILES);
+
+ if (pid < 0) {
+ printk(KERN_ERR "init_pcmcia_cs: fork failed: errno %d\n", -pid);
+ return pid;
+ }
+
if (do_apm)
pm_register(PM_SYS_DEV, PM_SYS_PCMCIA, handle_pm_event);
#ifdef CONFIG_PROC_FS
@@ -2417,6 +2480,14 @@
static void __exit exit_pcmcia_cs(void)
{
printk(KERN_INFO "unloading PCMCIA Card Services\n");
+
+ /* Tell the event thread to die */
+ event_thread_leaving = 1;
+ wake_up(&event_thread_wq);
+
+ /* Wait for it... */
+ down(&event_thread_exit_sem);
+
#ifdef CONFIG_PROC_FS
if (proc_pccard) {
remove_proc_entry("pccard", proc_bus);
Index: drivers/pcmcia/i82365.c
===================================================================
RCS file: /inst/cvs/linux/drivers/pcmcia/Attic/i82365.c,v
retrieving revision 1.1.2.14
diff -u -r1.1.2.14 i82365.c
--- drivers/pcmcia/i82365.c 2000/06/07 14:48:18 1.1.2.14
+++ drivers/pcmcia/i82365.c 2000/11/16 11:52:11
@@ -859,6 +859,28 @@

/*====================================================================*/

+static u_int pending_events[8];
+static spinlock_t pending_event_lock = SPIN_LOCK_UNLOCKED;
+
+static void pcic_bh(void *dummy)
+{
+ u_int events;
+ int i;
+
+ for (i=0; i < sockets; i++) {
+ spin_lock_irq(&pending_event_lock);
+ events = pending_events[i];
+ pending_events[i] = 0;
+ spin_unlock_irq(&pending_event_lock);
+ if (socket[i].handler)
+ socket[i].handler(socket[i].info, events);
+ }
+}
+
+static struct tq_struct pcic_task = {
+ routine: pcic_bh
+};
+
static void pcic_interrupt(int irq, void *dev,
struct pt_regs *regs)
{
@@ -893,8 +915,13 @@
}
ISA_UNLOCK(i, flags);
DEBUG(2, "i82365: socket %d event 0x%02x\n", i, events);
- if (events)
- socket[i].handler(socket[i].info, events);
+
+ if (events) {
+ spin_lock(&pending_event_lock);
+ pending_events[i] |= events;
+ spin_unlock(&pending_event_lock);
+ pcmcia_queue_task(&pcic_task);
+ }
active |= events;
}
if (!active) break;
Index: drivers/pcmcia/pci_socket.c
===================================================================
RCS file: /inst/cvs/linux/drivers/pcmcia/Attic/pci_socket.c,v
retrieving revision 1.1.2.7
diff -u -r1.1.2.7 pci_socket.c
--- drivers/pcmcia/pci_socket.c 2000/08/01 14:18:00 1.1.2.7
+++ drivers/pcmcia/pci_socket.c 2000/11/16 11:52:11
@@ -177,7 +177,7 @@
socket->dev = dev;
socket->op = ops;
dev->driver_data = socket;
- init_waitqueue_head(&socket->wait);
+ spin_lock_init(&socket->event_lock);
return socket->op->open(socket);
}

Index: drivers/pcmcia/pci_socket.h
===================================================================
RCS file: /inst/cvs/linux/drivers/pcmcia/Attic/pci_socket.h,v
retrieving revision 1.1.2.5
diff -u -r1.1.2.5 pci_socket.h
--- drivers/pcmcia/pci_socket.h 2000/07/06 15:28:43 1.1.2.5
+++ drivers/pcmcia/pci_socket.h 2000/11/16 11:52:11
@@ -18,9 +18,11 @@
void *info;
struct pci_socket_ops *op;
socket_cap_t cap;
- wait_queue_head_t wait;
+ spinlock_t event_lock;
unsigned int events;
struct socket_info_t *pcmcia_socket;
+ struct tq_struct tq_task;
+ struct timer_list poll_timer;

/* A few words of private data for the low-level driver.. */
unsigned int private[8];
Index: drivers/pcmcia/tcic.c
===================================================================
RCS file: /inst/cvs/linux/drivers/pcmcia/Attic/tcic.c,v
retrieving revision 1.1.2.9
diff -u -r1.1.2.9 tcic.c
--- drivers/pcmcia/tcic.c 2000/06/07 14:48:18 1.1.2.9
+++ drivers/pcmcia/tcic.c 2000/11/16 11:52:11
@@ -530,6 +530,28 @@

/*====================================================================*/

+static u_int pending_events[2];
+static spinlock_t pending_event_lock = SPIN_LOCK_UNLOCKED;
+
+static void tcic_bh(void *dummy)
+{
+ u_int events;
+ int i;
+
+ for (i=0; i < sockets; i++) {
+ spin_lock_irq(&pending_event_lock);
+ events = pending_events[i];
+ pending_events[i] = 0;
+ spin_unlock_irq(&pending_event_lock);
+ if (socket_table[i].handler)
+ socket_table[i].handler(socket_table[i].info, events);
+ }
+}
+
+static struct tq_struct tcic_task = {
+ routine: tcic_bh
+};
+
static void tcic_interrupt(int irq, void *dev, struct pt_regs *regs)
{
int i, quick = 0;
@@ -567,9 +589,13 @@
events |= (latch & TCIC_SSTAT_RDY) ? SS_READY : 0;
events |= (latch & TCIC_SSTAT_LBAT1) ? SS_BATDEAD : 0;
events |= (latch & TCIC_SSTAT_LBAT2) ? SS_BATWARN : 0;
+ }
+ if (events) {
+ spin_lock(&pending_event_lock);
+ pending_events[i] |= events;
+ spin_unlock(&pending_event_lock);
+ pcmcia_queue_task(&tcic_task);
}
- if (events)
- socket_table[i].handler(socket_table[i].info, events);
}

/* Schedule next poll, if needed */
Index: drivers/pcmcia/yenta.c
===================================================================
RCS file: /inst/cvs/linux/drivers/pcmcia/Attic/yenta.c,v
retrieving revision 1.1.2.25
diff -u -r1.1.2.25 yenta.c
--- drivers/pcmcia/yenta.c 2000/11/10 14:56:32 1.1.2.25
+++ drivers/pcmcia/yenta.c 2000/11/16 11:52:11
@@ -10,7 +10,10 @@
#include <linux/delay.h>
#include <linux/module.h>

+#include <pcmcia/version.h>
+#include <pcmcia/cs_types.h>
#include <pcmcia/ss.h>
+#include <pcmcia/cs.h>

#include <asm/io.h>

@@ -464,6 +467,20 @@
return events;
}

+
+static void yenta_bh(void *data)
+{
+ pci_socket_t *socket = data;
+ unsigned int events;
+
+ spin_lock_irq(&socket->event_lock);
+ events = socket->events;
+ socket->events = 0;
+ spin_unlock_irq(&socket->event_lock);
+ if (socket->handler)
+ socket->handler(socket->info, events);
+}
+
static void yenta_interrupt(int irq, void *dev_id, struct pt_regs *regs)
{
unsigned int events;
@@ -471,11 +488,22 @@

events = yenta_events(socket);
if (events) {
+ spin_lock(&socket->event_lock);
socket->events |= events;
- wake_up_interruptible(&socket->wait);
+ spin_unlock(&socket->event_lock);
+ pcmcia_queue_task(&socket->tq_task);
}
}

+static void yenta_interrupt_wrapper(unsigned long data)
+{
+ pci_socket_t *socket = (pci_socket_t *) data;
+
+ yenta_interrupt(0, (void *)socket, NULL);
+ socket->poll_timer.expires = jiffies + HZ;
+ add_timer(&socket->poll_timer);
+}
+
/*
* Only probe "regular" interrupts, don't
* touch dangerous spots like the mouse irq,
@@ -545,24 +573,24 @@

extern void cardbus_register(pci_socket_t *socket);

-/*
- * Watch a socket every second (and possibly in a
- * more timely manner if the state change interrupt
- * works..)
+/*
+ * 'Bottom half' for the yenta_open routine. Allocate the interrupt line
+ * and register the socket with the upper layers.
*/
-static int yenta_socket_thread(void * data)
+static void yenta_open_bh(void * data)
{
pci_socket_t * socket = (pci_socket_t *) data;
- DECLARE_WAITQUEUE(wait, current);

- MOD_INC_USE_COUNT;
- daemonize();
- strcpy(current->comm, "CardBus Watcher");
+ /* It's OK to overwrite this now */
+ socket->tq_task.routine = yenta_bh;

- if (socket->cb_irq && request_irq(socket->cb_irq, yenta_interrupt, SA_SHIRQ, socket->dev->name, socket)) {
- printk ("Yenta: unable to register irq %d\n", socket->cb_irq);
- MOD_DEC_USE_COUNT;
- return (1);
+ if (!socket->cb_irq || request_irq(socket->cb_irq, yenta_interrupt, SA_SHIRQ, socket->dev->name, socket)) {
+ /* No IRQ or request_irq failed. Poll */
+ socket->cb_irq = 0; /* But zero is a valid IRQ number. */
+ socket->poll_timer.function = yenta_interrupt_wrapper;
+ socket->poll_timer.data = (unsigned long)socket;
+ socket->poll_timer.expires = jiffies + HZ;
+ add_timer(&socket->poll_timer);
}

/* Figure out what the dang thing can do for the PCMCIA layer... */
@@ -572,23 +600,7 @@
/* Register it with the pcmcia layer.. */
cardbus_register(socket);

- do {
- unsigned int events = socket->events | yenta_events(socket);
-
- if (events) {
- socket->events = 0;
- if (socket->handler)
- socket->handler(socket->info, events);
- }
-
- current->state = TASK_INTERRUPTIBLE;
- add_wait_queue(&socket->wait, &wait);
- if (!socket->events)
- schedule_timeout(HZ);
- remove_wait_queue(&socket->wait, &wait);
- } while (!signal_pending(current));
MOD_DEC_USE_COUNT;
- return 0;
}

static void yenta_clear_maps(pci_socket_t *socket)
@@ -745,6 +757,9 @@
{
if (sock->cb_irq)
free_irq(sock->cb_irq, sock);
+ else
+ del_timer_sync(&sock->poll_timer);
+
if (sock->base)
iounmap(sock->base);
}
@@ -835,8 +850,17 @@
}
}
}
+
+ /* Get the PCMCIA kernel thread to complete the
+ initialisation later. We can't do this here,
+ because, er, because Linus says so :)
+ */
+ socket->tq_task.routine = yenta_open_bh;
+ socket->tq_task.data = socket;
+
+ MOD_INC_USE_COUNT;
+ pcmcia_queue_task(&socket->tq_task);

- kernel_thread(yenta_socket_thread, socket, CLONE_FS | CLONE_FILES | CLONE_SIGHAND);
return 0;
}

Index: include/linux/kernel.h
===================================================================
RCS file: /inst/cvs/linux/include/linux/kernel.h,v
retrieving revision 1.1.1.1.2.11
diff -u -r1.1.1.1.2.11 kernel.h
--- include/linux/kernel.h 2000/09/26 18:57:59 1.1.1.1.2.11
+++ include/linux/kernel.h 2000/11/16 11:52:11
@@ -45,10 +45,14 @@
#define FASTCALL(x) x
#endif

+struct semaphore;
+
extern struct notifier_block *panic_notifier_list;
NORET_TYPE void panic(const char * fmt, ...)
__attribute__ ((NORET_AND format (printf, 1, 2)));
NORET_TYPE void do_exit(long error_code)
+ ATTRIB_NORET;
+NORET_TYPE void up_and_exit(struct semaphore *, long)
ATTRIB_NORET;
extern unsigned long simple_strtoul(const char *,char **,unsigned int);
extern long simple_strtol(const char *,char **,unsigned int);
Index: include/pcmcia/cs.h
===================================================================
RCS file: /inst/cvs/linux/include/pcmcia/Attic/cs.h,v
retrieving revision 1.1.2.8
diff -u -r1.1.2.8 cs.h
--- include/pcmcia/cs.h 2000/09/07 08:26:16 1.1.2.8
+++ include/pcmcia/cs.h 2000/11/16 11:52:11
@@ -443,6 +443,7 @@
int pcmcia_map_mem_page(window_handle_t win, memreq_t *req);
int pcmcia_modify_configuration(client_handle_t handle, modconf_t *mod);
int pcmcia_modify_window(window_handle_t win, modwin_t *req);
+void pcmcia_queue_task(struct tq_struct *task);
int pcmcia_register_client(client_handle_t *handle, client_reg_t *req);
int pcmcia_release_configuration(client_handle_t handle);
int pcmcia_release_io(client_handle_t handle, io_req_t *req);
Index: kernel/exit.c
===================================================================
RCS file: /inst/cvs/linux/kernel/exit.c,v
retrieving revision 1.3.2.29
diff -u -r1.3.2.29 exit.c
--- kernel/exit.c 2000/09/18 13:54:01 1.3.2.29
+++ kernel/exit.c 2000/11/16 11:52:11
@@ -467,6 +467,14 @@
goto fake_volatile;
}

+NORET_TYPE void up_and_exit(struct semaphore *sem, long code)
+{
+ if (sem)
+ up(sem);
+
+ do_exit(code);
+}
+
asmlinkage long sys_exit(int error_code)
{
do_exit((error_code&0xff)<<8);
Index: kernel/ksyms.c
===================================================================
RCS file: /inst/cvs/linux/kernel/ksyms.c,v
retrieving revision 1.3.2.65
diff -u -r1.3.2.65 ksyms.c
--- kernel/ksyms.c 2000/11/13 11:00:18 1.3.2.65
+++ kernel/ksyms.c 2000/11/16 11:52:11
@@ -417,6 +417,7 @@
EXPORT_SYMBOL(iomem_resource);

/* process management */
+EXPORT_SYMBOL(up_and_exit);
EXPORT_SYMBOL(__wake_up);
EXPORT_SYMBOL(wake_up_process);
EXPORT_SYMBOL(sleep_on);

--
dwmw2

2000-11-16 16:39:42

by Alan

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

> It's purposefully not on Ted's critical list, the official line is "use
> pcmcia_cs external package" if you need i82365 or tcic instead of yenta
> AFAIK. However... fixing things and being able to support all pcmcia
> and cardbus adapters would be wonderful.

>From a practical point of view that currently means 'delete Linus tree pcmcia
regardless of what you are doing' since the modules from David Hinds and Linus
pcmcia are not 100% binary compatible for all cases.

It isnt possible for anyone to ship a useful system with Linus pcmcia unless
the ISA stuff is fixed

Alan

2000-11-16 16:46:42

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

Alan Cox wrote:
> the modules from David Hinds and Linus
> pcmcia are not 100% binary compatible for all cases.

What cases are these?

David's been pretty good about putting 2.4.x support into pcmcia_cs
package...

--
Jeff Garzik |
Building 1024 | The chief enemy of creativity is "good" sense
MandrakeSoft | -- Picasso

2000-11-16 16:51:22

by Alan

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

> > pcmcia are not 100% binary compatible for all cases.
>
> What cases are these?
>
> David's been pretty good about putting 2.4.x support into pcmcia_cs
> package...

Build a tree with the kernel pcmcia cs, build the pcmcia modules from David
Hinds on the same tree. Attempt to randomly load David's driver modules from
yenta and vice versa.

2000-11-16 16:59:12

by Tobias Ringstrom

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

On Thu, 16 Nov 2000, David Woodhouse wrote:
>
> [email protected] said:
> > Umm.. Linus drivers dont appear to be SMP safe on unload
>
> AFAIK, no kernel threads are currently SMP safe on unload. However,
> the PCMCIA thread would be safe with the patch below, and we could fairly
> easily convert the others to use up_and_exit() once it's available.
>
> Anyone using PCMCIA or CardBus with 2.4, even if you have a non-CardBus
> i82365 or TCIC controller for which the driver was disabled in test11-pre5,
> please could you test this? Especially if you have TCIC, in fact, because
> it's already been tested successfully on yenta and i82365.
>
> (pcmcia-dif-7). Linus, this is the full patch against pre5, including the
> incremental part I sent this morning.

I have finally managed to find time to test it, and after five minutes, I
must say that it looks very good. I was able to both plug and unplug
cards, and cardmgr did what it was supposed to do. If something bad turns
up, I will let you know.

I vote for inclusion! :-)

/Tobias

dmesg info:

Linux PCMCIA Card Services 3.1.22
options: [pci] [cardbus] [pm]
Intel PCIC probe:
Vadem VG-469 ISA-to-PCMCIA at port 0x3e0 ofs 0x00, 2 sockets
host opts [0]: none
host opts [1]: none
ISA irqs (scanned) = 3,4,5,7 polling interval = 1000 ms

2000-11-16 21:57:29

by Theodore Tso

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

Date: Sat, 1 Jan 2000 02:54:52 +0000
From: Pavel Machek <[email protected]>
Cc: David Woodhouse <[email protected]>, David Hinds <[email protected]>,
[email protected], [email protected],
[email protected], [email protected]

> It's purposefully not on Ted's critical list, the official line is "use
> pcmcia_cs external package" if you need i82365 or tcic instead of yenta

Ted, is this true? It would be wonderfull to be able to use i82365 without
need for pcmcia_cs...

I think in-kernel pcmcia crashing even on simple things *is* critical bug.

It was Linus who said that Pcmcia crashing wasn't necessarily a critical
bug since 2.2 didn't have Pcmcia support in-kernel, and that for 2.4, it
was likely that most distributions would be best served to ship with
David Hind's external pcmcia driver.

That was several months ago, and perhaps things have changed. But that
was I didn't spend time worrying about tracking PCMCIA bug reports;
there were a non-trivial number of them, and they were mostly of the
form "doesn't work on XXX hardware", "causes kernel oops on YYYY
hardware", etc.

- Ted

2000-11-16 22:12:31

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

On Thu, Nov 16, 2000 at 01:26:36PM -0800, [email protected] wrote:
>
> > Ted, is this true? It would be wonderfull to be able to use i82365 without
> > need for pcmcia_cs...
>
> > I think in-kernel pcmcia crashing even on simple things *is* critical bug.

It wasn't a critical bug, in the sense that because the i82365 driver
did not work, it was not even offered as a config option. If anything
it was merely a missing feature. And not a critical one, since the
feature was available outside of the kernel.

It is a moot point, since i82365 works with the new patches.

> That was several months ago, and perhaps things have changed. But that
> was I didn't spend time worrying about tracking PCMCIA bug reports;
> there were a non-trivial number of them, and they were mostly of the
> form "doesn't work on XXX hardware", "causes kernel oops on YYYY
> hardware", etc.

Some of these have been resolved, but some remain. They have not been
easy things to decipher since they involve interactions between the
PCI subsystem, PCMCIA, and specific hardware.

-- Dave

2000-11-17 09:46:55

by Russell King

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

Alan Cox writes:
> >From a practical point of view that currently means 'delete Linus tree pcmcia
> regardless of what you are doing' since the modules from David Hinds and Linus
> pcmcia are not 100% binary compatible for all cases.

However, deleting that code would render a significant number of ARM platforms
without PCMCIA support, which would be real bad.
_____
|_____| ------------------------------------------------- ---+---+-
| | Russell King [email protected] --- ---
| | | | http://www.arm.linux.org.uk/personal/aboutme.html / / |
| +-+-+ --- -+-
/ | THE developer of ARM Linux |+| /|\
/ | | | --- |
+-+-+ ------------------------------------------------- /\\\ |

2000-11-17 11:25:45

by Alan

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

> > regardless of what you are doing' since the modules from David Hinds and Linus
> > pcmcia are not 100% binary compatible for all cases.
>
> However, deleting that code would render a significant number of ARM platforms
> without PCMCIA support, which would be real bad.

It would actually have made no difference as said code didnt actually work
anyway. Dwmw2 seems to have solved that

2000-11-17 16:47:48

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

On Fri, 17 Nov 2000, Russell King wrote:

> Alan Cox writes:
> > >From a practical point of view that currently means 'delete Linus tree pcmcia
> > regardless of what you are doing' since the modules from David Hinds and Linus
> > pcmcia are not 100% binary compatible for all cases.
>
> However, deleting that code would render a significant number of ARM platforms
> without PCMCIA support, which would be real bad.

Right now, I suspect that the in-kernel pcmcia code is actually at the
point where it _is_ possible to use it. David Hinds has been keeping the
cs layer in synch with the external versions, and tons of people have
helped make the low-level drivers stable again.

If somebody still has a problem with the in-kernel stuff, speak up.

Linus

2000-11-17 16:52:18

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

On Fri, 17 Nov 2000, Alan Cox wrote:

> > > regardless of what you are doing' since the modules from David Hinds and Linus
> > > pcmcia are not 100% binary compatible for all cases.
> >
> > However, deleting that code would render a significant number of ARM platforms
> > without PCMCIA support, which would be real bad.
>
> It would actually have made no difference as said code didnt actually work
> anyway. Dwmw2 seems to have solved that

Alan, Russell is talking about CardBus controllers (it's also PCMCIA, in
fact, these days it's the _only_ pcmcia in any machine made less than five
years ago).

The patches to get i82365 and TCIC up and running again are interesting
mainly for laptops with i486 CPUs and for desktops with pcmcia add-in
cards (which are basically always ISA i82365-clones). They aren't
interesting to ARM, I suspect.

Linus

2000-11-17 17:00:30

by Alan

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

> Alan, Russell is talking about CardBus controllers (it's also PCMCIA, in
> fact, these days it's the _only_ pcmcia in any machine made less than five
> years ago).

I have at least two machines here that are < 2 years old but disagree
with you. Once is only months old.

> The patches to get i82365 and TCIC up and running again are interesting
> mainly for laptops with i486 CPUs and for desktops with pcmcia add-in
> cards (which are basically always ISA i82365-clones). They aren't
> interesting to ARM, I suspect.

Much ARM stuff has embedded PCMCIA controllers not cardbus, ditto most MIPS
WinCE hardware

Alan

2000-11-17 17:05:52

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

[email protected] said:
> If somebody still has a problem with the in-kernel stuff, speak up.

I have an i82092AA evaluation board:

00:06.0 PCMCIA bridge: Intel Corporation 82092AA_0 (rev 02)
Flags: slow devsel, IRQ 27
I/O ports at 8400 [size=4]

I have three problems:

1. I have to specify the i365_base parameter when loading i82365,o

2. Even when I specify cs_irq=27, it resorts to polling:

Intel PCIC probe:
Intel i82365sl DF ISA-to-PCMCIA at port 0x8400 ofs 0x00, 2 sockets
host opts [0]: none
host opts [1]: none
ISA irqs (default) = none! polling interval = 1000 ms
Intel i82365sl DF ISA-to-PCMCIA at port 0x8400 ofs 0x80, 2 sockets
host opts [2]: none
host opts [3]: none
ISA irqs (default) = none! polling interval = 1000 ms

3. Note that it finds no IRQs available for the cards' IREQ to use. This is
on an Alpha SX164.

I'll fix it over the weekend - basically it just looks like a little too
much was stripped out of i82365.c when we started handling CardBus bridges
elsewhere. We ought to still handle PCI->PCMCIA bridges which aren't
CardBus-capable.

As a separate issue, the IDE on the same chip appears to be confusing the
kernel. I seem to end up with the generic IDE driver driving the on-board
CY82C693, which means I can't do DMA. That may be SRM's fault, though.

00:06.1 IDE interface: Intel Corporation 82092AA_1 (rev 02) (prog-if 00 [])
Flags: slow devsel, IRQ 31
I/O ports at 1090 [size=8]
I/O ports at 1098
I/O ports at 10a0 [size=8]
I/O ports at 10a8 [size=8]

00:08.1 IDE interface: Contaq Microsystems 82c693 (prog-if 80 [Master])
Flags: bus master, medium devsel, latency 0
I/O ports at 01f0 [size=8]
I/O ports at 03f4
I/O ports at 1080 [size=16]

ide: Assuming 33MHz system bus speed for PIO modes; override with idebus=xx
PCI_IDE: unknown IDE controller on PCI bus 00 device 31, VID=8086, DID=1222
PCI_IDE: chipset revision 2
PCI_IDE: not 100% native mode: will probe irqs later
CY82C693: IDE controller on PCI bus 00 dev 41
CY82C693: chipset revision 0
CY82C693: not 100% native mode: will probe irqs later
CY82C693U driver v0.34 99-13-12 Andreas S. Krebs ([email protected])
CY82C693: port 0x01f0 already claimed by ide0
CY82C693: port 0x0170 already claimed by ide1
CY82C693: neither IDE port enabled (BIOS)

--
dwmw2

2000-11-17 17:06:01

by Russell King

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

Linus Torvalds writes:
> On Fri, 17 Nov 2000, Alan Cox wrote:
> Alan, Russell is talking about CardBus controllers (it's also PCMCIA, in
> fact, these days it's the _only_ pcmcia in any machine made less than five
> years ago).

Actually, I wasn't. I was referring to the embedded-type ARM devices of which
I have two sat in front of me (both are manufactured within the past year so
are "current") and about half the platforms that "ARM Linux" covers have some
form of PCMCIA.

Some ARM CPUs even have the PCMCIA controller embedded within them (look at
arch/arm/tools/mach-types - each entry containing a reference to SA1100 means
that particular platform has the ability to use PCMCIA).

All of the drivers for these devices were written around the in-kernel PCMCIA
code.
_____
|_____| ------------------------------------------------- ---+---+-
| | Russell King [email protected] --- ---
| | | | http://www.arm.linux.org.uk/personal/aboutme.html / / |
| +-+-+ --- -+-
/ | THE developer of ARM Linux |+| /|\
/ | | | --- |
+-+-+ ------------------------------------------------- /\\\ |

2000-11-17 17:06:22

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

On Fri, 17 Nov 2000, Alan Cox wrote:

> > Alan, Russell is talking about CardBus controllers (it's also PCMCIA, in
> > fact, these days it's the _only_ pcmcia in any machine made less than five
> > years ago).
>
> I have at least two machines here that are < 2 years old but disagree
> with you. Once is only months old.

Who makes those pieces of crap? And who _buys_ them? I can understand it
in embedded stuff simply because the chips are simpler and smaller, but in
a laptop you should definitely try to avoid it.

Linus

2000-11-17 17:10:11

by Alan

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

> Who makes those pieces of crap? And who _buys_ them? I can understand it
> in embedded stuff simply because the chips are simpler and smaller, but in
> a laptop you should definitely try to avoid it.

These are mostly handhelds where the space/power issues get important. And as
to who makes them, well intel and various mips chip makers. Who buys them.
Everyone frantically trying to get to the shop as a new iPaq stock comes in 8)

Alan

2000-11-17 17:11:42

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

David Woodhouse wrote:
>
> [email protected] said:
> > If somebody still has a problem with the in-kernel stuff, speak up.
>
> I have an i82092AA evaluation board:
>
> 00:06.0 PCMCIA bridge: Intel Corporation 82092AA_0 (rev 02)
> Flags: slow devsel, IRQ 27
> I/O ports at 8400 [size=4]
>
> I have three problems:
>
> 1. I have to specify the i365_base parameter when loading i82365,o
>
> 2. Even when I specify cs_irq=27, it resorts to polling:
>
> Intel PCIC probe:
> Intel i82365sl DF ISA-to-PCMCIA at port 0x8400 ofs 0x00, 2 sockets
> host opts [0]: none
> host opts [1]: none
> ISA irqs (default) = none! polling interval = 1000 ms
> Intel i82365sl DF ISA-to-PCMCIA at port 0x8400 ofs 0x80, 2 sockets
> host opts [2]: none
> host opts [3]: none
> ISA irqs (default) = none! polling interval = 1000 ms

For these two, it sounds to me like you need to be doing a PCI probe,
and getting the irq and I/O port info from pci_dev. And calling
pci_enable_device, which may or may not be a showstopper here...

Jeff

--
Jeff Garzik |
Building 1024 | The chief enemy of creativity is "good" sense
MandrakeSoft | -- Picasso

2000-11-17 17:14:32

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

[email protected] said:
> For these two, it sounds to me like you need to be doing a PCI probe,
> and getting the irq and I/O port info from pci_dev. And calling
> pci_enable_device, which may or may not be a showstopper here...

Yep. The same code is already present in David Hinds' i82365.c, but appears
to have been stripped out when CardBus sockets started to be supported
elsewhere.

--
dwmw2

2000-11-17 17:18:22

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

On Fri, 17 Nov 2000, Jeff Garzik wrote:
> >
> > 2. Even when I specify cs_irq=27, it resorts to polling:
> >
> > Intel PCIC probe:
> > Intel i82365sl DF ISA-to-PCMCIA at port 0x8400 ofs 0x00, 2 sockets
> > host opts [0]: none
> > host opts [1]: none
> > ISA irqs (default) = none! polling interval = 1000 ms
> > Intel i82365sl DF ISA-to-PCMCIA at port 0x8400 ofs 0x80, 2 sockets
> > host opts [2]: none
> > host opts [3]: none
> > ISA irqs (default) = none! polling interval = 1000 ms
>
> For these two, it sounds to me like you need to be doing a PCI probe,
> and getting the irq and I/O port info from pci_dev. And calling
> pci_enable_device, which may or may not be a showstopper here...

The i82365 stuff actually used to do much of this, but it was so
intimately intertwined with the cardbus handling that I pruned it out for
my sanity.

It should be possible to do the same thing with a nice simple concentrated
PCI probe, instead of having stuff quite as spread out as it used to be.

As to why it doesn't show any ISA interrupts, who knows... Some of the PCI
PCMCIA bridges need to be initialized.

Linus

2000-11-17 17:20:02

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

[email protected] said:
> It should be possible to do the same thing with a nice simple
> concentrated PCI probe, instead of having stuff quite as spread out as
> it used to be.

That's the plan.

> As to why it doesn't show any ISA interrupts, who knows... Some of
> the PCI PCMCIA bridges need to be initialized.

ISTR it worked fine in an x86 box. I'll play with it over the weekend.

--
dwmw2

2000-11-17 20:30:49

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

> 2. Even when I specify cs_irq=27, it resorts to polling:
>
> Intel PCIC probe:
> Intel i82365sl DF ISA-to-PCMCIA at port 0x8400 ofs 0x00, 2 sockets
> host opts [0]: none
> host opts [1]: none
> ISA irqs (default) = none! polling interval = 1000 ms

The driver means it when it says "ISA irqs". A value of 27 is not
valid for cs_irq because this is an ISA irq number and must be 0..15;
this is not a property of the host system, this is a property of the
i82365 register specification. PCI interrupts have to be handled
differently (well, the stripped-down i82365 driver just can't handle
them at all)

-- Dave

2000-11-17 21:02:14

by Barry K. Nathan

[permalink] [raw]

Subject: 2.4's internal PCMCIA works for me (was Re: [PATCH] pcmcia event thread)

Linus Torvalds wrote:

> Right now, I suspect that the in-kernel pcmcia code is actually at the
> point where it _is_ possible to use it. David Hinds has been keeping the
> cs layer in synch with the external versions, and tons of people have
> helped make the low-level drivers stable again.
>
> If somebody still has a problem with the in-kernel stuff, speak up.

I'm going to speak up as someone who uses the in-kernel code without
problems (on my Dell Inspiron 5000e and Dell/3Com CardBus 10/100 NIC).
The in-kernel support always seems to get a bad rap, so I want to mention
that, for some people anyway, the in-kernel code works just as well as
the external code.

I do understand that the in-kernel support isn't as mature as the external
support yet. However, it isn't universally broken and useless either.

-Barry K. Nathan <[email protected]>

2000-11-17 21:36:35

[permalink] [raw]

Subject: Re: 2.4's internal PCMCIA works for me (was Re: [PATCH] pcmcia event thread)

On Fri, Nov 17, 2000 at 12:30:38PM -0800, Barry K. Nathan wrote:
>
> I do understand that the in-kernel support isn't as mature as the external
> support yet. However, it isn't universally broken and useless either.

That's certainly true; it should work fine for the large majority of
configurations. I think the non-platform-specific issues are mostly
resolved.

-- Dave

2000-11-18 10:26:48

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

Linus Torvalds wrote:
[...]

> If somebody still has a problem with the in-kernel stuff, speak up.

The kernel's irq detection for the card sockets doesn't work for me. It's the NEC
Versa LX story. The DH code also reports no IRQ found but still figures out a
working IRQ (normally 3) and assigns it for the tulip card. I use the i82365 module
w/ the DH code. The below is the output of the kernel pcmcia code.

[side note for David Hinds - please let me make both kernel pcmcia core modules and
dh modules without having to do workarounds]

(pci=biosirq makes no change in any discernable fashion, parts omitted for technical
clarity)

# dmesg
Linux PCMCIA Card Services 3.1.22
options: [pci] [cardbus] [pm]
PCI: No IRQ known for interrupt pin B of device 00:03.1. Please try using
pci=biosirq.
PCI: No IRQ known for interrupt pin A of device 00:03.0. Please try using
pci=biosirq.
Intel PCIC probe: not found.
Databook TCIC-2 PCMCIA probe: not found.
Yenta IRQ list 0c90, PCI irq0
Socket status: 30000860
Yenta IRQ list 0898, PCI irq0
Socket status: 30000046
[...]
cs: cb_alloc(bus 2): vendor 0x1011, device 0x0019
PCI: Enabling device 02:00.0 (0000 -> 0003)
Linux Tulip driver version 0.9.11 (November 3, 2000)
PCI: No IRQ known for interrupt pin A of device 02:00.0. Please try using
pci=biosirq.
PCI: Setting latency timer of device 02:00.0 to 64
eth0: Digital DS21143 Tulip rev 65 at 0x1800, 00:E0:98:70:1E:AF, IRQ 0.
eth0: EEPROM default media type Autosense.
eth0: Index #0 - Media MII (#11) described by a 21142 MII PHY (3) block.
eth0: MII transceiver #0 config 3000 status 7809 advertising 01e1.
call_usermodehelper[/sbin/hotplug]: no root fs

# cardctl config
Socket 0:
Vcc 3.3V Vpp1 3.3V Vpp2 3.3V
interface type is "cardbus"
function 0:
Socket 1:
not configured

# lspci -vvv
00:03.0 CardBus bridge: Ricoh Co Ltd RL5c478 (rev 03)
Subsystem: NEC Corporation: Unknown device 8039
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping-
SERR- FastB2B-
Status: Cap+ 66Mhz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort-
<MAbort- >SERR- <PERR-
Latency: 168
Interrupt: pin A routed to IRQ 0
Region 0: Memory at 10000000 (32-bit, non-prefetchable) [size=4K]
Bus: primary=00, secondary=02, subordinate=02, sec-latency=176
Memory window 0: 10c00000-10fff000 (prefetchable)
Memory window 1: 11000000-113ff000
I/O window 0: 00001800-000018ff
I/O window 1: 00001c00-00001cff
BridgeCtl: Parity- SERR- ISA- VGA- MAbort- >Reset- 16bInt+ PostWrite+
16-bit legacy interface ports at 0001

00:03.1 CardBus bridge: Ricoh Co Ltd RL5c478 (rev 03)
Subsystem: NEC Corporation: Unknown device 8039
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping-
SERR- FastB2B-
Status: Cap+ 66Mhz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort-
<MAbort- >SERR- <PERR-
Latency: 168
Interrupt: pin B routed to IRQ 0
Region 0: Memory at 10001000 (32-bit, non-prefetchable) [size=4K]
Bus: primary=00, secondary=06, subordinate=06, sec-latency=176
Memory window 0: 10400000-107ff000 (prefetchable)
Memory window 1: 10800000-10bff000
I/O window 0: 00001000-000010ff
I/O window 1: 00001400-000014ff
BridgeCtl: Parity- SERR- ISA- VGA- MAbort- >Reset- 16bInt- PostWrite+
16-bit legacy interface ports at 0001

# dump_pirq
Interrupt routing table found at address 0xf5a80:
Version 1.0, size 0x0080
Interrupt router is device 00:07.0
PCI exclusive interrupt mask: 0x0000
Compatible router: vendor 0x8086 device 0x1234

Device 00:03.0 (slot 0):
INTA: link 0x60, irq mask 0x0420
INTB: link 0x61, irq mask 0x0420

Interrupt router: Intel 82371AB PIIX4/PIIX4E PCI-to-ISA bridge
PIRQ1 (link 0x60): irq 10
PIRQ2 (link 0x61): irq 5
PIRQ3 (link 0x62): unrouted
PIRQ4 (link 0x63): irq 9
Serial IRQ: [enabled] [continuous] [frame=21] [pulse=4]

Attachments:

2000-11-18 16:34:42

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

On Sat, 18 Nov 2000, David Ford wrote:

> Linus Torvalds wrote:
> [...]
>
> > If somebody still has a problem with the in-kernel stuff, speak up.
>
> The kernel's irq detection for the card sockets doesn't work for me. It's the NEC
> Versa LX story. The DH code also reports no IRQ found but still figures out a
> working IRQ (normally 3) and assigns it for the tulip card. I use the i82365 module
> w/ the DH code. The below is the output of the kernel pcmcia code.

> PCI: No IRQ known for interrupt pin B of device 00:03.1. Please try using
> pci=biosirq.
> PCI: No IRQ known for interrupt pin A of device 00:03.0. Please try using
> pci=biosirq.

Strange. Your interrupt router is a bog-standard PIIX4, we know how to
route the thing, AND your device shows up:

> # dump_pirq
> Interrupt routing table found at address 0xf5a80:
> Version 1.0, size 0x0080
> Interrupt router is device 00:07.0
> PCI exclusive interrupt mask: 0x0000
> Compatible router: vendor 0x8086 device 0x1234
>
> Device 00:03.0 (slot 0):
> INTA: link 0x60, irq mask 0x0420
> INTB: link 0x61, irq mask 0x0420
>
> Interrupt router: Intel 82371AB PIIX4/PIIX4E PCI-to-ISA bridge
> PIRQ1 (link 0x60): irq 10
> PIRQ2 (link 0x61): irq 5
> PIRQ3 (link 0x62): unrouted
> PIRQ4 (link 0x63): irq 9
> Serial IRQ: [enabled] [continuous] [frame=21] [pulse=4]

Can you (you've probably done this before, but anyway) enable DEBUG in
arch/i386/kernel/pci-i386.h? I wonder if the kernel for some strange
reason doesn't find your router, even though "dump_pirq" obviously does..
If there's something wrong with the checksumming for example..

Linus

2000-11-18 20:11:50

by Pavel Machek

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

Hi!

> [email protected] said:
> > Umm.. Linus drivers dont appear to be SMP safe on unload
>
> AFAIK, no kernel threads are currently SMP safe on unload. However,
> the PCMCIA thread would be safe with the patch below, and we could fairly
> easily convert the others to use up_and_exit() once it's available.
>
> Anyone using PCMCIA or CardBus with 2.4, even if you have a non-CardBus
> i82365 or TCIC controller for which the driver was disabled in test11-pre5,
> please could you test this? Especially if you have TCIC, in fact, because
> it's already been tested successfully on yenta and i82365.

Thanx a lot, it fixed problems with my i82365.

Pavel
--
I'm [email protected]. "In my country we have almost anarchy and I don't care."
Panos Katsaloulis describing me w.r.t. patents at [email protected]

2000-11-18 22:46:40

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

On Sat, Nov 18, 2000 at 08:03:51AM -0800, Linus Torvalds wrote:
>
> Strange. Your interrupt router is a bog-standard PIIX4, we know how to
> route the thing, AND your device shows up:
>
> > # dump_pirq
> > Interrupt routing table found at address 0xf5a80:
> > Version 1.0, size 0x0080
> > Interrupt router is device 00:07.0
> > PCI exclusive interrupt mask: 0x0000
> > Compatible router: vendor 0x8086 device 0x1234

Oh... the kernel pci-irq code looks for the "compatible router" if it
is set; if unset, then it looks up the ID's of the router device.

0x8086, 0x1234 is not a known router type, so the kernel decides it
can't interpret the routing table.

0x8086, 0x1234 is listed in pci_ids.h as an 82371MX. I'm suspicious
of that: the MX chipset has an 82443MX, not an 82371. In any case, I
think pci-irq.c should check both sets of ID's for a match.

-- Dave

2000-11-19 06:03:23

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

Linus Torvalds wrote:

> Can you (you've probably done this before, but anyway) enable DEBUG in
> arch/i386/kernel/pci-i386.h? I wonder if the kernel for some strange
> reason doesn't find your router, even though "dump_pirq" obviously does..
> If there's something wrong with the checksumming for example..

..building now.

-d

Attachments:

2000-11-19 06:06:32

[permalink] [raw]

Subject: Re: [PATCH] pcmcia event thread. (fwd)

On Sat, 18 Nov 2000, David Ford wrote:
> Linus Torvalds wrote:
>
> > Can you (you've probably done this before, but anyway) enable DEBUG in
> > arch/i386/kernel/pci-i386.h? I wonder if the kernel for some strange
> > reason doesn't find your router, even though "dump_pirq" obviously does..
> > If there's something wrong with the checksumming for example..
>
> ..building now.

Actually, try this patch first. It adds the PCI_DEVICE_ID_INTEL_82371MX
router type, and also makes the PCI router search fall back more
gracefully on the device it actually found if there is not an exact match
on the "compatible router" entry...

It should make Linux find and accept the chip you have. Knock wood.

Linus

--- v2.4.0-test10/linux/arch/i386/kernel/pci-irq.c Tue Oct 31 12:42:26 2000
+++ linux/arch/i386/kernel/pci-irq.c Sat Nov 18 21:11:19 2000
@@ -283,12 +297,19 @@
{ "PIIX", PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371FB_0, pirq_piix_get, pirq_piix_set },
{ "PIIX", PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371SB_0, pirq_piix_get, pirq_piix_set },
{ "PIIX", PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371AB_0, pirq_piix_get, pirq_piix_set },
+ { "PIIX", PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371MX, pirq_piix_get, pirq_piix_set },
{ "PIIX", PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82443MX_0, pirq_piix_get, pirq_piix_set },
+
{ "ALI", PCI_VENDOR_ID_AL, PCI_DEVICE_ID_AL_M1533, pirq_ali_get, pirq_ali_set },
+
{ "VIA", PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C586_0, pirq_via_get, pirq_via_set },
{ "VIA", PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C596, pirq_via_get, pirq_via_set },
{ "VIA", PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686, pirq_via_get, pirq_via_set },
+
{ "OPTI", PCI_VENDOR_ID_OPTI, PCI_DEVICE_ID_OPTI_82C700, pirq_opti_get, pirq_opti_set },
+
+ { "NatSemi", PCI_VENDOR_ID_CYRIX, PCI_DEVICE_ID_CYRIX_5520, pirq_cyrix_get, pirq_cyrix_set },
+
{ "default", 0, 0, NULL, NULL }
};

@@ -298,7 +319,6 @@
static void __init pirq_find_router(void)
{
struct irq_routing_table *rt = pirq_table;
- u16 rvendor, rdevice;
struct irq_router *r;

#ifdef CONFIG_PCI_BIOS
@@ -308,32 +328,31 @@
return;
}
#endif
- if (!(pirq_router_dev = pci_find_slot(rt->rtr_bus, rt->rtr_devfn))) {
+ /* fall back to default router if nothing else found */
+ pirq_router = pirq_routers + sizeof(pirq_routers) / sizeof(pirq_routers[0]) - 1;
+
+ pirq_router_dev = pci_find_slot(rt->rtr_bus, rt->rtr_devfn);
+ if (!pirq_router_dev) {
DBG("PCI: Interrupt router not found at %02x:%02x\n", rt->rtr_bus, rt->rtr_devfn);
- /* fall back to default router */
- pirq_router = pirq_routers + sizeof(pirq_routers) / sizeof(pirq_routers[0]) - 1;
return;
}
- if (rt->rtr_vendor) {
- rvendor = rt->rtr_vendor;
- rdevice = rt->rtr_device;
- } else {
- /*
- * Several BIOSes forget to set the router type. In such cases, we
- * use chip vendor/device. This doesn't guarantee us semantics of
- * PIRQ values, but was found to work in practice and it's still
- * better than not trying.
- */
- DBG("PCI: Guessed interrupt router ID from %s\n", pirq_router_dev->slot_name);
- rvendor = pirq_router_dev->vendor;
- rdevice = pirq_router_dev->device;
- }
- for(r=pirq_routers; r->vendor; r++)
- if (r->vendor == rvendor && r->device == rdevice)
+
+ for(r=pirq_routers; r->vendor; r++) {
+ /* Exact match against router table entry? Use it! */
+ if (r->vendor == rt->rtr_vendor && r->device == rt->rtr_device) {
+ pirq_router = r;
break;
- pirq_router = r;
- printk("PCI: Using IRQ router %s [%04x/%04x] at %s\n", r->name,
- rvendor, rdevice, pirq_router_dev->slot_name);
+ }
+ /* Match against router device entry? Use it as a fallback */
+ if (r->vendor == pirq_router_dev->vendor && r->device == pirq_router_dev->device) {
+ pirq_router = r;
+ }
+ }
+ printk("PCI: Using IRQ router %s [%04x/%04x] at %s\n",
+ pirq_router->name,
+ pirq_router_dev->vendor,
+ pirq_router_dev->device,
+ pirq_router_dev->slot_name);
}

static struct irq_info *pirq_get_info(struct pci_dev *dev, int pin)

2000-11-19 07:02:03

[permalink] [raw]

Subject: [FIXED!] Re: [PATCH] pcmcia event thread. (fwd)

It's time for a week long party!

It works great, interrupts are even delivered to the tulip handler :)

Attached is the boot dmesg in case any aesthetic changes are desired.

Ted, please scratch off the NEC Versa LX PCMCIA entries as fixed, I assume this patch will go in on the
next release.

MEGA thank yous,
-d

----
PCI: BIOS32 Service Directory structure at 0xc00fdb70
PCI: BIOS32 Service Directory entry at 0xfdb80
PCI: BIOS probe returned s=00 hw=01 ver=02.10 l=01
PCI: PCI BIOS revision 2.10 entry at 0xfdba1, last bus=1
PCI: Using configuration type 1
PCI: Probing PCI hardware
PCI: IDE base address fixup for 00:07.1
PCI: Scanning for ghost devices on bus 0
PCI: Scanning for ghost devices on bus 1
PCI: IRQ init
PCI: Interrupt Routing Table found at 0xc00f5a80
00:01 slot=00 0:60/0420 1:00/0000 2:00/0000 3:00/0000
00:03 slot=00 0:60/0420 1:61/0420 2:00/0000 3:00/0000
00:04 slot=00 0:61/0420 1:00/0000 2:00/0000 3:00/0000
00:05 slot=00 0:60/0420 1:00/0000 2:00/0000 3:00/0000
00:07 slot=00 0:fe/4000 1:ff/8000 2:00/0000 3:63/0200
00:11 slot=01 0:60/0420 1:61/0420 2:62/0800 3:61/0420
PCI: Using IRQ router PIIX [8086/7110] at 00:07.0
PCI: IRQ fixup
00:03.0: ignoring bogus IRQ 255
00:03.1: ignoring bogus IRQ 255
IRQ for 00:03.0(0) via 00:03.0 -> PIRQ 60, mask 0420, excl 0000 -> got IRQ 10
PCI: Found IRQ 10 for device 00:03.0
PCI: The same IRQ used for device 01:00.0
IRQ for 00:03.1(1) via 00:03.1 -> PIRQ 61, mask 0420, excl 0000 -> got IRQ 5
PCI: Found IRQ 5 for device 00:03.1
PCI: The same IRQ used for device 00:04.0
PCI: Allocating resources
PCI: Resource e0000000-efffffff (f=1208, d=0, p=0)
PCI: Resource 0000ec00-0000ecff (f=101, d=0, p=0)
PCI: Resource 0000ffa0-0000ffaf (f=101, d=0, p=0)
PCI: Resource 0000ef80-0000ef9f (f=101, d=0, p=0)
PCI: Resource fd000000-fdffffff (f=200, d=0, p=0)
PCI: Resource 0000dc00-0000dcff (f=101, d=0, p=0)
PCI: Resource feaff000-feafffff (f=200, d=0, p=0)
PCI: Sorting device list...
Limiting direct PCI/PCI transfers.

Linux PCMCIA Card Services 3.1.22
options: [pci] [cardbus] [pm]
Intel PCIC probe: not found.
Databook TCIC-2 PCMCIA probe: not found.
usb.c: registered new driver usbdevfs
usb.c: registered new driver hub
Yenta IRQ list 0898, PCI irq10
Socket status: 30000860
Yenta IRQ list 0898, PCI irq5
Socket status: 30000046

cs: cb_alloc(bus 2): vendor 0x1011, device 0x0019
PCI: Enabling device 02:00.0 (0000 -> 0003)
Linux Tulip driver version 0.9.11 (November 3, 2000)
PCI: Setting latency timer of device 02:00.0 to 64
eth1: Digital DS21143 Tulip rev 65 at 0x1800, 00:E0:98:70:1E:AF, IRQ 10.
eth1: EEPROM default media type Autosense.
eth1: Index #0 - Media MII (#11) described by a 21142 MII PHY (3) block.
eth1: MII transceiver #0 config 3000 status 7809 advertising 01e1.
call_usermodehelper[/sbin/hotplug]: no root fs

Linus Torvalds wrote:

> On Sat, 18 Nov 2000, David Ford wrote:
> > Linus Torvalds wrote:
> >
> > > Can you (you've probably done this before, but anyway) enable DEBUG in
> > > arch/i386/kernel/pci-i386.h? I wonder if the kernel for some strange
> > > reason doesn't find your router, even though "dump_pirq" obviously does..
> > > If there's something wrong with the checksumming for example..
> >
> > ..building now.
>
> Actually, try this patch first. It adds the PCI_DEVICE_ID_INTEL_82371MX
> router type, and also makes the PCI router search fall back more
> gracefully on the device it actually found if there is not an exact match
> on the "compatible router" entry...
>
> It should make Linux find and accept the chip you have. Knock wood.
>
> Linus
>
> --- v2.4.0-test10/linux/arch/i386/kernel/pci-irq.c Tue Oct 31 12:42:26 2000
> +++ linux/arch/i386/kernel/pci-irq.c Sat Nov 18 21:11:19 2000
> @@ -283,12 +297,19 @@
> { "PIIX", PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371FB_0, pirq_piix_get, pirq_piix_set },
> { "PIIX", PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371SB_0, pirq_piix_get, pirq_piix_set },
> { "PIIX", PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371AB_0, pirq_piix_get, pirq_piix_set },
> + { "PIIX", PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371MX, pirq_piix_get, pirq_piix_set },
> { "PIIX", PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82443MX_0, pirq_piix_get, pirq_piix_set },
> +
> { "ALI", PCI_VENDOR_ID_AL, PCI_DEVICE_ID_AL_M1533, pirq_ali_get, pirq_ali_set },
> +
> { "VIA", PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C586_0, pirq_via_get, pirq_via_set },
> { "VIA", PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C596, pirq_via_get, pirq_via_set },
> { "VIA", PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686, pirq_via_get, pirq_via_set },
> +
> { "OPTI", PCI_VENDOR_ID_OPTI, PCI_DEVICE_ID_OPTI_82C700, pirq_opti_get, pirq_opti_set },
> +
> + { "NatSemi", PCI_VENDOR_ID_CYRIX, PCI_DEVICE_ID_CYRIX_5520, pirq_cyrix_get, pirq_cyrix_set },
> +
> { "default", 0, 0, NULL, NULL }
> };
>
> @@ -298,7 +319,6 @@
> static void __init pirq_find_router(void)
> {
> struct irq_routing_table *rt = pirq_table;
> - u16 rvendor, rdevice;
> struct irq_router *r;
>
> #ifdef CONFIG_PCI_BIOS
> @@ -308,32 +328,31 @@
> return;
> }
> #endif
> - if (!(pirq_router_dev = pci_find_slot(rt->rtr_bus, rt->rtr_devfn))) {
> + /* fall back to default router if nothing else found */
> + pirq_router = pirq_routers + sizeof(pirq_routers) / sizeof(pirq_routers[0]) - 1;
> +
> + pirq_router_dev = pci_find_slot(rt->rtr_bus, rt->rtr_devfn);
> + if (!pirq_router_dev) {
> DBG("PCI: Interrupt router not found at %02x:%02x\n", rt->rtr_bus, rt->rtr_devfn);
> - /* fall back to default router */
> - pirq_router = pirq_routers + sizeof(pirq_routers) / sizeof(pirq_routers[0]) - 1;
> return;
> }
> - if (rt->rtr_vendor) {
> - rvendor = rt->rtr_vendor;
> - rdevice = rt->rtr_device;
> - } else {
> - /*
> - * Several BIOSes forget to set the router type. In such cases, we
> - * use chip vendor/device. This doesn't guarantee us semantics of
> - * PIRQ values, but was found to work in practice and it's still
> - * better than not trying.
> - */
> - DBG("PCI: Guessed interrupt router ID from %s\n", pirq_router_dev->slot_name);
> - rvendor = pirq_router_dev->vendor;
> - rdevice = pirq_router_dev->device;
> - }
> - for(r=pirq_routers; r->vendor; r++)
> - if (r->vendor == rvendor && r->device == rdevice)
> +
> + for(r=pirq_routers; r->vendor; r++) {
> + /* Exact match against router table entry? Use it! */
> + if (r->vendor == rt->rtr_vendor && r->device == rt->rtr_device) {
> + pirq_router = r;
> break;
> - pirq_router = r;
> - printk("PCI: Using IRQ router %s [%04x/%04x] at %s\n", r->name,
> - rvendor, rdevice, pirq_router_dev->slot_name);
> + }
> + /* Match against router device entry? Use it as a fallback */
> + if (r->vendor == pirq_router_dev->vendor && r->device == pirq_router_dev->device) {
> + pirq_router = r;
> + }
> + }
> + printk("PCI: Using IRQ router %s [%04x/%04x] at %s\n",
> + pirq_router->name,
> + pirq_router_dev->vendor,
> + pirq_router_dev->device,
> + pirq_router_dev->slot_name);
> }
>
> static struct irq_info *pirq_get_info(struct pci_dev *dev, int pin)

Attachments:

2000-11-19 07:22:41

by Andrew Park

[permalink] [raw]

Subject: neighbour table?

I get a message

neighbour table overflow

What does that mean? It seems that

net/ipv4/route.c

is the place where it prints this. But under what circumstances
does this happen?
Thanks

-A.

2000-11-19 07:28:22

[permalink] [raw]

Subject: Re: neighbour table?

Andrew Park wrote:

> I get a message
>
> neighbour table overflow
>
> What does that mean? It seems that
>
> net/ipv4/route.c
>
> is the place where it prints this. But under what circumstances
> does this happen?
> Thanks

It means you set the link state of eth0 up before lo.

Be sure lo is established before eth0 and you won't see this message.

-d

Attachments:

2000-11-19 10:59:39

by Eric W. Biederman

[permalink] [raw]

Subject: Re: neighbour table?

David Ford <[email protected]> writes:

> Andrew Park wrote:
>
> > I get a message
> >
> > neighbour table overflow
> >
> > What does that mean? It seems that
> >
> > net/ipv4/route.c
> >
> > is the place where it prints this. But under what circumstances
> > does this happen?
> > Thanks
>
> It means you set the link state of eth0 up before lo.
>
> Be sure lo is established before eth0 and you won't see this message.

Hmm. How does the interaction work. I've been meaning to track it for
a while but haven't yet.

>From the cases I have observed it seems to be connected with arp requests
that aren't answered. (I.e when something is misconfigured and you try to nfsroot off
of the wrong ip on your subnet)
And I keep thinking neighbour table underflow would have been a better message.

Eric

2000-11-19 11:10:54

[permalink] [raw]

Subject: Re: neighbour table?

"Eric W. Biederman" wrote:

> > Be sure lo is established before eth0 and you won't see this message.
>
> Hmm. How does the interaction work. I've been meaning to track it for
> a while but haven't yet.
>
> >From the cases I have observed it seems to be connected with arp requests
> that aren't answered. (I.e when something is misconfigured and you try to nfsroot off
> of the wrong ip on your subnet)
> And I keep thinking neighbour table underflow would have been a better message.

I'm not sure, I'm repeating what Alexey (iirc) has said in the past. I've been
there/done that and sure enough making sure 'lo' is brought up first prevents that
message. As to the NFS, no idea, I've never messed w/ nfsroot.

-d

Attachments: