- 
                Notifications
    
You must be signed in to change notification settings  - Fork 8.2k
 
drivers: adc: sam0: Adjust resolution with the oversampling factor #75201
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
Conversation
| 
           Hello @Robibobo1, and thank you very much for your first pull request to the Zephyr project!  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Address it in the driver for the device, this is not how other ADCs in tree work
| 
           Sorry for my wrong commit, this is my first time contributing  | 
    
| 
           Check the contribution guidelines, I think you should squash all the commits, and avoid merge commits.  | 
    
1f2eb7f    to
    2fdc829      
    Compare
  
    | 
           @anangl @nandojve please take a look @Robibobo1 please address the failed compliance check  | 
    
86cc5ff    to
    ec978ea      
    Compare
  
            
          
                drivers/adc/adc_sam0.c
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please make the commit title clear that this is specific to sam, ie it should start with drivers: adc_sam0:
| 
           Hi @attie-argentum , Could you check this change?  | 
    
| 
           This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commit title needs to be adjusted
The old SAM0 adc drivers forces you to put 12 bits when using oversampling. The adc_raw_to_millivolts_dt function is not working properly because if we uses an oversampling of 8, we get 16 bits of resolution. The function uses directly the resolution in the DT, so the voltage will not be correct. To counter this, i forced the user to put the right resolution for his oversampling factor. Fixes: zephyrproject-rtos#74607 Signed-off-by: Robin Carrupt <[email protected]>
ec978ea    to
    0e49a16      
    Compare
  
    | LOG_ERR("Oversampling requires minimum 13 bit resolution"); | ||
| return -EINVAL; | ||
| } | ||
| ADC_RESSEL(adc) = ADC_RESSEL_12BIT; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ADC resolution is 12bit max.

However, if you want to Accumulate, Average or Oversampling, see

Check below sections:
45.6.2.9 Accumulation
45.6.2.10 Averaging
45.6.2.11 Oversampling and Decimation
This change must work for all sam0 series.
This possible it related to #76963
| 
           This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.  | 
    
| 
           This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.  | 
    
When using the oversampling factor on the SAMD21 family, the value that you get from the conversion is 16 bits instead of 12.
Using a factor of 8 adds 3 bits, a factor of 6 adds 2 bits, etc. I corrected this by dividing the factor by 2 and adding it to the resolution variable.
I didn't find other MCU families that use hardware oversampling. If there are others, we need to check if this is correct for that type of ADC.
Fixes: #74607
Signed-off-by: Robin Carrupt [email protected]