| Message ID | 1541197576-19730-3-git-send-email-jsanka@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,1/3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file | expand |
Hi (sending again since I screwed up my previous reply), On Fri, Nov 2, 2018 at 3:26 PM Jeykumar Sankaran <jsanka@codeaurora.org> wrote: > > Add dsi active/suspend pinctrl nodes to sdm845 SoC dts. > > Changes in v4: > - patch introduced in the series > > Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org> > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index 5728b4c..35df5d2 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -822,6 +822,20 @@ > interrupt-controller; > #interrupt-cells = <2>; > > + dpu_dsi_active: dpu-dsi-active { > + pinmux { > + pins = "gpio6", "gpio52"; > + function = "gpio"; > + }; > + }; > + > + dpu_dsi_suspend: dpu-dsi-suspend { > + pinmux { > + pins = "gpio6", "gpio52"; > + function = "gpio"; > + }; > + }; > + Ugh, I should have noticed this in my previous reply. Sorry! ...looking closer I see that these two pins are MTP-specific. They belong fully in the MTP device tree file. Other sdm845 boards won't necessarily have the same functions on the same pins so they don't belong here in the SoC file. Also as a note: once you move them there they should no longer go in the section "PINCTRL - additions to nodes defined in sdm845.dtsi". They should go under the tlmm in the section "PINCTRL - board-specific pinctrl". Please make sure they are alphabetical there. Since these are board specific, node names should be based on the schematic name. ...and in this case since they are two distinct named pins I'd probably have a separate node for each one. So I think you want something like this in the mtp file: (untested) disp_mode_sel_active: disp-mode-sel-active { pinmux { pins = "gpio52"; function = "gpio"; }; pinconf { pins = "gpio52"; drive-strength = <8>; bias-disable; }; disp_mode_sel_sleep: disp-mode-sel-sleep { pinmux { pins = "gpio52"; function = "gpio"; }; pinconf { pins = "gpio52"; drive-strength = <2>; bias-pull-down; }; ...and then one for gpio6: lcd0_reset_n_active: lcd0-reset-n-active { ... }; --- I'm kinda curious if the sleep stuff is all truly necessary though. Specifically I don't _think_ that the pulls are affected by the drive strength. So either we continue driving this pin while we're sleeping in which case the drive strength matters and the pull doesn't. ...or we aren't driving it in sleep and the pull might matter but the drive strength doesn't. I definitely see a lot of this cruft coming from the Qualcomm downstream without anyone who can explain to me that it's useful. It seems like everyone just blindly copies it from someone else. As further evidence that this isn't currently doing anything, as far as I can tell the v10 panel driver for this panel doesn't actually even select the suspend state. Maybe we can just drop the whole "active" vs. "sleep" for now and we can introduce it later when we show that it's useful for something. Then we can confirm if it's the drive strength that's useful or the pull down and we can also confirm that we don't end up going to sleep with the pin being driven in the opposite direction of the pull. Maybe +Bjorn or +Stephen has a different opinion though... -Doug
On Tue 06 Nov 16:31 PST 2018, Doug Anderson wrote: [..] > Maybe we can just drop the whole "active" vs. "sleep" for now and we > can introduce it later when we show that it's useful for something. [..] > Maybe +Bjorn or +Stephen has a different opinion though... > This sounds like a good suggestion, in particular we know that it won't break like SPI did once we select the "sleep" state sometime in the future. Regards, Bjorn
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index 5728b4c..35df5d2 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -822,6 +822,20 @@ interrupt-controller; #interrupt-cells = <2>; + dpu_dsi_active: dpu-dsi-active { + pinmux { + pins = "gpio6", "gpio52"; + function = "gpio"; + }; + }; + + dpu_dsi_suspend: dpu-dsi-suspend { + pinmux { + pins = "gpio6", "gpio52"; + function = "gpio"; + }; + }; + qup_i2c0_default: qup-i2c0-default { pinmux { pins = "gpio0", "gpio1";
Add dsi active/suspend pinctrl nodes to sdm845 SoC dts. Changes in v4: - patch introduced in the series Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org> --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 14 ++++++++++++++ 1 file changed, 14 insertions(+)