Re: [PATCH v4 6/6] arm64: dts: qcom: milos-fairphone-fp6: Add NFC

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The 'wake-gpios' pin (gpio7) is entirely omitted from the pinctrl configuration.
--

--- Patch [6]: [PATCH v4 6/6] arm64: dts: qcom: milos-fairphone-fp6: Add NFC ---
commit 971202c103843632f71a5371f28acebc71b278d0
Author: Jorijn van der Graaf <[email protected]>

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

This commit adds the Samsung S3NRN4V NCI NFC controller on i2c1 for the
Fairphone 6. It configures the clock, power supply, and relevant GPIO pins
for interrupts, power-down, and wake functionality.

> diff --git a/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts b/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
> index ebdca0e2d2f3a..bdba848275db4 100644
> --- a/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
> +++ b/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
> @@ -601,9 +601,22 @@ &gcc {
>  };
>  
>  &i2c1 {
> -	/* Samsung NFC @ 0x27 */
> -
>  	status = "okay";
> +
> +	nfc@27 {
> +		compatible = "samsung,s3nrn4v";
> +		reg = <0x27>;
> +
> +		interrupts-extended = <&tlmm 31 IRQ_TYPE_EDGE_RISING>;
> +		en-gpios = <&tlmm 56 GPIO_ACTIVE_HIGH>;
> +		wake-gpios = <&tlmm 7 GPIO_ACTIVE_HIGH>;
> +		clocks = <&rpmhcc RPMH_LN_BB_CLK2>;
> +
> +		pvdd-supply = <&vreg_l20b>;
> +
> +		pinctrl-0 = <&nfc_default>;
> +		pinctrl-names = "default";
> +	};
>  };

[ ... ]

> @@ -825,6 +838,22 @@ &tlmm {
>  			       <13 1>, /* NC */
>  			       <63 2>; /* WLAN UART */
>  
> +	nfc_default: nfc-default-state {
> +		irq-pins {
> +			pins = "gpio31";
> +			function = "gpio";
> +			drive-strength = <2>;
> +			bias-disable;
> +		};
> +
> +		pd-pins {
> +			pins = "gpio56";
> +			function = "gpio";
> +			drive-strength = <2>;
> +			bias-pull-up;
> +		};
> +	};

[Severity: Medium]
The nfc@27 node defines wake-gpios on gpio7, but is this pin missing from
the pinctrl configuration?

Without an explicit pinctrl configuration for gpio7 in nfc_default, the
wake pin might be left in its bootloader or SoC default state.

Should there be a third pin configuration block here setting the function,
drive strength, and bias for gpio7 to ensure the wake line operates correctly?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6