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

[Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxx>:
> > 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 ...

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html