From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?SmFudXN6IFXFvHlja2k=?= Subject: Re: [RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines Date: Tue, 13 Jan 2015 14:52:04 +0100 Message-ID: <54B52304.1030008@elproma.com.pl> References: <1420900366-9169-1-git-send-email-j.uzycki@elproma.com.pl> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from v032797.home.net.pl ([89.161.177.31]:51334 "HELO v032797.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751448AbbAMNwI (ORCPT ); Tue, 13 Jan 2015 08:52:08 -0500 In-Reply-To: Sender: linux-gpio-owner@vger.kernel.org List-Id: linux-gpio@vger.kernel.org To: Richard Genoud Cc: Greg Kroah-Hartman , Linus Walleij , =?UTF-8?B?VXdlIEtsZWluZS1Lw7ZuaWc=?= , Alexander Shiyan , fabio.estevam@freescale.com, Fabio Estevam , "linux-serial@vger.kernel.org" , "linux-gpio@vger.kernel.org" , Alexandre Courbot , "linux-arm-kernel@lists.infradead.org" W dniu 2015-01-13 o 14:04, Richard Genoud pisze: > 2015-01-10 15:32 GMT+01:00 Janusz Uzycki : >> 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 >> --- >> >> 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 >> #include >> #include >> +#include >> >> #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 >> #include >> #include >> +#include >> >> 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); >> + > This leads to a compile error : > CC drivers/tty/serial/atmel_serial.o > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/atmel_serial.c: In function 'atmel_disable_ms.part.22': > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:536:25: error: called from here > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:539:25: error: called from here > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:542:25: error: called from here > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:545:25: error: called from here > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/atmel_serial.c: In function 'atmel_enable_ms': > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:503:25: error: called from here > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:506:25: error: called from here > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:509:25: error: called from here > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:512:25: error: called from here > make[3]: *** [drivers/tty/serial/atmel_serial.o] Error 1 > make[2]: *** [drivers/tty/serial] Error 2 > make[1]: *** [drivers/tty] Error 2 > make: *** [drivers] Error 2 Do you compile atmel_serial as module? I compiled built-in and it was fine for me. So the function should be exported like mctrl_gpio_to_gpiod() I guess. An other reason can be you have CONFIG_GPIOLIB=n ? In fact, mctrl_gpio_is_gpio() should depend on CONFIG_GPIOLIB for empty body. > > >> #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 >> -- >> 1.7.11.3 >> > I think you should also update the documentation on mctrl_ helpers : > Documentation/serial/driver Right! > > I'm going to do some tests on atmel_serial.c Thanks, Janusz > > regards, > Richard. From mboxrd@z Thu Jan 1 00:00:00 1970 From: j.uzycki@elproma.com.pl (=?UTF-8?B?SmFudXN6IFXFvHlja2k=?=) Date: Tue, 13 Jan 2015 14:52:04 +0100 Subject: [RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines In-Reply-To: References: <1420900366-9169-1-git-send-email-j.uzycki@elproma.com.pl> Message-ID: <54B52304.1030008@elproma.com.pl> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org W dniu 2015-01-13 o 14:04, Richard Genoud pisze: > 2015-01-10 15:32 GMT+01:00 Janusz Uzycki : >> 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 >> --- >> >> 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 >> #include >> #include >> +#include >> >> #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 >> #include >> #include >> +#include >> >> 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); >> + > This leads to a compile error : > CC drivers/tty/serial/atmel_serial.o > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/atmel_serial.c: In function 'atmel_disable_ms.part.22': > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:536:25: error: called from here > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:539:25: error: called from here > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:542:25: error: called from here > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:545:25: error: called from here > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/atmel_serial.c: In function 'atmel_enable_ms': > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:503:25: error: called from here > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:506:25: error: called from here > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:509:25: error: called from here > In file included from drivers/tty/serial/atmel_serial.c:64:0: > drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in > call to always_inline 'mctrl_gpio_is_gpio': function body not > available > drivers/tty/serial/atmel_serial.c:512:25: error: called from here > make[3]: *** [drivers/tty/serial/atmel_serial.o] Error 1 > make[2]: *** [drivers/tty/serial] Error 2 > make[1]: *** [drivers/tty] Error 2 > make: *** [drivers] Error 2 Do you compile atmel_serial as module? I compiled built-in and it was fine for me. So the function should be exported like mctrl_gpio_to_gpiod() I guess. An other reason can be you have CONFIG_GPIOLIB=n ? In fact, mctrl_gpio_is_gpio() should depend on CONFIG_GPIOLIB for empty body. > > >> #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 >> -- >> 1.7.11.3 >> > I think you should also update the documentation on mctrl_ helpers : > Documentation/serial/driver Right! > > I'm going to do some tests on atmel_serial.c Thanks, Janusz > > regards, > Richard.