[PATCH 1/2] dm: core: Fix allocation of empty of-platdata

[PATCH 1/2] dm: core: Fix allocation of empty of-platdata

[Simon Glass]

With of-platdata we always have a dtv struct that holds the platform data
provided by the driver_info record. However, this struct can be empty if
there are no actual devicetree properties provided.

The upshot of empty platform data is that it will end up as a zero-size
member in the BSS section, which is fine. But if the driver specifies
plat_auto then it expects the correct amount of space to be allocated.

At present this does not happen, since device_bind() assumes that the
platform-data size will always be >0. As a result we end up not
allocating the space and just use the BSS region, overwriting whatever
other contents are present.

Fix this by removing the condition that platform data be non-empty, always
allocating space if requested.

This fixes a strange bug that has been lurking since of-platdata was
implemented. It has likely never been noticed since devices normally have
at least some devicetree properties, BSS is seldom used on SPL, the dtv
structs are normally at the end of bss and the overwriting only happens
if a driver changes its platform data.

It was discovered using sandbox_spl, which exercises more features than
a normal board might, and the critical global_data variable 'gd' happened
to be at the end of BSS.

Fixes: 9fa28190091 ("dm: core: Expand platdata for of-platdata devices")
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/core/device.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/core/device.c b/drivers/core/device.c
index 8629df8defb..009e57e18cc 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -91,15 +91,19 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv,
 	if (auto_seq && !(uc->uc_drv->flags & DM_UC_FLAG_NO_AUTO_SEQ))
 		dev->seq_ = uclass_find_next_free_seq(uc);
 
+	/* Check if we need to allocate plat */
 	if (drv->plat_auto) {
 		bool alloc = !plat;
 
+		/*
+		 * For of-platdata, we try use the existing data, but if
+		 * plat_auto is larger, we must allocate a new space
+		 */
 		if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
-			if (of_plat_size) {
+			if (of_plat_size)
 				dev_or_flags(dev, DM_FLAG_OF_PLATDATA);
-				if (of_plat_size < drv->plat_auto)
-					alloc = true;
-			}
+			if (of_plat_size < drv->plat_auto)
+				alloc = true;
 		}
 		if (alloc) {
 			dev_or_flags(dev, DM_FLAG_ALLOC_PDATA);
@@ -108,6 +112,11 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv,
 				ret = -ENOMEM;
 				goto fail_alloc1;
 			}
+
+			/*
+			 * For of-platdata, copy the old plat into the new
+			 * space
+			 */
 			if (CONFIG_IS_ENABLED(OF_PLATDATA) && plat)
 				memcpy(ptr, plat, of_plat_size);
 			dev_set_plat(dev, ptr);
-- 
2.30.0.365.g02bc693789-goog

[PATCH 2/2] dm: core: Add DM_DEVICE_REMOVE condition to all exit paths

[Simon Glass]

At present device_bind() does some unnecessary work if a device fails to
bind in SPL. Add the missing conditions.

Also fix a style nit in the same function while we are here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/core/device.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/core/device.c b/drivers/core/device.c
index 009e57e18cc..c0547b75c76 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -136,9 +136,8 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv,
 
 	if (parent) {
 		size = parent->driver->per_child_plat_auto;
-		if (!size) {
+		if (!size)
 			size = parent->uclass->uc_drv->per_child_plat_auto;
-		}
 		if (size) {
 			dev_or_flags(dev, DM_FLAG_ALLOC_PARENT_PDATA);
 			ptr = calloc(1, size);
@@ -208,14 +207,18 @@ fail_uclass_bind:
 		}
 	}
 fail_alloc3:
-	if (dev_get_flags(dev) & DM_FLAG_ALLOC_UCLASS_PDATA) {
-		free(dev_get_uclass_plat(dev));
-		dev_set_uclass_plat(dev, NULL);
+	if (CONFIG_IS_ENABLED(DM_DEVICE_REMOVE)) {
+		if (dev_get_flags(dev) & DM_FLAG_ALLOC_UCLASS_PDATA) {
+			free(dev_get_uclass_plat(dev));
+			dev_set_uclass_plat(dev, NULL);
+		}
 	}
 fail_alloc2:
-	if (dev_get_flags(dev) & DM_FLAG_ALLOC_PDATA) {
-		free(dev_get_plat(dev));
-		dev_set_plat(dev, NULL);
+	if (CONFIG_IS_ENABLED(DM_DEVICE_REMOVE)) {
+		if (dev_get_flags(dev) & DM_FLAG_ALLOC_PDATA) {
+			free(dev_get_plat(dev));
+			dev_set_plat(dev, NULL);
+		}
 	}
 fail_alloc1:
 	devres_release_all(dev);
-- 
2.30.0.365.g02bc693789-goog

[PATCH 2/2] dm: core: Add DM_DEVICE_REMOVE condition to all exit paths

[Simon Glass]

At present device_bind() does some unnecessary work if a device fails to
bind in SPL. Add the missing conditions.

Also fix a style nit in the same function while we are here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/core/device.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

Applied to u-boot-dm, thanks!

[PATCH 1/2] dm: core: Fix allocation of empty of-platdata

[Simon Glass]

With of-platdata we always have a dtv struct that holds the platform data
provided by the driver_info record. However, this struct can be empty if
there are no actual devicetree properties provided.

The upshot of empty platform data is that it will end up as a zero-size
member in the BSS section, which is fine. But if the driver specifies
plat_auto then it expects the correct amount of space to be allocated.

At present this does not happen, since device_bind() assumes that the
platform-data size will always be >0. As a result we end up not
allocating the space and just use the BSS region, overwriting whatever
other contents are present.

Fix this by removing the condition that platform data be non-empty, always
allocating space if requested.

This fixes a strange bug that has been lurking since of-platdata was
implemented. It has likely never been noticed since devices normally have
at least some devicetree properties, BSS is seldom used on SPL, the dtv
structs are normally at the end of bss and the overwriting only happens
if a driver changes its platform data.

It was discovered using sandbox_spl, which exercises more features than
a normal board might, and the critical global_data variable 'gd' happened
to be at the end of BSS.

Fixes: 9fa28190091 ("dm: core: Expand platdata for of-platdata devices")
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/core/device.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

Applied to u-boot-dm, thanks!