Booting v6.8 results in a hang on various IPQ5018 based boards.
Investigating the problem showed that the hang happens when the
clk_alpha_pll_stromer_plus_set_rate() function tries to write
into the PLL_MODE register of the APSS PLL.
Checking the downstream code revealed that it uses [1] stromer
specific operations for IPQ5018, whereas in the current code
the stromer plus specific operations are used.
The ops in the 'ipq_pll_stromer_plus' clock definition can't be
changed since that is needed for IPQ5332, so add a new alpha pll
clock declaration which uses the correct stromer ops and use this
new clock for IPQ5018 to avoid the boot failure.
Also, change pll_type in 'ipq5018_pll_data' to
CLK_ALPHA_PLL_TYPE_STROMER to better reflect that it is a Stromer
PLL and change the apss_ipq_pll_probe() function accordingly.
1. https://git.codelinaro.org/clo/qsdk/oss/kernel/linux-ipq-5.4/-/blob/NHSS.QSDK.12.4/drivers/clk/qcom/apss-ipq5018.c#L67
Cc: [email protected]
Fixes: 50492f929486 ("clk: qcom: apss-ipq-pll: add support for IPQ5018")
Signed-off-by: Gabor Juhos <[email protected]>
---
Changes in v2:
- extend commit description due to the changes
- add a comment about why CLK_ALPHA_PLL_TYPE_STROMER_PLUS register offsets
are used
- constify hw clock init data (Stephen)
- change pll_type in ipq5018_pll_data to CLK_ALPHA_PLL_TYPE_STROMER (Konrad)
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Based on v6.8.
---
drivers/clk/qcom/apss-ipq-pll.c | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
index 678b805f13d45..dfffec2f06ae7 100644
--- a/drivers/clk/qcom/apss-ipq-pll.c
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -55,6 +55,29 @@ static struct clk_alpha_pll ipq_pll_huayra = {
},
};
+static struct clk_alpha_pll ipq_pll_stromer = {
+ .offset = 0x0,
+ /*
+ * Reuse CLK_ALPHA_PLL_TYPE_STROMER_PLUS register offsets.
+ * Although this is a bit confusing, but the offset values
+ * are correct nevertheless.
+ */
+ .regs = ipq_pll_offsets[CLK_ALPHA_PLL_TYPE_STROMER_PLUS],
+ .flags = SUPPORTS_DYNAMIC_UPDATE,
+ .clkr = {
+ .enable_reg = 0x0,
+ .enable_mask = BIT(0),
+ .hw.init = &(const struct clk_init_data) {
+ .name = "a53pll",
+ .parent_data = &(const struct clk_parent_data) {
+ .fw_name = "xo",
+ },
+ .num_parents = 1,
+ .ops = &clk_alpha_pll_stromer_ops,
+ },
+ },
+};
+
static struct clk_alpha_pll ipq_pll_stromer_plus = {
.offset = 0x0,
.regs = ipq_pll_offsets[CLK_ALPHA_PLL_TYPE_STROMER_PLUS],
@@ -144,8 +167,8 @@ struct apss_pll_data {
};
static const struct apss_pll_data ipq5018_pll_data = {
- .pll_type = CLK_ALPHA_PLL_TYPE_STROMER_PLUS,
- .pll = &ipq_pll_stromer_plus,
+ .pll_type = CLK_ALPHA_PLL_TYPE_STROMER,
+ .pll = &ipq_pll_stromer,
.pll_config = &ipq5018_pll_config,
};
@@ -203,7 +226,8 @@ static int apss_ipq_pll_probe(struct platform_device *pdev)
if (data->pll_type == CLK_ALPHA_PLL_TYPE_HUAYRA)
clk_alpha_pll_configure(data->pll, regmap, data->pll_config);
- else if (data->pll_type == CLK_ALPHA_PLL_TYPE_STROMER_PLUS)
+ else if (data->pll_type == CLK_ALPHA_PLL_TYPE_STROMER ||
+ data->pll_type == CLK_ALPHA_PLL_TYPE_STROMER_PLUS)
clk_stromer_pll_configure(data->pll, regmap, data->pll_config);
ret = devm_clk_register_regmap(dev, &data->pll->clkr);
---
base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
change-id: 20240311-apss-ipq-pll-ipq5018-hang-74d9a8f47136
Best regards,
--
Gabor Juhos <[email protected]>
On 3/15/2024 9:46 PM, Gabor Juhos wrote:
> Booting v6.8 results in a hang on various IPQ5018 based boards.
> Investigating the problem showed that the hang happens when the
> clk_alpha_pll_stromer_plus_set_rate() function tries to write
> into the PLL_MODE register of the APSS PLL.
>
> Checking the downstream code revealed that it uses [1] stromer
> specific operations for IPQ5018, whereas in the current code
> the stromer plus specific operations are used.
>
> The ops in the 'ipq_pll_stromer_plus' clock definition can't be
> changed since that is needed for IPQ5332, so add a new alpha pll
> clock declaration which uses the correct stromer ops and use this
> new clock for IPQ5018 to avoid the boot failure.
>
> Also, change pll_type in 'ipq5018_pll_data' to
> CLK_ALPHA_PLL_TYPE_STROMER to better reflect that it is a Stromer
> PLL and change the apss_ipq_pll_probe() function accordingly.
>
> 1. https://git.codelinaro.org/clo/qsdk/oss/kernel/linux-ipq-5.4/-/blob/NHSS.QSDK.12.4/drivers/clk/qcom/apss-ipq5018.c#L67
>
> Cc: [email protected]
> Fixes: 50492f929486 ("clk: qcom: apss-ipq-pll: add support for IPQ5018")
> Signed-off-by: Gabor Juhos <[email protected]>
> ---
> Changes in v2:
> - extend commit description due to the changes
> - add a comment about why CLK_ALPHA_PLL_TYPE_STROMER_PLUS register offsets
> are used
> - constify hw clock init data (Stephen)
> - change pll_type in ipq5018_pll_data to CLK_ALPHA_PLL_TYPE_STROMER (Konrad)
> - Link to v1: https://lore.kernel.org/r/[email protected]
I don't see a reason why my tags are dropped, nevertheless
Tested-by: Kathiravan Thirumoorthy <[email protected]>
Reviewed-by: Kathiravan Thirumoorthy <[email protected]>
> ---
> Based on v6.8.
> ---
> drivers/clk/qcom/apss-ipq-pll.c | 30 +++++++++++++++++++++++++++---
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
> index 678b805f13d45..dfffec2f06ae7 100644
> --- a/drivers/clk/qcom/apss-ipq-pll.c
> +++ b/drivers/clk/qcom/apss-ipq-pll.c
> @@ -55,6 +55,29 @@ static struct clk_alpha_pll ipq_pll_huayra = {
> },
> };
>
> +static struct clk_alpha_pll ipq_pll_stromer = {
> + .offset = 0x0,
> + /*
> + * Reuse CLK_ALPHA_PLL_TYPE_STROMER_PLUS register offsets.
> + * Although this is a bit confusing, but the offset values
> + * are correct nevertheless.
> + */
> + .regs = ipq_pll_offsets[CLK_ALPHA_PLL_TYPE_STROMER_PLUS],
> + .flags = SUPPORTS_DYNAMIC_UPDATE,
> + .clkr = {
> + .enable_reg = 0x0,
> + .enable_mask = BIT(0),
> + .hw.init = &(const struct clk_init_data) {
> + .name = "a53pll",
> + .parent_data = &(const struct clk_parent_data) {
> + .fw_name = "xo",
> + },
> + .num_parents = 1,
> + .ops = &clk_alpha_pll_stromer_ops,
> + },
> + },
> +};
> +
> static struct clk_alpha_pll ipq_pll_stromer_plus = {
> .offset = 0x0,
> .regs = ipq_pll_offsets[CLK_ALPHA_PLL_TYPE_STROMER_PLUS],
> @@ -144,8 +167,8 @@ struct apss_pll_data {
> };
>
> static const struct apss_pll_data ipq5018_pll_data = {
> - .pll_type = CLK_ALPHA_PLL_TYPE_STROMER_PLUS,
> - .pll = &ipq_pll_stromer_plus,
> + .pll_type = CLK_ALPHA_PLL_TYPE_STROMER,
> + .pll = &ipq_pll_stromer,
> .pll_config = &ipq5018_pll_config,
> };
>
> @@ -203,7 +226,8 @@ static int apss_ipq_pll_probe(struct platform_device *pdev)
>
> if (data->pll_type == CLK_ALPHA_PLL_TYPE_HUAYRA)
> clk_alpha_pll_configure(data->pll, regmap, data->pll_config);
> - else if (data->pll_type == CLK_ALPHA_PLL_TYPE_STROMER_PLUS)
> + else if (data->pll_type == CLK_ALPHA_PLL_TYPE_STROMER ||
> + data->pll_type == CLK_ALPHA_PLL_TYPE_STROMER_PLUS)
> clk_stromer_pll_configure(data->pll, regmap, data->pll_config);
>
> ret = devm_clk_register_regmap(dev, &data->pll->clkr);
>
> ---
> base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
> change-id: 20240311-apss-ipq-pll-ipq5018-hang-74d9a8f47136
>
> Best regards,
2024. 03. 18. 6:39 keltezéssel, Kathiravan Thirumoorthy írta:
..
>> Changes in v2:
>> - extend commit description due to the changes
>> - add a comment about why CLK_ALPHA_PLL_TYPE_STROMER_PLUS register offsets
>> are used
>> - constify hw clock init data (Stephen)
>> - change pll_type in ipq5018_pll_data to CLK_ALPHA_PLL_TYPE_STROMER (Konrad)
>> - Link to v1:
>> https://lore.kernel.org/r/[email protected]
>
>
> I don't see a reason why my tags are dropped, nevertheless
Sorry, that is my fault. Since I have modified the patch, I was not sure that
the tags are applicable to the new version or not.
> Tested-by: Kathiravan Thirumoorthy <[email protected]>
> Reviewed-by: Kathiravan Thirumoorthy <[email protected]>
Thanks!
-Gabor
On Fri, 15 Mar 2024 17:16:41 +0100, Gabor Juhos wrote:
> Booting v6.8 results in a hang on various IPQ5018 based boards.
> Investigating the problem showed that the hang happens when the
> clk_alpha_pll_stromer_plus_set_rate() function tries to write
> into the PLL_MODE register of the APSS PLL.
>
> Checking the downstream code revealed that it uses [1] stromer
> specific operations for IPQ5018, whereas in the current code
> the stromer plus specific operations are used.
>
> [...]
Applied, thanks!
[1/1] clk: qcom: apss-ipq-pll: use stromer ops for IPQ5018 to fix boot failure
commit: 5fce38e2a1a97900989d9fedebcf5a4dacdaee30
Best regards,
--
Bjorn Andersson <[email protected]>