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

[Besar Wicaksono]

Hi Will,


> -----Original Message-----
> From: Will Deacon <will@xxxxxxxxxx>
> Sent: Monday, November 7, 2022 1:11 PM
> To: Besar Wicaksono <bwicaksono@xxxxxxxxxx>
> Cc: rafael@xxxxxxxxxx; lenb@xxxxxxxxxx; catalin.marinas@xxxxxxx;
> lorenzo.pieralisi@xxxxxxx; guohanjun@xxxxxxxxxx;
> sudeep.holla@xxxxxxx; linux-tegra@xxxxxxxxxxxxxxx; Thierry Reding
> <treding@xxxxxxxxxx>; Jonathan Hunter <jonathanh@xxxxxxxxxx>; Vikram
> Sethi <vsethi@xxxxxxxxxx>; linux-acpi@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> 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@xxxxxxxxxx/

Regards,
Besar

> Will