Devicetree
 help / color / mirror / Atom feed
* [PATCH v4 0/3] nfc: st-nci: Fairphone 5 NFC bring-up (ST21NFCD)
@ 2026-09-02 21:22 Kristian Brox
  2026-09-02 21:22 ` [PATCH v4 1/3] dt-bindings: net: nfc: add st,st21nfcd Kristian Brox
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Kristian Brox @ 2026-09-02 21:22 UTC (permalink / raw)
  To: David Heidelberg, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Krzysztof Kozlowski, Konrad Dybcio, Luca Weiss, Dmitry Baryshkov,
	oe-linux-nfc, netdev, devicetree, linux-kernel, linux-arm-msm,
	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.

Patches are against linux-nfc/for-next (Linux 7.3-rc1). Hardware test
was on a Fairphone 5 running postmarketOS:

- 7.1.2 sc7280, earlier raw-NCI modules: nfctool Powered: Yes;
  initiator poll / neard: NTAG 215, NDEF URI read OK
- 7.2.0-nfc-test+ (sc7280-mainline), this v4 st-nci as modules:
  10x initiator poll finds Type 2 in 0.13-0.14 s, no
  "unsupported ntf opcode 0xf02". Repeat poll with a Type 2 tag
  in the field stays Powered: Yes. Empty-field poll SIGINT
  (nfctool -p, no tag) also stays Powered: Yes.

The ndlc: hexdumps are pr_debug. SIGINT during CORE_RESET (nfctool -0
then Ctrl-C during -1) wedges stock nci core; that is not this series.

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.

Signed-off-by: Kristian Brox <isyourbrainfoss@proton.me>
---
Link to v3:
https://lore.kernel.org/oe-linux-nfc/20260820-fp5-st21nfcd-v3-v3-0-5ee0a7f44d04@proton.me/

Changes in v4:
- Driver: consume ST proprietary RF NTF 0xf02 (Luca)
- Driver: set raw_nci before nci_register_device (Sashiko / David)
- Driver: enable vdd-io and SYS_CLK before driving reset
- Driver: keep named i2c/acpi id initializers; do not add trailing
  commas on existing of_match entries (David)
- Driver: reset pulse uses gpiod_is_active_low so new DT can be
  GPIO_ACTIVE_LOW without breaking old nfcb DTS (Krzysztof)
- Binding: ST21NFCD is not I2C-only (data brief lists SPI); do not
  put st,st21nfcd in the I2C-only spi-max-frequency:false enum
- Binding: SPI $ref only in the non-I2C else (not a global allOf item)
- Binding: clocks/vdd-io-supply on the existing I2C example; drop the
  extra example (Krzysztof)
- DTS: reset-gpios GPIO_ACTIVE_LOW (Krzysztof). Physical GPIO 38 HIGH
  is the run level. Konrad R-b not carried (polarity changed).

Changes in v3:
- Binding: new compatible is a different ST part (not a driver-sharing note)
- Binding: drop NDLC/raw-NCI description on compatible
- Binding example uses interrupts-extended
- DTS: one nfc_default pinctrl group, pins sorted, no output-high
- Fresh series (not a reply to v1/v2)

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

 .../devicetree/bindings/net/nfc/st,st-nci.yaml     | 49 +++++++++----
 arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 34 ++++++++-
 drivers/nfc/st-nci/core.c                          | 22 ++++++
 drivers/nfc/st-nci/i2c.c                           | 82 ++++++++++++++++++++--
 drivers/nfc/st-nci/ndlc.c                          | 27 +++++--
 drivers/nfc/st-nci/ndlc.h                          |  5 +-
 drivers/nfc/st-nci/se.c                            |  3 +
 drivers/nfc/st-nci/spi.c                           |  2 +-
 drivers/nfc/st-nci/st-nci.h                        |  2 +
 9 files changed, 197 insertions(+), 29 deletions(-)
---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260902-fp5-st21nfcd-v4-1685be1d7c52

Best regards,
--  
Kristian Brox <isyourbrainfoss@proton.me>



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v4 1/3] dt-bindings: net: nfc: add st,st21nfcd
  2026-09-02 21:22 [PATCH v4 0/3] nfc: st-nci: Fairphone 5 NFC bring-up (ST21NFCD) Kristian Brox
@ 2026-09-02 21:22 ` Kristian Brox
  2026-09-03 13:47   ` Krzysztof Kozlowski
  2026-09-02 21:22 ` [PATCH v4 2/3] nfc: st-nci: add raw NCI path for ST21NFCD Kristian Brox
  2026-09-02 21:22 ` [PATCH v4 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC Kristian Brox
  2 siblings, 1 reply; 9+ messages in thread
From: Kristian Brox @ 2026-09-02 21:22 UTC (permalink / raw)
  To: David Heidelberg, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Krzysztof Kozlowski, Konrad Dybcio, Luca Weiss, Dmitry Baryshkov,
	oe-linux-nfc, netdev, devicetree, linux-kernel, linux-arm-msm,
	Kristian Brox

Add a compatible for the ST ST21NFCD NFC controller. Document optional
SYS_CLK (clocks) and VPS_IO (vdd-io-supply).

ST21NFCD is not I2C-only; the ST data brief lists I2C and SPI.
Split the spi-max-frequency if/else: forbid it for
st,st21nfcb-i2c and st,st21nfcc-i2c, require it for st,st21nfcb-spi,
and leave it optional for st,st21nfcd. Add clocks and vdd-io-supply to
the existing st,st21nfcb-i2c example instead of a second I2C example.

Signed-off-by: Kristian Brox <isyourbrainfoss@proton.me>
---
 .../devicetree/bindings/net/nfc/st,st-nci.yaml     | 49 +++++++++++++++-------
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/nfc/st,st-nci.yaml b/Documentation/devicetree/bindings/net/nfc/st,st-nci.yaml
index 1dcbddbc5a74..8abafb3d8994 100644
--- a/Documentation/devicetree/bindings/net/nfc/st,st-nci.yaml
+++ b/Documentation/devicetree/bindings/net/nfc/st,st-nci.yaml
@@ -15,6 +15,7 @@ properties:
       - st,st21nfcb-i2c
       - st,st21nfcb-spi
       - st,st21nfcc-i2c
+      - st,st21nfcd
 
   reset-gpios:
     description: Output GPIO pin used for resetting the controller
@@ -36,26 +37,43 @@ 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
   - reg
   - reset-gpios
 
-if:
-  properties:
-    compatible:
-      contains:
-        enum:
-          - st,st21nfcb-i2c
-          - st,st21nfcc-i2c
-then:
-  properties:
-    spi-max-frequency: false
-else:
-  $ref: /schemas/spi/spi-peripheral-props.yaml#
-  required:
-    - spi-max-frequency
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - st,st21nfcb-i2c
+              - st,st21nfcc-i2c
+    then:
+      properties:
+        spi-max-frequency: false
+    else:
+      $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: st,st21nfcb-spi
+    then:
+      required:
+        - spi-max-frequency
 
 unevaluatedProperties: false
 
@@ -76,6 +94,9 @@ examples:
             interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
             reset-gpios = <&gpio5 29 GPIO_ACTIVE_HIGH>;
 
+            clocks = <&clk>;
+            vdd-io-supply = <&vdd_io>;
+
             ese-present;
             uicc-present;
         };

-- 
2.55.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 2/3] nfc: st-nci: add raw NCI path for ST21NFCD
  2026-09-02 21:22 [PATCH v4 0/3] nfc: st-nci: Fairphone 5 NFC bring-up (ST21NFCD) Kristian Brox
  2026-09-02 21:22 ` [PATCH v4 1/3] dt-bindings: net: nfc: add st,st21nfcd Kristian Brox
@ 2026-09-02 21:22 ` Kristian Brox
  2026-09-03 21:22   ` sashiko-bot
  2026-09-02 21:22 ` [PATCH v4 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC Kristian Brox
  2 siblings, 1 reply; 9+ messages in thread
From: Kristian Brox @ 2026-09-02 21:22 UTC (permalink / raw)
  To: David Heidelberg, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Krzysztof Kozlowski, Konrad Dybcio, Luca Weiss, Dmitry Baryshkov,
	oe-linux-nfc, netdev, devicetree, linux-kernel, linux-arm-msm,
	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
- consume proprietary RF NTF 0xf02 (GID 0xf, OID 0x02)

Set the raw_nci flag before nci_register_device so ndlc_send does
not push an NDLC PCB into a 0-headroom skb. Enable optional vdd-io
(VPS_IO) and SYS_CLK before driving reset.

Reset is active-low. Existing boards describe it as GPIO_ACTIVE_HIGH
and the driver treats gpiod_set_value(reset, 1) as chip-running.
Keep that path so old DTS is unchanged. If the GPIO is active-low,
pulse logical 1 then 0 (assert, then deassert).

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>
---
 drivers/nfc/st-nci/core.c   | 22 ++++++++++++
 drivers/nfc/st-nci/i2c.c    | 82 ++++++++++++++++++++++++++++++++++++++++++---
 drivers/nfc/st-nci/ndlc.c   | 27 +++++++++++----
 drivers/nfc/st-nci/ndlc.h   |  5 ++-
 drivers/nfc/st-nci/se.c     |  3 ++
 drivers/nfc/st-nci/spi.c    |  2 +-
 drivers/nfc/st-nci/st-nci.h |  2 ++
 7 files changed, 129 insertions(+), 14 deletions(-)

diff --git a/drivers/nfc/st-nci/core.c b/drivers/nfc/st-nci/core.c
index a367136d4330..d7802434abda 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;
 
@@ -84,12 +89,29 @@ static int st_nci_prop_rsp_packet(struct nci_dev *ndev,
 	return 0;
 }
 
+/*
+ * ST21NFCD emits proprietary NCI NTFs (GID 0xf, OID 0x02) on CORE_RESET
+ * and during each RF poll loop. The payload is an RF trace; tags are
+ * still reported with the standard RF_INTF_ACTIVATED_NTF. Consume the
+ * packet so nci_ntf_packet does not log "unsupported ntf opcode 0xf02".
+ */
+static int st_nci_prop_rf_ntf_packet(struct nci_dev *ndev,
+				     struct sk_buff *skb)
+{
+	return 0;
+}
+
 static const struct nci_driver_ops st_nci_prop_ops[] = {
 	{
 		.opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
 					  ST_NCI_CORE_PROP),
 		.rsp = st_nci_prop_rsp_packet,
 	},
+	{
+		.opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
+					  ST_NCI_PROP_RF_NTF),
+		.ntf = st_nci_prop_rf_ntf_packet,
+	},
 };
 
 static const struct nci_ops st_nci_ops = {
diff --git a/drivers/nfc/st-nci/i2c.c b/drivers/nfc/st-nci/i2c.c
index 152c20b6bb01..2ec029f31c67 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;
 
@@ -44,9 +55,20 @@ static int st_nci_i2c_enable(void *phy_id)
 {
 	struct st_nci_i2c_phy *phy = phy_id;
 
-	gpiod_set_value(phy->gpiod_reset, 0);
-	usleep_range(10000, 15000);
-	gpiod_set_value(phy->gpiod_reset, 1);
+	/*
+	 * Existing DTS uses GPIO_ACTIVE_HIGH and treats logical 1 as
+	 * chip-running. GPIO_ACTIVE_LOW: logical 1 asserts reset
+	 * (physical LOW).
+	 */
+	if (gpiod_is_active_low(phy->gpiod_reset)) {
+		gpiod_set_value(phy->gpiod_reset, 1);
+		usleep_range(10000, 15000);
+		gpiod_set_value(phy->gpiod_reset, 0);
+	} else {
+		gpiod_set_value(phy->gpiod_reset, 0);
+		usleep_range(10000, 15000);
+		gpiod_set_value(phy->gpiod_reset, 1);
+	}
 	usleep_range(80000, 85000);
 
 	if (phy->ndlc->powered == 0 && phy->irq_active == 0) {
@@ -111,6 +133,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 +269,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);
 
@@ -218,6 +278,14 @@ static int st_nci_i2c_probe(struct i2c_client *client)
 	if (r)
 		dev_dbg(dev, "Unable to add GPIO mapping table\n");
 
+	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");
+
 	/* Get RESET GPIO */
 	phy->gpiod_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(phy->gpiod_reset)) {
@@ -231,8 +299,10 @@ static int st_nci_i2c_probe(struct i2c_client *client)
 				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->ndlc, &phy->se_status);
+			phy->raw_nci ? ST_NCI_RAW_FRAME_HEADROOM :
+				       ST_NCI_FRAME_HEADROOM,
+			ST_NCI_FRAME_TAILROOM,
+			&phy->ndlc, &phy->se_status, phy->raw_nci);
 	if (r < 0) {
 		nfc_err(&client->dev, "Unable to register ndlc layer\n");
 		return r;
@@ -273,6 +343,8 @@ static const struct of_device_id of_st_nci_i2c_match[] = {
 	{ .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 be4808859cfa..13026c4facb3 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) {
@@ -251,7 +262,8 @@ static void ndlc_t2_timeout(struct timer_list *t)
 
 int ndlc_probe(void *phy_id, const struct nfc_phy_ops *phy_ops,
 	       struct device *dev, int phy_headroom, int phy_tailroom,
-	       struct llt_ndlc **ndlc_id, struct st_nci_se_status *se_status)
+	       struct llt_ndlc **ndlc_id, struct st_nci_se_status *se_status,
+	       bool raw_nci)
 {
 	struct llt_ndlc *ndlc;
 
@@ -263,6 +275,7 @@ int ndlc_probe(void *phy_id, const struct nfc_phy_ops *phy_ops,
 	ndlc->phy_id = phy_id;
 	ndlc->dev = dev;
 	ndlc->powered = 0;
+	ndlc->raw_nci = raw_nci;
 
 	*ndlc_id = ndlc;
 
diff --git a/drivers/nfc/st-nci/ndlc.h b/drivers/nfc/st-nci/ndlc.h
index c24ce9b0df52..0b12e12e47d2 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);
@@ -47,6 +49,7 @@ int ndlc_send(struct llt_ndlc *ndlc, struct sk_buff *skb);
 void ndlc_recv(struct llt_ndlc *ndlc, struct sk_buff *skb);
 int ndlc_probe(void *phy_id, const struct nfc_phy_ops *phy_ops,
 	       struct device *dev, int phy_headroom, int phy_tailroom,
-	       struct llt_ndlc **ndlc_id, struct st_nci_se_status *se_status);
+	       struct llt_ndlc **ndlc_id, struct st_nci_se_status *se_status,
+	       bool raw_nci);
 void ndlc_remove(struct llt_ndlc *ndlc);
 #endif /* __LOCAL_NDLC_H__ */
diff --git a/drivers/nfc/st-nci/se.c b/drivers/nfc/st-nci/se.c
index 607ec768eb7b..44cc102bcde2 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;
diff --git a/drivers/nfc/st-nci/spi.c b/drivers/nfc/st-nci/spi.c
index 7948c7e0c88c..1ce80a85bfe6 100644
--- a/drivers/nfc/st-nci/spi.c
+++ b/drivers/nfc/st-nci/spi.c
@@ -246,7 +246,7 @@ static int st_nci_spi_probe(struct spi_device *dev)
 
 	r = ndlc_probe(phy, &spi_phy_ops, &dev->dev,
 			ST_NCI_FRAME_HEADROOM, ST_NCI_FRAME_TAILROOM,
-			&phy->ndlc, &phy->se_status);
+			&phy->ndlc, &phy->se_status, false);
 	if (r < 0) {
 		nfc_err(&dev->dev, "Unable to register ndlc layer\n");
 		return r;
diff --git a/drivers/nfc/st-nci/st-nci.h b/drivers/nfc/st-nci/st-nci.h
index 5286071e52cf..aa8782c95d2f 100644
--- a/drivers/nfc/st-nci/st-nci.h
+++ b/drivers/nfc/st-nci/st-nci.h
@@ -15,6 +15,8 @@
 
 #define ST_NCI_CORE_PROP                0x01
 #define ST_NCI_SET_NFC_MODE             0x02
+/* ST21NFCD proprietary RF activity / poll-trace notification (GID 0xf) */
+#define ST_NCI_PROP_RF_NTF		0x02
 
 /*
  * ref ISO7816-3 chap 8.1. the initial character TS is followed by a

-- 
2.55.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC
  2026-09-02 21:22 [PATCH v4 0/3] nfc: st-nci: Fairphone 5 NFC bring-up (ST21NFCD) Kristian Brox
  2026-09-02 21:22 ` [PATCH v4 1/3] dt-bindings: net: nfc: add st,st21nfcd Kristian Brox
  2026-09-02 21:22 ` [PATCH v4 2/3] nfc: st-nci: add raw NCI path for ST21NFCD Kristian Brox
@ 2026-09-02 21:22 ` Kristian Brox
  2026-09-03  6:49   ` Konrad Dybcio
                     ` (2 more replies)
  2 siblings, 3 replies; 9+ messages in thread
From: Kristian Brox @ 2026-09-02 21:22 UTC (permalink / raw)
  To: David Heidelberg, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Krzysztof Kozlowski, Konrad Dybcio, Luca Weiss, Dmitry Baryshkov,
	oe-linux-nfc, netdev, devicetree, linux-kernel, linux-arm-msm,
	Kristian Brox

Enable the ST21NFCD on i2c9 (0x08), IRQ TLMM 41, reset TLMM 38
active-low. 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>
---
 arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 34 +++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
index f41ad446a39e..b028d1bb25d4 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
@@ -933,7 +933,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_LOW>;
+
+		pinctrl-0 = <&nfc_default>;
+		pinctrl-names = "default";
+
+		clocks = <&rpmhcc RPMH_LN_BB_CLK2>;
+		vdd-io-supply = <&vreg_l18b>;
+
+		ese-present;
+		uicc-present;
+	};
+
 	/* VL53L3 ToF @ 29 */
 };
 
@@ -1462,6 +1478,22 @@ hall_sensor_default: hall-sensor-default-state {
 		drive-strength = <2>;
 		bias-pull-up;
 	};
+
+	nfc_default: nfc-default-state {
+		reset-pins {
+			pins = "gpio38";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-disable;
+		};
+
+		int-pins {
+			pins = "gpio41";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-disable;
+		};
+	};
 };
 
 &uart5 {

-- 
2.55.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC
  2026-09-02 21:22 ` [PATCH v4 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC Kristian Brox
@ 2026-09-03  6:49   ` Konrad Dybcio
  2026-09-03  7:49   ` Abel Vesa
  2026-09-03 21:22   ` sashiko-bot
  2 siblings, 0 replies; 9+ messages in thread
From: Konrad Dybcio @ 2026-09-03  6:49 UTC (permalink / raw)
  To: Kristian Brox, David Heidelberg, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Krzysztof Kozlowski, Luca Weiss, Dmitry Baryshkov, oe-linux-nfc,
	netdev, devicetree, linux-kernel, linux-arm-msm

On 9/2/26 11:22 PM, Kristian Brox wrote:
> Enable the ST21NFCD on i2c9 (0x08), IRQ TLMM 41, reset TLMM 38
> active-low. 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>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC
  2026-09-02 21:22 ` [PATCH v4 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC Kristian Brox
  2026-09-03  6:49   ` Konrad Dybcio
@ 2026-09-03  7:49   ` Abel Vesa
  2026-09-03 21:22   ` sashiko-bot
  2 siblings, 0 replies; 9+ messages in thread
From: Abel Vesa @ 2026-09-03  7:49 UTC (permalink / raw)
  To: Kristian Brox
  Cc: David Heidelberg, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski,
	Konrad Dybcio, Luca Weiss, Dmitry Baryshkov, oe-linux-nfc, netdev,
	devicetree, linux-kernel, linux-arm-msm

On 26-09-02 21:22:25, Kristian Brox wrote:
> Enable the ST21NFCD on i2c9 (0x08), IRQ TLMM 41, reset TLMM 38
> active-low. 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>

Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: net: nfc: add st,st21nfcd
  2026-09-02 21:22 ` [PATCH v4 1/3] dt-bindings: net: nfc: add st,st21nfcd Kristian Brox
@ 2026-09-03 13:47   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-03 13:47 UTC (permalink / raw)
  To: Kristian Brox
  Cc: David Heidelberg, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio, Konrad Dybcio,
	Luca Weiss, Dmitry Baryshkov, oe-linux-nfc, netdev, devicetree,
	linux-kernel, linux-arm-msm

On Wed, Sep 02, 2026 at 09:22:18PM +0000, Kristian Brox wrote:
> Add a compatible for the ST ST21NFCD NFC controller. Document optional
> SYS_CLK (clocks) and VPS_IO (vdd-io-supply).
> 
> ST21NFCD is not I2C-only; the ST data brief lists I2C and SPI.
> Split the spi-max-frequency if/else: forbid it for
> st,st21nfcb-i2c and st,st21nfcc-i2c, require it for st,st21nfcb-spi,
> and leave it optional for st,st21nfcd. Add clocks and vdd-io-supply to
> the existing st,st21nfcb-i2c example instead of a second I2C example.
> 
> Signed-off-by: Kristian Brox <isyourbrainfoss@proton.me>
> ---
>  .../devicetree/bindings/net/nfc/st,st-nci.yaml     | 49 +++++++++++++++-------
>  1 file changed, 35 insertions(+), 14 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC
  2026-09-02 21:22 ` [PATCH v4 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC Kristian Brox
  2026-09-03  6:49   ` Konrad Dybcio
  2026-09-03  7:49   ` Abel Vesa
@ 2026-09-03 21:22   ` sashiko-bot
  2 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-09-03 21:22 UTC (permalink / raw)
  To: Kristian Brox; +Cc: robh, conor+dt, devicetree

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

Pre-existing issues:
- [High] Leaked NCI device and potential Use-After-Free on IRQ request failure
- [High] Double IRQ disable causing permanent interrupt loss in ST_NCI
--

commit f44916e46bbad8ef743764748ded612267568862
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-low. Compatible is st,st21nfcd (raw NCI).

> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> index f41ad446a39e2..b028d1bb25d4c 100644
> --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> @@ -933,7 +933,23 @@ redriver_ss_in: endpoint {
>  &i2c9 {
>  	status = "okay";
>  
> -	/* ST21NFC NFC @ 28 */
> +	nfc@8 {
> +		compatible = "st,st21nfcd";

[Severity: High]
This isn't a bug introduced by this patch, but I noticed some pre-existing
issues in the st-nci driver that this enables.

Does st_nci_i2c_probe() leak the NCI device and cause a use-after-free if
the IRQ request fails?

drivers/nfc/st-nci/i2c.c:st_nci_i2c_probe() {
    ...
	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;
}

If devm_request_threaded_irq() fails, it returns the error without calling
ndlc_remove() to unregister and free the NCI device. This leaves a dangling
network interface registered, retaining pointers to freed devres memory (phy
and ndlc structures). Any subsequent userspace access could dereference
freed memory.


[Severity: High]
This is a pre-existing issue, but can a race condition cause a double IRQ
disable and permanent interrupt loss?

In ndlc_close(), ndlc->powered is set to 0 before calling
ndlc->ops->disable(). If an interrupt fires during this window, the IRQ
thread races with the shutdown path:

drivers/nfc/st-nci/i2c.c:st_nci_irq_thread_fn() {
    ...
	if (!phy->ndlc->powered) {
		st_nci_i2c_disable(phy);
		return IRQ_HANDLED;
	}
    ...
}

drivers/nfc/st-nci/i2c.c:st_nci_i2c_disable() {
    ...
	disable_irq_nosync(phy->i2c_dev->irq);
	phy->irq_active = false;
}

Both paths could call disable_irq_nosync(), incrementing the IRQ descriptor
depth to 2. Subsequent enable_irq() calls would only decrement it once,
leaving the NFC controller permanently unresponsive to interrupts.

> +		reg = <0x08>;
> +
> +		interrupts-extended = <&tlmm 41 IRQ_TYPE_LEVEL_HIGH>;
> +		reset-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-fp5-st21nfcd-v4-v4-0-ded2f1c501be@proton.me?part=3

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 2/3] nfc: st-nci: add raw NCI path for ST21NFCD
  2026-09-02 21:22 ` [PATCH v4 2/3] nfc: st-nci: add raw NCI path for ST21NFCD Kristian Brox
@ 2026-09-03 21:22   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-09-03 21:22 UTC (permalink / raw)
  To: Kristian Brox; +Cc: robh, conor+dt, devicetree

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

Pre-existing issues:
- [Critical] Buffer overflow (`skb_over_panic`) in `st_nci_i2c_read()` due to insufficient SKB allocation size.
- [High] Missing cleanup of NCI device on IRQ request failure leads to Use-After-Free.
- [High] Improper teardown ordering in `ndlc_remove()` allows asynchronous work to execute after structures are freed.
- [High] Memory leak of supervisor `skb` frames in `llt_ndlc_rcv_queue()`.
--

commit c09112faf79ae0f9e5be39ae7d2605850d39efc5
Author: Kristian Brox <isyourbrainfoss@proton.me>

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

This commit adds raw NCI support for the ST21NFCD NFC controller. It bypasses
the NDLC PCB headers and ACK timers for I2C communication and consumes
proprietary RF activity notifications.

> diff --git a/drivers/nfc/st-nci/i2c.c b/drivers/nfc/st-nci/i2c.c
> index 152c20b6bb01a..2ec029f31c673 100644
> --- a/drivers/nfc/st-nci/i2c.c
> +++ b/drivers/nfc/st-nci/i2c.c
[ ... ]
> @@ -111,6 +133,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_I2C_MIN_SIZE);
>  	if (r < 0) {  /* Retry, chip was in standby */
>  		usleep_range(1000, 4000);
>  		r = i2c_master_recv(client, buf, ST_NCI_I2C_MIN_SIZE);
>  	}
> 
>  	if (r != ST_NCI_I2C_MIN_SIZE)
>  		return -EREMOTEIO;
> 
>  	len = be16_to_cpu(*(__be16 *) (buf + 2));
>  	if (len > ST_NCI_I2C_MAX_SIZE) {
>  		nfc_err(&client->dev, "invalid frame len\n");
>  		return -EBADMSG;
>  	}
> 
>  	*skb = alloc_skb(ST_NCI_I2C_MIN_SIZE + len, GFP_KERNEL);
>  	if (*skb == NULL)
>  		return -ENOMEM;
> 
>  	skb_reserve(*skb, ST_NCI_I2C_MIN_SIZE);
>  	skb_put(*skb, ST_NCI_I2C_MIN_SIZE);
>  	memcpy((*skb)->data, buf, ST_NCI_I2C_MIN_SIZE);
> 
>  	if (!len)
>  		return 0;
> 
>  	r = i2c_master_recv(client, buf, len);
>  	if (r != len) {
>  		kfree_skb(*skb);
>  		return -EREMOTEIO;
>  	}
> 
>  	skb_put(*skb, len);

[Severity: Critical]
This is a pre-existing issue, but does this allocation provide enough room
for both the reserved headroom and the data being put into the SKB?

When allocating an SKB of size ST_NCI_I2C_MIN_SIZE + len and then reserving
ST_NCI_I2C_MIN_SIZE bytes, the available tailroom is reduced to just len.
Since the code subsequently calls skb_put() for ST_NCI_I2C_MIN_SIZE bytes
and then later for len bytes, the total space requested is
ST_NCI_I2C_MIN_SIZE + len. Because the tailroom is only len, can this cause
a buffer overflow leading to an skb_over_panic?

[ ... ]
> @@ -231,8 +299,10 @@ static int st_nci_i2c_probe(struct i2c_client *client)
[ ... ]
>  	r = ndlc_probe(phy, &i2c_phy_ops, &client->dev,
> -			ST_NCI_FRAME_HEADROOM, ST_NCI_FRAME_TAILROOM,
> -			&phy->ndlc, &phy->se_status);
> +			phy->raw_nci ? ST_NCI_RAW_FRAME_HEADROOM :
> +				       ST_NCI_FRAME_HEADROOM,
> +			ST_NCI_FRAME_TAILROOM,
> +			&phy->ndlc, &phy->se_status, phy->raw_nci);
>  	if (r < 0) {
>  		nfc_err(&client->dev, "Unable to register ndlc layer\n");
>  		return r;
>  	}
> 
>  	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 this error path properly clean up the
NCI device?

If devm_request_threaded_irq() fails, the function logs an error and
returns directly, allowing devres to free the memory. However, the NCI
device that was registered earlier via ndlc_probe() -> st_nci_probe()
is never unregistered because ndlc_remove() is not called.

Can this leave the NCI core and network stack with visibility to a freed
device, eventually causing a use-after-free?

> diff --git a/drivers/nfc/st-nci/ndlc.c b/drivers/nfc/st-nci/ndlc.c
> index be4808859cfa9..13026c4facb3a 100644
> --- a/drivers/nfc/st-nci/ndlc.c
> +++ b/drivers/nfc/st-nci/ndlc.c
[ ... ]
> @@ -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) {
> 			switch (pcb & PCB_SYNC_MASK) {
> 			case PCB_SYNC_ACK:
> 				skb = skb_dequeue(&ndlc->ack_pending_q);
> 				kfree_skb(skb);
> 				timer_delete_sync(&ndlc->t1_timer);

[Severity: High]
This is a pre-existing issue, but does this loop leak the received supervisor
frame?

When an incoming frame is dequeued from ndlc->rcv_q, it is stored in the
local skb variable. In the PCB_SYNC_ACK case, this variable is overwritten
with a frame from ack_pending_q, which is then freed. The original received
ACK skb is never freed.

Similarly, for PCB_SYNC_NACK and PCB_SYNC_WAIT, the switch breaks and the loop
continues without freeing the original skb. Does this leak a frame for every
acknowledgment received?

[ ... ]

While looking at the lifecycle management in ndlc.c, I noticed another
pre-existing issue in ndlc_remove():

	/* cancel work */
	cancel_work_sync(&ndlc->sm_work);

	st_nci_remove(ndlc->ndev);

[Severity: High]
This isn't a bug introduced by this patch, but is the teardown ordering here
safe?

The asynchronous work queue sm_work is cancelled before the NCI device is
unregistered via st_nci_remove(). Because the NCI device is still fully
registered, a concurrent thread like the network stack can send a frame by
calling nci_send_frame() -> ndlc_send(), which unconditionally executes
schedule_work(&ndlc->sm_work).

Could this allow the re-armed work queue to execute after devres has freed
the ndlc structures, leading to a use-after-free?

> diff --git a/drivers/nfc/st-nci/spi.c b/drivers/nfc/st-nci/spi.c
> index 7948c7e0c88c0..1ce80a85bfe6a 100644
> --- a/drivers/nfc/st-nci/spi.c
> +++ b/drivers/nfc/st-nci/spi.c
[ ... ]
> @@ -246,7 +246,7 @@ static int st_nci_spi_probe(struct spi_device *dev)
>  
>  	r = ndlc_probe(phy, &spi_phy_ops, &dev->dev,
>  			ST_NCI_FRAME_HEADROOM, ST_NCI_FRAME_TAILROOM,
> -			&phy->ndlc, &phy->se_status);
> +			&phy->ndlc, &phy->se_status, false);
>  	if (r < 0) {
>  		nfc_err(&dev->dev, "Unable to register ndlc layer\n");
>  		return r;
>  	}
>  
>  	phy->irq_active = true;
>  	r = devm_request_threaded_irq(&dev->dev, dev->irq, NULL,
>  				st_nci_irq_thread_fn,
>  				IRQF_ONESHOT,
>  				ST_NCI_SPI_DRIVER_NAME, phy);
>  	if (r < 0)
>  		nfc_err(&dev->dev, "Unable to register IRQ handler\n");
>  
>  	return r;
>  }

[Severity: High]
This is a pre-existing issue, but st_nci_spi_probe() appears to have the
exact same error handling flaw as st_nci_i2c_probe().

If devm_request_threaded_irq() fails, the NCI device registered by
ndlc_probe() is never unregistered, leaving a dangling reference.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-fp5-st21nfcd-v4-v4-0-ded2f1c501be@proton.me?part=2

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-09-03 21:23 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02 21:22 [PATCH v4 0/3] nfc: st-nci: Fairphone 5 NFC bring-up (ST21NFCD) Kristian Brox
2026-09-02 21:22 ` [PATCH v4 1/3] dt-bindings: net: nfc: add st,st21nfcd Kristian Brox
2026-09-03 13:47   ` Krzysztof Kozlowski
2026-09-02 21:22 ` [PATCH v4 2/3] nfc: st-nci: add raw NCI path for ST21NFCD Kristian Brox
2026-09-03 21:22   ` sashiko-bot
2026-09-02 21:22 ` [PATCH v4 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC Kristian Brox
2026-09-03  6:49   ` Konrad Dybcio
2026-09-03  7:49   ` Abel Vesa
2026-09-03 21:22   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox