On 6/29/21 2:38 AM, Bjorn Helgaas wrote: > On Thu, Jun 24, 2021 at 05:40:40PM -0500, Bjorn Helgaas wrote: [snip] >>> >>> So let's just move all the IRQ init before the pci_host_probe() call, that >>> will prevent issues like this and seems to be the correct thing to do too. >> >> Previously we registered rockchip_pcie_subsys_irq_handler() and >> rockchip_pcie_client_irq_handler() before the PCIe clocks were >> enabled. That's a problem because they depend on those clocks being >> enabled, and your patch fixes that. >> >> rockchip_pcie_legacy_int_handler() depends on rockchip->irq_domain, >> which isn't initialized until rockchip_pcie_init_irq_domain(). >> Previously we registered rockchip_pcie_legacy_int_handler() as the >> handler for the "legacy" IRQ before rockchip_pcie_init_irq_domain(). >> >> I think your patch *also* fixes that problem, right? > > The lack of consistency in how we use > irq_set_chained_handler_and_data() really bugs me. > > Your patch fixes the ordering issue where we installed > rockchip_pcie_legacy_int_handler() before initializing data > (rockchip->irq_domain) that it depends on. > > But AFAICT, rockchip still has the problem that we don't *unregister* > rockchip_pcie_legacy_int_handler() when the rockchip-pcie module is > removed. Doesn't this mean that if we unload the module, then receive > an interrupt from the device, we'll try to call a function that is no > longer present? > Good question, I don't to be honest. I'll have to dig deeper on this but my experience is that the module removal (and device unbind) is not that well tested on ARM device drivers in general. Best regards, -- Javier Martinez Canillas Software Engineer New Platform Technologies Enablement team RHEL Engineering
