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.

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

Add support for Chromium OS EC devices #249

Merged
merged 8 commits into from
Jun 16, 2024

Conversation

fandango96
Copy link
Contributor

@fandango96 fandango96 commented Jun 9, 2024

This PR adds the CROS_EC_DEV_IOCXCMD_V2 and CROS_EC_DEV_IOCEVENTMASK_V2 ioctls necessary for testing Chromium OS EC devices. My main use-case is to add support for the FPMCU to allow automated testing in libfprint.

PR showing use of the newly added ioctls here: https://gitlab.freedesktop.org/libfprint/libfprint/-/merge_requests/493

@fandango96
Copy link
Contributor Author

not sure why the build fails only on s390x

@fandango96 fandango96 marked this pull request as draft June 9, 2024 20:07
@fandango96 fandango96 marked this pull request as ready for review June 9, 2024 23:00
@martinpitt
Copy link
Owner

Thanks @fandango96 ! Sorry, I don't have much time this week for a detailed review, as I'm travelling. But for s390 the build log shows a unit test failure:

# DEBUG: umockdev-ioctl.vala:904: Aborting client because ioctl tree for /dev/cros_fp is empty
stderr:
ioctl_varlenstruct_init_from_text: ioctl C014EC00: expected data length 67108884, but got 24 bytes from text data
ERROR: libumockdev-preload: Server requested abort on device /dev/cros_fp, exiting
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――

s390x is big endian, this smells like reading a 32 bit int the wrong way around?

@fandango96
Copy link
Contributor Author

Thank you @martinpitt, that was it - fixed.

meson.build Outdated Show resolved Hide resolved
src/ioctl_tree.c Outdated Show resolved Hide resolved
Copy link
Owner

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

This looks fine, thanks! Do you need this in a new release, or is git main enough for the libfprint MR?

@martinpitt martinpitt merged commit f2d7f3b into martinpitt:main Jun 16, 2024
23 checks passed
@fandango96
Copy link
Contributor Author

This looks fine, thanks! Do you need this in a new release, or is git main enough for the libfprint MR?

Thanks! git main is enough for the libfprint MR

@fandango96
Copy link
Contributor Author

This looks fine, thanks! Do you need this in a new release, or is git main enough for the libfprint MR?

@martinpitt looks like the CI doesn't need a new release (as it uses main) but we do a version check during runtime and I'll need to update to a version including this change. Are you planning to do a new release soon?

@martinpitt
Copy link
Owner

@fandango96 Sure! I just released https://github.com/martinpitt/umockdev/releases/tag/0.18.4 and will upload to Fedora and Debian now.

@fandango96
Copy link
Contributor Author

@fandango96 Sure! I just released https://github.com/martinpitt/umockdev/releases/tag/0.18.4 and will upload to Fedora and Debian now.

Thank you!

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

Successfully merging this pull request may close these issues.

2 participants