Message ID | 20221020082205.20505-2-lvjianmin@loongson.cn |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4ac7:0:0:0:0:0 with SMTP id y7csp136205wrs; Thu, 20 Oct 2022 01:34:49 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5DUb5/lXt0qsz0ScKfj/+ofA0B+bUL+sUeBp7zQIRyRco1hDPEWOkhWbamc3Dvm19nkyDR X-Received: by 2002:a17:906:5dda:b0:78d:e7d2:7499 with SMTP id p26-20020a1709065dda00b0078de7d27499mr10108720ejv.588.1666254889452; Thu, 20 Oct 2022 01:34:49 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666254889; cv=none; d=google.com; s=arc-20160816; b=eAJRuijph+y+AmDjPt6IYQxcpn7J+cgX0NXcqf8ZRKv/l9sivlu6IPQSco7JrxmVCI 5iOSZ22Z961jflzPCk2bjPr0jz7ILylBh9M0+Wu78M8n1vu+qAkWFuY2/45LpFBbkBwg FQcjrQpVnY/9aYB7lstv3D/gmNcNt96HERdiKmOehx15h0g76OvpXGtlUr9mud4jH/Qs zCJgXq3965REZCXUGx2qMCzI81x5qdSpYi4QzB+GfxcaayASqoskxqN4LEkD4pp3Kn9O 3I7+Id7LXbQww743RILQIq50uY8HJMCSjHoFncqTUhL5AvbASNNazkuq63TJPpsnEZMN vdHw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=kSuFB1QWIffB/NcM+KOoJZWOVxC0FRjVYnrK+9hs5ts=; b=RKN8eQ8HiW7QyzfblEjVo4Aax+PQSrv6w4ig9XH6XjrjGwEaDFhVGWM0cadCYtIMZ4 YAApKHDx1p8CI4aQwfqUfmVlmqaF65NItDvRy2VoZD1R4KwFN9t3uoYGGm7Gm0I2CdKm H6kIux2i19pRTDMO0Lo48a6qEqqCwRHmTo/Mszs0LlAbLXjO1s1nA2DTjx3sFIDNTefp z1No/kC03oG4j2vlEGSuhEpp1rDzSh5svSpg6fKcWw6dh49YuZ39TbbXMeuHnBs5Pi9T gNrXKhi1oyw1WCIjYYpEfa8mmQz6Dj/uTzjDwaAlZtoX+ZFPp/OmlQDWxbRqLWmLrpol VRkQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id k5-20020a1709062a4500b0079195ddab1bsi5995270eje.221.2022.10.20.01.34.22; Thu, 20 Oct 2022 01:34:49 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230441AbiJTIWa (ORCPT <rfc822;realc9580@gmail.com> + 99 others); Thu, 20 Oct 2022 04:22:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34460 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230280AbiJTIWK (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 20 Oct 2022 04:22:10 -0400 Received: from loongson.cn (mail.loongson.cn [114.242.206.163]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 6FB6215B110; Thu, 20 Oct 2022 01:22:07 -0700 (PDT) Received: from loongson.cn (unknown [10.20.42.32]) by gateway (Coremail) with SMTP id _____8BxLtsuBVFjJvwAAA--.5004S3; Thu, 20 Oct 2022 16:22:06 +0800 (CST) Received: from loongson-pc.loongson.cn (unknown [10.20.42.32]) by localhost.localdomain (Coremail) with SMTP id AQAAf8CxLuItBVFjxM4BAA--.7555S3; Thu, 20 Oct 2022 16:22:05 +0800 (CST) From: Jianmin Lv <lvjianmin@loongson.cn> To: Thomas Gleixner <tglx@linutronix.de>, Marc Zyngier <maz@kernel.org> Cc: linux-kernel@vger.kernel.org, loongarch@lists.linux.dev, Jiaxun Yang <jiaxun.yang@flygoat.com>, Huacai Chen <chenhuacai@loongson.cn>, Bjorn Helgaas <bhelgaas@google.com>, Len Brown <lenb@kernel.org>, rafael@kernel.org, linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org Subject: [PATCH V4 1/4] ACPI / PCI: fix LPIC IRQ model default PCI IRQ polarity Date: Thu, 20 Oct 2022 16:22:02 +0800 Message-Id: <20221020082205.20505-2-lvjianmin@loongson.cn> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20221020082205.20505-1-lvjianmin@loongson.cn> References: <20221020082205.20505-1-lvjianmin@loongson.cn> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-CM-TRANSID: AQAAf8CxLuItBVFjxM4BAA--.7555S3 X-CM-SenderInfo: 5oymxthqpl0qxorr0wxvrqhubq/ X-Coremail-Antispam: 1Uk129KBjvJXoWxJrW3XFW3ZFWfZFyktFWxXrb_yoW8ArWxpF ZF93Wayr48tFs8XwsFk3WxZry5J3Z3Cryjkrs5Cw4Uua1Dur4IqFyxWFW3Jr98WFZrZa1U ZryYyw48Way7uFJanT9S1TB71UUUUj7qnTZGkaVYY2UrUUUUj1kv1TuYvTs0mT0YCTnIWj qI5I8CrVACY4xI64kE6c02F40Ex7xfYxn0WfASr-VFAUDa7-sFnT9fnUUIcSsGvfJTRUUU bS8YFVCjjxCrM7AC8VAFwI0_Jr0_Gr1l1xkIjI8I6I8E6xAIw20EY4v20xvaj40_Wr0E3s 1l1IIY67AEw4v_JF0_JFyl8cAvFVAK0II2c7xJM28CjxkF64kEwVA0rcxSw2x7M28EF7xv wVC0I7IYx2IY67AKxVW8JVW5JwA2z4x0Y4vE2Ix0cI8IcVCY1x0267AKxVW8JVWxJwA2z4 x0Y4vEx4A2jsIE14v26r4UJVWxJr1l84ACjcxK6I8E87Iv6xkF7I0E14v26F4UJVW0owAa w2AFwI0_JF0_Jw1le2I262IYc4CY6c8Ij28IcVAaY2xG8wAqjxCEc2xF0cIa020Ex4CE44 I27wAqx4xG64xvF2IEw4CE5I8CrVC2j2WlYx0E2Ix0cI8IcVAFwI0_Jw0_WrylYx0Ex4A2 jsIE14v26r4j6F4UMcvjeVCFs4IE7xkEbVWUJVW8JwACjcxG0xvY0x0EwIxGrwCY1x0262 kKe7AKxVWUAVWUtwCF04k20xvY0x0EwIxGrwCFx2IqxVCFs4IE7xkEbVWUJVW8JwCFI7km 07C267AKxVWUAVWUtwC20s026c02F40E14v26r1j6r18MI8I3I0E7480Y4vE14v26r106r 1rMI8E67AF67kF1VAFwI0_Jw0_GFylIxkGc2Ij64vIr41lIxAIcVC0I7IYx2IY67AKxVWU CVW8JwCI42IY6xIIjxv20xvEc7CjxVAFwI0_Gr0_Cr1lIxAIcVCF04k26cxKx2IYs7xG6r 1j6r1xMIIF0xvEx4A2jsIE14v26r4j6F4UMIIF0xvEx4A2jsIEc7CjxVAFwI0_Gr0_Gr1U YxBIdaVFxhVjvjDU0xZFpf9x07jr6p9UUUUU= X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS, SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747194886558016070?= X-GMAIL-MSGID: =?utf-8?q?1747194886558016070?= |
Series |
irqchip: Support to set irq type for ACPI path
|
|
Commit Message
吕建民
Oct. 20, 2022, 8:22 a.m. UTC
On LoongArch ACPI based systems, the PCI devices (e.g. sata
controlers and PCI-to-to PCI bridge controlers) existed in
Loongson chipsets output high-level interrupt signal to the
interrupt controller they connected to, while the IRQs are
active low from the perspective of PCI(in 2.2.6. Interrupt
Pins, "Interrupts on PCI are optional and defined as level
sensitive, asserted low), which means that the interrupt
output of PCI devices plugged into PCI-to-to PCI bridges of
Loongson chipset will be also converted to high-level. So
high level triggered type is required to be passed to
acpi_register_gsi() when creating mappings for PCI devices.
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
---
drivers/acpi/pci_irq.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
Comments
On Thu, Oct 20, 2022 at 04:22:02PM +0800, Jianmin Lv wrote: > On LoongArch ACPI based systems, the PCI devices (e.g. sata > controlers and PCI-to-to PCI bridge controlers) existed in > Loongson chipsets output high-level interrupt signal to the > interrupt controller they connected to, I assume the active high behavior is hardware behavior that is independent of the fact that you're using ACPI firmware on the hardware. If so, I would omit "ACPI based". s/sata/SATA/ s/controlers/controllers/ (twice) s/PCI-to-to PCI/PCI-to-PCI/ s/existed in/in/ s/they connected/they are connected/ > while the IRQs are > active low from the perspective of PCI(in 2.2.6. Interrupt > Pins, "Interrupts on PCI are optional and defined as level > sensitive, asserted low), I don't think you need this spec reference, since "asserted low" is the standard thing that happens everywhere. But if you do want it, it needs to specify which spec it refers to, e.g., "Conventional PCI r3.0, sec 2.2.6" so it's not confused with the PCIe spec. The quote from the spec itself should be terminated with a close quote ("), i.e., "Interrupts on PCI ... asserted low" > which means that the interrupt > output of PCI devices plugged into PCI-to-to PCI bridges of > Loongson chipset will be also converted to high-level. So > high level triggered type is required to be passed to > acpi_register_gsi() when creating mappings for PCI devices. This is the part where I was hoping for a reference to a spec that talks about how PCI interrupts are inverted. The inverter is the part that's special here. I see that ACPI r6.5, sec 5.2.12, mentions LPIC, but it doesn't mention the inverter. It has a lot more mentions of GIC, but also no details about an inverter. I suppose that would be in the GIC spec, which I'm not familiar with. The point is that one should be able to write this code from a spec, without having to empirically discover the interrupt polarity. What spec tells you about using ACTIVE_HIGH here? s/PCI-to-to PCI/PCI-to-PCI/ again Rewrap the log to fill 75 columns like the rest of the history. > Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn> > --- > drivers/acpi/pci_irq.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c > index 08e15774fb9f..ff30ceca2203 100644 > --- a/drivers/acpi/pci_irq.c > +++ b/drivers/acpi/pci_irq.c > @@ -387,13 +387,15 @@ int acpi_pci_irq_enable(struct pci_dev *dev) > u8 pin; > int triggering = ACPI_LEVEL_SENSITIVE; > /* > - * On ARM systems with the GIC interrupt model, level interrupts > + * On ARM systems with the GIC interrupt model, or LoongArch > + * systems with the LPIC interrupt model, level interrupts Is "LoongArch" required in this comment? Might the LPIC model be used on non-LoongArch systems? I see it follows the example of "ARM systems". In my opinion, "ARM" probably should be removed, too, because the code checks only for GIC or LPIC; it doesn't check for ARM or LoongArch. If GIC is restricted to ARM and LPIC is restricted to LoongArch, that's fine, but that constraint should be expressed somewhere else and doesn't need to be repeated here. > * are always polarity high by specification; PCI legacy > * IRQs lines are inverted before reaching the interrupt > * controller and must therefore be considered active high > * as default. > */ > - int polarity = acpi_irq_model == ACPI_IRQ_MODEL_GIC ? > + int polarity = acpi_irq_model == ACPI_IRQ_MODEL_GIC || > + acpi_irq_model == ACPI_IRQ_MODEL_LPIC ? > ACPI_ACTIVE_HIGH : ACPI_ACTIVE_LOW; > char *link = NULL; > char link_desc[16]; > -- > 2.31.1 >
On 2022/10/21 上午12:47, Bjorn Helgaas wrote: > On Thu, Oct 20, 2022 at 04:22:02PM +0800, Jianmin Lv wrote: >> On LoongArch ACPI based systems, the PCI devices (e.g. sata >> controlers and PCI-to-to PCI bridge controlers) existed in >> Loongson chipsets output high-level interrupt signal to the >> interrupt controller they connected to, > > I assume the active high behavior is hardware behavior that is > independent of the fact that you're using ACPI firmware on the > hardware. If so, I would omit "ACPI based". > > s/sata/SATA/ > s/controlers/controllers/ (twice) > s/PCI-to-to PCI/PCI-to-PCI/ > s/existed in/in/ > s/they connected/they are connected/ > Ok, thanks, I'll improve them again. >> while the IRQs are >> active low from the perspective of PCI(in 2.2.6. Interrupt >> Pins, "Interrupts on PCI are optional and defined as level >> sensitive, asserted low), > > I don't think you need this spec reference, since "asserted low" is > the standard thing that happens everywhere. But if you do want it, it > needs to specify which spec it refers to, e.g., "Conventional PCI > r3.0, sec 2.2.6" so it's not confused with the PCIe spec. > > The quote from the spec itself should be terminated with a close quote > ("), i.e., > > "Interrupts on PCI ... asserted low" > Ok, thanks, I'll specify spec version with correct pattern. >> which means that the interrupt >> output of PCI devices plugged into PCI-to-to PCI bridges of >> Loongson chipset will be also converted to high-level. So >> high level triggered type is required to be passed to >> acpi_register_gsi() when creating mappings for PCI devices. > > This is the part where I was hoping for a reference to a spec that > talks about how PCI interrupts are inverted. The inverter is the part > that's special here. > > I see that ACPI r6.5, sec 5.2.12, mentions LPIC, but it doesn't > mention the inverter. It has a lot more mentions of GIC, but also no > details about an inverter. I suppose that would be in the GIC spec, > which I'm not familiar with. > > The point is that one should be able to write this code from a spec, > without having to empirically discover the interrupt polarity. What > spec tells you about using ACTIVE_HIGH here? > Yes, no mentions for the inverter in ACPI spec, the description about device interrupt type can be found in Loongson chipset manual: https://github.com/loongson/LoongArch-Documentation/blob/main/docs/Loongson-7A1000-usermanual-EN/interrupt-controller/device-interrupt-types.adoc where the interrupts coming from interrupt source are level triggered and active high except some specific device such as AC97 DMA and GPIO. > s/PCI-to-to PCI/PCI-to-PCI/ again > > Rewrap the log to fill 75 columns like the rest of the history. > Ok, thanks. >> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn> >> --- >> drivers/acpi/pci_irq.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c >> index 08e15774fb9f..ff30ceca2203 100644 >> --- a/drivers/acpi/pci_irq.c >> +++ b/drivers/acpi/pci_irq.c >> @@ -387,13 +387,15 @@ int acpi_pci_irq_enable(struct pci_dev *dev) >> u8 pin; >> int triggering = ACPI_LEVEL_SENSITIVE; >> /* >> - * On ARM systems with the GIC interrupt model, level interrupts >> + * On ARM systems with the GIC interrupt model, or LoongArch >> + * systems with the LPIC interrupt model, level interrupts > > Is "LoongArch" required in this comment? Might the LPIC model be used > on non-LoongArch systems? > Just like GIC is restricted to ARM, and LPIC is restricted to LoongArch, as you mentioned below. So LPIC model will be not used on non-LoongArch systems. > I see it follows the example of "ARM systems". In my opinion, "ARM" > probably should be removed, too, because the code checks only for GIC > or LPIC; it doesn't check for ARM or LoongArch. > > If GIC is restricted to ARM and LPIC is restricted to LoongArch, > that's fine, but that constraint should be expressed somewhere else > and doesn't need to be repeated here. > Though the definition and constraints for GIC and LPIC are explicitly expressed in ACPI spec, to be clear, repeating the relation here only with short words maybe worthy so that people understand the workaround conveniently without having to referencing ACPI spec, right? >> * are always polarity high by specification; PCI legacy >> * IRQs lines are inverted before reaching the interrupt >> * controller and must therefore be considered active high >> * as default. >> */ >> - int polarity = acpi_irq_model == ACPI_IRQ_MODEL_GIC ? >> + int polarity = acpi_irq_model == ACPI_IRQ_MODEL_GIC || >> + acpi_irq_model == ACPI_IRQ_MODEL_LPIC ? >> ACPI_ACTIVE_HIGH : ACPI_ACTIVE_LOW; >> char *link = NULL; >> char link_desc[16]; >> -- >> 2.31.1 >>
On Fri, Oct 21, 2022 at 09:58:57AM +0800, Jianmin Lv wrote: > On 2022/10/21 上午12:47, Bjorn Helgaas wrote: > > On Thu, Oct 20, 2022 at 04:22:02PM +0800, Jianmin Lv wrote: > > > On LoongArch ACPI based systems, the PCI devices (e.g. sata > > > controlers and PCI-to-to PCI bridge controlers) existed in > > > Loongson chipsets output high-level interrupt signal to the > > > interrupt controller they connected to, > > The point is that one should be able to write this code from a spec, > > without having to empirically discover the interrupt polarity. What > > spec tells you about using ACTIVE_HIGH here? > > > Yes, no mentions for the inverter in ACPI spec, the description about > device interrupt type can be found in Loongson chipset manual: > > https://github.com/loongson/LoongArch-Documentation/blob/main/docs/Loongson-7A1000-usermanual-EN/interrupt-controller/device-interrupt-types.adoc That's the kind of reference I was looking for. The link to HTML is convenient in some ways, but since specs evolve over time and URLs are ephemeral, I think a citation like "Loongson 7A1000 Bridge User Manual v2.00, sec 5.3" is more likely to be useful far in the future. Bjorn
On 2022/10/21 下午8:01, Bjorn Helgaas wrote: > On Fri, Oct 21, 2022 at 09:58:57AM +0800, Jianmin Lv wrote: >> On 2022/10/21 上午12:47, Bjorn Helgaas wrote: >>> On Thu, Oct 20, 2022 at 04:22:02PM +0800, Jianmin Lv wrote: >>>> On LoongArch ACPI based systems, the PCI devices (e.g. sata >>>> controlers and PCI-to-to PCI bridge controlers) existed in >>>> Loongson chipsets output high-level interrupt signal to the >>>> interrupt controller they connected to, > >>> The point is that one should be able to write this code from a spec, >>> without having to empirically discover the interrupt polarity. What >>> spec tells you about using ACTIVE_HIGH here? >>> >> Yes, no mentions for the inverter in ACPI spec, the description about >> device interrupt type can be found in Loongson chipset manual: >> >> https://github.com/loongson/LoongArch-Documentation/blob/main/docs/Loongson-7A1000-usermanual-EN/interrupt-controller/device-interrupt-types.adoc > > That's the kind of reference I was looking for. The link to HTML is > convenient in some ways, but since specs evolve over time and URLs are > ephemeral, I think a citation like "Loongson 7A1000 Bridge User Manual > v2.00, sec 5.3" is more likely to be useful far in the future. > Ok, good suggestion, thanks. > Bjorn >
diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c index 08e15774fb9f..ff30ceca2203 100644 --- a/drivers/acpi/pci_irq.c +++ b/drivers/acpi/pci_irq.c @@ -387,13 +387,15 @@ int acpi_pci_irq_enable(struct pci_dev *dev) u8 pin; int triggering = ACPI_LEVEL_SENSITIVE; /* - * On ARM systems with the GIC interrupt model, level interrupts + * On ARM systems with the GIC interrupt model, or LoongArch + * systems with the LPIC interrupt model, level interrupts * are always polarity high by specification; PCI legacy * IRQs lines are inverted before reaching the interrupt * controller and must therefore be considered active high * as default. */ - int polarity = acpi_irq_model == ACPI_IRQ_MODEL_GIC ? + int polarity = acpi_irq_model == ACPI_IRQ_MODEL_GIC || + acpi_irq_model == ACPI_IRQ_MODEL_LPIC ? ACPI_ACTIVE_HIGH : ACPI_ACTIVE_LOW; char *link = NULL; char link_desc[16];