linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] driver core: platform: reorder functions
@ 2020-11-19 12:46 Uwe Kleine-König
  2020-11-19 12:46 ` [PATCH 2/3] driver core: platform: change logic implementing platform_driver_probe Uwe Kleine-König
  2020-11-19 12:46 ` [PATCH 3/3] driver core: platform: use bus_type functions Uwe Kleine-König
  0 siblings, 2 replies; 7+ messages in thread
From: Uwe Kleine-König @ 2020-11-19 12:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J . Wysocki 
  Cc: kernel, linux-kernel, Russell King

This way all callbacks and structures used to initialize
platform_bus_type are defined just before platform_bus_type and in the
same order. Also move platform_drv_probe_fail just before it's only
user.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/base/platform.c | 333 ++++++++++++++++++++--------------------
 1 file changed, 167 insertions(+), 166 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 88aef93eb4dd..b36ab5831cb6 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -772,11 +772,6 @@ static int platform_drv_probe(struct device *_dev)
 	return ret;
 }
 
-static int platform_drv_probe_fail(struct device *_dev)
-{
-	return -ENXIO;
-}
-
 static int platform_drv_remove(struct device *_dev)
 {
 	struct platform_driver *drv = to_platform_driver(_dev->driver);
@@ -827,6 +822,11 @@ void platform_driver_unregister(struct platform_driver *drv)
 }
 EXPORT_SYMBOL_GPL(platform_driver_unregister);
 
+static int platform_drv_probe_fail(struct device *_dev)
+{
+	return -ENXIO;
+}
+
 /**
  * __platform_driver_probe - register driver for non-hotpluggable device
  * @drv: platform driver structure
@@ -1017,129 +1017,6 @@ void platform_unregister_drivers(struct platform_driver * const *drivers,
 }
 EXPORT_SYMBOL_GPL(platform_unregister_drivers);
 
-/* modalias support enables more hands-off userspace setup:
- * (a) environment variable lets new-style hotplug events work once system is
- *     fully running:  "modprobe $MODALIAS"
- * (b) sysfs attribute lets new-style coldplug recover from hotplug events
- *     mishandled before system is fully running:  "modprobe $(cat modalias)"
- */
-static ssize_t modalias_show(struct device *dev,
-			     struct device_attribute *attr, char *buf)
-{
-	struct platform_device *pdev = to_platform_device(dev);
-	int len;
-
-	len = of_device_modalias(dev, buf, PAGE_SIZE);
-	if (len != -ENODEV)
-		return len;
-
-	len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1);
-	if (len != -ENODEV)
-		return len;
-
-	return sysfs_emit(buf, "platform:%s\n", pdev->name);
-}
-static DEVICE_ATTR_RO(modalias);
-
-static ssize_t driver_override_store(struct device *dev,
-				     struct device_attribute *attr,
-				     const char *buf, size_t count)
-{
-	struct platform_device *pdev = to_platform_device(dev);
-	char *driver_override, *old, *cp;
-
-	/* We need to keep extra room for a newline */
-	if (count >= (PAGE_SIZE - 1))
-		return -EINVAL;
-
-	driver_override = kstrndup(buf, count, GFP_KERNEL);
-	if (!driver_override)
-		return -ENOMEM;
-
-	cp = strchr(driver_override, '\n');
-	if (cp)
-		*cp = '\0';
-
-	device_lock(dev);
-	old = pdev->driver_override;
-	if (strlen(driver_override)) {
-		pdev->driver_override = driver_override;
-	} else {
-		kfree(driver_override);
-		pdev->driver_override = NULL;
-	}
-	device_unlock(dev);
-
-	kfree(old);
-
-	return count;
-}
-
-static ssize_t driver_override_show(struct device *dev,
-				    struct device_attribute *attr, char *buf)
-{
-	struct platform_device *pdev = to_platform_device(dev);
-	ssize_t len;
-
-	device_lock(dev);
-	len = sysfs_emit(buf, "%s\n", pdev->driver_override);
-	device_unlock(dev);
-
-	return len;
-}
-static DEVICE_ATTR_RW(driver_override);
-
-static ssize_t numa_node_show(struct device *dev,
-			      struct device_attribute *attr, char *buf)
-{
-	return sysfs_emit(buf, "%d\n", dev_to_node(dev));
-}
-static DEVICE_ATTR_RO(numa_node);
-
-static umode_t platform_dev_attrs_visible(struct kobject *kobj, struct attribute *a,
-		int n)
-{
-	struct device *dev = container_of(kobj, typeof(*dev), kobj);
-
-	if (a == &dev_attr_numa_node.attr &&
-			dev_to_node(dev) == NUMA_NO_NODE)
-		return 0;
-
-	return a->mode;
-}
-
-static struct attribute *platform_dev_attrs[] = {
-	&dev_attr_modalias.attr,
-	&dev_attr_numa_node.attr,
-	&dev_attr_driver_override.attr,
-	NULL,
-};
-
-static struct attribute_group platform_dev_group = {
-	.attrs = platform_dev_attrs,
-	.is_visible = platform_dev_attrs_visible,
-};
-__ATTRIBUTE_GROUPS(platform_dev);
-
-static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
-{
-	struct platform_device	*pdev = to_platform_device(dev);
-	int rc;
-
-	/* Some devices have extra OF data and an OF-style MODALIAS */
-	rc = of_device_uevent_modalias(dev, env);
-	if (rc != -ENODEV)
-		return rc;
-
-	rc = acpi_device_uevent_modalias(dev, env);
-	if (rc != -ENODEV)
-		return rc;
-
-	add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX,
-			pdev->name);
-	return 0;
-}
-
 static const struct platform_device_id *platform_match_id(
 			const struct platform_device_id *id,
 			struct platform_device *pdev)
@@ -1154,44 +1031,6 @@ static const struct platform_device_id *platform_match_id(
 	return NULL;
 }
 
-/**
- * platform_match - bind platform device to platform driver.
- * @dev: device.
- * @drv: driver.
- *
- * Platform device IDs are assumed to be encoded like this:
- * "<name><instance>", where <name> is a short description of the type of
- * device, like "pci" or "floppy", and <instance> is the enumerated
- * instance of the device, like '0' or '42'.  Driver IDs are simply
- * "<name>".  So, extract the <name> from the platform_device structure,
- * and compare it against the name of the driver. Return whether they match
- * or not.
- */
-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);
-
-	/* When driver_override is set, only bind to the matching driver */
-	if (pdev->driver_override)
-		return !strcmp(pdev->driver_override, drv->name);
-
-	/* Attempt an OF style match first */
-	if (of_driver_match_device(dev, drv))
-		return 1;
-
-	/* Then try ACPI style match */
-	if (acpi_driver_match_device(dev, drv))
-		return 1;
-
-	/* Then try to match against the id table */
-	if (pdrv->id_table)
-		return platform_match_id(pdrv->id_table, pdev) != NULL;
-
-	/* fall-back to driver name match */
-	return (strcmp(pdev->name, drv->name) == 0);
-}
-
 #ifdef CONFIG_PM_SLEEP
 
 static int platform_legacy_suspend(struct device *dev, pm_message_t mesg)
@@ -1336,6 +1175,168 @@ int platform_pm_restore(struct device *dev)
 
 #endif /* CONFIG_HIBERNATE_CALLBACKS */
 
+/* modalias support enables more hands-off userspace setup:
+ * (a) environment variable lets new-style hotplug events work once system is
+ *     fully running:  "modprobe $MODALIAS"
+ * (b) sysfs attribute lets new-style coldplug recover from hotplug events
+ *     mishandled before system is fully running:  "modprobe $(cat modalias)"
+ */
+static ssize_t modalias_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	int len;
+
+	len = of_device_modalias(dev, buf, PAGE_SIZE);
+	if (len != -ENODEV)
+		return len;
+
+	len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1);
+	if (len != -ENODEV)
+		return len;
+
+	return sysfs_emit(buf, "platform:%s\n", pdev->name);
+}
+static DEVICE_ATTR_RO(modalias);
+
+static ssize_t numa_node_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%d\n", dev_to_node(dev));
+}
+static DEVICE_ATTR_RO(numa_node);
+
+static ssize_t driver_override_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	ssize_t len;
+
+	device_lock(dev);
+	len = sysfs_emit(buf, "%s\n", pdev->driver_override);
+	device_unlock(dev);
+
+	return len;
+}
+
+static ssize_t driver_override_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	char *driver_override, *old, *cp;
+
+	/* We need to keep extra room for a newline */
+	if (count >= (PAGE_SIZE - 1))
+		return -EINVAL;
+
+	driver_override = kstrndup(buf, count, GFP_KERNEL);
+	if (!driver_override)
+		return -ENOMEM;
+
+	cp = strchr(driver_override, '\n');
+	if (cp)
+		*cp = '\0';
+
+	device_lock(dev);
+	old = pdev->driver_override;
+	if (strlen(driver_override)) {
+		pdev->driver_override = driver_override;
+	} else {
+		kfree(driver_override);
+		pdev->driver_override = NULL;
+	}
+	device_unlock(dev);
+
+	kfree(old);
+
+	return count;
+}
+static DEVICE_ATTR_RW(driver_override);
+
+static struct attribute *platform_dev_attrs[] = {
+	&dev_attr_modalias.attr,
+	&dev_attr_numa_node.attr,
+	&dev_attr_driver_override.attr,
+	NULL,
+};
+
+static umode_t platform_dev_attrs_visible(struct kobject *kobj, struct attribute *a,
+		int n)
+{
+	struct device *dev = container_of(kobj, typeof(*dev), kobj);
+
+	if (a == &dev_attr_numa_node.attr &&
+			dev_to_node(dev) == NUMA_NO_NODE)
+		return 0;
+
+	return a->mode;
+}
+
+static struct attribute_group platform_dev_group = {
+	.attrs = platform_dev_attrs,
+	.is_visible = platform_dev_attrs_visible,
+};
+__ATTRIBUTE_GROUPS(platform_dev);
+
+
+/**
+ * platform_match - bind platform device to platform driver.
+ * @dev: device.
+ * @drv: driver.
+ *
+ * Platform device IDs are assumed to be encoded like this:
+ * "<name><instance>", where <name> is a short description of the type of
+ * device, like "pci" or "floppy", and <instance> is the enumerated
+ * instance of the device, like '0' or '42'.  Driver IDs are simply
+ * "<name>".  So, extract the <name> from the platform_device structure,
+ * and compare it against the name of the driver. Return whether they match
+ * or not.
+ */
+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);
+
+	/* When driver_override is set, only bind to the matching driver */
+	if (pdev->driver_override)
+		return !strcmp(pdev->driver_override, drv->name);
+
+	/* Attempt an OF style match first */
+	if (of_driver_match_device(dev, drv))
+		return 1;
+
+	/* Then try ACPI style match */
+	if (acpi_driver_match_device(dev, drv))
+		return 1;
+
+	/* Then try to match against the id table */
+	if (pdrv->id_table)
+		return platform_match_id(pdrv->id_table, pdev) != NULL;
+
+	/* fall-back to driver name match */
+	return (strcmp(pdev->name, drv->name) == 0);
+}
+
+static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+	struct platform_device	*pdev = to_platform_device(dev);
+	int rc;
+
+	/* Some devices have extra OF data and an OF-style MODALIAS */
+	rc = of_device_uevent_modalias(dev, env);
+	if (rc != -ENODEV)
+		return rc;
+
+	rc = acpi_device_uevent_modalias(dev, env);
+	if (rc != -ENODEV)
+		return rc;
+
+	add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX,
+			pdev->name);
+	return 0;
+}
+
 int platform_dma_configure(struct device *dev)
 {
 	enum dev_dma_attr attr;

base-commit: 09162bc32c880a791c6c0668ce0745cf7958f576
-- 
2.28.0


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

* [PATCH 2/3] driver core: platform: change logic implementing platform_driver_probe
  2020-11-19 12:46 [PATCH 1/3] driver core: platform: reorder functions Uwe Kleine-König
@ 2020-11-19 12:46 ` Uwe Kleine-König
  2020-11-19 12:46 ` [PATCH 3/3] driver core: platform: use bus_type functions Uwe Kleine-König
  1 sibling, 0 replies; 7+ messages in thread
From: Uwe Kleine-König @ 2020-11-19 12:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J . Wysocki 
  Cc: kernel, linux-kernel, Russell King

Instead of overwriting the core driver's probe function handle probing
devices for drivers loaded by platform_driver_probe() in the platform
driver probe function.

The intended goal is to not have to change the probe function to
simplify converting the platform bus to use bus functions.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/base/platform.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index b36ab5831cb6..b847f5f8f992 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -743,12 +743,25 @@ struct platform_device *platform_device_register_full(
 }
 EXPORT_SYMBOL_GPL(platform_device_register_full);
 
+static int platform_probe_fail(struct platform_device *pdev);
+
 static int platform_drv_probe(struct device *_dev)
 {
 	struct platform_driver *drv = to_platform_driver(_dev->driver);
 	struct platform_device *dev = to_platform_device(_dev);
 	int ret;
 
+	/*
+	 * A driver registered using platform_driver_probe() cannot be bound
+	 * again later because the probe function usually lives in __init code
+	 * and so is gone. For these drivers .probe is set to
+	 * platform_probe_fail in __platform_driver_probe(). Don't even
+	 * prepare clocks and PM domains for these to match the traditional
+	 * behaviour.
+	 */
+	if (unlikely(drv->probe == platform_probe_fail))
+		return -ENXIO;
+
 	ret = of_clk_set_defaults(_dev->of_node, false);
 	if (ret < 0)
 		return ret;
@@ -822,7 +835,7 @@ void platform_driver_unregister(struct platform_driver *drv)
 }
 EXPORT_SYMBOL_GPL(platform_driver_unregister);
 
-static int platform_drv_probe_fail(struct device *_dev)
+static int platform_probe_fail(struct platform_device *pdev)
 {
 	return -ENXIO;
 }
@@ -887,10 +900,9 @@ int __init_or_module __platform_driver_probe(struct platform_driver *drv,
 	 * new devices fail.
 	 */
 	spin_lock(&drv->driver.bus->p->klist_drivers.k_lock);
-	drv->probe = NULL;
+	drv->probe = platform_probe_fail;
 	if (code == 0 && list_empty(&drv->driver.p->klist_devices.k_list))
 		retval = -ENODEV;
-	drv->driver.probe = platform_drv_probe_fail;
 	spin_unlock(&drv->driver.bus->p->klist_drivers.k_lock);
 
 	if (code != retval)
-- 
2.28.0


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

* [PATCH 3/3] driver core: platform: use bus_type functions
  2020-11-19 12:46 [PATCH 1/3] driver core: platform: reorder functions Uwe Kleine-König
  2020-11-19 12:46 ` [PATCH 2/3] driver core: platform: change logic implementing platform_driver_probe Uwe Kleine-König
@ 2020-11-19 12:46 ` Uwe Kleine-König
       [not found]   ` <CGME20201211161217eucas1p1e66553fbb8d113b417e29150951df683@eucas1p1.samsung.com>
                     ` (2 more replies)
  1 sibling, 3 replies; 7+ messages in thread
From: Uwe Kleine-König @ 2020-11-19 12:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J . Wysocki 
  Cc: kernel, linux-kernel, Russell King

This works towards the goal mentioned in 2006 in commit 594c8281f905
("[PATCH] Add bus_type probe, remove, shutdown methods.").

The functions are moved to where the other bus_type functions are
defined and renamed to match the already established naming scheme.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/base/platform.c | 132 ++++++++++++++++++++--------------------
 1 file changed, 65 insertions(+), 67 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index b847f5f8f992..8ad06daa2eaa 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -743,70 +743,6 @@ struct platform_device *platform_device_register_full(
 }
 EXPORT_SYMBOL_GPL(platform_device_register_full);
 
-static int platform_probe_fail(struct platform_device *pdev);
-
-static int platform_drv_probe(struct device *_dev)
-{
-	struct platform_driver *drv = to_platform_driver(_dev->driver);
-	struct platform_device *dev = to_platform_device(_dev);
-	int ret;
-
-	/*
-	 * A driver registered using platform_driver_probe() cannot be bound
-	 * again later because the probe function usually lives in __init code
-	 * and so is gone. For these drivers .probe is set to
-	 * platform_probe_fail in __platform_driver_probe(). Don't even
-	 * prepare clocks and PM domains for these to match the traditional
-	 * behaviour.
-	 */
-	if (unlikely(drv->probe == platform_probe_fail))
-		return -ENXIO;
-
-	ret = of_clk_set_defaults(_dev->of_node, false);
-	if (ret < 0)
-		return ret;
-
-	ret = dev_pm_domain_attach(_dev, true);
-	if (ret)
-		goto out;
-
-	if (drv->probe) {
-		ret = drv->probe(dev);
-		if (ret)
-			dev_pm_domain_detach(_dev, true);
-	}
-
-out:
-	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
-		dev_warn(_dev, "probe deferral not supported\n");
-		ret = -ENXIO;
-	}
-
-	return ret;
-}
-
-static int platform_drv_remove(struct device *_dev)
-{
-	struct platform_driver *drv = to_platform_driver(_dev->driver);
-	struct platform_device *dev = to_platform_device(_dev);
-	int ret = 0;
-
-	if (drv->remove)
-		ret = drv->remove(dev);
-	dev_pm_domain_detach(_dev, true);
-
-	return ret;
-}
-
-static void platform_drv_shutdown(struct device *_dev)
-{
-	struct platform_driver *drv = to_platform_driver(_dev->driver);
-	struct platform_device *dev = to_platform_device(_dev);
-
-	if (drv->shutdown)
-		drv->shutdown(dev);
-}
-
 /**
  * __platform_driver_register - register a driver for platform-level devices
  * @drv: platform driver structure
@@ -817,9 +753,6 @@ int __platform_driver_register(struct platform_driver *drv,
 {
 	drv->driver.owner = owner;
 	drv->driver.bus = &platform_bus_type;
-	drv->driver.probe = platform_drv_probe;
-	drv->driver.remove = platform_drv_remove;
-	drv->driver.shutdown = platform_drv_shutdown;
 
 	return driver_register(&drv->driver);
 }
@@ -1349,6 +1282,68 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
 	return 0;
 }
 
+static int platform_probe(struct device *_dev)
+{
+	struct platform_driver *drv = to_platform_driver(_dev->driver);
+	struct platform_device *dev = to_platform_device(_dev);
+	int ret;
+
+	/*
+	 * A driver registered using platform_driver_probe() cannot be bound
+	 * again later because the probe function usually lives in __init code
+	 * and so is gone. For these drivers .probe is set to
+	 * platform_probe_fail in __platform_driver_probe(). Don't even prepare
+	 * clocks and PM domains for these to match the traditional behaviour.
+	 */
+	if (unlikely(drv->probe == platform_probe_fail))
+		return -ENXIO;
+
+	ret = of_clk_set_defaults(_dev->of_node, false);
+	if (ret < 0)
+		return ret;
+
+	ret = dev_pm_domain_attach(_dev, true);
+	if (ret)
+		goto out;
+
+	if (drv->probe) {
+		ret = drv->probe(dev);
+		if (ret)
+			dev_pm_domain_detach(_dev, true);
+	}
+
+out:
+	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
+		dev_warn(_dev, "probe deferral not supported\n");
+		ret = -ENXIO;
+	}
+
+	return ret;
+}
+
+static int platform_remove(struct device *_dev)
+{
+	struct platform_driver *drv = to_platform_driver(_dev->driver);
+	struct platform_device *dev = to_platform_device(_dev);
+	int ret = 0;
+
+	if (drv->remove)
+		ret = drv->remove(dev);
+	dev_pm_domain_detach(_dev, true);
+
+	return ret;
+}
+
+static void platform_shutdown(struct device *_dev)
+{
+	struct platform_driver *drv = to_platform_driver(_dev->driver);
+	struct platform_device *dev = to_platform_device(_dev);
+
+	if (drv->shutdown)
+		drv->shutdown(dev);
+}
+
+
 int platform_dma_configure(struct device *dev)
 {
 	enum dev_dma_attr attr;
@@ -1375,6 +1370,9 @@ struct bus_type platform_bus_type = {
 	.dev_groups	= platform_dev_groups,
 	.match		= platform_match,
 	.uevent		= platform_uevent,
+	.probe		= platform_probe,
+	.remove		= platform_remove,
+	.shutdown	= platform_shutdown,
 	.dma_configure	= platform_dma_configure,
 	.pm		= &platform_dev_pm_ops,
 };
-- 
2.28.0


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

* Re: [PATCH 3/3] driver core: platform: use bus_type functions
       [not found]   ` <CGME20201211161217eucas1p1e66553fbb8d113b417e29150951df683@eucas1p1.samsung.com>
@ 2020-12-11 16:12     ` Marek Szyprowski
  2020-12-12 21:13       ` Uwe Kleine-König
  0 siblings, 1 reply; 7+ messages in thread
From: Marek Szyprowski @ 2020-12-11 16:12 UTC (permalink / raw)
  To: Uwe Kleine-König, Greg Kroah-Hartman, Rafael J . Wysocki
  Cc: kernel, linux-kernel, Russell King

Hi Uwe,

On 19.11.2020 13:46, Uwe Kleine-König wrote:
> This works towards the goal mentioned in 2006 in commit 594c8281f905
> ("[PATCH] Add bus_type probe, remove, shutdown methods.").
>
> The functions are moved to where the other bus_type functions are
> defined and renamed to match the already established naming scheme.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>   drivers/base/platform.c | 132 ++++++++++++++++++++--------------------
>   1 file changed, 65 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index b847f5f8f992..8ad06daa2eaa 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -743,70 +743,6 @@ struct platform_device *platform_device_register_full(
>   }
>   EXPORT_SYMBOL_GPL(platform_device_register_full);
>   
> -static int platform_probe_fail(struct platform_device *pdev);
> -
> -static int platform_drv_probe(struct device *_dev)
> -{
> -	struct platform_driver *drv = to_platform_driver(_dev->driver);
> -	struct platform_device *dev = to_platform_device(_dev);
> -	int ret;
> -
> -	/*
> -	 * A driver registered using platform_driver_probe() cannot be bound
> -	 * again later because the probe function usually lives in __init code
> -	 * and so is gone. For these drivers .probe is set to
> -	 * platform_probe_fail in __platform_driver_probe(). Don't even
> -	 * prepare clocks and PM domains for these to match the traditional
> -	 * behaviour.
> -	 */
> -	if (unlikely(drv->probe == platform_probe_fail))
> -		return -ENXIO;
> -
> -	ret = of_clk_set_defaults(_dev->of_node, false);
> -	if (ret < 0)
> -		return ret;
> -
> -	ret = dev_pm_domain_attach(_dev, true);
> -	if (ret)
> -		goto out;
> -
> -	if (drv->probe) {
> -		ret = drv->probe(dev);
> -		if (ret)
> -			dev_pm_domain_detach(_dev, true);
> -	}
> -
> -out:
> -	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
> -		dev_warn(_dev, "probe deferral not supported\n");
> -		ret = -ENXIO;
> -	}
> -
> -	return ret;
> -}
> -
> -static int platform_drv_remove(struct device *_dev)
> -{
> -	struct platform_driver *drv = to_platform_driver(_dev->driver);
> -	struct platform_device *dev = to_platform_device(_dev);
> -	int ret = 0;
> -
> -	if (drv->remove)
> -		ret = drv->remove(dev);
> -	dev_pm_domain_detach(_dev, true);
> -
> -	return ret;
> -}
> -
> -static void platform_drv_shutdown(struct device *_dev)
> -{
> -	struct platform_driver *drv = to_platform_driver(_dev->driver);
> -	struct platform_device *dev = to_platform_device(_dev);
> -
> -	if (drv->shutdown)
> -		drv->shutdown(dev);
> -}
> -
>   /**
>    * __platform_driver_register - register a driver for platform-level devices
>    * @drv: platform driver structure
> @@ -817,9 +753,6 @@ int __platform_driver_register(struct platform_driver *drv,
>   {
>   	drv->driver.owner = owner;
>   	drv->driver.bus = &platform_bus_type;
> -	drv->driver.probe = platform_drv_probe;
> -	drv->driver.remove = platform_drv_remove;
> -	drv->driver.shutdown = platform_drv_shutdown;
>   
>   	return driver_register(&drv->driver);
>   }
> @@ -1349,6 +1282,68 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
>   	return 0;
>   }
>   
> +static int platform_probe(struct device *_dev)
> +{
> +	struct platform_driver *drv = to_platform_driver(_dev->driver);
> +	struct platform_device *dev = to_platform_device(_dev);
> +	int ret;
> +
> +	/*
> +	 * A driver registered using platform_driver_probe() cannot be bound
> +	 * again later because the probe function usually lives in __init code
> +	 * and so is gone. For these drivers .probe is set to
> +	 * platform_probe_fail in __platform_driver_probe(). Don't even prepare
> +	 * clocks and PM domains for these to match the traditional behaviour.
> +	 */
> +	if (unlikely(drv->probe == platform_probe_fail))
> +		return -ENXIO;
> +
> +	ret = of_clk_set_defaults(_dev->of_node, false);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = dev_pm_domain_attach(_dev, true);
> +	if (ret)
> +		goto out;
> +
> +	if (drv->probe) {
> +		ret = drv->probe(dev);
> +		if (ret)
> +			dev_pm_domain_detach(_dev, true);
> +	}
> +
> +out:
> +	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
> +		dev_warn(_dev, "probe deferral not supported\n");
> +		ret = -ENXIO;
> +	}
> +
> +	return ret;
> +}
> +
> +static int platform_remove(struct device *_dev)
> +{
> +	struct platform_driver *drv = to_platform_driver(_dev->driver);
> +	struct platform_device *dev = to_platform_device(_dev);
> +	int ret = 0;
> +
> +	if (drv->remove)
> +		ret = drv->remove(dev);
> +	dev_pm_domain_detach(_dev, true);
> +
> +	return ret;
> +}
> +
> +static void platform_shutdown(struct device *_dev)
> +{
> +	struct platform_driver *drv = to_platform_driver(_dev->driver);
> +	struct platform_device *dev = to_platform_device(_dev);
> +
> +	if (drv->shutdown)
> +		drv->shutdown(dev);
> +}

This will be called on unbound devices, what causes crash (observed on 
today's linux-next):

Will now restart.
8<--- cut here ---
Unable to handle kernel paging request at virtual address fffffff4
pgd = 289f4b67
[fffffff4] *pgd=6ffff841, *pte=00000000, *ppte=00000000
Internal error: Oops: 27 [#1] SMP ARM
Modules linked in: usb_f_ecm g_ether usb_f_rndis u_ether libcomposite 
brcmfmac sha256_generic libsha256 sha256_arm cfg80211 brcmutil 
panel_samsung_s6e8aa0 s5p_csi
CPU: 0 PID: 1715 Comm: reboot Not tainted 
5.10.0-rc7-next-20201211-00069-g1e8aa883315f #9935
Hardware name: Samsung Exynos (Flattened Device Tree)
PC is at platform_shutdown+0x8/0x18
LR is at device_shutdown+0x18c/0x25c
...
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 4348404a  DAC: 00000051
Process reboot (pid: 1715, stack limit = 0xd050391e)
Stack: (0xc3405e60 to 0xc3406000)
[<c0a38bfc>] (platform_shutdown) from [<c0a34f78>] 
(device_shutdown+0x18c/0x25c)
[<c0a34f78>] (device_shutdown) from [<c036d3cc>] (kernel_restart+0xc/0x68)
[<c036d3cc>] (kernel_restart) from [<c036d680>] 
(__do_sys_reboot+0x154/0x1f0)
[<c036d680>] (__do_sys_reboot) from [<c03000c0>] (ret_fast_syscall+0x0/0x58)
Exception stack(0xc3405fa8 to 0xc3405ff0)
...
---[ end trace f39e94d5d6fd45bf ]---


> +
> +
>   int platform_dma_configure(struct device *dev)
>   {
>   	enum dev_dma_attr attr;
> @@ -1375,6 +1370,9 @@ struct bus_type platform_bus_type = {
>   	.dev_groups	= platform_dev_groups,
>   	.match		= platform_match,
>   	.uevent		= platform_uevent,
> +	.probe		= platform_probe,
> +	.remove		= platform_remove,
> +	.shutdown	= platform_shutdown,
>   	.dma_configure	= platform_dma_configure,
>   	.pm		= &platform_dev_pm_ops,
>   };

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


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

* Re: [PATCH 3/3] driver core: platform: use bus_type functions
  2020-11-19 12:46 ` [PATCH 3/3] driver core: platform: use bus_type functions Uwe Kleine-König
       [not found]   ` <CGME20201211161217eucas1p1e66553fbb8d113b417e29150951df683@eucas1p1.samsung.com>
@ 2020-12-11 16:45   ` Qian Cai
  2020-12-12  2:01   ` Guenter Roeck
  2 siblings, 0 replies; 7+ messages in thread
From: Qian Cai @ 2020-12-11 16:45 UTC (permalink / raw)
  To: Uwe Kleine-König, Greg Kroah-Hartman, Rafael J . Wysocki
  Cc: kernel, linux-kernel, Russell King, Stephen Rothwell,
	Linux Next Mailing List

On Thu, 2020-11-19 at 13:46 +0100, Uwe Kleine-König wrote:
> This works towards the goal mentioned in 2006 in commit 594c8281f905
> ("[PATCH] Add bus_type probe, remove, shutdown methods.").
> 
> The functions are moved to where the other bus_type functions are
> defined and renamed to match the already established naming scheme.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reverting this commit from today's linux-next fixed a crash during shutdown.

.config: https://cailca.coding.net/public/linux/mm/git/files/master/x86.config

[ 9771.596916][T113465] BUG: unable to handle page fault for address: ffffffffffffffe8
[ 9771.604627][T113465] #PF: supervisor read access in kernel mode
[ 9771.610581][T113465] #PF: error_code(0x0000) - not-present page
[ 9771.616533][T113465] PGD 19c1e17067 P4D 19c1e17067 PUD 19c1e19067 PMD 0 
[ 9771.623279][T113465] Oops: 0000 [#1] SMP KASAN PTI
[ 9771.628098][T113465] CPU: 22 PID: 113465 Comm: reboot Tainted: G          IO      5.10.0-rc7-next-20201211 #1
[ 9771.638071][T113465] Hardware name: HPE ProLiant DL560 Gen10/ProLiant DL560 Gen10, BIOS U34 11/13/2019
[ 9771.647431][T113465] RIP: 0010:platform_shutdown+0x44/0x70
platform_shutdown at drivers/base/platform.c:1357
[ 9771.652956][T113465] Code: fa 48 c1 ea 03 80 3c 02 00 75 3d 48 b8 00 00 00 00 00 fc ff df 48 8b 6b 68 48 8d 7d e8 48 89 fa 48 c1 ea 03 80 3c 02 00 75 17 <48> 8b 45 e8 48 85 c0 74 0b 48 8d 7b f0 5b 5d e9 08 45 6c 00 5b 5d
[ 9771.672623][T113465] RSP: 0018:ffffc90008a77d38 EFLAGS: 00010246
[ 9771.678665][T113465] RAX: dffffc0000000000 RBX: ffff888860d78810 RCX: ffff888860d78870
[ 9771.686628][T113465] RDX: 1ffffffffffffffd RSI: 0000000000000001 RDI: ffffffffffffffe8
[ 9771.694591][T113465] RBP: 0000000000000000 R08: ffffed110c1af166 R09: ffffed110c1af166
[ 9771.702555][T113465] R10: ffff888860d78b2b R11: ffffed110c1af165 R12: ffff888860d78810
[ 9771.710516][T113465] R13: ffff888860d78920 R14: fffffbfff2db0008 R15: ffff888860d78818
[ 9771.718478][T113465] FS:  00007f3434549540(0000) GS:ffff88901f500000(0000) knlGS:0000000000000000
[ 9771.727402][T113465] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 9771.733966][T113465] CR2: ffffffffffffffe8 CR3: 000000092e9c0004 CR4: 00000000007706e0
[ 9771.741929][T113465] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 9771.749890][T113465] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 9771.757852][T113465] PKRU: 55555554
[ 9771.761359][T113465] Call Trace:
[ 9771.764604][T113465]  device_shutdown+0x2ec/0x540
[ 9771.769335][T113465]  kernel_restart+0xe/0x40
[ 9771.773721][T113465]  __do_sys_reboot+0x143/0x2b0
[ 9771.778450][T113465]  ? kernel_power_off+0xa0/0xa0
[ 9771.783269][T113465]  ? debug_object_deactivate+0x3b0/0x3b0
[ 9771.788877][T113465]  ? syscall_enter_from_user_mode+0x17/0x40
[ 9771.794747][T113465]  ? rcu_read_lock_sched_held+0xa1/0xd0
[ 9771.800267][T113465]  ? lockdep_hardirqs_on_prepare+0x27c/0x3d0
[ 9771.806221][T113465]  ? syscall_enter_from_user_mode+0x1c/0x40
[ 9771.812087][T113465]  do_syscall_64+0x33/0x40
[ 9771.816472][T113465]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 9771.822340][T113465] RIP: 0033:0x7f343379b857
[ 9771.826724][T113465] Code: 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 89 fa be 69 19 12 28 bf ad de e1 fe b8 a9 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 01 c3 48 8b 15 01 86 2c 00 f7 d8 64 89 02 b8
[ 9771.846392][T113465] RSP: 002b:00007ffef9f85e58 EFLAGS: 00000246 ORIG_RAX: 00000000000000a9
[ 9771.854791][T113465] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f343379b857
[ 9771.862752][T113465] RDX: 0000000001234567 RSI: 0000000028121969 RDI: 00000000fee1dead
[ 9771.870713][T113465] RBP: 00007ffef9f85ea0 R08: 0000000000000002 R09: 0000000000000000
[ 9771.878673][T113465] R10: 000000000000004b R11: 0000000000000246 R12: 0000000000000001
[ 9771.886635][T113465] R13: 00000000fffffffe R14: 0000000000000006 R15: 0000000000000000
[ 9771.894596][T113465] Modules linked in: isofs cdrom fuse loop nls_ascii nls_cp437 vfat fat kvm_intel kvm ses enclosure irqbypass efivarfs ip_tables x_tables sd_mod tg3 nvme firmware_class smartpqi nvme_core libphy scsi_transport_sas dm_mirror dm_region_hash dm_log dm_mod [last unloaded: dummy_del_mod]
[ 9771.921472][T113465] CR2: ffffffffffffffe8
[ 9771.925590][T113465] ---[ end trace 8a3c9cffc1068bd2 ]---
[ 9771.931017][T113465] RIP: 0010:platform_shutdown+0x44/0x70
[ 9771.936535][T113465] Code: fa 48 c1 ea 03 80 3c 02 00 75 3d 48 b8 00 00 00 00 00 fc ff df 48 8b 6b 68 48 8d 7d e8 48 89 fa 48 c1 ea 03 80 3c 02 00 75 17 <48> 8b 45 e8 48 85 c0 74 0b 48 8d 7b f0 5b 5d e9 08 45 6c 00 5b 5d
[ 9771.956204][T113465] RSP: 0018:ffffc90008a77d38 EFLAGS: 00010246

> ---
>  drivers/base/platform.c | 132 ++++++++++++++++++++--------------------
>  1 file changed, 65 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index b847f5f8f992..8ad06daa2eaa 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -743,70 +743,6 @@ struct platform_device *platform_device_register_full(
>  }
>  EXPORT_SYMBOL_GPL(platform_device_register_full);
>  
> -static int platform_probe_fail(struct platform_device *pdev);
> -
> -static int platform_drv_probe(struct device *_dev)
> -{
> -	struct platform_driver *drv = to_platform_driver(_dev->driver);
> -	struct platform_device *dev = to_platform_device(_dev);
> -	int ret;
> -
> -	/*
> -	 * A driver registered using platform_driver_probe() cannot be bound
> -	 * again later because the probe function usually lives in __init code
> -	 * and so is gone. For these drivers .probe is set to
> -	 * platform_probe_fail in __platform_driver_probe(). Don't even
> -	 * prepare clocks and PM domains for these to match the traditional
> -	 * behaviour.
> -	 */
> -	if (unlikely(drv->probe == platform_probe_fail))
> -		return -ENXIO;
> -
> -	ret = of_clk_set_defaults(_dev->of_node, false);
> -	if (ret < 0)
> -		return ret;
> -
> -	ret = dev_pm_domain_attach(_dev, true);
> -	if (ret)
> -		goto out;
> -
> -	if (drv->probe) {
> -		ret = drv->probe(dev);
> -		if (ret)
> -			dev_pm_domain_detach(_dev, true);
> -	}
> -
> -out:
> -	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
> -		dev_warn(_dev, "probe deferral not supported\n");
> -		ret = -ENXIO;
> -	}
> -
> -	return ret;
> -}
> -
> -static int platform_drv_remove(struct device *_dev)
> -{
> -	struct platform_driver *drv = to_platform_driver(_dev->driver);
> -	struct platform_device *dev = to_platform_device(_dev);
> -	int ret = 0;
> -
> -	if (drv->remove)
> -		ret = drv->remove(dev);
> -	dev_pm_domain_detach(_dev, true);
> -
> -	return ret;
> -}
> -
> -static void platform_drv_shutdown(struct device *_dev)
> -{
> -	struct platform_driver *drv = to_platform_driver(_dev->driver);
> -	struct platform_device *dev = to_platform_device(_dev);
> -
> -	if (drv->shutdown)
> -		drv->shutdown(dev);
> -}
> -
>  /**
>   * __platform_driver_register - register a driver for platform-level devices
>   * @drv: platform driver structure
> @@ -817,9 +753,6 @@ int __platform_driver_register(struct platform_driver *drv,
>  {
>  	drv->driver.owner = owner;
>  	drv->driver.bus = &platform_bus_type;
> -	drv->driver.probe = platform_drv_probe;
> -	drv->driver.remove = platform_drv_remove;
> -	drv->driver.shutdown = platform_drv_shutdown;
>  
>  	return driver_register(&drv->driver);
>  }
> @@ -1349,6 +1282,68 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
>  	return 0;
>  }
>  
> +static int platform_probe(struct device *_dev)
> +{
> +	struct platform_driver *drv = to_platform_driver(_dev->driver);
> +	struct platform_device *dev = to_platform_device(_dev);
> +	int ret;
> +
> +	/*
> +	 * A driver registered using platform_driver_probe() cannot be bound
> +	 * again later because the probe function usually lives in __init code
> +	 * and so is gone. For these drivers .probe is set to
> +	 * platform_probe_fail in __platform_driver_probe(). Don't even prepare
> +	 * clocks and PM domains for these to match the traditional behaviour.
> +	 */
> +	if (unlikely(drv->probe == platform_probe_fail))
> +		return -ENXIO;
> +
> +	ret = of_clk_set_defaults(_dev->of_node, false);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = dev_pm_domain_attach(_dev, true);
> +	if (ret)
> +		goto out;
> +
> +	if (drv->probe) {
> +		ret = drv->probe(dev);
> +		if (ret)
> +			dev_pm_domain_detach(_dev, true);
> +	}
> +
> +out:
> +	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
> +		dev_warn(_dev, "probe deferral not supported\n");
> +		ret = -ENXIO;
> +	}
> +
> +	return ret;
> +}
> +
> +static int platform_remove(struct device *_dev)
> +{
> +	struct platform_driver *drv = to_platform_driver(_dev->driver);
> +	struct platform_device *dev = to_platform_device(_dev);
> +	int ret = 0;
> +
> +	if (drv->remove)
> +		ret = drv->remove(dev);
> +	dev_pm_domain_detach(_dev, true);
> +
> +	return ret;
> +}
> +
> +static void platform_shutdown(struct device *_dev)
> +{
> +	struct platform_driver *drv = to_platform_driver(_dev->driver);
> +	struct platform_device *dev = to_platform_device(_dev);
> +
> +	if (drv->shutdown)
> +		drv->shutdown(dev);
> +}
> +
> +
>  int platform_dma_configure(struct device *dev)
>  {
>  	enum dev_dma_attr attr;
> @@ -1375,6 +1370,9 @@ struct bus_type platform_bus_type = {
>  	.dev_groups	= platform_dev_groups,
>  	.match		= platform_match,
>  	.uevent		= platform_uevent,
> +	.probe		= platform_probe,
> +	.remove		= platform_remove,
> +	.shutdown	= platform_shutdown,
>  	.dma_configure	= platform_dma_configure,
>  	.pm		= &platform_dev_pm_ops,
>  };


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

* Re: [PATCH 3/3] driver core: platform: use bus_type functions
  2020-11-19 12:46 ` [PATCH 3/3] driver core: platform: use bus_type functions Uwe Kleine-König
       [not found]   ` <CGME20201211161217eucas1p1e66553fbb8d113b417e29150951df683@eucas1p1.samsung.com>
  2020-12-11 16:45   ` Qian Cai
@ 2020-12-12  2:01   ` Guenter Roeck
  2 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2020-12-12  2:01 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Greg Kroah-Hartman, Rafael J . Wysocki ,
	kernel, linux-kernel, Russell King

On Thu, Nov 19, 2020 at 01:46:11PM +0100, Uwe Kleine-König wrote:
> This works towards the goal mentioned in 2006 in commit 594c8281f905
> ("[PATCH] Add bus_type probe, remove, shutdown methods.").
> 
> The functions are moved to where the other bus_type functions are
> defined and renamed to match the already established naming scheme.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Qemu test results:
	total: 426 pass: 91 fail: 335

This patch isn't responsible for all the crashes (-next is in pretty bad
shape), but for a good chunk of it.

Guenter

---
Bisect results for arbitrary arm64 test:

# bad: [3cc2bd440f2171f093b3a8480a4b54d8c270ed38] Add linux-next specific files for 20201211
# good: [0477e92881850d44910a7e94fc2c46f96faa131f] Linux 5.10-rc7
git bisect start 'HEAD' 'v5.10-rc7'
# good: [0a701401d4e29d9e73f0f3cc02179fc6c9191646] Merge remote-tracking branch 'crypto/master'
git bisect good 0a701401d4e29d9e73f0f3cc02179fc6c9191646
# good: [6fd39ad603b113e9c68180b9138084710c036e34] Merge remote-tracking branch 'spi/for-next'
git bisect good 6fd39ad603b113e9c68180b9138084710c036e34
# bad: [c96b2eec436e87b8c673213b203559bed9e551b9] Merge remote-tracking branch 'vfio/next'
git bisect bad c96b2eec436e87b8c673213b203559bed9e551b9
# good: [f99c2fbbff522300c309e517be1f5bed4bd34704] Merge remote-tracking branch 'kvm-arm/next'
git bisect good f99c2fbbff522300c309e517be1f5bed4bd34704
# bad: [0e3f63470c00704498be2bfac586076cfa93214f] Merge remote-tracking branch 'usb-chipidea-next/for-usb-next'
git bisect bad 0e3f63470c00704498be2bfac586076cfa93214f
# bad: [903821bc4404ae12d4e50e95fb5c2d7b46f4d1c6] Merge remote-tracking branch 'driver-core/driver-core-next'
git bisect bad 903821bc4404ae12d4e50e95fb5c2d7b46f4d1c6
# good: [0cd3f561efa9adce840140720e0581355db3e554] platform/x86: ISST: Mark mmio_range_devid_0 and mmio_range_devid_1 with static keyword
git bisect good 0cd3f561efa9adce840140720e0581355db3e554
# good: [bd7cf676c3ed2fc91e777d91c3bf9220e84da2ad] Merge remote-tracking branch 'chrome-platform/for-next'
git bisect good bd7cf676c3ed2fc91e777d91c3bf9220e84da2ad
# good: [d475f8ea98a039e51d27f5557dc17333cf8a52f6] driver core: Fix a couple of typos
git bisect good d475f8ea98a039e51d27f5557dc17333cf8a52f6
# good: [16c1af8b52ea282b098c9b7506f3f4d0d3953260] Merge remote-tracking branch 'leds/for-next'
git bisect good 16c1af8b52ea282b098c9b7506f3f4d0d3953260
# bad: [feaba5932b6f4bfc875c874a3b7a28c7f05f5a77] vfio: platform: Switch to use platform_get_mem_or_io()
git bisect bad feaba5932b6f4bfc875c874a3b7a28c7f05f5a77
# bad: [9c30921fe7994907e0b3e0637b2c8c0fc4b5171f] driver core: platform: use bus_type functions
git bisect bad 9c30921fe7994907e0b3e0637b2c8c0fc4b5171f
# good: [e21d740a3fe5ad2db7b5f5c2331fe2b713b1edba] driver core: platform: reorder functions
git bisect good e21d740a3fe5ad2db7b5f5c2331fe2b713b1edba
# good: [16085668eacdc56c46652d0f3bfef81ecace57de] driver core: platform: change logic implementing platform_driver_probe
git bisect good 16085668eacdc56c46652d0f3bfef81ecace57de
# first bad commit: [9c30921fe7994907e0b3e0637b2c8c0fc4b5171f] driver core: platform: use bus_type functions

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

* Re: [PATCH 3/3] driver core: platform: use bus_type functions
  2020-12-11 16:12     ` Marek Szyprowski
@ 2020-12-12 21:13       ` Uwe Kleine-König
  0 siblings, 0 replies; 7+ messages in thread
From: Uwe Kleine-König @ 2020-12-12 21:13 UTC (permalink / raw)
  To: Marek Szyprowski, Qian Cai, Guenter Roeck
  Cc: Greg Kroah-Hartman, Rafael J . Wysocki, Russell King,
	linux-kernel, kernel, Dmitry Baryshkov

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

Hi Marek,

On Fri, Dec 11, 2020 at 05:12:16PM +0100, Marek Szyprowski wrote:
> On 19.11.2020 13:46, Uwe Kleine-König wrote:
> > +static void platform_shutdown(struct device *_dev)
> > +{
> > +	struct platform_driver *drv = to_platform_driver(_dev->driver);
> > +	struct platform_device *dev = to_platform_device(_dev);
> > +
> > +	if (drv->shutdown)
> > +		drv->shutdown(dev);
> > +}
> 
> This will be called on unbound devices, what causes crash (observed on 
> today's linux-next):
> 
> Will now restart.
> 8<--- cut here ---
> Unable to handle kernel paging request at virtual address fffffff4
> pgd = 289f4b67
> [fffffff4] *pgd=6ffff841, *pte=00000000, *ppte=00000000
> Internal error: Oops: 27 [#1] SMP ARM
> Modules linked in: usb_f_ecm g_ether usb_f_rndis u_ether libcomposite 
> brcmfmac sha256_generic libsha256 sha256_arm cfg80211 brcmutil 
> panel_samsung_s6e8aa0 s5p_csi
> CPU: 0 PID: 1715 Comm: reboot Not tainted 
> 5.10.0-rc7-next-20201211-00069-g1e8aa883315f #9935
> Hardware name: Samsung Exynos (Flattened Device Tree)
> PC is at platform_shutdown+0x8/0x18
> LR is at device_shutdown+0x18c/0x25c
> ...
> Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> Control: 10c5387d  Table: 4348404a  DAC: 00000051
> Process reboot (pid: 1715, stack limit = 0xd050391e)
> Stack: (0xc3405e60 to 0xc3406000)
> [<c0a38bfc>] (platform_shutdown) from [<c0a34f78>] 
> (device_shutdown+0x18c/0x25c)
> [<c0a34f78>] (device_shutdown) from [<c036d3cc>] (kernel_restart+0xc/0x68)
> [<c036d3cc>] (kernel_restart) from [<c036d680>] 
> (__do_sys_reboot+0x154/0x1f0)
> [<c036d680>] (__do_sys_reboot) from [<c03000c0>] (ret_fast_syscall+0x0/0x58)
> Exception stack(0xc3405fa8 to 0xc3405ff0)
> ...
> ---[ end trace f39e94d5d6fd45bf ]---

Dmitry Baryshkov already proposed a fix, see
https://lore.kernel.org/r/20201212011426.163071-1-dmitry.baryshkov@linaro.org

I expect a v2 soon.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

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

end of thread, other threads:[~2020-12-12 21:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-19 12:46 [PATCH 1/3] driver core: platform: reorder functions Uwe Kleine-König
2020-11-19 12:46 ` [PATCH 2/3] driver core: platform: change logic implementing platform_driver_probe Uwe Kleine-König
2020-11-19 12:46 ` [PATCH 3/3] driver core: platform: use bus_type functions Uwe Kleine-König
     [not found]   ` <CGME20201211161217eucas1p1e66553fbb8d113b417e29150951df683@eucas1p1.samsung.com>
2020-12-11 16:12     ` Marek Szyprowski
2020-12-12 21:13       ` Uwe Kleine-König
2020-12-11 16:45   ` Qian Cai
2020-12-12  2:01   ` Guenter Roeck

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