| Message ID | 20240208062836.19767-3-quic_tdas@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add updates for clock controllers to support QCM6490 | expand |
On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote: > > The PLL configuration needs to be skipped when remoteproc brings the > LPASS out of reset. Why? > > Also update the lpassaudio_cc_reset regmap name and max register to handle > the regmap conflict warning between lpassaudio_cc_reset and lpassaudio_cc. Separate patch, please. > > Fixes: a9dd26639d05 ("clk: qcom: lpass: Add support for LPASS clock controller for SC7280") > Signed-off-by: Taniya Das <quic_tdas@quicinc.com> > --- > drivers/clk/qcom/lpassaudiocc-sc7280.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c > index c43d0b1af7f7..2619a8ced9d5 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> > @@ -766,11 +767,13 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev) > goto exit; > } > > - clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config); > + if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-skip-pll")) { > + clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config); > > - /* PLL settings */ > - regmap_write(regmap, 0x4, 0x3b); > - regmap_write(regmap, 0x8, 0xff05); > + /* PLL settings */ > + regmap_write(regmap, 0x4, 0x3b); > + regmap_write(regmap, 0x8, 0xff05); > + } > > ret = qcom_cc_really_probe(pdev, &lpass_audio_cc_sc7280_desc, regmap); > if (ret) { > @@ -778,6 +781,9 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev) > goto exit; > } > > + lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc_reset"; > + lpass_audio_cc_sc7280_regmap_config.max_register = 0xc8; > + > ret = qcom_cc_probe_by_index(pdev, 1, &lpass_audio_cc_reset_sc7280_desc); > if (ret) { > dev_err(&pdev->dev, "Failed to register LPASS AUDIO CC Resets\n"); > -- > 2.17.1 > >
Thanks for your review Dmitry. On 2/8/2024 12:32 PM, Dmitry Baryshkov wrote: > On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote: >> >> The PLL configuration needs to be skipped when remoteproc brings the >> LPASS out of reset. > > Why? > On QCM6490 boards, the HLOS is not given access to program the PLL of Low Power Audio Subsystem. Also on these boards the per-requisite of a GDSC is not available. Thus we need to skip the PLL programming. I will update the commit text in my next series. >> >> Also update the lpassaudio_cc_reset regmap name and max register to handle >> the regmap conflict warning between lpassaudio_cc_reset and lpassaudio_cc. > > Separate patch, please. > Yes, I will take care. >> >> Fixes: a9dd26639d05 ("clk: qcom: lpass: Add support for LPASS clock controller for SC7280") >> Signed-off-by: Taniya Das <quic_tdas@quicinc.com> >> --- >> drivers/clk/qcom/lpassaudiocc-sc7280.c | 14 ++++++++++---- >> 1 file changed, 10 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c >> index c43d0b1af7f7..2619a8ced9d5 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> >> @@ -766,11 +767,13 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev) >> goto exit; >> } >> >> - clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config); >> + if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-skip-pll")) { >> + clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config); >> >> - /* PLL settings */ >> - regmap_write(regmap, 0x4, 0x3b); >> - regmap_write(regmap, 0x8, 0xff05); >> + /* PLL settings */ >> + regmap_write(regmap, 0x4, 0x3b); >> + regmap_write(regmap, 0x8, 0xff05); >> + } >> >> ret = qcom_cc_really_probe(pdev, &lpass_audio_cc_sc7280_desc, regmap); >> if (ret) { >> @@ -778,6 +781,9 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev) >> goto exit; >> } >> >> + lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc_reset"; >> + lpass_audio_cc_sc7280_regmap_config.max_register = 0xc8; >> + >> ret = qcom_cc_probe_by_index(pdev, 1, &lpass_audio_cc_reset_sc7280_desc); >> if (ret) { >> dev_err(&pdev->dev, "Failed to register LPASS AUDIO CC Resets\n"); >> -- >> 2.17.1 >> >> > >
On Wed, 6 Mar 2024 at 08:56, Taniya Das <quic_tdas@quicinc.com> wrote: > > Thanks for your review Dmitry. > > On 2/8/2024 12:32 PM, Dmitry Baryshkov wrote: > > On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote: > >> > >> The PLL configuration needs to be skipped when remoteproc brings the > >> LPASS out of reset. > > > > Why? > > > > On QCM6490 boards, the HLOS is not given access to program the PLL of > Low Power Audio Subsystem. Also on these boards the per-requisite of a > GDSC is not available. Thus we need to skip the PLL programming. > > I will update the commit text in my next series. Please use SoC-specific compatible instead of adding extra properties. > > >> > >> Also update the lpassaudio_cc_reset regmap name and max register to handle > >> the regmap conflict warning between lpassaudio_cc_reset and lpassaudio_cc. > > > > Separate patch, please. > > > > Yes, I will take care. > > >> > >> Fixes: a9dd26639d05 ("clk: qcom: lpass: Add support for LPASS clock controller for SC7280") > >> Signed-off-by: Taniya Das <quic_tdas@quicinc.com> > >> --- > >> drivers/clk/qcom/lpassaudiocc-sc7280.c | 14 ++++++++++---- > >> 1 file changed, 10 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c > >> index c43d0b1af7f7..2619a8ced9d5 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> > >> @@ -766,11 +767,13 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev) > >> goto exit; > >> } > >> > >> - clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config); > >> + if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-skip-pll")) { > >> + clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config); > >> > >> - /* PLL settings */ > >> - regmap_write(regmap, 0x4, 0x3b); > >> - regmap_write(regmap, 0x8, 0xff05); > >> + /* PLL settings */ > >> + regmap_write(regmap, 0x4, 0x3b); > >> + regmap_write(regmap, 0x8, 0xff05); > >> + } > >> > >> ret = qcom_cc_really_probe(pdev, &lpass_audio_cc_sc7280_desc, regmap); > >> if (ret) { > >> @@ -778,6 +781,9 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev) > >> goto exit; > >> } > >> > >> + lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc_reset"; > >> + lpass_audio_cc_sc7280_regmap_config.max_register = 0xc8; > >> + > >> ret = qcom_cc_probe_by_index(pdev, 1, &lpass_audio_cc_reset_sc7280_desc); > >> if (ret) { > >> dev_err(&pdev->dev, "Failed to register LPASS AUDIO CC Resets\n"); > >> -- > >> 2.17.1 > >> > >> > > > > > > -- > Thanks & Regards, > Taniya Das.
On 2/8/24 07:28, Taniya Das wrote: > The PLL configuration needs to be skipped when remoteproc brings the > LPASS out of reset. > > Also update the lpassaudio_cc_reset regmap name and max register to handle > the regmap conflict warning between lpassaudio_cc_reset and lpassaudio_cc. > > Fixes: a9dd26639d05 ("clk: qcom: lpass: Add support for LPASS clock controller for SC7280") > Signed-off-by: Taniya Das <quic_tdas@quicinc.com> > --- > drivers/clk/qcom/lpassaudiocc-sc7280.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c > index c43d0b1af7f7..2619a8ced9d5 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> > @@ -766,11 +767,13 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev) > goto exit; > } > > - clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config); > + if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-skip-pll")) { Big no-no. > + clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config); > > - /* PLL settings */ > - regmap_write(regmap, 0x4, 0x3b); > - regmap_write(regmap, 0x8, 0xff05); > + /* PLL settings */ > + regmap_write(regmap, 0x4, 0x3b); > + regmap_write(regmap, 0x8, 0xff05); Model these properly and use the abstracted clock (re)configuration functions. Add the unreachable clocks to `protected-clocks = <>` and make sure that the aforementioned configure calls check if the PLL was really registered. > + } > > ret = qcom_cc_really_probe(pdev, &lpass_audio_cc_sc7280_desc, regmap); > if (ret) { > @@ -778,6 +781,9 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev) > goto exit; > } > > + lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc_reset"; Ugh.. are these really not contiguous, or were the register ranges misrepresented from the start? > + lpass_audio_cc_sc7280_regmap_config.max_register = 0xc8; Provide the real size of the block in .max_register instead, unconditionally Konrad
diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c index c43d0b1af7f7..2619a8ced9d5 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> @@ -766,11 +767,13 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev) goto exit; } - clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config); + if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-skip-pll")) { + clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config); - /* PLL settings */ - regmap_write(regmap, 0x4, 0x3b); - regmap_write(regmap, 0x8, 0xff05); + /* PLL settings */ + regmap_write(regmap, 0x4, 0x3b); + regmap_write(regmap, 0x8, 0xff05); + } ret = qcom_cc_really_probe(pdev, &lpass_audio_cc_sc7280_desc, regmap); if (ret) { @@ -778,6 +781,9 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev) goto exit; } + lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc_reset"; + lpass_audio_cc_sc7280_regmap_config.max_register = 0xc8; + ret = qcom_cc_probe_by_index(pdev, 1, &lpass_audio_cc_reset_sc7280_desc); if (ret) { dev_err(&pdev->dev, "Failed to register LPASS AUDIO CC Resets\n");
The PLL configuration needs to be skipped when remoteproc brings the LPASS out of reset. Also update the lpassaudio_cc_reset regmap name and max register to handle the regmap conflict warning between lpassaudio_cc_reset and lpassaudio_cc. Fixes: a9dd26639d05 ("clk: qcom: lpass: Add support for LPASS clock controller for SC7280") Signed-off-by: Taniya Das <quic_tdas@quicinc.com> --- drivers/clk/qcom/lpassaudiocc-sc7280.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) -- 2.17.1