All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/8] Make deferring probe forever optional
@ 2018-05-24 17:50 ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel

This series came out of a discussion on the ARM boot-architecture
list[1] about DT forwards and backwards compatibility issues. There are
issues with newer DTs breaking on older, stable kernels. Some of these
are difficult to solve, but cases of optional devices not having
kernel support should be solvable.

I tested this on a RPi3 B with the pinctrl driver forced off. With this
change, the MMC/SD and UART drivers can function without the pinctrl
driver.

v2:
 - Add a DT property for pinctrl to flag using defaults
 - Add a debug timeout to stop deferring some number of seconds after
   initcalls are done (giving modules a chance to load)
 - Split pinctrl support to its own patch
 - WARN when we stop deferring probe for a device
 - Add IOMMU support
 - Add PM domain support

Rob

[1] https://lists.linaro.org/pipermail/boot-architecture/2018-April/000466.html


Rob Herring (8):
  driver core: make deferring probe after init optional
  driver core: add a deferred probe timeout
  dt-bindings: pinctrl: add a 'pinctrl-use-default' property
  arm: dts: bcm283x: mark the UART pin muxing nodes with
    pinctrl-use-default
  pinctrl: optionally stop deferring probe at end of initcalls
  iommu: Stop deferring probe at end of initcalls
  iommu: Remove IOMMU_OF_DECLARE
  PM / Domains: Stop deferring probe at the end of initcall

 .../admin-guide/kernel-parameters.txt         |  7 +++
 .../bindings/pinctrl/pinctrl-bindings.txt     |  6 +++
 arch/arm/boot/dts/bcm283x.dtsi                |  2 +
 drivers/base/dd.c                             | 43 +++++++++++++++++++
 drivers/base/power/domain.c                   |  2 +-
 drivers/iommu/arm-smmu-v3.c                   |  2 -
 drivers/iommu/arm-smmu.c                      |  7 ---
 drivers/iommu/exynos-iommu.c                  |  2 -
 drivers/iommu/ipmmu-vmsa.c                    |  3 --
 drivers/iommu/msm_iommu.c                     |  2 -
 drivers/iommu/of_iommu.c                      | 21 +--------
 drivers/iommu/qcom_iommu.c                    |  2 -
 drivers/iommu/rockchip-iommu.c                |  2 -
 drivers/pinctrl/devicetree.c                  | 14 ++++--
 include/linux/device.h                        |  2 +
 include/linux/of_iommu.h                      |  4 --
 16 files changed, 73 insertions(+), 48 deletions(-)

--
2.17.0

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

* [PATCH v2 0/8] Make deferring probe forever optional
@ 2018-05-24 17:50 ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

This series came out of a discussion on the ARM boot-architecture
list[1] about DT forwards and backwards compatibility issues. There are
issues with newer DTs breaking on older, stable kernels. Some of these
are difficult to solve, but cases of optional devices not having
kernel support should be solvable.

I tested this on a RPi3 B with the pinctrl driver forced off. With this
change, the MMC/SD and UART drivers can function without the pinctrl
driver.

v2:
 - Add a DT property for pinctrl to flag using defaults
 - Add a debug timeout to stop deferring some number of seconds after
   initcalls are done (giving modules a chance to load)
 - Split pinctrl support to its own patch
 - WARN when we stop deferring probe for a device
 - Add IOMMU support
 - Add PM domain support

Rob

[1] https://lists.linaro.org/pipermail/boot-architecture/2018-April/000466.html


Rob Herring (8):
  driver core: make deferring probe after init optional
  driver core: add a deferred probe timeout
  dt-bindings: pinctrl: add a 'pinctrl-use-default' property
  arm: dts: bcm283x: mark the UART pin muxing nodes with
    pinctrl-use-default
  pinctrl: optionally stop deferring probe at end of initcalls
  iommu: Stop deferring probe at end of initcalls
  iommu: Remove IOMMU_OF_DECLARE
  PM / Domains: Stop deferring probe at the end of initcall

 .../admin-guide/kernel-parameters.txt         |  7 +++
 .../bindings/pinctrl/pinctrl-bindings.txt     |  6 +++
 arch/arm/boot/dts/bcm283x.dtsi                |  2 +
 drivers/base/dd.c                             | 43 +++++++++++++++++++
 drivers/base/power/domain.c                   |  2 +-
 drivers/iommu/arm-smmu-v3.c                   |  2 -
 drivers/iommu/arm-smmu.c                      |  7 ---
 drivers/iommu/exynos-iommu.c                  |  2 -
 drivers/iommu/ipmmu-vmsa.c                    |  3 --
 drivers/iommu/msm_iommu.c                     |  2 -
 drivers/iommu/of_iommu.c                      | 21 +--------
 drivers/iommu/qcom_iommu.c                    |  2 -
 drivers/iommu/rockchip-iommu.c                |  2 -
 drivers/pinctrl/devicetree.c                  | 14 ++++--
 include/linux/device.h                        |  2 +
 include/linux/of_iommu.h                      |  4 --
 16 files changed, 73 insertions(+), 48 deletions(-)

--
2.17.0

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
       [not found] ` <20180524175024.19874-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
  2018-05-24 17:50     ` Rob Herring
@ 2018-05-24 17:50   ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel

Deferred probe will currently wait forever on dependent devices to probe,
but sometimes a driver will never exist. It's also not always critical for
a driver to exist. Platforms can rely on default configuration from the
bootloader or reset defaults for things such as pinctrl and power domains.
This is often the case with initial platform support until various drivers
get enabled. There's at least 2 scenarios where deferred probe can render
a platform broken. Both involve using a DT which has more devices and
dependencies than the kernel supports. The 1st case is a driver may be
disabled in the kernel config. The 2nd case is the kernel version may
simply not have the dependent driver. This can happen if using a newer DT
(provided by firmware perhaps) with a stable kernel version.

Subsystems or drivers may opt-in to this behavior by calling
driver_deferred_probe_check_init_done() instead of just returning
-EPROBE_DEFER. They may use additional information from DT or kernel's
config to decide whether to continue to defer probe or not.

Cc: Alexander Graf <agraf@suse.de>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/base/dd.c      | 17 +++++++++++++++++
 include/linux/device.h |  2 ++
 2 files changed, 19 insertions(+)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index c9f54089429b..d6034718da6f 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -226,6 +226,16 @@ void device_unblock_probing(void)
 	driver_deferred_probe_trigger();
 }
 
+int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
+{
+	if (optional && initcalls_done) {
+		dev_WARN(dev, "ignoring dependency for device, assuming no driver");
+		return -ENODEV;
+	}
+
+	return -EPROBE_DEFER;
+}
+
 /**
  * deferred_probe_initcall() - Enable probing of deferred devices
  *
@@ -240,6 +250,13 @@ static int deferred_probe_initcall(void)
 	/* Sort as many dependencies as possible before exiting initcalls */
 	flush_work(&deferred_probe_work);
 	initcalls_done = true;
+
+	/*
+	 * Trigger deferred probe again, this time we won't defer anything
+	 * that is optional
+	 */
+	driver_deferred_probe_trigger();
+	flush_work(&deferred_probe_work);
 	return 0;
 }
 late_initcall(deferred_probe_initcall);
diff --git a/include/linux/device.h b/include/linux/device.h
index 477956990f5e..f3dafd44c285 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -334,6 +334,8 @@ struct device *driver_find_device(struct device_driver *drv,
 				  struct device *start, void *data,
 				  int (*match)(struct device *dev, void *data));
 
+int driver_deferred_probe_check_init_done(struct device *dev, bool optional);
+
 /**
  * struct subsys_interface - interfaces to device functions
  * @name:       name of the device function
-- 
2.17.0

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-24 17:50   ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	boot-architecture-cunTk1MwBs8s++Sfvej+rw,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Deferred probe will currently wait forever on dependent devices to probe,
but sometimes a driver will never exist. It's also not always critical for
a driver to exist. Platforms can rely on default configuration from the
bootloader or reset defaults for things such as pinctrl and power domains.
This is often the case with initial platform support until various drivers
get enabled. There's at least 2 scenarios where deferred probe can render
a platform broken. Both involve using a DT which has more devices and
dependencies than the kernel supports. The 1st case is a driver may be
disabled in the kernel config. The 2nd case is the kernel version may
simply not have the dependent driver. This can happen if using a newer DT
(provided by firmware perhaps) with a stable kernel version.

Subsystems or drivers may opt-in to this behavior by calling
driver_deferred_probe_check_init_done() instead of just returning
-EPROBE_DEFER. They may use additional information from DT or kernel's
config to decide whether to continue to defer probe or not.

Cc: Alexander Graf <agraf@suse.de>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/base/dd.c      | 17 +++++++++++++++++
 include/linux/device.h |  2 ++
 2 files changed, 19 insertions(+)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index c9f54089429b..d6034718da6f 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -226,6 +226,16 @@ void device_unblock_probing(void)
 	driver_deferred_probe_trigger();
 }
 
+int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
+{
+	if (optional && initcalls_done) {
+		dev_WARN(dev, "ignoring dependency for device, assuming no driver");
+		return -ENODEV;
+	}
+
+	return -EPROBE_DEFER;
+}
+
 /**
  * deferred_probe_initcall() - Enable probing of deferred devices
  *
@@ -240,6 +250,13 @@ static int deferred_probe_initcall(void)
 	/* Sort as many dependencies as possible before exiting initcalls */
 	flush_work(&deferred_probe_work);
 	initcalls_done = true;
+
+	/*
+	 * Trigger deferred probe again, this time we won't defer anything
+	 * that is optional
+	 */
+	driver_deferred_probe_trigger();
+	flush_work(&deferred_probe_work);
 	return 0;
 }
 late_initcall(deferred_probe_initcall);
diff --git a/include/linux/device.h b/include/linux/device.h
index 477956990f5e..f3dafd44c285 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -334,6 +334,8 @@ struct device *driver_find_device(struct device_driver *drv,
 				  struct device *start, void *data,
 				  int (*match)(struct device *dev, void *data));
 
+int driver_deferred_probe_check_init_done(struct device *dev, bool optional);
+
 /**
  * struct subsys_interface - interfaces to device functions
  * @name:       name of the device function
-- 
2.17.0

_______________________________________________
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-24 17:50   ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

Deferred probe will currently wait forever on dependent devices to probe,
but sometimes a driver will never exist. It's also not always critical for
a driver to exist. Platforms can rely on default configuration from the
bootloader or reset defaults for things such as pinctrl and power domains.
This is often the case with initial platform support until various drivers
get enabled. There's at least 2 scenarios where deferred probe can render
a platform broken. Both involve using a DT which has more devices and
dependencies than the kernel supports. The 1st case is a driver may be
disabled in the kernel config. The 2nd case is the kernel version may
simply not have the dependent driver. This can happen if using a newer DT
(provided by firmware perhaps) with a stable kernel version.

Subsystems or drivers may opt-in to this behavior by calling
driver_deferred_probe_check_init_done() instead of just returning
-EPROBE_DEFER. They may use additional information from DT or kernel's
config to decide whether to continue to defer probe or not.

Cc: Alexander Graf <agraf@suse.de>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/base/dd.c      | 17 +++++++++++++++++
 include/linux/device.h |  2 ++
 2 files changed, 19 insertions(+)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index c9f54089429b..d6034718da6f 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -226,6 +226,16 @@ void device_unblock_probing(void)
 	driver_deferred_probe_trigger();
 }
 
+int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
+{
+	if (optional && initcalls_done) {
+		dev_WARN(dev, "ignoring dependency for device, assuming no driver");
+		return -ENODEV;
+	}
+
+	return -EPROBE_DEFER;
+}
+
 /**
  * deferred_probe_initcall() - Enable probing of deferred devices
  *
@@ -240,6 +250,13 @@ static int deferred_probe_initcall(void)
 	/* Sort as many dependencies as possible before exiting initcalls */
 	flush_work(&deferred_probe_work);
 	initcalls_done = true;
+
+	/*
+	 * Trigger deferred probe again, this time we won't defer anything
+	 * that is optional
+	 */
+	driver_deferred_probe_trigger();
+	flush_work(&deferred_probe_work);
 	return 0;
 }
 late_initcall(deferred_probe_initcall);
diff --git a/include/linux/device.h b/include/linux/device.h
index 477956990f5e..f3dafd44c285 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -334,6 +334,8 @@ struct device *driver_find_device(struct device_driver *drv,
 				  struct device *start, void *data,
 				  int (*match)(struct device *dev, void *data));
 
+int driver_deferred_probe_check_init_done(struct device *dev, bool optional);
+
 /**
  * struct subsys_interface - interfaces to device functions
  * @name:       name of the device function
-- 
2.17.0

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

* [PATCH v2 2/8] driver core: add a deferred probe timeout
  2018-05-24 17:50 ` Rob Herring
@ 2018-05-24 17:50   ` Rob Herring
  -1 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel

Deferring probe can wait forever on dependencies that may never appear
for a variety of reasons. This can be difficult to debug especially if
the console has dependencies or userspace fails to boot to a shell. Add
a timeout to retry probing without possibly optional dependencies and to
dump out the deferred probe pending list after retrying.

This mechanism is intended for debug purposes. It won't work for the
console which needs to be enabled before userspace starts. However, if
the console's dependencies are resolved, then the kernel log will be
printed (as opposed to no output).

Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../admin-guide/kernel-parameters.txt         |  7 +++++
 drivers/base/dd.c                             | 28 ++++++++++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 11fc28ecdb6d..dd3f40b34a24 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -809,6 +809,13 @@
 			Defaults to the default architecture's huge page size
 			if not specified.
 
+	deferred_probe_timeout=
+			[KNL] Set a timeout in seconds for deferred probe to
+			give up waiting on dependencies to probe. Only specific
+			dependencies (subsystems or drivers) that have opted in
+			will be ignored. This option also dumps out devices
+			still on the deferred probe list after retrying.
+
 	dhash_entries=	[KNL]
 			Set number of hash buckets for dentry cache.
 
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index d6034718da6f..4133b240c7e4 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -226,9 +226,17 @@ void device_unblock_probing(void)
 	driver_deferred_probe_trigger();
 }
 
+static int deferred_probe_timeout = -1;
+static int __init deferred_probe_timeout_setup(char *str)
+{
+	deferred_probe_timeout = simple_strtol(str, NULL, 0);
+	return 1;
+}
+__setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
+
 int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
 {
-	if (optional && initcalls_done) {
+	if ((optional || !deferred_probe_timeout) && initcalls_done) {
 		dev_WARN(dev, "ignoring dependency for device, assuming no driver");
 		return -ENODEV;
 	}
@@ -236,6 +244,19 @@ int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
 	return -EPROBE_DEFER;
 }
 
+static void deferred_probe_timeout_work_func(struct work_struct *work)
+{
+	struct device_private *private, *p;
+
+	deferred_probe_timeout = 0;
+	driver_deferred_probe_trigger();
+	flush_work(&deferred_probe_work);
+
+	list_for_each_entry_safe(private, p, &deferred_probe_pending_list, deferred_probe)
+		dev_info(private->device, "deferred probe pending");
+}
+static DECLARE_DELAYED_WORK(deferred_probe_timeout_work, deferred_probe_timeout_work_func);
+
 /**
  * deferred_probe_initcall() - Enable probing of deferred devices
  *
@@ -257,6 +278,11 @@ static int deferred_probe_initcall(void)
 	 */
 	driver_deferred_probe_trigger();
 	flush_work(&deferred_probe_work);
+
+	if (deferred_probe_timeout > 0) {
+		schedule_delayed_work(&deferred_probe_timeout_work,
+			deferred_probe_timeout * HZ);
+	}
 	return 0;
 }
 late_initcall(deferred_probe_initcall);
-- 
2.17.0

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

* [PATCH v2 2/8] driver core: add a deferred probe timeout
@ 2018-05-24 17:50   ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

Deferring probe can wait forever on dependencies that may never appear
for a variety of reasons. This can be difficult to debug especially if
the console has dependencies or userspace fails to boot to a shell. Add
a timeout to retry probing without possibly optional dependencies and to
dump out the deferred probe pending list after retrying.

This mechanism is intended for debug purposes. It won't work for the
console which needs to be enabled before userspace starts. However, if
the console's dependencies are resolved, then the kernel log will be
printed (as opposed to no output).

Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../admin-guide/kernel-parameters.txt         |  7 +++++
 drivers/base/dd.c                             | 28 ++++++++++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 11fc28ecdb6d..dd3f40b34a24 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -809,6 +809,13 @@
 			Defaults to the default architecture's huge page size
 			if not specified.
 
+	deferred_probe_timeout=
+			[KNL] Set a timeout in seconds for deferred probe to
+			give up waiting on dependencies to probe. Only specific
+			dependencies (subsystems or drivers) that have opted in
+			will be ignored. This option also dumps out devices
+			still on the deferred probe list after retrying.
+
 	dhash_entries=	[KNL]
 			Set number of hash buckets for dentry cache.
 
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index d6034718da6f..4133b240c7e4 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -226,9 +226,17 @@ void device_unblock_probing(void)
 	driver_deferred_probe_trigger();
 }
 
+static int deferred_probe_timeout = -1;
+static int __init deferred_probe_timeout_setup(char *str)
+{
+	deferred_probe_timeout = simple_strtol(str, NULL, 0);
+	return 1;
+}
+__setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
+
 int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
 {
-	if (optional && initcalls_done) {
+	if ((optional || !deferred_probe_timeout) && initcalls_done) {
 		dev_WARN(dev, "ignoring dependency for device, assuming no driver");
 		return -ENODEV;
 	}
@@ -236,6 +244,19 @@ int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
 	return -EPROBE_DEFER;
 }
 
+static void deferred_probe_timeout_work_func(struct work_struct *work)
+{
+	struct device_private *private, *p;
+
+	deferred_probe_timeout = 0;
+	driver_deferred_probe_trigger();
+	flush_work(&deferred_probe_work);
+
+	list_for_each_entry_safe(private, p, &deferred_probe_pending_list, deferred_probe)
+		dev_info(private->device, "deferred probe pending");
+}
+static DECLARE_DELAYED_WORK(deferred_probe_timeout_work, deferred_probe_timeout_work_func);
+
 /**
  * deferred_probe_initcall() - Enable probing of deferred devices
  *
@@ -257,6 +278,11 @@ static int deferred_probe_initcall(void)
 	 */
 	driver_deferred_probe_trigger();
 	flush_work(&deferred_probe_work);
+
+	if (deferred_probe_timeout > 0) {
+		schedule_delayed_work(&deferred_probe_timeout_work,
+			deferred_probe_timeout * HZ);
+	}
 	return 0;
 }
 late_initcall(deferred_probe_initcall);
-- 
2.17.0

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

* [PATCH v2 3/8] dt-bindings: pinctrl: add a 'pinctrl-use-default' property
  2018-05-24 17:50 ` Rob Herring
@ 2018-05-24 17:50   ` Rob Herring
  -1 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel

Pin setup may be optional in some cases such as the reset default works
or the pin setup is done by the bootloader. In these cases, it is optional
for the OS to support managing the pin controller and pin setup. In order
to support this scenario, add a property 'pinctrl-use-default' to indicate
that the pin configuration is optional.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/pinctrl/pinctrl-bindings.txt        | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
index ad9bbbba36e9..cef2b5855d60 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -103,6 +103,12 @@ Optional properties:
 #pinctrl-cells:	Number of pin control cells in addition to the index within the
 		pin controller device instance
 
+pinctrl-use-default: Boolean. Indicates that the OS can use the boot default
+		pin configuration. This allows using an OS that does not have a
+		driver for the pin controller. This property can be set either
+		globally for the pin controller or in child nodes for individual
+		pin group control.
+
 Pin controller devices should contain the pin configuration nodes that client
 devices reference.
 
-- 
2.17.0

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

* [PATCH v2 3/8] dt-bindings: pinctrl: add a 'pinctrl-use-default' property
@ 2018-05-24 17:50   ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

Pin setup may be optional in some cases such as the reset default works
or the pin setup is done by the bootloader. In these cases, it is optional
for the OS to support managing the pin controller and pin setup. In order
to support this scenario, add a property 'pinctrl-use-default' to indicate
that the pin configuration is optional.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/pinctrl/pinctrl-bindings.txt        | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
index ad9bbbba36e9..cef2b5855d60 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -103,6 +103,12 @@ Optional properties:
 #pinctrl-cells:	Number of pin control cells in addition to the index within the
 		pin controller device instance
 
+pinctrl-use-default: Boolean. Indicates that the OS can use the boot default
+		pin configuration. This allows using an OS that does not have a
+		driver for the pin controller. This property can be set either
+		globally for the pin controller or in child nodes for individual
+		pin group control.
+
 Pin controller devices should contain the pin configuration nodes that client
 devices reference.
 
-- 
2.17.0

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

* [PATCH v2 4/8] arm: dts: bcm283x: mark the UART pin muxing nodes with pinctrl-use-default
  2018-05-24 17:50 ` Rob Herring
@ 2018-05-24 17:50   ` Rob Herring
  -1 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel

Signed-off-by: Rob Herring <robh@kernel.org>
---
 arch/arm/boot/dts/bcm283x.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
index ac00e730f898..c8b8ede3d273 100644
--- a/arch/arm/boot/dts/bcm283x.dtsi
+++ b/arch/arm/boot/dts/bcm283x.dtsi
@@ -321,6 +321,7 @@
 			};
 
 			uart0_gpio14: uart0_gpio14 {
+				pinctrl-use-default;
 				brcm,pins = <14 15>;
 				brcm,function = <BCM2835_FSEL_ALT0>;
 			};
@@ -353,6 +354,7 @@
 			};
 
 			uart1_gpio14: uart1_gpio14 {
+				pinctrl-use-default;
 				brcm,pins = <14 15>;
 				brcm,function = <BCM2835_FSEL_ALT5>;
 			};
-- 
2.17.0

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

* [PATCH v2 4/8] arm: dts: bcm283x: mark the UART pin muxing nodes with pinctrl-use-default
@ 2018-05-24 17:50   ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Rob Herring <robh@kernel.org>
---
 arch/arm/boot/dts/bcm283x.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
index ac00e730f898..c8b8ede3d273 100644
--- a/arch/arm/boot/dts/bcm283x.dtsi
+++ b/arch/arm/boot/dts/bcm283x.dtsi
@@ -321,6 +321,7 @@
 			};
 
 			uart0_gpio14: uart0_gpio14 {
+				pinctrl-use-default;
 				brcm,pins = <14 15>;
 				brcm,function = <BCM2835_FSEL_ALT0>;
 			};
@@ -353,6 +354,7 @@
 			};
 
 			uart1_gpio14: uart1_gpio14 {
+				pinctrl-use-default;
 				brcm,pins = <14 15>;
 				brcm,function = <BCM2835_FSEL_ALT5>;
 			};
-- 
2.17.0

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

* [PATCH v2 5/8] pinctrl: optionally stop deferring probe at end of initcalls
       [not found] ` <20180524175024.19874-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
  2018-05-24 17:50     ` Rob Herring
@ 2018-05-24 17:50   ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel

If the pinctrl node in DT indicates that pin setup is optional and the
defaults can be used with the 'pinctrl-use-default', then only defer probe
until initcalls are done. This gives platforms the option to work without
their pinctrl driver being enabled.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/pinctrl/devicetree.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index b601039d6c69..74a31074b406 100644
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -110,17 +110,23 @@ static int dt_to_map_one_config(struct pinctrl *p,
 	int ret;
 	struct pinctrl_map *map;
 	unsigned num_maps;
+	bool pctl_optional = false;
 
 	/* Find the pin controller containing np_config */
 	np_pctldev = of_node_get(np_config);
 	for (;;) {
+		if (!pctl_optional)
+			pctl_optional = of_property_read_bool(np_pctldev, "pinctrl-use-default");
+
 		np_pctldev = of_get_next_parent(np_pctldev);
 		if (!np_pctldev || of_node_is_root(np_pctldev)) {
-			dev_info(p->dev, "could not find pctldev for node %pOF, deferring probe\n",
-				np_config);
 			of_node_put(np_pctldev);
-			/* OK let's just assume this will appear later then */
-			return -EPROBE_DEFER;
+			ret = driver_deferred_probe_check_init_done(p->dev, pctl_optional);
+			if (ret == -EPROBE_DEFER)
+				/* OK let's just assume this will appear later then */
+				dev_info(p->dev, "could not find pctldev for node %pOF, deferring probe\n",
+					np_config);
+			return ret;
 		}
 		/* If we're creating a hog we can use the passed pctldev */
 		if (pctldev && (np_pctldev == p->dev->of_node))
-- 
2.17.0

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

* [PATCH v2 5/8] pinctrl: optionally stop deferring probe at end of initcalls
@ 2018-05-24 17:50   ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	boot-architecture-cunTk1MwBs8s++Sfvej+rw,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

If the pinctrl node in DT indicates that pin setup is optional and the
defaults can be used with the 'pinctrl-use-default', then only defer probe
until initcalls are done. This gives platforms the option to work without
their pinctrl driver being enabled.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/pinctrl/devicetree.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index b601039d6c69..74a31074b406 100644
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -110,17 +110,23 @@ static int dt_to_map_one_config(struct pinctrl *p,
 	int ret;
 	struct pinctrl_map *map;
 	unsigned num_maps;
+	bool pctl_optional = false;
 
 	/* Find the pin controller containing np_config */
 	np_pctldev = of_node_get(np_config);
 	for (;;) {
+		if (!pctl_optional)
+			pctl_optional = of_property_read_bool(np_pctldev, "pinctrl-use-default");
+
 		np_pctldev = of_get_next_parent(np_pctldev);
 		if (!np_pctldev || of_node_is_root(np_pctldev)) {
-			dev_info(p->dev, "could not find pctldev for node %pOF, deferring probe\n",
-				np_config);
 			of_node_put(np_pctldev);
-			/* OK let's just assume this will appear later then */
-			return -EPROBE_DEFER;
+			ret = driver_deferred_probe_check_init_done(p->dev, pctl_optional);
+			if (ret == -EPROBE_DEFER)
+				/* OK let's just assume this will appear later then */
+				dev_info(p->dev, "could not find pctldev for node %pOF, deferring probe\n",
+					np_config);
+			return ret;
 		}
 		/* If we're creating a hog we can use the passed pctldev */
 		if (pctldev && (np_pctldev == p->dev->of_node))
-- 
2.17.0

_______________________________________________
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture

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

* [PATCH v2 5/8] pinctrl: optionally stop deferring probe at end of initcalls
@ 2018-05-24 17:50   ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

If the pinctrl node in DT indicates that pin setup is optional and the
defaults can be used with the 'pinctrl-use-default', then only defer probe
until initcalls are done. This gives platforms the option to work without
their pinctrl driver being enabled.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/pinctrl/devicetree.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index b601039d6c69..74a31074b406 100644
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -110,17 +110,23 @@ static int dt_to_map_one_config(struct pinctrl *p,
 	int ret;
 	struct pinctrl_map *map;
 	unsigned num_maps;
+	bool pctl_optional = false;
 
 	/* Find the pin controller containing np_config */
 	np_pctldev = of_node_get(np_config);
 	for (;;) {
+		if (!pctl_optional)
+			pctl_optional = of_property_read_bool(np_pctldev, "pinctrl-use-default");
+
 		np_pctldev = of_get_next_parent(np_pctldev);
 		if (!np_pctldev || of_node_is_root(np_pctldev)) {
-			dev_info(p->dev, "could not find pctldev for node %pOF, deferring probe\n",
-				np_config);
 			of_node_put(np_pctldev);
-			/* OK let's just assume this will appear later then */
-			return -EPROBE_DEFER;
+			ret = driver_deferred_probe_check_init_done(p->dev, pctl_optional);
+			if (ret == -EPROBE_DEFER)
+				/* OK let's just assume this will appear later then */
+				dev_info(p->dev, "could not find pctldev for node %pOF, deferring probe\n",
+					np_config);
+			return ret;
 		}
 		/* If we're creating a hog we can use the passed pctldev */
 		if (pctldev && (np_pctldev == p->dev->of_node))
-- 
2.17.0

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

* [PATCH v2 6/8] iommu: Stop deferring probe at end of initcalls
       [not found] ` <20180524175024.19874-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
  2018-05-24 17:50     ` Rob Herring
@ 2018-05-24 17:50   ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel, iommu

The IOMMU subsystem has its own mechanism to not defer probe if driver
support is missing. Now that the driver core supports stopping deferring
probe if drivers aren't built-in (and probed), use the driver core
support so the IOMMU specific support can be removed.

Cc: Joerg Roedel <joro@8bytes.org>
Cc: iommu@lists.linux-foundation.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/iommu/of_iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 5c36a8b7656a..2aac8387717c 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -133,7 +133,7 @@ static int of_iommu_xlate(struct device *dev,
 	 * a proper probe-ordering dependency mechanism in future.
 	 */
 	if (!ops)
-		return -EPROBE_DEFER;
+		return driver_deferred_probe_check_init_done(dev, true);
 
 	return ops->of_xlate(dev, iommu_spec);
 }
-- 
2.17.0

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

* [PATCH v2 6/8] iommu: Stop deferring probe at end of initcalls
@ 2018-05-24 17:50   ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	boot-architecture-cunTk1MwBs8s++Sfvej+rw,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

The IOMMU subsystem has its own mechanism to not defer probe if driver
support is missing. Now that the driver core supports stopping deferring
probe if drivers aren't built-in (and probed), use the driver core
support so the IOMMU specific support can be removed.

Cc: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/iommu/of_iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 5c36a8b7656a..2aac8387717c 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -133,7 +133,7 @@ static int of_iommu_xlate(struct device *dev,
 	 * a proper probe-ordering dependency mechanism in future.
 	 */
 	if (!ops)
-		return -EPROBE_DEFER;
+		return driver_deferred_probe_check_init_done(dev, true);
 
 	return ops->of_xlate(dev, iommu_spec);
 }
-- 
2.17.0

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

* [PATCH v2 6/8] iommu: Stop deferring probe at end of initcalls
@ 2018-05-24 17:50   ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

The IOMMU subsystem has its own mechanism to not defer probe if driver
support is missing. Now that the driver core supports stopping deferring
probe if drivers aren't built-in (and probed), use the driver core
support so the IOMMU specific support can be removed.

Cc: Joerg Roedel <joro@8bytes.org>
Cc: iommu at lists.linux-foundation.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/iommu/of_iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 5c36a8b7656a..2aac8387717c 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -133,7 +133,7 @@ static int of_iommu_xlate(struct device *dev,
 	 * a proper probe-ordering dependency mechanism in future.
 	 */
 	if (!ops)
-		return -EPROBE_DEFER;
+		return driver_deferred_probe_check_init_done(dev, true);
 
 	return ops->of_xlate(dev, iommu_spec);
 }
-- 
2.17.0

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

* [PATCH v2 7/8] iommu: Remove IOMMU_OF_DECLARE
  2018-05-24 17:50 ` Rob Herring
  (?)
@ 2018-05-24 17:50     ` Rob Herring
  -1 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	boot-architecture-cunTk1MwBs8s++Sfvej+rw,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Heiko Stuebner,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Will Deacon,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Krzysztof Kozlowski,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Kukjin Kim,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Now that we use the driver core to stop deferred probe for missing
drivers, IOMMU_OF_DECLARE can be removed.

This is slightly less optimal than having a list of built-in drivers in
that we'll now defer probe twice before giving up. This shouldn't have a
significant impact on boot times as past discussions about deferred
probe have given no evidence of deferred probe having a substantial
impact.

Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
Cc: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
Cc: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
Cc: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Kukjin Kim <kgene-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Krzysztof Kozlowski <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Rob Clark <robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c    |  2 --
 drivers/iommu/arm-smmu.c       |  7 -------
 drivers/iommu/exynos-iommu.c   |  2 --
 drivers/iommu/ipmmu-vmsa.c     |  3 ---
 drivers/iommu/msm_iommu.c      |  2 --
 drivers/iommu/of_iommu.c       | 19 +------------------
 drivers/iommu/qcom_iommu.c     |  2 --
 drivers/iommu/rockchip-iommu.c |  2 --
 include/linux/of_iommu.h       |  4 ----
 9 files changed, 1 insertion(+), 42 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 1d647104bccc..22bdabd3d8e0 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2915,8 +2915,6 @@ static struct platform_driver arm_smmu_driver = {
 };
 module_platform_driver(arm_smmu_driver);
 
-IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3");
-
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
 MODULE_AUTHOR("Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 69e7c60792a8..9dd7cbaa3b0c 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2211,13 +2211,6 @@ static struct platform_driver arm_smmu_driver = {
 };
 module_platform_driver(arm_smmu_driver);
 
-IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1");
-IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2");
-IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400");
-IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401");
-IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500");
-IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2");
-
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
 MODULE_AUTHOR("Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 85879cfec52f..b128cb4372d3 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1390,5 +1390,3 @@ static int __init exynos_iommu_init(void)
 	return ret;
 }
 core_initcall(exynos_iommu_init);
-
-IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu");
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 40ae6e87cb88..f026aa16d5f1 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1108,9 +1108,6 @@ static void __exit ipmmu_exit(void)
 subsys_initcall(ipmmu_init);
 module_exit(ipmmu_exit);
 
-IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa");
-IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795");
-
 MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
 MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 0d3350463a3f..27377742600d 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -877,7 +877,5 @@ static void __exit msm_iommu_driver_exit(void)
 subsys_initcall(msm_iommu_driver_init);
 module_exit(msm_iommu_driver_exit);
 
-IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu");
-
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Stepan Moskovchenko <stepanm-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>");
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 2aac8387717c..1904ccf9fc4e 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -27,9 +27,6 @@
 
 #define NO_IOMMU	1
 
-static const struct of_device_id __iommu_of_table_sentinel
-	__used __section(__iommu_of_table_end);
-
 /**
  * of_get_dma_window - Parse *dma-window property and returns 0 if found.
  *
@@ -98,19 +95,6 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
 }
 EXPORT_SYMBOL_GPL(of_get_dma_window);
 
-static bool of_iommu_driver_present(struct device_node *np)
-{
-	/*
-	 * If the IOMMU still isn't ready by the time we reach init, assume
-	 * it never will be. We don't want to defer indefinitely, nor attempt
-	 * to dereference __iommu_of_table after it's been freed.
-	 */
-	if (system_state >= SYSTEM_RUNNING)
-		return false;
-
-	return of_match_node(&__iommu_of_table, np);
-}
-
 static int of_iommu_xlate(struct device *dev,
 			  struct of_phandle_args *iommu_spec)
 {
@@ -120,8 +104,7 @@ static int of_iommu_xlate(struct device *dev,
 
 	ops = iommu_ops_from_fwnode(fwnode);
 	if ((ops && !ops->of_xlate) ||
-	    !of_device_is_available(iommu_spec->np) ||
-	    (!ops && !of_iommu_driver_present(iommu_spec->np)))
+	    !of_device_is_available(iommu_spec->np))
 		return NO_IOMMU;
 
 	err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
index 65b9c99707f8..fa0f6c39a144 100644
--- a/drivers/iommu/qcom_iommu.c
+++ b/drivers/iommu/qcom_iommu.c
@@ -947,7 +947,5 @@ static void __exit qcom_iommu_exit(void)
 module_init(qcom_iommu_init);
 module_exit(qcom_iommu_exit);
 
-IOMMU_OF_DECLARE(qcom_iommu_dev, "qcom,msm-iommu-v1");
-
 MODULE_DESCRIPTION("IOMMU API for QCOM IOMMU v1 implementations");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 0468acfa131f..90d37f29c24c 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1284,8 +1284,6 @@ static int __init rk_iommu_init(void)
 }
 subsys_initcall(rk_iommu_init);
 
-IOMMU_OF_DECLARE(rk_iommu_of, "rockchip,iommu");
-
 MODULE_DESCRIPTION("IOMMU API for Rockchip");
 MODULE_AUTHOR("Simon Xue <xxm-TNX95d0MmH7DzftRWevZcw@public.gmane.org> and Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>");
 MODULE_ALIAS("platform:rockchip-iommu");
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index 4fa654e4b5a9..f3d40dd7bb66 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -32,8 +32,4 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
 
 #endif	/* CONFIG_OF_IOMMU */
 
-extern struct of_device_id __iommu_of_table;
-
-#define IOMMU_OF_DECLARE(name, compat)	OF_DECLARE_1(iommu, name, compat, NULL)
-
 #endif /* __OF_IOMMU_H */
-- 
2.17.0

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

* [PATCH v2 7/8] iommu: Remove IOMMU_OF_DECLARE
@ 2018-05-24 17:50     ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel,
	Will Deacon, Marek Szyprowski, Kukjin Kim, Krzysztof Kozlowski,
	Rob Clark, Heiko Stuebner, iommu, linux-samsung-soc,
	linux-arm-msm, linux-rockchip

Now that we use the driver core to stop deferred probe for missing
drivers, IOMMU_OF_DECLARE can be removed.

This is slightly less optimal than having a list of built-in drivers in
that we'll now defer probe twice before giving up. This shouldn't have a
significant impact on boot times as past discussions about deferred
probe have given no evidence of deferred probe having a substantial
impact.

Cc: Will Deacon <will.deacon@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Kukjin Kim <kgene@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: iommu@lists.linux-foundation.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-rockchip@lists.infradead.org
Cc: devicetree@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/iommu/arm-smmu-v3.c    |  2 --
 drivers/iommu/arm-smmu.c       |  7 -------
 drivers/iommu/exynos-iommu.c   |  2 --
 drivers/iommu/ipmmu-vmsa.c     |  3 ---
 drivers/iommu/msm_iommu.c      |  2 --
 drivers/iommu/of_iommu.c       | 19 +------------------
 drivers/iommu/qcom_iommu.c     |  2 --
 drivers/iommu/rockchip-iommu.c |  2 --
 include/linux/of_iommu.h       |  4 ----
 9 files changed, 1 insertion(+), 42 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 1d647104bccc..22bdabd3d8e0 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2915,8 +2915,6 @@ static struct platform_driver arm_smmu_driver = {
 };
 module_platform_driver(arm_smmu_driver);
 
-IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3");
-
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
 MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 69e7c60792a8..9dd7cbaa3b0c 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2211,13 +2211,6 @@ static struct platform_driver arm_smmu_driver = {
 };
 module_platform_driver(arm_smmu_driver);
 
-IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1");
-IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2");
-IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400");
-IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401");
-IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500");
-IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2");
-
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
 MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 85879cfec52f..b128cb4372d3 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1390,5 +1390,3 @@ static int __init exynos_iommu_init(void)
 	return ret;
 }
 core_initcall(exynos_iommu_init);
-
-IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu");
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 40ae6e87cb88..f026aa16d5f1 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1108,9 +1108,6 @@ static void __exit ipmmu_exit(void)
 subsys_initcall(ipmmu_init);
 module_exit(ipmmu_exit);
 
-IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa");
-IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795");
-
 MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
 MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 0d3350463a3f..27377742600d 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -877,7 +877,5 @@ static void __exit msm_iommu_driver_exit(void)
 subsys_initcall(msm_iommu_driver_init);
 module_exit(msm_iommu_driver_exit);
 
-IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu");
-
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Stepan Moskovchenko <stepanm@codeaurora.org>");
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 2aac8387717c..1904ccf9fc4e 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -27,9 +27,6 @@
 
 #define NO_IOMMU	1
 
-static const struct of_device_id __iommu_of_table_sentinel
-	__used __section(__iommu_of_table_end);
-
 /**
  * of_get_dma_window - Parse *dma-window property and returns 0 if found.
  *
@@ -98,19 +95,6 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
 }
 EXPORT_SYMBOL_GPL(of_get_dma_window);
 
-static bool of_iommu_driver_present(struct device_node *np)
-{
-	/*
-	 * If the IOMMU still isn't ready by the time we reach init, assume
-	 * it never will be. We don't want to defer indefinitely, nor attempt
-	 * to dereference __iommu_of_table after it's been freed.
-	 */
-	if (system_state >= SYSTEM_RUNNING)
-		return false;
-
-	return of_match_node(&__iommu_of_table, np);
-}
-
 static int of_iommu_xlate(struct device *dev,
 			  struct of_phandle_args *iommu_spec)
 {
@@ -120,8 +104,7 @@ static int of_iommu_xlate(struct device *dev,
 
 	ops = iommu_ops_from_fwnode(fwnode);
 	if ((ops && !ops->of_xlate) ||
-	    !of_device_is_available(iommu_spec->np) ||
-	    (!ops && !of_iommu_driver_present(iommu_spec->np)))
+	    !of_device_is_available(iommu_spec->np))
 		return NO_IOMMU;
 
 	err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
index 65b9c99707f8..fa0f6c39a144 100644
--- a/drivers/iommu/qcom_iommu.c
+++ b/drivers/iommu/qcom_iommu.c
@@ -947,7 +947,5 @@ static void __exit qcom_iommu_exit(void)
 module_init(qcom_iommu_init);
 module_exit(qcom_iommu_exit);
 
-IOMMU_OF_DECLARE(qcom_iommu_dev, "qcom,msm-iommu-v1");
-
 MODULE_DESCRIPTION("IOMMU API for QCOM IOMMU v1 implementations");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 0468acfa131f..90d37f29c24c 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1284,8 +1284,6 @@ static int __init rk_iommu_init(void)
 }
 subsys_initcall(rk_iommu_init);
 
-IOMMU_OF_DECLARE(rk_iommu_of, "rockchip,iommu");
-
 MODULE_DESCRIPTION("IOMMU API for Rockchip");
 MODULE_AUTHOR("Simon Xue <xxm@rock-chips.com> and Daniel Kurtz <djkurtz@chromium.org>");
 MODULE_ALIAS("platform:rockchip-iommu");
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index 4fa654e4b5a9..f3d40dd7bb66 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -32,8 +32,4 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
 
 #endif	/* CONFIG_OF_IOMMU */
 
-extern struct of_device_id __iommu_of_table;
-
-#define IOMMU_OF_DECLARE(name, compat)	OF_DECLARE_1(iommu, name, compat, NULL)
-
 #endif /* __OF_IOMMU_H */
-- 
2.17.0

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

* [PATCH v2 7/8] iommu: Remove IOMMU_OF_DECLARE
@ 2018-05-24 17:50     ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

Now that we use the driver core to stop deferred probe for missing
drivers, IOMMU_OF_DECLARE can be removed.

This is slightly less optimal than having a list of built-in drivers in
that we'll now defer probe twice before giving up. This shouldn't have a
significant impact on boot times as past discussions about deferred
probe have given no evidence of deferred probe having a substantial
impact.

Cc: Will Deacon <will.deacon@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Kukjin Kim <kgene@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: iommu at lists.linux-foundation.org
Cc: linux-samsung-soc at vger.kernel.org
Cc: linux-arm-msm at vger.kernel.org
Cc: linux-rockchip at lists.infradead.org
Cc: devicetree at vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/iommu/arm-smmu-v3.c    |  2 --
 drivers/iommu/arm-smmu.c       |  7 -------
 drivers/iommu/exynos-iommu.c   |  2 --
 drivers/iommu/ipmmu-vmsa.c     |  3 ---
 drivers/iommu/msm_iommu.c      |  2 --
 drivers/iommu/of_iommu.c       | 19 +------------------
 drivers/iommu/qcom_iommu.c     |  2 --
 drivers/iommu/rockchip-iommu.c |  2 --
 include/linux/of_iommu.h       |  4 ----
 9 files changed, 1 insertion(+), 42 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 1d647104bccc..22bdabd3d8e0 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2915,8 +2915,6 @@ static struct platform_driver arm_smmu_driver = {
 };
 module_platform_driver(arm_smmu_driver);
 
-IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3");
-
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
 MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 69e7c60792a8..9dd7cbaa3b0c 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2211,13 +2211,6 @@ static struct platform_driver arm_smmu_driver = {
 };
 module_platform_driver(arm_smmu_driver);
 
-IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1");
-IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2");
-IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400");
-IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401");
-IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500");
-IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2");
-
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
 MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 85879cfec52f..b128cb4372d3 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1390,5 +1390,3 @@ static int __init exynos_iommu_init(void)
 	return ret;
 }
 core_initcall(exynos_iommu_init);
-
-IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu");
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 40ae6e87cb88..f026aa16d5f1 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1108,9 +1108,6 @@ static void __exit ipmmu_exit(void)
 subsys_initcall(ipmmu_init);
 module_exit(ipmmu_exit);
 
-IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa");
-IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795");
-
 MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
 MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 0d3350463a3f..27377742600d 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -877,7 +877,5 @@ static void __exit msm_iommu_driver_exit(void)
 subsys_initcall(msm_iommu_driver_init);
 module_exit(msm_iommu_driver_exit);
 
-IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu");
-
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Stepan Moskovchenko <stepanm@codeaurora.org>");
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 2aac8387717c..1904ccf9fc4e 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -27,9 +27,6 @@
 
 #define NO_IOMMU	1
 
-static const struct of_device_id __iommu_of_table_sentinel
-	__used __section(__iommu_of_table_end);
-
 /**
  * of_get_dma_window - Parse *dma-window property and returns 0 if found.
  *
@@ -98,19 +95,6 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
 }
 EXPORT_SYMBOL_GPL(of_get_dma_window);
 
-static bool of_iommu_driver_present(struct device_node *np)
-{
-	/*
-	 * If the IOMMU still isn't ready by the time we reach init, assume
-	 * it never will be. We don't want to defer indefinitely, nor attempt
-	 * to dereference __iommu_of_table after it's been freed.
-	 */
-	if (system_state >= SYSTEM_RUNNING)
-		return false;
-
-	return of_match_node(&__iommu_of_table, np);
-}
-
 static int of_iommu_xlate(struct device *dev,
 			  struct of_phandle_args *iommu_spec)
 {
@@ -120,8 +104,7 @@ static int of_iommu_xlate(struct device *dev,
 
 	ops = iommu_ops_from_fwnode(fwnode);
 	if ((ops && !ops->of_xlate) ||
-	    !of_device_is_available(iommu_spec->np) ||
-	    (!ops && !of_iommu_driver_present(iommu_spec->np)))
+	    !of_device_is_available(iommu_spec->np))
 		return NO_IOMMU;
 
 	err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
index 65b9c99707f8..fa0f6c39a144 100644
--- a/drivers/iommu/qcom_iommu.c
+++ b/drivers/iommu/qcom_iommu.c
@@ -947,7 +947,5 @@ static void __exit qcom_iommu_exit(void)
 module_init(qcom_iommu_init);
 module_exit(qcom_iommu_exit);
 
-IOMMU_OF_DECLARE(qcom_iommu_dev, "qcom,msm-iommu-v1");
-
 MODULE_DESCRIPTION("IOMMU API for QCOM IOMMU v1 implementations");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 0468acfa131f..90d37f29c24c 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1284,8 +1284,6 @@ static int __init rk_iommu_init(void)
 }
 subsys_initcall(rk_iommu_init);
 
-IOMMU_OF_DECLARE(rk_iommu_of, "rockchip,iommu");
-
 MODULE_DESCRIPTION("IOMMU API for Rockchip");
 MODULE_AUTHOR("Simon Xue <xxm@rock-chips.com> and Daniel Kurtz <djkurtz@chromium.org>");
 MODULE_ALIAS("platform:rockchip-iommu");
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index 4fa654e4b5a9..f3d40dd7bb66 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -32,8 +32,4 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
 
 #endif	/* CONFIG_OF_IOMMU */
 
-extern struct of_device_id __iommu_of_table;
-
-#define IOMMU_OF_DECLARE(name, compat)	OF_DECLARE_1(iommu, name, compat, NULL)
-
 #endif /* __OF_IOMMU_H */
-- 
2.17.0

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

* [PATCH v2 8/8] PM / Domains: Stop deferring probe at the end of initcall
  2018-05-24 17:50 ` Rob Herring
@ 2018-05-24 17:50   ` Rob Herring
  -1 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel,
	Pavel Machek, Len Brown, linux-pm

All PM domain drivers must be built-in (at least those using DT), so
there is no point deferring probe after initcalls are done. Continuing
to defer probe may prevent booting successfully even if managing PM
domains is not required. This can happen if the user failed to enable
the driver or if power-domains are added to a platform's DT, but there
is not yet a driver (e.g. a new DTB with an old kernel).

Call the driver core function driver_deferred_probe_check_init_done()
instead of just returning -EPROBE_DEFER to stop deferring probe when
initcalls are done.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Kevin Hilman <khilman@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Len Brown <len.brown@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/base/power/domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 1ea0e2502e8e..6398cf786e6a 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2218,7 +2218,7 @@ int genpd_dev_pm_attach(struct device *dev)
 		mutex_unlock(&gpd_list_lock);
 		dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
 			__func__, PTR_ERR(pd));
-		return -EPROBE_DEFER;
+		return driver_deferred_probe_check_init_done(dev, true);
 	}
 
 	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
-- 
2.17.0

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

* [PATCH v2 8/8] PM / Domains: Stop deferring probe at the end of initcall
@ 2018-05-24 17:50   ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

All PM domain drivers must be built-in (at least those using DT), so
there is no point deferring probe after initcalls are done. Continuing
to defer probe may prevent booting successfully even if managing PM
domains is not required. This can happen if the user failed to enable
the driver or if power-domains are added to a platform's DT, but there
is not yet a driver (e.g. a new DTB with an old kernel).

Call the driver core function driver_deferred_probe_check_init_done()
instead of just returning -EPROBE_DEFER to stop deferring probe when
initcalls are done.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Kevin Hilman <khilman@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Len Brown <len.brown@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-pm at vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/base/power/domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 1ea0e2502e8e..6398cf786e6a 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2218,7 +2218,7 @@ int genpd_dev_pm_attach(struct device *dev)
 		mutex_unlock(&gpd_list_lock);
 		dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
 			__func__, PTR_ERR(pd));
-		return -EPROBE_DEFER;
+		return driver_deferred_probe_check_init_done(dev, true);
 	}
 
 	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
-- 
2.17.0

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

* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
  2018-05-24 17:50   ` Rob Herring
@ 2018-05-24 18:18     ` Mark Brown
  -1 siblings, 0 replies; 66+ messages in thread
From: Mark Brown @ 2018-05-24 18:18 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Frank Rowand, linux-kernel,
	devicetree, boot-architecture, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 598 bytes --]

On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:

> Subsystems or drivers may opt-in to this behavior by calling
> driver_deferred_probe_check_init_done() instead of just returning
> -EPROBE_DEFER. They may use additional information from DT or kernel's
> config to decide whether to continue to defer probe or not.

Should userspace have some involvement in this decision?  It knows if
it's got any intention of loading modules for example.  Kernel config
checks might be good enough, though it's going to be a pain to work out
if the relevant driver is built as a module for example.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-24 18:18     ` Mark Brown
  0 siblings, 0 replies; 66+ messages in thread
From: Mark Brown @ 2018-05-24 18:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:

> Subsystems or drivers may opt-in to this behavior by calling
> driver_deferred_probe_check_init_done() instead of just returning
> -EPROBE_DEFER. They may use additional information from DT or kernel's
> config to decide whether to continue to defer probe or not.

Should userspace have some involvement in this decision?  It knows if
it's got any intention of loading modules for example.  Kernel config
checks might be good enough, though it's going to be a pain to work out
if the relevant driver is built as a module for example.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180524/252c8b9e/attachment.sig>

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

* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
  2018-05-24 17:50   ` Rob Herring
@ 2018-05-24 18:56     ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 66+ messages in thread
From: Greg Kroah-Hartman @ 2018-05-24 18:56 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linus Walleij, Alexander Graf, Bjorn Andersson,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, Joerg Roedel,
	Robin Murphy, Mark Brown, Frank Rowand, linux-kernel, devicetree,
	boot-architecture, linux-arm-kernel

On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
> Deferred probe will currently wait forever on dependent devices to probe,
> but sometimes a driver will never exist. It's also not always critical for
> a driver to exist. Platforms can rely on default configuration from the
> bootloader or reset defaults for things such as pinctrl and power domains.
> This is often the case with initial platform support until various drivers
> get enabled. There's at least 2 scenarios where deferred probe can render
> a platform broken. Both involve using a DT which has more devices and
> dependencies than the kernel supports. The 1st case is a driver may be
> disabled in the kernel config. The 2nd case is the kernel version may
> simply not have the dependent driver. This can happen if using a newer DT
> (provided by firmware perhaps) with a stable kernel version.
> 
> Subsystems or drivers may opt-in to this behavior by calling
> driver_deferred_probe_check_init_done() instead of just returning
> -EPROBE_DEFER. They may use additional information from DT or kernel's
> config to decide whether to continue to defer probe or not.
> 
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/base/dd.c      | 17 +++++++++++++++++
>  include/linux/device.h |  2 ++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index c9f54089429b..d6034718da6f 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>  	driver_deferred_probe_trigger();
>  }
>  
> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
> +{
> +	if (optional && initcalls_done) {
> +		dev_WARN(dev, "ignoring dependency for device, assuming no driver");

You really only need dev_warn(), here, right?

thanks,

greg k-h

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-24 18:56     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 66+ messages in thread
From: Greg Kroah-Hartman @ 2018-05-24 18:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
> Deferred probe will currently wait forever on dependent devices to probe,
> but sometimes a driver will never exist. It's also not always critical for
> a driver to exist. Platforms can rely on default configuration from the
> bootloader or reset defaults for things such as pinctrl and power domains.
> This is often the case with initial platform support until various drivers
> get enabled. There's at least 2 scenarios where deferred probe can render
> a platform broken. Both involve using a DT which has more devices and
> dependencies than the kernel supports. The 1st case is a driver may be
> disabled in the kernel config. The 2nd case is the kernel version may
> simply not have the dependent driver. This can happen if using a newer DT
> (provided by firmware perhaps) with a stable kernel version.
> 
> Subsystems or drivers may opt-in to this behavior by calling
> driver_deferred_probe_check_init_done() instead of just returning
> -EPROBE_DEFER. They may use additional information from DT or kernel's
> config to decide whether to continue to defer probe or not.
> 
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/base/dd.c      | 17 +++++++++++++++++
>  include/linux/device.h |  2 ++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index c9f54089429b..d6034718da6f 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>  	driver_deferred_probe_trigger();
>  }
>  
> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
> +{
> +	if (optional && initcalls_done) {
> +		dev_WARN(dev, "ignoring dependency for device, assuming no driver");

You really only need dev_warn(), here, right?

thanks,

greg k-h

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

* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
  2018-05-24 17:50   ` Rob Herring
@ 2018-05-24 19:00     ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 66+ messages in thread
From: Greg Kroah-Hartman @ 2018-05-24 19:00 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linus Walleij, Alexander Graf, Bjorn Andersson,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, Joerg Roedel,
	Robin Murphy, Mark Brown, Frank Rowand, linux-kernel, devicetree,
	boot-architecture, linux-arm-kernel

On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
> Deferred probe will currently wait forever on dependent devices to probe,
> but sometimes a driver will never exist. It's also not always critical for
> a driver to exist. Platforms can rely on default configuration from the
> bootloader or reset defaults for things such as pinctrl and power domains.
> This is often the case with initial platform support until various drivers
> get enabled. There's at least 2 scenarios where deferred probe can render
> a platform broken. Both involve using a DT which has more devices and
> dependencies than the kernel supports. The 1st case is a driver may be
> disabled in the kernel config. The 2nd case is the kernel version may
> simply not have the dependent driver. This can happen if using a newer DT
> (provided by firmware perhaps) with a stable kernel version.
> 
> Subsystems or drivers may opt-in to this behavior by calling
> driver_deferred_probe_check_init_done() instead of just returning
> -EPROBE_DEFER. They may use additional information from DT or kernel's
> config to decide whether to continue to defer probe or not.
> 
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/base/dd.c      | 17 +++++++++++++++++
>  include/linux/device.h |  2 ++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index c9f54089429b..d6034718da6f 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>  	driver_deferred_probe_trigger();
>  }
>  
> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
> +{
> +	if (optional && initcalls_done) {

Wait, what's the "optional" mess here?

The caller knows this value, so why do you need to even pass it in here?

And bool values that are not obvious are horrid.  I had to go look this
up when reading the later patches that just passed "true" in this
variable as I had no idea what that meant.

So as-is, no, this isn't ok, sorry.

And at the least, this needs some kerneldoc to explain it :)

thanks,

greg k-h

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-24 19:00     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 66+ messages in thread
From: Greg Kroah-Hartman @ 2018-05-24 19:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
> Deferred probe will currently wait forever on dependent devices to probe,
> but sometimes a driver will never exist. It's also not always critical for
> a driver to exist. Platforms can rely on default configuration from the
> bootloader or reset defaults for things such as pinctrl and power domains.
> This is often the case with initial platform support until various drivers
> get enabled. There's at least 2 scenarios where deferred probe can render
> a platform broken. Both involve using a DT which has more devices and
> dependencies than the kernel supports. The 1st case is a driver may be
> disabled in the kernel config. The 2nd case is the kernel version may
> simply not have the dependent driver. This can happen if using a newer DT
> (provided by firmware perhaps) with a stable kernel version.
> 
> Subsystems or drivers may opt-in to this behavior by calling
> driver_deferred_probe_check_init_done() instead of just returning
> -EPROBE_DEFER. They may use additional information from DT or kernel's
> config to decide whether to continue to defer probe or not.
> 
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/base/dd.c      | 17 +++++++++++++++++
>  include/linux/device.h |  2 ++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index c9f54089429b..d6034718da6f 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>  	driver_deferred_probe_trigger();
>  }
>  
> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
> +{
> +	if (optional && initcalls_done) {

Wait, what's the "optional" mess here?

The caller knows this value, so why do you need to even pass it in here?

And bool values that are not obvious are horrid.  I had to go look this
up when reading the later patches that just passed "true" in this
variable as I had no idea what that meant.

So as-is, no, this isn't ok, sorry.

And at the least, this needs some kerneldoc to explain it :)

thanks,

greg k-h

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

* Re: [PATCH v2 2/8] driver core: add a deferred probe timeout
  2018-05-24 17:50   ` Rob Herring
@ 2018-05-24 19:01     ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 66+ messages in thread
From: Greg Kroah-Hartman @ 2018-05-24 19:01 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linus Walleij, Alexander Graf, Bjorn Andersson,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, Joerg Roedel,
	Robin Murphy, Mark Brown, Frank Rowand, linux-kernel, devicetree,
	boot-architecture, linux-arm-kernel

On Thu, May 24, 2018 at 12:50:18PM -0500, Rob Herring wrote:
> Deferring probe can wait forever on dependencies that may never appear
> for a variety of reasons. This can be difficult to debug especially if
> the console has dependencies or userspace fails to boot to a shell. Add
> a timeout to retry probing without possibly optional dependencies and to
> dump out the deferred probe pending list after retrying.
> 
> This mechanism is intended for debug purposes. It won't work for the
> console which needs to be enabled before userspace starts. However, if
> the console's dependencies are resolved, then the kernel log will be
> printed (as opposed to no output).
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  .../admin-guide/kernel-parameters.txt         |  7 +++++
>  drivers/base/dd.c                             | 28 ++++++++++++++++++-
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 11fc28ecdb6d..dd3f40b34a24 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -809,6 +809,13 @@
>  			Defaults to the default architecture's huge page size
>  			if not specified.
>  
> +	deferred_probe_timeout=
> +			[KNL] Set a timeout in seconds for deferred probe to
> +			give up waiting on dependencies to probe. Only specific
> +			dependencies (subsystems or drivers) that have opted in
> +			will be ignored. This option also dumps out devices
> +			still on the deferred probe list after retrying.

Doesn't sound like a debugging-only option.  I can see devices enabling
this when they figure out that's the only way their platform can boot :)

thanks,

greg k-h

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

* [PATCH v2 2/8] driver core: add a deferred probe timeout
@ 2018-05-24 19:01     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 66+ messages in thread
From: Greg Kroah-Hartman @ 2018-05-24 19:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 24, 2018 at 12:50:18PM -0500, Rob Herring wrote:
> Deferring probe can wait forever on dependencies that may never appear
> for a variety of reasons. This can be difficult to debug especially if
> the console has dependencies or userspace fails to boot to a shell. Add
> a timeout to retry probing without possibly optional dependencies and to
> dump out the deferred probe pending list after retrying.
> 
> This mechanism is intended for debug purposes. It won't work for the
> console which needs to be enabled before userspace starts. However, if
> the console's dependencies are resolved, then the kernel log will be
> printed (as opposed to no output).
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  .../admin-guide/kernel-parameters.txt         |  7 +++++
>  drivers/base/dd.c                             | 28 ++++++++++++++++++-
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 11fc28ecdb6d..dd3f40b34a24 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -809,6 +809,13 @@
>  			Defaults to the default architecture's huge page size
>  			if not specified.
>  
> +	deferred_probe_timeout=
> +			[KNL] Set a timeout in seconds for deferred probe to
> +			give up waiting on dependencies to probe. Only specific
> +			dependencies (subsystems or drivers) that have opted in
> +			will be ignored. This option also dumps out devices
> +			still on the deferred probe list after retrying.

Doesn't sound like a debugging-only option.  I can see devices enabling
this when they figure out that's the only way their platform can boot :)

thanks,

greg k-h

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

* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
  2018-05-24 18:56     ` Greg Kroah-Hartman
@ 2018-05-24 19:42       ` Rob Herring
  -1 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 19:42 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Linus Walleij, Alexander Graf, Bjorn Andersson,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, Joerg Roedel,
	Robin Murphy, Mark Brown, Frank Rowand, linux-kernel, devicetree,
	Architecture Mailman List,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Thu, May 24, 2018 at 1:56 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>> Deferred probe will currently wait forever on dependent devices to probe,
>> but sometimes a driver will never exist. It's also not always critical for
>> a driver to exist. Platforms can rely on default configuration from the
>> bootloader or reset defaults for things such as pinctrl and power domains.
>> This is often the case with initial platform support until various drivers
>> get enabled. There's at least 2 scenarios where deferred probe can render
>> a platform broken. Both involve using a DT which has more devices and
>> dependencies than the kernel supports. The 1st case is a driver may be
>> disabled in the kernel config. The 2nd case is the kernel version may
>> simply not have the dependent driver. This can happen if using a newer DT
>> (provided by firmware perhaps) with a stable kernel version.
>>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>>
>> Cc: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>>  drivers/base/dd.c      | 17 +++++++++++++++++
>>  include/linux/device.h |  2 ++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index c9f54089429b..d6034718da6f 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>>       driver_deferred_probe_trigger();
>>  }
>>
>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
>> +{
>> +     if (optional && initcalls_done) {
>> +             dev_WARN(dev, "ignoring dependency for device, assuming no driver");
>
> You really only need dev_warn(), here, right?

No, the screaming is on purpose.

Bjorn had concerns about debugging/supporting subtle problems that
could stem from this. Such as electrical settings not quite right that
cause intermittent errors.

Rob

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-24 19:42       ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 19:42 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 24, 2018 at 1:56 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>> Deferred probe will currently wait forever on dependent devices to probe,
>> but sometimes a driver will never exist. It's also not always critical for
>> a driver to exist. Platforms can rely on default configuration from the
>> bootloader or reset defaults for things such as pinctrl and power domains.
>> This is often the case with initial platform support until various drivers
>> get enabled. There's at least 2 scenarios where deferred probe can render
>> a platform broken. Both involve using a DT which has more devices and
>> dependencies than the kernel supports. The 1st case is a driver may be
>> disabled in the kernel config. The 2nd case is the kernel version may
>> simply not have the dependent driver. This can happen if using a newer DT
>> (provided by firmware perhaps) with a stable kernel version.
>>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>>
>> Cc: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>>  drivers/base/dd.c      | 17 +++++++++++++++++
>>  include/linux/device.h |  2 ++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index c9f54089429b..d6034718da6f 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>>       driver_deferred_probe_trigger();
>>  }
>>
>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
>> +{
>> +     if (optional && initcalls_done) {
>> +             dev_WARN(dev, "ignoring dependency for device, assuming no driver");
>
> You really only need dev_warn(), here, right?

No, the screaming is on purpose.

Bjorn had concerns about debugging/supporting subtle problems that
could stem from this. Such as electrical settings not quite right that
cause intermittent errors.

Rob

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

* Re: [PATCH v2 2/8] driver core: add a deferred probe timeout
  2018-05-24 19:01     ` Greg Kroah-Hartman
@ 2018-05-24 19:45       ` Rob Herring
  -1 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 19:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Linus Walleij, Alexander Graf, Bjorn Andersson,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, Joerg Roedel,
	Robin Murphy, Mark Brown, Frank Rowand, linux-kernel, devicetree,
	Architecture Mailman List,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Thu, May 24, 2018 at 2:01 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, May 24, 2018 at 12:50:18PM -0500, Rob Herring wrote:
>> Deferring probe can wait forever on dependencies that may never appear
>> for a variety of reasons. This can be difficult to debug especially if
>> the console has dependencies or userspace fails to boot to a shell. Add
>> a timeout to retry probing without possibly optional dependencies and to
>> dump out the deferred probe pending list after retrying.
>>
>> This mechanism is intended for debug purposes. It won't work for the
>> console which needs to be enabled before userspace starts. However, if
>> the console's dependencies are resolved, then the kernel log will be
>> printed (as opposed to no output).
>>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>>  .../admin-guide/kernel-parameters.txt         |  7 +++++
>>  drivers/base/dd.c                             | 28 ++++++++++++++++++-
>>  2 files changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 11fc28ecdb6d..dd3f40b34a24 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -809,6 +809,13 @@
>>                       Defaults to the default architecture's huge page size
>>                       if not specified.
>>
>> +     deferred_probe_timeout=
>> +                     [KNL] Set a timeout in seconds for deferred probe to
>> +                     give up waiting on dependencies to probe. Only specific
>> +                     dependencies (subsystems or drivers) that have opted in
>> +                     will be ignored. This option also dumps out devices
>> +                     still on the deferred probe list after retrying.
>
> Doesn't sound like a debugging-only option.  I can see devices enabling
> this when they figure out that's the only way their platform can boot :)

Here's some rope...

No doubt it can be abused. So are you saying don't call it a debug
option or hide it behind a config option? And for the latter, what's
one that distros don't just turn on?

Rob

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

* [PATCH v2 2/8] driver core: add a deferred probe timeout
@ 2018-05-24 19:45       ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 19:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 24, 2018 at 2:01 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, May 24, 2018 at 12:50:18PM -0500, Rob Herring wrote:
>> Deferring probe can wait forever on dependencies that may never appear
>> for a variety of reasons. This can be difficult to debug especially if
>> the console has dependencies or userspace fails to boot to a shell. Add
>> a timeout to retry probing without possibly optional dependencies and to
>> dump out the deferred probe pending list after retrying.
>>
>> This mechanism is intended for debug purposes. It won't work for the
>> console which needs to be enabled before userspace starts. However, if
>> the console's dependencies are resolved, then the kernel log will be
>> printed (as opposed to no output).
>>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>>  .../admin-guide/kernel-parameters.txt         |  7 +++++
>>  drivers/base/dd.c                             | 28 ++++++++++++++++++-
>>  2 files changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 11fc28ecdb6d..dd3f40b34a24 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -809,6 +809,13 @@
>>                       Defaults to the default architecture's huge page size
>>                       if not specified.
>>
>> +     deferred_probe_timeout=
>> +                     [KNL] Set a timeout in seconds for deferred probe to
>> +                     give up waiting on dependencies to probe. Only specific
>> +                     dependencies (subsystems or drivers) that have opted in
>> +                     will be ignored. This option also dumps out devices
>> +                     still on the deferred probe list after retrying.
>
> Doesn't sound like a debugging-only option.  I can see devices enabling
> this when they figure out that's the only way their platform can boot :)

Here's some rope...

No doubt it can be abused. So are you saying don't call it a debug
option or hide it behind a config option? And for the latter, what's
one that distros don't just turn on?

Rob

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

* Re: [PATCH v2 2/8] driver core: add a deferred probe timeout
  2018-05-24 19:45       ` Rob Herring
@ 2018-05-24 19:51         ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 66+ messages in thread
From: Greg Kroah-Hartman @ 2018-05-24 19:51 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linus Walleij, Alexander Graf, Bjorn Andersson,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, Joerg Roedel,
	Robin Murphy, Mark Brown, Frank Rowand, linux-kernel, devicetree,
	Architecture Mailman List,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Thu, May 24, 2018 at 02:45:48PM -0500, Rob Herring wrote:
> On Thu, May 24, 2018 at 2:01 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Thu, May 24, 2018 at 12:50:18PM -0500, Rob Herring wrote:
> >> Deferring probe can wait forever on dependencies that may never appear
> >> for a variety of reasons. This can be difficult to debug especially if
> >> the console has dependencies or userspace fails to boot to a shell. Add
> >> a timeout to retry probing without possibly optional dependencies and to
> >> dump out the deferred probe pending list after retrying.
> >>
> >> This mechanism is intended for debug purposes. It won't work for the
> >> console which needs to be enabled before userspace starts. However, if
> >> the console's dependencies are resolved, then the kernel log will be
> >> printed (as opposed to no output).
> >>
> >> Signed-off-by: Rob Herring <robh@kernel.org>
> >> ---
> >>  .../admin-guide/kernel-parameters.txt         |  7 +++++
> >>  drivers/base/dd.c                             | 28 ++++++++++++++++++-
> >>  2 files changed, 34 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> >> index 11fc28ecdb6d..dd3f40b34a24 100644
> >> --- a/Documentation/admin-guide/kernel-parameters.txt
> >> +++ b/Documentation/admin-guide/kernel-parameters.txt
> >> @@ -809,6 +809,13 @@
> >>                       Defaults to the default architecture's huge page size
> >>                       if not specified.
> >>
> >> +     deferred_probe_timeout=
> >> +                     [KNL] Set a timeout in seconds for deferred probe to
> >> +                     give up waiting on dependencies to probe. Only specific
> >> +                     dependencies (subsystems or drivers) that have opted in
> >> +                     will be ignored. This option also dumps out devices
> >> +                     still on the deferred probe list after retrying.
> >
> > Doesn't sound like a debugging-only option.  I can see devices enabling
> > this when they figure out that's the only way their platform can boot :)
> 
> Here's some rope...
> 
> No doubt it can be abused. So are you saying don't call it a debug
> option or hide it behind a config option? And for the latter, what's
> one that distros don't just turn on?

If it is a debug option, make it obvious it's only for debugging.  The
commit log here says it's a debug option, but your documentation does
not say that at all, and that's what people will read.

Well, they might read it, probably not.  But at least give us something
to point at when they mess things up :)

thanks,

greg k-h

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

* [PATCH v2 2/8] driver core: add a deferred probe timeout
@ 2018-05-24 19:51         ` Greg Kroah-Hartman
  0 siblings, 0 replies; 66+ messages in thread
From: Greg Kroah-Hartman @ 2018-05-24 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 24, 2018 at 02:45:48PM -0500, Rob Herring wrote:
> On Thu, May 24, 2018 at 2:01 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Thu, May 24, 2018 at 12:50:18PM -0500, Rob Herring wrote:
> >> Deferring probe can wait forever on dependencies that may never appear
> >> for a variety of reasons. This can be difficult to debug especially if
> >> the console has dependencies or userspace fails to boot to a shell. Add
> >> a timeout to retry probing without possibly optional dependencies and to
> >> dump out the deferred probe pending list after retrying.
> >>
> >> This mechanism is intended for debug purposes. It won't work for the
> >> console which needs to be enabled before userspace starts. However, if
> >> the console's dependencies are resolved, then the kernel log will be
> >> printed (as opposed to no output).
> >>
> >> Signed-off-by: Rob Herring <robh@kernel.org>
> >> ---
> >>  .../admin-guide/kernel-parameters.txt         |  7 +++++
> >>  drivers/base/dd.c                             | 28 ++++++++++++++++++-
> >>  2 files changed, 34 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> >> index 11fc28ecdb6d..dd3f40b34a24 100644
> >> --- a/Documentation/admin-guide/kernel-parameters.txt
> >> +++ b/Documentation/admin-guide/kernel-parameters.txt
> >> @@ -809,6 +809,13 @@
> >>                       Defaults to the default architecture's huge page size
> >>                       if not specified.
> >>
> >> +     deferred_probe_timeout=
> >> +                     [KNL] Set a timeout in seconds for deferred probe to
> >> +                     give up waiting on dependencies to probe. Only specific
> >> +                     dependencies (subsystems or drivers) that have opted in
> >> +                     will be ignored. This option also dumps out devices
> >> +                     still on the deferred probe list after retrying.
> >
> > Doesn't sound like a debugging-only option.  I can see devices enabling
> > this when they figure out that's the only way their platform can boot :)
> 
> Here's some rope...
> 
> No doubt it can be abused. So are you saying don't call it a debug
> option or hide it behind a config option? And for the latter, what's
> one that distros don't just turn on?

If it is a debug option, make it obvious it's only for debugging.  The
commit log here says it's a debug option, but your documentation does
not say that at all, and that's what people will read.

Well, they might read it, probably not.  But at least give us something
to point at when they mess things up :)

thanks,

greg k-h

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

* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
  2018-05-24 18:18     ` Mark Brown
@ 2018-05-24 20:25       ` Rob Herring
  -1 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 20:25 UTC (permalink / raw)
  To: Mark Brown
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Frank Rowand, linux-kernel,
	devicetree, Architecture Mailman List,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Thu, May 24, 2018 at 1:18 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>
> Should userspace have some involvement in this decision?  It knows if
> it's got any intention of loading modules for example.  Kernel config
> checks might be good enough, though it's going to be a pain to work out
> if the relevant driver is built as a module for example.

I looked into whether we could hook into uevents in some way. If we
knew when all the coldplug events had been handled, that would be
sufficient. But it doesn't look to me like we can tell when that
happens with the uevent netlink socket. I think about the only thing
we can tell is if userspace has opened a socket. I'm not all that
familiar with how the whole sequence works, so other opinions on this
would be helpful.

Also, for this to work with serial consoles, we have to make the
decision before we get to userspace. I couldn't get systemd to create
serial gettys either if we deferred later. There's some dependence on
/dev/console, but I didn't get to the bottom of it.

Rob

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-24 20:25       ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 20:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 24, 2018 at 1:18 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>
> Should userspace have some involvement in this decision?  It knows if
> it's got any intention of loading modules for example.  Kernel config
> checks might be good enough, though it's going to be a pain to work out
> if the relevant driver is built as a module for example.

I looked into whether we could hook into uevents in some way. If we
knew when all the coldplug events had been handled, that would be
sufficient. But it doesn't look to me like we can tell when that
happens with the uevent netlink socket. I think about the only thing
we can tell is if userspace has opened a socket. I'm not all that
familiar with how the whole sequence works, so other opinions on this
would be helpful.

Also, for this to work with serial consoles, we have to make the
decision before we get to userspace. I couldn't get systemd to create
serial gettys either if we deferred later. There's some dependence on
/dev/console, but I didn't get to the bottom of it.

Rob

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

* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
  2018-05-24 19:00     ` Greg Kroah-Hartman
@ 2018-05-24 20:57       ` Rob Herring
  -1 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 20:57 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Linus Walleij, Alexander Graf, Bjorn Andersson,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, Joerg Roedel,
	Robin Murphy, Mark Brown, Frank Rowand, linux-kernel, devicetree,
	Architecture Mailman List,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Thu, May 24, 2018 at 2:00 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>> Deferred probe will currently wait forever on dependent devices to probe,
>> but sometimes a driver will never exist. It's also not always critical for
>> a driver to exist. Platforms can rely on default configuration from the
>> bootloader or reset defaults for things such as pinctrl and power domains.
>> This is often the case with initial platform support until various drivers
>> get enabled. There's at least 2 scenarios where deferred probe can render
>> a platform broken. Both involve using a DT which has more devices and
>> dependencies than the kernel supports. The 1st case is a driver may be
>> disabled in the kernel config. The 2nd case is the kernel version may
>> simply not have the dependent driver. This can happen if using a newer DT
>> (provided by firmware perhaps) with a stable kernel version.
>>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>>
>> Cc: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>>  drivers/base/dd.c      | 17 +++++++++++++++++
>>  include/linux/device.h |  2 ++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index c9f54089429b..d6034718da6f 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>>       driver_deferred_probe_trigger();
>>  }
>>
>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
>> +{
>> +     if (optional && initcalls_done) {
>
> Wait, what's the "optional" mess here?

My intent was that subsystems just always call this function and never
return EPROBE_DEFER themselves. Then the driver core can make
decisions as to what to do (such as the timeout added in the next
patch). Or it can print common error/debug messages. So optional is a
hint to allow subsystems per device control.

>
> The caller knows this value, so why do you need to even pass it in here?

Because regardless of the value, we always stop deferring when/if we
hit the timeout and the caller doesn't know about the timeout. If we
get rid of it, we'd need functions for both init done and for deferred
timeout.

> And bool values that are not obvious are horrid.  I had to go look this
> up when reading the later patches that just passed "true" in this
> variable as I had no idea what that meant.

Perhaps inverting it and calling it "keep_deferring" would be better.
However, the flag is ignored if we have timed out.

>
> So as-is, no, this isn't ok, sorry.
>
> And at the least, this needs some kerneldoc to explain it :)

That part is easy enough to fix.

Rob

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-24 20:57       ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 20:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 24, 2018 at 2:00 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>> Deferred probe will currently wait forever on dependent devices to probe,
>> but sometimes a driver will never exist. It's also not always critical for
>> a driver to exist. Platforms can rely on default configuration from the
>> bootloader or reset defaults for things such as pinctrl and power domains.
>> This is often the case with initial platform support until various drivers
>> get enabled. There's at least 2 scenarios where deferred probe can render
>> a platform broken. Both involve using a DT which has more devices and
>> dependencies than the kernel supports. The 1st case is a driver may be
>> disabled in the kernel config. The 2nd case is the kernel version may
>> simply not have the dependent driver. This can happen if using a newer DT
>> (provided by firmware perhaps) with a stable kernel version.
>>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>>
>> Cc: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>>  drivers/base/dd.c      | 17 +++++++++++++++++
>>  include/linux/device.h |  2 ++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index c9f54089429b..d6034718da6f 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>>       driver_deferred_probe_trigger();
>>  }
>>
>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
>> +{
>> +     if (optional && initcalls_done) {
>
> Wait, what's the "optional" mess here?

My intent was that subsystems just always call this function and never
return EPROBE_DEFER themselves. Then the driver core can make
decisions as to what to do (such as the timeout added in the next
patch). Or it can print common error/debug messages. So optional is a
hint to allow subsystems per device control.

>
> The caller knows this value, so why do you need to even pass it in here?

Because regardless of the value, we always stop deferring when/if we
hit the timeout and the caller doesn't know about the timeout. If we
get rid of it, we'd need functions for both init done and for deferred
timeout.

> And bool values that are not obvious are horrid.  I had to go look this
> up when reading the later patches that just passed "true" in this
> variable as I had no idea what that meant.

Perhaps inverting it and calling it "keep_deferring" would be better.
However, the flag is ignored if we have timed out.

>
> So as-is, no, this isn't ok, sorry.
>
> And at the least, this needs some kerneldoc to explain it :)

That part is easy enough to fix.

Rob

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

* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
  2018-05-24 17:50   ` Rob Herring
@ 2018-05-24 22:28     ` Bjorn Andersson
  -1 siblings, 0 replies; 66+ messages in thread
From: Bjorn Andersson @ 2018-05-24 22:28 UTC (permalink / raw)
  To: Rob Herring, Arnd Bergmann
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, Joerg Roedel,
	Robin Murphy, Mark Brown, Frank Rowand, linux-kernel, devicetree,
	boot-architecture, linux-arm-kernel

On Thu 24 May 10:50 PDT 2018, Rob Herring wrote:

> Deferred probe will currently wait forever on dependent devices to probe,
> but sometimes a driver will never exist. It's also not always critical for
> a driver to exist. Platforms can rely on default configuration from the
> bootloader or reset defaults for things such as pinctrl and power domains.
> This is often the case with initial platform support until various drivers
> get enabled. There's at least 2 scenarios where deferred probe can render
> a platform broken. Both involve using a DT which has more devices and
> dependencies than the kernel supports. The 1st case is a driver may be
> disabled in the kernel config. The 2nd case is the kernel version may
> simply not have the dependent driver. This can happen if using a newer DT
> (provided by firmware perhaps) with a stable kernel version.
> 
> Subsystems or drivers may opt-in to this behavior by calling
> driver_deferred_probe_check_init_done() instead of just returning
> -EPROBE_DEFER. They may use additional information from DT or kernel's
> config to decide whether to continue to defer probe or not.
> 

For builtin drivers this still looks reasonable.

But I would like to have an additional clarification here stating that
drivers that might be targeted by this query must not be compiled as
modules.

And I would prefer to see an ack from e.g. Arnd that arm-soc is okay
that we drop "tristate" on drivers affected by this; e.g. if we put this
in the pinctrl core then all pinctrl drivers should be "bool" and so
should any i2c, ssbi and spmi buses and drivers be.

Regards,
Bjorn

> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/base/dd.c      | 17 +++++++++++++++++
>  include/linux/device.h |  2 ++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index c9f54089429b..d6034718da6f 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>  	driver_deferred_probe_trigger();
>  }
>  
> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
> +{
> +	if (optional && initcalls_done) {
> +		dev_WARN(dev, "ignoring dependency for device, assuming no driver");
> +		return -ENODEV;
> +	}
> +
> +	return -EPROBE_DEFER;
> +}
> +
>  /**
>   * deferred_probe_initcall() - Enable probing of deferred devices
>   *
> @@ -240,6 +250,13 @@ static int deferred_probe_initcall(void)
>  	/* Sort as many dependencies as possible before exiting initcalls */
>  	flush_work(&deferred_probe_work);
>  	initcalls_done = true;
> +
> +	/*
> +	 * Trigger deferred probe again, this time we won't defer anything
> +	 * that is optional
> +	 */
> +	driver_deferred_probe_trigger();
> +	flush_work(&deferred_probe_work);
>  	return 0;
>  }
>  late_initcall(deferred_probe_initcall);
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 477956990f5e..f3dafd44c285 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -334,6 +334,8 @@ struct device *driver_find_device(struct device_driver *drv,
>  				  struct device *start, void *data,
>  				  int (*match)(struct device *dev, void *data));
>  
> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional);
> +
>  /**
>   * struct subsys_interface - interfaces to device functions
>   * @name:       name of the device function
> -- 
> 2.17.0
> 

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-24 22:28     ` Bjorn Andersson
  0 siblings, 0 replies; 66+ messages in thread
From: Bjorn Andersson @ 2018-05-24 22:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu 24 May 10:50 PDT 2018, Rob Herring wrote:

> Deferred probe will currently wait forever on dependent devices to probe,
> but sometimes a driver will never exist. It's also not always critical for
> a driver to exist. Platforms can rely on default configuration from the
> bootloader or reset defaults for things such as pinctrl and power domains.
> This is often the case with initial platform support until various drivers
> get enabled. There's at least 2 scenarios where deferred probe can render
> a platform broken. Both involve using a DT which has more devices and
> dependencies than the kernel supports. The 1st case is a driver may be
> disabled in the kernel config. The 2nd case is the kernel version may
> simply not have the dependent driver. This can happen if using a newer DT
> (provided by firmware perhaps) with a stable kernel version.
> 
> Subsystems or drivers may opt-in to this behavior by calling
> driver_deferred_probe_check_init_done() instead of just returning
> -EPROBE_DEFER. They may use additional information from DT or kernel's
> config to decide whether to continue to defer probe or not.
> 

For builtin drivers this still looks reasonable.

But I would like to have an additional clarification here stating that
drivers that might be targeted by this query must not be compiled as
modules.

And I would prefer to see an ack from e.g. Arnd that arm-soc is okay
that we drop "tristate" on drivers affected by this; e.g. if we put this
in the pinctrl core then all pinctrl drivers should be "bool" and so
should any i2c, ssbi and spmi buses and drivers be.

Regards,
Bjorn

> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/base/dd.c      | 17 +++++++++++++++++
>  include/linux/device.h |  2 ++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index c9f54089429b..d6034718da6f 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>  	driver_deferred_probe_trigger();
>  }
>  
> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
> +{
> +	if (optional && initcalls_done) {
> +		dev_WARN(dev, "ignoring dependency for device, assuming no driver");
> +		return -ENODEV;
> +	}
> +
> +	return -EPROBE_DEFER;
> +}
> +
>  /**
>   * deferred_probe_initcall() - Enable probing of deferred devices
>   *
> @@ -240,6 +250,13 @@ static int deferred_probe_initcall(void)
>  	/* Sort as many dependencies as possible before exiting initcalls */
>  	flush_work(&deferred_probe_work);
>  	initcalls_done = true;
> +
> +	/*
> +	 * Trigger deferred probe again, this time we won't defer anything
> +	 * that is optional
> +	 */
> +	driver_deferred_probe_trigger();
> +	flush_work(&deferred_probe_work);
>  	return 0;
>  }
>  late_initcall(deferred_probe_initcall);
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 477956990f5e..f3dafd44c285 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -334,6 +334,8 @@ struct device *driver_find_device(struct device_driver *drv,
>  				  struct device *start, void *data,
>  				  int (*match)(struct device *dev, void *data));
>  
> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional);
> +
>  /**
>   * struct subsys_interface - interfaces to device functions
>   * @name:       name of the device function
> -- 
> 2.17.0
> 

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

* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
  2018-05-24 22:28     ` Bjorn Andersson
@ 2018-05-24 23:47       ` Rob Herring
  -1 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 23:47 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, Joerg Roedel,
	Robin Murphy, Mark Brown, Frank Rowand, linux-kernel, devicetree,
	Architecture Mailman List,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Thu, May 24, 2018 at 5:28 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Thu 24 May 10:50 PDT 2018, Rob Herring wrote:
>
>> Deferred probe will currently wait forever on dependent devices to probe,
>> but sometimes a driver will never exist. It's also not always critical for
>> a driver to exist. Platforms can rely on default configuration from the
>> bootloader or reset defaults for things such as pinctrl and power domains.
>> This is often the case with initial platform support until various drivers
>> get enabled. There's at least 2 scenarios where deferred probe can render
>> a platform broken. Both involve using a DT which has more devices and
>> dependencies than the kernel supports. The 1st case is a driver may be
>> disabled in the kernel config. The 2nd case is the kernel version may
>> simply not have the dependent driver. This can happen if using a newer DT
>> (provided by firmware perhaps) with a stable kernel version.
>>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>>
>
> For builtin drivers this still looks reasonable.
>
> But I would like to have an additional clarification here stating that
> drivers that might be targeted by this query must not be compiled as
> modules.
>
> And I would prefer to see an ack from e.g. Arnd that arm-soc is okay
> that we drop "tristate" on drivers affected by this; e.g. if we put this
> in the pinctrl core then all pinctrl drivers should be "bool" and so
> should any i2c, ssbi and spmi buses and drivers be.

For pinctrl, you are not affected now unless you change your DT. I
made pinctrl opt-in since as you said it could lead to some
intermittent problems. If iommu or power domains are required, then it
should fail pretty hard.

However, good luck getting your serial console to work if it has a
pinctrl dependency and that's a module. It won't work because the
console has to be up before userspace. Of course, you can just rely on
earlycon, but that just proves that pinctrl is not required (at least
for 1 serial pin). :)

Rob

>
> Regards,
> Bjorn
>
>> Cc: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>>  drivers/base/dd.c      | 17 +++++++++++++++++
>>  include/linux/device.h |  2 ++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index c9f54089429b..d6034718da6f 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>>       driver_deferred_probe_trigger();
>>  }
>>
>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
>> +{
>> +     if (optional && initcalls_done) {
>> +             dev_WARN(dev, "ignoring dependency for device, assuming no driver");
>> +             return -ENODEV;
>> +     }
>> +
>> +     return -EPROBE_DEFER;
>> +}
>> +
>>  /**
>>   * deferred_probe_initcall() - Enable probing of deferred devices
>>   *
>> @@ -240,6 +250,13 @@ static int deferred_probe_initcall(void)
>>       /* Sort as many dependencies as possible before exiting initcalls */
>>       flush_work(&deferred_probe_work);
>>       initcalls_done = true;
>> +
>> +     /*
>> +      * Trigger deferred probe again, this time we won't defer anything
>> +      * that is optional
>> +      */
>> +     driver_deferred_probe_trigger();
>> +     flush_work(&deferred_probe_work);
>>       return 0;
>>  }
>>  late_initcall(deferred_probe_initcall);
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index 477956990f5e..f3dafd44c285 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -334,6 +334,8 @@ struct device *driver_find_device(struct device_driver *drv,
>>                                 struct device *start, void *data,
>>                                 int (*match)(struct device *dev, void *data));
>>
>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional);
>> +
>>  /**
>>   * struct subsys_interface - interfaces to device functions
>>   * @name:       name of the device function
>> --
>> 2.17.0
>>

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-24 23:47       ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-24 23:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 24, 2018 at 5:28 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Thu 24 May 10:50 PDT 2018, Rob Herring wrote:
>
>> Deferred probe will currently wait forever on dependent devices to probe,
>> but sometimes a driver will never exist. It's also not always critical for
>> a driver to exist. Platforms can rely on default configuration from the
>> bootloader or reset defaults for things such as pinctrl and power domains.
>> This is often the case with initial platform support until various drivers
>> get enabled. There's at least 2 scenarios where deferred probe can render
>> a platform broken. Both involve using a DT which has more devices and
>> dependencies than the kernel supports. The 1st case is a driver may be
>> disabled in the kernel config. The 2nd case is the kernel version may
>> simply not have the dependent driver. This can happen if using a newer DT
>> (provided by firmware perhaps) with a stable kernel version.
>>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>>
>
> For builtin drivers this still looks reasonable.
>
> But I would like to have an additional clarification here stating that
> drivers that might be targeted by this query must not be compiled as
> modules.
>
> And I would prefer to see an ack from e.g. Arnd that arm-soc is okay
> that we drop "tristate" on drivers affected by this; e.g. if we put this
> in the pinctrl core then all pinctrl drivers should be "bool" and so
> should any i2c, ssbi and spmi buses and drivers be.

For pinctrl, you are not affected now unless you change your DT. I
made pinctrl opt-in since as you said it could lead to some
intermittent problems. If iommu or power domains are required, then it
should fail pretty hard.

However, good luck getting your serial console to work if it has a
pinctrl dependency and that's a module. It won't work because the
console has to be up before userspace. Of course, you can just rely on
earlycon, but that just proves that pinctrl is not required (at least
for 1 serial pin). :)

Rob

>
> Regards,
> Bjorn
>
>> Cc: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>>  drivers/base/dd.c      | 17 +++++++++++++++++
>>  include/linux/device.h |  2 ++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index c9f54089429b..d6034718da6f 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>>       driver_deferred_probe_trigger();
>>  }
>>
>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
>> +{
>> +     if (optional && initcalls_done) {
>> +             dev_WARN(dev, "ignoring dependency for device, assuming no driver");
>> +             return -ENODEV;
>> +     }
>> +
>> +     return -EPROBE_DEFER;
>> +}
>> +
>>  /**
>>   * deferred_probe_initcall() - Enable probing of deferred devices
>>   *
>> @@ -240,6 +250,13 @@ static int deferred_probe_initcall(void)
>>       /* Sort as many dependencies as possible before exiting initcalls */
>>       flush_work(&deferred_probe_work);
>>       initcalls_done = true;
>> +
>> +     /*
>> +      * Trigger deferred probe again, this time we won't defer anything
>> +      * that is optional
>> +      */
>> +     driver_deferred_probe_trigger();
>> +     flush_work(&deferred_probe_work);
>>       return 0;
>>  }
>>  late_initcall(deferred_probe_initcall);
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index 477956990f5e..f3dafd44c285 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -334,6 +334,8 @@ struct device *driver_find_device(struct device_driver *drv,
>>                                 struct device *start, void *data,
>>                                 int (*match)(struct device *dev, void *data));
>>
>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional);
>> +
>>  /**
>>   * struct subsys_interface - interfaces to device functions
>>   * @name:       name of the device function
>> --
>> 2.17.0
>>

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

* Re: [PATCH v2 7/8] iommu: Remove IOMMU_OF_DECLARE
  2018-05-24 17:50     ` Rob Herring
  (?)
@ 2018-05-25 11:31         ` Will Deacon
  -1 siblings, 0 replies; 66+ messages in thread
From: Will Deacon @ 2018-05-25 11:31 UTC (permalink / raw)
  To: Rob Herring
  Cc: Ulf Hansson, Heiko Stuebner, Linus Walleij, Bjorn Andersson,
	Frank Rowand, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	Kevin Hilman, Alexander Graf, Krzysztof Kozlowski,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Kukjin Kim,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	boot-architecture-cunTk1MwBs8s++Sfvej+rw,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Mark Brown,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Greg Kroah-Hartman, Rafael J. Wysocki,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

On Thu, May 24, 2018 at 12:50:23PM -0500, Rob Herring wrote:
> Now that we use the driver core to stop deferred probe for missing
> drivers, IOMMU_OF_DECLARE can be removed.
> 
> This is slightly less optimal than having a list of built-in drivers in
> that we'll now defer probe twice before giving up. This shouldn't have a
> significant impact on boot times as past discussions about deferred
> probe have given no evidence of deferred probe having a substantial
> impact.
> 
> Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> Cc: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> Cc: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
> Cc: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Kukjin Kim <kgene-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Krzysztof Kozlowski <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Rob Clark <robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu-v3.c    |  2 --
>  drivers/iommu/arm-smmu.c       |  7 -------
>  drivers/iommu/exynos-iommu.c   |  2 --
>  drivers/iommu/ipmmu-vmsa.c     |  3 ---
>  drivers/iommu/msm_iommu.c      |  2 --
>  drivers/iommu/of_iommu.c       | 19 +------------------
>  drivers/iommu/qcom_iommu.c     |  2 --
>  drivers/iommu/rockchip-iommu.c |  2 --
>  include/linux/of_iommu.h       |  4 ----
>  9 files changed, 1 insertion(+), 42 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 1d647104bccc..22bdabd3d8e0 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2915,8 +2915,6 @@ static struct platform_driver arm_smmu_driver = {
>  };
>  module_platform_driver(arm_smmu_driver);
>  
> -IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3");
> -
>  MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
>  MODULE_AUTHOR("Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>");
>  MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 69e7c60792a8..9dd7cbaa3b0c 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -2211,13 +2211,6 @@ static struct platform_driver arm_smmu_driver = {
>  };
>  module_platform_driver(arm_smmu_driver);
>  
> -IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1");
> -IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2");
> -IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400");
> -IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401");
> -IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500");
> -IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2");
> -
>  MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
>  MODULE_AUTHOR("Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>");
>  MODULE_LICENSE("GPL v2");

For the SMMU drivers:

Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>

Will

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

* Re: [PATCH v2 7/8] iommu: Remove IOMMU_OF_DECLARE
@ 2018-05-25 11:31         ` Will Deacon
  0 siblings, 0 replies; 66+ messages in thread
From: Will Deacon @ 2018-05-25 11:31 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand,
	linux-kernel, devicetree, boot-architecture, linux-arm-kernel,
	Marek Szyprowski, Kukjin Kim, Krzysztof Kozlowski, Rob Clark,
	Heiko Stuebner, iommu, linux-samsung-soc, linux-arm-msm,
	linux-rockchip

On Thu, May 24, 2018 at 12:50:23PM -0500, Rob Herring wrote:
> Now that we use the driver core to stop deferred probe for missing
> drivers, IOMMU_OF_DECLARE can be removed.
> 
> This is slightly less optimal than having a list of built-in drivers in
> that we'll now defer probe twice before giving up. This shouldn't have a
> significant impact on boot times as past discussions about deferred
> probe have given no evidence of deferred probe having a substantial
> impact.
> 
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Kukjin Kim <kgene@kernel.org>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: iommu@lists.linux-foundation.org
> Cc: linux-samsung-soc@vger.kernel.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: linux-rockchip@lists.infradead.org
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/iommu/arm-smmu-v3.c    |  2 --
>  drivers/iommu/arm-smmu.c       |  7 -------
>  drivers/iommu/exynos-iommu.c   |  2 --
>  drivers/iommu/ipmmu-vmsa.c     |  3 ---
>  drivers/iommu/msm_iommu.c      |  2 --
>  drivers/iommu/of_iommu.c       | 19 +------------------
>  drivers/iommu/qcom_iommu.c     |  2 --
>  drivers/iommu/rockchip-iommu.c |  2 --
>  include/linux/of_iommu.h       |  4 ----
>  9 files changed, 1 insertion(+), 42 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 1d647104bccc..22bdabd3d8e0 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2915,8 +2915,6 @@ static struct platform_driver arm_smmu_driver = {
>  };
>  module_platform_driver(arm_smmu_driver);
>  
> -IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3");
> -
>  MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
>  MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
>  MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 69e7c60792a8..9dd7cbaa3b0c 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -2211,13 +2211,6 @@ static struct platform_driver arm_smmu_driver = {
>  };
>  module_platform_driver(arm_smmu_driver);
>  
> -IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1");
> -IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2");
> -IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400");
> -IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401");
> -IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500");
> -IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2");
> -
>  MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
>  MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
>  MODULE_LICENSE("GPL v2");

For the SMMU drivers:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

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

* [PATCH v2 7/8] iommu: Remove IOMMU_OF_DECLARE
@ 2018-05-25 11:31         ` Will Deacon
  0 siblings, 0 replies; 66+ messages in thread
From: Will Deacon @ 2018-05-25 11:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 24, 2018 at 12:50:23PM -0500, Rob Herring wrote:
> Now that we use the driver core to stop deferred probe for missing
> drivers, IOMMU_OF_DECLARE can be removed.
> 
> This is slightly less optimal than having a list of built-in drivers in
> that we'll now defer probe twice before giving up. This shouldn't have a
> significant impact on boot times as past discussions about deferred
> probe have given no evidence of deferred probe having a substantial
> impact.
> 
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Kukjin Kim <kgene@kernel.org>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: iommu at lists.linux-foundation.org
> Cc: linux-samsung-soc at vger.kernel.org
> Cc: linux-arm-msm at vger.kernel.org
> Cc: linux-rockchip at lists.infradead.org
> Cc: devicetree at vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/iommu/arm-smmu-v3.c    |  2 --
>  drivers/iommu/arm-smmu.c       |  7 -------
>  drivers/iommu/exynos-iommu.c   |  2 --
>  drivers/iommu/ipmmu-vmsa.c     |  3 ---
>  drivers/iommu/msm_iommu.c      |  2 --
>  drivers/iommu/of_iommu.c       | 19 +------------------
>  drivers/iommu/qcom_iommu.c     |  2 --
>  drivers/iommu/rockchip-iommu.c |  2 --
>  include/linux/of_iommu.h       |  4 ----
>  9 files changed, 1 insertion(+), 42 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 1d647104bccc..22bdabd3d8e0 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2915,8 +2915,6 @@ static struct platform_driver arm_smmu_driver = {
>  };
>  module_platform_driver(arm_smmu_driver);
>  
> -IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3");
> -
>  MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
>  MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
>  MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 69e7c60792a8..9dd7cbaa3b0c 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -2211,13 +2211,6 @@ static struct platform_driver arm_smmu_driver = {
>  };
>  module_platform_driver(arm_smmu_driver);
>  
> -IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1");
> -IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2");
> -IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400");
> -IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401");
> -IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500");
> -IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2");
> -
>  MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
>  MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
>  MODULE_LICENSE("GPL v2");

For the SMMU drivers:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

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

* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
  2018-05-24 18:18     ` Mark Brown
@ 2018-05-25 11:47       ` Robin Murphy
  -1 siblings, 0 replies; 66+ messages in thread
From: Robin Murphy @ 2018-05-25 11:47 UTC (permalink / raw)
  To: Mark Brown, Rob Herring
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Frank Rowand, linux-kernel, devicetree,
	boot-architecture, linux-arm-kernel

On 24/05/18 19:18, Mark Brown wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
> 
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
> 
> Should userspace have some involvement in this decision?  It knows if
> it's got any intention of loading modules for example.  Kernel config
> checks might be good enough, though it's going to be a pain to work out
> if the relevant driver is built as a module for example.

Arguably userspace has some control over that already, as in many cases 
it can just unbind and reprobe the consumer driver after loading the 
provider driver (in my silly IOMMU-drivers-as-modules PoC a while ago I 
was delighted to find that it can really be that simple). It's a bit 
harder when the device is the primary console or root filesystem, but I 
think that's effectively just another variant of the "defer until a 
module is loaded" chicken-and-egg problem.

Robin.

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-25 11:47       ` Robin Murphy
  0 siblings, 0 replies; 66+ messages in thread
From: Robin Murphy @ 2018-05-25 11:47 UTC (permalink / raw)
  To: linux-arm-kernel

On 24/05/18 19:18, Mark Brown wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
> 
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
> 
> Should userspace have some involvement in this decision?  It knows if
> it's got any intention of loading modules for example.  Kernel config
> checks might be good enough, though it's going to be a pain to work out
> if the relevant driver is built as a module for example.

Arguably userspace has some control over that already, as in many cases 
it can just unbind and reprobe the consumer driver after loading the 
provider driver (in my silly IOMMU-drivers-as-modules PoC a while ago I 
was delighted to find that it can really be that simple). It's a bit 
harder when the device is the primary console or root filesystem, but I 
think that's effectively just another variant of the "defer until a 
module is loaded" chicken-and-egg problem.

Robin.

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

* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
  2018-05-24 20:57       ` Rob Herring
@ 2018-05-25 12:20         ` Robin Murphy
  -1 siblings, 0 replies; 66+ messages in thread
From: Robin Murphy @ 2018-05-25 12:20 UTC (permalink / raw)
  To: Rob Herring, Greg Kroah-Hartman
  Cc: Linus Walleij, Alexander Graf, Bjorn Andersson,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, Joerg Roedel,
	Mark Brown, Frank Rowand, linux-kernel, devicetree,
	Architecture Mailman List,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On 24/05/18 21:57, Rob Herring wrote:
> On Thu, May 24, 2018 at 2:00 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>>> Deferred probe will currently wait forever on dependent devices to probe,
>>> but sometimes a driver will never exist. It's also not always critical for
>>> a driver to exist. Platforms can rely on default configuration from the
>>> bootloader or reset defaults for things such as pinctrl and power domains.
>>> This is often the case with initial platform support until various drivers
>>> get enabled. There's at least 2 scenarios where deferred probe can render
>>> a platform broken. Both involve using a DT which has more devices and
>>> dependencies than the kernel supports. The 1st case is a driver may be
>>> disabled in the kernel config. The 2nd case is the kernel version may
>>> simply not have the dependent driver. This can happen if using a newer DT
>>> (provided by firmware perhaps) with a stable kernel version.
>>>
>>> Subsystems or drivers may opt-in to this behavior by calling
>>> driver_deferred_probe_check_init_done() instead of just returning
>>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>>> config to decide whether to continue to defer probe or not.
>>>
>>> Cc: Alexander Graf <agraf@suse.de>
>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>> ---
>>>   drivers/base/dd.c      | 17 +++++++++++++++++
>>>   include/linux/device.h |  2 ++
>>>   2 files changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>>> index c9f54089429b..d6034718da6f 100644
>>> --- a/drivers/base/dd.c
>>> +++ b/drivers/base/dd.c
>>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>>>        driver_deferred_probe_trigger();
>>>   }
>>>
>>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
>>> +{
>>> +     if (optional && initcalls_done) {
>>
>> Wait, what's the "optional" mess here?
> 
> My intent was that subsystems just always call this function and never
> return EPROBE_DEFER themselves. Then the driver core can make
> decisions as to what to do (such as the timeout added in the next
> patch). Or it can print common error/debug messages. So optional is a
> hint to allow subsystems per device control.

Maybe just driver_defer_probe() might be a more descriptive name? To me, 
calling "foo_check_x()" with a parameter that says "I don't actually 
care about x" is the really unintuitive bit.

>>
>> The caller knows this value, so why do you need to even pass it in here?
> 
> Because regardless of the value, we always stop deferring when/if we
> hit the timeout and the caller doesn't know about the timeout. If we
> get rid of it, we'd need functions for both init done and for deferred
> timeout.
> 
>> And bool values that are not obvious are horrid.  I had to go look this
>> up when reading the later patches that just passed "true" in this
>> variable as I had no idea what that meant.
> 
> Perhaps inverting it and calling it "keep_deferring" would be better.
> However, the flag is ignored if we have timed out.

Perhaps an enum (or bitmask of named flags) then? That would allow the 
most readability at callsites, plus it seems quite likely that we may 
want intermediate degrees of "deferral strictness" eventually.

Robin.

>>
>> So as-is, no, this isn't ok, sorry.
>>
>> And at the least, this needs some kerneldoc to explain it :)
> 
> That part is easy enough to fix.
> 
> Rob
> 

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-25 12:20         ` Robin Murphy
  0 siblings, 0 replies; 66+ messages in thread
From: Robin Murphy @ 2018-05-25 12:20 UTC (permalink / raw)
  To: linux-arm-kernel

On 24/05/18 21:57, Rob Herring wrote:
> On Thu, May 24, 2018 at 2:00 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>>> Deferred probe will currently wait forever on dependent devices to probe,
>>> but sometimes a driver will never exist. It's also not always critical for
>>> a driver to exist. Platforms can rely on default configuration from the
>>> bootloader or reset defaults for things such as pinctrl and power domains.
>>> This is often the case with initial platform support until various drivers
>>> get enabled. There's at least 2 scenarios where deferred probe can render
>>> a platform broken. Both involve using a DT which has more devices and
>>> dependencies than the kernel supports. The 1st case is a driver may be
>>> disabled in the kernel config. The 2nd case is the kernel version may
>>> simply not have the dependent driver. This can happen if using a newer DT
>>> (provided by firmware perhaps) with a stable kernel version.
>>>
>>> Subsystems or drivers may opt-in to this behavior by calling
>>> driver_deferred_probe_check_init_done() instead of just returning
>>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>>> config to decide whether to continue to defer probe or not.
>>>
>>> Cc: Alexander Graf <agraf@suse.de>
>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>> ---
>>>   drivers/base/dd.c      | 17 +++++++++++++++++
>>>   include/linux/device.h |  2 ++
>>>   2 files changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>>> index c9f54089429b..d6034718da6f 100644
>>> --- a/drivers/base/dd.c
>>> +++ b/drivers/base/dd.c
>>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>>>        driver_deferred_probe_trigger();
>>>   }
>>>
>>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
>>> +{
>>> +     if (optional && initcalls_done) {
>>
>> Wait, what's the "optional" mess here?
> 
> My intent was that subsystems just always call this function and never
> return EPROBE_DEFER themselves. Then the driver core can make
> decisions as to what to do (such as the timeout added in the next
> patch). Or it can print common error/debug messages. So optional is a
> hint to allow subsystems per device control.

Maybe just driver_defer_probe() might be a more descriptive name? To me, 
calling "foo_check_x()" with a parameter that says "I don't actually 
care about x" is the really unintuitive bit.

>>
>> The caller knows this value, so why do you need to even pass it in here?
> 
> Because regardless of the value, we always stop deferring when/if we
> hit the timeout and the caller doesn't know about the timeout. If we
> get rid of it, we'd need functions for both init done and for deferred
> timeout.
> 
>> And bool values that are not obvious are horrid.  I had to go look this
>> up when reading the later patches that just passed "true" in this
>> variable as I had no idea what that meant.
> 
> Perhaps inverting it and calling it "keep_deferring" would be better.
> However, the flag is ignored if we have timed out.

Perhaps an enum (or bitmask of named flags) then? That would allow the 
most readability at callsites, plus it seems quite likely that we may 
want intermediate degrees of "deferral strictness" eventually.

Robin.

>>
>> So as-is, no, this isn't ok, sorry.
>>
>> And at the least, this needs some kerneldoc to explain it :)
> 
> That part is easy enough to fix.
> 
> Rob
> 

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

* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
  2018-05-25 12:20         ` Robin Murphy
@ 2018-05-25 17:35           ` Rob Herring
  -1 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-25 17:35 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Mark Brown, Frank Rowand, linux-kernel, devicetree,
	Architecture Mailman List,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Fri, May 25, 2018 at 7:20 AM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 24/05/18 21:57, Rob Herring wrote:
>>
>> On Thu, May 24, 2018 at 2:00 PM, Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>>>
>>> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>>>>
>>>> Deferred probe will currently wait forever on dependent devices to
>>>> probe,
>>>> but sometimes a driver will never exist. It's also not always critical
>>>> for
>>>> a driver to exist. Platforms can rely on default configuration from the
>>>> bootloader or reset defaults for things such as pinctrl and power
>>>> domains.
>>>> This is often the case with initial platform support until various
>>>> drivers
>>>> get enabled. There's at least 2 scenarios where deferred probe can
>>>> render
>>>> a platform broken. Both involve using a DT which has more devices and
>>>> dependencies than the kernel supports. The 1st case is a driver may be
>>>> disabled in the kernel config. The 2nd case is the kernel version may
>>>> simply not have the dependent driver. This can happen if using a newer
>>>> DT
>>>> (provided by firmware perhaps) with a stable kernel version.
>>>>
>>>> Subsystems or drivers may opt-in to this behavior by calling
>>>> driver_deferred_probe_check_init_done() instead of just returning
>>>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>>>> config to decide whether to continue to defer probe or not.
>>>>
>>>> Cc: Alexander Graf <agraf@suse.de>
>>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>>> ---
>>>>   drivers/base/dd.c      | 17 +++++++++++++++++
>>>>   include/linux/device.h |  2 ++
>>>>   2 files changed, 19 insertions(+)
>>>>
>>>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>>>> index c9f54089429b..d6034718da6f 100644
>>>> --- a/drivers/base/dd.c
>>>> +++ b/drivers/base/dd.c
>>>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>>>>        driver_deferred_probe_trigger();
>>>>   }
>>>>
>>>> +int driver_deferred_probe_check_init_done(struct device *dev, bool
>>>> optional)
>>>> +{
>>>> +     if (optional && initcalls_done) {
>>>
>>>
>>> Wait, what's the "optional" mess here?
>>
>>
>> My intent was that subsystems just always call this function and never
>> return EPROBE_DEFER themselves. Then the driver core can make
>> decisions as to what to do (such as the timeout added in the next
>> patch). Or it can print common error/debug messages. So optional is a
>> hint to allow subsystems per device control.
>
>
> Maybe just driver_defer_probe() might be a more descriptive name? To me,
> calling "foo_check_x()" with a parameter that says "I don't actually care
> about x" is the really unintuitive bit.

All the other (though static or internal to driver core) functions are
prefixed driver_deferred_probe_* so I was trying to remain consistent
there. You're right though, with the timeout it's not just whether
initcalls are done. It's really "get the return value depending on the
core's deferred probe state". So perhaps one of these:

driver_deferred_probe_get_return_val()
driver_deferred_probe_handle_return()

The other option would be a more straight-forward functions that just
returns a bool on whether to continue deferring and leave the return
code handling to the caller:

if (driver_deferred_probe_enabled_for_builtin(dev))
  return -EPROBE_DEFER;
else
  return -ENODEV;

The pinctrl case would look like this:

builtin_only = of_property_read_bool(np_pctldev, "pinctrl-use-default");
if (builtin_only && driver_deferred_probe_enabled_for_builtin(dev))
  return -EPROBE_DEFER;
else if (!builtin_only && driver_deferred_probe_enabled(dev))
  return -EPROBE_DEFER;
else
  return -ENODEV;

I still prefer the former, picking the bike shed color is easier with
the latter.

>>>
>>> The caller knows this value, so why do you need to even pass it in here?
>>
>>
>> Because regardless of the value, we always stop deferring when/if we
>> hit the timeout and the caller doesn't know about the timeout. If we
>> get rid of it, we'd need functions for both init done and for deferred
>> timeout.
>>
>>> And bool values that are not obvious are horrid.  I had to go look this
>>> up when reading the later patches that just passed "true" in this
>>> variable as I had no idea what that meant.
>>
>>
>> Perhaps inverting it and calling it "keep_deferring" would be better.
>> However, the flag is ignored if we have timed out.
>
>
> Perhaps an enum (or bitmask of named flags) then? That would allow the most
> readability at callsites, plus it seems quite likely that we may want
> intermediate degrees of "deferral strictness" eventually.

A bitmask is just 32 booleans stuffed into one parameter which I can
guess Greg's opinion on.

I can't really think of other flags we might need here. If we added
some userspace trigger saying module loading is done, I don't think
we'd need that to be per caller.

Rob

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-25 17:35           ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-25 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, May 25, 2018 at 7:20 AM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 24/05/18 21:57, Rob Herring wrote:
>>
>> On Thu, May 24, 2018 at 2:00 PM, Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>>>
>>> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>>>>
>>>> Deferred probe will currently wait forever on dependent devices to
>>>> probe,
>>>> but sometimes a driver will never exist. It's also not always critical
>>>> for
>>>> a driver to exist. Platforms can rely on default configuration from the
>>>> bootloader or reset defaults for things such as pinctrl and power
>>>> domains.
>>>> This is often the case with initial platform support until various
>>>> drivers
>>>> get enabled. There's at least 2 scenarios where deferred probe can
>>>> render
>>>> a platform broken. Both involve using a DT which has more devices and
>>>> dependencies than the kernel supports. The 1st case is a driver may be
>>>> disabled in the kernel config. The 2nd case is the kernel version may
>>>> simply not have the dependent driver. This can happen if using a newer
>>>> DT
>>>> (provided by firmware perhaps) with a stable kernel version.
>>>>
>>>> Subsystems or drivers may opt-in to this behavior by calling
>>>> driver_deferred_probe_check_init_done() instead of just returning
>>>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>>>> config to decide whether to continue to defer probe or not.
>>>>
>>>> Cc: Alexander Graf <agraf@suse.de>
>>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>>> ---
>>>>   drivers/base/dd.c      | 17 +++++++++++++++++
>>>>   include/linux/device.h |  2 ++
>>>>   2 files changed, 19 insertions(+)
>>>>
>>>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>>>> index c9f54089429b..d6034718da6f 100644
>>>> --- a/drivers/base/dd.c
>>>> +++ b/drivers/base/dd.c
>>>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>>>>        driver_deferred_probe_trigger();
>>>>   }
>>>>
>>>> +int driver_deferred_probe_check_init_done(struct device *dev, bool
>>>> optional)
>>>> +{
>>>> +     if (optional && initcalls_done) {
>>>
>>>
>>> Wait, what's the "optional" mess here?
>>
>>
>> My intent was that subsystems just always call this function and never
>> return EPROBE_DEFER themselves. Then the driver core can make
>> decisions as to what to do (such as the timeout added in the next
>> patch). Or it can print common error/debug messages. So optional is a
>> hint to allow subsystems per device control.
>
>
> Maybe just driver_defer_probe() might be a more descriptive name? To me,
> calling "foo_check_x()" with a parameter that says "I don't actually care
> about x" is the really unintuitive bit.

All the other (though static or internal to driver core) functions are
prefixed driver_deferred_probe_* so I was trying to remain consistent
there. You're right though, with the timeout it's not just whether
initcalls are done. It's really "get the return value depending on the
core's deferred probe state". So perhaps one of these:

driver_deferred_probe_get_return_val()
driver_deferred_probe_handle_return()

The other option would be a more straight-forward functions that just
returns a bool on whether to continue deferring and leave the return
code handling to the caller:

if (driver_deferred_probe_enabled_for_builtin(dev))
  return -EPROBE_DEFER;
else
  return -ENODEV;

The pinctrl case would look like this:

builtin_only = of_property_read_bool(np_pctldev, "pinctrl-use-default");
if (builtin_only && driver_deferred_probe_enabled_for_builtin(dev))
  return -EPROBE_DEFER;
else if (!builtin_only && driver_deferred_probe_enabled(dev))
  return -EPROBE_DEFER;
else
  return -ENODEV;

I still prefer the former, picking the bike shed color is easier with
the latter.

>>>
>>> The caller knows this value, so why do you need to even pass it in here?
>>
>>
>> Because regardless of the value, we always stop deferring when/if we
>> hit the timeout and the caller doesn't know about the timeout. If we
>> get rid of it, we'd need functions for both init done and for deferred
>> timeout.
>>
>>> And bool values that are not obvious are horrid.  I had to go look this
>>> up when reading the later patches that just passed "true" in this
>>> variable as I had no idea what that meant.
>>
>>
>> Perhaps inverting it and calling it "keep_deferring" would be better.
>> However, the flag is ignored if we have timed out.
>
>
> Perhaps an enum (or bitmask of named flags) then? That would allow the most
> readability at callsites, plus it seems quite likely that we may want
> intermediate degrees of "deferral strictness" eventually.

A bitmask is just 32 booleans stuffed into one parameter which I can
guess Greg's opinion on.

I can't really think of other flags we might need here. If we added
some userspace trigger saying module loading is done, I don't think
we'd need that to be per caller.

Rob

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

* Re: [PATCH v2 7/8] iommu: Remove IOMMU_OF_DECLARE
  2018-05-24 17:50     ` Rob Herring
  (?)
@ 2018-05-28  6:53         ` Marek Szyprowski
  -1 siblings, 0 replies; 66+ messages in thread
From: Marek Szyprowski @ 2018-05-28  6:53 UTC (permalink / raw)
  To: Rob Herring, Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	boot-architecture-cunTk1MwBs8s++Sfvej+rw,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Heiko Stuebner,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Will Deacon,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Krzysztof Kozlowski,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Kukjin Kim,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Rob,

On 2018-05-24 19:50, Rob Herring wrote:
> Now that we use the driver core to stop deferred probe for missing
> drivers, IOMMU_OF_DECLARE can be removed.
>
> This is slightly less optimal than having a list of built-in drivers in
> that we'll now defer probe twice before giving up. This shouldn't have a
> significant impact on boot times as past discussions about deferred
> probe have given no evidence of deferred probe having a substantial
> impact.
>
> Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> Cc: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> Cc: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
> Cc: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Kukjin Kim <kgene-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Krzysztof Kozlowski <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Rob Clark <robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

For Exynos IOMMU:

Acked-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

> ---
>   drivers/iommu/arm-smmu-v3.c    |  2 --
>   drivers/iommu/arm-smmu.c       |  7 -------
>   drivers/iommu/exynos-iommu.c   |  2 --
>   drivers/iommu/ipmmu-vmsa.c     |  3 ---
>   drivers/iommu/msm_iommu.c      |  2 --
>   drivers/iommu/of_iommu.c       | 19 +------------------
>   drivers/iommu/qcom_iommu.c     |  2 --
>   drivers/iommu/rockchip-iommu.c |  2 --
>   include/linux/of_iommu.h       |  4 ----
>   9 files changed, 1 insertion(+), 42 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 1d647104bccc..22bdabd3d8e0 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2915,8 +2915,6 @@ static struct platform_driver arm_smmu_driver = {
>   };
>   module_platform_driver(arm_smmu_driver);
>   
> -IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3");
> -
>   MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
>   MODULE_AUTHOR("Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>");
>   MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 69e7c60792a8..9dd7cbaa3b0c 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -2211,13 +2211,6 @@ static struct platform_driver arm_smmu_driver = {
>   };
>   module_platform_driver(arm_smmu_driver);
>   
> -IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1");
> -IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2");
> -IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400");
> -IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401");
> -IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500");
> -IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2");
> -
>   MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
>   MODULE_AUTHOR("Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>");
>   MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 85879cfec52f..b128cb4372d3 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -1390,5 +1390,3 @@ static int __init exynos_iommu_init(void)
>   	return ret;
>   }
>   core_initcall(exynos_iommu_init);
> -
> -IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu");
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index 40ae6e87cb88..f026aa16d5f1 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -1108,9 +1108,6 @@ static void __exit ipmmu_exit(void)
>   subsys_initcall(ipmmu_init);
>   module_exit(ipmmu_exit);
>   
> -IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa");
> -IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795");
> -
>   MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
>   MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>");
>   MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
> index 0d3350463a3f..27377742600d 100644
> --- a/drivers/iommu/msm_iommu.c
> +++ b/drivers/iommu/msm_iommu.c
> @@ -877,7 +877,5 @@ static void __exit msm_iommu_driver_exit(void)
>   subsys_initcall(msm_iommu_driver_init);
>   module_exit(msm_iommu_driver_exit);
>   
> -IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu");
> -
>   MODULE_LICENSE("GPL v2");
>   MODULE_AUTHOR("Stepan Moskovchenko <stepanm-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>");
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 2aac8387717c..1904ccf9fc4e 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -27,9 +27,6 @@
>   
>   #define NO_IOMMU	1
>   
> -static const struct of_device_id __iommu_of_table_sentinel
> -	__used __section(__iommu_of_table_end);
> -
>   /**
>    * of_get_dma_window - Parse *dma-window property and returns 0 if found.
>    *
> @@ -98,19 +95,6 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
>   }
>   EXPORT_SYMBOL_GPL(of_get_dma_window);
>   
> -static bool of_iommu_driver_present(struct device_node *np)
> -{
> -	/*
> -	 * If the IOMMU still isn't ready by the time we reach init, assume
> -	 * it never will be. We don't want to defer indefinitely, nor attempt
> -	 * to dereference __iommu_of_table after it's been freed.
> -	 */
> -	if (system_state >= SYSTEM_RUNNING)
> -		return false;
> -
> -	return of_match_node(&__iommu_of_table, np);
> -}
> -
>   static int of_iommu_xlate(struct device *dev,
>   			  struct of_phandle_args *iommu_spec)
>   {
> @@ -120,8 +104,7 @@ static int of_iommu_xlate(struct device *dev,
>   
>   	ops = iommu_ops_from_fwnode(fwnode);
>   	if ((ops && !ops->of_xlate) ||
> -	    !of_device_is_available(iommu_spec->np) ||
> -	    (!ops && !of_iommu_driver_present(iommu_spec->np)))
> +	    !of_device_is_available(iommu_spec->np))
>   		return NO_IOMMU;
>   
>   	err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
> index 65b9c99707f8..fa0f6c39a144 100644
> --- a/drivers/iommu/qcom_iommu.c
> +++ b/drivers/iommu/qcom_iommu.c
> @@ -947,7 +947,5 @@ static void __exit qcom_iommu_exit(void)
>   module_init(qcom_iommu_init);
>   module_exit(qcom_iommu_exit);
>   
> -IOMMU_OF_DECLARE(qcom_iommu_dev, "qcom,msm-iommu-v1");
> -
>   MODULE_DESCRIPTION("IOMMU API for QCOM IOMMU v1 implementations");
>   MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 0468acfa131f..90d37f29c24c 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -1284,8 +1284,6 @@ static int __init rk_iommu_init(void)
>   }
>   subsys_initcall(rk_iommu_init);
>   
> -IOMMU_OF_DECLARE(rk_iommu_of, "rockchip,iommu");
> -
>   MODULE_DESCRIPTION("IOMMU API for Rockchip");
>   MODULE_AUTHOR("Simon Xue <xxm-TNX95d0MmH7DzftRWevZcw@public.gmane.org> and Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>");
>   MODULE_ALIAS("platform:rockchip-iommu");
> diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
> index 4fa654e4b5a9..f3d40dd7bb66 100644
> --- a/include/linux/of_iommu.h
> +++ b/include/linux/of_iommu.h
> @@ -32,8 +32,4 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
>   
>   #endif	/* CONFIG_OF_IOMMU */
>   
> -extern struct of_device_id __iommu_of_table;
> -
> -#define IOMMU_OF_DECLARE(name, compat)	OF_DECLARE_1(iommu, name, compat, NULL)
> -
>   #endif /* __OF_IOMMU_H */

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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

* Re: [PATCH v2 7/8] iommu: Remove IOMMU_OF_DECLARE
@ 2018-05-28  6:53         ` Marek Szyprowski
  0 siblings, 0 replies; 66+ messages in thread
From: Marek Szyprowski @ 2018-05-28  6:53 UTC (permalink / raw)
  To: Rob Herring, Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel,
	Will Deacon, Kukjin Kim, Krzysztof Kozlowski, Rob Clark,
	Heiko Stuebner, iommu, linux-samsung-soc, linux-arm-msm,
	linux-rockchip

Hi Rob,

On 2018-05-24 19:50, Rob Herring wrote:
> Now that we use the driver core to stop deferred probe for missing
> drivers, IOMMU_OF_DECLARE can be removed.
>
> This is slightly less optimal than having a list of built-in drivers in
> that we'll now defer probe twice before giving up. This shouldn't have a
> significant impact on boot times as past discussions about deferred
> probe have given no evidence of deferred probe having a substantial
> impact.
>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Kukjin Kim <kgene@kernel.org>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: iommu@lists.linux-foundation.org
> Cc: linux-samsung-soc@vger.kernel.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: linux-rockchip@lists.infradead.org
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>

For Exynos IOMMU:

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
>   drivers/iommu/arm-smmu-v3.c    |  2 --
>   drivers/iommu/arm-smmu.c       |  7 -------
>   drivers/iommu/exynos-iommu.c   |  2 --
>   drivers/iommu/ipmmu-vmsa.c     |  3 ---
>   drivers/iommu/msm_iommu.c      |  2 --
>   drivers/iommu/of_iommu.c       | 19 +------------------
>   drivers/iommu/qcom_iommu.c     |  2 --
>   drivers/iommu/rockchip-iommu.c |  2 --
>   include/linux/of_iommu.h       |  4 ----
>   9 files changed, 1 insertion(+), 42 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 1d647104bccc..22bdabd3d8e0 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2915,8 +2915,6 @@ static struct platform_driver arm_smmu_driver = {
>   };
>   module_platform_driver(arm_smmu_driver);
>   
> -IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3");
> -
>   MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
>   MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
>   MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 69e7c60792a8..9dd7cbaa3b0c 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -2211,13 +2211,6 @@ static struct platform_driver arm_smmu_driver = {
>   };
>   module_platform_driver(arm_smmu_driver);
>   
> -IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1");
> -IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2");
> -IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400");
> -IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401");
> -IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500");
> -IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2");
> -
>   MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
>   MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
>   MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 85879cfec52f..b128cb4372d3 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -1390,5 +1390,3 @@ static int __init exynos_iommu_init(void)
>   	return ret;
>   }
>   core_initcall(exynos_iommu_init);
> -
> -IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu");
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index 40ae6e87cb88..f026aa16d5f1 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -1108,9 +1108,6 @@ static void __exit ipmmu_exit(void)
>   subsys_initcall(ipmmu_init);
>   module_exit(ipmmu_exit);
>   
> -IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa");
> -IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795");
> -
>   MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
>   MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
>   MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
> index 0d3350463a3f..27377742600d 100644
> --- a/drivers/iommu/msm_iommu.c
> +++ b/drivers/iommu/msm_iommu.c
> @@ -877,7 +877,5 @@ static void __exit msm_iommu_driver_exit(void)
>   subsys_initcall(msm_iommu_driver_init);
>   module_exit(msm_iommu_driver_exit);
>   
> -IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu");
> -
>   MODULE_LICENSE("GPL v2");
>   MODULE_AUTHOR("Stepan Moskovchenko <stepanm@codeaurora.org>");
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 2aac8387717c..1904ccf9fc4e 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -27,9 +27,6 @@
>   
>   #define NO_IOMMU	1
>   
> -static const struct of_device_id __iommu_of_table_sentinel
> -	__used __section(__iommu_of_table_end);
> -
>   /**
>    * of_get_dma_window - Parse *dma-window property and returns 0 if found.
>    *
> @@ -98,19 +95,6 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
>   }
>   EXPORT_SYMBOL_GPL(of_get_dma_window);
>   
> -static bool of_iommu_driver_present(struct device_node *np)
> -{
> -	/*
> -	 * If the IOMMU still isn't ready by the time we reach init, assume
> -	 * it never will be. We don't want to defer indefinitely, nor attempt
> -	 * to dereference __iommu_of_table after it's been freed.
> -	 */
> -	if (system_state >= SYSTEM_RUNNING)
> -		return false;
> -
> -	return of_match_node(&__iommu_of_table, np);
> -}
> -
>   static int of_iommu_xlate(struct device *dev,
>   			  struct of_phandle_args *iommu_spec)
>   {
> @@ -120,8 +104,7 @@ static int of_iommu_xlate(struct device *dev,
>   
>   	ops = iommu_ops_from_fwnode(fwnode);
>   	if ((ops && !ops->of_xlate) ||
> -	    !of_device_is_available(iommu_spec->np) ||
> -	    (!ops && !of_iommu_driver_present(iommu_spec->np)))
> +	    !of_device_is_available(iommu_spec->np))
>   		return NO_IOMMU;
>   
>   	err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
> index 65b9c99707f8..fa0f6c39a144 100644
> --- a/drivers/iommu/qcom_iommu.c
> +++ b/drivers/iommu/qcom_iommu.c
> @@ -947,7 +947,5 @@ static void __exit qcom_iommu_exit(void)
>   module_init(qcom_iommu_init);
>   module_exit(qcom_iommu_exit);
>   
> -IOMMU_OF_DECLARE(qcom_iommu_dev, "qcom,msm-iommu-v1");
> -
>   MODULE_DESCRIPTION("IOMMU API for QCOM IOMMU v1 implementations");
>   MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 0468acfa131f..90d37f29c24c 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -1284,8 +1284,6 @@ static int __init rk_iommu_init(void)
>   }
>   subsys_initcall(rk_iommu_init);
>   
> -IOMMU_OF_DECLARE(rk_iommu_of, "rockchip,iommu");
> -
>   MODULE_DESCRIPTION("IOMMU API for Rockchip");
>   MODULE_AUTHOR("Simon Xue <xxm@rock-chips.com> and Daniel Kurtz <djkurtz@chromium.org>");
>   MODULE_ALIAS("platform:rockchip-iommu");
> diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
> index 4fa654e4b5a9..f3d40dd7bb66 100644
> --- a/include/linux/of_iommu.h
> +++ b/include/linux/of_iommu.h
> @@ -32,8 +32,4 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
>   
>   #endif	/* CONFIG_OF_IOMMU */
>   
> -extern struct of_device_id __iommu_of_table;
> -
> -#define IOMMU_OF_DECLARE(name, compat)	OF_DECLARE_1(iommu, name, compat, NULL)
> -
>   #endif /* __OF_IOMMU_H */

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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

* [PATCH v2 7/8] iommu: Remove IOMMU_OF_DECLARE
@ 2018-05-28  6:53         ` Marek Szyprowski
  0 siblings, 0 replies; 66+ messages in thread
From: Marek Szyprowski @ 2018-05-28  6:53 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Rob,

On 2018-05-24 19:50, Rob Herring wrote:
> Now that we use the driver core to stop deferred probe for missing
> drivers, IOMMU_OF_DECLARE can be removed.
>
> This is slightly less optimal than having a list of built-in drivers in
> that we'll now defer probe twice before giving up. This shouldn't have a
> significant impact on boot times as past discussions about deferred
> probe have given no evidence of deferred probe having a substantial
> impact.
>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Kukjin Kim <kgene@kernel.org>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: iommu at lists.linux-foundation.org
> Cc: linux-samsung-soc at vger.kernel.org
> Cc: linux-arm-msm at vger.kernel.org
> Cc: linux-rockchip at lists.infradead.org
> Cc: devicetree at vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>

For Exynos IOMMU:

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
>   drivers/iommu/arm-smmu-v3.c    |  2 --
>   drivers/iommu/arm-smmu.c       |  7 -------
>   drivers/iommu/exynos-iommu.c   |  2 --
>   drivers/iommu/ipmmu-vmsa.c     |  3 ---
>   drivers/iommu/msm_iommu.c      |  2 --
>   drivers/iommu/of_iommu.c       | 19 +------------------
>   drivers/iommu/qcom_iommu.c     |  2 --
>   drivers/iommu/rockchip-iommu.c |  2 --
>   include/linux/of_iommu.h       |  4 ----
>   9 files changed, 1 insertion(+), 42 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 1d647104bccc..22bdabd3d8e0 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2915,8 +2915,6 @@ static struct platform_driver arm_smmu_driver = {
>   };
>   module_platform_driver(arm_smmu_driver);
>   
> -IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3");
> -
>   MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
>   MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
>   MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 69e7c60792a8..9dd7cbaa3b0c 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -2211,13 +2211,6 @@ static struct platform_driver arm_smmu_driver = {
>   };
>   module_platform_driver(arm_smmu_driver);
>   
> -IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1");
> -IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2");
> -IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400");
> -IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401");
> -IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500");
> -IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2");
> -
>   MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
>   MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
>   MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 85879cfec52f..b128cb4372d3 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -1390,5 +1390,3 @@ static int __init exynos_iommu_init(void)
>   	return ret;
>   }
>   core_initcall(exynos_iommu_init);
> -
> -IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu");
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index 40ae6e87cb88..f026aa16d5f1 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -1108,9 +1108,6 @@ static void __exit ipmmu_exit(void)
>   subsys_initcall(ipmmu_init);
>   module_exit(ipmmu_exit);
>   
> -IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa");
> -IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795");
> -
>   MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
>   MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
>   MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
> index 0d3350463a3f..27377742600d 100644
> --- a/drivers/iommu/msm_iommu.c
> +++ b/drivers/iommu/msm_iommu.c
> @@ -877,7 +877,5 @@ static void __exit msm_iommu_driver_exit(void)
>   subsys_initcall(msm_iommu_driver_init);
>   module_exit(msm_iommu_driver_exit);
>   
> -IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu");
> -
>   MODULE_LICENSE("GPL v2");
>   MODULE_AUTHOR("Stepan Moskovchenko <stepanm@codeaurora.org>");
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 2aac8387717c..1904ccf9fc4e 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -27,9 +27,6 @@
>   
>   #define NO_IOMMU	1
>   
> -static const struct of_device_id __iommu_of_table_sentinel
> -	__used __section(__iommu_of_table_end);
> -
>   /**
>    * of_get_dma_window - Parse *dma-window property and returns 0 if found.
>    *
> @@ -98,19 +95,6 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
>   }
>   EXPORT_SYMBOL_GPL(of_get_dma_window);
>   
> -static bool of_iommu_driver_present(struct device_node *np)
> -{
> -	/*
> -	 * If the IOMMU still isn't ready by the time we reach init, assume
> -	 * it never will be. We don't want to defer indefinitely, nor attempt
> -	 * to dereference __iommu_of_table after it's been freed.
> -	 */
> -	if (system_state >= SYSTEM_RUNNING)
> -		return false;
> -
> -	return of_match_node(&__iommu_of_table, np);
> -}
> -
>   static int of_iommu_xlate(struct device *dev,
>   			  struct of_phandle_args *iommu_spec)
>   {
> @@ -120,8 +104,7 @@ static int of_iommu_xlate(struct device *dev,
>   
>   	ops = iommu_ops_from_fwnode(fwnode);
>   	if ((ops && !ops->of_xlate) ||
> -	    !of_device_is_available(iommu_spec->np) ||
> -	    (!ops && !of_iommu_driver_present(iommu_spec->np)))
> +	    !of_device_is_available(iommu_spec->np))
>   		return NO_IOMMU;
>   
>   	err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
> index 65b9c99707f8..fa0f6c39a144 100644
> --- a/drivers/iommu/qcom_iommu.c
> +++ b/drivers/iommu/qcom_iommu.c
> @@ -947,7 +947,5 @@ static void __exit qcom_iommu_exit(void)
>   module_init(qcom_iommu_init);
>   module_exit(qcom_iommu_exit);
>   
> -IOMMU_OF_DECLARE(qcom_iommu_dev, "qcom,msm-iommu-v1");
> -
>   MODULE_DESCRIPTION("IOMMU API for QCOM IOMMU v1 implementations");
>   MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 0468acfa131f..90d37f29c24c 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -1284,8 +1284,6 @@ static int __init rk_iommu_init(void)
>   }
>   subsys_initcall(rk_iommu_init);
>   
> -IOMMU_OF_DECLARE(rk_iommu_of, "rockchip,iommu");
> -
>   MODULE_DESCRIPTION("IOMMU API for Rockchip");
>   MODULE_AUTHOR("Simon Xue <xxm@rock-chips.com> and Daniel Kurtz <djkurtz@chromium.org>");
>   MODULE_ALIAS("platform:rockchip-iommu");
> diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
> index 4fa654e4b5a9..f3d40dd7bb66 100644
> --- a/include/linux/of_iommu.h
> +++ b/include/linux/of_iommu.h
> @@ -32,8 +32,4 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
>   
>   #endif	/* CONFIG_OF_IOMMU */
>   
> -extern struct of_device_id __iommu_of_table;
> -
> -#define IOMMU_OF_DECLARE(name, compat)	OF_DECLARE_1(iommu, name, compat, NULL)
> -
>   #endif /* __OF_IOMMU_H */

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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

* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
  2018-05-24 18:18     ` Mark Brown
@ 2018-05-29  5:12       ` Frank Rowand
  -1 siblings, 0 replies; 66+ messages in thread
From: Frank Rowand @ 2018-05-29  5:12 UTC (permalink / raw)
  To: Mark Brown, Rob Herring
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, linux-kernel, devicetree,
	boot-architecture, linux-arm-kernel

On 05/24/18 11:18, Mark Brown wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
> 
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
> 
> Should userspace have some involvement in this decision?  It knows if
> it's got any intention of loading modules for example.  Kernel config
> checks might be good enough, though it's going to be a pain to work out
> if the relevant driver is built as a module for example.
> 

A parallel issue is that loading an overlay could provide the resource
that will allow the deferred probe to complete.  (That is, once we
finish implementing the run time overlays feature.)

-Frank

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-29  5:12       ` Frank Rowand
  0 siblings, 0 replies; 66+ messages in thread
From: Frank Rowand @ 2018-05-29  5:12 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/24/18 11:18, Mark Brown wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
> 
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
> 
> Should userspace have some involvement in this decision?  It knows if
> it's got any intention of loading modules for example.  Kernel config
> checks might be good enough, though it's going to be a pain to work out
> if the relevant driver is built as a module for example.
> 

A parallel issue is that loading an overlay could provide the resource
that will allow the deferred probe to complete.  (That is, once we
finish implementing the run time overlays feature.)

-Frank

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

* Re: [PATCH v2 0/8] Make deferring probe forever optional
  2018-05-24 17:50 ` Rob Herring
@ 2018-05-29 11:48   ` Joerg Roedel
  -1 siblings, 0 replies; 66+ messages in thread
From: Joerg Roedel @ 2018-05-29 11:48 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Robin Murphy, Mark Brown, Frank Rowand, linux-kernel, devicetree,
	boot-architecture, linux-arm-kernel

On Thu, May 24, 2018 at 12:50:16PM -0500, Rob Herring wrote:
> Rob Herring (8):
>   driver core: make deferring probe after init optional
>   driver core: add a deferred probe timeout
>   dt-bindings: pinctrl: add a 'pinctrl-use-default' property
>   arm: dts: bcm283x: mark the UART pin muxing nodes with
>     pinctrl-use-default
>   pinctrl: optionally stop deferring probe at end of initcalls
>   iommu: Stop deferring probe at end of initcalls
>   iommu: Remove IOMMU_OF_DECLARE
>   PM / Domains: Stop deferring probe at the end of initcall
> 
>  .../admin-guide/kernel-parameters.txt         |  7 +++
>  .../bindings/pinctrl/pinctrl-bindings.txt     |  6 +++
>  arch/arm/boot/dts/bcm283x.dtsi                |  2 +
>  drivers/base/dd.c                             | 43 +++++++++++++++++++
>  drivers/base/power/domain.c                   |  2 +-
>  drivers/iommu/arm-smmu-v3.c                   |  2 -
>  drivers/iommu/arm-smmu.c                      |  7 ---
>  drivers/iommu/exynos-iommu.c                  |  2 -
>  drivers/iommu/ipmmu-vmsa.c                    |  3 --
>  drivers/iommu/msm_iommu.c                     |  2 -
>  drivers/iommu/of_iommu.c                      | 21 +--------
>  drivers/iommu/qcom_iommu.c                    |  2 -
>  drivers/iommu/rockchip-iommu.c                |  2 -
>  drivers/pinctrl/devicetree.c                  | 14 ++++--
>  include/linux/device.h                        |  2 +
>  include/linux/of_iommu.h                      |  4 --
>  16 files changed, 73 insertions(+), 48 deletions(-)

For the IOMMU bits:

Acked-by: Joerg Roedel <jroedel@suse.de>

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

* [PATCH v2 0/8] Make deferring probe forever optional
@ 2018-05-29 11:48   ` Joerg Roedel
  0 siblings, 0 replies; 66+ messages in thread
From: Joerg Roedel @ 2018-05-29 11:48 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 24, 2018 at 12:50:16PM -0500, Rob Herring wrote:
> Rob Herring (8):
>   driver core: make deferring probe after init optional
>   driver core: add a deferred probe timeout
>   dt-bindings: pinctrl: add a 'pinctrl-use-default' property
>   arm: dts: bcm283x: mark the UART pin muxing nodes with
>     pinctrl-use-default
>   pinctrl: optionally stop deferring probe at end of initcalls
>   iommu: Stop deferring probe at end of initcalls
>   iommu: Remove IOMMU_OF_DECLARE
>   PM / Domains: Stop deferring probe at the end of initcall
> 
>  .../admin-guide/kernel-parameters.txt         |  7 +++
>  .../bindings/pinctrl/pinctrl-bindings.txt     |  6 +++
>  arch/arm/boot/dts/bcm283x.dtsi                |  2 +
>  drivers/base/dd.c                             | 43 +++++++++++++++++++
>  drivers/base/power/domain.c                   |  2 +-
>  drivers/iommu/arm-smmu-v3.c                   |  2 -
>  drivers/iommu/arm-smmu.c                      |  7 ---
>  drivers/iommu/exynos-iommu.c                  |  2 -
>  drivers/iommu/ipmmu-vmsa.c                    |  3 --
>  drivers/iommu/msm_iommu.c                     |  2 -
>  drivers/iommu/of_iommu.c                      | 21 +--------
>  drivers/iommu/qcom_iommu.c                    |  2 -
>  drivers/iommu/rockchip-iommu.c                |  2 -
>  drivers/pinctrl/devicetree.c                  | 14 ++++--
>  include/linux/device.h                        |  2 +
>  include/linux/of_iommu.h                      |  4 --
>  16 files changed, 73 insertions(+), 48 deletions(-)

For the IOMMU bits:

Acked-by: Joerg Roedel <jroedel@suse.de>

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

* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-29 14:46         ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-29 14:46 UTC (permalink / raw)
  To: Frank Rowand
  Cc: Mark Brown, Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, linux-kernel, devicetree,
	Architecture Mailman List,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Tue, May 29, 2018 at 12:12 AM, Frank Rowand <frowand.list@gmail.com> wrote:
> On 05/24/18 11:18, Mark Brown wrote:
>> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>>
>>> Subsystems or drivers may opt-in to this behavior by calling
>>> driver_deferred_probe_check_init_done() instead of just returning
>>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>>> config to decide whether to continue to defer probe or not.
>>
>> Should userspace have some involvement in this decision?  It knows if
>> it's got any intention of loading modules for example.  Kernel config
>> checks might be good enough, though it's going to be a pain to work out
>> if the relevant driver is built as a module for example.
>>
>
> A parallel issue is that loading an overlay could provide the resource
> that will allow the deferred probe to complete.  (That is, once we
> finish implementing the run time overlays feature.)

I'd like to see an actual example where that could happen. I agree you
could craft it, but would it really be a valid partitioning? For
example, SoC pinctrl, iommu, or power domains defined in an overlay
would not be something valid to apply during kernel boot or after boot
(though putting those into overlays is exactly what Alex wants to do).

Rob

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

* Re: [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-29 14:46         ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-29 14:46 UTC (permalink / raw)
  To: Frank Rowand
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Ulf Hansson,
	Architecture Mailman List, Kevin Hilman, Greg Kroah-Hartman,
	Linus Walleij, Rafael J. Wysocki, Alexander Graf,
	Bjorn Andersson, Mark Brown,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Robin Murphy, Joerg Roedel, linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Tue, May 29, 2018 at 12:12 AM, Frank Rowand <frowand.list@gmail.com> wrote:
> On 05/24/18 11:18, Mark Brown wrote:
>> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>>
>>> Subsystems or drivers may opt-in to this behavior by calling
>>> driver_deferred_probe_check_init_done() instead of just returning
>>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>>> config to decide whether to continue to defer probe or not.
>>
>> Should userspace have some involvement in this decision?  It knows if
>> it's got any intention of loading modules for example.  Kernel config
>> checks might be good enough, though it's going to be a pain to work out
>> if the relevant driver is built as a module for example.
>>
>
> A parallel issue is that loading an overlay could provide the resource
> that will allow the deferred probe to complete.  (That is, once we
> finish implementing the run time overlays feature.)

I'd like to see an actual example where that could happen. I agree you
could craft it, but would it really be a valid partitioning? For
example, SoC pinctrl, iommu, or power domains defined in an overlay
would not be something valid to apply during kernel boot or after boot
(though putting those into overlays is exactly what Alex wants to do).

Rob
_______________________________________________
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture

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

* [PATCH v2 1/8] driver core: make deferring probe after init optional
@ 2018-05-29 14:46         ` Rob Herring
  0 siblings, 0 replies; 66+ messages in thread
From: Rob Herring @ 2018-05-29 14:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 29, 2018 at 12:12 AM, Frank Rowand <frowand.list@gmail.com> wrote:
> On 05/24/18 11:18, Mark Brown wrote:
>> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>>
>>> Subsystems or drivers may opt-in to this behavior by calling
>>> driver_deferred_probe_check_init_done() instead of just returning
>>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>>> config to decide whether to continue to defer probe or not.
>>
>> Should userspace have some involvement in this decision?  It knows if
>> it's got any intention of loading modules for example.  Kernel config
>> checks might be good enough, though it's going to be a pain to work out
>> if the relevant driver is built as a module for example.
>>
>
> A parallel issue is that loading an overlay could provide the resource
> that will allow the deferred probe to complete.  (That is, once we
> finish implementing the run time overlays feature.)

I'd like to see an actual example where that could happen. I agree you
could craft it, but would it really be a valid partitioning? For
example, SoC pinctrl, iommu, or power domains defined in an overlay
would not be something valid to apply during kernel boot or after boot
(though putting those into overlays is exactly what Alex wants to do).

Rob

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

* Re: [PATCH v2 5/8] pinctrl: optionally stop deferring probe at end of initcalls
@ 2018-05-30  7:00     ` Linus Walleij
  0 siblings, 0 replies; 66+ messages in thread
From: Linus Walleij @ 2018-05-30  7:00 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, Alexander Graf, Bjorn Andersson,
	Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, Joerg Roedel,
	Robin Murphy, Mark Brown, Frank Rowand, linux-kernel,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	boot-architecture, Linux ARM

On Thu, May 24, 2018 at 7:50 PM, Rob Herring <robh@kernel.org> wrote:

> If the pinctrl node in DT indicates that pin setup is optional and the
> defaults can be used with the 'pinctrl-use-default', then only defer probe
> until initcalls are done. This gives platforms the option to work without
> their pinctrl driver being enabled.
>
> Signed-off-by: Rob Herring <robh@kernel.org>

I trust that you know what you're doing so I guess
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

* Re: [PATCH v2 5/8] pinctrl: optionally stop deferring probe at end of initcalls
@ 2018-05-30  7:00     ` Linus Walleij
  0 siblings, 0 replies; 66+ messages in thread
From: Linus Walleij @ 2018-05-30  7:00 UTC (permalink / raw)
  To: Rob Herring
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Ulf Hansson, boot-architecture-cunTk1MwBs8s++Sfvej+rw,
	Frank Rowand, Kevin Hilman, Greg Kroah-Hartman, Joerg Roedel,
	Rafael J. Wysocki, Alexander Graf, Bjorn Andersson, Mark Brown,
	Linux ARM, Robin Murphy, linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Thu, May 24, 2018 at 7:50 PM, Rob Herring <robh@kernel.org> wrote:

> If the pinctrl node in DT indicates that pin setup is optional and the
> defaults can be used with the 'pinctrl-use-default', then only defer probe
> until initcalls are done. This gives platforms the option to work without
> their pinctrl driver being enabled.
>
> Signed-off-by: Rob Herring <robh@kernel.org>

I trust that you know what you're doing so I guess
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
_______________________________________________
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture

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

* [PATCH v2 5/8] pinctrl: optionally stop deferring probe at end of initcalls
@ 2018-05-30  7:00     ` Linus Walleij
  0 siblings, 0 replies; 66+ messages in thread
From: Linus Walleij @ 2018-05-30  7:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 24, 2018 at 7:50 PM, Rob Herring <robh@kernel.org> wrote:

> If the pinctrl node in DT indicates that pin setup is optional and the
> defaults can be used with the 'pinctrl-use-default', then only defer probe
> until initcalls are done. This gives platforms the option to work without
> their pinctrl driver being enabled.
>
> Signed-off-by: Rob Herring <robh@kernel.org>

I trust that you know what you're doing so I guess
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

end of thread, other threads:[~2018-05-30  7:00 UTC | newest]

Thread overview: 66+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-24 17:50 [PATCH v2 0/8] Make deferring probe forever optional Rob Herring
2018-05-24 17:50 ` Rob Herring
2018-05-24 17:50 ` [PATCH v2 1/8] driver core: make deferring probe after init optional Rob Herring
2018-05-24 17:50   ` Rob Herring
2018-05-24 17:50   ` Rob Herring
2018-05-24 18:18   ` Mark Brown
2018-05-24 18:18     ` Mark Brown
2018-05-24 20:25     ` Rob Herring
2018-05-24 20:25       ` Rob Herring
2018-05-25 11:47     ` Robin Murphy
2018-05-25 11:47       ` Robin Murphy
2018-05-29  5:12     ` Frank Rowand
2018-05-29  5:12       ` Frank Rowand
2018-05-29 14:46       ` Rob Herring
2018-05-29 14:46         ` Rob Herring
2018-05-29 14:46         ` Rob Herring
2018-05-24 18:56   ` Greg Kroah-Hartman
2018-05-24 18:56     ` Greg Kroah-Hartman
2018-05-24 19:42     ` Rob Herring
2018-05-24 19:42       ` Rob Herring
2018-05-24 19:00   ` Greg Kroah-Hartman
2018-05-24 19:00     ` Greg Kroah-Hartman
2018-05-24 20:57     ` Rob Herring
2018-05-24 20:57       ` Rob Herring
2018-05-25 12:20       ` Robin Murphy
2018-05-25 12:20         ` Robin Murphy
2018-05-25 17:35         ` Rob Herring
2018-05-25 17:35           ` Rob Herring
2018-05-24 22:28   ` Bjorn Andersson
2018-05-24 22:28     ` Bjorn Andersson
2018-05-24 23:47     ` Rob Herring
2018-05-24 23:47       ` Rob Herring
2018-05-24 17:50 ` [PATCH v2 2/8] driver core: add a deferred probe timeout Rob Herring
2018-05-24 17:50   ` Rob Herring
2018-05-24 19:01   ` Greg Kroah-Hartman
2018-05-24 19:01     ` Greg Kroah-Hartman
2018-05-24 19:45     ` Rob Herring
2018-05-24 19:45       ` Rob Herring
2018-05-24 19:51       ` Greg Kroah-Hartman
2018-05-24 19:51         ` Greg Kroah-Hartman
2018-05-24 17:50 ` [PATCH v2 3/8] dt-bindings: pinctrl: add a 'pinctrl-use-default' property Rob Herring
2018-05-24 17:50   ` Rob Herring
2018-05-24 17:50 ` [PATCH v2 4/8] arm: dts: bcm283x: mark the UART pin muxing nodes with pinctrl-use-default Rob Herring
2018-05-24 17:50   ` Rob Herring
2018-05-24 17:50 ` [PATCH v2 5/8] pinctrl: optionally stop deferring probe at end of initcalls Rob Herring
2018-05-24 17:50   ` Rob Herring
2018-05-24 17:50   ` Rob Herring
2018-05-30  7:00   ` Linus Walleij
2018-05-30  7:00     ` Linus Walleij
2018-05-30  7:00     ` Linus Walleij
2018-05-24 17:50 ` [PATCH v2 6/8] iommu: Stop " Rob Herring
2018-05-24 17:50   ` Rob Herring
2018-05-24 17:50   ` Rob Herring
     [not found] ` <20180524175024.19874-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2018-05-24 17:50   ` [PATCH v2 7/8] iommu: Remove IOMMU_OF_DECLARE Rob Herring
2018-05-24 17:50     ` Rob Herring
2018-05-24 17:50     ` Rob Herring
     [not found]     ` <20180524175024.19874-8-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2018-05-25 11:31       ` Will Deacon
2018-05-25 11:31         ` Will Deacon
2018-05-25 11:31         ` Will Deacon
2018-05-28  6:53       ` Marek Szyprowski
2018-05-28  6:53         ` Marek Szyprowski
2018-05-28  6:53         ` Marek Szyprowski
2018-05-24 17:50 ` [PATCH v2 8/8] PM / Domains: Stop deferring probe at the end of initcall Rob Herring
2018-05-24 17:50   ` Rob Herring
2018-05-29 11:48 ` [PATCH v2 0/8] Make deferring probe forever optional Joerg Roedel
2018-05-29 11:48   ` Joerg Roedel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.