diff mbox

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

Message ID 1362414054-23092-1-git-send-email-hechtb+renesas@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bastian Hecht March 4, 2013, 4:20 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>
---
v4:
Added missing OF header

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

Comments

Arnd Bergmann March 4, 2013, 4:20 p.m. UTC | #1
On Monday 04 March 2013 17:20:52 Bastian Hecht wrote:
> 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..6ad1adf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt
> @@ -0,0 +1,53 @@
> +* 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.

Why capital letters here? Maybe just list

	"renesas,sci-uart", "renesas,scif-uart", ...

> +- reg : Address and length of the register set for the device
> +- interrupts : Should contain the following IRQs: ERI, RXI, TXI and BRI.

You probably want to require the "interrupt-names" property as well
then.

> +- 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)

Maybe replace this with a "clock-frequency" property? This may
be what the registers contain, but it is not very readable.

> +Optional properties:
> +- renesas,autoconf : Set if device is capable of auto configuration
> +- renesas,regtype : Overwrite the register layout. In most cases you can rely
> +  on auto-probing (omit this property or set to 0) but some legacy devices
> +  use a non-default register layout. Possible layouts are
> +  0 = SCIx_PROBE_REGTYPE (default)
> +  1 = SCIx_SCI_REGTYPE
> +  2 = SCIx_IRDA_REGTYPE
> +  3 = SCIx_SCIFA_REGTYPE
> +  4 = SCIx_SCIFB_REGTYPE
> +  5 = SCIx_SH2_SCIF_FIFODATA_REGTYPE
> +  6 = SCIx_SH3_SCIF_REGTYPE
> +  7 = SCIx_SH4_SCIF_REGTYPE
> +  8 = SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE
> +  9 = SCIx_SH4_SCIF_FIFODATA_REGTYPE
> + 10 = SCIx_SH7705_SCIF_REGTYPE

Why do you keep these separate from the "compatible" property?
By definition, the register layout is already determined by
"compatible".

> +#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);
> +
> +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;
> +

You can remove the #ifdef by replacing it with 

	if (!IS_ENABLED(CONFIG_OF) || !np)
		return NULL;

here. This gives better compile time coverage and more readable code.

Otherwise looks very nice!

	Arnd
Bastian Hecht March 5, 2013, 12:58 p.m. UTC | #2
Hi Arnd,

thanks for having a look at it.

2013/3/4 Arnd Bergmann <arnd@arndb.de>:
> On Monday 04 March 2013 17:20:52 Bastian Hecht wrote:
>> 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..6ad1adf
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt
>> @@ -0,0 +1,53 @@
>> +* 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.
>
> Why capital letters here? Maybe just list
>
>         "renesas,sci-uart", "renesas,scif-uart", ...

Yes - changed.

>> +- reg : Address and length of the register set for the device
>> +- interrupts : Should contain the following IRQs: ERI, RXI, TXI and BRI.
>
> You probably want to require the "interrupt-names" property as well
> then.

I added the interrupt-names property but still enforce the ordering in
the binding specs.
Don't know if we want the extra overhead to scan for strings or just
take a certain order for granted.
If we want to change it to parsing, it would be better to switch
completely to platform_get_irq_byname() and change all the current
platform code, else we will produce code overhead.

>> +- 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)
>
> Maybe replace this with a "clock-frequency" property? This may
> be what the registers contain, but it is not very readable.

Hmm... do you want a frequency in absolute terms? And then calculate
the best SCBRR value for it?
I'm unsure about this, either you must exactly hit it or accept
tolerances? As something in between I renamed the property to
"renesas,clock-algorithm", but I don't know if this is what you want.

>> +Optional properties:
>> +- renesas,autoconf : Set if device is capable of auto configuration
>> +- renesas,regtype : Overwrite the register layout. In most cases you can rely
>> +  on auto-probing (omit this property or set to 0) but some legacy devices
>> +  use a non-default register layout. Possible layouts are
>> +  0 = SCIx_PROBE_REGTYPE (default)
>> +  1 = SCIx_SCI_REGTYPE
>> +  2 = SCIx_IRDA_REGTYPE
>> +  3 = SCIx_SCIFA_REGTYPE
>> +  4 = SCIx_SCIFB_REGTYPE
>> +  5 = SCIx_SH2_SCIF_FIFODATA_REGTYPE
>> +  6 = SCIx_SH3_SCIF_REGTYPE
>> +  7 = SCIx_SH4_SCIF_REGTYPE
>> +  8 = SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE
>> +  9 = SCIx_SH4_SCIF_FIFODATA_REGTYPE
>> + 10 = SCIx_SH7705_SCIF_REGTYPE
>
> Why do you keep these separate from the "compatible" property?
> By definition, the register layout is already determined by
> "compatible".

Ok, I've melted the register type and port type (defined in
include/uapi/linux/serial_core.h) definition into the compatible
property. Looks good!

>> +#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);
>> +
>> +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;
>> +
>
> You can remove the #ifdef by replacing it with
>
>         if (!IS_ENABLED(CONFIG_OF) || !np)
>                 return NULL;
>
> here. This gives better compile time coverage and more readable code.

Alright, done!

> Otherwise looks very nice!

Cheers!

 Bastian


>         Arnd
Arnd Bergmann March 5, 2013, 7:26 p.m. UTC | #3
On Tuesday 05 March 2013, Bastian Hecht wrote:
> 2013/3/4 Arnd Bergmann <arnd@arndb.de>:
> > On Monday 04 March 2013 17:20:52 Bastian Hecht wrote:
> >> +- reg : Address and length of the register set for the device
> >> +- interrupts : Should contain the following IRQs: ERI, RXI, TXI and BRI.
> >
> > You probably want to require the "interrupt-names" property as well
> > then.
> 
> I added the interrupt-names property but still enforce the ordering in
> the binding specs.

Yes, that is the correct approach.

> Don't know if we want the extra overhead to scan for strings or just
> take a certain order for granted.
> If we want to change it to parsing, it would be better to switch
> completely to platform_get_irq_byname() and change all the current
> platform code, else we will produce code overhead.

Right, there is no need to change the code for that, but I think it
makes sense to do the binding the way you describe, in case someone
later wants to move to platform_get_irq_byname. A lot of people prefer
that for some reason, although I think the traditional numbering is
just as good.

> >> +- 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)
> >
> > Maybe replace this with a "clock-frequency" property? This may
> > be what the registers contain, but it is not very readable.
> 
> Hmm... do you want a frequency in absolute terms? And then calculate
> the best SCBRR value for it?
> I'm unsure about this, either you must exactly hit it or accept
> tolerances? As something in between I renamed the property to
> "renesas,clock-algorithm", but I don't know if this is what you want.

I meant the absolute frequency in HZ, since that is what we do
for the 8250 uart and other devices, but only if that is sufficient
for your device.

	Arnd
Paul Mundt March 6, 2013, 12:50 a.m. UTC | #4
On Tue, Mar 05, 2013 at 07:26:25PM +0000, Arnd Bergmann wrote:
> On Tuesday 05 March 2013, Bastian Hecht wrote:
> > >> +- 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)
> > >
> > > Maybe replace this with a "clock-frequency" property? This may
> > > be what the registers contain, but it is not very readable.
> > 
> > Hmm... do you want a frequency in absolute terms? And then calculate
> > the best SCBRR value for it?
> > I'm unsure about this, either you must exactly hit it or accept
> > tolerances? As something in between I renamed the property to
> > "renesas,clock-algorithm", but I don't know if this is what you want.
> 
> I meant the absolute frequency in HZ, since that is what we do
> for the 8250 uart and other devices, but only if that is sufficient
> for your device.
> 
No, we still need to figure out how to generate that baud rate, whether
it needs an internal or external clock for driving the rate, etc. The
frequency in and of itself doesn't provide this information, and various
parts use different algorithms for factoring the baud rate generator,
even varying across otherwise identical ports. It's unfortunately not
possible to infer anything about the SCBRR algorithm from port type or
specified baud rate.

The algorithm IDs here are wholly arbitrary anyways, but are the
variations I came up with from roughly 60-70 different CPUs.
Bastian Hecht March 6, 2013, 10:19 a.m. UTC | #5
Hi all,

2013/3/6 Paul Mundt <lethal@linux-sh.org>:
> On Tue, Mar 05, 2013 at 07:26:25PM +0000, Arnd Bergmann wrote:
>> On Tuesday 05 March 2013, Bastian Hecht wrote:
>> > >> +- 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)
>> > >
>> > > Maybe replace this with a "clock-frequency" property? This may
>> > > be what the registers contain, but it is not very readable.
>> >
>> > Hmm... do you want a frequency in absolute terms? And then calculate
>> > the best SCBRR value for it?
>> > I'm unsure about this, either you must exactly hit it or accept
>> > tolerances? As something in between I renamed the property to
>> > "renesas,clock-algorithm", but I don't know if this is what you want.
>>
>> I meant the absolute frequency in HZ, since that is what we do
>> for the 8250 uart and other devices, but only if that is sufficient
>> for your device.
>>
> No, we still need to figure out how to generate that baud rate, whether
> it needs an internal or external clock for driving the rate, etc. The
> frequency in and of itself doesn't provide this information, and various
> parts use different algorithms for factoring the baud rate generator,
> even varying across otherwise identical ports. It's unfortunately not
> possible to infer anything about the SCBRR algorithm from port type or
> specified baud rate.
>
> The algorithm IDs here are wholly arbitrary anyways, but are the
> variations I came up with from roughly 60-70 different CPUs.

So if we stick with the notion of the algrithm ID everything is settled now.

I will prepare a v6 that I will post as the whole patchset and an
additional incremental patch 1/3 from v3 to v6 for Paul's repo. Then
we can see if the SCBRR thing needs further discussion and if the rest
of the patchset is final.

Thanks to you all,

 Bastian
Arnd Bergmann March 6, 2013, 10:28 a.m. UTC | #6
On Wednesday 06 March 2013, Bastian Hecht wrote:
> 2013/3/6 Paul Mundt <lethal@linux-sh.org>:
> > No, we still need to figure out how to generate that baud rate, whether
> > it needs an internal or external clock for driving the rate, etc. The
> > frequency in and of itself doesn't provide this information, and various
> > parts use different algorithms for factoring the baud rate generator,
> > even varying across otherwise identical ports. It's unfortunately not
> > possible to infer anything about the SCBRR algorithm from port type or
> > specified baud rate.
> >
> > The algorithm IDs here are wholly arbitrary anyways, but are the
> > variations I came up with from roughly 60-70 different CPUs.

Ok, I see.

> So if we stick with the notion of the algrithm ID everything is settled now.
> 
> I will prepare a v6 that I will post as the whole patchset and an
> additional incremental patch 1/3 from v3 to v6 for Paul's repo. Then
> we can see if the SCBRR thing needs further discussion and if the rest
> of the patchset is final.

I can't think of anything better either, so let's stick to putting the
algorithm ID into the DT binding.

	Arnd
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..6ad1adf
--- /dev/null
+++ b/Documentation/devicetree/bindings/tty/serial/renesas,sci-serial.txt
@@ -0,0 +1,53 @@ 
+* 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. In most cases you can rely
+  on auto-probing (omit this property or set to 0) but some legacy devices
+  use a non-default register layout. Possible layouts are
+  0 = SCIx_PROBE_REGTYPE (default)
+  1 = SCIx_SCI_REGTYPE
+  2 = SCIx_IRDA_REGTYPE
+  3 = SCIx_SCIFA_REGTYPE
+  4 = SCIx_SCIFB_REGTYPE
+  5 = SCIx_SH2_SCIF_FIFODATA_REGTYPE
+  6 = SCIx_SH3_SCIF_REGTYPE
+  7 = SCIx_SH4_SCIF_REGTYPE
+  8 = SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE
+  9 = SCIx_SH4_SCIF_FIFODATA_REGTYPE
+ 10 = 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..cc1b69c 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,112 @@  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);
+
+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;
+	}
+	val = be32_to_cpup(prop);
+	if (val <= SCBRR_ALGO_INVALID || val >= SCBRR_NR_ALGOS) {
+		dev_err(&pdev->dev, "DT prop scbrr-algo-id 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;
+
+	prop = of_get_property(np, "renesas,regtype", NULL);
+	if (prop) {
+		val = be32_to_cpup(prop);
+		if (val < SCIx_PROBE_REGTYPE || val >= SCIx_NR_REGTYPES) {
+			dev_err(&pdev->dev, "DT prop regtype out of range\n");
+			return NULL;
+		}
+		p->regtype = val;
+	}
+
+	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 +2492,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 +2504,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 +2569,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)