Hi,
On 1/18/21 2:02 PM, Mark Brown wrote:
On Sun, Jan 17, 2021 at 10:22:50PM +0100, Hans de Goede wrote:
- /*
* Some DSDTs wrongly declare the IRQ trigger-type as IRQF_TRIGGER_FALLING* The IRQ line will stay low when a new IRQ event happens between reading* the IRQ status flags and acknowledging them. When the IRQ line stays* low like this the IRQ will never trigger again when its type is set* to IRQF_TRIGGER_FALLING. Correct the IRQ trigger-type to fix this.*/- arizona->pdata.irq_flags = IRQF_TRIGGER_LOW;
Are you sure that all the relevant interrupt controllers support active low interrupts? There were issues on some systems with missing support for active low interrupts (see the bodge in wm8994-irq.c to work around them) - it's entirely likely that there are DSDTs that are just plain buggy here but if someone's copying and pasting it smells like there may be some systems that actually need an edge triggered interrupt that they're getting it from.
I'm only aware of one series of devices / models which actually use the combination of ACPI enumeration and the WM5102 codec, and that is the Lenovo Yoga Tablet 2 series (in 8, 10 and 13 inch versions shipping with both Windows and Android). These all use a Bay Trail SoC which is capable of using active low interrupts.
More in general I'm not aware of any (recent-ish) x86 GPIO controllers not being able to do active low interrupts. In theory we could hit this code path on ARM devices using ACPI enumeration, but I don't think it is likely we will see a combination of ARM + ACPI enumeration + WM5102 + GPIO controller not capable of active-low interrupts.
This overriding of the flags definitely is necessary on the Lenovo devices in question. I could add a "if (dmi_name_in_vendors("LENOVO"))" guard around it, but that seems unnecessary.
Regards,
Hans
- /* Wait 200 ms after jack insertion */
- arizona->pdata.micd_detect_debounce = 200;
- /* Use standard AOSP values for headset-button mappings */
- arizona->pdata.micd_ranges = arizona_micd_aosp_ranges;
- arizona->pdata.num_micd_ranges = ARRAY_SIZE(arizona_micd_aosp_ranges);
- return 0;
+}
+static const struct acpi_device_id arizona_acpi_match[] = {
- {
.id = "WM510204",.driver_data = WM5102,- },
- {
.id = "WM510205",.driver_data = WM5102,- },
- { }
+}; +MODULE_DEVICE_TABLE(acpi, arizona_acpi_match); +#else +static int arizona_spi_acpi_probe(struct arizona *arizona) +{
- return -ENODEV;
+} +#endif
static int arizona_spi_probe(struct spi_device *spi) { const struct spi_device_id *id = spi_get_device_id(spi); @@ -77,6 +191,12 @@ static int arizona_spi_probe(struct spi_device *spi) arizona->dev = &spi->dev; arizona->irq = spi->irq;
- if (has_acpi_companion(&spi->dev)) {
ret = arizona_spi_acpi_probe(arizona);if (ret)return ret;- }
- return arizona_dev_init(arizona);
}
@@ -104,6 +224,7 @@ static struct spi_driver arizona_spi_driver = { .name = "arizona", .pm = &arizona_pm_ops, .of_match_table = of_match_ptr(arizona_of_match),
.acpi_match_table = ACPI_PTR(arizona_acpi_match),-- 2.28.0