diff mbox

[2/3] ARM: kirkwood: Move the crypto node under the mbus node

Message ID 20130917184309.GE21230@obsidianresearch.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jason Gunthorpe Sept. 17, 2013, 6:43 p.m. UTC
There should be no nodes that are not children of the mbus. Move
the crypto node under the mbus.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
 arch/arm/boot/dts/kirkwood.dtsi | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

[oops, ARM: kirkwood - Remove kirkwood_setup_wins and rely on
 the DT binding was 1/3]

Comments

Sebastian Hesselbarth Sept. 17, 2013, 6:57 p.m. UTC | #1
On 09/17/2013 08:43 PM, Jason Gunthorpe wrote:
> There should be no nodes that are not children of the mbus. Move
> the crypto node under the mbus.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
>   arch/arm/boot/dts/kirkwood.dtsi | 23 +++++++++++------------
>   1 file changed, 11 insertions(+), 12 deletions(-)

Jason,

I'd prefer not to move crypto out of internal regs. It is IP located
in internal registers. What about moving the crypto _sram_ to mmio-sram
compatible node and link it to crypto node?

That would require to update mv_cesa of course.

Sebastian

> [oops, ARM: kirkwood - Remove kirkwood_setup_wins and rely on
>   the DT binding was 1/3]
>
> diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
> index d1bbe95..76b1627 100644
> --- a/arch/arm/boot/dts/kirkwood.dtsi
> +++ b/arch/arm/boot/dts/kirkwood.dtsi
> @@ -35,13 +35,22 @@
>   		controller = <&mbusc>;
>   		pcie-mem-aperture = <0xe0000000 0x10000000>; /* 256 MiB memory space */
>   		pcie-io-aperture  = <0xf2000000 0x100000>;   /*   1 MiB    I/O space */
> +
> +		crypto@0301 {
> +			compatible = "marvell,orion-crypto";
> +			reg = <MBUS_ID(0xf0, 0x01) 0x30000 0x10000>,
> +			      <MBUS_ID(0x03, 0x01) 0 0x800>;
> +			reg-names = "regs", "sram";
> +			interrupts = <22>;
> +			clocks = <&gate_clk 17>;
> +			status = "okay";
> +		};
>   	};
>
>   	ocp@f1000000 {
>   		compatible = "simple-bus";
>   		ranges = <0x00000000 0xf1000000 0x0100000
> -		          0xf4000000 0xf4000000 0x0000400
> -		          0xf5000000 0xf5000000 0x0000400>;
> +		          0xf4000000 0xf4000000 0x0000400>;
>   		#address-cells = <1>;
>   		#size-cells = <1>;
>
> @@ -222,16 +231,6 @@
>   			status = "disabled";
>   		};
>
> -		crypto@30000 {
> -			compatible = "marvell,orion-crypto";
> -			reg = <0x30000 0x10000>,
> -			      <0xf5000000 0x800>;
> -			reg-names = "regs", "sram";
> -			interrupts = <22>;
> -			clocks = <&gate_clk 17>;
> -			status = "okay";
> -		};
> -
>   		mdio: mdio-bus@72004 {
>   			compatible = "marvell,orion-mdio";
>   			#address-cells = <1>;
>
Jason Gunthorpe Sept. 17, 2013, 7:20 p.m. UTC | #2
On Tue, Sep 17, 2013 at 08:57:13PM +0200, Sebastian Hesselbarth wrote:
> On 09/17/2013 08:43 PM, Jason Gunthorpe wrote:
> >There should be no nodes that are not children of the mbus. Move
> >the crypto node under the mbus.
> >
> >Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> >  arch/arm/boot/dts/kirkwood.dtsi | 23 +++++++++++------------
> >  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> I'd prefer not to move crypto out of internal regs. It is IP located
> in internal registers. 

Well, that isn't entirely right. Internal registers isn't an IP block,
it is yet another internal bus. MBUS is bridged to the (presumably low
speed) internal registers bus, and then the internal registers bus is
routed to IP block.

There is a page in the manual that describes each of the IP blocks
hanging off the internal registers bus.

The crypto IP block has connections to two busses. One to MBUS and one
to internal-regs bus. The sram is dual ported (mbus and IP both access
it), it is an integral part of the crypto IP.

So, I think the current binding is fine, and this change makes it very
clear that it is a single IP block with connections to two of the
internal busses.

Certainly is is cleaner than contaminating the internal bus node's
decode ranges with addresses that are not decoded by the MBUS to
internal bus bridge.

Anyhow, I don't want to churn the DT binding for crypto, we are
supposed to be trying to keep these things stable and there is no
functional change or bug fix in splitting the node into two.

Jason
Jason Cooper Oct. 1, 2013, 4:35 p.m. UTC | #3
On Tue, Sep 17, 2013 at 12:43:09PM -0600, Jason Gunthorpe wrote:
> There should be no nodes that are not children of the mbus. Move
> the crypto node under the mbus.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
>  arch/arm/boot/dts/kirkwood.dtsi | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)

Applied to mvebu/dt

thx,

Jason.
diff mbox

Patch

diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
index d1bbe95..76b1627 100644
--- a/arch/arm/boot/dts/kirkwood.dtsi
+++ b/arch/arm/boot/dts/kirkwood.dtsi
@@ -35,13 +35,22 @@ 
 		controller = <&mbusc>;
 		pcie-mem-aperture = <0xe0000000 0x10000000>; /* 256 MiB memory space */
 		pcie-io-aperture  = <0xf2000000 0x100000>;   /*   1 MiB    I/O space */
+
+		crypto@0301 {
+			compatible = "marvell,orion-crypto";
+			reg = <MBUS_ID(0xf0, 0x01) 0x30000 0x10000>,
+			      <MBUS_ID(0x03, 0x01) 0 0x800>;
+			reg-names = "regs", "sram";
+			interrupts = <22>;
+			clocks = <&gate_clk 17>;
+			status = "okay";
+		};
 	};
 
 	ocp@f1000000 {
 		compatible = "simple-bus";
 		ranges = <0x00000000 0xf1000000 0x0100000
-		          0xf4000000 0xf4000000 0x0000400
-		          0xf5000000 0xf5000000 0x0000400>;
+		          0xf4000000 0xf4000000 0x0000400>;
 		#address-cells = <1>;
 		#size-cells = <1>;
 
@@ -222,16 +231,6 @@ 
 			status = "disabled";
 		};
 
-		crypto@30000 {
-			compatible = "marvell,orion-crypto";
-			reg = <0x30000 0x10000>,
-			      <0xf5000000 0x800>;
-			reg-names = "regs", "sram";
-			interrupts = <22>;
-			clocks = <&gate_clk 17>;
-			status = "okay";
-		};
-
 		mdio: mdio-bus@72004 {
 			compatible = "marvell,orion-mdio";
 			#address-cells = <1>;