On Wednesday 06 February 2008 01:32, Andrew Morton wrote: > > Looks OK. Minorish things from a quick scan: > > > > tosakbd_scankeyboard() looks like it'll spend a perfectly wicked amount of > time under spin_lock_irqsave(). >
I think you are right. I will check with Dmitry if it can be relaxed a bit. > > > This code, in tosakbd_probe(): > > +fail2: > + while (--i >= 0) > + gpio_free(TOSA_GPIO_KEY_STROBE(i)); > + > + i = TOSA_KEY_SENSE_NUM; > +fail: > + while (--i >= 0) { > + free_irq(gpio_to_irq(TOSA_GPIO_KEY_SENSE(i)), pdev); > + gpio_free(TOSA_GPIO_KEY_SENSE(i)); > + } > > looks like it'll free irqs and gpios which were never allocated (if i < > TOSA_KEY_SENSE_NUM on entry). > Umm? There are 2 groups of gpios (sense and strobe) with sense group registered first. Looks ok to me. > > > +static int __devinit tosakbd_probe(struct platform_device *pdev) { > > please integrate checkpatch into your merging process. > Will do. > > > > i8042_platform_init(): > > +#if defined(__i386__) || defined(__x86_64__) > > use #ifdef CONFIG_X86? > I considered it but above was tested and in line with the style of the rest of the file... -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/