Add resets and remove qdsp6ss clcok controller for audioreach based platforms.
Changes since v5:
-- Fix compilation issue.
Changes since v4:
-- Update Fixes tag in Merge lpasscc into lpass_aon patch.
-- Revert removal of clk_regmap structure in Merge lpasscc into lpass_aon patch.
Changes since v3:
-- Remove duplicate clock resets patch.
-- Add binding headers for q6 clocks.
-- Create new patch for merging lpasscc q6 clocks into lpass_aon.
-- Create new patches for handling conflicts of ADSP and bypass solution.
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 (6):
dt-bindings: clock: qcom,sc7280-lpasscc: Add qcom,adsp-pil-mode
property
dt-bindings: clock: lpassaudiocc-sc7280: Add binding headers for
lpasscc
clk: qcom: lpasscc-sc7280: Skip qdsp6ss clock registration
clk: qcom: lpasscorecc-sc7280: Skip lpasscorecc registration
clk: qcom: lpassaudiocc-sc7280: Merge lpasscc into lpass_aon
clk: qcom: lpassaudiocc-sc7280: Skip lpass_aon_cc_pll config
.../devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml | 7 +++++++
drivers/clk/qcom/lpassaudiocc-sc7280.c | 16 +++++++++++-----
drivers/clk/qcom/lpasscc-sc7280.c | 12 +++++++-----
drivers/clk/qcom/lpasscorecc-sc7280.c | 3 +++
include/dt-bindings/clock/qcom,lpassaudiocc-sc7280.h | 2 ++
5 files changed, 30 insertions(+), 10 deletions(-)
--
2.7.4
Add binding headers for lpasscc clocks to merge lpasscc clocks into
lpass_aon clk_regmap structure.
Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
---
include/dt-bindings/clock/qcom,lpassaudiocc-sc7280.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/dt-bindings/clock/qcom,lpassaudiocc-sc7280.h b/include/dt-bindings/clock/qcom,lpassaudiocc-sc7280.h
index 22dcd47..c72a980 100644
--- a/include/dt-bindings/clock/qcom,lpassaudiocc-sc7280.h
+++ b/include/dt-bindings/clock/qcom,lpassaudiocc-sc7280.h
@@ -41,6 +41,8 @@
#define LPASS_AON_CC_TX_MCLK_CLK 8
#define LPASS_AON_CC_TX_MCLK_RCG_CLK_SRC 9
#define LPASS_AON_CC_VA_MEM0_CLK 10
+#define LPASS_Q6_AHBM_CLK 11
+#define LPASS_Q6_AHBS_CLK 12
/* LPASS_AON_CC power domains */
#define LPASS_AON_CC_LPASS_AUDIO_HM_GDSC 0
--
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]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Stephen Boyd <[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
Skip lpasscorecc clocks registration for ADSP based platforms
as it's causing NOC errors when ADSP based clocks are enabled.
Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
Tested-by: Mohammad Rafi Shaik <[email protected]>
---
drivers/clk/qcom/lpasscorecc-sc7280.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/clk/qcom/lpasscorecc-sc7280.c b/drivers/clk/qcom/lpasscorecc-sc7280.c
index 6ad19b0..3aa16d8 100644
--- a/drivers/clk/qcom/lpasscorecc-sc7280.c
+++ b/drivers/clk/qcom/lpasscorecc-sc7280.c
@@ -395,6 +395,9 @@ static int lpass_core_cc_sc7280_probe(struct platform_device *pdev)
const struct qcom_cc_desc *desc;
struct regmap *regmap;
+ if (of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode"))
+ return 0;
+
lpass_core_cc_sc7280_regmap_config.name = "lpass_core_cc";
lpass_core_cc_sc7280_regmap_config.max_register = 0x4f004;
desc = &lpass_core_cc_sc7280_desc;
--
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]>
Reviewed-by: Stephen Boyd <[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
Merge lpasscc clocks into lpass_aon clk_regmap structure as they
are using same register space.
Add conditional check for doing lpasscc clock registration only
if regname specified in device tree node.
In existing implementation, lpasscc clocks and lpass_aon clocks are
being registered exclusively and overlapping if both of them are
to be used.
This is required to avoid such overlapping and to register
lpasscc clocks and lpass_aon clocks simultaneously.
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/lpassaudiocc-sc7280.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
index 1339f92..8e2f433 100644
--- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
+++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
@@ -660,6 +660,8 @@ static struct clk_regmap *lpass_aon_cc_sc7280_clocks[] = {
[LPASS_AON_CC_TX_MCLK_2X_CLK] = &lpass_aon_cc_tx_mclk_2x_clk.clkr,
[LPASS_AON_CC_TX_MCLK_CLK] = &lpass_aon_cc_tx_mclk_clk.clkr,
[LPASS_AON_CC_TX_MCLK_RCG_CLK_SRC] = &lpass_aon_cc_tx_mclk_rcg_clk_src.clkr,
+ [LPASS_Q6_AHBM_CLK] = &lpass_q6ss_ahbm_clk.clkr,
+ [LPASS_Q6_AHBS_CLK] = &lpass_q6ss_ahbs_clk.clkr,
};
static struct gdsc *lpass_aon_cc_sc7280_gdscs[] = {
@@ -819,6 +821,7 @@ static int lpass_aon_cc_sc7280_probe(struct platform_device *pdev)
{
const struct qcom_cc_desc *desc;
struct regmap *regmap;
+ struct resource *res;
int ret;
ret = lpass_audio_setup_runtime_pm(pdev);
@@ -826,10 +829,12 @@ static int lpass_aon_cc_sc7280_probe(struct platform_device *pdev)
return ret;
if (of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode")) {
- lpass_audio_cc_sc7280_regmap_config.name = "cc";
- desc = &lpass_cc_sc7280_desc;
- ret = qcom_cc_probe(pdev, desc);
- goto exit;
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cc");
+ if (res) {
+ lpass_audio_cc_sc7280_regmap_config.name = "cc";
+ desc = &lpass_cc_sc7280_desc;
+ return qcom_cc_probe(pdev, desc);
+ }
}
lpass_audio_cc_sc7280_regmap_config.name = "lpasscc_aon";
--
2.7.4
Skip lpass_aon_cc_pll configuration for ADSP based platforms
based on qcom,adsp-pil-mode property.
This is to avoid ADSP out of reset fail.
Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
Tested-by: Mohammad Rafi Shaik <[email protected]>
---
drivers/clk/qcom/lpassaudiocc-sc7280.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
index 8e2f433..1511337 100644
--- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
+++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
@@ -847,7 +847,8 @@ static int lpass_aon_cc_sc7280_probe(struct platform_device *pdev)
goto exit;
}
- clk_lucid_pll_configure(&lpass_aon_cc_pll, regmap, &lpass_aon_cc_pll_config);
+ if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode"))
+ clk_lucid_pll_configure(&lpass_aon_cc_pll, regmap, &lpass_aon_cc_pll_config);
ret = qcom_cc_really_probe(pdev, &lpass_aon_cc_sc7280_desc, regmap);
if (ret) {
--
2.7.4
Quoting Srinivasa Rao Mandadapu (2023-01-26 02:14:23)
> Skip lpasscorecc clocks registration for ADSP based platforms
> as it's causing NOC errors when ADSP based clocks are enabled.
>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> Tested-by: Mohammad Rafi Shaik <[email protected]>
> ---
> drivers/clk/qcom/lpasscorecc-sc7280.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/clk/qcom/lpasscorecc-sc7280.c b/drivers/clk/qcom/lpasscorecc-sc7280.c
> index 6ad19b0..3aa16d8 100644
> --- a/drivers/clk/qcom/lpasscorecc-sc7280.c
> +++ b/drivers/clk/qcom/lpasscorecc-sc7280.c
> @@ -395,6 +395,9 @@ static int lpass_core_cc_sc7280_probe(struct platform_device *pdev)
> const struct qcom_cc_desc *desc;
> struct regmap *regmap;
>
> + if (of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode"))
Why is this node enabled in DT at all if it doesn't provide any clks?
Quoting Srinivasa Rao Mandadapu (2023-01-26 02:14:24)
> Merge lpasscc clocks into lpass_aon clk_regmap structure as they
> are using same register space.
> Add conditional check for doing lpasscc clock registration only
> if regname specified in device tree node.
> In existing implementation, lpasscc clocks and lpass_aon clocks are
> being registered exclusively and overlapping if both of them are
> to be used.
> This is required to avoid such overlapping and to register
> lpasscc clocks and lpass_aon clocks simultaneously.
Can you describe the register ranges that are overlapping?
Here's what I see in DT right now:
lpasscc: lpasscc@3000000 {
compatible = "qcom,sc7280-lpasscc";
reg = <0 0x03000000 0 0x40>,
<0 0x03c04000 0 0x4>;
...
};
lpass_audiocc: clock-controller@3300000 {
compatible = "qcom,sc7280-lpassaudiocc";
reg = <0 0x03300000 0 0x30000>,
<0 0x032a9000 0 0x1000>;
...
};
lpass_aon: clock-controller@3380000 {
compatible = "qcom,sc7280-lpassaoncc";
reg = <0 0x03380000 0 0x30000>;
...
};
lpass_core: clock-controller@3900000 {
compatible = "qcom,sc7280-lpasscorecc";
reg = <0 0x03900000 0 0x50000>;
...
};
Presumably lpascc is really supposed to be a node named
'clock-controller' and is the node that is overlapping with lpass_aon?
>
> 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/lpassaudiocc-sc7280.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> index 1339f92..8e2f433 100644
> --- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
> +++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> @@ -826,10 +829,12 @@ static int lpass_aon_cc_sc7280_probe(struct platform_device *pdev)
> return ret;
>
> if (of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode")) {
> - lpass_audio_cc_sc7280_regmap_config.name = "cc";
> - desc = &lpass_cc_sc7280_desc;
> - ret = qcom_cc_probe(pdev, desc);
> - goto exit;
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cc");
We shouldn't need to check for reg-name property. Instead, the index
should be the only thing that matters.
> + if (res) {
> + lpass_audio_cc_sc7280_regmap_config.name = "cc";
> + desc = &lpass_cc_sc7280_desc;
Quoting Srinivasa Rao Mandadapu (2023-01-26 02:14:25)
> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> index 8e2f433..1511337 100644
> --- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
> +++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> @@ -847,7 +847,8 @@ static int lpass_aon_cc_sc7280_probe(struct platform_device *pdev)
> goto exit;
> }
>
> - clk_lucid_pll_configure(&lpass_aon_cc_pll, regmap, &lpass_aon_cc_pll_config);
> + if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode"))
Please add a comment like
/*
* ADSP firmware is in control of this PLL frequency when
* remoteproc is used. Skip frequency configuration in that
* case.
*/
> + clk_lucid_pll_configure(&lpass_aon_cc_pll, regmap, &lpass_aon_cc_pll_config);
>
> ret = qcom_cc_really_probe(pdev, &lpass_aon_cc_sc7280_desc, regmap);
> if (ret) {
On 1/31/2023 6:24 AM, Stephen Boyd wrote:
Thanks for your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2023-01-26 02:14:23)
>> Skip lpasscorecc clocks registration for ADSP based platforms
>> as it's causing NOC errors when ADSP based clocks are enabled.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
>> Tested-by: Mohammad Rafi Shaik <[email protected]>
>> ---
>> drivers/clk/qcom/lpasscorecc-sc7280.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/lpasscorecc-sc7280.c b/drivers/clk/qcom/lpasscorecc-sc7280.c
>> index 6ad19b0..3aa16d8 100644
>> --- a/drivers/clk/qcom/lpasscorecc-sc7280.c
>> +++ b/drivers/clk/qcom/lpasscorecc-sc7280.c
>> @@ -395,6 +395,9 @@ static int lpass_core_cc_sc7280_probe(struct platform_device *pdev)
>> const struct qcom_cc_desc *desc;
>> struct regmap *regmap;
>>
>> + if (of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode"))
> Why is this node enabled in DT at all if it doesn't provide any clks?
Yes. Agree that we can disable this node in board specific DTS file. As
I thought, disabling node is not appropriate,
preferred this way.
Anyway, as suggested, will drop this patch and disable node in DT.
On 1/31/2023 6:36 AM, Stephen Boyd wrote:
Thanks for your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2023-01-26 02:14:25)
>> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
>> index 8e2f433..1511337 100644
>> --- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
>> +++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
>> @@ -847,7 +847,8 @@ static int lpass_aon_cc_sc7280_probe(struct platform_device *pdev)
>> goto exit;
>> }
>>
>> - clk_lucid_pll_configure(&lpass_aon_cc_pll, regmap, &lpass_aon_cc_pll_config);
>> + if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode"))
>
> Please add a comment like
Okay, Will do accordingly.
>
> /*
> * ADSP firmware is in control of this PLL frequency when
> * remoteproc is used. Skip frequency configuration in that
> * case.
> */
>
>> + clk_lucid_pll_configure(&lpass_aon_cc_pll, regmap, &lpass_aon_cc_pll_config);
>>
>> ret = qcom_cc_really_probe(pdev, &lpass_aon_cc_sc7280_desc, regmap);
>> if (ret) {
On 1/31/2023 6:34 AM, Stephen Boyd wrote:
Thanks for your Time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2023-01-26 02:14:24)
>> Merge lpasscc clocks into lpass_aon clk_regmap structure as they
>> are using same register space.
>> Add conditional check for doing lpasscc clock registration only
>> if regname specified in device tree node.
>> In existing implementation, lpasscc clocks and lpass_aon clocks are
>> being registered exclusively and overlapping if both of them are
>> to be used.
>> This is required to avoid such overlapping and to register
>> lpasscc clocks and lpass_aon clocks simultaneously.
> Can you describe the register ranges that are overlapping?
Okay. Will add register ranges in description.
>
> Here's what I see in DT right now:
>
> lpasscc: lpasscc@3000000 {
> compatible = "qcom,sc7280-lpasscc";
> reg = <0 0x03000000 0 0x40>,
> <0 0x03c04000 0 0x4>;
> ...
> };
>
> lpass_audiocc: clock-controller@3300000 {
> compatible = "qcom,sc7280-lpassaudiocc";
> reg = <0 0x03300000 0 0x30000>,
> <0 0x032a9000 0 0x1000>;
> ...
> };
>
> lpass_aon: clock-controller@3380000 {
> compatible = "qcom,sc7280-lpassaoncc";
> reg = <0 0x03380000 0 0x30000>;
> ...
> };
>
> lpass_core: clock-controller@3900000 {
> compatible = "qcom,sc7280-lpasscorecc";
> reg = <0 0x03900000 0 0x50000>;
> ...
> };
>
> Presumably lpascc is really supposed to be a node named
> 'clock-controller' and is the node that is overlapping with lpass_aon?
Okay. As it's been coming previous patches, didn't change the name.
May be we need to do it as separate patch.
Yes. It's overlapping with lpass_aon ( <0 0x03380000 0 0x30000>).
CC clocks range is <0 0x03389000 0 0x24>;
>
>> 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/lpassaudiocc-sc7280.c | 13 +++++++++----
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
>> index 1339f92..8e2f433 100644
>> --- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
>> +++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
>> @@ -826,10 +829,12 @@ static int lpass_aon_cc_sc7280_probe(struct platform_device *pdev)
>> return ret;
>>
>> if (of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode")) {
>> - lpass_audio_cc_sc7280_regmap_config.name = "cc";
>> - desc = &lpass_cc_sc7280_desc;
>> - ret = qcom_cc_probe(pdev, desc);
>> - goto exit;
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cc");
> We shouldn't need to check for reg-name property. Instead, the index
> should be the only thing that matters.
As qcom_cc_probe() function is mapping the zero index reg property, and
in next implementation qcom_cc_really_probe() is also probing zero index
reg property,
unable to map the same region twice.
Hence all I want here is to skip this cc clock probing by keeping some
check.
If we remove, it may cause ABI break.
>
>> + if (res) {
>> + lpass_audio_cc_sc7280_regmap_config.name = "cc";
>> + desc = &lpass_cc_sc7280_desc;
Quoting Srinivasa Rao Mandadapu (2023-01-31 01:29:16)
>
> On 1/31/2023 6:34 AM, Stephen Boyd wrote:
> Thanks for your Time Stephen!!!
> > Quoting Srinivasa Rao Mandadapu (2023-01-26 02:14:24)
> >> Merge lpasscc clocks into lpass_aon clk_regmap structure as they
> >> are using same register space.
> >> Add conditional check for doing lpasscc clock registration only
> >> if regname specified in device tree node.
> >> In existing implementation, lpasscc clocks and lpass_aon clocks are
> >> being registered exclusively and overlapping if both of them are
> >> to be used.
> >> This is required to avoid such overlapping and to register
> >> lpasscc clocks and lpass_aon clocks simultaneously.
> > Can you describe the register ranges that are overlapping?
> Okay. Will add register ranges in description.
Thanks!
> >
> > Here's what I see in DT right now:
> >
> > lpasscc: lpasscc@3000000 {
> > compatible = "qcom,sc7280-lpasscc";
> > reg = <0 0x03000000 0 0x40>,
> > <0 0x03c04000 0 0x4>;
> > ...
> > };
> >
> > lpass_audiocc: clock-controller@3300000 {
> > compatible = "qcom,sc7280-lpassaudiocc";
> > reg = <0 0x03300000 0 0x30000>,
> > <0 0x032a9000 0 0x1000>;
> > ...
> > };
> >
> > lpass_aon: clock-controller@3380000 {
> > compatible = "qcom,sc7280-lpassaoncc";
> > reg = <0 0x03380000 0 0x30000>;
> > ...
> > };
> >
> > lpass_core: clock-controller@3900000 {
> > compatible = "qcom,sc7280-lpasscorecc";
> > reg = <0 0x03900000 0 0x50000>;
> > ...
> > };
> >
> > Presumably lpascc is really supposed to be a node named
> > 'clock-controller' and is the node that is overlapping with lpass_aon?
>
> Okay. As it's been coming previous patches, didn't change the name.
>
> May be we need to do it as separate patch.
Sure, another patch to rename lpasscc to clock-controller would be
appreciated.
>
> Yes. It's overlapping with lpass_aon ( <0 0x03380000 0 0x30000>).
>
> CC clocks range is <0 0x03389000 0 0x24>;
Is that a new register range for lpasscc? Why do we have that node at
all? Can we add different properties to the existing lpass_audiocc,
lpass_aon, or lpass_core nodes to indicate what clks should or shouldn't
be registered or provided to the kernel?
>
> >
> >> 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/lpassaudiocc-sc7280.c | 13 +++++++++----
> >> 1 file changed, 9 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> >> index 1339f92..8e2f433 100644
> >> --- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
> >> +++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> >> @@ -826,10 +829,12 @@ static int lpass_aon_cc_sc7280_probe(struct platform_device *pdev)
> >> return ret;
> >>
> >> if (of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode")) {
> >> - lpass_audio_cc_sc7280_regmap_config.name = "cc";
> >> - desc = &lpass_cc_sc7280_desc;
> >> - ret = qcom_cc_probe(pdev, desc);
> >> - goto exit;
> >> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cc");
> > We shouldn't need to check for reg-name property. Instead, the index
> > should be the only thing that matters.
>
> As qcom_cc_probe() function is mapping the zero index reg property, and
>
> in next implementation qcom_cc_really_probe() is also probing zero index
> reg property,
>
> unable to map the same region twice.
Use qcom_cc_probe_by_index()?
>
> Hence all I want here is to skip this cc clock probing by keeping some
> check.
>
> If we remove, it may cause ABI break.
>
I'm not sure what you mean here about ABI break, but hopefully just
using qcom_cc_probe_by_index() works!
On 2/1/2023 1:56 AM, Stephen Boyd wrote:
Thanks for your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2023-01-31 01:29:16)
>> On 1/31/2023 6:34 AM, Stephen Boyd wrote:
>> Thanks for your Time Stephen!!!
>>> Quoting Srinivasa Rao Mandadapu (2023-01-26 02:14:24)
>>>> Merge lpasscc clocks into lpass_aon clk_regmap structure as they
>>>> are using same register space.
>>>> Add conditional check for doing lpasscc clock registration only
>>>> if regname specified in device tree node.
>>>> In existing implementation, lpasscc clocks and lpass_aon clocks are
>>>> being registered exclusively and overlapping if both of them are
>>>> to be used.
>>>> This is required to avoid such overlapping and to register
>>>> lpasscc clocks and lpass_aon clocks simultaneously.
>>> Can you describe the register ranges that are overlapping?
>> Okay. Will add register ranges in description.
> Thanks!
>
>>> Here's what I see in DT right now:
>>>
>>> lpasscc: lpasscc@3000000 {
>>> compatible = "qcom,sc7280-lpasscc";
>>> reg = <0 0x03000000 0 0x40>,
>>> <0 0x03c04000 0 0x4>;
>>> ...
>>> };
>>>
>>> lpass_audiocc: clock-controller@3300000 {
>>> compatible = "qcom,sc7280-lpassaudiocc";
>>> reg = <0 0x03300000 0 0x30000>,
>>> <0 0x032a9000 0 0x1000>;
>>> ...
>>> };
>>>
>>> lpass_aon: clock-controller@3380000 {
>>> compatible = "qcom,sc7280-lpassaoncc";
>>> reg = <0 0x03380000 0 0x30000>;
>>> ...
>>> };
>>>
>>> lpass_core: clock-controller@3900000 {
>>> compatible = "qcom,sc7280-lpasscorecc";
>>> reg = <0 0x03900000 0 0x50000>;
>>> ...
>>> };
>>>
>>> Presumably lpascc is really supposed to be a node named
>>> 'clock-controller' and is the node that is overlapping with lpass_aon?
>> Okay. As it's been coming previous patches, didn't change the name.
>>
>> May be we need to do it as separate patch.
> Sure, another patch to rename lpasscc to clock-controller would be
> appreciated.
>
>> Yes. It's overlapping with lpass_aon ( <0 0x03380000 0 0x30000>).
>>
>> CC clocks range is <0 0x03389000 0 0x24>;
> Is that a new register range for lpasscc? Why do we have that node at
> all? Can we add different properties to the existing lpass_audiocc,
> lpass_aon, or lpass_core nodes to indicate what clks should or shouldn't
> be registered or provided to the kernel?
It's not new register range. They are actually AHBM and AHBS clock
registers within lpass_aon regmap range.
Here what I meant lpasscc clocks are not of lpasscc node. I am sorry to
make you confused.
As the reg-name used as "CC", mentioning it as lpasscc clocks. I will
rephrase commit message and re-post.
Previously these two clocks are registered separately. Now we are
merging them into lpass_aon clk reg space.
>
>>>> 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/lpassaudiocc-sc7280.c | 13 +++++++++----
>>>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
>>>> index 1339f92..8e2f433 100644
>>>> --- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
>>>> +++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
>>>> @@ -826,10 +829,12 @@ static int lpass_aon_cc_sc7280_probe(struct platform_device *pdev)
>>>> return ret;
>>>>
>>>> if (of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode")) {
>>>> - lpass_audio_cc_sc7280_regmap_config.name = "cc";
>>>> - desc = &lpass_cc_sc7280_desc;
>>>> - ret = qcom_cc_probe(pdev, desc);
>>>> - goto exit;
>>>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cc");
>>> We shouldn't need to check for reg-name property. Instead, the index
>>> should be the only thing that matters.
>> As qcom_cc_probe() function is mapping the zero index reg property, and
>>
>> in next implementation qcom_cc_really_probe() is also probing zero index
>> reg property,
>>
>> unable to map the same region twice.
> Use qcom_cc_probe_by_index()?
With this, if we mention some index and if it's not present in DT, it
will return error.
Is it okay if error is ignored and proceed?
>
>> Hence all I want here is to skip this cc clock probing by keeping some
>> check.
>>
>> If we remove, it may cause ABI break.
>>
> I'm not sure what you mean here about ABI break, but hopefully just
> using qcom_cc_probe_by_index() works!