Subject: [RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines

From: [email-masked] (Janusz Użycki)
Date: Thu, 22 Jan 2015 11:33:22 +0100

Hi,

v3 is almost ready but on the moment I have no time to finish. I think 
I'll do it for 1-2 weeks.

best regards
Janusz

W dniu 2015-01-10 o 15:32, Janusz Uzycki pisze:
> A driver using mctrl_gpio called gpiod_get_direction() to check if
> gpio is input line. However .get_direction callback is not available
> for all platforms. The patch allows to avoid the function.
> The patch introduces the following helpers:
> - mctrl_gpio_request_irqs
> - mctrl_gpio_free_irqs
> - mctrl_gpio_enable_ms
> - mctrl_gpio_disable_ms
> They allow to:
> - simplify drivers which use mctrl_gpio
> - hide irq table in mctrl_gpio
> - use default irq handler for gpios
> - better separate code for gpio modem lines from uart's code
> In addition mctrl_gpio_init() has been renamed to mctrl_gpio_init_dt()
> to focus DT usage. Also mctrl_gpio_init_dt() initializes irq table for
> gpios now and passes struct uart_port into struct mctrl_gpios.
> This resulted in changed mctrl_gpio_init_dt() parameter.
> It also requires port->dev is set before the function is called.
>
> There were also fixed:
> - irq = 0 means invalid/unused (-EINVAL no more used)
> - mctrl_gpio_request_irqs() doesn't use negative enum value
>    if request_irq() failed. It just calls mctrl_gpio_free_irqs().
>
> The mctrl_gpio_is_gpio() inline function is under discussion
> and likely it can replace exported mctrl_gpio_to_gpiod() function.
>
> IRQ_NOAUTOEN setting and request_irq() order was not commented
> but it looks right according to other drivers.
>
> Signed-off-by: Janusz Uzycki <[email-masked]>
> ---
>
> The patch requires to update the drivers which use mctrl_gpio:
> - atmel_serial
> - mxs-auart
> - clps711x
>
> Changes since RFC v1:
>   - patch renamed from:
>     ("serial: mctrl-gpio: Add irqs helpers for input lines")
>     to:
>     ("tty: serial_mctrl_gpio: Add irqs helpers for input lines")
>   - mctrl_gpio_request_irqs: changed mctrl_gpio_free_irqs() and
>     dev_err() order to make debug easier
>   - added patches for atmel_serial and clps711x serial drivers
>
> The patch applies to next (3.19.0-rc2) and was tested with mxs-auart
> using kernel 3.14 and 3.18. It wasn't tested on the next (only compiled).
>
> The patchset delivers patches for mxs-auart, atmel_serial and clps711x.
> atmel_serial and clps711x were not tested - only compile tests were done.
>
> ---
>   drivers/tty/serial/serial_mctrl_gpio.c | 109 ++++++++++++++++++++++++++++++++-
>   drivers/tty/serial/serial_mctrl_gpio.h |  59 +++++++++++++++++-
>   2 files changed, 163 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
> index a38596c..215b15c 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.c
> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> @@ -19,11 +19,15 @@
>   #include <linux/device.h>
>   #include <linux/gpio/consumer.h>
>   #include <linux/termios.h>
> +#include <linux/irq.h>
>   
>   #include "serial_mctrl_gpio.h"
>   
>   struct mctrl_gpios {
> +	struct uart_port *port;
>   	struct gpio_desc *gpio[UART_GPIO_MAX];
> +	int irq[UART_GPIO_MAX];
> +	unsigned int mctrl_prev;
>   };
>   
>   static const struct {
> @@ -72,6 +76,12 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>   }
>   EXPORT_SYMBOL_GPL(mctrl_gpio_to_gpiod);
>   
> +inline
> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx gidx)
> +{
> +	return !IS_ERR_OR_NULL(gpios->gpio[gidx]);
> +}
> +
>   unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>   {
>   	enum mctrl_gpio_idx i;
> @@ -96,8 +106,9 @@ unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>   }
>   EXPORT_SYMBOL_GPL(mctrl_gpio_get);
>   
> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx)
>   {
> +	struct device *dev = port->dev;
>   	struct mctrl_gpios *gpios;
>   	enum mctrl_gpio_idx i;
>   	int err;
> @@ -106,6 +117,7 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>   	if (!gpios)
>   		return ERR_PTR(-ENOMEM);
>   
> +	gpios->port = port;
>   	for (i = 0; i < UART_GPIO_MAX; i++) {
>   		gpios->gpio[i] = devm_gpiod_get_index(dev,
>   						      mctrl_gpios_desc[i].name,
> @@ -128,11 +140,13 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>   			devm_gpiod_put(dev, gpios->gpio[i]);
>   			gpios->gpio[i] = NULL;
>   		}
> +		if (gpios->gpio[i] && !mctrl_gpios_desc[i].dir_out)
> +			gpios->irq[i] = gpiod_to_irq(gpios->gpio[i]);
>   	}
>   
>   	return gpios;
>   }
> -EXPORT_SYMBOL_GPL(mctrl_gpio_init);
> +EXPORT_SYMBOL_GPL(mctrl_gpio_init_dt);
>   
>   void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>   {
> @@ -147,3 +161,94 @@ void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>   	devm_kfree(dev, gpios);
>   }
>   EXPORT_SYMBOL_GPL(mctrl_gpio_free);
> +
> +/*
> + * Dealing with GPIO interrupt
> + */
> +#define MCTRL_ANY_DELTA        (TIOCM_RI | TIOCM_DSR | TIOCM_CD | TIOCM_CTS)
> +static irqreturn_t mctrl_gpio_irq_handle(int irq, void *context)
> +{
> +	struct mctrl_gpios *gpios = context;
> +	struct uart_port *port = gpios->port;
> +	u32 mctrl = gpios->mctrl_prev;
> +	u32 mctrl_diff;
> +
> +	mctrl_gpio_get(gpios, &mctrl);
> +
> +	mctrl_diff = mctrl ^ gpios->mctrl_prev;
> +	gpios->mctrl_prev = mctrl;
> +	if (mctrl_diff & MCTRL_ANY_DELTA && port->state != NULL) {
> +		if (mctrl_diff & TIOCM_RI)
> +			port->icount.rng++;
> +		if (mctrl_diff & TIOCM_DSR)
> +			port->icount.dsr++;
> +		if (mctrl_diff & TIOCM_CD)
> +			uart_handle_dcd_change(port, mctrl & TIOCM_CD);
> +		if (mctrl_diff & TIOCM_CTS)
> +			uart_handle_cts_change(port, mctrl & TIOCM_CTS);
> +
> +		wake_up_interruptible(&port->state->port.delta_msr_wait);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios)
> +{
> +	struct uart_port *port = gpios->port;
> +	int *irq = gpios->irq;
> +	enum mctrl_gpio_idx i;
> +	int err = 0;
> +
> +	for (i = 0; i < UART_GPIO_MAX; i++) {
> +		if (irq[i] <= 0)
> +			continue;
> +
> +		irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
> +		err = request_irq(irq[i], mctrl_gpio_irq_handle,
> +				  IRQ_TYPE_EDGE_BOTH,
> +				  dev_name(port->dev), gpios);
> +		if (err) {
> +			dev_err(port->dev, "%s: Can't get %d irq\n",
> +				__func__, irq[i]);
> +			mctrl_gpio_free_irqs(gpios);
> +			break;
> +		}
> +	}
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_request_irqs);
> +
> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios)
> +{
> +	enum mctrl_gpio_idx i;
> +
> +	for (i = 0; i < UART_GPIO_MAX; i++)
> +		if (gpios->irq[i] > 0)
> +			free_irq(gpios->irq[i], gpios);
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_free_irqs);
> +
> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
> +{
> +	enum mctrl_gpio_idx i;
> +
> +	/* get initial status of modem lines GPIOs */
> +	mctrl_gpio_get(gpios, &gpios->mctrl_prev);
> +
> +	for (i = 0; i < UART_GPIO_MAX; i++)
> +		if (gpios->irq[i] > 0)
> +			enable_irq(gpios->irq[i]);
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_enable_ms);
> +
> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
> +{
> +	enum mctrl_gpio_idx i;
> +
> +	for (i = 0; i < UART_GPIO_MAX; i++)
> +		if (gpios->irq[i] > 0)
> +			disable_irq(gpios->irq[i]);
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_disable_ms);
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
> index 400ba04..13ba3f4 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.h
> +++ b/drivers/tty/serial/serial_mctrl_gpio.h
> @@ -21,6 +21,7 @@
>   #include <linux/err.h>
>   #include <linux/device.h>
>   #include <linux/gpio/consumer.h>
> +#include <linux/serial_core.h>
>   
>   enum mctrl_gpio_idx {
>   	UART_GPIO_CTS,
> @@ -40,6 +41,13 @@ enum mctrl_gpio_idx {
>    */
>   struct mctrl_gpios;
>   
> +/*
> + * Return true if gidx is GPIO line, otherwise false.
> + * It assumes that gpios is allocated and not NULL.
> + */
> +inline
> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx gidx);
> +
>   #ifdef CONFIG_GPIOLIB
>   
>   /*
> @@ -60,12 +68,13 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>   				      enum mctrl_gpio_idx gidx);
>   
>   /*
> - * Request and set direction of modem control lines GPIOs.
> + * Request and set direction of modem control lines GPIOs. DT is used.
> + * Initialize irq table for GPIOs.
>    * devm_* functions are used, so there's no need to call mctrl_gpio_free().
>    * Returns a pointer to the allocated mctrl structure if ok, -ENOMEM on
>    * allocation error.
>    */
> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx);
> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx);
>   
>   /*
>    * Free the mctrl_gpios structure.
> @@ -74,6 +83,28 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx);
>    */
>   void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios);
>   
> +/*
> + * Request irqs for input lines GPIOs. The irqs are set disabled
> + * and triggered on both edges.
> + * Returns zero if OK, otherwise an error code.
> + */
> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios);
> +
> +/*
> + * Free irqs for input lines GPIOs.
> + */
> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios);
> +
> +/*
> + * Disable modem status interrupts assigned to GPIOs
> + */
> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios);
> +
> +/*
> + * Enable modem status interrupts assigned to GPIOs
> + */
> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios);
> +
>   #else /* GPIOLIB */
>   
>   static inline
> @@ -95,7 +126,7 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>   }
>   
>   static inline
> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx)
>   {
>   	return ERR_PTR(-ENOSYS);
>   }
> @@ -105,6 +136,28 @@ void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>   {
>   }
>   
> +static inline
> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios)
> +{
> +	/*return -ENOTSUP;*/
> +	return 0;
> +}
> +
> +static inline
> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios)
> +{
> +}
> +
> +static inline
> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
> +{
> +}
> +
> +static inline
> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
> +{
> +}
> +
>   #endif /* GPIOLIB */
>   
>   #endif