On 31/07/2023 07:29, Krishna Chaitanya Chundru wrote: > > On 7/28/2023 9:27 PM, Krzysztof Kozlowski wrote: >> On 28/07/2023 17:10, Krishna Chaitanya Chundru wrote: >>> On 7/28/2023 5:33 PM, Krzysztof Kozlowski wrote: >>>> On 28/07/2023 12:39, Krishna chaitanya chundru wrote: >>>>> Add PCIe dtsi node for PCIe0 controller on sc7280 platform. >>>>> >>>>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@xxxxxxxxxxx> >>>> Thank you for your patch. There is something to discuss/improve. >>>> >>>> >>>>> + pcie0_phy: phy@1c06000 { >>>>> + compatible = "qcom,sm8250-qmp-gen3x1-pcie-phy"; >>>>> + reg = <0 0x01c06000 0 0x1c0>; >>>>> + #address-cells = <2>; >>>>> + #size-cells = <2>; >>>>> + ranges; >>>>> + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, >>>>> + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, >>>>> + <&gcc GCC_PCIE_CLKREF_EN>, >>>>> + <&gcc GCC_PCIE0_PHY_RCHNG_CLK>; >>>>> + clock-names = "aux", "cfg_ahb", "ref", "refgen"; >>>>> + >>>>> + resets = <&gcc GCC_PCIE_0_PHY_BCR>; >>>>> + reset-names = "phy"; >>>>> + >>>>> + assigned-clocks = <&gcc GCC_PCIE0_PHY_RCHNG_CLK>; >>>>> + assigned-clock-rates = <100000000>; >>>>> + >>>>> + status = "disabled"; >>>>> + >>>>> + pcie0_lane: phy@1c0e6200 { >>>> Isn't this old-style of bindings? Wasn't there a change? On what tree >>>> did you base it? >> The work was here: >> https://lore.kernel.org/all/20230324022514.1800382-5-dmitry.baryshkov@xxxxxxxxxx/ >> >> But I don't remember the status. >> >>> Let me rebase and send it again. >> This anyway looks like wrong compatible. You used sm8250. > > The patch was send on latest linux-next only and the above change is not > merged yet. I don't think we will want old DTS syntax... but this actually depends on the status of Dmitry's patchset. > > We are using the same compatible string as sm8250 because the phy is > same both from hardware and software perspective for sm8250. > > that is why we are using the same compatible string. > > Can you let me know if we want create a separate compatible string for > this even though we are using same phy? https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#42 Best regards, Krzysztof