-
Notifications
You must be signed in to change notification settings - Fork 8.3k
boards: ti: lp_am243x: initial support #91986
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
|
Hello @spencerjcoward, and thank you very much for your first pull request to the Zephyr project! |
|
@natto1784 please take a look. |
|
Pinging @m-braunschweig, as they have also worked on adding extensive AM243LP support in their fork |
m-braunschweig
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.
Seems mosty good except some old EVM references. I have problems running the blinky sample with additionally CONFIG_LOG and CONFIG_ASSERT enabled. Since the Data Fault Address Register held an address in the external RAM memory space I assume that something like RSC tried to log data and this resulted in a fault.
Aditionally I think it might be better to name the SoC before the fact that it's a launchpad so it's consistent with the 243evm.
Also as Natto mentioned I also worked on adding Zephyr support for this board (that's also intended to be added upstream). However I didn't take a look at the M4 processor core and ADC/I2C so I think it would be better to continue with this PR
| The board physically contains: | ||
|
|
||
| - Memory. | ||
|
|
||
| - 256KB of SRAM | ||
|
|
||
| - Debug | ||
|
|
||
| - XDS110 based JTAG |
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 is not right for the AM243x LaunchPad. It has instead
- 512 Mb of Infineon NOR Flash connected via QuadSPI
- DIP switches to change the boot mode
- Buttons to reset parts of the SoC in different ways
- A push button connected to a GPIO pin
- Multiple LEDs (some for power indication and some connected to GPIO pins)
- A XDS110 debug probe (JTAG emulation)
| DDR RAM | ||
| ------- | ||
|
|
||
| The board has 2GB of DDR RAM available. This board configuration allocates: | ||
|
|
||
| - 4KB Resource Table at 0xa4100000 for M4 | ||
| - 4KB Resource Table at 0xa0100000 for R5F0_0 | ||
| - 8MB Shared Memory at 0xa5000000 for inter-processor communication |
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.
There is no external RAM connected ot the AM243x Launchpad; the 0xaXXXXXXX memory area is where external RAM would be accessible
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 see that i missed these non existent ddr locations still being referenced in the dts, how would you like me to resolve the current need for RSC_TABLE? SOC_SERIES_AM6X_M4 selects this in Kconfig, should i reserve a piece of sram to contain these?
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.
also i did do a test where i remove all the nodes referencing the external ddr (ipc, shm, rsc table) and put OPENAMP_RSC_TABLE=n in the defconfig for the m4 and then i am able to turn on CONFIG_LOG and CONFIG_ASSERT.
Would it be best to leave the RSC table and IPC for another PR and just disable that feature for 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.
I think for RSC / ipc it would be good to put the memory regions into one of the unused SRAM areas (based on the default layout of the TI MCU+ SDK). If you want to do it in this PR on in another PR is up to you.
also i did do a test where i remove all the nodes referencing the external ddr (ipc, shm, rsc table) and put OPENAMP_RSC_TABLE=n in the defconfig for the m4 and then i am able to turn on CONFIG_LOG and CONFIG_ASSERT.
I will create a separate comment for this since there are multiple problems (also in already existing code). Please also note that I'm focussing on using Zephyr on the Cortex-R5 and no the M4
| .. list-table:: UART Boot Mode | ||
| :header-rows: 1 | ||
|
|
||
| * - SW2 [1:7] |
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.
On the LaunchPad there is only one switch row named SW4 fot selecting the boot mode
| .. list-table:: OSPI Boot Mode | ||
| :header-rows: 1 | ||
|
|
||
| * - SW2 [1:7] |
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 SW4 comment
| rsc_table: memory@a4100000 { | ||
| compatible = "zephyr,memory-region", "mmio-sram"; | ||
| reg = <0xa4100000 DT_SIZE_K(4)>; | ||
| zephyr,memory-region = "RSC_TABLE"; | ||
| }; | ||
|
|
||
| ipc_shm0: memory@a5000000 { | ||
| compatible = "zephyr,memory-region", "mmio-sram"; | ||
| reg = <0xa5000000 DT_SIZE_M(8)>; | ||
| zephyr,memory-region = "IPC_SHM"; | ||
| }; |
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 points to non-existant external DRAM
| adc0 = &adc0; | ||
| }; | ||
|
|
||
| leds: 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.
There are more LEDs on the board but at least one can't be controlled due to #81100.
It might be a good idea to mention that at least as comment
| rsc_table: memory@a0100000 { | ||
| compatible = "zephyr,memory-region", "mmio-sram"; | ||
| reg = <0xa0100000 DT_SIZE_K(4)>; | ||
| zephyr,memory-region = "RSC_TABLE"; | ||
| zephyr,memory-attr = <( DT_MEM_ARM(ATTR_MPU_RAM) )>; | ||
| }; | ||
|
|
||
| ipc_shm0: memory@a5000000 { | ||
| compatible = "zephyr,memory-region", "mmio-sram"; | ||
| reg = <0xa5000000 DT_SIZE_M(8)>; | ||
| zephyr,memory-region = "IPC_SHM"; | ||
| zephyr,memory-attr = <( DT_MEM_ARM(ATTR_MPU_IO) )>; | ||
| }; |
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.
External RAM doesn't exist
| CONFIG_UART_CONSOLE=y | ||
|
|
||
| # Enable MPU | ||
| CONFIG_ARM_MPU=y |
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.
Ideally the MPU should be generally enabled for the AM2434 and not for every board that supports it manually. However the Kconfig needs some cleanup anyway IMO so I wouldn't do it in this PR
Add support for the TI AM243x LaunchPad board. This board is based on the work done for the am243x_evm board, with modifications to the device tree and board configuration files to suit the LaunchPad's hardware. With reduced image size for board. Signed-off-by: Spencer J. Coward <[email protected]>
Address comments from PR 91986, relating to the formatting of the board.cmake file and the dtsi files for the lp_am243x board. Signed-off-by: Spencer J. Coward <[email protected]>
|
An offtopic suggestion: Maybe we ought to put the C-file MPU configuration in |
|
I had a look at running this PR on the R5 core (board qualifier:
Also: These are mostly existing Kconfig problems that were already mentioned in the EVM PR but postponed, so the scope of the PR for initial AM2434 support didn't increase further. I don't personally know what the best way would be to go forward with this (on one hand the Kconfig things should be fixed and on the other hand the scope of this PR shouldn't be increased (too much)). If you are ok with it and want it I could create a commit that you can include into this PR via cherry-picking. However it would also be possible to clean up the Kconfig in a separate PR either before or after this is merged. I would like to hear comments from the other people in this PR what they think the best solution would be |
I personally think the The mix between MPU configuration inside the devicetree and an additional configuration file also seems a bit weird to me, however when I looked last time it seemed like most platforms preferred configuration inside the devicetree. The problem is that it seems like there are no general guidelines on what should be preferred. Though the |
|
Yes it is indeed legacy. The problem with using |
af2e856 to
eb68a98
Compare
|
|
|
||
| if(CONFIG_SOC_AM2434_M4) | ||
| board_runner_args(openocd "--no-init" "--no-halt" "--no-targets" "--gdb-client-port=3338") | ||
| board_runner_args(openocd "--no-init" "--no-halt" "--no-targets" "--gdb-client-port=3338") |
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.
no fixup commits, squash
| channel@0 { | ||
| reg = <0>; | ||
| ti,open-delay = <0>; | ||
| zephyr,gain = "ADC_GAIN_1"; | ||
| zephyr,reference = "ADC_REF_INTERNAL"; | ||
| zephyr,acquisition-time = <0>; | ||
| zephyr,resolution = <12>; | ||
| zephyr,oversampling = <4>; | ||
| }; | ||
|
|
||
| channel@1 { | ||
| reg = <1>; | ||
| ti,open-delay = <0>; | ||
| zephyr,gain = "ADC_GAIN_1"; | ||
| zephyr,reference = "ADC_REF_INTERNAL"; | ||
| zephyr,acquisition-time = <0>; | ||
| zephyr,resolution = <12>; | ||
| zephyr,oversampling = <4>; | ||
| }; | ||
|
|
||
| channel@2 { | ||
| reg = <2>; | ||
| ti,open-delay = <0>; | ||
| zephyr,gain = "ADC_GAIN_1"; | ||
| zephyr,reference = "ADC_REF_INTERNAL"; | ||
| zephyr,acquisition-time = <0>; | ||
| zephyr,resolution = <12>; | ||
| zephyr,oversampling = <4>; | ||
| }; | ||
|
|
||
| channel@3 { | ||
| reg = <3>; | ||
| ti,open-delay = <0>; | ||
| zephyr,gain = "ADC_GAIN_1"; | ||
| zephyr,reference = "ADC_REF_INTERNAL"; | ||
| zephyr,acquisition-time = <0>; | ||
| zephyr,resolution = <12>; | ||
| zephyr,oversampling = <4>; | ||
| }; | ||
|
|
||
| channel@4 { | ||
| reg = <4>; | ||
| ti,open-delay = <0>; | ||
| zephyr,gain = "ADC_GAIN_1"; | ||
| zephyr,reference = "ADC_REF_INTERNAL"; | ||
| zephyr,acquisition-time = <0>; | ||
| zephyr,resolution = <12>; | ||
| zephyr,oversampling = <4>; | ||
| }; | ||
|
|
||
| channel@5 { | ||
| reg = <5>; | ||
| ti,open-delay = <0>; | ||
| zephyr,gain = "ADC_GAIN_1"; | ||
| zephyr,reference = "ADC_REF_INTERNAL"; | ||
| zephyr,acquisition-time = <0>; | ||
| zephyr,resolution = <12>; | ||
| zephyr,oversampling = <4>; | ||
| }; | ||
|
|
||
| channel@6 { | ||
| reg = <6>; | ||
| ti,open-delay = <0>; | ||
| zephyr,gain = "ADC_GAIN_1"; | ||
| zephyr,reference = "ADC_REF_INTERNAL"; | ||
| zephyr,acquisition-time = <0>; | ||
| zephyr,resolution = <12>; | ||
| zephyr,oversampling = <4>; | ||
| }; | ||
|
|
||
| channel@7 { | ||
| reg = <7>; | ||
| ti,open-delay = <0>; | ||
| zephyr,gain = "ADC_GAIN_1"; | ||
| zephyr,reference = "ADC_REF_INTERNAL"; | ||
| zephyr,acquisition-time = <0>; | ||
| zephyr,resolution = <12>; | ||
| zephyr,oversampling = <4>; | ||
| }; | ||
| }; |
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.
These should not be here, I made a PR to change that for am243x evm as well: #92170
| gpio0_led1: test_led1_green { | ||
| pinmux = <K3_PINMUX(0x58, PIN_OUTPUT, MUX_MODE_7)>; | ||
| }; | ||
| }; 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.
missing newline
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |



Add support for the TI AM243x LaunchPad board. This board is based on the work done for the am243x_evm board, with modifications to the device tree and board configuration files to suit the LaunchPad's hardware.