On Fri, Oct 9, 2026 at 3:36 PM <[email protected]> wrote: > > 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 <[email protected]> > > 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?
This is harmless: devm_pm_set_wake_irq() doesn't request a new IRQ, it only marks the existing one as the wake IRQ, and the PM core only arms it when wakeup is enabled. i2c-hid has the same sequence (the I2C core sets the wake IRQ before the panel follower check). > > -- > Sashiko AI review · > https://sashiko.dev/#/patchset/[email protected]?part=11
