Update adc driver to support nRF54L15 #68692

kl-cruz · 2024-02-07T13:31:41Z

No description provided.

MaureenHelm · 2024-04-10T21:07:38Z

Needs to be rebased.

@anangl please take a look

kl-cruz · 2024-04-12T12:20:57Z

Twister tests issues are not related to this PR. Bug raised: #71437

anangl · 2024-04-17T14:35:09Z

drivers/adc/adc_nrfx_saadc.c

 BUILD_ASSERT((NRF_SAADC_AIN0 == NRF_SAADC_INPUT_AIN0) &&
-	     (NRF_SAADC_AIN1 == NRF_SAADC_INPUT_AIN1) &&


There is no need to change the indentation here.

anangl · 2024-04-17T14:40:18Z

drivers/adc/adc_nrfx_saadc.c

-	     (NRF_SAADC_VDDHDIV5 == NRF_SAADC_INPUT_VDDHDIV5) &&
+			 && (NRF_SAADC_VDDHDIV5 == NRF_SAADC_INPUT_VDDHDIV5)


Instead of doing this, I'd rather suggest to use the following below:

#if defined(SAADC_CH_PSELP_PSELP_VDD) (NRF_SAADC_VDD == NRF_SAADC_INPUT_VDD) && #endif 1,

This way we'll limit the amount of changes.

anangl · 2024-04-17T14:46:05Z

drivers/adc/adc_nrfx_saadc.c

 	case ADC_GAIN_1:
 		config.gain = NRF_SAADC_GAIN1;
 		break;
 	case ADC_GAIN_2:
 		config.gain = NRF_SAADC_GAIN2;
 		break;
+#if defined(NRF_SAADC_GAIN4)


Suggested change

#if defined(NRF_SAADC_GAIN4)

#if defined(SAADC_CH_CONFIG_GAIN_Gain4)

anangl · 2024-04-17T15:38:44Z

drivers/adc/adc_nrfx_saadc.c

@@ -6,7 +6,7 @@

 #define ADC_CONTEXT_USES_KERNEL_TIMER
 #include "adc_context.h"
-#include <hal/nrf_saadc.h>
+#include <nrfx_saadc.h>


This is misleading. The nrfx_saadc driver is not used here.

Suggested change

#include <nrfx_saadc.h>

#include <haly/nrfy_saadc.h>

Regarding NRFX_SAADC_SAMPLES_TO_BYTES() macro, see the next comment.

anangl · 2024-04-17T15:52:15Z

drivers/adc/adc_nrfx_saadc.c

+		nrf_saadc_value_t *buffer;
+
+		#if (NRF_SAADC_8BIT_SAMPLE_WIDTH == 8)
+		if (nrfy_saadc_resolution_get(NRF_SAADC) == NRF_SAADC_RESOLUTION_8BIT) {
+			buffer = (uint8_t *)nrfy_saadc_buffer_pointer_get(NRF_SAADC) +
+				 nrfy_saadc_amount_get(NRF_SAADC);
+		} else
+		#endif
+		{
+			buffer = (uint16_t *)nrfy_saadc_buffer_pointer_get(NRF_SAADC) +
+				 nrfy_saadc_amount_get(NRF_SAADC);
+		}


Instead of this, I'd suggest to add a function like:

static uint32_t samples_to_bytes(const struct adc_sequence *sequence, uint16_t number_of_samples) { if (NRF_SAADC_8BIT_SAMPLE_WIDTH == 8 && sequence->resolution == 8) { return number_of_samples; } else { return number_of_samples * 2; } }

and use it here:

nrf_saadc_value_t *buffer = (uint8_t *)nrf_saadc_buffer_pointer_get(NRF_SAADC) + samples_to_bytes(&ctx->sequence, nrfy_saadc_amount_get(NRF_SAADC));

and in check_buffer_size():

needed_buffer_size = samples_to_bytes(sequence, active_channels);

Looks good. It makes the code more readable

anangl · 2024-04-17T15:58:43Z

samples/drivers/adc/boards/nrf54l15pdk_nrf54l15_cpuapp.overlay

+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Copyright (c) 2023 Nordic Semiconductor ASA


Suggested change

* Copyright (c) 2023 Nordic Semiconductor ASA

* Copyright (c) 2024 Nordic Semiconductor ASA

anangl · 2024-04-17T15:59:17Z

tests/drivers/adc/adc_api/boards/nrf54l15pdk_nrf54l15_cpuapp.overlay

+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Copyright (c) 2023 Karol Lasończyk <[email protected]>


Suggested change

* Copyright (c) 2023 Karol Lasończyk <[email protected]>

* Copyright (c) 2024 Nordic Semiconductor ASA

anangl · 2024-04-17T16:00:45Z

drivers/adc/adc_nrfx_saadc.c

+				     : 1)));
+
+	tacq = (nrf_saadc_acqtime_t)(acq_time / MINIMUM_ACQ_TIME_IN_NS) - 1;
+	if ((tacq > SAADC_CH_CONFIG_TACQ_Max) || (acq_time < MINIMUM_ACQ_TIME_IN_NS)) {


Suggested change

if ((tacq > SAADC_CH_CONFIG_TACQ_Max) || (acq_time < MINIMUM_ACQ_TIME_IN_NS)) {

if ((tacq > NRF_SAADC_ACQTIME_MAX) || (acq_time < MINIMUM_ACQ_TIME_IN_NS)) {

MaureenHelm · 2024-04-26T19:34:06Z

Need to rebase and resolve conflicts

Simple enabling adc node. Upstream PR: zephyrproject-rtos/zephyr#68692 Applied as noup because it can't be applied cleanly. Signed-off-by: Karol Lasończyk <[email protected]> (cherry picked from commit 88adda3b14f726645c0a3ebac6d6fab571144f11)

jaz1-nordic · 2024-05-08T12:26:37Z

boards/nordic/nrf54l15pdk/nrf54l15pdk_nrf54l15_cpuapp.dts

@@ -147,4 +147,7 @@
 		t-enter-dpd = <10000>;
 		t-exit-dpd = <35000>;
 	};
+


Suggested change

};

jaz1-nordic · 2024-05-08T12:28:39Z

boards/nordic/nrf54l15pdk/nrf54l15pdk_nrf54l15_cpuapp.dts

@@ -147,4 +147,7 @@
 		t-enter-dpd = <10000>;
 		t-exit-dpd = <35000>;
 	};
+
+&adc {
+	status = "okay";


What is the reason to enable adc at board level?
Can't this be moved to board overlay for individual tests/samples?

Enabling it on board level is consistent with other Nordic DKs

Expand supported list. Upstream PR: zephyrproject-rtos/zephyr#68692. Signed-off-by: Bartlomiej Buczek <[email protected]>

Expands driver to cover nRF54L15 features like AIN as GPIO configuration, new reference voltage, different set of supported gain options. Signed-off-by: Karol Lasończyk <[email protected]>

Simple enabling adc node. Signed-off-by: Karol Lasończyk <[email protected]>

New file with ADC configuration for nRF54L15 PDK. Signed-off-by: Karol Lasończyk <[email protected]>

New file to make test works. Signed-off-by: Karol Lasończyk <[email protected]>

Expand supported list. Signed-off-by: Karol Lasończyk <[email protected]>

Expand supported list. Upstream PR: zephyrproject-rtos/zephyr#68692. Signed-off-by: Bartlomiej Buczek <[email protected]>

masz-nordic

Looks good, some additions in nrfx will be needed to tidy up in the future.

masz-nordic · 2024-05-15T09:32:05Z

drivers/adc/adc_nrfx_saadc.c

+static const uint8_t saadc_psels[NRF_SAADC_AIN7 + 1] = {
+	[NRF_SAADC_AIN0] = NRF_PIN_PORT_TO_PIN_NUMBER(4U, 1),
+	[NRF_SAADC_AIN1] = NRF_PIN_PORT_TO_PIN_NUMBER(5U, 1),
+	[NRF_SAADC_AIN2] = NRF_PIN_PORT_TO_PIN_NUMBER(6U, 1),
+	[NRF_SAADC_AIN3] = NRF_PIN_PORT_TO_PIN_NUMBER(7U, 1),
+	[NRF_SAADC_AIN4] = NRF_PIN_PORT_TO_PIN_NUMBER(11U, 1),
+	[NRF_SAADC_AIN5] = NRF_PIN_PORT_TO_PIN_NUMBER(12U, 1),
+	[NRF_SAADC_AIN6] = NRF_PIN_PORT_TO_PIN_NUMBER(13U, 1),
+	[NRF_SAADC_AIN7] = NRF_PIN_PORT_TO_PIN_NUMBER(14U, 1),
+};


We will need to find a better solution for AIN lookup in the future, this does not scale well.

masz-nordic · 2024-05-15T09:43:50Z

drivers/adc/adc_nrfx_saadc.c

+	nrf_saadc_acqtime_t tacq = 0;
+	uint16_t acq_time =
+		(acquisition_time == ADC_ACQ_TIME_DEFAULT
+			 ? DEFAULT_ACQ_TIME_IN_NS
+			 : (ADC_ACQ_TIME_VALUE(acquisition_time) *
+			    (ADC_ACQ_TIME_UNIT(acquisition_time) == ADC_ACQ_TIME_MICROSECONDS
+				     ? 1000
+				     : 1)));
+
+	tacq = (nrf_saadc_acqtime_t)(acq_time / MINIMUM_ACQ_TIME_IN_NS) - 1;
+	if ((tacq > NRF_SAADC_ACQTIME_MAX) || (acq_time < MINIMUM_ACQ_TIME_IN_NS)) {
+		result = -EINVAL;
+	} else {
+		*p_tacq_val = tacq;
+	}


I suppose we need a function for that in HAL.

masz-nordic · 2024-05-15T09:47:30Z

drivers/adc/adc_nrfx_saadc.c


 	if (channel_id >= SAADC_CH_NUM) {
 		return -EINVAL;
 	}

 	switch (channel_cfg->gain) {
+#if defined(SAADC_CH_CONFIG_GAIN_Gain1_6)


MDK use should be avoided, but I don't see a real alternative for now.

kl-cruz requested a review from carlescufi as a code owner February 7, 2024 13:31

zephyrbot added area: Samples Samples area: ADC Analog-to-Digital Converter (ADC) platform: nRF Nordic nRFx labels Feb 7, 2024

zephyrbot requested review from anangl, kartben and nashif February 7, 2024 13:32

zephyrbot assigned anangl Feb 7, 2024

kl-cruz mentioned this pull request Feb 13, 2024

Update adc driver to support nRF54L15 nrfconnect/sdk-zephyr#1494

Merged

kl-cruz force-pushed the nrf54l15_saadc_support branch from 7e04c33 to 540cab8 Compare March 1, 2024 15:30

kl-cruz mentioned this pull request Apr 11, 2024

Update adc driver to support nRF54H20 #71395

Merged

kl-cruz force-pushed the nrf54l15_saadc_support branch from 540cab8 to e145427 Compare April 11, 2024 13:40

zephyrbot requested a review from decsny April 11, 2024 13:40

kl-cruz mentioned this pull request Apr 12, 2024

drivers: adc: adc_api test failed on some platforms #71437

Closed

nordic-segl approved these changes Apr 17, 2024

View reviewed changes

anangl requested changes Apr 17, 2024

View reviewed changes

kl-cruz dismissed stale reviews from nika-nordic and anangl via b2b09d5 May 8, 2024 06:34

kl-cruz force-pushed the nrf54l15_saadc_support branch from f42e1eb to b2b09d5 Compare May 8, 2024 06:34

zephyrbot requested review from jaz1-nordic, magp-nordic and masz-nordic May 8, 2024 06:34

nordic-babu mentioned this pull request May 8, 2024

[nrf fromlist] boards: nordic: Add ADC to supported in nRF54L15 PDK nrfconnect/sdk-zephyr#1687

Merged

jaz1-nordic reviewed May 8, 2024

View reviewed changes

rlubos pushed a commit to nrfconnect/sdk-zephyr that referenced this pull request May 8, 2024

[nrf fromlist] boards: nordic: Add ADC to supported in nRF54L15 PDK

27319a0

Expand supported list. Upstream PR: zephyrproject-rtos/zephyr#68692. Signed-off-by: Bartlomiej Buczek <[email protected]>

kl-cruz added 2 commits May 10, 2024 09:45

drivers: adc: Update adc driver to support nRF54L15 device

7112b1a

Expands driver to cover nRF54L15 features like AIN as GPIO configuration, new reference voltage, different set of supported gain options. Signed-off-by: Karol Lasończyk <[email protected]>

boards: Enable ADC in nRF54L15 PDK configuration

cc24ae8

Simple enabling adc node. Signed-off-by: Karol Lasończyk <[email protected]>

kl-cruz force-pushed the nrf54l15_saadc_support branch from b2b09d5 to d944384 Compare May 10, 2024 07:56

kl-cruz added 3 commits May 10, 2024 10:54

samples: adc: Add overlay for nRF54L15 PDK

b93879c

New file with ADC configuration for nRF54L15 PDK. Signed-off-by: Karol Lasończyk <[email protected]>

tests: adc: Add overlay for nRF54L15 PDK for adc_api test

e8627f9

New file to make test works. Signed-off-by: Karol Lasończyk <[email protected]>

boards: nordic: Add ADC to supported in nRF54L15 PDK

3441974

Expand supported list. Signed-off-by: Karol Lasończyk <[email protected]>

kl-cruz force-pushed the nrf54l15_saadc_support branch from d944384 to 3441974 Compare May 10, 2024 08:54

57300 pushed a commit to 57300/sdk-zephyr that referenced this pull request May 14, 2024

[nrf fromlist] boards: nordic: Add ADC to supported in nRF54L15 PDK

09417ac

Expand supported list. Upstream PR: zephyrproject-rtos/zephyr#68692. Signed-off-by: Bartlomiej Buczek <[email protected]>

masz-nordic approved these changes May 15, 2024

View reviewed changes

anangl approved these changes May 15, 2024

View reviewed changes

fabiobaltieri merged commit 394e097 into zephyrproject-rtos:main May 15, 2024
22 checks passed

kl-cruz deleted the nrf54l15_saadc_support branch June 4, 2024 06:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update adc driver to support nRF54L15 #68692

Update adc driver to support nRF54L15 #68692

kl-cruz commented Feb 7, 2024

MaureenHelm commented Apr 10, 2024

kl-cruz commented Apr 12, 2024

anangl Apr 17, 2024

anangl Apr 17, 2024

anangl Apr 17, 2024

anangl Apr 17, 2024

anangl Apr 17, 2024

kl-cruz Apr 18, 2024

anangl Apr 17, 2024

anangl Apr 17, 2024

anangl Apr 17, 2024

MaureenHelm commented Apr 26, 2024

jaz1-nordic May 8, 2024

jaz1-nordic May 8, 2024

kl-cruz May 10, 2024

masz-nordic left a comment

masz-nordic May 15, 2024

masz-nordic May 15, 2024

masz-nordic May 15, 2024

		BUILD_ASSERT((NRF_SAADC_AIN0 == NRF_SAADC_INPUT_AIN0) &&
		(NRF_SAADC_AIN1 == NRF_SAADC_INPUT_AIN1) &&

		(NRF_SAADC_VDDHDIV5 == NRF_SAADC_INPUT_VDDHDIV5) &&
		&& (NRF_SAADC_VDDHDIV5 == NRF_SAADC_INPUT_VDDHDIV5)

	#if defined(NRF_SAADC_GAIN4)
	#if defined(SAADC_CH_CONFIG_GAIN_Gain4)

	* Copyright (c) 2023 Nordic Semiconductor ASA
	* Copyright (c) 2024 Nordic Semiconductor ASA

	* Copyright (c) 2023 Karol Lasończyk <[email protected]>
	* Copyright (c) 2024 Nordic Semiconductor ASA

	if ((tacq > SAADC_CH_CONFIG_TACQ_Max) \|\| (acq_time < MINIMUM_ACQ_TIME_IN_NS)) {
	if ((tacq > NRF_SAADC_ACQTIME_MAX) \|\| (acq_time < MINIMUM_ACQ_TIME_IN_NS)) {

Update adc driver to support nRF54L15 #68692

Update adc driver to support nRF54L15 #68692

Conversation

kl-cruz commented Feb 7, 2024

MaureenHelm commented Apr 10, 2024

kl-cruz commented Apr 12, 2024

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaureenHelm commented Apr 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masz-nordic left a 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