Le 30/09/2015 10:19, Uwe Kleine-König a écrit :
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
Tested-by: Nicolas Ferre <nicolas.ferre@xxxxxxxxx>
But still a little remarks below...
> ---
> drivers/tty/serial/atmel_serial.c | 140 +++++---------------------------------
> 1 file changed, 18 insertions(+), 122 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 7502af04f21b..ce2c9327dcad 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -149,7 +149,6 @@ struct atmel_uart_port {
> struct circ_buf rx_ring;
>
> struct mctrl_gpios *gpios;
> - int gpio_irq[UART_GPIO_MAX];
> unsigned int tx_done_mask;
> u32 fifo_size;
> u32 rts_high;
> @@ -544,27 +543,21 @@ static void atmel_enable_ms(struct uart_port *port)
>
> atmel_port->ms_irq_enabled = true;
So, what about removing this boolean as well? I'd prefer to have this
security only managed by mctrl_on in the mctrl_gpio_enable_ms() function...
> - if (atmel_port->gpio_irq[UART_GPIO_CTS] >= 0)
> - enable_irq(atmel_port->gpio_irq[UART_GPIO_CTS]);
> - else
> + if (!mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_CTS))
> ier |= ATMEL_US_CTSIC;
>
> - if (atmel_port->gpio_irq[UART_GPIO_DSR] >= 0)
> - enable_irq(atmel_port->gpio_irq[UART_GPIO_DSR]);
> - else
> + if (!mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_DSR))
> ier |= ATMEL_US_DSRIC;
>
> - if (atmel_port->gpio_irq[UART_GPIO_RI] >= 0)
> - enable_irq(atmel_port->gpio_irq[UART_GPIO_RI]);
> - else
> + if (!mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_RI))
> ier |= ATMEL_US_RIIC;
>
> - if (atmel_port->gpio_irq[UART_GPIO_DCD] >= 0)
> - enable_irq(atmel_port->gpio_irq[UART_GPIO_DCD]);
> - else
> + if (!mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_DCD))
> ier |= ATMEL_US_DCDIC;
>
> atmel_uart_writel(port, ATMEL_US_IER, ier);
> +
> + mctrl_gpio_enable_ms(atmel_port->gpios);
> }
>
> /*
> @@ -583,24 +576,18 @@ static void atmel_disable_ms(struct uart_port *port)
>
> atmel_port->ms_irq_enabled = false;
Ditto.
>
> - if (atmel_port->gpio_irq[UART_GPIO_CTS] >= 0)
> - disable_irq(atmel_port->gpio_irq[UART_GPIO_CTS]);
> - else
> + mctrl_gpio_disable_ms(atmel_port->gpios);
> +
> + if (!mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_CTS))
> idr |= ATMEL_US_CTSIC;
>
> - if (atmel_port->gpio_irq[UART_GPIO_DSR] >= 0)
> - disable_irq(atmel_port->gpio_irq[UART_GPIO_DSR]);
> - else
> + if (!mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_DSR))
> idr |= ATMEL_US_DSRIC;
>
> - if (atmel_port->gpio_irq[UART_GPIO_RI] >= 0)
> - disable_irq(atmel_port->gpio_irq[UART_GPIO_RI]);
> - else
> + if (!mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_RI))
> idr |= ATMEL_US_RIIC;
>
> - if (atmel_port->gpio_irq[UART_GPIO_DCD] >= 0)
> - disable_irq(atmel_port->gpio_irq[UART_GPIO_DCD]);
> - else
> + if (!mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_DCD))
> idr |= ATMEL_US_DCDIC;
>
> atmel_uart_writel(port, ATMEL_US_IDR, idr);
> @@ -1258,7 +1245,6 @@ static irqreturn_t atmel_interrupt(int irq, void *dev_id)
> struct uart_port *port = dev_id;
> struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> unsigned int status, pending, mask, pass_counter = 0;
> - bool gpio_handled = false;
>
> spin_lock(&atmel_port->lock_suspended);
>
> @@ -1266,24 +1252,6 @@ static irqreturn_t atmel_interrupt(int irq, void *dev_id)
> status = atmel_get_lines_status(port);
> mask = atmel_uart_readl(port, ATMEL_US_IMR);
> pending = status & mask;
> - if (!gpio_handled) {
> - /*
> - * Dealing with GPIO interrupt
> - */
> - if (irq == atmel_port->gpio_irq[UART_GPIO_CTS])
> - pending |= ATMEL_US_CTSIC;
> -
> - if (irq == atmel_port->gpio_irq[UART_GPIO_DSR])
> - pending |= ATMEL_US_DSRIC;
> -
> - if (irq == atmel_port->gpio_irq[UART_GPIO_RI])
> - pending |= ATMEL_US_RIIC;
> -
> - if (irq == atmel_port->gpio_irq[UART_GPIO_DCD])
> - pending |= ATMEL_US_DCDIC;
> -
> - gpio_handled = true;
> - }
> if (!pending)
> break;
>
> @@ -1773,45 +1741,6 @@ static void atmel_get_ip_name(struct uart_port *port)
> }
> }
>
> -static void atmel_free_gpio_irq(struct uart_port *port)
> -{
> - struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> - enum mctrl_gpio_idx i;
> -
> - for (i = 0; i < UART_GPIO_MAX; i++)
> - if (atmel_port->gpio_irq[i] >= 0)
> - free_irq(atmel_port->gpio_irq[i], port);
> -}
> -
> -static int atmel_request_gpio_irq(struct uart_port *port)
> -{
> - struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> - int *irq = atmel_port->gpio_irq;
> - enum mctrl_gpio_idx i;
> - int err = 0;
> -
> - for (i = 0; (i < UART_GPIO_MAX) && !err; i++) {
> - if (irq[i] < 0)
> - continue;
> -
> - irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
> - err = request_irq(irq[i], atmel_interrupt, IRQ_TYPE_EDGE_BOTH,
> - "atmel_serial", port);
> - if (err)
> - dev_err(port->dev, "atmel_startup - Can't get %d irq\n",
> - irq[i]);
> - }
> -
> - /*
> - * If something went wrong, rollback.
> - */
> - while (err && (--i >= 0))
> - if (irq[i] >= 0)
> - free_irq(irq[i], port);
> -
> - return err;
> -}
> -
Cool to not have to deal with this anymore...
> /*
> * Perform initialization and enable port for reception
> */
> @@ -1841,13 +1770,6 @@ static int atmel_startup(struct uart_port *port)
> return retval;
> }
>
> - /*
> - * Get the GPIO lines IRQ
> - */
> - retval = atmel_request_gpio_irq(port);
> - if (retval)
> - goto free_irq;
> -
> tasklet_enable(&atmel_port->tasklet);
>
> /*
> @@ -1943,11 +1865,6 @@ static int atmel_startup(struct uart_port *port)
> }
>
> return 0;
> -
> -free_irq:
> - free_irq(port->irq, port);
> -
> - return retval;
> }
>
> /*
> @@ -2013,7 +1930,6 @@ static void atmel_shutdown(struct uart_port *port)
> * Free the interrupts
> */
> free_irq(port->irq, port);
> - atmel_free_gpio_irq(port);
>
> atmel_port->ms_irq_enabled = false;
>
> @@ -2681,26 +2597,6 @@ static int atmel_serial_resume(struct platform_device *pdev)
> #define atmel_serial_resume NULL
> #endif
>
> -static int atmel_init_gpios(struct atmel_uart_port *p, struct device *dev)
> -{
> - enum mctrl_gpio_idx i;
> - struct gpio_desc *gpiod;
> -
> - p->gpios = mctrl_gpio_init_noauto(dev, 0);
> - if (IS_ERR(p->gpios))
> - return PTR_ERR(p->gpios);
> -
> - for (i = 0; i < UART_GPIO_MAX; i++) {
> - gpiod = mctrl_gpio_to_gpiod(p->gpios, i);
> - if (gpiod && (gpiod_get_direction(gpiod) == GPIOF_DIR_IN))
> - p->gpio_irq[i] = gpiod_to_irq(gpiod);
> - else
> - p->gpio_irq[i] = -EINVAL;
> - }
> -
> - return 0;
> -}
> -
> static void atmel_serial_probe_fifos(struct atmel_uart_port *port,
> struct platform_device *pdev)
> {
> @@ -2783,16 +2679,16 @@ static int atmel_serial_probe(struct platform_device *pdev)
>
> spin_lock_init(&port->lock_suspended);
>
> - ret = atmel_init_gpios(port, &pdev->dev);
> - if (ret < 0) {
> - dev_err(&pdev->dev, "Failed to initialize GPIOs.");
> - goto err_clear_bit;
> - }
> -
> ret = atmel_init_port(port, pdev);
> if (ret)
> goto err_clear_bit;
>
> + port->gpios = mctrl_gpio_init(&port->uart, 0);
> + if (IS_ERR(port->gpios)) {
> + ret = PTR_ERR(port->gpios);
> + goto err_clear_bit;
> + }
> +
> if (!atmel_use_pdc_rx(&port->uart)) {
> ret = -ENOMEM;
> data = kmalloc(sizeof(struct atmel_uart_char)
>
--
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
