diff mbox series

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

Message ID 20200506080242.18623-3-hyunki00.koo@samsung.com (mailing list archive)
State Not Applicable
Headers show
Series [v10,1/3] serial: samsung: Replace rd_regb/wr_regb with rd_reg/wr_reg | expand

Commit Message

Hyunki Koo May 6, 2020, 8:02 a.m. UTC
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(-)

Comments

Bartlomiej Zolnierkiewicz May 6, 2020, 11:02 a.m. UTC | #1
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,
>
Hyunki Koo May 6, 2020, 11:05 p.m. UTC | #2
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
Jiri Slaby May 19, 2020, 5:55 a.m. UTC | #3
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,
Tudor Ambarus Nov. 30, 2023, 10:19 a.m. UTC | #4
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,
Youngmin Nam Nov. 30, 2023, 11:20 a.m. UTC | #5
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,
> 
>
diff mbox series

Patch

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,