qdss: refactoring the code#66
Conversation
hangzqcom
left a comment
There was a problem hiding this comment.
Review
The three-commit structure is clean and the core logic changes are correct: removing the duplicate EvtDeviceFileClose, eliminating the registry-based function-type detection, and inverting the drain guards so QDSS (not DPL) gets the background pipe drain. A few issues need attention before merge.
Blocking
PortName is never initialized (QDBPNP.c): sprintf(pDevContext->PortName, ...) was removed from EvtDriverDeviceAdd and nothing replaces it. PortName is used as the %s prefix in 50+ QDB_DbgPrint calls across the driver. With a zero-initialized context it will always print an empty string, making WPP traces unreadable. Either populate it from the symbolic link / device description string (as QDBPNP_CreateSymbolicName already builds), or use RtlStringCbPrintfA to copy a short identifier there.
Non-blocking
Stale comment in EvtDevicePrepareHW: the comment immediately before the QDBRD_AllocateRequestsRx / QDBRD_PipeDrainStart call block still reads // Start to drain IN pipe for DPL. After the drain-guard inversion it should read // Start to drain IN pipe for QDSS.
WPP_GLOBALLOGGER removed without explanation (QDBMAIN.c): removing this macro silently disables the WPP global-logger session, which is used for boot-time tracing. If this is intentional (e.g. the global logger is no longer needed), add a comment explaining why; otherwise restore it.
Missing break in default case (QDBPNP_SetFunctionProtocol): the default: block sets FunctionType and calls QDB_DbgPrint but has no break. It is the last case so there is no fall-through bug, but adding break is consistent with the other cases and avoids a future maintenance hazard.
ecfb129 to
fbc1f1c
Compare
8b917c8 to
e08cc9f
Compare
Signed-off-by: Chenxi Han <chehan@qti.qualcomm.com>
…from INF Signed-off-by: Chenxi Han <chehan@qti.qualcomm.com>
Signed-off-by: Chenxi Han <chehan@qti.qualcomm.com>
Signed-off-by: Chenxi Han <chehan@qti.qualcomm.com>
Signed-off-by: Chenxi Han <chehan@qti.qualcomm.com>
Signed-off-by: Chenxi Han <chehan@qti.qualcomm.com>
e08cc9f to
04b2cff
Compare
|
The PR is updated with new commit ids. |
Signed-off-by: Chenxi Han <chehan@qti.qualcomm.com>
04b2cff to
2bb26db
Compare
Commits
1.
49822d7— qdss: add shared registry functionsIntroduces a new module (
QDBREG.c/QDBREG.h) centralising all driver software-key access. Six typed helpers are added:QDBREG_GetDriverRegistryStringW,QDBREG_SetDriverRegistryStringW,QDBREG_SetDriverRegistryStringA(convertsPCHAR→ Unicode viaRtlAnsiStringToUnicodeString),QDBREG_SetDriverRegistryDword,QDBREG_GetDriverRegistryDword, andQDBREG_DeleteDriverRegistryValue. All write/delete operations open the registry key withKEY_SET_VALUE.QDBREG.c(+239 lines) andQDBREG.h(+135 lines)qdbusb.vcxprojFiles changed:
QDBREG.c,QDBREG.h,qdbusb.vcxproj— net +376 lines2.
0e0367c— qdss: fix trace draining and remove hardcoded device-type dependency from INFFixes the pipe-drain for QDSS device type in
QDBRD.c:QDBRD_PipeDrainStart,QDBRD_PipeDrainStop,QDBRD_SendIOBlock,QDBRD_AllocateRequestsRx, andQDBRD_FreeIoBuffer.qdbusb.infis rewritten to remove hardcoded device-type assumptions, making the INF applicable across configurations without per-device editing.Files changed:
QDBPNP.c,QDBPNP.h,QDBRD.c,qdbusb.inf— net −29 lines3.
d7c0f62— qdss: update port name, friendly name, and symbolic link creationRewrites
QDBPNP_CreateSymbolicNameto eliminate fixed-size stack buffers (~8 KB). Device properties are now queried with kmdf apiWdfDeviceAllocAndQueryProperty, which allocates exactly the required bytes from non-paged pool and is freed immediately after use.DEVPKEY_Device_FriendlyNameis updated viaWdfDeviceAssignPropertyusing theWDF_DEVICE_PROPERTY_DATAAPI.portNameis updated to use deviceinstanceIdrather than incremental counter.Files changed:
QDBPNP.c(+125/−97) — net +28 lines4.
34d9bef— qdss: refactor pnp functions to use shared registry functionsReplaces open-coded
WdfDeviceOpenRegistryKey/WdfRegistry*/WdfRegistryClosesequences inQDBPNP_GetCID,QDBPNP_GetDeviceSerialNumber,QDBPNP_GetParentDeviceName, andQDBPNP_SetStampwith theQDBREG_*helpers.Files changed:
QDBMAIN.h,QDBPNP.c,QDBPNP.h— net −90 lines5.
84a0425— qdss: refactor IOCTL handler functionsRefactors
QDBDSP_IoDeviceControlto replace the legacy WDM-styleQDBDSP_IrpIoCompletionhandler (which accessed raw IRP fields directly) with a newQDBDSP_GetDPLStatshelper that usesWdfRequestRetrieveOutputBufferandWdfRequestCompleteWithInformation. Removes unused local variables and simplifies the IOCTL dispatch logic.Files changed:
QDBDSP.c(+63/−144),QDBDSP.h— net −81 lines6.
6781390— qdss: inline QDBWT_WriteUSB into QDBWT_IoWriteQDBWT_WriteUSBhad exactly one call site and re-fetchedpDevContextandfileContextredundantly fromQueueandRequest, both of which are already resolved in the caller. The function body is inlined intoQDBWT_IoWrite, eliminating two superfluous context lookups and simplifying the call graph.QDBWT_WriteUSBis removed fromQDBWT.h. No functional change.Files changed:
QDBWT.c(+46/−121),QDBWT.h(−7) — net −75 lines7.
04b2cff— qdss: remove unnecessary codes, update header includesLarge cleanup pass removing dead code no longer reachable after the preceding refactors:
QDBMAIN_AllocateUnicodeStringandQDBMAIN_GetRegistrySettingsfromQDBMAIN.c; drop their declarations fromQDBMAIN.hQDBPNP_SetStampand several other obsolete PNP helpers fromQDBPNP.c; clean upQDBPNP.hQDBRD_PipeDrainCompletionfromQDBRD.cDEVICE_CONTEXTandFILE_CONTEXTinQDBMAIN.h#includedirectives acrossQDBDEV.c,QDBDSP.c, andQDBRD.hFiles changed: 11 files — net −646 lines