From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E3B3CC433EF for ; Fri, 8 Oct 2021 17:48:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C5F4560F6E for ; Fri, 8 Oct 2021 17:48:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231237AbhJHRuK (ORCPT ); Fri, 8 Oct 2021 13:50:10 -0400 Received: from foss.arm.com ([217.140.110.172]:38918 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231217AbhJHRuH (ORCPT ); Fri, 8 Oct 2021 13:50:07 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8332B1063; Fri, 8 Oct 2021 10:48:11 -0700 (PDT) Received: from lpieralisi (e121166-lin.cambridge.arm.com [10.1.196.255]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D6FFC3F766; Fri, 8 Oct 2021 10:48:09 -0700 (PDT) Date: Fri, 8 Oct 2021 18:48:03 +0100 From: Lorenzo Pieralisi To: Bjorn Andersson Cc: Bjorn Helgaas , Bjorn Helgaas , Rob Herring , Jingoo Han , Gustavo Pimentel , Krzysztof Wilczy??ski , Stanimir Varbanov , linux-arm-msm@vger.kernel.org, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, vidyas@nvidia.com Subject: Re: [PATCH v2 1/2] PCI: dwc: Perform host_init() before registering msi Message-ID: <20211008174803.GA32277@lpieralisi> References: <20210824202925.GA3491441@bjorn-Precision-5520> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org [+Vidya] On Tue, Aug 24, 2021 at 02:23:14PM -0700, Bjorn Andersson wrote: > 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. Vidya, can you look into this please ? In the meantime I would merge this series. Thanks, Lorenzo > > Regards, > Bjorn