On Thu, Dec 30, 2021 at 08:12:27PM +0100, Hans de Goede wrote: > On 12/30/21 19:56, Stephan Gerhold wrote: > > On Mon, Dec 27, 2021 at 04:33:44PM +0100, Hans de Goede wrote: > >> Some boards have the codec IRQ hooked-up as normally, so the driver can > >> still do things like headset vs headphones and button-press detection, > >> but instead of using one of the JD pins of the codec, an external GPIO > >> is used to report the jack-presence switch status of the jack. > >> > >> Add support for boards which have this setup and which specify which > >> external GPIO to use in the special Android AMCR0F28 ACPI device. > >> > >> And add a quirk for the Asus TF103C tablet which uses this setup. > >> > > > > Can you clarify what exactly is the difference between the setup on > > ME176C and the TF103C? I'm a bit confused why you're using the GpioInt > > as GPIO for TF103C and as IRQ for ME176C. It's GPO2 pin 0x0004 for both > > of them as far as I can tell. > > > > If I remember correctly the vendor kernel from ASUS also used it as > > simple GPIO on ME176C. I'm not sure if it actually belongs to the > > RT5640, I just tried using it in a way that is compatible with your > > headphone detection code. :) > > > > Before I switched to your code I was actually using it as simple GPIO > > similar to your changes here (this could only detect headphones though): > > https://github.com/me176c-dev/linux-me176c/commit/ea3de8e47414371fdeeae819c686f737c02fac7d#diff-28a5a6c5e3db2a315d022023f3cda69ef0475ef036e22dd5ffa0fb4af31c9f81 > > > > In other words, my question is: Should we also use BYT_RT5640_JD_SRC_EXT_GPIO > > for ME176C? Or can TF103C also use the same setup as ME176C? > > The Bay Trail SoC GPO2 pin 4 is connected to the codecs GPIO1 pin, > which is best thought of as the codecs IRQ pin, because that is how > the rt5640 codec driver configures it: > > /* Selecting GPIO01 as an interrupt */ > snd_soc_component_update_bits(component, RT5640_GPIO_CTRL1, > RT5640_GP1_PIN_MASK, RT5640_GP1_PIN_IRQ); > > /* Set GPIO1 output */ > snd_soc_component_update_bits(component, RT5640_GPIO_CTRL3, > RT5640_GP1_PF_MASK, RT5640_GP1_PF_OUT); > > This may seem to be directly connected to the jack-inserted switch > of the physical jack, but it is not, the IRQ just happens to go > low/high together with the jack state (the IRQ handler is sensitive > to both edges). > > But when low (jack inserted), it can go high again even though the > jack is not removed *at all*. This happens on an overcurrent situation > on the mic bias current, which is how heapdhones vs headset are > detected (headphones short the mic contact triggering OVCD). So it > really is a totally different pin from the jack-inserted switch, > it just seems to be directly connected most of the time. > > On the ME176C the actual physical jack-inserted switch is connected > to the JD2 aka IN4N pin of the codec and the codec driver checks > for jack-insertion like this: > > val = snd_soc_component_read(component, RT5640_INT_IRQ_ST); > > if (rt5640->jd_inverted) > return !(val & RT5640_JD_STATUS); > else > return (val & RT5640_JD_STATUS); > > And the physical jack-inserted switch is *also* connected to > the Bay Trail SoC GPO2 pin 27 (second GPIO resource in the AMCROf28 > device), but we ignore that since before this patch-set the rt5640 > codec code assumes the switch is always connected to one of the > codecs JD pins. > Ahh, I think I mixed up the first and second GPIO resource in the AMCR0F28 ACPI device. I thought you're using pin 4 as GPIO here but it's actually pin 27. Looks I was also using pin 27 in my "driver" back then. When quickly looking at the following line in your patch and my "driver" static const struct acpi_gpio_params amcr0f28_jd_gpio = { 1, 0, false }; ... I had a 50 percent chance if it refers to resource 1 or resource 0 and clearly I guessed wrong. :( > > On the Asus TF103C, the codecs GPIO1 aka IRQ pin is connected > to the Bay Trail SoC's GPO2 pin 4 just like on the ME176C. > > What is different is that the jack's physical insertion switch > is *only* connected to the Bay Trail SoC's GPO2 pin 27 and not > connected to any of the codecs JD1/JD2 pins *at all* so we need > to work around that. > > I hope this helps explain. > Yep it does, thanks for the detailed explanation! Stephan
