[PATCH v2 2/2] arm64: dts: qcom: sdm845-shift-axolotl: Enable NFC

David Heidelberg via B4 Relay posted 2 patches 1 week, 5 days ago
[PATCH v2 2/2] arm64: dts: qcom: sdm845-shift-axolotl: Enable NFC
Posted by David Heidelberg via B4 Relay 1 week, 5 days ago
From: David Heidelberg <david@ixit.cz>

Definition of the NFC.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts | 33 +++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts b/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts
index 51b041f91d3e2..eea63aa170b16 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts
@@ -431,6 +431,25 @@ &gpu_zap_shader {
 	firmware-name = "qcom/sdm845/SHIFT/axolotl/a630_zap.mbn";
 };
 
+&i2c3 {
+	clock-frequency = <400000>;
+
+	status = "okay";
+
+	nfc@28 {
+		compatible = "nxp,pn553", "nxp,nxp-nci-i2c";
+		reg = <0x28>;
+
+		interrupts-extended = <&tlmm 63 IRQ_TYPE_EDGE_RISING>;
+
+		enable-gpios = <&tlmm 12 GPIO_ACTIVE_HIGH>;
+		firmware-gpios = <&tlmm 62 GPIO_ACTIVE_HIGH>;
+
+		pinctrl-0 = <&nfc_int_default &nfc_enable_default>;
+		pinctrl-names = "default";
+	};
+};
+
 &i2c5 {
 	status = "okay";
 
@@ -609,6 +628,20 @@ &slpi_pas {
 &tlmm {
 	gpio-reserved-ranges = <0 4>, <81 4>;
 
+	nfc_int_default: nfc-int-default-state {
+		pins = "gpio63";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+
+	nfc_enable_default: nfc-enable-default-state {
+		pins = "gpio12", "gpio62";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+
 	sde_dsi_active: sde-dsi-active-state {
 		pins = "gpio6", "gpio11";
 		function = "gpio";

-- 
2.53.0
Re: [PATCH v2 2/2] arm64: dts: qcom: sdm845-shift-axolotl: Enable NFC
Posted by Konrad Dybcio 1 week, 5 days ago
On 3/24/26 12:20 AM, David Heidelberg via B4 Relay wrote:
> From: David Heidelberg <david@ixit.cz>
> 
> Definition of the NFC.

"meh" commit message

[...]

> +	nfc_enable_default: nfc-enable-default-state {
> +		pins = "gpio12", "gpio62";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-pull-up;

Are you sure about pulling up an active-high pin?

FWIW TLMM subnodes are best sorted by pin index (although the file
currently doesn't really do that) as per dts coding style

Konrad
Re: [PATCH v2 2/2] arm64: dts: qcom: sdm845-shift-axolotl: Enable NFC
Posted by David Heidelberg 1 week, 5 days ago

On 24/03/2026 14:12, Konrad Dybcio wrote:
> On 3/24/26 12:20 AM, David Heidelberg via B4 Relay wrote:
>> From: David Heidelberg <david@ixit.cz>
>>
>> Definition of the NFC.
> 
> "meh" commit message
> 
> [...]
> 
>> +	nfc_enable_default: nfc-enable-default-state {
>> +		pins = "gpio12", "gpio62";
>> +		function = "gpio";
>> +		drive-strength = <2>;
>> +		bias-pull-up;
> 
> Are you sure about pulling up an active-high pin?

I'm not sure, but downstream does it (and "works for me"). Maybe Alexander would 
know more details here.

David

> 
> FWIW TLMM subnodes are best sorted by pin index (although the file
> currently doesn't really do that) as per dts coding style

I assume when I group the -pins into -state it doesn't apply anymore? As I don't 
feel having pins relevant to one device / subsystem all over the place is extra 
clean.

David

P.S. before v3 code is here
https://codeberg.org/sdm845/linux/commits/branch/b4/oneplus-nfc

> 
> Konrad

-- 
David Heidelberg
Re: [PATCH v2 2/2] arm64: dts: qcom: sdm845-shift-axolotl: Enable NFC
Posted by Konrad Dybcio 1 week, 4 days ago
On 3/24/26 7:08 PM, David Heidelberg wrote:
> 
> 
> On 24/03/2026 14:12, Konrad Dybcio wrote:
>> On 3/24/26 12:20 AM, David Heidelberg via B4 Relay wrote:
>>> From: David Heidelberg <david@ixit.cz>
>>>
>>> Definition of the NFC.
>>
>> "meh" commit message
>>
>> [...]
>>
>>> +    nfc_enable_default: nfc-enable-default-state {
>>> +        pins = "gpio12", "gpio62";
>>> +        function = "gpio";
>>> +        drive-strength = <2>;
>>> +        bias-pull-up;
>>
>> Are you sure about pulling up an active-high pin?
> 
> I'm not sure, but downstream does it (and "works for me"). Maybe Alexander would know more details here.

Would changing it to bias-disable also "work for you"?

> 
> David
> 
>>
>> FWIW TLMM subnodes are best sorted by pin index (although the file
>> currently doesn't really do that) as per dts coding style
> 
> I assume when I group the -pins into -state it doesn't apply anymore? As I don't feel having pins relevant to one device / subsystem all over the place is extra clean.

Krzysztof?

Konrad
Re: [PATCH v2 2/2] arm64: dts: qcom: sdm845-shift-axolotl: Enable NFC
Posted by David Heidelberg 1 week, 3 days ago
On 25/03/2026 12:20, Konrad Dybcio wrote:
> On 3/24/26 7:08 PM, David Heidelberg wrote:
>>
>>
>> On 24/03/2026 14:12, Konrad Dybcio wrote:
>>> On 3/24/26 12:20 AM, David Heidelberg via B4 Relay wrote:
>>>> From: David Heidelberg <david@ixit.cz>
>>>>
>>>> Definition of the NFC.
>>>
>>> "meh" commit message
>>>
>>> [...]
>>>
>>>> +    nfc_enable_default: nfc-enable-default-state {
>>>> +        pins = "gpio12", "gpio62";
>>>> +        function = "gpio";
>>>> +        drive-strength = <2>;
>>>> +        bias-pull-up;

               bias-disable;
>>>
>>> Are you sure about pulling up an active-high pin?
>>
>> I'm not sure, but downstream does it (and "works for me"). Maybe Alexander would know more details here.
> 
> Would changing it to bias-disable also "work for you"?

Yeah, works for me. Should OnePlus 6 do the same?

Looking at OP6 datasheet, there is no pull-up/down on 12,62,nor IRQ 63.

David

> 
>>
>> David
>>
>>>
>>> FWIW TLMM subnodes are best sorted by pin index (although the file
>>> currently doesn't really do that) as per dts coding style
>>
>> I assume when I group the -pins into -state it doesn't apply anymore? As I don't feel having pins relevant to one device / subsystem all over the place is extra clean.
> 
> Krzysztof?
> 
> Konrad

-- 
David Heidelberg

Re: [PATCH v2 2/2] arm64: dts: qcom: sdm845-shift-axolotl: Enable NFC
Posted by Konrad Dybcio 1 week, 3 days ago
On 3/25/26 9:17 PM, David Heidelberg wrote:
> On 25/03/2026 12:20, Konrad Dybcio wrote:
>> On 3/24/26 7:08 PM, David Heidelberg wrote:
>>>
>>>
>>> On 24/03/2026 14:12, Konrad Dybcio wrote:
>>>> On 3/24/26 12:20 AM, David Heidelberg via B4 Relay wrote:
>>>>> From: David Heidelberg <david@ixit.cz>
>>>>>
>>>>> Definition of the NFC.
>>>>
>>>> "meh" commit message
>>>>
>>>> [...]
>>>>
>>>>> +    nfc_enable_default: nfc-enable-default-state {
>>>>> +        pins = "gpio12", "gpio62";
>>>>> +        function = "gpio";
>>>>> +        drive-strength = <2>;
>>>>> +        bias-pull-up;
> 
>               bias-disable;
>>>>
>>>> Are you sure about pulling up an active-high pin?
>>>
>>> I'm not sure, but downstream does it (and "works for me"). Maybe Alexander would know more details here.
>>
>> Would changing it to bias-disable also "work for you"?
> 
> Yeah, works for me. Should OnePlus 6 do the same?
> 
> Looking at OP6 datasheet, there is no pull-up/down on 12,62,nor IRQ 63.

Generally the internal bias would be used to counteract random noise posing
as signal, to ensure the line is kept in the "inactive" state when not
actively driven

I can see that the driver initially requests both to LOW and then sets it to
high based on the desired mode in nxp_nci_i2c_set_mode(), so pulling up is
perhaps never really desired.

Konrad
Re: [PATCH v2 2/2] arm64: dts: qcom: sdm845-shift-axolotl: Enable NFC
Posted by Krzysztof Kozlowski 1 week, 4 days ago
On 25/03/2026 12:20, Konrad Dybcio wrote:
>>
>>>
>>> FWIW TLMM subnodes are best sorted by pin index (although the file
>>> currently doesn't really do that) as per dts coding style
>>
>> I assume when I group the -pins into -state it doesn't apply anymore? As I don't feel having pins relevant to one device / subsystem all over the place is extra clean.
> 

	nfc_int_default: nfc-int-default-state {
		pins = "gpio63";
	};

	nfc_enable_default: nfc-enable-default-state {
		pins = "gpio12", "gpio62";
	};

 	sde_dsi_active: sde-dsi-active-state {
 		pins = "gpio6", "gpio11";
	}

Let's imagine future possible implementation of DTS coding style
linter/checkpatch. How it would sort the nodes? Either by node name or
the first value in "pins", this this would be:

 	sde_dsi_active: sde-dsi-active-state {
 		pins = "gpio6", "gpio11";
	}

	nfc_enable_default: nfc-enable-default-state {
		pins = "gpio12", "gpio62";
	};

	nfc_int_default: nfc-int-default-state {
		pins = "gpio63";
	};

So that's how you code. Less work for future linter/checkpatch.

The trouble is that "pins" property sorting can result in nodes being
spread all over, imagine:

	nfc_enable_default: nfc-enable-default-state {
		pins = "gpio5", "gpio62";
			// ^^^^^ DIFFERENCE!
	};

 	sde_dsi_active: sde-dsi-active-state {
 		pins = "gpio6", "gpio11";
	}

	nfc_int_default: nfc-int-default-state {
		pins = "gpio63";
	};

That's why I would propose to keep everything sorted by node name, but I
am fine with both choices. Qualcomm maintainers decide about such
detailed style they want to impose.

Best regards,
Krzysztof
Re: [PATCH v2 2/2] arm64: dts: qcom: sdm845-shift-axolotl: Enable NFC
Posted by Konrad Dybcio 1 week, 4 days ago
On 3/25/26 12:31 PM, Krzysztof Kozlowski wrote:
> On 25/03/2026 12:20, Konrad Dybcio wrote:
>>>
>>>>
>>>> FWIW TLMM subnodes are best sorted by pin index (although the file
>>>> currently doesn't really do that) as per dts coding style
>>>
>>> I assume when I group the -pins into -state it doesn't apply anymore? As I don't feel having pins relevant to one device / subsystem all over the place is extra clean.
>>
> 
> 	nfc_int_default: nfc-int-default-state {
> 		pins = "gpio63";
> 	};
> 
> 	nfc_enable_default: nfc-enable-default-state {
> 		pins = "gpio12", "gpio62";
> 	};
> 
>  	sde_dsi_active: sde-dsi-active-state {
>  		pins = "gpio6", "gpio11";
> 	}
> 
> Let's imagine future possible implementation of DTS coding style
> linter/checkpatch. How it would sort the nodes? Either by node name or
> the first value in "pins", this this would be:
> 
>  	sde_dsi_active: sde-dsi-active-state {
>  		pins = "gpio6", "gpio11";
> 	}
> 
> 	nfc_enable_default: nfc-enable-default-state {
> 		pins = "gpio12", "gpio62";
> 	};
> 
> 	nfc_int_default: nfc-int-default-state {
> 		pins = "gpio63";
> 	};
> 
> So that's how you code. Less work for future linter/checkpatch.
> 
> The trouble is that "pins" property sorting can result in nodes being
> spread all over, imagine:
> 
> 	nfc_enable_default: nfc-enable-default-state {
> 		pins = "gpio5", "gpio62";
> 			// ^^^^^ DIFFERENCE!
> 	};
> 
>  	sde_dsi_active: sde-dsi-active-state {
>  		pins = "gpio6", "gpio11";
> 	}
> 
> 	nfc_int_default: nfc-int-default-state {
> 		pins = "gpio63";
> 	};
> 
> That's why I would propose to keep everything sorted by node name, but I
> am fine with both choices. Qualcomm maintainers decide about such
> detailed style they want to impose.

At LPC we agreed that the last sentence should not be the case ;)

I think I'm fine with "children by name" then..

Konrad