On Thu, Oct 17, 2019 at 02:16:09PM +0000, Oleksandr Suvorov wrote: > All versions of driver sgtl5000 (since creating in 2011) has a bug in > sgtl5000_probe(): > ... > snd_soc_write(codec, SGTL5000_CHIP_ANA_CTRL, > SGTL5000_HP_ZCD_EN | > SGTL5000_ADC_ZCD_EN); > ... > This command rewrites the whole register value instead of just enabling > ZCD feature for headphone and adc. > This register has bits for HP/LineOut/ADC muting, thus sgtl5000_probe() > always unmutes HP/LineOut/ADC. Yes, or at the very least this is a badly documented bit of intentional code. I suspect it may be the latter but at this point we can't tell. > 1. drop this patch and revert 631bc8f0134ae in stable versions 4.19, > 5.2, 5.3. > So the bug with unmuting all outputs and ADC on device probing will > still present in all kernel versions that include sgtl500 codec driver. This patch here being adding the userspace control of the switch and 631bc8f0134ae being the patch that made the ZC change only update the specific bits rather than write an absolute value to the register. This means that we end up with the audio unmuted but no user control over this at runtime. From a user perspective I think this is fine, it's not ideal that there's no control but they can still record. Please include human readable descriptions of things like commits and issues being discussed in e-mail in your mails, this makes them much easier for humans to read. > 2. keep 631bc8f0134ae and add 694b14554d75f to 4.19, 5.2 and 5.3. This means the patch that makes ZC only update the ZC bits and also the patch that makes the mutes user controllable, the default being muted. As I pointed out up thread this would mean that someone upgrading to a newer stable may need to change their userspace to do the unmute instead of having things unconditionally unmuted by the driver. This is not really what people expect from stable updates, we want them to be able to pull these in without thinking about it. i To backport the addition of the controls to stable we'd need an additional change which sets the default for this control to unmuted, there's a case for having such a change upstream regardless but it's still not clear if any of these changes are really fixes in the sense that we mean for stable. > 3. add 631bc8f0134ae to 4.4, 4.9 and 4.14 > add 694b14554d75f to 4.4-5.3 > add 904a987345258 to 4.4 This is basically the same as 2 except it adds some more user controllable mute controls with 904a987345258 and does different things in different versions for reasons I'm not clear on. It has the same issue. > So this bug will be fixed in all supported versions. It is not clear that this is even a bug in the first place, it's not full functionality but that doesn't mean that it's a bug it just means that there's some missing functionality.
Attachment:
signature.asc
Description: PGP signature
