| Message ID | 1416997898.17328.5.camel@kxue-X58A-UD3R (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Wed, Nov 26, 2014 at 06:31:38PM +0800, Ken Xue wrote: > On ?, 2014-11-24 at 02:47 +0100, Rafael J. Wysocki wrote: > > On Monday, November 24, 2014 01:02:30 AM Xue, Ken wrote: > > > > > > On Tuesday, November 18, 2014 01:58:11 PM Ken Xue wrote: > > > > This new feature is to interpret AMD specific ACPI device to platform > > > > device such as I2C, UART found on AMD CZ and later chipsets. It is > > > > based on example INTEL LPSS. Now, it can support AMD I2C & UART. > > > > > > > > Signed-off-by: Ken Xue <Ken.Xue@amd.com> > > > > Signed-off-by: Jeff Wu <Jeff.Wu@amd.com> > > > > > > Generally speaking, this seems to duplicate much code from acpi_lpss which should be re-used instead. What about moving the code that will be common between acpi_lpss and the new driver into a new file (say acpi_soc.c)? > > > > > > Also, you need to avoid automatic creation of platform devices when !X86_AMD_PLATFORM_DEVICE in analogy with what acpi_lpss does, or bad things will happen. > > > > > > [ken] sounds fair enough. Let me take action to merge drivers to acpi_soc.c ? or you have other plan? > > > > I'd prefer the common code to reside in one file (or one .c file and one header > > file), and the driver-specific code to stay in separate per-driver files. > > > [Ken] I wrote a proto type for acpi_soc.c. please help to confirm if it > can match your ideal. if yes, i will submit a new patch after do more > test and refine codes. I think it will impact lpss driver greatly, even > i have taken it into account. below codes is for acpi_soc.c. In general looks good. I have few comments though. > > >From fc323fb7b3b4cbb79bda05ce3b1d6d8dfe5e883b Mon Sep 17 00:00:00 2001 > From: Ken Xue <Ken.Xue@amd.com> > Date: Wed, 26 Nov 2014 17:15:30 +0800 > Subject: [PATCH] This is proto type for acpi_soc. > > Signed-off-by: Ken Xue <Ken.Xue@amd.com> > --- > arch/x86/Kconfig | 11 +++ > drivers/acpi/Makefile | 2 +- > drivers/acpi/acpi_apd.c | 123 ++++++++++++++++++++++++++++ > drivers/acpi/acpi_soc.c | 213 > ++++++++++++++++++++++++++++++++++++++++++++++++ This is line-wrapped, please make sure when you submit the actual patch that it is formatted properly. Also you need proper changelog etc. > drivers/acpi/acpi_soc.h | 91 +++++++++++++++++++++ > drivers/acpi/internal.h | 6 ++ > drivers/acpi/scan.c | 1 + > 7 files changed, 446 insertions(+), 1 deletion(-) > create mode 100755 drivers/acpi/acpi_apd.c > create mode 100755 drivers/acpi/acpi_soc.c > create mode 100755 drivers/acpi/acpi_soc.h > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index ded8a67..6402c79f 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -495,6 +495,17 @@ config X86_INTEL_LPSS > things like clock tree (common clock framework) and pincontrol > which are needed by the LPSS peripheral drivers. > > +config X86_AMD_PLATFORM_DEVICE > + bool "AMD ACPI2Platform devices support" > + depends on ACPI > + select COMMON_CLK > + select PINCTRL > + ---help--- > + Select to interpret AMD specific ACPI device to platform device > + such as I2C, UART found on AMD CARRIZO and later chipset. Selecting > + this option enables things like clock tree (common clock framework) > + and pinctrl. > + > config IOSF_MBI > tristate "Intel SoC IOSF Sideband support for SoC platforms" > depends on PCI > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile > index c3b2fcb..b07003a 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 acpi_apd.o > acpi-y += acpi_platform.o > acpi-y += acpi_pnp.o > acpi-y += int340x_thermal.o > diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c > new file mode 100755 > index 0000000..c7e916b > --- /dev/null > +++ b/drivers/acpi/acpi_apd.c > @@ -0,0 +1,123 @@ > +/* > + * AMD ACPI support for ACPI2platform device. > + * > + * Copyright (c) 2014, AMD Corporation. > + * Authors: Ken Xue <Ken.Xue@amd.com> > + * Wu, Jeff <Jeff.Wu@amd.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/clk.h> > +#include <linux/clkdev.h> > +#include <linux/clk-provider.h> > +#include <linux/err.h> > +#include <linux/io.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > + > +#include "internal.h" > +#include "acpi_soc.h" > + > +struct acpi_soc asoc; static? Also Is "asoc" good name? That might get confused with Alsa SoC (ASoC). > + > +#ifdef X86_AMD_PLATFORM_DEVICE > +static int acpi_apd_setup(struct acpi_soc_dev_private_data *pdata) > +{ > + struct acpi_soc_dev_desc *dev_desc = pdata->dev_desc; > + struct clk *clk = ERR_PTR(-ENODEV); > + > + if (dev_desc->clk) > + return 0; > + > + if (dev_desc->fixed_clk_rate) { > + clk = clk_register_fixed_rate(&pdata->adev->dev, > + dev_name(&pdata->adev->dev), > + NULL, CLK_IS_ROOT, dev_desc->rate); > + dev_desc->clk = clk; > + clk_register_clkdev(clk, NULL, dev_name(&pdata->adev->dev)); > + } > + > + return 0; > +} > + > +static struct acpi_soc_dev_desc cz_i2c_desc = { > + .setup = acpi_apd_setup; > + .fixed_clk_rate = 133000000, Oh, good so now we can get rid the hack we did for i2c-designware-platdrv.c with this commit: a445900c906092f i2c: designware: Add support for AMD I2C controller Since now you have means to pass clock to the driver. Are you going to handle that driver as well? > +}; > + > +static struct acpi_soc_dev_desc cz_uart_desc = { > + .setup = acpi_apd_setup; > + .fixed_clk_rate = 48000000, > +}; > + > +#else > + > +#define APD_ADDR(desc) (0UL) > + > +#endif /* CONFIG_X86_INTEL_LPSS */ > + > +static struct acpi_device_id acpi_apd_device_ids[] = { const? > + /* Generic apd devices */ > + { "AMD0010", APD_ADDR(cz_i2c_desc) }, > + { "AMD0020", APD_ADDR(cz_uart_desc) }, > + { } > +}; > + > +#ifdef X86_AMD_PLATFORM_DEVICE > + > +static ssize_t apd_device_desc_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + int ret; > + struct acpi_device *adev; > + struct acpi_soc_dev_private_data *pdata; > + struct acpi_soc_dev_desc *dev_desc; > + > + ret = acpi_bus_get_device(ACPI_HANDLE(dev), &adev); > + if (WARN_ON(ret)) > + return ret; > + > + pdata = acpi_driver_data(adev); > + if (WARN_ON(!pdata || !pdata->dev_desc)) > + return -ENODEV; > + > + dev_desc = pdata->dev_desc; > + if (dev_desc->fixed_clk_rate) > + return sprintf(buf, "Required fix rate clk %s: %ld\n", > + dev_desc->clk->name, > + dev_desc->fixed_clk_rate); > + else > + return sprintf(buf, "No need clk\n"); > +} > + > +static DEVICE_ATTR(device_desc, S_IRUSR, apd_device_desc_show, NULL); > + > +static struct attribute *apd_attrs[] = { > + &dev_attr_device_desc.attr, > + NULL, > +}; > + > +static struct attribute_group apd_attr_group = { > + .attrs = apd_attrs, > + .name = "apd", > +}; This requires updating sysfs ABI but then again, do you really need the above? And does it belong to sysfs in the first place? > + > +void __init acpi_apd_init(void) > +{ > + asoc.ids = acpi_apd_device_ids; > + asoc.attr_group = &apd_attr_group; > + register_acpi_soc(&asoc, true); > +} > + > +#else > + > +void __init acpi_apd_init(void) > +{ > + asoc.ids = acpi_apd_device_ids; > + register_acpi_soc(&asoc, false); > +} > +#endif > diff --git a/drivers/acpi/acpi_soc.c b/drivers/acpi/acpi_soc.c > new file mode 100755 > index 0000000..99df1ab > --- /dev/null > +++ b/drivers/acpi/acpi_soc.c > @@ -0,0 +1,213 @@ > +/* > + * ACPI SOC support for Intel Lynxpoint LPSS and AMD APD. > + * > + * Copyright (C) 2015, Intel Corporation & AMD Corporation > + * Authors: Ken Xue <Ken.Xue@amd.com> > + * Mika Westerberg <mika.westerberg@linux.intel.com> > + * Rafael J. Wysocki <rafael.j.wysocki@intel.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/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" > + > + Delete the extra blank line > +ACPI_MODULE_NAME("acpi_soc"); > + > +/*A list for all acpi soc device*/ > +static LIST_HEAD(asoc_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->dev_desc = dev_desc; > + > + /*setup device by a hook routine*/ The comment should look like this /* Setup device by hook routine */ but I think it does not provide any new information so you can just drop it. > + 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 *asoc_entry; > + const struct acpi_device_id *id = NULL; > + > + list_for_each_entry(asoc_entry, &asoc_list, list) { > + id = acpi_match_device(asoc_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 (asoc_entry->pm_domain) > + pdev->dev.pm_domain = asoc_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 (asoc_entry->pm_domain) > + pdev->dev.pm_domain = asoc_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) && > asoc_entry->pm_domain) > + return sysfs_create_group(&pdev->dev.kobj, > + asoc_entry->attr_group); > + case BUS_NOTIFY_DEL_DEVICE: > + if ((pdata->dev_desc->flags & ACPI_SOC_SYSFS) && > asoc_entry->pm_domain) > + sysfs_remove_group(&pdev->dev.kobj, asoc_entry->attr_group); > + default: > + 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); > + return; > +} > + > +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); > + return; > +} > + > +void register_acpi_soc(struct acpi_soc *asoc, bool > disable_scan_handler) > +{ > + struct acpi_scan_handler *acpi_soc_handler; > + > + INIT_LIST_HEAD(&asoc->list); > + list_add(&asoc->list, &asoc_list); > + > + acpi_soc_handler = kzalloc(sizeof(struct acpi_scan_handler), > GFP_KERNEL); > + acpi_soc_handler->ids = asoc->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); > +} > +EXPORT_SYMBOL_GPL(register_acpi_soc); I don't think we want to export these to modules. > diff --git a/drivers/acpi/acpi_soc.h b/drivers/acpi/acpi_soc.h > new file mode 100755 > index 0000000..96db30e > --- /dev/null > +++ b/drivers/acpi/acpi_soc.h And all this belongs to drivers/acpi/internal.h. > @@ -0,0 +1,91 @@ > +/* > + * ACPI SOC support for Intel Lynxpoint LPSS and AMD APD. > + * > + * Copyright (C) 2015, Intel Corporation & AMD Corporation > + * Authors: Ken Xue <Ken.Xue@amd.com> > + * Mika Westerberg <mika.westerberg@linux.intel.com> > + * Rafael J. Wysocki <rafael.j.wysocki@intel.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. > + */ > +#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) > + > +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; > +}; > + > +/** > + * struct acpi_soc_dev_desc - a descriptor for acpi device > + * @flags: 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 > + */ > +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 > + * Delete the above line. > + */ > +struct acpi_soc_dev_private_data { > + void __iomem *mmio_base; > + resource_size_t mmio_size; > + > + const struct acpi_soc_dev_desc *dev_desc; > + struct acpi_device *adev; > +}; > + > +/** > + * register_acpi_soc - register a new acpi soc > + * @asoc: acpi soc > + * @disable_scan_handler: true means remove deafult scan handle > + * false means use deafult scan handle > + * > + * register a new acpi soc into asoc_list and install deafult scan > handle. > + */ > +void register_acpi_soc(struct acpi_soc *asoc, bool > disable_scan_handler); > + > +#endif > diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h > index 447f6d6..c8a0e8e 100644 > --- a/drivers/acpi/internal.h > +++ b/drivers/acpi/internal.h > @@ -68,6 +68,12 @@ static inline void acpi_debugfs_init(void) > { return; } > #endif > void acpi_lpss_init(void); > > +#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE > +void acpi_apd_init(void); > +#else > +static inline void acpi_apd_init(void) {} > +#endif > + > acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src); > bool acpi_queue_hotplug_work(struct work_struct *work); > void acpi_device_hotplug(struct acpi_device *adev, u32 src); > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index 0476e90..24fef2b 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -2349,6 +2349,7 @@ int __init acpi_scan_init(void) > acpi_pci_link_init(); > acpi_processor_init(); > acpi_lpss_init(); > + acpi_apd_init(); > acpi_cmos_rtc_init(); > acpi_container_init(); > acpi_memory_hotplug_init(); > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 27, 2014 at 06:06:35PM +0100, Rafael J. Wysocki wrote: > On Thursday, November 27, 2014 01:46:37 PM Mika Westerberg wrote: > > On Wed, Nov 26, 2014 at 06:31:38PM +0800, Ken Xue wrote: > > [cut] > > > > diff --git a/drivers/acpi/acpi_soc.h b/drivers/acpi/acpi_soc.h > > > new file mode 100755 > > > index 0000000..96db30e > > > --- /dev/null > > > +++ b/drivers/acpi/acpi_soc.h > > > > And all this belongs to drivers/acpi/internal.h. > > I requested the new file. > > I don't really want things specific to SoC drivers to be mixed up with ACPI core > stuff. It may even be good to create a separate subdir for the SoC stuff, but > that can be done later. I see, thanks for the explanation. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday, November 27, 2014 01:46:37 PM Mika Westerberg wrote: > On Wed, Nov 26, 2014 at 06:31:38PM +0800, Ken Xue wrote: [cut] > > diff --git a/drivers/acpi/acpi_soc.h b/drivers/acpi/acpi_soc.h > > new file mode 100755 > > index 0000000..96db30e > > --- /dev/null > > +++ b/drivers/acpi/acpi_soc.h > > And all this belongs to drivers/acpi/internal.h. I requested the new file. I don't really want things specific to SoC drivers to be mixed up with ACPI core stuff. It may even be good to create a separate subdir for the SoC stuff, but that can be done later.
On Thu, 2014-11-27 at 13:46 +0200, Mika Westerberg wrote: > On Wed, Nov 26, 2014 at 06:31:38PM +0800, Ken Xue wrote: > > On Monday, 2014-11-24 at 02:47 +0100, Rafael J. Wysocki wrote: > > > On Monday, November 24, 2014 01:02:30 AM Xue, Ken wrote: > > > > > > > > On Tuesday, November 18, 2014 01:58:11 PM Ken Xue wrote: > > > > > This new feature is to interpret AMD specific ACPI device to platform > > > > > device such as I2C, UART found on AMD CZ and later chipsets. It is > > > > > based on example INTEL LPSS. Now, it can support AMD I2C & UART. > > > > > > > > > > Signed-off-by: Ken Xue <Ken.Xue@amd.com> > > > > > Signed-off-by: Jeff Wu <Jeff.Wu@amd.com> > > > > > > > > Generally speaking, this seems to duplicate much code from acpi_lpss which should be re-used instead. What about moving the code that will be common between acpi_lpss and the new driver into a new file (say acpi_soc.c)? > > > > > > > > Also, you need to avoid automatic creation of platform devices when !X86_AMD_PLATFORM_DEVICE in analogy with what acpi_lpss does, or bad things will happen. > > > > > > > > [ken] sounds fair enough. Let me take action to merge drivers to acpi_soc.c ? or you have other plan? > > > > > > I'd prefer the common code to reside in one file (or one .c file and one header > > > file), and the driver-specific code to stay in separate per-driver files. > > > > > [Ken] I wrote a proto type for acpi_soc.c. please help to confirm if it > > can match your ideal. if yes, i will submit a new patch after do more > > test and refine codes. I think it will impact lpss driver greatly, even > > i have taken it into account. below codes is for acpi_soc.c. > > In general looks good. I have few comments though. [Ken] thanks for your comments. > > > > >From fc323fb7b3b4cbb79bda05ce3b1d6d8dfe5e883b Mon Sep 17 00:00:00 2001 > > From: Ken Xue <Ken.Xue@amd.com> > > Date: Wed, 26 Nov 2014 17:15:30 +0800 > > Subject: [PATCH] This is proto type for acpi_soc. > > > > Signed-off-by: Ken Xue <Ken.Xue@amd.com> > > --- > > arch/x86/Kconfig | 11 +++ > > drivers/acpi/Makefile | 2 +- > > drivers/acpi/acpi_apd.c | 123 ++++++++++++++++++++++++++++ > > drivers/acpi/acpi_soc.c | 213 > > ++++++++++++++++++++++++++++++++++++++++++++++++ > > This is line-wrapped, please make sure when you submit the actual patch > that it is formatted properly. Also you need proper changelog etc. [Ken] sure. > > +#include "internal.h" > > +#include "acpi_soc.h" > > + > > +struct acpi_soc asoc; > > static? > > Also Is "asoc" good name? That might get confused with Alsa SoC (ASoC). [Ken] I will use "static", and change name to be a_soc. > > + > > +static struct acpi_soc_dev_desc cz_i2c_desc = { > > + .setup = acpi_apd_setup; > > + .fixed_clk_rate = 133000000, > > Oh, good so now we can get rid the hack we did for > i2c-designware-platdrv.c with this commit: > > a445900c906092f i2c: designware: Add support for AMD I2C controller > > Since now you have means to pass clock to the driver. > > Are you going to handle that driver as well? [Ken]sure, thanks. this was one of reasons to create AMD APD. > > +}; > > + > > +static struct acpi_soc_dev_desc cz_uart_desc = { > > + .setup = acpi_apd_setup; > > + .fixed_clk_rate = 48000000, > > +}; > > + > > +#else > > + > > +#define APD_ADDR(desc) (0UL) > > + > > +#endif /* CONFIG_X86_INTEL_LPSS */ > > + > > +static struct acpi_device_id acpi_apd_device_ids[] = { > > const? [ken]No. "acpi_soc_dev_desc" may be modified later. > > + /* Generic apd devices */ > > + { "AMD0010", APD_ADDR(cz_i2c_desc) }, > > + { "AMD0020", APD_ADDR(cz_uart_desc) }, > > + { } > > +}; > > + > > +#ifdef X86_AMD_PLATFORM_DEVICE > > + > > +static ssize_t apd_device_desc_show(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + int ret; > > + struct acpi_device *adev; > > + struct acpi_soc_dev_private_data *pdata; > > + struct acpi_soc_dev_desc *dev_desc; > > + > > + ret = acpi_bus_get_device(ACPI_HANDLE(dev), &adev); > > + if (WARN_ON(ret)) > > + return ret; > > + > > + pdata = acpi_driver_data(adev); > > + if (WARN_ON(!pdata || !pdata->dev_desc)) > > + return -ENODEV; > > + > > + dev_desc = pdata->dev_desc; > > + if (dev_desc->fixed_clk_rate) > > + return sprintf(buf, "Required fix rate clk %s: %ld\n", > > + dev_desc->clk->name, > > + dev_desc->fixed_clk_rate); > > + else > > + return sprintf(buf, "No need clk\n"); > > +} > > + > > +static DEVICE_ATTR(device_desc, S_IRUSR, apd_device_desc_show, NULL); > > + > > +static struct attribute *apd_attrs[] = { > > + &dev_attr_device_desc.attr, > > + NULL, > > +}; > > + > > +static struct attribute_group apd_attr_group = { > > + .attrs = apd_attrs, > > + .name = "apd", > > +}; > > This requires updating sysfs ABI but then again, do you really need the > above? And does it belong to sysfs in the first place? [Ken] just want to output some debug information with sysfs. I think i can add sysfs interface after APD has rich features in the future. > > > > +#include "internal.h" > > +#include "acpi_soc.h" > > + > > + > > Delete the extra blank line [ken] got it. > > > > + pdata->dev_desc = dev_desc; > > + > > + /*setup device by a hook routine*/ > > The comment should look like this > > /* Setup device by hook routine */ > > but I think it does not provide any new information so you can just drop > it. [Ken] i will remove it. > > + > > +void register_acpi_soc(struct acpi_soc *asoc, bool > > disable_scan_handler) > > +{ > > + struct acpi_scan_handler *acpi_soc_handler; > > + > > + INIT_LIST_HEAD(&asoc->list); > > + list_add(&asoc->list, &asoc_list); > > + > > + acpi_soc_handler = kzalloc(sizeof(struct acpi_scan_handler), > > GFP_KERNEL); > > + acpi_soc_handler->ids = asoc->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); > > +} > > +EXPORT_SYMBOL_GPL(register_acpi_soc); > > I don't think we want to export these to modules. [ken]i will remove it. > > + > > +/** > > + * 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 > > + * > > Delete the above line. [Ken]ok. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index ded8a67..6402c79f 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -495,6 +495,17 @@ config X86_INTEL_LPSS things like clock tree (common clock framework) and pincontrol which are needed by the LPSS peripheral drivers. +config X86_AMD_PLATFORM_DEVICE + bool "AMD ACPI2Platform devices support" + depends on ACPI + select COMMON_CLK + select PINCTRL + ---help--- + Select to interpret AMD specific ACPI device to platform device + such as I2C, UART found on AMD CARRIZO and later chipset. Selecting + this option enables things like clock tree (common clock framework) + and pinctrl. + config IOSF_MBI tristate "Intel SoC IOSF Sideband support for SoC platforms" depends on PCI diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index c3b2fcb..b07003a 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 acpi_apd.o acpi-y += acpi_platform.o acpi-y += acpi_pnp.o acpi-y += int340x_thermal.o diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c new file mode 100755 index 0000000..c7e916b --- /dev/null +++ b/drivers/acpi/acpi_apd.c @@ -0,0 +1,123 @@ +/* + * AMD ACPI support for ACPI2platform device. + * + * Copyright (c) 2014, AMD Corporation. + * Authors: Ken Xue <Ken.Xue@amd.com> + * Wu, Jeff <Jeff.Wu@amd.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/clk.h> +#include <linux/clkdev.h> +#include <linux/clk-provider.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> + +#include "internal.h" +#include "acpi_soc.h" + +struct acpi_soc asoc; + +#ifdef X86_AMD_PLATFORM_DEVICE +static int acpi_apd_setup(struct acpi_soc_dev_private_data *pdata) +{ + struct acpi_soc_dev_desc *dev_desc = pdata->dev_desc; + struct clk *clk = ERR_PTR(-ENODEV); + + if (dev_desc->clk) + return 0; + + if (dev_desc->fixed_clk_rate) { + clk = clk_register_fixed_rate(&pdata->adev->dev, + dev_name(&pdata->adev->dev), + NULL, CLK_IS_ROOT, dev_desc->rate); + dev_desc->clk = clk; + clk_register_clkdev(clk, NULL, dev_name(&pdata->adev->dev)); + } + + return 0; +} + +static struct acpi_soc_dev_desc cz_i2c_desc = { + .setup = acpi_apd_setup; + .fixed_clk_rate = 133000000, +}; + +static struct acpi_soc_dev_desc cz_uart_desc = { + .setup = acpi_apd_setup; + .fixed_clk_rate = 48000000, +}; + +#else + +#define APD_ADDR(desc) (0UL) + +#endif /* CONFIG_X86_INTEL_LPSS */ + +static struct acpi_device_id acpi_apd_device_ids[] = { + /* Generic apd devices */ + { "AMD0010", APD_ADDR(cz_i2c_desc) }, + { "AMD0020", APD_ADDR(cz_uart_desc) }, + { } +}; + +#ifdef X86_AMD_PLATFORM_DEVICE + +static ssize_t apd_device_desc_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + int ret; + struct acpi_device *adev; + struct acpi_soc_dev_private_data *pdata; + struct acpi_soc_dev_desc *dev_desc; + + ret = acpi_bus_get_device(ACPI_HANDLE(dev), &adev); + if (WARN_ON(ret)) + return ret; + + pdata = acpi_driver_data(adev); + if (WARN_ON(!pdata || !pdata->dev_desc)) + return -ENODEV; + + dev_desc = pdata->dev_desc; + if (dev_desc->fixed_clk_rate) + return sprintf(buf, "Required fix rate clk %s: %ld\n", + dev_desc->clk->name, + dev_desc->fixed_clk_rate); + else + return sprintf(buf, "No need clk\n"); +} + +static DEVICE_ATTR(device_desc, S_IRUSR, apd_device_desc_show, NULL); + +static struct attribute *apd_attrs[] = { + &dev_attr_device_desc.attr, + NULL, +}; + +static struct attribute_group apd_attr_group = { + .attrs = apd_attrs, + .name = "apd", +}; + +void __init acpi_apd_init(void) +{ + asoc.ids = acpi_apd_device_ids; + asoc.attr_group = &apd_attr_group; + register_acpi_soc(&asoc, true); +} + +#else + +void __init acpi_apd_init(void) +{ + asoc.ids = acpi_apd_device_ids; + register_acpi_soc(&asoc, false); +} +#endif diff --git a/drivers/acpi/acpi_soc.c b/drivers/acpi/acpi_soc.c new file mode 100755 index 0000000..99df1ab --- /dev/null +++ b/drivers/acpi/acpi_soc.c @@ -0,0 +1,213 @@ +/* + * ACPI SOC support for Intel Lynxpoint LPSS and AMD APD. + * + * Copyright (C) 2015, Intel Corporation & AMD Corporation + * Authors: Ken Xue <Ken.Xue@amd.com> + * Mika Westerberg <mika.westerberg@linux.intel.com> + * Rafael J. Wysocki <rafael.j.wysocki@intel.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/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(asoc_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->dev_desc = dev_desc; + + /*setup device by a hook routine*/ + 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 *asoc_entry; + const struct acpi_device_id *id = NULL; + + list_for_each_entry(asoc_entry, &asoc_list, list) { + id = acpi_match_device(asoc_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 (asoc_entry->pm_domain) + pdev->dev.pm_domain = asoc_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 (asoc_entry->pm_domain) + pdev->dev.pm_domain = asoc_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) && asoc_entry->pm_domain) + return sysfs_create_group(&pdev->dev.kobj, + asoc_entry->attr_group); + case BUS_NOTIFY_DEL_DEVICE: + if ((pdata->dev_desc->flags & ACPI_SOC_SYSFS) && asoc_entry->pm_domain) + sysfs_remove_group(&pdev->dev.kobj, asoc_entry->attr_group); + default: + 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); + return; +} + +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); + return; +} + +void register_acpi_soc(struct acpi_soc *asoc, bool disable_scan_handler) +{ + struct acpi_scan_handler *acpi_soc_handler; + + INIT_LIST_HEAD(&asoc->list); + list_add(&asoc->list, &asoc_list); + + acpi_soc_handler = kzalloc(sizeof(struct acpi_scan_handler), GFP_KERNEL); + acpi_soc_handler->ids = asoc->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); +} +EXPORT_SYMBOL_GPL(register_acpi_soc); diff --git a/drivers/acpi/acpi_soc.h b/drivers/acpi/acpi_soc.h new file mode 100755 index 0000000..96db30e --- /dev/null +++ b/drivers/acpi/acpi_soc.h @@ -0,0 +1,91 @@ +/* + * ACPI SOC support for Intel Lynxpoint LPSS and AMD APD. + * + * Copyright (C) 2015, Intel Corporation & AMD Corporation + * Authors: Ken Xue <Ken.Xue@amd.com> + * Mika Westerberg <mika.westerberg@linux.intel.com> + * Rafael J. Wysocki <rafael.j.wysocki@intel.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. + */ +#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) + +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; +}; + +/** + * struct acpi_soc_dev_desc - a descriptor for acpi device + * @flags: 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 + */ +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 + * + */ +struct acpi_soc_dev_private_data { + void __iomem *mmio_base; + resource_size_t mmio_size; + + const struct acpi_soc_dev_desc *dev_desc; + struct acpi_device *adev; +}; + +/** + * register_acpi_soc - register a new acpi soc + * @asoc: acpi soc + * @disable_scan_handler: true means remove deafult scan handle + * false means use deafult scan handle + * + * register a new acpi soc into asoc_list and install deafult scan handle. + */ +void register_acpi_soc(struct acpi_soc *asoc, bool disable_scan_handler); + +#endif diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 447f6d6..c8a0e8e 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -68,6 +68,12 @@ static inline void acpi_debugfs_init(void) { return; } #endif void acpi_lpss_init(void); +#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE +void acpi_apd_init(void); +#else +static inline void acpi_apd_init(void) {} +#endif + acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src); bool acpi_queue_hotplug_work(struct work_struct *work); void acpi_device_hotplug(struct acpi_device *adev, u32 src); diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 0476e90..24fef2b 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -2349,6 +2349,7 @@ int __init acpi_scan_init(void) acpi_pci_link_init(); acpi_processor_init(); acpi_lpss_init(); + acpi_apd_init(); acpi_cmos_rtc_init(); acpi_container_init(); acpi_memory_hotplug_init();