[PATCH v3 0/5] nfc: s3fwrn5: support the S3NRN4V variant

[PATCH v3 0/5] nfc: s3fwrn5: support the S3NRN4V variant

[Jorijn van der Graaf]

This adds support for the Samsung S3NRN4V, a later part in the Samsung
NCI NFC controller line the s3fwrn5 driver covers, found e.g. on the
Fairphone 6 (SM7635).

The S3NRN4V differs from the already-supported parts in three ways: it
ships with working firmware behind a bootloader protocol the driver
does not implement (so firmware download is skipped), it loads its RF
calibration data through a different proprietary command (DUAL_OPTION),
and it gates its reference clock through a CLK_REQ line that the driver
must service for the chip to be able to generate the 13.56 MHz poll
carrier.

Patches 1-2 are preparatory cleanups from the v2 review; patch 3 adds
the compatible and the clk-req-gpios property to the binding; patch 4
implements the variant in the driver; patch 5 adds the Fairphone 6
board nodes (it depends on patch 3 and can equally go through the qcom
tree once the binding has landed).

Tested on a Fairphone 6 running a milos-mainline kernel: reader mode
polls and reads ISO 14443-4 tags reliably, from a fresh boot and across
driver reloads, with the calibration files present at their new path
(load success verified in the log) and also absent (the chip falls back
to its flash defaults and still reads).

Luca, the two questions from the v2 thread (revision/SKU variance of
the calibration tables, linux-firmware submission) are still open;
nothing in this series depends on the answers now that the variant
property is deferred.

The series is based on the nfc tree's for-next.

Changes in v3:
 - Call the DUAL_OPTION payload what it is -- RF calibration data --
   in the driver commit message, comments and log strings (David).
 - Load the calibration files from a chip-scoped directory,
   samsung/s3nrn4v/{hwreg,swreg}.bin, instead of flat
   sec_s3nrn4v_*.bin names (David). The samsung,calibration-variant
   property discussed in the v2 thread is deferred until a device
   actually needs a second set of tables; it can be added compatibly
   on top.
 - Request the calibration files with firmware_request_nowarn(): they
   are optional by design and the driver logs its own warning, the way
   ath10k requests its calibration files.
 - Reword "S3FWRN5-family" in the binding and driver commit messages
   to "a later part in the Samsung (NCI) NFC controller line" (David).
   The binding change is commit-message-only, its content is unchanged
   from v2, so Conor's Acked-by is retained.
 - New patch 1: drop of_match_ptr() so the OF table is referenced in
   every configuration (its __maybe_unused half is already in the nfc
   tree as commit 9739e07fdb29 ("nfc: Drop __maybe_unused from
   of_device_id tables")).
 - New patch 2: spell out the "s3fwrn5_i2c" driver-name string
   (Suggested by David).
 - New patch 5: the Fairphone 6 board nodes, including the
   pvdd-supply the binding marks required for the S3NRN4V.
 - Rebase onto the nfc tree's for-next; the new of_device_id entries
   follow the style of commit 179851e63fde ("nfc: Unify style of
   of_device_id arrays").

Changes in v2:
 - Drop the -i2c bus suffix from the new compatible: it is now plain
   samsung,s3nrn4v (Requested by: Conor Dooley).
 - Close a race in the probe-time CLK_REQ seeding by reading the GPIO
   level under clk_lock, so a stale level can never overwrite a fresher
   state applied by the irq thread (found by the Sashiko AI review of
   v1).
 - Binding completeness: document the PVDD supply (required for the
   S3NRN4V), add an S3NRN4V example exercising the new properties, make
   clk-req-gpios depend on clocks, and describe the CLK_REQ pin in
   hardware terms.
 - Rework the binding commit message: justify the GPIO modelling in
   hardware terms and explain why no fallback compatible applies.
 - Add an s3nrn4v i2c_device_id entry carrying the variant so both
   match paths agree, and describe the of_match_ptr() removal in the
   driver commit message.
 - Reject malformed calibration blobs (word alignment, single-byte
   section index bound) up front instead of failing at STOP_UPDATE.
 - Handle gpiod_get_value_cansleep() failure in the CLK_REQ sync
   instead of treating an error as "clock off".

v2: https://lore.kernel.org/20260705190621.128257-1-jorijnvdgraaf@catcrafts.net
v1: https://lore.kernel.org/20260703202601.78563-1-jorijnvdgraaf@catcrafts.net

Jorijn van der Graaf (5):
  nfc: s3fwrn5: drop of_match_ptr() from the I2C driver
  nfc: s3fwrn5: use the "s3fwrn5_i2c" string literal directly
  dt-bindings: net: nfc: samsung,s3fwrn5: add S3NRN4V and clk-req-gpios
  nfc: s3fwrn5: support the S3NRN4V variant
  arm64: dts: qcom: milos-fairphone-fp6: Add NFC

 .../bindings/net/nfc/samsung,s3fwrn5.yaml     |  65 +++++++-
 .../boot/dts/qcom/milos-fairphone-fp6.dts     |  49 +++++-
 drivers/nfc/s3fwrn5/core.c                    |  41 ++++-
 drivers/nfc/s3fwrn5/i2c.c                     | 142 ++++++++++++++++--
 drivers/nfc/s3fwrn5/nci.c                     | 121 ++++++++++++++-
 drivers/nfc/s3fwrn5/nci.h                     |  32 +++-
 drivers/nfc/s3fwrn5/s3fwrn5.h                 |  15 ++-
 drivers/nfc/s3fwrn5/uart.c                    |   2 +-
 8 files changed, 446 insertions(+), 21 deletions(-)


base-commit: f0054d864e6e734add71e29961f5ae15211ad24d
-- 
2.55.0

[PATCH v3 1/5] nfc: s3fwrn5: drop of_match_ptr() from the I2C driver

[Jorijn van der Graaf]

Since commit 9739e07fdb29 ("nfc: Drop __maybe_unused from of_device_id
tables") the OF match table relies on its MODULE_DEVICE_TABLE()
reference to count as used, while of_match_ptr() still discards the
.of_match_table reference on !CONFIG_OF builds. Drop of_match_ptr() so
every configuration uses the same code; the S3NRN4V support added later
in this series carries its variant selector in this table's match data.

Assisted-by: Claude:claude-fable-5
Assisted-by: Claude:claude-opus-5
Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
---
 drivers/nfc/s3fwrn5/i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index 4ba762611711..cb73993446e0 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -219,7 +219,7 @@ MODULE_DEVICE_TABLE(of, of_s3fwrn5_i2c_match);
 static struct i2c_driver s3fwrn5_i2c_driver = {
 	.driver = {
 		.name = S3FWRN5_I2C_DRIVER_NAME,
-		.of_match_table = of_match_ptr(of_s3fwrn5_i2c_match),
+		.of_match_table = of_s3fwrn5_i2c_match,
 	},
 	.probe = s3fwrn5_i2c_probe,
 	.remove = s3fwrn5_i2c_remove,

base-commit: f0054d864e6e734add71e29961f5ae15211ad24d
-- 
2.55.0

[PATCH v3 2/5] nfc: s3fwrn5: use the "s3fwrn5_i2c" string literal directly

[Jorijn van der Graaf]

The S3FWRN5_I2C_DRIVER_NAME define is an indirection for a string that
is fixed ABI (the driver name) and only used within this file. Spell
the string out at its three users and drop the define.

Suggested-by: David Heidelberg <david@ixit.cz>
Assisted-by: Claude:claude-fable-5
Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
---
 drivers/nfc/s3fwrn5/i2c.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index cb73993446e0..c528c085b43e 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -16,8 +16,6 @@
 
 #include "phy_common.h"
 
-#define S3FWRN5_I2C_DRIVER_NAME "s3fwrn5_i2c"
-
 struct s3fwrn5_i2c_phy {
 	struct phy_common common;
 	struct i2c_client *i2c_dev;
@@ -186,7 +184,7 @@ static int s3fwrn5_i2c_probe(struct i2c_client *client)
 
 	ret = devm_request_threaded_irq(&client->dev, phy->i2c_dev->irq, NULL,
 		s3fwrn5_i2c_irq_thread_fn, IRQF_ONESHOT,
-		S3FWRN5_I2C_DRIVER_NAME, phy);
+		"s3fwrn5_i2c", phy);
 	if (ret)
 		goto s3fwrn5_remove;
 
@@ -205,7 +203,7 @@ static void s3fwrn5_i2c_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id s3fwrn5_i2c_id_table[] = {
-	{ .name = S3FWRN5_I2C_DRIVER_NAME },
+	{ .name = "s3fwrn5_i2c" },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, s3fwrn5_i2c_id_table);
@@ -218,7 +216,7 @@ MODULE_DEVICE_TABLE(of, of_s3fwrn5_i2c_match);
 
 static struct i2c_driver s3fwrn5_i2c_driver = {
 	.driver = {
-		.name = S3FWRN5_I2C_DRIVER_NAME,
+		.name = "s3fwrn5_i2c",
 		.of_match_table = of_s3fwrn5_i2c_match,
 	},
 	.probe = s3fwrn5_i2c_probe,
-- 
2.55.0

[PATCH v3 3/5] dt-bindings: net: nfc: samsung,s3fwrn5: add S3NRN4V and clk-req-gpios

[Jorijn van der Graaf]

The S3NRN4V is a later part in the Samsung NCI NFC controller line
this binding documents, found e.g. on the Fairphone 6 (SM7635). Its
host interface is NCI over I2C like the S3FWRN5, but it is not
compatible with any of the existing parts, so no fallback compatible
applies: its bootloader speaks a different protocol and its RF
configuration is loaded through a different proprietary command set.

Document the optional clk-req-gpios property: the controller's CLK_REQ
output is asserted for as long as the chip needs its external reference
clock, notably while generating the 13.56 MHz poll carrier. The pin is
a level signal reflecting the chip's current clock demand, not a
one-shot event, so it is modelled as a GPIO. No user of the handshake
is known on the already-supported parts, so the property is restricted
to the S3NRN4V (easily relaxed should one appear), and it depends on
clocks, as its purpose is gating an external clock.

Also document the PVDD supply, the externally switched rail powering
the controller (boards feed it from a PMIC LDO). It is required for the
new device; deployed DTs for the existing parts never described a
supply, so for those it stays optional. Add an example for the new
device exercising the new properties.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Assisted-by: Claude:claude-opus-4-8
Assisted-by: Claude:claude-fable-5
Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
---
 .../bindings/net/nfc/samsung,s3fwrn5.yaml     | 65 ++++++++++++++++++-
 1 file changed, 64 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
index 12baee45752c..1e784a90d015 100644
--- a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
+++ b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
@@ -14,12 +14,20 @@ properties:
     enum:
       - samsung,s3fwrn5-i2c
       - samsung,s3fwrn82
+      - samsung,s3nrn4v
 
   en-gpios:
     maxItems: 1
     description:
       Output GPIO pin used for enabling/disabling the chip
 
+  clk-req-gpios:
+    maxItems: 1
+    description:
+      Input GPIO pin connected to the controller's CLK_REQ output, which the
+      controller asserts for as long as it requires its external reference
+      clock.
+
   interrupts:
     maxItems: 1
 
@@ -29,6 +37,9 @@ properties:
   clocks:
     maxItems: 1
 
+  pvdd-supply:
+    description: PVDD power supply
+
   wake-gpios:
     maxItems: 1
     description:
@@ -53,17 +64,45 @@ required:
   - en-gpios
   - wake-gpios
 
+# The clock-request handshake gates an external clock, so it needs one.
+dependencies:
+  clk-req-gpios: [ clocks ]
+
 allOf:
   - if:
       properties:
         compatible:
           contains:
-            const: samsung,s3fwrn5-i2c
+            enum:
+              - samsung,s3fwrn5-i2c
+              - samsung,s3nrn4v
     then:
       required:
         - interrupts
         - reg
 
+  # No user of the clock-request handshake is known on the other parts.
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              const: samsung,s3nrn4v
+    then:
+      properties:
+        clk-req-gpios: false
+
+  # Deployed DTs for the older parts never described a supply, so PVDD is
+  # only required for the new device.
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: samsung,s3nrn4v
+    then:
+      required:
+        - pvdd-supply
+
 examples:
   - |
     #include <dt-bindings/gpio/gpio.h>
@@ -97,3 +136,27 @@ examples:
 
         };
     };
+  # S3NRN4V with a clock-request gated external clock
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        nfc@27 {
+            compatible = "samsung,s3nrn4v";
+            reg = <0x27>;
+
+            interrupt-parent = <&tlmm>;
+            interrupts = <31 IRQ_TYPE_EDGE_RISING>;
+
+            en-gpios = <&tlmm 56 GPIO_ACTIVE_HIGH>;
+            wake-gpios = <&tlmm 7 GPIO_ACTIVE_HIGH>;
+            clk-req-gpios = <&tlmm 6 GPIO_ACTIVE_HIGH>;
+
+            clocks = <&rpmhcc 4>;
+            pvdd-supply = <&nfc_pvdd>;
+        };
+    };
-- 
2.55.0

[PATCH v3 4/5] nfc: s3fwrn5: support the S3NRN4V variant

[Jorijn van der Graaf]

The S3NRN4V (e.g. on the Fairphone 6, SM7635) is a later part in the
Samsung NFC controller line this driver covers. It needs different
bring-up, selected with a new samsung,s3nrn4v compatible:

 - It ships with working firmware behind a bootloader protocol this
   driver does not implement (GET_BOOTINFO times out), so the firmware
   download step is skipped. Its RF calibration data is loaded with the
   proprietary DUAL_OPTION command (the HW and SW blobs merged into a
   single stream) instead of the START/SET/STOP_RFREG sequence.

 - Its reference clock speed is configured with the single-byte FW_CFG
   form, sent from the ->setup hook (after CORE_RESET, before CORE_INIT).
   The selector value (0x11) is taken from the vendor configuration for
   this part; its encoding is not documented.

 - It gates its XI clock through a CLK_REQ line: the chip drives it high
   when it needs the clock, notably to synthesise the 13.56 MHz poll
   carrier. Left always-on, the free-running clock never lets the chip's
   TX PLL lock on a fresh start and it cannot poll (it falls back to
   listen only). Service the handshake when a clk-req GPIO is described,
   gating the clock on it; without one the clock stays always-on.

The calibration data is requested from a chip-scoped directory,
samsung/s3nrn4v/hwreg.bin and samsung/s3nrn4v/swreg.bin, rather than the
flat sec_s3fwrn5_rfreg.bin name the older parts use, leaving room for
per-device data should a second S3NRN4V design need it.

The variant is carried as match data by both the OF and the I2C device
id tables so the two match paths agree.

The error policy differs between the two configuration steps on purpose:
a clock misconfiguration is fatal (a ->setup failure aborts CORE_INIT),
whereas a calibration data failure is only warned about and bring-up
continues, since the chip falls back to the values programmed in its
flash and NFC may still work. The files are requested with
firmware_request_nowarn() accordingly.

Unlike the host-endian word read in the legacy rfreg path, the
DUAL_OPTION checksum is accumulated with get_unaligned_le32() and emitted
little-endian explicitly, so it is correct regardless of CPU endianness.

Existing S3FWRN5 / S3FWRN82 setups keep the firmware-download path and
the always-on clock, unchanged.

Assisted-by: Claude:claude-opus-4-8
Assisted-by: Claude:claude-fable-5
Assisted-by: Claude:claude-opus-5
Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
---
 drivers/nfc/s3fwrn5/core.c    |  41 ++++++++++-
 drivers/nfc/s3fwrn5/i2c.c     | 134 ++++++++++++++++++++++++++++++++--
 drivers/nfc/s3fwrn5/nci.c     | 121 +++++++++++++++++++++++++++++-
 drivers/nfc/s3fwrn5/nci.h     |  32 +++++++-
 drivers/nfc/s3fwrn5/s3fwrn5.h |  15 +++-
 drivers/nfc/s3fwrn5/uart.c    |   2 +-
 6 files changed, 332 insertions(+), 13 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/core.c b/drivers/nfc/s3fwrn5/core.c
index af0fa8bd970b..4f5b9e008773 100644
--- a/drivers/nfc/s3fwrn5/core.c
+++ b/drivers/nfc/s3fwrn5/core.c
@@ -122,11 +122,48 @@ static int s3fwrn5_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
 	return 0;
 }
 
+static int s3fwrn5_nci_setup(struct nci_dev *ndev)
+{
+	struct s3fwrn5_info *info = nci_get_drvdata(ndev);
+
+	/*
+	 * Runs after CORE_RESET, before CORE_INIT. The S3NRN4V needs its
+	 * reference clock configured here (the downstream stack does it in the
+	 * bootloader, before CORE_RESET, but this is the earliest hook the NCI
+	 * core offers and the chip accepts it).
+	 */
+	if (info->variant == S3FWRN5_VARIANT_S3NRN4V)
+		return s3fwrn5_nci_clk_cfg(info);
+
+	return 0;
+}
+
 static int s3fwrn5_nci_post_setup(struct nci_dev *ndev)
 {
 	struct s3fwrn5_info *info = nci_get_drvdata(ndev);
 	int ret;
 
+	if (info->variant == S3FWRN5_VARIANT_S3NRN4V) {
+		/*
+		 * The S3NRN4V ships with working firmware behind a bootloader
+		 * protocol this driver does not implement, so there is no
+		 * download step; the NCI core has already done CORE_RESET +
+		 * CORE_INIT. Just load the RF calibration data via DUAL_OPTION.
+		 */
+		ret = s3fwrn5_nci_rf_configure_dual(info,
+						    "samsung/s3nrn4v/hwreg.bin",
+						    "samsung/s3nrn4v/swreg.bin");
+		/*
+		 * Keep going even if the calibration data could not be loaded:
+		 * the chip still enumerates and falls back to the values
+		 * programmed in its flash, so NFC may work anyway.
+		 */
+		if (ret < 0)
+			dev_warn(&ndev->nfc_dev->dev,
+				 "RF calibration data update failed (%d)\n", ret);
+		return 0;
+	}
+
 	if (s3fwrn5_firmware_init(info)) {
 		//skip bootloader mode
 		return 0;
@@ -152,13 +189,14 @@ static const struct nci_ops s3fwrn5_nci_ops = {
 	.open = s3fwrn5_nci_open,
 	.close = s3fwrn5_nci_close,
 	.send = s3fwrn5_nci_send,
+	.setup = s3fwrn5_nci_setup,
 	.post_setup = s3fwrn5_nci_post_setup,
 	.prop_ops = s3fwrn5_nci_prop_ops,
 	.n_prop_ops = ARRAY_SIZE(s3fwrn5_nci_prop_ops),
 };
 
 int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, struct device *pdev,
-	const struct s3fwrn5_phy_ops *phy_ops)
+	const struct s3fwrn5_phy_ops *phy_ops, enum s3fwrn5_variant variant)
 {
 	struct s3fwrn5_info *info;
 	int ret;
@@ -170,6 +208,7 @@ int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, struct device *pdev,
 	info->phy_id = phy_id;
 	info->pdev = pdev;
 	info->phy_ops = phy_ops;
+	info->variant = variant;
 	mutex_init(&info->mutex);
 
 	s3fwrn5_set_mode(info, S3FWRN5_MODE_COLD);
diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index c528c085b43e..b01b2e870fee 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -21,9 +21,76 @@ struct s3fwrn5_i2c_phy {
 	struct i2c_client *i2c_dev;
 	struct clk *clk;
 
+	/*
+	 * Optional hardware clock-request handshake. When a CLK_REQ GPIO is
+	 * wired, the chip drives it high while it needs its XI clock -- notably
+	 * to generate the poll/reader carrier -- and the clock is gated on it
+	 * instead of being left always-on (which never lets the chip's TX PLL
+	 * lock on a fresh clock start, leaving it unable to poll).
+	 */
+	struct gpio_desc *gpio_clk_req;
+	bool clk_on;
+	struct mutex clk_lock;	/* serialises clk_on against the CLK_REQ irq */
+
 	unsigned int irq_skip:1;
 };
 
+static void s3fwrn5_i2c_clk_set_locked(struct s3fwrn5_i2c_phy *phy, bool on)
+{
+	lockdep_assert_held(&phy->clk_lock);
+
+	if (on && !phy->clk_on) {
+		int ret = clk_prepare_enable(phy->clk);
+
+		if (ret == 0)
+			phy->clk_on = true;
+		else
+			dev_warn_once(&phy->i2c_dev->dev,
+				      "failed to enable clock (%d); NFC may not poll\n",
+				      ret);
+	} else if (!on && phy->clk_on) {
+		clk_disable_unprepare(phy->clk);
+		phy->clk_on = false;
+	}
+}
+
+/*
+ * Apply the current CLK_REQ level. Reading the GPIO under clk_lock makes
+ * concurrent callers (the CLK_REQ irq thread and the probe-time seeding)
+ * safe: whoever runs last applies a level read after the earlier update,
+ * never a stale one.
+ */
+static void s3fwrn5_i2c_clk_sync(struct s3fwrn5_i2c_phy *phy)
+{
+	int level;
+
+	mutex_lock(&phy->clk_lock);
+	level = gpiod_get_value_cansleep(phy->gpio_clk_req);
+	if (level >= 0)
+		s3fwrn5_i2c_clk_set_locked(phy, level > 0);
+	else
+		dev_warn_once(&phy->i2c_dev->dev,
+			      "failed to read CLK_REQ (%d); keeping clock state\n",
+			      level);
+	mutex_unlock(&phy->clk_lock);
+}
+
+static void s3fwrn5_i2c_clk_disable_action(void *data)
+{
+	struct s3fwrn5_i2c_phy *phy = data;
+
+	mutex_lock(&phy->clk_lock);
+	s3fwrn5_i2c_clk_set_locked(phy, false);
+	mutex_unlock(&phy->clk_lock);
+}
+
+static irqreturn_t s3fwrn5_i2c_clk_req_thread(int irq, void *phy_id)
+{
+	s3fwrn5_i2c_clk_sync(phy_id);
+
+	return IRQ_HANDLED;
+}
+
 static void s3fwrn5_i2c_set_mode(void *phy_id, enum s3fwrn5_mode mode)
 {
 	struct s3fwrn5_i2c_phy *phy = phy_id;
@@ -144,6 +211,7 @@ static irqreturn_t s3fwrn5_i2c_irq_thread_fn(int irq, void *phy_id)
 
 static int s3fwrn5_i2c_probe(struct i2c_client *client)
 {
+	enum s3fwrn5_variant variant;
 	struct s3fwrn5_i2c_phy *phy;
 	int ret;
 
@@ -170,15 +238,61 @@ static int s3fwrn5_i2c_probe(struct i2c_client *client)
 	 * S3FWRN5 depends on a clock input ("XI" pin) to function properly.
 	 * Depending on the hardware configuration this could be an always-on
 	 * oscillator or some external clock that must be explicitly enabled.
-	 * Make sure the clock is running before starting S3FWRN5.
+	 *
+	 * If a CLK_REQ GPIO is wired, the chip gates the clock itself (driving
+	 * CLK_REQ high when it needs XI); service that handshake. Otherwise just
+	 * make sure the clock is running before starting S3FWRN5.
 	 */
-	phy->clk = devm_clk_get_optional_enabled(&client->dev, NULL);
-	if (IS_ERR(phy->clk))
-		return dev_err_probe(&client->dev, PTR_ERR(phy->clk),
-				     "failed to get clock\n");
+	mutex_init(&phy->clk_lock);
+	phy->gpio_clk_req = devm_gpiod_get_optional(&client->dev, "clk-req",
+						    GPIOD_IN);
+	if (IS_ERR(phy->gpio_clk_req))
+		return PTR_ERR(phy->gpio_clk_req);
+
+	if (phy->gpio_clk_req) {
+		int clk_req_irq;
+
+		phy->clk = devm_clk_get_optional(&client->dev, NULL);
+		if (IS_ERR(phy->clk))
+			return dev_err_probe(&client->dev, PTR_ERR(phy->clk),
+					     "failed to get clock\n");
+
+		/*
+		 * Unlike the always-on branch below, this clock is enabled by
+		 * hand from the CLK_REQ handler, so devm will not disable it on
+		 * unbind. Gate it off explicitly if it is still on at teardown.
+		 */
+		ret = devm_add_action_or_reset(&client->dev,
+					       s3fwrn5_i2c_clk_disable_action,
+					       phy);
+		if (ret)
+			return ret;
+
+		clk_req_irq = gpiod_to_irq(phy->gpio_clk_req);
+		if (clk_req_irq < 0)
+			return clk_req_irq;
+
+		ret = devm_request_threaded_irq(&client->dev, clk_req_irq, NULL,
+						s3fwrn5_i2c_clk_req_thread,
+						IRQF_TRIGGER_RISING |
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT,
+						"s3fwrn5_clk_req", phy);
+		if (ret)
+			return ret;
+
+		/* Seed the clock state from the current CLK_REQ level. */
+		s3fwrn5_i2c_clk_sync(phy);
+	} else {
+		phy->clk = devm_clk_get_optional_enabled(&client->dev, NULL);
+		if (IS_ERR(phy->clk))
+			return dev_err_probe(&client->dev, PTR_ERR(phy->clk),
+					     "failed to get clock\n");
+	}
 
+	variant = (uintptr_t)i2c_get_match_data(client);
 	ret = s3fwrn5_probe(&phy->common.ndev, phy, &phy->i2c_dev->dev,
-			    &i2c_phy_ops);
+			    &i2c_phy_ops, variant);
 	if (ret < 0)
 		return ret;
 
@@ -203,13 +317,17 @@ static void s3fwrn5_i2c_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id s3fwrn5_i2c_id_table[] = {
-	{ .name = "s3fwrn5_i2c" },
+	{ .name = "s3fwrn5_i2c", .driver_data = S3FWRN5_VARIANT_FWDL },
+	{ .name = "s3nrn4v", .driver_data = S3FWRN5_VARIANT_S3NRN4V },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, s3fwrn5_i2c_id_table);
 
 static const struct of_device_id of_s3fwrn5_i2c_match[] = {
-	{ .compatible = "samsung,s3fwrn5-i2c" },
+	{ .compatible = "samsung,s3fwrn5-i2c",
+	  .data = (void *)S3FWRN5_VARIANT_FWDL },
+	{ .compatible = "samsung,s3nrn4v",
+	  .data = (void *)S3FWRN5_VARIANT_S3NRN4V },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, of_s3fwrn5_i2c_match);
diff --git a/drivers/nfc/s3fwrn5/nci.c b/drivers/nfc/s3fwrn5/nci.c
index 5a9de11bbece..46cfd6557a14 100644
--- a/drivers/nfc/s3fwrn5/nci.c
+++ b/drivers/nfc/s3fwrn5/nci.c
@@ -8,6 +8,9 @@
 
 #include <linux/completion.h>
 #include <linux/firmware.h>
+#include <linux/minmax.h>
+#include <linux/slab.h>
+#include <linux/unaligned.h>
 
 #include "s3fwrn5.h"
 #include "nci.h"
@@ -20,7 +23,7 @@ static int s3fwrn5_nci_prop_rsp(struct nci_dev *ndev, struct sk_buff *skb)
 	return 0;
 }
 
-const struct nci_driver_ops s3fwrn5_nci_prop_ops[4] = {
+const struct nci_driver_ops s3fwrn5_nci_prop_ops[5] = {
 	{
 		.opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
 				NCI_PROP_SET_RFREG),
@@ -41,6 +44,11 @@ const struct nci_driver_ops s3fwrn5_nci_prop_ops[4] = {
 				NCI_PROP_FW_CFG),
 		.rsp = s3fwrn5_nci_prop_rsp,
 	},
+	{
+		.opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
+				NCI_PROP_DUAL_OPTION),
+		.rsp = s3fwrn5_nci_prop_rsp,
+	},
 };
 
 #define S3FWRN5_RFREG_SECTION_SIZE 252
@@ -117,3 +125,114 @@ int s3fwrn5_nci_rf_configure(struct s3fwrn5_info *info, const char *fw_name)
 	release_firmware(fw);
 	return ret;
 }
+
+/*
+ * Configure the reference clock. The S3NRN4V expects the single-byte FW_CFG
+ * form (just the clock-speed selector). The downstream stack sends this in the
+ * bootloader before CORE_RESET; the earliest the mainline NCI core lets us in
+ * is the ->setup hook (after CORE_RESET, before CORE_INIT), which works.
+ */
+int s3fwrn5_nci_clk_cfg(struct s3fwrn5_info *info)
+{
+	u8 clk_speed = NCI_PROP_FW_CFG_CLK_SPEED;
+
+	return nci_prop_cmd(info->ndev, NCI_PROP_FW_CFG, 1, &clk_speed);
+}
+
+/*
+ * S3NRN4V RF calibration data update. The HW and SW calibration blobs are
+ * merged into a single stream (HW first) and pushed via the DUAL_OPTION
+ * command: START_UPDATE, one SET_OPTION per 252-byte section, then STOP_UPDATE
+ * carrying a 16-bit checksum (running sum of the merged stream as 32-bit
+ * words).
+ */
+int s3fwrn5_nci_rf_configure_dual(struct s3fwrn5_info *info,
+				  const char *hw_name, const char *sw_name)
+{
+	const struct firmware *hw_fw = NULL, *sw_fw = NULL;
+	struct nci_prop_dual_set_option_cmd set_option;
+	struct device *dev = &info->ndev->nfc_dev->dev;
+	size_t merged_size, i, len;
+	u8 *merged = NULL;
+	u8 stop_cmd[3];
+	u32 checksum;
+	u8 sub_oid;
+	int ret;
+
+	ret = firmware_request_nowarn(&hw_fw, hw_name, dev);
+	if (ret < 0)
+		return ret;
+	ret = firmware_request_nowarn(&sw_fw, sw_name, dev);
+	if (ret < 0)
+		goto out_hw;
+
+	merged_size = hw_fw->size + sw_fw->size;
+
+	/*
+	 * The stream is checksummed as 32-bit words and pushed in at most 256
+	 * sections (the section index is a single byte); reject blobs that
+	 * would silently break either.
+	 */
+	if (merged_size % 4 ||
+	    merged_size > 256 * NCI_PROP_DUAL_SECTION_SIZE) {
+		dev_err(dev, "invalid calibration data size (%zu)\n", merged_size);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	merged = kmalloc(merged_size, GFP_KERNEL);
+	if (!merged) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	memcpy(merged, hw_fw->data, hw_fw->size);
+	memcpy(merged + hw_fw->size, sw_fw->data, sw_fw->size);
+
+	/* Running sum of the merged stream as little-endian 32-bit words. */
+	checksum = 0;
+	for (i = 0; i + 4 <= merged_size; i += 4)
+		checksum += get_unaligned_le32(merged + i);
+
+	dev_dbg(dev, "calibration data update: %s + %s\n", hw_name, sw_name);
+
+	/* START_UPDATE */
+	sub_oid = NCI_PROP_DUAL_SUB_START_UPDATE;
+	ret = nci_prop_cmd(info->ndev, NCI_PROP_DUAL_OPTION, 1, &sub_oid);
+	if (ret < 0) {
+		dev_err(dev, "Unable to start calibration data update\n");
+		goto out;
+	}
+
+	/* SET_OPTION per section */
+	set_option.sub_oid = NCI_PROP_DUAL_SUB_SET_OPTION;
+	set_option.index = 0;
+	for (i = 0; i < merged_size; i += NCI_PROP_DUAL_SECTION_SIZE) {
+		len = min_t(size_t, merged_size - i, NCI_PROP_DUAL_SECTION_SIZE);
+		memcpy(set_option.data, merged + i, len);
+		ret = nci_prop_cmd(info->ndev, NCI_PROP_DUAL_OPTION,
+				   len + 2, (__u8 *)&set_option);
+		if (ret < 0) {
+			dev_err(dev, "calibration data update error (code=%d)\n",
+				ret);
+			goto out;
+		}
+		set_option.index++;
+	}
+
+	/* STOP_UPDATE with checksum */
+	stop_cmd[0] = NCI_PROP_DUAL_SUB_STOP_UPDATE;
+	put_unaligned_le16(checksum, &stop_cmd[1]);
+	ret = nci_prop_cmd(info->ndev, NCI_PROP_DUAL_OPTION, 3, stop_cmd);
+	if (ret < 0) {
+		dev_err(dev, "Unable to stop calibration data update\n");
+		goto out;
+	}
+
+	dev_dbg(dev, "calibration data update: success\n");
+out:
+	kfree(merged);
+	release_firmware(sw_fw);
+out_hw:
+	release_firmware(hw_fw);
+	return ret;
+}
diff --git a/drivers/nfc/s3fwrn5/nci.h b/drivers/nfc/s3fwrn5/nci.h
index bc4bce2bbc4d..e1c0955bbf7e 100644
--- a/drivers/nfc/s3fwrn5/nci.h
+++ b/drivers/nfc/s3fwrn5/nci.h
@@ -40,6 +40,13 @@ struct nci_prop_stop_rfreg_rsp {
 
 #define NCI_PROP_FW_CFG		0x28
 
+/*
+ * Single-byte FW_CFG payload (clock-speed selector) for the S3NRN4V reference
+ * clock. Taken from the vendor configuration for this part (the encoding is
+ * not documented).
+ */
+#define NCI_PROP_FW_CFG_CLK_SPEED	0x11
+
 struct nci_prop_fw_cfg_cmd {
 	__u8 clk_type;
 	__u8 clk_speed;
@@ -50,7 +57,30 @@ struct nci_prop_fw_cfg_rsp {
 	__u8 status;
 };
 
-extern const struct nci_driver_ops s3fwrn5_nci_prop_ops[4];
+/*
+ * The S3NRN4V loads its RF calibration data through a single "dual option"
+ * command (a sub-OID selects the operation) instead of the START/SET/STOP_RFREG
+ * opcodes above, and expects the HW and SW calibration blobs merged into one
+ * stream.
+ */
+#define NCI_PROP_DUAL_OPTION		0x2a
+
+#define NCI_PROP_DUAL_SUB_START_UPDATE	0x01
+#define NCI_PROP_DUAL_SUB_SET_OPTION	0x02
+#define NCI_PROP_DUAL_SUB_STOP_UPDATE	0x03
+
+#define NCI_PROP_DUAL_SECTION_SIZE	252
+
+struct nci_prop_dual_set_option_cmd {
+	__u8 sub_oid;	/* NCI_PROP_DUAL_SUB_SET_OPTION */
+	__u8 index;
+	__u8 data[NCI_PROP_DUAL_SECTION_SIZE];
+};
+
+extern const struct nci_driver_ops s3fwrn5_nci_prop_ops[5];
 int s3fwrn5_nci_rf_configure(struct s3fwrn5_info *info, const char *fw_name);
+int s3fwrn5_nci_rf_configure_dual(struct s3fwrn5_info *info,
+				  const char *hw_name, const char *sw_name);
+int s3fwrn5_nci_clk_cfg(struct s3fwrn5_info *info);
 
 #endif /* __LOCAL_S3FWRN5_NCI_H_ */
diff --git a/drivers/nfc/s3fwrn5/s3fwrn5.h b/drivers/nfc/s3fwrn5/s3fwrn5.h
index 2b492236090b..2a78c5ef0606 100644
--- a/drivers/nfc/s3fwrn5/s3fwrn5.h
+++ b/drivers/nfc/s3fwrn5/s3fwrn5.h
@@ -21,6 +21,18 @@ enum s3fwrn5_mode {
 	S3FWRN5_MODE_FW,
 };
 
+enum s3fwrn5_variant {
+	/* S3FWRN5 / S3FWRN82: firmware is downloaded by this driver */
+	S3FWRN5_VARIANT_FWDL,
+	/*
+	 * S3NRN4V: ships with working firmware behind a bootloader protocol
+	 * this driver does not implement; skip the download, configure the
+	 * clock (FW_CFG) and load the RF calibration data via the DUAL_OPTION
+	 * cmd.
+	 */
+	S3FWRN5_VARIANT_S3NRN4V,
+};
+
 struct s3fwrn5_phy_ops {
 	void (*set_wake)(void *id, bool sleep);
 	void (*set_mode)(void *id, enum s3fwrn5_mode);
@@ -36,6 +48,7 @@ struct s3fwrn5_info {
 	const struct s3fwrn5_phy_ops *phy_ops;
 
 	struct s3fwrn5_fw_info fw_info;
+	enum s3fwrn5_variant variant;
 
 	struct mutex mutex;
 };
@@ -78,7 +91,7 @@ static inline int s3fwrn5_write(struct s3fwrn5_info *info, struct sk_buff *skb)
 }
 
 int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, struct device *pdev,
-	const struct s3fwrn5_phy_ops *phy_ops);
+	const struct s3fwrn5_phy_ops *phy_ops, enum s3fwrn5_variant variant);
 void s3fwrn5_remove(struct nci_dev *ndev);
 
 int s3fwrn5_recv_frame(struct nci_dev *ndev, struct sk_buff *skb,
diff --git a/drivers/nfc/s3fwrn5/uart.c b/drivers/nfc/s3fwrn5/uart.c
index 8f142a255101..75397c3dbc58 100644
--- a/drivers/nfc/s3fwrn5/uart.c
+++ b/drivers/nfc/s3fwrn5/uart.c
@@ -136,7 +136,7 @@ static int s3fwrn82_uart_probe(struct serdev_device *serdev)
 	}
 
 	ret = s3fwrn5_probe(&phy->common.ndev, phy, &phy->ser_dev->dev,
-			    &uart_phy_ops);
+			    &uart_phy_ops, S3FWRN5_VARIANT_FWDL);
 	if (ret < 0)
 		goto err_serdev;
 
-- 
2.55.0

[PATCH v3 5/5] arm64: dts: qcom: milos-fairphone-fp6: Add NFC

[Jorijn van der Graaf]

Add the Samsung S3NRN4V NCI NFC controller on i2c1. Its XI clock is
provided by the RF_CLK2 PMIC buffer and gated through the controller's
CLK_REQ line on tlmm GPIO6. PVDD is fed from PM7550 LDO20, the rail the
downstream driver switches for this chip.

The enable line is routed to the chip's power-down input, which is
asserted high to turn the chip off (the downstream driver treats VEN as
active-low on this design) -- hence GPIO_ACTIVE_HIGH, unlike the
exynos5433-tm2 wiring of the same driver. The pin is pulled up so the
chip stays off while the line is not driven.

Assisted-by: Claude:claude-opus-4-8
Assisted-by: Claude:claude-fable-5
Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
---
 .../boot/dts/qcom/milos-fairphone-fp6.dts     | 49 ++++++++++++++++++-
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts b/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
index ebdca0e2d2f3..a1b86e4092fe 100644
--- a/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
+++ b/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
@@ -601,9 +601,29 @@ &gcc {
 };
 
 &i2c1 {
-	/* Samsung NFC @ 0x27 */
-
 	status = "okay";
+
+	/*
+	 * Samsung S3NRN4V NFC controller. XI is driven by the RF_CLK2 PMIC
+	 * buffer; the chip has no oscillator of its own and gates the clock
+	 * via its CLK_REQ line, so the clock must be voted on in response
+	 * to it.
+	 */
+	nfc@27 {
+		compatible = "samsung,s3nrn4v";
+		reg = <0x27>;
+
+		clk-req-gpios = <&tlmm 6 GPIO_ACTIVE_HIGH>;
+		clocks = <&rpmhcc RPMH_RF_CLK2>;
+		en-gpios = <&tlmm 56 GPIO_ACTIVE_HIGH>;
+		interrupt-parent = <&tlmm>;
+		interrupts = <31 IRQ_TYPE_EDGE_RISING>;
+		pinctrl-0 = <&nfc_clk_req_default>, <&nfc_irq_default>,
+			    <&nfc_pd_default>;
+		pinctrl-names = "default";
+		pvdd-supply = <&vreg_l20b>;
+		wake-gpios = <&tlmm 7 GPIO_ACTIVE_HIGH>;
+	};
 };
 
 &i2c3 {
@@ -825,6 +845,31 @@ &tlmm {
 			       <13 1>, /* NC */
 			       <63 2>; /* WLAN UART */
 
+	nfc_clk_req_default: nfc-clk-req-default-state {
+		pins = "gpio6";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	nfc_irq_default: nfc-irq-default-state {
+		pins = "gpio31";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	/*
+	 * Pulled up so the NFC chip stays powered down while the line is
+	 * not driven (it is the chip's active-high power-down input).
+	 */
+	nfc_pd_default: nfc-pd-default-state {
+		pins = "gpio56";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+
 	sdc2_card_det_n: sdc2-card-det-state {
 		pins = "gpio65";
 		function = "gpio";
-- 
2.55.0

Re: [PATCH v3 5/5] arm64: dts: qcom: milos-fairphone-fp6: Add NFC

[Konrad Dybcio]

On 7/27/26 12:24 AM, Jorijn van der Graaf wrote:
> Add the Samsung S3NRN4V NCI NFC controller on i2c1. Its XI clock is
> provided by the RF_CLK2 PMIC buffer and gated through the controller's
> CLK_REQ line on tlmm GPIO6. PVDD is fed from PM7550 LDO20, the rail the
> downstream driver switches for this chip.
> 
> The enable line is routed to the chip's power-down input, which is
> asserted high to turn the chip off (the downstream driver treats VEN as
> active-low on this design) -- hence GPIO_ACTIVE_HIGH, unlike the
> exynos5433-tm2 wiring of the same driver. The pin is pulled up so the
> chip stays off while the line is not driven.
> 
> Assisted-by: Claude:claude-opus-4-8
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
> ---
>  .../boot/dts/qcom/milos-fairphone-fp6.dts     | 49 ++++++++++++++++++-
>  1 file changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts b/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
> index ebdca0e2d2f3..a1b86e4092fe 100644
> --- a/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
> +++ b/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
> @@ -601,9 +601,29 @@ &gcc {
>  };
>  
>  &i2c1 {
> -	/* Samsung NFC @ 0x27 */
> -
>  	status = "okay";
> +
> +	/*
> +	 * Samsung S3NRN4V NFC controller. XI is driven by the RF_CLK2 PMIC
> +	 * buffer; the chip has no oscillator of its own and gates the clock
> +	 * via its CLK_REQ line, so the clock must be voted on in response
> +	 * to it.
> +	 */

This seems like something that's more fitting of the commit message
and/or dt-bindings (either description: or in a YAML comment)
> +	nfc@27 {
> +		compatible = "samsung,s3nrn4v";
> +		reg = <0x27>;
> +
> +		clk-req-gpios = <&tlmm 6 GPIO_ACTIVE_HIGH>;
> +		clocks = <&rpmhcc RPMH_RF_CLK2>;
> +		en-gpios = <&tlmm 56 GPIO_ACTIVE_HIGH>;
> +		interrupt-parent = <&tlmm>;
> +		interrupts = <31 IRQ_TYPE_EDGE_RISING>;

interrupts-extended = <&tlmm 31 IRQ..>

> +		pinctrl-0 = <&nfc_clk_req_default>, <&nfc_irq_default>,
> +			    <&nfc_pd_default>;

You can create a group of pins (see below

> +		pinctrl-names = "default";
> +		pvdd-supply = <&vreg_l20b>;
> +		wake-gpios = <&tlmm 7 GPIO_ACTIVE_HIGH>;

Please group the -gpios properties together

> +	};
>  };
>  
>  &i2c3 {
> @@ -825,6 +845,31 @@ &tlmm {
>  			       <13 1>, /* NC */
>  			       <63 2>; /* WLAN UART */
>  
> +	nfc_clk_req_default: nfc-clk-req-default-state {
> +		pins = "gpio6";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +	};
> +
> +	nfc_irq_default: nfc-irq-default-state {
> +		pins = "gpio31";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +	};
> +
> +	/*
> +	 * Pulled up so the NFC chip stays powered down while the line is
> +	 * not driven (it is the chip's active-high power-down input).
> +	 */
> +	nfc_pd_default: nfc-pd-default-state {
> +		pins = "gpio56";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-pull-up;
> +	};

You can group them into something like:

nfc_default_state: nfc-default-state {
	nfc-clk-req-default-pins {
		gpios = ""
		...
	};

	...
};

Konrad

Re: [PATCH v3 5/5] arm64: dts: qcom: milos-fairphone-fp6: Add NFC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The NFC controller will remain unpowered because the `s3fwrn5` driver lacks regulator support, so the `pvdd-supply` specified in the device tree will never be enabled.
--

--- Patch [5]: [PATCH v3 5/5] arm64: dts: qcom: milos-fairphone-fp6: Add NFC ---
commit a37bde53492f31e7077b36b4e030e872e07827b0
Author: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>

arm64: dts: qcom: milos-fairphone-fp6: Add NFC

This commit adds the Samsung S3NRN4V NCI NFC controller on i2c1 to the
device tree. It configures the necessary clocks, interrupts, and assigns
the PVDD supply to the PM7550 LDO20 regulator.

> diff --git a/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts b/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
> index ebdca0e2d2f3a..a1b86e4092fe2 100644
> --- a/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
> +++ b/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
> @@ -601,9 +601,29 @@ &gcc {
[ ... ]
> +	nfc@27 {
> +		compatible = "samsung,s3nrn4v";
> +		reg = <0x27>;
> +
> +		clk-req-gpios = <&tlmm 6 GPIO_ACTIVE_HIGH>;
> +		clocks = <&rpmhcc RPMH_RF_CLK2>;
> +		en-gpios = <&tlmm 56 GPIO_ACTIVE_HIGH>;
> +		interrupt-parent = <&tlmm>;
> +		interrupts = <31 IRQ_TYPE_EDGE_RISING>;
> +		pinctrl-0 = <&nfc_clk_req_default>, <&nfc_irq_default>,
> +			    <&nfc_pd_default>;
> +		pinctrl-names = "default";
> +		pvdd-supply = <&vreg_l20b>;

[Severity: High]
Will the NFC controller remain unpowered here?

Looking at the upstream driver in drivers/nfc/s3fwrn5/i2c.c in
s3fwrn5_i2c_probe(), it parses and initializes GPIOs and clocks:

    phy->common.gpio_en = devm_gpiod_get(&client->dev, "en", GPIOD_OUT_HIGH);

However, it appears to omit requesting or enabling any regulators like
pvdd-supply. Since vreg_l20b is not marked as always-on, won't the regulator
core leave it disabled, preventing the chip from functioning?

> +		wake-gpios = <&tlmm 7 GPIO_ACTIVE_HIGH>;
> +	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726222405.153008-1-jorijnvdgraaf@catcrafts.net?part=5