linux-kernel - Re: [PATCH] gpio: pcf857x: cleanup irq_demux

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 for Android: free password hash cracker in your pocket

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

Date:	Thu, 29 Aug 2013 19:24:11 +0200
From:	Linus Walleij <linus.walleij@...aro.org>
To:	George Cherian <george.cherian@...com>
Cc:	Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
	Nikolay Balandin <n.a.balandin@...il.com>,
	Grant Likely <grant.likely@...aro.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Linux-OMAP <linux-omap@...r.kernel.org>
Subject: Re: [PATCH] gpio: pcf857x: cleanup irq_demux_work and use threaded irq

On Thu, Aug 29, 2013 at 4:11 PM, George Cherian <george.cherian@...com> wrote:
> On 8/29/2013 6:27 PM, Linus Walleij wrote:

>>>          int                     irq;            /* real irq number */
>>> +       int                     irq_mapped;     /* mapped gpio irqs  */
>>
>> This seems like an u32 or atleast unsigned, and state that its one
>> bit flag per IRQ. How many GPIO lines are there?
>
> pcf857x driver supports expanders with 8 and 16 gpio lines.

OK should be an u16 then?

>>>   static int pcf857x_irq_domain_map(struct irq_domain *domain, unsigned
>>> int virq,
>>>                                   irq_hw_number_t hw)
>>>   {
>>> +       struct pcf857x  *gpio = domain->host_data;
>>>          irq_set_chip_and_handler(virq,
>>>                                   &dummy_irq_chip,
>>>                                   handle_level_irq);
>>> +       set_irq_flags(virq, IRQF_VALID);
>>> +       gpio->irq_mapped |= (1 << hw);
>>
>> I'm a bit uneasy about this. It feels like its the irqdomain's
>> responsibility to keep track of whether an IRQ is mapped
>> or not.
>
> Mainly these expanders dont have an ier sort of registers and if at all the
> initial value is not set proper
> then it gives bad irq prints only once per changed bit for which there is no
> handler.
> Should I drop this in v2?

I don't know, I mean if it's causing a problem or filling the console
with error prints it is really annoying, and if there is nothing to do
about the condition then there is not much more we can do I guess.

But please take a second look.

Yours,
Linus Walleij
--
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/