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

sam4l: usb: handle return code, long buffers #1894

Merged
merged 3 commits into from
Jun 4, 2020
Merged

sam4l: usb: handle return code, long buffers #1894

merged 3 commits into from
Jun 4, 2020

Conversation

bradjc
Copy link
Contributor

@bradjc bradjc commented May 29, 2020 •
edited
Loading

  1. The OkSetAddress return type was split off from Ok, but not added to the return code handler for the SAM4L. This puts it back.
  2. The capsule uses 64 byte buffers, while the sam4l driver originally only used 8 byte buffers. Since we only seem to use the pointer address in the driver, and not the actual length of the buffer, I think it should be fine if the buffers are too long.

Testing Strategy

This pull request was tested by running the usb test on imix and verifying that my laptop recognizes the USB device.

TODO or Help Wanted

n/a

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make format.
  • Fixed errors surfaced by make clippy.

@bradjc
sam4l: usb: handle OkSetAddress return code
670e529 
This was split off from `Ok`, but not added to the return code handler
for the SAM4L. Kinda feel like this one is on Rust, since it shouldn't
compile without handling this case :(.
@bradjc bradjc added the sam4l Change pertains to the SAM4L MCU. label May 29, 2020
brghena
brghena previously approved these changes May 29, 2020
View reviewed changes
@bradjc bradjc added the blocked Waiting on something, like a different PR or a dependency. label May 29, 2020
@bradjc
sam4l: usb: ensure buffers are at least 8 bytes
bdf7c12 
As far as I can tell, we only ever use the pointer address for these
buffers, and not their length, so it should be fine if the buffers are
too long (say 64 bytes rather than 8, as the capsule uses).
@bradjc bradjc removed the blocked Waiting on something, like a different PR or a dependency. label May 29, 2020
@bradjc bradjc changed the title sam4l: usb: handle OkSetAddress return code sam4l: usb: handle return code, long buffers May 29, 2020
@bradjc
Copy link
Contributor Author

bradjc commented May 29, 2020

Unblocked, I forgot I had to look into the 64 vs. 8 byte buffer issue.

brghena
brghena requested changes May 29, 2020
View reviewed changes
Copy link
Contributor

@brghena brghena left a comment

Choose a reason for hiding this comment

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

There are two places in handle_ctrl_endpoint_interrupt where packet_bytes == 8 is used. I think the latter (for transfer_complete) should be changed to 64. I'm not sure about the former.

@bradjc
sam4l: usb: update check for buffer full
075fc31 
Buffer size increased from 8 to 64.
@bradjc
Copy link
Contributor Author

bradjc commented May 29, 2020

Ok yup I agree. The first one I think is an "extra" check, because the received packet is always 8 bytes. We could remove the check, but it would fail if we set it to 64.

@bradjc bradjc requested a review from brghena June 2, 2020 15:18
@brghena
Copy link
Contributor

brghena commented Jun 4, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Jun 4, 2020

Build succeeded:

@bors bors bot merged commit 29597ae into master Jun 4, 2020
@bors bors bot deleted the sam4l-usb-fix branch June 4, 2020 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brghena brghena brghena approved these changes

Assignees
No one assigned
Labels
sam4l Change pertains to the SAM4L MCU.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bradjc @brghena