[PATCH v10 1/3] serial: samsung: Replace rd_regb/wr_regb with rd_reg/wr_reg

[PATCH v10 1/3] serial: samsung: Replace rd_regb/wr_regb with rd_reg/wr_reg

[Hyunki Koo]

This patch change the name of macro for general usage.

Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Tested on Odroid HC1 (Exynos5422):
Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/tty/serial/samsung_tty.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index 73f951d65b93..326b0164609c 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -154,10 +154,10 @@ struct s3c24xx_uart_port {
 #define portaddrl(port, reg) \
 	((unsigned long *)(unsigned long)((port)->membase + (reg)))
 
-#define rd_regb(port, reg) (readb_relaxed(portaddr(port, reg)))
+#define rd_reg(port, reg) (readb_relaxed(portaddr(port, reg)))
 #define rd_regl(port, reg) (readl_relaxed(portaddr(port, reg)))
 
-#define wr_regb(port, reg, val) writeb_relaxed(val, portaddr(port, reg))
+#define wr_reg(port, reg, val) writeb_relaxed(val, portaddr(port, reg))
 #define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg))
 
 /* Byte-order aware bit setting/clearing functions. */
@@ -714,7 +714,7 @@ static void s3c24xx_serial_rx_drain_fifo(struct s3c24xx_uart_port *ourport)
 		fifocnt--;
 
 		uerstat = rd_regl(port, S3C2410_UERSTAT);
-		ch = rd_regb(port, S3C2410_URXH);
+		ch = rd_reg(port, S3C2410_URXH);
 
 		if (port->flags & UPF_CONS_FLOW) {
 			int txe = s3c24xx_serial_txempty_nofifo(port);
@@ -826,7 +826,7 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
 	}
 
 	if (port->x_char) {
-		wr_regb(port, S3C2410_UTXH, port->x_char);
+		wr_reg(port, S3C2410_UTXH, port->x_char);
 		port->icount.tx++;
 		port->x_char = 0;
 		goto out;
@@ -852,7 +852,7 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
 		if (rd_regl(port, S3C2410_UFSTAT) & ourport->info->tx_fifofull)
 			break;
 
-		wr_regb(port, S3C2410_UTXH, xmit->buf[xmit->tail]);
+		wr_reg(port, S3C2410_UTXH, xmit->buf[xmit->tail]);
 		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
 		port->icount.tx++;
 		count--;
@@ -916,7 +916,7 @@ static unsigned int s3c24xx_serial_tx_empty(struct uart_port *port)
 /* no modem control lines */
 static unsigned int s3c24xx_serial_get_mctrl(struct uart_port *port)
 {
-	unsigned int umstat = rd_regb(port, S3C2410_UMSTAT);
+	unsigned int umstat = rd_reg(port, S3C2410_UMSTAT);
 
 	if (umstat & S3C2410_UMSTAT_CTS)
 		return TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
@@ -2185,7 +2185,7 @@ static int s3c24xx_serial_get_poll_char(struct uart_port *port)
 	if (s3c24xx_serial_rx_fifocnt(ourport, ufstat) == 0)
 		return NO_POLL_CHAR;
 
-	return rd_regb(port, S3C2410_URXH);
+	return rd_reg(port, S3C2410_URXH);
 }
 
 static void s3c24xx_serial_put_poll_char(struct uart_port *port,
@@ -2200,7 +2200,7 @@ static void s3c24xx_serial_put_poll_char(struct uart_port *port,
 
 	while (!s3c24xx_serial_console_txrdy(port, ufcon))
 		cpu_relax();
-	wr_regb(port, S3C2410_UTXH, c);
+	wr_reg(port, S3C2410_UTXH, c);
 }
 
 #endif /* CONFIG_CONSOLE_POLL */
@@ -2212,7 +2212,7 @@ s3c24xx_serial_console_putchar(struct uart_port *port, int ch)
 
 	while (!s3c24xx_serial_console_txrdy(port, ufcon))
 		cpu_relax();
-	wr_regb(port, S3C2410_UTXH, ch);
+	wr_reg(port, S3C2410_UTXH, ch);
 }
 
 static void
-- 
2.15.0.rc1

[PATCH v10 2/3] dt-bindings: serial: Add reg-io-width compatible

[Hyunki Koo]

Add a description for reg-io-width options for the samsung serial
UART peripheral.

Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/serial/samsung_uart.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/samsung_uart.yaml b/Documentation/devicetree/bindings/serial/samsung_uart.yaml
index 9d2ce347875b..a57b1233c691 100644
--- a/Documentation/devicetree/bindings/serial/samsung_uart.yaml
+++ b/Documentation/devicetree/bindings/serial/samsung_uart.yaml
@@ -29,6 +29,14 @@ properties:
   reg:
     maxItems: 1
 
+  reg-io-width:
+    description: |
+      The size (in bytes) of the IO accesses that should be performed
+      on the device.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+      - enum: [ 1, 4 ]
+
   clocks:
     minItems: 2
     maxItems: 5
-- 
2.15.0.rc1

[PATCH v10 3/3] tty: samsung_tty: 32-bit access for TX/RX hold registers

[Hyunki Koo]

Support 32-bit access for the TX/RX hold registers UTXH and URXH.

This is required for some newer SoCs.

Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Tested on Odroid HC1 (Exynos5422):
Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/tty/serial/samsung_tty.c | 62 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 57 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index 326b0164609c..6ef614d8648c 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -154,10 +154,33 @@ struct s3c24xx_uart_port {
 #define portaddrl(port, reg) \
 	((unsigned long *)(unsigned long)((port)->membase + (reg)))
 
-#define rd_reg(port, reg) (readb_relaxed(portaddr(port, reg)))
+static u32 rd_reg(struct uart_port *port, u32 reg)
+{
+	switch (port->iotype) {
+	case UPIO_MEM:
+		return readb_relaxed(portaddr(port, reg));
+	case UPIO_MEM32:
+		return readl_relaxed(portaddr(port, reg));
+	default:
+		return 0;
+	}
+	return 0;
+}
+
 #define rd_regl(port, reg) (readl_relaxed(portaddr(port, reg)))
 
-#define wr_reg(port, reg, val) writeb_relaxed(val, portaddr(port, reg))
+static void wr_reg(struct uart_port *port, u32 reg, u32 val)
+{
+	switch (port->iotype) {
+	case UPIO_MEM:
+		writeb_relaxed(val, portaddr(port, reg));
+		break;
+	case UPIO_MEM32:
+		writel_relaxed(val, portaddr(port, reg));
+		break;
+	}
+}
+
 #define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg))
 
 /* Byte-order aware bit setting/clearing functions. */
@@ -1974,7 +1997,7 @@ static int s3c24xx_serial_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct s3c24xx_uart_port *ourport;
 	int index = probe_index;
-	int ret;
+	int ret, prop = 0;
 
 	if (np) {
 		ret = of_alias_get_id(np, "serial");
@@ -2000,10 +2023,27 @@ static int s3c24xx_serial_probe(struct platform_device *pdev)
 			dev_get_platdata(&pdev->dev) :
 			ourport->drv_data->def_cfg;
 
-	if (np)
+	if (np) {
 		of_property_read_u32(np,
 			"samsung,uart-fifosize", &ourport->port.fifosize);
 
+		if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
+			switch (prop) {
+			case 1:
+				ourport->port.iotype = UPIO_MEM;
+				break;
+			case 4:
+				ourport->port.iotype = UPIO_MEM32;
+				break;
+			default:
+				dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n",
+						prop);
+				ret = -EINVAL;
+				break;
+			}
+		}
+	}
+
 	if (ourport->drv_data->fifosize[index])
 		ourport->port.fifosize = ourport->drv_data->fifosize[index];
 	else if (ourport->info->fifosize)
@@ -2587,6 +2627,18 @@ module_platform_driver(samsung_serial_driver);
  * Early console.
  */
 
+static void wr_reg_barrier(struct uart_port *port, u32 reg, u32 val)
+{
+	switch (port->iotype) {
+	case UPIO_MEM:
+		writeb(val, portaddr(port, reg));
+		break;
+	case UPIO_MEM32:
+		writel(val, portaddr(port, reg));
+		break;
+	}
+}
+
 struct samsung_early_console_data {
 	u32 txfull_mask;
 };
@@ -2612,7 +2664,7 @@ static void samsung_early_putc(struct uart_port *port, int c)
 	else
 		samsung_early_busyuart(port);
 
-	writeb(c, port->membase + S3C2410_UTXH);
+	wr_reg_barrier(port, S3C2410_UTXH, c);
 }
 
 static void samsung_early_write(struct console *con, const char *s,
-- 
2.15.0.rc1

Re: [PATCH v10 3/3] tty: samsung_tty: 32-bit access for TX/RX hold registers

[Bartlomiej Zolnierkiewicz]

Hi!

On 5/6/20 10:02 AM, Hyunki Koo wrote:
> Support 32-bit access for the TX/RX hold registers UTXH and URXH.
> 
> This is required for some newer SoCs.

Krzysztof has asked this previously but I couldn't find the answer in
previous mails:

Do you plan to upstream support for these newer SoCs?

If not (i.e. this code is only to support Android GKI) then the code
you are adding now may be removed at any time later during cleanups
(due to lack of the in-kernel users).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> Tested on Odroid HC1 (Exynos5422):
> Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  drivers/tty/serial/samsung_tty.c | 62 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 57 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
> index 326b0164609c..6ef614d8648c 100644
> --- a/drivers/tty/serial/samsung_tty.c
> +++ b/drivers/tty/serial/samsung_tty.c
> @@ -154,10 +154,33 @@ struct s3c24xx_uart_port {
>  #define portaddrl(port, reg) \
>  	((unsigned long *)(unsigned long)((port)->membase + (reg)))
>  
> -#define rd_reg(port, reg) (readb_relaxed(portaddr(port, reg)))
> +static u32 rd_reg(struct uart_port *port, u32 reg)
> +{
> +	switch (port->iotype) {
> +	case UPIO_MEM:
> +		return readb_relaxed(portaddr(port, reg));
> +	case UPIO_MEM32:
> +		return readl_relaxed(portaddr(port, reg));
> +	default:
> +		return 0;
> +	}
> +	return 0;
> +}
> +
>  #define rd_regl(port, reg) (readl_relaxed(portaddr(port, reg)))
>  
> -#define wr_reg(port, reg, val) writeb_relaxed(val, portaddr(port, reg))
> +static void wr_reg(struct uart_port *port, u32 reg, u32 val)
> +{
> +	switch (port->iotype) {
> +	case UPIO_MEM:
> +		writeb_relaxed(val, portaddr(port, reg));
> +		break;
> +	case UPIO_MEM32:
> +		writel_relaxed(val, portaddr(port, reg));
> +		break;
> +	}
> +}
> +
>  #define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg))
>  
>  /* Byte-order aware bit setting/clearing functions. */
> @@ -1974,7 +1997,7 @@ static int s3c24xx_serial_probe(struct platform_device *pdev)
>  	struct device_node *np = pdev->dev.of_node;
>  	struct s3c24xx_uart_port *ourport;
>  	int index = probe_index;
> -	int ret;
> +	int ret, prop = 0;
>  
>  	if (np) {
>  		ret = of_alias_get_id(np, "serial");
> @@ -2000,10 +2023,27 @@ static int s3c24xx_serial_probe(struct platform_device *pdev)
>  			dev_get_platdata(&pdev->dev) :
>  			ourport->drv_data->def_cfg;
>  
> -	if (np)
> +	if (np) {
>  		of_property_read_u32(np,
>  			"samsung,uart-fifosize", &ourport->port.fifosize);
>  
> +		if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
> +			switch (prop) {
> +			case 1:
> +				ourport->port.iotype = UPIO_MEM;
> +				break;
> +			case 4:
> +				ourport->port.iotype = UPIO_MEM32;
> +				break;
> +			default:
> +				dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n",
> +						prop);
> +				ret = -EINVAL;
> +				break;
> +			}
> +		}
> +	}
> +
>  	if (ourport->drv_data->fifosize[index])
>  		ourport->port.fifosize = ourport->drv_data->fifosize[index];
>  	else if (ourport->info->fifosize)
> @@ -2587,6 +2627,18 @@ module_platform_driver(samsung_serial_driver);
>   * Early console.
>   */
>  
> +static void wr_reg_barrier(struct uart_port *port, u32 reg, u32 val)
> +{
> +	switch (port->iotype) {
> +	case UPIO_MEM:
> +		writeb(val, portaddr(port, reg));
> +		break;
> +	case UPIO_MEM32:
> +		writel(val, portaddr(port, reg));
> +		break;
> +	}
> +}
> +
>  struct samsung_early_console_data {
>  	u32 txfull_mask;
>  };
> @@ -2612,7 +2664,7 @@ static void samsung_early_putc(struct uart_port *port, int c)
>  	else
>  		samsung_early_busyuart(port);
>  
> -	writeb(c, port->membase + S3C2410_UTXH);
> +	wr_reg_barrier(port, S3C2410_UTXH, c);
>  }
>  
>  static void samsung_early_write(struct console *con, const char *s,
> 

RE: [PATCH v10 3/3] tty: samsung_tty: 32-bit access for TX/RX hold registers

[Hyunki Koo]

On 5/6/20 8:03 PM, Bartlomiej Zolnierkiewicz wrote:
> On 5/6/20 10:02 AM, Hyunki Koo wrote:
> > Support 32-bit access for the TX/RX hold registers UTXH and URXH.
> >
> > This is required for some newer SoCs.
> 
> Krzysztof has asked this previously but I couldn't find the answer in
> previous mails:
> 
> Do you plan to upstream support for these newer SoCs?
> 
> If not (i.e. this code is only to support Android GKI) then the code you are
> adding now may be removed at any time later during cleanups (due to
> lack of the in-kernel users).
> 
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 
> > Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com>
> > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Tested on Odroid
> > HC1 (Exynos5422):
> > Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
> > ---
> >  drivers/tty/serial/samsung_tty.c | 62
> > ++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 57 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/tty/serial/samsung_tty.c
> > b/drivers/tty/serial/samsung_tty.c
> > index 326b0164609c..6ef614d8648c 100644
> > --- a/drivers/tty/serial/samsung_tty.c
> > +++ b/drivers/tty/serial/samsung_tty.c
> > @@ -154,10 +154,33 @@ struct s3c24xx_uart_port {  #define
> > portaddrl(port, reg) \
> >  	((unsigned long *)(unsigned long)((port)->membase + (reg)))
> >
> > -#define rd_reg(port, reg) (readb_relaxed(portaddr(port, reg)))
> > +static u32 rd_reg(struct uart_port *port, u32 reg) {
> > +	switch (port->iotype) {
> > +	case UPIO_MEM:
> > +		return readb_relaxed(portaddr(port, reg));
> > +	case UPIO_MEM32:
> > +		return readl_relaxed(portaddr(port, reg));
> > +	default:
> > +		return 0;
> > +	}
> > +	return 0;
> > +}
> > +
> >  #define rd_regl(port, reg) (readl_relaxed(portaddr(port, reg)))
> >
> > -#define wr_reg(port, reg, val) writeb_relaxed(val, portaddr(port,
> > reg))
> > +static void wr_reg(struct uart_port *port, u32 reg, u32 val) {
> > +	switch (port->iotype) {
> > +	case UPIO_MEM:
> > +		writeb_relaxed(val, portaddr(port, reg));
> > +		break;
> > +	case UPIO_MEM32:
> > +		writel_relaxed(val, portaddr(port, reg));
> > +		break;
> > +	}
> > +}
> > +
> >  #define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port,
> > reg))
> >
> >  /* Byte-order aware bit setting/clearing functions. */ @@ -1974,7
> > +1997,7 @@ static int s3c24xx_serial_probe(struct platform_device
> *pdev)
> >  	struct device_node *np = pdev->dev.of_node;
> >  	struct s3c24xx_uart_port *ourport;
> >  	int index = probe_index;
> > -	int ret;
> > +	int ret, prop = 0;
> >
> >  	if (np) {
> >  		ret = of_alias_get_id(np, "serial"); @@ -2000,10
> +2023,27 @@ static
> > int s3c24xx_serial_probe(struct platform_device *pdev)
> >  			dev_get_platdata(&pdev->dev) :
> >  			ourport->drv_data->def_cfg;
> >
> > -	if (np)
> > +	if (np) {
> >  		of_property_read_u32(np,
> >  			"samsung,uart-fifosize", &ourport->port.fifosize);
> >
> > +		if (of_property_read_u32(np, "reg-io-width", &prop) ==
> 0) {
> > +			switch (prop) {
> > +			case 1:
> > +				ourport->port.iotype = UPIO_MEM;
> > +				break;
> > +			case 4:
> > +				ourport->port.iotype = UPIO_MEM32;
> > +				break;
> > +			default:
> > +				dev_warn(&pdev->dev, "unsupported
> reg-io-width (%d)\n",
> > +						prop);
> > +				ret = -EINVAL;
> > +				break;
> > +			}
> > +		}
> > +	}
> > +
> >  	if (ourport->drv_data->fifosize[index])
> >  		ourport->port.fifosize = ourport->drv_data-
> >fifosize[index];
> >  	else if (ourport->info->fifosize)
> > @@ -2587,6 +2627,18 @@
> module_platform_driver(samsung_serial_driver);
> >   * Early console.
> >   */
> >
> > +static void wr_reg_barrier(struct uart_port *port, u32 reg, u32 val)
> > +{
> > +	switch (port->iotype) {
> > +	case UPIO_MEM:
> > +		writeb(val, portaddr(port, reg));
> > +		break;
> > +	case UPIO_MEM32:
> > +		writel(val, portaddr(port, reg));
> > +		break;
> > +	}
> > +}
> > +
> >  struct samsung_early_console_data {
> >  	u32 txfull_mask;
> >  };
> > @@ -2612,7 +2664,7 @@ static void samsung_early_putc(struct
> uart_port *port, int c)
> >  	else
> >  		samsung_early_busyuart(port);
> >
> > -	writeb(c, port->membase + S3C2410_UTXH);
> > +	wr_reg_barrier(port, S3C2410_UTXH, c);
> >  }
> >
> >  static void samsung_early_write(struct console *con, const char *s,
> >
Hi
We don't have plan to upstream for new SOCs yet,
This code is only to support Android GKI, 
But it should not be not removed 

Re: [PATCH v10 3/3] tty: samsung_tty: 32-bit access for TX/RX hold registers

[Jiri Slaby]

On 06. 05. 20, 10:02, Hyunki Koo wrote:
> Support 32-bit access for the TX/RX hold registers UTXH and URXH.
> 
> This is required for some newer SoCs.
> 
> Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> Tested on Odroid HC1 (Exynos5422):
> Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  drivers/tty/serial/samsung_tty.c | 62 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 57 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
> index 326b0164609c..6ef614d8648c 100644
> --- a/drivers/tty/serial/samsung_tty.c
> +++ b/drivers/tty/serial/samsung_tty.c
...
> @@ -2000,10 +2023,27 @@ static int s3c24xx_serial_probe(struct platform_device *pdev)
>  			dev_get_platdata(&pdev->dev) :
>  			ourport->drv_data->def_cfg;
>  
> -	if (np)
> +	if (np) {
>  		of_property_read_u32(np,
>  			"samsung,uart-fifosize", &ourport->port.fifosize);
>  
> +		if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
> +			switch (prop) {
> +			case 1:
> +				ourport->port.iotype = UPIO_MEM;
> +				break;
> +			case 4:
> +				ourport->port.iotype = UPIO_MEM32;
> +				break;
> +			default:
> +				dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n",
> +						prop);
> +				ret = -EINVAL;
> +				break;

This ret value is unused. Did you intend to return here?

thanks,
-- 
js
suse labs

Re: [PATCH v10 3/3] tty: samsung_tty: 32-bit access for TX/RX hold registers

[Tudor Ambarus]

I'm trying my luck on a 3 year old patch, fingers crossed :).

On 5/6/20 09:02, Hyunki Koo wrote:

Hi, Hyunki!

> Support 32-bit access for the TX/RX hold registers UTXH and URXH.
> 
> This is required for some newer SoCs.

Is the 32-bit access mandatory for the newer SoCs? I'm asking because I
see a SError Interrupt when using 8-bit accesses on gs101 (pixel6). Can
you confirm if gs101 USI UART is restricted to only 32 bit data width
accesses?

If 32-bit accesses are mandatory, we should impose this restriction so
that we avoid crashing the kernel on wrong user configuration. I have
some patches ready, waiting just for a confirmation.

Below is the crash that I see. Thanks!

[    1.215459] SError Interrupt on CPU6, code 0x00000000be000411 -- SError
[    1.215464] CPU: 6 PID: 1 Comm: swapper/0 Not tainted
6.6.0-rc1-00021-g31831088e862-dirty #88
[    1.215468] Hardware name: Oriole DVT (DT)
[    1.215470] pstate: 204000c5 (nzCv daIF +PAN -UAO -TCO -DIT -SSBS
BTYPE=--)
[    1.215473] pc : s3c24xx_serial_console_putchar+0x1c/0x90
[    1.215487] lr : uart_console_write+0x50/0x6c
[    1.215491] sp : ffff80008007b6c0
[    1.215492] x29: ffff80008007b6c0 x28: 0000000000000000 x27:
ffffc0ec2c855900
[    1.215498] x26: ffffc0ec2bdee008 x25: 0000000000000000 x24:
ffffc0ec2c529a40
[    1.215501] x23: ffff80008007b830 x22: ffffc0ec2c9578b9 x21:
ffffc0ec2c9b8330
[    1.215504] x20: ffffc0ec2aa0d684 x19: ffffc0ec2c95788a x18:
0000000000000006
[    1.215507] x17: 0000000000000000 x16: 0000000000000001 x15:
ffff80008007b460
[    1.215510] x14: 0000000000000000 x13: 64656c62616e6520 x12:
5d30434153797474
[    1.215513] x11: 5b20656c6f736e6f x10: 63203a6b746e6972 x9 :
656c62616e65205d
[    1.215515] x8 : 304341537974745b x7 : 205d303534353132 x6 :
322e31202020205b
[    1.215518] x5 : ffffc0ec2c9b82f0 x4 : ffffc0ec2b5bc1c0 x3 :
ffff800080061000
[    1.215521] x2 : 0000000000000111 x1 : 0000000000000020 x0 :
ffffc0ec2c9b8330
[    1.215526] Kernel panic - not syncing: Asynchronous SError Interrupt
[    1.215528] CPU: 6 PID: 1 Comm: swapper/0 Not tainted
6.6.0-rc1-00021-g31831088e862-dirty #88
[    1.215531] Hardware name: Oriole DVT (DT)
[    1.215532] Call trace:
[    1.215533]  dump_backtrace+0x90/0xe8
[    1.215542]  show_stack+0x18/0x24
[    1.215547]  dump_stack_lvl+0x48/0x60
[    1.215555]  dump_stack+0x18/0x24
[    1.215559]  panic+0x31c/0x378
[    1.215565]  nmi_panic+0x8c/0x90
[    1.215569]  arm64_serror_panic+0x6c/0x78
[    1.215571]  do_serror+0x3c/0x70
[    1.215574]  el1h_64_error_handler+0x30/0x48
[    1.215581]  el1h_64_error+0x64/0x68
[    1.215583]  s3c24xx_serial_console_putchar+0x1c/0x90
[    1.215588]  s3c24xx_serial_console_write+0xb8/0xfc
[    1.215593]  console_flush_all.isra.0+0x168/0x3ac
[    1.215600]  console_unlock+0x58/0xdc
[    1.215604]  vprintk_emit+0x1a8/0x2b0
[    1.215608]  vprintk_default+0x38/0x44
[    1.215613]  vprintk+0xbc/0xd0
[    1.215617]  _printk+0x5c/0x84
[    1.215621]  register_console+0x2a0/0x400
[    1.215625]  serial_core_register_port+0x61c/0x658
[    1.215630]  serial_ctrl_register_port+0x10/0x1c
[    1.215634]  uart_add_one_port+0x10/0x1c
[    1.215639]  s3c24xx_serial_probe+0x360/0x6fc
[    1.215644]  platform_probe+0x68/0xc8
[    1.215650]  really_probe+0x148/0x2b0
[    1.215653]  __driver_probe_device+0x78/0x12c
[    1.215656]  driver_probe_device+0x3c/0x15c
[    1.215658]  __driver_attach+0x90/0x19c
[    1.215661]  bus_for_each_dev+0x78/0xd4
[    1.215668]  driver_attach+0x24/0x30
[    1.215670]  bus_add_driver+0xe8/0x1f8
[    1.215672]  driver_register+0x5c/0x124
[    1.215675]  __platform_driver_register+0x28/0x34
[    1.215678]  samsung_serial_init+0x30/0x8c
[    1.215687]  do_one_initcall+0x7c/0x1c0
[    1.215689]  kernel_init_freeable+0x1bc/0x284
[    1.215695]  kernel_init+0x24/0x1dc
[    1.215697]  ret_from_fork+0x10/0x20
[    1.215701] SMP: stopping secondary CPUs
[    1.215708] Kernel Offset: 0x40ebaa200000 from 0xffff800080000000
[    1.215710] PHYS_OFFSET: 0xffffc18980000000
[    1.215711] CPU features: 0xc0000001,70020143,1000720b
[    1.215714] Memory Limit: none
> 
> Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> Tested on Odroid HC1 (Exynos5422):
> Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  drivers/tty/serial/samsung_tty.c | 62 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 57 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
> index 326b0164609c..6ef614d8648c 100644
> --- a/drivers/tty/serial/samsung_tty.c
> +++ b/drivers/tty/serial/samsung_tty.c
> @@ -154,10 +154,33 @@ struct s3c24xx_uart_port {
>  #define portaddrl(port, reg) \
>  	((unsigned long *)(unsigned long)((port)->membase + (reg)))
>  
> -#define rd_reg(port, reg) (readb_relaxed(portaddr(port, reg)))
> +static u32 rd_reg(struct uart_port *port, u32 reg)
> +{
> +	switch (port->iotype) {
> +	case UPIO_MEM:
> +		return readb_relaxed(portaddr(port, reg));
> +	case UPIO_MEM32:
> +		return readl_relaxed(portaddr(port, reg));
> +	default:
> +		return 0;
> +	}
> +	return 0;
> +}
> +
>  #define rd_regl(port, reg) (readl_relaxed(portaddr(port, reg)))
>  
> -#define wr_reg(port, reg, val) writeb_relaxed(val, portaddr(port, reg))
> +static void wr_reg(struct uart_port *port, u32 reg, u32 val)
> +{
> +	switch (port->iotype) {
> +	case UPIO_MEM:
> +		writeb_relaxed(val, portaddr(port, reg));
> +		break;
> +	case UPIO_MEM32:
> +		writel_relaxed(val, portaddr(port, reg));
> +		break;
> +	}
> +}
> +
>  #define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg))
>  
>  /* Byte-order aware bit setting/clearing functions. */
> @@ -1974,7 +1997,7 @@ static int s3c24xx_serial_probe(struct platform_device *pdev)
>  	struct device_node *np = pdev->dev.of_node;
>  	struct s3c24xx_uart_port *ourport;
>  	int index = probe_index;
> -	int ret;
> +	int ret, prop = 0;
>  
>  	if (np) {
>  		ret = of_alias_get_id(np, "serial");
> @@ -2000,10 +2023,27 @@ static int s3c24xx_serial_probe(struct platform_device *pdev)
>  			dev_get_platdata(&pdev->dev) :
>  			ourport->drv_data->def_cfg;
>  
> -	if (np)
> +	if (np) {
>  		of_property_read_u32(np,
>  			"samsung,uart-fifosize", &ourport->port.fifosize);
>  
> +		if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
> +			switch (prop) {
> +			case 1:
> +				ourport->port.iotype = UPIO_MEM;
> +				break;
> +			case 4:
> +				ourport->port.iotype = UPIO_MEM32;
> +				break;
> +			default:
> +				dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n",
> +						prop);
> +				ret = -EINVAL;
> +				break;
> +			}
> +		}
> +	}
> +
>  	if (ourport->drv_data->fifosize[index])
>  		ourport->port.fifosize = ourport->drv_data->fifosize[index];
>  	else if (ourport->info->fifosize)
> @@ -2587,6 +2627,18 @@ module_platform_driver(samsung_serial_driver);
>   * Early console.
>   */
>  
> +static void wr_reg_barrier(struct uart_port *port, u32 reg, u32 val)
> +{
> +	switch (port->iotype) {
> +	case UPIO_MEM:
> +		writeb(val, portaddr(port, reg));
> +		break;
> +	case UPIO_MEM32:
> +		writel(val, portaddr(port, reg));
> +		break;
> +	}
> +}
> +
>  struct samsung_early_console_data {
>  	u32 txfull_mask;
>  };
> @@ -2612,7 +2664,7 @@ static void samsung_early_putc(struct uart_port *port, int c)
>  	else
>  		samsung_early_busyuart(port);
>  
> -	writeb(c, port->membase + S3C2410_UTXH);
> +	wr_reg_barrier(port, S3C2410_UTXH, c);
>  }
>  
>  static void samsung_early_write(struct console *con, const char *s,

Re: [PATCH v10 3/3] tty: samsung_tty: 32-bit access for TX/RX hold registers

[Youngmin Nam]

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

On Thu, Nov 30, 2023 at 10:19:40AM +0000, Tudor Ambarus wrote:
> I'm trying my luck on a 3 year old patch, fingers crossed :).
> 
> On 5/6/20 09:02, Hyunki Koo wrote:
> 
> Hi, Hyunki!
> 
> > Support 32-bit access for the TX/RX hold registers UTXH and URXH.
> > 
> > This is required for some newer SoCs.
> 
> Is the 32-bit access mandatory for the newer SoCs? I'm asking because I
> see a SError Interrupt when using 8-bit accesses on gs101 (pixel6). Can
> you confirm if gs101 USI UART is restricted to only 32 bit data width
> accesses?
> 
> If 32-bit accesses are mandatory, we should impose this restriction so
> that we avoid crashing the kernel on wrong user configuration. I have
> some patches ready, waiting just for a confirmation.

Hi.

Yes, some newer exynos SoCs allow only 32bit access to UART TX / RX register.
This feature is related with SoC Bus architecture so it should be controlled by "reg-io-width" in
UART device tree.

Thanks.

> 
> Below is the crash that I see. Thanks!
> 
> [    1.215459] SError Interrupt on CPU6, code 0x00000000be000411 -- SError
> [    1.215464] CPU: 6 PID: 1 Comm: swapper/0 Not tainted
> 6.6.0-rc1-00021-g31831088e862-dirty #88
> [    1.215468] Hardware name: Oriole DVT (DT)
> [    1.215470] pstate: 204000c5 (nzCv daIF +PAN -UAO -TCO -DIT -SSBS
> BTYPE=--)
> [    1.215473] pc : s3c24xx_serial_console_putchar+0x1c/0x90
> [    1.215487] lr : uart_console_write+0x50/0x6c
> [    1.215491] sp : ffff80008007b6c0
> [    1.215492] x29: ffff80008007b6c0 x28: 0000000000000000 x27:
> ffffc0ec2c855900
> [    1.215498] x26: ffffc0ec2bdee008 x25: 0000000000000000 x24:
> ffffc0ec2c529a40
> [    1.215501] x23: ffff80008007b830 x22: ffffc0ec2c9578b9 x21:
> ffffc0ec2c9b8330
> [    1.215504] x20: ffffc0ec2aa0d684 x19: ffffc0ec2c95788a x18:
> 0000000000000006
> [    1.215507] x17: 0000000000000000 x16: 0000000000000001 x15:
> ffff80008007b460
> [    1.215510] x14: 0000000000000000 x13: 64656c62616e6520 x12:
> 5d30434153797474
> [    1.215513] x11: 5b20656c6f736e6f x10: 63203a6b746e6972 x9 :
> 656c62616e65205d
> [    1.215515] x8 : 304341537974745b x7 : 205d303534353132 x6 :
> 322e31202020205b
> [    1.215518] x5 : ffffc0ec2c9b82f0 x4 : ffffc0ec2b5bc1c0 x3 :
> ffff800080061000
> [    1.215521] x2 : 0000000000000111 x1 : 0000000000000020 x0 :
> ffffc0ec2c9b8330
> [    1.215526] Kernel panic - not syncing: Asynchronous SError Interrupt
> [    1.215528] CPU: 6 PID: 1 Comm: swapper/0 Not tainted
> 6.6.0-rc1-00021-g31831088e862-dirty #88
> [    1.215531] Hardware name: Oriole DVT (DT)
> [    1.215532] Call trace:
> [    1.215533]  dump_backtrace+0x90/0xe8
> [    1.215542]  show_stack+0x18/0x24
> [    1.215547]  dump_stack_lvl+0x48/0x60
> [    1.215555]  dump_stack+0x18/0x24
> [    1.215559]  panic+0x31c/0x378
> [    1.215565]  nmi_panic+0x8c/0x90
> [    1.215569]  arm64_serror_panic+0x6c/0x78
> [    1.215571]  do_serror+0x3c/0x70
> [    1.215574]  el1h_64_error_handler+0x30/0x48
> [    1.215581]  el1h_64_error+0x64/0x68
> [    1.215583]  s3c24xx_serial_console_putchar+0x1c/0x90
> [    1.215588]  s3c24xx_serial_console_write+0xb8/0xfc
> [    1.215593]  console_flush_all.isra.0+0x168/0x3ac
> [    1.215600]  console_unlock+0x58/0xdc
> [    1.215604]  vprintk_emit+0x1a8/0x2b0
> [    1.215608]  vprintk_default+0x38/0x44
> [    1.215613]  vprintk+0xbc/0xd0
> [    1.215617]  _printk+0x5c/0x84
> [    1.215621]  register_console+0x2a0/0x400
> [    1.215625]  serial_core_register_port+0x61c/0x658
> [    1.215630]  serial_ctrl_register_port+0x10/0x1c
> [    1.215634]  uart_add_one_port+0x10/0x1c
> [    1.215639]  s3c24xx_serial_probe+0x360/0x6fc
> [    1.215644]  platform_probe+0x68/0xc8
> [    1.215650]  really_probe+0x148/0x2b0
> [    1.215653]  __driver_probe_device+0x78/0x12c
> [    1.215656]  driver_probe_device+0x3c/0x15c
> [    1.215658]  __driver_attach+0x90/0x19c
> [    1.215661]  bus_for_each_dev+0x78/0xd4
> [    1.215668]  driver_attach+0x24/0x30
> [    1.215670]  bus_add_driver+0xe8/0x1f8
> [    1.215672]  driver_register+0x5c/0x124
> [    1.215675]  __platform_driver_register+0x28/0x34
> [    1.215678]  samsung_serial_init+0x30/0x8c
> [    1.215687]  do_one_initcall+0x7c/0x1c0
> [    1.215689]  kernel_init_freeable+0x1bc/0x284
> [    1.215695]  kernel_init+0x24/0x1dc
> [    1.215697]  ret_from_fork+0x10/0x20
> [    1.215701] SMP: stopping secondary CPUs
> [    1.215708] Kernel Offset: 0x40ebaa200000 from 0xffff800080000000
> [    1.215710] PHYS_OFFSET: 0xffffc18980000000
> [    1.215711] CPU features: 0xc0000001,70020143,1000720b
> [    1.215714] Memory Limit: none
> > 
> > Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com>
> > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> > Tested on Odroid HC1 (Exynos5422):
> > Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
> > ---
> >  drivers/tty/serial/samsung_tty.c | 62 ++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 57 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
> > index 326b0164609c..6ef614d8648c 100644
> > --- a/drivers/tty/serial/samsung_tty.c
> > +++ b/drivers/tty/serial/samsung_tty.c
> > @@ -154,10 +154,33 @@ struct s3c24xx_uart_port {
> >  #define portaddrl(port, reg) \
> >  	((unsigned long *)(unsigned long)((port)->membase + (reg)))
> >  
> > -#define rd_reg(port, reg) (readb_relaxed(portaddr(port, reg)))
> > +static u32 rd_reg(struct uart_port *port, u32 reg)
> > +{
> > +	switch (port->iotype) {
> > +	case UPIO_MEM:
> > +		return readb_relaxed(portaddr(port, reg));
> > +	case UPIO_MEM32:
> > +		return readl_relaxed(portaddr(port, reg));
> > +	default:
> > +		return 0;
> > +	}
> > +	return 0;
> > +}
> > +
> >  #define rd_regl(port, reg) (readl_relaxed(portaddr(port, reg)))
> >  
> > -#define wr_reg(port, reg, val) writeb_relaxed(val, portaddr(port, reg))
> > +static void wr_reg(struct uart_port *port, u32 reg, u32 val)
> > +{
> > +	switch (port->iotype) {
> > +	case UPIO_MEM:
> > +		writeb_relaxed(val, portaddr(port, reg));
> > +		break;
> > +	case UPIO_MEM32:
> > +		writel_relaxed(val, portaddr(port, reg));
> > +		break;
> > +	}
> > +}
> > +
> >  #define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg))
> >  
> >  /* Byte-order aware bit setting/clearing functions. */
> > @@ -1974,7 +1997,7 @@ static int s3c24xx_serial_probe(struct platform_device *pdev)
> >  	struct device_node *np = pdev->dev.of_node;
> >  	struct s3c24xx_uart_port *ourport;
> >  	int index = probe_index;
> > -	int ret;
> > +	int ret, prop = 0;
> >  
> >  	if (np) {
> >  		ret = of_alias_get_id(np, "serial");
> > @@ -2000,10 +2023,27 @@ static int s3c24xx_serial_probe(struct platform_device *pdev)
> >  			dev_get_platdata(&pdev->dev) :
> >  			ourport->drv_data->def_cfg;
> >  
> > -	if (np)
> > +	if (np) {
> >  		of_property_read_u32(np,
> >  			"samsung,uart-fifosize", &ourport->port.fifosize);
> >  
> > +		if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
> > +			switch (prop) {
> > +			case 1:
> > +				ourport->port.iotype = UPIO_MEM;
> > +				break;
> > +			case 4:
> > +				ourport->port.iotype = UPIO_MEM32;
> > +				break;
> > +			default:
> > +				dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n",
> > +						prop);
> > +				ret = -EINVAL;
> > +				break;
> > +			}
> > +		}
> > +	}
> > +
> >  	if (ourport->drv_data->fifosize[index])
> >  		ourport->port.fifosize = ourport->drv_data->fifosize[index];
> >  	else if (ourport->info->fifosize)
> > @@ -2587,6 +2627,18 @@ module_platform_driver(samsung_serial_driver);
> >   * Early console.
> >   */
> >  
> > +static void wr_reg_barrier(struct uart_port *port, u32 reg, u32 val)
> > +{
> > +	switch (port->iotype) {
> > +	case UPIO_MEM:
> > +		writeb(val, portaddr(port, reg));
> > +		break;
> > +	case UPIO_MEM32:
> > +		writel(val, portaddr(port, reg));
> > +		break;
> > +	}
> > +}
> > +
> >  struct samsung_early_console_data {
> >  	u32 txfull_mask;
> >  };
> > @@ -2612,7 +2664,7 @@ static void samsung_early_putc(struct uart_port *port, int c)
> >  	else
> >  		samsung_early_busyuart(port);
> >  
> > -	writeb(c, port->membase + S3C2410_UTXH);
> > +	wr_reg_barrier(port, S3C2410_UTXH, c);
> >  }
> >  
> >  static void samsung_early_write(struct console *con, const char *s,
> 
> 

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