Re: [PATCH V8 5/6] ACPI: Support the probing on the devices which apply indirect-IO

[dann frazier]

On Thu, Mar 30, 2017 at 9:26 AM, zhichang.yuan
<yuanzhichang@xxxxxxxxxxxxx> wrote:
> On some platforms(such as Hip06/Hip07), the legacy ISA/LPC devices access I/O
> with some special host-local I/O ports known on x86. To access the I/O
> peripherals, an indirect-IO mechanism is introduced to mapped the host-local
> I/O to system logical/fake PIO similar the PCI MMIO on architectures where no
> separate I/O space exists. Just as PCI MMIO, the host I/O range should be
> registered before probing the downstream devices and set up the I/O mapping.
> But current ACPI bus probing doesn't support these indirect-IO hosts/devices.
>
> This patch introdueces a new ACPI handler for this device category. Through the
> handler attach callback, the indirect-IO hosts I/O registration is done and
> all peripherals' I/O resources are translated into logic/fake PIO before
> starting the enumeration.
>
> Signed-off-by: zhichang.yuan <yuanzhichang@xxxxxxxxxxxxx>
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@xxxxxxxxxx>
> ---
>  drivers/acpi/Makefile          |   1 +
>  drivers/acpi/acpi_indirectio.c | 344 +++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/internal.h        |   5 +
>  drivers/acpi/scan.c            |   1 +
>  4 files changed, 351 insertions(+)
>  create mode 100644 drivers/acpi/acpi_indirectio.c
>
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index a391bbc..10e5f2b 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -57,6 +57,7 @@ acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
>  acpi-y                         += acpi_lpat.o
>  acpi-$(CONFIG_ACPI_GENERIC_GSI) += irq.o
>  acpi-$(CONFIG_ACPI_WATCHDOG)   += acpi_watchdog.o
> +acpi-$(CONFIG_INDIRECT_PIO)    += acpi_indirectio.o
>
>  # These are (potentially) separate modules
>
> diff --git a/drivers/acpi/acpi_indirectio.c b/drivers/acpi/acpi_indirectio.c
> new file mode 100644
> index 0000000..c8c80b5
> --- /dev/null
> +++ b/drivers/acpi/acpi_indirectio.c
> @@ -0,0 +1,344 @@
> +/*
> + * ACPI support for indirect-IO bus.
> + *
> + * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved.
> + * Author: Zhichang Yuan <yuanzhichang@xxxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/platform_device.h>
> +#include <linux/logic_pio.h>
> +
> +#include "internal.h"
> +
> +ACPI_MODULE_NAME("indirect IO");
> +
> +#define INDIRECT_IO_INFO(desc) ((unsigned long)&desc)
> +
> +struct lpc_private_data {
> +       resource_size_t io_size;
> +       resource_size_t io_start;
> +};
> +
> +struct indirectio_device_desc {
> +       void *pdata; /* device relevant info data */
> +       int (*pre_setup)(struct acpi_device *adev, void *pdata);
> +};
> +
> +static struct lpc_private_data lpc_data = {
> +       .io_size = LPC_BUS_IO_SIZE,
> +       .io_start = LPC_MIN_BUS_RANGE,
> +};
> +
> +static inline bool acpi_logicio_supported_resource(struct acpi_resource *res)
> +{
> +       switch (res->type) {
> +       case ACPI_RESOURCE_TYPE_ADDRESS32:
> +       case ACPI_RESOURCE_TYPE_ADDRESS64:
> +               return true;
> +       }
> +       return false;
> +}
> +
> +static acpi_status acpi_count_logiciores(struct acpi_resource *res,
> +                                          void *data)
> +{
> +       int *res_cnt = data;
> +
> +       if (acpi_logicio_supported_resource(res) &&
> +               !acpi_dev_filter_resource_type(res, IORESOURCE_IO))
> +               (*res_cnt)++;
> +
> +       return AE_OK;
> +}
> +
> +static acpi_status acpi_read_one_logiciores(struct acpi_resource *res,
> +               void *data)
> +{
> +       struct acpi_resource **resource = data;
> +
> +       if (acpi_logicio_supported_resource(res) &&
> +               !acpi_dev_filter_resource_type(res, IORESOURCE_IO)) {
> +               memcpy((*resource), res, sizeof(struct acpi_resource));
> +               (*resource)->length = sizeof(struct acpi_resource);
> +               (*resource)->type = res->type;
> +               (*resource)++;
> +       }
> +
> +       return AE_OK;
> +}
> +
> +static acpi_status
> +acpi_build_logiciores_template(struct acpi_device *adev,
> +                       struct acpi_buffer *buffer)
> +{
> +       acpi_handle handle = adev->handle;
> +       struct acpi_resource *resource;
> +       acpi_status status;
> +       int res_cnt = 0;
> +
> +       status = acpi_walk_resources(handle, METHOD_NAME__CRS,
> +                                    acpi_count_logiciores, &res_cnt);
> +       if (ACPI_FAILURE(status) || !res_cnt) {
> +               dev_err(&adev->dev, "can't evaluate _CRS: %d\n", status);
> +               return -EINVAL;
> +       }
> +
> +       buffer->length = sizeof(struct acpi_resource) * (res_cnt + 1) + 1;
> +       buffer->pointer = kzalloc(buffer->length - 1, GFP_KERNEL);

(Seth Forshee noticed this issue, just passing it on)

Should this just allocate the full buffer->length? That would keep the
length attribute accurate (possibly avoiding an off-by-1 error later).
It's not clear what the trailing byte is needed for, but other drivers
allocate it as well (drivers/acpi/pci_link.c and
drivers/platform/x86/sony-laptop.c).

 -dann