Re: [PATCH] nfc: nxp-nci: i2c: restore IRQ trigger fallback

Re: [PATCH] nfc: nxp-nci: i2c: restore IRQ trigger fallback

[David Heidelberg]

On 11/03/2026 12:08, Luca Stefani wrote:
> 
> On 11/03/2026 10:26, Carl Lee wrote:
>> The driver previously relied on IRQF_TRIGGER_RISING when requesting
>> the interrupt. This was removed to rely on the trigger type provided
>> by firmware.
>>
>> However, some platforms do not propagate the interrupt trigger type
>> to the IRQ descriptor, resulting in interrupts not being triggered.
>>
>> Use the trigger type provided by firmware when available and fall
>> back to the historically used rising-edge trigger otherwise.
>>
>> Signed-off-by: Carl Lee<carl.lee@amd.com>
>> ---
>>   drivers/nfc/nxp-nci/i2c.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
>> index 6a5ce8ff91f0..6339586a6a1b 100644
>> --- a/drivers/nfc/nxp-nci/i2c.c
>> +++ b/drivers/nfc/nxp-nci/i2c.c
>> @@ -268,6 +268,7 @@ static int nxp_nci_i2c_probe(struct i2c_client *client)
>>       struct device *dev = &client->dev;
>>       struct nxp_nci_i2c_phy *phy;
>>       int r;
>> +    unsigned long irqflags;
>>       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>>           nfc_err(&client->dev, "Need I2C_FUNC_I2C\n");
>> @@ -303,9 +304,17 @@ static int nxp_nci_i2c_probe(struct i2c_client *client)
>>       if (r < 0)
>>           return r;
>> +    /* Prefer the trigger type configured by firmware.
>> +     * Some platforms do not provide it, so fall back to the
>> +     * historically used rising-edge trigger.
>> +     */
>> +    irqflags = irq_get_trigger_type(client->irq);
> Doesn't build, needs #include <linux/irq.h>
>> +    if (!irqflags)
>> +        irqflags = IRQF_TRIGGER_RISING;
>> +
>>       r = request_threaded_irq(client->irq, NULL,
>>                    nxp_nci_i2c_irq_thread_fn,
>> -                 IRQF_ONESHOT,
>> +                 irqflags | IRQF_ONESHOT,
>>                    NXP_NCI_I2C_DRIVER_NAME, phy);
>>       if (r < 0)
>>           nfc_err(&client->dev, "Unable to register IRQ handler\n");
>>
>> ---
>> base-commit: 7109a2155340cc7b21f27e832ece6df03592f2e8
>> change-id: 20260311-nfc-nxp-nci-i2c-restore-irq-trigger-fallback-cda942530c60
>>
>> Best regards,
> 
> My ACPI NXP1001 table specifies ActiveHigh that properly gets translated into 
> IRQF_TRIGGER_HIGH.

Hello Luca,

I'm looking into the issue, I wonder
would make sense to provide ACPI quirk then?

David
> 
> Sadly this seems to just show the same storm as before, I played a bit with the 
> driver and noticed that it is simply broken on my device, all I get are timeouts 
> if I read the NCI device.
> 
> I could get it working (reading tags) by hacking the driver a bit, first of all 
> I disable the irq as soon as I get it, otherwise I keep getting interrupts when 
> the device is in MODE_COLD that would end up setting hard_fault to EREMOTEIO. 
> With that in place I re-implemented the enable/disable routine in 
> nxp_nci_i2c_set_mode, and that seems to be enough to make it working.

-- 
David Heidelberg

Re: [PATCH] nfc: nxp-nci: i2c: restore IRQ trigger fallback

[Luca Stefani]

On 11/05/2026 12:11, David Heidelberg wrote:
> On 11/03/2026 12:08, Luca Stefani wrote:
>>
>> On 11/03/2026 10:26, Carl Lee wrote:
>>> The driver previously relied on IRQF_TRIGGER_RISING when requesting
>>> the interrupt. This was removed to rely on the trigger type provided
>>> by firmware.
>>>
>>> However, some platforms do not propagate the interrupt trigger type
>>> to the IRQ descriptor, resulting in interrupts not being triggered.
>>>
>>> Use the trigger type provided by firmware when available and fall
>>> back to the historically used rising-edge trigger otherwise.
>>>
>>> Signed-off-by: Carl Lee<carl.lee@amd.com>
>>> ---
>>>   drivers/nfc/nxp-nci/i2c.c | 11 ++++++++++-
>>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
>>> index 6a5ce8ff91f0..6339586a6a1b 100644
>>> --- a/drivers/nfc/nxp-nci/i2c.c
>>> +++ b/drivers/nfc/nxp-nci/i2c.c
>>> @@ -268,6 +268,7 @@ static int nxp_nci_i2c_probe(struct i2c_client 
>>> *client)
>>>       struct device *dev = &client->dev;
>>>       struct nxp_nci_i2c_phy *phy;
>>>       int r;
>>> +    unsigned long irqflags;
>>>       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>>>           nfc_err(&client->dev, "Need I2C_FUNC_I2C\n");
>>> @@ -303,9 +304,17 @@ static int nxp_nci_i2c_probe(struct i2c_client 
>>> *client)
>>>       if (r < 0)
>>>           return r;
>>> +    /* Prefer the trigger type configured by firmware.
>>> +     * Some platforms do not provide it, so fall back to the
>>> +     * historically used rising-edge trigger.
>>> +     */
>>> +    irqflags = irq_get_trigger_type(client->irq);
>> Doesn't build, needs #include <linux/irq.h>
>>> +    if (!irqflags)
>>> +        irqflags = IRQF_TRIGGER_RISING;
>>> +
>>>       r = request_threaded_irq(client->irq, NULL,
>>>                    nxp_nci_i2c_irq_thread_fn,
>>> -                 IRQF_ONESHOT,
>>> +                 irqflags | IRQF_ONESHOT,
>>>                    NXP_NCI_I2C_DRIVER_NAME, phy);
>>>       if (r < 0)
>>>           nfc_err(&client->dev, "Unable to register IRQ handler\n");
>>>
>>> ---
>>> base-commit: 7109a2155340cc7b21f27e832ece6df03592f2e8
>>> change-id: 
>>> 20260311-nfc-nxp-nci-i2c-restore-irq-trigger-fallback-cda942530c60
>>>
>>> Best regards,
>>
>> My ACPI NXP1001 table specifies ActiveHigh that properly gets 
>> translated into IRQF_TRIGGER_HIGH.
>
> Hello Luca,
>
> I'm looking into the issue, I wonder
> would make sense to provide ACPI quirk then?

The old driver didn't really work for me as far as I remember, so if you 
just want to fix the storm, that's fine by me, but it's more a bandaid 
than anything else.

Ideally we'd make the driver work but I didn't get much further compared 
to the diff I sent above.

>
> David
>>
>> Sadly this seems to just show the same storm as before, I played a 
>> bit with the driver and noticed that it is simply broken on my 
>> device, all I get are timeouts if I read the NCI device.
>>
>> I could get it working (reading tags) by hacking the driver a bit, 
>> first of all I disable the irq as soon as I get it, otherwise I keep 
>> getting interrupts when the device is in MODE_COLD that would end up 
>> setting hard_fault to EREMOTEIO. With that in place I re-implemented 
>> the enable/disable routine in nxp_nci_i2c_set_mode, and that seems to 
>> be enough to make it working.
>