Messages in this thread | | | Date | Fri, 15 Dec 2023 00:33:56 +0100 | Subject | Re: [PATCH v3 1/2] arm64: dts: qcom: x1e80100: Add more support | From | Konrad Dybcio <> |
| |
On 12/14/23 23:48, Abel Vesa wrote: > Add nodes for the following: > - ADSP and CDSP > - IPCC > - USB SubSys 0, 1 and 2 (along with the PHYs) > - PCIe 4 and 6 and related PHYs > - DISP, GPU, TCSR and CAM clock controllers > - AOSS QMP > - Display controllers (along with the PHYs) > > Co-developed-by: Rajendra Nayak <quic_rjendra@quicinc.com> > Signed-off-by: Rajendra Nayak <quic_rjendra@quicinc.com> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > --- [...]
> + > + interrupts-extended = <&intc GIC_SPI 370 IRQ_TYPE_LEVEL_HIGH>, > + <&pdc 10 IRQ_TYPE_EDGE_RISING>, > + <&pdc 57 IRQ_TYPE_EDGE_RISING>, > + <&pdc 58 IRQ_TYPE_LEVEL_HIGH>; the interrupt types seem wrong (ss vs dp/dm lvl vs edge, looks like a cut-n-paste reordering mistake)
btw the edge-triggered ones should be _BOTH, see Johan's recent treewide cleanups [...]
> + usb_2_dwc3: usb@a200000 { > + compatible = "snps,dwc3"; > + reg = <0 0x0a200000 0 0xcd00>; > + interrupts = <GIC_SPI 241 IRQ_TYPE_LEVEL_HIGH>; > + iommus = <&apps_smmu 0x14e0 0x0>; > + phys = <&usb_2_hsphy>; > + phy-names = "usb2-phy"; maximum-speed = "high-speed"
would be fitting here as well [...]
> + interrupts-extended = <&intc GIC_SPI 371 IRQ_TYPE_LEVEL_HIGH>, > + <&pdc 17 IRQ_TYPE_LEVEL_HIGH>, > + <&pdc 15 IRQ_TYPE_EDGE_RISING>, > + <&pdc 61 IRQ_TYPE_EDGE_RISING>; edges [...]
> + interrupts-extended = <&intc GIC_SPI 372 IRQ_TYPE_LEVEL_HIGH>, > + <&pdc 47 IRQ_TYPE_LEVEL_HIGH>, > + <&pdc 11 IRQ_TYPE_EDGE_RISING>, > + <&pdc 60 IRQ_TYPE_EDGE_RISING>; edges [...]
> + mdss: display-subsystem@ae00000 { > + compatible = "qcom,x1e80100-mdss"; > + reg = <0 0x0ae00000 0 0x1000>; > + reg-names = "mdss"; > + > + interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>; > + > + clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>, > + <&gcc GCC_DISP_AHB_CLK>, > + <&gcc GCC_DISP_HF_AXI_CLK>, > + <&dispcc DISP_CC_MDSS_MDP_CLK>; no -names?
> + > + assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK_SRC>; > + assigned-clock-parents = <&dispcc DISP_CC_PLL0>; raised eyebrow emoji - why would this be necessary?
> + > + resets = <&dispcc DISP_CC_MDSS_CORE_BCR>; > + > + interconnects = <&mmss_noc MASTER_MDP 0 &gem_noc SLAVE_LLCC 0>, QCOM_ICC_TAG_ALWAYS
[...]
> + assigned-clocks = <&dispcc DISP_CC_MDSS_VSYNC_CLK>; > + assigned-clock-rates = <19200000>; I was wondering why we keep copypasting this.. the clk driver only support this single frequency, can it be dropped?
[...]
> + opp-514000000 { > + opp-hz = /bits/ 64 <514000000>; > + required-opps = <&rpmhpd_opp_nom>; > + }; Is there no 575MHz level?
[...]
> + dispcc: clock-controller@af00000 { > + compatible = "qcom,x1e80100-dispcc"; > + reg = <0 0x0af00000 0 0x20000>; > + clocks = <&bi_tcxo_div2>, > + <&bi_tcxo_ao_div2>, > + <&gcc GCC_DISP_AHB_CLK>, > + <&sleep_clk>, > + <0>, /* dsi0 */ > + <0>, > + <0>, /* dsi1 */ > + <0>, > + <&usb_1_ss0_qmpphy QMP_USB43DP_DP_LINK_CLK>, /* dp0 */ > + <&usb_1_ss0_qmpphy QMP_USB43DP_DP_VCO_DIV_CLK>, /* dp0 */ > + <0>, /* dp1 */ > + <0>, > + <0>, /* dp2 */ should these not be filled out as well?
usb1_ss1_qmpphy
and
mdss_dp2_phy
respectively, I think?
> + <0>, > + <&mdss_dp3_phy 0>, /* dp3 */ > + <&mdss_dp3_phy 1>; > + power-domains = <&rpmhpd RPMHPD_MMCX>; > + #clock-cells = <1>; > + #reset-cells = <1>; > + #power-domain-cells = <1>; > + }; [...]
> + > + interrupts-extended = <&pdc 6 IRQ_TYPE_EDGE_RISING>, > + <&smp2p_adsp_in 0 IRQ_TYPE_EDGE_RISING>, > + <&smp2p_adsp_in 1 IRQ_TYPE_EDGE_RISING>, > + <&smp2p_adsp_in 2 IRQ_TYPE_EDGE_RISING>, > + <&smp2p_adsp_in 3 IRQ_TYPE_EDGE_RISING>; > + interrupt-names = "wdog", "fatal", "ready", > + "handover", "stop-ack"; One a line? :(
Konrad
| |