On Tue, Oct 07, 2014 at 10:03:00PM +0200, Christophe Ricard wrote: > +_irq_probe: > + /* IRQ */ > + r = irq_of_parse_and_map(pp, 0); > + if (r < 0) { > + pr_err("Unable to get irq, error: %d\n", r); > + interrupts = 0; > + goto _end; > + } > + interrupts = 1; > + client->irq = r; client->irq is already set correctly by of_i2c_register_devices - so I don't think this sequence is necessary? > + if (interrupts) { > + r = devm_gpio_request_one(&client->dev, pdata->io_serirq, > + GPIOF_IN, "TPM IO_SERIRQ"); Similarly, I wonder if pdata->io_serirq is just duplication of client->irq and that should be set by the creator instead? > +#ifdef CONFIG_OF > +static const struct of_device_id of_st33zp24_i2c_match[] = { > + { .compatible = "st,st33zp24_i2c", }, > + {} > +}; missing: MODULE_DEVICE_TABLE(of, of_st33zp24_i2c_match); > + #ifdef CONFIG_OF > + .of_match_table = of_match_ptr(of_st33zp24_i2c_match), > + #endif The #ifdef is unnecessary, the of_match_ptr macro takes care of that already. Jason -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html