Add canfd peripheral definitions, take 2 #15
Conversation
| - name: MCAN | ||
| array: | ||
| len: 1 | ||
| stride: 252 | ||
| byte_offset: 28672 | ||
| block: CANFD | ||
| - name: TI_WRAPPER | ||
| array: | ||
| len: 1 | ||
| stride: 2560 | ||
| byte_offset: 29184 | ||
| block: TI_WRAPPER |
There was a problem hiding this comment.
From the old PR: TI's SVDs are kind of bad in this respect, these probably need to be merged up into this block.
I am still not quite sure how to do this nicely. Maybe someone has more experience on how to do this.
There was a problem hiding this comment.
Maybe the best way to go about this is:
- Use
!MergeBlocksto mergeTI_WRAPPERandCANFDintoGPRCM. Set GPRCM as main. - Rename GPRCM to
CANFD.
This does result in some inconsistency with other modules having an explicit GPRCM block, but duplicating 4 enums across every hardware module isn't a big deal.
There was a problem hiding this comment.
Ok this seems a little weird. Probably my expertise is not really high enough.
When I just test to merge MCAN (which is CANFD without the rename we do) and TI_WRAPPER into some new block TEST the fieldsets from the TI_WRAPPER are lost. When I try to merge into GPRCM with:
- !MergeBlocks
from: (MCAN|TI_WRAPPER)
to: GPRCM
it creates a GPRCM block that only contains the MCAN fields:
block/GPRCM:
items:
- name: CREL
description: MCAN Core Release Register.
byte_offset: 0
access: Read
fieldset: CREL
- name: ENDN
description: MCAN Endian Register.
byte_offset: 4
access: Read
fieldset: ENDN
- name: DBTP
description: MCAN Data Bit Timing and Prescaler Register.
byte_offset: 12
fieldset: DBTP
- name: TEST
description: MCAN Test Register.
byte_offset: 16
fieldset: TEST
- name: RWD
description: MCAN RAM Watchdog.
byte_offset: 20
fieldset: RWD
- name: CCCR
description: MCAN CC Control Register.
byte_offset: 24
fieldset: CCCR
...
I also tried:
- !MergeBlocks
from: (GPRCM|MCAN|TI_WRAPPER)
to: GPRCM
which does nothing at all. I also don't really get the main in the !MergeBlocks so I probably need a little more help on that.
There was a problem hiding this comment.
Hmm thats rather inconvenient. I tried locally a few other things and could not get the merging to work. This might require a custom transform in chiptool in the future.
TI does have a few SVD errors when running ./d extract-all:
processing MSPM0C110X ...No Peripheral
processing MSPM0G110X ...OTHER FAILURE
processing MSPM0G150X ...OTHER FAILURE
processing MSPM0G151X ...No Peripheral
processing MSPM0G310X ...OTHER FAILURE
processing MSPM0G350X ...OTHER FAILURE
processing MSPM0G351X ...OK
processing MSPM0H321X ...No Peripheral
processing MSPM0L110X ...OTHER FAILURE
processing MSPM0L111X ...No Peripheral
processing MSPM0L122X ...No Peripheral
processing MSPM0L130X ...OTHER FAILURE
processing MSPM0L134X ...OTHER FAILURE
processing MSPM0L222X ...No Peripheral
processing MSPS003FX ...No Peripheral
I wonder if the G350X peripheral is as bad as the G351X peripheral or not? It may require a manual change to the SVDs to generate, but we can note that.
So we have some options if the G350X after fixes isn't suitable:
- Rename in sequence to make the fields more reasonable. So
GPRCMwould becomeCANFDand then leaveMCANas is.TI_WRAPPERI don't know what to name. - Use the output after an initial transform and clean up by hand. This can be error prone.
- Do the work now and write a transform to do what we want in chiptool.
I'm fine with 1 or 3. Although I don't want to block you from doing other stuff, so feel free to do option 1. I'll look at option 3 some day. Probably worth opening an issue in chiptool about this case.
If you can could you also let TI know their CANFD definitions are inconvenient to use?
There was a problem hiding this comment.
Do you get the SVD errors with CANFD? Because for me it works fine
$ ./d extract-all CANFD0
processing MSPM0C110X ...No Peripheral
processing MSPM0G110X ...No Peripheral
processing MSPM0G150X ...No Peripheral
processing MSPM0G151X ...No Peripheral
processing MSPM0G310X ...OK
processing MSPM0G350X ...OK
processing MSPM0G351X ...OK
processing MSPM0H321X ...No Peripheral
processing MSPM0L110X ...No Peripheral
processing MSPM0L111X ...No Peripheral
processing MSPM0L122X ...No Peripheral
processing MSPM0L130X ...No Peripheral
processing MSPM0L134X ...No Peripheral
processing MSPM0L222X ...No Peripheral
processing MSPS003FX ...No PeripheralI also checked that with the transform we do not have major changes between mspm0g310x, mspm0g350x und mpsm0g351x.
I will check option 3, but probably don't have a lot of time.
Currently there is no pressure to finish this from my side. Maybe if someone is watching this PR and need this merged (because they want to continue the HAL 😉 ), let us know and I think it would be fine to go forward with option 1 and fix it later.
What do you mean with the inconvenient use? I think they are aware of their bad structure. Is there anything else?
There was a problem hiding this comment.
Do you get the SVD errors with CANFD? Because for me it works fine
Maybe that is a sign my submodule/chiptool build is old.
What do you mean with the inconvenient use?
I am referring to structure there. But it is still usable.
There was a problem hiding this comment.
I checked the IDE XML definitions for CANFD to see if it differed and it looks to have the same layout as the SVDs with the TI_WRAPPER.
There was a problem hiding this comment.
Sadly it seems that I don't have time to support the chiptool issue.
We could think about moving forward with the TI_WRAPPER.
There was a problem hiding this comment.
Did you need the canfd driver done in a relatively short time frame? If so I can try to find some time to see if I could make chiptool transforms to merge up TI_WRAPPER.
There was a problem hiding this comment.
Nah, don't need to rush this.
Just thought it would be nice to finish it. But I don't need this any time soon.
3 participants
Continuation of #2