Message ID | 20210225221310.1939599-1-dianders@chromium.org |
---|---|
Headers | show |
Series | arm64: dts: qcom: Update sc7180-trogdor variants from downstream | expand |
Hi, On Thu, Feb 25, 2021 at 2:55 PM Konrad Dybcio <konrad.dybcio@somainline.org> wrote: > > Hi, > > > > > > +&pri_mi2s_active { > > + pinconf { > > + pins = "gpio53", "gpio54", "gpio55", "gpio56"; > > + drive-strength = <2>; > > + bias-pull-down; > > + }; > > +}; > > + > > You can omit pinconf{}, so the outcome would be: > &pri_mi2s_active { > > pins = ... > > ... > > }; > > > This makes the DTs ever so shorter and is the style that's currently used for new submissions. > > Same goes for the nodes that are being referenced. Yes, I agree. That definitely makes sense going forward, but I think it'll just add to the confusion to switch a dts for a given SoC mid-stride. ...or, if we do switch the style it should be done in a separate (no-op) patch series. This series is already giant enough... -Doug
On Thu, Feb 25, 2021 at 02:12:59PM -0800, Douglas Anderson wrote: > In general pinconf belongs in board files, not SoC files. Move it to > the only current user (trogdor). Also adjust the drive strengths and > pulls. > > Cc: V Sujith Kumar Reddy <vsujithk@codeaurora.org> > Cc: Srinivasa Rao Mandadapu <srivasam@codeaurora.org> > Cc: Tzung-Bi Shih <tzungbi@chromium.org> > Cc: Judy Hsiao <judyhsiao@chromium.org> > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > This should replace the patch ("Asoc: qcom: dts: Change MI2S GPIO > configuration to pulldown") [1]. > > [1] https://lore.kernel.org/r/1605526408-15671-1-git-send-email-srivasam@codeaurora.org > > arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 24 ++++++++++++++++++++ > arch/arm64/boot/dts/qcom/sc7180.dtsi | 18 --------------- > 2 files changed, 24 insertions(+), 18 deletions(-) Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
On Thu, Feb 25, 2021 at 02:13:05PM -0800, Douglas Anderson wrote: > From: Venkata Lakshmi Narayana Gubba <gubbaven@codeaurora.org> > > Removed voting for RPMH_RF_CLK2 which is not required as it is > getting managed by BT SoC through SW_CTRL line. > > Cc: Matthias Kaehlcke <mka@chromium.org> > Signed-off-by: Venkata Lakshmi Narayana Gubba <gubbaven@codeaurora.org> > Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
On Thu, Feb 25, 2021 at 02:13:08PM -0800, Douglas Anderson wrote: > This is a SKU variant of lazor. Add it. This squashes the downstream > patches to support this hardware. > > NOTES: > - The non-touch SKU actually has "innolux,n116bca" but that driver is > still pending in simple-panel. The bindings have been Acked though. > Things work well enough with the "innolux,n116bge" timings for now, > though. > - The wonky special dts just for "-rev4" arguably doesn't need to go > upstream since they weren't widely distributed, but since a few > people have them we might as well. If it ever causes problems we > can delete it. > > Cc: Stephen Boyd <swboyd@chromium.org> > Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
On Thu, Feb 25, 2021 at 02:13:10PM -0800, Douglas Anderson wrote: > This is a trogdor variant. This is mostly a grab from the downstream > tree with notable exceptions: > - I skip -rev0. This was a super early build and there's no advantage > of long term support. > - I remove sound node since sound hasn't landed upstream yet. > > Cc: Gwendal Grignou <gwendal@chromium.org> > Cc: Matthias Kaehlcke <mka@chromium.org> > Cc: Stephen Boyd <swboyd@chromium.org> > Cc: Tzung-Bi Shih <tzungbi@chromium.org> > Cc: Judy Hsiao <judyhsiao@chromium.org> > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- Matches downstream except for the sound node and -rev0, which are mentioned in the commit message. Also looks sane to me otherwise from a high level inspection. Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Quoting Douglas Anderson (2021-02-25 14:13:10) > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi > new file mode 100644 > index 000000000000..5def9953d82b > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi > @@ -0,0 +1,249 @@ [...] > + > +/* > + * There's no SAR sensor, so i2c5 is re-purposed. We leave the > + * proximity@28 node under i2c5 (from trogdor.dtsi) since it's "disabled" > + * and doesn't hurt. > + */ > +i2c_wlc: &i2c5 { > + /* Currently not connected to anything; see b/168652326 */ > +}; Can we remove this? As far as I know this will always be this way and thus doesn't provide anything meaningful to leave this bug comment here that doesn't work for people. > + > +&i2c7 { > + status = "disabled"; > +};
Hi, On Fri, Feb 26, 2021 at 10:45 AM Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Douglas Anderson (2021-02-25 14:13:10) > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi > > new file mode 100644 > > index 000000000000..5def9953d82b > > --- /dev/null > > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi > > @@ -0,0 +1,249 @@ > [...] > > + > > +/* > > + * There's no SAR sensor, so i2c5 is re-purposed. We leave the > > + * proximity@28 node under i2c5 (from trogdor.dtsi) since it's "disabled" > > + * and doesn't hurt. > > + */ > > +i2c_wlc: &i2c5 { > > + /* Currently not connected to anything; see b/168652326 */ > > +}; > > Can we remove this? As far as I know this will always be this way and > thus doesn't provide anything meaningful to leave this bug comment here > that doesn't work for people. Yeah, that sounds good. We just want to delete this whole comment and the node. That seems like enough reason to repost the series. I'll plan to do it early next week. Of course, I wouldn't object to any of these things: * This patch landing and having the node and I'll do a follow-up patch to remove it. * Bjorn removing the comment and node as he applies. -Doug
On 26/02/2021 01:12, Douglas Anderson wrote: > From: Stephen Boyd <swboyd@chromium.org> > > Drop the old node and add the new one in its place. > > Cc: Stephen Boyd <swboyd@chromium.org> > Cc: Jeykumar Sankaran <jsanka@codeaurora.org> > Cc: Chandan Uddaraju <chandanu@codeaurora.org> > Cc: Vara Reddy <varar@codeaurora.org> > Cc: Tanmay Shah <tanmay@codeaurora.org> > Cc: Rob Clark <robdclark@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > [dianders: Adjusted due to DP not itself not in upstream dts yet] > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > > arch/arm64/boot/dts/qcom/sc7180.dtsi | 23 ++++++++++++++++------- > 1 file changed, 16 insertions(+), 7 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi > index 1ea3344ab62c..60248a6757d8 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi > @@ -2770,12 +2770,11 @@ usb_1_hsphy: phy@88e3000 { > }; > > usb_1_qmpphy: phy-wrapper@88e9000 { > - compatible = "qcom,sc7180-qmp-usb3-phy"; > + compatible = "qcom,sc7180-qmp-usb3-dp-phy"; > reg = <0 0x088e9000 0 0x18c>, > - <0 0x088e8000 0 0x38>; > - reg-names = "reg-base", "dp_com"; > + <0 0x088e8000 0 0x38>, Technically this should be 0x3c. Offset 0x38 is USB3_DP_COM_REVISION_ID3 (not used by the current driver though). > + <0 0x088ea000 0 0x40>; I think 0x40 is not enough here. This is a serdes region and qmp_v3_dp_serdes_tbl contains registers 0x148 and 0x154. > status = "disabled"; > - #clock-cells = <1>; > #address-cells = <2>; > #size-cells = <2>; > ranges; > @@ -2790,7 +2789,7 @@ usb_1_qmpphy: phy-wrapper@88e9000 { > <&gcc GCC_USB3_DP_PHY_PRIM_BCR>; > reset-names = "phy", "common"; > > - usb_1_ssphy: phy@88e9200 { > + usb_1_ssphy: usb3-phy@88e9200 { > reg = <0 0x088e9200 0 0x128>, > <0 0x088e9400 0 0x200>, > <0 0x088e9c00 0 0x218>, > @@ -2803,6 +2802,16 @@ usb_1_ssphy: phy@88e9200 { > clock-names = "pipe0"; > clock-output-names = "usb3_phy_pipe_clk_src"; > }; > + > + dp_phy: dp-phy@88ea200 { > + reg = <0 0x088ea200 0 0x200>, > + <0 0x088ea400 0 0x200>, > + <0 0x088eaa00 0 0x200>, > + <0 0x088ea600 0 0x200>, > + <0 0x088ea800 0 0x200>; > + #clock-cells = <1>; > + #phy-cells = <0>; > + }; > }; > > dc_noc: interconnect@9160000 { > @@ -3166,8 +3175,8 @@ dispcc: clock-controller@af00000 { > <&gcc GCC_DISP_GPLL0_CLK_SRC>, > <&dsi_phy 0>, > <&dsi_phy 1>, > - <0>, > - <0>; > + <&dp_phy 0>, > + <&dp_phy 1>; > clock-names = "bi_tcxo", > "gcc_disp_gpll0_clk_src", > "dsi0_phy_pll_out_byteclk", >
Hi, On Sat, Mar 13, 2021 at 4:28 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > > usb_1_qmpphy: phy-wrapper@88e9000 { > > - compatible = "qcom,sc7180-qmp-usb3-phy"; > > + compatible = "qcom,sc7180-qmp-usb3-dp-phy"; > > reg = <0 0x088e9000 0 0x18c>, > > - <0 0x088e8000 0 0x38>; > > - reg-names = "reg-base", "dp_com"; > > + <0 0x088e8000 0 0x38>, > > Technically this should be 0x3c. Offset 0x38 is USB3_DP_COM_REVISION_ID3 > (not used by the current driver though). > > > + <0 0x088ea000 0 0x40>; > > I think 0x40 is not enough here. > This is a serdes region and qmp_v3_dp_serdes_tbl contains registers > 0x148 and 0x154. OK! https://lore.kernel.org/r/20210315103836.1.I9a97120319d43b42353aeac4d348624d60687df7@changeid -Doug