From: David Daney <[email protected]>
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
From: David Daney <[email protected]>
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
From: David Daney <[email protected]>
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 <[email protected]>
---
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
From: David Daney <[email protected]>
The serial port changes make it advisable to enable the proper UART
drivers.
Signed-off-by: David Daney <[email protected]>
---
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
From: David Daney <[email protected]>
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 <[email protected]>
---
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
From: David Daney <[email protected]>
We will use 8250_dw instead.
Signed-off-by: David Daney <[email protected]>
---
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
On Tue, Jun 18, 2013 at 12:12:53PM -0700, David Daney wrote:
> From: David Daney <[email protected]>
>
> 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 <[email protected]>
> ---
> drivers/tty/serial/8250/8250_dw.c | 45 +++++++++++++++++++++++++++++++++------
> 1 file changed, 39 insertions(+), 6 deletions(-)
Acked-by: Greg Kroah-Hartman <[email protected]>
On Tue, Jun 18, 2013 at 12:12:50PM -0700, David Daney wrote:
> From: David Daney <[email protected]>
>
> 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
On Tue, Jun 18, 2013 at 12:12:50PM -0700, David Daney wrote:
> From: David Daney <[email protected]>
>
> 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
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 <[email protected]>
>>
>> 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
On Tue, Jun 18, 2013 at 12:12:50PM -0700, David Daney wrote:
> From: David Daney <[email protected]>
>
> 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 <[email protected]>
for the series.
Thanks,
Jamie
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
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
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 <[email protected]>
<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
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
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 <[email protected]>
>
> <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
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
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.