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

[Luca Stefani <luca.stefani.ge1@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2650 bytes --]


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.

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.

[-- Attachment #2: nxp.diff --]
[-- Type: text/x-patch, Size: 2448 bytes --]

diff --git a/drivers/nfc/nxp-nci/core.c b/drivers/nfc/nxp-nci/core.c
index 66b198663387..1bb5995fcdb4 100644
--- a/drivers/nfc/nxp-nci/core.c
+++ b/drivers/nfc/nxp-nci/core.c
@@ -190,10 +190,10 @@ void nxp_nci_remove(struct nci_dev *ndev)
 	if (info->phy_ops->set_mode)
 		info->phy_ops->set_mode(info->phy_id, NXP_NCI_MODE_COLD);
 
+	mutex_unlock(&info->info_lock);
+
 	nci_unregister_device(ndev);
 	nci_free_device(ndev);
-
-	mutex_unlock(&info->info_lock);
 }
 EXPORT_SYMBOL(nxp_nci_remove);
 
diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index 7aaab92c616c..1e344e7afeab 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -43,19 +43,24 @@ struct nxp_nci_i2c_phy {
 			 */
 };
 
-static int nxp_nci_i2c_set_mode(void *phy_id,
-				    enum nxp_nci_mode mode)
+static int nxp_nci_i2c_set_mode(void *phy_id, enum nxp_nci_mode mode)
 {
-	struct nxp_nci_i2c_phy *phy = (struct nxp_nci_i2c_phy *) phy_id;
-
-	gpiod_set_value(phy->gpiod_fw, (mode == NXP_NCI_MODE_FW) ? 1 : 0);
-	gpiod_set_value(phy->gpiod_en, (mode != NXP_NCI_MODE_COLD) ? 1 : 0);
-	usleep_range(10000, 15000);
-
-	if (mode == NXP_NCI_MODE_COLD)
-		phy->hard_fault = 0;
-
-	return 0;
+    struct nxp_nci_i2c_phy *phy = (struct nxp_nci_i2c_phy *) phy_id;
+
+    if (mode == NXP_NCI_MODE_COLD) {
+        disable_irq(phy->i2c_dev->irq);
+        gpiod_set_value(phy->gpiod_fw, 0);
+        gpiod_set_value(phy->gpiod_en, 0);
+        phy->hard_fault = 0;
+        usleep_range(10000, 15000);
+    } else {
+        gpiod_set_value(phy->gpiod_fw, (mode == NXP_NCI_MODE_FW) ? 1 : 0);
+        gpiod_set_value(phy->gpiod_en, 1);
+        msleep(150);
+        phy->hard_fault = 0;
+        enable_irq(phy->i2c_dev->irq);
+    }
+    return 0;
 }
 
 static int nxp_nci_i2c_write(void *phy_id, struct sk_buff *skb)
@@ -317,8 +322,12 @@ static int nxp_nci_i2c_probe(struct i2c_client *client)
 				 nxp_nci_i2c_irq_thread_fn,
 				 irqflags | IRQF_ONESHOT,
 				 NXP_NCI_I2C_DRIVER_NAME, phy);
-	if (r < 0)
+	if (r < 0) {
 		nfc_err(&client->dev, "Unable to register IRQ handler\n");
+		return r;
+	}
+
+	disable_irq(client->irq);
 
 	return r;
 }
@@ -327,8 +336,8 @@ static void nxp_nci_i2c_remove(struct i2c_client *client)
 {
 	struct nxp_nci_i2c_phy *phy = i2c_get_clientdata(client);
 
-	nxp_nci_remove(phy->ndev);
 	free_irq(client->irq, phy);
+	nxp_nci_remove(phy->ndev);
 }
 
 static const struct i2c_device_id nxp_nci_i2c_id_table[] = {