On Mon, Apr 14, 2025 at 04:40:26PM +0800, Yan, Dongcheng wrote:
On 4/14/2025 4:11 PM, Andy Shevchenko wrote:
On Mon, Apr 14, 2025 at 03:52:50PM +0800, Yan, Dongcheng wrote:
On 4/11/2025 4:33 PM, Hans de Goede wrote:
On 11-Apr-25 10:23 AM, Dongcheng Yan wrote:
...
- case INT3472_GPIO_TYPE_HOTPLUG_DETECT:
*con_id = "hpd";
*gpio_flags = GPIO_LOOKUP_FLAGS_DEFAULT;
This looks wrong, we really need to clearly provide a polarity here since the ACPI GPIO resources do not provide one.
I tested gpio_flags=GPIO_LOOKUP_FLAGS_DEFAULT/HIGH/LOW, the lt6911uxe driver can pass the test and work normally.
I doubt you tested that correctly. It's impossible to have level triggered event to work with either polarity. It might be also a bug in the code lurking somewhere, but it would be unlikely (taking into account amount of systems relying on this).
Is it edge triggered event?
It is an edge triggered event in lt6911uxe. In order to better adapt to other uses, "hpd" is meaningful to specify a polarity here.
In lt6911uxe, GPIO "hpd" is used as irq, and set irq-flag to IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT. So no matter rising or falling, driver can work normally. " ret = request_threaded_irq(gpiod_to_irq(lt6911uxe->irq_gpio), NULL, lt6911uxe_threaded_irq_fn, IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT, NULL, lt6911uxe); "
So, the driver must not override the firmware, if there is no bugs. So, why do you even use those flags there? It seems like a bad code in the driver that doesn't look correct to me.