| Message ID | 1420900366-9169-1-git-send-email-j.uzycki@elproma.com.pl |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Jan 10, 2015 at 11:32 PM, Janusz Uzycki <j.uzycki@elproma.com.pl> wrote: > 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. I cannot provide a full review on this part of the kernel, but this series is also useful in that it will allow us to make gpiod_get_direction() private. This function should never have been public in the first place. The series, Acked-by: Alexandre Courbot <acourbot@nvidia.com> -- 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
On Sat, Jan 10, 2015 at 03:32:43PM +0100, Janusz Uzycki wrote: > 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() Note that this breaks the drivers that are already using mctrl_gpio_init. This is fixed up in the follow up patches, but still you break bisection. > 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. I suggest to add a comment for that. Something like: /* * XXX: This is not undone in the error path. To be fixed * properly this needs to be set atomically together with * request_irq. */ > Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl> There is no binding documentation for mctrl. This should be fixed, too. > --- > > 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 > -- > 1.7.11.3 > >
Hello Uwe. W dniu 2015-01-13 o 09:03, Uwe Kleine-König pisze: > On Sat, Jan 10, 2015 at 03:32:43PM +0100, Janusz Uzycki wrote: >> 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() > Note that this breaks the drivers that are already using > mctrl_gpio_init. This is fixed up in the follow up patches, but still > you break bisection. Could you give me more details about "break bisection" problem and how to correct it? I've decided to change the name in the same patch as parameter has been also changed. >> 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. What do you think about? mctrl_gpio_to_gpiod() could be removed. >> >> IRQ_NOAUTOEN setting and request_irq() order was not commented >> but it looks right according to other drivers. > I suggest to add a comment for that. Something like: > > /* > * XXX: This is not undone in the error path. To be fixed > * properly this needs to be set atomically together with > * request_irq. > */ OK. What does XXX mean? Since v1 I've seen more drivers set the flag but I didn't find any comment about. > >> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl> > There is no binding documentation for mctrl. This should be fixed, too. The bindings are added to atmel_serial and mxs-auart drivers because they use mctrl. Is it possible to improve bindings and how? > >> --- >> >> 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); It looks the functions should check if gpios is not NULL or drivers should fail probe on it. Which is the right way? >> 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 >> -- >> 1.7.11.3 >> >> -- 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
2015-01-10 15:32 GMT+01:00 Janusz Uzycki <j.uzycki@elproma.com.pl>: > 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 <j.uzycki@elproma.com.pl> > --- > > 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); > + 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 > #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 I'm going to do some tests on atmel_serial.c regards, Richard. -- 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
W dniu 2015-01-13 o 14:04, Richard Genoud pisze: > 2015-01-10 15:32 GMT+01:00 Janusz Uzycki <j.uzycki@elproma.com.pl>: >> 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 <j.uzycki@elproma.com.pl> >> --- >> >> 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); >> + > 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. -- 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
2015-01-13 14:52 GMT+01:00 Janusz Użycki <j.uzycki@elproma.com.pl>: > > W dniu 2015-01-13 o 14:04, Richard Genoud pisze: > >> 2015-01-10 15:32 GMT+01:00 Janusz Uzycki <j.uzycki@elproma.com.pl>: >>> 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); >>> + >> >> 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. > well, I'm also compiling it built-in (I even have config_module=N) kernel is 3.19-rc3 with your patches ontop. I attached my .config (saved as a defconfig) arm-linux-gcc (GCC) 4.7.3
W dniu 2015-01-13 o 15:30, Richard Genoud pisze: > 2015-01-13 14:52 GMT+01:00 Janusz Użycki <j.uzycki@elproma.com.pl>: >> W dniu 2015-01-13 o 14:04, Richard Genoud pisze: >> >>> 2015-01-10 15:32 GMT+01:00 Janusz Uzycki <j.uzycki@elproma.com.pl>: >>>> 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); >>>> + >>> 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. >> > well, I'm also compiling it built-in (I even have config_module=N) You have no CONFIG_GPIOLIB enabled and it is the bug in the patchset. Thanks. Janusz > kernel is 3.19-rc3 with your patches ontop. > I attached my .config (saved as a defconfig) > > arm-linux-gcc (GCC) 4.7.3 -- 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
2015-01-13 15:33 GMT+01:00 Janusz Użycki <j.uzycki@elproma.com.pl>: > > W dniu 2015-01-13 o 15:30, Richard Genoud pisze: > >> 2015-01-13 14:52 GMT+01:00 Janusz Użycki <j.uzycki@elproma.com.pl>: >>> >>> W dniu 2015-01-13 o 14:04, Richard Genoud pisze: >>> >>>> 2015-01-10 15:32 GMT+01:00 Janusz Uzycki <j.uzycki@elproma.com.pl>: >>>>> >>>>> 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); >>>>> + >>>> >>>> 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. >>> >> well, I'm also compiling it built-in (I even have config_module=N) > > > You have no CONFIG_GPIOLIB enabled and it is the bug in the patchset. > Thanks. > Janusz Actually, I have CONFIG_GPIOLIB=y, only it doesn't appears in the defconfig. I did: # cp defconfig arch/arm/configs/toto_defconfig # ARCH=arm make toto_defconfig # grep CONFIG_GPIOLIB .config CONFIG_GPIOLIB=y CONFIG_GPIOLIB_IRQCHIP=y # LANG=C ARCH=arm CROSS_COMPILE=~/dev/LNS/buildroot-VIP/output/host/usr/bin/arm-linux- make -j12 uImage [...] 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: -- 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
W dniu 2015-01-13 o 15:41, Richard Genoud pisze: > 2015-01-13 15:33 GMT+01:00 Janusz Użycki <j.uzycki@elproma.com.pl>: >> W dniu 2015-01-13 o 15:30, Richard Genoud pisze: >> >>> 2015-01-13 14:52 GMT+01:00 Janusz Użycki <j.uzycki@elproma.com.pl>: >>>> W dniu 2015-01-13 o 14:04, Richard Genoud pisze: >>>> >>>>> 2015-01-10 15:32 GMT+01:00 Janusz Uzycki <j.uzycki@elproma.com.pl>: >>>>>> 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); >>>>>> + >>>>> 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. >>>> >>> well, I'm also compiling it built-in (I even have config_module=N) >> >> You have no CONFIG_GPIOLIB enabled and it is the bug in the patchset. >> Thanks. >> Janusz > Actually, I have CONFIG_GPIOLIB=y, only it doesn't appears in the defconfig. > I did: > # cp defconfig arch/arm/configs/toto_defconfig > # ARCH=arm make toto_defconfig > > # grep CONFIG_GPIOLIB .config > CONFIG_GPIOLIB=y > CONFIG_GPIOLIB_IRQCHIP=y > > # LANG=C ARCH=arm > CROSS_COMPILE=~/dev/LNS/buildroot-VIP/output/host/usr/bin/arm-linux- > make -j12 uImage > [...] > 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: OK, I will fix the bug. It's interesting I compiled it using gcc version 4.2.4. thanks, Janusz -- 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
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 <j.uzycki@elproma.com.pl> > --- > > 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 -- 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
Hi Janusz, On Thu, Jan 22, 2015 at 8:33 AM, Janusz Użycki <j.uzycki@elproma.com.pl> wrote: > 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. When you send v3, please remove RFC from the subject. Thanks -- 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
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
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 <j.uzycki@elproma.com.pl> --- 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(-)