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

["Liao, Bard" <yung-chuan.liao@linux.intel.com>]

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.