Use MbedCRC for LittleFS #7824

deepikabhavnani · 2018-08-17T22:25:46Z

Description

916dec9fe4d7f7fc376c5979550b17a247d7173b : CRC used in LittleFS is Reversed ANSI, hence new polynomial added. Reversed polynomials perform shift in reverse direction of standard polynomial, and we do not have option to notify reverse shift to hardware. Hence this option is available in software only.

Pull request type

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

~~Dependent on #7793~~

deepikabhavnani · 2018-08-21T15:07:28Z

Preceding PR merged, hence rebased

geky · 2018-08-21T18:45:05Z

features/filesystem/littlefs/littlefs/lfs_util.c

 // Only compile if user does not provide custom config
 #ifndef LFS_CONFIG

+#ifdef __MBED__


nit: Could this be moved to the header file? And just make the crc function declared LittleFileSystem.cpp named "lfs_crc"?

geky · 2018-08-21T18:46:17Z

Looks good 👍 Though need to double check that the resulting disk image is portable.

0xc0170 · 2018-08-22T12:13:32Z

@deepikabhavnani Is this ready for review or still needs work?

deepikabhavnani · 2018-08-22T19:28:48Z

@0xc0170 - Its ready for review

0xc0170 · 2018-08-23T12:51:51Z

features/filesystem/littlefs/littlefs/lfs_util.c

-
-
-// Software CRC implementation with small lookup table
+#ifndef __MBED__


shouldn't we rather provide LFS_CONFIG than changing directly littlefs (another update might replace this?)

@0xc0170 - Yes we can but I assumed LFS_CONFIG is apart from mbed-os changes.

mbed-os/features/filesystem/littlefs/littlefs/lfs_util.h

Line 47 in 6bb2f50

#ifdef __MBED__

LFS_CONFIG was added after we first integrated LittleFS into Mbed OS, so that's why lfs_util.c/h have been modified directly.

git subtrees handle this surprisingly gracefully, so moving to LFS_CONFIG has been low priority.

As part of this PR, shall we update "lfs_util.c/h" or add separate files?

Up to you, adding separate files can be a separate PR

Up to you, adding separate files can be a separate PR

Will stay with this for now. Thanks

geky · 2018-08-23T19:26:28Z

This has a risk of effecting LittleFS structures on disk. @cmonr Could I request we hold off on this until 5.10?

cmonr · 2018-08-24T23:00:09Z

This has a risk of effecting LittleFS structures on disk. @cmonr Could I request we hold off on this until 5.10?

This is why I like to tag PRs as soon as possible :)

deepikabhavnani · 2018-08-24T23:40:49Z

Code freeze for 5.10 is before 5.9.7 and this PR is on top of other 3 CRC PR’s. Please tag all 4 to same release

cmonr · 2018-08-29T15:16:54Z

@geky @0xc0170 Good with this PR?

0xc0170 · 2018-08-30T12:11:04Z

@geky please finalize the review

geky · 2018-08-30T20:18:08Z

Sorry for delay, I wanted to verify that this did not break portability. It looks good to me 👍

@deepikabhavnani, thanks for putting this together

mbed-ci · 2018-09-03T02:56:58Z

Build : SUCCESS

Build number : 2990
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7824/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-09-03T06:39:40Z

Test : FAILURE

Build number : 2753
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7824/2753

0xc0170 · 2018-09-03T06:42:20Z

2 test failures related to the filesystem, please review

mbed-ci · 2018-09-03T07:12:56Z

Exporter Build : SUCCESS

Build number : 2600
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7824/

When length is zero, the buffer is not accessed. The CRC functions are used inside several other layers where a 0-length buffer may have meanings in different contexts and being able to pass 0-length NULL buffers to CRC as a noop simplifies the code.

adbridge · 2018-09-03T14:22:06Z

/morph build

mbed-ci · 2018-09-03T15:48:49Z

Build : SUCCESS

Build number : 2997
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7824/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-09-03T22:35:09Z

Exporter Build : FAILURE

Build number : 2608
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7824/

mbed-ci · 2018-09-03T23:23:08Z

Test : FAILURE

Build number : 2761
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7824/2761

cmonr · 2018-09-04T14:10:58Z

Marking this for RC2.

cmonr · 2018-09-04T19:41:49Z

/morph export-build

mbed-ci · 2018-09-04T23:36:52Z

Exporter Build : SUCCESS

Build number : 2616
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7824/

cmonr · 2018-09-05T02:34:51Z

/morph test

mbed-ci · 2018-09-05T04:25:39Z

Test : SUCCESS

Build number : 2773
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7824/2773

deepikabhavnani added the needs: work label Aug 17, 2018

deepikabhavnani force-pushed the lfs_crc branch from 916dec9 to 5d5a732 Compare August 17, 2018 22:42

0xc0170 added the needs: preceding PR label Aug 20, 2018

deepikabhavnani force-pushed the lfs_crc branch from 5d5a732 to 80d6f64 Compare August 21, 2018 15:06

deepikabhavnani force-pushed the lfs_crc branch from 80d6f64 to 4986673 Compare August 21, 2018 15:14

deepikabhavnani removed the needs: preceding PR label Aug 21, 2018

deepikabhavnani requested a review from geky August 21, 2018 15:25

deepikabhavnani force-pushed the lfs_crc branch from 4986673 to 6aab43c Compare August 21, 2018 17:47

geky reviewed Aug 21, 2018

View reviewed changes

This was referenced Aug 22, 2018

Template specialization didnt work after addition of default constructor #7793

Merged

Optimized CRC implementation - (32-bit ANSI polynomial) #7735

Merged

0xc0170 added needs: review and removed needs: work labels Aug 23, 2018

0xc0170 requested a review from a team August 23, 2018 12:50

0xc0170 reviewed Aug 23, 2018

View reviewed changes

cmonr added the release-version: 5.9.7 label Aug 23, 2018

cmonr added release-version: 5.10.0-rc1 and removed release-version: 5.9.7 labels Aug 24, 2018

0xc0170 approved these changes Aug 30, 2018

View reviewed changes

deepikabhavnani force-pushed the lfs_crc branch from 9078aab to ce8731a Compare August 30, 2018 19:57

cmonr added needs: CI and removed needs: work labels Sep 3, 2018

0xc0170 added needs: work and removed needs: CI labels Sep 3, 2018

cmonr added needs: CI and removed needs: work labels Sep 4, 2018

0xc0170 added needs: work and removed needs: CI labels Sep 4, 2018

cmonr added release-version: 5.10.0-rc2 and removed release-version: 5.10.0-rc1 labels Sep 4, 2018

cmonr added needs: CI and removed needs: work labels Sep 5, 2018

0xc0170 added ready for merge and removed needs: CI labels Sep 5, 2018

cmonr merged commit f940443 into ARMmbed:master Sep 5, 2018

0xc0170 removed the ready for merge label Sep 5, 2018



		// Software CRC implementation with small lookup table
		#ifndef __MBED__

Use MbedCRC for LittleFS #7824

Use MbedCRC for LittleFS #7824

Uh oh!

Conversation

deepikabhavnani commented Aug 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Uh oh!

deepikabhavnani commented Aug 21, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geky commented Aug 21, 2018

Uh oh!

0xc0170 commented Aug 22, 2018

Uh oh!

deepikabhavnani commented Aug 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geky commented Aug 23, 2018

Uh oh!

cmonr commented Aug 24, 2018

Uh oh!

deepikabhavnani commented Aug 24, 2018

Uh oh!

cmonr commented Aug 29, 2018

Uh oh!

0xc0170 commented Aug 30, 2018

Uh oh!

geky commented Aug 30, 2018

Uh oh!

mbed-ci commented Sep 3, 2018

Triggering tests

Uh oh!

mbed-ci commented Sep 3, 2018

Uh oh!

0xc0170 commented Sep 3, 2018

Uh oh!

mbed-ci commented Sep 3, 2018

Uh oh!

adbridge commented Sep 3, 2018

Uh oh!

mbed-ci commented Sep 3, 2018

Triggering tests

Uh oh!

mbed-ci commented Sep 3, 2018

Uh oh!

mbed-ci commented Sep 3, 2018

Uh oh!

cmonr commented Sep 4, 2018

Uh oh!

cmonr commented Sep 4, 2018

Uh oh!

mbed-ci commented Sep 4, 2018

Uh oh!

cmonr commented Sep 5, 2018

Uh oh!

mbed-ci commented Sep 5, 2018

Uh oh!

Uh oh!

deepikabhavnani commented Aug 17, 2018 •

edited

Loading