[PATCH 0/4] arm64: dts: qcom: dts improvements

[PATCH 0/4] arm64: dts: qcom: dts improvements

[Damien Riegel]

The goal of this series was to add missing I2C bindings for BLSP_I2C1,
BLSP_I2C3, and BLSP_I2C5. But while working on this, I noticed some
styling issues and decided to tackle them in the same patchset, mostly
because they touch the same files and the same people will be involved
for the review. Let me know if you'd rather see them sent separately.

First patch reindents the wcd_codec block in pm8916.dtsi with respect
to the kernel standards.

Second patch and third patch makes existing I2C more in sync with the
rest of the bindings by dropping assignments to a property that doesn't
take a value, and by making their style matches thus of SPI bindings.

Last patch add bindings of I2C cores 1, 3, and 5.

Damien Riegel (4):
  arm64: dts: qcom: pm8916: fix wcd_codec indentation
  arm64: dts: qcom: msm8916-pins: remove assignments to bias-disable
  arm64: dts: qcom: msm8916: normalize I2C bindings
  arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5

 arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 84 +++++++++++++++++++++++++++---
 arch/arm64/boot/dts/qcom/msm8916.dtsi      | 57 +++++++++++++++++---
 arch/arm64/boot/dts/qcom/pm8916.dtsi       | 82 ++++++++++++++---------------
 3 files changed, 169 insertions(+), 54 deletions(-)

-- 
2.15.0

[PATCH 1/4] arm64: dts: qcom: pm8916: fix wcd_codec indentation

[Damien Riegel]

Indentation did not respect kernel standards, so fix that for the usual
indent with tabs, align with spaces. While at it, remove some empty
lines before and after the closing parenthesis of this block.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
---
 arch/arm64/boot/dts/qcom/pm8916.dtsi | 82 ++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 42 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pm8916.dtsi b/arch/arm64/boot/dts/qcom/pm8916.dtsi
index 53deebf9f515..d19f4cb9a5f3 100644
--- a/arch/arm64/boot/dts/qcom/pm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8916.dtsi
@@ -96,47 +96,45 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-                wcd_codec: codec@f000 {
-                       compatible = "qcom,pm8916-wcd-analog-codec";
-	               reg = <0xf000 0x200>;
-	               reg-names = "pmic-codec-core";
-	               clocks = <&gcc GCC_CODEC_DIGCODEC_CLK>;
-	               clock-names = "mclk";
-	               interrupt-parent = <&spmi_bus>;
-	               interrupts = <0x1 0xf0 0x0 IRQ_TYPE_NONE>,
-	                            <0x1 0xf0 0x1 IRQ_TYPE_NONE>,
-	                            <0x1 0xf0 0x2 IRQ_TYPE_NONE>,
-	                            <0x1 0xf0 0x3 IRQ_TYPE_NONE>,
-	                            <0x1 0xf0 0x4 IRQ_TYPE_NONE>,
-	                            <0x1 0xf0 0x5 IRQ_TYPE_NONE>,
-	                            <0x1 0xf0 0x6 IRQ_TYPE_NONE>,
-	                            <0x1 0xf0 0x7 IRQ_TYPE_NONE>,
-	                            <0x1 0xf1 0x0 IRQ_TYPE_NONE>,
-	                            <0x1 0xf1 0x1 IRQ_TYPE_NONE>,
-	                            <0x1 0xf1 0x2 IRQ_TYPE_NONE>,
-	                            <0x1 0xf1 0x3 IRQ_TYPE_NONE>,
-	                            <0x1 0xf1 0x4 IRQ_TYPE_NONE>,
-	                            <0x1 0xf1 0x5 IRQ_TYPE_NONE>;
-	               interrupt-names = "cdc_spk_cnp_int",
-	                                 "cdc_spk_clip_int",
-	                                 "cdc_spk_ocp_int",
-	                                 "mbhc_ins_rem_det1",
-	                                 "mbhc_but_rel_det",
-	                                 "mbhc_but_press_det",
-	                                 "mbhc_ins_rem_det",
-	                                 "mbhc_switch_int",
-	                                 "cdc_ear_ocp_int",
-	                                 "cdc_hphr_ocp_int",
-	                                 "cdc_hphl_ocp_det",
-	                                 "cdc_ear_cnp_int",
-	                                 "cdc_hphr_cnp_int",
-	                                 "cdc_hphl_cnp_int";
-	               vdd-cdc-io-supply = <&pm8916_l5>;
-	               vdd-cdc-tx-rx-cx-supply = <&pm8916_l5>;
-	               vdd-micbias-supply = <&pm8916_l13>;
-	               #sound-dai-cells = <1>;
-
-                };
-
+		wcd_codec: codec@f000 {
+			compatible = "qcom,pm8916-wcd-analog-codec";
+			reg = <0xf000 0x200>;
+			reg-names = "pmic-codec-core";
+			clocks = <&gcc GCC_CODEC_DIGCODEC_CLK>;
+			clock-names = "mclk";
+			interrupt-parent = <&spmi_bus>;
+			interrupts = <0x1 0xf0 0x0 IRQ_TYPE_NONE>,
+				     <0x1 0xf0 0x1 IRQ_TYPE_NONE>,
+				     <0x1 0xf0 0x2 IRQ_TYPE_NONE>,
+				     <0x1 0xf0 0x3 IRQ_TYPE_NONE>,
+				     <0x1 0xf0 0x4 IRQ_TYPE_NONE>,
+				     <0x1 0xf0 0x5 IRQ_TYPE_NONE>,
+				     <0x1 0xf0 0x6 IRQ_TYPE_NONE>,
+				     <0x1 0xf0 0x7 IRQ_TYPE_NONE>,
+				     <0x1 0xf1 0x0 IRQ_TYPE_NONE>,
+				     <0x1 0xf1 0x1 IRQ_TYPE_NONE>,
+				     <0x1 0xf1 0x2 IRQ_TYPE_NONE>,
+				     <0x1 0xf1 0x3 IRQ_TYPE_NONE>,
+				     <0x1 0xf1 0x4 IRQ_TYPE_NONE>,
+				     <0x1 0xf1 0x5 IRQ_TYPE_NONE>;
+			interrupt-names = "cdc_spk_cnp_int",
+					  "cdc_spk_clip_int",
+					  "cdc_spk_ocp_int",
+					  "mbhc_ins_rem_det1",
+					  "mbhc_but_rel_det",
+					  "mbhc_but_press_det",
+					  "mbhc_ins_rem_det",
+					  "mbhc_switch_int",
+					  "cdc_ear_ocp_int",
+					  "cdc_hphr_ocp_int",
+					  "cdc_hphl_ocp_det",
+					  "cdc_ear_cnp_int",
+					  "cdc_hphr_cnp_int",
+					  "cdc_hphl_cnp_int";
+			vdd-cdc-io-supply = <&pm8916_l5>;
+			vdd-cdc-tx-rx-cx-supply = <&pm8916_l5>;
+			vdd-micbias-supply = <&pm8916_l13>;
+			#sound-dai-cells = <1>;
+		};
 	};
 };
-- 
2.15.0

Re: [PATCH 1/4] arm64: dts: qcom: pm8916: fix wcd_codec indentation

[Bjorn Andersson]

On Wed 01 Nov 10:53 PDT 2017, Damien Riegel wrote:

> Indentation did not respect kernel standards, so fix that for the usual
> indent with tabs, align with spaces. While at it, remove some empty
> lines before and after the closing parenthesis of this block.
> 
> Signed-off-by: Damien Riegel <damien.riegel-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Regards,
Bjorn

> ---
>  arch/arm64/boot/dts/qcom/pm8916.dtsi | 82 ++++++++++++++++++------------------
>  1 file changed, 40 insertions(+), 42 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm8916.dtsi b/arch/arm64/boot/dts/qcom/pm8916.dtsi
> index 53deebf9f515..d19f4cb9a5f3 100644
> --- a/arch/arm64/boot/dts/qcom/pm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm8916.dtsi
> @@ -96,47 +96,45 @@
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  
> -                wcd_codec: codec@f000 {
> -                       compatible = "qcom,pm8916-wcd-analog-codec";
> -	               reg = <0xf000 0x200>;
> -	               reg-names = "pmic-codec-core";
> -	               clocks = <&gcc GCC_CODEC_DIGCODEC_CLK>;
> -	               clock-names = "mclk";
> -	               interrupt-parent = <&spmi_bus>;
> -	               interrupts = <0x1 0xf0 0x0 IRQ_TYPE_NONE>,
> -	                            <0x1 0xf0 0x1 IRQ_TYPE_NONE>,
> -	                            <0x1 0xf0 0x2 IRQ_TYPE_NONE>,
> -	                            <0x1 0xf0 0x3 IRQ_TYPE_NONE>,
> -	                            <0x1 0xf0 0x4 IRQ_TYPE_NONE>,
> -	                            <0x1 0xf0 0x5 IRQ_TYPE_NONE>,
> -	                            <0x1 0xf0 0x6 IRQ_TYPE_NONE>,
> -	                            <0x1 0xf0 0x7 IRQ_TYPE_NONE>,
> -	                            <0x1 0xf1 0x0 IRQ_TYPE_NONE>,
> -	                            <0x1 0xf1 0x1 IRQ_TYPE_NONE>,
> -	                            <0x1 0xf1 0x2 IRQ_TYPE_NONE>,
> -	                            <0x1 0xf1 0x3 IRQ_TYPE_NONE>,
> -	                            <0x1 0xf1 0x4 IRQ_TYPE_NONE>,
> -	                            <0x1 0xf1 0x5 IRQ_TYPE_NONE>;
> -	               interrupt-names = "cdc_spk_cnp_int",
> -	                                 "cdc_spk_clip_int",
> -	                                 "cdc_spk_ocp_int",
> -	                                 "mbhc_ins_rem_det1",
> -	                                 "mbhc_but_rel_det",
> -	                                 "mbhc_but_press_det",
> -	                                 "mbhc_ins_rem_det",
> -	                                 "mbhc_switch_int",
> -	                                 "cdc_ear_ocp_int",
> -	                                 "cdc_hphr_ocp_int",
> -	                                 "cdc_hphl_ocp_det",
> -	                                 "cdc_ear_cnp_int",
> -	                                 "cdc_hphr_cnp_int",
> -	                                 "cdc_hphl_cnp_int";
> -	               vdd-cdc-io-supply = <&pm8916_l5>;
> -	               vdd-cdc-tx-rx-cx-supply = <&pm8916_l5>;
> -	               vdd-micbias-supply = <&pm8916_l13>;
> -	               #sound-dai-cells = <1>;
> -
> -                };
> -
> +		wcd_codec: codec@f000 {
> +			compatible = "qcom,pm8916-wcd-analog-codec";
> +			reg = <0xf000 0x200>;
> +			reg-names = "pmic-codec-core";
> +			clocks = <&gcc GCC_CODEC_DIGCODEC_CLK>;
> +			clock-names = "mclk";
> +			interrupt-parent = <&spmi_bus>;
> +			interrupts = <0x1 0xf0 0x0 IRQ_TYPE_NONE>,
> +				     <0x1 0xf0 0x1 IRQ_TYPE_NONE>,
> +				     <0x1 0xf0 0x2 IRQ_TYPE_NONE>,
> +				     <0x1 0xf0 0x3 IRQ_TYPE_NONE>,
> +				     <0x1 0xf0 0x4 IRQ_TYPE_NONE>,
> +				     <0x1 0xf0 0x5 IRQ_TYPE_NONE>,
> +				     <0x1 0xf0 0x6 IRQ_TYPE_NONE>,
> +				     <0x1 0xf0 0x7 IRQ_TYPE_NONE>,
> +				     <0x1 0xf1 0x0 IRQ_TYPE_NONE>,
> +				     <0x1 0xf1 0x1 IRQ_TYPE_NONE>,
> +				     <0x1 0xf1 0x2 IRQ_TYPE_NONE>,
> +				     <0x1 0xf1 0x3 IRQ_TYPE_NONE>,
> +				     <0x1 0xf1 0x4 IRQ_TYPE_NONE>,
> +				     <0x1 0xf1 0x5 IRQ_TYPE_NONE>;
> +			interrupt-names = "cdc_spk_cnp_int",
> +					  "cdc_spk_clip_int",
> +					  "cdc_spk_ocp_int",
> +					  "mbhc_ins_rem_det1",
> +					  "mbhc_but_rel_det",
> +					  "mbhc_but_press_det",
> +					  "mbhc_ins_rem_det",
> +					  "mbhc_switch_int",
> +					  "cdc_ear_ocp_int",
> +					  "cdc_hphr_ocp_int",
> +					  "cdc_hphl_ocp_det",
> +					  "cdc_ear_cnp_int",
> +					  "cdc_hphr_cnp_int",
> +					  "cdc_hphl_cnp_int";
> +			vdd-cdc-io-supply = <&pm8916_l5>;
> +			vdd-cdc-tx-rx-cx-supply = <&pm8916_l5>;
> +			vdd-micbias-supply = <&pm8916_l13>;
> +			#sound-dai-cells = <1>;
> +		};
>  	};
>  };
> -- 
> 2.15.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] arm64: dts: qcom: msm8916-pins: remove assignments to bias-disable

[Damien Riegel]

Drop assignments to bias-disable as the documentation [1] states that
this property doesn't take a value. Other occurrences of this property
respect that.

[1] Documentation/devicetree/bindings/pinctrl/qcom,msm8916-pinctrl.txt

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
---
 arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
index 4cb0b5834143..c67ad8ed8b60 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
@@ -278,7 +278,7 @@
 		pinconf {
 			pins = "gpio6", "gpio7";
 			drive-strength = <16>;
-			bias-disable = <0>;
+			bias-disable;
 		};
 	};
 
@@ -290,7 +290,7 @@
 		pinconf {
 			pins = "gpio6", "gpio7";
 			drive-strength = <2>;
-			bias-disable = <0>;
+			bias-disable;
 		};
 	};
 
@@ -302,7 +302,7 @@
 		pinconf {
 			pins = "gpio14", "gpio15";
 			drive-strength = <16>;
-			bias-disable = <0>;
+			bias-disable;
 		};
 	};
 
@@ -314,7 +314,7 @@
 		pinconf {
 			pins = "gpio14", "gpio15";
 			drive-strength = <2>;
-			bias-disable = <0>;
+			bias-disable;
 		};
 	};
 
@@ -326,7 +326,7 @@
 		pinconf {
 			pins = "gpio22", "gpio23";
 			drive-strength = <16>;
-			bias-disable = <0>;
+			bias-disable;
 		};
 	};
 
@@ -338,7 +338,7 @@
 		pinconf {
 			pins = "gpio22", "gpio23";
 			drive-strength = <2>;
-			bias-disable = <0>;
+			bias-disable;
 		};
 	};
 
-- 
2.15.0

Re: [PATCH 2/4] arm64: dts: qcom: msm8916-pins: remove assignments to bias-disable

[Bjorn Andersson]

On Wed 01 Nov 10:53 PDT 2017, Damien Riegel wrote:

> Drop assignments to bias-disable as the documentation [1] states that
> this property doesn't take a value. Other occurrences of this property
> respect that.
> 
> [1] Documentation/devicetree/bindings/pinctrl/qcom,msm8916-pinctrl.txt
> 

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

Regards,
Bjorn

> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> ---
>  arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
> index 4cb0b5834143..c67ad8ed8b60 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
> @@ -278,7 +278,7 @@
>  		pinconf {
>  			pins = "gpio6", "gpio7";
>  			drive-strength = <16>;
> -			bias-disable = <0>;
> +			bias-disable;
>  		};
>  	};
>  
> @@ -290,7 +290,7 @@
>  		pinconf {
>  			pins = "gpio6", "gpio7";
>  			drive-strength = <2>;
> -			bias-disable = <0>;
> +			bias-disable;
>  		};
>  	};
>  
> @@ -302,7 +302,7 @@
>  		pinconf {
>  			pins = "gpio14", "gpio15";
>  			drive-strength = <16>;
> -			bias-disable = <0>;
> +			bias-disable;
>  		};
>  	};
>  
> @@ -314,7 +314,7 @@
>  		pinconf {
>  			pins = "gpio14", "gpio15";
>  			drive-strength = <2>;
> -			bias-disable = <0>;
> +			bias-disable;
>  		};
>  	};
>  
> @@ -326,7 +326,7 @@
>  		pinconf {
>  			pins = "gpio22", "gpio23";
>  			drive-strength = <16>;
> -			bias-disable = <0>;
> +			bias-disable;
>  		};
>  	};
>  
> @@ -338,7 +338,7 @@
>  		pinconf {
>  			pins = "gpio22", "gpio23";
>  			drive-strength = <2>;
> -			bias-disable = <0>;
> +			bias-disable;
>  		};
>  	};
>  
> -- 
> 2.15.0
> 

[PATCH 3/4] arm64: dts: qcom: msm8916: normalize I2C bindings

[Damien Riegel]

The QUP core can be used either for I2C or SPI, so the same IP is mapped
by a driver or the other. SPI bindings use a leading 0 for the start
address and a size of 0x600, I2C bindings don't have the leading 0 and
have a size 0x1000.

To make them more similar, add the leading 0 to I2C bindings and changes
the size to 0x600, as the driver only accesses registers up to address
0x408. Also align the second entry of the clocks array.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index e16ba8334518..de25bd6070f5 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -457,10 +457,10 @@
 
 		blsp_i2c2: i2c@78b6000 {
 			compatible = "qcom,i2c-qup-v2.2.1";
-			reg = <0x78b6000 0x1000>;
+			reg = <0x078b6000 0x600>;
 			interrupts = <GIC_SPI 96 0>;
 			clocks = <&gcc GCC_BLSP1_AHB_CLK>,
-				<&gcc GCC_BLSP1_QUP2_I2C_APPS_CLK>;
+				 <&gcc GCC_BLSP1_QUP2_I2C_APPS_CLK>;
 			clock-names = "iface", "core";
 			pinctrl-names = "default", "sleep";
 			pinctrl-0 = <&i2c2_default>;
@@ -472,10 +472,10 @@
 
 		blsp_i2c4: i2c@78b8000 {
 			compatible = "qcom,i2c-qup-v2.2.1";
-			reg = <0x78b8000 0x1000>;
+			reg = <0x078b8000 0x600>;
 			interrupts = <GIC_SPI 98 0>;
 			clocks = <&gcc GCC_BLSP1_AHB_CLK>,
-				<&gcc GCC_BLSP1_QUP4_I2C_APPS_CLK>;
+				 <&gcc GCC_BLSP1_QUP4_I2C_APPS_CLK>;
 			clock-names = "iface", "core";
 			pinctrl-names = "default", "sleep";
 			pinctrl-0 = <&i2c4_default>;
@@ -487,10 +487,10 @@
 
 		blsp_i2c6: i2c@78ba000 {
 			compatible = "qcom,i2c-qup-v2.2.1";
-			reg = <0x78ba000 0x1000>;
+			reg = <0x078ba000 0x600>;
 			interrupts = <GIC_SPI 100 0>;
 			clocks = <&gcc GCC_BLSP1_AHB_CLK>,
-				<&gcc GCC_BLSP1_QUP6_I2C_APPS_CLK>;
+				 <&gcc GCC_BLSP1_QUP6_I2C_APPS_CLK>;
 			clock-names = "iface", "core";
 			pinctrl-names = "default", "sleep";
 			pinctrl-0 = <&i2c6_default>;
-- 
2.15.0

Re: [PATCH 3/4] arm64: dts: qcom: msm8916: normalize I2C bindings

[Bjorn Andersson]

On Wed 01 Nov 10:53 PDT 2017, Damien Riegel wrote:

> The QUP core can be used either for I2C or SPI, so the same IP is mapped
> by a driver or the other. SPI bindings use a leading 0 for the start
> address and a size of 0x600, I2C bindings don't have the leading 0 and
> have a size 0x1000.
> 
> To make them more similar, add the leading 0 to I2C bindings and changes
> the size to 0x600, as the driver only accesses registers up to address
> 0x408. Also align the second entry of the clocks array.
> 

The correct size for these blocks are 0x500, please update this. Other
than that this looks good.

Regards,
Bjorn

> Signed-off-by: Damien Riegel <damien.riegel-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8916.dtsi | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index e16ba8334518..de25bd6070f5 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -457,10 +457,10 @@
>  
>  		blsp_i2c2: i2c@78b6000 {
>  			compatible = "qcom,i2c-qup-v2.2.1";
> -			reg = <0x78b6000 0x1000>;
> +			reg = <0x078b6000 0x600>;
>  			interrupts = <GIC_SPI 96 0>;
>  			clocks = <&gcc GCC_BLSP1_AHB_CLK>,
> -				<&gcc GCC_BLSP1_QUP2_I2C_APPS_CLK>;
> +				 <&gcc GCC_BLSP1_QUP2_I2C_APPS_CLK>;
>  			clock-names = "iface", "core";
>  			pinctrl-names = "default", "sleep";
>  			pinctrl-0 = <&i2c2_default>;
> @@ -472,10 +472,10 @@
>  
>  		blsp_i2c4: i2c@78b8000 {
>  			compatible = "qcom,i2c-qup-v2.2.1";
> -			reg = <0x78b8000 0x1000>;
> +			reg = <0x078b8000 0x600>;
>  			interrupts = <GIC_SPI 98 0>;
>  			clocks = <&gcc GCC_BLSP1_AHB_CLK>,
> -				<&gcc GCC_BLSP1_QUP4_I2C_APPS_CLK>;
> +				 <&gcc GCC_BLSP1_QUP4_I2C_APPS_CLK>;
>  			clock-names = "iface", "core";
>  			pinctrl-names = "default", "sleep";
>  			pinctrl-0 = <&i2c4_default>;
> @@ -487,10 +487,10 @@
>  
>  		blsp_i2c6: i2c@78ba000 {
>  			compatible = "qcom,i2c-qup-v2.2.1";
> -			reg = <0x78ba000 0x1000>;
> +			reg = <0x078ba000 0x600>;
>  			interrupts = <GIC_SPI 100 0>;
>  			clocks = <&gcc GCC_BLSP1_AHB_CLK>,
> -				<&gcc GCC_BLSP1_QUP6_I2C_APPS_CLK>;
> +				 <&gcc GCC_BLSP1_QUP6_I2C_APPS_CLK>;
>  			clock-names = "iface", "core";
>  			pinctrl-names = "default", "sleep";
>  			pinctrl-0 = <&i2c6_default>;
> -- 
> 2.15.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5

[Damien Riegel]

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
---
 arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 72 ++++++++++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/msm8916.dtsi      | 45 +++++++++++++++++++
 2 files changed, 117 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
index c67ad8ed8b60..1cec5b30ed6e 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
@@ -270,6 +270,30 @@
 		};
 	};
 
+	i2c1_default: i2c1_default {
+		pinmux {
+			function = "blsp_i2c1";
+			pins = "gpio2", "gpio3";
+		};
+		pinconf {
+			pins = "gpio2", "gpio3";
+			drive-strength = <16>;
+			bias-disable;
+		};
+	};
+
+	i2c1_sleep: i2c1_sleep {
+		pinmux {
+			function = "gpio";
+			pins = "gpio2", "gpio3";
+		};
+		pinconf {
+			pins = "gpio2", "gpio3";
+			drive-strength = <2>;
+			bias-disable;
+		};
+	};
+
 	i2c2_default: i2c2_default {
 		pinmux {
 			function = "blsp_i2c2";
@@ -294,6 +318,30 @@
 		};
 	};
 
+	i2c3_default: i2c3_default {
+		pinmux {
+			function = "blsp_i2c3";
+			pins = "gpio10", "gpio11";
+		};
+		pinconf {
+			pins = "gpio10", "gpio11";
+			drive-strength = <16>;
+			bias-disable;
+		};
+	};
+
+	i2c3_sleep: i2c3_sleep {
+		pinmux {
+			function = "gpio";
+			pins = "gpio10", "gpio11";
+		};
+		pinconf {
+			pins = "gpio10", "gpio11";
+			drive-strength = <2>;
+			bias-disable;
+		};
+	};
+
 	i2c4_default: i2c4_default {
 		pinmux {
 			function = "blsp_i2c4";
@@ -318,6 +366,30 @@
 		};
 	};
 
+	i2c5_default: i2c5_default {
+		pinmux {
+			function = "blsp_i2c5";
+			pins = "gpio18", "gpio19";
+		};
+		pinconf {
+			pins = "gpio18", "gpio19";
+			drive-strength = <16>;
+			bias-disable;
+		};
+	};
+
+	i2c5_sleep: i2c5_sleep {
+		pinmux {
+			function = "gpio";
+			pins = "gpio18", "gpio19";
+		};
+		pinconf {
+			pins = "gpio18", "gpio19";
+			drive-strength = <2>;
+			bias-disable;
+		};
+	};
+
 	i2c6_default: i2c6_default {
 		pinmux {
 			function = "blsp_i2c6";
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index de25bd6070f5..bdc4cb6f66d4 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -455,6 +455,21 @@
 			status = "disabled";
 		};
 
+		blsp_i2c1: i2c@78b5000 {
+			compatible = "qcom,i2c-qup-v2.2.1";
+			reg = <0x078b5000 0x600>;
+			interrupts = <GIC_SPI 95 0>;
+			clocks = <&gcc GCC_BLSP1_AHB_CLK>,
+				 <&gcc GCC_BLSP1_QUP1_I2C_APPS_CLK>;
+			clock-names = "iface", "core";
+			pinctrl-names = "default", "sleep";
+			pinctrl-0 = <&i2c1_default>;
+			pinctrl-1 = <&i2c1_sleep>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
 		blsp_i2c2: i2c@78b6000 {
 			compatible = "qcom,i2c-qup-v2.2.1";
 			reg = <0x078b6000 0x600>;
@@ -470,6 +485,21 @@
 			status = "disabled";
 		};
 
+		blsp_i2c3: i2c@78b7000 {
+			compatible = "qcom,i2c-qup-v2.2.1";
+			reg = <0x078b7000 0x600>;
+			interrupts = <GIC_SPI 97 0>;
+			clocks = <&gcc GCC_BLSP1_AHB_CLK>,
+				 <&gcc GCC_BLSP1_QUP3_I2C_APPS_CLK>;
+			clock-names = "iface", "core";
+			pinctrl-names = "default", "sleep";
+			pinctrl-0 = <&i2c3_default>;
+			pinctrl-1 = <&i2c3_sleep>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
 		blsp_i2c4: i2c@78b8000 {
 			compatible = "qcom,i2c-qup-v2.2.1";
 			reg = <0x078b8000 0x600>;
@@ -485,6 +515,21 @@
 			status = "disabled";
 		};
 
+		blsp_i2c5: i2c@78b9000 {
+			compatible = "qcom,i2c-qup-v2.2.1";
+			reg = <0x078b9000 0x600>;
+			interrupts = <GIC_SPI 99 0>;
+			clocks = <&gcc GCC_BLSP1_AHB_CLK>,
+				 <&gcc GCC_BLSP1_QUP5_I2C_APPS_CLK>;
+			clock-names = "iface", "core";
+			pinctrl-names = "default", "sleep";
+			pinctrl-0 = <&i2c5_default>;
+			pinctrl-1 = <&i2c5_sleep>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
 		blsp_i2c6: i2c@78ba000 {
 			compatible = "qcom,i2c-qup-v2.2.1";
 			reg = <0x078ba000 0x600>;
-- 
2.15.0

Re: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5

[Bjorn Andersson]

On Wed 01 Nov 10:53 PDT 2017, Damien Riegel wrote:

I think it's better to use the word "nodes" (add nodes...)

> Signed-off-by: Damien Riegel <damien.riegel-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 72 ++++++++++++++++++++++++++++++
>  arch/arm64/boot/dts/qcom/msm8916.dtsi      | 45 +++++++++++++++++++
>  2 files changed, 117 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
> index c67ad8ed8b60..1cec5b30ed6e 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
> @@ -270,6 +270,30 @@
>  		};
>  	};
>  
> +	i2c1_default: i2c1_default {
> +		pinmux {
> +			function = "blsp_i2c1";
> +			pins = "gpio2", "gpio3";
> +		};
> +		pinconf {
> +			pins = "gpio2", "gpio3";
> +			drive-strength = <16>;
> +			bias-disable;
> +		};

pinconf is typically board specific, so please leave these out from the
base dtsi.

> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index de25bd6070f5..bdc4cb6f66d4 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -455,6 +455,21 @@
>  			status = "disabled";
>  		};
>  
> +		blsp_i2c1: i2c@78b5000 {
> +			compatible = "qcom,i2c-qup-v2.2.1";
> +			reg = <0x078b5000 0x600>;

Size is 0x500.

> +			interrupts = <GIC_SPI 95 0>;
> +			clocks = <&gcc GCC_BLSP1_AHB_CLK>,
> +				 <&gcc GCC_BLSP1_QUP1_I2C_APPS_CLK>;
> +			clock-names = "iface", "core";
> +			pinctrl-names = "default", "sleep";
> +			pinctrl-0 = <&i2c1_default>;
> +			pinctrl-1 = <&i2c1_sleep>;

Please omit the pinctrl-* properties from the base dtsi (when it's not
hard coded things for the platform).

> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +		};
> +
>  		blsp_i2c2: i2c@78b6000 {
>  			compatible = "qcom,i2c-qup-v2.2.1";
>  			reg = <0x078b6000 0x600>;

Otherwise this looks good!

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5

[Damien Riegel]

Hi Bjorn,


On Thu, Nov 09, 2017 at 09:00:16AM -0800, Bjorn Andersson wrote:
> On Wed 01 Nov 10:53 PDT 2017, Damien Riegel wrote:
> 
> I think it's better to use the word "nodes" (add nodes...)

Will reword that.

> 
> > Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> > ---
> >  arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 72 ++++++++++++++++++++++++++++++
> >  arch/arm64/boot/dts/qcom/msm8916.dtsi      | 45 +++++++++++++++++++
> >  2 files changed, 117 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
> > index c67ad8ed8b60..1cec5b30ed6e 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
> > @@ -270,6 +270,30 @@
> >  		};
> >  	};
> >  
> > +	i2c1_default: i2c1_default {
> > +		pinmux {
> > +			function = "blsp_i2c1";
> > +			pins = "gpio2", "gpio3";
> > +		};
> > +		pinconf {
> > +			pins = "gpio2", "gpio3";
> > +			drive-strength = <16>;
> > +			bias-disable;
> > +		};
> 
> pinconf is typically board specific, so please leave these out from the
> base dtsi.

I don't mind dropping that, but pinconf for i2c{2,4,6} is already in
msm8916-pins.dtsi, so we should either drop them all, or add pinconf for
i2c{1,3,5} for consistency. And if I read the pinctrl driver correctly,
I2C cannot be routed to other pins, the only properties that users may
want to modify are drive-strength and bias. Let me know which option you
prefer.


> > diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> > index de25bd6070f5..bdc4cb6f66d4 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> > @@ -455,6 +455,21 @@
> >  			status = "disabled";
> >  		};
> >  
> > +		blsp_i2c1: i2c@78b5000 {
> > +			compatible = "qcom,i2c-qup-v2.2.1";
> > +			reg = <0x078b5000 0x600>;
> 
> Size is 0x500.

Will update that (and will also do that for patch 3)

> > +			interrupts = <GIC_SPI 95 0>;
> > +			clocks = <&gcc GCC_BLSP1_AHB_CLK>,
> > +				 <&gcc GCC_BLSP1_QUP1_I2C_APPS_CLK>;
> > +			clock-names = "iface", "core";
> > +			pinctrl-names = "default", "sleep";
> > +			pinctrl-0 = <&i2c1_default>;
> > +			pinctrl-1 = <&i2c1_sleep>;
> 
> Please omit the pinctrl-* properties from the base dtsi (when it's not
> hard coded things for the platform).
> 
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +			status = "disabled";
> > +		};
> > +
> >  		blsp_i2c2: i2c@78b6000 {
> >  			compatible = "qcom,i2c-qup-v2.2.1";
> >  			reg = <0x078b6000 0x600>;
> 
> Otherwise this looks good!

Thank you for the review,
-- 
Damien

Re: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5

[Bjorn Andersson]

On Thu 09 Nov 09:14 PST 2017, Damien Riegel wrote:

> Hi Bjorn,
> 
> 
> On Thu, Nov 09, 2017 at 09:00:16AM -0800, Bjorn Andersson wrote:
> > On Wed 01 Nov 10:53 PDT 2017, Damien Riegel wrote:
> > 
> > I think it's better to use the word "nodes" (add nodes...)
> 
> Will reword that.
> 
> > 
> > > Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> > > ---
> > >  arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 72 ++++++++++++++++++++++++++++++
> > >  arch/arm64/boot/dts/qcom/msm8916.dtsi      | 45 +++++++++++++++++++
> > >  2 files changed, 117 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
> > > index c67ad8ed8b60..1cec5b30ed6e 100644
> > > --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
> > > @@ -270,6 +270,30 @@
> > >  		};
> > >  	};
> > >  
> > > +	i2c1_default: i2c1_default {
> > > +		pinmux {
> > > +			function = "blsp_i2c1";
> > > +			pins = "gpio2", "gpio3";
> > > +		};
> > > +		pinconf {
> > > +			pins = "gpio2", "gpio3";
> > > +			drive-strength = <16>;
> > > +			bias-disable;
> > > +		};
> > 
> > pinconf is typically board specific, so please leave these out from the
> > base dtsi.
> 
> I don't mind dropping that, but pinconf for i2c{2,4,6} is already in
> msm8916-pins.dtsi, so we should either drop them all, or add pinconf for
> i2c{1,3,5} for consistency.

You're right, this needs to be consistent. So I would like us to update
the existing nodes,

> And if I read the pinctrl driver correctly,
> I2C cannot be routed to other pins, the only properties that users may
> want to modify are drive-strength and bias. Let me know which option you
> prefer.
> 

I discussed the matter with my colleagues and we concluded that we want
to keep the muxing in the platform.dtsi and move the specification of
the electrical properties (pinconf) to the board.dts(i).

We did discuss having some "default values" in the platform file, but
pushing these down to the board file gives an indication to others that
these values are board specific.


PS. If you're willing to provide a patch for the existing nodes I would
be happy to review this as well!

Regards,
Bjorn

Re: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5

[Damien Riegel]

On Fri, Nov 10, 2017 at 11:56:12PM -0800, Bjorn Andersson wrote:
> On Thu 09 Nov 09:14 PST 2017, Damien Riegel wrote:
> 
> > Hi Bjorn,
> > 
> > 
> > On Thu, Nov 09, 2017 at 09:00:16AM -0800, Bjorn Andersson wrote:
> > > On Wed 01 Nov 10:53 PDT 2017, Damien Riegel wrote:
> > > 
> > > I think it's better to use the word "nodes" (add nodes...)
> > 
> > Will reword that.
> > 
> > > 
> > > > Signed-off-by: Damien Riegel <damien.riegel-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org>
> > > > ---
> > > >  arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 72 ++++++++++++++++++++++++++++++
> > > >  arch/arm64/boot/dts/qcom/msm8916.dtsi      | 45 +++++++++++++++++++
> > > >  2 files changed, 117 insertions(+)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
> > > > index c67ad8ed8b60..1cec5b30ed6e 100644
> > > > --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
> > > > @@ -270,6 +270,30 @@
> > > >  		};
> > > >  	};
> > > >  
> > > > +	i2c1_default: i2c1_default {
> > > > +		pinmux {
> > > > +			function = "blsp_i2c1";
> > > > +			pins = "gpio2", "gpio3";
> > > > +		};
> > > > +		pinconf {
> > > > +			pins = "gpio2", "gpio3";
> > > > +			drive-strength = <16>;
> > > > +			bias-disable;
> > > > +		};
> > > 
> > > pinconf is typically board specific, so please leave these out from the
> > > base dtsi.
> > 
> > I don't mind dropping that, but pinconf for i2c{2,4,6} is already in
> > msm8916-pins.dtsi, so we should either drop them all, or add pinconf for
> > i2c{1,3,5} for consistency.
> 
> You're right, this needs to be consistent. So I would like us to update
> the existing nodes,
> 
> > And if I read the pinctrl driver correctly,
> > I2C cannot be routed to other pins, the only properties that users may
> > want to modify are drive-strength and bias. Let me know which option you
> > prefer.
> > 
> 
> I discussed the matter with my colleagues and we concluded that we want
> to keep the muxing in the platform.dtsi and move the specification of
> the electrical properties (pinconf) to the board.dts(i).
> 
> We did discuss having some "default values" in the platform file, but
> pushing these down to the board file gives an indication to others that
> these values are board specific.

Okay that makes sense. I'll work on a patchset to do that.

Regards,
-- 
Damien
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html