-
Notifications
You must be signed in to change notification settings - Fork 86
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
Initial i.MX93 support #189
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Indan Zupancic <[email protected]>
Same LPUART as imx8qxp. Timers seem new and needs a new driver. For now, use ARM generic timer when KernelArmExportPCNTUser and KernelArmExportPTMRUser are enabled. Signed-off-by: Indan Zupancic <[email protected]>
As far as we know, you added support for the low-power timer already. Is it only the case that this comment is out of date now? |
The corresponding commit is still missing on this branch. (Not that it's required for the port, it would just be a nice additional feature to have). |
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.
This looks good from my side. It would be nice to add the LPTR even just for convenience. Happy for it to go into this PR or a separate one.
More detail in my mail. To summarize, at least on the MCIMX93-SOMB2 module the 32 kHz clock is significantly out-of-sync with the 24 MHz clock (assuming they didn't put a 25 MHz crystal on by mistake), most likely because the 24 MHz clock is inaccurate by about 2.5%. This makes the SCHED0011 test fail because it relies on the clocks to be mostly in sync. Another issue is that taking a timestamp takes 0.5 us, which is extremely slow. Basically we have four choices:
Currently I have a driver with a define at the top that chooses between the two clock sources. |
I pushed the LPTMR, I made the clock source a config option, with the 24 MHz source as default to avoid breaking sel4test. |
The clock mystery also has been solved: I measured the clocks with a borrowed oscilloscope and that proofs that the 24MHz is fine, but the 32 kHz one is garbage. They put a 32.768 kHz crystal on the dev board instead of a 32 kHz one, probably because it's cheaper. That explains the 2.4% difference I see. They even documented it in MCIMX93EVKUM.pdf and the schematics, but I missed it before. I'm going to complain about this to NXP. Those dev boards are expensive, so you'd expect them to not cut corners like this. If they do things like this, you can as well get a much cheaper, but probably higher quality i.MX93 board from someone else instead. I've updated the commit message to document this crystal problem. |
Never mind, I'm just stupid: The 32kHz crystal is supposed to be 32.768 kHz. Updating the driver to that and change to the 32 kHz cock source as default. I've been misled by 32kHz dividing one second in a nice round nanosecond number, but that's not what the crystal value was chosen for, it was chosen to divide one second in a power-of-two number of ticks. |
Only used if the Arm generic timer cannot be used. The clock source is configurable. The default is the 32.768 kHz clock, as that is more accurate for timekeeping and keeps running when the board is in low power state and can wake up the CPU. Retrieving the time takes about 0.5 us, so this timer is very slow. Both LPMTR1 and LPTMR2 are used because you are not allowed to change the compare value while the timer is running. Signed-off-by: Indan Zupancic <[email protected]>
Could someone review my timer driver code? I'm not very familiar with the util_libs' driver framework, it's a bit confusing. Other timer drivers seem to be either over complicating things, or I'm missing something. |
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.
Happy with the updates and with the figured out mystery of the 32 kHz clock. The driver code as such looks good to me, but I can't say I'm an expert on these.
Works with the ARM generic timer when
KernelArmExportPCNTUser
andKernelArmExportPTMRUser
are enabled. The Low Power Timer driver is in progress and will be added soon.