Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Target hooks #1942

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

AndrewD
Copy link
Collaborator

@AndrewD AndrewD commented Apr 26, 2024

This allows the target to supply custom target_init() and target_boot() functions, via a library linked with the bios. eg:

    # add custom target specific library to the bios:
    src="libtarget"
    src_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), src))
    builder.add_software_package(src, src_dir)
    builder.add_software_library(src)

@enjoy-digital
Copy link
Owner

Thanks @AndrewD, this looks good. I'll review it soon.

Comment on lines +15 to +16
extern void target_init(void) __attribute__((weak));
extern void target_boot(void) __attribute__((weak));
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not something like:

void target_nop() {}
void target_init(void) __attribute__((weak, alias("target_nop")));
void target_boot(void) __attribute__((weak, alias("target_nop")));

to avoid having to test if function exist

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes that's another alternative I onsidered.I like the fact that the test makes it's clear that it's optional, but that's personal preference.
There doesn't appear to have been any goal to minimise code size in the bios so i opted for clarity.

@trabucayre
Copy link
Collaborator

For target_boot and target_init I think its a good idea to allows target/specific libraries to do some tasks, at some points, without having to modify existing code.
But for check_image_in_flash and copy_image_from_flash_to_ram its less clear. Is it to allows custom code to check and load firmware for a custom boot mode more or less similar to flashboot() ?

@AndrewD
Copy link
Collaborator Author

AndrewD commented May 31, 2024

But for check_image_in_flash and copy_image_from_flash_to_ram its less clear. Is it to allows custom code to check and load firmware for a custom boot mode more or less similar to flashboot() ?

Yes: for example a target_boot() that can validate a dtd, copy a kernel and copy then run opensbi is one such usage.

@AndrewD
Copy link
Collaborator Author

AndrewD commented May 31, 2024

I would also like to rename for check_image_in_flash and copy_image_from_flash_to_ram but didn't to minimise the diff

Andrew Dennison and others added 3 commits July 29, 2024 09:25
  - It's a 64bit not 32bit unsigned and it caused overflow/negative
    cycles display.
This allows the target to supply custom target_init() and target_boot()
functions, eg:

    # add custom target specific library to the bios:
    src="libtarget"
    src_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), src))
    builder.add_software_package(src, src_dir)
    builder.add_software_library(src)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants