Skip to content

Conversation

@glneo
Copy link
Contributor

@glneo glneo commented Apr 15, 2024

This PR adds SoC support for TI J721e, specifically the R5 cores found in the MAIN domain (there is an R5 in the MCU domain we will add support for later). We also add board support for the Beaglebone AI64 which uses the J721e SoC.

TRM for J721e: https://www.ti.com/lit/zip/spruil1
BeagleBone AI-64: https://beagleboard.org/ai-64

Note: this is the continuation of the PR #59191 which went stale and was closed. Updates include HWMv2 and other small bug fixes. This is based on #71526 which is the first patch here (included here also so this series can still build).

@zephyrbot zephyrbot added area: Devicetree Binding PR modifies or adds a Device Tree binding platform: TI K3 Texas Instruments Keystone 3 Processors platform: TI SimpleLink Texas Instruments SimpleLink MCU area: Timer Timer area: Pinctrl platform: BeagleBoard BeagleBoard.org Foundation labels Apr 15, 2024
Copy link
Contributor

@gramsay0 gramsay0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good, mostly questions on things I'm uncertain on

@glneo glneo force-pushed the ai64-r5 branch 2 times, most recently from 0a82a6f to 16ae360 Compare April 22, 2024 14:29
@glneo glneo force-pushed the ai64-r5 branch 2 times, most recently from 8d0b8ce to eb8019b Compare May 1, 2024 14:24
dnltz
dnltz previously approved these changes May 1, 2024
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this not XIP? Is there some relocation happening?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda, we don't have FLASH memory to execute from, we always run out of RAM. Setting XIP causes some other issues as we do not have a hard-coded flash memory region. At some point we could consider the location Linux loads the binary to as the "flash" region, but that would need other refactoring and since we are loaded into memory that is not ROM we don't get much from XIP being set.

@zephyrbot zephyrbot added the area: UART Universal Asynchronous Receiver-Transmitter label Oct 11, 2024
slpp95prashanth and others added 3 commits October 12, 2024 11:44
Add initial SoC support for the TI J721E SoC series Cortex-R5 core.

TRM for J721e https://www.ti.com/lit/zip/spruil1
File: spruil1c.pdf

Signed-off-by: Prashanth S <[email protected]>
Signed-off-by: Andrew Davis <[email protected]>
Add initial BeagleBone AI-64 support.

BeagleBone AI-64: https://www.beagleboard.org/boards/beaglebone-ai-64

Signed-off-by: Prashanth S <[email protected]>
Signed-off-by: Andrew Davis <[email protected]>
The default configuration for PINCTRL should not be set with
the other default configurations in .defconfig, instead select
a default value as part of defining the UART driver.

Signed-off-by: Andrew Davis <[email protected]>
@jadonk
Copy link
Member

jadonk commented Oct 15, 2024

@nordicjm @gmarull have you reviewed since latest push? Do we have a good pattern for PINCTRL? I'll be working with @Ayush1325 and all to get fixes for other Beagles.

@nordicjm
Copy link
Contributor

@nordicjm @gmarull have you reviewed since latest push? Do we have a good pattern for PINCTRL? I'll be working with @Ayush1325 and all to get fixes for other Beagles.

Drivers that need it must select it, it must not be in any defconfig files. If a board needs it for a board's .c file then it must be selected by the board Kconfig file

Copy link
Member

@jadonk jadonk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please read my question on SOC vs. SOC_PART_NUMBER -- not sure why they are different. This isn't a gate to my approval, just a query.

config SOC
default "am6234" if SOC_AM6234_M4 || SOC_AM6234_A53
default "am6442" if SOC_AM6442_M4
default "j721e" if SOC_J721E_MAIN_R5F0_0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems inconsistent with soc/ti/k3/am6x/Kconfig SOC_PART_NUMBER. Should it be leading caps, all caps or lower-case? Why would the SOC_PART_NUMBER be different than SOC?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For SOC I found the following in the porting guide:

config SOC
    default "SoC name" if SOC_<SOC_NAME>

Notice that ``SOC_NAME`` is a pure upper case version of the SoC name.

Notice that the string value must match the SoC name used in the :file:`soc.yml` file.

To me that seems to indicate all lower to match what we do in soc.yml.

For SOC_PART_NUMBER, to be honest I have no clue, I don't see it used anywhere either, I just added it based on what the other TI SoCs had done, but maybe they didn't need it either?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SOC must be lower, SOC_PART_NUMBER does not exist, i.e. it's a custom field, it has no relation to anything and is not used by anything, it can be safely removed unless you have HAL code using it

@jadonk
Copy link
Member

jadonk commented Oct 21, 2024

@gmarull it seems the updates were made. Any changes remaining?

@gmarull gmarull dismissed their stale review October 21, 2024 07:04

addressed

@kartben kartben assigned jadonk and unassigned gmarull Oct 23, 2024
@aescolar aescolar merged commit 9d0da02 into zephyrproject-rtos:main Oct 23, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Devicetree Binding PR modifies or adds a Device Tree binding area: Pinctrl area: UART Universal Asynchronous Receiver-Transmitter platform: BeagleBoard BeagleBoard.org Foundation platform: TI K3 Texas Instruments Keystone 3 Processors platform: TI SimpleLink Texas Instruments SimpleLink MCU

Projects

None yet

Development

Successfully merging this pull request may close these issues.