nvs: also support sector_size of 64KB

[mjchen0]

Allows NVS to work with flash device configured to use only 64KB block erase.

[Laczen]

Hi @mjchen0, the limitation of NVS to smaller sector sizes is an internal property. The proposed change does not enable the use of larger sectors.

[mjchen0]

Hi @mjchen0, the limitation of NVS to smaller sector sizes is an internal property. The proposed change does not enable the use of larger sectors.

Sorry, my title wasn't correct (I've changed it). I wasn't trying to support arbitrarily large sectors. I was just trying to make NVS work with FLASH that has only 64KB erase units. NVS seems to have a max sector size of 64KB. It's not described in the documentation of nvs.rts, but appears due to the representation of address as described in nvs_priv.h:

/*                                                                                                                                 
 * MASKS AND SHIFT FOR ADDRESSES                                                                                                   
 * an address in nvs is an uint32_t where:                                                                                         
 *   high 2 bytes represent the sector number                                                                                      
 *   low 2 bytes represent the offset in a sector                                                                                  
 */                                                                                                                                
#define ADDR_SECT_MASK 0xFFFF0000                                                                                                  
#define ADDR_SECT_SHIFT 16                                                                                                         
#define ADDR_OFFS_MASK 0x0000FFFF 

With 16-bits for offset, a 64KB sector size is possible (offset 0 to 0xffff) except that the sector_size field itself is uint16_t so 64KB couldn't be stored as the size.

[Laczen]

Hi @mjchen0, the limitation of NVS to smaller sector sizes is an internal property. The proposed change does not enable the use of larger sectors.

Sorry, my title wasn't correct (I've changed it). I wasn't trying to support arbitrarily large sectors. I was just trying to make NVS work with FLASH that has only 64KB erase units. NVS seems to have a max sector size of 64KB. It's not described in the documentation of nvs.rts, but appears due to the representation of address as described in nvs_priv.h:

/*                                                                                                                                 
 * MASKS AND SHIFT FOR ADDRESSES                                                                                                   
 * an address in nvs is an uint32_t where:                                                                                         
 *   high 2 bytes represent the sector number                                                                                      
 *   low 2 bytes represent the offset in a sector                                                                                  
 */                                                                                                                                
#define ADDR_SECT_MASK 0xFFFF0000                                                                                                  
#define ADDR_SECT_SHIFT 16                                                                                                         
#define ADDR_OFFS_MASK 0x0000FFFF 

With 16-bits for offset, a 64KB sector size is possible (offset 0 to 0xffff) except that the sector_size field itself is uint16_t so 64KB couldn't be stored as the size.

Ok, makes sense. Could you extend the PR with a check that sector_size is <= 64kB ? Also change the title to reflect "<=64kB".

[mjchen0]

I added the test in nvs_mount() that the sector_size isn't greater than 64KB.

I also added a new test case for native_sim with 64KB erase blocks, and a test that the nvs_mount() returns the expected error for a bad sector_size value.

[Laczen]

Thanks

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud