[V3,1/8] drivers: Add boot constraints core

Message ID 1a844e27ee7e0b22acf8ea582bf4e8d35f54c84a.1501578037.git.viresh.kumar@linaro.org
State New
Headers show
Series
  • drivers: Boot Constraints core
Related show

Commit Message

Viresh Kumar Aug. 1, 2017, 9:23 a.m.
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 deffered probing.

Tested-by: Rajendra Nayak <rnayak@codeaurora.org>

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

---
 drivers/base/Kconfig                   |  10 ++
 drivers/base/Makefile                  |   1 +
 drivers/base/boot_constraints/Makefile |   3 +
 drivers/base/boot_constraints/core.c   | 199 +++++++++++++++++++++++++++++++++
 drivers/base/boot_constraints/core.h   |  33 ++++++
 drivers/base/dd.c                      |  20 ++--
 include/linux/boot_constraint.h        |  46 ++++++++
 7 files changed, 305 insertions(+), 7 deletions(-)
 create mode 100644 drivers/base/boot_constraints/Makefile
 create mode 100644 drivers/base/boot_constraints/core.c
 create mode 100644 drivers/base/boot_constraints/core.h
 create mode 100644 include/linux/boot_constraint.h

-- 
2.13.0.71.gd7076ec9c9cb

Comments

Greg KH Aug. 29, 2017, 6:39 a.m. | #1
On Tue, Aug 01, 2017 at 02:53:42PM +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 deffered probing.


How is this information getting to the kernel from the bootloader?  I
didn't see where that happened, just a single example driver that
somehow "knew" what had to happen, which seems odd...

This is a lot of new code for no users, I would like to see at least 3
real drivers that are using it before we merge it, as then you have a
chance of getting the user/kernel api correct.

thanks,

greg k-h
Viresh Kumar Aug. 29, 2017, 9:52 a.m. | #2
On 29-08-17, 08:39, Greg Kroah-Hartman wrote:
> How is this information getting to the kernel from the bootloader?  I

> didn't see where that happened, just a single example driver that

> somehow "knew" what had to happen, which seems odd...


I tried to do it with DT earlier, but we couldn't reach to an agreement on what
bindings to add and so this series started doing things the old way. The kernel
would have platform specific drivers, which would exactly know what constraints
to set and we wouldn't need the bootloaders to pass anything for now.

> This is a lot of new code for no users,


I agree and so added the patch 8/8 to show a real user. I will convert that to a
patch going forward, which can be merged along with this series.

> I would like to see at least 3

> real drivers that are using it before we merge it, as then you have a

> chance of getting the user/kernel api correct.


Hmm, so I am quite sure that this is a fairly generic problem to solve,
specifically for all the handheld devices. I can get code for few of the Qcom
SoCs but they may end up using the same platform driver. Not sure if I can get
code for multiple SoC families in the beginning.

-- 
viresh
Greg KH Aug. 29, 2017, 12:03 p.m. | #3
On Tue, Aug 29, 2017 at 11:52:17AM +0200, Viresh Kumar wrote:
> On 29-08-17, 08:39, Greg Kroah-Hartman wrote:

> > How is this information getting to the kernel from the bootloader?  I

> > didn't see where that happened, just a single example driver that

> > somehow "knew" what had to happen, which seems odd...

> 

> I tried to do it with DT earlier, but we couldn't reach to an agreement on what

> bindings to add and so this series started doing things the old way. The kernel

> would have platform specific drivers, which would exactly know what constraints

> to set and we wouldn't need the bootloaders to pass anything for now.


Who couldn't reach an agreement?  So you gave up and decided to make a
whole bunch of kernel code instead of just using new DT entries?  That's
crazy...

> > This is a lot of new code for no users,

> 

> I agree and so added the patch 8/8 to show a real user. I will convert that to a

> patch going forward, which can be merged along with this series.

> 

> > I would like to see at least 3

> > real drivers that are using it before we merge it, as then you have a

> > chance of getting the user/kernel api correct.

> 

> Hmm, so I am quite sure that this is a fairly generic problem to solve,

> specifically for all the handheld devices. I can get code for few of the Qcom

> SoCs but they may end up using the same platform driver. Not sure if I can get

> code for multiple SoC families in the beginning.


Let's see a working system or two first here please.

But you are implying that existing handheld devices need this problem
solved, how do they do it today without this code as obviously they are
shipping working solutions.

thanks,

greg k-h
Viresh Kumar Sept. 4, 2017, 9:15 a.m. | #4
On 29-08-17, 14:03, Greg Kroah-Hartman wrote:
> Who couldn't reach an agreement?


Rob Herring (DT Maintainer) didn't like the first set of bindings and wasn't
convinced that we need any new bindings for this purpose to begin with.

> So you gave up and decided to make a

> whole bunch of kernel code instead of just using new DT entries?  That's

> crazy...


Its not a lot really. Most of the code is anyways required, the only extra part
is the platform specific drivers, which are replacing what the DT would have
done. So, it shouldn't be that big of a deal I suppose.

> Let's see a working system or two first here please.


Sure, I will get as many converted as possible.

> But you are implying that existing handheld devices need this problem

> solved, how do they do it today without this code as obviously they are

> shipping working solutions.


So yeah, LCD is a common usecase but the configurations aren't always shared. It
may not be an issue with private clock/regulator resources, but with shared
ones.

Though even with the private resources, we may want the clock/domains/regulators
to stay powered on and I assume that the platforms would be doing hacky stuff to
get that all working right now.

-- 
viresh
Greg KH Sept. 4, 2017, 9:38 a.m. | #5
On Mon, Sep 04, 2017 at 11:15:47AM +0200, Viresh Kumar wrote:
> On 29-08-17, 14:03, Greg Kroah-Hartman wrote:

> > Who couldn't reach an agreement?

> 

> Rob Herring (DT Maintainer) didn't like the first set of bindings and wasn't

> convinced that we need any new bindings for this purpose to begin with.


So Rob wanted more kernel code and less bindings?
Ok, I'll ask for Rob to sign off on these patches then :)

> > But you are implying that existing handheld devices need this problem

> > solved, how do they do it today without this code as obviously they are

> > shipping working solutions.

> 

> So yeah, LCD is a common usecase but the configurations aren't always shared. It

> may not be an issue with private clock/regulator resources, but with shared

> ones.

> 

> Though even with the private resources, we may want the clock/domains/regulators

> to stay powered on and I assume that the platforms would be doing hacky stuff to

> get that all working right now.


As you have the source for a number of such systems, you might want to
verify this please.  Also, what is going to cause any new systems to use
this new api?

thanks,

greg k-h
Viresh Kumar Sept. 19, 2017, 10:46 p.m. | #6
On 04-09-17, 11:38, Greg Kroah-Hartman wrote:
> On Mon, Sep 04, 2017 at 11:15:47AM +0200, Viresh Kumar wrote:

> > On 29-08-17, 14:03, Greg Kroah-Hartman wrote:

> > > Who couldn't reach an agreement?

> > 

> > Rob Herring (DT Maintainer) didn't like the first set of bindings and wasn't

> > convinced that we need any new bindings for this purpose to begin with.

> 

> So Rob wanted more kernel code and less bindings?


We had a similar thread sometime back (Performance states for power
domains), where we couldn't get to the bindings in the first go. Rob
suggested that we can actually get the code in first and then we can
look at how to get stuff from DT once we understand it more.

He wasn't happy with the bindings I proposed and so I took the same
approach here. Its not a lot of code we are going to have to remove
once we move to DT, so it ain't that bad.

> Ok, I'll ask for Rob to sign off on these patches then :)


I can't give any guarantee of that ;)

> As you have the source for a number of such systems, you might want to

> verify this please.


I will try to talk to few guys during Linaro connect next week and
also look into source for such stuff.

> Also, what is going to cause any new systems to use

> this new api?


As far as my part go, I can do my best to advertise it with articles
and presentations. But eventually people have to get motivated to use
it themselves :)

What else can I do as an author here? I will do whatever it takes to
get people to use it.

-- 
viresh

Patch

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index f046d21de57d..2333db2a33b7 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -347,4 +347,14 @@  config GENERIC_ARCH_TOPOLOGY
 	  appropriate scaling, sysfs interface for changing capacity values at
 	  runtime.
 
+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.
+
 endmenu
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 397e5c344e6a..8c37ca07114b 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -5,6 +5,7 @@  obj-y			:= component.o core.o bus.o dd.o syscore.o \
 			   cpu.o firmware.o init.o map.o devres.o \
 			   attribute_container.o transport_class.o \
 			   topology.o container.o property.o cacheinfo.o
+obj-$(CONFIG_DEV_BOOT_CONSTRAINTS) += boot_constraints/
 obj-$(CONFIG_DEVTMPFS)	+= devtmpfs.o
 obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
 obj-y			+= power/
diff --git a/drivers/base/boot_constraints/Makefile b/drivers/base/boot_constraints/Makefile
new file mode 100644
index 000000000000..0f2680177974
--- /dev/null
+++ b/drivers/base/boot_constraints/Makefile
@@ -0,0 +1,3 @@ 
+# Makefile for device boot constraints
+
+obj-y := core.o
diff --git a/drivers/base/boot_constraints/core.c b/drivers/base/boot_constraints/core.c
new file mode 100644
index 000000000000..366a05d6d9ba
--- /dev/null
+++ b/drivers/base/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/base/boot_constraints/core.h b/drivers/base/boot_constraints/core.h
new file mode 100644
index 000000000000..7ba4ac172c09
--- /dev/null
+++ b/drivers/base/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/drivers/base/dd.c b/drivers/base/dd.c
index c17fefc77345..2262a4a4c0e4 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>
@@ -383,15 +384,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/include/linux/boot_constraint.h b/include/linux/boot_constraint.h
new file mode 100644
index 000000000000..ae34fee547c5
--- /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 */