Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758076Ab3CTQyN (ORCPT ); Wed, 20 Mar 2013 12:54:13 -0400 Received: from mail-da0-f45.google.com ([209.85.210.45]:48684 "EHLO mail-da0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755601Ab3CTQyK convert rfc822-to-8bit (ORCPT ); Wed, 20 Mar 2013 12:54:10 -0400 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT To: "Rafael J. Wysocki" , LKML From: Mike Turquette In-Reply-To: <1490138.pE1ld6sXHn@vostro.rjw.lan> Cc: ACPI Devel Maling List , Mika Westerberg , "Kristen C. Accardi" , Len Brown References: <1878438.IOSu0hheXQ@vostro.rjw.lan> <1490138.pE1ld6sXHn@vostro.rjw.lan> Message-ID: <20130320165401.18043.17922@quantum> User-Agent: alot/0.3.3+ Subject: Re: [PATCH 1/2] ACPI / scan: Add special handler for Intel Lynxpoint LPSS devices Date: Wed, 20 Mar 2013 09:54:01 -0700 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 21980 Lines: 598 Quoting Rafael J. Wysocki (2013-03-02 14:27:52) > From: Rafael J. Wysocki > > Devices on the Intel Lynxpoint Low Power Subsystem (LPSS) have some > common features that aren't shared with any other platform devices, > including the clock and LTR (Latency Tolerance Reporting) registers. > It is better to handle those features in common code than to bother > device drivers with doing that (I/O functionality-wise the LPSS > devices are generally compatible with other devices that don't > have those special registers and may be handled by the same drivers). > > The clock registers of the LPSS devices are now taken care of by > the special clk-x86-lpss driver, but the MMIO mappings used for > accessing those registers can also be used for accessing the LTR > registers on those devices (LTR support for the Lynxpoint LPSS is > going to be added by a subsequent patch). Thus it is convenient > to add a special ACPI scan handler for the Lynxpoint LPSS devices > that will create the MMIO mappings for accessing the clock (and > LTR in the future) registers and will register the LPSS devices' > clocks, so the clk-x86-lpss driver will only need to take care of > the main Lynxpoint LPSS clock. > > Introduce a special ACPI scan handler for Intel Lynxpoint LPSS > devices as described above. This also reduces overhead related to > browsing the ACPI namespace in search of the LPSS devices before the > registration of their clocks, removes some LPSS-specific (and > somewhat ugly) code from acpi_platform.c and shrinks the overall code > size slightly. > > Signed-off-by: Mika Westerberg > Signed-off-by: Rafael J. Wysocki Better late than never. For the clk changes: Acked-by: Mike Turquette > --- > drivers/acpi/Makefile | 1 > drivers/acpi/acpi_lpss.c | 163 +++++++++++++++++++++++++++++++++ > drivers/acpi/acpi_platform.c | 40 -------- > drivers/acpi/internal.h | 8 + > drivers/acpi/scan.c | 1 > drivers/clk/x86/Makefile | 2 > drivers/clk/x86/clk-lpss.c | 99 -------------------- > drivers/clk/x86/clk-lpss.h | 36 ------- > drivers/clk/x86/clk-lpt.c | 40 -------- > include/linux/platform_data/clk-lpss.h | 18 +++ > 10 files changed, 195 insertions(+), 213 deletions(-) > create mode 100644 drivers/acpi/acpi_lpss.c > delete mode 100644 drivers/clk/x86/clk-lpss.c > delete mode 100644 drivers/clk/x86/clk-lpss.h > create mode 100644 include/linux/platform_data/clk-lpss.h > > Index: linux-pm/drivers/acpi/Makefile > =================================================================== > --- linux-pm.orig/drivers/acpi/Makefile > +++ linux-pm/drivers/acpi/Makefile > @@ -39,6 +39,7 @@ acpi-y += ec.o > acpi-$(CONFIG_ACPI_DOCK) += dock.o > acpi-y += pci_root.o pci_link.o pci_irq.o > acpi-y += csrt.o > +acpi-$(CONFIG_X86_INTEL_LPSS) += acpi_lpss.o > acpi-y += acpi_platform.o > acpi-y += power.o > acpi-y += event.o > Index: linux-pm/drivers/acpi/acpi_lpss.c > =================================================================== > --- /dev/null > +++ linux-pm/drivers/acpi/acpi_lpss.c > @@ -0,0 +1,163 @@ > +/* > + * ACPI support for Intel Lynxpoint LPSS. > + * > + * Copyright (C) 2013, Intel Corporation > + * Authors: Mika Westerberg > + * Rafael J. Wysocki > + * > + * 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "internal.h" > + > +ACPI_MODULE_NAME("acpi_lpss"); > + > +#define LPSS_CLK_OFFSET 0x800 > +#define LPSS_CLK_SIZE 0x04 > + > +struct lpss_device_desc { > + bool clk_required; > + const char *clk_parent; > +}; > + > +struct lpss_private_data { > + void __iomem *mmio_base; > + resource_size_t mmio_size; > + struct clk *clk; > + const struct lpss_device_desc *dev_desc; > +}; > + > +static struct lpss_device_desc lpt_dev_desc = { > + .clk_required = true, > + .clk_parent = "lpss_clk", > +}; > + > +static const struct acpi_device_id acpi_lpss_device_ids[] = { > + /* Lynxpoint LPSS devices */ > + { "INT33C0", (unsigned long)&lpt_dev_desc }, > + { "INT33C1", (unsigned long)&lpt_dev_desc }, > + { "INT33C2", (unsigned long)&lpt_dev_desc }, > + { "INT33C3", (unsigned long)&lpt_dev_desc }, > + { "INT33C4", (unsigned long)&lpt_dev_desc }, > + { "INT33C5", (unsigned long)&lpt_dev_desc }, > + { "INT33C6", }, > + { "INT33C7", }, > + > + { } > +}; > + > +static int is_memory(struct acpi_resource *res, void *not_used) > +{ > + struct resource r; > + return !acpi_dev_resource_memory(res, &r); > +} > + > +/* LPSS main clock device. */ > +static struct platform_device *lpss_clk_dev; > + > +static inline void lpt_register_clock_device(void) > +{ > + lpss_clk_dev = platform_device_register_simple("clk-lpt", -1, NULL, 0); > +} > + > +static int register_device_clock(struct acpi_device *adev, > + struct lpss_private_data *pdata) > +{ > + const struct lpss_device_desc *dev_desc = pdata->dev_desc; > + > + if (!lpss_clk_dev) > + lpt_register_clock_device(); > + > + if (!dev_desc->clk_parent || !pdata->mmio_base > + || pdata->mmio_size < LPSS_CLK_OFFSET + LPSS_CLK_SIZE) > + return -ENODATA; > + > + pdata->clk = clk_register_gate(NULL, dev_name(&adev->dev), > + dev_desc->clk_parent, 0, > + pdata->mmio_base + LPSS_CLK_OFFSET, > + 0, 0, NULL); > + if (IS_ERR(pdata->clk)) > + return PTR_ERR(pdata->clk); > + > + clk_register_clkdev(pdata->clk, NULL, dev_name(&adev->dev)); > + return 0; > +} > + > +static int acpi_lpss_create_device(struct acpi_device *adev, > + const struct acpi_device_id *id) > +{ > + struct lpss_device_desc *dev_desc; > + struct lpss_private_data *pdata; > + struct resource_list_entry *rentry; > + struct list_head resource_list; > + int ret; > + > + dev_desc = (struct lpss_device_desc *)id->driver_data; > + if (!dev_desc) > + return acpi_create_platform_device(adev, id); > + > + 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) { > + pdata->mmio_size = resource_size(&rentry->res); > + pdata->mmio_base = ioremap(rentry->res.start, > + pdata->mmio_size); > + pdata->dev_desc = dev_desc; > + break; > + } > + > + acpi_dev_free_resource_list(&resource_list); > + > + if (dev_desc->clk_required) { > + ret = register_device_clock(adev, pdata); > + if (ret) { > + /* > + * Skip the device, but don't terminate the namespace > + * scan. > + */ > + ret = 0; > + goto err_out; > + } > + } > + > + adev->driver_data = pdata; > + ret = acpi_create_platform_device(adev, id); > + if (ret > 0) > + return ret; > + > + adev->driver_data = NULL; > + > + err_out: > + kfree(pdata); > + return ret; > +} > + > +static struct acpi_scan_handler lpss_handler = { > + .ids = acpi_lpss_device_ids, > + .attach = acpi_lpss_create_device, > +}; > + > +void __init acpi_lpss_init(void) > +{ > + if (!lpt_clk_init()) > + acpi_scan_add_handler(&lpss_handler); > +} > Index: linux-pm/drivers/acpi/acpi_platform.c > =================================================================== > --- linux-pm.orig/drivers/acpi/acpi_platform.c > +++ linux-pm/drivers/acpi/acpi_platform.c > @@ -22,9 +22,6 @@ > > ACPI_MODULE_NAME("platform"); > > -/* Flags for acpi_create_platform_device */ > -#define ACPI_PLATFORM_CLK BIT(0) > - > /* > * The following ACPI IDs are known to be suitable for representing as > * platform devices. > @@ -33,33 +30,9 @@ static const struct acpi_device_id acpi_ > > { "PNP0D40" }, > > - /* Haswell LPSS devices */ > - { "INT33C0", ACPI_PLATFORM_CLK }, > - { "INT33C1", ACPI_PLATFORM_CLK }, > - { "INT33C2", ACPI_PLATFORM_CLK }, > - { "INT33C3", ACPI_PLATFORM_CLK }, > - { "INT33C4", ACPI_PLATFORM_CLK }, > - { "INT33C5", ACPI_PLATFORM_CLK }, > - { "INT33C6", ACPI_PLATFORM_CLK }, > - { "INT33C7", ACPI_PLATFORM_CLK }, > - > { } > }; > > -static int acpi_create_platform_clks(struct acpi_device *adev) > -{ > - static struct platform_device *pdev; > - > - /* Create Lynxpoint LPSS clocks */ > - if (!pdev && !strncmp(acpi_device_hid(adev), "INT33C", 6)) { > - pdev = platform_device_register_simple("clk-lpt", -1, NULL, 0); > - if (IS_ERR(pdev)) > - return PTR_ERR(pdev); > - } > - > - return 0; > -} > - > /** > * acpi_create_platform_device - Create platform device for ACPI device node > * @adev: ACPI device node to create a platform device for. > @@ -71,10 +44,9 @@ static int acpi_create_platform_clks(str > * > * Name of the platform device will be the same as @adev's. > */ > -static int acpi_create_platform_device(struct acpi_device *adev, > - const struct acpi_device_id *id) > +int acpi_create_platform_device(struct acpi_device *adev, > + const struct acpi_device_id *id) > { > - unsigned long flags = id->driver_data; > struct platform_device *pdev = NULL; > struct acpi_device *acpi_parent; > struct platform_device_info pdevinfo; > @@ -83,14 +55,6 @@ static int acpi_create_platform_device(s > struct resource *resources; > int count; > > - if (flags & ACPI_PLATFORM_CLK) { > - int ret = acpi_create_platform_clks(adev); > - if (ret) { > - dev_err(&adev->dev, "failed to create clocks\n"); > - return ret; > - } > - } > - > /* If the ACPI node already has a physical device attached, skip it. */ > if (adev->physical_node_count) > return 0; > Index: linux-pm/drivers/acpi/internal.h > =================================================================== > --- linux-pm.orig/drivers/acpi/internal.h > +++ linux-pm/drivers/acpi/internal.h > @@ -48,6 +48,11 @@ int acpi_debugfs_init(void); > #else > static inline void acpi_debugfs_init(void) { return; } > #endif > +#ifdef CONFIG_X86_INTEL_LPSS > +void acpi_lpss_init(void); > +#else > +static inline void acpi_lpss_init(void) {} > +#endif > > /* -------------------------------------------------------------------------- > Device Node Initialization / Removal > @@ -131,4 +136,7 @@ static inline void suspend_nvs_restore(v > -------------------------------------------------------------------------- */ > struct platform_device; > > +int acpi_create_platform_device(struct acpi_device *adev, > + const struct acpi_device_id *id); > + > #endif /* _ACPI_INTERNAL_H_ */ > Index: linux-pm/drivers/acpi/scan.c > =================================================================== > --- linux-pm.orig/drivers/acpi/scan.c > +++ linux-pm/drivers/acpi/scan.c > @@ -1788,6 +1788,7 @@ int __init acpi_scan_init(void) > acpi_pci_root_init(); > acpi_pci_link_init(); > acpi_platform_init(); > + acpi_lpss_init(); > acpi_csrt_init(); > acpi_container_init(); > acpi_pci_slot_init(); > Index: linux-pm/drivers/clk/x86/Makefile > =================================================================== > --- linux-pm.orig/drivers/clk/x86/Makefile > +++ linux-pm/drivers/clk/x86/Makefile > @@ -1,2 +1,2 @@ > -clk-x86-lpss-objs := clk-lpss.o clk-lpt.o > +clk-x86-lpss-objs := clk-lpt.o > obj-$(CONFIG_X86_INTEL_LPSS) += clk-x86-lpss.o > Index: linux-pm/drivers/clk/x86/clk-lpss.c > =================================================================== > --- linux-pm.orig/drivers/clk/x86/clk-lpss.c > +++ /dev/null > @@ -1,99 +0,0 @@ > -/* > - * Intel Low Power Subsystem clocks. > - * > - * Copyright (C) 2013, Intel Corporation > - * Authors: Mika Westerberg > - * Heikki Krogerus > - * > - * 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 > -#include > -#include > -#include > -#include > -#include > - > -static int clk_lpss_is_mmio_resource(struct acpi_resource *res, void *data) > -{ > - struct resource r; > - return !acpi_dev_resource_memory(res, &r); > -} > - > -static acpi_status clk_lpss_find_mmio(acpi_handle handle, u32 level, > - void *data, void **retval) > -{ > - struct resource_list_entry *rentry; > - struct list_head resource_list; > - struct acpi_device *adev; > - const char *uid = data; > - int ret; > - > - if (acpi_bus_get_device(handle, &adev)) > - return AE_OK; > - > - if (uid) { > - if (!adev->pnp.unique_id) > - return AE_OK; > - if (strcmp(uid, adev->pnp.unique_id)) > - return AE_OK; > - } > - > - INIT_LIST_HEAD(&resource_list); > - ret = acpi_dev_get_resources(adev, &resource_list, > - clk_lpss_is_mmio_resource, NULL); > - if (ret < 0) > - return AE_NO_MEMORY; > - > - list_for_each_entry(rentry, &resource_list, node) > - if (resource_type(&rentry->res) == IORESOURCE_MEM) { > - *(struct resource *)retval = rentry->res; > - break; > - } > - > - acpi_dev_free_resource_list(&resource_list); > - return AE_OK; > -} > - > -/** > - * clk_register_lpss_gate - register LPSS clock gate > - * @name: name of this clock gate > - * @parent_name: parent clock name > - * @hid: ACPI _HID of the device > - * @uid: ACPI _UID of the device (optional) > - * @offset: LPSS PRV_CLOCK_PARAMS offset > - * > - * Creates and registers LPSS clock gate. > - */ > -struct clk *clk_register_lpss_gate(const char *name, const char *parent_name, > - const char *hid, const char *uid, > - unsigned offset) > -{ > - struct resource res = { }; > - void __iomem *mmio_base; > - acpi_status status; > - struct clk *clk; > - > - /* > - * First try to look the device and its mmio resource from the > - * ACPI namespace. > - */ > - status = acpi_get_devices(hid, clk_lpss_find_mmio, (void *)uid, > - (void **)&res); > - if (ACPI_FAILURE(status) || !res.start) > - return ERR_PTR(-ENODEV); > - > - mmio_base = ioremap(res.start, resource_size(&res)); > - if (!mmio_base) > - return ERR_PTR(-ENOMEM); > - > - clk = clk_register_gate(NULL, name, parent_name, 0, mmio_base + offset, > - 0, 0, NULL); > - if (IS_ERR(clk)) > - iounmap(mmio_base); > - > - return clk; > -} > Index: linux-pm/drivers/clk/x86/clk-lpss.h > =================================================================== > --- linux-pm.orig/drivers/clk/x86/clk-lpss.h > +++ /dev/null > @@ -1,36 +0,0 @@ > -/* > - * Intel Low Power Subsystem clock. > - * > - * Copyright (C) 2013, Intel Corporation > - * Authors: Mika Westerberg > - * Heikki Krogerus > - * > - * 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 __CLK_LPSS_H > -#define __CLK_LPSS_H > - > -#include > -#include > -#include > - > -#ifdef CONFIG_ACPI > -extern struct clk *clk_register_lpss_gate(const char *name, > - const char *parent_name, > - const char *hid, const char *uid, > - unsigned offset); > -#else > -static inline struct clk *clk_register_lpss_gate(const char *name, > - const char *parent_name, > - const char *hid, > - const char *uid, > - unsigned offset) > -{ > - return ERR_PTR(-ENODEV); > -} > -#endif > - > -#endif /* __CLK_LPSS_H */ > Index: linux-pm/drivers/clk/x86/clk-lpt.c > =================================================================== > --- linux-pm.orig/drivers/clk/x86/clk-lpt.c > +++ linux-pm/drivers/clk/x86/clk-lpt.c > @@ -10,7 +10,6 @@ > * published by the Free Software Foundation. > */ > > -#include > #include > #include > #include > @@ -18,8 +17,6 @@ > #include > #include > > -#include "clk-lpss.h" > - > #define PRV_CLOCK_PARAMS 0x800 > > static int lpt_clk_probe(struct platform_device *pdev) > @@ -34,40 +31,6 @@ static int lpt_clk_probe(struct platform > > /* Shared DMA clock */ > clk_register_clkdev(clk, "hclk", "INTL9C60.0.auto"); > - > - /* SPI clocks */ > - clk = clk_register_lpss_gate("spi0_clk", "lpss_clk", "INT33C0", NULL, > - PRV_CLOCK_PARAMS); > - if (!IS_ERR(clk)) > - clk_register_clkdev(clk, NULL, "INT33C0:00"); > - > - clk = clk_register_lpss_gate("spi1_clk", "lpss_clk", "INT33C1", NULL, > - PRV_CLOCK_PARAMS); > - if (!IS_ERR(clk)) > - clk_register_clkdev(clk, NULL, "INT33C1:00"); > - > - /* I2C clocks */ > - clk = clk_register_lpss_gate("i2c0_clk", "lpss_clk", "INT33C2", NULL, > - PRV_CLOCK_PARAMS); > - if (!IS_ERR(clk)) > - clk_register_clkdev(clk, NULL, "INT33C2:00"); > - > - clk = clk_register_lpss_gate("i2c1_clk", "lpss_clk", "INT33C3", NULL, > - PRV_CLOCK_PARAMS); > - if (!IS_ERR(clk)) > - clk_register_clkdev(clk, NULL, "INT33C3:00"); > - > - /* UART clocks */ > - clk = clk_register_lpss_gate("uart0_clk", "lpss_clk", "INT33C4", NULL, > - PRV_CLOCK_PARAMS); > - if (!IS_ERR(clk)) > - clk_register_clkdev(clk, NULL, "INT33C4:00"); > - > - clk = clk_register_lpss_gate("uart1_clk", "lpss_clk", "INT33C5", NULL, > - PRV_CLOCK_PARAMS); > - if (!IS_ERR(clk)) > - clk_register_clkdev(clk, NULL, "INT33C5:00"); > - > return 0; > } > > @@ -79,8 +42,7 @@ static struct platform_driver lpt_clk_dr > .probe = lpt_clk_probe, > }; > > -static int __init lpt_clk_init(void) > +int __init lpt_clk_init(void) > { > return platform_driver_register(&lpt_clk_driver); > } > -arch_initcall(lpt_clk_init); > Index: linux-pm/include/linux/platform_data/clk-lpss.h > =================================================================== > --- /dev/null > +++ linux-pm/include/linux/platform_data/clk-lpss.h > @@ -0,0 +1,18 @@ > +/* > + * Intel Low Power Subsystem clocks. > + * > + * Copyright (C) 2013, Intel Corporation > + * Authors: Mika Westerberg > + * Rafael J. Wysocki > + * > + * 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 __CLK_LPSS_H > +#define __CLK_LPSS_H > + > +extern int lpt_clk_init(void); > + > +#endif /* __CLK_LPSS_H */ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/