On Tue 24 Aug 13:29 PDT 2021, Bjorn Helgaas wrote: > On Tue, Aug 24, 2021 at 01:15:49PM -0700, Bjorn Andersson wrote: > > On Tue 24 Aug 12:05 PDT 2021, Bjorn Helgaas wrote: > > > > > On Mon, Aug 23, 2021 at 08:49:57AM -0700, Bjorn Andersson wrote: > > > > On the Qualcomm sc8180x platform the bootloader does something related > > > > to PCI that leaves a pending "msi" interrupt, which with the current > > > > ordering often fires before init has a chance to enable the clocks that > > > > are necessary for the interrupt handler to access the hardware. > > > > > > > > Move the host_init() call before the registration of the "msi" interrupt > > > > handler to ensure the host driver has a chance to enable the clocks. > > > > > > Did you audit other drivers for similar issues? If they do, we should > > > fix them all at once. > > > > I only looked at the DesignWware drivers, in an attempt to find any > > issues the proposed reordering. > > > > The set of bugs causes by drivers registering interrupts before critical > > resources tends to be rather visible and I don't know if there's much > > value in speculatively "fixing" drivers. > > > > E.g. a quick look through the drivers I see a similar pattern in > > pci-tegra.c, but it's unlikely that they have the similar problem in > > practice and I have no way to validate that a change to the order would > > have a positive effect - or any side effects. > > > > Or am I misunderstanding your request? > > That is exactly my request. Okay. > I'm not sure if the potential issue you > noticed in pci-tegra.c is similar to the one I mentioned here: > > https://lore.kernel.org/linux-pci/20210624224040.GA3567297@bjorn-Precision-5520/ > As I still have the tegra driver open, I share your concern about the use of potentially uninitialized variables. The problem I was concerned about was however the same as in my patch and the rockchip one, that if the tegra hardware isn't clocked the pm_runtime_get_sync() (which would turn on power and clock) happens after setting up the msi chain handler... > but I am actually in favor of speculatively fixing drivers even though > they're hard to test. Code like this tends to get copied to other > places, and fixing several drivers sometimes exposes opportunities for > refactoring and sharing code. > Looking through the other cases mentioned in your reply above certainly gives a feeling that this problem has been inherited from driver to driver... I've added a ticket to my backlog to take a deeper look at this. Regards, Bjorn
