[PATCH v5 00/11] serial: 8250: dw: RZN1 DMA support

[PATCH v5 00/11] serial: 8250: dw: RZN1 DMA support

[Miquel Raynal]

Hello,

Support for the RZN1 DMA engine allows us adapt a little bit the 8250 DW
UART driver with to bring DMA support for this SoC.

This short series applies on top of the series bringing RZN1 DMA
support, currently on its v10, see [1]. Technically speaking, only the DT
patch needs to be applied after [1]. The other patches can come in at
any moment, because if no "dmas" property is provided in the DT, DMA
support will simply be ignored.

[1] https://lore.kernel.org/dmaengine/20220412193936.63355-1-miquel.raynal@bootlin.com/T/#t

Thanks,
Miquèl

Changes in v5:
* Collected the various tags.
* Updated the style to fit Andy's requests.
* Moved the usr register offset to pdata as advised by Andy.
* Moved the addition of the property.h header to the right patch.
* Return earlier from the IRQ handler whenever possible.
* Moved the dw8250_readl/writel_ext() helpers to dwlib.h so that they
  could be reused by the patch adding rzn1 support.

Changes in v4:
* Squashed the patch changing the quirks type with the patch introducing
  the pdata structure. This way I don't need to go through the uintptr_t
  step. 
* Added a pdata stub to the DW compatible. This way "if (pdata)" checks
  are no longer needed and we can avoid ternary operators disliked by
  Greg.
* Saved the pdata structure pointer inside the per-device structure to
  avoid calling the same device property heplper over and over again. 
* Changed the style a bit to fit Andy's requirements.

Changes in v3:
* Added Geert's Reviewed-by's.
* Used forward declaration in dwlib.h when relevant.
* Reordered the headers when necessary.
* Added a patch changing the quirks type from unsigned long to unsigned
  int as advised by Andy.
* s/up->port.dev/p->dev/ when relevant.
* Added a debug message in dwlib when no CPR is available.
* Dropped the CPR_DMA_EXTRA bit addition: we assume DMA is there.
* Changed the prefix of certain definitions.

Changes in v2:
* Rebased on top of Emil's patches. Added platform data structures
  instead of raw quirk integers in order to provide a CPR value.
* Added includes in dwlib.h even though it's not particularly useful, it
  may help preventing a build error later on if we ever decide to include
  this file from another location.
* Dropped the call to ->tx_dma and instead implemented a callback that
  can be called from serial8250_tx/rx_dma.
* Used the device API instead of the of API.
* Changed the logic about DMA capabilities to avoid breaking existing
  designs.
* Introduced a new quirk related to the flow-control feature of the
  RZ/N1 version of the UART controller when used with DMA.
* Re-arranged the entire series as advised by Andy and Geert.
* Added several preparation patches to ease the review of various
  functional changes.

Miquel Raynal (8):
  serial: 8250: dw: Use the device API
  serial: 8250: dw: Create a more generic platform data structure
  serial: 8250: dw: Move the USR register to pdata
  serial: 8250: dw: Allow to use a fallback CPR value if not synthesized
  serial: 8250: dma: Allow driver operations before starting DMA
    transfers
  serial: 8250: dw: Introduce an rx_timeout variable in the IRQ path
  serial: 8250: dw: Move the IO accessors to 8250_dwlib.h
  ARM: dts: r9a06g032: Fill the UART DMA properties

Phil Edworthy (3):
  serial: 8250: dw: Move definitions to the shared header
  serial: 8250: dw: Add support for DMA flow controlling devices
  serial: 8250: dw: Improve RZN1 support

 arch/arm/boot/dts/r9a06g032.dtsi     |  15 +++
 drivers/tty/serial/8250/8250.h       |  18 ++++
 drivers/tty/serial/8250/8250_dma.c   |   4 +
 drivers/tty/serial/8250/8250_dw.c    | 135 ++++++++++++++++++++-------
 drivers/tty/serial/8250/8250_dwlib.c |  26 ++----
 drivers/tty/serial/8250/8250_dwlib.h |  48 ++++++++++
 6 files changed, 195 insertions(+), 51 deletions(-)

-- 
2.27.0

[PATCH v5 01/11] serial: 8250: dw: Move definitions to the shared header

[Miquel Raynal]

From: Phil Edworthy <phil.edworthy@renesas.com>

Move the per-device structure and a helper out of the main .c file, into
a shared header as they will both be reused from another .c file.

There is no functional change.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
[miquel.raynal@bootlin.com: Extracted from a bigger change]
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_dw.c    | 21 ---------------------
 drivers/tty/serial/8250/8250_dwlib.h | 26 ++++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 96a62e95726b..1666041513a5 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -42,27 +42,6 @@
 #define DW_UART_QUIRK_ARMADA_38X	BIT(1)
 #define DW_UART_QUIRK_SKIP_SET_RATE	BIT(2)
 
-struct dw8250_data {
-	struct dw8250_port_data	data;
-
-	u8			usr_reg;
-	int			msr_mask_on;
-	int			msr_mask_off;
-	struct clk		*clk;
-	struct clk		*pclk;
-	struct notifier_block	clk_notifier;
-	struct work_struct	clk_work;
-	struct reset_control	*rst;
-
-	unsigned int		skip_autocfg:1;
-	unsigned int		uart_16550_compatible:1;
-};
-
-static inline struct dw8250_data *to_dw8250_data(struct dw8250_port_data *data)
-{
-	return container_of(data, struct dw8250_data, data);
-}
-
 static inline struct dw8250_data *clk_to_dw8250_data(struct notifier_block *nb)
 {
 	return container_of(nb, struct dw8250_data, clk_notifier);
diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
index 83d528e5cc21..72e7dbcccad0 100644
--- a/drivers/tty/serial/8250/8250_dwlib.h
+++ b/drivers/tty/serial/8250/8250_dwlib.h
@@ -1,10 +1,15 @@
 /* SPDX-License-Identifier: GPL-2.0+ */
 /* Synopsys DesignWare 8250 library header file. */
 
+#include <linux/notifier.h>
 #include <linux/types.h>
+#include <linux/workqueue.h>
 
 #include "8250.h"
 
+struct clk;
+struct reset_control;
+
 struct dw8250_port_data {
 	/* Port properties */
 	int			line;
@@ -16,5 +21,26 @@ struct dw8250_port_data {
 	u8			dlf_size;
 };
 
+struct dw8250_data {
+	struct dw8250_port_data	data;
+
+	u8			usr_reg;
+	int			msr_mask_on;
+	int			msr_mask_off;
+	struct clk		*clk;
+	struct clk		*pclk;
+	struct notifier_block	clk_notifier;
+	struct work_struct	clk_work;
+	struct reset_control	*rst;
+
+	unsigned int		skip_autocfg:1;
+	unsigned int		uart_16550_compatible:1;
+};
+
 void dw8250_do_set_termios(struct uart_port *p, struct ktermios *termios, struct ktermios *old);
 void dw8250_setup_port(struct uart_port *p);
+
+static inline struct dw8250_data *to_dw8250_data(struct dw8250_port_data *data)
+{
+	return container_of(data, struct dw8250_data, data);
+}
-- 
2.27.0

[PATCH v5 02/11] serial: 8250: dw: Use the device API

[Miquel Raynal]

Use the device API instead of the of_* API.
While at it move this operation outside of the if block to reduce the
indentation level.

Cc: Emil Renner Berthing <kernel@esmil.dk>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_dw.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 1666041513a5..e9f8b65563a1 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -19,6 +19,7 @@
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/workqueue.h>
 #include <linux/notifier.h>
 #include <linux/slab.h>
@@ -371,9 +372,9 @@ static bool dw8250_idma_filter(struct dma_chan *chan, void *param)
 static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
 {
 	struct device_node *np = p->dev->of_node;
+	unsigned long quirks = (unsigned long)device_get_match_data(p->dev);
 
 	if (np) {
-		unsigned long quirks = (unsigned long)of_device_get_match_data(p->dev);
 		int id;
 
 		/* get index of serial line, if found in DT aliases */
-- 
2.27.0

[PATCH v5 03/11] serial: 8250: dw: Create a more generic platform data structure

[Miquel Raynal]

Before adding more platform data information, let's turn the quirks
information as being a member of a wider structure. More fields will be
added later.

At the same time, change the quirks type to unsigned int, as its size
won't change between setups and we don't really need this variable to be
more than a few bits wide anyway.

Provide a stub to the compatibles without quirks to simplify handling.
Keep two different empty structure for the base DW compatible and the
Renesas one because the latter will soon be fulfilled anyway.

Cc: Emil Renner Berthing <kernel@esmil.dk>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_dw.c    | 33 ++++++++++++++++++++++------
 drivers/tty/serial/8250/8250_dwlib.h |  5 +++++
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index e9f8b65563a1..93b112f3bc49 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -372,7 +372,7 @@ static bool dw8250_idma_filter(struct dma_chan *chan, void *param)
 static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
 {
 	struct device_node *np = p->dev->of_node;
-	unsigned long quirks = (unsigned long)device_get_match_data(p->dev);
+	unsigned int quirks = data->pdata->quirks;
 
 	if (np) {
 		int id;
@@ -463,6 +463,7 @@ static int dw8250_probe(struct platform_device *pdev)
 
 	data->data.dma.fn = dw8250_fallback_dma_filter;
 	data->usr_reg = DW_UART_USR;
+	data->pdata = device_get_match_data(p->dev);
 	p->private_data = &data->data;
 
 	data->uart_16550_compatible = device_property_read_bool(dev,
@@ -679,13 +680,31 @@ static const struct dev_pm_ops dw8250_pm_ops = {
 	SET_RUNTIME_PM_OPS(dw8250_runtime_suspend, dw8250_runtime_resume, NULL)
 };
 
+static const struct dw8250_platform_data dw8250_dw_apb = {
+};
+
+static const struct dw8250_platform_data dw8250_octeon_3860_data = {
+	.quirks = DW_UART_QUIRK_OCTEON,
+};
+
+static const struct dw8250_platform_data dw8250_armada_38x_data = {
+	.quirks = DW_UART_QUIRK_ARMADA_38X,
+};
+
+static const struct dw8250_platform_data dw8250_renesas_rzn1_data = {
+};
+
+static const struct dw8250_platform_data dw8250_starfive_jh7100_data = {
+	.quirks = DW_UART_QUIRK_SKIP_SET_RATE,
+};
+
 static const struct of_device_id dw8250_of_match[] = {
-	{ .compatible = "snps,dw-apb-uart" },
-	{ .compatible = "cavium,octeon-3860-uart", .data = (void *)DW_UART_QUIRK_OCTEON },
-	{ .compatible = "marvell,armada-38x-uart", .data = (void *)DW_UART_QUIRK_ARMADA_38X },
-	{ .compatible = "renesas,rzn1-uart" },
-	{ .compatible = "starfive,jh7100-hsuart",  .data = (void *)DW_UART_QUIRK_SKIP_SET_RATE },
-	{ .compatible = "starfive,jh7100-uart",    .data = (void *)DW_UART_QUIRK_SKIP_SET_RATE },
+	{ .compatible = "snps,dw-apb-uart", .data = &dw8250_dw_apb },
+	{ .compatible = "cavium,octeon-3860-uart", .data = &dw8250_octeon_3860_data },
+	{ .compatible = "marvell,armada-38x-uart", .data = &dw8250_armada_38x_data },
+	{ .compatible = "renesas,rzn1-uart", .data = &dw8250_renesas_rzn1_data },
+	{ .compatible = "starfive,jh7100-hsuart", .data = &dw8250_starfive_jh7100_data },
+	{ .compatible = "starfive,jh7100-uart", .data = &dw8250_starfive_jh7100_data },
 	{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, dw8250_of_match);
diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
index 72e7dbcccad0..68bb81bee660 100644
--- a/drivers/tty/serial/8250/8250_dwlib.h
+++ b/drivers/tty/serial/8250/8250_dwlib.h
@@ -21,8 +21,13 @@ struct dw8250_port_data {
 	u8			dlf_size;
 };
 
+struct dw8250_platform_data {
+	unsigned int quirks;
+};
+
 struct dw8250_data {
 	struct dw8250_port_data	data;
+	const struct dw8250_platform_data *pdata;
 
 	u8			usr_reg;
 	int			msr_mask_on;
-- 
2.27.0

[PATCH v5 04/11] serial: 8250: dw: Move the USR register to pdata

[Miquel Raynal]

This offset is a good candidate to pdata's because it changes depending
on the vendor implementation. Let's move the usr_reg entry from regular
to pdata. This way we can drop initializing it at run time.

Let's also use a define for it instead of defining only the default
value.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/tty/serial/8250/8250_dw.c    | 11 ++++++++---
 drivers/tty/serial/8250/8250_dwlib.h |  2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 93b112f3bc49..babf5dc597a8 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -35,6 +35,8 @@
 /* Offsets for the DesignWare specific registers */
 #define DW_UART_USR	0x1f /* UART Status Register */
 
+#define OCTEON_UART_USR	0x27 /* UART Status Register */
+
 /* DesignWare specific register fields */
 #define DW_UART_MCR_SIRE		BIT(6)
 
@@ -251,7 +253,7 @@ static int dw8250_handle_irq(struct uart_port *p)
 
 	if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
 		/* Clear the USR */
-		(void)p->serial_in(p, d->usr_reg);
+		(void)p->serial_in(p, d->pdata->usr_reg);
 
 		return 1;
 	}
@@ -387,7 +389,6 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
 			p->serial_out = dw8250_serial_outq;
 			p->flags = UPF_SKIP_TEST | UPF_SHARE_IRQ | UPF_FIXED_TYPE;
 			p->type = PORT_OCTEON;
-			data->usr_reg = 0x27;
 			data->skip_autocfg = true;
 		}
 #endif
@@ -462,7 +463,6 @@ static int dw8250_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	data->data.dma.fn = dw8250_fallback_dma_filter;
-	data->usr_reg = DW_UART_USR;
 	data->pdata = device_get_match_data(p->dev);
 	p->private_data = &data->data;
 
@@ -681,20 +681,25 @@ static const struct dev_pm_ops dw8250_pm_ops = {
 };
 
 static const struct dw8250_platform_data dw8250_dw_apb = {
+	.usr_reg = DW_UART_USR,
 };
 
 static const struct dw8250_platform_data dw8250_octeon_3860_data = {
+	.usr_reg = OCTEON_UART_USR,
 	.quirks = DW_UART_QUIRK_OCTEON,
 };
 
 static const struct dw8250_platform_data dw8250_armada_38x_data = {
+	.usr_reg = DW_UART_USR,
 	.quirks = DW_UART_QUIRK_ARMADA_38X,
 };
 
 static const struct dw8250_platform_data dw8250_renesas_rzn1_data = {
+	.usr_reg = DW_UART_USR,
 };
 
 static const struct dw8250_platform_data dw8250_starfive_jh7100_data = {
+	.usr_reg = DW_UART_USR,
 	.quirks = DW_UART_QUIRK_SKIP_SET_RATE,
 };
 
diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
index 68bb81bee660..0df6baa6eaee 100644
--- a/drivers/tty/serial/8250/8250_dwlib.h
+++ b/drivers/tty/serial/8250/8250_dwlib.h
@@ -22,6 +22,7 @@ struct dw8250_port_data {
 };
 
 struct dw8250_platform_data {
+	u8 usr_reg;
 	unsigned int quirks;
 };
 
@@ -29,7 +30,6 @@ struct dw8250_data {
 	struct dw8250_port_data	data;
 	const struct dw8250_platform_data *pdata;
 
-	u8			usr_reg;
 	int			msr_mask_on;
 	int			msr_mask_off;
 	struct clk		*clk;
-- 
2.27.0

Re: [PATCH v5 04/11] serial: 8250: dw: Move the USR register to pdata

[Andy Shevchenko]

On Wed, Apr 13, 2022 at 09:51:34AM +0200, Miquel Raynal wrote:
> This offset is a good candidate to pdata's because it changes depending
> on the vendor implementation. Let's move the usr_reg entry from regular
> to pdata. This way we can drop initializing it at run time.
> 
> Let's also use a define for it instead of defining only the default
> value.

Yep, thanks!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/tty/serial/8250/8250_dw.c    | 11 ++++++++---
>  drivers/tty/serial/8250/8250_dwlib.h |  2 +-
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index 93b112f3bc49..babf5dc597a8 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -35,6 +35,8 @@
>  /* Offsets for the DesignWare specific registers */
>  #define DW_UART_USR	0x1f /* UART Status Register */
>  
> +#define OCTEON_UART_USR	0x27 /* UART Status Register */
> +
>  /* DesignWare specific register fields */
>  #define DW_UART_MCR_SIRE		BIT(6)
>  
> @@ -251,7 +253,7 @@ static int dw8250_handle_irq(struct uart_port *p)
>  
>  	if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
>  		/* Clear the USR */
> -		(void)p->serial_in(p, d->usr_reg);
> +		(void)p->serial_in(p, d->pdata->usr_reg);
>  
>  		return 1;
>  	}
> @@ -387,7 +389,6 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
>  			p->serial_out = dw8250_serial_outq;
>  			p->flags = UPF_SKIP_TEST | UPF_SHARE_IRQ | UPF_FIXED_TYPE;
>  			p->type = PORT_OCTEON;
> -			data->usr_reg = 0x27;
>  			data->skip_autocfg = true;
>  		}
>  #endif
> @@ -462,7 +463,6 @@ static int dw8250_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	data->data.dma.fn = dw8250_fallback_dma_filter;
> -	data->usr_reg = DW_UART_USR;
>  	data->pdata = device_get_match_data(p->dev);
>  	p->private_data = &data->data;
>  
> @@ -681,20 +681,25 @@ static const struct dev_pm_ops dw8250_pm_ops = {
>  };
>  
>  static const struct dw8250_platform_data dw8250_dw_apb = {
> +	.usr_reg = DW_UART_USR,
>  };
>  
>  static const struct dw8250_platform_data dw8250_octeon_3860_data = {
> +	.usr_reg = OCTEON_UART_USR,
>  	.quirks = DW_UART_QUIRK_OCTEON,
>  };
>  
>  static const struct dw8250_platform_data dw8250_armada_38x_data = {
> +	.usr_reg = DW_UART_USR,
>  	.quirks = DW_UART_QUIRK_ARMADA_38X,
>  };
>  
>  static const struct dw8250_platform_data dw8250_renesas_rzn1_data = {
> +	.usr_reg = DW_UART_USR,
>  };
>  
>  static const struct dw8250_platform_data dw8250_starfive_jh7100_data = {
> +	.usr_reg = DW_UART_USR,
>  	.quirks = DW_UART_QUIRK_SKIP_SET_RATE,
>  };
>  
> diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
> index 68bb81bee660..0df6baa6eaee 100644
> --- a/drivers/tty/serial/8250/8250_dwlib.h
> +++ b/drivers/tty/serial/8250/8250_dwlib.h
> @@ -22,6 +22,7 @@ struct dw8250_port_data {
>  };
>  
>  struct dw8250_platform_data {
> +	u8 usr_reg;
>  	unsigned int quirks;
>  };
>  
> @@ -29,7 +30,6 @@ struct dw8250_data {
>  	struct dw8250_port_data	data;
>  	const struct dw8250_platform_data *pdata;
>  
> -	u8			usr_reg;
>  	int			msr_mask_on;
>  	int			msr_mask_off;
>  	struct clk		*clk;
> -- 
> 2.27.0
> 

-- 
With Best Regards,
Andy Shevchenko

[PATCH v5 05/11] serial: 8250: dw: Allow to use a fallback CPR value if not synthesized

[Miquel Raynal]

DW UART controllers can be synthesized without the CPR register.
In this case, allow to the platform information to provide a CPR value.

Co-developed-by: Phil Edworthy <phil.edworthy@renesas.com>
Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/tty/serial/8250/8250_dwlib.c | 10 +++++++---
 drivers/tty/serial/8250/8250_dwlib.h |  1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
index 622d3b0d89e7..22154b5848d5 100644
--- a/drivers/tty/serial/8250/8250_dwlib.c
+++ b/drivers/tty/serial/8250/8250_dwlib.c
@@ -89,6 +89,8 @@ EXPORT_SYMBOL_GPL(dw8250_do_set_termios);
 
 void dw8250_setup_port(struct uart_port *p)
 {
+	struct dw8250_port_data *pd = p->private_data;
+	struct dw8250_data *data = to_dw8250_data(pd);
 	struct uart_8250_port *up = up_to_u8250p(p);
 	u32 reg;
 
@@ -108,14 +110,16 @@ void dw8250_setup_port(struct uart_port *p)
 	dw8250_writel_ext(p, DW_UART_DLF, 0);
 
 	if (reg) {
-		struct dw8250_port_data *d = p->private_data;
-
-		d->dlf_size = fls(reg);
+		pd->dlf_size = fls(reg);
 		p->get_divisor = dw8250_get_divisor;
 		p->set_divisor = dw8250_set_divisor;
 	}
 
 	reg = dw8250_readl_ext(p, DW_UART_CPR);
+	if (!reg) {
+		reg = data->pdata->cpr;
+		dev_dbg(p->dev, "CPR is not available, using 0x%08x instead\n", reg);
+	}
 	if (!reg)
 		return;
 
diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
index 0df6baa6eaee..412f4e83e0b3 100644
--- a/drivers/tty/serial/8250/8250_dwlib.h
+++ b/drivers/tty/serial/8250/8250_dwlib.h
@@ -24,6 +24,7 @@ struct dw8250_port_data {
 struct dw8250_platform_data {
 	u8 usr_reg;
 	unsigned int quirks;
+	u32 cpr;
 };
 
 struct dw8250_data {
-- 
2.27.0

Re: [PATCH v5 05/11] serial: 8250: dw: Allow to use a fallback CPR value if not synthesized

[Andy Shevchenko]

On Wed, Apr 13, 2022 at 09:51:35AM +0200, Miquel Raynal wrote:
> DW UART controllers can be synthesized without the CPR register.
> In this case, allow to the platform information to provide a CPR value.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Some nit-picks below.

> Co-developed-by: Phil Edworthy <phil.edworthy@renesas.com>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/tty/serial/8250/8250_dwlib.c | 10 +++++++---
>  drivers/tty/serial/8250/8250_dwlib.h |  1 +
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
> index 622d3b0d89e7..22154b5848d5 100644
> --- a/drivers/tty/serial/8250/8250_dwlib.c
> +++ b/drivers/tty/serial/8250/8250_dwlib.c
> @@ -89,6 +89,8 @@ EXPORT_SYMBOL_GPL(dw8250_do_set_termios);
>  
>  void dw8250_setup_port(struct uart_port *p)
>  {
> +	struct dw8250_port_data *pd = p->private_data;
> +	struct dw8250_data *data = to_dw8250_data(pd);
>  	struct uart_8250_port *up = up_to_u8250p(p);
>  	u32 reg;
>  
> @@ -108,14 +110,16 @@ void dw8250_setup_port(struct uart_port *p)
>  	dw8250_writel_ext(p, DW_UART_DLF, 0);
>  
>  	if (reg) {
> -		struct dw8250_port_data *d = p->private_data;
> -
> -		d->dlf_size = fls(reg);
> +		pd->dlf_size = fls(reg);
>  		p->get_divisor = dw8250_get_divisor;
>  		p->set_divisor = dw8250_set_divisor;
>  	}
>  
>  	reg = dw8250_readl_ext(p, DW_UART_CPR);
> +	if (!reg) {
> +		reg = data->pdata->cpr;
> +		dev_dbg(p->dev, "CPR is not available, using 0x%08x instead\n", reg);
> +	}
>  	if (!reg)
>  		return;
>  
> diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
> index 0df6baa6eaee..412f4e83e0b3 100644
> --- a/drivers/tty/serial/8250/8250_dwlib.h
> +++ b/drivers/tty/serial/8250/8250_dwlib.h
> @@ -24,6 +24,7 @@ struct dw8250_port_data {
>  struct dw8250_platform_data {
>  	u8 usr_reg;
>  	unsigned int quirks;
> +	u32 cpr;

I would probably name it cpr_val (to be in align with _reg above) and move it
after the usr_reg member. But it's minor, up to you.

>  };
>  
>  struct dw8250_data {

-- 
With Best Regards,
Andy Shevchenko

[PATCH v5 06/11] serial: 8250: dma: Allow driver operations before starting DMA transfers

[Miquel Raynal]

One situation where this could be used is when configuring the UART
controller to be the DMA flow controller. This is a typical case where
the driver might need to program a few more registers before starting a
DMA transfer. Provide the necessary infrastructure to support this
case.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250.h     | 18 ++++++++++++++++++
 drivers/tty/serial/8250/8250_dma.c |  4 ++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index db784ace25d8..d19f24e4d13e 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -17,6 +17,8 @@
 struct uart_8250_dma {
 	int (*tx_dma)(struct uart_8250_port *p);
 	int (*rx_dma)(struct uart_8250_port *p);
+	void (*prepare_tx_dma)(struct uart_8250_port *p);
+	void (*prepare_rx_dma)(struct uart_8250_port *p);
 
 	/* Filter function */
 	dma_filter_fn		fn;
@@ -301,6 +303,22 @@ extern int serial8250_rx_dma(struct uart_8250_port *);
 extern void serial8250_rx_dma_flush(struct uart_8250_port *);
 extern int serial8250_request_dma(struct uart_8250_port *);
 extern void serial8250_release_dma(struct uart_8250_port *);
+
+static inline void serial8250_do_prepare_tx_dma(struct uart_8250_port *p)
+{
+	struct uart_8250_dma *dma = p->dma;
+
+	if (dma->prepare_tx_dma)
+		dma->prepare_tx_dma(p);
+}
+
+static inline void serial8250_do_prepare_rx_dma(struct uart_8250_port *p)
+{
+	struct uart_8250_dma *dma = p->dma;
+
+	if (dma->prepare_rx_dma)
+		dma->prepare_rx_dma(p);
+}
 #else
 static inline int serial8250_tx_dma(struct uart_8250_port *p)
 {
diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index b3c3f7e5851a..1bdc8d6432fe 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -86,6 +86,8 @@ int serial8250_tx_dma(struct uart_8250_port *p)
 
 	dma->tx_size = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
 
+	serial8250_do_prepare_tx_dma(p);
+
 	desc = dmaengine_prep_slave_single(dma->txchan,
 					   dma->tx_addr + xmit->tail,
 					   dma->tx_size, DMA_MEM_TO_DEV,
@@ -123,6 +125,8 @@ int serial8250_rx_dma(struct uart_8250_port *p)
 	if (dma->rx_running)
 		return 0;
 
+	serial8250_do_prepare_rx_dma(p);
+
 	desc = dmaengine_prep_slave_single(dma->rxchan, dma->rx_addr,
 					   dma->rx_size, DMA_DEV_TO_MEM,
 					   DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
-- 
2.27.0

[PATCH v5 07/11] serial: 8250: dw: Introduce an rx_timeout variable in the IRQ path

[Miquel Raynal]

In a next change we are going to need the same Rx timeout condition as
we already have in the IRQ handling code. Let's just create a boolean to
clarify what this operation does before reusing it.

There is no functional change.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_dw.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index babf5dc597a8..238bcdf1bab0 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -225,6 +225,7 @@ static int dw8250_handle_irq(struct uart_port *p)
 	struct uart_8250_port *up = up_to_u8250p(p);
 	struct dw8250_data *d = to_dw8250_data(p->private_data);
 	unsigned int iir = p->serial_in(p, UART_IIR);
+	bool rx_timeout = (iir & 0x3f) == UART_IIR_RX_TIMEOUT;
 	unsigned int status;
 	unsigned long flags;
 
@@ -238,7 +239,7 @@ static int dw8250_handle_irq(struct uart_port *p)
 	 * This problem has only been observed so far when not in DMA mode
 	 * so we limit the workaround only to non-DMA mode.
 	 */
-	if (!up->dma && ((iir & 0x3f) == UART_IIR_RX_TIMEOUT)) {
+	if (!up->dma && rx_timeout) {
 		spin_lock_irqsave(&p->lock, flags);
 		status = p->serial_in(p, UART_LSR);
 
-- 
2.27.0

[PATCH v5 08/11] serial: 8250: dw: Move the IO accessors to 8250_dwlib.h

[Miquel Raynal]

These accessors should be used instead of the regular readl/writel()
helpers. In order to use them also from 8250_dw.c in this directory,
move the helpers to 8250_dwlib.h

There is no functional change.

There is no need for declaring `struct uart_port` or even UPIO_MEM32BE
which both are already included in the 8250_dwlib.h header by 8250.h.

Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/tty/serial/8250/8250_dwlib.c | 16 ----------------
 drivers/tty/serial/8250/8250_dwlib.h | 16 ++++++++++++++++
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
index 22154b5848d5..02b6422c26c7 100644
--- a/drivers/tty/serial/8250/8250_dwlib.c
+++ b/drivers/tty/serial/8250/8250_dwlib.c
@@ -3,7 +3,6 @@
 
 #include <linux/bitops.h>
 #include <linux/device.h>
-#include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/serial_8250.h>
 #include <linux/serial_core.h>
@@ -32,21 +31,6 @@
 /* Helper for FIFO size calculation */
 #define DW_UART_CPR_FIFO_SIZE(a)	(((a >> 16) & 0xff) * 16)
 
-static inline u32 dw8250_readl_ext(struct uart_port *p, int offset)
-{
-	if (p->iotype == UPIO_MEM32BE)
-		return ioread32be(p->membase + offset);
-	return readl(p->membase + offset);
-}
-
-static inline void dw8250_writel_ext(struct uart_port *p, int offset, u32 reg)
-{
-	if (p->iotype == UPIO_MEM32BE)
-		iowrite32be(reg, p->membase + offset);
-	else
-		writel(reg, p->membase + offset);
-}
-
 /*
  * divisor = div(I) + div(F)
  * "I" means integer, "F" means fractional
diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
index 412f4e83e0b3..b2a7cff671bf 100644
--- a/drivers/tty/serial/8250/8250_dwlib.h
+++ b/drivers/tty/serial/8250/8250_dwlib.h
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0+ */
 /* Synopsys DesignWare 8250 library header file. */
 
+#include <linux/io.h>
 #include <linux/notifier.h>
 #include <linux/types.h>
 #include <linux/workqueue.h>
@@ -50,3 +51,18 @@ static inline struct dw8250_data *to_dw8250_data(struct dw8250_port_data *data)
 {
 	return container_of(data, struct dw8250_data, data);
 }
+
+static inline u32 dw8250_readl_ext(struct uart_port *p, int offset)
+{
+	if (p->iotype == UPIO_MEM32BE)
+		return ioread32be(p->membase + offset);
+	return readl(p->membase + offset);
+}
+
+static inline void dw8250_writel_ext(struct uart_port *p, int offset, u32 reg)
+{
+	if (p->iotype == UPIO_MEM32BE)
+		iowrite32be(reg, p->membase + offset);
+	else
+		writel(reg, p->membase + offset);
+}
-- 
2.27.0

Re: [PATCH v5 08/11] serial: 8250: dw: Move the IO accessors to 8250_dwlib.h

[Andy Shevchenko]

On Wed, Apr 13, 2022 at 09:51:38AM +0200, Miquel Raynal wrote:
> These accessors should be used instead of the regular readl/writel()
> helpers. In order to use them also from 8250_dw.c in this directory,
> move the helpers to 8250_dwlib.h
> 
> There is no functional change.
> 
> There is no need for declaring `struct uart_port` or even UPIO_MEM32BE
> which both are already included in the 8250_dwlib.h header by 8250.h.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/tty/serial/8250/8250_dwlib.c | 16 ----------------
>  drivers/tty/serial/8250/8250_dwlib.h | 16 ++++++++++++++++
>  2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
> index 22154b5848d5..02b6422c26c7 100644
> --- a/drivers/tty/serial/8250/8250_dwlib.c
> +++ b/drivers/tty/serial/8250/8250_dwlib.c
> @@ -3,7 +3,6 @@
>  
>  #include <linux/bitops.h>
>  #include <linux/device.h>
> -#include <linux/io.h>
>  #include <linux/kernel.h>
>  #include <linux/serial_8250.h>
>  #include <linux/serial_core.h>
> @@ -32,21 +31,6 @@
>  /* Helper for FIFO size calculation */
>  #define DW_UART_CPR_FIFO_SIZE(a)	(((a >> 16) & 0xff) * 16)
>  
> -static inline u32 dw8250_readl_ext(struct uart_port *p, int offset)
> -{
> -	if (p->iotype == UPIO_MEM32BE)
> -		return ioread32be(p->membase + offset);
> -	return readl(p->membase + offset);
> -}
> -
> -static inline void dw8250_writel_ext(struct uart_port *p, int offset, u32 reg)
> -{
> -	if (p->iotype == UPIO_MEM32BE)
> -		iowrite32be(reg, p->membase + offset);
> -	else
> -		writel(reg, p->membase + offset);
> -}
> -
>  /*
>   * divisor = div(I) + div(F)
>   * "I" means integer, "F" means fractional
> diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
> index 412f4e83e0b3..b2a7cff671bf 100644
> --- a/drivers/tty/serial/8250/8250_dwlib.h
> +++ b/drivers/tty/serial/8250/8250_dwlib.h
> @@ -1,6 +1,7 @@
>  /* SPDX-License-Identifier: GPL-2.0+ */
>  /* Synopsys DesignWare 8250 library header file. */
>  
> +#include <linux/io.h>
>  #include <linux/notifier.h>
>  #include <linux/types.h>
>  #include <linux/workqueue.h>
> @@ -50,3 +51,18 @@ static inline struct dw8250_data *to_dw8250_data(struct dw8250_port_data *data)
>  {
>  	return container_of(data, struct dw8250_data, data);
>  }
> +
> +static inline u32 dw8250_readl_ext(struct uart_port *p, int offset)
> +{
> +	if (p->iotype == UPIO_MEM32BE)
> +		return ioread32be(p->membase + offset);
> +	return readl(p->membase + offset);
> +}
> +
> +static inline void dw8250_writel_ext(struct uart_port *p, int offset, u32 reg)
> +{
> +	if (p->iotype == UPIO_MEM32BE)
> +		iowrite32be(reg, p->membase + offset);
> +	else
> +		writel(reg, p->membase + offset);
> +}
> -- 
> 2.27.0
> 

-- 
With Best Regards,
Andy Shevchenko

[PATCH v5 09/11] serial: 8250: dw: Add support for DMA flow controlling devices

[Miquel Raynal]

From: Phil Edworthy <phil.edworthy@renesas.com>

DW based controllers like the one on Renesas RZ/N1 must be programmed as
flow controllers when using DMA.

* Table 11.45 of the system manual, "Flow Control Combinations", states
  that using UART with DMA requires setting the DMA in the peripheral
  flow controller mode regardless of the direction.

* Chapter 11.6.1.3 of the system manual, "Basic Interface Definitions",
  explains that the burst size in the above case must be configured in
  the peripheral's register DEST/SRC_BURST_SIZE.

Experiments shown that upon Rx timeout, the DMA transaction needed to be
manually cleared as well.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Co-developed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/tty/serial/8250/8250_dw.c | 64 +++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 238bcdf1bab0..f8e762e3ef0f 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -34,16 +34,28 @@
 
 /* Offsets for the DesignWare specific registers */
 #define DW_UART_USR	0x1f /* UART Status Register */
+#define DW_UART_DMASA	0xa8 /* DMA Software Ack */
+
+#define RZN1_UART_TDMACR 0x10c /* DMA Control Register Transmit Mode */
+#define RZN1_UART_RDMACR 0x110 /* DMA Control Register Receive Mode */
 
 #define OCTEON_UART_USR	0x27 /* UART Status Register */
 
 /* DesignWare specific register fields */
 #define DW_UART_MCR_SIRE		BIT(6)
 
+/* Renesas specific register fields */
+#define RZN1_UART_xDMACR_DMA_EN		BIT(0)
+#define RZN1_UART_xDMACR_1_WORD_BURST	(0 << 1)
+#define RZN1_UART_xDMACR_4_WORD_BURST	(1 << 1)
+#define RZN1_UART_xDMACR_8_WORD_BURST	(3 << 1)
+#define RZN1_UART_xDMACR_BLK_SZ(x)	((x) << 3)
+
 /* Quirks */
 #define DW_UART_QUIRK_OCTEON		BIT(0)
 #define DW_UART_QUIRK_ARMADA_38X	BIT(1)
 #define DW_UART_QUIRK_SKIP_SET_RATE	BIT(2)
+#define DW_UART_QUIRK_IS_DMA_FC		BIT(3)
 
 static inline struct dw8250_data *clk_to_dw8250_data(struct notifier_block *nb)
 {
@@ -226,6 +238,7 @@ static int dw8250_handle_irq(struct uart_port *p)
 	struct dw8250_data *d = to_dw8250_data(p->private_data);
 	unsigned int iir = p->serial_in(p, UART_IIR);
 	bool rx_timeout = (iir & 0x3f) == UART_IIR_RX_TIMEOUT;
+	unsigned int quirks = d->pdata->quirks;
 	unsigned int status;
 	unsigned long flags;
 
@@ -249,6 +262,15 @@ static int dw8250_handle_irq(struct uart_port *p)
 		spin_unlock_irqrestore(&p->lock, flags);
 	}
 
+	/* Manually stop the Rx DMA transfer when acting as flow controller */
+	if (quirks & DW_UART_QUIRK_IS_DMA_FC && up->dma && up->dma->rx_running && rx_timeout) {
+		status = p->serial_in(p, UART_LSR);
+		if (status & (UART_LSR_DR | UART_LSR_BI)) {
+			dw8250_writel_ext(p, RZN1_UART_RDMACR, 0);
+			dw8250_writel_ext(p, DW_UART_DMASA, 1);
+		}
+	}
+
 	if (serial8250_handle_irq(p, iir))
 		return 1;
 
@@ -372,6 +394,42 @@ static bool dw8250_idma_filter(struct dma_chan *chan, void *param)
 	return param == chan->device->dev;
 }
 
+static u32 dw8250_rzn1_get_dmacr_burst(int max_burst)
+{
+	if (max_burst >= 8)
+		return RZN1_UART_xDMACR_8_WORD_BURST;
+	else if (max_burst >= 4)
+		return RZN1_UART_xDMACR_4_WORD_BURST;
+	else
+		return RZN1_UART_xDMACR_1_WORD_BURST;
+}
+
+static void dw8250_prepare_tx_dma(struct uart_8250_port *p)
+{
+	struct uart_port *up = &p->port;
+	struct uart_8250_dma *dma = p->dma;
+	u32 val;
+
+	dw8250_writel_ext(up, RZN1_UART_TDMACR, 0);
+	val = dw8250_rzn1_get_dmacr_burst(dma->txconf.dst_maxburst) |
+	      RZN1_UART_xDMACR_BLK_SZ(dma->tx_size) |
+	      RZN1_UART_xDMACR_DMA_EN;
+	dw8250_writel_ext(up, RZN1_UART_TDMACR, val);
+}
+
+static void dw8250_prepare_rx_dma(struct uart_8250_port *p)
+{
+	struct uart_port *up = &p->port;
+	struct uart_8250_dma *dma = p->dma;
+	u32 val;
+
+	dw8250_writel_ext(up, RZN1_UART_RDMACR, 0);
+	val = dw8250_rzn1_get_dmacr_burst(dma->rxconf.src_maxburst) |
+	      RZN1_UART_xDMACR_BLK_SZ(dma->rx_size) |
+	      RZN1_UART_xDMACR_DMA_EN;
+	dw8250_writel_ext(up, RZN1_UART_RDMACR, val);
+}
+
 static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
 {
 	struct device_node *np = p->dev->of_node;
@@ -404,6 +462,12 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
 			p->serial_out = dw8250_serial_out38x;
 		if (quirks & DW_UART_QUIRK_SKIP_SET_RATE)
 			p->set_termios = dw8250_do_set_termios;
+		if (quirks & DW_UART_QUIRK_IS_DMA_FC) {
+			data->data.dma.txconf.device_fc = 1;
+			data->data.dma.rxconf.device_fc = 1;
+			data->data.dma.prepare_tx_dma = dw8250_prepare_tx_dma;
+			data->data.dma.prepare_rx_dma = dw8250_prepare_rx_dma;
+		}
 
 	} else if (acpi_dev_present("APMC0D08", NULL, -1)) {
 		p->iotype = UPIO_MEM32;
-- 
2.27.0

Re: [PATCH v5 09/11] serial: 8250: dw: Add support for DMA flow controlling devices

[Andy Shevchenko]

On Wed, Apr 13, 2022 at 09:51:39AM +0200, Miquel Raynal wrote:
> From: Phil Edworthy <phil.edworthy@renesas.com>
> 
> DW based controllers like the one on Renesas RZ/N1 must be programmed as
> flow controllers when using DMA.
> 
> * Table 11.45 of the system manual, "Flow Control Combinations", states
>   that using UART with DMA requires setting the DMA in the peripheral
>   flow controller mode regardless of the direction.
> 
> * Chapter 11.6.1.3 of the system manual, "Basic Interface Definitions",
>   explains that the burst size in the above case must be configured in
>   the peripheral's register DEST/SRC_BURST_SIZE.
> 
> Experiments shown that upon Rx timeout, the DMA transaction needed to be
> manually cleared as well.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
One remark below.

> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Co-developed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/tty/serial/8250/8250_dw.c | 64 +++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index 238bcdf1bab0..f8e762e3ef0f 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -34,16 +34,28 @@
>  
>  /* Offsets for the DesignWare specific registers */
>  #define DW_UART_USR	0x1f /* UART Status Register */
> +#define DW_UART_DMASA	0xa8 /* DMA Software Ack */

> +#define RZN1_UART_TDMACR 0x10c /* DMA Control Register Transmit Mode */
> +#define RZN1_UART_RDMACR 0x110 /* DMA Control Register Receive Mode */

This group is different to the DW and OCTEON, but OCTEON starts with smaller
number. I dunno how this will be in the future and if it makes sense to order
now, i.e. to put these two after OCTEON group. In either case I'm fine.

>  #define OCTEON_UART_USR	0x27 /* UART Status Register */
>  
>  /* DesignWare specific register fields */
>  #define DW_UART_MCR_SIRE		BIT(6)
>  
> +/* Renesas specific register fields */
> +#define RZN1_UART_xDMACR_DMA_EN		BIT(0)
> +#define RZN1_UART_xDMACR_1_WORD_BURST	(0 << 1)
> +#define RZN1_UART_xDMACR_4_WORD_BURST	(1 << 1)
> +#define RZN1_UART_xDMACR_8_WORD_BURST	(3 << 1)
> +#define RZN1_UART_xDMACR_BLK_SZ(x)	((x) << 3)
> +
>  /* Quirks */
>  #define DW_UART_QUIRK_OCTEON		BIT(0)
>  #define DW_UART_QUIRK_ARMADA_38X	BIT(1)
>  #define DW_UART_QUIRK_SKIP_SET_RATE	BIT(2)
> +#define DW_UART_QUIRK_IS_DMA_FC		BIT(3)
>  
>  static inline struct dw8250_data *clk_to_dw8250_data(struct notifier_block *nb)
>  {
> @@ -226,6 +238,7 @@ static int dw8250_handle_irq(struct uart_port *p)
>  	struct dw8250_data *d = to_dw8250_data(p->private_data);
>  	unsigned int iir = p->serial_in(p, UART_IIR);
>  	bool rx_timeout = (iir & 0x3f) == UART_IIR_RX_TIMEOUT;
> +	unsigned int quirks = d->pdata->quirks;
>  	unsigned int status;
>  	unsigned long flags;
>  
> @@ -249,6 +262,15 @@ static int dw8250_handle_irq(struct uart_port *p)
>  		spin_unlock_irqrestore(&p->lock, flags);
>  	}
>  
> +	/* Manually stop the Rx DMA transfer when acting as flow controller */
> +	if (quirks & DW_UART_QUIRK_IS_DMA_FC && up->dma && up->dma->rx_running && rx_timeout) {
> +		status = p->serial_in(p, UART_LSR);
> +		if (status & (UART_LSR_DR | UART_LSR_BI)) {
> +			dw8250_writel_ext(p, RZN1_UART_RDMACR, 0);
> +			dw8250_writel_ext(p, DW_UART_DMASA, 1);
> +		}
> +	}
> +
>  	if (serial8250_handle_irq(p, iir))
>  		return 1;
>  
> @@ -372,6 +394,42 @@ static bool dw8250_idma_filter(struct dma_chan *chan, void *param)
>  	return param == chan->device->dev;
>  }
>  
> +static u32 dw8250_rzn1_get_dmacr_burst(int max_burst)
> +{
> +	if (max_burst >= 8)
> +		return RZN1_UART_xDMACR_8_WORD_BURST;
> +	else if (max_burst >= 4)
> +		return RZN1_UART_xDMACR_4_WORD_BURST;
> +	else
> +		return RZN1_UART_xDMACR_1_WORD_BURST;
> +}
> +
> +static void dw8250_prepare_tx_dma(struct uart_8250_port *p)
> +{
> +	struct uart_port *up = &p->port;
> +	struct uart_8250_dma *dma = p->dma;
> +	u32 val;
> +
> +	dw8250_writel_ext(up, RZN1_UART_TDMACR, 0);
> +	val = dw8250_rzn1_get_dmacr_burst(dma->txconf.dst_maxburst) |
> +	      RZN1_UART_xDMACR_BLK_SZ(dma->tx_size) |
> +	      RZN1_UART_xDMACR_DMA_EN;
> +	dw8250_writel_ext(up, RZN1_UART_TDMACR, val);
> +}
> +
> +static void dw8250_prepare_rx_dma(struct uart_8250_port *p)
> +{
> +	struct uart_port *up = &p->port;
> +	struct uart_8250_dma *dma = p->dma;
> +	u32 val;
> +
> +	dw8250_writel_ext(up, RZN1_UART_RDMACR, 0);
> +	val = dw8250_rzn1_get_dmacr_burst(dma->rxconf.src_maxburst) |
> +	      RZN1_UART_xDMACR_BLK_SZ(dma->rx_size) |
> +	      RZN1_UART_xDMACR_DMA_EN;
> +	dw8250_writel_ext(up, RZN1_UART_RDMACR, val);
> +}
> +
>  static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
>  {
>  	struct device_node *np = p->dev->of_node;
> @@ -404,6 +462,12 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
>  			p->serial_out = dw8250_serial_out38x;
>  		if (quirks & DW_UART_QUIRK_SKIP_SET_RATE)
>  			p->set_termios = dw8250_do_set_termios;
> +		if (quirks & DW_UART_QUIRK_IS_DMA_FC) {
> +			data->data.dma.txconf.device_fc = 1;
> +			data->data.dma.rxconf.device_fc = 1;
> +			data->data.dma.prepare_tx_dma = dw8250_prepare_tx_dma;
> +			data->data.dma.prepare_rx_dma = dw8250_prepare_rx_dma;
> +		}
>  
>  	} else if (acpi_dev_present("APMC0D08", NULL, -1)) {
>  		p->iotype = UPIO_MEM32;
> -- 
> 2.27.0
> 

-- 
With Best Regards,
Andy Shevchenko

[PATCH v5 10/11] serial: 8250: dw: Improve RZN1 support

[Miquel Raynal]

From: Phil Edworthy <phil.edworthy@renesas.com>

Renesas RZ/N1 SoC features a slightly modified DW UART.

On this SoC, the CPR register value is known but not synthetized in
hardware. We hence need to provide a CPR value in the platform
data. This version of the controller also relies on acting as flow
controller when using DMA, so we need to provide the
"is dma flow controller" quirk.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Co-developed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_dw.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index f8e762e3ef0f..a4f46456677c 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -761,6 +761,8 @@ static const struct dw8250_platform_data dw8250_armada_38x_data = {
 
 static const struct dw8250_platform_data dw8250_renesas_rzn1_data = {
 	.usr_reg = DW_UART_USR,
+	.quirks = DW_UART_QUIRK_IS_DMA_FC,
+	.cpr = 0x00012f32,
 };
 
 static const struct dw8250_platform_data dw8250_starfive_jh7100_data = {
-- 
2.27.0

[PATCH v5 11/11] ARM: dts: r9a06g032: Fill the UART DMA properties

[Miquel Raynal]

UART 0 to 2 do not have DMA support, while UART 3 to 7 do.

Fill the "dmas" and "dma-names" properties for each of these nodes.

Please mind that these nodes go through the dmamux node which will
redirect the requests to the right DMA controller. The first 4 cells of
the "dmas" properties will be transferred as-is to the DMA
controllers. The last 2 cells are consumed by the dmamux. Which means
cell 0 and 4 are almost redundant, one giving the controller request ID
and the other the dmamux channel which is a 1:1 translation of the
request IDs, shifted by 16 when pointing to the second DMA controller.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm/boot/dts/r9a06g032.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
index c854aa4cfa77..4288b935fcea 100644
--- a/arch/arm/boot/dts/r9a06g032.dtsi
+++ b/arch/arm/boot/dts/r9a06g032.dtsi
@@ -144,6 +144,9 @@ uart3: serial@50000000 {
 			reg-io-width = <4>;
 			clocks = <&sysctrl R9A06G032_CLK_UART3>, <&sysctrl R9A06G032_HCLK_UART3>;
 			clock-names = "baudclk", "apb_pclk";
+			dmas =  <&dmamux 0 0 0 0 0 1>,
+				<&dmamux 1 0 0 0 1 1>;
+			dma-names = "rx", "tx";
 			status = "disabled";
 		};
 
@@ -155,6 +158,9 @@ uart4: serial@50001000 {
 			reg-io-width = <4>;
 			clocks = <&sysctrl R9A06G032_CLK_UART4>, <&sysctrl R9A06G032_HCLK_UART4>;
 			clock-names = "baudclk", "apb_pclk";
+			dmas =  <&dmamux 2 0 0 0 2 1>,
+				<&dmamux 3 0 0 0 3 1>;
+			dma-names = "rx", "tx";
 			status = "disabled";
 		};
 
@@ -166,6 +172,9 @@ uart5: serial@50002000 {
 			reg-io-width = <4>;
 			clocks = <&sysctrl R9A06G032_CLK_UART5>, <&sysctrl R9A06G032_HCLK_UART5>;
 			clock-names = "baudclk", "apb_pclk";
+			dmas =  <&dmamux 4 0 0 0 4 1>,
+				<&dmamux 5 0 0 0 5 1>;
+			dma-names = "rx", "tx";
 			status = "disabled";
 		};
 
@@ -177,6 +186,9 @@ uart6: serial@50003000 {
 			reg-io-width = <4>;
 			clocks = <&sysctrl R9A06G032_CLK_UART6>, <&sysctrl R9A06G032_HCLK_UART6>;
 			clock-names = "baudclk", "apb_pclk";
+			dmas =  <&dmamux 6 0 0 0 6 1>,
+				<&dmamux 7 0 0 0 7 1>;
+			dma-names = "rx", "tx";
 			status = "disabled";
 		};
 
@@ -188,6 +200,9 @@ uart7: serial@50004000 {
 			reg-io-width = <4>;
 			clocks = <&sysctrl R9A06G032_CLK_UART7>, <&sysctrl R9A06G032_HCLK_UART7>;
 			clock-names = "baudclk", "apb_pclk";
+			dmas =  <&dmamux 4 0 0 0 20 1>,
+				<&dmamux 5 0 0 0 21 1>;
+			dma-names = "rx", "tx";
 			status = "disabled";
 		};
 
-- 
2.27.0

Re: [PATCH v5 00/11] serial: 8250: dw: RZN1 DMA support

[Greg Kroah-Hartman]

On Wed, Apr 13, 2022 at 09:51:30AM +0200, Miquel Raynal wrote:
> Hello,
> 
> Support for the RZN1 DMA engine allows us adapt a little bit the 8250 DW
> UART driver with to bring DMA support for this SoC.
> 
> This short series applies on top of the series bringing RZN1 DMA
> support, currently on its v10, see [1]. Technically speaking, only the DT
> patch needs to be applied after [1]. The other patches can come in at
> any moment, because if no "dmas" property is provided in the DT, DMA
> support will simply be ignored.
> 
> [1] https://lore.kernel.org/dmaengine/20220412193936.63355-1-miquel.raynal@bootlin.com/T/#t

Can you rebase on my tty-next branch please?  This series does not apply
anymore.

thanks,

greg k-h

Re: [PATCH v5 00/11] serial: 8250: dw: RZN1 DMA support

[Miquel Raynal]

Hi Greg,

gregkh@linuxfoundation.org wrote on Fri, 15 Apr 2022 11:36:37 +0200:

> On Wed, Apr 13, 2022 at 09:51:30AM +0200, Miquel Raynal wrote:
> > Hello,
> > 
> > Support for the RZN1 DMA engine allows us adapt a little bit the 8250 DW
> > UART driver with to bring DMA support for this SoC.
> > 
> > This short series applies on top of the series bringing RZN1 DMA
> > support, currently on its v10, see [1]. Technically speaking, only the DT
> > patch needs to be applied after [1]. The other patches can come in at
> > any moment, because if no "dmas" property is provided in the DT, DMA
> > support will simply be ignored.
> > 
> > [1] https://lore.kernel.org/dmaengine/20220412193936.63355-1-miquel.raynal@bootlin.com/T/#t  
> 
> Can you rebase on my tty-next branch please?  This series does not apply
> anymore.

Yes of course.

Thanks,
Miquèl

Re: [PATCH v5 00/11] serial: 8250: dw: RZN1 DMA support

[Miquel Raynal]

Hello,

miquel.raynal@bootlin.com wrote on Wed, 20 Apr 2022 10:56:31 +0200:

> Hi Greg,
> 
> gregkh@linuxfoundation.org wrote on Fri, 15 Apr 2022 11:36:37 +0200:
> 
> > On Wed, Apr 13, 2022 at 09:51:30AM +0200, Miquel Raynal wrote:  
> > > Hello,
> > > 
> > > Support for the RZN1 DMA engine allows us adapt a little bit the 8250 DW
> > > UART driver with to bring DMA support for this SoC.
> > > 
> > > This short series applies on top of the series bringing RZN1 DMA
> > > support, currently on its v10, see [1]. Technically speaking, only the DT
> > > patch needs to be applied after [1]. The other patches can come in at
> > > any moment, because if no "dmas" property is provided in the DT, DMA
> > > support will simply be ignored.
> > > 
> > > [1] https://lore.kernel.org/dmaengine/20220412193936.63355-1-miquel.raynal@bootlin.com/T/#t    
> > 
> > Can you rebase on my tty-next branch please?  This series does not apply
> > anymore.  
> 
> Yes of course.

Actually the conflict comes from the fact that I was asked to
rebase this series on top of a couple of changes for supporting a
Starfive controller (jh7100-hsuart) which impacted the same area as I
was touching.

It turns out this series was not merged during the last merge cycle as
expected. It seems to be because it received comments that were raised
when reviewing my own series and which I actually fixed.

Anyway, I've dropped the commit adding jh7100 support but squashed the
patch bringing platform data to 8250_dw.c so that this series can now
stand alone (see v6). It will be very easy to rebase the series from
Emil on top of this one.

Thanks,
Miquèl