Skip to content

Conversation

@slpp95prashanth
Copy link
Contributor

@slpp95prashanth slpp95prashanth commented Jun 13, 2023

This PR (non-working port) adds device tree and soc support for TI J721e_r5 and Beaglebone AI64 board support for hello_world application.

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

BeagleBone AI_64 https://beagleboard.org/ai-64

Signed-off-by: Prashanth S [email protected]

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hello @slpp95prashanth, and thank you very much for your first pull request to the Zephyr project!

A project maintainer just triggered our CI pipeline to run it against your PR and ensure it's compliant and doesn't cause any issues. You might want to take this opportunity to review the project's Contributor Expectations and make any updates to your pull request if necessary. 😊

@slpp95prashanth slpp95prashanth force-pushed the zephyr branch 4 times, most recently from 2a833bc to 352efc5 Compare June 14, 2023 04:58
@vaishnavachath vaishnavachath changed the title Add TI J721e_r5 and BeagleBone AI64 hello_world support Add TI J721e R5 and BeagleBone AI64 R5 initial support Jun 14, 2023
@slpp95prashanth slpp95prashanth force-pushed the zephyr branch 3 times, most recently from b248774 to a17f9b7 Compare June 16, 2023 08:14
Copy link
Member

@cfriedt cfriedt left a comment

Choose a reason for hiding this comment

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

I thought that filenames in the board directory should match the board name. I think that's a convention.

@slpp95prashanth slpp95prashanth force-pushed the zephyr branch 3 times, most recently from 173ac96 to ac938e2 Compare June 20, 2023 14:07
@zephyrbot zephyrbot added area: Interrupt Controller area: Devicetree Binding PR modifies or adds a Device Tree binding labels Jun 20, 2023
@slpp95prashanth slpp95prashanth force-pushed the zephyr branch 2 times, most recently from 513e99c to 6107782 Compare June 22, 2023 08:01
Copy link
Contributor

@carlocaione carlocaione left a comment

Choose a reason for hiding this comment

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

I'm quite strongly opinionated about adding support in-tree for a board without an interrupt controller and only with the hello world sample running. The feature set for a new SoC / board should be enough to at least pass the kernel tests.

@zephyrbot zephyrbot added the platform: TI K3 Texas Instruments Keystone 3 Processors label Aug 18, 2023
Copy link
Contributor

@carlocaione carlocaione left a comment

Choose a reason for hiding this comment

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

I'm tired of this.

Did you at least try to compile this?

# west build -b beagle_bbai64_r5 samples/hello_world -p

dts/arm/ti/j721e.dtsi:10:10: fatal error: zephyr/dt-bindings/interrupt-controller/ti-vim.h: No such file or directory
   10 | #include <zephyr/dt-bindings/interrupt-controller/ti-vim.h>

I honestly feel you are wasting mine and maintainers time here. I'm going to remove myself from this PR.

Comment on lines 16 to 19
Copy link
Contributor

Choose a reason for hiding this comment

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

remove if not needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@carlocaione j721e_init is passed to SYS_INIT as a initialisation function.

Copy link
Member

Choose a reason for hiding this comment

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

+1, drop if empty

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we pass NULL as a first argument to SYS_INIT()?

Copy link
Member

Choose a reason for hiding this comment

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

what's the point of a no-op SYS_INIT...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing SYS_INIT() and j721e_init().

Copy link
Member

Choose a reason for hiding this comment

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

@slpp95prashanth , sorry for the late response, but you will need to atleast unlock the CTRL MMR regions during, all the necessary regions might not always be unlocked by bootloader for you.
See https://github.com/zephyrproject-rtos/zephyr/blob/main/soc/arm/ti_k3/am62x_m4/soc.c#L47

@carlocaione carlocaione requested review from carlocaione and removed request for carlocaione August 21, 2023 06:51
@slpp95prashanth
Copy link
Contributor Author

slpp95prashanth commented Aug 21, 2023

I'm tired of this.

Did you at least try to compile this?

# west build -b beagle_bbai64_r5 samples/hello_world -p

dts/arm/ti/j721e.dtsi:10:10: fatal error: zephyr/dt-bindings/interrupt-controller/ti-vim.h: No such file or directory
   10 | #include <zephyr/dt-bindings/interrupt-controller/ti-vim.h>

I honestly feel you are wasting mine and maintainers time here. I'm going to remove myself from this PR.

@carlocaione
Yes I compiled and tested.

I apologize, but this PR depends on #60856 and #61020 which is mentioned in #59191 (comment).

I do not have any intention to waste yours and maintainers time, please do not say like that.

Add initial Device Tree support for the TI J721E SoC series.

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

Signed-off-by: Prashanth S <[email protected]>
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 example capitalized?

I should have read this section before commenting above. I was thinking you were talking about west flash. Can you implement west flash to flash the image and provide documentation on what it does?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I can implement west flash for this.

Comment on lines +76 to +80
Copy link
Member

Choose a reason for hiding this comment

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

why not implement a proper runner so you can west flash?

Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@KarthikL1729 can you please update this?

Comment on lines +32 to +33
Copy link
Member

Choose a reason for hiding this comment

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

-1, please make sure drivers select this

Copy link
Contributor Author

@slpp95prashanth slpp95prashanth Sep 4, 2023

Choose a reason for hiding this comment

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

@gmarull should I remove this from here, and add "select PINCTRL" in drivers.
Or select at both places.

Comment on lines 16 to 19
Copy link
Member

Choose a reason for hiding this comment

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

+1, drop if empty

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]>
Add initial BeagleBone AI-64 support

BeagleBone AI_64 https://beagleboard.org/ai-64

Signed-off-by: Prashanth S <[email protected]>
@slpp95prashanth
Copy link
Contributor Author

I am adding west flash support, I will try to add ASAP.

@slpp95prashanth slpp95prashanth marked this pull request as draft September 12, 2023 02:12
@github-actions
Copy link

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.

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: Interrupt Controller area: Pinctrl platform: TI K3 Texas Instruments Keystone 3 Processors platform: TI SimpleLink Texas Instruments SimpleLink MCU Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants