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

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?

• prop->scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY;
• prop->quirks = SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY;
• prop->paging_support = true;
• /*

• * port = 1 for headphone playback
  

• * port = 2 for headset-mic capture
  

• * port = 3 for speaker playback
  

• * port = 6 for digital-mic capture
  

• */
  

• prop->source_ports = BIT(6) | BIT(2); /* BITMAP: 01000100 */
• prop->sink_ports = BIT(3) | BIT(1); /* BITMAP: 00001010 */
• nval = hweight32(prop->source_ports);
• prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,

• sizeof(*prop->src_dpn_prop), GFP_KERNEL);
  

• if (!prop->src_dpn_prop)

• return -ENOMEM;
  

• i = 0;
• dpn = prop->src_dpn_prop;
• addr = prop->source_ports;
• for_each_set_bit(bit, &addr, 32) {

• dpn[i].num = bit;
  

• dpn[i].type = SDW_DPN_FULL;
  

• dpn[i].simple_ch_prep_sm = true;
  

• dpn[i].ch_prep_timeout = 10;
  

• i++;
  

• }
• /* do this again for sink now */
• nval = hweight32(prop->sink_ports);
• prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,

• sizeof(*prop->sink_dpn_prop), GFP_KERNEL);
  

• if (!prop->sink_dpn_prop)

• return -ENOMEM;
  

• j = 0;
• dpn = prop->sink_dpn_prop;
• addr = prop->sink_ports;
• for_each_set_bit(bit, &addr, 32) {

• dpn[j].num = bit;
  

• dpn[j].type = SDW_DPN_FULL;
  

• dpn[j].simple_ch_prep_sm = true;
  

• dpn[j].ch_prep_timeout = 10;
  

• j++;
  

• }
• /* set the timeout values */
• prop->clk_stop_timeout = 200;
• /* wake-up event */
• prop->wake_capable = 1;
• /* Three data lanes are supported by rt721-sdca codec */
• prop->lane_control_support = true;

this doesn't seem needed if we already read information on lane support.

+static void rt721_sdca_dmic_preset(struct rt721_sdca_priv *rt721) +{

• /* Power down group1/2/3_mic_pdb */
• rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,

• RT721_MISC_POWER_CTL31, 0x8000);
  

• /* VREF_HV_EN_AUTO_FAST */
• rt721_sdca_index_write(rt721, RT721_ANA_POW_PART,

• RT721_VREF1_HV_CTRL1, 0xe000);
  

• /* Power up group1/2/3_mic_pdb */
• rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,

• RT721_MISC_POWER_CTL31, 0x8007);
  

• /* Set AD07/08 power entity floating control */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_ENT_FLOAT_CTL9, 0x2a2a);
  

• /* Set AD10 power entity floating control */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_ENT_FLOAT_CTL10, 0x2a00);
  

• /* Set DMIC1/DMIC2 power entity floating control */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_ENT_FLOAT_CTL6, 0x2a2a);
  

• /* Set DMIC1/DMIC2 IT entity floating control */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_ENT_FLOAT_CTL5, 0x2626);
  

• /* Set AD10 FU entity floating control */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_ENT_FLOAT_CTL8, 0x1e00);
  

• /* Set DMIC1/DMIC2 FU input gain floating control */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_ENT_FLOAT_CTL7, 0x1515);
  

• /* Set DMIC2 FU input gain channel floating control */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_CH_FLOAT_CTL3, 0x0304);
  

• /* Set AD10 FU channel floating control */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_CH_FLOAT_CTL4, 0x0304);
  

• /* vf71f_r12_07_06 and vf71f_r13_07_06 = 2’b00 */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_HDA_LEGACY_CTL1, 0x0000);
  

• /* Enable vf707_r12_05/vf707_r13_05 */
• regmap_write(rt721->regmap,

• SDW_SDCA_CTL(FUNC_NUM_MIC_ARRAY, RT721_SDCA_ENT_IT26,
  

• 	RT721_SDCA_CTL_VENDOR_DEF, 0), 0x01);
  

• /* Set usd_flag_sel, usd_in_sel */
• regmap_write(rt721->mbq_regmap, 0x5910009, 0x2e01);
• /* Set RC calibration */
• rt721_sdca_index_write(rt721, RT721_RC_CALIB_CTRL,

• RT721_RC_CALIB_CTRL0, 0x0b00);
  

• rt721_sdca_index_write(rt721, RT721_RC_CALIB_CTRL,

• RT721_RC_CALIB_CTRL0, 0x0b40);
  

• /* Fine tune PDE2A latency */
• regmap_write(rt721->regmap, 0x2f5c, 0x25);

+}

Humm, isn't all this supposed to be handled with blind writes? It seems odd to hard-code all this, no?

+static void rt721_sdca_amp_preset(struct rt721_sdca_priv *rt721) +{;

• /* Power down group1/2/3_mic_pdb */
• rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,

• RT721_MISC_POWER_CTL31, 0x8000);
  

• /* VREF_HV_EN_AUTO_FAST */
• rt721_sdca_index_write(rt721, RT721_ANA_POW_PART,

• RT721_VREF1_HV_CTRL1, 0xe000);
  

• /* Power up group1/2/3_mic_pdb */
• rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,

• RT721_MISC_POWER_CTL31, 0x8007);
  

• /* Reset dc_cal_top */
• regmap_write(rt721->mbq_regmap, 0x5810000, 0x6420);
• /* Turn back to normal dc_cal_top */
• regmap_write(rt721->mbq_regmap, 0x5810000, 0x6421);
• /* W1C Trigger Calibration */
• regmap_write(rt721->mbq_regmap, 0x5810000, 0xe421);
• /* DAC04 FU entity floating control */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_CH_FLOAT_CTL6, 0x5561);
  

• /* Set EAPD high */
• rt721_sdca_index_write(rt721, RT721_VENDOR_REG,

• RT721_GPIO_PAD_CTRL5, 0x8003);
  

• /* Enable vf707_r14 */
• regmap_write(rt721->regmap,

• SDW_SDCA_CTL(FUNC_NUM_AMP, RT721_SDCA_ENT_OT23,
  

• 	RT721_SDCA_CTL_VENDOR_DEF, 0), 0x04);
  

• /* FU 23 SPK mute control - L */
• regmap_write(rt721->regmap,

• SDW_SDCA_CTL(FUNC_NUM_AMP, RT721_SDCA_ENT_PDE23,
  

• 	RT721_SDCA_CTL_FU_MUTE, CH_01), 0x00);
  

• /* FU 23 SPK mute control - R */
• regmap_write(rt721->regmap,

• SDW_SDCA_CTL(FUNC_NUM_AMP, RT721_SDCA_ENT_PDE23,
  

• 	RT721_SDCA_CTL_FU_MUTE, CH_02), 0x00);
  

• /* FU 55 DAC04 mute control - L */
• regmap_write(rt721->regmap,

• SDW_SDCA_CTL(FUNC_NUM_AMP, RT721_SDCA_ENT_FU55,
  

• 	RT721_SDCA_CTL_FU_MUTE, CH_01), 0x00);
  

• /* FU 55 DAC04 mute control - R */
• regmap_write(rt721->regmap,

• SDW_SDCA_CTL(FUNC_NUM_AMP, RT721_SDCA_ENT_FU55,
  

• 	RT721_SDCA_CTL_FU_MUTE, CH_02), 0x00);
  

+}

+static void rt721_sdca_jack_preset(struct rt721_sdca_priv *rt721) +{

• /* Power down group1/2/3_mic_pdb */
• rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,

• RT721_MISC_POWER_CTL31, 0x8000);
  

• /* VREF_HV_EN_AUTO_FAST */
• rt721_sdca_index_write(rt721, RT721_ANA_POW_PART,

• RT721_VREF1_HV_CTRL1, 0xe000);
  

• /* Power up group1/2/3_mic_pdb */
• rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,

• RT721_MISC_POWER_CTL31, 0x8007);
  

• /* GE0 mode related control */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_GE_REL_CTRL1, 0x8011);
  

• /* Button A, B, C, D bypass mode */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_UMP_HID_CTRL3, 0xcf00);
  

• /* HID1 slot enable */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_UMP_HID_CTRL4, 0x000f);
  

• /* Report ID for HID1 */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_UMP_HID_CTRL1, 0x1100);
  

• /* OSC/OOC for slot 2, 3 */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_UMP_HID_CTRL5, 0x0c12);
  

• /* Set JD de-bounce clock control */
• rt721_sdca_index_write(rt721, RT721_JD_CTRL,

• RT721_JD_1PIN_GAT_CTRL2, 0xc002);
  

• /* RC calibration -1 */
• rt721_sdca_index_write(rt721, RT721_RC_CALIB_CTRL,

• RT721_RC_CALIB_CTRL0, 0x0b00);
  

• /* RC calibration -2 */
• rt721_sdca_index_write(rt721, RT721_RC_CALIB_CTRL,

• RT721_RC_CALIB_CTRL0, 0x0b40);
  

• /* pow_clk_12p288mhz_dre03 change to register mode */
• rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,

• RT721_UAJ_TOP_TCON14, 0x3333);
  

• /* Tune calibration timing control */
• regmap_write(rt721->mbq_regmap, 0x5810035, 0x0036);
• /* calibration HP amp output select control from Efuse */
• regmap_write(rt721->mbq_regmap, 0x5810030, 0xee00);
• /* FSM related control */
• rt721_sdca_index_write(rt721, RT721_CAP_PORT_CTRL,

• RT721_HP_AMP_2CH_CAL1, 0x0140);
  

• /* HP calibration related control */
• regmap_write(rt721->mbq_regmap, 0x5810000, 0x0021);
• /* W1C HP calibration*/
• regmap_write(rt721->mbq_regmap, 0x5810000, 0x8021);
• /* reg_sel_cin_hp_0010/0011 */
• rt721_sdca_index_write(rt721, RT721_CAP_PORT_CTRL,

• RT721_HP_AMP_2CH_CAL18, 0x5522);
  

• regmap_write(rt721->mbq_regmap, 0x5b10007, 0x2000);
• /* sel_sensing_lr_hp */
• regmap_write(rt721->mbq_regmap, 0x5B10017, 0x1b0f);
• /* Release HP-JD */
• rt721_sdca_index_write(rt721, RT721_CBJ_CTRL,

• RT721_CBJ_A0_GAT_CTRL1, 0x2a02);
  

• /* en_osw gating auto done bit */
• rt721_sdca_index_write(rt721, RT721_CAP_PORT_CTRL,

• RT721_HP_AMP_2CH_CAL4, 0xa105);
  

• /* pow_clk_en_sw_amp_detect_sel to register mode */
• rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,

• RT721_UAJ_TOP_TCON14, 0x3b33);
  

• /* cp_sw_hp to auto mode */
• regmap_write(rt721->mbq_regmap, 0x310400, 0x3023);
• /* pow_clk_en_sw_amp_detect power up */
• rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,

• RT721_UAJ_TOP_TCON14, 0x3f33);
  

• rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,

• RT721_UAJ_TOP_TCON13, 0x6048);
  

• /* switch size detect threshold */
• regmap_write(rt721->mbq_regmap, 0x310401, 0x3000);
• regmap_write(rt721->mbq_regmap, 0x310402, 0x1b00);
• /* en_hp_amp_detect auto mode */
• regmap_write(rt721->mbq_regmap, 0x310300, 0x000f);
• /* amp detect threshold */
• regmap_write(rt721->mbq_regmap, 0x310301, 0x3000);
• regmap_write(rt721->mbq_regmap, 0x310302, 0x1b00);
• /* gating_sdw_link_rst_n_1_cbj_reg */
• rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,

• RT721_UAJ_TOP_TCON17, 0x0008);
  

• /* CKXEN_SDAC chopper function */
• rt721_sdca_index_write(rt721, RT721_DAC_CTRL,

• RT721_DAC_2CH_CTRL3, 0x55ff);
  

• /* CKXSEL_SDAC chopper frequency */
• rt721_sdca_index_write(rt721, RT721_DAC_CTRL,

• RT721_DAC_2CH_CTRL4, 0xcc00);
  

• /* Bias current for SDAC */
• rt721_sdca_index_write(rt721, RT721_ANA_POW_PART,

• RT721_MBIAS_LV_CTRL2, 0x6677);
  

• /* VREF2 level selection */
• rt721_sdca_index_write(rt721, RT721_ANA_POW_PART,

• RT721_VREF2_LV_CTRL1, 0x7600);
  

• /* ADC09/MIC2 power entity floating control */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_ENT_FLOAT_CTL2, 0x1234);
  

• /* LINE2 power entity floating control */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_ENT_FLOAT_CTL3, 0x3512);
  

• /* DAC03/HP power entity floating control */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_ENT_FLOAT_CTL1, 0x4040);
  

• /* ADC27 power entity floating control */
• rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,

• RT721_ENT_FLOAT_CTL4, 0x1201);
  

• /* Fine tune PDE40 latency */
• regmap_write(rt721->regmap, 0x2f58, 0x07);

+}

same here, shouldn't this come from ACPI/blind write tables?

+enum rt721_sdca_jd_src {

• RT721_JD_NULL,
• RT721_JD1,
• RT721_JD2,

+};

is this supported in SDCA? I can't recall seeing this for other drivers.