[PATCH v6 1/3] serial: sh-sci: Add OF support

[PATCH v6 1/3] serial: sh-sci: Add OF support

[Bastian Hecht]

We add the capabilty to probe Renesas SCI devices using Device Tree setup.

Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
Reviewed-by: Paul Mundt <lethal@linux-sh.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
v6:
putting it all together: I posted v5 as an incremental patch - here
the full version based on Simon Horman's tree topic/intc-of.

I will post another incremental version as well.

- rename sci at xxxxxxxx to serial at xxxxxxxx
- stick to scbrr-algorithm

 .../bindings/tty/serial/renesas,sci-serial.txt     |   47 +++++++
 drivers/tty/serial/sh-sci.c                        |  146 +++++++++++++++++++-
 include/linux/serial_sci.h                         |    4 +
 3 files changed, 193 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt

diff --git a/Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt
new file mode 100644
index 0000000..d80039e
--- /dev/null
+++ b/Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt
@@ -0,0 +1,47 @@
+* Renesas SH-Mobile Serial Communication Interface
+
+Required properties:
+- compatible : Should be "renesas,sci-<port type>-uart", where <port type> is
+  "sci", "scif", "irda", "scifa", "scifb"
+  or for legacy devices
+  "sh2_scif_fifodata", "sh3_scif", "sh4_scif", "sh4_scif_no_scsptr",
+  "sh4_scif_fifodata", "sh7705_scif".
+- reg : Address and length of the register set for the device
+- interrupts : Should contain the following IRQs in this order:
+	ERI: receive-error interrupt
+	RXI: receive-FIFO-data-full interrupt
+	TXI: transmit-FIFO-data-empty interrupt
+	BRI: break reception interrupt
+- interrupt-names: The IRQ names "eri", "rxi", "txi" and "bri".
+- cell-index : The device id.
+- renesas,scscr : Should contain a bitfield used by the Serial Control Register.
+  b7 = SCSCR_TIE
+  b6 = SCSCR_RIE
+  b5 = SCSCR_TE
+  b4 = SCSCR_RE
+  b3 = SCSCR_REIE
+  b2 = SCSCR_TOIE
+  b1 = SCSCR_CKE1
+  b0 = SCSCR_CKE0
+- renesas,scbrr-algorithm : Choose an algorithm ID for the baud rate generator.
+  1 = SCBRR_ALGO_1 ((clk + 16 * bps) / (16 * bps) - 1)
+  2 = SCBRR_ALGO_2 ((clk + 16 * bps) / (32 * bps) - 1)
+  3 = SCBRR_ALGO_3 (((clk * 2) + 16 * bps) / (16 * bps) - 1)
+  4 = SCBRR_ALGO_4 (((clk * 2) + 16 * bps) / (32 * bps) - 1)
+  5 = SCBRR_ALGO_5 (((clk * 1000 / 32) / bps) - 1)
+
+Optional properties:
+- renesas,autoconf : Set if device is capable of auto configuration.
+
+Example:
+	serial at e6c50000 {
+		compatible = "renesas,sci-scifa-uart";
+		interrupt-parent = <&intca>;
+		reg = <0xe6c50000 0x100>;
+		interrupts = <0x0c20>, <0x0c20>, <0x0c20>, <0x0c20>;
+		interrupt-names = "eri", "rxi", "txi", "bri";
+		cell-index = <1>;
+		renesas,scscr = <0x30>;
+		renesas,scbrr-algorithm = <4>;
+		renesas,autoconf;
+	};
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 6147756..03bb740 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -52,6 +52,7 @@
 #include <linux/scatterlist.h>
 #include <linux/slab.h>
 #include <linux/gpio.h>
+#include <linux/of.h>
 
 #ifdef CONFIG_SUPERH
 #include <asm/sh_bios.h>
@@ -2353,6 +2354,131 @@ static int sci_remove(struct platform_device *dev)
 	return 0;
 }
 
+static const struct of_device_id of_sci_match[] = {
+	{ .compatible = "renesas,sci-sci-uart",
+		.data = (void *)SCIx_SCI_REGTYPE },
+	{ .compatible = "renesas,sci-scif-uart",
+		.data = (void *)SCIx_SH4_SCIF_REGTYPE, },
+	{ .compatible = "renesas,sci-irda-uart",
+		.data = (void *)SCIx_IRDA_REGTYPE },
+	{ .compatible = "renesas,sci-scifa-uart",
+		.data = (void *)SCIx_SCIFA_REGTYPE },
+	{ .compatible = "renesas,sci-scifb-uart",
+		.data = (void *)SCIx_SCIFB_REGTYPE },
+	{ .compatible = "renesas,sci-sh2_scif_fifodata-uart",
+		.data = (void *)SCIx_SH2_SCIF_FIFODATA_REGTYPE },
+	{ .compatible = "renesas,sci-sh3_scif-uart",
+		.data = (void *)SCIx_SH3_SCIF_REGTYPE },
+	{ .compatible = "renesas,sci-sh4_scif-uart",
+		.data = (void *)SCIx_SH4_SCIF_REGTYPE },
+	{ .compatible = "renesas,sci-sh4_scif_no_scsptr-uart",
+		.data = (void *)SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE },
+	{ .compatible = "renesas,sci-sh4_scif_fifodata-uart",
+		.data = (void *)SCIx_SH4_SCIF_FIFODATA_REGTYPE },
+	{ .compatible = "renesas,sci-sh7705_scif-uart",
+		.data = (void *)SCIx_SH7705_SCIF_REGTYPE },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_sci_match);
+
+static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev,
+								int *dev_id)
+{
+	struct plat_sci_port *p;
+	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *match;
+	struct resource *res;
+	const __be32 *prop;
+	int i, irq, val;
+
+	if (!IS_ENABLED(CONFIG_OF) || !np)
+		return NULL;
+
+	match = of_match_node(of_sci_match, pdev->dev.of_node);
+	if (!match || !match->data) {
+		dev_err(&pdev->dev, "OF match error\n");
+		return NULL;
+	}
+
+	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;
+
+	for (i = 0; i < SCIx_NR_IRQS; i++) {
+		irq = platform_get_irq(pdev, i);
+		if (irq < 0) {
+			dev_err(&pdev->dev, "failed to get irq data %d\n", i);
+			return NULL;
+		}
+		p->irqs[i] = irq;
+	}
+
+	prop = of_get_property(np, "cell-index", NULL);
+	if (!prop) {
+		dev_err(&pdev->dev, "required DT prop cell-index missing\n");
+		return NULL;
+	}
+	*dev_id = be32_to_cpup(prop);
+
+	prop = of_get_property(np, "renesas,scscr", NULL);
+	if (!prop) {
+		dev_err(&pdev->dev, "required DT prop scscr missing\n");
+		return NULL;
+	}
+	p->scscr = be32_to_cpup(prop);
+
+	prop = of_get_property(np, "renesas,scbrr-algorithm", NULL);
+	if (!prop) {
+		dev_err(&pdev->dev, "required DT prop scbrr-algorithm missing\n");
+		return NULL;
+	}
+	val = be32_to_cpup(prop);
+	if (val <= SCBRR_ALGO_INVALID || val >= SCBRR_NR_ALGOS) {
+		dev_err(&pdev->dev, "DT prop clock-algorithm out of range\n");
+		return NULL;
+	}
+	p->scbrr_algo_id = val;
+
+	p->flags = UPF_IOREMAP;
+	if (of_get_property(np, "renesas,autoconf", NULL))
+		p->flags |= UPF_BOOT_AUTOCONF;
+
+	p->regtype = (unsigned int)match->data;
+
+	switch (p->regtype) {
+	case SCIx_SCI_REGTYPE:
+		p->type = PORT_SCI;
+		break;
+	case SCIx_SH4_SCIF_REGTYPE:
+		p->type = PORT_SCIF;
+		break;
+	case SCIx_IRDA_REGTYPE:
+		p->type = PORT_IRDA;
+		break;
+	case SCIx_SCIFA_REGTYPE:
+		p->type = PORT_SCIFA;
+		break;
+	case SCIx_SCIFB_REGTYPE:
+		p->type = PORT_SCIFB;
+		break;
+	default:
+		/* legacy register sets default to PORT_SCIF */
+		p->type = PORT_SCIF;
+		break;
+	}
+
+	return p;
+}
+
 static int sci_probe_single(struct platform_device *dev,
 				      unsigned int index,
 				      struct plat_sci_port *p,
@@ -2385,9 +2511,9 @@ static int sci_probe_single(struct platform_device *dev,
 
 static int sci_probe(struct platform_device *dev)
 {
-	struct plat_sci_port *p = dev->dev.platform_data;
-	struct sci_port *sp = &sci_ports[dev->id];
-	int ret;
+	struct plat_sci_port *p;
+	struct sci_port *sp;
+	int ret, dev_id = dev->id;
 
 	/*
 	 * If we've come here via earlyprintk initialization, head off to
@@ -2397,9 +2523,20 @@ 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);
+	else
+		p = dev->dev.platform_data;
+
+	if (!p) {
+		dev_err(&dev->dev, "no setup data supplied\n");
+		return -EINVAL;
+	}
+
+	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;
 
@@ -2451,6 +2588,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),
 	},
 };
 
diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
index eb763ad..857eec4 100644
--- a/include/linux/serial_sci.h
+++ b/include/linux/serial_sci.h
@@ -11,11 +11,15 @@
 #define SCIx_NOT_SUPPORTED	(-1)
 
 enum {
+	SCBRR_ALGO_INVALID,
+
 	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_5,		/* (((clk * 1000 / 32) / bps) - 1) */
+
+	SCBRR_NR_ALGOS,
 };
 
 #define SCSCR_TIE	(1 << 7)
-- 
1.7.9.5

[PATCH v6 2/3] ARM: mach-shmobile: r8a7740: Add DT names to clock list

[Bastian Hecht]

This adds temporarily the alternative device names to the clock list
that are used when booting via Device Tree setup.

Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
---
 arch/arm/mach-shmobile/clock-r8a7740.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/mach-shmobile/clock-r8a7740.c b/arch/arm/mach-shmobile/clock-r8a7740.c
index 71fc400..b9d6312 100644
--- a/arch/arm/mach-shmobile/clock-r8a7740.c
+++ b/arch/arm/mach-shmobile/clock-r8a7740.c
@@ -593,18 +593,27 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("sh_mobile_ceu.1",	&mstp_clks[MSTP128]),
 
 	CLKDEV_DEV_ID("sh-sci.4",		&mstp_clks[MSTP200]),
+	CLKDEV_DEV_ID("e6c80000.serial",	&mstp_clks[MSTP200]),
 	CLKDEV_DEV_ID("sh-sci.3",		&mstp_clks[MSTP201]),
+	CLKDEV_DEV_ID("e6c70000.serial",	&mstp_clks[MSTP201]),
 	CLKDEV_DEV_ID("sh-sci.2",		&mstp_clks[MSTP202]),
+	CLKDEV_DEV_ID("e6c60000.serial",	&mstp_clks[MSTP202]),
 	CLKDEV_DEV_ID("sh-sci.1",		&mstp_clks[MSTP203]),
+	CLKDEV_DEV_ID("e6c50000.serial",	&mstp_clks[MSTP203]),
 	CLKDEV_DEV_ID("sh-sci.0",		&mstp_clks[MSTP204]),
+	CLKDEV_DEV_ID("e6c40000.serial",	&mstp_clks[MSTP204]),
 	CLKDEV_DEV_ID("sh-sci.8",		&mstp_clks[MSTP206]),
+	CLKDEV_DEV_ID("e6c30000.serial",	&mstp_clks[MSTP206]),
 	CLKDEV_DEV_ID("sh-sci.5",		&mstp_clks[MSTP207]),
+	CLKDEV_DEV_ID("e6cb0000.serial",	&mstp_clks[MSTP207]),
 	CLKDEV_DEV_ID("sh-dma-engine.3",	&mstp_clks[MSTP214]),
 	CLKDEV_DEV_ID("sh-dma-engine.2",	&mstp_clks[MSTP216]),
 	CLKDEV_DEV_ID("sh-dma-engine.1",	&mstp_clks[MSTP217]),
 	CLKDEV_DEV_ID("sh-dma-engine.0",	&mstp_clks[MSTP218]),
 	CLKDEV_DEV_ID("sh-sci.7",		&mstp_clks[MSTP222]),
+	CLKDEV_DEV_ID("e6cd0000.serial",	&mstp_clks[MSTP222]),
 	CLKDEV_DEV_ID("sh-sci.6",		&mstp_clks[MSTP230]),
+	CLKDEV_DEV_ID("e6cc0000.serial",	&mstp_clks[MSTP230]),
 
 	CLKDEV_DEV_ID("sh_cmt.10",		&mstp_clks[MSTP329]),
 	CLKDEV_DEV_ID("sh_fsi2",		&mstp_clks[MSTP328]),
-- 
1.7.9.5

[PATCH v6 3/3] ARM: mach-shmobile: r8a7740: Setup the serial devices using DT

[Bastian Hecht]

We can now use the Device Tree for bringing up our serial devices on
the SoC r8a7740.

We remove the power domain association. We will move the info into the
DT setup as soon as we have support for it. For now this is fine as we
use the power domain governor "pm_domain_always_on_gov" for A3SP.

Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
---
 arch/arm/boot/dts/r8a7740.dtsi         |  108 +++++++++++++++++++
 arch/arm/mach-shmobile/setup-r8a7740.c |  180 --------------------------------
 2 files changed, 108 insertions(+), 180 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7740.dtsi b/arch/arm/boot/dts/r8a7740.dtsi
index 82faf52..049e54b 100644
--- a/arch/arm/boot/dts/r8a7740.dtsi
+++ b/arch/arm/boot/dts/r8a7740.dtsi
@@ -767,4 +767,112 @@
 		gpio-controller;
 		#gpio-cells = <2>;
 	};
+
+	serial at e6c40000 {
+		compatible = "renesas,sci-scifa-uart";
+		interrupt-parent = <&intca>;
+		reg = <0xe6c40000 0x100>;
+		interrupts = <0x0c00>, <0x0c00>, <0x0c00>, <0x0c00>;
+		interrupt-names = "eri", "rxi", "txi", "bri";
+		cell-index = <0>;
+		renesas,scscr = <0x30>;
+		renesas,scbrr-algorithm = <4>;
+		renesas,autoconf;
+	};
+
+	serial at e6c50000 {
+		compatible = "renesas,sci-scifa-uart";
+		interrupt-parent = <&intca>;
+		reg = <0xe6c50000 0x100>;
+		interrupts = <0x0c20>, <0x0c20>, <0x0c20>, <0x0c20>;
+		interrupt-names = "eri", "rxi", "txi", "bri";
+		cell-index = <1>;
+		renesas,scscr = <0x30>;
+		renesas,scbrr-algorithm = <4>;
+		renesas,autoconf;
+	};
+
+	serial at e6c60000 {
+		compatible = "renesas,sci-scifa-uart";
+		interrupt-parent = <&intca>;
+		reg = <0xe6c60000 0x100>;
+		interrupts = <0x0c40>, <0x0c40>, <0x0c40>, <0x0c40>;
+		interrupt-names = "eri", "rxi", "txi", "bri";
+		cell-index = <2>;
+		renesas,scscr = <0x30>;
+		renesas,scbrr-algorithm = <4>;
+		renesas,autoconf;
+	};
+
+	serial at e6c70000 {
+		compatible = "renesas,sci-scifa-uart";
+		interrupt-parent = <&intca>;
+		reg = <0xe6c70000 0x100>;
+		interrupts = <0x0c60>, <0x0c60>, <0x0c60>, <0x0c60>;
+		interrupt-names = "eri", "rxi", "txi", "bri";
+		cell-index = <3>;
+		renesas,scscr = <0x30>;
+		renesas,scbrr-algorithm = <4>;
+		renesas,autoconf;
+	};
+
+	serial at e6c80000 {
+		compatible = "renesas,sci-scifa-uart";
+		interrupt-parent = <&intca>;
+		reg = <0xe6c80000 0x100>;
+		interrupts = <0x0d20>, <0x0d20>, <0x0d20>, <0x0d20>;
+		interrupt-names = "eri", "rxi", "txi", "bri";
+		cell-index = <4>;
+		renesas,scscr = <0x30>;
+		renesas,scbrr-algorithm = <4>;
+		renesas,autoconf;
+	};
+
+	serial at e6cb0000 {
+		compatible = "renesas,sci-scifa-uart";
+		interrupt-parent = <&intca>;
+		reg = <0xe6cb0000 0x100>;
+		interrupts = <0x0d40>, <0x0d40>, <0x0d40>, <0x0d40>;
+		interrupt-names = "eri", "rxi", "txi", "bri";
+		cell-index = <5>;
+		renesas,scscr = <0x30>;
+		renesas,scbrr-algorithm = <4>;
+		renesas,autoconf;
+	};
+
+	serial at e6cc0000 {
+		compatible = "renesas,sci-scifa-uart";
+		interrupt-parent = <&intca>;
+		reg = <0xe6cc0000 0x100>;
+		interrupts = <0x04c0>, <0x04c0>, <0x04c0>, <0x04c0>;
+		interrupt-names = "eri", "rxi", "txi", "bri";
+		cell-index = <6>;
+		renesas,scscr = <0x30>;
+		renesas,scbrr-algorithm = <4>;
+		renesas,autoconf;
+	};
+
+	serial at 0xe6cd0000 {
+		compatible = "renesas,sci-scifa-uart";
+		interrupt-parent = <&intca>;
+		reg = <0xe6cd0000 0x100>;
+		interrupts = <0x04e0>, <0x04e0>, <0x04e0>, <0x04e0>;
+		interrupt-names = "eri", "rxi", "txi", "bri";
+		cell-index = <7>;
+		renesas,scscr = <0x30>;
+		renesas,scbrr-algorithm = <4>;
+		renesas,autoconf;
+	};
+
+	serial at e6c30000 {
+		compatible = "renesas,sci-scifb-uart";
+		interrupt-parent = <&intca>;
+		reg = <0xe6c30000 0x100>;
+		interrupts = <0x0d60>, <0x0d60>, <0x0d60>, <0x0d60>;
+		interrupt-names = "eri", "rxi", "txi", "bri";
+		cell-index = <8>;
+		renesas,scscr = <0x30>;
+		renesas,scbrr-algorithm = <4>;
+		renesas,autoconf;
+	};
 };
diff --git a/arch/arm/mach-shmobile/setup-r8a7740.c b/arch/arm/mach-shmobile/setup-r8a7740.c
index f35e7c2..5a563cd 100644
--- a/arch/arm/mach-shmobile/setup-r8a7740.c
+++ b/arch/arm/mach-shmobile/setup-r8a7740.c
@@ -93,168 +93,6 @@ void __init r8a7740_pinmux_init(void)
 	platform_device_register(&r8a7740_pfc_device);
 }
 
-/* SCIFA0 */
-static struct plat_sci_port scif0_platform_data = {
-	.mapbase	= 0xe6c40000,
-	.flags		= UPF_BOOT_AUTOCONF,
-	.scscr		= SCSCR_RE | SCSCR_TE,
-	.scbrr_algo_id	= SCBRR_ALGO_4,
-	.type		= PORT_SCIFA,
-	.irqs		= SCIx_IRQ_MUXED(evt2irq(0x0c00)),
-};
-
-static struct platform_device scif0_device = {
-	.name		= "sh-sci",
-	.id		= 0,
-	.dev		= {
-		.platform_data	= &scif0_platform_data,
-	},
-};
-
-/* SCIFA1 */
-static struct plat_sci_port scif1_platform_data = {
-	.mapbase	= 0xe6c50000,
-	.flags		= UPF_BOOT_AUTOCONF,
-	.scscr		= SCSCR_RE | SCSCR_TE,
-	.scbrr_algo_id	= SCBRR_ALGO_4,
-	.type		= PORT_SCIFA,
-	.irqs		= SCIx_IRQ_MUXED(evt2irq(0x0c20)),
-};
-
-static struct platform_device scif1_device = {
-	.name		= "sh-sci",
-	.id		= 1,
-	.dev		= {
-		.platform_data	= &scif1_platform_data,
-	},
-};
-
-/* SCIFA2 */
-static struct plat_sci_port scif2_platform_data = {
-	.mapbase	= 0xe6c60000,
-	.flags		= UPF_BOOT_AUTOCONF,
-	.scscr		= SCSCR_RE | SCSCR_TE,
-	.scbrr_algo_id	= SCBRR_ALGO_4,
-	.type		= PORT_SCIFA,
-	.irqs		= SCIx_IRQ_MUXED(evt2irq(0x0c40)),
-};
-
-static struct platform_device scif2_device = {
-	.name		= "sh-sci",
-	.id		= 2,
-	.dev		= {
-		.platform_data	= &scif2_platform_data,
-	},
-};
-
-/* SCIFA3 */
-static struct plat_sci_port scif3_platform_data = {
-	.mapbase	= 0xe6c70000,
-	.flags		= UPF_BOOT_AUTOCONF,
-	.scscr		= SCSCR_RE | SCSCR_TE,
-	.scbrr_algo_id	= SCBRR_ALGO_4,
-	.type		= PORT_SCIFA,
-	.irqs		= SCIx_IRQ_MUXED(evt2irq(0x0c60)),
-};
-
-static struct platform_device scif3_device = {
-	.name		= "sh-sci",
-	.id		= 3,
-	.dev		= {
-		.platform_data	= &scif3_platform_data,
-	},
-};
-
-/* SCIFA4 */
-static struct plat_sci_port scif4_platform_data = {
-	.mapbase	= 0xe6c80000,
-	.flags		= UPF_BOOT_AUTOCONF,
-	.scscr		= SCSCR_RE | SCSCR_TE,
-	.scbrr_algo_id	= SCBRR_ALGO_4,
-	.type		= PORT_SCIFA,
-	.irqs		= SCIx_IRQ_MUXED(evt2irq(0x0d20)),
-};
-
-static struct platform_device scif4_device = {
-	.name		= "sh-sci",
-	.id		= 4,
-	.dev		= {
-		.platform_data	= &scif4_platform_data,
-	},
-};
-
-/* SCIFA5 */
-static struct plat_sci_port scif5_platform_data = {
-	.mapbase	= 0xe6cb0000,
-	.flags		= UPF_BOOT_AUTOCONF,
-	.scscr		= SCSCR_RE | SCSCR_TE,
-	.scbrr_algo_id	= SCBRR_ALGO_4,
-	.type		= PORT_SCIFA,
-	.irqs		= SCIx_IRQ_MUXED(evt2irq(0x0d40)),
-};
-
-static struct platform_device scif5_device = {
-	.name		= "sh-sci",
-	.id		= 5,
-	.dev		= {
-		.platform_data	= &scif5_platform_data,
-	},
-};
-
-/* SCIFA6 */
-static struct plat_sci_port scif6_platform_data = {
-	.mapbase	= 0xe6cc0000,
-	.flags		= UPF_BOOT_AUTOCONF,
-	.scscr		= SCSCR_RE | SCSCR_TE,
-	.scbrr_algo_id	= SCBRR_ALGO_4,
-	.type		= PORT_SCIFA,
-	.irqs		= SCIx_IRQ_MUXED(evt2irq(0x04c0)),
-};
-
-static struct platform_device scif6_device = {
-	.name		= "sh-sci",
-	.id		= 6,
-	.dev		= {
-		.platform_data	= &scif6_platform_data,
-	},
-};
-
-/* SCIFA7 */
-static struct plat_sci_port scif7_platform_data = {
-	.mapbase	= 0xe6cd0000,
-	.flags		= UPF_BOOT_AUTOCONF,
-	.scscr		= SCSCR_RE | SCSCR_TE,
-	.scbrr_algo_id	= SCBRR_ALGO_4,
-	.type		= PORT_SCIFA,
-	.irqs		= SCIx_IRQ_MUXED(evt2irq(0x04e0)),
-};
-
-static struct platform_device scif7_device = {
-	.name		= "sh-sci",
-	.id		= 7,
-	.dev		= {
-		.platform_data	= &scif7_platform_data,
-	},
-};
-
-/* SCIFB */
-static struct plat_sci_port scifb_platform_data = {
-	.mapbase	= 0xe6c30000,
-	.flags		= UPF_BOOT_AUTOCONF,
-	.scscr		= SCSCR_RE | SCSCR_TE,
-	.scbrr_algo_id	= SCBRR_ALGO_4,
-	.type		= PORT_SCIFB,
-	.irqs		= SCIx_IRQ_MUXED(evt2irq(0x0d60)),
-};
-
-static struct platform_device scifb_device = {
-	.name		= "sh-sci",
-	.id		= 8,
-	.dev		= {
-		.platform_data	= &scifb_platform_data,
-	},
-};
-
 /* CMT */
 static struct sh_timer_config cmt10_platform_data = {
 	.name = "CMT10",
@@ -379,15 +217,6 @@ static struct platform_device tmu02_device = {
 };
 
 static struct platform_device *r8a7740_devices_dt[] __initdata = {
-	&scif0_device,
-	&scif1_device,
-	&scif2_device,
-	&scif3_device,
-	&scif4_device,
-	&scif5_device,
-	&scif6_device,
-	&scif7_device,
-	&scifb_device,
 	&cmt10_device,
 	&tmu00_device,
 	&tmu01_device,
@@ -812,15 +641,6 @@ void __init r8a7740_add_standard_devices(void)
 
 	/* add devices to PM domain  */
 
-	rmobile_add_device_to_domain("A3SP",	&scif0_device);
-	rmobile_add_device_to_domain("A3SP",	&scif1_device);
-	rmobile_add_device_to_domain("A3SP",	&scif2_device);
-	rmobile_add_device_to_domain("A3SP",	&scif3_device);
-	rmobile_add_device_to_domain("A3SP",	&scif4_device);
-	rmobile_add_device_to_domain("A3SP",	&scif5_device);
-	rmobile_add_device_to_domain("A3SP",	&scif6_device);
-	rmobile_add_device_to_domain("A3SP",	&scif7_device);
-	rmobile_add_device_to_domain("A3SP",	&scifb_device);
 	rmobile_add_device_to_domain("A3SP",	&i2c1_device);
 }
 
-- 
1.7.9.5

[PATCH v6 1/3] serial: sh-sci: Add OF support

[Laurent Pinchart]

Hi Bastian,

Thanks for the patch.

On Wednesday 06 March 2013 12:30:35 Bastian Hecht wrote:
> We add the capabilty to probe Renesas SCI devices using Device Tree setup.
> 
> Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
> Reviewed-by: Paul Mundt <lethal@linux-sh.org>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v6:
> putting it all together: I posted v5 as an incremental patch - here
> the full version based on Simon Horman's tree topic/intc-of.
> 
> I will post another incremental version as well.
> 
> - rename sci at xxxxxxxx to serial at xxxxxxxx
> - stick to scbrr-algorithm
> 
>  .../bindings/tty/serial/renesas,sci-serial.txt     |   47 +++++++
>  drivers/tty/serial/sh-sci.c                        |  146
> +++++++++++++++++++- include/linux/serial_sci.h                         |  
>  4 +
>  3 files changed, 193 insertions(+), 4 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt
> 
> diff --git
> a/Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt
> b/Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt new
> file mode 100644
> index 0000000..d80039e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt
> @@ -0,0 +1,47 @@
> +* Renesas SH-Mobile Serial Communication Interface
> +
> +Required properties:
> +- compatible : Should be "renesas,sci-<port type>-uart", where <port type>
> is +  "sci", "scif", "irda", "scifa", "scifb"
> +  or for legacy devices
> +  "sh2_scif_fifodata", "sh3_scif", "sh4_scif", "sh4_scif_no_scsptr",
> +  "sh4_scif_fifodata", "sh7705_scif".
> +- reg : Address and length of the register set for the device
> +- interrupts : Should contain the following IRQs in this order:
> +	ERI: receive-error interrupt
> +	RXI: receive-FIFO-data-full interrupt
> +	TXI: transmit-FIFO-data-empty interrupt
> +	BRI: break reception interrupt
> +- interrupt-names: The IRQ names "eri", "rxi", "txi" and "bri".
> +- cell-index : The device id.
> +- renesas,scscr : Should contain a bitfield used by the Serial Control
> Register.
> +  b7 = SCSCR_TIE
> +  b6 = SCSCR_RIE
> +  b5 = SCSCR_TE
> +  b4 = SCSCR_RE
> +  b3 = SCSCR_REIE
> +  b2 = SCSCR_TOIE
> +  b1 = SCSCR_CKE1
> +  b0 = SCSCR_CKE0

What is that for exactly ?

> +- renesas,scbrr-algorithm : Choose an algorithm ID for the baud rate
> generator.
> +  1 = SCBRR_ALGO_1 ((clk + 16 * bps) / (16 * bps) - 1)
> +  2 = SCBRR_ALGO_2 ((clk + 16 * bps) / (32 * bps) - 1)
> +  3 = SCBRR_ALGO_3 (((clk * 2) + 16 * bps) / (16 * bps) - 1)
> +  4 = SCBRR_ALGO_4 (((clk * 2) + 16 * bps) / (32 * bps) - 1)
> +  5 = SCBRR_ALGO_5 (((clk * 1000 / 32) / bps) - 1)

Isn't it a property specific to our implementation of the driver instead of a 
hardware property ? How is one supposed to select the right algorithm ?

> +Optional properties:
> +- renesas,autoconf : Set if device is capable of auto configuration.
> +
> +Example:
> +	serial at e6c50000 {
> +		compatible = "renesas,sci-scifa-uart";
> +		interrupt-parent = <&intca>;
> +		reg = <0xe6c50000 0x100>;
> +		interrupts = <0x0c20>, <0x0c20>, <0x0c20>, <0x0c20>;
> +		interrupt-names = "eri", "rxi", "txi", "bri";
> +		cell-index = <1>;
> +		renesas,scscr = <0x30>;
> +		renesas,scbrr-algorithm = <4>;
> +		renesas,autoconf;
> +	};
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 6147756..03bb740 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -52,6 +52,7 @@
>  #include <linux/scatterlist.h>
>  #include <linux/slab.h>
>  #include <linux/gpio.h>
> +#include <linux/of.h>
> 
>  #ifdef CONFIG_SUPERH
>  #include <asm/sh_bios.h>
> @@ -2353,6 +2354,131 @@ static int sci_remove(struct platform_device *dev)
>  	return 0;
>  }
> 
> +static const struct of_device_id of_sci_match[] = {
> +	{ .compatible = "renesas,sci-sci-uart",
> +		.data = (void *)SCIx_SCI_REGTYPE },
> +	{ .compatible = "renesas,sci-scif-uart",
> +		.data = (void *)SCIx_SH4_SCIF_REGTYPE, },
> +	{ .compatible = "renesas,sci-irda-uart",
> +		.data = (void *)SCIx_IRDA_REGTYPE },
> +	{ .compatible = "renesas,sci-scifa-uart",
> +		.data = (void *)SCIx_SCIFA_REGTYPE },
> +	{ .compatible = "renesas,sci-scifb-uart",
> +		.data = (void *)SCIx_SCIFB_REGTYPE },
> +	{ .compatible = "renesas,sci-sh2_scif_fifodata-uart",
> +		.data = (void *)SCIx_SH2_SCIF_FIFODATA_REGTYPE },
> +	{ .compatible = "renesas,sci-sh3_scif-uart",
> +		.data = (void *)SCIx_SH3_SCIF_REGTYPE },
> +	{ .compatible = "renesas,sci-sh4_scif-uart",
> +		.data = (void *)SCIx_SH4_SCIF_REGTYPE },
> +	{ .compatible = "renesas,sci-sh4_scif_no_scsptr-uart",
> +		.data = (void *)SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE },
> +	{ .compatible = "renesas,sci-sh4_scif_fifodata-uart",
> +		.data = (void *)SCIx_SH4_SCIF_FIFODATA_REGTYPE },
> +	{ .compatible = "renesas,sci-sh7705_scif-uart",
> +		.data = (void *)SCIx_SH7705_SCIF_REGTYPE },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, of_sci_match);
> +
> +static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev,
> +								int *dev_id)
> +{
> +	struct plat_sci_port *p;
> +	struct device_node *np = pdev->dev.of_node;
> +	const struct of_device_id *match;
> +	struct resource *res;
> +	const __be32 *prop;
> +	int i, irq, val;
> +
> +	if (!IS_ENABLED(CONFIG_OF) || !np)
> +		return NULL;
> +
> +	match = of_match_node(of_sci_match, pdev->dev.of_node);
> +	if (!match || !match->data) {
> +		dev_err(&pdev->dev, "OF match error\n");
> +		return NULL;
> +	}
> +
> +	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;
> +
> +	for (i = 0; i < SCIx_NR_IRQS; i++) {
> +		irq = platform_get_irq(pdev, i);
> +		if (irq < 0) {
> +			dev_err(&pdev->dev, "failed to get irq data %d\n", i);
> +			return NULL;
> +		}
> +		p->irqs[i] = irq;
> +	}
> +
> +	prop = of_get_property(np, "cell-index", NULL);
> +	if (!prop) {
> +		dev_err(&pdev->dev, "required DT prop cell-index missing\n");
> +		return NULL;
> +	}
> +	*dev_id = be32_to_cpup(prop);
> +
> +	prop = of_get_property(np, "renesas,scscr", NULL);
> +	if (!prop) {
> +		dev_err(&pdev->dev, "required DT prop scscr missing\n");
> +		return NULL;
> +	}
> +	p->scscr = be32_to_cpup(prop);
> +
> +	prop = of_get_property(np, "renesas,scbrr-algorithm", NULL);
> +	if (!prop) {
> +		dev_err(&pdev->dev, "required DT prop scbrr-algorithm missing\n");
> +		return NULL;
> +	}
> +	val = be32_to_cpup(prop);
> +	if (val <= SCBRR_ALGO_INVALID || val >= SCBRR_NR_ALGOS) {
> +		dev_err(&pdev->dev, "DT prop clock-algorithm out of range\n");
> +		return NULL;
> +	}
> +	p->scbrr_algo_id = val;
> +
> +	p->flags = UPF_IOREMAP;
> +	if (of_get_property(np, "renesas,autoconf", NULL))
> +		p->flags |= UPF_BOOT_AUTOCONF;
> +
> +	p->regtype = (unsigned int)match->data;
> +
> +	switch (p->regtype) {
> +	case SCIx_SCI_REGTYPE:
> +		p->type = PORT_SCI;
> +		break;
> +	case SCIx_SH4_SCIF_REGTYPE:
> +		p->type = PORT_SCIF;
> +		break;
> +	case SCIx_IRDA_REGTYPE:
> +		p->type = PORT_IRDA;
> +		break;
> +	case SCIx_SCIFA_REGTYPE:
> +		p->type = PORT_SCIFA;
> +		break;
> +	case SCIx_SCIFB_REGTYPE:
> +		p->type = PORT_SCIFB;
> +		break;
> +	default:
> +		/* legacy register sets default to PORT_SCIF */
> +		p->type = PORT_SCIF;
> +		break;
> +	}
> +
> +	return p;
> +}
> +
>  static int sci_probe_single(struct platform_device *dev,
>  				      unsigned int index,
>  				      struct plat_sci_port *p,
> @@ -2385,9 +2511,9 @@ static int sci_probe_single(struct platform_device
> *dev,
> 
>  static int sci_probe(struct platform_device *dev)
>  {
> -	struct plat_sci_port *p = dev->dev.platform_data;
> -	struct sci_port *sp = &sci_ports[dev->id];
> -	int ret;
> +	struct plat_sci_port *p;
> +	struct sci_port *sp;
> +	int ret, dev_id = dev->id;
> 
>  	/*
>  	 * If we've come here via earlyprintk initialization, head off to
> @@ -2397,9 +2523,20 @@ 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);
> +	else
> +		p = dev->dev.platform_data;
> +
> +	if (!p) {
> +		dev_err(&dev->dev, "no setup data supplied\n");
> +		return -EINVAL;
> +	}
> +
> +	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;
> 
> @@ -2451,6 +2588,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),
>  	},
>  };
> 
> diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
> index eb763ad..857eec4 100644
> --- a/include/linux/serial_sci.h
> +++ b/include/linux/serial_sci.h
> @@ -11,11 +11,15 @@
>  #define SCIx_NOT_SUPPORTED	(-1)
> 
>  enum {
> +	SCBRR_ALGO_INVALID,
> +
>  	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_5,		/* (((clk * 1000 / 32) / bps) - 1) */
> +
> +	SCBRR_NR_ALGOS,
>  };
> 
>  #define SCSCR_TIE	(1 << 7)
-- 
Regards,

Laurent Pinchart

[PATCH v6 1/3] serial: sh-sci: Add OF support

[Laurent Pinchart]

On Wednesday 06 March 2013 13:10:55 Laurent Pinchart wrote:
> On Wednesday 06 March 2013 12:30:35 Bastian Hecht wrote:
> > We add the capabilty to probe Renesas SCI devices using Device Tree setup.
> > 
> > Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
> > Reviewed-by: Paul Mundt <lethal@linux-sh.org>
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > v6:
> > putting it all together: I posted v5 as an incremental patch - here
> > the full version based on Simon Horman's tree topic/intc-of.
> > 
> > I will post another incremental version as well.
> > 
> > - rename sci at xxxxxxxx to serial at xxxxxxxx
> > - stick to scbrr-algorithm
> > 
> >  .../bindings/tty/serial/renesas,sci-serial.txt     |   47 +++++++
> >  drivers/tty/serial/sh-sci.c                        |  146 ++++++++++++++-
> >  include/linux/serial_sci.h                         |    4 +
> >  3 files changed, 193 insertions(+), 4 deletions(-)
> >  create mode 100644
> > 
> > Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt
> > b/Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt new
> > file mode 100644
> > index 0000000..d80039e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt
> > @@ -0,0 +1,47 @@
> > +* Renesas SH-Mobile Serial Communication Interface
> > +
> > +Required properties:
> > +- compatible : Should be "renesas,sci-<port type>-uart", where <port
> > type>
> > is +  "sci", "scif", "irda", "scifa", "scifb"
> > +  or for legacy devices
> > +  "sh2_scif_fifodata", "sh3_scif", "sh4_scif", "sh4_scif_no_scsptr",
> > +  "sh4_scif_fifodata", "sh7705_scif".
> > +- reg : Address and length of the register set for the device
> > +- interrupts : Should contain the following IRQs in this order:
> > +	ERI: receive-error interrupt
> > +	RXI: receive-FIFO-data-full interrupt
> > +	TXI: transmit-FIFO-data-empty interrupt
> > +	BRI: break reception interrupt
> > +- interrupt-names: The IRQ names "eri", "rxi", "txi" and "bri".
> > +- cell-index : The device id.
> > +- renesas,scscr : Should contain a bitfield used by the Serial Control
> > Register.
> > +  b7 = SCSCR_TIE
> > +  b6 = SCSCR_RIE
> > +  b5 = SCSCR_TE
> > +  b4 = SCSCR_RE
> > +  b3 = SCSCR_REIE
> > +  b2 = SCSCR_TOIE
> > +  b1 = SCSCR_CKE1
> > +  b0 = SCSCR_CKE0
> 
> What is that for exactly ?
> 
> > +- renesas,scbrr-algorithm : Choose an algorithm ID for the baud rate
> > generator.
> > +  1 = SCBRR_ALGO_1 ((clk + 16 * bps) / (16 * bps) - 1)
> > +  2 = SCBRR_ALGO_2 ((clk + 16 * bps) / (32 * bps) - 1)
> > +  3 = SCBRR_ALGO_3 (((clk * 2) + 16 * bps) / (16 * bps) - 1)
> > +  4 = SCBRR_ALGO_4 (((clk * 2) + 16 * bps) / (32 * bps) - 1)
> > +  5 = SCBRR_ALGO_5 (((clk * 1000 / 32) / bps) - 1)
> 
> Isn't it a property specific to our implementation of the driver instead of
> a hardware property ? How is one supposed to select the right algorithm ?

I realize I haven't expressed myself very clearly here. What I mean is that 
expressing how the baud rate generator works internally using an algorithm ID 
is pretty specific to the Linux driver.

I assume that, for a given serial port on a given SoC, the algorithm that 
matches the baud rate generator hardware implementation needs to be selected. 
Can't this information be inferred from the compatible string and port number 
?

-- 
Regards,

Laurent Pinchart

[PATCH v6 1/3] serial: sh-sci: Add OF support

[Paul Mundt]

On Wed, Mar 06, 2013 at 01:25:16PM +0100, Laurent Pinchart wrote:
> On Wednesday 06 March 2013 13:10:55 Laurent Pinchart wrote:
> > On Wednesday 06 March 2013 12:30:35 Bastian Hecht wrote:
> > > +- renesas,scbrr-algorithm : Choose an algorithm ID for the baud rate
> > > generator.
> > > +  1 = SCBRR_ALGO_1 ((clk + 16 * bps) / (16 * bps) - 1)
> > > +  2 = SCBRR_ALGO_2 ((clk + 16 * bps) / (32 * bps) - 1)
> > > +  3 = SCBRR_ALGO_3 (((clk * 2) + 16 * bps) / (16 * bps) - 1)
> > > +  4 = SCBRR_ALGO_4 (((clk * 2) + 16 * bps) / (32 * bps) - 1)
> > > +  5 = SCBRR_ALGO_5 (((clk * 1000 / 32) / bps) - 1)
> > 
> > Isn't it a property specific to our implementation of the driver instead of
> > a hardware property ? How is one supposed to select the right algorithm ?
> 
> I realize I haven't expressed myself very clearly here. What I mean is that 
> expressing how the baud rate generator works internally using an algorithm ID 
> is pretty specific to the Linux driver.
> 
> I assume that, for a given serial port on a given SoC, the algorithm that 
> matches the baud rate generator hardware implementation needs to be selected. 
> Can't this information be inferred from the compatible string and port number 
> ?
> 
Now that we have split things out in to regtype for the compat string we
may be able to get away with selecting defaults that match the regtype
and drop it from the binding (assuming all future parts remain relatively
sane), but it would still complicate things if we were to ever do DT
support for legacy CPUs. It's not enough to tie to the port type, as
there are plenty of cases where the port type uses a non-standard algo on
some CPU subtypes (although things are pretty consistent for the arm side
so far).

[PATCH v6 1/3] serial: sh-sci: Add OF support

[Bastian Hecht]

Hi Paul,

2013/3/6 Paul Mundt <lethal@linux-sh.org>:
> On Wed, Mar 06, 2013 at 01:25:16PM +0100, Laurent Pinchart wrote:
>> On Wednesday 06 March 2013 13:10:55 Laurent Pinchart wrote:
>> > On Wednesday 06 March 2013 12:30:35 Bastian Hecht wrote:
>> > > +- renesas,scbrr-algorithm : Choose an algorithm ID for the baud rate
>> > > generator.
>> > > +  1 = SCBRR_ALGO_1 ((clk + 16 * bps) / (16 * bps) - 1)
>> > > +  2 = SCBRR_ALGO_2 ((clk + 16 * bps) / (32 * bps) - 1)
>> > > +  3 = SCBRR_ALGO_3 (((clk * 2) + 16 * bps) / (16 * bps) - 1)
>> > > +  4 = SCBRR_ALGO_4 (((clk * 2) + 16 * bps) / (32 * bps) - 1)
>> > > +  5 = SCBRR_ALGO_5 (((clk * 1000 / 32) / bps) - 1)
>> >
>> > Isn't it a property specific to our implementation of the driver instead of
>> > a hardware property ? How is one supposed to select the right algorithm ?
>>
>> I realize I haven't expressed myself very clearly here. What I mean is that
>> expressing how the baud rate generator works internally using an algorithm ID
>> is pretty specific to the Linux driver.
>>
>> I assume that, for a given serial port on a given SoC, the algorithm that
>> matches the baud rate generator hardware implementation needs to be selected.
>> Can't this information be inferred from the compatible string and port number
>> ?
>>
> Now that we have split things out in to regtype for the compat string we
> may be able to get away with selecting defaults that match the regtype
> and drop it from the binding (assuming all future parts remain relatively
> sane), but it would still complicate things if we were to ever do DT
> support for legacy CPUs. It's not enough to tie to the port type, as
> there are plenty of cases where the port type uses a non-standard algo on
> some CPU subtypes (although things are pretty consistent for the arm side
> so far).

regarding Laurent's question about the SCSCR register, do you think we
can assume the same there?
If so we could start out omitting these 2 properties and rely on
adding optional properties later in case we want to support legacy
stuff.
I don't see where it complicates things as we probably got the minimal
binding set then.

Thanks,

 Bastian

[PATCH v6 1/3] serial: sh-sci: Add OF support

[Arnd Bergmann]

On Wednesday 06 March 2013, Paul Mundt wrote:
> Now that we have split things out in to regtype for the compat string we
> may be able to get away with selecting defaults that match the regtype
> and drop it from the binding (assuming all future parts remain relatively
> sane), but it would still complicate things if we were to ever do DT
> support for legacy CPUs. It's not enough to tie to the port type, as
> there are plenty of cases where the port type uses a non-standard algo on
> some CPU subtypes (although things are pretty consistent for the arm side
> so far).

Maybe we could document it as an optional property that we may use for the
legacy CPUs if necessary, but put it into the ARM dts files or into
the driver implementation for the time being.

	Arnd

[PATCH v6 1/3] serial: sh-sci: Add OF support

[Bastian Hecht]

Hi Laurent,

>> +- renesas,scscr : Should contain a bitfield used by the Serial Control
>> Register.
>> +  b7 = SCSCR_TIE
>> +  b6 = SCSCR_RIE
>> +  b5 = SCSCR_TE
>> +  b4 = SCSCR_RE
>> +  b3 = SCSCR_REIE
>> +  b2 = SCSCR_TOIE
>> +  b1 = SCSCR_CKE1
>> +  b0 = SCSCR_CKE0
>
> What is that for exactly ?

What I can see from 3 different datasheets (sh7372, sh73a0, r8a7740)
the first 2 bits differ in meaning.

On r8a7740 it depends: In asynchronous mode the first 2 bits CKE0/1
define whether you want to use an external clock to drive the baud
generator or if you want to use the internal SUB clock. If you use the
SUB clock you can further define if you want to use a subscaled SUB
clock in the SCSMR register. In synchronous mode you must rely on the
internal clock for the baud generator and can select if the SCK pin is
used as clock input or output.

On sh73a0 and sh7372 it's used to control the output clock SCK (set it
to high impedance or actually output a clock when in sychronous mode)

Bit 2 and 3 don't exist in my datasheets.

The other bits define under which conditions you receive interrupts
(send FIFO empty, receive FIFO full) and which bits you need to start
transfers (one bit to start sending, one receiving). The bits 8 to 31
are used to set up DMA transfers and various interrupt events like
error status and finish transfers. I haven't included them as they are
not used in the driver.

Thanks,

Bastian

[PATCH v6 1/3] serial: sh-sci: Add OF support

[Laurent Pinchart]

Hi Bastian,

On Thursday 07 March 2013 11:26:29 Bastian Hecht wrote:
> Hi Laurent,
> 
> >> +- renesas,scscr : Should contain a bitfield used by the Serial Control
> >> Register.
> >> +  b7 = SCSCR_TIE
> >> +  b6 = SCSCR_RIE
> >> +  b5 = SCSCR_TE
> >> +  b4 = SCSCR_RE
> >> +  b3 = SCSCR_REIE
> >> +  b2 = SCSCR_TOIE
> >> +  b1 = SCSCR_CKE1
> >> +  b0 = SCSCR_CKE0
> > 
> > What is that for exactly ?
> 
> What I can see from 3 different datasheets (sh7372, sh73a0, r8a7740)
> the first 2 bits differ in meaning.
> 
> On r8a7740 it depends: In asynchronous mode the first 2 bits CKE0/1
> define whether you want to use an external clock to drive the baud
> generator or if you want to use the internal SUB clock. If you use the
> SUB clock you can further define if you want to use a subscaled SUB
> clock in the SCSMR register. In synchronous mode you must rely on the
> internal clock for the baud generator and can select if the SCK pin is
> used as clock input or output.

What about adding an optional source clock property to the bindings, that 
would contain the phandle of the source clock ? If the property is absent then 
the internal clock would be used.

How is subscaling used in practice ?

> On sh73a0 and sh7372 it's used to control the output clock SCK (set it
> to high impedance or actually output a clock when in sychronous mode)

Maybe we could add a synchronous mode property to the DT bindings and use it 
to infer the bit values.

> Bit 2 and 3 don't exist in my datasheets.
> 
> The other bits define under which conditions you receive interrupts
> (send FIFO empty, receive FIFO full) and which bits you need to start
> transfers (one bit to start sending, one receiving). The bits 8 to 31
> are used to set up DMA transfers and various interrupt events like
> error status and finish transfers. I haven't included them as they are
> not used in the driver.

If they're not used maybe we can drop them :-)

-- 
Regards,

Laurent Pinchart

[PATCH v6 1/3] serial: sh-sci: Add OF support

[Paul Mundt]

On Mon, Mar 11, 2013 at 11:32:31PM +0100, Laurent Pinchart wrote:
> On Thursday 07 March 2013 11:26:29 Bastian Hecht wrote:
> > >> +- renesas,scscr : Should contain a bitfield used by the Serial Control
> > >> Register.
> > >> +  b7 = SCSCR_TIE
> > >> +  b6 = SCSCR_RIE
> > >> +  b5 = SCSCR_TE
> > >> +  b4 = SCSCR_RE
> > >> +  b3 = SCSCR_REIE
> > >> +  b2 = SCSCR_TOIE
> > >> +  b1 = SCSCR_CKE1
> > >> +  b0 = SCSCR_CKE0
> > > 
> > > What is that for exactly ?
> > 
> > What I can see from 3 different datasheets (sh7372, sh73a0, r8a7740)
> > the first 2 bits differ in meaning.
> > 
> > On r8a7740 it depends: In asynchronous mode the first 2 bits CKE0/1
> > define whether you want to use an external clock to drive the baud
> > generator or if you want to use the internal SUB clock. If you use the
> > SUB clock you can further define if you want to use a subscaled SUB
> > clock in the SCSMR register. In synchronous mode you must rely on the
> > internal clock for the baud generator and can select if the SCK pin is
> > used as clock input or output.
> 
> What about adding an optional source clock property to the bindings, that 
> would contain the phandle of the source clock ? If the property is absent then 
> the internal clock would be used.
> 
We can't really use the internal oscillator at the moment, it's one of
the things that is blocked on adoption of the common struct clk work. The
baud rate generator algo will likewise differ depending on whether it's
using an internally generated clock or not, which also makes a different
set of baud rates available.

My plan originally was to wait until the common struct clk stuff came
along well enough that we could provide a clock dynamically depending on
the target baud (or for avoiding cpufreq pre/post-change notifier
chains), but this obviously hasn't happened yet.

I think your idea for the abstraction with the phandle sounds like the
right approach, but we aren't presently in a state where we can handle
that.

> > Bit 2 and 3 don't exist in my datasheets.
> > 
> > The other bits define under which conditions you receive interrupts
> > (send FIFO empty, receive FIFO full) and which bits you need to start
> > transfers (one bit to start sending, one receiving). The bits 8 to 31
> > are used to set up DMA transfers and various interrupt events like
> > error status and finish transfers. I haven't included them as they are
> > not used in the driver.
> 
> If they're not used maybe we can drop them :-)
> 
Obviously they're used, otherwise they wouldn't be defined in the common
header. Whether they are used by these specific CPUs or not isn't
relevant, as there are plenty of cases where they are. grep harder.

[PATCH v6 1/3] serial: sh-sci: Add OF support

[Laurent Pinchart]

Hi Paul,

On Tuesday 12 March 2013 17:20:39 Paul Mundt wrote:
> On Mon, Mar 11, 2013 at 11:32:31PM +0100, Laurent Pinchart wrote:
> > On Thursday 07 March 2013 11:26:29 Bastian Hecht wrote:
> > > >> +- renesas,scscr : Should contain a bitfield used by the Serial
> > > >> Control
> > > >> Register.
> > > >> +  b7 = SCSCR_TIE
> > > >> +  b6 = SCSCR_RIE
> > > >> +  b5 = SCSCR_TE
> > > >> +  b4 = SCSCR_RE
> > > >> +  b3 = SCSCR_REIE
> > > >> +  b2 = SCSCR_TOIE
> > > >> +  b1 = SCSCR_CKE1
> > > >> +  b0 = SCSCR_CKE0
> > > > 
> > > > What is that for exactly ?
> > > 
> > > What I can see from 3 different datasheets (sh7372, sh73a0, r8a7740)
> > > the first 2 bits differ in meaning.
> > > 
> > > On r8a7740 it depends: In asynchronous mode the first 2 bits CKE0/1
> > > define whether you want to use an external clock to drive the baud
> > > generator or if you want to use the internal SUB clock. If you use the
> > > SUB clock you can further define if you want to use a subscaled SUB
> > > clock in the SCSMR register. In synchronous mode you must rely on the
> > > internal clock for the baud generator and can select if the SCK pin is
> > > used as clock input or output.
> > 
> > What about adding an optional source clock property to the bindings, that
> > would contain the phandle of the source clock ? If the property is absent
> > then the internal clock would be used.
> 
> We can't really use the internal oscillator at the moment, it's one of
> the things that is blocked on adoption of the common struct clk work. The
> baud rate generator algo will likewise differ depending on whether it's
> using an internally generated clock or not, which also makes a different
> set of baud rates available.
> 
> My plan originally was to wait until the common struct clk stuff came
> along well enough that we could provide a clock dynamically depending on
> the target baud (or for avoiding cpufreq pre/post-change notifier
> chains), but this obviously hasn't happened yet.
> 
> I think your idea for the abstraction with the phandle sounds like the
> right approach, but we aren't presently in a state where we can handle
> that.

Do the platforms that will be converted to DT currently need to make the clock 
source configurable ? If not we could just leave it out of the DT bindings 
until needed. Hopefully by then we'll have a common clock framework 
implementation.

> > > Bit 2 and 3 don't exist in my datasheets.
> > > 
> > > The other bits define under which conditions you receive interrupts
> > > (send FIFO empty, receive FIFO full) and which bits you need to start
> > > transfers (one bit to start sending, one receiving). The bits 8 to 31
> > > are used to set up DMA transfers and various interrupt events like
> > > error status and finish transfers. I haven't included them as they are
> > > not used in the driver.
> > 
> > If they're not used maybe we can drop them :-)
> 
> Obviously they're used, otherwise they wouldn't be defined in the common
> header. Whether they are used by these specific CPUs or not isn't
> relevant, as there are plenty of cases where they are. grep harder.

My point is that if they're only used by arm/sh/* SoCs we don't need to 
support them in DT.

-- 
Regards,

Laurent Pinchart

[PATCH v6 1/3] serial: sh-sci: Add OF support

[Bastian Hecht]

Hi,

2013/3/12 Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> Hi Paul,
>
> On Tuesday 12 March 2013 17:20:39 Paul Mundt wrote:
>> On Mon, Mar 11, 2013 at 11:32:31PM +0100, Laurent Pinchart wrote:
>> > On Thursday 07 March 2013 11:26:29 Bastian Hecht wrote:
>> > > >> +- renesas,scscr : Should contain a bitfield used by the Serial
>> > > >> Control
>> > > >> Register.
>> > > >> +  b7 = SCSCR_TIE
>> > > >> +  b6 = SCSCR_RIE
>> > > >> +  b5 = SCSCR_TE
>> > > >> +  b4 = SCSCR_RE
>> > > >> +  b3 = SCSCR_REIE
>> > > >> +  b2 = SCSCR_TOIE
>> > > >> +  b1 = SCSCR_CKE1
>> > > >> +  b0 = SCSCR_CKE0
>> > > >
>> > > > What is that for exactly ?
>> > >
>> > > What I can see from 3 different datasheets (sh7372, sh73a0, r8a7740)
>> > > the first 2 bits differ in meaning.
>> > >
>> > > On r8a7740 it depends: In asynchronous mode the first 2 bits CKE0/1
>> > > define whether you want to use an external clock to drive the baud
>> > > generator or if you want to use the internal SUB clock. If you use the
>> > > SUB clock you can further define if you want to use a subscaled SUB
>> > > clock in the SCSMR register. In synchronous mode you must rely on the
>> > > internal clock for the baud generator and can select if the SCK pin is
>> > > used as clock input or output.
>> >
>> > What about adding an optional source clock property to the bindings, that
>> > would contain the phandle of the source clock ? If the property is absent
>> > then the internal clock would be used.
>>
>> We can't really use the internal oscillator at the moment, it's one of
>> the things that is blocked on adoption of the common struct clk work. The
>> baud rate generator algo will likewise differ depending on whether it's
>> using an internally generated clock or not, which also makes a different
>> set of baud rates available.
>>
>> My plan originally was to wait until the common struct clk stuff came
>> along well enough that we could provide a clock dynamically depending on
>> the target baud (or for avoiding cpufreq pre/post-change notifier
>> chains), but this obviously hasn't happened yet.
>>
>> I think your idea for the abstraction with the phandle sounds like the
>> right approach, but we aren't presently in a state where we can handle
>> that.
>
> Do the platforms that will be converted to DT currently need to make the clock
> source configurable ? If not we could just leave it out of the DT bindings
> until needed. Hopefully by then we'll have a common clock framework
> implementation.

The r8a7779 does use the external clock (according to my r8a7740
datasheet) while the other mainline SoCs use the internal one.

>> > > Bit 2 and 3 don't exist in my datasheets.
>> > >
>> > > The other bits define under which conditions you receive interrupts
>> > > (send FIFO empty, receive FIFO full) and which bits you need to start
>> > > transfers (one bit to start sending, one receiving). The bits 8 to 31
>> > > are used to set up DMA transfers and various interrupt events like
>> > > error status and finish transfers. I haven't included them as they are
>> > > not used in the driver.
>> >
>> > If they're not used maybe we can drop them :-)
>>
>> Obviously they're used, otherwise they wouldn't be defined in the common
>> header. Whether they are used by these specific CPUs or not isn't
>> relevant, as there are plenty of cases where they are. grep harder.
>
> My point is that if they're only used by arm/sh/* SoCs we don't need to
> support them in DT.

Assuming that SCSCR_TE and SCSCR_RE are always used and we could
handle the clock flags with a phandle and the other flags are only
used in the realtime domain (where we don't want DT support?), I
suppose we could indeed drop the whole property - but we would have to
wait. Otherwise it's not so shiny but we could get it merged now. I
don't know how to proceed now, any decision makers here?

Cheers,

 Bastian


> --
> Regards,
>
> Laurent Pinchart
>

[PATCH v6 1/3] serial: sh-sci: Add OF support

[Laurent Pinchart]

Hi Bastian,

On Tuesday 19 March 2013 16:10:57 Bastian Hecht wrote:
> 2013/3/12 Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> > On Tuesday 12 March 2013 17:20:39 Paul Mundt wrote:
> >> On Mon, Mar 11, 2013 at 11:32:31PM +0100, Laurent Pinchart wrote:
> >> > On Thursday 07 March 2013 11:26:29 Bastian Hecht wrote:
> >> > > >> +- renesas,scscr : Should contain a bitfield used by the Serial
> >> > > >> Control
> >> > > >> Register.
> >> > > >> +  b7 = SCSCR_TIE
> >> > > >> +  b6 = SCSCR_RIE
> >> > > >> +  b5 = SCSCR_TE
> >> > > >> +  b4 = SCSCR_RE
> >> > > >> +  b3 = SCSCR_REIE
> >> > > >> +  b2 = SCSCR_TOIE
> >> > > >> +  b1 = SCSCR_CKE1
> >> > > >> +  b0 = SCSCR_CKE0
> >> > > > 
> >> > > > What is that for exactly ?
> >> > > 
> >> > > What I can see from 3 different datasheets (sh7372, sh73a0, r8a7740)
> >> > > the first 2 bits differ in meaning.
> >> > > 
> >> > > On r8a7740 it depends: In asynchronous mode the first 2 bits CKE0/1
> >> > > define whether you want to use an external clock to drive the baud
> >> > > generator or if you want to use the internal SUB clock. If you use
> >> > > the SUB clock you can further define if you want to use a subscaled
> >> > > SUB clock in the SCSMR register. In synchronous mode you must rely on
> >> > > the internal clock for the baud generator and can select if the SCK
> >> > > pin is used as clock input or output.
> >> > 
> >> > What about adding an optional source clock property to the bindings,
> >> > that would contain the phandle of the source clock ? If the property is
> >> > absent then the internal clock would be used.
> >> 
> >> We can't really use the internal oscillator at the moment, it's one of
> >> the things that is blocked on adoption of the common struct clk work. The
> >> baud rate generator algo will likewise differ depending on whether it's
> >> using an internally generated clock or not, which also makes a different
> >> set of baud rates available.
> >> 
> >> My plan originally was to wait until the common struct clk stuff came
> >> along well enough that we could provide a clock dynamically depending on
> >> the target baud (or for avoiding cpufreq pre/post-change notifier
> >> chains), but this obviously hasn't happened yet.
> >> 
> >> I think your idea for the abstraction with the phandle sounds like the
> >> right approach, but we aren't presently in a state where we can handle
> >> that.
> > 
> > Do the platforms that will be converted to DT currently need to make the
> > clock source configurable ? If not we could just leave it out of the DT
> > bindings until needed. Hopefully by then we'll have a common clock
> > framework implementation.
> 
> The r8a7779 does use the external clock (according to my r8a7740
> datasheet) while the other mainline SoCs use the internal one.
> 
> >> > > Bit 2 and 3 don't exist in my datasheets.
> >> > > 
> >> > > The other bits define under which conditions you receive interrupts
> >> > > (send FIFO empty, receive FIFO full) and which bits you need to start
> >> > > transfers (one bit to start sending, one receiving). The bits 8 to 31
> >> > > are used to set up DMA transfers and various interrupt events like
> >> > > error status and finish transfers. I haven't included them as they
> >> > > are not used in the driver.
> >> > 
> >> > If they're not used maybe we can drop them :-)
> >> 
> >> Obviously they're used, otherwise they wouldn't be defined in the common
> >> header. Whether they are used by these specific CPUs or not isn't
> >> relevant, as there are plenty of cases where they are. grep harder.
> > 
> > My point is that if they're only used by arm/sh/* SoCs we don't need to
> > support them in DT.
> 
> Assuming that SCSCR_TE and SCSCR_RE are always used and we could handle the
> clock flags with a phandle and the other flags are only used in the realtime
> domain (where we don't want DT support?), I suppose we could indeed drop the
> whole property - but we would have to wait. Otherwise it's not so shiny but
> we could get it merged now. I don't know how to proceed now, any decision
> makers here?

My main concern is that DT bindings are ABIs that need to be supported 
forever. This is why I'd rather not add those flags to the DT bindings now 
only to deprecate them in a couple of kernel versions.

-- 
Regards,

Laurent Pinchart