cip-dev.lists.cip-project.org archive mirror
 help / color / mirror / Atom feed
* [cip-dev] [PATCH 4.4.y-cip 0/4] serial mctrl_gpio/sh-sci Fixes
@ 2020-09-03 14:27 Lad Prabhakar
  2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 1/4] serial: mctrl_gpio: Check if GPIO property exisits before requesting it Lad Prabhakar
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Lad Prabhakar @ 2020-09-03 14:27 UTC (permalink / raw)
  To: cip-dev, Nobuhiro Iwamatsu, Pavel Machek; +Cc: Biju Das

[-- Attachment #1: Type: text/plain, Size: 707 bytes --]

Hi All,

This patch series adds minor fixes to sh-sci and serial_mctrl_gpio driver.
All patches in this series are cherry-picked from mainline.

Cheers,
Prabhakar


Frieder Schrempf (1):
  serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS

Geert Uytterhoeven (2):
  serial: sh-sci: Terminate TX DMA during buffer flushing
  serial: sh-sci: Don't check for mctrl_gpio_to_gpiod() returning error

Stefan Roese (1):
  serial: mctrl_gpio: Check if GPIO property exisits before requesting
    it

 drivers/tty/serial/serial_mctrl_gpio.c | 15 +++++++++++++++
 drivers/tty/serial/sh-sci.c            | 25 +++++++++++++++----------
 2 files changed, 30 insertions(+), 10 deletions(-)

-- 
2.17.1


[-- Attachment #2: Type: text/plain, Size: 419 bytes --]

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#5390): https://lists.cip-project.org/g/cip-dev/message/5390
Mute This Topic: https://lists.cip-project.org/mt/76607960/4520388
Group Owner: cip-dev+owner@lists.cip-project.org
Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy  [cip-dev@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [cip-dev] [PATCH 4.4.y-cip 1/4] serial: mctrl_gpio: Check if GPIO property exisits before requesting it
  2020-09-03 14:27 [cip-dev] [PATCH 4.4.y-cip 0/4] serial mctrl_gpio/sh-sci Fixes Lad Prabhakar
@ 2020-09-03 14:27 ` Lad Prabhakar
  2020-09-03 20:43   ` Pavel Machek
  2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 2/4] serial: sh-sci: Terminate TX DMA during buffer flushing Lad Prabhakar
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Lad Prabhakar @ 2020-09-03 14:27 UTC (permalink / raw)
  To: cip-dev, Nobuhiro Iwamatsu, Pavel Machek; +Cc: Biju Das

[-- Attachment #1: Type: text/plain, Size: 3884 bytes --]

From: Stefan Roese <sr@denx.de>

commit d99482673f950817b30caf3fcdfb31179b050ce1 upstream.

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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[PL: Included slab.h for build error]
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/tty/serial/serial_mctrl_gpio.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index 2b5329a3d716..18dfba725239 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -20,7 +20,9 @@
 #include <linux/gpio/consumer.h>
 #include <linux/termios.h>
 #include <linux/serial_core.h>
+#include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/property.h>
 
 #include "serial_mctrl_gpio.h"
 
@@ -102,6 +104,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.17.1


[-- Attachment #2: Type: text/plain, Size: 419 bytes --]

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#5392): https://lists.cip-project.org/g/cip-dev/message/5392
Mute This Topic: https://lists.cip-project.org/mt/76607963/4520388
Group Owner: cip-dev+owner@lists.cip-project.org
Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy  [cip-dev@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [cip-dev] [PATCH 4.4.y-cip 2/4] serial: sh-sci: Terminate TX DMA during buffer flushing
  2020-09-03 14:27 [cip-dev] [PATCH 4.4.y-cip 0/4] serial mctrl_gpio/sh-sci Fixes Lad Prabhakar
  2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 1/4] serial: mctrl_gpio: Check if GPIO property exisits before requesting it Lad Prabhakar
@ 2020-09-03 14:27 ` Lad Prabhakar
  2020-09-03 20:46   ` Pavel Machek
  2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 3/4] serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS Lad Prabhakar
  2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 4/4] serial: sh-sci: Don't check for mctrl_gpio_to_gpiod() returning error Lad Prabhakar
  3 siblings, 1 reply; 9+ messages in thread
From: Lad Prabhakar @ 2020-09-03 14:27 UTC (permalink / raw)
  To: cip-dev, Nobuhiro Iwamatsu, Pavel Machek; +Cc: Biju Das

[-- Attachment #1: Type: text/plain, Size: 1766 bytes --]

From: Geert Uytterhoeven <geert+renesas@glider.be>

commit 775b7ffd7d6d5db320d99b0a485c51e04dfcf9f1 upstream.

While the .flush_buffer() callback clears sci_port.tx_dma_len since
commit 1cf4a7efdc71cab8 ("serial: sh-sci: Fix race condition causing
garbage during shutdown"), it does not terminate a transmit DMA
operation that may be in progress.

Fix this by terminating any pending DMA operations, and resetting the
corresponding cookie.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com>

Link: https://lore.kernel.org/r/20190624123540.20629-3-geert+renesas@glider.be
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/tty/serial/sh-sci.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 2b5b342b7462..166ca04aad4e 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1475,11 +1475,18 @@ static void sci_free_dma(struct uart_port *port)
 
 static void sci_flush_buffer(struct uart_port *port)
 {
+	struct sci_port *s = to_sci_port(port);
+
 	/*
 	 * In uart_flush_buffer(), the xmit circular buffer has just been
-	 * cleared, so we have to reset tx_dma_len accordingly.
+	 * cleared, so we have to reset tx_dma_len accordingly, and stop any
+	 * pending transfers
 	 */
-	to_sci_port(port)->tx_dma_len = 0;
+	s->tx_dma_len = 0;
+	if (s->chan_tx) {
+		dmaengine_terminate_async(s->chan_tx);
+		s->cookie_tx = -EINVAL;
+	}
 }
 #else /* !CONFIG_SERIAL_SH_SCI_DMA */
 static inline void sci_request_dma(struct uart_port *port)
-- 
2.17.1


[-- Attachment #2: Type: text/plain, Size: 419 bytes --]

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#5391): https://lists.cip-project.org/g/cip-dev/message/5391
Mute This Topic: https://lists.cip-project.org/mt/76607962/4520388
Group Owner: cip-dev+owner@lists.cip-project.org
Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy  [cip-dev@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [cip-dev] [PATCH 4.4.y-cip 3/4] serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS
  2020-09-03 14:27 [cip-dev] [PATCH 4.4.y-cip 0/4] serial mctrl_gpio/sh-sci Fixes Lad Prabhakar
  2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 1/4] serial: mctrl_gpio: Check if GPIO property exisits before requesting it Lad Prabhakar
  2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 2/4] serial: sh-sci: Terminate TX DMA during buffer flushing Lad Prabhakar
@ 2020-09-03 14:27 ` Lad Prabhakar
  2020-09-03 20:51   ` Pavel Machek
  2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 4/4] serial: sh-sci: Don't check for mctrl_gpio_to_gpiod() returning error Lad Prabhakar
  3 siblings, 1 reply; 9+ messages in thread
From: Lad Prabhakar @ 2020-09-03 14:27 UTC (permalink / raw)
  To: cip-dev, Nobuhiro Iwamatsu, Pavel Machek; +Cc: Biju Das

[-- Attachment #1: Type: text/plain, Size: 1194 bytes --]

From: Frieder Schrempf <frieder.schrempf@kontron.de>

commit e55a09732be9b4e13cf3b5d2b9bb41b3e60e5ea6 upstream.

Now that the mctrl_gpio code returns NULL instead of ERR_PTR(-ENOSYS)
if CONFIG_GPIOLIB is disabled, we can safely remove this check.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Acked-by: Uwe Kleine-Knig <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20190802100349.8659-3-frieder.schrempf@kontron.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/tty/serial/sh-sci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 166ca04aad4e..7d7008e4dc75 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2778,7 +2778,7 @@ static int sci_probe_single(struct platform_device *dev,
 		return ret;
 
 	sciport->gpios = mctrl_gpio_init(&sciport->port, 0);
-	if (IS_ERR(sciport->gpios) && PTR_ERR(sciport->gpios) != -ENOSYS)
+	if (IS_ERR(sciport->gpios))
 		return PTR_ERR(sciport->gpios);
 
 	if (p->capabilities & SCIx_HAVE_RTSCTS) {
-- 
2.17.1


[-- Attachment #2: Type: text/plain, Size: 419 bytes --]

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#5393): https://lists.cip-project.org/g/cip-dev/message/5393
Mute This Topic: https://lists.cip-project.org/mt/76607964/4520388
Group Owner: cip-dev+owner@lists.cip-project.org
Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy  [cip-dev@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [cip-dev] [PATCH 4.4.y-cip 4/4] serial: sh-sci: Don't check for mctrl_gpio_to_gpiod() returning error
  2020-09-03 14:27 [cip-dev] [PATCH 4.4.y-cip 0/4] serial mctrl_gpio/sh-sci Fixes Lad Prabhakar
                   ` (2 preceding siblings ...)
  2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 3/4] serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS Lad Prabhakar
@ 2020-09-03 14:27 ` Lad Prabhakar
  2020-09-03 20:54   ` Pavel Machek
  3 siblings, 1 reply; 9+ messages in thread
From: Lad Prabhakar @ 2020-09-03 14:27 UTC (permalink / raw)
  To: cip-dev, Nobuhiro Iwamatsu, Pavel Machek; +Cc: Biju Das

[-- Attachment #1: Type: text/plain, Size: 2435 bytes --]

From: Geert Uytterhoeven <geert+renesas@glider.be>

commit a16c4c5a9cb6368a08c457b6b2dc0be25958dfc0 upstream.

Since commit 1d267ea6539f2663 ("serial: mctrl-gpio: simplify init
routine"), mctrl_gpio_init() returns failure if the assignment to any
member of the gpio array results in an error pointer.
Since commit c359522194593815 ("serial: mctrl_gpio: Avoid probe failures
in case of missing gpiolib"), mctrl_gpio_to_gpiod() returns NULL in the
!CONFIG_GPIOLIB case.
Hence there is no longer a need to check for mctrl_gpio_to_gpiod()
returning an error value.  A simple NULL check is sufficient.

This follows the spirit of commit 445df7ff3fd1a0a9 ("serial: mctrl-gpio:
drop usages of IS_ERR_OR_NULL") in the mctrl-gpio core.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/20190814092924.13857-4-geert+renesas@glider.be
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/tty/serial/sh-sci.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 7d7008e4dc75..c40cf1cb45d2 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1871,12 +1871,12 @@ static unsigned int sci_get_mctrl(struct uart_port *port)
 	if (s->autorts) {
 		if (sci_get_cts(port))
 			mctrl |= TIOCM_CTS;
-	} else if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_CTS))) {
+	} else if (!mctrl_gpio_to_gpiod(gpios, UART_GPIO_CTS)) {
 		mctrl |= TIOCM_CTS;
 	}
-	if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_DSR)))
+	if (!mctrl_gpio_to_gpiod(gpios, UART_GPIO_DSR))
 		mctrl |= TIOCM_DSR;
-	if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_DCD)))
+	if (!mctrl_gpio_to_gpiod(gpios, UART_GPIO_DCD))
 		mctrl |= TIOCM_CAR;
 
 	return mctrl;
@@ -2782,10 +2782,8 @@ static int sci_probe_single(struct platform_device *dev,
 		return PTR_ERR(sciport->gpios);
 
 	if (p->capabilities & SCIx_HAVE_RTSCTS) {
-		if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(sciport->gpios,
-							UART_GPIO_CTS)) ||
-		    !IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(sciport->gpios,
-							UART_GPIO_RTS))) {
+		if (mctrl_gpio_to_gpiod(sciport->gpios, UART_GPIO_CTS) ||
+		    mctrl_gpio_to_gpiod(sciport->gpios, UART_GPIO_RTS)) {
 			dev_err(&dev->dev, "Conflicting RTS/CTS config\n");
 			return -EINVAL;
 		}
-- 
2.17.1


[-- Attachment #2: Type: text/plain, Size: 419 bytes --]

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#5394): https://lists.cip-project.org/g/cip-dev/message/5394
Mute This Topic: https://lists.cip-project.org/mt/76607965/4520388
Group Owner: cip-dev+owner@lists.cip-project.org
Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy  [cip-dev@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [cip-dev] [PATCH 4.4.y-cip 1/4] serial: mctrl_gpio: Check if GPIO property exisits before requesting it
  2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 1/4] serial: mctrl_gpio: Check if GPIO property exisits before requesting it Lad Prabhakar
@ 2020-09-03 20:43   ` Pavel Machek
  0 siblings, 0 replies; 9+ messages in thread
From: Pavel Machek @ 2020-09-03 20:43 UTC (permalink / raw)
  To: Lad Prabhakar; +Cc: cip-dev, Nobuhiro Iwamatsu, Pavel Machek, Biju Das


[-- Attachment #1.1: Type: text/plain, Size: 600 bytes --]

Hi!

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

Ok, so this fixes serial_mctrl_gpio on some ACPI systems. It should be
a NOP on Renesas hardware, right?

Do you need this one in 4.4? Was it tested on problematic ACPI system?

Best regards,
								Pavel

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 419 bytes --]

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#5408): https://lists.cip-project.org/g/cip-dev/message/5408
Mute This Topic: https://lists.cip-project.org/mt/76607963/4520388
Group Owner: cip-dev+owner@lists.cip-project.org
Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy  [cip-dev@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [cip-dev] [PATCH 4.4.y-cip 2/4] serial: sh-sci: Terminate TX DMA during buffer flushing
  2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 2/4] serial: sh-sci: Terminate TX DMA during buffer flushing Lad Prabhakar
@ 2020-09-03 20:46   ` Pavel Machek
  0 siblings, 0 replies; 9+ messages in thread
From: Pavel Machek @ 2020-09-03 20:46 UTC (permalink / raw)
  To: Lad Prabhakar; +Cc: cip-dev, Nobuhiro Iwamatsu, Pavel Machek, Biju Das


[-- Attachment #1.1: Type: text/plain, Size: 781 bytes --]

Hi!

> From: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> commit 775b7ffd7d6d5db320d99b0a485c51e04dfcf9f1 upstream.
> 
> While the .flush_buffer() callback clears sci_port.tx_dma_len since
> commit 1cf4a7efdc71cab8 ("serial: sh-sci: Fix race condition causing
> garbage during shutdown"), it does not terminate a transmit DMA
> operation that may be in progress.
> 
> Fix this by terminating any pending DMA operations, and resetting the
> corresponding cookie.

What is the impact of the bug? DMA running during reboot, but not
doing any harm?

Anyway, this looks okay to me.

Best regards,
								Pavel

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 419 bytes --]

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#5409): https://lists.cip-project.org/g/cip-dev/message/5409
Mute This Topic: https://lists.cip-project.org/mt/76607962/4520388
Group Owner: cip-dev+owner@lists.cip-project.org
Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy  [cip-dev@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [cip-dev] [PATCH 4.4.y-cip 3/4] serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS
  2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 3/4] serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS Lad Prabhakar
@ 2020-09-03 20:51   ` Pavel Machek
  0 siblings, 0 replies; 9+ messages in thread
From: Pavel Machek @ 2020-09-03 20:51 UTC (permalink / raw)
  To: Lad Prabhakar; +Cc: cip-dev, Nobuhiro Iwamatsu, Pavel Machek, Biju Das


[-- Attachment #1.1: Type: text/plain, Size: 1092 bytes --]

Hi!

> Now that the mctrl_gpio code returns NULL instead of ERR_PTR(-ENOSYS)
> if CONFIG_GPIOLIB is disabled, we can safely remove this check.

No, sorry, I don't think this is correct.

In 4.4, we still have

static inline struct gpio_desc *__must_check
devm_gpiod_get_index_optional(struct device *dev, const char *con_id,
                                unsigned int index, enum gpiod_flags flags)
{
        return ERR_PTR(-ENOSYS);
}

and that propagates through mctrl_gpio_init().

NAK.
								Pavel

> +++ b/drivers/tty/serial/sh-sci.c
> @@ -2778,7 +2778,7 @@ static int sci_probe_single(struct platform_device *dev,
>  		return ret;
>  
>  	sciport->gpios = mctrl_gpio_init(&sciport->port, 0);
> -	if (IS_ERR(sciport->gpios) && PTR_ERR(sciport->gpios) != -ENOSYS)
> +	if (IS_ERR(sciport->gpios))
>  		return PTR_ERR(sciport->gpios);
>  
>  	if (p->capabilities & SCIx_HAVE_RTSCTS) {
> -- 
> 2.17.1
> 

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 419 bytes --]

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#5410): https://lists.cip-project.org/g/cip-dev/message/5410
Mute This Topic: https://lists.cip-project.org/mt/76607964/4520388
Group Owner: cip-dev+owner@lists.cip-project.org
Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy  [cip-dev@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [cip-dev] [PATCH 4.4.y-cip 4/4] serial: sh-sci: Don't check for mctrl_gpio_to_gpiod() returning error
  2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 4/4] serial: sh-sci: Don't check for mctrl_gpio_to_gpiod() returning error Lad Prabhakar
@ 2020-09-03 20:54   ` Pavel Machek
  0 siblings, 0 replies; 9+ messages in thread
From: Pavel Machek @ 2020-09-03 20:54 UTC (permalink / raw)
  To: Lad Prabhakar; +Cc: cip-dev, Nobuhiro Iwamatsu, Pavel Machek, Biju Das


[-- Attachment #1.1: Type: text/plain, Size: 2073 bytes --]

Hi!

> Since commit 1d267ea6539f2663 ("serial: mctrl-gpio: simplify init
> routine"), mctrl_gpio_init() returns failure if the assignment to any
> member of the gpio array results in an error pointer.
> Since commit c359522194593815 ("serial: mctrl_gpio: Avoid probe failures
> in case of missing gpiolib"), mctrl_gpio_to_gpiod() returns NULL in the
> !CONFIG_GPIOLIB case.

Quick git log shows that "serial: mctrl_gpio: Avoid probe failures" is
not mentioned in 4.4 commit history. That very probably means that
this patch is not safe for 4.4 (situation similar to previous patch).

NAK.

Best regards,
								Pavel

> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1871,12 +1871,12 @@ static unsigned int sci_get_mctrl(struct uart_port *port)
>  	if (s->autorts) {
>  		if (sci_get_cts(port))
>  			mctrl |= TIOCM_CTS;
> -	} else if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_CTS))) {
> +	} else if (!mctrl_gpio_to_gpiod(gpios, UART_GPIO_CTS)) {
>  		mctrl |= TIOCM_CTS;
>  	}
> -	if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_DSR)))
> +	if (!mctrl_gpio_to_gpiod(gpios, UART_GPIO_DSR))
>  		mctrl |= TIOCM_DSR;
> -	if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_DCD)))
> +	if (!mctrl_gpio_to_gpiod(gpios, UART_GPIO_DCD))
>  		mctrl |= TIOCM_CAR;
>  
>  	return mctrl;
> @@ -2782,10 +2782,8 @@ static int sci_probe_single(struct platform_device *dev,
>  		return PTR_ERR(sciport->gpios);
>  
>  	if (p->capabilities & SCIx_HAVE_RTSCTS) {
> -		if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(sciport->gpios,
> -							UART_GPIO_CTS)) ||
> -		    !IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(sciport->gpios,
> -							UART_GPIO_RTS))) {
> +		if (mctrl_gpio_to_gpiod(sciport->gpios, UART_GPIO_CTS) ||
> +		    mctrl_gpio_to_gpiod(sciport->gpios, UART_GPIO_RTS)) {
>  			dev_err(&dev->dev, "Conflicting RTS/CTS config\n");
>  			return -EINVAL;
>  		}
> -- 
> 2.17.1
> 

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 419 bytes --]

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#5411): https://lists.cip-project.org/g/cip-dev/message/5411
Mute This Topic: https://lists.cip-project.org/mt/76607965/4520388
Group Owner: cip-dev+owner@lists.cip-project.org
Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy  [cip-dev@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-09-03 20:54 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-03 14:27 [cip-dev] [PATCH 4.4.y-cip 0/4] serial mctrl_gpio/sh-sci Fixes Lad Prabhakar
2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 1/4] serial: mctrl_gpio: Check if GPIO property exisits before requesting it Lad Prabhakar
2020-09-03 20:43   ` Pavel Machek
2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 2/4] serial: sh-sci: Terminate TX DMA during buffer flushing Lad Prabhakar
2020-09-03 20:46   ` Pavel Machek
2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 3/4] serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS Lad Prabhakar
2020-09-03 20:51   ` Pavel Machek
2020-09-03 14:27 ` [cip-dev] [PATCH 4.4.y-cip 4/4] serial: sh-sci: Don't check for mctrl_gpio_to_gpiod() returning error Lad Prabhakar
2020-09-03 20:54   ` Pavel Machek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).