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

Cleanups #384

Merged
merged 12 commits into from
Mar 14, 2016
Merged

Cleanups #384

merged 12 commits into from
Mar 14, 2016

Conversation

mcoquelin-stm32
Copy link
Contributor

Dear Texane,

This pull-request contains some preliminary reworks to have complete error propagation from libusb to st-flash tool.

I also changed some loaders to "commonize" their handling.

Please let me know what you thinkabout it.
Thanks in advance,
Maxime

@mcoquelin-stm32
read_debug32: Use a pointer instead of returning the value
bd27213 
This rework is done in order to prepare for propagating errors returned by
libusb.

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
@mcoquelin-stm32
stlink_core_id: No need to return core_id value
5fcad7d 
This rework is done in order to prepare for propagating errors returned by
libusb.

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
@mcoquelin-stm32
stlink_target_voltage: Check for libusb error
89c3b14 
_stlink_usb_target_voltage already returns an error value.
If value return is positive, this is a voltage, if negative this is an error.
Check the return on callers side to inform there is an error in reading the
voltage, instead of notifying of a too low voltage.

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
@mcoquelin-stm32
Make the backends propagate errors
64a48c7 
As the libusb returns errors, make the backends propagates them so that
callers can decide to continue or stop task execution.

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
@mcoquelin-stm32
stlink-common: Make stlink API propagate backend errors
93e958f 
Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
@mcoquelin-stm32
st-flash: Improve error handling
a9f00bc 
Now that libusb errors are propagated up to stlink API, we can handle these
errors.

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
@mcoquelin-stm32
stlink-common: Introduce type of flash controller enum
abcd47f 
Having a type of flash information has some advantages:
 - Make the code easier to read
 - Make adding family derivatives easier (only add a new entry in header file)
 - Make the backends error propagation easier to implement (less places to fix)

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
@mcoquelin-stm32
stlink-common: Simplify run_flash_loader()
5693181 
This patch simplifies run_flash_loader() function in preparation for error
propagation from backends.

Doing this, we have less call sites for stlink API.

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
@mcoquelin-stm32
stlink-common: Update STM32L0 and STM32L1 loader ABI
907383d 
This patch invert source and destination registers in the stm32l0 and stm32l1
loaders, so that it follows the same ABI as other stm32 loaders.

Doing that, the run_flash_loader() function can be simplified a little.

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
@mcoquelin-stm32
stlink-common: Update STM32L0/1 loaders to return remaining count in r2
e43a737 
All the loaders returns remaining work count in r2, except stm32l0/1.
Make these loaders behaving as the others to simplify run_flash_loader() code.

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
@mcoquelin-stm32
stlink-common: Fix STM32L4 loader write count to reflect 64bits granu…
d0458ee 
…larity

The stm32l4 loader expects a count of 32 bits words while its granularity is
really 64 bits.

This patch fixes this to simplify count calculation in run_flash_loader().

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
@mcoquelin-stm32
stlink-common: Ensure flash type is properly declared in device params
faa19b5 
Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
@texane
Copy link
Collaborator

texane commented Mar 14, 2016

Hi,

thanks for the work, looks fine. However, it won't be merged it as is.

Until now, a single commit targets a specific feature / fix which makes
the code easy to revert in case of regression, an important aspect of
this tool given the number of supported platforms.

Thus, I encourage you to break the commit into individual pieces.

Cheers

2016-03-14 15:26 GMT+01:00 mcoquelin-stm32 notifications@github.com:

Dear Texane,

This pull-request contains some preliminary reworks to have complete error
propagation from libusb to st-flash tool.

I also changed some loaders to "commonize" their handling.

Please let me know what you thinkabout it.
Thanks in advance,

Maxime

You can view, comment on, or merge this pull request online at:

#384
Commit Summary

  • read_debug32: Use a pointer instead of returning the value
  • stlink_core_id: No need to return core_id value
  • stlink_target_voltage: Check for libusb error
  • Make the backends propagate errors
  • stlink-common: Make stlink API propagate backend errors
  • st-flash: Improve error handling
  • stlink-common: Introduce type of flash controller enum
  • stlink-common: Simplify run_flash_loader()
  • stlink-common: Update STM32L0 and STM32L1 loader ABI
  • stlink-common: Update STM32L0/1 loaders to return remaining count in
    r2
  • stlink-common: Fix STM32L4 loader write count to reflect 64bits
    granularity
  • stlink-common: Ensure flash type is properly declared in device
    params

File Changes

Patch Links:

—
Reply to this email directly or view it on GitHub
#384.

@mcoquelin-stm32
Copy link
Contributor Author

Hi,

Thanks for the review.
Actually, that's what I did, this is split in 12 commits:
@mcoquelin-stm32 read_debug32: Use a pointer instead of returning the value … bd27213
@mcoquelin-stm32 stlink_core_id: No need to return core_id value … 5fcad7d
@mcoquelin-stm32 stlink_target_voltage: Check for libusb error … 89c3b14
@mcoquelin-stm32 Make the backends propagate errors … 64a48c7
@mcoquelin-stm32 stlink-common: Make stlink API propagate backend errors … 93e958f
@mcoquelin-stm32 st-flash: Improve error handling … a9f00bc
@mcoquelin-stm32 stlink-common: Introduce type of flash controller enum … abcd47f
@mcoquelin-stm32 stlink-common: Simplify run_flash_loader() … 5693181
@mcoquelin-stm32 stlink-common: Update STM32L0 and STM32L1 loader ABI … 907383d
@mcoquelin-stm32 stlink-common: Update STM32L0/1 loaders to return remaining count in r2 … e43a737
@mcoquelin-stm32 stlink-common: Fix STM32L4 loader write count to reflect 64bits granu… … d0458ee
@mcoquelin-stm32 stlink-common: Ensure flash type is properly declared in device params … faa19b5
Should it be split in more commits?

Thanks in advance,
Maxime

texane added a commit that referenced this pull request Mar 14, 2016
@texane
Merge pull request #384 from mcoquelin-stm32/cleanups
f157237 
Cleanups
@texane texane merged commit f157237 into stlink-org:master Mar 14, 2016
@mcoquelin-stm32
Copy link
Contributor Author

Thanks!

@texane
Copy link
Collaborator

texane commented Mar 14, 2016

No, that is fine. Thanks

@tomdeboer tomdeboer mentioned this pull request May 4, 2016
Closed
@Nightwalker-87 Nightwalker-87 added this to the v1.2.0 and older milestone Mar 14, 2020
@stlink-org stlink-org locked as resolved and limited conversation to collaborators Apr 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
code/refactoring component/st-flash dependency/libusb status/resolved
Projects
None yet
Milestone
v1.2.0 and older
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mcoquelin-stm32 @texane @Nightwalker-87