On Wed, Dec 03, 2014 at 08:33:37PM +0800, Ken Xue wrote: > This patch is supposed to deliver some common codes for AMD APD and > INTEL LPSS. It can help to convert some specific acpi devices to be > platform devices. > > Signed-off-by: Ken Xue <Ken.Xue@xxxxxxx> > --- > drivers/acpi/Makefile | 2 +- > drivers/acpi/acpi_soc.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++++ > drivers/acpi/acpi_soc.h | 90 +++++++++++++++++++++ > 3 files changed, 302 insertions(+), 1 deletion(-) > create mode 100644 drivers/acpi/acpi_soc.c > create mode 100644 drivers/acpi/acpi_soc.h > > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile > index c3b2fcb..ae3397d 100644 > --- a/drivers/acpi/Makefile > +++ b/drivers/acpi/Makefile > @@ -40,7 +40,7 @@ acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) += processor_pdc.o > acpi-y += ec.o > acpi-$(CONFIG_ACPI_DOCK) += dock.o > acpi-y += pci_root.o pci_link.o pci_irq.o > -acpi-y += acpi_lpss.o > +acpi-y += acpi_soc.o acpi_lpss.o I think you need to convert acpi_lpss.c to use this new acpi_soc.c. > acpi-y += acpi_platform.o > acpi-y += acpi_pnp.o > acpi-y += int340x_thermal.o > diff --git a/drivers/acpi/acpi_soc.c b/drivers/acpi/acpi_soc.c > new file mode 100644 > index 0000000..25089a0 > --- /dev/null > +++ b/drivers/acpi/acpi_soc.c > @@ -0,0 +1,211 @@ > +/* > + * ACPI SOC support for Intel Lynxpoint LPSS and AMD APD. > + * > + * Copyright (C) 2015, Intel Corporation & AMD Corporation > + * Authors: Ken Xue <Ken.Xue@xxxxxxx> > + * Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> > + * Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > + * > + * 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/err.h> > +#include <linux/io.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > +#include <linux/delay.h> > +#include <linux/pm_domain.h> > + > +#include "internal.h" > +#include "acpi_soc.h" > + > +ACPI_MODULE_NAME("acpi_soc"); > + > +/* A list for all acpi soc device */ > +static LIST_HEAD(a_soc_list); > + > +static int is_memory(struct acpi_resource *res, void *not_used) > +{ > + struct resource r; > + > + return !acpi_dev_resource_memory(res, &r); > +} > + > +static int acpi_soc_create_device(struct acpi_device *adev, > + const struct acpi_device_id *id) > +{ > + struct acpi_soc_dev_desc *dev_desc; > + struct acpi_soc_dev_private_data *pdata; > + struct resource_list_entry *rentry; > + struct list_head resource_list; > + struct platform_device *pdev; > + int ret; > + > + dev_desc = (struct acpi_soc_dev_desc *)id->driver_data; > + if (!dev_desc) { > + pdev = acpi_create_platform_device(adev); > + return IS_ERR_OR_NULL(pdev) ? PTR_ERR(pdev) : 1; > + } > + pdata = kzalloc(sizeof(*pdata), GFP_KERNEL); > + if (!pdata) > + return -ENOMEM; > + > + INIT_LIST_HEAD(&resource_list); > + ret = acpi_dev_get_resources(adev, &resource_list, is_memory, NULL); > + if (ret < 0) > + goto err_out; > + > + list_for_each_entry(rentry, &resource_list, node) > + if (resource_type(&rentry->res) == IORESOURCE_MEM) { > + if (dev_desc->mem_size_override) > + pdata->mmio_size = dev_desc->mem_size_override; > + else > + pdata->mmio_size = resource_size(&rentry->res); > + pdata->mmio_base = ioremap(rentry->res.start, > + pdata->mmio_size); > + break; > + } > + > + acpi_dev_free_resource_list(&resource_list); > + > + pdata->adev = adev; > + pdata->dev_desc = dev_desc; > + > + if (dev_desc->setup) > + dev_desc->setup(pdata); > + > + /* > + * This works around a known issue in ACPI tables where acpi soc devices > + * have _PS0 and _PS3 without _PSC (and no power resources), so > + * acpi_bus_init_power() will assume that the BIOS has put them into D0. > + */ > + ret = acpi_device_fix_up_power(adev); > + if (ret) { > + /* Skip the device, but continue the namespace scan. */ > + ret = 0; > + goto err_out; > + } > + > + adev->driver_data = pdata; > + pdev = acpi_create_platform_device(adev); > + if (!IS_ERR_OR_NULL(pdev)) > + return 1; > + > + ret = PTR_ERR(pdev); > + adev->driver_data = NULL; > + > + err_out: > + kfree(pdata); > + return ret; > +} > + > +static int acpi_soc_platform_notify(struct notifier_block *nb, > + unsigned long action, void *data) > +{ > + struct platform_device *pdev = to_platform_device(data); > + struct acpi_soc_dev_private_data *pdata; > + struct acpi_device *adev; > + struct acpi_soc *a_soc_entry; > + const struct acpi_device_id *id = NULL; > + > + list_for_each_entry(a_soc_entry, &a_soc_list, list) { > + id = acpi_match_device(a_soc_entry->ids, &pdev->dev); > + if (!id) > + break; > + } > + > + if (!id || !id->driver_data) > + return 0; > + > + if (acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev)) > + return 0; > + > + pdata = acpi_driver_data(adev); > + if (!pdata || !pdata->mmio_base) > + return 0; > + > + switch (action) { > + case BUS_NOTIFY_BOUND_DRIVER: > + if ((pdata->dev_desc->flags & ACPI_SOC_PM)) { > + if (a_soc_entry->pm_domain) > + pdev->dev.pm_domain = a_soc_entry->pm_domain; > + else if (pdata->dev_desc->flags & ACPI_SOC_PM_ON) > + dev_pm_domain_attach(&pdev->dev, true); > + else > + dev_pm_domain_attach(&pdev->dev, false); > + } > + break; > + case BUS_NOTIFY_UNBOUND_DRIVER: > + if ((pdata->dev_desc->flags & ACPI_SOC_PM)) { > + if (a_soc_entry->pm_domain) > + pdev->dev.pm_domain = a_soc_entry->pm_domain; > + else if (pdata->dev_desc->flags & ACPI_SOC_PM_ON) > + dev_pm_domain_detach(&pdev->dev, true); > + else > + dev_pm_domain_detach(&pdev->dev, false); > + } > + break; > + case BUS_NOTIFY_ADD_DEVICE: > + if ((pdata->dev_desc->flags & ACPI_SOC_SYSFS) > + && a_soc_entry->pm_domain) > + return sysfs_create_group(&pdev->dev.kobj, > + a_soc_entry->attr_group); > + case BUS_NOTIFY_DEL_DEVICE: > + if ((pdata->dev_desc->flags & ACPI_SOC_SYSFS) > + && a_soc_entry->pm_domain) > + sysfs_remove_group(&pdev->dev.kobj, > + a_soc_entry->attr_group); > + break; > + } > + > + return 0; > +} > + > +static struct notifier_block acpi_soc_nb = { > + .notifier_call = acpi_soc_platform_notify, > +}; > + > +static void acpi_soc_bind(struct device *dev) > +{ > + struct acpi_soc_dev_private_data *pdata; > + > + pdata = acpi_driver_data(ACPI_COMPANION(dev)); > + > + if (!pdata || !pdata->dev_desc || !pdata->dev_desc->bind) > + return; > + > + pdata->dev_desc->bind(pdata); > +} > + > +static void acpi_soc_unbind(struct device *dev) > +{ > + struct acpi_soc_dev_private_data *pdata; > + > + pdata = acpi_driver_data(ACPI_COMPANION(dev)); > + > + if (!pdata || !pdata->dev_desc || !pdata->dev_desc->unbind) > + return; > + > + pdata->dev_desc->unbind(pdata); > +} > + > +void register_acpi_soc(struct acpi_soc *a_soc, bool disable_scan_handler) Would it work better if we pass acpi_scan_handler as a parameter to this function? Then if it is NULL, it means that it will not register scan handler at all. And then provide the default handler in acpi_soc.h so that you can do register_acpi_soc(&soc, default_acpi_scan_handler); or register_acpi_soc(&soc, NULL); depending the case. But also support passing custom scan handler. > +{ > + struct acpi_scan_handler *acpi_soc_handler; > + > + INIT_LIST_HEAD(&a_soc->list); > + list_add(&a_soc->list, &a_soc_list); > + > + acpi_soc_handler = kzalloc(sizeof(*acpi_soc_handler), GFP_KERNEL); > + acpi_soc_handler->ids = a_soc->ids; > + if (!disable_scan_handler) { > + acpi_soc_handler->attach = acpi_soc_create_device; > + acpi_soc_handler->bind = acpi_soc_bind; > + acpi_soc_handler->unbind = acpi_soc_unbind; > + } > + acpi_scan_add_handler(acpi_soc_handler); > + bus_register_notifier(&platform_bus_type, &acpi_soc_nb); > +} > diff --git a/drivers/acpi/acpi_soc.h b/drivers/acpi/acpi_soc.h > new file mode 100644 > index 0000000..cc270a5 > --- /dev/null > +++ b/drivers/acpi/acpi_soc.h > @@ -0,0 +1,90 @@ > +/* > + * ACPI SOC support for Intel Lynxpoint LPSS and AMD APD. > + * > + * Copyright (C) 2015, Intel Corporation & AMD Corporation > + * Authors: Ken Xue <Ken.Xue@xxxxxxx> > + * Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> > + * Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > + * > + * 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. > + */ > +#ifndef _ACPI_SOC_H > +#define _ACPI_SOC_H > + > +#include <linux/acpi.h> > +#include <linux/clk.h> > +#include <linux/pm.h> > + > +/* Flags */ > +#define ACPI_SOC_SYSFS BIT(0) > +#define ACPI_SOC_PM BIT(1) > +#define ACPI_SOC_PM_ON BIT(2) Would be nice to document what those mean. > + > +struct acpi_soc_dev_private_data; > + > +/** > + * struct acpi_soc - acpi soc > + * @list: list head > + * @ids: all acpi device ids for acpi soc > + * @pm_domain: power domain for all acpi device;can be NULL > + * @attr_group: attribute group for sysfs support of acpi soc;can be NULL > + */ > +struct acpi_soc { > + struct list_head list; > + struct acpi_device_id *ids; > + struct dev_pm_domain *pm_domain; > + struct attribute_group *attr_group; These are tab aligned but list is using spaces. Please be consistent. > +}; > + > +/** > + * struct acpi_soc_dev_desc - a descriptor for acpi device > + * @flags: some device feature flags Please a bit better documentation than "some device feature flags". > + * @clk: clock device > + * @fixed_clk_rate: fixed rate input clock source for acpi device; > + * 0 means no fixed rate input clock source > + * @mem_size_override: a workaround for override device memsize; > + * 0 means no needs for this WA > + * @setup: a hook routine to set device resource during create platform device > + * @bind: a hook of acpi_scan_handler.bind > + * @unbind: a hook of acpi_scan_handler.unbind > + * > + *device description defined as acpi_device_id.driver_data Missing space. > + */ > +struct acpi_soc_dev_desc { > + unsigned int flags; > + struct clk *clk; > + unsigned int fixed_clk_rate; > + size_t mem_size_override; > + int (*setup)(struct acpi_soc_dev_private_data *pdata); > + void (*bind)(struct acpi_soc_dev_private_data *pdata); > + void (*unbind)(struct acpi_soc_dev_private_data *pdata); > +}; > + > +/** > + * struct acpi_soc_dev_private_data - acpi device private data > + * @mmio_base: virtual memory base addr of the device > + * @mmio_size: device memory size > + * @dev_desc: device description > + * @adev: apci device apci -> acpi > + */ > +struct acpi_soc_dev_private_data { > + void __iomem *mmio_base; > + resource_size_t mmio_size; > + > + struct acpi_soc_dev_desc *dev_desc; > + struct acpi_device *adev; > +}; > + > +/** > + * register_acpi_soc - register a new acpi soc > + * @a_soc: acpi soc > + * @disable_scan_handler: true means remove default scan handle > + * false means use default scan handle > + * > + * register a new acpi soc into asoc_list and install default scan handle. > + */ IMHO function kernel-docs belong close to the function implementation, e.g in this case to acpi_soc.c. > +void register_acpi_soc(struct acpi_soc *a_soc, bool disable_scan_handler); > + > +#endif > -- > 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html