[alsa-devel] Found bugs in tlv320aic3x.c driver, how do I report them?
Hi. I came across some bugs in sound/soc/codecs/tlv320aic3x.c, wherein it writes non-zero values to reserved registers on the tlv320aic3104 (I found it in 4.1.4, but it's still in 4.3-rc3). I have an untested proposed patch. But I don't know anything about how to report it or offer it.
What should I do? Thanks.
For reference:
$ git diff -- sound/soc/codecs/tlv320aic3x.c diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 51c4713..79c0ca0 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -1509,14 +1509,24 @@ static int aic3x_init(struct snd_soc_codec *codec) snd_soc_write(codec, PGAL_2_LLOPM_VOL, DEFAULT_VOL); snd_soc_write(codec, PGAR_2_RLOPM_VOL, DEFAULT_VOL);
- /* Line2 to HP Bypass default volume, disconnect from Output Mixer */ - snd_soc_write(codec, LINE2L_2_HPLOUT_VOL, DEFAULT_VOL); - snd_soc_write(codec, LINE2R_2_HPROUT_VOL, DEFAULT_VOL); - snd_soc_write(codec, LINE2L_2_HPLCOM_VOL, DEFAULT_VOL); - snd_soc_write(codec, LINE2R_2_HPRCOM_VOL, DEFAULT_VOL); - /* Line2 Line Out default volume, disconnect from Output Mixer */ - snd_soc_write(codec, LINE2L_2_LLOPM_VOL, DEFAULT_VOL); - snd_soc_write(codec, LINE2R_2_RLOPM_VOL, DEFAULT_VOL); + if (aic3x->model == AIC3X_MODEL_3104) { + /* On tlv320aic3014, these registers are reserved and must be written 0 */ + snd_soc_write(codec, LINE2L_2_HPLOUT_VOL, 0); + snd_soc_write(codec, LINE2R_2_HPROUT_VOL, 0); + snd_soc_write(codec, LINE2L_2_HPLCOM_VOL, 0); + snd_soc_write(codec, LINE2R_2_HPRCOM_VOL, 0); + snd_soc_write(codec, LINE2L_2_LLOPM_VOL, 0); + snd_soc_write(codec, LINE2R_2_RLOPM_VOL, 0); + } else { + /* Line2 to HP Bypass default volume, disconnect from Output Mixer */ + snd_soc_write(codec, LINE2L_2_HPLOUT_VOL, DEFAULT_VOL); + snd_soc_write(codec, LINE2R_2_HPROUT_VOL, DEFAULT_VOL); + snd_soc_write(codec, LINE2L_2_HPLCOM_VOL, DEFAULT_VOL); + snd_soc_write(codec, LINE2R_2_HPRCOM_VOL, DEFAULT_VOL); + /* Line2 Line Out default volume, disconnect from Output Mixer */ + snd_soc_write(codec, LINE2L_2_LLOPM_VOL, DEFAULT_VOL); + snd_soc_write(codec, LINE2R_2_RLOPM_VOL, DEFAULT_VOL); + }
switch (aic3x->model) { case AIC3X_MODEL_3X:
Dear Rick Mann,
On Tue, Sep 29, 2015 at 12:38 PM, Rick Mann rmann@latencyzero.com wrote:
Hi. I came across some bugs in sound/soc/codecs/tlv320aic3x.c, wherein it writes non-zero values to reserved registers on the tlv320aic3104 (I found it in 4.1.4, but it's still in 4.3-rc3). I have an untested proposed patch. But I don't know anything about how to report it or offer it.
What should I do? Thanks.
Follow this: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documen...
For reference:
$ git diff -- sound/soc/codecs/tlv320aic3x.c diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 51c4713..79c0ca0 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -1509,14 +1509,24 @@ static int aic3x_init(struct snd_soc_codec *codec) snd_soc_write(codec, PGAL_2_LLOPM_VOL, DEFAULT_VOL); snd_soc_write(codec, PGAR_2_RLOPM_VOL, DEFAULT_VOL);
/* Line2 to HP Bypass default volume, disconnect from Output Mixer */snd_soc_write(codec, LINE2L_2_HPLOUT_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2R_2_HPROUT_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2L_2_HPLCOM_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2R_2_HPRCOM_VOL, DEFAULT_VOL);/* Line2 Line Out default volume, disconnect from Output Mixer */snd_soc_write(codec, LINE2L_2_LLOPM_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2R_2_RLOPM_VOL, DEFAULT_VOL);
if (aic3x->model == AIC3X_MODEL_3104) {/* On tlv320aic3014, these registers are reserved and must be written 0 */snd_soc_write(codec, LINE2L_2_HPLOUT_VOL, 0);snd_soc_write(codec, LINE2R_2_HPROUT_VOL, 0);snd_soc_write(codec, LINE2L_2_HPLCOM_VOL, 0);snd_soc_write(codec, LINE2R_2_HPRCOM_VOL, 0);snd_soc_write(codec, LINE2L_2_LLOPM_VOL, 0);snd_soc_write(codec, LINE2R_2_RLOPM_VOL, 0);
Nothing should be done for these registers on 3104, not even setting them to 0. The datasheet says "Do not write to this register.".
} else {/* Line2 to HP Bypass default volume, disconnect from Output Mixer */snd_soc_write(codec, LINE2L_2_HPLOUT_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2R_2_HPROUT_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2L_2_HPLCOM_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2R_2_HPRCOM_VOL, DEFAULT_VOL);/* Line2 Line Out default volume, disconnect from Output Mixer */snd_soc_write(codec, LINE2L_2_LLOPM_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2R_2_RLOPM_VOL, DEFAULT_VOL);}
Correct.
switch (aic3x->model) { case AIC3X_MODEL_3X:
Best regards, Benoît
On Sep 29, 2015, at 11:47 , Benoît Thébaudeau benoit.thebaudeau.dev@gmail.com wrote:
Dear Rick Mann,
On Tue, Sep 29, 2015 at 12:38 PM, Rick Mann rmann@latencyzero.com wrote:
Hi. I came across some bugs in sound/soc/codecs/tlv320aic3x.c, wherein it writes non-zero values to reserved registers on the tlv320aic3104 (I found it in 4.1.4, but it's still in 4.3-rc3). I have an untested proposed patch. But I don't know anything about how to report it or offer it.
What should I do? Thanks.
Follow this: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documen...
Thanks, will do.
Nothing should be done for these registers on 3104, not even setting them to 0. The datasheet says "Do not write to this register.".
Indeed, it does say that. I'll change the code.
However, it also says not to write certain bits within a register, which is impossible (e.g. Table 88, Register 86, bit D2, "Reserved. Do not write to this register bit.").
} else {/* Line2 to HP Bypass default volume, disconnect from Output Mixer */snd_soc_write(codec, LINE2L_2_HPLOUT_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2R_2_HPROUT_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2L_2_HPLCOM_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2R_2_HPRCOM_VOL, DEFAULT_VOL);/* Line2 Line Out default volume, disconnect from Output Mixer */snd_soc_write(codec, LINE2L_2_LLOPM_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2R_2_RLOPM_VOL, DEFAULT_VOL);}Correct.
Thanks!
Dear Rick Mann,
On Tue, Sep 29, 2015 at 9:21 PM, Rick Mann rmann@latencyzero.com wrote:
On Sep 29, 2015, at 11:47 , Benoît Thébaudeau benoit.thebaudeau.dev@gmail.com wrote: On Tue, Sep 29, 2015 at 12:38 PM, Rick Mann rmann@latencyzero.com wrote: Nothing should be done for these registers on 3104, not even setting them to 0. The datasheet says "Do not write to this register.".
Indeed, it does say that. I'll change the code.
However, it also says not to write certain bits within a register, which is impossible (e.g. Table 88, Register 86, bit D2, "Reserved. Do not write to this register bit.").
This is just a bug resulting from a copy/paste in the datasheet, but the idea is clear: do not touch reserved items when you can avoid doing so. This bit is even read-only besides being reserved, so writing it can't be harmful. The general rule for similar bits that are writable and reserved is either to write them to 0 or to leave them unchanged when writing the other bits of the register, depending on the chip and registers.
Best regards, Benoît
Against what repo/branch should I submit this patch? broonie/sound/asoc-v4.3-rc2?
On Sep 29, 2015, at 11:47 , Benoît Thébaudeau benoit.thebaudeau.dev@gmail.com wrote:
Dear Rick Mann,
On Tue, Sep 29, 2015 at 12:38 PM, Rick Mann rmann@latencyzero.com wrote:
Hi. I came across some bugs in sound/soc/codecs/tlv320aic3x.c, wherein it writes non-zero values to reserved registers on the tlv320aic3104 (I found it in 4.1.4, but it's still in 4.3-rc3). I have an untested proposed patch. But I don't know anything about how to report it or offer it.
What should I do? Thanks.
Follow this: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documen...
For reference:
$ git diff -- sound/soc/codecs/tlv320aic3x.c diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 51c4713..79c0ca0 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -1509,14 +1509,24 @@ static int aic3x_init(struct snd_soc_codec *codec) snd_soc_write(codec, PGAL_2_LLOPM_VOL, DEFAULT_VOL); snd_soc_write(codec, PGAR_2_RLOPM_VOL, DEFAULT_VOL);
/* Line2 to HP Bypass default volume, disconnect from Output Mixer */snd_soc_write(codec, LINE2L_2_HPLOUT_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2R_2_HPROUT_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2L_2_HPLCOM_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2R_2_HPRCOM_VOL, DEFAULT_VOL);/* Line2 Line Out default volume, disconnect from Output Mixer */snd_soc_write(codec, LINE2L_2_LLOPM_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2R_2_RLOPM_VOL, DEFAULT_VOL);
if (aic3x->model == AIC3X_MODEL_3104) {/* On tlv320aic3014, these registers are reserved and must be written 0 */snd_soc_write(codec, LINE2L_2_HPLOUT_VOL, 0);snd_soc_write(codec, LINE2R_2_HPROUT_VOL, 0);snd_soc_write(codec, LINE2L_2_HPLCOM_VOL, 0);snd_soc_write(codec, LINE2R_2_HPRCOM_VOL, 0);snd_soc_write(codec, LINE2L_2_LLOPM_VOL, 0);snd_soc_write(codec, LINE2R_2_RLOPM_VOL, 0);Nothing should be done for these registers on 3104, not even setting them to 0. The datasheet says "Do not write to this register.".
} else {/* Line2 to HP Bypass default volume, disconnect from Output Mixer */snd_soc_write(codec, LINE2L_2_HPLOUT_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2R_2_HPROUT_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2L_2_HPLCOM_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2R_2_HPRCOM_VOL, DEFAULT_VOL);/* Line2 Line Out default volume, disconnect from Output Mixer */snd_soc_write(codec, LINE2L_2_LLOPM_VOL, DEFAULT_VOL);snd_soc_write(codec, LINE2R_2_RLOPM_VOL, DEFAULT_VOL);}Correct.
switch (aic3x->model) { case AIC3X_MODEL_3X:Best regards, Benoît _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Dear Rick Mann,
On Wed, Sep 30, 2015 at 3:27 AM, Rick Mann rmann@latencyzero.com wrote:
Against what repo/branch should I submit this patch? broonie/sound/asoc-v4.3-rc2?
The affected lines of this driver are probably the same in all the latest trees, so it does not matter much. This branch seems to be the best fit: https://git.kernel.org/cgit/linux/kernel/git/broonie/sound.git/log/?h=for-ne...
Best regards, Benoît
P.S.: Top-posting is highly discouraged on the mailing lists.
participants (2)
-
Benoît Thébaudeau -
Rick Mann