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

Microchip vendor files migration process #20457

Open
dylad opened this issue Mar 12, 2024 · 8 comments
Open

Microchip vendor files migration process #20457

dylad opened this issue Mar 12, 2024 · 8 comments
Labels
Community: help wanted The contributors require help from other members of the community

Comments

@dylad
Copy link
Member

dylad commented Mar 12, 2024

It is time again to upgrade our Microchip vendor files for our supported SAM0 devices. Unfortunately this time, things will be a little bit more trickier than before as Microchip decided to rework their vendor files:

  • Support for structures bitfields dropped.
  • Peripheral structures now utilize u8, u16, u32 registers instead of individual register structures (due to the drop of bitfields support).
  • Peripherals structure were also renamed (At least they are no longer in camel case so they fit our coding convention yay!)
  • Numerous small changes that will require tedious adjustments.

So why should we do it ?
Because bug fixes, new silicon version added, but above all, to be able to add support for new families of microcontrollers from Microchip.

Migration plan:

  • Option 1
    Create a dedicated branch on RIOT-OS/RIOT repo, push all changes there kindly, and create a gigantic PR that will be painful funny to review.
    Obviously, this is something I'd like to avoid.
  • Option 2:
    Proceed by smaller steps across several releases even if it will increase the overall amount of work.
    To do so, I propose the following steps:
    • Eliminate all usage of bitfields in our codebase.
    • Develop wrappers/macros to maintain backward compatibility for register access in all board and peripheral drivers (although this is not enjoyable).
    • Introduce new vendor headers into cpu/sam0_common/include/vendor_ng (these will NOT be included in compilation yet).
    • Gradually migrate each family to observe issues and address them diligently.
    • Remove old vendor headers.
    • Undo all changes related to register access modifications.

I am well aware, this will add significant amount of work but IMO, this is the price to pay to keep PR size reasonable. This will also prevent issue for people maintaining boards outside of the tree through EXTERNAL_BOARD_DIRS

For those curious about the changes, you will find all Microchip atpack here

Download the archive of any supported family, and compare the content of the include/ folder against cpu/sam0_common/include/vendor/XXXX

Obviously, all of this is debatable, and I'll take all ideas to reduce this burden.

@dylad dylad added the Community: help wanted The contributors require help from other members of the community label Mar 12, 2024
@dylad
Copy link
Member Author

dylad commented May 30, 2024

After some thoughts, here is my proposal regarding this migration:

  • Remove all bitfields usage in our codebase (no more foo->bar.bit.xyz only foo->bar.reg)
  • Creation of a new header for missing declarations
  • Replace all single bit field using new declarations from the newly generated header (FOO_BAR_BITFIELD -> FOO_BAR_BITFIELD_Msk)
  • Creation of a wrapper for register calls
  • Make use of new wrapper in boards/ and cpu/
  • Add a deprecation notice during build
  • Add new Microchip vendor files under cpu/sam0_common/include/vendor_ng
  • use AT BOARD LEVEL a new variable to switch to new headers
  • Starting cleanup after one or two releases

@dylad
Copy link
Member Author

dylad commented May 30, 2024

Regarding step 2 & 3:
These are mandatory because of the way Microchip build their new vendor files.
Currently when the bitfield inside a register is more than 1 bit. We have

FOO_BAR_Pos
FOO_BAR_Msk
FOO_BAR(value)

This is the same AFAICT on new vendor files.
However, there is a difference when the bitfield is only one bit wide.
For example RTC_CTRLA_ENABLE (current header we have) is now spelled RTC_CTRLA_ENABLE_Msk on new vendor.
On new vendor files, Microchip decided to unify the use of _Msk for every bitfields.

This is why I propose to introduce a new header, which will be generated though a script.
The idea will be to parsed the current headers, look for both FOO_BAR_Pos and FOO_BAR_Msk. If both exist, we're good. If only _Pos exists, then generated a _Msk so we can match the behavior of the new vendor files.
Of course, this header will be dropped at the end of the migration process during the cleanup.

@dylad
Copy link
Member Author

dylad commented May 30, 2024

About the wrapper, I was thinking about something like

#if USE_MCHP_VENDOR_NG
  #define REG(dev, offset) ((dev)->dev##_##offset)
#else
  #define REG(dev, offset) ((dev)->offset.reg)
#endif

On new header, Microchip slightly change the new of every registers inside a peripheral.
For instance
RTC->CTRLA.reg (current header) should be use as RTC->RTC_CTRLA (on new header).
With the proposed macro, it will be REG(RTC, CTRLA)

I think such macro will allow us to avoid some pain.

@kfessel
Copy link
Contributor

kfessel commented May 31, 2024

Microchip is right in their decision to drop bitfields that is to much comfort.
Changing bits should be a pain in the rear end with macros all over place.

The only positive thing about bitfield removal is that this might make more people aware that changing a bit often is a register read modify write ( and since they a usually marked volatile each bit modification has their own). But for that they would not need to also drop the structures just the mapping to addresses.

@dylad
Copy link
Member Author

dylad commented May 31, 2024

Changing bits should be a pain in the rear end with macros all over place.

Hopefully, all those macros will be there for a given time. This way, users will have time to adapt their out of the tree code.
However, I am planning on cleaning up everything (and thus remove all macros) after one or two releases.
So yeah, it will take time but in the end, it will look "as before".

@kfessel
Copy link
Contributor

kfessel commented May 31, 2024

maybe they just want to kill of c development by droping the wins of 15 year and go back to 1990s C

@dylad
Copy link
Member Author

dylad commented May 31, 2024

maybe they just want to kill of c development by droping the wins of 15 year and go back to 1990s C

I think, this is related to their recent migration to MPLABX and their will to unify SAM0 to PIC32.
I've complained to them (through a ticket) about this. Especially doing this WITHOUT deprecation notice.
They acknowledge the issue but they were like "it is what it is"

@kfessel
Copy link
Contributor

kfessel commented May 31, 2024

sounds little like they dislike their customers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community: help wanted The contributors require help from other members of the community
Projects
None yet
Development

No branches or pull requests

2 participants