Re: [PATCH v3 2/13] drm/vino: add the USB transport

[sashiko-bot@kernel.org]

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 <mike@fireburn.co.uk>

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/20260826163913.7052-1-mike@fireburn.co.uk?part=2