| Message ID | 1516725385-24535-8-git-send-email-john.garry@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On 23/01/2018 16:36, John Garry 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. As their I/O space > are not memory mapped like PCI/PCIE MMIO host bridges, this patch is meant > to support a new class of I/O host controllers where the local IO ports of > the children devices are translated into the Indirect I/O address space. > > Through the handler attach callback, all the I/O translations are done > before starting the enumeration on children devices and the translated > addresses are replaced in the children resources. > Hi Lorenzo, Rafael, Mika, I would really appreciate if you could have a look at this patch. The current solution here is to scan the ACPI devices under these indirectIO hosts and create MFDs for the children. I think another solution - which you may prefer - is to avoid adding this scan handler (and all this other scan code) and add a check like acpi_is_serial_bus_slave() [which checks the device parent versus a list of known indirectIO hosts] to not enumerate these children, and do it from the LLDD host probe instead (https://lkml.org/lkml/2017/6/16/250) John > Signed-off-by: John Garry <john.garry@huawei.com> > Signed-off-by: Zhichang Yuan <yuanzhichang@hisilicon.com> > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com> > --- > drivers/acpi/arm64/Makefile | 1 + > drivers/acpi/arm64/acpi_indirectio.c | 273 +++++++++++++++++++++++++++++++++++ > drivers/acpi/internal.h | 5 + > drivers/acpi/scan.c | 1 + > 4 files changed, 280 insertions(+) > create mode 100644 drivers/acpi/arm64/acpi_indirectio.c > > diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile > index 1017def..f4a7f46 100644 > --- a/drivers/acpi/arm64/Makefile > +++ b/drivers/acpi/arm64/Makefile > @@ -1,2 +1,3 @@ > obj-$(CONFIG_ACPI_IORT) += iort.o > obj-$(CONFIG_ACPI_GTDT) += gtdt.o > +obj-$(CONFIG_INDIRECT_PIO) += acpi_indirectio.o > diff --git a/drivers/acpi/arm64/acpi_indirectio.c b/drivers/acpi/arm64/acpi_indirectio.c > new file mode 100644 > index 0000000..2649f57 > --- /dev/null > +++ b/drivers/acpi/arm64/acpi_indirectio.c > @@ -0,0 +1,273 @@ > +/* > + * ACPI support for indirect-IO bus. > + * > + * Copyright (C) 2017 HiSilicon Limited, All Rights Reserved. > + * Author: Gabriele Paoloni <gabriele.paoloni@huawei.com> > + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com> > + * Author: John Garry <john.garry@huawei.com> > + * > + * 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/logic_pio.h> > +#include <linux/mfd/core.h> > +#include <linux/platform_device.h> > + > +ACPI_MODULE_NAME("indirect IO"); > + > +#define ACPI_INDIRECTIO_NAME_LENGTH 255 > + > +#define INDIRECT_IO_INFO(desc) ((unsigned long)&desc) > + > +struct acpi_indirectio_mfd_cell { > + struct mfd_cell_acpi_match acpi_match; > + char name[ACPI_INDIRECTIO_NAME_LENGTH]; > + char pnpid[ACPI_INDIRECTIO_NAME_LENGTH]; > +}; > + > +struct acpi_indirectio_host_data { > + resource_size_t io_size; > + resource_size_t io_start; > +}; > + > +struct acpi_indirectio_device_desc { > + struct acpi_indirectio_host_data pdata; /* device relevant info data */ > + int (*pre_setup)(struct acpi_device *adev, > + struct acpi_indirectio_host_data *pdata); > +}; > + > +static int acpi_translate_logicio_res(struct acpi_device *adev, > + struct acpi_device *host, struct resource *resource) > +{ > + unsigned long sys_port; > + struct device *dev = &adev->dev; > + resource_size_t length = resource->end - resource->start; > + > + sys_port = logic_pio_trans_hwaddr(&host->fwnode, resource->start, > + length); > + > + if (sys_port == -1) { > + dev_err(dev, "translate bus-addr(0x%llx) fail!\n", > + resource->start); > + return -EFAULT; > + } > + > + resource->start = sys_port; > + resource->end = sys_port + length; > + > + return 0; > +} > + > +/* > + * update/set the current I/O resource of the designated device node. > + * after this calling, the enumeration can be started as the I/O resource > + * had been translated to logicial I/O from bus-local I/O. > + * > + * @child: the device node to be updated the I/O resource; > + * @hostdev: the device node where 'adev' is attached, which can be not > + * the parent of 'adev'; > + * @res: double pointer to be set to the address of the updated resources > + * @num_res: address of the variable to contain the number of updated resources > + * > + * return 0 when successful, negative is for failure. > + */ > +int acpi_indirectio_set_logicio_res(struct device *child, > + struct device *hostdev, > + const struct resource **res, > + int *num_res) > +{ > + struct acpi_device *adev; > + struct acpi_device *host; > + struct resource_entry *rentry; > + LIST_HEAD(resource_list); > + struct resource *resources = NULL; > + int count; > + int i; > + int ret = -EIO; > + > + if (!child || !hostdev) > + return -EINVAL; > + > + host = to_acpi_device(hostdev); > + adev = to_acpi_device(child); > + > + /* check the device state */ > + if (!adev->status.present) { > + dev_info(child, "ACPI: device is not present!\n"); > + return 0; > + } > + /* whether the child had been enumerated? */ > + if (acpi_device_enumerated(adev)) { > + dev_info(child, "ACPI: had been enumerated!\n"); > + return 0; > + } > + > + count = acpi_dev_get_resources(adev, &resource_list, NULL, NULL); > + if (count <= 0) { > + dev_err(&adev->dev, "failed to get ACPI resources\n"); > + return count ? count : -EIO; > + } > + > + resources = kcalloc(count, sizeof(struct resource), GFP_KERNEL); > + if (!resources) { > + acpi_dev_free_resource_list(&resource_list); > + return -ENOMEM; > + } > + count = 0; > + list_for_each_entry(rentry, &resource_list, node) > + resources[count++] = *rentry->res; > + > + acpi_dev_free_resource_list(&resource_list); > + > + /* translate the I/O resources */ > + for (i = 0; i < count; i++) { > + if (resources[i].flags & IORESOURCE_IO) { > + ret = acpi_translate_logicio_res(adev, host, > + &resources[i]); > + if (ret) { > + kfree(resources); > + dev_err(child, > + "Translate I/O range failed (%d)!\n", > + ret); > + return ret; > + } > + } > + } > + *res = resources; > + *num_res = count; > + > + return ret; > +} > + > +int > +acpi_indirectio_pre_setup(struct acpi_device *adev, > + struct acpi_indirectio_host_data *pdata) > +{ > + struct platform_device *pdev; > + struct mfd_cell *mfd_cells; > + struct logic_pio_hwaddr *range; > + struct acpi_device *child; > + struct acpi_indirectio_mfd_cell *acpi_indirectio_mfd_cells; > + int size, ret, count = 0, cell_num = 0; > + > + range = kzalloc(sizeof(*range), GFP_KERNEL); > + if (!range) > + return -ENOMEM; > + range->fwnode = &adev->fwnode; > + range->flags = PIO_INDIRECT; > + range->size = pdata->io_size; > + range->hw_start = pdata->io_start; > + > + ret = logic_pio_register_range(range); > + if (ret) > + goto free_range; > + > + list_for_each_entry(child, &adev->children, node) > + cell_num++; > + > + /* allocate the mfd cells */ > + size = sizeof(*mfd_cells) + sizeof(*acpi_indirectio_mfd_cells); > + mfd_cells = kcalloc(cell_num, size, GFP_KERNEL); > + if (!mfd_cells) { > + ret = -ENOMEM; > + goto free_range; > + } > + > + acpi_indirectio_mfd_cells = (struct acpi_indirectio_mfd_cell *) > + &mfd_cells[cell_num]; > + /* Only consider the children of the host */ > + list_for_each_entry(child, &adev->children, node) { > + struct mfd_cell *mfd_cell = &mfd_cells[count]; > + struct acpi_indirectio_mfd_cell *acpi_indirectio_mfd_cell = > + &acpi_indirectio_mfd_cells[count]; > + const struct mfd_cell_acpi_match *acpi_match = > + &acpi_indirectio_mfd_cell->acpi_match; > + char *name = &acpi_indirectio_mfd_cell[count].name[0]; > + char *pnpid = &acpi_indirectio_mfd_cell[count].pnpid[0]; > + struct mfd_cell_acpi_match match = { > + .pnpid = pnpid, > + }; > + > + snprintf(name, ACPI_INDIRECTIO_NAME_LENGTH, "indirect-io-%s", > + acpi_device_hid(child)); > + snprintf(pnpid, ACPI_INDIRECTIO_NAME_LENGTH, "%s", > + acpi_device_hid(child)); > + > + memcpy((void *)acpi_match, (void *)&match, sizeof(*acpi_match)); > + mfd_cell->name = name; > + mfd_cell->acpi_match = acpi_match; > + > + ret = > + acpi_indirectio_set_logicio_res(&child->dev, > + &adev->dev, > + &mfd_cell->resources, > + &mfd_cell->num_resources); > + if (ret) { > + dev_err(&child->dev, "set resource failed (%d)\n", ret); > + goto free_mfd_res; > + } > + count++; > + } > + > + pdev = acpi_create_platform_device(adev, NULL); > + if (IS_ERR_OR_NULL(pdev)) { > + dev_err(&adev->dev, "Create platform device for host failed!\n"); > + ret = PTR_ERR(pdev); > + goto free_mfd_res; > + } > + acpi_device_set_enumerated(adev); > + > + ret = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, > + mfd_cells, cell_num, NULL, 0, NULL); > + if (ret) { > + dev_err(&pdev->dev, "failed to add mfd cells (%d)\n", ret); > + goto free_mfd_res; > + } > + > + return ret; > + > +free_mfd_res: > + while (cell_num--) > + kfree(mfd_cells[cell_num].resources); > + kfree(mfd_cells); > +free_range: > + kfree(range); > + > + return ret; > +} > + > +/* All the host devices which apply indirect-IO can be listed here. */ > +static const struct acpi_device_id acpi_indirect_host_id[] = { > + {""}, > +}; > + > +static int acpi_indirectio_attach(struct acpi_device *adev, > + const struct acpi_device_id *id) > +{ > + struct acpi_indirectio_device_desc *hostdata; > + int ret; > + > + hostdata = (struct acpi_indirectio_device_desc *)id->driver_data; > + if (!hostdata || !hostdata->pre_setup) > + return -EINVAL; > + > + ret = hostdata->pre_setup(adev, &hostdata->pdata); > + > + if (ret < 0) > + return ret; > + > + return 1; > +} > + > +static struct acpi_scan_handler acpi_indirect_handler = { > + .ids = acpi_indirect_host_id, > + .attach = acpi_indirectio_attach, > +}; > + > +void __init acpi_indirectio_scan_init(void) > +{ > + acpi_scan_add_handler(&acpi_indirect_handler); > +} > diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h > index 1d0a501..d6b1a95 100644 > --- a/drivers/acpi/internal.h > +++ b/drivers/acpi/internal.h > @@ -31,6 +31,11 @@ > void acpi_platform_init(void); > void acpi_pnp_init(void); > void acpi_int340x_thermal_init(void); > +#ifdef CONFIG_INDIRECT_PIO > +void acpi_indirectio_scan_init(void); > +#else > +static inline void acpi_indirectio_scan_init(void) {} > +#endif > #ifdef CONFIG_ARM_AMBA > void acpi_amba_init(void); > #else > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index b0fe527..8ea6f69 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -2141,6 +2141,7 @@ int __init acpi_scan_init(void) > acpi_amba_init(); > acpi_watchdog_init(); > acpi_init_lpit(); > + acpi_indirectio_scan_init(); > > acpi_scan_add_handler(&generic_device_handler); > >
On Tue, Jan 23, 2018 at 5:36 PM, John Garry <john.garry@huawei.com> 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. As their I/O space > are not memory mapped like PCI/PCIE MMIO host bridges, this patch is meant > to support a new class of I/O host controllers where the local IO ports of > the children devices are translated into the Indirect I/O address space. > > Through the handler attach callback, all the I/O translations are done > before starting the enumeration on children devices and the translated > addresses are replaced in the children resources. The changelog is somewhat dry for a patch adding over 300 lines of new code and a new file for that matter. > Signed-off-by: John Garry <john.garry@huawei.com> > Signed-off-by: Zhichang Yuan <yuanzhichang@hisilicon.com> > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com> > --- > drivers/acpi/arm64/Makefile | 1 + > drivers/acpi/arm64/acpi_indirectio.c | 273 +++++++++++++++++++++++++++++++++++ > drivers/acpi/internal.h | 5 + > drivers/acpi/scan.c | 1 + > 4 files changed, 280 insertions(+) > create mode 100644 drivers/acpi/arm64/acpi_indirectio.c > > diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile > index 1017def..f4a7f46 100644 > --- a/drivers/acpi/arm64/Makefile > +++ b/drivers/acpi/arm64/Makefile > @@ -1,2 +1,3 @@ > obj-$(CONFIG_ACPI_IORT) += iort.o > obj-$(CONFIG_ACPI_GTDT) += gtdt.o > +obj-$(CONFIG_INDIRECT_PIO) += acpi_indirectio.o > diff --git a/drivers/acpi/arm64/acpi_indirectio.c b/drivers/acpi/arm64/acpi_indirectio.c > new file mode 100644 > index 0000000..2649f57 > --- /dev/null > +++ b/drivers/acpi/arm64/acpi_indirectio.c > @@ -0,0 +1,273 @@ SPDX license identifier here? > +/* > + * ACPI support for indirect-IO bus. > + * > + * Copyright (C) 2017 HiSilicon Limited, All Rights Reserved. > + * Author: Gabriele Paoloni <gabriele.paoloni@huawei.com> > + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com> > + * Author: John Garry <john.garry@huawei.com> > + * > + * 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. And then you can skip the above. Also I would like to see some description of what's there in this file to appear here. > + */ > + > +#include <linux/acpi.h> > +#include <linux/logic_pio.h> > +#include <linux/mfd/core.h> > +#include <linux/platform_device.h> > + > +ACPI_MODULE_NAME("indirect IO"); > + > +#define ACPI_INDIRECTIO_NAME_LENGTH 255 > + > +#define INDIRECT_IO_INFO(desc) ((unsigned long)&desc) > + > +struct acpi_indirectio_mfd_cell { > + struct mfd_cell_acpi_match acpi_match; > + char name[ACPI_INDIRECTIO_NAME_LENGTH]; > + char pnpid[ACPI_INDIRECTIO_NAME_LENGTH]; > +}; > + > +struct acpi_indirectio_host_data { > + resource_size_t io_size; > + resource_size_t io_start; > +}; > + > +struct acpi_indirectio_device_desc { Why don't you use a consistent naming convention and call this acpi_indirect_io_device_desc (and analogously everywhere above and below)? > + struct acpi_indirectio_host_data pdata; /* device relevant info data */ > + int (*pre_setup)(struct acpi_device *adev, > + struct acpi_indirectio_host_data *pdata); > +}; > + > +static int acpi_translate_logicio_res(struct acpi_device *adev, > + struct acpi_device *host, struct resource *resource) > +{ > + unsigned long sys_port; > + struct device *dev = &adev->dev; > + resource_size_t length = resource->end - resource->start; > + > + sys_port = logic_pio_trans_hwaddr(&host->fwnode, resource->start, > + length); > + > + if (sys_port == -1) { Would if (sysp_port < 0) not work here? > + dev_err(dev, "translate bus-addr(0x%llx) fail!\n", > + resource->start); That's not a very informative message. What are users expected to do in response to seeing it? > + return -EFAULT; > + } > + > + resource->start = sys_port; > + resource->end = sys_port + length; > + > + return 0; > +} > + > +/* > + * update/set the current I/O resource of the designated device node. > + * after this calling, the enumeration can be started as the I/O resource > + * had been translated to logicial I/O from bus-local I/O. > + * > + * @child: the device node to be updated the I/O resource; > + * @hostdev: the device node where 'adev' is attached, which can be not > + * the parent of 'adev'; > + * @res: double pointer to be set to the address of the updated resources > + * @num_res: address of the variable to contain the number of updated resources > + * > + * return 0 when successful, negative is for failure. > + */ The above should be a proper kerneldoc comment. > +int acpi_indirectio_set_logicio_res(struct device *child, > + struct device *hostdev, > + const struct resource **res, > + int *num_res) > +{ > + struct acpi_device *adev; > + struct acpi_device *host; > + struct resource_entry *rentry; > + LIST_HEAD(resource_list); > + struct resource *resources = NULL; > + int count; > + int i; > + int ret = -EIO; > + > + if (!child || !hostdev) > + return -EINVAL; > + > + host = to_acpi_device(hostdev); > + adev = to_acpi_device(child); > + > + /* check the device state */ > + if (!adev->status.present) { > + dev_info(child, "ACPI: device is not present!\n"); > + return 0; > + } > + /* whether the child had been enumerated? */ > + if (acpi_device_enumerated(adev)) { > + dev_info(child, "ACPI: had been enumerated!\n"); > + return 0; > + } > + > + count = acpi_dev_get_resources(adev, &resource_list, NULL, NULL); > + if (count <= 0) { > + dev_err(&adev->dev, "failed to get ACPI resources\n"); > + return count ? count : -EIO; > + } > + > + resources = kcalloc(count, sizeof(struct resource), GFP_KERNEL); > + if (!resources) { > + acpi_dev_free_resource_list(&resource_list); > + return -ENOMEM; > + } > + count = 0; > + list_for_each_entry(rentry, &resource_list, node) > + resources[count++] = *rentry->res; > + > + acpi_dev_free_resource_list(&resource_list); > + > + /* translate the I/O resources */ > + for (i = 0; i < count; i++) { > + if (resources[i].flags & IORESOURCE_IO) { > + ret = acpi_translate_logicio_res(adev, host, > + &resources[i]); You don't need to break this line as far as I'm concerned. > + if (ret) { > + kfree(resources); > + dev_err(child, > + "Translate I/O range failed (%d)!\n", > + ret); And this too. > + return ret; > + } > + } > + } > + *res = resources; > + *num_res = count; > + > + return ret; > +} > + > +int Don't break the line here, please. > +acpi_indirectio_pre_setup(struct acpi_device *adev, > + struct acpi_indirectio_host_data *pdata) As a non-static function, this requires a kerneldoc comment. In particular, the comment should describe who and when is expected to call this function (and which also applies to the comment preceding the previous function). Apparently, they both need to be called before the initial namespace scan for the acpi_indirectio_attach() below to work, right? > +{ > + struct platform_device *pdev; > + struct mfd_cell *mfd_cells; > + struct logic_pio_hwaddr *range; > + struct acpi_device *child; > + struct acpi_indirectio_mfd_cell *acpi_indirectio_mfd_cells; > + int size, ret, count = 0, cell_num = 0; > + > + range = kzalloc(sizeof(*range), GFP_KERNEL); > + if (!range) > + return -ENOMEM; > + range->fwnode = &adev->fwnode; > + range->flags = PIO_INDIRECT; > + range->size = pdata->io_size; > + range->hw_start = pdata->io_start; > + > + ret = logic_pio_register_range(range); > + if (ret) > + goto free_range; > + > + list_for_each_entry(child, &adev->children, node) > + cell_num++; > + > + /* allocate the mfd cells */ > + size = sizeof(*mfd_cells) + sizeof(*acpi_indirectio_mfd_cells); > + mfd_cells = kcalloc(cell_num, size, GFP_KERNEL); > + if (!mfd_cells) { > + ret = -ENOMEM; > + goto free_range; > + } > + > + acpi_indirectio_mfd_cells = (struct acpi_indirectio_mfd_cell *) > + &mfd_cells[cell_num]; > + /* Only consider the children of the host */ > + list_for_each_entry(child, &adev->children, node) { > + struct mfd_cell *mfd_cell = &mfd_cells[count]; > + struct acpi_indirectio_mfd_cell *acpi_indirectio_mfd_cell = > + &acpi_indirectio_mfd_cells[count]; > + const struct mfd_cell_acpi_match *acpi_match = > + &acpi_indirectio_mfd_cell->acpi_match; > + char *name = &acpi_indirectio_mfd_cell[count].name[0]; > + char *pnpid = &acpi_indirectio_mfd_cell[count].pnpid[0]; > + struct mfd_cell_acpi_match match = { > + .pnpid = pnpid, > + }; > + > + snprintf(name, ACPI_INDIRECTIO_NAME_LENGTH, "indirect-io-%s", > + acpi_device_hid(child)); > + snprintf(pnpid, ACPI_INDIRECTIO_NAME_LENGTH, "%s", > + acpi_device_hid(child)); > + > + memcpy((void *)acpi_match, (void *)&match, sizeof(*acpi_match)); > + mfd_cell->name = name; > + mfd_cell->acpi_match = acpi_match; > + > + ret = > + acpi_indirectio_set_logicio_res(&child->dev, > + &adev->dev, > + &mfd_cell->resources, > + &mfd_cell->num_resources); > + if (ret) { > + dev_err(&child->dev, "set resource failed (%d)\n", ret); > + goto free_mfd_res; > + } > + count++; > + } > + > + pdev = acpi_create_platform_device(adev, NULL); > + if (IS_ERR_OR_NULL(pdev)) { > + dev_err(&adev->dev, "Create platform device for host failed!\n"); > + ret = PTR_ERR(pdev); > + goto free_mfd_res; > + } > + acpi_device_set_enumerated(adev); > + > + ret = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, > + mfd_cells, cell_num, NULL, 0, NULL); > + if (ret) { > + dev_err(&pdev->dev, "failed to add mfd cells (%d)\n", ret); > + goto free_mfd_res; > + } > + > + return ret; > + > +free_mfd_res: > + while (cell_num--) > + kfree(mfd_cells[cell_num].resources); > + kfree(mfd_cells); > +free_range: > + kfree(range); > + > + return ret; > +} > + > +/* All the host devices which apply indirect-IO can be listed here. */ > +static const struct acpi_device_id acpi_indirect_host_id[] = { > + {""}, > +}; > + > +static int acpi_indirectio_attach(struct acpi_device *adev, > + const struct acpi_device_id *id) > +{ > + struct acpi_indirectio_device_desc *hostdata; > + int ret; > + > + hostdata = (struct acpi_indirectio_device_desc *)id->driver_data; > + if (!hostdata || !hostdata->pre_setup) > + return -EINVAL; > + > + ret = hostdata->pre_setup(adev, &hostdata->pdata); > + > + if (ret < 0) > + return ret; > + > + return 1; > +} > + > +static struct acpi_scan_handler acpi_indirect_handler = { acpi_indirect_io_handler? > + .ids = acpi_indirect_host_id, > + .attach = acpi_indirectio_attach, > +}; > + > +void __init acpi_indirectio_scan_init(void) > +{ > + acpi_scan_add_handler(&acpi_indirect_handler); > +} > diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h > index 1d0a501..d6b1a95 100644 > --- a/drivers/acpi/internal.h > +++ b/drivers/acpi/internal.h > @@ -31,6 +31,11 @@ > void acpi_platform_init(void); > void acpi_pnp_init(void); > void acpi_int340x_thermal_init(void); > +#ifdef CONFIG_INDIRECT_PIO > +void acpi_indirectio_scan_init(void); > +#else > +static inline void acpi_indirectio_scan_init(void) {} > +#endif > #ifdef CONFIG_ARM_AMBA > void acpi_amba_init(void); > #else > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index b0fe527..8ea6f69 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -2141,6 +2141,7 @@ int __init acpi_scan_init(void) > acpi_amba_init(); > acpi_watchdog_init(); > acpi_init_lpit(); > + acpi_indirectio_scan_init(); > > acpi_scan_add_handler(&generic_device_handler); > > -- > 1.9.1 >
On 04/02/2018 07:45, Rafael J. Wysocki wrote: > On Tue, Jan 23, 2018 at 5:36 PM, John Garry <john.garry@huawei.com> 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. As their I/O space >> are not memory mapped like PCI/PCIE MMIO host bridges, this patch is meant >> to support a new class of I/O host controllers where the local IO ports of >> the children devices are translated into the Indirect I/O address space. >> >> Through the handler attach callback, all the I/O translations are done >> before starting the enumeration on children devices and the translated >> addresses are replaced in the children resources. > Hi Rafael, Thanks for checking. > The changelog is somewhat dry for a patch adding over 300 lines of new > code and a new file for that matter. Will add more. Indeed MFD is not even mentioned. > >> Signed-off-by: John Garry <john.garry@huawei.com> >> Signed-off-by: Zhichang Yuan <yuanzhichang@hisilicon.com> >> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com> >> --- >> drivers/acpi/arm64/Makefile | 1 + >> drivers/acpi/arm64/acpi_indirectio.c | 273 +++++++++++++++++++++++++++++++++++ >> drivers/acpi/internal.h | 5 + >> drivers/acpi/scan.c | 1 + >> 4 files changed, 280 insertions(+) >> create mode 100644 drivers/acpi/arm64/acpi_indirectio.c >> >> diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile >> index 1017def..f4a7f46 100644 >> --- a/drivers/acpi/arm64/Makefile >> +++ b/drivers/acpi/arm64/Makefile >> @@ -1,2 +1,3 @@ >> obj-$(CONFIG_ACPI_IORT) += iort.o >> obj-$(CONFIG_ACPI_GTDT) += gtdt.o >> +obj-$(CONFIG_INDIRECT_PIO) += acpi_indirectio.o >> diff --git a/drivers/acpi/arm64/acpi_indirectio.c b/drivers/acpi/arm64/acpi_indirectio.c >> new file mode 100644 >> index 0000000..2649f57 >> --- /dev/null >> +++ b/drivers/acpi/arm64/acpi_indirectio.c >> @@ -0,0 +1,273 @@ > > SPDX license identifier here? Will fix. I also need to fixup the other new files in the patchset. > >> +/* >> + * ACPI support for indirect-IO bus. >> + * >> + * Copyright (C) 2017 HiSilicon Limited, All Rights Reserved. >> + * Author: Gabriele Paoloni <gabriele.paoloni@huawei.com> >> + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com> >> + * Author: John Garry <john.garry@huawei.com> >> + * >> + * 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. > > And then you can skip the above. > > Also I would like to see some description of what's there in this file > to appear here. Sure. > >> + */ >> + >> +#include <linux/acpi.h> >> +#include <linux/logic_pio.h> >> +#include <linux/mfd/core.h> >> +#include <linux/platform_device.h> >> + >> +ACPI_MODULE_NAME("indirect IO"); >> + >> +#define ACPI_INDIRECTIO_NAME_LENGTH 255 >> + >> +#define INDIRECT_IO_INFO(desc) ((unsigned long)&desc) >> + >> +struct acpi_indirectio_mfd_cell { >> + struct mfd_cell_acpi_match acpi_match; >> + char name[ACPI_INDIRECTIO_NAME_LENGTH]; >> + char pnpid[ACPI_INDIRECTIO_NAME_LENGTH]; >> +}; >> + >> +struct acpi_indirectio_host_data { >> + resource_size_t io_size; >> + resource_size_t io_start; >> +}; >> + >> +struct acpi_indirectio_device_desc { > > Why don't you use a consistent naming convention and call this > acpi_indirect_io_device_desc (and analogously everywhere above and > below)? Right, I can correct the file to have consistent symbol prefixes. > >> + struct acpi_indirectio_host_data pdata; /* device relevant info data */ >> + int (*pre_setup)(struct acpi_device *adev, >> + struct acpi_indirectio_host_data *pdata); >> +}; >> + >> +static int acpi_translate_logicio_res(struct acpi_device *adev, >> + struct acpi_device *host, struct resource *resource) >> +{ >> + unsigned long sys_port; >> + struct device *dev = &adev->dev; >> + resource_size_t length = resource->end - resource->start; >> + >> + sys_port = logic_pio_trans_hwaddr(&host->fwnode, resource->start, >> + length); >> + >> + if (sys_port == -1) { > > Would if (sysp_port < 0) not work here? I don't think so because sys_port is an unsigned value. I should change this if statement to use -1UL. > >> + dev_err(dev, "translate bus-addr(0x%llx) fail!\n", >> + resource->start); > > That's not a very informative message. What are users expected to do > in response to seeing it? Right, the user generally would not be able to decipher this, so I should add some more info on what the fallout will be. > >> + return -EFAULT; >> + } >> + >> + resource->start = sys_port; >> + resource->end = sys_port + length; >> + >> + return 0; >> +} >> + >> +/* >> + * update/set the current I/O resource of the designated device node. >> + * after this calling, the enumeration can be started as the I/O resource >> + * had been translated to logicial I/O from bus-local I/O. >> + * >> + * @child: the device node to be updated the I/O resource; >> + * @hostdev: the device node where 'adev' is attached, which can be not >> + * the parent of 'adev'; >> + * @res: double pointer to be set to the address of the updated resources >> + * @num_res: address of the variable to contain the number of updated resources >> + * >> + * return 0 when successful, negative is for failure. >> + */ > > The above should be a proper kerneldoc comment. > >> +int acpi_indirectio_set_logicio_res(struct device *child, This should actually be static, so I think we can avoid the kerneldoc comment? >> + struct device *hostdev, >> + const struct resource **res, >> + int *num_res) >> +{ >> + struct acpi_device *adev; >> + struct acpi_device *host; >> + struct resource_entry *rentry; >> + LIST_HEAD(resource_list); >> + struct resource *resources = NULL; >> + int count; >> + int i; >> + int ret = -EIO; >> + >> + if (!child || !hostdev) >> + return -EINVAL; >> + >> + host = to_acpi_device(hostdev); >> + adev = to_acpi_device(child); >> + >> + /* check the device state */ >> + if (!adev->status.present) { >> + dev_info(child, "ACPI: device is not present!\n"); >> + return 0; >> + } >> + /* whether the child had been enumerated? */ >> + if (acpi_device_enumerated(adev)) { >> + dev_info(child, "ACPI: had been enumerated!\n"); >> + return 0; >> + } >> + >> + count = acpi_dev_get_resources(adev, &resource_list, NULL, NULL); >> + if (count <= 0) { >> + dev_err(&adev->dev, "failed to get ACPI resources\n"); >> + return count ? count : -EIO; >> + } >> + >> + resources = kcalloc(count, sizeof(struct resource), GFP_KERNEL); >> + if (!resources) { >> + acpi_dev_free_resource_list(&resource_list); >> + return -ENOMEM; >> + } >> + count = 0; >> + list_for_each_entry(rentry, &resource_list, node) >> + resources[count++] = *rentry->res; >> + >> + acpi_dev_free_resource_list(&resource_list); >> + >> + /* translate the I/O resources */ >> + for (i = 0; i < count; i++) { >> + if (resources[i].flags & IORESOURCE_IO) { >> + ret = acpi_translate_logicio_res(adev, host, >> + &resources[i]); > > You don't need to break this line as far as I'm concerned. This is just to keep checkpatch happy. I could move the complete function call to a single line. And also shortening some symbols will help. > >> + if (ret) { >> + kfree(resources); >> + dev_err(child, >> + "Translate I/O range failed (%d)!\n", >> + ret); > > And this too. Again, just appeasing checkpatch > >> + return ret; >> + } >> + } >> + } >> + *res = resources; >> + *num_res = count; >> + >> + return ret; >> +} >> + >> +int > > Don't break the line here, please. Alright, will do. I thought that this was an accepted style when the arguments list is quite long. > >> +acpi_indirectio_pre_setup(struct acpi_device *adev, >> + struct acpi_indirectio_host_data *pdata) > > As a non-static function, this requires a kerneldoc comment. In fact this should be static. I should have fixed these already. > > In particular, the comment should describe who and when is expected to > call this function (and which also applies to the comment preceding > the previous function). > > Apparently, they both need to be called before the initial namespace > scan for the acpi_indirectio_attach() below to work, right? > Right, this needs to be prior to device enumeration in the ACPI namespace. >> +{ >> + struct platform_device *pdev; >> + struct mfd_cell *mfd_cells; >> + struct logic_pio_hwaddr *range; >> + struct acpi_device *child; >> + struct acpi_indirectio_mfd_cell *acpi_indirectio_mfd_cells; >> + int size, ret, count = 0, cell_num = 0; >> + >> + range = kzalloc(sizeof(*range), GFP_KERNEL); >> + if (!range) >> + return -ENOMEM; >> + range->fwnode = &adev->fwnode; >> + range->flags = PIO_INDIRECT; >> + range->size = pdata->io_size; >> + range->hw_start = pdata->io_start; >> + >> + ret = logic_pio_register_range(range); >> + if (ret) >> + goto free_range; >> + >> + list_for_each_entry(child, &adev->children, node) >> + cell_num++; >> + >> + /* allocate the mfd cells */ >> + size = sizeof(*mfd_cells) + sizeof(*acpi_indirectio_mfd_cells); >> + mfd_cells = kcalloc(cell_num, size, GFP_KERNEL); >> + if (!mfd_cells) { >> + ret = -ENOMEM; >> + goto free_range; >> + } >> + >> + acpi_indirectio_mfd_cells = (struct acpi_indirectio_mfd_cell *) >> + &mfd_cells[cell_num]; >> + /* Only consider the children of the host */ >> + list_for_each_entry(child, &adev->children, node) { >> + struct mfd_cell *mfd_cell = &mfd_cells[count]; >> + struct acpi_indirectio_mfd_cell *acpi_indirectio_mfd_cell = >> + &acpi_indirectio_mfd_cells[count]; >> + const struct mfd_cell_acpi_match *acpi_match = >> + &acpi_indirectio_mfd_cell->acpi_match; >> + char *name = &acpi_indirectio_mfd_cell[count].name[0]; >> + char *pnpid = &acpi_indirectio_mfd_cell[count].pnpid[0]; >> + struct mfd_cell_acpi_match match = { >> + .pnpid = pnpid, >> + }; >> + >> + snprintf(name, ACPI_INDIRECTIO_NAME_LENGTH, "indirect-io-%s", >> + acpi_device_hid(child)); >> + snprintf(pnpid, ACPI_INDIRECTIO_NAME_LENGTH, "%s", >> + acpi_device_hid(child)); >> + >> + memcpy((void *)acpi_match, (void *)&match, sizeof(*acpi_match)); >> + mfd_cell->name = name; >> + mfd_cell->acpi_match = acpi_match; >> + >> + ret = >> + acpi_indirectio_set_logicio_res(&child->dev, >> + &adev->dev, >> + &mfd_cell->resources, >> + &mfd_cell->num_resources); >> + if (ret) { >> + dev_err(&child->dev, "set resource failed (%d)\n", ret); >> + goto free_mfd_res; >> + } >> + count++; >> + } >> + >> + pdev = acpi_create_platform_device(adev, NULL); >> + if (IS_ERR_OR_NULL(pdev)) { >> + dev_err(&adev->dev, "Create platform device for host failed!\n"); >> + ret = PTR_ERR(pdev); >> + goto free_mfd_res; >> + } >> + acpi_device_set_enumerated(adev); >> + >> + ret = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, >> + mfd_cells, cell_num, NULL, 0, NULL); >> + if (ret) { >> + dev_err(&pdev->dev, "failed to add mfd cells (%d)\n", ret); >> + goto free_mfd_res; >> + } >> + >> + return ret; >> + >> +free_mfd_res: >> + while (cell_num--) >> + kfree(mfd_cells[cell_num].resources); >> + kfree(mfd_cells); >> +free_range: >> + kfree(range); >> + >> + return ret; >> +} >> + >> +/* All the host devices which apply indirect-IO can be listed here. */ >> +static const struct acpi_device_id acpi_indirect_host_id[] = { >> + {""}, >> +}; >> + >> +static int acpi_indirectio_attach(struct acpi_device *adev, >> + const struct acpi_device_id *id) >> +{ >> + struct acpi_indirectio_device_desc *hostdata; >> + int ret; >> + >> + hostdata = (struct acpi_indirectio_device_desc *)id->driver_data; >> + if (!hostdata || !hostdata->pre_setup) >> + return -EINVAL; >> + >> + ret = hostdata->pre_setup(adev, &hostdata->pdata); >> + >> + if (ret < 0) >> + return ret; >> + >> + return 1; >> +} >> + >> +static struct acpi_scan_handler acpi_indirect_handler = { > > acpi_indirect_io_handler? sure > >> + .ids = acpi_indirect_host_id, >> + .attach = acpi_indirectio_attach, >> +}; >> + >> +void __init acpi_indirectio_scan_init(void) >> +{ >> + acpi_scan_add_handler(&acpi_indirect_handler); >> +} >> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h >> index 1d0a501..d6b1a95 100644 >> --- a/drivers/acpi/internal.h >> +++ b/drivers/acpi/internal.h >> @@ -31,6 +31,11 @@ >> void acpi_platform_init(void); >> void acpi_pnp_init(void); >> void acpi_int340x_thermal_init(void); >> +#ifdef CONFIG_INDIRECT_PIO >> +void acpi_indirectio_scan_init(void); >> +#else >> +static inline void acpi_indirectio_scan_init(void) {} >> +#endif >> #ifdef CONFIG_ARM_AMBA >> void acpi_amba_init(void); >> #else >> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c >> index b0fe527..8ea6f69 100644 >> --- a/drivers/acpi/scan.c >> +++ b/drivers/acpi/scan.c >> @@ -2141,6 +2141,7 @@ int __init acpi_scan_init(void) >> acpi_amba_init(); >> acpi_watchdog_init(); >> acpi_init_lpit(); >> + acpi_indirectio_scan_init(); >> >> acpi_scan_add_handler(&generic_device_handler); >> >> -- >> 1.9.1 >> Thank you, John
On Mon, 2018-02-05 at 11:01 +0000, John Garry wrote: > On 04/02/2018 07:45, Rafael J. Wysocki wrote: > > + /* translate the I/O resources */ > > > + for (i = 0; i < count; i++) { > > > + if (resources[i].flags & IORESOURCE_IO) { > > > + ret = acpi_translate_logicio_res(adev, host, > > > + &resources[i]); > > > > You don't need to break this line as far as I'm concerned. > > This is just to keep checkpatch happy. I could move the complete > function call to a single line. And also shortening some symbols will help. This could also use continue as well /* translate the I/O resources */ for (i = 0; i < count; i++) { if (!(resources[i].flags & IORESOURCE_IO)) continue; ret = acpi_translate_logicio_res(adev, host, &resources[i]); if (ret) { kfree(resources); dev_err(child, "Translate I/O range failed (%d)!\n", ret); return ret; } }
On 05/02/2018 12:10, Joe Perches wrote: > On Mon, 2018-02-05 at 11:01 +0000, John Garry wrote: >> On 04/02/2018 07:45, Rafael J. Wysocki wrote: >>> + /* translate the I/O resources */ >>>> + for (i = 0; i < count; i++) { >>>> + if (resources[i].flags & IORESOURCE_IO) { >>>> + ret = acpi_translate_logicio_res(adev, host, >>>> + &resources[i]); >>> >>> You don't need to break this line as far as I'm concerned. >> >> This is just to keep checkpatch happy. I could move the complete >> function call to a single line. And also shortening some symbols will help. > > This could also use continue as well > > /* translate the I/O resources */ > for (i = 0; i < count; i++) { > if (!(resources[i].flags & IORESOURCE_IO)) > continue; > ret = acpi_translate_logicio_res(adev, host, &resources[i]); > if (ret) { > kfree(resources); > dev_err(child, "Translate I/O range failed (%d)!\n", > ret); > return ret; > } > } Sure, that reduces the indentation as well. cheers > > > . >
On Thu, Feb 1, 2018 at 1:32 PM, John Garry <john.garry@huawei.com> wrote: I'm not going through all patch, by one thing I would like you to pay attention on, i.e. printing resource_size_t and struct resource >> + dev_err(dev, "translate bus-addr(0x%llx) fail!\n", >> + resource->start); resource_size_t is dynamic width type, you will see a compiler warning. For this we have %pap specifier. Moreover, in some cases it's useful to print struct resource via %pR or %pr. Consider reading kernel documentation about these (printk-formats.rst). >> +struct acpi_indirectio_host_data { >> + resource_size_t io_size; >> + resource_size_t io_start; >> +}; Why not utilize struct resource? If it's coming from platform / firmware it should *never* have types like size_t, unsigned long, resource_size_t, etc. >> +/* All the host devices which apply indirect-IO can be listed here. */ >> +static const struct acpi_device_id acpi_indirect_host_id[] = { >> + {""}, >> +}; The idea of terminator is to be such (remove comma there). And it's basically redundant to have an empty string there. Moreover, it's a waste of resources in ro section.
On 05/02/2018 13:16, Andy Shevchenko wrote: > On Thu, Feb 1, 2018 at 1:32 PM, John Garry <john.garry@huawei.com> wrote: > > Hi Andy, > I'm not going through all patch, by one thing I would like you to pay > attention on, i.e. > printing resource_size_t and struct resource > >>> + dev_err(dev, "translate bus-addr(0x%llx) fail!\n", >>> + resource->start); > > resource_size_t is dynamic width type, you will see a compiler > warning. For this we have > %pap specifier. > > Moreover, in some cases it's useful to print struct resource via %pR or %pr. I have already fixed this up in my rework. > > Consider reading kernel documentation about these (printk-formats.rst). > >>> +struct acpi_indirectio_host_data { >>> + resource_size_t io_size; >>> + resource_size_t io_start; >>> +}; > > Why not utilize struct resource? > > If it's coming from platform / firmware it should *never* have types > like size_t, unsigned long, resource_size_t, etc. This is not coming from firmware, but it is a struct which we define in the kernel per host, describing that host bus address range for translation. In fact, generally io_start is 0 and the io_size would be PIO_INDIRECT_SIZE, so I'll consider removing it for now. > >>> +/* All the host devices which apply indirect-IO can be listed here. */ >>> +static const struct acpi_device_id acpi_indirect_host_id[] = { >>> + {""}, >>> +}; > > The idea of terminator is to be such (remove comma there). And it's > basically redundant to have an empty string there. Moreover, it's a > waste of resources in ro section. OK, the comma should be removed. So what is the preferred acpi device id array sentinel? I see {}, {"", 0}, and {""} used. > thank you, John
On Mon, Feb 5, 2018 at 4:25 PM, John Garry <john.garry@huawei.com> wrote: > On 05/02/2018 13:16, Andy Shevchenko wrote: >> On Thu, Feb 1, 2018 at 1:32 PM, John Garry <john.garry@huawei.com> wrote: >> resource_size_t is dynamic width type, you will see a compiler >> warning. For this we have >> %pap specifier. >> >> Moreover, in some cases it's useful to print struct resource via %pR or >> %pr. > I have already fixed this up in my rework. Good! >>>> +/* All the host devices which apply indirect-IO can be listed here. */ >>>> +static const struct acpi_device_id acpi_indirect_host_id[] = { >>>> + {""}, >>>> +}; >> >> >> The idea of terminator is to be such (remove comma there). And it's >> basically redundant to have an empty string there. Moreover, it's a >> waste of resources in ro section. > > > OK, the comma should be removed. So what is the preferred acpi device id > array sentinel? I see {}, {"", 0}, and {""} used. The rule of thumb --- whatever has more entries in the kernel. In case of similarity I would prefer simple {} because it suits for termination slightly better (for my opinion, some cases might potentially have {"", non_null_vakue}).
diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile index 1017def..f4a7f46 100644 --- a/drivers/acpi/arm64/Makefile +++ b/drivers/acpi/arm64/Makefile @@ -1,2 +1,3 @@ obj-$(CONFIG_ACPI_IORT) += iort.o obj-$(CONFIG_ACPI_GTDT) += gtdt.o +obj-$(CONFIG_INDIRECT_PIO) += acpi_indirectio.o diff --git a/drivers/acpi/arm64/acpi_indirectio.c b/drivers/acpi/arm64/acpi_indirectio.c new file mode 100644 index 0000000..2649f57 --- /dev/null +++ b/drivers/acpi/arm64/acpi_indirectio.c @@ -0,0 +1,273 @@ +/* + * ACPI support for indirect-IO bus. + * + * Copyright (C) 2017 HiSilicon Limited, All Rights Reserved. + * Author: Gabriele Paoloni <gabriele.paoloni@huawei.com> + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com> + * Author: John Garry <john.garry@huawei.com> + * + * 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/logic_pio.h> +#include <linux/mfd/core.h> +#include <linux/platform_device.h> + +ACPI_MODULE_NAME("indirect IO"); + +#define ACPI_INDIRECTIO_NAME_LENGTH 255 + +#define INDIRECT_IO_INFO(desc) ((unsigned long)&desc) + +struct acpi_indirectio_mfd_cell { + struct mfd_cell_acpi_match acpi_match; + char name[ACPI_INDIRECTIO_NAME_LENGTH]; + char pnpid[ACPI_INDIRECTIO_NAME_LENGTH]; +}; + +struct acpi_indirectio_host_data { + resource_size_t io_size; + resource_size_t io_start; +}; + +struct acpi_indirectio_device_desc { + struct acpi_indirectio_host_data pdata; /* device relevant info data */ + int (*pre_setup)(struct acpi_device *adev, + struct acpi_indirectio_host_data *pdata); +}; + +static int acpi_translate_logicio_res(struct acpi_device *adev, + struct acpi_device *host, struct resource *resource) +{ + unsigned long sys_port; + struct device *dev = &adev->dev; + resource_size_t length = resource->end - resource->start; + + sys_port = logic_pio_trans_hwaddr(&host->fwnode, resource->start, + length); + + if (sys_port == -1) { + dev_err(dev, "translate bus-addr(0x%llx) fail!\n", + resource->start); + return -EFAULT; + } + + resource->start = sys_port; + resource->end = sys_port + length; + + return 0; +} + +/* + * update/set the current I/O resource of the designated device node. + * after this calling, the enumeration can be started as the I/O resource + * had been translated to logicial I/O from bus-local I/O. + * + * @child: the device node to be updated the I/O resource; + * @hostdev: the device node where 'adev' is attached, which can be not + * the parent of 'adev'; + * @res: double pointer to be set to the address of the updated resources + * @num_res: address of the variable to contain the number of updated resources + * + * return 0 when successful, negative is for failure. + */ +int acpi_indirectio_set_logicio_res(struct device *child, + struct device *hostdev, + const struct resource **res, + int *num_res) +{ + struct acpi_device *adev; + struct acpi_device *host; + struct resource_entry *rentry; + LIST_HEAD(resource_list); + struct resource *resources = NULL; + int count; + int i; + int ret = -EIO; + + if (!child || !hostdev) + return -EINVAL; + + host = to_acpi_device(hostdev); + adev = to_acpi_device(child); + + /* check the device state */ + if (!adev->status.present) { + dev_info(child, "ACPI: device is not present!\n"); + return 0; + } + /* whether the child had been enumerated? */ + if (acpi_device_enumerated(adev)) { + dev_info(child, "ACPI: had been enumerated!\n"); + return 0; + } + + count = acpi_dev_get_resources(adev, &resource_list, NULL, NULL); + if (count <= 0) { + dev_err(&adev->dev, "failed to get ACPI resources\n"); + return count ? count : -EIO; + } + + resources = kcalloc(count, sizeof(struct resource), GFP_KERNEL); + if (!resources) { + acpi_dev_free_resource_list(&resource_list); + return -ENOMEM; + } + count = 0; + list_for_each_entry(rentry, &resource_list, node) + resources[count++] = *rentry->res; + + acpi_dev_free_resource_list(&resource_list); + + /* translate the I/O resources */ + for (i = 0; i < count; i++) { + if (resources[i].flags & IORESOURCE_IO) { + ret = acpi_translate_logicio_res(adev, host, + &resources[i]); + if (ret) { + kfree(resources); + dev_err(child, + "Translate I/O range failed (%d)!\n", + ret); + return ret; + } + } + } + *res = resources; + *num_res = count; + + return ret; +} + +int +acpi_indirectio_pre_setup(struct acpi_device *adev, + struct acpi_indirectio_host_data *pdata) +{ + struct platform_device *pdev; + struct mfd_cell *mfd_cells; + struct logic_pio_hwaddr *range; + struct acpi_device *child; + struct acpi_indirectio_mfd_cell *acpi_indirectio_mfd_cells; + int size, ret, count = 0, cell_num = 0; + + range = kzalloc(sizeof(*range), GFP_KERNEL); + if (!range) + return -ENOMEM; + range->fwnode = &adev->fwnode; + range->flags = PIO_INDIRECT; + range->size = pdata->io_size; + range->hw_start = pdata->io_start; + + ret = logic_pio_register_range(range); + if (ret) + goto free_range; + + list_for_each_entry(child, &adev->children, node) + cell_num++; + + /* allocate the mfd cells */ + size = sizeof(*mfd_cells) + sizeof(*acpi_indirectio_mfd_cells); + mfd_cells = kcalloc(cell_num, size, GFP_KERNEL); + if (!mfd_cells) { + ret = -ENOMEM; + goto free_range; + } + + acpi_indirectio_mfd_cells = (struct acpi_indirectio_mfd_cell *) + &mfd_cells[cell_num]; + /* Only consider the children of the host */ + list_for_each_entry(child, &adev->children, node) { + struct mfd_cell *mfd_cell = &mfd_cells[count]; + struct acpi_indirectio_mfd_cell *acpi_indirectio_mfd_cell = + &acpi_indirectio_mfd_cells[count]; + const struct mfd_cell_acpi_match *acpi_match = + &acpi_indirectio_mfd_cell->acpi_match; + char *name = &acpi_indirectio_mfd_cell[count].name[0]; + char *pnpid = &acpi_indirectio_mfd_cell[count].pnpid[0]; + struct mfd_cell_acpi_match match = { + .pnpid = pnpid, + }; + + snprintf(name, ACPI_INDIRECTIO_NAME_LENGTH, "indirect-io-%s", + acpi_device_hid(child)); + snprintf(pnpid, ACPI_INDIRECTIO_NAME_LENGTH, "%s", + acpi_device_hid(child)); + + memcpy((void *)acpi_match, (void *)&match, sizeof(*acpi_match)); + mfd_cell->name = name; + mfd_cell->acpi_match = acpi_match; + + ret = + acpi_indirectio_set_logicio_res(&child->dev, + &adev->dev, + &mfd_cell->resources, + &mfd_cell->num_resources); + if (ret) { + dev_err(&child->dev, "set resource failed (%d)\n", ret); + goto free_mfd_res; + } + count++; + } + + pdev = acpi_create_platform_device(adev, NULL); + if (IS_ERR_OR_NULL(pdev)) { + dev_err(&adev->dev, "Create platform device for host failed!\n"); + ret = PTR_ERR(pdev); + goto free_mfd_res; + } + acpi_device_set_enumerated(adev); + + ret = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, + mfd_cells, cell_num, NULL, 0, NULL); + if (ret) { + dev_err(&pdev->dev, "failed to add mfd cells (%d)\n", ret); + goto free_mfd_res; + } + + return ret; + +free_mfd_res: + while (cell_num--) + kfree(mfd_cells[cell_num].resources); + kfree(mfd_cells); +free_range: + kfree(range); + + return ret; +} + +/* All the host devices which apply indirect-IO can be listed here. */ +static const struct acpi_device_id acpi_indirect_host_id[] = { + {""}, +}; + +static int acpi_indirectio_attach(struct acpi_device *adev, + const struct acpi_device_id *id) +{ + struct acpi_indirectio_device_desc *hostdata; + int ret; + + hostdata = (struct acpi_indirectio_device_desc *)id->driver_data; + if (!hostdata || !hostdata->pre_setup) + return -EINVAL; + + ret = hostdata->pre_setup(adev, &hostdata->pdata); + + if (ret < 0) + return ret; + + return 1; +} + +static struct acpi_scan_handler acpi_indirect_handler = { + .ids = acpi_indirect_host_id, + .attach = acpi_indirectio_attach, +}; + +void __init acpi_indirectio_scan_init(void) +{ + acpi_scan_add_handler(&acpi_indirect_handler); +} diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 1d0a501..d6b1a95 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -31,6 +31,11 @@ void acpi_platform_init(void); void acpi_pnp_init(void); void acpi_int340x_thermal_init(void); +#ifdef CONFIG_INDIRECT_PIO +void acpi_indirectio_scan_init(void); +#else +static inline void acpi_indirectio_scan_init(void) {} +#endif #ifdef CONFIG_ARM_AMBA void acpi_amba_init(void); #else diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index b0fe527..8ea6f69 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -2141,6 +2141,7 @@ int __init acpi_scan_init(void) acpi_amba_init(); acpi_watchdog_init(); acpi_init_lpit(); + acpi_indirectio_scan_init(); acpi_scan_add_handler(&generic_device_handler);