[PATCH 0/5] MIPS/tty/8250: Use standard 8250 drivers for OCTEON

[PATCH 0/5] MIPS/tty/8250: Use standard 8250 drivers for OCTEON

[David Daney]

From: David Daney <david.daney@cavium.com>

Get rid of the custom OCTEON UART probe code and use 8250_dw instead.

The first patch just gets rid of Ralf's Kconfig workarounds for the
real problem, which is OCTEON's inclomplete serial support.

Then we just make minor patches to 8250_dw, and rip out all this
OCTEON code.

Since the patches are all interdependent, we might want to merge them
via a single tree (perhaps Ralf's MIPS tree).

David Daney (5):
  Revert "MIPS: Octeon: Fix build error if CONFIG_SERIAL_8250=n"
  MIPS: OCTEON: Set proper UART clock in internal device trees.
  tty/8250_dw: Add support for OCTEON UARTS.
  MIPS: OCTEON: Remove custom serial setup code.
  MIPS: Update cavium_octeon_defconfig

 arch/mips/cavium-octeon/Makefile          |   1 -
 arch/mips/cavium-octeon/octeon-platform.c |   9 ++-
 arch/mips/cavium-octeon/serial.c          | 109 ------------------------------
 arch/mips/configs/cavium_octeon_defconfig |   4 +-
 drivers/tty/serial/8250/8250_dw.c         |  45 ++++++++++--
 5 files changed, 48 insertions(+), 120 deletions(-)
 delete mode 100644 arch/mips/cavium-octeon/serial.c

-- 
1.7.11.7

[PATCH 1/5] Revert "MIPS: Octeon: Fix build error if CONFIG_SERIAL_8250=n"

[David Daney]

From: David Daney <david.daney@cavium.com>

This reverts commit fc0fcde2ea9740944acf6134d2c84983d1297bc1.
---
 arch/mips/cavium-octeon/Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/mips/cavium-octeon/Makefile b/arch/mips/cavium-octeon/Makefile
index 643809f..e3fd50c 100644
--- a/arch/mips/cavium-octeon/Makefile
+++ b/arch/mips/cavium-octeon/Makefile
@@ -12,13 +12,12 @@
 CFLAGS_octeon-platform.o = -I$(src)/../../../scripts/dtc/libfdt
 CFLAGS_setup.o = -I$(src)/../../../scripts/dtc/libfdt
 
-obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o
+obj-y := cpu.o setup.o serial.o octeon-platform.o octeon-irq.o csrc-octeon.o
 obj-y += dma-octeon.o
 obj-y += octeon-memcpy.o
 obj-y += executive/
 
 obj-$(CONFIG_MTD)		      += flash_setup.o
-obj-$(CONFIG_SERIAL_8250)	      += serial.o
 obj-$(CONFIG_SMP)		      += smp.o
 obj-$(CONFIG_OCTEON_ILM)	      += oct_ilm.o
 
-- 
1.7.11.7

[PATCH 2/5] MIPS: OCTEON: Set proper UART clock in internal device trees.

[David Daney]

From: David Daney <david.daney@cavium.com>

Following patch to use generic 8250 drivers will need proper clock
information.  So when using the internal device tree, populate the
"clock-frequency" property with the correct value.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 arch/mips/cavium-octeon/octeon-platform.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/mips/cavium-octeon/octeon-platform.c b/arch/mips/cavium-octeon/octeon-platform.c
index 389512e..7b746e7 100644
--- a/arch/mips/cavium-octeon/octeon-platform.c
+++ b/arch/mips/cavium-octeon/octeon-platform.c
@@ -490,8 +490,15 @@ int __init octeon_prune_device_tree(void)
 
 		if (alias_prop) {
 			uart = fdt_path_offset(initial_boot_params, alias_prop);
-			if (uart_mask & (1 << i))
+			if (uart_mask & (1 << i)) {
+				__be32 f;
+
+				f = cpu_to_be32(octeon_get_io_clock_rate());
+				fdt_setprop_inplace(initial_boot_params,
+						    uart, "clock-frequency",
+						    &f, sizeof(f));
 				continue;
+			}
 			pr_debug("Deleting uart%d\n", i);
 			fdt_nop_node(initial_boot_params, uart);
 			fdt_nop_property(initial_boot_params, aliases,
-- 
1.7.11.7

[PATCH 3/5] tty/8250_dw: Add support for OCTEON UARTS.

[David Daney]

From: David Daney <david.daney@cavium.com>

A few differences needed by OCTEON:

o These are DWC UARTS, but have USR at a different offset.

o OCTEON must have 64-bit wide register accesses, so we have OCTEON
  specific register accessors.

o No UCV register, so we hard code some properties.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/tty/serial/8250/8250_dw.c | 45 +++++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index d07b6af..a50c1d5 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -57,8 +57,30 @@ struct dw8250_data {
 	int		last_lcr;
 	int		line;
 	struct clk	*clk;
+	u8		usr_reg;
+	bool		no_ucv;
 };
 
+static unsigned int dw8250_serial_inq(struct uart_port *p, int offset)
+{
+	offset <<= p->regshift;
+
+	return (u8)__raw_readq(p->membase + offset);
+}
+
+static void dw8250_serial_outq(struct uart_port *p, int offset, int value)
+{
+	struct dw8250_data *d = p->private_data;
+
+	if (offset == UART_LCR)
+		d->last_lcr = value;
+
+	offset <<= p->regshift;
+	__raw_writeq(value, p->membase + offset);
+	dw8250_serial_inq(p, UART_LCR);
+}
+
+
 static void dw8250_serial_out(struct uart_port *p, int offset, int value)
 {
 	struct dw8250_data *d = p->private_data;
@@ -104,7 +126,7 @@ static int dw8250_handle_irq(struct uart_port *p)
 		return 1;
 	} else if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
 		/* Clear the USR and write the LCR again. */
-		(void)p->serial_in(p, DW_UART_USR);
+		(void)p->serial_in(p, d->usr_reg);
 		p->serial_out(p, UART_LCR, d->last_lcr);
 
 		return 1;
@@ -125,12 +147,20 @@ dw8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
 		pm_runtime_put_sync_suspend(port->dev);
 }
 
-static int dw8250_probe_of(struct uart_port *p)
+static int dw8250_probe_of(struct uart_port *p,
+			   struct dw8250_data *data)
 {
 	struct device_node	*np = p->dev->of_node;
 	u32			val;
 
-	if (!of_property_read_u32(np, "reg-io-width", &val)) {
+	if (of_device_is_compatible(np, "cavium,octeon-3860-uart")) {
+		p->serial_in = dw8250_serial_inq;
+		p->serial_out = dw8250_serial_outq;
+		p->flags = ASYNC_SKIP_TEST | UPF_SHARE_IRQ | UPF_FIXED_TYPE;
+		p->type = PORT_OCTEON;
+		data->usr_reg = 0x27;
+		data->no_ucv = true;
+	} else if (!of_property_read_u32(np, "reg-io-width", &val)) {
 		switch (val) {
 		case 1:
 			break;
@@ -259,6 +289,7 @@ static int dw8250_probe(struct platform_device *pdev)
 	if (!data)
 		return -ENOMEM;
 
+	data->usr_reg = DW_UART_USR;
 	data->clk = devm_clk_get(&pdev->dev, NULL);
 	if (!IS_ERR(data->clk)) {
 		clk_prepare_enable(data->clk);
@@ -270,10 +301,8 @@ static int dw8250_probe(struct platform_device *pdev)
 	uart.port.serial_out = dw8250_serial_out;
 	uart.port.private_data = data;
 
-	dw8250_setup_port(&uart);
-
 	if (pdev->dev.of_node) {
-		err = dw8250_probe_of(&uart.port);
+		err = dw8250_probe_of(&uart.port, data);
 		if (err)
 			return err;
 	} else if (ACPI_HANDLE(&pdev->dev)) {
@@ -284,6 +313,9 @@ static int dw8250_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	if (!data->no_ucv)
+		dw8250_setup_port(&uart);
+
 	data->line = serial8250_register_8250_port(&uart);
 	if (data->line < 0)
 		return data->line;
@@ -362,6 +394,7 @@ static const struct dev_pm_ops dw8250_pm_ops = {
 
 static const struct of_device_id dw8250_of_match[] = {
 	{ .compatible = "snps,dw-apb-uart" },
+	{ .compatible = "cavium,octeon-3860-uart" },
 	{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, dw8250_of_match);
-- 
1.7.11.7

[PATCH 4/5] MIPS: OCTEON: Remove custom serial setup code.

[David Daney]

From: David Daney <david.daney@cavium.com>

We will use 8250_dw instead.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 arch/mips/cavium-octeon/Makefile |   2 +-
 arch/mips/cavium-octeon/serial.c | 109 ---------------------------------------
 2 files changed, 1 insertion(+), 110 deletions(-)
 delete mode 100644 arch/mips/cavium-octeon/serial.c

diff --git a/arch/mips/cavium-octeon/Makefile b/arch/mips/cavium-octeon/Makefile
index e3fd50c..4e95204 100644
--- a/arch/mips/cavium-octeon/Makefile
+++ b/arch/mips/cavium-octeon/Makefile
@@ -12,7 +12,7 @@
 CFLAGS_octeon-platform.o = -I$(src)/../../../scripts/dtc/libfdt
 CFLAGS_setup.o = -I$(src)/../../../scripts/dtc/libfdt
 
-obj-y := cpu.o setup.o serial.o octeon-platform.o octeon-irq.o csrc-octeon.o
+obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o
 obj-y += dma-octeon.o
 obj-y += octeon-memcpy.o
 obj-y += executive/
diff --git a/arch/mips/cavium-octeon/serial.c b/arch/mips/cavium-octeon/serial.c
deleted file mode 100644
index f393f65..0000000
--- a/arch/mips/cavium-octeon/serial.c
+++ /dev/null
@@ -1,109 +0,0 @@
-/*
- * This file is subject to the terms and conditions of the GNU General Public
- * License.  See the file "COPYING" in the main directory of this archive
- * for more details.
- *
- * Copyright (C) 2004-2007 Cavium Networks
- */
-#include <linux/console.h>
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/platform_device.h>
-#include <linux/serial.h>
-#include <linux/serial_8250.h>
-#include <linux/serial_reg.h>
-#include <linux/tty.h>
-#include <linux/irq.h>
-
-#include <asm/time.h>
-
-#include <asm/octeon/octeon.h>
-
-#define DEBUG_UART 1
-
-unsigned int octeon_serial_in(struct uart_port *up, int offset)
-{
-	int rv = cvmx_read_csr((uint64_t)(up->membase + (offset << 3)));
-	if (offset == UART_IIR && (rv & 0xf) == 7) {
-		/* Busy interrupt, read the USR (39) and try again. */
-		cvmx_read_csr((uint64_t)(up->membase + (39 << 3)));
-		rv = cvmx_read_csr((uint64_t)(up->membase + (offset << 3)));
-	}
-	return rv;
-}
-
-void octeon_serial_out(struct uart_port *up, int offset, int value)
-{
-	/*
-	 * If bits 6 or 7 of the OCTEON UART's LCR are set, it quits
-	 * working.
-	 */
-	if (offset == UART_LCR)
-		value &= 0x9f;
-	cvmx_write_csr((uint64_t)(up->membase + (offset << 3)), (u8)value);
-}
-
-static int octeon_serial_probe(struct platform_device *pdev)
-{
-	int irq, res;
-	struct resource *res_mem;
-	struct uart_8250_port up;
-
-	/* All adaptors have an irq.  */
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
-	memset(&up, 0, sizeof(up));
-
-	up.port.flags = ASYNC_SKIP_TEST | UPF_SHARE_IRQ | UPF_FIXED_TYPE;
-	up.port.type = PORT_OCTEON;
-	up.port.iotype = UPIO_MEM;
-	up.port.regshift = 3;
-	up.port.dev = &pdev->dev;
-
-	if (octeon_is_simulation())
-		/* Make simulator output fast*/
-		up.port.uartclk = 115200 * 16;
-	else
-		up.port.uartclk = octeon_get_io_clock_rate();
-
-	up.port.serial_in = octeon_serial_in;
-	up.port.serial_out = octeon_serial_out;
-	up.port.irq = irq;
-
-	res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (res_mem == NULL) {
-		dev_err(&pdev->dev, "found no memory resource\n");
-		return -ENXIO;
-	}
-	up.port.mapbase = res_mem->start;
-	up.port.membase = ioremap(res_mem->start, resource_size(res_mem));
-
-	res = serial8250_register_8250_port(&up);
-
-	return res >= 0 ? 0 : res;
-}
-
-static struct of_device_id octeon_serial_match[] = {
-	{
-		.compatible = "cavium,octeon-3860-uart",
-	},
-	{},
-};
-MODULE_DEVICE_TABLE(of, octeon_serial_match);
-
-static struct platform_driver octeon_serial_driver = {
-	.probe		= octeon_serial_probe,
-	.driver		= {
-		.owner	= THIS_MODULE,
-		.name	= "octeon_serial",
-		.of_match_table = octeon_serial_match,
-	},
-};
-
-static int __init octeon_serial_init(void)
-{
-	return platform_driver_register(&octeon_serial_driver);
-}
-late_initcall(octeon_serial_init);
-- 
1.7.11.7

[PATCH 5/5] MIPS: Update cavium_octeon_defconfig

[David Daney]

From: David Daney <david.daney@cavium.com>

The serial port changes make it advisable to enable the proper UART
drivers.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 arch/mips/configs/cavium_octeon_defconfig | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/mips/configs/cavium_octeon_defconfig b/arch/mips/configs/cavium_octeon_defconfig
index 1888e5f..dace582 100644
--- a/arch/mips/configs/cavium_octeon_defconfig
+++ b/arch/mips/configs/cavium_octeon_defconfig
@@ -1,13 +1,11 @@
 CONFIG_CAVIUM_OCTEON_SOC=y
 CONFIG_CAVIUM_CN63XXP1=y
 CONFIG_CAVIUM_OCTEON_CVMSEG_SIZE=2
-CONFIG_SPARSEMEM_MANUAL=y
 CONFIG_TRANSPARENT_HUGEPAGE=y
 CONFIG_SMP=y
 CONFIG_NR_CPUS=32
 CONFIG_HZ_100=y
 CONFIG_PREEMPT=y
-CONFIG_EXPERIMENTAL=y
 CONFIG_SYSVIPC=y
 CONFIG_POSIX_MQUEUE=y
 CONFIG_BSD_PROCESS_ACCT=y
@@ -50,7 +48,6 @@ CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
 # CONFIG_FW_LOADER is not set
 CONFIG_MTD=y
 # CONFIG_MTD_OF_PARTS is not set
-CONFIG_MTD_CHAR=y
 CONFIG_MTD_BLOCK=y
 CONFIG_MTD_CFI=y
 CONFIG_MTD_CFI_AMDSTD=y
@@ -114,6 +111,7 @@ CONFIG_SERIAL_8250=y
 CONFIG_SERIAL_8250_CONSOLE=y
 CONFIG_SERIAL_8250_NR_UARTS=2
 CONFIG_SERIAL_8250_RUNTIME_UARTS=2
+CONFIG_SERIAL_8250_DW=y
 # CONFIG_HW_RANDOM is not set
 CONFIG_I2C=y
 CONFIG_I2C_OCTEON=y
-- 
1.7.11.7

Re: [PATCH 3/5] tty/8250_dw: Add support for OCTEON UARTS.

[Greg Kroah-Hartman]

On Tue, Jun 18, 2013 at 12:12:53PM -0700, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> A few differences needed by OCTEON:
> 
> o These are DWC UARTS, but have USR at a different offset.
> 
> o OCTEON must have 64-bit wide register accesses, so we have OCTEON
>   specific register accessors.
> 
> o No UCV register, so we hard code some properties.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  drivers/tty/serial/8250/8250_dw.c | 45 +++++++++++++++++++++++++++++++++------
>  1 file changed, 39 insertions(+), 6 deletions(-)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 0/5] MIPS/tty/8250: Use standard 8250 drivers for OCTEON

[Greg Kroah-Hartman]

On Tue, Jun 18, 2013 at 12:12:50PM -0700, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> Get rid of the custom OCTEON UART probe code and use 8250_dw instead.
> 
> The first patch just gets rid of Ralf's Kconfig workarounds for the
> real problem, which is OCTEON's inclomplete serial support.
> 
> Then we just make minor patches to 8250_dw, and rip out all this
> OCTEON code.
> 
> Since the patches are all interdependent, we might want to merge them
> via a single tree (perhaps Ralf's MIPS tree).

That's fine with me, I've acked the tty driver changes so feel free to
take all of these through the mips tree.

thanks,

greg k-h

Re: [PATCH 0/5] MIPS/tty/8250: Use standard 8250 drivers for OCTEON

[Ralf Baechle]

On Tue, Jun 18, 2013 at 12:12:50PM -0700, David Daney wrote:

> From: David Daney <david.daney@cavium.com>
> 
> Get rid of the custom OCTEON UART probe code and use 8250_dw instead.
> 
> The first patch just gets rid of Ralf's Kconfig workarounds for the
> real problem, which is OCTEON's inclomplete serial support.
> 
> Then we just make minor patches to 8250_dw, and rip out all this
> OCTEON code.
> 
> Since the patches are all interdependent, we might want to merge them
> via a single tree (perhaps Ralf's MIPS tree).

Looks good - I was trying to come up with a kludge good enough for 3.10;
this may be a bit too large ...

  Ralf

Re: [PATCH 0/5] MIPS/tty/8250: Use standard 8250 drivers for OCTEON

[David Daney]

On 06/18/2013 12:36 PM, Ralf Baechle wrote:
> On Tue, Jun 18, 2013 at 12:12:50PM -0700, David Daney wrote:
>
>> From: David Daney <david.daney@cavium.com>
>>
>> Get rid of the custom OCTEON UART probe code and use 8250_dw instead.
>>
>> The first patch just gets rid of Ralf's Kconfig workarounds for the
>> real problem, which is OCTEON's inclomplete serial support.
>>
>> Then we just make minor patches to 8250_dw, and rip out all this
>> OCTEON code.
>>
>> Since the patches are all interdependent, we might want to merge them
>> via a single tree (perhaps Ralf's MIPS tree).
>
> Looks good - I was trying to come up with a kludge good enough for 3.10;
> this may be a bit too large ...
>

At this point I think it is fine if some random configs for OCTEON fail 
to build in v3.10.

I was thinking that this would be for 3.11


David Daney

Re: [PATCH 0/5] MIPS/tty/8250: Use standard 8250 drivers for OCTEON

[Jamie Iles]

On Tue, Jun 18, 2013 at 12:12:50PM -0700, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> Get rid of the custom OCTEON UART probe code and use 8250_dw instead.
> 
> The first patch just gets rid of Ralf's Kconfig workarounds for the
> real problem, which is OCTEON's inclomplete serial support.
> 
> Then we just make minor patches to 8250_dw, and rip out all this
> OCTEON code.
> 
> Since the patches are all interdependent, we might want to merge them
> via a single tree (perhaps Ralf's MIPS tree).

Looks good!

Reviewed-by: Jamie Iles <jamie@jamieiles.com>

for the series.

Thanks,

Jamie

Re: [PATCH 3/5] tty/8250_dw: Add support for OCTEON UARTS.

[Arnd Bergmann]

On Tuesday 18 June 2013 12:12:53 David Daney wrote:
> +static unsigned int dw8250_serial_inq(struct uart_port *p, int offset)
> +{
> +       offset <<= p->regshift;
> +
> +       return (u8)__raw_readq(p->membase + offset);
> +}
> +
> +static void dw8250_serial_outq(struct uart_port *p, int offset, int value)
> +{
> +       struct dw8250_data *d = p->private_data;
> +
> +       if (offset == UART_LCR)
> +               d->last_lcr = value;
> +
> +       offset <<= p->regshift;
> +       __raw_writeq(value, p->membase + offset);
> +       dw8250_serial_inq(p, UART_LCR);
> +}

This breaks building on 32 bit architectures as I found on my daily ARM
builds: __raw_writeq cannot be defined on architectures that don't have
native 64 bit data access instructions. It's also wrong to use the
__raw_* variant, which is not guaranteed to be atomic and is not
endian-safe.

	Arnd

Re: [PATCH 3/5] tty/8250_dw: Add support for OCTEON UARTS.

[Ralf Baechle]

On Wed, Jun 19, 2013 at 12:01:06PM +0200, Arnd Bergmann wrote:

> This breaks building on 32 bit architectures as I found on my daily ARM
> builds: __raw_writeq cannot be defined on architectures that don't have
> native 64 bit data access instructions. It's also wrong to use the
> __raw_* variant, which is not guaranteed to be atomic and is not
> endian-safe.

I've dequeued the series from the mips-next tree until David has a chance
to fix this.

  Ralf

Re: [PATCH 3/5] tty/8250_dw: Add support for OCTEON UARTS.

[Heikki Krogerus]

On Tue, Jun 18, 2013 at 12:12:53PM -0700, David Daney wrote:
> A few differences needed by OCTEON:
> 
> o These are DWC UARTS, but have USR at a different offset.
> 
> o OCTEON must have 64-bit wide register accesses, so we have OCTEON
>   specific register accessors.
> 
> o No UCV register, so we hard code some properties.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>

<snip>

> @@ -270,10 +301,8 @@ static int dw8250_probe(struct platform_device *pdev)
>  	uart.port.serial_out = dw8250_serial_out;
>  	uart.port.private_data = data;
>  
> -	dw8250_setup_port(&uart);
> -
>  	if (pdev->dev.of_node) {
> -		err = dw8250_probe_of(&uart.port);
> +		err = dw8250_probe_of(&uart.port, data);
>  		if (err)
>  			return err;
>  	} else if (ACPI_HANDLE(&pdev->dev)) {
> @@ -284,6 +313,9 @@ static int dw8250_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> +	if (!data->no_ucv)
> +		dw8250_setup_port(&uart);

Moving the dw8250_setup_port() call here breaks dw8250_probe_acpi(). It
expects values from the CPR register for the DMA burst size calculation.

The DMA support can be moved to a separate function. This way it can
be called after this point, and it will then be available for both DT
and ACPI. I can make a patch tomorrow. That should solve this issue.

Thanks,

-- 
heikki

Re: [PATCH 3/5] tty/8250_dw: Add support for OCTEON UARTS.

[David Daney]

On 06/19/2013 03:01 AM, Arnd Bergmann wrote:
> On Tuesday 18 June 2013 12:12:53 David Daney wrote:
>> +static unsigned int dw8250_serial_inq(struct uart_port *p, int offset)
>> +{
>> +       offset <<= p->regshift;
>> +
>> +       return (u8)__raw_readq(p->membase + offset);
>> +}
>> +
>> +static void dw8250_serial_outq(struct uart_port *p, int offset, int value)
>> +{
>> +       struct dw8250_data *d = p->private_data;
>> +
>> +       if (offset == UART_LCR)
>> +               d->last_lcr = value;
>> +
>> +       offset <<= p->regshift;
>> +       __raw_writeq(value, p->membase + offset);
>> +       dw8250_serial_inq(p, UART_LCR);
>> +}
>
> This breaks building on 32 bit architectures as I found on my daily ARM
> builds: __raw_writeq cannot be defined on architectures that don't have
> native 64 bit data access instructions.

I will rework the patch to avoid this problem.


> It's also wrong to use the
> __raw_* variant, which is not guaranteed to be atomic and is not
> endian-safe.

We do runtime probing and only use this function on platforms where it 
is appropriate, so atomicity is not an issue.  As for endianess, I used 
the __raw_ variant precisely because it is correct for both big and 
little endian kernels.

David Daney

Re: [PATCH 3/5] tty/8250_dw: Add support for OCTEON UARTS.

[David Daney]

On 06/19/2013 07:10 AM, Heikki Krogerus wrote:
> On Tue, Jun 18, 2013 at 12:12:53PM -0700, David Daney wrote:
>> A few differences needed by OCTEON:
>>
>> o These are DWC UARTS, but have USR at a different offset.
>>
>> o OCTEON must have 64-bit wide register accesses, so we have OCTEON
>>    specific register accessors.
>>
>> o No UCV register, so we hard code some properties.
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>
> <snip>
>
>> @@ -270,10 +301,8 @@ static int dw8250_probe(struct platform_device *pdev)
>>   	uart.port.serial_out = dw8250_serial_out;
>>   	uart.port.private_data = data;
>>
>> -	dw8250_setup_port(&uart);
>> -
>>   	if (pdev->dev.of_node) {
>> -		err = dw8250_probe_of(&uart.port);
>> +		err = dw8250_probe_of(&uart.port, data);
>>   		if (err)
>>   			return err;
>>   	} else if (ACPI_HANDLE(&pdev->dev)) {
>> @@ -284,6 +313,9 @@ static int dw8250_probe(struct platform_device *pdev)
>>   		return -ENODEV;
>>   	}
>>
>> +	if (!data->no_ucv)
>> +		dw8250_setup_port(&uart);
>
> Moving the dw8250_setup_port() call here breaks dw8250_probe_acpi(). It
> expects values from the CPR register for the DMA burst size calculation.
>
> The DMA support can be moved to a separate function. This way it can
> be called after this point, and it will then be available for both DT
> and ACPI. I can make a patch tomorrow. That should solve this issue.
>

I am reworking the patch because other problems were found.  I will try 
to get this part right in the next version.

David Daney

Re: [PATCH 3/5] tty/8250_dw: Add support for OCTEON UARTS.

[Arnd Bergmann]

On Wednesday 19 June 2013, David Daney wrote:
> On 06/19/2013 03:01 AM, Arnd Bergmann wrote:

> > It's also wrong to use the
> > __raw_* variant, which is not guaranteed to be atomic and is not
> > endian-safe.
> 
> We do runtime probing and only use this function on platforms where it 
> is appropriate, so atomicity is not an issue.  As for endianess, I used 
> the __raw_ variant precisely because it is correct for both big and 
> little endian kernels.

You don't know what the compiler turns a __raw_writeq into, it could
always to eight byte wise stores, that's why typically writeq is
an inline assembly while __raw_writeq is just a pointer dereference.

__raw_* never do endian swaps, so it will be wrong on either big-endian
CPUs or on little-endian CPUs, depending on what the MMIO register
needs.

	Arnd

Re: [PATCH 3/5] tty/8250_dw: Add support for OCTEON UARTS.

[David Daney]

On 06/19/2013 11:52 AM, Arnd Bergmann wrote:
> On Wednesday 19 June 2013, David Daney wrote:
>> On 06/19/2013 03:01 AM, Arnd Bergmann wrote:
>
>>> It's also wrong to use the
>>> __raw_* variant, which is not guaranteed to be atomic and is not
>>> endian-safe.
>>
>> We do runtime probing and only use this function on platforms where it
>> is appropriate, so atomicity is not an issue.  As for endianess, I used
>> the __raw_ variant precisely because it is correct for both big and
>> little endian kernels.
>
> You don't know what the compiler turns a __raw_writeq into, it could
> always to eight byte wise stores, that's why typically writeq is
> an inline assembly while __raw_writeq is just a pointer dereference.

Well, I do know that for the cases of interest, it will be a single load 
or store, but it is moot, as I rewrote that part.

>
> __raw_* never do endian swaps,

Yes, I know that.

> so it will be wrong on either big-endian
> CPUs or on little-endian CPUs, depending on what the MMIO register
> needs.

Please see the instruction set reference manual 
(MD00087-2B-MIPS64BIS-AFP-03.51 or similar) available at:

http://www.mips.com/products/architectures/mips64/#specifications

... for why you are mistaken.  Pay particular attention to the low order 
address bit scrambling on narrow loads and stores and how this results 
in uniform (not affected by processor endian mode) load and store 
results for aligned 64-bit accesses.  In effect, it is magic, and 
__raw_writeq yields correct results in both big and little endian modes 
of operation.

David Daney.