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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to