Attention is currently required from: Ravi kumar, Paul Menzel, mturney mturney, Julius Werner. Sandeep Maheswaram has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56091 )
Change subject: soc/qualcomm/common/usb: Add support for common USB driver ......................................................................
Patch Set 38:
(5 comments)
File src/soc/qualcomm/common/include/soc/usb/snps_usb_phy.h:
https://review.coreboot.org/c/coreboot/+/56091/comment/508a4ed0_16bdb9f4 PS34, Line 41: #define QC_GENMASK(h, l) (BIT(h + 1) - BIT(l))
Use the existing GENMASK() macro from helpers.h rather than rolling your own. […]
Done
https://review.coreboot.org/c/coreboot/+/56091/comment/74466b2c_124d6e8c PS34, Line 43: #define SLEEPM BIT(0)
Please don't define constants with no namespacing like this in global headers. […]
Done
File src/soc/qualcomm/common/include/soc/usb/usb_common.h:
https://review.coreboot.org/c/coreboot/+/56091/comment/4abebcb7_b5135720 PS34, Line 3: #if CONFIG(QUSB_PHY)
Please don't conditionally include things like this, it easily becomes confusing. […]
Done
https://review.coreboot.org/c/coreboot/+/56091/comment/330364ea_4e4eb15c PS34, Line 39: * USB BASE ADDRESSES
What is this comment supposed to mean?
Done
File src/soc/qualcomm/common/usb/snps_usb_phy.c:
https://review.coreboot.org/c/coreboot/+/56091/comment/9842848a_e89e9099 PS34, Line 8: static void qcom_snps_hsphy_write_mask(u32 *offset, u32 mask, u32 val)
We already have clrsetbits32() for this. […]
Done