2015-04-28 17:47:13

by J. German Rivera

[permalink] [raw]
Subject: [PATCH 0/7] staging: fsl-mc: New functionality to the MC bus driver

This patch series includes new functionality for the Freescale fsl-mc
bus driver.

Patch 1: MC bus IRQ support
Patch 2: add device binding path 'driver_override'
Patch 3: Propagate driver_override for a child DPRC's children
Patch 4: Upgraded MC bus driver to match MC fw 7.0.0
Patch 5: Allow the MC bus driver to run without GIC support
Patch 6: Add locking to serialize mc_send_command() calls
Patch 7: Use DPMCP IRQ and completion var to wait for MC


2015-04-28 18:02:34

by J. German Rivera

[permalink] [raw]
Subject: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

All the IRQs for DPAA2 objects in the same DPRC must use
the ICID of that DPRC, as their device Id in the GIC-ITS.
Thus, all these IRQs must share the same ITT table in GIC. As
a result, a pool of IRQs with the same device Id must be
preallocated per DPRC (fsl-mc bus instance). So, the fsl-mc
bus object allocator is extended to also provide services
to allocate IRQs to DPAA2 devices, from their parent fsl-mc bus
IRQ pool.

Also, the interrupt handler for DPRC IRQs is added. DPRC IRQs are
generated for hot plug events related to DPAA2 objects in a given
DPRC. These events include, creating/destroying DPAA2 objects in
the DPRC, changing the "plugged" state of DPAA2 objects and moving
objects between DPRCs.

Signed-off-by: J. German Rivera <[email protected]>
Change-Id: I2a986c465989c3811de19cfe9ed0b77168250cb1
Reviewed-on: http://git.am.freescale.net:8181/33626
Tested-by: Review Code-CDREVIEW <[email protected]>
Reviewed-by: Stuart Yoder <[email protected]>
---
drivers/staging/fsl-mc/TODO | 3 +
drivers/staging/fsl-mc/bus/dprc-driver.c | 337 +++++++++++++++++++++++++-
drivers/staging/fsl-mc/bus/mc-allocator.c | 105 +++++++++
drivers/staging/fsl-mc/bus/mc-bus.c | 352 +++++++++++++++++++++++++++-
drivers/staging/fsl-mc/include/mc-private.h | 26 +-
drivers/staging/fsl-mc/include/mc.h | 25 ++
6 files changed, 837 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/fsl-mc/TODO b/drivers/staging/fsl-mc/TODO
index d78288b..1b8868d 100644
--- a/drivers/staging/fsl-mc/TODO
+++ b/drivers/staging/fsl-mc/TODO
@@ -8,6 +8,9 @@
* Add at least one device driver for a DPAA2 object (child device of the
fsl-mc bus).

+* Enable code blocks guarded by #ifdef GIC_ITS_MC_SUPPORT, when GIC-ITS
+ support for the MC MSIs gets merged.
+
Please send any patches to Greg Kroah-Hartman <[email protected]>,
[email protected], [email protected],
[email protected]
diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c
index 35c06cf..8531ba8 100644
--- a/drivers/staging/fsl-mc/bus/dprc-driver.c
+++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
@@ -13,6 +13,7 @@
#include "../include/mc-sys.h"
#include <linux/module.h>
#include <linux/slab.h>
+#include <linux/interrupt.h>
#include "dprc-cmd.h"

struct dprc_child_objs {
@@ -241,6 +242,7 @@ static void dprc_cleanup_all_resource_pools(struct fsl_mc_device *mc_bus_dev)
* dprc_scan_objects - Discover objects in a DPRC
*
* @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
+ * @total_irq_count: total number of IRQs needed by objects in the DPRC.
*
* Detects objects added and removed from a DPRC and synchronizes the
* state of the Linux bus driver, MC by adding and removing
@@ -254,11 +256,13 @@ static void dprc_cleanup_all_resource_pools(struct fsl_mc_device *mc_bus_dev)
* populated before they can get allocation requests from probe callbacks
* of the device drivers for the non-allocatable devices.
*/
-int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev)
+int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev,
+ unsigned int *total_irq_count)
{
int num_child_objects;
int dprc_get_obj_failures;
int error;
+ unsigned int irq_count = mc_bus_dev->obj_desc.irq_count;
struct dprc_obj_desc *child_obj_desc_array = NULL;

error = dprc_get_obj_count(mc_bus_dev->mc_io,
@@ -305,6 +309,7 @@ int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev)
continue;
}

+ irq_count += obj_desc->irq_count;
dev_dbg(&mc_bus_dev->dev,
"Discovered object: type %s, id %d\n",
obj_desc->type, obj_desc->id);
@@ -317,6 +322,7 @@ int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev)
}
}

+ *total_irq_count = irq_count;
dprc_remove_devices(mc_bus_dev, child_obj_desc_array,
num_child_objects);

@@ -339,9 +345,10 @@ EXPORT_SYMBOL_GPL(dprc_scan_objects);
* bus driver with the actual state of the MC by adding and removing
* devices as appropriate.
*/
-int dprc_scan_container(struct fsl_mc_device *mc_bus_dev)
+static int dprc_scan_container(struct fsl_mc_device *mc_bus_dev)
{
int error;
+ unsigned int irq_count;
struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_bus_dev);

dprc_init_all_resource_pools(mc_bus_dev);
@@ -350,17 +357,313 @@ int dprc_scan_container(struct fsl_mc_device *mc_bus_dev)
* Discover objects in the DPRC:
*/
mutex_lock(&mc_bus->scan_mutex);
- error = dprc_scan_objects(mc_bus_dev);
+ error = dprc_scan_objects(mc_bus_dev, &irq_count);
mutex_unlock(&mc_bus->scan_mutex);
if (error < 0)
goto error;

+ if (!mc_bus->irq_resources) {
+ irq_count += FSL_MC_IRQ_POOL_MAX_EXTRA_IRQS;
+ error = fsl_mc_populate_irq_pool(mc_bus, irq_count);
+ if (error < 0)
+ goto error;
+ }
+
return 0;
error:
dprc_cleanup_all_resource_pools(mc_bus_dev);
return error;
}
-EXPORT_SYMBOL_GPL(dprc_scan_container);
+
+/**
+ * dprc_irq0_handler - Regular ISR for DPRC interrupt 0
+ *
+ * @irq: IRQ number of the interrupt being handled
+ * @arg: Pointer to device structure
+ */
+static irqreturn_t dprc_irq0_handler(int irq_num, void *arg)
+{
+ return IRQ_WAKE_THREAD;
+}
+
+/**
+ * dprc_irq0_handler_thread - Handler thread function for DPRC interrupt 0
+ *
+ * @irq: IRQ number of the interrupt being handled
+ * @arg: Pointer to device structure
+ */
+static irqreturn_t dprc_irq0_handler_thread(int irq_num, void *arg)
+{
+ int error;
+ uint32_t status;
+ struct device *dev = (struct device *)arg;
+ struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+ struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_dev);
+ struct fsl_mc_io *mc_io = mc_dev->mc_io;
+ int irq_index = 0;
+
+ dev_dbg(dev, "DPRC IRQ %d\n", irq_num);
+ if (WARN_ON(!(mc_dev->flags & FSL_MC_IS_DPRC)))
+ return IRQ_HANDLED;
+
+ mutex_lock(&mc_bus->scan_mutex);
+ if (WARN_ON(mc_dev->irqs[irq_index]->irq_number != (uint32_t)irq_num))
+ goto out;
+
+ error = dprc_get_irq_status(mc_io, mc_dev->mc_handle, irq_index,
+ &status);
+ if (error < 0) {
+ dev_err(dev,
+ "dprc_get_irq_status() failed: %d\n", error);
+ goto out;
+ }
+
+ error = dprc_clear_irq_status(mc_io, mc_dev->mc_handle, irq_index,
+ status);
+ if (error < 0) {
+ dev_err(dev,
+ "dprc_clear_irq_status() failed: %d\n", error);
+ goto out;
+ }
+
+ if (status & (DPRC_IRQ_EVENT_OBJ_ADDED |
+ DPRC_IRQ_EVENT_OBJ_REMOVED |
+ DPRC_IRQ_EVENT_CONTAINER_DESTROYED |
+ DPRC_IRQ_EVENT_OBJ_DESTROYED |
+ DPRC_IRQ_EVENT_OBJ_CREATED)) {
+ unsigned int irq_count;
+
+ error = dprc_scan_objects(mc_dev, &irq_count);
+ if (error < 0) {
+ dev_err(dev, "dprc_scan_objects() failed: %d\n", error);
+ goto out;
+ }
+
+ WARN_ON((int16_t)irq_count < 0);
+
+ if ((int16_t)irq_count >
+ mc_bus->resource_pools[FSL_MC_POOL_IRQ].max_count) {
+ dev_warn(dev,
+ "IRQs needed (%u) exceed IRQs preallocated (%u)\n",
+ irq_count,
+ mc_bus->resource_pools[FSL_MC_POOL_IRQ].
+ max_count);
+ }
+ }
+
+out:
+ mutex_unlock(&mc_bus->scan_mutex);
+ return IRQ_HANDLED;
+}
+
+/*
+ * Disable and clear interrupts for a given DPRC object
+ */
+static int disable_dprc_irqs(struct fsl_mc_device *mc_dev)
+{
+ int i;
+ int error;
+ struct fsl_mc_io *mc_io = mc_dev->mc_io;
+ int irq_count = mc_dev->obj_desc.irq_count;
+
+ if (WARN_ON(irq_count == 0))
+ return -EINVAL;
+
+ for (i = 0; i < irq_count; i++) {
+ /*
+ * Disable generation of interrupt i, while we configure it:
+ */
+ error = dprc_set_irq_enable(mc_io, mc_dev->mc_handle, i, 0);
+ if (error < 0) {
+ dev_err(&mc_dev->dev,
+ "dprc_set_irq_enable() failed: %d\n", error);
+
+ return error;
+ }
+
+ /*
+ * Disable all interrupt causes for interrupt i:
+ */
+ error = dprc_set_irq_mask(mc_io, mc_dev->mc_handle, i, 0x0);
+ if (error < 0) {
+ dev_err(&mc_dev->dev,
+ "dprc_set_irq_mask() failed: %d\n", error);
+
+ return error;
+ }
+
+ /*
+ * Clear any leftover interrupt i:
+ */
+ error = dprc_clear_irq_status(mc_io, mc_dev->mc_handle, i,
+ ~0x0U);
+ if (error < 0) {
+ dev_err(&mc_dev->dev,
+ "dprc_clear_irq_status() failed: %d\n",
+ error);
+ return error;
+ }
+ }
+
+ return 0;
+}
+
+static void unregister_dprc_irq_handlers(struct fsl_mc_device *mc_dev)
+{
+ int i;
+ struct fsl_mc_device_irq *irq;
+ int irq_count = mc_dev->obj_desc.irq_count;
+
+ for (i = 0; i < irq_count; i++) {
+ irq = mc_dev->irqs[i];
+ devm_free_irq(&mc_dev->dev, irq->irq_number,
+ &mc_dev->dev);
+ }
+}
+
+static int register_dprc_irq_handlers(struct fsl_mc_device *mc_dev)
+{
+ static const struct irq_handler {
+ irq_handler_t irq_handler;
+ irq_handler_t irq_handler_thread;
+ const char *irq_name;
+ } irq_handlers[] = {
+ [0] = {
+ .irq_handler = dprc_irq0_handler,
+ .irq_handler_thread = dprc_irq0_handler_thread,
+ .irq_name = "FSL MC DPRC irq0",
+ },
+ };
+
+ unsigned int i;
+ int error;
+ struct fsl_mc_device_irq *irq;
+ unsigned int num_irq_handlers_registered = 0;
+ int irq_count = mc_dev->obj_desc.irq_count;
+
+ if (WARN_ON(irq_count != ARRAY_SIZE(irq_handlers)))
+ return -EINVAL;
+
+ for (i = 0; i < ARRAY_SIZE(irq_handlers); i++) {
+ irq = mc_dev->irqs[i];
+ error = devm_request_threaded_irq(&mc_dev->dev,
+ irq->irq_number,
+ irq_handlers[i].irq_handler,
+ irq_handlers[i].
+ irq_handler_thread,
+ IRQF_NO_SUSPEND |
+ IRQF_ONESHOT,
+ irq_handlers[i].irq_name,
+ &mc_dev->dev);
+ if (error < 0) {
+ dev_err(&mc_dev->dev,
+ "devm_request_threaded_irq() failed: %d\n",
+ error);
+ goto error_unregister_irq_handlers;
+ }
+
+ /*
+ * Program the MSI (paddr, value) pair in the device:
+ *
+ * TODO: This needs to be moved to mc_bus_msi_domain_write_msg()
+ * when the MC object-independent dprc_set_irq() flib API
+ * becomes available
+ */
+ error = dprc_set_irq(mc_dev->mc_io, mc_dev->mc_handle,
+ i, irq->msi_paddr,
+ irq->msi_value,
+ irq->irq_number);
+ if (error < 0) {
+ dev_err(&mc_dev->dev,
+ "mc_set_irq() failed: %d\n", error);
+ goto error_unregister_irq_handlers;
+ }
+
+ num_irq_handlers_registered++;
+ }
+
+ return 0;
+
+error_unregister_irq_handlers:
+ for (i = 0; i < num_irq_handlers_registered; i++) {
+ irq = mc_dev->irqs[i];
+ devm_free_irq(&mc_dev->dev, irq->irq_number,
+ &mc_dev->dev);
+ }
+
+ return error;
+}
+
+static int enable_dprc_irqs(struct fsl_mc_device *mc_dev)
+{
+ int i;
+ int error;
+ int irq_count = mc_dev->obj_desc.irq_count;
+
+ for (i = 0; i < irq_count; i++) {
+ /*
+ * Enable all interrupt causes for the interrupt:
+ */
+ error = dprc_set_irq_mask(mc_dev->mc_io,
+ mc_dev->mc_handle,
+ i,
+ ~0x0u);
+ if (error < 0) {
+ dev_err(&mc_dev->dev,
+ "dprc_set_irq_mask() failed: %d\n", error);
+
+ return error;
+ }
+
+ /*
+ * Enable generation of the interrupt:
+ */
+ error = dprc_set_irq_enable(mc_dev->mc_io,
+ mc_dev->mc_handle,
+ i, 1);
+ if (error < 0) {
+ dev_err(&mc_dev->dev,
+ "dprc_set_irq_enable() failed: %d\n", error);
+
+ return error;
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * Setup interrupts for a given DPRC device
+ */
+static int dprc_setup_irqs(struct fsl_mc_device *mc_dev)
+{
+ int error;
+
+ error = fsl_mc_allocate_irqs(mc_dev);
+ if (error < 0)
+ return error;
+
+ error = disable_dprc_irqs(mc_dev);
+ if (error < 0)
+ goto error_free_irqs;
+
+ error = register_dprc_irq_handlers(mc_dev);
+ if (error < 0)
+ goto error_free_irqs;
+
+ error = enable_dprc_irqs(mc_dev);
+ if (error < 0)
+ goto error_unregister_irq_handlers;
+
+ return 0;
+
+error_unregister_irq_handlers:
+ unregister_dprc_irq_handlers(mc_dev);
+
+error_free_irqs:
+ fsl_mc_free_irqs(mc_dev);
+ return error;
+}

/**
* dprc_probe - callback invoked when a DPRC is being bound to this driver
@@ -415,10 +718,20 @@ static int dprc_probe(struct fsl_mc_device *mc_dev)
if (error < 0)
goto error_cleanup_open;

+ /*
+ * Configure interrupts for the DPRC object associated with this MC bus:
+ */
+ error = dprc_setup_irqs(mc_dev);
+ if (error < 0)
+ goto error_cleanup_open;
+
dev_info(&mc_dev->dev, "DPRC device bound to driver");
return 0;

error_cleanup_open:
+ if (mc_bus->irq_resources)
+ fsl_mc_cleanup_irq_pool(mc_bus);
+
(void)dprc_close(mc_dev->mc_io, mc_dev->mc_handle);

error_cleanup_mc_io:
@@ -426,6 +739,16 @@ error_cleanup_mc_io:
return error;
}

+/*
+ * Tear down interrupts for a given DPRC object
+ */
+static void dprc_teardown_irqs(struct fsl_mc_device *mc_dev)
+{
+ (void)disable_dprc_irqs(mc_dev);
+ unregister_dprc_irq_handlers(mc_dev);
+ fsl_mc_free_irqs(mc_dev);
+}
+
/**
* dprc_remove - callback invoked when a DPRC is being unbound from this driver
*
@@ -439,18 +762,24 @@ error_cleanup_mc_io:
static int dprc_remove(struct fsl_mc_device *mc_dev)
{
int error;
+ struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_dev);

if (WARN_ON(strcmp(mc_dev->obj_desc.type, "dprc") != 0))
return -EINVAL;
if (WARN_ON(!mc_dev->mc_io))
return -EINVAL;

+ if (WARN_ON(!mc_bus->irq_resources))
+ return -EINVAL;
+
+ dprc_teardown_irqs(mc_dev);
device_for_each_child(&mc_dev->dev, NULL, __fsl_mc_device_remove);
dprc_cleanup_all_resource_pools(mc_dev);
error = dprc_close(mc_dev->mc_io, mc_dev->mc_handle);
if (error < 0)
dev_err(&mc_dev->dev, "dprc_close() failed: %d\n", error);

+ fsl_mc_cleanup_irq_pool(mc_bus);
dev_info(&mc_dev->dev, "DPRC device unbound from driver");
return 0;
}
diff --git a/drivers/staging/fsl-mc/bus/mc-allocator.c b/drivers/staging/fsl-mc/bus/mc-allocator.c
index e36235d..27497e7 100644
--- a/drivers/staging/fsl-mc/bus/mc-allocator.c
+++ b/drivers/staging/fsl-mc/bus/mc-allocator.c
@@ -160,6 +160,7 @@ static const char *const fsl_mc_pool_type_strings[] = {
[FSL_MC_POOL_DPMCP] = "dpmcp",
[FSL_MC_POOL_DPBP] = "dpbp",
[FSL_MC_POOL_DPCON] = "dpcon",
+ [FSL_MC_POOL_IRQ] = "irq",
};

static int __must_check object_type_to_pool_type(const char *object_type,
@@ -473,6 +474,110 @@ void fsl_mc_object_free(struct fsl_mc_device *mc_adev)
EXPORT_SYMBOL_GPL(fsl_mc_object_free);

/**
+ * It allocates the IRQs required by a given MC object device. The
+ * IRQs are allocated from the interrupt pool associated with the
+ * MC bus that contains the device, if the device is not a DPRC device.
+ * Otherwise, the IRQs are allocated from the interrupt pool associated
+ * with the MC bus that represents the DPRC device itself.
+ */
+int __must_check fsl_mc_allocate_irqs(struct fsl_mc_device *mc_dev)
+{
+ int i;
+ int irq_count;
+ int res_allocated_count = 0;
+ int error = -EINVAL;
+ struct fsl_mc_device_irq **irqs = NULL;
+ struct fsl_mc_bus *mc_bus;
+ struct fsl_mc_resource_pool *res_pool;
+
+ if (WARN_ON(mc_dev->irqs))
+ goto error;
+
+ irq_count = mc_dev->obj_desc.irq_count;
+ if (WARN_ON(irq_count == 0))
+ goto error;
+
+ if (strcmp(mc_dev->obj_desc.type, "dprc") == 0)
+ mc_bus = to_fsl_mc_bus(mc_dev);
+ else
+ mc_bus = to_fsl_mc_bus(to_fsl_mc_device(mc_dev->dev.parent));
+
+ if (WARN_ON(!mc_bus->irq_resources))
+ goto error;
+
+ res_pool = &mc_bus->resource_pools[FSL_MC_POOL_IRQ];
+ if (res_pool->free_count < irq_count) {
+ dev_err(&mc_dev->dev,
+ "Not able to allocate %u irqs for device\n", irq_count);
+ error = -ENOSPC;
+ goto error;
+ }
+
+ irqs = devm_kzalloc(&mc_dev->dev, irq_count * sizeof(irqs[0]),
+ GFP_KERNEL);
+ if (!irqs) {
+ error = -ENOMEM;
+ dev_err(&mc_dev->dev, "No memory to allocate irqs[]\n");
+ goto error;
+ }
+
+ for (i = 0; i < irq_count; i++) {
+ struct fsl_mc_resource *resource;
+
+ error = fsl_mc_resource_allocate(mc_bus, FSL_MC_POOL_IRQ,
+ &resource);
+ if (error < 0)
+ goto error;
+
+ irqs[i] = to_fsl_mc_irq(resource);
+ res_allocated_count++;
+ }
+
+ mc_dev->irqs = irqs;
+ return 0;
+error:
+ for (i = 0; i < res_allocated_count; i++)
+ fsl_mc_resource_free(&irqs[i]->resource);
+
+ if (irqs)
+ devm_kfree(&mc_dev->dev, irqs);
+
+ return error;
+}
+EXPORT_SYMBOL_GPL(fsl_mc_allocate_irqs);
+
+/*
+ * It frees the IRQs that were allocated for a MC object device, by
+ * returning them to the corresponding interrupt pool.
+ */
+void fsl_mc_free_irqs(struct fsl_mc_device *mc_dev)
+{
+ int i;
+ int irq_count;
+ struct fsl_mc_bus *mc_bus;
+
+ if (WARN_ON(!mc_dev->irqs))
+ return;
+
+ irq_count = mc_dev->obj_desc.irq_count;
+
+ if (strcmp(mc_dev->obj_desc.type, "dprc") == 0)
+ mc_bus = to_fsl_mc_bus(mc_dev);
+ else
+ mc_bus = to_fsl_mc_bus(to_fsl_mc_device(mc_dev->dev.parent));
+
+ if (WARN_ON(!mc_bus->irq_resources))
+ return;
+
+ for (i = 0; i < irq_count; i++)
+ fsl_mc_resource_free(&mc_dev->irqs[i]->resource);
+
+ devm_kfree(&mc_dev->dev, mc_dev->irqs);
+ mc_dev->irqs = NULL;
+}
+EXPORT_SYMBOL_GPL(fsl_mc_free_irqs);
+
+/**
* fsl_mc_allocator_probe - callback invoked when an allocatable device is
* being added to the system
*/
diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c
index 23512d0..7332b26 100644
--- a/drivers/staging/fsl-mc/bus/mc-bus.c
+++ b/drivers/staging/fsl-mc/bus/mc-bus.c
@@ -15,11 +15,27 @@
#include <linux/of_address.h>
#include <linux/ioport.h>
#include <linux/slab.h>
+#include <linux/irqchip/arm-gic-v3.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
#include <linux/limits.h>
+#include <linux/bitops.h>
#include "../include/dpmng.h"
#include "../include/mc-sys.h"
#include "dprc-cmd.h"

+/*
+ * IOMMU stream ID flags
+ */
+#define STREAM_ID_PL_MASK BIT(9) /* privilege level */
+#define STREAM_ID_BMT_MASK BIT(8) /* bypass memory translation */
+#define STREAM_ID_VA_MASK BIT(7) /* virtual address translation
+ * (two-stage translation) */
+#define STREAM_ID_ICID_MASK (BIT(7) - 1) /* isolation context ID
+ * (translation context) */
+
+#define MAX_STREAM_ID_ICID STREAM_ID_ICID_MASK
+
static struct kmem_cache *mc_dev_cache;

/**
@@ -295,8 +311,9 @@ static int fsl_mc_device_get_mmio_regions(struct fsl_mc_device *mc_dev,
&regions[i].start);
if (error < 0) {
dev_err(parent_dev,
- "Invalid MC address: %#llx\n",
- region_desc.base_paddr);
+ "Invalid MC address: %#llx (for %s.%d\'s region %d)\n",
+ region_desc.base_paddr,
+ obj_desc->type, obj_desc->id, i);
goto error_cleanup_regions;
}

@@ -352,6 +369,13 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
mc_dev->obj_desc = *obj_desc;
mc_dev->mc_io = mc_io;
device_initialize(&mc_dev->dev);
+
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
+ INIT_LIST_HEAD(&mc_dev->dev.msi_list);
+#endif
mc_dev->dev.parent = parent_dev;
mc_dev->dev.bus = &fsl_mc_bus_type;
dev_set_name(&mc_dev->dev, "%s.%d", obj_desc->type, obj_desc->id);
@@ -482,6 +506,312 @@ void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
}
EXPORT_SYMBOL_GPL(fsl_mc_device_remove);

+/*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
+static int mc_bus_msi_prepare(struct irq_domain *domain, struct device *dev,
+ int nvec, msi_alloc_info_t *info)
+{
+ int error;
+ u32 its_dev_id;
+ struct dprc_attributes dprc_attr;
+ struct fsl_mc_device *mc_bus_dev = to_fsl_mc_device(dev);
+
+ if (WARN_ON(!(mc_bus_dev->flags & FSL_MC_IS_DPRC)))
+ return -EINVAL;
+
+ error = dprc_get_attributes(mc_bus_dev->mc_io,
+ mc_bus_dev->mc_handle, &dprc_attr);
+ if (error < 0) {
+ dev_err(&mc_bus_dev->dev,
+ "dprc_get_attributes() failed: %d\n",
+ error);
+ return error;
+ }
+
+ /*
+ * Build the device Id to be passed to the GIC-ITS:
+ *
+ * NOTE: This device id corresponds to the IOMMU stream ID
+ * associated with the DPRC object.
+ */
+ its_dev_id = mc_bus_dev->icid;
+ if (its_dev_id > STREAM_ID_ICID_MASK) {
+ dev_err(&mc_bus_dev->dev,
+ "Invalid ICID: %#x\n", its_dev_id);
+ return -ERANGE;
+ }
+
+ if (dprc_attr.options & DPRC_CFG_OPT_IOMMU_BYPASS)
+ its_dev_id |= STREAM_ID_PL_MASK | STREAM_ID_BMT_MASK;
+
+ return __its_msi_prepare(domain->parent, its_dev_id, dev, nvec, info);
+}
+
+static void mc_bus_mask_msi_irq(struct irq_data *d)
+{
+ /* Bus specefic Mask */
+ irq_chip_mask_parent(d);
+}
+
+static void mc_bus_unmask_msi_irq(struct irq_data *d)
+{
+ /* Bus specefic unmask */
+ irq_chip_unmask_parent(d);
+}
+
+static void mc_bus_msi_domain_write_msg(struct irq_data *irq_data,
+ struct msi_msg *msg)
+{
+ struct msi_desc *msi_entry = irq_data->msi_desc;
+ struct fsl_mc_device *mc_bus_dev = to_fsl_mc_device(msi_entry->dev);
+ struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_bus_dev);
+ struct fsl_mc_device_irq *irq_res =
+ &mc_bus->irq_resources[msi_entry->msi_attrib.entry_nr];
+
+ if (irq_res->irq_number == irq_data->irq) {
+ /*
+ * write msg->address_hi/lo to irq_resource
+ */
+ irq_res->msi_paddr =
+ ((u64)msg->address_hi << 32) | msg->address_lo;
+ irq_res->msi_value = msg->data;
+ }
+}
+
+static struct irq_chip mc_bus_msi_irq_chip = {
+ .name = "fsl-mc-bus-msi",
+ .irq_unmask = mc_bus_unmask_msi_irq,
+ .irq_mask = mc_bus_mask_msi_irq,
+ .irq_eoi = irq_chip_eoi_parent,
+ .irq_write_msi_msg = mc_bus_msi_domain_write_msg,
+};
+
+static struct msi_domain_ops mc_bus_msi_ops = {
+ .msi_prepare = mc_bus_msi_prepare,
+};
+
+static struct msi_domain_info mc_bus_msi_domain_info = {
+ .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+ MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX),
+ .ops = &mc_bus_msi_ops,
+ .chip = &mc_bus_msi_irq_chip,
+};
+
+static int create_mc_irq_domain(struct platform_device *mc_pdev,
+ struct irq_domain **new_irq_domain)
+{
+ int error;
+ struct device_node *its_of_node;
+ struct irq_domain *its_domain;
+ struct irq_domain *irq_domain;
+ struct device_node *mc_of_node = mc_pdev->dev.of_node;
+
+ its_of_node = of_parse_phandle(mc_of_node, "lpi-parent", 0);
+ if (!its_of_node) {
+ dev_err(&mc_pdev->dev,
+ "lpi-parent phandle missing for %s\n",
+ mc_of_node->full_name);
+ return -ENOENT;
+ }
+
+ /*
+ * Extract MSI parent node:
+ */
+ its_domain = irq_find_host(its_of_node);
+ if (!its_domain) {
+ dev_err(&mc_pdev->dev, "Unable to find parent domain\n");
+ error = -ENOENT;
+ goto cleanup_its_of_node;
+ }
+
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
+ irq_domain = msi_create_irq_domain(mc_of_node, &mc_bus_msi_domain_info,
+ its_domain->parent);
+ if (!irq_domain) {
+ dev_err(&mc_pdev->dev, "Failed to allocate msi_domain\n");
+ error = -ENOMEM;
+ goto cleanup_its_of_node;
+ }
+
+ dev_dbg(&mc_pdev->dev, "Allocated MSI domain\n");
+#else
+ irq_domain = NULL;
+#endif
+ *new_irq_domain = irq_domain;
+ return 0;
+
+cleanup_its_of_node:
+ of_node_put(its_of_node);
+ return error;
+}
+#endif /* GIC_ITS_MC_SUPPORT */
+
+/*
+ * Initialize the interrupt pool associated with a MC bus.
+ * It allocates a block of IRQs from the GIC-ITS
+ */
+int __must_check fsl_mc_populate_irq_pool(struct fsl_mc_bus *mc_bus,
+ unsigned int irq_count)
+{
+ unsigned int i;
+ struct fsl_mc_device_irq *irq_resources;
+ struct fsl_mc_device_irq *irq_res;
+ struct fsl_mc_device *mc_bus_dev = &mc_bus->mc_dev;
+ struct fsl_mc_resource_pool *res_pool =
+ &mc_bus->resource_pools[FSL_MC_POOL_IRQ];
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
+ int error;
+ struct msi_desc *msi_entry;
+ struct msi_desc *next_msi_entry;
+ struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);
+
+ /*
+ * Detect duplicate invocations of this function:
+ */
+ if (WARN_ON(!list_empty(&mc_bus_dev->dev.msi_list)))
+ return -EINVAL;
+#endif
+
+ if (WARN_ON(irq_count == 0 ||
+ irq_count > FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS))
+ return -EINVAL;
+
+ irq_resources =
+ devm_kzalloc(&mc_bus_dev->dev,
+ sizeof(*irq_resources) * irq_count,
+ GFP_KERNEL);
+ if (!irq_resources)
+ return -ENOMEM;
+
+ for (i = 0; i < irq_count; i++) {
+ irq_res = &irq_resources[i];
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
+ msi_entry = alloc_msi_entry(&mc_bus_dev->dev);
+ if (!msi_entry) {
+ dev_err(&mc_bus_dev->dev, "Failed to allocate msi entry\n");
+ error = -ENOMEM;
+ goto cleanup_msi_entries;
+ }
+
+ msi_entry->msi_attrib.is_msix = 1;
+ msi_entry->msi_attrib.is_64 = 1;
+ msi_entry->msi_attrib.entry_nr = i;
+ msi_entry->nvec_used = 1;
+ list_add_tail(&msi_entry->list, &mc_bus_dev->dev.msi_list);
+#endif
+
+ /*
+ * NOTE: irq_res->msi_paddr will be set by the
+ * mc_bus_msi_domain_write_msg() callback
+ */
+ irq_res->resource.type = res_pool->type;
+ irq_res->resource.data = irq_res;
+ irq_res->resource.parent_pool = res_pool;
+ INIT_LIST_HEAD(&irq_res->resource.node);
+ list_add_tail(&irq_res->resource.node, &res_pool->free_list);
+ }
+
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
+ /*
+ * NOTE: Calling this function will trigger the invocation of the
+ * mc_bus_msi_prepare() callback
+ */
+ error = msi_domain_alloc_irqs(mc->irq_domain,
+ &mc_bus_dev->dev, irq_count);
+
+ if (error) {
+ dev_err(&mc_bus_dev->dev, "Failed to allocate IRQs\n");
+ goto cleanup_msi_entries;
+ }
+#endif
+
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
+ for_each_msi_entry(msi_entry, &mc_bus_dev->dev) {
+ u32 irq_num = msi_entry->irq;
+
+ irq_res = &irq_resources[msi_entry->msi_attrib.entry_nr];
+ irq_res->irq_number = irq_num;
+ irq_res->resource.id = irq_num;
+ }
+#endif
+
+ res_pool->max_count = irq_count;
+ res_pool->free_count = irq_count;
+ mc_bus->irq_resources = irq_resources;
+ return 0;
+
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
+cleanup_msi_entries:
+ list_for_each_entry_safe(msi_entry, next_msi_entry,
+ &mc_bus_dev->dev.msi_list, list)
+ kfree(msi_entry);
+
+ devm_kfree(&mc_bus_dev->dev, irq_resources);
+ return error;
+#endif
+}
+EXPORT_SYMBOL_GPL(fsl_mc_populate_irq_pool);
+
+/**
+ * Teardown the interrupt pool associated with an MC bus.
+ * It frees the IRQs that were allocated to the pool, back to the GIC-ITS.
+ */
+void fsl_mc_cleanup_irq_pool(struct fsl_mc_bus *mc_bus)
+{
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
+ struct msi_desc *msi_entry;
+ struct msi_desc *next_msi_entry;
+ struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);
+#endif
+ struct fsl_mc_resource_pool *res_pool =
+ &mc_bus->resource_pools[FSL_MC_POOL_IRQ];
+
+ if (WARN_ON(res_pool->max_count == 0))
+ return;
+
+ if (WARN_ON(res_pool->free_count != res_pool->max_count))
+ return;
+
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
+ msi_domain_free_irqs(mc->irq_domain, &mc_bus->mc_dev.dev);
+ list_for_each_entry_safe(msi_entry, next_msi_entry,
+ &mc_bus->mc_dev.dev.msi_list, list)
+ kfree(msi_entry);
+#endif
+
+ devm_kfree(&mc_bus->mc_dev.dev, mc_bus->irq_resources);
+ res_pool->max_count = 0;
+ res_pool->free_count = 0;
+ mc_bus->irq_resources = NULL;
+}
+EXPORT_SYMBOL_GPL(fsl_mc_cleanup_irq_pool);
+
static int parse_mc_ranges(struct device *dev,
int *paddr_cells,
int *mc_addr_cells,
@@ -613,6 +943,15 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, mc);

/*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
+ error = create_mc_irq_domain(pdev, &mc->irq_domain);
+ if (error < 0)
+ return error;
+#endif
+
+ /*
* Get physical address of MC portal for the root DPRC:
*/
error = of_address_to_resource(pdev->dev.of_node, 0, &res);
@@ -620,7 +959,7 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
dev_err(&pdev->dev,
"of_address_to_resource() failed for %s\n",
pdev->dev.of_node->full_name);
- return error;
+ goto error_cleanup_irq_domain;
}

mc_portal_phys_addr = res.start;
@@ -628,7 +967,7 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
error = fsl_create_mc_io(&pdev->dev, mc_portal_phys_addr,
mc_portal_size, NULL, 0, &mc_io);
if (error < 0)
- return error;
+ goto error_cleanup_irq_domain;

error = mc_get_version(mc_io, &mc_version);
if (error != 0) {
@@ -673,6 +1012,7 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
obj_desc.id = container_id;
obj_desc.ver_major = DPRC_VER_MAJOR;
obj_desc.ver_minor = DPRC_VER_MINOR;
+ obj_desc.irq_count = 1;
obj_desc.region_count = 0;

error = fsl_mc_device_add(&obj_desc, mc_io, &pdev->dev, &mc_bus_dev);
@@ -684,6 +1024,9 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)

error_cleanup_mc_io:
fsl_destroy_mc_io(mc_io);
+
+error_cleanup_irq_domain:
+ irq_domain_remove(mc->irq_domain);
return error;
}

@@ -698,6 +1041,7 @@ static int fsl_mc_bus_remove(struct platform_device *pdev)
if (WARN_ON(&mc->root_mc_bus_dev->dev != fsl_mc_bus_type.dev_root))
return -EINVAL;

+ irq_domain_remove(mc->irq_domain);
fsl_mc_device_remove(mc->root_mc_bus_dev);
dev_info(&pdev->dev, "Root MC bus device removed");
return 0;
diff --git a/drivers/staging/fsl-mc/include/mc-private.h b/drivers/staging/fsl-mc/include/mc-private.h
index c045f49..6e33942 100644
--- a/drivers/staging/fsl-mc/include/mc-private.h
+++ b/drivers/staging/fsl-mc/include/mc-private.h
@@ -27,12 +27,26 @@
strcmp(_obj_type, "dpcon") == 0)

/**
+ * Maximum number of total IRQs that can be pre-allocated for an MC bus'
+ * IRQ pool
+ */
+#define FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS 256
+
+/**
+ * Maximum number of extra IRQs pre-reallocated for an MC bus' IRQ pool,
+ * to be used by dynamically created MC objects
+ */
+#define FSL_MC_IRQ_POOL_MAX_EXTRA_IRQS 64
+
+/**
* struct fsl_mc - Private data of a "fsl,qoriq-mc" platform device
* @root_mc_bus_dev: MC object device representing the root DPRC
+ * @irq_domain: IRQ domain for the fsl-mc bus type
* @addr_translation_ranges: array of bus to system address translation ranges
*/
struct fsl_mc {
struct fsl_mc_device *root_mc_bus_dev;
+ struct irq_domain *irq_domain;
uint8_t num_translation_ranges;
struct fsl_mc_addr_translation_range *translation_ranges;
};
@@ -76,11 +90,13 @@ struct fsl_mc_resource_pool {
* @resource_pools: array of resource pools (one pool per resource type)
* for this MC bus. These resources represent allocatable entities
* from the physical DPRC.
+ * @irq_resources: Pointer to array of IRQ objects for the IRQ pool.
* @scan_mutex: Serializes bus scanning
*/
struct fsl_mc_bus {
struct fsl_mc_device mc_dev;
struct fsl_mc_resource_pool resource_pools[FSL_MC_NUM_POOL_TYPES];
+ struct fsl_mc_device_irq *irq_resources;
struct mutex scan_mutex; /* serializes bus scanning */
};

@@ -94,9 +110,8 @@ int __must_check fsl_mc_device_add(struct dprc_obj_desc *obj_desc,

void fsl_mc_device_remove(struct fsl_mc_device *mc_dev);

-int dprc_scan_container(struct fsl_mc_device *mc_bus_dev);
-
-int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev);
+int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev,
+ unsigned int *total_irq_count);

int __init dprc_driver_init(void);

@@ -113,4 +128,9 @@ int __must_check fsl_mc_resource_allocate(struct fsl_mc_bus *mc_bus,

void fsl_mc_resource_free(struct fsl_mc_resource *resource);

+int __must_check fsl_mc_populate_irq_pool(struct fsl_mc_bus *mc_bus,
+ unsigned int irq_count);
+
+void fsl_mc_cleanup_irq_pool(struct fsl_mc_bus *mc_bus);
+
#endif /* _FSL_MC_PRIVATE_H_ */
diff --git a/drivers/staging/fsl-mc/include/mc.h b/drivers/staging/fsl-mc/include/mc.h
index fa02ef0..531cb8c 100644
--- a/drivers/staging/fsl-mc/include/mc.h
+++ b/drivers/staging/fsl-mc/include/mc.h
@@ -14,6 +14,7 @@
#include <linux/device.h>
#include <linux/mod_devicetable.h>
#include <linux/list.h>
+#include <linux/interrupt.h>
#include "../include/dprc.h"

#define FSL_MC_VENDOR_FREESCALE 0x1957
@@ -75,6 +76,7 @@ enum fsl_mc_pool_type {
FSL_MC_POOL_DPMCP = 0x0, /* corresponds to "dpmcp" in the MC */
FSL_MC_POOL_DPBP, /* corresponds to "dpbp" in the MC */
FSL_MC_POOL_DPCON, /* corresponds to "dpcon" in the MC */
+ FSL_MC_POOL_IRQ,

/*
* NOTE: New resource pool types must be added before this entry
@@ -104,6 +106,23 @@ struct fsl_mc_resource {
};

/**
+ * struct fsl_mc_device_irq - MC object device message-based interrupt
+ * @msi_paddr: message-based interrupt physical address
+ * @msi_value: message-based interrupt data value
+ * @irq_number: Linux IRQ number assigned to the interrupt
+ * @resource: MC generic resource associated with the interrupt
+ */
+struct fsl_mc_device_irq {
+ phys_addr_t msi_paddr;
+ uint32_t msi_value;
+ uint32_t irq_number;
+ struct fsl_mc_resource resource;
+};
+
+#define to_fsl_mc_irq(_mc_resource) \
+ container_of(_mc_resource, struct fsl_mc_device_irq, resource)
+
+/**
* Bit masks for a MC object device (struct fsl_mc_device) flags
*/
#define FSL_MC_IS_DPRC 0x0001
@@ -124,6 +143,7 @@ struct fsl_mc_resource {
* NULL if none.
* @obj_desc: MC description of the DPAA device
* @regions: pointer to array of MMIO region entries
+ * @irqs: pointer to array of pointers to interrupts allocated to this device
* @resource: generic resource associated with this MC object device, if any.
*
* Generic device object for MC object devices that are "attached" to a
@@ -155,6 +175,7 @@ struct fsl_mc_device {
struct fsl_mc_io *mc_io;
struct dprc_obj_desc obj_desc;
struct resource *regions;
+ struct fsl_mc_device_irq **irqs;
struct fsl_mc_resource *resource;
};

@@ -196,6 +217,10 @@ int __must_check fsl_mc_object_allocate(struct fsl_mc_device *mc_dev,

void fsl_mc_object_free(struct fsl_mc_device *mc_adev);

+int __must_check fsl_mc_allocate_irqs(struct fsl_mc_device *mc_dev);
+
+void fsl_mc_free_irqs(struct fsl_mc_device *mc_dev);
+
extern struct bus_type fsl_mc_bus_type;

#endif /* _FSL_MC_H_ */
--
2.3.3

2015-04-28 17:47:26

by J. German Rivera

[permalink] [raw]
Subject: [PATCH 2/7] staging: fsl_-mc: add device binding path 'driver_override'

From: Bharat Bhushan <[email protected]>

This patch is required for vfio-fsl-mc meta driver to successfully bind
layerscape container devices for device passthrough. This patch adds
a mechanism to allow a layerscape device to specify a driver rather than
a layerscape driver provide a device match.

This patch is based on following proposed patches for PCI and platform devices
- https://lkml.org/lkml/2014/4/8/571 :- For Platform devices
- http://lists-archives.com/linux-kernel/28030441-pci-introduce-new-device-binding-path-using-pci_dev-driver_override.html :- For PCI devices

Example to allow a device (dprc.1) to specifically bind
with driver (vfio-fsl-mc):-
- echo vfio-fsl-mc > /sys/bus/fsl-mc/devices/dprc.1/driver_override
- echo dprc.1 > /sys/bus/fsl-mc/drivers/fsl_mc_dprc/unbind
- echo dprc.1 > /sys/bus/fsl-mc/drivers/vfio-fsl-mc/bind

Signed-off-by: J. German Rivera <[email protected]>
Change-Id: I11303bfe0196540e931dd2108a8b957dd45ae083
Reviewed-on: http://git.am.freescale.net:8181/34235
Reviewed-by: Stuart Yoder <[email protected]>
Tested-by: Stuart Yoder <[email protected]>
---
drivers/staging/fsl-mc/bus/mc-bus.c | 67 +++++++++++++++++++++++++++++++++++++
drivers/staging/fsl-mc/include/mc.h | 2 ++
2 files changed, 69 insertions(+)

diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c
index 7332b26..a419c2f 100644
--- a/drivers/staging/fsl-mc/bus/mc-bus.c
+++ b/drivers/staging/fsl-mc/bus/mc-bus.c
@@ -58,6 +58,12 @@ static int fsl_mc_bus_match(struct device *dev, struct device_driver *drv)
if (WARN_ON(!fsl_mc_bus_type.dev_root))
goto out;

+ /* When driver_override is set, only bind to the matching driver */
+ if (mc_dev->driver_override) {
+ found = !strcmp(mc_dev->driver_override, mc_drv->driver.name);
+ goto out;
+ }
+
if (!mc_drv->match_id_table)
goto out;

@@ -116,10 +122,69 @@ static int fsl_mc_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
return 0;
}

+static ssize_t driver_override_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+ const char *driver_override, *old = mc_dev->driver_override;
+ char *cp;
+
+ if (WARN_ON(dev->bus != &fsl_mc_bus_type))
+ return -EINVAL;
+
+ if (count > PATH_MAX)
+ return -EINVAL;
+
+ driver_override = kstrndup(buf, count, GFP_KERNEL);
+ if (!driver_override)
+ return -ENOMEM;
+
+ cp = strchr(driver_override, '\n');
+ if (cp)
+ *cp = '\0';
+
+ if (strlen(driver_override)) {
+ mc_dev->driver_override = driver_override;
+ } else {
+ kfree(driver_override);
+ mc_dev->driver_override = NULL;
+ }
+
+ kfree(old);
+
+ return count;
+}
+
+static ssize_t driver_override_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+
+ return sprintf(buf, "%s\n", mc_dev->driver_override);
+}
+
+static DEVICE_ATTR_RW(driver_override);
+
+static struct attribute *fsl_mc_dev_attrs[] = {
+ &dev_attr_driver_override.attr,
+ NULL,
+};
+
+static const struct attribute_group fsl_mc_dev_group = {
+ .attrs = fsl_mc_dev_attrs,
+};
+
+static const struct attribute_group *fsl_mc_dev_groups[] = {
+ &fsl_mc_dev_group,
+ NULL,
+};
+
struct bus_type fsl_mc_bus_type = {
.name = "fsl-mc",
.match = fsl_mc_bus_match,
.uevent = fsl_mc_bus_uevent,
+ .dev_groups = fsl_mc_dev_groups,
};
EXPORT_SYMBOL_GPL(fsl_mc_bus_type);

@@ -499,6 +564,8 @@ void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
fsl_mc_bus_type.dev_root = NULL;
}

+ kfree(mc_dev->driver_override);
+ mc_dev->driver_override = NULL;
if (mc_bus)
devm_kfree(mc_dev->dev.parent, mc_bus);
else
diff --git a/drivers/staging/fsl-mc/include/mc.h b/drivers/staging/fsl-mc/include/mc.h
index 531cb8c..f965c4d 100644
--- a/drivers/staging/fsl-mc/include/mc.h
+++ b/drivers/staging/fsl-mc/include/mc.h
@@ -145,6 +145,7 @@ struct fsl_mc_device_irq {
* @regions: pointer to array of MMIO region entries
* @irqs: pointer to array of pointers to interrupts allocated to this device
* @resource: generic resource associated with this MC object device, if any.
+ * @driver_override: Driver name to force a match
*
* Generic device object for MC object devices that are "attached" to a
* MC bus.
@@ -177,6 +178,7 @@ struct fsl_mc_device {
struct resource *regions;
struct fsl_mc_device_irq **irqs;
struct fsl_mc_resource *resource;
+ const char *driver_override;
};

#define to_fsl_mc_device(_dev) \
--
2.3.3

2015-04-28 17:47:31

by J. German Rivera

[permalink] [raw]
Subject: [PATCH 3/7] staging: fsl-mc: Propagate driver_override for a child DPRC's children

When a child DPRC is bound to the vfio_fsl_mc driver via driver_override,
its own children should not be bound to corresponding host kernel
drivers, but instead should be bound to the vfio_fsl_mc driver as
well.

Currently, when a child container is scanned by the vfio_fsl_mc driver,
child devices found are automatically bound to corresponding host kernel
drivers (e.g., DPMCP and DPBP objects are bound to the fsl_mc_allocator
driver, DPNI objects are bound to the ldpaa_eth driver, etc), Then,
the user has to manually unbind these child devices from their drivers,
set the driver_override sysfs attribute to vfio_fsl_mc driver, for each
of them and rebind them.

Signed-off-by: J. German Rivera <[email protected]>
Change-Id: I51e633367b019da1cd45bd4898e46be577b8ce42
Reviewed-on: http://git.am.freescale.net:8181/34567
Tested-by: Review Code-CDREVIEW <[email protected]>
Reviewed-by: Stuart Yoder <[email protected]>
Tested-by: Stuart Yoder <[email protected]>
---
drivers/staging/fsl-mc/bus/dprc-driver.c | 14 ++++++++++----
drivers/staging/fsl-mc/bus/mc-bus.c | 16 +++++++++++++++-
drivers/staging/fsl-mc/include/mc-private.h | 2 ++
3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c
index 8531ba8..ce88859 100644
--- a/drivers/staging/fsl-mc/bus/dprc-driver.c
+++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
@@ -151,6 +151,8 @@ static void check_plugged_state_change(struct fsl_mc_device *mc_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
+ * @driver_override: driver override to apply to new objects found in the DPRC,
+ * or NULL, if none.
* @obj_desc_array: array of device descriptors for child devices currently
* present in the physical DPRC.
* @num_child_objects_in_mc: number of entries in obj_desc_array
@@ -160,6 +162,7 @@ static void check_plugged_state_change(struct fsl_mc_device *mc_dev,
* in the physical DPRC.
*/
static void dprc_add_new_devices(struct fsl_mc_device *mc_bus_dev,
+ const char *driver_override,
struct dprc_obj_desc *obj_desc_array,
int num_child_objects_in_mc)
{
@@ -183,7 +186,7 @@ static void dprc_add_new_devices(struct fsl_mc_device *mc_bus_dev,
}

error = fsl_mc_device_add(obj_desc, NULL, &mc_bus_dev->dev,
- &child_dev);
+ driver_override, &child_dev);
if (error < 0)
continue;
}
@@ -242,6 +245,8 @@ static void dprc_cleanup_all_resource_pools(struct fsl_mc_device *mc_bus_dev)
* dprc_scan_objects - Discover objects in a DPRC
*
* @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
+ * @driver_override: driver override to apply to new objects found in the DPRC,
+ * or NULL, if none.
* @total_irq_count: total number of IRQs needed by objects in the DPRC.
*
* Detects objects added and removed from a DPRC and synchronizes the
@@ -257,6 +262,7 @@ static void dprc_cleanup_all_resource_pools(struct fsl_mc_device *mc_bus_dev)
* of the device drivers for the non-allocatable devices.
*/
int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev,
+ const char *driver_override,
unsigned int *total_irq_count)
{
int num_child_objects;
@@ -326,7 +332,7 @@ int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev,
dprc_remove_devices(mc_bus_dev, child_obj_desc_array,
num_child_objects);

- dprc_add_new_devices(mc_bus_dev, child_obj_desc_array,
+ dprc_add_new_devices(mc_bus_dev, driver_override, child_obj_desc_array,
num_child_objects);

if (child_obj_desc_array)
@@ -357,7 +363,7 @@ static int dprc_scan_container(struct fsl_mc_device *mc_bus_dev)
* Discover objects in the DPRC:
*/
mutex_lock(&mc_bus->scan_mutex);
- error = dprc_scan_objects(mc_bus_dev, &irq_count);
+ error = dprc_scan_objects(mc_bus_dev, NULL, &irq_count);
mutex_unlock(&mc_bus->scan_mutex);
if (error < 0)
goto error;
@@ -433,7 +439,7 @@ static irqreturn_t dprc_irq0_handler_thread(int irq_num, void *arg)
DPRC_IRQ_EVENT_OBJ_CREATED)) {
unsigned int irq_count;

- error = dprc_scan_objects(mc_dev, &irq_count);
+ error = dprc_scan_objects(mc_dev, NULL, &irq_count);
if (error < 0) {
dev_err(dev, "dprc_scan_objects() failed: %d\n", error);
goto out;
diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c
index a419c2f..08fd559 100644
--- a/drivers/staging/fsl-mc/bus/mc-bus.c
+++ b/drivers/staging/fsl-mc/bus/mc-bus.c
@@ -401,6 +401,7 @@ error_cleanup_regions:
int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
struct fsl_mc_io *mc_io,
struct device *parent_dev,
+ const char *driver_override,
struct fsl_mc_device **new_mc_dev)
{
int error;
@@ -433,6 +434,18 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,

mc_dev->obj_desc = *obj_desc;
mc_dev->mc_io = mc_io;
+ if (driver_override) {
+ /*
+ * We trust driver_override, so we don't need to use
+ * kstrndup() here
+ */
+ mc_dev->driver_override = kstrdup(driver_override, GFP_KERNEL);
+ if (!mc_dev->driver_override) {
+ error = -ENOMEM;
+ goto error_cleanup_dev;
+ }
+ }
+
device_initialize(&mc_dev->dev);

/*
@@ -1082,7 +1095,8 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
obj_desc.irq_count = 1;
obj_desc.region_count = 0;

- error = fsl_mc_device_add(&obj_desc, mc_io, &pdev->dev, &mc_bus_dev);
+ error = fsl_mc_device_add(&obj_desc, mc_io, &pdev->dev, NULL,
+ &mc_bus_dev);
if (error < 0)
goto error_cleanup_mc_io;

diff --git a/drivers/staging/fsl-mc/include/mc-private.h b/drivers/staging/fsl-mc/include/mc-private.h
index 6e33942..5b9c8f2 100644
--- a/drivers/staging/fsl-mc/include/mc-private.h
+++ b/drivers/staging/fsl-mc/include/mc-private.h
@@ -106,11 +106,13 @@ struct fsl_mc_bus {
int __must_check fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
struct fsl_mc_io *mc_io,
struct device *parent_dev,
+ const char *driver_override,
struct fsl_mc_device **new_mc_dev);

void fsl_mc_device_remove(struct fsl_mc_device *mc_dev);

int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev,
+ const char *driver_override,
unsigned int *total_irq_count);

int __init dprc_driver_init(void);
--
2.3.3

2015-04-28 17:47:20

by J. German Rivera

[permalink] [raw]
Subject: [PATCH 4/7] staging: fsl-mc: Upgraded MC bus driver to match MC fw 7.0.0

- Migrated MC bus driver to use DPRC flib 0.6.
- Changed IRQ setup infrastructure to be able to program MSIs
for MC objects in an object-independent way.

Signed-off-by: J. German Rivera <[email protected]>
---
drivers/staging/fsl-mc/bus/dpmcp-cmd.h | 79 ------------
drivers/staging/fsl-mc/bus/dprc-cmd.h | 6 +-
drivers/staging/fsl-mc/bus/dprc-driver.c | 106 +++++++++++++---
drivers/staging/fsl-mc/bus/dprc.c | 179 +++++++++++++++++++++++-----
drivers/staging/fsl-mc/bus/mc-allocator.c | 26 ++--
drivers/staging/fsl-mc/bus/mc-bus.c | 125 ++++++++-----------
drivers/staging/fsl-mc/bus/mc-sys.c | 6 +-
drivers/staging/fsl-mc/include/dpmng.h | 4 +-
drivers/staging/fsl-mc/include/dprc.h | 114 +++++++++++++-----
drivers/staging/fsl-mc/include/mc-private.h | 29 ++++-
drivers/staging/fsl-mc/include/mc.h | 4 +
11 files changed, 431 insertions(+), 247 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/dpmcp-cmd.h b/drivers/staging/fsl-mc/bus/dpmcp-cmd.h
index 57f326b..62bdc18 100644
--- a/drivers/staging/fsl-mc/bus/dpmcp-cmd.h
+++ b/drivers/staging/fsl-mc/bus/dpmcp-cmd.h
@@ -54,83 +54,4 @@
#define DPMCP_CMDID_GET_IRQ_STATUS 0x016
#define DPMCP_CMDID_CLEAR_IRQ_STATUS 0x017

-/* cmd, param, offset, width, type, arg_name */
-#define DPMCP_CMD_CREATE(cmd, cfg) \
- MC_CMD_OP(cmd, 0, 0, 32, int, cfg->portal_id)
-
-/* cmd, param, offset, width, type, arg_name */
-#define DPMCP_CMD_SET_IRQ(cmd, irq_index, irq_addr, irq_val, user_irq_id) \
-do { \
- MC_CMD_OP(cmd, 0, 0, 8, uint8_t, irq_index);\
- MC_CMD_OP(cmd, 0, 32, 32, uint32_t, irq_val);\
- MC_CMD_OP(cmd, 1, 0, 64, uint64_t, irq_addr); \
- MC_CMD_OP(cmd, 2, 0, 32, int, user_irq_id); \
-} while (0)
-
-/* cmd, param, offset, width, type, arg_name */
-#define DPMCP_CMD_GET_IRQ(cmd, irq_index) \
- MC_CMD_OP(cmd, 0, 32, 8, uint8_t, irq_index)
-
-/* cmd, param, offset, width, type, arg_name */
-#define DPMCP_RSP_GET_IRQ(cmd, type, irq_addr, irq_val, user_irq_id) \
-do { \
- MC_RSP_OP(cmd, 0, 0, 32, uint32_t, irq_val); \
- MC_RSP_OP(cmd, 1, 0, 64, uint64_t, irq_addr); \
- MC_RSP_OP(cmd, 2, 0, 32, int, user_irq_id); \
- MC_RSP_OP(cmd, 2, 32, 32, int, type); \
-} while (0)
-
-/* cmd, param, offset, width, type, arg_name */
-#define DPMCP_CMD_SET_IRQ_ENABLE(cmd, irq_index, en) \
-do { \
- MC_CMD_OP(cmd, 0, 0, 8, uint8_t, en); \
- MC_CMD_OP(cmd, 0, 32, 8, uint8_t, irq_index);\
-} while (0)
-
-/* cmd, param, offset, width, type, arg_name */
-#define DPMCP_CMD_GET_IRQ_ENABLE(cmd, irq_index) \
- MC_CMD_OP(cmd, 0, 32, 8, uint8_t, irq_index)
-
-/* cmd, param, offset, width, type, arg_name */
-#define DPMCP_RSP_GET_IRQ_ENABLE(cmd, en) \
- MC_RSP_OP(cmd, 0, 0, 8, uint8_t, en)
-
-/* cmd, param, offset, width, type, arg_name */
-#define DPMCP_CMD_SET_IRQ_MASK(cmd, irq_index, mask) \
-do { \
- MC_CMD_OP(cmd, 0, 0, 32, uint32_t, mask);\
- MC_CMD_OP(cmd, 0, 32, 8, uint8_t, irq_index);\
-} while (0)
-
-/* cmd, param, offset, width, type, arg_name */
-#define DPMCP_CMD_GET_IRQ_MASK(cmd, irq_index) \
- MC_CMD_OP(cmd, 0, 32, 8, uint8_t, irq_index)
-
-/* cmd, param, offset, width, type, arg_name */
-#define DPMCP_RSP_GET_IRQ_MASK(cmd, mask) \
- MC_RSP_OP(cmd, 0, 0, 32, uint32_t, mask)
-
-/* cmd, param, offset, width, type, arg_name */
-#define DPMCP_CMD_GET_IRQ_STATUS(cmd, irq_index) \
- MC_CMD_OP(cmd, 0, 32, 8, uint8_t, irq_index)
-
-/* cmd, param, offset, width, type, arg_name */
-#define DPMCP_RSP_GET_IRQ_STATUS(cmd, status) \
- MC_RSP_OP(cmd, 0, 0, 32, uint32_t, status)
-
-/* cmd, param, offset, width, type, arg_name */
-#define DPMCP_CMD_CLEAR_IRQ_STATUS(cmd, irq_index, status) \
-do { \
- MC_CMD_OP(cmd, 0, 0, 32, uint32_t, status); \
- MC_CMD_OP(cmd, 0, 32, 8, uint8_t, irq_index);\
-} while (0)
-
-/* cmd, param, offset, width, type, arg_name */
-#define DPMCP_RSP_GET_ATTRIBUTES(cmd, attr) \
-do { \
- MC_RSP_OP(cmd, 0, 32, 32, int, attr->id);\
- MC_RSP_OP(cmd, 1, 0, 16, uint16_t, attr->version.major);\
- MC_RSP_OP(cmd, 1, 16, 16, uint16_t, attr->version.minor);\
-} while (0)
-
#endif /* _FSL_DPMCP_CMD_H */
diff --git a/drivers/staging/fsl-mc/bus/dprc-cmd.h b/drivers/staging/fsl-mc/bus/dprc-cmd.h
index 0920248..df5ad5f 100644
--- a/drivers/staging/fsl-mc/bus/dprc-cmd.h
+++ b/drivers/staging/fsl-mc/bus/dprc-cmd.h
@@ -41,7 +41,7 @@
#define _FSL_DPRC_CMD_H

/* DPRC Version */
-#define DPRC_VER_MAJOR 3
+#define DPRC_VER_MAJOR 4
#define DPRC_VER_MINOR 0

/* Command IDs */
@@ -72,12 +72,14 @@
#define DPRC_CMDID_GET_RES_COUNT 0x15B
#define DPRC_CMDID_GET_RES_IDS 0x15C
#define DPRC_CMDID_GET_OBJ_REG 0x15E
+#define DPRC_CMDID_OBJ_SET_IRQ 0x15F
+#define DPRC_CMDID_OBJ_GET_IRQ 0x160
+#define DPRC_CMDID_SET_OBJ_LABEL 0x161

#define DPRC_CMDID_CONNECT 0x167
#define DPRC_CMDID_DISCONNECT 0x168
#define DPRC_CMDID_GET_POOL 0x169
#define DPRC_CMDID_GET_POOL_COUNT 0x16A
-#define DPRC_CMDID_GET_PORTAL_PADDR 0x16B

#define DPRC_CMDID_GET_CONNECTION 0x16C

diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c
index ce88859..b84cca3 100644
--- a/drivers/staging/fsl-mc/bus/dprc-driver.c
+++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
@@ -233,12 +233,17 @@ static void dprc_cleanup_resource_pool(struct fsl_mc_device *mc_bus_dev,
WARN_ON(free_count != res_pool->free_count);
}

+/*
+ * Clean up all resource pools other than the IRQ pool
+ */
static void dprc_cleanup_all_resource_pools(struct fsl_mc_device *mc_bus_dev)
{
int pool_type;

- for (pool_type = 0; pool_type < FSL_MC_NUM_POOL_TYPES; pool_type++)
- dprc_cleanup_resource_pool(mc_bus_dev, pool_type);
+ for (pool_type = 0; pool_type < FSL_MC_NUM_POOL_TYPES; pool_type++) {
+ if (pool_type != FSL_MC_POOL_IRQ)
+ dprc_cleanup_resource_pool(mc_bus_dev, pool_type);
+ }
}

/**
@@ -343,6 +348,57 @@ int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev,
EXPORT_SYMBOL_GPL(dprc_scan_objects);

/**
+ * dprc_lookup_object - Finds a given MC object in a DPRC and returns
+ * the index of the object in the DPRC
+ *
+ * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
+ * @child_dev: pointer to the fsl-mc device to be looked up
+ * @child_obj_index: output parameter to hold the index of the object
+ */
+int dprc_lookup_object(struct fsl_mc_device *mc_bus_dev,
+ struct fsl_mc_device *child_dev,
+ uint32_t *child_obj_index)
+{
+ int i;
+ int num_child_objects;
+ int error;
+
+ error = dprc_get_obj_count(mc_bus_dev->mc_io,
+ mc_bus_dev->mc_handle,
+ &num_child_objects);
+ if (error < 0) {
+ dev_err(&mc_bus_dev->dev, "dprc_get_obj_count() failed: %d\n",
+ error);
+ return error;
+ }
+
+ for (i = 0; i < num_child_objects; i++) {
+ struct dprc_obj_desc obj_desc;
+
+ error = dprc_get_obj(mc_bus_dev->mc_io,
+ mc_bus_dev->mc_handle,
+ i, &obj_desc);
+ if (error < 0) {
+ dev_err(&mc_bus_dev->dev,
+ "dprc_get_obj(i=%d) failed: %d\n",
+ i, error);
+ return error;
+ }
+
+ if (strcmp(obj_desc.type, child_dev->obj_desc.type) == 0 &&
+ obj_desc.id == child_dev->obj_desc.id) {
+ *child_obj_index = i;
+ return 0;
+ }
+ }
+
+ dev_err(&mc_bus_dev->dev, "%s.%u not found\n",
+ child_dev->obj_desc.type, child_dev->obj_desc.id);
+
+ return -ENODEV;
+}
+
+/**
* dprc_scan_container - Scans a physical DPRC and synchronizes Linux bus state
*
* @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
@@ -377,6 +433,7 @@ static int dprc_scan_container(struct fsl_mc_device *mc_bus_dev)

return 0;
error:
+ device_for_each_child(&mc_bus_dev->dev, NULL, __fsl_mc_device_remove);
dprc_cleanup_all_resource_pools(mc_bus_dev);
return error;
}
@@ -552,6 +609,22 @@ static int register_dprc_irq_handlers(struct fsl_mc_device *mc_dev)

for (i = 0; i < ARRAY_SIZE(irq_handlers); i++) {
irq = mc_dev->irqs[i];
+
+ if (WARN_ON(irq->dev_irq_index != i)) {
+ error = -EINVAL;
+ goto error_unregister_irq_handlers;
+ }
+
+ /*
+ * NOTE: Normally, devm_request_threaded_irq() programs the MSI
+ * physically in the device (by invoking a device-specific
+ * callback). However, for MC IRQs, we have to program the MSI
+ * outside of this callback, because this callback is invoked
+ * with interrupts disabled, and we don't have a reliable
+ * way of sending commands to the MC from atomic context.
+ * The MC callback just set the msi_paddr and msi_value
+ * fields of the irq structure.
+ */
error = devm_request_threaded_irq(&mc_dev->dev,
irq->irq_number,
irq_handlers[i].irq_handler,
@@ -568,24 +641,19 @@ static int register_dprc_irq_handlers(struct fsl_mc_device *mc_dev)
goto error_unregister_irq_handlers;
}

- /*
- * Program the MSI (paddr, value) pair in the device:
- *
- * TODO: This needs to be moved to mc_bus_msi_domain_write_msg()
- * when the MC object-independent dprc_set_irq() flib API
- * becomes available
- */
- error = dprc_set_irq(mc_dev->mc_io, mc_dev->mc_handle,
- i, irq->msi_paddr,
+ num_irq_handlers_registered++;
+ error = dprc_set_irq(mc_dev->mc_io,
+ mc_dev->mc_handle,
+ i,
+ irq->msi_paddr,
irq->msi_value,
irq->irq_number);
if (error < 0) {
dev_err(&mc_dev->dev,
- "mc_set_irq() failed: %d\n", error);
+ "dprc_set_irq() failed for IRQ %u: %d\n",
+ i, error);
goto error_unregister_irq_handlers;
}
-
- num_irq_handlers_registered++;
}

return 0;
@@ -729,15 +797,17 @@ static int dprc_probe(struct fsl_mc_device *mc_dev)
*/
error = dprc_setup_irqs(mc_dev);
if (error < 0)
- goto error_cleanup_open;
+ goto error_cleanup_dprc_scan;

dev_info(&mc_dev->dev, "DPRC device bound to driver");
return 0;

-error_cleanup_open:
- if (mc_bus->irq_resources)
- fsl_mc_cleanup_irq_pool(mc_bus);
+error_cleanup_dprc_scan:
+ device_for_each_child(&mc_dev->dev, NULL, __fsl_mc_device_remove);
+ dprc_cleanup_all_resource_pools(mc_dev);
+ fsl_mc_cleanup_irq_pool(mc_bus);

+error_cleanup_open:
(void)dprc_close(mc_dev->mc_io, mc_dev->mc_handle);

error_cleanup_mc_io:
diff --git a/drivers/staging/fsl-mc/bus/dprc.c b/drivers/staging/fsl-mc/bus/dprc.c
index 19b26e6..7a37d2d 100644
--- a/drivers/staging/fsl-mc/bus/dprc.c
+++ b/drivers/staging/fsl-mc/bus/dprc.c
@@ -73,7 +73,7 @@ int dprc_create_container(struct fsl_mc_io *mc_io,
uint16_t token,
struct dprc_cfg *cfg,
int *child_container_id,
- uint64_t *child_portal_paddr)
+ uint64_t *child_portal_offset)
{
struct mc_command cmd = { 0 };
int err;
@@ -82,6 +82,22 @@ int dprc_create_container(struct fsl_mc_io *mc_io,
cmd.params[0] |= mc_enc(32, 16, cfg->icid);
cmd.params[0] |= mc_enc(0, 32, cfg->options);
cmd.params[1] |= mc_enc(32, 32, cfg->portal_id);
+ cmd.params[2] |= mc_enc(0, 8, cfg->label[0]);
+ cmd.params[2] |= mc_enc(8, 8, cfg->label[1]);
+ cmd.params[2] |= mc_enc(16, 8, cfg->label[2]);
+ cmd.params[2] |= mc_enc(24, 8, cfg->label[3]);
+ cmd.params[2] |= mc_enc(32, 8, cfg->label[4]);
+ cmd.params[2] |= mc_enc(40, 8, cfg->label[5]);
+ cmd.params[2] |= mc_enc(48, 8, cfg->label[6]);
+ cmd.params[2] |= mc_enc(56, 8, cfg->label[7]);
+ cmd.params[3] |= mc_enc(0, 8, cfg->label[8]);
+ cmd.params[3] |= mc_enc(8, 8, cfg->label[9]);
+ cmd.params[3] |= mc_enc(16, 8, cfg->label[10]);
+ cmd.params[3] |= mc_enc(24, 8, cfg->label[11]);
+ cmd.params[3] |= mc_enc(32, 8, cfg->label[12]);
+ cmd.params[3] |= mc_enc(40, 8, cfg->label[13]);
+ cmd.params[3] |= mc_enc(48, 8, cfg->label[14]);
+ cmd.params[3] |= mc_enc(56, 8, cfg->label[15]);

cmd.header = mc_encode_cmd_header(DPRC_CMDID_CREATE_CONT,
MC_CMD_PRI_LOW, token);
@@ -93,10 +109,11 @@ int dprc_create_container(struct fsl_mc_io *mc_io,

/* retrieve response parameters */
*child_container_id = mc_dec(cmd.params[1], 0, 32);
- *child_portal_paddr = mc_dec(cmd.params[2], 0, 64);
+ *child_portal_offset = mc_dec(cmd.params[2], 0, 64);

return 0;
}
+EXPORT_SYMBOL(dprc_create_container);

int dprc_destroy_container(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -112,6 +129,7 @@ int dprc_destroy_container(struct fsl_mc_io *mc_io,
/* send command to mc*/
return mc_send_command(mc_io, &cmd);
}
+EXPORT_SYMBOL(dprc_destroy_container);

int dprc_reset_container(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -127,6 +145,7 @@ int dprc_reset_container(struct fsl_mc_io *mc_io,
/* send command to mc*/
return mc_send_command(mc_io, &cmd);
}
+EXPORT_SYMBOL(dprc_reset_container);

int dprc_get_irq(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -158,6 +177,41 @@ int dprc_get_irq(struct fsl_mc_io *mc_io,

return 0;
}
+EXPORT_SYMBOL(dprc_get_irq);
+
+int dprc_obj_get_irq(struct fsl_mc_io *mc_io,
+ uint16_t token,
+ int obj_index,
+ uint8_t irq_index,
+ int *type,
+ uint64_t *irq_addr,
+ uint32_t *irq_val,
+ int *user_irq_id)
+{
+ struct mc_command cmd = { 0 };
+ int err;
+
+ /* prepare command */
+ cmd.header = mc_encode_cmd_header(DPRC_CMDID_OBJ_GET_IRQ,
+ MC_CMD_PRI_LOW,
+ token);
+
+ cmd.params[0] |= mc_enc(0, 32, obj_index);
+ cmd.params[0] |= mc_enc(32, 8, irq_index);
+
+ /* send command to mc*/
+ err = mc_send_command(mc_io, &cmd);
+ if (err)
+ return err;
+
+ /* retrieve response parameters */
+ *irq_val = mc_dec(cmd.params[0], 0, 32);
+ *irq_addr = mc_dec(cmd.params[1], 0, 64);
+ *user_irq_id = mc_dec(cmd.params[2], 0, 32);
+ *type = mc_dec(cmd.params[2], 32, 32);
+ return 0;
+}
+EXPORT_SYMBOL(dprc_obj_get_irq);

int dprc_set_irq(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -180,6 +234,33 @@ int dprc_set_irq(struct fsl_mc_io *mc_io,
/* send command to mc*/
return mc_send_command(mc_io, &cmd);
}
+EXPORT_SYMBOL(dprc_set_irq);
+
+int dprc_obj_set_irq(struct fsl_mc_io *mc_io,
+ uint16_t token,
+ int obj_index,
+ uint8_t irq_index,
+ uint64_t irq_addr,
+ uint32_t irq_val,
+ int user_irq_id)
+{
+ struct mc_command cmd = { 0 };
+
+ /* prepare command */
+ cmd.header = mc_encode_cmd_header(DPRC_CMDID_OBJ_SET_IRQ,
+ MC_CMD_PRI_LOW,
+ token);
+
+ cmd.params[0] |= mc_enc(32, 8, irq_index);
+ cmd.params[0] |= mc_enc(0, 32, irq_val);
+ cmd.params[1] |= mc_enc(0, 64, irq_addr);
+ cmd.params[2] |= mc_enc(0, 32, user_irq_id);
+ cmd.params[2] |= mc_enc(32, 32, obj_index);
+
+ /* send command to mc*/
+ return mc_send_command(mc_io, &cmd);
+}
+EXPORT_SYMBOL(dprc_obj_set_irq);

int dprc_get_irq_enable(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -204,6 +285,7 @@ int dprc_get_irq_enable(struct fsl_mc_io *mc_io,

return 0;
}
+EXPORT_SYMBOL(dprc_get_irq_enable);

int dprc_set_irq_enable(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -221,6 +303,7 @@ int dprc_set_irq_enable(struct fsl_mc_io *mc_io,
/* send command to mc*/
return mc_send_command(mc_io, &cmd);
}
+EXPORT_SYMBOL(dprc_set_irq_enable);

int dprc_get_irq_mask(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -245,6 +328,7 @@ int dprc_get_irq_mask(struct fsl_mc_io *mc_io,

return 0;
}
+EXPORT_SYMBOL(dprc_get_irq_mask);

int dprc_set_irq_mask(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -262,6 +346,7 @@ int dprc_set_irq_mask(struct fsl_mc_io *mc_io,
/* send command to mc*/
return mc_send_command(mc_io, &cmd);
}
+EXPORT_SYMBOL(dprc_set_irq_mask);

int dprc_get_irq_status(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -286,6 +371,7 @@ int dprc_get_irq_status(struct fsl_mc_io *mc_io,

return 0;
}
+EXPORT_SYMBOL(dprc_get_irq_status);

int dprc_clear_irq_status(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -303,6 +389,7 @@ int dprc_clear_irq_status(struct fsl_mc_io *mc_io,
/* send command to mc*/
return mc_send_command(mc_io, &cmd);
}
+EXPORT_SYMBOL(dprc_clear_irq_status);

int dprc_get_attributes(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -331,6 +418,7 @@ int dprc_get_attributes(struct fsl_mc_io *mc_io,

return 0;
}
+EXPORT_SYMBOL(dprc_get_attributes);

int dprc_set_res_quota(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -365,6 +453,7 @@ int dprc_set_res_quota(struct fsl_mc_io *mc_io,
/* send command to mc*/
return mc_send_command(mc_io, &cmd);
}
+EXPORT_SYMBOL(dprc_set_res_quota);

int dprc_get_res_quota(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -406,6 +495,7 @@ int dprc_get_res_quota(struct fsl_mc_io *mc_io,

return 0;
}
+EXPORT_SYMBOL(dprc_get_res_quota);

int dprc_assign(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -441,6 +531,7 @@ int dprc_assign(struct fsl_mc_io *mc_io,
/* send command to mc*/
return mc_send_command(mc_io, &cmd);
}
+EXPORT_SYMBOL(dprc_assign);

int dprc_unassign(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -477,6 +568,7 @@ int dprc_unassign(struct fsl_mc_io *mc_io,
/* send command to mc*/
return mc_send_command(mc_io, &cmd);
}
+EXPORT_SYMBOL(dprc_unassign);

int dprc_get_pool_count(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -499,6 +591,7 @@ int dprc_get_pool_count(struct fsl_mc_io *mc_io,

return 0;
}
+EXPORT_SYMBOL(dprc_get_pool_count);

int dprc_get_pool(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -539,6 +632,7 @@ int dprc_get_pool(struct fsl_mc_io *mc_io,

return 0;
}
+EXPORT_SYMBOL(dprc_get_pool);

int dprc_get_obj_count(struct fsl_mc_io *mc_io, uint16_t token, int *obj_count)
{
@@ -604,7 +698,22 @@ int dprc_get_obj(struct fsl_mc_io *mc_io,
obj_desc->type[13] = mc_dec(cmd.params[4], 40, 8);
obj_desc->type[14] = mc_dec(cmd.params[4], 48, 8);
obj_desc->type[15] = '\0';
-
+ obj_desc->label[0] = mc_dec(cmd.params[5], 0, 8);
+ obj_desc->label[1] = mc_dec(cmd.params[5], 8, 8);
+ obj_desc->label[2] = mc_dec(cmd.params[5], 16, 8);
+ obj_desc->label[3] = mc_dec(cmd.params[5], 24, 8);
+ obj_desc->label[4] = mc_dec(cmd.params[5], 32, 8);
+ obj_desc->label[5] = mc_dec(cmd.params[5], 40, 8);
+ obj_desc->label[6] = mc_dec(cmd.params[5], 48, 8);
+ obj_desc->label[7] = mc_dec(cmd.params[5], 56, 8);
+ obj_desc->label[8] = mc_dec(cmd.params[6], 0, 8);
+ obj_desc->label[9] = mc_dec(cmd.params[6], 8, 8);
+ obj_desc->label[10] = mc_dec(cmd.params[6], 16, 8);
+ obj_desc->label[11] = mc_dec(cmd.params[6], 24, 8);
+ obj_desc->label[12] = mc_dec(cmd.params[6], 32, 8);
+ obj_desc->label[13] = mc_dec(cmd.params[6], 40, 8);
+ obj_desc->label[14] = mc_dec(cmd.params[6], 48, 8);
+ obj_desc->label[15] = '\0';
return 0;
}
EXPORT_SYMBOL(dprc_get_obj);
@@ -696,31 +805,6 @@ int dprc_get_res_ids(struct fsl_mc_io *mc_io,
}
EXPORT_SYMBOL(dprc_get_res_ids);

-int dprc_get_portal_paddr(struct fsl_mc_io *mc_io,
- uint16_t token,
- int portal_id,
- uint64_t *portal_addr)
-{
- struct mc_command cmd = { 0 };
- int err;
-
- /* prepare command */
- cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_PORTAL_PADDR,
- MC_CMD_PRI_LOW, token);
- cmd.params[0] |= mc_enc(0, 32, portal_id);
-
- /* send command to mc*/
- err = mc_send_command(mc_io, &cmd);
- if (err)
- return err;
-
- /* retrieve response parameters */
- *portal_addr = mc_dec(cmd.params[1], 0, 64);
-
- return 0;
-}
-EXPORT_SYMBOL(dprc_get_portal_paddr);
-
int dprc_get_obj_region(struct fsl_mc_io *mc_io,
uint16_t token,
char *obj_type,
@@ -759,13 +843,47 @@ int dprc_get_obj_region(struct fsl_mc_io *mc_io,
return err;

/* retrieve response parameters */
- region_desc->base_paddr = mc_dec(cmd.params[1], 0, 64);
+ region_desc->base_offset = mc_dec(cmd.params[1], 0, 64);
region_desc->size = mc_dec(cmd.params[2], 0, 32);

return 0;
}
EXPORT_SYMBOL(dprc_get_obj_region);

+int dprc_set_obj_label(struct fsl_mc_io *mc_io,
+ uint16_t token,
+ int obj_index,
+ char *label)
+{
+ struct mc_command cmd = { 0 };
+
+ /* prepare command */
+ cmd.header = mc_encode_cmd_header(DPRC_CMDID_SET_OBJ_LABEL,
+ MC_CMD_PRI_LOW, token);
+
+ cmd.params[0] |= mc_enc(0, 32, obj_index);
+ cmd.params[1] |= mc_enc(0, 8, label[0]);
+ cmd.params[1] |= mc_enc(8, 8, label[1]);
+ cmd.params[1] |= mc_enc(16, 8, label[2]);
+ cmd.params[1] |= mc_enc(24, 8, label[3]);
+ cmd.params[1] |= mc_enc(32, 8, label[4]);
+ cmd.params[1] |= mc_enc(40, 8, label[5]);
+ cmd.params[1] |= mc_enc(48, 8, label[6]);
+ cmd.params[1] |= mc_enc(56, 8, label[7]);
+ cmd.params[2] |= mc_enc(0, 8, label[8]);
+ cmd.params[2] |= mc_enc(8, 8, label[9]);
+ cmd.params[2] |= mc_enc(16, 8, label[10]);
+ cmd.params[2] |= mc_enc(24, 8, label[11]);
+ cmd.params[2] |= mc_enc(32, 8, label[12]);
+ cmd.params[2] |= mc_enc(40, 8, label[13]);
+ cmd.params[2] |= mc_enc(48, 8, label[14]);
+ cmd.params[2] |= mc_enc(56, 8, label[15]);
+
+ /* send command to mc*/
+ return mc_send_command(mc_io, &cmd);
+}
+EXPORT_SYMBOL(dprc_set_obj_label);
+
int dprc_connect(struct fsl_mc_io *mc_io,
uint16_t token,
const struct dprc_endpoint *endpoint1,
@@ -817,6 +935,7 @@ int dprc_connect(struct fsl_mc_io *mc_io,
/* send command to mc*/
return mc_send_command(mc_io, &cmd);
}
+EXPORT_SYMBOL(dprc_connect);

int dprc_disconnect(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -850,6 +969,7 @@ int dprc_disconnect(struct fsl_mc_io *mc_io,
/* send command to mc*/
return mc_send_command(mc_io, &cmd);
}
+EXPORT_SYMBOL(dprc_disconnect);

int dprc_get_connection(struct fsl_mc_io *mc_io,
uint16_t token,
@@ -911,3 +1031,4 @@ int dprc_get_connection(struct fsl_mc_io *mc_io,

return 0;
}
+EXPORT_SYMBOL(dprc_get_connection);
diff --git a/drivers/staging/fsl-mc/bus/mc-allocator.c b/drivers/staging/fsl-mc/bus/mc-allocator.c
index 27497e7..0c5fad0 100644
--- a/drivers/staging/fsl-mc/bus/mc-allocator.c
+++ b/drivers/staging/fsl-mc/bus/mc-allocator.c
@@ -531,13 +531,19 @@ int __must_check fsl_mc_allocate_irqs(struct fsl_mc_device *mc_dev)

irqs[i] = to_fsl_mc_irq(resource);
res_allocated_count++;
+
+ WARN_ON(irqs[i]->mc_dev);
+ irqs[i]->mc_dev = mc_dev;
+ irqs[i]->dev_irq_index = i;
}

mc_dev->irqs = irqs;
return 0;
error:
- for (i = 0; i < res_allocated_count; i++)
+ for (i = 0; i < res_allocated_count; i++) {
+ irqs[i]->mc_dev = NULL;
fsl_mc_resource_free(&irqs[i]->resource);
+ }

if (irqs)
devm_kfree(&mc_dev->dev, irqs);
@@ -555,8 +561,9 @@ void fsl_mc_free_irqs(struct fsl_mc_device *mc_dev)
int i;
int irq_count;
struct fsl_mc_bus *mc_bus;
+ struct fsl_mc_device_irq **irqs = mc_dev->irqs;

- if (WARN_ON(!mc_dev->irqs))
+ if (WARN_ON(!irqs))
return;

irq_count = mc_dev->obj_desc.irq_count;
@@ -569,8 +576,11 @@ void fsl_mc_free_irqs(struct fsl_mc_device *mc_dev)
if (WARN_ON(!mc_bus->irq_resources))
return;

- for (i = 0; i < irq_count; i++)
- fsl_mc_resource_free(&mc_dev->irqs[i]->resource);
+ for (i = 0; i < irq_count; i++) {
+ WARN_ON(!irqs[i]->mc_dev);
+ irqs[i]->mc_dev = NULL;
+ fsl_mc_resource_free(&irqs[i]->resource);
+ }

devm_kfree(&mc_dev->dev, mc_dev->irqs);
mc_dev->irqs = NULL;
@@ -604,8 +614,8 @@ static int fsl_mc_allocator_probe(struct fsl_mc_device *mc_dev)
if (error < 0)
goto error;

- dev_info(&mc_dev->dev,
- "Allocatable MC object device bound to fsl_mc_allocator driver");
+ dev_dbg(&mc_dev->dev,
+ "Allocatable MC object device bound to fsl_mc_allocator driver");
return 0;
error:

@@ -627,8 +637,8 @@ static int fsl_mc_allocator_remove(struct fsl_mc_device *mc_dev)
if (error < 0)
goto out;

- dev_info(&mc_dev->dev,
- "Allocatable MC object device unbound from fsl_mc_allocator driver");
+ dev_dbg(&mc_dev->dev,
+ "Allocatable MC object device unbound from fsl_mc_allocator driver");
error = 0;
out:
return error;
diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c
index 08fd559..55a8b86 100644
--- a/drivers/staging/fsl-mc/bus/mc-bus.c
+++ b/drivers/staging/fsl-mc/bus/mc-bus.c
@@ -315,7 +315,8 @@ common_cleanup:
return error;
}

-static int translate_mc_addr(uint64_t mc_addr, phys_addr_t *phys_addr)
+static int translate_mc_addr(enum fsl_mc_region_types mc_region_type,
+ uint64_t mc_offset, phys_addr_t *phys_addr)
{
int i;
struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);
@@ -324,7 +325,7 @@ static int translate_mc_addr(uint64_t mc_addr, phys_addr_t *phys_addr)
/*
* Do identity mapping:
*/
- *phys_addr = mc_addr;
+ *phys_addr = mc_offset;
return 0;
}

@@ -332,10 +333,11 @@ static int translate_mc_addr(uint64_t mc_addr, phys_addr_t *phys_addr)
struct fsl_mc_addr_translation_range *range =
&mc->translation_ranges[i];

- if (mc_addr >= range->start_mc_addr &&
- mc_addr < range->end_mc_addr) {
+ if (mc_region_type == range->mc_region_type &&
+ mc_offset >= range->start_mc_offset &&
+ mc_offset < range->end_mc_offset) {
*phys_addr = range->start_phys_addr +
- (mc_addr - range->start_mc_addr);
+ (mc_offset - range->start_mc_offset);
return 0;
}
}
@@ -351,6 +353,22 @@ static int fsl_mc_device_get_mmio_regions(struct fsl_mc_device *mc_dev,
struct resource *regions;
struct dprc_obj_desc *obj_desc = &mc_dev->obj_desc;
struct device *parent_dev = mc_dev->dev.parent;
+ enum fsl_mc_region_types mc_region_type;
+
+ if (strcmp(obj_desc->type, "dprc") == 0 ||
+ strcmp(obj_desc->type, "dpmcp") == 0) {
+ mc_region_type = FSL_MC_PORTAL;
+ } else if (strcmp(obj_desc->type, "dpio") == 0) {
+ mc_region_type = FSL_QBMAN_PORTAL;
+ } else {
+ /*
+ * This function should not have been called for this MC object
+ * type, as this object type is not supposed to have MMIO
+ * regions
+ */
+ WARN_ON(true);
+ return -EINVAL;
+ }

regions = kmalloc_array(obj_desc->region_count,
sizeof(regions[0]), GFP_KERNEL);
@@ -370,14 +388,14 @@ static int fsl_mc_device_get_mmio_regions(struct fsl_mc_device *mc_dev,
goto error_cleanup_regions;
}

- WARN_ON(region_desc.base_paddr == 0x0);
WARN_ON(region_desc.size == 0);
- error = translate_mc_addr(region_desc.base_paddr,
+ error = translate_mc_addr(mc_region_type,
+ region_desc.base_offset,
&regions[i].start);
if (error < 0) {
dev_err(parent_dev,
- "Invalid MC address: %#llx (for %s.%d\'s region %d)\n",
- region_desc.base_paddr,
+ "Invalid MC offset: %#llx (for %s.%d\'s region %d)\n",
+ region_desc.base_offset,
obj_desc->type, obj_desc->id, i);
goto error_cleanup_regions;
}
@@ -447,13 +465,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
}

device_initialize(&mc_dev->dev);
-
- /*
- * FIXME: Enable this code when the GIC-ITS MC support patch is merged
- */
-#ifdef GIC_ITS_MC_SUPPORT
INIT_LIST_HEAD(&mc_dev->dev.msi_list);
-#endif
mc_dev->dev.parent = parent_dev;
mc_dev->dev.bus = &fsl_mc_bus_type;
dev_set_name(&mc_dev->dev, "%s.%d", obj_desc->type, obj_desc->id);
@@ -586,10 +598,6 @@ void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
}
EXPORT_SYMBOL_GPL(fsl_mc_device_remove);

-/*
- * FIXME: Enable this code when the GIC-ITS MC support patch is merged
- */
-#ifdef GIC_ITS_MC_SUPPORT
static int mc_bus_msi_prepare(struct irq_domain *domain, struct device *dev,
int nvec, msi_alloc_info_t *info)
{
@@ -641,6 +649,10 @@ static void mc_bus_unmask_msi_irq(struct irq_data *d)
irq_chip_unmask_parent(d);
}

+/*
+ * This function is invoked from devm_request_irq(),
+ * devm_request_threaded_irq(), dev_free_irq()
+ */
static void mc_bus_msi_domain_write_msg(struct irq_data *irq_data,
struct msi_msg *msg)
{
@@ -657,6 +669,13 @@ static void mc_bus_msi_domain_write_msg(struct irq_data *irq_data,
irq_res->msi_paddr =
((u64)msg->address_hi << 32) | msg->address_lo;
irq_res->msi_value = msg->data;
+
+ /*
+ * NOTE: We cannot do the actual programming of the MSI
+ * in the MC, as this function is invoked in atomic context
+ * (interrupts disabled) and we cannot reliably send MC commands
+ * in atomic context.
+ */
}
}

@@ -706,10 +725,6 @@ static int create_mc_irq_domain(struct platform_device *mc_pdev,
goto cleanup_its_of_node;
}

- /*
- * FIXME: Enable this code when the GIC-ITS MC support patch is merged
- */
-#ifdef GIC_ITS_MC_SUPPORT
irq_domain = msi_create_irq_domain(mc_of_node, &mc_bus_msi_domain_info,
its_domain->parent);
if (!irq_domain) {
@@ -719,9 +734,6 @@ static int create_mc_irq_domain(struct platform_device *mc_pdev,
}

dev_dbg(&mc_pdev->dev, "Allocated MSI domain\n");
-#else
- irq_domain = NULL;
-#endif
*new_irq_domain = irq_domain;
return 0;

@@ -729,7 +741,6 @@ cleanup_its_of_node:
of_node_put(its_of_node);
return error;
}
-#endif /* GIC_ITS_MC_SUPPORT */

/*
* Initialize the interrupt pool associated with a MC bus.
@@ -739,26 +750,21 @@ int __must_check fsl_mc_populate_irq_pool(struct fsl_mc_bus *mc_bus,
unsigned int irq_count)
{
unsigned int i;
+ struct msi_desc *msi_entry;
+ struct msi_desc *next_msi_entry;
struct fsl_mc_device_irq *irq_resources;
struct fsl_mc_device_irq *irq_res;
+ int error;
struct fsl_mc_device *mc_bus_dev = &mc_bus->mc_dev;
+ struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);
struct fsl_mc_resource_pool *res_pool =
&mc_bus->resource_pools[FSL_MC_POOL_IRQ];
- /*
- * FIXME: Enable this code when the GIC-ITS MC support patch is merged
- */
-#ifdef GIC_ITS_MC_SUPPORT
- int error;
- struct msi_desc *msi_entry;
- struct msi_desc *next_msi_entry;
- struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);

/*
* Detect duplicate invocations of this function:
*/
if (WARN_ON(!list_empty(&mc_bus_dev->dev.msi_list)))
return -EINVAL;
-#endif

if (WARN_ON(irq_count == 0 ||
irq_count > FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS))
@@ -773,10 +779,6 @@ int __must_check fsl_mc_populate_irq_pool(struct fsl_mc_bus *mc_bus,

for (i = 0; i < irq_count; i++) {
irq_res = &irq_resources[i];
- /*
- * FIXME: Enable this code when the GIC-ITS MC support patch is merged
- */
-#ifdef GIC_ITS_MC_SUPPORT
msi_entry = alloc_msi_entry(&mc_bus_dev->dev);
if (!msi_entry) {
dev_err(&mc_bus_dev->dev, "Failed to allocate msi entry\n");
@@ -789,7 +791,6 @@ int __must_check fsl_mc_populate_irq_pool(struct fsl_mc_bus *mc_bus,
msi_entry->msi_attrib.entry_nr = i;
msi_entry->nvec_used = 1;
list_add_tail(&msi_entry->list, &mc_bus_dev->dev.msi_list);
-#endif

/*
* NOTE: irq_res->msi_paddr will be set by the
@@ -803,10 +804,6 @@ int __must_check fsl_mc_populate_irq_pool(struct fsl_mc_bus *mc_bus,
}

/*
- * FIXME: Enable this code when the GIC-ITS MC support patch is merged
- */
-#ifdef GIC_ITS_MC_SUPPORT
- /*
* NOTE: Calling this function will trigger the invocation of the
* mc_bus_msi_prepare() callback
*/
@@ -817,12 +814,7 @@ int __must_check fsl_mc_populate_irq_pool(struct fsl_mc_bus *mc_bus,
dev_err(&mc_bus_dev->dev, "Failed to allocate IRQs\n");
goto cleanup_msi_entries;
}
-#endif

- /*
- * FIXME: Enable this code when the GIC-ITS MC support patch is merged
- */
-#ifdef GIC_ITS_MC_SUPPORT
for_each_msi_entry(msi_entry, &mc_bus_dev->dev) {
u32 irq_num = msi_entry->irq;

@@ -830,17 +822,12 @@ int __must_check fsl_mc_populate_irq_pool(struct fsl_mc_bus *mc_bus,
irq_res->irq_number = irq_num;
irq_res->resource.id = irq_num;
}
-#endif

res_pool->max_count = irq_count;
res_pool->free_count = irq_count;
mc_bus->irq_resources = irq_resources;
return 0;

- /*
- * FIXME: Enable this code when the GIC-ITS MC support patch is merged
- */
-#ifdef GIC_ITS_MC_SUPPORT
cleanup_msi_entries:
list_for_each_entry_safe(msi_entry, next_msi_entry,
&mc_bus_dev->dev.msi_list, list)
@@ -848,7 +835,6 @@ cleanup_msi_entries:

devm_kfree(&mc_bus_dev->dev, irq_resources);
return error;
-#endif
}
EXPORT_SYMBOL_GPL(fsl_mc_populate_irq_pool);

@@ -858,32 +844,25 @@ EXPORT_SYMBOL_GPL(fsl_mc_populate_irq_pool);
*/
void fsl_mc_cleanup_irq_pool(struct fsl_mc_bus *mc_bus)
{
- /*
- * FIXME: Enable this code when the GIC-ITS MC support patch is merged
- */
-#ifdef GIC_ITS_MC_SUPPORT
struct msi_desc *msi_entry;
struct msi_desc *next_msi_entry;
struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);
-#endif
struct fsl_mc_resource_pool *res_pool =
&mc_bus->resource_pools[FSL_MC_POOL_IRQ];

+ if (WARN_ON(!mc_bus->irq_resources))
+ return;
+
if (WARN_ON(res_pool->max_count == 0))
return;

if (WARN_ON(res_pool->free_count != res_pool->max_count))
return;

- /*
- * FIXME: Enable this code when the GIC-ITS MC support patch is merged
- */
-#ifdef GIC_ITS_MC_SUPPORT
msi_domain_free_irqs(mc->irq_domain, &mc_bus->mc_dev.dev);
list_for_each_entry_safe(msi_entry, next_msi_entry,
&mc_bus->mc_dev.dev.msi_list, list)
kfree(msi_entry);
-#endif

devm_kfree(&mc_bus->mc_dev.dev, mc_bus->irq_resources);
res_pool->max_count = 0;
@@ -984,12 +963,14 @@ static int get_mc_addr_translation_ranges(struct device *dev,
for (i = 0; i < *num_ranges; ++i) {
struct fsl_mc_addr_translation_range *range = &(*ranges)[i];

- range->start_mc_addr = of_read_number(cell, mc_addr_cells);
+ range->mc_region_type = of_read_number(cell, 1);
+ range->start_mc_offset = of_read_number(cell + 1,
+ mc_addr_cells - 1);
cell += mc_addr_cells;
range->start_phys_addr = of_read_number(cell, paddr_cells);
cell += paddr_cells;
- range->end_mc_addr = range->start_mc_addr +
- of_read_number(cell, mc_size_cells);
+ range->end_mc_offset = range->start_mc_offset +
+ of_read_number(cell, mc_size_cells);

cell += mc_size_cells;
}
@@ -1021,15 +1002,9 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
return -ENOMEM;

platform_set_drvdata(pdev, mc);
-
- /*
- * FIXME: Enable this code when the GIC-ITS MC support patch is merged
- */
-#ifdef GIC_ITS_MC_SUPPORT
error = create_mc_irq_domain(pdev, &mc->irq_domain);
if (error < 0)
return error;
-#endif

/*
* Get physical address of MC portal for the root DPRC:
diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c b/drivers/staging/fsl-mc/bus/mc-sys.c
index 5737f59..9ae000c 100644
--- a/drivers/staging/fsl-mc/bus/mc-sys.c
+++ b/drivers/staging/fsl-mc/bus/mc-sys.c
@@ -256,8 +256,10 @@ int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
* TODO: When MC command completion interrupts are supported
* call wait function here instead of usleep_range()
*/
- usleep_range(MC_CMD_COMPLETION_POLLING_MIN_SLEEP_USECS,
- MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS);
+ if (preemptible()) {
+ usleep_range(MC_CMD_COMPLETION_POLLING_MIN_SLEEP_USECS,
+ MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS);
+ }

if (time_after_eq(jiffies, jiffies_until_timeout)) {
pr_debug("MC command timed out (portal: %#llx, obj handle: %#x, command: %#x)\n",
diff --git a/drivers/staging/fsl-mc/include/dpmng.h b/drivers/staging/fsl-mc/include/dpmng.h
index 1b052b8..a38eb1a 100644
--- a/drivers/staging/fsl-mc/include/dpmng.h
+++ b/drivers/staging/fsl-mc/include/dpmng.h
@@ -41,11 +41,11 @@ struct fsl_mc_io;
/**
* Management Complex firmware version information
*/
-#define MC_VER_MAJOR 6
+#define MC_VER_MAJOR 7
#define MC_VER_MINOR 0

/**
- * struct mc_versoin
+ * struct mc_version
* @major: Major version number: incremented on API compatibility changes
* @minor: Minor version number: incremented on API additions (that are
* backward compatible); reset when major version is incremented
diff --git a/drivers/staging/fsl-mc/include/dprc.h b/drivers/staging/fsl-mc/include/dprc.h
index f1862a7..610ea31 100644
--- a/drivers/staging/fsl-mc/include/dprc.h
+++ b/drivers/staging/fsl-mc/include/dprc.h
@@ -99,7 +99,7 @@ int dprc_close(struct fsl_mc_io *mc_io, uint16_t token);
/* Object initialization allowed - software context associated with this
* container is allowed to invoke object initialization operations.
*/
-#define DPRC_CFG_OPT_OBJ_CREATE_ALLOWED 0x00000004
+#define DPRC_CFG_OPT_OBJ_CREATE_ALLOWED 0x00000004

/* Topology change allowed - software context associated with this
* container is allowed to invoke topology operations, such as attach/detach
@@ -115,6 +115,9 @@ int dprc_close(struct fsl_mc_io *mc_io, uint16_t token);
/* AIOP - Indicates that container belongs to AIOP. */
#define DPRC_CFG_OPT_AIOP 0x00000020

+/* IRQ Config - Indicates that the container allowed to configure its IRQs. */
+#define DPRC_CFG_OPT_IRQ_CFG_ALLOWED 0x00000040
+
/**
* struct dprc_cfg - Container configuration options
* @icid: Container's ICID; if set to 'DPRC_GET_ICID_FROM_POOL', a free
@@ -122,11 +125,13 @@ int dprc_close(struct fsl_mc_io *mc_io, uint16_t token);
* @portal_id: Portal ID; if set to 'DPRC_GET_PORTAL_ID_FROM_POOL', a free
* portal ID is allocated by the DPRC
* @options: Combination of 'DPRC_CFG_OPT_<X>' options
+ * @label: Object's label
*/
struct dprc_cfg {
uint16_t icid;
int portal_id;
uint64_t options;
+ char label[16];
};

/**
@@ -135,8 +140,7 @@ struct dprc_cfg {
* @token: Token of DPRC object
* @cfg: Child container configuration
* @child_container_id: Returned child container ID
- * @child_portal_paddr: Returned base physical address of the
- * child portal
+ * @child_portal_offset: Returned child portal offset from MC portal base
*
* Return: '0' on Success; Error code otherwise.
*/
@@ -144,7 +148,7 @@ int dprc_create_container(struct fsl_mc_io *mc_io,
uint16_t token,
struct dprc_cfg *cfg,
int *child_container_id,
- uint64_t *child_portal_paddr);
+ uint64_t *child_portal_offset);

/**
* dprc_destroy_container() - Destroy child container.
@@ -201,16 +205,20 @@ int dprc_reset_container(struct fsl_mc_io *mc_io,
/* Number of dprc's IRQs */
#define DPRC_NUM_OF_IRQS 1

-/* Object irq events */
+/* DPRC IRQ events */

-/* IRQ event - Indicates that a new object assigned to the container */
+/* IRQ event - Indicates that a new object added to the container */
#define DPRC_IRQ_EVENT_OBJ_ADDED 0x00000001
-/* IRQ event - Indicates that an object was unassigned from the container */
+
+/* IRQ event - Indicates that an object was removed from the container */
#define DPRC_IRQ_EVENT_OBJ_REMOVED 0x00000002
-/* IRQ event - Indicates that resources assigned to the container */
+
+/* IRQ event - Indicates that resources added to the container */
#define DPRC_IRQ_EVENT_RES_ADDED 0x00000004
-/* IRQ event - Indicates that resources unassigned from the container */
+
+/* IRQ event - Indicates that resources removed from the container */
#define DPRC_IRQ_EVENT_RES_REMOVED 0x00000008
+
/* IRQ event - Indicates that one of the descendant containers that opened by
* this container is destroyed
*/
@@ -610,6 +618,7 @@ int dprc_get_obj_count(struct fsl_mc_io *mc_io, uint16_t token, int *obj_count);
* @irq_count: Number of interrupts supported by the object
* @region_count: Number of mappable regions supported by the object
* @state: Object state: combination of DPRC_OBJ_STATE_ states
+ * @label: Object label
*/
struct dprc_obj_desc {
char type[16];
@@ -620,6 +629,7 @@ struct dprc_obj_desc {
uint8_t irq_count;
uint8_t region_count;
uint32_t state;
+ char label[16];
};

/**
@@ -642,6 +652,53 @@ int dprc_get_obj(struct fsl_mc_io *mc_io,
struct dprc_obj_desc *obj_desc);

/**
+ * dprc_obj_set_irq() - Set IRQ information for object to trigger an interrupt.
+ * @mc_io: Pointer to MC portal's I/O object
+ * @token: Token of DPRC object
+ * @obj_index: Index of the object to set its IRQ (< obj_count returned from
+ * dprc_get_obj_count())
+ * @irq_index: Identifies the interrupt index to configure
+ * @irq_addr: Address that must be written to
+ * signal a message-based interrupt
+ * @irq_val: Value to write into irq_addr address
+ * @user_irq_id: Returned a user defined number associated with this IRQ
+ *
+ * Return: '0' on Success; Error code otherwise.
+ */
+int dprc_obj_set_irq(struct fsl_mc_io *mc_io,
+ uint16_t token,
+ int obj_index,
+ uint8_t irq_index,
+ uint64_t irq_addr,
+ uint32_t irq_val,
+ int user_irq_id);
+
+/**
+ * dprc_obj_get_irq() - Get IRQ information from object.
+ * @mc_io: Pointer to MC portal's I/O object
+ * @token: Token of DPRC object
+ * @obj_index: Index of the object to get its IRQ (< obj_count returned from
+ * dprc_get_obj_count())
+ * @irq_index: The interrupt index to configure
+ * @type: Returned interrupt type: 0 represents message interrupt
+ * type (both irq_addr and irq_val are valid)
+ * @irq_addr: Returned address that must be written to
+ * signal the message-based interrupt
+ * @irq_val: Value to write into irq_addr address
+ * @user_irq_id: A user defined number associated with this IRQ
+ *
+ * Return: '0' on Success; Error code otherwise.
+ */
+int dprc_obj_get_irq(struct fsl_mc_io *mc_io,
+ uint16_t token,
+ int obj_index,
+ uint8_t irq_index,
+ int *type,
+ uint64_t *irq_addr,
+ uint32_t *irq_val,
+ int *user_irq_id);
+
+/**
* dprc_get_res_count() - Obtains the number of free resources that are assigned
* to this container, by pool type
* @mc_io: Pointer to MC portal's I/O object
@@ -699,26 +756,15 @@ int dprc_get_res_ids(struct fsl_mc_io *mc_io,
struct dprc_res_ids_range_desc *range_desc);

/**
- * dprc_get_portal_paddr() - Get the physical address of MC portals
- * @mc_io: Pointer to MC portal's I/O object
- * @token: Token of DPRC object
- * @portal_id: MC portal ID
- * @portal_addr: The physical address of the MC portal ID
- *
- * Return: '0' on Success; Error code otherwise.
- */
-int dprc_get_portal_paddr(struct fsl_mc_io *mc_io,
- uint16_t token,
- int portal_id,
- uint64_t *portal_addr);
-
-/**
* struct dprc_region_desc - Mappable region descriptor
- * @base_paddr: Region base physical address
+ * @base_offset: Region offset from region's base address.
+ * For DPMCP and DPRC objects, region base is offset from SoC MC portals
+ * base address; For DPIO, region base is offset from SoC QMan portals
+ * base address
* @size: Region size (in bytes)
*/
struct dprc_region_desc {
- uint64_t base_paddr;
+ uint64_t base_offset;
uint32_t size;
};

@@ -741,6 +787,20 @@ int dprc_get_obj_region(struct fsl_mc_io *mc_io,
struct dprc_region_desc *region_desc);

/**
+ * dprc_set_obj_label() - Set object label.
+ * @mc_io: Pointer to MC portal's I/O object
+ * @token: Token of DPRC object
+ * @obj_index; Object index
+ * @label: The required label. The maximum length is 16 chars.
+ *
+ * Return: '0' on Success; Error code otherwise.
+ */
+int dprc_set_obj_label(struct fsl_mc_io *mc_io,
+ uint16_t token,
+ int obj_index,
+ char *label);
+
+/**
* struct dprc_endpoint - Endpoint description for link connect/disconnect
* operations
* @type: Endpoint object type: NULL terminated string
@@ -783,8 +843,8 @@ int dprc_disconnect(struct fsl_mc_io *mc_io,
/**
* dprc_get_connection() - Get connected endpoint and link status if connection
* exists.
-* @mc_io Pointer to MC portal's I/O object
-* @token Token of DPRC object
+* @mc_io Pointer to MC portal's I/O object
+* @token Token of DPRC object
* @endpoint1 Endpoint 1 configuration parameters
* @endpoint2 Returned endpoint 2 configuration parameters
* @state: Returned link state: 1 - link is up, 0 - link is down
diff --git a/drivers/staging/fsl-mc/include/mc-private.h b/drivers/staging/fsl-mc/include/mc-private.h
index 5b9c8f2..67ba488 100644
--- a/drivers/staging/fsl-mc/include/mc-private.h
+++ b/drivers/staging/fsl-mc/include/mc-private.h
@@ -52,16 +52,31 @@ struct fsl_mc {
};

/**
+ * enum mc_region_types - Types of MC MMIO regions
+ */
+enum fsl_mc_region_types {
+ FSL_MC_PORTAL = 0x0,
+ FSL_QBMAN_PORTAL,
+
+ /*
+ * New offset types must be added above this entry
+ */
+ FSL_NUM_MC_OFFSET_TYPES
+};
+
+/**
* struct fsl_mc_addr_translation_range - bus to system address translation
* range
- * @start_mc_addr: Start MC address of the range being translated
- * @end_mc_addr: MC address of the first byte after the range (last MC
- * address of the range is end_mc_addr - 1)
+ * @mc_region_type: Type of MC region for the range being translated
+ * @start_mc_offset: Start MC offset of the range being translated
+ * @end_mc_offset: MC offset of the first byte after the range (last MC
+ * offset of the range is end_mc_offset - 1)
* @start_phys_addr: system physical address corresponding to start_mc_addr
*/
struct fsl_mc_addr_translation_range {
- uint64_t start_mc_addr;
- uint64_t end_mc_addr;
+ enum fsl_mc_region_types mc_region_type;
+ uint64_t start_mc_offset;
+ uint64_t end_mc_offset;
phys_addr_t start_phys_addr;
};

@@ -115,6 +130,10 @@ int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev,
const char *driver_override,
unsigned int *total_irq_count);

+int dprc_lookup_object(struct fsl_mc_device *mc_bus_dev,
+ struct fsl_mc_device *child_dev,
+ uint32_t *child_obj_index);
+
int __init dprc_driver_init(void);

void __exit dprc_driver_exit(void);
diff --git a/drivers/staging/fsl-mc/include/mc.h b/drivers/staging/fsl-mc/include/mc.h
index f965c4d..0a2f381d 100644
--- a/drivers/staging/fsl-mc/include/mc.h
+++ b/drivers/staging/fsl-mc/include/mc.h
@@ -110,12 +110,16 @@ struct fsl_mc_resource {
* @msi_paddr: message-based interrupt physical address
* @msi_value: message-based interrupt data value
* @irq_number: Linux IRQ number assigned to the interrupt
+ * @mc_dev: MC object device that owns this interrupt
+ * @dev_irq_index: device-relative IRQ index
* @resource: MC generic resource associated with the interrupt
*/
struct fsl_mc_device_irq {
phys_addr_t msi_paddr;
uint32_t msi_value;
uint32_t irq_number;
+ struct fsl_mc_device *mc_dev;
+ uint8_t dev_irq_index;
struct fsl_mc_resource resource;
};

--
2.3.3

2015-04-28 18:03:20

by J. German Rivera

[permalink] [raw]
Subject: [PATCH 5/7] staging: fsl-mc: Allow the MC bus driver to run without GIC support

If the lpi-parent property is not present in the fsl,qoriq-mc node
of the device tree, the MC bus driver will assume that the GIC is not
supported.

This change is made in order to be able to use the MC bus driver in a
KVM VM, without having GIC-ITS support in guests. Added function
fsl_mc_interrupts_supported(), which can be called from DPAA2 object
drivers.

Signed-off-by: J. German Rivera <[email protected]>
Change-Id: I881ab2c45c949e55cfafe1d281a7a31560955e5b
Reviewed-on: http://git.am.freescale.net:8181/34712
Tested-by: Review Code-CDREVIEW <[email protected]>
Reviewed-by: Stuart Yoder <[email protected]>
---
drivers/staging/fsl-mc/bus/dprc-driver.c | 31 +++++++++++++++++++----------
drivers/staging/fsl-mc/bus/mc-allocator.c | 4 ++++
drivers/staging/fsl-mc/bus/mc-bus.c | 16 +++++++++++++--
drivers/staging/fsl-mc/include/mc-private.h | 4 ++++
drivers/staging/fsl-mc/include/mc.h | 2 ++
5 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c
index b84cca3..ff321c9 100644
--- a/drivers/staging/fsl-mc/bus/dprc-driver.c
+++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
@@ -412,6 +412,7 @@ static int dprc_scan_container(struct fsl_mc_device *mc_bus_dev)
int error;
unsigned int irq_count;
struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_bus_dev);
+ struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);

dprc_init_all_resource_pools(mc_bus_dev);

@@ -424,7 +425,7 @@ static int dprc_scan_container(struct fsl_mc_device *mc_bus_dev)
if (error < 0)
goto error;

- if (!mc_bus->irq_resources) {
+ if (mc->gic_supported && !mc_bus->irq_resources) {
irq_count += FSL_MC_IRQ_POOL_MAX_EXTRA_IRQS;
error = fsl_mc_populate_irq_pool(mc_bus, irq_count);
if (error < 0)
@@ -754,6 +755,7 @@ static int dprc_probe(struct fsl_mc_device *mc_dev)
int error;
size_t region_size;
struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_dev);
+ struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);

if (WARN_ON(strcmp(mc_dev->obj_desc.type, "dprc") != 0))
return -EINVAL;
@@ -792,12 +794,15 @@ static int dprc_probe(struct fsl_mc_device *mc_dev)
if (error < 0)
goto error_cleanup_open;

- /*
- * Configure interrupts for the DPRC object associated with this MC bus:
- */
- error = dprc_setup_irqs(mc_dev);
- if (error < 0)
- goto error_cleanup_dprc_scan;
+ if (mc->gic_supported) {
+ /*
+ * Configure interrupts for the DPRC object associated with
+ * this MC bus:
+ */
+ error = dprc_setup_irqs(mc_dev);
+ if (error < 0)
+ goto error_cleanup_dprc_scan;
+ }

dev_info(&mc_dev->dev, "DPRC device bound to driver");
return 0;
@@ -805,7 +810,8 @@ static int dprc_probe(struct fsl_mc_device *mc_dev)
error_cleanup_dprc_scan:
device_for_each_child(&mc_dev->dev, NULL, __fsl_mc_device_remove);
dprc_cleanup_all_resource_pools(mc_dev);
- fsl_mc_cleanup_irq_pool(mc_bus);
+ if (mc->gic_supported)
+ fsl_mc_cleanup_irq_pool(mc_bus);

error_cleanup_open:
(void)dprc_close(mc_dev->mc_io, mc_dev->mc_handle);
@@ -839,6 +845,7 @@ static int dprc_remove(struct fsl_mc_device *mc_dev)
{
int error;
struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_dev);
+ struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);

if (WARN_ON(strcmp(mc_dev->obj_desc.type, "dprc") != 0))
return -EINVAL;
@@ -848,14 +855,18 @@ static int dprc_remove(struct fsl_mc_device *mc_dev)
if (WARN_ON(!mc_bus->irq_resources))
return -EINVAL;

- dprc_teardown_irqs(mc_dev);
+ if (mc->gic_supported)
+ dprc_teardown_irqs(mc_dev);
+
device_for_each_child(&mc_dev->dev, NULL, __fsl_mc_device_remove);
dprc_cleanup_all_resource_pools(mc_dev);
error = dprc_close(mc_dev->mc_io, mc_dev->mc_handle);
if (error < 0)
dev_err(&mc_dev->dev, "dprc_close() failed: %d\n", error);

- fsl_mc_cleanup_irq_pool(mc_bus);
+ if (mc->gic_supported)
+ fsl_mc_cleanup_irq_pool(mc_bus);
+
dev_info(&mc_dev->dev, "DPRC device unbound from driver");
return 0;
}
diff --git a/drivers/staging/fsl-mc/bus/mc-allocator.c b/drivers/staging/fsl-mc/bus/mc-allocator.c
index 0c5fad0..313a533 100644
--- a/drivers/staging/fsl-mc/bus/mc-allocator.c
+++ b/drivers/staging/fsl-mc/bus/mc-allocator.c
@@ -489,6 +489,10 @@ int __must_check fsl_mc_allocate_irqs(struct fsl_mc_device *mc_dev)
struct fsl_mc_device_irq **irqs = NULL;
struct fsl_mc_bus *mc_bus;
struct fsl_mc_resource_pool *res_pool;
+ struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);
+
+ if (!mc->gic_supported)
+ return -ENOTSUPP;

if (WARN_ON(mc_dev->irqs))
goto error;
diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c
index 55a8b86..be2faf6 100644
--- a/drivers/staging/fsl-mc/bus/mc-bus.c
+++ b/drivers/staging/fsl-mc/bus/mc-bus.c
@@ -287,6 +287,14 @@ void fsl_mc_driver_unregister(struct fsl_mc_driver *mc_driver)
}
EXPORT_SYMBOL_GPL(fsl_mc_driver_unregister);

+bool fsl_mc_interrupts_supported(void)
+{
+ struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);
+
+ return mc->gic_supported;
+}
+EXPORT_SYMBOL_GPL(fsl_mc_interrupts_supported);
+
static int get_dprc_icid(struct fsl_mc_io *mc_io,
int container_id, uint16_t *icid)
{
@@ -1003,8 +1011,12 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, mc);
error = create_mc_irq_domain(pdev, &mc->irq_domain);
- if (error < 0)
- return error;
+ if (error < 0) {
+ dev_warn(&pdev->dev,
+ "WARNING: MC bus driver will run without interrupt support\n");
+ } else {
+ mc->gic_supported = true;
+ }

/*
* Get physical address of MC portal for the root DPRC:
diff --git a/drivers/staging/fsl-mc/include/mc-private.h b/drivers/staging/fsl-mc/include/mc-private.h
index 67ba488..0757258 100644
--- a/drivers/staging/fsl-mc/include/mc-private.h
+++ b/drivers/staging/fsl-mc/include/mc-private.h
@@ -42,11 +42,15 @@
* struct fsl_mc - Private data of a "fsl,qoriq-mc" platform device
* @root_mc_bus_dev: MC object device representing the root DPRC
* @irq_domain: IRQ domain for the fsl-mc bus type
+ * @gic_supported: boolean flag that indicates if the GIC interrupt controller
+ * is supported.
+ * @num_translation_ranges: number of entries in addr_translation_ranges
* @addr_translation_ranges: array of bus to system address translation ranges
*/
struct fsl_mc {
struct fsl_mc_device *root_mc_bus_dev;
struct irq_domain *irq_domain;
+ bool gic_supported;
uint8_t num_translation_ranges;
struct fsl_mc_addr_translation_range *translation_ranges;
};
diff --git a/drivers/staging/fsl-mc/include/mc.h b/drivers/staging/fsl-mc/include/mc.h
index 0a2f381d..c2a702e4 100644
--- a/drivers/staging/fsl-mc/include/mc.h
+++ b/drivers/staging/fsl-mc/include/mc.h
@@ -209,6 +209,8 @@ int __must_check __fsl_mc_driver_register(struct fsl_mc_driver *fsl_mc_driver,

void fsl_mc_driver_unregister(struct fsl_mc_driver *driver);

+bool fsl_mc_interrupts_supported(void);
+
int __must_check fsl_mc_portal_allocate(struct fsl_mc_device *mc_dev,
uint16_t mc_io_flags,
struct fsl_mc_io **new_mc_io);
--
2.3.3

2015-04-28 17:47:16

by J. German Rivera

[permalink] [raw]
Subject: [PATCH 6/7] staging: fsl-mc: Add locking to serialize mc_send_command() calls

Add a locking mechanism to serialize mc_send_command() calls that use
the same fsl_mc_io object (same MC portal). All mc_send_command() calls
with the same fsl_m_io object have to be either from non-atomic context
or from atomic context, but not both. When the fsl_mc_io object is
created the owner needs to know in which type of context the fsl_mc_io
object is going to be used. A flag passed-in to fsl_create_mc_io()
will indicate whether the fsl_mc_io object will be used in atomic or
non-atomic context. If the fsl_mc_io object is going to be used in
non-atomic context, mc_send_command() calls with it will be
serialized using a mutex. Otherwise, if the fsl_mc_io object is
going to be used in atomic context, mc_semd_command() calls with it
will be serialized using a spinlock.

Signed-off-by: J. German Rivera <[email protected]>
Change-Id: Icb770cd36e204ee6a17ad0f81e1d31cc9fe96816
Reviewed-on: http://git.am.freescale.net:8181/34876
Tested-by: Review Code-CDREVIEW <[email protected]>
Reviewed-by: Stuart Yoder <[email protected]>
---
drivers/staging/fsl-mc/bus/mc-sys.c | 36 ++++++++++++++++++++++++++++-----
drivers/staging/fsl-mc/include/mc-sys.h | 23 +++++++++++++++++++--
2 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c b/drivers/staging/fsl-mc/bus/mc-sys.c
index 9ae000c..1679054 100644
--- a/drivers/staging/fsl-mc/bus/mc-sys.c
+++ b/drivers/staging/fsl-mc/bus/mc-sys.c
@@ -86,6 +86,11 @@ int __must_check fsl_create_mc_io(struct device *dev,
mc_io->portal_phys_addr = mc_portal_phys_addr;
mc_io->portal_size = mc_portal_size;
mc_io->resource = resource;
+ if (flags & FSL_MC_IO_ATOMIC_CONTEXT_PORTAL)
+ spin_lock_init(&mc_io->spinlock);
+ else
+ mutex_init(&mc_io->mutex);
+
res = devm_request_mem_region(dev,
mc_portal_phys_addr,
mc_portal_size,
@@ -230,15 +235,26 @@ static inline enum mc_cmd_status mc_read_response(struct mc_command __iomem *
* @cmd: command to be sent
*
* Returns '0' on Success; Error code otherwise.
- *
- * NOTE: This function cannot be invoked from from atomic contexts.
*/
int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
{
+ int error;
enum mc_cmd_status status;
unsigned long jiffies_until_timeout =
jiffies + MC_CMD_COMPLETION_TIMEOUT_JIFFIES;

+ if (preemptible()) {
+ if (WARN_ON(mc_io->flags & FSL_MC_IO_ATOMIC_CONTEXT_PORTAL))
+ return -EINVAL;
+
+ mutex_lock(&mc_io->mutex);
+ } else {
+ if (WARN_ON(!(mc_io->flags & FSL_MC_IO_ATOMIC_CONTEXT_PORTAL)))
+ return -EINVAL;
+
+ spin_lock(&mc_io->spinlock);
+ }
+
/*
* Send command to the MC hardware:
*/
@@ -269,7 +285,8 @@ int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
(unsigned int)
MC_CMD_HDR_READ_CMDID(cmd->header));

- return -ETIMEDOUT;
+ error = -ETIMEDOUT;
+ goto common_exit;
}
}

@@ -281,9 +298,18 @@ int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
mc_status_to_string(status),
(unsigned int)status);

- return mc_status_to_error(status);
+ error = mc_status_to_error(status);
+ goto common_exit;
}

- return 0;
+ error = 0;
+
+common_exit:
+ if (preemptible())
+ mutex_unlock(&mc_io->mutex);
+ else
+ spin_unlock(&mc_io->spinlock);
+
+ return error;
}
EXPORT_SYMBOL(mc_send_command);
diff --git a/drivers/staging/fsl-mc/include/mc-sys.h b/drivers/staging/fsl-mc/include/mc-sys.h
index cb3b5a2..24e51f7 100644
--- a/drivers/staging/fsl-mc/include/mc-sys.h
+++ b/drivers/staging/fsl-mc/include/mc-sys.h
@@ -39,6 +39,13 @@
#include <linux/errno.h>
#include <linux/io.h>
#include <linux/dma-mapping.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+
+/**
+ * Bit masks for a MC I/O object (struct fsl_mc_io) flags
+ */
+#define FSL_MC_IO_ATOMIC_CONTEXT_PORTAL 0x0001

struct fsl_mc_resource;
struct mc_command;
@@ -53,14 +60,26 @@ struct mc_command;
* @resource: generic resource associated with the MC portal if
* the MC portal came from a resource pool, or NULL if the MC portal
* is permanently bound to a device (e.g., a DPRC)
+ * @mutex: Mutex to serialize mc_send_command() calls that use the same MC
+ * portal, if the fsl_mc_io object was created with the
+ * FSL_MC_IO_ATOMIC_CONTEXT_PORTAL flag off. mc_send_command() calls for this
+ * fsl_mc_io object must be made only from non-atomic context.
+ * @spinlock: Spinlock to serialize mc_send_command() calls that use the same MC
+ * portal, if the fsl_mc_io object was created with the
+ * FSL_MC_IO_ATOMIC_CONTEXT_PORTAL flag on. mc_send_command() calls for this
+ * fsl_mc_io object must be made only from atomic context.
*/
struct fsl_mc_io {
struct device *dev;
- uint32_t flags;
- uint32_t portal_size;
+ uint16_t flags;
+ uint16_t portal_size;
phys_addr_t portal_phys_addr;
void __iomem *portal_virt_addr;
struct fsl_mc_resource *resource;
+ union {
+ struct mutex mutex; /* serializes mc_send_command() calls */
+ spinlock_t spinlock; /* serializes mc_send_command() calls */
+ };
};

int __must_check fsl_create_mc_io(struct device *dev,
--
2.3.3

2015-04-28 18:02:55

by J. German Rivera

[permalink] [raw]
Subject: [PATCH 7/7] staging: fsl-mc: Use DPMCP IRQ and completion var to wait for MC

- Refactored fsl_mc_io object to have a DPMCP object attached to it
- Created DPMCP object for DPRC's built-in portal, so that waiting
on MC command completions for MC commands sent on the DPRC's built-in
portal can be done using a DPMCP interrupt and a Linux completion
variable. For most cases, mc_send_command() will wait on this
completion variable, instead of doing polling. This completion
variable will be signaled from the DPMCP IRQ handler.

Signed-off-by: J. German Rivera <[email protected]>
Change-Id: Iab294be9c00fb029702cc8625eaf29ba058fa960
Reviewed-on: http://git.am.freescale.net:8181/34276
Tested-by: Review Code-CDREVIEW <[email protected]>
Reviewed-by: Stuart Yoder <[email protected]>
---
drivers/staging/fsl-mc/bus/dprc-driver.c | 231 +++++++++++-----
drivers/staging/fsl-mc/bus/mc-allocator.c | 107 +++----
drivers/staging/fsl-mc/bus/mc-bus.c | 95 ++++++-
drivers/staging/fsl-mc/bus/mc-sys.c | 414 +++++++++++++++++++++++++---
drivers/staging/fsl-mc/include/mc-private.h | 6 +-
drivers/staging/fsl-mc/include/mc-sys.h | 37 ++-
6 files changed, 721 insertions(+), 169 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c
index ff321c9..0fb065c 100644
--- a/drivers/staging/fsl-mc/bus/dprc-driver.c
+++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
@@ -15,6 +15,7 @@
#include <linux/slab.h>
#include <linux/interrupt.h>
#include "dprc-cmd.h"
+#include "dpmcp.h"

struct dprc_child_objs {
int child_count;
@@ -348,57 +349,6 @@ int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev,
EXPORT_SYMBOL_GPL(dprc_scan_objects);

/**
- * dprc_lookup_object - Finds a given MC object in a DPRC and returns
- * the index of the object in the DPRC
- *
- * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
- * @child_dev: pointer to the fsl-mc device to be looked up
- * @child_obj_index: output parameter to hold the index of the object
- */
-int dprc_lookup_object(struct fsl_mc_device *mc_bus_dev,
- struct fsl_mc_device *child_dev,
- uint32_t *child_obj_index)
-{
- int i;
- int num_child_objects;
- int error;
-
- error = dprc_get_obj_count(mc_bus_dev->mc_io,
- mc_bus_dev->mc_handle,
- &num_child_objects);
- if (error < 0) {
- dev_err(&mc_bus_dev->dev, "dprc_get_obj_count() failed: %d\n",
- error);
- return error;
- }
-
- for (i = 0; i < num_child_objects; i++) {
- struct dprc_obj_desc obj_desc;
-
- error = dprc_get_obj(mc_bus_dev->mc_io,
- mc_bus_dev->mc_handle,
- i, &obj_desc);
- if (error < 0) {
- dev_err(&mc_bus_dev->dev,
- "dprc_get_obj(i=%d) failed: %d\n",
- i, error);
- return error;
- }
-
- if (strcmp(obj_desc.type, child_dev->obj_desc.type) == 0 &&
- obj_desc.id == child_dev->obj_desc.id) {
- *child_obj_index = i;
- return 0;
- }
- }
-
- dev_err(&mc_bus_dev->dev, "%s.%u not found\n",
- child_dev->obj_desc.type, child_dev->obj_desc.id);
-
- return -ENODEV;
-}
-
-/**
* dprc_scan_container - Scans a physical DPRC and synchronizes Linux bus state
*
* @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
@@ -412,7 +362,6 @@ static int dprc_scan_container(struct fsl_mc_device *mc_bus_dev)
int error;
unsigned int irq_count;
struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_bus_dev);
- struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);

dprc_init_all_resource_pools(mc_bus_dev);

@@ -425,7 +374,7 @@ static int dprc_scan_container(struct fsl_mc_device *mc_bus_dev)
if (error < 0)
goto error;

- if (mc->gic_supported && !mc_bus->irq_resources) {
+ if (fsl_mc_interrupts_supported() && !mc_bus->irq_resources) {
irq_count += FSL_MC_IRQ_POOL_MAX_EXTRA_IRQS;
error = fsl_mc_populate_irq_pool(mc_bus, irq_count);
if (error < 0)
@@ -466,7 +415,8 @@ static irqreturn_t dprc_irq0_handler_thread(int irq_num, void *arg)
struct fsl_mc_io *mc_io = mc_dev->mc_io;
int irq_index = 0;

- dev_dbg(dev, "DPRC IRQ %d\n", irq_num);
+ dev_dbg(dev, "DPRC IRQ %d triggered on CPU %u\n",
+ irq_num, smp_processor_id());
if (WARN_ON(!(mc_dev->flags & FSL_MC_IS_DPRC)))
return IRQ_HANDLED;

@@ -540,7 +490,8 @@ static int disable_dprc_irqs(struct fsl_mc_device *mc_dev)
error = dprc_set_irq_enable(mc_io, mc_dev->mc_handle, i, 0);
if (error < 0) {
dev_err(&mc_dev->dev,
- "dprc_set_irq_enable() failed: %d\n", error);
+ "Disabling DPRC IRQ %d failed: dprc_set_irq_enable() failed: %d\n",
+ i, error);

return error;
}
@@ -551,7 +502,8 @@ static int disable_dprc_irqs(struct fsl_mc_device *mc_dev)
error = dprc_set_irq_mask(mc_io, mc_dev->mc_handle, i, 0x0);
if (error < 0) {
dev_err(&mc_dev->dev,
- "dprc_set_irq_mask() failed: %d\n", error);
+ "Disabling DPRC IRQ %d failed: dprc_set_irq_mask() failed: %d\n",
+ i, error);

return error;
}
@@ -563,8 +515,9 @@ static int disable_dprc_irqs(struct fsl_mc_device *mc_dev)
~0x0U);
if (error < 0) {
dev_err(&mc_dev->dev,
- "dprc_clear_irq_status() failed: %d\n",
- error);
+ "Disabling DPRC IRQ %d failed: dprc_clear_irq_status() failed: %d\n",
+ i, error);
+
return error;
}
}
@@ -685,7 +638,8 @@ static int enable_dprc_irqs(struct fsl_mc_device *mc_dev)
~0x0u);
if (error < 0) {
dev_err(&mc_dev->dev,
- "dprc_set_irq_mask() failed: %d\n", error);
+ "Enabling DPRC IRQ %d failed: dprc_set_irq_mask() failed: %d\n",
+ i, error);

return error;
}
@@ -698,7 +652,8 @@ static int enable_dprc_irqs(struct fsl_mc_device *mc_dev)
i, 1);
if (error < 0) {
dev_err(&mc_dev->dev,
- "dprc_set_irq_enable() failed: %d\n", error);
+ "Enabling DPRC IRQ %d failed: dprc_set_irq_enable() failed: %d\n",
+ i, error);

return error;
}
@@ -740,6 +695,95 @@ error_free_irqs:
return error;
}

+/*
+ * Creates a DPMCP for a DPRC's built-in MC portal
+ */
+static int dprc_create_dpmcp(struct fsl_mc_device *dprc_dev)
+{
+ int error;
+ struct dpmcp_cfg dpmcp_cfg;
+ uint16_t dpmcp_handle;
+ struct dprc_res_req res_req;
+ struct dpmcp_attr dpmcp_attr;
+ struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(dprc_dev);
+
+ dpmcp_cfg.portal_id = mc_bus->dprc_attr.portal_id;
+ error = dpmcp_create(dprc_dev->mc_io, &dpmcp_cfg, &dpmcp_handle);
+ if (error < 0) {
+ dev_err(&dprc_dev->dev, "dpmcp_create() failed: %d\n",
+ error);
+ return error;
+ }
+
+ /*
+ * Set the state of the newly created DPMCP object to be "plugged":
+ */
+
+ error = dpmcp_get_attributes(dprc_dev->mc_io, dpmcp_handle,
+ &dpmcp_attr);
+ if (error < 0) {
+ dev_err(&dprc_dev->dev, "dpmcp_get_attributes() failed: %d\n",
+ error);
+ goto error_destroy_dpmcp;
+ }
+
+ if (WARN_ON(dpmcp_attr.id != mc_bus->dprc_attr.portal_id)) {
+ error = -EINVAL;
+ goto error_destroy_dpmcp;
+ }
+
+ strcpy(res_req.type, "dpmcp");
+ res_req.num = 1;
+ res_req.options =
+ (DPRC_RES_REQ_OPT_EXPLICIT | DPRC_RES_REQ_OPT_PLUGGED);
+ res_req.id_base_align = dpmcp_attr.id;
+
+ error = dprc_assign(dprc_dev->mc_io,
+ dprc_dev->mc_handle,
+ dprc_dev->obj_desc.id,
+ &res_req);
+
+ if (error < 0) {
+ dev_err(&dprc_dev->dev, "dprc_assign() failed: %d\n", error);
+ goto error_destroy_dpmcp;
+ }
+
+ (void)dpmcp_close(dprc_dev->mc_io, dpmcp_handle);
+ return 0;
+
+error_destroy_dpmcp:
+ (void)dpmcp_destroy(dprc_dev->mc_io, dpmcp_handle);
+ return error;
+}
+
+/*
+ * Destroys the DPMCP for a DPRC's built-in MC portal
+ */
+static void dprc_destroy_dpmcp(struct fsl_mc_device *dprc_dev)
+{
+ int error;
+ uint16_t dpmcp_handle;
+ struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(dprc_dev);
+
+ if (WARN_ON(!dprc_dev->mc_io || dprc_dev->mc_io->dpmcp_dev))
+ return;
+
+ error = dpmcp_open(dprc_dev->mc_io, mc_bus->dprc_attr.portal_id,
+ &dpmcp_handle);
+ if (error < 0) {
+ dev_err(&dprc_dev->dev, "dpmcp_open() failed: %d\n",
+ error);
+ return;
+ }
+
+ error = dpmcp_destroy(dprc_dev->mc_io, dpmcp_handle);
+ if (error < 0) {
+ dev_err(&dprc_dev->dev, "dpmcp_destroy() failed: %d\n",
+ error);
+ return;
+ }
+}
+
/**
* dprc_probe - callback invoked when a DPRC is being bound to this driver
*
@@ -755,7 +799,7 @@ static int dprc_probe(struct fsl_mc_device *mc_dev)
int error;
size_t region_size;
struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_dev);
- struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);
+ bool mc_io_created = false;

if (WARN_ON(strcmp(mc_dev->obj_desc.type, "dprc") != 0))
return -EINVAL;
@@ -776,6 +820,8 @@ static int dprc_probe(struct fsl_mc_device *mc_dev)
NULL, 0, &mc_dev->mc_io);
if (error < 0)
return error;
+
+ mc_io_created = true;
}

error = dprc_open(mc_dev->mc_io, mc_dev->obj_desc.id,
@@ -785,16 +831,55 @@ static int dprc_probe(struct fsl_mc_device *mc_dev)
goto error_cleanup_mc_io;
}

+ error = dprc_get_attributes(mc_dev->mc_io, mc_dev->mc_handle,
+ &mc_bus->dprc_attr);
+ if (error < 0) {
+ dev_err(&mc_dev->dev, "dprc_get_attributes() failed: %d\n",
+ error);
+ goto error_cleanup_open;
+ }
+
+ if (fsl_mc_interrupts_supported()) {
+ /*
+ * Create DPMCP for the DPRC's built-in portal:
+ */
+ error = dprc_create_dpmcp(mc_dev);
+ if (error < 0)
+ goto error_cleanup_open;
+ }
+
mutex_init(&mc_bus->scan_mutex);

/*
- * Discover MC objects in DPRC object:
+ * Discover MC objects in the DPRC object:
*/
error = dprc_scan_container(mc_dev);
if (error < 0)
- goto error_cleanup_open;
+ goto error_destroy_dpmcp;
+
+ if (fsl_mc_interrupts_supported()) {
+ /*
+ * The fsl_mc_device object associated with the DPMCP object
+ * created above was created as part of the
+ * dprc_scan_container() call above:
+ */
+ if (WARN_ON(!mc_dev->mc_io->dpmcp_dev)) {
+ error = -EINVAL;
+ goto error_cleanup_dprc_scan;
+ }
+
+ /*
+ * Configure interrupt for the DPMCP object associated with the
+ * DPRC object's built-in portal:
+ *
+ * NOTE: We have to do this after calling dprc_scan_container(),
+ * since dprc_scan_container() will populate the IRQ pool for
+ * this DPRC.
+ */
+ error = fsl_mc_io_setup_dpmcp_irq(mc_dev->mc_io);
+ if (error < 0)
+ goto error_cleanup_dprc_scan;

- if (mc->gic_supported) {
/*
* Configure interrupts for the DPRC object associated with
* this MC bus:
@@ -808,16 +893,23 @@ static int dprc_probe(struct fsl_mc_device *mc_dev)
return 0;

error_cleanup_dprc_scan:
+ fsl_mc_io_unset_dpmcp(mc_dev->mc_io);
device_for_each_child(&mc_dev->dev, NULL, __fsl_mc_device_remove);
dprc_cleanup_all_resource_pools(mc_dev);
- if (mc->gic_supported)
+ if (fsl_mc_interrupts_supported())
fsl_mc_cleanup_irq_pool(mc_bus);

+error_destroy_dpmcp:
+ dprc_destroy_dpmcp(mc_dev);
+
error_cleanup_open:
(void)dprc_close(mc_dev->mc_io, mc_dev->mc_handle);

error_cleanup_mc_io:
- fsl_destroy_mc_io(mc_dev->mc_io);
+ if (mc_io_created) {
+ fsl_destroy_mc_io(mc_dev->mc_io);
+ mc_dev->mc_io = NULL;
+ }
return error;
}

@@ -845,7 +937,6 @@ static int dprc_remove(struct fsl_mc_device *mc_dev)
{
int error;
struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_dev);
- struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);

if (WARN_ON(strcmp(mc_dev->obj_desc.type, "dprc") != 0))
return -EINVAL;
@@ -855,16 +946,18 @@ static int dprc_remove(struct fsl_mc_device *mc_dev)
if (WARN_ON(!mc_bus->irq_resources))
return -EINVAL;

- if (mc->gic_supported)
+ if (fsl_mc_interrupts_supported())
dprc_teardown_irqs(mc_dev);

+ fsl_mc_io_unset_dpmcp(mc_dev->mc_io);
device_for_each_child(&mc_dev->dev, NULL, __fsl_mc_device_remove);
dprc_cleanup_all_resource_pools(mc_dev);
+ dprc_destroy_dpmcp(mc_dev);
error = dprc_close(mc_dev->mc_io, mc_dev->mc_handle);
if (error < 0)
dev_err(&mc_dev->dev, "dprc_close() failed: %d\n", error);

- if (mc->gic_supported)
+ if (fsl_mc_interrupts_supported())
fsl_mc_cleanup_irq_pool(mc_bus);

dev_info(&mc_dev->dev, "DPRC device unbound from driver");
diff --git a/drivers/staging/fsl-mc/bus/mc-allocator.c b/drivers/staging/fsl-mc/bus/mc-allocator.c
index 313a533..d1bfa3a 100644
--- a/drivers/staging/fsl-mc/bus/mc-allocator.c
+++ b/drivers/staging/fsl-mc/bus/mc-allocator.c
@@ -111,7 +111,7 @@ static int __must_check fsl_mc_resource_pool_remove_device(struct fsl_mc_device
goto out;

resource = mc_dev->resource;
- if (WARN_ON(resource->data != mc_dev))
+ if (WARN_ON(!resource || resource->data != mc_dev))
goto out;

mc_bus_dev = to_fsl_mc_device(mc_dev->dev.parent);
@@ -285,7 +285,7 @@ int __must_check fsl_mc_portal_allocate(struct fsl_mc_device *mc_dev,
struct fsl_mc_bus *mc_bus;
phys_addr_t mc_portal_phys_addr;
size_t mc_portal_size;
- struct fsl_mc_device *mc_adev;
+ struct fsl_mc_device *dpmcp_dev;
int error = -EINVAL;
struct fsl_mc_resource *resource = NULL;
struct fsl_mc_io *mc_io = NULL;
@@ -305,23 +305,24 @@ int __must_check fsl_mc_portal_allocate(struct fsl_mc_device *mc_dev,
if (error < 0)
return error;

- mc_adev = resource->data;
- if (WARN_ON(!mc_adev))
+ dpmcp_dev = resource->data;
+ if (WARN_ON(!dpmcp_dev ||
+ strcmp(dpmcp_dev->obj_desc.type, "dpmcp") != 0))
goto error_cleanup_resource;

- if (WARN_ON(mc_adev->obj_desc.region_count == 0))
+ if (WARN_ON(dpmcp_dev->obj_desc.region_count == 0))
goto error_cleanup_resource;

- mc_portal_phys_addr = mc_adev->regions[0].start;
- mc_portal_size = mc_adev->regions[0].end -
- mc_adev->regions[0].start + 1;
+ mc_portal_phys_addr = dpmcp_dev->regions[0].start;
+ mc_portal_size = dpmcp_dev->regions[0].end -
+ dpmcp_dev->regions[0].start + 1;

if (WARN_ON(mc_portal_size != mc_bus_dev->mc_io->portal_size))
goto error_cleanup_resource;

error = fsl_create_mc_io(&mc_bus_dev->dev,
mc_portal_phys_addr,
- mc_portal_size, resource,
+ mc_portal_size, dpmcp_dev,
mc_io_flags, &mc_io);
if (error < 0)
goto error_cleanup_resource;
@@ -343,12 +344,26 @@ EXPORT_SYMBOL_GPL(fsl_mc_portal_allocate);
*/
void fsl_mc_portal_free(struct fsl_mc_io *mc_io)
{
+ struct fsl_mc_device *dpmcp_dev;
struct fsl_mc_resource *resource;

- resource = mc_io->resource;
- if (WARN_ON(resource->type != FSL_MC_POOL_DPMCP))
+ /*
+ * Every mc_io obtained by calling fsl_mc_portal_allocate() is supposed
+ * to have a DPMCP object associated with.
+ */
+ dpmcp_dev = mc_io->dpmcp_dev;
+ if (WARN_ON(!dpmcp_dev))
+ return;
+ if (WARN_ON(strcmp(dpmcp_dev->obj_desc.type, "dpmcp") != 0))
+ return;
+ if (WARN_ON(dpmcp_dev->mc_io != mc_io))
+ return;
+
+ resource = dpmcp_dev->resource;
+ if (WARN_ON(!resource || resource->type != FSL_MC_POOL_DPMCP))
return;
- if (WARN_ON(!resource->data))
+
+ if (WARN_ON(resource->data != dpmcp_dev))
return;

fsl_destroy_mc_io(mc_io);
@@ -364,31 +379,14 @@ EXPORT_SYMBOL_GPL(fsl_mc_portal_free);
int fsl_mc_portal_reset(struct fsl_mc_io *mc_io)
{
int error;
- uint16_t token;
- struct fsl_mc_resource *resource = mc_io->resource;
- struct fsl_mc_device *mc_dev = resource->data;
-
- if (WARN_ON(resource->type != FSL_MC_POOL_DPMCP))
- return -EINVAL;
+ struct fsl_mc_device *dpmcp_dev = mc_io->dpmcp_dev;

- if (WARN_ON(!mc_dev))
+ if (WARN_ON(!dpmcp_dev))
return -EINVAL;

- error = dpmcp_open(mc_io, mc_dev->obj_desc.id, &token);
+ error = dpmcp_reset(mc_io, dpmcp_dev->mc_handle);
if (error < 0) {
- dev_err(&mc_dev->dev, "dpmcp_open() failed: %d\n", error);
- return error;
- }
-
- error = dpmcp_reset(mc_io, token);
- if (error < 0) {
- dev_err(&mc_dev->dev, "dpmcp_reset() failed: %d\n", error);
- return error;
- }
-
- error = dpmcp_close(mc_io, token);
- if (error < 0) {
- dev_err(&mc_dev->dev, "dpmcp_close() failed: %d\n", error);
+ dev_err(&dpmcp_dev->dev, "dpmcp_reset() failed: %d\n", error);
return error;
}

@@ -610,13 +608,28 @@ static int fsl_mc_allocator_probe(struct fsl_mc_device *mc_dev)
goto error;

mc_bus = to_fsl_mc_bus(mc_bus_dev);
- error = object_type_to_pool_type(mc_dev->obj_desc.type, &pool_type);
- if (error < 0)
- goto error;

- error = fsl_mc_resource_pool_add_device(mc_bus, pool_type, mc_dev);
- if (error < 0)
- goto error;
+ /*
+ * If mc_dev is the DPMCP object for the parent DPRC's built-in
+ * portal, we don't add this DPMCP to the DPMCP object pool,
+ * but instead allocate it directly to the parent DPRC (mc_bus_dev):
+ */
+ if (strcmp(mc_dev->obj_desc.type, "dpmcp") == 0 &&
+ mc_dev->obj_desc.id == mc_bus->dprc_attr.portal_id) {
+ error = fsl_mc_io_set_dpmcp(mc_bus_dev->mc_io, mc_dev);
+ if (error < 0)
+ goto error;
+ } else {
+ error = object_type_to_pool_type(mc_dev->obj_desc.type,
+ &pool_type);
+ if (error < 0)
+ goto error;
+
+ error = fsl_mc_resource_pool_add_device(mc_bus, pool_type,
+ mc_dev);
+ if (error < 0)
+ goto error;
+ }

dev_dbg(&mc_dev->dev,
"Allocatable MC object device bound to fsl_mc_allocator driver");
@@ -632,20 +645,20 @@ error:
*/
static int fsl_mc_allocator_remove(struct fsl_mc_device *mc_dev)
{
- int error = -EINVAL;
+ int error;

if (WARN_ON(!FSL_MC_IS_ALLOCATABLE(mc_dev->obj_desc.type)))
- goto out;
+ return -EINVAL;

- error = fsl_mc_resource_pool_remove_device(mc_dev);
- if (error < 0)
- goto out;
+ if (mc_dev->resource) {
+ error = fsl_mc_resource_pool_remove_device(mc_dev);
+ if (error < 0)
+ return error;
+ }

dev_dbg(&mc_dev->dev,
"Allocatable MC object device unbound from fsl_mc_allocator driver");
- error = 0;
-out:
- return error;
+ return 0;
}

static const struct fsl_mc_device_match_id match_id_table[] = {
diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c
index be2faf6..aeed752 100644
--- a/drivers/staging/fsl-mc/bus/mc-bus.c
+++ b/drivers/staging/fsl-mc/bus/mc-bus.c
@@ -473,7 +473,13 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
}

device_initialize(&mc_dev->dev);
+
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
INIT_LIST_HEAD(&mc_dev->dev.msi_list);
+#endif
mc_dev->dev.parent = parent_dev;
mc_dev->dev.bus = &fsl_mc_bus_type;
dev_set_name(&mc_dev->dev, "%s.%d", obj_desc->type, obj_desc->id);
@@ -595,6 +601,11 @@ void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)

if (&mc_dev->dev == fsl_mc_bus_type.dev_root)
fsl_mc_bus_type.dev_root = NULL;
+ } else if (strcmp(mc_dev->obj_desc.type, "dpmcp") == 0) {
+ if (mc_dev->mc_io) {
+ fsl_destroy_mc_io(mc_dev->mc_io);
+ mc_dev->mc_io = NULL;
+ }
}

kfree(mc_dev->driver_override);
@@ -606,6 +617,10 @@ void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
}
EXPORT_SYMBOL_GPL(fsl_mc_device_remove);

+/*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
static int mc_bus_msi_prepare(struct irq_domain *domain, struct device *dev,
int nvec, msi_alloc_info_t *info)
{
@@ -670,19 +685,23 @@ static void mc_bus_msi_domain_write_msg(struct irq_data *irq_data,
struct fsl_mc_device_irq *irq_res =
&mc_bus->irq_resources[msi_entry->msi_attrib.entry_nr];

+ /*
+ * NOTE: This function is invoked with interrupts disabled
+ */
+
if (irq_res->irq_number == irq_data->irq) {
/*
* write msg->address_hi/lo to irq_resource
*/
irq_res->msi_paddr =
((u64)msg->address_hi << 32) | msg->address_lo;
+
irq_res->msi_value = msg->data;

/*
- * NOTE: We cannot do the actual programming of the MSI
- * in the MC, as this function is invoked in atomic context
- * (interrupts disabled) and we cannot reliably send MC commands
- * in atomic context.
+ * NOTE: We cannot do the actual programming of the MSI,
+ * in the MC, here, because calling dprc_obj_set_irq()
+ * is not reliable, as object indexes may change under us.
*/
}
}
@@ -749,6 +768,7 @@ cleanup_its_of_node:
of_node_put(its_of_node);
return error;
}
+#endif /* GIC_ITS_MC_SUPPORT */

/*
* Initialize the interrupt pool associated with a MC bus.
@@ -758,21 +778,26 @@ int __must_check fsl_mc_populate_irq_pool(struct fsl_mc_bus *mc_bus,
unsigned int irq_count)
{
unsigned int i;
- struct msi_desc *msi_entry;
- struct msi_desc *next_msi_entry;
struct fsl_mc_device_irq *irq_resources;
struct fsl_mc_device_irq *irq_res;
- int error;
struct fsl_mc_device *mc_bus_dev = &mc_bus->mc_dev;
- struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);
struct fsl_mc_resource_pool *res_pool =
&mc_bus->resource_pools[FSL_MC_POOL_IRQ];
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
+ int error;
+ struct msi_desc *msi_entry;
+ struct msi_desc *next_msi_entry;
+ struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);

/*
* Detect duplicate invocations of this function:
*/
if (WARN_ON(!list_empty(&mc_bus_dev->dev.msi_list)))
return -EINVAL;
+#endif

if (WARN_ON(irq_count == 0 ||
irq_count > FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS))
@@ -787,6 +812,10 @@ int __must_check fsl_mc_populate_irq_pool(struct fsl_mc_bus *mc_bus,

for (i = 0; i < irq_count; i++) {
irq_res = &irq_resources[i];
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
msi_entry = alloc_msi_entry(&mc_bus_dev->dev);
if (!msi_entry) {
dev_err(&mc_bus_dev->dev, "Failed to allocate msi entry\n");
@@ -799,6 +828,7 @@ int __must_check fsl_mc_populate_irq_pool(struct fsl_mc_bus *mc_bus,
msi_entry->msi_attrib.entry_nr = i;
msi_entry->nvec_used = 1;
list_add_tail(&msi_entry->list, &mc_bus_dev->dev.msi_list);
+#endif

/*
* NOTE: irq_res->msi_paddr will be set by the
@@ -812,6 +842,10 @@ int __must_check fsl_mc_populate_irq_pool(struct fsl_mc_bus *mc_bus,
}

/*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
+ /*
* NOTE: Calling this function will trigger the invocation of the
* mc_bus_msi_prepare() callback
*/
@@ -822,7 +856,12 @@ int __must_check fsl_mc_populate_irq_pool(struct fsl_mc_bus *mc_bus,
dev_err(&mc_bus_dev->dev, "Failed to allocate IRQs\n");
goto cleanup_msi_entries;
}
+#endif

+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
for_each_msi_entry(msi_entry, &mc_bus_dev->dev) {
u32 irq_num = msi_entry->irq;

@@ -830,12 +869,17 @@ int __must_check fsl_mc_populate_irq_pool(struct fsl_mc_bus *mc_bus,
irq_res->irq_number = irq_num;
irq_res->resource.id = irq_num;
}
+#endif

res_pool->max_count = irq_count;
res_pool->free_count = irq_count;
mc_bus->irq_resources = irq_resources;
return 0;

+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
cleanup_msi_entries:
list_for_each_entry_safe(msi_entry, next_msi_entry,
&mc_bus_dev->dev.msi_list, list)
@@ -843,6 +887,7 @@ cleanup_msi_entries:

devm_kfree(&mc_bus_dev->dev, irq_resources);
return error;
+#endif
}
EXPORT_SYMBOL_GPL(fsl_mc_populate_irq_pool);

@@ -852,9 +897,14 @@ EXPORT_SYMBOL_GPL(fsl_mc_populate_irq_pool);
*/
void fsl_mc_cleanup_irq_pool(struct fsl_mc_bus *mc_bus)
{
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
struct msi_desc *msi_entry;
struct msi_desc *next_msi_entry;
struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);
+#endif
struct fsl_mc_resource_pool *res_pool =
&mc_bus->resource_pools[FSL_MC_POOL_IRQ];

@@ -867,10 +917,15 @@ void fsl_mc_cleanup_irq_pool(struct fsl_mc_bus *mc_bus)
if (WARN_ON(res_pool->free_count != res_pool->max_count))
return;

+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
msi_domain_free_irqs(mc->irq_domain, &mc_bus->mc_dev.dev);
list_for_each_entry_safe(msi_entry, next_msi_entry,
&mc_bus->mc_dev.dev.msi_list, list)
kfree(msi_entry);
+#endif

devm_kfree(&mc_bus->mc_dev.dev, mc_bus->irq_resources);
res_pool->max_count = 0;
@@ -1010,6 +1065,11 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
return -ENOMEM;

platform_set_drvdata(pdev, mc);
+
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
error = create_mc_irq_domain(pdev, &mc->irq_domain);
if (error < 0) {
dev_warn(&pdev->dev,
@@ -1017,6 +1077,7 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
} else {
mc->gic_supported = true;
}
+#endif

/*
* Get physical address of MC portal for the root DPRC:
@@ -1094,7 +1155,14 @@ error_cleanup_mc_io:
fsl_destroy_mc_io(mc_io);

error_cleanup_irq_domain:
- irq_domain_remove(mc->irq_domain);
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
+ if (mc->gic_supported)
+ irq_domain_remove(mc->irq_domain);
+#endif
+
return error;
}

@@ -1109,7 +1177,14 @@ static int fsl_mc_bus_remove(struct platform_device *pdev)
if (WARN_ON(&mc->root_mc_bus_dev->dev != fsl_mc_bus_type.dev_root))
return -EINVAL;

- irq_domain_remove(mc->irq_domain);
+ /*
+ * FIXME: Enable this code when the GIC-ITS MC support patch is merged
+ */
+#ifdef GIC_ITS_MC_SUPPORT
+ if (mc->gic_supported)
+ irq_domain_remove(mc->irq_domain);
+#endif
+
fsl_mc_device_remove(mc->root_mc_bus_dev);
dev_info(&pdev->dev, "Root MC bus device removed");
return 0;
diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c b/drivers/staging/fsl-mc/bus/mc-sys.c
index 1679054..4628746 100644
--- a/drivers/staging/fsl-mc/bus/mc-sys.c
+++ b/drivers/staging/fsl-mc/bus/mc-sys.c
@@ -34,10 +34,13 @@

#include "../include/mc-sys.h"
#include "../include/mc-cmd.h"
+#include "../include/mc.h"
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/ioport.h>
#include <linux/device.h>
+#include <linux/interrupt.h>
+#include "dpmcp.h"

/**
* Timeout in jiffies to wait for the completion of an MC command
@@ -55,6 +58,230 @@
((uint16_t)mc_dec((_hdr), MC_CMD_HDR_CMDID_O, MC_CMD_HDR_CMDID_S))

/**
+ * dpmcp_irq0_handler - Regular ISR for DPMCP interrupt 0
+ *
+ * @irq: IRQ number of the interrupt being handled
+ * @arg: Pointer to device structure
+ */
+static irqreturn_t dpmcp_irq0_handler(int irq_num, void *arg)
+{
+ struct device *dev = (struct device *)arg;
+ struct fsl_mc_device *dpmcp_dev = to_fsl_mc_device(dev);
+
+ dev_dbg(dev, "DPMCP IRQ %d triggered on CPU %u\n", irq_num,
+ smp_processor_id());
+
+ if (WARN_ON(dpmcp_dev->irqs[0]->irq_number != (uint32_t)irq_num))
+ goto out;
+
+ if (WARN_ON(!dpmcp_dev->mc_io))
+ goto out;
+
+ /*
+ * NOTE: We cannot invoke MC flib function here
+ */
+
+ complete(&dpmcp_dev->mc_io->mc_command_done_completion);
+out:
+ return IRQ_HANDLED;
+}
+
+/*
+ * Disable and clear interrupts for a given DPMCP object
+ */
+static int disable_dpmcp_irq(struct fsl_mc_device *dpmcp_dev)
+{
+ int error;
+
+ /*
+ * Disable generation of the DPMCP interrupt:
+ */
+ error = dpmcp_set_irq_enable(dpmcp_dev->mc_io,
+ dpmcp_dev->mc_handle,
+ DPMCP_IRQ_INDEX, 0);
+ if (error < 0) {
+ dev_err(&dpmcp_dev->dev,
+ "dpmcp_set_irq_enable() failed: %d\n", error);
+
+ return error;
+ }
+
+ /*
+ * Disable all DPMCP interrupt causes:
+ */
+ error = dpmcp_set_irq_mask(dpmcp_dev->mc_io, dpmcp_dev->mc_handle,
+ DPMCP_IRQ_INDEX, 0x0);
+ if (error < 0) {
+ dev_err(&dpmcp_dev->dev,
+ "dpmcp_set_irq_mask() failed: %d\n", error);
+
+ return error;
+ }
+
+ /*
+ * Clear any leftover interrupts:
+ */
+ error = dpmcp_clear_irq_status(dpmcp_dev->mc_io, dpmcp_dev->mc_handle,
+ DPMCP_IRQ_INDEX, ~0x0U);
+ if (error < 0) {
+ dev_err(&dpmcp_dev->dev,
+ "dpmcp_clear_irq_status() failed: %d\n",
+ error);
+ return error;
+ }
+
+ return 0;
+}
+
+static void unregister_dpmcp_irq_handler(struct fsl_mc_device *dpmcp_dev)
+{
+ struct fsl_mc_device_irq *irq = dpmcp_dev->irqs[DPMCP_IRQ_INDEX];
+
+ devm_free_irq(&dpmcp_dev->dev, irq->irq_number, &dpmcp_dev->dev);
+}
+
+static int register_dpmcp_irq_handler(struct fsl_mc_device *dpmcp_dev)
+{
+ int error;
+ struct fsl_mc_device_irq *irq = dpmcp_dev->irqs[DPMCP_IRQ_INDEX];
+
+ error = devm_request_irq(&dpmcp_dev->dev,
+ irq->irq_number,
+ dpmcp_irq0_handler,
+ IRQF_NO_SUSPEND | IRQF_ONESHOT,
+ "FSL MC DPMCP irq0",
+ &dpmcp_dev->dev);
+ if (error < 0) {
+ dev_err(&dpmcp_dev->dev,
+ "devm_request_irq() failed: %d\n",
+ error);
+ return error;
+ }
+
+ error = dpmcp_set_irq(dpmcp_dev->mc_io,
+ dpmcp_dev->mc_handle,
+ DPMCP_IRQ_INDEX,
+ irq->msi_paddr,
+ irq->msi_value,
+ irq->irq_number);
+ if (error < 0) {
+ dev_err(&dpmcp_dev->dev,
+ "dpmcp_set_irq() failed: %d\n", error);
+ goto error_unregister_irq_handler;
+ }
+
+ return 0;
+
+error_unregister_irq_handler:
+ devm_free_irq(&dpmcp_dev->dev, irq->irq_number, &dpmcp_dev->dev);
+ return error;
+}
+
+static int enable_dpmcp_irq(struct fsl_mc_device *dpmcp_dev)
+{
+ int error;
+
+ /*
+ * Enable MC command completion event to trigger DPMCP interrupt:
+ */
+ error = dpmcp_set_irq_mask(dpmcp_dev->mc_io,
+ dpmcp_dev->mc_handle,
+ DPMCP_IRQ_INDEX,
+ DPMCP_IRQ_EVENT_CMD_DONE);
+ if (error < 0) {
+ dev_err(&dpmcp_dev->dev,
+ "dpmcp_set_irq_mask() failed: %d\n", error);
+
+ return error;
+ }
+
+ /*
+ * Enable generation of the interrupt:
+ */
+ error = dpmcp_set_irq_enable(dpmcp_dev->mc_io,
+ dpmcp_dev->mc_handle,
+ DPMCP_IRQ_INDEX, 1);
+ if (error < 0) {
+ dev_err(&dpmcp_dev->dev,
+ "dpmcp_set_irq_enable() failed: %d\n", error);
+
+ return error;
+ }
+
+ return 0;
+}
+
+/*
+ * Setup MC command completion interrupt for the DPMCP device associated with a
+ * given fsl_mc_io object
+ */
+int fsl_mc_io_setup_dpmcp_irq(struct fsl_mc_io *mc_io)
+{
+ int error;
+ struct fsl_mc_device *dpmcp_dev = mc_io->dpmcp_dev;
+
+ if (WARN_ON(!dpmcp_dev))
+ return -EINVAL;
+
+ if (WARN_ON(!fsl_mc_interrupts_supported()))
+ return -EINVAL;
+
+ if (WARN_ON(dpmcp_dev->obj_desc.irq_count != 1))
+ return -EINVAL;
+
+ if (WARN_ON(!dpmcp_dev->mc_io))
+ return -EINVAL;
+
+ error = fsl_mc_allocate_irqs(dpmcp_dev);
+ if (error < 0)
+ return error;
+
+ error = disable_dpmcp_irq(dpmcp_dev);
+ if (error < 0)
+ goto error_free_irqs;
+
+ error = register_dpmcp_irq_handler(dpmcp_dev);
+ if (error < 0)
+ goto error_free_irqs;
+
+ error = enable_dpmcp_irq(dpmcp_dev);
+ if (error < 0)
+ goto error_unregister_irq_handler;
+
+ mc_io->mc_command_done_irq_armed = true;
+ return 0;
+
+error_unregister_irq_handler:
+ unregister_dpmcp_irq_handler(dpmcp_dev);
+
+error_free_irqs:
+ fsl_mc_free_irqs(dpmcp_dev);
+ return error;
+}
+EXPORT_SYMBOL_GPL(fsl_mc_io_setup_dpmcp_irq);
+
+/*
+ * Tear down interrupts for the DPMCP device associated with a given fsl_mc_io
+ * object
+ */
+static void teardown_dpmcp_irq(struct fsl_mc_io *mc_io)
+{
+ struct fsl_mc_device *dpmcp_dev = mc_io->dpmcp_dev;
+
+ if (WARN_ON(!dpmcp_dev))
+ return;
+ if (WARN_ON(!fsl_mc_interrupts_supported()))
+ return;
+ if (WARN_ON(!dpmcp_dev->irqs))
+ return;
+
+ mc_io->mc_command_done_irq_armed = false;
+ (void)disable_dpmcp_irq(dpmcp_dev);
+ unregister_dpmcp_irq_handler(dpmcp_dev);
+ fsl_mc_free_irqs(dpmcp_dev);
+}
+
+/**
* Creates an MC I/O object
*
* @dev: device to be associated with the MC I/O object
@@ -70,9 +297,10 @@
int __must_check fsl_create_mc_io(struct device *dev,
phys_addr_t mc_portal_phys_addr,
uint32_t mc_portal_size,
- struct fsl_mc_resource *resource,
+ struct fsl_mc_device *dpmcp_dev,
uint32_t flags, struct fsl_mc_io **new_mc_io)
{
+ int error;
struct fsl_mc_io *mc_io;
void __iomem *mc_portal_virt_addr;
struct resource *res;
@@ -85,11 +313,13 @@ int __must_check fsl_create_mc_io(struct device *dev,
mc_io->flags = flags;
mc_io->portal_phys_addr = mc_portal_phys_addr;
mc_io->portal_size = mc_portal_size;
- mc_io->resource = resource;
- if (flags & FSL_MC_IO_ATOMIC_CONTEXT_PORTAL)
+ mc_io->mc_command_done_irq_armed = false;
+ if (flags & FSL_MC_IO_ATOMIC_CONTEXT_PORTAL) {
spin_lock_init(&mc_io->spinlock);
- else
+ } else {
mutex_init(&mc_io->mutex);
+ init_completion(&mc_io->mc_command_done_completion);
+ }

res = devm_request_mem_region(dev,
mc_portal_phys_addr,
@@ -113,8 +343,26 @@ int __must_check fsl_create_mc_io(struct device *dev,
}

mc_io->portal_virt_addr = mc_portal_virt_addr;
+ if (dpmcp_dev) {
+ error = fsl_mc_io_set_dpmcp(mc_io, dpmcp_dev);
+ if (error < 0)
+ goto error_destroy_mc_io;
+
+ if (!(flags & FSL_MC_IO_ATOMIC_CONTEXT_PORTAL) &&
+ fsl_mc_interrupts_supported()) {
+ error = fsl_mc_io_setup_dpmcp_irq(mc_io);
+ if (error < 0)
+ goto error_destroy_mc_io;
+ }
+ }
+
*new_mc_io = mc_io;
return 0;
+
+error_destroy_mc_io:
+ fsl_destroy_mc_io(mc_io);
+ return error;
+
}
EXPORT_SYMBOL_GPL(fsl_create_mc_io);

@@ -125,6 +373,11 @@ EXPORT_SYMBOL_GPL(fsl_create_mc_io);
*/
void fsl_destroy_mc_io(struct fsl_mc_io *mc_io)
{
+ struct fsl_mc_device *dpmcp_dev = mc_io->dpmcp_dev;
+
+ if (dpmcp_dev)
+ fsl_mc_io_unset_dpmcp(mc_io);
+
devm_iounmap(mc_io->dev, mc_io->portal_virt_addr);
devm_release_mem_region(mc_io->dev,
mc_io->portal_phys_addr,
@@ -135,6 +388,60 @@ void fsl_destroy_mc_io(struct fsl_mc_io *mc_io)
}
EXPORT_SYMBOL_GPL(fsl_destroy_mc_io);

+int fsl_mc_io_set_dpmcp(struct fsl_mc_io *mc_io,
+ struct fsl_mc_device *dpmcp_dev)
+{
+ int error;
+
+ if (WARN_ON(!dpmcp_dev))
+ return -EINVAL;
+
+ if (WARN_ON(mc_io->dpmcp_dev))
+ return -EINVAL;
+
+ if (WARN_ON(dpmcp_dev->mc_io))
+ return -EINVAL;
+
+ if (!(mc_io->flags & FSL_MC_IO_ATOMIC_CONTEXT_PORTAL)) {
+ error = dpmcp_open(mc_io, dpmcp_dev->obj_desc.id,
+ &dpmcp_dev->mc_handle);
+ if (error < 0)
+ return error;
+ }
+
+ mc_io->dpmcp_dev = dpmcp_dev;
+ dpmcp_dev->mc_io = mc_io;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(fsl_mc_io_set_dpmcp);
+
+void fsl_mc_io_unset_dpmcp(struct fsl_mc_io *mc_io)
+{
+ int error;
+ struct fsl_mc_device *dpmcp_dev = mc_io->dpmcp_dev;
+
+ if (WARN_ON(!dpmcp_dev))
+ return;
+
+ if (WARN_ON(dpmcp_dev->mc_io != mc_io))
+ return;
+
+ if (!(mc_io->flags & FSL_MC_IO_ATOMIC_CONTEXT_PORTAL)) {
+ if (dpmcp_dev->irqs)
+ teardown_dpmcp_irq(mc_io);
+
+ error = dpmcp_close(mc_io, dpmcp_dev->mc_handle);
+ if (error < 0) {
+ dev_err(&dpmcp_dev->dev, "dpmcp_close() failed: %d\n",
+ error);
+ }
+ }
+
+ mc_io->dpmcp_dev = NULL;
+ dpmcp_dev->mc_io = NULL;
+}
+EXPORT_SYMBOL_GPL(fsl_mc_io_unset_dpmcp);
+
static int mc_status_to_error(enum mc_cmd_status status)
{
static const int mc_status_to_error_map[] = {
@@ -228,8 +535,68 @@ static inline enum mc_cmd_status mc_read_response(struct mc_command __iomem *
return status;
}

+static int mc_completion_wait(struct fsl_mc_io *mc_io, struct mc_command *cmd,
+ enum mc_cmd_status *mc_status)
+{
+ enum mc_cmd_status status;
+ unsigned long jiffies_left;
+
+ if (WARN_ON(!mc_io->dpmcp_dev))
+ return -EINVAL;
+
+ for (;;) {
+ status = mc_read_response(mc_io->portal_virt_addr, cmd);
+ if (status != MC_CMD_STATUS_READY)
+ break;
+
+ jiffies_left = wait_for_completion_timeout(
+ &mc_io->mc_command_done_completion,
+ MC_CMD_COMPLETION_TIMEOUT_JIFFIES);
+ if (jiffies_left == 0)
+ return -ETIMEDOUT;
+ }
+
+ *mc_status = status;
+ return 0;
+}
+
+static int mc_polling_wait(struct fsl_mc_io *mc_io, struct mc_command *cmd,
+ enum mc_cmd_status *mc_status)
+{
+ enum mc_cmd_status status;
+ unsigned long jiffies_until_timeout =
+ jiffies + MC_CMD_COMPLETION_TIMEOUT_JIFFIES;
+
+ for (;;) {
+ status = mc_read_response(mc_io->portal_virt_addr, cmd);
+ if (status != MC_CMD_STATUS_READY)
+ break;
+
+ if (preemptible()) {
+ usleep_range(MC_CMD_COMPLETION_POLLING_MIN_SLEEP_USECS,
+ MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS);
+ } else {
+ udelay(MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS);
+ }
+
+ if (time_after_eq(jiffies, jiffies_until_timeout)) {
+ pr_debug("MC command timed out (portal: %#llx, obj handle: %#x, command: %#x)\n",
+ mc_io->portal_phys_addr,
+ (unsigned int)
+ MC_CMD_HDR_READ_TOKEN(cmd->header),
+ (unsigned int)
+ MC_CMD_HDR_READ_CMDID(cmd->header));
+
+ return -ETIMEDOUT;
+ }
+ }
+
+ *mc_status = status;
+ return 0;
+}
+
/**
- * Sends an command to the MC device using the given MC I/O object
+ * Sends a command to the MC device using the given MC I/O object
*
* @mc_io: MC I/O object to be used
* @cmd: command to be sent
@@ -240,8 +607,10 @@ int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
{
int error;
enum mc_cmd_status status;
- unsigned long jiffies_until_timeout =
- jiffies + MC_CMD_COMPLETION_TIMEOUT_JIFFIES;
+
+ /*
+ * NOTE: This function may be invoked from atomic context
+ */

if (preemptible()) {
if (WARN_ON(mc_io->flags & FSL_MC_IO_ATOMIC_CONTEXT_PORTAL))
@@ -263,32 +632,13 @@ int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
/*
* Wait for response from the MC hardware:
*/
- for (;;) {
- status = mc_read_response(mc_io->portal_virt_addr, cmd);
- if (status != MC_CMD_STATUS_READY)
- break;
-
- /*
- * TODO: When MC command completion interrupts are supported
- * call wait function here instead of usleep_range()
- */
- if (preemptible()) {
- usleep_range(MC_CMD_COMPLETION_POLLING_MIN_SLEEP_USECS,
- MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS);
- }
-
- if (time_after_eq(jiffies, jiffies_until_timeout)) {
- pr_debug("MC command timed out (portal: %#llx, obj handle: %#x, command: %#x)\n",
- mc_io->portal_phys_addr,
- (unsigned int)
- MC_CMD_HDR_READ_TOKEN(cmd->header),
- (unsigned int)
- MC_CMD_HDR_READ_CMDID(cmd->header));
+ if (mc_io->mc_command_done_irq_armed && preemptible())
+ error = mc_completion_wait(mc_io, cmd, &status);
+ else
+ error = mc_polling_wait(mc_io, cmd, &status);

- error = -ETIMEDOUT;
- goto common_exit;
- }
- }
+ if (error < 0)
+ goto common_exit;

if (status != MC_CMD_STATUS_OK) {
pr_debug("MC command failed: portal: %#llx, obj handle: %#x, command: %#x, status: %s (%#x)\n",
diff --git a/drivers/staging/fsl-mc/include/mc-private.h b/drivers/staging/fsl-mc/include/mc-private.h
index 0757258..90b059a 100644
--- a/drivers/staging/fsl-mc/include/mc-private.h
+++ b/drivers/staging/fsl-mc/include/mc-private.h
@@ -111,12 +111,14 @@ struct fsl_mc_resource_pool {
* from the physical DPRC.
* @irq_resources: Pointer to array of IRQ objects for the IRQ pool.
* @scan_mutex: Serializes bus scanning
+ * @dprc_attr: DPRC attributes
*/
struct fsl_mc_bus {
struct fsl_mc_device mc_dev;
struct fsl_mc_resource_pool resource_pools[FSL_MC_NUM_POOL_TYPES];
struct fsl_mc_device_irq *irq_resources;
struct mutex scan_mutex; /* serializes bus scanning */
+ struct dprc_attributes dprc_attr;
};

#define to_fsl_mc_bus(_mc_dev) \
@@ -134,10 +136,6 @@ int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev,
const char *driver_override,
unsigned int *total_irq_count);

-int dprc_lookup_object(struct fsl_mc_device *mc_bus_dev,
- struct fsl_mc_device *child_dev,
- uint32_t *child_obj_index);
-
int __init dprc_driver_init(void);

void __exit dprc_driver_exit(void);
diff --git a/drivers/staging/fsl-mc/include/mc-sys.h b/drivers/staging/fsl-mc/include/mc-sys.h
index 24e51f7..6234425 100644
--- a/drivers/staging/fsl-mc/include/mc-sys.h
+++ b/drivers/staging/fsl-mc/include/mc-sys.h
@@ -39,6 +39,7 @@
#include <linux/errno.h>
#include <linux/io.h>
#include <linux/dma-mapping.h>
+#include <linux/completion.h>
#include <linux/mutex.h>
#include <linux/spinlock.h>

@@ -57,9 +58,11 @@ struct mc_command;
* @portal_size: MC command portal size in bytes
* @portal_phys_addr: MC command portal physical address
* @portal_virt_addr: MC command portal virtual address
- * @resource: generic resource associated with the MC portal if
- * the MC portal came from a resource pool, or NULL if the MC portal
- * is permanently bound to a device (e.g., a DPRC)
+ * @dpmcp_dev: pointer to the DPMCP device associated with the MC portal.
+ * @mc_command_done_irq_armed: Flag indicating that the MC command done IRQ
+ * is currently armed.
+ * @mc_command_done_completion: Completion variable to be signaled when an MC
+ * command sent to the MC fw is completed.
* @mutex: Mutex to serialize mc_send_command() calls that use the same MC
* portal, if the fsl_mc_io object was created with the
* FSL_MC_IO_ATOMIC_CONTEXT_PORTAL flag off. mc_send_command() calls for this
@@ -75,21 +78,41 @@ struct fsl_mc_io {
uint16_t portal_size;
phys_addr_t portal_phys_addr;
void __iomem *portal_virt_addr;
- struct fsl_mc_resource *resource;
+ struct fsl_mc_device *dpmcp_dev;
union {
- struct mutex mutex; /* serializes mc_send_command() calls */
- spinlock_t spinlock; /* serializes mc_send_command() calls */
+ /*
+ * These fields are only meaningful if the
+ * FSL_MC_IO_ATOMIC_CONTEXT_PORTAL flag is not set
+ */
+ struct {
+ struct mutex mutex; /* serializes mc_send_command() */
+ struct completion mc_command_done_completion;
+ bool mc_command_done_irq_armed;
+ };
+
+ /*
+ * This field is only meaningful if the
+ * FSL_MC_IO_ATOMIC_CONTEXT_PORTAL flag is set
+ */
+ spinlock_t spinlock; /* serializes mc_send_command() */
};
};

int __must_check fsl_create_mc_io(struct device *dev,
phys_addr_t mc_portal_phys_addr,
uint32_t mc_portal_size,
- struct fsl_mc_resource *resource,
+ struct fsl_mc_device *dpmcp_dev,
uint32_t flags, struct fsl_mc_io **new_mc_io);

void fsl_destroy_mc_io(struct fsl_mc_io *mc_io);

+int fsl_mc_io_set_dpmcp(struct fsl_mc_io *mc_io,
+ struct fsl_mc_device *dpmcp_dev);
+
+void fsl_mc_io_unset_dpmcp(struct fsl_mc_io *mc_io);
+
+int fsl_mc_io_setup_dpmcp_irq(struct fsl_mc_io *mc_io);
+
int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd);

#endif /* _FSL_MC_SYS_H */
--
2.3.3

2015-04-30 11:50:23

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

On Tue, Apr 28, 2015 at 12:39:04PM -0500, J. German Rivera wrote:
> Change-Id: I2a986c465989c3811de19cfe9ed0b77168250cb1
> Reviewed-on: http://git.am.freescale.net:8181/33626
> Tested-by: Review Code-CDREVIEW <[email protected]>

These things are totally useless to the rest of us. Don't add them.


> diff --git a/drivers/staging/fsl-mc/TODO b/drivers/staging/fsl-mc/TODO
> index d78288b..1b8868d 100644
> --- a/drivers/staging/fsl-mc/TODO
> +++ b/drivers/staging/fsl-mc/TODO
> @@ -8,6 +8,9 @@
> * Add at least one device driver for a DPAA2 object (child device of the
> fsl-mc bus).
>
> +* Enable code blocks guarded by #ifdef GIC_ITS_MC_SUPPORT, when GIC-ITS
> + support for the MC MSIs gets merged.
> +

When will that be? I'd really prefer to not add these ifdeffed bits at
all.

> + if (status & (DPRC_IRQ_EVENT_OBJ_ADDED |
> + DPRC_IRQ_EVENT_OBJ_REMOVED |
> + DPRC_IRQ_EVENT_CONTAINER_DESTROYED |
> + DPRC_IRQ_EVENT_OBJ_DESTROYED |
> + DPRC_IRQ_EVENT_OBJ_CREATED)) {
> + unsigned int irq_count;
> +
> + error = dprc_scan_objects(mc_dev, &irq_count);
> + if (error < 0) {
> + dev_err(dev, "dprc_scan_objects() failed: %d\n", error);
> + goto out;
> + }
> +
> + WARN_ON((int16_t)irq_count < 0);

This code is doing "WARN_ON(test_bit(15, (unsigned long *)&irq_count));".
That seems like nonsense. Anyway, just delete the WARN_ON().

> +
> + if ((int16_t)irq_count >
> + mc_bus->resource_pools[FSL_MC_POOL_IRQ].max_count) {

Why are we casting this? Also can you align it like:

if (irq_count >
mc_bus->resource_pools[FSL_MC_POOL_IRQ].max_count) {

[tab][tab][space][space][space][space]mc_bus->resource_pools[

That way you can tell the if condition from the indented block. Plus
that is standard kernel indenting style these days.


[ snip ]

> + irqs = devm_kzalloc(&mc_dev->dev, irq_count * sizeof(irqs[0]),
> + GFP_KERNEL);
> + if (!irqs) {
> + error = -ENOMEM;
> + dev_err(&mc_dev->dev, "No memory to allocate irqs[]\n");
> + goto error;

I kind of hate One Err Style error handling, because the error labels
are so general... You can never guess the point of it until you scroll
down to read what "goto error;" does. The error handling here calls
devm_kfree() which is not needed... devm_ functions automatically
clean up after themselves. This seems a pattern throughout. Do a
search for devm_free() and see which ones are really needed or not.

Also the error message isn't needed here. kzalloc() has its own better
error messages built-in. Adding these error messages which will never
be printed is just a waste of RAM.

In other words this should look like:

irqs = devm_kcalloc(&mc_dev->dev, sizeof(*irqs), irq_count,
GFP_KERNEL);
if (!irqs)
return -ENOMEM;

regards,
dan carpenter

2015-04-30 12:13:26

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 4/7] staging: fsl-mc: Upgraded MC bus driver to match MC fw 7.0.0

On Tue, Apr 28, 2015 at 12:39:07PM -0500, J. German Rivera wrote:
> - Migrated MC bus driver to use DPRC flib 0.6.

What does this mean? What is a flib?

After reading the patch, apparently it means that we can remove all the
ifdefs from patch 1. :)

> - Changed IRQ setup infrastructure to be able to program MSIs
> for MC objects in an object-independent way.

Are these two things related?

Why does this patch add so many new EXPORTS()s?

regards,
dan carpenter

2015-04-30 12:59:31

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 6/7] staging: fsl-mc: Add locking to serialize mc_send_command() calls

On Tue, Apr 28, 2015 at 12:39:09PM -0500, J. German Rivera wrote:
> @@ -230,15 +235,26 @@ static inline enum mc_cmd_status mc_read_response(struct mc_command __iomem *
> * @cmd: command to be sent
> *
> * Returns '0' on Success; Error code otherwise.
> - *
> - * NOTE: This function cannot be invoked from from atomic contexts.
> */
> int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
> {
> + int error;
> enum mc_cmd_status status;
> unsigned long jiffies_until_timeout =
> jiffies + MC_CMD_COMPLETION_TIMEOUT_JIFFIES;

We busy loop while holding a spinlock for half a second. That seems
bad.

>
> + if (preemptible()) {

This is wrong. If the user asked for spinlocks they should always
get spinlocks. It shouldn't matter that they are not currently holding
a different lock.

I'm skeptical of this locking anyway.

Also what about if they have PREEMPT disabled? There aren't any users
for this stuff anyway so it's impossible to review how people are
FSL_MC_IO_ATOMIC_CONTEXT_PORTAL.

Let's wait until there is a user before looking at this.

> - return mc_status_to_error(status);
> + error = mc_status_to_error(status);
> + goto common_exit;
> }
>
> - return 0;
> + error = 0;
> +
> +common_exit:

Just name this unlock:.

> + if (preemptible())
> + mutex_unlock(&mc_io->mutex);
> + else
> + spin_unlock(&mc_io->spinlock);
> +
> + return error;
> }

regards,
dan carpenter

2015-04-30 13:01:43

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 7/7] staging: fsl-mc: Use DPMCP IRQ and completion var to wait for MC

On Tue, Apr 28, 2015 at 12:39:10PM -0500, J. German Rivera wrote:
> /**
> - * dprc_lookup_object - Finds a given MC object in a DPRC and returns
> - * the index of the object in the DPRC
> - *
> - * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
> - * @child_dev: pointer to the fsl-mc device to be looked up
> - * @child_obj_index: output parameter to hold the index of the object
> - */
> -int dprc_lookup_object(struct fsl_mc_device *mc_bus_dev,
> - struct fsl_mc_device *child_dev,
> - uint32_t *child_obj_index)

This patchset is kind of jumbled up. We introduced this function in
patch 4. There were never any users. Now we are deleting it in patch
7.

regards,
dan carpenter

2015-05-04 22:09:22

by J. German Rivera

[permalink] [raw]
Subject: RE: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

Dan,

Thanks for your comments. My replies inline.

Thanks,

German

> -----Original Message-----
> From: Dan Carpenter [mailto:[email protected]]
> Sent: Thursday, April 30, 2015 6:50 AM
> To: Rivera Jose-B46482
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Yoder Stuart-
> B08248; Sharma Bhupesh-B45370; [email protected]; Hamciuc Bogdan-BHAMCIU1;
> Erez Nir-RM30794; katz Itai-RM05202; Wood Scott-B07421; Marginean
> Alexandru-R89243; Schmitt Richard-B43082
> Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
>
> On Tue, Apr 28, 2015 at 12:39:04PM -0500, J. German Rivera wrote:
> > Change-Id: I2a986c465989c3811de19cfe9ed0b77168250cb1
> > Reviewed-on: http://git.am.freescale.net:8181/33626
> > Tested-by: Review Code-CDREVIEW <[email protected]>
>
> These things are totally useless to the rest of us. Don't add them.
>
Agreed. I'll remove them in the next respin.

>
> > diff --git a/drivers/staging/fsl-mc/TODO b/drivers/staging/fsl-mc/TODO
> > index d78288b..1b8868d 100644
> > --- a/drivers/staging/fsl-mc/TODO
> > +++ b/drivers/staging/fsl-mc/TODO
> > @@ -8,6 +8,9 @@
> > * Add at least one device driver for a DPAA2 object (child device of
> the
> > fsl-mc bus).
> >
> > +* Enable code blocks guarded by #ifdef GIC_ITS_MC_SUPPORT, when
> > +GIC-ITS
> > + support for the MC MSIs gets merged.
> > +
>
> When will that be? I'd really prefer to not add these ifdeffed bits at
> all.
>
The owner of the GIC-ITS support patches would need to answer the "when"
question. These #ifdefs are needed to be able to make the code compile
without the GIC-ITS support in place. Of course, the code will not be moved
out of staging with these #ifdefs. There is already an item for this
in the drivers/staging/fsl-mc/TODO file.

> > + if (status & (DPRC_IRQ_EVENT_OBJ_ADDED |
> > + DPRC_IRQ_EVENT_OBJ_REMOVED |
> > + DPRC_IRQ_EVENT_CONTAINER_DESTROYED |
> > + DPRC_IRQ_EVENT_OBJ_DESTROYED |
> > + DPRC_IRQ_EVENT_OBJ_CREATED)) {
> > + unsigned int irq_count;
> > +
> > + error = dprc_scan_objects(mc_dev, &irq_count);
> > + if (error < 0) {
> > + dev_err(dev, "dprc_scan_objects() failed: %d\n",
> error);
> > + goto out;
> > + }
> > +
> > + WARN_ON((int16_t)irq_count < 0);
>
> This code is doing "WARN_ON(test_bit(15, (unsigned long *)&irq_count));".
> That seems like nonsense. Anyway, just delete the WARN_ON().
>
I disagree. This WARN_ON is checking that irq_count is in the expected range
(it fits in int16_t as a positive number). The dprc_scan_objects() function
expects irq_count to be of type "unsigned int" (which is 32-bit unsigned)

> > +
> > + if ((int16_t)irq_count >
> > + mc_bus->resource_pools[FSL_MC_POOL_IRQ].max_count) {
>
> Why are we casting this? Also can you align it like:
>
This casting is done for safety, to prevent the comparison to be done
in "unsigned int" due to integer promotion rules.

> if (irq_count >
> mc_bus->resource_pools[FSL_MC_POOL_IRQ].max_count) {
>
> [tab][tab][space][space][space][space]mc_bus->resource_pools[
>
> That way you can tell the if condition from the indented block. Plus
> that is standard kernel indenting style these days.
>
Agreed. I'll change this in the next respin.

>
> [ snip ]
>
> > + irqs = devm_kzalloc(&mc_dev->dev, irq_count * sizeof(irqs[0]),
> > + GFP_KERNEL);
> > + if (!irqs) {
> > + error = -ENOMEM;
> > + dev_err(&mc_dev->dev, "No memory to allocate irqs[]\n");
> > + goto error;
>
> I kind of hate One Err Style error handling, because the error labels are
> so general... You can never guess the point of it until you scroll down
>
Agreed. I will replace the generic error cleanup exit point with
error-specific cleanup exit points, and return directly when no cleanup
is needed.

> to read what "goto error;" does. The error handling here calls
> devm_kfree() which is not needed... devm_ functions automatically clean
> up after themselves. This seems a pattern throughout. Do a search for
> devm_free() and see which ones are really needed or not.
>
I know that memory allocated with devm_kzalloc() is freed at the end of the
lifetime of the device it is attached to. However, in error paths, why wait
until the device is destroyed? Why not free the memory earlier so that it
can be used for other purposes?

> Also the error message isn't needed here. kzalloc() has its own better
> error messages built-in. Adding these error messages which will never be
> printed is just a waste of RAM.
>
Agreed. Error message removed.

> In other words this should look like:
>
> irqs = devm_kcalloc(&mc_dev->dev, sizeof(*irqs), irq_count,
> GFP_KERNEL);
> if (!irqs)
> return -ENOMEM;
>
> regards,
> dan carpenter

2015-05-05 00:13:12

by J. German Rivera

[permalink] [raw]
Subject: RE: [PATCH 4/7] staging: fsl-mc: Upgraded MC bus driver to match MC fw 7.0.0



> -----Original Message-----
> From: Dan Carpenter [mailto:[email protected]]
> Sent: Thursday, April 30, 2015 7:13 AM
> To: Rivera Jose-B46482
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Yoder Stuart-
> B08248; Sharma Bhupesh-B45370; [email protected]; Hamciuc Bogdan-BHAMCIU1;
> Erez Nir-RM30794; katz Itai-RM05202; Wood Scott-B07421; Marginean
> Alexandru-R89243; Schmitt Richard-B43082
> Subject: Re: [PATCH 4/7] staging: fsl-mc: Upgraded MC bus driver to match
> MC fw 7.0.0
>
> On Tue, Apr 28, 2015 at 12:39:07PM -0500, J. German Rivera wrote:
> > - Migrated MC bus driver to use DPRC flib 0.6.
>
> What does this mean? What is a flib?
>
The DPRC flib is the API to manipulate DPRC objects.

> After reading the patch, apparently it means that we can remove all the
> ifdefs from patch 1. :)
>
No, we cannot as the required GIC-ITS support is not upstream yet.
I added some of the #ifdefs back that were removed by mistake.

> > - Changed IRQ setup infrastructure to be able to program MSIs
> > for MC objects in an object-independent way.
>
> Are these two things related?
>
No.

> Why does this patch add so many new EXPORTS()s?
>
Removed new EXPORTs added as they are not needed yet.

> regards,
> dan carpenter

2015-05-05 08:49:11

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

On Mon, May 04, 2015 at 10:09:08PM +0000, Jose Rivera wrote:
> > > + WARN_ON((int16_t)irq_count < 0);
> >
> > This code is doing "WARN_ON(test_bit(15, (unsigned long *)&irq_count));".
> > That seems like nonsense. Anyway, just delete the WARN_ON().
> >
> I disagree. This WARN_ON is checking that irq_count is in the expected range
> (it fits in int16_t as a positive number). The dprc_scan_objects() function
> expects irq_count to be of type "unsigned int" (which is 32-bit unsigned)
>

You're not allowed to disagree because it's a testable thing and not an
opinion about style or something. :P What you want is:

WARN_ON(irq_count > SHRT_MAX);

> > > +
> > > + if ((int16_t)irq_count >
> > > + mc_bus->resource_pools[FSL_MC_POOL_IRQ].max_count) {
> >
> > Why are we casting this? Also can you align it like:
> >
> This casting is done for safety, to prevent the comparison to be done
> in "unsigned int" due to integer promotion rules.

We are truncating away the top bytes but then we use them later.
Fortunately we use them only to print out a warning, but if we used them
for anything else it would be a serious bug.

Are you expecting .max_count to be negative?

If not then both sides are positive and type promotion is fine. We can
delete the first (buggy) warning, like I said and just leave the second
warning. It will now complain if any of bits 16 to 31 are set where
before it wouldn't.

> > to read what "goto error;" does. The error handling here calls
> > devm_kfree() which is not needed... devm_ functions automatically clean
> > up after themselves. This seems a pattern throughout. Do a search for
> > devm_free() and see which ones are really needed or not.
> >
> I know that memory allocated with devm_kzalloc() is freed at the end of the
> lifetime of the device it is attached to. However, in error paths, why wait
> until the device is destroyed? Why not free the memory earlier so that it
> can be used for other purposes?

My understanding is that devm_ functions are supposed to be used in the
probe() functions to simplify the error handling. So hopefully the
device lifetime ends as soon as this function returns a failure.

devm_ function are not a use them everywhere because now the kernel has
garbage collection type thing.

regards,
dan carpenter

2015-05-05 08:52:22

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 4/7] staging: fsl-mc: Upgraded MC bus driver to match MC fw 7.0.0

On Mon, May 04, 2015 at 11:58:13PM +0000, Jose Rivera wrote:
> >
> > On Tue, Apr 28, 2015 at 12:39:07PM -0500, J. German Rivera wrote:
> > > - Migrated MC bus driver to use DPRC flib 0.6.
> >
> > What does this mean? What is a flib?
> >
> The DPRC flib is the API to manipulate DPRC objects.
>
> > After reading the patch, apparently it means that we can remove all the
> > ifdefs from patch 1. :)
> >
> No, we cannot as the required GIC-ITS support is not upstream yet.
> I added some of the #ifdefs back that were removed by mistake.
>
> > > - Changed IRQ setup infrastructure to be able to program MSIs
> > > for MC objects in an object-independent way.
> >
> > Are these two things related?
> >
> No.

When I asked this function I was partly wondering why they were broken
up the way they are. This changelog was not enough information to start
reviewing the patch.

regards,
dan carpenter

2015-05-05 16:11:47

by J. German Rivera

[permalink] [raw]
Subject: RE: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support



> -----Original Message-----
> From: Dan Carpenter [mailto:[email protected]]
> Sent: Tuesday, May 05, 2015 3:49 AM
> To: Rivera Jose-B46482
> Cc: [email protected]; [email protected]; [email protected]; Sharma
> Bhupesh-B45370; [email protected]; [email protected];
> Yoder Stuart-B08248; Wood Scott-B07421; Erez Nir-RM30794; katz Itai-
> RM05202; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Schmitt
> Richard-B43082
> Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
>
> On Mon, May 04, 2015 at 10:09:08PM +0000, Jose Rivera wrote:
> > > > + WARN_ON((int16_t)irq_count < 0);
> > >
> > > This code is doing "WARN_ON(test_bit(15, (unsigned long
> *)&irq_count));".
> > > That seems like nonsense. Anyway, just delete the WARN_ON().
> > >
> > I disagree. This WARN_ON is checking that irq_count is in the expected
> > range (it fits in int16_t as a positive number). The
> > dprc_scan_objects() function expects irq_count to be of type "unsigned
> > int" (which is 32-bit unsigned)
> >
>
> You're not allowed to disagree because it's a testable thing and not an
> opinion about style or something. :P What you want is:
>
> WARN_ON(irq_count > SHRT_MAX);
>
I see your point now. The check "(int16_t)irq_count < 0)" will not be able
to catch 0x10000 > 0x7fff, but "irq_count > SHRT_MAX) will. So I'll
make the suggested change, but I would prefer to use S16_MAX rather than
SHRT_MAX.

> > > > +
> > > > + if ((int16_t)irq_count >
> > > > + mc_bus-
> >resource_pools[FSL_MC_POOL_IRQ].max_count) {
> > >
> > > Why are we casting this? Also can you align it like:
> > >
> > This casting is done for safety, to prevent the comparison to be done
> > in "unsigned int" due to integer promotion rules.
>
> We are truncating away the top bytes but then we use them later.
> Fortunately we use them only to print out a warning, but if we used them
> for anything else it would be a serious bug.
>
> Are you expecting .max_count to be negative?
>
No.

> If not then both sides are positive and type promotion is fine. We can
> delete the first (buggy) warning, like I said and just leave the second
> warning. It will now complain if any of bits 16 to 31 are set where
> before it wouldn't.
>
Agreed. I'll remove the (int16_t) type cast from the "if". So, I'll change
this code snippet to be like this:

WARN_ON(irq_count > S16_MAX);

if (irq_count >
mc_bus->resource_pools[FSL_MC_POOL_IRQ].max_count)
dev_warn(...);


Although the WARN_ON seems redundant with the "if", it catches a different
problem. The WARN_ON() catches irq_count to be out of range, the "if"
tells when we run out of IRQ resources fro a valid irq_count.

> > > to read what "goto error;" does. The error handling here calls
> > > devm_kfree() which is not needed... devm_ functions automatically
> > > clean up after themselves. This seems a pattern throughout. Do a
> > > search for
> > > devm_free() and see which ones are really needed or not.
> > >
> > I know that memory allocated with devm_kzalloc() is freed at the end
> > of the lifetime of the device it is attached to. However, in error
> > paths, why wait until the device is destroyed? Why not free the memory
> > earlier so that it can be used for other purposes?
>
Why then do the devm_kfree() function exist?

I will not remove the devm_free() calls unless the upstream maintainer
requires me to do so.

> My understanding is that devm_ functions are supposed to be used in the
> probe() functions to simplify the error handling. So hopefully the
> device lifetime ends as soon as this function returns a failure.
>
> devm_ function are not a use them everywhere because now the kernel has
> garbage collection type thing.
>
> regards,
> dan carpenter

2015-05-05 16:20:31

by J. German Rivera

[permalink] [raw]
Subject: RE: [PATCH 6/7] staging: fsl-mc: Add locking to serialize mc_send_command() calls



> -----Original Message-----
> From: Dan Carpenter [mailto:[email protected]]
> Sent: Thursday, April 30, 2015 7:59 AM
> To: Rivera Jose-B46482
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Yoder Stuart-
> B08248; Sharma Bhupesh-B45370; [email protected]; Hamciuc Bogdan-BHAMCIU1;
> Erez Nir-RM30794; katz Itai-RM05202; Wood Scott-B07421; Marginean
> Alexandru-R89243; Schmitt Richard-B43082
> Subject: Re: [PATCH 6/7] staging: fsl-mc: Add locking to serialize
> mc_send_command() calls
>
> On Tue, Apr 28, 2015 at 12:39:09PM -0500, J. German Rivera wrote:
> > @@ -230,15 +235,26 @@ static inline enum mc_cmd_status
> mc_read_response(struct mc_command __iomem *
> > * @cmd: command to be sent
> > *
> > * Returns '0' on Success; Error code otherwise.
> > - *
> > - * NOTE: This function cannot be invoked from from atomic contexts.
> > */
> > int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
> > {
> > + int error;
> > enum mc_cmd_status status;
> > unsigned long jiffies_until_timeout =
> > jiffies + MC_CMD_COMPLETION_TIMEOUT_JIFFIES;
>
> We busy loop while holding a spinlock for half a second. That seems bad.
>
What would be a reasonable max time for holding a spinlock here?

> >
> > + if (preemptible()) {
>
> This is wrong. If the user asked for spinlocks they should always get
> spinlocks. It shouldn't matter that they are not currently holding a
> different lock.
>
> I'm skeptical of this locking anyway.
>
> Also what about if they have PREEMPT disabled? There aren't any users
> for this stuff anyway so it's impossible to review how people are
> FSL_MC_IO_ATOMIC_CONTEXT_PORTAL.
>
> Let's wait until there is a user before looking at this.
>
> > - return mc_status_to_error(status);
> > + error = mc_status_to_error(status);
> > + goto common_exit;
> > }
> >
> > - return 0;
> > + error = 0;
> > +
> > +common_exit:
>
> Just name this unlock:.
>
> > + if (preemptible())
> > + mutex_unlock(&mc_io->mutex);
> > + else
> > + spin_unlock(&mc_io->spinlock);
> > +
> > + return error;
> > }
>
> regards,
> dan carpenter

2015-05-05 16:40:33

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

On Tue, May 05, 2015 at 04:08:49PM +0000, Jose Rivera wrote:
> > Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
> >
> > On Mon, May 04, 2015 at 10:09:08PM +0000, Jose Rivera wrote:
> > > > > + WARN_ON((int16_t)irq_count < 0);
> > > >
> > > > This code is doing "WARN_ON(test_bit(15, (unsigned long
> > *)&irq_count));".
> > > > That seems like nonsense. Anyway, just delete the WARN_ON().
> > > >
> > > I disagree. This WARN_ON is checking that irq_count is in the expected
> > > range (it fits in int16_t as a positive number). The
> > > dprc_scan_objects() function expects irq_count to be of type "unsigned
> > > int" (which is 32-bit unsigned)
> > >
> >
> > You're not allowed to disagree because it's a testable thing and not an
> > opinion about style or something. :P What you want is:
> >
> > WARN_ON(irq_count > SHRT_MAX);
> >
> I see your point now. The check "(int16_t)irq_count < 0)" will not be able
> to catch 0x10000 > 0x7fff, but "irq_count > SHRT_MAX) will. So I'll
> make the suggested change, but I would prefer to use S16_MAX rather than
> SHRT_MAX.
>

Huh? I didn't even know about the S16_MAX definition. There are
literally no users of it in the kernel. It's not very fair because
there are few users of SHRT_MAX. But there are literally no users of
S32_MAX in the kernel and 358 users of INT_MAX.

Don't insist that you must be special and different from everyone else.

> > > > to read what "goto error;" does. The error handling here calls
> > > > devm_kfree() which is not needed... devm_ functions automatically
> > > > clean up after themselves. This seems a pattern throughout. Do a
> > > > search for
> > > > devm_free() and see which ones are really needed or not.
> > > >
> > > I know that memory allocated with devm_kzalloc() is freed at the end
> > > of the lifetime of the device it is attached to. However, in error
> > > paths, why wait until the device is destroyed? Why not free the memory
> > > earlier so that it can be used for other purposes?
> >
> Why then do the devm_kfree() function exist?
>
> I will not remove the devm_free() calls unless the upstream maintainer
> requires me to do so.

I'm not saying remove them all. I'm saying:

1) I am concerned that you seem to thing devm_ is garbage collection.
2) Remove the ones which are not needed, because it is a lot of work
for me to figure this out so I was hoping you could do it.

regards,
dan carpenter

2015-05-05 19:42:43

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

On Tue, 2015-05-05 at 11:08 -0500, Rivera Jose-B46482 wrote:
> > > > to read what "goto error;" does. The error handling here calls
> > > > devm_kfree() which is not needed... devm_ functions automatically
> > > > clean up after themselves. This seems a pattern throughout. Do a
> > > > search for
> > > > devm_free() and see which ones are really needed or not.
> > > >
> > > I know that memory allocated with devm_kzalloc() is freed at the end
> > > of the lifetime of the device it is attached to. However, in error
> > > paths, why wait until the device is destroyed? Why not free the memory
> > > earlier so that it can be used for other purposes?
> >
> Why then do the devm_kfree() function exist?
>
> I will not remove the devm_free() calls unless the upstream maintainer
> requires me to do so.

The whole point of devm is to simplify (often poorly tested) error
paths. You're trying to optimize the error path instead of simplify it,
which doesn't make sense.

devm_kfree() is for the uncommon case when you want to free the memory
in a situation where the device isn't being unbound any time soon, but
you still want the memory to be handled by devm for some reason.

Most users of devm_kzalloc() do not use devm_kfree():
scott@snotra:~/fsl/git/linux/upstream$ git grep devm_kzalloc|wc -l
2750
scott@snotra:~/fsl/git/linux/upstream$ git grep devm_kfree|wc -l
118

-Scott

2015-05-05 19:57:05

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

On Tue, 2015-05-05 at 19:40 +0300, Dan Carpenter wrote:
> On Tue, May 05, 2015 at 04:08:49PM +0000, Jose Rivera wrote:
> > > Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
> > >
> > > On Mon, May 04, 2015 at 10:09:08PM +0000, Jose Rivera wrote:
> > > > > > + WARN_ON((int16_t)irq_count < 0);
> > > > >
> > > > > This code is doing "WARN_ON(test_bit(15, (unsigned long
> > > *)&irq_count));".
> > > > > That seems like nonsense. Anyway, just delete the WARN_ON().
> > > > >
> > > > I disagree. This WARN_ON is checking that irq_count is in the expected
> > > > range (it fits in int16_t as a positive number). The
> > > > dprc_scan_objects() function expects irq_count to be of type "unsigned
> > > > int" (which is 32-bit unsigned)
> > > >
> > >
> > > You're not allowed to disagree because it's a testable thing and not an
> > > opinion about style or something. :P What you want is:
> > >
> > > WARN_ON(irq_count > SHRT_MAX);
> > >
> > I see your point now. The check "(int16_t)irq_count < 0)" will not be able
> > to catch 0x10000 > 0x7fff, but "irq_count > SHRT_MAX) will. So I'll
> > make the suggested change, but I would prefer to use S16_MAX rather than
> > SHRT_MAX.
> >
>
> Huh? I didn't even know about the S16_MAX definition. There are
> literally no users of it in the kernel. It's not very fair because
> there are few users of SHRT_MAX. But there are literally no users of
> S32_MAX in the kernel and 358 users of INT_MAX.
>
> Don't insist that you must be special and different from everyone else.

There are some users of U16_MAX, U32_MAX, and U64_MAX. Why use a limit
for a different type than is being used? Why have s16/s32 at all if
you're going to conflate it with short/int elsewhere?

That said, I don't see where this code is actually using s16 (or
int16_t) for irq_count except in these weird error checks. German, why
do you need to check against 0x7fff (whatever you call it) at all?
Won't comparing to a promoted-to-unsigned-int .max_count (as you do
immediately after the WARN_ON) suffice?

-Scott

2015-05-05 20:23:40

by J. German Rivera

[permalink] [raw]
Subject: RE: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support



> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, May 05, 2015 2:57 PM
> To: Dan Carpenter
> Cc: Rivera Jose-B46482; [email protected]; Yoder Stuart-B08248;
> Hamciuc Bogdan-BHAMCIU1; [email protected]; Sharma Bhupesh-B45370;
> [email protected]; [email protected]; [email protected];
> Erez Nir-RM30794; katz Itai-RM05202; Marginean Alexandru-R89243; Schmitt
> Richard-B43082
> Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
>
> On Tue, 2015-05-05 at 19:40 +0300, Dan Carpenter wrote:
> > On Tue, May 05, 2015 at 04:08:49PM +0000, Jose Rivera wrote:
> > > > Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
> > > >
> > > > On Mon, May 04, 2015 at 10:09:08PM +0000, Jose Rivera wrote:
> > > > > > > + WARN_ON((int16_t)irq_count < 0);
> > > > > >
> > > > > > This code is doing "WARN_ON(test_bit(15, (unsigned long
> > > > *)&irq_count));".
> > > > > > That seems like nonsense. Anyway, just delete the WARN_ON().
> > > > > >
> > > > > I disagree. This WARN_ON is checking that irq_count is in the
> > > > > expected range (it fits in int16_t as a positive number). The
> > > > > dprc_scan_objects() function expects irq_count to be of type
> > > > > "unsigned int" (which is 32-bit unsigned)
> > > > >
> > > >
> > > > You're not allowed to disagree because it's a testable thing and
> > > > not an opinion about style or something. :P What you want is:
> > > >
> > > > WARN_ON(irq_count > SHRT_MAX);
> > > >
> > > I see your point now. The check "(int16_t)irq_count < 0)" will not
> > > be able to catch 0x10000 > 0x7fff, but "irq_count > SHRT_MAX) will.
> > > So I'll make the suggested change, but I would prefer to use S16_MAX
> > > rather than SHRT_MAX.
> > >
> >
> > Huh? I didn't even know about the S16_MAX definition. There are
> > literally no users of it in the kernel. It's not very fair because
> > there are few users of SHRT_MAX. But there are literally no users of
> > S32_MAX in the kernel and 358 users of INT_MAX.
> >
> > Don't insist that you must be special and different from everyone else.
>
> There are some users of U16_MAX, U32_MAX, and U64_MAX. Why use a limit
> for a different type than is being used? Why have s16/s32 at all if
> you're going to conflate it with short/int elsewhere?
>
> That said, I don't see where this code is actually using s16 (or
> int16_t) for irq_count except in these weird error checks. German, why
> do you need to check against 0x7fff (whatever you call it) at all?
> Won't comparing to a promoted-to-unsigned-int .max_count (as you do
> immediately after the WARN_ON) suffice?
>
mc_bus->resource_pools[FSL_MC_POOL_IRQ].max_count is of type int16_t
(and is so, because its value comes from an MC API that returns
an int16_t). The reason for checking irq_count against 0x7ffff is to
catch the case in which irq_count is out of range (irq_count originates
from values coming from the MC device, so we should do some validation
before using it)

Thanks,

German


> -Scott
>

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2015-05-05 20:26:26

by J. German Rivera

[permalink] [raw]
Subject: RE: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support



> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, May 05, 2015 2:42 PM
> To: Rivera Jose-B46482
> Cc: Dan Carpenter; [email protected]; [email protected];
> [email protected]; Sharma Bhupesh-B45370; [email protected]; linux-
> [email protected]; Yoder Stuart-B08248; Erez Nir-RM30794; katz Itai-
> RM05202; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Schmitt
> Richard-B43082
> Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
>
> On Tue, 2015-05-05 at 11:08 -0500, Rivera Jose-B46482 wrote:
> > > > > to read what "goto error;" does. The error handling here calls
> > > > > devm_kfree() which is not needed... devm_ functions
> > > > > automatically clean up after themselves. This seems a pattern
> > > > > throughout. Do a search for
> > > > > devm_free() and see which ones are really needed or not.
> > > > >
> > > > I know that memory allocated with devm_kzalloc() is freed at the
> > > > end of the lifetime of the device it is attached to. However, in
> > > > error paths, why wait until the device is destroyed? Why not free
> > > > the memory earlier so that it can be used for other purposes?
> > >
> > Why then do the devm_kfree() function exist?
> >
> > I will not remove the devm_free() calls unless the upstream maintainer
> > requires me to do so.
>
> The whole point of devm is to simplify (often poorly tested) error paths.
> You're trying to optimize the error path instead of simplify it, which
> doesn't make sense.
>
> devm_kfree() is for the uncommon case when you want to free the memory in
> a situation where the device isn't being unbound any time soon, but you
> still want the memory to be handled by devm for some reason.
>
> Most users of devm_kzalloc() do not use devm_kfree():
> scott@snotra:~/fsl/git/linux/upstream$ git grep devm_kzalloc|wc -l
> 2750
> scott@snotra:~/fsl/git/linux/upstream$ git grep devm_kfree|wc -l
> 118
>
Ok, I'll remove the devm_kfree() calls.

Thanks,

German

> -Scott
>

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2015-05-05 20:40:27

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

On Tue, 2015-05-05 at 15:22 -0500, Rivera Jose-B46482 wrote:
>
>
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, May 05, 2015 2:57 PM
> > To: Dan Carpenter
> > Cc: Rivera Jose-B46482; [email protected]; Yoder Stuart-B08248;
> > Hamciuc Bogdan-BHAMCIU1; [email protected]; Sharma Bhupesh-B45370;
> > [email protected]; [email protected]; [email protected];
> > Erez Nir-RM30794; katz Itai-RM05202; Marginean Alexandru-R89243; Schmitt
> > Richard-B43082
> > Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
> >
> > On Tue, 2015-05-05 at 19:40 +0300, Dan Carpenter wrote:
> > > On Tue, May 05, 2015 at 04:08:49PM +0000, Jose Rivera wrote:
> > > > > Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
> > > > >
> > > > > On Mon, May 04, 2015 at 10:09:08PM +0000, Jose Rivera wrote:
> > > > > > > > + WARN_ON((int16_t)irq_count < 0);
> > > > > > >
> > > > > > > This code is doing "WARN_ON(test_bit(15, (unsigned long
> > > > > *)&irq_count));".
> > > > > > > That seems like nonsense. Anyway, just delete the WARN_ON().
> > > > > > >
> > > > > > I disagree. This WARN_ON is checking that irq_count is in the
> > > > > > expected range (it fits in int16_t as a positive number). The
> > > > > > dprc_scan_objects() function expects irq_count to be of type
> > > > > > "unsigned int" (which is 32-bit unsigned)
> > > > > >
> > > > >
> > > > > You're not allowed to disagree because it's a testable thing and
> > > > > not an opinion about style or something. :P What you want is:
> > > > >
> > > > > WARN_ON(irq_count > SHRT_MAX);
> > > > >
> > > > I see your point now. The check "(int16_t)irq_count < 0)" will not
> > > > be able to catch 0x10000 > 0x7fff, but "irq_count > SHRT_MAX) will.
> > > > So I'll make the suggested change, but I would prefer to use S16_MAX
> > > > rather than SHRT_MAX.
> > > >
> > >
> > > Huh? I didn't even know about the S16_MAX definition. There are
> > > literally no users of it in the kernel. It's not very fair because
> > > there are few users of SHRT_MAX. But there are literally no users of
> > > S32_MAX in the kernel and 358 users of INT_MAX.
> > >
> > > Don't insist that you must be special and different from everyone else.
> >
> > There are some users of U16_MAX, U32_MAX, and U64_MAX. Why use a limit
> > for a different type than is being used? Why have s16/s32 at all if
> > you're going to conflate it with short/int elsewhere?
> >
> > That said, I don't see where this code is actually using s16 (or
> > int16_t) for irq_count except in these weird error checks. German, why
> > do you need to check against 0x7fff (whatever you call it) at all?
> > Won't comparing to a promoted-to-unsigned-int .max_count (as you do
> > immediately after the WARN_ON) suffice?
> >
> mc_bus->resource_pools[FSL_MC_POOL_IRQ].max_count is of type int16_t
> (and is so, because its value comes from an MC API that returns
> an int16_t). The reason for checking irq_count against 0x7ffff is to
> catch the case in which irq_count is out of range (irq_count originates
> from values coming from the MC device, so we should do some validation
> before using it)

Comparing irq_count with max_count will catch any condition that
comparing irq_count with 0x7fff will catch.

-Scott

2015-05-06 06:43:09

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

On Tue, May 05, 2015 at 02:56:41PM -0500, Scott Wood wrote:
> > Don't insist that you must be special and different from everyone else.
>
> There are some users of U16_MAX, U32_MAX, and U64_MAX. Why use a limit
> for a different type than is being used? Why have s16/s32 at all if
> you're going to conflate it with short/int elsewhere?

I'm don't care about SHRT_MAX vs S16_MAX, I'm just saying generally,
people should fight every instinct they have to be unique in the kernel.

regards,
dan carpenter