raiden00pl commented on code in PR #17007:
URL: https://github.com/apache/nuttx/pull/17007#discussion_r2354468408


##########
arch/arm/src/stm32h5/stm32_adc.h:
##########
@@ -498,8 +521,8 @@ extern "C"
 
 struct adc_dev_s;
 struct adc_dev_s *stm32h5_adc_initialize(int intf,
-                                         const uint8_t *chanlist,
-                                         int nchannels);
+                                         struct stm32_adc_channel_s *chanlist
+                                         , int nchannels);

Review Comment:
   > I evaluated the other implementations in the stm32 family. I did not like 
any of them for the stm32h5. The stm32f7 could work technically, but honestly I 
think my implementation is better. The below structures make little sense for 
the h5. all_same is unnecessary, each channel sample time is independent of the 
other. To me, this structure did not make a lot of sense for the h5, and the 
parts that did overly complicated things.
   
   But it works, has been here for many years, and doesn't break interface 
compatibility between ST families. Personal preferences shouldn't be the 
determining factor here. The ADC in the STM32H5 is nothing special, this IP 
core is used in other ST chips.
   
   I'm not saying your change doesn't make life easier. I'm saying this change 
brings fewer benefits than the maintenance nightmare it will cause later.
   
   > I also needed to find a way to allow for differential inputs. I did not 
see any STM32 drivers that actually implemented differential mode. Passing the 
mode in the structure, like the nrf52 does, seemed like a good way for the 
board level to indicate to the lower level driver that the channels needs to be 
configured in differential mode.
   
   What do you need for diff input? If I remember correctly, just configure 
GPIO as analog, set STM32_ADC_DIFSEL and set ADCALDIF. This can be solved 
trivially with two ifdefs and the STM32_ADC_DIFSEL definition in board.h. There 
is no need to break compatibility between interfaces.
   
   > I used the nrf52 as a reference. So this isn't completely out of nowhere. 
It was suggested to me by a colleague as an example and I thought it worked 
well for my application.
   
   nrf52 is not an STM32 chip. I've been saying from the beginning that what 
matters is consistency in the ST family, not across all architectures.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to