All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] arm64: dts: qcom: dts improvements
@ 2017-11-01 17:53 ` Damien Riegel
  0 siblings, 0 replies; 28+ messages in thread
From: Damien Riegel @ 2017-11-01 17:53 UTC (permalink / raw)
  To: linux-arm-msm, linux-soc, devicetree, linux-arm-kernel, linux-kernel
  Cc: Andy Gross, David Brown, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, kernel, 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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 0/4] arm64: dts: qcom: dts improvements
@ 2017-11-01 17:53 ` Damien Riegel
  0 siblings, 0 replies; 28+ messages in thread
From: Damien Riegel @ 2017-11-01 17:53 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/4] arm64: dts: qcom: pm8916: fix wcd_codec indentation
  2017-11-01 17:53 ` Damien Riegel
@ 2017-11-01 17:53   ` Damien Riegel
  -1 siblings, 0 replies; 28+ messages in thread
From: Damien Riegel @ 2017-11-01 17:53 UTC (permalink / raw)
  To: linux-arm-msm, linux-soc, devicetree, linux-arm-kernel, linux-kernel
  Cc: Andy Gross, David Brown, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, kernel, 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 1/4] arm64: dts: qcom: pm8916: fix wcd_codec indentation
@ 2017-11-01 17:53   ` Damien Riegel
  0 siblings, 0 replies; 28+ messages in thread
From: Damien Riegel @ 2017-11-01 17:53 UTC (permalink / raw)
  To: linux-arm-kernel

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 at 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 at 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 2/4] arm64: dts: qcom: msm8916-pins: remove assignments to bias-disable
  2017-11-01 17:53 ` Damien Riegel
@ 2017-11-01 17:53   ` Damien Riegel
  -1 siblings, 0 replies; 28+ messages in thread
From: Damien Riegel @ 2017-11-01 17:53 UTC (permalink / raw)
  To: linux-arm-msm, linux-soc, devicetree, linux-arm-kernel, linux-kernel
  Cc: Andy Gross, David Brown, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, kernel, 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 2/4] arm64: dts: qcom: msm8916-pins: remove assignments to bias-disable
@ 2017-11-01 17:53   ` Damien Riegel
  0 siblings, 0 replies; 28+ messages in thread
From: Damien Riegel @ 2017-11-01 17:53 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 3/4] arm64: dts: qcom: msm8916: normalize I2C bindings
  2017-11-01 17:53 ` Damien Riegel
@ 2017-11-01 17:53   ` Damien Riegel
  -1 siblings, 0 replies; 28+ messages in thread
From: Damien Riegel @ 2017-11-01 17:53 UTC (permalink / raw)
  To: linux-arm-msm, linux-soc, devicetree, linux-arm-kernel, linux-kernel
  Cc: Andy Gross, David Brown, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, kernel, 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 3/4] arm64: dts: qcom: msm8916: normalize I2C bindings
@ 2017-11-01 17:53   ` Damien Riegel
  0 siblings, 0 replies; 28+ messages in thread
From: Damien Riegel @ 2017-11-01 17:53 UTC (permalink / raw)
  To: linux-arm-kernel

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 at 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 at 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 at 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5
  2017-11-01 17:53 ` Damien Riegel
@ 2017-11-01 17:53   ` Damien Riegel
  -1 siblings, 0 replies; 28+ messages in thread
From: Damien Riegel @ 2017-11-01 17:53 UTC (permalink / raw)
  To: linux-arm-msm, linux-soc, devicetree, linux-arm-kernel, linux-kernel
  Cc: Andy Gross, David Brown, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, kernel, 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5
@ 2017-11-01 17:53   ` Damien Riegel
  0 siblings, 0 replies; 28+ messages in thread
From: Damien Riegel @ 2017-11-01 17:53 UTC (permalink / raw)
  To: linux-arm-kernel

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 at 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 at 78b6000 {
 			compatible = "qcom,i2c-qup-v2.2.1";
 			reg = <0x078b6000 0x600>;
@@ -470,6 +485,21 @@
 			status = "disabled";
 		};
 
+		blsp_i2c3: i2c at 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 at 78b8000 {
 			compatible = "qcom,i2c-qup-v2.2.1";
 			reg = <0x078b8000 0x600>;
@@ -485,6 +515,21 @@
 			status = "disabled";
 		};
 
+		blsp_i2c5: i2c at 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 at 78ba000 {
 			compatible = "qcom,i2c-qup-v2.2.1";
 			reg = <0x078ba000 0x600>;
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/4] arm64: dts: qcom: pm8916: fix wcd_codec indentation
  2017-11-01 17:53   ` Damien Riegel
  (?)
@ 2017-11-09 16:49       ` Bjorn Andersson
  -1 siblings, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2017-11-09 16:49 UTC (permalink / raw)
  To: Damien Riegel
  Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andy Gross, David Brown,
	Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	kernel-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/4] arm64: dts: qcom: pm8916: fix wcd_codec indentation
@ 2017-11-09 16:49       ` Bjorn Andersson
  0 siblings, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2017-11-09 16:49 UTC (permalink / raw)
  To: Damien Riegel
  Cc: linux-arm-msm, linux-soc, devicetree, linux-arm-kernel,
	linux-kernel, Andy Gross, David Brown, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, kernel

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@savoirfairelinux.com>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.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
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/4] arm64: dts: qcom: pm8916: fix wcd_codec indentation
@ 2017-11-09 16:49       ` Bjorn Andersson
  0 siblings, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2017-11-09 16:49 UTC (permalink / raw)
  To: linux-arm-kernel

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@savoirfairelinux.com>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.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 at 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 at 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
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/4] arm64: dts: qcom: msm8916-pins: remove assignments to bias-disable
  2017-11-01 17:53   ` Damien Riegel
@ 2017-11-09 16:50     ` Bjorn Andersson
  -1 siblings, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2017-11-09 16:50 UTC (permalink / raw)
  To: Damien Riegel
  Cc: linux-arm-msm, linux-soc, devicetree, linux-arm-kernel,
	linux-kernel, Andy Gross, David Brown, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, kernel

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
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 2/4] arm64: dts: qcom: msm8916-pins: remove assignments to bias-disable
@ 2017-11-09 16:50     ` Bjorn Andersson
  0 siblings, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2017-11-09 16:50 UTC (permalink / raw)
  To: linux-arm-kernel

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
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/4] arm64: dts: qcom: msm8916: normalize I2C bindings
  2017-11-01 17:53   ` Damien Riegel
  (?)
@ 2017-11-09 16:56       ` Bjorn Andersson
  -1 siblings, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2017-11-09 16:56 UTC (permalink / raw)
  To: Damien Riegel
  Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andy Gross, David Brown,
	Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	kernel-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/4] arm64: dts: qcom: msm8916: normalize I2C bindings
@ 2017-11-09 16:56       ` Bjorn Andersson
  0 siblings, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2017-11-09 16:56 UTC (permalink / raw)
  To: Damien Riegel
  Cc: linux-arm-msm, linux-soc, devicetree, linux-arm-kernel,
	linux-kernel, Andy Gross, David Brown, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, kernel

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@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
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 3/4] arm64: dts: qcom: msm8916: normalize I2C bindings
@ 2017-11-09 16:56       ` Bjorn Andersson
  0 siblings, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2017-11-09 16:56 UTC (permalink / raw)
  To: linux-arm-kernel

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@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 at 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 at 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 at 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
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5
  2017-11-01 17:53   ` Damien Riegel
  (?)
@ 2017-11-09 17:00       ` Bjorn Andersson
  -1 siblings, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2017-11-09 17:00 UTC (permalink / raw)
  To: Damien Riegel
  Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andy Gross, David Brown,
	Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	kernel-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5
@ 2017-11-09 17:00       ` Bjorn Andersson
  0 siblings, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2017-11-09 17:00 UTC (permalink / raw)
  To: Damien Riegel
  Cc: linux-arm-msm, linux-soc, devicetree, linux-arm-kernel,
	linux-kernel, Andy Gross, David Brown, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, kernel

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@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.

> 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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5
@ 2017-11-09 17:00       ` Bjorn Andersson
  0 siblings, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2017-11-09 17:00 UTC (permalink / raw)
  To: linux-arm-kernel

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@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.

> 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 at 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 at 78b6000 {
>  			compatible = "qcom,i2c-qup-v2.2.1";
>  			reg = <0x078b6000 0x600>;

Otherwise this looks good!

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5
  2017-11-09 17:00       ` Bjorn Andersson
@ 2017-11-09 17:14         ` Damien Riegel
  -1 siblings, 0 replies; 28+ messages in thread
From: Damien Riegel @ 2017-11-09 17:14 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-arm-msm, linux-soc, devicetree, linux-arm-kernel,
	linux-kernel, Andy Gross, David Brown, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, kernel

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5
@ 2017-11-09 17:14         ` Damien Riegel
  0 siblings, 0 replies; 28+ messages in thread
From: Damien Riegel @ 2017-11-09 17:14 UTC (permalink / raw)
  To: linux-arm-kernel

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 at 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 at 78b6000 {
> >  			compatible = "qcom,i2c-qup-v2.2.1";
> >  			reg = <0x078b6000 0x600>;
> 
> Otherwise this looks good!

Thank you for the review,
-- 
Damien

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5
  2017-11-09 17:14         ` Damien Riegel
@ 2017-11-11  7:56           ` Bjorn Andersson
  -1 siblings, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2017-11-11  7:56 UTC (permalink / raw)
  To: Damien Riegel, linux-arm-msm, linux-soc, devicetree,
	linux-arm-kernel, linux-kernel, Andy Gross, David Brown,
	Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, kernel

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5
@ 2017-11-11  7:56           ` Bjorn Andersson
  0 siblings, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2017-11-11  7:56 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5
  2017-11-11  7:56           ` Bjorn Andersson
  (?)
@ 2017-11-14 23:13             ` Damien Riegel
  -1 siblings, 0 replies; 28+ messages in thread
From: Damien Riegel @ 2017-11-14 23:13 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andy Gross, David Brown,
	Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	kernel-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5
@ 2017-11-14 23:13             ` Damien Riegel
  0 siblings, 0 replies; 28+ messages in thread
From: Damien Riegel @ 2017-11-14 23:13 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-arm-msm, linux-soc, devicetree, linux-arm-kernel,
	linux-kernel, Andy Gross, David Brown, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, kernel

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@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.

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

Regards,
-- 
Damien

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5
@ 2017-11-14 23:13             ` Damien Riegel
  0 siblings, 0 replies; 28+ messages in thread
From: Damien Riegel @ 2017-11-14 23:13 UTC (permalink / raw)
  To: linux-arm-kernel

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@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.

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

Regards,
-- 
Damien

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2017-11-14 23:13 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-01 17:53 [PATCH 0/4] arm64: dts: qcom: dts improvements Damien Riegel
2017-11-01 17:53 ` Damien Riegel
2017-11-01 17:53 ` [PATCH 1/4] arm64: dts: qcom: pm8916: fix wcd_codec indentation Damien Riegel
2017-11-01 17:53   ` Damien Riegel
     [not found]   ` <20171101175335.22123-2-damien.riegel-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org>
2017-11-09 16:49     ` Bjorn Andersson
2017-11-09 16:49       ` Bjorn Andersson
2017-11-09 16:49       ` Bjorn Andersson
2017-11-01 17:53 ` [PATCH 2/4] arm64: dts: qcom: msm8916-pins: remove assignments to bias-disable Damien Riegel
2017-11-01 17:53   ` Damien Riegel
2017-11-09 16:50   ` Bjorn Andersson
2017-11-09 16:50     ` Bjorn Andersson
2017-11-01 17:53 ` [PATCH 3/4] arm64: dts: qcom: msm8916: normalize I2C bindings Damien Riegel
2017-11-01 17:53   ` Damien Riegel
     [not found]   ` <20171101175335.22123-4-damien.riegel-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org>
2017-11-09 16:56     ` Bjorn Andersson
2017-11-09 16:56       ` Bjorn Andersson
2017-11-09 16:56       ` Bjorn Andersson
2017-11-01 17:53 ` [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5 Damien Riegel
2017-11-01 17:53   ` Damien Riegel
     [not found]   ` <20171101175335.22123-5-damien.riegel-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org>
2017-11-09 17:00     ` Bjorn Andersson
2017-11-09 17:00       ` Bjorn Andersson
2017-11-09 17:00       ` Bjorn Andersson
2017-11-09 17:14       ` Damien Riegel
2017-11-09 17:14         ` Damien Riegel
2017-11-11  7:56         ` Bjorn Andersson
2017-11-11  7:56           ` Bjorn Andersson
2017-11-14 23:13           ` Damien Riegel
2017-11-14 23:13             ` Damien Riegel
2017-11-14 23:13             ` Damien Riegel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.