diff mbox series

[v4,3/3] update tas27xx.h to support either TAS2764 or TAS2780

Message ID 20220323042644.635-3-13691752556@139.com (mailing list archive)
State New, archived
Headers show
Series [v4,1/3] rename tas2764 to tas27xx-Makefile and Kconfig | expand

Commit Message

Raphael-Xu March 23, 2022, 4:26 a.m. UTC 
Signed-off-by: Raphael-Xu <13691752556@139.com>
---
 sound/soc/codecs/tas27xx.h | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

Comments

Amadeusz Sławiński March 23, 2022, 8:29 a.m. UTC | #1 
On 3/23/2022 5:26 AM, Raphael-Xu wrote:
> Signed-off-by: Raphael-Xu <13691752556@139.com>
> ---
>   sound/soc/codecs/tas27xx.h | 27 +++++++++++++++++++--------
>   1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/sound/soc/codecs/tas27xx.h b/sound/soc/codecs/tas27xx.h
> index 67d6fd903c42..02b29c030d37 100644
> --- a/sound/soc/codecs/tas27xx.h
> +++ b/sound/soc/codecs/tas27xx.h
> @@ -1,18 +1,20 @@
>   /* SPDX-License-Identifier: GPL-2.0-only */
>   /*
> - * tas2764.h - ALSA SoC Texas Instruments TAS2764 Mono Audio Amplifier
> + * tas27xx.h - ALSA SoC Texas Instruments TAS2764/TAS2780
> + *		Mono Audio Amplifier
>    *
> - * Copyright (C) 2020 Texas Instruments Incorporated -  https://www.ti.com
> + * Copyright (C) 2022 Texas Instruments Incorporated -
> + *		https://www.ti.com
>    *
> - * Author: Dan Murphy <dmurphy@ti.com>
> + * Author:
>    */
>   
> -#ifndef __TAS2764__
> -#define __TAS2764__
> +#ifndef __TAS27XX__H_
> +#define __TAS27XX__H_
>   
>   /* Book Control Register */
> -#define TAS2764_BOOKCTL_PAGE	0
> -#define TAS2764_BOOKCTL_REG	127
> +#define TAS27XX_BOOKCTL_PAGE	0
> +#define TAS27XX_BOOKCTL_REG	127
>   #define TAS2764_REG(page, reg)	((page * 128) + reg)
>   
>   /* Page */
> @@ -77,6 +79,10 @@
>   #define TAS2764_TDM_CFG3_RXS_SHIFT	0x4
>   #define TAS2764_TDM_CFG3_MASK		GENMASK(3, 0)
>   
> +/* TDM Configuration Reg4 */
> +#define TAS2764_TDM_CFG4		TAS2764_REG(0X0, 0x0d)
> +#define TAS2764_TDM_CFG4_TX_OFFSET_MASK	GENMASK(3, 1)
> +
>   /* TDM Configuration Reg5 */
>   #define TAS2764_TDM_CFG5		TAS2764_REG(0X0, 0x0e)
>   #define TAS2764_TDM_CFG5_VSNS_MASK	BIT(6)
> @@ -89,4 +95,9 @@
>   #define TAS2764_TDM_CFG6_ISNS_ENABLE	BIT(6)
>   #define TAS2764_TDM_CFG6_50_MASK	GENMASK(5, 0)
>   
> -#endif /* __TAS2764__ */
> +/* INT&CLK CFG */
> +#define TAS27XX_CLK_CFG			TAS2764_REG(0X0, 0x5c)
> +#define TAS27XX_CLK_CFG_MASK		GENMASK(7, 6)
> +#define TAS27XX_CLK_CFG_ENABLE		(BIT(7) | BIT(6))
> +
> +#endif /* __TAS27XX__H_ */
> \ No newline at end of file

And this patch should probably go before patch 2, otherwise there will 
be build failure on patch 2?
Xu, Yang March 23, 2022, 9:41 a.m. UTC | #2 
Hi Slawinski,

Thanks for your feedback.Here is the target we want to do when submitting all the patchs:
1. rename tas2764.c to tas27xx.c
2.rename tas2764.h to tas27xx.h
3.update Makefile
4.update Kconfig
5.rename tas2764.yaml to tas27xx.yaml
6.update tas27xx.c to support either TAS2764 or TAS2780
7. update tas27xx.h to support either TAS2764 or TAS2780
As just to make everything clear,we plan to do item1 to 5 firstly.Could you let us know what's your suggestion?Should we stop this patch routing and start a new patch submit process?
Regarding item 6 to 7,we can submit separate patch process after we finish item 1 to 5.

Regards
Raphael

-----Original Message-----
From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com> 
Sent: Wednesday, March 23, 2022 4:29 PM
To: Raphael-Xu <13691752556@139.com>; broonie@kernel.org
Cc: Navada Kanyana, Mukund <navada@ti.com>; alsa-devel@alsa-project.org; Ding, Shenghao <shenghao-ding@ti.com>; Xu, Yang <raphael-xu@ti.com>
Subject: [EXTERNAL] Re: [PATCH v4 3/3] update tas27xx.h to support either TAS2764 or TAS2780

On 3/23/2022 5:26 AM, Raphael-Xu wrote:
> Signed-off-by: Raphael-Xu <13691752556@139.com>
> ---
>   sound/soc/codecs/tas27xx.h | 27 +++++++++++++++++++--------
>   1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/sound/soc/codecs/tas27xx.h b/sound/soc/codecs/tas27xx.h 
> index 67d6fd903c42..02b29c030d37 100644
> --- a/sound/soc/codecs/tas27xx.h
> +++ b/sound/soc/codecs/tas27xx.h
> @@ -1,18 +1,20 @@
>   /* SPDX-License-Identifier: GPL-2.0-only */
>   /*
> - * tas2764.h - ALSA SoC Texas Instruments TAS2764 Mono Audio 
> Amplifier
> + * tas27xx.h - ALSA SoC Texas Instruments TAS2764/TAS2780
> + *		Mono Audio Amplifier
>    *
> - * Copyright (C) 2020 Texas Instruments Incorporated -  
> https://www.ti.com
> + * Copyright (C) 2022 Texas Instruments Incorporated -
> + *		https://www.ti.com
>    *
> - * Author: Dan Murphy <dmurphy@ti.com>
> + * Author:
>    */
>   
> -#ifndef __TAS2764__
> -#define __TAS2764__
> +#ifndef __TAS27XX__H_
> +#define __TAS27XX__H_
>   
>   /* Book Control Register */
> -#define TAS2764_BOOKCTL_PAGE	0
> -#define TAS2764_BOOKCTL_REG	127
> +#define TAS27XX_BOOKCTL_PAGE	0
> +#define TAS27XX_BOOKCTL_REG	127
>   #define TAS2764_REG(page, reg)	((page * 128) + reg)
>   
>   /* Page */
> @@ -77,6 +79,10 @@
>   #define TAS2764_TDM_CFG3_RXS_SHIFT	0x4
>   #define TAS2764_TDM_CFG3_MASK		GENMASK(3, 0)
>   
> +/* TDM Configuration Reg4 */
> +#define TAS2764_TDM_CFG4		TAS2764_REG(0X0, 0x0d)
> +#define TAS2764_TDM_CFG4_TX_OFFSET_MASK	GENMASK(3, 1)
> +
>   /* TDM Configuration Reg5 */
>   #define TAS2764_TDM_CFG5		TAS2764_REG(0X0, 0x0e)
>   #define TAS2764_TDM_CFG5_VSNS_MASK	BIT(6)
> @@ -89,4 +95,9 @@
>   #define TAS2764_TDM_CFG6_ISNS_ENABLE	BIT(6)
>   #define TAS2764_TDM_CFG6_50_MASK	GENMASK(5, 0)
>   
> -#endif /* __TAS2764__ */
> +/* INT&CLK CFG */
> +#define TAS27XX_CLK_CFG			TAS2764_REG(0X0, 0x5c)
> +#define TAS27XX_CLK_CFG_MASK		GENMASK(7, 6)
> +#define TAS27XX_CLK_CFG_ENABLE		(BIT(7) | BIT(6))
> +
> +#endif /* __TAS27XX__H_ */
> \ No newline at end of file

And this patch should probably go before patch 2, otherwise there will be build failure on patch 2?
Amadeusz Sławiński March 23, 2022, 9:56 a.m. UTC | #3 
On 3/23/2022 10:41 AM, Xu, Yang wrote:
> Hi Slawinski,
> 
> Thanks for your feedback.Here is the target we want to do when submitting all the patchs:
> 1. rename tas2764.c to tas27xx.c
> 2.rename tas2764.h to tas27xx.h
> 3.update Makefile
> 4.update Kconfig
> 5.rename tas2764.yaml to tas27xx.yaml
> 6.update tas27xx.c to support either TAS2764 or TAS2780
> 7. update tas27xx.h to support either TAS2764 or TAS2780
> As just to make everything clear,we plan to do item1 to 5 firstly.Could you let us know what's your suggestion?Should we stop this patch routing and start a new patch submit process?
> Regarding item 6 to 7,we can submit separate patch process after we finish item 1 to 5.
> 
> Regards
> Raphael

Hi,

this would be probably overdoing it, 4 patches should be enough.
Patch 1 and 3 seem ok to me, I would just split patch 2, and reorder a 
bit, so something like:
1. [PATCH v4 1/3] rename tas2764 to tas27xx-Makefile and Kconfig
2. here patch renaming variables (2764 -> xxxx)
3. [PATCH v4 3/3] update tas27xx.h to support either TAS2764 or TAS2780
4. here patch adding TAS2780 support

The reason why patch 3, should go before one adding support is that 
there is dependency on information present in header, and you don't want 
to break build when someone does git bisect with your driver enabled.

> 
> -----Original Message-----
> From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> Sent: Wednesday, March 23, 2022 4:29 PM
> To: Raphael-Xu <13691752556@139.com>; broonie@kernel.org
> Cc: Navada Kanyana, Mukund <navada@ti.com>; alsa-devel@alsa-project.org; Ding, Shenghao <shenghao-ding@ti.com>; Xu, Yang <raphael-xu@ti.com>
> Subject: [EXTERNAL] Re: [PATCH v4 3/3] update tas27xx.h to support either TAS2764 or TAS2780
> 
> On 3/23/2022 5:26 AM, Raphael-Xu wrote:
>> Signed-off-by: Raphael-Xu <13691752556@139.com>
>> ---
>>    sound/soc/codecs/tas27xx.h | 27 +++++++++++++++++++--------
>>    1 file changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/sound/soc/codecs/tas27xx.h b/sound/soc/codecs/tas27xx.h
>> index 67d6fd903c42..02b29c030d37 100644
>> --- a/sound/soc/codecs/tas27xx.h
>> +++ b/sound/soc/codecs/tas27xx.h
>> @@ -1,18 +1,20 @@
>>    /* SPDX-License-Identifier: GPL-2.0-only */
>>    /*
>> - * tas2764.h - ALSA SoC Texas Instruments TAS2764 Mono Audio
>> Amplifier
>> + * tas27xx.h - ALSA SoC Texas Instruments TAS2764/TAS2780
>> + *		Mono Audio Amplifier
>>     *
>> - * Copyright (C) 2020 Texas Instruments Incorporated -
>> https://www.ti.com
>> + * Copyright (C) 2022 Texas Instruments Incorporated -
>> + *		https://www.ti.com
>>     *
>> - * Author: Dan Murphy <dmurphy@ti.com>
>> + * Author:
>>     */
>>    
>> -#ifndef __TAS2764__
>> -#define __TAS2764__
>> +#ifndef __TAS27XX__H_
>> +#define __TAS27XX__H_
>>    
>>    /* Book Control Register */
>> -#define TAS2764_BOOKCTL_PAGE	0
>> -#define TAS2764_BOOKCTL_REG	127
>> +#define TAS27XX_BOOKCTL_PAGE	0
>> +#define TAS27XX_BOOKCTL_REG	127
>>    #define TAS2764_REG(page, reg)	((page * 128) + reg)
>>    
>>    /* Page */
>> @@ -77,6 +79,10 @@
>>    #define TAS2764_TDM_CFG3_RXS_SHIFT	0x4
>>    #define TAS2764_TDM_CFG3_MASK		GENMASK(3, 0)
>>    
>> +/* TDM Configuration Reg4 */
>> +#define TAS2764_TDM_CFG4		TAS2764_REG(0X0, 0x0d)
>> +#define TAS2764_TDM_CFG4_TX_OFFSET_MASK	GENMASK(3, 1)
>> +
>>    /* TDM Configuration Reg5 */
>>    #define TAS2764_TDM_CFG5		TAS2764_REG(0X0, 0x0e)
>>    #define TAS2764_TDM_CFG5_VSNS_MASK	BIT(6)
>> @@ -89,4 +95,9 @@
>>    #define TAS2764_TDM_CFG6_ISNS_ENABLE	BIT(6)
>>    #define TAS2764_TDM_CFG6_50_MASK	GENMASK(5, 0)
>>    
>> -#endif /* __TAS2764__ */
>> +/* INT&CLK CFG */
>> +#define TAS27XX_CLK_CFG			TAS2764_REG(0X0, 0x5c)
>> +#define TAS27XX_CLK_CFG_MASK		GENMASK(7, 6)
>> +#define TAS27XX_CLK_CFG_ENABLE		(BIT(7) | BIT(6))
>> +
>> +#endif /* __TAS27XX__H_ */
>> \ No newline at end of file
> 
> And this patch should probably go before patch 2, otherwise there will be build failure on patch 2?
Mark Brown March 23, 2022, 4:50 p.m. UTC | #4 
On Wed, Mar 23, 2022 at 10:56:04AM +0100, Amadeusz Sławiński wrote:

> this would be probably overdoing it, 4 patches should be enough.
> Patch 1 and 3 seem ok to me, I would just split patch 2, and reorder a bit,
> so something like:
> 1. [PATCH v4 1/3] rename tas2764 to tas27xx-Makefile and Kconfig
> 2. here patch renaming variables (2764 -> xxxx)
> 3. [PATCH v4 3/3] update tas27xx.h to support either TAS2764 or TAS2780
> 4. here patch adding TAS2780 support

That looks like a good plan.

> The reason why patch 3, should go before one adding support is that there is
> dependency on information present in header, and you don't want to break
> build when someone does git bisect with your driver enabled.

Indeed I test for this when applying patches.
diff mbox series

Patch

diff --git a/sound/soc/codecs/tas27xx.h b/sound/soc/codecs/tas27xx.h
index 67d6fd903c42..02b29c030d37 100644
--- a/sound/soc/codecs/tas27xx.h
+++ b/sound/soc/codecs/tas27xx.h
@@ -1,18 +1,20 @@ 
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * tas2764.h - ALSA SoC Texas Instruments TAS2764 Mono Audio Amplifier
+ * tas27xx.h - ALSA SoC Texas Instruments TAS2764/TAS2780
+ *		Mono Audio Amplifier
  *
- * Copyright (C) 2020 Texas Instruments Incorporated -  https://www.ti.com
+ * Copyright (C) 2022 Texas Instruments Incorporated -
+ *		https://www.ti.com
  *
- * Author: Dan Murphy <dmurphy@ti.com>
+ * Author:
  */
 
-#ifndef __TAS2764__
-#define __TAS2764__
+#ifndef __TAS27XX__H_
+#define __TAS27XX__H_
 
 /* Book Control Register */
-#define TAS2764_BOOKCTL_PAGE	0
-#define TAS2764_BOOKCTL_REG	127
+#define TAS27XX_BOOKCTL_PAGE	0
+#define TAS27XX_BOOKCTL_REG	127
 #define TAS2764_REG(page, reg)	((page * 128) + reg)
 
 /* Page */
@@ -77,6 +79,10 @@ 
 #define TAS2764_TDM_CFG3_RXS_SHIFT	0x4
 #define TAS2764_TDM_CFG3_MASK		GENMASK(3, 0)
 
+/* TDM Configuration Reg4 */
+#define TAS2764_TDM_CFG4		TAS2764_REG(0X0, 0x0d)
+#define TAS2764_TDM_CFG4_TX_OFFSET_MASK	GENMASK(3, 1)
+
 /* TDM Configuration Reg5 */
 #define TAS2764_TDM_CFG5		TAS2764_REG(0X0, 0x0e)
 #define TAS2764_TDM_CFG5_VSNS_MASK	BIT(6)
@@ -89,4 +95,9 @@ 
 #define TAS2764_TDM_CFG6_ISNS_ENABLE	BIT(6)
 #define TAS2764_TDM_CFG6_50_MASK	GENMASK(5, 0)
 
-#endif /* __TAS2764__ */
+/* INT&CLK CFG */
+#define TAS27XX_CLK_CFG			TAS2764_REG(0X0, 0x5c)
+#define TAS27XX_CLK_CFG_MASK		GENMASK(7, 6)
+#define TAS27XX_CLK_CFG_ENABLE		(BIT(7) | BIT(6))
+
+#endif /* __TAS27XX__H_ */
\ No newline at end of file