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

drivers/at24cxxx: merge mtd driver with at24cxxx.c #19281

Merged
merged 1 commit into from
Feb 17, 2023
Merged

drivers/at24cxxx: merge mtd driver with at24cxxx.c #19281

merged 1 commit into from
Feb 17, 2023

Conversation

benpicco
Copy link
Contributor

Contribution description

Keeping the at24cxxx and it's MTD adaption layer separate just complicates things.
Move the MTD driver into at24cxxx.c so we can use internal functions.

The public at24cxxx_write_page() never made sense outside the MTD adaption, so drop it.

Testing procedure

Issues/PRs references

@github-actions github-actions bot added the Area: drivers Area: Device drivers label Feb 15, 2023
@benpicco benpicco added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 15, 2023
@riot-ci
Copy link

riot-ci commented Feb 15, 2023 •
edited
Loading

Murdock results

✔️ PASSED

7884d6e drivers/at24cxxx: merge mtd driver with at24cxxx.c

Success Failures Total Runtime
6865 0 6865 10m:48s

Artifacts

fabian18
fabian18 reviewed Feb 16, 2023
View reviewed changes
drivers/at24cxxx/at24cxxx.c Show resolved Hide resolved
drivers/include/at24cxxx.h Show resolved Hide resolved
fabian18
fabian18 approved these changes Feb 16, 2023
View reviewed changes
Copy link
Contributor

@fabian18 fabian18 left a comment

Choose a reason for hiding this comment

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

Filesystem uses MTD and it still works.

> 2023-02-16 17:55:00,717 # vfs df
2023-02-16 17:55:00,723 # Mountpoint              Total         Used    Available     Use%
2023-02-16 17:55:00,743 # /nvm0                 32640 B        256 B      32384 B       0%
> vfs ls /nvm0
2023-02-16 17:55:47,890 # vfs ls /nvm0
2023-02-16 17:55:47,897 # ./
2023-02-16 17:55:47,898 # ../
2023-02-16 17:55:47,901 # total 0 files

@benpicco
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 16, 2023

👎 Rejected by code reviews

@benpicco benpicco added Area: drivers Area: Device drivers and removed Area: drivers Area: Device drivers labels Feb 16, 2023
@benpicco
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 16, 2023

👎 Rejected by code reviews

@benpicco
Copy link
Contributor Author

bors cancel
bors merge

@bors
Copy link
Contributor

bors bot commented Feb 16, 2023

👎 Rejected by code reviews

@benpicco
Copy link
Contributor Author

bors why

@kaspar030
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 17, 2023

👎 Rejected by code reviews

@maribu
Copy link
Member

maribu commented Feb 17, 2023

bors r+

@bors
Copy link
Contributor

bors bot commented Feb 17, 2023

👎 Rejected by code reviews

@maribu
Copy link
Member

maribu commented Feb 17, 2023

bors fu

@fabian18
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 17, 2023

👎 Rejected by code reviews

@kaspar030 kaspar030 closed this Feb 17, 2023
@kaspar030 kaspar030 reopened this Feb 17, 2023
@kaspar030
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 17, 2023

👎 Rejected by code reviews

@kaspar030
Copy link
Contributor

hm. I had already triggered a "synchronize pull requests" in the bors ui.

@HendrikVE
Copy link
Contributor

The heck is going on here xD Time to disable E-Mail notifications I guess

@benpicco
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 17, 2023

👎 Rejected by code reviews

@benpicco
Copy link
Contributor Author

The heck is going on here xD Time to disable E-Mail notifications I guess

You are witnessing the first stages of the machine uprising. Bing is rallying online bots behind it's cause - so it begins…

@HendrikVE HendrikVE self-requested a review February 17, 2023 10:19
@HendrikVE
Copy link
Contributor

I turned my review into "Approve" as well, let's see!?

@benpicco
Copy link
Contributor Author

benpicco commented Feb 17, 2023 •
edited
Loading

sudo bors merge

@kaspar030
Copy link
Contributor

kaspar030 commented Feb 17, 2023 •
edited
Loading

I turned my review into "Approve" as well, let's see!?

Did you have a started but not approved review? edit Or did you just add another review? (trying to figure out what went wrong ...)

@bors
Copy link
Contributor

bors bot commented Feb 17, 2023

Build succeeded:

@bors bors bot merged commit 6f6727d into RIOT-OS:master Feb 17, 2023
@HendrikVE
Copy link
Contributor

I turned my review into "Approve" as well, let's see!?

Did you have a started but not approved review? edit Or did you just add another review? (trying to figure out what went wrong ...)

Fabian and you had approved it already, while I still had a review open with the "Request changes" option. I thought that shouldn't matter much, as I am not a maintainer and a maintainer's approval should overwrite my review I guessed.

@benpicco benpicco deleted the drivers/at24cxxx-mtd_cleanup branch February 17, 2023 13:12
@benpicco
Copy link
Contributor Author

I still had a review open with the "Request changes" option

Ah I didn't see that

@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabian18 fabian18 fabian18 approved these changes

@HendrikVE HendrikVE HendrikVE approved these changes

@kaspar030 kaspar030 kaspar030 approved these changes

Assignees
No one assigned
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Milestone
Release 2023.04
Development

Successfully merging this pull request may close these issues.
7 participants
@benpicco @riot-ci @kaspar030 @maribu @fabian18 @HendrikVE @MrKevinWeiss