| Message ID | CAGm1_kvfs6HGLxYxg=MAoeZ3qq-mzrS_tan+Q67z7K7TWGRjPw@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2017-07-26 at 15:55 +0200, Yegor Yefremov wrote: > On Wed, Jul 26, 2017 at 3:35 PM, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Wed, 2017-07-26 at 15:26 +0200, Yegor Yefremov wrote: > > The problen now while property _is_ there in ACPI table. > > > > > || > > > + of_property_read_bool(dev- > > > >of_node, > > > "wakeup-source")) > > > > ...and this should be device_property besides that fact that you > > need to > > check it once in _noauto() and forbid requesting GPIOs by returning > > an > > error immediately. > > Something like that: Precisely. > @@ -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(-EINVAL); ...and this patch should be in _before_ the one which enables mctrl for 8250. > > P.S. Take into account that use of this property should be described > > in > > the bindings. Before that it should be discussed (I pointed above > > what > > might be cons of use this name/property). > > OK. Let's wait for suggestions. > I would recommend to prepare and send a formal patch with Cc list of all stakeholders (some developers from serial, DT, ARM, x86). In a commit message would be good to explain we are trying to distinguish the purpose of GPIOs in UART description (DT or ACPI or even platform code [via built-in device properties]).
On Wed, Jul 26, 2017 at 4:58 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Wed, 2017-07-26 at 15:55 +0200, Yegor Yefremov wrote: >> On Wed, Jul 26, 2017 at 3:35 PM, Andy Shevchenko >> <andriy.shevchenko@linux.intel.com> wrote: >> > On Wed, 2017-07-26 at 15:26 +0200, Yegor Yefremov wrote: > >> > The problen now while property _is_ there in ACPI table. >> > >> > > || >> > > + of_property_read_bool(dev- >> > > >of_node, >> > > "wakeup-source")) >> > >> > ...and this should be device_property besides that fact that you >> > need to >> > check it once in _noauto() and forbid requesting GPIOs by returning >> > an >> > error immediately. >> >> Something like that: > > Precisely. > >> @@ -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(-EINVAL); > > ...and this patch should be in _before_ the one which enables mctrl for > 8250. ACK >> > P.S. Take into account that use of this property should be described >> > in >> > the bindings. Before that it should be discussed (I pointed above >> > what >> > might be cons of use this name/property). >> >> OK. Let's wait for suggestions. >> > > I would recommend to prepare and send a formal patch with Cc list of all > stakeholders (some developers from serial, DT, ARM, x86). > > In a commit message would be good to explain we are trying to > distinguish the purpose of GPIOs in UART description (DT or ACPI or even > platform code [via built-in device properties]). I think we should return -ENOSYS otherwise the serial port won't be registered at all, when -EINVAL is returned. I'll prepare the patch series. Yegor -- 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 --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c index d2da6aa..8871213 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(-EINVAL); + gpios = devm_kzalloc(dev, sizeof(*gpios), GFP_KERNEL); if (!gpios)