Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Medium] The example `cold-reset.sh` script fails to find native PCIe hotplug 
(pciehp) slots for directly attached devices because it initializes its 
topology traversal with the parent directory of the device, skipping the 
device's own PCI address which is what actually matches the hotplug slot's 
`address` attribute.
--

--- Patch [2]: [PATCH v13 2/4] drm/doc: Document DRM_WEDGE_RECOVERY_COLD_RESET 
recovery method ---
commit ae4802bfe15243552790a2e115f55653f35d380a
Author: Mallesh Koujalagi <[email protected]>

drm/doc: Document DRM_WEDGE_RECOVERY_COLD_RESET recovery method

This commit adds documentation for the WEDGED=cold-reset recovery method
in the DRM subsystem. It explains the consumer expectations and provides
an example udev rule alongside a recovery script for cycling PCIe slot power.

> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index 93df92c4ac8cf..52255247a6db9 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
[ ... ]
> +Example - cold-reset
> +--------------------
> +
> +Udev rule::
> +
> +    SUBSYSTEM=="drm", ENV{WEDGED}=="cold-reset", DEVPATH=="*/drm/card[0-9]", 
> \
> +    RUN+="/path/to/cold-reset.sh $env{DEVPATH}"
> +
> +Recovery script::
[ ... ]
> +    dev=$(basename "$syspath")
> +    [ -e "$PCI_DEVS/$dev" ] || die "not a PCI device: $dev"
> +    echo "device : $dev"
> +
> +    slot=""
> +    walk=$(dirname "$(readlink -f "$PCI_DEVS/$dev")")

[Severity: Medium]
Does this topology traversal intentionally skip the device's own PCI address?

By initializing walk with the parent directory via dirname, it appears the
script will miss the device itself when checking for matching hotplug slot
addresses in the loop below. This seems like it could cause the script to fail
to find native PCIe hotplug slots for directly attached devices, leading to a
"no hotplug slot with power control found" error when users try to run it.

> +
> +    while true; do
> +        ancestor=$(basename "$walk")
> +        case "$ancestor" in pci*) break ;; esac  # reached the virtual bus 
> root
> +
> +        ancestor_nofn=${ancestor%.*}  # strip function: 0000:03:01.0 -> 
> 0000:03:01
[ ... ]

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to