On 20/12/2022 22:30, Dmitry Baryshkov wrote:
On Tue, 20 Dec 2022 at 21:33, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote:
On 20.12.2022 19:57, Lux Aliaga wrote:
On 16/12/2022 08:24, Konrad Dybcio wrote:
On 15.12.2022 20:04, Lux Aliaga wrote:
Adds a UFS host controller node and its corresponding PHY to
the sm6125 platform.
Signed-off-by: Lux Aliaga <they@xxxxxxxxx>
---
Please include a changelog, I don't know what you changed and
what you didn't. Also, you sent 4 revisions in one day, not
letting others review it.
arch/arm64/boot/dts/qcom/sm6125.dtsi | 67 ++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi
index 7e25a4f85594..b64c5bc1452f 100644
--- a/arch/arm64/boot/dts/qcom/sm6125.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi
@@ -508,6 +508,73 @@ sdhc_2: mmc@4784000 {
status = "disabled";
};
+ ufs_mem_hc: ufs@4804000 {
+ compatible = "qcom,sm6125-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
+ reg = <0x04804000 0x3000>, <0x04810000 0x8000>;
+ reg-names = "std", "ice";
+ interrupts = <GIC_SPI 356 IRQ_TYPE_LEVEL_HIGH>;
+ phys = <&ufs_mem_phy_lanes>;
+ phy-names = "ufsphy";
+ lanes-per-direction = <1>;
+ #reset-cells = <1>;
+ resets = <&gcc GCC_UFS_PHY_BCR>;
+ reset-names = "rst";
+
+ clock-names = "core_clk",
+ "bus_aggr_clk",
+ "iface_clk",
+ "core_clk_unipro",
+ "ref_clk",
+ "tx_lane0_sync_clk",
+ "rx_lane0_sync_clk",
+ "ice_core_clk";
+ clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
+ <&gcc GCC_SYS_NOC_UFS_PHY_AXI_CLK>,
+ <&gcc GCC_UFS_PHY_AHB_CLK>,
+ <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>,
+ <&rpmcc RPM_SMD_XO_CLK_SRC>,
+ <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
+ <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>,
+ <&gcc GCC_UFS_PHY_ICE_CORE_CLK>;
+ freq-table-hz = <50000000 240000000>,
+ <0 0>,
+ <0 0>,
+ <37500000 150000000>,
+ <0 0>,
+ <0 0>,
+ <0 0>,
+ <75000000 300000000>;
+
+ non-removable;
+ status = "disabled";
+ };
+
+ ufs_mem_phy: phy@4807000 {
+ compatible = "qcom,sm6115-qmp-ufs-phy";
Krzysztof asked you to add a SoC-specific compatible in v1.
I'm working on adding a new compatible for sm6125's UFS PHY. Should I copy sm6115's tables or just reference them in the sm6125's config in drivers/phy/qualcomm/phy-qcom-qmp-ufs.c?
If they're identical, you can just do something like this:
compatible = "qcom,sm6125-qmp-ufs-phy", "qcom,sm6115-qmp-ufs-phy";
Ugh. I'd prefer to see either of the compatible strings here, but not
both of them.
I hit send too quick, so the justification didn't get in.
Currently we list a single compatibility string for all QMP PHYs. Having
just a single exception stands out, so I'd advise against doing that
(despite Konrad's suggestion being technically correct).
And ensure your newly added compatible is documented in bindings.
This way, the driver will fall back to the 6115 compatible that's
defined in .c, but if we ever need to adjust something specific
for 6125, we will just use the define that we added here. That's
important, as we're supposed to stay backwards-compatible with
old device trees.
Also, wrap your emails at around 80 chars or so, some people
are grumpy about that :P
Konrad
--
With best wishes
Dmitry
