* [PATCH 3/3 v8] tty/serial/8250: use mctrl_gpio helpers
From: Stefan Roese @ 2019-06-18 8:32 UTC (permalink / raw)
To: linux-serial
Cc: linux-kernel, Yegor Yefremov, Andy Shevchenko, Mika Westerberg,
Giulio Benetti, Greg Kroah-Hartman
In-Reply-To: <20190618083200.30234-1-sr@denx.de>
From: Yegor Yefremov <yegorslists@googlemail.com>
This patch permits the usage for GPIOs to control
the CTS/RTS/DTR/DSR/DCD/RI signals.
Changed by Stefan:
Only call mctrl_gpio_init(), if the device has no ACPI companion device
to not break existing ACPI based systems. Also only use the mctrl_gpio_
functions when "gpios" is available.
Use MSR / MCR <-> TIOCM wrapper functions.
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
Signed-off-by: Stefan Roese <sr@denx.de>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Tested-by: Yegor Yefremov <yegorslists@googlemail.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Giulio Benetti <giulio.benetti@micronovasrl.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v8:
- Rebased on top of "tty-next"
v7:
- Change serial8250_do_get_mctrl() so that systems with a "mixed setup"
(i.e. CTS controlled by UART but other status pins controlled by GPIO)
are also supported again (Yegor)
v6:
- Use newly introduced TIOCM <-> MCR/MSR wrapper functions
- serial8250_in_MCR(): Don't save the already read MCR bits in TIOCM
format but "or" them later to the GPIO MCR value
- Correctly use "!up->gpios" (Andy)
- Removed Mika's reviewed by tag (because of changes)
v5:
- Dropped a few "if (up->gpios)" checks, as the mctrl_gpio_foo() API
handles gpios == NULL (return)
- 8250_omap: Changed "IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(up->gpios, ...))"
to "up->gpios == NULL", as mctrl_gpio_to_gpiod() does not handle
gpios == NULL correctly.
v4:
- Added Mika's reviewed by tag
- Added Johan to Cc
v3:
- Only call mctrl_gpio_init(), if the device has no ACPI companion device
to not break existing ACPI based systems, as suggested by Andy
v2:
- No change
Please note that this patch was already applied before [1]. And later
reverted [2] because it introduced problems on some x86 based boards
(ACPI GPIO related). Here a detailed description of the issue at that
time:
https://lkml.org/lkml/2016/8/9/357
http://www.spinics.net/lists/linux-serial/msg23071.html
This is a re-send of the original patch that was applied at that time.
With patch 1/2 from this series this issue should be fixed now (please
note that I can't test it on such an x86 platform causing these
problems).
Andy (or Mika), perhaps it would be possible for you to test this
patch again, now with patch 1/2 of this series applied as well?
That would be really helpful.
Thanks,
Stefan
[1] 4ef03d328769 ("tty/serial/8250: use mctrl_gpio helpers")
[2] 5db4f7f80d16 ("Revert "tty/serial/8250: use mctrl_gpio helpers"")
.../devicetree/bindings/serial/8250.txt | 19 ++++++++++++
drivers/tty/serial/8250/8250.h | 18 +++++++++++-
drivers/tty/serial/8250/8250_core.c | 17 +++++++++++
drivers/tty/serial/8250/8250_omap.c | 29 ++++++++++---------
drivers/tty/serial/8250/8250_port.c | 11 ++++++-
drivers/tty/serial/8250/Kconfig | 1 +
include/linux/serial_8250.h | 1 +
7 files changed, 81 insertions(+), 15 deletions(-)
diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
index 3cba12f855b7..20d351f268ef 100644
--- a/Documentation/devicetree/bindings/serial/8250.txt
+++ b/Documentation/devicetree/bindings/serial/8250.txt
@@ -53,6 +53,9 @@ Optional properties:
programmable TX FIFO thresholds.
- resets : phandle + reset specifier pairs
- overrun-throttle-ms : how long to pause uart rx when input overrun is encountered.
+- {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD
+ line respectively. It will use specified GPIO instead of the peripheral
+ function pin for the UART feature. If unsure, don't specify this property.
Note:
* fsl,ns16550:
@@ -74,3 +77,19 @@ Example:
interrupts = <10>;
reg-shift = <2>;
};
+
+Example for OMAP UART using GPIO-based modem control signals:
+
+ uart4: serial@49042000 {
+ compatible = "ti,omap3-uart";
+ reg = <0x49042000 0x400>;
+ interrupts = <80>;
+ ti,hwmods = "uart4";
+ clock-frequency = <48000000>;
+ cts-gpios = <&gpio3 5 GPIO_ACTIVE_LOW>;
+ rts-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
+ dtr-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
+ dsr-gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
+ dcd-gpios = <&gpio1 14 GPIO_ACTIVE_LOW>;
+ rng-gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
+ };
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 793da2e510e0..75c7c5449461 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -11,6 +11,8 @@
#include <linux/serial_reg.h>
#include <linux/dmaengine.h>
+#include "../serial_mctrl_gpio.h"
+
struct uart_8250_dma {
int (*tx_dma)(struct uart_8250_port *p);
int (*rx_dma)(struct uart_8250_port *p);
@@ -196,11 +198,25 @@ static inline int serial8250_MSR_to_TIOCM(int msr)
static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
{
serial_out(up, UART_MCR, value);
+
+ if (up->gpios)
+ mctrl_gpio_set(up->gpios, serial8250_MCR_to_TIOCM(value));
}
static inline int serial8250_in_MCR(struct uart_8250_port *up)
{
- return serial_in(up, UART_MCR);
+ int mctrl;
+
+ mctrl = serial_in(up, UART_MCR);
+
+ if (up->gpios) {
+ unsigned int mctrl_gpio = 0;
+
+ mctrl_gpio = mctrl_gpio_get_outputs(up->gpios, &mctrl_gpio);
+ mctrl |= serial8250_TIOCM_to_MCR(mctrl_gpio);
+ }
+
+ return mctrl;
}
#if defined(__alpha__) && !defined(CONFIG_PCI)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index e441221e04b9..a4470771005f 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -14,6 +14,7 @@
* serial8250_register_8250_port() ports
*/
+#include <linux/acpi.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/ioport.h>
@@ -982,6 +983,8 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
uart = serial8250_find_match_or_unused(&up->port);
if (uart && uart->port.type != PORT_8250_CIR) {
+ struct mctrl_gpios *gpios;
+
if (uart->port.dev)
uart_remove_one_port(&serial8250_reg, &uart->port);
@@ -1016,6 +1019,20 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
if (up->port.flags & UPF_FIXED_TYPE)
uart->port.type = up->port.type;
+ /*
+ * Only call mctrl_gpio_init(), if the device has no ACPI
+ * companion device
+ */
+ if (!has_acpi_companion(uart->port.dev)) {
+ gpios = mctrl_gpio_init(&uart->port, 0);
+ if (IS_ERR(gpios)) {
+ if (PTR_ERR(gpios) != -ENOSYS)
+ return PTR_ERR(gpios);
+ } else {
+ uart->gpios = gpios;
+ }
+ }
+
serial8250_set_defaults(uart);
/* Possibly override default I/O functions. */
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 0a8316632d75..d5bbfc8f2284 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -141,18 +141,20 @@ static void omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
serial8250_do_set_mctrl(port, mctrl);
- /*
- * Turn off autoRTS if RTS is lowered and restore autoRTS setting
- * if RTS is raised
- */
- lcr = serial_in(up, UART_LCR);
- serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
- if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
- priv->efr |= UART_EFR_RTS;
- else
- priv->efr &= ~UART_EFR_RTS;
- serial_out(up, UART_EFR, priv->efr);
- serial_out(up, UART_LCR, lcr);
+ if (!up->gpios) {
+ /*
+ * Turn off autoRTS if RTS is lowered and restore autoRTS
+ * setting if RTS is raised
+ */
+ lcr = serial_in(up, UART_LCR);
+ serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+ if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
+ priv->efr |= UART_EFR_RTS;
+ else
+ priv->efr &= ~UART_EFR_RTS;
+ serial_out(up, UART_EFR, priv->efr);
+ serial_out(up, UART_LCR, lcr);
+ }
}
/*
@@ -453,7 +455,8 @@ static void omap_8250_set_termios(struct uart_port *port,
priv->efr = 0;
up->port.status &= ~(UPSTAT_AUTOCTS | UPSTAT_AUTORTS | UPSTAT_AUTOXOFF);
- if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW) {
+ if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW &&
+ !up->gpios) {
/* Enable AUTOCTS (autoRTS is enabled when RTS is raised) */
up->port.status |= UPSTAT_AUTOCTS | UPSTAT_AUTORTS;
priv->efr |= UART_EFR_CTS;
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 19791d3694c6..bbecbc505497 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1662,6 +1662,8 @@ static void serial8250_disable_ms(struct uart_port *port)
if (up->bugs & UART_BUG_NOMSR)
return;
+ mctrl_gpio_disable_ms(up->gpios);
+
up->ier &= ~UART_IER_MSI;
serial_port_out(port, UART_IER, up->ier);
}
@@ -1674,6 +1676,8 @@ static void serial8250_enable_ms(struct uart_port *port)
if (up->bugs & UART_BUG_NOMSR)
return;
+ mctrl_gpio_enable_ms(up->gpios);
+
up->ier |= UART_IER_MSI;
serial8250_rpm_get(up);
@@ -1945,12 +1949,17 @@ unsigned int serial8250_do_get_mctrl(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
unsigned int status;
+ unsigned int val;
serial8250_rpm_get(up);
status = serial8250_modem_status(up);
serial8250_rpm_put(up);
- return serial8250_MSR_to_TIOCM(status);
+ val = serial8250_MSR_to_TIOCM(status);
+ if (up->gpios)
+ return mctrl_gpio_get(up->gpios, &val);
+
+ return val;
}
EXPORT_SYMBOL_GPL(serial8250_do_get_mctrl);
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 296115f6a4d8..509f6a3bb9ff 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -8,6 +8,7 @@ config SERIAL_8250
tristate "8250/16550 and compatible serial support"
depends on !S390
select SERIAL_CORE
+ select SERIAL_MCTRL_GPIO if GPIOLIB
---help---
This selects whether you want to include the driver for the standard
serial ports. The standard answer is Y. People who might say N
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 5e0b59422a68..bb2bc99388ca 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -110,6 +110,7 @@ struct uart_8250_port {
* if no_console_suspend
*/
unsigned char probe;
+ struct mctrl_gpios *gpios;
#define UART_PROBE_RSA (1 << 0)
/*
--
2.22.0
^ permalink raw reply related
* [PATCH 2/3 v8] serial: 8250: Add MSR/MCR TIOCM conversion wrapper functions
From: Stefan Roese @ 2019-06-18 8:31 UTC (permalink / raw)
To: linux-serial
Cc: linux-kernel, Andy Shevchenko, Mika Westerberg, Yegor Yefremov,
Greg Kroah-Hartman, Giulio Benetti
In-Reply-To: <20190618083200.30234-1-sr@denx.de>
This patch adds wrapper functions to convert MSR <-> TIOCM and also
MCR <-> TIOCM. These functions are used now in serial8250_do_set_mctrl()
and serial8250_do_get_mctrl().
Signed-off-by: Stefan Roese <sr@denx.de>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Tested-by: Yegor Yefremov <yegorslists@googlemail.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
v8:
- Rebased on top of "tty-next"
v7:
- No change
v6:
- New patch
drivers/tty/serial/8250/8250.h | 54 +++++++++++++++++++++++++++++
drivers/tty/serial/8250/8250_port.c | 25 ++-----------
2 files changed, 57 insertions(+), 22 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index ebfb0bd5bef5..793da2e510e0 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -139,6 +139,60 @@ void serial8250_rpm_put_tx(struct uart_8250_port *p);
int serial8250_em485_init(struct uart_8250_port *p);
void serial8250_em485_destroy(struct uart_8250_port *p);
+/* MCR <-> TIOCM conversion */
+static inline int serial8250_TIOCM_to_MCR(int tiocm)
+{
+ int mcr = 0;
+
+ if (tiocm & TIOCM_RTS)
+ mcr |= UART_MCR_RTS;
+ if (tiocm & TIOCM_DTR)
+ mcr |= UART_MCR_DTR;
+ if (tiocm & TIOCM_OUT1)
+ mcr |= UART_MCR_OUT1;
+ if (tiocm & TIOCM_OUT2)
+ mcr |= UART_MCR_OUT2;
+ if (tiocm & TIOCM_LOOP)
+ mcr |= UART_MCR_LOOP;
+
+ return mcr;
+}
+
+static inline int serial8250_MCR_to_TIOCM(int mcr)
+{
+ int tiocm = 0;
+
+ if (mcr & UART_MCR_RTS)
+ tiocm |= TIOCM_RTS;
+ if (mcr & UART_MCR_DTR)
+ tiocm |= TIOCM_DTR;
+ if (mcr & UART_MCR_OUT1)
+ tiocm |= TIOCM_OUT1;
+ if (mcr & UART_MCR_OUT2)
+ tiocm |= TIOCM_OUT2;
+ if (mcr & UART_MCR_LOOP)
+ tiocm |= TIOCM_LOOP;
+
+ return tiocm;
+}
+
+/* MSR <-> TIOCM conversion */
+static inline int serial8250_MSR_to_TIOCM(int msr)
+{
+ int tiocm = 0;
+
+ if (msr & UART_MSR_DCD)
+ tiocm |= TIOCM_CAR;
+ if (msr & UART_MSR_RI)
+ tiocm |= TIOCM_RNG;
+ if (msr & UART_MSR_DSR)
+ tiocm |= TIOCM_DSR;
+ if (msr & UART_MSR_CTS)
+ tiocm |= TIOCM_CTS;
+
+ return tiocm;
+}
+
static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
{
serial_out(up, UART_MCR, value);
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 60c9cea70128..19791d3694c6 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1945,22 +1945,12 @@ unsigned int serial8250_do_get_mctrl(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
unsigned int status;
- unsigned int ret;
serial8250_rpm_get(up);
status = serial8250_modem_status(up);
serial8250_rpm_put(up);
- ret = 0;
- if (status & UART_MSR_DCD)
- ret |= TIOCM_CAR;
- if (status & UART_MSR_RI)
- ret |= TIOCM_RNG;
- if (status & UART_MSR_DSR)
- ret |= TIOCM_DSR;
- if (status & UART_MSR_CTS)
- ret |= TIOCM_CTS;
- return ret;
+ return serial8250_MSR_to_TIOCM(status);
}
EXPORT_SYMBOL_GPL(serial8250_do_get_mctrl);
@@ -1974,18 +1964,9 @@ static unsigned int serial8250_get_mctrl(struct uart_port *port)
void serial8250_do_set_mctrl(struct uart_port *port, unsigned int mctrl)
{
struct uart_8250_port *up = up_to_u8250p(port);
- unsigned char mcr = 0;
+ unsigned char mcr;
- if (mctrl & TIOCM_RTS)
- mcr |= UART_MCR_RTS;
- if (mctrl & TIOCM_DTR)
- mcr |= UART_MCR_DTR;
- if (mctrl & TIOCM_OUT1)
- mcr |= UART_MCR_OUT1;
- if (mctrl & TIOCM_OUT2)
- mcr |= UART_MCR_OUT2;
- if (mctrl & TIOCM_LOOP)
- mcr |= UART_MCR_LOOP;
+ mcr = serial8250_TIOCM_to_MCR(mctrl);
mcr = (mcr & up->mcr_mask) | up->mcr_force | up->mcr;
--
2.22.0
^ permalink raw reply related
* [PATCH 1/3 v8] serial: mctrl_gpio: Check if GPIO property exisits before requesting it
From: Stefan Roese @ 2019-06-18 8:31 UTC (permalink / raw)
To: linux-serial
Cc: linux-kernel, Mika Westerberg, Andy Shevchenko, Yegor Yefremov,
Greg Kroah-Hartman, Giulio Benetti
This patch adds a check for the GPIOs property existence, before the
GPIO is requested. This fixes an issue seen when the 8250 mctrl_gpio
support is added (2nd patch in this patch series) on x86 platforms using
ACPI.
Here Mika's comments from 2016-08-09:
"
I noticed that with v4.8-rc1 serial console of some of our Broxton
systems does not work properly anymore. I'm able to see output but input
does not work.
I bisected it down to commit 4ef03d328769eddbfeca1f1c958fdb181a69c341
("tty/serial/8250: use mctrl_gpio helpers").
The reason why it fails is that in ACPI we do not have names for GPIOs
(except when _DSD is used) so we use the "idx" to index into _CRS GPIO
resources. Now mctrl_gpio_init_noauto() goes through a list of GPIOs
calling devm_gpiod_get_index_optional() passing "idx" of 0 for each. The
UART device in Broxton has following (simplified) ACPI description:
Device (URT4)
{
...
Name (_CRS, ResourceTemplate () {
GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
"\\_SB.GPO0", 0x00, ResourceConsumer)
{
0x003A
}
GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
"\\_SB.GPO0", 0x00, ResourceConsumer)
{
0x003D
}
})
In this case it finds the first GPIO (0x003A which happens to be RX pin
for that UART), turns it into GPIO which then breaks input for the UART
device. This also breaks systems with bluetooth connected to UART (those
typically have some GPIOs in their _CRS).
Any ideas how to fix this?
We cannot just drop the _CRS index lookup fallback because that would
break many existing machines out there so maybe we can limit this to
only DT enabled machines. Or alternatively probe if the property first
exists before trying to acquire the GPIOs (using
device_property_present()).
"
This patch implements the fix suggested by Mika in his statement above.
Signed-off-by: Stefan Roese <sr@denx.de>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Yegor Yefremov <yegorslists@googlemail.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
v8:
- Rebased on top of "tty-next"
v7:
- Include <linux/property.h> to fix compile breakage on OMAP
v6:
- No change
v5:
- Simplified the code a bit (Andy)
- Added gpio_str == NULL handling (Andy)
v4:
- Add missing free() calls (Johan)
- Added Mika's reviewed by tag
- Added Johan to Cc
v3:
- No change
v2:
- Include the problem description and analysis from Mika into the commit
text, as suggested by Greg.
drivers/tty/serial/serial_mctrl_gpio.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index 39ed56214cd3..2b400189be91 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -12,6 +12,7 @@
#include <linux/termios.h>
#include <linux/serial_core.h>
#include <linux/module.h>
+#include <linux/property.h>
#include "serial_mctrl_gpio.h"
@@ -116,6 +117,19 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct device *dev, unsigned int idx)
for (i = 0; i < UART_GPIO_MAX; i++) {
enum gpiod_flags flags;
+ char *gpio_str;
+ bool present;
+
+ /* Check if GPIO property exists and continue if not */
+ gpio_str = kasprintf(GFP_KERNEL, "%s-gpios",
+ mctrl_gpios_desc[i].name);
+ if (!gpio_str)
+ continue;
+
+ present = device_property_present(dev, gpio_str);
+ kfree(gpio_str);
+ if (!present)
+ continue;
if (mctrl_gpios_desc[i].dir_out)
flags = GPIOD_OUT_LOW;
--
2.22.0
^ permalink raw reply related
* Re: [PATCH] tty: serial_core: recover uport->cons->cflag on uart_close
From: Greg KH @ 2019-06-18 7:51 UTC (permalink / raw)
To: kpark3469; +Cc: jslaby, linux-serial, linux-kernel, keun-o.park
In-Reply-To: <1560697034-4807-1-git-send-email-kpark3469@gmail.com>
On Sun, Jun 16, 2019 at 06:57:14PM +0400, kpark3469@gmail.com wrote:
> From: Sahara <keun-o.park@darkmatter.ae>
I need a "full" name here please, something you use to sign legal
documents.
> Since uart_close was converted to use tty_port_close, uart_shutdown
> also moved to uart_tty_port_shutdown, which means it does not backup
> tty's termios to uart_port.console.cflag when console is closed and
> uart_console is true.
> By losing this value, serial console was not set correctly especially
> after suspend/resume when there is no consumer of console device.
> This problem resets console driver's configuration to an unwanted value
> and may give a performance regression in the system eventually.
> This patch fixes the bug introduced from v4.9 kernel.
>
> Fixes: 761ed4a94582 ("tty: serial_core: convert uart_close to use tty_port_close")
> Reported-by: Jouni Linnamaa <Jouni.Linnamaa@darkmatter.ae>
> Signed-off-by: Sahara <keun-o.park@darkmatter.ae>
Same here as well.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 2/3 v7] serial: 8250: Add MSR/MCR TIOCM conversion wrapper functions
From: Greg Kroah-Hartman @ 2019-06-18 7:50 UTC (permalink / raw)
To: Stefan Roese
Cc: linux-serial, linux-kernel, Andy Shevchenko, Mika Westerberg,
Yegor Yefremov, Giulio Benetti
In-Reply-To: <20190617145952.4848-2-sr@denx.de>
On Mon, Jun 17, 2019 at 04:59:51PM +0200, Stefan Roese wrote:
> This patch adds wrapper functions to convert MSR <-> TIOCM and also
> MCR <-> TIOCM. These functions are used now in serial8250_do_set_mctrl()
> and serial8250_do_get_mctrl().
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Yegor Yefremov <yegorslists@googlemail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
> v7:
> - No change
>
> v6:
> - New patch
>
> drivers/tty/serial/8250/8250.h | 54 +++++++++++++++++++++++++++++
> drivers/tty/serial/8250/8250_port.c | 25 ++-----------
> 2 files changed, 57 insertions(+), 22 deletions(-)
This patch does not apply to my tty-next tree. Please rebase the series
and resend.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 2/2 v5] tty/serial/8250: use mctrl_gpio helpers
From: Greg Kroah-Hartman @ 2019-06-18 7:49 UTC (permalink / raw)
To: Stefan Roese
Cc: linux-serial, linux-kernel, Yegor Yefremov, Mika Westerberg,
Andy Shevchenko, Giulio Benetti
In-Reply-To: <20190611105603.4435-2-sr@denx.de>
On Tue, Jun 11, 2019 at 12:56:03PM +0200, Stefan Roese wrote:
> From: Yegor Yefremov <yegorslists@googlemail.com>
>
> This patch permits the usage for GPIOs to control
> the CTS/RTS/DTR/DSR/DCD/RI signals.
>
> Changed by Stefan:
> Only call mctrl_gpio_init(), if the device has no ACPI companion device
> to not break existing ACPI based systems. Also only use the mctrl_gpio_
> functions when "gpios" is available.
>
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Please do not add a signed-off-by from people for an old patch that was
reverted. It implies that I still agree with this change :(
greg k-h
^ permalink raw reply
* Re: [PATCH] serial: sh-sci: fix uninitialized variable warning
From: Greg Kroah-Hartman @ 2019-06-18 7:38 UTC (permalink / raw)
To: Charles; +Cc: Jiri Slaby, linux-serial, linux-kernel, rodrigosiqueiramelo
In-Reply-To: <20190613180824.6ajwjelzr5fmjnie@debie>
On Thu, Jun 13, 2019 at 03:08:24PM -0300, Charles wrote:
> Avoid following compiler warning on uninitialized variable
>
> In file included from ./include/linux/rwsem.h:16:0,
> from ./include/linux/notifier.h:15,
> from ./include/linux/clk.h:17,
> from drivers/tty/serial/sh-sci.c:24:
> drivers/tty/serial/sh-sci.c: In function ‘sci_dma_rx_submit’:
> ./include/linux/spinlock.h:288:3: warning: ‘flags’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> _raw_spin_unlock_irqrestore(lock, flags); \
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/tty/serial/sh-sci.c:1353:16: note: ‘flags’ was declared here
> unsigned long flags;
> ^~~~~
What version of gcc is doing this? It should be smarter than that,
perhaps you should just upgrade.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 3/3 v7] tty/serial/8250: use mctrl_gpio helpers
From: Yegor Yefremov @ 2019-06-18 7:30 UTC (permalink / raw)
To: Stefan Roese
Cc: linux-serial, kernel list, Andy Shevchenko, Mika Westerberg,
Giulio Benetti, Greg Kroah-Hartman
In-Reply-To: <20190617145952.4848-3-sr@denx.de>
On Mon, Jun 17, 2019 at 5:00 PM Stefan Roese <sr@denx.de> wrote:
>
> From: Yegor Yefremov <yegorslists@googlemail.com>
>
> This patch permits the usage for GPIOs to control
> the CTS/RTS/DTR/DSR/DCD/RI signals.
>
> Changed by Stefan:
> Only call mctrl_gpio_init(), if the device has no ACPI companion device
> to not break existing ACPI based systems. Also only use the mctrl_gpio_
> functions when "gpios" is available.
>
> Use MSR / MCR <-> TIOCM wrapper functions.
>
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Giulio Benetti <giulio.benetti@micronovasrl.com>
> Cc: Yegor Yefremov <yegorslists@googlemail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Tested-by: Yegor Yefremov <yegorslists@googlemail.com>
Thanks for reviving my initial patch series and bringing them into shape.
Yegor
> ---
> v7:
> - Change serial8250_do_get_mctrl() so that systems with a "mixed setup"
> (i.e. CTS controlled by UART but other status pins controlled by GPIO)
> are also supported again (Yegor)
>
> v6:
> - Use newly introduced TIOCM <-> MCR/MSR wrapper functions
> - serial8250_in_MCR(): Don't save the already read MCR bits in TIOCM
> format but "or" them later to the GPIO MCR value
> - Correctly use "!up->gpios" (Andy)
> - Removed Mika's reviewed by tag (because of changes)
>
> v5:
> - Dropped a few "if (up->gpios)" checks, as the mctrl_gpio_foo() API
> handles gpios == NULL (return)
> - 8250_omap: Changed "IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(up->gpios, ...))"
> to "up->gpios == NULL", as mctrl_gpio_to_gpiod() does not handle
> gpios == NULL correctly.
>
> v4:
> - Added Mika's reviewed by tag
> - Added Johan to Cc
>
> v3:
> - Only call mctrl_gpio_init(), if the device has no ACPI companion device
> to not break existing ACPI based systems, as suggested by Andy
>
> v2:
> - No change
>
> Please note that this patch was already applied before [1]. And later
> reverted [2] because it introduced problems on some x86 based boards
> (ACPI GPIO related). Here a detailed description of the issue at that
> time:
>
> https://lkml.org/lkml/2016/8/9/357
> http://www.spinics.net/lists/linux-serial/msg23071.html
>
> This is a re-send of the original patch that was applied at that time.
> With patch 1/2 from this series this issue should be fixed now (please
> note that I can't test it on such an x86 platform causing these
> problems).
>
> Andy (or Mika), perhaps it would be possible for you to test this
> patch again, now with patch 1/2 of this series applied as well?
> That would be really helpful.
>
> Thanks,
> Stefan
>
> [1] 4ef03d328769 ("tty/serial/8250: use mctrl_gpio helpers")
> [2] 5db4f7f80d16 ("Revert "tty/serial/8250: use mctrl_gpio helpers"")
>
> .../devicetree/bindings/serial/8250.txt | 19 ++++++++++++
> drivers/tty/serial/8250/8250.h | 18 +++++++++++-
> drivers/tty/serial/8250/8250_core.c | 17 +++++++++++
> drivers/tty/serial/8250/8250_omap.c | 29 ++++++++++---------
> drivers/tty/serial/8250/8250_port.c | 11 ++++++-
> drivers/tty/serial/8250/Kconfig | 1 +
> include/linux/serial_8250.h | 1 +
> 7 files changed, 81 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
> index 3cba12f855b7..20d351f268ef 100644
> --- a/Documentation/devicetree/bindings/serial/8250.txt
> +++ b/Documentation/devicetree/bindings/serial/8250.txt
> @@ -53,6 +53,9 @@ Optional properties:
> programmable TX FIFO thresholds.
> - resets : phandle + reset specifier pairs
> - overrun-throttle-ms : how long to pause uart rx when input overrun is encountered.
> +- {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD
> + line respectively. It will use specified GPIO instead of the peripheral
> + function pin for the UART feature. If unsure, don't specify this property.
>
> Note:
> * fsl,ns16550:
> @@ -74,3 +77,19 @@ Example:
> interrupts = <10>;
> reg-shift = <2>;
> };
> +
> +Example for OMAP UART using GPIO-based modem control signals:
> +
> + uart4: serial@49042000 {
> + compatible = "ti,omap3-uart";
> + reg = <0x49042000 0x400>;
> + interrupts = <80>;
> + ti,hwmods = "uart4";
> + clock-frequency = <48000000>;
> + cts-gpios = <&gpio3 5 GPIO_ACTIVE_LOW>;
> + rts-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
> + dtr-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
> + dsr-gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
> + dcd-gpios = <&gpio1 14 GPIO_ACTIVE_LOW>;
> + rng-gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
> + };
> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> index 793da2e510e0..75c7c5449461 100644
> --- a/drivers/tty/serial/8250/8250.h
> +++ b/drivers/tty/serial/8250/8250.h
> @@ -11,6 +11,8 @@
> #include <linux/serial_reg.h>
> #include <linux/dmaengine.h>
>
> +#include "../serial_mctrl_gpio.h"
> +
> struct uart_8250_dma {
> int (*tx_dma)(struct uart_8250_port *p);
> int (*rx_dma)(struct uart_8250_port *p);
> @@ -196,11 +198,25 @@ static inline int serial8250_MSR_to_TIOCM(int msr)
> static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
> {
> serial_out(up, UART_MCR, value);
> +
> + if (up->gpios)
> + mctrl_gpio_set(up->gpios, serial8250_MCR_to_TIOCM(value));
> }
>
> static inline int serial8250_in_MCR(struct uart_8250_port *up)
> {
> - return serial_in(up, UART_MCR);
> + int mctrl;
> +
> + mctrl = serial_in(up, UART_MCR);
> +
> + if (up->gpios) {
> + unsigned int mctrl_gpio = 0;
> +
> + mctrl_gpio = mctrl_gpio_get_outputs(up->gpios, &mctrl_gpio);
> + mctrl |= serial8250_TIOCM_to_MCR(mctrl_gpio);
> + }
> +
> + return mctrl;
> }
>
> #if defined(__alpha__) && !defined(CONFIG_PCI)
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index e441221e04b9..a4470771005f 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -14,6 +14,7 @@
> * serial8250_register_8250_port() ports
> */
>
> +#include <linux/acpi.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> #include <linux/ioport.h>
> @@ -982,6 +983,8 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
>
> uart = serial8250_find_match_or_unused(&up->port);
> if (uart && uart->port.type != PORT_8250_CIR) {
> + struct mctrl_gpios *gpios;
> +
> if (uart->port.dev)
> uart_remove_one_port(&serial8250_reg, &uart->port);
>
> @@ -1016,6 +1019,20 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
> if (up->port.flags & UPF_FIXED_TYPE)
> uart->port.type = up->port.type;
>
> + /*
> + * Only call mctrl_gpio_init(), if the device has no ACPI
> + * companion device
> + */
> + if (!has_acpi_companion(uart->port.dev)) {
> + gpios = mctrl_gpio_init(&uart->port, 0);
> + if (IS_ERR(gpios)) {
> + if (PTR_ERR(gpios) != -ENOSYS)
> + return PTR_ERR(gpios);
> + } else {
> + uart->gpios = gpios;
> + }
> + }
> +
> serial8250_set_defaults(uart);
>
> /* Possibly override default I/O functions. */
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 0a8316632d75..d5bbfc8f2284 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -141,18 +141,20 @@ static void omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
>
> serial8250_do_set_mctrl(port, mctrl);
>
> - /*
> - * Turn off autoRTS if RTS is lowered and restore autoRTS setting
> - * if RTS is raised
> - */
> - lcr = serial_in(up, UART_LCR);
> - serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
> - if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
> - priv->efr |= UART_EFR_RTS;
> - else
> - priv->efr &= ~UART_EFR_RTS;
> - serial_out(up, UART_EFR, priv->efr);
> - serial_out(up, UART_LCR, lcr);
> + if (!up->gpios) {
> + /*
> + * Turn off autoRTS if RTS is lowered and restore autoRTS
> + * setting if RTS is raised
> + */
> + lcr = serial_in(up, UART_LCR);
> + serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
> + if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
> + priv->efr |= UART_EFR_RTS;
> + else
> + priv->efr &= ~UART_EFR_RTS;
> + serial_out(up, UART_EFR, priv->efr);
> + serial_out(up, UART_LCR, lcr);
> + }
> }
>
> /*
> @@ -453,7 +455,8 @@ static void omap_8250_set_termios(struct uart_port *port,
> priv->efr = 0;
> up->port.status &= ~(UPSTAT_AUTOCTS | UPSTAT_AUTORTS | UPSTAT_AUTOXOFF);
>
> - if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW) {
> + if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW &&
> + !up->gpios) {
> /* Enable AUTOCTS (autoRTS is enabled when RTS is raised) */
> up->port.status |= UPSTAT_AUTOCTS | UPSTAT_AUTORTS;
> priv->efr |= UART_EFR_CTS;
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 47f0a8d01a57..ff3dd0c1c90a 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1662,6 +1662,8 @@ static void serial8250_disable_ms(struct uart_port *port)
> if (up->bugs & UART_BUG_NOMSR)
> return;
>
> + mctrl_gpio_disable_ms(up->gpios);
> +
> up->ier &= ~UART_IER_MSI;
> serial_port_out(port, UART_IER, up->ier);
> }
> @@ -1674,6 +1676,8 @@ static void serial8250_enable_ms(struct uart_port *port)
> if (up->bugs & UART_BUG_NOMSR)
> return;
>
> + mctrl_gpio_enable_ms(up->gpios);
> +
> up->ier |= UART_IER_MSI;
>
> serial8250_rpm_get(up);
> @@ -1944,12 +1948,17 @@ unsigned int serial8250_do_get_mctrl(struct uart_port *port)
> {
> struct uart_8250_port *up = up_to_u8250p(port);
> unsigned int status;
> + unsigned int val;
>
> serial8250_rpm_get(up);
> status = serial8250_modem_status(up);
> serial8250_rpm_put(up);
>
> - return serial8250_MSR_to_TIOCM(status);
> + val = serial8250_MSR_to_TIOCM(status);
> + if (up->gpios)
> + return mctrl_gpio_get(up->gpios, &val);
> +
> + return val;
> }
> EXPORT_SYMBOL_GPL(serial8250_do_get_mctrl);
>
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 296115f6a4d8..509f6a3bb9ff 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -8,6 +8,7 @@ config SERIAL_8250
> tristate "8250/16550 and compatible serial support"
> depends on !S390
> select SERIAL_CORE
> + select SERIAL_MCTRL_GPIO if GPIOLIB
> ---help---
> This selects whether you want to include the driver for the standard
> serial ports. The standard answer is Y. People who might say N
> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> index 5e0b59422a68..bb2bc99388ca 100644
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -110,6 +110,7 @@ struct uart_8250_port {
> * if no_console_suspend
> */
> unsigned char probe;
> + struct mctrl_gpios *gpios;
> #define UART_PROBE_RSA (1 << 0)
>
> /*
> --
> 2.22.0
>
^ permalink raw reply
* Re: [PATCH 2/3 v7] serial: 8250: Add MSR/MCR TIOCM conversion wrapper functions
From: Yegor Yefremov @ 2019-06-18 7:28 UTC (permalink / raw)
To: Stefan Roese
Cc: linux-serial, kernel list, Andy Shevchenko, Mika Westerberg,
Greg Kroah-Hartman, Giulio Benetti
In-Reply-To: <20190617145952.4848-2-sr@denx.de>
On Mon, Jun 17, 2019 at 4:59 PM Stefan Roese <sr@denx.de> wrote:
>
> This patch adds wrapper functions to convert MSR <-> TIOCM and also
> MCR <-> TIOCM. These functions are used now in serial8250_do_set_mctrl()
> and serial8250_do_get_mctrl().
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Yegor Yefremov <yegorslists@googlemail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Giulio Benetti <giulio.benetti@micronovasrl.com>
Tested-by: Yegor Yefremov <yegorslists@googlemail.com>
> ---
> v7:
> - No change
>
> v6:
> - New patch
>
> drivers/tty/serial/8250/8250.h | 54 +++++++++++++++++++++++++++++
> drivers/tty/serial/8250/8250_port.c | 25 ++-----------
> 2 files changed, 57 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> index ebfb0bd5bef5..793da2e510e0 100644
> --- a/drivers/tty/serial/8250/8250.h
> +++ b/drivers/tty/serial/8250/8250.h
> @@ -139,6 +139,60 @@ void serial8250_rpm_put_tx(struct uart_8250_port *p);
> int serial8250_em485_init(struct uart_8250_port *p);
> void serial8250_em485_destroy(struct uart_8250_port *p);
>
> +/* MCR <-> TIOCM conversion */
> +static inline int serial8250_TIOCM_to_MCR(int tiocm)
> +{
> + int mcr = 0;
> +
> + if (tiocm & TIOCM_RTS)
> + mcr |= UART_MCR_RTS;
> + if (tiocm & TIOCM_DTR)
> + mcr |= UART_MCR_DTR;
> + if (tiocm & TIOCM_OUT1)
> + mcr |= UART_MCR_OUT1;
> + if (tiocm & TIOCM_OUT2)
> + mcr |= UART_MCR_OUT2;
> + if (tiocm & TIOCM_LOOP)
> + mcr |= UART_MCR_LOOP;
> +
> + return mcr;
> +}
> +
> +static inline int serial8250_MCR_to_TIOCM(int mcr)
> +{
> + int tiocm = 0;
> +
> + if (mcr & UART_MCR_RTS)
> + tiocm |= TIOCM_RTS;
> + if (mcr & UART_MCR_DTR)
> + tiocm |= TIOCM_DTR;
> + if (mcr & UART_MCR_OUT1)
> + tiocm |= TIOCM_OUT1;
> + if (mcr & UART_MCR_OUT2)
> + tiocm |= TIOCM_OUT2;
> + if (mcr & UART_MCR_LOOP)
> + tiocm |= TIOCM_LOOP;
> +
> + return tiocm;
> +}
> +
> +/* MSR <-> TIOCM conversion */
> +static inline int serial8250_MSR_to_TIOCM(int msr)
> +{
> + int tiocm = 0;
> +
> + if (msr & UART_MSR_DCD)
> + tiocm |= TIOCM_CAR;
> + if (msr & UART_MSR_RI)
> + tiocm |= TIOCM_RNG;
> + if (msr & UART_MSR_DSR)
> + tiocm |= TIOCM_DSR;
> + if (msr & UART_MSR_CTS)
> + tiocm |= TIOCM_CTS;
> +
> + return tiocm;
> +}
> +
> static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
> {
> serial_out(up, UART_MCR, value);
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 2304a84eee3b..47f0a8d01a57 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1944,22 +1944,12 @@ unsigned int serial8250_do_get_mctrl(struct uart_port *port)
> {
> struct uart_8250_port *up = up_to_u8250p(port);
> unsigned int status;
> - unsigned int ret;
>
> serial8250_rpm_get(up);
> status = serial8250_modem_status(up);
> serial8250_rpm_put(up);
>
> - ret = 0;
> - if (status & UART_MSR_DCD)
> - ret |= TIOCM_CAR;
> - if (status & UART_MSR_RI)
> - ret |= TIOCM_RNG;
> - if (status & UART_MSR_DSR)
> - ret |= TIOCM_DSR;
> - if (status & UART_MSR_CTS)
> - ret |= TIOCM_CTS;
> - return ret;
> + return serial8250_MSR_to_TIOCM(status);
> }
> EXPORT_SYMBOL_GPL(serial8250_do_get_mctrl);
>
> @@ -1973,18 +1963,9 @@ static unsigned int serial8250_get_mctrl(struct uart_port *port)
> void serial8250_do_set_mctrl(struct uart_port *port, unsigned int mctrl)
> {
> struct uart_8250_port *up = up_to_u8250p(port);
> - unsigned char mcr = 0;
> + unsigned char mcr;
>
> - if (mctrl & TIOCM_RTS)
> - mcr |= UART_MCR_RTS;
> - if (mctrl & TIOCM_DTR)
> - mcr |= UART_MCR_DTR;
> - if (mctrl & TIOCM_OUT1)
> - mcr |= UART_MCR_OUT1;
> - if (mctrl & TIOCM_OUT2)
> - mcr |= UART_MCR_OUT2;
> - if (mctrl & TIOCM_LOOP)
> - mcr |= UART_MCR_LOOP;
> + mcr = serial8250_TIOCM_to_MCR(mctrl);
>
> mcr = (mcr & up->mcr_mask) | up->mcr_force | up->mcr;
>
> --
> 2.22.0
>
^ permalink raw reply
* Re: [PATCH 1/3 v7] serial: mctrl_gpio: Check if GPIO property exisits before requesting it
From: Yegor Yefremov @ 2019-06-18 7:28 UTC (permalink / raw)
To: Stefan Roese
Cc: linux-serial, kernel list, Mika Westerberg, Andy Shevchenko,
Greg Kroah-Hartman, Giulio Benetti
In-Reply-To: <20190617145952.4848-1-sr@denx.de>
On Mon, Jun 17, 2019 at 5:00 PM Stefan Roese <sr@denx.de> wrote:
>
> This patch adds a check for the GPIOs property existence, before the
> GPIO is requested. This fixes an issue seen when the 8250 mctrl_gpio
> support is added (2nd patch in this patch series) on x86 platforms using
> ACPI.
>
> Here Mika's comments from 2016-08-09:
>
> "
> I noticed that with v4.8-rc1 serial console of some of our Broxton
> systems does not work properly anymore. I'm able to see output but input
> does not work.
>
> I bisected it down to commit 4ef03d328769eddbfeca1f1c958fdb181a69c341
> ("tty/serial/8250: use mctrl_gpio helpers").
>
> The reason why it fails is that in ACPI we do not have names for GPIOs
> (except when _DSD is used) so we use the "idx" to index into _CRS GPIO
> resources. Now mctrl_gpio_init_noauto() goes through a list of GPIOs
> calling devm_gpiod_get_index_optional() passing "idx" of 0 for each. The
> UART device in Broxton has following (simplified) ACPI description:
>
> Device (URT4)
> {
> ...
> Name (_CRS, ResourceTemplate () {
> GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
> "\\_SB.GPO0", 0x00, ResourceConsumer)
> {
> 0x003A
> }
> GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
> "\\_SB.GPO0", 0x00, ResourceConsumer)
> {
> 0x003D
> }
> })
>
> In this case it finds the first GPIO (0x003A which happens to be RX pin
> for that UART), turns it into GPIO which then breaks input for the UART
> device. This also breaks systems with bluetooth connected to UART (those
> typically have some GPIOs in their _CRS).
>
> Any ideas how to fix this?
>
> We cannot just drop the _CRS index lookup fallback because that would
> break many existing machines out there so maybe we can limit this to
> only DT enabled machines. Or alternatively probe if the property first
> exists before trying to acquire the GPIOs (using
> device_property_present()).
> "
>
> This patch implements the fix suggested by Mika in his statement above.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Yegor Yefremov <yegorslists@googlemail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Giulio Benetti <giulio.benetti@micronovasrl.com>
Tested-by: Yegor Yefremov <yegorslists@googlemail.com>
> ---
> v7:
> - Include <linux/property.h> to fix compile breakage on OMAP
>
> v6:
> - No change
>
> v5:
> - Simplified the code a bit (Andy)
> - Added gpio_str == NULL handling (Andy)
>
> v4:
> - Add missing free() calls (Johan)
> - Added Mika's reviewed by tag
> - Added Johan to Cc
>
> v3:
> - No change
>
> v2:
> - Include the problem description and analysis from Mika into the commit
> text, as suggested by Greg.
>
> drivers/tty/serial/serial_mctrl_gpio.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
> index 39ed56214cd3..2b400189be91 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.c
> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> @@ -12,6 +12,7 @@
> #include <linux/termios.h>
> #include <linux/serial_core.h>
> #include <linux/module.h>
> +#include <linux/property.h>
>
> #include "serial_mctrl_gpio.h"
>
> @@ -116,6 +117,19 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct device *dev, unsigned int idx)
>
> for (i = 0; i < UART_GPIO_MAX; i++) {
> enum gpiod_flags flags;
> + char *gpio_str;
> + bool present;
> +
> + /* Check if GPIO property exists and continue if not */
> + gpio_str = kasprintf(GFP_KERNEL, "%s-gpios",
> + mctrl_gpios_desc[i].name);
> + if (!gpio_str)
> + continue;
> +
> + present = device_property_present(dev, gpio_str);
> + kfree(gpio_str);
> + if (!present)
> + continue;
>
> if (mctrl_gpios_desc[i].dir_out)
> flags = GPIOD_OUT_LOW;
> --
> 2.22.0
>
^ permalink raw reply
* Re: [PATCH 1/6] dt-bindings: arm: ti: Add bindings for J721E SoC
From: Nishanth Menon @ 2019-06-17 15:46 UTC (permalink / raw)
To: Rob Herring
Cc: Arnd Bergmann, Olof Johansson, Santosh Shilimkar, Will Deacon,
Catalin Marinas, Greg Kroah-Hartman, Mark Rutland, linux-serial,
linux-kernel, devicetree, linux-arm-kernel, Tony Lindgren,
Russell King, Tero Kristo
In-Reply-To: <20190614164526.GA14925@bogus>
On 10:45-20190614, Rob Herring wrote:
> On Wed, May 22, 2019 at 11:19:16AM -0500, Nishanth Menon wrote:
> > The J721E SoC belongs to the K3 Multicore SoC architecture platform,
> > providing advanced system integration to enable lower system costs
> > of automotive applications such as infotainment, cluster, premium
> > Audio, Gateway, industrial and a range of broad market applications.
> > This SoC is designed around reducing the system cost by eliminating
> > the need of an external system MCU and is targeted towards ASIL-B/C
> > certification/requirements in addition to allowing complex software
> > and system use-cases.
> >
> > Some highlights of this SoC are:
> > * Dual Cortex-A72s in a single cluster, three clusters of lockstep
> > capable dual Cortex-R5F MCUs, Deep-learning Matrix Multiply Accelerator(MMA),
> > C7x floating point Vector DSP, Two C66x floating point DSPs.
> > * 3D GPU PowerVR Rogue 8XE GE8430
> > * Vision Processing Accelerator (VPAC) with image signal processor and Depth
> > and Motion Processing Accelerator (DMPAC)
> > * Two Gigabit Industrial Communication Subsystems (ICSSG), each with dual
> > PRUs and dual RTUs
> > * Two CSI2.0 4L RX plus one CSI2.0 4L TX, one eDP/DP, One DSI Tx, and
> > up to two DPI interfaces.
> > * Integrated Ethernet switch supporting up to a total of 8 external ports in
> > addition to legacy Ethernet switch of up to 2 ports.
> > * System MMU (SMMU) Version 3.0 and advanced virtualisation
> > capabilities.
> > * Upto 4 PCIe-GEN3 controllers, 2 USB3.0 Dual-role device subsystems,
> > 16 MCANs, 12 McASP, eMMC and SD, UFS, OSPI/HyperBus memory controller, QSPI,
> > I3C and I2C, eCAP/eQEP, eHRPWM, MLB among other peripherals.
> > * Two hardware accelerator block containing AES/DES/SHA/MD5 called SA2UL
> > management.
> > * Configurable L3 Cache and IO-coherent architecture with high data throughput
> > capable distributed DMA architecture under NAVSS
> > * Centralized System Controller for Security, Power, and Resource
> > Management (DMSC)
> >
> > See J721E Technical Reference Manual (SPRUIL1, May 2019)
> > for further details: http://www.ti.com/lit/pdf/spruil1
> >
> > Signed-off-by: Nishanth Menon <nm@ti.com>
> > ---
> > Documentation/devicetree/bindings/arm/ti/k3.txt | 3 +++
> > 1 file changed, 3 insertions(+)
>
> Okay for now, but please convert K3 and other TI SoCs to schema soon.
>
> Reviewed-by: Rob Herring <robh@kernel.org>
Thanks Rob. Will do the change to rst soon (hopefully post in the 5.3 window).
--
Regards,
Nishanth Menon
^ permalink raw reply
* [PATCH 3/3 v7] tty/serial/8250: use mctrl_gpio helpers
From: Stefan Roese @ 2019-06-17 14:59 UTC (permalink / raw)
To: linux-serial
Cc: linux-kernel, Yegor Yefremov, Andy Shevchenko, Mika Westerberg,
Giulio Benetti, Greg Kroah-Hartman
In-Reply-To: <20190617145952.4848-1-sr@denx.de>
From: Yegor Yefremov <yegorslists@googlemail.com>
This patch permits the usage for GPIOs to control
the CTS/RTS/DTR/DSR/DCD/RI signals.
Changed by Stefan:
Only call mctrl_gpio_init(), if the device has no ACPI companion device
to not break existing ACPI based systems. Also only use the mctrl_gpio_
functions when "gpios" is available.
Use MSR / MCR <-> TIOCM wrapper functions.
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stefan Roese <sr@denx.de>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Giulio Benetti <giulio.benetti@micronovasrl.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v7:
- Change serial8250_do_get_mctrl() so that systems with a "mixed setup"
(i.e. CTS controlled by UART but other status pins controlled by GPIO)
are also supported again (Yegor)
v6:
- Use newly introduced TIOCM <-> MCR/MSR wrapper functions
- serial8250_in_MCR(): Don't save the already read MCR bits in TIOCM
format but "or" them later to the GPIO MCR value
- Correctly use "!up->gpios" (Andy)
- Removed Mika's reviewed by tag (because of changes)
v5:
- Dropped a few "if (up->gpios)" checks, as the mctrl_gpio_foo() API
handles gpios == NULL (return)
- 8250_omap: Changed "IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(up->gpios, ...))"
to "up->gpios == NULL", as mctrl_gpio_to_gpiod() does not handle
gpios == NULL correctly.
v4:
- Added Mika's reviewed by tag
- Added Johan to Cc
v3:
- Only call mctrl_gpio_init(), if the device has no ACPI companion device
to not break existing ACPI based systems, as suggested by Andy
v2:
- No change
Please note that this patch was already applied before [1]. And later
reverted [2] because it introduced problems on some x86 based boards
(ACPI GPIO related). Here a detailed description of the issue at that
time:
https://lkml.org/lkml/2016/8/9/357
http://www.spinics.net/lists/linux-serial/msg23071.html
This is a re-send of the original patch that was applied at that time.
With patch 1/2 from this series this issue should be fixed now (please
note that I can't test it on such an x86 platform causing these
problems).
Andy (or Mika), perhaps it would be possible for you to test this
patch again, now with patch 1/2 of this series applied as well?
That would be really helpful.
Thanks,
Stefan
[1] 4ef03d328769 ("tty/serial/8250: use mctrl_gpio helpers")
[2] 5db4f7f80d16 ("Revert "tty/serial/8250: use mctrl_gpio helpers"")
.../devicetree/bindings/serial/8250.txt | 19 ++++++++++++
drivers/tty/serial/8250/8250.h | 18 +++++++++++-
drivers/tty/serial/8250/8250_core.c | 17 +++++++++++
drivers/tty/serial/8250/8250_omap.c | 29 ++++++++++---------
drivers/tty/serial/8250/8250_port.c | 11 ++++++-
drivers/tty/serial/8250/Kconfig | 1 +
include/linux/serial_8250.h | 1 +
7 files changed, 81 insertions(+), 15 deletions(-)
diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
index 3cba12f855b7..20d351f268ef 100644
--- a/Documentation/devicetree/bindings/serial/8250.txt
+++ b/Documentation/devicetree/bindings/serial/8250.txt
@@ -53,6 +53,9 @@ Optional properties:
programmable TX FIFO thresholds.
- resets : phandle + reset specifier pairs
- overrun-throttle-ms : how long to pause uart rx when input overrun is encountered.
+- {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD
+ line respectively. It will use specified GPIO instead of the peripheral
+ function pin for the UART feature. If unsure, don't specify this property.
Note:
* fsl,ns16550:
@@ -74,3 +77,19 @@ Example:
interrupts = <10>;
reg-shift = <2>;
};
+
+Example for OMAP UART using GPIO-based modem control signals:
+
+ uart4: serial@49042000 {
+ compatible = "ti,omap3-uart";
+ reg = <0x49042000 0x400>;
+ interrupts = <80>;
+ ti,hwmods = "uart4";
+ clock-frequency = <48000000>;
+ cts-gpios = <&gpio3 5 GPIO_ACTIVE_LOW>;
+ rts-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
+ dtr-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
+ dsr-gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
+ dcd-gpios = <&gpio1 14 GPIO_ACTIVE_LOW>;
+ rng-gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
+ };
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 793da2e510e0..75c7c5449461 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -11,6 +11,8 @@
#include <linux/serial_reg.h>
#include <linux/dmaengine.h>
+#include "../serial_mctrl_gpio.h"
+
struct uart_8250_dma {
int (*tx_dma)(struct uart_8250_port *p);
int (*rx_dma)(struct uart_8250_port *p);
@@ -196,11 +198,25 @@ static inline int serial8250_MSR_to_TIOCM(int msr)
static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
{
serial_out(up, UART_MCR, value);
+
+ if (up->gpios)
+ mctrl_gpio_set(up->gpios, serial8250_MCR_to_TIOCM(value));
}
static inline int serial8250_in_MCR(struct uart_8250_port *up)
{
- return serial_in(up, UART_MCR);
+ int mctrl;
+
+ mctrl = serial_in(up, UART_MCR);
+
+ if (up->gpios) {
+ unsigned int mctrl_gpio = 0;
+
+ mctrl_gpio = mctrl_gpio_get_outputs(up->gpios, &mctrl_gpio);
+ mctrl |= serial8250_TIOCM_to_MCR(mctrl_gpio);
+ }
+
+ return mctrl;
}
#if defined(__alpha__) && !defined(CONFIG_PCI)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index e441221e04b9..a4470771005f 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -14,6 +14,7 @@
* serial8250_register_8250_port() ports
*/
+#include <linux/acpi.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/ioport.h>
@@ -982,6 +983,8 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
uart = serial8250_find_match_or_unused(&up->port);
if (uart && uart->port.type != PORT_8250_CIR) {
+ struct mctrl_gpios *gpios;
+
if (uart->port.dev)
uart_remove_one_port(&serial8250_reg, &uart->port);
@@ -1016,6 +1019,20 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
if (up->port.flags & UPF_FIXED_TYPE)
uart->port.type = up->port.type;
+ /*
+ * Only call mctrl_gpio_init(), if the device has no ACPI
+ * companion device
+ */
+ if (!has_acpi_companion(uart->port.dev)) {
+ gpios = mctrl_gpio_init(&uart->port, 0);
+ if (IS_ERR(gpios)) {
+ if (PTR_ERR(gpios) != -ENOSYS)
+ return PTR_ERR(gpios);
+ } else {
+ uart->gpios = gpios;
+ }
+ }
+
serial8250_set_defaults(uart);
/* Possibly override default I/O functions. */
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 0a8316632d75..d5bbfc8f2284 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -141,18 +141,20 @@ static void omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
serial8250_do_set_mctrl(port, mctrl);
- /*
- * Turn off autoRTS if RTS is lowered and restore autoRTS setting
- * if RTS is raised
- */
- lcr = serial_in(up, UART_LCR);
- serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
- if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
- priv->efr |= UART_EFR_RTS;
- else
- priv->efr &= ~UART_EFR_RTS;
- serial_out(up, UART_EFR, priv->efr);
- serial_out(up, UART_LCR, lcr);
+ if (!up->gpios) {
+ /*
+ * Turn off autoRTS if RTS is lowered and restore autoRTS
+ * setting if RTS is raised
+ */
+ lcr = serial_in(up, UART_LCR);
+ serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+ if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
+ priv->efr |= UART_EFR_RTS;
+ else
+ priv->efr &= ~UART_EFR_RTS;
+ serial_out(up, UART_EFR, priv->efr);
+ serial_out(up, UART_LCR, lcr);
+ }
}
/*
@@ -453,7 +455,8 @@ static void omap_8250_set_termios(struct uart_port *port,
priv->efr = 0;
up->port.status &= ~(UPSTAT_AUTOCTS | UPSTAT_AUTORTS | UPSTAT_AUTOXOFF);
- if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW) {
+ if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW &&
+ !up->gpios) {
/* Enable AUTOCTS (autoRTS is enabled when RTS is raised) */
up->port.status |= UPSTAT_AUTOCTS | UPSTAT_AUTORTS;
priv->efr |= UART_EFR_CTS;
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 47f0a8d01a57..ff3dd0c1c90a 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1662,6 +1662,8 @@ static void serial8250_disable_ms(struct uart_port *port)
if (up->bugs & UART_BUG_NOMSR)
return;
+ mctrl_gpio_disable_ms(up->gpios);
+
up->ier &= ~UART_IER_MSI;
serial_port_out(port, UART_IER, up->ier);
}
@@ -1674,6 +1676,8 @@ static void serial8250_enable_ms(struct uart_port *port)
if (up->bugs & UART_BUG_NOMSR)
return;
+ mctrl_gpio_enable_ms(up->gpios);
+
up->ier |= UART_IER_MSI;
serial8250_rpm_get(up);
@@ -1944,12 +1948,17 @@ unsigned int serial8250_do_get_mctrl(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
unsigned int status;
+ unsigned int val;
serial8250_rpm_get(up);
status = serial8250_modem_status(up);
serial8250_rpm_put(up);
- return serial8250_MSR_to_TIOCM(status);
+ val = serial8250_MSR_to_TIOCM(status);
+ if (up->gpios)
+ return mctrl_gpio_get(up->gpios, &val);
+
+ return val;
}
EXPORT_SYMBOL_GPL(serial8250_do_get_mctrl);
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 296115f6a4d8..509f6a3bb9ff 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -8,6 +8,7 @@ config SERIAL_8250
tristate "8250/16550 and compatible serial support"
depends on !S390
select SERIAL_CORE
+ select SERIAL_MCTRL_GPIO if GPIOLIB
---help---
This selects whether you want to include the driver for the standard
serial ports. The standard answer is Y. People who might say N
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 5e0b59422a68..bb2bc99388ca 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -110,6 +110,7 @@ struct uart_8250_port {
* if no_console_suspend
*/
unsigned char probe;
+ struct mctrl_gpios *gpios;
#define UART_PROBE_RSA (1 << 0)
/*
--
2.22.0
^ permalink raw reply related
* [PATCH 2/3 v7] serial: 8250: Add MSR/MCR TIOCM conversion wrapper functions
From: Stefan Roese @ 2019-06-17 14:59 UTC (permalink / raw)
To: linux-serial
Cc: linux-kernel, Andy Shevchenko, Mika Westerberg, Yegor Yefremov,
Greg Kroah-Hartman, Giulio Benetti
In-Reply-To: <20190617145952.4848-1-sr@denx.de>
This patch adds wrapper functions to convert MSR <-> TIOCM and also
MCR <-> TIOCM. These functions are used now in serial8250_do_set_mctrl()
and serial8250_do_get_mctrl().
Signed-off-by: Stefan Roese <sr@denx.de>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
v7:
- No change
v6:
- New patch
drivers/tty/serial/8250/8250.h | 54 +++++++++++++++++++++++++++++
drivers/tty/serial/8250/8250_port.c | 25 ++-----------
2 files changed, 57 insertions(+), 22 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index ebfb0bd5bef5..793da2e510e0 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -139,6 +139,60 @@ void serial8250_rpm_put_tx(struct uart_8250_port *p);
int serial8250_em485_init(struct uart_8250_port *p);
void serial8250_em485_destroy(struct uart_8250_port *p);
+/* MCR <-> TIOCM conversion */
+static inline int serial8250_TIOCM_to_MCR(int tiocm)
+{
+ int mcr = 0;
+
+ if (tiocm & TIOCM_RTS)
+ mcr |= UART_MCR_RTS;
+ if (tiocm & TIOCM_DTR)
+ mcr |= UART_MCR_DTR;
+ if (tiocm & TIOCM_OUT1)
+ mcr |= UART_MCR_OUT1;
+ if (tiocm & TIOCM_OUT2)
+ mcr |= UART_MCR_OUT2;
+ if (tiocm & TIOCM_LOOP)
+ mcr |= UART_MCR_LOOP;
+
+ return mcr;
+}
+
+static inline int serial8250_MCR_to_TIOCM(int mcr)
+{
+ int tiocm = 0;
+
+ if (mcr & UART_MCR_RTS)
+ tiocm |= TIOCM_RTS;
+ if (mcr & UART_MCR_DTR)
+ tiocm |= TIOCM_DTR;
+ if (mcr & UART_MCR_OUT1)
+ tiocm |= TIOCM_OUT1;
+ if (mcr & UART_MCR_OUT2)
+ tiocm |= TIOCM_OUT2;
+ if (mcr & UART_MCR_LOOP)
+ tiocm |= TIOCM_LOOP;
+
+ return tiocm;
+}
+
+/* MSR <-> TIOCM conversion */
+static inline int serial8250_MSR_to_TIOCM(int msr)
+{
+ int tiocm = 0;
+
+ if (msr & UART_MSR_DCD)
+ tiocm |= TIOCM_CAR;
+ if (msr & UART_MSR_RI)
+ tiocm |= TIOCM_RNG;
+ if (msr & UART_MSR_DSR)
+ tiocm |= TIOCM_DSR;
+ if (msr & UART_MSR_CTS)
+ tiocm |= TIOCM_CTS;
+
+ return tiocm;
+}
+
static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
{
serial_out(up, UART_MCR, value);
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 2304a84eee3b..47f0a8d01a57 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1944,22 +1944,12 @@ unsigned int serial8250_do_get_mctrl(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
unsigned int status;
- unsigned int ret;
serial8250_rpm_get(up);
status = serial8250_modem_status(up);
serial8250_rpm_put(up);
- ret = 0;
- if (status & UART_MSR_DCD)
- ret |= TIOCM_CAR;
- if (status & UART_MSR_RI)
- ret |= TIOCM_RNG;
- if (status & UART_MSR_DSR)
- ret |= TIOCM_DSR;
- if (status & UART_MSR_CTS)
- ret |= TIOCM_CTS;
- return ret;
+ return serial8250_MSR_to_TIOCM(status);
}
EXPORT_SYMBOL_GPL(serial8250_do_get_mctrl);
@@ -1973,18 +1963,9 @@ static unsigned int serial8250_get_mctrl(struct uart_port *port)
void serial8250_do_set_mctrl(struct uart_port *port, unsigned int mctrl)
{
struct uart_8250_port *up = up_to_u8250p(port);
- unsigned char mcr = 0;
+ unsigned char mcr;
- if (mctrl & TIOCM_RTS)
- mcr |= UART_MCR_RTS;
- if (mctrl & TIOCM_DTR)
- mcr |= UART_MCR_DTR;
- if (mctrl & TIOCM_OUT1)
- mcr |= UART_MCR_OUT1;
- if (mctrl & TIOCM_OUT2)
- mcr |= UART_MCR_OUT2;
- if (mctrl & TIOCM_LOOP)
- mcr |= UART_MCR_LOOP;
+ mcr = serial8250_TIOCM_to_MCR(mctrl);
mcr = (mcr & up->mcr_mask) | up->mcr_force | up->mcr;
--
2.22.0
^ permalink raw reply related
* [PATCH 1/3 v7] serial: mctrl_gpio: Check if GPIO property exisits before requesting it
From: Stefan Roese @ 2019-06-17 14:59 UTC (permalink / raw)
To: linux-serial
Cc: linux-kernel, Mika Westerberg, Andy Shevchenko, Yegor Yefremov,
Greg Kroah-Hartman, Giulio Benetti
This patch adds a check for the GPIOs property existence, before the
GPIO is requested. This fixes an issue seen when the 8250 mctrl_gpio
support is added (2nd patch in this patch series) on x86 platforms using
ACPI.
Here Mika's comments from 2016-08-09:
"
I noticed that with v4.8-rc1 serial console of some of our Broxton
systems does not work properly anymore. I'm able to see output but input
does not work.
I bisected it down to commit 4ef03d328769eddbfeca1f1c958fdb181a69c341
("tty/serial/8250: use mctrl_gpio helpers").
The reason why it fails is that in ACPI we do not have names for GPIOs
(except when _DSD is used) so we use the "idx" to index into _CRS GPIO
resources. Now mctrl_gpio_init_noauto() goes through a list of GPIOs
calling devm_gpiod_get_index_optional() passing "idx" of 0 for each. The
UART device in Broxton has following (simplified) ACPI description:
Device (URT4)
{
...
Name (_CRS, ResourceTemplate () {
GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
"\\_SB.GPO0", 0x00, ResourceConsumer)
{
0x003A
}
GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
"\\_SB.GPO0", 0x00, ResourceConsumer)
{
0x003D
}
})
In this case it finds the first GPIO (0x003A which happens to be RX pin
for that UART), turns it into GPIO which then breaks input for the UART
device. This also breaks systems with bluetooth connected to UART (those
typically have some GPIOs in their _CRS).
Any ideas how to fix this?
We cannot just drop the _CRS index lookup fallback because that would
break many existing machines out there so maybe we can limit this to
only DT enabled machines. Or alternatively probe if the property first
exists before trying to acquire the GPIOs (using
device_property_present()).
"
This patch implements the fix suggested by Mika in his statement above.
Signed-off-by: Stefan Roese <sr@denx.de>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
v7:
- Include <linux/property.h> to fix compile breakage on OMAP
v6:
- No change
v5:
- Simplified the code a bit (Andy)
- Added gpio_str == NULL handling (Andy)
v4:
- Add missing free() calls (Johan)
- Added Mika's reviewed by tag
- Added Johan to Cc
v3:
- No change
v2:
- Include the problem description and analysis from Mika into the commit
text, as suggested by Greg.
drivers/tty/serial/serial_mctrl_gpio.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index 39ed56214cd3..2b400189be91 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -12,6 +12,7 @@
#include <linux/termios.h>
#include <linux/serial_core.h>
#include <linux/module.h>
+#include <linux/property.h>
#include "serial_mctrl_gpio.h"
@@ -116,6 +117,19 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct device *dev, unsigned int idx)
for (i = 0; i < UART_GPIO_MAX; i++) {
enum gpiod_flags flags;
+ char *gpio_str;
+ bool present;
+
+ /* Check if GPIO property exists and continue if not */
+ gpio_str = kasprintf(GFP_KERNEL, "%s-gpios",
+ mctrl_gpios_desc[i].name);
+ if (!gpio_str)
+ continue;
+
+ present = device_property_present(dev, gpio_str);
+ kfree(gpio_str);
+ if (!present)
+ continue;
if (mctrl_gpios_desc[i].dir_out)
flags = GPIOD_OUT_LOW;
--
2.22.0
^ permalink raw reply related
* Re: [PATCH 3/3 v6] tty/serial/8250: use mctrl_gpio helpers
From: Yegor Yefremov @ 2019-06-17 12:51 UTC (permalink / raw)
To: Stefan Roese
Cc: linux-serial, kernel list, Mika Westerberg, Andy Shevchenko,
Giulio Benetti, Greg Kroah-Hartman
In-Reply-To: <d62c1a2b-3e24-c109-a7fb-57190388d75f@denx.de>
On Mon, Jun 17, 2019 at 2:42 PM Stefan Roese <sr@denx.de> wrote:
>
> On 17.06.19 11:51, Yegor Yefremov wrote:
>
> <snip>
>
> >>> @@ -1944,11 +1948,15 @@ unsigned int serial8250_do_get_mctrl(struct uart_port *port)
> >>> {
> >>> struct uart_8250_port *up = up_to_u8250p(port);
> >>> unsigned int status;
> >>> + unsigned int val = 0;
> >>>
> >>> serial8250_rpm_get(up);
> >>> status = serial8250_modem_status(up);
> >>> serial8250_rpm_put(up);
> >>>
> >>> + if (up->gpios)
> >>> + return mctrl_gpio_get(up->gpios, &val);
> >>> +
> >>
> >> What happens when you have a mixed setup i.e. CTS controlled by UART
> >> but other status pins controlled by GPIO? In this case CTS status
> >> won't be returned. Do I see it right?
>
> Yes, your analysis does seem to be correct. Please note that I did
> not intentionally did change it this way. I was not thinking about
> such a "mixed design".
>
> > What about something like this:
> >
> > unsigned int serial8250_do_get_mctrl(struct uart_port *port)
> > {
> > struct uart_8250_port *up = up_to_u8250p(port);
> > unsigned int status;
> > unsigned int val;
> >
> > serial8250_rpm_get(up);
> > status = serial8250_modem_status(up);
> > serial8250_rpm_put(up);
> >
> > val = serial8250_MSR_to_TIOCM(status);
> > if (up->gpios)
> > mctrl_gpio_get(up->gpios, &val);
> >
> > return val;
> > }
>
> Looks good to me, thanks. Do you have such a setup with some modem
> control signal handled via GPIO and some via the UART? Could you
> test such a change?
I already have :-)
This my DTS file:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/am335x-baltos-ir5221.dts#n38
Yegor
^ permalink raw reply
* Re: [PATCH 3/3 v6] tty/serial/8250: use mctrl_gpio helpers
From: Stefan Roese @ 2019-06-17 12:42 UTC (permalink / raw)
To: Yegor Yefremov
Cc: linux-serial, kernel list, Mika Westerberg, Andy Shevchenko,
Giulio Benetti, Greg Kroah-Hartman
In-Reply-To: <CAGm1_ksdQ5CNLGGNzHKBNKeLE3ByHvPyOkjYNoWWM+rw0q214Q@mail.gmail.com>
On 17.06.19 11:51, Yegor Yefremov wrote:
<snip>
>>> @@ -1944,11 +1948,15 @@ unsigned int serial8250_do_get_mctrl(struct uart_port *port)
>>> {
>>> struct uart_8250_port *up = up_to_u8250p(port);
>>> unsigned int status;
>>> + unsigned int val = 0;
>>>
>>> serial8250_rpm_get(up);
>>> status = serial8250_modem_status(up);
>>> serial8250_rpm_put(up);
>>>
>>> + if (up->gpios)
>>> + return mctrl_gpio_get(up->gpios, &val);
>>> +
>>
>> What happens when you have a mixed setup i.e. CTS controlled by UART
>> but other status pins controlled by GPIO? In this case CTS status
>> won't be returned. Do I see it right?
Yes, your analysis does seem to be correct. Please note that I did
not intentionally did change it this way. I was not thinking about
such a "mixed design".
> What about something like this:
>
> unsigned int serial8250_do_get_mctrl(struct uart_port *port)
> {
> struct uart_8250_port *up = up_to_u8250p(port);
> unsigned int status;
> unsigned int val;
>
> serial8250_rpm_get(up);
> status = serial8250_modem_status(up);
> serial8250_rpm_put(up);
>
> val = serial8250_MSR_to_TIOCM(status);
> if (up->gpios)
> mctrl_gpio_get(up->gpios, &val);
>
> return val;
> }
Looks good to me, thanks. Do you have such a setup with some modem
control signal handled via GPIO and some via the UART? Could you
test such a change?
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH 3/3 v6] tty/serial/8250: use mctrl_gpio helpers
From: Yegor Yefremov @ 2019-06-17 9:51 UTC (permalink / raw)
To: Stefan Roese
Cc: linux-serial, kernel list, Mika Westerberg, Andy Shevchenko,
Giulio Benetti, Greg Kroah-Hartman
In-Reply-To: <CAGm1_kuyt5ue_3CuvryXw8L0=z0Bti5BeQMA50yRYhFmffcJuQ@mail.gmail.com>
On Mon, Jun 17, 2019 at 11:40 AM Yegor Yefremov
<yegorslists@googlemail.com> wrote:
>
> On Thu, Jun 13, 2019 at 5:45 PM Stefan Roese <sr@denx.de> wrote:
> >
> > From: Yegor Yefremov <yegorslists@googlemail.com>
> >
> > This patch permits the usage for GPIOs to control
> > the CTS/RTS/DTR/DSR/DCD/RI signals.
> >
> > Changed by Stefan:
> > Only call mctrl_gpio_init(), if the device has no ACPI companion device
> > to not break existing ACPI based systems. Also only use the mctrl_gpio_
> > functions when "gpios" is available.
> >
> > Use MSR / MCR <-> TIOCM wrapper functions.
> >
> > Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Signed-off-by: Stefan Roese <sr@denx.de>
> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Giulio Benetti <giulio.benetti@micronovasrl.com>
> > Cc: Yegor Yefremov <yegorslists@googlemail.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > v6:
> > - Use newly introduced TIOCM <-> MCR/MSR wrapper functions
> > - serial8250_in_MCR(): Don't save the already read MCR bits in TIOCM
> > format but "or" them later to the GPIO MCR value
> > - Correctly use "!up->gpios" (Andy)
> > - Removed Mika's reviewed by tag (because of changes)
> >
> > v5:
> > - Dropped a few "if (up->gpios)" checks, as the mctrl_gpio_foo() API
> > handles gpios == NULL (return)
> > - 8250_omap: Changed "IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(up->gpios, ...))"
> > to "up->gpios == NULL", as mctrl_gpio_to_gpiod() does not handle
> > gpios == NULL correctly.
> >
> > v4:
> > - Added Mika's reviewed by tag
> > - Added Johan to Cc
> >
> > v3:
> > - Only call mctrl_gpio_init(), if the device has no ACPI companion device
> > to not break existing ACPI based systems, as suggested by Andy
> >
> > v2:
> > - No change
> >
> > Please note that this patch was already applied before [1]. And later
> > reverted [2] because it introduced problems on some x86 based boards
> > (ACPI GPIO related). Here a detailed description of the issue at that
> > time:
> >
> > https://lkml.org/lkml/2016/8/9/357
> > http://www.spinics.net/lists/linux-serial/msg23071.html
> >
> > This is a re-send of the original patch that was applied at that time.
> > With patch 1/2 from this series this issue should be fixed now (please
> > note that I can't test it on such an x86 platform causing these
> > problems).
> >
> > Andy (or Mika), perhaps it would be possible for you to test this
> > patch again, now with patch 1/2 of this series applied as well?
> > That would be really helpful.
> >
> > Thanks,
> > Stefan
> >
> > [1] 4ef03d328769 ("tty/serial/8250: use mctrl_gpio helpers")
> > [2] 5db4f7f80d16 ("Revert "tty/serial/8250: use mctrl_gpio helpers"")
> >
> > .../devicetree/bindings/serial/8250.txt | 19 ++++++++++++
> > drivers/tty/serial/8250/8250.h | 18 +++++++++++-
> > drivers/tty/serial/8250/8250_core.c | 17 +++++++++++
> > drivers/tty/serial/8250/8250_omap.c | 29 ++++++++++---------
> > drivers/tty/serial/8250/8250_port.c | 8 +++++
> > drivers/tty/serial/8250/Kconfig | 1 +
> > include/linux/serial_8250.h | 1 +
> > 7 files changed, 79 insertions(+), 14 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
> > index 3cba12f855b7..20d351f268ef 100644
> > --- a/Documentation/devicetree/bindings/serial/8250.txt
> > +++ b/Documentation/devicetree/bindings/serial/8250.txt
> > @@ -53,6 +53,9 @@ Optional properties:
> > programmable TX FIFO thresholds.
> > - resets : phandle + reset specifier pairs
> > - overrun-throttle-ms : how long to pause uart rx when input overrun is encountered.
> > +- {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD
> > + line respectively. It will use specified GPIO instead of the peripheral
> > + function pin for the UART feature. If unsure, don't specify this property.
> >
> > Note:
> > * fsl,ns16550:
> > @@ -74,3 +77,19 @@ Example:
> > interrupts = <10>;
> > reg-shift = <2>;
> > };
> > +
> > +Example for OMAP UART using GPIO-based modem control signals:
> > +
> > + uart4: serial@49042000 {
> > + compatible = "ti,omap3-uart";
> > + reg = <0x49042000 0x400>;
> > + interrupts = <80>;
> > + ti,hwmods = "uart4";
> > + clock-frequency = <48000000>;
> > + cts-gpios = <&gpio3 5 GPIO_ACTIVE_LOW>;
> > + rts-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
> > + dtr-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
> > + dsr-gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
> > + dcd-gpios = <&gpio1 14 GPIO_ACTIVE_LOW>;
> > + rng-gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
> > + };
> > diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> > index 793da2e510e0..75c7c5449461 100644
> > --- a/drivers/tty/serial/8250/8250.h
> > +++ b/drivers/tty/serial/8250/8250.h
> > @@ -11,6 +11,8 @@
> > #include <linux/serial_reg.h>
> > #include <linux/dmaengine.h>
> >
> > +#include "../serial_mctrl_gpio.h"
> > +
> > struct uart_8250_dma {
> > int (*tx_dma)(struct uart_8250_port *p);
> > int (*rx_dma)(struct uart_8250_port *p);
> > @@ -196,11 +198,25 @@ static inline int serial8250_MSR_to_TIOCM(int msr)
> > static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
> > {
> > serial_out(up, UART_MCR, value);
> > +
> > + if (up->gpios)
> > + mctrl_gpio_set(up->gpios, serial8250_MCR_to_TIOCM(value));
> > }
> >
> > static inline int serial8250_in_MCR(struct uart_8250_port *up)
> > {
> > - return serial_in(up, UART_MCR);
> > + int mctrl;
> > +
> > + mctrl = serial_in(up, UART_MCR);
> > +
> > + if (up->gpios) {
> > + unsigned int mctrl_gpio = 0;
> > +
> > + mctrl_gpio = mctrl_gpio_get_outputs(up->gpios, &mctrl_gpio);
> > + mctrl |= serial8250_TIOCM_to_MCR(mctrl_gpio);
> > + }
> > +
> > + return mctrl;
> > }
> >
> > #if defined(__alpha__) && !defined(CONFIG_PCI)
> > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> > index e441221e04b9..a4470771005f 100644
> > --- a/drivers/tty/serial/8250/8250_core.c
> > +++ b/drivers/tty/serial/8250/8250_core.c
> > @@ -14,6 +14,7 @@
> > * serial8250_register_8250_port() ports
> > */
> >
> > +#include <linux/acpi.h>
> > #include <linux/module.h>
> > #include <linux/moduleparam.h>
> > #include <linux/ioport.h>
> > @@ -982,6 +983,8 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
> >
> > uart = serial8250_find_match_or_unused(&up->port);
> > if (uart && uart->port.type != PORT_8250_CIR) {
> > + struct mctrl_gpios *gpios;
> > +
> > if (uart->port.dev)
> > uart_remove_one_port(&serial8250_reg, &uart->port);
> >
> > @@ -1016,6 +1019,20 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
> > if (up->port.flags & UPF_FIXED_TYPE)
> > uart->port.type = up->port.type;
> >
> > + /*
> > + * Only call mctrl_gpio_init(), if the device has no ACPI
> > + * companion device
> > + */
> > + if (!has_acpi_companion(uart->port.dev)) {
> > + gpios = mctrl_gpio_init(&uart->port, 0);
> > + if (IS_ERR(gpios)) {
> > + if (PTR_ERR(gpios) != -ENOSYS)
> > + return PTR_ERR(gpios);
> > + } else {
> > + uart->gpios = gpios;
> > + }
> > + }
> > +
> > serial8250_set_defaults(uart);
> >
> > /* Possibly override default I/O functions. */
> > diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> > index 0a8316632d75..d5bbfc8f2284 100644
> > --- a/drivers/tty/serial/8250/8250_omap.c
> > +++ b/drivers/tty/serial/8250/8250_omap.c
> > @@ -141,18 +141,20 @@ static void omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
> >
> > serial8250_do_set_mctrl(port, mctrl);
> >
> > - /*
> > - * Turn off autoRTS if RTS is lowered and restore autoRTS setting
> > - * if RTS is raised
> > - */
> > - lcr = serial_in(up, UART_LCR);
> > - serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
> > - if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
> > - priv->efr |= UART_EFR_RTS;
> > - else
> > - priv->efr &= ~UART_EFR_RTS;
> > - serial_out(up, UART_EFR, priv->efr);
> > - serial_out(up, UART_LCR, lcr);
> > + if (!up->gpios) {
> > + /*
> > + * Turn off autoRTS if RTS is lowered and restore autoRTS
> > + * setting if RTS is raised
> > + */
> > + lcr = serial_in(up, UART_LCR);
> > + serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
> > + if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
> > + priv->efr |= UART_EFR_RTS;
> > + else
> > + priv->efr &= ~UART_EFR_RTS;
> > + serial_out(up, UART_EFR, priv->efr);
> > + serial_out(up, UART_LCR, lcr);
> > + }
> > }
> >
> > /*
> > @@ -453,7 +455,8 @@ static void omap_8250_set_termios(struct uart_port *port,
> > priv->efr = 0;
> > up->port.status &= ~(UPSTAT_AUTOCTS | UPSTAT_AUTORTS | UPSTAT_AUTOXOFF);
> >
> > - if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW) {
> > + if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW &&
> > + !up->gpios) {
> > /* Enable AUTOCTS (autoRTS is enabled when RTS is raised) */
> > up->port.status |= UPSTAT_AUTOCTS | UPSTAT_AUTORTS;
> > priv->efr |= UART_EFR_CTS;
> > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> > index 47f0a8d01a57..bc4a5e7f7f63 100644
> > --- a/drivers/tty/serial/8250/8250_port.c
> > +++ b/drivers/tty/serial/8250/8250_port.c
> > @@ -1662,6 +1662,8 @@ static void serial8250_disable_ms(struct uart_port *port)
> > if (up->bugs & UART_BUG_NOMSR)
> > return;
> >
> > + mctrl_gpio_disable_ms(up->gpios);
> > +
> > up->ier &= ~UART_IER_MSI;
> > serial_port_out(port, UART_IER, up->ier);
> > }
> > @@ -1674,6 +1676,8 @@ static void serial8250_enable_ms(struct uart_port *port)
> > if (up->bugs & UART_BUG_NOMSR)
> > return;
> >
> > + mctrl_gpio_enable_ms(up->gpios);
> > +
> > up->ier |= UART_IER_MSI;
> >
> > serial8250_rpm_get(up);
> > @@ -1944,11 +1948,15 @@ unsigned int serial8250_do_get_mctrl(struct uart_port *port)
> > {
> > struct uart_8250_port *up = up_to_u8250p(port);
> > unsigned int status;
> > + unsigned int val = 0;
> >
> > serial8250_rpm_get(up);
> > status = serial8250_modem_status(up);
> > serial8250_rpm_put(up);
> >
> > + if (up->gpios)
> > + return mctrl_gpio_get(up->gpios, &val);
> > +
>
> What happens when you have a mixed setup i.e. CTS controlled by UART
> but other status pins controlled by GPIO? In this case CTS status
> won't be returned. Do I see it right?
What about something like this:
unsigned int serial8250_do_get_mctrl(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
unsigned int status;
unsigned int val;
serial8250_rpm_get(up);
status = serial8250_modem_status(up);
serial8250_rpm_put(up);
val = serial8250_MSR_to_TIOCM(status);
if (up->gpios)
mctrl_gpio_get(up->gpios, &val);
return val;
}
> Yegor
>
> > return serial8250_MSR_to_TIOCM(status);
> > }
> > EXPORT_SYMBOL_GPL(serial8250_do_get_mctrl);
> > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> > index 296115f6a4d8..509f6a3bb9ff 100644
> > --- a/drivers/tty/serial/8250/Kconfig
> > +++ b/drivers/tty/serial/8250/Kconfig
> > @@ -8,6 +8,7 @@ config SERIAL_8250
> > tristate "8250/16550 and compatible serial support"
> > depends on !S390
> > select SERIAL_CORE
> > + select SERIAL_MCTRL_GPIO if GPIOLIB
> > ---help---
> > This selects whether you want to include the driver for the standard
> > serial ports. The standard answer is Y. People who might say N
> > diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> > index 5e0b59422a68..bb2bc99388ca 100644
> > --- a/include/linux/serial_8250.h
> > +++ b/include/linux/serial_8250.h
> > @@ -110,6 +110,7 @@ struct uart_8250_port {
> > * if no_console_suspend
> > */
> > unsigned char probe;
> > + struct mctrl_gpios *gpios;
> > #define UART_PROBE_RSA (1 << 0)
> >
> > /*
> > --
> > 2.22.0
> >
^ permalink raw reply
* Re: [PATCH 3/3 v6] tty/serial/8250: use mctrl_gpio helpers
From: Yegor Yefremov @ 2019-06-17 9:40 UTC (permalink / raw)
To: Stefan Roese
Cc: linux-serial, kernel list, Mika Westerberg, Andy Shevchenko,
Giulio Benetti, Greg Kroah-Hartman
In-Reply-To: <20190613154542.32438-3-sr@denx.de>
On Thu, Jun 13, 2019 at 5:45 PM Stefan Roese <sr@denx.de> wrote:
>
> From: Yegor Yefremov <yegorslists@googlemail.com>
>
> This patch permits the usage for GPIOs to control
> the CTS/RTS/DTR/DSR/DCD/RI signals.
>
> Changed by Stefan:
> Only call mctrl_gpio_init(), if the device has no ACPI companion device
> to not break existing ACPI based systems. Also only use the mctrl_gpio_
> functions when "gpios" is available.
>
> Use MSR / MCR <-> TIOCM wrapper functions.
>
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Giulio Benetti <giulio.benetti@micronovasrl.com>
> Cc: Yegor Yefremov <yegorslists@googlemail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> v6:
> - Use newly introduced TIOCM <-> MCR/MSR wrapper functions
> - serial8250_in_MCR(): Don't save the already read MCR bits in TIOCM
> format but "or" them later to the GPIO MCR value
> - Correctly use "!up->gpios" (Andy)
> - Removed Mika's reviewed by tag (because of changes)
>
> v5:
> - Dropped a few "if (up->gpios)" checks, as the mctrl_gpio_foo() API
> handles gpios == NULL (return)
> - 8250_omap: Changed "IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(up->gpios, ...))"
> to "up->gpios == NULL", as mctrl_gpio_to_gpiod() does not handle
> gpios == NULL correctly.
>
> v4:
> - Added Mika's reviewed by tag
> - Added Johan to Cc
>
> v3:
> - Only call mctrl_gpio_init(), if the device has no ACPI companion device
> to not break existing ACPI based systems, as suggested by Andy
>
> v2:
> - No change
>
> Please note that this patch was already applied before [1]. And later
> reverted [2] because it introduced problems on some x86 based boards
> (ACPI GPIO related). Here a detailed description of the issue at that
> time:
>
> https://lkml.org/lkml/2016/8/9/357
> http://www.spinics.net/lists/linux-serial/msg23071.html
>
> This is a re-send of the original patch that was applied at that time.
> With patch 1/2 from this series this issue should be fixed now (please
> note that I can't test it on such an x86 platform causing these
> problems).
>
> Andy (or Mika), perhaps it would be possible for you to test this
> patch again, now with patch 1/2 of this series applied as well?
> That would be really helpful.
>
> Thanks,
> Stefan
>
> [1] 4ef03d328769 ("tty/serial/8250: use mctrl_gpio helpers")
> [2] 5db4f7f80d16 ("Revert "tty/serial/8250: use mctrl_gpio helpers"")
>
> .../devicetree/bindings/serial/8250.txt | 19 ++++++++++++
> drivers/tty/serial/8250/8250.h | 18 +++++++++++-
> drivers/tty/serial/8250/8250_core.c | 17 +++++++++++
> drivers/tty/serial/8250/8250_omap.c | 29 ++++++++++---------
> drivers/tty/serial/8250/8250_port.c | 8 +++++
> drivers/tty/serial/8250/Kconfig | 1 +
> include/linux/serial_8250.h | 1 +
> 7 files changed, 79 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
> index 3cba12f855b7..20d351f268ef 100644
> --- a/Documentation/devicetree/bindings/serial/8250.txt
> +++ b/Documentation/devicetree/bindings/serial/8250.txt
> @@ -53,6 +53,9 @@ Optional properties:
> programmable TX FIFO thresholds.
> - resets : phandle + reset specifier pairs
> - overrun-throttle-ms : how long to pause uart rx when input overrun is encountered.
> +- {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD
> + line respectively. It will use specified GPIO instead of the peripheral
> + function pin for the UART feature. If unsure, don't specify this property.
>
> Note:
> * fsl,ns16550:
> @@ -74,3 +77,19 @@ Example:
> interrupts = <10>;
> reg-shift = <2>;
> };
> +
> +Example for OMAP UART using GPIO-based modem control signals:
> +
> + uart4: serial@49042000 {
> + compatible = "ti,omap3-uart";
> + reg = <0x49042000 0x400>;
> + interrupts = <80>;
> + ti,hwmods = "uart4";
> + clock-frequency = <48000000>;
> + cts-gpios = <&gpio3 5 GPIO_ACTIVE_LOW>;
> + rts-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
> + dtr-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
> + dsr-gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
> + dcd-gpios = <&gpio1 14 GPIO_ACTIVE_LOW>;
> + rng-gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
> + };
> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> index 793da2e510e0..75c7c5449461 100644
> --- a/drivers/tty/serial/8250/8250.h
> +++ b/drivers/tty/serial/8250/8250.h
> @@ -11,6 +11,8 @@
> #include <linux/serial_reg.h>
> #include <linux/dmaengine.h>
>
> +#include "../serial_mctrl_gpio.h"
> +
> struct uart_8250_dma {
> int (*tx_dma)(struct uart_8250_port *p);
> int (*rx_dma)(struct uart_8250_port *p);
> @@ -196,11 +198,25 @@ static inline int serial8250_MSR_to_TIOCM(int msr)
> static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
> {
> serial_out(up, UART_MCR, value);
> +
> + if (up->gpios)
> + mctrl_gpio_set(up->gpios, serial8250_MCR_to_TIOCM(value));
> }
>
> static inline int serial8250_in_MCR(struct uart_8250_port *up)
> {
> - return serial_in(up, UART_MCR);
> + int mctrl;
> +
> + mctrl = serial_in(up, UART_MCR);
> +
> + if (up->gpios) {
> + unsigned int mctrl_gpio = 0;
> +
> + mctrl_gpio = mctrl_gpio_get_outputs(up->gpios, &mctrl_gpio);
> + mctrl |= serial8250_TIOCM_to_MCR(mctrl_gpio);
> + }
> +
> + return mctrl;
> }
>
> #if defined(__alpha__) && !defined(CONFIG_PCI)
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index e441221e04b9..a4470771005f 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -14,6 +14,7 @@
> * serial8250_register_8250_port() ports
> */
>
> +#include <linux/acpi.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> #include <linux/ioport.h>
> @@ -982,6 +983,8 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
>
> uart = serial8250_find_match_or_unused(&up->port);
> if (uart && uart->port.type != PORT_8250_CIR) {
> + struct mctrl_gpios *gpios;
> +
> if (uart->port.dev)
> uart_remove_one_port(&serial8250_reg, &uart->port);
>
> @@ -1016,6 +1019,20 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
> if (up->port.flags & UPF_FIXED_TYPE)
> uart->port.type = up->port.type;
>
> + /*
> + * Only call mctrl_gpio_init(), if the device has no ACPI
> + * companion device
> + */
> + if (!has_acpi_companion(uart->port.dev)) {
> + gpios = mctrl_gpio_init(&uart->port, 0);
> + if (IS_ERR(gpios)) {
> + if (PTR_ERR(gpios) != -ENOSYS)
> + return PTR_ERR(gpios);
> + } else {
> + uart->gpios = gpios;
> + }
> + }
> +
> serial8250_set_defaults(uart);
>
> /* Possibly override default I/O functions. */
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 0a8316632d75..d5bbfc8f2284 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -141,18 +141,20 @@ static void omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
>
> serial8250_do_set_mctrl(port, mctrl);
>
> - /*
> - * Turn off autoRTS if RTS is lowered and restore autoRTS setting
> - * if RTS is raised
> - */
> - lcr = serial_in(up, UART_LCR);
> - serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
> - if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
> - priv->efr |= UART_EFR_RTS;
> - else
> - priv->efr &= ~UART_EFR_RTS;
> - serial_out(up, UART_EFR, priv->efr);
> - serial_out(up, UART_LCR, lcr);
> + if (!up->gpios) {
> + /*
> + * Turn off autoRTS if RTS is lowered and restore autoRTS
> + * setting if RTS is raised
> + */
> + lcr = serial_in(up, UART_LCR);
> + serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
> + if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
> + priv->efr |= UART_EFR_RTS;
> + else
> + priv->efr &= ~UART_EFR_RTS;
> + serial_out(up, UART_EFR, priv->efr);
> + serial_out(up, UART_LCR, lcr);
> + }
> }
>
> /*
> @@ -453,7 +455,8 @@ static void omap_8250_set_termios(struct uart_port *port,
> priv->efr = 0;
> up->port.status &= ~(UPSTAT_AUTOCTS | UPSTAT_AUTORTS | UPSTAT_AUTOXOFF);
>
> - if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW) {
> + if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW &&
> + !up->gpios) {
> /* Enable AUTOCTS (autoRTS is enabled when RTS is raised) */
> up->port.status |= UPSTAT_AUTOCTS | UPSTAT_AUTORTS;
> priv->efr |= UART_EFR_CTS;
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 47f0a8d01a57..bc4a5e7f7f63 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1662,6 +1662,8 @@ static void serial8250_disable_ms(struct uart_port *port)
> if (up->bugs & UART_BUG_NOMSR)
> return;
>
> + mctrl_gpio_disable_ms(up->gpios);
> +
> up->ier &= ~UART_IER_MSI;
> serial_port_out(port, UART_IER, up->ier);
> }
> @@ -1674,6 +1676,8 @@ static void serial8250_enable_ms(struct uart_port *port)
> if (up->bugs & UART_BUG_NOMSR)
> return;
>
> + mctrl_gpio_enable_ms(up->gpios);
> +
> up->ier |= UART_IER_MSI;
>
> serial8250_rpm_get(up);
> @@ -1944,11 +1948,15 @@ unsigned int serial8250_do_get_mctrl(struct uart_port *port)
> {
> struct uart_8250_port *up = up_to_u8250p(port);
> unsigned int status;
> + unsigned int val = 0;
>
> serial8250_rpm_get(up);
> status = serial8250_modem_status(up);
> serial8250_rpm_put(up);
>
> + if (up->gpios)
> + return mctrl_gpio_get(up->gpios, &val);
> +
What happens when you have a mixed setup i.e. CTS controlled by UART
but other status pins controlled by GPIO? In this case CTS status
won't be returned. Do I see it right?
Yegor
> return serial8250_MSR_to_TIOCM(status);
> }
> EXPORT_SYMBOL_GPL(serial8250_do_get_mctrl);
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 296115f6a4d8..509f6a3bb9ff 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -8,6 +8,7 @@ config SERIAL_8250
> tristate "8250/16550 and compatible serial support"
> depends on !S390
> select SERIAL_CORE
> + select SERIAL_MCTRL_GPIO if GPIOLIB
> ---help---
> This selects whether you want to include the driver for the standard
> serial ports. The standard answer is Y. People who might say N
> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> index 5e0b59422a68..bb2bc99388ca 100644
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -110,6 +110,7 @@ struct uart_8250_port {
> * if no_console_suspend
> */
> unsigned char probe;
> + struct mctrl_gpios *gpios;
> #define UART_PROBE_RSA (1 << 0)
>
> /*
> --
> 2.22.0
>
^ permalink raw reply
* Re: [RFC v2 00/11] DVFS in the OPP core
From: Viresh Kumar @ 2019-06-17 4:26 UTC (permalink / raw)
To: Rajendra Nayak
Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
dri-devel, linux-scsi, swboyd, ulf.hansson, dianders, rafael
In-Reply-To: <20190320094918.20234-1-rnayak@codeaurora.org>
On 20-03-19, 15:19, Rajendra Nayak wrote:
> This is a v2 of the RFC posted earlier by Stephen Boyd [1]
>
> As part of v2 I still follow the same approach of dev_pm_opp_set_rate()
> API using clk framework to round the frequency passed and making it
> accept 0 as a valid frequency indicating the frequency isn't required
> anymore. It just has a few more drivers converted to use this approach
> like dsi/dpu and ufs.
> ufs demonstrates the case of having to handle multiple power domains, one
> of which is scalable.
>
> The patches are based on 5.1-rc1 and depend on some ufs fixes I posted
> earlier [2] and a DT patch to include the rpmpd header [3]
>
> [1] https://lkml.org/lkml/2019/1/28/2086
> [2] https://lkml.org/lkml/2019/3/8/70
> [3] https://lkml.org/lkml/2019/3/20/120
>
> Rajendra Nayak (10):
> OPP: Make dev_pm_opp_set_rate() with freq=0 as valid
>
> Stephen Boyd (1):
> OPP: Don't overwrite rounded clk rate
I have applied modified version of these two patches to the OPP tree now.
Thanks.
--
viresh
^ permalink raw reply
* Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
From: Rajendra Nayak @ 2019-06-17 4:25 UTC (permalink / raw)
To: Viresh Kumar
Cc: swboyd, vincent.guittot, mturquette, linux-kernel, linux-arm-msm,
linux-pm, linux-serial, linux-spi, dri-devel, linux-scsi,
ulf.hansson, dianders, rafael
In-Reply-To: <20190617041721.5xdr3kl4xxe6gy4m@vireshk-i7>
On 6/17/2019 9:47 AM, Viresh Kumar wrote:
> On 17-06-19, 09:37, Rajendra Nayak wrote:
>>
>>
>> On 6/17/2019 9:20 AM, Viresh Kumar wrote:
>>> On 14-06-19, 10:57, Viresh Kumar wrote:
>>>> Hmm, so this patch won't break anything and I am inclined to apply it again :)
>>>>
>>>> Does anyone see any other issues with it, which I might be missing ?
>>>
>>> I have updated the commit log a bit more to clarify on things, please let me
>>> know if it looks okay.
>>>
>>> opp: Don't overwrite rounded clk rate
>>> The OPP table normally contains 'fmax' values corresponding to the
>>> voltage or performance levels of each OPP, but we don't necessarily want
>>> all the devices to run at fmax all the time. Running at fmax makes sense
>>> for devices like CPU/GPU, which have a finite amount of work to do and
>>> since a specific amount of energy is consumed at an OPP, its better to
>>> run at the highest possible frequency for that voltage value.
>>> On the other hand, we have IO devices which need to run at specific
>>> frequencies only for their proper functioning, instead of maximum
>>> possible frequency.
>>> The OPP core currently roundup to the next possible OPP for a frequency
>>> and select the fmax value. To support the IO devices by the OPP core,
>>> lets do the roundup to fetch the voltage or performance state values,
>>> but not use the OPP frequency value. Rather use the value returned by
>>> clk_round_rate().
>>> The current user, cpufreq, of dev_pm_opp_set_rate() already does the
>>> rounding to the next OPP before calling this routine and it won't
>>> have any side affects because of this change.
>>
>> Looks good to me. Should this also be documented someplace that dev_pm_opp_set_rate()
>> would not be able to distinguish between its users trying to scale CPU/GPU's vs IO
>> devices, so its the callers responsibility to round it accordingly before calling the
>> API?
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 0fbc77f05048..bae94bfa1e96 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -751,8 +751,11 @@ static int _set_required_opps(struct device *dev,
> * @dev: device for which we do this operation
> * @target_freq: frequency to achieve
> *
> - * This configures the power-supplies and clock source to the levels specified
> - * by the OPP corresponding to the target_freq.
> + * This configures the power-supplies to the levels specified by the OPP
> + * corresponding to the target_freq, and programs the clock to a value <=
> + * target_freq, as rounded by clk_round_rate(). Device wanting to run at fmax
> + * provided by the opp, should have already rounded to the target OPP's
> + * frequency.
> */
Perfect, thanks.
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
From: Viresh Kumar @ 2019-06-17 4:17 UTC (permalink / raw)
To: Rajendra Nayak
Cc: swboyd, vincent.guittot, mturquette, linux-kernel, linux-arm-msm,
linux-pm, linux-serial, linux-spi, dri-devel, linux-scsi,
ulf.hansson, dianders, rafael
In-Reply-To: <a912c8b2-080d-7ab7-670b-b687ec3a2c92@codeaurora.org>
On 17-06-19, 09:37, Rajendra Nayak wrote:
>
>
> On 6/17/2019 9:20 AM, Viresh Kumar wrote:
> > On 14-06-19, 10:57, Viresh Kumar wrote:
> > > Hmm, so this patch won't break anything and I am inclined to apply it again :)
> > >
> > > Does anyone see any other issues with it, which I might be missing ?
> >
> > I have updated the commit log a bit more to clarify on things, please let me
> > know if it looks okay.
> >
> > opp: Don't overwrite rounded clk rate
> > The OPP table normally contains 'fmax' values corresponding to the
> > voltage or performance levels of each OPP, but we don't necessarily want
> > all the devices to run at fmax all the time. Running at fmax makes sense
> > for devices like CPU/GPU, which have a finite amount of work to do and
> > since a specific amount of energy is consumed at an OPP, its better to
> > run at the highest possible frequency for that voltage value.
> > On the other hand, we have IO devices which need to run at specific
> > frequencies only for their proper functioning, instead of maximum
> > possible frequency.
> > The OPP core currently roundup to the next possible OPP for a frequency
> > and select the fmax value. To support the IO devices by the OPP core,
> > lets do the roundup to fetch the voltage or performance state values,
> > but not use the OPP frequency value. Rather use the value returned by
> > clk_round_rate().
> > The current user, cpufreq, of dev_pm_opp_set_rate() already does the
> > rounding to the next OPP before calling this routine and it won't
> > have any side affects because of this change.
>
> Looks good to me. Should this also be documented someplace that dev_pm_opp_set_rate()
> would not be able to distinguish between its users trying to scale CPU/GPU's vs IO
> devices, so its the callers responsibility to round it accordingly before calling the
> API?
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 0fbc77f05048..bae94bfa1e96 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -751,8 +751,11 @@ static int _set_required_opps(struct device *dev,
* @dev: device for which we do this operation
* @target_freq: frequency to achieve
*
- * This configures the power-supplies and clock source to the levels specified
- * by the OPP corresponding to the target_freq.
+ * This configures the power-supplies to the levels specified by the OPP
+ * corresponding to the target_freq, and programs the clock to a value <=
+ * target_freq, as rounded by clk_round_rate(). Device wanting to run at fmax
+ * provided by the opp, should have already rounded to the target OPP's
+ * frequency.
*/
--
viresh
^ permalink raw reply related
* Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
From: Rajendra Nayak @ 2019-06-17 4:07 UTC (permalink / raw)
To: Viresh Kumar, swboyd, vincent.guittot, mturquette
Cc: ulf.hansson, dianders, linux-scsi, linux-pm, linux-arm-msm,
rafael, linux-kernel, dri-devel, linux-spi, linux-serial
In-Reply-To: <20190617035058.veo7uwqjrpa6kykt@vireshk-i7>
On 6/17/2019 9:20 AM, Viresh Kumar wrote:
> On 14-06-19, 10:57, Viresh Kumar wrote:
>> Hmm, so this patch won't break anything and I am inclined to apply it again :)
>>
>> Does anyone see any other issues with it, which I might be missing ?
>
> I have updated the commit log a bit more to clarify on things, please let me
> know if it looks okay.
>
> opp: Don't overwrite rounded clk rate
>
> The OPP table normally contains 'fmax' values corresponding to the
> voltage or performance levels of each OPP, but we don't necessarily want
> all the devices to run at fmax all the time. Running at fmax makes sense
> for devices like CPU/GPU, which have a finite amount of work to do and
> since a specific amount of energy is consumed at an OPP, its better to
> run at the highest possible frequency for that voltage value.
>
> On the other hand, we have IO devices which need to run at specific
> frequencies only for their proper functioning, instead of maximum
> possible frequency.
>
> The OPP core currently roundup to the next possible OPP for a frequency
> and select the fmax value. To support the IO devices by the OPP core,
> lets do the roundup to fetch the voltage or performance state values,
> but not use the OPP frequency value. Rather use the value returned by
> clk_round_rate().
>
> The current user, cpufreq, of dev_pm_opp_set_rate() already does the
> rounding to the next OPP before calling this routine and it won't
> have any side affects because of this change.
Looks good to me. Should this also be documented someplace that dev_pm_opp_set_rate()
would not be able to distinguish between its users trying to scale CPU/GPU's vs IO
devices, so its the callers responsibility to round it accordingly before calling the
API?
>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> [ Viresh: Massaged changelog and use temp_opp variable instead ]
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [RFC v2 02/11] OPP: Make dev_pm_opp_set_rate() with freq=0 as valid
From: Rajendra Nayak @ 2019-06-17 4:04 UTC (permalink / raw)
To: Viresh Kumar
Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
dri-devel, linux-scsi, swboyd, ulf.hansson, dianders, rafael
In-Reply-To: <20190614063210.lfsquoycronah3fe@vireshk-i7>
On 6/14/2019 12:02 PM, Viresh Kumar wrote:
> On 20-03-19, 15:19, Rajendra Nayak wrote:
>> For devices with performance state, we use dev_pm_opp_set_rate()
>> to set the appropriate clk rate and the performance state.
>> We do need a way to *remove* the performance state vote when
>> we idle the device and turn the clocks off. Use dev_pm_opp_set_rate()
>> with freq=0 to achieve this.
>>
>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>> ---
>> drivers/opp/core.c | 18 ++++++++++++------
>> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> What about this instead ?
yes, this works, thanks for updating the patch.
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 2fe96c2363a3..9accf8bb6afc 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -711,7 +711,7 @@ static int _set_required_opps(struct device *dev,
>
> /* Single genpd case */
> if (!genpd_virt_devs) {
> - pstate = opp->required_opps[0]->pstate;
> + pstate = likely(opp) ? opp->required_opps[0]->pstate : 0;
> ret = dev_pm_genpd_set_performance_state(dev, pstate);
> if (ret) {
> dev_err(dev, "Failed to set performance state of %s: %d (%d)\n",
> @@ -729,7 +729,7 @@ static int _set_required_opps(struct device *dev,
> mutex_lock(&opp_table->genpd_virt_dev_lock);
>
> for (i = 0; i < opp_table->required_opp_count; i++) {
> - pstate = opp->required_opps[i]->pstate;
> + pstate = likely(opp) ? opp->required_opps[i]->pstate : 0;
>
> if (!genpd_virt_devs[i])
> continue;
> @@ -770,14 +770,13 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
>
> if (unlikely(!target_freq)) {
> if (opp_table->required_opp_tables) {
> - /* drop the performance state vote */
> - dev_pm_genpd_set_performance_state(dev, 0);
> - return 0;
> + ret = _set_required_opps(dev, opp_table, NULL);
> } else {
> - dev_err(dev, "%s: Invalid target frequency %lu\n", __func__,
> - target_freq);
> - return -EINVAL;
> + dev_err(dev, "target frequency can't be 0\n");
> + ret = -EINVAL;
> }
> +
> + goto put_opp_table;
> }
>
> clk = opp_table->clk;
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
From: Viresh Kumar @ 2019-06-17 3:50 UTC (permalink / raw)
To: swboyd, Rajendra Nayak, vincent.guittot, mturquette
Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
dri-devel, linux-scsi, ulf.hansson, dianders, rafael
In-Reply-To: <20190614052732.4w6vvwwich2h4cgu@vireshk-i7>
On 14-06-19, 10:57, Viresh Kumar wrote:
> Hmm, so this patch won't break anything and I am inclined to apply it again :)
>
> Does anyone see any other issues with it, which I might be missing ?
I have updated the commit log a bit more to clarify on things, please let me
know if it looks okay.
opp: Don't overwrite rounded clk rate
The OPP table normally contains 'fmax' values corresponding to the
voltage or performance levels of each OPP, but we don't necessarily want
all the devices to run at fmax all the time. Running at fmax makes sense
for devices like CPU/GPU, which have a finite amount of work to do and
since a specific amount of energy is consumed at an OPP, its better to
run at the highest possible frequency for that voltage value.
On the other hand, we have IO devices which need to run at specific
frequencies only for their proper functioning, instead of maximum
possible frequency.
The OPP core currently roundup to the next possible OPP for a frequency
and select the fmax value. To support the IO devices by the OPP core,
lets do the roundup to fetch the voltage or performance state values,
but not use the OPP frequency value. Rather use the value returned by
clk_round_rate().
The current user, cpufreq, of dev_pm_opp_set_rate() already does the
rounding to the next OPP before calling this routine and it won't
have any side affects because of this change.
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
[ Viresh: Massaged changelog and use temp_opp variable instead ]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* [PATCH] tty: serial_core: recover uport->cons->cflag on uart_close
From: kpark3469 @ 2019-06-16 14:57 UTC (permalink / raw)
To: gregkh; +Cc: jslaby, linux-serial, linux-kernel, keun-o.park
From: Sahara <keun-o.park@darkmatter.ae>
Since uart_close was converted to use tty_port_close, uart_shutdown
also moved to uart_tty_port_shutdown, which means it does not backup
tty's termios to uart_port.console.cflag when console is closed and
uart_console is true.
By losing this value, serial console was not set correctly especially
after suspend/resume when there is no consumer of console device.
This problem resets console driver's configuration to an unwanted value
and may give a performance regression in the system eventually.
This patch fixes the bug introduced from v4.9 kernel.
Fixes: 761ed4a94582 ("tty: serial_core: convert uart_close to use tty_port_close")
Reported-by: Jouni Linnamaa <Jouni.Linnamaa@darkmatter.ae>
Signed-off-by: Sahara <keun-o.park@darkmatter.ae>
---
drivers/tty/serial/serial_core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 83f4dd0bfd74..a52afceb2f4e 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1533,6 +1533,7 @@ static void uart_set_termios(struct tty_struct *tty,
static void uart_close(struct tty_struct *tty, struct file *filp)
{
struct uart_state *state = tty->driver_data;
+ struct uart_port *uport = uart_port_check(state);
if (!state) {
struct uart_driver *drv = tty->driver->driver_state;
@@ -1548,6 +1549,8 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
pr_debug("uart_close(%d) called\n", tty->index);
+ if (uport && uart_console(uport))
+ uport->cons->cflag = tty->termios.c_cflag;
tty_port_close(tty->port, tty, filp);
}
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox