diff mbox series

[02/13] ASoC: amd: acp: refactor acp i2s clock generation code

Message ID 20231021145110.478744-2-Syed.SabaKareem@amd.com
State Accepted
Commit 40f74d5f09d7c068bd7a980dc06a688dc8d2b3e3
Headers show
Series None | expand

Commit Message

Saba Kareem, Syed Oct. 21, 2023, 2:50 p.m. UTC 
Refactor acp i2s LRCLK,BCLK generation code and move to commnon file.

Signed-off-by: Syed Saba Kareem <Syed.SabaKareem@amd.com>
---
 sound/soc/amd/acp/acp-i2s.c | 32 ++++++++++++++++++++++++++++++
 sound/soc/amd/acp/amd.h     | 39 -------------------------------------
 2 files changed, 32 insertions(+), 39 deletions(-)

Comments

Amadeusz Sławiński Oct. 23, 2023, 7:37 a.m. UTC | #1 
On 10/21/2023 4:50 PM, Syed Saba Kareem wrote:
> Refactor acp i2s LRCLK,BCLK generation code and move to commnon file.
> 
> Signed-off-by: Syed Saba Kareem <Syed.SabaKareem@amd.com>
> ---
>   sound/soc/amd/acp/acp-i2s.c | 32 ++++++++++++++++++++++++++++++
>   sound/soc/amd/acp/amd.h     | 39 -------------------------------------
>   2 files changed, 32 insertions(+), 39 deletions(-)
> 
> diff --git a/sound/soc/amd/acp/acp-i2s.c b/sound/soc/amd/acp/acp-i2s.c
> index df350014966a..59d3a499771a 100644
> --- a/sound/soc/amd/acp/acp-i2s.c
> +++ b/sound/soc/amd/acp/acp-i2s.c
> @@ -20,10 +20,42 @@
>   #include <sound/soc.h>
>   #include <sound/soc-dai.h>
>   #include <linux/dma-mapping.h>
> +#include <linux/bitfield.h>
>   
>   #include "amd.h"
>   
>   #define DRV_NAME "acp_i2s_playcap"
> +#define	I2S_MASTER_MODE_ENABLE		1
> +#define	I2S_MODE_ENABLE			0
> +#define	I2S_FORMAT_MODE			GENMASK(1, 1)
> +#define	LRCLK_DIV_FIELD			GENMASK(10, 2)
> +#define	BCLK_DIV_FIELD			GENMASK(23, 11)
> +
> +static inline void acp_set_i2s_clk(struct acp_dev_data *adata, int dai_id)
> +{
> +	u32 i2s_clk_reg, val;
> +
> +	switch (dai_id) {
> +	case I2S_SP_INSTANCE:
> +		i2s_clk_reg = ACP_I2STDM0_MSTRCLKGEN;
> +		break;
> +	case I2S_BT_INSTANCE:
> +		i2s_clk_reg = ACP_I2STDM1_MSTRCLKGEN;
> +		break;
> +	case I2S_HS_INSTANCE:
> +		i2s_clk_reg = ACP_I2STDM2_MSTRCLKGEN;
> +		break;
> +	default:
> +		i2s_clk_reg = ACP_I2STDM0_MSTRCLKGEN;
> +		break;
> +	}
> +
> +	val  = I2S_MASTER_MODE_ENABLE;
> +	val |= I2S_MODE_ENABLE & BIT(1);

There is I2S_FORMAT_MODE define, you probably want to use it instead of 
BIT(1), so there is no "magic number" mask?

> +	val |= FIELD_PREP(LRCLK_DIV_FIELD, adata->lrclk_div);
> +	val |= FIELD_PREP(BCLK_DIV_FIELD, adata->bclk_div);
> +	writel(val, adata->acp_base + i2s_clk_reg);
> +}
>   
>   static int acp_i2s_set_fmt(struct snd_soc_dai *cpu_dai,
>   			   unsigned int fmt)
> diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h
> index 487eefa5985f..87d1e1f7d6b6 100644
> --- a/sound/soc/amd/acp/amd.h
> +++ b/sound/soc/amd/acp/amd.h
> @@ -188,17 +188,6 @@ struct acp_dev_data {
>   	u32 xfer_rx_resolution[3];
>   };
>   
> -union acp_i2stdm_mstrclkgen {
> -	struct {
> -		u32 i2stdm_master_mode : 1;
> -		u32 i2stdm_format_mode : 1;
> -		u32 i2stdm_lrclk_div_val : 9;
> -		u32 i2stdm_bclk_div_val : 11;
> -		u32:10;
> -	} bitfields, bits;
> -	u32  u32_all;
> -};
> -
>   extern const struct snd_soc_dai_ops asoc_acp_cpu_dai_ops;
>   extern const struct snd_soc_dai_ops acp_dmic_dai_ops;
>   
> @@ -276,32 +265,4 @@ static inline u64 acp_get_byte_count(struct acp_dev_data *adata, int dai_id, int
>   POINTER_RETURN_BYTES:
>   	return byte_count;
>   }
> -
> -static inline void acp_set_i2s_clk(struct acp_dev_data *adata, int dai_id)
> -{
> -	union acp_i2stdm_mstrclkgen mclkgen;
> -	u32 master_reg;
> -
> -	switch (dai_id) {
> -	case I2S_SP_INSTANCE:
> -		master_reg = ACP_I2STDM0_MSTRCLKGEN;
> -		break;
> -	case I2S_BT_INSTANCE:
> -		master_reg = ACP_I2STDM1_MSTRCLKGEN;
> -		break;
> -	case I2S_HS_INSTANCE:
> -		master_reg = ACP_I2STDM2_MSTRCLKGEN;
> -		break;
> -	default:
> -		master_reg = ACP_I2STDM0_MSTRCLKGEN;
> -		break;
> -	}
> -
> -	mclkgen.bits.i2stdm_master_mode = 0x1;
> -	mclkgen.bits.i2stdm_format_mode = 0x00;
> -
> -	mclkgen.bits.i2stdm_bclk_div_val = adata->bclk_div;
> -	mclkgen.bits.i2stdm_lrclk_div_val = adata->lrclk_div;
> -	writel(mclkgen.u32_all, adata->acp_base + master_reg);
> -}
>   #endif
syed saba kareem Oct. 26, 2023, 10:01 a.m. UTC | #2 
On 10/23/23 13:07, Amadeusz Sławiński wrote:
> On 10/21/2023 4:50 PM, Syed Saba Kareem wrote:
>> Refactor acp i2s LRCLK,BCLK generation code and move to commnon file.
>>
>> Signed-off-by: Syed Saba Kareem <Syed.SabaKareem@amd.com>
>> ---
>>   sound/soc/amd/acp/acp-i2s.c | 32 ++++++++++++++++++++++++++++++
>>   sound/soc/amd/acp/amd.h     | 39 -------------------------------------
>>   2 files changed, 32 insertions(+), 39 deletions(-)
>>
>> diff --git a/sound/soc/amd/acp/acp-i2s.c b/sound/soc/amd/acp/acp-i2s.c
>> index df350014966a..59d3a499771a 100644
>> --- a/sound/soc/amd/acp/acp-i2s.c
>> +++ b/sound/soc/amd/acp/acp-i2s.c
>> @@ -20,10 +20,42 @@
>>   #include <sound/soc.h>
>>   #include <sound/soc-dai.h>
>>   #include <linux/dma-mapping.h>
>> +#include <linux/bitfield.h>
>>     #include "amd.h"
>>     #define DRV_NAME "acp_i2s_playcap"
>> +#define    I2S_MASTER_MODE_ENABLE        1
>> +#define    I2S_MODE_ENABLE            0
>> +#define    I2S_FORMAT_MODE            GENMASK(1, 1)
>> +#define    LRCLK_DIV_FIELD            GENMASK(10, 2)
>> +#define    BCLK_DIV_FIELD            GENMASK(23, 11)
>> +
>> +static inline void acp_set_i2s_clk(struct acp_dev_data *adata, int 
>> dai_id)
>> +{
>> +    u32 i2s_clk_reg, val;
>> +
>> +    switch (dai_id) {
>> +    case I2S_SP_INSTANCE:
>> +        i2s_clk_reg = ACP_I2STDM0_MSTRCLKGEN;
>> +        break;
>> +    case I2S_BT_INSTANCE:
>> +        i2s_clk_reg = ACP_I2STDM1_MSTRCLKGEN;
>> +        break;
>> +    case I2S_HS_INSTANCE:
>> +        i2s_clk_reg = ACP_I2STDM2_MSTRCLKGEN;
>> +        break;
>> +    default:
>> +        i2s_clk_reg = ACP_I2STDM0_MSTRCLKGEN;
>> +        break;
>> +    }
>> +
>> +    val  = I2S_MASTER_MODE_ENABLE;
>> +    val |= I2S_MODE_ENABLE & BIT(1);
>
> There is I2S_FORMAT_MODE define, you probably want to use it instead 
> of BIT(1), so there is no "magic number" mask?
>
This has to be corrected.
For TDM mode case , bit position 1 should be set.
     if (adata->tdm_mode)
      val |= BIT(1);

I2S_MODE_ENABLE , I2S_FORMAT_MODE macros can be dropped.
Will push the changes as an incremental patch.


>> +    val |= FIELD_PREP(LRCLK_DIV_FIELD, adata->lrclk_div);
>> +    val |= FIELD_PREP(BCLK_DIV_FIELD, adata->bclk_div);
>> +    writel(val, adata->acp_base + i2s_clk_reg);
>> +}
>>     static int acp_i2s_set_fmt(struct snd_soc_dai *cpu_dai,
>>                  unsigned int fmt)
>> diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h
>> index 487eefa5985f..87d1e1f7d6b6 100644
>> --- a/sound/soc/amd/acp/amd.h
>> +++ b/sound/soc/amd/acp/amd.h
>> @@ -188,17 +188,6 @@ struct acp_dev_data {
>>       u32 xfer_rx_resolution[3];
>>   };
>>   -union acp_i2stdm_mstrclkgen {
>> -    struct {
>> -        u32 i2stdm_master_mode : 1;
>> -        u32 i2stdm_format_mode : 1;
>> -        u32 i2stdm_lrclk_div_val : 9;
>> -        u32 i2stdm_bclk_div_val : 11;
>> -        u32:10;
>> -    } bitfields, bits;
>> -    u32  u32_all;
>> -};
>> -
>>   extern const struct snd_soc_dai_ops asoc_acp_cpu_dai_ops;
>>   extern const struct snd_soc_dai_ops acp_dmic_dai_ops;
>>   @@ -276,32 +265,4 @@ static inline u64 acp_get_byte_count(struct 
>> acp_dev_data *adata, int dai_id, int
>>   POINTER_RETURN_BYTES:
>>       return byte_count;
>>   }
>> -
>> -static inline void acp_set_i2s_clk(struct acp_dev_data *adata, int 
>> dai_id)
>> -{
>> -    union acp_i2stdm_mstrclkgen mclkgen;
>> -    u32 master_reg;
>> -
>> -    switch (dai_id) {
>> -    case I2S_SP_INSTANCE:
>> -        master_reg = ACP_I2STDM0_MSTRCLKGEN;
>> -        break;
>> -    case I2S_BT_INSTANCE:
>> -        master_reg = ACP_I2STDM1_MSTRCLKGEN;
>> -        break;
>> -    case I2S_HS_INSTANCE:
>> -        master_reg = ACP_I2STDM2_MSTRCLKGEN;
>> -        break;
>> -    default:
>> -        master_reg = ACP_I2STDM0_MSTRCLKGEN;
>> -        break;
>> -    }
>> -
>> -    mclkgen.bits.i2stdm_master_mode = 0x1;
>> -    mclkgen.bits.i2stdm_format_mode = 0x00;
>> -
>> -    mclkgen.bits.i2stdm_bclk_div_val = adata->bclk_div;
>> -    mclkgen.bits.i2stdm_lrclk_div_val = adata->lrclk_div;
>> -    writel(mclkgen.u32_all, adata->acp_base + master_reg);
>> -}
>>   #endif
>
diff mbox series

Patch

diff --git a/sound/soc/amd/acp/acp-i2s.c b/sound/soc/amd/acp/acp-i2s.c
index df350014966a..59d3a499771a 100644
--- a/sound/soc/amd/acp/acp-i2s.c
+++ b/sound/soc/amd/acp/acp-i2s.c
@@ -20,10 +20,42 @@ 
 #include <sound/soc.h>
 #include <sound/soc-dai.h>
 #include <linux/dma-mapping.h>
+#include <linux/bitfield.h>
 
 #include "amd.h"
 
 #define DRV_NAME "acp_i2s_playcap"
+#define	I2S_MASTER_MODE_ENABLE		1
+#define	I2S_MODE_ENABLE			0
+#define	I2S_FORMAT_MODE			GENMASK(1, 1)
+#define	LRCLK_DIV_FIELD			GENMASK(10, 2)
+#define	BCLK_DIV_FIELD			GENMASK(23, 11)
+
+static inline void acp_set_i2s_clk(struct acp_dev_data *adata, int dai_id)
+{
+	u32 i2s_clk_reg, val;
+
+	switch (dai_id) {
+	case I2S_SP_INSTANCE:
+		i2s_clk_reg = ACP_I2STDM0_MSTRCLKGEN;
+		break;
+	case I2S_BT_INSTANCE:
+		i2s_clk_reg = ACP_I2STDM1_MSTRCLKGEN;
+		break;
+	case I2S_HS_INSTANCE:
+		i2s_clk_reg = ACP_I2STDM2_MSTRCLKGEN;
+		break;
+	default:
+		i2s_clk_reg = ACP_I2STDM0_MSTRCLKGEN;
+		break;
+	}
+
+	val  = I2S_MASTER_MODE_ENABLE;
+	val |= I2S_MODE_ENABLE & BIT(1);
+	val |= FIELD_PREP(LRCLK_DIV_FIELD, adata->lrclk_div);
+	val |= FIELD_PREP(BCLK_DIV_FIELD, adata->bclk_div);
+	writel(val, adata->acp_base + i2s_clk_reg);
+}
 
 static int acp_i2s_set_fmt(struct snd_soc_dai *cpu_dai,
 			   unsigned int fmt)
diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h
index 487eefa5985f..87d1e1f7d6b6 100644
--- a/sound/soc/amd/acp/amd.h
+++ b/sound/soc/amd/acp/amd.h
@@ -188,17 +188,6 @@  struct acp_dev_data {
 	u32 xfer_rx_resolution[3];
 };
 
-union acp_i2stdm_mstrclkgen {
-	struct {
-		u32 i2stdm_master_mode : 1;
-		u32 i2stdm_format_mode : 1;
-		u32 i2stdm_lrclk_div_val : 9;
-		u32 i2stdm_bclk_div_val : 11;
-		u32:10;
-	} bitfields, bits;
-	u32  u32_all;
-};
-
 extern const struct snd_soc_dai_ops asoc_acp_cpu_dai_ops;
 extern const struct snd_soc_dai_ops acp_dmic_dai_ops;
 
@@ -276,32 +265,4 @@  static inline u64 acp_get_byte_count(struct acp_dev_data *adata, int dai_id, int
 POINTER_RETURN_BYTES:
 	return byte_count;
 }
-
-static inline void acp_set_i2s_clk(struct acp_dev_data *adata, int dai_id)
-{
-	union acp_i2stdm_mstrclkgen mclkgen;
-	u32 master_reg;
-
-	switch (dai_id) {
-	case I2S_SP_INSTANCE:
-		master_reg = ACP_I2STDM0_MSTRCLKGEN;
-		break;
-	case I2S_BT_INSTANCE:
-		master_reg = ACP_I2STDM1_MSTRCLKGEN;
-		break;
-	case I2S_HS_INSTANCE:
-		master_reg = ACP_I2STDM2_MSTRCLKGEN;
-		break;
-	default:
-		master_reg = ACP_I2STDM0_MSTRCLKGEN;
-		break;
-	}
-
-	mclkgen.bits.i2stdm_master_mode = 0x1;
-	mclkgen.bits.i2stdm_format_mode = 0x00;
-
-	mclkgen.bits.i2stdm_bclk_div_val = adata->bclk_div;
-	mclkgen.bits.i2stdm_lrclk_div_val = adata->lrclk_div;
-	writel(mclkgen.u32_all, adata->acp_base + master_reg);
-}
 #endif