This series is "v2" of my "smattering of misc dts cleanups" series
plus v3 of the tail end of the series adding herobrine-rev1. I've set
the version number to the larger of the two to (I hope) help
allevitate confusion.
For the cleanups, there's not a lot holding this series together
except that it fixes a smattering of random dts stuff that I noticed
recently. There are not a lot of dependencies and some of the patches
could be reordered if desired.
Hopefully these look OK and can be applied quickly to avoid conflicts
with other work going on.
For herobrine-rev1, it can be noted that it's likely
that with the introduction of -rev1 we can drop -rev0 support, but
we'll keep it for now (though we won't try to "fit it in" and share
code with it). This series is confirmed to boot atop the top of
the linux qualcomm tree, commit a5ee6b7720cb ("Merge branches
'arm64-defconfig-for-5.18', 'arm64-for-5.18', 'dts-for-5.18',
'arm64-fixes-for-5.17' and 'dts-fixes-for-5.17' into for-next")
Changes in v3:
- Removed extra blank lines
- ("Fix sort order of dp_hot_plug_det") new for v3.
- ("Add edp_out port and HPD lines") new for v3.
- ("Move pcie1_clkreq pull / drive str to boards") new for v3.
- ("sc7280-idp: Disable pull from pcie1_clkreq") new for v3.
- ("Remove dp_hot_plug_det pull from SoC dtsi file") new for v3.
- ("Add a blank line in the dp node") new for v3.
- Rebased atop dts cleanup patches.
- Add regulator suffix as per dts cleanup patches.
- Set PCIe bias / pull as per dts cleanup patches.
- Add dp_hot_plug_det pull as per dts cleanup patches.
- Setup SD card same as dts cleanup patches.
- ("sc7280: Add the CPU compatible to the soc@0 node") new for v3.
- ("Remove "qcom,sc7280" from top-level") patch new for v3.
Changes in v2:
- Herobrine compatible on one line, not two
- Wording change in comments for components enabled per-board
- Always sort "bias" above "drive-strength" in pinctrl.
- Properly sort "hub_en" pinctrl.
- Two comments moved from multiline to single line.
- Space after "/delete-property/"
Douglas Anderson (14):
arm64: dts: qcom: sc7180-trogdor: Add "-regulator" suffix to
pp3300_hub
arm64: dts: qcom: sc7280-herobrine: Consistently add "-regulator"
suffix
arm64: dts: qcom: sc7280: Properly sort sdc pinctrl lines
arm64: dts: qcom: sc7280: Clean up sdc1 / sdc2 pinctrl
arm64: dts: qcom: sc7280-idp: No need for "input-enable" on sw_ctrl
arm64: dts: qcom: sc7280: Fix sort order of dp_hot_plug_det /
pcie1_clkreq_n
arm64: dts: qcom: sc7280: Add edp_out port and HPD lines
arm64: dts: qcom: sc7280: Move pcie1_clkreq pull / drive str to boards
arm64: dts: qcom: sc7280: Disable pull from pcie1_clkreq
arm64: dts: qcom: sc7280: Move dp_hot_plug_det pull from SoC dtsi file
arm64: dts: qcom: sc7280: Add a blank line in the dp node
arm64: dts: qcom: sc7280: Add herobrine-r1
arm64: dts: qcom: sc7280: Add the CPU compatible to the soc@0 node
arm64: dts: qcom: sc7280: Remove "qcom,sc7280" from top-level of
boards
arch/arm64/boot/dts/qcom/Makefile | 1 +
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 2 +-
arch/arm64/boot/dts/qcom/sc7280-crd.dts | 2 +-
.../qcom/sc7280-herobrine-herobrine-r0.dts | 97 +--
.../qcom/sc7280-herobrine-herobrine-r1.dts | 313 +++++++
.../arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 785 ++++++++++++++++++
arch/arm64/boot/dts/qcom/sc7280-idp.dts | 2 +-
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 99 +--
arch/arm64/boot/dts/qcom/sc7280-idp2.dts | 2 +-
arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi | 547 ++++++++++++
arch/arm64/boot/dts/qcom/sc7280.dtsi | 182 ++--
11 files changed, 1845 insertions(+), 187 deletions(-)
create mode 100644 arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
create mode 100644 arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
create mode 100644 arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
--
2.35.0.rc2.247.g8bbb082509-goog
Some of the fixed regulators were missing the "-regulator" suffix. Add
it to be consistent within the file and consistent with the fixed
regulators in sc7180-trogdor.
Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
(no changes since v1)
.../boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
index ad4fe288b53c..f159b5a6d7ef 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
@@ -177,7 +177,7 @@ pp3300_tp: pp3300-tp-regulator {
vin-supply = <&pp3300_z1>;
};
- pp2850_uf_cam: pp2850-uf-cam {
+ pp2850_uf_cam: pp2850-uf-cam-regulator {
compatible = "regulator-fixed";
regulator-name = "pp2850_uf_cam";
@@ -192,7 +192,7 @@ pp2850_uf_cam: pp2850-uf-cam {
vin-supply = <&pp3300_cam>;
};
- pp2850_vcm_wf_cam: pp2850-vcm-wf-cam {
+ pp2850_vcm_wf_cam: pp2850-vcm-wf-cam-regulator {
compatible = "regulator-fixed";
regulator-name = "pp2850_vcm_wf_cam";
@@ -207,7 +207,7 @@ pp2850_vcm_wf_cam: pp2850-vcm-wf-cam {
vin-supply = <&pp3300_cam>;
};
- pp2850_wf_cam: pp2850-wf-cam {
+ pp2850_wf_cam: pp2850-wf-cam-regulator {
compatible = "regulator-fixed";
regulator-name = "pp2850_wf_cam";
@@ -251,7 +251,7 @@ pp1800_fp: pp1800-fp-regulator {
status = "disabled";
};
- pp1800_uf_cam: pp1800-uf-cam {
+ pp1800_uf_cam: pp1800-uf-cam-regulator {
compatible = "regulator-fixed";
regulator-name = "pp1800_uf_cam";
@@ -271,7 +271,7 @@ pp1800_uf_cam: pp1800-uf-cam {
vin-supply = <&pp1800_l19b>;
};
- pp1800_wf_cam: pp1800-wf-cam {
+ pp1800_wf_cam: pp1800-wf-cam-regulator {
compatible = "regulator-fixed";
regulator-name = "pp1800_wf_cam";
@@ -291,7 +291,7 @@ pp1800_wf_cam: pp1800-wf-cam {
vin-supply = <&pp1800_l19b>;
};
- pp1200_wf_cam: pp1200-wf-cam {
+ pp1200_wf_cam: pp1200-wf-cam-regulator {
compatible = "regulator-fixed";
regulator-name = "pp1200_wf_cam";
--
2.35.0.rc2.247.g8bbb082509-goog
I believe that the PCIe clkreq pin is an output. That means we
shouldn't have a pull enabled for it. Turn it off.
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v3:
- ("sc7280-idp: Disable pull from pcie1_clkreq") new for v3.
arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts | 2 +-
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
index 82c3c8f0342b..3c5aab225748 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
@@ -827,7 +827,7 @@ &usb_2_hsphy {
/* PINCTRL - additions to nodes defined in sc7280.dtsi */
&pcie1_clkreq_n {
- bias-pull-up;
+ bias-disable;
drive-strength = <2>;
};
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 6e20e8c07402..9140dca3b72a 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -367,7 +367,7 @@ key_vol_up_default: key-vol-up-default {
};
&pcie1_clkreq_n {
- bias-pull-up;
+ bias-disable;
drive-strength = <2>;
};
--
2.35.0.rc2.247.g8bbb082509-goog
Hi,
On Thu, Feb 3, 2022 at 1:59 PM Stephen Boyd <[email protected]> wrote:
>
> Quoting Doug Anderson (2022-02-03 13:53:09)
> > Hi,
> >
> > On Thu, Feb 3, 2022 at 1:42 PM Stephen Boyd <[email protected]> wrote:
> > >
> > > Quoting Douglas Anderson (2022-02-02 13:23:43)
> > > > I believe that the PCIe clkreq pin is an output. That means we
> > > > shouldn't have a pull enabled for it. Turn it off.
> > >
> > > It sounds like it's a request from the PCI device to the PCI phy that
> > > the clk should be on. I googled pcie clkreq open drain and this pdf[1]
> > > says
> > >
> > > "The CLKREQ# signal is an open drain, active low signal that is driven
> > > low by the PCI Express M.2 add-I Card function to request that the PCI
> > > Express reference clock be available (active clock state) in order to
> > > allow the PCI Express interface to send/receive data"
> > >
> > > so presumably if there isn't an external pull on the signal the open
> > > drain feature will not work and the PCIe device won't be able to drive
> > > it low.
> > >
> > > [1] https://advdownload.advantech.com/productfile/PIS/96FD80-P512-LIS/Product%20-%20Datasheet/96FD80-P512-LIS_datasheet20180110154919.pdf
> >
> > Yeah, I had some trouble figuring this out too, so if someone knows
> > better than me then I'm more than happy to take advice here. I thought
> > I had found something claiming that "clkreq" was an output and on the
> > schematic I have from Qualcomm it shows an arrow going out from the
> > SoC for this signal indicating that it's an output from the SoC. Of
> > course, those arrows are notoriously wrong but at least it's one piece
> > of evidence that someone thought this was an output from the SoC.
> >
> > Hrm, but I just checked the sc7280 "datasheet" which claims that this
> > is an input. Sigh.
> >
> > I guess the options are:
> > * If we're sure this is an input to the SoC then I think we should
> > remove the drive-strength, right?
> > * If we don't know then I guess we can leave both?
>
> I'll wait for qcom folks to confirm. Maybe it's bidirectional because it
> is an open drain signal. I'm showing my cards that I'm no PCIe expert :)
Ah ha! I searched some more and found a Qualcomm PCIe user guide on this!
CLKREQ# signal properties – Bi-directional clock request signals
whether the RC or AP requires control
So it sounds as if leaving it as pull-up and having drive-strength as
2 is right. tl;dr: drop this patch from the series...
> > In any case, for now we can just drop this patch?
> >
>
> Sounds good to me. It needs to be resolved through for herobrine-r1?
Yup. I responded to that patch and for now I'll wait for Bjorn to give
me direction on how to handle it.
-Doug
On Wed, Feb 02, 2022 at 01:23:43PM -0800, Douglas Anderson wrote:
> I believe that the PCIe clkreq pin is an output. That means we
> shouldn't have a pull enabled for it. Turn it off.
>
> Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
On Wed, 2 Feb 2022 13:23:34 -0800, Douglas Anderson wrote:
> This series is "v2" of my "smattering of misc dts cleanups" series
> plus v3 of the tail end of the series adding herobrine-rev1. I've set
> the version number to the larger of the two to (I hope) help
> allevitate confusion.
>
> For the cleanups, there's not a lot holding this series together
> except that it fixes a smattering of random dts stuff that I noticed
> recently. There are not a lot of dependencies and some of the patches
> could be reordered if desired.
>
> [...]
Applied, thanks!
[01/14] arm64: dts: qcom: sc7180-trogdor: Add "-regulator" suffix to pp3300_hub
commit: 171bac46700fcdb2310209dffb382533fe54522a
[02/14] arm64: dts: qcom: sc7280-herobrine: Consistently add "-regulator" suffix
commit: 7a86ac04056569bf5ec663fbb02d79c5e304545a
[03/14] arm64: dts: qcom: sc7280: Properly sort sdc pinctrl lines
commit: b1969bc522187dc6f436301eb71051b24135b607
[04/14] arm64: dts: qcom: sc7280: Clean up sdc1 / sdc2 pinctrl
commit: f9800dde34e678d7ed1de9e95b4b65a257fd0f93
[05/14] arm64: dts: qcom: sc7280-idp: No need for "input-enable" on sw_ctrl
commit: 8fdedd6c64643884dc6bbf6d9a7aabda1713354f
[06/14] arm64: dts: qcom: sc7280: Fix sort order of dp_hot_plug_det / pcie1_clkreq_n
commit: bbef2a9ca08749c89925d2bb49f4ce1c945acc90
[07/14] arm64: dts: qcom: sc7280: Add edp_out port and HPD lines
commit: 118cd3b8ec0db02eb7306c30c1551ef9af885689
[08/14] arm64: dts: qcom: sc7280: Move pcie1_clkreq pull / drive str to boards
commit: 376e9183c1d1dde6972257a823cf484cc5124b7b
[10/14] arm64: dts: qcom: sc7280: Move dp_hot_plug_det pull from SoC dtsi file
commit: ad4152d6e2599c62ef012e528acc5e77ca6765c1
[11/14] arm64: dts: qcom: sc7280: Add a blank line in the dp node
commit: 96b34a6ea7d03876fb9b82ac8db5648a24fc7b2e
Best regards,
--
Bjorn Andersson <[email protected]>
Quoting Douglas Anderson (2022-02-02 13:23:43)
> I believe that the PCIe clkreq pin is an output. That means we
> shouldn't have a pull enabled for it. Turn it off.
It sounds like it's a request from the PCI device to the PCI phy that
the clk should be on. I googled pcie clkreq open drain and this pdf[1]
says
"The CLKREQ# signal is an open drain, active low signal that is driven
low by the PCI Express M.2 add-I Card function to request that the PCI
Express reference clock be available (active clock state) in order to
allow the PCI Express interface to send/receive data"
so presumably if there isn't an external pull on the signal the open
drain feature will not work and the PCIe device won't be able to drive
it low.
[1] https://advdownload.advantech.com/productfile/PIS/96FD80-P512-LIS/Product%20-%20Datasheet/96FD80-P512-LIS_datasheet20180110154919.pdf
Like dp_out, we should have defined edp_out in sc7280.dtsi so we don't
need to do this in the board files.
Like dp_hot_plug_det, we should define edp_hot_plug_det in
sc7280.dtsi.
We should set the default pinctrl for edp_hot_plug_det in
sc7280.dtsi. NOTE: this is _unlike_ the dp_hot_plug_det. It is
reasonable that in some boards the dedicated DP Hot Plug Detect will
not be hooked up in favor of Type C mechanisms. This is unlike eDP
where the Hot Plug Detect line (which functions as "panel ready" in
eDP) is highly likely to be used by boards.
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v3:
- ("Add edp_out port and HPD lines") new for v3.
arch/arm64/boot/dts/qcom/sc7280.dtsi | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 4d5892411a38..3f9837921c17 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3010,6 +3010,8 @@ mdss_dsi_phy: phy@ae94400 {
mdss_edp: edp@aea0000 {
compatible = "qcom,sc7280-edp";
+ pinctrl-names = "default";
+ pinctrl-0 = <&edp_hot_plug_det>;
reg = <0 0xaea0000 0 0x200>,
<0 0xaea0200 0 0x200>,
@@ -3052,12 +3054,18 @@ mdss_edp: edp@aea0000 {
ports {
#address-cells = <1>;
#size-cells = <0>;
+
port@0 {
reg = <0>;
edp_in: endpoint {
remote-endpoint = <&dpu_intf5_out>;
};
};
+
+ port@1 {
+ reg = <1>;
+ edp_out: endpoint { };
+ };
};
edp_opp_table: opp-table {
@@ -3277,6 +3285,11 @@ dp_hot_plug_det: dp-hot-plug-det {
bias-disable;
};
+ edp_hot_plug_det: edp-hot-plug-det {
+ pins = "gpio60";
+ function = "edp_hot";
+ };
+
pcie1_clkreq_n: pcie1-clkreq-n {
pins = "gpio79";
function = "pcie1_clkreqn";
--
2.35.0.rc2.247.g8bbb082509-goog
All of the other fixed regulators have the "-regulator" suffix. Add it
to pp3300_hub to match.
Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
(no changes since v1)
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index 7d8bf66e8ffe..78296ed6fd29 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -284,7 +284,7 @@ pp3300_fp_tp: pp3300-fp-tp-regulator {
vin-supply = <&pp3300_a>;
};
- pp3300_hub: pp3300-hub {
+ pp3300_hub: pp3300-hub-regulator {
compatible = "regulator-fixed";
regulator-name = "pp3300_hub";
--
2.35.0.rc2.247.g8bbb082509-goog
The sdc1 / sdc2 pinctrl lines were randomly stuffed in the middle of
the qup pinctrl lines. Sort them properly. This is a no-op
change. Just code movement.
Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
(no changes since v1)
arch/arm64/boot/dts/qcom/sc7280.dtsi | 154 +++++++++++++--------------
1 file changed, 77 insertions(+), 77 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index d4009cc0bb78..40cb414bc377 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3783,83 +3783,6 @@ qup_uart7_rx: qup-uart7-rx {
function = "qup07";
};
- sdc1_on: sdc1-on {
- clk {
- pins = "sdc1_clk";
- };
-
- cmd {
- pins = "sdc1_cmd";
- };
-
- data {
- pins = "sdc1_data";
- };
-
- rclk {
- pins = "sdc1_rclk";
- };
- };
-
- sdc1_off: sdc1-off {
- clk {
- pins = "sdc1_clk";
- drive-strength = <2>;
- bias-bus-hold;
- };
-
- cmd {
- pins = "sdc1_cmd";
- drive-strength = <2>;
- bias-bus-hold;
- };
-
- data {
- pins = "sdc1_data";
- drive-strength = <2>;
- bias-bus-hold;
- };
-
- rclk {
- pins = "sdc1_rclk";
- bias-bus-hold;
- };
- };
-
- sdc2_on: sdc2-on {
- clk {
- pins = "sdc2_clk";
- };
-
- cmd {
- pins = "sdc2_cmd";
- };
-
- data {
- pins = "sdc2_data";
- };
- };
-
- sdc2_off: sdc2-off {
- clk {
- pins = "sdc2_clk";
- drive-strength = <2>;
- bias-bus-hold;
- };
-
- cmd {
- pins ="sdc2_cmd";
- drive-strength = <2>;
- bias-bus-hold;
- };
-
- data {
- pins ="sdc2_data";
- drive-strength = <2>;
- bias-bus-hold;
- };
- };
-
qup_uart8_cts: qup-uart8-cts {
pins = "gpio32";
function = "qup10";
@@ -4019,6 +3942,83 @@ qup_uart15_rx: qup-uart15-rx {
pins = "gpio63";
function = "qup17";
};
+
+ sdc1_on: sdc1-on {
+ clk {
+ pins = "sdc1_clk";
+ };
+
+ cmd {
+ pins = "sdc1_cmd";
+ };
+
+ data {
+ pins = "sdc1_data";
+ };
+
+ rclk {
+ pins = "sdc1_rclk";
+ };
+ };
+
+ sdc1_off: sdc1-off {
+ clk {
+ pins = "sdc1_clk";
+ drive-strength = <2>;
+ bias-bus-hold;
+ };
+
+ cmd {
+ pins = "sdc1_cmd";
+ drive-strength = <2>;
+ bias-bus-hold;
+ };
+
+ data {
+ pins = "sdc1_data";
+ drive-strength = <2>;
+ bias-bus-hold;
+ };
+
+ rclk {
+ pins = "sdc1_rclk";
+ bias-bus-hold;
+ };
+ };
+
+ sdc2_on: sdc2-on {
+ clk {
+ pins = "sdc2_clk";
+ };
+
+ cmd {
+ pins = "sdc2_cmd";
+ };
+
+ data {
+ pins = "sdc2_data";
+ };
+ };
+
+ sdc2_off: sdc2-off {
+ clk {
+ pins = "sdc2_clk";
+ drive-strength = <2>;
+ bias-bus-hold;
+ };
+
+ cmd {
+ pins ="sdc2_cmd";
+ drive-strength = <2>;
+ bias-bus-hold;
+ };
+
+ data {
+ pins ="sdc2_data";
+ drive-strength = <2>;
+ bias-bus-hold;
+ };
+ };
};
imem@146a5000 {
--
2.35.0.rc2.247.g8bbb082509-goog
Hi,
On Thu, Feb 3, 2022 at 1:42 PM Stephen Boyd <[email protected]> wrote:
>
> Quoting Douglas Anderson (2022-02-02 13:23:43)
> > I believe that the PCIe clkreq pin is an output. That means we
> > shouldn't have a pull enabled for it. Turn it off.
>
> It sounds like it's a request from the PCI device to the PCI phy that
> the clk should be on. I googled pcie clkreq open drain and this pdf[1]
> says
>
> "The CLKREQ# signal is an open drain, active low signal that is driven
> low by the PCI Express M.2 add-I Card function to request that the PCI
> Express reference clock be available (active clock state) in order to
> allow the PCI Express interface to send/receive data"
>
> so presumably if there isn't an external pull on the signal the open
> drain feature will not work and the PCIe device won't be able to drive
> it low.
>
> [1] https://advdownload.advantech.com/productfile/PIS/96FD80-P512-LIS/Product%20-%20Datasheet/96FD80-P512-LIS_datasheet20180110154919.pdf
Yeah, I had some trouble figuring this out too, so if someone knows
better than me then I'm more than happy to take advice here. I thought
I had found something claiming that "clkreq" was an output and on the
schematic I have from Qualcomm it shows an arrow going out from the
SoC for this signal indicating that it's an output from the SoC. Of
course, those arrows are notoriously wrong but at least it's one piece
of evidence that someone thought this was an output from the SoC.
Hrm, but I just checked the sc7280 "datasheet" which claims that this
is an input. Sigh.
I guess the options are:
* If we're sure this is an input to the SoC then I think we should
remove the drive-strength, right?
* If we don't know then I guess we can leave both?
In any case, for now we can just drop this patch?
-Doug
Quoting Doug Anderson (2022-02-03 13:53:09)
> Hi,
>
> On Thu, Feb 3, 2022 at 1:42 PM Stephen Boyd <[email protected]> wrote:
> >
> > Quoting Douglas Anderson (2022-02-02 13:23:43)
> > > I believe that the PCIe clkreq pin is an output. That means we
> > > shouldn't have a pull enabled for it. Turn it off.
> >
> > It sounds like it's a request from the PCI device to the PCI phy that
> > the clk should be on. I googled pcie clkreq open drain and this pdf[1]
> > says
> >
> > "The CLKREQ# signal is an open drain, active low signal that is driven
> > low by the PCI Express M.2 add-I Card function to request that the PCI
> > Express reference clock be available (active clock state) in order to
> > allow the PCI Express interface to send/receive data"
> >
> > so presumably if there isn't an external pull on the signal the open
> > drain feature will not work and the PCIe device won't be able to drive
> > it low.
> >
> > [1] https://advdownload.advantech.com/productfile/PIS/96FD80-P512-LIS/Product%20-%20Datasheet/96FD80-P512-LIS_datasheet20180110154919.pdf
>
> Yeah, I had some trouble figuring this out too, so if someone knows
> better than me then I'm more than happy to take advice here. I thought
> I had found something claiming that "clkreq" was an output and on the
> schematic I have from Qualcomm it shows an arrow going out from the
> SoC for this signal indicating that it's an output from the SoC. Of
> course, those arrows are notoriously wrong but at least it's one piece
> of evidence that someone thought this was an output from the SoC.
>
> Hrm, but I just checked the sc7280 "datasheet" which claims that this
> is an input. Sigh.
>
> I guess the options are:
> * If we're sure this is an input to the SoC then I think we should
> remove the drive-strength, right?
> * If we don't know then I guess we can leave both?
I'll wait for qcom folks to confirm. Maybe it's bidirectional because it
is an open drain signal. I'm showing my cards that I'm no PCIe expert :)
>
>
> In any case, for now we can just drop this patch?
>
Sounds good to me. It needs to be resolved through for herobrine-r1?
Quoting Douglas Anderson (2022-02-02 13:23:36)
> Some of the fixed regulators were missing the "-regulator" suffix. Add
> it to be consistent within the file and consistent with the fixed
> regulators in sc7180-trogdor.
>
> Signed-off-by: Douglas Anderson <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
> ---
Reviewed-by: Stephen Boyd <[email protected]>
Quoting Douglas Anderson (2022-02-02 13:23:41)
> Like dp_out, we should have defined edp_out in sc7280.dtsi so we don't
> need to do this in the board files.
>
> Like dp_hot_plug_det, we should define edp_hot_plug_det in
> sc7280.dtsi.
>
> We should set the default pinctrl for edp_hot_plug_det in
> sc7280.dtsi. NOTE: this is _unlike_ the dp_hot_plug_det. It is
> reasonable that in some boards the dedicated DP Hot Plug Detect will
> not be hooked up in favor of Type C mechanisms. This is unlike eDP
> where the Hot Plug Detect line (which functions as "panel ready" in
> eDP) is highly likely to be used by boards.
>
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
Reviewed-by: Stephen Boyd <[email protected]>