Hi,
On 12/28/20 1:21 PM, Mark Brown wrote:
On Sun, Dec 27, 2020 at 10:12:19PM +0100, Hans de Goede wrote:
The Linux Arizona driver uses the MFD framework to create several sub-devices for the Arizona codec and then uses a driver per function.
The jack-detect support for the Arizona codec is handled by the extcon-arizona driver. This driver exports info about the jack state to userspace through the standard extcon sysfs class interface.
But standard Linux userspace does not monitor/use the extcon sysfs interface for jack-detection.
This seems like the wrong layer to fix this problem at, this issue will apply to all extcon devices that can detect audio.
Well, the problem really is that using extcon to report jack-state is rather unusual to do, extcon-arizona.c is the only extcon driver which deals with jack-state (typically extcon is used for things like determining the type of charger connected to an USB charging port):
[hans@x1 linux]$ grep -lr EXTCON_JACK_HEADPHONE drivers/extcon/ drivers/extcon/extcon-arizona.c drivers/extcon/extcon.c
And more in general AFAIK extcon is sort of deprecated and it is not advised to use it for new code. I would esp. not expect it to be used for new jack-detection code since we already have standard uAPI support for that through sound/core/jack.c .
So extcon-arizona really is the odd duck here and writing some generic extcon to sound/core/jack.c glue seems unnecessary since we are just trying dealing with one special case here.
Also at first I tried to use extcon-glue like code in sound/soc/intel/boards/bytcr_wm5102.c making it listen for extcon events and have sound/soc/intel/boards/bytcr_wm5102.c report jack events instead of sharing the jack with extcon-arizona.c through the shared MFD data struct. But that did not work, because the extcon-arizona.c probe function already (before this patch-set) has this:
if (!arizona->dapm || !arizona->dapm->card) return -EPROBE_DEFER;
Which means that the sound/soc/intel/boards/bytcr_wm5102.c machine driver must first complete calling devm_snd_soc_register_card() before the extcon driver will bind and register the extcon device.
But listening to extcon events requires the machine driver to do an: extcon_get_extcon_dev("arizona-extcon") and as long as that returns NULL, return -EPROBE_DEFER.
So now we have the machine-driver's probe returning with -EPROBE_DEFER until the extcon driver shows up and the other-way around, so neither ever binds.
I could have fixed this by making the machine driver bind without the extcon driver being bound and then poll every second for the extcon device to show up, and once it has shown up stop polling and register the jack, once it has the extcon device.
But that seems quite ugly, so I did not even try to implement that coming up with this solution instead which is much more KISS really.
Also note that sharing the jack is necessary to avoid creating 2 separate input_device-s for the headset, which also looks weird / ugly.
Besides being ugly, there also is another potential problem with polling to wait for the extcon device to show up: the jack must be registered before the card registration completes otherwise snd_jack_dev_register will not run, since we are post registration. But I guess that the sound core might be smart enough to call the dev_register op immediately if the card has already been registered ?
TL;DR: writing a generic solution for what is a special case used in just driver seems like overkill and also writing such a generic solution is not easily possible because of probe ordering issues. So instead I've gone with this approach which is a much simpler solution and as such seems a better way to deal with this special case.
Regards,
Hans