Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH v2 0/2] support 8250-mtk uart in-band wake up
From: Claire Chang @ 2019-05-27  6:55 UTC (permalink / raw)
  To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
  Cc: Claire Chang, drinkcat-F7+t8E8rja9g9hUCZPvPmw,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	changqi.hu-NuS5LvNUpcJWk0Htik3J/w

changes in v2:
	1. add commit message
	2. drop return err in the resume path
	3. ignore disable_irq_wake()'s return value

Claire Chang (2):
  dt-bindings: serial: add documentation for Rx in-band wakeup support
  uart: mediatek: support Rx in-band wakeup

 .../devicetree/bindings/serial/mtk-uart.txt   | 13 ++++++++--
 drivers/tty/serial/8250/8250_mtk.c            | 26 +++++++++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)

-- 
2.22.0.rc1.257.g3120a18244-goog

^ permalink raw reply

* Re: [PATCH 2/2] serial: 8250-mtk: modify uart DMA rx
From: Vinod Koul @ 2019-05-27  6:40 UTC (permalink / raw)
  To: Long Cheng
  Cc: Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee, Sean Wang,
	Nicolas Boichat, Matthias Brugger, Dan Williams,
	Greg Kroah-Hartman, Jiri Slaby, Sean Wang, dmaengine, devicetree,
	linux-arm-kernel, linux-mediatek, linux-kernel, linux-serial,
	srv_heupstream, Yingjoe Chen, YT Shen, Zhenbao Liu
In-Reply-To: <1558596909-14084-3-git-send-email-long.cheng@mediatek.com>

On 23-05-19, 15:35, Long Cheng wrote:
> Modify uart rx and complete for DMA

Reviewed-by: Vinod Koul <vkoul@kernel.org>

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH v2 20/45] drivers: tty: serial: msm_serial: use devm_* functions
From: Bjorn Andersson @ 2019-05-27  5:57 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-kernel, gregkh, eric, stefan.wahren, f.fainelli, rjui,
	sbranden, bcm-kernel-feedback-list, andriy.shevchenko, vz,
	matthias.bgg, yamada.masahiro, tklauser, richard.genoud, macro,
	u.kleine-koenig, kernel, slemieux.tyco, andy.gross, david.brown,
	shawnguo, s.hauer, festevam, linux-imx, baohua, jacmet,
	linux-serial, linux-arm-msm, linuxppc-dev
In-Reply-To: <1552602855-26086-21-git-send-email-info@metux.net>

On Thu 14 Mar 15:33 PDT 2019, Enrico Weigelt, metux IT consult wrote:

> Use the safer devm versions of memory mapping functions.
> 
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>

As pointed out by others, this resource does not follow the life cycle
of the port->dev, so I don't think this improves the code.

Regards,
Bjorn

> ---
>  drivers/tty/serial/msm_serial.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
> index 1090960..e8e0c87 100644
> --- a/drivers/tty/serial/msm_serial.c
> +++ b/drivers/tty/serial/msm_serial.c
> @@ -1324,8 +1324,8 @@ static void msm_release_port(struct uart_port *port)
>  		return;
>  	size = resource_size(uart_resource);
>  
> -	release_mem_region(port->mapbase, size);
> -	iounmap(port->membase);
> +	devm_release_mem_region(port->dev, port->mapbase, size);
> +	devm_iounmap(port->dev, port->membase);
>  	port->membase = NULL;
>  }
>  
> @@ -1342,10 +1342,13 @@ static int msm_request_port(struct uart_port *port)
>  
>  	size = resource_size(uart_resource);
>  
> -	if (!request_mem_region(port->mapbase, size, "msm_serial"))
> +	if (!devm_request_mem_region(port->dev,
> +				     port->mapbase,
> +				     size,
> +				     "msm_serial"))
>  		return -EBUSY;
>  
> -	port->membase = ioremap(port->mapbase, size);
> +	port->membase = ioremap(port->dev, port->mapbase, size);
>  	if (!port->membase) {
>  		ret = -EBUSY;
>  		goto fail_release_port;
> @@ -1354,7 +1357,7 @@ static int msm_request_port(struct uart_port *port)
>  	return 0;
>  
>  fail_release_port:
> -	release_mem_region(port->mapbase, size);
> +	devm_release_mem_region(port->dev, port->mapbase, size);
>  	return ret;
>  }
>  
> -- 
> 1.9.1
> 

^ permalink raw reply

* [PATCH] serial: Fix an invalid comparing statement
From: Sugaya Taichi @ 2019-05-27  5:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Takao Orito, Kazuhiro Kasai, Shinji Kanematsu, Jassi Brar,
	Masami Hiramatsu, linux-kernel, linux-serial, Sugaya Taichi

Drop the if-statement which refers to 8th bit field of u8 variable.
The bit field is no longer used.

Fixes: ba44dc043004 ("serial: Add Milbeaut serial control")
Reported-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
---
 drivers/tty/serial/milbeaut_usio.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/milbeaut_usio.c b/drivers/tty/serial/milbeaut_usio.c
index 949ab7e..d7207ab 100644
--- a/drivers/tty/serial/milbeaut_usio.c
+++ b/drivers/tty/serial/milbeaut_usio.c
@@ -56,7 +56,6 @@
 #define MLB_USIO_SSR_FRE		BIT(4)
 #define MLB_USIO_SSR_PE			BIT(5)
 #define MLB_USIO_SSR_REC		BIT(7)
-#define MLB_USIO_SSR_BRK		BIT(8)
 #define MLB_USIO_FCR_FE1		BIT(0)
 #define MLB_USIO_FCR_FE2		BIT(1)
 #define MLB_USIO_FCR_FCL1		BIT(2)
@@ -180,18 +179,14 @@ static void mlb_usio_rx_chars(struct uart_port *port)
 		if (status & MLB_USIO_SSR_ORE)
 			port->icount.overrun++;
 		status &= port->read_status_mask;
-		if (status & MLB_USIO_SSR_BRK) {
-			flag = TTY_BREAK;
+		if (status & MLB_USIO_SSR_PE) {
+			flag = TTY_PARITY;
 			ch = 0;
 		} else
-			if (status & MLB_USIO_SSR_PE) {
-				flag = TTY_PARITY;
+			if (status & MLB_USIO_SSR_FRE) {
+				flag = TTY_FRAME;
 				ch = 0;
-			} else
-				if (status & MLB_USIO_SSR_FRE) {
-					flag = TTY_FRAME;
-					ch = 0;
-				}
+			}
 		if (flag)
 			uart_insert_char(port, status, MLB_USIO_SSR_ORE,
 					 ch, flag);
-- 
1.9.1

^ permalink raw reply related

* [PATCH 1/1] dt-bindings: stm32: serial: Add optional reset
From: Erwan Le Ray @ 2019-05-24 15:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Mark Rutland, Maxime Coquelin,
	Alexandre Torgue
  Cc: linux-serial, devicetree, linux-stm32, linux-arm-kernel,
	linux-kernel, Fabrice Gasnier, Erwan Le Ray

STM32 serial can be reset via reset controller.
Add an optional reset property to stm32 usart bindings.

Signed-off-by: Erwan Le Ray <erwan.leray@st.com>

diff --git a/Documentation/devicetree/bindings/serial/st,stm32-usart.txt b/Documentation/devicetree/bindings/serial/st,stm32-usart.txt
index 9d3efed..a6b1948 100644
--- a/Documentation/devicetree/bindings/serial/st,stm32-usart.txt
+++ b/Documentation/devicetree/bindings/serial/st,stm32-usart.txt
@@ -13,6 +13,7 @@ Required properties:
 - clocks: The input clock of the USART instance
 
 Optional properties:
+- resets: Must contain the phandle to the reset controller.
 - pinctrl: The reference on the pins configuration
 - st,hw-flow-ctrl: bool flag to enable hardware flow control.
 - rs485-rts-delay, rs485-rx-during-tx, rs485-rts-active-low,
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH 1/2 v2] serial: mctrl_gpio: Check if GPIO property exisits before requesting it
From: Andy Shevchenko @ 2019-05-24 13:46 UTC (permalink / raw)
  To: Stefan Roese
  Cc: Mika Westerberg, open list:SERIAL DRIVERS,
	Linux Kernel Mailing List, Yegor Yefremov, Greg Kroah-Hartman,
	Giulio Benetti
In-Reply-To: <287cdcc8-9a8f-4583-8be9-bd1f95936733@denx.de>

On Fri, May 24, 2019 at 01:29:34PM +0200, Stefan Roese wrote:
> On 24.05.19 13:11, Andy Shevchenko wrote:
> > On Fri, May 24, 2019 at 1:21 PM Mika Westerberg
> > <mika.westerberg@linux.intel.com> wrote:
> > > 
> > > On Fri, May 24, 2019 at 11:48:24AM +0200, Stefan Roese 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.
> > > > 
> > 
> > We have a board where ASL provides _DSD for CTS and RxD pins.
> > I'm afraid this won't work on it.
> 
> With "won't work" you mean, that the GPIO can't be used for modem
> control in this case in the current implementation (with this
> patchset)? Or do you mean, that the breakage (input does not work
> on Broxton systems) will not be solved by this patch?

It will solve RxD case, due to mctrl doesn't count RxD as a "control" line.

Though we have CTS pin defined for the same purpose, which means the hardware
flow control won't work on a subset of Broxton boards.

> If its the former, then I think that solving this issue is something
> for a new patch, to support modem-control on such platforms as well
> (if needed).

> Please note that this patch is not trying to get modem-control working
> on such ACPI based systems.

I understand that. At the same time it should not break existing systems.

> Its targeted for device-tree enabled
> platforms, using the 8250 serial driver, here specifically a MIPS
> MT7688 based board. And just wants to fix the latter issue mentioned
> above so that the 8250 modem-control support can be accepted in
> mainline.

As I said already we have to distinguish *the purpose* of these GPIOs.
(like CTS).

Can we apply this if and only if the device has no ACPI companion device?

In this case DT will work as you expect and ACPI won't be broken.

> > Basically we need to understand the use of the GPIOs in UART. In our
> > case it's an out-of-band wake up source for UART.
> > Simply requiring GPIOs to be present is not enough.
> > 
> > Perhaps property like 'modem-control-gpio-in-use' (this seems a bad
> > name, given for sake of example).

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH 1/2 v2] serial: mctrl_gpio: Check if GPIO property exisits before requesting it
From: Giulio Benetti @ 2019-05-24 12:09 UTC (permalink / raw)
  To: Stefan Roese, Andy Shevchenko, Mika Westerberg
  Cc: open list:SERIAL DRIVERS, Linux Kernel Mailing List,
	Andy Shevchenko, Yegor Yefremov, Greg Kroah-Hartman
In-Reply-To: <287cdcc8-9a8f-4583-8be9-bd1f95936733@denx.de>

Hello Stefan,

Il 24/05/2019 13:29, Stefan Roese ha scritto:
> On 24.05.19 13:11, Andy Shevchenko wrote:
>> On Fri, May 24, 2019 at 1:21 PM Mika Westerberg
>> <mika.westerberg@linux.intel.com> wrote:
>>>
>>> On Fri, May 24, 2019 at 11:48:24AM +0200, Stefan Roese 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.
>>>>
>>
>> We have a board where ASL provides _DSD for CTS and RxD pins.
>> I'm afraid this won't work on it.
> 
> With "won't work" you mean, that the GPIO can't be used for modem
> control in this case in the current implementation (with this
> patchset)? Or do you mean, that the breakage (input does not work
> on Broxton systems) will not be solved by this patch?
> 
> If its the former, then I think that solving this issue is something
> for a new patch, to support modem-control on such platforms as well
> (if needed).
> 
> Please note that this patch is not trying to get modem-control working
> on such ACPI based systems. Its targeted for device-tree enabled
> platforms, using the 8250 serial driver, here specifically a MIPS
> MT7688 based board. And just wants to fix the latter issue mentioned
> above so that the 8250 modem-control support can be accepted in
> mainline.

Take a look here:
https://lkml.org/lkml/2018/6/5/253

It's about waking up 8250 UART.
As I remember that is the problem.
I wanted to try to fix it but had no time.

What it broken as I remember is the capability to wake up linux on uart 
RX. Hope I've understood right at that time.

Best regards
-- 
Giulio Benetti
CTO

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

>> Basically we need to understand the use of the GPIOs in UART. In our
>> case it's an out-of-band wake up source for UART.
>> Simply requiring GPIOs to be present is not enough.
>>
>> Perhaps property like 'modem-control-gpio-in-use' (this seems a bad
>> name, given for sake of example).
> 
> Thanks,
> Stefan
> 

^ permalink raw reply

* Re: [PATCH 1/2 v2] serial: mctrl_gpio: Check if GPIO property exisits before requesting it
From: Stefan Roese @ 2019-05-24 11:29 UTC (permalink / raw)
  To: Andy Shevchenko, Mika Westerberg
  Cc: open list:SERIAL DRIVERS, Linux Kernel Mailing List,
	Andy Shevchenko, Yegor Yefremov, Greg Kroah-Hartman,
	Giulio Benetti
In-Reply-To: <CAHp75VcMVrYv1MXmmqE9fDXShS=Y8pPdWZ4f1neo=ne88TLZDg@mail.gmail.com>

On 24.05.19 13:11, Andy Shevchenko wrote:
> On Fri, May 24, 2019 at 1:21 PM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
>>
>> On Fri, May 24, 2019 at 11:48:24AM +0200, Stefan Roese 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.
>>>
> 
> We have a board where ASL provides _DSD for CTS and RxD pins.
> I'm afraid this won't work on it.

With "won't work" you mean, that the GPIO can't be used for modem
control in this case in the current implementation (with this
patchset)? Or do you mean, that the breakage (input does not work
on Broxton systems) will not be solved by this patch?

If its the former, then I think that solving this issue is something
for a new patch, to support modem-control on such platforms as well
(if needed).

Please note that this patch is not trying to get modem-control working
on such ACPI based systems. Its targeted for device-tree enabled
platforms, using the 8250 serial driver, here specifically a MIPS
MT7688 based board. And just wants to fix the latter issue mentioned
above so that the 8250 modem-control support can be accepted in
mainline.
  
> Basically we need to understand the use of the GPIOs in UART. In our
> case it's an out-of-band wake up source for UART.
> Simply requiring GPIOs to be present is not enough.
> 
> Perhaps property like 'modem-control-gpio-in-use' (this seems a bad
> name, given for sake of example).

Thanks,
Stefan

^ permalink raw reply

* Re: [PATCH 1/2 v2] serial: mctrl_gpio: Check if GPIO property exisits before requesting it
From: Andy Shevchenko @ 2019-05-24 11:11 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Stefan Roese, open list:SERIAL DRIVERS, Linux Kernel Mailing List,
	Andy Shevchenko, Yegor Yefremov, Greg Kroah-Hartman,
	Giulio Benetti
In-Reply-To: <20190524102002.GT2781@lahna.fi.intel.com>

On Fri, May 24, 2019 at 1:21 PM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> On Fri, May 24, 2019 at 11:48:24AM +0200, Stefan Roese 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.
> >

We have a board where ASL provides _DSD for CTS and RxD pins.
I'm afraid this won't work on it.

Basically we need to understand the use of the GPIOs in UART. In our
case it's an out-of-band wake up source for UART.
Simply requiring GPIOs to be present is not enough.

Perhaps property like 'modem-control-gpio-in-use' (this seems a bad
name, given for sake of example).

> > Signed-off-by: Stefan Roese <sr@denx.de>
> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH 1/2 v2] serial: mctrl_gpio: Check if GPIO property exisits before requesting it
From: Mika Westerberg @ 2019-05-24 10:20 UTC (permalink / raw)
  To: Stefan Roese
  Cc: linux-serial, linux-kernel, Andy Shevchenko, Yegor Yefremov,
	Greg Kroah-Hartman, Giulio Benetti
In-Reply-To: <20190524094825.16151-1-sr@denx.de>

On Fri, May 24, 2019 at 11:48:24AM +0200, Stefan Roese 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>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

^ permalink raw reply

* [PATCH 2/2 v2] tty/serial/8250: use mctrl_gpio helpers
From: Stefan Roese @ 2019-05-24  9:48 UTC (permalink / raw)
  To: linux-serial
  Cc: linux-kernel, Yegor Yefremov, Mika Westerberg, Andy Shevchenko,
	Giulio Benetti, Greg Kroah-Hartman
In-Reply-To: <20190524094825.16151-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.

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>
---
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                | 35 ++++++++++++++++++-
 drivers/tty/serial/8250/8250_core.c           |  9 +++++
 drivers/tty/serial/8250/8250_omap.c           | 31 +++++++++-------
 drivers/tty/serial/8250/8250_port.c           |  7 +++-
 drivers/tty/serial/8250/Kconfig               |  1 +
 include/linux/serial_8250.h                   |  1 +
 7 files changed, 88 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 ebfb0bd5bef5..e59625bdb007 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);
@@ -141,12 +143,43 @@ void serial8250_em485_destroy(struct uart_8250_port *p);
 
 static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
 {
+	int mctrl_gpio = 0;
+
 	serial_out(up, UART_MCR, value);
+
+	if (value & UART_MCR_RTS)
+		mctrl_gpio |= TIOCM_RTS;
+	if (value & UART_MCR_DTR)
+		mctrl_gpio |= TIOCM_DTR;
+
+	mctrl_gpio_set(up->gpios, mctrl_gpio);
 }
 
 static inline int serial8250_in_MCR(struct uart_8250_port *up)
 {
-	return serial_in(up, UART_MCR);
+	int mctrl, mctrl_gpio = 0;
+
+	mctrl = serial_in(up, UART_MCR);
+
+	/* save current MCR values */
+	if (mctrl & UART_MCR_RTS)
+		mctrl_gpio |= TIOCM_RTS;
+	if (mctrl & UART_MCR_DTR)
+		mctrl_gpio |= TIOCM_DTR;
+
+	mctrl_gpio = mctrl_gpio_get_outputs(up->gpios, &mctrl_gpio);
+
+	if (mctrl_gpio & TIOCM_RTS)
+		mctrl |= UART_MCR_RTS;
+	else
+		mctrl &= ~UART_MCR_RTS;
+
+	if (mctrl_gpio & TIOCM_DTR)
+		mctrl |= UART_MCR_DTR;
+	else
+		mctrl &= ~UART_MCR_DTR;
+
+	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..ec0c5448c192 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -982,6 +982,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 +1018,13 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
 		if (up->port.flags & UPF_FIXED_TYPE)
 			uart->port.type = up->port.type;
 
+		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..562ac73d9f2f 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -141,18 +141,21 @@ 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 (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(up->gpios,
+						UART_GPIO_RTS))) {
+		/*
+		 * 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 +456,9 @@ 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
+		&& IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(up->gpios,
+							UART_GPIO_RTS))) {
 		/* 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 2304a84eee3b..220a78287d88 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);
@@ -1959,7 +1963,8 @@ unsigned int serial8250_do_get_mctrl(struct uart_port *port)
 		ret |= TIOCM_DSR;
 	if (status & UART_MSR_CTS)
 		ret |= TIOCM_CTS;
-	return ret;
+
+	return mctrl_gpio_get(up->gpios, &ret);
 }
 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 5a655ba8d273..14cd763baf78 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -114,6 +114,7 @@ struct uart_8250_port {
 						 *   if no_console_suspend
 						 */
 	unsigned char		probe;
+	struct mctrl_gpios	*gpios;
 #define UART_PROBE_RSA	(1 << 0)
 
 	/*
-- 
2.21.0

^ permalink raw reply related

* [PATCH 1/2 v2] serial: mctrl_gpio: Check if GPIO property exisits before requesting it
From: Stefan Roese @ 2019-05-24  9:48 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>
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>
---
v2:
- Include the problem description and analysis from Mika into the commit
  text, as suggested by Greg.

 drivers/tty/serial/serial_mctrl_gpio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index 39ed56214cd3..cac50b20a119 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -116,6 +116,13 @@ 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;
+
+		/* Check if GPIO property exists and continue if not */
+		gpio_str = kasprintf(GFP_KERNEL, "%s-gpios",
+				     mctrl_gpios_desc[i].name);
+		if (!device_property_present(dev, gpio_str))
+			continue;
 
 		if (mctrl_gpios_desc[i].dir_out)
 			flags = GPIOD_OUT_LOW;
-- 
2.21.0

^ permalink raw reply related

* Re: [PATCH 1/2] serial: mctrl_gpio: Check if GPIO property exisits before requesting it
From: Greg Kroah-Hartman @ 2019-05-24  7:56 UTC (permalink / raw)
  To: Stefan Roese
  Cc: linux-serial, linux-kernel, Mika Westerberg, Andy Shevchenko,
	Yegor Yefremov, Giulio Benetti
In-Reply-To: <20190522121117.14347-1-sr@denx.de>

On Wed, May 22, 2019 at 02:11:16PM +0200, Stefan Roese 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. Please find a details problem description here:
> 
> https://lkml.org/lkml/2016/8/9/357

Can you change this to a lore.kernel.org link instead?

Actually, just put the information in here, no one should ever have to
search somewhere else to determine what happened in a changelog entry.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: serial: add documentation for Rx in-band wakeup support
From: Greg KH @ 2019-05-24  7:55 UTC (permalink / raw)
  To: Claire Chang
  Cc: linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	changqi.hu-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <20190521084701.100179-2-tientzu-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

On Tue, May 21, 2019 at 04:47:00PM +0800, Claire Chang wrote:
> Signed-off-by: Claire Chang <tientzu-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---

I can not take patches without any changelog text at all.  Part of
writing a good patch is actually describing why you are making a change.

thanks,

greg k-h

^ permalink raw reply

* Re: [RFC v2 00/11] DVFS in the OPP core
From: Rajendra Nayak @ 2019-05-24  6:03 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: ulf.hansson, dianders, vincent.guittot, linux-scsi, linux-pm,
	linux-arm-msm, rafael, linux-kernel, dri-devel, linux-spi,
	linux-serial, swboyd
In-Reply-To: <20190521062248.ogjetb2rwtqekflx@vireshk-i7>


On 5/21/2019 11:52 AM, Viresh Kumar wrote:
> 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
> 
> Hi Rajendra,
> 
> I am inclined to apply/push this series for 5.3-rc1, will it be
> possible for you to spend some time on this at priority ?

Hey Viresh, I was on vacation, just got back. I will refresh this series
and address your previous feedback, I haven't received much feedback for the
driver changes :/ but we can atleast review and get the OPP layer changes
finalized. thanks.

-- 
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: [PATCH v13 1/2] arm: dts: mt2712: add uart APDMA to device tree
From: Matthias Brugger @ 2019-05-23 17:08 UTC (permalink / raw)
  To: Long Cheng, Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland,
	Ryder Lee, Sean Wang, Nicolas Boichat
  Cc: Dan Williams, Greg Kroah-Hartman, Jiri Slaby, Sean Wang,
	dmaengine, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, linux-serial, srv_heupstream, Yingjoe Chen, YT Shen,
	Zhenbao Liu
In-Reply-To: <434cbd9b-face-de45-0d17-4096ad81a7b9@gmail.com>



On 23/05/2019 19:04, Matthias Brugger wrote:
> 
> 
> On 23/05/2019 09:35, Long Cheng wrote:
>> 1. add uart APDMA controller device node
>> 2. add uart 0/1/2/3/4/5 DMA function
>>
>> Signed-off-by: Long Cheng <long.cheng@mediatek.com>
>> ---
>>  arch/arm64/boot/dts/mediatek/mt2712e.dtsi |   51 +++++++++++++++++++++++++++++
>>  1 file changed, 51 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
>> index 43307ba..a7a7362 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
>> +++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
>> @@ -300,6 +300,9 @@
>>  		interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_LOW>;
>>  		clocks = <&baud_clk>, <&sys_clk>;
>>  		clock-names = "baud", "bus";
>> +		dmas = <&apdma 10
>> +			&apdma 11>;
>> +		dma-names = "tx", "rx";
>>  		status = "disabled";
>>  	};
>>  
>> @@ -369,6 +372,39 @@
>>  			 (GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_HIGH)>;
>>  	};
>>  
>> +	apdma: dma-controller@11000400 {
>> +		compatible = "mediatek,mt2712-uart-dma",
>> +			     "mediatek,mt6577-uart-dma";
> 
> I was able to find a binding descpription but no actual driver.
> drivers/dma/mediatek only has hsdma and cqdma but no apdma driver.
> 
> Seems there is something missing here.
> 

Sorry I just realized that tje driver got merged from v12.

Regards,
Matthias

> Regards,
> Matthias
> 
>> +		reg = <0 0x11000400 0 0x80>,
>> +		      <0 0x11000480 0 0x80>,
>> +		      <0 0x11000500 0 0x80>,
>> +		      <0 0x11000580 0 0x80>,
>> +		      <0 0x11000600 0 0x80>,
>> +		      <0 0x11000680 0 0x80>,
>> +		      <0 0x11000700 0 0x80>,
>> +		      <0 0x11000780 0 0x80>,
>> +		      <0 0x11000800 0 0x80>,
>> +		      <0 0x11000880 0 0x80>,
>> +		      <0 0x11000900 0 0x80>,
>> +		      <0 0x11000980 0 0x80>;
>> +		interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_LOW>,
>> +			     <GIC_SPI 104 IRQ_TYPE_LEVEL_LOW>,
>> +			     <GIC_SPI 105 IRQ_TYPE_LEVEL_LOW>,
>> +			     <GIC_SPI 106 IRQ_TYPE_LEVEL_LOW>,
>> +			     <GIC_SPI 107 IRQ_TYPE_LEVEL_LOW>,
>> +			     <GIC_SPI 108 IRQ_TYPE_LEVEL_LOW>,
>> +			     <GIC_SPI 109 IRQ_TYPE_LEVEL_LOW>,
>> +			     <GIC_SPI 110 IRQ_TYPE_LEVEL_LOW>,
>> +			     <GIC_SPI 111 IRQ_TYPE_LEVEL_LOW>,
>> +			     <GIC_SPI 112 IRQ_TYPE_LEVEL_LOW>,
>> +			     <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>,
>> +			     <GIC_SPI 114 IRQ_TYPE_LEVEL_LOW>;
>> +		dma-requests = <12>;
>> +		clocks = <&pericfg CLK_PERI_AP_DMA>;
>> +		clock-names = "apdma";
>> +		#dma-cells = <1>;
>> +	};
>> +
>>  	auxadc: adc@11001000 {
>>  		compatible = "mediatek,mt2712-auxadc";
>>  		reg = <0 0x11001000 0 0x1000>;
>> @@ -385,6 +421,9 @@
>>  		interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
>>  		clocks = <&baud_clk>, <&sys_clk>;
>>  		clock-names = "baud", "bus";
>> +		dmas = <&apdma 0
>> +			&apdma 1>;
>> +		dma-names = "tx", "rx";
>>  		status = "disabled";
>>  	};
>>  
>> @@ -395,6 +434,9 @@
>>  		interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
>>  		clocks = <&baud_clk>, <&sys_clk>;
>>  		clock-names = "baud", "bus";
>> +		dmas = <&apdma 2
>> +			&apdma 3>;
>> +		dma-names = "tx", "rx";
>>  		status = "disabled";
>>  	};
>>  
>> @@ -405,6 +447,9 @@
>>  		interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_LOW>;
>>  		clocks = <&baud_clk>, <&sys_clk>;
>>  		clock-names = "baud", "bus";
>> +		dmas = <&apdma 4
>> +			&apdma 5>;
>> +		dma-names = "tx", "rx";
>>  		status = "disabled";
>>  	};
>>  
>> @@ -415,6 +460,9 @@
>>  		interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_LOW>;
>>  		clocks = <&baud_clk>, <&sys_clk>;
>>  		clock-names = "baud", "bus";
>> +		dmas = <&apdma 6
>> +			&apdma 7>;
>> +		dma-names = "tx", "rx";
>>  		status = "disabled";
>>  	};
>>  
>> @@ -629,6 +677,9 @@
>>  		interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_LOW>;
>>  		clocks = <&baud_clk>, <&sys_clk>;
>>  		clock-names = "baud", "bus";
>> +		dmas = <&apdma 8
>> +			&apdma 9>;
>> +		dma-names = "tx", "rx";
>>  		status = "disabled";
>>  	};
>>  
>>

^ permalink raw reply

* Re: [PATCH v13 1/2] arm: dts: mt2712: add uart APDMA to device tree
From: Matthias Brugger @ 2019-05-23 17:04 UTC (permalink / raw)
  To: Long Cheng, Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland,
	Ryder Lee, Sean Wang, Nicolas Boichat
  Cc: Dan Williams, Greg Kroah-Hartman, Jiri Slaby, Sean Wang,
	dmaengine, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, linux-serial, srv_heupstream, Yingjoe Chen, YT Shen,
	Zhenbao Liu
In-Reply-To: <1558596909-14084-2-git-send-email-long.cheng@mediatek.com>



On 23/05/2019 09:35, Long Cheng wrote:
> 1. add uart APDMA controller device node
> 2. add uart 0/1/2/3/4/5 DMA function
> 
> Signed-off-by: Long Cheng <long.cheng@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt2712e.dtsi |   51 +++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> index 43307ba..a7a7362 100644
> --- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> @@ -300,6 +300,9 @@
>  		interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_LOW>;
>  		clocks = <&baud_clk>, <&sys_clk>;
>  		clock-names = "baud", "bus";
> +		dmas = <&apdma 10
> +			&apdma 11>;
> +		dma-names = "tx", "rx";
>  		status = "disabled";
>  	};
>  
> @@ -369,6 +372,39 @@
>  			 (GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_HIGH)>;
>  	};
>  
> +	apdma: dma-controller@11000400 {
> +		compatible = "mediatek,mt2712-uart-dma",
> +			     "mediatek,mt6577-uart-dma";

I was able to find a binding descpription but no actual driver.
drivers/dma/mediatek only has hsdma and cqdma but no apdma driver.

Seems there is something missing here.

Regards,
Matthias

> +		reg = <0 0x11000400 0 0x80>,
> +		      <0 0x11000480 0 0x80>,
> +		      <0 0x11000500 0 0x80>,
> +		      <0 0x11000580 0 0x80>,
> +		      <0 0x11000600 0 0x80>,
> +		      <0 0x11000680 0 0x80>,
> +		      <0 0x11000700 0 0x80>,
> +		      <0 0x11000780 0 0x80>,
> +		      <0 0x11000800 0 0x80>,
> +		      <0 0x11000880 0 0x80>,
> +		      <0 0x11000900 0 0x80>,
> +		      <0 0x11000980 0 0x80>;
> +		interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_SPI 104 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_SPI 105 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_SPI 106 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_SPI 107 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_SPI 108 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_SPI 109 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_SPI 110 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_SPI 111 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_SPI 112 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_SPI 114 IRQ_TYPE_LEVEL_LOW>;
> +		dma-requests = <12>;
> +		clocks = <&pericfg CLK_PERI_AP_DMA>;
> +		clock-names = "apdma";
> +		#dma-cells = <1>;
> +	};
> +
>  	auxadc: adc@11001000 {
>  		compatible = "mediatek,mt2712-auxadc";
>  		reg = <0 0x11001000 0 0x1000>;
> @@ -385,6 +421,9 @@
>  		interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
>  		clocks = <&baud_clk>, <&sys_clk>;
>  		clock-names = "baud", "bus";
> +		dmas = <&apdma 0
> +			&apdma 1>;
> +		dma-names = "tx", "rx";
>  		status = "disabled";
>  	};
>  
> @@ -395,6 +434,9 @@
>  		interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
>  		clocks = <&baud_clk>, <&sys_clk>;
>  		clock-names = "baud", "bus";
> +		dmas = <&apdma 2
> +			&apdma 3>;
> +		dma-names = "tx", "rx";
>  		status = "disabled";
>  	};
>  
> @@ -405,6 +447,9 @@
>  		interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_LOW>;
>  		clocks = <&baud_clk>, <&sys_clk>;
>  		clock-names = "baud", "bus";
> +		dmas = <&apdma 4
> +			&apdma 5>;
> +		dma-names = "tx", "rx";
>  		status = "disabled";
>  	};
>  
> @@ -415,6 +460,9 @@
>  		interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_LOW>;
>  		clocks = <&baud_clk>, <&sys_clk>;
>  		clock-names = "baud", "bus";
> +		dmas = <&apdma 6
> +			&apdma 7>;
> +		dma-names = "tx", "rx";
>  		status = "disabled";
>  	};
>  
> @@ -629,6 +677,9 @@
>  		interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_LOW>;
>  		clocks = <&baud_clk>, <&sys_clk>;
>  		clock-names = "baud", "bus";
> +		dmas = <&apdma 8
> +			&apdma 9>;
> +		dma-names = "tx", "rx";
>  		status = "disabled";
>  	};
>  
> 

^ permalink raw reply

* Re: [PATCH] serial-uartlite: Fix null-ptr-deref in ulite_exit
From: Michal Simek @ 2019-05-23 12:47 UTC (permalink / raw)
  To: Johan Hovold, Michal Simek
  Cc: Greg KH, YueHaibing, jslaby, shubhrajyoti.datta, linux-kernel,
	linux-serial
In-Reply-To: <20190523123140.GF568@localhost>

On 23. 05. 19 14:31, Johan Hovold wrote:
> On Thu, May 23, 2019 at 12:46:38PM +0200, Michal Simek wrote:
>> Hi Johan,
>>
>> On 23. 05. 19 11:18, Johan Hovold wrote:
>>> On Tue, May 21, 2019 at 12:10:59PM +0200, Greg Kroah-Hartman wrote:
>>>> On Fri, May 17, 2019 at 09:55:02AM +0200, Johan Hovold wrote:
>>>>> On Thu, May 16, 2019 at 12:09:31PM +0800, YueHaibing wrote:
>>>>>> If ulite_probe is not called or failed to registed
>>>>>> uart_register_driver, unload the module will call
>>>>>> uart_unregister_driver, which will tigger NULL
>>>>>> pointer dereference like this:
>>>>>>
>>>>>> BUG: KASAN: null-ptr-deref in tty_unregister_driver+0x19/0x100
>>>>>> Read of size 4 at addr 0000000000000034 by task syz-executor.0/4246
>>>>>
>>>>>> This patch fix this by moving uart_unregister_driver
>>>>>> to ulite_remove.
>>>>>>
>>>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>>>>> Fixes: 415b43bdb008 ("tty: serial: uartlite: Move uart register to probe")
>>>>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>>>>>> ---
>>>>>>  drivers/tty/serial/uartlite.c | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
>>>>>> index b8b912b..2e49fb6 100644
>>>>>> --- a/drivers/tty/serial/uartlite.c
>>>>>> +++ b/drivers/tty/serial/uartlite.c
>>>>>> @@ -867,6 +867,7 @@ static int ulite_remove(struct platform_device *pdev)
>>>>>>  	pm_runtime_disable(&pdev->dev);
>>>>>>  	pm_runtime_set_suspended(&pdev->dev);
>>>>>>  	pm_runtime_dont_use_autosuspend(&pdev->dev);
>>>>>> +	uart_unregister_driver(&ulite_uart_driver);
>>>>>
>>>>> This broken. Consider what happens if you have tho ports registered and
>>>>> you unbind the first.
>>>>>
>>>>> Someone else sent a fix for this here
>>>>>
>>>>> 	https://lkml.kernel.org/r/20190514033219.169947-1-wangkefeng.wang@huawei.com
>>>>>
>>>>> That fix also has some issues, but is still better given the current
>>>>> state this driver is in.
>>>>
>>>> I'm not taking any of these patches until people agree on what needs to
>>>> be done here :)
>>>
>>> Good. :)
>>>
>>>> Why is this driver so "special" it is having these types of problems?
>>>> Why can't it do what all other drivers do in this case?
>>>
>>> Apparently some vendor-tree hacks has made it into mainline.
>>
>> I have designed this change and it didn't go to vendor tree first.
>> I have been also asking if this is the right way to go.
>> You can find the whole series started with this RFC
>> https://lore.kernel.org/lkml/e2039dc5-92ec-d3a1-bc4f-6565a8c901ac@suse.de/t/
>>
>> https://lkml.org/lkml/2018/6/6/346
>> and then
>>
>> https://lkml.org/lkml/2018/9/3/404
> 
> Looks like you didn't get any real feedback to your first two RFC
> series, before you resent as non-RFC and it was merged without any
> discussion. :/
> 
>> And even this step was discussed before in
>> [RFC PATCH 0/4] serial: uartps: Dynamic allocation
>> which was July 2017.
> 
> Yeah, you definitely tried to get feedback on this.
> 
>> And all these patches have been merged by Greg and then I have taken
>> them back to Xilinx Linux tree.
> 
> Right, I had missed some of the back story.
> 
>> And the same concept was also applied to uartlite and intention is also
>> to apply it to pl011 driver to avoid this warning.
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/amba-pl011.c?h=v5.2-rc1#n2538
> 
> Perhaps you should hold off on spreading this further until it can be
> reviewed. Looks like it's mostly contained to xilinx_uartps right now.
> 
>>> They're trying to register one tty/uart driver per physical port
>>> which sounds like a terrible idea. And even the implementation is
>>> broken as these bug reports indicate.
>>
>> I have followed standard process and I have been asking for advices how
>> to do it without hardcoded any number and limiting amount of
>> pre-registered uarts because adding Kconfig options for NR_UARTs was
>> NACK in past too.
> 
> Yep, you clearly tried to get feedback, but our process fails sometimes.
> 
>> If you know better way how this can be done, please let us know.
> 
> Having separate tty drivers and allocating a new major number for every
> serial port clearly isn't the right way at least (and especially not for
> fpga uarts of which there could be plenty).


There is no fixed major number for uartps which is not the case for
uartlite.
It means from description you can see that every uartps instance is
getting different major number and also different minor number.
I am aware about this and it was also described. I have one issue
regarding this flying around. But thinking to save non 0 major number
after first registration and pass it to other instances to avoid this
behavior.

In uartlite where dynamic major number allocation is not present you get
correct behavior.
uartlite0 204/187
uartlite1 204/188
etc.

> If you can't implement what you're after with the current serial-core
> and tty infrastructure, those subsystems may need to be updated first.

I didn't need to change anything in tty core because all of this is
possible to do. There was one patch in connection to reading aliases
which was properly reviewed and applied.
In general DT aliases behavior should be the same across all boards but
in reality it is not.

>> And if there is bug we should definitely fix it.
> 
> Yes, but not by papering over the problem. 

sure. This is the first time I hear about it. Based on my understanding
and current linux-next this patch is wrong. If there is any other series
please keep in me in CC and I will take a look at will test it on HW.


>>> A series was apparently first posted for xilinks_uartps.c, but that one
>>> has not yet been merged AFAICT.
>>
>> Series have been applied by Great 2018-09-18. Feel free to check log here:
> 
> "Greg the Great" does have a nice ring to it. ;)


:-) nice typo.

> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/tty/serial/xilinx_uartps.c?h=v5.2-rc1
> 
> Yeah, sorry, I was obviously looking at the wrong tree this morning.

good.

> 
>>> A similar series was later posted for
>>> uartlite.c, but only the first half or so got in:
>>>
>>> 	https://lkml.kernel.org/r/1539685088-13465-1-git-send-email-shubhrajyoti.datta@gmail.com
>>>
>>> Actually, it looks like the problems started already with:
>>>
>>> 	https://lkml.kernel.org/r/1533545534-24458-1-git-send-email-shubhrajyoti.datta@gmail.com
>>>
>>> So the first broken commit is
>>>
>>> 	415b43bdb008 ("tty: serial: uartlite: Move uart register to probe")
>>>
>>> I think the offending patches should be reverted, but unfortunately they
>>> may no longer revert cleanly since there were some new features (e.g.
>>> runtime pm) thrown in the mix.
>>
>> Similar changes have been sent for uartlite but they should use the same
>> concept as I started to use for uartps.
>> But based on what I see in linux-next these patches haven't been merged
>> there.
>> That's why I don't understand the reason for this patch.
> 
> As I mentioned above, only half of the uartlite series was merged, but
> that was enough to break module unload.

I didn't strictly follow this. Internally I have reviewed it but didn't
watch what exactly was merged to mainline.

Thanks,
Michal

^ permalink raw reply

* Re: [PATCH] serial-uartlite: Fix null-ptr-deref in ulite_exit
From: Johan Hovold @ 2019-05-23 12:31 UTC (permalink / raw)
  To: Michal Simek
  Cc: Johan Hovold, Greg KH, YueHaibing, jslaby, shubhrajyoti.datta,
	linux-kernel, linux-serial
In-Reply-To: <3bdfe2ab-daec-e222-8e2b-cac96fd218a2@xilinx.com>

On Thu, May 23, 2019 at 12:46:38PM +0200, Michal Simek wrote:
> Hi Johan,
> 
> On 23. 05. 19 11:18, Johan Hovold wrote:
> > On Tue, May 21, 2019 at 12:10:59PM +0200, Greg Kroah-Hartman wrote:
> >> On Fri, May 17, 2019 at 09:55:02AM +0200, Johan Hovold wrote:
> >>> On Thu, May 16, 2019 at 12:09:31PM +0800, YueHaibing wrote:
> >>>> If ulite_probe is not called or failed to registed
> >>>> uart_register_driver, unload the module will call
> >>>> uart_unregister_driver, which will tigger NULL
> >>>> pointer dereference like this:
> >>>>
> >>>> BUG: KASAN: null-ptr-deref in tty_unregister_driver+0x19/0x100
> >>>> Read of size 4 at addr 0000000000000034 by task syz-executor.0/4246
> >>>
> >>>> This patch fix this by moving uart_unregister_driver
> >>>> to ulite_remove.
> >>>>
> >>>> Reported-by: Hulk Robot <hulkci@huawei.com>
> >>>> Fixes: 415b43bdb008 ("tty: serial: uartlite: Move uart register to probe")
> >>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> >>>> ---
> >>>>  drivers/tty/serial/uartlite.c | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
> >>>> index b8b912b..2e49fb6 100644
> >>>> --- a/drivers/tty/serial/uartlite.c
> >>>> +++ b/drivers/tty/serial/uartlite.c
> >>>> @@ -867,6 +867,7 @@ static int ulite_remove(struct platform_device *pdev)
> >>>>  	pm_runtime_disable(&pdev->dev);
> >>>>  	pm_runtime_set_suspended(&pdev->dev);
> >>>>  	pm_runtime_dont_use_autosuspend(&pdev->dev);
> >>>> +	uart_unregister_driver(&ulite_uart_driver);
> >>>
> >>> This broken. Consider what happens if you have tho ports registered and
> >>> you unbind the first.
> >>>
> >>> Someone else sent a fix for this here
> >>>
> >>> 	https://lkml.kernel.org/r/20190514033219.169947-1-wangkefeng.wang@huawei.com
> >>>
> >>> That fix also has some issues, but is still better given the current
> >>> state this driver is in.
> >>
> >> I'm not taking any of these patches until people agree on what needs to
> >> be done here :)
> > 
> > Good. :)
> > 
> >> Why is this driver so "special" it is having these types of problems?
> >> Why can't it do what all other drivers do in this case?
> > 
> > Apparently some vendor-tree hacks has made it into mainline.
> 
> I have designed this change and it didn't go to vendor tree first.
> I have been also asking if this is the right way to go.
> You can find the whole series started with this RFC
> https://lore.kernel.org/lkml/e2039dc5-92ec-d3a1-bc4f-6565a8c901ac@suse.de/t/
> 
> https://lkml.org/lkml/2018/6/6/346
> and then
> 
> https://lkml.org/lkml/2018/9/3/404

Looks like you didn't get any real feedback to your first two RFC
series, before you resent as non-RFC and it was merged without any
discussion. :/

> And even this step was discussed before in
> [RFC PATCH 0/4] serial: uartps: Dynamic allocation
> which was July 2017.

Yeah, you definitely tried to get feedback on this.

> And all these patches have been merged by Greg and then I have taken
> them back to Xilinx Linux tree.

Right, I had missed some of the back story.

> And the same concept was also applied to uartlite and intention is also
> to apply it to pl011 driver to avoid this warning.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/amba-pl011.c?h=v5.2-rc1#n2538

Perhaps you should hold off on spreading this further until it can be
reviewed. Looks like it's mostly contained to xilinx_uartps right now.

> > They're trying to register one tty/uart driver per physical port
> > which sounds like a terrible idea. And even the implementation is
> > broken as these bug reports indicate.
> 
> I have followed standard process and I have been asking for advices how
> to do it without hardcoded any number and limiting amount of
> pre-registered uarts because adding Kconfig options for NR_UARTs was
> NACK in past too.

Yep, you clearly tried to get feedback, but our process fails sometimes.

> If you know better way how this can be done, please let us know.

Having separate tty drivers and allocating a new major number for every
serial port clearly isn't the right way at least (and especially not for
fpga uarts of which there could be plenty).

If you can't implement what you're after with the current serial-core
and tty infrastructure, those subsystems may need to be updated first.

> And if there is bug we should definitely fix it.

Yes, but not by papering over the problem. 

> > A series was apparently first posted for xilinks_uartps.c, but that one
> > has not yet been merged AFAICT.
> 
> Series have been applied by Great 2018-09-18. Feel free to check log here:

"Greg the Great" does have a nice ring to it. ;)

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/tty/serial/xilinx_uartps.c?h=v5.2-rc1

Yeah, sorry, I was obviously looking at the wrong tree this morning.

> > A similar series was later posted for
> > uartlite.c, but only the first half or so got in:
> > 
> > 	https://lkml.kernel.org/r/1539685088-13465-1-git-send-email-shubhrajyoti.datta@gmail.com
> > 
> > Actually, it looks like the problems started already with:
> > 
> > 	https://lkml.kernel.org/r/1533545534-24458-1-git-send-email-shubhrajyoti.datta@gmail.com
> > 
> > So the first broken commit is
> > 
> > 	415b43bdb008 ("tty: serial: uartlite: Move uart register to probe")
> > 
> > I think the offending patches should be reverted, but unfortunately they
> > may no longer revert cleanly since there were some new features (e.g.
> > runtime pm) thrown in the mix.
> 
> Similar changes have been sent for uartlite but they should use the same
> concept as I started to use for uartps.
> But based on what I see in linux-next these patches haven't been merged
> there.
> That's why I don't understand the reason for this patch.

As I mentioned above, only half of the uartlite series was merged, but
that was enough to break module unload.

> If you want to see latest state of uartlite you can find it out here.
> https://github.com/Xilinx/linux-xlnx

Thanks for the pointer.

Johan

^ permalink raw reply

* Re: [PATCH] serial-uartlite: Fix null-ptr-deref in ulite_exit
From: Michal Simek @ 2019-05-23 10:46 UTC (permalink / raw)
  To: Johan Hovold, Greg KH
  Cc: YueHaibing, jslaby, shubhrajyoti.datta, linux-kernel,
	linux-serial, Michal Simek
In-Reply-To: <20190523091839.GC568@localhost>

Hi Johan,

On 23. 05. 19 11:18, Johan Hovold wrote:
> On Tue, May 21, 2019 at 12:10:59PM +0200, Greg Kroah-Hartman wrote:
>> On Fri, May 17, 2019 at 09:55:02AM +0200, Johan Hovold wrote:
>>> On Thu, May 16, 2019 at 12:09:31PM +0800, YueHaibing wrote:
>>>> If ulite_probe is not called or failed to registed
>>>> uart_register_driver, unload the module will call
>>>> uart_unregister_driver, which will tigger NULL
>>>> pointer dereference like this:
>>>>
>>>> BUG: KASAN: null-ptr-deref in tty_unregister_driver+0x19/0x100
>>>> Read of size 4 at addr 0000000000000034 by task syz-executor.0/4246
>>>
>>>> This patch fix this by moving uart_unregister_driver
>>>> to ulite_remove.
>>>>
>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>>> Fixes: 415b43bdb008 ("tty: serial: uartlite: Move uart register to probe")
>>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>>>> ---
>>>>  drivers/tty/serial/uartlite.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
>>>> index b8b912b..2e49fb6 100644
>>>> --- a/drivers/tty/serial/uartlite.c
>>>> +++ b/drivers/tty/serial/uartlite.c
>>>> @@ -867,6 +867,7 @@ static int ulite_remove(struct platform_device *pdev)
>>>>  	pm_runtime_disable(&pdev->dev);
>>>>  	pm_runtime_set_suspended(&pdev->dev);
>>>>  	pm_runtime_dont_use_autosuspend(&pdev->dev);
>>>> +	uart_unregister_driver(&ulite_uart_driver);
>>>
>>> This broken. Consider what happens if you have tho ports registered and
>>> you unbind the first.
>>>
>>> Someone else sent a fix for this here
>>>
>>> 	https://lkml.kernel.org/r/20190514033219.169947-1-wangkefeng.wang@huawei.com
>>>
>>> That fix also has some issues, but is still better given the current
>>> state this driver is in.
>>
>> I'm not taking any of these patches until people agree on what needs to
>> be done here :)
> 
> Good. :)
> 
>> Why is this driver so "special" it is having these types of problems?
>> Why can't it do what all other drivers do in this case?
> 
> Apparently some vendor-tree hacks has made it into mainline.

I have designed this change and it didn't go to vendor tree first.
I have been also asking if this is the right way to go.
You can find the whole series started with this RFC
https://lore.kernel.org/lkml/e2039dc5-92ec-d3a1-bc4f-6565a8c901ac@suse.de/t/

https://lkml.org/lkml/2018/6/6/346
and then

https://lkml.org/lkml/2018/9/3/404


And even this step was discussed before in
[RFC PATCH 0/4] serial: uartps: Dynamic allocation
which was July 2017.

And all these patches have been merged by Greg and then I have taken
them back to Xilinx Linux tree.

And the same concept was also applied to uartlite and intention is also
to apply it to pl011 driver to avoid this warning.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/amba-pl011.c?h=v5.2-rc1#n2538

> They're
> trying to register one tty/uart driver per physical port which sounds
> like a terrible idea. And even the implementation is broken as these bug
> reports indicate.

I have followed standard process and I have been asking for advices how
to do it without hardcoded any number and limiting amount of
pre-registered uarts because adding Kconfig options for NR_UARTs was
NACK in past too.
If you know better way how this can be done, please let us know.

And if there is bug we should definitely fix it.


> A series was apparently first posted for xilinks_uartps.c, but that one
> has not yet been merged AFAICT.

Series have been applied by Great 2018-09-18. Feel free to check log here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/tty/serial/xilinx_uartps.c?h=v5.2-rc1


> A similar series was later posted for
> uartlite.c, but only the first half or so got in:
> 
> 	https://lkml.kernel.org/r/1539685088-13465-1-git-send-email-shubhrajyoti.datta@gmail.com
> 
> Actually, it looks like the problems started already with:
> 
> 	https://lkml.kernel.org/r/1533545534-24458-1-git-send-email-shubhrajyoti.datta@gmail.com
> 
> So the first broken commit is
> 
> 	415b43bdb008 ("tty: serial: uartlite: Move uart register to probe")
> 
> I think the offending patches should be reverted, but unfortunately they
> may no longer revert cleanly since there were some new features (e.g.
> runtime pm) thrown in the mix.

Similar changes have been sent for uartlite but they should use the same
concept as I started to use for uartps.
But based on what I see in linux-next these patches haven't been merged
there.
That's why I don't understand the reason for this patch.

If you want to see latest state of uartlite you can find it out here.
https://github.com/Xilinx/linux-xlnx

Thanks,
Michal

^ permalink raw reply

* Re: [PATCH] serial-uartlite: Fix null-ptr-deref in ulite_exit
From: Johan Hovold @ 2019-05-23  9:18 UTC (permalink / raw)
  To: Greg KH
  Cc: Johan Hovold, YueHaibing, jslaby, shubhrajyoti.datta,
	linux-kernel, linux-serial, Michal Simek
In-Reply-To: <20190521101059.GB13612@kroah.com>

On Tue, May 21, 2019 at 12:10:59PM +0200, Greg Kroah-Hartman wrote:
> On Fri, May 17, 2019 at 09:55:02AM +0200, Johan Hovold wrote:
> > On Thu, May 16, 2019 at 12:09:31PM +0800, YueHaibing wrote:
> > > If ulite_probe is not called or failed to registed
> > > uart_register_driver, unload the module will call
> > > uart_unregister_driver, which will tigger NULL
> > > pointer dereference like this:
> > > 
> > > BUG: KASAN: null-ptr-deref in tty_unregister_driver+0x19/0x100
> > > Read of size 4 at addr 0000000000000034 by task syz-executor.0/4246
> > 
> > > This patch fix this by moving uart_unregister_driver
> > > to ulite_remove.
> > > 
> > > Reported-by: Hulk Robot <hulkci@huawei.com>
> > > Fixes: 415b43bdb008 ("tty: serial: uartlite: Move uart register to probe")
> > > Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> > > ---
> > >  drivers/tty/serial/uartlite.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
> > > index b8b912b..2e49fb6 100644
> > > --- a/drivers/tty/serial/uartlite.c
> > > +++ b/drivers/tty/serial/uartlite.c
> > > @@ -867,6 +867,7 @@ static int ulite_remove(struct platform_device *pdev)
> > >  	pm_runtime_disable(&pdev->dev);
> > >  	pm_runtime_set_suspended(&pdev->dev);
> > >  	pm_runtime_dont_use_autosuspend(&pdev->dev);
> > > +	uart_unregister_driver(&ulite_uart_driver);
> > 
> > This broken. Consider what happens if you have tho ports registered and
> > you unbind the first.
> > 
> > Someone else sent a fix for this here
> > 
> > 	https://lkml.kernel.org/r/20190514033219.169947-1-wangkefeng.wang@huawei.com
> > 
> > That fix also has some issues, but is still better given the current
> > state this driver is in.
> 
> I'm not taking any of these patches until people agree on what needs to
> be done here :)

Good. :)

> Why is this driver so "special" it is having these types of problems?
> Why can't it do what all other drivers do in this case?

Apparently some vendor-tree hacks has made it into mainline. They're
trying to register one tty/uart driver per physical port which sounds
like a terrible idea. And even the implementation is broken as these bug
reports indicate.

A series was apparently first posted for xilinks_uartps.c, but that one
has not yet been merged AFAICT. A similar series was later posted for
uartlite.c, but only the first half or so got in:

	https://lkml.kernel.org/r/1539685088-13465-1-git-send-email-shubhrajyoti.datta@gmail.com

Actually, it looks like the problems started already with:

	https://lkml.kernel.org/r/1533545534-24458-1-git-send-email-shubhrajyoti.datta@gmail.com

So the first broken commit is

	415b43bdb008 ("tty: serial: uartlite: Move uart register to probe")

I think the offending patches should be reverted, but unfortunately they
may no longer revert cleanly since there were some new features (e.g.
runtime pm) thrown in the mix.

Johan

^ permalink raw reply

* [PATCH 2/2] serial: 8250-mtk: modify uart DMA rx
From: Long Cheng @ 2019-05-23  7:35 UTC (permalink / raw)
  To: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
	Sean Wang, Nicolas Boichat, Matthias Brugger
  Cc: Dan Williams, Greg Kroah-Hartman, Jiri Slaby, Sean Wang,
	dmaengine, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, linux-serial, srv_heupstream, Yingjoe Chen, YT Shen,
	Zhenbao Liu, Long Cheng
In-Reply-To: <1558596909-14084-1-git-send-email-long.cheng@mediatek.com>

Modify uart rx and complete for DMA

Signed-off-by: Long Cheng <long.cheng@mediatek.com>
---
 drivers/tty/serial/8250/8250_mtk.c |   49 +++++++++++++++---------------------
 1 file changed, 20 insertions(+), 29 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index 417c7c8..f470ded 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -47,7 +47,6 @@
 #define MTK_UART_DMA_EN_RX	0x5
 
 #define MTK_UART_ESCAPE_CHAR	0x77	/* Escape char added under sw fc */
-#define MTK_UART_TX_SIZE	UART_XMIT_SIZE
 #define MTK_UART_RX_SIZE	0x8000
 #define MTK_UART_TX_TRIGGER	1
 #define MTK_UART_RX_TRIGGER	MTK_UART_RX_SIZE
@@ -89,28 +88,30 @@ static void mtk8250_dma_rx_complete(void *param)
 	struct mtk8250_data *data = up->port.private_data;
 	struct tty_port *tty_port = &up->port.state->port;
 	struct dma_tx_state state;
+	int copied, total, cnt;
 	unsigned char *ptr;
-	int copied;
 
-	dma_sync_single_for_cpu(dma->rxchan->device->dev, dma->rx_addr,
-				dma->rx_size, DMA_FROM_DEVICE);
+	if (data->rx_status == DMA_RX_SHUTDOWN)
+		return;
 
 	dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state);
+	total = dma->rx_size - state.residue;
+	cnt = total;
 
-	if (data->rx_status == DMA_RX_SHUTDOWN)
-		return;
+	if ((data->rx_pos + cnt) > dma->rx_size)
+		cnt = dma->rx_size - data->rx_pos;
 
-	if ((data->rx_pos + state.residue) <= dma->rx_size) {
-		ptr = (unsigned char *)(data->rx_pos + dma->rx_buf);
-		copied = tty_insert_flip_string(tty_port, ptr, state.residue);
-	} else {
-		ptr = (unsigned char *)(data->rx_pos + dma->rx_buf);
-		copied = tty_insert_flip_string(tty_port, ptr,
-						dma->rx_size - data->rx_pos);
+	ptr = (unsigned char *)(data->rx_pos + dma->rx_buf);
+	copied = tty_insert_flip_string(tty_port, ptr, cnt);
+	data->rx_pos += cnt;
+
+	if (total > cnt) {
 		ptr = (unsigned char *)(dma->rx_buf);
-		copied += tty_insert_flip_string(tty_port, ptr,
-				data->rx_pos + state.residue - dma->rx_size);
+		cnt = total - cnt;
+		copied += tty_insert_flip_string(tty_port, ptr, cnt);
+		data->rx_pos = cnt;
 	}
+
 	up->port.icount.rx += copied;
 
 	tty_flip_buffer_push(tty_port);
@@ -121,9 +122,7 @@ static void mtk8250_dma_rx_complete(void *param)
 static void mtk8250_rx_dma(struct uart_8250_port *up)
 {
 	struct uart_8250_dma *dma = up->dma;
-	struct mtk8250_data *data = up->port.private_data;
 	struct dma_async_tx_descriptor	*desc;
-	struct dma_tx_state	 state;
 
 	desc = dmaengine_prep_slave_single(dma->rxchan, dma->rx_addr,
 					   dma->rx_size, DMA_DEV_TO_MEM,
@@ -138,12 +137,6 @@ static void mtk8250_rx_dma(struct uart_8250_port *up)
 
 	dma->rx_cookie = dmaengine_submit(desc);
 
-	dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state);
-	data->rx_pos = state.residue;
-
-	dma_sync_single_for_device(dma->rxchan->device->dev, dma->rx_addr,
-				   dma->rx_size, DMA_FROM_DEVICE);
-
 	dma_async_issue_pending(dma->rxchan);
 }
 
@@ -156,13 +149,11 @@ static void mtk8250_dma_enable(struct uart_8250_port *up)
 	if (data->rx_status != DMA_RX_START)
 		return;
 
-	dma->rxconf.direction		= DMA_DEV_TO_MEM;
-	dma->rxconf.src_addr_width	= dma->rx_size / 1024;
-	dma->rxconf.src_addr		= dma->rx_addr;
+	dma->rxconf.src_port_window_size	= dma->rx_size;
+	dma->rxconf.src_addr				= dma->rx_addr;
 
-	dma->txconf.direction		= DMA_MEM_TO_DEV;
-	dma->txconf.dst_addr_width	= MTK_UART_TX_SIZE / 1024;
-	dma->txconf.dst_addr		= dma->tx_addr;
+	dma->txconf.dst_port_window_size	= UART_XMIT_SIZE;
+	dma->txconf.dst_addr				= dma->tx_addr;
 
 	serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR |
 		UART_FCR_CLEAR_XMIT);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v13 1/2] arm: dts: mt2712: add uart APDMA to device tree
From: Long Cheng @ 2019-05-23  7:35 UTC (permalink / raw)
  To: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
	Sean Wang, Nicolas Boichat, Matthias Brugger
  Cc: Dan Williams, Greg Kroah-Hartman, Jiri Slaby, Sean Wang,
	dmaengine, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, linux-serial, srv_heupstream, Yingjoe Chen, YT Shen,
	Zhenbao Liu, Long Cheng
In-Reply-To: <1558596909-14084-1-git-send-email-long.cheng@mediatek.com>

1. add uart APDMA controller device node
2. add uart 0/1/2/3/4/5 DMA function

Signed-off-by: Long Cheng <long.cheng@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi |   51 +++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
index 43307ba..a7a7362 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -300,6 +300,9 @@
 		interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_LOW>;
 		clocks = <&baud_clk>, <&sys_clk>;
 		clock-names = "baud", "bus";
+		dmas = <&apdma 10
+			&apdma 11>;
+		dma-names = "tx", "rx";
 		status = "disabled";
 	};
 
@@ -369,6 +372,39 @@
 			 (GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_HIGH)>;
 	};
 
+	apdma: dma-controller@11000400 {
+		compatible = "mediatek,mt2712-uart-dma",
+			     "mediatek,mt6577-uart-dma";
+		reg = <0 0x11000400 0 0x80>,
+		      <0 0x11000480 0 0x80>,
+		      <0 0x11000500 0 0x80>,
+		      <0 0x11000580 0 0x80>,
+		      <0 0x11000600 0 0x80>,
+		      <0 0x11000680 0 0x80>,
+		      <0 0x11000700 0 0x80>,
+		      <0 0x11000780 0 0x80>,
+		      <0 0x11000800 0 0x80>,
+		      <0 0x11000880 0 0x80>,
+		      <0 0x11000900 0 0x80>,
+		      <0 0x11000980 0 0x80>;
+		interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 104 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 105 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 106 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 107 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 108 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 109 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 110 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 111 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 112 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 114 IRQ_TYPE_LEVEL_LOW>;
+		dma-requests = <12>;
+		clocks = <&pericfg CLK_PERI_AP_DMA>;
+		clock-names = "apdma";
+		#dma-cells = <1>;
+	};
+
 	auxadc: adc@11001000 {
 		compatible = "mediatek,mt2712-auxadc";
 		reg = <0 0x11001000 0 0x1000>;
@@ -385,6 +421,9 @@
 		interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
 		clocks = <&baud_clk>, <&sys_clk>;
 		clock-names = "baud", "bus";
+		dmas = <&apdma 0
+			&apdma 1>;
+		dma-names = "tx", "rx";
 		status = "disabled";
 	};
 
@@ -395,6 +434,9 @@
 		interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
 		clocks = <&baud_clk>, <&sys_clk>;
 		clock-names = "baud", "bus";
+		dmas = <&apdma 2
+			&apdma 3>;
+		dma-names = "tx", "rx";
 		status = "disabled";
 	};
 
@@ -405,6 +447,9 @@
 		interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_LOW>;
 		clocks = <&baud_clk>, <&sys_clk>;
 		clock-names = "baud", "bus";
+		dmas = <&apdma 4
+			&apdma 5>;
+		dma-names = "tx", "rx";
 		status = "disabled";
 	};
 
@@ -415,6 +460,9 @@
 		interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_LOW>;
 		clocks = <&baud_clk>, <&sys_clk>;
 		clock-names = "baud", "bus";
+		dmas = <&apdma 6
+			&apdma 7>;
+		dma-names = "tx", "rx";
 		status = "disabled";
 	};
 
@@ -629,6 +677,9 @@
 		interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_LOW>;
 		clocks = <&baud_clk>, <&sys_clk>;
 		clock-names = "baud", "bus";
+		dmas = <&apdma 8
+			&apdma 9>;
+		dma-names = "tx", "rx";
 		status = "disabled";
 	};
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v13 0/2] add uart DMA function
From: Long Cheng @ 2019-05-23  7:35 UTC (permalink / raw)
  To: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
	Sean Wang, Nicolas Boichat, Matthias Brugger
  Cc: Dan Williams, Greg Kroah-Hartman, Jiri Slaby, Sean Wang,
	dmaengine, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, linux-serial, srv_heupstream, Yingjoe Chen, YT Shen,
	Zhenbao Liu, Long Cheng

In Mediatek SOCs, the uart can support DMA function.
Base on DMA engine formwork, we add the DMA code to support uart. And put the code under drivers/dma/mediatek.

This series contains document bindings, Kconfig to control the function enable or not,
device tree including interrupt and dma device node, the code of UART DMA

Changes compared to v12
-rename parameters
-remove direction
Changes compared to v11
-modify TX/RX
-pause function by software
Changes compared to v10
-modify DMA tx status function
-modify 8250_mtk for DMA rx
-add notes to binding Document.
Changes compared to v9
-rename dt-bindings file
-remove direction from device_config
-simplified code
Changes compared to v8
-revise missing items
Changes compared to v7:
-modify apdma uart tx
Changes compared to v6:
-Correct spelling
Changes compared to v5:
-move 'requst irqs' to alloc channel
-remove tasklet.
Changes compared to v4:
-modify Kconfig depends on.
Changes compared to v3:
-fix CONFIG_PM, will cause build fail
Changes compared to v2:
-remove unimportant parameters
-instead of cookie, use APIs of virtual channel.
-use of_dma_xlate_by_chan_id.
Changes compared to v1:
-mian revised file, 8250_mtk_dma.c
--parameters renamed for standard
--remove atomic operation

Long Cheng (2):
  arm: dts: mt2712: add uart APDMA to device tree
  serial: 8250-mtk: modify uart DMA rx

 arch/arm64/boot/dts/mediatek/mt2712e.dtsi |   51 +++++++++++++++++++++++++++++
 drivers/tty/serial/8250/8250_mtk.c        |   49 +++++++++++----------------
 2 files changed, 71 insertions(+), 29 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* [PATCH 6/6] arm64: defconfig: Enable TI's J721E SoC platform
From: Nishanth Menon @ 2019-05-22 16:19 UTC (permalink / raw)
  To: Arnd Bergmann, Olof Johansson, Santosh Shilimkar, Will Deacon,
	Catalin Marinas, Greg Kroah-Hartman, Mark Rutland, Rob Herring
  Cc: linux-serial, linux-kernel, devicetree, linux-arm-kernel,
	Tony Lindgren, Russell King, Tero Kristo, Nishanth Menon
In-Reply-To: <20190522161921.20750-1-nm@ti.com>

Enable J721E SoC support from TI.

Signed-off-by: Nishanth Menon <nm@ti.com>
---

NOTE:
 - I will resubmit this patch (defconfig update) separately once again once
   patches 1-7 hit the next tree or for 5.3-rc2 which ever is convenient.

 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 4d583514258c..83a509dc247d 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -696,6 +696,7 @@ CONFIG_ARCH_TEGRA_210_SOC=y
 CONFIG_ARCH_TEGRA_186_SOC=y
 CONFIG_ARCH_TEGRA_194_SOC=y
 CONFIG_ARCH_K3_AM6_SOC=y
+CONFIG_ARCH_K3_J721E_SOC=y
 CONFIG_SOC_TI=y
 CONFIG_TI_SCI_PM_DOMAINS=y
 CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND=y
-- 
2.21.0.777.g83232e38648b

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox