[PATCH V4 00/12] drivers: Boot Constraints core

[PATCH V4 00/12] drivers: Boot Constraints core

[Viresh Kumar]

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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V4 01/12] of: platform: Add of_find_amba_device_by_node()

[Viresh Kumar]

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

[PATCH V4 02/12] of: platform: Make of_platform_bus_create() global

[Viresh Kumar]

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

Re: [PATCH V4 00/12] drivers: Boot Constraints core

[Rob Herring]

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
>

Re: [PATCH V4 00/12] drivers: Boot Constraints core

[Viresh Kumar]

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

Re: [PATCH V4 00/12] drivers: Boot Constraints core

[Rob Herring]

On Tue, Oct 31, 2017 at 5:01 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> 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

Re: [PATCH V4 00/12] drivers: Boot Constraints core

[Viresh Kumar]

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4 00/12] drivers: Boot Constraints core

[Greg Kroah-Hartman]

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

Re: [PATCH V4 00/12] drivers: Boot Constraints core

[Viresh Kumar]

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html