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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9D8F1C433EF for ; Sat, 18 Jun 2022 17:22:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236273AbiFRRWP (ORCPT ); Sat, 18 Jun 2022 13:22:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45610 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231142AbiFRRWM (ORCPT ); Sat, 18 Jun 2022 13:22:12 -0400 Received: from sin.source.kernel.org (sin.source.kernel.org [IPv6:2604:1380:40e1:4800::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5A8DD140CF for ; Sat, 18 Jun 2022 10:22:11 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sin.source.kernel.org (Postfix) with ESMTPS id CBBE5CE02C5 for ; Sat, 18 Jun 2022 17:22:09 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 19120C3411A; Sat, 18 Jun 2022 17:22:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1655572928; bh=jhTcRmVj6BvTv+laefnZT4jEHAzQmvt5bGfy9OB8ugg=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=AEbQM4oIogvyPY8KDUlGb7J+kpZOdzMHXN9laD3GgkC3Jx6xSplXnm7hgiDkNcLTE DpW9IDv2oiQnsHmSANM328O0W5ObtBrxWhvmIOQqWxEZ38emSQMm5JhPm3ZId2m7Es vi3INLOOSYF22/WT9ZuNlOf+6auTv53gZ26b9PnIh13DaYuRdsAv0hGFMcUcTWmRKq jf5N2fw0QkPL1HG+tYhiqpnqo55djqgAcCdIvIzQqDizg2y1QI4FZygYNPJRlM0YNO mLSSvJWo3KDjh7EqAclabWyV9Ez5Avlqfn6zIFoCq5twB8YIMJP2O8sju+kPUU/893 f6yMjOZvIrEVw== Received: from 82-132-215-29.dab.02.net ([82.132.215.29] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1o2c97-001VtI-MU; Sat, 18 Jun 2022 18:22:05 +0100 Date: Sat, 18 Jun 2022 18:22:03 +0100 Message-ID: <877d5dga4k.wl-maz@kernel.org> From: Marc Zyngier To: Jianmin Lv Cc: Thomas Gleixner , linux-kernel@vger.kernel.org, Hanjun Guo , Lorenzo Pieralisi , Jiaxun Yang , Huacai Chen Subject: Re: [PATCH V12 04/10] irqchip: create library file for LoongArch irqchip driver In-Reply-To: <1655273250-23495-5-git-send-email-lvjianmin@loongson.cn> References: <1655273250-23495-1-git-send-email-lvjianmin@loongson.cn> <1655273250-23495-5-git-send-email-lvjianmin@loongson.cn> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 82.132.215.29 X-SA-Exim-Rcpt-To: lvjianmin@loongson.cn, tglx@linutronix.de, linux-kernel@vger.kernel.org, guohanjun@huawei.com, lorenzo.pieralisi@arm.com, jiaxun.yang@flygoat.com, chenhuacai@loongson.cn X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 15 Jun 2022 07:07:24 +0100, Jianmin Lv wrote: > > The library file contains following content: > - Implement acpi_get_gsi_domain_id callback. > - Implement initialization of vector group entries and APIs > for building hierachy irqdomains. > > Signed-off-by: Jianmin Lv > --- > drivers/irqchip/Makefile | 2 +- > drivers/irqchip/irq-loongarch-pic-common.c | 122 +++++++++++++++++++++++++++++ > drivers/irqchip/irq-loongarch-pic-common.h | 39 +++++++++ > 3 files changed, 162 insertions(+), 1 deletion(-) > create mode 100644 drivers/irqchip/irq-loongarch-pic-common.c > create mode 100644 drivers/irqchip/irq-loongarch-pic-common.h > > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile > index 6894a13..2d0d871 100644 > --- a/drivers/irqchip/Makefile > +++ b/drivers/irqchip/Makefile > @@ -103,7 +103,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o > obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o > obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o > obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o > -obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o > +obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o irq-loongarch-pic-common.o > obj-$(CONFIG_LOONGSON_LIOINTC) += irq-loongson-liointc.o > obj-$(CONFIG_LOONGSON_HTPIC) += irq-loongson-htpic.o > obj-$(CONFIG_LOONGSON_HTVEC) += irq-loongson-htvec.o > diff --git a/drivers/irqchip/irq-loongarch-pic-common.c b/drivers/irqchip/irq-loongarch-pic-common.c > new file mode 100644 > index 0000000..2f75362 > --- /dev/null > +++ b/drivers/irqchip/irq-loongarch-pic-common.c > @@ -0,0 +1,122 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2022 Loongson Limited, All Rights Reserved. > + */ > + > +#include > +#include > +#include > +#include "irq-loongarch-pic-common.h" > + > +static struct acpi_vector_group vector_group[MAX_IO_PICS]; > + > +struct acpi_madt_bio_pic *acpi_pchpic[MAX_IO_PICS]; > + > +struct fwnode_handle *liointc_handle; > +struct fwnode_handle *pch_lpc_handle; > +struct fwnode_handle *pch_msi_handle[MAX_IO_PICS]; > +struct fwnode_handle *pch_pic_handle[MAX_IO_PICS]; Why aren't these in individual drivers, and then have accessors to retrieve them? > + > +static int find_pch_pic(u32 gsi) > +{ > + int i, start, end; > + > + /* Find the PCH_PIC that manages this GSI. */ > + for (i = 0; i < MAX_IO_PICS; i++) { > + struct acpi_madt_bio_pic *irq_cfg = acpi_pchpic[i]; > + > + if (!irq_cfg) > + return -1; > + > + start = irq_cfg->gsi_base; > + end = irq_cfg->gsi_base + irq_cfg->size; > + if (gsi >= start && gsi < end) > + return i; > + } > + > + pr_err("ERROR: Unable to locate PCH_PIC for GSI %d\n", gsi); > + return -1; > +} Same thing. This really should be in the PCH driver, and be called by lpic_get_gsi_domain(). > + > +struct fwnode_handle *lpic_get_gsi_domain_id(u32 gsi) > +{ > + int id; > + struct fwnode_handle *domain_handle = NULL; > + > + switch (gsi) { > + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ: > + if (liointc_handle) > + domain_handle = liointc_handle; > + break; > + > + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ: > + if (pch_lpc_handle) > + domain_handle = pch_lpc_handle; > + break; > + > + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ: > + id = find_pch_pic(gsi); > + if (id >= 0 && pch_pic_handle[id]) > + domain_handle = pch_pic_handle[id]; > + > + break; > + } > + > + return domain_handle; > +} > + > +static int pci_mcfg_parse(struct acpi_table_header *header) > +{ > + struct acpi_table_mcfg *mcfg; > + struct acpi_mcfg_allocation *mptr; > + int i, n; > + > + if (header->length < sizeof(struct acpi_table_mcfg)) > + return -EINVAL; > + > + n = (header->length - sizeof(struct acpi_table_mcfg)) / > + sizeof(struct acpi_mcfg_allocation); > + mcfg = (struct acpi_table_mcfg *)header; > + mptr = (struct acpi_mcfg_allocation *) &mcfg[1]; > + > + for (i = 0; i < n; i++, mptr++) > + vector_group[mptr->pci_segment].node = (mptr->address >> 44) & 0xf; > + > + return 0; > +} > + > +void __init init_vector_parent_group(void) > +{ > + acpi_table_parse(ACPI_SIG_MCFG, pci_mcfg_parse); > +} I really don't think the PCI code should be anywhere near this. Frankly, this file looks like a dumping ground for totally unrelated stuff. > + > +void acpi_set_vector_parent(int node, struct irq_domain *parent) > +{ > + int i; > + > + if (cpu_has_flatmode) > + node = cpu_to_node(node * CORES_PER_EIO_NODE); > + > + for (i = 0; i < MAX_IO_PICS; i++) { > + if (node == vector_group[i].node) { > + vector_group[i].parent = parent; > + return; > + } > + } > +} > + > +struct irq_domain *acpi_get_msi_parent(int index) > +{ > + return vector_group[index].parent; > +} > + > +struct irq_domain *acpi_get_pch_parent(int node) > +{ > + int i; > + > + for (i = 0; i < MAX_IO_PICS; i++) { > + if (node == vector_group[i].node) > + return vector_group[i].parent; > + } > + return NULL; > +} Same thing. There is nothing "common" here. All this should be split in the various drivers, where they belong. M. -- Without deviation from the norm, progress is not possible.