[PATCH 0/6] staging: fsl-mc: cleanup and minor corrections

[PATCH 0/6] staging: fsl-mc: cleanup and minor corrections

[J. German Rivera]

This patch series includes some cleanup/refactoring and minor correction
patches for the Freescale fsl-mc bus driver.

Patch 1: Name MC object devices using decimal numbers
Patch 2: Removed reordering of MC objects during bus scan
Patch 3: Bind/unbind driver when MC object is plugged/unplugged
Patch 4: Fix crash in fsl_mc_device_remove()
Patch 5: Refactored fsl_mc_object_allocator driver init/exit
Patch 6: Changed version matching rules for MC object drivers

[PATCH 1/6] staging: fsl-mc: Name MC object devices using decimal numbers

[J. German Rivera]

MC object devices were being named using hexadecimaal numbers.
This was not consistent with the object naming conventions used
by MC DPLs and the MC restool.

Signed-off-by: J. German Rivera <German.Rivera@freescale.com>
---
 drivers/staging/fsl-mc/bus/mc-bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c
index b4e9022..de15fa9 100644
--- a/drivers/staging/fsl-mc/bus/mc-bus.c
+++ b/drivers/staging/fsl-mc/bus/mc-bus.c
@@ -333,7 +333,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
 	device_initialize(&mc_dev->dev);
 	mc_dev->dev.parent = parent_dev;
 	mc_dev->dev.bus = &fsl_mc_bus_type;
-	dev_set_name(&mc_dev->dev, "%s.%x", obj_desc->type, obj_desc->id);
+	dev_set_name(&mc_dev->dev, "%s.%d", obj_desc->type, obj_desc->id);
 
 	if (strcmp(obj_desc->type, "dprc") == 0) {
 		struct fsl_mc_io *mc_io2;
-- 
2.3.3

[PATCH 2/6] staging: fsl-mc: Removed reordering of MC objects during bus scan

[J. German Rivera]

MC objects discovered during an MC bus scan were being reordered
to ensure that all allocatable objects are probed before all
non-allocatable objects. However, this is not necessary, as
drivers of non-allocatable objects, that allocate allocatable
objects in their probe function, can return -EPROBE_DEFER
if such allocations fail.

Signed-off-by: J. German Rivera <German.Rivera@freescale.com>
---
 drivers/staging/fsl-mc/bus/dprc-driver.c | 32 --------------------------------
 1 file changed, 32 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c
index 65de1d75..f02e5e4 100644
--- a/drivers/staging/fsl-mc/bus/dprc-driver.c
+++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
@@ -205,36 +205,6 @@ static void dprc_cleanup_all_resource_pools(struct fsl_mc_device *mc_bus_dev)
 		dprc_cleanup_resource_pool(mc_bus_dev, pool_type);
 }
 
-static void reorder_obj_desc_array(struct dprc_obj_desc *obj_desc_array,
-				   int num_devs)
-{
-	struct dprc_obj_desc tmp;
-	struct dprc_obj_desc *top_cursor = &obj_desc_array[0];
-	struct dprc_obj_desc *bottom_cursor = &obj_desc_array[num_devs - 1];
-
-	/*
-	 * Reorder entries in obj_desc_array so that all allocatable devices
-	 * are placed before all non-allocatable devices:
-	 *
-	 * Loop Invariant: everything before top_cursor is allocatable and
-	 * everything after bottom_cursor is non-allocatable.
-	 */
-	while (top_cursor < bottom_cursor) {
-		if (FSL_MC_IS_ALLOCATABLE(top_cursor->type)) {
-			top_cursor++;
-		} else {
-			if (FSL_MC_IS_ALLOCATABLE(bottom_cursor->type)) {
-				tmp = *bottom_cursor;
-				*bottom_cursor = *top_cursor;
-				*top_cursor = tmp;
-				top_cursor++;
-			}
-
-			bottom_cursor--;
-		}
-	}
-}
-
 /**
  * dprc_scan_objects - Discover objects in a DPRC
  *
@@ -313,8 +283,6 @@ int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev)
 				"%d out of %d devices could not be retrieved\n",
 				dprc_get_obj_failures, num_child_objects);
 		}
-
-		reorder_obj_desc_array(child_obj_desc_array, num_child_objects);
 	}
 
 	dprc_remove_devices(mc_bus_dev, child_obj_desc_array,
-- 
2.3.3

[PATCH 3/6] staging: fsl-mc: Bind/unbind driver when MC object is plugged/unplugged

[J. German Rivera]

Signed-off-by: J. German Rivera <German.Rivera@freescale.com>
---
 drivers/staging/fsl-mc/bus/dprc-driver.c | 39 +++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c
index f02e5e4..9568eea 100644
--- a/drivers/staging/fsl-mc/bus/dprc-driver.c
+++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
@@ -112,6 +112,41 @@ static struct fsl_mc_device *fsl_mc_device_lookup(struct dprc_obj_desc
 }
 
 /**
+ * check_plugged_state_change - Check change in an MC object's plugged state
+ *
+ * @mc_dev: pointer to the fsl-mc device for a given MC object
+ * @obj_desc: pointer to the MC object's descriptor in the MC
+ *
+ * If the plugged state has changed from unplugged to plugged, the fsl-mc
+ * device is bound to the corresponding device driver.
+ * If the plugged state has changed from plugged to unplugged, the fsl-mc
+ * device is unbound from the corresponding device driver.
+ */
+static void check_plugged_state_change(struct fsl_mc_device *mc_dev,
+				       struct dprc_obj_desc *obj_desc)
+{
+	int error;
+	uint32_t plugged_flag_at_mc =
+			(obj_desc->state & DPRC_OBJ_STATE_PLUGGED);
+
+	if (plugged_flag_at_mc !=
+	    (mc_dev->obj_desc.state & DPRC_OBJ_STATE_PLUGGED)) {
+		if (plugged_flag_at_mc) {
+			mc_dev->obj_desc.state |= DPRC_OBJ_STATE_PLUGGED;
+			error = device_attach(&mc_dev->dev);
+			if (error < 0) {
+				dev_err(&mc_dev->dev,
+					"device_attach() failed: %d\n",
+					error);
+			}
+		} else {
+			mc_dev->obj_desc.state &= ~DPRC_OBJ_STATE_PLUGGED;
+			device_release_driver(&mc_dev->dev);
+		}
+	}
+}
+
+/**
  * dprc_add_new_devices - Adds devices to the logical bus for a DPRC
  *
  * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
@@ -142,8 +177,10 @@ static void dprc_add_new_devices(struct fsl_mc_device *mc_bus_dev,
 		 * Check if device is already known to Linux:
 		 */
 		child_dev = fsl_mc_device_lookup(obj_desc, mc_bus_dev);
-		if (child_dev)
+		if (child_dev) {
+			check_plugged_state_change(child_dev, obj_desc);
 			continue;
+		}
 
 		error = fsl_mc_device_add(obj_desc, mc_io, &mc_bus_dev->dev,
 					  &child_dev);
-- 
2.3.3

[PATCH 4/6] staging: fsl-mc: Fix crash in fsl_mc_device_remove()

[J. German Rivera]

Only call fsl_mc_io_destroy() if the DPRC being removed
actually had an mc_io object associated with. Child DPRCs
that have not been bound to the DPRC driver or the VFIO driver
will not have an mc_io associated with them.

Signed-off-by: J. German Rivera <German.Rivera@freescale.com>
---
 drivers/staging/fsl-mc/bus/dprc-driver.c | 9 ++-------
 drivers/staging/fsl-mc/bus/mc-bus.c      | 9 +++++----
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c
index 9568eea..35c06cf 100644
--- a/drivers/staging/fsl-mc/bus/dprc-driver.c
+++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
@@ -167,7 +167,6 @@ static void dprc_add_new_devices(struct fsl_mc_device *mc_bus_dev,
 
 	for (i = 0; i < num_child_objects_in_mc; i++) {
 		struct fsl_mc_device *child_dev;
-		struct fsl_mc_io *mc_io = NULL;
 		struct dprc_obj_desc *obj_desc = &obj_desc_array[i];
 
 		if (strlen(obj_desc->type) == 0)
@@ -182,14 +181,10 @@ static void dprc_add_new_devices(struct fsl_mc_device *mc_bus_dev,
 			continue;
 		}
 
-		error = fsl_mc_device_add(obj_desc, mc_io, &mc_bus_dev->dev,
+		error = fsl_mc_device_add(obj_desc, NULL, &mc_bus_dev->dev,
 					  &child_dev);
-		if (error < 0) {
-			if (mc_io)
-				fsl_destroy_mc_io(mc_io);
-
+		if (error < 0)
 			continue;
-		}
 	}
 }
 
diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c
index de15fa9..d943d99 100644
--- a/drivers/staging/fsl-mc/bus/mc-bus.c
+++ b/drivers/staging/fsl-mc/bus/mc-bus.c
@@ -444,15 +444,16 @@ void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
 	put_device(&mc_dev->dev);
 
 	if (strcmp(mc_dev->obj_desc.type, "dprc") == 0) {
-		struct fsl_mc_io *mc_io = mc_dev->mc_io;
-
 		mc_bus = to_fsl_mc_bus(mc_dev);
-		fsl_destroy_mc_io(mc_io);
+		if (mc_dev->mc_io) {
+			fsl_destroy_mc_io(mc_dev->mc_io);
+			mc_dev->mc_io = NULL;
+		}
+
 		if (&mc_dev->dev == fsl_mc_bus_type.dev_root)
 			fsl_mc_bus_type.dev_root = NULL;
 	}
 
-	mc_dev->mc_io = NULL;
 	if (mc_bus)
 		devm_kfree(mc_dev->dev.parent, mc_bus);
 	else
-- 
2.3.3

[PATCH 5/6] staging: fsl-mc: Refactored fsl_mc_object_allocator driver init/exit

[J. German Rivera]

 The fsl_mc_allocator driver does not need to be its own module
 as it is tightly integrated into the MC bus main driver. It is really
 just a sub-component of the MC bus driver. By not making fsl_mc_allocator
 its own module, we can have more control of when its initialization happens
 and we want it to happen before any driver that depends on the MC bus
 driver gets initialized.

Signed-off-by: J. German Rivera <German.Rivera@freescale.com>
---
 drivers/staging/fsl-mc/bus/Makefile         | 12 +++++-------
 drivers/staging/fsl-mc/bus/mc-allocator.c   | 12 ++++++++----
 drivers/staging/fsl-mc/bus/mc-bus.c         |  8 ++++++++
 drivers/staging/fsl-mc/include/mc-private.h |  4 ++++
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/Makefile b/drivers/staging/fsl-mc/bus/Makefile
index bd09fc8..25433a9 100644
--- a/drivers/staging/fsl-mc/bus/Makefile
+++ b/drivers/staging/fsl-mc/bus/Makefile
@@ -5,15 +5,13 @@
 #
 # This file is released under the GPLv2
 #
-obj-$(CONFIG_FSL_MC_BUS) += mc-bus-driver.o \
-			    mc-allocator-driver.o
+obj-$(CONFIG_FSL_MC_BUS) += mc-bus-driver.o
 
 mc-bus-driver-objs := mc-bus.o \
 		      mc-sys.o \
 		      dprc.o \
 		      dpmng.o \
-		      dprc-driver.o
-
-mc-allocator-driver-objs := mc-allocator.o \
-			    dpmcp.o \
-			    dpbp.o
+		      dprc-driver.o \
+		      mc-allocator.o \
+		      dpmcp.o \
+		      dpbp.o
diff --git a/drivers/staging/fsl-mc/bus/mc-allocator.c b/drivers/staging/fsl-mc/bus/mc-allocator.c
index cc4a7d3..e36235d 100644
--- a/drivers/staging/fsl-mc/bus/mc-allocator.c
+++ b/drivers/staging/fsl-mc/bus/mc-allocator.c
@@ -562,8 +562,12 @@ static struct fsl_mc_driver fsl_mc_allocator_driver = {
 	.remove = fsl_mc_allocator_remove,
 };
 
-module_fsl_mc_driver(fsl_mc_allocator_driver);
+int __init fsl_mc_allocator_driver_init(void)
+{
+	return fsl_mc_driver_register(&fsl_mc_allocator_driver);
+}
 
-MODULE_AUTHOR("Freescale Semiconductor Inc.");
-MODULE_DESCRIPTION("Freescale's MC object device allocator");
-MODULE_LICENSE("GPL");
+void __exit fsl_mc_allocator_driver_exit(void)
+{
+	fsl_mc_driver_unregister(&fsl_mc_allocator_driver);
+}
diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c
index d943d99..b347db9 100644
--- a/drivers/staging/fsl-mc/bus/mc-bus.c
+++ b/drivers/staging/fsl-mc/bus/mc-bus.c
@@ -730,8 +730,15 @@ static int __init fsl_mc_bus_driver_init(void)
 	if (error < 0)
 		goto error_cleanup_driver;
 
+	error = fsl_mc_allocator_driver_init();
+	if (error < 0)
+		goto error_cleanup_dprc_driver;
+
 	return 0;
 
+error_cleanup_dprc_driver:
+	dprc_driver_exit();
+
 error_cleanup_driver:
 	platform_driver_unregister(&fsl_mc_bus_driver);
 
@@ -750,6 +757,7 @@ static void __exit fsl_mc_bus_driver_exit(void)
 	if (WARN_ON(!mc_dev_cache))
 		return;
 
+	fsl_mc_allocator_driver_exit();
 	dprc_driver_exit();
 	platform_driver_unregister(&fsl_mc_bus_driver);
 	bus_unregister(&fsl_mc_bus_type);
diff --git a/drivers/staging/fsl-mc/include/mc-private.h b/drivers/staging/fsl-mc/include/mc-private.h
index 8e67075..c045f49 100644
--- a/drivers/staging/fsl-mc/include/mc-private.h
+++ b/drivers/staging/fsl-mc/include/mc-private.h
@@ -102,6 +102,10 @@ int __init dprc_driver_init(void);
 
 void __exit dprc_driver_exit(void);
 
+int __init fsl_mc_allocator_driver_init(void);
+
+void __exit fsl_mc_allocator_driver_exit(void);
+
 int __must_check fsl_mc_resource_allocate(struct fsl_mc_bus *mc_bus,
 					  enum fsl_mc_pool_type pool_type,
 					  struct fsl_mc_resource
-- 
2.3.3

[PATCH 6/6] staging: fsl-mc: Changed version matching rules for MC object drivers

[J. German Rivera]

Before this change, we were requiring a complete version match (major and
minor version numbers) between MC objects and corresponding drivers, to
allow MC objects to be bound to their drivers. We realized that a mismatch
in minor version numbers should be tolerated, as long as the major version
numbers match. This allows the driver to decide what to do in the minor
version mismatch case. For example, a driver may decide to run with
downgraded functionality if the MC firmware object has older minor version
number than the driver. Also, a driver with older minor version than the
MC firmware object may decide to run even though it cannot use newer
functionality of the MC object.

As part of this change, the dpmng Flib version was also updated
to match the latest MC firmware version.

Signed-off-by: J. German Rivera <German.Rivera@freescale.com>
---
 drivers/staging/fsl-mc/bus/mc-bus.c    | 29 +++++++++++++++++++++++++----
 drivers/staging/fsl-mc/include/dpmng.h |  2 +-
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c
index b347db9..23512d0 100644
--- a/drivers/staging/fsl-mc/bus/mc-bus.c
+++ b/drivers/staging/fsl-mc/bus/mc-bus.c
@@ -36,6 +36,8 @@ static int fsl_mc_bus_match(struct device *dev, struct device_driver *drv)
 	struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
 	struct fsl_mc_driver *mc_drv = to_fsl_mc_driver(drv);
 	bool found = false;
+	bool major_version_mismatch = false;
+	bool minor_version_mismatch = false;
 
 	if (WARN_ON(!fsl_mc_bus_type.dev_root))
 		goto out;
@@ -57,14 +59,33 @@ static int fsl_mc_bus_match(struct device *dev, struct device_driver *drv)
 	 */
 	for (id = mc_drv->match_id_table; id->vendor != 0x0; id++) {
 		if (id->vendor == mc_dev->obj_desc.vendor &&
-		    strcmp(id->obj_type, mc_dev->obj_desc.type) == 0 &&
-		    id->ver_major == mc_dev->obj_desc.ver_major &&
-		    id->ver_minor == mc_dev->obj_desc.ver_minor) {
-			found = true;
+		    strcmp(id->obj_type, mc_dev->obj_desc.type) == 0) {
+			if (id->ver_major == mc_dev->obj_desc.ver_major) {
+				found = true;
+				if (id->ver_minor != mc_dev->obj_desc.ver_minor)
+					minor_version_mismatch = true;
+			} else {
+				major_version_mismatch = true;
+			}
+
 			break;
 		}
 	}
 
+	if (major_version_mismatch) {
+		dev_warn(dev,
+			 "Major version mismatch: driver version %u.%u, MC object version %u.%u\n",
+			 id->ver_major, id->ver_minor,
+			 mc_dev->obj_desc.ver_major,
+			 mc_dev->obj_desc.ver_minor);
+	} else if (minor_version_mismatch) {
+		dev_warn(dev,
+			 "Minor version mismatch: driver version %u.%u, MC object version %u.%u\n",
+			 id->ver_major, id->ver_minor,
+			 mc_dev->obj_desc.ver_major,
+			 mc_dev->obj_desc.ver_minor);
+	}
+
 out:
 	dev_dbg(dev, "%smatched\n", found ? "" : "not ");
 	return found;
diff --git a/drivers/staging/fsl-mc/include/dpmng.h b/drivers/staging/fsl-mc/include/dpmng.h
index 0fc0a57..1b052b8 100644
--- a/drivers/staging/fsl-mc/include/dpmng.h
+++ b/drivers/staging/fsl-mc/include/dpmng.h
@@ -41,7 +41,7 @@ struct fsl_mc_io;
 /**
  * Management Complex firmware version information
  */
-#define MC_VER_MAJOR 5
+#define MC_VER_MAJOR 6
 #define MC_VER_MINOR 0
 
 /**
-- 
2.3.3

Re: [PATCH 6/6] staging: fsl-mc: Changed version matching rules for MC object drivers

[Alexander Graf]

On 03/27/2015 10:01 PM, J. German Rivera wrote:
> Before this change, we were requiring a complete version match (major and
> minor version numbers) between MC objects and corresponding drivers, to
> allow MC objects to be bound to their drivers. We realized that a mismatch
> in minor version numbers should be tolerated, as long as the major version
> numbers match. This allows the driver to decide what to do in the minor
> version mismatch case. For example, a driver may decide to run with
> downgraded functionality if the MC firmware object has older minor version
> number than the driver. Also, a driver with older minor version than the
> MC firmware object may decide to run even though it cannot use newer
> functionality of the MC object.
>
> As part of this change, the dpmng Flib version was also updated
> to match the latest MC firmware version.
>
> Signed-off-by: J. German Rivera <German.Rivera@freescale.com>

I think this is a step into the right direction, but you really don't 
want to match only when the minor equals. Usually you'd like something like

   if (cur_minor > max_minor) {
     dev_warn("Unknown version %d.%d of fsl-mc detected. Please update 
your kernel if you encounter problems.")
   }

but always assume that cur_minor < max_minor works. In cases where the 
protocol did change between minors, add code to support the older minors 
as well.


Alex

Re: [PATCH 0/6] staging: fsl-mc: cleanup and minor corrections

[Alexander Graf]

On 03/27/2015 10:01 PM, J. German Rivera wrote:
> This patch series includes some cleanup/refactoring and minor correction
> patches for the Freescale fsl-mc bus driver.
>
> Patch 1: Name MC object devices using decimal numbers
> Patch 2: Removed reordering of MC objects during bus scan
> Patch 3: Bind/unbind driver when MC object is plugged/unplugged
> Patch 4: Fix crash in fsl_mc_device_remove()
> Patch 5: Refactored fsl_mc_object_allocator driver init/exit
> Patch 6: Changed version matching rules for MC object drivers
>

Acked-by: Alexander Graf <agraf@suse.de>


Alex