| Message ID | 20211227153344.155803-5-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/5] ASoC: rt5640: Change jack_work to a delayed_work | expand |
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? Thanks, Stephan
Hi, 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. 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. and yes this means that we *could* also use BYT_RT5640_JD_SRC_EXT_GPIO on the ME176C (and actually on quite a few BYT devices) but it does not gain us anything, it is just another method of getting the same information, since the jack-inserted switch is connected to both the codec and the Bay Trail SoC. Regards, Hans
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
diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c index f37ab44ae957..2ace32c03ec9 100644 --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -40,6 +40,8 @@ enum { BYT_RT5640_NO_INTERNAL_MIC_MAP, }; +#define RT5640_JD_SRC_EXT_GPIO 0x0f + enum { BYT_RT5640_JD_SRC_GPIO1 = (RT5640_JD_SRC_GPIO1 << 4), BYT_RT5640_JD_SRC_JD1_IN4P = (RT5640_JD_SRC_JD1_IN4P << 4), @@ -47,6 +49,7 @@ enum { BYT_RT5640_JD_SRC_GPIO2 = (RT5640_JD_SRC_GPIO2 << 4), BYT_RT5640_JD_SRC_GPIO3 = (RT5640_JD_SRC_GPIO3 << 4), BYT_RT5640_JD_SRC_GPIO4 = (RT5640_JD_SRC_GPIO4 << 4), + BYT_RT5640_JD_SRC_EXT_GPIO = (RT5640_JD_SRC_EXT_GPIO << 4) }; enum { @@ -627,6 +630,19 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = { BYT_RT5640_SSP0_AIF2 | BYT_RT5640_MCLK_EN), }, + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), + DMI_MATCH(DMI_PRODUCT_NAME, "TF103C"), + }, + .driver_data = (void *)(BYT_RT5640_IN1_MAP | + BYT_RT5640_JD_SRC_EXT_GPIO | + BYT_RT5640_OVCD_TH_2000UA | + BYT_RT5640_OVCD_SF_0P75 | + BYT_RT5640_SSP0_AIF1 | + BYT_RT5640_MCLK_EN | + BYT_RT5640_USE_AMCR0F28), + }, { /* Chuwi Vi8 (CWI506) */ .matches = { DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Insyde"), @@ -1083,9 +1099,11 @@ static int byt_rt5640_add_codec_device_props(struct device *i2c_dev, } if (BYT_RT5640_JDSRC(byt_rt5640_quirk)) { - props[cnt++] = PROPERTY_ENTRY_U32( - "realtek,jack-detect-source", - BYT_RT5640_JDSRC(byt_rt5640_quirk)); + if (BYT_RT5640_JDSRC(byt_rt5640_quirk) != RT5640_JD_SRC_EXT_GPIO) { + props[cnt++] = PROPERTY_ENTRY_U32( + "realtek,jack-detect-source", + BYT_RT5640_JDSRC(byt_rt5640_quirk)); + } props[cnt++] = PROPERTY_ENTRY_U32( "realtek,over-current-threshold-microamp", @@ -1113,6 +1131,13 @@ static int byt_rt5640_add_codec_device_props(struct device *i2c_dev, } /* Some Android devs specify IRQs/GPIOS in a special AMCR0F28 ACPI device */ +static const struct acpi_gpio_params amcr0f28_jd_gpio = { 1, 0, false }; + +static const struct acpi_gpio_mapping amcr0f28_gpios[] = { + { "rt5640-jd-gpios", &amcr0f28_jd_gpio, 1 }, + { } +}; + static int byt_rt5640_get_amcr0f28_settings(struct snd_soc_card *card) { struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card); @@ -1133,6 +1158,18 @@ static int byt_rt5640_get_amcr0f28_settings(struct snd_soc_card *card) goto put_adev; } + if (BYT_RT5640_JDSRC(byt_rt5640_quirk) == RT5640_JD_SRC_EXT_GPIO) { + acpi_dev_add_driver_gpios(adev, amcr0f28_gpios); + data->jd_gpio = devm_fwnode_gpiod_get(card->dev, acpi_fwnode_handle(adev), + "rt5640-jd", GPIOD_IN, "rt5640-jd"); + acpi_dev_remove_driver_gpios(adev); + + if (IS_ERR(data->jd_gpio)) { + ret = PTR_ERR(data->jd_gpio); + dev_err(card->dev, "error %d getting jd GPIO\n", ret); + } + } + put_adev: acpi_dev_put(adev); return ret;
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. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- sound/soc/intel/boards/bytcr_rt5640.c | 43 +++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-)