On 07.06.2022 14:48, Jean Delvare wrote: > 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. > OK >> 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? > Yes, this would be better. Will be part of v2. >> + >> 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). >
