Skip to content

Conversation

hvraven
Copy link
Contributor

@hvraven hvraven commented Aug 20, 2025

The bits for COMP1 exactly match those in the existing definition of comp_v3. COMP2 has an extra bit WINMODE, used to connect the positive inputs of both comparators. This is not described at the moment. I was not sure how this should be described, and don't need it at the moment.

The used register definition is lacking the description for the INxSEL and INMESEL registers. These also have variations between the different comparator units, as they select the GPIO pins used. There also appears to be an error in the reference manual (RM0394 on page 538), as for COMP1 the description of INMSEL states that 0b110 uses INMESEL for selection, whereas the description in the INMESEL register states that 0b111 should be used. The later would be in line with COMP2. I currently don't have an easy way to check which one is correct. It also doesn't affect this change, as the enums are not described.
EDIT: The later also matches the values in the tables on page 527, so 0b111 is the correct setting to use the INMESEL register

The bits for `COMP1` exactly match those in the existing definition of
`comp_v3`. `COMP2` has an extra bit `WINMODE`, used to connect the
positive inputs of both comparators. This is not described at the
moment.
@embassy-ci
Copy link

embassy-ci bot commented Aug 20, 2025

@hvraven
Copy link
Contributor Author

hvraven commented Aug 20, 2025

There is an actual difference, probably requiring an independent copy of the register definitions. PWRMODE does not support low speed in this variant, the bits also map to medium speed.
Looking at the other definitions, which naming scheme is preferred for register definitions? Some use Pwrmode (following the ST register naming), while others use full names, such as PowerMode.

With some guidance on the differences between COMP1 and COMP2 (only the extra bit in COMP2), I will prepare a new PR with an independent definition if desired. Until then I would keep it open, as it is not really wrong, and an easy fix to make the registers accessible in the STM32L4 family

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.

1 participant