Skip to content

ks: Add support for libusb communication #100

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lixkel
Copy link

@lixkel lixkel commented Apr 11, 2025

This patch adds support for libusb communication with devices in edl mode, while retaining the existing implementation using file descriptors. It's unclear to me if the file descriptor implementation is actively used by anyone (it doesn't work very well for me), so feedback on its usage and potential deletion is welcome.

@lumag
Copy link
Contributor

lumag commented May 1, 2025

cc @quic-jhugo
Could you please take a look

@quic-jhugo
Copy link
Contributor

(commenting before I actually have a look at the code)
The file descriptor thing is probably not used with the upstream kernel. I don't think the wwan subsystem has any Sahara devices exposed to userspace, although it could.
AIC100 has an in-kernel Sahara implementation now, mostly as a result of failing to get an acceptable file descriptor interface designed.
Most of the downstream Sahara implementations on Linux, and also the windows implementations, directly pipe the Sahara channel to userspace via a chardev. This qdl project would be useful there with the file descriptor interface.

@@ -62,6 +52,7 @@ int main(int argc, char **argv)
{"version", no_argument, 0, 'v'},
{"port", required_argument, 0, 'p'},
Copy link
Contributor

Choose a reason for hiding this comment

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

This is still listed as required, but the documentation changes above suggest that this PR is making it optional

Copy link
Author

Choose a reason for hiding this comment

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

Thats the has_args field, which states that the flag requires argument not that the flag itself is required

{

return write(qdl->fd, buf, len);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

How does removing these definitions not cause a compile error?

Copy link
Author

Choose a reason for hiding this comment

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

These functions are defined in qdl.h and already implemented in usb.c, I edited the usb.c implementation so that if the fd field in qdl struct is defined the file descriptor is used instead of libusb. This solution is not ideal, but its the best I could come up with without more extensive changes to the codebase, which seem pointless as only ks uses file descriptors.

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.

3 participants