Hi,
On 14-Apr-25 13:04, Hans de Goede wrote:
Hi,
On 14-Apr-25 11:59, Yan, Dongcheng wrote:
Hi Andy and Hans,
I found the description of lt6911uxe's GPIO in the spec: GPIO5 is used as the interrupt signal (50ms low level) to inform SOC start reading registers from 6911UXE;
So setting the polarity as GPIO_ACTIVE_LOW is acceptable?
Yes that is acceptable, thank you for looking this up.
p.s.
Note that setting GPIO_ACTIVE_LOW will invert the values returned by gpiod_get_value(), so if the driver uses that you will need to fix this in the driver.
Hmm, thinking more about this, I just realized that this is an input pin to the CPU, not an output pin like all other pins described by the INT3472 device. I missed that as first.
In that case using GPIO_LOOKUP_FLAGS_DEFAULT as before probably makes the most sense. Please add a comment that this is an input pin to the INT3472 patch and keep GPIO_LOOKUP_FLAGS_DEFAULT for the next version.
Regards,
Hans
Regards,
Hans
We used RISING and FALLING in irq(not GPIO) to ensure that HDMI events will not be lost to the greatest extent possible.
Thanks, Dongcheng
On 4/14/2025 4:49 PM, Andy Shevchenko wrote:
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.