* [PATCH 0/4] ALSA: hda/cirrus: Make CS8409 driver more generic by using fixups
@ 2021-03-12 18:44 Vitaly Rodionov
2021-03-12 18:44 ` [PATCH 1/4] ALSA: hda/cirrus: Add error handling into CS8409 I2C functions Vitaly Rodionov
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Vitaly Rodionov @ 2021-03-12 18:44 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai; +Cc: alsa-devel, patches, linux-kernel
This series of patches will address comments by Pierre-Louis Bossart,
cleans up patch_cirrus.c source, reducing checkpatch.pl warnings from 19 to 0,
fixing an issue reported by Canonical: BugLink: https://bugs.launchpad.net/bugs/1918378,
and makes the CS8409 patch more generic by using fixups.
Stefan Binding (4):
ALSA: hda/cirrus: Add error handling into CS8409 I2C functions
ALSA: hda/cirrus: Cleanup patch_cirrus.c code.
ALSA: hda/cirrus: Fix CS42L42 Headset Mic volume control name
ALSA: hda/cirrus: Make CS8409 driver more generic by using fixups.
sound/pci/hda/patch_cirrus.c | 506 ++++++++++++++++-------------------
1 file changed, 228 insertions(+), 278 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/4] ALSA: hda/cirrus: Add error handling into CS8409 I2C functions
2021-03-12 18:44 [PATCH 0/4] ALSA: hda/cirrus: Make CS8409 driver more generic by using fixups Vitaly Rodionov
@ 2021-03-12 18:44 ` Vitaly Rodionov
2021-03-12 18:44 ` [PATCH 2/4] ALSA: hda/cirrus: Cleanup patch_cirrus.c code Vitaly Rodionov
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Vitaly Rodionov @ 2021-03-12 18:44 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai
Cc: alsa-devel, patches, linux-kernel, Stefan Binding
From: Stefan Binding <sbinding@opensource.cirrus.com>
Tested on DELL Inspiron-3505, DELL Inspiron-3501, DELL Inspiron-3500
Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
Signed-off-by: Vitaly Rodionov <vitalyr@opensource.cirrus.com>
---
sound/pci/hda/patch_cirrus.c | 95 +++++++++++++++++++++---------------
1 file changed, 56 insertions(+), 39 deletions(-)
diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index 6a9e5c803977..ca8b522b1d6d 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -1508,7 +1508,7 @@ static void cs8409_enable_i2c_clock(struct hda_codec *codec, unsigned int flag)
static int cs8409_i2c_wait_complete(struct hda_codec *codec)
{
int repeat = 5;
- unsigned int retval = 0;
+ unsigned int retval;
do {
retval = cs_vendor_coef_get(codec, CIR_I2C_STATUS);
@@ -1520,78 +1520,82 @@ static int cs8409_i2c_wait_complete(struct hda_codec *codec)
} while (repeat);
- return repeat > 0 ? 0 : -1;
+ return !!repeat;
}
-/* CS8409 slave i2cRead */
-static unsigned int cs8409_i2c_read(struct hda_codec *codec,
+/* CS8409 slave i2cRead.
+ * Returns negative on error, otherwise returns read value in bits 0-7.
+ */
+static 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;
+ unsigned int read_data;
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) {
+ if (!cs8409_i2c_wait_complete(codec)) {
codec_err(codec,
- "%s() Paged Transaction Failed 0x%02x : 0x%04x = 0x%02x\n",
- __func__, i2c_address, i2c_reg, retval);
+ "%s() Paged Transaction Failed 0x%02x : 0x%04x\n",
+ __func__, i2c_address, i2c_reg);
+ return -EIO;
}
}
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);
+ if (!cs8409_i2c_wait_complete(codec)) {
+ codec_err(codec, "%s() Transaction Failed 0x%02x : 0x%04x\n",
+ __func__, i2c_address, i2c_reg);
+ return -EIO;
}
/* Register in bits 15-8 and the data in 7-0 */
- retval = cs_vendor_coef_get(codec, CIR_I2C_QREAD);
- retval &= 0x0ff;
+ read_data = cs_vendor_coef_get(codec, CIR_I2C_QREAD);
cs8409_enable_i2c_clock(codec, 0);
- return retval;
+ return read_data & 0x0ff;
}
/* CS8409 slave i2cWrite */
-static unsigned int cs8409_i2c_write(struct hda_codec *codec,
+static 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;
+ unsigned int i2c_reg_data;
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) {
+ if (!cs8409_i2c_wait_complete(codec)) {
codec_err(codec,
- "%s() Paged Transaction Failed 0x%02x : 0x%04x = 0x%02x\n",
- __func__, i2c_address, i2c_reg, retval);
+ "%s() Paged Transaction Failed 0x%02x : 0x%04x\n",
+ __func__, i2c_address, i2c_reg);
+ return -EIO;
}
}
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);
+ if (!cs8409_i2c_wait_complete(codec)) {
+ codec_err(codec, "%s() Transaction Failed 0x%02x : 0x%04x\n",
+ __func__, i2c_address, i2c_reg);
+ return -EIO;
}
cs8409_enable_i2c_clock(codec, 0);
- return retval;
+ return 0;
}
static int cs8409_cs42l42_volume_info(struct snd_kcontrol *kcontrol,
@@ -1624,14 +1628,27 @@ static int cs8409_cs42l42_volume_info(struct snd_kcontrol *kcontrol,
static void cs8409_cs42l42_update_volume(struct hda_codec *codec)
{
struct cs_spec *spec = codec->spec;
+ int data;
mutex_lock(&spec->cs8409_i2c_mux);
- spec->cs42l42_hp_volume[0] = -(cs8409_i2c_read(codec, CS42L42_I2C_ADDR,
- CS8409_CS42L42_REG_HS_VOLUME_CHA, 1));
- spec->cs42l42_hp_volume[1] = -(cs8409_i2c_read(codec, CS42L42_I2C_ADDR,
- CS8409_CS42L42_REG_HS_VOLUME_CHB, 1));
- spec->cs42l42_hs_mic_volume[0] = -(cs8409_i2c_read(codec, CS42L42_I2C_ADDR,
- CS8409_CS42L42_REG_AMIC_VOLUME, 1));
+ data = cs8409_i2c_read(codec, CS42L42_I2C_ADDR,
+ CS8409_CS42L42_REG_HS_VOLUME_CHA, 1);
+ if (data >= 0)
+ spec->cs42l42_hp_volume[0] = -data;
+ else
+ spec->cs42l42_hp_volume[0] = CS8409_CS42L42_HP_VOL_REAL_MIN;
+ data = cs8409_i2c_read(codec, CS42L42_I2C_ADDR,
+ CS8409_CS42L42_REG_HS_VOLUME_CHB, 1);
+ if (data >= 0)
+ spec->cs42l42_hp_volume[1] = -data;
+ else
+ spec->cs42l42_hp_volume[1] = CS8409_CS42L42_HP_VOL_REAL_MIN;
+ data = cs8409_i2c_read(codec, CS42L42_I2C_ADDR,
+ CS8409_CS42L42_REG_AMIC_VOLUME, 1);
+ if (data >= 0)
+ spec->cs42l42_hs_mic_volume[0] = -data;
+ else
+ spec->cs42l42_hs_mic_volume[0] = CS8409_CS42L42_AMIC_VOL_REAL_MIN;
mutex_unlock(&spec->cs8409_i2c_mux);
spec->cs42l42_volume_init = 1;
}
@@ -1782,7 +1799,7 @@ static void cs8409_cs42l42_reset(struct hda_codec *codec)
}
/* Configure CS42L42 slave codec for jack autodetect */
-static int cs8409_cs42l42_enable_jack_detect(struct hda_codec *codec)
+static void cs8409_cs42l42_enable_jack_detect(struct hda_codec *codec)
{
struct cs_spec *spec = codec->spec;
@@ -1804,8 +1821,6 @@ static int cs8409_cs42l42_enable_jack_detect(struct hda_codec *codec)
cs8409_i2c_write(codec, CS42L42_I2C_ADDR, 0x1b79, 0x00, 1);
mutex_unlock(&spec->cs8409_i2c_mux);
-
- return 0;
}
/* Enable and run CS42L42 slave codec jack auto detect */
@@ -1860,9 +1875,9 @@ static void cs8409_jack_unsol_event(struct hda_codec *codec, unsigned int res)
{
struct cs_spec *spec = codec->spec;
int status_changed = 0;
- unsigned int reg_cdc_status = 0;
- unsigned int reg_hs_status = 0;
- unsigned int reg_ts_status = 0;
+ int reg_cdc_status;
+ int reg_hs_status;
+ int reg_ts_status;
int type = 0;
struct hda_jack_tbl *jk;
@@ -1881,13 +1896,15 @@ static void cs8409_jack_unsol_event(struct hda_codec *codec, unsigned int res)
reg_hs_status = cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x1124, 1);
reg_ts_status = cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x130f, 1);
- /* Clear interrupts */
+ /* Clear interrupts, by reading interrupt status registers */
cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x1b7b, 1);
- cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x1308, 1);
- cs8409_i2c_read(codec, CS42L42_I2C_ADDR, 0x130f, 1);
mutex_unlock(&spec->cs8409_i2c_mux);
+ /* If status values are < 0, read error has occurred. */
+ if ((reg_cdc_status < 0) || (reg_hs_status < 0) || (reg_ts_status < 0))
+ return;
+
/* HSDET_AUTO_DONE */
if (reg_cdc_status & CS42L42_HSDET_AUTO_DONE) {
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/4] ALSA: hda/cirrus: Cleanup patch_cirrus.c code.
2021-03-12 18:44 [PATCH 0/4] ALSA: hda/cirrus: Make CS8409 driver more generic by using fixups Vitaly Rodionov
2021-03-12 18:44 ` [PATCH 1/4] ALSA: hda/cirrus: Add error handling into CS8409 I2C functions Vitaly Rodionov
@ 2021-03-12 18:44 ` Vitaly Rodionov
2021-03-12 18:44 ` [PATCH 3/4] ALSA: hda/cirrus: Fix CS42L42 Headset Mic volume control name Vitaly Rodionov
2021-03-12 18:44 ` [PATCH 4/4] ALSA: hda/cirrus: Make CS8409 driver more generic by using fixups Vitaly Rodionov
3 siblings, 0 replies; 6+ messages in thread
From: Vitaly Rodionov @ 2021-03-12 18:44 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai
Cc: alsa-devel, patches, linux-kernel, Stefan Binding
From: Stefan Binding <sbinding@opensource.cirrus.com>
Minor changes, clean up code, remove unnecessary
initialization of variables, reduced number of
warnings from ./scripts/checkpatch.pl from 19 to 0
Tested on DELL Inspiron-3505, DELL Inspiron-3501, DELL Inspiron-3500
Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
Signed-off-by: Vitaly Rodionov <vitalyr@opensource.cirrus.com>
---
sound/pci/hda/patch_cirrus.c | 155 ++++++++++++++++++-----------------
1 file changed, 78 insertions(+), 77 deletions(-)
diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index ca8b522b1d6d..20e2cc433c6e 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -127,7 +127,7 @@ enum {
* 1 DAC => HP(sense) / Speakers,
* 1 ADC <= LineIn(sense) / MicIn / DMicIn,
* 1 SPDIF OUT => SPDIF Trasmitter(sense)
-*/
+ */
#define CS4210_DAC_NID 0x02
#define CS4210_ADC_NID 0x03
#define CS4210_VENDOR_NID 0x0B
@@ -146,6 +146,7 @@ enum {
static inline int cs_vendor_coef_get(struct hda_codec *codec, unsigned int idx)
{
struct cs_spec *spec = codec->spec;
+
snd_hda_codec_write(codec, spec->vendor_nid, 0,
AC_VERB_SET_COEF_INDEX, idx);
return snd_hda_codec_read(codec, spec->vendor_nid, 0,
@@ -156,6 +157,7 @@ static inline void cs_vendor_coef_set(struct hda_codec *codec, unsigned int idx,
unsigned int coef)
{
struct cs_spec *spec = codec->spec;
+
snd_hda_codec_write(codec, spec->vendor_nid, 0,
AC_VERB_SET_COEF_INDEX, idx);
snd_hda_codec_write(codec, spec->vendor_nid, 0,
@@ -192,6 +194,7 @@ static void cs_automute(struct hda_codec *codec)
static bool is_active_pin(struct hda_codec *codec, hda_nid_t nid)
{
unsigned int val;
+
val = snd_hda_codec_get_pincfg(codec, nid);
return (get_defcfg_connect(val) != AC_JACK_PORT_NONE);
}
@@ -210,7 +213,7 @@ static void init_input_coef(struct hda_codec *codec)
coef |= 1 << 3; /* DMIC1 2 chan on, GPIO0 off
* No effect if SPDIF_OUT2 is
* selected in IDX_SPDIF_CTL.
- */
+ */
cs_vendor_coef_set(codec, IDX_BEEP_CFG, coef);
}
@@ -284,13 +287,6 @@ static const struct hda_verb cs_errata_init_verbs[] = {
{0x11, AC_VERB_SET_COEF_INDEX, 0x0001},
{0x11, AC_VERB_SET_PROC_COEF, 0x0008},
{0x11, AC_VERB_SET_PROC_STATE, 0x00},
-
-#if 0 /* Don't to set to D3 as we are in power-up sequence */
- {0x07, AC_VERB_SET_POWER_STATE, 0x03}, /* S/PDIF Rx: D3 */
- {0x08, AC_VERB_SET_POWER_STATE, 0x03}, /* S/PDIF Tx: D3 */
- /*{0x01, AC_VERB_SET_POWER_STATE, 0x03},*/ /* AFG: D3 This is already handled */
-#endif
-
{} /* terminator */
};
@@ -378,8 +374,10 @@ static int cs_parse_auto_config(struct hda_codec *codec)
/* keep the ADCs powered up when it's dynamically switchable */
if (spec->gen.dyn_adc_switch) {
unsigned int done = 0;
+
for (i = 0; i < spec->gen.input_mux.num_items; i++) {
int idx = spec->gen.dyn_adc_idx[i];
+
if (done & (1 << idx))
continue;
snd_hda_gen_fix_pin_power(codec,
@@ -513,6 +511,7 @@ static void cs420x_fixup_gpio_13(struct hda_codec *codec,
{
if (action == HDA_FIXUP_ACT_PRE_PROBE) {
struct cs_spec *spec = codec->spec;
+
spec->gpio_eapd_hp = 2; /* GPIO1 = headphones */
spec->gpio_eapd_speaker = 8; /* GPIO3 = speakers */
spec->gpio_mask = spec->gpio_dir =
@@ -525,6 +524,7 @@ static void cs420x_fixup_gpio_23(struct hda_codec *codec,
{
if (action == HDA_FIXUP_ACT_PRE_PROBE) {
struct cs_spec *spec = codec->spec;
+
spec->gpio_eapd_hp = 4; /* GPIO2 = headphones */
spec->gpio_eapd_speaker = 8; /* GPIO3 = speakers */
spec->gpio_mask = spec->gpio_dir =
@@ -669,6 +669,7 @@ static void cs4208_fixup_gpio0(struct hda_codec *codec,
{
if (action == HDA_FIXUP_ACT_PRE_PROBE) {
struct cs_spec *spec = codec->spec;
+
spec->gpio_eapd_hp = 0;
spec->gpio_eapd_speaker = 1;
spec->gpio_mask = spec->gpio_dir =
@@ -823,7 +824,7 @@ static int patch_cs4208(struct hda_codec *codec)
* 1 DAC => HP(sense) / Speakers,
* 1 ADC <= LineIn(sense) / MicIn / DMicIn,
* 1 SPDIF OUT => SPDIF Trasmitter(sense)
-*/
+ */
/* CS4210 board names */
static const struct hda_model_fixup cs421x_models[] = {
@@ -866,6 +867,7 @@ static void cs421x_fixup_sense_b(struct hda_codec *codec,
const struct hda_fixup *fix, int action)
{
struct cs_spec *spec = codec->spec;
+
if (action == HDA_FIXUP_ACT_PRE_PROBE)
spec->sense_b = 1;
}
@@ -891,9 +893,9 @@ static const struct hda_verb cs421x_coef_init_verbs[] = {
{0x0B, AC_VERB_SET_PROC_STATE, 1},
{0x0B, AC_VERB_SET_COEF_INDEX, CS421X_IDX_DEV_CFG},
/*
- Disable Coefficient Index Auto-Increment(DAI)=1,
- PDREF=0
- */
+ * Disable Coefficient Index Auto-Increment(DAI)=1,
+ * PDREF=0
+ */
{0x0B, AC_VERB_SET_PROC_COEF, 0x0001 },
{0x0B, AC_VERB_SET_COEF_INDEX, CS421X_IDX_ADC_CFG},
@@ -980,12 +982,12 @@ static int cs421x_boost_vol_put(struct snd_kcontrol *kcontrol,
coef &= ~0x0003;
coef |= (vol & 0x0003);
- if (original_coef == coef)
- return 0;
- else {
+ if (original_coef != coef) {
cs_vendor_coef_set(codec, CS421X_IDX_SPK_CTL, coef);
return 1;
}
+
+ return 0;
}
static const struct snd_kcontrol_new cs421x_speaker_boost_ctl = {
@@ -1024,8 +1026,8 @@ static void cs4210_pinmux_init(struct hda_codec *codec)
is_active_pin(codec, CS421X_DMIC_PIN_NID)) {
/*
- GPIO or SENSE_B forced - disconnect the DMIC pin.
- */
+ * GPIO or SENSE_B forced - disconnect the DMIC pin.
+ */
def_conf = snd_hda_codec_get_pincfg(codec, CS421X_DMIC_PIN_NID);
def_conf &= ~AC_DEFCFG_PORT_CONN;
def_conf |= (AC_JACK_PORT_NONE << AC_DEFCFG_PORT_CONN_SHIFT);
@@ -1064,6 +1066,7 @@ static void parse_cs421x_digital(struct hda_codec *codec)
for (i = 0; i < cfg->dig_outs; i++) {
hda_nid_t nid = cfg->dig_out_pins[i];
+
if (get_wcaps(codec, nid) & AC_WCAP_UNSOL_CAP) {
spec->spdif_detect = 1;
snd_hda_jack_detect_enable_callback(codec, nid,
@@ -1142,9 +1145,9 @@ static int cs421x_parse_auto_config(struct hda_codec *codec)
#ifdef CONFIG_PM
/*
- Manage PDREF, when transitioning to D3hot
- (DAC,ADC) -> D3, PDREF=1, AFG->D3
-*/
+ * Manage PDREF, when transitioning to D3hot
+ * (DAC,ADC) -> D3, PDREF=1, AFG->D3
+ */
static int cs421x_suspend(struct hda_codec *codec)
{
struct cs_spec *spec = codec->spec;
@@ -1195,10 +1198,10 @@ static int patch_cs4210(struct hda_codec *codec)
snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
/*
- Update the GPIO/DMIC/SENSE_B pinmux before the configuration
- is auto-parsed. If GPIO or SENSE_B is forced, DMIC input
- is disabled.
- */
+ * Update the GPIO/DMIC/SENSE_B pinmux before the configuration
+ * is auto-parsed. If GPIO or SENSE_B is forced, DMIC input
+ * is disabled.
+ */
cs4210_pinmux_init(codec);
err = cs421x_parse_auto_config(codec);
@@ -1496,8 +1499,8 @@ static const struct cs8409_cir_param cs8409_cs42l42_hw_cfg[] = {
/* Enable I2C clocks */
static void cs8409_enable_i2c_clock(struct hda_codec *codec, unsigned int flag)
{
- unsigned int retval = 0;
- unsigned int newval = 0;
+ unsigned int retval;
+ unsigned int newval;
retval = cs_vendor_coef_get(codec, 0x0);
newval = (flag) ? (retval | 0x8) : (retval & 0xfffffff7);
@@ -1669,13 +1672,13 @@ static int cs8409_cs42l42_volume_get(struct snd_kcontrol *kcontrol,
}
switch (nid) {
case CS8409_CS42L42_HP_PIN_NID:
- if (chs & 1)
+ if (chs & BIT(0))
*valp++ = spec->cs42l42_hp_volume[0];
- if (chs & 2)
+ if (chs & BIT(1))
*valp++ = spec->cs42l42_hp_volume[1];
break;
case CS8409_CS42L42_AMIC_PIN_NID:
- if (chs & 1)
+ if (chs & BIT(0))
*valp++ = spec->cs42l42_hs_mic_volume[0];
break;
default:
@@ -1693,19 +1696,19 @@ static int cs8409_cs42l42_volume_put(struct snd_kcontrol *kcontrol,
int chs = get_amp_channels(kcontrol);
long *valp = ucontrol->value.integer.value;
int change = 0;
- char vol = 0;
+ char vol;
snd_hda_power_up(codec);
switch (nid) {
case CS8409_CS42L42_HP_PIN_NID:
mutex_lock(&spec->cs8409_i2c_mux);
- if (chs & 1) {
+ if (chs & BIT(0)) {
vol = -(*valp);
change = cs8409_i2c_write(codec, CS42L42_I2C_ADDR,
CS8409_CS42L42_REG_HS_VOLUME_CHA, vol, 1);
valp++;
}
- if (chs & 2) {
+ if (chs & BIT(1)) {
vol = -(*valp);
change |= cs8409_i2c_write(codec, CS42L42_I2C_ADDR,
CS8409_CS42L42_REG_HS_VOLUME_CHB, vol, 1);
@@ -1714,7 +1717,7 @@ static int cs8409_cs42l42_volume_put(struct snd_kcontrol *kcontrol,
break;
case CS8409_CS42L42_AMIC_PIN_NID:
mutex_lock(&spec->cs8409_i2c_mux);
- if (chs & 1) {
+ if (chs & BIT(0)) {
change = cs8409_i2c_write(
codec, CS42L42_I2C_ADDR,
CS8409_CS42L42_REG_AMIC_VOLUME, (char)*valp, 1);
@@ -1878,7 +1881,7 @@ static void cs8409_jack_unsol_event(struct hda_codec *codec, unsigned int res)
int reg_cdc_status;
int reg_hs_status;
int reg_ts_status;
- int type = 0;
+ int type;
struct hda_jack_tbl *jk;
/* jack_unsol_event() will be called every time gpio line changing state.
@@ -1955,7 +1958,7 @@ static void cs8409_jack_unsol_event(struct hda_codec *codec, unsigned int res)
if (status_changed) {
snd_hda_set_pin_ctl(codec, CS8409_CS42L42_SPK_PIN_NID,
- (spec->cs42l42_hp_jack_in)?0 : PIN_OUT);
+ spec->cs42l42_hp_jack_in ? 0 : PIN_OUT);
/* Report jack*/
jk = snd_hda_jack_tbl_get_mst(codec, CS8409_CS42L42_HP_PIN_NID, 0);
@@ -2043,18 +2046,18 @@ static void cs8409_enable_ur(struct hda_codec *codec, int flag)
/* GPIO4 INT# and GPIO3 WAKE# */
snd_hda_codec_write(codec, codec->core.afg,
0, AC_VERB_SET_GPIO_UNSOLICITED_RSP_MASK,
- flag?(GPIO3_INT | GPIO4_INT) : 0);
+ flag ? (GPIO3_INT | GPIO4_INT) : 0);
snd_hda_codec_write(codec, codec->core.afg,
0, AC_VERB_SET_UNSOLICITED_ENABLE,
- flag?AC_UNSOL_ENABLED : 0);
+ flag ? AC_UNSOL_ENABLED : 0);
}
/* Vendor specific HW configuration
* PLL, ASP, I2C, SPI, GPIOs, DMIC etc...
*/
-static int cs8409_cs42l42_hw_init(struct hda_codec *codec)
+static void cs8409_cs42l42_hw_init(struct hda_codec *codec)
{
const struct cs8409_cir_param *seq = cs8409_cs42l42_hw_cfg;
struct cs_spec *spec = codec->spec;
@@ -2094,14 +2097,16 @@ static int cs8409_cs42l42_hw_init(struct hda_codec *codec)
if (spec->cs42l42_volume_init) {
mutex_lock(&spec->cs8409_i2c_mux);
cs8409_i2c_write(codec, CS42L42_I2C_ADDR,
- CS8409_CS42L42_REG_HS_VOLUME_CHA, -spec->cs42l42_hp_volume[0],
+ CS8409_CS42L42_REG_HS_VOLUME_CHA,
+ -spec->cs42l42_hp_volume[0],
1);
cs8409_i2c_write(codec, CS42L42_I2C_ADDR,
- CS8409_CS42L42_REG_HS_VOLUME_CHB, -spec->cs42l42_hp_volume[1],
+ CS8409_CS42L42_REG_HS_VOLUME_CHB,
+ -spec->cs42l42_hp_volume[1],
1);
- cs8409_i2c_write(
- codec, CS42L42_I2C_ADDR,
- CS8409_CS42L42_REG_AMIC_VOLUME, spec->cs42l42_hs_mic_volume[0],
+ cs8409_i2c_write(codec, CS42L42_I2C_ADDR,
+ CS8409_CS42L42_REG_AMIC_VOLUME,
+ spec->cs42l42_hs_mic_volume[0],
1);
mutex_unlock(&spec->cs8409_i2c_mux);
}
@@ -2112,15 +2117,11 @@ static int cs8409_cs42l42_hw_init(struct hda_codec *codec)
/* Enable Unsolicited Response */
cs8409_enable_ur(codec, 1);
-
- return 1;
}
static int cs8409_cs42l42_init(struct hda_codec *codec)
{
- int ret = 0;
-
- ret = snd_hda_gen_init(codec);
+ int ret = snd_hda_gen_init(codec);
if (!ret) {
/* On Dell platforms with suspend D3 mode support we
@@ -2152,9 +2153,9 @@ static const struct hda_codec_ops cs8409_cs42l42_patch_ops = {
static int cs8409_cs42l42_fixup(struct hda_codec *codec)
{
- int err = 0;
+ int err;
struct cs_spec *spec = codec->spec;
- unsigned int pincap = 0;
+ unsigned int caps;
/* Basic initial sequence for specific hw configuration */
snd_hda_sequence_write(codec, cs8409_cs42l42_init_verbs);
@@ -2169,25 +2170,29 @@ static int cs8409_cs42l42_fixup(struct hda_codec *codec)
* 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,
+ caps = snd_hdac_read_parm(&codec->core, CS8409_CS42L42_HP_PIN_NID,
+ AC_PAR_PIN_CAP);
+ if (caps >= 0)
+ snd_hdac_override_parm(&codec->core,
CS8409_CS42L42_HP_PIN_NID, AC_PAR_PIN_CAP,
- (pincap | (AC_PINCAP_IMP_SENSE | AC_PINCAP_PRES_DETECT)));
+ (caps | (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,
+ caps = snd_hdac_read_parm(&codec->core, CS8409_CS42L42_AMIC_PIN_NID,
+ AC_PAR_PIN_CAP);
+ if (caps >= 0)
+ snd_hdac_override_parm(&codec->core,
CS8409_CS42L42_AMIC_PIN_NID, AC_PAR_PIN_CAP,
- (pincap | (AC_PINCAP_IMP_SENSE | AC_PINCAP_PRES_DETECT)));
+ (caps | (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));
+ caps = get_wcaps(codec, CS8409_CS42L42_HP_PIN_NID);
+ if (caps >= 0)
+ snd_hda_override_wcaps(codec, CS8409_CS42L42_HP_PIN_NID,
+ (caps | 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));
+ caps = get_wcaps(codec, CS8409_CS42L42_AMIC_PIN_NID);
+ if (caps >= 0)
+ snd_hda_override_wcaps(codec, CS8409_CS42L42_AMIC_PIN_NID,
+ (caps | AC_WCAP_UNSOL_CAP));
snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
@@ -2209,7 +2214,7 @@ static int cs8409_cs42l42_fixup(struct hda_codec *codec)
snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PROBE);
- return err;
+ return 0;
}
static int cs8409_cs42l42_exec_verb(struct hdac_device *dev,
@@ -2218,11 +2223,8 @@ static int cs8409_cs42l42_exec_verb(struct hdac_device *dev,
struct hda_codec *codec = container_of(dev, struct hda_codec, core);
struct cs_spec *spec = codec->spec;
- unsigned int nid = 0;
- unsigned int verb = 0;
-
- nid = ((cmd >> 20) & 0x07f);
- verb = ((cmd >> 8) & 0x0fff);
+ unsigned int nid = ((cmd >> 20) & 0x07f);
+ unsigned int verb = ((cmd >> 8) & 0x0fff);
/* CS8409 pins have no AC_PINSENSE_PRESENCE
* capabilities. We have to intercept 2 calls for pins 0x24 and 0x34
@@ -2298,9 +2300,6 @@ static int patch_cs8409(struct hda_codec *codec)
spec->cs42l42_mic_jack_in = 0;
err = cs8409_cs42l42_fixup(codec);
-
- if (err > 0)
- err = cs8409_cs42l42_hw_init(codec);
break;
default:
@@ -2309,10 +2308,12 @@ static int patch_cs8409(struct hda_codec *codec)
codec->bus->pci->subsystem_device);
break;
}
- if (err < 0)
- cs_free(codec);
- else
+
+ if (!err) {
+ cs8409_cs42l42_hw_init(codec);
snd_hda_codec_set_name(codec, "CS8409/CS42L42");
+ } else
+ cs_free(codec);
return err;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/4] ALSA: hda/cirrus: Fix CS42L42 Headset Mic volume control name
2021-03-12 18:44 [PATCH 0/4] ALSA: hda/cirrus: Make CS8409 driver more generic by using fixups Vitaly Rodionov
2021-03-12 18:44 ` [PATCH 1/4] ALSA: hda/cirrus: Add error handling into CS8409 I2C functions Vitaly Rodionov
2021-03-12 18:44 ` [PATCH 2/4] ALSA: hda/cirrus: Cleanup patch_cirrus.c code Vitaly Rodionov
@ 2021-03-12 18:44 ` Vitaly Rodionov
2021-03-12 18:44 ` [PATCH 4/4] ALSA: hda/cirrus: Make CS8409 driver more generic by using fixups Vitaly Rodionov
3 siblings, 0 replies; 6+ messages in thread
From: Vitaly Rodionov @ 2021-03-12 18:44 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai
Cc: alsa-devel, patches, linux-kernel, Stefan Binding, You-Sheng Yang
From: Stefan Binding <sbinding@opensource.cirrus.com>
Existing name "Headset Mic Volume Control" causes multiple Microphone
entries to appear in UI. Using name "Mic Volume Control" ensures only a
single Microphone entry exists when the Headset is connected.
Tested on DELL Inspiron-3505, DELL Inspiron-3501, DELL Inspiron-3500
Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
Signed-off-by: Vitaly Rodionov <vitalyr@opensource.cirrus.com>
BugLink: https://bugs.launchpad.net/bugs/1918378
Reported-and-tested-by: You-Sheng Yang <vicamo.yang@canonical.com>
---
sound/pci/hda/patch_cirrus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index 20e2cc433c6e..6495da523ea3 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -1760,7 +1760,7 @@ static const struct snd_kcontrol_new cs8409_cs42l42_hp_volume_mixer = {
static const struct snd_kcontrol_new cs8409_cs42l42_amic_volume_mixer = {
.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
.index = 0,
- .name = "Headset Mic Capture Volume",
+ .name = "Mic Capture Volume",
.subdevice = (HDA_SUBDEV_AMP_FLAG | HDA_SUBDEV_NID_FLAG),
.access = (SNDRV_CTL_ELEM_ACCESS_READWRITE
| SNDRV_CTL_ELEM_ACCESS_TLV_READ),
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] ALSA: hda/cirrus: Make CS8409 driver more generic by using fixups.
2021-03-12 18:44 [PATCH 0/4] ALSA: hda/cirrus: Make CS8409 driver more generic by using fixups Vitaly Rodionov
` (2 preceding siblings ...)
2021-03-12 18:44 ` [PATCH 3/4] ALSA: hda/cirrus: Fix CS42L42 Headset Mic volume control name Vitaly Rodionov
@ 2021-03-12 18:44 ` Vitaly Rodionov
2021-03-12 21:59 ` kernel test robot
3 siblings, 1 reply; 6+ messages in thread
From: Vitaly Rodionov @ 2021-03-12 18:44 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai
Cc: alsa-devel, patches, linux-kernel, Stefan Binding
From: Stefan Binding <sbinding@opensource.cirrus.com>
CS8409/CS42L42 Driver currently does most of the platform specific
setup inside the main body of the code, however, this setup can be
moved into fixup functions, to make the driver more generic.
Making the driver more generic, allows the driver to use the
cs_parse_auto_config function in the patch function. This function
forces all of the ADCs to be permanently powered, which means the
cap_sync_hook function is no longer needed to restart the stream, when
the jack has been ejected.
Since the codec is re-initialized on every init/resume, there is no
need to add specific verbs to be run on init, and instead these can
be combined with the initialization verbs, which are run on init.
In addition, the extra fixup verbs are no longer required, since this
is taken care of elsewhere.
Tested on DELL Inspiron-3505, DELL Inspiron-3501, DELL Inspiron-3500
Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
Signed-off-by: Vitaly Rodionov <vitalyr@opensource.cirrus.com>
---
sound/pci/hda/patch_cirrus.c | 304 ++++++++++++++---------------------
1 file changed, 118 insertions(+), 186 deletions(-)
diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index 6495da523ea3..8cb91ca1720a 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -1292,9 +1292,14 @@ enum {
CS8409_BULLSEYE,
CS8409_WARLOCK,
CS8409_CYBORG,
- CS8409_VERBS,
+ CS8409_FIXUPS,
};
+static void cs8409_cs42l42_fixups(struct hda_codec *codec,
+ const struct hda_fixup *fix, int action);
+static int cs8409_cs42l42_exec_verb(struct hdac_device *dev,
+ unsigned int cmd, unsigned int flags, unsigned int *res);
+
/* Dell Inspiron models with cs8409/cs42l42 */
static const struct hda_model_fixup cs8409_models[] = {
{ .id = CS8409_BULLSEYE, .name = "bullseye" },
@@ -1357,6 +1362,22 @@ static const struct hda_verb cs8409_cs42l42_init_verbs[] = {
{ 0x47, AC_VERB_SET_PROC_COEF, 0x0080 }, /* I2C mode */
{ 0x47, AC_VERB_SET_COEF_INDEX, 0x005b }, /* Set I2C bus speed */
{ 0x47, AC_VERB_SET_PROC_COEF, 0x0200 }, /* 100kHz I2C_STO = 2 */
+ { 0x24, AC_VERB_SET_CONFIG_DEFAULT_BYTES_0, 0xF0 }, /* Widget node ASP-1-TX */
+ { 0x24, AC_VERB_SET_CONFIG_DEFAULT_BYTES_1, 0x20 },
+ { 0x24, AC_VERB_SET_CONFIG_DEFAULT_BYTES_2, 0x21 },
+ { 0x24, AC_VERB_SET_CONFIG_DEFAULT_BYTES_3, 0x04 },
+ { 0x34, AC_VERB_SET_CONFIG_DEFAULT_BYTES_0, 0x50 }, /* Widget node ASP-1-RX0 */
+ { 0x34, AC_VERB_SET_CONFIG_DEFAULT_BYTES_1, 0x20 },
+ { 0x34, AC_VERB_SET_CONFIG_DEFAULT_BYTES_2, 0xa1 },
+ { 0x34, AC_VERB_SET_CONFIG_DEFAULT_BYTES_3, 0x04 },
+ { 0x2C, AC_VERB_SET_CONFIG_DEFAULT_BYTES_0, 0xF0 }, /* Widget node ASP-2-TX */
+ { 0x2C, AC_VERB_SET_CONFIG_DEFAULT_BYTES_1, 0x00 },
+ { 0x2C, AC_VERB_SET_CONFIG_DEFAULT_BYTES_2, 0x10 },
+ { 0x2C, AC_VERB_SET_CONFIG_DEFAULT_BYTES_3, 0x90 },
+ { 0x44, AC_VERB_SET_CONFIG_DEFAULT_BYTES_0, 0x90 }, /* Widget node DMIC-1 */
+ { 0x44, AC_VERB_SET_CONFIG_DEFAULT_BYTES_1, 0x00 },
+ { 0x44, AC_VERB_SET_CONFIG_DEFAULT_BYTES_2, 0xA0 },
+ { 0x44, AC_VERB_SET_CONFIG_DEFAULT_BYTES_3, 0x90 },
{} /* terminator */
};
@@ -1368,48 +1389,28 @@ static const struct hda_pintbl cs8409_cs42l42_pincfgs[] = {
{} /* terminator */
};
-static const struct hda_verb cs8409_cs42l42_add_verbs[] = {
- { 0x24, 0x71c, 0xF0 }, /* Widget node ASP-1-TX */
- { 0x24, 0x71d, 0x20 },
- { 0x24, 0x71e, 0x21 },
- { 0x24, 0x71f, 0x04 },
- { 0x34, 0x71c, 0x50 }, /* Widget node ASP-1-RX0 */
- { 0x34, 0x71d, 0x20 },
- { 0x34, 0x71e, 0xa1 },
- { 0x34, 0x71f, 0x04 },
- { 0x2C, 0x71c, 0xF0 }, /* Widget node ASP-2-TX */
- { 0x2C, 0x71d, 0x00 },
- { 0x2C, 0x71e, 0x10 },
- { 0x2C, 0x71f, 0x90 },
- { 0x44, 0x71c, 0x90 }, /* Widget node DMIC-1 */
- { 0x44, 0x71d, 0x00 },
- { 0x44, 0x71e, 0xA0 },
- { 0x44, 0x71f, 0x90 },
- {} /* terminator */
-};
-
static const struct hda_fixup cs8409_fixups[] = {
[CS8409_BULLSEYE] = {
.type = HDA_FIXUP_PINS,
.v.pins = cs8409_cs42l42_pincfgs,
.chained = true,
- .chain_id = CS8409_VERBS,
+ .chain_id = CS8409_FIXUPS,
},
[CS8409_WARLOCK] = {
.type = HDA_FIXUP_PINS,
.v.pins = cs8409_cs42l42_pincfgs,
.chained = true,
- .chain_id = CS8409_VERBS,
+ .chain_id = CS8409_FIXUPS,
},
[CS8409_CYBORG] = {
.type = HDA_FIXUP_PINS,
.v.pins = cs8409_cs42l42_pincfgs,
.chained = true,
- .chain_id = CS8409_VERBS,
+ .chain_id = CS8409_FIXUPS,
},
- [CS8409_VERBS] = {
- .type = HDA_FIXUP_VERBS,
- .v.verbs = cs8409_cs42l42_add_verbs,
+ [CS8409_FIXUPS] = {
+ .type = HDA_FIXUP_FUNC,
+ .v.func = cs8409_cs42l42_fixups,
},
};
@@ -1975,26 +1976,6 @@ static void cs8409_jack_unsol_event(struct hda_codec *codec, unsigned int res)
}
}
-static int cs8409_cs42l42_build_controls(struct hda_codec *codec)
-{
- int err;
-
- err = snd_hda_gen_build_controls(codec);
- if (err < 0)
- return err;
-
- snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_BUILD);
-
- /* Run jack auto detect first time on boot
- * after controls have been added, to check if jack has
- * been already plugged in
- */
- cs8409_cs42l42_run_jack_detect(codec);
- usleep_range(100000, 150000);
-
- return 0;
-}
-
#ifdef CONFIG_PM
/* Manage PDREF, when transition to D3hot */
static int cs8409_suspend(struct hda_codec *codec)
@@ -2015,31 +1996,6 @@ static int cs8409_suspend(struct hda_codec *codec)
}
#endif
-static void cs8409_cs42l42_cap_sync_hook(struct hda_codec *codec,
- struct snd_kcontrol *kcontrol,
- struct snd_ctl_elem_value *ucontrol)
-{
- struct cs_spec *spec = codec->spec;
- unsigned int curval, expval;
- /* CS8409 DMIC Pin only allows the setting of the Stream Parameters in
- * Power State D0. When a headset is unplugged, and the path is switched to
- * the DMIC, the Stream is restarted with the new ADC, but this is done in
- * Power State D3. Restart the Stream now DMIC is in D0.
- */
- if (spec->gen.cur_adc == CS8409_CS42L42_DMIC_ADC_PIN_NID) {
- curval = snd_hda_codec_read(codec, spec->gen.cur_adc,
- 0, AC_VERB_GET_CONV, 0);
- expval = (spec->gen.cur_adc_stream_tag << 4) | 0;
- if (curval != expval) {
- codec_dbg(codec, "%s Restarting Stream after DMIC switch\n", __func__);
- __snd_hda_codec_cleanup_stream(codec, spec->gen.cur_adc, 1);
- snd_hda_codec_setup_stream(codec, spec->gen.cur_adc,
- spec->gen.cur_adc_stream_tag, 0,
- spec->gen.cur_adc_format);
- }
- }
-}
-
/* Enable/Disable Unsolicited Response for gpio(s) 3,4 */
static void cs8409_enable_ur(struct hda_codec *codec, int flag)
{
@@ -2123,25 +2079,14 @@ static int cs8409_cs42l42_init(struct hda_codec *codec)
{
int ret = snd_hda_gen_init(codec);
- if (!ret) {
- /* On Dell platforms with suspend D3 mode support we
- * have to re-initialise cs8409 bridge and companion
- * cs42l42 codec
- */
- snd_hda_sequence_write(codec, cs8409_cs42l42_init_verbs);
- snd_hda_sequence_write(codec, cs8409_cs42l42_add_verbs);
-
- cs8409_cs42l42_hw_init(codec);
-
- cs8409_cs42l42_run_jack_detect(codec);
- usleep_range(100000, 150000);
- }
+ if (!ret)
+ snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_INIT);
return ret;
}
static const struct hda_codec_ops cs8409_cs42l42_patch_ops = {
- .build_controls = cs8409_cs42l42_build_controls,
+ .build_controls = cs_build_controls,
.build_pcms = snd_hda_gen_build_pcms,
.init = cs8409_cs42l42_init,
.free = cs_free,
@@ -2151,70 +2096,95 @@ static const struct hda_codec_ops cs8409_cs42l42_patch_ops = {
#endif
};
-static int cs8409_cs42l42_fixup(struct hda_codec *codec)
+static void cs8409_cs42l42_fixups(struct hda_codec *codec,
+ const struct hda_fixup *fix, int action)
{
- int err;
struct cs_spec *spec = codec->spec;
unsigned int caps;
- /* 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.
- */
- caps = snd_hdac_read_parm(&codec->core, CS8409_CS42L42_HP_PIN_NID,
- AC_PAR_PIN_CAP);
- if (caps >= 0)
- snd_hdac_override_parm(&codec->core,
- CS8409_CS42L42_HP_PIN_NID, AC_PAR_PIN_CAP,
- (caps | (AC_PINCAP_IMP_SENSE | AC_PINCAP_PRES_DETECT)));
-
- caps = snd_hdac_read_parm(&codec->core, CS8409_CS42L42_AMIC_PIN_NID,
- AC_PAR_PIN_CAP);
- if (caps >= 0)
- snd_hdac_override_parm(&codec->core,
- CS8409_CS42L42_AMIC_PIN_NID, AC_PAR_PIN_CAP,
- (caps | (AC_PINCAP_IMP_SENSE | AC_PINCAP_PRES_DETECT)));
-
- caps = get_wcaps(codec, CS8409_CS42L42_HP_PIN_NID);
- if (caps >= 0)
- snd_hda_override_wcaps(codec, CS8409_CS42L42_HP_PIN_NID,
- (caps | AC_WCAP_UNSOL_CAP));
-
- caps = get_wcaps(codec, CS8409_CS42L42_AMIC_PIN_NID);
- if (caps >= 0)
- snd_hda_override_wcaps(codec, CS8409_CS42L42_AMIC_PIN_NID,
- (caps | AC_WCAP_UNSOL_CAP));
+ switch (action) {
+ case HDA_FIXUP_ACT_PRE_PROBE:
+ snd_hda_add_verbs(codec, cs8409_cs42l42_init_verbs);
+ /* verb exec op override */
+ spec->exec_verb = codec->core.exec_verb;
+ codec->core.exec_verb = cs8409_cs42l42_exec_verb;
- snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
+ mutex_init(&spec->cs8409_i2c_mux);
- err = snd_hda_parse_pin_defcfg(codec, &spec->gen.autocfg, 0, 0);
- if (err < 0)
- return err;
+ codec->patch_ops = cs8409_cs42l42_patch_ops;
- err = snd_hda_gen_parse_auto_config(codec, &spec->gen.autocfg);
- if (err < 0)
- return err;
+ spec->gen.suppress_auto_mute = 1;
+ spec->gen.no_primary_hp = 1;
+ spec->gen.suppress_vmaster = 1;
- if (!snd_hda_gen_add_kctl(
- &spec->gen, NULL, &cs8409_cs42l42_hp_volume_mixer))
- return -1;
+ /* GPIO 5 out, 3,4 in */
+ spec->gpio_dir = GPIO5_INT;
+ spec->gpio_data = 0;
+ spec->gpio_mask = 0x03f;
- if (!snd_hda_gen_add_kctl(
- &spec->gen, NULL, &cs8409_cs42l42_amic_volume_mixer))
- return -1;
+ spec->cs42l42_hp_jack_in = 0;
+ spec->cs42l42_mic_jack_in = 0;
- snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PROBE);
+ /* Basic initial sequence for specific hw configuration */
+ snd_hda_sequence_write(codec, cs8409_cs42l42_init_verbs);
- return 0;
+ /* 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.
+ */
+ caps = snd_hdac_read_parm(&codec->core, CS8409_CS42L42_HP_PIN_NID,
+ AC_PAR_PIN_CAP);
+ if (caps >= 0)
+ snd_hdac_override_parm(&codec->core,
+ CS8409_CS42L42_HP_PIN_NID, AC_PAR_PIN_CAP,
+ (caps | (AC_PINCAP_IMP_SENSE | AC_PINCAP_PRES_DETECT)));
+
+ caps = snd_hdac_read_parm(&codec->core, CS8409_CS42L42_AMIC_PIN_NID,
+ AC_PAR_PIN_CAP);
+ if (caps >= 0)
+ snd_hdac_override_parm(&codec->core,
+ CS8409_CS42L42_AMIC_PIN_NID, AC_PAR_PIN_CAP,
+ (caps | (AC_PINCAP_IMP_SENSE | AC_PINCAP_PRES_DETECT)));
+
+ caps = get_wcaps(codec, CS8409_CS42L42_HP_PIN_NID);
+ if (caps >= 0)
+ snd_hda_override_wcaps(codec, CS8409_CS42L42_HP_PIN_NID,
+ (caps | AC_WCAP_UNSOL_CAP));
+
+ caps = get_wcaps(codec, CS8409_CS42L42_AMIC_PIN_NID);
+ if (caps >= 0)
+ snd_hda_override_wcaps(codec, CS8409_CS42L42_AMIC_PIN_NID,
+ (caps | AC_WCAP_UNSOL_CAP));
+ break;
+ case HDA_FIXUP_ACT_PROBE:
+ snd_hda_gen_add_kctl(&spec->gen,
+ NULL, &cs8409_cs42l42_hp_volume_mixer);
+ snd_hda_gen_add_kctl(&spec->gen,
+ NULL, &cs8409_cs42l42_amic_volume_mixer);
+ cs8409_cs42l42_hw_init(codec);
+ snd_hda_codec_set_name(codec, "CS8409/CS42L42");
+ break;
+ case HDA_FIXUP_ACT_INIT:
+ cs8409_cs42l42_hw_init(codec);
+ // Fall through
+ case HDA_FIXUP_ACT_BUILD:
+ /* Run jack auto detect first time on boot
+ * after controls have been added, to check if jack has
+ * been already plugged in.
+ * Run immediately after init.
+ */
+ cs8409_cs42l42_run_jack_detect(codec);
+ usleep_range(100000, 150000);
+ break;
+ default:
+ break;
+ }
}
static int cs8409_cs42l42_exec_verb(struct hdac_device *dev,
@@ -2255,11 +2225,9 @@ static int cs8409_cs42l42_exec_verb(struct hdac_device *dev,
static int patch_cs8409(struct hda_codec *codec)
{
- struct cs_spec *spec;
- int err = -EINVAL;
+ int err;
- spec = cs_alloc_spec(codec, CS8409_VENDOR_NID);
- if (!spec)
+ if (!cs_alloc_spec(codec, CS8409_VENDOR_NID))
return -ENOMEM;
snd_hda_pick_fixup(codec,
@@ -2270,52 +2238,16 @@ static int patch_cs8409(struct hda_codec *codec)
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);
-
- /* verb exec op override */
- spec->exec_verb = codec->core.exec_verb;
- codec->core.exec_verb = cs8409_cs42l42_exec_verb;
-
- mutex_init(&spec->cs8409_i2c_mux);
-
- codec->patch_ops = cs8409_cs42l42_patch_ops;
-
- spec->gen.cap_sync_hook = cs8409_cs42l42_cap_sync_hook;
-
- spec->gen.suppress_auto_mute = 1;
- spec->gen.no_primary_hp = 1;
- spec->gen.suppress_vmaster = 1;
- /* GPIO 5 out, 3,4 in */
- spec->gpio_dir = GPIO5_INT;
- spec->gpio_data = 0;
- spec->gpio_mask = 0x03f;
-
- spec->cs42l42_hp_jack_in = 0;
- spec->cs42l42_mic_jack_in = 0;
-
- err = cs8409_cs42l42_fixup(codec);
- break;
-
- default:
- codec_err(codec, "VID=%08x, DEV=%08x not supported\n",
- codec->bus->pci->subsystem_vendor,
- codec->bus->pci->subsystem_device);
- break;
- }
+ snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
- if (!err) {
- cs8409_cs42l42_hw_init(codec);
- snd_hda_codec_set_name(codec, "CS8409/CS42L42");
- } else
+ err = cs_parse_auto_config(codec);
+ if (err < 0) {
cs_free(codec);
+ return err;
+ }
- return err;
+ snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PROBE);
+ return 0;
}
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 4/4] ALSA: hda/cirrus: Make CS8409 driver more generic by using fixups.
2021-03-12 18:44 ` [PATCH 4/4] ALSA: hda/cirrus: Make CS8409 driver more generic by using fixups Vitaly Rodionov
@ 2021-03-12 21:59 ` kernel test robot
0 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2021-03-12 21:59 UTC (permalink / raw)
To: Vitaly Rodionov, Jaroslav Kysela, Takashi Iwai
Cc: kbuild-all, alsa-devel, patches, linux-kernel, Stefan Binding
[-- Attachment #1: Type: text/plain, Size: 8986 bytes --]
Hi Vitaly,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on sound/for-next]
[also build test WARNING on next-20210312]
[cannot apply to v5.12-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Vitaly-Rodionov/ALSA-hda-cirrus-Make-CS8409-driver-more-generic-by-using-fixups/20210313-024937
base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
config: microblaze-randconfig-m031-20210312 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
New smatch warnings:
sound/pci/hda/patch_cirrus.c:2143 cs8409_cs42l42_fixups() warn: always true condition '(caps >= 0) => (0-u32max >= 0)'
Old smatch warnings:
sound/pci/hda/patch_cirrus.c:2150 cs8409_cs42l42_fixups() warn: always true condition '(caps >= 0) => (0-u32max >= 0)'
sound/pci/hda/patch_cirrus.c:2156 cs8409_cs42l42_fixups() warn: always true condition '(caps >= 0) => (0-u32max >= 0)'
sound/pci/hda/patch_cirrus.c:2161 cs8409_cs42l42_fixups() warn: always true condition '(caps >= 0) => (0-u32max >= 0)'
vim +2143 sound/pci/hda/patch_cirrus.c
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2098
89b3eea4b1e18f Stefan Binding 2021-03-12 2099 static void cs8409_cs42l42_fixups(struct hda_codec *codec,
89b3eea4b1e18f Stefan Binding 2021-03-12 2100 const struct hda_fixup *fix, int action)
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2101 {
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2102 struct cs_spec *spec = codec->spec;
76e2b57d1d60d1 Stefan Binding 2021-03-12 2103 unsigned int caps;
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2104
89b3eea4b1e18f Stefan Binding 2021-03-12 2105 switch (action) {
89b3eea4b1e18f Stefan Binding 2021-03-12 2106 case HDA_FIXUP_ACT_PRE_PROBE:
89b3eea4b1e18f Stefan Binding 2021-03-12 2107 snd_hda_add_verbs(codec, cs8409_cs42l42_init_verbs);
89b3eea4b1e18f Stefan Binding 2021-03-12 2108 /* verb exec op override */
89b3eea4b1e18f Stefan Binding 2021-03-12 2109 spec->exec_verb = codec->core.exec_verb;
89b3eea4b1e18f Stefan Binding 2021-03-12 2110 codec->core.exec_verb = cs8409_cs42l42_exec_verb;
89b3eea4b1e18f Stefan Binding 2021-03-12 2111
89b3eea4b1e18f Stefan Binding 2021-03-12 2112 mutex_init(&spec->cs8409_i2c_mux);
89b3eea4b1e18f Stefan Binding 2021-03-12 2113
89b3eea4b1e18f Stefan Binding 2021-03-12 2114 codec->patch_ops = cs8409_cs42l42_patch_ops;
89b3eea4b1e18f Stefan Binding 2021-03-12 2115
89b3eea4b1e18f Stefan Binding 2021-03-12 2116 spec->gen.suppress_auto_mute = 1;
89b3eea4b1e18f Stefan Binding 2021-03-12 2117 spec->gen.no_primary_hp = 1;
89b3eea4b1e18f Stefan Binding 2021-03-12 2118 spec->gen.suppress_vmaster = 1;
89b3eea4b1e18f Stefan Binding 2021-03-12 2119
89b3eea4b1e18f Stefan Binding 2021-03-12 2120 /* GPIO 5 out, 3,4 in */
89b3eea4b1e18f Stefan Binding 2021-03-12 2121 spec->gpio_dir = GPIO5_INT;
89b3eea4b1e18f Stefan Binding 2021-03-12 2122 spec->gpio_data = 0;
89b3eea4b1e18f Stefan Binding 2021-03-12 2123 spec->gpio_mask = 0x03f;
89b3eea4b1e18f Stefan Binding 2021-03-12 2124
89b3eea4b1e18f Stefan Binding 2021-03-12 2125 spec->cs42l42_hp_jack_in = 0;
89b3eea4b1e18f Stefan Binding 2021-03-12 2126 spec->cs42l42_mic_jack_in = 0;
89b3eea4b1e18f Stefan Binding 2021-03-12 2127
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2128 /* Basic initial sequence for specific hw configuration */
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2129 snd_hda_sequence_write(codec, cs8409_cs42l42_init_verbs);
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2130
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2131 /* CS8409 is simple HDA bridge and intended to be used with a remote
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2132 * companion codec. Most of input/output PIN(s) have only basic
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2133 * capabilities. NID(s) 0x24 and 0x34 have only OUTC and INC
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2134 * capabilities and no presence detect capable (PDC) and call to
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2135 * snd_hda_gen_build_controls() will mark them as non detectable
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2136 * phantom jacks. However, in this configuration companion codec
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2137 * CS42L42 is connected to these pins and it has jack detect
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2138 * capabilities. We have to override pin capabilities,
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2139 * otherwise they will not be created as input devices.
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2140 */
76e2b57d1d60d1 Stefan Binding 2021-03-12 2141 caps = snd_hdac_read_parm(&codec->core, CS8409_CS42L42_HP_PIN_NID,
76e2b57d1d60d1 Stefan Binding 2021-03-12 2142 AC_PAR_PIN_CAP);
76e2b57d1d60d1 Stefan Binding 2021-03-12 @2143 if (caps >= 0)
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2144 snd_hdac_override_parm(&codec->core,
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2145 CS8409_CS42L42_HP_PIN_NID, AC_PAR_PIN_CAP,
76e2b57d1d60d1 Stefan Binding 2021-03-12 2146 (caps | (AC_PINCAP_IMP_SENSE | AC_PINCAP_PRES_DETECT)));
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2147
76e2b57d1d60d1 Stefan Binding 2021-03-12 2148 caps = snd_hdac_read_parm(&codec->core, CS8409_CS42L42_AMIC_PIN_NID,
76e2b57d1d60d1 Stefan Binding 2021-03-12 2149 AC_PAR_PIN_CAP);
76e2b57d1d60d1 Stefan Binding 2021-03-12 2150 if (caps >= 0)
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2151 snd_hdac_override_parm(&codec->core,
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2152 CS8409_CS42L42_AMIC_PIN_NID, AC_PAR_PIN_CAP,
76e2b57d1d60d1 Stefan Binding 2021-03-12 2153 (caps | (AC_PINCAP_IMP_SENSE | AC_PINCAP_PRES_DETECT)));
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2154
76e2b57d1d60d1 Stefan Binding 2021-03-12 2155 caps = get_wcaps(codec, CS8409_CS42L42_HP_PIN_NID);
76e2b57d1d60d1 Stefan Binding 2021-03-12 2156 if (caps >= 0)
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2157 snd_hda_override_wcaps(codec, CS8409_CS42L42_HP_PIN_NID,
76e2b57d1d60d1 Stefan Binding 2021-03-12 2158 (caps | AC_WCAP_UNSOL_CAP));
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2159
76e2b57d1d60d1 Stefan Binding 2021-03-12 2160 caps = get_wcaps(codec, CS8409_CS42L42_AMIC_PIN_NID);
76e2b57d1d60d1 Stefan Binding 2021-03-12 2161 if (caps >= 0)
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2162 snd_hda_override_wcaps(codec, CS8409_CS42L42_AMIC_PIN_NID,
76e2b57d1d60d1 Stefan Binding 2021-03-12 2163 (caps | AC_WCAP_UNSOL_CAP));
89b3eea4b1e18f Stefan Binding 2021-03-12 2164 break;
89b3eea4b1e18f Stefan Binding 2021-03-12 2165 case HDA_FIXUP_ACT_PROBE:
89b3eea4b1e18f Stefan Binding 2021-03-12 2166 snd_hda_gen_add_kctl(&spec->gen,
89b3eea4b1e18f Stefan Binding 2021-03-12 2167 NULL, &cs8409_cs42l42_hp_volume_mixer);
89b3eea4b1e18f Stefan Binding 2021-03-12 2168 snd_hda_gen_add_kctl(&spec->gen,
89b3eea4b1e18f Stefan Binding 2021-03-12 2169 NULL, &cs8409_cs42l42_amic_volume_mixer);
89b3eea4b1e18f Stefan Binding 2021-03-12 2170 cs8409_cs42l42_hw_init(codec);
89b3eea4b1e18f Stefan Binding 2021-03-12 2171 snd_hda_codec_set_name(codec, "CS8409/CS42L42");
89b3eea4b1e18f Stefan Binding 2021-03-12 2172 break;
89b3eea4b1e18f Stefan Binding 2021-03-12 2173 case HDA_FIXUP_ACT_INIT:
89b3eea4b1e18f Stefan Binding 2021-03-12 2174 cs8409_cs42l42_hw_init(codec);
89b3eea4b1e18f Stefan Binding 2021-03-12 2175 // Fall through
89b3eea4b1e18f Stefan Binding 2021-03-12 2176 case HDA_FIXUP_ACT_BUILD:
89b3eea4b1e18f Stefan Binding 2021-03-12 2177 /* Run jack auto detect first time on boot
89b3eea4b1e18f Stefan Binding 2021-03-12 2178 * after controls have been added, to check if jack has
89b3eea4b1e18f Stefan Binding 2021-03-12 2179 * been already plugged in.
89b3eea4b1e18f Stefan Binding 2021-03-12 2180 * Run immediately after init.
89b3eea4b1e18f Stefan Binding 2021-03-12 2181 */
89b3eea4b1e18f Stefan Binding 2021-03-12 2182 cs8409_cs42l42_run_jack_detect(codec);
89b3eea4b1e18f Stefan Binding 2021-03-12 2183 usleep_range(100000, 150000);
89b3eea4b1e18f Stefan Binding 2021-03-12 2184 break;
89b3eea4b1e18f Stefan Binding 2021-03-12 2185 default:
89b3eea4b1e18f Stefan Binding 2021-03-12 2186 break;
89b3eea4b1e18f Stefan Binding 2021-03-12 2187 }
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2188 }
6cc7e93f46a5ce Vitaly Rodionov 2021-03-06 2189
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28024 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-12 22:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-12 18:44 [PATCH 0/4] ALSA: hda/cirrus: Make CS8409 driver more generic by using fixups Vitaly Rodionov
2021-03-12 18:44 ` [PATCH 1/4] ALSA: hda/cirrus: Add error handling into CS8409 I2C functions Vitaly Rodionov
2021-03-12 18:44 ` [PATCH 2/4] ALSA: hda/cirrus: Cleanup patch_cirrus.c code Vitaly Rodionov
2021-03-12 18:44 ` [PATCH 3/4] ALSA: hda/cirrus: Fix CS42L42 Headset Mic volume control name Vitaly Rodionov
2021-03-12 18:44 ` [PATCH 4/4] ALSA: hda/cirrus: Make CS8409 driver more generic by using fixups Vitaly Rodionov
2021-03-12 21:59 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).