[PATCH v2] [POWERPC] Enable correct operation of serial ports with nonzero regshift.

[PATCH v2] [POWERPC] Enable correct operation of serial ports with nonzero regshift.

[Pavel Kiryukhin]

Add regshift reading to serial drivers.
This enables correct operation of serial ports with 
nonzero regshift such as Xilinx opb 16550 uart.

Signed-off-by: Pavel Kiryukhin <pkiryukhin@ru.mvista.com>

 arch/powerpc/kernel/legacy_serial.c |    6 +++++-
 drivers/serial/of_serial.c          |    6 ++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/legacy_serial.c b/arch/powerpc/kernel/legacy_serial.c
index 61dd174..949e41c 100644
--- a/arch/powerpc/kernel/legacy_serial.c
+++ b/arch/powerpc/kernel/legacy_serial.c
@@ -50,7 +50,7 @@ static int __init add_legacy_port(struct device_node *np, int want_index,
 				  phys_addr_t taddr, unsigned long irq,
 				  upf_t flags, int irq_check_parent)
 {
-	const u32 *clk, *spd;
+	const u32 *clk, *spd, *regshift;
 	u32 clock = BASE_BAUD * 16;
 	int index;
 
@@ -62,6 +62,9 @@ static int __init add_legacy_port(struct device_node *np, int want_index,
 	/* get default speed if present */
 	spd = of_get_property(np, "current-speed", NULL);
 
+	/* get regshift if present*/
+	regshift = of_get_property(np, "reg-shift", NULL);
+
 	/* If we have a location index, then try to use it */
 	if (want_index >= 0 && want_index < MAX_LEGACY_SERIAL_PORTS)
 		index = want_index;
@@ -104,6 +107,7 @@ static int __init add_legacy_port(struct device_node *np, int want_index,
 	legacy_serial_ports[index].uartclk = clock;
 	legacy_serial_ports[index].irq = irq;
 	legacy_serial_ports[index].flags = flags;
+	legacy_serial_ports[index].regshift = regshift ? (u8)*regshift : 0;
 	legacy_serial_infos[index].taddr = taddr;
 	legacy_serial_infos[index].np = of_node_get(np);
 	legacy_serial_infos[index].clock = clock;
diff --git a/drivers/serial/of_serial.c b/drivers/serial/of_serial.c
index a64d858..3f09c5a 100644
--- a/drivers/serial/of_serial.c
+++ b/drivers/serial/of_serial.c
@@ -30,7 +30,7 @@ static int __devinit of_platform_serial_setup(struct of_device *ofdev,
 {
 	struct resource resource;
 	struct device_node *np = ofdev->node;
-	const unsigned int *clk, *spd;
+	const unsigned int *clk, *spd, *regshift;
 	int ret;
 
 	memset(port, 0, sizeof *port);
@@ -40,7 +40,7 @@ static int __devinit of_platform_serial_setup(struct of_device *ofdev,
 		dev_warn(&ofdev->dev, "no clock-frequency property set\n");
 		return -ENODEV;
 	}
-
+	regshift = of_get_property(np, "reg-shift", NULL);
 	ret = of_address_to_resource(np, 0, &resource);
 	if (ret) {
 		dev_warn(&ofdev->dev, "invalid address\n");
@@ -57,6 +57,8 @@ static int __devinit of_platform_serial_setup(struct of_device *ofdev,
 		| UPF_FIXED_PORT;
 	port->dev = &ofdev->dev;
 	port->custom_divisor = *clk / (16 * (*spd));
+	if (regshift)
+		port->regshift = *regshift;
 
 	return 0;
 }

Re: [PATCH v2] [POWERPC] Enable correct operation of serial ports with

[Milton Miller]

On Sat Feb 16 00:30:09 EST 2008, Pavel Kiryukhin wrote:
> Add regshift reading to serial drivers.
> This enables correct operation of serial ports with
> nonzero regshift such as Xilinx opb 16550 uart.
>
> Signed-off-by: Pavel Kiryukhin <pkiryukhin at ru.mvista.com>


This patch is incomplete in that it does not add support to recognize 
any such nodes.

As discussed in the previous submissions, serial ports that attach the 
registers with offsets must get a new compatible name, as the binding 
for ns16550 does not include this reg--shift property.  It would 
therefore be wrong to claim compatibility with it, as older consumers 
of the device tree would bind to this node without the support.   (Its 
ok to check for this port always, but we should only expect it on the 
new compatible type).

We need to define this new type, add it to the doc, and add code to 
find such ports (presently we only search for ns16550 and device_type 
serial on isa bus).


> @@ -40,7 +40,7 @@ static int __devinit of_platform_serial_setup(struct 
> of_device *ofdev,
>                 dev_warn(&ofdev->dev, "no clock-frequency property 
> set\n");
>                 return -ENODEV;
>         }
> -
> +       regshift = of_get_property(np, "reg-shift", NULL);
>         ret = of_address_to_resource(np, 0, &resource);
>         if (ret) {
>                 dev_warn(&ofdev->dev, "invalid address\n");

Looking at the code I was going to say "lets put this optional property 
with the other one (spd) that doesn't have the if next to it."

However, I now see that the existence of spd (current-speed) is simply 
never checked, and if its missing this code will oops with a NULL 
pointer.  Could you prepare a patch to fix that?  I'm not sure if we 
need to fail if its not present, or we can just skip setting 
port->custom_divisor.  You will need to look at the driver to see what 
it requires.

And I think that fetching this property should be moved to the end 
instead of being mixed with the properties we require, but that is just 
my preference.

> @@ -57,6 +57,8 @@ static int __devinit of_platform_serial_setup(struct 
> of_device *ofdev,
>                 | UPF_FIXED_PORT;
>         port->dev = &ofdev->dev;
>         port->custom_divisor = *clk / (16 * (*spd));
> +       if (regshift)
> +               port->regshift = *regshift;
>
>         return 0;
>  }

milton