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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 0D894C001DC for ; Mon, 31 Jul 2023 12:09:15 +0000 (UTC) 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:In-Reply-To:References:Cc:To:Subject: From:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=r6SQuTPpcaDwpVv8WuOlTf3BOr6rCx5a932nwoHZjOg=; b=v06Z2tcCDHL4yi nOmfC9jkYHwfkaG6BHiB8QyW/gKWFo/biHTzC2dybqYRRyYi38T7rOlVTCCBmnn0tEyO8B6dstAVA geoQh5wtLk4Xk1SKjx0YgWjsjDShJxUvTfy1grw8zke21qwPIWYYLF6eZQpJW1hWnBFSd3KieCNsw P9hZ45EJXn73PXyBMPSWZx3U4/fefoj+pekg9uknnJFY9+FfxxvVg/hQiDcyHCXUI9UoxKc1pU5SC o4hc5jJ6eslW8x5zg8TMGF/cvtm/BEWmhtQNGK7N46zmk60elLr87PJXO5X3dOIc6Cl7NOig980km SotqpLfpVl1NqFlesaUw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qQRhl-00FVmW-0D; Mon, 31 Jul 2023 12:08:53 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qQRhh-00FVlO-1O for linux-arm-kernel@lists.infradead.org; Mon, 31 Jul 2023 12:08:51 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A407DD75; Mon, 31 Jul 2023 05:09:27 -0700 (PDT) Received: from [10.163.53.56] (unknown [10.163.53.56]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 362FA3F5A1; Mon, 31 Jul 2023 05:08:41 -0700 (PDT) Message-ID: <3ee165d7-3727-53cc-295d-a2108734952d@arm.com> Date: Mon, 31 Jul 2023 17:38:38 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 From: Anshuman Khandual Subject: Re: [PATCH 1/3] arm_pmu: acpi: Add a representative platform device for TRBE To: Will Deacon Cc: linux-arm-kernel@lists.infradead.org, suzuki.poulose@arm.com, Catalin Marinas , Mark Rutland , linux-kernel@vger.kernel.org References: <20230728112733.359620-1-anshuman.khandual@arm.com> <20230728112733.359620-2-anshuman.khandual@arm.com> <20230728144056.GE21718@willie-the-truck> Content-Language: en-US In-Reply-To: <20230728144056.GE21718@willie-the-truck> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230731_050849_562861_8ED6A3E8 X-CRM114-Status: GOOD ( 23.66 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 7/28/23 20:10, Will Deacon wrote: > On Fri, Jul 28, 2023 at 04:57:31PM +0530, Anshuman Khandual wrote: >> ACPI TRBE does not have a HID for identification which could create and add >> a platform device into the platform bus. Also without a platform device, it >> cannot be probed and bound to a platform driver. >> >> This creates a dummy platform device for TRBE after ascertaining that ACPI >> provides required interrupts uniformly across all cpus on the system. This >> device gets created inside drivers/perf/arm_pmu_acpi.c to accommodate TRBE >> being built as a module. >> >> Cc: Catalin Marinas >> Cc: Will Deacon >> Cc: Mark Rutland >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Anshuman Khandual > > --->8 > >> diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c >> index 90815ad762eb..dd3df6729808 100644 >> --- a/drivers/perf/arm_pmu_acpi.c >> +++ b/drivers/perf/arm_pmu_acpi.c >> @@ -139,6 +139,68 @@ static inline void arm_spe_acpi_register_device(void) >> } >> #endif /* CONFIG_ARM_SPE_PMU */ >> >> +#ifdef CONFIG_CORESIGHT_TRBE >> +static struct resource trbe_acpi_resources[] = { >> + { >> + /* irq */ >> + .flags = IORESOURCE_IRQ, >> + } >> +}; >> + >> +static struct platform_device trbe_acpi_dev = { >> + .name = ARMV8_TRBE_PDEV_NAME, >> + .id = -1, >> + .resource = trbe_acpi_resources, >> + .num_resources = ARRAY_SIZE(trbe_acpi_resources) >> +}; >> + >> +static void arm_trbe_acpi_register_device(void) >> +{ >> + int cpu, hetid, irq, ret; >> + bool first = true; >> + u16 gsi = 0; >> + >> + for_each_possible_cpu(cpu) { >> + struct acpi_madt_generic_interrupt *gicc; >> + >> + gicc = acpi_cpu_get_madt_gicc(cpu); >> + if (gicc->header.length < ACPI_MADT_GICC_TRBE) >> + return; >> + >> + if (first) { >> + gsi = gicc->trbe_interrupt; >> + if (!gsi) >> + return; >> + >> + hetid = find_acpi_cpu_topology_hetero_id(cpu); >> + first = false; >> + } else if ((gsi != gicc->trbe_interrupt) || >> + (hetid != find_acpi_cpu_topology_hetero_id(cpu))) { >> + pr_warn("ACPI: TRBE must be homogeneous\n"); >> + return; >> + } >> + } >> + >> + irq = acpi_register_gsi(NULL, gsi, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_HIGH); >> + if (irq < 0) { >> + pr_warn("ACPI: TRBE Unable to register interrupt: %d\n", gsi); >> + return; >> + } >> + trbe_acpi_resources[0].start = irq; >> + >> + ret = platform_device_register(&trbe_acpi_dev); >> + if (ret < 0) { >> + pr_warn("ACPI: TRBE: Unable to register device\n"); >> + acpi_unregister_gsi(gsi); >> + } >> +} >> +#else >> +static inline void arm_trbe_acpi_register_device(void) >> +{ >> + >> +} >> +#endif /* CONFIG_CORESIGHT_TRBE */ > > This looks like you ran s/spe/trbe/ over the SPE device registration > code :) Yeah, almost :) > > Please can you refactor things so we don't have all the duplication? I > suspect this won't be the last device which needs the same treatement. Should the refactoring just accommodate SPE, and TRBE or make it more generic to accommodate future devices as well. Something like the following enumeration. enum arm_platform_device { ARM_PLATFORM_DEVICE_SPE, ARM_PLATFORM_DEVICE_TRBE, ARM_PLATFORM_DEVICE_MAX, }; But that would require adding some helper functions to select these following elements based on the above enumeration via a common function - gicc->XXX_interrupt - ACPI_MADT_GICC_SPE/TRBE for header length comparison - static struct platform_device/resources (static objects in the file) Seems like will add much more code for a refactor. Did you have something else in mind for the refactor. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel