-
Notifications
You must be signed in to change notification settings - Fork 206
Update Zynq portable for Xilinx SDT drivers #1227
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
Conversation
|
Thanks for contributing to FreeRTOS+TCP. |
| #define ZYNQ_SCUGIC_0_BASEADDR XPAR_PS7_SCUGIC_0_BASEADDR | ||
| #define ZYNQ_ETHERNET_0_BASEADDR XPAR_PS7_ETHERNET_0_BASEADDR | ||
| #if ( XPAR_XEMACPS_NUM_INSTANCES > 1 ) | ||
| #define ZYNQ_ETHERNET_1_BASEADDR XPAR_PS7_ETHERNET_1_BASEADDR |
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.
When I'm building on an environment that doesn't have SDT defined and XPAR_XEMACPS_NUM_INSTANCES > 1, there is a build failure:
Description Resource Path Location Type
'XPAR_PS7_ETHERNET_1_DEVICE_ID' undeclared here (not in a function); did you mean 'XPAR_PS7_ETHERNET_0_DEVICE_ID'? NetworkInterface.c /aws_demos/libraries/freertos_plus/standard/freertos_plus_tcp/source/portable/NetworkInterface/Zynq line 185 C/C++ Problem
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 should be resolved after adding the defines back in.
| .DeviceId = XPAR_PS7_ETHERNET_1_DEVICE_ID, /**< Unique ID of device */ | ||
| .BaseAddress = XPAR_PS7_ETHERNET_1_BASEADDR /**< Physical base address of IPIF registers */ | ||
| #ifndef SDT | ||
| .DeviceId = XPAR_PS7_ETHERNET_1_DEVICE_ID, /**< Unique ID of device */ |
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.
When I'm building on an environment that doesn't have SDT defined and XPAR_XEMACPS_NUM_INSTANCES > 1, there is a build failure:
Description Resource Path Location Type
'XPAR_PS7_ETHERNET_1_DEVICE_ID' undeclared here (not in a function); did you mean 'XPAR_PS7_ETHERNET_0_DEVICE_ID'? NetworkInterface.c /aws_demos/libraries/freertos_plus/standard/freertos_plus_tcp/source/portable/NetworkInterface/Zynq line 185 C/C++ Problem
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 should be resolved after adding the defines back in.
| #endif | ||
|
|
||
| #define XPAR_PS7_ETHERNET_1_DEVICE_ID 1 | ||
| #define XPAR_PS7_ETHERNET_1_BASEADDR 0xE000C000 |
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.
Any particular reason these macros are removed now?
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.
Thanks for the comment. This must be the cause of the other errors so I will comment here.
I should have mentioned that my dev board only has 1 ethernet, so I wasn't able to check this case. XPAR_PS7_ETHERNET_0_DEVICE_ID and XPAR_PS7_ETHERNET_0_BASEADDR are defined in the xilinx file xparameters.h. I had assumed that xilinx would provide these definitions for additional ethernet, but maybe it is a bug that they are missing from the xilinx file. The deleted lines may have been a workaround for that.
I could look at your xparameters.h file if you post it. But it sounds like XPAR_PS7_ETHERNET_1_DEVICE_ID is missing. In that case it is no problem to revert the deletion. Is XPAR_PS7_ETHERNET_1_BASEADDR also missing from the xparameters.h?
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.
But it sounds like XPAR_PS7_ETHERNET_1_DEVICE_ID is missing. In that case it is no problem to revert the deletion. Is XPAR_PS7_ETHERNET_1_BASEADDR also missing from the xparameters.h?
Yes, both macros are missing; I just defined XPAR_XEMACPS_NUM_INSTANCES to 2 to just check the build. I believe ideally the xparameters.h should be updated with macros.
For backward compatibility for anyone already using the library with XPAR_XEMACPS_NUM_INSTANCES > 1, I believe it would be good to keep those macros optionally defined if they're not defined in xparameters.h maybe with #ifndef's.
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.
I have put the defines back in with #ifndef. They will be active when SDT is not defined.
Description
Sometime around 2023, Xilinx started introducing breaking changes to their drivers along with their new IDE. As an example you can look at this file where they use the macro SDT to detect whether the new or old drivers are being used: https://github.com/Xilinx/embeddedsw/blob/master/XilinxProcessorIPLib/drivers/emacps/examples/xemacps_example_intr_dma.c
This PR is updating the Zynq portable code so that it works with both the new and old drivers.
Test Steps
I have tested on real hardware for a few days now. Testing included using both the IDE with new drivers (Vitis 2024.2) and old drivers (Vitis Classic 2024.2).
The changes are all within the portable layer, so I don't believe they are covered by any unit tests.
Checklist:
Related Issue
I posted some more details in this discussion: #1225
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.