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.

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

BL808 Support #57

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

BL808 Support #57

wants to merge 12 commits into from

Conversation

Pavlos1
Copy link

@Pavlos1 Pavlos1 commented Nov 30, 2023

Currently only supports the IOT single-download mode. Write mode would require some refactoring, especially if we want to support multi-group flashing.

Also I don't own any other BLxxx hardware, so I highly recommend testing to make sure I haven't broken support for the other chips.

include/blisp_struct.h Show resolved Hide resolved
include/crc.h Outdated Show resolved Hide resolved
lib/blisp.c Outdated Show resolved Hide resolved
lib/blisp.c Show resolved Hide resolved
lib/blisp.c Outdated Show resolved Hide resolved
lib/blisp.c Outdated Show resolved Hide resolved
lib/blisp.c Outdated Show resolved Hide resolved
tools/blisp/src/common.c Outdated Show resolved Hide resolved
I missed the existing one in include/blisp_util.h
Compatability isues in older compilers are possible as a result
of this change.
@Pavlos1 Pavlos1 requested a review from robertlipe December 1, 2023 05:21
Copy link
Collaborator

@robertlipe robertlipe left a comment

Choose a reason for hiding this comment

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

It's pretty unbelievable that they push all this into uploaders.

Please think more about the error codes and whether Bouffalo takes the blame for typos, but it's otherwise OK with me.

I'll leave final approval/acceptance to Gaimee.

lib/chip/blisp_chip_bl808.c Show resolved Hide resolved
lib/chip/blisp_chip_bl808.c Show resolved Hide resolved
@Pavlos1
Copy link
Author

Pavlos1 commented Dec 3, 2023

Update: turns out the structs are slightly different; two of the fields differ (qpiJedecIdCmd and qpiJedecIdCmdDmyClk are replaced by enter32BitsAddrCmd and exit32BitsAddrCmd in the BL808 version, respectively.) Unifying the structs may still be possible, but is out of scope for this PR.

I have noticed that the flash configuration struct is duplicated in blisp_struct.h (including typos in the comments) as a result of this PR. This is because, while the BL808 bootheader is different to the bootheader used by the other BLxxx chips, it does use the same format for the flash configuration data.

I am debating whether to try to combine them. It might make some definitions look a bit weird, i.e.

struct __attribute__((packed, aligned(4))) bl808_bootheader_t {
    uint32_t magiccode; /* 4 */
    uint32_t rivison;   /* 4 */

    struct boot_flash_cfg_t flash_cfg; /* NOTE: BL808 uses the same flash configuration */ /* 4 + 84 + 4 */
    struct bl808_boot_clk_cfg_t clk_cfg; /* 4 + 20 + 4 */

    struct bl808_boot_basic_cfg_t basic_cfg; /* 4 + 4 + 4 + 4 + 4*8 */

    struct bl808_boot_cpu_cfg_t cpu_cfg[3]; /*24*3 */

    uint32_t boot2_pt_table_0_rsvd; /* address of partition table 0 */ /* 4 */
    uint32_t boot2_pt_table_1_rsvd; /* address of partition table 1 */ /* 4 */

    uint32_t flash_cfg_table_addr; /* address of flashcfg table list */ /* 4 */
    uint32_t flash_cfg_table_len; /* flashcfg table list len */         /* 4 */

    uint32_t rsvd0[8]; /* rsvd */
    uint32_t rsvd1[8]; /* rsvd */

    uint32_t rsvd3[5]; /* 20 */

    uint32_t crc32; /* 4 */
};

Thoughts?

This was a leftover from previous testing. It did work at 2Mbaud,
but I was only testing on Linux and do not want to introduce
a regression for other (i.e. MacOS) users.
@Pavlos1
Copy link
Author

Pavlos1 commented Feb 19, 2024

I have some additional changes (baud rate, chip erase command line arguments) on another branch.

I was initially planning on opening a separate PR if/when this one was merged. Let me know if it would be easier to just combine them into this PR instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants