Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The fallback loop bypasses or corrupts the DP AUX transaction tracing provided by `drm_dp_dump_access()`, hiding hardware failures and injecting fake successful transactions into the debug logs. --
commit bfb3e55f35cdb8d13250c7d1d77fedcf644031fd Author: Chia-Lin Kao (AceLan) <[email protected]> drm/dp: Move byte-by-byte AUX read fallback to drm_dp_dpcd_read() This commit moves the workaround for USB-C hubs with buggy firmware from drm_dp_dpcd_read_data() down into drm_dp_dpcd_read(). It ensures all direct and indirect callers benefit from the fallback mechanism and uses drm_dp_dpcd_read_byte() to properly handle short transfers. > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c > b/drivers/gpu/drm/display/drm_dp_helper.c > --- a/drivers/gpu/drm/display/drm_dp_helper.c > +++ b/drivers/gpu/drm/display/drm_dp_helper.c [ ... ] > @@ -762,6 +762,27 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, > unsigned int offset, > ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset, > buffer, size); > > + if (ret < 0 && size > 1) { > + size_t i; > + u8 *buf = buffer; > + int err; > + > + /* > + * Workaround for USB-C hubs/adapters with buggy firmware that > fail > + * multi-byte AUX reads but work with single-byte reads. > + * Known affected devices: > + * - Lenovo USB-C to VGA adapter (VIA VL817, idVendor=17ef, > idProduct=7217) > + * - Dell DA310 USB-C hub (idVendor=413c, idProduct=c010) > + * Attempt byte-by-byte reading as a fallback. > + */ > + for (i = 0; i < size; i++) { > + err = drm_dp_dpcd_read_byte(aux, offset + i, &buf[i]); > + if (err < 0) > + return err; [Severity: Medium] Does this early return bypass the tracing for the initial failed multi-byte read? If the fallback loop fails halfway, returning err directly skips the drm_dp_dump_access() call at the end of the function, which means the initial multi-byte hardware failure is silently lost from the DP trace. > + } > + ret = size; [Severity: Medium] If the fallback loop succeeds, ret is set to size and falls through to drm_dp_dump_access(). Does this incorrectly log a single successful multi-byte physical transaction that never actually occurred on the wire? Since it was fulfilled via single-byte reads (which themselves were also logged), this might inject fake successful transactions into the debug logs and degrade debuggability for hardware transaction issues. > + } > + > drm_dp_dump_access(aux, DP_AUX_NATIVE_READ, offset, buffer, ret); > return ret; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
