Pico flash size bytes #159

2021-02-18T18:18:53-06:00

kilograham commented

2021-02-18 18:18:53 -06:00

(Migrated from github.com)

No description provided.

Wren6991 (Migrated from github.com) reviewed

2021-02-18 18:18:53 -06:00

lurch commented

2021-02-18 18:54:56 -06:00

(Migrated from github.com)

Looking at the other #define values in platform_defs.h, should XOSC_MHZ also be moved elsewhere? The RP2040 datasheet says "The RP2040 supports 1MHz to 15MHz crystals" so I guess XOSC_MHZ might be a board-level thing rather than a platform-level thing?

Looking at the other `#define` values in `platform_defs.h`, should `XOSC_MHZ` also be moved elsewhere? The RP2040 datasheet says "The RP2040 supports 1MHz to 15MHz crystals" so I guess XOSC_MHZ might be a board-level thing rather than a platform-level thing?

Wren6991 (Migrated from github.com) reviewed

2021-02-19 06:38:29 -06:00

src/rp2_common/hardware_flash/include/hardware/flash.h

					
				@ -44,6 +33,22 @@

				 * \include flash_program.c

Is -1 the correct default value? E.g. see here in hardware_flash.c

#ifdef PICO_FLASH_SIZE_BYTES
    hard_assert(flash_offs + count <= PICO_FLASH_SIZE_BYTES);
#endif

Seems this would give a mixed sign comparison if the board header doesn't define flash size. Maybe it should be left undefined, or set to 16M?

Is -1 the correct default value? E.g. see here in `hardware_flash.c` ``` #ifdef PICO_FLASH_SIZE_BYTES hard_assert(flash_offs + count <= PICO_FLASH_SIZE_BYTES); #endif ``` Seems this would give a mixed sign comparison if the board header doesn't define flash size. Maybe it should be left undefined, or set to 16M?

Wren6991 commented

2021-02-19 06:40:00 -06:00

(Migrated from github.com)

Looking at the other #define values in platform_defs.h, should XOSC_MHZ also be moved elsewhere? The RP2040 datasheet says "The RP2040 supports 1MHz to 15MHz crystals" so I guess XOSC_MHZ might be a board-level thing rather than a platform-level thing?

Yeah the initial clock setup needs more thought in general, e.g. we should be able to support systems with no crystal at all, but currently that requires you to modify or roll your own runtime_init. It should also be hoisted up over the data init in crt0 to speed that up. I'll open an issue for it

> Looking at the other `#define` values in `platform_defs.h`, should `XOSC_MHZ` also be moved elsewhere? The RP2040 datasheet says "The RP2040 supports 1MHz to 15MHz crystals" so I guess XOSC_MHZ might be a board-level thing rather than a platform-level thing? Yeah the initial clock setup needs more thought in general, e.g. we should be able to support systems with no crystal at all, but currently that requires you to modify or roll your own `runtime_init`. It should also be hoisted up over the data init in crt0 to speed that up. I'll open an issue for it

kilograham commented

2021-02-19 13:45:29 -06:00

(Migrated from github.com)

fixes #134

lurch (Migrated from github.com) reviewed

2021-02-25 05:33:52 -06:00

src/rp2_common/hardware_flash/include/hardware/flash.h

Probably ought to remove the default from this line too?
(I've just checked, and all the scripts/programs dealing with pico_configs.tsv cope fine with default being absent)

Probably ought to remove the default from this line too? (I've just checked, and all the scripts/programs dealing with `pico_configs.tsv` cope fine with `default` being absent)

lurch (Migrated from github.com) reviewed

2021-02-25 07:17:58 -06:00

src/rp2_common/hardware_flash/include/hardware/flash.h

#183 enforces this 🙂

#183 enforces this :slightly_smiling_face:

liamfraser (Migrated from github.com) reviewed

2021-02-25 10:46:16 -06:00

liamfraser (Migrated from github.com) reviewed

2021-02-25 10:52:57 -06:00

liamfraser (Migrated from github.com) approved these changes

2021-02-25 10:58:52 -06:00

lurch (Migrated from github.com) reviewed

2021-02-25 11:00:48 -06:00

src/rp2_common/hardware_flash/include/hardware/flash.h

You still need to fix the PICO_CONFIG line to remove the default, i.e.

// PICO_CONFIG: PICO_FLASH_SIZE_BYTES, size of primary flash in bytes, type=int, group=hardware_flash

You still need to fix the PICO_CONFIG line to remove the `default`, i.e. ``` // PICO_CONFIG: PICO_FLASH_SIZE_BYTES, size of primary flash in bytes, type=int, group=hardware_flash ```

Sign in to join this conversation.

No reviewers

Wren6991

liamfraser