Re: [PATCH v9 3/6] drm/doc: Document DRM_WEDGE_RECOVERY_COLD_RESET recovery method

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The example recovery script fails to find the power control slot for directly connected PCI devices due to starting its sysfs traversal one level too high.
- [Low] The new table row for `cold-reset` exceeds the column width defined by the reStructuredText table underlines, which will cause a 'Malformed table' warning and break documentation builds.
--

commit affb0b839cc810c897bbc60a895efd262a86bba0
Author: Mallesh Koujalagi <mallesh.koujalagi@intel.com>

drm/doc: Document DRM_WEDGE_RECOVERY_COLD_RESET recovery method

This patch documents the WEDGED=cold-reset recovery method, which is
used when a device requires a complete power cycle to recover from
an error condition. It also adds an example userspace recovery script.

> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index 93df92c4ac8cf..43cdac65d04f0 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
[ ... ]
> @@ -441,6 +441,7 @@ following expectations.
>      rebind          unbind + bind driver
>      bus-reset       unbind + bus reset/re-enumeration + bind
>      vendor-specific vendor specific recovery method
> +    cold-reset      remove device + slot power cycle + rescan
>      unknown         consumer policy
>      =============== ========================================

[Severity: Low]
Will this exceed the column width defined by the reStructuredText table
underlines?

The Sphinx docutils parser treats table underlines as strict column width
boundaries. The second column's underline is 40 characters long, but the
new text 'remove device + slot power cycle + rescan' is 41 characters.

This usually causes a malformed table warning which can break kernel
documentation builds that treat warnings as errors.

[ ... ]
> @@ -530,6 +542,77 @@ Recovery script::
[ ... ]
> +    PCI_DEVS=/sys/bus/pci/devices
> +    PCI_SLOTS=/sys/bus/pci/slots
> +
> +    syspath=$(readlink -f "/sys/$1/device" 2>/dev/null || readlink -f "/sys/$1" 2>/dev/null)
> +    [ -n "$syspath" ] || die "cannot resolve sysfs path for: $1"
> +
> +    dev=$(basename "$syspath")
> +    [ -e "$PCI_DEVS/$dev" ] || die "not a PCI device: $dev"
> +    echo "device : $dev"
> +
> +    slot=""
> +    walk=$(dirname "$(readlink -f "$PCI_DEVS/$dev")")
> +
> +    while true; do
> +        ancestor=$(basename "$walk")

[Severity: Medium]
Does using dirname here skip the device's own sysfs path, causing the script
to compare the parent bridge's address to the slot's child address?

When the script executes, it initializes walk using dirname, which skips the
target device. It compares the parent bridge's address against the slot
address attribute.

However, hotplug slot address attributes contain the address of the child
(the device plugged into the slot), not the parent bridge. This could cause
the script to fail to find the power control slot for directly connected PCI
devices and abort with a 'no hotplug slot' error.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622131458.144873-8-mallesh.koujalagi@intel.com?part=3