| Message ID | 20240816-qcm6490-lpass-reset-v1-2-a11f33cad3c5@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Update LPASS Audio clock driver for QCM6490 board | expand |
On 16/08/2024 10:32, 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@quicinc.com> > --- > 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 45e726477086..b64393089263 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 }, That's odd to see sc7280 reset added for qcm6490, but not used fot sc7280 at all. Didn't you mean here lpass_audio_cc_qcm6409_desc? Best regards, Krzysztof
On 8/17/2024 2:55 PM, Krzysztof Kozlowski wrote: > On 16/08/2024 10:32, 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@quicinc.com> >> --- >> 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 45e726477086..b64393089263 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 }, > > That's odd to see sc7280 reset added for qcm6490, but not used fot > sc7280 at all. Didn't you mean here lpass_audio_cc_qcm6409_desc? > > The resets descriptor(lpass_audio_cc_reset_sc7280_desc) is not part of the global clock descriptor(lpass_cc_sc7280_desc) as these are part of different regmaps. On a non-QCM6490(SC7280) boards the resets are registered after the global descriptor is registered. But on QCM6490 board we need to register only the reset descriptor and no clocks are to be handled/registered and thus passed the match data for QCM6490 boards only. > Best regards, > Krzysztof >
On 13/09/2024 07:31, Taniya Das wrote: > > > On 8/17/2024 2:55 PM, Krzysztof Kozlowski wrote: >> On 16/08/2024 10:32, 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@quicinc.com> >>> --- >>> 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 45e726477086..b64393089263 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 }, >> >> That's odd to see sc7280 reset added for qcm6490, but not used fot >> sc7280 at all. Didn't you mean here lpass_audio_cc_qcm6409_desc? >> >> > The resets descriptor(lpass_audio_cc_reset_sc7280_desc) is not part of > the global clock descriptor(lpass_cc_sc7280_desc) as these are part of > different regmaps. > > On a non-QCM6490(SC7280) boards the resets are registered after the > global descriptor is registered. > > But on QCM6490 board we need to register only the reset descriptor and > no clocks are to be handled/registered and thus passed the match data > for QCM6490 boards only. Yeah, but why this is sc7280? Best regards, Krzysztof
On Mon, Sep 16, 2024 at 10:33:21AM GMT, Krzysztof Kozlowski wrote: > On 13/09/2024 07:31, Taniya Das wrote: > > > > > > On 8/17/2024 2:55 PM, Krzysztof Kozlowski wrote: > >> On 16/08/2024 10:32, 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@quicinc.com> > >>> --- > >>> 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 45e726477086..b64393089263 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 }, > >> > >> That's odd to see sc7280 reset added for qcm6490, but not used fot > >> sc7280 at all. Didn't you mean here lpass_audio_cc_qcm6409_desc? > >> > >> > > The resets descriptor(lpass_audio_cc_reset_sc7280_desc) is not part of > > the global clock descriptor(lpass_cc_sc7280_desc) as these are part of > > different regmaps. > > > > On a non-QCM6490(SC7280) boards the resets are registered after the > > global descriptor is registered. > > > > But on QCM6490 board we need to register only the reset descriptor and > > no clocks are to be handled/registered and thus passed the match data > > for QCM6490 boards only. > > Yeah, but why this is sc7280? Because it's more or less the same HW, different TZ and hyp firmware?
On 16/09/2024 10:55, Dmitry Baryshkov wrote: > On Mon, Sep 16, 2024 at 10:33:21AM GMT, Krzysztof Kozlowski wrote: >> On 13/09/2024 07:31, Taniya Das wrote: >>> >>> >>> On 8/17/2024 2:55 PM, Krzysztof Kozlowski wrote: >>>> On 16/08/2024 10:32, 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@quicinc.com> >>>>> --- >>>>> 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 45e726477086..b64393089263 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 }, >>>> >>>> That's odd to see sc7280 reset added for qcm6490, but not used fot >>>> sc7280 at all. Didn't you mean here lpass_audio_cc_qcm6409_desc? >>>> >>>> >>> The resets descriptor(lpass_audio_cc_reset_sc7280_desc) is not part of >>> the global clock descriptor(lpass_cc_sc7280_desc) as these are part of >>> different regmaps. >>> >>> On a non-QCM6490(SC7280) boards the resets are registered after the >>> global descriptor is registered. >>> >>> But on QCM6490 board we need to register only the reset descriptor and >>> no clocks are to be handled/registered and thus passed the match data >>> for QCM6490 boards only. >> >> Yeah, but why this is sc7280? > > Because it's more or less the same HW, different TZ and hyp firmware? > Hm, ok, probably I missed something from the context. Best regards, Krzysztof
diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c index 45e726477086..b64393089263 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); + 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;
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@quicinc.com> --- drivers/clk/qcom/lpassaudiocc-sc7280.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)