Re: 8250: Use mctrl_gpio helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2017-07-26 at 15:26 +0200, Yegor Yefremov wrote:
> On Tue, Jul 25, 2017 at 11:18 AM, Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > On Tue, 2017-07-25 at 07:43 +0200, Yegor Yefremov wrote:
> > > On Mon, Jul 24, 2017 at 8:48 PM, Yegor Yefremov
> > > <yegorslists@xxxxxxxxxxxxxx> wrote:
> > > > On Mon, Jul 24, 2017 at 8:39 PM, Andy Shevchenko
> > > > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > > > > Hi!
> > > > > 
> > > > > Since my big ACPI GPIO fix made the vanilla, I think we may
> > > > > return
> > > > > back
> > > > > the commit 4ef03d328769 ("tty/serial/8250: use mctrl_gpio
> > > > > helpers").
> > > > > 
> > > > > I just have tested it on two x86 boards:
> > > > > 1) Broxton (without _DSD properties)
> > > > > 2) ApolloLake (with _DSD properties for RX and CTS pins)
> > > > > 
> > > > > Opinions, more testing?
> > 
> > Alas, I did more deep testing and the patch breaks the console.
> > 
> > It looks like we need to distinguish what those GPIOs are used for:
> > a) modem control lines, or
> > b) wakeup source.
> > 
> > There are few options to distinguish:
> > 1) check if GPIO resource is marked as wakeup source (ACPI only)
> > 2) use "wakeup-source" device property, for now looks like there is
> > no
> > serial driver is using it (might be collision with the real wake
> > capable
> > serial drivers)
> > 3) similar to 2), though introduce another property like "oob-
> > wakeup-
> > source" or any variations of that
> > 4) something else?
> > 

> AFAIK this patch was needed in order to avoid serial port breakage,
> where some modem signals were defined in ACPI tables. Mika and some
> other devs reported the issue as "tty/serial/8250: use mctrl_gpio
> helpers" was part of the kernel.

I am one from those devs.


> Perhaps I should just augment my patch like this?
> 

> +               sprintf(mctrl_property, "%s-gpios",
> mctrl_gpios_desc[i].name);
> +               if (!device_property_present(dev, mctrl_property)

This is not needed any more. My patch series for GPIO ACPI library makes
this clean (no fallback is allowed if name is supplied).

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.

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).

-- 
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux