Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: drivers: dac channel structure has a buffered parameter boolean #58932

Merged
merged 1 commit into from
Jun 8, 2023

Conversation

FRASTM
Copy link
Collaborator

@FRASTM FRASTM commented Jun 7, 2023

Add the '.buffered' field to the dac_channel_cfg structure when testing the DAC.
This boolean parameter is initialised to 'true' to PASS the test.
It follows the #57730

Fixes #58931

Signed-off-by: Francois Ramu [email protected]

Add the '.buffered' field to the dac_channel_cfg structure
when testing the DAC.
This boolean parameter is initialised to 'true' to PASS the test.
It follows the zephyrproject-rtos#57730
Also changed for the samples/drivers/dac

Signed-off-by: Francois Ramu <[email protected]>
@FRASTM FRASTM self-assigned this Jun 7, 2023
@FRASTM FRASTM added area: Tests Issues related to a particular existing or missing test area: DAC Digital-to-Analog Converter labels Jun 7, 2023
@FRASTM FRASTM marked this pull request as ready for review June 7, 2023 09:13
@FRASTM FRASTM requested a review from nashif as a code owner June 7, 2023 09:13
@FRASTM FRASTM requested review from erwango, gautierg-st and Desvauxm-st and removed request for martinjaeger June 7, 2023 09:13
@FRASTM
Copy link
Collaborator Author

FRASTM commented Jun 7, 2023

$ west build -p auto -b nucleo_wl55jc tests/drivers/dac/dac_loopback/
when pin A2 is connected to A4 on the arduino connector CN8 of the nucleo_wl55jc board

*** Booting Zephyr OS build v3.4.0-rc2-101-g7b2034aaecc4 ***                    
Running TESTSUITE dac_loopback                                                  
===================================================================             
START - test_dac_loopback                                                       
 PASS - test_dac_loopback in 0.011 seconds                                      
===================================================================             
TESTSUITE dac_loopback succeeded                                                
                                                                                
------ TESTSUITE SUMMARY START ------                                           
                                                                                
SUITE PASS - 100.00% [dac_loopback]: pass = 1, fail = 0, skip = 0, total = 1 dus
 - PASS - [dac_loopback.test_dac_loopback] duration = 0.011 seconds             
                                                                                
------ TESTSUITE SUMMARY END ------                                             
                                                                                
================================
```===================================             
PROJECT EXECUTION SUCCESSFUL                                                    

Copy link
Member

@martinjaeger martinjaeger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue does not seem to be the not enabled buffer, but incorrect output configuration. We just didn't realize this before, as an enabled buffer automatically connects the DAC to the external pin.

I think we should use LL_DAC_SetOutputConnection to fix the driver instead.

@FRASTM
Copy link
Collaborator Author

FRASTM commented Jun 8, 2023

The issue does not seem to be the not enabled buffer, but incorrect output configuration. We just didn't realize this before, as an enabled buffer automatically connects the DAC to the external pin.

I think we should use LL_DAC_SetOutputConnection to fix the driver instead.

Do you mean LL_DAC_SetOutputBuffer( ... OutputBuffer ... )
plus LL_DAC_SetOutputConnection( LL_DAC_OUTPUT_CONNECT_GPIO)

@martinjaeger
Copy link
Member

Do you mean LL_DAC_SetOutputBuffer( ... OutputBuffer ... ) plus LL_DAC_SetOutputConnection( LL_DAC_OUTPUT_CONNECT_GPIO)

Yes, I think that would be the correct way. What do you think?

@FRASTM
Copy link
Collaborator Author

FRASTM commented Jun 8, 2023

Do you mean LL_DAC_SetOutputBuffer( ... OutputBuffer ... ) plus LL_DAC_SetOutputConnection( LL_DAC_OUTPUT_CONNECT_GPIO)

Yes, I think that would be the correct way. What do you think?

That's better, but initializing the .buffered = true in the tests/drivers/dac/dac_loopback/src/test_dac.c is mandatory.

diff --git a/drivers/dac/dac_stm32.c b/drivers/dac/dac_stm32.c
index bfbdc5003f..c49b6ef20e 100644
--- a/drivers/dac/dac_stm32.c
+++ b/drivers/dac/dac_stm32.c
@@ -111,6 +111,11 @@ static int dac_stm32_channel_setup(const struct device *dev,
 		table_channels[channel_cfg->channel_id - STM32_FIRST_CHANNEL],
 		output_buffer);
 
+	/* By default, the output pin is considered as GPIO output pin */
+	LL_DAC_SetOutputConnection(cfg->base,
+		table_channels[channel_cfg->channel_id - STM32_FIRST_CHANNEL],
+		LL_DAC_OUTPUT_CONNECT_GPIO);
+
 	LL_DAC_Enable(cfg->base,
 		table_channels[channel_cfg->channel_id - STM32_FIRST_CHANNEL]);
 

@martinjaeger
Copy link
Member

I have tested with nucleo_l073rz and unfortunately, it does not have the LL_DAC_SetOutputConnection and compilation fails. Looks like there are some differences between the MCU series.

Regarding the original discussion whether the output buffer is needed or not: I have checked the impedance of the DAC and the ADC in the STM32L072 datasheet.

DAC

image

ADC

image

They are quite close and without a capacitor the DAC impedance does indeed seem to be too high to drive the ADC. I have added a 100 nF capacitor and it works even without the buffer.

As we don't have the cap in standard Nucleo boards I agree to enable the buffer, now that I understand why it doesn't work without the buffer.

@nashif nashif merged commit c908aea into zephyrproject-rtos:main Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: DAC Digital-to-Analog Converter area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests/drivers/dac loopback failed due to un-initialised dac_channel_cfg structure
5 participants