From owner-freebsd-mobile Wed Sep 24 07:15:39 1997 Return-Path: Received: (from root@localhost) by hub.freebsd.org (8.8.7/8.8.7) id HAA00647 for mobile-outgoing; Wed, 24 Sep 1997 07:15:39 -0700 (PDT) Received: from Octopussy.MI.Uni-Koeln.DE (Octopussy.MI.Uni-Koeln.DE [134.95.166.20]) by hub.freebsd.org (8.8.7/8.8.7) with SMTP id HAA00598; Wed, 24 Sep 1997 07:15:30 -0700 (PDT) Received: from x14.mi.uni-koeln.de ([134.95.219.124]) by Octopussy.MI.Uni-Koeln.DE with SMTP id AA16896 (5.67b/IDA-1.5); Wed, 24 Sep 1997 16:12:57 +0200 Received: (from se@localhost) by x14.mi.uni-koeln.de (8.8.7/8.6.9) id OAA01092; Wed, 24 Sep 1997 14:47:45 +0200 (CEST) X-Face: " Date: Wed, 24 Sep 1997 14:47:44 +0200 From: Stefan Esser To: Nate Williams Cc: mobile@FreeBSD.ORG, current@FreeBSD.ORG Subject: Re: PCCARD in -current broken References: <199709231929.NAA08312@rocky.mt.sri.com> <19970923230018.00034@mi.uni-koeln.de> <199709232246.QAA09189@rocky.mt.sri.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Mailer: Mutt 0.74 In-Reply-To: <199709232246.QAA09189@rocky.mt.sri.com>; from Nate Williams on Tue, Sep 23, 1997 at 04:46:56PM -0600 Sender: owner-freebsd-mobile@FreeBSD.ORG X-Loop: FreeBSD.org Precedence: bulk On Sep 23, Nate Williams wrote: > > There should not be a problem. ISA does not > > (should not, I didn't check the sources > > recently) register the handler as shared, > > and this will prevent another handler (both > > shared or exclusive) to be registered. > > register_intr() sets the INTR_EXCL flag just before it calls > intr_create() (so far so good), but in intr_connect(), the code to check > for that flag is ifdef's out: Well, I have implemented experimental resource registration and check code, but come to the conclusion, that it should be done quite differently ... > int > intr_connect(intrec *idesc) > { > .... > #ifdef RESOURCE_CHECK > resflag = (idesc->flags & INTR_EXCL) ? RESF_NONE : RESF_SHARED; > if (resource_claim(idesc->devdata, REST_INT, resflag, irq, irq) == 0) > #endif /* RESOURCE_CHECK */ > { > > So, we don't even check to see if INTR_EXCL is used. Well, you don't. I do, but I don't rely on that check :) > How does the new code realize that the interrupt is exclusive then? I have implemented a very simple set of functions to claim and check resource usage, and you see the resource_claim() call above. (The code does know resource types (e.g. REST_INT is interrupt), the minimum and maximum value (both passed as "irq" in the example), and a flag that currently only takes the value RESF_SHARED, if a resource may be shared with other uses, as long as all of them have RESF_SHARED set. > > Well, ISA interrupts should be registered in > > a way that guarantees they are not shared. > > How? But the code in -current does not use this function, and to make sure, that no other interrupt is added in combination with an exclusive interrupt, there is the test in add_intrdesc(), which I outlined in my previous reply. > So, can I rely on register_intr() return a negative # for failure? Well, if it fails for you, then there definitely is a bug in the check I added to add_intrdesc(), which is there to avoid just this situation. I assume you are sure that the PCCARD code is only called after the ISA devices are attached ? > (Here's the code in question.) > static u_int > build_freelist(u_int pcic_mask) > { > inthand2_t *nullfunc; > int irq; > u_int mask, freemask; > > /* No free IRQs (yet). */ > freemask = 0; > > /* Walk through all of the IRQ's and find any that aren't allocated. */ > for (irq = 0; irq < ICU_LEN; irq++) { > /* > * If the PCIC controller can't generate it, don't > * bother checking to see if it it's free. > */ > mask = 1 << irq; > if (!(mask & pcic_mask)) continue; > > /* See if the IRQ is free. */ > if (register_intr(irq, 0, 0, nullfunc, NULL, irq) == 0) { > /* Give it back, but add it to the mask */ > INTRMASK(freemask, mask); > unregister_intr(irq, nullfunc); > } > } > #ifdef PCIC_DEBUG > printf("Freelist of IRQ's <0x%x>\n", freemask); > #endif > return freemask; > } Looks fine to me. Lets see what happens after you call register_intr(): [ Much simplified extract from /sys/kern/kern_intr.c ! ] /** INTR_EXCL is added to flags and passed to intr_create(). **/ int register_intr(int intr, int device_id, u_int flags, inthand2_t handler, u_int *maskptr, int unit) { flags |= INTR_EXCL; idesc = intr_create((void *)device_id, intr, handler, (void*)unit, maskptr, flags); return (intr_connect(idesc)); } /** The flags parameter is put into the idesc structure. **/ intrec * intr_create(void *dev_instance, int irq, inthand2_t handler, void *arg, intrmask_t *maskptr, int flags) { idesc = malloc(sizeof *idesc, M_DEVBUF, M_WAITOK); if (idesc) { idesc->flags = flags; } return (idesc); } /** Return the error code from add_intrdesc (rest of code not shown). **/ int intr_connect(intrec *idesc) { errcode = add_intrdesc(idesc); return (errcode); } /** If this is the first handler for this irq, just register it. **/ /** If another handler has been registered before, then make sure, **/ /** that both the old and new handler are for a device that supports **/ /** shared interrupts (does not have INTR_EXCL set). Only the first **/ /** handler can have INTR_EXCL set, since no second one could have **/ /** been added ... **/ static int add_intrdesc(intrec *idesc) { if (head == NULL) { /* first handler for this irq, just install it */ if (icu_setup(irq, idesc->handler, idesc->argument, idesc->maskptr, idesc->flags) != 0) return (-1); } else { if ((idesc->flags & INTR_EXCL) != 0 || (head->flags & INTR_EXCL) != 0) { /* * can't append new handler, if either list head or * new handler do not allow interrupts to be shared */ printf("\tdevice combination doesn't support shared irq%d\n", irq); return (-1); } /* DO REAL WORK */ } return (0); } Well, in fact you should see the "device combination doesn't support ..." message printed, when you call register_intr() to check for the interrupt to be available. If you don't then there is a good chance, that you perform this test, before the ISA devices are attached, and there is no collision reported until the PCCARD attach tries to register a handler for an assumed free IRQ, which has been claimed by an ISA driver in between ... Regards, Stefan