linux-kernel - Re: [PATCH] mfd: cottula: add cottula board

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 20 Dec 2014 10:43:48 +0100
From:	Robert Jarzmik <robert.jarzmik@...e.fr>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	Samuel Ortiz <sameo@...ux.intel.com>,
	Lee Jones <lee.jones@...aro.org>,
	Grant Likely <grant.likely@...aro.org>,
	Rob Herring <robh+dt@...nel.org>, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: Re: [PATCH] mfd: cottula: add cottula board

Robert Jarzmik <robert.jarzmik@...e.fr> writes:

> Arnd Bergmann <arnd@...db.de> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/