-
Notifications
You must be signed in to change notification settings - Fork 930
Added Corundum support for ADRV9009ZU11EG #3049
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
base: main
Are you sure you want to change the base?
Conversation
- Added three new build params. for ZynqMp defconfig to support the Corundum features - As of now, Corundum support is only available for ADRV9009ZU11EG/ADRV2CRR variant of the project - arch: arm64: dts: xilinx: zynqmp-adrv9009-zu11eg-reva-adrv2crr-fmc-reva.dts : added the settings for Corundum to work (they also work with the default project) - arch: arm64: dts: xilixn: zynqmp-adrv9009-zu11eg-revb-adrv2crr-fmc-revb-jesd204-fsm-100-qsfp.dts: the corresponding devicetree Corundum on ADRV9009ZU11EG Signed-off-by: Cristian Mihai Popa <cristianmihai.popa@analog.com>
stefpopa
left a comment
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.
A few clarifications are required, otherwise, looks good.
| resets = <&zynqmp_reset 117>; // ZYNQMP_RESET_PS_PL1 | ||
| reset-names = "reset"; | ||
| }; | ||
| }; No newline at end of file |
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.
you have a missing newline at the end of this file
| assigned-clock-rates = <10000>, <1413120000>, <30720000>; | ||
| <&ad9545_clock AD9545_CLK_PLL AD9545_PLL1>, | ||
| <&ad9545_clock AD9545_CLK_OUT AD9545_Q1B>; | ||
| assigned-clock-rates = <10000>, <1562500000>, <156250000>; |
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.
your commit does not explain why these changes are needed. Is this a breaking change for the configurations that don't use corundum?
| ad9545_apll0: pll-clk@AD9545_PLL0 { | ||
| reg = <AD9545_PLL0>; | ||
| ad9545_apll1: pll-clk@AD9545_PLL1 { | ||
| reg = <AD9545_PLL1>; |
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.
same comment as above, this commit does not explain why this change is needed.
| reg = <0x0 0xa0000000 0x1000000>; | ||
| reg-names = "csr"; | ||
| interrupt-parent = <&gic>; | ||
| interrupts = <0 93 1>; |
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 interrupt line is hardcoded. Magic number 93 without explanation, flag value 1 should use symbolic constants
|
|
||
| / { | ||
| model = "Analog Devices ADRV9009ZU11EG Corundum support"; | ||
| /delete-node/ leds; |
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.
why delete LEDs? Please add an explanation. Is it related to the corundum?
PR Description
Corresponding HDL PR: analogdevicesinc/hdl#1971
PR Type
PR Checklist