Skip to content
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.

Sign up for GitHub

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

Jump to bottom

imx6ull: boot from plo on nor flash #662

Merged
merged 1 commit into from
Jun 2, 2023
Merged

imx6ull: boot from plo on nor flash #662

merged 1 commit into from
Jun 2, 2023

Conversation

badochov
Copy link
Contributor

@badochov badochov commented Mar 15, 2023 •
edited
Loading

JIRA: RTOS-398

Description

boot from plo on nor flash.

  • phoenix-rtos-doc 4eaac00...5d1f42a (3):

    libc: add getopt, getpgid, getsid, memccpy, setenv, strncpy, truncate
    libc: documentation correction
    imx6ull: boot from nor (update submodules #139)

  • phoenix-rtos-hostutils 853e232...a56b518 (1):

    imx6ull: boot from qspi nor flash

  • plo 315360b...42bc591 (4):

    imx6ull: flash nor driver over qspi
    flash-imx6ull: Add optional logic to perform big buffer read.
    cmds: implement test-dev
    imx6ull: boot from qspi nor flash

Motivation and Context

imx6ull lacks support for starting from flash

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

How Has This Been Tested?

  • Already covered by automatic testing.
  • New test added: (add PR link here).
  • Tested by hand on: imx6ull.

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing linter checks and tests passed.
  • My changes generate no new compilation warnings for any of the targets.

Special treatment

@badochov badochov requested a review from agkaminski March 15, 2023 18:25
@badochov badochov mentioned this pull request Mar 15, 2023
Merged
13 tasks
@badochov badochov changed the title [WIP] imx6ull: boot from plo on nor flash imx6ull: boot from plo on nor flash Mar 17, 2023
@agkaminski
Copy link
Member

Converting to draft, as it needs submodule checkout before merge

@agkaminski agkaminski marked this pull request as draft March 17, 2023 12:59
@badochov badochov force-pushed the badochov/imx6ull-ploboot branch 3 times, most recently from c30b476 to ebf79a6 Compare March 20, 2023 17:28
@badochov badochov marked this pull request as ready for review March 20, 2023 17:28
@badochov badochov force-pushed the badochov/imx6ull-ploboot branch 2 times, most recently from 7bfe6c5 to baa1975 Compare March 22, 2023 10:59
nalajcie
nalajcie previously requested changes Mar 22, 2023
View reviewed changes
_targets/build.project.armv7a7-imx6ull Outdated
@@ -29,51 +29,60 @@ export PORTS_OPENVPN=y
export PORTS_JANSSON=y
export PORTS_CURL=y

#
# Boot configuration (project specific)
Copy link
Member

Choose a reason for hiding this comment

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

yeah, it should be in project build config, not in generic TARGET_SUBFAMILY one.

This file is also used by the DTR project so for now booting from NAND (with host syspage generation) needs to be retained.

The decision to use NOR/PLO as a booting device needs to be done in _projects/armv7a7-imx6ull-evk/build.project (but this file should probably support both options or keep only the common part of the booting procedure).

I'm voting request changes to avoid accidental merge of this PR before restoring NAND booting functionality. Please also test it on HW before the merge ;).

@badochov badochov force-pushed the badochov/imx6ull-ploboot branch 3 times, most recently from d5ef77d to d1433bc Compare March 22, 2023 12:22
@badochov badochov requested a review from nalajcie March 22, 2023 12:22
nalajcie
nalajcie reviewed Mar 22, 2023
View reviewed changes
Copy link
Member

@nalajcie nalajcie left a comment

Choose a reason for hiding this comment

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

nitpicks, changing to comment, please confirm that the PR was tested on DTR target before the merge

NOTE: maybe commit message title should be imx6ull: add option to boot from plo as we're not removing the previous functionality yet

_targets/build.project.armv7a7-imx6ull Outdated
b_mkscript_user "${USER_SCRIPT[@]}"
b_kernel_image "$PREFIX_BOOT/phoenix-kernel.img"
b_syspage_gen "$PREFIX_BOOT/phoenix-kernel.img" "$PLO_SCRIPT_DIR/$NAME_USER_SCRIPT"
fi;
Copy link
Member

Choose a reason for hiding this comment

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

unnecessary ;

_targets/build.project.armv7a7-imx6ull Outdated Show resolved Hide resolved
_targets/build.project.armv7a7-imx6ull Outdated Show resolved Hide resolved
_targets/build.project.armv7a7-imx6ull Outdated Show resolved Hide resolved
_targets/build.project.armv7a7-imx6ull Show resolved Hide resolved
@badochov badochov force-pushed the badochov/imx6ull-ploboot branch 2 times, most recently from 6e4bbbf to eb46cf2 Compare March 22, 2023 17:28
@badochov
Copy link
Contributor Author

imx6ull: add option to boot from plo

yup DTR was boroken, as this build uses PRIMARY_SCRIPT to detect which apps should be uploaded and in which format should the kernel be. I had to revert name change from USER_SCRIPT to PRIMARY_SCRIPT as this change breaks DTR. SHOULD the name PRIMARY_SCRIPT stay, as in other project USER_SCRIPT is used?

@badochov badochov force-pushed the badochov/imx6ull-ploboot branch 3 times, most recently from bb70f0b to 72f4dc1 Compare March 23, 2023 10:05
@nalajcie
Copy link
Member

imx6ull: add option to boot from plo

yup DTR was boroken, as this build uses PRIMARY_SCRIPT to detect which apps should be uploaded and in which format should the kernel be. I had to revert name change from USER_SCRIPT to PRIMARY_SCRIPT as this change breaks DTR. SHOULD the name PRIMARY_SCRIPT stay, as in other project USER_SCRIPT is used?

I'm not a plo expert, and the multitude of scripts are confusing for me, I think the PRIMARY script is always used (should be common for a given platform) and should call USER script (which would be different per project). Probably @agkaminski would know more about the intendent architecture.

@badochov badochov force-pushed the badochov/imx6ull-ploboot branch from 72f4dc1 to 3d851f0 Compare March 30, 2023 09:11
@agkaminski
Copy link
Member

It should be called USER_SCRIPT, but I'm opting to leave as is to keep DTR compatibility

@badochov badochov requested a review from nalajcie April 19, 2023 09:05
agkaminski
agkaminski previously approved these changes Apr 19, 2023
View reviewed changes
Copy link
Member

@agkaminski agkaminski left a comment

Choose a reason for hiding this comment

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

LGTM

@badochov badochov mentioned this pull request May 12, 2023
Merged
13 tasks
@badochov badochov force-pushed the badochov/imx6ull-ploboot branch from 3d851f0 to ac33b7d Compare May 15, 2023 14:53
@badochov badochov force-pushed the badochov/imx6ull-ploboot branch from ac33b7d to c9157b7 Compare May 15, 2023 17:11
agkaminski
agkaminski approved these changes May 17, 2023
View reviewed changes
Copy link
Member

@agkaminski agkaminski left a comment

Choose a reason for hiding this comment

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

LGTM

_targets/build.project.armv7a7-imx6ull Show resolved Hide resolved
nalajcie
nalajcie approved these changes Jun 2, 2023
View reviewed changes
_projects/armv7a7-imx6ull-evk/build.project
)


#TODO: if flash driver for imx6ull-evk implementation is introduced, psh should be replaced by imx6ul-flash and psd
Copy link
Member

Choose a reason for hiding this comment

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

nit: typo (second imx6ul -> missing l)

@badochov badochov merged commit 0414b09 into master Jun 2, 2023
@badochov badochov deleted the badochov/imx6ull-ploboot branch June 2, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agkaminski agkaminski agkaminski approved these changes

@nalajcie nalajcie nalajcie approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@badochov @agkaminski @nalajcie