nfc: Introduce Zero Latency IRQ #13248

dchat-nordic · 2023-11-28T08:43:39Z

Introduce Zero Latency IRQ in NFC Platform.
The Zero Latency IRQ is an improvement on NFC
processing latency with cost of a single SWI interrupt.

KAGA164 · 2023-11-28T10:30:36Z

subsys/nfc/lib/platform_internal_thread.c

-#endif /* CONFIG_NFC_LOW_LATENCY_IRQ */
+#endif /* CONFIG_NFC_ZERO_LATENCY_IRQ */
+
+#define HFCLK_NONE	0


KAGA164 · 2023-11-28T10:30:55Z

subsys/nfc/lib/platform_internal_thread.c

+static atomic_t clk_mode;
+#endif /* CONFIG_NFC_ZERO_LATENCY_IRQ */
+
+static void hfclk_set(uint32_t mode)


Suggested change

static void hfclk_set(uint32_t mode)

static void hfclk_set(uint32_t mode)

enum?

KAGA164 · 2023-11-28T10:33:53Z

subsys/nfc/lib/platform_internal_thread.c

 ISR_DIRECT_DECLARE(nfc_swi_handler)
 {
+	atomic_val_t tmp;
+
+	tmp = atomic_set(&clk_mode, HFCLK_NONE);


I am not sure if atomic would have any effect in our case. Probably using just volatile would make sense

KAGA164 · 2023-11-28T10:35:14Z

subsys/nfc/lib/platform_internal_thread.c

 ISR_DIRECT_DECLARE(nfc_swi_handler)
 {
+	atomic_val_t tmp;


You could remember previous clock status and check it in this function to prevent doing the same operation few times

peknis · 2023-12-07T06:35:38Z

doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst

+* Added:
+
+  * Support for zero-latency interrupts for NFC.


Suggested change

* Added:

* Support for zero-latency interrupts for NFC.

* Added support for zero-latency interrupts for all NFC libraries.

If this is added for all of them.

peknis · 2023-12-07T06:36:58Z

subsys/nfc/lib/platform_internal.h

@@ -33,6 +33,33 @@ extern "C" {
 */
 int nfc_platform_internal_init(nfc_lib_cb_resolve_t cb_rslv);

+/** @brief Function for initializing internal hfclk control.


Suggested change

/** @brief Function for initializing internal hfclk control.

/** @brief Initialize internal hfclk control.

What is hfclk? Should we open it in the first instance?

peknis · 2023-12-07T06:37:16Z

subsys/nfc/lib/platform_internal.h

+ */
+int nfc_platform_internal_hfclk_init(void);
+
+/** @brief Function for starting hfclk.


Suggested change

/** @brief Function for starting hfclk.

/** @brief Start hfclk.

peknis · 2023-12-07T06:37:36Z

subsys/nfc/lib/platform_internal.h

+ */
+int nfc_platform_internal_hfclk_start(void);
+
+/** @brief Function for stopping hfclk.


Suggested change

/** @brief Function for stopping hfclk.

/** @brief Stop hfclk.

maje-emb · 2023-12-11T11:43:09Z

subsys/nfc/lib/platform.c

+#ifdef CONFIG_NFC_ZERO_LATENCY_IRQ
+	IRQ_DIRECT_CONNECT(NFCT_IRQn, CONFIG_NFCT_IRQ_PRIORITY,
+			   nfc_isr_wrapper, IRQ_ZERO_LATENCY);
+#else
 	IRQ_DIRECT_CONNECT(NFCT_IRQn, CONFIG_NFCT_IRQ_PRIORITY,
 			   nfc_isr_wrapper, 0);
+#endif /* CONFIG_NFC_ZERO_LATENCY_IRQ */


Suggested change

#ifdef CONFIG_NFC_ZERO_LATENCY_IRQ

IRQ_DIRECT_CONNECT(NFCT_IRQn, CONFIG_NFCT_IRQ_PRIORITY,

nfc_isr_wrapper, IRQ_ZERO_LATENCY);

#else

IRQ_DIRECT_CONNECT(NFCT_IRQn, CONFIG_NFCT_IRQ_PRIORITY,

nfc_isr_wrapper, 0);

#endif /* CONFIG_NFC_ZERO_LATENCY_IRQ */

IRQ_DIRECT_CONNECT(NFCT_IRQn, CONFIG_NFCT_IRQ_PRIORITY,

nfc_isr_wrapper, NFCT_IRQ_FLAGS);

What do you think about using the NFCT_IRQ_FLAGS definition?
This way we avoid conditional compilation at this place.

KAGA164 · 2023-12-12T12:34:26Z

subsys/nfc/lib/Kconfig

@@ -80,14 +80,15 @@ config NFC_THREAD_PRIORITY

 endif# NFC_OWN_THREAD

-config NFC_LOW_LATENCY_IRQ
-	bool "Use low latency IRQ for NFC"
+config NFC_ZERO_LATENCY_IRQ


Experimental?

KAGA164 · 2023-12-12T12:35:57Z

subsys/nfc/lib/platform_internal_thread.c

+{
+	switch (event->evt_id) {
+	case NRFX_NFCT_EVT_FIELD_DETECTED:
+#ifdef CONFIG_NFC_ZERO_LATENCY_IRQ


My general suggestion is to avoid conditional compilation by using IS_ENABLED()

This can't be swapped here because of clk_mode variable which is conditional.

KAGA164 · 2023-12-12T12:50:36Z

I also noticed error log:
E: Tried to read header: 4 bytes, read 0.
E: Reading nfc ring buffer failed, resetting ring buffer.

NordicBuilder · 2023-12-20T09:38:51Z

Test specification

CI/Jenkins/NRF

Integration Platforms

CI/Jenkins/integration

Test Module	File based changes	Manually selected	West overwrite
test-sdk-find-my	X

Detailed information of selected test modules

Note: This message is automatically posted and updated by the CI

NordicBuilder · 2023-12-20T10:09:08Z

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

Introduce Zero Latency IRQ in NFC Platform. The Zero Latency IRQ is an improvement on NFC processing latency with cost of a single SWI interrupt. Jira: NCSDK-23347 Signed-off-by: Dominik Chat <[email protected]>

dchat-nordic requested a review from KAGA164 November 28, 2023 08:43

github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Nov 28, 2023

dchat-nordic added the DNM label Nov 28, 2023

KAGA164 suggested changes Nov 28, 2023

View reviewed changes

dchat-nordic force-pushed the nfc_zero branch 2 times, most recently from 10cd8b4 to 76a0aeb Compare November 29, 2023 11:23

github-actions bot added doc-required PR must not be merged without tech writer approval. and removed changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Nov 29, 2023

dchat-nordic requested a review from maje-emb November 29, 2023 11:23

dchat-nordic force-pushed the nfc_zero branch from 76a0aeb to 6d294c7 Compare November 29, 2023 11:28

dchat-nordic requested a review from KAGA164 November 30, 2023 09:02

dchat-nordic force-pushed the nfc_zero branch 2 times, most recently from 33d6809 to 458dce6 Compare November 30, 2023 12:56

dchat-nordic marked this pull request as ready for review December 6, 2023 10:08

dchat-nordic requested review from grochu, anangl and carlescufi as code owners December 6, 2023 10:08

dchat-nordic requested a review from peknis December 6, 2023 10:09

peknis requested changes Dec 7, 2023

View reviewed changes

maje-emb reviewed Dec 11, 2023

View reviewed changes

KAGA164 suggested changes Dec 12, 2023

View reviewed changes

dchat-nordic force-pushed the nfc_zero branch from 458dce6 to 12bc24f Compare December 20, 2023 09:35

dchat-nordic requested review from KAGA164, maje-emb and peknis December 20, 2023 10:16

peknis approved these changes Dec 20, 2023

View reviewed changes

nfc: Introduce Zero Latency IRQ

65d06a8

Introduce Zero Latency IRQ in NFC Platform. The Zero Latency IRQ is an improvement on NFC processing latency with cost of a single SWI interrupt. Jira: NCSDK-23347 Signed-off-by: Dominik Chat <[email protected]>

dchat-nordic force-pushed the nfc_zero branch from 12bc24f to 65d06a8 Compare January 15, 2024 09:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nfc: Introduce Zero Latency IRQ #13248

nfc: Introduce Zero Latency IRQ #13248

dchat-nordic commented Nov 28, 2023 •

edited

Loading

KAGA164 Nov 28, 2023

KAGA164 Nov 28, 2023

KAGA164 Nov 28, 2023

KAGA164 Nov 28, 2023

peknis Dec 7, 2023

peknis Dec 7, 2023 •

edited

Loading

peknis Dec 7, 2023

peknis Dec 7, 2023

maje-emb Dec 11, 2023

KAGA164 Dec 12, 2023

KAGA164 Dec 12, 2023

dchat-nordic Dec 20, 2023

KAGA164 commented Dec 12, 2023

NordicBuilder commented Dec 20, 2023 •

edited

Loading

NordicBuilder commented Dec 20, 2023

	static void hfclk_set(uint32_t mode)
	static void hfclk_set(uint32_t mode)

	/** @brief Function for initializing internal hfclk control.
	/** @brief Initialize internal hfclk control.

	/** @brief Function for starting hfclk.
	/** @brief Start hfclk.

	/** @brief Function for stopping hfclk.
	/** @brief Stop hfclk.

nfc: Introduce Zero Latency IRQ #13248

Are you sure you want to change the base?

nfc: Introduce Zero Latency IRQ #13248

Conversation

dchat-nordic commented Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peknis Dec 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KAGA164 commented Dec 12, 2023

NordicBuilder commented Dec 20, 2023 • edited Loading

Test specification

NordicBuilder commented Dec 20, 2023

dchat-nordic commented Nov 28, 2023 •

edited

Loading

peknis Dec 7, 2023 •

edited

Loading

NordicBuilder commented Dec 20, 2023 •

edited

Loading