Skip to content
This repository was archived by the owner on Apr 23, 2021. It is now read-only.

Example should now work with SD component #94

Merged
merged 1 commit into from
Jan 4, 2019

Conversation

yossi2le
Copy link
Contributor

Description

We are adding get_type to SDBlockDevice and the external sd-driver repo is deprecated. therefore I like to update the example to work with SDBlockDevice component which is part of mebd-os.
Currently, Travis is failing for the mbed-os PR #9135 and PR #9136 and after this change, they should pass.

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change

@deepikabhavnani
Copy link

@yossi2le - SD driver used in bootloader was 5.4 version, and updates on master will increase the code size for bootloader. See ARMmbed/mbed-os#8573

Option:

  1. Apply SD driver change for get_type on top of release version used by bootloader, and generate intermediate release.
  2. get_type commit was merge on master, so in order to use master add commits in Compile time config flag MBED_CONF_SD_CRC_ENABLED for CRC in SD mbed-os#8573 to sd-driver repo

@yossi2le
Copy link
Contributor Author

I am closing this PR and open a new one with only the sd-driver.lib changed to point to a new intermediate version of sd-driver repo as @deepikabhavnani proposed.

@yossi2le yossi2le closed this Dec 30, 2018
@c1728p9
Copy link
Contributor

c1728p9 commented Jan 2, 2019

@deepikabhavnani the bootloader in this example isn't size optimized so pulling in the newer SD driver shouldn't be a problem. A @yossi2le would it be possible to re-open this PR?

@deepikabhavnani
Copy link

@c1728p9 - Bootloader is not optimized by latest sd-driver in mbed-os repo is optimized and older version linked to the bootloader was optimized as well. Linking to this will add 1.5K more to the bootloader which needs to be resolved.

It will be a problem for devices which have bootloader working (including K64F)

@c1728p9
Copy link
Contributor

c1728p9 commented Jan 2, 2019

@deepikabhavnani the bootloader in this example can grow up to 256 KB so the additional 1.5K shouldn't be a problem.

@deepikabhavnani
Copy link

@deepikabhavnani the bootloader in this example can grow up to 256 KB so the additional 1.5K shouldn't be a problem.

good to know this..

@yossi2le
Copy link
Contributor Author

yossi2le commented Jan 3, 2019

@c1728p9 thanks for your response, It is really good to know that.
I personally have no problem reopen this PR. I really believe it will be better to have the sd-driver repo remove as it becomes deprecated and will not be maintained anymore. AFAIK the bootloader repository will also stop using the sd-driver external repo and shifts to use the component in the coming release.
@deepikabhavnani what do you say? shall we proceed with this change?

@yossi2le
Copy link
Contributor Author

yossi2le commented Jan 3, 2019

I reopen this PR before the weekend because this PR or #95 should be merged in order to unblock mbed-os PRs 9135 and 9136.
Can we proceed with this PR or #95?

@yossi2le yossi2le reopened this Jan 3, 2019
@deepikabhavnani
Copy link

deepikabhavnani commented Jan 3, 2019

. I really believe it will be better to have the sd-driver repo remove as it becomes deprecated and will not be maintained anymore. AFAIK the bootloader repository will also stop using the sd-driver external repo and shifts to use the component in the coming release

I agree with you on this, the example should be updated to use the sd-driver from Mbed OS instead of deprecated repo. Actually my bad, I should have advised to use the latest sd-driver on first place. Since we have already merged commits to old sd-driver repo, I will leave that to @c1728p9 to decide between this PR or #95

@deepikabhavnani
Copy link

I really believe it will be better to have the sd-driver repo remove as it becomes deprecated

To remove the deprecated repo's drop mail to @adbridge. Few storage repositories were deprecated recently and maintainers can decide on plan to archive them all together.

@cmonr
Copy link
Contributor

cmonr commented Jan 3, 2019

@deepikabhavnani FWDing deprecation message to @ARMmbed/mbed-os-maintainers

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

Successfully merging this pull request may close these issues.

4 participants