Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] dt-bindings: net: nfc: add st,st21nfcd-i2c
       [not found] ` <20260818205940.1973944-2-isyourbrainfoss@proton.me>
@ 2026-08-18 23:22   ` David Heidelberg
  0 siblings, 0 replies; 8+ messages in thread
From: David Heidelberg @ 2026-08-18 23:22 UTC (permalink / raw)
  To: Kristian Brox, oe-linux-nfc
  Cc: krzysztof.kozlowski, luca.weiss, linux-arm-msm, netdev

On 18/08/2026 22:59, 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 difference and add an I2C example without
> ese-present / uicc-present.
> 
> 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 1dcbddbc5..3523a8df0 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-i2c is ST21NFCD with raw NCI (no NDLC PCB).
>       enum:
>         - st,st21nfcb-i2c
>         - st,st21nfcb-spi
>         - st,st21nfcc-i2c
> +      - st,st21nfcd-i2c

Hello Kristian.

Thank you for your patches!

Before I do full review, for next revision, definitely the compatible should 
omit -i2c suffix, thus only `st,st21nfcd`.

If possible, send next revision with b4 (thou you don't have to, it's usually 
more fancy and convenient for both parties, takes care about CCing the right 
people, checkpatch etc.) [1].

Thanks
David

[1] https://b4.docs.kernel.org/en/latest/contributor/overview.html

[...]

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

* [PATCH v2 0/3] nfc: st-nci: Fairphone 5 NFC bring-up (ST21NFCD)
       [not found] <20260818205940.1973944-1-isyourbrainfoss@proton.me>
       [not found] ` <20260818205940.1973944-2-isyourbrainfoss@proton.me>
@ 2026-08-19 21:09 ` Kristian Brox
  2026-08-19 21:09   ` [PATCH v2 1/3] dt-bindings: net: nfc: add st,st21nfcd Kristian Brox
                     ` (2 more replies)
  1 sibling, 3 replies; 8+ messages in thread
From: Kristian Brox @ 2026-08-19 21:09 UTC (permalink / raw)
  To: oe-linux-nfc
  Cc: david, krzk+dt, devicetree, dmitry.baryshkov, luca.weiss,
	linux-arm-msm, netdev

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>
---


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

* [PATCH v2 1/3] dt-bindings: net: nfc: add st,st21nfcd
  2026-08-19 21:09 ` [PATCH v2 0/3] nfc: st-nci: Fairphone 5 NFC bring-up (ST21NFCD) Kristian Brox
@ 2026-08-19 21:09   ` Kristian Brox
  2026-08-19 21:47     ` David Heidelberg
  2026-08-20  6:05     ` Krzysztof Kozlowski
  2026-08-19 21:09   ` [PATCH v2 2/3] nfc: st-nci: add raw NCI path for ST21NFCD Kristian Brox
  2026-08-19 21:09   ` [PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC Kristian Brox
  2 siblings, 2 replies; 8+ messages in thread
From: Kristian Brox @ 2026-08-19 21:09 UTC (permalink / raw)
  To: oe-linux-nfc
  Cc: david, krzk+dt, devicetree, dmitry.baryshkov, luca.weiss,
	linux-arm-msm, netdev

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>


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

* [PATCH v2 2/3] nfc: st-nci: add raw NCI path for ST21NFCD
  2026-08-19 21:09 ` [PATCH v2 0/3] nfc: st-nci: Fairphone 5 NFC bring-up (ST21NFCD) Kristian Brox
  2026-08-19 21:09   ` [PATCH v2 1/3] dt-bindings: net: nfc: add st,st21nfcd Kristian Brox
@ 2026-08-19 21:09   ` Kristian Brox
  2026-08-19 21:09   ` [PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC Kristian Brox
  2 siblings, 0 replies; 8+ messages in thread
From: Kristian Brox @ 2026-08-19 21:09 UTC (permalink / raw)
  To: oe-linux-nfc
  Cc: david, krzk+dt, devicetree, dmitry.baryshkov, luca.weiss,
	linux-arm-msm, netdev

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;


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

* [PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC
  2026-08-19 21:09 ` [PATCH v2 0/3] nfc: st-nci: Fairphone 5 NFC bring-up (ST21NFCD) Kristian Brox
  2026-08-19 21:09   ` [PATCH v2 1/3] dt-bindings: net: nfc: add st,st21nfcd Kristian Brox
  2026-08-19 21:09   ` [PATCH v2 2/3] nfc: st-nci: add raw NCI path for ST21NFCD Kristian Brox
@ 2026-08-19 21:09   ` Kristian Brox
  2026-08-19 21:54     ` David Heidelberg
  2 siblings, 1 reply; 8+ messages in thread
From: Kristian Brox @ 2026-08-19 21:09 UTC (permalink / raw)
  To: oe-linux-nfc
  Cc: david, krzk+dt, devicetree, dmitry.baryshkov, luca.weiss,
	linux-arm-msm, netdev

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 {


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

* Re: [PATCH v2 1/3] dt-bindings: net: nfc: add st,st21nfcd
  2026-08-19 21:09   ` [PATCH v2 1/3] dt-bindings: net: nfc: add st,st21nfcd Kristian Brox
@ 2026-08-19 21:47     ` David Heidelberg
  2026-08-20  6:05     ` Krzysztof Kozlowski
  1 sibling, 0 replies; 8+ messages in thread
From: David Heidelberg @ 2026-08-19 21:47 UTC (permalink / raw)
  To: Kristian Brox, oe-linux-nfc
  Cc: krzk+dt, devicetree, dmitry.baryshkov, luca.weiss, linux-arm-msm,
	netdev

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>
> 


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

* Re: [PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC
  2026-08-19 21:09   ` [PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC Kristian Brox
@ 2026-08-19 21:54     ` David Heidelberg
  0 siblings, 0 replies; 8+ messages in thread
From: David Heidelberg @ 2026-08-19 21:54 UTC (permalink / raw)
  To: Kristian Brox, oe-linux-nfc
  Cc: krzk+dt, devicetree, dmitry.baryshkov, luca.weiss, linux-arm-msm,
	netdev

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 {
> 


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

* Re: [PATCH v2 1/3] dt-bindings: net: nfc: add st,st21nfcd
  2026-08-19 21:09   ` [PATCH v2 1/3] dt-bindings: net: nfc: add st,st21nfcd Kristian Brox
  2026-08-19 21:47     ` David Heidelberg
@ 2026-08-20  6:05     ` Krzysztof Kozlowski
  1 sibling, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2026-08-20  6:05 UTC (permalink / raw)
  To: Kristian Brox, oe-linux-nfc
  Cc: david, krzk+dt, devicetree, dmitry.baryshkov, luca.weiss,
	linux-arm-msm, netdev

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

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

end of thread, other threads:[~2026-08-20  6:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260818205940.1973944-1-isyourbrainfoss@proton.me>
     [not found] ` <20260818205940.1973944-2-isyourbrainfoss@proton.me>
2026-08-18 23:22   ` [PATCH 1/3] dt-bindings: net: nfc: add st,st21nfcd-i2c David Heidelberg
2026-08-19 21:09 ` [PATCH v2 0/3] nfc: st-nci: Fairphone 5 NFC bring-up (ST21NFCD) Kristian Brox
2026-08-19 21:09   ` [PATCH v2 1/3] dt-bindings: net: nfc: add st,st21nfcd Kristian Brox
2026-08-19 21:47     ` David Heidelberg
2026-08-20  6:05     ` Krzysztof Kozlowski
2026-08-19 21:09   ` [PATCH v2 2/3] nfc: st-nci: add raw NCI path for ST21NFCD Kristian Brox
2026-08-19 21:09   ` [PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: add ST21NFCD NFC Kristian Brox
2026-08-19 21:54     ` David Heidelberg

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