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

[Luca Weiss]

Hi Jorijn,

On Mon Jul 27, 2026 at 12:24 AM CEST, 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@xxxxxxxxxxxxx>
> ---
>  .../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.
> +	 */

I don't think this comment deserves to be in the dtsi, having these
details in the commit message is enough imo.

> +	nfc@27 {
> +		compatible = "samsung,s3nrn4v";
> +		reg = <0x27>;
> +
> +		clk-req-gpios = <&tlmm 6 GPIO_ACTIVE_HIGH>;
> +		clocks = <&rpmhcc RPMH_RF_CLK2>;

I'm not sure about these, the NFC chip is connected to PMK7635 CLK7_LN
which is part of the LNBB clocks, CLK1_RF - CLK5_RF are the RF clks,
CLK6_LN - CLK9_LN are the LNBB clocks so shouldn't this be <&rpmhcc
RPMH_LN_BB_CLK2>? But I honestly couldn't find anything in the
downstream dts so maybe either these clocks are always on anyways or
something else is going on there?

Regards
Luca

> +		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";