From patchwork Tue Jun 8 08:04:09 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Javier Martinez Canillas X-Patchwork-Id: 12305671 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5FC5DC47082 for ; Tue, 8 Jun 2021 08:05:52 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2D4D060FF1 for ; Tue, 8 Jun 2021 08:05:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2D4D060FF1 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-Id:Date:Subject:Cc :To:From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=h7hytLBeSM+JdYIdrODBfWYHlv0P6yWHnCcjZ7QnYvM=; b=RA9WMuZWJu43lw xYPZ1g6gYzgWcRGiqgJ6nEr1RPBQ1+PyspN2F7e6g2/kpDrnqVfqqiRKdVtf4yFwM0e8omlXOxw92 P10oJB7X7KDtToT/cRtPKSmspqWfNy3EZK+sXbP/jLVWZJfvV+mQppdwf8R1ncYZ7DDlhu6591OJO ND0tLPOUlXuG+XlhOXwAO7y7qRbw3aAcV5XvUKw3pLeqRf3XarBKfkntsQhlzuDnLigJyvdvkPH8+ bt/xbwRkiXAdJPFkJAU31C08ErMYeyJ6tWsSG5QNYM/S4TcAuiaBNr7y2vJnwA/ydxAAES4w6GQs8 fI8T6Bic+XgqtetNPXKQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lqWik-006wCu-6N; Tue, 08 Jun 2021 08:04:22 +0000 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lqWif-006wB3-BD for linux-arm-kernel@lists.infradead.org; Tue, 08 Jun 2021 08:04:18 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623139456; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=eRLfIlExKhLKW3ohxS3QmcrC8PB6Pwunoa93O+xqbII=; b=h1P11D0dibi7H1ucSSL+RGF5dvtFe2kb9esmB08rGU92x58G+6oB0elZDP/wmRIjPPHew+ o4WSvt13l3O2NPZR66PZAgAqPHjSuAHGcdNmWbpzbZvKReQ2dCRmHRXhX/3U2mEKa7+Ab3 umd6bbBvoBaRJuFhUh36tvVs584r7ow= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-537-nlHXkPe2PjKXlu3lpmcRgg-1; Tue, 08 Jun 2021 04:04:14 -0400 X-MC-Unique: nlHXkPe2PjKXlu3lpmcRgg-1 Received: by mail-wm1-f69.google.com with SMTP id f13-20020a05600c154db0290195f6af2ea9so283140wmg.1 for ; Tue, 08 Jun 2021 01:04:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=eRLfIlExKhLKW3ohxS3QmcrC8PB6Pwunoa93O+xqbII=; b=Dqyp50PF1Mpe7PikTIDAYTXH14HDBl8D2joSRhS5UBKpxy2ivxtT5WB+jv4ghtLT8W 9QyJFJuM3jGVbJf+ObhIypMurX0JSCZki7r9TbfJ6UKWKpYL2AtkoYnVF363TeP4+N89 deE8SaHdD2X6J2s2wuFsURnJJNhuwxCO0AF6tRnmvDbSGe+9tpLMse9CATnTlgNEcZfd wFroY1psLItr1HtYEzmLQ8IF0c7VkO79nJz97Vtdy9l5Uk4OyPXgPTMcqlr/WcePGoEf +1+7ivAJm6TPUdhHZ9KtHHJJ2eWTPmcmJ3/qkD60XJ+Tf54S01oSmpJOLaria1Vf9lg9 G1SA== X-Gm-Message-State: AOAM530Dh3/eloWft2wXsAzr9hAeXCX62+75xIfrFeqSvk9KxuRhAgf5 tz3Z6X7U7KNyiiVuun2m8Wz21sjzt+HMiNpMLQeneK9gtCIo+7gKKCKQnC/OCJFgxF44P8mZJPW CrdJ0dWekURWXCzzIsUqM15LiddTB1MwnH+s= X-Received: by 2002:a5d:5983:: with SMTP id n3mr8801724wri.241.1623139453734; Tue, 08 Jun 2021 01:04:13 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwMZdm0lOV6CTzcaqjiMsts4ZrMuE5GRipe8Uouaz7yC1zHJCdwRHLBh8j5A6Gyo/laf6JPcw== X-Received: by 2002:a5d:5983:: with SMTP id n3mr8801701wri.241.1623139453534; Tue, 08 Jun 2021 01:04:13 -0700 (PDT) Received: from minerva.redhat.com ([92.176.231.106]) by smtp.gmail.com with ESMTPSA id q11sm18515680wrx.80.2021.06.08.01.04.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 08 Jun 2021 01:04:13 -0700 (PDT) From: Javier Martinez Canillas To: linux-kernel@vger.kernel.org Cc: Javier Martinez Canillas , Peter Robinson , Shawn Lin , Bjorn Helgaas , Heiko Stuebner , Lorenzo Pieralisi , Rob Herring , linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org, linux-rockchip@lists.infradead.org Subject: [PATCH v2] PCI: rockchip: Avoid accessing PCIe registers with clocks gated Date: Tue, 8 Jun 2021 10:04:09 +0200 Message-Id: <20210608080409.1729276-1-javierm@redhat.com> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=javierm@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210608_010417_480971_4881C5BF X-CRM114-Status: GOOD ( 21.49 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org IRQ handlers that are registered for shared interrupts can be called at any time after have been registered using the request_irq() function. It's up to drivers to ensure that's always safe for these to be called. Both the "pcie-sys" and "pcie-client" interrupts are shared, but since their handlers are registered very early in the probe function, an error later can lead to these handlers being executed before all the required resources have been properly setup. For example, the rockchip_pcie_read() function used by these IRQ handlers expects that some PCIe clocks will already be enabled, otherwise trying to access the PCIe registers causes the read to hang and never return. The CONFIG_DEBUG_SHIRQ option tests if drivers are able to cope with their shared interrupt handlers being called, by generating a spurious interrupt just before a shared interrupt handler is unregistered. But this means that if the option is enabled, any error in the probe path of this driver could lead to one of the IRQ handlers to be executed. In a rockpro64 board, the following sequence of events happens: 1) "pcie-sys" IRQ is requested and its handler registered. 2) "pcie-client" IRQ is requested and its handler registered. 3) probe later fails due readl_poll_timeout() returning a timeout. 4) the "pcie-sys" IRQ is unregistered. 5) CONFIG_DEBUG_SHIRQ triggers a spurious interrupt. 6) "pcie-client" IRQ handler is called for this spurious interrupt. 7) IRQ handler tries to read PCIE_CLIENT_INT_STATUS with clocks gated. 8) the machine hangs because rockchip_pcie_read() call never returns. To avoid cases like this, the handlers don't have to be registered until very late in the probe function, once all the resources have been setup. 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. Reported-by: Peter Robinson Signed-off-by: Javier Martinez Canillas Acked-by: Shawn Lin Tested-by: Peter Robinson --- Changes in v2: - Add missing word in the commit message. - Include Shawn Lin's Acked-by tag. drivers/pci/controller/pcie-rockchip-host.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c index f1d08a1b159..78d04ac29cd 100644 --- a/drivers/pci/controller/pcie-rockchip-host.c +++ b/drivers/pci/controller/pcie-rockchip-host.c @@ -592,10 +592,6 @@ static int rockchip_pcie_parse_host_dt(struct rockchip_pcie *rockchip) if (err) return err; - err = rockchip_pcie_setup_irq(rockchip); - if (err) - return err; - rockchip->vpcie12v = devm_regulator_get_optional(dev, "vpcie12v"); if (IS_ERR(rockchip->vpcie12v)) { if (PTR_ERR(rockchip->vpcie12v) != -ENODEV) @@ -973,8 +969,6 @@ static int rockchip_pcie_probe(struct platform_device *pdev) if (err) goto err_vpcie; - rockchip_pcie_enable_interrupts(rockchip); - err = rockchip_pcie_init_irq_domain(rockchip); if (err < 0) goto err_deinit_port; @@ -992,6 +986,12 @@ static int rockchip_pcie_probe(struct platform_device *pdev) bridge->sysdata = rockchip; bridge->ops = &rockchip_pcie_ops; + err = rockchip_pcie_setup_irq(rockchip); + if (err) + goto err_remove_irq_domain; + + rockchip_pcie_enable_interrupts(rockchip); + err = pci_host_probe(bridge); if (err < 0) goto err_remove_irq_domain;