Re: [PATCH v2] ASoC: rt721-sdca: Add RT721 SDCA driver

On 9/24/2024 5:36 PM, Pierre-Louis Bossart wrote:

On 9/24/24 11:03, Jack Yu wrote:

...

This is the initial codec driver for rt721-sdca.

I wouldn't hurt to provide a short description. 722 has 3 functions, what about 721?

...

• case SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC, RT721_SDCA_ENT_USER_FU05, RT721_SDCA_CTL_FU_VOLUME,

• 	CH_L):
  

• case SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC, RT721_SDCA_ENT_USER_FU05, RT721_SDCA_CTL_FU_VOLUME,

• 	CH_R):
  

• case SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC, RT721_SDCA_ENT_USER_FU0F, RT721_SDCA_CTL_FU_VOLUME,

• 	CH_L):
  

• case SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC, RT721_SDCA_ENT_USER_FU0F, RT721_SDCA_CTL_FU_VOLUME,

• 	CH_R):
  

• case SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC, RT721_SDCA_ENT_PLATFORM_FU44,

• 	RT721_SDCA_CTL_FU_CH_GAIN, CH_L):
  

• case SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC, RT721_SDCA_ENT_PLATFORM_FU44,

• 	RT721_SDCA_CTL_FU_CH_GAIN, CH_R):
  

• case SDW_SDCA_CTL(FUNC_NUM_MIC_ARRAY, RT721_SDCA_ENT_USER_FU1E, RT721_SDCA_CTL_FU_VOLUME,

• 	CH_01):
  

• case SDW_SDCA_CTL(FUNC_NUM_MIC_ARRAY, RT721_SDCA_ENT_USER_FU1E, RT721_SDCA_CTL_FU_VOLUME,

• 	CH_02):
  

• case SDW_SDCA_CTL(FUNC_NUM_MIC_ARRAY, RT721_SDCA_ENT_USER_FU1E, RT721_SDCA_CTL_FU_VOLUME,

• 	CH_03):
  

• case SDW_SDCA_CTL(FUNC_NUM_MIC_ARRAY, RT721_SDCA_ENT_USER_FU1E, RT721_SDCA_CTL_FU_VOLUME,

• 	CH_04):
  

• case SDW_SDCA_CTL(FUNC_NUM_AMP, RT721_SDCA_ENT_USER_FU06, RT721_SDCA_CTL_FU_VOLUME, CH_L):
• case SDW_SDCA_CTL(FUNC_NUM_AMP, RT721_SDCA_ENT_USER_FU06, RT721_SDCA_CTL_FU_VOLUME, CH_R):

• return true;
  

That tells us it has 3 functions (jack, mic, amp), so what's different to RT722? could we have a single driver for both parts? A lot of this driver seems copy-pasted-renamed.

...

+static int rt721_sdca_read_prop(struct sdw_slave *slave) +{

• struct sdw_slave_prop *prop = &slave->prop;
• int nval;
• int i, j;
• u32 bit;
• unsigned long addr;
• struct sdw_dpn_prop *dpn;
• sdw_slave_read_prop(slave);

I thought we agreed to use a helper to read only the number of lanes from platform firmware?

Bard, did you share this already?

Not yet, that will be a part of multi-lane support.