Patchwork [U-Boot] mpc83xx fdt: do not adjust clock frequency of external UARTs

login
register
mail settings
Submitter David Müller (ELSOFT AG)
Date April 15, 2011, 12:30 p.m.
Message ID <1302870633-14522-1-git-send-email-d.mueller@elsoft.ch>
Download mbox | patch
Permalink /patch/91354/
State Rejected
Delegated to: Kim Phillips
Headers show

Comments

David Müller (ELSOFT AG) - April 15, 2011, 12:30 p.m.
The current 83xx FDT implementation adjusts the clock frequency of all
16550 UARTs found in the device tree. This behaviour is ok for the UARTs
which are part of the SoC, but wrong for any additional external UART.

Signed-off-by: David Müller <d.mueller@elsoft.ch>

---
 arch/powerpc/cpu/mpc83xx/fdt.c |   27 +++++++++++++++++++++++++--
 1 files changed, 25 insertions(+), 2 deletions(-)
Timur Tabi - April 15, 2011, 5:44 p.m.
On Fri, Apr 15, 2011 at 7:30 AM, David Müller <d.mueller@elsoft.ch> wrote:
> The current 83xx FDT implementation adjusts the clock frequency of all
> 16550 UARTs found in the device tree. This behaviour is ok for the UARTs
> which are part of the SoC, but wrong for any additional external UART.

Just out of curiosity -- do you have an example of a device tree with
an external UART?
David Müller (ELSOFT AG) - April 16, 2011, 5:30 a.m.
Hello

Timur Tabi wrote:
> Just out of curiosity -- do you have an example of a device tree with
> an external UART?

Yes.

Dave
Tabi Timur-B04825 - April 16, 2011, 12:41 p.m.
"David Müller (ELSOFT AG)" wrote:
>> >  Just out of curiosity -- do you have an example of a device tree with
>> >  an external UART?

> Yes.

May I see it?
David Müller (ELSOFT AG) - April 18, 2011, 6:58 a.m.
Tabi Timur-B04825 wrote:
> May I see it?

Please see attached file.

Dave
/*
 *
 * Copyright 2005, 2006 Freescale Semiconductor Inc.
 *
 * This program is free software; you can redistribute  it and/or modify it
 * under  the terms of  the GNU General  Public License as published by the
 * Free Software Foundation;  either version 2 of the  License, or (at your
 * option) any later version.
 * 
 * Modified for MIP470
 *
 */

/dts-v1/;

/ {
	model = "MIP470-1";
	compatible = "MIP470";
	#address-cells = <1>;
	#size-cells = <1>;

	aliases {
		ethernet0 = &enet0;
		ethernet1 = &enet1;
		serial0 = &serial0;
		serial1 = &serial1;
		serial2 = &serial2;
		serial3 = &serial3;
		pci0 = &pci0;
	};
	cpus {
		#address-cells = <1>;
		#size-cells = <0>;

		PowerPC,8347@0 {
			device_type = "cpu";
			reg = <0x0>;
			d-cache-line-size = <0x20>;	// 32 bytes
			i-cache-line-size = <0x20>;	// 32 bytes
			d-cache-size = <0x8000>;	// L1, 32K
			i-cache-size = <0x8000>;	// L1, 32K
			timebase-frequency = <0x0>;	// from bootloader
			bus-frequency = <0x0>;		// from bootloader
			clock-frequency = <0x0>;	// from bootloader
		};
	};
	memory {
		device_type = "memory";
		reg = <0x00000000 0x20000000>;	// 512MB at 0
	};
	soc8347@ff400000 {
		#address-cells = <1>;
		#size-cells = <1>;
		device_type = "soc";
		compatible = "simple-bus";
		ranges = <0x00000000 0xff400000 0x00100000>;
		reg = <0xff400000 0x00000200>;
		bus-frequency = <0>;

		wdt@200 {
			device_type = "watchdog";
			compatible = "mpc83xx_wdt";
			reg = <0x200 0x100>;
		};
		i2c@3000 {
			#address-cells = <1>;
			#size-cells = <0>;
			cell-index = <0>;
			compatible = "fsl-i2c";
			reg = <0x3000 0x100>;
			interrupts = <14 0x8>;
			interrupt-parent = <&ipic>;
			dfsrr;
		};
		i2c@3100 {
			#address-cells = <1>;
			#size-cells = <0>;
			cell-index = <1>;
			compatible = "fsl-i2c";
			reg = <0x3100 0x100>;
			interrupts = <15 0x8>;
			interrupt-parent = <&ipic>;
			dfsrr;
		};
		dma@82a8 {
			#address-cells = <1>;
			#size-cells = <1>;
			compatible = "fsl,mpc8349-dma", "fsl,elo-dma";
			reg = <0x82a8 4>;
			ranges = <0 0x8100 0x1a8>;
			interrupt-parent = <&ipic>;
			interrupts = <71 8>;
			cell-index = <0>;
			dma-channel@0 {
				compatible = "fsl,mpc8349-dma-channel", "fsl,elo-dma-channel";
				reg = <0 0x80>;
				cell-index = <0>;
				interrupt-parent = <&ipic>;
				interrupts = <71 8>;
			};
			dma-channel@80 {
				compatible = "fsl,mpc8349-dma-channel", "fsl,elo-dma-channel";
				reg = <0x80 0x80>;
				cell-index = <1>;
				interrupt-parent = <&ipic>;
				interrupts = <71 8>;
			};
			dma-channel@100 {
				compatible = "fsl,mpc8349-dma-channel", "fsl,elo-dma-channel";
				reg = <0x100 0x80>;
				cell-index = <2>;
				interrupt-parent = <&ipic>;
				interrupts = <71 8>;
			};
			dma-channel@180 {
				compatible = "fsl,mpc8349-dma-channel", "fsl,elo-dma-channel";
				reg = <0x180 0x28>;
				cell-index = <3>;
				interrupt-parent = <&ipic>;
				interrupts = <71 8>;
			};
		};

		usb@22000 {
			compatible = "fsl-usb2-mph";
			reg = <0x22000 0x1000>;
			#address-cells = <1>;
			#size-cells = <0>;
			interrupt-parent = <&ipic>;
			interrupts = <39 0x8>;
			phy_type = "ulpi";
			port1;
		};

		usb@23000 {
			compatible = "fsl-usb2-dr";
			reg = <0x23000 0x1000>;
			#address-cells = <1>;
			#size-cells = <0>;
			interrupt-parent = <&ipic>;
			interrupts = <38 0x8>;
			phy_type = "ulpi";
		};

		mdio@24520 {
			#address-cells = <1>;
			#size-cells = <0>;
			compatible = "fsl,gianfar-mdio";
			reg = <0x24520 0x20>;

			phy1: ethernet-phy@1 {
				interrupt-parent = <&ipic>;
				interrupts = <17 0x8>;
				reg = <0x1>;
				device_type = "ethernet-phy";
			};
			phy2: ethernet-phy@2 {
				interrupt-parent = <&ipic>;
				interrupts = <18 0x8>;
				reg = <0x2>;
				device_type = "ethernet-phy";
			};
		};

		enet0: ethernet@24000 {
			cell-index = <0>;
			device_type = "network";
			model = "TSEC";
			compatible = "gianfar";
			reg = <0x24000 0x1000>;
			local-mac-address = [ 00 00 00 00 00 00 ];	// from bootloader
			interrupts = <32 0x8 33 0x8 34 0x8>;
			interrupt-parent = <&ipic>;
			phy-handle = <&phy1>;
			linux,network-index = <0>;
		};

		enet1: ethernet@25000 {
			cell-index = <1>;
			device_type = "network";
			model = "TSEC";
			compatible = "gianfar";
			reg = <0x25000 0x1000>;
			local-mac-address = [ 00 00 00 00 00 00 ];	// from bootloader
			interrupts = <35 0x8 36 0x8 37 0x8>;
			interrupt-parent = <&ipic>;
			phy-handle = <&phy2>;
			linux,network-index = <1>;
		};

		serial0: serial@4500 {
			cell-index = <0>;
			device_type = "serial";
			compatible = "ns16550";
			reg = <0x4500 0x100>;
			clock-frequency = <0>;
			interrupts = <9 0x8>;
			interrupt-parent = <&ipic>;
		};

		serial1: serial@4600 {
			cell-index = <1>;
			device_type = "serial";
			compatible = "ns16550";
			reg = <0x4600 0x100>;
			clock-frequency = <0>;
			interrupts = <10 0x8>;
			interrupt-parent = <&ipic>;
		};

		ipic: pic@700 {
			interrupt-controller;
			#address-cells = <0>;
			#interrupt-cells = <2>;
			reg = <0x700 0x100>;
			device_type = "ipic";
		};
	};

	pci0: pci@ff408500 {
		cell-index = <1>;
		interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
		interrupt-map = <0xa000 0x0 0x0 0x1 &ipic 48 0x8

				 0xa800 0x0 0x0 0x1 &ipic 17 0x8
				 0xa800 0x0 0x0 0x2 &ipic 18 0x8
				 0xa800 0x0 0x0 0x3 &ipic 19 0x8
				 0xa800 0x0 0x0 0x4 &ipic 20 0x8

				 0xb000 0x0 0x0 0x1 &ipic 18 0x8
				 0xb000 0x0 0x0 0x2 &ipic 19 0x8
				 0xb000 0x0 0x0 0x3 &ipic 20 0x8
				 0xb000 0x0 0x0 0x4 &ipic 17 0x8

				 0xb800 0x0 0x0 0x1 &ipic 19 0x8
				 0xb800 0x0 0x0 0x2 &ipic 20 0x8
				 0xb800 0x0 0x0 0x3 &ipic 17 0x8
				 0xb800 0x0 0x0 0x4 &ipic 18 0x8

				 0xc000 0x0 0x0 0x1 &ipic 20 0x8
				 0xc000 0x0 0x0 0x2 &ipic 17 0x8
				 0xc000 0x0 0x0 0x3 &ipic 18 0x8
				 0xc000 0x0 0x0 0x4 &ipic 19 0x8>;
		interrupt-parent = <&ipic>;
		interrupts = <66 0x8>;
		bus-range = <0 0xff>;
		ranges = <0x02000000 0x0 0xd0000000 0xd0000000 0x0 0x10000000
			  0x42000000 0x0 0xc0000000 0xc0000000 0x0 0x10000000
			  0x01000000 0x0 0x00000000 0xe0000000 0x0 0x00100000>;
		clock-frequency = <33333333>;
		#interrupt-cells = <1>;
		#size-cells = <2>;
		#address-cells = <3>;
		reg = <0xff408500 0x100
		       0xff408300 0x8>;
		compatible = "fsl,mpc8349-pci";
		device_type = "pci";
	};
	localbus@80000000 {
		#address-cells = <2>;
		#size-cells = <1>;
		#interrupt-cells = <2>;
		device_type = "localbus";
		compatible = "simple-bus";
		reg = <0x80000000 0x1000>;
		
		ranges = <0 0x0 0x80000000 0x04000000
		          3 0x0 0x90000000 0x00400000>;
		flash@0,0 {
			#address-cells = <1>;
			#size-cells = <1>;
			compatible = "cfi-flash";
			reg = <0 0x0 0x04000000>;
			bank-width = <2>;
			device-width = <1>;
		};

		serial2: serial@3,0x10 {
			device_type = "serial";
			compatible = "ns16550";
			reg = <3 0x10 0x10>;
			clock-frequency = <14745600>;
			interrupts = <21 0x8>;
			interrupt-parent = <&ipic>;
		};

		serial3: serial@3,0x20 {
			device_type = "serial";
			compatible = "ns16550";
			reg = <3 0x20 0x10>;
			clock-frequency = <14745600>;
			interrupts = <22 0x8>;
			interrupt-parent = <&ipic>;
		};
	};
};
Kim Phillips - May 2, 2011, 4:33 p.m.
On Fri, 15 Apr 2011 14:30:33 +0200
David Müller <d.mueller@elsoft.ch> wrote:

> The current 83xx FDT implementation adjusts the clock frequency of all
> 16550 UARTs found in the device tree. This behaviour is ok for the UARTs
> which are part of the SoC, but wrong for any additional external UART.
> 
> Signed-off-by: David Müller <d.mueller@elsoft.ch>
> 
> ---
>  arch/powerpc/cpu/mpc83xx/fdt.c |   27 +++++++++++++++++++++++++--
>  1 files changed, 25 insertions(+), 2 deletions(-)

(a) this is not an mpc83xx-specific issue

(b) the fdt code here should instead be modified to only adjust clock
properties on UART nodes contained within the SoC node, not the entire
fdt.

Any UARTs not a part of the SoC should get their clocks updated in
board/ code (or, as in this case, set manually in the device tree).

Kim

Patch

diff --git a/arch/powerpc/cpu/mpc83xx/fdt.c b/arch/powerpc/cpu/mpc83xx/fdt.c
index daf73a6..e13e2b3 100644
--- a/arch/powerpc/cpu/mpc83xx/fdt.c
+++ b/arch/powerpc/cpu/mpc83xx/fdt.c
@@ -50,6 +50,7 @@  void ft_cpu_setup(void *blob, bd_t *bd)
 {
 	immap_t *immr = (immap_t *)CONFIG_SYS_IMMR;
 	int spridr = immr->sysconf.spridr;
+	int off;
 
 	/*
 	 * delete crypto node if not on an E-processor
@@ -131,8 +132,30 @@  void ft_cpu_setup(void *blob, bd_t *bd)
 #endif
 
 #ifdef CONFIG_SYS_NS16550
-	do_fixup_by_compat_u32(blob, "ns16550",
-		"clock-frequency", CONFIG_SYS_NS16550_CLK, 1);
+	/*
+	 * fixup all UART clocks for CPU internal UARTs
+	 * (clock-frequency is usually 0) but do not touch
+	 * any other (external) UARTs
+	 */
+	off = fdt_node_offset_by_compatible(blob, -1, "ns16550");
+
+	while (off != -FDT_ERR_NOTFOUND) {
+		const struct fdt_property *prop;
+		int len;
+
+		prop = fdt_get_property(blob, off, "clock-frequency", &len);
+		if (prop && (len == 4)) {
+			u32 *data = (u32 *)prop->data;
+			u32 val = fdt32_to_cpu(*data);
+			if (val == 0) {
+				val = cpu_to_fdt32(CONFIG_SYS_NS16550_CLK);
+				fdt_setprop(blob, off,
+					    "clock-frequency", &val, 4);
+			}
+		}
+
+		off = fdt_node_offset_by_compatible(blob, off, "ns16550");
+	}
 #endif
 
 	fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);