Skip to content

Conversation

@kapi-no
Copy link
Contributor

@kapi-no kapi-no commented Jun 6, 2025

Aligned the availability of the NFC_T2T_NRFXLIB and NFC_T4T_NRFXLIB Kconfig options with the status of the nfct DTS node.

@grochu
Copy link
Contributor

grochu commented Jun 9, 2025

Thanks for providing this PR! We still haven't managed to resolve it. Just a though from me - how about moving the dependency to the CONFIG_NFC_PLATFORM option in sdk-nrf? Both T2T and T4T require some platform implementation for the runtime environment they are used in. Like that we'd avoid dependency expansion in the nrfxlib, which in theory could be used outside of nrf-sdk.

@kapi-no
Copy link
Contributor Author

kapi-no commented Jun 10, 2025

Thanks for providing this PR! We still haven't managed to resolve it. Just a though from me - how about moving the dependency to the CONFIG_NFC_PLATFORM option in sdk-nrf? Both T2T and T4T require some platform implementation for the runtime environment they are used in. Like that we'd avoid dependency expansion in the nrfxlib, which in theory could be used outside of nrf-sdk.

I wanted to put this depends on statement initially in the NFC platform Kconfig:

https://github.com/nrfconnect/sdk-nrf/blob/main/subsys/nfc/lib/Kconfig

However, the current KConfig definition is written in such a way that it will fail silently for the user:

config NFC_PLATFORM
	bool "Common NFC configuration"
	default y if NFC_T2T_NRFXLIB || NFC_T4T_NRFXLIB
	select NRFX_NFCT
	select CLOCK_CONTROL
	help
	  Enable common configuration for the NFC

If you were to add depends on HAS_HW_NRF_NFCT in the NFC_PLATFORM Kconfig, you won't get a warning about unmet dependency (HAS_HW_NRF_NFCT) here because the NFC platform is not explicitly selected but is rather enabled by default. I think this approach would confuse users.

If you want to set it up like this, I can do that. The most important thing is that the build process for a project will fail with either approach.

Also, there is nothing stopping us from moving the NFC T2T/T4T library Kconfig to sdk-nrf. You can follow the approach used by the SDC team for the SDC library.

@grochu
Copy link
Contributor

grochu commented Jun 13, 2025

Thanks for providing this PR! We still haven't managed to resolve it. Just a though from me - how about moving the dependency to the CONFIG_NFC_PLATFORM option in sdk-nrf? Both T2T and T4T require some platform implementation for the runtime environment they are used in. Like that we'd avoid dependency expansion in the nrfxlib, which in theory could be used outside of nrf-sdk.

I wanted to put this depends on statement initially in the NFC platform Kconfig:

https://github.com/nrfconnect/sdk-nrf/blob/main/subsys/nfc/lib/Kconfig

However, the current KConfig definition is written in such a way that it will fail silently for the user:

config NFC_PLATFORM
	bool "Common NFC configuration"
	default y if NFC_T2T_NRFXLIB || NFC_T4T_NRFXLIB
	select NRFX_NFCT
	select CLOCK_CONTROL
	help
	  Enable common configuration for the NFC

If you were to add depends on HAS_HW_NRF_NFCT in the NFC_PLATFORM Kconfig, you won't get a warning about unmet dependency (HAS_HW_NRF_NFCT) here because the NFC platform is not explicitly selected but is rather enabled by default. I think this approach would confuse users.

If you want to set it up like this, I can do that. The most important thing is that the build process for a project will fail with either approach.

Also, there is nothing stopping us from moving the NFC T2T/T4T library Kconfig to sdk-nrf. You can follow the approach used by the SDC team for the SDC library.

I see, I didn't take this into account. NFC configurations need some general refactoring, we'll do this at some point, when there is capacity. For now, your approach looks good.

Aligned the availability of the NFC_T2T_NRFXLIB and NFC_T4T_NRFXLIB
Kconfig options with the status of the nfct DTS node.

Signed-off-by: Kamil Piszczek <[email protected]>
@kapi-no kapi-no force-pushed the nfc_library_aligned_with_nfct_node_status branch from a4cf776 to 4b761f6 Compare June 17, 2025 13:33
@kapi-no
Copy link
Contributor Author

kapi-no commented Jun 17, 2025

Pure rebase. The PR file diff has not changed

@kapi-no kapi-no merged commit 89a6654 into nrfconnect:main Jun 17, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants