Re: [PATCH v7 2/2] arm64: dts: qcom: sc7280: add lpass lpi pin controller node
From: Srinivasa Rao Mandadapu
Date: Tue Apr 12 2022 - 09:21:38 EST
On 4/12/2022 6:18 PM, Srinivasa Rao Mandadapu wrote:
On 4/12/2022 1:02 AM, Matthias Kaehlcke wrote:
Thanks for your time Matthias!!!
On Mon, Apr 11, 2022 at 07:23:04PM +0530, Srinivasa Rao Mandadapu wrote:
Add LPASS LPI pinctrl node required for Audio functionality on sc7280
based platforms.
Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@xxxxxxxxxxx>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@xxxxxxxxxxx>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@xxxxxxxxxxx>
---
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 84
++++++++++++++++++++++++
arch/arm64/boot/dts/qcom/sc7280.dtsi | 107
+++++++++++++++++++++++++++++++
2 files changed, 191 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 4ba2274..ea751dc 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -238,6 +238,90 @@
modem-init;
};
+&dmic01 {
Shouldn't these nodes be in the PINCTRL section at their respective
positions in alphabetical order?
These are not part of tlmm pin control section. These are part of
lpass_tlmm section.
In your previous comment you asked to remove &lpass_tlmm. Hence
brought out.
nit: since you are keeping the groups the group names are a bit
generic IMO.
e.g. it is fairly obvious that 'dmic01_clk' refers to a clock pin,
however
just 'dmic01' is a bit vague. You could consider adding the prefix
'lpass_'
to the group names for more clarity.
as dmic01 has both clk and data section, I don't think keeping clk is
appropriate here.
As these nodes are part of SC7280, i.e. qcom specific chipset, I feel
lpass_ is redundant.
If we add lpass_ to all dmic nodes, some node names are too lengthy.
+ clk {
+ drive-strength = <8>;
+ };
+};
+
+&dmic01_sleep {
+ clk {
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ data {
+ pull-down;
+ };
+};
+
+&dmic23 {
+ clk {
+ drive-strength = <8>;
+ };
+};
+
+&dmic23_sleep {
+ clk {
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ data {
+ pull-down;
+ };
+};
+
+&rx_swr {
+ clk {
+ drive-strength = <2>;
+ slew-rate = <1>;
+ bias-disable;
+ };
+
+ data {
+ drive-strength = <2>;
+ slew-rate = <1>;
+ bias-bus-hold;
+ };
+};
+
+&rx_swr_sleep {
+ clk {
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+
+ data {
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+};
+
+&tx_swr {
+ clk {
+ drive-strength = <2>;
+ slew-rate = <1>;
+ bias-disable;
+ };
+
+ data {
+ slew-rate = <1>;
+ bias-bus-hold;
+ };
+};
+
+&tx_swr_sleep {
+ clk {
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+
+ data {
+ bias-bus-hold;
+ };
+};
+
&pcie1 {
status = "okay";
perst-gpio = <&tlmm 2 GPIO_ACTIVE_LOW>;
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 8099c80..c692420 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -1987,6 +1987,113 @@
qcom,bcm-voters = <&apps_bcm_voter>;
};
+ lpass_tlmm: pinctrl@33c0000 {
+ compatible = "qcom,sc7280-lpass-lpi-pinctrl";
+ reg = <0 0x033c0000 0x0 0x20000>,
+ <0 0x03550000 0x0 0x10000>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&lpass_tlmm 0 0 15>;
+
+ #clock-cells = <1>;
+
+ dmic01: dmic01 {
+ clk {
+ pins = "gpio6";
From the schematics I interpret that the LPASS GPIOs 0-9 are mapped
to the
SC7280 GPIOs 144-153. Is that correct?
Yes. But we refer with GPIOs 0-9 in driver.
+ function = "dmic1_clk";
+ };
+
+ data {
+ pins = "gpio7";
+ function = "dmic1_data";
+ };
+ };
+
+ dmic01_sleep: dmic01-sleep {
+ clk {
+ pins = "gpio6";
+ function = "dmic1_clk";
+ };
+
+ data {
+ pins = "gpio7";
+ function = "dmic1_data";
+ };
+ };
+
+ dmic23: dmic23 {
+ clk {
+ pins = "gpio8";
+ function = "dmic2_clk";
+ };
+
+ data {
+ pins = "gpio9";
+ function = "dmic2_data";
+ };
+ };
+
+ dmic23_sleep: dmic23_sleep {
s/dmic23_sleep/dmic23-sleep/ for the node name.
Okay.
+ clk {
+ pins = "gpio8";
+ function = "dmic2_clk";
+ };
+
+ data {
+ pins = "gpio9";
+ function = "dmic2_data";
+ };
+ };
+
+ rx_swr: rx-swr {
+ clk {
+ pins = "gpio3";
+ function = "swr_rx_clk";
+ };
+
+ data {
+ pins = "gpio4", "gpio5";
+ function = "swr_rx_data";
+ };
+ };
+
+ rx_swr_sleep: rx-swr-sleep {
+ clk {
+ pins = "gpio3";
+ function = "swr_rx_clk";
+ };
+
+ data {
+ pins = "gpio4", "gpio5";
+ function = "swr_rx_data";
+ };
+ };
+
+ tx_swr: tx-swr {
+ clk {
+ pins = "gpio0";
+ function = "swr_tx_clk";
+ };
+
+ data {
+ pins = "gpio1", "gpio2", "gpio14";
+ function = "swr_tx_data";
+ };
+ };
+
+ tx_swr_sleep: tx-swr-sleep {
+ clk {
+ pins = "gpio0";
+ function = "swr_tx_clk";
+ };
+
+ data {
+ pins = "gpio1", "gpio2", "gpio14";
+ function = "swr_tx_data";
+ };
+ };
+ };
+
gpu: gpu@3d00000 {
compatible = "qcom,adreno-635.0", "qcom,adreno";
reg = <0 0x03d00000 0 0x40000>,
--
2.7.4