On Wed, 12 Feb 2025 at 19:15, Taniya Das <quic_tdas@xxxxxxxxxxx> wrote:
>
>
>
> On 2/12/2025 4:39 PM, Dmitry Baryshkov wrote:
> > On Wed, Feb 12, 2025 at 01:52:20PM +0530, Taniya Das wrote:
> >> On the QCM6490 boards the LPASS firmware controls the complete clock
> >> controller functionalities. But the LPASS resets are required to be
> >> controlled from the high level OS. The Audio SW driver should be able to
> >> assert/deassert the audio resets as required. Thus in clock driver add
> >> support for the resets.
> >>
> >> Signed-off-by: Taniya Das <quic_tdas@xxxxxxxxxxx>
> >> ---
> >> drivers/clk/qcom/lpassaudiocc-sc7280.c | 23 +++++++++++++++++++----
> >> 1 file changed, 19 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> >> index 45e7264770866f929a3f4663c477330f0bf7aa84..b6439308926371891cc5f9a5e0d4e8393641560d 100644
> >> --- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
> >> +++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> >> @@ -1,6 +1,7 @@
> >> // SPDX-License-Identifier: GPL-2.0-only
> >> /*
> >> * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> >> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> >> */
> >>
> >> #include <linux/clk-provider.h>
> >> @@ -713,14 +714,24 @@ static const struct qcom_reset_map lpass_audio_cc_sc7280_resets[] = {
> >> [LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
> >> };
> >>
> >> +static const struct regmap_config lpass_audio_cc_sc7280_reset_regmap_config = {
> >> + .name = "lpassaudio_cc_reset",
> >> + .reg_bits = 32,
> >> + .reg_stride = 4,
> >> + .val_bits = 32,
> >> + .fast_io = true,
> >> + .max_register = 0xc8,
> >> +};
> >> +
> >> static const struct qcom_cc_desc lpass_audio_cc_reset_sc7280_desc = {
> >> - .config = &lpass_audio_cc_sc7280_regmap_config,
> >> + .config = &lpass_audio_cc_sc7280_reset_regmap_config,
> >> .resets = lpass_audio_cc_sc7280_resets,
> >> .num_resets = ARRAY_SIZE(lpass_audio_cc_sc7280_resets),
> >> };
> >>
> >> static const struct of_device_id lpass_audio_cc_sc7280_match_table[] = {
> >> - { .compatible = "qcom,sc7280-lpassaudiocc" },
> >> + { .compatible = "qcom,qcm6490-lpassaudiocc", .data = &lpass_audio_cc_reset_sc7280_desc },
> >> + { .compatible = "qcom,sc7280-lpassaudiocc", .data = &lpass_audio_cc_sc7280_desc },
> >> { }
> >> };
> >> MODULE_DEVICE_TABLE(of, lpass_audio_cc_sc7280_match_table);
> >> @@ -752,13 +763,17 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
> >> struct regmap *regmap;
> >> int ret;
> >>
> >> + desc = device_get_match_data(&pdev->dev);
> >> +
> >> + if (desc->num_resets)
> >> + return qcom_cc_probe_by_index(pdev, 1, desc);
> >
> > Won't this break SC7280 support by causing an early return?
> >
>
> The resets are not defined for SC7280.
> static const struct qcom_cc_desc lpass_audio_cc_sc7280_desc = {
> .config = &lpass_audio_cc_sc7280_regmap_config,
> .clks = lpass_audio_cc_sc7280_clocks,
> .num_clks = ARRAY_SIZE(lpass_audio_cc_sc7280_clocks),
> };
>
> The reset get registered for SC7280 after the clocks are registered.
> qcom_cc_probe_by_index(pdev, 1, &lpass_audio_cc_reset_sc7280_desc);
Could you please make this condition more obvious and error-prone
rather than checking one particular non-obvious property?
>
> >> +
> >> ret = lpass_audio_setup_runtime_pm(pdev);
> >> if (ret)
> >> return ret;
> >>
> >> lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc";
> >> lpass_audio_cc_sc7280_regmap_config.max_register = 0x2f000;
> >> - desc = &lpass_audio_cc_sc7280_desc;
> >>
> >> regmap = qcom_cc_map(pdev, desc);
> >> if (IS_ERR(regmap)) {
> >> @@ -772,7 +787,7 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
> >> regmap_write(regmap, 0x4, 0x3b);
> >> regmap_write(regmap, 0x8, 0xff05);
> >>
> >> - ret = qcom_cc_really_probe(&pdev->dev, &lpass_audio_cc_sc7280_desc, regmap);
> >> + ret = qcom_cc_really_probe(&pdev->dev, desc, regmap);
> >> if (ret) {
> >> dev_err(&pdev->dev, "Failed to register LPASS AUDIO CC clocks\n");
> >> goto exit;
> >>
> >> --
> >> 2.45.2
> >>
> >
>
--
With best wishes
Dmitry
