Re: [PATCH] gpio: mxs: implement get_direction callback

["Janusz Użycki" <j.uzycki@elproma.com.pl>]

W dniu 2014-11-17 o 16:53, Uwe Kleine-König pisze:
> Hello,
>
> On Mon, Nov 17, 2014 at 11:05:51AM +0100, Richard Genoud wrote:
>> 2014-11-17 10:59 GMT+01:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
>>> Hello Richard,
>>>
>>>>>>>> So finally the prototypes would be:
>>>>>>>> int mctrl_gpio_request_irqs(struct mctrl_gpios*, struct
>>>>>>>> uart_port*, irqhandler_t);
>>>>>>>> void mctrl_gpio_free_irqs(struct mctrl_gpios*);
>>>>>> I think:
>>>>>>
>>>>>>          struct mctrl_gpios {
>>>>>>                  struct uart_port *port;
>>>>>>                  struct {
>>>>>>                          gpio_desc *gpio;
>>>>>>                          unsigned int irq;
>>>> I think it's just "int irq;" there
>>> irqs are unsigned. Some functions returning an irq use "int", but
>>> depending on who you ask this only for error reporting or a relict.
>>> Use 0 for invalid/unused in mctrl_gpio*.
>>>
>>>>> Yes. I tried to assign irq value in mctrl_gpio_init() only.
>>>>> There was another issue if CONFIG_GPIOLIB is not defined but it looks mctrl_
>>>>> disable/enable_ms()
>>>>> and mctrl_ irq handler solve the problem.
>>>>>
>>>>>>    Not sure there is a corresponding request_irq variant for that.
>>>>>
>>>>> What would you propose?
>>>> In atmel_request_gpio_irq(), the function irq_set_status_flags(irq,
>>>> IRQ_NOAUTOEN); is used before request_irq to prevent the irq from
>>>> being enabled when requested.
>>> I'm not sure this is allowed. How do you handle request_irq failing? (I
>>> just checked: you don't.) Consider another thread just doing
>>> request_irq($yourirq, ...) between
>>>
>>>          irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
>>>
>>> and
>>>
>>>          err = request_irq(irq[i], ...
>> well, in this case, request_irq() will fail and all the previously
>> requested irqs will be freed:
>>      /*
>>       * If something went wrong, rollback.
>>       */
>>      while (err && (--i >= 0))
>>          if (irq[i] >= 0)
>>              free_irq(irq[i], port);
> Just in case you didn't notice: Your statement is right, but for the
> other caller to request_irq there is something fishy. He gets
> IRQ_NOAUTOEN without being able to notice ...

Likely the gpio interrupts will never shared. We can say mctrl_gpio is 
the only owner
of a gpio after a request. So should we worry that IRQ_NOAUTOEN is hidden?

best regards
Janusz

>
> Best regards
> Uwe
>

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html