diff mbox

[1/3] serial: mctrl_gpio: restrict MCTRL initialization

Message ID 1501161456-13367-2-git-send-email-yegorslists@googlemail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Yegor Yefremov July 27, 2017, 1:17 p.m. UTC
From: Yegor Yefremov <yegorslists@googlemail.com>

GPIOs specified for serial port can be used either as GPIO driven MCTRL
or as a wakeup-source. Use the latter property to abort the MCTRL
initialization.

Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
---
 drivers/tty/serial/serial_mctrl_gpio.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Uwe Kleine-König July 27, 2017, 2 p.m. UTC | #1
Hello,

On Thu, Jul 27, 2017 at 03:17:34PM +0200, yegorslists@googlemail.com wrote:
> GPIOs specified for serial port can be used either as GPIO driven MCTRL
> or as a wakeup-source. Use the latter property to abort the MCTRL
> initialization.

Why is that an either-or and not an and?

Best regards
Uwe
Andy Shevchenko July 27, 2017, 2:01 p.m. UTC | #2
On Thu, 2017-07-27 at 15:17 +0200, yegorslists@googlemail.com wrote:
> From: Yegor Yefremov <yegorslists@googlemail.com>
> 
> GPIOs specified for serial port can be used either as GPIO driven
> MCTRL
> or as a wakeup-source. Use the latter property to abort the MCTRL
> initialization.

Please, elaborate on a choice of error code for this (-ENOSYS).

> 
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Suggested-by: I suppose.

I dunno about documentation part for this property, I leave it for DT
people to decide. Below part looks good to me.

> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> ---
>  drivers/tty/serial/serial_mctrl_gpio.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c
> b/drivers/tty/serial/serial_mctrl_gpio.c
> index d2da6aa..dce661c 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.c
> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> @@ -19,6 +19,7 @@
>  #include <linux/irq.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/termios.h>
> +#include <linux/property.h>
>  #include <linux/serial_core.h>
>  #include <linux/module.h>
>  
> @@ -118,6 +119,9 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct
> device *dev, unsigned int idx)
>  	struct mctrl_gpios *gpios;
>  	enum mctrl_gpio_idx i;
>  
> +	if (device_property_present(dev, "wakeup-source"))
> +		return ERR_PTR(-ENOSYS);
> +
>  	gpios = devm_kzalloc(dev, sizeof(*gpios), GFP_KERNEL);
>  	if (!gpios)
>  		return ERR_PTR(-ENOMEM);
Andy Shevchenko July 27, 2017, 2:55 p.m. UTC | #3
On Thu, 2017-07-27 at 16:00 +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Thu, Jul 27, 2017 at 03:17:34PM +0200, yegorslists@googlemail.com
> wrote:
> > GPIOs specified for serial port can be used either as GPIO driven
> > MCTRL
> > or as a wakeup-source. Use the latter property to abort the MCTRL
> > initialization.
> 
> Why is that an either-or and not an and?

That is how mctrl is designed right now.

Note marking the pin as a wakeup source inside mctrl driver will not
help since there is much bigger issue, i.e. pinctrl is not ready for a
such.
Yegor Yefremov July 28, 2017, 8:42 a.m. UTC | #4
On Thu, Jul 27, 2017 at 4:01 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Thu, 2017-07-27 at 15:17 +0200, yegorslists@googlemail.com wrote:
>> From: Yegor Yefremov <yegorslists@googlemail.com>
>>
>> GPIOs specified for serial port can be used either as GPIO driven
>> MCTRL
>> or as a wakeup-source. Use the latter property to abort the MCTRL
>> initialization.
>
> Please, elaborate on a choice of error code for this (-ENOSYS).

Something like this:

Return -ENOSYS as this would just skip mctrl initialization in the
case of wakeup source usage and not disable the port completely due to
failed serial8250_register_8250_port().

Yegor

>>
>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> Suggested-by: I suppose.
>
> I dunno about documentation part for this property, I leave it for DT
> people to decide. Below part looks good to me.
>
>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>> ---
>>  drivers/tty/serial/serial_mctrl_gpio.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c
>> b/drivers/tty/serial/serial_mctrl_gpio.c
>> index d2da6aa..dce661c 100644
>> --- a/drivers/tty/serial/serial_mctrl_gpio.c
>> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/irq.h>
>>  #include <linux/gpio/consumer.h>
>>  #include <linux/termios.h>
>> +#include <linux/property.h>
>>  #include <linux/serial_core.h>
>>  #include <linux/module.h>
>>
>> @@ -118,6 +119,9 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct
>> device *dev, unsigned int idx)
>>       struct mctrl_gpios *gpios;
>>       enum mctrl_gpio_idx i;
>>
>> +     if (device_property_present(dev, "wakeup-source"))
>> +             return ERR_PTR(-ENOSYS);
>> +
>>       gpios = devm_kzalloc(dev, sizeof(*gpios), GFP_KERNEL);
>>       if (!gpios)
>>               return ERR_PTR(-ENOMEM);
>
> --
> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index d2da6aa..dce661c 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -19,6 +19,7 @@ 
 #include <linux/irq.h>
 #include <linux/gpio/consumer.h>
 #include <linux/termios.h>
+#include <linux/property.h>
 #include <linux/serial_core.h>
 #include <linux/module.h>
 
@@ -118,6 +119,9 @@  struct mctrl_gpios *mctrl_gpio_init_noauto(struct device *dev, unsigned int idx)
 	struct mctrl_gpios *gpios;
 	enum mctrl_gpio_idx i;
 
+	if (device_property_present(dev, "wakeup-source"))
+		return ERR_PTR(-ENOSYS);
+
 	gpios = devm_kzalloc(dev, sizeof(*gpios), GFP_KERNEL);
 	if (!gpios)
 		return ERR_PTR(-ENOMEM);