Hi,
This series fixes the clocks supplied to QMP PHY IPs in the Qcom SoCs. All
of the Qcom SoCs except MSM8996 require 3 clocks for QMP UFS:
* ref - 19.2MHz reference clock from RPM/RPMh
* ref_aux - Auxiliary reference clock from GCC
* qref - QREF clock from GCC or TCSR (TCSR since SM8550)
MSM8996 only requires 'ref' and 'qref' clocks.
Hence, this series fixes the binding, DT and GCC driver to reflect the
actual clock topology.
Testing
=======
Tested on Qualcomm RB5 development board based on SM8250 SoC. I don't
expect this series to break other SoCs too.
- Mani
Manivannan Sadhasivam (16):
dt-bindings: phy: qmp-ufs: Fix PHY clocks
phy: qcom-qmp-ufs: Switch to devm_clk_bulk_get_all() API
dt-bindings: clock: qcom: Add missing UFS QREF clocks
clk: qcom: gcc-sc8180x: Add missing UFS QREF clocks
arm64: dts: qcom: msm8996: Fix UFS PHY clocks
arm64: dts: qcom: msm8998: Fix UFS PHY clocks
arm64: dts: qcom: sdm845: Fix UFS PHY clocks
arm64: dts: qcom: sm6115: Fix UFS PHY clocks
arm64: dts: qcom: sm6125: Fix UFS PHY clocks
arm64: dts: qcom: sm6350: Fix UFS PHY clocks
arm64: dts: qcom: sm8150: Fix UFS PHY clocks
arm64: dts: qcom: sm8250: Fix UFS PHY clocks
arm64: dts: qcom: sc8180x: Fix UFS PHY clocks
arm64: dts: qcom: sc8280xp: Fix UFS PHY clocks
arm64: dts: qcom: sm8350: Fix UFS PHY clocks
arm64: dts: qcom: sm8550: Fix UFS PHY clocks
.../phy/qcom,sc8280xp-qmp-ufs-phy.yaml | 47 +++++++-------
arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 +-
arch/arm64/boot/dts/qcom/msm8998.dtsi | 12 ++--
arch/arm64/boot/dts/qcom/sc8180x.dtsi | 6 +-
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 18 ++++--
arch/arm64/boot/dts/qcom/sdm845.dtsi | 8 ++-
arch/arm64/boot/dts/qcom/sm6115.dtsi | 8 ++-
arch/arm64/boot/dts/qcom/sm6125.dtsi | 8 ++-
arch/arm64/boot/dts/qcom/sm6350.dtsi | 8 ++-
arch/arm64/boot/dts/qcom/sm8150.dtsi | 8 ++-
arch/arm64/boot/dts/qcom/sm8250.dtsi | 8 ++-
arch/arm64/boot/dts/qcom/sm8350.dtsi | 8 ++-
arch/arm64/boot/dts/qcom/sm8550.dtsi | 9 ++-
drivers/clk/qcom/gcc-sc8180x.c | 28 +++++++++
drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 61 +++----------------
include/dt-bindings/clock/qcom,gcc-sc8180x.h | 2 +
16 files changed, 124 insertions(+), 119 deletions(-)
--
2.25.1
All QMP UFS PHYs except MSM8996 require 3 clocks:
* ref - 19.2MHz reference clock from RPMh
* ref_aux - Auxiliary reference clock from GCC
* qref - QREF clock from GCC or TCSR (since SM8550)
MSM8996 only requires 'ref' and 'qref' clocks. Hence, fix the binding to
reflect the actual clock topology.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
.../phy/qcom,sc8280xp-qmp-ufs-phy.yaml | 47 +++++++++----------
1 file changed, 21 insertions(+), 26 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
index f3a3296c811c..800f11b29dcd 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
@@ -37,15 +37,12 @@ properties:
maxItems: 1
clocks:
- minItems: 1
+ minItems: 2
maxItems: 3
clock-names:
- minItems: 1
- items:
- - const: ref
- - const: ref_aux
- - const: qref
+ minItems: 2
+ maxItems: 3
power-domains:
maxItems: 1
@@ -85,22 +82,9 @@ allOf:
compatible:
contains:
enum:
+ - qcom,msm8998-qmp-ufs-phy
- qcom,sa8775p-qmp-ufs-phy
- qcom,sc7280-qmp-ufs-phy
- - qcom,sm8450-qmp-ufs-phy
- then:
- properties:
- clocks:
- minItems: 3
- clock-names:
- minItems: 3
-
- - if:
- properties:
- compatible:
- contains:
- enum:
- - qcom,msm8998-qmp-ufs-phy
- qcom,sc8180x-qmp-ufs-phy
- qcom,sc8280xp-qmp-ufs-phy
- qcom,sdm845-qmp-ufs-phy
@@ -111,13 +95,18 @@ allOf:
- qcom,sm8150-qmp-ufs-phy
- qcom,sm8250-qmp-ufs-phy
- qcom,sm8350-qmp-ufs-phy
+ - qcom,sm8450-qmp-ufs-phy
- qcom,sm8550-qmp-ufs-phy
then:
properties:
clocks:
- maxItems: 2
+ minItems: 3
+ maxItems: 3
clock-names:
- maxItems: 2
+ items:
+ - const: ref
+ - const: ref_aux
+ - const: qref
- if:
properties:
@@ -128,22 +117,28 @@ allOf:
then:
properties:
clocks:
- maxItems: 1
+ minItems: 2
+ maxItems: 2
clock-names:
- maxItems: 1
+ items:
+ - const: ref
+ - const: qref
additionalProperties: false
examples:
- |
#include <dt-bindings/clock/qcom,gcc-sc8280xp.h>
+ #include <dt-bindings/clock/qcom,rpmh.h>
ufs_mem_phy: phy@1d87000 {
compatible = "qcom,sc8280xp-qmp-ufs-phy";
reg = <0x01d87000 0x1000>;
- clocks = <&gcc GCC_UFS_REF_CLKREF_CLK>, <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
- clock-names = "ref", "ref_aux";
+ clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
+ <&gcc GCC_UFS_REF_CLKREF_CLK>;
+
+ clock-names = "ref", "ref_aux", "qref";
power-domains = <&gcc UFS_PHY_GDSC>;
--
2.25.1
Device drivers should just rely on the clocks provided by the devicetree
and enable/disable them based on the requirement. There is no need to
validate the clocks provided by devicetree in the driver. That's the job
of DT schema.
So let's switch to devm_clk_bulk_get_all() API that just gets the clocks
provided by devicetree and remove hardcoded clocks info.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 61 +++----------------------
1 file changed, 7 insertions(+), 54 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
index 514fa14df634..174b105fda82 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
@@ -837,9 +837,6 @@ struct qmp_phy_cfg {
/* Additional sequence for HS G4 */
const struct qmp_phy_cfg_tbls tbls_hs_g4;
- /* clock ids to be requested */
- const char * const *clk_list;
- int num_clks;
/* regulators to be requested */
const char * const *vreg_list;
int num_vregs;
@@ -865,6 +862,7 @@ struct qmp_ufs {
void __iomem *rx2;
struct clk_bulk_data *clks;
+ int num_clks;
struct regulator_bulk_data *vregs;
struct reset_control *ufs_reset;
@@ -897,20 +895,6 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
readl(base + offset);
}
-/* list of clocks required by phy */
-static const char * const msm8996_ufs_phy_clk_l[] = {
- "ref",
-};
-
-/* the primary usb3 phy on sm8250 doesn't have a ref clock */
-static const char * const sm8450_ufs_phy_clk_l[] = {
- "qref", "ref", "ref_aux",
-};
-
-static const char * const sdm845_ufs_phy_clk_l[] = {
- "ref", "ref_aux",
-};
-
/* list of regulators */
static const char * const qmp_phy_vreg_l[] = {
"vdda-phy", "vdda-pll",
@@ -948,9 +932,6 @@ static const struct qmp_phy_cfg msm8996_ufsphy_cfg = {
.rx_num = ARRAY_SIZE(msm8996_ufsphy_rx),
},
- .clk_list = msm8996_ufs_phy_clk_l,
- .num_clks = ARRAY_SIZE(msm8996_ufs_phy_clk_l),
-
.vreg_list = qmp_phy_vreg_l,
.num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
@@ -986,8 +967,6 @@ static const struct qmp_phy_cfg sa8775p_ufsphy_cfg = {
.pcs = sm8350_ufsphy_g4_pcs,
.pcs_num = ARRAY_SIZE(sm8350_ufsphy_g4_pcs),
},
- .clk_list = sm8450_ufs_phy_clk_l,
- .num_clks = ARRAY_SIZE(sm8450_ufs_phy_clk_l),
.vreg_list = qmp_phy_vreg_l,
.num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
.regs = ufsphy_v5_regs_layout,
@@ -1020,8 +999,6 @@ static const struct qmp_phy_cfg sc7280_ufsphy_cfg = {
.pcs = sm8150_ufsphy_hs_g4_pcs,
.pcs_num = ARRAY_SIZE(sm8150_ufsphy_hs_g4_pcs),
},
- .clk_list = sm8450_ufs_phy_clk_l,
- .num_clks = ARRAY_SIZE(sm8450_ufs_phy_clk_l),
.vreg_list = qmp_phy_vreg_l,
.num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
.regs = ufsphy_v4_regs_layout,
@@ -1054,8 +1031,6 @@ static const struct qmp_phy_cfg sc8280xp_ufsphy_cfg = {
.pcs = sm8350_ufsphy_g4_pcs,
.pcs_num = ARRAY_SIZE(sm8350_ufsphy_g4_pcs),
},
- .clk_list = sdm845_ufs_phy_clk_l,
- .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
.vreg_list = qmp_phy_vreg_l,
.num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
.regs = ufsphy_v5_regs_layout,
@@ -1080,8 +1055,6 @@ static const struct qmp_phy_cfg sdm845_ufsphy_cfg = {
.serdes = sdm845_ufsphy_hs_b_serdes,
.serdes_num = ARRAY_SIZE(sdm845_ufsphy_hs_b_serdes),
},
- .clk_list = sdm845_ufs_phy_clk_l,
- .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
.vreg_list = qmp_phy_vreg_l,
.num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
.regs = ufsphy_v3_regs_layout,
@@ -1108,8 +1081,6 @@ static const struct qmp_phy_cfg sm6115_ufsphy_cfg = {
.serdes = sm6115_ufsphy_hs_b_serdes,
.serdes_num = ARRAY_SIZE(sm6115_ufsphy_hs_b_serdes),
},
- .clk_list = sdm845_ufs_phy_clk_l,
- .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
.vreg_list = qmp_phy_vreg_l,
.num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
.regs = ufsphy_v2_regs_layout,
@@ -1136,8 +1107,6 @@ static const struct qmp_phy_cfg sm7150_ufsphy_cfg = {
.serdes = sdm845_ufsphy_hs_b_serdes,
.serdes_num = ARRAY_SIZE(sdm845_ufsphy_hs_b_serdes),
},
- .clk_list = sdm845_ufs_phy_clk_l,
- .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
.vreg_list = qmp_phy_vreg_l,
.num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
.regs = ufsphy_v3_regs_layout,
@@ -1172,8 +1141,6 @@ static const struct qmp_phy_cfg sm8150_ufsphy_cfg = {
.pcs = sm8150_ufsphy_hs_g4_pcs,
.pcs_num = ARRAY_SIZE(sm8150_ufsphy_hs_g4_pcs),
},
- .clk_list = sdm845_ufs_phy_clk_l,
- .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
.vreg_list = qmp_phy_vreg_l,
.num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
.regs = ufsphy_v4_regs_layout,
@@ -1206,8 +1173,6 @@ static const struct qmp_phy_cfg sm8250_ufsphy_cfg = {
.pcs = sm8150_ufsphy_hs_g4_pcs,
.pcs_num = ARRAY_SIZE(sm8150_ufsphy_hs_g4_pcs),
},
- .clk_list = sdm845_ufs_phy_clk_l,
- .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
.vreg_list = qmp_phy_vreg_l,
.num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
.regs = ufsphy_v4_regs_layout,
@@ -1240,8 +1205,6 @@ static const struct qmp_phy_cfg sm8350_ufsphy_cfg = {
.pcs = sm8350_ufsphy_g4_pcs,
.pcs_num = ARRAY_SIZE(sm8350_ufsphy_g4_pcs),
},
- .clk_list = sdm845_ufs_phy_clk_l,
- .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
.vreg_list = qmp_phy_vreg_l,
.num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
.regs = ufsphy_v5_regs_layout,
@@ -1274,8 +1237,6 @@ static const struct qmp_phy_cfg sm8450_ufsphy_cfg = {
.pcs = sm8350_ufsphy_g4_pcs,
.pcs_num = ARRAY_SIZE(sm8350_ufsphy_g4_pcs),
},
- .clk_list = sm8450_ufs_phy_clk_l,
- .num_clks = ARRAY_SIZE(sm8450_ufs_phy_clk_l),
.vreg_list = qmp_phy_vreg_l,
.num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
.regs = ufsphy_v5_regs_layout,
@@ -1296,8 +1257,6 @@ static const struct qmp_phy_cfg sm8550_ufsphy_cfg = {
.pcs = sm8550_ufsphy_pcs,
.pcs_num = ARRAY_SIZE(sm8550_ufsphy_pcs),
},
- .clk_list = sdm845_ufs_phy_clk_l,
- .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
.vreg_list = qmp_phy_vreg_l,
.num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
.regs = ufsphy_v6_regs_layout,
@@ -1383,7 +1342,7 @@ static int qmp_ufs_com_init(struct qmp_ufs *qmp)
return ret;
}
- ret = clk_bulk_prepare_enable(cfg->num_clks, qmp->clks);
+ ret = clk_bulk_prepare_enable(qmp->num_clks, qmp->clks);
if (ret)
goto err_disable_regulators;
@@ -1403,7 +1362,7 @@ static int qmp_ufs_com_exit(struct qmp_ufs *qmp)
reset_control_assert(qmp->ufs_reset);
- clk_bulk_disable_unprepare(cfg->num_clks, qmp->clks);
+ clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
@@ -1573,19 +1532,13 @@ static int qmp_ufs_vreg_init(struct qmp_ufs *qmp)
static int qmp_ufs_clk_init(struct qmp_ufs *qmp)
{
- const struct qmp_phy_cfg *cfg = qmp->cfg;
struct device *dev = qmp->dev;
- int num = cfg->num_clks;
- int i;
- qmp->clks = devm_kcalloc(dev, num, sizeof(*qmp->clks), GFP_KERNEL);
- if (!qmp->clks)
- return -ENOMEM;
+ qmp->num_clks = devm_clk_bulk_get_all(dev, &qmp->clks);
+ if (qmp->num_clks < 0)
+ return qmp->num_clks;
- for (i = 0; i < num; i++)
- qmp->clks[i].id = cfg->clk_list[i];
-
- return devm_clk_bulk_get(dev, num, qmp->clks);
+ return 0;
}
static void qmp_ufs_clk_release_provider(void *res)
--
2.25.1
Add missing QREF clocks for UFS MEM and UFS CARD controllers.
Fixes: 0fadcdfdcf57 ("dt-bindings: clock: Add SC8180x GCC binding")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
include/dt-bindings/clock/qcom,gcc-sc8180x.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/dt-bindings/clock/qcom,gcc-sc8180x.h b/include/dt-bindings/clock/qcom,gcc-sc8180x.h
index e893415ae13d..90c6e021a035 100644
--- a/include/dt-bindings/clock/qcom,gcc-sc8180x.h
+++ b/include/dt-bindings/clock/qcom,gcc-sc8180x.h
@@ -246,6 +246,8 @@
#define GCC_PCIE_3_CLKREF_CLK 236
#define GCC_USB3_PRIM_CLKREF_CLK 237
#define GCC_USB3_SEC_CLKREF_CLK 238
+#define GCC_UFS_MEM_CLKREF_EN 239
+#define GCC_UFS_CARD_CLKREF_EN 240
#define GCC_EMAC_BCR 0
#define GCC_GPU_BCR 1
--
2.25.1
Add missing QREF clocks for UFS MEM and UFS CARD controllers.
Fixes: 4433594bbe5d ("clk: qcom: gcc: Add global clock controller driver for SC8180x")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/clk/qcom/gcc-sc8180x.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c
index ae2147381559..544567db45f1 100644
--- a/drivers/clk/qcom/gcc-sc8180x.c
+++ b/drivers/clk/qcom/gcc-sc8180x.c
@@ -3347,6 +3347,19 @@ static struct clk_branch gcc_ufs_card_2_unipro_core_clk = {
},
};
+static struct clk_branch gcc_ufs_card_clkref_en = {
+ .halt_reg = 0x8c004,
+ .halt_check = BRANCH_HALT,
+ .clkr = {
+ .enable_reg = 0x8c004,
+ .enable_mask = BIT(0),
+ .hw.init = &(const struct clk_init_data) {
+ .name = "gcc_ufs_card_clkref_en",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
static struct clk_branch gcc_ufs_card_ahb_clk = {
.halt_reg = 0x75014,
.halt_check = BRANCH_HALT,
@@ -3561,6 +3574,19 @@ static struct clk_branch gcc_ufs_card_unipro_core_hw_ctl_clk = {
},
};
+static struct clk_branch gcc_ufs_mem_clkref_en = {
+ .halt_reg = 0x8c000,
+ .halt_check = BRANCH_HALT,
+ .clkr = {
+ .enable_reg = 0x8c000,
+ .enable_mask = BIT(0),
+ .hw.init = &(const struct clk_init_data) {
+ .name = "gcc_ufs_mem_clkref_en",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
static struct clk_branch gcc_ufs_phy_ahb_clk = {
.halt_reg = 0x77014,
.halt_check = BRANCH_HALT,
@@ -4413,6 +4439,7 @@ static struct clk_regmap *gcc_sc8180x_clocks[] = {
[GCC_UFS_CARD_2_TX_SYMBOL_0_CLK] = &gcc_ufs_card_2_tx_symbol_0_clk.clkr,
[GCC_UFS_CARD_2_UNIPRO_CORE_CLK] = &gcc_ufs_card_2_unipro_core_clk.clkr,
[GCC_UFS_CARD_2_UNIPRO_CORE_CLK_SRC] = &gcc_ufs_card_2_unipro_core_clk_src.clkr,
+ [GCC_UFS_CARD_CLKREF_EN] = &gcc_ufs_card_clkref_en.clkr,
[GCC_UFS_CARD_AHB_CLK] = &gcc_ufs_card_ahb_clk.clkr,
[GCC_UFS_CARD_AXI_CLK] = &gcc_ufs_card_axi_clk.clkr,
[GCC_UFS_CARD_AXI_CLK_SRC] = &gcc_ufs_card_axi_clk_src.clkr,
@@ -4429,6 +4456,7 @@ static struct clk_regmap *gcc_sc8180x_clocks[] = {
[GCC_UFS_CARD_UNIPRO_CORE_CLK] = &gcc_ufs_card_unipro_core_clk.clkr,
[GCC_UFS_CARD_UNIPRO_CORE_CLK_SRC] = &gcc_ufs_card_unipro_core_clk_src.clkr,
[GCC_UFS_CARD_UNIPRO_CORE_HW_CTL_CLK] = &gcc_ufs_card_unipro_core_hw_ctl_clk.clkr,
+ [GCC_UFS_MEM_CLKREF_EN] = &gcc_ufs_mem_clkref_en.clkr,
[GCC_UFS_PHY_AHB_CLK] = &gcc_ufs_phy_ahb_clk.clkr,
[GCC_UFS_PHY_AXI_CLK] = &gcc_ufs_phy_axi_clk.clkr,
[GCC_UFS_PHY_AXI_CLK_SRC] = &gcc_ufs_phy_axi_clk_src.clkr,
--
2.25.1
QMP PHY used in MSM8998 requires 3 clocks:
* ref - 19.2MHz reference clock from RPM
* ref_aux - Auxiliary reference clock from GCC
* qref - QREF clock from GCC
Fixes: cd3dbe2a4e6c ("arm64: dts: qcom: msm8998: Add UFS nodes")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8998.dtsi | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index b6a3e6afaefd..d4c55e2b0043 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -1051,12 +1051,12 @@ ufsphy: phy@1da7000 {
status = "disabled";
ranges;
- clock-names =
- "ref",
- "ref_aux";
- clocks =
- <&gcc GCC_UFS_CLKREF_CLK>,
- <&gcc GCC_UFS_PHY_AUX_CLK>;
+ clocks = <&rpmcc RPM_SMD_LN_BB_CLK1>,
+ <&gcc GCC_UFS_PHY_AUX_CLK>,
+ <&gcc GCC_UFS_CLKREF_CLK>;
+ clock-names = "ref",
+ "ref_aux",
+ "qref";
reset-names = "ufsphy";
resets = <&ufshc 0>;
--
2.25.1
QMP PHY used in MSM8996 requires 2 clocks:
* ref - 19.2MHz reference clock from RPM
* qref - QREF clock from GCC
Fixes: 27520210e881 ("arm64: dts: qcom: msm8996: Use generic QMP driver for UFS")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 6ba9da9e6a8b..b235f1d651aa 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -2105,8 +2105,8 @@ ufsphy: phy@627000 {
#size-cells = <1>;
ranges;
- clocks = <&gcc GCC_UFS_CLKREF_CLK>;
- clock-names = "ref";
+ clocks = <&rpmcc RPM_SMD_LN_BB_CLK>, <&gcc GCC_UFS_CLKREF_CLK>;
+ clock-names = "ref", "qref";
resets = <&ufshc 0>;
reset-names = "ufsphy";
--
2.25.1
QMP PHY used in SDM845 requires 3 clocks:
* ref - 19.2MHz reference clock from RPMh
* ref_aux - Auxiliary reference clock from GCC
* qref - QREF clock from GCC
While at it, let's move 'clocks' property before 'clock-names' to match
the style used commonly.
Fixes: cc16687fbd74 ("arm64: dts: qcom: sdm845: add UFS controller")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index cb3bfd262851..a7529af5bc6d 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -2619,10 +2619,12 @@ ufs_mem_phy: phy@1d87000 {
#address-cells = <2>;
#size-cells = <2>;
ranges;
+ clocks = <&rpmhcc RPMH_CXO_CLK>,
+ <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
+ <&gcc GCC_UFS_MEM_CLKREF_CLK>;
clock-names = "ref",
- "ref_aux";
- clocks = <&gcc GCC_UFS_MEM_CLKREF_CLK>,
- <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
+ "ref_aux",
+ "qref";
resets = <&ufs_mem_hc 0>;
reset-names = "ufsphy";
--
2.25.1
QMP PHY used in SM6115 requires 3 clocks:
* ref - 19.2MHz reference clock from RPM
* ref_aux - Auxiliary reference clock from GCC
* qref - QREF clock from GCC
Fixes: 97e563bf5ba1 ("arm64: dts: qcom: sm6115: Add basic soc dtsi")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/qcom/sm6115.dtsi | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
index 839c60351240..40394c412fdf 100644
--- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
@@ -1033,8 +1033,12 @@ ufs_mem_phy: phy@4807000 {
#size-cells = <2>;
ranges;
- clocks = <&gcc GCC_UFS_CLKREF_CLK>, <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
- clock-names = "ref", "ref_aux";
+ clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
+ <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
+ <&gcc GCC_UFS_CLKREF_CLK>;
+ clock-names = "ref",
+ "ref_aux",
+ "qref";
resets = <&ufs_mem_hc 0>;
reset-names = "ufsphy";
--
2.25.1
QMP PHY used in SM6125 requires 3 clocks:
* ref - 19.2MHz reference clock from RPM
* ref_aux - Auxiliary reference clock from GCC
* qref - QREF clock from GCC
Fixes: f8399e8a2f80 ("arm64: dts: qcom: sm6125: Add UFS nodes")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/qcom/sm6125.dtsi | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi
index eb07eca3a48d..b46d3c1fa47a 100644
--- a/arch/arm64/boot/dts/qcom/sm6125.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi
@@ -812,10 +812,12 @@ ufs_mem_phy: phy@4807000 {
compatible = "qcom,sm6125-qmp-ufs-phy";
reg = <0x04807000 0xdb8>;
- clocks = <&gcc GCC_UFS_MEM_CLKREF_CLK>,
- <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
+ clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
+ <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
+ <&gcc GCC_UFS_MEM_CLKREF_CLK>;
clock-names = "ref",
- "ref_aux";
+ "ref_aux",
+ "qref";
resets = <&ufs_mem_hc 0>;
reset-names = "ufsphy";
--
2.25.1
QMP PHY used in SM8150 requires 3 clocks:
* ref - 19.2MHz reference clock from RPMh
* ref_aux - Auxiliary reference clock from GCC
* qref - QREF clock from GCC
While at it, let's move 'clocks' property before 'clock-names' to match
the style used commonly.
Fixes: 3834a2e92229 ("arm64: dts: qcom: sm8150: Add ufs nodes")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8150.dtsi | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
index 43d56968a382..18af94852974 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -2065,10 +2065,12 @@ ufs_mem_phy: phy@1d87000 {
#address-cells = <2>;
#size-cells = <2>;
ranges;
+ clocks = <&rpmhcc RPMH_CXO_CLK>,
+ <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
+ <&gcc GCC_UFS_MEM_CLKREF_CLK>;
clock-names = "ref",
- "ref_aux";
- clocks = <&gcc GCC_UFS_MEM_CLKREF_CLK>,
- <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
+ "ref_aux",
+ "qref";
power-domains = <&gcc UFS_PHY_GDSC>;
--
2.25.1
QMP PHY used in SM8250 requires 3 clocks:
* ref - 19.2MHz reference clock from RPMh
* ref_aux - Auxiliary reference clock from GCC
* qref - QREF clock from GCC
While at it, let's move 'clocks' property before 'clock-names' to match
the style used commonly.
Fixes: b7e2fba06622 ("arm64: dts: qcom: sm8250: Add UFS controller and PHY")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index c1b7f9620ec6..e47c515af6cf 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -2486,10 +2486,12 @@ ufs_mem_phy: phy@1d87000 {
#address-cells = <2>;
#size-cells = <2>;
ranges;
- clock-names = "ref",
- "ref_aux";
clocks = <&rpmhcc RPMH_CXO_CLK>,
- <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
+ <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
+ <&gcc GCC_UFS_1X_CLKREF_EN>;
+ clock-names = "ref",
+ "ref_aux",
+ "qref";
resets = <&ufs_mem_hc 0>;
reset-names = "ufsphy";
--
2.25.1
QMP PHY used in SM6350 requires 3 clocks:
* ref - 19.2MHz reference clock from RPMh
* ref_aux - Auxiliary reference clock from GCC
* qref - QREF clock from GCC
While at it, let's move 'clocks' property before 'clock-names' to match
the style used commonly.
Fixes: 5a814af5fc22 ("arm64: dts: qcom: sm6350: Add UFS nodes")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/qcom/sm6350.dtsi | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi
index 8fd6f4d03490..ef793d48316d 100644
--- a/arch/arm64/boot/dts/qcom/sm6350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi
@@ -1192,10 +1192,12 @@ ufs_mem_phy: phy@1d87000 {
#size-cells = <2>;
ranges;
+ clocks = <&rpmhcc RPMH_CXO_CLK>,
+ <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
+ <&gcc GCC_UFS_MEM_CLKREF_CLK>;
clock-names = "ref",
- "ref_aux";
- clocks = <&gcc GCC_UFS_MEM_CLKREF_CLK>,
- <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
+ "ref_aux",
+ "qref";
resets = <&ufs_mem_hc 0>;
reset-names = "ufsphy";
--
2.25.1
QMP PHY used in SC8180X requires 3 clocks:
* ref - 19.2MHz reference clock from RPMh
* ref_aux - Auxiliary reference clock from GCC
* qref - QREF clock from GCC
Fixes: 8575f197b077 ("arm64: dts: qcom: Introduce the SC8180x platform")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/qcom/sc8180x.dtsi | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc8180x.dtsi b/arch/arm64/boot/dts/qcom/sc8180x.dtsi
index 8bcc8c0bb0d0..5591e147bde1 100644
--- a/arch/arm64/boot/dts/qcom/sc8180x.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8180x.dtsi
@@ -2122,9 +2122,11 @@ ufs_mem_phy: phy-wrapper@1d87000 {
reg = <0 0x01d87000 0 0x1000>;
clocks = <&rpmhcc RPMH_CXO_CLK>,
- <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
+ <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
+ <&gcc GCC_UFS_MEM_CLKREF_EN>;
clock-names = "ref",
- "ref_aux";
+ "ref_aux",
+ "qref";
resets = <&ufs_mem_hc 0>;
reset-names = "ufsphy";
--
2.25.1
QMP PHY used in SM8350 requires 3 clocks:
* ref - 19.2MHz reference clock from RPMh
* ref_aux - Auxiliary reference clock from GCC
* qref - QREF clock from GCC
While at it, let's move 'clocks' property before 'clock-names' to match
the style used commonly.
Fixes: 59c7cf814783 ("arm64: dts: qcom: sm8350: Add UFS nodes")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8350.dtsi | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index f4b8439200f5..38a09d71b3e9 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -1727,10 +1727,12 @@ ufs_mem_phy: phy@1d87000 {
#address-cells = <2>;
#size-cells = <2>;
ranges;
- clock-names = "ref",
- "ref_aux";
clocks = <&rpmhcc RPMH_CXO_CLK>,
- <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
+ <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
+ <&gcc GCC_UFS_1_CLKREF_EN>;
+ clock-names = "ref",
+ "ref_aux",
+ "qref";
resets = <&ufs_mem_hc 0>;
reset-names = "ufsphy";
--
2.25.1
QMP PHY used in SC8280XP requires 3 clocks:
* ref - 19.2MHz reference clock from RPMh
* ref_aux - Auxiliary reference clock from GCC
* qref - QREF clock from GCC
Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index cad59af7ccef..37344abbe8bf 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -2256,9 +2256,12 @@ ufs_mem_phy: phy@1d87000 {
compatible = "qcom,sc8280xp-qmp-ufs-phy";
reg = <0 0x01d87000 0 0x1000>;
- clocks = <&gcc GCC_UFS_CARD_CLKREF_CLK>,
- <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
- clock-names = "ref", "ref_aux";
+ clocks = <&rpmhcc RPMH_CXO_CLK>,
+ <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
+ <&gcc GCC_UFS_CARD_CLKREF_CLK>;
+ clock-names = "ref",
+ "ref_aux",
+ "qref";
power-domains = <&gcc UFS_PHY_GDSC>;
@@ -2318,9 +2321,12 @@ ufs_card_phy: phy@1da7000 {
compatible = "qcom,sc8280xp-qmp-ufs-phy";
reg = <0 0x01da7000 0 0x1000>;
- clocks = <&gcc GCC_UFS_1_CARD_CLKREF_CLK>,
- <&gcc GCC_UFS_CARD_PHY_AUX_CLK>;
- clock-names = "ref", "ref_aux";
+ clocks = <&rpmhcc RPMH_CXO_CLK>,
+ <&gcc GCC_UFS_CARD_PHY_AUX_CLK>,
+ <&gcc GCC_UFS_1_CARD_CLKREF_CLK>;
+ clock-names = "ref",
+ "ref_aux",
+ "qref";
power-domains = <&gcc UFS_CARD_GDSC>;
--
2.25.1
QMP PHY used in SM8550 requires 3 clocks:
* ref - 19.2MHz reference clock from RPMh
* ref_aux - Auxiliary reference clock from GCC
* qref - QREF clock from TCSR
Fixes: 35cf1aaab169 ("arm64: dts: qcom: sm8550: Add UFS host controller and phy nodes")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8550.dtsi | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index baa8540868a4..386ffd0d72c4 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -1891,9 +1891,12 @@ crypto: crypto@1dfa000 {
ufs_mem_phy: phy@1d80000 {
compatible = "qcom,sm8550-qmp-ufs-phy";
reg = <0x0 0x01d80000 0x0 0x2000>;
- clocks = <&tcsr TCSR_UFS_CLKREF_EN>,
- <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
- clock-names = "ref", "ref_aux";
+ clocks = <&rpmhcc RPMH_CXO_CLK>,
+ <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
+ <&tcsr TCSR_UFS_CLKREF_EN>;
+ clock-names = "ref",
+ "ref_aux",
+ "qref";
power-domains = <&gcc UFS_MEM_PHY_GDSC>;
--
2.25.1
On Thu, Dec 14, 2023 at 02:40:45PM +0530, Manivannan Sadhasivam wrote:
> This series fixes the clocks supplied to QMP PHY IPs in the Qcom SoCs. All
> of the Qcom SoCs except MSM8996 require 3 clocks for QMP UFS:
>
> * ref - 19.2MHz reference clock from RPM/RPMh
> * ref_aux - Auxiliary reference clock from GCC
> * qref - QREF clock from GCC or TCSR (TCSR since SM8550)
>
> MSM8996 only requires 'ref' and 'qref' clocks.
>
> Hence, this series fixes the binding, DT and GCC driver to reflect the
> actual clock topology.
Is this based on documentation for all the SoCs or on inference from the
current (upstream and downstream) devicetrees?
Are you sure that you should not just describe that some of these UFS
reference clocks are sourced from CXO in the clock driver instead?
Take a look at commits
f446022b932a ("arm64: dts: qcom: sc8280xp: fix UFS reference clocks")
f6abcc21d943 ("clk: qcom: gcc-sc8280xp: add cxo as parent for three ufs ref clks")
Johan
On Thu, Dec 14, 2023 at 11:15:34AM +0100, Johan Hovold wrote:
> On Thu, Dec 14, 2023 at 02:40:45PM +0530, Manivannan Sadhasivam wrote:
>
> > This series fixes the clocks supplied to QMP PHY IPs in the Qcom SoCs. All
> > of the Qcom SoCs except MSM8996 require 3 clocks for QMP UFS:
> >
> > * ref - 19.2MHz reference clock from RPM/RPMh
> > * ref_aux - Auxiliary reference clock from GCC
> > * qref - QREF clock from GCC or TCSR (TCSR since SM8550)
> >
> > MSM8996 only requires 'ref' and 'qref' clocks.
> >
> > Hence, this series fixes the binding, DT and GCC driver to reflect the
> > actual clock topology.
>
> Is this based on documentation for all the SoCs or on inference from the
> current (upstream and downstream) devicetrees?
>
It is based on the internal documentation. Even downstream devicetrees are
wrong. I should've mentioned it in the cover letter.
> Are you sure that you should not just describe that some of these UFS
> reference clocks are sourced from CXO in the clock driver instead?
>
I don't get your comment fully. Could you please elaborate?
> Take a look at commits
>
> f446022b932a ("arm64: dts: qcom: sc8280xp: fix UFS reference clocks")
> f6abcc21d943 ("clk: qcom: gcc-sc8280xp: add cxo as parent for three ufs ref clks")
>
Btw, these commits are not accurate. In all the SoCs before SM8550, reference
clock for the UFS device comes from the UFS controller. There is a dedicated
register in UFSHC memory map that is being toggled by the driver to
enable/disable reference clock for the UFS device.
Since SM8550, reference clock is directly sourced from RPMh. I'm preparing a
series to fix it.
Unfortunately, this information is not depicted correctly in the downstream
devicetrees.
- Mani
> Johan
>
--
மணிவண்ணன் சதாசிவம்
[ +CC: Shazad ]
On Thu, Dec 14, 2023 at 04:09:07PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Dec 14, 2023 at 11:15:34AM +0100, Johan Hovold wrote:
> > On Thu, Dec 14, 2023 at 02:40:45PM +0530, Manivannan Sadhasivam wrote:
> >
> > > This series fixes the clocks supplied to QMP PHY IPs in the Qcom SoCs. All
> > > of the Qcom SoCs except MSM8996 require 3 clocks for QMP UFS:
> > >
> > > * ref - 19.2MHz reference clock from RPM/RPMh
> > > * ref_aux - Auxiliary reference clock from GCC
> > > * qref - QREF clock from GCC or TCSR (TCSR since SM8550)
> > >
> > > MSM8996 only requires 'ref' and 'qref' clocks.
> > >
> > > Hence, this series fixes the binding, DT and GCC driver to reflect the
> > > actual clock topology.
> >
> > Is this based on documentation for all the SoCs or on inference from the
> > current (upstream and downstream) devicetrees?
>
> It is based on the internal documentation. Even downstream devicetrees are
> wrong. I should've mentioned it in the cover letter.
>
> > Are you sure that you should not just describe that some of these UFS
> > reference clocks are sourced from CXO in the clock driver instead?
>
> I don't get your comment fully. Could you please elaborate?
Unless the PHY consumes CXO directly, it should not be included in the
binding as you are suggesting here.
We discussed this at some length at the time with Bjorn and Shazad who
had access to the documentation and the conclusion was that, at least on
sc8280xp, the PHY does not use CXO directly and instead it should be
described as a parent to the UFS refclocks in the clock driver:
https://lore.kernel.org/lkml/[email protected]/
The downstream devicetrees have a bad habit of including parent clocks
directly in the consumer node instead of modelling this in clock driver
also for other peripherals.
> > Take a look at commits
> >
> > f446022b932a ("arm64: dts: qcom: sc8280xp: fix UFS reference clocks")
> > f6abcc21d943 ("clk: qcom: gcc-sc8280xp: add cxo as parent for three ufs ref clks")
>
> Btw, these commits are not accurate. In all the SoCs before SM8550, reference
> clock for the UFS device comes from the UFS controller. There is a dedicated
> register in UFSHC memory map that is being toggled by the driver to
> enable/disable reference clock for the UFS device.
>
> Since SM8550, reference clock is directly sourced from RPMh. I'm preparing a
> series to fix it.
What exactly is wrong with those commits? We know that the controller
does not consume GCC_UFS_REF_CLKREF_CLK directly, but describing that as
such for now was a deliberate choice:
GCC_UFS_REF_CLKREF_CLK is the clock to the devices, but as we
don't represent the memory device explicitly it seems suitable
to use as "ref_clk" in the ufshc nodes - which would then match
the special handling of the "link clock" in the UFS driver.
> Unfortunately, this information is not depicted correctly in the downstream
> devicetrees.
I was hoping the information that those commits are based on would be
correct as it came from Qualcomm and Bjorn. I have no illusions about
the downstream devicetrees being correct. :)
Johan
+ Can
On Thu, Dec 14, 2023 at 12:00:40PM +0100, Johan Hovold wrote:
> [ +CC: Shazad ]
>
> On Thu, Dec 14, 2023 at 04:09:07PM +0530, Manivannan Sadhasivam wrote:
> > On Thu, Dec 14, 2023 at 11:15:34AM +0100, Johan Hovold wrote:
> > > On Thu, Dec 14, 2023 at 02:40:45PM +0530, Manivannan Sadhasivam wrote:
> > >
> > > > This series fixes the clocks supplied to QMP PHY IPs in the Qcom SoCs. All
> > > > of the Qcom SoCs except MSM8996 require 3 clocks for QMP UFS:
> > > >
> > > > * ref - 19.2MHz reference clock from RPM/RPMh
> > > > * ref_aux - Auxiliary reference clock from GCC
> > > > * qref - QREF clock from GCC or TCSR (TCSR since SM8550)
> > > >
> > > > MSM8996 only requires 'ref' and 'qref' clocks.
> > > >
> > > > Hence, this series fixes the binding, DT and GCC driver to reflect the
> > > > actual clock topology.
> > >
> > > Is this based on documentation for all the SoCs or on inference from the
> > > current (upstream and downstream) devicetrees?
> >
> > It is based on the internal documentation. Even downstream devicetrees are
> > wrong. I should've mentioned it in the cover letter.
> >
> > > Are you sure that you should not just describe that some of these UFS
> > > reference clocks are sourced from CXO in the clock driver instead?
> >
> > I don't get your comment fully. Could you please elaborate?
>
> Unless the PHY consumes CXO directly, it should not be included in the
> binding as you are suggesting here.
>
PHY is indeed directly consuming CXO. That's why I included it in the binding.
> We discussed this at some length at the time with Bjorn and Shazad who
> had access to the documentation and the conclusion was that, at least on
> sc8280xp, the PHY does not use CXO directly and instead it should be
> described as a parent to the UFS refclocks in the clock driver:
>
> https://lore.kernel.org/lkml/[email protected]/
>
> The downstream devicetrees have a bad habit of including parent clocks
> directly in the consumer node instead of modelling this in clock driver
> also for other peripherals.
>
No, I can assure that you got the wrong info. UFS PHY consumes the clock
directly from RPMh. It took me several days to dig through the UFS and PHY docs
and special thanks to Can Guo from UFS team, who provided much valuable
information about these clocks.
> > > Take a look at commits
> > >
> > > f446022b932a ("arm64: dts: qcom: sc8280xp: fix UFS reference clocks")
> > > f6abcc21d943 ("clk: qcom: gcc-sc8280xp: add cxo as parent for three ufs ref clks")
> >
> > Btw, these commits are not accurate. In all the SoCs before SM8550, reference
> > clock for the UFS device comes from the UFS controller. There is a dedicated
> > register in UFSHC memory map that is being toggled by the driver to
> > enable/disable reference clock for the UFS device.
> >
> > Since SM8550, reference clock is directly sourced from RPMh. I'm preparing a
> > series to fix it.
>
> What exactly is wrong with those commits? We know that the controller
> does not consume GCC_UFS_REF_CLKREF_CLK directly, but describing that as
> such for now was a deliberate choice:
>
> GCC_UFS_REF_CLKREF_CLK is the clock to the devices, but as we
> don't represent the memory device explicitly it seems suitable
> to use as "ref_clk" in the ufshc nodes - which would then match
> the special handling of the "link clock" in the UFS driver.
>
No, GCC_UFS_REF_CLKREF_CLK is _not_ the clock to UFS devices. I haven't found
information about this specific register in GCC. Initially I thought this is for
enabling QREF clocks for the UFS MEM phy, but I haven't found the answer yet.
But as I said earlier, reference clock to UFS devices comes directly from the
controller and there is a specfic register for controlling that. Starting from
SM8550, reference clock comes from RPMh.
> > Unfortunately, this information is not depicted correctly in the downstream
> > devicetrees.
>
> I was hoping the information that those commits are based on would be
> correct as it came from Qualcomm and Bjorn. I have no illusions about
> the downstream devicetrees being correct. :)
>
Unfortunately, you got inaccurate information. But that's very common, since
these kind of info are buried down in some docs and people's mind :)
- Mani
> Johan
--
மணிவண்ணன் சதாசிவம்
On Thu, Dec 14, 2023 at 04:44:09PM +0530, Manivannan Sadhasivam wrote:
> + Can
>
> On Thu, Dec 14, 2023 at 12:00:40PM +0100, Johan Hovold wrote:
> > [ +CC: Shazad ]
> >
> > On Thu, Dec 14, 2023 at 04:09:07PM +0530, Manivannan Sadhasivam wrote:
> > > On Thu, Dec 14, 2023 at 11:15:34AM +0100, Johan Hovold wrote:
> > > > On Thu, Dec 14, 2023 at 02:40:45PM +0530, Manivannan Sadhasivam wrote:
> > Unless the PHY consumes CXO directly, it should not be included in the
> > binding as you are suggesting here.
>
> PHY is indeed directly consuming CXO. That's why I included it in the binding.
Ok, good. It's a bit frustrating that people can even seem to agree on
answers to direct questions about that.
> > We discussed this at some length at the time with Bjorn and Shazad who
> > had access to the documentation and the conclusion was that, at least on
> > sc8280xp, the PHY does not use CXO directly and instead it should be
> > described as a parent to the UFS refclocks in the clock driver:
> >
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > The downstream devicetrees have a bad habit of including parent clocks
> > directly in the consumer node instead of modelling this in clock driver
> > also for other peripherals.
> >
>
> No, I can assure that you got the wrong info. UFS PHY consumes the clock
> directly from RPMh. It took me several days to dig through the UFS and PHY docs
> and special thanks to Can Guo from UFS team, who provided much valuable
> information about these clocks.
Sounds like you've done your research.
> > What exactly is wrong with those commits? We know that the controller
> > does not consume GCC_UFS_REF_CLKREF_CLK directly, but describing that as
> > such for now was a deliberate choice:
> >
> > GCC_UFS_REF_CLKREF_CLK is the clock to the devices, but as we
> > don't represent the memory device explicitly it seems suitable
> > to use as "ref_clk" in the ufshc nodes - which would then match
> > the special handling of the "link clock" in the UFS driver.
> >
>
> No, GCC_UFS_REF_CLKREF_CLK is _not_ the clock to UFS devices. I haven't found
> information about this specific register in GCC. Initially I thought this is for
> enabling QREF clocks for the UFS MEM phy, but I haven't found the answer yet.
Just quoting Bjorn.
> But as I said earlier, reference clock to UFS devices comes directly from the
> controller and there is a specfic register for controlling that. Starting from
> SM8550, reference clock comes from RPMh.
Sure, but that was only part of what those commits did or claimed. Bjorn
also explicitly stated that those refclocks were sourced from CXO, even
though I now see a claim from Shazad in that thread claiming the
opposite:
https://lore.kernel.org/all/[email protected]/
Without access to docs I can only ask questions and try to do tedious
inferences from incomplete open sources (e.g. downstream devicetrees).
Johan
On Thu, Dec 14, 2023 at 12:30:45PM +0100, Johan Hovold wrote:
> On Thu, Dec 14, 2023 at 04:44:09PM +0530, Manivannan Sadhasivam wrote:
> > + Can
> >
> > On Thu, Dec 14, 2023 at 12:00:40PM +0100, Johan Hovold wrote:
> > > [ +CC: Shazad ]
> > >
> > > On Thu, Dec 14, 2023 at 04:09:07PM +0530, Manivannan Sadhasivam wrote:
> > > > On Thu, Dec 14, 2023 at 11:15:34AM +0100, Johan Hovold wrote:
> > > > > On Thu, Dec 14, 2023 at 02:40:45PM +0530, Manivannan Sadhasivam wrote:
>
> > > Unless the PHY consumes CXO directly, it should not be included in the
> > > binding as you are suggesting here.
> >
> > PHY is indeed directly consuming CXO. That's why I included it in the binding.
>
> Ok, good. It's a bit frustrating that people can even seem to agree on
> answers to direct questions about that.
>
I can understand that.
> > > We discussed this at some length at the time with Bjorn and Shazad who
> > > had access to the documentation and the conclusion was that, at least on
> > > sc8280xp, the PHY does not use CXO directly and instead it should be
> > > described as a parent to the UFS refclocks in the clock driver:
> > >
> > > https://lore.kernel.org/lkml/[email protected]/
> > >
> > > The downstream devicetrees have a bad habit of including parent clocks
> > > directly in the consumer node instead of modelling this in clock driver
> > > also for other peripherals.
> > >
> >
> > No, I can assure that you got the wrong info. UFS PHY consumes the clock
> > directly from RPMh. It took me several days to dig through the UFS and PHY docs
> > and special thanks to Can Guo from UFS team, who provided much valuable
> > information about these clocks.
>
> Sounds like you've done your research.
>
> > > What exactly is wrong with those commits? We know that the controller
> > > does not consume GCC_UFS_REF_CLKREF_CLK directly, but describing that as
> > > such for now was a deliberate choice:
> > >
> > > GCC_UFS_REF_CLKREF_CLK is the clock to the devices, but as we
> > > don't represent the memory device explicitly it seems suitable
> > > to use as "ref_clk" in the ufshc nodes - which would then match
> > > the special handling of the "link clock" in the UFS driver.
> > >
> >
> > No, GCC_UFS_REF_CLKREF_CLK is _not_ the clock to UFS devices. I haven't found
> > information about this specific register in GCC. Initially I thought this is for
> > enabling QREF clocks for the UFS MEM phy, but I haven't found the answer yet.
>
> Just quoting Bjorn.
>
> > But as I said earlier, reference clock to UFS devices comes directly from the
> > controller and there is a specfic register for controlling that. Starting from
> > SM8550, reference clock comes from RPMh.
>
> Sure, but that was only part of what those commits did or claimed. Bjorn
> also explicitly stated that those refclocks were sourced from CXO, even
> though I now see a claim from Shazad in that thread claiming the
> opposite:
>
> https://lore.kernel.org/all/[email protected]/
To clarify further, what Shazad said about GCC_UFS_REF_CLKREF_CLK is correct.
This clock is not directly sourced by CXO, so it should be voted by the
_PHY_ driver separately along with CXO (which still feeds PHY). That's what I
represented in the binding.
>
> Without access to docs I can only ask questions and try to do tedious
> inferences from incomplete open sources (e.g. downstream devicetrees).
>
That's the life for most of us :) Even with access to internal docs, it is
difficult to find the information we are looking for. Because, a very few people
know where the information is buried.
- Mani
> Johan
--
மணிவண்ணன் சதாசிவம்
On Thu, Dec 14, 2023 at 02:40:48PM +0530, Manivannan Sadhasivam wrote:
> Add missing QREF clocks for UFS MEM and UFS CARD controllers.
>
> Fixes: 0fadcdfdcf57 ("dt-bindings: clock: Add SC8180x GCC binding")
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Conor Dooley <[email protected]>
Cheers,
Conor.
> ---
> include/dt-bindings/clock/qcom,gcc-sc8180x.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/dt-bindings/clock/qcom,gcc-sc8180x.h b/include/dt-bindings/clock/qcom,gcc-sc8180x.h
> index e893415ae13d..90c6e021a035 100644
> --- a/include/dt-bindings/clock/qcom,gcc-sc8180x.h
> +++ b/include/dt-bindings/clock/qcom,gcc-sc8180x.h
> @@ -246,6 +246,8 @@
> #define GCC_PCIE_3_CLKREF_CLK 236
> #define GCC_USB3_PRIM_CLKREF_CLK 237
> #define GCC_USB3_SEC_CLKREF_CLK 238
> +#define GCC_UFS_MEM_CLKREF_EN 239
> +#define GCC_UFS_CARD_CLKREF_EN 240
>
> #define GCC_EMAC_BCR 0
> #define GCC_GPU_BCR 1
> --
> 2.25.1
>
On Thu, Dec 14, 2023 at 02:40:46PM +0530, Manivannan Sadhasivam wrote:
> All QMP UFS PHYs except MSM8996 require 3 clocks:
>
> * ref - 19.2MHz reference clock from RPMh
> * ref_aux - Auxiliary reference clock from GCC
> * qref - QREF clock from GCC or TCSR (since SM8550)
>
> MSM8996 only requires 'ref' and 'qref' clocks. Hence, fix the binding to
> reflect the actual clock topology.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
Cheers,
Conor.
On Thu, Dec 14, 2023 at 02:40:46PM +0530, Manivannan Sadhasivam wrote:
> All QMP UFS PHYs except MSM8996 require 3 clocks:
>
> * ref - 19.2MHz reference clock from RPMh
> * ref_aux - Auxiliary reference clock from GCC
> * qref - QREF clock from GCC or TCSR (since SM8550)
>
> MSM8996 only requires 'ref' and 'qref' clocks. Hence, fix the binding to
> reflect the actual clock topology.
Breaking the ABI is okay because...? Please explain in the commit msg.
Rob
On Thu, Dec 14, 2023 at 11:20:51AM -0600, Rob Herring wrote:
> On Thu, Dec 14, 2023 at 02:40:46PM +0530, Manivannan Sadhasivam wrote:
> > All QMP UFS PHYs except MSM8996 require 3 clocks:
> >
> > * ref - 19.2MHz reference clock from RPMh
> > * ref_aux - Auxiliary reference clock from GCC
> > * qref - QREF clock from GCC or TCSR (since SM8550)
> >
> > MSM8996 only requires 'ref' and 'qref' clocks. Hence, fix the binding to
> > reflect the actual clock topology.
>
> Breaking the ABI is okay because...? Please explain in the commit msg.
>
I will update the commit message in v2.
- Mani
> Rob
>
--
மணிவண்ணன் சதாசிவம்