diff mbox

[v2,1/3] serial: sh-sci: Add OF support

Message ID 1361819215-4115-1-git-send-email-hechtb+renesas@gmail.com (mailing list archive)
State Superseded
Headers show

Commit Message

Bastian Hecht Feb. 25, 2013, 7:06 p.m. UTC
We add the capabilty to probe Renesas SCI devices using Device Tree setup.

Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
---
Based on the topic/all+next branch from the 17th Feb. I couldn't rebase it on the current HEAD as the armadillo-reference code currently isn't in.

changelog v2:
 - Renamed title to "Add OF support"
 - Removed fixed configurations from driver and added the following binding props:
	- renesas,scscr
	- renesas,scbrr-algo-id
	- renesas,autoconf
	- renesas,regtype
	See patch for descriptions

 .../bindings/tty/serial/renesas,sci-serial.txt     |   47 +++++++
 drivers/tty/serial/sh-sci.c                        |  131 +++++++++++++++++++-
 2 files changed, 174 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt

Comments

Paul Mundt Feb. 25, 2013, 8:35 p.m. UTC | #1
On Mon, Feb 25, 2013 at 01:06:53PM -0600, 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>
> ---
> Based on the topic/all+next branch from the 17th Feb. I couldn't rebase it on the current HEAD as the armadillo-reference code currently isn't in.
> 
> changelog v2:
>  - Renamed title to "Add OF support"
>  - Removed fixed configurations from driver and added the following binding props:
> 	- renesas,scscr
> 	- renesas,scbrr-algo-id
> 	- renesas,autoconf
> 	- renesas,regtype
> 	See patch for descriptions
> 
This looks much better, only a few minor nits.

> +Optional properties:
> +- renesas,autoconf : Set if device is capable of auto configuration
> +- renesas,regtype : Overwrite the register layout. Some legacy hardware versions
> +  use a non-default register layout. Possible layouts are
> +  0 = SCIx_SH2_SCIF_FIFODATA_REGTYPE
> +  1 = SCIx_SH3_SCIF_REGTYPE
> +  2 = SCIx_SH4_SCIF_REGTYPE
> +  3 = SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE
> +  4 = SCIx_SH4_SCIF_FIFODATA_REGTYPE
> +  5 = SCIx_SH7705_SCIF_REGTYPE
> +
Here I would still opt to provide the full regtype set, even for the
"standard" layouts, as it's quite possible that newer port-types will use
older layouts.

In this case you could keep 0 as the probe type (which would be the
default anyways, so effectively a no-op), and then do the upper bounding
via SCIx_NR_REGTYPES.

> +/* This array belongs to the DT binding definition "renesas,regtype" */
> +static const char sci_regtype_modes[] = {
> +	SCIx_SH2_SCIF_FIFODATA_REGTYPE,
> +	SCIx_SH3_SCIF_REGTYPE,
> +	SCIx_SH4_SCIF_REGTYPE,
> +	SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE,
> +	SCIx_SH4_SCIF_FIFODATA_REGTYPE,
> +	SCIx_SH7705_SCIF_REGTYPE
> +};
> +
At which point all of this can go away.

> +static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev,
> +								int *dev_id)
> +{
..
> +	prop = of_get_property(np, "renesas,regtype", NULL);
> +	if (prop) {
> +		val = be32_to_cpup(prop);
> +		if (val <= 0 || val >= ARRAY_SIZE(sci_regtype_modes)) {
> +			dev_err(&pdev->dev, "invalid DT prop regtype\n");
> +			return NULL;
> +		}
> +		p->regtype = sci_regtype_modes[be32_to_cpup(prop)];
> +	}
> +
You can then compare against SCIx_PROBE_REGTYPE and SCIx_NR_REGTYPES, and
assign p->regtype the be32_to_cpup() value directly. Which will also make
it trivial to add new regtypes in the future, we would only need to
update the binding documentation.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bastian Hecht Feb. 26, 2013, 3:09 p.m. UTC | #2
Hi Paul,

2013/2/25 Paul Mundt <lethal@linux-sh.org>:
> On Mon, Feb 25, 2013 at 01:06:53PM -0600, 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>
>> ---
>> Based on the topic/all+next branch from the 17th Feb. I couldn't rebase it on the current HEAD as the armadillo-reference code currently isn't in.
>>
>> changelog v2:
>>  - Renamed title to "Add OF support"
>>  - Removed fixed configurations from driver and added the following binding props:
>>       - renesas,scscr
>>       - renesas,scbrr-algo-id
>>       - renesas,autoconf
>>       - renesas,regtype
>>       See patch for descriptions
>>
> This looks much better, only a few minor nits.
>
>> +Optional properties:
>> +- renesas,autoconf : Set if device is capable of auto configuration
>> +- renesas,regtype : Overwrite the register layout. Some legacy hardware versions
>> +  use a non-default register layout. Possible layouts are
>> +  0 = SCIx_SH2_SCIF_FIFODATA_REGTYPE
>> +  1 = SCIx_SH3_SCIF_REGTYPE
>> +  2 = SCIx_SH4_SCIF_REGTYPE
>> +  3 = SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE
>> +  4 = SCIx_SH4_SCIF_FIFODATA_REGTYPE
>> +  5 = SCIx_SH7705_SCIF_REGTYPE
>> +
> Here I would still opt to provide the full regtype set, even for the
> "standard" layouts, as it's quite possible that newer port-types will use
> older layouts.
>
> In this case you could keep 0 as the probe type (which would be the
> default anyways, so effectively a no-op), and then do the upper bounding
> via SCIx_NR_REGTYPES.
>
>> +/* This array belongs to the DT binding definition "renesas,regtype" */
>> +static const char sci_regtype_modes[] = {
>> +     SCIx_SH2_SCIF_FIFODATA_REGTYPE,
>> +     SCIx_SH3_SCIF_REGTYPE,
>> +     SCIx_SH4_SCIF_REGTYPE,
>> +     SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE,
>> +     SCIx_SH4_SCIF_FIFODATA_REGTYPE,
>> +     SCIx_SH7705_SCIF_REGTYPE
>> +};
>> +
> At which point all of this can go away.
>
>> +static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev,
>> +                                                             int *dev_id)
>> +{
> ..
>> +     prop = of_get_property(np, "renesas,regtype", NULL);
>> +     if (prop) {
>> +             val = be32_to_cpup(prop);
>> +             if (val <= 0 || val >= ARRAY_SIZE(sci_regtype_modes)) {
>> +                     dev_err(&pdev->dev, "invalid DT prop regtype\n");
>> +                     return NULL;
>> +             }
>> +             p->regtype = sci_regtype_modes[be32_to_cpup(prop)];
>> +     }
>> +
> You can then compare against SCIx_PROBE_REGTYPE and SCIx_NR_REGTYPES, and
> assign p->regtype the be32_to_cpup() value directly. Which will also make
> it trivial to add new regtypes in the future, we would only need to
> update the binding documentation.

Ok, I slightly disliked that portion anyway in the code as we have the
full mapping from the SCBRR algo ID and only the partial mapping from
the register set. Additionally I wonder if I should add something like

enum {
+     SCBRR_ALGO_INVALID,
       SCBRR_ALGO_1,
       SCBRR_ALGO_2,
       SCBRR_ALGO_3,
       SCBRR_ALGO_4,
       SCBRR_ALGO_5,
};
...
-p->scbrr_algo_id = be32_to_cpup(prop) - 1;
+p->scbrr_algo_id = be32_to_cpup(prop);

to have a straight mapping.

Thanks,

 Bastian
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Mundt Feb. 27, 2013, 8:04 a.m. UTC | #3
On Tue, Feb 26, 2013 at 09:09:52AM -0600, Bastian Hecht wrote:
> 2013/2/25 Paul Mundt <lethal@linux-sh.org>:
> > You can then compare against SCIx_PROBE_REGTYPE and SCIx_NR_REGTYPES, and
> > assign p->regtype the be32_to_cpup() value directly. Which will also make
> > it trivial to add new regtypes in the future, we would only need to
> > update the binding documentation.
> 
> Ok, I slightly disliked that portion anyway in the code as we have the
> full mapping from the SCBRR algo ID and only the partial mapping from
> the register set. Additionally I wonder if I should add something like
> 
> enum {
> +     SCBRR_ALGO_INVALID,
>        SCBRR_ALGO_1,
>        SCBRR_ALGO_2,
>        SCBRR_ALGO_3,
>        SCBRR_ALGO_4,
>        SCBRR_ALGO_5,
> };
> ...
> -p->scbrr_algo_id = be32_to_cpup(prop) - 1;
> +p->scbrr_algo_id = be32_to_cpup(prop);
> 
> to have a straight mapping.

Yes, that would be my preferred option as well. I was going to comment on
the - 1 thing previously, but figured it was just some DT oddity.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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..f9163a3
--- /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> may be
+  SCI, SCIF, IRDA, SCIFA or SCIFB.
+- reg : Address and length of the register set for the device
+- interrupts : Should contain the following IRQs: 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-algo-id : Algorithm ID for the Bit Rate Register
+  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
+- renesas,regtype : Overwrite the register layout. Some legacy hardware versions
+  use a non-default register layout. Possible layouts are
+  0 = SCIx_SH2_SCIF_FIFODATA_REGTYPE
+  1 = SCIx_SH3_SCIF_REGTYPE
+  2 = SCIx_SH4_SCIF_REGTYPE
+  3 = SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE
+  4 = SCIx_SH4_SCIF_FIFODATA_REGTYPE
+  5 = SCIx_SH7705_SCIF_REGTYPE
+
+
+Example:
+	sci@0xe6c50000 {
+		compatible = "renesas,sci-SCIFA-uart";
+		interrupt-parent = <&intca>;
+		reg = <0xe6c50000 0x100>;
+		interrupts = <0x0c20>, <0x0c20>, <0x0c20>, <0x0c20>;
+		cell-index = <1>;
+		renesas,scscr = <0x30>;
+		renesas,scbrr-algo-id = <4>;
+		renesas,autoconf;
+	};
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 6147756..d183f3a 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2353,6 +2353,117 @@  static int sci_remove(struct platform_device *dev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id of_sci_match[] = {
+	{ .compatible = "renesas,sci-SCI-uart",
+		.data = (void *)PORT_SCI },
+	{ .compatible = "renesas,sci-SCIF-uart",
+		.data = (void *)PORT_SCIF },
+	{ .compatible = "renesas,sci-IRDA-uart",
+		.data = (void *)PORT_IRDA },
+	{ .compatible = "renesas,sci-SCIFA-uart",
+		.data = (void *)PORT_SCIFA },
+	{ .compatible = "renesas,sci-SCIFB-uart",
+		.data = (void *)PORT_SCIFB },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_sci_match);
+
+/* This array belongs to the DT binding definition "renesas,regtype" */
+static const char sci_regtype_modes[] = {
+	SCIx_SH2_SCIF_FIFODATA_REGTYPE,
+	SCIx_SH3_SCIF_REGTYPE,
+	SCIx_SH4_SCIF_REGTYPE,
+	SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE,
+	SCIx_SH4_SCIF_FIFODATA_REGTYPE,
+	SCIx_SH7705_SCIF_REGTYPE
+};
+
+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;
+
+	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-algo-id", NULL);
+	if (!prop) {
+		dev_err(&pdev->dev, "required DT prop scbrr-algo-id missing\n");
+		return NULL;
+	}
+	p->scbrr_algo_id = be32_to_cpup(prop) - 1;
+
+	p->flags = UPF_IOREMAP;
+	if (of_get_property(np, "renesas,autoconf", NULL))
+		p->flags |= UPF_BOOT_AUTOCONF;
+
+	prop = of_get_property(np, "renesas,regtype", NULL);
+	if (prop) {
+		val = be32_to_cpup(prop);
+		if (val <= 0 || val >= ARRAY_SIZE(sci_regtype_modes)) {
+			dev_err(&pdev->dev, "invalid DT prop regtype\n");
+			return NULL;
+		}
+		p->regtype = sci_regtype_modes[be32_to_cpup(prop)];
+	}
+
+	p->type = (unsigned int)match->data;
+
+	return p;
+}
+#else
+static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev,
+								int *dev_id)
+{
+	return NULL;
+}
+#endif /* CONFIG_OF */
+
 static int sci_probe_single(struct platform_device *dev,
 				      unsigned int index,
 				      struct plat_sci_port *p,
@@ -2385,9 +2496,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 +2508,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 +2573,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),
 	},
 };