[PATCH v2 0/3] nfc: st-nci: Fairphone 5 NFC bring-up (ST21NFCD)

[PATCH v2 0/3] nfc: st-nci: Fairphone 5 NFC bring-up (ST21NFCD)

[Kristian Brox]

This adds NFC on the Fairphone 5 (qcm6490). The board uses an ST21NFCD
on I2C. That part speaks raw NCI; the current st-nci driver always
wraps NDLC, so using st,st21nfcb-i2c leaves the adapter unusable.

The series adds a st,st21nfcd compatible for the raw-NCI path and
the Fairphone 5 DT node. Boards that already use st21nfcb / st21nfcc
keep the NDLC path.

Tested on a Fairphone 5 running postmarketOS, with these changes as
modules on a 7.1.2 sc7280 kernel:

- nfctool: Powered: Yes
- initiator poll / neard: NTAG 215, NDEF URI read OK

ese-present and uicc-present follow the public schematic (NFC_SWP1/SWP2:
SWP_SE to SIM1, SWP_UICC to SIM2). SE/HCE is not tested.

CLK_REQ (GPIO 39) is omitted, as on Fairphone 6 NFC. VBAT and VDD_TX sit
on VPH_PWR and are not modelled. VCC_UICC_IN (L4C) is not modelled;
UICC SWP is untested.

Changes in v2:
- Compatible is st,st21nfcd (no -i2c suffix)
- Sent without PGP/MIME
- DTS: interrupts-extended and pinctrl for IRQ/reset
- DTS: ese-present / uicc-present (schematic)
- DTS: SYS_CLK from LN_BB_CLK2, VPS_IO from L18B
- Binding: optional clocks and vdd-io-supply
- Driver: optional clk / vdd-io enable

Kristian Brox (3):
  dt-bindings: net: nfc: add st,st21nfcd
  nfc: st-nci: add raw NCI path for ST21NFCD
  arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC

Signed-off-by: Kristian Brox <isyourbrainfoss@proton.me>
---

[PATCH v2 1/3] dt-bindings: net: nfc: add st,st21nfcd

[Kristian Brox]

The ST21NFCD (e.g. Fairphone 5) speaks raw NCI on I2C. Existing
st,st21nfcb-* and st,st21nfcc-i2c compatibles stay NDLC. Add a
separate compatible so those boards are not switched to the wrong
framing.

Also document the optional SYS_CLK (clocks) and VPS_IO (vdd-io-supply)
used on Fairphone 5, and add an I2C example.

Signed-off-by: Kristian Brox <isyourbrainfoss@proton.me>
---
diff --git a/Documentation/devicetree/bindings/net/nfc/st,st-nci.yaml b/Documentation/devicetree/bindings/net/nfc/st,st-nci.yaml
index 1dcbddb..4bdbb36 100644
--- a/Documentation/devicetree/bindings/net/nfc/st,st-nci.yaml
+++ b/Documentation/devicetree/bindings/net/nfc/st,st-nci.yaml
@@ -11,10 +11,14 @@ maintainers:
 
 properties:
   compatible:
+    description: |
+      st,st21nfcb-* and st,st21nfcc-i2c use NDLC on the wire.
+      st,st21nfcd is ST21NFCD with raw NCI (no NDLC PCB).
     enum:
       - st,st21nfcb-i2c
       - st,st21nfcb-spi
       - st,st21nfcc-i2c
+      - st,st21nfcd
 
   reset-gpios:
     description: Output GPIO pin used for resetting the controller
@@ -36,6 +40,15 @@ properties:
       Specifies that the uicc swp signal can be physically connected to the
       controller
 
+  clocks:
+    maxItems: 1
+    description:
+      External reference clock connected to SYS_CLK.
+
+  vdd-io-supply:
+    description:
+      Digital I/O supply (VPS_IO).
+
 required:
   - compatible
   - interrupts
@@ -49,6 +62,7 @@ if:
         enum:
           - st,st21nfcb-i2c
           - st,st21nfcc-i2c
+          - st,st21nfcd
 then:
   properties:
     spi-max-frequency: false
@@ -81,6 +95,27 @@ examples:
         };
     };
 
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        nfc@8 {
+            compatible = "st,st21nfcd";
+            reg = <0x08>;
+
+            interrupt-parent = <&gpio5>;
+            interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
+            reset-gpios = <&gpio5 29 GPIO_ACTIVE_HIGH>;
+
+            clocks = <&clk>;
+            vdd-io-supply = <&vdd_io>;
+        };
+    };
+
   - |
     #include <dt-bindings/gpio/gpio.h>
     #include <dt-bindings/interrupt-controller/irq.h>

Re: [PATCH v2 1/3] dt-bindings: net: nfc: add st,st21nfcd

[David Heidelberg]

On 19/08/2026 23:09, Kristian Brox wrote:
> The ST21NFCD (e.g. Fairphone 5) speaks raw NCI on I2C. Existing
> st,st21nfcb-* and st,st21nfcc-i2c compatibles stay NDLC. Add a
> separate compatible so those boards are not switched to the wrong
> framing.

No, this is not reason why do you introduce new compatible. You do it because 
it's different HW. The fact it shares the driver is irrelevant in the dt-binding

> 
> Also document the optional SYS_CLK (clocks) and VPS_IO (vdd-io-supply)
> used on Fairphone 5, and add an I2C example.
> 
> Signed-off-by: Kristian Brox <isyourbrainfoss@proton.me>
> ---
> diff --git a/Documentation/devicetree/bindings/net/nfc/st,st-nci.yaml b/Documentation/devicetree/bindings/net/nfc/st,st-nci.yaml
> index 1dcbddb..4bdbb36 100644
> --- a/Documentation/devicetree/bindings/net/nfc/st,st-nci.yaml
> +++ b/Documentation/devicetree/bindings/net/nfc/st,st-nci.yaml
> @@ -11,10 +11,14 @@ maintainers:
>   
>   properties:
>     compatible:
> +    description: |
> +      st,st21nfcb-* and st,st21nfcc-i2c use NDLC on the wire.
> +      st,st21nfcd is ST21NFCD with raw NCI (no NDLC PCB).

I think you can omit this description completely here, but if someone had better 
idea where to put it, I'm open to it.


>       enum:
>         - st,st21nfcb-i2c
>         - st,st21nfcb-spi
>         - st,st21nfcc-i2c
> +      - st,st21nfcd
>   
>     reset-gpios:
>       description: Output GPIO pin used for resetting the controller
> @@ -36,6 +40,15 @@ properties:
>         Specifies that the uicc swp signal can be physically connected to the
>         controller
>   
> +  clocks:
> +    maxItems: 1
> +    description:
> +      External reference clock connected to SYS_CLK.
> +
> +  vdd-io-supply:
> +    description:
> +      Digital I/O supply (VPS_IO).
> +
>   required:
>     - compatible
>     - interrupts
> @@ -49,6 +62,7 @@ if:
>           enum:
>             - st,st21nfcb-i2c
>             - st,st21nfcc-i2c
> +          - st,st21nfcd
>   then:
>     properties:
>       spi-max-frequency: false
> @@ -81,6 +95,27 @@ examples:
>           };
>       };
>   
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        nfc@8 {
> +            compatible = "st,st21nfcd";
> +            reg = <0x08>;
> +
> +            interrupt-parent = <&gpio5>;
> +            interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;

use interrupts extended (interrupt-parent + interrupts in one line).

As a last point (for the series), never send a new series with Reply-to (as a 
followup to existing one). It looks messy, we have tools to track series :)
see: https://patchwork.kernel.org/project/oe-linux-nfc/list/

David

> +            reset-gpios = <&gpio5 29 GPIO_ACTIVE_HIGH>;
> +
> +            clocks = <&clk>;
> +            vdd-io-supply = <&vdd_io>;
> +        };
> +    };
> +
>     - |
>       #include <dt-bindings/gpio/gpio.h>
>       #include <dt-bindings/interrupt-controller/irq.h>
> 

Re: [PATCH v2 1/3] dt-bindings: net: nfc: add st,st21nfcd

[Krzysztof Kozlowski]

On 19/08/2026 23:09, Kristian Brox wrote:
> The ST21NFCD (e.g. Fairphone 5) speaks raw NCI on I2C. Existing
> st,st21nfcb-* and st,st21nfcc-i2c compatibles stay NDLC. Add a
> separate compatible so those boards are not switched to the wrong
> framing.
> 
> Also document the optional SYS_CLK (clocks) and VPS_IO (vdd-io-supply)
> used on Fairphone 5, and add an I2C example.
> 
> Signed-off-by: Kristian Brox <isyourbrainfoss@proton.me>

You already received below comment, so me needing to repeat is not the
right way because it means you just ignore me. So I assume you will
ignore rest of my comments as well.

Additionally, you need to develop on mainline, not postmarketos kernel.

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time.

Please kindly resend and include all necessary To/Cc entries.

Best regards,
Krzysztof

[PATCH v2 2/3] nfc: st-nci: add raw NCI path for ST21NFCD

[Kristian Brox]

ST21NFCD does not use NDLC. When the compatible is st,st21nfcd,
talk raw NCI:

- do not add or strip an NDLC PCB
- do not run the T1/T2 ACK timers
- I2C reads are a 3-byte NCI header plus payload
- skip proprietary SET_NFC_MODE and HCI SE discovery

Optionally enable clocks (SYS_CLK) and vdd-io (VPS_IO) when the
DT describes them. Existing st21nfcb / st21nfcc boards keep the
NDLC path and do not need those properties.

Tested on Fairphone 5: adapter powers up and reads an NTAG 215.

Signed-off-by: Kristian Brox <isyourbrainfoss@proton.me>
---
diff --git a/drivers/nfc/st-nci/core.c b/drivers/nfc/st-nci/core.c
index a367136..2356f16 100644
--- a/drivers/nfc/st-nci/core.c
+++ b/drivers/nfc/st-nci/core.c
@@ -18,8 +18,13 @@
 
 static int st_nci_init(struct nci_dev *ndev)
 {
+	struct st_nci_info *info = nci_get_drvdata(ndev);
 	struct nci_mode_set_cmd cmd;
 
+	/* ST21NFCD has no NDLC proprietary SET_NFC_MODE */
+	if (info->ndlc->raw_nci)
+		return 0;
+
 	cmd.cmd_type = ST_NCI_SET_NFC_MODE;
 	cmd.mode = 1;
 
diff --git a/drivers/nfc/st-nci/i2c.c b/drivers/nfc/st-nci/i2c.c
index 416770a..cfddb7f 100644
--- a/drivers/nfc/st-nci/i2c.c
+++ b/drivers/nfc/st-nci/i2c.c
@@ -10,10 +10,13 @@
 #include <linux/i2c.h>
 #include <linux/gpio/consumer.h>
 #include <linux/acpi.h>
+#include <linux/clk.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>
 #include <linux/nfc.h>
 #include <linux/of.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
 
 #include "st-nci.h"
 
@@ -22,10 +25,17 @@
 /* ndlc header */
 #define ST_NCI_FRAME_HEADROOM 1
 #define ST_NCI_FRAME_TAILROOM 0
+#define ST_NCI_RAW_FRAME_HEADROOM 0
 
 #define ST_NCI_I2C_MIN_SIZE 4   /* PCB(1) + NCI Packet header(3) */
+#define ST_NCI_NCI_HDR_SIZE 3   /* raw NCI: MT/PBF/GID + OID + len */
 #define ST_NCI_I2C_MAX_SIZE 250 /* req 4.2.1 */
 
+enum st_nci_i2c_proto {
+	ST_NCI_I2C_PROTO_NDLC = 0,
+	ST_NCI_I2C_PROTO_RAW_NCI,
+};
+
 #define ST_NCI_DRIVER_NAME "st_nci"
 #define ST_NCI_I2C_DRIVER_NAME "st_nci_i2c"
 
@@ -34,6 +44,7 @@ struct st_nci_i2c_phy {
 	struct llt_ndlc *ndlc;
 
 	bool irq_active;
+	bool raw_nci;
 
 	struct gpio_desc *gpiod_reset;
 
@@ -111,6 +122,42 @@ static int st_nci_i2c_read(struct st_nci_i2c_phy *phy,
 	u8 buf[ST_NCI_I2C_MAX_SIZE];
 	struct i2c_client *client = phy->i2c_dev;
 
+	if (phy->raw_nci) {
+		r = i2c_master_recv(client, buf, ST_NCI_NCI_HDR_SIZE);
+		if (r < 0) {
+			usleep_range(1000, 4000);
+			r = i2c_master_recv(client, buf, ST_NCI_NCI_HDR_SIZE);
+		}
+		if (r != ST_NCI_NCI_HDR_SIZE)
+			return -EREMOTEIO;
+
+		len = buf[2];
+		if (len > ST_NCI_I2C_MAX_SIZE) {
+			nfc_err(&client->dev, "invalid frame len\n");
+			return -EBADMSG;
+		}
+
+		*skb = alloc_skb(ST_NCI_NCI_HDR_SIZE + len, GFP_KERNEL);
+		if (!*skb)
+			return -ENOMEM;
+
+		skb_put(*skb, ST_NCI_NCI_HDR_SIZE);
+		memcpy((*skb)->data, buf, ST_NCI_NCI_HDR_SIZE);
+
+		if (!len)
+			return 0;
+
+		r = i2c_master_recv(client, buf, len);
+		if (r != len) {
+			kfree_skb(*skb);
+			return -EREMOTEIO;
+		}
+
+		skb_put(*skb, len);
+		memcpy((*skb)->data + ST_NCI_NCI_HDR_SIZE, buf, len);
+		return 0;
+	}
+
 	r = i2c_master_recv(client, buf, ST_NCI_I2C_MIN_SIZE);
 	if (r < 0) {  /* Retry, chip was in standby */
 		usleep_range(1000, 4000);
@@ -211,6 +258,8 @@ static int st_nci_i2c_probe(struct i2c_client *client)
 		return -ENOMEM;
 
 	phy->i2c_dev = client;
+	phy->raw_nci = (uintptr_t)device_get_match_data(dev) ==
+			ST_NCI_I2C_PROTO_RAW_NCI;
 
 	i2c_set_clientdata(client, phy);
 
@@ -225,19 +274,31 @@ static int st_nci_i2c_probe(struct i2c_client *client)
 		return -ENODEV;
 	}
 
+	r = devm_regulator_get_enable_optional(dev, "vdd-io");
+	if (r && r != -ENODEV)
+		return dev_err_probe(dev, r, "failed to enable vdd-io\n");
+
+	r = PTR_ERR_OR_ZERO(devm_clk_get_optional_enabled(dev, NULL));
+	if (r)
+		return dev_err_probe(dev, r, "failed to enable clock\n");
+
 	phy->se_status.is_ese_present =
 				device_property_read_bool(dev, "ese-present");
 	phy->se_status.is_uicc_present =
 				device_property_read_bool(dev, "uicc-present");
 
 	r = ndlc_probe(phy, &i2c_phy_ops, &client->dev,
-			ST_NCI_FRAME_HEADROOM, ST_NCI_FRAME_TAILROOM,
+			phy->raw_nci ? ST_NCI_RAW_FRAME_HEADROOM :
+				       ST_NCI_FRAME_HEADROOM,
+			ST_NCI_FRAME_TAILROOM,
 			&phy->ndlc, &phy->se_status);
 	if (r < 0) {
 		nfc_err(&client->dev, "Unable to register ndlc layer\n");
 		return r;
 	}
 
+	phy->ndlc->raw_nci = phy->raw_nci;
+
 	phy->irq_active = true;
 	r = devm_request_threaded_irq(&client->dev, client->irq, NULL,
 				st_nci_irq_thread_fn,
@@ -273,6 +334,8 @@ static const struct of_device_id of_st_nci_i2c_match[] __maybe_unused = {
 	{ .compatible = "st,st21nfcb-i2c", },
 	{ .compatible = "st,st21nfcb_i2c", },
 	{ .compatible = "st,st21nfcc-i2c", },
+	{ .compatible = "st,st21nfcd",
+	  .data = (void *)ST_NCI_I2C_PROTO_RAW_NCI },
 	{}
 };
 MODULE_DEVICE_TABLE(of, of_st_nci_i2c_match);
diff --git a/drivers/nfc/st-nci/ndlc.c b/drivers/nfc/st-nci/ndlc.c
index be48088..b319246 100644
--- a/drivers/nfc/st-nci/ndlc.c
+++ b/drivers/nfc/st-nci/ndlc.c
@@ -62,8 +62,9 @@ void ndlc_close(struct llt_ndlc *ndlc)
 	/* toggle reset pin */
 	ndlc->ops->enable(ndlc->phy_id);
 
-	nci_prop_cmd(ndlc->ndev, ST_NCI_CORE_PROP,
-		     sizeof(struct nci_mode_set_cmd), (__u8 *)&cmd);
+	if (!ndlc->raw_nci)
+		nci_prop_cmd(ndlc->ndev, ST_NCI_CORE_PROP,
+			     sizeof(struct nci_mode_set_cmd), (__u8 *)&cmd);
 
 	ndlc->powered = 0;
 	ndlc->ops->disable(ndlc->phy_id);
@@ -72,11 +73,13 @@ EXPORT_SYMBOL(ndlc_close);
 
 int ndlc_send(struct llt_ndlc *ndlc, struct sk_buff *skb)
 {
-	/* add ndlc header */
-	u8 pcb = PCB_TYPE_DATAFRAME | PCB_DATAFRAME_RETRANSMIT_NO |
-		PCB_FRAME_CRC_INFO_NOTPRESENT;
+	if (!ndlc->raw_nci) {
+		/* add ndlc header */
+		u8 pcb = PCB_TYPE_DATAFRAME | PCB_DATAFRAME_RETRANSMIT_NO |
+			PCB_FRAME_CRC_INFO_NOTPRESENT;
 
-	*(u8 *)skb_push(skb, 1) = pcb;
+		*(u8 *)skb_push(skb, 1) = pcb;
+	}
 	skb_queue_tail(&ndlc->send_q, skb);
 
 	schedule_work(&ndlc->sm_work);
@@ -103,6 +106,10 @@ static void llt_ndlc_send_queue(struct llt_ndlc *ndlc)
 			ndlc->hard_fault = r;
 			break;
 		}
+		if (ndlc->raw_nci) {
+			kfree_skb(skb);
+			continue;
+		}
 		time_sent = jiffies;
 		*(unsigned long *)skb->cb = time_sent;
 
@@ -154,6 +161,10 @@ static void llt_ndlc_rcv_queue(struct llt_ndlc *ndlc)
 		pr_debug("rcvQlen=%d\n", ndlc->rcv_q.qlen);
 
 	while ((skb = skb_dequeue(&ndlc->rcv_q)) != NULL) {
+		if (ndlc->raw_nci) {
+			nci_recv_frame(ndlc->ndev, skb);
+			continue;
+		}
 		pcb = skb->data[0];
 		skb_pull(skb, 1);
 		if ((pcb & PCB_TYPE_MASK) == PCB_TYPE_SUPERVISOR) {
diff --git a/drivers/nfc/st-nci/ndlc.h b/drivers/nfc/st-nci/ndlc.h
index c24ce9b..5c1f8ba 100644
--- a/drivers/nfc/st-nci/ndlc.h
+++ b/drivers/nfc/st-nci/ndlc.h
@@ -39,6 +39,8 @@ struct llt_ndlc {
 	 */
 	int hard_fault;
 	int powered;
+	/* ST21NFCD: raw NCI on the wire, no NDLC PCB / ACK timers */
+	bool raw_nci;
 };
 
 int ndlc_open(struct llt_ndlc *ndlc);
diff --git a/drivers/nfc/st-nci/se.c b/drivers/nfc/st-nci/se.c
index 607ec76..44cc102 100644
--- a/drivers/nfc/st-nci/se.c
+++ b/drivers/nfc/st-nci/se.c
@@ -621,6 +621,9 @@ int st_nci_discover_se(struct nci_dev *ndev)
 	int se_count = 0;
 	struct st_nci_info *info = nci_get_drvdata(ndev);
 
+	if (info->ndlc->raw_nci)
+		return 0;
+
 	r = st_nci_hci_network_init(ndev);
 	if (r != 0)
 		return r;

Re: [PATCH v2 2/3] nfc: st-nci: add raw NCI path for ST21NFCD

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:

New issues:
- [High] Late initialization of `raw_nci` causes an `skb_under_panic` due to a race with userspace.
- [High] Improper power sequencing by driving the reset GPIO high before enabling VDD-IO.

Pre-existing issues:
- [High] Failing probe IRQ registration leaks the NCI device and causes a Use-After-Free.
- [High] Unsafe teardown order in `ndlc_remove` allows a dangling worker to be rescheduled, leading to a Use-After-Free.
- [High] Buffer overflow and `skb_over_panic` due to tailroom consumption mismatch in the legacy read path.
- [High] Unaligned memory access when parsing the legacy NDLC frame length.
- [High] Permanent SKB memory leak when a hardware write fails in the worker thread.
--

--- Patch [2]: [PATCH v2 2/3] nfc: st-nci: add raw NCI path for ST21NFCD ---
commit 9b78e1755a6510b5c027dacfd61c724b1db74622
Author: Kristian Brox <isyourbrainfoss@proton.me>

nfc: st-nci: add raw NCI path for ST21NFCD

ST21NFCD does not use NDLC. When the compatible is st,st21nfcd,
talk raw NCI.

> diff --git a/drivers/nfc/st-nci/i2c.c b/drivers/nfc/st-nci/i2c.c
> index 9ae839a6f5cc0..566689d5aadb4 100644
> --- a/drivers/nfc/st-nci/i2c.c
> +++ b/drivers/nfc/st-nci/i2c.c
[ ... ]
> @@ -111,6 +122,42 @@ static int st_nci_i2c_read(struct st_nci_i2c_phy *phy,
>  	u8 buf[ST_NCI_I2C_MAX_SIZE];
>  	struct i2c_client *client = phy->i2c_dev;
>  
> +	if (phy->raw_nci) {
> +		r = i2c_master_recv(client, buf, ST_NCI_NCI_HDR_SIZE);
> +		if (r < 0) {
> +			usleep_range(1000, 4000);
> +			r = i2c_master_recv(client, buf, ST_NCI_NCI_HDR_SIZE);
> +		}
> +		if (r != ST_NCI_NCI_HDR_SIZE)
> +			return -EREMOTEIO;
> +
> +		len = buf[2];
> +		if (len > ST_NCI_I2C_MAX_SIZE) {
> +			nfc_err(&client->dev, "invalid frame len\n");
> +			return -EBADMSG;
> +		}
> +
> +		*skb = alloc_skb(ST_NCI_NCI_HDR_SIZE + len, GFP_KERNEL);
> +		if (!*skb)
> +			return -ENOMEM;
> +
> +		skb_put(*skb, ST_NCI_NCI_HDR_SIZE);
> +		memcpy((*skb)->data, buf, ST_NCI_NCI_HDR_SIZE);
> +
> +		if (!len)
> +			return 0;
> +
> +		r = i2c_master_recv(client, buf, len);
> +		if (r != len) {
> +			kfree_skb(*skb);
> +			return -EREMOTEIO;
> +		}
> +
> +		skb_put(*skb, len);
> +		memcpy((*skb)->data + ST_NCI_NCI_HDR_SIZE, buf, len);
> +		return 0;
> +	}
> +
>  	r = i2c_master_recv(client, buf, ST_NCI_I2C_MIN_SIZE);
>  	if (r < 0) {  /* Retry, chip was in standby */
>  		usleep_range(1000, 4000);

[Severity: High]
This is a pre-existing issue, but does the legacy read path consume more
headroom and tailroom than it allocates, causing an skb_over_panic?

st_nci_i2c_read() allocates an SKB of size 4 + len:

    *skb = alloc_skb(ST_NCI_I2C_MIN_SIZE + len, GFP_KERNEL);

Then it reserves 4 bytes, leaving len bytes of tailroom:

    skb_reserve(*skb, ST_NCI_I2C_MIN_SIZE);

But subsequent skb_put() calls consume 4 + len bytes of tailroom:

    skb_put(*skb, ST_NCI_I2C_MIN_SIZE);
    ...
    skb_put(*skb, len);

Because only len bytes were left in the tailroom, the operations require 4
more bytes than what was requested. If the allocation is just beneath a slab
boundary without extra padding, won't the final skb_put(len) trigger an
skb_over_panic?

[Severity: High]
This is also a pre-existing issue, but does parsing the legacy NDLC frame
length cause an unaligned memory access?

The frame header is read into a stack-allocated byte array:

    u8 buf[ST_NCI_I2C_MAX_SIZE];

Later in st_nci_i2c_read(), the code casts an unaligned offset directly to a
16-bit integer pointer:

    len = be16_to_cpu(*(__be16 *) (buf + 2));

On strict-alignment architectures, won't this unaligned dereference trigger
a kernel panic or alignment fault?

[ ... ]
> @@ -225,19 +274,31 @@ static int st_nci_i2c_probe(struct i2c_client *client)
>  		return -ENODEV;
>  	}
>  
> +	r = devm_regulator_get_enable_optional(dev, "vdd-io");
> +	if (r && r != -ENODEV)
> +		return dev_err_probe(dev, r, "failed to enable vdd-io\n");
> +
> +	r = PTR_ERR_OR_ZERO(devm_clk_get_optional_enabled(dev, NULL));
> +	if (r)
> +		return dev_err_probe(dev, r, "failed to enable clock\n");
> +

[Severity: High]
Can driving the reset GPIO high before enabling VDD-IO risk hardware damage?

In st_nci_i2c_probe(), the reset GPIO is requested and actively driven high
via GPIOD_OUT_HIGH:

    phy->gpiod_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);

The vdd-io regulator and system clocks are then requested and enabled after
the GPIO is driven here.

Driving an active HIGH signal into a CMOS IC's I/O pin before its VDD rail
is powered violates power sequencing requirements. This forces voltage
through the chip's internal ESD protection diodes, back-powering the chip.
Should the regulator and clocks be enabled before driving the GPIO high?

>  	phy->se_status.is_ese_present =
>  				device_property_read_bool(dev, "ese-present");
>  	phy->se_status.is_uicc_present =
>  				device_property_read_bool(dev, "uicc-present");
>  
>  	r = ndlc_probe(phy, &i2c_phy_ops, &client->dev,
> -			ST_NCI_FRAME_HEADROOM, ST_NCI_FRAME_TAILROOM,
> +			phy->raw_nci ? ST_NCI_RAW_FRAME_HEADROOM :
> +				       ST_NCI_FRAME_HEADROOM,
>  			ST_NCI_FRAME_TAILROOM,
>  			&phy->ndlc, &phy->se_status);
>  	if (r < 0) {
>  		nfc_err(&client->dev, "Unable to register ndlc layer\n");
>  		return r;
>  	}
>  
> +	phy->ndlc->raw_nci = phy->raw_nci;

[Severity: High]
Can assigning this flag after ndlc_probe() cause an skb_under_panic crash?

Because ndlc_probe() registers the NCI device, the interface is exposed to
userspace before phy->ndlc->raw_nci is initialized. If userspace brings up
the device immediately, st_nci_init() and subsequent commands can execute
while raw_nci is still false.

If ndlc_send() sees raw_nci == false, it will attempt to push the NDLC
header:

drivers/nfc/st-nci/ndlc.c:ndlc_send() {
    ...
    if (!ndlc->raw_nci) {
        ...
        *(u8 *)skb_push(skb, 1) = pcb;
    }
    ...
}

Because the SKB was allocated for a raw NCI device with tx_headroom = 0,
this skb_push() call would panic. Should raw_nci be initialized before
ndlc_probe() exposes the device to userspace?

>  
>  	phy->irq_active = true;
>  	r = devm_request_threaded_irq(&client->dev, client->irq, NULL,
>  				st_nci_irq_thread_fn,
>  				IRQF_ONESHOT,
>  				ST_NCI_DRIVER_NAME, phy);
>  	if (r < 0)
>  		nfc_err(&client->dev, "Unable to register IRQ handler\n");
>  
>  	return r;
>  }

[Severity: High]
This is a pre-existing issue, but does failing to register the IRQ here leak
the NCI device and cause a use-after-free?

ndlc_probe() allocates and registers the NCI device (ndev), but ndev is not
devres-managed and requires st_nci_remove() to unregister and free it.

If devm_request_threaded_irq() fails, the probe function immediately returns
an error code. This causes devres to free phy and other devm-managed memory,
but leaves the network interface registered. Could subsequent userspace
interaction with the lingering interface dereference the freed pointers?

> diff --git a/drivers/nfc/st-nci/ndlc.c b/drivers/nfc/st-nci/ndlc.c
> index be4808859cfa9..b3192460c96e3 100644
> --- a/drivers/nfc/st-nci/ndlc.c
> +++ b/drivers/nfc/st-nci/ndlc.c
[ ... ]
> @@ -103,6 +106,10 @@ static void llt_ndlc_send_queue(struct llt_ndlc *ndlc)
>  			ndlc->hard_fault = r;
>  			break;
>  		}
> +		if (ndlc->raw_nci) {
> +			kfree_skb(skb);
> +			continue;
> +		}

[Severity: High]
This isn't a bug introduced by this patch, but does failing a hardware write
leak the dequeued SKB permanently?

In llt_ndlc_send_queue(), an SKB is dequeued and transmitted:

    skb = skb_dequeue(&ndlc->send_q);
    r = ndlc->ops->write(ndlc->phy_id, skb);
    if (r < 0) {
        ndlc->hard_fault = r;
        break;
    }

If the write fails, the loop breaks immediately. The dequeued SKB is neither
freed nor re-queued, stranding the memory permanently. Furthermore, because
hard_fault is set, won't all subsequently scheduled SKBs also dequeue, fail,
and leak, allowing memory exhaustion?

[ ... ]
> @@ -154,6 +161,10 @@ static void llt_ndlc_rcv_queue(struct llt_ndlc *ndlc)
>  		pr_debug("rcvQlen=%d\n", ndlc->rcv_q.qlen);
>  
>  	while ((skb = skb_dequeue(&ndlc->rcv_q)) != NULL) {
> +		if (ndlc->raw_nci) {
> +			nci_recv_frame(ndlc->ndev, skb);
> +			continue;
> +		}
>  		pcb = skb->data[0];
>  		skb_pull(skb, 1);

[Severity: High]
This is also a pre-existing issue, but does ndlc_remove() have an unsafe
teardown order that can cause a use-after-free?

In ndlc_remove(), cancel_work_sync(&ndlc->sm_work) is called to cancel
pending work before the hardware IRQ is disabled in st_nci_remove():

drivers/nfc/st-nci/ndlc.c:ndlc_remove() {
    ...
    cancel_work_sync(&ndlc->sm_work);
    ...
    st_nci_remove(ndlc->ndev);
    ...
}

If an interrupt fires in the window between cancel_work_sync() and the IRQ
being disabled, st_nci_irq_thread_fn() will read a packet and call
ndlc_recv(), which executes schedule_work(&ndlc->sm_work).

When driver removal finishes, devres frees the ndlc structure, leaving the
newly scheduled worker to execute on freed memory. Is it possible to disable
the IRQ before cancelling the work?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819210906.2500789-1-isyourbrainfoss@proton.me?part=2

[PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC

[Kristian Brox]

Enable the ST21NFCD on i2c9 (0x08), IRQ TLMM 41, reset TLMM 38
active-high. Compatible is st,st21nfcd (raw NCI).

SYS_CLK is LN_BB_CLK2. VPS_IO is L18B (vreg_l18b). ese-present and
uicc-present follow the public schematic (NFC_SWP1/SWP2: SWP_SE to
SIM1, SWP_UICC to SIM2). Reader path is tested; SE/HCE is not.

Signed-off-by: Kristian Brox <isyourbrainfoss@proton.me>
---
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
index 29d6265..394237c 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
@@ -1004,7 +1004,23 @@
 &i2c9 {
 	status = "okay";
 
-	/* ST21NFC NFC @ 28 */
+	nfc@8 {
+		compatible = "st,st21nfcd";
+		reg = <0x08>;
+
+		interrupts-extended = <&tlmm 41 IRQ_TYPE_LEVEL_HIGH>;
+		reset-gpios = <&tlmm 38 GPIO_ACTIVE_HIGH>;
+
+		pinctrl-0 = <&nfc_int_default>, <&nfc_reset_default>;
+		pinctrl-names = "default";
+
+		clocks = <&rpmhcc RPMH_LN_BB_CLK2>;
+		vdd-io-supply = <&vreg_l18b>;
+
+		ese-present;
+		uicc-present;
+	};
+
 	/* VL53L3 ToF @ 29 */
 };
 
@@ -1649,6 +1665,21 @@
 		drive-strength = <2>;
 		bias-pull-up;
 	};
+
+	nfc_int_default: nfc-int-default-state {
+		pins = "gpio41";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	nfc_reset_default: nfc-reset-default-state {
+		pins = "gpio38";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+		output-high;
+	};
 };
 
 &uart5 {

Re: [PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC

[David Heidelberg]

On 19/08/2026 23:09, Kristian Brox wrote:
> Enable the ST21NFCD on i2c9 (0x08), IRQ TLMM 41, reset TLMM 38
> active-high. Compatible is st,st21nfcd (raw NCI).
> 
> SYS_CLK is LN_BB_CLK2. VPS_IO is L18B (vreg_l18b). ese-present and
> uicc-present follow the public schematic (NFC_SWP1/SWP2: SWP_SE to
> SIM1, SWP_UICC to SIM2). Reader path is tested; SE/HCE is not.
> 
> Signed-off-by: Kristian Brox <isyourbrainfoss@proton.me>
> ---
> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> index 29d6265..394237c 100644
> --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> @@ -1004,7 +1004,23 @@
>   &i2c9 {
>   	status = "okay";
>   
> -	/* ST21NFC NFC @ 28 */
> +	nfc@8 {
> +		compatible = "st,st21nfcd";
> +		reg = <0x08>;
> +
> +		interrupts-extended = <&tlmm 41 IRQ_TYPE_LEVEL_HIGH>;
> +		reset-gpios = <&tlmm 38 GPIO_ACTIVE_HIGH>;
> +
> +		pinctrl-0 = <&nfc_int_default>, <&nfc_reset_default>;
> +		pinctrl-names = "default";
> +
> +		clocks = <&rpmhcc RPMH_LN_BB_CLK2>;
> +		vdd-io-supply = <&vreg_l18b>;
> +
> +		ese-present;
> +		uicc-present;
> +	};
> +
>   	/* VL53L3 ToF @ 29 */
>   };
>   
> @@ -1649,6 +1665,21 @@
>   		drive-strength = <2>;
>   		bias-pull-up;
>   	};
> +
> +	nfc_int_default: nfc-int-default-state {
> +		pins = "gpio41";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +	};
> +
> +	nfc_reset_default: nfc-reset-default-state {
> +		pins = "gpio38";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +		output-high;
> +	};

Sort by pins property here. As a bonus, you can group these into something like:

nfc_default: nfc-default-state {
     reset-pins {}
     int-pins {}
}

this will make referencing the group here cleaner.

I think the output-high shouldn't be needed, as driver should take about setting 
the low/high as needed.

David

>   };
>   
>   &uart5 {
> 

Re: [PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The default pinctrl state applies `output-high` to the reset pin unconditionally at probe time, before the NFC chip's power supply (`vdd-io-supply`) is enabled, risking hardware latch-up.
- [Low] The `reset-gpios` polarity is specified as `GPIO_ACTIVE_HIGH` to work around inverted logic in the `st-nci` driver, resulting in an inaccurate hardware description.

Pre-existing issues:
- [High] The NCI device is registered and exposed to userspace before its state (the `raw_nci` flag and the IRQ handler) is fully initialized, creating race windows.
--

--- Patch [3]: [PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC ---
commit 5a4863ae6956a4a982d319ea2fa7fb68012f0fdf
Author: Kristian Brox <isyourbrainfoss@proton.me>

arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC

Enable the ST21NFCD on i2c9 (0x08), IRQ TLMM 41, reset TLMM 38
active-high. Compatible is st,st21nfcd (raw NCI).

SYS_CLK is LN_BB_CLK2. VPS_IO is L18B (vreg_l18b). ese-present and
uicc-present follow the public schematic (NFC_SWP1/SWP2: SWP_SE to
SIM1, SWP_UICC to SIM2). Reader path is tested; SE/HCE is not.

> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> index 04cb9230d29fd..c5d51826b6033 100644
> --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> @@ -908,7 +908,23 @@ redriver_ss_in: endpoint {
>  &i2c9 {
>  	status = "okay";
>  
> -	/* ST21NFC NFC @ 28 */
> +	nfc@8 {
> +		compatible = "st,st21nfcd";
> +		reg = <0x08>;
> +
> +		interrupts-extended = <&tlmm 41 IRQ_TYPE_LEVEL_HIGH>;
> +		reset-gpios = <&tlmm 38 GPIO_ACTIVE_HIGH>;

[Severity: Low]
Is this GPIO polarity specified as GPIO_ACTIVE_HIGH to work around inverted
logic in the st-nci driver?

The ST21NFCD physically uses an active-low reset pin. While specifying
GPIO_ACTIVE_HIGH compensates for the driver incorrectly handling it as an
active-high enable line, it forces the DT to describe the physical hardware
polarity incorrectly.

> +
> +		pinctrl-0 = <&nfc_int_default>, <&nfc_reset_default>;
> +		pinctrl-names = "default";
> +
> +		clocks = <&rpmhcc RPMH_LN_BB_CLK2>;
> +		vdd-io-supply = <&vreg_l18b>;

[ ... ]

> @@ -1410,6 +1426,21 @@ hall_sensor_default: hall-sensor-default-state {
>  		drive-strength = <2>;
>  		bias-pull-up;
>  	};
> +
> +	nfc_int_default: nfc-int-default-state {
> +		pins = "gpio41";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +	};
> +
> +	nfc_reset_default: nfc-reset-default-state {
> +		pins = "gpio38";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +		output-high;
> +	};

[Severity: High]
Could this output-high configuration cause a hardware latch-up?

This applies the output-high state to the reset pin unconditionally at probe
time. Driving 1.8V into the pin before the NFC chip's vdd-io-supply is
enabled forward-biases ESD protection diodes, which can cause back-powering
and excessive current draw.

[Severity: High]
This is a pre-existing issue, but I wanted to mention a race condition in the
NCI device registration that might be aggravated by this patch.

In drivers/nfc/st-nci/i2c.c:st_nci_i2c_probe(), the NCI device is registered
and exposed to userspace before its state (the raw_nci flag and the IRQ
handler) is fully initialized:

drivers/nfc/st-nci/i2c.c:st_nci_i2c_probe() {
    ...
	phy->ndlc->raw_nci = phy->raw_nci;

	phy->irq_active = true;
	r = devm_request_threaded_irq(&client->dev, client->irq, NULL,
				st_nci_irq_thread_fn,
				IRQF_ONESHOT,
				ST_NCI_DRIVER_NAME, phy);
    ...
}

Userspace can react to the netlink device registration uevent and bring up the
device before the probe function finishes executing, which can lead to dropped
interrupts or misprocessing of NFC frames.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819210906.2500789-1-isyourbrainfoss@proton.me?part=3