Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1201803 > unrolled thread

[PATCH v3 0/18] On-demand device probing

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/18] On-demand device probing Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-08-06 16:30 +0200
    [PATCH v3 02/18] of/platform: add of_platform_probe Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-08-06 16:30 +0200
      Re: [PATCH v3 02/18] of/platform: add of_platform_probe Mark Brown <broonie@kernel.org> - 2015-08-07 14:20 +0200
        Re: [PATCH v3 02/18] of/platform: add of_platform_probe Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-08-11 11:40 +0200
    [PATCH v3 05/18] regulator: core: Reduce critical area in _regulator_get Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-08-06 16:30 +0200
      Re: [PATCH v3 05/18] regulator: core: Reduce critical area in  _regulator_get Mark Brown <broonie@kernel.org> - 2015-08-07 14:10 +0200
    [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-08-06 16:30 +0200
      Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall Rob Herring <robherring2@gmail.com> - 2015-08-06 22:20 +0200
        Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-08-07 09:20 +0200
          Re: [PATCH v3 01/18] platform: delay OF device-driver matches until  late_initcall Grygorii Strashko <grygorii.strashko@ti.com> - 2015-08-07 19:10 +0200
            Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-08-09 15:10 +0200
              Re: [PATCH v3 01/18] platform: delay OF device-driver matches until  late_initcall Mark Brown <broonie@kernel.org> - 2015-08-10 12:30 +0200
              Re: [PATCH v3 01/18] platform: delay OF device-driver matches until  late_initcall Grygorii Strashko <grygorii.strashko@ti.com> - 2015-08-14 21:20 +0200
    Re: [PATCH v3 0/18] On-demand device probing Rob Herring <robherring2@gmail.com> - 2015-08-06 22:20 +0200
      Re: [PATCH v3 0/18] On-demand device probing Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-08-07 09:00 +0200

#1201803 — [PATCH v3 0/18] On-demand device probing

Hello,

I have a problem with the panel on my Tegra Chromebook taking longer
than expected to be ready during boot (Stéphane Marchesin reported what
is basically the same issue in [0]), and have looked into ordered
probing as a better way of solving this than moving nodes around in the
DT or playing with initcall levels and linking order.

While reading the thread [1] that Alexander Holler started with his
series to make probing order deterministic, it occurred to me that it
should be possible to achieve the same by probing devices as they are
referenced by other devices.

This basically reuses the information that is already implicit in the
probe() implementations, saving us from refactoring existing drivers or
adding information to DTBs.

During review of v1 of this series Linus Walleij suggested that it
should be the device driver core to make sure that dependencies are
ready before probing a device. I gave this idea a try [2] but Mark Brown
pointed out to the logic duplication between the resource acquisition
and dependency discovery code paths (though I think it's fairly minor).

To address that code duplication I experimented with Arnd's devm_probe
[3] concept of having drivers declare their dependencies instead of
acquiring them during probe, and while it worked [4], I don't think we
end up winning anything when compared to just probing devices on-demand
from resource getters.

One remaining objection is to the "sprinkling" of calls to
fwnode_ensure_device() in the resource getters of each subsystem, but I
think it's the right thing to do given that the storage of resources is
currently subsystem-specific.

We could avoid the above by moving resource storage into the core, but I
don't think there's a compelling case for that.

I have tested this on boards with Tegra, iMX.6, Exynos and OMAP SoCs,
and these patches were enough to eliminate all the deferred probes
(except one in PandaBoard because omap_dma_system doesn't have a
firmware node as of yet).

Have submitted a branch [5] with these patches to kernelci.org and I'm
currently trying to fix all regressions, usually due to code assuming
that devices will be probed in a specific order. Current results [6] are
348 passes, 30 fails and 42 unknowns (linux-next [7] is currently
387/3/23).

With this series I get the kernel to output to the panel in 0.5s,
instead of 2.8s.

Regards,

Tomeu

[0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html

[1] https://lkml.org/lkml/2014/5/12/452

[2] https://lkml.org/lkml/2015/6/17/305

[3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689

[4] https://lkml.org/lkml/2015/7/21/441a

[5] https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v5

[6] http://kernelci.org/boot/all/job/collabora/kernel/v4.2-rc5-6548-g632b98c83840/

[7] http://kernelci.org/boot/all/job/next/kernel/next-20150806/

Changes in v3:
- Only delay platform devices with OF nodes
- Set and use device_node.platform_dev instead of reversing the logic to
  find the platform device that encloses a device node.
- Drop the fwnode API to probe firmware nodes and add OF-only API for
  now. I think this same scheme could be used for machines with ACPI,
  but I haven't been able to find one that had to defer its probes because
  of the device probe order.
- Avoid unlocking the regulator device's mutex if we don't have a device

Changes in v2:
- Move delay to platform.c
- Acquire regulator device lock before returning from regulator_dev_lookup()

Tomeu Vizoso (18):
  platform: delay OF device-driver matches until late_initcall
  of/platform: add of_platform_probe
  gpio: Probe GPIO drivers on demand
  gpio: Probe pinctrl devices on demand
  regulator: core: Reduce critical area in _regulator_get
  regulator: core: Probe regulators on demand
  drm: Probe panels on demand
  drm/tegra: Probe dpaux devices on demand
  i2c: core: Probe i2c adapters and devices on demand
  pwm: Probe PWM chip devices on demand
  backlight: Probe backlight devices on demand
  usb: phy: Probe phy devices on demand
  clk: Probe clk providers on demand
  pinctrl: Probe pinctrl devices on demand
  phy: core: Probe phy providers on demand
  dma: of: Probe DMA controllers on demand
  power-supply: Probe power supplies on demand
  ASoC: core: Probe components on demand

 drivers/base/platform.c             |  29 ++++++++++
 drivers/clk/clk.c                   |   3 +
 drivers/dma/of-dma.c                |   3 +
 drivers/gpio/gpiolib-of.c           |   5 ++
 drivers/gpu/drm/drm_panel.c         |   3 +
 drivers/gpu/drm/tegra/dpaux.c       |   3 +
 drivers/i2c/i2c-core.c              |   5 ++
 drivers/of/platform.c               |  61 +++++++++++++++++++++
 drivers/phy/phy-core.c              |   3 +
 drivers/pinctrl/devicetree.c        |   3 +
 drivers/power/power_supply_core.c   |   3 +
 drivers/pwm/core.c                  |   3 +
 drivers/regulator/core.c            | 106 +++++++++++++++++++++---------------
 drivers/usb/phy/phy.c               |   3 +
 drivers/video/backlight/backlight.c |   3 +
 include/linux/of.h                  |   1 +
 include/linux/of_platform.h         |   2 +
 sound/soc/soc-core.c                |   7 ++-
 18 files changed, 200 insertions(+), 46 deletions(-)

-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1201804 — [PATCH v3 02/18] of/platform: add of_platform_probe

Walks the OF tree up and finds the closest ancestor that has a platform
device associated with it, probing it if isn't bound to a driver yet.

The above should ensure that the dependency represented by the passed OF
node is available, because probing a platform device should cause its
descendants to be probed as well.

Subsystems can use this when looking up resources for drivers, to reduce
the chances of deferred probes because of the probing order of devices.

Also adds a platform_dev member to struct device_node, pointing to the
platform device that was registered based on that node, if any.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3:
- Set and use device_node.platform_dev instead of reversing the logic to
  find the platform device that encloses a device node.
- Drop the fwnode API to probe firmware nodes and add OF-only API for
  now. I think this same scheme could be used for machines with ACPI,
  but I haven't been able to find one that had to defer its probes because
  of the device probe order.

Changes in v2: None

 drivers/of/platform.c       | 61 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h          |  1 +
 include/linux/of_platform.h |  2 ++
 3 files changed, 64 insertions(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 8a002d6151f2..a1930c0d1fee 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -192,6 +192,8 @@ static struct platform_device *of_platform_device_create_pdata(
 		goto err_clear_flag;
 	}
 
+	np->platform_dev = dev;
+
 	return dev;
 
 err_clear_flag:
@@ -501,6 +503,65 @@ void of_platform_depopulate(struct device *parent)
 }
 EXPORT_SYMBOL_GPL(of_platform_depopulate);
 
+/**
+ * of_platform_probe() - Probe platform device associated with OF node
+ * @np: node to probe
+ *
+ * Walks the OF tree up and finds the closest ancestor that has a platform
+ * device associated with it, probing it if isn't bound to a driver yet.
+ *
+ * The above should ensure that the dependency represented by the passed OF
+ * node is available, because probing a platform device should cause its
+ * descendants to be probed as well.
+ */
+void of_platform_probe(struct device_node *np)
+{
+	struct device_node *target;
+	struct platform_device *pdev = NULL;
+
+	if (!of_root || !of_node_check_flag(of_root, OF_POPULATED_BUS))
+		return;
+
+	if (!np)
+		return;
+
+	of_node_get(np);
+
+	for (target = np;
+	     !of_node_is_root(target);
+	     target = of_get_next_parent(target))
+		if (target->platform_dev) {
+			pdev = target->platform_dev;
+			break;
+		}
+
+	of_node_put(target);
+
+	if (!pdev) {
+		pr_warn("Couldn't find a platform device for node '%s'\n",
+			of_node_full_name(np));
+		return;
+	}
+
+	/*
+	 * Device is bound or is being probed right now. If we have bad luck
+	 * and the dependency isn't ready when it's needed, deferred probe
+	 * will save us.
+	 */
+	if (pdev->dev.driver)
+		return;
+
+	if (device_attach(&pdev->dev) != 1)
+		/*
+		 * This cannot be a warning for now because clock nodes have a
+		 * compatible string but the clock framework doesn't follow the
+		 * device/driver model.
+		 */
+		pr_debug("Probe failed for %s (%s)\n", of_node_full_name(np),
+			 dev_name(&pdev->dev));
+}
+EXPORT_SYMBOL_GPL(of_platform_probe);
+
 #ifdef CONFIG_OF_DYNAMIC
 static int of_platform_notify(struct notifier_block *nb,
 				unsigned long action, void *arg)
diff --git a/include/linux/of.h b/include/linux/of.h
index edc068d19c79..2ace86a5fa2d 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -52,6 +52,7 @@ struct device_node {
 	phandle phandle;
 	const char *full_name;
 	struct fwnode_handle fwnode;
+	struct platform_device *platform_dev;
 
 	struct	property *properties;
 	struct	property *deadprops;	/* removed properties */
diff --git a/include/linux/of_platform.h b/include/linux/of_platform.h
index 611a691145c4..91ca92c7c061 100644
--- a/include/linux/of_platform.h
+++ b/include/linux/of_platform.h
@@ -73,6 +73,7 @@ extern int of_platform_populate(struct device_node *root,
 				const struct of_dev_auxdata *lookup,
 				struct device *parent);
 extern void of_platform_depopulate(struct device *parent);
+extern void of_platform_probe(struct device_node *np);
 #else
 static inline int of_platform_populate(struct device_node *root,
 					const struct of_device_id *matches,
@@ -82,6 +83,7 @@ static inline int of_platform_populate(struct device_node *root,
 	return -ENODEV;
 }
 static inline void of_platform_depopulate(struct device *parent) { }
+static inline void of_platform_probe(struct device_node *np) { }
 #endif
 
 #if defined(CONFIG_OF_DYNAMIC) && defined(CONFIG_OF_ADDRESS)
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1202640 — Re: [PATCH v3 02/18] of/platform: add of_platform_probe

[Multipart message — attachments visible in raw view] — view raw

On Thu, Aug 06, 2015 at 04:11:39PM +0200, Tomeu Vizoso wrote:

> Walks the OF tree up and finds the closest ancestor that has a platform
> device associated with it, probing it if isn't bound to a driver yet.

> The above should ensure that the dependency represented by the passed OF
> node is available, because probing a platform device should cause its
> descendants to be probed as well.

This sounds like it's going to break in the case where we have MFDs that
represent their functions in DT (not a pattern I'm a fan of but it's a
thing people do).  We'll walk back to the platform device for the MFD
function, try to probe it and then give up.  Perhaps that's good enough
anyway but it's not clear to me why we don't just try every parent we
find?

I'm also not a fan of the fact that the interface here is explicitly
saying that we want to probe a platform device, that's an implementation
detail that callers shouldn't need to know about.  From the point of
view of the callers what they're trying to do is kick any dependencies
into being instantiated, the fact that we currently try to accomplish it
with platform devices isn't something they care about.

[toc] | [prev] | [next] | [standalone]

#1204930 — Re: [PATCH v3 02/18] of/platform: add of_platform_probe

On 7 August 2015 at 14:19, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Aug 06, 2015 at 04:11:39PM +0200, Tomeu Vizoso wrote:
>
>> Walks the OF tree up and finds the closest ancestor that has a platform
>> device associated with it, probing it if isn't bound to a driver yet.
>
>> The above should ensure that the dependency represented by the passed OF
>> node is available, because probing a platform device should cause its
>> descendants to be probed as well.
>
> This sounds like it's going to break in the case where we have MFDs that
> represent their functions in DT (not a pattern I'm a fan of but it's a
> thing people do).  We'll walk back to the platform device for the MFD
> function, try to probe it and then give up.  Perhaps that's good enough
> anyway but it's not clear to me why we don't just try every parent we
> find?

Agreed. In the attempt at probing dependencies before a device is
probed, I considered that a device's parent is also a dependency and
that worked well. From what I saw, few devices will defer their probe
if their parent hasn't been probed yet, assuming that it will have
probed already. But with simple-mfd and simple-bus that shouldn't be
relied upon as things will break if their parents defer their probe.
With async probing enabled this failure scenario becomes more
probable.

> I'm also not a fan of the fact that the interface here is explicitly
> saying that we want to probe a platform device, that's an implementation
> detail that callers shouldn't need to know about.  From the point of
> view of the callers what they're trying to do is kick any dependencies
> into being instantiated, the fact that we currently try to accomplish it
> with platform devices isn't something they care about.

Agreed.

Thanks,

Tomeu
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201805 — [PATCH v3 05/18] regulator: core: Reduce critical area in _regulator_get

...by moving the locking of regulator_list_mutex into
regulator_dev_lookup(), where it is iterated over.

The regulator device lock gets acquired before returning, and the caller
is responsible for releasing it after it's done with the regulator
device.

In _regulator_get() the regulator_list_mutex mutex is held for most of
the function, but is only strictly needed to protect the list lookups.

This change would be useful if for example regulator devices could be
registered on demand when a driver requests them. regulator_register()
could end up being called from within _regulator_get while the lock on
regulator_list_mutex is being held, causing a deadlock.

This backtrace illustrates the situation described above:

(regulator_register) from [<c05efe64>]
(devm_regulator_register+0x48/0x84)
(devm_regulator_register) from [<c05f0b20>]
(reg_fixed_voltage_probe+0x214/0x35c)
(reg_fixed_voltage_probe) from [<c06cc7fc>]
(platform_drv_probe+0x54/0xbc)
(platform_drv_probe) from [<c06caac8>] (driver_probe_device+0x184/0x2c4)
(driver_probe_device) from [<c06cac58>] (__device_attach+0x50/0x54)
(__device_attach) from [<c06c8eac>] (bus_for_each_drv+0x70/0xa4)
(bus_for_each_drv) from [<c06ca900>] (device_attach+0x90/0xa4)
(device_attach) from [<c06c9eb4>] (bus_probe_device+0x94/0xb8)
(bus_probe_device) from [<c06c7de8>] (device_add+0x384/0x580)
(device_add) from [<c095c104>] (of_device_add+0x44/0x4c)
(of_device_add) from [<c095c968>]
...
(regulator_dev_lookup) from [<c05ee7c0>] (_regulator_get+0x8c/0x26c)
(_regulator_get) from [<c05ee9c0>] (regulator_get+0x20/0x24)
(regulator_get) from [<c05efb1c>] (_devm_regulator_get+0xa4/0xc8)
(_devm_regulator_get) from [<c05efb5c>] (devm_regulator_get+0x1c/0x20)
(devm_regulator_get) from [<c06ba870>] (tegra_hdmi_probe+0xe0/0x278)
(tegra_hdmi_probe) from [<c06cc7fc>] (platform_drv_probe+0x54/0xbc)

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3:
- Avoid unlocking the regulator device's mutex if we don't have a device

Changes in v2:
- Acquire regulator device lock before returning from regulator_dev_lookup()

 drivers/regulator/core.c | 104 +++++++++++++++++++++++++++--------------------
 1 file changed, 59 insertions(+), 45 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 613034667b93..5f6c76142b08 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -110,6 +110,7 @@ static struct regulator *create_regulator(struct regulator_dev *rdev,
 					  struct device *dev,
 					  const char *supply_name);
 static void _regulator_put(struct regulator *regulator);
+static int _regulator_enable(struct regulator *regulator, bool do_lock);
 
 static const char *rdev_get_name(struct regulator_dev *rdev)
 {
@@ -1212,6 +1213,7 @@ static void unset_regulator_supplies(struct regulator_dev *rdev)
 
 #define REG_STR_SIZE	64
 
+/* rdev->mutex held by caller */
 static struct regulator *create_regulator(struct regulator_dev *rdev,
 					  struct device *dev,
 					  const char *supply_name)
@@ -1224,7 +1226,7 @@ static struct regulator *create_regulator(struct regulator_dev *rdev,
 	if (regulator == NULL)
 		return NULL;
 
-	mutex_lock(&rdev->mutex);
+	lockdep_assert_held_once(&rdev->mutex);
 	regulator->rdev = rdev;
 	list_add(&regulator->list, &rdev->consumer_list);
 
@@ -1276,12 +1278,10 @@ static struct regulator *create_regulator(struct regulator_dev *rdev,
 	    _regulator_is_enabled(rdev))
 		regulator->always_on = true;
 
-	mutex_unlock(&rdev->mutex);
 	return regulator;
 overflow_err:
 	list_del(&regulator->list);
 	kfree(regulator);
-	mutex_unlock(&rdev->mutex);
 	return NULL;
 }
 
@@ -1320,6 +1320,7 @@ static void regulator_supply_alias(struct device **dev, const char **supply)
 	}
 }
 
+/* Caller has to release r->mutex once done with r */
 static struct regulator_dev *regulator_dev_lookup(struct device *dev,
 						  const char *supply,
 						  int *ret)
@@ -1335,10 +1336,15 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
 	if (dev && dev->of_node) {
 		node = of_get_regulator(dev, supply);
 		if (node) {
+			mutex_lock(&regulator_list_mutex);
 			list_for_each_entry(r, &regulator_list, list)
 				if (r->dev.parent &&
-					node == r->dev.of_node)
+					node == r->dev.of_node) {
+					mutex_lock(&r->mutex);
+					mutex_unlock(&regulator_list_mutex);
 					return r;
+				}
+			mutex_unlock(&regulator_list_mutex);
 			*ret = -EPROBE_DEFER;
 			return NULL;
 		} else {
@@ -1356,9 +1362,14 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
 	if (dev)
 		devname = dev_name(dev);
 
+	mutex_lock(&regulator_list_mutex);
 	list_for_each_entry(r, &regulator_list, list)
-		if (strcmp(rdev_get_name(r), supply) == 0)
+		if (strcmp(rdev_get_name(r), supply) == 0) {
+			mutex_lock(&r->mutex);
+			mutex_unlock(&regulator_list_mutex);
 			return r;
+		}
+	mutex_unlock(&regulator_list_mutex);
 
 	list_for_each_entry(map, &regulator_map_list, list) {
 		/* If the mapping has a device set up it must match */
@@ -1400,6 +1411,7 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 	if (!r) {
 		if (have_full_constraints()) {
 			r = dummy_regulator_rdev;
+			mutex_lock(&r->mutex);
 		} else {
 			dev_err(dev, "Failed to resolve %s-supply for %s\n",
 				rdev->supply_name, rdev->desc->name);
@@ -1410,23 +1422,25 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 	/* Recursively resolve the supply of the supply */
 	ret = regulator_resolve_supply(r);
 	if (ret < 0)
-		return ret;
+		goto out;
 
 	ret = set_supply(rdev, r);
 	if (ret < 0)
-		return ret;
+		goto out;
 
 	/* Cascade always-on state to supply */
 	if (_regulator_is_enabled(rdev)) {
-		ret = regulator_enable(rdev->supply);
+		ret = _regulator_enable(rdev->supply, false);
 		if (ret < 0) {
 			if (rdev->supply)
 				_regulator_put(rdev->supply);
-			return ret;
+			goto out;
 		}
 	}
 
-	return 0;
+out:
+	mutex_unlock(&r->mutex);
+	return ret;
 }
 
 /* Internal regulator request function */
@@ -1451,8 +1465,6 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
 	else
 		ret = -EPROBE_DEFER;
 
-	mutex_lock(&regulator_list_mutex);
-
 	rdev = regulator_dev_lookup(dev, id, &ret);
 	if (rdev)
 		goto found;
@@ -1464,7 +1476,7 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
 	 * succeed, so, quit with appropriate error value
 	 */
 	if (ret && ret != -ENODEV)
-		goto out;
+		return regulator;
 
 	if (!devname)
 		devname = "deviceless";
@@ -1478,13 +1490,13 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
 			devname, id);
 
 		rdev = dummy_regulator_rdev;
+		mutex_lock(&rdev->mutex);
 		goto found;
 	/* Don't log an error when called from regulator_get_optional() */
 	} else if (!have_full_constraints() || exclusive) {
 		dev_warn(dev, "dummy supplies not allowed\n");
 	}
 
-	mutex_unlock(&regulator_list_mutex);
 	return regulator;
 
 found:
@@ -1526,8 +1538,7 @@ found:
 	}
 
 out:
-	mutex_unlock(&regulator_list_mutex);
-
+	mutex_unlock(&rdev->mutex);
 	return regulator;
 }
 
@@ -1986,12 +1997,24 @@ static int _regulator_do_enable(struct regulator_dev *rdev)
 	return 0;
 }
 
-/* locks held by regulator_enable() */
-static int _regulator_enable(struct regulator_dev *rdev)
+static int _regulator_enable(struct regulator *regulator, bool do_lock)
 {
-	int ret;
+	struct regulator_dev *rdev = regulator->rdev;
+	int ret = 0;
 
-	lockdep_assert_held_once(&rdev->mutex);
+	if (regulator->always_on)
+		return 0;
+
+	if (rdev->supply) {
+		ret = regulator_enable(rdev->supply);
+		if (ret != 0)
+			return ret;
+	}
+
+	if (do_lock)
+		mutex_lock(&rdev->mutex);
+	else
+		lockdep_assert_held_once(&rdev->mutex);
 
 	/* check voltage and requested load before enabling */
 	if (rdev->constraints &&
@@ -2002,23 +2025,33 @@ static int _regulator_enable(struct regulator_dev *rdev)
 		/* The regulator may on if it's not switchable or left on */
 		ret = _regulator_is_enabled(rdev);
 		if (ret == -EINVAL || ret == 0) {
-			if (!_regulator_can_change_status(rdev))
-				return -EPERM;
+			if (!_regulator_can_change_status(rdev)) {
+				ret = -EPERM;
+				goto out;
+			}
 
 			ret = _regulator_do_enable(rdev);
 			if (ret < 0)
-				return ret;
+				goto out;
 
 		} else if (ret < 0) {
 			rdev_err(rdev, "is_enabled() failed: %d\n", ret);
-			return ret;
+			goto out;
 		}
 		/* Fallthrough on positive return values - already enabled */
+		ret = 0;
 	}
 
 	rdev->use_count++;
 
-	return 0;
+out:
+	if (do_lock)
+		mutex_unlock(&rdev->mutex);
+
+	if (ret != 0 && rdev->supply)
+		regulator_disable(rdev->supply);
+
+	return ret;
 }
 
 /**
@@ -2034,26 +2067,7 @@ static int _regulator_enable(struct regulator_dev *rdev)
  */
 int regulator_enable(struct regulator *regulator)
 {
-	struct regulator_dev *rdev = regulator->rdev;
-	int ret = 0;
-
-	if (regulator->always_on)
-		return 0;
-
-	if (rdev->supply) {
-		ret = regulator_enable(rdev->supply);
-		if (ret != 0)
-			return ret;
-	}
-
-	mutex_lock(&rdev->mutex);
-	ret = _regulator_enable(rdev);
-	mutex_unlock(&rdev->mutex);
-
-	if (ret != 0 && rdev->supply)
-		regulator_disable(rdev->supply);
-
-	return ret;
+	return _regulator_enable(regulator, true);
 }
 EXPORT_SYMBOL_GPL(regulator_enable);
 
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1202624 — Re: [PATCH v3 05/18] regulator: core: Reduce critical area in _regulator_get

[Multipart message — attachments visible in raw view] — view raw

On Thu, Aug 06, 2015 at 04:11:42PM +0200, Tomeu Vizoso wrote:

> This backtrace illustrates the situation described above:
> 
> (regulator_register) from [<c05efe64>]
> (devm_regulator_register+0x48/0x84)

Please don't paste entire backtraces into commit messages, they are
enormous and contain very little useful content - they just obscure
actual information in the commit message.  If you feel there's useful
information in there just include edited highlights with only that.

> +static int _regulator_enable(struct regulator *regulator, bool do_lock)
>  {
> -	int ret;
> +	struct regulator_dev *rdev = regulator->rdev;
> +	int ret = 0;
>  
> -	lockdep_assert_held_once(&rdev->mutex);
> +	if (regulator->always_on)
> +		return 0;
> +
> +	if (rdev->supply) {
> +		ret = regulator_enable(rdev->supply);
> +		if (ret != 0)
> +			return ret;
> +	}
> +
> +	if (do_lock)
> +		mutex_lock(&rdev->mutex);
> +	else
> +		lockdep_assert_held_once(&rdev->mutex);

Eew.  This do_lock stuff is *not* nice and going to be fragile.  I'm not
a fan, we need something better.

[toc] | [prev] | [next] | [standalone]

#1201806 — [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

Delay matches of platform devices with OF nodes until late_initcall,
when we are sure that all built-in drivers have been registered already.
This is needed to prevent deferred probes because of some drivers not
having registered yet.

The reason why only platform devices are delayed is that some other
devices are expected to be probed earlier than late_initcall, for
example, the system PNP driver needs to probe its devices in
fs_initcall.

Additionally, only platform devices with OF nodes are delayed because
some machines may depend on oter platform devices being registered at
specific times.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v3:
- Only delay platform devices with OF nodes

Changes in v2:
- Move delay to platform.c

 drivers/base/platform.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 063f0ab15259..0848ece2bf9d 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -33,6 +33,8 @@
 /* For automatically allocated device IDs */
 static DEFINE_IDA(platform_devid_ida);
 
+static bool enable_matches;
+
 struct device platform_bus = {
 	.init_name	= "platform",
 };
@@ -839,6 +841,15 @@ static int platform_match(struct device *dev, struct device_driver *drv)
 	struct platform_device *pdev = to_platform_device(dev);
 	struct platform_driver *pdrv = to_platform_driver(drv);
 
+	/*
+	 * Delay matches of OF platform devices until late_initcall, when we
+	 * are sure that all built-in drivers have been registered already.
+	 * This is needed to prevent deferred probes because of some drivers
+	 * not having registered yet.
+	 */
+	if (dev->of_node && !enable_matches)
+		return 0;
+
 	/* When driver_override is set, only bind to the matching driver */
 	if (pdev->driver_override)
 		return !strcmp(pdev->driver_override, drv->name);
@@ -859,6 +870,24 @@ static int platform_match(struct device *dev, struct device_driver *drv)
 	return (strcmp(pdev->name, drv->name) == 0);
 }
 
+static int __probe_device(struct device *dev, void *data)
+{
+	if (dev->of_node)
+		device_initial_probe(dev);
+
+	return 0;
+}
+
+static int probe_delayed_matches_initcall(void)
+{
+	enable_matches = true;
+
+	bus_for_each_dev(&platform_bus_type, NULL, NULL, __probe_device);
+
+	return 0;
+}
+late_initcall(probe_delayed_matches_initcall);
+
 #ifdef CONFIG_PM_SLEEP
 
 static int platform_legacy_suspend(struct device *dev, pm_message_t mesg)
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201995 — Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> Delay matches of platform devices with OF nodes until late_initcall,
> when we are sure that all built-in drivers have been registered already.
> This is needed to prevent deferred probes because of some drivers not
> having registered yet.
>
> The reason why only platform devices are delayed is that some other
> devices are expected to be probed earlier than late_initcall, for
> example, the system PNP driver needs to probe its devices in
> fs_initcall.
>
> Additionally, only platform devices with OF nodes are delayed because
> some machines may depend on oter platform devices being registered at
> specific times.

How do we know that these probes occur before the unused clocks and
regulators are turned off? Just getting lucky (as is deferred probe)?
Can we do this one level earlier so we have a level left to do things
after probe.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1202341 — Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

On 6 August 2015 at 22:19, Rob Herring <robherring2@gmail.com> wrote:
> On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>> Delay matches of platform devices with OF nodes until late_initcall,
>> when we are sure that all built-in drivers have been registered already.
>> This is needed to prevent deferred probes because of some drivers not
>> having registered yet.
>>
>> The reason why only platform devices are delayed is that some other
>> devices are expected to be probed earlier than late_initcall, for
>> example, the system PNP driver needs to probe its devices in
>> fs_initcall.
>>
>> Additionally, only platform devices with OF nodes are delayed because
>> some machines may depend on oter platform devices being registered at
>> specific times.
>
> How do we know that these probes occur before the unused clocks and
> regulators are turned off? Just getting lucky (as is deferred probe)?
> Can we do this one level earlier so we have a level left to do things
> after probe.

Those are already late_initcall_sync so I guess we're fine.

Thanks,

Tomeu

> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1202935 — Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

On 08/07/2015 10:11 AM, Tomeu Vizoso wrote:
> On 6 August 2015 at 22:19, Rob Herring <robherring2@gmail.com> wrote:
>> On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>>> Delay matches of platform devices with OF nodes until late_initcall,
>>> when we are sure that all built-in drivers have been registered already.
>>> This is needed to prevent deferred probes because of some drivers not
>>> having registered yet.
>>>
>>> The reason why only platform devices are delayed is that some other
>>> devices are expected to be probed earlier than late_initcall, for
>>> example, the system PNP driver needs to probe its devices in
>>> fs_initcall.
>>>
>>> Additionally, only platform devices with OF nodes are delayed because
>>> some machines may depend on oter platform devices being registered at
>>> specific times.
>>
>> How do we know that these probes occur before the unused clocks and
>> regulators are turned off? Just getting lucky (as is deferred probe)?
>> Can we do this one level earlier so we have a level left to do things
>> after probe.
> 
> Those are already late_initcall_sync so I guess we're fine.

I wouldn't be so sure :(
FYI:
http://git.ti.com/ti-linux-kernel/ti-linux-kernel/commit/763d643bbfc0f445c6685c541fcae3c370e4314a


-- 
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1203639 — Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

On 7 August 2015 at 19:06, Grygorii Strashko <grygorii.strashko@ti.com> wrote:
> On 08/07/2015 10:11 AM, Tomeu Vizoso wrote:
>> On 6 August 2015 at 22:19, Rob Herring <robherring2@gmail.com> wrote:
>>> On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>>>> Delay matches of platform devices with OF nodes until late_initcall,
>>>> when we are sure that all built-in drivers have been registered already.
>>>> This is needed to prevent deferred probes because of some drivers not
>>>> having registered yet.
>>>>
>>>> The reason why only platform devices are delayed is that some other
>>>> devices are expected to be probed earlier than late_initcall, for
>>>> example, the system PNP driver needs to probe its devices in
>>>> fs_initcall.
>>>>
>>>> Additionally, only platform devices with OF nodes are delayed because
>>>> some machines may depend on oter platform devices being registered at
>>>> specific times.
>>>
>>> How do we know that these probes occur before the unused clocks and
>>> regulators are turned off? Just getting lucky (as is deferred probe)?
>>> Can we do this one level earlier so we have a level left to do things
>>> after probe.
>>
>> Those are already late_initcall_sync so I guess we're fine.
>
> I wouldn't be so sure :(
> FYI:
> http://git.ti.com/ti-linux-kernel/ti-linux-kernel/commit/763d643bbfc0f445c6685c541fcae3c370e4314a

If I understand the situation correctly, this is one more instance of
starting to do some work at some point and hoping that something else
that started before has already finished happening. If that's the
case, how does this series make that worst?

During this development I have found many hacks intended to put some
order, even if not enough care was taken to make sure that the order
was guaranteed. In general I would recommend for moving code into
proper drivers and have them to defer the probe of their devices if
some dependency isn't fulfilled at that moment.

Once that's done and we have a safe and reliable boot, we can avoid
those deferred probes by fulfilling the dependency on-demand as this
series shows.

There was some recent thread about how the disabling of unused clocks
and regulators isn't really safe because after late_initcall_sync more
drivers can be registered from modules. Furthermore, there's async
device probes.

Regards,

Tomeu

>
> --
> regards,
> -grygorii
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1204076 — Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

[Multipart message — attachments visible in raw view] — view raw

On Sun, Aug 09, 2015 at 03:03:14PM +0200, Tomeu Vizoso wrote:

> There was some recent thread about how the disabling of unused clocks
> and regulators isn't really safe because after late_initcall_sync more
> drivers can be registered from modules. Furthermore, there's async
> device probes.

What recent thread and why would that be unsafe?  Any driver using a
clock or regulator ought to be making sure that the clock or regulator
is enabled before it tries to use it.  The worst that should happen is
that something gets the power bounced during boot which isn't the end of
the world.

[toc] | [prev] | [next] | [standalone]

#1207875 — Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall

Hi Tomeu,

On 08/09/2015 04:03 PM, Tomeu Vizoso wrote:
> On 7 August 2015 at 19:06, Grygorii Strashko <grygorii.strashko@ti.com> wrote:
>> On 08/07/2015 10:11 AM, Tomeu Vizoso wrote:
>>> On 6 August 2015 at 22:19, Rob Herring <robherring2@gmail.com> wrote:
>>>> On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>>>>> Delay matches of platform devices with OF nodes until late_initcall,
>>>>> when we are sure that all built-in drivers have been registered already.
>>>>> This is needed to prevent deferred probes because of some drivers not
>>>>> having registered yet.
>>>>>
>>>>> The reason why only platform devices are delayed is that some other
>>>>> devices are expected to be probed earlier than late_initcall, for
>>>>> example, the system PNP driver needs to probe its devices in
>>>>> fs_initcall.
>>>>>
>>>>> Additionally, only platform devices with OF nodes are delayed because
>>>>> some machines may depend on oter platform devices being registered at
>>>>> specific times.
>>>>
>>>> How do we know that these probes occur before the unused clocks and
>>>> regulators are turned off? Just getting lucky (as is deferred probe)?
>>>> Can we do this one level earlier so we have a level left to do things
>>>> after probe.
>>>
>>> Those are already late_initcall_sync so I guess we're fine.
>>
>> I wouldn't be so sure :(
>> FYI:
>> http://git.ti.com/ti-linux-kernel/ti-linux-kernel/commit/763d643bbfc0f445c6685c541fcae3c370e4314a

I'm very sorry for delayed reply.

First of all, I'd like to say that proposition to move drivers probing/matching 
one layer early (device_initcall_sync) is reasonable 
(and I've already mentioned the same here https://lkml.org/lkml/2015/6/3/979).

> 
> If I understand the situation correctly, this is one more instance of
> starting to do some work at some point and hoping that something else
> that started before has already finished happening. If that's the
> case, how does this series make that worst?

You going to increase pressure on the late boot stages without taking
into account that there could be platforms which perfectly works
just using init_calls. And the fact that drivers will be not probed at
expected time could be very surprising for them.

Or, there should be an option to disable this functionality.

> 
> During this development I have found many hacks intended to put some
> order, even if not enough care was taken to make sure that the order
> was guaranteed. In general I would recommend for moving code into
> proper drivers and have them to defer the probe of their devices if
> some dependency isn't fulfilled at that moment.

Unfortunately, not everything can be moved to drivers or it could be
just unreasonable. 

> 
> Once that's done and we have a safe and reliable boot, we can avoid
> those deferred probes by fulfilling the dependency on-demand as this
> series shows.
> 
> There was some recent thread about how the disabling of unused clocks
> and regulators isn't really safe because after late_initcall_sync more
> drivers can be registered from modules. Furthermore, there's async
> device probes.

Clocks and regulators are safe, problems may happen first of all with 
platform's specific initcalls.


Just take a look how many late_initcalls defined now in Kernel
(grep -w -RI  "late_initcall" ./*) and everything which belongs
to  /arch/* will be called before your probe_delayed_matches_initcall().
The sequence of late_initcalls in /drivers/* is defined by
/drivers/Makefile. 

#
./arch/um/kernel/process.c:late_initcall(make_proc_sysemu);
./arch/um/drivers/umcast_kern.c:late_initcall(register_umcast);
./arch/um/drivers/stdio_console.c:late_initcall(stdio_init);
./arch/um/drivers/pcap_kern.c:late_initcall(register_pcap);
./arch/um/drivers/ssl.c:late_initcall(ssl_init);
./arch/um/drivers/slip_kern.c:late_initcall(register_slip);
./arch/um/drivers/vde_kern.c:late_initcall(register_vde);
./arch/um/drivers/mconsole_kern.c:late_initcall(mc_add_console);
./arch/um/drivers/ubd_kern.c:late_initcall(ubd_init);
./arch/um/drivers/slirp_kern.c:late_initcall(register_slirp);
./arch/um/drivers/daemon_kern.c:late_initcall(register_daemon);
./arch/um/os-Linux/drivers/tuntap_kern.c:late_initcall(register_tuntap);
./arch/um/os-Linux/drivers/ethertap_kern.c:late_initcall(register_ethertap);
./arch/arm/kernel/pj4-cp0.c:late_initcall(pj4_cp0_init);
./arch/arm/kernel/setup.c:late_initcall(init_machine_late);
./arch/arm/kernel/xscale-cp0.c:late_initcall(xscale_cp0_init);
./arch/arm/kernel/swp_emulate.c:late_initcall(swp_emulation_init);
./arch/arm/kernel/thumbee.c:late_initcall(thumbee_init);
./arch/arm/kernel/traps.c:late_initcall(arm_mrc_hook_init);
./arch/arm/mach-mmp/pm-pxa910.c:late_initcall(pxa910_pm_init);
./arch/arm/mach-mmp/pm-mmp2.c:late_initcall(mmp2_pm_init);
./arch/arm/common/bL_switcher.c:late_initcall(bL_switcher_init);
./arch/arm/mach-pxa/sharpsl_pm.c:late_initcall(sharpsl_pm_init);
./arch/arm/kvm/coproc_a15.c:late_initcall(coproc_a15_init);
./arch/arm/kvm/coproc_a7.c:late_initcall(coproc_a7_init);
./arch/arm/mach-omap1/clock.c:late_initcall(clk_disable_unused);
./arch/arm/mach-omap1/clock.c:late_initcall(omap_clk_enable_autoidle_all);
./arch/arm/mach-omap1/clock.c:late_initcall(clk_debugfs_init);
./arch/arm/xen/enlighten.c:late_initcall(xen_pm_init);
./arch/arm/probes/kprobes/test-core.c:late_initcall(run_all_tests);
./arch/arm/mach-mvebu/pm-board.c:late_initcall(mvebu_armada_xp_gp_pm_init);
./arch/unicore32/kernel/fpu-ucf64.c:late_initcall(ucf64_init);
./arch/avr32/mm/tlb.c:late_initcall(proctlb_init);
./arch/sh/kernel/cpu/sh4a/smp-shx3.c:late_initcall(register_shx3_cpu_notifier);
./arch/sh/kernel/cpu/shmobile/pm.c:late_initcall(sh_pm_init);
./arch/sh/boards/mach-hp6xx/pm.c:late_initcall(hp6x0_pm_init);
./arch/xtensa/platforms/iss/console.c:late_initcall(rs_init);
./arch/frv/kernel/setup.c:late_initcall(setup_arch_serial);
./arch/powerpc/kernel/machine_kexec.c:late_initcall(kexec_setup);
./arch/powerpc/kernel/machine_kexec_64.c:late_initcall(export_htab_values);
./arch/powerpc/kernel/setup-common.c:late_initcall(check_cache_coherency);
./arch/powerpc/kernel/iommu.c:late_initcall(fail_iommu_debugfs);
./arch/powerpc/lib/code-patching.c:late_initcall(test_code_patching);
./arch/powerpc/lib/feature-fixups.c:late_initcall(test_feature_fixups);
./arch/powerpc/sysdev/ppc4xx_cpm.c:late_initcall(cpm_init);
./arch/powerpc/sysdev/dart_iommu.c:late_initcall(iommu_init_late_dart);
./arch/powerpc/sysdev/msi_bitmap.c:late_initcall(msi_bitmap_selftest);
./arch/tile/kernel/hardwall.c:late_initcall(dev_hardwall_init);
./arch/tile/kernel/smpboot.c:late_initcall(reset_init_affinity);
./arch/arm64/kernel/fpsimd.c:late_initcall(fpsimd_init);
./arch/arm64/kernel/armv8_deprecated.c:late_initcall(armv8_deprecated_init);
./arch/arm64/kvm/sys_regs_generic_v8.c:late_initcall(sys_reg_genericv8_init);
./arch/m68k/kernel/early_printk.c:late_initcall(unregister_early_console);
./arch/sparc/kernel/leon_pmc.c:late_initcall(leon_pmc_install);
./arch/sparc/kernel/sstate.c:late_initcall(sstate_running);
./arch/metag/kernel/setup.c:late_initcall(init_machine_late);
./arch/x86/kernel/tboot.c:late_initcall(tboot_late_init);
./arch/x86/kernel/cpu/mcheck/mce-severity.c:late_initcall(severities_debugfs_init);
./arch/x86/kernel/cpu/mcheck/mce_amd.c:late_initcall(threshold_init_device);
./arch/x86/kernel/cpu/mcheck/mce.c:late_initcall(mcheck_debugfs_init);
./arch/x86/kernel/mpparse.c:late_initcall(update_mp_table);
./arch/x86/kernel/apic/x2apic_uv_x.c:late_initcall(uv_init_heartbeat);
./arch/x86/kernel/apic/apic.c:late_initcall(lapic_insert_resource);
./arch/x86/kernel/apic/vector.c:late_initcall(print_ICs);
./arch/x86/kernel/apic/probe_32.c:late_initcall(print_ipi_mode);
./arch/x86/kernel/acpi/boot.c:late_initcall(hpet_insert_resource);
./arch/x86/crypto/aesni-intel_glue.c:late_initcall(aesni_init);
./arch/x86/mm/tlb.c:late_initcall(create_tlb_single_page_flush_ceiling);
./arch/x86/mm/pat.c:late_initcall(pat_memtype_list_init);
./arch/x86/platform/intel-mid/device_libs/platform_gpio_keys.c:late_initcall(pb_keys_init);
./arch/x86/pci/mmconfig-shared.c:late_initcall(pci_mmcfg_late_insert_resources);
./arch/arc/kernel/setup.c:late_initcall(init_late_machine);
./arch/mips/alchemy/devboards/pm.c:late_initcall(pm_init);
./arch/mips/loongson64/common/mem.c:late_initcall(find_vga_mem_init);
./arch/mips/txx9/generic/setup_tx4927.c:late_initcall(tx4927_late_init);
./arch/mips/txx9/generic/setup_tx4939.c:late_initcall(tx4939_late_init);
./arch/mips/txx9/generic/setup_tx4938.c:late_initcall(tx4938_late_init);
./arch/mips/cavium-octeon/setup.c:late_initcall(octeon_no_pci_release);
./arch/mips/cavium-octeon/flash_setup.c:late_initcall(octeon_flash_init);
./arch/mips/cavium-octeon/smp.c:late_initcall(register_cavium_notifier);
./arch/mips/mti-malta/malta-pm.c:late_initcall(malta_pm_setup);
./arch/mips/jz4740/pm.c:late_initcall(jz4740_pm_init);
./arch/mips/sgi-ip27/ip27-timer.c:late_initcall(sgi_ip27_rtc_devinit);
./arch/blackfin/kernel/cplbinfo.c:late_initcall(cplbinfo_init);
./arch/blackfin/kernel/bfin_dma.c:late_initcall(proc_dma_init);
./arch/blackfin/kernel/nmi.c:late_initcall(init_nmi_wdt_syscore);
./arch/blackfin/mach-bf609/pm.c:late_initcall(bf609_init_pm);
./arch/blackfin/mm/sram-alloc.c:late_initcall(sram_proc_init);
./arch/blackfin/mm/isram-driver.c:late_initcall(isram_test_init);
./block/blk-core.c:late_initcall(fail_make_request_debugfs);
./block/blk-timeout.c:late_initcall(fail_io_timeout_debugfs);
./crypto/async_tx/raid6test.c:late_initcall(raid6_test);
./drivers/clk/clk.c:late_initcall(clk_debug_init);
./drivers/mtd/ubi/build.c:late_initcall(ubi_init);
./drivers/mtd/devices/block2mtd.c:late_initcall(block2mtd_init);
./drivers/devfreq/exynos/exynos5_bus.c:late_initcall(exynos5_busfreq_int_init);
./drivers/devfreq/exynos/exynos4_bus.c:late_initcall(exynos4_busfreq_init);
./drivers/gpu/drm/omapdrm/omap_drv.c:late_initcall(omap_drm_init);
./drivers/gpu/drm/gma500/psb_drv.c:late_initcall(psb_init);
./drivers/video/logo/logo.c:late_initcall(fb_logo_late_init);
./drivers/of/fdt.c:late_initcall(of_fdt_raw_init);
./drivers/base/dd.c:late_initcall(deferred_probe_initcall);
./drivers/base/power/trace.c:late_initcall(late_resume_init);
./drivers/base/power/domain.c:late_initcall(genpd_poweroff_unused);
./drivers/base/power/domain.c:late_initcall(pm_genpd_debug_init);
./drivers/mfd/wl1273-core.c:late_initcall(wl1273_core_init);
./drivers/cpufreq/speedstep-centrino.c:late_initcall(centrino_init);
./drivers/cpufreq/amd_freq_sensitivity.c:late_initcall(amd_freq_sensitivity_init);
./drivers/cpufreq/sfi-cpufreq.c:late_initcall(sfi_cpufreq_init);
./drivers/cpufreq/powernow-k8.c:late_initcall(powernowk8_init);
./drivers/cpufreq/acpi-cpufreq.c:late_initcall(acpi_cpufreq_init);
./drivers/cpufreq/ia64-acpi-cpufreq.c:late_initcall(acpi_cpufreq_init);
./drivers/cpufreq/p4-clockmod.c:late_initcall(cpufreq_p4_init);
./drivers/cpufreq/pcc-cpufreq.c:late_initcall(pcc_cpufreq_init);
./drivers/cpufreq/powernow-k7.c:late_initcall(powernow_init);
./drivers/cpufreq/longhaul.c:late_initcall(longhaul_init);
./drivers/cpufreq/s3c24xx-cpufreq-debugfs.c:late_initcall(s3c_freq_debugfs_init);
./drivers/cpufreq/at32ap-cpufreq.c:late_initcall(at32_cpufreq_init);
./drivers/cpufreq/s3c24xx-cpufreq.c:late_initcall(s3c_cpufreq_initcall);
./drivers/sh/clk/core.c:late_initcall(clk_late_init);
./drivers/sh/intc/userimask.c:late_initcall(userimask_sysdev_init);
./drivers/net/netconsole.c:late_initcall(init_netconsole);
./drivers/net/vxlan.c:late_initcall(vxlan_init_module);
./drivers/net/geneve.c:late_initcall(geneve_init_module);
./drivers/net/ethernet/ti/davinci_emac.c:late_initcall(davinci_emac_init);
./drivers/net/ethernet/ti/cpsw.c:late_initcall(cpsw_init);
./drivers/net/rionet.c:late_initcall(rionet_init);
./drivers/gpio/gpio-tegra.c:late_initcall(tegra_gpio_debuginit);
./drivers/block/hd.c:late_initcall(hd_init);
./drivers/thermal/db8500_cpufreq_cooling.c:late_initcall(db8500_cpufreq_cooling_init);
./drivers/staging/android/sync_debug.c:late_initcall(sync_debugfs_init);
./drivers/staging/wilc1000/linux_wlan.c:late_initcall(init_wilc_driver);
./drivers/iommu/dmar.c:late_initcall(dmar_free_unused_resources);
./drivers/watchdog/ie6xx_wdt.c:late_initcall(ie6xx_wdt_init);
./drivers/watchdog/intel_scu_watchdog.c:late_initcall(intel_scu_watchdog_init);
./drivers/tty/serial/mpsc.c:late_initcall(mpsc_late_console_init);
./drivers/media/platform/omap/omap_vout.c:late_initcall(omap_vout_init);
./drivers/media/pci/cx25821/cx25821-alsa.c:late_initcall(cx25821_alsa_init);
./drivers/media/pci/saa7134/saa7134-alsa.c:late_initcall(saa7134_alsa_init);
./drivers/dma/dmatest.c:late_initcall(dmatest_init);
./drivers/rapidio/rio-scan.c:late_initcall(rio_basic_attach);
./drivers/xen/xenbus/xenbus_probe_frontend.c:late_initcall(boot_wait_for_devices);
./drivers/platform/x86/dell-laptop.c:late_initcall(dell_init);
./drivers/firmware/memmap.c:late_initcall(firmware_memmap_init);
./drivers/firmware/edd.c:late_initcall(edd_init);
./drivers/firmware/efi/reboot.c:late_initcall(efi_shutdown_init);
./drivers/input/keyboard/gpio_keys.c:late_initcall(gpio_keys_init);
./drivers/pci/pci.c:late_initcall(pci_resource_alignment_sysfs_init);
./drivers/pci/pci-sysfs.c:late_initcall(pci_sysfs_init);
./drivers/macintosh/via-pmu-led.c:late_initcall(via_pmu_led_init);
./drivers/macintosh/via-pmu-event.c:late_initcall(via_pmu_event_init);
./drivers/ide/sgiioc4.c:late_initcall(ioc4_ide_init); /* Call only after IDE init is done */
./drivers/ide/ide-cs.c:late_initcall(init_ide_cs);
./drivers/rtc/Kconfig:	  The driver for this RTC device must be loaded before late_initcall
./drivers/rtc/hctosys.c:late_initcall(rtc_hctosys);
./drivers/irqchip/irq-vic.c:late_initcall(vic_pm_init);
./drivers/power/avs/smartreflex.c:late_initcall(sr_init);
./drivers/power/charger-manager.c:late_initcall(charger_manager_init);
./fs/btrfs/super.c:late_initcall(init_btrfs_fs);
./fs/afs/main.c:late_initcall(afs_init);	/* must be called after net/ to create socket */
./fs/ubifs/super.c:late_initcall(ubifs_init);
./kernel/sched/core.c:late_initcall(sched_init_debug);
./kernel/time/timekeeping_debug.c:late_initcall(tk_debug_sleep_time_init);
./kernel/taskstats.c:late_initcall(taskstats_init);
./kernel/printk/printk.c:late_initcall(printk_late_init);
./kernel/bpf/arraymap.c:late_initcall(register_array_map);
./kernel/bpf/arraymap.c:late_initcall(register_prog_array_map);
./kernel/bpf/hashtab.c:late_initcall(register_htab_map);
./kernel/trace/trace_events.c:late_initcall(event_trace_self_tests_init);
./kernel/trace/bpf_trace.c:late_initcall(register_kprobe_prog_ops);
./kernel/trace/trace.c:late_initcall(clear_boot_tracer);
./kernel/trace/trace_kprobe.c:late_initcall(kprobe_trace_self_tests_init);
./kernel/trace/trace_events_filter.c:late_initcall(ftrace_test_event_filter);
./kernel/trace/trace_kdb.c:late_initcall(kdb_ftrace_register);
./kernel/trace/ring_buffer.c:late_initcall(test_ringbuffer);
./kernel/kprobes.c:late_initcall(debugfs_kprobe_init);
./kernel/rcu/update.c:late_initcall(rcu_verify_early_boot_tests);
./kernel/panic.c:late_initcall(init_oops_id);
./kernel/system_keyring.c:late_initcall(load_system_certificate_list);
./kernel/power/qos.c:late_initcall(pm_qos_power_init);
./kernel/power/Kconfig:	  late_initcall.
./kernel/power/main.c:late_initcall(pm_debugfs_init);
./kernel/power/suspend_test.c:late_initcall(test_suspend);
./lib/list_sort.c:late_initcall(list_sort_test);
./lib/random32.c:late_initcall(prandom_reseed);
./mm/kmemleak.c:late_initcall(kmemleak_late_init);
./mm/failslab.c:late_initcall(failslab_debugfs_init);
./mm/memory.c:late_initcall(fault_around_debugfs);
./mm/early_ioremap.c:late_initcall(check_early_ioremap_leak);
./mm/cma_debug.c:late_initcall(cma_debugfs_init);
./mm/page_owner.c:late_initcall(pageowner_init)
./mm/page_alloc.c:late_initcall(fail_page_alloc_debugfs);
./mm/swapfile.c:late_initcall(max_swapfiles_check);
./mm/zswap.c:late_initcall(init_zswap);
./net/ipv4/ipconfig.c:late_initcall(ip_auto_config);
./net/ipv4/tcp_cong.c:late_initcall(tcp_congestion_default);
./net/core/filter.c:late_initcall(register_sk_filter_ops);
./net/bluetooth/selftest.c:late_initcall(bt_selftest_init);
./security/integrity/evm/evm_main.c:late_initcall(init_evm);
./security/integrity/ima/ima_main.c:late_initcall(init_ima);	/* Start IMA after the TPM is available */
./security/apparmor/crypto.c:late_initcall(init_profile_hash);
./security/keys/trusted.c:late_initcall(init_trusted);
./security/keys/encrypted-keys/encrypted.c:late_initcall(init_encrypted);
./security/keys/process_keys.c:late_initcall(init_root_keyring);
./sound/soc/fsl/p1022_rdk.c:late_initcall(p1022_rdk_init);
./sound/soc/fsl/phycore-ac97.c:late_initcall(imx_phycore_init);

-- 
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201996

On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> Hello,
>
> I have a problem with the panel on my Tegra Chromebook taking longer
> than expected to be ready during boot (Stéphane Marchesin reported what
> is basically the same issue in [0]), and have looked into ordered
> probing as a better way of solving this than moving nodes around in the
> DT or playing with initcall levels and linking order.
>
> While reading the thread [1] that Alexander Holler started with his
> series to make probing order deterministic, it occurred to me that it
> should be possible to achieve the same by probing devices as they are
> referenced by other devices.
>
> This basically reuses the information that is already implicit in the
> probe() implementations, saving us from refactoring existing drivers or
> adding information to DTBs.
>
> During review of v1 of this series Linus Walleij suggested that it
> should be the device driver core to make sure that dependencies are
> ready before probing a device. I gave this idea a try [2] but Mark Brown
> pointed out to the logic duplication between the resource acquisition
> and dependency discovery code paths (though I think it's fairly minor).
>
> To address that code duplication I experimented with Arnd's devm_probe
> [3] concept of having drivers declare their dependencies instead of
> acquiring them during probe, and while it worked [4], I don't think we
> end up winning anything when compared to just probing devices on-demand
> from resource getters.
>
> One remaining objection is to the "sprinkling" of calls to
> fwnode_ensure_device() in the resource getters of each subsystem, but I
> think it's the right thing to do given that the storage of resources is
> currently subsystem-specific.
>
> We could avoid the above by moving resource storage into the core, but I
> don't think there's a compelling case for that.
>
> I have tested this on boards with Tegra, iMX.6, Exynos and OMAP SoCs,
> and these patches were enough to eliminate all the deferred probes
> (except one in PandaBoard because omap_dma_system doesn't have a
> firmware node as of yet).
>
> Have submitted a branch [5] with these patches to kernelci.org and I'm
> currently trying to fix all regressions, usually due to code assuming
> that devices will be probed in a specific order. Current results [6] are
> 348 passes, 30 fails and 42 unknowns (linux-next [7] is currently
> 387/3/23).

This is a bit worrying. If this causes a high number of boot failures,
fixing the errors you can find is not the path forward as we can't
test a lot of platforms (and many people don't look at -next). We may
want to put this behind a kconfig option so that we can easily restore
old behavior it needed. Otherwise, we could have to revert the series.

Are all the commits before this series fixing boot failures? You can't
do dts updates as the fix or backwards compatibility will be broken.

>
> With this series I get the kernel to output to the panel in 0.5s,
> instead of 2.8s.
>
> Regards,
>
> Tomeu
>
> [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
>
> [1] https://lkml.org/lkml/2014/5/12/452
>
> [2] https://lkml.org/lkml/2015/6/17/305
>
> [3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689
>
> [4] https://lkml.org/lkml/2015/7/21/441a
>
> [5] https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v5
>
> [6] http://kernelci.org/boot/all/job/collabora/kernel/v4.2-rc5-6548-g632b98c83840/
>
> [7] http://kernelci.org/boot/all/job/next/kernel/next-20150806/
>
> Changes in v3:
> - Only delay platform devices with OF nodes
> - Set and use device_node.platform_dev instead of reversing the logic to
>   find the platform device that encloses a device node.

I still want this to be a struct device and not a struct
platform_device and am not convinced it can't be. It can simply be an
optimization of the existing function:

struct platform_device *of_find_device_by_node(struct device_node *np)
{
  if (node->device && node->device->bus == &platform_bus_type)
    return to_platform_device(node->device);
  return NULL;
}

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1202322

On 6 August 2015 at 22:14, Rob Herring <robherring2@gmail.com> wrote:
> On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>> Hello,
>>
>> I have a problem with the panel on my Tegra Chromebook taking longer
>> than expected to be ready during boot (Stéphane Marchesin reported what
>> is basically the same issue in [0]), and have looked into ordered
>> probing as a better way of solving this than moving nodes around in the
>> DT or playing with initcall levels and linking order.
>>
>> While reading the thread [1] that Alexander Holler started with his
>> series to make probing order deterministic, it occurred to me that it
>> should be possible to achieve the same by probing devices as they are
>> referenced by other devices.
>>
>> This basically reuses the information that is already implicit in the
>> probe() implementations, saving us from refactoring existing drivers or
>> adding information to DTBs.
>>
>> During review of v1 of this series Linus Walleij suggested that it
>> should be the device driver core to make sure that dependencies are
>> ready before probing a device. I gave this idea a try [2] but Mark Brown
>> pointed out to the logic duplication between the resource acquisition
>> and dependency discovery code paths (though I think it's fairly minor).
>>
>> To address that code duplication I experimented with Arnd's devm_probe
>> [3] concept of having drivers declare their dependencies instead of
>> acquiring them during probe, and while it worked [4], I don't think we
>> end up winning anything when compared to just probing devices on-demand
>> from resource getters.
>>
>> One remaining objection is to the "sprinkling" of calls to
>> fwnode_ensure_device() in the resource getters of each subsystem, but I
>> think it's the right thing to do given that the storage of resources is
>> currently subsystem-specific.
>>
>> We could avoid the above by moving resource storage into the core, but I
>> don't think there's a compelling case for that.
>>
>> I have tested this on boards with Tegra, iMX.6, Exynos and OMAP SoCs,
>> and these patches were enough to eliminate all the deferred probes
>> (except one in PandaBoard because omap_dma_system doesn't have a
>> firmware node as of yet).
>>
>> Have submitted a branch [5] with these patches to kernelci.org and I'm
>> currently trying to fix all regressions, usually due to code assuming
>> that devices will be probed in a specific order. Current results [6] are
>> 348 passes, 30 fails and 42 unknowns (linux-next [7] is currently
>> 387/3/23).
>
> This is a bit worrying. If this causes a high number of boot failures,
> fixing the errors you can find is not the path forward as we can't
> test a lot of platforms (and many people don't look at -next). We may
> want to put this behind a kconfig option so that we can easily restore
> old behavior it needed. Otherwise, we could have to revert the series.

A Kconfig sounds fine to me. Altogether, I don't think it's that bad
because only these boards are known to have broken because of this
series:

at91-sama5d3_xplained
sama5d35ek

ste-snowball

vexpress-v2p-ca15
vexpress-v2p-ca15
vexpress-v2p-ca15_a7
vexpress-v2p-ca15-tc1
vexpress-v2p-ca9

I assume there's only 3 different bugs to fix there, plus a race in
imx boards that I have only papered over with a delay so far.

The failure rate seems to be so high because each boot is a
combination of board+defconfig and there are duplicated boards in
several labs and many were just offline at that moment.

But I agree that there's no way I can test it on all supported hw, so
a Kconfig that people can quickly switch on to disable the feature
sounds good to me.

> Are all the commits before this series fixing boot failures? You can't
> do dts updates as the fix or backwards compatibility will be broken.

The gpio-ranges fix for Tegra has a commit that safeguards backwards
compatibility, and the typo in regulator names for ux500 doesn't
really break anything that I can see, I just stumped into it when
trying to blindly fix the boot for ste-snowball (I don't have access
to that hw).

>> With this series I get the kernel to output to the panel in 0.5s,
>> instead of 2.8s.
>>
>> Regards,
>>
>> Tomeu
>>
>> [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
>>
>> [1] https://lkml.org/lkml/2014/5/12/452
>>
>> [2] https://lkml.org/lkml/2015/6/17/305
>>
>> [3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689
>>
>> [4] https://lkml.org/lkml/2015/7/21/441a
>>
>> [5] https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v5
>>
>> [6] http://kernelci.org/boot/all/job/collabora/kernel/v4.2-rc5-6548-g632b98c83840/
>>
>> [7] http://kernelci.org/boot/all/job/next/kernel/next-20150806/
>>
>> Changes in v3:
>> - Only delay platform devices with OF nodes
>> - Set and use device_node.platform_dev instead of reversing the logic to
>>   find the platform device that encloses a device node.
>
> I still want this to be a struct device and not a struct
> platform_device and am not convinced it can't be. It can simply be an
> optimization of the existing function:

Now I realize what you meant, that makes sense to me.

Thanks,

Tomeu

> struct platform_device *of_find_device_by_node(struct device_node *np)
> {
>   if (node->device && node->device->bus == &platform_bus_type)
>     return to_platform_device(node->device);
>   return NULL;
> }
>
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web