Re: [PATCH v3 1/4] acpi: Move common tables helper functions to common lib

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Thu, 01 Jun 2023 14:31:52 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Some of the routines in ACPI driver/acpi/tables.c can be shared with
> parsing CDAT. CDAT is a device-provided data structure that is formatted
> similar to a platform provided ACPI table. CDAT is used by CXL and can
> exist on platforms that do not use ACPI. Split out the common routine
> from ACPI to accommodate platforms that do not support ACPI and move that
> to /lib. The common routines can be built outside of ACPI if
> FIRMWARE_TABLES is selected.
> 
> Link: https://lore.kernel.org/linux-cxl/CAJZ5v0jipbtTNnsA0-o5ozOk8ZgWnOg34m34a9pPenTyRLj=6A@mail.gmail.com/
> Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Minor comment to fix inline. With that tidied up

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> diff --git a/include/linux/fw_table.h b/include/linux/fw_table.h
> new file mode 100644
> index 000000000000..ff8fa58d5818
> --- /dev/null
> +++ b/include/linux/fw_table.h
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + *  fw_tables.h - Parsing support for ACPI and ACPI-like tables provided by
> + *                platform or device firmware
> + *
> + *  Copyright (C) 2001 Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
> + *  Copyright (C) 2023 Intel Corp.
> + */
> +#ifndef _FW_TABLE_H_
> +#define _FW_TABLE_H_
> +
> +union acpi_subtable_headers;
> +
> +typedef int (*acpi_tbl_entry_handler)(union acpi_subtable_headers *header,
> +				      const unsigned long end);
> +
> +typedef int (*acpi_tbl_entry_handler_arg)(union acpi_subtable_headers *header,
> +					  void *arg, const unsigned long end);
> +
> +struct acpi_subtable_proc {
> +	int id;
> +	acpi_tbl_entry_handler handler;
> +	acpi_tbl_entry_handler_arg handler_arg;
> +	void *arg;
> +	int count;
> +};
> +
> +#include <linux/acpi.h>
> +#include <acpi/acpi.h>

Includes mid way down the files is not a common pattern and I can't see why
it's particularly useful to do so here.

+ linux/acpi.h includes acpi/acpi.h and I can't see that changing any time
soon...

> +
> +union acpi_subtable_headers {
> +	struct acpi_subtable_header common;
> +	struct acpi_hmat_structure hmat;
> +	struct acpi_prmt_module_header prmt;
> +	struct acpi_cedt_header cedt;
> +};
> +
> +int acpi_parse_entries_array(char *id, unsigned long table_size,
> +			     struct acpi_table_header *table_header,
> +			     struct acpi_subtable_proc *proc,
> +			     int proc_num, unsigned int max_entries);
> +
> +#endif