Re: [PATCH] mfd: cottula: add cottula board

From: Robert Jarzmik
Date: Sat Dec 20 2014 - 04:44:11 EST


Robert Jarzmik <robert.jarzmik@xxxxxxx> writes:

> Arnd Bergmann <arnd@xxxxxxxx> writes:
>
>> On Monday 15 December 2014 00:10:06 Robert Jarzmik wrote:
>>> +
>>> + platform_set_drvdata(pdev, cot);
>>> + cot->gpio0 = gpiod_get(&pdev->dev, "lubbock_irq", 0);
>>> + if (IS_ERR(cot->gpio0)) {
>>> + dev_err(&pdev->dev, "Couldn't request GPIO : ret = %d\n", ret);
>>> + return PTR_ERR(cot->gpio0);
>>> + }
>>> + cot->irq = gpiod_to_irq(cot->gpio0);
>>> + if (cot->irq < 0)
>>> + return cot->irq;
>>> +
>>> + cot->irqdomain =
>>> + irq_domain_add_linear(pdev->dev.of_node, COTTULA_NB_IRQ,
>>> + &cottula_irq_domain_ops, cot);
>>> + if (!cot->irqdomain)
>>> + return -ENODEV;
>>> +
>>> + ret = 0;
>>> + if (base_irq)
>>> + ret = irq_create_strict_mappings(cot->irqdomain, base_irq, 0,
>>> + COTTULA_NB_IRQ);
>>>
>>
>> This looks a bit ambiguous: You get a GPIO line for the purpose of the
>> IRQ nesting but don't use the GPIO otherwise, and you pass the device's
>> own irq domain start as an IORESOURCE_IRQ resource.
>>
>> For consistency between DT and ATAGS based uses, and with similar DT
>> based drivers, I would instead recommend passing the parent irq (from
>> the GPIO) as an IORESOURCE_IRQ resource instead of a gpio lookup,
>> and passing the base_irq as platform_data for the ATAGS case.

Hi Arnd,

I thought again about the GPIO.

I put in the "gpiod_get()" call to ensure proper ordering between the gpio
probing and this driver probing. It ensured that this driver's probe will be
defered until the gpio driver is probed, which is the main purpose of this
patch (commit message).

If I pass an irq from the machine code, I loose this guarantee, don't I ?

Cheers.

--
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/