Re: [PATCH v5 11/11] HID: spi-hid: add panel follower support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] spi-hid: Inconsistent wake IRQ configuration for panel followers
--

commit 4c8d0ec463c5dbd300c392f3f924992096f8ebc1
Author: Jingyuan Liang <jingyliang@chromium.org>

HID: spi-hid: add panel follower support
    
This commit adds support to the spi-hid transport driver to act as a
panel follower, allowing its power state to be sequenced with a display
panel.

> diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
> --- a/drivers/hid/spi-hid/spi-hid-core.c
> +++ b/drivers/hid/spi-hid/spi-hid-core.c
[ ... ]
> @@ -1577,21 +1676,17 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
>  		}
>  	}
>  
> -	error = shid->ops->power_up(shid->ops);
> -	if (error) {
> -		dev_err(dev, "%s: could not power up\n", __func__);
> -		return error;
> -	}
> -
> -	error = shid->ops->deassert_reset(shid->ops);
> -	if (error) {
> -		dev_err(dev, "%s: failed to deassert reset: %d\n", __func__, error);
> -		shid->ops->power_down(shid->ops);
> -		return error;
> +	if (shid->is_panel_follower) {
> +		error = spi_hid_register_panel_follower(shid);
> +		if (error)
> +			return dev_err_probe(dev, error,
> +					     "Failed to register panel follower\n");

[Severity: Low]
Does this sequence leave a dangling, inactive wake IRQ attached to the
device when it is a panel follower?

In spi_hid_core_probe() just prior to this block, the code checks
device_can_wakeup(dev) and unconditionally allocates the wake IRQ via
devm_pm_set_wake_irq().

However, if the device is a panel follower, it calls
spi_hid_register_panel_follower() here. Inside that function, it checks
device_can_wakeup(dev) again, warns the user, and forcibly disables the
capability:

    if (device_can_wakeup(dev)) {
        dev_warn(dev, "Can't wakeup if following panel\n");
        device_set_wakeup_capable(dev, false);
    }

Because the capability is disabled after the IRQ is already requested, it
appears to waste resources by allocating an IRQ that is immediately rendered
unusable. Should the panel follower check and the potential clearing of the
wakeup capability occur before the wake IRQ is requested?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-send-upstream-v5-0-384af01da3ee@chromium.org?part=11