linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4 00/12] drivers: Boot Constraints core
@ 2017-10-29 13:48 Viresh Kumar
  2017-10-29 13:48 ` [PATCH V4 01/12] of: platform: Add of_find_amba_device_by_node() Viresh Kumar
                   ` (13 more replies)
  0 siblings, 14 replies; 40+ messages in thread
From: Viresh Kumar @ 2017-10-29 13:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Viresh Kumar, Vincent Guittot, Stephen Boyd, Rajendra Nayak,
	linux-kernel, linux-arm-kernel, robdclark, s.hauer, l.stach,
	shawnguo, fabio.estevam, nm, xuwei5, robh+dt, devicetree,
	Frank Rowand

Hi Greg,

Here is V4 of the boot constraints core based on your feedback from V3.
We now have support for three platforms (as you suggested) included in
this series: Hisilicon, IMX and Qualcomm.

I have tested the Hisilicon patches on hikey 9660 board, IMX stuff is
tested by Sascha (Pengutronix) on i.MX6 and Qualcomm stuff is tested by
Rajendra (Qualcomm) on Dragonboard 410C (This required some more patches
related to display driver which Rajendra should be sending separately
later on).


Problem statement:

Some devices are powered ON by the bootloader before the bootloader
handovers control to Linux. It maybe important for those devices to keep
working until the time a Linux device driver probes the device and
reconfigure its resources.

A typical example of that can be the LCD controller, which is used by
the bootloaders to show image(s) while the platform is booting into
Linux.  The LCD controller can be using some resources, like clk,
regulators, etc, that are shared between several devices. These shared
resources should be configured to satisfy need of all the users.  If
another device's (X) driver gets probed before the LCD controller driver
in this case, then it may end up disabling or reconfiguring these
resources to ranges satisfying the current users (only device X) and
that can make the LCD screen unstable.

Another case can be a debug serial port enabled from the bootloader.

Of course we can have more complex cases where the same resource is
getting used by two devices while the kernel boots and the order in
which devices get probed wouldn't matter as the other device will surely
break then.

There are also cases where the resources may not be shared, but the
kernel will disable them forcefully as no users may have appeared until
a certain point in kernel boot. This makes sure that the resources stay
enabled. A wide variety of constraints can be satisfied using the new
framework.

Proposed solution:

This series introduces the concept of "boot-constraints", which are set
by platform specific drivers (for now at least) at early init (like
subsys_initcall) and the kernel will keep satisfying them until the time
driver for such a device is probed (successfully or unsuccessfully).
Once the driver is probed, the driver core removes the constraints set
for the device. This series implements clk, regulator and PM domain
constraints currently.

Rebased over: driver-core/master
Targeted for: v4.16 (Sending it earlier for reviews mostly)

Pushed here:
git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/linux.git device/boot-constraints

V3->V4:
- Added support for imx, hikey and Qcom usecases.
- Enhanced boot constraints core to make drivers code easy and handle
  complex cases.
- Two new patches for OF included to provide APIs to boot constraint
  core.
- Removed the kernel parameter patch for now.
- Don't check return values of debugfs routines.
- Moved the boot constraints core from drivers/base/ to drivers/.

V2->V3:
- Removed DT support as we aren't sure about how to define the bindings
  yet.
- Added CLK and PM domain constraint types.
- A new directory is added for boot constraints, which will also contain
  platform specific drivers in future.
- Deferred devices are still supported, just that it wouldn't be called
  from generic code anymore but platform specific code.
- Tested on Qcom 410c dragonboard with display flash screen (Rajendra).
- Usual renaming/commit-log-updates/etc changes done.

V1->V2:
- Add support for setting constraints for devices created from DT.
- Allow handling deferred devices earlier then late_init.
- Remove 'default y' line from kconfig.
- Drop '=" after boot_constraints_disable kernel param.
- Dropped the dummy testing patch now.

--
viresh

Rajendra Nayak (1):
  boot_constraint: Add Qualcomm display controller constraints

Viresh Kumar (11):
  of: platform: Add of_find_amba_device_by_node()
  of: platform: Make of_platform_bus_create() global
  drivers: Add boot constraints core
  boot_constraint: Add support for supply constraints
  boot_constraint: Add support for clk constraints
  boot_constraint: Add support for PM constraints
  boot_constraint: Add debugfs support
  boot_constraint: Manage deferrable constraints
  boot_constraint: Add earlycon helper
  boot_constraint: Add support for Hisilicon platforms
  boot_constraint: Add support for IMX platform

 arch/arm/mach-imx/Kconfig                 |   1 +
 arch/arm64/Kconfig.platforms              |   2 +
 drivers/Kconfig                           |   2 +
 drivers/Makefile                          |   1 +
 drivers/base/dd.c                         |  32 +++-
 drivers/boot_constraints/Kconfig          |   9 +
 drivers/boot_constraints/Makefile         |   7 +
 drivers/boot_constraints/clk.c            |  73 ++++++++
 drivers/boot_constraints/core.c           | 271 ++++++++++++++++++++++++++++++
 drivers/boot_constraints/core.h           |  48 ++++++
 drivers/boot_constraints/deferrable_dev.c | 235 ++++++++++++++++++++++++++
 drivers/boot_constraints/hikey.c          | 145 ++++++++++++++++
 drivers/boot_constraints/imx.c            | 113 +++++++++++++
 drivers/boot_constraints/pm.c             |  31 ++++
 drivers/boot_constraints/qcom.c           | 123 ++++++++++++++
 drivers/boot_constraints/serial.c         |  28 +++
 drivers/boot_constraints/supply.c         | 107 ++++++++++++
 drivers/clk/imx/clk-imx25.c               |  12 --
 drivers/clk/imx/clk-imx27.c               |  13 --
 drivers/clk/imx/clk-imx31.c               |  12 --
 drivers/clk/imx/clk-imx35.c               |  10 --
 drivers/clk/imx/clk-imx51-imx53.c         |  16 --
 drivers/clk/imx/clk-imx6q.c               |   8 -
 drivers/clk/imx/clk-imx6sl.c              |   8 -
 drivers/clk/imx/clk-imx6sx.c              |   8 -
 drivers/clk/imx/clk-imx7d.c               |  14 --
 drivers/clk/imx/clk.c                     |  38 -----
 drivers/clk/imx/clk.h                     |   1 -
 drivers/of/platform.c                     |  28 ++-
 include/linux/boot_constraint.h           |  74 ++++++++
 include/linux/of_platform.h               |  21 +++
 31 files changed, 1340 insertions(+), 151 deletions(-)
 create mode 100644 drivers/boot_constraints/Kconfig
 create mode 100644 drivers/boot_constraints/Makefile
 create mode 100644 drivers/boot_constraints/clk.c
 create mode 100644 drivers/boot_constraints/core.c
 create mode 100644 drivers/boot_constraints/core.h
 create mode 100644 drivers/boot_constraints/deferrable_dev.c
 create mode 100644 drivers/boot_constraints/hikey.c
 create mode 100644 drivers/boot_constraints/imx.c
 create mode 100644 drivers/boot_constraints/pm.c
 create mode 100644 drivers/boot_constraints/qcom.c
 create mode 100644 drivers/boot_constraints/serial.c
 create mode 100644 drivers/boot_constraints/supply.c
 create mode 100644 include/linux/boot_constraint.h

-- 
2.15.0.rc1.236.g92ea95045093

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 01/12] of: platform: Add of_find_amba_device_by_node()
  2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
@ 2017-10-29 13:48 ` Viresh Kumar
  2017-10-29 13:48 ` [PATCH V4 02/12] of: platform: Make of_platform_bus_create() global Viresh Kumar
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 40+ messages in thread
From: Viresh Kumar @ 2017-10-29 13:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Viresh Kumar, Vincent Guittot, Stephen Boyd, Rajendra Nayak,
	linux-kernel, linux-arm-kernel, robdclark, s.hauer, l.stach,
	shawnguo, fabio.estevam, nm, xuwei5, robh+dt, Frank Rowand,
	devicetree

of_find_device_by_node() works only with platform devices. Lets create
one for amba devices as well.

Cc: Frank Rowand <frowand.list@gmail.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/of/platform.c       | 20 ++++++++++++++++++++
 include/linux/of_platform.h |  9 +++++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index ac15d0e3d27d..b38a8e5908a3 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -60,6 +60,26 @@ struct platform_device *of_find_device_by_node(struct device_node *np)
 }
 EXPORT_SYMBOL(of_find_device_by_node);
 
+#ifdef CONFIG_ARM_AMBA
+/**
+ * of_find_amba_device_by_node - Find the amba_device associated with a node
+ * @np: Pointer to device tree node
+ *
+ * Takes a reference to the embedded struct device which needs to be dropped
+ * after use.
+ *
+ * Returns amba_device pointer, or NULL if not found
+ */
+struct amba_device *of_find_amba_device_by_node(struct device_node *np)
+{
+	struct device *dev;
+
+	dev = bus_find_device(&amba_bustype, NULL, np, of_dev_node_match);
+	return dev ? to_amba_device(dev) : NULL;
+}
+EXPORT_SYMBOL(of_find_amba_device_by_node);
+#endif
+
 #ifdef CONFIG_OF_ADDRESS
 /*
  * The following routines scan a subtree and registers a device for
diff --git a/include/linux/of_platform.h b/include/linux/of_platform.h
index 8a561e08bc9e..fbf8a6443885 100644
--- a/include/linux/of_platform.h
+++ b/include/linux/of_platform.h
@@ -61,6 +61,15 @@ static inline struct platform_device *of_find_device_by_node(struct device_node
 }
 #endif
 
+#if defined(CONFIG_OF) && defined(CONFIG_ARM_AMBA)
+extern struct amba_device *of_find_amba_device_by_node(struct device_node *np);
+#else
+static inline struct amba_device *of_find_amba_device_by_node(struct device_node *np)
+{
+	return NULL;
+}
+#endif
+
 #ifdef CONFIG_OF_ADDRESS
 /* Platform drivers register/unregister */
 extern struct platform_device *of_device_alloc(struct device_node *np,
-- 
2.15.0.rc1.236.g92ea95045093

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH V4 02/12] of: platform: Make of_platform_bus_create() global
  2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
  2017-10-29 13:48 ` [PATCH V4 01/12] of: platform: Add of_find_amba_device_by_node() Viresh Kumar
@ 2017-10-29 13:48 ` Viresh Kumar
  2017-10-29 13:48 ` [PATCH V4 03/12] drivers: Add boot constraints core Viresh Kumar
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 40+ messages in thread
From: Viresh Kumar @ 2017-10-29 13:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Viresh Kumar, Vincent Guittot, Stephen Boyd, Rajendra Nayak,
	linux-kernel, linux-arm-kernel, robdclark, s.hauer, l.stach,
	shawnguo, fabio.estevam, nm, xuwei5, robh+dt, Frank Rowand,
	devicetree

The boot constraints core needs to create platform or AMBA devices
corresponding to a compatible string and not for rest of the nodes in
DT. of_platform_bus_create() fits in the best to achieve that.

Allow it to be used outside of platform.c.

Cc: Frank Rowand <frowand.list@gmail.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/of/platform.c       |  8 ++++----
 include/linux/of_platform.h | 12 ++++++++++++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index b38a8e5908a3..808628842ea8 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -362,10 +362,10 @@ static const struct of_dev_auxdata *of_dev_lookup(const struct of_dev_auxdata *l
  * Creates a platform_device for the provided device_node, and optionally
  * recursively create devices for all the child nodes.
  */
-static int of_platform_bus_create(struct device_node *bus,
-				  const struct of_device_id *matches,
-				  const struct of_dev_auxdata *lookup,
-				  struct device *parent, bool strict)
+int of_platform_bus_create(struct device_node *bus,
+			   const struct of_device_id *matches,
+			   const struct of_dev_auxdata *lookup,
+			   struct device *parent, bool strict)
 {
 	const struct of_dev_auxdata *auxdata;
 	struct device_node *child;
diff --git a/include/linux/of_platform.h b/include/linux/of_platform.h
index fbf8a6443885..3ee106018b60 100644
--- a/include/linux/of_platform.h
+++ b/include/linux/of_platform.h
@@ -82,6 +82,10 @@ extern struct platform_device *of_platform_device_create(struct device_node *np,
 						   struct device *parent);
 
 extern int of_platform_device_destroy(struct device *dev, void *data);
+extern int of_platform_bus_create(struct device_node *bus,
+				  const struct of_device_id *matches,
+				  const struct of_dev_auxdata *lookup,
+				  struct device *parent, bool strict);
 extern int of_platform_bus_probe(struct device_node *root,
 				 const struct of_device_id *matches,
 				 struct device *parent);
@@ -118,6 +122,14 @@ static inline int of_platform_device_destroy(struct device *dev, void *data)
 	return -ENODEV;
 }
 
+static inline int of_platform_bus_create(struct device_node *bus,
+					 const struct of_device_id *matches,
+					 const struct of_dev_auxdata *lookup,
+					 struct device *parent, bool strict)
+{
+	return -ENODEV;
+}
+
 static inline int of_platform_bus_probe(struct device_node *root,
 					const struct of_device_id *matches,
 					struct device *parent)
-- 
2.15.0.rc1.236.g92ea95045093

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH V4 03/12] drivers: Add boot constraints core
  2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
  2017-10-29 13:48 ` [PATCH V4 01/12] of: platform: Add of_find_amba_device_by_node() Viresh Kumar
  2017-10-29 13:48 ` [PATCH V4 02/12] of: platform: Make of_platform_bus_create() global Viresh Kumar
@ 2017-10-29 13:48 ` Viresh Kumar
  2017-12-13  9:42   ` Greg Kroah-Hartman
  2017-12-13  9:42   ` Greg Kroah-Hartman
  2017-10-29 13:48 ` [PATCH V4 04/12] boot_constraint: Add support for supply constraints Viresh Kumar
                   ` (10 subsequent siblings)
  13 siblings, 2 replies; 40+ messages in thread
From: Viresh Kumar @ 2017-10-29 13:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Viresh Kumar, Vincent Guittot, Stephen Boyd, Rajendra Nayak,
	linux-kernel, linux-arm-kernel, robdclark, s.hauer, l.stach,
	shawnguo, fabio.estevam, nm, xuwei5, robh+dt

Some devices are powered ON by the bootloader before the bootloader
handovers control to Linux. It maybe important for those devices to keep
working until the time a Linux device driver probes the device and
reconfigure its resources.

A typical example of that can be the LCD controller, which is used by
the bootloaders to show image(s) while the platform is booting into
Linux. The LCD controller can be using some resources, like clk,
regulators, PM domain, etc, that are shared between several devices.
These shared resources should be configured to satisfy need of all the
users. If another device's (X) driver gets probed before the LCD
controller driver in this case, then it may end up reconfiguring these
resources to ranges satisfying the current users (only device X) and
that can make the LCD screen unstable.

This patch introduces the concept of boot-constraints, which will be set
by the bootloaders and the kernel will satisfy them until the time
driver for such a device is probed (successfully or unsuccessfully).

The list of boot constraint types is empty for now, and will be
incrementally updated by later patches.

Only two routines are exposed by the boot constraints core for now:

- dev_boot_constraint_add(): This shall be called by parts of the kernel
  (before the device is probed) to set the constraints.

- dev_boot_constraints_remove(): This is called only by the driver core
  after a device is probed successfully or unsuccessfully. Special
  handling is done here for deferred probing.

Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/Kconfig                   |   2 +
 drivers/Makefile                  |   1 +
 drivers/base/dd.c                 |  20 ++--
 drivers/boot_constraints/Kconfig  |   9 ++
 drivers/boot_constraints/Makefile |   3 +
 drivers/boot_constraints/core.c   | 199 ++++++++++++++++++++++++++++++++++++++
 drivers/boot_constraints/core.h   |  33 +++++++
 include/linux/boot_constraint.h   |  46 +++++++++
 8 files changed, 306 insertions(+), 7 deletions(-)
 create mode 100644 drivers/boot_constraints/Kconfig
 create mode 100644 drivers/boot_constraints/Makefile
 create mode 100644 drivers/boot_constraints/core.c
 create mode 100644 drivers/boot_constraints/core.h
 create mode 100644 include/linux/boot_constraint.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 505c676fa9c7..e595ffad2214 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -4,6 +4,8 @@ source "drivers/amba/Kconfig"
 
 source "drivers/base/Kconfig"
 
+source "drivers/boot_constraints/Kconfig"
+
 source "drivers/bus/Kconfig"
 
 source "drivers/connector/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index d90fdc413648..29d03466cb2a 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -72,6 +72,7 @@ obj-$(CONFIG_FB_INTEL)          += video/fbdev/intelfb/
 obj-$(CONFIG_PARPORT)		+= parport/
 obj-$(CONFIG_NVM)		+= lightnvm/
 obj-y				+= base/ block/ misc/ mfd/ nfc/
+obj-$(CONFIG_DEV_BOOT_CONSTRAINTS) += boot_constraints/
 obj-$(CONFIG_LIBNVDIMM)		+= nvdimm/
 obj-$(CONFIG_DAX)		+= dax/
 obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf/
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index ad44b40fe284..4eec27fe2b2b 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -17,6 +17,7 @@
  * This file is released under the GPLv2
  */
 
+#include <linux/boot_constraint.h>
 #include <linux/device.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
@@ -409,15 +410,20 @@ static int really_probe(struct device *dev, struct device_driver *drv)
 	 */
 	devices_kset_move_last(dev);
 
-	if (dev->bus->probe) {
+	if (dev->bus->probe)
 		ret = dev->bus->probe(dev);
-		if (ret)
-			goto probe_failed;
-	} else if (drv->probe) {
+	else if (drv->probe)
 		ret = drv->probe(dev);
-		if (ret)
-			goto probe_failed;
-	}
+
+	/*
+	 * Remove boot constraints for both successful and unsuccessful probe(),
+	 * except for the case where EPROBE_DEFER is returned by probe().
+	 */
+	if (ret != -EPROBE_DEFER)
+		dev_boot_constraints_remove(dev);
+
+	if (ret)
+		goto probe_failed;
 
 	if (test_remove) {
 		test_remove = false;
diff --git a/drivers/boot_constraints/Kconfig b/drivers/boot_constraints/Kconfig
new file mode 100644
index 000000000000..77831af1c6fb
--- /dev/null
+++ b/drivers/boot_constraints/Kconfig
@@ -0,0 +1,9 @@
+config DEV_BOOT_CONSTRAINTS
+	bool "Boot constraints for devices"
+	help
+	  This enables boot constraints detection for devices. These constraints
+	  are (normally) set by the Bootloader and must be satisfied by the
+	  kernel until the relevant device driver is probed. Once the driver is
+	  probed, the constraint is dropped.
+
+	  If unsure, say N.
diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
new file mode 100644
index 000000000000..0f2680177974
--- /dev/null
+++ b/drivers/boot_constraints/Makefile
@@ -0,0 +1,3 @@
+# Makefile for device boot constraints
+
+obj-y := core.o
diff --git a/drivers/boot_constraints/core.c b/drivers/boot_constraints/core.c
new file mode 100644
index 000000000000..366a05d6d9ba
--- /dev/null
+++ b/drivers/boot_constraints/core.c
@@ -0,0 +1,199 @@
+/*
+ * This takes care of boot time device constraints, normally set by the
+ * Bootloader.
+ *
+ * Copyright (C) 2017 Linaro.
+ * Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#define pr_fmt(fmt) "Boot Constraints: " fmt
+
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+
+#include "core.h"
+
+#define for_each_constraint(_constraint, _temp, _cdev)		\
+	list_for_each_entry_safe(_constraint, _temp, &_cdev->constraints, node)
+
+/* Global list of all constraint devices currently registered */
+static LIST_HEAD(constraint_devices);
+static DEFINE_MUTEX(constraint_devices_mutex);
+
+/* Boot constraints core */
+
+static struct constraint_dev *constraint_device_find(struct device *dev)
+{
+	struct constraint_dev *cdev;
+
+	list_for_each_entry(cdev, &constraint_devices, node) {
+		if (cdev->dev == dev)
+			return cdev;
+	}
+
+	return NULL;
+}
+
+static struct constraint_dev *constraint_device_allocate(struct device *dev)
+{
+	struct constraint_dev *cdev;
+
+	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
+	if (!cdev)
+		return ERR_PTR(-ENOMEM);
+
+	cdev->dev = dev;
+	INIT_LIST_HEAD(&cdev->node);
+	INIT_LIST_HEAD(&cdev->constraints);
+
+	list_add(&cdev->node, &constraint_devices);
+
+	return cdev;
+}
+
+static void constraint_device_free(struct constraint_dev *cdev)
+{
+	list_del(&cdev->node);
+	kfree(cdev);
+}
+
+static struct constraint_dev *constraint_device_get(struct device *dev)
+{
+	struct constraint_dev *cdev;
+
+	cdev = constraint_device_find(dev);
+	if (cdev)
+		return cdev;
+
+	cdev = constraint_device_allocate(dev);
+	if (IS_ERR(cdev)) {
+		dev_err(dev, "Failed to add constraint dev (%ld)\n",
+			PTR_ERR(cdev));
+	}
+
+	return cdev;
+}
+
+static void constraint_device_put(struct constraint_dev *cdev)
+{
+	if (!list_empty(&cdev->constraints))
+		return;
+
+	constraint_device_free(cdev);
+}
+
+static struct constraint *constraint_allocate(struct constraint_dev *cdev,
+					enum dev_boot_constraint_type type)
+{
+	struct constraint *constraint;
+	int (*add)(struct constraint *constraint, void *data);
+	void (*remove)(struct constraint *constraint);
+
+	switch (type) {
+	default:
+		return ERR_PTR(-EINVAL);
+	}
+
+	constraint = kzalloc(sizeof(*constraint), GFP_KERNEL);
+	if (!constraint)
+		return ERR_PTR(-ENOMEM);
+
+	constraint->cdev = cdev;
+	constraint->type = type;
+	constraint->add = add;
+	constraint->remove = remove;
+	INIT_LIST_HEAD(&constraint->node);
+
+	list_add(&constraint->node, &cdev->constraints);
+
+	return constraint;
+}
+
+static void constraint_free(struct constraint *constraint)
+{
+	list_del(&constraint->node);
+	kfree(constraint);
+}
+
+int dev_boot_constraint_add(struct device *dev,
+			    struct dev_boot_constraint_info *info)
+{
+	struct constraint_dev *cdev;
+	struct constraint *constraint;
+	int ret;
+
+	mutex_lock(&constraint_devices_mutex);
+
+	/* Find or add the cdev type first */
+	cdev = constraint_device_get(dev);
+	if (IS_ERR(cdev)) {
+		ret = PTR_ERR(cdev);
+		goto unlock;
+	}
+
+	constraint = constraint_allocate(cdev, info->constraint.type);
+	if (IS_ERR(constraint)) {
+		dev_err(dev, "Failed to add constraint type: %d (%ld)\n",
+			info->constraint.type, PTR_ERR(constraint));
+		ret = PTR_ERR(constraint);
+		goto put_cdev;
+	}
+
+	constraint->free_resources = info->free_resources;
+	constraint->free_resources_data = info->free_resources_data;
+
+	/* Set constraint */
+	ret = constraint->add(constraint, info->constraint.data);
+	if (ret)
+		goto free_constraint;
+
+	dev_dbg(dev, "Added boot constraint-type (%d)\n",
+		info->constraint.type);
+
+	mutex_unlock(&constraint_devices_mutex);
+
+	return 0;
+
+free_constraint:
+	constraint_free(constraint);
+put_cdev:
+	constraint_device_put(cdev);
+unlock:
+	mutex_unlock(&constraint_devices_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dev_boot_constraint_add);
+
+static void constraint_remove(struct constraint *constraint)
+{
+	constraint->remove(constraint);
+
+	if (constraint->free_resources)
+		constraint->free_resources(constraint->free_resources_data);
+
+	constraint_free(constraint);
+}
+
+void dev_boot_constraints_remove(struct device *dev)
+{
+	struct constraint_dev *cdev;
+	struct constraint *constraint, *temp;
+
+	mutex_lock(&constraint_devices_mutex);
+
+	cdev = constraint_device_find(dev);
+	if (!cdev)
+		goto unlock;
+
+	for_each_constraint(constraint, temp, cdev)
+		constraint_remove(constraint);
+
+	constraint_device_put(cdev);
+unlock:
+	mutex_unlock(&constraint_devices_mutex);
+}
diff --git a/drivers/boot_constraints/core.h b/drivers/boot_constraints/core.h
new file mode 100644
index 000000000000..7ba4ac172c09
--- /dev/null
+++ b/drivers/boot_constraints/core.h
@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2017 Linaro.
+ * Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * This file is released under the GPLv2.
+ */
+#ifndef _CORE_H
+#define _CORE_H
+
+#include <linux/boot_constraint.h>
+#include <linux/device.h>
+#include <linux/list.h>
+
+struct constraint_dev {
+	struct device *dev;
+	struct list_head node;
+	struct list_head constraints;
+};
+
+struct constraint {
+	struct constraint_dev *cdev;
+	struct list_head node;
+	enum dev_boot_constraint_type type;
+	void (*free_resources)(void *data);
+	void *free_resources_data;
+
+	int (*add)(struct constraint *constraint, void *data);
+	void (*remove)(struct constraint *constraint);
+	void *private;
+};
+
+/* Forward declarations of constraint specific callbacks */
+#endif /* _CORE_H */
diff --git a/include/linux/boot_constraint.h b/include/linux/boot_constraint.h
new file mode 100644
index 000000000000..2b816bf74144
--- /dev/null
+++ b/include/linux/boot_constraint.h
@@ -0,0 +1,46 @@
+/*
+ * Boot constraints header.
+ *
+ * Copyright (C) 2017 Linaro.
+ * Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * This file is released under the GPLv2
+ */
+#ifndef _LINUX_BOOT_CONSTRAINT_H
+#define _LINUX_BOOT_CONSTRAINT_H
+
+#include <linux/err.h>
+#include <linux/types.h>
+
+struct device;
+
+enum dev_boot_constraint_type {
+	DEV_BOOT_CONSTRAINT_NONE,
+};
+
+struct dev_boot_constraint {
+	enum dev_boot_constraint_type type;
+	void *data;
+};
+
+struct dev_boot_constraint_info {
+	struct dev_boot_constraint constraint;
+
+	/* This will be called just before the constraint is removed */
+	void (*free_resources)(void *data);
+	void *free_resources_data;
+};
+
+#ifdef CONFIG_DEV_BOOT_CONSTRAINTS
+int dev_boot_constraint_add(struct device *dev,
+			    struct dev_boot_constraint_info *info);
+void dev_boot_constraints_remove(struct device *dev);
+#else
+static inline
+int dev_boot_constraint_add(struct device *dev,
+			    struct dev_boot_constraint_info *info)
+{ return -EINVAL; }
+static inline void dev_boot_constraints_remove(struct device *dev) {}
+#endif /* CONFIG_DEV_BOOT_CONSTRAINTS */
+
+#endif /* _LINUX_BOOT_CONSTRAINT_H */
-- 
2.15.0.rc1.236.g92ea95045093

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH V4 04/12] boot_constraint: Add support for supply constraints
  2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
                   ` (2 preceding siblings ...)
  2017-10-29 13:48 ` [PATCH V4 03/12] drivers: Add boot constraints core Viresh Kumar
@ 2017-10-29 13:48 ` Viresh Kumar
  2017-12-13  9:48   ` Greg Kroah-Hartman
  2017-10-29 13:48 ` [PATCH V4 05/12] boot_constraint: Add support for clk constraints Viresh Kumar
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 40+ messages in thread
From: Viresh Kumar @ 2017-10-29 13:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Viresh Kumar, Vincent Guittot, Stephen Boyd, Rajendra Nayak,
	linux-kernel, linux-arm-kernel, robdclark, s.hauer, l.stach,
	shawnguo, fabio.estevam, nm, xuwei5, robh+dt

This patch adds the first constraint type: power-supply.

The constraint is set by enabling the regulator and setting a voltage
range (if required) for the respective regulator device, which will be
honored by the regulator core even if more users turn up. Once the
device is probed, the regulator is released and the constraint is
removed.

Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/boot_constraints/Makefile |  2 +-
 drivers/boot_constraints/core.c   |  4 ++
 drivers/boot_constraints/core.h   |  3 ++
 drivers/boot_constraints/supply.c | 98 +++++++++++++++++++++++++++++++++++++++
 include/linux/boot_constraint.h   |  8 +++-
 5 files changed, 113 insertions(+), 2 deletions(-)
 create mode 100644 drivers/boot_constraints/supply.c

diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
index 0f2680177974..a45616f0c3b0 100644
--- a/drivers/boot_constraints/Makefile
+++ b/drivers/boot_constraints/Makefile
@@ -1,3 +1,3 @@
 # Makefile for device boot constraints
 
-obj-y := core.o
+obj-y := core.o supply.o
diff --git a/drivers/boot_constraints/core.c b/drivers/boot_constraints/core.c
index 366a05d6d9ba..b9c024a3bdf5 100644
--- a/drivers/boot_constraints/core.c
+++ b/drivers/boot_constraints/core.c
@@ -94,6 +94,10 @@ static struct constraint *constraint_allocate(struct constraint_dev *cdev,
 	void (*remove)(struct constraint *constraint);
 
 	switch (type) {
+	case DEV_BOOT_CONSTRAINT_SUPPLY:
+		add = constraint_supply_add;
+		remove = constraint_supply_remove;
+		break;
 	default:
 		return ERR_PTR(-EINVAL);
 	}
diff --git a/drivers/boot_constraints/core.h b/drivers/boot_constraints/core.h
index 7ba4ac172c09..73b9d2d22a12 100644
--- a/drivers/boot_constraints/core.h
+++ b/drivers/boot_constraints/core.h
@@ -30,4 +30,7 @@ struct constraint {
 };
 
 /* Forward declarations of constraint specific callbacks */
+int constraint_supply_add(struct constraint *constraint, void *data);
+void constraint_supply_remove(struct constraint *constraint);
+
 #endif /* _CORE_H */
diff --git a/drivers/boot_constraints/supply.c b/drivers/boot_constraints/supply.c
new file mode 100644
index 000000000000..30f816dbf12c
--- /dev/null
+++ b/drivers/boot_constraints/supply.c
@@ -0,0 +1,98 @@
+/*
+ * Copyright (C) 2017 Linaro.
+ * Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#define pr_fmt(fmt) "Supply Boot Constraints: " fmt
+
+#include <linux/err.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+#include "core.h"
+
+struct constraint_supply {
+	struct dev_boot_constraint_supply_info supply;
+	struct regulator *reg;
+};
+
+int constraint_supply_add(struct constraint *constraint, void *data)
+{
+	struct dev_boot_constraint_supply_info *supply = data;
+	struct constraint_supply *csupply;
+	struct device *dev = constraint->cdev->dev;
+	int ret;
+
+	csupply = kzalloc(sizeof(*csupply), GFP_KERNEL);
+	if (!csupply)
+		return -ENOMEM;
+
+	csupply->reg = regulator_get(dev, supply->name);
+	if (IS_ERR(csupply->reg)) {
+		ret = PTR_ERR(csupply->reg);
+		if (ret != -EPROBE_DEFER) {
+			dev_err(dev, "regulator_get() failed for %s (%d)\n",
+				supply->name, ret);
+		}
+		goto free;
+	}
+
+	if (supply->u_volt_min != 0 && supply->u_volt_max != 0) {
+		ret = regulator_set_voltage(csupply->reg, supply->u_volt_min,
+					    supply->u_volt_max);
+		if (ret) {
+			dev_err(dev, "regulator_set_voltage %s failed (%d)\n",
+				supply->name, ret);
+			goto free_regulator;
+		}
+	}
+
+	ret = regulator_enable(csupply->reg);
+	if (ret) {
+		dev_err(dev, "regulator_enable %s failed (%d)\n",
+			supply->name, ret);
+		goto remove_voltage;
+	}
+
+	memcpy(&csupply->supply, supply, sizeof(*supply));
+	csupply->supply.name = kstrdup_const(supply->name, GFP_KERNEL);
+	constraint->private = csupply;
+
+	return 0;
+
+remove_voltage:
+	if (supply->u_volt_min != 0 && supply->u_volt_max != 0)
+		regulator_set_voltage(csupply->reg, 0, INT_MAX);
+free_regulator:
+	regulator_put(csupply->reg);
+free:
+	kfree(csupply);
+
+	return ret;
+}
+
+void constraint_supply_remove(struct constraint *constraint)
+{
+	struct constraint_supply *csupply = constraint->private;
+	struct dev_boot_constraint_supply_info *supply = &csupply->supply;
+	struct device *dev = constraint->cdev->dev;
+	int ret;
+
+	kfree_const(supply->name);
+
+	ret = regulator_disable(csupply->reg);
+	if (ret)
+		dev_err(dev, "regulator_disable failed (%d)\n", ret);
+
+	if (supply->u_volt_min != 0 && supply->u_volt_max != 0) {
+		ret = regulator_set_voltage(csupply->reg, 0, INT_MAX);
+		if (ret)
+			dev_err(dev, "regulator_set_voltage failed (%d)\n",
+				ret);
+	}
+
+	regulator_put(csupply->reg);
+	kfree(csupply);
+}
diff --git a/include/linux/boot_constraint.h b/include/linux/boot_constraint.h
index 2b816bf74144..fc8ec0ee82f3 100644
--- a/include/linux/boot_constraint.h
+++ b/include/linux/boot_constraint.h
@@ -15,7 +15,13 @@
 struct device;
 
 enum dev_boot_constraint_type {
-	DEV_BOOT_CONSTRAINT_NONE,
+	DEV_BOOT_CONSTRAINT_SUPPLY,
+};
+
+struct dev_boot_constraint_supply_info {
+	const char *name;
+	unsigned int u_volt_min;
+	unsigned int u_volt_max;
 };
 
 struct dev_boot_constraint {
-- 
2.15.0.rc1.236.g92ea95045093

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH V4 05/12] boot_constraint: Add support for clk constraints
  2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
                   ` (3 preceding siblings ...)
  2017-10-29 13:48 ` [PATCH V4 04/12] boot_constraint: Add support for supply constraints Viresh Kumar
@ 2017-10-29 13:48 ` Viresh Kumar
  2017-12-13  9:48   ` Greg Kroah-Hartman
  2017-10-29 13:48 ` [PATCH V4 06/12] boot_constraint: Add support for PM constraints Viresh Kumar
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 40+ messages in thread
From: Viresh Kumar @ 2017-10-29 13:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Viresh Kumar, Vincent Guittot, Stephen Boyd, Rajendra Nayak,
	linux-kernel, linux-arm-kernel, robdclark, s.hauer, l.stach,
	shawnguo, fabio.estevam, nm, xuwei5, robh+dt

This patch adds the clk constraint type.

The constraint is set by enabling the clk for the device. Once the
device is probed, the clk is disabled and the constraint is removed.

We may want to do clk_set_rate() from here, but lets wait for some real
users that really want it.

Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/boot_constraints/Makefile |  2 +-
 drivers/boot_constraints/clk.c    | 70 +++++++++++++++++++++++++++++++++++++++
 drivers/boot_constraints/core.c   |  4 +++
 drivers/boot_constraints/core.h   |  3 ++
 include/linux/boot_constraint.h   |  5 +++
 5 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 drivers/boot_constraints/clk.c

diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
index a45616f0c3b0..3424379fd1e4 100644
--- a/drivers/boot_constraints/Makefile
+++ b/drivers/boot_constraints/Makefile
@@ -1,3 +1,3 @@
 # Makefile for device boot constraints
 
-obj-y := core.o supply.o
+obj-y := clk.o core.o supply.o
diff --git a/drivers/boot_constraints/clk.c b/drivers/boot_constraints/clk.c
new file mode 100644
index 000000000000..b5b1d63c3e76
--- /dev/null
+++ b/drivers/boot_constraints/clk.c
@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2017 Linaro.
+ * Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#define pr_fmt(fmt) "Clock Boot Constraints: " fmt
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+
+#include "core.h"
+
+struct constraint_clk {
+	struct dev_boot_constraint_clk_info clk_info;
+	struct clk *clk;
+};
+
+int constraint_clk_add(struct constraint *constraint, void *data)
+{
+	struct dev_boot_constraint_clk_info *clk_info = data;
+	struct constraint_clk *cclk;
+	struct device *dev = constraint->cdev->dev;
+	int ret;
+
+	cclk = kzalloc(sizeof(*cclk), GFP_KERNEL);
+	if (!cclk)
+		return -ENOMEM;
+
+	cclk->clk = clk_get(dev, clk_info->name);
+	if (IS_ERR(cclk->clk)) {
+		ret = PTR_ERR(cclk->clk);
+		if (ret != -EPROBE_DEFER) {
+			dev_err(dev, "clk_get() failed for %s (%d)\n",
+				clk_info->name, ret);
+		}
+		goto free;
+	}
+
+	ret = clk_prepare_enable(cclk->clk);
+	if (ret) {
+		dev_err(dev, "clk_prepare_enable() %s failed (%d)\n",
+			clk_info->name, ret);
+		goto put_clk;
+	}
+
+	cclk->clk_info.name = kstrdup_const(clk_info->name, GFP_KERNEL);
+	constraint->private = cclk;
+
+	return 0;
+
+put_clk:
+	clk_put(cclk->clk);
+free:
+	kfree(cclk);
+
+	return ret;
+}
+
+void constraint_clk_remove(struct constraint *constraint)
+{
+	struct constraint_clk *cclk = constraint->private;
+
+	kfree_const(cclk->clk_info.name);
+	clk_disable_unprepare(cclk->clk);
+	clk_put(cclk->clk);
+	kfree(cclk);
+}
diff --git a/drivers/boot_constraints/core.c b/drivers/boot_constraints/core.c
index b9c024a3bdf5..9213e56e8078 100644
--- a/drivers/boot_constraints/core.c
+++ b/drivers/boot_constraints/core.c
@@ -94,6 +94,10 @@ static struct constraint *constraint_allocate(struct constraint_dev *cdev,
 	void (*remove)(struct constraint *constraint);
 
 	switch (type) {
+	case DEV_BOOT_CONSTRAINT_CLK:
+		add = constraint_clk_add;
+		remove = constraint_clk_remove;
+		break;
 	case DEV_BOOT_CONSTRAINT_SUPPLY:
 		add = constraint_supply_add;
 		remove = constraint_supply_remove;
diff --git a/drivers/boot_constraints/core.h b/drivers/boot_constraints/core.h
index 73b9d2d22a12..4f28ac2ef691 100644
--- a/drivers/boot_constraints/core.h
+++ b/drivers/boot_constraints/core.h
@@ -30,6 +30,9 @@ struct constraint {
 };
 
 /* Forward declarations of constraint specific callbacks */
+int constraint_clk_add(struct constraint *constraint, void *data);
+void constraint_clk_remove(struct constraint *constraint);
+
 int constraint_supply_add(struct constraint *constraint, void *data);
 void constraint_supply_remove(struct constraint *constraint);
 
diff --git a/include/linux/boot_constraint.h b/include/linux/boot_constraint.h
index fc8ec0ee82f3..a74b261a4ee4 100644
--- a/include/linux/boot_constraint.h
+++ b/include/linux/boot_constraint.h
@@ -15,9 +15,14 @@
 struct device;
 
 enum dev_boot_constraint_type {
+	DEV_BOOT_CONSTRAINT_CLK,
 	DEV_BOOT_CONSTRAINT_SUPPLY,
 };
 
+struct dev_boot_constraint_clk_info {
+	const char *name;
+};
+
 struct dev_boot_constraint_supply_info {
 	const char *name;
 	unsigned int u_volt_min;
-- 
2.15.0.rc1.236.g92ea95045093

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH V4 06/12] boot_constraint: Add support for PM constraints
  2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
                   ` (4 preceding siblings ...)
  2017-10-29 13:48 ` [PATCH V4 05/12] boot_constraint: Add support for clk constraints Viresh Kumar
@ 2017-10-29 13:48 ` Viresh Kumar
  2017-12-13  9:48   ` Greg Kroah-Hartman
  2017-10-29 13:48 ` [PATCH V4 07/12] boot_constraint: Add debugfs support Viresh Kumar
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 40+ messages in thread
From: Viresh Kumar @ 2017-10-29 13:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Viresh Kumar, Vincent Guittot, Stephen Boyd, Rajendra Nayak,
	linux-kernel, linux-arm-kernel, robdclark, s.hauer, l.stach,
	shawnguo, fabio.estevam, nm, xuwei5, robh+dt

This patch adds the PM constraint type.

The constraint is set by attaching the power domain for the device,
which will also enable the power domain. This guarantees that the power
domain doesn't get shut down while being used.

We don't need to detach the power domain to remove the constraint as the
domain is attached only once, from here or before driver probe.

Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/boot_constraints/Makefile |  2 +-
 drivers/boot_constraints/core.c   |  4 ++++
 drivers/boot_constraints/core.h   |  3 +++
 drivers/boot_constraints/pm.c     | 24 ++++++++++++++++++++++++
 include/linux/boot_constraint.h   |  1 +
 5 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 drivers/boot_constraints/pm.c

diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
index 3424379fd1e4..b7ade1a7afb5 100644
--- a/drivers/boot_constraints/Makefile
+++ b/drivers/boot_constraints/Makefile
@@ -1,3 +1,3 @@
 # Makefile for device boot constraints
 
-obj-y := clk.o core.o supply.o
+obj-y := clk.o core.o pm.o supply.o
diff --git a/drivers/boot_constraints/core.c b/drivers/boot_constraints/core.c
index 9213e56e8078..f4d3520ddb04 100644
--- a/drivers/boot_constraints/core.c
+++ b/drivers/boot_constraints/core.c
@@ -98,6 +98,10 @@ static struct constraint *constraint_allocate(struct constraint_dev *cdev,
 		add = constraint_clk_add;
 		remove = constraint_clk_remove;
 		break;
+	case DEV_BOOT_CONSTRAINT_PM:
+		add = constraint_pm_add;
+		remove = constraint_pm_remove;
+		break;
 	case DEV_BOOT_CONSTRAINT_SUPPLY:
 		add = constraint_supply_add;
 		remove = constraint_supply_remove;
diff --git a/drivers/boot_constraints/core.h b/drivers/boot_constraints/core.h
index 4f28ac2ef691..a051c3d7c8ab 100644
--- a/drivers/boot_constraints/core.h
+++ b/drivers/boot_constraints/core.h
@@ -33,6 +33,9 @@ struct constraint {
 int constraint_clk_add(struct constraint *constraint, void *data);
 void constraint_clk_remove(struct constraint *constraint);
 
+int constraint_pm_add(struct constraint *constraint, void *data);
+void constraint_pm_remove(struct constraint *constraint);
+
 int constraint_supply_add(struct constraint *constraint, void *data);
 void constraint_supply_remove(struct constraint *constraint);
 
diff --git a/drivers/boot_constraints/pm.c b/drivers/boot_constraints/pm.c
new file mode 100644
index 000000000000..edba5eca5093
--- /dev/null
+++ b/drivers/boot_constraints/pm.c
@@ -0,0 +1,24 @@
+/*
+ * Copyright (C) 2017 Linaro.
+ * Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#define pr_fmt(fmt) "PM Boot Constraints: " fmt
+
+#include <linux/pm_domain.h>
+
+#include "core.h"
+
+int constraint_pm_add(struct constraint *constraint, void *data)
+{
+	struct device *dev = constraint->cdev->dev;
+
+	return dev_pm_domain_attach(dev, true);
+}
+
+void constraint_pm_remove(struct constraint *constraint)
+{
+	/* Nothing to do for now */
+}
diff --git a/include/linux/boot_constraint.h b/include/linux/boot_constraint.h
index a74b261a4ee4..637fe9d65536 100644
--- a/include/linux/boot_constraint.h
+++ b/include/linux/boot_constraint.h
@@ -16,6 +16,7 @@ struct device;
 
 enum dev_boot_constraint_type {
 	DEV_BOOT_CONSTRAINT_CLK,
+	DEV_BOOT_CONSTRAINT_PM,
 	DEV_BOOT_CONSTRAINT_SUPPLY,
 };
 
-- 
2.15.0.rc1.236.g92ea95045093

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH V4 07/12] boot_constraint: Add debugfs support
  2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
                   ` (5 preceding siblings ...)
  2017-10-29 13:48 ` [PATCH V4 06/12] boot_constraint: Add support for PM constraints Viresh Kumar
@ 2017-10-29 13:48 ` Viresh Kumar
  2017-10-29 15:09   ` Randy Dunlap
  2017-12-13  9:50   ` Greg Kroah-Hartman
  2017-10-29 13:48 ` [PATCH V4 08/12] boot_constraint: Manage deferrable constraints Viresh Kumar
                   ` (6 subsequent siblings)
  13 siblings, 2 replies; 40+ messages in thread
From: Viresh Kumar @ 2017-10-29 13:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Viresh Kumar, Vincent Guittot, Stephen Boyd, Rajendra Nayak,
	linux-kernel, linux-arm-kernel, robdclark, s.hauer, l.stach,
	shawnguo, fabio.estevam, nm, xuwei5, robh+dt

This patch adds debugfs support for boot constraints. This is how it
looks for a "vmmc-supply" constraint for the MMC device.

$ ls -R /sys/kernel/debug/boot_constraints/
/sys/kernel/debug/boot_constraints/:
f723d000.dwmmc0

/sys/kernel/debug/boot_constraints/f723d000.dwmmc0:
clk-ciu  pm-domain  supply-vmmc  supply-vmmcaux

/sys/kernel/debug/boot_constraints/f723d000.dwmmc0/clk-ciu:

/sys/kernel/debug/boot_constraints/f723d000.dwmmc0/pm-domain:

/sys/kernel/debug/boot_constraints/f723d000.dwmmc0/supply-vmmc:
u_volt_max  u_volt_min

/sys/kernel/debug/boot_constraints/f723d000.dwmmc0/supply-vmmcaux:
u_volt_max  u_volt_min

Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/boot_constraints/clk.c    |  3 ++
 drivers/boot_constraints/core.c   | 60 +++++++++++++++++++++++++++++++++++++++
 drivers/boot_constraints/core.h   |  6 ++++
 drivers/boot_constraints/pm.c     | 11 +++++--
 drivers/boot_constraints/supply.c |  9 ++++++
 5 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/drivers/boot_constraints/clk.c b/drivers/boot_constraints/clk.c
index b5b1d63c3e76..91b7b538ef32 100644
--- a/drivers/boot_constraints/clk.c
+++ b/drivers/boot_constraints/clk.c
@@ -49,6 +49,8 @@ int constraint_clk_add(struct constraint *constraint, void *data)
 	cclk->clk_info.name = kstrdup_const(clk_info->name, GFP_KERNEL);
 	constraint->private = cclk;
 
+	constraint_add_debugfs(constraint, clk_info->name);
+
 	return 0;
 
 put_clk:
@@ -63,6 +65,7 @@ void constraint_clk_remove(struct constraint *constraint)
 {
 	struct constraint_clk *cclk = constraint->private;
 
+	constraint_remove_debugfs(constraint);
 	kfree_const(cclk->clk_info.name);
 	clk_disable_unprepare(cclk->clk);
 	clk_put(cclk->clk);
diff --git a/drivers/boot_constraints/core.c b/drivers/boot_constraints/core.c
index f4d3520ddb04..707ffac690fc 100644
--- a/drivers/boot_constraints/core.c
+++ b/drivers/boot_constraints/core.c
@@ -24,6 +24,64 @@
 static LIST_HEAD(constraint_devices);
 static DEFINE_MUTEX(constraint_devices_mutex);
 
+/* Debugfs */
+
+static struct dentry *rootdir;
+
+static void constraint_device_add_debugfs(struct constraint_dev *cdev)
+{
+	struct device *dev = cdev->dev;
+
+	cdev->dentry = debugfs_create_dir(dev_name(dev), rootdir);
+}
+
+static void constraint_device_remove_debugfs(struct constraint_dev *cdev)
+{
+	debugfs_remove_recursive(cdev->dentry);
+}
+
+void constraint_add_debugfs(struct constraint *constraint, const char *suffix)
+{
+	struct device *dev = constraint->cdev->dev;
+	const char *prefix;
+	char name[NAME_MAX];
+
+	switch (constraint->type) {
+	case DEV_BOOT_CONSTRAINT_CLK:
+		prefix = "clk";
+		break;
+	case DEV_BOOT_CONSTRAINT_PM:
+		prefix = "pm";
+		break;
+	case DEV_BOOT_CONSTRAINT_SUPPLY:
+		prefix = "supply";
+		break;
+	default:
+		dev_err(dev, "%s: Constraint type (%d) not supported\n",
+			__func__, constraint->type);
+		return;
+	}
+
+	snprintf(name, NAME_MAX, "%s-%s", prefix, suffix);
+
+	constraint->dentry = debugfs_create_dir(name, constraint->cdev->dentry);
+}
+
+void constraint_remove_debugfs(struct constraint *constraint)
+{
+	debugfs_remove_recursive(constraint->dentry);
+}
+
+static int __init constraint_debugfs_init(void)
+{
+	/* Create /sys/kernel/debug/opp directory */
+	rootdir = debugfs_create_dir("boot_constraints", NULL);
+
+	return 0;
+}
+core_initcall(constraint_debugfs_init);
+
+
 /* Boot constraints core */
 
 static struct constraint_dev *constraint_device_find(struct device *dev)
@@ -51,12 +109,14 @@ static struct constraint_dev *constraint_device_allocate(struct device *dev)
 	INIT_LIST_HEAD(&cdev->constraints);
 
 	list_add(&cdev->node, &constraint_devices);
+	constraint_device_add_debugfs(cdev);
 
 	return cdev;
 }
 
 static void constraint_device_free(struct constraint_dev *cdev)
 {
+	constraint_device_remove_debugfs(cdev);
 	list_del(&cdev->node);
 	kfree(cdev);
 }
diff --git a/drivers/boot_constraints/core.h b/drivers/boot_constraints/core.h
index a051c3d7c8ab..ee84e237c66a 100644
--- a/drivers/boot_constraints/core.h
+++ b/drivers/boot_constraints/core.h
@@ -8,6 +8,7 @@
 #define _CORE_H
 
 #include <linux/boot_constraint.h>
+#include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/list.h>
 
@@ -15,6 +16,7 @@ struct constraint_dev {
 	struct device *dev;
 	struct list_head node;
 	struct list_head constraints;
+	struct dentry *dentry;
 };
 
 struct constraint {
@@ -23,12 +25,16 @@ struct constraint {
 	enum dev_boot_constraint_type type;
 	void (*free_resources)(void *data);
 	void *free_resources_data;
+	struct dentry *dentry;
 
 	int (*add)(struct constraint *constraint, void *data);
 	void (*remove)(struct constraint *constraint);
 	void *private;
 };
 
+void constraint_add_debugfs(struct constraint *constraint, const char *suffix);
+void constraint_remove_debugfs(struct constraint *constraint);
+
 /* Forward declarations of constraint specific callbacks */
 int constraint_clk_add(struct constraint *constraint, void *data);
 void constraint_clk_remove(struct constraint *constraint);
diff --git a/drivers/boot_constraints/pm.c b/drivers/boot_constraints/pm.c
index edba5eca5093..d74b776cdced 100644
--- a/drivers/boot_constraints/pm.c
+++ b/drivers/boot_constraints/pm.c
@@ -14,11 +14,18 @@
 int constraint_pm_add(struct constraint *constraint, void *data)
 {
 	struct device *dev = constraint->cdev->dev;
+	int ret;
 
-	return dev_pm_domain_attach(dev, true);
+	ret = dev_pm_domain_attach(dev, true);
+	if (ret)
+		return ret;
+
+	constraint_add_debugfs(constraint, "domain");
+
+	return 0;
 }
 
 void constraint_pm_remove(struct constraint *constraint)
 {
-	/* Nothing to do for now */
+	constraint_remove_debugfs(constraint);
 }
diff --git a/drivers/boot_constraints/supply.c b/drivers/boot_constraints/supply.c
index 30f816dbf12c..3d658410a082 100644
--- a/drivers/boot_constraints/supply.c
+++ b/drivers/boot_constraints/supply.c
@@ -60,6 +60,14 @@ int constraint_supply_add(struct constraint *constraint, void *data)
 	csupply->supply.name = kstrdup_const(supply->name, GFP_KERNEL);
 	constraint->private = csupply;
 
+	constraint_add_debugfs(constraint, supply->name);
+
+	debugfs_create_u32("u_volt_min", 0444, constraint->dentry,
+			   &csupply->supply.u_volt_min);
+
+	debugfs_create_u32("u_volt_max", 0444, constraint->dentry,
+			   &csupply->supply.u_volt_max);
+
 	return 0;
 
 remove_voltage:
@@ -80,6 +88,7 @@ void constraint_supply_remove(struct constraint *constraint)
 	struct device *dev = constraint->cdev->dev;
 	int ret;
 
+	constraint_remove_debugfs(constraint);
 	kfree_const(supply->name);
 
 	ret = regulator_disable(csupply->reg);
-- 
2.15.0.rc1.236.g92ea95045093

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH V4 08/12] boot_constraint: Manage deferrable constraints
  2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
                   ` (6 preceding siblings ...)
  2017-10-29 13:48 ` [PATCH V4 07/12] boot_constraint: Add debugfs support Viresh Kumar
@ 2017-10-29 13:48 ` Viresh Kumar
  2017-10-31 16:20   ` Rob Herring
  2017-12-13  9:53   ` Greg Kroah-Hartman
  2017-10-29 13:48 ` [PATCH V4 09/12] boot_constraint: Add earlycon helper Viresh Kumar
                   ` (5 subsequent siblings)
  13 siblings, 2 replies; 40+ messages in thread
From: Viresh Kumar @ 2017-10-29 13:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Viresh Kumar, Vincent Guittot, Stephen Boyd, Rajendra Nayak,
	linux-kernel, linux-arm-kernel, robdclark, s.hauer, l.stach,
	shawnguo, fabio.estevam, nm, xuwei5, robh+dt

It is possible that some of the resources aren't available at the time
constraints are getting set and the boot constraints core will return
-EPROBE_DEFER for them. In order to retry adding the constraints at a
later point of time (after the resource is added and before any of its
users come up), this patch proposes two things:

- Each constraint is represented by a virtual platform device, so that
  it is re-probed again until the time all the dependencies aren't met.
  The platform device is removed along with the constraint, with help of
  the free_resources() callback.

- Enable early defer probing support by calling
  driver_enable_deferred_probe(), so that the core retries probing
  deferred devices every time any device is bound to a driver. This
  makes sure that the constraint is set before any of the users of the
  resources come up.

This is tested on ARM64 Hikey board where probe was deferred for a
device.

Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/dd.c                         |  12 ++
 drivers/boot_constraints/Makefile         |   2 +-
 drivers/boot_constraints/deferrable_dev.c | 235 ++++++++++++++++++++++++++++++
 include/linux/boot_constraint.h           |  14 ++
 4 files changed, 262 insertions(+), 1 deletion(-)
 create mode 100644 drivers/boot_constraints/deferrable_dev.c

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 4eec27fe2b2b..19eff5d08b9a 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -228,6 +228,18 @@ void device_unblock_probing(void)
 	driver_deferred_probe_trigger();
 }
 
+/**
+ * driver_enable_deferred_probe() - Enable probing of deferred devices
+ *
+ * We don't want to get in the way when the bulk of drivers are getting probed
+ * and so deferred probe is disabled in the beginning. Enable it now because we
+ * need it.
+ */
+void driver_enable_deferred_probe(void)
+{
+	driver_deferred_probe_enable = true;
+}
+
 /**
  * deferred_probe_initcall() - Enable probing of deferred devices
  *
diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
index b7ade1a7afb5..a765094623a3 100644
--- a/drivers/boot_constraints/Makefile
+++ b/drivers/boot_constraints/Makefile
@@ -1,3 +1,3 @@
 # Makefile for device boot constraints
 
-obj-y := clk.o core.o pm.o supply.o
+obj-y := clk.o deferrable_dev.o core.o pm.o supply.o
diff --git a/drivers/boot_constraints/deferrable_dev.c b/drivers/boot_constraints/deferrable_dev.c
new file mode 100644
index 000000000000..04056f317aff
--- /dev/null
+++ b/drivers/boot_constraints/deferrable_dev.c
@@ -0,0 +1,235 @@
+/*
+ * Copyright (C) 2017 Linaro.
+ * Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#define pr_fmt(fmt) "Boot Constraints: " fmt
+
+#include <linux/amba/bus.h>
+#include <linux/err.h>
+#include <linux/idr.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include "core.h"
+
+static DEFINE_IDA(pdev_index);
+
+void driver_enable_deferred_probe(void);
+
+struct boot_constraint_pdata {
+	struct device *dev;
+	struct dev_boot_constraint constraint;
+	int probe_failed;
+	int index;
+};
+
+static void boot_constraint_remove(void *data)
+{
+	struct platform_device *pdev = data;
+	struct boot_constraint_pdata *pdata = dev_get_platdata(&pdev->dev);
+
+	ida_simple_remove(&pdev_index, pdata->index);
+	kfree(pdata->constraint.data);
+	platform_device_unregister(pdev);
+}
+
+/*
+ * A platform device is added for each and every constraint, to handle
+ * -EPROBE_DEFER properly.
+ */
+static int boot_constraint_probe(struct platform_device *pdev)
+{
+	struct boot_constraint_pdata *pdata = dev_get_platdata(&pdev->dev);
+	struct dev_boot_constraint_info info;
+	int ret;
+
+	if (WARN_ON(!pdata))
+		return -EINVAL;
+
+	info.constraint = pdata->constraint;
+	info.free_resources = boot_constraint_remove;
+	info.free_resources_data = pdev;
+
+	ret = dev_boot_constraint_add(pdata->dev, &info);
+	if (ret) {
+		if (ret == -EPROBE_DEFER)
+			driver_enable_deferred_probe();
+		else
+			pdata->probe_failed = ret;
+	}
+
+	return ret;
+}
+
+static struct platform_driver boot_constraint_driver = {
+	.driver = {
+		.name = "boot-constraints-dev",
+	},
+	.probe = boot_constraint_probe,
+};
+
+static int __init boot_constraint_init(void)
+{
+	return platform_driver_register(&boot_constraint_driver);
+}
+core_initcall(boot_constraint_init);
+
+static int boot_constraint_add_dev(struct device *dev,
+				   struct dev_boot_constraint *constraint)
+{
+	struct boot_constraint_pdata pdata = {
+		.dev = dev,
+		.constraint.type = constraint->type,
+	};
+	struct platform_device *pdev;
+	struct boot_constraint_pdata *pdev_pdata;
+	int size, ret;
+
+	switch (constraint->type) {
+	case DEV_BOOT_CONSTRAINT_CLK:
+		size = sizeof(struct dev_boot_constraint_clk_info);
+		break;
+	case DEV_BOOT_CONSTRAINT_PM:
+		size = 0;
+		break;
+	case DEV_BOOT_CONSTRAINT_SUPPLY:
+		size = sizeof(struct dev_boot_constraint_supply_info);
+		break;
+	default:
+		dev_err(dev, "%s: Constraint type (%d) not supported\n",
+			__func__, constraint->type);
+		return -EINVAL;
+	}
+
+	/* Will be freed from boot_constraint_remove() */
+	pdata.constraint.data = kmemdup(constraint->data, size, GFP_KERNEL);
+	if (!pdata.constraint.data)
+		return -ENOMEM;
+
+	ret = ida_simple_get(&pdev_index, 0, 256, GFP_KERNEL);
+	if (ret < 0) {
+		dev_err(dev, "failed to allocate index (%d)\n", ret);
+		goto free;
+	}
+
+	pdata.index = ret;
+
+	pdev = platform_device_register_data(NULL, "boot-constraints-dev", ret,
+					     &pdata, sizeof(pdata));
+	if (IS_ERR(pdev)) {
+		dev_err(dev, "%s: Failed to create pdev (%ld)\n", __func__,
+			PTR_ERR(pdev));
+		ret = PTR_ERR(pdev);
+		goto ida_remove;
+	}
+
+	/* Release resources if probe has failed */
+	pdev_pdata = dev_get_platdata(&pdev->dev);
+	if (pdev_pdata->probe_failed) {
+		ret = pdev_pdata->probe_failed;
+		goto remove_pdev;
+	}
+
+	return 0;
+
+remove_pdev:
+	platform_device_unregister(pdev);
+ida_remove:
+	ida_simple_remove(&pdev_index, pdata.index);
+free:
+	kfree(pdata.constraint.data);
+
+	return ret;
+}
+
+static int dev_boot_constraint_add_deferrable(struct device *dev,
+			struct dev_boot_constraint *constraints, int count)
+{
+	int ret, i;
+
+	for (i = 0; i < count; i++) {
+		ret = boot_constraint_add_dev(dev, &constraints[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/* This only creates platform devices for now */
+static void add_deferrable_of_single(struct device_node *np,
+				     struct dev_boot_constraint *constraints,
+				     int count)
+{
+	struct device *dev;
+	int ret;
+
+	if (!of_device_is_available(np))
+		return;
+
+	ret = of_platform_bus_create(np, NULL, NULL, NULL, false);
+	if (ret)
+		return;
+
+	if (of_device_is_compatible(np, "arm,primecell")) {
+		struct amba_device *adev = of_find_amba_device_by_node(np);
+
+		if (!adev) {
+			pr_err("Failed to find amba dev: %s\n", np->full_name);
+			return;
+		}
+		dev = &adev->dev;
+	} else {
+		struct platform_device *pdev = of_find_device_by_node(np);
+
+		if (!pdev) {
+			pr_err("Failed to find pdev: %s\n", np->full_name);
+			return;
+		}
+		dev = &pdev->dev;
+	}
+
+	ret = dev_boot_constraint_add_deferrable(dev, constraints, count);
+	if (ret)
+		dev_err(dev, "Failed to add boot constraint (%d)\n", ret);
+}
+
+/* Not all compatible device nodes may have boot constraints */
+static bool node_has_boot_constraints(struct device_node *np,
+				      struct dev_boot_constraint_of *oconst)
+{
+	int i;
+
+	if (!oconst->dev_names)
+		return true;
+
+	for (i = 0; i < oconst->dev_names_count; i++) {
+		if (!strcmp(oconst->dev_names[i], kbasename(np->full_name)))
+			return true;
+	}
+
+	return false;
+}
+
+void dev_boot_constraint_add_deferrable_of(struct dev_boot_constraint_of *oconst,
+					   int count)
+{
+	struct device_node *np;
+	int i;
+
+	for (i = 0; i < count; i++) {
+		for_each_compatible_node(np, NULL, oconst[i].compat) {
+			if (!node_has_boot_constraints(np, &oconst[i]))
+				continue;
+
+			add_deferrable_of_single(np, oconst[i].constraints,
+						 oconst[i].count);
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(dev_boot_constraint_add_deferrable_of);
diff --git a/include/linux/boot_constraint.h b/include/linux/boot_constraint.h
index 637fe9d65536..c110b36e490f 100644
--- a/include/linux/boot_constraint.h
+++ b/include/linux/boot_constraint.h
@@ -35,6 +35,15 @@ struct dev_boot_constraint {
 	void *data;
 };
 
+struct dev_boot_constraint_of {
+	const char *compat;
+	struct dev_boot_constraint *constraints;
+	unsigned int count;
+
+	const char **dev_names;
+	unsigned int dev_names_count;
+};
+
 struct dev_boot_constraint_info {
 	struct dev_boot_constraint constraint;
 
@@ -47,12 +56,17 @@ struct dev_boot_constraint_info {
 int dev_boot_constraint_add(struct device *dev,
 			    struct dev_boot_constraint_info *info);
 void dev_boot_constraints_remove(struct device *dev);
+void dev_boot_constraint_add_deferrable_of(struct dev_boot_constraint_of *oconst,
+					   int count);
 #else
 static inline
 int dev_boot_constraint_add(struct device *dev,
 			    struct dev_boot_constraint_info *info)
 { return -EINVAL; }
 static inline void dev_boot_constraints_remove(struct device *dev) {}
+void dev_boot_constraint_add_deferrable_of(struct dev_boot_constraint_of *oconst,
+					   int count)
+{ }
 #endif /* CONFIG_DEV_BOOT_CONSTRAINTS */
 
 #endif /* _LINUX_BOOT_CONSTRAINT_H */
-- 
2.15.0.rc1.236.g92ea95045093

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH V4 09/12] boot_constraint: Add earlycon helper
  2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
                   ` (7 preceding siblings ...)
  2017-10-29 13:48 ` [PATCH V4 08/12] boot_constraint: Manage deferrable constraints Viresh Kumar
@ 2017-10-29 13:48 ` Viresh Kumar
  2017-12-13  9:44   ` Greg Kroah-Hartman
  2017-10-29 13:48 ` [PATCH V4 10/12] boot_constraint: Add support for Hisilicon platforms Viresh Kumar
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 40+ messages in thread
From: Viresh Kumar @ 2017-10-29 13:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Viresh Kumar, Vincent Guittot, Stephen Boyd, Rajendra Nayak,
	linux-kernel, linux-arm-kernel, robdclark, s.hauer, l.stach,
	shawnguo, fabio.estevam, nm, xuwei5, robh+dt

Getting boot messages during initial kernel boot is a common problem,
which (almost) everyone wants to solve. Considering that this would be
required by multiple platforms, provide a helper to check if "earlycon"
or "earlyprintk" boot arguments are passed to kernel or not. The
platforms can use this helper to add serial constraints only if earlycon
if required.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/boot_constraints/Makefile |  2 +-
 drivers/boot_constraints/serial.c | 28 ++++++++++++++++++++++++++++
 include/linux/boot_constraint.h   |  2 ++
 3 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100644 drivers/boot_constraints/serial.c

diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
index a765094623a3..0d4f88bb767c 100644
--- a/drivers/boot_constraints/Makefile
+++ b/drivers/boot_constraints/Makefile
@@ -1,3 +1,3 @@
 # Makefile for device boot constraints
 
-obj-y := clk.o deferrable_dev.o core.o pm.o supply.o
+obj-y := clk.o deferrable_dev.o core.o pm.o serial.o supply.o
diff --git a/drivers/boot_constraints/serial.c b/drivers/boot_constraints/serial.c
new file mode 100644
index 000000000000..d0d07d4aa6af
--- /dev/null
+++ b/drivers/boot_constraints/serial.c
@@ -0,0 +1,28 @@
+/*
+ * This contains helpers related to serial boot constraints.
+ *
+ * Copyright (C) 2017 Linaro.
+ * Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/init.h>
+
+static bool earlycon_boot_constraints_enabled __initdata;
+
+bool __init boot_constraint_earlycon_enabled(void)
+{
+	return earlycon_boot_constraints_enabled;
+}
+
+static int __init enable_earlycon_boot_constraints(char *str)
+{
+	earlycon_boot_constraints_enabled = true;
+
+	return 0;
+}
+__setup_param("earlycon", boot_constraint_earlycon,
+	      enable_earlycon_boot_constraints, 0);
+__setup_param("earlyprintk", boot_constraint_earlyprintk,
+	      enable_earlycon_boot_constraints, 0);
diff --git a/include/linux/boot_constraint.h b/include/linux/boot_constraint.h
index c110b36e490f..aeada69b87e6 100644
--- a/include/linux/boot_constraint.h
+++ b/include/linux/boot_constraint.h
@@ -10,6 +10,7 @@
 #define _LINUX_BOOT_CONSTRAINT_H
 
 #include <linux/err.h>
+#include <linux/init.h>
 #include <linux/types.h>
 
 struct device;
@@ -58,6 +59,7 @@ int dev_boot_constraint_add(struct device *dev,
 void dev_boot_constraints_remove(struct device *dev);
 void dev_boot_constraint_add_deferrable_of(struct dev_boot_constraint_of *oconst,
 					   int count);
+bool __init boot_constraint_earlycon_enabled(void);
 #else
 static inline
 int dev_boot_constraint_add(struct device *dev,
-- 
2.15.0.rc1.236.g92ea95045093

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH V4 10/12] boot_constraint: Add support for Hisilicon platforms
  2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
                   ` (8 preceding siblings ...)
  2017-10-29 13:48 ` [PATCH V4 09/12] boot_constraint: Add earlycon helper Viresh Kumar
@ 2017-10-29 13:48 ` Viresh Kumar
  2017-12-13  9:47   ` Greg Kroah-Hartman
  2017-10-29 13:48 ` [PATCH V4 11/12] boot_constraint: Add support for IMX platform Viresh Kumar
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 40+ messages in thread
From: Viresh Kumar @ 2017-10-29 13:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Viresh Kumar, Vincent Guittot, Stephen Boyd, Rajendra Nayak,
	linux-kernel, linux-arm-kernel, robdclark, s.hauer, l.stach,
	shawnguo, fabio.estevam, nm, xuwei5, robh+dt

This adds boot constraint support for Hisilicon platforms. Currently
only one use case is supported: earlycon. One of the UART is enabled by
the bootloader and is used for early console in the kernel. The boot
constraint core handles it properly and removes constraints once the
serial device is probed by its driver.

This is tested on hi6220-hikey 96board.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm64/Kconfig.platforms      |   1 +
 drivers/boot_constraints/Makefile |   2 +
 drivers/boot_constraints/hikey.c  | 145 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 148 insertions(+)
 create mode 100644 drivers/boot_constraints/hikey.c

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 6b54ee8c1262..265df4a088ab 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -87,6 +87,7 @@ config ARCH_HISI
 	select ARM_TIMER_SP804
 	select HISILICON_IRQ_MBIGEN if PCI
 	select PINCTRL
+	select DEV_BOOT_CONSTRAINTS
 	help
 	  This enables support for Hisilicon ARMv8 SoC family
 
diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
index 0d4f88bb767c..43c89d2458e9 100644
--- a/drivers/boot_constraints/Makefile
+++ b/drivers/boot_constraints/Makefile
@@ -1,3 +1,5 @@
 # Makefile for device boot constraints
 
 obj-y := clk.o deferrable_dev.o core.o pm.o serial.o supply.o
+
+obj-$(CONFIG_ARCH_HISI)			+= hikey.o
diff --git a/drivers/boot_constraints/hikey.c b/drivers/boot_constraints/hikey.c
new file mode 100644
index 000000000000..5f69f9451d93
--- /dev/null
+++ b/drivers/boot_constraints/hikey.c
@@ -0,0 +1,145 @@
+/*
+ * This takes care of Hisilicon boot time device constraints, normally set by
+ * the Bootloader.
+ *
+ * Copyright (C) 2017 Linaro.
+ * Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/boot_constraint.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+
+struct hikey_machine_constraints {
+	struct dev_boot_constraint_of *dev_constraints;
+	unsigned int count;
+};
+
+static struct dev_boot_constraint_clk_info uart_iclk_info = {
+	.name = "uartclk",
+};
+
+static struct dev_boot_constraint_clk_info uart_pclk_info = {
+	.name = "apb_pclk",
+};
+
+static struct dev_boot_constraint hikey3660_uart_constraints[] = {
+	{
+		.type = DEV_BOOT_CONSTRAINT_CLK,
+		.data = &uart_iclk_info,
+	}, {
+		.type = DEV_BOOT_CONSTRAINT_CLK,
+		.data = &uart_pclk_info,
+	},
+};
+
+static const char * const uarts_hikey3660[] = {
+	"serial@fff32000",	/* UART 6 */
+};
+
+static struct dev_boot_constraint_of hikey3660_dev_constraints[] = {
+	{
+		.compat = "arm,pl011",
+		.constraints = hikey3660_uart_constraints,
+		.count = ARRAY_SIZE(hikey3660_uart_constraints),
+
+		.dev_names = uarts_hikey3660,
+		.dev_names_count = ARRAY_SIZE(uarts_hikey3660),
+	},
+};
+
+static struct hikey_machine_constraints hikey3660_constraints = {
+	.dev_constraints = hikey3660_dev_constraints,
+	.count = ARRAY_SIZE(hikey3660_dev_constraints),
+};
+
+static const char * const uarts_hikey6220[] = {
+	"uart@f7113000",	/* UART 3 */
+};
+
+static struct dev_boot_constraint_of hikey6220_dev_constraints[] = {
+	{
+		.compat = "arm,pl011",
+		.constraints = hikey3660_uart_constraints,
+		.count = ARRAY_SIZE(hikey3660_uart_constraints),
+
+		.dev_names = uarts_hikey6220,
+		.dev_names_count = ARRAY_SIZE(uarts_hikey6220),
+	},
+};
+
+static struct hikey_machine_constraints hikey6220_constraints = {
+	.dev_constraints = hikey6220_dev_constraints,
+	.count = ARRAY_SIZE(hikey6220_dev_constraints),
+};
+
+static struct dev_boot_constraint hikey3798cv200_uart_constraints[] = {
+	{
+		.type = DEV_BOOT_CONSTRAINT_CLK,
+		.data = &uart_pclk_info,
+	},
+};
+
+static const char * const uarts_hikey3798cv200[] = {
+	"serial@8b00000",	/* UART 0 */
+};
+
+static struct dev_boot_constraint_of hikey3798cv200_dev_constraints[] = {
+	{
+		.compat = "arm,pl011",
+		.constraints = hikey3798cv200_uart_constraints,
+		.count = ARRAY_SIZE(hikey3798cv200_uart_constraints),
+
+		.dev_names = uarts_hikey3798cv200,
+		.dev_names_count = ARRAY_SIZE(uarts_hikey3798cv200),
+	},
+};
+
+static struct hikey_machine_constraints hikey3798cv200_constraints = {
+	.dev_constraints = hikey3798cv200_dev_constraints,
+	.count = ARRAY_SIZE(hikey3798cv200_dev_constraints),
+};
+
+static const struct of_device_id machines[] __initconst = {
+	{ .compatible = "hisilicon,hi3660", .data = &hikey3660_constraints },
+	{ .compatible = "hisilicon,hi3798cv200", .data = &hikey3798cv200_constraints },
+	{ .compatible = "hisilicon,hi6220", .data = &hikey6220_constraints },
+	{ }
+};
+
+static int __init hikey_constraints_init(void)
+{
+	const struct hikey_machine_constraints *constraints;
+	const struct of_device_id *match;
+	struct device_node *np;
+
+	if (!boot_constraint_earlycon_enabled())
+		return 0;
+
+	np = of_find_node_by_path("/");
+	if (!np)
+		return -ENODEV;
+
+	match = of_match_node(machines, np);
+	of_node_put(np);
+
+	if (!match)
+		return 0;
+
+	constraints = match->data;
+	BUG_ON(!constraints);
+
+	dev_boot_constraint_add_deferrable_of(constraints->dev_constraints,
+					      constraints->count);
+
+	return 0;
+}
+
+/*
+ * The amba-pl011 driver registers itself from arch_initcall level. Setup the
+ * serial boot constraints before that in order not to miss any boot messages.
+ */
+postcore_initcall_sync(hikey_constraints_init);
-- 
2.15.0.rc1.236.g92ea95045093

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH V4 11/12] boot_constraint: Add support for IMX platform
  2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
                   ` (9 preceding siblings ...)
  2017-10-29 13:48 ` [PATCH V4 10/12] boot_constraint: Add support for Hisilicon platforms Viresh Kumar
@ 2017-10-29 13:48 ` Viresh Kumar
  2017-11-03  8:58   ` Sascha Hauer
  2017-10-29 13:49 ` [PATCH V4 12/12] boot_constraint: Add Qualcomm display controller constraints Viresh Kumar
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 40+ messages in thread
From: Viresh Kumar @ 2017-10-29 13:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Viresh Kumar, Vincent Guittot, Stephen Boyd, Rajendra Nayak,
	linux-kernel, linux-arm-kernel, robdclark, s.hauer, l.stach,
	shawnguo, fabio.estevam, nm, xuwei5, robh+dt

This adds boot constraint support for IMX platforms. Currently only one
use case is supported: earlycon. Some of the UARTs are enabled by the
bootloader and are used for early console in the kernel. The boot
constraint core handles them properly and removes them once the serial
device is probed by its driver.

This gets rid of lots of hacky code in the clock drivers.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm/mach-imx/Kconfig         |   1 +
 drivers/boot_constraints/Makefile |   1 +
 drivers/boot_constraints/imx.c    | 113 ++++++++++++++++++++++++++++++++++++++
 drivers/clk/imx/clk-imx25.c       |  12 ----
 drivers/clk/imx/clk-imx27.c       |  13 -----
 drivers/clk/imx/clk-imx31.c       |  12 ----
 drivers/clk/imx/clk-imx35.c       |  10 ----
 drivers/clk/imx/clk-imx51-imx53.c |  16 ------
 drivers/clk/imx/clk-imx6q.c       |   8 ---
 drivers/clk/imx/clk-imx6sl.c      |   8 ---
 drivers/clk/imx/clk-imx6sx.c      |   8 ---
 drivers/clk/imx/clk-imx7d.c       |  14 -----
 drivers/clk/imx/clk.c             |  38 -------------
 drivers/clk/imx/clk.h             |   1 -
 14 files changed, 115 insertions(+), 140 deletions(-)
 create mode 100644 drivers/boot_constraints/imx.c

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 782699e67600..9ea1fe32b280 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -4,6 +4,7 @@ menuconfig ARCH_MXC
 	select ARCH_SUPPORTS_BIG_ENDIAN
 	select CLKSRC_IMX_GPT
 	select GENERIC_IRQ_CHIP
+	select DEV_BOOT_CONSTRAINTS
 	select GPIOLIB
 	select PINCTRL
 	select PM_OPP if PM
diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
index 43c89d2458e9..3b5a87fcf099 100644
--- a/drivers/boot_constraints/Makefile
+++ b/drivers/boot_constraints/Makefile
@@ -3,3 +3,4 @@
 obj-y := clk.o deferrable_dev.o core.o pm.o serial.o supply.o
 
 obj-$(CONFIG_ARCH_HISI)			+= hikey.o
+obj-$(CONFIG_ARCH_MXC)			+= imx.o
diff --git a/drivers/boot_constraints/imx.c b/drivers/boot_constraints/imx.c
new file mode 100644
index 000000000000..a402dfdea1bb
--- /dev/null
+++ b/drivers/boot_constraints/imx.c
@@ -0,0 +1,113 @@
+/*
+ * This takes care of IMX boot time device constraints, normally set by the
+ * Bootloader.
+ *
+ * Copyright (C) 2017 Linaro.
+ * Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/boot_constraint.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+
+struct imx_machine_constraints {
+	struct dev_boot_constraint_of *dev_constraints;
+	unsigned int count;
+};
+
+static struct dev_boot_constraint_clk_info uart_ipg_clk_info = {
+	.name = "ipg",
+};
+
+static struct dev_boot_constraint_clk_info uart_per_clk_info = {
+	.name = "per",
+};
+
+static struct dev_boot_constraint imx_uart_constraints[] = {
+	{
+		.type = DEV_BOOT_CONSTRAINT_CLK,
+		.data = &uart_ipg_clk_info,
+	}, {
+		.type = DEV_BOOT_CONSTRAINT_CLK,
+		.data = &uart_per_clk_info,
+	},
+};
+
+static struct dev_boot_constraint_of imx_dev_constraints[] = {
+	{
+		.compat = "fsl,imx21-uart",
+		.constraints = imx_uart_constraints,
+		.count = ARRAY_SIZE(imx_uart_constraints),
+	},
+};
+
+static struct imx_machine_constraints imx_constraints = {
+	.dev_constraints = imx_dev_constraints,
+	.count = ARRAY_SIZE(imx_dev_constraints),
+};
+
+/* imx7 */
+static struct dev_boot_constraint_of imx7_dev_constraints[] = {
+	{
+		.compat = "fsl,imx6q-uart",
+		.constraints = imx_uart_constraints,
+		.count = ARRAY_SIZE(imx_uart_constraints),
+	},
+};
+
+static struct imx_machine_constraints imx7_constraints = {
+	.dev_constraints = imx7_dev_constraints,
+	.count = ARRAY_SIZE(imx7_dev_constraints),
+};
+
+static const struct of_device_id machines[] __initconst = {
+	{ .compatible = "fsl,imx25", .data = &imx_constraints },
+	{ .compatible = "fsl,imx27", .data = &imx_constraints },
+	{ .compatible = "fsl,imx31", .data = &imx_constraints },
+	{ .compatible = "fsl,imx35", .data = &imx_constraints },
+	{ .compatible = "fsl,imx50", .data = &imx_constraints },
+	{ .compatible = "fsl,imx51", .data = &imx_constraints },
+	{ .compatible = "fsl,imx53", .data = &imx_constraints },
+	{ .compatible = "fsl,imx6dl", .data = &imx_constraints },
+	{ .compatible = "fsl,imx6q", .data = &imx_constraints },
+	{ .compatible = "fsl,imx6qp", .data = &imx_constraints },
+	{ .compatible = "fsl,imx6sl", .data = &imx_constraints },
+	{ .compatible = "fsl,imx6sx", .data = &imx_constraints },
+	{ .compatible = "fsl,imx6ul", .data = &imx_constraints },
+	{ .compatible = "fsl,imx6ull", .data = &imx_constraints },
+	{ .compatible = "fsl,imx7d", .data = &imx7_constraints },
+	{ .compatible = "fsl,imx7s", .data = &imx7_constraints },
+	{ }
+};
+
+static int __init imx_constraints_init(void)
+{
+	const struct imx_machine_constraints *constraints;
+	const struct of_device_id *match;
+	struct device_node *np;
+
+	if (!boot_constraint_earlycon_enabled())
+		return 0;
+
+	np = of_find_node_by_path("/");
+	if (!np)
+		return -ENODEV;
+
+	match = of_match_node(machines, np);
+	of_node_put(np);
+
+	if (!match)
+		return 0;
+
+	constraints = match->data;
+	BUG_ON(!constraints);
+
+	dev_boot_constraint_add_deferrable_of(constraints->dev_constraints,
+					      constraints->count);
+
+	return 0;
+}
+subsys_initcall(imx_constraints_init);
diff --git a/drivers/clk/imx/clk-imx25.c b/drivers/clk/imx/clk-imx25.c
index 23686f756b5e..4df652cd912c 100644
--- a/drivers/clk/imx/clk-imx25.c
+++ b/drivers/clk/imx/clk-imx25.c
@@ -86,16 +86,6 @@ enum mx25_clks {
 
 static struct clk *clk[clk_max];
 
-static struct clk ** const uart_clks[] __initconst = {
-	&clk[uart_ipg_per],
-	&clk[uart1_ipg],
-	&clk[uart2_ipg],
-	&clk[uart3_ipg],
-	&clk[uart4_ipg],
-	&clk[uart5_ipg],
-	NULL
-};
-
 static int __init __mx25_clocks_init(void __iomem *ccm_base)
 {
 	BUG_ON(!ccm_base);
@@ -241,8 +231,6 @@ static int __init __mx25_clocks_init(void __iomem *ccm_base)
 	 */
 	clk_set_parent(clk[cko_sel], clk[ipg]);
 
-	imx_register_uart_clocks(uart_clks);
-
 	return 0;
 }
 
diff --git a/drivers/clk/imx/clk-imx27.c b/drivers/clk/imx/clk-imx27.c
index cf5cf75a4848..646d2075b510 100644
--- a/drivers/clk/imx/clk-imx27.c
+++ b/drivers/clk/imx/clk-imx27.c
@@ -47,17 +47,6 @@ static const char *ssi_sel_clks[] = { "spll_gate", "mpll", };
 static struct clk *clk[IMX27_CLK_MAX];
 static struct clk_onecell_data clk_data;
 
-static struct clk ** const uart_clks[] __initconst = {
-	&clk[IMX27_CLK_PER1_GATE],
-	&clk[IMX27_CLK_UART1_IPG_GATE],
-	&clk[IMX27_CLK_UART2_IPG_GATE],
-	&clk[IMX27_CLK_UART3_IPG_GATE],
-	&clk[IMX27_CLK_UART4_IPG_GATE],
-	&clk[IMX27_CLK_UART5_IPG_GATE],
-	&clk[IMX27_CLK_UART6_IPG_GATE],
-	NULL
-};
-
 static void __init _mx27_clocks_init(unsigned long fref)
 {
 	BUG_ON(!ccm);
@@ -174,8 +163,6 @@ static void __init _mx27_clocks_init(unsigned long fref)
 
 	clk_prepare_enable(clk[IMX27_CLK_EMI_AHB_GATE]);
 
-	imx_register_uart_clocks(uart_clks);
-
 	imx_print_silicon_rev("i.MX27", mx27_revision());
 }
 
diff --git a/drivers/clk/imx/clk-imx31.c b/drivers/clk/imx/clk-imx31.c
index cbce308aad04..d0a720b61aca 100644
--- a/drivers/clk/imx/clk-imx31.c
+++ b/drivers/clk/imx/clk-imx31.c
@@ -63,16 +63,6 @@ enum mx31_clks {
 static struct clk *clk[clk_max];
 static struct clk_onecell_data clk_data;
 
-static struct clk ** const uart_clks[] __initconst = {
-	&clk[ipg],
-	&clk[uart1_gate],
-	&clk[uart2_gate],
-	&clk[uart3_gate],
-	&clk[uart4_gate],
-	&clk[uart5_gate],
-	NULL
-};
-
 static void __init _mx31_clocks_init(void __iomem *base, unsigned long fref)
 {
 	clk[dummy] = imx_clk_fixed("dummy", 0);
@@ -208,8 +198,6 @@ int __init mx31_clocks_init(unsigned long fref)
 	clk_register_clkdev(clk[sdma_gate], NULL, "imx31-sdma");
 	clk_register_clkdev(clk[iim_gate], "iim", NULL);
 
-
-	imx_register_uart_clocks(uart_clks);
 	mxc_timer_init(MX31_GPT1_BASE_ADDR, MX31_INT_GPT, GPT_TYPE_IMX31);
 
 	return 0;
diff --git a/drivers/clk/imx/clk-imx35.c b/drivers/clk/imx/clk-imx35.c
index 203cad6c9aab..081aacd2335b 100644
--- a/drivers/clk/imx/clk-imx35.c
+++ b/drivers/clk/imx/clk-imx35.c
@@ -86,14 +86,6 @@ enum mx35_clks {
 
 static struct clk *clk[clk_max];
 
-static struct clk ** const uart_clks[] __initconst = {
-	&clk[ipg],
-	&clk[uart1_gate],
-	&clk[uart2_gate],
-	&clk[uart3_gate],
-	NULL
-};
-
 static void __init _mx35_clocks_init(void)
 {
 	void __iomem *base;
@@ -247,8 +239,6 @@ static void __init _mx35_clocks_init(void)
 	 */
 	clk_prepare_enable(clk[scc_gate]);
 
-	imx_register_uart_clocks(uart_clks);
-
 	imx_print_silicon_rev("i.MX35", mx35_revision());
 }
 
diff --git a/drivers/clk/imx/clk-imx51-imx53.c b/drivers/clk/imx/clk-imx51-imx53.c
index 7bcaf270db11..2da06f8ffae1 100644
--- a/drivers/clk/imx/clk-imx51-imx53.c
+++ b/drivers/clk/imx/clk-imx51-imx53.c
@@ -131,20 +131,6 @@ static const char *ieee1588_sels[] = { "pll3_sw", "pll4_sw", "dummy" /* usbphy2_
 static struct clk *clk[IMX5_CLK_END];
 static struct clk_onecell_data clk_data;
 
-static struct clk ** const uart_clks[] __initconst = {
-	&clk[IMX5_CLK_UART1_IPG_GATE],
-	&clk[IMX5_CLK_UART1_PER_GATE],
-	&clk[IMX5_CLK_UART2_IPG_GATE],
-	&clk[IMX5_CLK_UART2_PER_GATE],
-	&clk[IMX5_CLK_UART3_IPG_GATE],
-	&clk[IMX5_CLK_UART3_PER_GATE],
-	&clk[IMX5_CLK_UART4_IPG_GATE],
-	&clk[IMX5_CLK_UART4_PER_GATE],
-	&clk[IMX5_CLK_UART5_IPG_GATE],
-	&clk[IMX5_CLK_UART5_PER_GATE],
-	NULL
-};
-
 static void __init mx5_clocks_common_init(void __iomem *ccm_base)
 {
 	clk[IMX5_CLK_DUMMY]		= imx_clk_fixed("dummy", 0);
@@ -325,8 +311,6 @@ static void __init mx5_clocks_common_init(void __iomem *ccm_base)
 	clk_prepare_enable(clk[IMX5_CLK_TMAX1]);
 	clk_prepare_enable(clk[IMX5_CLK_TMAX2]); /* esdhc2, fec */
 	clk_prepare_enable(clk[IMX5_CLK_TMAX3]); /* esdhc1, esdhc4 */
-
-	imx_register_uart_clocks(uart_clks);
 }
 
 static void __init mx50_clocks_init(struct device_node *np)
diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
index c07df719b8a3..5af256a076a1 100644
--- a/drivers/clk/imx/clk-imx6q.c
+++ b/drivers/clk/imx/clk-imx6q.c
@@ -150,12 +150,6 @@ static inline int clk_on_imx6dl(void)
 	return of_machine_is_compatible("fsl,imx6dl");
 }
 
-static struct clk ** const uart_clks[] __initconst = {
-	&clk[IMX6QDL_CLK_UART_IPG],
-	&clk[IMX6QDL_CLK_UART_SERIAL],
-	NULL
-};
-
 static int ldb_di_sel_by_clock_id(int clock_id)
 {
 	switch (clock_id) {
@@ -918,7 +912,5 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 		clk_set_parent(clk[IMX6QDL_CLK_GPU2D_CORE_SEL],
 			       clk[IMX6QDL_CLK_PLL3_USB_OTG]);
 	}
-
-	imx_register_uart_clocks(uart_clks);
 }
 CLK_OF_DECLARE(imx6q, "fsl,imx6q-ccm", imx6q_clocks_init);
diff --git a/drivers/clk/imx/clk-imx6sl.c b/drivers/clk/imx/clk-imx6sl.c
index 9642cdf0fb88..97ab67783609 100644
--- a/drivers/clk/imx/clk-imx6sl.c
+++ b/drivers/clk/imx/clk-imx6sl.c
@@ -185,12 +185,6 @@ void imx6sl_set_wait_clk(bool enter)
 		imx6sl_enable_pll_arm(false);
 }
 
-static struct clk ** const uart_clks[] __initconst = {
-	&clks[IMX6SL_CLK_UART],
-	&clks[IMX6SL_CLK_UART_SERIAL],
-	NULL
-};
-
 static void __init imx6sl_clocks_init(struct device_node *ccm_node)
 {
 	struct device_node *np;
@@ -447,7 +441,5 @@ static void __init imx6sl_clocks_init(struct device_node *ccm_node)
 
 	clk_set_parent(clks[IMX6SL_CLK_LCDIF_AXI_SEL],
 		       clks[IMX6SL_CLK_PLL2_PFD2]);
-
-	imx_register_uart_clocks(uart_clks);
 }
 CLK_OF_DECLARE(imx6sl, "fsl,imx6sl-ccm", imx6sl_clocks_init);
diff --git a/drivers/clk/imx/clk-imx6sx.c b/drivers/clk/imx/clk-imx6sx.c
index e6d389e333d7..e38dfb855ae8 100644
--- a/drivers/clk/imx/clk-imx6sx.c
+++ b/drivers/clk/imx/clk-imx6sx.c
@@ -137,12 +137,6 @@ static u32 share_count_ssi3;
 static u32 share_count_sai1;
 static u32 share_count_sai2;
 
-static struct clk ** const uart_clks[] __initconst = {
-	&clks[IMX6SX_CLK_UART_IPG],
-	&clks[IMX6SX_CLK_UART_SERIAL],
-	NULL
-};
-
 static void __init imx6sx_clocks_init(struct device_node *ccm_node)
 {
 	struct device_node *np;
@@ -566,7 +560,5 @@ static void __init imx6sx_clocks_init(struct device_node *ccm_node)
 
 	clk_set_parent(clks[IMX6SX_CLK_QSPI1_SEL], clks[IMX6SX_CLK_PLL2_BUS]);
 	clk_set_parent(clks[IMX6SX_CLK_QSPI2_SEL], clks[IMX6SX_CLK_PLL2_BUS]);
-
-	imx_register_uart_clocks(uart_clks);
 }
 CLK_OF_DECLARE(imx6sx, "fsl,imx6sx-ccm", imx6sx_clocks_init);
diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
index 2305699db467..ec919758b2ae 100644
--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c
@@ -392,17 +392,6 @@ static int const clks_init_on[] __initconst = {
 
 static struct clk_onecell_data clk_data;
 
-static struct clk ** const uart_clks[] __initconst = {
-	&clks[IMX7D_UART1_ROOT_CLK],
-	&clks[IMX7D_UART2_ROOT_CLK],
-	&clks[IMX7D_UART3_ROOT_CLK],
-	&clks[IMX7D_UART4_ROOT_CLK],
-	&clks[IMX7D_UART5_ROOT_CLK],
-	&clks[IMX7D_UART6_ROOT_CLK],
-	&clks[IMX7D_UART7_ROOT_CLK],
-	NULL
-};
-
 static void __init imx7d_clocks_init(struct device_node *ccm_node)
 {
 	struct device_node *np;
@@ -893,8 +882,5 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
 
 	/* set uart module clock's parent clock source that must be great then 80MHz */
 	clk_set_parent(clks[IMX7D_UART1_ROOT_SRC], clks[IMX7D_OSC_24M_CLK]);
-
-	imx_register_uart_clocks(uart_clks);
-
 }
 CLK_OF_DECLARE(imx7d, "fsl,imx7d-ccm", imx7d_clocks_init);
diff --git a/drivers/clk/imx/clk.c b/drivers/clk/imx/clk.c
index a634b1185be3..df12b5307175 100644
--- a/drivers/clk/imx/clk.c
+++ b/drivers/clk/imx/clk.c
@@ -73,41 +73,3 @@ void imx_cscmr1_fixup(u32 *val)
 	*val ^= CSCMR1_FIXUP;
 	return;
 }
-
-static int imx_keep_uart_clocks __initdata;
-static struct clk ** const *imx_uart_clocks __initdata;
-
-static int __init imx_keep_uart_clocks_param(char *str)
-{
-	imx_keep_uart_clocks = 1;
-
-	return 0;
-}
-__setup_param("earlycon", imx_keep_uart_earlycon,
-	      imx_keep_uart_clocks_param, 0);
-__setup_param("earlyprintk", imx_keep_uart_earlyprintk,
-	      imx_keep_uart_clocks_param, 0);
-
-void __init imx_register_uart_clocks(struct clk ** const clks[])
-{
-	if (imx_keep_uart_clocks) {
-		int i;
-
-		imx_uart_clocks = clks;
-		for (i = 0; imx_uart_clocks[i]; i++)
-			clk_prepare_enable(*imx_uart_clocks[i]);
-	}
-}
-
-static int __init imx_clk_disable_uart(void)
-{
-	if (imx_keep_uart_clocks && imx_uart_clocks) {
-		int i;
-
-		for (i = 0; imx_uart_clocks[i]; i++)
-			clk_disable_unprepare(*imx_uart_clocks[i]);
-	}
-
-	return 0;
-}
-late_initcall_sync(imx_clk_disable_uart);
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index d54f0720afba..6fda6e1d181f 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -7,7 +7,6 @@
 extern spinlock_t imx_ccm_lock;
 
 void imx_check_clocks(struct clk *clks[], unsigned int count);
-void imx_register_uart_clocks(struct clk ** const clks[]);
 
 extern void imx_cscmr1_fixup(u32 *val);
 
-- 
2.15.0.rc1.236.g92ea95045093

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH V4 12/12] boot_constraint: Add Qualcomm display controller constraints
  2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
                   ` (10 preceding siblings ...)
  2017-10-29 13:48 ` [PATCH V4 11/12] boot_constraint: Add support for IMX platform Viresh Kumar
@ 2017-10-29 13:49 ` Viresh Kumar
  2017-10-30 22:07 ` [PATCH V4 00/12] drivers: Boot Constraints core Rob Herring
  2017-11-28  4:18 ` Viresh Kumar
  13 siblings, 0 replies; 40+ messages in thread
From: Viresh Kumar @ 2017-10-29 13:49 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Viresh Kumar, Vincent Guittot, Stephen Boyd, Rajendra Nayak,
	linux-kernel, linux-arm-kernel, robdclark, s.hauer, l.stach,
	shawnguo, fabio.estevam, nm, xuwei5, robh+dt

From: Rajendra Nayak <rnayak@codeaurora.org>

This sets boot constraints for the display controller used on Qualcomm
dragonboard 410c.

The display controlled is enabled by the bootloader to show a flash
screen during kernel boot. The handover to kernel should be without any
glitches on the screen.The resources of the display controller (like
regulators) are shared with other peripherals, which may reconfigure
those resources before the display driver comes up. The same problem can
happen if the display driver probes first, as the constraints of the
other devices (sharing same resources with display controller) may not
be honored anymore by the kernel.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm64/Kconfig.platforms      |   1 +
 drivers/boot_constraints/Makefile |   1 +
 drivers/boot_constraints/qcom.c   | 123 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 125 insertions(+)
 create mode 100644 drivers/boot_constraints/qcom.c

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 265df4a088ab..6343627cf105 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -134,6 +134,7 @@ config ARCH_QCOM
 	bool "Qualcomm Platforms"
 	select GPIOLIB
 	select PINCTRL
+	select DEV_BOOT_CONSTRAINTS
 	help
 	  This enables support for the ARMv8 based Qualcomm chipsets.
 
diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
index 3b5a87fcf099..e32751b70957 100644
--- a/drivers/boot_constraints/Makefile
+++ b/drivers/boot_constraints/Makefile
@@ -4,3 +4,4 @@ obj-y := clk.o deferrable_dev.o core.o pm.o serial.o supply.o
 
 obj-$(CONFIG_ARCH_HISI)			+= hikey.o
 obj-$(CONFIG_ARCH_MXC)			+= imx.o
+obj-$(CONFIG_ARCH_QCOM)			+= qcom.o
diff --git a/drivers/boot_constraints/qcom.c b/drivers/boot_constraints/qcom.c
new file mode 100644
index 000000000000..e89357670906
--- /dev/null
+++ b/drivers/boot_constraints/qcom.c
@@ -0,0 +1,123 @@
+/*
+ * This sets up Dragonboard 410c constraints on behalf of the bootloader, which
+ * uses display controller to display a flash screen during system boot.
+ *
+ * Copyright (C) 2017 Linaro.
+ * Viresh Kumar <viresh.kumar@linaro.org>
+ * Rajendra Nayak <rnayak@codeaurora.org>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/boot_constraint.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+
+static struct dev_boot_constraint_clk_info iface_clk_info = {
+	.name = "iface_clk",
+};
+
+static struct dev_boot_constraint_clk_info bus_clk_info = {
+	.name = "bus_clk",
+};
+
+static struct dev_boot_constraint_clk_info core_clk_info = {
+	.name = "core_clk",
+};
+
+static struct dev_boot_constraint_clk_info vsync_clk_info = {
+	.name = "vsync_clk",
+};
+
+static struct dev_boot_constraint_clk_info esc0_clk_info = {
+	.name = "core_clk",
+};
+
+static struct dev_boot_constraint_clk_info byte_clk_info = {
+	.name = "byte_clk",
+};
+
+static struct dev_boot_constraint_clk_info pixel_clk_info = {
+	.name = "pixel_clk",
+};
+
+static struct dev_boot_constraint_supply_info vdda_info = {
+	.name = "vdda"
+};
+
+static struct dev_boot_constraint_supply_info vddio_info = {
+	.name = "vddio"
+};
+
+static struct dev_boot_constraint constraints_mdss[] = {
+	{
+		.type = DEV_BOOT_CONSTRAINT_PM,
+		.data = NULL,
+	},
+};
+
+static struct dev_boot_constraint constraints_mdp[] = {
+	{
+		.type = DEV_BOOT_CONSTRAINT_CLK,
+		.data = &iface_clk_info,
+	}, {
+		.type = DEV_BOOT_CONSTRAINT_CLK,
+		.data = &bus_clk_info,
+	}, {
+		.type = DEV_BOOT_CONSTRAINT_CLK,
+		.data = &core_clk_info,
+	}, {
+		.type = DEV_BOOT_CONSTRAINT_CLK,
+		.data = &vsync_clk_info,
+	},
+};
+
+static struct dev_boot_constraint constraints_dsi[] = {
+	{
+		.type = DEV_BOOT_CONSTRAINT_CLK,
+		.data = &esc0_clk_info,
+	}, {
+		.type = DEV_BOOT_CONSTRAINT_CLK,
+		.data = &byte_clk_info,
+	}, {
+		.type = DEV_BOOT_CONSTRAINT_CLK,
+		.data = &pixel_clk_info,
+	}, {
+		.type = DEV_BOOT_CONSTRAINT_SUPPLY,
+		.data = &vdda_info,
+
+	}, {
+		.type = DEV_BOOT_CONSTRAINT_SUPPLY,
+		.data = &vddio_info,
+	},
+};
+
+static struct dev_boot_constraint_of constraints[] = {
+	{
+		.compat = "qcom,mdss",
+		.constraints = constraints_mdss,
+		.count = ARRAY_SIZE(constraints_mdss),
+	}, {
+		.compat = "qcom,mdp5",
+		.constraints = constraints_mdp,
+		.count = ARRAY_SIZE(constraints_mdp),
+	}, {
+		.compat = "qcom,mdss-dsi-ctrl",
+		.constraints = constraints_dsi,
+		.count = ARRAY_SIZE(constraints_dsi),
+	},
+};
+
+static int __init qcom_constraints_init(void)
+{
+	/* Only Dragonboard 410c is supported for now */
+	if (!of_machine_is_compatible("qcom,apq8016-sbc"))
+		return 0;
+
+	dev_boot_constraint_add_deferrable_of(constraints,
+					      ARRAY_SIZE(constraints));
+
+	return 0;
+}
+subsys_initcall(qcom_constraints_init);
-- 
2.15.0.rc1.236.g92ea95045093

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 07/12] boot_constraint: Add debugfs support
  2017-10-29 13:48 ` [PATCH V4 07/12] boot_constraint: Add debugfs support Viresh Kumar
@ 2017-10-29 15:09   ` Randy Dunlap
  2017-10-30  3:37     ` Viresh Kumar
  2017-12-13  9:50   ` Greg Kroah-Hartman
  1 sibling, 1 reply; 40+ messages in thread
From: Randy Dunlap @ 2017-10-29 15:09 UTC (permalink / raw)
  To: Viresh Kumar, Greg Kroah-Hartman
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt

On 10/29/17 06:48, Viresh Kumar wrote:
> This patch adds debugfs support for boot constraints. This is how it
> looks for a "vmmc-supply" constraint for the MMC device.
> 
> Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/boot_constraints/clk.c    |  3 ++
>  drivers/boot_constraints/core.c   | 60 +++++++++++++++++++++++++++++++++++++++
>  drivers/boot_constraints/core.h   |  6 ++++
>  drivers/boot_constraints/pm.c     | 11 +++++--
>  drivers/boot_constraints/supply.c |  9 ++++++
>  5 files changed, 87 insertions(+), 2 deletions(-)

Hi,
Does this build OK when CONFIG_DEBUG_FS is not enabled?

I didn't see any depends on or select DEBUG_FS or any use of
CONFIG_DEBUG_FS in any Makefile.


-- 
~Randy

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 07/12] boot_constraint: Add debugfs support
  2017-10-29 15:09   ` Randy Dunlap
@ 2017-10-30  3:37     ` Viresh Kumar
  2017-10-30  3:43       ` Randy Dunlap
  0 siblings, 1 reply; 40+ messages in thread
From: Viresh Kumar @ 2017-10-30  3:37 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Greg Kroah-Hartman, Vincent Guittot, Stephen Boyd,
	Rajendra Nayak, linux-kernel, linux-arm-kernel, robdclark,
	s.hauer, l.stach, shawnguo, fabio.estevam, nm, xuwei5, robh+dt

On 29-10-17, 08:09, Randy Dunlap wrote:
> On 10/29/17 06:48, Viresh Kumar wrote:
> > This patch adds debugfs support for boot constraints. This is how it
> > looks for a "vmmc-supply" constraint for the MMC device.
> > 
> > Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  drivers/boot_constraints/clk.c    |  3 ++
> >  drivers/boot_constraints/core.c   | 60 +++++++++++++++++++++++++++++++++++++++
> >  drivers/boot_constraints/core.h   |  6 ++++
> >  drivers/boot_constraints/pm.c     | 11 +++++--
> >  drivers/boot_constraints/supply.c |  9 ++++++
> >  5 files changed, 87 insertions(+), 2 deletions(-)
> 
> Hi,
> Does this build OK when CONFIG_DEBUG_FS is not enabled?
> 
> I didn't see any depends on or select DEBUG_FS or any use of
> CONFIG_DEBUG_FS in any Makefile.

As soon as I saw your reply, it looked like I have seen this email
earlier. :)

https://marc.info/?l=linux-kernel&m=149866480929111&w=2

And yes, it builds just fine as all the dummy helpers are in place.

-- 
viresh

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 07/12] boot_constraint: Add debugfs support
  2017-10-30  3:37     ` Viresh Kumar
@ 2017-10-30  3:43       ` Randy Dunlap
  0 siblings, 0 replies; 40+ messages in thread
From: Randy Dunlap @ 2017-10-30  3:43 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Greg Kroah-Hartman, Vincent Guittot, Stephen Boyd,
	Rajendra Nayak, linux-kernel, linux-arm-kernel, robdclark,
	s.hauer, l.stach, shawnguo, fabio.estevam, nm, xuwei5, robh+dt

On 10/29/17 20:37, Viresh Kumar wrote:
> On 29-10-17, 08:09, Randy Dunlap wrote:
>> On 10/29/17 06:48, Viresh Kumar wrote:
>>> This patch adds debugfs support for boot constraints. This is how it
>>> looks for a "vmmc-supply" constraint for the MMC device.
>>>
>>> Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
>>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>>> ---
>>>  drivers/boot_constraints/clk.c    |  3 ++
>>>  drivers/boot_constraints/core.c   | 60 +++++++++++++++++++++++++++++++++++++++
>>>  drivers/boot_constraints/core.h   |  6 ++++
>>>  drivers/boot_constraints/pm.c     | 11 +++++--
>>>  drivers/boot_constraints/supply.c |  9 ++++++
>>>  5 files changed, 87 insertions(+), 2 deletions(-)
>>
>> Hi,
>> Does this build OK when CONFIG_DEBUG_FS is not enabled?
>>
>> I didn't see any depends on or select DEBUG_FS or any use of
>> CONFIG_DEBUG_FS in any Makefile.
> 
> As soon as I saw your reply, it looked like I have seen this email
> earlier. :)
> 
> https://marc.info/?l=linux-kernel&m=149866480929111&w=2
> 
> And yes, it builds just fine as all the dummy helpers are in place.
> 

Thanks.

-- 
~Randy

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 00/12] drivers: Boot Constraints core
  2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
                   ` (11 preceding siblings ...)
  2017-10-29 13:49 ` [PATCH V4 12/12] boot_constraint: Add Qualcomm display controller constraints Viresh Kumar
@ 2017-10-30 22:07 ` Rob Herring
  2017-10-31 10:01   ` Viresh Kumar
  2017-11-28  4:18 ` Viresh Kumar
  13 siblings, 1 reply; 40+ messages in thread
From: Rob Herring @ 2017-10-30 22:07 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Greg Kroah-Hartman, Vincent Guittot, Stephen Boyd,
	Rajendra Nayak, linux-kernel, linux-arm-kernel, Rob Clark,
	Sascha Hauer, Lucas Stach, Shawn Guo, Fabio Estevam,
	Nishanth Menon, Wei Xu, devicetree, Frank Rowand

On Sun, Oct 29, 2017 at 8:48 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Hi Greg,
>
> Here is V4 of the boot constraints core based on your feedback from V3.
> We now have support for three platforms (as you suggested) included in
> this series: Hisilicon, IMX and Qualcomm.
>
> I have tested the Hisilicon patches on hikey 9660 board, IMX stuff is
> tested by Sascha (Pengutronix) on i.MX6 and Qualcomm stuff is tested by
> Rajendra (Qualcomm) on Dragonboard 410C (This required some more patches
> related to display driver which Rajendra should be sending separately
> later on).
>
>
> Problem statement:
>
> Some devices are powered ON by the bootloader before the bootloader
> handovers control to Linux. It maybe important for those devices to keep
> working until the time a Linux device driver probes the device and
> reconfigure its resources.
>
> A typical example of that can be the LCD controller, which is used by
> the bootloaders to show image(s) while the platform is booting into
> Linux.  The LCD controller can be using some resources, like clk,
> regulators, etc, that are shared between several devices. These shared
> resources should be configured to satisfy need of all the users.  If
> another device's (X) driver gets probed before the LCD controller driver
> in this case, then it may end up disabling or reconfiguring these
> resources to ranges satisfying the current users (only device X) and
> that can make the LCD screen unstable.

Would probing the display earlier solve this as that's also something
people care about (or work-around by initializing the display in the
bootloader). I guess if the display depends on resources A, B, and C,
other drivers could still each only depend on one of those and probe
successfully before the display does. However, the display driver
would still probe, so you could do something then to enable the
constraints. That would have the advantage that the driver knows what
resources it needs even if probe is deferred and you don't need
special DT bindings nor platform code.

Getting certain drivers/devices to probe first may just require
ensuring they get inserted at the beginning of the driver and device
lists which would be a pretty trivial change I think. That's ignoring
differing initcall levels though.

> Another case can be a debug serial port enabled from the bootloader.

Continuing the above thought, we could just match device nodes against
stdout-path as we create devices and then set a priority flag or
something that the driver core would handle.

Yes, it's triggered by a specific use case, but I'm not that convinced
that having things setup by the bootloader and needing a transparent
hand off is a general case. You've given 2 examples. I can't think of
many more. One is CAN bus which has realtime servicing requirements,
but I don't think this series solves that (OOT solutions I've seen
insert callbacks at random places during kernel init). The other case
is just things that have no driver at all, but resources need to
remain enabled. That should just be some platform code to enable those
things IMO.

> Of course we can have more complex cases where the same resource is
> getting used by two devices while the kernel boots and the order in
> which devices get probed wouldn't matter as the other device will surely
> break then.
>
> There are also cases where the resources may not be shared, but the
> kernel will disable them forcefully as no users may have appeared until
> a certain point in kernel boot. This makes sure that the resources stay
> enabled. A wide variety of constraints can be satisfied using the new
> framework.

This is only if the driver (or dependent driver) is a module though.
Is that something we really need to handle?

>
> Proposed solution:
>
> This series introduces the concept of "boot-constraints", which are set
> by platform specific drivers (for now at least) at early init (like
> subsys_initcall) and the kernel will keep satisfying them until the time
> driver for such a device is probed (successfully or unsuccessfully).
> Once the driver is probed, the driver core removes the constraints set
> for the device. This series implements clk, regulator and PM domain
> constraints currently.
>
> Rebased over: driver-core/master
> Targeted for: v4.16 (Sending it earlier for reviews mostly)
>
> Pushed here:
> git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/linux.git device/boot-constraints
>
> V3->V4:
> - Added support for imx, hikey and Qcom usecases.
> - Enhanced boot constraints core to make drivers code easy and handle
>   complex cases.
> - Two new patches for OF included to provide APIs to boot constraint
>   core.
> - Removed the kernel parameter patch for now.
> - Don't check return values of debugfs routines.
> - Moved the boot constraints core from drivers/base/ to drivers/.
>
> V2->V3:
> - Removed DT support as we aren't sure about how to define the bindings
>   yet.
> - Added CLK and PM domain constraint types.
> - A new directory is added for boot constraints, which will also contain
>   platform specific drivers in future.
> - Deferred devices are still supported, just that it wouldn't be called
>   from generic code anymore but platform specific code.
> - Tested on Qcom 410c dragonboard with display flash screen (Rajendra).
> - Usual renaming/commit-log-updates/etc changes done.
>
> V1->V2:
> - Add support for setting constraints for devices created from DT.
> - Allow handling deferred devices earlier then late_init.
> - Remove 'default y' line from kconfig.
> - Drop '=" after boot_constraints_disable kernel param.
> - Dropped the dummy testing patch now.
>
> --
> viresh
>
> Rajendra Nayak (1):
>   boot_constraint: Add Qualcomm display controller constraints
>
> Viresh Kumar (11):
>   of: platform: Add of_find_amba_device_by_node()
>   of: platform: Make of_platform_bus_create() global
>   drivers: Add boot constraints core
>   boot_constraint: Add support for supply constraints
>   boot_constraint: Add support for clk constraints
>   boot_constraint: Add support for PM constraints
>   boot_constraint: Add debugfs support
>   boot_constraint: Manage deferrable constraints
>   boot_constraint: Add earlycon helper
>   boot_constraint: Add support for Hisilicon platforms
>   boot_constraint: Add support for IMX platform
>
>  arch/arm/mach-imx/Kconfig                 |   1 +
>  arch/arm64/Kconfig.platforms              |   2 +
>  drivers/Kconfig                           |   2 +
>  drivers/Makefile                          |   1 +
>  drivers/base/dd.c                         |  32 +++-
>  drivers/boot_constraints/Kconfig          |   9 +
>  drivers/boot_constraints/Makefile         |   7 +

Maintainer for this?

>  drivers/boot_constraints/clk.c            |  73 ++++++++
>  drivers/boot_constraints/core.c           | 271 ++++++++++++++++++++++++++++++
>  drivers/boot_constraints/core.h           |  48 ++++++
>  drivers/boot_constraints/deferrable_dev.c | 235 ++++++++++++++++++++++++++
>  drivers/boot_constraints/hikey.c          | 145 ++++++++++++++++
>  drivers/boot_constraints/imx.c            | 113 +++++++++++++
>  drivers/boot_constraints/pm.c             |  31 ++++
>  drivers/boot_constraints/qcom.c           | 123 ++++++++++++++
>  drivers/boot_constraints/serial.c         |  28 +++

earlycon really. earlycon doesn't have to be a serial device.

>  drivers/boot_constraints/supply.c         | 107 ++++++++++++

Kind of a mixture of boards and subsystem handlers. Perhaps subsystem
handlers should go into the subsystems. Then you can get a lot more
opinions on this. :)


>  drivers/clk/imx/clk-imx25.c               |  12 --
>  drivers/clk/imx/clk-imx27.c               |  13 --
>  drivers/clk/imx/clk-imx31.c               |  12 --
>  drivers/clk/imx/clk-imx35.c               |  10 --
>  drivers/clk/imx/clk-imx51-imx53.c         |  16 --
>  drivers/clk/imx/clk-imx6q.c               |   8 -
>  drivers/clk/imx/clk-imx6sl.c              |   8 -
>  drivers/clk/imx/clk-imx6sx.c              |   8 -
>  drivers/clk/imx/clk-imx7d.c               |  14 --
>  drivers/clk/imx/clk.c                     |  38 -----
>  drivers/clk/imx/clk.h                     |   1 -
>  drivers/of/platform.c                     |  28 ++-
>  include/linux/boot_constraint.h           |  74 ++++++++
>  include/linux/of_platform.h               |  21 +++
>  31 files changed, 1340 insertions(+), 151 deletions(-)
>  create mode 100644 drivers/boot_constraints/Kconfig
>  create mode 100644 drivers/boot_constraints/Makefile
>  create mode 100644 drivers/boot_constraints/clk.c
>  create mode 100644 drivers/boot_constraints/core.c
>  create mode 100644 drivers/boot_constraints/core.h
>  create mode 100644 drivers/boot_constraints/deferrable_dev.c
>  create mode 100644 drivers/boot_constraints/hikey.c
>  create mode 100644 drivers/boot_constraints/imx.c
>  create mode 100644 drivers/boot_constraints/pm.c
>  create mode 100644 drivers/boot_constraints/qcom.c
>  create mode 100644 drivers/boot_constraints/serial.c
>  create mode 100644 drivers/boot_constraints/supply.c
>  create mode 100644 include/linux/boot_constraint.h
>
> --
> 2.15.0.rc1.236.g92ea95045093
>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 00/12] drivers: Boot Constraints core
  2017-10-30 22:07 ` [PATCH V4 00/12] drivers: Boot Constraints core Rob Herring
@ 2017-10-31 10:01   ` Viresh Kumar
  0 siblings, 0 replies; 40+ messages in thread
From: Viresh Kumar @ 2017-10-31 10:01 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, Vincent Guittot, Stephen Boyd,
	Rajendra Nayak, linux-kernel, linux-arm-kernel, Rob Clark,
	Sascha Hauer, Lucas Stach, Shawn Guo, Fabio Estevam,
	Nishanth Menon, Wei Xu, devicetree, Frank Rowand

Hi Rob,

On 30-10-17, 17:07, Rob Herring wrote:
> Would probing the display earlier solve this as that's also something
> people care about (or work-around by initializing the display in the
> bootloader).

Not always.

And probing the display earlier may result in breaking some other
devices. For example, the display driver may want to enable and then
disable its clock at some point of time in probe(). The enable will
get propagated to parent clocks of display clock as well and all will
have a refcount of 1. Now if display controller disables its clock,
the whole hierarchy of clocks may go down as no one else have taken a
reference of those clocks and other peripherals, used during booting
the kernel, that depend on the clk hierarchy may not work properly
after that until the time their driver comes up.

One more typical problem I saw during my testing on hikey was that
many debug messages were missing during kernel boot (though everything
was available with dmesg later on), that I never noticed earlier on.

After digging a bit I found that it is because of the way AMBA bus
works. It enables and disables the interface clock before calling
probe() (to read some registers) and the interface and functional
clocks were same for hikey UART. And that disabled the clock before
calling probe() which was enabled by the bootloader. And the message
started coming again only after sometime when tty device all setup
properly.

Over that the display driver can be compiled as a module (which we
will disallow with your suggestion). Plus all the resources for the
display driver may not be available early on and so the driver will be
probed later again. And by that time some of the resources may get
disabled/reconfigured.

> I guess if the display depends on resources A, B, and C,
> other drivers could still each only depend on one of those and probe
> successfully before the display does.

That would be tricky as well. Same example about refcounting of
clocks. What if the parent clock of display controller's clock gets
disabled, because another child of that parent clock has done a clk
enable/disable? And with amba probing, there are more chances of that
naturally.

My basic idea is that some code should come in and set these
constraints before any of the users of these resources come up (i.e.
display, serial, mmc drivers, etc). That is the only sane way with
which we would be able to have some guarantees here.

> However, the display driver
> would still probe,

As I said, it may be too late.

> so you could do something then to enable the
> constraints.
> That would have the advantage that the driver knows what
> resources it needs even if probe is deferred and you don't need
> special DT bindings nor platform code.

I am not sure if I fully understood some of your comments. Lets see if
I have :)

Are you saying that we set the same boot constraints (i.e. by calling
the same API as proposed in this patchset) from these generic drivers?
I agree that the driver would know all the details and would be in a
good position to judge all the resources it needs, but again, this may
be too late. And what if we have two drivers which want to come up
first and set these constraints? Like MMC and LCD sharing same
regulator or clk line ?

> Getting certain drivers/devices to probe first may just require
> ensuring they get inserted at the beginning of the driver and device
> lists which would be a pretty trivial change I think. That's ignoring
> differing initcall levels though.

Sure and that's what we have been doing so far. Hacking different
parts of kernel to force some kind of ordering :)

> > Another case can be a debug serial port enabled from the bootloader.
> 
> Continuing the above thought, we could just match device nodes against
> stdout-path as we create devices and then set a priority flag or
> something that the driver core would handle.
> 
> Yes, it's triggered by a specific use case, but I'm not that convinced
> that having things setup by the bootloader and needing a transparent
> hand off is a general case. You've given 2 examples. I can't think of
> many more. One is CAN bus which has realtime servicing requirements,
> but I don't think this series solves that (OOT solutions I've seen
> insert callbacks at random places during kernel init).

I am not sure what the problem is and I can't say if that can benefit
from this work. Someone also mentioned (privately) that handing off a
preboot DSP to kernel can also be a usecase for this framework. I am
not sure of the details though :)

> The other case
> is just things that have no driver at all, but resources need to
> remain enabled. That should just be some platform code to enable those
> things IMO.

Yeah, I am not really targeting those right now. They can have
always-ON kind of hints in DT and that should be all. I am
specifically targeting stuff where a driver takes over eventually.

> > Of course we can have more complex cases where the same resource is
> > getting used by two devices while the kernel boots and the order in
> > which devices get probed wouldn't matter as the other device will surely
> > break then.
> >
> > There are also cases where the resources may not be shared, but the
> > kernel will disable them forcefully as no users may have appeared until
> > a certain point in kernel boot. This makes sure that the resources stay
> > enabled. A wide variety of constraints can be satisfied using the new
> > framework.
> 
> This is only if the driver (or dependent driver) is a module though.
> Is that something we really need to handle?

Maybe, I am not sure.

Why shouldn't we allow the LCD driver to be present as a module? I am
just asking :)

> > Viresh Kumar (11):
> >   of: platform: Add of_find_amba_device_by_node()
> >   of: platform: Make of_platform_bus_create() global
> >   drivers: Add boot constraints core
> >   boot_constraint: Add support for supply constraints
> >   boot_constraint: Add support for clk constraints
> >   boot_constraint: Add support for PM constraints
> >   boot_constraint: Add debugfs support
> >   boot_constraint: Manage deferrable constraints
> >   boot_constraint: Add earlycon helper
> >   boot_constraint: Add support for Hisilicon platforms
> >   boot_constraint: Add support for IMX platform
> >
> >  arch/arm/mach-imx/Kconfig                 |   1 +
> >  arch/arm64/Kconfig.platforms              |   2 +
> >  drivers/Kconfig                           |   2 +
> >  drivers/Makefile                          |   1 +
> >  drivers/base/dd.c                         |  32 +++-
> >  drivers/boot_constraints/Kconfig          |   9 +
> >  drivers/boot_constraints/Makefile         |   7 +
> 
> Maintainer for this?

Sure, that's me. I am just holding off from touching MAINTAINERS file
until this stuff is accepted (at least informally).

> >  drivers/boot_constraints/clk.c            |  73 ++++++++
> >  drivers/boot_constraints/core.c           | 271 ++++++++++++++++++++++++++++++
> >  drivers/boot_constraints/core.h           |  48 ++++++
> >  drivers/boot_constraints/deferrable_dev.c | 235 ++++++++++++++++++++++++++
> >  drivers/boot_constraints/hikey.c          | 145 ++++++++++++++++
> >  drivers/boot_constraints/imx.c            | 113 +++++++++++++
> >  drivers/boot_constraints/pm.c             |  31 ++++
> >  drivers/boot_constraints/qcom.c           | 123 ++++++++++++++
> >  drivers/boot_constraints/serial.c         |  28 +++
> 
> earlycon really. earlycon doesn't have to be a serial device.

Ah, ok.

> >  drivers/boot_constraints/supply.c         | 107 ++++++++++++
> 
> Kind of a mixture of boards and subsystem handlers. Perhaps subsystem
> handlers should go into the subsystems. Then you can get a lot more
> opinions on this. :)

You don't want this to get merged, do you ? :)

Thanks for your feedback Rob. Really appreciate it.

-- 
viresh

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 08/12] boot_constraint: Manage deferrable constraints
  2017-10-29 13:48 ` [PATCH V4 08/12] boot_constraint: Manage deferrable constraints Viresh Kumar
@ 2017-10-31 16:20   ` Rob Herring
  2017-11-01  2:29     ` Viresh Kumar
  2017-12-13  9:53   ` Greg Kroah-Hartman
  1 sibling, 1 reply; 40+ messages in thread
From: Rob Herring @ 2017-10-31 16:20 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Greg Kroah-Hartman, Vincent Guittot, Stephen Boyd,
	Rajendra Nayak, linux-kernel, linux-arm-kernel, Rob Clark,
	Sascha Hauer, Lucas Stach, Shawn Guo, Fabio Estevam,
	Nishanth Menon, Wei Xu

On Sun, Oct 29, 2017 at 8:48 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> It is possible that some of the resources aren't available at the time
> constraints are getting set and the boot constraints core will return
> -EPROBE_DEFER for them. In order to retry adding the constraints at a
> later point of time (after the resource is added and before any of its
> users come up), this patch proposes two things:
>
> - Each constraint is represented by a virtual platform device, so that
>   it is re-probed again until the time all the dependencies aren't met.
>   The platform device is removed along with the constraint, with help of
>   the free_resources() callback.
>
> - Enable early defer probing support by calling
>   driver_enable_deferred_probe(), so that the core retries probing
>   deferred devices every time any device is bound to a driver. This
>   makes sure that the constraint is set before any of the users of the
>   resources come up.

What is the effect on boot time? It's highly platform dependent, but
the worst case could be pretty bad I think.

I don't see how this handles the case you mentioned where the amba
pclk gets disabled. It only works if the constraint device is added
before any others, but that is done with initcall level games.

Rob

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 08/12] boot_constraint: Manage deferrable constraints
  2017-10-31 16:20   ` Rob Herring
@ 2017-11-01  2:29     ` Viresh Kumar
  0 siblings, 0 replies; 40+ messages in thread
From: Viresh Kumar @ 2017-11-01  2:29 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, Vincent Guittot, Stephen Boyd,
	Rajendra Nayak, linux-kernel, linux-arm-kernel, Rob Clark,
	Sascha Hauer, Lucas Stach, Shawn Guo, Fabio Estevam,
	Nishanth Menon, Wei Xu

On 31 October 2017 at 16:20, Rob Herring <robh+dt@kernel.org> wrote:
> What is the effect on boot time? It's highly platform dependent, but
> the worst case could be pretty bad I think.

Yeah, it can increase considerably here and I have plans for that, just
that i didn't wanted to get them in the first iteration to keep things simple.

What we can (should?) do is, that the boot constraint framework can hook
into other frameworks like regulators/clk/PM, etc, so that whenever a new
clk/regulator is added to those frameworks, they check for pending
requests from boot constraint framework. If found, they can call a callback
of the boot constraint framework which will set the constraints to the resource
before anyone else gets a chance. At that point we can remove the early
defer probing support that this patch is adding. And things would be quite fast
then.

> I don't see how this handles the case you mentioned where the amba
> pclk gets disabled. It only works if the constraint device is added
> before any others, but that is done with initcall level games.

Yeah, so as I said earlier, the basic idea is that these constraints must get
set before any user driver (for constrained devices) comes up. And the
only way to do that is by making sure the constraints get added at early
initcall levels. The same is done for all the three example drivers I have
added.

The amba-pclk thing isn't a issue then, as that stuff happens only at probe
and not when the amba device is created.

--
viresh

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 11/12] boot_constraint: Add support for IMX platform
  2017-10-29 13:48 ` [PATCH V4 11/12] boot_constraint: Add support for IMX platform Viresh Kumar
@ 2017-11-03  8:58   ` Sascha Hauer
  0 siblings, 0 replies; 40+ messages in thread
From: Sascha Hauer @ 2017-11-03  8:58 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Greg Kroah-Hartman, Vincent Guittot, Stephen Boyd,
	Rajendra Nayak, linux-kernel, linux-arm-kernel, robdclark,
	l.stach, shawnguo, fabio.estevam, nm, xuwei5, robh+dt

On Sun, Oct 29, 2017 at 07:18:59PM +0530, Viresh Kumar wrote:
> This adds boot constraint support for IMX platforms. Currently only one
> use case is supported: earlycon. Some of the UARTs are enabled by the
> bootloader and are used for early console in the kernel. The boot
> constraint core handles them properly and removes them once the serial
> device is probed by its driver.
> 
> This gets rid of lots of hacky code in the clock drivers.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  arch/arm/mach-imx/Kconfig         |   1 +
>  drivers/boot_constraints/Makefile |   1 +
>  drivers/boot_constraints/imx.c    | 113 ++++++++++++++++++++++++++++++++++++++
>  drivers/clk/imx/clk-imx25.c       |  12 ----
>  drivers/clk/imx/clk-imx27.c       |  13 -----
>  drivers/clk/imx/clk-imx31.c       |  12 ----
>  drivers/clk/imx/clk-imx35.c       |  10 ----
>  drivers/clk/imx/clk-imx51-imx53.c |  16 ------
>  drivers/clk/imx/clk-imx6q.c       |   8 ---
>  drivers/clk/imx/clk-imx6sl.c      |   8 ---
>  drivers/clk/imx/clk-imx6sx.c      |   8 ---
>  drivers/clk/imx/clk-imx7d.c       |  14 -----
>  drivers/clk/imx/clk.c             |  38 -------------
>  drivers/clk/imx/clk.h             |   1 -
>  14 files changed, 115 insertions(+), 140 deletions(-)
>  create mode 100644 drivers/boot_constraints/imx.c
> 
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index 782699e67600..9ea1fe32b280 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -4,6 +4,7 @@ menuconfig ARCH_MXC
>  	select ARCH_SUPPORTS_BIG_ENDIAN
>  	select CLKSRC_IMX_GPT
>  	select GENERIC_IRQ_CHIP
> +	select DEV_BOOT_CONSTRAINTS
>  	select GPIOLIB
>  	select PINCTRL
>  	select PM_OPP if PM
> diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
> index 43c89d2458e9..3b5a87fcf099 100644
> --- a/drivers/boot_constraints/Makefile
> +++ b/drivers/boot_constraints/Makefile
> @@ -3,3 +3,4 @@
>  obj-y := clk.o deferrable_dev.o core.o pm.o serial.o supply.o
>  
>  obj-$(CONFIG_ARCH_HISI)			+= hikey.o
> +obj-$(CONFIG_ARCH_MXC)			+= imx.o
> +
> +static const struct of_device_id machines[] __initconst = {
> +	{ .compatible = "fsl,imx25", .data = &imx_constraints },
> +	{ .compatible = "fsl,imx27", .data = &imx_constraints },
> +	{ .compatible = "fsl,imx31", .data = &imx_constraints },
> +	{ .compatible = "fsl,imx35", .data = &imx_constraints },
> +	{ .compatible = "fsl,imx50", .data = &imx_constraints },
> +	{ .compatible = "fsl,imx51", .data = &imx_constraints },
> +	{ .compatible = "fsl,imx53", .data = &imx_constraints },
> +	{ .compatible = "fsl,imx6dl", .data = &imx_constraints },
> +	{ .compatible = "fsl,imx6q", .data = &imx_constraints },
> +	{ .compatible = "fsl,imx6qp", .data = &imx_constraints },
> +	{ .compatible = "fsl,imx6sl", .data = &imx_constraints },
> +	{ .compatible = "fsl,imx6sx", .data = &imx_constraints },
> +	{ .compatible = "fsl,imx6ul", .data = &imx_constraints },
> +	{ .compatible = "fsl,imx6ull", .data = &imx_constraints },
> +	{ .compatible = "fsl,imx7d", .data = &imx7_constraints },
> +	{ .compatible = "fsl,imx7s", .data = &imx7_constraints },
> +	{ }
> +};
> +
> +static int __init imx_constraints_init(void)
> +{
> +	const struct imx_machine_constraints *constraints;
> +	const struct of_device_id *match;
> +	struct device_node *np;
> +
> +	if (!boot_constraint_earlycon_enabled())
> +		return 0;
> +
> +	np = of_find_node_by_path("/");
> +	if (!np)
> +		return -ENODEV;
> +
> +	match = of_match_node(machines, np);
> +	of_node_put(np);
> +
> +	if (!match)
> +		return 0;
> +
> +	constraints = match->data;
> +	BUG_ON(!constraints);
> +
> +	dev_boot_constraint_add_deferrable_of(constraints->dev_constraints,
> +					      constraints->count);
> +
> +	return 0;
> +}
> +subsys_initcall(imx_constraints_init);

As said to Viresh privately: I would pretty much prefer this code being
hooked up to some SoC specific code which already knows the SoC type rather
than looping around the compatible array for all enabled machines (which
may even not only be i.MX for multi SoC kernels).
Vireshs response was that adding more SoC specific code may not be
wanted after we've finally got rid of most of it. So basically we need
a third opinion ;)

Other than that I tested an earlier version of this patch on i.MX6 and
it worked as expected.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 00/12] drivers: Boot Constraints core
  2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
                   ` (12 preceding siblings ...)
  2017-10-30 22:07 ` [PATCH V4 00/12] drivers: Boot Constraints core Rob Herring
@ 2017-11-28  4:18 ` Viresh Kumar
  2017-12-13  9:55   ` Greg Kroah-Hartman
  13 siblings, 1 reply; 40+ messages in thread
From: Viresh Kumar @ 2017-11-28  4:18 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt, devicetree, Frank Rowand

On 29-10-17, 19:18, Viresh Kumar wrote:
> Here is V4 of the boot constraints core based on your feedback from V3.
> We now have support for three platforms (as you suggested) included in
> this series: Hisilicon, IMX and Qualcomm.

Hi Greg,

I was waiting for rc1 to come out before sending a reminder for this
series and so here is one :)

-- 
viresh

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 03/12] drivers: Add boot constraints core
  2017-10-29 13:48 ` [PATCH V4 03/12] drivers: Add boot constraints core Viresh Kumar
@ 2017-12-13  9:42   ` Greg Kroah-Hartman
  2017-12-13  9:59     ` Viresh Kumar
  2017-12-13  9:42   ` Greg Kroah-Hartman
  1 sibling, 1 reply; 40+ messages in thread
From: Greg Kroah-Hartman @ 2017-12-13  9:42 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt

On Sun, Oct 29, 2017 at 07:18:51PM +0530, Viresh Kumar wrote:
> Some devices are powered ON by the bootloader before the bootloader
> handovers control to Linux. It maybe important for those devices to keep
> working until the time a Linux device driver probes the device and
> reconfigure its resources.
> 
> A typical example of that can be the LCD controller, which is used by
> the bootloaders to show image(s) while the platform is booting into
> Linux. The LCD controller can be using some resources, like clk,
> regulators, PM domain, etc, that are shared between several devices.
> These shared resources should be configured to satisfy need of all the
> users. If another device's (X) driver gets probed before the LCD
> controller driver in this case, then it may end up reconfiguring these
> resources to ranges satisfying the current users (only device X) and
> that can make the LCD screen unstable.
> 
> This patch introduces the concept of boot-constraints, which will be set
> by the bootloaders and the kernel will satisfy them until the time
> driver for such a device is probed (successfully or unsuccessfully).
> 
> The list of boot constraint types is empty for now, and will be
> incrementally updated by later patches.
> 
> Only two routines are exposed by the boot constraints core for now:
> 
> - dev_boot_constraint_add(): This shall be called by parts of the kernel
>   (before the device is probed) to set the constraints.
> 
> - dev_boot_constraints_remove(): This is called only by the driver core
>   after a device is probed successfully or unsuccessfully. Special
>   handling is done here for deferred probing.
> 
> Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Minor nits:

> ---
>  drivers/Kconfig                   |   2 +
>  drivers/Makefile                  |   1 +
>  drivers/base/dd.c                 |  20 ++--
>  drivers/boot_constraints/Kconfig  |   9 ++
>  drivers/boot_constraints/Makefile |   3 +
>  drivers/boot_constraints/core.c   | 199 ++++++++++++++++++++++++++++++++++++++
>  drivers/boot_constraints/core.h   |  33 +++++++
>  include/linux/boot_constraint.h   |  46 +++++++++
>  8 files changed, 306 insertions(+), 7 deletions(-)
>  create mode 100644 drivers/boot_constraints/Kconfig
>  create mode 100644 drivers/boot_constraints/Makefile
>  create mode 100644 drivers/boot_constraints/core.c
>  create mode 100644 drivers/boot_constraints/core.h
>  create mode 100644 include/linux/boot_constraint.h
> 
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 505c676fa9c7..e595ffad2214 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -4,6 +4,8 @@ source "drivers/amba/Kconfig"
>  
>  source "drivers/base/Kconfig"
>  
> +source "drivers/boot_constraints/Kconfig"
> +
>  source "drivers/bus/Kconfig"
>  
>  source "drivers/connector/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index d90fdc413648..29d03466cb2a 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -72,6 +72,7 @@ obj-$(CONFIG_FB_INTEL)          += video/fbdev/intelfb/
>  obj-$(CONFIG_PARPORT)		+= parport/
>  obj-$(CONFIG_NVM)		+= lightnvm/
>  obj-y				+= base/ block/ misc/ mfd/ nfc/
> +obj-$(CONFIG_DEV_BOOT_CONSTRAINTS) += boot_constraints/
>  obj-$(CONFIG_LIBNVDIMM)		+= nvdimm/
>  obj-$(CONFIG_DAX)		+= dax/
>  obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf/
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index ad44b40fe284..4eec27fe2b2b 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -17,6 +17,7 @@
>   * This file is released under the GPLv2
>   */

Can you rebase this patch, I think it will have conflicts or fuzz here.

>  
> +#include <linux/boot_constraint.h>
>  #include <linux/device.h>
>  #include <linux/delay.h>
>  #include <linux/dma-mapping.h>
> @@ -409,15 +410,20 @@ static int really_probe(struct device *dev, struct device_driver *drv)
>  	 */
>  	devices_kset_move_last(dev);
>  
> -	if (dev->bus->probe) {
> +	if (dev->bus->probe)
>  		ret = dev->bus->probe(dev);
> -		if (ret)
> -			goto probe_failed;
> -	} else if (drv->probe) {
> +	else if (drv->probe)
>  		ret = drv->probe(dev);
> -		if (ret)
> -			goto probe_failed;
> -	}
> +
> +	/*
> +	 * Remove boot constraints for both successful and unsuccessful probe(),
> +	 * except for the case where EPROBE_DEFER is returned by probe().
> +	 */
> +	if (ret != -EPROBE_DEFER)
> +		dev_boot_constraints_remove(dev);

This feels odd, but ok, I trust you :)

> +
> +	if (ret)
> +		goto probe_failed;
>  
>  	if (test_remove) {
>  		test_remove = false;
> diff --git a/drivers/boot_constraints/Kconfig b/drivers/boot_constraints/Kconfig
> new file mode 100644
> index 000000000000..77831af1c6fb
> --- /dev/null
> +++ b/drivers/boot_constraints/Kconfig
> @@ -0,0 +1,9 @@
> +config DEV_BOOT_CONSTRAINTS
> +	bool "Boot constraints for devices"
> +	help
> +	  This enables boot constraints detection for devices. These constraints
> +	  are (normally) set by the Bootloader and must be satisfied by the
> +	  kernel until the relevant device driver is probed. Once the driver is
> +	  probed, the constraint is dropped.
> +
> +	  If unsure, say N.
> diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
> new file mode 100644
> index 000000000000..0f2680177974
> --- /dev/null
> +++ b/drivers/boot_constraints/Makefile
> @@ -0,0 +1,3 @@
> +# Makefile for device boot constraints
> +
> +obj-y := core.o
> diff --git a/drivers/boot_constraints/core.c b/drivers/boot_constraints/core.c
> new file mode 100644
> index 000000000000..366a05d6d9ba
> --- /dev/null
> +++ b/drivers/boot_constraints/core.c
> @@ -0,0 +1,199 @@
> +/*
> + * This takes care of boot time device constraints, normally set by the
> + * Bootloader.
> + *
> + * Copyright (C) 2017 Linaro.
> + * Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * This file is released under the GPLv2.

Care to update this patch with the new SPDX format for licenses?

> + */
> +
> +#define pr_fmt(fmt) "Boot Constraints: " fmt

You don't have any pr_* calls, so this isn't needed :)

> +struct constraint {
> +	struct constraint_dev *cdev;
> +	struct list_head node;
> +	enum dev_boot_constraint_type type;
> +	void (*free_resources)(void *data);
> +	void *free_resources_data;
> +
> +	int (*add)(struct constraint *constraint, void *data);
> +	void (*remove)(struct constraint *constraint);
> +	void *private;
> +};
> +
> +/* Forward declarations of constraint specific callbacks */
> +#endif /* _CORE_H */

What is this comment at the end of the file for?

> diff --git a/include/linux/boot_constraint.h b/include/linux/boot_constraint.h
> new file mode 100644
> index 000000000000..2b816bf74144
> --- /dev/null
> +++ b/include/linux/boot_constraint.h
> @@ -0,0 +1,46 @@
> +/*
> + * Boot constraints header.
> + *
> + * Copyright (C) 2017 Linaro.
> + * Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * This file is released under the GPLv2
> + */
> +#ifndef _LINUX_BOOT_CONSTRAINT_H
> +#define _LINUX_BOOT_CONSTRAINT_H
> +
> +#include <linux/err.h>
> +#include <linux/types.h>
> +
> +struct device;
> +
> +enum dev_boot_constraint_type {
> +	DEV_BOOT_CONSTRAINT_NONE,
> +};
> +
> +struct dev_boot_constraint {
> +	enum dev_boot_constraint_type type;
> +	void *data;
> +};
> +
> +struct dev_boot_constraint_info {
> +	struct dev_boot_constraint constraint;
> +
> +	/* This will be called just before the constraint is removed */
> +	void (*free_resources)(void *data);
> +	void *free_resources_data;
> +};
> +
> +#ifdef CONFIG_DEV_BOOT_CONSTRAINTS
> +int dev_boot_constraint_add(struct device *dev,
> +			    struct dev_boot_constraint_info *info);
> +void dev_boot_constraints_remove(struct device *dev);
> +#else
> +static inline
> +int dev_boot_constraint_add(struct device *dev,
> +			    struct dev_boot_constraint_info *info)
> +{ return -EINVAL; }

Why return an error?  Shouldn't this just "succeed" if the option is not
built in?  What will you do with it if it fails because of this?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 03/12] drivers: Add boot constraints core
  2017-10-29 13:48 ` [PATCH V4 03/12] drivers: Add boot constraints core Viresh Kumar
  2017-12-13  9:42   ` Greg Kroah-Hartman
@ 2017-12-13  9:42   ` Greg Kroah-Hartman
  2017-12-13 10:00     ` Viresh Kumar
  1 sibling, 1 reply; 40+ messages in thread
From: Greg Kroah-Hartman @ 2017-12-13  9:42 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt

On Sun, Oct 29, 2017 at 07:18:51PM +0530, Viresh Kumar wrote:
> Some devices are powered ON by the bootloader before the bootloader
> handovers control to Linux. It maybe important for those devices to keep
> working until the time a Linux device driver probes the device and
> reconfigure its resources.
> 
> A typical example of that can be the LCD controller, which is used by
> the bootloaders to show image(s) while the platform is booting into
> Linux. The LCD controller can be using some resources, like clk,
> regulators, PM domain, etc, that are shared between several devices.
> These shared resources should be configured to satisfy need of all the
> users. If another device's (X) driver gets probed before the LCD
> controller driver in this case, then it may end up reconfiguring these
> resources to ranges satisfying the current users (only device X) and
> that can make the LCD screen unstable.
> 
> This patch introduces the concept of boot-constraints, which will be set
> by the bootloaders and the kernel will satisfy them until the time
> driver for such a device is probed (successfully or unsuccessfully).
> 
> The list of boot constraint types is empty for now, and will be
> incrementally updated by later patches.
> 
> Only two routines are exposed by the boot constraints core for now:

I think we need some documentation somewhere on how to use this, right?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 09/12] boot_constraint: Add earlycon helper
  2017-10-29 13:48 ` [PATCH V4 09/12] boot_constraint: Add earlycon helper Viresh Kumar
@ 2017-12-13  9:44   ` Greg Kroah-Hartman
  2017-12-14  5:22     ` Viresh Kumar
  0 siblings, 1 reply; 40+ messages in thread
From: Greg Kroah-Hartman @ 2017-12-13  9:44 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt

On Sun, Oct 29, 2017 at 07:18:57PM +0530, Viresh Kumar wrote:
> Getting boot messages during initial kernel boot is a common problem,
> which (almost) everyone wants to solve. Considering that this would be
> required by multiple platforms, provide a helper to check if "earlycon"
> or "earlyprintk" boot arguments are passed to kernel or not. The
> platforms can use this helper to add serial constraints only if earlycon
> if required.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/boot_constraints/Makefile |  2 +-
>  drivers/boot_constraints/serial.c | 28 ++++++++++++++++++++++++++++
>  include/linux/boot_constraint.h   |  2 ++
>  3 files changed, 31 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/boot_constraints/serial.c
> 
> diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
> index a765094623a3..0d4f88bb767c 100644
> --- a/drivers/boot_constraints/Makefile
> +++ b/drivers/boot_constraints/Makefile
> @@ -1,3 +1,3 @@
>  # Makefile for device boot constraints
>  
> -obj-y := clk.o deferrable_dev.o core.o pm.o supply.o
> +obj-y := clk.o deferrable_dev.o core.o pm.o serial.o supply.o
> diff --git a/drivers/boot_constraints/serial.c b/drivers/boot_constraints/serial.c
> new file mode 100644
> index 000000000000..d0d07d4aa6af
> --- /dev/null
> +++ b/drivers/boot_constraints/serial.c
> @@ -0,0 +1,28 @@
> +/*
> + * This contains helpers related to serial boot constraints.
> + *
> + * Copyright (C) 2017 Linaro.
> + * Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * This file is released under the GPLv2.
> + */
> +
> +#include <linux/init.h>
> +
> +static bool earlycon_boot_constraints_enabled __initdata;
> +
> +bool __init boot_constraint_earlycon_enabled(void)
> +{
> +	return earlycon_boot_constraints_enabled;
> +}
> +
> +static int __init enable_earlycon_boot_constraints(char *str)
> +{
> +	earlycon_boot_constraints_enabled = true;
> +
> +	return 0;
> +}
> +__setup_param("earlycon", boot_constraint_earlycon,
> +	      enable_earlycon_boot_constraints, 0);
> +__setup_param("earlyprintk", boot_constraint_earlyprintk,
> +	      enable_earlycon_boot_constraints, 0);
> diff --git a/include/linux/boot_constraint.h b/include/linux/boot_constraint.h
> index c110b36e490f..aeada69b87e6 100644
> --- a/include/linux/boot_constraint.h
> +++ b/include/linux/boot_constraint.h
> @@ -10,6 +10,7 @@
>  #define _LINUX_BOOT_CONSTRAINT_H
>  
>  #include <linux/err.h>
> +#include <linux/init.h>
>  #include <linux/types.h>
>  
>  struct device;
> @@ -58,6 +59,7 @@ int dev_boot_constraint_add(struct device *dev,
>  void dev_boot_constraints_remove(struct device *dev);
>  void dev_boot_constraint_add_deferrable_of(struct dev_boot_constraint_of *oconst,
>  					   int count);
> +bool __init boot_constraint_earlycon_enabled(void);
>  #else
>  static inline
>  int dev_boot_constraint_add(struct device *dev,

No need for this function if it's not enabled?

And this feels really odd, does it really save any work for the
individual "constraint" to check for this option?  I'm all for helper
functions, but this feels like more work than it's worth...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 10/12] boot_constraint: Add support for Hisilicon platforms
  2017-10-29 13:48 ` [PATCH V4 10/12] boot_constraint: Add support for Hisilicon platforms Viresh Kumar
@ 2017-12-13  9:47   ` Greg Kroah-Hartman
  2017-12-13 10:13     ` Viresh Kumar
  0 siblings, 1 reply; 40+ messages in thread
From: Greg Kroah-Hartman @ 2017-12-13  9:47 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt

On Sun, Oct 29, 2017 at 07:18:58PM +0530, Viresh Kumar wrote:
> This adds boot constraint support for Hisilicon platforms. Currently
> only one use case is supported: earlycon. One of the UART is enabled by
> the bootloader and is used for early console in the kernel. The boot
> constraint core handles it properly and removes constraints once the
> serial device is probed by its driver.
> 
> This is tested on hi6220-hikey 96board.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  arch/arm64/Kconfig.platforms      |   1 +
>  drivers/boot_constraints/Makefile |   2 +
>  drivers/boot_constraints/hikey.c  | 145 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 148 insertions(+)
>  create mode 100644 drivers/boot_constraints/hikey.c
> 
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index 6b54ee8c1262..265df4a088ab 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -87,6 +87,7 @@ config ARCH_HISI
>  	select ARM_TIMER_SP804
>  	select HISILICON_IRQ_MBIGEN if PCI
>  	select PINCTRL
> +	select DEV_BOOT_CONSTRAINTS
>  	help
>  	  This enables support for Hisilicon ARMv8 SoC family
>  
> diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
> index 0d4f88bb767c..43c89d2458e9 100644
> --- a/drivers/boot_constraints/Makefile
> +++ b/drivers/boot_constraints/Makefile
> @@ -1,3 +1,5 @@
>  # Makefile for device boot constraints
>  
>  obj-y := clk.o deferrable_dev.o core.o pm.o serial.o supply.o
> +
> +obj-$(CONFIG_ARCH_HISI)			+= hikey.o
> diff --git a/drivers/boot_constraints/hikey.c b/drivers/boot_constraints/hikey.c
> new file mode 100644
> index 000000000000..5f69f9451d93
> --- /dev/null
> +++ b/drivers/boot_constraints/hikey.c
> @@ -0,0 +1,145 @@
> +/*
> + * This takes care of Hisilicon boot time device constraints, normally set by
> + * the Bootloader.
> + *
> + * Copyright (C) 2017 Linaro.
> + * Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * This file is released under the GPLv2.
> + */
> +
> +#include <linux/boot_constraint.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +
> +struct hikey_machine_constraints {
> +	struct dev_boot_constraint_of *dev_constraints;
> +	unsigned int count;
> +};
> +
> +static struct dev_boot_constraint_clk_info uart_iclk_info = {
> +	.name = "uartclk",
> +};
> +
> +static struct dev_boot_constraint_clk_info uart_pclk_info = {
> +	.name = "apb_pclk",
> +};
> +
> +static struct dev_boot_constraint hikey3660_uart_constraints[] = {
> +	{
> +		.type = DEV_BOOT_CONSTRAINT_CLK,
> +		.data = &uart_iclk_info,
> +	}, {
> +		.type = DEV_BOOT_CONSTRAINT_CLK,
> +		.data = &uart_pclk_info,
> +	},
> +};
> +
> +static const char * const uarts_hikey3660[] = {
> +	"serial@fff32000",	/* UART 6 */
> +};
> +
> +static struct dev_boot_constraint_of hikey3660_dev_constraints[] = {
> +	{
> +		.compat = "arm,pl011",
> +		.constraints = hikey3660_uart_constraints,
> +		.count = ARRAY_SIZE(hikey3660_uart_constraints),
> +
> +		.dev_names = uarts_hikey3660,
> +		.dev_names_count = ARRAY_SIZE(uarts_hikey3660),
> +	},
> +};
> +
> +static struct hikey_machine_constraints hikey3660_constraints = {
> +	.dev_constraints = hikey3660_dev_constraints,
> +	.count = ARRAY_SIZE(hikey3660_dev_constraints),
> +};
> +
> +static const char * const uarts_hikey6220[] = {
> +	"uart@f7113000",	/* UART 3 */
> +};
> +
> +static struct dev_boot_constraint_of hikey6220_dev_constraints[] = {
> +	{
> +		.compat = "arm,pl011",
> +		.constraints = hikey3660_uart_constraints,
> +		.count = ARRAY_SIZE(hikey3660_uart_constraints),
> +
> +		.dev_names = uarts_hikey6220,
> +		.dev_names_count = ARRAY_SIZE(uarts_hikey6220),
> +	},
> +};
> +
> +static struct hikey_machine_constraints hikey6220_constraints = {
> +	.dev_constraints = hikey6220_dev_constraints,
> +	.count = ARRAY_SIZE(hikey6220_dev_constraints),
> +};
> +
> +static struct dev_boot_constraint hikey3798cv200_uart_constraints[] = {
> +	{
> +		.type = DEV_BOOT_CONSTRAINT_CLK,
> +		.data = &uart_pclk_info,
> +	},
> +};
> +
> +static const char * const uarts_hikey3798cv200[] = {
> +	"serial@8b00000",	/* UART 0 */
> +};
> +
> +static struct dev_boot_constraint_of hikey3798cv200_dev_constraints[] = {
> +	{
> +		.compat = "arm,pl011",
> +		.constraints = hikey3798cv200_uart_constraints,
> +		.count = ARRAY_SIZE(hikey3798cv200_uart_constraints),
> +
> +		.dev_names = uarts_hikey3798cv200,
> +		.dev_names_count = ARRAY_SIZE(uarts_hikey3798cv200),
> +	},
> +};
> +
> +static struct hikey_machine_constraints hikey3798cv200_constraints = {
> +	.dev_constraints = hikey3798cv200_dev_constraints,
> +	.count = ARRAY_SIZE(hikey3798cv200_dev_constraints),
> +};
> +
> +static const struct of_device_id machines[] __initconst = {
> +	{ .compatible = "hisilicon,hi3660", .data = &hikey3660_constraints },
> +	{ .compatible = "hisilicon,hi3798cv200", .data = &hikey3798cv200_constraints },
> +	{ .compatible = "hisilicon,hi6220", .data = &hikey6220_constraints },
> +	{ }
> +};
> +
> +static int __init hikey_constraints_init(void)
> +{
> +	const struct hikey_machine_constraints *constraints;
> +	const struct of_device_id *match;
> +	struct device_node *np;
> +
> +	if (!boot_constraint_earlycon_enabled())
> +		return 0;
> +
> +	np = of_find_node_by_path("/");

What is this for?

> +	if (!np)
> +		return -ENODEV;
> +
> +	match = of_match_node(machines, np);
> +	of_node_put(np);
> +
> +	if (!match)
> +		return 0;
> +
> +	constraints = match->data;
> +	BUG_ON(!constraints);

Never crash the device for a driver configuration issue.  That's going
to be bad.

> +	dev_boot_constraint_add_deferrable_of(constraints->dev_constraints,
> +					      constraints->count);
> +
> +	return 0;
> +}
> +
> +/*
> + * The amba-pl011 driver registers itself from arch_initcall level. Setup the
> + * serial boot constraints before that in order not to miss any boot messages.
> + */
> +postcore_initcall_sync(hikey_constraints_init);

Now you have to worry about the bootconstraints earlycon being called
before/after your code.  That's another linking order dependancy you
just created.  It feels more complex for something so "simple" as
looking for the earlycon flag...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 06/12] boot_constraint: Add support for PM constraints
  2017-10-29 13:48 ` [PATCH V4 06/12] boot_constraint: Add support for PM constraints Viresh Kumar
@ 2017-12-13  9:48   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 40+ messages in thread
From: Greg Kroah-Hartman @ 2017-12-13  9:48 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt

On Sun, Oct 29, 2017 at 07:18:54PM +0530, Viresh Kumar wrote:
> This patch adds the PM constraint type.
> 
> The constraint is set by attaching the power domain for the device,
> which will also enable the power domain. This guarantees that the power
> domain doesn't get shut down while being used.
> 
> We don't need to detach the power domain to remove the constraint as the
> domain is attached only once, from here or before driver probe.
> 
> Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/boot_constraints/Makefile |  2 +-
>  drivers/boot_constraints/core.c   |  4 ++++
>  drivers/boot_constraints/core.h   |  3 +++
>  drivers/boot_constraints/pm.c     | 24 ++++++++++++++++++++++++
>  include/linux/boot_constraint.h   |  1 +
>  5 files changed, 33 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/boot_constraints/pm.c
> 
> diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
> index 3424379fd1e4..b7ade1a7afb5 100644
> --- a/drivers/boot_constraints/Makefile
> +++ b/drivers/boot_constraints/Makefile
> @@ -1,3 +1,3 @@
>  # Makefile for device boot constraints
>  
> -obj-y := clk.o core.o supply.o
> +obj-y := clk.o core.o pm.o supply.o
> diff --git a/drivers/boot_constraints/core.c b/drivers/boot_constraints/core.c
> index 9213e56e8078..f4d3520ddb04 100644
> --- a/drivers/boot_constraints/core.c
> +++ b/drivers/boot_constraints/core.c
> @@ -98,6 +98,10 @@ static struct constraint *constraint_allocate(struct constraint_dev *cdev,
>  		add = constraint_clk_add;
>  		remove = constraint_clk_remove;
>  		break;
> +	case DEV_BOOT_CONSTRAINT_PM:
> +		add = constraint_pm_add;
> +		remove = constraint_pm_remove;
> +		break;
>  	case DEV_BOOT_CONSTRAINT_SUPPLY:
>  		add = constraint_supply_add;
>  		remove = constraint_supply_remove;
> diff --git a/drivers/boot_constraints/core.h b/drivers/boot_constraints/core.h
> index 4f28ac2ef691..a051c3d7c8ab 100644
> --- a/drivers/boot_constraints/core.h
> +++ b/drivers/boot_constraints/core.h
> @@ -33,6 +33,9 @@ struct constraint {
>  int constraint_clk_add(struct constraint *constraint, void *data);
>  void constraint_clk_remove(struct constraint *constraint);
>  
> +int constraint_pm_add(struct constraint *constraint, void *data);
> +void constraint_pm_remove(struct constraint *constraint);
> +
>  int constraint_supply_add(struct constraint *constraint, void *data);
>  void constraint_supply_remove(struct constraint *constraint);
>  
> diff --git a/drivers/boot_constraints/pm.c b/drivers/boot_constraints/pm.c
> new file mode 100644
> index 000000000000..edba5eca5093
> --- /dev/null
> +++ b/drivers/boot_constraints/pm.c
> @@ -0,0 +1,24 @@
> +/*
> + * Copyright (C) 2017 Linaro.
> + * Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * This file is released under the GPLv2.
> + */
> +
> +#define pr_fmt(fmt) "PM Boot Constraints: " fmt

You don't use this :(

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 05/12] boot_constraint: Add support for clk constraints
  2017-10-29 13:48 ` [PATCH V4 05/12] boot_constraint: Add support for clk constraints Viresh Kumar
@ 2017-12-13  9:48   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 40+ messages in thread
From: Greg Kroah-Hartman @ 2017-12-13  9:48 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt

On Sun, Oct 29, 2017 at 07:18:53PM +0530, Viresh Kumar wrote:
> This patch adds the clk constraint type.
> 
> The constraint is set by enabling the clk for the device. Once the
> device is probed, the clk is disabled and the constraint is removed.
> 
> We may want to do clk_set_rate() from here, but lets wait for some real
> users that really want it.
> 
> Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/boot_constraints/Makefile |  2 +-
>  drivers/boot_constraints/clk.c    | 70 +++++++++++++++++++++++++++++++++++++++
>  drivers/boot_constraints/core.c   |  4 +++
>  drivers/boot_constraints/core.h   |  3 ++
>  include/linux/boot_constraint.h   |  5 +++
>  5 files changed, 83 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/boot_constraints/clk.c
> 
> diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
> index a45616f0c3b0..3424379fd1e4 100644
> --- a/drivers/boot_constraints/Makefile
> +++ b/drivers/boot_constraints/Makefile
> @@ -1,3 +1,3 @@
>  # Makefile for device boot constraints
>  
> -obj-y := core.o supply.o
> +obj-y := clk.o core.o supply.o
> diff --git a/drivers/boot_constraints/clk.c b/drivers/boot_constraints/clk.c
> new file mode 100644
> index 000000000000..b5b1d63c3e76
> --- /dev/null
> +++ b/drivers/boot_constraints/clk.c
> @@ -0,0 +1,70 @@
> +/*
> + * Copyright (C) 2017 Linaro.
> + * Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * This file is released under the GPLv2.
> + */
> +
> +#define pr_fmt(fmt) "Clock Boot Constraints: " fmt

You don't use this :(

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 04/12] boot_constraint: Add support for supply constraints
  2017-10-29 13:48 ` [PATCH V4 04/12] boot_constraint: Add support for supply constraints Viresh Kumar
@ 2017-12-13  9:48   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 40+ messages in thread
From: Greg Kroah-Hartman @ 2017-12-13  9:48 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt

On Sun, Oct 29, 2017 at 07:18:52PM +0530, Viresh Kumar wrote:
> This patch adds the first constraint type: power-supply.
> 
> The constraint is set by enabling the regulator and setting a voltage
> range (if required) for the respective regulator device, which will be
> honored by the regulator core even if more users turn up. Once the
> device is probed, the regulator is released and the constraint is
> removed.
> 
> Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/boot_constraints/Makefile |  2 +-
>  drivers/boot_constraints/core.c   |  4 ++
>  drivers/boot_constraints/core.h   |  3 ++
>  drivers/boot_constraints/supply.c | 98 +++++++++++++++++++++++++++++++++++++++
>  include/linux/boot_constraint.h   |  8 +++-
>  5 files changed, 113 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/boot_constraints/supply.c
> 
> diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
> index 0f2680177974..a45616f0c3b0 100644
> --- a/drivers/boot_constraints/Makefile
> +++ b/drivers/boot_constraints/Makefile
> @@ -1,3 +1,3 @@
>  # Makefile for device boot constraints
>  
> -obj-y := core.o
> +obj-y := core.o supply.o
> diff --git a/drivers/boot_constraints/core.c b/drivers/boot_constraints/core.c
> index 366a05d6d9ba..b9c024a3bdf5 100644
> --- a/drivers/boot_constraints/core.c
> +++ b/drivers/boot_constraints/core.c
> @@ -94,6 +94,10 @@ static struct constraint *constraint_allocate(struct constraint_dev *cdev,
>  	void (*remove)(struct constraint *constraint);
>  
>  	switch (type) {
> +	case DEV_BOOT_CONSTRAINT_SUPPLY:
> +		add = constraint_supply_add;
> +		remove = constraint_supply_remove;
> +		break;
>  	default:
>  		return ERR_PTR(-EINVAL);
>  	}
> diff --git a/drivers/boot_constraints/core.h b/drivers/boot_constraints/core.h
> index 7ba4ac172c09..73b9d2d22a12 100644
> --- a/drivers/boot_constraints/core.h
> +++ b/drivers/boot_constraints/core.h
> @@ -30,4 +30,7 @@ struct constraint {
>  };
>  
>  /* Forward declarations of constraint specific callbacks */
> +int constraint_supply_add(struct constraint *constraint, void *data);
> +void constraint_supply_remove(struct constraint *constraint);
> +
>  #endif /* _CORE_H */
> diff --git a/drivers/boot_constraints/supply.c b/drivers/boot_constraints/supply.c
> new file mode 100644
> index 000000000000..30f816dbf12c
> --- /dev/null
> +++ b/drivers/boot_constraints/supply.c
> @@ -0,0 +1,98 @@
> +/*
> + * Copyright (C) 2017 Linaro.
> + * Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * This file is released under the GPLv2.
> + */
> +
> +#define pr_fmt(fmt) "Supply Boot Constraints: " fmt

And again :)

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 07/12] boot_constraint: Add debugfs support
  2017-10-29 13:48 ` [PATCH V4 07/12] boot_constraint: Add debugfs support Viresh Kumar
  2017-10-29 15:09   ` Randy Dunlap
@ 2017-12-13  9:50   ` Greg Kroah-Hartman
  1 sibling, 0 replies; 40+ messages in thread
From: Greg Kroah-Hartman @ 2017-12-13  9:50 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt

On Sun, Oct 29, 2017 at 07:18:55PM +0530, Viresh Kumar wrote:
> This patch adds debugfs support for boot constraints. This is how it
> looks for a "vmmc-supply" constraint for the MMC device.
> 
> $ ls -R /sys/kernel/debug/boot_constraints/
> /sys/kernel/debug/boot_constraints/:
> f723d000.dwmmc0
> 
> /sys/kernel/debug/boot_constraints/f723d000.dwmmc0:
> clk-ciu  pm-domain  supply-vmmc  supply-vmmcaux
> 
> /sys/kernel/debug/boot_constraints/f723d000.dwmmc0/clk-ciu:
> 
> /sys/kernel/debug/boot_constraints/f723d000.dwmmc0/pm-domain:
> 
> /sys/kernel/debug/boot_constraints/f723d000.dwmmc0/supply-vmmc:
> u_volt_max  u_volt_min
> 
> /sys/kernel/debug/boot_constraints/f723d000.dwmmc0/supply-vmmcaux:
> u_volt_max  u_volt_min
> 
> Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/boot_constraints/clk.c    |  3 ++
>  drivers/boot_constraints/core.c   | 60 +++++++++++++++++++++++++++++++++++++++
>  drivers/boot_constraints/core.h   |  6 ++++
>  drivers/boot_constraints/pm.c     | 11 +++++--
>  drivers/boot_constraints/supply.c |  9 ++++++
>  5 files changed, 87 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/boot_constraints/clk.c b/drivers/boot_constraints/clk.c
> index b5b1d63c3e76..91b7b538ef32 100644
> --- a/drivers/boot_constraints/clk.c
> +++ b/drivers/boot_constraints/clk.c
> @@ -49,6 +49,8 @@ int constraint_clk_add(struct constraint *constraint, void *data)
>  	cclk->clk_info.name = kstrdup_const(clk_info->name, GFP_KERNEL);
>  	constraint->private = cclk;
>  
> +	constraint_add_debugfs(constraint, clk_info->name);
> +
>  	return 0;
>  
>  put_clk:
> @@ -63,6 +65,7 @@ void constraint_clk_remove(struct constraint *constraint)
>  {
>  	struct constraint_clk *cclk = constraint->private;
>  
> +	constraint_remove_debugfs(constraint);
>  	kfree_const(cclk->clk_info.name);
>  	clk_disable_unprepare(cclk->clk);
>  	clk_put(cclk->clk);
> diff --git a/drivers/boot_constraints/core.c b/drivers/boot_constraints/core.c
> index f4d3520ddb04..707ffac690fc 100644
> --- a/drivers/boot_constraints/core.c
> +++ b/drivers/boot_constraints/core.c
> @@ -24,6 +24,64 @@
>  static LIST_HEAD(constraint_devices);
>  static DEFINE_MUTEX(constraint_devices_mutex);
>  
> +/* Debugfs */
> +
> +static struct dentry *rootdir;
> +
> +static void constraint_device_add_debugfs(struct constraint_dev *cdev)
> +{
> +	struct device *dev = cdev->dev;
> +
> +	cdev->dentry = debugfs_create_dir(dev_name(dev), rootdir);
> +}
> +
> +static void constraint_device_remove_debugfs(struct constraint_dev *cdev)
> +{
> +	debugfs_remove_recursive(cdev->dentry);
> +}
> +
> +void constraint_add_debugfs(struct constraint *constraint, const char *suffix)
> +{
> +	struct device *dev = constraint->cdev->dev;
> +	const char *prefix;
> +	char name[NAME_MAX];
> +
> +	switch (constraint->type) {
> +	case DEV_BOOT_CONSTRAINT_CLK:
> +		prefix = "clk";
> +		break;
> +	case DEV_BOOT_CONSTRAINT_PM:
> +		prefix = "pm";
> +		break;
> +	case DEV_BOOT_CONSTRAINT_SUPPLY:
> +		prefix = "supply";
> +		break;
> +	default:
> +		dev_err(dev, "%s: Constraint type (%d) not supported\n",
> +			__func__, constraint->type);
> +		return;
> +	}
> +
> +	snprintf(name, NAME_MAX, "%s-%s", prefix, suffix);
> +
> +	constraint->dentry = debugfs_create_dir(name, constraint->cdev->dentry);
> +}
> +
> +void constraint_remove_debugfs(struct constraint *constraint)
> +{
> +	debugfs_remove_recursive(constraint->dentry);
> +}
> +
> +static int __init constraint_debugfs_init(void)
> +{
> +	/* Create /sys/kernel/debug/opp directory */
> +	rootdir = debugfs_create_dir("boot_constraints", NULL);

Your comment makes no sense at all, it would be better, and correct, to
have no comment at all :)

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 08/12] boot_constraint: Manage deferrable constraints
  2017-10-29 13:48 ` [PATCH V4 08/12] boot_constraint: Manage deferrable constraints Viresh Kumar
  2017-10-31 16:20   ` Rob Herring
@ 2017-12-13  9:53   ` Greg Kroah-Hartman
  2017-12-13 10:27     ` Viresh Kumar
  1 sibling, 1 reply; 40+ messages in thread
From: Greg Kroah-Hartman @ 2017-12-13  9:53 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt

On Sun, Oct 29, 2017 at 07:18:56PM +0530, Viresh Kumar wrote:
> It is possible that some of the resources aren't available at the time
> constraints are getting set and the boot constraints core will return
> -EPROBE_DEFER for them. In order to retry adding the constraints at a
> later point of time (after the resource is added and before any of its
> users come up), this patch proposes two things:
> 
> - Each constraint is represented by a virtual platform device, so that
>   it is re-probed again until the time all the dependencies aren't met.
>   The platform device is removed along with the constraint, with help of
>   the free_resources() callback.
> 
> - Enable early defer probing support by calling
>   driver_enable_deferred_probe(), so that the core retries probing
>   deferred devices every time any device is bound to a driver. This
>   makes sure that the constraint is set before any of the users of the
>   resources come up.
> 
> This is tested on ARM64 Hikey board where probe was deferred for a
> device.
> 
> Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/base/dd.c                         |  12 ++
>  drivers/boot_constraints/Makefile         |   2 +-
>  drivers/boot_constraints/deferrable_dev.c | 235 ++++++++++++++++++++++++++++++
>  include/linux/boot_constraint.h           |  14 ++
>  4 files changed, 262 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/boot_constraints/deferrable_dev.c
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 4eec27fe2b2b..19eff5d08b9a 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -228,6 +228,18 @@ void device_unblock_probing(void)
>  	driver_deferred_probe_trigger();
>  }
>  
> +/**
> + * driver_enable_deferred_probe() - Enable probing of deferred devices
> + *
> + * We don't want to get in the way when the bulk of drivers are getting probed
> + * and so deferred probe is disabled in the beginning. Enable it now because we
> + * need it.
> + */
> +void driver_enable_deferred_probe(void)
> +{
> +	driver_deferred_probe_enable = true;
> +}
> +
>  /**
>   * deferred_probe_initcall() - Enable probing of deferred devices
>   *
> diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
> index b7ade1a7afb5..a765094623a3 100644
> --- a/drivers/boot_constraints/Makefile
> +++ b/drivers/boot_constraints/Makefile
> @@ -1,3 +1,3 @@
>  # Makefile for device boot constraints
>  
> -obj-y := clk.o core.o pm.o supply.o
> +obj-y := clk.o deferrable_dev.o core.o pm.o supply.o
> diff --git a/drivers/boot_constraints/deferrable_dev.c b/drivers/boot_constraints/deferrable_dev.c
> new file mode 100644
> index 000000000000..04056f317aff
> --- /dev/null
> +++ b/drivers/boot_constraints/deferrable_dev.c
> @@ -0,0 +1,235 @@
> +/*
> + * Copyright (C) 2017 Linaro.
> + * Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * This file is released under the GPLv2.
> + */
> +
> +#define pr_fmt(fmt) "Boot Constraints: " fmt

Hey, you use this one!

But you shouldn't :)

> +/* This only creates platform devices for now */
> +static void add_deferrable_of_single(struct device_node *np,
> +				     struct dev_boot_constraint *constraints,
> +				     int count)
> +{
> +	struct device *dev;
> +	int ret;
> +
> +	if (!of_device_is_available(np))
> +		return;
> +
> +	ret = of_platform_bus_create(np, NULL, NULL, NULL, false);
> +	if (ret)
> +		return;
> +
> +	if (of_device_is_compatible(np, "arm,primecell")) {

Why is "arm,primecell" in the core code here?

> +		struct amba_device *adev = of_find_amba_device_by_node(np);
> +
> +		if (!adev) {
> +			pr_err("Failed to find amba dev: %s\n", np->full_name);

Never use pr_* when you have a valid struct device to use.  Don't you
have one from the struct device_node * passed in here?

> +			return;
> +		}
> +		dev = &adev->dev;
> +	} else {
> +		struct platform_device *pdev = of_find_device_by_node(np);
> +
> +		if (!pdev) {
> +			pr_err("Failed to find pdev: %s\n", np->full_name);

Same here.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 00/12] drivers: Boot Constraints core
  2017-11-28  4:18 ` Viresh Kumar
@ 2017-12-13  9:55   ` Greg Kroah-Hartman
  2017-12-13 10:27     ` Viresh Kumar
  0 siblings, 1 reply; 40+ messages in thread
From: Greg Kroah-Hartman @ 2017-12-13  9:55 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt, devicetree, Frank Rowand

On Tue, Nov 28, 2017 at 09:48:18AM +0530, Viresh Kumar wrote:
> On 29-10-17, 19:18, Viresh Kumar wrote:
> > Here is V4 of the boot constraints core based on your feedback from V3.
> > We now have support for three platforms (as you suggested) included in
> > this series: Hisilicon, IMX and Qualcomm.
> 
> Hi Greg,
> 
> I was waiting for rc1 to come out before sending a reminder for this
> series and so here is one :)

I've reviewed it enough for now, needs a tiny bit of work, but looking
much better, nice job!

gre gk-h

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 03/12] drivers: Add boot constraints core
  2017-12-13  9:42   ` Greg Kroah-Hartman
@ 2017-12-13  9:59     ` Viresh Kumar
  0 siblings, 0 replies; 40+ messages in thread
From: Viresh Kumar @ 2017-12-13  9:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt

On 13-12-17, 10:42, Greg Kroah-Hartman wrote:
> On Sun, Oct 29, 2017 at 07:18:51PM +0530, Viresh Kumar wrote:
> > +	/*
> > +	 * Remove boot constraints for both successful and unsuccessful probe(),
> > +	 * except for the case where EPROBE_DEFER is returned by probe().
> > +	 */
> > +	if (ret != -EPROBE_DEFER)
> > +		dev_boot_constraints_remove(dev);
> 
> This feels odd, but ok, I trust you :)

I did this because it may not be right to keep the boot constraints up for a
device that failed to probe. For example, a LCD screen may continue wasting
power if its device failed to probe. At least I would like to see a real case
where we don't want to remove the constraints here on probe failure.

> > +/* Forward declarations of constraint specific callbacks */
> > +#endif /* _CORE_H */
> 
> What is this comment at the end of the file for?

Perhaps this should be moved to a later patch.

Ack for every other comment you gave.

-- 
viresh

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 03/12] drivers: Add boot constraints core
  2017-12-13  9:42   ` Greg Kroah-Hartman
@ 2017-12-13 10:00     ` Viresh Kumar
  0 siblings, 0 replies; 40+ messages in thread
From: Viresh Kumar @ 2017-12-13 10:00 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt

On 13-12-17, 10:42, Greg Kroah-Hartman wrote:
> On Sun, Oct 29, 2017 at 07:18:51PM +0530, Viresh Kumar wrote:
> > Some devices are powered ON by the bootloader before the bootloader
> > handovers control to Linux. It maybe important for those devices to keep
> > working until the time a Linux device driver probes the device and
> > reconfigure its resources.
> > 
> > A typical example of that can be the LCD controller, which is used by
> > the bootloaders to show image(s) while the platform is booting into
> > Linux. The LCD controller can be using some resources, like clk,
> > regulators, PM domain, etc, that are shared between several devices.
> > These shared resources should be configured to satisfy need of all the
> > users. If another device's (X) driver gets probed before the LCD
> > controller driver in this case, then it may end up reconfiguring these
> > resources to ranges satisfying the current users (only device X) and
> > that can make the LCD screen unstable.
> > 
> > This patch introduces the concept of boot-constraints, which will be set
> > by the bootloaders and the kernel will satisfy them until the time
> > driver for such a device is probed (successfully or unsuccessfully).
> > 
> > The list of boot constraint types is empty for now, and will be
> > incrementally updated by later patches.
> > 
> > Only two routines are exposed by the boot constraints core for now:
> 
> I think we need some documentation somewhere on how to use this, right?

Will add that in next version.

-- 
viresh

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 10/12] boot_constraint: Add support for Hisilicon platforms
  2017-12-13  9:47   ` Greg Kroah-Hartman
@ 2017-12-13 10:13     ` Viresh Kumar
  0 siblings, 0 replies; 40+ messages in thread
From: Viresh Kumar @ 2017-12-13 10:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt

On 13-12-17, 10:47, Greg Kroah-Hartman wrote:
> On Sun, Oct 29, 2017 at 07:18:58PM +0530, Viresh Kumar wrote:
> > +static const struct of_device_id machines[] __initconst = {
> > +	{ .compatible = "hisilicon,hi3660", .data = &hikey3660_constraints },
> > +	{ .compatible = "hisilicon,hi3798cv200", .data = &hikey3798cv200_constraints },
> > +	{ .compatible = "hisilicon,hi6220", .data = &hikey6220_constraints },
> > +	{ }
> > +};
> > +
> > +static int __init hikey_constraints_init(void)
> > +{
> > +	const struct hikey_machine_constraints *constraints;
> > +	const struct of_device_id *match;
> > +	struct device_node *np;
> > +
> > +	if (!boot_constraint_earlycon_enabled())
> > +		return 0;
> > +
> > +	np = of_find_node_by_path("/");
> 
> What is this for?

We need to match the above list of "machines" with the root node and "np" here
points to the root node.. and ...

> > +	if (!np)
> > +		return -ENODEV;
> > +
> > +	match = of_match_node(machines, np);

Its used here.

> > +	of_node_put(np);

> > +/*
> > + * The amba-pl011 driver registers itself from arch_initcall level. Setup the
> > + * serial boot constraints before that in order not to miss any boot messages.
> > + */
> > +postcore_initcall_sync(hikey_constraints_init);
> 
> Now you have to worry about the bootconstraints earlycon being called
> before/after your code.

For boot-constraints to work for any device, it is extremely important to add
the constraint before the device is probed by its driver, otherwise the driver
would end up re-configuring the resources. There is no other way then having
this order dependency here.

> That's another linking order dependancy you
> just created.  It feels more complex for something so "simple" as
> looking for the earlycon flag...

-- 
viresh

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 08/12] boot_constraint: Manage deferrable constraints
  2017-12-13  9:53   ` Greg Kroah-Hartman
@ 2017-12-13 10:27     ` Viresh Kumar
  2017-12-13 10:33       ` Russell King - ARM Linux
  0 siblings, 1 reply; 40+ messages in thread
From: Viresh Kumar @ 2017-12-13 10:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt

On 13-12-17, 10:53, Greg Kroah-Hartman wrote:
> On Sun, Oct 29, 2017 at 07:18:56PM +0530, Viresh Kumar wrote:
> > +static void add_deferrable_of_single(struct device_node *np,
> > +				     struct dev_boot_constraint *constraints,
> > +				     int count)
> > +{
> > +	struct device *dev;
> > +	int ret;
> > +
> > +	if (!of_device_is_available(np))
> > +		return;
> > +
> > +	ret = of_platform_bus_create(np, NULL, NULL, NULL, false);
> > +	if (ret)
> > +		return;
> > +
> > +	if (of_device_is_compatible(np, "arm,primecell")) {
> 
> Why is "arm,primecell" in the core code here?

All we need here is a struct device pointer to add constraints. But how we get
the device node depends on what bus type the device corresponds to. Currently
this only support amba and platform devices, but we may need to get spi, i2c,
etc later on.

How do you suggest to keep this stuff out of core here ? Are you asking me to
add a generic API in the OF core to find the struct device pointer using a node
pointer ?

> > +		struct amba_device *adev = of_find_amba_device_by_node(np);
> > +
> > +		if (!adev) {
> > +			pr_err("Failed to find amba dev: %s\n", np->full_name);
> 
> Never use pr_* when you have a valid struct device to use. 

Sure. I agree.

> Don't you
> have one from the struct device_node * passed in here?

The struct device_node doesn't contain a struct device * unfortunately. Will it
be acceptable to add one ? That will solve some controversial part of this
function for sure :)

-- 
viresh

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 00/12] drivers: Boot Constraints core
  2017-12-13  9:55   ` Greg Kroah-Hartman
@ 2017-12-13 10:27     ` Viresh Kumar
  0 siblings, 0 replies; 40+ messages in thread
From: Viresh Kumar @ 2017-12-13 10:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt, devicetree, Frank Rowand

On 13-12-17, 10:55, Greg Kroah-Hartman wrote:
> On Tue, Nov 28, 2017 at 09:48:18AM +0530, Viresh Kumar wrote:
> > On 29-10-17, 19:18, Viresh Kumar wrote:
> > > Here is V4 of the boot constraints core based on your feedback from V3.
> > > We now have support for three platforms (as you suggested) included in
> > > this series: Hisilicon, IMX and Qualcomm.
> > 
> > Hi Greg,
> > 
> > I was waiting for rc1 to come out before sending a reminder for this
> > series and so here is one :)
> 
> I've reviewed it enough for now, needs a tiny bit of work, but looking
> much better, nice job!

Thanks a lot for finding time to get this reviewed. Really appreciate that.

-- 
viresh

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 08/12] boot_constraint: Manage deferrable constraints
  2017-12-13 10:27     ` Viresh Kumar
@ 2017-12-13 10:33       ` Russell King - ARM Linux
  2017-12-13 14:39         ` Viresh Kumar
  0 siblings, 1 reply; 40+ messages in thread
From: Russell King - ARM Linux @ 2017-12-13 10:33 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Greg Kroah-Hartman, nm, Rajendra Nayak, s.hauer, Stephen Boyd,
	linux-kernel, xuwei5, robdclark, robh+dt, fabio.estevam,
	Vincent Guittot, shawnguo, linux-arm-kernel, l.stach

On Wed, Dec 13, 2017 at 03:57:07PM +0530, Viresh Kumar wrote:
> On 13-12-17, 10:53, Greg Kroah-Hartman wrote:
> > On Sun, Oct 29, 2017 at 07:18:56PM +0530, Viresh Kumar wrote:
> > > +static void add_deferrable_of_single(struct device_node *np,
> > > +				     struct dev_boot_constraint *constraints,
> > > +				     int count)
> > > +{
> > > +	struct device *dev;
> > > +	int ret;
> > > +
> > > +	if (!of_device_is_available(np))
> > > +		return;
> > > +
> > > +	ret = of_platform_bus_create(np, NULL, NULL, NULL, false);
> > > +	if (ret)
> > > +		return;
> > > +
> > > +	if (of_device_is_compatible(np, "arm,primecell")) {
> > 
> > Why is "arm,primecell" in the core code here?
> 
> All we need here is a struct device pointer to add constraints. But how we get
> the device node depends on what bus type the device corresponds to. Currently
> this only support amba and platform devices, but we may need to get spi, i2c,
> etc later on.
> 
> How do you suggest to keep this stuff out of core here ? Are you asking me to
> add a generic API in the OF core to find the struct device pointer using a node
> pointer ?

Why do we need this?  Why can't we lookup the "struct device" by DT
node, and then look at the device's bus type and decide what to do
from that?

Wouldn't a better solution be to use fwnode stuff for this, and
make the bus-type handling a property of the bus type itself,
pushing the bus specific code into the bus layer?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 08/12] boot_constraint: Manage deferrable constraints
  2017-12-13 10:33       ` Russell King - ARM Linux
@ 2017-12-13 14:39         ` Viresh Kumar
  0 siblings, 0 replies; 40+ messages in thread
From: Viresh Kumar @ 2017-12-13 14:39 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Greg Kroah-Hartman, nm, Rajendra Nayak, s.hauer, Stephen Boyd,
	linux-kernel, xuwei5, robdclark, robh+dt, fabio.estevam,
	Vincent Guittot, shawnguo, linux-arm-kernel, l.stach

On 13-12-17, 10:33, Russell King - ARM Linux wrote:
> On Wed, Dec 13, 2017 at 03:57:07PM +0530, Viresh Kumar wrote:
> > On 13-12-17, 10:53, Greg Kroah-Hartman wrote:
> > > On Sun, Oct 29, 2017 at 07:18:56PM +0530, Viresh Kumar wrote:
> > > > +static void add_deferrable_of_single(struct device_node *np,
> > > > +				     struct dev_boot_constraint *constraints,
> > > > +				     int count)
> > > > +{
> > > > +	struct device *dev;
> > > > +	int ret;
> > > > +
> > > > +	if (!of_device_is_available(np))
> > > > +		return;
> > > > +
> > > > +	ret = of_platform_bus_create(np, NULL, NULL, NULL, false);
> > > > +	if (ret)
> > > > +		return;
> > > > +
> > > > +	if (of_device_is_compatible(np, "arm,primecell")) {
> > > 
> > > Why is "arm,primecell" in the core code here?
> > 
> > All we need here is a struct device pointer to add constraints. But how we get
> > the device node depends on what bus type the device corresponds to. Currently
> > this only support amba and platform devices, but we may need to get spi, i2c,
> > etc later on.
> > 
> > How do you suggest to keep this stuff out of core here ? Are you asking me to
> > add a generic API in the OF core to find the struct device pointer using a node
> > pointer ?
> 
> Why do we need this?  Why can't we lookup the "struct device" by DT
> node, and then look at the device's bus type and decide what to do
> from that?

My requirement is only to get the struct device * for the DT node and I don't
really need to get into the bus specific details at all. I was not sure if there
is a way to lookup for the "struct device" by its DT node currently and so
depended on helpers like of_find_device_by_node(). Can you please point me to
the routine (or the way we can traverse all devices) ?

> Wouldn't a better solution be to use fwnode stuff for this, and
> make the bus-type handling a property of the bus type itself,
> pushing the bus specific code into the bus layer?

As I said earlier, I don't really need to work at the bus level. I just need the
device structure and so that may not be required.

-- 
viresh

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 09/12] boot_constraint: Add earlycon helper
  2017-12-13  9:44   ` Greg Kroah-Hartman
@ 2017-12-14  5:22     ` Viresh Kumar
  0 siblings, 0 replies; 40+ messages in thread
From: Viresh Kumar @ 2017-12-14  5:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Vincent Guittot, Stephen Boyd, Rajendra Nayak, linux-kernel,
	linux-arm-kernel, robdclark, s.hauer, l.stach, shawnguo,
	fabio.estevam, nm, xuwei5, robh+dt

On 13-12-17, 10:44, Greg Kroah-Hartman wrote:
> And this feels really odd, does it really save any work for the
> individual "constraint" to check for this option?  I'm all for helper
> functions, but this feels like more work than it's worth...

Ok, will move this code to individual platform driver files for now.

-- 
viresh

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2017-12-14  5:22 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
2017-10-29 13:48 ` [PATCH V4 01/12] of: platform: Add of_find_amba_device_by_node() Viresh Kumar
2017-10-29 13:48 ` [PATCH V4 02/12] of: platform: Make of_platform_bus_create() global Viresh Kumar
2017-10-29 13:48 ` [PATCH V4 03/12] drivers: Add boot constraints core Viresh Kumar
2017-12-13  9:42   ` Greg Kroah-Hartman
2017-12-13  9:59     ` Viresh Kumar
2017-12-13  9:42   ` Greg Kroah-Hartman
2017-12-13 10:00     ` Viresh Kumar
2017-10-29 13:48 ` [PATCH V4 04/12] boot_constraint: Add support for supply constraints Viresh Kumar
2017-12-13  9:48   ` Greg Kroah-Hartman
2017-10-29 13:48 ` [PATCH V4 05/12] boot_constraint: Add support for clk constraints Viresh Kumar
2017-12-13  9:48   ` Greg Kroah-Hartman
2017-10-29 13:48 ` [PATCH V4 06/12] boot_constraint: Add support for PM constraints Viresh Kumar
2017-12-13  9:48   ` Greg Kroah-Hartman
2017-10-29 13:48 ` [PATCH V4 07/12] boot_constraint: Add debugfs support Viresh Kumar
2017-10-29 15:09   ` Randy Dunlap
2017-10-30  3:37     ` Viresh Kumar
2017-10-30  3:43       ` Randy Dunlap
2017-12-13  9:50   ` Greg Kroah-Hartman
2017-10-29 13:48 ` [PATCH V4 08/12] boot_constraint: Manage deferrable constraints Viresh Kumar
2017-10-31 16:20   ` Rob Herring
2017-11-01  2:29     ` Viresh Kumar
2017-12-13  9:53   ` Greg Kroah-Hartman
2017-12-13 10:27     ` Viresh Kumar
2017-12-13 10:33       ` Russell King - ARM Linux
2017-12-13 14:39         ` Viresh Kumar
2017-10-29 13:48 ` [PATCH V4 09/12] boot_constraint: Add earlycon helper Viresh Kumar
2017-12-13  9:44   ` Greg Kroah-Hartman
2017-12-14  5:22     ` Viresh Kumar
2017-10-29 13:48 ` [PATCH V4 10/12] boot_constraint: Add support for Hisilicon platforms Viresh Kumar
2017-12-13  9:47   ` Greg Kroah-Hartman
2017-12-13 10:13     ` Viresh Kumar
2017-10-29 13:48 ` [PATCH V4 11/12] boot_constraint: Add support for IMX platform Viresh Kumar
2017-11-03  8:58   ` Sascha Hauer
2017-10-29 13:49 ` [PATCH V4 12/12] boot_constraint: Add Qualcomm display controller constraints Viresh Kumar
2017-10-30 22:07 ` [PATCH V4 00/12] drivers: Boot Constraints core Rob Herring
2017-10-31 10:01   ` Viresh Kumar
2017-11-28  4:18 ` Viresh Kumar
2017-12-13  9:55   ` Greg Kroah-Hartman
2017-12-13 10:27     ` Viresh Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).