lkml.org 
[lkml]   [2022]   [Feb]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/2] serial: mctrl_gpio: add a new API to enable / disable wake_irq
From
Date
Hi Andy,

On 2/4/22 10:07 AM, Andy Shevchenko wrote:
>
>
> On Thursday, February 3, 2022, Erwan Le Ray <erwan.leray@foss.st.com
> <mailto:erwan.leray@foss.st.com>> wrote:
>
> Add a new API to enable / disable wake_irq in order to enable gpio
> irqs as
> wakeup irqs for the uart port.
>
> Signed-off-by: Erwan Le Ray <erwan.leray@foss.st.com
> <mailto:erwan.leray@foss.st.com>>
>
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c
> b/drivers/tty/serial/serial_mctrl_gpio.c
> index c41d8911ce95..1663b3afc3a0 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.c
> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> @@ -299,4 +299,42 @@ void mctrl_gpio_disable_ms(struct mctrl_gpios
> *gpios)
>  }
>  EXPORT_SYMBOL_GPL(mctrl_gpio_disable_ms);
>
> +void mctrl_gpio_enable_irq_wake(struct mctrl_gpios *gpios)
> +{
> +       enum mctrl_gpio_idx i;
> +
> +       if (!gpios)
> +               return;
> +
> +       if (!gpios->mctrl_on)
> +               return;
> +
> +       for (i = 0; i < UART_GPIO_MAX; ++i) {
> +               if (!gpios->irq[i])
> +                       continue;
>
>
>
> Why not simply
>
>   if (gpios[])
>     enable_irq_...
>
> ?
>
> And same for disabling.
>
> +
> +               enable_irq_wake(gpios->irq[i]);
> +       }
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_enable_irq_wake);
> +
> +void mctrl_gpio_disable_irq_wake(struct mctrl_gpios *gpios)
> +{
> +       enum mctrl_gpio_idx i;
> +
> +       if (!gpios)
> +               return;
> +
> +       if (!gpios->mctrl_on)
> +               return;
> +
> +       for (i = 0; i < UART_GPIO_MAX; ++i) {
> +               if (!gpios->irq[i])
> +                       continue;
> +
> +               disable_irq_wake(gpios->irq[i]);
> +       }
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_disable_irq_wake);
> +
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.h
> b/drivers/tty/serial/serial_mctrl_gpio.h
> index b134a0ffc894..fc76910fb105 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.h
> +++ b/drivers/tty/serial/serial_mctrl_gpio.h
> @@ -91,6 +91,16 @@ void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios);
>   */
>  void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios);
>
> +/*
> + * Enable gpio wakeup interrupts to enable wake up source.
> + */
> +void mctrl_gpio_enable_irq_wake(struct mctrl_gpios *gpios);
> +
> +/*
> + * Disable gpio wakeup interrupts to enable wake up source.
> + */
> +void mctrl_gpio_disable_irq_wake(struct mctrl_gpios *gpios);
> +
>  #else /* GPIOLIB */
>
>  static inline
> @@ -142,6 +152,14 @@ static inline void mctrl_gpio_disable_ms(struct
> mctrl_gpios *gpios)
>  {
>  }
>
> +static inline void mctrl_gpio_enable_irq_wake(struct mctrl_gpios
> *gpios)
> +{
> +}
> +
> +static inline void mctrl_gpio_disable_irq_wake(struct mctrl_gpios
> *gpios)
> +{
> +}
> +
>  #endif /* GPIOLIB */
>
>  #endif
> --
> 2.17.1
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Thanks for your review.
I fully agree with your comment, but I wrote this code like it is to
keep the same structure than all the other ops of serial_mcrtrl_gpio
driver. I preferred keeping an homogeneous code in the driver rather
than breaking the driver homogeneity with the addition of an optimized code.

Greg, can you please indicate which solution you recommend ?

Cheers, Erwan.

\
 
 \ /
  Last update: 2022-02-04 16:43    [W:0.051 / U:0.452 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site