Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The commit message and driver documentation claim support for an interrupt endpoint, but it is never opened or exposed, leading to a meaningful mismatch and potential device wedging. --
commit a40630ab675c3654c37821d57cc77e281c7a9a17 Author: Mike Lothian <[email protected]> drm/vino: add the USB transport This commit introduces the USB transport for the dock. It adds a wrapper to manage endpoint resolution and provides functions to handle control and video data pipelines, as well as endpoint stall recovery. > diff --git a/drivers/gpu/drm/vino/usb_link.rs > b/drivers/gpu/drm/vino/usb_link.rs > new file mode 100644 > index 0000000000000..04b9131d05ed9 > --- /dev/null > +++ b/drivers/gpu/drm/vino/usb_link.rs [ ... ] > +#[derive(Clone, Copy)] > +pub(crate) struct Endpoints { > + /// EP02: host->dock control-plane bulk writes. > + pub(crate) ctrl_out: usb::Endpoint<usb::BulkOut>, > + /// EP84: dock->host control-plane bulk replies. > + pub(crate) ctrl_in: usb::Endpoint<usb::BulkIn>, > + /// Per-connector video bulk-OUT endpoints, from > [`DockProfile::video_endpoints`]. > + pub(crate) video: [usb::Endpoint<usb::BulkOut>; > drm_sink::MAX_CONNECTORS], > +} [Severity: Medium] The commit message mentions an interrupt endpoint the dock pushes on and states that this wrapper opens those pipes. Does the Endpoints struct omit the interrupt endpoint entirely? If the dock hardware requires its interrupt endpoint to be serviced to prevent its internal FIFO from filling up, could ignoring it wedge the dock? [ ... ] > + pub(crate) fn resolve<Ctx: device::DeviceContext>( > + intf: &usb::Interface<Ctx>, > + profile: &DockProfile, > + ) -> Result<(Self, u8)> { [ ... ] > + Ok(( > + Self { > + ctrl_out: intf.endpoint::<usb::BulkOut>(EP_CTRL_OUT)?, > + ctrl_in: intf.endpoint::<usb::BulkIn>(EP_CTRL_IN)?, > + video, > + }, > + connectors.min(profile.topology.connectors), > + )) > + } [Severity: Medium] In Endpoints::resolve(), the endpoint resolution logic does not seem to parse or open the interrupt endpoint. Does this contradict the commit message claim that the wrapper opens those pipes? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
