[EXTERNAL] Re: [PATCH v4 3/3] update tas27xx.h to support either TAS2764 or TAS2780

[Amadeusz Sławiński]

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 at linux.intel.com>
> Sent: Wednesday, March 23, 2022 4:29 PM
> To: Raphael-Xu <13691752556 at 139.com>; broonie at kernel.org
> Cc: Navada Kanyana, Mukund <navada at ti.com>; alsa-devel at alsa-project.org; Ding, Shenghao <shenghao-ding at ti.com>; Xu, Yang <raphael-xu at 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 at 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 at 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?