diff mbox series

[2/8] i2c: i801: make FEATURE_HOST_NOTIFY dependent on FEATURE_IRQ

Message ID 08ccd96f-deca-72b9-e14e-917434736ca3@gmail.com
State New
Headers show
Series i2c: i801: Series with minor improvements | expand

Commit Message

Heiner Kallweit April 15, 2022, 4:55 p.m. UTC
Host notification uses an interrupt, therefore it makes sense only if
interrupts are enabled. Make this dependency explicit by removing
FEATURE_HOST_NOTIFY if FEATURE_IRQ isn't set.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/i2c/busses/i2c-i801.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Heiner Kallweit Dec. 16, 2022, 8:23 p.m. UTC | #1
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@gmail.com>
>> ---
>>  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).
>
diff mbox series

Patch

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;
+
 	i801_add_tco(priv);
 
 	snprintf(priv->adapter.name, sizeof(priv->adapter.name),