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 X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 91ECEC64EB1 for ; Fri, 7 Dec 2018 09:46:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5A8C4208E7 for ; Fri, 7 Dec 2018 09:46:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5A8C4208E7 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-pci-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725985AbeLGJqE (ORCPT ); Fri, 7 Dec 2018 04:46:04 -0500 Received: from foss.arm.com ([217.140.101.70]:40412 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725976AbeLGJqE (ORCPT ); Fri, 7 Dec 2018 04:46:04 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DD426EBD; Fri, 7 Dec 2018 01:46:03 -0800 (PST) Received: from [10.1.196.62] (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3495B3F59C; Fri, 7 Dec 2018 01:46:02 -0800 (PST) Subject: Re: [PATCH 3/3] PCI: designware: Move interrupt acking into the proper callback To: Kishon Vijay Abraham I Cc: linux-pci@vger.kernel.org, Lorenzo Pieralisi , Bjorn Helgaas , Trent Piepho , Jingoo Han , Gustavo Pimentel , faiz_abbas@ti.com, Joao Pinto , Vignesh R References: <20181113225734.8026-1-marc.zyngier@arm.com> <20181113225734.8026-4-marc.zyngier@arm.com> <126f12da-b69d-ae3a-72bf-dc1bff22cd77@ti.com> <20181204134559.74957d43@why.wild-wind.fr.eu.org> <251318fd-c72c-3082-ac10-99f4312cbd52@ti.com> From: Marc Zyngier Openpgp: preference=signencrypt Autocrypt: addr=marc.zyngier@arm.com; prefer-encrypt=mutual; keydata= xsFNBE6Jf0UBEADLCxpix34Ch3kQKA9SNlVQroj9aHAEzzl0+V8jrvT9a9GkK+FjBOIQz4KE g+3p+lqgJH4NfwPm9H5I5e3wa+Scz9wAqWLTT772Rqb6hf6kx0kKd0P2jGv79qXSmwru28vJ t9NNsmIhEYwS5eTfCbsZZDCnR31J6qxozsDHpCGLHlYym/VbC199Uq/pN5gH+5JHZyhyZiNW ozUCjMqC4eNW42nYVKZQfbj/k4W9xFfudFaFEhAf/Vb1r6F05eBP1uopuzNkAN7vqS8XcgQH qXI357YC4ToCbmqLue4HK9+2mtf7MTdHZYGZ939OfTlOGuxFW+bhtPQzsHiW7eNe0ew0+LaL 3wdNzT5abPBscqXWVGsZWCAzBmrZato+Pd2bSCDPLInZV0j+rjt7MWiSxEAEowue3IcZA++7 ifTDIscQdpeKT8hcL+9eHLgoSDH62SlubO/y8bB1hV8JjLW/jQpLnae0oz25h39ij4ijcp8N t5slf5DNRi1NLz5+iaaLg4gaM3ywVK2VEKdBTg+JTg3dfrb3DH7ctTQquyKun9IVY8AsxMc6 lxl4HxrpLX7HgF10685GG5fFla7R1RUnW5svgQhz6YVU33yJjk5lIIrrxKI/wLlhn066mtu1 DoD9TEAjwOmpa6ofV6rHeBPehUwMZEsLqlKfLsl0PpsJwov8TQARAQABzSNNYXJjIFp5bmdp ZXIgPG1hcmMuenluZ2llckBhcm0uY29tPsLBewQTAQIAJQIbAwYLCQgHAwIGFQgCCQoLBBYC AwECHgECF4AFAk6NvYYCGQEACgkQI9DQutE9ekObww/+NcUATWXOcnoPflpYG43GZ0XjQLng LQFjBZL+CJV5+1XMDfz4ATH37cR+8gMO1UwmWPv5tOMKLHhw6uLxGG4upPAm0qxjRA/SE3LC 22kBjWiSMrkQgv5FDcwdhAcj8A+gKgcXBeyXsGBXLjo5UQOGvPTQXcqNXB9A3ZZN9vS6QUYN TXFjnUnzCJd+PVI/4jORz9EUVw1q/+kZgmA8/GhfPH3xNetTGLyJCJcQ86acom2liLZZX4+1 6Hda2x3hxpoQo7pTu+XA2YC4XyUstNDYIsE4F4NVHGi88a3N8yWE+Z7cBI2HjGvpfNxZnmKX 6bws6RQ4LHDPhy0yzWFowJXGTqM/e79c1UeqOVxKGFF3VhJJu1nMlh+5hnW4glXOoy/WmDEM UMbl9KbJUfo+GgIQGMp8mwgW0vK4HrSmevlDeMcrLdfbbFbcZLNeFFBn6KqxFZaTd+LpylIH bOPN6fy1Dxf7UZscogYw5Pt0JscgpciuO3DAZo3eXz6ffj2NrWchnbj+SpPBiH4srfFmHY+Y LBemIIOmSqIsjoSRjNEZeEObkshDVG5NncJzbAQY+V3Q3yo9og/8ZiaulVWDbcpKyUpzt7pv cdnY3baDE8ate/cymFP5jGJK++QCeA6u6JzBp7HnKbngqWa6g8qDSjPXBPCLmmRWbc5j0lvA 6ilrF8nOwU0ETol/RQEQAM/2pdLYCWmf3rtIiP8Wj5NwyjSL6/UrChXtoX9wlY8a4h3EX6E3 64snIJVMLbyr4bwdmPKULlny7T/R8dx/mCOWu/DztrVNQiXWOTKJnd/2iQblBT+W5W8ep/nS w3qUIckKwKdplQtzSKeE+PJ+GMS+DoNDDkcrVjUnsoCEr0aK3cO6g5hLGu8IBbC1CJYSpple VVb/sADnWF3SfUvJ/l4K8Uk4B4+X90KpA7U9MhvDTCy5mJGaTsFqDLpnqp/yqaT2P7kyMG2E w+eqtVIqwwweZA0S+tuqput5xdNAcsj2PugVx9tlw/LJo39nh8NrMxAhv5aQ+JJ2I8UTiHLX QvoC0Yc/jZX/JRB5r4x4IhK34Mv5TiH/gFfZbwxd287Y1jOaD9lhnke1SX5MXF7eCT3cgyB+ hgSu42w+2xYl3+rzIhQqxXhaP232t/b3ilJO00ZZ19d4KICGcakeiL6ZBtD8TrtkRiewI3v0 o8rUBWtjcDRgg3tWx/PcJvZnw1twbmRdaNvsvnlapD2Y9Js3woRLIjSAGOijwzFXSJyC2HU1 AAuR9uo4/QkeIrQVHIxP7TJZdJ9sGEWdeGPzzPlKLHwIX2HzfbdtPejPSXm5LJ026qdtJHgz BAb3NygZG6BH6EC1NPDQ6O53EXorXS1tsSAgp5ZDSFEBklpRVT3E0NrDABEBAAHCwV8EGAEC AAkFAk6Jf0UCGwwACgkQI9DQutE9ekMLBQ//U+Mt9DtFpzMCIHFPE9nNlsCm75j22lNiw6mX mx3cUA3pl+uRGQr/zQC5inQNtjFUmwGkHqrAw+SmG5gsgnM4pSdYvraWaCWOZCQCx1lpaCOl MotrNcwMJTJLQGc4BjJyOeSH59HQDitKfKMu/yjRhzT8CXhys6R0kYMrEN0tbe1cFOJkxSbV 0GgRTDF4PKyLT+RncoKxQe8lGxuk5614aRpBQa0LPafkirwqkUtxsPnarkPUEfkBlnIhAR8L kmneYLu0AvbWjfJCUH7qfpyS/FRrQCoBq9QIEcf2v1f0AIpA27f9KCEv5MZSHXGCdNcbjKw1 39YxYZhmXaHFKDSZIC29YhQJeXWlfDEDq6nIhvurZy3mSh2OMQgaIoFexPCsBBOclH8QUtMk a3jW/qYyrV+qUq9Wf3SKPrXf7B3xB332jFCETbyZQXqmowV+2b3rJFRWn5hK5B+xwvuxKyGq qDOGjof2dKl2zBIxbFgOclV7wqCVkhxSJi/QaOj2zBqSNPXga5DWtX3ekRnJLa1+ijXxmdjz hApihi08gwvP5G9fNGKQyRETePEtEAWt0b7dOqMzYBYGRVr7uS4uT6WP7fzOwAJC4lU7ZYWZ yVshCa0IvTtp1085RtT3qhh9mobkcZ+7cQOY+Tx2RGXS9WeOh2jZjdoWUv6CevXNQyOUXMM= Organization: ARM Ltd Message-ID: <1ff77544-fc1a-6569-1919-3f169319153f@arm.com> Date: Fri, 7 Dec 2018 09:45:59 +0000 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: <251318fd-c72c-3082-ac10-99f4312cbd52@ti.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On 07/12/2018 08:12, Kishon Vijay Abraham I wrote: > Hi Marc, > > On 04/12/18 7:15 PM, Marc Zyngier wrote: >> On Tue, 4 Dec 2018 15:50:32 +0530 >> Kishon Vijay Abraham I wrote: >> >>> Hi, >>> >>> On 14/11/18 4:27 AM, Marc Zyngier wrote: >>>> The write to the status register is really an ACK for the HW, >>>> and should be treated as such by the driver. Let's move it to the >>>> irq_ack callback, which will prevent people from moving it around >>>> in order to paper over other bugs. >>>> >>>> Signed-off-by: Marc Zyngier >>>> --- >>>> drivers/pci/controller/dwc/pcie-designware-host.c | 13 +++++++------ >>>> 1 file changed, 7 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c >>>> index 0a76948ed49e..f06e67c60593 100644 >>>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c >>>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c >>>> @@ -99,9 +99,6 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp) >>>> (i * MAX_MSI_IRQS_PER_CTRL) + >>>> pos); >>>> generic_handle_irq(irq); >>>> - dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + >>>> - (i * MSI_REG_CTRL_BLOCK_SIZE), >>>> - 4, 1 << pos); >>>> pos++; >>>> } >>>> } >>>> @@ -200,14 +197,18 @@ static void dw_pci_bottom_unmask(struct irq_data *data) >>>> >>>> static void dw_pci_bottom_ack(struct irq_data *d) >>>> { >>>> - struct msi_desc *msi = irq_data_get_msi_desc(d); >>>> - struct pcie_port *pp; >>>> + struct pcie_port *pp = irq_data_get_irq_chip_data(d); >>>> + unsigned int res, bit, ctrl; >>>> unsigned long flags; >>>> >>>> - pp = msi_desc_to_pci_sysdata(msi); >>>> + ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL; >>>> + res = ctrl * MSI_REG_CTRL_BLOCK_SIZE; >>>> + bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL; >>>> >>>> raw_spin_lock_irqsave(&pp->lock, flags); >>>> >>>> + dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + res, 4, 1 << bit); >>> >>> This register should be written only if msi_irq_ack callback is not populated >>> similar to other dw_pci_bottom_*() functions. >> >> Why? This was so far unconditionally written, and my understanding is >> that without this write, no further MSI can be delivered. > > Not all platforms invoke dw_handle_msi_irq() for handling MSI irq. > > Platforms that doesn't use the MSI functionality of Designware makes use of the > various callbacks like msi_irq_ack, msi_host_init etc., Keystone has MSI > controller in the Keystone wrapper, AM654 uses GIC ITS etc., > > The platforms that doesn't use MSI functionality of Designware doesn't have to > write to Designware's MSI configuration registers. Let's be clear: a platform that doesn't use the DW MSI functionality should never get anywhere this code. If they do, then that's a terrible bug, and it should be fixed by making the TI stuff standalone instead of calling into the internals. Frankly, this whole thing should be marked as BROKEN until it is sorted out for good. Thanks, M. -- Jazz is not dead. It just smells funny...