On Thu, 5 Aug 2021 22:04:18 +0200, Heiner Kallweit wrote: > On 05.08.2021 12:41, Jean Delvare wrote: > > On Sun, 01 Aug 2021 16:21:08 +0200, Heiner Kallweit wrote: > >> do_pci_enable_device() takes care that PCI_COMMAND_INTX_DISABLE > >> is cleared if a legacy interrupt is used. > > > > Only if pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin) returned a > > non-zero pin, if I read the code correctly. While I can't remember the > > context in which I wrote this piece of code, I suppose that pin == 0 > > was the situation where this test was needed. I mean, the board > > designer can legitimately not wire the interrupt pin, and require that > > polling is being used, right? > > I think we have such a use case, but it's handled in ACPI and results > in dev->irq == IRQ_NOTCONNECTED. But not all systems use ACPI. The i2c-i801 driver could be used on non-ACPI systems. I don't know if this is actually the case though. But we definitely allow building kernels with ACPI disabled and I2C_I801 enabled. > In case of pin == 0 pci_dev->irq is 0, and I'd expect that irq_to_desc(0) > returns NULL and request_threaded_irq() returns -EINVAL. This would > result in switching to polling. Reading the !CONFIG_SPARSE_IRQ version of that function, it doesn't seem so. irq_to_desc(0) would return &irq_desc[0]. IRQ 0 is not invalid, it was the system clock on legacy PC systems, and probably still is for compatibility reasons. I suppose the CONFIG_SPARSE_IRQ version of irq_to_desc() is compatible with that too. That being said, I suppose IRQ 0 is requested early at boot, so the i2c-i801 driver would get -EBUSY or similar when trying to request it, which in turn would result in falling back to polling mode, which is what we want. > Having said that I see no scenario where the check would be needed. > > > In your favor, I can't find any online kernel log with this message. > > However that doesn't mean I'm comfortable removing the safety check. I'm still uncertain about what to do here. On the one hand, the check can't hurt, and if we hit a corner case, could provide useful debugging information. On the other hand, it may be dead code if you are correct, and I don't like dead code. I suppose we could remove the code for now, and see if anyone reports a regression. -- Jean Delvare SUSE L3 Support
