Messages in this thread | | | Date | Sun, 12 Feb 2006 18:24:49 +0100 | From | Pierre Ossman <> | Subject | Re: [PATCH 2/2] [MMC] Secure Digital Host Controller Interface driver |
| |
Sergey Vlasov wrote:
> The interrupt handler can be called immediately after request_irq() > completes (even if you are sure that the device itself cannot generate > interrupts at this point, the interrupt line can be shared). And > host->lock is not yet initialized - oops... >
Ah. I'll fix that.
> > The same problem as with request_irq(), just from the other side - until > free_irq() returns, you may still get calls to your interrupt handler, > and host->ioaddr is already unmapped - oops again. >
Ditto.
>> + >> + mmc_free_host(mmc); >> +} >> + >> +static int __devinit sdhci_probe(struct pci_dev *pdev, >> + const struct pci_device_id *ent) >> +{ >> + int ret, i; >> + u8 slots; >> + struct sdhci_chip *chip; >> + >> + BUG_ON(pdev == NULL); >> + BUG_ON(ent == NULL); > > IMHO these BUG_ON() calls are overkill. >
I prefer BUG_ON():s since they print a line number. A page fault induced oops just gives me a byte offset into the compiled function.
> [...] >> +typedef struct sdhci_host *sdhci_host_p; > > The general policy seems to be "typedefs are evil"...
Fair enough. It didn't get much usage anyway.
Thanks for the code review. :)
Rgds Pierre - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |