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;
Subject: Re: [PATCH V3 1/4] ACPI / PCI: fix LPIC irq model default PCI IRQ
 polarity
To:     Bjorn Helgaas <helgaas@kernel.org>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Marc Zyngier <maz@kernel.org>, 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
References: <20221019160529.GA18372@bhelgaas>
From:   Jianmin Lv <lvjianmin@loongson.cn>
Message-ID: <0ebec963-d835-39cc-537a-bd58855b0171@loongson.cn>
Date:   Thu, 20 Oct 2022 11:16:35 +0800
User-Agent: Mozilla/5.0 (X11; Linux loongarch64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <20221019160529.GA18372@bhelgaas>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

Ok, thanks, I'll improve them as your suggestion.

On 2022/10/20 上午12:05, Bjorn Helgaas wrote:
> In the subject and the commit log below, figure out whether you want
> to use "irq" or "IRQ" and do it consistently.  I vote for "IRQ".  Also
> consider subject lines for the other patches.  Stuff like this is
> trivial, but it's an excuse for whoever will handle this (not me in
> this case) to put it off.  I also add "()" after function names for
> clarity.
> 
> On Sun, Oct 09, 2022 at 02:44:28PM +0800, Jianmin Lv wrote:
>> On LoongArch ACPI based systems, the irq trigger type of PCI devices
>> is high level, so high level triggered type is required to pass
>> to acpi_register_gsi when create irq mapping for PCI devices.
> 
> This isn't worded quite right.  The trigger type of PCI devices
> doesn't change just because you plug them into a LoongArch system
> instead of an x86 system.  The comment in the code reads better: The
> IRQs are active low from the perspective of PCI, but are inverted
> before the interrupt controller.
> 
> Including a reference here to the spec that mentions this inversion
> would help a lot.
> 
> s/when create mapping/when creating mappings/
> 
>> 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
>>   	 * 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
>>