[PATCH 0/6] ASoC: codecs: lpass: add support fro sm8450 and sc8280xp

[PATCH 0/6] ASoC: codecs: lpass: add support fro sm8450 and sc8280xp

[Srinivas Kandagatla]

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

[PATCH 1/6] ASoC: qcom: dt-bindings: add sm8450 and sc8280xp compatibles

[Srinivas Kandagatla]

This patch adds SM8450 and SC8280XP compatible entry for LPASS TX, RX, WSA
and VA codec macros.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 .../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

[PATCH 2/6] ASoC: codecs: wsa-macro: add support for sm8450 and sc8280xp

[Srinivas Kandagatla]

Add compatible for sm8450 and sc8280xp.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 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

[PATCH 3/6] ASoC: codecs: tx-macro: add support for sm8450 and sc8280xp

[Srinivas Kandagatla]

Add compatible for sm8450 and sc8280xp.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 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

[PATCH 4/6] ASoC: codecs: rx-macro: add support for sm8450 and sc8280xp

[Srinivas Kandagatla]

Add compatible for sm8450 and sc8280xp.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/codecs/lpass-rx-macro.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
index 3143f9cd7277..fa8a174ba7c0 100644
--- a/sound/soc/codecs/lpass-rx-macro.c
+++ b/sound/soc/codecs/lpass-rx-macro.c
@@ -3659,6 +3659,8 @@ static int rx_macro_remove(struct platform_device *pdev)
 static const struct of_device_id rx_macro_dt_match[] = {
 	{ .compatible = "qcom,sc7280-lpass-rx-macro" },
 	{ .compatible = "qcom,sm8250-lpass-rx-macro" },
+	{ .compatible = "qcom,sm8450-lpass-rx-macro" },
+	{ .compatible = "qcom,sc8280xp-lpass-rx-macro" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, rx_macro_dt_match);
-- 
2.21.0

[PATCH 5/6] ASoC: codecs: va-macro: clear the frame sync counter before enabling

[Srinivas Kandagatla]

Clear the frame sync counter before enabling it.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 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

[PATCH 6/6] ASoC: codecs: tx-macro: add support for sm8450 and sc8280xp

[Srinivas Kandagatla]

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 <srinivas.kandagatla@linaro.org>
---
 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

Re: [PATCH 1/6] ASoC: qcom: dt-bindings: add sm8450 and sc8280xp compatibles

[Rob Herring]

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 <srinivas.kandagatla@linaro.org>
> ---
>  .../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 <robh@kernel.org>

Re: [PATCH 2/6] ASoC: codecs: wsa-macro: add support for sm8450 and sc8280xp

[Rob Herring]

On Thu, Aug 18, 2022 at 02:46:15PM +0100, Srinivas Kandagatla wrote:
> Add compatible for sm8450 and sc8280xp.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  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

Re: [PATCH 2/6] ASoC: codecs: wsa-macro: add support for sm8450 and sc8280xp

[Srinivas Kandagatla]

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 <srinivas.kandagatla@linaro.org>
>> ---
>>   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

Re: [PATCH 2/6] ASoC: codecs: wsa-macro: add support for sm8450 and sc8280xp

[Krzysztof Kozlowski]

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 <srinivas.kandagatla@linaro.org>
>>> ---
>>>   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

Re: [PATCH 2/6] ASoC: codecs: wsa-macro: add support for sm8450 and sc8280xp

[Krzysztof Kozlowski]

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 <srinivas.kandagatla@linaro.org>
>>>> ---
>>>>   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

Re: [PATCH 2/6] ASoC: codecs: wsa-macro: add support for sm8450 and sc8280xp

[Srinivas Kandagatla]

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 <srinivas.kandagatla@linaro.org>
>>>> ---
>>>>    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

Re: [PATCH 2/6] ASoC: codecs: wsa-macro: add support for sm8450 and sc8280xp

[Krzysztof Kozlowski]

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 <srinivas.kandagatla@linaro.org>
>>>>> ---
>>>>>    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

Re: [PATCH 6/6] ASoC: codecs: tx-macro: add support for sm8450 and sc8280xp

[Krzysztof Kozlowski]

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 <srinivas.kandagatla@linaro.org>
> ---
>  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

Re: [PATCH 6/6] ASoC: codecs: tx-macro: add support for sm8450 and sc8280xp

[Srinivas Kandagatla]

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 <srinivas.kandagatla@linaro.org>
>> ---
>>   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