[PATCH 0/5] Add OF support to the sh-sci serial port driver

[PATCH 0/5] Add OF support to the sh-sci serial port driver

[Laurent Pinchart]

Hello,

This patch set adds device tree bindings for the sh sci serial port devices
and adds OF parsing to the sh-sci driver.

The bindings are based on Bastian Hecht's proposal (see
http://www.spinics.net/lists/arm-kernel/msg228129.html). The approach taken
here is more minimalistic: instead of describing all hardware characteristics
that vary between the SCI device revisions in DT (such as registers layout),
that information is stored in the driver and selected based on the compatible
property value. Only SCI revisions used on ARM devices are supported through
DT, as DT support for SuperH is nowhere down the line.

The first three patches clean up the sh-sci driver. They're not strictly
required for OF support but make sense nonetheless. The last two patches add
OF parsing to the sh-sci driver and create DT bindings documentation.

The patches have been test on a Lager board (r8a7790-based). Support for other
SoCs will be added as needed. Note that all current Renesas ARM SoCs seem to
be compatible with the generic (H)SCI(F)(AB) devices, but the plan is for
their DT bindings to list the SoC-specific version in case incompatibilities
are found later.

Cc: devicetree@vger.kernel.org

Bastian Hecht (1):
  serial: sh-sci: Add OF support

Laurent Pinchart (4):
  serial: sh-sci: Sort headers alphabetically
  serial: sh-sci: Remove baud rate calculation algorithm 5
  serial: sh-sci: Simplify baud rate calculation algorithms
  serial: sh-sci: Add device tree bindings documentation

 .../bindings/serial/renesas,sci-serial.txt         |  51 ++++++
 drivers/tty/serial/sh-sci.c                        | 192 +++++++++++++++++----
 include/linux/serial_sci.h                         |   9 +-
 3 files changed, 217 insertions(+), 35 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/serial/renesas,sci-serial.txt

-- 
Regards,

Laurent Pinchart

[PATCH 0/5] Add OF support to the sh-sci serial port driver

[Laurent Pinchart]

Hello,

This patch set adds device tree bindings for the sh sci serial port devices
and adds OF parsing to the sh-sci driver.

The bindings are based on Bastian Hecht's proposal (see
http://www.spinics.net/lists/arm-kernel/msg228129.html). The approach taken
here is more minimalistic: instead of describing all hardware characteristics
that vary between the SCI device revisions in DT (such as registers layout),
that information is stored in the driver and selected based on the compatible
property value. Only SCI revisions used on ARM devices are supported through
DT, as DT support for SuperH is nowhere down the line.

The first three patches clean up the sh-sci driver. They're not strictly
required for OF support but make sense nonetheless. The last two patches add
OF parsing to the sh-sci driver and create DT bindings documentation.

The patches have been test on a Lager board (r8a7790-based). Support for other
SoCs will be added as needed. Note that all current Renesas ARM SoCs seem to
be compatible with the generic (H)SCI(F)(AB) devices, but the plan is for
their DT bindings to list the SoC-specific version in case incompatibilities
are found later.

Cc: devicetree@vger.kernel.org

Bastian Hecht (1):
  serial: sh-sci: Add OF support

Laurent Pinchart (4):
  serial: sh-sci: Sort headers alphabetically
  serial: sh-sci: Remove baud rate calculation algorithm 5
  serial: sh-sci: Simplify baud rate calculation algorithms
  serial: sh-sci: Add device tree bindings documentation

 .../bindings/serial/renesas,sci-serial.txt         |  51 ++++++
 drivers/tty/serial/sh-sci.c                        | 192 +++++++++++++++++----
 include/linux/serial_sci.h                         |   9 +-
 3 files changed, 217 insertions(+), 35 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/serial/renesas,sci-serial.txt

-- 
Regards,

Laurent Pinchart

[PATCH 1/5] serial: sh-sci: Sort headers alphabetically

[Laurent Pinchart]

This helps locating duplicates.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/tty/serial/sh-sci.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 5377502..280753f 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -23,35 +23,35 @@
 
 #undef DEBUG
 
-#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/console.h>
+#include <linux/ctype.h>
+#include <linux/cpufreq.h>
+#include <linux/delay.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
 #include <linux/errno.h>
-#include <linux/sh_dma.h>
-#include <linux/timer.h>
+#include <linux/gpio.h>
+#include <linux/init.h>
 #include <linux/interrupt.h>
-#include <linux/tty.h>
-#include <linux/tty_flip.h>
-#include <linux/serial.h>
-#include <linux/major.h>
-#include <linux/string.h>
-#include <linux/sysrq.h>
 #include <linux/ioport.h>
+#include <linux/major.h>
+#include <linux/module.h>
 #include <linux/mm.h>
-#include <linux/init.h>
-#include <linux/delay.h>
-#include <linux/console.h>
-#include <linux/platform_device.h>
-#include <linux/serial_sci.h>
 #include <linux/notifier.h>
+#include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
-#include <linux/cpufreq.h>
-#include <linux/clk.h>
-#include <linux/ctype.h>
-#include <linux/err.h>
-#include <linux/dmaengine.h>
-#include <linux/dma-mapping.h>
 #include <linux/scatterlist.h>
+#include <linux/serial.h>
+#include <linux/serial_sci.h>
+#include <linux/sh_dma.h>
 #include <linux/slab.h>
-#include <linux/gpio.h>
+#include <linux/string.h>
+#include <linux/sysrq.h>
+#include <linux/timer.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
 
 #ifdef CONFIG_SUPERH
 #include <asm/sh_bios.h>
-- 
1.8.1.5

[PATCH 1/5] serial: sh-sci: Sort headers alphabetically

[Laurent Pinchart]

This helps locating duplicates.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/tty/serial/sh-sci.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 5377502..280753f 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -23,35 +23,35 @@
 
 #undef DEBUG
 
-#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/console.h>
+#include <linux/ctype.h>
+#include <linux/cpufreq.h>
+#include <linux/delay.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
 #include <linux/errno.h>
-#include <linux/sh_dma.h>
-#include <linux/timer.h>
+#include <linux/gpio.h>
+#include <linux/init.h>
 #include <linux/interrupt.h>
-#include <linux/tty.h>
-#include <linux/tty_flip.h>
-#include <linux/serial.h>
-#include <linux/major.h>
-#include <linux/string.h>
-#include <linux/sysrq.h>
 #include <linux/ioport.h>
+#include <linux/major.h>
+#include <linux/module.h>
 #include <linux/mm.h>
-#include <linux/init.h>
-#include <linux/delay.h>
-#include <linux/console.h>
-#include <linux/platform_device.h>
-#include <linux/serial_sci.h>
 #include <linux/notifier.h>
+#include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
-#include <linux/cpufreq.h>
-#include <linux/clk.h>
-#include <linux/ctype.h>
-#include <linux/err.h>
-#include <linux/dmaengine.h>
-#include <linux/dma-mapping.h>
 #include <linux/scatterlist.h>
+#include <linux/serial.h>
+#include <linux/serial_sci.h>
+#include <linux/sh_dma.h>
 #include <linux/slab.h>
-#include <linux/gpio.h>
+#include <linux/string.h>
+#include <linux/sysrq.h>
+#include <linux/timer.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
 
 #ifdef CONFIG_SUPERH
 #include <asm/sh_bios.h>
-- 
1.8.1.5

[PATCH 2/5] serial: sh-sci: Remove baud rate calculation algorithm 5

[Laurent Pinchart]

The algorithm isn't used, remove it.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/tty/serial/sh-sci.c | 2 --
 include/linux/serial_sci.h  | 1 -
 2 files changed, 3 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 280753f..2fc1a0e 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1820,8 +1820,6 @@ static unsigned int sci_scbrr_calc(unsigned int algo_id, unsigned int bps,
 		return (((freq * 2) + 16 * bps) / (16 * bps) - 1);
 	case SCBRR_ALGO_4:
 		return (((freq * 2) + 16 * bps) / (32 * bps) - 1);
-	case SCBRR_ALGO_5:
-		return (((freq * 1000 / 32) / bps) - 1);
 	}
 
 	/* Warn, but use a safe default */
diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
index d340497..803068d 100644
--- a/include/linux/serial_sci.h
+++ b/include/linux/serial_sci.h
@@ -15,7 +15,6 @@ enum {
 	SCBRR_ALGO_2,		/* ((clk + 16 * bps) / (32 * bps) - 1) */
 	SCBRR_ALGO_3,		/* (((clk * 2) + 16 * bps) / (16 * bps) - 1) */
 	SCBRR_ALGO_4,		/* (((clk * 2) + 16 * bps) / (32 * bps) - 1) */
-	SCBRR_ALGO_5,		/* (((clk * 1000 / 32) / bps) - 1) */
 	SCBRR_ALGO_6,		/* HSCIF variable sample rate algorithm */
 };
 
-- 
1.8.1.5

[PATCH 2/5] serial: sh-sci: Remove baud rate calculation algorithm 5

[Laurent Pinchart]

The algorithm isn't used, remove it.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/tty/serial/sh-sci.c | 2 --
 include/linux/serial_sci.h  | 1 -
 2 files changed, 3 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 280753f..2fc1a0e 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1820,8 +1820,6 @@ static unsigned int sci_scbrr_calc(unsigned int algo_id, unsigned int bps,
 		return (((freq * 2) + 16 * bps) / (16 * bps) - 1);
 	case SCBRR_ALGO_4:
 		return (((freq * 2) + 16 * bps) / (32 * bps) - 1);
-	case SCBRR_ALGO_5:
-		return (((freq * 1000 / 32) / bps) - 1);
 	}
 
 	/* Warn, but use a safe default */
diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
index d340497..803068d 100644
--- a/include/linux/serial_sci.h
+++ b/include/linux/serial_sci.h
@@ -15,7 +15,6 @@ enum {
 	SCBRR_ALGO_2,		/* ((clk + 16 * bps) / (32 * bps) - 1) */
 	SCBRR_ALGO_3,		/* (((clk * 2) + 16 * bps) / (16 * bps) - 1) */
 	SCBRR_ALGO_4,		/* (((clk * 2) + 16 * bps) / (32 * bps) - 1) */
-	SCBRR_ALGO_5,		/* (((clk * 1000 / 32) / bps) - 1) */
 	SCBRR_ALGO_6,		/* HSCIF variable sample rate algorithm */
 };
 
-- 
1.8.1.5

[PATCH 3/5] serial: sh-sci: Simplify baud rate calculation algorithms

[Laurent Pinchart]

Rewrite the baud rate register value calculations in easier to read
forms. The computed value isn't modified.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/tty/serial/sh-sci.c | 8 ++++----
 include/linux/serial_sci.h  | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 2fc1a0e..91ce9214b 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1813,13 +1813,13 @@ static unsigned int sci_scbrr_calc(unsigned int algo_id, unsigned int bps,
 {
 	switch (algo_id) {
 	case SCBRR_ALGO_1:
-		return ((freq + 16 * bps) / (16 * bps) - 1);
+		return freq / (16 * bps);
 	case SCBRR_ALGO_2:
-		return ((freq + 16 * bps) / (32 * bps) - 1);
+		return DIV_ROUND_CLOSEST(freq, 32 * bps) - 1;
 	case SCBRR_ALGO_3:
-		return (((freq * 2) + 16 * bps) / (16 * bps) - 1);
+		return freq / (8 * bps);
 	case SCBRR_ALGO_4:
-		return (((freq * 2) + 16 * bps) / (32 * bps) - 1);
+		return DIV_ROUND_CLOSEST(freq, 16 * bps) - 1;
 	}
 
 	/* Warn, but use a safe default */
diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
index 803068d..eb787d4 100644
--- a/include/linux/serial_sci.h
+++ b/include/linux/serial_sci.h
@@ -11,10 +11,10 @@
 #define SCIx_NOT_SUPPORTED	(-1)
 
 enum {
-	SCBRR_ALGO_1,		/* ((clk + 16 * bps) / (16 * bps) - 1) */
-	SCBRR_ALGO_2,		/* ((clk + 16 * bps) / (32 * bps) - 1) */
-	SCBRR_ALGO_3,		/* (((clk * 2) + 16 * bps) / (16 * bps) - 1) */
-	SCBRR_ALGO_4,		/* (((clk * 2) + 16 * bps) / (32 * bps) - 1) */
+	SCBRR_ALGO_1,		/* clk / (16 * bps) */
+	SCBRR_ALGO_2,		/* DIV_ROUND_CLOSEST(clk, 32 * bps) - 1 */
+	SCBRR_ALGO_3,		/* clk / (8 * bps) */
+	SCBRR_ALGO_4,		/* DIV_ROUND_CLOSEST(clk, 16 * bps) - 1 */
 	SCBRR_ALGO_6,		/* HSCIF variable sample rate algorithm */
 };
 
-- 
1.8.1.5

[PATCH 3/5] serial: sh-sci: Simplify baud rate calculation algorithms

[Laurent Pinchart]

Rewrite the baud rate register value calculations in easier to read
forms. The computed value isn't modified.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/tty/serial/sh-sci.c | 8 ++++----
 include/linux/serial_sci.h  | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 2fc1a0e..91ce9214b 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1813,13 +1813,13 @@ static unsigned int sci_scbrr_calc(unsigned int algo_id, unsigned int bps,
 {
 	switch (algo_id) {
 	case SCBRR_ALGO_1:
-		return ((freq + 16 * bps) / (16 * bps) - 1);
+		return freq / (16 * bps);
 	case SCBRR_ALGO_2:
-		return ((freq + 16 * bps) / (32 * bps) - 1);
+		return DIV_ROUND_CLOSEST(freq, 32 * bps) - 1;
 	case SCBRR_ALGO_3:
-		return (((freq * 2) + 16 * bps) / (16 * bps) - 1);
+		return freq / (8 * bps);
 	case SCBRR_ALGO_4:
-		return (((freq * 2) + 16 * bps) / (32 * bps) - 1);
+		return DIV_ROUND_CLOSEST(freq, 16 * bps) - 1;
 	}
 
 	/* Warn, but use a safe default */
diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
index 803068d..eb787d4 100644
--- a/include/linux/serial_sci.h
+++ b/include/linux/serial_sci.h
@@ -11,10 +11,10 @@
 #define SCIx_NOT_SUPPORTED	(-1)
 
 enum {
-	SCBRR_ALGO_1,		/* ((clk + 16 * bps) / (16 * bps) - 1) */
-	SCBRR_ALGO_2,		/* ((clk + 16 * bps) / (32 * bps) - 1) */
-	SCBRR_ALGO_3,		/* (((clk * 2) + 16 * bps) / (16 * bps) - 1) */
-	SCBRR_ALGO_4,		/* (((clk * 2) + 16 * bps) / (32 * bps) - 1) */
+	SCBRR_ALGO_1,		/* clk / (16 * bps) */
+	SCBRR_ALGO_2,		/* DIV_ROUND_CLOSEST(clk, 32 * bps) - 1 */
+	SCBRR_ALGO_3,		/* clk / (8 * bps) */
+	SCBRR_ALGO_4,		/* DIV_ROUND_CLOSEST(clk, 16 * bps) - 1 */
 	SCBRR_ALGO_6,		/* HSCIF variable sample rate algorithm */
 };
 
-- 
1.8.1.5

[PATCH 4/5] serial: sh-sci: Add OF support

[Laurent Pinchart]

From: Bastian Hecht <hechtb@gmail.com>

Extend the driver to with support for SCIx device tree bindings. A
minimal set of features is supported, additional properties can be added
later should the need to describe more device features arise.

Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/tty/serial/sh-sci.c | 140 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 137 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 91ce9214b..2ae4f1a 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -40,6 +40,7 @@
 #include <linux/module.h>
 #include <linux/mm.h>
 #include <linux/notifier.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/scatterlist.h>
@@ -2435,6 +2436,122 @@ static int sci_remove(struct platform_device *dev)
 	return 0;
 }
 
+struct sci_port_info {
+	unsigned int type;
+	unsigned int regtype;
+	unsigned int brr_algo;
+};
+
+static const struct of_device_id of_sci_match[] = {
+	{
+		.compatible = "renesas,scif-generic",
+		.data = (void *)&(const struct sci_port_info) {
+			.type = PORT_SCIF,
+			.regtype = SCIx_SH4_SCIF_REGTYPE,
+			.brr_algo = SCBRR_ALGO_2,
+		},
+	}, {
+		.compatible = "renesas,scifa-generic",
+		.data = (void *)&(const struct sci_port_info) {
+			.type = PORT_SCIFA,
+			.regtype = SCIx_SCIFA_REGTYPE,
+			.brr_algo = SCBRR_ALGO_4,
+		},
+	}, {
+		.compatible = "renesas,scifb-generic",
+		.data = (void *)&(const struct sci_port_info) {
+			.type = PORT_SCIFB,
+			.regtype = SCIx_SCIFB_REGTYPE,
+			.brr_algo = SCBRR_ALGO_4,
+		},
+	}, {
+		.compatible = "renesas,hscif-generic",
+		.data = (void *)&(const struct sci_port_info) {
+			.type = PORT_HSCIF,
+			.regtype = SCIx_HSCIF_REGTYPE,
+			.brr_algo = SCBRR_ALGO_6,
+		},
+	}, {
+		/* Terminator */
+	},
+};
+MODULE_DEVICE_TABLE(of, of_sci_match);
+
+static struct plat_sci_port *
+sci_parse_dt(struct platform_device *pdev, unsigned int *dev_id)
+{
+	static const char * const irq_names[] = { "eri", "rxi", "txi", "bri" };
+	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *match;
+	const struct sci_port_info *info;
+	struct plat_sci_port *p;
+	struct resource *res;
+	unsigned int i;
+	int irq;
+	int id;
+
+	if (!IS_ENABLED(CONFIG_OF) || !np)
+		return NULL;
+
+	match = of_match_node(of_sci_match, pdev->dev.of_node);
+	if (!match)
+		return NULL;
+
+	info = match->data;
+
+	p = devm_kzalloc(&pdev->dev, sizeof(struct plat_sci_port), GFP_KERNEL);
+	if (!p) {
+		dev_err(&pdev->dev, "failed to allocate DT config data\n");
+		return NULL;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "failed to get I/O memory\n");
+		return NULL;
+	}
+	p->mapbase = res->start;
+
+	/* SCIx devices use either a single unnamed IRQ or multiple named
+	 * IRQs. Try to get the named IRQs first.
+	 */
+	for (i = 0; i < SCIx_NR_IRQS; i++) {
+		irq = platform_get_irq_byname(pdev, irq_names[i]);
+		if (irq < 0)
+			break;
+
+		p->irqs[i] = irq;
+	}
+
+	if (i = 0) {
+		irq = platform_get_irq(pdev, 0);
+		if (irq < 0) {
+			dev_err(&pdev->dev, "no IRQ found\n");
+			return NULL;
+		}
+
+		for (i = 0; i < SCIx_NR_IRQS; i++)
+			p->irqs[i] = irq;
+	}
+
+	/* Get the line number for the aliases node. */
+	id = of_alias_get_id(np, "serial");
+	if (id < 0) {
+		dev_err(&pdev->dev, "failed to get alias id (%d)\n", id);
+		return NULL;
+	}
+
+	*dev_id = id;
+
+	p->flags = UPF_IOREMAP | UPF_BOOT_AUTOCONF;
+	p->type = info->type;
+	p->regtype = info->regtype;
+	p->scscr = SCSCR_RE | SCSCR_TE;
+	p->scbrr_algo_id = info->brr_algo;
+
+	return p;
+}
+
 static int sci_probe_single(struct platform_device *dev,
 				      unsigned int index,
 				      struct plat_sci_port *p,
@@ -2467,8 +2584,9 @@ static int sci_probe_single(struct platform_device *dev,
 
 static int sci_probe(struct platform_device *dev)
 {
-	struct plat_sci_port *p = dev_get_platdata(&dev->dev);
-	struct sci_port *sp = &sci_ports[dev->id];
+	struct plat_sci_port *p;
+	struct sci_port *sp;
+	unsigned int dev_id;
 	int ret;
 
 	/*
@@ -2479,9 +2597,24 @@ static int sci_probe(struct platform_device *dev)
 	if (is_early_platform_device(dev))
 		return sci_probe_earlyprintk(dev);
 
+	if (dev->dev.of_node) {
+		p = sci_parse_dt(dev, &dev_id);
+		if (p = NULL)
+			return -EINVAL;
+	} else {
+		p = dev->dev.platform_data;
+		if (p = NULL) {
+			dev_err(&dev->dev, "no platform data supplied\n");
+			return -EINVAL;
+		}
+
+		dev_id = dev->id;
+	}
+
+	sp = &sci_ports[dev_id];
 	platform_set_drvdata(dev, sp);
 
-	ret = sci_probe_single(dev, dev->id, p, sp);
+	ret = sci_probe_single(dev, dev_id, p, sp);
 	if (ret)
 		return ret;
 
@@ -2533,6 +2666,7 @@ static struct platform_driver sci_driver = {
 		.name	= "sh-sci",
 		.owner	= THIS_MODULE,
 		.pm	= &sci_dev_pm_ops,
+		.of_match_table = of_match_ptr(of_sci_match),
 	},
 };
 
-- 
1.8.1.5

[PATCH 4/5] serial: sh-sci: Add OF support

[Laurent Pinchart]

From: Bastian Hecht <hechtb@gmail.com>

Extend the driver to with support for SCIx device tree bindings. A
minimal set of features is supported, additional properties can be added
later should the need to describe more device features arise.

Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/tty/serial/sh-sci.c | 140 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 137 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 91ce9214b..2ae4f1a 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -40,6 +40,7 @@
 #include <linux/module.h>
 #include <linux/mm.h>
 #include <linux/notifier.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/scatterlist.h>
@@ -2435,6 +2436,122 @@ static int sci_remove(struct platform_device *dev)
 	return 0;
 }
 
+struct sci_port_info {
+	unsigned int type;
+	unsigned int regtype;
+	unsigned int brr_algo;
+};
+
+static const struct of_device_id of_sci_match[] = {
+	{
+		.compatible = "renesas,scif-generic",
+		.data = (void *)&(const struct sci_port_info) {
+			.type = PORT_SCIF,
+			.regtype = SCIx_SH4_SCIF_REGTYPE,
+			.brr_algo = SCBRR_ALGO_2,
+		},
+	}, {
+		.compatible = "renesas,scifa-generic",
+		.data = (void *)&(const struct sci_port_info) {
+			.type = PORT_SCIFA,
+			.regtype = SCIx_SCIFA_REGTYPE,
+			.brr_algo = SCBRR_ALGO_4,
+		},
+	}, {
+		.compatible = "renesas,scifb-generic",
+		.data = (void *)&(const struct sci_port_info) {
+			.type = PORT_SCIFB,
+			.regtype = SCIx_SCIFB_REGTYPE,
+			.brr_algo = SCBRR_ALGO_4,
+		},
+	}, {
+		.compatible = "renesas,hscif-generic",
+		.data = (void *)&(const struct sci_port_info) {
+			.type = PORT_HSCIF,
+			.regtype = SCIx_HSCIF_REGTYPE,
+			.brr_algo = SCBRR_ALGO_6,
+		},
+	}, {
+		/* Terminator */
+	},
+};
+MODULE_DEVICE_TABLE(of, of_sci_match);
+
+static struct plat_sci_port *
+sci_parse_dt(struct platform_device *pdev, unsigned int *dev_id)
+{
+	static const char * const irq_names[] = { "eri", "rxi", "txi", "bri" };
+	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *match;
+	const struct sci_port_info *info;
+	struct plat_sci_port *p;
+	struct resource *res;
+	unsigned int i;
+	int irq;
+	int id;
+
+	if (!IS_ENABLED(CONFIG_OF) || !np)
+		return NULL;
+
+	match = of_match_node(of_sci_match, pdev->dev.of_node);
+	if (!match)
+		return NULL;
+
+	info = match->data;
+
+	p = devm_kzalloc(&pdev->dev, sizeof(struct plat_sci_port), GFP_KERNEL);
+	if (!p) {
+		dev_err(&pdev->dev, "failed to allocate DT config data\n");
+		return NULL;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "failed to get I/O memory\n");
+		return NULL;
+	}
+	p->mapbase = res->start;
+
+	/* SCIx devices use either a single unnamed IRQ or multiple named
+	 * IRQs. Try to get the named IRQs first.
+	 */
+	for (i = 0; i < SCIx_NR_IRQS; i++) {
+		irq = platform_get_irq_byname(pdev, irq_names[i]);
+		if (irq < 0)
+			break;
+
+		p->irqs[i] = irq;
+	}
+
+	if (i == 0) {
+		irq = platform_get_irq(pdev, 0);
+		if (irq < 0) {
+			dev_err(&pdev->dev, "no IRQ found\n");
+			return NULL;
+		}
+
+		for (i = 0; i < SCIx_NR_IRQS; i++)
+			p->irqs[i] = irq;
+	}
+
+	/* Get the line number for the aliases node. */
+	id = of_alias_get_id(np, "serial");
+	if (id < 0) {
+		dev_err(&pdev->dev, "failed to get alias id (%d)\n", id);
+		return NULL;
+	}
+
+	*dev_id = id;
+
+	p->flags = UPF_IOREMAP | UPF_BOOT_AUTOCONF;
+	p->type = info->type;
+	p->regtype = info->regtype;
+	p->scscr = SCSCR_RE | SCSCR_TE;
+	p->scbrr_algo_id = info->brr_algo;
+
+	return p;
+}
+
 static int sci_probe_single(struct platform_device *dev,
 				      unsigned int index,
 				      struct plat_sci_port *p,
@@ -2467,8 +2584,9 @@ static int sci_probe_single(struct platform_device *dev,
 
 static int sci_probe(struct platform_device *dev)
 {
-	struct plat_sci_port *p = dev_get_platdata(&dev->dev);
-	struct sci_port *sp = &sci_ports[dev->id];
+	struct plat_sci_port *p;
+	struct sci_port *sp;
+	unsigned int dev_id;
 	int ret;
 
 	/*
@@ -2479,9 +2597,24 @@ static int sci_probe(struct platform_device *dev)
 	if (is_early_platform_device(dev))
 		return sci_probe_earlyprintk(dev);
 
+	if (dev->dev.of_node) {
+		p = sci_parse_dt(dev, &dev_id);
+		if (p == NULL)
+			return -EINVAL;
+	} else {
+		p = dev->dev.platform_data;
+		if (p == NULL) {
+			dev_err(&dev->dev, "no platform data supplied\n");
+			return -EINVAL;
+		}
+
+		dev_id = dev->id;
+	}
+
+	sp = &sci_ports[dev_id];
 	platform_set_drvdata(dev, sp);
 
-	ret = sci_probe_single(dev, dev->id, p, sp);
+	ret = sci_probe_single(dev, dev_id, p, sp);
 	if (ret)
 		return ret;
 
@@ -2533,6 +2666,7 @@ static struct platform_driver sci_driver = {
 		.name	= "sh-sci",
 		.owner	= THIS_MODULE,
 		.pm	= &sci_dev_pm_ops,
+		.of_match_table = of_match_ptr(of_sci_match),
 	},
 };
 
-- 
1.8.1.5

[PATCH 5/5] serial: sh-sci: Add device tree bindings documentation

[Laurent Pinchart]

Document the device tree bindings for the sci serial port devices.

Cc: devicetree@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../bindings/serial/renesas,sci-serial.txt         | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/renesas,sci-serial.txt

diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
new file mode 100644
index 0000000..5658b4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
@@ -0,0 +1,51 @@
+* Renesas SH-Mobile Serial Communication Interface
+
+Required properties:
+
+  - compatible: should be one of the following.
+
+    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
+    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible UART.
+    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB compatible UART.
+    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2) HSCIF compatible UART.
+    - "renesas,scif-generic" for generic SCIF compatible UART.
+    - "renesas,scifa-generic" for generic SCIFA compatible UART.
+    - "renesas,scifb-generic" for generic SCIFB compatible UART.
+    - "renesas,hscif-generic" for generic HSCIF compatible UART.
+
+    When compatible with the generic version, nodes must also list the
+    SoC-specific version corresponding to the platform.
+
+  - reg: Base address and length of the memory resource used by the UART.
+
+  - interrupt-parent: Reference to the parent interrupt controller.
+  - interrupts: Interrupt number(s). Depending on the SoC SCIx UARTs are tied
+    to one or multiple interrupt lines. When using multiple interrupt lines,
+    specify the interrupt names as described below.
+
+  - clocks: Reference to the SCIx UART interface clock.
+  - clock-names: Should be "sci_ick".
+
+Optional properties:
+
+  - interrupt-names: When using multiple interrupts, report the interrupt
+    names as "eri" (receive error), "rxi" (receive), "txi" (transmit) and
+    "bri" (break). When using a single interrupt this property should not be
+    present.
+
+Note: Each enabled SCIx UART should have an alias correctly numbered in the
+"aliases" node.
+
+Example:
+	aliases {
+		serial0 = &scifa0;
+	};
+
+	scifa0: serial@e6c40000 {
+		compatible = "renesas,scifa-r8a7790", "renesas,scifa-generic";
+		reg = <0 0xe6c40000 0 64>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 144 4>;
+		clocks = <&mstp2_clks 4>;
+		clock-names = "sci_ick";
+	};
-- 
1.8.1.5

[PATCH 5/5] serial: sh-sci: Add device tree bindings documentation

[Laurent Pinchart]

Document the device tree bindings for the sci serial port devices.

Cc: devicetree@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../bindings/serial/renesas,sci-serial.txt         | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/renesas,sci-serial.txt

diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
new file mode 100644
index 0000000..5658b4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
@@ -0,0 +1,51 @@
+* Renesas SH-Mobile Serial Communication Interface
+
+Required properties:
+
+  - compatible: should be one of the following.
+
+    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
+    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible UART.
+    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB compatible UART.
+    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2) HSCIF compatible UART.
+    - "renesas,scif-generic" for generic SCIF compatible UART.
+    - "renesas,scifa-generic" for generic SCIFA compatible UART.
+    - "renesas,scifb-generic" for generic SCIFB compatible UART.
+    - "renesas,hscif-generic" for generic HSCIF compatible UART.
+
+    When compatible with the generic version, nodes must also list the
+    SoC-specific version corresponding to the platform.
+
+  - reg: Base address and length of the memory resource used by the UART.
+
+  - interrupt-parent: Reference to the parent interrupt controller.
+  - interrupts: Interrupt number(s). Depending on the SoC SCIx UARTs are tied
+    to one or multiple interrupt lines. When using multiple interrupt lines,
+    specify the interrupt names as described below.
+
+  - clocks: Reference to the SCIx UART interface clock.
+  - clock-names: Should be "sci_ick".
+
+Optional properties:
+
+  - interrupt-names: When using multiple interrupts, report the interrupt
+    names as "eri" (receive error), "rxi" (receive), "txi" (transmit) and
+    "bri" (break). When using a single interrupt this property should not be
+    present.
+
+Note: Each enabled SCIx UART should have an alias correctly numbered in the
+"aliases" node.
+
+Example:
+	aliases {
+		serial0 = &scifa0;
+	};
+
+	scifa0: serial@e6c40000 {
+		compatible = "renesas,scifa-r8a7790", "renesas,scifa-generic";
+		reg = <0 0xe6c40000 0 64>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 144 4>;
+		clocks = <&mstp2_clks 4>;
+		clock-names = "sci_ick";
+	};
-- 
1.8.1.5

Re: [PATCH 3/5] serial: sh-sci: Simplify baud rate calculation algorithms

[Laurent Pinchart]

Hi Paul,

On Tuesday 29 October 2013 11:29:57 Laurent Pinchart wrote:
> Rewrite the baud rate register value calculations in easier to read
> forms. The computed value isn't modified.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/tty/serial/sh-sci.c | 8 ++++----
>  include/linux/serial_sci.h  | 8 ++++----
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 2fc1a0e..91ce9214b 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1813,13 +1813,13 @@ static unsigned int sci_scbrr_calc(unsigned int
> algo_id, unsigned int bps, {
>  	switch (algo_id) {
>  	case SCBRR_ALGO_1:
> -		return ((freq + 16 * bps) / (16 * bps) - 1);
> +		return freq / (16 * bps);
>  	case SCBRR_ALGO_2:
> -		return ((freq + 16 * bps) / (32 * bps) - 1);
> +		return DIV_ROUND_CLOSEST(freq, 32 * bps) - 1;
>  	case SCBRR_ALGO_3:
> -		return (((freq * 2) + 16 * bps) / (16 * bps) - 1);
> +		return freq / (8 * bps);
>  	case SCBRR_ALGO_4:
> -		return (((freq * 2) + 16 * bps) / (32 * bps) - 1);
> +		return DIV_ROUND_CLOSEST(freq, 16 * bps) - 1;
>  	}
> 
>  	/* Warn, but use a safe default */
> diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
> index 803068d..eb787d4 100644
> --- a/include/linux/serial_sci.h
> +++ b/include/linux/serial_sci.h
> @@ -11,10 +11,10 @@
>  #define SCIx_NOT_SUPPORTED	(-1)
> 
>  enum {
> -	SCBRR_ALGO_1,		/* ((clk + 16 * bps) / (16 * bps) - 1) */
> -	SCBRR_ALGO_2,		/* ((clk + 16 * bps) / (32 * bps) - 1) */
> -	SCBRR_ALGO_3,		/* (((clk * 2) + 16 * bps) / (16 * bps) - 1) */
> -	SCBRR_ALGO_4,		/* (((clk * 2) + 16 * bps) / (32 * bps) - 1) */
> +	SCBRR_ALGO_1,		/* clk / (16 * bps) */
> +	SCBRR_ALGO_2,		/* DIV_ROUND_CLOSEST(clk, 32 * bps) - 1 */
> +	SCBRR_ALGO_3,		/* clk / (8 * bps) */
> +	SCBRR_ALGO_4,		/* DIV_ROUND_CLOSEST(clk, 16 * bps) - 1 */
>  	SCBRR_ALGO_6,		/* HSCIF variable sample rate algorithm */
>  };

While touching this, I'd like to use it as an opportunity to clean things up. 
It looks to me like algorithms 1 and 3 should use DIV_ROUND_CLOSEST() - 1 
instead of rounding the value up as they currently do. I've checked the 
documentation for various SuperH SoCs and they all confirm my hypothesis. As I 
don't have access to any SuperH board I can't test this though.

Given that you have touched the baudrate calculation code in the past, could 
you give me an insight on why algorithms 1 and 3 are implemented without 
rounding to the closest value and without the - 1 ?

On the same subject, algorithm 3 is only used on sh7723 and sh7724 for the 
SCIFA. I've only managed to find a user's manual for the sh7724 (the publicly 
available sh7723 documentation just lists the IP cores and their features), 
and the SCIFA documentation seems to imply that, when using the default 1/16 
sampling rate, algorithm 4 should be used. Could you enlighten me on why the 
sh-sci driver assume a sampling rate of 1/8 instead of 1/16 for baudrate 
calculation for SCIFA on sh772[34] ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/5] serial: sh-sci: Simplify baud rate calculation algorithms

[Laurent Pinchart]

Hi Paul,

On Tuesday 29 October 2013 11:29:57 Laurent Pinchart wrote:
> Rewrite the baud rate register value calculations in easier to read
> forms. The computed value isn't modified.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/tty/serial/sh-sci.c | 8 ++++----
>  include/linux/serial_sci.h  | 8 ++++----
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 2fc1a0e..91ce9214b 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1813,13 +1813,13 @@ static unsigned int sci_scbrr_calc(unsigned int
> algo_id, unsigned int bps, {
>  	switch (algo_id) {
>  	case SCBRR_ALGO_1:
> -		return ((freq + 16 * bps) / (16 * bps) - 1);
> +		return freq / (16 * bps);
>  	case SCBRR_ALGO_2:
> -		return ((freq + 16 * bps) / (32 * bps) - 1);
> +		return DIV_ROUND_CLOSEST(freq, 32 * bps) - 1;
>  	case SCBRR_ALGO_3:
> -		return (((freq * 2) + 16 * bps) / (16 * bps) - 1);
> +		return freq / (8 * bps);
>  	case SCBRR_ALGO_4:
> -		return (((freq * 2) + 16 * bps) / (32 * bps) - 1);
> +		return DIV_ROUND_CLOSEST(freq, 16 * bps) - 1;
>  	}
> 
>  	/* Warn, but use a safe default */
> diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
> index 803068d..eb787d4 100644
> --- a/include/linux/serial_sci.h
> +++ b/include/linux/serial_sci.h
> @@ -11,10 +11,10 @@
>  #define SCIx_NOT_SUPPORTED	(-1)
> 
>  enum {
> -	SCBRR_ALGO_1,		/* ((clk + 16 * bps) / (16 * bps) - 1) */
> -	SCBRR_ALGO_2,		/* ((clk + 16 * bps) / (32 * bps) - 1) */
> -	SCBRR_ALGO_3,		/* (((clk * 2) + 16 * bps) / (16 * bps) - 1) */
> -	SCBRR_ALGO_4,		/* (((clk * 2) + 16 * bps) / (32 * bps) - 1) */
> +	SCBRR_ALGO_1,		/* clk / (16 * bps) */
> +	SCBRR_ALGO_2,		/* DIV_ROUND_CLOSEST(clk, 32 * bps) - 1 */
> +	SCBRR_ALGO_3,		/* clk / (8 * bps) */
> +	SCBRR_ALGO_4,		/* DIV_ROUND_CLOSEST(clk, 16 * bps) - 1 */
>  	SCBRR_ALGO_6,		/* HSCIF variable sample rate algorithm */
>  };

While touching this, I'd like to use it as an opportunity to clean things up. 
It looks to me like algorithms 1 and 3 should use DIV_ROUND_CLOSEST() - 1 
instead of rounding the value up as they currently do. I've checked the 
documentation for various SuperH SoCs and they all confirm my hypothesis. As I 
don't have access to any SuperH board I can't test this though.

Given that you have touched the baudrate calculation code in the past, could 
you give me an insight on why algorithms 1 and 3 are implemented without 
rounding to the closest value and without the - 1 ?

On the same subject, algorithm 3 is only used on sh7723 and sh7724 for the 
SCIFA. I've only managed to find a user's manual for the sh7724 (the publicly 
available sh7723 documentation just lists the IP cores and their features), 
and the SCIFA documentation seems to imply that, when using the default 1/16 
sampling rate, algorithm 4 should be used. Could you enlighten me on why the 
sh-sci driver assume a sampling rate of 1/8 instead of 1/16 for baudrate 
calculation for SCIFA on sh772[34] ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/5] Add OF support to the sh-sci serial port driver

[Simon Horman]

[ CCed Greg KH. It is my assumption that he will take this series
  once it is ready ]

On Tue, Oct 29, 2013 at 11:29:54AM +0100, Laurent Pinchart wrote:
> Hello,
> 
> This patch set adds device tree bindings for the sh sci serial port devices
> and adds OF parsing to the sh-sci driver.
> 
> The bindings are based on Bastian Hecht's proposal (see
> http://www.spinics.net/lists/arm-kernel/msg228129.html). The approach taken
> here is more minimalistic: instead of describing all hardware characteristics
> that vary between the SCI device revisions in DT (such as registers layout),
> that information is stored in the driver and selected based on the compatible
> property value. Only SCI revisions used on ARM devices are supported through
> DT, as DT support for SuperH is nowhere down the line.
> 
> The first three patches clean up the sh-sci driver. They're not strictly
> required for OF support but make sense nonetheless. The last two patches add
> OF parsing to the sh-sci driver and create DT bindings documentation.
> 
> The patches have been test on a Lager board (r8a7790-based). Support for other
> SoCs will be added as needed. Note that all current Renesas ARM SoCs seem to
> be compatible with the generic (H)SCI(F)(AB) devices, but the plan is for
> their DT bindings to list the SoC-specific version in case incompatibilities
> are found later.
> 
> Cc: devicetree@vger.kernel.org
> 
> Bastian Hecht (1):
>   serial: sh-sci: Add OF support
> 
> Laurent Pinchart (4):
>   serial: sh-sci: Sort headers alphabetically
>   serial: sh-sci: Remove baud rate calculation algorithm 5
>   serial: sh-sci: Simplify baud rate calculation algorithms
>   serial: sh-sci: Add device tree bindings documentation

Acked-by: Simon Horman <horms+renesas@verge.net.au>

Laurent, is this series waiting on anything other than
a review of the bindings from the Device-Tree people?

Re: [PATCH 0/5] Add OF support to the sh-sci serial port driver

[Simon Horman]

[ CCed Greg KH. It is my assumption that he will take this series
  once it is ready ]

On Tue, Oct 29, 2013 at 11:29:54AM +0100, Laurent Pinchart wrote:
> Hello,
> 
> This patch set adds device tree bindings for the sh sci serial port devices
> and adds OF parsing to the sh-sci driver.
> 
> The bindings are based on Bastian Hecht's proposal (see
> http://www.spinics.net/lists/arm-kernel/msg228129.html). The approach taken
> here is more minimalistic: instead of describing all hardware characteristics
> that vary between the SCI device revisions in DT (such as registers layout),
> that information is stored in the driver and selected based on the compatible
> property value. Only SCI revisions used on ARM devices are supported through
> DT, as DT support for SuperH is nowhere down the line.
> 
> The first three patches clean up the sh-sci driver. They're not strictly
> required for OF support but make sense nonetheless. The last two patches add
> OF parsing to the sh-sci driver and create DT bindings documentation.
> 
> The patches have been test on a Lager board (r8a7790-based). Support for other
> SoCs will be added as needed. Note that all current Renesas ARM SoCs seem to
> be compatible with the generic (H)SCI(F)(AB) devices, but the plan is for
> their DT bindings to list the SoC-specific version in case incompatibilities
> are found later.
> 
> Cc: devicetree@vger.kernel.org
> 
> Bastian Hecht (1):
>   serial: sh-sci: Add OF support
> 
> Laurent Pinchart (4):
>   serial: sh-sci: Sort headers alphabetically
>   serial: sh-sci: Remove baud rate calculation algorithm 5
>   serial: sh-sci: Simplify baud rate calculation algorithms
>   serial: sh-sci: Add device tree bindings documentation

Acked-by: Simon Horman <horms+renesas@verge.net.au>

Laurent, is this series waiting on anything other than
a review of the bindings from the Device-Tree people?

Re: [PATCH 0/5] Add OF support to the sh-sci serial port driver

[Laurent Pinchart]

Hi Simon,

On Thursday 31 October 2013 14:21:16 Simon Horman wrote:
> [ CCed Greg KH. It is my assumption that he will take this series
>   once it is ready ]
> 
> On Tue, Oct 29, 2013 at 11:29:54AM +0100, Laurent Pinchart wrote:
> > Hello,
> > 
> > This patch set adds device tree bindings for the sh sci serial port
> > devices and adds OF parsing to the sh-sci driver.
> > 
> > The bindings are based on Bastian Hecht's proposal (see
> > http://www.spinics.net/lists/arm-kernel/msg228129.html). The approach
> > taken here is more minimalistic: instead of describing all hardware
> > characteristics that vary between the SCI device revisions in DT (such as
> > registers layout), that information is stored in the driver and selected
> > based on the compatible property value. Only SCI revisions used on ARM
> > devices are supported through DT, as DT support for SuperH is nowhere
> > down the line.
> > 
> > The first three patches clean up the sh-sci driver. They're not strictly
> > required for OF support but make sense nonetheless. The last two patches
> > add OF parsing to the sh-sci driver and create DT bindings documentation.
> > 
> > The patches have been test on a Lager board (r8a7790-based). Support for
> > other SoCs will be added as needed. Note that all current Renesas ARM
> > SoCs seem to be compatible with the generic (H)SCI(F)(AB) devices, but
> > the plan is for their DT bindings to list the SoC-specific version in
> > case incompatibilities are found later.
> > 
> > Cc: devicetree@vger.kernel.org
> > 
> > Bastian Hecht (1):
> >   serial: sh-sci: Add OF support
> > 
> > Laurent Pinchart (4):
> >   serial: sh-sci: Sort headers alphabetically
> >   serial: sh-sci: Remove baud rate calculation algorithm 5
> >   serial: sh-sci: Simplify baud rate calculation algorithms
> >   serial: sh-sci: Add device tree bindings documentation
> 
> Acked-by: Simon Horman <horms+renesas@verge.net.au>
> 
> Laurent, is this series waiting on anything other than a review of the
> bindings from the Device-Tree people?

It's only waiting for review of the DT bindings and their implementation 
(patches 4/5 and 5/5). As I've modified Bastian's patch in 4/5 I'd like to 
give him an opportunity to review it as well.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/5] Add OF support to the sh-sci serial port driver

[Laurent Pinchart]

Hi Simon,

On Thursday 31 October 2013 14:21:16 Simon Horman wrote:
> [ CCed Greg KH. It is my assumption that he will take this series
>   once it is ready ]
> 
> On Tue, Oct 29, 2013 at 11:29:54AM +0100, Laurent Pinchart wrote:
> > Hello,
> > 
> > This patch set adds device tree bindings for the sh sci serial port
> > devices and adds OF parsing to the sh-sci driver.
> > 
> > The bindings are based on Bastian Hecht's proposal (see
> > http://www.spinics.net/lists/arm-kernel/msg228129.html). The approach
> > taken here is more minimalistic: instead of describing all hardware
> > characteristics that vary between the SCI device revisions in DT (such as
> > registers layout), that information is stored in the driver and selected
> > based on the compatible property value. Only SCI revisions used on ARM
> > devices are supported through DT, as DT support for SuperH is nowhere
> > down the line.
> > 
> > The first three patches clean up the sh-sci driver. They're not strictly
> > required for OF support but make sense nonetheless. The last two patches
> > add OF parsing to the sh-sci driver and create DT bindings documentation.
> > 
> > The patches have been test on a Lager board (r8a7790-based). Support for
> > other SoCs will be added as needed. Note that all current Renesas ARM
> > SoCs seem to be compatible with the generic (H)SCI(F)(AB) devices, but
> > the plan is for their DT bindings to list the SoC-specific version in
> > case incompatibilities are found later.
> > 
> > Cc: devicetree@vger.kernel.org
> > 
> > Bastian Hecht (1):
> >   serial: sh-sci: Add OF support
> > 
> > Laurent Pinchart (4):
> >   serial: sh-sci: Sort headers alphabetically
> >   serial: sh-sci: Remove baud rate calculation algorithm 5
> >   serial: sh-sci: Simplify baud rate calculation algorithms
> >   serial: sh-sci: Add device tree bindings documentation
> 
> Acked-by: Simon Horman <horms+renesas@verge.net.au>
> 
> Laurent, is this series waiting on anything other than a review of the
> bindings from the Device-Tree people?

It's only waiting for review of the DT bindings and their implementation 
(patches 4/5 and 5/5). As I've modified Bastian's patch in 4/5 I'd like to 
give him an opportunity to review it as well.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 5/5] serial: sh-sci: Add device tree bindings documentation

[Kumar Gala]

On Oct 29, 2013, at 5:29 AM, Laurent Pinchart wrote:

> Document the device tree bindings for the sci serial port devices.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> .../bindings/serial/renesas,sci-serial.txt         | 51 ++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> 
> diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> new file mode 100644
> index 0000000..5658b4f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> @@ -0,0 +1,51 @@
> +* Renesas SH-Mobile Serial Communication Interface
> +
> +Required properties:
> +
> +  - compatible: should be one of the following.

Being pedantic, but how about saying 'one of the following types (scif, scifa, scifb, or hscif)

> +
> +    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
> +    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible UART.
> +    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB compatible UART.
> +    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2) HSCIF compatible UART.
> +    - "renesas,scif-generic" for generic SCIF compatible UART.
> +    - "renesas,scifa-generic" for generic SCIFA compatible UART.
> +    - "renesas,scifb-generic" for generic SCIFB compatible UART.
> +    - "renesas,hscif-generic" for generic HSCIF compatible UART.
> +
> +    When compatible with the generic version, nodes must also list the
> +    SoC-specific version corresponding to the platform.
> +
> +  - reg: Base address and length of the memory resource used by the UART.
> +
> +  - interrupt-parent: Reference to the parent interrupt controller.
> +  - interrupts: Interrupt number(s). Depending on the SoC SCIx UARTs are tied
> +    to one or multiple interrupt lines. When using multiple interrupt lines,
> +    specify the interrupt names as described below.
> +
> +  - clocks: Reference to the SCIx UART interface clock.
> +  - clock-names: Should be "sci_ick".
> +
> +Optional properties:
> +
> +  - interrupt-names: When using multiple interrupts, report the interrupt
> +    names as "eri" (receive error), "rxi" (receive), "txi" (transmit) and
> +    "bri" (break). When using a single interrupt this property should not be
> +    present.

Hmm, is there a reason not to just have the 4 interrupts always present and in the case they are all wired to the same interrupt, just have the interrupts all be the same?

example:

interrupts = <0 144 4>, <0 144 4>, <0 144 4>, <0 144 4>;


> +
> +Note: Each enabled SCIx UART should have an alias correctly numbered in the
> +"aliases" node.
> +
> +Example:
> +	aliases {
> +		serial0 = &scifa0;
> +	};
> +
> +	scifa0: serial@e6c40000 {
> +		compatible = "renesas,scifa-r8a7790", "renesas,scifa-generic";
> +		reg = <0 0xe6c40000 0 64>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 144 4>;
> +		clocks = <&mstp2_clks 4>;
> +		clock-names = "sci_ick";
> +	};

might be nice to have an example with the 4 interrupts.

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 5/5] serial: sh-sci: Add device tree bindings documentation

[Kumar Gala]

On Oct 29, 2013, at 5:29 AM, Laurent Pinchart wrote:

> Document the device tree bindings for the sci serial port devices.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> .../bindings/serial/renesas,sci-serial.txt         | 51 ++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> 
> diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> new file mode 100644
> index 0000000..5658b4f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> @@ -0,0 +1,51 @@
> +* Renesas SH-Mobile Serial Communication Interface
> +
> +Required properties:
> +
> +  - compatible: should be one of the following.

Being pedantic, but how about saying 'one of the following types (scif, scifa, scifb, or hscif)

> +
> +    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
> +    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible UART.
> +    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB compatible UART.
> +    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2) HSCIF compatible UART.
> +    - "renesas,scif-generic" for generic SCIF compatible UART.
> +    - "renesas,scifa-generic" for generic SCIFA compatible UART.
> +    - "renesas,scifb-generic" for generic SCIFB compatible UART.
> +    - "renesas,hscif-generic" for generic HSCIF compatible UART.
> +
> +    When compatible with the generic version, nodes must also list the
> +    SoC-specific version corresponding to the platform.
> +
> +  - reg: Base address and length of the memory resource used by the UART.
> +
> +  - interrupt-parent: Reference to the parent interrupt controller.
> +  - interrupts: Interrupt number(s). Depending on the SoC SCIx UARTs are tied
> +    to one or multiple interrupt lines. When using multiple interrupt lines,
> +    specify the interrupt names as described below.
> +
> +  - clocks: Reference to the SCIx UART interface clock.
> +  - clock-names: Should be "sci_ick".
> +
> +Optional properties:
> +
> +  - interrupt-names: When using multiple interrupts, report the interrupt
> +    names as "eri" (receive error), "rxi" (receive), "txi" (transmit) and
> +    "bri" (break). When using a single interrupt this property should not be
> +    present.

Hmm, is there a reason not to just have the 4 interrupts always present and in the case they are all wired to the same interrupt, just have the interrupts all be the same?

example:

interrupts = <0 144 4>, <0 144 4>, <0 144 4>, <0 144 4>;


> +
> +Note: Each enabled SCIx UART should have an alias correctly numbered in the
> +"aliases" node.
> +
> +Example:
> +	aliases {
> +		serial0 = &scifa0;
> +	};
> +
> +	scifa0: serial@e6c40000 {
> +		compatible = "renesas,scifa-r8a7790", "renesas,scifa-generic";
> +		reg = <0 0xe6c40000 0 64>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 144 4>;
> +		clocks = <&mstp2_clks 4>;
> +		clock-names = "sci_ick";
> +	};

might be nice to have an example with the 4 interrupts.

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 5/5] serial: sh-sci: Add device tree bindings documentation

[Laurent Pinchart]

Hi Kumar,

Thank you for the review.

On Thursday 31 October 2013 09:42:48 Kumar Gala wrote:
> On Oct 29, 2013, at 5:29 AM, Laurent Pinchart wrote:
> > Document the device tree bindings for the sci serial port devices.
> > 
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > .../bindings/serial/renesas,sci-serial.txt         | 51 ++++++++++++++++++
> > 1 file changed, 51 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt new
> > file mode 100644
> > index 0000000..5658b4f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > @@ -0,0 +1,51 @@
> > +* Renesas SH-Mobile Serial Communication Interface
> > +
> > +Required properties:
> > +
> > +  - compatible: should be one of the following.
> 
> Being pedantic, but how about saying 'one of the following types (scif,
> scifa, scifb, or hscif)

Sounds good to me.

> > +
> > +    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
> > +    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible
> > UART.
> > +    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB compatible
> > UART.
> > +    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2) HSCIF compatible
> > UART.
> > +    - "renesas,scif-generic" for generic SCIF compatible UART.
> > +    - "renesas,scifa-generic" for generic SCIFA compatible UART.
> > +    - "renesas,scifb-generic" for generic SCIFB compatible UART.
> > +    - "renesas,hscif-generic" for generic HSCIF compatible UART.
> > +
> > +    When compatible with the generic version, nodes must also list the
> > +    SoC-specific version corresponding to the platform.
> > +
> > +  - reg: Base address and length of the memory resource used by the UART.
> > +
> > +  - interrupt-parent: Reference to the parent interrupt controller.
> > +  - interrupts: Interrupt number(s). Depending on the SoC SCIx UARTs are
> > tied
> > +    to one or multiple interrupt lines. When using multiple interrupt
> > lines,
> > +    specify the interrupt names as described below.
> > +
> > +  - clocks: Reference to the SCIx UART interface clock.
> > +  - clock-names: Should be "sci_ick".
> > +
> > +Optional properties:
> > +
> > +  - interrupt-names: When using multiple interrupts, report the interrupt
> > +    names as "eri" (receive error), "rxi" (receive), "txi" (transmit) and
> > +    "bri" (break). When using a single interrupt this property should not
> > be +    present.
> 
> Hmm, is there a reason not to just have the 4 interrupts always present and
> in the case they are all wired to the same interrupt, just have the
> interrupts all be the same?
> 
> example:
> 
> interrupts = <0 144 4>, <0 144 4>, <0 144 4>, <0 144 4>;

The multiple interrupts case is the exception, I thought it would be easier to 
specify a single interrupt in the general case and have a more complex binding 
for the exceptions.

> > +
> > +Note: Each enabled SCIx UART should have an alias correctly numbered in
> > the +"aliases" node.
> > +
> > +Example:
> > +	aliases {
> > +		serial0 = &scifa0;
> > +	};
> > +
> > +	scifa0: serial@e6c40000 {
> > +		compatible = "renesas,scifa-r8a7790", "renesas,scifa-generic";
> > +		reg = <0 0xe6c40000 0 64>;
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <0 144 4>;
> > +		clocks = <&mstp2_clks 4>;
> > +		clock-names = "sci_ick";
> > +	};
> 
> might be nice to have an example with the 4 interrupts.

The example would be pretty theoretical, as the r8a7790 uses a single 
interrupt in all cases. What about adding such an example when adding an SoC 
that uses multiple interrupts to the DT bindings documentation ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 5/5] serial: sh-sci: Add device tree bindings documentation

[Laurent Pinchart]

Hi Kumar,

Thank you for the review.

On Thursday 31 October 2013 09:42:48 Kumar Gala wrote:
> On Oct 29, 2013, at 5:29 AM, Laurent Pinchart wrote:
> > Document the device tree bindings for the sci serial port devices.
> > 
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > .../bindings/serial/renesas,sci-serial.txt         | 51 ++++++++++++++++++
> > 1 file changed, 51 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt new
> > file mode 100644
> > index 0000000..5658b4f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > @@ -0,0 +1,51 @@
> > +* Renesas SH-Mobile Serial Communication Interface
> > +
> > +Required properties:
> > +
> > +  - compatible: should be one of the following.
> 
> Being pedantic, but how about saying 'one of the following types (scif,
> scifa, scifb, or hscif)

Sounds good to me.

> > +
> > +    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
> > +    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible
> > UART.
> > +    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB compatible
> > UART.
> > +    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2) HSCIF compatible
> > UART.
> > +    - "renesas,scif-generic" for generic SCIF compatible UART.
> > +    - "renesas,scifa-generic" for generic SCIFA compatible UART.
> > +    - "renesas,scifb-generic" for generic SCIFB compatible UART.
> > +    - "renesas,hscif-generic" for generic HSCIF compatible UART.
> > +
> > +    When compatible with the generic version, nodes must also list the
> > +    SoC-specific version corresponding to the platform.
> > +
> > +  - reg: Base address and length of the memory resource used by the UART.
> > +
> > +  - interrupt-parent: Reference to the parent interrupt controller.
> > +  - interrupts: Interrupt number(s). Depending on the SoC SCIx UARTs are
> > tied
> > +    to one or multiple interrupt lines. When using multiple interrupt
> > lines,
> > +    specify the interrupt names as described below.
> > +
> > +  - clocks: Reference to the SCIx UART interface clock.
> > +  - clock-names: Should be "sci_ick".
> > +
> > +Optional properties:
> > +
> > +  - interrupt-names: When using multiple interrupts, report the interrupt
> > +    names as "eri" (receive error), "rxi" (receive), "txi" (transmit) and
> > +    "bri" (break). When using a single interrupt this property should not
> > be +    present.
> 
> Hmm, is there a reason not to just have the 4 interrupts always present and
> in the case they are all wired to the same interrupt, just have the
> interrupts all be the same?
> 
> example:
> 
> interrupts = <0 144 4>, <0 144 4>, <0 144 4>, <0 144 4>;

The multiple interrupts case is the exception, I thought it would be easier to 
specify a single interrupt in the general case and have a more complex binding 
for the exceptions.

> > +
> > +Note: Each enabled SCIx UART should have an alias correctly numbered in
> > the +"aliases" node.
> > +
> > +Example:
> > +	aliases {
> > +		serial0 = &scifa0;
> > +	};
> > +
> > +	scifa0: serial@e6c40000 {
> > +		compatible = "renesas,scifa-r8a7790", "renesas,scifa-generic";
> > +		reg = <0 0xe6c40000 0 64>;
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <0 144 4>;
> > +		clocks = <&mstp2_clks 4>;
> > +		clock-names = "sci_ick";
> > +	};
> 
> might be nice to have an example with the 4 interrupts.

The example would be pretty theoretical, as the r8a7790 uses a single 
interrupt in all cases. What about adding such an example when adding an SoC 
that uses multiple interrupts to the DT bindings documentation ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 5/5] serial: sh-sci: Add device tree bindings documentation

[Kumar Gala]

On Oct 31, 2013, at 9:55 AM, Laurent Pinchart wrote:

> Hi Kumar,
> 
> Thank you for the review.
> 
> On Thursday 31 October 2013 09:42:48 Kumar Gala wrote:
>> On Oct 29, 2013, at 5:29 AM, Laurent Pinchart wrote:
>>> Document the device tree bindings for the sci serial port devices.
>>> 
>>> Cc: devicetree@vger.kernel.org
>>> Signed-off-by: Laurent Pinchart
>>> <laurent.pinchart+renesas@ideasonboard.com>
>>> ---
>>> .../bindings/serial/renesas,sci-serial.txt         | 51 ++++++++++++++++++
>>> 1 file changed, 51 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
>>> 
>>> diff --git
>>> a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
>>> b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt new
>>> file mode 100644
>>> index 0000000..5658b4f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
>>> @@ -0,0 +1,51 @@
>>> +* Renesas SH-Mobile Serial Communication Interface
>>> +
>>> +Required properties:
>>> +
>>> +  - compatible: should be one of the following.
>> 
>> Being pedantic, but how about saying 'one of the following types (scif,
>> scifa, scifb, or hscif)
> 
> Sounds good to me.
> 
>>> +
>>> +    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
>>> +    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible
>>> UART.
>>> +    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB compatible
>>> UART.
>>> +    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2) HSCIF compatible
>>> UART.
>>> +    - "renesas,scif-generic" for generic SCIF compatible UART.
>>> +    - "renesas,scifa-generic" for generic SCIFA compatible UART.
>>> +    - "renesas,scifb-generic" for generic SCIFB compatible UART.
>>> +    - "renesas,hscif-generic" for generic HSCIF compatible UART.
>>> +
>>> +    When compatible with the generic version, nodes must also list the
>>> +    SoC-specific version corresponding to the platform.
>>> +
>>> +  - reg: Base address and length of the memory resource used by the UART.
>>> +
>>> +  - interrupt-parent: Reference to the parent interrupt controller.
>>> +  - interrupts: Interrupt number(s). Depending on the SoC SCIx UARTs are
>>> tied
>>> +    to one or multiple interrupt lines. When using multiple interrupt
>>> lines,
>>> +    specify the interrupt names as described below.
>>> +
>>> +  - clocks: Reference to the SCIx UART interface clock.
>>> +  - clock-names: Should be "sci_ick".
>>> +
>>> +Optional properties:
>>> +
>>> +  - interrupt-names: When using multiple interrupts, report the interrupt
>>> +    names as "eri" (receive error), "rxi" (receive), "txi" (transmit) and
>>> +    "bri" (break). When using a single interrupt this property should not
>>> be +    present.
>> 
>> Hmm, is there a reason not to just have the 4 interrupts always present and
>> in the case they are all wired to the same interrupt, just have the
>> interrupts all be the same?
>> 
>> example:
>> 
>> interrupts = <0 144 4>, <0 144 4>, <0 144 4>, <0 144 4>;
> 
> The multiple interrupts case is the exception, I thought it would be easier to 
> specify a single interrupt in the general case and have a more complex binding 
> for the exceptions.
> 
>>> +
>>> +Note: Each enabled SCIx UART should have an alias correctly numbered in
>>> the +"aliases" node.
>>> +
>>> +Example:
>>> +	aliases {
>>> +		serial0 = &scifa0;
>>> +	};
>>> +
>>> +	scifa0: serial@e6c40000 {
>>> +		compatible = "renesas,scifa-r8a7790", "renesas,scifa-generic";
>>> +		reg = <0 0xe6c40000 0 64>;
>>> +		interrupt-parent = <&gic>;
>>> +		interrupts = <0 144 4>;
>>> +		clocks = <&mstp2_clks 4>;
>>> +		clock-names = "sci_ick";
>>> +	};
>> 
>> might be nice to have an example with the 4 interrupts.
> 
> The example would be pretty theoretical, as the r8a7790 uses a single 
> interrupt in all cases. What about adding such an example when adding an SoC 
> that uses multiple interrupts to the DT bindings documentation ?

I'm fine with that if we strip the multiple interrupt bits out of the binding until a device/compatible exists that needs it.

- k
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 5/5] serial: sh-sci: Add device tree bindings documentation

[Kumar Gala]

On Oct 31, 2013, at 9:55 AM, Laurent Pinchart wrote:

> Hi Kumar,
> 
> Thank you for the review.
> 
> On Thursday 31 October 2013 09:42:48 Kumar Gala wrote:
>> On Oct 29, 2013, at 5:29 AM, Laurent Pinchart wrote:
>>> Document the device tree bindings for the sci serial port devices.
>>> 
>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> Signed-off-by: Laurent Pinchart
>>> <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
>>> ---
>>> .../bindings/serial/renesas,sci-serial.txt         | 51 ++++++++++++++++++
>>> 1 file changed, 51 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
>>> 
>>> diff --git
>>> a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
>>> b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt new
>>> file mode 100644
>>> index 0000000..5658b4f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
>>> @@ -0,0 +1,51 @@
>>> +* Renesas SH-Mobile Serial Communication Interface
>>> +
>>> +Required properties:
>>> +
>>> +  - compatible: should be one of the following.
>> 
>> Being pedantic, but how about saying 'one of the following types (scif,
>> scifa, scifb, or hscif)
> 
> Sounds good to me.
> 
>>> +
>>> +    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
>>> +    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible
>>> UART.
>>> +    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB compatible
>>> UART.
>>> +    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2) HSCIF compatible
>>> UART.
>>> +    - "renesas,scif-generic" for generic SCIF compatible UART.
>>> +    - "renesas,scifa-generic" for generic SCIFA compatible UART.
>>> +    - "renesas,scifb-generic" for generic SCIFB compatible UART.
>>> +    - "renesas,hscif-generic" for generic HSCIF compatible UART.
>>> +
>>> +    When compatible with the generic version, nodes must also list the
>>> +    SoC-specific version corresponding to the platform.
>>> +
>>> +  - reg: Base address and length of the memory resource used by the UART.
>>> +
>>> +  - interrupt-parent: Reference to the parent interrupt controller.
>>> +  - interrupts: Interrupt number(s). Depending on the SoC SCIx UARTs are
>>> tied
>>> +    to one or multiple interrupt lines. When using multiple interrupt
>>> lines,
>>> +    specify the interrupt names as described below.
>>> +
>>> +  - clocks: Reference to the SCIx UART interface clock.
>>> +  - clock-names: Should be "sci_ick".
>>> +
>>> +Optional properties:
>>> +
>>> +  - interrupt-names: When using multiple interrupts, report the interrupt
>>> +    names as "eri" (receive error), "rxi" (receive), "txi" (transmit) and
>>> +    "bri" (break). When using a single interrupt this property should not
>>> be +    present.
>> 
>> Hmm, is there a reason not to just have the 4 interrupts always present and
>> in the case they are all wired to the same interrupt, just have the
>> interrupts all be the same?
>> 
>> example:
>> 
>> interrupts = <0 144 4>, <0 144 4>, <0 144 4>, <0 144 4>;
> 
> The multiple interrupts case is the exception, I thought it would be easier to 
> specify a single interrupt in the general case and have a more complex binding 
> for the exceptions.
> 
>>> +
>>> +Note: Each enabled SCIx UART should have an alias correctly numbered in
>>> the +"aliases" node.
>>> +
>>> +Example:
>>> +	aliases {
>>> +		serial0 = &scifa0;
>>> +	};
>>> +
>>> +	scifa0: serial@e6c40000 {
>>> +		compatible = "renesas,scifa-r8a7790", "renesas,scifa-generic";
>>> +		reg = <0 0xe6c40000 0 64>;
>>> +		interrupt-parent = <&gic>;
>>> +		interrupts = <0 144 4>;
>>> +		clocks = <&mstp2_clks 4>;
>>> +		clock-names = "sci_ick";
>>> +	};
>> 
>> might be nice to have an example with the 4 interrupts.
> 
> The example would be pretty theoretical, as the r8a7790 uses a single 
> interrupt in all cases. What about adding such an example when adding an SoC 
> that uses multiple interrupts to the DT bindings documentation ?

I'm fine with that if we strip the multiple interrupt bits out of the binding until a device/compatible exists that needs it.

- k
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

--
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 0/5] Add OF support to the sh-sci serial port driver

[Simon Horman]

On Thu, Oct 31, 2013 at 01:30:12PM +0100, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Thursday 31 October 2013 14:21:16 Simon Horman wrote:
> > [ CCed Greg KH. It is my assumption that he will take this series
> >   once it is ready ]
> > 
> > On Tue, Oct 29, 2013 at 11:29:54AM +0100, Laurent Pinchart wrote:
> > > Hello,
> > > 
> > > This patch set adds device tree bindings for the sh sci serial port
> > > devices and adds OF parsing to the sh-sci driver.
> > > 
> > > The bindings are based on Bastian Hecht's proposal (see
> > > http://www.spinics.net/lists/arm-kernel/msg228129.html). The approach
> > > taken here is more minimalistic: instead of describing all hardware
> > > characteristics that vary between the SCI device revisions in DT (such as
> > > registers layout), that information is stored in the driver and selected
> > > based on the compatible property value. Only SCI revisions used on ARM
> > > devices are supported through DT, as DT support for SuperH is nowhere
> > > down the line.
> > > 
> > > The first three patches clean up the sh-sci driver. They're not strictly
> > > required for OF support but make sense nonetheless. The last two patches
> > > add OF parsing to the sh-sci driver and create DT bindings documentation.
> > > 
> > > The patches have been test on a Lager board (r8a7790-based). Support for
> > > other SoCs will be added as needed. Note that all current Renesas ARM
> > > SoCs seem to be compatible with the generic (H)SCI(F)(AB) devices, but
> > > the plan is for their DT bindings to list the SoC-specific version in
> > > case incompatibilities are found later.
> > > 
> > > Cc: devicetree@vger.kernel.org
> > > 
> > > Bastian Hecht (1):
> > >   serial: sh-sci: Add OF support
> > > 
> > > Laurent Pinchart (4):
> > >   serial: sh-sci: Sort headers alphabetically
> > >   serial: sh-sci: Remove baud rate calculation algorithm 5
> > >   serial: sh-sci: Simplify baud rate calculation algorithms
> > >   serial: sh-sci: Add device tree bindings documentation
> > 
> > Acked-by: Simon Horman <horms+renesas@verge.net.au>
> > 
> > Laurent, is this series waiting on anything other than a review of the
> > bindings from the Device-Tree people?
> 
> It's only waiting for review of the DT bindings and their implementation 
> (patches 4/5 and 5/5). As I've modified Bastian's patch in 4/5 I'd like to 
> give him an opportunity to review it as well.

Understood.

Re: [PATCH 0/5] Add OF support to the sh-sci serial port driver

[Simon Horman]

On Thu, Oct 31, 2013 at 01:30:12PM +0100, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Thursday 31 October 2013 14:21:16 Simon Horman wrote:
> > [ CCed Greg KH. It is my assumption that he will take this series
> >   once it is ready ]
> > 
> > On Tue, Oct 29, 2013 at 11:29:54AM +0100, Laurent Pinchart wrote:
> > > Hello,
> > > 
> > > This patch set adds device tree bindings for the sh sci serial port
> > > devices and adds OF parsing to the sh-sci driver.
> > > 
> > > The bindings are based on Bastian Hecht's proposal (see
> > > http://www.spinics.net/lists/arm-kernel/msg228129.html). The approach
> > > taken here is more minimalistic: instead of describing all hardware
> > > characteristics that vary between the SCI device revisions in DT (such as
> > > registers layout), that information is stored in the driver and selected
> > > based on the compatible property value. Only SCI revisions used on ARM
> > > devices are supported through DT, as DT support for SuperH is nowhere
> > > down the line.
> > > 
> > > The first three patches clean up the sh-sci driver. They're not strictly
> > > required for OF support but make sense nonetheless. The last two patches
> > > add OF parsing to the sh-sci driver and create DT bindings documentation.
> > > 
> > > The patches have been test on a Lager board (r8a7790-based). Support for
> > > other SoCs will be added as needed. Note that all current Renesas ARM
> > > SoCs seem to be compatible with the generic (H)SCI(F)(AB) devices, but
> > > the plan is for their DT bindings to list the SoC-specific version in
> > > case incompatibilities are found later.
> > > 
> > > Cc: devicetree@vger.kernel.org
> > > 
> > > Bastian Hecht (1):
> > >   serial: sh-sci: Add OF support
> > > 
> > > Laurent Pinchart (4):
> > >   serial: sh-sci: Sort headers alphabetically
> > >   serial: sh-sci: Remove baud rate calculation algorithm 5
> > >   serial: sh-sci: Simplify baud rate calculation algorithms
> > >   serial: sh-sci: Add device tree bindings documentation
> > 
> > Acked-by: Simon Horman <horms+renesas@verge.net.au>
> > 
> > Laurent, is this series waiting on anything other than a review of the
> > bindings from the Device-Tree people?
> 
> It's only waiting for review of the DT bindings and their implementation 
> (patches 4/5 and 5/5). As I've modified Bastian's patch in 4/5 I'd like to 
> give him an opportunity to review it as well.

Understood.

Re: [PATCH 4/5] serial: sh-sci: Add OF support

[Bastian Hecht]

Hi Laurent!

> +struct sci_port_info {
> +       unsigned int type;
> +       unsigned int regtype;
> +       unsigned int brr_algo;
> +};
> +
> +static const struct of_device_id of_sci_match[] = {
> +       {
> +               .compatible = "renesas,scif-generic",
> +               .data = (void *)&(const struct sci_port_info) {
> +                       .type = PORT_SCIF,
> +                       .regtype = SCIx_SH4_SCIF_REGTYPE,
> +                       .brr_algo = SCBRR_ALGO_2,
> +               },
> +       }, {
> +               .compatible = "renesas,scifa-generic",
> +               .data = (void *)&(const struct sci_port_info) {
> +                       .type = PORT_SCIFA,
> +                       .regtype = SCIx_SCIFA_REGTYPE,
> +                       .brr_algo = SCBRR_ALGO_4,
> +               },
> +       }, {
> +               .compatible = "renesas,scifb-generic",
> +               .data = (void *)&(const struct sci_port_info) {
> +                       .type = PORT_SCIFB,
> +                       .regtype = SCIx_SCIFB_REGTYPE,
> +                       .brr_algo = SCBRR_ALGO_4,
> +               },
> +       }, {
> +               .compatible = "renesas,hscif-generic",
> +               .data = (void *)&(const struct sci_port_info) {
> +                       .type = PORT_HSCIF,
> +                       .regtype = SCIx_HSCIF_REGTYPE,
> +                       .brr_algo = SCBRR_ALGO_6,
> +               },
> +       }, {
> +               /* Terminator */
> +       },
> +};
> +MODULE_DEVICE_TABLE(of, of_sci_match);

I like this version way better than my initial approach, given that we
only support ARM based ports and we hope the regtype and brr_algo
won't deviate a lot from the given generic settings for new chips -
else the of_sci_match[] array might become a bit bloated. This was the
former reasoning to introduce all the clumsy OF fields.

I'm happy that the sci OF support will finally make it into the
mainline, thanks Laurent!

Bastian

Re: [PATCH 4/5] serial: sh-sci: Add OF support

[Bastian Hecht]

Hi Laurent!

> +struct sci_port_info {
> +       unsigned int type;
> +       unsigned int regtype;
> +       unsigned int brr_algo;
> +};
> +
> +static const struct of_device_id of_sci_match[] = {
> +       {
> +               .compatible = "renesas,scif-generic",
> +               .data = (void *)&(const struct sci_port_info) {
> +                       .type = PORT_SCIF,
> +                       .regtype = SCIx_SH4_SCIF_REGTYPE,
> +                       .brr_algo = SCBRR_ALGO_2,
> +               },
> +       }, {
> +               .compatible = "renesas,scifa-generic",
> +               .data = (void *)&(const struct sci_port_info) {
> +                       .type = PORT_SCIFA,
> +                       .regtype = SCIx_SCIFA_REGTYPE,
> +                       .brr_algo = SCBRR_ALGO_4,
> +               },
> +       }, {
> +               .compatible = "renesas,scifb-generic",
> +               .data = (void *)&(const struct sci_port_info) {
> +                       .type = PORT_SCIFB,
> +                       .regtype = SCIx_SCIFB_REGTYPE,
> +                       .brr_algo = SCBRR_ALGO_4,
> +               },
> +       }, {
> +               .compatible = "renesas,hscif-generic",
> +               .data = (void *)&(const struct sci_port_info) {
> +                       .type = PORT_HSCIF,
> +                       .regtype = SCIx_HSCIF_REGTYPE,
> +                       .brr_algo = SCBRR_ALGO_6,
> +               },
> +       }, {
> +               /* Terminator */
> +       },
> +};
> +MODULE_DEVICE_TABLE(of, of_sci_match);

I like this version way better than my initial approach, given that we
only support ARM based ports and we hope the regtype and brr_algo
won't deviate a lot from the given generic settings for new chips -
else the of_sci_match[] array might become a bit bloated. This was the
former reasoning to introduce all the clumsy OF fields.

I'm happy that the sci OF support will finally make it into the
mainline, thanks Laurent!

Bastian

Re: [PATCH 5/5] serial: sh-sci: Add device tree bindings documentation

[Laurent Pinchart]

Hi Kumar,

On Thursday 31 October 2013 10:00:36 Kumar Gala wrote:
> On Oct 31, 2013, at 9:55 AM, Laurent Pinchart wrote:
> > On Thursday 31 October 2013 09:42:48 Kumar Gala wrote:
> >> On Oct 29, 2013, at 5:29 AM, Laurent Pinchart wrote:
> >>> Document the device tree bindings for the sci serial port devices.
> >>> 
> >>> Cc: devicetree@vger.kernel.org
> >>> Signed-off-by: Laurent Pinchart
> >>> <laurent.pinchart+renesas@ideasonboard.com>
> >>> ---
> >>> .../bindings/serial/renesas,sci-serial.txt         | 51 ++++++++++++++++
> >>> 1 file changed, 51 insertions(+)
> >>> create mode 100644
> >>> Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> >>> 
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> >>> b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt new
> >>> file mode 100644
> >>> index 0000000..5658b4f
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> >>> @@ -0,0 +1,51 @@
> >>> +* Renesas SH-Mobile Serial Communication Interface
> >>> +
> >>> +Required properties:
> >>> +
> >>> +  - compatible: should be one of the following.
> >> 
> >> Being pedantic, but how about saying 'one of the following types (scif,
> >> scifa, scifb, or hscif)
> > 
> > Sounds good to me.
> > 
> >>> +
> >>> +    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible
> >>> UART.
> >>> +    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible
> >>> UART.
> >>> +    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB compatible
> >>> UART.
> >>> +    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2) HSCIF compatible
> >>> UART.
> >>> +    - "renesas,scif-generic" for generic SCIF compatible UART.
> >>> +    - "renesas,scifa-generic" for generic SCIFA compatible UART.
> >>> +    - "renesas,scifb-generic" for generic SCIFB compatible UART.
> >>> +    - "renesas,hscif-generic" for generic HSCIF compatible UART.
> >>> +
> >>> +    When compatible with the generic version, nodes must also list the
> >>> +    SoC-specific version corresponding to the platform.
> >>> +
> >>> +  - reg: Base address and length of the memory resource used by the
> >>> UART.
> >>> +
> >>> +  - interrupt-parent: Reference to the parent interrupt controller.
> >>> +  - interrupts: Interrupt number(s). Depending on the SoC SCIx UARTs
> >>> are
> >>> tied
> >>> +    to one or multiple interrupt lines. When using multiple interrupt
> >>> lines,
> >>> +    specify the interrupt names as described below.
> >>> +
> >>> +  - clocks: Reference to the SCIx UART interface clock.
> >>> +  - clock-names: Should be "sci_ick".
> >>> +
> >>> +Optional properties:
> >>> +
> >>> +  - interrupt-names: When using multiple interrupts, report the
> >>> interrupt
> >>> +    names as "eri" (receive error), "rxi" (receive), "txi" (transmit)
> >>> and
> >>> +    "bri" (break). When using a single interrupt this property should
> >>> not
> >>> be +    present.
> >> 
> >> Hmm, is there a reason not to just have the 4 interrupts always present
> >> and in the case they are all wired to the same interrupt, just have the
> >> interrupts all be the same?
> >> 
> >> example:
> >> 
> >> interrupts = <0 144 4>, <0 144 4>, <0 144 4>, <0 144 4>;
> > 
> > The multiple interrupts case is the exception, I thought it would be
> > easier to specify a single interrupt in the general case and have a more
> > complex binding for the exceptions.
> > 
> >>> +
> >>> +Note: Each enabled SCIx UART should have an alias correctly numbered in
> >>> the +"aliases" node.
> >>> +
> >>> +Example:
> >>> +	aliases {
> >>> +		serial0 = &scifa0;
> >>> +	};
> >>> +
> >>> +	scifa0: serial@e6c40000 {
> >>> +		compatible = "renesas,scifa-r8a7790", "renesas,scifa-generic";
> >>> +		reg = <0 0xe6c40000 0 64>;
> >>> +		interrupt-parent = <&gic>;
> >>> +		interrupts = <0 144 4>;
> >>> +		clocks = <&mstp2_clks 4>;
> >>> +		clock-names = "sci_ick";
> >>> +	};
> >> 
> >> might be nice to have an example with the 4 interrupts.
> > 
> > The example would be pretty theoretical, as the r8a7790 uses a single
> > interrupt in all cases. What about adding such an example when adding an
> > SoC that uses multiple interrupts to the DT bindings documentation ?
> 
> I'm fine with that if we strip the multiple interrupt bits out of the
> binding until a device/compatible exists that needs it.

I'll do that and repost.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 5/5] serial: sh-sci: Add device tree bindings documentation

[Laurent Pinchart]

Hi Kumar,

On Thursday 31 October 2013 10:00:36 Kumar Gala wrote:
> On Oct 31, 2013, at 9:55 AM, Laurent Pinchart wrote:
> > On Thursday 31 October 2013 09:42:48 Kumar Gala wrote:
> >> On Oct 29, 2013, at 5:29 AM, Laurent Pinchart wrote:
> >>> Document the device tree bindings for the sci serial port devices.
> >>> 
> >>> Cc: devicetree@vger.kernel.org
> >>> Signed-off-by: Laurent Pinchart
> >>> <laurent.pinchart+renesas@ideasonboard.com>
> >>> ---
> >>> .../bindings/serial/renesas,sci-serial.txt         | 51 ++++++++++++++++
> >>> 1 file changed, 51 insertions(+)
> >>> create mode 100644
> >>> Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> >>> 
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> >>> b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt new
> >>> file mode 100644
> >>> index 0000000..5658b4f
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> >>> @@ -0,0 +1,51 @@
> >>> +* Renesas SH-Mobile Serial Communication Interface
> >>> +
> >>> +Required properties:
> >>> +
> >>> +  - compatible: should be one of the following.
> >> 
> >> Being pedantic, but how about saying 'one of the following types (scif,
> >> scifa, scifb, or hscif)
> > 
> > Sounds good to me.
> > 
> >>> +
> >>> +    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible
> >>> UART.
> >>> +    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible
> >>> UART.
> >>> +    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB compatible
> >>> UART.
> >>> +    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2) HSCIF compatible
> >>> UART.
> >>> +    - "renesas,scif-generic" for generic SCIF compatible UART.
> >>> +    - "renesas,scifa-generic" for generic SCIFA compatible UART.
> >>> +    - "renesas,scifb-generic" for generic SCIFB compatible UART.
> >>> +    - "renesas,hscif-generic" for generic HSCIF compatible UART.
> >>> +
> >>> +    When compatible with the generic version, nodes must also list the
> >>> +    SoC-specific version corresponding to the platform.
> >>> +
> >>> +  - reg: Base address and length of the memory resource used by the
> >>> UART.
> >>> +
> >>> +  - interrupt-parent: Reference to the parent interrupt controller.
> >>> +  - interrupts: Interrupt number(s). Depending on the SoC SCIx UARTs
> >>> are
> >>> tied
> >>> +    to one or multiple interrupt lines. When using multiple interrupt
> >>> lines,
> >>> +    specify the interrupt names as described below.
> >>> +
> >>> +  - clocks: Reference to the SCIx UART interface clock.
> >>> +  - clock-names: Should be "sci_ick".
> >>> +
> >>> +Optional properties:
> >>> +
> >>> +  - interrupt-names: When using multiple interrupts, report the
> >>> interrupt
> >>> +    names as "eri" (receive error), "rxi" (receive), "txi" (transmit)
> >>> and
> >>> +    "bri" (break). When using a single interrupt this property should
> >>> not
> >>> be +    present.
> >> 
> >> Hmm, is there a reason not to just have the 4 interrupts always present
> >> and in the case they are all wired to the same interrupt, just have the
> >> interrupts all be the same?
> >> 
> >> example:
> >> 
> >> interrupts = <0 144 4>, <0 144 4>, <0 144 4>, <0 144 4>;
> > 
> > The multiple interrupts case is the exception, I thought it would be
> > easier to specify a single interrupt in the general case and have a more
> > complex binding for the exceptions.
> > 
> >>> +
> >>> +Note: Each enabled SCIx UART should have an alias correctly numbered in
> >>> the +"aliases" node.
> >>> +
> >>> +Example:
> >>> +	aliases {
> >>> +		serial0 = &scifa0;
> >>> +	};
> >>> +
> >>> +	scifa0: serial@e6c40000 {
> >>> +		compatible = "renesas,scifa-r8a7790", "renesas,scifa-generic";
> >>> +		reg = <0 0xe6c40000 0 64>;
> >>> +		interrupt-parent = <&gic>;
> >>> +		interrupts = <0 144 4>;
> >>> +		clocks = <&mstp2_clks 4>;
> >>> +		clock-names = "sci_ick";
> >>> +	};
> >> 
> >> might be nice to have an example with the 4 interrupts.
> > 
> > The example would be pretty theoretical, as the r8a7790 uses a single
> > interrupt in all cases. What about adding such an example when adding an
> > SoC that uses multiple interrupts to the DT bindings documentation ?
> 
> I'm fine with that if we strip the multiple interrupt bits out of the
> binding until a device/compatible exists that needs it.

I'll do that and repost.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/5] serial: sh-sci: Add OF support

[Laurent Pinchart]

Hi Bastian,

On Friday 01 November 2013 10:47:37 Bastian Hecht wrote:
> Hi Laurent!
> 
> > +struct sci_port_info {
> > +       unsigned int type;
> > +       unsigned int regtype;
> > +       unsigned int brr_algo;
> > +};
> > +
> > +static const struct of_device_id of_sci_match[] = {
> > +       {
> > +               .compatible = "renesas,scif-generic",
> > +               .data = (void *)&(const struct sci_port_info) {
> > +                       .type = PORT_SCIF,
> > +                       .regtype = SCIx_SH4_SCIF_REGTYPE,
> > +                       .brr_algo = SCBRR_ALGO_2,
> > +               },
> > +       }, {
> > +               .compatible = "renesas,scifa-generic",
> > +               .data = (void *)&(const struct sci_port_info) {
> > +                       .type = PORT_SCIFA,
> > +                       .regtype = SCIx_SCIFA_REGTYPE,
> > +                       .brr_algo = SCBRR_ALGO_4,
> > +               },
> > +       }, {
> > +               .compatible = "renesas,scifb-generic",
> > +               .data = (void *)&(const struct sci_port_info) {
> > +                       .type = PORT_SCIFB,
> > +                       .regtype = SCIx_SCIFB_REGTYPE,
> > +                       .brr_algo = SCBRR_ALGO_4,
> > +               },
> > +       }, {
> > +               .compatible = "renesas,hscif-generic",
> > +               .data = (void *)&(const struct sci_port_info) {
> > +                       .type = PORT_HSCIF,
> > +                       .regtype = SCIx_HSCIF_REGTYPE,
> > +                       .brr_algo = SCBRR_ALGO_6,
> > +               },
> > +       }, {
> > +               /* Terminator */
> > +       },
> > +};
> > +MODULE_DEVICE_TABLE(of, of_sci_match);
> 
> I like this version way better than my initial approach,

Thank you.

> given that we only support ARM based ports and we hope the regtype and
> brr_algo won't deviate a lot from the given generic settings for new chips

Hopefully :-) I'm trying to simplify the baud rate algorithms mess, I should 
be able to post patches soon, but I need to test them on hardware first. Would 
you have a sh7780, sh7785 or sh7786 board by any chance ?

> - else the of_sci_match[] array might become a bit bloated. This was the
> former reasoning to introduce all the clumsy OF fields.
> 
> I'm happy that the sci OF support will finally make it into the mainline,
> thanks Laurent!

You're welcome.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/5] serial: sh-sci: Add OF support

[Laurent Pinchart]

Hi Bastian,

On Friday 01 November 2013 10:47:37 Bastian Hecht wrote:
> Hi Laurent!
> 
> > +struct sci_port_info {
> > +       unsigned int type;
> > +       unsigned int regtype;
> > +       unsigned int brr_algo;
> > +};
> > +
> > +static const struct of_device_id of_sci_match[] = {
> > +       {
> > +               .compatible = "renesas,scif-generic",
> > +               .data = (void *)&(const struct sci_port_info) {
> > +                       .type = PORT_SCIF,
> > +                       .regtype = SCIx_SH4_SCIF_REGTYPE,
> > +                       .brr_algo = SCBRR_ALGO_2,
> > +               },
> > +       }, {
> > +               .compatible = "renesas,scifa-generic",
> > +               .data = (void *)&(const struct sci_port_info) {
> > +                       .type = PORT_SCIFA,
> > +                       .regtype = SCIx_SCIFA_REGTYPE,
> > +                       .brr_algo = SCBRR_ALGO_4,
> > +               },
> > +       }, {
> > +               .compatible = "renesas,scifb-generic",
> > +               .data = (void *)&(const struct sci_port_info) {
> > +                       .type = PORT_SCIFB,
> > +                       .regtype = SCIx_SCIFB_REGTYPE,
> > +                       .brr_algo = SCBRR_ALGO_4,
> > +               },
> > +       }, {
> > +               .compatible = "renesas,hscif-generic",
> > +               .data = (void *)&(const struct sci_port_info) {
> > +                       .type = PORT_HSCIF,
> > +                       .regtype = SCIx_HSCIF_REGTYPE,
> > +                       .brr_algo = SCBRR_ALGO_6,
> > +               },
> > +       }, {
> > +               /* Terminator */
> > +       },
> > +};
> > +MODULE_DEVICE_TABLE(of, of_sci_match);
> 
> I like this version way better than my initial approach,

Thank you.

> given that we only support ARM based ports and we hope the regtype and
> brr_algo won't deviate a lot from the given generic settings for new chips

Hopefully :-) I'm trying to simplify the baud rate algorithms mess, I should 
be able to post patches soon, but I need to test them on hardware first. Would 
you have a sh7780, sh7785 or sh7786 board by any chance ?

> - else the of_sci_match[] array might become a bit bloated. This was the
> former reasoning to introduce all the clumsy OF fields.
> 
> I'm happy that the sci OF support will finally make it into the mainline,
> thanks Laurent!

You're welcome.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/5] serial: sh-sci: Add OF support

[Bastian Hecht]

>> given that we only support ARM based ports and we hope the regtype and
>> brr_algo won't deviate a lot from the given generic settings for new chips
>
> Hopefully :-) I'm trying to simplify the baud rate algorithms mess, I should
> be able to post patches soon, but I need to test them on hardware first. Would
> you have a sh7780, sh7785 or sh7786 board by any chance ?

No never touched these unfortunately.

Cheers,

 Bastian

Re: [PATCH 4/5] serial: sh-sci: Add OF support

[Bastian Hecht]

>> given that we only support ARM based ports and we hope the regtype and
>> brr_algo won't deviate a lot from the given generic settings for new chips
>
> Hopefully :-) I'm trying to simplify the baud rate algorithms mess, I should
> be able to post patches soon, but I need to test them on hardware first. Would
> you have a sh7780, sh7785 or sh7786 board by any chance ?

No never touched these unfortunately.

Cheers,

 Bastian