2022-04-16 03:12:05

by Yassine Oudjana

[permalink] [raw]
Subject: [PATCH RESEND v2 0/9] Add support for MSM8996 Pro

MSM8996 Pro (also known as MSM8996SG) is a newer revision of MSM8996
with different CPU/CBF/GPU frequencies and CPR parameters. Its CBF clock
also has a different divisor.

This series handles the difference in the CBF clock and adds a new DTSI for
MSM8996 Pro with CPU and GPU OPPs. It also removes reading msm-id from SMEM
in qcom-cpufreq-nvmem as it becomes no longer necessary with the introduction.
of a separate device tree. Separating MSM8996 and MSM8996 Pro will help with
implementing CBF scaling and CPR; as they have different CPR parameters
and CPU:CBF OPP mapping which is difficult to implement in the same cluster
OPP tables.

Dependencies:
- clk: qcom: msm8996-cpu: Add CBF support
https://lore.kernel.org/linux-arm-msm/[email protected]/#t
- arm64: dts: qcom: msm8996: Add support for the CBF clock
https://lore.kernel.org/linux-arm-msm/[email protected]/

Changes since v1:
- Rebase DT changes on already merged patches[1][2].
- Add more details to commit messages.
- Split removing MSM8996 Pro speed bin bits from opp-supported-hw into
a separate patch.
- Rename msm8996-xiaomi-scorpio.dts to msm8996pro-xiaomi-scorpio.dts

[1] https://lore.kernel.org/linux-arm-msm/[email protected]/T/#m6e1341ccfa50d11d221ba8c618f73c21a83b8acb
[2] https://lore.kernel.org/linux-arm-msm/[email protected]/T/#m36f194cd9da1fee7058a88412985aab10c499fa7

Yassine Oudjana (9):
dt-bindings: clk: qcom: msm8996-apcc: Add CBF
dt-bindings: clk: qcom: msm8996-apcc: Add MSM8996 Pro compatible
clk: qcom: msm8996-cpu: Add MSM8996 Pro CBF support
cpufreq: qcom_cpufreq_nvmem: Simplify reading kryo speedbin
dt-bindings: opp: opp-v2-kryo-cpu: Remove SMEM
arm64: dts: qcom: msm8996: Remove MSM8996 Pro speed bins from cluster
OPP tables
dt-bindings: arm: qcom: Add MSM8996 Pro compatible
arm64: dts: qcom: msm8996: Add MSM8996 Pro support
arm64: dts: qcom: msm8996-xiaomi-scorpio: Use MSM8996 Pro

.../devicetree/bindings/arm/qcom.yaml | 5 +
.../bindings/clock/qcom,msm8996-apcc.yaml | 11 +-
.../bindings/opp/opp-v2-kryo-cpu.yaml | 56 ++--
arch/arm64/boot/dts/qcom/Makefile | 2 +-
.../boot/dts/qcom/msm8996-xiaomi-common.dtsi | 3 -
.../boot/dts/qcom/msm8996-xiaomi-gemini.dts | 1 +
arch/arm64/boot/dts/qcom/msm8996.dtsi | 82 ++---
...rpio.dts => msm8996pro-xiaomi-scorpio.dts} | 4 +-
arch/arm64/boot/dts/qcom/msm8996pro.dtsi | 281 ++++++++++++++++++
drivers/clk/qcom/clk-cpu-8996.c | 61 ++--
drivers/cpufreq/Kconfig.arm | 1 -
drivers/cpufreq/qcom-cpufreq-nvmem.c | 75 +----
12 files changed, 410 insertions(+), 172 deletions(-)
rename arch/arm64/boot/dts/qcom/{msm8996-xiaomi-scorpio.dts => msm8996pro-xiaomi-scorpio.dts} (99%)
create mode 100644 arch/arm64/boot/dts/qcom/msm8996pro.dtsi

--
2.35.1


2022-04-16 03:12:13

by Yassine Oudjana

[permalink] [raw]
Subject: [PATCH RESEND v2 1/9] dt-bindings: clk: qcom: msm8996-apcc: Add CBF

Add CBF clock and reg.

Signed-off-by: Yassine Oudjana <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
.../devicetree/bindings/clock/qcom,msm8996-apcc.yaml | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
index a20cb10636dd..325f8aef53b2 100644
--- a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
@@ -10,8 +10,8 @@ maintainers:
- Loic Poulain <[email protected]>

description: |
- Qualcomm CPU clock controller for MSM8996 CPUs, clock 0 is for Power cluster
- and clock 1 is for Perf cluster.
+ Qualcomm CPU clock controller for MSM8996 CPUs, clock 0 is for Power cluster,
+ clock 1 is for Perf cluster, and clock 2 is for Coherent bus fabric (CBF).

properties:
compatible:
@@ -19,7 +19,9 @@ properties:
- qcom,msm8996-apcc

reg:
- maxItems: 1
+ items:
+ - description: Cluster clock registers
+ - description: CBF clock registers

'#clock-cells':
const: 1
@@ -49,6 +51,6 @@ examples:
- |
kryocc: clock-controller@6400000 {
compatible = "qcom,msm8996-apcc";
- reg = <0x6400000 0x90000>;
+ reg = <0x6400000 0x90000>, <0x09a11000 0x10000>;
#clock-cells = <1>;
};
--
2.35.1

2022-04-16 03:12:19

by Yassine Oudjana

[permalink] [raw]
Subject: [PATCH RESEND v2 3/9] clk: qcom: msm8996-cpu: Add MSM8996 Pro CBF support

MSM8996 Pro (MSM8996SG) has a /4 divisor on the CBF clock
instead of /2. This allows it to reach a lower minimum frequency
of 192000000Hz compared to 307200000Hz on regular MSM8996.
Add support for setting the CBF clock divisor to /4 for MSM8996 Pro.

Signed-off-by: Yassine Oudjana <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/clk-cpu-8996.c | 61 +++++++++++++++++++++------------
1 file changed, 40 insertions(+), 21 deletions(-)

diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c
index 8afc271f92d0..231d8224fa16 100644
--- a/drivers/clk/qcom/clk-cpu-8996.c
+++ b/drivers/clk/qcom/clk-cpu-8996.c
@@ -70,11 +70,11 @@ enum _pmux_input {

enum {
CBF_PLL_INDEX = 1,
- CBF_DIV_2_INDEX,
+ CBF_DIV_INDEX,
CBF_SAFE_INDEX
};

-#define DIV_2_THRESHOLD 600000000
+#define DIV_THRESHOLD 600000000
#define PWRCL_REG_OFFSET 0x0
#define PERFCL_REG_OFFSET 0x80000
#define MUX_OFFSET 0x40
@@ -142,6 +142,17 @@ static const struct alpha_pll_config cbfpll_config = {
.early_output_mask = BIT(3),
};

+static const struct alpha_pll_config cbfpll_config_pro = {
+ .l = 72,
+ .config_ctl_val = 0x200d4aa8,
+ .config_ctl_hi_val = 0x006,
+ .pre_div_mask = BIT(12),
+ .post_div_mask = 0x3 << 8,
+ .post_div_val = 0x3 << 8,
+ .main_output_mask = BIT(0),
+ .early_output_mask = BIT(3),
+};
+
static struct clk_alpha_pll perfcl_pll = {
.offset = PERFCL_REG_OFFSET,
.regs = prim_pll_regs,
@@ -230,7 +241,8 @@ struct clk_cpu_8996_mux {
u8 width;
struct notifier_block nb;
struct clk_hw *pll;
- struct clk_hw *pll_div_2;
+ struct clk_hw *pll_div;
+ u8 div;
struct clk_regmap clkr;
};

@@ -280,11 +292,11 @@ static int clk_cpu_8996_mux_determine_rate(struct clk_hw *hw,
struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw);
struct clk_hw *parent = cpuclk->pll;

- if (cpuclk->pll_div_2 && req->rate < DIV_2_THRESHOLD) {
- if (req->rate < (DIV_2_THRESHOLD / 2))
+ if (cpuclk->pll_div && req->rate < DIV_THRESHOLD) {
+ if (req->rate < (DIV_THRESHOLD / cpuclk->div))
return -EINVAL;

- parent = cpuclk->pll_div_2;
+ parent = cpuclk->pll_div;
}

req->best_parent_rate = clk_hw_round_rate(parent, req->rate);
@@ -336,7 +348,8 @@ static struct clk_cpu_8996_mux pwrcl_pmux = {
.shift = 0,
.width = 2,
.pll = &pwrcl_pll.clkr.hw,
- .pll_div_2 = &pwrcl_smux.clkr.hw,
+ .pll_div = &pwrcl_smux.clkr.hw,
+ .div = 2,
.nb.notifier_call = cpu_clk_notifier_cb,
.clkr.hw.init = &(struct clk_init_data) {
.name = "pwrcl_pmux",
@@ -358,7 +371,8 @@ static struct clk_cpu_8996_mux perfcl_pmux = {
.shift = 0,
.width = 2,
.pll = &perfcl_pll.clkr.hw,
- .pll_div_2 = &perfcl_smux.clkr.hw,
+ .pll_div = &perfcl_smux.clkr.hw,
+ .div = 2,
.nb.notifier_call = cpu_clk_notifier_cb,
.clkr.hw.init = &(struct clk_init_data) {
.name = "perfcl_pmux",
@@ -481,19 +495,23 @@ static int qcom_cbf_clk_msm8996_register_clks(struct device *dev,
struct regmap *regmap)
{
int ret;
+ bool is_pro = of_device_is_compatible(dev->of_node, "qcom,msm8996pro-apcc");

- cbf_mux.pll_div_2 = clk_hw_register_fixed_factor(dev, "cbf_pll_main",
- "cbf_pll", CLK_SET_RATE_PARENT,
- 1, 2);
- if (IS_ERR(cbf_mux.pll_div_2)) {
+ cbf_mux.div = is_pro ? 4 : 2;
+ cbf_mux.pll_div = clk_hw_register_fixed_factor(dev, "cbf_pll_main",
+ "cbf_pll", CLK_SET_RATE_PARENT,
+ 1, cbf_mux.div);
+
+ if (IS_ERR(cbf_mux.pll_div)) {
dev_err(dev, "Failed to initialize cbf_pll_main\n");
- return PTR_ERR(cbf_mux.pll_div_2);
+ return PTR_ERR(cbf_mux.pll_div);
}

ret = devm_clk_register_regmap(dev, cbf_msm8996_clks[0]);
ret = devm_clk_register_regmap(dev, cbf_msm8996_clks[1]);

- clk_alpha_pll_configure(&cbf_pll, regmap, &cbfpll_config);
+ clk_alpha_pll_configure(&cbf_pll, regmap, is_pro ?
+ &cbfpll_config_pro : &cbfpll_config);
clk_set_rate(cbf_pll.clkr.hw.clk, 614400000);
clk_prepare_enable(cbf_pll.clkr.hw.clk);
clk_notifier_register(cbf_mux.clkr.hw.clk, &cbf_mux.nb);
@@ -575,7 +593,7 @@ static int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
qcom_cpu_clk_msm8996_acd_init(base);
break;
case POST_RATE_CHANGE:
- if (cnd->new_rate < DIV_2_THRESHOLD)
+ if (cnd->new_rate < DIV_THRESHOLD)
ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw,
DIV_2_INDEX);
else
@@ -600,15 +618,15 @@ static int cbf_clk_notifier_cb(struct notifier_block *nb, unsigned long event,

switch (event) {
case PRE_RATE_CHANGE:
- parent = clk_hw_get_parent_by_index(&cbfclk->clkr.hw, CBF_DIV_2_INDEX);
- ret = clk_cpu_8996_mux_set_parent(&cbfclk->clkr.hw, CBF_DIV_2_INDEX);
+ parent = clk_hw_get_parent_by_index(&cbfclk->clkr.hw, CBF_DIV_INDEX);
+ ret = clk_cpu_8996_mux_set_parent(&cbfclk->clkr.hw, CBF_DIV_INDEX);

- if (cnd->old_rate > DIV_2_THRESHOLD && cnd->new_rate < DIV_2_THRESHOLD)
- ret = clk_set_rate(parent->clk, cnd->old_rate / 2);
+ if (cnd->old_rate > DIV_THRESHOLD && cnd->new_rate < DIV_THRESHOLD)
+ ret = clk_set_rate(parent->clk, cnd->old_rate / cbfclk->div);
break;
case POST_RATE_CHANGE:
- if (cnd->new_rate < DIV_2_THRESHOLD)
- ret = clk_cpu_8996_mux_set_parent(&cbfclk->clkr.hw, CBF_DIV_2_INDEX);
+ if (cnd->new_rate < DIV_THRESHOLD)
+ ret = clk_cpu_8996_mux_set_parent(&cbfclk->clkr.hw, CBF_DIV_INDEX);
else {
parent = clk_hw_get_parent_by_index(&cbfclk->clkr.hw, CBF_PLL_INDEX);
ret = clk_set_rate(parent->clk, cnd->new_rate);
@@ -676,6 +694,7 @@ static int qcom_cpu_clk_msm8996_driver_remove(struct platform_device *pdev)

static const struct of_device_id qcom_cpu_clk_msm8996_match_table[] = {
{ .compatible = "qcom,msm8996-apcc" },
+ { .compatible = "qcom,msm8996pro-apcc" },
{}
};
MODULE_DEVICE_TABLE(of, qcom_cpu_clk_msm8996_match_table);
--
2.35.1

2022-04-16 03:12:26

by Yassine Oudjana

[permalink] [raw]
Subject: [PATCH RESEND v2 6/9] arm64: dts: qcom: msm8996: Remove MSM8996 Pro speed bins from cluster OPP tables

Now that qcom-cpufreq-nvmem doesn't use SMEM to combine both MSM8996
and MSM8996 Pro speed bins into the same supported-hw bitmask, remove
bits 4,5,6 from all opp-supported-hw in cluster OPPs. These bits will
be placed in a separate device tree for MSM8996 Pro.

Signed-off-by: Yassine Oudjana <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8996.dtsi | 82 +++++++++++++--------------
1 file changed, 41 insertions(+), 41 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 6fb3ef9df05b..5695671bb934 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -142,82 +142,82 @@ cluster0_opp: opp-table-cluster0 {
/* Nominal fmax for now */
opp-307200000 {
opp-hz = /bits/ 64 <307200000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-422400000 {
opp-hz = /bits/ 64 <422400000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-480000000 {
opp-hz = /bits/ 64 <480000000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-556800000 {
opp-hz = /bits/ 64 <556800000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-652800000 {
opp-hz = /bits/ 64 <652800000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-729600000 {
opp-hz = /bits/ 64 <729600000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-844800000 {
opp-hz = /bits/ 64 <844800000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-960000000 {
opp-hz = /bits/ 64 <960000000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1036800000 {
opp-hz = /bits/ 64 <1036800000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1113600000 {
opp-hz = /bits/ 64 <1113600000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1190400000 {
opp-hz = /bits/ 64 <1190400000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1228800000 {
opp-hz = /bits/ 64 <1228800000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1324800000 {
opp-hz = /bits/ 64 <1324800000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1401600000 {
opp-hz = /bits/ 64 <1401600000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1478400000 {
opp-hz = /bits/ 64 <1478400000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1593600000 {
opp-hz = /bits/ 64 <1593600000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
};
@@ -230,127 +230,127 @@ cluster1_opp: opp-table-cluster1 {
/* Nominal fmax for now */
opp-307200000 {
opp-hz = /bits/ 64 <307200000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-403200000 {
opp-hz = /bits/ 64 <403200000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-480000000 {
opp-hz = /bits/ 64 <480000000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-556800000 {
opp-hz = /bits/ 64 <556800000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-652800000 {
opp-hz = /bits/ 64 <652800000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-729600000 {
opp-hz = /bits/ 64 <729600000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-806400000 {
opp-hz = /bits/ 64 <806400000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-883200000 {
opp-hz = /bits/ 64 <883200000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-940800000 {
opp-hz = /bits/ 64 <940800000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1036800000 {
opp-hz = /bits/ 64 <1036800000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1113600000 {
opp-hz = /bits/ 64 <1113600000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1190400000 {
opp-hz = /bits/ 64 <1190400000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1248000000 {
opp-hz = /bits/ 64 <1248000000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1324800000 {
opp-hz = /bits/ 64 <1324800000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1401600000 {
opp-hz = /bits/ 64 <1401600000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1478400000 {
opp-hz = /bits/ 64 <1478400000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1555200000 {
opp-hz = /bits/ 64 <1555200000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1632000000 {
opp-hz = /bits/ 64 <1632000000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1708800000 {
opp-hz = /bits/ 64 <1708800000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1785600000 {
opp-hz = /bits/ 64 <1785600000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1824000000 {
opp-hz = /bits/ 64 <1824000000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1920000000 {
opp-hz = /bits/ 64 <1920000000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-1996800000 {
opp-hz = /bits/ 64 <1996800000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-2073600000 {
opp-hz = /bits/ 64 <2073600000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
opp-2150400000 {
opp-hz = /bits/ 64 <2150400000>;
- opp-supported-hw = <0x77>;
+ opp-supported-hw = <0x7>;
clock-latency-ns = <200000>;
};
};
--
2.35.1

2022-04-16 03:12:25

by Yassine Oudjana

[permalink] [raw]
Subject: [PATCH RESEND v2 4/9] cpufreq: qcom_cpufreq_nvmem: Simplify reading kryo speedbin

MSM8996 and MSM8996 Pro have different OPPs with different dependencies on
CPR and CBF levels. Sharing the same OPP tables will make implementing CPR
and CBF scaling quite difficult, as it will become necessary to use
opp-supported-hw not only to choose CPU OPPs, but to also choose their
required CPR and CBF OPPs which are different on the same CPU OPP between
MSM8996 and MSM8996 Pro. The best solution would be to make a new device
tree for MSM8996 Pro which would override the OPP tables from the existing
MSM8996 device tree.

In preparation for adding a separate device tree for MSM8996 Pro, skip
reading msm-id from smem and just read the speedbin efuse.

Signed-off-by: Yassine Oudjana <[email protected]>
---
drivers/cpufreq/Kconfig.arm | 1 -
drivers/cpufreq/qcom-cpufreq-nvmem.c | 75 +++-------------------------
2 files changed, 6 insertions(+), 70 deletions(-)

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 954749afb5fe..7d9798bc5753 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -154,7 +154,6 @@ config ARM_QCOM_CPUFREQ_NVMEM
tristate "Qualcomm nvmem based CPUFreq"
depends on ARCH_QCOM
depends on QCOM_QFPROM
- depends on QCOM_SMEM
select PM_OPP
help
This adds the CPUFreq driver for Qualcomm Kryo SoC based boards.
diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
index 6dfa86971a75..a2b895a930cb 100644
--- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -9,8 +9,8 @@
* based on the silicon variant in use. Qualcomm Process Voltage Scaling Tables
* defines the voltage and frequency value based on the msm-id in SMEM
* and speedbin blown in the efuse combination.
- * The qcom-cpufreq-nvmem driver reads the msm-id and efuse value from the SoC
- * to provide the OPP framework with required information.
+ * The qcom-cpufreq-nvmem driver reads efuse value from the SoC to provide the
+ * OPP framework with required information.
* This is used to determine the voltage and frequency value for each OPP of
* operating-points-v2 table when it is parsed by the OPP framework.
*/
@@ -27,22 +27,6 @@
#include <linux/pm_domain.h>
#include <linux/pm_opp.h>
#include <linux/slab.h>
-#include <linux/soc/qcom/smem.h>
-
-#define MSM_ID_SMEM 137
-
-enum _msm_id {
- MSM8996V3 = 0xF6ul,
- APQ8096V3 = 0x123ul,
- MSM8996SG = 0x131ul,
- APQ8096SG = 0x138ul,
-};
-
-enum _msm8996_version {
- MSM8996_V3,
- MSM8996_SG,
- NUM_OF_MSM8996_VERSIONS,
-};

struct qcom_cpufreq_drv;

@@ -142,35 +126,6 @@ static void get_krait_bin_format_b(struct device *cpu_dev,
dev_dbg(cpu_dev, "PVS version: %d\n", *pvs_ver);
}

-static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
-{
- size_t len;
- u32 *msm_id;
- enum _msm8996_version version;
-
- msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY, MSM_ID_SMEM, &len);
- if (IS_ERR(msm_id))
- return NUM_OF_MSM8996_VERSIONS;
-
- /* The first 4 bytes are format, next to them is the actual msm-id */
- msm_id++;
-
- switch ((enum _msm_id)*msm_id) {
- case MSM8996V3:
- case APQ8096V3:
- version = MSM8996_V3;
- break;
- case MSM8996SG:
- case APQ8096SG:
- version = MSM8996_SG;
- break;
- default:
- version = NUM_OF_MSM8996_VERSIONS;
- }
-
- return version;
-}
-
static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
struct nvmem_cell *speedbin_nvmem,
char **pvs_name,
@@ -178,30 +133,13 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
{
size_t len;
u8 *speedbin;
- enum _msm8996_version msm8996_version;
*pvs_name = NULL;

- msm8996_version = qcom_cpufreq_get_msm_id();
- if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
- dev_err(cpu_dev, "Not Snapdragon 820/821!");
- return -ENODEV;
- }
-
speedbin = nvmem_cell_read(speedbin_nvmem, &len);
if (IS_ERR(speedbin))
return PTR_ERR(speedbin);

- switch (msm8996_version) {
- case MSM8996_V3:
- drv->versions = 1 << (unsigned int)(*speedbin);
- break;
- case MSM8996_SG:
- drv->versions = 1 << ((unsigned int)(*speedbin) + 4);
- break;
- default:
- BUG();
- break;
- }
+ drv->versions = 1 << (unsigned int)(*speedbin);

kfree(speedbin);
return 0;
@@ -464,10 +402,9 @@ static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
MODULE_DEVICE_TABLE(of, qcom_cpufreq_match_list);

/*
- * Since the driver depends on smem and nvmem drivers, which may
- * return EPROBE_DEFER, all the real activity is done in the probe,
- * which may be defered as well. The init here is only registering
- * the driver and the platform device.
+ * Since the driver depends on the nvmem driver, which may return EPROBE_DEFER,
+ * all the real activity is done in the probe, which may be defered as well.
+ * The init here is only registering the driver and the platform device.
*/
static int __init qcom_cpufreq_init(void)
{
--
2.35.1

2022-04-16 03:12:29

by Yassine Oudjana

[permalink] [raw]
Subject: [PATCH RESEND v2 7/9] dt-bindings: arm: qcom: Add MSM8996 Pro compatible

Add a qcom,msm8996pro compatible and move xiaomi,scorpio to the
same items list as it.

Signed-off-by: Yassine Oudjana <[email protected]>
---
Documentation/devicetree/bindings/arm/qcom.yaml | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 129cdd246223..dcf2e0102857 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -186,7 +186,12 @@ properties:
- sony,kagura-row
- sony,keyaki-row
- xiaomi,gemini
+ - const: qcom,msm8996
+
+ - items:
+ - enum:
- xiaomi,scorpio
+ - const: qcom,msm8996pro
- const: qcom,msm8996

- items:
--
2.35.1

2022-04-16 03:12:35

by Yassine Oudjana

[permalink] [raw]
Subject: [PATCH RESEND v2 8/9] arm64: dts: qcom: msm8996: Add MSM8996 Pro support

Add a new DTSI for MSM8996 Pro (MSM8996SG) with msm-id and CPU/GPU OPPs.
CBF OPPs and CPR parameters will be added to it as well once support for
CBF scaling and CPR is introduced.

Signed-off-by: Yassine Oudjana <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8996pro.dtsi | 281 +++++++++++++++++++++++
1 file changed, 281 insertions(+)
create mode 100644 arch/arm64/boot/dts/qcom/msm8996pro.dtsi

diff --git a/arch/arm64/boot/dts/qcom/msm8996pro.dtsi b/arch/arm64/boot/dts/qcom/msm8996pro.dtsi
new file mode 100644
index 000000000000..8c8dd5614f4d
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8996pro.dtsi
@@ -0,0 +1,281 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2021, Yassine Oudjana <[email protected]>
+ */
+
+#include "msm8996.dtsi"
+
+/*
+ * MSM8996 Pro (also known as MSM8996SG) is a revision of MSM8996 with
+ * different CPU, CBF and GPU frequencies as well as CPR parameters.
+ */
+/delete-node/ &cluster0_opp;
+/delete-node/ &cluster1_opp;
+
+/ {
+ qcom,msm-id = <305 0x10000>;
+
+ cluster0_opp: opp_table0 {
+ compatible = "operating-points-v2-kryo-cpu";
+ nvmem-cells = <&speedbin_efuse>;
+ opp-shared;
+
+ opp-307200000 {
+ opp-hz = /bits/ 64 <307200000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-384000000 {
+ opp-hz = /bits/ 64 <384000000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-460800000 {
+ opp-hz = /bits/ 64 <460800000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-537600000 {
+ opp-hz = /bits/ 64 <537600000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-614400000 {
+ opp-hz = /bits/ 64 <614400000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-691200000 {
+ opp-hz = /bits/ 64 <691200000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-768000000 {
+ opp-hz = /bits/ 64 <768000000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-844800000 {
+ opp-hz = /bits/ 64 <844800000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-902400000 {
+ opp-hz = /bits/ 64 <902400000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-979200000 {
+ opp-hz = /bits/ 64 <979200000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1056000000 {
+ opp-hz = /bits/ 64 <1056000000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1132800000 {
+ opp-hz = /bits/ 64 <1132800000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1209600000 {
+ opp-hz = /bits/ 64 <1209600000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1286400000 {
+ opp-hz = /bits/ 64 <1286400000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1363200000 {
+ opp-hz = /bits/ 64 <1363200000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1440000000 {
+ opp-hz = /bits/ 64 <1440000000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1516800000 {
+ opp-hz = /bits/ 64 <1516800000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1593600000 {
+ opp-hz = /bits/ 64 <1593600000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1996800000 {
+ opp-hz = /bits/ 64 <1996800000>;
+ opp-supported-hw = <0x2>;
+ clock-latency-ns = <200000>;
+ };
+ opp-2188800000 {
+ opp-hz = /bits/ 64 <2188800000>;
+ opp-supported-hw = <0x1>;
+ clock-latency-ns = <200000>;
+ };
+ };
+
+ cluster1_opp: opp_table1 {
+ compatible = "operating-points-v2-kryo-cpu";
+ nvmem-cells = <&speedbin_efuse>;
+ opp-shared;
+
+ opp-307200000 {
+ opp-hz = /bits/ 64 <307200000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-384000000 {
+ opp-hz = /bits/ 64 <384000000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-460800000 {
+ opp-hz = /bits/ 64 <460800000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-537600000 {
+ opp-hz = /bits/ 64 <537600000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-614400000 {
+ opp-hz = /bits/ 64 <614400000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-691200000 {
+ opp-hz = /bits/ 64 <691200000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-748800000 {
+ opp-hz = /bits/ 64 <748800000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-825600000 {
+ opp-hz = /bits/ 64 <825600000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-902400000 {
+ opp-hz = /bits/ 64 <902400000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-979200000 {
+ opp-hz = /bits/ 64 <979200000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1056000000 {
+ opp-hz = /bits/ 64 <1056000000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1132800000 {
+ opp-hz = /bits/ 64 <1132800000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1209600000 {
+ opp-hz = /bits/ 64 <1209600000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1286400000 {
+ opp-hz = /bits/ 64 <1286400000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1363200000 {
+ opp-hz = /bits/ 64 <1363200000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1440000000 {
+ opp-hz = /bits/ 64 <1440000000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1516800000 {
+ opp-hz = /bits/ 64 <1516800000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1593600000 {
+ opp-hz = /bits/ 64 <1593600000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1670400000 {
+ opp-hz = /bits/ 64 <1670400000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1747200000 {
+ opp-hz = /bits/ 64 <1747200000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1824000000 {
+ opp-hz = /bits/ 64 <1824000000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1900800000 {
+ opp-hz = /bits/ 64 <1900800000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <200000>;
+ };
+ opp-1977600000 {
+ opp-hz = /bits/ 64 <1977600000>;
+ opp-supported-hw = <0x3>;
+ clock-latency-ns = <200000>;
+ };
+ opp-2054400000 {
+ opp-hz = /bits/ 64 <2054400000>;
+ opp-supported-hw = <0x3>;
+ clock-latency-ns = <200000>;
+ };
+ opp-2150400000 {
+ opp-hz = /bits/ 64 <2150400000>;
+ opp-supported-hw = <0x3>;
+ clock-latency-ns = <200000>;
+ };
+ opp-2246400000 {
+ opp-hz = /bits/ 64 <2246400000>;
+ opp-supported-hw = <0x1>;
+ clock-latency-ns = <200000>;
+ };
+ opp-2342400000 {
+ opp-hz = /bits/ 64 <2342400000>;
+ opp-supported-hw = <0x1>;
+ clock-latency-ns = <200000>;
+ };
+ };
+};
+
+&gpu_opp_table {
+ /*
+ * All MSM8996 GPU OPPs are available on MSM8996 Pro,
+ * in addition to one:
+ */
+ opp-652800000 {
+ opp-hz = /bits/ 64 <652800000>;
+ opp-supported-hw = <0x1>;
+ };
+};
+
+&kryocc {
+ compatible = "qcom,msm8996pro-apcc";
+};
--
2.35.1

2022-04-16 03:12:51

by Yassine Oudjana

[permalink] [raw]
Subject: [PATCH RESEND v2 9/9] arm64: dts: qcom: msm8996-xiaomi-scorpio: Use MSM8996 Pro

The Xiaomi Mi Note 2 has the MSM8996 Pro SoC. Rename the dts
to match, include msm8996pro.dtsi, and add the qcom,msm8996pro
compatible. To do that, the msm8996.dtsi include in msm8996-xiaomi-common
has to be moved to msm8996-xiaomi-gemini, the only device that needs it
included after this change. The msm-id is also removed as it is now defined
in msm8996pro.dtsi.

Signed-off-by: Yassine Oudjana <[email protected]>
---
arch/arm64/boot/dts/qcom/Makefile | 2 +-
arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi | 3 ---
arch/arm64/boot/dts/qcom/msm8996-xiaomi-gemini.dts | 1 +
...m8996-xiaomi-scorpio.dts => msm8996pro-xiaomi-scorpio.dts} | 4 ++--
4 files changed, 4 insertions(+), 6 deletions(-)
rename arch/arm64/boot/dts/qcom/{msm8996-xiaomi-scorpio.dts => msm8996pro-xiaomi-scorpio.dts} (99%)

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index f9e6343acd03..72b8fcdd9074 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -37,7 +37,7 @@ dtb-$(CONFIG_ARCH_QCOM) += msm8996-sony-xperia-tone-dora.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8996-sony-xperia-tone-kagura.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8996-sony-xperia-tone-keyaki.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8996-xiaomi-gemini.dtb
-dtb-$(CONFIG_ARCH_QCOM) += msm8996-xiaomi-scorpio.dtb
+dtb-$(CONFIG_ARCH_QCOM) += msm8996pro-xiaomi-scorpio.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8998-asus-novago-tp370ql.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8998-fxtec-pro1.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8998-hp-envy-x2.dtb
diff --git a/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi b/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi
index 7a9fcbe9bb31..1bdd3f09f536 100644
--- a/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi
@@ -3,9 +3,6 @@
* Copyright (c) 2020, Yassine Oudjana <[email protected]>
*/

-/dts-v1/;
-
-#include "msm8996.dtsi"
#include "pm8994.dtsi"
#include "pmi8994.dtsi"
#include <dt-bindings/input/input.h>
diff --git a/arch/arm64/boot/dts/qcom/msm8996-xiaomi-gemini.dts b/arch/arm64/boot/dts/qcom/msm8996-xiaomi-gemini.dts
index 34f82e06ef53..e360187109a2 100644
--- a/arch/arm64/boot/dts/qcom/msm8996-xiaomi-gemini.dts
+++ b/arch/arm64/boot/dts/qcom/msm8996-xiaomi-gemini.dts
@@ -5,6 +5,7 @@

/dts-v1/;

+#include "msm8996.dtsi"
#include "msm8996-xiaomi-common.dtsi"
#include <dt-bindings/sound/qcom,q6afe.h>
#include <dt-bindings/sound/qcom,q6asm.h>
diff --git a/arch/arm64/boot/dts/qcom/msm8996-xiaomi-scorpio.dts b/arch/arm64/boot/dts/qcom/msm8996pro-xiaomi-scorpio.dts
similarity index 99%
rename from arch/arm64/boot/dts/qcom/msm8996-xiaomi-scorpio.dts
rename to arch/arm64/boot/dts/qcom/msm8996pro-xiaomi-scorpio.dts
index 27a45ddbb5bd..2028325e1c0f 100644
--- a/arch/arm64/boot/dts/qcom/msm8996-xiaomi-scorpio.dts
+++ b/arch/arm64/boot/dts/qcom/msm8996pro-xiaomi-scorpio.dts
@@ -5,6 +5,7 @@

/dts-v1/;

+#include "msm8996pro.dtsi"
#include "msm8996-xiaomi-common.dtsi"
#include "pmi8996.dtsi"
#include <dt-bindings/sound/qcom,q6afe.h>
@@ -12,9 +13,8 @@

/ {
model = "Xiaomi Mi Note 2";
- compatible = "xiaomi,scorpio", "qcom,msm8996";
+ compatible = "xiaomi,scorpio", "qcom,msm8996pro", "qcom,msm8996";
chassis-type = "handset";
- qcom,msm-id = <305 0x10000>;
qcom,board-id = <34 0>;

chosen {
--
2.35.1

2022-04-19 11:37:32

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 1/9] dt-bindings: clk: qcom: msm8996-apcc: Add CBF

On 16/04/2022 04:56, Yassine Oudjana wrote:
> Add CBF clock and reg.
>
> Signed-off-by: Yassine Oudjana <[email protected]>
> Acked-by: Rob Herring <[email protected]>
> ---
> .../devicetree/bindings/clock/qcom,msm8996-apcc.yaml | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
> index a20cb10636dd..325f8aef53b2 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
> @@ -10,8 +10,8 @@ maintainers:
> - Loic Poulain <[email protected]>
>
> description: |
> - Qualcomm CPU clock controller for MSM8996 CPUs, clock 0 is for Power cluster
> - and clock 1 is for Perf cluster.
> + Qualcomm CPU clock controller for MSM8996 CPUs, clock 0 is for Power cluster,
> + clock 1 is for Perf cluster, and clock 2 is for Coherent bus fabric (CBF).
>
> properties:
> compatible:
> @@ -19,7 +19,9 @@ properties:
> - qcom,msm8996-apcc
>
> reg:
> - maxItems: 1
> + items:
> + - description: Cluster clock registers
> + - description: CBF clock registers

This breaks the ABI (which might be okay or might be not, but was not
mentioned in the commit) and breaks existing DTSes. Please fix them
before this patch.

Best regards,
Krzysztof

2022-04-19 18:12:16

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 1/9] dt-bindings: clk: qcom: msm8996-apcc: Add CBF

On 18/04/2022 21:12, Yassine Oudjana wrote:
>
> On Mon, Apr 18 2022 at 18:04:08 +0200, Krzysztof Kozlowski
> <[email protected]> wrote:
>> On 16/04/2022 04:56, Yassine Oudjana wrote:
>>> Add CBF clock and reg.
>>>
>>> Signed-off-by: Yassine Oudjana <[email protected]>
>>> Acked-by: Rob Herring <[email protected]>
>>> ---
>>> .../devicetree/bindings/clock/qcom,msm8996-apcc.yaml | 10
>>> ++++++----
>>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
>>> b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
>>> index a20cb10636dd..325f8aef53b2 100644
>>> --- a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
>>> +++ b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
>>> @@ -10,8 +10,8 @@ maintainers:
>>> - Loic Poulain <[email protected]>
>>>
>>> description: |
>>> - Qualcomm CPU clock controller for MSM8996 CPUs, clock 0 is for
>>> Power cluster
>>> - and clock 1 is for Perf cluster.
>>> + Qualcomm CPU clock controller for MSM8996 CPUs, clock 0 is for
>>> Power cluster,
>>> + clock 1 is for Perf cluster, and clock 2 is for Coherent bus
>>> fabric (CBF).
>>>
>>> properties:
>>> compatible:
>>> @@ -19,7 +19,9 @@ properties:
>>> - qcom,msm8996-apcc
>>>
>>> reg:
>>> - maxItems: 1
>>> + items:
>>> + - description: Cluster clock registers
>>> + - description: CBF clock registers
>>
>> This breaks the ABI (which might be okay or might be not, but was not
>> mentioned in the commit) and breaks existing DTSes. Please fix them
>> before this patch.
>
> This is only documenting changes made in an earlier patch[1] this
> series depends on,

So this other patch breaks the ABI. Was it accepted? The patch you wrote
here should go together with the clock change.

> and the DTSes are fixed in another patch[2] that
> is also listed as a dependency in the cover letter (both patches
> aren't applied yet). Shouldn't the ABI changes should be mentioned in
> those patches instead?

They should be mentioned in the clock driver or here, because usually
they come together. :)

Best regards,
Krzysztof

2022-04-20 02:06:04

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 3/9] clk: qcom: msm8996-cpu: Add MSM8996 Pro CBF support

On 16/04/2022 04:56, Yassine Oudjana wrote:
> MSM8996 Pro (MSM8996SG) has a /4 divisor on the CBF clock
> instead of /2. This allows it to reach a lower minimum frequency
> of 192000000Hz compared to 307200000Hz on regular MSM8996.
> Add support for setting the CBF clock divisor to /4 for MSM8996 Pro.
>
> Signed-off-by: Yassine Oudjana <[email protected]>

Your From does not match Signed-off-by, here and in all other patches.
Please fix the author in the commits and then send a v3.

Best regards,
Krzysztof

2022-04-21 13:23:42

by Yassine Oudjana

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 1/9] dt-bindings: clk: qcom: msm8996-apcc: Add CBF


On Mon, Apr 18 2022 at 18:04:08 +0200, Krzysztof Kozlowski
<[email protected]> wrote:
> On 16/04/2022 04:56, Yassine Oudjana wrote:
>> Add CBF clock and reg.
>>
>> Signed-off-by: Yassine Oudjana <[email protected]>
>> Acked-by: Rob Herring <[email protected]>
>> ---
>> .../devicetree/bindings/clock/qcom,msm8996-apcc.yaml | 10
>> ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
>> b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
>> index a20cb10636dd..325f8aef53b2 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
>> @@ -10,8 +10,8 @@ maintainers:
>> - Loic Poulain <[email protected]>
>>
>> description: |
>> - Qualcomm CPU clock controller for MSM8996 CPUs, clock 0 is for
>> Power cluster
>> - and clock 1 is for Perf cluster.
>> + Qualcomm CPU clock controller for MSM8996 CPUs, clock 0 is for
>> Power cluster,
>> + clock 1 is for Perf cluster, and clock 2 is for Coherent bus
>> fabric (CBF).
>>
>> properties:
>> compatible:
>> @@ -19,7 +19,9 @@ properties:
>> - qcom,msm8996-apcc
>>
>> reg:
>> - maxItems: 1
>> + items:
>> + - description: Cluster clock registers
>> + - description: CBF clock registers
>
> This breaks the ABI (which might be okay or might be not, but was not
> mentioned in the commit) and breaks existing DTSes. Please fix them
> before this patch.

This is only documenting changes made in an earlier patch[1] this
series depends on, and the DTSes are fixed in another patch[2] that
is also listed as a dependency in the cover letter (both patches
aren't applied yet). Shouldn't the ABI changes should be mentioned in
those patches instead?

[1]
https://lore.kernel.org/linux-arm-msm/[email protected]/
[2]
https://lore.kernel.org/linux-arm-msm/[email protected]/


2022-04-22 07:00:14

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 8/9] arm64: dts: qcom: msm8996: Add MSM8996 Pro support

On 16/04/2022 04:56, Yassine Oudjana wrote:
> Add a new DTSI for MSM8996 Pro (MSM8996SG) with msm-id and CPU/GPU OPPs.
> CBF OPPs and CPR parameters will be added to it as well once support for
> CBF scaling and CPR is introduced.
>

Thank you for your patch. There is something to discuss/improve.

> +
> +#include "msm8996.dtsi"
> +
> +/*
> + * MSM8996 Pro (also known as MSM8996SG) is a revision of MSM8996 with
> + * different CPU, CBF and GPU frequencies as well as CPR parameters.
> + */
> +/delete-node/ &cluster0_opp;
> +/delete-node/ &cluster1_opp;
> +
> +/ {
> + qcom,msm-id = <305 0x10000>;
> +
> + cluster0_opp: opp_table0 {

No underscores in node names. If this comes from original file, should
be fixed in separate commit.


Best regards,
Krzysztof

2022-04-22 19:34:48

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 7/9] dt-bindings: arm: qcom: Add MSM8996 Pro compatible

On 16/04/2022 04:56, Yassine Oudjana wrote:
> Add a qcom,msm8996pro compatible and move xiaomi,scorpio to the
> same items list as it.
>

Acked-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof