THis patchset adds support for SM8450 and SC8280XP SoC.
Tested SmartSpeakers and Headset on SM8450 MTP and
Lenovo Thinkpad X13s.
THanks,
Srini
Srinivas Kandagatla (6):
ASoC: qcom: dt-bindings: add sm8450 and sc8280xp compatibles
ASoC: codecs: wsa-macro: add support for sm8450 and sc8280xp
ASoC: codecs: tx-macro: add support for sm8450 and sc8280xp
ASoC: codecs: rx-macro: add support for sm8450 and sc8280xp
ASoC: codecs: va-macro: clear the frame sync counter before enabling
ASoC: codecs: tx-macro: add support for sm8450 and sc8280xp
.../bindings/sound/qcom,lpass-rx-macro.yaml | 2 +
.../bindings/sound/qcom,lpass-tx-macro.yaml | 2 +
.../bindings/sound/qcom,lpass-va-macro.yaml | 2 +
.../bindings/sound/qcom,lpass-wsa-macro.yaml | 2 +
sound/soc/codecs/lpass-rx-macro.c | 2 +
sound/soc/codecs/lpass-tx-macro.c | 2 +
sound/soc/codecs/lpass-va-macro.c | 72 +++++++++++++++++--
sound/soc/codecs/lpass-wsa-macro.c | 2 +
8 files changed, 82 insertions(+), 4 deletions(-)
--
2.21.0
Add compatible for sm8450 and sc8280xp.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/lpass-tx-macro.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sound/soc/codecs/lpass-tx-macro.c b/sound/soc/codecs/lpass-tx-macro.c
index 55503ba480bb..4384266847ad 100644
--- a/sound/soc/codecs/lpass-tx-macro.c
+++ b/sound/soc/codecs/lpass-tx-macro.c
@@ -1988,6 +1988,8 @@ static const struct dev_pm_ops tx_macro_pm_ops = {
static const struct of_device_id tx_macro_dt_match[] = {
{ .compatible = "qcom,sc7280-lpass-tx-macro" },
{ .compatible = "qcom,sm8250-lpass-tx-macro" },
+ { .compatible = "qcom,sm8450-lpass-tx-macro" },
+ { .compatible = "qcom,sc8280xp-lpass-tx-macro" },
{ }
};
MODULE_DEVICE_TABLE(of, tx_macro_dt_match);
--
2.21.0
LPASS VA Macro now has soundwire master to deal with access to
analog mic in low power island use cases.
This is added after sc8280xp, add support for this.
Along with this also add compatibles for sm8450 and sc8280xp.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/lpass-va-macro.c | 64 ++++++++++++++++++++++++++++++-
1 file changed, 62 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
index a35f684053d2..f8b0c8caa1db 100644
--- a/sound/soc/codecs/lpass-va-macro.c
+++ b/sound/soc/codecs/lpass-va-macro.c
@@ -25,6 +25,10 @@
#define CDC_VA_FS_CONTROL_EN BIT(0)
#define CDC_VA_FS_COUNTER_CLR BIT(1)
#define CDC_VA_CLK_RST_CTRL_SWR_CONTROL (0x0008)
+#define CDC_VA_SWR_RESET_MASK BIT(1)
+#define CDC_VA_SWR_RESET_ENABLE BIT(1)
+#define CDC_VA_SWR_CLK_EN_MASK BIT(0)
+#define CDC_VA_SWR_CLK_ENABLE BIT(0)
#define CDC_VA_TOP_CSR_TOP_CFG0 (0x0080)
#define CDC_VA_FS_BROADCAST_EN BIT(1)
#define CDC_VA_TOP_CSR_DMIC0_CTL (0x0084)
@@ -66,6 +70,8 @@
#define CDC_VA_TOP_CSR_SWR_MIC_CTL0 (0x00D0)
#define CDC_VA_TOP_CSR_SWR_MIC_CTL1 (0x00D4)
#define CDC_VA_TOP_CSR_SWR_MIC_CTL2 (0x00D8)
+#define CDC_VA_SWR_MIC_CLK_SEL_0_1_MASK (0xEE)
+#define CDC_VA_SWR_MIC_CLK_SEL_0_1_DIV1 (0xCC)
#define CDC_VA_TOP_CSR_SWR_CTRL (0x00DC)
#define CDC_VA_INP_MUX_ADC_MUX0_CFG0 (0x0100)
#define CDC_VA_INP_MUX_ADC_MUX0_CFG1 (0x0104)
@@ -194,6 +200,8 @@ struct va_macro {
unsigned long active_ch_mask[VA_MACRO_MAX_DAIS];
unsigned long active_ch_cnt[VA_MACRO_MAX_DAIS];
u16 dmic_clk_div;
+ bool has_swr_master;
+ bool reset_swr;
int dec_mode[VA_MACRO_NUM_DECIMATORS];
struct regmap *regmap;
@@ -325,6 +333,9 @@ static bool va_is_rw_register(struct device *dev, unsigned int reg)
case CDC_VA_TOP_CSR_DMIC2_CTL:
case CDC_VA_TOP_CSR_DMIC3_CTL:
case CDC_VA_TOP_CSR_DMIC_CFG:
+ case CDC_VA_TOP_CSR_SWR_MIC_CTL0:
+ case CDC_VA_TOP_CSR_SWR_MIC_CTL1:
+ case CDC_VA_TOP_CSR_SWR_MIC_CTL2:
case CDC_VA_TOP_CSR_DEBUG_BUS:
case CDC_VA_TOP_CSR_DEBUG_EN:
case CDC_VA_TOP_CSR_TX_I2S_CTL:
@@ -1306,12 +1317,40 @@ static const struct snd_soc_component_driver va_macro_component_drv = {
static int fsgen_gate_enable(struct clk_hw *hw)
{
- return va_macro_mclk_enable(to_va_macro(hw), true);
+ struct va_macro *va = to_va_macro(hw);
+ struct regmap *regmap = va->regmap;
+ int ret;
+
+ ret = va_macro_mclk_enable(va, true);
+ if (!va->has_swr_master)
+ return ret;
+
+ if (va->reset_swr)
+ regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_VA_SWR_RESET_MASK,
+ CDC_VA_SWR_RESET_ENABLE);
+
+ regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_VA_SWR_CLK_EN_MASK,
+ CDC_VA_SWR_CLK_ENABLE);
+ if (va->reset_swr)
+ regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_VA_SWR_RESET_MASK, 0x0);
+ va->reset_swr = false;
+
+ return ret;
}
static void fsgen_gate_disable(struct clk_hw *hw)
{
- va_macro_mclk_enable(to_va_macro(hw), false);
+ struct va_macro *va = to_va_macro(hw);
+ struct regmap *regmap = va->regmap;
+
+ if (va->has_swr_master)
+ regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_VA_SWR_CLK_EN_MASK, 0x0);
+
+ va_macro_mclk_enable(va, false);
}
static int fsgen_gate_is_enabled(struct clk_hw *hw)
@@ -1459,6 +1498,11 @@ static int va_macro_probe(struct platform_device *pdev)
dev_set_drvdata(dev, va);
+ if (of_device_is_compatible(dev->of_node, "qcom,sm8450-lpass-va-macro") ||
+ of_device_is_compatible(dev->of_node, "qcom,sc8280xp-lpass-va-macro")) {
+ va->has_swr_master = true;
+ va->reset_swr = true;
+ }
/* mclk rate */
clk_set_rate(va->mclk, 2 * VA_MACRO_MCLK_FREQ);
@@ -1484,6 +1528,20 @@ static int va_macro_probe(struct platform_device *pdev)
goto err_clkout;
}
+ if (va->has_swr_master) {
+ /* Set default CLK div to 1 */
+ regmap_update_bits(va->regmap, CDC_VA_TOP_CSR_SWR_MIC_CTL0,
+ CDC_VA_SWR_MIC_CLK_SEL_0_1_MASK,
+ CDC_VA_SWR_MIC_CLK_SEL_0_1_DIV1);
+ regmap_update_bits(va->regmap, CDC_VA_TOP_CSR_SWR_MIC_CTL1,
+ CDC_VA_SWR_MIC_CLK_SEL_0_1_MASK,
+ CDC_VA_SWR_MIC_CLK_SEL_0_1_DIV1);
+ regmap_update_bits(va->regmap, CDC_VA_TOP_CSR_SWR_MIC_CTL2,
+ CDC_VA_SWR_MIC_CLK_SEL_0_1_MASK,
+ CDC_VA_SWR_MIC_CLK_SEL_0_1_DIV1);
+
+ }
+
ret = devm_snd_soc_register_component(dev, &va_macro_component_drv,
va_macro_dais,
ARRAY_SIZE(va_macro_dais));
@@ -1560,6 +1618,8 @@ static const struct dev_pm_ops va_macro_pm_ops = {
static const struct of_device_id va_macro_dt_match[] = {
{ .compatible = "qcom,sc7280-lpass-va-macro" },
{ .compatible = "qcom,sm8250-lpass-va-macro" },
+ { .compatible = "qcom,sm8450-lpass-va-macro" },
+ { .compatible = "qcom,sc8280xp-lpass-va-macro" },
{}
};
MODULE_DEVICE_TABLE(of, va_macro_dt_match);
--
2.21.0
Clear the frame sync counter before enabling it.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/lpass-va-macro.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
index 1ea10dc70748..a35f684053d2 100644
--- a/sound/soc/codecs/lpass-va-macro.c
+++ b/sound/soc/codecs/lpass-va-macro.c
@@ -23,6 +23,7 @@
#define CDC_VA_MCLK_CONTROL_EN BIT(0)
#define CDC_VA_CLK_RST_CTRL_FS_CNT_CONTROL (0x0004)
#define CDC_VA_FS_CONTROL_EN BIT(0)
+#define CDC_VA_FS_COUNTER_CLR BIT(1)
#define CDC_VA_CLK_RST_CTRL_SWR_CONTROL (0x0008)
#define CDC_VA_TOP_CSR_TOP_CFG0 (0x0080)
#define CDC_VA_FS_BROADCAST_EN BIT(1)
@@ -423,9 +424,12 @@ static int va_clk_rsc_fs_gen_request(struct va_macro *va, bool enable)
regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_MCLK_CONTROL,
CDC_VA_MCLK_CONTROL_EN,
CDC_VA_MCLK_CONTROL_EN);
-
+ /* clear the fs counter */
+ regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_FS_CNT_CONTROL,
+ CDC_VA_FS_CONTROL_EN | CDC_VA_FS_COUNTER_CLR,
+ CDC_VA_FS_CONTROL_EN | CDC_VA_FS_COUNTER_CLR);
regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_FS_CNT_CONTROL,
- CDC_VA_FS_CONTROL_EN,
+ CDC_VA_FS_CONTROL_EN | CDC_VA_FS_COUNTER_CLR,
CDC_VA_FS_CONTROL_EN);
regmap_update_bits(regmap, CDC_VA_TOP_CSR_TOP_CFG0,
--
2.21.0
Add compatible for sm8450 and sc8280xp.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/lpass-wsa-macro.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
index 27da6c6c3c5a..f82c297ea3ab 100644
--- a/sound/soc/codecs/lpass-wsa-macro.c
+++ b/sound/soc/codecs/lpass-wsa-macro.c
@@ -2561,6 +2561,8 @@ static const struct dev_pm_ops wsa_macro_pm_ops = {
static const struct of_device_id wsa_macro_dt_match[] = {
{.compatible = "qcom,sc7280-lpass-wsa-macro"},
{.compatible = "qcom,sm8250-lpass-wsa-macro"},
+ {.compatible = "qcom,sm8450-lpass-wsa-macro"},
+ {.compatible = "qcom,sc8280xp-lpass-wsa-macro" },
{}
};
MODULE_DEVICE_TABLE(of, wsa_macro_dt_match);
--
2.21.0
This patch adds SM8450 and SC8280XP compatible entry for LPASS TX, RX, WSA
and VA codec macros.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
.../devicetree/bindings/sound/qcom,lpass-rx-macro.yaml | 2 ++
.../devicetree/bindings/sound/qcom,lpass-tx-macro.yaml | 2 ++
.../devicetree/bindings/sound/qcom,lpass-va-macro.yaml | 2 ++
.../devicetree/bindings/sound/qcom,lpass-wsa-macro.yaml | 2 ++
4 files changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-rx-macro.yaml b/Documentation/devicetree/bindings/sound/qcom,lpass-rx-macro.yaml
index a6905bcf89d2..1de11e7f33bb 100644
--- a/Documentation/devicetree/bindings/sound/qcom,lpass-rx-macro.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,lpass-rx-macro.yaml
@@ -14,6 +14,8 @@ properties:
enum:
- qcom,sc7280-lpass-rx-macro
- qcom,sm8250-lpass-rx-macro
+ - qcom,sm8450-lpass-rx-macro
+ - qcom,sc8280xp-lpass-rx-macro
reg:
maxItems: 1
diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-tx-macro.yaml b/Documentation/devicetree/bindings/sound/qcom,lpass-tx-macro.yaml
index 324595a62ae8..de8297b358e8 100644
--- a/Documentation/devicetree/bindings/sound/qcom,lpass-tx-macro.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,lpass-tx-macro.yaml
@@ -14,6 +14,8 @@ properties:
enum:
- qcom,sc7280-lpass-tx-macro
- qcom,sm8250-lpass-tx-macro
+ - qcom,sm8450-lpass-tx-macro
+ - qcom,sc8280xp-lpass-tx-macro
reg:
maxItems: 1
diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-va-macro.yaml b/Documentation/devicetree/bindings/sound/qcom,lpass-va-macro.yaml
index 7b4cc84eda8c..9f473c08cb2e 100644
--- a/Documentation/devicetree/bindings/sound/qcom,lpass-va-macro.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,lpass-va-macro.yaml
@@ -14,6 +14,8 @@ properties:
enum:
- qcom,sc7280-lpass-va-macro
- qcom,sm8250-lpass-va-macro
+ - qcom,sm8450-lpass-va-macro
+ - qcom,sc8280xp-lpass-va-macro
reg:
maxItems: 1
diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-wsa-macro.yaml b/Documentation/devicetree/bindings/sound/qcom,lpass-wsa-macro.yaml
index 13cdb8a10687..4959ad658eac 100644
--- a/Documentation/devicetree/bindings/sound/qcom,lpass-wsa-macro.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,lpass-wsa-macro.yaml
@@ -14,6 +14,8 @@ properties:
enum:
- qcom,sc7280-lpass-wsa-macro
- qcom,sm8250-lpass-wsa-macro
+ - qcom,sm8450-lpass-wsa-macro
+ - qcom,sc8280xp-lpass-wsa-macro
reg:
maxItems: 1
--
2.21.0
On Thu, Aug 18, 2022 at 02:46:15PM +0100, Srinivas Kandagatla wrote:
> Add compatible for sm8450 and sc8280xp.
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> sound/soc/codecs/lpass-wsa-macro.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
> index 27da6c6c3c5a..f82c297ea3ab 100644
> --- a/sound/soc/codecs/lpass-wsa-macro.c
> +++ b/sound/soc/codecs/lpass-wsa-macro.c
> @@ -2561,6 +2561,8 @@ static const struct dev_pm_ops wsa_macro_pm_ops = {
> static const struct of_device_id wsa_macro_dt_match[] = {
> {.compatible = "qcom,sc7280-lpass-wsa-macro"},
> {.compatible = "qcom,sm8250-lpass-wsa-macro"},
> + {.compatible = "qcom,sm8450-lpass-wsa-macro"},
> + {.compatible = "qcom,sc8280xp-lpass-wsa-macro" },
Looks like these are backwards compatible with the existing versions,
why not reflect that in the binding?
Rob
On Thu, Aug 18, 2022 at 02:46:14PM +0100, Srinivas Kandagatla wrote:
> This patch adds SM8450 and SC8280XP compatible entry for LPASS TX, RX, WSA
> and VA codec macros.
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> .../devicetree/bindings/sound/qcom,lpass-rx-macro.yaml | 2 ++
> .../devicetree/bindings/sound/qcom,lpass-tx-macro.yaml | 2 ++
> .../devicetree/bindings/sound/qcom,lpass-va-macro.yaml | 2 ++
> .../devicetree/bindings/sound/qcom,lpass-wsa-macro.yaml | 2 ++
> 4 files changed, 8 insertions(+)
Acked-by: Rob Herring <[email protected]>
On 31/08/2022 12:17, Srinivas Kandagatla wrote:
>
>
> On 18/08/2022 18:12, Rob Herring wrote:
>> On Thu, Aug 18, 2022 at 02:46:15PM +0100, Srinivas Kandagatla wrote:
>>> Add compatible for sm8450 and sc8280xp.
>>>
>>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>>> ---
>>> sound/soc/codecs/lpass-wsa-macro.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
>>> index 27da6c6c3c5a..f82c297ea3ab 100644
>>> --- a/sound/soc/codecs/lpass-wsa-macro.c
>>> +++ b/sound/soc/codecs/lpass-wsa-macro.c
>>> @@ -2561,6 +2561,8 @@ static const struct dev_pm_ops wsa_macro_pm_ops = {
>>> static const struct of_device_id wsa_macro_dt_match[] = {
>>> {.compatible = "qcom,sc7280-lpass-wsa-macro"},
>>> {.compatible = "qcom,sm8250-lpass-wsa-macro"},
>>> + {.compatible = "qcom,sm8450-lpass-wsa-macro"},
>>> + {.compatible = "qcom,sc8280xp-lpass-wsa-macro" },
>>
>> Looks like these are backwards compatible with the existing versions,
>> why not reflect that in the binding?
> Backward compatibility is not always true, some of the registers and
> there defaults tend to change across SoCs. Having SoC specific
> compatible could help us deal with this and also make code more inline
> with other codec macros in LPASS IP.
I am not saying that there should be no SoC specific compatible. This
one is a must, but the question why duplicating the entries and not
using fallback?
Best regards,
Krzysztof
On 18/08/2022 18:12, Rob Herring wrote:
> On Thu, Aug 18, 2022 at 02:46:15PM +0100, Srinivas Kandagatla wrote:
>> Add compatible for sm8450 and sc8280xp.
>>
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>> ---
>> sound/soc/codecs/lpass-wsa-macro.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
>> index 27da6c6c3c5a..f82c297ea3ab 100644
>> --- a/sound/soc/codecs/lpass-wsa-macro.c
>> +++ b/sound/soc/codecs/lpass-wsa-macro.c
>> @@ -2561,6 +2561,8 @@ static const struct dev_pm_ops wsa_macro_pm_ops = {
>> static const struct of_device_id wsa_macro_dt_match[] = {
>> {.compatible = "qcom,sc7280-lpass-wsa-macro"},
>> {.compatible = "qcom,sm8250-lpass-wsa-macro"},
>> + {.compatible = "qcom,sm8450-lpass-wsa-macro"},
>> + {.compatible = "qcom,sc8280xp-lpass-wsa-macro" },
>
> Looks like these are backwards compatible with the existing versions,
> why not reflect that in the binding?
Backward compatibility is not always true, some of the registers and
there defaults tend to change across SoCs. Having SoC specific
compatible could help us deal with this and also make code more inline
with other codec macros in LPASS IP.
--srini
>
> Rob
On 31/08/2022 12:19, Krzysztof Kozlowski wrote:
> On 31/08/2022 12:17, Srinivas Kandagatla wrote:
>>
>>
>> On 18/08/2022 18:12, Rob Herring wrote:
>>> On Thu, Aug 18, 2022 at 02:46:15PM +0100, Srinivas Kandagatla wrote:
>>>> Add compatible for sm8450 and sc8280xp.
>>>>
>>>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>>>> ---
>>>> sound/soc/codecs/lpass-wsa-macro.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
>>>> index 27da6c6c3c5a..f82c297ea3ab 100644
>>>> --- a/sound/soc/codecs/lpass-wsa-macro.c
>>>> +++ b/sound/soc/codecs/lpass-wsa-macro.c
>>>> @@ -2561,6 +2561,8 @@ static const struct dev_pm_ops wsa_macro_pm_ops = {
>>>> static const struct of_device_id wsa_macro_dt_match[] = {
>>>> {.compatible = "qcom,sc7280-lpass-wsa-macro"},
>>>> {.compatible = "qcom,sm8250-lpass-wsa-macro"},
>>>> + {.compatible = "qcom,sm8450-lpass-wsa-macro"},
>>>> + {.compatible = "qcom,sc8280xp-lpass-wsa-macro" },
>>>
>>> Looks like these are backwards compatible with the existing versions,
>>> why not reflect that in the binding?
>> Backward compatibility is not always true, some of the registers and
>> there defaults tend to change across SoCs. Having SoC specific
>> compatible could help us deal with this and also make code more inline
>> with other codec macros in LPASS IP.
>
> I am not saying that there should be no SoC specific compatible. This
s/I am/We are/
I really thought that it was my comment. :)
> one is a must, but the question why duplicating the entries and not
> using fallback?
Best regards,
Krzysztof
On 31/08/2022 10:19, Krzysztof Kozlowski wrote:
> On 31/08/2022 12:17, Srinivas Kandagatla wrote:
>>
>>
>> On 18/08/2022 18:12, Rob Herring wrote:
>>> On Thu, Aug 18, 2022 at 02:46:15PM +0100, Srinivas Kandagatla wrote:
>>>> Add compatible for sm8450 and sc8280xp.
>>>>
>>>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>>>> ---
>>>> sound/soc/codecs/lpass-wsa-macro.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
>>>> index 27da6c6c3c5a..f82c297ea3ab 100644
>>>> --- a/sound/soc/codecs/lpass-wsa-macro.c
>>>> +++ b/sound/soc/codecs/lpass-wsa-macro.c
>>>> @@ -2561,6 +2561,8 @@ static const struct dev_pm_ops wsa_macro_pm_ops = {
>>>> static const struct of_device_id wsa_macro_dt_match[] = {
>>>> {.compatible = "qcom,sc7280-lpass-wsa-macro"},
>>>> {.compatible = "qcom,sm8250-lpass-wsa-macro"},
>>>> + {.compatible = "qcom,sm8450-lpass-wsa-macro"},
>>>> + {.compatible = "qcom,sc8280xp-lpass-wsa-macro" },
>>>
>>> Looks like these are backwards compatible with the existing versions,
>>> why not reflect that in the binding?
>> Backward compatibility is not always true, some of the registers and
>> there defaults tend to change across SoCs. Having SoC specific
>> compatible could help us deal with this and also make code more inline
>> with other codec macros in LPASS IP.
>
> I am not saying that there should be no SoC specific compatible. This
> one is a must, but the question why duplicating the entries and not
> using fallback?
You mean using fallback compatible "qcom,sc7280-lpass-wsa-macro" in
sc8280xp devicetree and not add new compatibles in the driver?
The reason for adding this new compatible strings is that macros in this
lpass codec that differ form each SoC.
ex: [PATCH 6/6] ASoC: codecs: tx-macro: add support for sm8450 and
sc8280xp and there is a pending patch on va-macro that has soundwire
controller frame sync and reset control which is moved from tx-macro to
va-macro.
so DT might endup with mix of compatibles for same LPASS Codec like this:
"qcom,sc7280-lpass-wsa-macro"
"qcom,sc8280xp-lpass-va-macro"
"qcom,sc8280xp-lpass-tx-macro"
"qcom,sc8280xp-lpass-rx-macro"
AFAIU, the fallback thing will work for things that are identical but in
this case they differ across SoCs, and having SoC specific compatibles
in now would help handle this.
thanks,
srini
>
> Best regards,
> Krzysztof
On 31/08/2022 13:37, Srinivas Kandagatla wrote:
>
>
> On 31/08/2022 10:19, Krzysztof Kozlowski wrote:
>> On 31/08/2022 12:17, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 18/08/2022 18:12, Rob Herring wrote:
>>>> On Thu, Aug 18, 2022 at 02:46:15PM +0100, Srinivas Kandagatla wrote:
>>>>> Add compatible for sm8450 and sc8280xp.
>>>>>
>>>>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>>>>> ---
>>>>> sound/soc/codecs/lpass-wsa-macro.c | 2 ++
>>>>> 1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
>>>>> index 27da6c6c3c5a..f82c297ea3ab 100644
>>>>> --- a/sound/soc/codecs/lpass-wsa-macro.c
>>>>> +++ b/sound/soc/codecs/lpass-wsa-macro.c
>>>>> @@ -2561,6 +2561,8 @@ static const struct dev_pm_ops wsa_macro_pm_ops = {
>>>>> static const struct of_device_id wsa_macro_dt_match[] = {
>>>>> {.compatible = "qcom,sc7280-lpass-wsa-macro"},
>>>>> {.compatible = "qcom,sm8250-lpass-wsa-macro"},
>>>>> + {.compatible = "qcom,sm8450-lpass-wsa-macro"},
>>>>> + {.compatible = "qcom,sc8280xp-lpass-wsa-macro" },
>>>>
>>>> Looks like these are backwards compatible with the existing versions,
>>>> why not reflect that in the binding?
>>> Backward compatibility is not always true, some of the registers and
>>> there defaults tend to change across SoCs. Having SoC specific
>>> compatible could help us deal with this and also make code more inline
>>> with other codec macros in LPASS IP.
>>
>> I am not saying that there should be no SoC specific compatible. This
>> one is a must, but the question why duplicating the entries and not
>> using fallback?
>
> You mean using fallback compatible "qcom,sc7280-lpass-wsa-macro" in
> sc8280xp devicetree and not add new compatibles in the driver?
>
> The reason for adding this new compatible strings is that macros in this
> lpass codec that differ form each SoC.
> ex: [PATCH 6/6] ASoC: codecs: tx-macro: add support for sm8450 and
> sc8280xp and there is a pending patch on va-macro that has soundwire
> controller frame sync and reset control which is moved from tx-macro to
> va-macro.
>
> so DT might endup with mix of compatibles for same LPASS Codec like this:
>
> "qcom,sc7280-lpass-wsa-macro"
> "qcom,sc8280xp-lpass-va-macro"
> "qcom,sc8280xp-lpass-tx-macro"
> "qcom,sc8280xp-lpass-rx-macro"
>
> AFAIU, the fallback thing will work for things that are identical but in
> this case they differ across SoCs, and having SoC specific compatibles
> in now would help handle this.
Ahh, I see now. The true problem is that driver encodes compatibles in
several places. That's very confusing design - variants should be rather
customized via driver data, not via multiple of_device_is_compatible()
inside the code.
Best regards,
Krzysztof
On 18/08/2022 16:46, Srinivas Kandagatla wrote:
> LPASS VA Macro now has soundwire master to deal with access to
> analog mic in low power island use cases.
>
> This is added after sc8280xp, add support for this.
> Along with this also add compatibles for sm8450 and sc8280xp.
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> sound/soc/codecs/lpass-va-macro.c | 64 ++++++++++++++++++++++++++++++-
> 1 file changed, 62 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
> index a35f684053d2..f8b0c8caa1db 100644
> --- a/sound/soc/codecs/lpass-va-macro.c
> +++ b/sound/soc/codecs/lpass-va-macro.c
> @@ -25,6 +25,10 @@
> #define CDC_VA_FS_CONTROL_EN BIT(0)
> #define CDC_VA_FS_COUNTER_CLR BIT(1)
> #define CDC_VA_CLK_RST_CTRL_SWR_CONTROL (0x0008)
> +#define CDC_VA_SWR_RESET_MASK BIT(1)
> +#define CDC_VA_SWR_RESET_ENABLE BIT(1)
> +#define CDC_VA_SWR_CLK_EN_MASK BIT(0)
> +#define CDC_VA_SWR_CLK_ENABLE BIT(0)
> #define CDC_VA_TOP_CSR_TOP_CFG0 (0x0080)
> #define CDC_VA_FS_BROADCAST_EN BIT(1)
> #define CDC_VA_TOP_CSR_DMIC0_CTL (0x0084)
> @@ -66,6 +70,8 @@
> #define CDC_VA_TOP_CSR_SWR_MIC_CTL0 (0x00D0)
> #define CDC_VA_TOP_CSR_SWR_MIC_CTL1 (0x00D4)
> #define CDC_VA_TOP_CSR_SWR_MIC_CTL2 (0x00D8)
> +#define CDC_VA_SWR_MIC_CLK_SEL_0_1_MASK (0xEE)
> +#define CDC_VA_SWR_MIC_CLK_SEL_0_1_DIV1 (0xCC)
> #define CDC_VA_TOP_CSR_SWR_CTRL (0x00DC)
> #define CDC_VA_INP_MUX_ADC_MUX0_CFG0 (0x0100)
> #define CDC_VA_INP_MUX_ADC_MUX0_CFG1 (0x0104)
> @@ -194,6 +200,8 @@ struct va_macro {
> unsigned long active_ch_mask[VA_MACRO_MAX_DAIS];
> unsigned long active_ch_cnt[VA_MACRO_MAX_DAIS];
> u16 dmic_clk_div;
> + bool has_swr_master;
> + bool reset_swr;
>
> int dec_mode[VA_MACRO_NUM_DECIMATORS];
> struct regmap *regmap;
> @@ -325,6 +333,9 @@ static bool va_is_rw_register(struct device *dev, unsigned int reg)
> case CDC_VA_TOP_CSR_DMIC2_CTL:
> case CDC_VA_TOP_CSR_DMIC3_CTL:
> case CDC_VA_TOP_CSR_DMIC_CFG:
> + case CDC_VA_TOP_CSR_SWR_MIC_CTL0:
> + case CDC_VA_TOP_CSR_SWR_MIC_CTL1:
> + case CDC_VA_TOP_CSR_SWR_MIC_CTL2:
> case CDC_VA_TOP_CSR_DEBUG_BUS:
> case CDC_VA_TOP_CSR_DEBUG_EN:
> case CDC_VA_TOP_CSR_TX_I2S_CTL:
> @@ -1306,12 +1317,40 @@ static const struct snd_soc_component_driver va_macro_component_drv = {
>
> static int fsgen_gate_enable(struct clk_hw *hw)
> {
> - return va_macro_mclk_enable(to_va_macro(hw), true);
> + struct va_macro *va = to_va_macro(hw);
> + struct regmap *regmap = va->regmap;
> + int ret;
> +
> + ret = va_macro_mclk_enable(va, true);
> + if (!va->has_swr_master)
> + return ret;
> +
> + if (va->reset_swr)
> + regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
> + CDC_VA_SWR_RESET_MASK,
> + CDC_VA_SWR_RESET_ENABLE);
> +
> + regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
> + CDC_VA_SWR_CLK_EN_MASK,
> + CDC_VA_SWR_CLK_ENABLE);
> + if (va->reset_swr)
> + regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
> + CDC_VA_SWR_RESET_MASK, 0x0);
> + va->reset_swr = false;
> +
> + return ret;
> }
>
> static void fsgen_gate_disable(struct clk_hw *hw)
> {
> - va_macro_mclk_enable(to_va_macro(hw), false);
> + struct va_macro *va = to_va_macro(hw);
> + struct regmap *regmap = va->regmap;
> +
> + if (va->has_swr_master)
> + regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
> + CDC_VA_SWR_CLK_EN_MASK, 0x0);
> +
> + va_macro_mclk_enable(va, false);
> }
>
> static int fsgen_gate_is_enabled(struct clk_hw *hw)
> @@ -1459,6 +1498,11 @@ static int va_macro_probe(struct platform_device *pdev)
>
> dev_set_drvdata(dev, va);
>
> + if (of_device_is_compatible(dev->of_node, "qcom,sm8450-lpass-va-macro") ||
> + of_device_is_compatible(dev->of_node, "qcom,sc8280xp-lpass-va-macro")) {
> + va->has_swr_master = true;
> + va->reset_swr = true;
This should go to driver_data. Either via quirks/flags or device type
(enum for each device). Usually the first (flags) is more flexible if
you want to support many devices.
This also explains Rob's concerns about unneeded entries in of_device_id
table.
Best regards,
Krzysztof
On 01/09/2022 08:30, Krzysztof Kozlowski wrote:
> On 18/08/2022 16:46, Srinivas Kandagatla wrote:
>> LPASS VA Macro now has soundwire master to deal with access to
>> analog mic in low power island use cases.
>>
>> This is added after sc8280xp, add support for this.
>> Along with this also add compatibles for sm8450 and sc8280xp.
>>
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>> ---
>> sound/soc/codecs/lpass-va-macro.c | 64 ++++++++++++++++++++++++++++++-
>> 1 file changed, 62 insertions(+), 2 deletions(-)
>>
>> diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
>> index a35f684053d2..f8b0c8caa1db 100644
>> --- a/sound/soc/codecs/lpass-va-macro.c
>> +++ b/sound/soc/codecs/lpass-va-macro.c
>> @@ -25,6 +25,10 @@
>> #define CDC_VA_FS_CONTROL_EN BIT(0)
>> #define CDC_VA_FS_COUNTER_CLR BIT(1)
>> #define CDC_VA_CLK_RST_CTRL_SWR_CONTROL (0x0008)
>> +#define CDC_VA_SWR_RESET_MASK BIT(1)
>> +#define CDC_VA_SWR_RESET_ENABLE BIT(1)
>> +#define CDC_VA_SWR_CLK_EN_MASK BIT(0)
>> +#define CDC_VA_SWR_CLK_ENABLE BIT(0)
>> #define CDC_VA_TOP_CSR_TOP_CFG0 (0x0080)
>> #define CDC_VA_FS_BROADCAST_EN BIT(1)
>> #define CDC_VA_TOP_CSR_DMIC0_CTL (0x0084)
>> @@ -66,6 +70,8 @@
>> #define CDC_VA_TOP_CSR_SWR_MIC_CTL0 (0x00D0)
>> #define CDC_VA_TOP_CSR_SWR_MIC_CTL1 (0x00D4)
>> #define CDC_VA_TOP_CSR_SWR_MIC_CTL2 (0x00D8)
>> +#define CDC_VA_SWR_MIC_CLK_SEL_0_1_MASK (0xEE)
>> +#define CDC_VA_SWR_MIC_CLK_SEL_0_1_DIV1 (0xCC)
>> #define CDC_VA_TOP_CSR_SWR_CTRL (0x00DC)
>> #define CDC_VA_INP_MUX_ADC_MUX0_CFG0 (0x0100)
>> #define CDC_VA_INP_MUX_ADC_MUX0_CFG1 (0x0104)
>> @@ -194,6 +200,8 @@ struct va_macro {
>> unsigned long active_ch_mask[VA_MACRO_MAX_DAIS];
>> unsigned long active_ch_cnt[VA_MACRO_MAX_DAIS];
>> u16 dmic_clk_div;
>> + bool has_swr_master;
>> + bool reset_swr;
>>
>> int dec_mode[VA_MACRO_NUM_DECIMATORS];
>> struct regmap *regmap;
>> @@ -325,6 +333,9 @@ static bool va_is_rw_register(struct device *dev, unsigned int reg)
>> case CDC_VA_TOP_CSR_DMIC2_CTL:
>> case CDC_VA_TOP_CSR_DMIC3_CTL:
>> case CDC_VA_TOP_CSR_DMIC_CFG:
>> + case CDC_VA_TOP_CSR_SWR_MIC_CTL0:
>> + case CDC_VA_TOP_CSR_SWR_MIC_CTL1:
>> + case CDC_VA_TOP_CSR_SWR_MIC_CTL2:
>> case CDC_VA_TOP_CSR_DEBUG_BUS:
>> case CDC_VA_TOP_CSR_DEBUG_EN:
>> case CDC_VA_TOP_CSR_TX_I2S_CTL:
>> @@ -1306,12 +1317,40 @@ static const struct snd_soc_component_driver va_macro_component_drv = {
>>
>> static int fsgen_gate_enable(struct clk_hw *hw)
>> {
>> - return va_macro_mclk_enable(to_va_macro(hw), true);
>> + struct va_macro *va = to_va_macro(hw);
>> + struct regmap *regmap = va->regmap;
>> + int ret;
>> +
>> + ret = va_macro_mclk_enable(va, true);
>> + if (!va->has_swr_master)
>> + return ret;
>> +
>> + if (va->reset_swr)
>> + regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
>> + CDC_VA_SWR_RESET_MASK,
>> + CDC_VA_SWR_RESET_ENABLE);
>> +
>> + regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
>> + CDC_VA_SWR_CLK_EN_MASK,
>> + CDC_VA_SWR_CLK_ENABLE);
>> + if (va->reset_swr)
>> + regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
>> + CDC_VA_SWR_RESET_MASK, 0x0);
>> + va->reset_swr = false;
>> +
>> + return ret;
>> }
>>
>> static void fsgen_gate_disable(struct clk_hw *hw)
>> {
>> - va_macro_mclk_enable(to_va_macro(hw), false);
>> + struct va_macro *va = to_va_macro(hw);
>> + struct regmap *regmap = va->regmap;
>> +
>> + if (va->has_swr_master)
>> + regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
>> + CDC_VA_SWR_CLK_EN_MASK, 0x0);
>> +
>> + va_macro_mclk_enable(va, false);
>> }
>>
>> static int fsgen_gate_is_enabled(struct clk_hw *hw)
>> @@ -1459,6 +1498,11 @@ static int va_macro_probe(struct platform_device *pdev)
>>
>> dev_set_drvdata(dev, va);
>>
>> + if (of_device_is_compatible(dev->of_node, "qcom,sm8450-lpass-va-macro") ||
>> + of_device_is_compatible(dev->of_node, "qcom,sc8280xp-lpass-va-macro")) {
>> + va->has_swr_master = true;
>> + va->reset_swr = true;
>
> This should go to driver_data. Either via quirks/flags or device type
> (enum for each device). Usually the first (flags) is more flexible if
> you want to support many devices.
Yes, at-least this case is easily doable with driver_data, let me try to
add this in next spin.
--srini
>
> This also explains Rob's concerns about unneeded entries in of_device_id
> table.
>
>
> Best regards,
> Krzysztof