Re: [PATCH v3 2/4] ALSA: hda/cirrus: Add support for CS8409 HDA bridge and CS42L42 companion codec.

+/* Enable I2C clocks */ +static void cs8409_enable_i2c_clock(struct hda_codec *codec, unsigned int flag) +{

• unsigned int retval = 0;
• unsigned int newval = 0;

initializations not needed

• retval = cs_vendor_coef_get(codec, 0x0);
• newval = (flag) ? (retval | 0x8) : (retval & 0xfffffff7);
• cs_vendor_coef_set(codec, 0x0, newval);

+}

+/* Wait I2C transaction */ +static int cs8409_i2c_wait_complete(struct hda_codec *codec) +{

• int repeat = 5;
• unsigned int retval = 0;

initialization not needed.

• do {

• retval = cs_vendor_coef_get(codec, CIR_I2C_STATUS);
  

• if ((retval & 0x18) != 0x18) {
  

• 	usleep_range(2000, 4000);
  

• 	--repeat;
  

• } else
  

• 	break;
  

• } while (repeat);
• return repeat > 0 ? 0 : -1;

can we simplify by returning !!repeat ?

+}

+/* CS8409 slave i2cRead */ +static unsigned int cs8409_i2c_read(struct hda_codec *codec,

• unsigned int i2c_address,
  

• unsigned int i2c_reg,
  

• unsigned int paged)
  

+{

• unsigned int i2c_reg_data;
• unsigned int retval = 0;
• cs8409_enable_i2c_clock(codec, 1);
• cs_vendor_coef_set(codec, CIR_I2C_ADDR, i2c_address);
• if (paged) {

• cs_vendor_coef_set(codec, CIR_I2C_QWRITE, i2c_reg >> 8);
  

• if (cs8409_i2c_wait_complete(codec) == -1) {
  

• 	codec_err(codec,
  

• 		"%s() Paged Transaction Failed 0x%02x : 0x%04x = 0x%02x\n",
  

• 		__func__, i2c_address, i2c_reg, retval);
  

return an error?

• }
  

• }
• i2c_reg_data = (i2c_reg << 8) & 0x0ffff;
• cs_vendor_coef_set(codec, CIR_I2C_QREAD, i2c_reg_data);
• if (cs8409_i2c_wait_complete(codec) == -1) {

• codec_err(codec, "%s() Transaction Failed 0x%02x : 0x%04x = 0x%02x\n",
  

• 	__func__, i2c_address, i2c_reg, retval);
  

return and error?

• }
• /* Register in bits 15-8 and the data in 7-0 */
• retval = cs_vendor_coef_get(codec, CIR_I2C_QREAD);
• retval &= 0x0ff;
• cs8409_enable_i2c_clock(codec, 0);
• return retval;

+}

+/* CS8409 slave i2cWrite */ +static unsigned int cs8409_i2c_write(struct hda_codec *codec,

• unsigned int i2c_address, unsigned int i2c_reg,
  

• unsigned int i2c_data,
  

• unsigned int paged)
  

+{

• unsigned int retval = 0;
• unsigned int i2c_reg_data = 0;
• cs8409_enable_i2c_clock(codec, 1);
• cs_vendor_coef_set(codec, CIR_I2C_ADDR, i2c_address);
• if (paged) {

• cs_vendor_coef_set(codec, CIR_I2C_QWRITE, i2c_reg >> 8);
  

• if (cs8409_i2c_wait_complete(codec) == -1) {
  

• 	codec_err(codec,
  

• 		"%s() Paged Transaction Failed 0x%02x : 0x%04x = 0x%02x\n",
  

• 		__func__, i2c_address, i2c_reg, retval);
  

return error?

• }
  

• }
• i2c_reg_data = ((i2c_reg << 8) & 0x0ff00) | (i2c_data & 0x0ff);
• cs_vendor_coef_set(codec, CIR_I2C_QWRITE, i2c_reg_data);
• if (cs8409_i2c_wait_complete(codec) == -1) {

• codec_err(codec, "%s() Transaction Failed 0x%02x : 0x%04x = 0x%02x\n",
  

• 	__func__, i2c_address, i2c_reg, retval);
  

return error?

• }
• cs8409_enable_i2c_clock(codec, 0);
• return retval;

+}

+/* Assert/release RTS# line to CS42L42 */ +static void cs8409_cs42l42_reset(struct hda_codec *codec) +{

• /* Assert RTS# line */
• snd_hda_codec_write(codec,

• 	codec->core.afg, 0, AC_VERB_SET_GPIO_DATA, 0);
  

• /* wait ~10ms */
• usleep_range(10000, 15000);
• /* Release RTS# line */
• snd_hda_codec_write(codec,

• 	codec->core.afg, 0, AC_VERB_SET_GPIO_DATA, GPIO5_INT);
  

• /* wait ~10ms */
• usleep_range(10000, 15000);
• /* Clear interrupts status */
• cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x1308, 1);
• cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x1309, 1);
• cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x130A, 1);
• cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x130F, 1);

clear on read?

+static int cs8409_cs42l42_fixup(struct hda_codec *codec) +{

• int err = 0;

useless init

• struct cs_spec *spec = codec->spec;
• unsigned int pincap = 0;
• /* Basic initial sequence for specific hw configuration */
• snd_hda_sequence_write(codec, cs8409_cs42l42_init_verbs);
• /* CS8409 is simple HDA bridge and intended to be used with a remote

• * companion codec. Most of input/output PIN(s) have only basic
  

• * capabilities. NID(s) 0x24 and 0x34 have only OUTC and INC
  

• * capabilities and no presence detect capable (PDC) and call to
  

• * snd_hda_gen_build_controls() will mark them as non detectable
  

• * phantom jacks. However, in this configuration companion codec
  

• * CS42L42 is connected to these pins and it has jack detect
  

• * capabilities. We have to override pin capabilities,
  

• * otherwise they will not be created as input devices.
  

• */
  

• _snd_hdac_read_parm(&codec->core,

• 	CS8409_CS42L42_HP_PIN_NID, AC_PAR_PIN_CAP, &pincap);
  

• snd_hdac_override_parm(&codec->core,

• 	CS8409_CS42L42_HP_PIN_NID, AC_PAR_PIN_CAP,
  

• 	(pincap | (AC_PINCAP_IMP_SENSE | AC_PINCAP_PRES_DETECT)));
  

• _snd_hdac_read_parm(&codec->core, CS8409_CS42L42_AMIC_PIN_NID,

• 	AC_PAR_PIN_CAP, &pincap);
  

• snd_hdac_override_parm(&codec->core,

• 	CS8409_CS42L42_AMIC_PIN_NID, AC_PAR_PIN_CAP,
  

• 	(pincap | (AC_PINCAP_IMP_SENSE | AC_PINCAP_PRES_DETECT)));
  

• snd_hda_override_wcaps(codec, CS8409_CS42L42_HP_PIN_NID,

• 	(get_wcaps(codec, CS8409_CS42L42_HP_PIN_NID) | AC_WCAP_UNSOL_CAP));
  

• snd_hda_override_wcaps(codec, CS8409_CS42L42_AMIC_PIN_NID,

• 	(get_wcaps(codec, CS8409_CS42L42_AMIC_PIN_NID) | AC_WCAP_UNSOL_CAP));
  

• snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
• err = snd_hda_parse_pin_defcfg(codec, &spec->gen.autocfg, 0, 0);
• if (err < 0)

• return err;
  

• err = snd_hda_gen_parse_auto_config(codec, &spec->gen.autocfg);
• if (err < 0)

• return err;
  

• snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PROBE);
• return err;

+}

+static int patch_cs8409(struct hda_codec *codec) +{

• struct cs_spec *spec;
• int err = -EINVAL;
• spec = cs_alloc_spec(codec, CS8409_VENDOR_NID);
• if (!spec)

• return -ENOMEM;
  

• snd_hda_pick_fixup(codec,

• 	cs8409_models, cs8409_fixup_tbl, cs8409_fixups);
  

• codec_dbg(codec, "Picked ID=%d, VID=%08x, DEV=%08x\n",

• 	codec->fixup_id,
  

• 	codec->bus->pci->subsystem_vendor,
  

• 	codec->bus->pci->subsystem_device);
  

• switch (codec->fixup_id) {
• /* Dell platforms with CS42L42 companion codec */
• case CS8409_BULLSEYE:
• case CS8409_WARLOCK:
• case CS8409_CYBORG:

• snd_hda_add_verbs(codec, cs8409_cs42l42_add_verbs);
  

• codec->patch_ops = cs8409_cs42l42_patch_ops;
  

• spec->gen.suppress_auto_mute = 1;
  

• spec->gen.no_primary_hp = 1;
  

• /* GPIO 5 out, 3,4 in */
  

• spec->gpio_dir = GPIO5_INT;
  

• spec->gpio_data = 0;
  

• spec->gpio_mask = 0x03f;
  

• err = cs8409_cs42l42_fixup(codec);
  

• if (err > 0)
  

better keep the convention that errors are negative and zero is success.

• 	err = cs8409_cs42l42_hw_init(codec);
  

• break;
  

• default:

• codec_err(codec, "VID=%08x, DEV=%08x not supported\n",
  

• 		codec->bus->pci->subsystem_vendor,
  

• 		codec->bus->pci->subsystem_device);
  

• break;
  

• }
• if (err < 0)

• cs_free(codec);
  

• else

• snd_hda_codec_set_name(codec, "CS8409/CS42L42");
  

• return err;

+}

/*

• patch entries

@@ -1229,6 +1804,7 @@ static const struct hda_device_id snd_hda_id_cirrus[] = { HDA_CODEC_ENTRY(0x10134208, "CS4208", patch_cs4208), HDA_CODEC_ENTRY(0x10134210, "CS4210", patch_cs4210), HDA_CODEC_ENTRY(0x10134213, "CS4213", patch_cs4213),

• HDA_CODEC_ENTRY(0x10138409, "CS8409", patch_cs8409), {} /* terminator */ }; MODULE_DEVICE_TABLE(hdaudio, snd_hda_id_cirrus);