From 79703e7f641f6af81a13f4d7dedfeaae8bdd0021 Mon Sep 17 00:00:00 2001 From: Taniya Das Date: Thu, 13 Jan 2022 15:56:33 +0530 Subject: [PATCH 1/4] clk: qcom: clk-branch: Add support for Mem branch mem ops CBCRs with memories need an update for memory before enabling the clock. Change-Id: I4d6856640c209471d057a91938c1d51014463550 Signed-off-by: Taniya Das --- drivers/clk/qcom/clk-branch.c | 60 +++++++++++++++++++++++++++++++++++ drivers/clk/qcom/clk-branch.h | 4 +++ drivers/clk/qcom/clk-opp.h | 2 +- 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/drivers/clk/qcom/clk-branch.c b/drivers/clk/qcom/clk-branch.c index 8777eab86b01..4f967084137e 100644 --- a/drivers/clk/qcom/clk-branch.c +++ b/drivers/clk/qcom/clk-branch.c @@ -184,6 +184,12 @@ static void clk_branch2_list_registers(struct seq_file *f, struct clk_hw *hw) {"APSS_SLEEP_VOTE", 0x4}, }; + static struct clk_register_data data2[] = { + {"MEM_ENABLE", 0x0}, + {"MEM_ENABLE_ACK", 0x0}, + {"MEM_ENABLE_ACK_MASK", 0x0}, + }; + size = ARRAY_SIZE(data); for (i = 0; i < size; i++) { @@ -204,6 +210,18 @@ static void clk_branch2_list_registers(struct seq_file *f, struct clk_hw *hw) } } } + + if (br->mem_enable_reg && br->mem_ack_reg) { + regmap_read(br->clkr.regmap, br->mem_enable_reg + + data2[0].offset, &val); + clock_debug_output(f, "%20s: 0x%.8x\n", data2[0].name, val); + + regmap_read(br->clkr.regmap, br->mem_ack_reg + + data2[1].offset, &val); + clock_debug_output(f, "%20s: 0x%.8x\n", data2[1].name, val); + clock_debug_output(f, "%20s: 0x%.8x\n", data2[2].name, + br->mem_enable_ack_bit); + } } static int clk_branch2_set_flags(struct clk_hw *hw, unsigned long flags) @@ -260,6 +278,39 @@ static int clk_branch2_init(struct clk_hw *hw) return 0; } +static int clk_branch2_mem_enable(struct clk_hw *hw) +{ + struct clk_branch *br = to_clk_branch(hw); + u32 val; + int count = 200; + + regmap_update_bits(br->clkr.regmap, br->mem_enable_reg, + br->mem_enable_ack_bit, br->mem_enable_ack_bit); + + regmap_read(br->clkr.regmap, br->mem_ack_reg, &val); + + pr_debug("%s Val 0x%x\n", __func__, val); + while (count-- > 0) { + if (val & br->mem_enable_ack_bit) { + pr_debug("%s Val 0x%x\n", __func__, val); + return clk_branch2_enable(hw); + } + udelay(1); + regmap_read(br->clkr.regmap, br->mem_ack_reg, &val); + } + + return -EBUSY; +} + +static void clk_branch2_mem_disable(struct clk_hw *hw) +{ + struct clk_branch *br = to_clk_branch(hw); + + regmap_update_bits(br->clkr.regmap, br->mem_enable_reg, + br->mem_enable_ack_bit, 0); + return clk_branch2_disable(hw); +} + const struct clk_ops clk_branch2_ops = { .prepare = clk_prepare_regmap, .unprepare = clk_unprepare_regmap, @@ -290,6 +341,15 @@ const struct clk_ops clk_branch2_force_off_ops = { }; EXPORT_SYMBOL(clk_branch2_force_off_ops); +const struct clk_ops clk_branch2_mem_ops = { + .enable = clk_branch2_mem_enable, + .disable = clk_branch2_mem_disable, + .is_enabled = clk_is_enabled_regmap, + .init = clk_branch2_init, + .debug_init = clk_branch_debug_init, +}; +EXPORT_SYMBOL(clk_branch2_mem_ops); + static unsigned long clk_branch2_hw_ctl_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) { diff --git a/drivers/clk/qcom/clk-branch.h b/drivers/clk/qcom/clk-branch.h index eeabc1573bec..2657c0ab7486 100644 --- a/drivers/clk/qcom/clk-branch.h +++ b/drivers/clk/qcom/clk-branch.h @@ -23,8 +23,11 @@ struct clk_branch { u32 hwcg_reg; u32 halt_reg; + u32 mem_enable_reg; + u32 mem_ack_reg; u8 hwcg_bit; u8 halt_bit; + u8 mem_enable_ack_bit; u8 halt_check; #define BRANCH_VOTED BIT(7) /* Delay on disable */ #define BRANCH_HALT 0 /* pol: 1 = halt */ @@ -43,6 +46,7 @@ extern const struct clk_ops clk_branch2_hw_ctl_ops; extern const struct clk_ops clk_branch_simple_ops; extern const struct clk_ops clk_branch2_aon_ops; extern const struct clk_ops clk_branch2_force_off_ops; +extern const struct clk_ops clk_branch2_mem_ops; #define to_clk_branch(_hw) \ container_of(to_clk_regmap(_hw), struct clk_branch, clkr) diff --git a/drivers/clk/qcom/clk-opp.h b/drivers/clk/qcom/clk-opp.h index aba1ee18bff5..eaf25b853073 100644 --- a/drivers/clk/qcom/clk-opp.h +++ b/drivers/clk/qcom/clk-opp.h @@ -7,7 +7,7 @@ void clk_hw_populate_clock_opp_table(struct device_node *np, struct clk_hw *hw); -#define MAX_LEN_OPP_HANDLE 50 +#define MAX_LEN_OPP_HANDLE 100 #define LEN_OPP_HANDLE 16 #endif From 7093f030929f50e2bd7a165b2914aaf2d53ff72b Mon Sep 17 00:00:00 2001 From: Mike Tipton Date: Mon, 6 Jun 2022 16:37:22 -0700 Subject: [PATCH 2/4] clk: qcom: gdsc-regulator: Add BW voting support When certain GDSCs are enabled they cause their core to permanently assert active signals to RPMh. These active signals will prevent NOCs from powering down. So if BW votes are removed prior to disabling the GDSC, then BCM will get stuck in the power down sequence and rpmh driver timeouts. Prevent this by placing minimum BW votes whenever the GDSC is enabled. Change-Id: Iadd0bac37a6d04fd87c507fc7eee23e203891a6c Signed-off-by: Mike Tipton --- drivers/clk/qcom/gdsc-regulator.c | 49 +++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/drivers/clk/qcom/gdsc-regulator.c b/drivers/clk/qcom/gdsc-regulator.c index d1f2130ab049..7c9d80dacf2c 100644 --- a/drivers/clk/qcom/gdsc-regulator.c +++ b/drivers/clk/qcom/gdsc-regulator.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright (c) 2016-2021, The Linux Foundation. All rights reserved. + * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved. */ #include @@ -23,6 +24,7 @@ #include #include #include +#include #include "../../regulator/internal.h" #include "gdsc-debug.h" @@ -74,6 +76,7 @@ struct gdsc { struct mbox_client mbox_client; struct mbox_chan *mbox; struct reset_control **reset_clocks; + struct icc_path **paths; bool toggle_logic; bool retain_ff_enable; bool resets_asserted; @@ -88,6 +91,7 @@ struct gdsc { int reset_count; int root_clk_idx; int sw_reset_count; + int path_count; u32 gds_timeout; bool skip_disable_before_enable; bool skip_disable; @@ -258,6 +262,14 @@ static int gdsc_enable(struct regulator_dev *rdev) } if (sc->toggle_logic) { + for (i = 0; i < sc->path_count; i++) { + ret = icc_set_bw(sc->paths[i], 1, 1); + if (ret) { + dev_err(&rdev->dev, "Failed to vote BW for %d, ret=%d\n", i, ret); + return ret; + } + } + if (sc->sw_reset_count) { for (i = 0; i < sc->sw_reset_count; i++) regmap_set_bits(sc->sw_resets[i], REG_OFFSET, @@ -471,6 +483,13 @@ static int gdsc_disable(struct regulator_dev *rdev) regmap_write(sc->domain_addr, REG_OFFSET, regval); } + for (i = 0; i < sc->path_count; i++) { + ret = icc_set_bw(sc->paths[i], 0, 0); + if (ret) { + dev_err(&rdev->dev, "Failed to unvote BW for %d: %d\n", i, ret); + return ret; + } + } } else { for (i = sc->reset_count - 1; i >= 0; i--) reset_control_assert(sc->reset_clocks[i]); @@ -747,6 +766,16 @@ static int gdsc_parse_dt_data(struct gdsc *sc, struct device *dev, return sc->clock_count; } + sc->path_count = of_property_count_strings(dev->of_node, + "interconnect-names"); + if (sc->path_count == -EINVAL) { + sc->path_count = 0; + } else if (sc->path_count < 0) { + dev_err(dev, "Failed to get interconnect names, ret=%d\n", + sc->path_count); + return sc->path_count; + } + sc->root_en = of_property_read_bool(dev->of_node, "qcom,enable-root-clk"); sc->force_root_en = of_property_read_bool(dev->of_node, @@ -872,6 +901,26 @@ static int gdsc_get_resources(struct gdsc *sc, struct platform_device *pdev) sc->root_clk_idx = i; } + sc->paths = devm_kcalloc(dev, sc->path_count, sizeof(*sc->paths), GFP_KERNEL); + if (sc->path_count && !sc->paths) + return -ENOMEM; + + for (i = 0; i < sc->path_count; i++) { + const char *name; + + of_property_read_string_index(dev->of_node, "interconnect-names", i, + &name); + + sc->paths[i] = of_icc_get(dev, name); + if (IS_ERR(sc->paths[i])) { + ret = PTR_ERR(sc->paths[i]); + if (ret != -EPROBE_DEFER) + dev_err(dev, "Failed to get path %s, ret=%d\n", + name, ret); + return ret; + } + } + if ((sc->root_en || sc->force_root_en) && (sc->root_clk_idx == -1)) { dev_err(dev, "Failed to get root clock name\n"); return -EINVAL; From 8d62f9855dc161f3fba658051af6c458bec2a2d5 Mon Sep 17 00:00:00 2001 From: Vivek Aknurwar Date: Fri, 15 Apr 2022 09:20:36 -0700 Subject: [PATCH 3/4] clk: qcom: Add gdsc_time trace Add gdsc_time trace debugfs file to log gdsc state when they are enabled/disabled. This trace also logs time taken for each gdsc enable/disable call. Also swap gdscr_status enum value to align gdsc enable/disable trace as 1/0 respectively. Change-Id: Ic57f7a0fc44df890e6418d257fe76dafacab55e9 Signed-off-by: Vivek Aknurwar --- drivers/clk/qcom/gdsc-regulator.c | 12 ++++++-- drivers/clk/qcom/trace-gdsc.h | 49 +++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 drivers/clk/qcom/trace-gdsc.h diff --git a/drivers/clk/qcom/gdsc-regulator.c b/drivers/clk/qcom/gdsc-regulator.c index 7c9d80dacf2c..60f4b7695a47 100644 --- a/drivers/clk/qcom/gdsc-regulator.c +++ b/drivers/clk/qcom/gdsc-regulator.c @@ -29,6 +29,9 @@ #include "../../regulator/internal.h" #include "gdsc-debug.h" +#define CREATE_TRACE_POINTS +#include "trace-gdsc.h" + /* GDSCR */ #define PWR_ON_MASK BIT(31) #define CLK_DIS_WAIT_MASK (0xF << 12) @@ -100,8 +103,8 @@ struct gdsc { }; enum gdscr_status { - ENABLED, DISABLED, + ENABLED, }; static inline u32 gdsc_mb(struct gdsc *gds) @@ -148,8 +151,11 @@ static int poll_gdsc_status(struct gdsc *sc, enum gdscr_status status) break; } - if (val) + if (val) { + trace_gdsc_time(sc->rdesc.name, status, + sc->gds_timeout - count, 0); return 0; + } /* * There is no guarantee about the delay needed for the enable * bit in the GDSCR to be set or reset after the GDSC state @@ -160,6 +166,8 @@ static int poll_gdsc_status(struct gdsc *sc, enum gdscr_status status) udelay(1); } + trace_gdsc_time(sc->rdesc.name, status, + sc->gds_timeout - count, 1); return -ETIMEDOUT; } diff --git a/drivers/clk/qcom/trace-gdsc.h b/drivers/clk/qcom/trace-gdsc.h new file mode 100644 index 000000000000..e5314df67d50 --- /dev/null +++ b/drivers/clk/qcom/trace-gdsc.h @@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved. + */ + +#undef TRACE_SYSTEM +#define TRACE_SYSTEM clk_gdsc + +#if !defined(_TRACE_CLOCK_GDSC_H) || defined(TRACE_HEADER_MULTI_READ) +#define _TRACE_CLOCK_GDSC + +#include + +TRACE_EVENT(gdsc_time, + + TP_PROTO(const char *name, u32 enabling, u32 time_us, u32 timed_out), + + TP_ARGS(name, enabling, time_us, timed_out), + + TP_STRUCT__entry( + __string(name, name) + __field(unsigned int, enabling) + __field(unsigned int, time_us) + __field(unsigned int, timed_out) + ), + + TP_fast_assign( + __assign_str(name, name); + __entry->enabling = enabling; + __entry->time_us = time_us; + __entry->timed_out = timed_out; + ), + + TP_printk("%s enabling:%d time_us:%d timed_out:%d", + __get_str(name), __entry->enabling, + __entry->time_us, __entry->timed_out) +); + +#endif /* _TRACE_CLOCK_GDSC */ + +/* This part must be outside protection */ + +#undef TRACE_INCLUDE_PATH +#define TRACE_INCLUDE_PATH . + +#undef TRACE_INCLUDE_FILE +#define TRACE_INCLUDE_FILE trace-gdsc + +#include From 8fff5a3f7a3f85752a99a616aaae9cfad7fb8267 Mon Sep 17 00:00:00 2001 From: Mike Tipton Date: Tue, 3 May 2022 11:20:22 -0700 Subject: [PATCH 4/4] clk: qcom: clk-alpha-pll: Don't round up alpha value in HW PLLs with a narrow ALPHA can't hit all frequencies precisely and may be under by a few hundred Hz. When this happens we currently increment ALPHA by 1 in round_rate() to push the frequency slightly above the requested rate. However, when clk_set_rate() is called on a child of the PLL, then the final ALPHA will be ALPHA+N where N depends on the number of children in the chain. This is because clk_calc_new_rates() calls round_rate() recursively for each parent in the chain, each time using the previous iteration's ALPHA+1 based frequency. This results in a higher frequency than intended. The overall frequency delta is small and generally not a functional issue, but it's different than the HW recommendations and also causes certain frequency validation tests to fail. Additionally, this causes clk_propagate_rate_change() to short-circuit before notifying all children in some cases. If clk_set_rate() on a child results in a PLL frequency of ALPHA+N but the PLL is already configured to this rate, then clk_calc_new_rates() will return the PLL as the top clock, even though pll->rate == pll->new_rate. This causes clk_propagate_rate_change() to return immediately. This happens because clk_calc_new_rates() is recursively called for the parent whenever best_parent_rate != parent->rate. And since ALPHA isn't incremented to its final ALPHA+N value until the final parent iteration, then best_parent_rate is slightly lower than parent->rate. Returning early from clk_propagate_rate_change() can result in undervoltage, since we vote for increased voltage in the pre_rate_change() callback. To fix these issues, don't round up by incrementing the HW ALPHA value. Instead, keep the PLL configured to the slightly lower frequency and report its frequency as the rounded up version. Change-Id: I9aa7c1269735c07ef2524b08306edb6427af3629 Signed-off-by: Mike Tipton --- drivers/clk/qcom/clk-alpha-pll.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c index 3df042b60a51..9b12b174da48 100644 --- a/drivers/clk/qcom/clk-alpha-pll.c +++ b/drivers/clk/qcom/clk-alpha-pll.c @@ -542,7 +542,17 @@ static void clk_alpha_pll_disable(struct clk_hw *hw) static unsigned long alpha_pll_calc_rate(u64 prate, u32 l, u32 a, u32 alpha_width) { - return (prate * l) + ((prate * a) >> ALPHA_SHIFT(alpha_width)); + unsigned long rate; + + rate = (prate * l) + ((prate * a) >> ALPHA_SHIFT(alpha_width)); + + /* + * PLLs with narrow ALPHA (e.g. 16 bits) aren't able to hit all + * frequencies precisely and may be under by a few hundred Hz. Round to + * the nearest KHz to avoid reporting strange, slightly lower than + * requested frequencies. The small delta has no functional impact. + */ + return roundup(rate, 1000); } static unsigned long @@ -564,10 +574,7 @@ alpha_pll_round_rate(unsigned long rate, unsigned long prate, u32 *l, u64 *a, /* Upper ALPHA_BITWIDTH bits of Alpha */ quotient = remainder << ALPHA_SHIFT(alpha_width); - remainder = do_div(quotient, prate); - - if (remainder) - quotient++; + do_div(quotient, prate); *a = quotient; return alpha_pll_calc_rate(prate, *l, *a, alpha_width); @@ -749,7 +756,7 @@ alpha_huayra_pll_calc_rate(u64 prate, u32 l, u32 a) if (a >= BIT(PLL_HUAYRA_ALPHA_WIDTH - 1)) l -= 1; - return (prate * l) + (prate * a >> PLL_HUAYRA_ALPHA_WIDTH); + return alpha_pll_calc_rate(prate, l, a, PLL_HUAYRA_ALPHA_WIDTH); } static unsigned long @@ -769,10 +776,7 @@ alpha_huayra_pll_round_rate(unsigned long rate, unsigned long prate, } quotient = remainder << PLL_HUAYRA_ALPHA_WIDTH; - remainder = do_div(quotient, prate); - - if (remainder) - quotient++; + do_div(quotient, prate); /* * alpha_val should be in two’s complement number in the range