Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1363644 > unrolled thread

[PATCH 00/12] ARM: dts: Add dragonboard-600c support.

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/12] ARM: dts: Add dragonboard-600c support. Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-03-23 20:50 +0100
    [PATCH 03/12] ARM: dts: apq8064: add gsbi7 i2c support Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-03-23 20:50 +0100
      Re: [PATCH 03/12] ARM: dts: apq8064: add gsbi7 i2c support Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-03-29 16:40 +0200
    [PATCH 07/12] ARM: dts: dragonboard-600c: add usb support Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-03-23 20:50 +0100
      Re: [PATCH 07/12] ARM: dts: dragonboard-600c: add usb support Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-03-29 16:40 +0200
    [PATCH 01/12] ARM: dts: apq8064: fix the pinctrls for i2c and spi Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-03-23 20:50 +0100
      Re: [PATCH 01/12] ARM: dts: apq8064: fix the pinctrls for i2c and spi Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-03-29 16:30 +0200
        Re: [PATCH 01/12] ARM: dts: apq8064: fix the pinctrls for i2c and spi Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-03-29 17:10 +0200
          Re: [PATCH 01/12] ARM: dts: apq8064: fix the pinctrls for i2c and spi Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-03-29 18:20 +0200
    Re: [PATCH 00/12] ARM: dts: Add dragonboard-600c support. Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-03-29 17:00 +0200
      Re: [PATCH 00/12] ARM: dts: Add dragonboard-600c support. Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-03-29 17:10 +0200

#1363644 — [PATCH 00/12] ARM: dts: Add dragonboard-600c support.

Hi Andy,

This patchset adds support to Dragonboard 600c which is based on APQ8064.

With this patchset, except spi I was able to test all the below features
on this board on top of linus master branch.
1> i2c
2> spi
3> sd/mmc with card detect
4> eMMC
5> USB
6> SATA
7> on board Ethernet based on PCIE.
8> user and activity leds.

AHCI SATA is broken on APQ8064 on mainline, due to changes in libahci, fix is
being discussed which should not effect this dt patches anyway.
This patchset also has a fix to i2c/spi pinctrls which was not set correctly
in my previous apq8064 patches, I have verified this patchset with eeprom and
sensors on the board, you might want to take that patch in next rc.

Thanks,
srini

Srinivas Kandagatla (12):
  ARM: dts: apq8064: fix the pinctrls for i2c and spi
  ARM: dts: apq8064: add support to gsbi1 uart
  ARM: dts: apq8064: add gsbi7 i2c support
  ARM: dts: dragonboard-600c: add board support with serial
  ARM: dts: dragonboard-600c: add pmic regulator supplies
  ARM: dts: dragonboard-600c: Add eMMC and SD card support
  ARM: dts: dragonboard-600c: add usb support
  ARM: dts: dragonboard-600c: add pcie support
  ARM: dts: dragonboard-600c: add on board sata support.
  ARM: dts: dragonboard-600c: Add on board leds support
  ARM: dts: dragonboard-600c: add i2c support
  ARM: dts: dragonboard-600c: add spi support

 arch/arm/boot/dts/Makefile                         |   1 +
 .../arm/boot/dts/qcom-apq8064-dragonboard-600c.dts | 377 +++++++++++++++++++++
 arch/arm/boot/dts/qcom-apq8064-pins.dtsi           |  39 +++
 arch/arm/boot/dts/qcom-apq8064.dtsi                |  41 ++-
 4 files changed, 452 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm/boot/dts/qcom-apq8064-dragonboard-600c.dts

-- 
2.5.0

[toc] | [next] | [standalone]

#1363645 — [PATCH 03/12] ARM: dts: apq8064: add gsbi7 i2c support

This patch adds support to gsbi7 i2c which is used in some of the new
boards.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 arch/arm/boot/dts/qcom-apq8064-pins.dtsi | 25 +++++++++++++++++++++++++
 arch/arm/boot/dts/qcom-apq8064.dtsi      | 13 +++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-apq8064-pins.dtsi b/arch/arm/boot/dts/qcom-apq8064-pins.dtsi
index 8bb5e5f..4102a98 100644
--- a/arch/arm/boot/dts/qcom-apq8064-pins.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064-pins.dtsi
@@ -219,4 +219,29 @@
 			function = "gsbi7";
 		};
 	};
+
+	i2c7_pins: i2c7 {
+		mux {
+			pins = "gpio84", "gpio85";
+			function = "gsbi7";
+		};
+
+		pinconf {
+			pins = "gpio84", "gpio85";
+			drive-strength = <16>;
+			bias-disable;
+		};
+	};
+
+	i2c7_pins_sleep: i2c7_pins_sleep {
+		mux {
+			pins = "gpio84", "gpio85";
+			function = "gpio";
+		};
+		pinconf {
+			pins = "gpio84", "gpio85";
+			drive-strength = <2>;
+			bias-disable = <0>;
+		};
+	};
 };
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index 81b4290..f064f59 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -417,6 +417,19 @@
 				clock-names = "core", "iface";
 				status = "disabled";
 			};
+
+			gsbi7_i2c: i2c@16680000 {
+				compatible = "qcom,i2c-qup-v1.1.1";
+				pinctrl-0 = <&i2c7_pins>;
+				pinctrl-1 = <&i2c7_pins_sleep>;
+				pinctrl-names = "default", "sleep";
+				reg = <0x16680000 0x1000>;
+				interrupts = <GIC_SPI 159 IRQ_TYPE_NONE>;
+				clocks = <&gcc GSBI7_QUP_CLK>,
+					 <&gcc GSBI7_H_CLK>;
+				clock-names = "core", "iface";
+				status = "disabled";
+			};
 		};
 
 		rng@1a500000 {
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1366339 — Re: [PATCH 03/12] ARM: dts: apq8064: add gsbi7 i2c support

On Wed 23 Mar 12:47 PDT 2016, Srinivas Kandagatla wrote:

> This patch adds support to gsbi7 i2c which is used in some of the new
> boards.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  arch/arm/boot/dts/qcom-apq8064-pins.dtsi | 25 +++++++++++++++++++++++++
>  arch/arm/boot/dts/qcom-apq8064.dtsi      | 13 +++++++++++++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/qcom-apq8064-pins.dtsi b/arch/arm/boot/dts/qcom-apq8064-pins.dtsi
> index 8bb5e5f..4102a98 100644
> --- a/arch/arm/boot/dts/qcom-apq8064-pins.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8064-pins.dtsi
> @@ -219,4 +219,29 @@
>  			function = "gsbi7";
>  		};
>  	};
> +
> +	i2c7_pins: i2c7 {
> +		mux {
> +			pins = "gpio84", "gpio85";
> +			function = "gsbi7";
> +		};
> +
> +		pinconf {
> +			pins = "gpio84", "gpio85";
> +			drive-strength = <16>;
> +			bias-disable;
> +		};
> +	};
> +
> +	i2c7_pins_sleep: i2c7_pins_sleep {
> +		mux {
> +			pins = "gpio84", "gpio85";
> +			function = "gpio";
> +		};
> +		pinconf {
> +			pins = "gpio84", "gpio85";
> +			drive-strength = <2>;
> +			bias-disable = <0>;
> +		};
> +	};

I've been going back and forth about having default pinconfig settings
in the platform dtsi and having devices to override these later.

I do find it cleaner to push this down to the board, as they should
always be verified and tweaked by a hw guy for the specific board.


For some cases this isn't really the case, based on hard design
requirements, but for i2c it is.

>  };
> diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> index 81b4290..f064f59 100644
> --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> @@ -417,6 +417,19 @@
>  				clock-names = "core", "iface";
>  				status = "disabled";
>  			};
> +
> +			gsbi7_i2c: i2c@16680000 {

We normally don't need to reference the i2c adaptors, so please drop
this label.

> +				compatible = "qcom,i2c-qup-v1.1.1";
> +				pinctrl-0 = <&i2c7_pins>;
> +				pinctrl-1 = <&i2c7_pins_sleep>;
> +				pinctrl-names = "default", "sleep";
> +				reg = <0x16680000 0x1000>;
> +				interrupts = <GIC_SPI 159 IRQ_TYPE_NONE>;
> +				clocks = <&gcc GSBI7_QUP_CLK>,
> +					 <&gcc GSBI7_H_CLK>;
> +				clock-names = "core", "iface";
> +				status = "disabled";
> +			};

Regards,
Bjorn

[toc] | [prev] | [next] | [standalone]

#1363647 — [PATCH 07/12] ARM: dts: dragonboard-600c: add usb support

This patch adds usb host and otg support on board with required
regulators.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 .../arm/boot/dts/qcom-apq8064-dragonboard-600c.dts | 64 ++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-apq8064-dragonboard-600c.dts b/arch/arm/boot/dts/qcom-apq8064-dragonboard-600c.dts
index a977314..5374510 100644
--- a/arch/arm/boot/dts/qcom-apq8064-dragonboard-600c.dts
+++ b/arch/arm/boot/dts/qcom-apq8064-dragonboard-600c.dts
@@ -60,6 +60,12 @@
 					bias-pull-down;
 				};
 
+				s3 {
+					regulator-min-microvolt = <1000000>;
+					regulator-max-microvolt = <1400000>;
+					qcom,switch-mode-frequency = <4800000>;
+				};
+
 				s4 {
 					regulator-min-microvolt	= <1800000>;
 					regulator-max-microvolt	= <1800000>;
@@ -74,6 +80,18 @@
 					qcom,switch-mode-frequency = <3200000>;
 				 };
 
+				l3 {
+					regulator-min-microvolt = <3050000>;
+					regulator-max-microvolt = <3300000>;
+					bias-pull-down;
+				};
+
+				l4 {
+					regulator-min-microvolt = <1000000>;
+					regulator-max-microvolt = <1800000>;
+					bias-pull-down;
+				};
+
 				l5 {
 					regulator-min-microvolt = <2750000>;
 					regulator-max-microvolt = <3000000>;
@@ -85,6 +103,12 @@
 					regulator-max-microvolt = <2950000>;
 					bias-pull-down;
 				};
+
+				l23 {
+					regulator-min-microvolt = <1700000>;
+					regulator-max-microvolt = <1900000>;
+					bias-pull-down;
+				};
 			};
 		};
 
@@ -111,6 +135,46 @@
 			};
 		};
 
+		/* OTG */
+		phy@12500000 {
+			status		= "okay";
+			dr_mode		= "peripheral";
+			vddcx-supply	= <&pm8921_s3>;
+			v3p3-supply	= <&pm8921_l3>;
+			v1p8-supply	= <&pm8921_l4>;
+		};
+
+		phy@12520000 {
+			status		= "okay";
+			vddcx-supply	= <&pm8921_s3>;
+			v3p3-supply	= <&pm8921_l3>;
+			v1p8-supply	= <&pm8921_l23>;
+		};
+
+		phy@12530000 {
+			status		= "okay";
+			vddcx-supply	= <&pm8921_s3>;
+			v3p3-supply	= <&pm8921_l3>;
+			v1p8-supply	= <&pm8921_l23>;
+		};
+
+		gadget@12500000 {
+			status = "okay";
+		};
+
+		/* OTG */
+		usb@12500000 {
+			status = "okay";
+		};
+
+		usb@12520000 {
+			status = "okay";
+		};
+
+		usb@12530000 {
+			status = "okay";
+		};
+
 		amba {
 			/* eMMC */
 			sdcc@12400000 {
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1366340 — Re: [PATCH 07/12] ARM: dts: dragonboard-600c: add usb support

On Wed 23 Mar 12:47 PDT 2016, Srinivas Kandagatla wrote:

> This patch adds usb host and otg support on board with required
> regulators.
> 

Please fold the regulators into the regulator patch instead.

For the rest:

Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  .../arm/boot/dts/qcom-apq8064-dragonboard-600c.dts | 64 ++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/qcom-apq8064-dragonboard-600c.dts b/arch/arm/boot/dts/qcom-apq8064-dragonboard-600c.dts
> index a977314..5374510 100644
> --- a/arch/arm/boot/dts/qcom-apq8064-dragonboard-600c.dts
> +++ b/arch/arm/boot/dts/qcom-apq8064-dragonboard-600c.dts
> @@ -60,6 +60,12 @@
>  					bias-pull-down;
>  				};
>  
> +				s3 {
> +					regulator-min-microvolt = <1000000>;
> +					regulator-max-microvolt = <1400000>;
> +					qcom,switch-mode-frequency = <4800000>;
> +				};
> +
>  				s4 {
>  					regulator-min-microvolt	= <1800000>;
>  					regulator-max-microvolt	= <1800000>;
> @@ -74,6 +80,18 @@
>  					qcom,switch-mode-frequency = <3200000>;
>  				 };
>  
> +				l3 {
> +					regulator-min-microvolt = <3050000>;
> +					regulator-max-microvolt = <3300000>;
> +					bias-pull-down;
> +				};
> +
> +				l4 {
> +					regulator-min-microvolt = <1000000>;
> +					regulator-max-microvolt = <1800000>;
> +					bias-pull-down;
> +				};
> +
>  				l5 {
>  					regulator-min-microvolt = <2750000>;
>  					regulator-max-microvolt = <3000000>;
> @@ -85,6 +103,12 @@
>  					regulator-max-microvolt = <2950000>;
>  					bias-pull-down;
>  				};
> +
> +				l23 {
> +					regulator-min-microvolt = <1700000>;
> +					regulator-max-microvolt = <1900000>;
> +					bias-pull-down;
> +				};
>  			};
>  		};
>  
> @@ -111,6 +135,46 @@
>  			};
>  		};
>  
> +		/* OTG */
> +		phy@12500000 {
> +			status		= "okay";
> +			dr_mode		= "peripheral";
> +			vddcx-supply	= <&pm8921_s3>;
> +			v3p3-supply	= <&pm8921_l3>;
> +			v1p8-supply	= <&pm8921_l4>;
> +		};
> +
> +		phy@12520000 {
> +			status		= "okay";
> +			vddcx-supply	= <&pm8921_s3>;
> +			v3p3-supply	= <&pm8921_l3>;
> +			v1p8-supply	= <&pm8921_l23>;
> +		};
> +
> +		phy@12530000 {
> +			status		= "okay";
> +			vddcx-supply	= <&pm8921_s3>;
> +			v3p3-supply	= <&pm8921_l3>;
> +			v1p8-supply	= <&pm8921_l23>;
> +		};
> +
> +		gadget@12500000 {
> +			status = "okay";
> +		};
> +
> +		/* OTG */
> +		usb@12500000 {
> +			status = "okay";
> +		};
> +
> +		usb@12520000 {
> +			status = "okay";
> +		};
> +
> +		usb@12530000 {
> +			status = "okay";
> +		};
> +
>  		amba {
>  			/* eMMC */
>  			sdcc@12400000 {
> -- 
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [next] | [standalone]

#1363648 — [PATCH 01/12] ARM: dts: apq8064: fix the pinctrls for i2c and spi

This patch fixes pinctrls for spi and i2c nodes whose default and sleep
states are together, which is incorrect.

Without this patch i2c/spi would not be functional.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 arch/arm/boot/dts/qcom-apq8064.dtsi | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index 65d0e8d..c6ff8fc 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -227,7 +227,8 @@
 
 			gsbi1_i2c: i2c@12460000 {
 				compatible = "qcom,i2c-qup-v1.1.1";
-				pinctrl-0 = <&i2c1_pins &i2c1_pins_sleep>;
+				pinctrl-0 = <&i2c1_pins>;
+				pinctrl-1 = <&i2c1_pins_sleep>;
 				pinctrl-names = "default", "sleep";
 				reg = <0x12460000 0x1000>;
 				interrupts = <0 194 IRQ_TYPE_NONE>;
@@ -255,7 +256,8 @@
 			gsbi2_i2c: i2c@124a0000 {
 				compatible = "qcom,i2c-qup-v1.1.1";
 				reg = <0x124a0000 0x1000>;
-				pinctrl-0 = <&i2c2_pins &i2c2_pins_sleep>;
+				pinctrl-0 = <&i2c2_pins>;
+				pinctrl-1 = <&i2c2_pins_sleep>;
 				pinctrl-names = "default", "sleep";
 				interrupts = <0 196 IRQ_TYPE_NONE>;
 				clocks = <&gcc GSBI2_QUP_CLK>, <&gcc GSBI2_H_CLK>;
@@ -277,7 +279,8 @@
 			ranges;
 			gsbi3_i2c: i2c@16280000 {
 				compatible = "qcom,i2c-qup-v1.1.1";
-				pinctrl-0 = <&i2c3_pins &i2c3_pins_sleep>;
+				pinctrl-0 = <&i2c3_pins>;
+				pinctrl-1 = <&i2c3_pins_sleep>;
 				pinctrl-names = "default", "sleep";
 				reg = <0x16280000 0x1000>;
 				interrupts = <GIC_SPI 151 IRQ_TYPE_NONE>;
@@ -302,7 +305,8 @@
 
 			gsbi4_i2c: i2c@16380000 {
 				compatible = "qcom,i2c-qup-v1.1.1";
-				pinctrl-0 = <&i2c4_pins &i2c4_pins_sleep>;
+				pinctrl-0 = <&i2c4_pins>;
+				pinctrl-1 = <&i2c4_pins_sleep>;
 				pinctrl-names = "default", "sleep";
 				reg = <0x16380000 0x1000>;
 				interrupts = <GIC_SPI 153 IRQ_TYPE_NONE>;
@@ -337,7 +341,8 @@
 				compatible = "qcom,spi-qup-v1.1.1";
 				reg = <0x1a280000 0x1000>;
 				interrupts = <0 155 0>;
-				pinctrl-0 = <&spi5_default &spi5_sleep>;
+				pinctrl-0 = <&spi5_default>;
+				pinctrl-1 = <&spi5_sleep>;
 				pinctrl-names = "default", "sleep";
 				clocks = <&gcc GSBI5_QUP_CLK>, <&gcc GSBI5_H_CLK>;
 				clock-names = "core", "iface";
@@ -370,7 +375,8 @@
 
 			gsbi6_i2c: i2c@16580000 {
 				compatible = "qcom,i2c-qup-v1.1.1";
-				pinctrl-0 = <&i2c6_pins &i2c6_pins_sleep>;
+				pinctrl-0 = <&i2c6_pins>;
+				pinctrl-1 = <&i2c6_pins_sleep>;
 				pinctrl-names = "default", "sleep";
 				reg = <0x16580000 0x1000>;
 				interrupts = <GIC_SPI 157 IRQ_TYPE_NONE>;
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1366329 — Re: [PATCH 01/12] ARM: dts: apq8064: fix the pinctrls for i2c and spi

On Wed 23 Mar 12:47 PDT 2016, Srinivas Kandagatla wrote:

> This patch fixes pinctrls for spi and i2c nodes whose default and sleep
> states are together, which is incorrect.
> 
> Without this patch i2c/spi would not be functional.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

The change in itself is sound, but I don't understand why i2c/spi fails
to function if we don't bring them to a sleep state. Can you please
update the commit message with an explanation?


PS. Whenever there's multiple states for a thing I do prefer prefixing
them _a and _s to highlight that difference (not only suffixing the
sleep state). And use abbreviations :)

Regards,
Bjorn

> ---
>  arch/arm/boot/dts/qcom-apq8064.dtsi | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> index 65d0e8d..c6ff8fc 100644
> --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> @@ -227,7 +227,8 @@
>  
>  			gsbi1_i2c: i2c@12460000 {
>  				compatible = "qcom,i2c-qup-v1.1.1";
> -				pinctrl-0 = <&i2c1_pins &i2c1_pins_sleep>;
> +				pinctrl-0 = <&i2c1_pins>;
> +				pinctrl-1 = <&i2c1_pins_sleep>;
>  				pinctrl-names = "default", "sleep";
>  				reg = <0x12460000 0x1000>;
>  				interrupts = <0 194 IRQ_TYPE_NONE>;
> @@ -255,7 +256,8 @@
>  			gsbi2_i2c: i2c@124a0000 {
>  				compatible = "qcom,i2c-qup-v1.1.1";
>  				reg = <0x124a0000 0x1000>;
> -				pinctrl-0 = <&i2c2_pins &i2c2_pins_sleep>;
> +				pinctrl-0 = <&i2c2_pins>;
> +				pinctrl-1 = <&i2c2_pins_sleep>;
>  				pinctrl-names = "default", "sleep";
>  				interrupts = <0 196 IRQ_TYPE_NONE>;
>  				clocks = <&gcc GSBI2_QUP_CLK>, <&gcc GSBI2_H_CLK>;
> @@ -277,7 +279,8 @@
>  			ranges;
>  			gsbi3_i2c: i2c@16280000 {
>  				compatible = "qcom,i2c-qup-v1.1.1";
> -				pinctrl-0 = <&i2c3_pins &i2c3_pins_sleep>;
> +				pinctrl-0 = <&i2c3_pins>;
> +				pinctrl-1 = <&i2c3_pins_sleep>;
>  				pinctrl-names = "default", "sleep";
>  				reg = <0x16280000 0x1000>;
>  				interrupts = <GIC_SPI 151 IRQ_TYPE_NONE>;
> @@ -302,7 +305,8 @@
>  
>  			gsbi4_i2c: i2c@16380000 {
>  				compatible = "qcom,i2c-qup-v1.1.1";
> -				pinctrl-0 = <&i2c4_pins &i2c4_pins_sleep>;
> +				pinctrl-0 = <&i2c4_pins>;
> +				pinctrl-1 = <&i2c4_pins_sleep>;
>  				pinctrl-names = "default", "sleep";
>  				reg = <0x16380000 0x1000>;
>  				interrupts = <GIC_SPI 153 IRQ_TYPE_NONE>;
> @@ -337,7 +341,8 @@
>  				compatible = "qcom,spi-qup-v1.1.1";
>  				reg = <0x1a280000 0x1000>;
>  				interrupts = <0 155 0>;
> -				pinctrl-0 = <&spi5_default &spi5_sleep>;
> +				pinctrl-0 = <&spi5_default>;
> +				pinctrl-1 = <&spi5_sleep>;
>  				pinctrl-names = "default", "sleep";
>  				clocks = <&gcc GSBI5_QUP_CLK>, <&gcc GSBI5_H_CLK>;
>  				clock-names = "core", "iface";
> @@ -370,7 +375,8 @@
>  
>  			gsbi6_i2c: i2c@16580000 {
>  				compatible = "qcom,i2c-qup-v1.1.1";
> -				pinctrl-0 = <&i2c6_pins &i2c6_pins_sleep>;
> +				pinctrl-0 = <&i2c6_pins>;
> +				pinctrl-1 = <&i2c6_pins_sleep>;
>  				pinctrl-names = "default", "sleep";
>  				reg = <0x16580000 0x1000>;
>  				interrupts = <GIC_SPI 157 IRQ_TYPE_NONE>;
> -- 
> 2.5.0
> 

[toc] | [prev] | [next] | [standalone]

#1366371 — Re: [PATCH 01/12] ARM: dts: apq8064: fix the pinctrls for i2c and spi

On 29/03/16 15:28, Bjorn Andersson wrote:
> On Wed 23 Mar 12:47 PDT 2016, Srinivas Kandagatla wrote:
>
>> This patch fixes pinctrls for spi and i2c nodes whose default and sleep
>> states are together, which is incorrect.
>>
>> Without this patch i2c/spi would not be functional.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> The change in itself is sound, but I don't understand why i2c/spi fails
> to function if we don't bring them to a sleep state. Can you please
> update the commit message with an explanation?

Yes, with the existing code the device would endup configuring the 
default pinstate to sleep pinconf. So the i2c bus would not be functional.

If you try mainline on any 8064 based boards you would easily reproduce 
the bug. For example read the eeprom on IFC6410.
>
>
> PS. Whenever there's multiple states for a thing I do prefer prefixing
> them _a and _s to highlight that difference (not only suffixing the
> sleep state). And use abbreviations :)

If I search for _sleep in dts folder these are widely used, am not sure 
which is the prefered way to do this, as long as its readable by user am 
ok to do it either way.

>
> Regards,
> Bjorn
>
>> ---
>>   arch/arm/boot/dts/qcom-apq8064.dtsi | 18 ++++++++++++------
>>   1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
>> index 65d0e8d..c6ff8fc 100644
>> --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
>> +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
>> @@ -227,7 +227,8 @@
>>
>>   			gsbi1_i2c: i2c@12460000 {
>>   				compatible = "qcom,i2c-qup-v1.1.1";
>> -				pinctrl-0 = <&i2c1_pins &i2c1_pins_sleep>;
>> +				pinctrl-0 = <&i2c1_pins>;
>> +				pinctrl-1 = <&i2c1_pins_sleep>;
>>   				pinctrl-names = "default", "sleep";
>>   				reg = <0x12460000 0x1000>;
>>   				interrupts = <0 194 IRQ_TYPE_NONE>;
>> @@ -255,7 +256,8 @@
>>   			gsbi2_i2c: i2c@124a0000 {
>>   				compatible = "qcom,i2c-qup-v1.1.1";
>>   				reg = <0x124a0000 0x1000>;
>> -				pinctrl-0 = <&i2c2_pins &i2c2_pins_sleep>;
>> +				pinctrl-0 = <&i2c2_pins>;
>> +				pinctrl-1 = <&i2c2_pins_sleep>;
>>   				pinctrl-names = "default", "sleep";
>>   				interrupts = <0 196 IRQ_TYPE_NONE>;
>>   				clocks = <&gcc GSBI2_QUP_CLK>, <&gcc GSBI2_H_CLK>;
>> @@ -277,7 +279,8 @@
>>   			ranges;
>>   			gsbi3_i2c: i2c@16280000 {
>>   				compatible = "qcom,i2c-qup-v1.1.1";
>> -				pinctrl-0 = <&i2c3_pins &i2c3_pins_sleep>;
>> +				pinctrl-0 = <&i2c3_pins>;
>> +				pinctrl-1 = <&i2c3_pins_sleep>;
>>   				pinctrl-names = "default", "sleep";
>>   				reg = <0x16280000 0x1000>;
>>   				interrupts = <GIC_SPI 151 IRQ_TYPE_NONE>;
>> @@ -302,7 +305,8 @@
>>
>>   			gsbi4_i2c: i2c@16380000 {
>>   				compatible = "qcom,i2c-qup-v1.1.1";
>> -				pinctrl-0 = <&i2c4_pins &i2c4_pins_sleep>;
>> +				pinctrl-0 = <&i2c4_pins>;
>> +				pinctrl-1 = <&i2c4_pins_sleep>;
>>   				pinctrl-names = "default", "sleep";
>>   				reg = <0x16380000 0x1000>;
>>   				interrupts = <GIC_SPI 153 IRQ_TYPE_NONE>;
>> @@ -337,7 +341,8 @@
>>   				compatible = "qcom,spi-qup-v1.1.1";
>>   				reg = <0x1a280000 0x1000>;
>>   				interrupts = <0 155 0>;
>> -				pinctrl-0 = <&spi5_default &spi5_sleep>;
>> +				pinctrl-0 = <&spi5_default>;
>> +				pinctrl-1 = <&spi5_sleep>;
>>   				pinctrl-names = "default", "sleep";
>>   				clocks = <&gcc GSBI5_QUP_CLK>, <&gcc GSBI5_H_CLK>;
>>   				clock-names = "core", "iface";
>> @@ -370,7 +375,8 @@
>>
>>   			gsbi6_i2c: i2c@16580000 {
>>   				compatible = "qcom,i2c-qup-v1.1.1";
>> -				pinctrl-0 = <&i2c6_pins &i2c6_pins_sleep>;
>> +				pinctrl-0 = <&i2c6_pins>;
>> +				pinctrl-1 = <&i2c6_pins_sleep>;
>>   				pinctrl-names = "default", "sleep";
>>   				reg = <0x16580000 0x1000>;
>>   				interrupts = <GIC_SPI 157 IRQ_TYPE_NONE>;
>> --
>> 2.5.0
>>

[toc] | [prev] | [next] | [standalone]

#1366433 — Re: [PATCH 01/12] ARM: dts: apq8064: fix the pinctrls for i2c and spi

On Tue 29 Mar 08:02 PDT 2016, Srinivas Kandagatla wrote:

> 
> 
> On 29/03/16 15:28, Bjorn Andersson wrote:
> >On Wed 23 Mar 12:47 PDT 2016, Srinivas Kandagatla wrote:
> >
> >>This patch fixes pinctrls for spi and i2c nodes whose default and sleep
> >>states are together, which is incorrect.
> >>
> >>Without this patch i2c/spi would not be functional.
> >>
> >>Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> >
> >The change in itself is sound, but I don't understand why i2c/spi fails
> >to function if we don't bring them to a sleep state. Can you please
> >update the commit message with an explanation?
> 
> Yes, with the existing code the device would endup configuring the default
> pinstate to sleep pinconf. So the i2c bus would not be functional.
> 

Ohh sorry, not sure why I didn't see that. Your fix is obviously
correct.

Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> If you try mainline on any 8064 based boards you would easily reproduce the
> bug. For example read the eeprom on IFC6410.
> >
> >
> >PS. Whenever there's multiple states for a thing I do prefer prefixing
> >them _a and _s to highlight that difference (not only suffixing the
> >sleep state). And use abbreviations :)
> 
> If I search for _sleep in dts folder these are widely used, am not sure
> which is the prefered way to do this, as long as its readable by user am ok
> to do it either way.
> 
> >
> >Regards,
> >Bjorn
> >
> >>---
> >>  arch/arm/boot/dts/qcom-apq8064.dtsi | 18 ++++++++++++------
> >>  1 file changed, 12 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> >>index 65d0e8d..c6ff8fc 100644
> >>--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> >>+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> >>@@ -227,7 +227,8 @@
> >>
> >>  			gsbi1_i2c: i2c@12460000 {
> >>  				compatible = "qcom,i2c-qup-v1.1.1";
> >>-				pinctrl-0 = <&i2c1_pins &i2c1_pins_sleep>;
> >>+				pinctrl-0 = <&i2c1_pins>;
> >>+				pinctrl-1 = <&i2c1_pins_sleep>;
> >>  				pinctrl-names = "default", "sleep";
> >>  				reg = <0x12460000 0x1000>;
> >>  				interrupts = <0 194 IRQ_TYPE_NONE>;
> >>@@ -255,7 +256,8 @@
> >>  			gsbi2_i2c: i2c@124a0000 {
> >>  				compatible = "qcom,i2c-qup-v1.1.1";
> >>  				reg = <0x124a0000 0x1000>;
> >>-				pinctrl-0 = <&i2c2_pins &i2c2_pins_sleep>;
> >>+				pinctrl-0 = <&i2c2_pins>;
> >>+				pinctrl-1 = <&i2c2_pins_sleep>;
> >>  				pinctrl-names = "default", "sleep";
> >>  				interrupts = <0 196 IRQ_TYPE_NONE>;
> >>  				clocks = <&gcc GSBI2_QUP_CLK>, <&gcc GSBI2_H_CLK>;
> >>@@ -277,7 +279,8 @@
> >>  			ranges;
> >>  			gsbi3_i2c: i2c@16280000 {
> >>  				compatible = "qcom,i2c-qup-v1.1.1";
> >>-				pinctrl-0 = <&i2c3_pins &i2c3_pins_sleep>;
> >>+				pinctrl-0 = <&i2c3_pins>;
> >>+				pinctrl-1 = <&i2c3_pins_sleep>;
> >>  				pinctrl-names = "default", "sleep";
> >>  				reg = <0x16280000 0x1000>;
> >>  				interrupts = <GIC_SPI 151 IRQ_TYPE_NONE>;
> >>@@ -302,7 +305,8 @@
> >>
> >>  			gsbi4_i2c: i2c@16380000 {
> >>  				compatible = "qcom,i2c-qup-v1.1.1";
> >>-				pinctrl-0 = <&i2c4_pins &i2c4_pins_sleep>;
> >>+				pinctrl-0 = <&i2c4_pins>;
> >>+				pinctrl-1 = <&i2c4_pins_sleep>;
> >>  				pinctrl-names = "default", "sleep";
> >>  				reg = <0x16380000 0x1000>;
> >>  				interrupts = <GIC_SPI 153 IRQ_TYPE_NONE>;
> >>@@ -337,7 +341,8 @@
> >>  				compatible = "qcom,spi-qup-v1.1.1";
> >>  				reg = <0x1a280000 0x1000>;
> >>  				interrupts = <0 155 0>;
> >>-				pinctrl-0 = <&spi5_default &spi5_sleep>;
> >>+				pinctrl-0 = <&spi5_default>;
> >>+				pinctrl-1 = <&spi5_sleep>;
> >>  				pinctrl-names = "default", "sleep";
> >>  				clocks = <&gcc GSBI5_QUP_CLK>, <&gcc GSBI5_H_CLK>;
> >>  				clock-names = "core", "iface";
> >>@@ -370,7 +375,8 @@
> >>
> >>  			gsbi6_i2c: i2c@16580000 {
> >>  				compatible = "qcom,i2c-qup-v1.1.1";
> >>-				pinctrl-0 = <&i2c6_pins &i2c6_pins_sleep>;
> >>+				pinctrl-0 = <&i2c6_pins>;
> >>+				pinctrl-1 = <&i2c6_pins_sleep>;
> >>  				pinctrl-names = "default", "sleep";
> >>  				reg = <0x16580000 0x1000>;
> >>  				interrupts = <GIC_SPI 157 IRQ_TYPE_NONE>;
> >>--
> >>2.5.0
> >>

[toc] | [prev] | [next] | [standalone]

#1366363

On Wed 23 Mar 12:46 PDT 2016, Srinivas Kandagatla wrote:

> Hi Andy,
> 
> This patchset adds support to Dragonboard 600c which is based on APQ8064.
> 
> With this patchset, except spi I was able to test all the below features
> on this board on top of linus master branch.
> 1> i2c
> 2> spi
> 3> sd/mmc with card detect
> 4> eMMC
> 5> USB
> 6> SATA
> 7> on board Ethernet based on PCIE.
> 8> user and activity leds.
> 
> AHCI SATA is broken on APQ8064 on mainline, due to changes in libahci, fix is
> being discussed which should not effect this dt patches anyway.
> This patchset also has a fix to i2c/spi pinctrls which was not set correctly
> in my previous apq8064 patches, I have verified this patchset with eeprom and
> sensors on the board, you might want to take that patch in next rc.
> 
> Thanks,
> srini
> 
> Srinivas Kandagatla (12):
>   ARM: dts: apq8064: fix the pinctrls for i2c and spi
>   ARM: dts: apq8064: add support to gsbi1 uart
>   ARM: dts: apq8064: add gsbi7 i2c support
>   ARM: dts: dragonboard-600c: add board support with serial
>   ARM: dts: dragonboard-600c: add pmic regulator supplies
>   ARM: dts: dragonboard-600c: Add eMMC and SD card support
>   ARM: dts: dragonboard-600c: add usb support
>   ARM: dts: dragonboard-600c: add pcie support
>   ARM: dts: dragonboard-600c: add on board sata support.
>   ARM: dts: dragonboard-600c: Add on board leds support
>   ARM: dts: dragonboard-600c: add i2c support
>   ARM: dts: dragonboard-600c: add spi support

Please abbreviate db600c (and drop those upper case As).

> 
>  arch/arm/boot/dts/Makefile                         |   1 +
>  .../arm/boot/dts/qcom-apq8064-dragonboard-600c.dts | 377 +++++++++++++++++++++

Please make this qcom-apq8064-arrow-db600c.dts

>  arch/arm/boot/dts/qcom-apq8064-pins.dtsi           |  39 +++
>  arch/arm/boot/dts/qcom-apq8064.dtsi                |  41 ++-
>  4 files changed, 452 insertions(+), 6 deletions(-)
>  create mode 100644 arch/arm/boot/dts/qcom-apq8064-dragonboard-600c.dts

Regards,
Bjorn

[toc] | [prev] | [next] | [standalone]

#1366372

On 29/03/16 15:54, Bjorn Andersson wrote:
> On Wed 23 Mar 12:46 PDT 2016, Srinivas Kandagatla wrote:
>
>> Hi Andy,
>>
>> This patchset adds support to Dragonboard 600c which is based on APQ8064.
>>
>> With this patchset, except spi I was able to test all the below features
>> on this board on top of linus master branch.
>> 1> i2c
>> 2> spi
>> 3> sd/mmc with card detect
>> 4> eMMC
>> 5> USB
>> 6> SATA
>> 7> on board Ethernet based on PCIE.
>> 8> user and activity leds.
>>
>> AHCI SATA is broken on APQ8064 on mainline, due to changes in libahci, fix is
>> being discussed which should not effect this dt patches anyway.
>> This patchset also has a fix to i2c/spi pinctrls which was not set correctly
>> in my previous apq8064 patches, I have verified this patchset with eeprom and
>> sensors on the board, you might want to take that patch in next rc.
>>
>> Thanks,
>> srini
>>
>> Srinivas Kandagatla (12):
>>    ARM: dts: apq8064: fix the pinctrls for i2c and spi
>>    ARM: dts: apq8064: add support to gsbi1 uart
>>    ARM: dts: apq8064: add gsbi7 i2c support
>>    ARM: dts: dragonboard-600c: add board support with serial
>>    ARM: dts: dragonboard-600c: add pmic regulator supplies
>>    ARM: dts: dragonboard-600c: Add eMMC and SD card support
>>    ARM: dts: dragonboard-600c: add usb support
>>    ARM: dts: dragonboard-600c: add pcie support
>>    ARM: dts: dragonboard-600c: add on board sata support.
>>    ARM: dts: dragonboard-600c: Add on board leds support
>>    ARM: dts: dragonboard-600c: add i2c support
>>    ARM: dts: dragonboard-600c: add spi support
>
> Please abbreviate db600c (and drop those upper case As).

Yep. Will do.
>
>>
>>   arch/arm/boot/dts/Makefile                         |   1 +
>>   .../arm/boot/dts/qcom-apq8064-dragonboard-600c.dts | 377 +++++++++++++++++++++
>
> Please make this qcom-apq8064-arrow-db600c.dts
Ok, makes sense.

thanks
srini
>
>>   arch/arm/boot/dts/qcom-apq8064-pins.dtsi           |  39 +++
>>   arch/arm/boot/dts/qcom-apq8064.dtsi                |  41 ++-
>>   4 files changed, 452 insertions(+), 6 deletions(-)
>>   create mode 100644 arch/arm/boot/dts/qcom-apq8064-dragonboard-600c.dts
>
> Regards,
> Bjorn
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web