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

usb: Fix and cleanup quirks list #1153

Merged
merged 2 commits into from
May 11, 2024
Merged

usb: Fix and cleanup quirks list #1153

merged 2 commits into from
May 11, 2024

Conversation

VexedUXR
Copy link
Contributor

@VexedUXR VexedUXR commented Apr 4, 2024

The USB_QUIRK_VP macro was being misused in the usb_quirks list, e.g USB_QUIRK_VP(USB_VENDOR_MPMAN, 0, UQ_MSC_NO_SYNC_CACHE, UQ_MATCH_VENDOR_ONLY) would set lo_res and hi_res to the first two specified quirks.

I've replaced it with a new macro, USB_QUIRK_VO, which removes the unneeded verbosity. Having this macro also removes the need for the dummy products used to specify vendor only quirks.

I also separated the usb quirks that target specific revisions from those that dont. A lot of the quirks dont use
lo_rev and hi_rev, so we can abstract the 0x0000, 0xffff into a macro.

Edit: Forgot to mention that I also replaced a | with ,. Looks like it was left over from eb58441
https://github.com/freebsd/freebsd-src/blob/main/sys/dev/usb/quirk/usb_quirk.c#L335

@bsdimp
Copy link
Member

bsdimp commented Apr 19, 2024

I'm struggling with how to review this.
I think that we need to break it up into smaller chunks so we can review and verify it does what it's trying to do.
Can you separate all that out please?

@VexedUXR
Copy link
Contributor Author

VexedUXR commented Apr 20, 2024

I separated the changes into two commits:
one replacing USB_QUIRK_VP and another adding USB_QUIRK_REV.

A large part of the changes are in the second commit, not sure I can separate them though.

Edit: Also fix some > 80 character warnings

@bsdimp
Copy link
Member

bsdimp commented Apr 23, 2024

the latest changes look like they can be reviewed, so I'll go over them with a fine toothed comb to see if anything stands out.

In some cases, the USB_QUIRK_VP macro was being misused. Instead of
setting quirks to the intended value, the first two supplied quirks
would go into lo_rev and hi_rev. Replace it with USB_QUIRK_VO which only
takes the needed args. This also makes the Dummy products, which where
being used to correctly set vendor only quirks, not necessary.

Reviewed by: imp
Pull Request: freebsd#1153
Seperate usb quirks that target specific revisions from those that
dont. Alot of the quirks dont use lo_rev and hi_rev, so we can abstract
the 0x0000, 0xffff into a macro.

[[ This commit is a bit more churn than we like. I carefully reviewed
   each one and they are all good. The end product is better -- imp ]]

Reviewed by: imp
Pull Request: freebsd#1153
@freebsd-git freebsd-git merged commit 881ae76 into freebsd:main May 11, 2024
7 of 9 checks passed
@VexedUXR
Copy link
Contributor Author

That's new :)
Thanks.

@VexedUXR VexedUXR deleted the USBQ branch May 12, 2024 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants