[PATCH V3 0/7] serial: Configure {big,native}-endian MMIO accesses via DT

[PATCH V3 0/7] serial: Configure {big,native}-endian MMIO accesses via DT

[Kevin Cernekee]

My last submission attempted to work around serial driver coexistence
problems on multiplatform kernels.  Since there are still questions
surrounding the best way to solve that problem, this patch series
will focus on the narrower topic of big endian MMIO support on serial.


V2->V3:

 - Document the new DT properties.

 - Add libfdt-based wrapper, to complement the "struct device_node" based
   version.

 - Restructure early_init_dt_scan_chosen_serial() changes to use a
   temporary variable, so it is easy to add more of_setup_earlycon()
   properties later.

 - Make of_serial and serial8250 honor the new "big-endian" property.


This series applies cleanly to:

git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git devicetree/next-overlay

but was tested on the mips-for-linux-next branch because my BE platform
isn't supported in mainline yet.


Kevin Cernekee (7):
  of: Add helper function to check MMIO register endianness
  of/fdt: Add endianness helper function for early init code
  of: Document {little,big,native}-endian bindings
  serial: core: Add big-endian iotype
  serial: earlycon: Set UPIO_MEM32BE based on DT properties
  serial: of_serial: Support big-endian register accesses
  serial: 8250: Add support for big-endian MMIO accesses

 .../devicetree/bindings/common-properties.txt      | 60 ++++++++++++++++++++++
 drivers/of/base.c                                  | 23 +++++++++
 drivers/of/fdt.c                                   | 26 +++++++++-
 drivers/tty/serial/8250/8250_core.c                | 20 ++++++++
 drivers/tty/serial/8250/8250_early.c               |  5 ++
 drivers/tty/serial/earlycon.c                      |  4 +-
 drivers/tty/serial/of_serial.c                     |  3 +-
 drivers/tty/serial/serial_core.c                   |  2 +
 include/linux/of.h                                 |  6 +++
 include/linux/of_fdt.h                             |  2 +
 include/linux/serial_core.h                        | 15 +++---
 11 files changed, 155 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/common-properties.txt

-- 
2.1.0

[PATCH V3 1/7] of: Add helper function to check MMIO register endianness

[Kevin Cernekee]

SoC peripherals can come in several different flavors:

 - little-endian: registers always need to be accessed in LE mode (so the
   kernel should perform a swap if the CPU is running BE)

 - big-endian: registers always need to be accessed in BE mode (so the
   kernel should perform a swap if the CPU is running LE)

 - native-endian: the bus will automatically swap accesses, so the kernel
   should never swap

Introduce a function that checks an OF device node to see whether it
contains a "big-endian" or "native-endian" property.  For the former case,
always return true.  For the latter case, return true iff the kernel was
built for BE (implying that the BE MMIO accessors do not perform a swap).
Otherwise return false, assuming LE registers.

LE registers are assumed by default because most existing drivers (libahci,
serial8250, usb) always use readl/writel in the absence of instructions
to the contrary, so that will be our fallback.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
---
 drivers/of/base.c  | 23 +++++++++++++++++++++++
 include/linux/of.h |  6 ++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 81c095f..35d95a1 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -552,6 +552,29 @@ bool of_device_is_available(const struct device_node *device)
 EXPORT_SYMBOL(of_device_is_available);
 
 /**
+ *  of_device_is_big_endian - check if a device has BE registers
+ *
+ *  @device: Node to check for endianness
+ *
+ *  Returns true if the device has a "big-endian" property, or if the kernel
+ *  was compiled for BE *and* the device has a "native-endian" property.
+ *  Returns false otherwise.
+ *
+ *  Callers would nominally use ioread32be/iowrite32be if
+ *  of_device_is_big_endian() == true, or readl/writel otherwise.
+ */
+bool of_device_is_big_endian(const struct device_node *device)
+{
+	if (of_property_read_bool(device, "big-endian"))
+		return true;
+	if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) &&
+	    of_property_read_bool(device, "native-endian"))
+		return true;
+	return false;
+}
+EXPORT_SYMBOL(of_device_is_big_endian);
+
+/**
  *	of_get_parent - Get a node's parent if any
  *	@node:	Node to get parent
  *
diff --git a/include/linux/of.h b/include/linux/of.h
index 7aaaa59..fc70b01 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -276,6 +276,7 @@ extern int of_property_read_string_helper(struct device_node *np,
 extern int of_device_is_compatible(const struct device_node *device,
 				   const char *);
 extern bool of_device_is_available(const struct device_node *device);
+extern bool of_device_is_big_endian(const struct device_node *device);
 extern const void *of_get_property(const struct device_node *node,
 				const char *name,
 				int *lenp);
@@ -431,6 +432,11 @@ static inline bool of_device_is_available(const struct device_node *device)
 	return false;
 }
 
+static inline bool of_device_is_big_endian(const struct device_node *device)
+{
+	return false;
+}
+
 static inline struct property *of_find_property(const struct device_node *np,
 						const char *name,
 						int *lenp)
-- 
2.1.0

[PATCH V3 2/7] of/fdt: Add endianness helper function for early init code

[Kevin Cernekee]

Provide a libfdt-based equivalent for of_device_is_big_endian(), suitable
for use in the early_init_* functions.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
---
 drivers/of/fdt.c       | 19 +++++++++++++++++++
 include/linux/of_fdt.h |  2 ++
 2 files changed, 21 insertions(+)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 30e97bc..658656f 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -107,6 +107,25 @@ int of_fdt_is_compatible(const void *blob,
 }
 
 /**
+ * of_fdt_is_big_endian - Return true if given node needs BE MMIO accesses
+ * @blob: A device tree blob
+ * @node: node to test
+ *
+ * Returns true if the node has a "big-endian" property, or if the kernel
+ * was compiled for BE *and* the node has a "native-endian" property.
+ * Returns false otherwise.
+ */
+bool of_fdt_is_big_endian(const void *blob, unsigned long node)
+{
+	if (fdt_getprop(blob, node, "big-endian", NULL))
+		return true;
+	if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) &&
+	    fdt_getprop(blob, node, "native-endian", NULL))
+		return true;
+	return false;
+}
+
+/**
  * of_fdt_match - Return true if node matches a list of compatible values
  */
 int of_fdt_match(const void *blob, unsigned long node,
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 0ff360d5b..587ee50 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -33,6 +33,8 @@ extern void *of_fdt_get_property(const void *blob,
 extern int of_fdt_is_compatible(const void *blob,
 				unsigned long node,
 				const char *compat);
+extern bool of_fdt_is_big_endian(const void *blob,
+				 unsigned long node);
 extern int of_fdt_match(const void *blob, unsigned long node,
 			const char *const *compat);
 extern void of_fdt_unflatten_tree(unsigned long *blob,
-- 
2.1.0

[PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Kevin Cernekee]

These apply to newly converted drivers, like serial8250/libahci/...
The examples were adapted from the regmap bindings document.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
---
 .../devicetree/bindings/common-properties.txt      | 60 ++++++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/common-properties.txt

diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
new file mode 100644
index 0000000..21044a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/common-properties.txt
@@ -0,0 +1,60 @@
+Common properties
+
+The ePAPR specification does not define any properties related to hardware
+byteswapping, but endianness issues show up frequently in porting Linux to
+different machine types.  This document attempts to provide a consistent
+way of handling byteswapping across drivers.
+
+Optional properties:
+ - big-endian: Boolean; force big endian register accesses
+   unconditionally (e.g. ioread32be/iowrite32be).  Use this if you
+   know the peripheral always needs to be accessed in BE mode.
+ - little-endian: Boolean; force little endian register accesses
+   unconditionally (e.g. readl/writel).  Use this if you know the
+   peripheral always needs to be accessed in LE mode.  This is the
+   default.
+ - native-endian: Boolean; always use register accesses matched to the
+   endianness of the kernel binary (e.g. LE vmlinux -> readl/writel,
+   BE vmlinux -> ioread32be/iowrite32be).  In this case no byteswaps
+   will ever be performed.  Use this if the hardware "self-adjusts"
+   register endianness based on the CPU's configured endianness.
+
+Note that regmap, in contrast, defaults to native-endian.  But this
+document is targeted for existing drivers, most of which currently use
+readl/writel because they expect to be accessing PCI/PCIe devices rather
+than memory-mapped SoC peripherals.  Since the readl/writel accessors
+perform a byteswap on BE systems, this means that the drivers in question
+are implicitly "little-endian".
+
+Examples:
+Scenario 1 : CPU in LE mode & device in LE mode.
+dev: dev@40031000 {
+	      compatible = "name";
+	      reg = <0x40031000 0x1000>;
+	      ...
+	      native-endian;
+};
+
+Scenario 2 : CPU in LE mode & device in BE mode.
+dev: dev@40031000 {
+	      compatible = "name";
+	      reg = <0x40031000 0x1000>;
+	      ...
+	      big-endian;
+};
+
+Scenario 3 : CPU in BE mode & device in BE mode.
+dev: dev@40031000 {
+	      compatible = "name";
+	      reg = <0x40031000 0x1000>;
+	      ...
+	      native-endian;
+};
+
+Scenario 4 : CPU in BE mode & device in LE mode.
+dev: dev@40031000 {
+	      compatible = "name";
+	      reg = <0x40031000 0x1000>;
+	      ...
+	      little-endian;
+};
-- 
2.1.0

[PATCH V3 4/7] serial: core: Add big-endian iotype

[Kevin Cernekee]

Since most drivers interpret UPIO_MEM32 to mean "little-endian" and use
readl/writel to access the registers, add a parallel UPIO_MEM32BE to
request the use of big-endian MMIO accessors (ioread32be/iowrite32be).

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
---
 drivers/tty/serial/serial_core.c |  2 ++
 include/linux/serial_core.h      | 13 +++++++------
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index eaeb9a0..ecc7d6c 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2092,6 +2092,7 @@ uart_report_port(struct uart_driver *drv, struct uart_port *port)
 		break;
 	case UPIO_MEM:
 	case UPIO_MEM32:
+	case UPIO_MEM32BE:
 	case UPIO_AU:
 	case UPIO_TSI:
 		snprintf(address, sizeof(address),
@@ -2736,6 +2737,7 @@ int uart_match_port(struct uart_port *port1, struct uart_port *port2)
 		       (port1->hub6   == port2->hub6);
 	case UPIO_MEM:
 	case UPIO_MEM32:
+	case UPIO_MEM32BE:
 	case UPIO_AU:
 	case UPIO_TSI:
 		return (port1->mapbase == port2->mapbase);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 21c2e05..d2d5bf6 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -140,12 +140,13 @@ struct uart_port {
 	unsigned char		iotype;			/* io access style */
 	unsigned char		unused1;
 
-#define UPIO_PORT		(0)
-#define UPIO_HUB6		(1)
-#define UPIO_MEM		(2)
-#define UPIO_MEM32		(3)
-#define UPIO_AU			(4)			/* Au1x00 and RT288x type IO */
-#define UPIO_TSI		(5)			/* Tsi108/109 type IO */
+#define UPIO_PORT		(0)			/* 8b I/O port access */
+#define UPIO_HUB6		(1)			/* Hub6 ISA card */
+#define UPIO_MEM		(2)			/* 8b MMIO access */
+#define UPIO_MEM32		(3)			/* 32b little endian */
+#define UPIO_MEM32BE		(4)			/* 32b big endian */
+#define UPIO_AU			(5)			/* Au1x00 and RT288x type IO */
+#define UPIO_TSI		(6)			/* Tsi108/109 type IO */
 
 	unsigned int		read_status_mask;	/* driver specific */
 	unsigned int		ignore_status_mask;	/* driver specific */
-- 
2.1.0

[PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Kevin Cernekee]

If an earlycon (stdout-path) node is being used, check for "big-endian"
or "native-endian" properties and pass the appropriate iotype to the
driver.

Note that LE sets UPIO_MEM (8-bit) but BE sets UPIO_MEM32BE (32-bit).  The
big-endian property only really makes sense in the context of 32-bit
registers, since 8-bit accesses never require data swapping.

At some point, the of_earlycon code may want to pass in the reg-io-width,
reg-offset, and reg-shift parameters too.

Signed-off-by: Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/of/fdt.c              | 7 ++++++-
 drivers/tty/serial/earlycon.c | 4 ++--
 include/linux/serial_core.h   | 2 +-
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 658656f..9d21472 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -794,6 +794,8 @@ int __init early_init_dt_scan_chosen_serial(void)
 
 	while (match->compatible[0]) {
 		unsigned long addr;
+		unsigned char iotype = UPIO_MEM;
+
 		if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
 			match++;
 			continue;
@@ -803,7 +805,10 @@ int __init early_init_dt_scan_chosen_serial(void)
 		if (!addr)
 			return -ENXIO;
 
-		of_setup_earlycon(addr, match->data);
+		if (of_fdt_is_big_endian(fdt, offset))
+			iotype = UPIO_MEM32BE;
+
+		of_setup_earlycon(addr, iotype, match->data);
 		return 0;
 	}
 	return -ENODEV;
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index a514ee6..548f7d7 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -148,13 +148,13 @@ int __init setup_earlycon(char *buf, const char *match,
 	return 0;
 }
 
-int __init of_setup_earlycon(unsigned long addr,
+int __init of_setup_earlycon(unsigned long addr, unsigned char iotype,
 			     int (*setup)(struct earlycon_device *, const char *))
 {
 	int err;
 	struct uart_port *port = &early_console_dev.port;
 
-	port->iotype = UPIO_MEM;
+	port->iotype = iotype;
 	port->mapbase = addr;
 	port->uartclk = BASE_BAUD * 16;
 	port->membase = earlycon_map(addr, SZ_4K);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index d2d5bf6..0d60c64 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -310,7 +310,7 @@ struct earlycon_device {
 int setup_earlycon(char *buf, const char *match,
 		   int (*setup)(struct earlycon_device *, const char *));
 
-extern int of_setup_earlycon(unsigned long addr,
+extern int of_setup_earlycon(unsigned long addr, unsigned char iotype,
 			     int (*setup)(struct earlycon_device *, const char *));
 
 #define EARLYCON_DECLARE(name, func) \
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Kevin Cernekee]

If an earlycon (stdout-path) node is being used, check for "big-endian"
or "native-endian" properties and pass the appropriate iotype to the
driver.

Note that LE sets UPIO_MEM (8-bit) but BE sets UPIO_MEM32BE (32-bit).  The
big-endian property only really makes sense in the context of 32-bit
registers, since 8-bit accesses never require data swapping.

At some point, the of_earlycon code may want to pass in the reg-io-width,
reg-offset, and reg-shift parameters too.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
---
 drivers/of/fdt.c              | 7 ++++++-
 drivers/tty/serial/earlycon.c | 4 ++--
 include/linux/serial_core.h   | 2 +-
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 658656f..9d21472 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -794,6 +794,8 @@ int __init early_init_dt_scan_chosen_serial(void)
 
 	while (match->compatible[0]) {
 		unsigned long addr;
+		unsigned char iotype = UPIO_MEM;
+
 		if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
 			match++;
 			continue;
@@ -803,7 +805,10 @@ int __init early_init_dt_scan_chosen_serial(void)
 		if (!addr)
 			return -ENXIO;
 
-		of_setup_earlycon(addr, match->data);
+		if (of_fdt_is_big_endian(fdt, offset))
+			iotype = UPIO_MEM32BE;
+
+		of_setup_earlycon(addr, iotype, match->data);
 		return 0;
 	}
 	return -ENODEV;
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index a514ee6..548f7d7 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -148,13 +148,13 @@ int __init setup_earlycon(char *buf, const char *match,
 	return 0;
 }
 
-int __init of_setup_earlycon(unsigned long addr,
+int __init of_setup_earlycon(unsigned long addr, unsigned char iotype,
 			     int (*setup)(struct earlycon_device *, const char *))
 {
 	int err;
 	struct uart_port *port = &early_console_dev.port;
 
-	port->iotype = UPIO_MEM;
+	port->iotype = iotype;
 	port->mapbase = addr;
 	port->uartclk = BASE_BAUD * 16;
 	port->membase = earlycon_map(addr, SZ_4K);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index d2d5bf6..0d60c64 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -310,7 +310,7 @@ struct earlycon_device {
 int setup_earlycon(char *buf, const char *match,
 		   int (*setup)(struct earlycon_device *, const char *));
 
-extern int of_setup_earlycon(unsigned long addr,
+extern int of_setup_earlycon(unsigned long addr, unsigned char iotype,
 			     int (*setup)(struct earlycon_device *, const char *));
 
 #define EARLYCON_DECLARE(name, func) \
-- 
2.1.0

[PATCH V3 6/7] serial: of_serial: Support big-endian register accesses

[Kevin Cernekee]

If the device node has a "big-endian" property and 32-bit registers, tell
the serial driver to use UPIO_MEM32BE instead of UPIO_MEM32.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
---
 drivers/tty/serial/of_serial.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
index 56982da4..efac1dd 100644
--- a/drivers/tty/serial/of_serial.c
+++ b/drivers/tty/serial/of_serial.c
@@ -109,7 +109,8 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
 			port->iotype = UPIO_MEM;
 			break;
 		case 4:
-			port->iotype = UPIO_MEM32;
+			port->iotype = of_device_is_big_endian(np) ?
+				       UPIO_MEM32BE : UPIO_MEM32;
 			break;
 		default:
 			dev_warn(&ofdev->dev, "unsupported reg-io-width (%d)\n",
-- 
2.1.0

[PATCH V3 7/7] serial: 8250: Add support for big-endian MMIO accesses

[Kevin Cernekee]

Add cases for UPIO_MEM32BE wherever there are currently cases handling
UPIO_MEM32.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
---
 drivers/tty/serial/8250/8250_core.c  | 20 ++++++++++++++++++++
 drivers/tty/serial/8250/8250_early.c |  5 +++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index ca5cfdc..53982f0 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -428,6 +428,18 @@ static unsigned int mem32_serial_in(struct uart_port *p, int offset)
 	return readl(p->membase + offset);
 }
 
+static void mem32be_serial_out(struct uart_port *p, int offset, int value)
+{
+	offset = offset << p->regshift;
+	iowrite32be(value, p->membase + offset);
+}
+
+static unsigned int mem32be_serial_in(struct uart_port *p, int offset)
+{
+	offset = offset << p->regshift;
+	return ioread32be(p->membase + offset);
+}
+
 static unsigned int io_serial_in(struct uart_port *p, int offset)
 {
 	offset = offset << p->regshift;
@@ -466,6 +478,11 @@ static void set_io_from_upio(struct uart_port *p)
 		p->serial_out = mem32_serial_out;
 		break;
 
+	case UPIO_MEM32BE:
+		p->serial_in = mem32be_serial_in;
+		p->serial_out = mem32be_serial_out;
+		break;
+
 #if defined(CONFIG_MIPS_ALCHEMY) || defined(CONFIG_SERIAL_8250_RT288X)
 	case UPIO_AU:
 		p->serial_in = au_serial_in;
@@ -491,6 +508,7 @@ serial_port_out_sync(struct uart_port *p, int offset, int value)
 	switch (p->iotype) {
 	case UPIO_MEM:
 	case UPIO_MEM32:
+	case UPIO_MEM32BE:
 	case UPIO_AU:
 		p->serial_out(p, offset, value);
 		p->serial_in(p, UART_LCR);	/* safe, no side-effects */
@@ -2655,6 +2673,7 @@ static int serial8250_request_std_resource(struct uart_8250_port *up)
 	case UPIO_AU:
 	case UPIO_TSI:
 	case UPIO_MEM32:
+	case UPIO_MEM32BE:
 	case UPIO_MEM:
 		if (!port->mapbase)
 			break;
@@ -2691,6 +2710,7 @@ static void serial8250_release_std_resource(struct uart_8250_port *up)
 	case UPIO_AU:
 	case UPIO_TSI:
 	case UPIO_MEM32:
+	case UPIO_MEM32BE:
 	case UPIO_MEM:
 		if (!port->mapbase)
 			break;
diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index 4858b8a..33a0fb6 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -45,6 +45,8 @@ unsigned int __weak __init serial8250_early_in(struct uart_port *port, int offse
 		return readb(port->membase + offset);
 	case UPIO_MEM32:
 		return readl(port->membase + (offset << 2));
+	case UPIO_MEM32BE:
+		return ioread32be(port->membase + (offset << 2));
 	case UPIO_PORT:
 		return inb(port->iobase + offset);
 	default:
@@ -61,6 +63,9 @@ void __weak __init serial8250_early_out(struct uart_port *port, int offset, int
 	case UPIO_MEM32:
 		writel(value, port->membase + (offset << 2));
 		break;
+	case UPIO_MEM32BE:
+		iowrite32be(value, port->membase + (offset << 2));
+		break;
 	case UPIO_PORT:
 		outb(value, port->iobase + offset);
 		break;
-- 
2.1.0

Re: [PATCH V3 0/7] serial: Configure {big,native}-endian MMIO accesses via DT

[Florian Fainelli]

On 11/24/2014 03:36 PM, Kevin Cernekee wrote:
> My last submission attempted to work around serial driver coexistence
> problems on multiplatform kernels.  Since there are still questions
> surrounding the best way to solve that problem, this patch series
> will focus on the narrower topic of big endian MMIO support on serial.

FWIW:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

> 
> 
> V2->V3:
> 
>  - Document the new DT properties.
> 
>  - Add libfdt-based wrapper, to complement the "struct device_node" based
>    version.
> 
>  - Restructure early_init_dt_scan_chosen_serial() changes to use a
>    temporary variable, so it is easy to add more of_setup_earlycon()
>    properties later.
> 
>  - Make of_serial and serial8250 honor the new "big-endian" property.
> 
> 
> This series applies cleanly to:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git devicetree/next-overlay
> 
> but was tested on the mips-for-linux-next branch because my BE platform
> isn't supported in mainline yet.
> 
> 
> Kevin Cernekee (7):
>   of: Add helper function to check MMIO register endianness
>   of/fdt: Add endianness helper function for early init code
>   of: Document {little,big,native}-endian bindings
>   serial: core: Add big-endian iotype
>   serial: earlycon: Set UPIO_MEM32BE based on DT properties
>   serial: of_serial: Support big-endian register accesses
>   serial: 8250: Add support for big-endian MMIO accesses
> 
>  .../devicetree/bindings/common-properties.txt      | 60 ++++++++++++++++++++++
>  drivers/of/base.c                                  | 23 +++++++++
>  drivers/of/fdt.c                                   | 26 +++++++++-
>  drivers/tty/serial/8250/8250_core.c                | 20 ++++++++
>  drivers/tty/serial/8250/8250_early.c               |  5 ++
>  drivers/tty/serial/earlycon.c                      |  4 +-
>  drivers/tty/serial/of_serial.c                     |  3 +-
>  drivers/tty/serial/serial_core.c                   |  2 +
>  include/linux/of.h                                 |  6 +++
>  include/linux/of_fdt.h                             |  2 +
>  include/linux/serial_core.h                        | 15 +++---
>  11 files changed, 155 insertions(+), 11 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/common-properties.txt
> 

Re: [PATCH V3 0/7] serial: Configure {big,native}-endian MMIO accesses via DT

[Arnd Bergmann]

On Tuesday 25 November 2014, Kevin Cernekee wrote:
> 
> My last submission attempted to work around serial driver coexistence
> problems on multiplatform kernels.  Since there are still questions
> surrounding the best way to solve that problem, this patch series
> will focus on the narrower topic of big endian MMIO support on serial.
> 
> 
> V2->V3:
> 
>  - Document the new DT properties.
> 
>  - Add libfdt-based wrapper, to complement the "struct device_node" based
>    version.
> 
>  - Restructure early_init_dt_scan_chosen_serial() changes to use a
>    temporary variable, so it is easy to add more of_setup_earlycon()
>    properties later.
> 
>  - Make of_serial and serial8250 honor the new "big-endian" property.

Looks all good to me,

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH V3 0/7] serial: Configure {big,native}-endian MMIO accesses via DT

[Grant Likely]

On Mon, 24 Nov 2014 15:36:15 -0800
, Kevin Cernekee <cernekee@gmail.com>
 wrote:
> My last submission attempted to work around serial driver coexistence
> problems on multiplatform kernels.  Since there are still questions
> surrounding the best way to solve that problem, this patch series
> will focus on the narrower topic of big endian MMIO support on serial.
> 
> 
> V2->V3:
> 
>  - Document the new DT properties.
> 
>  - Add libfdt-based wrapper, to complement the "struct device_node" based
>    version.
> 
>  - Restructure early_init_dt_scan_chosen_serial() changes to use a
>    temporary variable, so it is easy to add more of_setup_earlycon()
>    properties later.
> 
>  - Make of_serial and serial8250 honor the new "big-endian" property.
> 
> 
> This series applies cleanly to:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git devicetree/next-overlay
> 
> but was tested on the mips-for-linux-next branch because my BE platform
> isn't supported in mainline yet.

For the whole series:
Acked-by: Grant Likely <grant.likely@linaro.org>

Greg, which tree do you want to merge this through? My DT tree, or the
tty tree?

g.

> 
> 
> Kevin Cernekee (7):
>   of: Add helper function to check MMIO register endianness
>   of/fdt: Add endianness helper function for early init code
>   of: Document {little,big,native}-endian bindings
>   serial: core: Add big-endian iotype
>   serial: earlycon: Set UPIO_MEM32BE based on DT properties
>   serial: of_serial: Support big-endian register accesses
>   serial: 8250: Add support for big-endian MMIO accesses
> 
>  .../devicetree/bindings/common-properties.txt      | 60 ++++++++++++++++++++++
>  drivers/of/base.c                                  | 23 +++++++++
>  drivers/of/fdt.c                                   | 26 +++++++++-
>  drivers/tty/serial/8250/8250_core.c                | 20 ++++++++
>  drivers/tty/serial/8250/8250_early.c               |  5 ++
>  drivers/tty/serial/earlycon.c                      |  4 +-
>  drivers/tty/serial/of_serial.c                     |  3 +-
>  drivers/tty/serial/serial_core.c                   |  2 +
>  include/linux/of.h                                 |  6 +++
>  include/linux/of_fdt.h                             |  2 +
>  include/linux/serial_core.h                        | 15 +++---
>  11 files changed, 155 insertions(+), 11 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/common-properties.txt
> 
> -- 
> 2.1.0
> 

Re: [PATCH V3 0/7] serial: Configure {big,native}-endian MMIO accesses via DT

[Greg KH]

On Tue, Nov 25, 2014 at 03:10:18PM +0000, Grant Likely wrote:
> On Mon, 24 Nov 2014 15:36:15 -0800
> , Kevin Cernekee <cernekee@gmail.com>
>  wrote:
> > My last submission attempted to work around serial driver coexistence
> > problems on multiplatform kernels.  Since there are still questions
> > surrounding the best way to solve that problem, this patch series
> > will focus on the narrower topic of big endian MMIO support on serial.
> > 
> > 
> > V2->V3:
> > 
> >  - Document the new DT properties.
> > 
> >  - Add libfdt-based wrapper, to complement the "struct device_node" based
> >    version.
> > 
> >  - Restructure early_init_dt_scan_chosen_serial() changes to use a
> >    temporary variable, so it is easy to add more of_setup_earlycon()
> >    properties later.
> > 
> >  - Make of_serial and serial8250 honor the new "big-endian" property.
> > 
> > 
> > This series applies cleanly to:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git devicetree/next-overlay
> > 
> > but was tested on the mips-for-linux-next branch because my BE platform
> > isn't supported in mainline yet.
> 
> For the whole series:
> Acked-by: Grant Likely <grant.likely@linaro.org>
> 
> Greg, which tree do you want to merge this through? My DT tree, or the
> tty tree?

I can take these through my tty tree, thanks.

greg k-h

Re: [PATCH V3 0/7] serial: Configure {big,native}-endian MMIO accesses via DT

[Greg KH]

On Tue, Nov 25, 2014 at 09:38:59AM -0800, Greg KH wrote:
> On Tue, Nov 25, 2014 at 03:10:18PM +0000, Grant Likely wrote:
> > On Mon, 24 Nov 2014 15:36:15 -0800
> > , Kevin Cernekee <cernekee@gmail.com>
> >  wrote:
> > > My last submission attempted to work around serial driver coexistence
> > > problems on multiplatform kernels.  Since there are still questions
> > > surrounding the best way to solve that problem, this patch series
> > > will focus on the narrower topic of big endian MMIO support on serial.
> > > 
> > > 
> > > V2->V3:
> > > 
> > >  - Document the new DT properties.
> > > 
> > >  - Add libfdt-based wrapper, to complement the "struct device_node" based
> > >    version.
> > > 
> > >  - Restructure early_init_dt_scan_chosen_serial() changes to use a
> > >    temporary variable, so it is easy to add more of_setup_earlycon()
> > >    properties later.
> > > 
> > >  - Make of_serial and serial8250 honor the new "big-endian" property.
> > > 
> > > 
> > > This series applies cleanly to:
> > > 
> > > git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git devicetree/next-overlay
> > > 
> > > but was tested on the mips-for-linux-next branch because my BE platform
> > > isn't supported in mainline yet.
> > 
> > For the whole series:
> > Acked-by: Grant Likely <grant.likely@linaro.org>
> > 
> > Greg, which tree do you want to merge this through? My DT tree, or the
> > tty tree?
> 
> I can take these through my tty tree, thanks.

I take that back, it doesn't apply to my tty tree due to changes in the
of codebase.  So feel free to take all of these through your DT tree
please:

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

Re: [PATCH V3 0/7] serial: Configure {big,native}-endian MMIO accesses via DT

[Grant Likely]

On Tue, Nov 25, 2014 at 9:11 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Nov 25, 2014 at 09:38:59AM -0800, Greg KH wrote:
>> On Tue, Nov 25, 2014 at 03:10:18PM +0000, Grant Likely wrote:
>> > On Mon, 24 Nov 2014 15:36:15 -0800
>> > , Kevin Cernekee <cernekee@gmail.com>
>> >  wrote:
>> > > My last submission attempted to work around serial driver coexistence
>> > > problems on multiplatform kernels.  Since there are still questions
>> > > surrounding the best way to solve that problem, this patch series
>> > > will focus on the narrower topic of big endian MMIO support on serial.
>> > >
>> > >
>> > > V2->V3:
>> > >
>> > >  - Document the new DT properties.
>> > >
>> > >  - Add libfdt-based wrapper, to complement the "struct device_node" based
>> > >    version.
>> > >
>> > >  - Restructure early_init_dt_scan_chosen_serial() changes to use a
>> > >    temporary variable, so it is easy to add more of_setup_earlycon()
>> > >    properties later.
>> > >
>> > >  - Make of_serial and serial8250 honor the new "big-endian" property.
>> > >
>> > >
>> > > This series applies cleanly to:
>> > >
>> > > git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git devicetree/next-overlay
>> > >
>> > > but was tested on the mips-for-linux-next branch because my BE platform
>> > > isn't supported in mainline yet.
>> >
>> > For the whole series:
>> > Acked-by: Grant Likely <grant.likely@linaro.org>
>> >
>> > Greg, which tree do you want to merge this through? My DT tree, or the
>> > tty tree?
>>
>> I can take these through my tty tree, thanks.
>
> I take that back, it doesn't apply to my tty tree due to changes in the
> of codebase.  So feel free to take all of these through your DT tree
> please:
>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Applied all 7 patches. Thanks.

g.

Re: [PATCH V3 0/7] serial: Configure {big,native}-endian MMIO accesses via DT

[Kevin Cernekee]

On Wed, Nov 26, 2014 at 5:14 AM, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Nov 25, 2014 at 9:11 PM, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
>> On Tue, Nov 25, 2014 at 09:38:59AM -0800, Greg KH wrote:
>>> On Tue, Nov 25, 2014 at 03:10:18PM +0000, Grant Likely wrote:
>>> > On Mon, 24 Nov 2014 15:36:15 -0800
>>> > , Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> >  wrote:
>>> > > My last submission attempted to work around serial driver coexistence
>>> > > problems on multiplatform kernels.  Since there are still questions
>>> > > surrounding the best way to solve that problem, this patch series
>>> > > will focus on the narrower topic of big endian MMIO support on serial.
>>> > >
>>> > >
>>> > > V2->V3:
>>> > >
>>> > >  - Document the new DT properties.
>>> > >
>>> > >  - Add libfdt-based wrapper, to complement the "struct device_node" based
>>> > >    version.
>>> > >
>>> > >  - Restructure early_init_dt_scan_chosen_serial() changes to use a
>>> > >    temporary variable, so it is easy to add more of_setup_earlycon()
>>> > >    properties later.
>>> > >
>>> > >  - Make of_serial and serial8250 honor the new "big-endian" property.
>>> > >
>>> > >
>>> > > This series applies cleanly to:
>>> > >
>>> > > git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git devicetree/next-overlay
>>> > >
>>> > > but was tested on the mips-for-linux-next branch because my BE platform
>>> > > isn't supported in mainline yet.
>>> >
>>> > For the whole series:
>>> > Acked-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> >
>>> > Greg, which tree do you want to merge this through? My DT tree, or the
>>> > tty tree?
>>>
>>> I can take these through my tty tree, thanks.
>>
>> I take that back, it doesn't apply to my tty tree due to changes in the
>> of codebase.  So feel free to take all of these through your DT tree
>> please:
>>
>> Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
>
> Applied all 7 patches. Thanks.

Hi guys,

I don't see these patches in devicetree-next or mainline.  Would you
like me to rebase + resend?

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 0/7] serial: Configure {big,native}-endian MMIO accesses via DT

[Kevin Cernekee]

On Wed, Nov 26, 2014 at 5:14 AM, Grant Likely <grant.likely@linaro.org> wrote:
> On Tue, Nov 25, 2014 at 9:11 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> On Tue, Nov 25, 2014 at 09:38:59AM -0800, Greg KH wrote:
>>> On Tue, Nov 25, 2014 at 03:10:18PM +0000, Grant Likely wrote:
>>> > On Mon, 24 Nov 2014 15:36:15 -0800
>>> > , Kevin Cernekee <cernekee@gmail.com>
>>> >  wrote:
>>> > > My last submission attempted to work around serial driver coexistence
>>> > > problems on multiplatform kernels.  Since there are still questions
>>> > > surrounding the best way to solve that problem, this patch series
>>> > > will focus on the narrower topic of big endian MMIO support on serial.
>>> > >
>>> > >
>>> > > V2->V3:
>>> > >
>>> > >  - Document the new DT properties.
>>> > >
>>> > >  - Add libfdt-based wrapper, to complement the "struct device_node" based
>>> > >    version.
>>> > >
>>> > >  - Restructure early_init_dt_scan_chosen_serial() changes to use a
>>> > >    temporary variable, so it is easy to add more of_setup_earlycon()
>>> > >    properties later.
>>> > >
>>> > >  - Make of_serial and serial8250 honor the new "big-endian" property.
>>> > >
>>> > >
>>> > > This series applies cleanly to:
>>> > >
>>> > > git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git devicetree/next-overlay
>>> > >
>>> > > but was tested on the mips-for-linux-next branch because my BE platform
>>> > > isn't supported in mainline yet.
>>> >
>>> > For the whole series:
>>> > Acked-by: Grant Likely <grant.likely@linaro.org>
>>> >
>>> > Greg, which tree do you want to merge this through? My DT tree, or the
>>> > tty tree?
>>>
>>> I can take these through my tty tree, thanks.
>>
>> I take that back, it doesn't apply to my tty tree due to changes in the
>> of codebase.  So feel free to take all of these through your DT tree
>> please:
>>
>> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> Applied all 7 patches. Thanks.

Hi guys,

I don't see these patches in devicetree-next or mainline.  Would you
like me to rebase + resend?

Thanks.

Re: [PATCH V3 0/7] serial: Configure {big,native}-endian MMIO accesses via DT

[Rob Herring]

On Sat, Feb 21, 2015 at 2:53 PM, Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Nov 26, 2014 at 5:14 AM, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On Tue, Nov 25, 2014 at 9:11 PM, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
>>> On Tue, Nov 25, 2014 at 09:38:59AM -0800, Greg KH wrote:
>>>> On Tue, Nov 25, 2014 at 03:10:18PM +0000, Grant Likely wrote:
>>>> > On Mon, 24 Nov 2014 15:36:15 -0800
>>>> > , Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> >  wrote:
>>>> > > My last submission attempted to work around serial driver coexistence
>>>> > > problems on multiplatform kernels.  Since there are still questions
>>>> > > surrounding the best way to solve that problem, this patch series
>>>> > > will focus on the narrower topic of big endian MMIO support on serial.
>>>> > >
>>>> > >
>>>> > > V2->V3:
>>>> > >
>>>> > >  - Document the new DT properties.
>>>> > >
>>>> > >  - Add libfdt-based wrapper, to complement the "struct device_node" based
>>>> > >    version.
>>>> > >
>>>> > >  - Restructure early_init_dt_scan_chosen_serial() changes to use a
>>>> > >    temporary variable, so it is easy to add more of_setup_earlycon()
>>>> > >    properties later.
>>>> > >
>>>> > >  - Make of_serial and serial8250 honor the new "big-endian" property.
>>>> > >
>>>> > >
>>>> > > This series applies cleanly to:
>>>> > >
>>>> > > git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git devicetree/next-overlay
>>>> > >
>>>> > > but was tested on the mips-for-linux-next branch because my BE platform
>>>> > > isn't supported in mainline yet.
>>>> >
>>>> > For the whole series:
>>>> > Acked-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>> >
>>>> > Greg, which tree do you want to merge this through? My DT tree, or the
>>>> > tty tree?
>>>>
>>>> I can take these through my tty tree, thanks.
>>>
>>> I take that back, it doesn't apply to my tty tree due to changes in the
>>> of codebase.  So feel free to take all of these through your DT tree
>>> please:
>>>
>>> Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
>>
>> Applied all 7 patches. Thanks.
>
> Hi guys,
>
> I don't see these patches in devicetree-next or mainline.  Would you
> like me to rebase + resend?

Not sure what happened there, but since they conflicted for Greg, it's
probably best to rebase to 3.20-rc and resend them.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 0/7] serial: Configure {big,native}-endian MMIO accesses via DT

[Rob Herring]

On Sat, Feb 21, 2015 at 2:53 PM, Kevin Cernekee <cernekee@gmail.com> wrote:
> On Wed, Nov 26, 2014 at 5:14 AM, Grant Likely <grant.likely@linaro.org> wrote:
>> On Tue, Nov 25, 2014 at 9:11 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>>> On Tue, Nov 25, 2014 at 09:38:59AM -0800, Greg KH wrote:
>>>> On Tue, Nov 25, 2014 at 03:10:18PM +0000, Grant Likely wrote:
>>>> > On Mon, 24 Nov 2014 15:36:15 -0800
>>>> > , Kevin Cernekee <cernekee@gmail.com>
>>>> >  wrote:
>>>> > > My last submission attempted to work around serial driver coexistence
>>>> > > problems on multiplatform kernels.  Since there are still questions
>>>> > > surrounding the best way to solve that problem, this patch series
>>>> > > will focus on the narrower topic of big endian MMIO support on serial.
>>>> > >
>>>> > >
>>>> > > V2->V3:
>>>> > >
>>>> > >  - Document the new DT properties.
>>>> > >
>>>> > >  - Add libfdt-based wrapper, to complement the "struct device_node" based
>>>> > >    version.
>>>> > >
>>>> > >  - Restructure early_init_dt_scan_chosen_serial() changes to use a
>>>> > >    temporary variable, so it is easy to add more of_setup_earlycon()
>>>> > >    properties later.
>>>> > >
>>>> > >  - Make of_serial and serial8250 honor the new "big-endian" property.
>>>> > >
>>>> > >
>>>> > > This series applies cleanly to:
>>>> > >
>>>> > > git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git devicetree/next-overlay
>>>> > >
>>>> > > but was tested on the mips-for-linux-next branch because my BE platform
>>>> > > isn't supported in mainline yet.
>>>> >
>>>> > For the whole series:
>>>> > Acked-by: Grant Likely <grant.likely@linaro.org>
>>>> >
>>>> > Greg, which tree do you want to merge this through? My DT tree, or the
>>>> > tty tree?
>>>>
>>>> I can take these through my tty tree, thanks.
>>>
>>> I take that back, it doesn't apply to my tty tree due to changes in the
>>> of codebase.  So feel free to take all of these through your DT tree
>>> please:
>>>
>>> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>
>> Applied all 7 patches. Thanks.
>
> Hi guys,
>
> I don't see these patches in devicetree-next or mainline.  Would you
> like me to rebase + resend?

Not sure what happened there, but since they conflicted for Greg, it's
probably best to rebase to 3.20-rc and resend them.

Rob

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

Hi Kevin,

On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
> If an earlycon (stdout-path) node is being used, check for "big-endian"
> or "native-endian" properties and pass the appropriate iotype to the
> driver.
> 
> Note that LE sets UPIO_MEM (8-bit) but BE sets UPIO_MEM32BE (32-bit).  The
> big-endian property only really makes sense in the context of 32-bit
> registers, since 8-bit accesses never require data swapping.
> 
> At some point, the of_earlycon code may want to pass in the reg-io-width,
> reg-offset, and reg-shift parameters too.
> 
> Signed-off-by: Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/of/fdt.c              | 7 ++++++-
>  drivers/tty/serial/earlycon.c | 4 ++--
>  include/linux/serial_core.h   | 2 +-
>  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 658656f..9d21472 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -794,6 +794,8 @@ int __init early_init_dt_scan_chosen_serial(void)
>  
>  	while (match->compatible[0]) {
>  		unsigned long addr;
> +		unsigned char iotype = UPIO_MEM;
> +
>  		if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
>  			match++;
>  			continue;
> @@ -803,7 +805,10 @@ int __init early_init_dt_scan_chosen_serial(void)
>  		if (!addr)
>  			return -ENXIO;
>  
> -		of_setup_earlycon(addr, match->data);
> +		if (of_fdt_is_big_endian(fdt, offset))
> +			iotype = UPIO_MEM32BE;
> +
> +		of_setup_earlycon(addr, iotype, match->data);

I know these got ACKs already but as you point out in the commit log,
earlycon _will_ need reg-io-width, reg-offset and reg-shift. Since the
distinction between early_init_dt_scan_chosen_serial() and
of_setup_earlycon() is arbitrary, I'd rather see of_setup_earlycon()
taught to properly decode of_serial driver bindings instead of a
stack of parameters to of_setup_earlycon().

In fact, this patch allows a mis-defined devicetree to bring up a
functioning earlycon because the 'big-endian' property is directly
associated with UPIO_MEM32BE, which will create incompatibility problems
when DT earlycon is fixed to decode the of_serial DT bindings.

[rant]
In general, the ability to query devicetree from all over the
kernel creates all kinds of compatibility issues which eventually
will cause unresolvable breakage.

The same rigor applied to ioctls is the analysis required for how
DT bindings are used in the kernel.

I realize that since this particular case only applies to earlycon, it's
no big deal, but if this same mistake had been made in the of_serial
driver, the serial core would be permanently stuck with the
'big-endian' property == UPIO_MEM32BE which could impact future designs.
[/rant]

Regards,
Peter Hurley

>  		return 0;
>  	}
>  	return -ENODEV;
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index a514ee6..548f7d7 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -148,13 +148,13 @@ int __init setup_earlycon(char *buf, const char *match,
>  	return 0;
>  }
>  
> -int __init of_setup_earlycon(unsigned long addr,
> +int __init of_setup_earlycon(unsigned long addr, unsigned char iotype,
>  			     int (*setup)(struct earlycon_device *, const char *))
>  {
>  	int err;
>  	struct uart_port *port = &early_console_dev.port;
>  
> -	port->iotype = UPIO_MEM;
> +	port->iotype = iotype;
>  	port->mapbase = addr;
>  	port->uartclk = BASE_BAUD * 16;
>  	port->membase = earlycon_map(addr, SZ_4K);
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index d2d5bf6..0d60c64 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -310,7 +310,7 @@ struct earlycon_device {
>  int setup_earlycon(char *buf, const char *match,
>  		   int (*setup)(struct earlycon_device *, const char *));
>  
> -extern int of_setup_earlycon(unsigned long addr,
> +extern int of_setup_earlycon(unsigned long addr, unsigned char iotype,
>  			     int (*setup)(struct earlycon_device *, const char *));
>  
>  #define EARLYCON_DECLARE(name, func) \
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

Hi Kevin,

On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
> If an earlycon (stdout-path) node is being used, check for "big-endian"
> or "native-endian" properties and pass the appropriate iotype to the
> driver.
> 
> Note that LE sets UPIO_MEM (8-bit) but BE sets UPIO_MEM32BE (32-bit).  The
> big-endian property only really makes sense in the context of 32-bit
> registers, since 8-bit accesses never require data swapping.
> 
> At some point, the of_earlycon code may want to pass in the reg-io-width,
> reg-offset, and reg-shift parameters too.
> 
> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
> ---
>  drivers/of/fdt.c              | 7 ++++++-
>  drivers/tty/serial/earlycon.c | 4 ++--
>  include/linux/serial_core.h   | 2 +-
>  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 658656f..9d21472 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -794,6 +794,8 @@ int __init early_init_dt_scan_chosen_serial(void)
>  
>  	while (match->compatible[0]) {
>  		unsigned long addr;
> +		unsigned char iotype = UPIO_MEM;
> +
>  		if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
>  			match++;
>  			continue;
> @@ -803,7 +805,10 @@ int __init early_init_dt_scan_chosen_serial(void)
>  		if (!addr)
>  			return -ENXIO;
>  
> -		of_setup_earlycon(addr, match->data);
> +		if (of_fdt_is_big_endian(fdt, offset))
> +			iotype = UPIO_MEM32BE;
> +
> +		of_setup_earlycon(addr, iotype, match->data);

I know these got ACKs already but as you point out in the commit log,
earlycon _will_ need reg-io-width, reg-offset and reg-shift. Since the
distinction between early_init_dt_scan_chosen_serial() and
of_setup_earlycon() is arbitrary, I'd rather see of_setup_earlycon()
taught to properly decode of_serial driver bindings instead of a
stack of parameters to of_setup_earlycon().

In fact, this patch allows a mis-defined devicetree to bring up a
functioning earlycon because the 'big-endian' property is directly
associated with UPIO_MEM32BE, which will create incompatibility problems
when DT earlycon is fixed to decode the of_serial DT bindings.

[rant]
In general, the ability to query devicetree from all over the
kernel creates all kinds of compatibility issues which eventually
will cause unresolvable breakage.

The same rigor applied to ioctls is the analysis required for how
DT bindings are used in the kernel.

I realize that since this particular case only applies to earlycon, it's
no big deal, but if this same mistake had been made in the of_serial
driver, the serial core would be permanently stuck with the
'big-endian' property == UPIO_MEM32BE which could impact future designs.
[/rant]

Regards,
Peter Hurley

>  		return 0;
>  	}
>  	return -ENODEV;
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index a514ee6..548f7d7 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -148,13 +148,13 @@ int __init setup_earlycon(char *buf, const char *match,
>  	return 0;
>  }
>  
> -int __init of_setup_earlycon(unsigned long addr,
> +int __init of_setup_earlycon(unsigned long addr, unsigned char iotype,
>  			     int (*setup)(struct earlycon_device *, const char *))
>  {
>  	int err;
>  	struct uart_port *port = &early_console_dev.port;
>  
> -	port->iotype = UPIO_MEM;
> +	port->iotype = iotype;
>  	port->mapbase = addr;
>  	port->uartclk = BASE_BAUD * 16;
>  	port->membase = earlycon_map(addr, SZ_4K);
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index d2d5bf6..0d60c64 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -310,7 +310,7 @@ struct earlycon_device {
>  int setup_earlycon(char *buf, const char *match,
>  		   int (*setup)(struct earlycon_device *, const char *));
>  
> -extern int of_setup_earlycon(unsigned long addr,
> +extern int of_setup_earlycon(unsigned long addr, unsigned char iotype,
>  			     int (*setup)(struct earlycon_device *, const char *));
>  
>  #define EARLYCON_DECLARE(name, func) \
> 

Re: [PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Peter Hurley]

On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
> These apply to newly converted drivers, like serial8250/libahci/...
> The examples were adapted from the regmap bindings document.
> 
> Signed-off-by: Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/common-properties.txt      | 60 ++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/common-properties.txt
> 
> diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
> new file mode 100644
> index 0000000..21044a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/common-properties.txt
> @@ -0,0 +1,60 @@
> +Common properties
> +
> +The ePAPR specification does not define any properties related to hardware
> +byteswapping, but endianness issues show up frequently in porting Linux to
> +different machine types.  This document attempts to provide a consistent
> +way of handling byteswapping across drivers.
> +
> +Optional properties:
> + - big-endian: Boolean; force big endian register accesses
> +   unconditionally (e.g. ioread32be/iowrite32be).  Use this if you
> +   know the peripheral always needs to be accessed in BE mode.
> + - little-endian: Boolean; force little endian register accesses
> +   unconditionally (e.g. readl/writel).  Use this if you know the
> +   peripheral always needs to be accessed in LE mode.  This is the
> +   default.

There is a fundamental problem with specifying the default in DT bindings.
How can drivers which are currently native-endian support big-endian?

If the driver is converted to support big-endian, every previous
devicetree will be invalid with the new kernel (because those devicetrees
don't specify 'native-endian').

IOW, consider if the default were 'native-endian'. How would the 8250
driver support existing devicetrees?

Regards,
Peter Hurley


> + - native-endian: Boolean; always use register accesses matched to the
> +   endianness of the kernel binary (e.g. LE vmlinux -> readl/writel,
> +   BE vmlinux -> ioread32be/iowrite32be).  In this case no byteswaps
> +   will ever be performed.  Use this if the hardware "self-adjusts"
> +   register endianness based on the CPU's configured endianness.
> +
> +Note that regmap, in contrast, defaults to native-endian.  But this
> +document is targeted for existing drivers, most of which currently use
> +readl/writel because they expect to be accessing PCI/PCIe devices rather
> +than memory-mapped SoC peripherals.  Since the readl/writel accessors
> +perform a byteswap on BE systems, this means that the drivers in question
> +are implicitly "little-endian".
> +
> +Examples:
> +Scenario 1 : CPU in LE mode & device in LE mode.
> +dev: dev@40031000 {
> +	      compatible = "name";
> +	      reg = <0x40031000 0x1000>;
> +	      ...
> +	      native-endian;
> +};
> +
> +Scenario 2 : CPU in LE mode & device in BE mode.
> +dev: dev@40031000 {
> +	      compatible = "name";
> +	      reg = <0x40031000 0x1000>;
> +	      ...
> +	      big-endian;
> +};
> +
> +Scenario 3 : CPU in BE mode & device in BE mode.
> +dev: dev@40031000 {
> +	      compatible = "name";
> +	      reg = <0x40031000 0x1000>;
> +	      ...
> +	      native-endian;
> +};
> +
> +Scenario 4 : CPU in BE mode & device in LE mode.
> +dev: dev@40031000 {
> +	      compatible = "name";
> +	      reg = <0x40031000 0x1000>;
> +	      ...
> +	      little-endian;
> +};
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Peter Hurley]

On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
> These apply to newly converted drivers, like serial8250/libahci/...
> The examples were adapted from the regmap bindings document.
> 
> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
> ---
>  .../devicetree/bindings/common-properties.txt      | 60 ++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/common-properties.txt
> 
> diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
> new file mode 100644
> index 0000000..21044a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/common-properties.txt
> @@ -0,0 +1,60 @@
> +Common properties
> +
> +The ePAPR specification does not define any properties related to hardware
> +byteswapping, but endianness issues show up frequently in porting Linux to
> +different machine types.  This document attempts to provide a consistent
> +way of handling byteswapping across drivers.
> +
> +Optional properties:
> + - big-endian: Boolean; force big endian register accesses
> +   unconditionally (e.g. ioread32be/iowrite32be).  Use this if you
> +   know the peripheral always needs to be accessed in BE mode.
> + - little-endian: Boolean; force little endian register accesses
> +   unconditionally (e.g. readl/writel).  Use this if you know the
> +   peripheral always needs to be accessed in LE mode.  This is the
> +   default.

There is a fundamental problem with specifying the default in DT bindings.
How can drivers which are currently native-endian support big-endian?

If the driver is converted to support big-endian, every previous
devicetree will be invalid with the new kernel (because those devicetrees
don't specify 'native-endian').

IOW, consider if the default were 'native-endian'. How would the 8250
driver support existing devicetrees?

Regards,
Peter Hurley


> + - native-endian: Boolean; always use register accesses matched to the
> +   endianness of the kernel binary (e.g. LE vmlinux -> readl/writel,
> +   BE vmlinux -> ioread32be/iowrite32be).  In this case no byteswaps
> +   will ever be performed.  Use this if the hardware "self-adjusts"
> +   register endianness based on the CPU's configured endianness.
> +
> +Note that regmap, in contrast, defaults to native-endian.  But this
> +document is targeted for existing drivers, most of which currently use
> +readl/writel because they expect to be accessing PCI/PCIe devices rather
> +than memory-mapped SoC peripherals.  Since the readl/writel accessors
> +perform a byteswap on BE systems, this means that the drivers in question
> +are implicitly "little-endian".
> +
> +Examples:
> +Scenario 1 : CPU in LE mode & device in LE mode.
> +dev: dev@40031000 {
> +	      compatible = "name";
> +	      reg = <0x40031000 0x1000>;
> +	      ...
> +	      native-endian;
> +};
> +
> +Scenario 2 : CPU in LE mode & device in BE mode.
> +dev: dev@40031000 {
> +	      compatible = "name";
> +	      reg = <0x40031000 0x1000>;
> +	      ...
> +	      big-endian;
> +};
> +
> +Scenario 3 : CPU in BE mode & device in BE mode.
> +dev: dev@40031000 {
> +	      compatible = "name";
> +	      reg = <0x40031000 0x1000>;
> +	      ...
> +	      native-endian;
> +};
> +
> +Scenario 4 : CPU in BE mode & device in LE mode.
> +dev: dev@40031000 {
> +	      compatible = "name";
> +	      reg = <0x40031000 0x1000>;
> +	      ...
> +	      little-endian;
> +};
> 

Re: [PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Kevin Cernekee]

On Mon, Mar 2, 2015 at 5:14 AM, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org> wrote:
> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
>> These apply to newly converted drivers, like serial8250/libahci/...
>> The examples were adapted from the regmap bindings document.
>>
>> Signed-off-by: Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  .../devicetree/bindings/common-properties.txt      | 60 ++++++++++++++++++++++
>>  1 file changed, 60 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/common-properties.txt
>>
>> diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
>> new file mode 100644
>> index 0000000..21044a4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/common-properties.txt
>> @@ -0,0 +1,60 @@
>> +Common properties
>> +
>> +The ePAPR specification does not define any properties related to hardware
>> +byteswapping, but endianness issues show up frequently in porting Linux to
>> +different machine types.  This document attempts to provide a consistent
>> +way of handling byteswapping across drivers.
>> +
>> +Optional properties:
>> + - big-endian: Boolean; force big endian register accesses
>> +   unconditionally (e.g. ioread32be/iowrite32be).  Use this if you
>> +   know the peripheral always needs to be accessed in BE mode.
>> + - little-endian: Boolean; force little endian register accesses
>> +   unconditionally (e.g. readl/writel).  Use this if you know the
>> +   peripheral always needs to be accessed in LE mode.  This is the
>> +   default.
>
> There is a fundamental problem with specifying the default in DT bindings.
> How can drivers which are currently native-endian support big-endian?
>
> If the driver is converted to support big-endian, every previous
> devicetree will be invalid with the new kernel (because those devicetrees
> don't specify 'native-endian').
>
> IOW, consider if the default were 'native-endian'. How would the 8250
> driver support existing devicetrees?

Correct.  This scheme is intended for drivers like 8250 and libahci
which currently default to little-endian by virtue of using
readl/writel for MMIO accesses.  Drivers that default to native-endian
should specify that in their bindings documents, similar to
Documentation/devicetree/bindings/regmap/regmap.txt.

In practice we might not see too many cases of native-endian drivers
that need to be converted to work in forced big-endian mode anyway,
because most uses of the __raw_* accessors are found in SoC-specific
code.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Kevin Cernekee]

On Mon, Mar 2, 2015 at 5:14 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
>> These apply to newly converted drivers, like serial8250/libahci/...
>> The examples were adapted from the regmap bindings document.
>>
>> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
>> ---
>>  .../devicetree/bindings/common-properties.txt      | 60 ++++++++++++++++++++++
>>  1 file changed, 60 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/common-properties.txt
>>
>> diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
>> new file mode 100644
>> index 0000000..21044a4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/common-properties.txt
>> @@ -0,0 +1,60 @@
>> +Common properties
>> +
>> +The ePAPR specification does not define any properties related to hardware
>> +byteswapping, but endianness issues show up frequently in porting Linux to
>> +different machine types.  This document attempts to provide a consistent
>> +way of handling byteswapping across drivers.
>> +
>> +Optional properties:
>> + - big-endian: Boolean; force big endian register accesses
>> +   unconditionally (e.g. ioread32be/iowrite32be).  Use this if you
>> +   know the peripheral always needs to be accessed in BE mode.
>> + - little-endian: Boolean; force little endian register accesses
>> +   unconditionally (e.g. readl/writel).  Use this if you know the
>> +   peripheral always needs to be accessed in LE mode.  This is the
>> +   default.
>
> There is a fundamental problem with specifying the default in DT bindings.
> How can drivers which are currently native-endian support big-endian?
>
> If the driver is converted to support big-endian, every previous
> devicetree will be invalid with the new kernel (because those devicetrees
> don't specify 'native-endian').
>
> IOW, consider if the default were 'native-endian'. How would the 8250
> driver support existing devicetrees?

Correct.  This scheme is intended for drivers like 8250 and libahci
which currently default to little-endian by virtue of using
readl/writel for MMIO accesses.  Drivers that default to native-endian
should specify that in their bindings documents, similar to
Documentation/devicetree/bindings/regmap/regmap.txt.

In practice we might not see too many cases of native-endian drivers
that need to be converted to work in forced big-endian mode anyway,
because most uses of the __raw_* accessors are found in SoC-specific
code.

Re: [PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Peter Hurley]

On 03/02/2015 09:56 AM, Kevin Cernekee wrote:
> On Mon, Mar 2, 2015 at 5:14 AM, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org> wrote:
>> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
>>> These apply to newly converted drivers, like serial8250/libahci/...
>>> The examples were adapted from the regmap bindings document.
>>>
>>> Signed-off-by: Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>>  .../devicetree/bindings/common-properties.txt      | 60 ++++++++++++++++++++++
>>>  1 file changed, 60 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/common-properties.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
>>> new file mode 100644
>>> index 0000000..21044a4
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/common-properties.txt
>>> @@ -0,0 +1,60 @@
>>> +Common properties
>>> +
>>> +The ePAPR specification does not define any properties related to hardware
>>> +byteswapping, but endianness issues show up frequently in porting Linux to
>>> +different machine types.  This document attempts to provide a consistent
>>> +way of handling byteswapping across drivers.
>>> +
>>> +Optional properties:
>>> + - big-endian: Boolean; force big endian register accesses
>>> +   unconditionally (e.g. ioread32be/iowrite32be).  Use this if you
>>> +   know the peripheral always needs to be accessed in BE mode.
>>> + - little-endian: Boolean; force little endian register accesses
>>> +   unconditionally (e.g. readl/writel).  Use this if you know the
>>> +   peripheral always needs to be accessed in LE mode.  This is the
>>> +   default.
>>
>> There is a fundamental problem with specifying the default in DT bindings.
>> How can drivers which are currently native-endian support big-endian?
>>
>> If the driver is converted to support big-endian, every previous
>> devicetree will be invalid with the new kernel (because those devicetrees
>> don't specify 'native-endian').
>>
>> IOW, consider if the default were 'native-endian'. How would the 8250
>> driver support existing devicetrees?
> 
> Correct.  This scheme is intended for drivers like 8250 and libahci
> which currently default to little-endian by virtue of using
> readl/writel for MMIO accesses.  Drivers that default to native-endian
> should specify that in their bindings documents, similar to
> Documentation/devicetree/bindings/regmap/regmap.txt.

Which effectively means if a user can't upgrade their devicetree, they
can't upgrade their kernel. I don't think that flies.

It's exactly this kind of stuff that prompted Jonathan Corbet's article,
"Device trees as ABI"  http://lwn.net/Articles/561462

Why not leave the default unspecified?

Regards,
Peter Hurley

> In practice we might not see too many cases of native-endian drivers
> that need to be converted to work in forced big-endian mode anyway,
> because most uses of the __raw_* accessors are found in SoC-specific
> code.


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Peter Hurley]

On 03/02/2015 09:56 AM, Kevin Cernekee wrote:
> On Mon, Mar 2, 2015 at 5:14 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
>>> These apply to newly converted drivers, like serial8250/libahci/...
>>> The examples were adapted from the regmap bindings document.
>>>
>>> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
>>> ---
>>>  .../devicetree/bindings/common-properties.txt      | 60 ++++++++++++++++++++++
>>>  1 file changed, 60 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/common-properties.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
>>> new file mode 100644
>>> index 0000000..21044a4
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/common-properties.txt
>>> @@ -0,0 +1,60 @@
>>> +Common properties
>>> +
>>> +The ePAPR specification does not define any properties related to hardware
>>> +byteswapping, but endianness issues show up frequently in porting Linux to
>>> +different machine types.  This document attempts to provide a consistent
>>> +way of handling byteswapping across drivers.
>>> +
>>> +Optional properties:
>>> + - big-endian: Boolean; force big endian register accesses
>>> +   unconditionally (e.g. ioread32be/iowrite32be).  Use this if you
>>> +   know the peripheral always needs to be accessed in BE mode.
>>> + - little-endian: Boolean; force little endian register accesses
>>> +   unconditionally (e.g. readl/writel).  Use this if you know the
>>> +   peripheral always needs to be accessed in LE mode.  This is the
>>> +   default.
>>
>> There is a fundamental problem with specifying the default in DT bindings.
>> How can drivers which are currently native-endian support big-endian?
>>
>> If the driver is converted to support big-endian, every previous
>> devicetree will be invalid with the new kernel (because those devicetrees
>> don't specify 'native-endian').
>>
>> IOW, consider if the default were 'native-endian'. How would the 8250
>> driver support existing devicetrees?
> 
> Correct.  This scheme is intended for drivers like 8250 and libahci
> which currently default to little-endian by virtue of using
> readl/writel for MMIO accesses.  Drivers that default to native-endian
> should specify that in their bindings documents, similar to
> Documentation/devicetree/bindings/regmap/regmap.txt.

Which effectively means if a user can't upgrade their devicetree, they
can't upgrade their kernel. I don't think that flies.

It's exactly this kind of stuff that prompted Jonathan Corbet's article,
"Device trees as ABI"  http://lwn.net/Articles/561462

Why not leave the default unspecified?

Regards,
Peter Hurley

> In practice we might not see too many cases of native-endian drivers
> that need to be converted to work in forced big-endian mode anyway,
> because most uses of the __raw_* accessors are found in SoC-specific
> code.

Re: [PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Kevin Cernekee]

On Mon, Mar 2, 2015 at 8:08 AM, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org> wrote:
> On 03/02/2015 09:56 AM, Kevin Cernekee wrote:
>> On Mon, Mar 2, 2015 at 5:14 AM, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org> wrote:
>>> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
>>>> These apply to newly converted drivers, like serial8250/libahci/...
>>>> The examples were adapted from the regmap bindings document.
>>>>
>>>> Signed-off-by: Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>>  .../devicetree/bindings/common-properties.txt      | 60 ++++++++++++++++++++++
>>>>  1 file changed, 60 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/common-properties.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
>>>> new file mode 100644
>>>> index 0000000..21044a4
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/common-properties.txt
>>>> @@ -0,0 +1,60 @@
>>>> +Common properties
>>>> +
>>>> +The ePAPR specification does not define any properties related to hardware
>>>> +byteswapping, but endianness issues show up frequently in porting Linux to
>>>> +different machine types.  This document attempts to provide a consistent
>>>> +way of handling byteswapping across drivers.
>>>> +
>>>> +Optional properties:
>>>> + - big-endian: Boolean; force big endian register accesses
>>>> +   unconditionally (e.g. ioread32be/iowrite32be).  Use this if you
>>>> +   know the peripheral always needs to be accessed in BE mode.
>>>> + - little-endian: Boolean; force little endian register accesses
>>>> +   unconditionally (e.g. readl/writel).  Use this if you know the
>>>> +   peripheral always needs to be accessed in LE mode.  This is the
>>>> +   default.
>>>
>>> There is a fundamental problem with specifying the default in DT bindings.
>>> How can drivers which are currently native-endian support big-endian?
>>>
>>> If the driver is converted to support big-endian, every previous
>>> devicetree will be invalid with the new kernel (because those devicetrees
>>> don't specify 'native-endian').
>>>
>>> IOW, consider if the default were 'native-endian'. How would the 8250
>>> driver support existing devicetrees?
>>
>> Correct.  This scheme is intended for drivers like 8250 and libahci
>> which currently default to little-endian by virtue of using
>> readl/writel for MMIO accesses.  Drivers that default to native-endian
>> should specify that in their bindings documents, similar to
>> Documentation/devicetree/bindings/regmap/regmap.txt.
>
> Which effectively means if a user can't upgrade their devicetree, they
> can't upgrade their kernel. I don't think that flies.

This doesn't change the behavior of pre-existing drivers that
implement the *-endian properties in a different way.  There are not
many of these drivers and they can be documented as special cases.

> It's exactly this kind of stuff that prompted Jonathan Corbet's article,
> "Device trees as ABI"  http://lwn.net/Articles/561462
>
> Why not leave the default unspecified?

The document aims to provide a consistent way of handling DT
endianness properties across (compliant) drivers.  It is confusing if
one new driver defaults to little-endian, and another new driver
defaults to native-endian.

And since most of the commonly used drivers already implement
little-endian MMIO accesses, that is the default.  My personal
preference would have been native-endian since that seems more common
on the hardware side, but defaulting to little-endian prevents
breaking the device tree "ABI" on existing systems.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Kevin Cernekee]

On Mon, Mar 2, 2015 at 8:08 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On 03/02/2015 09:56 AM, Kevin Cernekee wrote:
>> On Mon, Mar 2, 2015 at 5:14 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
>>>> These apply to newly converted drivers, like serial8250/libahci/...
>>>> The examples were adapted from the regmap bindings document.
>>>>
>>>> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
>>>> ---
>>>>  .../devicetree/bindings/common-properties.txt      | 60 ++++++++++++++++++++++
>>>>  1 file changed, 60 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/common-properties.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
>>>> new file mode 100644
>>>> index 0000000..21044a4
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/common-properties.txt
>>>> @@ -0,0 +1,60 @@
>>>> +Common properties
>>>> +
>>>> +The ePAPR specification does not define any properties related to hardware
>>>> +byteswapping, but endianness issues show up frequently in porting Linux to
>>>> +different machine types.  This document attempts to provide a consistent
>>>> +way of handling byteswapping across drivers.
>>>> +
>>>> +Optional properties:
>>>> + - big-endian: Boolean; force big endian register accesses
>>>> +   unconditionally (e.g. ioread32be/iowrite32be).  Use this if you
>>>> +   know the peripheral always needs to be accessed in BE mode.
>>>> + - little-endian: Boolean; force little endian register accesses
>>>> +   unconditionally (e.g. readl/writel).  Use this if you know the
>>>> +   peripheral always needs to be accessed in LE mode.  This is the
>>>> +   default.
>>>
>>> There is a fundamental problem with specifying the default in DT bindings.
>>> How can drivers which are currently native-endian support big-endian?
>>>
>>> If the driver is converted to support big-endian, every previous
>>> devicetree will be invalid with the new kernel (because those devicetrees
>>> don't specify 'native-endian').
>>>
>>> IOW, consider if the default were 'native-endian'. How would the 8250
>>> driver support existing devicetrees?
>>
>> Correct.  This scheme is intended for drivers like 8250 and libahci
>> which currently default to little-endian by virtue of using
>> readl/writel for MMIO accesses.  Drivers that default to native-endian
>> should specify that in their bindings documents, similar to
>> Documentation/devicetree/bindings/regmap/regmap.txt.
>
> Which effectively means if a user can't upgrade their devicetree, they
> can't upgrade their kernel. I don't think that flies.

This doesn't change the behavior of pre-existing drivers that
implement the *-endian properties in a different way.  There are not
many of these drivers and they can be documented as special cases.

> It's exactly this kind of stuff that prompted Jonathan Corbet's article,
> "Device trees as ABI"  http://lwn.net/Articles/561462
>
> Why not leave the default unspecified?

The document aims to provide a consistent way of handling DT
endianness properties across (compliant) drivers.  It is confusing if
one new driver defaults to little-endian, and another new driver
defaults to native-endian.

And since most of the commonly used drivers already implement
little-endian MMIO accesses, that is the default.  My personal
preference would have been native-endian since that seems more common
on the hardware side, but defaulting to little-endian prevents
breaking the device tree "ABI" on existing systems.

Re: [PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Peter Hurley]

On 03/02/2015 11:28 AM, Kevin Cernekee wrote:
> On Mon, Mar 2, 2015 at 8:08 AM, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org> wrote:
>> On 03/02/2015 09:56 AM, Kevin Cernekee wrote:
>>> On Mon, Mar 2, 2015 at 5:14 AM, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org> wrote:
>>>> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
>>>>> These apply to newly converted drivers, like serial8250/libahci/...
>>>>> The examples were adapted from the regmap bindings document.
>>>>>
>>>>> Signed-off-by: Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>> ---
>>>>>  .../devicetree/bindings/common-properties.txt      | 60 ++++++++++++++++++++++
>>>>>  1 file changed, 60 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/common-properties.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
>>>>> new file mode 100644
>>>>> index 0000000..21044a4
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/common-properties.txt
>>>>> @@ -0,0 +1,60 @@
>>>>> +Common properties
>>>>> +
>>>>> +The ePAPR specification does not define any properties related to hardware
>>>>> +byteswapping, but endianness issues show up frequently in porting Linux to
>>>>> +different machine types.  This document attempts to provide a consistent
>>>>> +way of handling byteswapping across drivers.
>>>>> +
>>>>> +Optional properties:
>>>>> + - big-endian: Boolean; force big endian register accesses
>>>>> +   unconditionally (e.g. ioread32be/iowrite32be).  Use this if you
>>>>> +   know the peripheral always needs to be accessed in BE mode.
>>>>> + - little-endian: Boolean; force little endian register accesses
>>>>> +   unconditionally (e.g. readl/writel).  Use this if you know the
>>>>> +   peripheral always needs to be accessed in LE mode.  This is the
>>>>> +   default.
>>>>
>>>> There is a fundamental problem with specifying the default in DT bindings.
>>>> How can drivers which are currently native-endian support big-endian?
>>>>
>>>> If the driver is converted to support big-endian, every previous
>>>> devicetree will be invalid with the new kernel (because those devicetrees
>>>> don't specify 'native-endian').
>>>>
>>>> IOW, consider if the default were 'native-endian'. How would the 8250
>>>> driver support existing devicetrees?
>>>
>>> Correct.  This scheme is intended for drivers like 8250 and libahci
>>> which currently default to little-endian by virtue of using
>>> readl/writel for MMIO accesses.  Drivers that default to native-endian
>>> should specify that in their bindings documents, similar to
>>> Documentation/devicetree/bindings/regmap/regmap.txt.
>>
>> Which effectively means if a user can't upgrade their devicetree, they
>> can't upgrade their kernel. I don't think that flies.
> 
> This doesn't change the behavior of pre-existing drivers that
> implement the *-endian properties in a different way.  There are not
> many of these drivers and they can be documented as special cases.

Yeah, ok, as long as there's no expectation that existing drivers
meet this criteria when they add big-endian support.

>> It's exactly this kind of stuff that prompted Jonathan Corbet's article,
>> "Device trees as ABI"  http://lwn.net/Articles/561462
>>
>> Why not leave the default unspecified?
> 
> The document aims to provide a consistent way of handling DT
> endianness properties across (compliant) drivers.  It is confusing if
> one new driver defaults to little-endian, and another new driver
> defaults to native-endian.

Ok. How many 4.0 driver + DT submissions that are native-endian are
declaring this binding?


> And since most of the commonly used drivers already implement
> little-endian MMIO accesses, that is the default.  My personal
> preference would have been native-endian since that seems more common
> on the hardware side, but defaulting to little-endian prevents
> breaking the device tree "ABI" on existing systems.

That was basically my point; there's no way to meet these goals
for existing, native-endian drivers without breakage (just as there
would have been no way if native-endian had been the default).

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Peter Hurley]

On 03/02/2015 11:28 AM, Kevin Cernekee wrote:
> On Mon, Mar 2, 2015 at 8:08 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> On 03/02/2015 09:56 AM, Kevin Cernekee wrote:
>>> On Mon, Mar 2, 2015 at 5:14 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
>>>>> These apply to newly converted drivers, like serial8250/libahci/...
>>>>> The examples were adapted from the regmap bindings document.
>>>>>
>>>>> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
>>>>> ---
>>>>>  .../devicetree/bindings/common-properties.txt      | 60 ++++++++++++++++++++++
>>>>>  1 file changed, 60 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/common-properties.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
>>>>> new file mode 100644
>>>>> index 0000000..21044a4
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/common-properties.txt
>>>>> @@ -0,0 +1,60 @@
>>>>> +Common properties
>>>>> +
>>>>> +The ePAPR specification does not define any properties related to hardware
>>>>> +byteswapping, but endianness issues show up frequently in porting Linux to
>>>>> +different machine types.  This document attempts to provide a consistent
>>>>> +way of handling byteswapping across drivers.
>>>>> +
>>>>> +Optional properties:
>>>>> + - big-endian: Boolean; force big endian register accesses
>>>>> +   unconditionally (e.g. ioread32be/iowrite32be).  Use this if you
>>>>> +   know the peripheral always needs to be accessed in BE mode.
>>>>> + - little-endian: Boolean; force little endian register accesses
>>>>> +   unconditionally (e.g. readl/writel).  Use this if you know the
>>>>> +   peripheral always needs to be accessed in LE mode.  This is the
>>>>> +   default.
>>>>
>>>> There is a fundamental problem with specifying the default in DT bindings.
>>>> How can drivers which are currently native-endian support big-endian?
>>>>
>>>> If the driver is converted to support big-endian, every previous
>>>> devicetree will be invalid with the new kernel (because those devicetrees
>>>> don't specify 'native-endian').
>>>>
>>>> IOW, consider if the default were 'native-endian'. How would the 8250
>>>> driver support existing devicetrees?
>>>
>>> Correct.  This scheme is intended for drivers like 8250 and libahci
>>> which currently default to little-endian by virtue of using
>>> readl/writel for MMIO accesses.  Drivers that default to native-endian
>>> should specify that in their bindings documents, similar to
>>> Documentation/devicetree/bindings/regmap/regmap.txt.
>>
>> Which effectively means if a user can't upgrade their devicetree, they
>> can't upgrade their kernel. I don't think that flies.
> 
> This doesn't change the behavior of pre-existing drivers that
> implement the *-endian properties in a different way.  There are not
> many of these drivers and they can be documented as special cases.

Yeah, ok, as long as there's no expectation that existing drivers
meet this criteria when they add big-endian support.

>> It's exactly this kind of stuff that prompted Jonathan Corbet's article,
>> "Device trees as ABI"  http://lwn.net/Articles/561462
>>
>> Why not leave the default unspecified?
> 
> The document aims to provide a consistent way of handling DT
> endianness properties across (compliant) drivers.  It is confusing if
> one new driver defaults to little-endian, and another new driver
> defaults to native-endian.

Ok. How many 4.0 driver + DT submissions that are native-endian are
declaring this binding?


> And since most of the commonly used drivers already implement
> little-endian MMIO accesses, that is the default.  My personal
> preference would have been native-endian since that seems more common
> on the hardware side, but defaulting to little-endian prevents
> breaking the device tree "ABI" on existing systems.

That was basically my point; there's no way to meet these goals
for existing, native-endian drivers without breakage (just as there
would have been no way if native-endian had been the default).

Regards,
Peter Hurley

Re: [PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Kevin Cernekee]

On Mon, Mar 2, 2015 at 9:45 AM, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org> wrote:
>> This doesn't change the behavior of pre-existing drivers that
>> implement the *-endian properties in a different way.  There are not
>> many of these drivers and they can be documented as special cases.
>
> Yeah, ok, as long as there's no expectation that existing drivers
> meet this criteria when they add big-endian support.

The intention is to make it easy for existing drivers with LE register
accesses (i.e. mostly drivers taken from an x86 + PCI environment) to
work on systems with native to BE register accesses.  8250 and USB are
the first two examples of this.

>>> It's exactly this kind of stuff that prompted Jonathan Corbet's article,
>>> "Device trees as ABI"  http://lwn.net/Articles/561462
>>>
>>> Why not leave the default unspecified?
>>
>> The document aims to provide a consistent way of handling DT
>> endianness properties across (compliant) drivers.  It is confusing if
>> one new driver defaults to little-endian, and another new driver
>> defaults to native-endian.
>
> Ok. How many 4.0 driver + DT submissions that are native-endian are
> declaring this binding?
>
>
>> And since most of the commonly used drivers already implement
>> little-endian MMIO accesses, that is the default.  My personal
>> preference would have been native-endian since that seems more common
>> on the hardware side, but defaulting to little-endian prevents
>> breaking the device tree "ABI" on existing systems.
>
> That was basically my point; there's no way to meet these goals
> for existing, native-endian drivers without breakage (just as there
> would have been no way if native-endian had been the default).

I am not aware of any cases where the new binding needs to be applied
to a driver that is currently native-endian.  Grepping through the
tree for __raw_readl, I see lots of SoC-specific drivers but not a lot
of generic drivers shared by different types of platforms.  Most of
the time we can assume that whoever wrote the driver for their SoC has
figured out the endian situation.  But obviously there could be
exceptions, e.g. new chips using a different endian mode with the same
hardware block.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Kevin Cernekee]

On Mon, Mar 2, 2015 at 9:45 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> This doesn't change the behavior of pre-existing drivers that
>> implement the *-endian properties in a different way.  There are not
>> many of these drivers and they can be documented as special cases.
>
> Yeah, ok, as long as there's no expectation that existing drivers
> meet this criteria when they add big-endian support.

The intention is to make it easy for existing drivers with LE register
accesses (i.e. mostly drivers taken from an x86 + PCI environment) to
work on systems with native to BE register accesses.  8250 and USB are
the first two examples of this.

>>> It's exactly this kind of stuff that prompted Jonathan Corbet's article,
>>> "Device trees as ABI"  http://lwn.net/Articles/561462
>>>
>>> Why not leave the default unspecified?
>>
>> The document aims to provide a consistent way of handling DT
>> endianness properties across (compliant) drivers.  It is confusing if
>> one new driver defaults to little-endian, and another new driver
>> defaults to native-endian.
>
> Ok. How many 4.0 driver + DT submissions that are native-endian are
> declaring this binding?
>
>
>> And since most of the commonly used drivers already implement
>> little-endian MMIO accesses, that is the default.  My personal
>> preference would have been native-endian since that seems more common
>> on the hardware side, but defaulting to little-endian prevents
>> breaking the device tree "ABI" on existing systems.
>
> That was basically my point; there's no way to meet these goals
> for existing, native-endian drivers without breakage (just as there
> would have been no way if native-endian had been the default).

I am not aware of any cases where the new binding needs to be applied
to a driver that is currently native-endian.  Grepping through the
tree for __raw_readl, I see lots of SoC-specific drivers but not a lot
of generic drivers shared by different types of platforms.  Most of
the time we can assume that whoever wrote the driver for their SoC has
figured out the endian situation.  But obviously there could be
exceptions, e.g. new chips using a different endian mode with the same
hardware block.

Re: [PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Grant Likely]

On Mon, 2 Mar 2015 10:57:41 -0800
, Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
 wrote:
> On Mon, Mar 2, 2015 at 9:45 AM, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org> wrote:
> >> This doesn't change the behavior of pre-existing drivers that
> >> implement the *-endian properties in a different way.  There are not
> >> many of these drivers and they can be documented as special cases.
> >
> > Yeah, ok, as long as there's no expectation that existing drivers
> > meet this criteria when they add big-endian support.
> 
> The intention is to make it easy for existing drivers with LE register
> accesses (i.e. mostly drivers taken from an x86 + PCI environment) to
> work on systems with native to BE register accesses.  8250 and USB are
> the first two examples of this.

I think the right solution here is to drop any specified default in the
common properties binding and replace with something like, "If a binding
supports these properties, then that binding should also specify the
default if none of these properties are present. In such cases,
little-endian is the preferred default, but it is not a requirement"

g.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Grant Likely]

On Mon, 2 Mar 2015 10:57:41 -0800
, Kevin Cernekee <cernekee@gmail.com>
 wrote:
> On Mon, Mar 2, 2015 at 9:45 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
> >> This doesn't change the behavior of pre-existing drivers that
> >> implement the *-endian properties in a different way.  There are not
> >> many of these drivers and they can be documented as special cases.
> >
> > Yeah, ok, as long as there's no expectation that existing drivers
> > meet this criteria when they add big-endian support.
> 
> The intention is to make it easy for existing drivers with LE register
> accesses (i.e. mostly drivers taken from an x86 + PCI environment) to
> work on systems with native to BE register accesses.  8250 and USB are
> the first two examples of this.

I think the right solution here is to drop any specified default in the
common properties binding and replace with something like, "If a binding
supports these properties, then that binding should also specify the
default if none of these properties are present. In such cases,
little-endian is the preferred default, but it is not a requirement"

g.

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Grant Likely]

On Sun, 01 Mar 2015 17:23:11 -0500
, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
 wrote:
> Hi Kevin,
> 
> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
> > If an earlycon (stdout-path) node is being used, check for "big-endian"
> > or "native-endian" properties and pass the appropriate iotype to the
> > driver.
> > 
> > Note that LE sets UPIO_MEM (8-bit) but BE sets UPIO_MEM32BE (32-bit).  The
> > big-endian property only really makes sense in the context of 32-bit
> > registers, since 8-bit accesses never require data swapping.
> > 
> > At some point, the of_earlycon code may want to pass in the reg-io-width,
> > reg-offset, and reg-shift parameters too.
> > 
> > Signed-off-by: Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> >  drivers/of/fdt.c              | 7 ++++++-
> >  drivers/tty/serial/earlycon.c | 4 ++--
> >  include/linux/serial_core.h   | 2 +-
> >  3 files changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > index 658656f..9d21472 100644
> > --- a/drivers/of/fdt.c
> > +++ b/drivers/of/fdt.c
> > @@ -794,6 +794,8 @@ int __init early_init_dt_scan_chosen_serial(void)
> >  
> >  	while (match->compatible[0]) {
> >  		unsigned long addr;
> > +		unsigned char iotype = UPIO_MEM;
> > +
> >  		if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
> >  			match++;
> >  			continue;
> > @@ -803,7 +805,10 @@ int __init early_init_dt_scan_chosen_serial(void)
> >  		if (!addr)
> >  			return -ENXIO;
> >  
> > -		of_setup_earlycon(addr, match->data);
> > +		if (of_fdt_is_big_endian(fdt, offset))
> > +			iotype = UPIO_MEM32BE;
> > +
> > +		of_setup_earlycon(addr, iotype, match->data);
> 
> I know these got ACKs already but as you point out in the commit log,
> earlycon _will_ need reg-io-width, reg-offset and reg-shift. Since the
> distinction between early_init_dt_scan_chosen_serial() and
> of_setup_earlycon() is arbitrary, I'd rather see of_setup_earlycon()
> taught to properly decode of_serial driver bindings instead of a
> stack of parameters to of_setup_earlycon().
> 
> In fact, this patch allows a mis-defined devicetree to bring up a
> functioning earlycon because the 'big-endian' property is directly
> associated with UPIO_MEM32BE, which will create incompatibility problems
> when DT earlycon is fixed to decode the of_serial DT bindings.

That's a good point. This hasn't been merged yet, so there isn't any
impact on addressing this. I would propose that for consistency, the
earlycon code should always default to 8-bit access. if big-endian
accesses are required, then reg-io-width + big-endian must be specified.

Something like the following would do it and would be future-proof. We
can add support for 16 or 64bit big or little endian access if it ever
became necessary.

static int of_flat_dt_get_iotype(unsigned long node)
{
	int size, width = 1;
	__be32 *prop;
	bool bigendian = false;

	if (of_get_flat_dt_prop(node, "big-endian", NULL));
		bigendian = true;

	prop = of_get_flat_dt_prop(node, "reg-io-width", &size);
	if (prop) {
		if (size != sizeof(u32))
			return -EINVAL;
		width = fdt32_to_cpu(*prop);
	}

	switch (width) {
	case 1:
		return UPIO_MEM; /* big-endian flag has no effect */
	case 4:
		return bigendian ? UPIO_MEM32BE : UPIO_MEM32;
	default:
		return -EINVAL;
	}
}

...

	iotype = of_fdt_get_iotype(fdt, offset);
	if (iotype == UPIO_INVAL)
		/*fail*/

g.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Grant Likely]

On Sun, 01 Mar 2015 17:23:11 -0500
, Peter Hurley <peter@hurleysoftware.com>
 wrote:
> Hi Kevin,
> 
> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
> > If an earlycon (stdout-path) node is being used, check for "big-endian"
> > or "native-endian" properties and pass the appropriate iotype to the
> > driver.
> > 
> > Note that LE sets UPIO_MEM (8-bit) but BE sets UPIO_MEM32BE (32-bit).  The
> > big-endian property only really makes sense in the context of 32-bit
> > registers, since 8-bit accesses never require data swapping.
> > 
> > At some point, the of_earlycon code may want to pass in the reg-io-width,
> > reg-offset, and reg-shift parameters too.
> > 
> > Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
> > ---
> >  drivers/of/fdt.c              | 7 ++++++-
> >  drivers/tty/serial/earlycon.c | 4 ++--
> >  include/linux/serial_core.h   | 2 +-
> >  3 files changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > index 658656f..9d21472 100644
> > --- a/drivers/of/fdt.c
> > +++ b/drivers/of/fdt.c
> > @@ -794,6 +794,8 @@ int __init early_init_dt_scan_chosen_serial(void)
> >  
> >  	while (match->compatible[0]) {
> >  		unsigned long addr;
> > +		unsigned char iotype = UPIO_MEM;
> > +
> >  		if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
> >  			match++;
> >  			continue;
> > @@ -803,7 +805,10 @@ int __init early_init_dt_scan_chosen_serial(void)
> >  		if (!addr)
> >  			return -ENXIO;
> >  
> > -		of_setup_earlycon(addr, match->data);
> > +		if (of_fdt_is_big_endian(fdt, offset))
> > +			iotype = UPIO_MEM32BE;
> > +
> > +		of_setup_earlycon(addr, iotype, match->data);
> 
> I know these got ACKs already but as you point out in the commit log,
> earlycon _will_ need reg-io-width, reg-offset and reg-shift. Since the
> distinction between early_init_dt_scan_chosen_serial() and
> of_setup_earlycon() is arbitrary, I'd rather see of_setup_earlycon()
> taught to properly decode of_serial driver bindings instead of a
> stack of parameters to of_setup_earlycon().
> 
> In fact, this patch allows a mis-defined devicetree to bring up a
> functioning earlycon because the 'big-endian' property is directly
> associated with UPIO_MEM32BE, which will create incompatibility problems
> when DT earlycon is fixed to decode the of_serial DT bindings.

That's a good point. This hasn't been merged yet, so there isn't any
impact on addressing this. I would propose that for consistency, the
earlycon code should always default to 8-bit access. if big-endian
accesses are required, then reg-io-width + big-endian must be specified.

Something like the following would do it and would be future-proof. We
can add support for 16 or 64bit big or little endian access if it ever
became necessary.

static int of_flat_dt_get_iotype(unsigned long node)
{
	int size, width = 1;
	__be32 *prop;
	bool bigendian = false;

	if (of_get_flat_dt_prop(node, "big-endian", NULL));
		bigendian = true;

	prop = of_get_flat_dt_prop(node, "reg-io-width", &size);
	if (prop) {
		if (size != sizeof(u32))
			return -EINVAL;
		width = fdt32_to_cpu(*prop);
	}

	switch (width) {
	case 1:
		return UPIO_MEM; /* big-endian flag has no effect */
	case 4:
		return bigendian ? UPIO_MEM32BE : UPIO_MEM32;
	default:
		return -EINVAL;
	}
}

...

	iotype = of_fdt_get_iotype(fdt, offset);
	if (iotype == UPIO_INVAL)
		/*fail*/

g.

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

Hi Grant,

On 03/27/2015 09:36 PM, Grant Likely wrote:
> On Sun, 01 Mar 2015 17:23:11 -0500
> , Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
>  wrote:
>> Hi Kevin,
>>
>> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
>>> If an earlycon (stdout-path) node is being used, check for "big-endian"
>>> or "native-endian" properties and pass the appropriate iotype to the
>>> driver.
>>>
>>> Note that LE sets UPIO_MEM (8-bit) but BE sets UPIO_MEM32BE (32-bit).  The
>>> big-endian property only really makes sense in the context of 32-bit
>>> registers, since 8-bit accesses never require data swapping.
>>>
>>> At some point, the of_earlycon code may want to pass in the reg-io-width,
>>> reg-offset, and reg-shift parameters too.
>>>
>>> Signed-off-by: Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>>  drivers/of/fdt.c              | 7 ++++++-
>>>  drivers/tty/serial/earlycon.c | 4 ++--
>>>  include/linux/serial_core.h   | 2 +-
>>>  3 files changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>> index 658656f..9d21472 100644
>>> --- a/drivers/of/fdt.c
>>> +++ b/drivers/of/fdt.c
>>> @@ -794,6 +794,8 @@ int __init early_init_dt_scan_chosen_serial(void)
>>>  
>>>  	while (match->compatible[0]) {
>>>  		unsigned long addr;
>>> +		unsigned char iotype = UPIO_MEM;
>>> +
>>>  		if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
>>>  			match++;
>>>  			continue;
>>> @@ -803,7 +805,10 @@ int __init early_init_dt_scan_chosen_serial(void)
>>>  		if (!addr)
>>>  			return -ENXIO;
>>>  
>>> -		of_setup_earlycon(addr, match->data);
>>> +		if (of_fdt_is_big_endian(fdt, offset))
>>> +			iotype = UPIO_MEM32BE;
>>> +
>>> +		of_setup_earlycon(addr, iotype, match->data);
>>
>> I know these got ACKs already but as you point out in the commit log,
>> earlycon _will_ need reg-io-width, reg-offset and reg-shift. Since the
>> distinction between early_init_dt_scan_chosen_serial() and
>> of_setup_earlycon() is arbitrary, I'd rather see of_setup_earlycon()
>> taught to properly decode of_serial driver bindings instead of a
>> stack of parameters to of_setup_earlycon().
>>
>> In fact, this patch allows a mis-defined devicetree to bring up a
>> functioning earlycon because the 'big-endian' property is directly
>> associated with UPIO_MEM32BE, which will create incompatibility problems
>> when DT earlycon is fixed to decode the of_serial DT bindings.
> 
> That's a good point. This hasn't been merged yet, so there isn't any
> impact on addressing this. I would propose that for consistency, the
> earlycon code should always default to 8-bit access. if big-endian
> accesses are required, then reg-io-width + big-endian must be specified.
> 
> Something like the following would do it and would be future-proof. We
> can add support for 16 or 64bit big or little endian access if it ever
> became necessary.

I was planning on adding MEM32BE support to OF earlycon on top of my
patch series 'OF earlycon cleanup', which adds full support for the
of_serial driver DT properties (among other things).

Unfortunately, that series is waiting on two things:
1. libfdt upstream patch, which I submitted but was referred back to me
to add test cases. That was 3 weeks ago and I simply haven't had a free
day to burn to figure out how their test matrix is organized. I don't
think that's going to change anytime soon; I might just abandon that patch
and do the string manipulation on the stack.

ATM, earlycon is still broken if stdout-path options have been set.

2. Rob never got back to me on my query [1] to unify the OF_EARLYCON_DECLARE
macro with the EARLYCON_DECLARE macro so that all earlycon consoles
are named.

Regards,
Peter Hurley

[1] https://lkml.org/lkml/2015/3/6/571

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

Hi Grant,

On 03/27/2015 09:36 PM, Grant Likely wrote:
> On Sun, 01 Mar 2015 17:23:11 -0500
> , Peter Hurley <peter@hurleysoftware.com>
>  wrote:
>> Hi Kevin,
>>
>> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
>>> If an earlycon (stdout-path) node is being used, check for "big-endian"
>>> or "native-endian" properties and pass the appropriate iotype to the
>>> driver.
>>>
>>> Note that LE sets UPIO_MEM (8-bit) but BE sets UPIO_MEM32BE (32-bit).  The
>>> big-endian property only really makes sense in the context of 32-bit
>>> registers, since 8-bit accesses never require data swapping.
>>>
>>> At some point, the of_earlycon code may want to pass in the reg-io-width,
>>> reg-offset, and reg-shift parameters too.
>>>
>>> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
>>> ---
>>>  drivers/of/fdt.c              | 7 ++++++-
>>>  drivers/tty/serial/earlycon.c | 4 ++--
>>>  include/linux/serial_core.h   | 2 +-
>>>  3 files changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>> index 658656f..9d21472 100644
>>> --- a/drivers/of/fdt.c
>>> +++ b/drivers/of/fdt.c
>>> @@ -794,6 +794,8 @@ int __init early_init_dt_scan_chosen_serial(void)
>>>  
>>>  	while (match->compatible[0]) {
>>>  		unsigned long addr;
>>> +		unsigned char iotype = UPIO_MEM;
>>> +
>>>  		if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
>>>  			match++;
>>>  			continue;
>>> @@ -803,7 +805,10 @@ int __init early_init_dt_scan_chosen_serial(void)
>>>  		if (!addr)
>>>  			return -ENXIO;
>>>  
>>> -		of_setup_earlycon(addr, match->data);
>>> +		if (of_fdt_is_big_endian(fdt, offset))
>>> +			iotype = UPIO_MEM32BE;
>>> +
>>> +		of_setup_earlycon(addr, iotype, match->data);
>>
>> I know these got ACKs already but as you point out in the commit log,
>> earlycon _will_ need reg-io-width, reg-offset and reg-shift. Since the
>> distinction between early_init_dt_scan_chosen_serial() and
>> of_setup_earlycon() is arbitrary, I'd rather see of_setup_earlycon()
>> taught to properly decode of_serial driver bindings instead of a
>> stack of parameters to of_setup_earlycon().
>>
>> In fact, this patch allows a mis-defined devicetree to bring up a
>> functioning earlycon because the 'big-endian' property is directly
>> associated with UPIO_MEM32BE, which will create incompatibility problems
>> when DT earlycon is fixed to decode the of_serial DT bindings.
> 
> That's a good point. This hasn't been merged yet, so there isn't any
> impact on addressing this. I would propose that for consistency, the
> earlycon code should always default to 8-bit access. if big-endian
> accesses are required, then reg-io-width + big-endian must be specified.
> 
> Something like the following would do it and would be future-proof. We
> can add support for 16 or 64bit big or little endian access if it ever
> became necessary.

I was planning on adding MEM32BE support to OF earlycon on top of my
patch series 'OF earlycon cleanup', which adds full support for the
of_serial driver DT properties (among other things).

Unfortunately, that series is waiting on two things:
1. libfdt upstream patch, which I submitted but was referred back to me
to add test cases. That was 3 weeks ago and I simply haven't had a free
day to burn to figure out how their test matrix is organized. I don't
think that's going to change anytime soon; I might just abandon that patch
and do the string manipulation on the stack.

ATM, earlycon is still broken if stdout-path options have been set.

2. Rob never got back to me on my query [1] to unify the OF_EARLYCON_DECLARE
macro with the EARLYCON_DECLARE macro so that all earlycon consoles
are named.

Regards,
Peter Hurley

[1] https://lkml.org/lkml/2015/3/6/571

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Kevin Cernekee]

On Sat, Mar 28, 2015 at 10:01 AM, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org> wrote:
>>> I know these got ACKs already but as you point out in the commit log,
>>> earlycon _will_ need reg-io-width, reg-offset and reg-shift. Since the
>>> distinction between early_init_dt_scan_chosen_serial() and
>>> of_setup_earlycon() is arbitrary, I'd rather see of_setup_earlycon()
>>> taught to properly decode of_serial driver bindings instead of a
>>> stack of parameters to of_setup_earlycon().
>>>
>>> In fact, this patch allows a mis-defined devicetree to bring up a
>>> functioning earlycon because the 'big-endian' property is directly
>>> associated with UPIO_MEM32BE, which will create incompatibility problems
>>> when DT earlycon is fixed to decode the of_serial DT bindings.
>>
>> That's a good point. This hasn't been merged yet, so there isn't any
>> impact on addressing this. I would propose that for consistency, the
>> earlycon code should always default to 8-bit access. if big-endian
>> accesses are required, then reg-io-width + big-endian must be specified.
>>
>> Something like the following would do it and would be future-proof. We
>> can add support for 16 or 64bit big or little endian access if it ever
>> became necessary.
>
> I was planning on adding MEM32BE support to OF earlycon on top of my
> patch series 'OF earlycon cleanup', which adds full support for the
> of_serial driver DT properties (among other things).

Hi Peter,

This is my latest work-in-progress, incorporating the feedback from
you and Grant:

https://github.com/cernekee/linux/commits/endian

Not sure if this code plays nice with your recent cleanups?  If we're
touching the same files/functions we should probably coordinate.

Also, it is untested, as I do not currently have access to BE systems.
If I get desperate I can try it on an LE system, adding the big-endian
properties in DT and then hacking the 8250 driver to swap LE accesses
for BE accesses.

> Unfortunately, that series is waiting on two things:
> 1. libfdt upstream patch, which I submitted but was referred back to me
> to add test cases. That was 3 weeks ago and I simply haven't had a free
> day to burn to figure out how their test matrix is organized. I don't
> think that's going to change anytime soon; I might just abandon that patch
> and do the string manipulation on the stack.
>
> ATM, earlycon is still broken if stdout-path options have been set.
>
> 2. Rob never got back to me on my query [1] to unify the OF_EARLYCON_DECLARE
> macro with the EARLYCON_DECLARE macro so that all earlycon consoles
> are named.

Side note:

AFAIK we still have a problem if somebody wants to build serial8250 +
(any other tty driver that occupies major 4 / minor 64) into the same
kernel, and use DT to pick the correct driver at runtime.
serial8250_init() starts registering ports before it knows whether the
system even has an 8250.  I talked to Rob about it earlier this week
and he suggested that you might have some thoughts on how to fix it.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Kevin Cernekee]

On Sat, Mar 28, 2015 at 10:01 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>> I know these got ACKs already but as you point out in the commit log,
>>> earlycon _will_ need reg-io-width, reg-offset and reg-shift. Since the
>>> distinction between early_init_dt_scan_chosen_serial() and
>>> of_setup_earlycon() is arbitrary, I'd rather see of_setup_earlycon()
>>> taught to properly decode of_serial driver bindings instead of a
>>> stack of parameters to of_setup_earlycon().
>>>
>>> In fact, this patch allows a mis-defined devicetree to bring up a
>>> functioning earlycon because the 'big-endian' property is directly
>>> associated with UPIO_MEM32BE, which will create incompatibility problems
>>> when DT earlycon is fixed to decode the of_serial DT bindings.
>>
>> That's a good point. This hasn't been merged yet, so there isn't any
>> impact on addressing this. I would propose that for consistency, the
>> earlycon code should always default to 8-bit access. if big-endian
>> accesses are required, then reg-io-width + big-endian must be specified.
>>
>> Something like the following would do it and would be future-proof. We
>> can add support for 16 or 64bit big or little endian access if it ever
>> became necessary.
>
> I was planning on adding MEM32BE support to OF earlycon on top of my
> patch series 'OF earlycon cleanup', which adds full support for the
> of_serial driver DT properties (among other things).

Hi Peter,

This is my latest work-in-progress, incorporating the feedback from
you and Grant:

https://github.com/cernekee/linux/commits/endian

Not sure if this code plays nice with your recent cleanups?  If we're
touching the same files/functions we should probably coordinate.

Also, it is untested, as I do not currently have access to BE systems.
If I get desperate I can try it on an LE system, adding the big-endian
properties in DT and then hacking the 8250 driver to swap LE accesses
for BE accesses.

> Unfortunately, that series is waiting on two things:
> 1. libfdt upstream patch, which I submitted but was referred back to me
> to add test cases. That was 3 weeks ago and I simply haven't had a free
> day to burn to figure out how their test matrix is organized. I don't
> think that's going to change anytime soon; I might just abandon that patch
> and do the string manipulation on the stack.
>
> ATM, earlycon is still broken if stdout-path options have been set.
>
> 2. Rob never got back to me on my query [1] to unify the OF_EARLYCON_DECLARE
> macro with the EARLYCON_DECLARE macro so that all earlycon consoles
> are named.

Side note:

AFAIK we still have a problem if somebody wants to build serial8250 +
(any other tty driver that occupies major 4 / minor 64) into the same
kernel, and use DT to pick the correct driver at runtime.
serial8250_init() starts registering ports before it knows whether the
system even has an 8250.  I talked to Rob about it earlier this week
and he suggested that you might have some thoughts on how to fix it.

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Grant Likely]

On Sat, 28 Mar 2015 13:01:24 -0400
, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
 wrote:
> Hi Grant,
> 
> On 03/27/2015 09:36 PM, Grant Likely wrote:
> > On Sun, 01 Mar 2015 17:23:11 -0500
> > , Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
> >  wrote:
> >> Hi Kevin,
> >>
> >> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
> >>> If an earlycon (stdout-path) node is being used, check for "big-endian"
> >>> or "native-endian" properties and pass the appropriate iotype to the
> >>> driver.
> >>>
> >>> Note that LE sets UPIO_MEM (8-bit) but BE sets UPIO_MEM32BE (32-bit).  The
> >>> big-endian property only really makes sense in the context of 32-bit
> >>> registers, since 8-bit accesses never require data swapping.
> >>>
> >>> At some point, the of_earlycon code may want to pass in the reg-io-width,
> >>> reg-offset, and reg-shift parameters too.
> >>>
> >>> Signed-off-by: Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>> ---
> >>>  drivers/of/fdt.c              | 7 ++++++-
> >>>  drivers/tty/serial/earlycon.c | 4 ++--
> >>>  include/linux/serial_core.h   | 2 +-
> >>>  3 files changed, 9 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> >>> index 658656f..9d21472 100644
> >>> --- a/drivers/of/fdt.c
> >>> +++ b/drivers/of/fdt.c
> >>> @@ -794,6 +794,8 @@ int __init early_init_dt_scan_chosen_serial(void)
> >>>  
> >>>  	while (match->compatible[0]) {
> >>>  		unsigned long addr;
> >>> +		unsigned char iotype = UPIO_MEM;
> >>> +
> >>>  		if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
> >>>  			match++;
> >>>  			continue;
> >>> @@ -803,7 +805,10 @@ int __init early_init_dt_scan_chosen_serial(void)
> >>>  		if (!addr)
> >>>  			return -ENXIO;
> >>>  
> >>> -		of_setup_earlycon(addr, match->data);
> >>> +		if (of_fdt_is_big_endian(fdt, offset))
> >>> +			iotype = UPIO_MEM32BE;
> >>> +
> >>> +		of_setup_earlycon(addr, iotype, match->data);
> >>
> >> I know these got ACKs already but as you point out in the commit log,
> >> earlycon _will_ need reg-io-width, reg-offset and reg-shift. Since the
> >> distinction between early_init_dt_scan_chosen_serial() and
> >> of_setup_earlycon() is arbitrary, I'd rather see of_setup_earlycon()
> >> taught to properly decode of_serial driver bindings instead of a
> >> stack of parameters to of_setup_earlycon().
> >>
> >> In fact, this patch allows a mis-defined devicetree to bring up a
> >> functioning earlycon because the 'big-endian' property is directly
> >> associated with UPIO_MEM32BE, which will create incompatibility problems
> >> when DT earlycon is fixed to decode the of_serial DT bindings.
> > 
> > That's a good point. This hasn't been merged yet, so there isn't any
> > impact on addressing this. I would propose that for consistency, the
> > earlycon code should always default to 8-bit access. if big-endian
> > accesses are required, then reg-io-width + big-endian must be specified.
> > 
> > Something like the following would do it and would be future-proof. We
> > can add support for 16 or 64bit big or little endian access if it ever
> > became necessary.
> 
> I was planning on adding MEM32BE support to OF earlycon on top of my
> patch series 'OF earlycon cleanup', which adds full support for the
> of_serial driver DT properties (among other things).
> 
> Unfortunately, that series is waiting on two things:
> 1. libfdt upstream patch, which I submitted but was referred back to me
> to add test cases. That was 3 weeks ago and I simply haven't had a free
> day to burn to figure out how their test matrix is organized. I don't
> think that's going to change anytime soon; I might just abandon that patch
> and do the string manipulation on the stack.
> 
> ATM, earlycon is still broken if stdout-path options have been set.

I don't seem to have that patch. Can you send it to me please?

I do have a thought though. Would it be better to teach
fdt_path_offset() to recognize the ':' delimiter?  It's never a valid
character for a path.

The unittests are easy. "make check" builds and runs them. Adding a test
is as simple as editing tests/parent_offset.c. main() calls check_path()
several times to test calls to fdt_path_offset(). The tests can be added
directly to that file.

g.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Grant Likely]

On Sat, 28 Mar 2015 13:01:24 -0400
, Peter Hurley <peter@hurleysoftware.com>
 wrote:
> Hi Grant,
> 
> On 03/27/2015 09:36 PM, Grant Likely wrote:
> > On Sun, 01 Mar 2015 17:23:11 -0500
> > , Peter Hurley <peter@hurleysoftware.com>
> >  wrote:
> >> Hi Kevin,
> >>
> >> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
> >>> If an earlycon (stdout-path) node is being used, check for "big-endian"
> >>> or "native-endian" properties and pass the appropriate iotype to the
> >>> driver.
> >>>
> >>> Note that LE sets UPIO_MEM (8-bit) but BE sets UPIO_MEM32BE (32-bit).  The
> >>> big-endian property only really makes sense in the context of 32-bit
> >>> registers, since 8-bit accesses never require data swapping.
> >>>
> >>> At some point, the of_earlycon code may want to pass in the reg-io-width,
> >>> reg-offset, and reg-shift parameters too.
> >>>
> >>> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
> >>> ---
> >>>  drivers/of/fdt.c              | 7 ++++++-
> >>>  drivers/tty/serial/earlycon.c | 4 ++--
> >>>  include/linux/serial_core.h   | 2 +-
> >>>  3 files changed, 9 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> >>> index 658656f..9d21472 100644
> >>> --- a/drivers/of/fdt.c
> >>> +++ b/drivers/of/fdt.c
> >>> @@ -794,6 +794,8 @@ int __init early_init_dt_scan_chosen_serial(void)
> >>>  
> >>>  	while (match->compatible[0]) {
> >>>  		unsigned long addr;
> >>> +		unsigned char iotype = UPIO_MEM;
> >>> +
> >>>  		if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
> >>>  			match++;
> >>>  			continue;
> >>> @@ -803,7 +805,10 @@ int __init early_init_dt_scan_chosen_serial(void)
> >>>  		if (!addr)
> >>>  			return -ENXIO;
> >>>  
> >>> -		of_setup_earlycon(addr, match->data);
> >>> +		if (of_fdt_is_big_endian(fdt, offset))
> >>> +			iotype = UPIO_MEM32BE;
> >>> +
> >>> +		of_setup_earlycon(addr, iotype, match->data);
> >>
> >> I know these got ACKs already but as you point out in the commit log,
> >> earlycon _will_ need reg-io-width, reg-offset and reg-shift. Since the
> >> distinction between early_init_dt_scan_chosen_serial() and
> >> of_setup_earlycon() is arbitrary, I'd rather see of_setup_earlycon()
> >> taught to properly decode of_serial driver bindings instead of a
> >> stack of parameters to of_setup_earlycon().
> >>
> >> In fact, this patch allows a mis-defined devicetree to bring up a
> >> functioning earlycon because the 'big-endian' property is directly
> >> associated with UPIO_MEM32BE, which will create incompatibility problems
> >> when DT earlycon is fixed to decode the of_serial DT bindings.
> > 
> > That's a good point. This hasn't been merged yet, so there isn't any
> > impact on addressing this. I would propose that for consistency, the
> > earlycon code should always default to 8-bit access. if big-endian
> > accesses are required, then reg-io-width + big-endian must be specified.
> > 
> > Something like the following would do it and would be future-proof. We
> > can add support for 16 or 64bit big or little endian access if it ever
> > became necessary.
> 
> I was planning on adding MEM32BE support to OF earlycon on top of my
> patch series 'OF earlycon cleanup', which adds full support for the
> of_serial driver DT properties (among other things).
> 
> Unfortunately, that series is waiting on two things:
> 1. libfdt upstream patch, which I submitted but was referred back to me
> to add test cases. That was 3 weeks ago and I simply haven't had a free
> day to burn to figure out how their test matrix is organized. I don't
> think that's going to change anytime soon; I might just abandon that patch
> and do the string manipulation on the stack.
> 
> ATM, earlycon is still broken if stdout-path options have been set.

I don't seem to have that patch. Can you send it to me please?

I do have a thought though. Would it be better to teach
fdt_path_offset() to recognize the ':' delimiter?  It's never a valid
character for a path.

The unittests are easy. "make check" builds and runs them. Adding a test
is as simple as editing tests/parent_offset.c. main() calls check_path()
several times to test calls to fdt_path_offset(). The tests can be added
directly to that file.

g.

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Rob Herring]

On Sat, Mar 28, 2015 at 12:01 PM, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org> wrote:
> Hi Grant,
>
> On 03/27/2015 09:36 PM, Grant Likely wrote:
>> On Sun, 01 Mar 2015 17:23:11 -0500
>> , Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
>>  wrote:

[...]

>> Something like the following would do it and would be future-proof. We
>> can add support for 16 or 64bit big or little endian access if it ever
>> became necessary.
>
> I was planning on adding MEM32BE support to OF earlycon on top of my
> patch series 'OF earlycon cleanup', which adds full support for the
> of_serial driver DT properties (among other things).
>
> Unfortunately, that series is waiting on two things:
> 1. libfdt upstream patch, which I submitted but was referred back to me
> to add test cases. That was 3 weeks ago and I simply haven't had a free
> day to burn to figure out how their test matrix is organized. I don't
> think that's going to change anytime soon; I might just abandon that patch
> and do the string manipulation on the stack.
>
> ATM, earlycon is still broken if stdout-path options have been set.

Given David was okay with the patch itself, I suppose we could go
ahead and apply it to the kernel copy. If you're going to require
testcases, it should be trivial and/or documented on how to add...

> 2. Rob never got back to me on my query [1] to unify the OF_EARLYCON_DECLARE
> macro with the EARLYCON_DECLARE macro so that all earlycon consoles
> are named.

Sorry about that. I had thought about doing the same thing. At least
unifying the macros, but not necessarily the tables. If it is also
extendable to other firmware interfaces like ACPI perhaps that would
be good.

Rob

>
> Regards,
> Peter Hurley
>
> [1] https://lkml.org/lkml/2015/3/6/571
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Rob Herring]

On Sat, Mar 28, 2015 at 12:01 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
> Hi Grant,
>
> On 03/27/2015 09:36 PM, Grant Likely wrote:
>> On Sun, 01 Mar 2015 17:23:11 -0500
>> , Peter Hurley <peter@hurleysoftware.com>
>>  wrote:

[...]

>> Something like the following would do it and would be future-proof. We
>> can add support for 16 or 64bit big or little endian access if it ever
>> became necessary.
>
> I was planning on adding MEM32BE support to OF earlycon on top of my
> patch series 'OF earlycon cleanup', which adds full support for the
> of_serial driver DT properties (among other things).
>
> Unfortunately, that series is waiting on two things:
> 1. libfdt upstream patch, which I submitted but was referred back to me
> to add test cases. That was 3 weeks ago and I simply haven't had a free
> day to burn to figure out how their test matrix is organized. I don't
> think that's going to change anytime soon; I might just abandon that patch
> and do the string manipulation on the stack.
>
> ATM, earlycon is still broken if stdout-path options have been set.

Given David was okay with the patch itself, I suppose we could go
ahead and apply it to the kernel copy. If you're going to require
testcases, it should be trivial and/or documented on how to add...

> 2. Rob never got back to me on my query [1] to unify the OF_EARLYCON_DECLARE
> macro with the EARLYCON_DECLARE macro so that all earlycon consoles
> are named.

Sorry about that. I had thought about doing the same thing. At least
unifying the macros, but not necessarily the tables. If it is also
extendable to other firmware interfaces like ACPI perhaps that would
be good.

Rob

>
> Regards,
> Peter Hurley
>
> [1] https://lkml.org/lkml/2015/3/6/571
>

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

On 04/02/2015 09:32 AM, Grant Likely wrote:
> On Sat, 28 Mar 2015 13:01:24 -0400
> , Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
>  wrote:
>> Hi Grant,
>>
>> On 03/27/2015 09:36 PM, Grant Likely wrote:
>>> On Sun, 01 Mar 2015 17:23:11 -0500
>>> , Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
>>>  wrote:
>>>> Hi Kevin,
>>>>
>>>> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
>>>>> If an earlycon (stdout-path) node is being used, check for "big-endian"
>>>>> or "native-endian" properties and pass the appropriate iotype to the
>>>>> driver.
>>>>>
>>>>> Note that LE sets UPIO_MEM (8-bit) but BE sets UPIO_MEM32BE (32-bit).  The
>>>>> big-endian property only really makes sense in the context of 32-bit
>>>>> registers, since 8-bit accesses never require data swapping.
>>>>>
>>>>> At some point, the of_earlycon code may want to pass in the reg-io-width,
>>>>> reg-offset, and reg-shift parameters too.
>>>>>
>>>>> Signed-off-by: Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>> ---
>>>>>  drivers/of/fdt.c              | 7 ++++++-
>>>>>  drivers/tty/serial/earlycon.c | 4 ++--
>>>>>  include/linux/serial_core.h   | 2 +-
>>>>>  3 files changed, 9 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>>>> index 658656f..9d21472 100644
>>>>> --- a/drivers/of/fdt.c
>>>>> +++ b/drivers/of/fdt.c
>>>>> @@ -794,6 +794,8 @@ int __init early_init_dt_scan_chosen_serial(void)
>>>>>  
>>>>>  	while (match->compatible[0]) {
>>>>>  		unsigned long addr;
>>>>> +		unsigned char iotype = UPIO_MEM;
>>>>> +
>>>>>  		if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
>>>>>  			match++;
>>>>>  			continue;
>>>>> @@ -803,7 +805,10 @@ int __init early_init_dt_scan_chosen_serial(void)
>>>>>  		if (!addr)
>>>>>  			return -ENXIO;
>>>>>  
>>>>> -		of_setup_earlycon(addr, match->data);
>>>>> +		if (of_fdt_is_big_endian(fdt, offset))
>>>>> +			iotype = UPIO_MEM32BE;
>>>>> +
>>>>> +		of_setup_earlycon(addr, iotype, match->data);
>>>>
>>>> I know these got ACKs already but as you point out in the commit log,
>>>> earlycon _will_ need reg-io-width, reg-offset and reg-shift. Since the
>>>> distinction between early_init_dt_scan_chosen_serial() and
>>>> of_setup_earlycon() is arbitrary, I'd rather see of_setup_earlycon()
>>>> taught to properly decode of_serial driver bindings instead of a
>>>> stack of parameters to of_setup_earlycon().
>>>>
>>>> In fact, this patch allows a mis-defined devicetree to bring up a
>>>> functioning earlycon because the 'big-endian' property is directly
>>>> associated with UPIO_MEM32BE, which will create incompatibility problems
>>>> when DT earlycon is fixed to decode the of_serial DT bindings.
>>>
>>> That's a good point. This hasn't been merged yet, so there isn't any
>>> impact on addressing this. I would propose that for consistency, the
>>> earlycon code should always default to 8-bit access. if big-endian
>>> accesses are required, then reg-io-width + big-endian must be specified.
>>>
>>> Something like the following would do it and would be future-proof. We
>>> can add support for 16 or 64bit big or little endian access if it ever
>>> became necessary.
>>
>> I was planning on adding MEM32BE support to OF earlycon on top of my
>> patch series 'OF earlycon cleanup', which adds full support for the
>> of_serial driver DT properties (among other things).
>>
>> Unfortunately, that series is waiting on two things:
>> 1. libfdt upstream patch, which I submitted but was referred back to me
>> to add test cases. That was 3 weeks ago and I simply haven't had a free
>> day to burn to figure out how their test matrix is organized. I don't
>> think that's going to change anytime soon; I might just abandon that patch
>> and do the string manipulation on the stack.
>>
>> ATM, earlycon is still broken if stdout-path options have been set.
> 
> I don't seem to have that patch. Can you send it to me please?

Will do.

> I do have a thought though. Would it be better to teach
> fdt_path_offset() to recognize the ':' delimiter?  It's never a valid
> character for a path.
>
> The unittests are easy. "make check" builds and runs them. Adding a test
> is as simple as editing tests/parent_offset.c. main() calls check_path()
> several times to test calls to fdt_path_offset(). The tests can be added
> directly to that file.

Well, the patch reimplements fdt_path_offset() in terms of a new
length-limited function, fdt_path_offset_namelen(). So the existing tests
of fdt_path_offset() are already exercising the patch.

The issue is that the unit tests for fdt_path_offset_namelen() will need
additional nodes and properties defined to properly test the functionality,
and it's not clear to which fdt files these changes are required.

Not that I can't figure it out, but right now, I just have way more
pressing matters, like outstanding regressions and the 8250 split, both
of which are overdue.

Regards,
Peter Hurley


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

On 04/02/2015 09:32 AM, Grant Likely wrote:
> On Sat, 28 Mar 2015 13:01:24 -0400
> , Peter Hurley <peter@hurleysoftware.com>
>  wrote:
>> Hi Grant,
>>
>> On 03/27/2015 09:36 PM, Grant Likely wrote:
>>> On Sun, 01 Mar 2015 17:23:11 -0500
>>> , Peter Hurley <peter@hurleysoftware.com>
>>>  wrote:
>>>> Hi Kevin,
>>>>
>>>> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
>>>>> If an earlycon (stdout-path) node is being used, check for "big-endian"
>>>>> or "native-endian" properties and pass the appropriate iotype to the
>>>>> driver.
>>>>>
>>>>> Note that LE sets UPIO_MEM (8-bit) but BE sets UPIO_MEM32BE (32-bit).  The
>>>>> big-endian property only really makes sense in the context of 32-bit
>>>>> registers, since 8-bit accesses never require data swapping.
>>>>>
>>>>> At some point, the of_earlycon code may want to pass in the reg-io-width,
>>>>> reg-offset, and reg-shift parameters too.
>>>>>
>>>>> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
>>>>> ---
>>>>>  drivers/of/fdt.c              | 7 ++++++-
>>>>>  drivers/tty/serial/earlycon.c | 4 ++--
>>>>>  include/linux/serial_core.h   | 2 +-
>>>>>  3 files changed, 9 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>>>> index 658656f..9d21472 100644
>>>>> --- a/drivers/of/fdt.c
>>>>> +++ b/drivers/of/fdt.c
>>>>> @@ -794,6 +794,8 @@ int __init early_init_dt_scan_chosen_serial(void)
>>>>>  
>>>>>  	while (match->compatible[0]) {
>>>>>  		unsigned long addr;
>>>>> +		unsigned char iotype = UPIO_MEM;
>>>>> +
>>>>>  		if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
>>>>>  			match++;
>>>>>  			continue;
>>>>> @@ -803,7 +805,10 @@ int __init early_init_dt_scan_chosen_serial(void)
>>>>>  		if (!addr)
>>>>>  			return -ENXIO;
>>>>>  
>>>>> -		of_setup_earlycon(addr, match->data);
>>>>> +		if (of_fdt_is_big_endian(fdt, offset))
>>>>> +			iotype = UPIO_MEM32BE;
>>>>> +
>>>>> +		of_setup_earlycon(addr, iotype, match->data);
>>>>
>>>> I know these got ACKs already but as you point out in the commit log,
>>>> earlycon _will_ need reg-io-width, reg-offset and reg-shift. Since the
>>>> distinction between early_init_dt_scan_chosen_serial() and
>>>> of_setup_earlycon() is arbitrary, I'd rather see of_setup_earlycon()
>>>> taught to properly decode of_serial driver bindings instead of a
>>>> stack of parameters to of_setup_earlycon().
>>>>
>>>> In fact, this patch allows a mis-defined devicetree to bring up a
>>>> functioning earlycon because the 'big-endian' property is directly
>>>> associated with UPIO_MEM32BE, which will create incompatibility problems
>>>> when DT earlycon is fixed to decode the of_serial DT bindings.
>>>
>>> That's a good point. This hasn't been merged yet, so there isn't any
>>> impact on addressing this. I would propose that for consistency, the
>>> earlycon code should always default to 8-bit access. if big-endian
>>> accesses are required, then reg-io-width + big-endian must be specified.
>>>
>>> Something like the following would do it and would be future-proof. We
>>> can add support for 16 or 64bit big or little endian access if it ever
>>> became necessary.
>>
>> I was planning on adding MEM32BE support to OF earlycon on top of my
>> patch series 'OF earlycon cleanup', which adds full support for the
>> of_serial driver DT properties (among other things).
>>
>> Unfortunately, that series is waiting on two things:
>> 1. libfdt upstream patch, which I submitted but was referred back to me
>> to add test cases. That was 3 weeks ago and I simply haven't had a free
>> day to burn to figure out how their test matrix is organized. I don't
>> think that's going to change anytime soon; I might just abandon that patch
>> and do the string manipulation on the stack.
>>
>> ATM, earlycon is still broken if stdout-path options have been set.
> 
> I don't seem to have that patch. Can you send it to me please?

Will do.

> I do have a thought though. Would it be better to teach
> fdt_path_offset() to recognize the ':' delimiter?  It's never a valid
> character for a path.
>
> The unittests are easy. "make check" builds and runs them. Adding a test
> is as simple as editing tests/parent_offset.c. main() calls check_path()
> several times to test calls to fdt_path_offset(). The tests can be added
> directly to that file.

Well, the patch reimplements fdt_path_offset() in terms of a new
length-limited function, fdt_path_offset_namelen(). So the existing tests
of fdt_path_offset() are already exercising the patch.

The issue is that the unit tests for fdt_path_offset_namelen() will need
additional nodes and properties defined to properly test the functionality,
and it's not clear to which fdt files these changes are required.

Not that I can't figure it out, but right now, I just have way more
pressing matters, like outstanding regressions and the 8250 split, both
of which are overdue.

Regards,
Peter Hurley

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

On 04/02/2015 09:46 AM, Rob Herring wrote:
> Sorry about that. I had thought about doing the same thing. At least
> unifying the macros, but not necessarily the tables. If it is also
> extendable to other firmware interfaces like ACPI perhaps that would
> be good.

No need to apologize; I'll make those changes and resubmit for your
review.

Regards,
Peter Hurley

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

On 04/02/2015 09:46 AM, Rob Herring wrote:
> Sorry about that. I had thought about doing the same thing. At least
> unifying the macros, but not necessarily the tables. If it is also
> extendable to other firmware interfaces like ACPI perhaps that would
> be good.

No need to apologize; I'll make those changes and resubmit for your
review.

Regards,
Peter Hurley

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

Hi Kevin,

On 03/28/2015 03:28 PM, Kevin Cernekee wrote:
> On Sat, Mar 28, 2015 at 10:01 AM, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org> wrote:
>>>> I know these got ACKs already but as you point out in the commit log,
>>>> earlycon _will_ need reg-io-width, reg-offset and reg-shift. Since the
>>>> distinction between early_init_dt_scan_chosen_serial() and
>>>> of_setup_earlycon() is arbitrary, I'd rather see of_setup_earlycon()
>>>> taught to properly decode of_serial driver bindings instead of a
>>>> stack of parameters to of_setup_earlycon().
>>>>
>>>> In fact, this patch allows a mis-defined devicetree to bring up a
>>>> functioning earlycon because the 'big-endian' property is directly
>>>> associated with UPIO_MEM32BE, which will create incompatibility problems
>>>> when DT earlycon is fixed to decode the of_serial DT bindings.
>>>
>>> That's a good point. This hasn't been merged yet, so there isn't any
>>> impact on addressing this. I would propose that for consistency, the
>>> earlycon code should always default to 8-bit access. if big-endian
>>> accesses are required, then reg-io-width + big-endian must be specified.
>>>
>>> Something like the following would do it and would be future-proof. We
>>> can add support for 16 or 64bit big or little endian access if it ever
>>> became necessary.
>>
>> I was planning on adding MEM32BE support to OF earlycon on top of my
>> patch series 'OF earlycon cleanup', which adds full support for the
>> of_serial driver DT properties (among other things).
> 
> Hi Peter,
> 
> This is my latest work-in-progress, incorporating the feedback from
> you and Grant:
> 
> https://github.com/cernekee/linux/commits/endian
> 
> Not sure if this code plays nice with your recent cleanups?  If we're
> touching the same files/functions we should probably coordinate.

Ok, I'll look over your git tree and add whatever's required to
earlycon.

> Also, it is untested, as I do not currently have access to BE systems.
> If I get desperate I can try it on an LE system, adding the big-endian
> properties in DT and then hacking the 8250 driver to swap LE accesses
> for BE accesses.

Ok.

Regards,
Peter Hurley

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

Hi Kevin,

On 03/28/2015 03:28 PM, Kevin Cernekee wrote:
> On Sat, Mar 28, 2015 at 10:01 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>> I know these got ACKs already but as you point out in the commit log,
>>>> earlycon _will_ need reg-io-width, reg-offset and reg-shift. Since the
>>>> distinction between early_init_dt_scan_chosen_serial() and
>>>> of_setup_earlycon() is arbitrary, I'd rather see of_setup_earlycon()
>>>> taught to properly decode of_serial driver bindings instead of a
>>>> stack of parameters to of_setup_earlycon().
>>>>
>>>> In fact, this patch allows a mis-defined devicetree to bring up a
>>>> functioning earlycon because the 'big-endian' property is directly
>>>> associated with UPIO_MEM32BE, which will create incompatibility problems
>>>> when DT earlycon is fixed to decode the of_serial DT bindings.
>>>
>>> That's a good point. This hasn't been merged yet, so there isn't any
>>> impact on addressing this. I would propose that for consistency, the
>>> earlycon code should always default to 8-bit access. if big-endian
>>> accesses are required, then reg-io-width + big-endian must be specified.
>>>
>>> Something like the following would do it and would be future-proof. We
>>> can add support for 16 or 64bit big or little endian access if it ever
>>> became necessary.
>>
>> I was planning on adding MEM32BE support to OF earlycon on top of my
>> patch series 'OF earlycon cleanup', which adds full support for the
>> of_serial driver DT properties (among other things).
> 
> Hi Peter,
> 
> This is my latest work-in-progress, incorporating the feedback from
> you and Grant:
> 
> https://github.com/cernekee/linux/commits/endian
> 
> Not sure if this code plays nice with your recent cleanups?  If we're
> touching the same files/functions we should probably coordinate.

Ok, I'll look over your git tree and add whatever's required to
earlycon.

> Also, it is untested, as I do not currently have access to BE systems.
> If I get desperate I can try it on an LE system, adding the big-endian
> properties in DT and then hacking the 8250 driver to swap LE accesses
> for BE accesses.

Ok.

Regards,
Peter Hurley

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

On 03/28/2015 03:28 PM, Kevin Cernekee wrote:
> Side note:
> 
> AFAIK we still have a problem if somebody wants to build serial8250 +
> (any other tty driver that occupies major 4 / minor 64) into the same
> kernel, and use DT to pick the correct driver at runtime.

Yep, exactly.

> serial8250_init() starts registering ports before it knows whether the
> system even has an 8250.

This behavior is required to support hw configuration from userspace.

> I talked to Rob about it earlier this week
> and he suggested that you might have some thoughts on how to fix it.
 
A big piece of that will land in 4.01 (once I get the actual 8250 split
commit to Greg).

The 8250 driver has been split into a legacy/universal 8250 driver
and separate port ops module; one of the benefits of this is that the
objectionable legacy behavior of the 8250 driver (especially limitations
wrt ttyS sharing) can be left behind in that driver without breaking
existing users.

The idea is that an alternate 8250 driver(s) with none of the legacy
baggage can be taught to share (4,64) ports with other drivers.
Unfortunately, I haven't made further progress with because of other
projects.

Regards,
Peter Hurley




--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

On 03/28/2015 03:28 PM, Kevin Cernekee wrote:
> Side note:
> 
> AFAIK we still have a problem if somebody wants to build serial8250 +
> (any other tty driver that occupies major 4 / minor 64) into the same
> kernel, and use DT to pick the correct driver at runtime.

Yep, exactly.

> serial8250_init() starts registering ports before it knows whether the
> system even has an 8250.

This behavior is required to support hw configuration from userspace.

> I talked to Rob about it earlier this week
> and he suggested that you might have some thoughts on how to fix it.
 
A big piece of that will land in 4.01 (once I get the actual 8250 split
commit to Greg).

The 8250 driver has been split into a legacy/universal 8250 driver
and separate port ops module; one of the benefits of this is that the
objectionable legacy behavior of the 8250 driver (especially limitations
wrt ttyS sharing) can be left behind in that driver without breaking
existing users.

The idea is that an alternate 8250 driver(s) with none of the legacy
baggage can be taught to share (4,64) ports with other drivers.
Unfortunately, I haven't made further progress with because of other
projects.

Regards,
Peter Hurley

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

On 04/02/2015 11:35 AM, Peter Hurley wrote:
> On 04/02/2015 09:46 AM, Rob Herring wrote:
>> Sorry about that. I had thought about doing the same thing. At least
>> unifying the macros, but not necessarily the tables. If it is also
>> extendable to other firmware interfaces like ACPI perhaps that would
>> be good.
> 
> No need to apologize; I'll make those changes and resubmit for your
> review.

What about something like the following?

This patch makes both __earlycon_table and __earlycon_of_table
arrays of struct earlycon_id, and a follow-on patch would use the
earlycon name to initialize the struct console fields.

The benefits of this approach are
1. diagnostics can readily identify the earlycon if there is some error
2. it would be trivial to enable both command line and devicetree
   earlycon from the same earlycon declaration.

And a single table is doable from this point.

AFAICT there is no benefit to using actual OF tables, and I see no
other reasonable way to initialize the name of the struct console
for devicetree earlycons.

Regards,
Peter Hurley


--- >% ---
Subject: [PATCH] serial: earlycon: Use common framework for earlycon
 declarations

Use common table definition and implementation macro to declare an
earlycon, but retain separate tables for command line and devicetree.
Add __EARLYCON_DECLARE() macro to instance a unique earlycon
declaration for the specified table.

This enables all earlycons to properly initialize the earlycon console
structure with name and index.

Signed-off-by: Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
---
 drivers/of/fdt.c                  |  6 +++---
 drivers/tty/serial/earlycon.c     |  3 +--
 include/asm-generic/vmlinux.lds.h |  8 ++++++--
 include/linux/serial_core.h       | 30 +++++++++++++++++++++---------
 4 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 7cef9f9..f640efa 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -760,14 +760,14 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
 #endif /* CONFIG_BLK_DEV_INITRD */
 
 #ifdef CONFIG_SERIAL_EARLYCON
-extern struct of_device_id __earlycon_of_table[];
+extern struct earlycon_id __earlycon_of_table[];
 
 static int __init early_init_dt_scan_chosen_serial(void)
 {
 	int offset;
 	const char *p, *q;
 	int l;
-	const struct of_device_id *match = __earlycon_of_table;
+	const struct earlycon_id *match = __earlycon_of_table;
 	const void *fdt = initial_boot_params;
 
 	offset = fdt_path_offset(fdt, "/chosen");
@@ -800,7 +800,7 @@ static int __init early_init_dt_scan_chosen_serial(void)
 		if (!addr)
 			return -ENXIO;
 
-		of_setup_earlycon(addr, match->data);
+		of_setup_earlycon(addr, match->setup);
 		return 0;
 	}
 	return -ENODEV;
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 5fdc9f3..bf7eb76 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -19,7 +19,6 @@
 #include <linux/io.h>
 #include <linux/serial_core.h>
 #include <linux/sizes.h>
-#include <linux/mod_devicetable.h>
 
 #ifdef CONFIG_FIX_EARLYCON_MEM
 #include <asm/fixmap.h>
@@ -41,7 +40,7 @@ extern struct earlycon_id __earlycon_table[];
 static const struct earlycon_id __earlycon_table_sentinel
 	__used __section(__earlycon_table_end);
 
-static const struct of_device_id __earlycon_of_table_sentinel
+static const struct earlycon_id __earlycon_of_table_sentinel
 	__used __section(__earlycon_of_table_end);
 
 static void __iomem * __init earlycon_map(unsigned long paddr, size_t size)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 561daf4..7322c30 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -155,8 +155,13 @@
 			 VMLINUX_SYMBOL(__earlycon_table) = .;	\
 			 *(__earlycon_table)			\
 			 *(__earlycon_table_end)
+#define EARLYCON_OF_TABLE()	STRUCT_ALIGN();				 \
+				VMLINUX_SYMBOL(__earlycon_of_table) = .; \
+				*(__earlycon_of_table)			 \
+				*(__earlycon_of_table_end)
 #else
 #define EARLYCON_TABLE()
+#define EARLYCON_OF_TABLE()
 #endif
 
 #define ___OF_TABLE(cfg, name)	_OF_TABLE_##cfg(name)
@@ -175,7 +180,6 @@
 #define IOMMU_OF_TABLES()	OF_TABLE(CONFIG_OF_IOMMU, iommu)
 #define RESERVEDMEM_OF_TABLES()	OF_TABLE(CONFIG_OF_RESERVED_MEM, reservedmem)
 #define CPU_METHOD_OF_TABLES()	OF_TABLE(CONFIG_SMP, cpu_method)
-#define EARLYCON_OF_TABLES()	OF_TABLE(CONFIG_SERIAL_EARLYCON, earlycon)
 
 #define KERNEL_DTB()							\
 	STRUCT_ALIGN();							\
@@ -512,7 +516,7 @@
 	KERNEL_DTB()							\
 	IRQCHIP_OF_MATCH_TABLE()					\
 	EARLYCON_TABLE()						\
-	EARLYCON_OF_TABLES()
+	EARLYCON_OF_TABLE()
 
 #define INIT_TEXT							\
 	*(.init.text)							\
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 025dad9..9455e97 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -341,22 +341,34 @@ struct earlycon_device {
 
 struct earlycon_id {
 	char	name[16];
+	char	compatible[128];
 	int	(*setup)(struct earlycon_device *, const char *options);
 } __aligned(32);
 
+#define EARLYCON_TABLE __used __section(__earlycon_table)
+
+#ifdef CONFIG_OF
+#define EARLYCON_OF_TABLE __used __section(__earlycon_of_table)
+#else
+#define EARLYCON_OF_TABLE __attribute__((unused))
+#endif
+
+#define __EARLYCON_DECLARE(_name, compat, fn, pre, section)		\
+	static const struct earlycon_id __UNIQUE_ID(pre##_name) section	\
+		 = { .name = __stringify(_name),			\
+		     .compatible = compat,				\
+		     .setup = fn  }
+
+#define EARLYCON_DECLARE(_name, fn)					\
+	__EARLYCON_DECLARE(_name, "", fn, __earlycon_, EARLYCON_TABLE)
+
+#define OF_EARLYCON_DECLARE(_name, compat, fn)				\
+	__EARLYCON_DECLARE(_name, compat, fn, __earlycon_of_, EARLYCON_OF_TABLE)
+
 extern int setup_earlycon(char *buf);
 extern int of_setup_earlycon(unsigned long addr,
 			     int (*setup)(struct earlycon_device *, const char *));
 
-#define EARLYCON_DECLARE(_name, func)					\
-	static const struct earlycon_id __earlycon_##_name		\
-		__used __section(__earlycon_table)			\
-		 = { .name  = __stringify(_name),			\
-		     .setup = func  }
-
-#define OF_EARLYCON_DECLARE(name, compat, fn)				\
-	_OF_DECLARE(earlycon, name, compat, fn, void *)
-
 struct uart_port *uart_get_console(struct uart_port *ports, int nr,
 				   struct console *c);
 int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr,
-- 
2.3.5


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

On 04/02/2015 11:35 AM, Peter Hurley wrote:
> On 04/02/2015 09:46 AM, Rob Herring wrote:
>> Sorry about that. I had thought about doing the same thing. At least
>> unifying the macros, but not necessarily the tables. If it is also
>> extendable to other firmware interfaces like ACPI perhaps that would
>> be good.
> 
> No need to apologize; I'll make those changes and resubmit for your
> review.

What about something like the following?

This patch makes both __earlycon_table and __earlycon_of_table
arrays of struct earlycon_id, and a follow-on patch would use the
earlycon name to initialize the struct console fields.

The benefits of this approach are
1. diagnostics can readily identify the earlycon if there is some error
2. it would be trivial to enable both command line and devicetree
   earlycon from the same earlycon declaration.

And a single table is doable from this point.

AFAICT there is no benefit to using actual OF tables, and I see no
other reasonable way to initialize the name of the struct console
for devicetree earlycons.

Regards,
Peter Hurley


--- >% ---
Subject: [PATCH] serial: earlycon: Use common framework for earlycon
 declarations

Use common table definition and implementation macro to declare an
earlycon, but retain separate tables for command line and devicetree.
Add __EARLYCON_DECLARE() macro to instance a unique earlycon
declaration for the specified table.

This enables all earlycons to properly initialize the earlycon console
structure with name and index.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/of/fdt.c                  |  6 +++---
 drivers/tty/serial/earlycon.c     |  3 +--
 include/asm-generic/vmlinux.lds.h |  8 ++++++--
 include/linux/serial_core.h       | 30 +++++++++++++++++++++---------
 4 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 7cef9f9..f640efa 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -760,14 +760,14 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
 #endif /* CONFIG_BLK_DEV_INITRD */
 
 #ifdef CONFIG_SERIAL_EARLYCON
-extern struct of_device_id __earlycon_of_table[];
+extern struct earlycon_id __earlycon_of_table[];
 
 static int __init early_init_dt_scan_chosen_serial(void)
 {
 	int offset;
 	const char *p, *q;
 	int l;
-	const struct of_device_id *match = __earlycon_of_table;
+	const struct earlycon_id *match = __earlycon_of_table;
 	const void *fdt = initial_boot_params;
 
 	offset = fdt_path_offset(fdt, "/chosen");
@@ -800,7 +800,7 @@ static int __init early_init_dt_scan_chosen_serial(void)
 		if (!addr)
 			return -ENXIO;
 
-		of_setup_earlycon(addr, match->data);
+		of_setup_earlycon(addr, match->setup);
 		return 0;
 	}
 	return -ENODEV;
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 5fdc9f3..bf7eb76 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -19,7 +19,6 @@
 #include <linux/io.h>
 #include <linux/serial_core.h>
 #include <linux/sizes.h>
-#include <linux/mod_devicetable.h>
 
 #ifdef CONFIG_FIX_EARLYCON_MEM
 #include <asm/fixmap.h>
@@ -41,7 +40,7 @@ extern struct earlycon_id __earlycon_table[];
 static const struct earlycon_id __earlycon_table_sentinel
 	__used __section(__earlycon_table_end);
 
-static const struct of_device_id __earlycon_of_table_sentinel
+static const struct earlycon_id __earlycon_of_table_sentinel
 	__used __section(__earlycon_of_table_end);
 
 static void __iomem * __init earlycon_map(unsigned long paddr, size_t size)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 561daf4..7322c30 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -155,8 +155,13 @@
 			 VMLINUX_SYMBOL(__earlycon_table) = .;	\
 			 *(__earlycon_table)			\
 			 *(__earlycon_table_end)
+#define EARLYCON_OF_TABLE()	STRUCT_ALIGN();				 \
+				VMLINUX_SYMBOL(__earlycon_of_table) = .; \
+				*(__earlycon_of_table)			 \
+				*(__earlycon_of_table_end)
 #else
 #define EARLYCON_TABLE()
+#define EARLYCON_OF_TABLE()
 #endif
 
 #define ___OF_TABLE(cfg, name)	_OF_TABLE_##cfg(name)
@@ -175,7 +180,6 @@
 #define IOMMU_OF_TABLES()	OF_TABLE(CONFIG_OF_IOMMU, iommu)
 #define RESERVEDMEM_OF_TABLES()	OF_TABLE(CONFIG_OF_RESERVED_MEM, reservedmem)
 #define CPU_METHOD_OF_TABLES()	OF_TABLE(CONFIG_SMP, cpu_method)
-#define EARLYCON_OF_TABLES()	OF_TABLE(CONFIG_SERIAL_EARLYCON, earlycon)
 
 #define KERNEL_DTB()							\
 	STRUCT_ALIGN();							\
@@ -512,7 +516,7 @@
 	KERNEL_DTB()							\
 	IRQCHIP_OF_MATCH_TABLE()					\
 	EARLYCON_TABLE()						\
-	EARLYCON_OF_TABLES()
+	EARLYCON_OF_TABLE()
 
 #define INIT_TEXT							\
 	*(.init.text)							\
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 025dad9..9455e97 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -341,22 +341,34 @@ struct earlycon_device {
 
 struct earlycon_id {
 	char	name[16];
+	char	compatible[128];
 	int	(*setup)(struct earlycon_device *, const char *options);
 } __aligned(32);
 
+#define EARLYCON_TABLE __used __section(__earlycon_table)
+
+#ifdef CONFIG_OF
+#define EARLYCON_OF_TABLE __used __section(__earlycon_of_table)
+#else
+#define EARLYCON_OF_TABLE __attribute__((unused))
+#endif
+
+#define __EARLYCON_DECLARE(_name, compat, fn, pre, section)		\
+	static const struct earlycon_id __UNIQUE_ID(pre##_name) section	\
+		 = { .name = __stringify(_name),			\
+		     .compatible = compat,				\
+		     .setup = fn  }
+
+#define EARLYCON_DECLARE(_name, fn)					\
+	__EARLYCON_DECLARE(_name, "", fn, __earlycon_, EARLYCON_TABLE)
+
+#define OF_EARLYCON_DECLARE(_name, compat, fn)				\
+	__EARLYCON_DECLARE(_name, compat, fn, __earlycon_of_, EARLYCON_OF_TABLE)
+
 extern int setup_earlycon(char *buf);
 extern int of_setup_earlycon(unsigned long addr,
 			     int (*setup)(struct earlycon_device *, const char *));
 
-#define EARLYCON_DECLARE(_name, func)					\
-	static const struct earlycon_id __earlycon_##_name		\
-		__used __section(__earlycon_table)			\
-		 = { .name  = __stringify(_name),			\
-		     .setup = func  }
-
-#define OF_EARLYCON_DECLARE(name, compat, fn)				\
-	_OF_DECLARE(earlycon, name, compat, fn, void *)
-
 struct uart_port *uart_get_console(struct uart_port *ports, int nr,
 				   struct console *c);
 int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr,
-- 
2.3.5

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Rob Herring]

On Mon, Apr 6, 2015 at 12:36 PM, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org> wrote:
> On 04/02/2015 11:35 AM, Peter Hurley wrote:
>> On 04/02/2015 09:46 AM, Rob Herring wrote:
>>> Sorry about that. I had thought about doing the same thing. At least
>>> unifying the macros, but not necessarily the tables. If it is also
>>> extendable to other firmware interfaces like ACPI perhaps that would
>>> be good.
>>
>> No need to apologize; I'll make those changes and resubmit for your
>> review.
>
> What about something like the following?
>
> This patch makes both __earlycon_table and __earlycon_of_table
> arrays of struct earlycon_id, and a follow-on patch would use the
> earlycon name to initialize the struct console fields.
>
> The benefits of this approach are
> 1. diagnostics can readily identify the earlycon if there is some error
> 2. it would be trivial to enable both command line and devicetree
>    earlycon from the same earlycon declaration.
>
> And a single table is doable from this point.
>
> AFAICT there is no benefit to using actual OF tables, and I see no
> other reasonable way to initialize the name of the struct console
> for devicetree earlycons.

Just to align all the OF linker sections, but having a common table is
better. So the patch looks okay to me.

Rob

>
> Regards,
> Peter Hurley
>
>
> --- >% ---
> Subject: [PATCH] serial: earlycon: Use common framework for earlycon
>  declarations
>
> Use common table definition and implementation macro to declare an
> earlycon, but retain separate tables for command line and devicetree.
> Add __EARLYCON_DECLARE() macro to instance a unique earlycon
> declaration for the specified table.
>
> This enables all earlycons to properly initialize the earlycon console
> structure with name and index.
>
> Signed-off-by: Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
> ---
>  drivers/of/fdt.c                  |  6 +++---
>  drivers/tty/serial/earlycon.c     |  3 +--
>  include/asm-generic/vmlinux.lds.h |  8 ++++++--
>  include/linux/serial_core.h       | 30 +++++++++++++++++++++---------
>  4 files changed, 31 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 7cef9f9..f640efa 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -760,14 +760,14 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
>  #endif /* CONFIG_BLK_DEV_INITRD */
>
>  #ifdef CONFIG_SERIAL_EARLYCON
> -extern struct of_device_id __earlycon_of_table[];
> +extern struct earlycon_id __earlycon_of_table[];
>
>  static int __init early_init_dt_scan_chosen_serial(void)
>  {
>         int offset;
>         const char *p, *q;
>         int l;
> -       const struct of_device_id *match = __earlycon_of_table;
> +       const struct earlycon_id *match = __earlycon_of_table;
>         const void *fdt = initial_boot_params;
>
>         offset = fdt_path_offset(fdt, "/chosen");
> @@ -800,7 +800,7 @@ static int __init early_init_dt_scan_chosen_serial(void)
>                 if (!addr)
>                         return -ENXIO;
>
> -               of_setup_earlycon(addr, match->data);
> +               of_setup_earlycon(addr, match->setup);
>                 return 0;
>         }
>         return -ENODEV;
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index 5fdc9f3..bf7eb76 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -19,7 +19,6 @@
>  #include <linux/io.h>
>  #include <linux/serial_core.h>
>  #include <linux/sizes.h>
> -#include <linux/mod_devicetable.h>
>
>  #ifdef CONFIG_FIX_EARLYCON_MEM
>  #include <asm/fixmap.h>
> @@ -41,7 +40,7 @@ extern struct earlycon_id __earlycon_table[];
>  static const struct earlycon_id __earlycon_table_sentinel
>         __used __section(__earlycon_table_end);
>
> -static const struct of_device_id __earlycon_of_table_sentinel
> +static const struct earlycon_id __earlycon_of_table_sentinel
>         __used __section(__earlycon_of_table_end);
>
>  static void __iomem * __init earlycon_map(unsigned long paddr, size_t size)
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 561daf4..7322c30 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -155,8 +155,13 @@
>                          VMLINUX_SYMBOL(__earlycon_table) = .;  \
>                          *(__earlycon_table)                    \
>                          *(__earlycon_table_end)
> +#define EARLYCON_OF_TABLE()    STRUCT_ALIGN();                          \
> +                               VMLINUX_SYMBOL(__earlycon_of_table) = .; \
> +                               *(__earlycon_of_table)                   \
> +                               *(__earlycon_of_table_end)
>  #else
>  #define EARLYCON_TABLE()
> +#define EARLYCON_OF_TABLE()
>  #endif
>
>  #define ___OF_TABLE(cfg, name) _OF_TABLE_##cfg(name)
> @@ -175,7 +180,6 @@
>  #define IOMMU_OF_TABLES()      OF_TABLE(CONFIG_OF_IOMMU, iommu)
>  #define RESERVEDMEM_OF_TABLES()        OF_TABLE(CONFIG_OF_RESERVED_MEM, reservedmem)
>  #define CPU_METHOD_OF_TABLES() OF_TABLE(CONFIG_SMP, cpu_method)
> -#define EARLYCON_OF_TABLES()   OF_TABLE(CONFIG_SERIAL_EARLYCON, earlycon)
>
>  #define KERNEL_DTB()                                                   \
>         STRUCT_ALIGN();                                                 \
> @@ -512,7 +516,7 @@
>         KERNEL_DTB()                                                    \
>         IRQCHIP_OF_MATCH_TABLE()                                        \
>         EARLYCON_TABLE()                                                \
> -       EARLYCON_OF_TABLES()
> +       EARLYCON_OF_TABLE()
>
>  #define INIT_TEXT                                                      \
>         *(.init.text)                                                   \
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 025dad9..9455e97 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -341,22 +341,34 @@ struct earlycon_device {
>
>  struct earlycon_id {
>         char    name[16];
> +       char    compatible[128];
>         int     (*setup)(struct earlycon_device *, const char *options);
>  } __aligned(32);
>
> +#define EARLYCON_TABLE __used __section(__earlycon_table)
> +
> +#ifdef CONFIG_OF
> +#define EARLYCON_OF_TABLE __used __section(__earlycon_of_table)
> +#else
> +#define EARLYCON_OF_TABLE __attribute__((unused))
> +#endif
> +
> +#define __EARLYCON_DECLARE(_name, compat, fn, pre, section)            \
> +       static const struct earlycon_id __UNIQUE_ID(pre##_name) section \
> +                = { .name = __stringify(_name),                        \
> +                    .compatible = compat,                              \
> +                    .setup = fn  }
> +
> +#define EARLYCON_DECLARE(_name, fn)                                    \
> +       __EARLYCON_DECLARE(_name, "", fn, __earlycon_, EARLYCON_TABLE)
> +
> +#define OF_EARLYCON_DECLARE(_name, compat, fn)                         \
> +       __EARLYCON_DECLARE(_name, compat, fn, __earlycon_of_, EARLYCON_OF_TABLE)
> +
>  extern int setup_earlycon(char *buf);
>  extern int of_setup_earlycon(unsigned long addr,
>                              int (*setup)(struct earlycon_device *, const char *));
>
> -#define EARLYCON_DECLARE(_name, func)                                  \
> -       static const struct earlycon_id __earlycon_##_name              \
> -               __used __section(__earlycon_table)                      \
> -                = { .name  = __stringify(_name),                       \
> -                    .setup = func  }
> -
> -#define OF_EARLYCON_DECLARE(name, compat, fn)                          \
> -       _OF_DECLARE(earlycon, name, compat, fn, void *)
> -
>  struct uart_port *uart_get_console(struct uart_port *ports, int nr,
>                                    struct console *c);
>  int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr,
> --
> 2.3.5
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Rob Herring]

On Mon, Apr 6, 2015 at 12:36 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On 04/02/2015 11:35 AM, Peter Hurley wrote:
>> On 04/02/2015 09:46 AM, Rob Herring wrote:
>>> Sorry about that. I had thought about doing the same thing. At least
>>> unifying the macros, but not necessarily the tables. If it is also
>>> extendable to other firmware interfaces like ACPI perhaps that would
>>> be good.
>>
>> No need to apologize; I'll make those changes and resubmit for your
>> review.
>
> What about something like the following?
>
> This patch makes both __earlycon_table and __earlycon_of_table
> arrays of struct earlycon_id, and a follow-on patch would use the
> earlycon name to initialize the struct console fields.
>
> The benefits of this approach are
> 1. diagnostics can readily identify the earlycon if there is some error
> 2. it would be trivial to enable both command line and devicetree
>    earlycon from the same earlycon declaration.
>
> And a single table is doable from this point.
>
> AFAICT there is no benefit to using actual OF tables, and I see no
> other reasonable way to initialize the name of the struct console
> for devicetree earlycons.

Just to align all the OF linker sections, but having a common table is
better. So the patch looks okay to me.

Rob

>
> Regards,
> Peter Hurley
>
>
> --- >% ---
> Subject: [PATCH] serial: earlycon: Use common framework for earlycon
>  declarations
>
> Use common table definition and implementation macro to declare an
> earlycon, but retain separate tables for command line and devicetree.
> Add __EARLYCON_DECLARE() macro to instance a unique earlycon
> declaration for the specified table.
>
> This enables all earlycons to properly initialize the earlycon console
> structure with name and index.
>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
>  drivers/of/fdt.c                  |  6 +++---
>  drivers/tty/serial/earlycon.c     |  3 +--
>  include/asm-generic/vmlinux.lds.h |  8 ++++++--
>  include/linux/serial_core.h       | 30 +++++++++++++++++++++---------
>  4 files changed, 31 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 7cef9f9..f640efa 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -760,14 +760,14 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
>  #endif /* CONFIG_BLK_DEV_INITRD */
>
>  #ifdef CONFIG_SERIAL_EARLYCON
> -extern struct of_device_id __earlycon_of_table[];
> +extern struct earlycon_id __earlycon_of_table[];
>
>  static int __init early_init_dt_scan_chosen_serial(void)
>  {
>         int offset;
>         const char *p, *q;
>         int l;
> -       const struct of_device_id *match = __earlycon_of_table;
> +       const struct earlycon_id *match = __earlycon_of_table;
>         const void *fdt = initial_boot_params;
>
>         offset = fdt_path_offset(fdt, "/chosen");
> @@ -800,7 +800,7 @@ static int __init early_init_dt_scan_chosen_serial(void)
>                 if (!addr)
>                         return -ENXIO;
>
> -               of_setup_earlycon(addr, match->data);
> +               of_setup_earlycon(addr, match->setup);
>                 return 0;
>         }
>         return -ENODEV;
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index 5fdc9f3..bf7eb76 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -19,7 +19,6 @@
>  #include <linux/io.h>
>  #include <linux/serial_core.h>
>  #include <linux/sizes.h>
> -#include <linux/mod_devicetable.h>
>
>  #ifdef CONFIG_FIX_EARLYCON_MEM
>  #include <asm/fixmap.h>
> @@ -41,7 +40,7 @@ extern struct earlycon_id __earlycon_table[];
>  static const struct earlycon_id __earlycon_table_sentinel
>         __used __section(__earlycon_table_end);
>
> -static const struct of_device_id __earlycon_of_table_sentinel
> +static const struct earlycon_id __earlycon_of_table_sentinel
>         __used __section(__earlycon_of_table_end);
>
>  static void __iomem * __init earlycon_map(unsigned long paddr, size_t size)
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 561daf4..7322c30 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -155,8 +155,13 @@
>                          VMLINUX_SYMBOL(__earlycon_table) = .;  \
>                          *(__earlycon_table)                    \
>                          *(__earlycon_table_end)
> +#define EARLYCON_OF_TABLE()    STRUCT_ALIGN();                          \
> +                               VMLINUX_SYMBOL(__earlycon_of_table) = .; \
> +                               *(__earlycon_of_table)                   \
> +                               *(__earlycon_of_table_end)
>  #else
>  #define EARLYCON_TABLE()
> +#define EARLYCON_OF_TABLE()
>  #endif
>
>  #define ___OF_TABLE(cfg, name) _OF_TABLE_##cfg(name)
> @@ -175,7 +180,6 @@
>  #define IOMMU_OF_TABLES()      OF_TABLE(CONFIG_OF_IOMMU, iommu)
>  #define RESERVEDMEM_OF_TABLES()        OF_TABLE(CONFIG_OF_RESERVED_MEM, reservedmem)
>  #define CPU_METHOD_OF_TABLES() OF_TABLE(CONFIG_SMP, cpu_method)
> -#define EARLYCON_OF_TABLES()   OF_TABLE(CONFIG_SERIAL_EARLYCON, earlycon)
>
>  #define KERNEL_DTB()                                                   \
>         STRUCT_ALIGN();                                                 \
> @@ -512,7 +516,7 @@
>         KERNEL_DTB()                                                    \
>         IRQCHIP_OF_MATCH_TABLE()                                        \
>         EARLYCON_TABLE()                                                \
> -       EARLYCON_OF_TABLES()
> +       EARLYCON_OF_TABLE()
>
>  #define INIT_TEXT                                                      \
>         *(.init.text)                                                   \
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 025dad9..9455e97 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -341,22 +341,34 @@ struct earlycon_device {
>
>  struct earlycon_id {
>         char    name[16];
> +       char    compatible[128];
>         int     (*setup)(struct earlycon_device *, const char *options);
>  } __aligned(32);
>
> +#define EARLYCON_TABLE __used __section(__earlycon_table)
> +
> +#ifdef CONFIG_OF
> +#define EARLYCON_OF_TABLE __used __section(__earlycon_of_table)
> +#else
> +#define EARLYCON_OF_TABLE __attribute__((unused))
> +#endif
> +
> +#define __EARLYCON_DECLARE(_name, compat, fn, pre, section)            \
> +       static const struct earlycon_id __UNIQUE_ID(pre##_name) section \
> +                = { .name = __stringify(_name),                        \
> +                    .compatible = compat,                              \
> +                    .setup = fn  }
> +
> +#define EARLYCON_DECLARE(_name, fn)                                    \
> +       __EARLYCON_DECLARE(_name, "", fn, __earlycon_, EARLYCON_TABLE)
> +
> +#define OF_EARLYCON_DECLARE(_name, compat, fn)                         \
> +       __EARLYCON_DECLARE(_name, compat, fn, __earlycon_of_, EARLYCON_OF_TABLE)
> +
>  extern int setup_earlycon(char *buf);
>  extern int of_setup_earlycon(unsigned long addr,
>                              int (*setup)(struct earlycon_device *, const char *));
>
> -#define EARLYCON_DECLARE(_name, func)                                  \
> -       static const struct earlycon_id __earlycon_##_name              \
> -               __used __section(__earlycon_table)                      \
> -                = { .name  = __stringify(_name),                       \
> -                    .setup = func  }
> -
> -#define OF_EARLYCON_DECLARE(name, compat, fn)                          \
> -       _OF_DECLARE(earlycon, name, compat, fn, void *)
> -
>  struct uart_port *uart_get_console(struct uart_port *ports, int nr,
>                                    struct console *c);
>  int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr,
> --
> 2.3.5
>
>

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

Hi Kevin,

On 04/02/2015 11:36 AM, Peter Hurley wrote:
> On 03/28/2015 03:28 PM, Kevin Cernekee wrote:
>> Hi Peter,
>>
>> This is my latest work-in-progress, incorporating the feedback from
>> you and Grant:
>>
>> https://github.com/cernekee/linux/commits/endian
>>
>> Not sure if this code plays nice with your recent cleanups?  If we're
>> touching the same files/functions we should probably coordinate.
> 
> Ok, I'll look over your git tree and add whatever's required to
> earlycon.

Could you submit the first 5 patches from your 'endian' branch, so
that big-endian support can be added to earlycon?

Regards,
Peter Hurley

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

[Peter Hurley]

Hi Kevin,

On 04/02/2015 11:36 AM, Peter Hurley wrote:
> On 03/28/2015 03:28 PM, Kevin Cernekee wrote:
>> Hi Peter,
>>
>> This is my latest work-in-progress, incorporating the feedback from
>> you and Grant:
>>
>> https://github.com/cernekee/linux/commits/endian
>>
>> Not sure if this code plays nice with your recent cleanups?  If we're
>> touching the same files/functions we should probably coordinate.
> 
> Ok, I'll look over your git tree and add whatever's required to
> earlycon.

Could you submit the first 5 patches from your 'endian' branch, so
that big-endian support can be added to earlycon?

Regards,
Peter Hurley