diff mbox

[3/4] arm64: dts: marvell: Enable second SDHCI controller in Armada 37xx

Message ID 20170608165125.27630-4-gregory.clement@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gregory CLEMENT June 8, 2017, 4:51 p.m. UTC 
From: Konstantin Porotchkin <kostap@marvell.com>

The Armada 37xx SOCs has 2 SDHCI interfaces. This patch adds the second
one.

Moroever, the Armada 37xx DB v2 board populates the 2 SDHCI interfaces.

The second interface is using plugable module that can either
have an SD connector or eMMC on it.
This patch adds support for SD module in the device DT.

[ gregory.clement@free-electrons.com:
 - Add more detail in commit log
 - Sort the dt node in address order
 - Document the SD slot in the dts ]

Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-3720-db.dts |  9 +++++++++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi   | 11 +++++++++++
 2 files changed, 20 insertions(+)

Comments

Thomas Petazzoni June 8, 2017, 7:17 p.m. UTC | #1 
Hello,

On Thu,  8 Jun 2017 18:51:24 +0200, Gregory CLEMENT wrote:

> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> index 4d495ec39202..01a9c30be38d 100644
> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> @@ -292,6 +292,17 @@
>  				};
>  			};
>  
> +			sdhci1: sdhci@d0000 {
> +				compatible = "marvell,armada-3700-sdhci",
> +				"marvell,sdhci-xenon";

One more tab on this second line would be nice.

> +				reg = <0xd0000 0x300
> +				       0x1e808 0x4>;

I'd prefer:

				reg = <0xd0000 0x300>, <0x1e808 0x4>;

Best regards,

Thomas
Gregory CLEMENT June 13, 2017, 10:10 a.m. UTC | #2 
Hi Thomas,
 
 On jeu., juin 08 2017, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Hello,
>
> On Thu,  8 Jun 2017 18:51:24 +0200, Gregory CLEMENT wrote:
>
>> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
>> index 4d495ec39202..01a9c30be38d 100644
>> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
>> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
>> @@ -292,6 +292,17 @@
>>  				};
>>  			};
>>  
>> +			sdhci1: sdhci@d0000 {
>> +				compatible = "marvell,armada-3700-sdhci",
>> +				"marvell,sdhci-xenon";
>
> One more tab on this second line would be nice.

I agree but and I will aligned all the other entries too in a separate patch.

>
>> +				reg = <0xd0000 0x300
>> +				       0x1e808 0x4>;
>
> I'd prefer:
>
> 				reg = <0xd0000 0x300>, <0x1e808 0x4>;

For this one I think it is a matter of taste. I had a look on the other
files and it seems more common to put them on multi lines that on one
single line. And personally I prefer it, for me it seems more visible
that we use 2 set of registers.

So I will keep it.

Thanks,

Gregory

>
> Best regards,
>
> Thomas
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
Thomas Petazzoni June 13, 2017, 11:25 a.m. UTC | #3 
Hello,

On Tue, 13 Jun 2017 12:10:29 +0200, Gregory CLEMENT wrote:

> >> +				reg = <0xd0000 0x300
> >> +				       0x1e808 0x4>;  
> >
> > I'd prefer:
> >
> > 				reg = <0xd0000 0x300>, <0x1e808 0x4>;  
> 
> For this one I think it is a matter of taste. I had a look on the other
> files and it seems more common to put them on multi lines that on one
> single line. And personally I prefer it, for me it seems more visible
> that we use 2 set of registers.

I'm not talking about multiple lines vs. one line. I'm talking about:

	reg = <X Y
	       A B>;

vs.

	reg = <X Y>,
	      <A B>;

Best regards,

Thomas
Gregory CLEMENT June 13, 2017, 12:22 p.m. UTC | #4 
Hi Thomas,
 
 On mar., juin 13 2017, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Hello,
>
> On Tue, 13 Jun 2017 12:10:29 +0200, Gregory CLEMENT wrote:
>
>> >> +				reg = <0xd0000 0x300
>> >> +				       0x1e808 0x4>;  
>> >
>> > I'd prefer:
>> >
>> > 				reg = <0xd0000 0x300>, <0x1e808 0x4>;  
>> 
>> For this one I think it is a matter of taste. I had a look on the other
>> files and it seems more common to put them on multi lines that on one
>> single line. And personally I prefer it, for me it seems more visible
>> that we use 2 set of registers.
>
> I'm not talking about multiple lines vs. one line. I'm talking about:
>
> 	reg = <X Y
> 	       A B>;
>
> vs.
>
> 	reg = <X Y>,
> 	      <A B>;

OK, this case I agree! And I also have to fix this file for the existing
sdhci and xor node.

Gregory


>
> Best regards,
>
> Thomas
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
diff mbox

Patch

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
index 3ee920ac0015..eab3bc7e6382 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
@@ -158,6 +158,15 @@ 
 	status = "okay";
 };
 
+/* SD slot module on CON14(V2.0)/CON15(V1.4) */
+&sdhci1 {
+	wp-inverted;
+	cd-gpios = <&gpiosb 2 GPIO_ACTIVE_LOW>;
+	bus-width = <4>;
+	marvell,pad-type = "sd";
+	status = "okay";
+};
+
 &spi0 {
 	status = "okay";
 	pinctrl-names = "default";
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 4d495ec39202..01a9c30be38d 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -292,6 +292,17 @@ 
 				};
 			};
 
+			sdhci1: sdhci@d0000 {
+				compatible = "marvell,armada-3700-sdhci",
+				"marvell,sdhci-xenon";
+				reg = <0xd0000 0x300
+				       0x1e808 0x4>;
+				interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&nb_periph_clk 0>;
+				clock-names = "core";
+				status = "disabled";
+			};
+
 			sdhci0: sdhci@d8000 {
 				compatible = "marvell,armada-3700-sdhci",
 				"marvell,sdhci-xenon";