Skip to content

LPC176X: Fix flash program size #6413

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

Merged
merged 2 commits into from
Mar 29, 2018
Merged

LPC176X: Fix flash program size #6413

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d76d511
LPC176X: Fix flash program size
toyowata Mar 21, 2018
3acdc81
Add alignment check in the flash_program_page
toyowata Mar 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 37 additions & 23 deletions targets/TARGET_NXP/TARGET_LPC176X/device/flash_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,42 +112,56 @@ int32_t flash_program_page(flash_t *obj, uint32_t address,
const uint8_t *data, uint32_t size)
{
unsigned long n;
const uint32_t copySize = 1024; // should be 256|512|1024|4096
uint8_t *alignedData, *source;

// always malloc outside critical section
uint8_t *alignedData = malloc(size);
alignedData = malloc(size);
Copy link
Contributor

Choose a reason for hiding this comment

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

it could be as @davidsaada proposed - smaller program page size if it is defined, or what I was initially thinking, was allocating only copySize buffer on heap (because that one is used for flashing). It increases the runtime but making it possible to work for any app (1k of heap should be available). ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure it would help in this case. I suspect that the extra allocation of 1024 bytes already pushes this test over the limit. However, it's worth the try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@0xc0170 @davidsaada In the current code, the reason of the malloc usage is to handle unaligned the data buffer here:

https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_NXP/TARGET_LPC176X/device/flash_api.c#L132-L137

However, there is an alignment check logic here before calling the HAL driver:
https://github.com/ARMmbed/mbed-os/blob/master/drivers/FlashIAP.cpp#L101

I think therefore, the malloc in the flash_program_page() is not required and it should just check the size of flash data.

I am going to modify this and create new commit for review next week.

Copy link
Contributor

Choose a reason for hiding this comment

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

@toyowata You can still check the alignment for the (unlikely) case this code is not called from the driver (or if the driver logic is changed for some reason), and only then perform the malloc + memcpy. In the likely aligned case, you can point to the user buffer.

if (alignedData == 0) {
return (1);
}

n = GetSecNum(address); // Get Sector Number

core_util_critical_section_enter();
IAP.cmd = 50;// Prepare Sector for Write
IAP.par[0] = n;// Start Sector
IAP.par[1] = n;// End Sector
IAP_Call (&IAP.cmd, &IAP.stat);// Call IAP Command
if (IAP.stat) {
return (1); // Command Failed
}
memcpy(alignedData, data, size);
source = alignedData;

IAP.cmd = 51; // Copy RAM to Flash
IAP.par[0] = address;// Destination Flash Address
core_util_critical_section_enter();

if ((unsigned long)data%4==0) { // Word boundary
IAP.par[1] = (unsigned long)data;// Source RAM Address
} else {
memcpy(alignedData,data,size);
IAP.par[1] = (unsigned long)alignedData; // Source RAM Address
while (size) {
/*
Prepare_Sector_for_Write command must be exected before
Copy_RAM_to_Flash command.
*/
IAP.cmd = 50; // Prepare Sector for Write
IAP.par[0] = n; // Start Sector
IAP.par[1] = n; // End Sector
IAP_Call (&IAP.cmd, &IAP.stat); // Call IAP Command
if (IAP.stat) {
return (1); // Command Failed
}

IAP.cmd = 51; // Copy RAM to Flash
IAP.par[0] = address; // Destination Flash Address
IAP.par[1] = (unsigned long)source; // Source RAM Address
IAP.par[2] = copySize; // number of bytes to be written
IAP.par[3] = CCLK; // CCLK in kHz
IAP_Call (&IAP.cmd, &IAP.stat); // Call IAP Command
if (IAP.stat) {
return (1); // Command Failed
}

source += copySize;
size -= copySize;
address += copySize;
}

IAP.par[2] = 1024; // Fixed Page Size
IAP.par[3] = CCLK;// CCLK in kHz
IAP_Call (&IAP.cmd, &IAP.stat);// Call IAP Command
core_util_critical_section_exit();

if(alignedData !=0) { // We allocated our own memory
if(alignedData != 0) { // We allocated our own memory
free(alignedData);
}

if (IAP.stat) {
return (1); // Command Failed
}
return (0); // Finished without Errors
}

Expand Down