Add resets and remove qdsp6ss clcok controller for audioreach based platforms.
Changes since v2:
-- Revert removing qdsp6ss clock control.
-- Add Conditional check for qdsp6ss clock registration.
Changes since v1:
-- Update commit message.
-- Remove qdsp6ss clock control.
Srinivasa Rao Mandadapu (4):
dt-bindings: clock: qcom,sc7280-lpasscc: Add qcom,adsp-pil-mode
property
dt-bindings: clock: qcom,sc7280-lpasscc: Add resets for audioreach
clk: qcom: lpasscc-sc7280: Skip qdsp6ss clock registration
clk: qcom: lpasscc-sc7280: Add resets for audioreach
.../bindings/clock/qcom,sc7280-lpasscc.yaml | 19 ++++++++++--
drivers/clk/qcom/lpasscc-sc7280.c | 35 ++++++++++++++++++----
2 files changed, 47 insertions(+), 7 deletions(-)
--
2.7.4
Add support for LPASS audio clock gating for RX/TX/SWA core bus clocks
for audioreach based SC7280 platforms.
Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
Tested-by: Mohammad Rafi Shaik <[email protected]>
---
.../devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml
index 97c6bd9..054c496 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml
@@ -31,15 +31,20 @@ properties:
'#clock-cells':
const: 1
+ '#reset-cells':
+ const: 1
+
reg:
items:
- description: LPASS qdsp6ss register
- description: LPASS top-cc register
+ - description: LPASS reset-cgcr register
reg-names:
items:
- const: qdsp6ss
- const: top_cc
+ - const: reset_cgcr
qcom,adsp-pil-mode:
description:
@@ -62,11 +67,14 @@ examples:
#include <dt-bindings/clock/qcom,lpass-sc7280.h>
clock-controller@3000000 {
compatible = "qcom,sc7280-lpasscc";
- reg = <0x03000000 0x40>, <0x03c04000 0x4>;
- reg-names = "qdsp6ss", "top_cc";
+ reg = <0x03000000 0x40>,
+ <0x03c04000 0x4>,
+ <0x032a9000 0x1000>;
+ reg-names = "qdsp6ss", "top_cc", "reset_cgcr";
clocks = <&gcc GCC_CFG_NOC_LPASS_CLK>;
clock-names = "iface";
qcom,adsp-pil-mode;
#clock-cells = <1>;
+ #reset-cells = <1>;
};
...
--
2.7.4
The clock gating control for TX/RX/WSA core bus clocks would be required
to be reset(moved from hardware control) from audio core driver. Thus
add the support for the reset clocks in audioreach based clock driver.
Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
Tested-by: Mohammad Rafi Shaik <[email protected]>
---
drivers/clk/qcom/lpasscc-sc7280.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/clk/qcom/lpasscc-sc7280.c b/drivers/clk/qcom/lpasscc-sc7280.c
index 85dd5b9..1efb72d 100644
--- a/drivers/clk/qcom/lpasscc-sc7280.c
+++ b/drivers/clk/qcom/lpasscc-sc7280.c
@@ -12,10 +12,12 @@
#include <linux/regmap.h>
#include <dt-bindings/clock/qcom,lpass-sc7280.h>
+#include <dt-bindings/clock/qcom,lpassaudiocc-sc7280.h>
#include "clk-regmap.h"
#include "clk-branch.h"
#include "common.h"
+#include "reset.h"
static struct clk_branch lpass_top_cc_lpi_q6_axim_hs_clk = {
.halt_reg = 0x0,
@@ -102,6 +104,18 @@ static const struct qcom_cc_desc lpass_qdsp6ss_sc7280_desc = {
.num_clks = ARRAY_SIZE(lpass_qdsp6ss_sc7280_clocks),
};
+static const struct qcom_reset_map lpass_cc_sc7280_resets[] = {
+ [LPASS_AUDIO_SWR_RX_CGCR] = { 0xa0, 1 },
+ [LPASS_AUDIO_SWR_TX_CGCR] = { 0xa8, 1 },
+ [LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
+};
+
+static const struct qcom_cc_desc lpass_audio_cc_reset_sc7280_desc = {
+ .config = &lpass_regmap_config,
+ .resets = lpass_cc_sc7280_resets,
+ .num_resets = ARRAY_SIZE(lpass_cc_sc7280_resets),
+};
+
static int lpass_cc_sc7280_probe(struct platform_device *pdev)
{
const struct qcom_cc_desc *desc;
@@ -134,6 +148,15 @@ static int lpass_cc_sc7280_probe(struct platform_device *pdev)
if (ret)
goto destroy_pm_clk;
+ if (of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode")) {
+ lpass_regmap_config.name = "reset_cgcr";
+ desc = &lpass_audio_cc_reset_sc7280_desc;
+
+ ret = qcom_cc_probe_by_index(pdev, 2, desc);
+ if (ret)
+ goto destroy_pm_clk;
+ }
+
return 0;
destroy_pm_clk:
--
2.7.4
When this property is set, the remoteproc is used to boot the
LPASS and therefore qdsp6ss clocks would be used to bring LPASS
out of reset, hence they are directly controlled by the remoteproc.
This is a cleanup done to handle overlap of regmap of lpasscc
and adsp remoteproc blocks.
Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
Tested-by: Mohammad Rafi Shaik <[email protected]>
---
Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml
index 6151fde..97c6bd9 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml
@@ -41,6 +41,12 @@ properties:
- const: qdsp6ss
- const: top_cc
+ qcom,adsp-pil-mode:
+ description:
+ Indicates if the LPASS would be brought out of reset using
+ remoteproc peripheral loader.
+ type: boolean
+
required:
- compatible
- reg
@@ -60,6 +66,7 @@ examples:
reg-names = "qdsp6ss", "top_cc";
clocks = <&gcc GCC_CFG_NOC_LPASS_CLK>;
clock-names = "iface";
+ qcom,adsp-pil-mode;
#clock-cells = <1>;
};
...
--
2.7.4
The qdsp6ss memory region is being shared by ADSP remoteproc device and
lpasscc clock device, hence causing memory conflict.
As the qdsp6ss clocks are being enabled in remoteproc driver, skip qdsp6ss
clock registration if "qcom,adsp-pil-mode" is enabled.
Fixes: 4ab43d171181 ("clk: qcom: Add lpass clock controller driver for SC7280")
Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
Tested-by: Mohammad Rafi Shaik <[email protected]>
---
drivers/clk/qcom/lpasscc-sc7280.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/clk/qcom/lpasscc-sc7280.c b/drivers/clk/qcom/lpasscc-sc7280.c
index 5c1e17b..85dd5b9 100644
--- a/drivers/clk/qcom/lpasscc-sc7280.c
+++ b/drivers/clk/qcom/lpasscc-sc7280.c
@@ -118,12 +118,14 @@ static int lpass_cc_sc7280_probe(struct platform_device *pdev)
goto destroy_pm_clk;
}
- lpass_regmap_config.name = "qdsp6ss";
- desc = &lpass_qdsp6ss_sc7280_desc;
+ if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode")) {
+ lpass_regmap_config.name = "qdsp6ss";
+ desc = &lpass_qdsp6ss_sc7280_desc;
- ret = qcom_cc_probe_by_index(pdev, 0, desc);
- if (ret)
- goto destroy_pm_clk;
+ ret = qcom_cc_probe_by_index(pdev, 0, desc);
+ if (ret)
+ goto destroy_pm_clk;
+ }
lpass_regmap_config.name = "top_cc";
desc = &lpass_cc_top_sc7280_desc;
--
2.7.4
On 04/01/2023 17:21, Srinivasa Rao Mandadapu wrote:
> When this property is set, the remoteproc is used to boot the
> LPASS and therefore qdsp6ss clocks would be used to bring LPASS
> out of reset, hence they are directly controlled by the remoteproc.
>
> This is a cleanup done to handle overlap of regmap of lpasscc
> and adsp remoteproc blocks.
>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> Tested-by: Mohammad Rafi Shaik <[email protected]>
> ---
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On 04/01/2023 17:21, Srinivasa Rao Mandadapu wrote:
> Add support for LPASS audio clock gating for RX/TX/SWA core bus clocks
> for audioreach based SC7280 platforms.
>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> Tested-by: Mohammad Rafi Shaik <[email protected]>
> ---
> .../devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
Quoting Srinivasa Rao Mandadapu (2023-01-04 08:21:34)
> When this property is set, the remoteproc is used to boot the
> LPASS and therefore qdsp6ss clocks would be used to bring LPASS
> out of reset, hence they are directly controlled by the remoteproc.
>
> This is a cleanup done to handle overlap of regmap of lpasscc
> and adsp remoteproc blocks.
>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> Tested-by: Mohammad Rafi Shaik <[email protected]>
> ---
Reviewed-by: Stephen Boyd <[email protected]>
Quoting Srinivasa Rao Mandadapu (2023-01-04 08:21:37)
> The clock gating control for TX/RX/WSA core bus clocks would be required
> to be reset(moved from hardware control) from audio core driver. Thus
> add the support for the reset clocks in audioreach based clock driver.
>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> Tested-by: Mohammad Rafi Shaik <[email protected]>
> ---
> drivers/clk/qcom/lpasscc-sc7280.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/clk/qcom/lpasscc-sc7280.c b/drivers/clk/qcom/lpasscc-sc7280.c
> index 85dd5b9..1efb72d 100644
> --- a/drivers/clk/qcom/lpasscc-sc7280.c
> +++ b/drivers/clk/qcom/lpasscc-sc7280.c
> @@ -12,10 +12,12 @@
> #include <linux/regmap.h>
>
> #include <dt-bindings/clock/qcom,lpass-sc7280.h>
> +#include <dt-bindings/clock/qcom,lpassaudiocc-sc7280.h>
>
> #include "clk-regmap.h"
> #include "clk-branch.h"
> #include "common.h"
> +#include "reset.h"
>
> static struct clk_branch lpass_top_cc_lpi_q6_axim_hs_clk = {
> .halt_reg = 0x0,
> @@ -102,6 +104,18 @@ static const struct qcom_cc_desc lpass_qdsp6ss_sc7280_desc = {
> .num_clks = ARRAY_SIZE(lpass_qdsp6ss_sc7280_clocks),
> };
>
> +static const struct qcom_reset_map lpass_cc_sc7280_resets[] = {
> + [LPASS_AUDIO_SWR_RX_CGCR] = { 0xa0, 1 },
> + [LPASS_AUDIO_SWR_TX_CGCR] = { 0xa8, 1 },
> + [LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
Why are we adding these resets again? These are already exposed in
lpassaudiocc-sc7280.c
Quoting Srinivasa Rao Mandadapu (2023-01-04 08:21:36)
> The qdsp6ss memory region is being shared by ADSP remoteproc device and
> lpasscc clock device, hence causing memory conflict.
> As the qdsp6ss clocks are being enabled in remoteproc driver, skip qdsp6ss
> clock registration if "qcom,adsp-pil-mode" is enabled.
>
> Fixes: 4ab43d171181 ("clk: qcom: Add lpass clock controller driver for SC7280")
>
No newline here.
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> Tested-by: Mohammad Rafi Shaik <[email protected]>
> ---
Reviewed-by: Stephen Boyd <[email protected]>
On 1/12/2023 2:54 AM, Stephen Boyd wrote:
Thanks for your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2023-01-04 08:21:37)
>> The clock gating control for TX/RX/WSA core bus clocks would be required
>> to be reset(moved from hardware control) from audio core driver. Thus
>> add the support for the reset clocks in audioreach based clock driver.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
>> Tested-by: Mohammad Rafi Shaik <[email protected]>
>> ---
>> drivers/clk/qcom/lpasscc-sc7280.c | 23 +++++++++++++++++++++++
>> 1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/lpasscc-sc7280.c b/drivers/clk/qcom/lpasscc-sc7280.c
>> index 85dd5b9..1efb72d 100644
>> --- a/drivers/clk/qcom/lpasscc-sc7280.c
>> +++ b/drivers/clk/qcom/lpasscc-sc7280.c
>> @@ -12,10 +12,12 @@
>> #include <linux/regmap.h>
>>
>> #include <dt-bindings/clock/qcom,lpass-sc7280.h>
>> +#include <dt-bindings/clock/qcom,lpassaudiocc-sc7280.h>
>>
>> #include "clk-regmap.h"
>> #include "clk-branch.h"
>> #include "common.h"
>> +#include "reset.h"
>>
>> static struct clk_branch lpass_top_cc_lpi_q6_axim_hs_clk = {
>> .halt_reg = 0x0,
>> @@ -102,6 +104,18 @@ static const struct qcom_cc_desc lpass_qdsp6ss_sc7280_desc = {
>> .num_clks = ARRAY_SIZE(lpass_qdsp6ss_sc7280_clocks),
>> };
>>
>> +static const struct qcom_reset_map lpass_cc_sc7280_resets[] = {
>> + [LPASS_AUDIO_SWR_RX_CGCR] = { 0xa0, 1 },
>> + [LPASS_AUDIO_SWR_TX_CGCR] = { 0xa8, 1 },
>> + [LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
> Why are we adding these resets again? These are already exposed in
> lpassaudiocc-sc7280.c
As explained in previous versions, legacy path nodes are not being used
in ADSP based platforms, due to conflicts.
Hence lpasscc node alone being used exclusively in ADSP based solution,
resets are added.
In probe also, these reset controls are enabled based on
"qcom,adsp-pil-mode" property.
Quoting Srinivasa Rao Mandadapu (2023-01-11 23:53:23)
>
> On 1/12/2023 2:54 AM, Stephen Boyd wrote:
> Thanks for your time Stephen!!!
> > Quoting Srinivasa Rao Mandadapu (2023-01-04 08:21:37)
> >> diff --git a/drivers/clk/qcom/lpasscc-sc7280.c b/drivers/clk/qcom/lpasscc-sc7280.c
> >> index 85dd5b9..1efb72d 100644
> >> --- a/drivers/clk/qcom/lpasscc-sc7280.c
> >> +++ b/drivers/clk/qcom/lpasscc-sc7280.c
> >> @@ -102,6 +104,18 @@ static const struct qcom_cc_desc lpass_qdsp6ss_sc7280_desc = {
> >> .num_clks = ARRAY_SIZE(lpass_qdsp6ss_sc7280_clocks),
> >> };
> >>
> >> +static const struct qcom_reset_map lpass_cc_sc7280_resets[] = {
> >> + [LPASS_AUDIO_SWR_RX_CGCR] = { 0xa0, 1 },
> >> + [LPASS_AUDIO_SWR_TX_CGCR] = { 0xa8, 1 },
> >> + [LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
> > Why are we adding these resets again? These are already exposed in
> > lpassaudiocc-sc7280.c
>
> As explained in previous versions, legacy path nodes are not being used
> in ADSP based platforms, due to conflicts.
What is legacy path nodes?
>
> Hence lpasscc node alone being used exclusively in ADSP based solution,
> resets are added.
I think I understand..
>
> In probe also, these reset controls are enabled based on
> "qcom,adsp-pil-mode" property.
>
but now I'm super confused! Please help me! We shouldn't have two
different device nodes for the same physical hardware registers.
Instead, we should have one node. The "qcom,adsp-pil-mode" property was
supposed to indicate the different mode of operation.
Maybe the audio clk and reset drivers on sc7280 are duplicating each
other and one of them can be removed?
On 1/13/2023 1:05 AM, Stephen Boyd wrote:
> Quoting Srinivasa Rao Mandadapu (2023-01-11 23:53:23)
>> On 1/12/2023 2:54 AM, Stephen Boyd wrote:
>> Thanks for your time Stephen!!!
>>> Quoting Srinivasa Rao Mandadapu (2023-01-04 08:21:37)
>>>> diff --git a/drivers/clk/qcom/lpasscc-sc7280.c b/drivers/clk/qcom/lpasscc-sc7280.c
>>>> index 85dd5b9..1efb72d 100644
>>>> --- a/drivers/clk/qcom/lpasscc-sc7280.c
>>>> +++ b/drivers/clk/qcom/lpasscc-sc7280.c
>>>> @@ -102,6 +104,18 @@ static const struct qcom_cc_desc lpass_qdsp6ss_sc7280_desc = {
>>>> .num_clks = ARRAY_SIZE(lpass_qdsp6ss_sc7280_clocks),
>>>> };
>>>>
>>>> +static const struct qcom_reset_map lpass_cc_sc7280_resets[] = {
>>>> + [LPASS_AUDIO_SWR_RX_CGCR] = { 0xa0, 1 },
>>>> + [LPASS_AUDIO_SWR_TX_CGCR] = { 0xa8, 1 },
>>>> + [LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
>>> Why are we adding these resets again? These are already exposed in
>>> lpassaudiocc-sc7280.c
>> As explained in previous versions, legacy path nodes are not being used
>> in ADSP based platforms, due to conflicts.
> What is legacy path nodes?
Legacy path nodes are for ADSP bypass use case such as nodes
lpass_audiocc, lpass_core, etc.
>
>> Hence lpasscc node alone being used exclusively in ADSP based solution,
>> resets are added.
> I think I understand..
>
>> In probe also, these reset controls are enabled based on
>> "qcom,adsp-pil-mode" property.
>>
> but now I'm super confused! Please help me! We shouldn't have two
> different device nodes for the same physical hardware registers.
> Instead, we should have one node. The "qcom,adsp-pil-mode" property was
> supposed to indicate the different mode of operation.
>
> Maybe the audio clk and reset drivers on sc7280 are duplicating each
> other and one of them can be removed?
Yes agreed, that for controlling same registers from two different
drivers is not good solution.
But, when we are validating ADSP solution, we have seen issues like ADSP
is not coming out of reset with the existing bypass mode
clock drivers(lpassaudiocc_sc7280.c and lpasscoreecc_sc7280.c) enabled.
As per your suggestion, will try to address that issues with
"qcom,adsp-pil-mode" property and avoid duplicating reset control code
in lpasscc driver(lpasscc_sc7280.c).