Mark Rutland <mark.rutland@xxxxxxx> writes: > Hi Robert, > > Given the addition of an of_device_id table and some (implicit) property > parsing, this requires a device tree binding document. Yes, you're right. I'll prepare that. >> + ret = devm_request_irq(&pdev->dev, cot->irq, lubbock_irq_handler, 0, >> + dev_name(&pdev->dev), cot); >> + if (ret == -ENOSYS) >> + return -EPROBE_DEFER; >> + if (ret) { >> + dev_err(&pdev->dev, "Couldn't request GPIO : ret = %d\n", ret); >> + return ret; >> + } >> + irq_set_irq_type(cot->irq, IRQ_TYPE_EDGE_FALLING); > > Shouldn't that be in the interrupt-specifier when using DT? Maybe. But what about the platform data case ? I think I wanted to express the multiple-NOR gate binding all incoming level interrupt into the output cot->irq interrupt line going to GPIO0. Do you have an idea for the platform data case ? >> +static const struct of_device_id lubbock_id_table[] = { >> + { .compatible = "marvell,lubbock_io", }, > When PXA25x it was Intel, not Marvell. So I think the vendor prefix > should be "intel". > > Also s/_/-/ in property names and compatible strings please. Ok for v3. Thanks for the review. -- Robert -- 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