RE: [PATCH v2] ACPI: ARM Performance Monitoring Unit Table (APMT) initial support

Besar Wicaksono
2022-11-11T23:55:25Z

Hi Will,

> -----Original Message-----
> From: Will Deacon <will@kernel.org>
> Sent: Monday, November 7, 2022 1:11 PM
> To: Besar Wicaksono <bwicaksono@nvidia.com>
> Cc: rafael@kernel.org; lenb@kernel.org; catalin.marinas@arm.com;
> lorenzo.pieralisi@arm.com; guohanjun@huawei.com;
> sudeep.holla@arm.com; linux-tegra@vger.kernel.org; Thierry Reding
> <treding@nvidia.com>; Jonathan Hunter <jonathanh@nvidia.com>; Vikram
> Sethi <vsethi@nvidia.com>; linux-acpi@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v2] ACPI: ARM Performance Monitoring Unit Table
> (APMT) initial support
>
> External email: Use caution opening links or attachments
>
>
> On Wed, Sep 28, 2022 at 07:28:34PM -0500, Besar Wicaksono wrote:
> > diff --git a/drivers/acpi/arm64/apmt.c b/drivers/acpi/arm64/apmt.c
> > new file mode 100644
> > index 000000000000..f55167ca51e7
> > --- /dev/null
> > +++ b/drivers/acpi/arm64/apmt.c
> > @@ -0,0 +1,177 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * ARM APMT table support.
> > + * Design document number: ARM DEN0117.
> > + *
> > + * Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES.
> > + *
> > + */
> > +
> > +#define pr_fmt(fmt) "ACPI: APMT: " fmt
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/acpi_apmt.h>
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/platform_device.h>
> > +
> > +#define DEV_NAME "arm-cs-arch-pmu"
> > +
> > +/* There can be up to 3 resources: page 0 and 1 address, and interrupt. */
> > +#define DEV_MAX_RESOURCE_COUNT 3
> > +
> > +/* Root pointer to the mapped APMT table */
> > +static struct acpi_table_header *apmt_table;
> > +
> > +static int __init apmt_init_resources(struct resource *res,
> > + struct acpi_apmt_node *node)
> > +{
> > + int irq, trigger;
> > + int num_res = 0;
> > +
> > + res[num_res].start = node->base_address0;
> > + res[num_res].end = node->base_address0 + SZ_4K - 1;
> > + res[num_res].flags = IORESOURCE_MEM;
> > +
> > + num_res++;
> > +
> > + res[num_res].start = node->base_address1;
> > + res[num_res].end = node->base_address1 + SZ_4K - 1;
> > + res[num_res].flags = IORESOURCE_MEM;
> > +
> > + num_res++;
> > +
> > + if (node->ovflw_irq != 0) {
> > + trigger = (node->ovflw_irq_flags &
> ACPI_APMT_OVFLW_IRQ_FLAGS_MODE);
> > + trigger = (trigger ==
> ACPI_APMT_OVFLW_IRQ_FLAGS_MODE_LEVEL) ?
> > + ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
> > + irq = acpi_register_gsi(NULL, node->ovflw_irq, trigger,
> > + ACPI_ACTIVE_HIGH);
> > +
> > + if (irq <= 0) {
> > + pr_warn("APMT could not register gsi hwirq %d\n", irq);
> > + return num_res;
> > + }
> > +
> > + res[num_res].start = irq;
> > + res[num_res].end = irq;
> > + res[num_res].flags = IORESOURCE_IRQ;
> > +
> > + num_res++;
> > + }
> > +
> > + return num_res;
> > +}
> > +
> > +/**
> > + * apmt_add_platform_device() - Allocate a platform device for APMT
> node
> > + * @node: Pointer to device ACPI APMT node
> > + *
> > + * Returns: 0 on success, <0 failure
> > + */
> > +static int __init apmt_add_platform_device(struct acpi_apmt_node
> *node,
> > + struct fwnode_handle *fwnode)
>
> I queued this already, but my testing kicked up a nit that 'fwnode' is not
> described in the kerneldoc. Please can you send a follow-up fix for that,
> based on for-next/acpi? The indentation is also pretty weird with extra
> parameter.
>

Thanks for getting this patch. I have sent the follow-up fix with:
https://lore.kernel.org/linux-arm-kernel/20221111234323.16182-1-bwicaksono@nvidia.com/

Regards,
Besar

> Will