Hi Heiner, On Fri, 15 Apr 2022 18:55:10 +0200, Heiner Kallweit wrote: > Host notification uses an interrupt, therefore it makes sense only if > interrupts are enabled. It would be nice to have this comment in the code itself. > Make this dependency explicit by removing > FEATURE_HOST_NOTIFY if FEATURE_IRQ isn't set. > > Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx> > --- > drivers/i2c/busses/i2c-i801.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c > index c481f121d..eccdc7035 100644 > --- a/drivers/i2c/busses/i2c-i801.c > +++ b/drivers/i2c/busses/i2c-i801.c > @@ -1756,6 +1756,9 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id) > dev_info(&dev->dev, "SMBus using %s\n", > priv->features & FEATURE_IRQ ? "PCI interrupt" : "polling"); > > + if (!(priv->features & FEATURE_IRQ)) > + priv->features &= ~FEATURE_HOST_NOTIFY; Earlier in this function, there's an action which depends on the FEATURE_HOST_NOTIFY flag being set. While this will only result in a useless action and wouldn't cause a bug as far as I can see, wouldn't it be cleaner to move that piece of code *after* the check you're adding? > + > i801_add_tco(priv); > > snprintf(priv->adapter.name, sizeof(priv->adapter.name), Looks good, tested OK on my system (non-regression only, I'm not using the Host Notify feature). -- Jean Delvare SUSE L3 Support
