linux-kernel - Re: [PATCH 4/5] ARM: dove: relocate internal registers device nodes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130729144124.GC2417@localhost>
Date:	Mon, 29 Jul 2013 11:41:25 -0300
From:	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>
To:	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
Cc:	Russell King <linux@....linux.org.uk>,
	Jason Cooper <jason@...edaemon.net>,
	Andrew Lunn <andrew@...n.ch>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Gregory Clement <gregory.clement@...e-electrons.com>
Subject: Re: [PATCH 4/5] ARM: dove: relocate internal registers device nodes

Sebastian,

On Mon, Jul 29, 2013 at 02:31:53PM +0200, Sebastian Hesselbarth wrote:
> With mbus node in place, now relocate all internal device nodes
> to internal-regs node with proper address ranges.
> 
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
> ---
> Cc: Russell King <linux@....linux.org.uk>
> Cc: Jason Cooper <jason@...edaemon.net>
> Cc: Andrew Lunn <andrew@...n.ch>
> Cc: Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linux-kernel@...r.kernel.org
> ---
>  arch/arm/boot/dts/dove.dtsi |  955 +++++++++++++++++++++----------------------
>  1 file changed, 474 insertions(+), 481 deletions(-)
> 

I think you might do this in a slightly different way, and achieve a
less intrusive patch (fwiw, i personally try to avoid intrusive changes).

Maybe it's just a matter of taste, so it's up to you to decide
Anyway, here's my proposal:

----------------------------8<----------------------------------------
diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index db9ae00..0fbc30e 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -5,6 +5,7 @@
 / {
 	compatible = "marvell,dove";
 	model = "Marvell Armada 88AP510 SoC";
+	interrupt-parent = <&intc>;
 
 	aliases {
 		gpio0 = &gpio0;
@@ -41,22 +42,17 @@
 			  MBUS_ID(0x01, 0xfd) 0 0xf8000000 0x8000000   /* BootROM  128M */
 			  MBUS_ID(0x03, 0x01) 0 0xc8000000 0x0100000   /* CESA SRAM  1M */
 			  MBUS_ID(0x0d, 0x00) 0 0xf0000000 0x0100000>; /* PMU  SRAM  1M */
-	};
+};
 
-	soc@...00000 {
+mbus {
+	internal-regs {
 		compatible = "simple-bus";
 		#address-cells = <1>;
 		#size-cells = <1>;
-		interrupt-parent = <&intc>;
-
-		ranges = <0xc8000000 0xc8000000 0x0100000   /* CESA SRAM   1M */
-		          0xe0000000 0xe0000000 0x8000000   /* PCIe0 Mem 128M */
-		          0xe8000000 0xe8000000 0x8000000   /* PCIe1 Mem 128M */
-		          0xf0000000 0xf0000000 0x0100000   /* ScratchPad  1M */
-		          0x00000000 0xf1000000 0x1000000   /* SB/NB regs 16M */
-		          0xf2000000 0xf2000000 0x0100000   /* PCIe0 I/O   1M */
-		          0xf2100000 0xf2100000 0x0100000   /* PCIe0 I/O   1M */
-		          0xf8000000 0xf8000000 0x8000000>; /* BootROM   128M */
+		ranges = <0x00000000 MBUS_ID(0xf0, 0x01) 0 0x0100000   /* MBUS regs  1M */
+			  0x00800000 MBUS_ID(0xf0, 0x02) 0 0x1000000   /* AXI  regs 16M */
+			  0xffffe000 MBUS_ID(0x03, 0x01) 0 0x0000800   /* CESA SRAM  2k */
+			  0xfffff000 MBUS_ID(0x0d, 0x00) 0 0x0000800>; /* PMU  SRAM  2k */
 
 		mbusc: mbus-ctrl@...00 {
 			compatible = "marvell,mbus-controller";
@@ -443,7 +439,7 @@
 		crypto: crypto-engine@...00 {
 			compatible = "marvell,orion-crypto";
 			reg = <0x30000 0x10000>,
-			      <0xc8000000 0x800>;
+			      <0xffffe000 0x800>;
 			reg-names = "regs", "sram";
 			interrupts = <31>;
 			clocks = <&gate_clk 15>;
@@ -465,7 +461,6 @@
 
 			channel1 {
 				interrupts = <40>;
-				dmacap,memset;
 				dmacap,memcpy;
 				dmacap,xor;
 			};
@@ -486,7 +481,6 @@
 
 			channel1 {
 				interrupts = <43>;
-				dmacap,memset;
 				dmacap,memcpy;
 				dmacap,xor;
 			};
----------------------------------->8-----------------------------------

It has a much nicer diffstat:

 arch/arm/boot/dts/dove.dtsi | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

And in addition, it allows to see some (maybe unrelated?) changes
other than the node-relocation.

I've seen these kind of things in (e.g.) omap3-igep0020.dts, and it
seems to me it could be a cleaner way of doing this relocation.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/