[PATCH v1 0/3] mfd: introduce a driver for LPSS devices on SPT

[PATCH v1 0/3] mfd: introduce a driver for LPSS devices on SPT

[Andy Shevchenko]

The new coming Intel platforms such as Skylake will contain Sunrisepoint PCH.
This patch brings the driver for such devices found on Sunrisepoint PCH.

The patch depends on published earlier [1] "clkdev: prevent potential memory
leak when used in modules".

[1] http://www.spinics.net/lists/kernel/msg1950939.html

Andy Shevchenko (1):
  mfd: Add support for Intel Sunrisepoint LPSS devices

Mika Westerberg (2):
  PM / QoS: Make it possible to expose device latency tolerance to
    userspace
  ACPI / PM: Attach ACPI power domain only once

 drivers/acpi/device_pm.c      |   8 +
 drivers/acpi/internal.h       |   2 +
 drivers/acpi/scan.c           |  46 ++--
 drivers/base/power/power.h    |   2 +
 drivers/base/power/qos.c      |  37 +++
 drivers/base/power/sysfs.c    |  11 +
 drivers/mfd/Kconfig           |  24 ++
 drivers/mfd/Makefile          |   3 +
 drivers/mfd/intel-lpss-acpi.c |  84 +++++++
 drivers/mfd/intel-lpss-pci.c  | 106 +++++++++
 drivers/mfd/intel-lpss.c      | 523 ++++++++++++++++++++++++++++++++++++++++++
 drivers/mfd/intel-lpss.h      |  62 +++++
 include/linux/pm_qos.h        |   5 +
 13 files changed, 899 insertions(+), 14 deletions(-)
 create mode 100644 drivers/mfd/intel-lpss-acpi.c
 create mode 100644 drivers/mfd/intel-lpss-pci.c
 create mode 100644 drivers/mfd/intel-lpss.c
 create mode 100644 drivers/mfd/intel-lpss.h

-- 
2.1.4

[PATCH v1 1/3] PM / QoS: Make it possible to expose device latency tolerance to userspace

[Andy Shevchenko]

From: Mika Westerberg <mika.westerberg@linux.intel.com>

Typically when a device is created the bus core it belongs to (for example
PCI) does not know if the device supports things like latency tolerance.
This is left to the driver that binds to the device in question. However,
at that time the device has already been created and there is no way to set
its dev->power.set_latency_tolerance anymore.

So follow what has been done for other PM QoS attributes as well and allow
drivers to expose and hide latency tolerance from userspace, if the device
supports it.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/power/power.h |  2 ++
 drivers/base/power/qos.c   | 37 +++++++++++++++++++++++++++++++++++++
 drivers/base/power/sysfs.c | 11 +++++++++++
 include/linux/pm_qos.h     |  5 +++++
 4 files changed, 55 insertions(+)

diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
index b6b8a27..0e62fb2 100644
--- a/drivers/base/power/power.h
+++ b/drivers/base/power/power.h
@@ -33,6 +33,8 @@ extern int pm_qos_sysfs_add_resume_latency(struct device *dev);
 extern void pm_qos_sysfs_remove_resume_latency(struct device *dev);
 extern int pm_qos_sysfs_add_flags(struct device *dev);
 extern void pm_qos_sysfs_remove_flags(struct device *dev);
+extern int pm_qos_sysfs_add_latency_tolerance(struct device *dev);
+extern void pm_qos_sysfs_remove_latency_tolerance(struct device *dev);
 
 #else /* CONFIG_PM */
 
diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index e56d538..7f3646e 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -883,3 +883,40 @@ int dev_pm_qos_update_user_latency_tolerance(struct device *dev, s32 val)
 	mutex_unlock(&dev_pm_qos_mtx);
 	return ret;
 }
+
+/**
+ * dev_pm_qos_expose_latency_tolerance - Expose latency tolerance to userspace
+ * @dev: Device whose latency tolerance to expose
+ */
+int dev_pm_qos_expose_latency_tolerance(struct device *dev)
+{
+	int ret;
+
+	if (!dev->power.set_latency_tolerance)
+		return -EINVAL;
+
+	mutex_lock(&dev_pm_qos_sysfs_mtx);
+	ret = pm_qos_sysfs_add_latency_tolerance(dev);
+	mutex_unlock(&dev_pm_qos_sysfs_mtx);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_qos_expose_latency_tolerance);
+
+/**
+ * dev_pm_qos_hide_latency_tolerance - Hide latency tolerance from userspace
+ * @dev: Device whose latency tolerance to hide
+ */
+void dev_pm_qos_hide_latency_tolerance(struct device *dev)
+{
+	mutex_lock(&dev_pm_qos_sysfs_mtx);
+	pm_qos_sysfs_remove_latency_tolerance(dev);
+	mutex_unlock(&dev_pm_qos_sysfs_mtx);
+
+	/* Remove the request from user space now */
+	pm_runtime_get_sync(dev);
+	dev_pm_qos_update_user_latency_tolerance(dev,
+		PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT);
+	pm_runtime_put(dev);
+}
+EXPORT_SYMBOL_GPL(dev_pm_qos_hide_latency_tolerance);
diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
index d2be3f9..a7b4679 100644
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -738,6 +738,17 @@ void pm_qos_sysfs_remove_flags(struct device *dev)
 	sysfs_unmerge_group(&dev->kobj, &pm_qos_flags_attr_group);
 }
 
+int pm_qos_sysfs_add_latency_tolerance(struct device *dev)
+{
+	return sysfs_merge_group(&dev->kobj,
+				 &pm_qos_latency_tolerance_attr_group);
+}
+
+void pm_qos_sysfs_remove_latency_tolerance(struct device *dev)
+{
+	sysfs_unmerge_group(&dev->kobj, &pm_qos_latency_tolerance_attr_group);
+}
+
 void rpm_sysfs_remove(struct device *dev)
 {
 	sysfs_unmerge_group(&dev->kobj, &pm_runtime_attr_group);
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 7b3ae0c..0f65d36 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -161,6 +161,8 @@ void dev_pm_qos_hide_flags(struct device *dev);
 int dev_pm_qos_update_flags(struct device *dev, s32 mask, bool set);
 s32 dev_pm_qos_get_user_latency_tolerance(struct device *dev);
 int dev_pm_qos_update_user_latency_tolerance(struct device *dev, s32 val);
+int dev_pm_qos_expose_latency_tolerance(struct device *dev);
+void dev_pm_qos_hide_latency_tolerance(struct device *dev);
 
 static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev)
 {
@@ -229,6 +231,9 @@ static inline s32 dev_pm_qos_get_user_latency_tolerance(struct device *dev)
 			{ return PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT; }
 static inline int dev_pm_qos_update_user_latency_tolerance(struct device *dev, s32 val)
 			{ return 0; }
+static inline int dev_pm_qos_expose_latency_tolerance(struct device *dev)
+			{ return 0; }
+static inline void dev_pm_qos_hide_latency_tolerance(struct device *dev) {}
 
 static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev) { return 0; }
 static inline s32 dev_pm_qos_requested_flags(struct device *dev) { return 0; }
-- 
2.1.4

[PATCH v1 2/3] ACPI / PM: Attach ACPI power domain only once

[Andy Shevchenko]

From: Mika Westerberg <mika.westerberg@linux.intel.com>

Some devices, like MFD subdevices, share a single ACPI companion device so
that they are able to access their resources and children. However,
currently all these subdevices are attached to the ACPI power domain and
this might cause that the power methods for the companion device get called
more than once.

In order to solve this we attach the ACPI power domain only to the first
physical device that is bound to the ACPI companion device. In case of MFD
devices, this is the parent MFD device itself.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/device_pm.c |  8 ++++++++
 drivers/acpi/internal.h  |  2 ++
 drivers/acpi/scan.c      | 46 ++++++++++++++++++++++++++++++++--------------
 3 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index 735db11..7d0c7e9 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -1103,6 +1103,14 @@ int acpi_dev_pm_attach(struct device *dev, bool power_on)
 	if (dev->pm_domain)
 		return -EEXIST;
 
+	/*
+	 * Only attach the power domain to the first device if the
+	 * companion is shared by multiple. This is to prevent doing power
+	 * management twice.
+	 */
+	if (!acpi_device_is_first_physical_node(adev, dev))
+		return -EBUSY;
+
 	acpi_add_pm_notifier(adev, dev, acpi_pm_notify_work_func);
 	dev->pm_domain = &acpi_general_pm_domain;
 	if (power_on) {
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 56b321a..2aa8430 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -96,6 +96,8 @@ void acpi_device_add_finalize(struct acpi_device *device);
 void acpi_free_pnp_ids(struct acpi_device_pnp *pnp);
 bool acpi_device_is_present(struct acpi_device *adev);
 bool acpi_device_is_battery(struct acpi_device *adev);
+bool acpi_device_is_first_physical_node(struct acpi_device *adev,
+					const struct device *dev);
 
 /* --------------------------------------------------------------------------
                                   Power Resource
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index bbca783..1beef99 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -188,6 +188,37 @@ static int create_modalias(struct acpi_device *acpi_dev, char *modalias,
 	return len;
 }
 
+/**
+ * acpi_device_is_first_physical_node - Is given dev first physical node
+ * @adev: ACPI companion device
+ * @dev: Physical device to check
+ *
+ * Function checks if given @dev is the first physical devices attached to
+ * the ACPI companion device. This distinction is needed in some cases
+ * where the same companion device is shared between many physical devices.
+ *
+ * Note that the caller have to provide valid @adev pointer.
+ */
+bool acpi_device_is_first_physical_node(struct acpi_device *adev,
+					const struct device *dev)
+{
+	bool ret;
+
+	mutex_lock(&adev->physical_node_lock);
+	if (list_empty(&adev->physical_node_list)) {
+		ret = false;
+	} else {
+		const struct acpi_device_physical_node *node;
+
+		node = list_first_entry(&adev->physical_node_list,
+					struct acpi_device_physical_node, node);
+		ret = node->dev == dev;
+	}
+	mutex_unlock(&adev->physical_node_lock);
+
+	return ret;
+}
+
 /*
  * acpi_companion_match() - Can we match via ACPI companion device
  * @dev: Device in question
@@ -211,7 +242,6 @@ static int create_modalias(struct acpi_device *acpi_dev, char *modalias,
 static bool acpi_companion_match(const struct device *dev)
 {
 	struct acpi_device *adev;
-	bool ret;
 
 	adev = ACPI_COMPANION(dev);
 	if (!adev)
@@ -220,19 +250,7 @@ static bool acpi_companion_match(const struct device *dev)
 	if (list_empty(&adev->pnp.ids))
 		return false;
 
-	mutex_lock(&adev->physical_node_lock);
-	if (list_empty(&adev->physical_node_list)) {
-		ret = false;
-	} else {
-		const struct acpi_device_physical_node *node;
-
-		node = list_first_entry(&adev->physical_node_list,
-					struct acpi_device_physical_node, node);
-		ret = node->dev == dev;
-	}
-	mutex_unlock(&adev->physical_node_lock);
-
-	return ret;
+	return acpi_device_is_first_physical_node(adev, dev);
 }
 
 /*
-- 
2.1.4

[PATCH v1 3/3] mfd: Add support for Intel Sunrisepoint LPSS devices

[Andy Shevchenko]

The new coming Intel platforms such as Skylake will contain Sunrisepoint PCH.
The main difference to the previous platforms is that the LPSS devices are
compound devices where usually main (SPI, HSUART, or I2C) and DMA IPs are
present.

This patch brings the driver for such devices found on Sunrisepoint PCH.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/Kconfig           |  24 ++
 drivers/mfd/Makefile          |   3 +
 drivers/mfd/intel-lpss-acpi.c |  84 +++++++
 drivers/mfd/intel-lpss-pci.c  | 106 +++++++++
 drivers/mfd/intel-lpss.c      | 523 ++++++++++++++++++++++++++++++++++++++++++
 drivers/mfd/intel-lpss.h      |  62 +++++
 6 files changed, 802 insertions(+)
 create mode 100644 drivers/mfd/intel-lpss-acpi.c
 create mode 100644 drivers/mfd/intel-lpss-pci.c
 create mode 100644 drivers/mfd/intel-lpss.c
 create mode 100644 drivers/mfd/intel-lpss.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index d5ad04d..b1a6778 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -325,6 +325,30 @@ config INTEL_SOC_PMIC
 	  thermal, charger and related power management functions
 	  on these systems.
 
+config MFD_INTEL_LPSS
+	tristate "Intel Low Power Subsystem support"
+	depends on X86
+	select COMMON_CLK
+	select MFD_CORE
+	help
+	  This driver provides necessary plumbing for Intel Low Power
+	  Subsystem (LPSS) devices such as I2C, SPI and HS-UART starting
+	  from Intel Sunrisepoint (Intel Skylake PCH) and newer chipsets.
+
+config MFD_INTEL_LPSS_ACPI
+	tristate "Intel Low Power Subsystem support in ACPI mode"
+	depends on MFD_INTEL_LPSS && ACPI
+	help
+	  This driver support Intel Low Power Subsystem devices in ACPI
+	  mode.
+
+config MFD_INTEL_LPSS_PCI
+	tristate "Intel Low Power Subsystem support in PCI mode"
+	depends on MFD_INTEL_LPSS && PCI
+	help
+	  This driver support Intel Low Power Subsystem devices in PCI
+	  mode.
+
 config MFD_INTEL_MSIC
 	bool "Intel MSIC"
 	depends on INTEL_SCU_IPC
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 0e5cfeb..cdf29b9 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -161,6 +161,9 @@ obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
 obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
 obj-$(CONFIG_MFD_AAT2870_CORE)	+= aat2870-core.o
 obj-$(CONFIG_MFD_ATMEL_HLCDC)	+= atmel-hlcdc.o
+obj-$(CONFIG_MFD_INTEL_LPSS)	+= intel-lpss.o
+obj-$(CONFIG_MFD_INTEL_LPSS_PCI)	+= intel-lpss-pci.o
+obj-$(CONFIG_MFD_INTEL_LPSS_ACPI)	+= intel-lpss-acpi.o
 obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
 obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
 obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
diff --git a/drivers/mfd/intel-lpss-acpi.c b/drivers/mfd/intel-lpss-acpi.c
new file mode 100644
index 0000000..0d92d73
--- /dev/null
+++ b/drivers/mfd/intel-lpss-acpi.c
@@ -0,0 +1,84 @@
+/*
+ * Intel LPSS ACPI support.
+ *
+ * Copyright (C) 2015, Intel Corporation
+ *
+ * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+ *          Mika Westerberg <mika.westerberg@linux.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/ioport.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
+#include <linux/platform_device.h>
+
+#include "intel-lpss.h"
+
+static const struct intel_lpss_platform_info spt_info = {
+	.clk_rate = 120000000,
+};
+
+static const struct acpi_device_id intel_lpss_acpi_ids[] = {
+	/* SPT */
+	{ "INT3446", (kernel_ulong_t)&spt_info },
+	{ "INT3447", (kernel_ulong_t)&spt_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, intel_lpss_acpi_ids);
+
+static int intel_lpss_acpi_probe(struct platform_device *pdev)
+{
+	struct intel_lpss_platform_info *info;
+	const struct acpi_device_id *id;
+
+	id = acpi_match_device(intel_lpss_acpi_ids, &pdev->dev);
+	if (!id)
+		return -ENODEV;
+
+	info = devm_kmemdup(&pdev->dev, (void *)id->driver_data, sizeof(*info),
+			    GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	info->mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	info->irq = platform_get_irq(pdev, 0);
+
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
+	return intel_lpss_probe(&pdev->dev, info);
+}
+
+static int intel_lpss_acpi_remove(struct platform_device *pdev)
+{
+	intel_lpss_remove(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+}
+
+static INTEL_LPSS_PM_OPS(intel_lpss_acpi_pm_ops);
+
+static struct platform_driver intel_lpss_acpi_driver = {
+	.probe = intel_lpss_acpi_probe,
+	.remove = intel_lpss_acpi_remove,
+	.driver = {
+		.name = "intel-lpss",
+		.acpi_match_table = intel_lpss_acpi_ids,
+		.pm = &intel_lpss_acpi_pm_ops,
+	},
+};
+
+module_platform_driver(intel_lpss_acpi_driver);
+
+MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
+MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
+MODULE_DESCRIPTION("Intel LPSS ACPI driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
new file mode 100644
index 0000000..fd8d25d
--- /dev/null
+++ b/drivers/mfd/intel-lpss-pci.c
@@ -0,0 +1,106 @@
+/*
+ * Intel LPSS PCI support.
+ *
+ * Copyright (C) 2015, Intel Corporation
+ *
+ * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+ *          Mika Westerberg <mika.westerberg@linux.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/ioport.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
+
+#include "intel-lpss.h"
+
+static int intel_lpss_pci_probe(struct pci_dev *pdev,
+				const struct pci_device_id *id)
+{
+	struct intel_lpss_platform_info *info;
+	int ret;
+
+	ret = pcim_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	info = devm_kmemdup(&pdev->dev, (void *)id->driver_data, sizeof(*info),
+			    GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	info->mem = &pdev->resource[0];
+	info->irq = pdev->irq;
+
+	ret = intel_lpss_probe(&pdev->dev, info);
+	if (ret)
+		return ret;
+
+	pm_runtime_put(&pdev->dev);
+	return 0;
+}
+
+static void intel_lpss_pci_remove(struct pci_dev *pdev)
+{
+	pm_runtime_get_sync(&pdev->dev);
+	intel_lpss_remove(&pdev->dev);
+}
+
+static INTEL_LPSS_PM_OPS(intel_lpss_pci_pm_ops);
+
+static const struct intel_lpss_platform_info spt_info = {
+	.clk_rate = 120000000,
+};
+
+static const struct intel_lpss_platform_info spt_uart_info = {
+	.clk_rate = 120000000,
+	.clk_con_id = "baudclk",
+};
+
+static const struct pci_device_id intel_lpss_pci_ids[] = {
+	/* SPT-LP */
+	{ PCI_VDEVICE(INTEL, 0x9d27), (kernel_ulong_t)&spt_uart_info },
+	{ PCI_VDEVICE(INTEL, 0x9d28), (kernel_ulong_t)&spt_uart_info },
+	{ PCI_VDEVICE(INTEL, 0x9d29), (kernel_ulong_t)&spt_info },
+	{ PCI_VDEVICE(INTEL, 0x9d2a), (kernel_ulong_t)&spt_info },
+	{ PCI_VDEVICE(INTEL, 0x9d60), (kernel_ulong_t)&spt_info },
+	{ PCI_VDEVICE(INTEL, 0x9d61), (kernel_ulong_t)&spt_info },
+	{ PCI_VDEVICE(INTEL, 0x9d62), (kernel_ulong_t)&spt_info },
+	{ PCI_VDEVICE(INTEL, 0x9d63), (kernel_ulong_t)&spt_info },
+	{ PCI_VDEVICE(INTEL, 0x9d64), (kernel_ulong_t)&spt_info },
+	{ PCI_VDEVICE(INTEL, 0x9d65), (kernel_ulong_t)&spt_info },
+	{ PCI_VDEVICE(INTEL, 0x9d66), (kernel_ulong_t)&spt_uart_info },
+	/* SPT-H */
+	{ PCI_VDEVICE(INTEL, 0xa127), (kernel_ulong_t)&spt_uart_info },
+	{ PCI_VDEVICE(INTEL, 0xa128), (kernel_ulong_t)&spt_uart_info },
+	{ PCI_VDEVICE(INTEL, 0xa129), (kernel_ulong_t)&spt_info },
+	{ PCI_VDEVICE(INTEL, 0xa12a), (kernel_ulong_t)&spt_info },
+	{ PCI_VDEVICE(INTEL, 0xa160), (kernel_ulong_t)&spt_info },
+	{ PCI_VDEVICE(INTEL, 0xa161), (kernel_ulong_t)&spt_info },
+	{ PCI_VDEVICE(INTEL, 0xa166), (kernel_ulong_t)&spt_uart_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(pci, intel_lpss_pci_ids);
+
+static struct pci_driver intel_lpss_pci_driver = {
+	.name = "intel-lpss",
+	.id_table = intel_lpss_pci_ids,
+	.probe = intel_lpss_pci_probe,
+	.remove = intel_lpss_pci_remove,
+	.driver = {
+		.pm = &intel_lpss_pci_pm_ops,
+	},
+};
+
+module_pci_driver(intel_lpss_pci_driver);
+
+MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
+MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
+MODULE_DESCRIPTION("Intel LPSS PCI driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
new file mode 100644
index 0000000..78d1921
--- /dev/null
+++ b/drivers/mfd/intel-lpss.c
@@ -0,0 +1,523 @@
+/*
+ * Intel Sunrisepoint LPSS core support.
+ *
+ * Copyright (C) 2015, Intel Corporation
+ *
+ * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+ *          Mika Westerberg <mika.westerberg@linux.intel.com>
+ *          Heikki Krogerus <heikki.krogerus@linux.intel.com>
+ *          Jarkko Nikula <jarkko.nikula@linux.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/clk.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/debugfs.h>
+#include <linux/idr.h>
+#include <linux/ioport.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mfd/core.h>
+#include <linux/platform_data/dma-dw.h>
+#include <linux/pm_qos.h>
+#include <linux/pm_runtime.h>
+#include <linux/seq_file.h>
+
+#include "intel-lpss.h"
+
+#define LPSS_DEV_OFFSET		0x000
+#define LPSS_DEV_SIZE		0x200
+#define LPSS_PRIV_OFFSET	0x200
+#define LPSS_PRIV_SIZE		0x100
+#define LPSS_IDMA_OFFSET	0x800
+#define LPSS_IDMA_SIZE		0x800
+
+/* Offsets from lpss->priv */
+#define LPSS_PRIV_RESETS		0x04
+#define LPSS_PRIV_RESETS_FUNC		BIT(2)
+#define LPSS_PRIV_RESETS_IDMA		0x3
+
+#define LPSS_PRIV_ACTIVELTR		0x10
+#define LPSS_PRIV_IDLELTR		0x14
+
+#define LPSS_PRIV_LTR_REQ		BIT(15)
+#define LPSS_PRIV_LTR_SCALE_MASK	0xc00
+#define LPSS_PRIV_LTR_SCALE_1US		0x800
+#define LPSS_PRIV_LTR_SCALE_32US	0xc00
+#define LPSS_PRIV_LTR_VALUE_MASK	0x3ff
+
+#define LPSS_PRIV_CLK_GATE		0x38
+#define LPSS_PRIV_CLK_GATE_IP_SHIFT	0
+#define LPSS_PRIV_CLK_GATE_IDMA_SHIFT	2
+#define LPSS_PRIV_CLK_GATE_EN_MASK	0x2 /* Dynamic gating disabled */
+
+#define LPSS_PRIV_CAPS			0xfc
+#define LPSS_PRIV_CAPS_NO_IDMA		BIT(8)
+#define LPSS_PRIV_CAPS_TYPE_SHIFT	4
+#define LPSS_PRIV_CAPS_TYPE_MASK	(0xf << LPSS_PRIV_CAPS_TYPE_SHIFT)
+
+/* This matches the type field in CAPS register */
+enum intel_lpss_dev_type {
+	LPSS_DEV_I2C = 0,
+	LPSS_DEV_UART,
+	LPSS_DEV_SPI,
+};
+
+struct intel_lpss {
+	const struct intel_lpss_platform_info *info;
+	enum intel_lpss_dev_type type;
+	struct clk *clk;
+	struct clk_lookup **clocks;
+	unsigned int ndevs;
+	const struct mfd_cell *devs;
+	struct device *dev;
+	void __iomem *priv;
+	int devid;
+	u32 caps;
+	u32 active_ltr;
+	u32 idle_ltr;
+	struct dentry *debugfs;
+};
+
+static const struct dw_dma_platform_data intel_lpss_idma_pdata = {
+	.type = DW_DMAC_TYPE_INTEL,
+	.nr_channels = 2,
+	.is_private = true,
+	.block_size = 0x1ffff,
+	.nr_masters = 1,
+	.data_width = {2},
+};
+
+static const struct resource intel_lpss_dev_resources[] = {
+	DEFINE_RES_MEM_NAMED(LPSS_DEV_OFFSET, LPSS_DEV_SIZE, "lpss_dev"),
+	DEFINE_RES_MEM_NAMED(LPSS_PRIV_OFFSET, LPSS_PRIV_SIZE, "lpss_priv"),
+	DEFINE_RES_IRQ(0),
+};
+
+static const struct resource intel_lpss_idma_resources[] = {
+	DEFINE_RES_MEM(LPSS_IDMA_OFFSET, LPSS_IDMA_SIZE),
+	DEFINE_RES_IRQ(0),
+};
+
+/*
+ * Cells needs to be ordered so that the iDMA is created first. This is
+ * because we need to be sure the DMA is available when the host controller
+ * driver is probed.
+ */
+static const struct mfd_cell intel_lpss_i2c_devs[] = {
+	{
+		.name = "dw_dmac",
+		.num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
+		.resources = intel_lpss_idma_resources,
+		.platform_data = (void *)&intel_lpss_idma_pdata,
+		.pdata_size = sizeof(intel_lpss_idma_pdata),
+	},
+	{
+		.name = "i2c_designware",
+		.num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
+		.resources = intel_lpss_dev_resources,
+	},
+};
+
+static const struct mfd_cell intel_lpss_uart_devs[] = {
+	{
+		.name = "dw_dmac",
+		.num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
+		.resources = intel_lpss_idma_resources,
+		.platform_data = (void *)&intel_lpss_idma_pdata,
+		.pdata_size = sizeof(intel_lpss_idma_pdata),
+	},
+	{
+		.name = "dw-apb-uart",
+		.num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
+		.resources = intel_lpss_dev_resources,
+	},
+};
+
+static const struct mfd_cell intel_lpss_spi_devs[] = {
+	{
+		.name = "dw_dmac",
+		.num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
+		.resources = intel_lpss_idma_resources,
+		.platform_data = (void *)&intel_lpss_idma_pdata,
+		.pdata_size = sizeof(intel_lpss_idma_pdata),
+	},
+	{
+		.name = "pxa2xx-spi",
+		.num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
+		.resources = intel_lpss_dev_resources,
+	},
+};
+
+static DEFINE_IDA(intel_lpss_devid_ida);
+static struct dentry *intel_lpss_debugfs;
+
+static void intel_lpss_cache_ltr(struct intel_lpss *lpss)
+{
+	lpss->active_ltr = readl(lpss->priv + LPSS_PRIV_ACTIVELTR);
+	lpss->idle_ltr = readl(lpss->priv + LPSS_PRIV_IDLELTR);
+}
+
+static int intel_lpss_debugfs_add(struct intel_lpss *lpss)
+{
+	struct dentry *dir;
+
+	dir = debugfs_create_dir(dev_name(lpss->dev), intel_lpss_debugfs);
+	if (IS_ERR(dir))
+		return PTR_ERR(dir);
+
+	/* Cache the values into lpss structure */
+	intel_lpss_cache_ltr(lpss);
+
+	debugfs_create_x32("capabilities", S_IRUGO, dir, &lpss->caps);
+	debugfs_create_x32("active_ltr", S_IRUGO, dir, &lpss->active_ltr);
+	debugfs_create_x32("idle_ltr", S_IRUGO, dir, &lpss->idle_ltr);
+
+	lpss->debugfs = dir;
+	return 0;
+}
+
+static void intel_lpss_debugfs_remove(struct intel_lpss *lpss)
+{
+	debugfs_remove_recursive(lpss->debugfs);
+}
+
+static void intel_lpss_ltr_set(struct device *dev, s32 val)
+{
+	struct intel_lpss *lpss = dev_get_drvdata(dev);
+	u32 ltr;
+
+	ltr = readl(lpss->priv + LPSS_PRIV_ACTIVELTR);
+
+	if (val == PM_QOS_LATENCY_ANY || val < 0) {
+		ltr &= ~LPSS_PRIV_LTR_REQ;
+	} else {
+		ltr |= LPSS_PRIV_LTR_REQ;
+		ltr &= ~LPSS_PRIV_LTR_SCALE_MASK;
+		ltr &= ~LPSS_PRIV_LTR_VALUE_MASK;
+
+		if (val > LPSS_PRIV_LTR_VALUE_MASK)
+			ltr |= LPSS_PRIV_LTR_SCALE_32US | val >> 5;
+		else
+			ltr |= LPSS_PRIV_LTR_SCALE_1US | val;
+	}
+
+	if (ltr == lpss->active_ltr)
+		return;
+
+	writel(ltr, lpss->priv + LPSS_PRIV_ACTIVELTR);
+	writel(ltr, lpss->priv + LPSS_PRIV_IDLELTR);
+
+	/* Cache the values into lpss structure */
+	intel_lpss_cache_ltr(lpss);
+}
+
+static void intel_lpss_ltr_add(struct intel_lpss *lpss)
+{
+	lpss->dev->power.set_latency_tolerance = intel_lpss_ltr_set;
+	dev_pm_qos_expose_latency_tolerance(lpss->dev);
+}
+
+static void intel_lpss_ltr_remove(struct intel_lpss *lpss)
+{
+	dev_pm_qos_hide_latency_tolerance(lpss->dev);
+	lpss->dev->power.set_latency_tolerance = NULL;
+}
+
+static int intel_lpss_assign_devs(struct intel_lpss *lpss)
+{
+	unsigned int type;
+
+	type = lpss->caps & LPSS_PRIV_CAPS_TYPE_MASK;
+	type >>= LPSS_PRIV_CAPS_TYPE_SHIFT;
+
+	switch (type) {
+	case LPSS_DEV_I2C:
+		lpss->devs = intel_lpss_i2c_devs;
+		break;
+	case LPSS_DEV_UART:
+		lpss->devs = intel_lpss_uart_devs;
+		break;
+	case LPSS_DEV_SPI:
+		lpss->devs = intel_lpss_spi_devs;
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	lpss->type = type;
+
+	return 0;
+}
+
+static bool intel_lpss_has_idma(const struct intel_lpss *lpss)
+{
+	return (lpss->caps & LPSS_PRIV_CAPS_NO_IDMA) == 0;
+}
+
+static void intel_lpss_reset(const struct intel_lpss *lpss)
+{
+	u32 value = LPSS_PRIV_RESETS_FUNC | LPSS_PRIV_RESETS_IDMA;
+
+	/* Bring out the device from reset */
+	writel(value, lpss->priv + LPSS_PRIV_RESETS);
+}
+
+static void intel_lpss_unregister_clock_tree(struct clk *clk)
+{
+	struct clk *parent;
+
+	while (clk) {
+		parent = clk_get_parent(clk);
+		clk_unregister(clk);
+		clk = parent;
+	}
+}
+
+static int intel_lpss_register_clock_divider(struct intel_lpss *lpss,
+					     const char *devname,
+					     struct clk **clk)
+{
+	char name[32];
+	struct clk *tmp = *clk;
+
+	snprintf(name, sizeof(name), "%s-enable", devname);
+	tmp = clk_register_gate(NULL, name, __clk_get_name(tmp), 0,
+				lpss->priv, 0, 0, NULL);
+	if (IS_ERR(tmp))
+		return PTR_ERR(tmp);
+
+	snprintf(name, sizeof(name), "%s-div", devname);
+	tmp = clk_register_fractional_divider(NULL, name, __clk_get_name(tmp),
+					      0, lpss->priv, 1, 15, 16, 15, 0,
+					      NULL);
+	if (IS_ERR(tmp))
+		return PTR_ERR(tmp);
+	*clk = tmp;
+
+	snprintf(name, sizeof(name), "%s-update", devname);
+	tmp = clk_register_gate(NULL, name, __clk_get_name(tmp),
+				CLK_SET_RATE_PARENT, lpss->priv, 31, 0, NULL);
+	if (IS_ERR(tmp))
+		return PTR_ERR(tmp);
+	*clk = tmp;
+
+	return 0;
+}
+
+static int intel_lpss_register_clock(struct intel_lpss *lpss)
+{
+	const struct mfd_cell *devs = lpss->devs;
+	struct clk_lookup **clocks;
+	struct clk *clk, *root;
+	char devname[24];
+	int ret = -ENOMEM;
+
+	if (!lpss->info->clk_rate)
+		return 0;
+
+	clocks = devm_kcalloc(lpss->dev, lpss->ndevs, sizeof(*clocks),
+			      GFP_KERNEL);
+	if (!clocks)
+		return -ENOMEM;
+
+	/* Root clock */
+	root = clk_register_fixed_rate(NULL, dev_name(lpss->dev), NULL,
+				       CLK_IS_ROOT, lpss->info->clk_rate);
+	if (IS_ERR(root))
+		return PTR_ERR(root);
+
+	clk = root;
+
+	snprintf(devname, sizeof(devname), "%s.%d", devs[1].name, lpss->devid);
+
+	/*
+	 * Support for clock divider only if it has some preset value.
+	 * Otherwise we assume that the divider is not used.
+	 */
+	if (lpss->type != LPSS_DEV_I2C) {
+		ret = intel_lpss_register_clock_divider(lpss, devname, &clk);
+		if (ret)
+			goto err_clk_register;
+	}
+
+	/* Clock for the host controller */
+	clocks[0] = clk_register_clkdev(clk, lpss->info->clk_con_id, "%s",
+					devname);
+	if (IS_ERR(clocks[0]))
+		goto err_clk_register;
+
+	/*
+	 * If the slice has integrated DMA block create clock for that as
+	 * well now.
+	 */
+	if (lpss->ndevs > 1) {
+		clocks[1] = clk_register_clkdev(root, "hclk", "%s.%d",
+						devs[0].name, lpss->devid);
+		if (IS_ERR(clocks[1]))
+			goto err_hclk_register;
+	}
+
+	lpss->clocks = clocks;
+	lpss->clk = clk;
+
+	return 0;
+
+err_hclk_register:
+	clkdev_drop(clocks[0]);
+
+err_clk_register:
+	intel_lpss_unregister_clock_tree(clk);
+
+	return ret;
+}
+
+static void intel_lpss_unregister_clock(struct intel_lpss *lpss)
+{
+	unsigned int i;
+
+	if (IS_ERR_OR_NULL(lpss->clk))
+		return;
+
+	for (i = 0; i < lpss->ndevs; i++)
+		clkdev_drop(lpss->clocks[i]);
+
+	intel_lpss_unregister_clock_tree(lpss->clk);
+}
+
+int intel_lpss_probe(struct device *dev,
+		     const struct intel_lpss_platform_info *info)
+{
+	const struct mfd_cell *devs;
+	struct intel_lpss *lpss;
+	struct resource priv;
+	int ret;
+
+	if (!info || !info->mem || info->irq <= 0)
+		return -EINVAL;
+
+	lpss = devm_kzalloc(dev, sizeof(*lpss), GFP_KERNEL);
+	if (!lpss)
+		return -ENOMEM;
+
+	memset(&priv, 0, sizeof(priv));
+	priv.start = info->mem->start + LPSS_PRIV_OFFSET;
+	priv.end = priv.start + LPSS_PRIV_SIZE - 1;
+	priv.flags = IORESOURCE_MEM;
+
+	lpss->priv = devm_ioremap_resource(dev, &priv);
+	if (!lpss->priv)
+		return -ENOMEM;
+
+	intel_lpss_reset(lpss);
+
+	lpss->info = info;
+	lpss->dev = dev;
+	lpss->caps = readl(lpss->priv + LPSS_PRIV_CAPS);
+	lpss->ndevs = intel_lpss_has_idma(lpss) ? 2 : 1;
+
+	dev_set_drvdata(dev, lpss);
+
+	ret = intel_lpss_assign_devs(lpss);
+	if (ret < 0)
+		return ret;
+
+	lpss->devid = ida_simple_get(&intel_lpss_devid_ida, 0, 0, GFP_KERNEL);
+	if (lpss->devid < 0)
+		return lpss->devid;
+
+	ret = intel_lpss_register_clock(lpss);
+	if (ret < 0)
+		goto err_clk_register;
+
+	intel_lpss_ltr_add(lpss);
+
+	ret = intel_lpss_debugfs_add(lpss);
+	if (ret)
+		dev_warn(lpss->dev, "Failed to create debugfs entries\n");
+
+	devs = intel_lpss_has_idma(lpss) ? lpss->devs : lpss->devs + 1;
+	ret = mfd_add_devices(dev, lpss->devid, devs, lpss->ndevs,
+			      info->mem, info->irq, NULL);
+	if (ret < 0)
+		goto err_remove_ltr;
+
+	return 0;
+
+err_remove_ltr:
+	intel_lpss_debugfs_remove(lpss);
+	intel_lpss_ltr_remove(lpss);
+
+err_clk_register:
+	ida_simple_remove(&intel_lpss_devid_ida, lpss->devid);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(intel_lpss_probe);
+
+void intel_lpss_remove(struct device *dev)
+{
+	struct intel_lpss *lpss = dev_get_drvdata(dev);
+
+	mfd_remove_devices(dev);
+	intel_lpss_debugfs_remove(lpss);
+	intel_lpss_ltr_remove(lpss);
+	intel_lpss_unregister_clock(lpss);
+	ida_simple_remove(&intel_lpss_devid_ida, lpss->devid);
+}
+EXPORT_SYMBOL_GPL(intel_lpss_remove);
+
+static int resume_lpss_device(struct device *dev, void *data)
+{
+	pm_runtime_resume(dev);
+	return 0;
+}
+
+int intel_lpss_prepare(struct device *dev)
+{
+	/*
+	 * Resume both child devices before entering system sleep. This
+	 * ensures that they are in proper state before they get suspended.
+	 */
+	device_for_each_child(dev, NULL, resume_lpss_device);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(intel_lpss_prepare);
+
+int intel_lpss_suspend(struct device *dev)
+{
+	return 0;
+}
+EXPORT_SYMBOL_GPL(intel_lpss_suspend);
+
+int intel_lpss_resume(struct device *dev)
+{
+	struct intel_lpss *lpss = dev_get_drvdata(dev);
+	intel_lpss_reset(lpss);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(intel_lpss_resume);
+
+static int __init intel_lpss_init(void)
+{
+	intel_lpss_debugfs = debugfs_create_dir("intel_lpss", NULL);
+	return 0;
+}
+module_init(intel_lpss_init);
+
+static void __exit intel_lpss_exit(void)
+{
+	debugfs_remove(intel_lpss_debugfs);
+}
+module_exit(intel_lpss_exit);
+
+MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
+MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
+MODULE_AUTHOR("Heikki Krogerus <heikki.krogerus@linux.intel.com>");
+MODULE_AUTHOR("Jarkko Nikula <jarkko.nikula@linux.intel.com>");
+MODULE_DESCRIPTION("Intel LPSS core driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
new file mode 100644
index 0000000..f28cb28
--- /dev/null
+++ b/drivers/mfd/intel-lpss.h
@@ -0,0 +1,62 @@
+/*
+ * Intel LPSS core support.
+ *
+ * Copyright (C) 2015, Intel Corporation
+ *
+ * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+ *          Mika Westerberg <mika.westerberg@linux.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 __MFD_INTEL_LPSS_H
+#define __MFD_INTEL_LPSS_H
+
+struct device;
+struct resource;
+
+struct intel_lpss_platform_info {
+	struct resource *mem;
+	int irq;
+	unsigned long clk_rate;
+	const char *clk_con_id;
+};
+
+int intel_lpss_probe(struct device *dev,
+		     const struct intel_lpss_platform_info *info);
+void intel_lpss_remove(struct device *dev);
+
+#ifdef CONFIG_PM
+int intel_lpss_prepare(struct device *dev);
+int intel_lpss_suspend(struct device *dev);
+int intel_lpss_resume(struct device *dev);
+
+#ifdef CONFIG_PM_SLEEP
+#define INTEL_LPSS_SLEEP_PM_OPS			\
+	.prepare = intel_lpss_prepare,		\
+	.suspend = intel_lpss_suspend,		\
+	.resume = intel_lpss_resume,		\
+	.freeze = intel_lpss_suspend,		\
+	.thaw = intel_lpss_resume,		\
+	.poweroff = intel_lpss_suspend,		\
+	.restore = intel_lpss_resume,
+#endif
+
+#define INTEL_LPSS_RUNTIME_PM_OPS		\
+	.runtime_suspend = intel_lpss_suspend,	\
+	.runtime_resume = intel_lpss_resume,
+
+#else /* !CONFIG_PM */
+#define INTEL_LPSS_SLEEP_PM_OPS
+#define INTEL_LPSS_RUNTIME_PM_OPS
+#endif /* CONFIG_PM */
+
+#define INTEL_LPSS_PM_OPS(name)			\
+const struct dev_pm_ops name = {		\
+	INTEL_LPSS_SLEEP_PM_OPS			\
+	INTEL_LPSS_RUNTIME_PM_OPS		\
+}
+
+#endif /* __MFD_INTEL_LPSS_H */
-- 
2.1.4

Re: [PATCH v1 3/3] mfd: Add support for Intel Sunrisepoint LPSS devices

[Lee Jones]

On Tue, 31 Mar 2015, Andy Shevchenko wrote:

> The new coming Intel platforms such as Skylake will contain Sunrisepoint PCH.
> The main difference to the previous platforms is that the LPSS devices are
> compound devices where usually main (SPI, HSUART, or I2C) and DMA IPs are
> present.
> 
> This patch brings the driver for such devices found on Sunrisepoint PCH.

I'm not convinced it's really an MFD.  What does this hardware look
like?  Are the Designware devices really in the same memory/register
space as the LPSS registers?

> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/Kconfig           |  24 ++
>  drivers/mfd/Makefile          |   3 +
>  drivers/mfd/intel-lpss-acpi.c |  84 +++++++
>  drivers/mfd/intel-lpss-pci.c  | 106 +++++++++
>  drivers/mfd/intel-lpss.c      | 523 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/mfd/intel-lpss.h      |  62 +++++

I'd prefer to see this in /include

>  6 files changed, 802 insertions(+)
>  create mode 100644 drivers/mfd/intel-lpss-acpi.c
>  create mode 100644 drivers/mfd/intel-lpss-pci.c
>  create mode 100644 drivers/mfd/intel-lpss.c
>  create mode 100644 drivers/mfd/intel-lpss.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index d5ad04d..b1a6778 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -325,6 +325,30 @@ config INTEL_SOC_PMIC
>  	  thermal, charger and related power management functions
>  	  on these systems.
>  
> +config MFD_INTEL_LPSS
> +	tristate "Intel Low Power Subsystem support"
> +	depends on X86
> +	select COMMON_CLK
> +	select MFD_CORE
> +	help
> +	  This driver provides necessary plumbing for Intel Low Power
> +	  Subsystem (LPSS) devices such as I2C, SPI and HS-UART starting
> +	  from Intel Sunrisepoint (Intel Skylake PCH) and newer chipsets.
> +
> +config MFD_INTEL_LPSS_ACPI
> +	tristate "Intel Low Power Subsystem support in ACPI mode"
> +	depends on MFD_INTEL_LPSS && ACPI
> +	help
> +	  This driver support Intel Low Power Subsystem devices in ACPI
> +	  mode.
> +
> +config MFD_INTEL_LPSS_PCI
> +	tristate "Intel Low Power Subsystem support in PCI mode"
> +	depends on MFD_INTEL_LPSS && PCI
> +	help
> +	  This driver support Intel Low Power Subsystem devices in PCI
> +	  mode.
> +
>  config MFD_INTEL_MSIC
>  	bool "Intel MSIC"
>  	depends on INTEL_SCU_IPC
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 0e5cfeb..cdf29b9 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -161,6 +161,9 @@ obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
>  obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
>  obj-$(CONFIG_MFD_AAT2870_CORE)	+= aat2870-core.o
>  obj-$(CONFIG_MFD_ATMEL_HLCDC)	+= atmel-hlcdc.o
> +obj-$(CONFIG_MFD_INTEL_LPSS)	+= intel-lpss.o
> +obj-$(CONFIG_MFD_INTEL_LPSS_PCI)	+= intel-lpss-pci.o
> +obj-$(CONFIG_MFD_INTEL_LPSS_ACPI)	+= intel-lpss-acpi.o
>  obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
>  obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
>  obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
> diff --git a/drivers/mfd/intel-lpss-acpi.c b/drivers/mfd/intel-lpss-acpi.c
> new file mode 100644
> index 0000000..0d92d73
> --- /dev/null
> +++ b/drivers/mfd/intel-lpss-acpi.c
> @@ -0,0 +1,84 @@
> +/*
> + * Intel LPSS ACPI support.
> + *
> + * Copyright (C) 2015, Intel Corporation
> + *
> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> + *          Mika Westerberg <mika.westerberg@linux.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/ioport.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pm.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/platform_device.h>
> +
> +#include "intel-lpss.h"
> +
> +static const struct intel_lpss_platform_info spt_info = {
> +	.clk_rate = 120000000,
> +};
> +
> +static const struct acpi_device_id intel_lpss_acpi_ids[] = {
> +	/* SPT */
> +	{ "INT3446", (kernel_ulong_t)&spt_info },
> +	{ "INT3447", (kernel_ulong_t)&spt_info },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, intel_lpss_acpi_ids);
> +
> +static int intel_lpss_acpi_probe(struct platform_device *pdev)
> +{
> +	struct intel_lpss_platform_info *info;
> +	const struct acpi_device_id *id;
> +
> +	id = acpi_match_device(intel_lpss_acpi_ids, &pdev->dev);
> +	if (!id)
> +		return -ENODEV;
> +
> +	info = devm_kmemdup(&pdev->dev, (void *)id->driver_data, sizeof(*info),
> +			    GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	info->mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	info->irq = platform_get_irq(pdev, 0);
> +
> +	pm_runtime_set_active(&pdev->dev);
> +	pm_runtime_enable(&pdev->dev);
> +
> +	return intel_lpss_probe(&pdev->dev, info);
> +}
> +
> +static int intel_lpss_acpi_remove(struct platform_device *pdev)
> +{
> +	intel_lpss_remove(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static INTEL_LPSS_PM_OPS(intel_lpss_acpi_pm_ops);
> +
> +static struct platform_driver intel_lpss_acpi_driver = {
> +	.probe = intel_lpss_acpi_probe,
> +	.remove = intel_lpss_acpi_remove,
> +	.driver = {
> +		.name = "intel-lpss",
> +		.acpi_match_table = intel_lpss_acpi_ids,
> +		.pm = &intel_lpss_acpi_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(intel_lpss_acpi_driver);
> +
> +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
> +MODULE_DESCRIPTION("Intel LPSS ACPI driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
> new file mode 100644
> index 0000000..fd8d25d
> --- /dev/null
> +++ b/drivers/mfd/intel-lpss-pci.c
> @@ -0,0 +1,106 @@
> +/*
> + * Intel LPSS PCI support.
> + *
> + * Copyright (C) 2015, Intel Corporation
> + *
> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> + *          Mika Westerberg <mika.westerberg@linux.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/ioport.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/pm.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "intel-lpss.h"
> +
> +static int intel_lpss_pci_probe(struct pci_dev *pdev,
> +				const struct pci_device_id *id)
> +{
> +	struct intel_lpss_platform_info *info;
> +	int ret;
> +
> +	ret = pcim_enable_device(pdev);
> +	if (ret)
> +		return ret;
> +
> +	info = devm_kmemdup(&pdev->dev, (void *)id->driver_data, sizeof(*info),
> +			    GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	info->mem = &pdev->resource[0];
> +	info->irq = pdev->irq;
> +
> +	ret = intel_lpss_probe(&pdev->dev, info);
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_put(&pdev->dev);
> +	return 0;
> +}
> +
> +static void intel_lpss_pci_remove(struct pci_dev *pdev)
> +{
> +	pm_runtime_get_sync(&pdev->dev);
> +	intel_lpss_remove(&pdev->dev);
> +}
> +
> +static INTEL_LPSS_PM_OPS(intel_lpss_pci_pm_ops);
> +
> +static const struct intel_lpss_platform_info spt_info = {
> +	.clk_rate = 120000000,
> +};
> +
> +static const struct intel_lpss_platform_info spt_uart_info = {
> +	.clk_rate = 120000000,
> +	.clk_con_id = "baudclk",
> +};
> +
> +static const struct pci_device_id intel_lpss_pci_ids[] = {
> +	/* SPT-LP */
> +	{ PCI_VDEVICE(INTEL, 0x9d27), (kernel_ulong_t)&spt_uart_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d28), (kernel_ulong_t)&spt_uart_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d29), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d2a), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d60), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d61), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d62), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d63), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d64), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d65), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d66), (kernel_ulong_t)&spt_uart_info },
> +	/* SPT-H */
> +	{ PCI_VDEVICE(INTEL, 0xa127), (kernel_ulong_t)&spt_uart_info },
> +	{ PCI_VDEVICE(INTEL, 0xa128), (kernel_ulong_t)&spt_uart_info },
> +	{ PCI_VDEVICE(INTEL, 0xa129), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0xa12a), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0xa160), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0xa161), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0xa166), (kernel_ulong_t)&spt_uart_info },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(pci, intel_lpss_pci_ids);
> +
> +static struct pci_driver intel_lpss_pci_driver = {
> +	.name = "intel-lpss",
> +	.id_table = intel_lpss_pci_ids,
> +	.probe = intel_lpss_pci_probe,
> +	.remove = intel_lpss_pci_remove,
> +	.driver = {
> +		.pm = &intel_lpss_pci_pm_ops,
> +	},
> +};
> +
> +module_pci_driver(intel_lpss_pci_driver);
> +
> +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
> +MODULE_DESCRIPTION("Intel LPSS PCI driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> new file mode 100644
> index 0000000..78d1921
> --- /dev/null
> +++ b/drivers/mfd/intel-lpss.c
> @@ -0,0 +1,523 @@
> +/*
> + * Intel Sunrisepoint LPSS core support.
> + *
> + * Copyright (C) 2015, Intel Corporation
> + *
> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> + *          Mika Westerberg <mika.westerberg@linux.intel.com>
> + *          Heikki Krogerus <heikki.krogerus@linux.intel.com>
> + *          Jarkko Nikula <jarkko.nikula@linux.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/clk.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/debugfs.h>
> +#include <linux/idr.h>
> +#include <linux/ioport.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mfd/core.h>
> +#include <linux/platform_data/dma-dw.h>
> +#include <linux/pm_qos.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/seq_file.h>
> +
> +#include "intel-lpss.h"
> +
> +#define LPSS_DEV_OFFSET		0x000
> +#define LPSS_DEV_SIZE		0x200
> +#define LPSS_PRIV_OFFSET	0x200
> +#define LPSS_PRIV_SIZE		0x100
> +#define LPSS_IDMA_OFFSET	0x800
> +#define LPSS_IDMA_SIZE		0x800
> +
> +/* Offsets from lpss->priv */
> +#define LPSS_PRIV_RESETS		0x04
> +#define LPSS_PRIV_RESETS_FUNC		BIT(2)
> +#define LPSS_PRIV_RESETS_IDMA		0x3
> +
> +#define LPSS_PRIV_ACTIVELTR		0x10
> +#define LPSS_PRIV_IDLELTR		0x14
> +
> +#define LPSS_PRIV_LTR_REQ		BIT(15)
> +#define LPSS_PRIV_LTR_SCALE_MASK	0xc00
> +#define LPSS_PRIV_LTR_SCALE_1US		0x800
> +#define LPSS_PRIV_LTR_SCALE_32US	0xc00
> +#define LPSS_PRIV_LTR_VALUE_MASK	0x3ff
> +
> +#define LPSS_PRIV_CLK_GATE		0x38
> +#define LPSS_PRIV_CLK_GATE_IP_SHIFT	0
> +#define LPSS_PRIV_CLK_GATE_IDMA_SHIFT	2
> +#define LPSS_PRIV_CLK_GATE_EN_MASK	0x2 /* Dynamic gating disabled */
> +
> +#define LPSS_PRIV_CAPS			0xfc
> +#define LPSS_PRIV_CAPS_NO_IDMA		BIT(8)
> +#define LPSS_PRIV_CAPS_TYPE_SHIFT	4
> +#define LPSS_PRIV_CAPS_TYPE_MASK	(0xf << LPSS_PRIV_CAPS_TYPE_SHIFT)
> +
> +/* This matches the type field in CAPS register */
> +enum intel_lpss_dev_type {
> +	LPSS_DEV_I2C = 0,
> +	LPSS_DEV_UART,
> +	LPSS_DEV_SPI,
> +};
> +
> +struct intel_lpss {
> +	const struct intel_lpss_platform_info *info;
> +	enum intel_lpss_dev_type type;
> +	struct clk *clk;
> +	struct clk_lookup **clocks;
> +	unsigned int ndevs;
> +	const struct mfd_cell *devs;
> +	struct device *dev;
> +	void __iomem *priv;
> +	int devid;
> +	u32 caps;
> +	u32 active_ltr;
> +	u32 idle_ltr;
> +	struct dentry *debugfs;
> +};
> +
> +static const struct dw_dma_platform_data intel_lpss_idma_pdata = {
> +	.type = DW_DMAC_TYPE_INTEL,
> +	.nr_channels = 2,
> +	.is_private = true,
> +	.block_size = 0x1ffff,
> +	.nr_masters = 1,
> +	.data_width = {2},
> +};
> +
> +static const struct resource intel_lpss_dev_resources[] = {
> +	DEFINE_RES_MEM_NAMED(LPSS_DEV_OFFSET, LPSS_DEV_SIZE, "lpss_dev"),
> +	DEFINE_RES_MEM_NAMED(LPSS_PRIV_OFFSET, LPSS_PRIV_SIZE, "lpss_priv"),
> +	DEFINE_RES_IRQ(0),
> +};
> +
> +static const struct resource intel_lpss_idma_resources[] = {
> +	DEFINE_RES_MEM(LPSS_IDMA_OFFSET, LPSS_IDMA_SIZE),
> +	DEFINE_RES_IRQ(0),
> +};
> +
> +/*
> + * Cells needs to be ordered so that the iDMA is created first. This is
> + * because we need to be sure the DMA is available when the host controller
> + * driver is probed.
> + */
> +static const struct mfd_cell intel_lpss_i2c_devs[] = {
> +	{
> +		.name = "dw_dmac",
> +		.num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
> +		.resources = intel_lpss_idma_resources,
> +		.platform_data = (void *)&intel_lpss_idma_pdata,
> +		.pdata_size = sizeof(intel_lpss_idma_pdata),
> +	},
> +	{
> +		.name = "i2c_designware",
> +		.num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
> +		.resources = intel_lpss_dev_resources,
> +	},
> +};
> +
> +static const struct mfd_cell intel_lpss_uart_devs[] = {
> +	{
> +		.name = "dw_dmac",
> +		.num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
> +		.resources = intel_lpss_idma_resources,
> +		.platform_data = (void *)&intel_lpss_idma_pdata,
> +		.pdata_size = sizeof(intel_lpss_idma_pdata),
> +	},
> +	{
> +		.name = "dw-apb-uart",
> +		.num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
> +		.resources = intel_lpss_dev_resources,
> +	},
> +};
> +
> +static const struct mfd_cell intel_lpss_spi_devs[] = {
> +	{
> +		.name = "dw_dmac",
> +		.num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
> +		.resources = intel_lpss_idma_resources,
> +		.platform_data = (void *)&intel_lpss_idma_pdata,
> +		.pdata_size = sizeof(intel_lpss_idma_pdata),
> +	},
> +	{
> +		.name = "pxa2xx-spi",
> +		.num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
> +		.resources = intel_lpss_dev_resources,
> +	},
> +};
> +
> +static DEFINE_IDA(intel_lpss_devid_ida);
> +static struct dentry *intel_lpss_debugfs;
> +
> +static void intel_lpss_cache_ltr(struct intel_lpss *lpss)
> +{
> +	lpss->active_ltr = readl(lpss->priv + LPSS_PRIV_ACTIVELTR);
> +	lpss->idle_ltr = readl(lpss->priv + LPSS_PRIV_IDLELTR);
> +}
> +
> +static int intel_lpss_debugfs_add(struct intel_lpss *lpss)
> +{
> +	struct dentry *dir;
> +
> +	dir = debugfs_create_dir(dev_name(lpss->dev), intel_lpss_debugfs);
> +	if (IS_ERR(dir))
> +		return PTR_ERR(dir);
> +
> +	/* Cache the values into lpss structure */
> +	intel_lpss_cache_ltr(lpss);
> +
> +	debugfs_create_x32("capabilities", S_IRUGO, dir, &lpss->caps);
> +	debugfs_create_x32("active_ltr", S_IRUGO, dir, &lpss->active_ltr);
> +	debugfs_create_x32("idle_ltr", S_IRUGO, dir, &lpss->idle_ltr);
> +
> +	lpss->debugfs = dir;
> +	return 0;
> +}
> +
> +static void intel_lpss_debugfs_remove(struct intel_lpss *lpss)
> +{
> +	debugfs_remove_recursive(lpss->debugfs);
> +}
> +
> +static void intel_lpss_ltr_set(struct device *dev, s32 val)
> +{
> +	struct intel_lpss *lpss = dev_get_drvdata(dev);
> +	u32 ltr;
> +
> +	ltr = readl(lpss->priv + LPSS_PRIV_ACTIVELTR);
> +
> +	if (val == PM_QOS_LATENCY_ANY || val < 0) {
> +		ltr &= ~LPSS_PRIV_LTR_REQ;
> +	} else {
> +		ltr |= LPSS_PRIV_LTR_REQ;
> +		ltr &= ~LPSS_PRIV_LTR_SCALE_MASK;
> +		ltr &= ~LPSS_PRIV_LTR_VALUE_MASK;
> +
> +		if (val > LPSS_PRIV_LTR_VALUE_MASK)
> +			ltr |= LPSS_PRIV_LTR_SCALE_32US | val >> 5;
> +		else
> +			ltr |= LPSS_PRIV_LTR_SCALE_1US | val;
> +	}
> +
> +	if (ltr == lpss->active_ltr)
> +		return;
> +
> +	writel(ltr, lpss->priv + LPSS_PRIV_ACTIVELTR);
> +	writel(ltr, lpss->priv + LPSS_PRIV_IDLELTR);
> +
> +	/* Cache the values into lpss structure */
> +	intel_lpss_cache_ltr(lpss);
> +}
> +
> +static void intel_lpss_ltr_add(struct intel_lpss *lpss)
> +{
> +	lpss->dev->power.set_latency_tolerance = intel_lpss_ltr_set;
> +	dev_pm_qos_expose_latency_tolerance(lpss->dev);
> +}
> +
> +static void intel_lpss_ltr_remove(struct intel_lpss *lpss)
> +{
> +	dev_pm_qos_hide_latency_tolerance(lpss->dev);
> +	lpss->dev->power.set_latency_tolerance = NULL;
> +}
> +
> +static int intel_lpss_assign_devs(struct intel_lpss *lpss)
> +{
> +	unsigned int type;
> +
> +	type = lpss->caps & LPSS_PRIV_CAPS_TYPE_MASK;
> +	type >>= LPSS_PRIV_CAPS_TYPE_SHIFT;
> +
> +	switch (type) {
> +	case LPSS_DEV_I2C:
> +		lpss->devs = intel_lpss_i2c_devs;
> +		break;
> +	case LPSS_DEV_UART:
> +		lpss->devs = intel_lpss_uart_devs;
> +		break;
> +	case LPSS_DEV_SPI:
> +		lpss->devs = intel_lpss_spi_devs;
> +		break;
> +	default:
> +		return -ENODEV;
> +	}
> +
> +	lpss->type = type;
> +
> +	return 0;
> +}
> +
> +static bool intel_lpss_has_idma(const struct intel_lpss *lpss)
> +{
> +	return (lpss->caps & LPSS_PRIV_CAPS_NO_IDMA) == 0;
> +}
> +
> +static void intel_lpss_reset(const struct intel_lpss *lpss)
> +{
> +	u32 value = LPSS_PRIV_RESETS_FUNC | LPSS_PRIV_RESETS_IDMA;
> +
> +	/* Bring out the device from reset */
> +	writel(value, lpss->priv + LPSS_PRIV_RESETS);
> +}
> +
> +static void intel_lpss_unregister_clock_tree(struct clk *clk)
> +{
> +	struct clk *parent;
> +
> +	while (clk) {
> +		parent = clk_get_parent(clk);
> +		clk_unregister(clk);
> +		clk = parent;
> +	}
> +}
> +
> +static int intel_lpss_register_clock_divider(struct intel_lpss *lpss,
> +					     const char *devname,
> +					     struct clk **clk)
> +{
> +	char name[32];
> +	struct clk *tmp = *clk;
> +
> +	snprintf(name, sizeof(name), "%s-enable", devname);
> +	tmp = clk_register_gate(NULL, name, __clk_get_name(tmp), 0,
> +				lpss->priv, 0, 0, NULL);
> +	if (IS_ERR(tmp))
> +		return PTR_ERR(tmp);
> +
> +	snprintf(name, sizeof(name), "%s-div", devname);
> +	tmp = clk_register_fractional_divider(NULL, name, __clk_get_name(tmp),
> +					      0, lpss->priv, 1, 15, 16, 15, 0,
> +					      NULL);
> +	if (IS_ERR(tmp))
> +		return PTR_ERR(tmp);
> +	*clk = tmp;
> +
> +	snprintf(name, sizeof(name), "%s-update", devname);
> +	tmp = clk_register_gate(NULL, name, __clk_get_name(tmp),
> +				CLK_SET_RATE_PARENT, lpss->priv, 31, 0, NULL);
> +	if (IS_ERR(tmp))
> +		return PTR_ERR(tmp);
> +	*clk = tmp;
> +
> +	return 0;
> +}
> +
> +static int intel_lpss_register_clock(struct intel_lpss *lpss)
> +{
> +	const struct mfd_cell *devs = lpss->devs;
> +	struct clk_lookup **clocks;
> +	struct clk *clk, *root;
> +	char devname[24];
> +	int ret = -ENOMEM;
> +
> +	if (!lpss->info->clk_rate)
> +		return 0;
> +
> +	clocks = devm_kcalloc(lpss->dev, lpss->ndevs, sizeof(*clocks),
> +			      GFP_KERNEL);
> +	if (!clocks)
> +		return -ENOMEM;
> +
> +	/* Root clock */
> +	root = clk_register_fixed_rate(NULL, dev_name(lpss->dev), NULL,
> +				       CLK_IS_ROOT, lpss->info->clk_rate);
> +	if (IS_ERR(root))
> +		return PTR_ERR(root);
> +
> +	clk = root;
> +
> +	snprintf(devname, sizeof(devname), "%s.%d", devs[1].name, lpss->devid);
> +
> +	/*
> +	 * Support for clock divider only if it has some preset value.
> +	 * Otherwise we assume that the divider is not used.
> +	 */
> +	if (lpss->type != LPSS_DEV_I2C) {
> +		ret = intel_lpss_register_clock_divider(lpss, devname, &clk);
> +		if (ret)
> +			goto err_clk_register;
> +	}
> +
> +	/* Clock for the host controller */
> +	clocks[0] = clk_register_clkdev(clk, lpss->info->clk_con_id, "%s",
> +					devname);
> +	if (IS_ERR(clocks[0]))
> +		goto err_clk_register;
> +
> +	/*
> +	 * If the slice has integrated DMA block create clock for that as
> +	 * well now.
> +	 */
> +	if (lpss->ndevs > 1) {
> +		clocks[1] = clk_register_clkdev(root, "hclk", "%s.%d",
> +						devs[0].name, lpss->devid);
> +		if (IS_ERR(clocks[1]))
> +			goto err_hclk_register;
> +	}
> +
> +	lpss->clocks = clocks;
> +	lpss->clk = clk;
> +
> +	return 0;
> +
> +err_hclk_register:
> +	clkdev_drop(clocks[0]);
> +
> +err_clk_register:
> +	intel_lpss_unregister_clock_tree(clk);
> +
> +	return ret;
> +}
> +
> +static void intel_lpss_unregister_clock(struct intel_lpss *lpss)
> +{
> +	unsigned int i;
> +
> +	if (IS_ERR_OR_NULL(lpss->clk))
> +		return;
> +
> +	for (i = 0; i < lpss->ndevs; i++)
> +		clkdev_drop(lpss->clocks[i]);
> +
> +	intel_lpss_unregister_clock_tree(lpss->clk);
> +}

Any reason all this clock stuff isn't in drivers/clock?

> +int intel_lpss_probe(struct device *dev,
> +		     const struct intel_lpss_platform_info *info)
> +{
> +	const struct mfd_cell *devs;
> +	struct intel_lpss *lpss;
> +	struct resource priv;
> +	int ret;
> +
> +	if (!info || !info->mem || info->irq <= 0)
> +		return -EINVAL;
> +
> +	lpss = devm_kzalloc(dev, sizeof(*lpss), GFP_KERNEL);
> +	if (!lpss)
> +		return -ENOMEM;
> +
> +	memset(&priv, 0, sizeof(priv));
> +	priv.start = info->mem->start + LPSS_PRIV_OFFSET;
> +	priv.end = priv.start + LPSS_PRIV_SIZE - 1;
> +	priv.flags = IORESOURCE_MEM;
> +
> +	lpss->priv = devm_ioremap_resource(dev, &priv);
> +	if (!lpss->priv)
> +		return -ENOMEM;
> +
> +	intel_lpss_reset(lpss);
> +
> +	lpss->info = info;
> +	lpss->dev = dev;
> +	lpss->caps = readl(lpss->priv + LPSS_PRIV_CAPS);
> +	lpss->ndevs = intel_lpss_has_idma(lpss) ? 2 : 1;
> +
> +	dev_set_drvdata(dev, lpss);
> +
> +	ret = intel_lpss_assign_devs(lpss);
> +	if (ret < 0)
> +		return ret;
> +
> +	lpss->devid = ida_simple_get(&intel_lpss_devid_ida, 0, 0, GFP_KERNEL);
> +	if (lpss->devid < 0)
> +		return lpss->devid;
> +
> +	ret = intel_lpss_register_clock(lpss);
> +	if (ret < 0)
> +		goto err_clk_register;
> +
> +	intel_lpss_ltr_add(lpss);
> +
> +	ret = intel_lpss_debugfs_add(lpss);
> +	if (ret)
> +		dev_warn(lpss->dev, "Failed to create debugfs entries\n");
> +
> +	devs = intel_lpss_has_idma(lpss) ? lpss->devs : lpss->devs + 1;
> +	ret = mfd_add_devices(dev, lpss->devid, devs, lpss->ndevs,
> +			      info->mem, info->irq, NULL);
> +	if (ret < 0)
> +		goto err_remove_ltr;
> +
> +	return 0;
> +
> +err_remove_ltr:
> +	intel_lpss_debugfs_remove(lpss);
> +	intel_lpss_ltr_remove(lpss);
> +
> +err_clk_register:
> +	ida_simple_remove(&intel_lpss_devid_ida, lpss->devid);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(intel_lpss_probe);
> +
> +void intel_lpss_remove(struct device *dev)
> +{
> +	struct intel_lpss *lpss = dev_get_drvdata(dev);
> +
> +	mfd_remove_devices(dev);
> +	intel_lpss_debugfs_remove(lpss);
> +	intel_lpss_ltr_remove(lpss);
> +	intel_lpss_unregister_clock(lpss);
> +	ida_simple_remove(&intel_lpss_devid_ida, lpss->devid);
> +}
> +EXPORT_SYMBOL_GPL(intel_lpss_remove);
> +
> +static int resume_lpss_device(struct device *dev, void *data)
> +{
> +	pm_runtime_resume(dev);
> +	return 0;
> +}
> +
> +int intel_lpss_prepare(struct device *dev)
> +{
> +	/*
> +	 * Resume both child devices before entering system sleep. This
> +	 * ensures that they are in proper state before they get suspended.
> +	 */
> +	device_for_each_child(dev, NULL, resume_lpss_device);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(intel_lpss_prepare);
> +
> +int intel_lpss_suspend(struct device *dev)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(intel_lpss_suspend);
> +
> +int intel_lpss_resume(struct device *dev)
> +{
> +	struct intel_lpss *lpss = dev_get_drvdata(dev);
> +	intel_lpss_reset(lpss);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(intel_lpss_resume);
> +
> +static int __init intel_lpss_init(void)
> +{
> +	intel_lpss_debugfs = debugfs_create_dir("intel_lpss", NULL);
> +	return 0;
> +}
> +module_init(intel_lpss_init);
> +
> +static void __exit intel_lpss_exit(void)
> +{
> +	debugfs_remove(intel_lpss_debugfs);
> +}
> +module_exit(intel_lpss_exit);
> +
> +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
> +MODULE_AUTHOR("Heikki Krogerus <heikki.krogerus@linux.intel.com>");
> +MODULE_AUTHOR("Jarkko Nikula <jarkko.nikula@linux.intel.com>");
> +MODULE_DESCRIPTION("Intel LPSS core driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
> new file mode 100644
> index 0000000..f28cb28
> --- /dev/null
> +++ b/drivers/mfd/intel-lpss.h
> @@ -0,0 +1,62 @@
> +/*
> + * Intel LPSS core support.
> + *
> + * Copyright (C) 2015, Intel Corporation
> + *
> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> + *          Mika Westerberg <mika.westerberg@linux.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 __MFD_INTEL_LPSS_H
> +#define __MFD_INTEL_LPSS_H
> +
> +struct device;
> +struct resource;
> +
> +struct intel_lpss_platform_info {
> +	struct resource *mem;
> +	int irq;
> +	unsigned long clk_rate;
> +	const char *clk_con_id;
> +};
> +
> +int intel_lpss_probe(struct device *dev,
> +		     const struct intel_lpss_platform_info *info);
> +void intel_lpss_remove(struct device *dev);
> +
> +#ifdef CONFIG_PM
> +int intel_lpss_prepare(struct device *dev);
> +int intel_lpss_suspend(struct device *dev);
> +int intel_lpss_resume(struct device *dev);
> +
> +#ifdef CONFIG_PM_SLEEP
> +#define INTEL_LPSS_SLEEP_PM_OPS			\
> +	.prepare = intel_lpss_prepare,		\
> +	.suspend = intel_lpss_suspend,		\
> +	.resume = intel_lpss_resume,		\
> +	.freeze = intel_lpss_suspend,		\
> +	.thaw = intel_lpss_resume,		\
> +	.poweroff = intel_lpss_suspend,		\
> +	.restore = intel_lpss_resume,
> +#endif
> +
> +#define INTEL_LPSS_RUNTIME_PM_OPS		\
> +	.runtime_suspend = intel_lpss_suspend,	\
> +	.runtime_resume = intel_lpss_resume,
> +
> +#else /* !CONFIG_PM */
> +#define INTEL_LPSS_SLEEP_PM_OPS
> +#define INTEL_LPSS_RUNTIME_PM_OPS
> +#endif /* CONFIG_PM */
> +
> +#define INTEL_LPSS_PM_OPS(name)			\
> +const struct dev_pm_ops name = {		\
> +	INTEL_LPSS_SLEEP_PM_OPS			\
> +	INTEL_LPSS_RUNTIME_PM_OPS		\
> +}
> +
> +#endif /* __MFD_INTEL_LPSS_H */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 3/3] mfd: Add support for Intel Sunrisepoint LPSS devices

[Mika Westerberg]

On Tue, Apr 28, 2015 at 02:32:59PM +0100, Lee Jones wrote:
> On Tue, 31 Mar 2015, Andy Shevchenko wrote:
> 
> > The new coming Intel platforms such as Skylake will contain Sunrisepoint PCH.
> > The main difference to the previous platforms is that the LPSS devices are
> > compound devices where usually main (SPI, HSUART, or I2C) and DMA IPs are
> > present.
> > 
> > This patch brings the driver for such devices found on Sunrisepoint PCH.
> 
> I'm not convinced it's really an MFD.  What does this hardware look
> like?  Are the Designware devices really in the same memory/register
> space as the LPSS registers?

Yes they are - there is only single MMIO BAR per PCI device holding, the
host controller, iDMA and convergence layer registers.

Re: [PATCH v1 3/3] mfd: Add support for Intel Sunrisepoint LPSS devices

[Lee Jones]

On Tue, 28 Apr 2015, Mika Westerberg wrote:
> On Tue, Apr 28, 2015 at 02:32:59PM +0100, Lee Jones wrote:
> > On Tue, 31 Mar 2015, Andy Shevchenko wrote:
> > 
> > > The new coming Intel platforms such as Skylake will contain Sunrisepoint PCH.
> > > The main difference to the previous platforms is that the LPSS devices are
> > > compound devices where usually main (SPI, HSUART, or I2C) and DMA IPs are
> > > present.
> > > 
> > > This patch brings the driver for such devices found on Sunrisepoint PCH.
> > 
> > I'm not convinced it's really an MFD.  What does this hardware look
> > like?  Are the Designware devices really in the same memory/register
> > space as the LPSS registers?
> 
> Yes they are - there is only single MMIO BAR per PCI device holding, the
> host controller, iDMA and convergence layer registers.

Are there publicly available docs?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 3/3] mfd: Add support for Intel Sunrisepoint LPSS devices

[Jarkko Nikula]

Hi

On 04/28/2015 08:57 PM, Lee Jones wrote:
> On Tue, 28 Apr 2015, Mika Westerberg wrote:
>> On Tue, Apr 28, 2015 at 02:32:59PM +0100, Lee Jones wrote:
>>> I'm not convinced it's really an MFD.  What does this hardware look
>>> like?  Are the Designware devices really in the same memory/register
>>> space as the LPSS registers?
>>
>> Yes they are - there is only single MMIO BAR per PCI device holding, the
>> host controller, iDMA and convergence layer registers.
>
> Are there publicly available docs?
>
https://download.01.org/future-platform-configuration-hub/skylake/register-definitions/332219_001_Final.pdf

For instance UART Memory Mapped Registers (Chapter 1.2) starts from UART 
MMIO BAR + offset 0 (LPSS_DEV_OFFSET in the patch), UART Additional 
Registers (Ch 1.3) from offset 0x200 (LPSS_PRIV_OFFSET) and UART DMA 
Controller Register (Ch 1.4) from offset 0x800 (LPSS_IDMA_OFFSET).

Idea here is that MFD layer here takes case of reset and clock control 
as well as register the host controllers and integrated DMAs as platform 
devices (not all have the iDMA and thus conditional registering of it 
using the intel_lpss_has_idma()).

That allow us to keep this MFD part, host controller drivers and 
drivers/dma/dw/ changes independent from each other. Fox example host 
controller driver can detect the iDMA by checking is there a "lpss_priv" 
resource and set the slave DMA parameters and DMA filter function 
accordingly (which picks the channel only from associated iDMA device).

-- 
Jarkko

Re: [PATCH v1 3/3] mfd: Add support for Intel Sunrisepoint LPSS devices

[Mika Westerberg]

On Tue, Apr 28, 2015 at 06:57:35PM +0100, Lee Jones wrote:
> On Tue, 28 Apr 2015, Mika Westerberg wrote:
> > On Tue, Apr 28, 2015 at 02:32:59PM +0100, Lee Jones wrote:
> > > On Tue, 31 Mar 2015, Andy Shevchenko wrote:
> > > 
> > > > The new coming Intel platforms such as Skylake will contain Sunrisepoint PCH.
> > > > The main difference to the previous platforms is that the LPSS devices are
> > > > compound devices where usually main (SPI, HSUART, or I2C) and DMA IPs are
> > > > present.
> > > > 
> > > > This patch brings the driver for such devices found on Sunrisepoint PCH.
> > > 
> > > I'm not convinced it's really an MFD.  What does this hardware look
> > > like?  Are the Designware devices really in the same memory/register
> > > space as the LPSS registers?
> > 
> > Yes they are - there is only single MMIO BAR per PCI device holding, the
> > host controller, iDMA and convergence layer registers.
> 
> Are there publicly available docs?

Yes, there is temporary spec here:

https://download.01.org/future-platform-configuration-hub/skylake/register-definitions/332219_001_Final.pdf

(it will be part of the PCH public documentation, once it is released).

Re: [PATCH v1 3/3] mfd: Add support for Intel Sunrisepoint LPSS devices

[Lee Jones]

On Tue, 31 Mar 2015, Andy Shevchenko wrote:
> The new coming Intel platforms such as Skylake will contain Sunrisepoint PCH.

Can you use the same naming conventions as the Skylake's predecessors?

Currently lpc_ich and lpc_sch exist.

> The main difference to the previous platforms is that the LPSS devices are
> compound devices where usually main (SPI, HSUART, or I2C) and DMA IPs are
> present.
> 
> This patch brings the driver for such devices found on Sunrisepoint PCH.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/Kconfig           |  24 ++
>  drivers/mfd/Makefile          |   3 +
>  drivers/mfd/intel-lpss-acpi.c |  84 +++++++
>  drivers/mfd/intel-lpss-pci.c  | 106 +++++++++
>  drivers/mfd/intel-lpss.c      | 523 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/mfd/intel-lpss.h      |  62 +++++

Can you move this to include/?

>  6 files changed, 802 insertions(+)
>  create mode 100644 drivers/mfd/intel-lpss-acpi.c
>  create mode 100644 drivers/mfd/intel-lpss-pci.c
>  create mode 100644 drivers/mfd/intel-lpss.c
>  create mode 100644 drivers/mfd/intel-lpss.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index d5ad04d..b1a6778 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -325,6 +325,30 @@ config INTEL_SOC_PMIC
>  	  thermal, charger and related power management functions
>  	  on these systems.
>  
> +config MFD_INTEL_LPSS
> +	tristate "Intel Low Power Subsystem support"
> +	depends on X86
> +	select COMMON_CLK
> +	select MFD_CORE
> +	help
> +	  This driver provides necessary plumbing for Intel Low Power
> +	  Subsystem (LPSS) devices such as I2C, SPI and HS-UART starting
> +	  from Intel Sunrisepoint (Intel Skylake PCH) and newer chipsets.
> +
> +config MFD_INTEL_LPSS_ACPI
> +	tristate "Intel Low Power Subsystem support in ACPI mode"
> +	depends on MFD_INTEL_LPSS && ACPI
> +	help
> +	  This driver support Intel Low Power Subsystem devices in ACPI
> +	  mode.
> +
> +config MFD_INTEL_LPSS_PCI
> +	tristate "Intel Low Power Subsystem support in PCI mode"
> +	depends on MFD_INTEL_LPSS && PCI
> +	help
> +	  This driver support Intel Low Power Subsystem devices in PCI
> +	  mode.
> +
>  config MFD_INTEL_MSIC
>  	bool "Intel MSIC"
>  	depends on INTEL_SCU_IPC
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 0e5cfeb..cdf29b9 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -161,6 +161,9 @@ obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
>  obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
>  obj-$(CONFIG_MFD_AAT2870_CORE)	+= aat2870-core.o
>  obj-$(CONFIG_MFD_ATMEL_HLCDC)	+= atmel-hlcdc.o
> +obj-$(CONFIG_MFD_INTEL_LPSS)	+= intel-lpss.o
> +obj-$(CONFIG_MFD_INTEL_LPSS_PCI)	+= intel-lpss-pci.o
> +obj-$(CONFIG_MFD_INTEL_LPSS_ACPI)	+= intel-lpss-acpi.o
>  obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
>  obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
>  obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
> diff --git a/drivers/mfd/intel-lpss-acpi.c b/drivers/mfd/intel-lpss-acpi.c
> new file mode 100644
> index 0000000..0d92d73
> --- /dev/null
> +++ b/drivers/mfd/intel-lpss-acpi.c
> @@ -0,0 +1,84 @@
> +/*
> + * Intel LPSS ACPI support.
> + *
> + * Copyright (C) 2015, Intel Corporation
> + *
> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> + *          Mika Westerberg <mika.westerberg@linux.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/ioport.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pm.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/platform_device.h>
> +
> +#include "intel-lpss.h"
> +
> +static const struct intel_lpss_platform_info spt_info = {
> +	.clk_rate = 120000000,
> +};
> +
> +static const struct acpi_device_id intel_lpss_acpi_ids[] = {
> +	/* SPT */
> +	{ "INT3446", (kernel_ulong_t)&spt_info },
> +	{ "INT3447", (kernel_ulong_t)&spt_info },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, intel_lpss_acpi_ids);
> +
> +static int intel_lpss_acpi_probe(struct platform_device *pdev)
> +{
> +	struct intel_lpss_platform_info *info;
> +	const struct acpi_device_id *id;
> +
> +	id = acpi_match_device(intel_lpss_acpi_ids, &pdev->dev);
> +	if (!id)
> +		return -ENODEV;
> +
> +	info = devm_kmemdup(&pdev->dev, (void *)id->driver_data, sizeof(*info),
> +			    GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	info->mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	info->irq = platform_get_irq(pdev, 0);
> +
> +	pm_runtime_set_active(&pdev->dev);
> +	pm_runtime_enable(&pdev->dev);
> +
> +	return intel_lpss_probe(&pdev->dev, info);
> +}
> +
> +static int intel_lpss_acpi_remove(struct platform_device *pdev)
> +{
> +	intel_lpss_remove(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static INTEL_LPSS_PM_OPS(intel_lpss_acpi_pm_ops);
> +
> +static struct platform_driver intel_lpss_acpi_driver = {
> +	.probe = intel_lpss_acpi_probe,
> +	.remove = intel_lpss_acpi_remove,
> +	.driver = {
> +		.name = "intel-lpss",
> +		.acpi_match_table = intel_lpss_acpi_ids,
> +		.pm = &intel_lpss_acpi_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(intel_lpss_acpi_driver);
> +
> +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
> +MODULE_DESCRIPTION("Intel LPSS ACPI driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
> new file mode 100644
> index 0000000..fd8d25d
> --- /dev/null
> +++ b/drivers/mfd/intel-lpss-pci.c
> @@ -0,0 +1,106 @@
> +/*
> + * Intel LPSS PCI support.
> + *
> + * Copyright (C) 2015, Intel Corporation
> + *
> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> + *          Mika Westerberg <mika.westerberg@linux.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/ioport.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/pm.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "intel-lpss.h"
> +
> +static int intel_lpss_pci_probe(struct pci_dev *pdev,
> +				const struct pci_device_id *id)
> +{
> +	struct intel_lpss_platform_info *info;
> +	int ret;
> +
> +	ret = pcim_enable_device(pdev);
> +	if (ret)
> +		return ret;
> +
> +	info = devm_kmemdup(&pdev->dev, (void *)id->driver_data, sizeof(*info),
> +			    GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	info->mem = &pdev->resource[0];
> +	info->irq = pdev->irq;
> +
> +	ret = intel_lpss_probe(&pdev->dev, info);
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_put(&pdev->dev);
> +	return 0;
> +}
> +
> +static void intel_lpss_pci_remove(struct pci_dev *pdev)
> +{
> +	pm_runtime_get_sync(&pdev->dev);
> +	intel_lpss_remove(&pdev->dev);
> +}
> +
> +static INTEL_LPSS_PM_OPS(intel_lpss_pci_pm_ops);
> +
> +static const struct intel_lpss_platform_info spt_info = {
> +	.clk_rate = 120000000,
> +};
> +
> +static const struct intel_lpss_platform_info spt_uart_info = {
> +	.clk_rate = 120000000,
> +	.clk_con_id = "baudclk",
> +};
> +
> +static const struct pci_device_id intel_lpss_pci_ids[] = {
> +	/* SPT-LP */
> +	{ PCI_VDEVICE(INTEL, 0x9d27), (kernel_ulong_t)&spt_uart_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d28), (kernel_ulong_t)&spt_uart_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d29), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d2a), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d60), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d61), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d62), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d63), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d64), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d65), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d66), (kernel_ulong_t)&spt_uart_info },
> +	/* SPT-H */
> +	{ PCI_VDEVICE(INTEL, 0xa127), (kernel_ulong_t)&spt_uart_info },
> +	{ PCI_VDEVICE(INTEL, 0xa128), (kernel_ulong_t)&spt_uart_info },
> +	{ PCI_VDEVICE(INTEL, 0xa129), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0xa12a), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0xa160), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0xa161), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0xa166), (kernel_ulong_t)&spt_uart_info },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(pci, intel_lpss_pci_ids);
> +
> +static struct pci_driver intel_lpss_pci_driver = {
> +	.name = "intel-lpss",
> +	.id_table = intel_lpss_pci_ids,
> +	.probe = intel_lpss_pci_probe,
> +	.remove = intel_lpss_pci_remove,
> +	.driver = {
> +		.pm = &intel_lpss_pci_pm_ops,
> +	},
> +};
> +
> +module_pci_driver(intel_lpss_pci_driver);
> +
> +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
> +MODULE_DESCRIPTION("Intel LPSS PCI driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> new file mode 100644
> index 0000000..78d1921
> --- /dev/null
> +++ b/drivers/mfd/intel-lpss.c
> @@ -0,0 +1,523 @@
> +/*
> + * Intel Sunrisepoint LPSS core support.
> + *
> + * Copyright (C) 2015, Intel Corporation
> + *
> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> + *          Mika Westerberg <mika.westerberg@linux.intel.com>
> + *          Heikki Krogerus <heikki.krogerus@linux.intel.com>
> + *          Jarkko Nikula <jarkko.nikula@linux.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/clk.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/debugfs.h>
> +#include <linux/idr.h>
> +#include <linux/ioport.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mfd/core.h>
> +#include <linux/platform_data/dma-dw.h>
> +#include <linux/pm_qos.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/seq_file.h>
> +
> +#include "intel-lpss.h"
> +
> +#define LPSS_DEV_OFFSET		0x000
> +#define LPSS_DEV_SIZE		0x200
> +#define LPSS_PRIV_OFFSET	0x200
> +#define LPSS_PRIV_SIZE		0x100
> +#define LPSS_IDMA_OFFSET	0x800
> +#define LPSS_IDMA_SIZE		0x800
> +
> +/* Offsets from lpss->priv */
> +#define LPSS_PRIV_RESETS		0x04
> +#define LPSS_PRIV_RESETS_FUNC		BIT(2)
> +#define LPSS_PRIV_RESETS_IDMA		0x3
> +
> +#define LPSS_PRIV_ACTIVELTR		0x10
> +#define LPSS_PRIV_IDLELTR		0x14
> +
> +#define LPSS_PRIV_LTR_REQ		BIT(15)
> +#define LPSS_PRIV_LTR_SCALE_MASK	0xc00
> +#define LPSS_PRIV_LTR_SCALE_1US		0x800
> +#define LPSS_PRIV_LTR_SCALE_32US	0xc00
> +#define LPSS_PRIV_LTR_VALUE_MASK	0x3ff
> +
> +#define LPSS_PRIV_CLK_GATE		0x38
> +#define LPSS_PRIV_CLK_GATE_IP_SHIFT	0
> +#define LPSS_PRIV_CLK_GATE_IDMA_SHIFT	2
> +#define LPSS_PRIV_CLK_GATE_EN_MASK	0x2 /* Dynamic gating disabled */
> +
> +#define LPSS_PRIV_CAPS			0xfc
> +#define LPSS_PRIV_CAPS_NO_IDMA		BIT(8)
> +#define LPSS_PRIV_CAPS_TYPE_SHIFT	4
> +#define LPSS_PRIV_CAPS_TYPE_MASK	(0xf << LPSS_PRIV_CAPS_TYPE_SHIFT)
> +
> +/* This matches the type field in CAPS register */
> +enum intel_lpss_dev_type {
> +	LPSS_DEV_I2C = 0,
> +	LPSS_DEV_UART,
> +	LPSS_DEV_SPI,
> +};
> +
> +struct intel_lpss {
> +	const struct intel_lpss_platform_info *info;
> +	enum intel_lpss_dev_type type;
> +	struct clk *clk;
> +	struct clk_lookup **clocks;
> +	unsigned int ndevs;
> +	const struct mfd_cell *devs;
> +	struct device *dev;
> +	void __iomem *priv;
> +	int devid;
> +	u32 caps;
> +	u32 active_ltr;
> +	u32 idle_ltr;
> +	struct dentry *debugfs;
> +};
> +
> +static const struct dw_dma_platform_data intel_lpss_idma_pdata = {
> +	.type = DW_DMAC_TYPE_INTEL,
> +	.nr_channels = 2,
> +	.is_private = true,
> +	.block_size = 0x1ffff,
> +	.nr_masters = 1,
> +	.data_width = {2},

.data_width = { 2 },

> +};
> +
> +static const struct resource intel_lpss_dev_resources[] = {
> +	DEFINE_RES_MEM_NAMED(LPSS_DEV_OFFSET, LPSS_DEV_SIZE, "lpss_dev"),
> +	DEFINE_RES_MEM_NAMED(LPSS_PRIV_OFFSET, LPSS_PRIV_SIZE, "lpss_priv"),
> +	DEFINE_RES_IRQ(0),
> +};

Why has this been called 'dev' and 'priv'?  Neither of which are
particularlly helpful address names and I can't find them in the
datasheet.  Why not 'base' and 'extended', or something?

> +static const struct resource intel_lpss_idma_resources[] = {
> +	DEFINE_RES_MEM(LPSS_IDMA_OFFSET, LPSS_IDMA_SIZE),
> +	DEFINE_RES_IRQ(0),
> +};
> +
> +/*
> + * Cells needs to be ordered so that the iDMA is created first. This is
> + * because we need to be sure the DMA is available when the host controller
> + * driver is probed.
> + */
> +static const struct mfd_cell intel_lpss_i2c_devs[] = {
> +	{
> +		.name = "dw_dmac",
> +		.num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
> +		.resources = intel_lpss_idma_resources,
> +		.platform_data = (void *)&intel_lpss_idma_pdata,
> +		.pdata_size = sizeof(intel_lpss_idma_pdata),
> +	},
> +	{
> +		.name = "i2c_designware",
> +		.num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
> +		.resources = intel_lpss_dev_resources,
> +	},
> +};
> +
> +static const struct mfd_cell intel_lpss_uart_devs[] = {
> +	{
> +		.name = "dw_dmac",
> +		.num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
> +		.resources = intel_lpss_idma_resources,
> +		.platform_data = (void *)&intel_lpss_idma_pdata,
> +		.pdata_size = sizeof(intel_lpss_idma_pdata),
> +	},
> +	{
> +		.name = "dw-apb-uart",
> +		.num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
> +		.resources = intel_lpss_dev_resources,
> +	},
> +};
> +
> +static const struct mfd_cell intel_lpss_spi_devs[] = {
> +	{
> +		.name = "dw_dmac",
> +		.num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
> +		.resources = intel_lpss_idma_resources,
> +		.platform_data = (void *)&intel_lpss_idma_pdata,
> +		.pdata_size = sizeof(intel_lpss_idma_pdata),
> +	},
> +	{
> +		.name = "pxa2xx-spi",
> +		.num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
> +		.resources = intel_lpss_dev_resources,
> +	},
> +};
> +
> +static DEFINE_IDA(intel_lpss_devid_ida);

What's wrong with using PLATFORM_DEVID_AUTO?

> +static struct dentry *intel_lpss_debugfs;
> +
> +static void intel_lpss_cache_ltr(struct intel_lpss *lpss)
> +{
> +	lpss->active_ltr = readl(lpss->priv + LPSS_PRIV_ACTIVELTR);
> +	lpss->idle_ltr = readl(lpss->priv + LPSS_PRIV_IDLELTR);
> +}
> +
> +static int intel_lpss_debugfs_add(struct intel_lpss *lpss)
> +{
> +	struct dentry *dir;
> +
> +	dir = debugfs_create_dir(dev_name(lpss->dev), intel_lpss_debugfs);
> +	if (IS_ERR(dir))
> +		return PTR_ERR(dir);
> +
> +	/* Cache the values into lpss structure */
> +	intel_lpss_cache_ltr(lpss);
> +
> +	debugfs_create_x32("capabilities", S_IRUGO, dir, &lpss->caps);
> +	debugfs_create_x32("active_ltr", S_IRUGO, dir, &lpss->active_ltr);
> +	debugfs_create_x32("idle_ltr", S_IRUGO, dir, &lpss->idle_ltr);
> +
> +	lpss->debugfs = dir;
> +	return 0;
> +}
> +
> +static void intel_lpss_debugfs_remove(struct intel_lpss *lpss)
> +{
> +	debugfs_remove_recursive(lpss->debugfs);
> +}

A comment here explaining what you're doing below wouldn't go amiss.

> +static void intel_lpss_ltr_set(struct device *dev, s32 val)
> +{
> +	struct intel_lpss *lpss = dev_get_drvdata(dev);
> +	u32 ltr;
> +
> +	ltr = readl(lpss->priv + LPSS_PRIV_ACTIVELTR);
> +
> +	if (val == PM_QOS_LATENCY_ANY || val < 0) {
> +		ltr &= ~LPSS_PRIV_LTR_REQ;
> +	} else {
> +		ltr |= LPSS_PRIV_LTR_REQ;
> +		ltr &= ~LPSS_PRIV_LTR_SCALE_MASK;
> +		ltr &= ~LPSS_PRIV_LTR_VALUE_MASK;
> +
> +		if (val > LPSS_PRIV_LTR_VALUE_MASK)
> +			ltr |= LPSS_PRIV_LTR_SCALE_32US | val >> 5;
> +		else
> +			ltr |= LPSS_PRIV_LTR_SCALE_1US | val;
> +	}
> +
> +	if (ltr == lpss->active_ltr)
> +		return;
> +
> +	writel(ltr, lpss->priv + LPSS_PRIV_ACTIVELTR);
> +	writel(ltr, lpss->priv + LPSS_PRIV_IDLELTR);
> +
> +	/* Cache the values into lpss structure */
> +	intel_lpss_cache_ltr(lpss);
> +}
> +
> +static void intel_lpss_ltr_add(struct intel_lpss *lpss)
> +{
> +	lpss->dev->power.set_latency_tolerance = intel_lpss_ltr_set;
> +	dev_pm_qos_expose_latency_tolerance(lpss->dev);
> +}
> +
> +static void intel_lpss_ltr_remove(struct intel_lpss *lpss)
> +{
> +	dev_pm_qos_hide_latency_tolerance(lpss->dev);
> +	lpss->dev->power.set_latency_tolerance = NULL;
> +}
> +
> +static int intel_lpss_assign_devs(struct intel_lpss *lpss)
> +{
> +	unsigned int type;
> +
> +	type = lpss->caps & LPSS_PRIV_CAPS_TYPE_MASK;
> +	type >>= LPSS_PRIV_CAPS_TYPE_SHIFT;
> +
> +	switch (type) {
> +	case LPSS_DEV_I2C:
> +		lpss->devs = intel_lpss_i2c_devs;
> +		break;
> +	case LPSS_DEV_UART:
> +		lpss->devs = intel_lpss_uart_devs;
> +		break;
> +	case LPSS_DEV_SPI:
> +		lpss->devs = intel_lpss_spi_devs;
> +		break;
> +	default:
> +		return -ENODEV;
> +	}
> +
> +	lpss->type = type;
> +
> +	return 0;
> +}
> +
> +static bool intel_lpss_has_idma(const struct intel_lpss *lpss)
> +{
> +	return (lpss->caps & LPSS_PRIV_CAPS_NO_IDMA) == 0;
> +}
> +
> +static void intel_lpss_reset(const struct intel_lpss *lpss)

Not very descriptive function name.  'de|assert'?

> +{
> +	u32 value = LPSS_PRIV_RESETS_FUNC | LPSS_PRIV_RESETS_IDMA;
> +
> +	/* Bring out the device from reset */
> +	writel(value, lpss->priv + LPSS_PRIV_RESETS);
> +}

All of this clock stuff below need s Clock Ack.

> +static void intel_lpss_unregister_clock_tree(struct clk *clk)
> +{
> +	struct clk *parent;
> +
> +	while (clk) {
> +		parent = clk_get_parent(clk);
> +		clk_unregister(clk);
> +		clk = parent;
> +	}
> +}
> +
> +static int intel_lpss_register_clock_divider(struct intel_lpss *lpss,
> +					     const char *devname,
> +					     struct clk **clk)
> +{
> +	char name[32];
> +	struct clk *tmp = *clk;
> +
> +	snprintf(name, sizeof(name), "%s-enable", devname);
> +	tmp = clk_register_gate(NULL, name, __clk_get_name(tmp), 0,
> +				lpss->priv, 0, 0, NULL);
> +	if (IS_ERR(tmp))
> +		return PTR_ERR(tmp);
> +
> +	snprintf(name, sizeof(name), "%s-div", devname);
> +	tmp = clk_register_fractional_divider(NULL, name, __clk_get_name(tmp),
> +					      0, lpss->priv, 1, 15, 16, 15, 0,
> +					      NULL);
> +	if (IS_ERR(tmp))
> +		return PTR_ERR(tmp);
> +	*clk = tmp;
> +
> +	snprintf(name, sizeof(name), "%s-update", devname);
> +	tmp = clk_register_gate(NULL, name, __clk_get_name(tmp),
> +				CLK_SET_RATE_PARENT, lpss->priv, 31, 0, NULL);
> +	if (IS_ERR(tmp))
> +		return PTR_ERR(tmp);
> +	*clk = tmp;
> +
> +	return 0;
> +}
> +
> +static int intel_lpss_register_clock(struct intel_lpss *lpss)
> +{
> +	const struct mfd_cell *devs = lpss->devs;
> +	struct clk_lookup **clocks;
> +	struct clk *clk, *root;
> +	char devname[24];
> +	int ret = -ENOMEM;
> +
> +	if (!lpss->info->clk_rate)
> +		return 0;
> +
> +	clocks = devm_kcalloc(lpss->dev, lpss->ndevs, sizeof(*clocks),
> +			      GFP_KERNEL);
> +	if (!clocks)
> +		return -ENOMEM;
> +
> +	/* Root clock */
> +	root = clk_register_fixed_rate(NULL, dev_name(lpss->dev), NULL,
> +				       CLK_IS_ROOT, lpss->info->clk_rate);
> +	if (IS_ERR(root))
> +		return PTR_ERR(root);
> +
> +	clk = root;
> +
> +	snprintf(devname, sizeof(devname), "%s.%d", devs[1].name, lpss->devid);
> +
> +	/*
> +	 * Support for clock divider only if it has some preset value.
> +	 * Otherwise we assume that the divider is not used.
> +	 */
> +	if (lpss->type != LPSS_DEV_I2C) {
> +		ret = intel_lpss_register_clock_divider(lpss, devname, &clk);
> +		if (ret)
> +			goto err_clk_register;
> +	}
> +
> +	/* Clock for the host controller */
> +	clocks[0] = clk_register_clkdev(clk, lpss->info->clk_con_id, "%s",
> +					devname);
> +	if (IS_ERR(clocks[0]))
> +		goto err_clk_register;
> +
> +	/*
> +	 * If the slice has integrated DMA block create clock for that as
> +	 * well now.
> +	 */
> +	if (lpss->ndevs > 1) {
> +		clocks[1] = clk_register_clkdev(root, "hclk", "%s.%d",
> +						devs[0].name, lpss->devid);
> +		if (IS_ERR(clocks[1]))
> +			goto err_hclk_register;
> +	}
> +
> +	lpss->clocks = clocks;
> +	lpss->clk = clk;
> +
> +	return 0;
> +
> +err_hclk_register:
> +	clkdev_drop(clocks[0]);
> +
> +err_clk_register:
> +	intel_lpss_unregister_clock_tree(clk);
> +
> +	return ret;
> +}
> +
> +static void intel_lpss_unregister_clock(struct intel_lpss *lpss)
> +{
> +	unsigned int i;
> +
> +	if (IS_ERR_OR_NULL(lpss->clk))
> +		return;
> +
> +	for (i = 0; i < lpss->ndevs; i++)
> +		clkdev_drop(lpss->clocks[i]);
> +
> +	intel_lpss_unregister_clock_tree(lpss->clk);
> +}
> +
> +int intel_lpss_probe(struct device *dev,
> +		     const struct intel_lpss_platform_info *info)
> +{
> +	const struct mfd_cell *devs;
> +	struct intel_lpss *lpss;
> +	struct resource priv;
> +	int ret;
> +
> +	if (!info || !info->mem || info->irq <= 0)
> +		return -EINVAL;
> +
> +	lpss = devm_kzalloc(dev, sizeof(*lpss), GFP_KERNEL);
> +	if (!lpss)
> +		return -ENOMEM;
> +
> +	memset(&priv, 0, sizeof(priv));
> +	priv.start = info->mem->start + LPSS_PRIV_OFFSET;
> +	priv.end = priv.start + LPSS_PRIV_SIZE - 1;
> +	priv.flags = IORESOURCE_MEM;
> +
> +	lpss->priv = devm_ioremap_resource(dev, &priv);
> +	if (!lpss->priv)
> +		return -ENOMEM;
> +
> +	intel_lpss_reset(lpss);
> +
> +	lpss->info = info;
> +	lpss->dev = dev;
> +	lpss->caps = readl(lpss->priv + LPSS_PRIV_CAPS);
> +	lpss->ndevs = intel_lpss_has_idma(lpss) ? 2 : 1;

This is fragile.

I'd prefer you dynamically allocate resources here, instead of
statically up the top then 'jumping' over the ones you don't need.

> +	dev_set_drvdata(dev, lpss);
> +
> +	ret = intel_lpss_assign_devs(lpss);
> +	if (ret < 0)
> +		return ret;

Is ret allowed to be >0?  If not just 'if (ret)'.

> +	lpss->devid = ida_simple_get(&intel_lpss_devid_ida, 0, 0, GFP_KERNEL);
> +	if (lpss->devid < 0)
> +		return lpss->devid;

PLATFORM_DEVID_AUTO does this for you.

> +	ret = intel_lpss_register_clock(lpss);
> +	if (ret < 0)
> +		goto err_clk_register;
> +
> +	intel_lpss_ltr_add(lpss);

Add what, to what?  What is 'ltr'?

> +	ret = intel_lpss_debugfs_add(lpss);
> +	if (ret)
> +		dev_warn(lpss->dev, "Failed to create debugfs entries\n");
> +
> +	devs = intel_lpss_has_idma(lpss) ? lpss->devs : lpss->devs + 1;

*cringe*

> +	ret = mfd_add_devices(dev, lpss->devid, devs, lpss->ndevs,
> +			      info->mem, info->irq, NULL);
> +	if (ret < 0)
> +		goto err_remove_ltr;
> +
> +	return 0;
> +
> +err_remove_ltr:
> +	intel_lpss_debugfs_remove(lpss);
> +	intel_lpss_ltr_remove(lpss);
> +
> +err_clk_register:
> +	ida_simple_remove(&intel_lpss_devid_ida, lpss->devid);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(intel_lpss_probe);
> +
> +void intel_lpss_remove(struct device *dev)
> +{
> +	struct intel_lpss *lpss = dev_get_drvdata(dev);
> +
> +	mfd_remove_devices(dev);
> +	intel_lpss_debugfs_remove(lpss);
> +	intel_lpss_ltr_remove(lpss);
> +	intel_lpss_unregister_clock(lpss);
> +	ida_simple_remove(&intel_lpss_devid_ida, lpss->devid);
> +}
> +EXPORT_SYMBOL_GPL(intel_lpss_remove);
> +
> +static int resume_lpss_device(struct device *dev, void *data)
> +{
> +	pm_runtime_resume(dev);
> +	return 0;
> +}
> +
> +int intel_lpss_prepare(struct device *dev)
> +{
> +	/*
> +	 * Resume both child devices before entering system sleep. This
> +	 * ensures that they are in proper state before they get suspended.
> +	 */
> +	device_for_each_child(dev, NULL, resume_lpss_device);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(intel_lpss_prepare);
> +
> +int intel_lpss_suspend(struct device *dev)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(intel_lpss_suspend);
> +
> +int intel_lpss_resume(struct device *dev)
> +{
> +	struct intel_lpss *lpss = dev_get_drvdata(dev);
> +	intel_lpss_reset(lpss);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(intel_lpss_resume);
> +
> +static int __init intel_lpss_init(void)
> +{
> +	intel_lpss_debugfs = debugfs_create_dir("intel_lpss", NULL);
> +	return 0;
> +}
> +module_init(intel_lpss_init);
> +
> +static void __exit intel_lpss_exit(void)
> +{
> +	debugfs_remove(intel_lpss_debugfs);
> +}
> +module_exit(intel_lpss_exit);
> +
> +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
> +MODULE_AUTHOR("Heikki Krogerus <heikki.krogerus@linux.intel.com>");
> +MODULE_AUTHOR("Jarkko Nikula <jarkko.nikula@linux.intel.com>");
> +MODULE_DESCRIPTION("Intel LPSS core driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
> new file mode 100644
> index 0000000..f28cb28
> --- /dev/null
> +++ b/drivers/mfd/intel-lpss.h
> @@ -0,0 +1,62 @@
> +/*
> + * Intel LPSS core support.
> + *
> + * Copyright (C) 2015, Intel Corporation
> + *
> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> + *          Mika Westerberg <mika.westerberg@linux.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 __MFD_INTEL_LPSS_H
> +#define __MFD_INTEL_LPSS_H
> +
> +struct device;
> +struct resource;
> +
> +struct intel_lpss_platform_info {
> +	struct resource *mem;
> +	int irq;
> +	unsigned long clk_rate;
> +	const char *clk_con_id;
> +};
> +
> +int intel_lpss_probe(struct device *dev,
> +		     const struct intel_lpss_platform_info *info);
> +void intel_lpss_remove(struct device *dev);
> +
> +#ifdef CONFIG_PM
> +int intel_lpss_prepare(struct device *dev);
> +int intel_lpss_suspend(struct device *dev);
> +int intel_lpss_resume(struct device *dev);

Why are you exporting these?

> +#ifdef CONFIG_PM_SLEEP
> +#define INTEL_LPSS_SLEEP_PM_OPS			\
> +	.prepare = intel_lpss_prepare,		\
> +	.suspend = intel_lpss_suspend,		\
> +	.resume = intel_lpss_resume,		\
> +	.freeze = intel_lpss_suspend,		\
> +	.thaw = intel_lpss_resume,		\
> +	.poweroff = intel_lpss_suspend,		\
> +	.restore = intel_lpss_resume,
> +#endif

SET_SYSTEM_SLEEP_PM_OPS?

> +#define INTEL_LPSS_RUNTIME_PM_OPS		\
> +	.runtime_suspend = intel_lpss_suspend,	\
> +	.runtime_resume = intel_lpss_resume,

SET_RUNTIME_PM_OPS

> +#else /* !CONFIG_PM */
> +#define INTEL_LPSS_SLEEP_PM_OPS
> +#define INTEL_LPSS_RUNTIME_PM_OPS
> +#endif /* CONFIG_PM */
> +
> +#define INTEL_LPSS_PM_OPS(name)			\
> +const struct dev_pm_ops name = {		\
> +	INTEL_LPSS_SLEEP_PM_OPS			\
> +	INTEL_LPSS_RUNTIME_PM_OPS		\
> +}

Why is this in the header?

> +#endif /* __MFD_INTEL_LPSS_H */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 3/3] mfd: Add support for Intel Sunrisepoint LPSS devices

[Andy Shevchenko]

On Wed, Apr 29, 2015 at 11:23 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 31 Mar 2015, Andy Shevchenko wrote:

Thanks for review. My comments / answers below.

>> The new coming Intel platforms such as Skylake will contain Sunrisepoint PCH.
>
> Can you use the same naming conventions as the Skylake's predecessors?
>
> Currently lpc_ich and lpc_sch exist.

As far as I understand it is not an LPC bus, thus seems above is
irrelevant. Moreover, PCH in Skylake is something new if looking into
hardware.

>
>> The main difference to the previous platforms is that the LPSS devices are
>> compound devices where usually main (SPI, HSUART, or I2C) and DMA IPs are
>> present.
>>
>> This patch brings the driver for such devices found on Sunrisepoint PCH.
>>
>> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>>  drivers/mfd/Kconfig           |  24 ++
>>  drivers/mfd/Makefile          |   3 +
>>  drivers/mfd/intel-lpss-acpi.c |  84 +++++++
>>  drivers/mfd/intel-lpss-pci.c  | 106 +++++++++
>>  drivers/mfd/intel-lpss.c      | 523 ++++++++++++++++++++++++++++++++++++++++++
>>  drivers/mfd/intel-lpss.h      |  62 +++++
>
> Can you move this to include/?

What the point? I don't see the users for that now or in the nearest future.

>
>>  6 files changed, 802 insertions(+)
>>  create mode 100644 drivers/mfd/intel-lpss-acpi.c
>>  create mode 100644 drivers/mfd/intel-lpss-pci.c
>>  create mode 100644 drivers/mfd/intel-lpss.c
>>  create mode 100644 drivers/mfd/intel-lpss.h
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index d5ad04d..b1a6778 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -325,6 +325,30 @@ config INTEL_SOC_PMIC
>>         thermal, charger and related power management functions
>>         on these systems.
>>
>> +config MFD_INTEL_LPSS
>> +     tristate "Intel Low Power Subsystem support"
>> +     depends on X86
>> +     select COMMON_CLK
>> +     select MFD_CORE
>> +     help
>> +       This driver provides necessary plumbing for Intel Low Power
>> +       Subsystem (LPSS) devices such as I2C, SPI and HS-UART starting
>> +       from Intel Sunrisepoint (Intel Skylake PCH) and newer chipsets.
>> +
>> +config MFD_INTEL_LPSS_ACPI
>> +     tristate "Intel Low Power Subsystem support in ACPI mode"
>> +     depends on MFD_INTEL_LPSS && ACPI
>> +     help
>> +       This driver support Intel Low Power Subsystem devices in ACPI
>> +       mode.
>> +
>> +config MFD_INTEL_LPSS_PCI
>> +     tristate "Intel Low Power Subsystem support in PCI mode"
>> +     depends on MFD_INTEL_LPSS && PCI
>> +     help
>> +       This driver support Intel Low Power Subsystem devices in PCI
>> +       mode.
>> +
>>  config MFD_INTEL_MSIC
>>       bool "Intel MSIC"
>>       depends on INTEL_SCU_IPC
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index 0e5cfeb..cdf29b9 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -161,6 +161,9 @@ obj-$(CONFIG_TPS65911_COMPARATOR) += tps65911-comparator.o
>>  obj-$(CONFIG_MFD_TPS65090)   += tps65090.o
>>  obj-$(CONFIG_MFD_AAT2870_CORE)       += aat2870-core.o
>>  obj-$(CONFIG_MFD_ATMEL_HLCDC)        += atmel-hlcdc.o
>> +obj-$(CONFIG_MFD_INTEL_LPSS) += intel-lpss.o
>> +obj-$(CONFIG_MFD_INTEL_LPSS_PCI)     += intel-lpss-pci.o
>> +obj-$(CONFIG_MFD_INTEL_LPSS_ACPI)    += intel-lpss-acpi.o
>>  obj-$(CONFIG_MFD_INTEL_MSIC) += intel_msic.o
>>  obj-$(CONFIG_MFD_PALMAS)     += palmas.o
>>  obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
>> diff --git a/drivers/mfd/intel-lpss-acpi.c b/drivers/mfd/intel-lpss-acpi.c
>> new file mode 100644
>> index 0000000..0d92d73
>> --- /dev/null
>> +++ b/drivers/mfd/intel-lpss-acpi.c
>> @@ -0,0 +1,84 @@
>> +/*
>> + * Intel LPSS ACPI support.
>> + *
>> + * Copyright (C) 2015, Intel Corporation
>> + *
>> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> + *          Mika Westerberg <mika.westerberg@linux.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/ioport.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/pm.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "intel-lpss.h"
>> +
>> +static const struct intel_lpss_platform_info spt_info = {
>> +     .clk_rate = 120000000,
>> +};
>> +
>> +static const struct acpi_device_id intel_lpss_acpi_ids[] = {
>> +     /* SPT */
>> +     { "INT3446", (kernel_ulong_t)&spt_info },
>> +     { "INT3447", (kernel_ulong_t)&spt_info },
>> +     { }
>> +};
>> +MODULE_DEVICE_TABLE(acpi, intel_lpss_acpi_ids);
>> +
>> +static int intel_lpss_acpi_probe(struct platform_device *pdev)
>> +{
>> +     struct intel_lpss_platform_info *info;
>> +     const struct acpi_device_id *id;
>> +
>> +     id = acpi_match_device(intel_lpss_acpi_ids, &pdev->dev);
>> +     if (!id)
>> +             return -ENODEV;
>> +
>> +     info = devm_kmemdup(&pdev->dev, (void *)id->driver_data, sizeof(*info),
>> +                         GFP_KERNEL);
>> +     if (!info)
>> +             return -ENOMEM;
>> +
>> +     info->mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +     info->irq = platform_get_irq(pdev, 0);
>> +
>> +     pm_runtime_set_active(&pdev->dev);
>> +     pm_runtime_enable(&pdev->dev);
>> +
>> +     return intel_lpss_probe(&pdev->dev, info);
>> +}
>> +
>> +static int intel_lpss_acpi_remove(struct platform_device *pdev)
>> +{
>> +     intel_lpss_remove(&pdev->dev);
>> +     pm_runtime_disable(&pdev->dev);
>> +
>> +     return 0;
>> +}
>> +
>> +static INTEL_LPSS_PM_OPS(intel_lpss_acpi_pm_ops);
>> +
>> +static struct platform_driver intel_lpss_acpi_driver = {
>> +     .probe = intel_lpss_acpi_probe,
>> +     .remove = intel_lpss_acpi_remove,
>> +     .driver = {
>> +             .name = "intel-lpss",
>> +             .acpi_match_table = intel_lpss_acpi_ids,
>> +             .pm = &intel_lpss_acpi_pm_ops,
>> +     },
>> +};
>> +
>> +module_platform_driver(intel_lpss_acpi_driver);
>> +
>> +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
>> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
>> +MODULE_DESCRIPTION("Intel LPSS ACPI driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
>> new file mode 100644
>> index 0000000..fd8d25d
>> --- /dev/null
>> +++ b/drivers/mfd/intel-lpss-pci.c
>> @@ -0,0 +1,106 @@
>> +/*
>> + * Intel LPSS PCI support.
>> + *
>> + * Copyright (C) 2015, Intel Corporation
>> + *
>> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> + *          Mika Westerberg <mika.westerberg@linux.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/ioport.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/pci.h>
>> +#include <linux/pm.h>
>> +#include <linux/pm_runtime.h>
>> +
>> +#include "intel-lpss.h"
>> +
>> +static int intel_lpss_pci_probe(struct pci_dev *pdev,
>> +                             const struct pci_device_id *id)
>> +{
>> +     struct intel_lpss_platform_info *info;
>> +     int ret;
>> +
>> +     ret = pcim_enable_device(pdev);
>> +     if (ret)
>> +             return ret;
>> +
>> +     info = devm_kmemdup(&pdev->dev, (void *)id->driver_data, sizeof(*info),
>> +                         GFP_KERNEL);
>> +     if (!info)
>> +             return -ENOMEM;
>> +
>> +     info->mem = &pdev->resource[0];
>> +     info->irq = pdev->irq;
>> +
>> +     ret = intel_lpss_probe(&pdev->dev, info);
>> +     if (ret)
>> +             return ret;
>> +
>> +     pm_runtime_put(&pdev->dev);
>> +     return 0;
>> +}
>> +
>> +static void intel_lpss_pci_remove(struct pci_dev *pdev)
>> +{
>> +     pm_runtime_get_sync(&pdev->dev);
>> +     intel_lpss_remove(&pdev->dev);
>> +}
>> +
>> +static INTEL_LPSS_PM_OPS(intel_lpss_pci_pm_ops);
>> +
>> +static const struct intel_lpss_platform_info spt_info = {
>> +     .clk_rate = 120000000,
>> +};
>> +
>> +static const struct intel_lpss_platform_info spt_uart_info = {
>> +     .clk_rate = 120000000,
>> +     .clk_con_id = "baudclk",
>> +};
>> +
>> +static const struct pci_device_id intel_lpss_pci_ids[] = {
>> +     /* SPT-LP */
>> +     { PCI_VDEVICE(INTEL, 0x9d27), (kernel_ulong_t)&spt_uart_info },
>> +     { PCI_VDEVICE(INTEL, 0x9d28), (kernel_ulong_t)&spt_uart_info },
>> +     { PCI_VDEVICE(INTEL, 0x9d29), (kernel_ulong_t)&spt_info },
>> +     { PCI_VDEVICE(INTEL, 0x9d2a), (kernel_ulong_t)&spt_info },
>> +     { PCI_VDEVICE(INTEL, 0x9d60), (kernel_ulong_t)&spt_info },
>> +     { PCI_VDEVICE(INTEL, 0x9d61), (kernel_ulong_t)&spt_info },
>> +     { PCI_VDEVICE(INTEL, 0x9d62), (kernel_ulong_t)&spt_info },
>> +     { PCI_VDEVICE(INTEL, 0x9d63), (kernel_ulong_t)&spt_info },
>> +     { PCI_VDEVICE(INTEL, 0x9d64), (kernel_ulong_t)&spt_info },
>> +     { PCI_VDEVICE(INTEL, 0x9d65), (kernel_ulong_t)&spt_info },
>> +     { PCI_VDEVICE(INTEL, 0x9d66), (kernel_ulong_t)&spt_uart_info },
>> +     /* SPT-H */
>> +     { PCI_VDEVICE(INTEL, 0xa127), (kernel_ulong_t)&spt_uart_info },
>> +     { PCI_VDEVICE(INTEL, 0xa128), (kernel_ulong_t)&spt_uart_info },
>> +     { PCI_VDEVICE(INTEL, 0xa129), (kernel_ulong_t)&spt_info },
>> +     { PCI_VDEVICE(INTEL, 0xa12a), (kernel_ulong_t)&spt_info },
>> +     { PCI_VDEVICE(INTEL, 0xa160), (kernel_ulong_t)&spt_info },
>> +     { PCI_VDEVICE(INTEL, 0xa161), (kernel_ulong_t)&spt_info },
>> +     { PCI_VDEVICE(INTEL, 0xa166), (kernel_ulong_t)&spt_uart_info },
>> +     { }
>> +};
>> +MODULE_DEVICE_TABLE(pci, intel_lpss_pci_ids);
>> +
>> +static struct pci_driver intel_lpss_pci_driver = {
>> +     .name = "intel-lpss",
>> +     .id_table = intel_lpss_pci_ids,
>> +     .probe = intel_lpss_pci_probe,
>> +     .remove = intel_lpss_pci_remove,
>> +     .driver = {
>> +             .pm = &intel_lpss_pci_pm_ops,
>> +     },
>> +};
>> +
>> +module_pci_driver(intel_lpss_pci_driver);
>> +
>> +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
>> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
>> +MODULE_DESCRIPTION("Intel LPSS PCI driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
>> new file mode 100644
>> index 0000000..78d1921
>> --- /dev/null
>> +++ b/drivers/mfd/intel-lpss.c
>> @@ -0,0 +1,523 @@
>> +/*
>> + * Intel Sunrisepoint LPSS core support.
>> + *
>> + * Copyright (C) 2015, Intel Corporation
>> + *
>> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> + *          Mika Westerberg <mika.westerberg@linux.intel.com>
>> + *          Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> + *          Jarkko Nikula <jarkko.nikula@linux.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/clk.h>
>> +#include <linux/clkdev.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/debugfs.h>
>> +#include <linux/idr.h>
>> +#include <linux/ioport.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mfd/core.h>
>> +#include <linux/platform_data/dma-dw.h>
>> +#include <linux/pm_qos.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/seq_file.h>
>> +
>> +#include "intel-lpss.h"
>> +
>> +#define LPSS_DEV_OFFSET              0x000
>> +#define LPSS_DEV_SIZE                0x200
>> +#define LPSS_PRIV_OFFSET     0x200
>> +#define LPSS_PRIV_SIZE               0x100
>> +#define LPSS_IDMA_OFFSET     0x800
>> +#define LPSS_IDMA_SIZE               0x800
>> +
>> +/* Offsets from lpss->priv */
>> +#define LPSS_PRIV_RESETS             0x04
>> +#define LPSS_PRIV_RESETS_FUNC                BIT(2)
>> +#define LPSS_PRIV_RESETS_IDMA                0x3
>> +
>> +#define LPSS_PRIV_ACTIVELTR          0x10
>> +#define LPSS_PRIV_IDLELTR            0x14
>> +
>> +#define LPSS_PRIV_LTR_REQ            BIT(15)
>> +#define LPSS_PRIV_LTR_SCALE_MASK     0xc00
>> +#define LPSS_PRIV_LTR_SCALE_1US              0x800
>> +#define LPSS_PRIV_LTR_SCALE_32US     0xc00
>> +#define LPSS_PRIV_LTR_VALUE_MASK     0x3ff
>> +
>> +#define LPSS_PRIV_CLK_GATE           0x38
>> +#define LPSS_PRIV_CLK_GATE_IP_SHIFT  0
>> +#define LPSS_PRIV_CLK_GATE_IDMA_SHIFT        2
>> +#define LPSS_PRIV_CLK_GATE_EN_MASK   0x2 /* Dynamic gating disabled */
>> +
>> +#define LPSS_PRIV_CAPS                       0xfc
>> +#define LPSS_PRIV_CAPS_NO_IDMA               BIT(8)
>> +#define LPSS_PRIV_CAPS_TYPE_SHIFT    4
>> +#define LPSS_PRIV_CAPS_TYPE_MASK     (0xf << LPSS_PRIV_CAPS_TYPE_SHIFT)
>> +
>> +/* This matches the type field in CAPS register */
>> +enum intel_lpss_dev_type {
>> +     LPSS_DEV_I2C = 0,
>> +     LPSS_DEV_UART,
>> +     LPSS_DEV_SPI,
>> +};
>> +
>> +struct intel_lpss {
>> +     const struct intel_lpss_platform_info *info;
>> +     enum intel_lpss_dev_type type;
>> +     struct clk *clk;
>> +     struct clk_lookup **clocks;
>> +     unsigned int ndevs;
>> +     const struct mfd_cell *devs;
>> +     struct device *dev;
>> +     void __iomem *priv;
>> +     int devid;
>> +     u32 caps;
>> +     u32 active_ltr;
>> +     u32 idle_ltr;
>> +     struct dentry *debugfs;
>> +};
>> +
>> +static const struct dw_dma_platform_data intel_lpss_idma_pdata = {
>> +     .type = DW_DMAC_TYPE_INTEL,
>> +     .nr_channels = 2,
>> +     .is_private = true,
>> +     .block_size = 0x1ffff,
>> +     .nr_masters = 1,
>> +     .data_width = {2},
>
> .data_width = { 2 },

Will fix.

>
>> +};
>> +
>> +static const struct resource intel_lpss_dev_resources[] = {
>> +     DEFINE_RES_MEM_NAMED(LPSS_DEV_OFFSET, LPSS_DEV_SIZE, "lpss_dev"),
>> +     DEFINE_RES_MEM_NAMED(LPSS_PRIV_OFFSET, LPSS_PRIV_SIZE, "lpss_priv"),
>> +     DEFINE_RES_IRQ(0),
>> +};
>
> Why has this been called 'dev' and 'priv'?  Neither of which are
> particularlly helpful address names and I can't find them in the
> datasheet.  Why not 'base' and 'extended', or something?

dev stands for main device controller
priv -- for private space. The abbrevation is inherited from LPSS
drivers for BayTrail and Co.

In documentation priv space called 'additional registers'.

>
>> +static const struct resource intel_lpss_idma_resources[] = {
>> +     DEFINE_RES_MEM(LPSS_IDMA_OFFSET, LPSS_IDMA_SIZE),
>> +     DEFINE_RES_IRQ(0),
>> +};
>> +
>> +/*
>> + * Cells needs to be ordered so that the iDMA is created first. This is
>> + * because we need to be sure the DMA is available when the host controller
>> + * driver is probed.
>> + */
>> +static const struct mfd_cell intel_lpss_i2c_devs[] = {
>> +     {
>> +             .name = "dw_dmac",
>> +             .num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
>> +             .resources = intel_lpss_idma_resources,
>> +             .platform_data = (void *)&intel_lpss_idma_pdata,
>> +             .pdata_size = sizeof(intel_lpss_idma_pdata),
>> +     },
>> +     {
>> +             .name = "i2c_designware",
>> +             .num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
>> +             .resources = intel_lpss_dev_resources,
>> +     },
>> +};
>> +
>> +static const struct mfd_cell intel_lpss_uart_devs[] = {
>> +     {
>> +             .name = "dw_dmac",
>> +             .num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
>> +             .resources = intel_lpss_idma_resources,
>> +             .platform_data = (void *)&intel_lpss_idma_pdata,
>> +             .pdata_size = sizeof(intel_lpss_idma_pdata),
>> +     },
>> +     {
>> +             .name = "dw-apb-uart",
>> +             .num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
>> +             .resources = intel_lpss_dev_resources,
>> +     },
>> +};
>> +
>> +static const struct mfd_cell intel_lpss_spi_devs[] = {
>> +     {
>> +             .name = "dw_dmac",
>> +             .num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
>> +             .resources = intel_lpss_idma_resources,
>> +             .platform_data = (void *)&intel_lpss_idma_pdata,
>> +             .pdata_size = sizeof(intel_lpss_idma_pdata),
>> +     },
>> +     {
>> +             .name = "pxa2xx-spi",
>> +             .num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
>> +             .resources = intel_lpss_dev_resources,
>> +     },
>> +};
>> +
>> +static DEFINE_IDA(intel_lpss_devid_ida);
>
> What's wrong with using PLATFORM_DEVID_AUTO?

We have to provide right clocks.
Is it possible to use AUTO and be sure that right clocks will go to
corresponding consumers?

>
>> +static struct dentry *intel_lpss_debugfs;
>> +
>> +static void intel_lpss_cache_ltr(struct intel_lpss *lpss)
>> +{
>> +     lpss->active_ltr = readl(lpss->priv + LPSS_PRIV_ACTIVELTR);
>> +     lpss->idle_ltr = readl(lpss->priv + LPSS_PRIV_IDLELTR);
>> +}
>> +
>> +static int intel_lpss_debugfs_add(struct intel_lpss *lpss)
>> +{
>> +     struct dentry *dir;
>> +
>> +     dir = debugfs_create_dir(dev_name(lpss->dev), intel_lpss_debugfs);
>> +     if (IS_ERR(dir))
>> +             return PTR_ERR(dir);
>> +
>> +     /* Cache the values into lpss structure */
>> +     intel_lpss_cache_ltr(lpss);
>> +
>> +     debugfs_create_x32("capabilities", S_IRUGO, dir, &lpss->caps);
>> +     debugfs_create_x32("active_ltr", S_IRUGO, dir, &lpss->active_ltr);
>> +     debugfs_create_x32("idle_ltr", S_IRUGO, dir, &lpss->idle_ltr);
>> +
>> +     lpss->debugfs = dir;
>> +     return 0;
>> +}
>> +
>> +static void intel_lpss_debugfs_remove(struct intel_lpss *lpss)
>> +{
>> +     debugfs_remove_recursive(lpss->debugfs);
>> +}
>
> A comment here explaining what you're doing below wouldn't go amiss.

Ok.

>
>> +static void intel_lpss_ltr_set(struct device *dev, s32 val)
>> +{
>> +     struct intel_lpss *lpss = dev_get_drvdata(dev);
>> +     u32 ltr;
>> +
>> +     ltr = readl(lpss->priv + LPSS_PRIV_ACTIVELTR);
>> +
>> +     if (val == PM_QOS_LATENCY_ANY || val < 0) {
>> +             ltr &= ~LPSS_PRIV_LTR_REQ;
>> +     } else {
>> +             ltr |= LPSS_PRIV_LTR_REQ;
>> +             ltr &= ~LPSS_PRIV_LTR_SCALE_MASK;
>> +             ltr &= ~LPSS_PRIV_LTR_VALUE_MASK;
>> +
>> +             if (val > LPSS_PRIV_LTR_VALUE_MASK)
>> +                     ltr |= LPSS_PRIV_LTR_SCALE_32US | val >> 5;
>> +             else
>> +                     ltr |= LPSS_PRIV_LTR_SCALE_1US | val;
>> +     }
>> +
>> +     if (ltr == lpss->active_ltr)
>> +             return;
>> +
>> +     writel(ltr, lpss->priv + LPSS_PRIV_ACTIVELTR);
>> +     writel(ltr, lpss->priv + LPSS_PRIV_IDLELTR);
>> +
>> +     /* Cache the values into lpss structure */
>> +     intel_lpss_cache_ltr(lpss);
>> +}
>> +
>> +static void intel_lpss_ltr_add(struct intel_lpss *lpss)
>> +{
>> +     lpss->dev->power.set_latency_tolerance = intel_lpss_ltr_set;
>> +     dev_pm_qos_expose_latency_tolerance(lpss->dev);
>> +}
>> +
>> +static void intel_lpss_ltr_remove(struct intel_lpss *lpss)
>> +{
>> +     dev_pm_qos_hide_latency_tolerance(lpss->dev);
>> +     lpss->dev->power.set_latency_tolerance = NULL;
>> +}
>> +
>> +static int intel_lpss_assign_devs(struct intel_lpss *lpss)
>> +{
>> +     unsigned int type;
>> +
>> +     type = lpss->caps & LPSS_PRIV_CAPS_TYPE_MASK;
>> +     type >>= LPSS_PRIV_CAPS_TYPE_SHIFT;
>> +
>> +     switch (type) {
>> +     case LPSS_DEV_I2C:
>> +             lpss->devs = intel_lpss_i2c_devs;
>> +             break;
>> +     case LPSS_DEV_UART:
>> +             lpss->devs = intel_lpss_uart_devs;
>> +             break;
>> +     case LPSS_DEV_SPI:
>> +             lpss->devs = intel_lpss_spi_devs;
>> +             break;
>> +     default:
>> +             return -ENODEV;
>> +     }
>> +
>> +     lpss->type = type;
>> +
>> +     return 0;
>> +}
>> +
>> +static bool intel_lpss_has_idma(const struct intel_lpss *lpss)
>> +{
>> +     return (lpss->caps & LPSS_PRIV_CAPS_NO_IDMA) == 0;
>> +}
>> +
>> +static void intel_lpss_reset(const struct intel_lpss *lpss)
>
> Not very descriptive function name.  'de|assert'?

It actually handles the reset signal. I don't see how de/assert will be better.

>
>> +{
>> +     u32 value = LPSS_PRIV_RESETS_FUNC | LPSS_PRIV_RESETS_IDMA;
>> +
>> +     /* Bring out the device from reset */
>> +     writel(value, lpss->priv + LPSS_PRIV_RESETS);
>> +}
>
> All of this clock stuff below need s Clock Ack.

So, is anyone in your mind to whom we can Cc the new version?

>
>> +static void intel_lpss_unregister_clock_tree(struct clk *clk)
>> +{
>> +     struct clk *parent;
>> +
>> +     while (clk) {
>> +             parent = clk_get_parent(clk);
>> +             clk_unregister(clk);
>> +             clk = parent;
>> +     }
>> +}
>> +
>> +static int intel_lpss_register_clock_divider(struct intel_lpss *lpss,
>> +                                          const char *devname,
>> +                                          struct clk **clk)
>> +{
>> +     char name[32];
>> +     struct clk *tmp = *clk;
>> +
>> +     snprintf(name, sizeof(name), "%s-enable", devname);
>> +     tmp = clk_register_gate(NULL, name, __clk_get_name(tmp), 0,
>> +                             lpss->priv, 0, 0, NULL);
>> +     if (IS_ERR(tmp))
>> +             return PTR_ERR(tmp);
>> +
>> +     snprintf(name, sizeof(name), "%s-div", devname);
>> +     tmp = clk_register_fractional_divider(NULL, name, __clk_get_name(tmp),
>> +                                           0, lpss->priv, 1, 15, 16, 15, 0,
>> +                                           NULL);
>> +     if (IS_ERR(tmp))
>> +             return PTR_ERR(tmp);
>> +     *clk = tmp;
>> +
>> +     snprintf(name, sizeof(name), "%s-update", devname);
>> +     tmp = clk_register_gate(NULL, name, __clk_get_name(tmp),
>> +                             CLK_SET_RATE_PARENT, lpss->priv, 31, 0, NULL);
>> +     if (IS_ERR(tmp))
>> +             return PTR_ERR(tmp);
>> +     *clk = tmp;
>> +
>> +     return 0;
>> +}
>> +
>> +static int intel_lpss_register_clock(struct intel_lpss *lpss)
>> +{
>> +     const struct mfd_cell *devs = lpss->devs;
>> +     struct clk_lookup **clocks;
>> +     struct clk *clk, *root;
>> +     char devname[24];
>> +     int ret = -ENOMEM;
>> +
>> +     if (!lpss->info->clk_rate)
>> +             return 0;
>> +
>> +     clocks = devm_kcalloc(lpss->dev, lpss->ndevs, sizeof(*clocks),
>> +                           GFP_KERNEL);
>> +     if (!clocks)
>> +             return -ENOMEM;
>> +
>> +     /* Root clock */
>> +     root = clk_register_fixed_rate(NULL, dev_name(lpss->dev), NULL,
>> +                                    CLK_IS_ROOT, lpss->info->clk_rate);
>> +     if (IS_ERR(root))
>> +             return PTR_ERR(root);
>> +
>> +     clk = root;
>> +
>> +     snprintf(devname, sizeof(devname), "%s.%d", devs[1].name, lpss->devid);
>> +
>> +     /*
>> +      * Support for clock divider only if it has some preset value.
>> +      * Otherwise we assume that the divider is not used.
>> +      */
>> +     if (lpss->type != LPSS_DEV_I2C) {
>> +             ret = intel_lpss_register_clock_divider(lpss, devname, &clk);
>> +             if (ret)
>> +                     goto err_clk_register;
>> +     }
>> +
>> +     /* Clock for the host controller */
>> +     clocks[0] = clk_register_clkdev(clk, lpss->info->clk_con_id, "%s",
>> +                                     devname);
>> +     if (IS_ERR(clocks[0]))
>> +             goto err_clk_register;
>> +
>> +     /*
>> +      * If the slice has integrated DMA block create clock for that as
>> +      * well now.
>> +      */
>> +     if (lpss->ndevs > 1) {
>> +             clocks[1] = clk_register_clkdev(root, "hclk", "%s.%d",
>> +                                             devs[0].name, lpss->devid);
>> +             if (IS_ERR(clocks[1]))
>> +                     goto err_hclk_register;
>> +     }
>> +
>> +     lpss->clocks = clocks;
>> +     lpss->clk = clk;
>> +
>> +     return 0;
>> +
>> +err_hclk_register:
>> +     clkdev_drop(clocks[0]);
>> +
>> +err_clk_register:
>> +     intel_lpss_unregister_clock_tree(clk);
>> +
>> +     return ret;
>> +}
>> +
>> +static void intel_lpss_unregister_clock(struct intel_lpss *lpss)
>> +{
>> +     unsigned int i;
>> +
>> +     if (IS_ERR_OR_NULL(lpss->clk))
>> +             return;
>> +
>> +     for (i = 0; i < lpss->ndevs; i++)
>> +             clkdev_drop(lpss->clocks[i]);
>> +
>> +     intel_lpss_unregister_clock_tree(lpss->clk);
>> +}
>> +
>> +int intel_lpss_probe(struct device *dev,
>> +                  const struct intel_lpss_platform_info *info)
>> +{
>> +     const struct mfd_cell *devs;
>> +     struct intel_lpss *lpss;
>> +     struct resource priv;
>> +     int ret;
>> +
>> +     if (!info || !info->mem || info->irq <= 0)
>> +             return -EINVAL;
>> +
>> +     lpss = devm_kzalloc(dev, sizeof(*lpss), GFP_KERNEL);
>> +     if (!lpss)
>> +             return -ENOMEM;
>> +
>> +     memset(&priv, 0, sizeof(priv));
>> +     priv.start = info->mem->start + LPSS_PRIV_OFFSET;
>> +     priv.end = priv.start + LPSS_PRIV_SIZE - 1;
>> +     priv.flags = IORESOURCE_MEM;
>> +
>> +     lpss->priv = devm_ioremap_resource(dev, &priv);
>> +     if (!lpss->priv)
>> +             return -ENOMEM;
>> +
>> +     intel_lpss_reset(lpss);
>> +
>> +     lpss->info = info;
>> +     lpss->dev = dev;
>> +     lpss->caps = readl(lpss->priv + LPSS_PRIV_CAPS);
>> +     lpss->ndevs = intel_lpss_has_idma(lpss) ? 2 : 1;
>
> This is fragile.
>
> I'd prefer you dynamically allocate resources here, instead of
> statically up the top then 'jumping' over the ones you don't need.

(1)

>
>> +     dev_set_drvdata(dev, lpss);
>> +
>> +     ret = intel_lpss_assign_devs(lpss);
>> +     if (ret < 0)
>> +             return ret;
>
> Is ret allowed to be >0?  If not just 'if (ret)'.

It might be a leftover when we previously returned a type of the
device. Will amend.

>
>> +     lpss->devid = ida_simple_get(&intel_lpss_devid_ida, 0, 0, GFP_KERNEL);
>> +     if (lpss->devid < 0)
>> +             return lpss->devid;
>
> PLATFORM_DEVID_AUTO does this for you.

What about clock matching?

>
>> +     ret = intel_lpss_register_clock(lpss);
>> +     if (ret < 0)
>> +             goto err_clk_register;
>> +
>> +     intel_lpss_ltr_add(lpss);
>
> Add what, to what?  What is 'ltr'?

LTR stands for Latency Tolerance Reporting, the PCI property.

https://www.pcisig.com/specifications/pciexpress/specifications/ECN_LatencyTolnReporting_14Aug08.pdf

>
>> +     ret = intel_lpss_debugfs_add(lpss);
>> +     if (ret)
>> +             dev_warn(lpss->dev, "Failed to create debugfs entries\n");
>> +
>> +     devs = intel_lpss_has_idma(lpss) ? lpss->devs : lpss->devs + 1;
>
> *cringe*

Here the comment also for (1).

In case of DMA IP present on compound device we have to register it
first, otherwise main device driver may miss that (SPI checks DMA on
->probe() stage). Unfortunately there is no possibility to tell MFD
framework the order of the device initialization except the direct
order in the array. That's why this solution was implemented. If you
have any better idea, please, share.

>
>> +     ret = mfd_add_devices(dev, lpss->devid, devs, lpss->ndevs,
>> +                           info->mem, info->irq, NULL);
>> +     if (ret < 0)
>> +             goto err_remove_ltr;
>> +
>> +     return 0;
>> +
>> +err_remove_ltr:
>> +     intel_lpss_debugfs_remove(lpss);
>> +     intel_lpss_ltr_remove(lpss);
>> +
>> +err_clk_register:
>> +     ida_simple_remove(&intel_lpss_devid_ida, lpss->devid);
>> +
>> +     return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(intel_lpss_probe);
>> +
>> +void intel_lpss_remove(struct device *dev)
>> +{
>> +     struct intel_lpss *lpss = dev_get_drvdata(dev);
>> +
>> +     mfd_remove_devices(dev);
>> +     intel_lpss_debugfs_remove(lpss);
>> +     intel_lpss_ltr_remove(lpss);
>> +     intel_lpss_unregister_clock(lpss);
>> +     ida_simple_rnemove(&intel_lpss_devid_ida, lpss->devid);
>> +}
>> +EXPORT_SYMBOL_GPL(intel_lpss_remove);
>> +
>> +static int resume_lpss_device(struct device *dev, void *data)
>> +{
>> +     pm_runtime_resume(dev);
>> +     return 0;
>> +}
>> +
>> +int intel_lpss_prepare(struct device *dev)
>> +{
>> +     /*
>> +      * Resume both child devices before entering system sleep. This
>> +      * ensures that they are in proper state before they get suspended.
>> +      */
>> +     device_for_each_child(dev, NULL, resume_lpss_device);
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(intel_lpss_prepare);
>> +
>> +int intel_lpss_suspend(struct device *dev)
>> +{
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(intel_lpss_suspend);
>> +
>> +int intel_lpss_resume(struct device *dev)
>> +{
>> +     struct intel_lpss *lpss = dev_get_drvdata(dev);
>> +     intel_lpss_reset(lpss);
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(intel_lpss_resume);
>> +
>> +static int __init intel_lpss_init(void)
>> +{
>> +     intel_lpss_debugfs = debugfs_create_dir("intel_lpss", NULL);
>> +     return 0;
>> +}
>> +module_init(intel_lpss_init);
>> +
>> +static void __exit intel_lpss_exit(void)
>> +{
>> +     debugfs_remove(intel_lpss_debugfs);
>> +}
>> +module_exit(intel_lpss_exit);
>> +
>> +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
>> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
>> +MODULE_AUTHOR("Heikki Krogerus <heikki.krogerus@linux.intel.com>");
>> +MODULE_AUTHOR("Jarkko Nikula <jarkko.nikula@linux.intel.com>");
>> +MODULE_DESCRIPTION("Intel LPSS core driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
>> new file mode 100644
>> index 0000000..f28cb28
>> --- /dev/null
>> +++ b/drivers/mfd/intel-lpss.h
>> @@ -0,0 +1,62 @@
>> +/*
>> + * Intel LPSS core support.
>> + *
>> + * Copyright (C) 2015, Intel Corporation
>> + *
>> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> + *          Mika Westerberg <mika.westerberg@linux.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 __MFD_INTEL_LPSS_H
>> +#define __MFD_INTEL_LPSS_H
>> +
>> +struct device;
>> +struct resource;
>> +
>> +struct intel_lpss_platform_info {
>> +     struct resource *mem;
>> +     int irq;
>> +     unsigned long clk_rate;
>> +     const char *clk_con_id;
>> +};
>> +
>> +int intel_lpss_probe(struct device *dev,
>> +                  const struct intel_lpss_platform_info *info);
>> +void intel_lpss_remove(struct device *dev);
>> +
>> +#ifdef CONFIG_PM
>> +int intel_lpss_prepare(struct device *dev);
>> +int intel_lpss_suspend(struct device *dev);
>> +int intel_lpss_resume(struct device *dev);
>
> Why are you exporting these?

LPSS devices are located in its own power domain. That's why we do all this.

>
>> +#ifdef CONFIG_PM_SLEEP
>> +#define INTEL_LPSS_SLEEP_PM_OPS                      \
>> +     .prepare = intel_lpss_prepare,          \
>> +     .suspend = intel_lpss_suspend,          \
>> +     .resume = intel_lpss_resume,            \
>> +     .freeze = intel_lpss_suspend,           \
>> +     .thaw = intel_lpss_resume,              \
>> +     .poweroff = intel_lpss_suspend,         \
>> +     .restore = intel_lpss_resume,
>> +#endif
>
> SET_SYSTEM_SLEEP_PM_OPS?
>
>> +#define INTEL_LPSS_RUNTIME_PM_OPS            \
>> +     .runtime_suspend = intel_lpss_suspend,  \
>> +     .runtime_resume = intel_lpss_resume,
>
> SET_RUNTIME_PM_OPS
>
>> +#else /* !CONFIG_PM */
>> +#define INTEL_LPSS_SLEEP_PM_OPS
>> +#define INTEL_LPSS_RUNTIME_PM_OPS
>> +#endif /* CONFIG_PM */
>> +
>> +#define INTEL_LPSS_PM_OPS(name)                      \
>> +const struct dev_pm_ops name = {             \
>> +     INTEL_LPSS_SLEEP_PM_OPS                 \
>> +     INTEL_LPSS_RUNTIME_PM_OPS               \
>> +}
>
> Why is this in the header?
>
>> +#endif /* __MFD_INTEL_LPSS_H */



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 3/3] mfd: Add support for Intel Sunrisepoint LPSS devices

[Lee Jones]

On Wed, 29 Apr 2015, Andy Shevchenko wrote:

> On Wed, Apr 29, 2015 at 11:23 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Tue, 31 Mar 2015, Andy Shevchenko wrote:
> 
> Thanks for review. My comments / answers below.
> 
> >> The new coming Intel platforms such as Skylake will contain Sunrisepoint PCH.
> >
> > Can you use the same naming conventions as the Skylake's predecessors?
> >
> > Currently lpc_ich and lpc_sch exist.
> 
> As far as I understand it is not an LPC bus, thus seems above is
> irrelevant. Moreover, PCH in Skylake is something new if looking into
> hardware.

Perhaps the aforementioned files just need 'intel_' appended then.

> >> The main difference to the previous platforms is that the LPSS devices are
> >> compound devices where usually main (SPI, HSUART, or I2C) and DMA IPs are
> >> present.
> >>
> >> This patch brings the driver for such devices found on Sunrisepoint PCH.
> >>
> >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> ---
> >>  drivers/mfd/Kconfig           |  24 ++
> >>  drivers/mfd/Makefile          |   3 +
> >>  drivers/mfd/intel-lpss-acpi.c |  84 +++++++
> >>  drivers/mfd/intel-lpss-pci.c  | 106 +++++++++
> >>  drivers/mfd/intel-lpss.c      | 523 ++++++++++++++++++++++++++++++++++++++++++
> >>  drivers/mfd/intel-lpss.h      |  62 +++++
> >
> > Can you move this to include/?
> 
> What the point? I don't see the users for that now or in the nearest future.

The point is to keep drivers/mfd/ clean rather than to share the header
with external entities.

> >>  6 files changed, 802 insertions(+)
> >>  create mode 100644 drivers/mfd/intel-lpss-acpi.c
> >>  create mode 100644 drivers/mfd/intel-lpss-pci.c
> >>  create mode 100644 drivers/mfd/intel-lpss.c
> >>  create mode 100644 drivers/mfd/intel-lpss.h
> >>
> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> >> index d5ad04d..b1a6778 100644
> >> --- a/drivers/mfd/Kconfig
> >> +++ b/drivers/mfd/Kconfig
> >> @@ -325,6 +325,30 @@ config INTEL_SOC_PMIC
> >>         thermal, charger and related power management functions
> >>         on these systems.
> >>
> >> +config MFD_INTEL_LPSS
> >> +     tristate "Intel Low Power Subsystem support"
> >> +     depends on X86
> >> +     select COMMON_CLK
> >> +     select MFD_CORE
> >> +     help
> >> +       This driver provides necessary plumbing for Intel Low Power
> >> +       Subsystem (LPSS) devices such as I2C, SPI and HS-UART starting
> >> +       from Intel Sunrisepoint (Intel Skylake PCH) and newer chipsets.
> >> +
> >> +config MFD_INTEL_LPSS_ACPI
> >> +     tristate "Intel Low Power Subsystem support in ACPI mode"
> >> +     depends on MFD_INTEL_LPSS && ACPI
> >> +     help
> >> +       This driver support Intel Low Power Subsystem devices in ACPI
> >> +       mode.
> >> +
> >> +config MFD_INTEL_LPSS_PCI
> >> +     tristate "Intel Low Power Subsystem support in PCI mode"
> >> +     depends on MFD_INTEL_LPSS && PCI
> >> +     help
> >> +       This driver support Intel Low Power Subsystem devices in PCI
> >> +       mode.
> >> +
> >>  config MFD_INTEL_MSIC
> >>       bool "Intel MSIC"
> >>       depends on INTEL_SCU_IPC
> >> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> >> index 0e5cfeb..cdf29b9 100644
> >> --- a/drivers/mfd/Makefile
> >> +++ b/drivers/mfd/Makefile
> >> @@ -161,6 +161,9 @@ obj-$(CONFIG_TPS65911_COMPARATOR) += tps65911-comparator.o
> >>  obj-$(CONFIG_MFD_TPS65090)   += tps65090.o
> >>  obj-$(CONFIG_MFD_AAT2870_CORE)       += aat2870-core.o
> >>  obj-$(CONFIG_MFD_ATMEL_HLCDC)        += atmel-hlcdc.o
> >> +obj-$(CONFIG_MFD_INTEL_LPSS) += intel-lpss.o
> >> +obj-$(CONFIG_MFD_INTEL_LPSS_PCI)     += intel-lpss-pci.o
> >> +obj-$(CONFIG_MFD_INTEL_LPSS_ACPI)    += intel-lpss-acpi.o
> >>  obj-$(CONFIG_MFD_INTEL_MSIC) += intel_msic.o
> >>  obj-$(CONFIG_MFD_PALMAS)     += palmas.o
> >>  obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
> >> diff --git a/drivers/mfd/intel-lpss-acpi.c b/drivers/mfd/intel-lpss-acpi.c
> >> new file mode 100644
> >> index 0000000..0d92d73
> >> --- /dev/null
> >> +++ b/drivers/mfd/intel-lpss-acpi.c
> >> @@ -0,0 +1,84 @@
> >> +/*
> >> + * Intel LPSS ACPI support.
> >> + *
> >> + * Copyright (C) 2015, Intel Corporation
> >> + *
> >> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> + *          Mika Westerberg <mika.westerberg@linux.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/ioport.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/module.h>
> >> +#include <linux/pm.h>
> >> +#include <linux/pm_runtime.h>
> >> +#include <linux/platform_device.h>
> >> +
> >> +#include "intel-lpss.h"
> >> +
> >> +static const struct intel_lpss_platform_info spt_info = {
> >> +     .clk_rate = 120000000,
> >> +};
> >> +
> >> +static const struct acpi_device_id intel_lpss_acpi_ids[] = {
> >> +     /* SPT */
> >> +     { "INT3446", (kernel_ulong_t)&spt_info },
> >> +     { "INT3447", (kernel_ulong_t)&spt_info },
> >> +     { }
> >> +};
> >> +MODULE_DEVICE_TABLE(acpi, intel_lpss_acpi_ids);
> >> +
> >> +static int intel_lpss_acpi_probe(struct platform_device *pdev)
> >> +{
> >> +     struct intel_lpss_platform_info *info;
> >> +     const struct acpi_device_id *id;
> >> +
> >> +     id = acpi_match_device(intel_lpss_acpi_ids, &pdev->dev);
> >> +     if (!id)
> >> +             return -ENODEV;
> >> +
> >> +     info = devm_kmemdup(&pdev->dev, (void *)id->driver_data, sizeof(*info),
> >> +                         GFP_KERNEL);
> >> +     if (!info)
> >> +             return -ENOMEM;
> >> +
> >> +     info->mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> +     info->irq = platform_get_irq(pdev, 0);
> >> +
> >> +     pm_runtime_set_active(&pdev->dev);
> >> +     pm_runtime_enable(&pdev->dev);
> >> +
> >> +     return intel_lpss_probe(&pdev->dev, info);
> >> +}
> >> +
> >> +static int intel_lpss_acpi_remove(struct platform_device *pdev)
> >> +{
> >> +     intel_lpss_remove(&pdev->dev);
> >> +     pm_runtime_disable(&pdev->dev);
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +static INTEL_LPSS_PM_OPS(intel_lpss_acpi_pm_ops);
> >> +
> >> +static struct platform_driver intel_lpss_acpi_driver = {
> >> +     .probe = intel_lpss_acpi_probe,
> >> +     .remove = intel_lpss_acpi_remove,
> >> +     .driver = {
> >> +             .name = "intel-lpss",
> >> +             .acpi_match_table = intel_lpss_acpi_ids,
> >> +             .pm = &intel_lpss_acpi_pm_ops,
> >> +     },
> >> +};
> >> +
> >> +module_platform_driver(intel_lpss_acpi_driver);
> >> +
> >> +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
> >> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
> >> +MODULE_DESCRIPTION("Intel LPSS ACPI driver");
> >> +MODULE_LICENSE("GPL v2");
> >> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
> >> new file mode 100644
> >> index 0000000..fd8d25d
> >> --- /dev/null
> >> +++ b/drivers/mfd/intel-lpss-pci.c
> >> @@ -0,0 +1,106 @@
> >> +/*
> >> + * Intel LPSS PCI support.
> >> + *
> >> + * Copyright (C) 2015, Intel Corporation
> >> + *
> >> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> + *          Mika Westerberg <mika.westerberg@linux.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/ioport.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/module.h>
> >> +#include <linux/pci.h>
> >> +#include <linux/pm.h>
> >> +#include <linux/pm_runtime.h>
> >> +
> >> +#include "intel-lpss.h"
> >> +
> >> +static int intel_lpss_pci_probe(struct pci_dev *pdev,
> >> +                             const struct pci_device_id *id)
> >> +{
> >> +     struct intel_lpss_platform_info *info;
> >> +     int ret;
> >> +
> >> +     ret = pcim_enable_device(pdev);
> >> +     if (ret)
> >> +             return ret;
> >> +
> >> +     info = devm_kmemdup(&pdev->dev, (void *)id->driver_data, sizeof(*info),
> >> +                         GFP_KERNEL);
> >> +     if (!info)
> >> +             return -ENOMEM;
> >> +
> >> +     info->mem = &pdev->resource[0];
> >> +     info->irq = pdev->irq;
> >> +
> >> +     ret = intel_lpss_probe(&pdev->dev, info);
> >> +     if (ret)
> >> +             return ret;
> >> +
> >> +     pm_runtime_put(&pdev->dev);
> >> +     return 0;
> >> +}
> >> +
> >> +static void intel_lpss_pci_remove(struct pci_dev *pdev)
> >> +{
> >> +     pm_runtime_get_sync(&pdev->dev);
> >> +     intel_lpss_remove(&pdev->dev);
> >> +}
> >> +
> >> +static INTEL_LPSS_PM_OPS(intel_lpss_pci_pm_ops);
> >> +
> >> +static const struct intel_lpss_platform_info spt_info = {
> >> +     .clk_rate = 120000000,
> >> +};
> >> +
> >> +static const struct intel_lpss_platform_info spt_uart_info = {
> >> +     .clk_rate = 120000000,
> >> +     .clk_con_id = "baudclk",
> >> +};
> >> +
> >> +static const struct pci_device_id intel_lpss_pci_ids[] = {
> >> +     /* SPT-LP */
> >> +     { PCI_VDEVICE(INTEL, 0x9d27), (kernel_ulong_t)&spt_uart_info },
> >> +     { PCI_VDEVICE(INTEL, 0x9d28), (kernel_ulong_t)&spt_uart_info },
> >> +     { PCI_VDEVICE(INTEL, 0x9d29), (kernel_ulong_t)&spt_info },
> >> +     { PCI_VDEVICE(INTEL, 0x9d2a), (kernel_ulong_t)&spt_info },
> >> +     { PCI_VDEVICE(INTEL, 0x9d60), (kernel_ulong_t)&spt_info },
> >> +     { PCI_VDEVICE(INTEL, 0x9d61), (kernel_ulong_t)&spt_info },
> >> +     { PCI_VDEVICE(INTEL, 0x9d62), (kernel_ulong_t)&spt_info },
> >> +     { PCI_VDEVICE(INTEL, 0x9d63), (kernel_ulong_t)&spt_info },
> >> +     { PCI_VDEVICE(INTEL, 0x9d64), (kernel_ulong_t)&spt_info },
> >> +     { PCI_VDEVICE(INTEL, 0x9d65), (kernel_ulong_t)&spt_info },
> >> +     { PCI_VDEVICE(INTEL, 0x9d66), (kernel_ulong_t)&spt_uart_info },
> >> +     /* SPT-H */
> >> +     { PCI_VDEVICE(INTEL, 0xa127), (kernel_ulong_t)&spt_uart_info },
> >> +     { PCI_VDEVICE(INTEL, 0xa128), (kernel_ulong_t)&spt_uart_info },
> >> +     { PCI_VDEVICE(INTEL, 0xa129), (kernel_ulong_t)&spt_info },
> >> +     { PCI_VDEVICE(INTEL, 0xa12a), (kernel_ulong_t)&spt_info },
> >> +     { PCI_VDEVICE(INTEL, 0xa160), (kernel_ulong_t)&spt_info },
> >> +     { PCI_VDEVICE(INTEL, 0xa161), (kernel_ulong_t)&spt_info },
> >> +     { PCI_VDEVICE(INTEL, 0xa166), (kernel_ulong_t)&spt_uart_info },
> >> +     { }
> >> +};
> >> +MODULE_DEVICE_TABLE(pci, intel_lpss_pci_ids);
> >> +
> >> +static struct pci_driver intel_lpss_pci_driver = {
> >> +     .name = "intel-lpss",
> >> +     .id_table = intel_lpss_pci_ids,
> >> +     .probe = intel_lpss_pci_probe,
> >> +     .remove = intel_lpss_pci_remove,
> >> +     .driver = {
> >> +             .pm = &intel_lpss_pci_pm_ops,
> >> +     },
> >> +};
> >> +
> >> +module_pci_driver(intel_lpss_pci_driver);
> >> +
> >> +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
> >> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
> >> +MODULE_DESCRIPTION("Intel LPSS PCI driver");
> >> +MODULE_LICENSE("GPL v2");
> >> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> >> new file mode 100644
> >> index 0000000..78d1921
> >> --- /dev/null
> >> +++ b/drivers/mfd/intel-lpss.c
> >> @@ -0,0 +1,523 @@
> >> +/*
> >> + * Intel Sunrisepoint LPSS core support.
> >> + *
> >> + * Copyright (C) 2015, Intel Corporation
> >> + *
> >> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> + *          Mika Westerberg <mika.westerberg@linux.intel.com>
> >> + *          Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >> + *          Jarkko Nikula <jarkko.nikula@linux.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/clk.h>
> >> +#include <linux/clkdev.h>
> >> +#include <linux/clk-provider.h>
> >> +#include <linux/debugfs.h>
> >> +#include <linux/idr.h>
> >> +#include <linux/ioport.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/module.h>
> >> +#include <linux/mfd/core.h>
> >> +#include <linux/platform_data/dma-dw.h>
> >> +#include <linux/pm_qos.h>
> >> +#include <linux/pm_runtime.h>
> >> +#include <linux/seq_file.h>
> >> +
> >> +#include "intel-lpss.h"
> >> +
> >> +#define LPSS_DEV_OFFSET              0x000
> >> +#define LPSS_DEV_SIZE                0x200
> >> +#define LPSS_PRIV_OFFSET     0x200
> >> +#define LPSS_PRIV_SIZE               0x100
> >> +#define LPSS_IDMA_OFFSET     0x800
> >> +#define LPSS_IDMA_SIZE               0x800
> >> +
> >> +/* Offsets from lpss->priv */
> >> +#define LPSS_PRIV_RESETS             0x04
> >> +#define LPSS_PRIV_RESETS_FUNC                BIT(2)
> >> +#define LPSS_PRIV_RESETS_IDMA                0x3
> >> +
> >> +#define LPSS_PRIV_ACTIVELTR          0x10
> >> +#define LPSS_PRIV_IDLELTR            0x14
> >> +
> >> +#define LPSS_PRIV_LTR_REQ            BIT(15)
> >> +#define LPSS_PRIV_LTR_SCALE_MASK     0xc00
> >> +#define LPSS_PRIV_LTR_SCALE_1US              0x800
> >> +#define LPSS_PRIV_LTR_SCALE_32US     0xc00
> >> +#define LPSS_PRIV_LTR_VALUE_MASK     0x3ff
> >> +
> >> +#define LPSS_PRIV_CLK_GATE           0x38
> >> +#define LPSS_PRIV_CLK_GATE_IP_SHIFT  0
> >> +#define LPSS_PRIV_CLK_GATE_IDMA_SHIFT        2
> >> +#define LPSS_PRIV_CLK_GATE_EN_MASK   0x2 /* Dynamic gating disabled */
> >> +
> >> +#define LPSS_PRIV_CAPS                       0xfc
> >> +#define LPSS_PRIV_CAPS_NO_IDMA               BIT(8)
> >> +#define LPSS_PRIV_CAPS_TYPE_SHIFT    4
> >> +#define LPSS_PRIV_CAPS_TYPE_MASK     (0xf << LPSS_PRIV_CAPS_TYPE_SHIFT)
> >> +
> >> +/* This matches the type field in CAPS register */
> >> +enum intel_lpss_dev_type {
> >> +     LPSS_DEV_I2C = 0,
> >> +     LPSS_DEV_UART,
> >> +     LPSS_DEV_SPI,
> >> +};
> >> +
> >> +struct intel_lpss {
> >> +     const struct intel_lpss_platform_info *info;
> >> +     enum intel_lpss_dev_type type;
> >> +     struct clk *clk;
> >> +     struct clk_lookup **clocks;
> >> +     unsigned int ndevs;
> >> +     const struct mfd_cell *devs;
> >> +     struct device *dev;
> >> +     void __iomem *priv;
> >> +     int devid;
> >> +     u32 caps;
> >> +     u32 active_ltr;
> >> +     u32 idle_ltr;
> >> +     struct dentry *debugfs;
> >> +};
> >> +
> >> +static const struct dw_dma_platform_data intel_lpss_idma_pdata = {
> >> +     .type = DW_DMAC_TYPE_INTEL,
> >> +     .nr_channels = 2,
> >> +     .is_private = true,
> >> +     .block_size = 0x1ffff,
> >> +     .nr_masters = 1,
> >> +     .data_width = {2},
> >
> > .data_width = { 2 },
> 
> Will fix.
> 
> >
> >> +};
> >> +
> >> +static const struct resource intel_lpss_dev_resources[] = {
> >> +     DEFINE_RES_MEM_NAMED(LPSS_DEV_OFFSET, LPSS_DEV_SIZE, "lpss_dev"),
> >> +     DEFINE_RES_MEM_NAMED(LPSS_PRIV_OFFSET, LPSS_PRIV_SIZE, "lpss_priv"),
> >> +     DEFINE_RES_IRQ(0),
> >> +};
> >
> > Why has this been called 'dev' and 'priv'?  Neither of which are
> > particularlly helpful address names and I can't find them in the
> > datasheet.  Why not 'base' and 'extended', or something?
> 
> dev stands for main device controller
> priv -- for private space. The abbrevation is inherited from LPSS
> drivers for BayTrail and Co.
> 
> In documentation priv space called 'additional registers'.

In my mind 'additional registers' != 'private registers'.

> >> +static const struct resource intel_lpss_idma_resources[] = {
> >> +     DEFINE_RES_MEM(LPSS_IDMA_OFFSET, LPSS_IDMA_SIZE),
> >> +     DEFINE_RES_IRQ(0),
> >> +};
> >> +
> >> +/*
> >> + * Cells needs to be ordered so that the iDMA is created first. This is
> >> + * because we need to be sure the DMA is available when the host controller
> >> + * driver is probed.
> >> + */
> >> +static const struct mfd_cell intel_lpss_i2c_devs[] = {
> >> +     {
> >> +             .name = "dw_dmac",
> >> +             .num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
> >> +             .resources = intel_lpss_idma_resources,
> >> +             .platform_data = (void *)&intel_lpss_idma_pdata,
> >> +             .pdata_size = sizeof(intel_lpss_idma_pdata),
> >> +     },
> >> +     {
> >> +             .name = "i2c_designware",
> >> +             .num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
> >> +             .resources = intel_lpss_dev_resources,
> >> +     },
> >> +};
> >> +
> >> +static const struct mfd_cell intel_lpss_uart_devs[] = {
> >> +     {
> >> +             .name = "dw_dmac",
> >> +             .num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
> >> +             .resources = intel_lpss_idma_resources,
> >> +             .platform_data = (void *)&intel_lpss_idma_pdata,
> >> +             .pdata_size = sizeof(intel_lpss_idma_pdata),
> >> +     },
> >> +     {
> >> +             .name = "dw-apb-uart",
> >> +             .num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
> >> +             .resources = intel_lpss_dev_resources,
> >> +     },
> >> +};
> >> +
> >> +static const struct mfd_cell intel_lpss_spi_devs[] = {
> >> +     {
> >> +             .name = "dw_dmac",
> >> +             .num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
> >> +             .resources = intel_lpss_idma_resources,
> >> +             .platform_data = (void *)&intel_lpss_idma_pdata,
> >> +             .pdata_size = sizeof(intel_lpss_idma_pdata),
> >> +     },
> >> +     {
> >> +             .name = "pxa2xx-spi",
> >> +             .num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
> >> +             .resources = intel_lpss_dev_resources,
> >> +     },
> >> +};
> >> +
> >> +static DEFINE_IDA(intel_lpss_devid_ida);
> >
> > What's wrong with using PLATFORM_DEVID_AUTO?
> 
> We have to provide right clocks.
> Is it possible to use AUTO and be sure that right clocks will go to
> corresponding consumers?

I guess Mike will have to help us with that, as I've never seen it
done this way before.

I guess you need to create a clock driver and register it properly as
a platform device.

> >> +static struct dentry *intel_lpss_debugfs;
> >> +
> >> +static void intel_lpss_cache_ltr(struct intel_lpss *lpss)
> >> +{
> >> +     lpss->active_ltr = readl(lpss->priv + LPSS_PRIV_ACTIVELTR);
> >> +     lpss->idle_ltr = readl(lpss->priv + LPSS_PRIV_IDLELTR);
> >> +}
> >> +
> >> +static int intel_lpss_debugfs_add(struct intel_lpss *lpss)
> >> +{
> >> +     struct dentry *dir;
> >> +
> >> +     dir = debugfs_create_dir(dev_name(lpss->dev), intel_lpss_debugfs);
> >> +     if (IS_ERR(dir))
> >> +             return PTR_ERR(dir);
> >> +
> >> +     /* Cache the values into lpss structure */
> >> +     intel_lpss_cache_ltr(lpss);
> >> +
> >> +     debugfs_create_x32("capabilities", S_IRUGO, dir, &lpss->caps);
> >> +     debugfs_create_x32("active_ltr", S_IRUGO, dir, &lpss->active_ltr);
> >> +     debugfs_create_x32("idle_ltr", S_IRUGO, dir, &lpss->idle_ltr);
> >> +
> >> +     lpss->debugfs = dir;
> >> +     return 0;
> >> +}
> >> +
> >> +static void intel_lpss_debugfs_remove(struct intel_lpss *lpss)
> >> +{
> >> +     debugfs_remove_recursive(lpss->debugfs);
> >> +}
> >
> > A comment here explaining what you're doing below wouldn't go amiss.
> 
> Ok.
> 
> >
> >> +static void intel_lpss_ltr_set(struct device *dev, s32 val)
> >> +{
> >> +     struct intel_lpss *lpss = dev_get_drvdata(dev);
> >> +     u32 ltr;
> >> +
> >> +     ltr = readl(lpss->priv + LPSS_PRIV_ACTIVELTR);
> >> +
> >> +     if (val == PM_QOS_LATENCY_ANY || val < 0) {
> >> +             ltr &= ~LPSS_PRIV_LTR_REQ;
> >> +     } else {
> >> +             ltr |= LPSS_PRIV_LTR_REQ;
> >> +             ltr &= ~LPSS_PRIV_LTR_SCALE_MASK;
> >> +             ltr &= ~LPSS_PRIV_LTR_VALUE_MASK;
> >> +
> >> +             if (val > LPSS_PRIV_LTR_VALUE_MASK)
> >> +                     ltr |= LPSS_PRIV_LTR_SCALE_32US | val >> 5;
> >> +             else
> >> +                     ltr |= LPSS_PRIV_LTR_SCALE_1US | val;
> >> +     }
> >> +
> >> +     if (ltr == lpss->active_ltr)
> >> +             return;
> >> +
> >> +     writel(ltr, lpss->priv + LPSS_PRIV_ACTIVELTR);
> >> +     writel(ltr, lpss->priv + LPSS_PRIV_IDLELTR);
> >> +
> >> +     /* Cache the values into lpss structure */
> >> +     intel_lpss_cache_ltr(lpss);
> >> +}
> >> +
> >> +static void intel_lpss_ltr_add(struct intel_lpss *lpss)
> >> +{
> >> +     lpss->dev->power.set_latency_tolerance = intel_lpss_ltr_set;
> >> +     dev_pm_qos_expose_latency_tolerance(lpss->dev);
> >> +}
> >> +
> >> +static void intel_lpss_ltr_remove(struct intel_lpss *lpss)
> >> +{
> >> +     dev_pm_qos_hide_latency_tolerance(lpss->dev);
> >> +     lpss->dev->power.set_latency_tolerance = NULL;
> >> +}
> >> +
> >> +static int intel_lpss_assign_devs(struct intel_lpss *lpss)
> >> +{
> >> +     unsigned int type;
> >> +
> >> +     type = lpss->caps & LPSS_PRIV_CAPS_TYPE_MASK;
> >> +     type >>= LPSS_PRIV_CAPS_TYPE_SHIFT;
> >> +
> >> +     switch (type) {
> >> +     case LPSS_DEV_I2C:
> >> +             lpss->devs = intel_lpss_i2c_devs;
> >> +             break;
> >> +     case LPSS_DEV_UART:
> >> +             lpss->devs = intel_lpss_uart_devs;
> >> +             break;
> >> +     case LPSS_DEV_SPI:
> >> +             lpss->devs = intel_lpss_spi_devs;
> >> +             break;
> >> +     default:
> >> +             return -ENODEV;
> >> +     }
> >> +
> >> +     lpss->type = type;
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +static bool intel_lpss_has_idma(const struct intel_lpss *lpss)
> >> +{
> >> +     return (lpss->caps & LPSS_PRIV_CAPS_NO_IDMA) == 0;
> >> +}
> >> +
> >> +static void intel_lpss_reset(const struct intel_lpss *lpss)
> >
> > Not very descriptive function name.  'de|assert'?
> 
> It actually handles the reset signal. I don't see how de/assert will be better.

What do you mean by 'handle'?  By the comment below it appers to
"bring the device out of reset" i.e. deassert the reset line, no?

intel_lpss_deassert_reset()?

> >> +{
> >> +     u32 value = LPSS_PRIV_RESETS_FUNC | LPSS_PRIV_RESETS_IDMA;
> >> +
> >> +     /* Bring out the device from reset */
> >> +     writel(value, lpss->priv + LPSS_PRIV_RESETS);
> >> +}
> >
> > All of this clock stuff below need s Clock Ack.
> 
> So, is anyone in your mind to whom we can Cc the new version?

COMMON CLK FRAMEWORK
M:      Mike Turquette <mturquette@linaro.org>
M:      Stephen Boyd <sboyd@codeaurora.org>
L:      linux-kernel@vger.kernel.org
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
S:      Maintained
F:      drivers/clk/
X:      drivers/clk/clkdev.c
F:      include/linux/clk-pr*
F:      include/linux/clk/

> >> +static void intel_lpss_unregister_clock_tree(struct clk *clk)
> >> +{
> >> +     struct clk *parent;
> >> +
> >> +     while (clk) {
> >> +             parent = clk_get_parent(clk);
> >> +             clk_unregister(clk);
> >> +             clk = parent;
> >> +     }
> >> +}
> >> +
> >> +static int intel_lpss_register_clock_divider(struct intel_lpss *lpss,
> >> +                                          const char *devname,
> >> +                                          struct clk **clk)
> >> +{
> >> +     char name[32];
> >> +     struct clk *tmp = *clk;
> >> +
> >> +     snprintf(name, sizeof(name), "%s-enable", devname);
> >> +     tmp = clk_register_gate(NULL, name, __clk_get_name(tmp), 0,
> >> +                             lpss->priv, 0, 0, NULL);
> >> +     if (IS_ERR(tmp))
> >> +             return PTR_ERR(tmp);
> >> +
> >> +     snprintf(name, sizeof(name), "%s-div", devname);
> >> +     tmp = clk_register_fractional_divider(NULL, name, __clk_get_name(tmp),
> >> +                                           0, lpss->priv, 1, 15, 16, 15, 0,
> >> +                                           NULL);
> >> +     if (IS_ERR(tmp))
> >> +             return PTR_ERR(tmp);
> >> +     *clk = tmp;
> >> +
> >> +     snprintf(name, sizeof(name), "%s-update", devname);
> >> +     tmp = clk_register_gate(NULL, name, __clk_get_name(tmp),
> >> +                             CLK_SET_RATE_PARENT, lpss->priv, 31, 0, NULL);
> >> +     if (IS_ERR(tmp))
> >> +             return PTR_ERR(tmp);
> >> +     *clk = tmp;
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +static int intel_lpss_register_clock(struct intel_lpss *lpss)
> >> +{
> >> +     const struct mfd_cell *devs = lpss->devs;
> >> +     struct clk_lookup **clocks;
> >> +     struct clk *clk, *root;
> >> +     char devname[24];
> >> +     int ret = -ENOMEM;
> >> +
> >> +     if (!lpss->info->clk_rate)
> >> +             return 0;
> >> +
> >> +     clocks = devm_kcalloc(lpss->dev, lpss->ndevs, sizeof(*clocks),
> >> +                           GFP_KERNEL);
> >> +     if (!clocks)
> >> +             return -ENOMEM;
> >> +
> >> +     /* Root clock */
> >> +     root = clk_register_fixed_rate(NULL, dev_name(lpss->dev), NULL,
> >> +                                    CLK_IS_ROOT, lpss->info->clk_rate);
> >> +     if (IS_ERR(root))
> >> +             return PTR_ERR(root);
> >> +
> >> +     clk = root;
> >> +
> >> +     snprintf(devname, sizeof(devname), "%s.%d", devs[1].name, lpss->devid);
> >> +
> >> +     /*
> >> +      * Support for clock divider only if it has some preset value.
> >> +      * Otherwise we assume that the divider is not used.
> >> +      */
> >> +     if (lpss->type != LPSS_DEV_I2C) {
> >> +             ret = intel_lpss_register_clock_divider(lpss, devname, &clk);
> >> +             if (ret)
> >> +                     goto err_clk_register;
> >> +     }
> >> +
> >> +     /* Clock for the host controller */
> >> +     clocks[0] = clk_register_clkdev(clk, lpss->info->clk_con_id, "%s",
> >> +                                     devname);
> >> +     if (IS_ERR(clocks[0]))
> >> +             goto err_clk_register;
> >> +
> >> +     /*
> >> +      * If the slice has integrated DMA block create clock for that as
> >> +      * well now.
> >> +      */
> >> +     if (lpss->ndevs > 1) {
> >> +             clocks[1] = clk_register_clkdev(root, "hclk", "%s.%d",
> >> +                                             devs[0].name, lpss->devid);
> >> +             if (IS_ERR(clocks[1]))
> >> +                     goto err_hclk_register;
> >> +     }
> >> +
> >> +     lpss->clocks = clocks;
> >> +     lpss->clk = clk;
> >> +
> >> +     return 0;
> >> +
> >> +err_hclk_register:
> >> +     clkdev_drop(clocks[0]);
> >> +
> >> +err_clk_register:
> >> +     intel_lpss_unregister_clock_tree(clk);
> >> +
> >> +     return ret;
> >> +}
> >> +
> >> +static void intel_lpss_unregister_clock(struct intel_lpss *lpss)
> >> +{
> >> +     unsigned int i;
> >> +
> >> +     if (IS_ERR_OR_NULL(lpss->clk))
> >> +             return;
> >> +
> >> +     for (i = 0; i < lpss->ndevs; i++)
> >> +             clkdev_drop(lpss->clocks[i]);
> >> +
> >> +     intel_lpss_unregister_clock_tree(lpss->clk);
> >> +}
> >> +
> >> +int intel_lpss_probe(struct device *dev,
> >> +                  const struct intel_lpss_platform_info *info)
> >> +{
> >> +     const struct mfd_cell *devs;
> >> +     struct intel_lpss *lpss;
> >> +     struct resource priv;
> >> +     int ret;
> >> +
> >> +     if (!info || !info->mem || info->irq <= 0)
> >> +             return -EINVAL;
> >> +
> >> +     lpss = devm_kzalloc(dev, sizeof(*lpss), GFP_KERNEL);
> >> +     if (!lpss)
> >> +             return -ENOMEM;
> >> +
> >> +     memset(&priv, 0, sizeof(priv));
> >> +     priv.start = info->mem->start + LPSS_PRIV_OFFSET;
> >> +     priv.end = priv.start + LPSS_PRIV_SIZE - 1;
> >> +     priv.flags = IORESOURCE_MEM;
> >> +
> >> +     lpss->priv = devm_ioremap_resource(dev, &priv);
> >> +     if (!lpss->priv)
> >> +             return -ENOMEM;
> >> +
> >> +     intel_lpss_reset(lpss);
> >> +
> >> +     lpss->info = info;
> >> +     lpss->dev = dev;
> >> +     lpss->caps = readl(lpss->priv + LPSS_PRIV_CAPS);
> >> +     lpss->ndevs = intel_lpss_has_idma(lpss) ? 2 : 1;
> >
> > This is fragile.
> >
> > I'd prefer you dynamically allocate resources here, instead of
> > statically up the top then 'jumping' over the ones you don't need.
> 
> (1)
> 
> >
> >> +     dev_set_drvdata(dev, lpss);
> >> +
> >> +     ret = intel_lpss_assign_devs(lpss);
> >> +     if (ret < 0)
> >> +             return ret;
> >
> > Is ret allowed to be >0?  If not just 'if (ret)'.
> 
> It might be a leftover when we previously returned a type of the
> device. Will amend.
> 
> >
> >> +     lpss->devid = ida_simple_get(&intel_lpss_devid_ida, 0, 0, GFP_KERNEL);
> >> +     if (lpss->devid < 0)
> >> +             return lpss->devid;
> >
> > PLATFORM_DEVID_AUTO does this for you.
> 
> What about clock matching?

This is a pretty weird case.  Hopefully Mike or Stephen will have a
better idea.  Normally the clock handing will be handled by the Clock
Framework and we don't have to worry about these things.

Don't ACPI/PCI have ways to link to clocks in the same way DT does?

> >> +     ret = intel_lpss_register_clock(lpss);
> >> +     if (ret < 0)
> >> +             goto err_clk_register;
> >> +
> >> +     intel_lpss_ltr_add(lpss);
> >
> > Add what, to what?  What is 'ltr'?
> 
> LTR stands for Latency Tolerance Reporting, the PCI property.
> 
> https://www.pcisig.com/specifications/pciexpress/specifications/ECN_LatencyTolnReporting_14Aug08.pdf

So what's the _add part? What are you adding and to what?

> >> +     ret = intel_lpss_debugfs_add(lpss);
> >> +     if (ret)
> >> +             dev_warn(lpss->dev, "Failed to create debugfs entries\n");
> >> +
> >> +     devs = intel_lpss_has_idma(lpss) ? lpss->devs : lpss->devs + 1;
> >
> > *cringe*
> 
> Here the comment also for (1).
> 
> In case of DMA IP present on compound device we have to register it
> first, otherwise main device driver may miss that (SPI checks DMA on
> ->probe() stage). Unfortunately there is no possibility to tell MFD
> framework the order of the device initialization except the direct
> order in the array. That's why this solution was implemented. If you
> have any better idea, please, share.

Relying on the order a given set of devices will probe() is fragile.

That's why -EPROBE_DEFER exists.

> >> +     ret = mfd_add_devices(dev, lpss->devid, devs, lpss->ndevs,
> >> +                           info->mem, info->irq, NULL);
> >> +     if (ret < 0)
> >> +             goto err_remove_ltr;
> >> +
> >> +     return 0;
> >> +
> >> +err_remove_ltr:
> >> +     intel_lpss_debugfs_remove(lpss);
> >> +     intel_lpss_ltr_remove(lpss);
> >> +
> >> +err_clk_register:
> >> +     ida_simple_remove(&intel_lpss_devid_ida, lpss->devid);
> >> +
> >> +     return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(intel_lpss_probe);
> >> +
> >> +void intel_lpss_remove(struct device *dev)
> >> +{
> >> +     struct intel_lpss *lpss = dev_get_drvdata(dev);
> >> +
> >> +     mfd_remove_devices(dev);
> >> +     intel_lpss_debugfs_remove(lpss);
> >> +     intel_lpss_ltr_remove(lpss);
> >> +     intel_lpss_unregister_clock(lpss);
> >> +     ida_simple_rnemove(&intel_lpss_devid_ida, lpss->devid);

Does this even compile?

> >> +}
> >> +EXPORT_SYMBOL_GPL(intel_lpss_remove);
> >> +
> >> +static int resume_lpss_device(struct device *dev, void *data)
> >> +{
> >> +     pm_runtime_resume(dev);
> >> +     return 0;
> >> +}
> >> +
> >> +int intel_lpss_prepare(struct device *dev)
> >> +{
> >> +     /*
> >> +      * Resume both child devices before entering system sleep. This
> >> +      * ensures that they are in proper state before they get suspended.
> >> +      */
> >> +     device_for_each_child(dev, NULL, resume_lpss_device);
> >> +     return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(intel_lpss_prepare);
> >> +
> >> +int intel_lpss_suspend(struct device *dev)
> >> +{
> >> +     return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(intel_lpss_suspend);
> >> +
> >> +int intel_lpss_resume(struct device *dev)
> >> +{
> >> +     struct intel_lpss *lpss = dev_get_drvdata(dev);
> >> +     intel_lpss_reset(lpss);
> >> +     return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(intel_lpss_resume);
> >> +
> >> +static int __init intel_lpss_init(void)
> >> +{
> >> +     intel_lpss_debugfs = debugfs_create_dir("intel_lpss", NULL);
> >> +     return 0;
> >> +}
> >> +module_init(intel_lpss_init);
> >> +
> >> +static void __exit intel_lpss_exit(void)
> >> +{
> >> +     debugfs_remove(intel_lpss_debugfs);
> >> +}
> >> +module_exit(intel_lpss_exit);
> >> +
> >> +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
> >> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
> >> +MODULE_AUTHOR("Heikki Krogerus <heikki.krogerus@linux.intel.com>");
> >> +MODULE_AUTHOR("Jarkko Nikula <jarkko.nikula@linux.intel.com>");
> >> +MODULE_DESCRIPTION("Intel LPSS core driver");
> >> +MODULE_LICENSE("GPL v2");
> >> diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
> >> new file mode 100644
> >> index 0000000..f28cb28
> >> --- /dev/null
> >> +++ b/drivers/mfd/intel-lpss.h
> >> @@ -0,0 +1,62 @@
> >> +/*
> >> + * Intel LPSS core support.
> >> + *
> >> + * Copyright (C) 2015, Intel Corporation
> >> + *
> >> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> + *          Mika Westerberg <mika.westerberg@linux.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 __MFD_INTEL_LPSS_H
> >> +#define __MFD_INTEL_LPSS_H
> >> +
> >> +struct device;
> >> +struct resource;
> >> +
> >> +struct intel_lpss_platform_info {
> >> +     struct resource *mem;
> >> +     int irq;
> >> +     unsigned long clk_rate;
> >> +     const char *clk_con_id;
> >> +};
> >> +
> >> +int intel_lpss_probe(struct device *dev,
> >> +                  const struct intel_lpss_platform_info *info);
> >> +void intel_lpss_remove(struct device *dev);
> >> +
> >> +#ifdef CONFIG_PM
> >> +int intel_lpss_prepare(struct device *dev);
> >> +int intel_lpss_suspend(struct device *dev);
> >> +int intel_lpss_resume(struct device *dev);
> >
> > Why are you exporting these?
> 
> LPSS devices are located in its own power domain. That's why we do all this.

Not sure I get that.

Looking closer it appears that the reason you do it is because you
are registering the power ops in the bus specific (-acpi.c, -pci.c)
probes.  What does that have to do with power domains?

> >> +#ifdef CONFIG_PM_SLEEP
> >> +#define INTEL_LPSS_SLEEP_PM_OPS                      \
> >> +     .prepare = intel_lpss_prepare,          \
> >> +     .suspend = intel_lpss_suspend,          \
> >> +     .resume = intel_lpss_resume,            \
> >> +     .freeze = intel_lpss_suspend,           \
> >> +     .thaw = intel_lpss_resume,              \
> >> +     .poweroff = intel_lpss_suspend,         \
> >> +     .restore = intel_lpss_resume,
> >> +#endif
> >
> > SET_SYSTEM_SLEEP_PM_OPS?
> >
> >> +#define INTEL_LPSS_RUNTIME_PM_OPS            \
> >> +     .runtime_suspend = intel_lpss_suspend,  \
> >> +     .runtime_resume = intel_lpss_resume,
> >
> > SET_RUNTIME_PM_OPS
> >
> >> +#else /* !CONFIG_PM */
> >> +#define INTEL_LPSS_SLEEP_PM_OPS
> >> +#define INTEL_LPSS_RUNTIME_PM_OPS
> >> +#endif /* CONFIG_PM */
> >> +
> >> +#define INTEL_LPSS_PM_OPS(name)                      \
> >> +const struct dev_pm_ops name = {             \
> >> +     INTEL_LPSS_SLEEP_PM_OPS                 \
> >> +     INTEL_LPSS_RUNTIME_PM_OPS               \
> >> +}
> >
> > Why is this in the header?
> >
> >> +#endif /* __MFD_INTEL_LPSS_H */
> 
> 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 3/3] mfd: Add support for Intel Sunrisepoint LPSS devices

[Mika Westerberg]

On Wed, Apr 29, 2015 at 04:06:19PM +0100, Lee Jones wrote:
> > >> +static const struct mfd_cell intel_lpss_uart_devs[] = {
> > >> +     {
> > >> +             .name = "dw_dmac",
> > >> +             .num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
> > >> +             .resources = intel_lpss_idma_resources,
> > >> +             .platform_data = (void *)&intel_lpss_idma_pdata,
> > >> +             .pdata_size = sizeof(intel_lpss_idma_pdata),
> > >> +     },
> > >> +     {
> > >> +             .name = "dw-apb-uart",
> > >> +             .num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
> > >> +             .resources = intel_lpss_dev_resources,
> > >> +     },
> > >> +};
> > >> +
> > >> +static const struct mfd_cell intel_lpss_spi_devs[] = {
> > >> +     {
> > >> +             .name = "dw_dmac",
> > >> +             .num_resources = ARRAY_SIZE(intel_lpss_idma_resources),
> > >> +             .resources = intel_lpss_idma_resources,
> > >> +             .platform_data = (void *)&intel_lpss_idma_pdata,
> > >> +             .pdata_size = sizeof(intel_lpss_idma_pdata),
> > >> +     },
> > >> +     {
> > >> +             .name = "pxa2xx-spi",
> > >> +             .num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
> > >> +             .resources = intel_lpss_dev_resources,
> > >> +     },
> > >> +};
> > >> +
> > >> +static DEFINE_IDA(intel_lpss_devid_ida);
> > >
> > > What's wrong with using PLATFORM_DEVID_AUTO?
> > 
> > We have to provide right clocks.
> > Is it possible to use AUTO and be sure that right clocks will go to
> > corresponding consumers?
> 
> I guess Mike will have to help us with that, as I've never seen it
> done this way before.
> 
> I guess you need to create a clock driver and register it properly as
> a platform device.

The reason why we generate the id here is that we need a way to provide
a clock with correct name to host controller and DMA drivers, and that
needs to be done before those two platform devices get created.

Moreover this makes it easy to match right DMA to right host controller
from sysfs as they are always look like:

   intel-lpss.X +-> host-controller.X
                |
                +-> DMA.X

X is the id we generate in the MFD driver.

Re: [PATCH v1 3/3] mfd: Add support for Intel Sunrisepoint LPSS devices

[Andy Shevchenko]

On Wed, 2015-04-29 at 16:06 +0100, Lee Jones wrote:
> On Wed, 29 Apr 2015, Andy Shevchenko wrote:
> > On Wed, Apr 29, 2015 at 11:23 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > > On Tue, 31 Mar 2015, Andy Shevchenko wrote:
> > 
> > Thanks for review. My comments / answers below.
> > 
> > >> The new coming Intel platforms such as Skylake will contain Sunrisepoint PCH.
> > >
> > > Can you use the same naming conventions as the Skylake's predecessors?
> > >
> > > Currently lpc_ich and lpc_sch exist.
> > 
> > As far as I understand it is not an LPC bus, thus seems above is
> > irrelevant. Moreover, PCH in Skylake is something new if looking into
> > hardware.
> 
> Perhaps the aforementioned files just need 'intel_' appended then.

Might be. Not concern of this series anyway.

> > >> The main difference to the previous platforms is that the LPSS devices are
> > >> compound devices where usually main (SPI, HSUART, or I2C) and DMA IPs are
> > >> present.
> > >>
> > >> This patch brings the driver for such devices found on Sunrisepoint PCH.
> > >>
> > >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > >> ---
> > >>  drivers/mfd/Kconfig           |  24 ++
> > >>  drivers/mfd/Makefile          |   3 +
> > >>  drivers/mfd/intel-lpss-acpi.c |  84 +++++++
> > >>  drivers/mfd/intel-lpss-pci.c  | 106 +++++++++
> > >>  drivers/mfd/intel-lpss.c      | 523 ++++++++++++++++++++++++++++++++++++++++++
> > >>  drivers/mfd/intel-lpss.h      |  62 +++++
> > >
> > > Can you move this to include/?
> > 
> > What the point? I don't see the users for that now or in the nearest future.
> 
> The point is to keep drivers/mfd/ clean rather than to share the header
> with external entities.

I don't get this. Why should we go to global space with the header which
is local to couple of modules located under drivers/mfd?

[]

> > >
> > >> +};
> > >> +
> > >> +static const struct resource intel_lpss_dev_resources[] = {
> > >> +     DEFINE_RES_MEM_NAMED(LPSS_DEV_OFFSET, LPSS_DEV_SIZE, "lpss_dev"),
> > >> +     DEFINE_RES_MEM_NAMED(LPSS_PRIV_OFFSET, LPSS_PRIV_SIZE, "lpss_priv"),
> > >> +     DEFINE_RES_IRQ(0),
> > >> +};
> > >
> > > Why has this been called 'dev' and 'priv'?  Neither of which are
> > > particularlly helpful address names and I can't find them in the
> > > datasheet.  Why not 'base' and 'extended', or something?
> > 
> > dev stands for main device controller
> > priv -- for private space. The abbrevation is inherited from LPSS
> > drivers for BayTrail and Co.
> > 
> > In documentation priv space called 'additional registers'.
> 
> In my mind 'additional registers' != 'private registers'.

This documentation somehow hides the purpose of the set of register
called as 'additional'. Their function is to provide a convergence layer
for both devices in MFD cell. Previously (Baytrail, Braswell) we used
name LPSS_PRIV for everything related to that 'additional' register
space. We would like to keep that way.

> > >> +static DEFINE_IDA(intel_lpss_devid_ida);
> > >
> > > What's wrong with using PLATFORM_DEVID_AUTO?
> > 
> > We have to provide right clocks.
> > Is it possible to use AUTO and be sure that right clocks will go to
> > corresponding consumers?
> 
> I guess Mike will have to help us with that, as I've never seen it
> done this way before.
> 
> I guess you need to create a clock driver and register it properly as
> a platform device.

Mika had answered you. I hope he has a valid point.

> > >> +static bool intel_lpss_has_idma(const struct intel_lpss *lpss)
> > >> +{
> > >> +     return (lpss->caps & LPSS_PRIV_CAPS_NO_IDMA) == 0;
> > >> +}
> > >> +
> > >> +static void intel_lpss_reset(const struct intel_lpss *lpss)
> > >
> > > Not very descriptive function name.  'de|assert'?
> > 
> > It actually handles the reset signal. I don't see how de/assert will be better.
> 
> What do you mean by 'handle'?  By the comment below it appers to
> "bring the device out of reset" i.e. deassert the reset line, no?
> 
> intel_lpss_deassert_reset()?

Ok.

> > >> +int intel_lpss_probe(struct device *dev,
> > >> +                  const struct intel_lpss_platform_info *info)
> > >> +{
> > >> +     const struct mfd_cell *devs;
> > >> +     struct intel_lpss *lpss;
> > >> +     struct resource priv;
> > >> +     int ret;
> > >> +
> > >> +     if (!info || !info->mem || info->irq <= 0)
> > >> +             return -EINVAL;
> > >> +
> > >> +     lpss = devm_kzalloc(dev, sizeof(*lpss), GFP_KERNEL);
> > >> +     if (!lpss)
> > >> +             return -ENOMEM;
> > >> +
> > >> +     memset(&priv, 0, sizeof(priv));
> > >> +     priv.start = info->mem->start + LPSS_PRIV_OFFSET;
> > >> +     priv.end = priv.start + LPSS_PRIV_SIZE - 1;
> > >> +     priv.flags = IORESOURCE_MEM;
> > >> +
> > >> +     lpss->priv = devm_ioremap_resource(dev, &priv);
> > >> +     if (!lpss->priv)
> > >> +             return -ENOMEM;
> > >> +
> > >> +     intel_lpss_reset(lpss);
> > >> +
> > >> +     lpss->info = info;
> > >> +     lpss->dev = dev;
> > >> +     lpss->caps = readl(lpss->priv + LPSS_PRIV_CAPS);
> > >> +     lpss->ndevs = intel_lpss_has_idma(lpss) ? 2 : 1;
> > >
> > > This is fragile.
> > >
> > > I'd prefer you dynamically allocate resources here, instead of
> > > statically up the top then 'jumping' over the ones you don't need.
> > 
> > (1)

> > >> +     ret = intel_lpss_register_clock(lpss);
> > >> +     if (ret < 0)
> > >> +             goto err_clk_register;
> > >> +
> > >> +     intel_lpss_ltr_add(lpss);
> > >
> > > Add what, to what?  What is 'ltr'?
> > 
> > LTR stands for Latency Tolerance Reporting, the PCI property.
> > 
> > https://www.pcisig.com/specifications/pciexpress/specifications/ECN_LatencyTolnReporting_14Aug08.pdf
> 
> So what's the _add part? What are you adding and to what?

So, we rename in new version like this
_add -> _expose
_remove -> _hide

in correspondence with PM QoS calls.

> > >> +     ret = intel_lpss_debugfs_add(lpss);
> > >> +     if (ret)
> > >> +             dev_warn(lpss->dev, "Failed to create debugfs entries\n");
> > >> +
> > >> +     devs = intel_lpss_has_idma(lpss) ? lpss->devs : lpss->devs + 1;
> > >
> > > *cringe*
> > 
> > Here the comment also for (1).
> > 
> > In case of DMA IP present on compound device we have to register it
> > first, otherwise main device driver may miss that (SPI checks DMA on
> > ->probe() stage). Unfortunately there is no possibility to tell MFD
> > framework the order of the device initialization except the direct
> > order in the array. That's why this solution was implemented. If you
> > have any better idea, please, share.
> 
> Relying on the order a given set of devices will probe() is fragile.
> 
> That's why -EPROBE_DEFER exists.

Let's consider this case in more generic way (arbitrary host controller
and arbitrary DMA that could or, what is important, *couldn't* be used.
The DMA channel is optional to many drivers to go well. For example SPI
controller when gets no DMA channel will easily switch to PIO mode and
user will be happy by having working, though slow, device.

Thus EDEFER_PROBE doesn't solve the problem but makes situation even
worse!

> > >> +     ida_simple_rnemove(&intel_lpss_devid_ida, lpss->devid);
> Does this even compile?

Of course it does. I have no idea how it was happened in the mail, but
in sources we have ida_simple_remove().

> > >> +}
> > >> +EXPORT_SYMBOL_GPL(intel_lpss_remove);
> > >> +
> > >> +static int resume_lpss_device(struct device *dev, void *data)
> > >> +{
> > >> +     pm_runtime_resume(dev);
> > >> +     return 0;
> > >> +}
> > >> +
> > >> +int intel_lpss_prepare(struct device *dev)
> > >> +{
> > >> +     /*
> > >> +      * Resume both child devices before entering system sleep. This
> > >> +      * ensures that they are in proper state before they get suspended.
> > >> +      */
> > >> +     device_for_each_child(dev, NULL, resume_lpss_device);
> > >> +     return 0;
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(intel_lpss_prepare);
> > >> +
> > >> +int intel_lpss_suspend(struct device *dev)
> > >> +{
> > >> +     return 0;
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(intel_lpss_suspend);
> > >> +
> > >> +int intel_lpss_resume(struct device *dev)
> > >> +{
> > >> +     struct intel_lpss *lpss = dev_get_drvdata(dev);
> > >> +     intel_lpss_reset(lpss);
> > >> +     return 0;
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(intel_lpss_resume);
> > >> +
> > >> +static int __init intel_lpss_init(void)
> > >> +{
> > >> +     intel_lpss_debugfs = debugfs_create_dir("intel_lpss", NULL);
> > >> +     return 0;
> > >> +}
> > >> +module_init(intel_lpss_init);
> > >> +
> > >> +static void __exit intel_lpss_exit(void)
> > >> +{
> > >> +     debugfs_remove(intel_lpss_debugfs);
> > >> +}
> > >> +module_exit(intel_lpss_exit);
> > >> +
> > >> +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
> > >> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
> > >> +MODULE_AUTHOR("Heikki Krogerus <heikki.krogerus@linux.intel.com>");
> > >> +MODULE_AUTHOR("Jarkko Nikula <jarkko.nikula@linux.intel.com>");
> > >> +MODULE_DESCRIPTION("Intel LPSS core driver");
> > >> +MODULE_LICENSE("GPL v2");
> > >> diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
> > >> new file mode 100644
> > >> index 0000000..f28cb28
> > >> --- /dev/null
> > >> +++ b/drivers/mfd/intel-lpss.h
> > >> @@ -0,0 +1,62 @@
> > >> +/*
> > >> + * Intel LPSS core support.
> > >> + *
> > >> + * Copyright (C) 2015, Intel Corporation
> > >> + *
> > >> + * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > >> + *          Mika Westerberg <mika.westerberg@linux.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 __MFD_INTEL_LPSS_H
> > >> +#define __MFD_INTEL_LPSS_H
> > >> +
> > >> +struct device;
> > >> +struct resource;
> > >> +
> > >> +struct intel_lpss_platform_info {
> > >> +     struct resource *mem;
> > >> +     int irq;
> > >> +     unsigned long clk_rate;
> > >> +     const char *clk_con_id;
> > >> +};
> > >> +
> > >> +int intel_lpss_probe(struct device *dev,
> > >> +                  const struct intel_lpss_platform_info *info);
> > >> +void intel_lpss_remove(struct device *dev);
> > >> +
> > >> +#ifdef CONFIG_PM
> > >> +int intel_lpss_prepare(struct device *dev);
> > >> +int intel_lpss_suspend(struct device *dev);
> > >> +int intel_lpss_resume(struct device *dev);
> > >
> > > Why are you exporting these?
> > 
> > LPSS devices are located in its own power domain. That's why we do all this.
> 
> Not sure I get that.
> 
> Looking closer it appears that the reason you do it is because you
> are registering the power ops in the bus specific (-acpi.c, -pci.c)
> probes.  What does that have to do with power domains?

Oops, my mistake since I was thinking it was done in similar way to
acpi_lpss.c driver, but you are right. It's about sharing for -acpi.c
and -pci.c.

(2)

> 
> > >> +#ifdef CONFIG_PM_SLEEP
> > >> +#define INTEL_LPSS_SLEEP_PM_OPS                      \
> > >> +     .prepare = intel_lpss_prepare,          \
> > >> +     .suspend = intel_lpss_suspend,          \
> > >> +     .resume = intel_lpss_resume,            \
> > >> +     .freeze = intel_lpss_suspend,           \
> > >> +     .thaw = intel_lpss_resume,              \
> > >> +     .poweroff = intel_lpss_suspend,         \
> > >> +     .restore = intel_lpss_resume,
> > >> +#endif
> > >
> > > SET_SYSTEM_SLEEP_PM_OPS?

Doesn't cover .prepare.

> > >
> > >> +#define INTEL_LPSS_RUNTIME_PM_OPS            \
> > >> +     .runtime_suspend = intel_lpss_suspend,  \
> > >> +     .runtime_resume = intel_lpss_resume,
> > >
> > > SET_RUNTIME_PM_OPS

Since above doesn't cover prepare we follow the same style.

> > >
> > >> +#else /* !CONFIG_PM */
> > >> +#define INTEL_LPSS_SLEEP_PM_OPS
> > >> +#define INTEL_LPSS_RUNTIME_PM_OPS
> > >> +#endif /* CONFIG_PM */
> > >> +
> > >> +#define INTEL_LPSS_PM_OPS(name)                      \
> > >> +const struct dev_pm_ops name = {             \
> > >> +     INTEL_LPSS_SLEEP_PM_OPS                 \
> > >> +     INTEL_LPSS_RUNTIME_PM_OPS               \
> > >> +}
> > >
> > > Why is this in the header?

Same as in (2)

We are about to send v2 of the series which is reworked a lot. We
addressed most of your comments regarding to function names, LTR stuff
and minor amendments.

I think we may continue discussion on new series then.

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy