linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series
@ 2014-09-19 22:49 J. German Rivera
  2014-09-19 22:49 ` [PATCH 1/3 v2] drivers/bus: Added Freescale Management Complex APIs J. German Rivera
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: J. German Rivera @ 2014-09-19 22:49 UTC (permalink / raw)
  To: gregkh, arnd, linux-kernel
  Cc: stuart.yoder, Kim.Phillips, scottwood, agraf, linuxppc-release

This patch series introduces Linux support for the Freescale
Management Complex (fsl-mc) hardware. This patch series is dependent
on the patch series "ARM64: Add support for FSL's LS2085A SoC"
(http://thread.gmane.org/gmane.linux.ports.arm.kernel/351829)

The fsl-mc is a hardware resource manager that manages specialized
hardware objects used in network-oriented packet processing
applications.  After the fsl-mc block is enabled, pools of hardware
resources are available, such as queues, buffer poools, I/O
interfaces.  These resources are building blocks that can be
used to create functional hardware objects such as network
interfaces, crypto accelerator instances, or L2 switches.

All the fsl-mc managed hardware resources/objects are represented in
a physical grouping mechanism called a 'container' or DPRC (data
path resource container).

>From the point of view of an OS, a DPRC functions similar to a plug
and play bus.  Using fsl-mc commands software can enumerate the
contents of the DPRC discovering the hardware objects present
and binding them to drivers.  Hardware objects can be created
and removed dynamically, providing hot pluggability of the hardware
objects.

Software contexts interact with the fsl-mc by sending commands through
a memory mapped hardware interface called an "MC portal". Every
fsl-mc object type has a command set to manage the objects. Key
DPRC commands include:
   -create/destroy a DPRC
   -enumerate objects and resource pools in the DPRC, including
    identifying mappable regions and the number of IRQs an object
    may have
   -IRQ configuration
   -move objects/resources between DPRCs
   -connecting objects (e.g. connecting a network interface to
    an L2 switch port)
   -reset

Patch 1 contains a minimal set of low level functions to send an
d receive commands to the fsl-mc. It includes support for basic
management commands and commands to manipulate DPRC objects.

Patch 2 contains a platform device driver that sets up and registers
the basic bus infrastructure including support for adding/removing
devices, register/unregister of drivers, and bus match support
to bind devices to drivers.

Patch 3 contains an driver that manages DPRC objects (the
container that holds the hardware resources). This driver
functions as a bus controller and handles enumeration
of the objects in the container and hotplug events.

CHANGE HISTORY

Issues pending resolution not addressed by v2:
- What to do with Doxygen comments in patch 1
- Whether to move or not FSL-specific header files added in include/linux,
  by this patch series, to another location
- Whether to change or not the wording of the licenses used in some
  source files

- Checkpatch warnings:
  * WARNING: DT compatible string "fsl,qoriq-mc" appears un-documented
    This warning will go away once the prerequisite patch series is
    accepted upstream.
  * WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
    This warning still happens even after adding MAINTAINERS file entries in
    the same patch where new files were added
- Sparse warning:
  * drivers/bus/fsl-mc/fsl_mc_sys.c:193:9: warning: context imbalance in 'mc_send_command' - different lock contexts for basic block
    This warning is caused by conditionally acquiring/releasing a lock in function mc_send_command().
    Not clear how to solve this warning, as we need to be able to decide at
    run time based on a flag whether to acquire the lock or not.

Changes in v2:
- Added reference to the patch series this patch is dependent on for
  device tree and device tree bindings changes. (See above).
- Removed patch 4 (update to the MAINTAINERS file), as this is done
  now in patch 1
- Addressed comments from Joe Perches, Kim Phillips, Alex Graf
  and Scott Wood. Details in each patch.

Changes in v1:
- No changes since RFC v4

Changes in RFC v4:
- Fixed parameter mismatch for device_find_child() call in fsl_mc_dprc.c
- Added back the dma_mask field to struct fsl_mc_device as it is needed
  by some MC child device drivers. However, the default DMA mask now
  does not have the 32-bit limitation of the original patch series (v1).

Changes in RFC v3:
Rework to address comments from Arnd Bergmann:
- Removed per-bus list of children, and instead use device_for_each_child()
- Use the same structure (struct fsl_mc_device) to represent both
  bus devices and their children.
http://www.mail-archive.com/linux-kernel%40vger.kernel.org/msg711301.html

Changes in RFC v2:
Rework to address comments from Arnd Bergmann:
- Removed fsl_mc_bus structure and global variable
- Removed the 'magic' fields from all structs
- Removed NULL initializers
- Replaced all occurrences of EXPORT_SYMBOL() with EXPORT_SYMBOL_GPL()
- Removed function dprc_parse_dt_node(), and replaced its call by a
  direct call to of_address_to_resource()
- Removed struct fsl_mc_device_region and use standard 'struct resource'
  instead.
- Removed dma_mask field from 'struct fsl_mc_device' as it is not currently
  being used.
- Removed redundant 'driver' field from struct fsl_mc_device
- Removed the container field. We can get the parent DPRC of a given dev,
  from its dev.parent field.
http://www.mail-archive.com/linux-kernel%40vger.kernel.org/msg708858.html


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

* [PATCH 1/3 v2] drivers/bus: Added Freescale Management Complex APIs
  2014-09-19 22:49 [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series J. German Rivera
@ 2014-09-19 22:49 ` J. German Rivera
  2014-09-24  0:49   ` Kim Phillips
  2014-09-19 22:49 ` [PATCH 2/3 v2] drivers/bus: Freescale Management Complex (fsl-mc) bus driver J. German Rivera
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 20+ messages in thread
From: J. German Rivera @ 2014-09-19 22:49 UTC (permalink / raw)
  To: gregkh, arnd, linux-kernel
  Cc: stuart.yoder, Kim.Phillips, scottwood, agraf, linuxppc-release,
	J. German Rivera

From: "J. German Rivera" <German.Rivera@freescale.com>

APIs to access the Management Complex (MC) hardware
module of Freescale LS2 SoCs. This patch includes
APIs to check the MC firmware version and to manipulate
DPRC objects in the MC.

Signed-off-by: J. German Rivera <German.Rivera@freescale.com>
Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
---
Changes in v2:
- Addressed comment from Joe Perches:
  * Refactored logic to actively fail on err and proceed at
    the same indent level on success, for all functions in dprc.c
    and dpmng.c.

- Addressed comments from Kim Phillips:
  * Fixed warning in mc_send_command
  * Changed serialization logic in mc_send_command() to only use
    spinlock_irqsave() when both threads and interrupt handlers
    concurrently access the same portal.
  * Changed switch to lookup table in mc_status_to_error()
  * Removed macros iowrite64(), ioread64(), ENOTSUP from fsl_mc_sys.h
  * Removed #ifdef side for FSL_MC_FIRMWARE in fsl_mc_cmd.h
  * Changed non-devm_ API calls to devm_ API calls and refactored
    fsl_create_mc_io()/fsl_destroy_mc_io() to simplify cleanup logic.

- Addressed comments from Scott Wood:
  * Return -ENXIO instead of -EFAULT when ioremap_nocache() fails

- Addressed comments from Alex Graf:
  * Added MAINTAINERS file entries for new files
  * Added dev param to fsl_create_mc_io(), to enable the use
    of devm_ APIs in this function and fsl_destroy_mc_io().
  * Changed the value of the timeout for MC command completion
    to be a function of HZ instead of a hard-coded jiffies value.

 MAINTAINERS                        |    8 +
 drivers/bus/fsl-mc/dpmng.c         |   95 +++++
 drivers/bus/fsl-mc/dprc.c          |  520 ++++++++++++++++++++++++
 drivers/bus/fsl-mc/fsl_dpmng_cmd.h |   83 ++++
 drivers/bus/fsl-mc/fsl_dprc_cmd.h  |  545 +++++++++++++++++++++++++
 drivers/bus/fsl-mc/fsl_mc_sys.c    |  195 +++++++++
 include/linux/fsl_dpmng.h          |  120 ++++++
 include/linux/fsl_dprc.h           |  790 ++++++++++++++++++++++++++++++++++++
 include/linux/fsl_mc_cmd.h         |  168 ++++++++
 include/linux/fsl_mc_sys.h         |   89 ++++
 10 files changed, 2613 insertions(+)
 create mode 100644 drivers/bus/fsl-mc/dpmng.c
 create mode 100644 drivers/bus/fsl-mc/dprc.c
 create mode 100644 drivers/bus/fsl-mc/fsl_dpmng_cmd.h
 create mode 100644 drivers/bus/fsl-mc/fsl_dprc_cmd.h
 create mode 100644 drivers/bus/fsl-mc/fsl_mc_sys.c
 create mode 100644 include/linux/fsl_dpmng.h
 create mode 100644 include/linux/fsl_dprc.h
 create mode 100644 include/linux/fsl_mc_cmd.h
 create mode 100644 include/linux/fsl_mc_sys.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 809ecd6..1bdccf4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3875,6 +3875,14 @@ F:	sound/soc/fsl/fsl*
 F:	sound/soc/fsl/imx*
 F:	sound/soc/fsl/mpc8610_hpcd.c

+FREESCALE QORIQ MANAGEMENT COMPLEX DRIVER
+M:	J. German Rivera <German.Rivera@freescale.com>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	include/linux/fsl_mc*
+F:	include/linux/fsl_dp*
+F:	drivers/bus/fsl-mc/*
+
 FREEVXFS FILESYSTEM
 M:	Christoph Hellwig <hch@infradead.org>
 W:	ftp://ftp.openlinux.org/pub/people/hch/vxfs
diff --git a/drivers/bus/fsl-mc/dpmng.c b/drivers/bus/fsl-mc/dpmng.c
new file mode 100644
index 0000000..4fa70a8
--- /dev/null
+++ b/drivers/bus/fsl-mc/dpmng.c
@@ -0,0 +1,95 @@
+/* Copyright 2013-2014 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *     * Neither the name of Freescale Semiconductor nor the
+ *       names of its contributors may be used to endorse or promote products
+ *       derived from this software without specific prior written permission.
+ *
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <linux/fsl_mc_sys.h>
+#include <linux/fsl_mc_cmd.h>
+#include <linux/fsl_dpmng.h>
+#include "fsl_dpmng_cmd.h"
+
+int mc_get_version(struct fsl_mc_io *mc_io, struct mc_version *mc_ver_info)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	cmd.header = mc_encode_cmd_header(DPMNG_CMDID_GET_VERSION,
+					  DPMNG_CMDSZ_GET_VERSION,
+					  MC_CMD_PRI_LOW, 0);
+
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+		DPMNG_RSP_GET_VERSION(cmd, mc_ver_info);
+
+	return 0;
+}
+
+int dpmng_reset_aiop(struct fsl_mc_io *mc_io, int aiop_tile_id)
+{
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(DPMNG_CMDID_RESET_AIOP,
+					  DPMNG_CMDSZ_RESET_AIOP,
+					  MC_CMD_PRI_LOW, 0);
+
+	DPMNG_CMD_RESET_AIOP(cmd, aiop_tile_id);
+
+	return mc_send_command(mc_io, &cmd);
+}
+
+int dpmng_load_aiop(struct fsl_mc_io *mc_io,
+		    int aiop_tile_id, uint8_t *img_addr, int img_size)
+{
+	struct mc_command cmd = { 0 };
+	uint64_t img_paddr = virt_to_phys(img_addr);
+
+	cmd.header = mc_encode_cmd_header(DPMNG_CMDID_LOAD_AIOP,
+					  DPMNG_CMDSZ_LOAD_AIOP,
+					  MC_CMD_PRI_LOW, 0);
+
+	DPMNG_CMD_LOAD_AIOP(cmd, aiop_tile_id, img_size, img_paddr);
+
+	return mc_send_command(mc_io, &cmd);
+}
+
+int dpmng_run_aiop(struct fsl_mc_io *mc_io,
+		   int aiop_tile_id, uint32_t cores_mask, uint64_t options)
+{
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(DPMNG_CMDID_RUN_AIOP,
+					  DPMNG_CMDSZ_RUN_AIOP,
+					  MC_CMD_PRI_LOW, 0);
+
+	DPMNG_CMD_RUN_AIOP(cmd, aiop_tile_id, cores_mask, options);
+
+	return mc_send_command(mc_io, &cmd);
+}
diff --git a/drivers/bus/fsl-mc/dprc.c b/drivers/bus/fsl-mc/dprc.c
new file mode 100644
index 0000000..0134d49
--- /dev/null
+++ b/drivers/bus/fsl-mc/dprc.c
@@ -0,0 +1,520 @@
+/* Copyright 2013-2014 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *     * Neither the name of Freescale Semiconductor nor the
+ *       names of its contributors may be used to endorse or promote products
+ *       derived from this software without specific prior written permission.
+ *
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <linux/fsl_mc_sys.h>
+#include <linux/fsl_mc_cmd.h>
+#include <linux/fsl_dprc.h>
+#include "fsl_dprc_cmd.h"
+
+int dprc_get_container_id(struct fsl_mc_io *mc_io, int *container_id)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_CONT_ID,
+					  DPRC_CMDSZ_GET_CONT_ID,
+					  MC_CMD_PRI_LOW, 0);
+
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_GET_CONTAINER_ID(cmd, *container_id);
+	return 0;
+}
+
+int dprc_open(struct fsl_mc_io *mc_io, int container_id, uint16_t *dprc_handle)
+{
+	int err;
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(MC_DPRC_CMDID_OPEN,
+					  MC_CMD_OPEN_SIZE, MC_CMD_PRI_LOW, 0);
+
+	DPRC_CMD_OPEN(cmd, container_id);
+
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	*dprc_handle = MC_CMD_HDR_READ_AUTHID(cmd.header);
+	return 0;
+}
+
+int dprc_close(struct fsl_mc_io *mc_io, uint16_t dprc_handle)
+{
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(MC_CMDID_CLOSE,
+					  MC_CMD_CLOSE_SIZE,
+					  MC_CMD_PRI_HIGH, dprc_handle);
+
+	return mc_send_command(mc_io, &cmd);
+}
+
+int dprc_create_container(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			  struct dprc_cfg *cfg,
+			  int *child_container_id,
+			  uint64_t *child_portal_paddr)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_CREATE_CONT,
+					  DPRC_CMDSZ_CREATE_CONT,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_CREATE_CONTAINER(cmd, cfg);
+
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_CREATE_CONTAINER(cmd, *child_container_id,
+				  *child_portal_paddr);
+
+	return 0;
+}
+
+int dprc_destroy_container(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			   int child_container_id)
+{
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_DESTROY_CONT,
+					  DPRC_CMDSZ_DESTROY_CONT,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_DESTROY_CONTAINER(cmd, child_container_id);
+	return mc_send_command(mc_io, &cmd);
+}
+
+int dprc_reset_container(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			 int child_container_id)
+{
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_RESET_CONT,
+					  DPRC_CMDSZ_RESET_CONT,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_RESET_CONTAINER(cmd, child_container_id);
+	return mc_send_command(mc_io, &cmd);
+}
+
+int dprc_set_res_quota(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		       int child_container_id, char *type, uint16_t quota)
+{
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_SET_RES_QUOTA,
+					  DPRC_CMDSZ_SET_RES_QUOTA,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_SET_RES_QUOTA(cmd, child_container_id, type, quota);
+	return mc_send_command(mc_io, &cmd);
+}
+
+int dprc_get_res_quota(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		       int child_container_id, char *type, uint16_t *quota)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_RES_QUOTA,
+					  DPRC_CMDSZ_GET_RES_QUOTA,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_GET_RES_QUOTA(cmd, child_container_id, type);
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_GET_RES_QUOTA(cmd, *quota);
+	return 0;
+}
+
+int dprc_assign(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		int container_id, struct dprc_res_req *res_req)
+{
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_ASSIGN,
+					  DPRC_CMDSZ_ASSIGN,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_ASSIGN(cmd, container_id, res_req);
+	return mc_send_command(mc_io, &cmd);
+}
+
+int dprc_unassign(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		  int child_container_id, struct dprc_res_req *res_req)
+{
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_UNASSIGN,
+					  DPRC_CMDSZ_UNASSIGN,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_UNASSIGN(cmd, child_container_id, res_req);
+	return mc_send_command(mc_io, &cmd);
+}
+
+int dprc_get_obj_count(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		       int *obj_count)
+{
+	int err;
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_OBJ_COUNT,
+					  DPRC_CMDSZ_GET_OBJ_COUNT,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_GET_OBJ_COUNT(cmd, *obj_count);
+	return 0;
+}
+
+int dprc_get_obj(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		 int obj_index, struct dprc_obj_desc *obj_desc)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_OBJECT,
+					  DPRC_CMDSZ_GET_OBJECT,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_GET_OBJECT(cmd, obj_index);
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_GET_OBJECT(cmd, obj_desc);
+	return 0;
+}
+
+int dprc_get_res_count(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		       char *type, int *res_count)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	*res_count = 0;
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_RES_COUNT,
+					  DPRC_CMDSZ_GET_RES_COUNT,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_GET_RES_COUNT(cmd, type);
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_GET_RES_COUNT(cmd, *res_count);
+	return 0;
+}
+
+int dprc_get_res_ids(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		     char *type, struct dprc_res_ids_range_desc *range_desc)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_RES_IDS,
+					  DPRC_CMDSZ_GET_RES_IDS,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_GET_RES_IDS(cmd, type, range_desc);
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_GET_RES_IDS(cmd, range_desc);
+	return 0;
+}
+
+int dprc_get_attributes(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			struct dprc_attributes *attr)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_ATTR,
+					  DPRC_CMDSZ_GET_ATTR,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_GET_ATTRIBUTES(cmd, attr);
+	return 0;
+}
+
+int dprc_get_obj_region(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			char *obj_type,
+			int obj_id,
+			uint8_t region_index,
+			struct dprc_region_desc *region_desc)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_OBJ_REG,
+					  DPRC_CMDSZ_GET_OBJ_REG,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_GET_OBJ_REGION(cmd, obj_id, region_index, obj_type);
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_GET_OBJ_REGION(cmd, region_desc);
+	return 0;
+}
+
+int dprc_get_irq(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		 uint8_t irq_index,
+		 int *type,
+		 uint64_t *irq_paddr, uint32_t *irq_val, int *user_irq_id)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_IRQ,
+					  DPRC_CMDSZ_GET_IRQ,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_GET_IRQ(cmd, irq_index);
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_GET_IRQ(cmd, *irq_val, *irq_paddr, *user_irq_id, *type);
+	return 0;
+}
+
+int dprc_set_irq(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		 uint8_t irq_index,
+		 uint64_t irq_paddr, uint32_t irq_val, int user_irq_id)
+{
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_SET_IRQ,
+					  DPRC_CMDSZ_SET_IRQ,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_SET_IRQ(cmd, irq_index, irq_val, irq_paddr, user_irq_id);
+	return mc_send_command(mc_io, &cmd);
+}
+
+int dprc_get_irq_enable(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			uint8_t irq_index, uint8_t *enable_state)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_IRQ_ENABLE,
+					  DPRC_CMDSZ_GET_IRQ_ENABLE,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_GET_IRQ_ENABLE(cmd, irq_index);
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_GET_IRQ_ENABLE(cmd, *enable_state);
+	return 0;
+}
+
+int dprc_set_irq_enable(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			uint8_t irq_index, uint8_t enable_state)
+{
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_SET_IRQ_ENABLE,
+					  DPRC_CMDSZ_SET_IRQ_ENABLE,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_SET_IRQ_ENABLE(cmd, enable_state, irq_index);
+	return mc_send_command(mc_io, &cmd);
+}
+
+int dprc_get_irq_mask(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		      uint8_t irq_index, uint32_t *mask)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_IRQ_MASK,
+					  DPRC_CMDSZ_GET_IRQ_MASK,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_GET_IRQ_MASK(cmd, irq_index);
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_GET_IRQ_MASK(cmd, *mask);
+	return 0;
+}
+
+int dprc_set_irq_mask(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		      uint8_t irq_index, uint32_t mask)
+{
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_SET_IRQ_MASK,
+					  DPRC_CMDSZ_SET_IRQ_MASK,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_SET_IRQ_MASK(cmd, mask, irq_index);
+	return mc_send_command(mc_io, &cmd);
+}
+
+int dprc_get_irq_status(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			uint8_t irq_index, uint32_t *status)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_IRQ_STATUS,
+					  DPRC_CMDSZ_GET_IRQ_STATUS,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_GET_IRQ_STATUS(cmd, irq_index);
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_GET_IRQ_STATUS(cmd, *status);
+	return 0;
+}
+
+int dprc_clear_irq_status(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			  uint8_t irq_index, uint32_t status)
+{
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_CLEAR_IRQ_STATUS,
+					  DPRC_CMDSZ_CLEAR_IRQ_STATUS,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_CLEAR_IRQ_STATUS(cmd, status, irq_index);
+	return mc_send_command(mc_io, &cmd);
+}
+
+int dprc_get_pool_count(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			int *pool_count)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_POOL_COUNT,
+					  DPRC_CMDSZ_GET_POOL_COUNT,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_GET_POOL_COUNT(cmd, *pool_count);
+	return 0;
+}
+
+int dprc_get_pool(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		  int pool_index, char *type)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_POOL,
+					  DPRC_CMDSZ_GET_POOL,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_GET_POOL(cmd, pool_index);
+
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_GET_POOL(cmd, type);
+	return 0;
+}
+
+int dprc_get_portal_paddr(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			  int portal_id, uint64_t *portal_addr)
+{
+	struct mc_command cmd = { 0 };
+	int err;
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_PORTAL_PADDR,
+					  DPRC_CMDSZ_GET_PORTAL_PADDR,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_GET_PORTAL_PADDR(cmd, portal_id);
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return err;
+
+	DPRC_RSP_GET_PORTAL_PADDR(cmd, *portal_addr);
+	return 0;
+}
+
+int dprc_connect(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		 struct dprc_endpoint *endpoint1,
+		 struct dprc_endpoint *endpoint2)
+{
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_CONNECT,
+					  DPRC_CMDSZ_CONNECT,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_CONNECT(cmd, endpoint1, endpoint2);
+	return mc_send_command(mc_io, &cmd);
+}
+
+int dprc_disconnect(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		    struct dprc_endpoint *endpoint)
+{
+	struct mc_command cmd = { 0 };
+
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_DISCONNECT,
+					  DPRC_CMDSZ_DISCONNECT,
+					  MC_CMD_PRI_LOW, dprc_handle);
+
+	DPRC_CMD_DISCONNECT(cmd, endpoint);
+	return mc_send_command(mc_io, &cmd);
+}
diff --git a/drivers/bus/fsl-mc/fsl_dpmng_cmd.h b/drivers/bus/fsl-mc/fsl_dpmng_cmd.h
new file mode 100644
index 0000000..5445455
--- /dev/null
+++ b/drivers/bus/fsl-mc/fsl_dpmng_cmd.h
@@ -0,0 +1,83 @@
+/* Copyright 2013-2014 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *     * Neither the name of Freescale Semiconductor nor the
+ *       names of its contributors may be used to endorse or promote products
+ *       derived from this software without specific prior written permission.
+ *
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/**************************************************************************//*
+ @File          fsl_dpmng_cmd.h
+
+ @Description   defines portal commands
+
+ @Cautions      None.
+ *//***************************************************************************/
+
+#ifndef __FSL_DPMNG_CMD_H
+#define __FSL_DPMNG_CMD_H
+
+/* Command IDs */
+#define DPMNG_CMDID_GET_VERSION			0x831
+#define DPMNG_CMDID_RESET_AIOP			0x832
+#define DPMNG_CMDID_LOAD_AIOP			0x833
+#define DPMNG_CMDID_RUN_AIOP			0x834
+
+/* Command sizes */
+#define DPMNG_CMDSZ_GET_VERSION			(8 * 2)
+#define DPMNG_CMDSZ_RESET_AIOP			8
+#define DPMNG_CMDSZ_LOAD_AIOP			(8 * 2)
+#define DPMNG_CMDSZ_RUN_AIOP			(8 * 2)
+
+/*	param, offset, width,	type,		arg_name */
+#define DPMNG_RSP_GET_VERSION(cmd, mc_ver_info) \
+do { \
+	MC_RSP_PARAM_OP(cmd, 0,	0,	32, uint32_t,	mc_ver_info->revision);\
+	MC_RSP_PARAM_OP(cmd, 0,	32,	32, uint32_t,	mc_ver_info->major); \
+	MC_RSP_PARAM_OP(cmd, 1,	0,	8,  uint32_t,	mc_ver_info->minor); \
+} while (0)
+
+/*	param, offset, width,	type,		arg_name */
+#define DPMNG_CMD_RESET_AIOP(cmd, aiop_tile_id) \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	32,	int,	aiop_tile_id)
+
+/*	param, offset, width,	type,		arg_name */
+#define DPMNG_CMD_LOAD_AIOP(cmd, aiop_tile_id, img_size, img_paddr) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	32, int,	aiop_tile_id); \
+	MC_CMD_PARAM_OP(cmd, 0,	32,	32, int,	img_size); \
+	MC_CMD_PARAM_OP(cmd, 1,	0,	64, uint64_t,	img_paddr); \
+} while (0)
+
+/*	param, offset, width,	type,		arg_name */
+#define DPMNG_CMD_RUN_AIOP(cmd, aiop_tile_id, cores_mask, options) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	32,	int,		aiop_tile_id); \
+	MC_CMD_PARAM_OP(cmd, 0,	32,	32,	uint32_t,	cores_mask); \
+	MC_CMD_PARAM_OP(cmd, 1,	0,	64,	uint64_t,	options); \
+} while (0)
+
+#endif /* __FSL_DPMNG_CMD_H */
diff --git a/drivers/bus/fsl-mc/fsl_dprc_cmd.h b/drivers/bus/fsl-mc/fsl_dprc_cmd.h
new file mode 100644
index 0000000..e4d1fdc
--- /dev/null
+++ b/drivers/bus/fsl-mc/fsl_dprc_cmd.h
@@ -0,0 +1,545 @@
+/* Copyright 2013-2014 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *     * Neither the name of Freescale Semiconductor nor the
+ *       names of its contributors may be used to endorse or promote products
+ *       derived from this software without specific prior written permission.
+ *
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/**************************************************************************//*
+ @File          fsl_dprc_cmd.h
+
+ @Description   defines dprc portal commands
+
+ @Cautions      None.
+ *//***************************************************************************/
+
+#ifndef _FSL_DPRC_CMD_H
+#define _FSL_DPRC_CMD_H
+
+/* DPRC Version */
+#define DPRC_VER_MAJOR				1
+#define DPRC_VER_MINOR				0
+
+/* Command IDs */
+#define MC_CMDID_CLOSE				0x800
+#define MC_DPRC_CMDID_OPEN			0x805
+#define MC_DPRC_CMDID_CREATE			0x905
+
+#define DPRC_CMDID_CREATE_CONT			0x151
+#define DPRC_CMDID_DESTROY_CONT			0x152
+#define DPRC_CMDID_GET_CONT_ID			0x830
+#define DPRC_CMDID_RESET_CONT			0x154
+#define DPRC_CMDID_SET_RES_QUOTA		0x155
+#define DPRC_CMDID_GET_RES_QUOTA		0x156
+#define DPRC_CMDID_ASSIGN			0x157
+#define DPRC_CMDID_UNASSIGN			0x158
+#define DPRC_CMDID_GET_OBJ_COUNT		0x159
+#define DPRC_CMDID_GET_OBJECT			0x15A
+#define DPRC_CMDID_GET_RES_COUNT		0x15B
+#define DPRC_CMDID_GET_RES_IDS			0x15C
+#define DPRC_CMDID_GET_ATTR			0x15D
+#define DPRC_CMDID_GET_OBJ_REG			0x15E
+#define DPRC_CMDID_SET_IRQ			0x15F
+#define DPRC_CMDID_GET_IRQ			0x160
+#define DPRC_CMDID_SET_IRQ_ENABLE		0x161
+#define DPRC_CMDID_GET_IRQ_ENABLE		0x162
+#define DPRC_CMDID_SET_IRQ_MASK			0x163
+#define DPRC_CMDID_GET_IRQ_MASK			0x164
+#define DPRC_CMDID_GET_IRQ_STATUS		0x165
+#define DPRC_CMDID_CLEAR_IRQ_STATUS		0x166
+#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
+
+/* Command sizes */
+#define MC_CMD_OPEN_SIZE			8
+#define MC_CMD_CLOSE_SIZE			0
+#define DPRC_CMDSZ_CREATE_CONT			(8 * 2)
+#define DPRC_CMDSZ_DESTROY_CONT			8
+#define DPRC_CMDSZ_GET_CONT_ID			8
+#define DPRC_CMDSZ_RESET_CONT			8
+#define DPRC_CMDSZ_SET_RES_QUOTA		(8 * 3)
+#define DPRC_CMDSZ_GET_RES_QUOTA		(8 * 3)
+#define DPRC_CMDSZ_ASSIGN			(8 * 4)
+#define DPRC_CMDSZ_UNASSIGN			(8 * 4)
+#define DPRC_CMDSZ_GET_OBJ_COUNT		0
+#define DPRC_CMDSZ_GET_OBJECT			(8 * 5)
+#define DPRC_CMDSZ_GET_RES_COUNT		8
+#define DPRC_CMDSZ_GET_RES_IDS			(8 * 4)
+#define DPRC_CMDSZ_GET_ATTR			(8 * 3)
+#define DPRC_CMDSZ_GET_OBJ_REG			(8 * 5)
+#define DPRC_CMDSZ_SET_IRQ			(8 * 3)
+#define DPRC_CMDSZ_GET_IRQ			(8 * 3)
+#define DPRC_CMDSZ_SET_IRQ_ENABLE		8
+#define DPRC_CMDSZ_GET_IRQ_ENABLE		8
+#define DPRC_CMDSZ_SET_IRQ_MASK			8
+#define DPRC_CMDSZ_GET_IRQ_MASK			8
+#define DPRC_CMDSZ_GET_IRQ_STATUS		8
+#define DPRC_CMDSZ_CLEAR_IRQ_STATUS		8
+#define DPRC_CMDSZ_CONNECT			(8 * 7)
+#define DPRC_CMDSZ_DISCONNECT			(8 * 3)
+#define DPRC_CMDSZ_GET_POOL			(8 * 3)
+#define DPRC_CMDSZ_GET_POOL_COUNT		8
+#define DPRC_CMDSZ_GET_PORTAL_PADDR		(8 * 2)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_OPEN(cmd, container_id) \
+	MC_CMD_PARAM_OP(cmd, 0, 0, 32,	int, container_id)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_RSP_GET_CONTAINER_ID(cmd, container_id) \
+	MC_RSP_PARAM_OP(cmd, 0,  0, 32, int, container_id)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_CREATE_CONTAINER(cmd, cfg) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 0, 32, 16, uint16_t, cfg->icid); \
+	MC_CMD_PARAM_OP(cmd, 0, 0,  32, uint32_t, cfg->options); \
+	MC_CMD_PARAM_OP(cmd, 1, 32, 32, int,	  cfg->portal_id); \
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_RSP_CREATE_CONTAINER(cmd, child_container_id, child_portal_paddr) \
+do { \
+	MC_RSP_PARAM_OP(cmd, 1, 0, 32,	int,	  child_container_id); \
+	MC_RSP_PARAM_OP(cmd, 2, 0, 64,	uint64_t, child_portal_paddr); \
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_DESTROY_CONTAINER(cmd, child_container_id) \
+	MC_CMD_PARAM_OP(cmd, 0,	0, 32, int, child_container_id)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_RESET_CONTAINER(cmd, child_container_id) \
+	MC_CMD_PARAM_OP(cmd, 0,	0, 32, int, child_container_id)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_SET_RES_QUOTA(cmd, child_container_id, type, quota) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 0,	 0,	32,	int,	  child_container_id); \
+	MC_CMD_PARAM_OP(cmd, 0,  32,	16,	uint16_t, quota);\
+	MC_CMD_PARAM_OP(cmd, 1,  0,	8,	char,	  type[0]);\
+	MC_CMD_PARAM_OP(cmd, 1,  8,	8,	char,	  type[1]);\
+	MC_CMD_PARAM_OP(cmd, 1,  16,	8,	char,	  type[2]);\
+	MC_CMD_PARAM_OP(cmd, 1,  24,	8,	char,	  type[3]);\
+	MC_CMD_PARAM_OP(cmd, 1,  32,	8,	char,	  type[4]);\
+	MC_CMD_PARAM_OP(cmd, 1,  40,	8,	char,	  type[5]);\
+	MC_CMD_PARAM_OP(cmd, 1,  48,	8,	char,	  type[6]);\
+	MC_CMD_PARAM_OP(cmd, 1,  56,	8,	char,	  type[7]);\
+	MC_CMD_PARAM_OP(cmd, 2,  0,	8,	char,	  type[8]);\
+	MC_CMD_PARAM_OP(cmd, 2,  8,	8,	char,	  type[9]);\
+	MC_CMD_PARAM_OP(cmd, 2,  16,	8,	char,	  type[10]);\
+	MC_CMD_PARAM_OP(cmd, 2,  24,	8,	char,	  type[11]);\
+	MC_CMD_PARAM_OP(cmd, 2,  32,	8,	char,	  type[12]);\
+	MC_CMD_PARAM_OP(cmd, 2,  40,	8,	char,	  type[13]);\
+	MC_CMD_PARAM_OP(cmd, 2,  48,	8,	char,	  type[14]);\
+	MC_CMD_PARAM_OP(cmd, 2,  56,	8,	char,	  type[15]);\
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_GET_RES_QUOTA(cmd, child_container_id, type) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	32,	int,	child_container_id); \
+	MC_CMD_PARAM_OP(cmd, 1,  0,	8,	char,	type[0]);\
+	MC_CMD_PARAM_OP(cmd, 1,  8,	8,	char,	type[1]);\
+	MC_CMD_PARAM_OP(cmd, 1,  16,	8,	char,	type[2]);\
+	MC_CMD_PARAM_OP(cmd, 1,  24,	8,	char,	type[3]);\
+	MC_CMD_PARAM_OP(cmd, 1,  32,	8,	char,	type[4]);\
+	MC_CMD_PARAM_OP(cmd, 1,  40,	8,	char,	type[5]);\
+	MC_CMD_PARAM_OP(cmd, 1,  48,	8,	char,	type[6]);\
+	MC_CMD_PARAM_OP(cmd, 1,  56,	8,	char,	type[7]);\
+	MC_CMD_PARAM_OP(cmd, 2,  0,	8,	char,	type[8]);\
+	MC_CMD_PARAM_OP(cmd, 2,  8,	8,	char,	type[9]);\
+	MC_CMD_PARAM_OP(cmd, 2,  16,	8,	char,	type[10]);\
+	MC_CMD_PARAM_OP(cmd, 2,  24,	8,	char,	type[11]);\
+	MC_CMD_PARAM_OP(cmd, 2,  32,	8,	char,	type[12]);\
+	MC_CMD_PARAM_OP(cmd, 2,  40,	8,	char,	type[13]);\
+	MC_CMD_PARAM_OP(cmd, 2,  48,	8,	char,	type[14]);\
+	MC_CMD_PARAM_OP(cmd, 2,  56,	8,	char,	type[15]);\
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_RSP_GET_RES_QUOTA(cmd, quota) \
+	MC_RSP_PARAM_OP(cmd, 0,	32,	16,	uint16_t,	quota)
+
+/*	param, offset, width,	type,		arg_name */
+#define DPRC_CMD_ASSIGN(cmd, container_id, res_req) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	32, int,      container_id); \
+	MC_CMD_PARAM_OP(cmd, 0,	32,	32, uint32_t, res_req->options);\
+	MC_CMD_PARAM_OP(cmd, 1,	0,	32, uint32_t, res_req->num); \
+	MC_CMD_PARAM_OP(cmd, 1,	32,	32, int,      res_req->id_base_align); \
+	MC_CMD_PARAM_OP(cmd, 2,	0,	8,  char,     res_req->type[0]);\
+	MC_CMD_PARAM_OP(cmd, 2,	8,	8,  char,     res_req->type[1]);\
+	MC_CMD_PARAM_OP(cmd, 2,	16,	8,  char,     res_req->type[2]);\
+	MC_CMD_PARAM_OP(cmd, 2,	24,	8,  char,     res_req->type[3]);\
+	MC_CMD_PARAM_OP(cmd, 2,	32,	8,  char,     res_req->type[4]);\
+	MC_CMD_PARAM_OP(cmd, 2,	40,	8,  char,     res_req->type[5]);\
+	MC_CMD_PARAM_OP(cmd, 2,	48,	8,  char,     res_req->type[6]);\
+	MC_CMD_PARAM_OP(cmd, 2,	56,	8,  char,     res_req->type[7]);\
+	MC_CMD_PARAM_OP(cmd, 3,	0,	8,  char,     res_req->type[8]);\
+	MC_CMD_PARAM_OP(cmd, 3,	8,	8,  char,     res_req->type[9]);\
+	MC_CMD_PARAM_OP(cmd, 3,	16,	8,  char,     res_req->type[10]);\
+	MC_CMD_PARAM_OP(cmd, 3,	24,	8,  char,     res_req->type[11]);\
+	MC_CMD_PARAM_OP(cmd, 3,	32,	8,  char,     res_req->type[12]);\
+	MC_CMD_PARAM_OP(cmd, 3,	40,	8,  char,     res_req->type[13]);\
+	MC_CMD_PARAM_OP(cmd, 3,	48,	8,  char,     res_req->type[14]);\
+	MC_CMD_PARAM_OP(cmd, 3,	56,	8,  char,     res_req->type[15]);\
+} while (0)
+
+/*	param, offset, width,	type,		arg_name */
+#define DPRC_CMD_UNASSIGN(cmd, child_container_id, res_req) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	32, int,     child_container_id); \
+	MC_CMD_PARAM_OP(cmd, 0,	32,	32, uint32_t, res_req->options);\
+	MC_CMD_PARAM_OP(cmd, 1,	0,	32, uint32_t, res_req->num); \
+	MC_CMD_PARAM_OP(cmd, 1,	32,	32, int,      res_req->id_base_align); \
+	MC_CMD_PARAM_OP(cmd, 2,	0,	8,  char,     res_req->type[0]);\
+	MC_CMD_PARAM_OP(cmd, 2,	8,	8,  char,     res_req->type[1]);\
+	MC_CMD_PARAM_OP(cmd, 2,	16,	8,  char,     res_req->type[2]);\
+	MC_CMD_PARAM_OP(cmd, 2,	24,	8,  char,     res_req->type[3]);\
+	MC_CMD_PARAM_OP(cmd, 2,	32,	8,  char,     res_req->type[4]);\
+	MC_CMD_PARAM_OP(cmd, 2,	40,	8,  char,     res_req->type[5]);\
+	MC_CMD_PARAM_OP(cmd, 2,	48,	8,  char,     res_req->type[6]);\
+	MC_CMD_PARAM_OP(cmd, 2,	56,	8,  char,     res_req->type[7]);\
+	MC_CMD_PARAM_OP(cmd, 3,	0,	8,  char,     res_req->type[8]);\
+	MC_CMD_PARAM_OP(cmd, 3,	8,	8,  char,     res_req->type[9]);\
+	MC_CMD_PARAM_OP(cmd, 3,	16,	8,  char,     res_req->type[10]);\
+	MC_CMD_PARAM_OP(cmd, 3,	24,	8,  char,     res_req->type[11]);\
+	MC_CMD_PARAM_OP(cmd, 3,	32,	8,  char,     res_req->type[12]);\
+	MC_CMD_PARAM_OP(cmd, 3,	40,	8,  char,     res_req->type[13]);\
+	MC_CMD_PARAM_OP(cmd, 3,	48,	8,  char,     res_req->type[14]);\
+	MC_CMD_PARAM_OP(cmd, 3,	56,	8,  char,     res_req->type[15]);\
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_RSP_GET_OBJ_COUNT(cmd, obj_count) \
+	MC_RSP_PARAM_OP(cmd, 0,	32,	32,	int,	obj_count)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_GET_OBJECT(cmd, obj_index) \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	32,	int,	obj_index)
+
+/*	param, offset, width,	type,		arg_name */
+#define DPRC_RSP_GET_OBJECT(cmd, obj_desc) \
+do { \
+	MC_RSP_PARAM_OP(cmd, 0,	32,	32, int,      obj_desc->id); \
+	MC_RSP_PARAM_OP(cmd, 1,	0,	16, uint16_t, obj_desc->vendor); \
+	MC_RSP_PARAM_OP(cmd, 1,	16,	8,  uint8_t,  obj_desc->irq_count); \
+	MC_RSP_PARAM_OP(cmd, 1,	24,	8,  uint8_t,  obj_desc->region_count); \
+	MC_RSP_PARAM_OP(cmd, 1,	32,	32, uint32_t, obj_desc->state);\
+	MC_RSP_PARAM_OP(cmd, 2,	0,	32, uint32_t, obj_desc->ver_minor); \
+	MC_RSP_PARAM_OP(cmd, 2,	32,	32, uint32_t, obj_desc->ver_major); \
+	MC_RSP_PARAM_OP(cmd, 3,  0,	8,  char,     obj_desc->type[0]);\
+	MC_RSP_PARAM_OP(cmd, 3,  8,	8,  char,     obj_desc->type[1]);\
+	MC_RSP_PARAM_OP(cmd, 3,  16,	8,  char,     obj_desc->type[2]);\
+	MC_RSP_PARAM_OP(cmd, 3,  24,	8,  char,     obj_desc->type[3]);\
+	MC_RSP_PARAM_OP(cmd, 3,  32,	8,  char,     obj_desc->type[4]);\
+	MC_RSP_PARAM_OP(cmd, 3,  40,	8,  char,     obj_desc->type[5]);\
+	MC_RSP_PARAM_OP(cmd, 3,  48,	8,  char,     obj_desc->type[6]);\
+	MC_RSP_PARAM_OP(cmd, 3,  56,	8,  char,     obj_desc->type[7]);\
+	MC_RSP_PARAM_OP(cmd, 4,  0,	8,  char,     obj_desc->type[8]);\
+	MC_RSP_PARAM_OP(cmd, 4,  8,	8,  char,     obj_desc->type[9]);\
+	MC_RSP_PARAM_OP(cmd, 4,  16,	8,  char,     obj_desc->type[10]);\
+	MC_RSP_PARAM_OP(cmd, 4,  24,	8,  char,     obj_desc->type[11]);\
+	MC_RSP_PARAM_OP(cmd, 4,  32,	8,  char,     obj_desc->type[12]);\
+	MC_RSP_PARAM_OP(cmd, 4,  40,	8,  char,     obj_desc->type[13]);\
+	MC_RSP_PARAM_OP(cmd, 4,  48,	8,  char,     obj_desc->type[14]);\
+	MC_RSP_PARAM_OP(cmd, 4,  56,	8,  char,     obj_desc->type[15]);\
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_GET_RES_COUNT(cmd, type) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 1,  0,	8,	char,		type[0]);\
+	MC_CMD_PARAM_OP(cmd, 1,  8,	8,	char,		type[1]);\
+	MC_CMD_PARAM_OP(cmd, 1,  16,	8,	char,		type[2]);\
+	MC_CMD_PARAM_OP(cmd, 1,  24,	8,	char,		type[3]);\
+	MC_CMD_PARAM_OP(cmd, 1,  32,	8,	char,		type[4]);\
+	MC_CMD_PARAM_OP(cmd, 1,  40,	8,	char,		type[5]);\
+	MC_CMD_PARAM_OP(cmd, 1,  48,	8,	char,		type[6]);\
+	MC_CMD_PARAM_OP(cmd, 1,  56,	8,	char,		type[7]);\
+	MC_CMD_PARAM_OP(cmd, 2,  0,	8,	char,		type[8]);\
+	MC_CMD_PARAM_OP(cmd, 2,  8,	8,	char,		type[9]);\
+	MC_CMD_PARAM_OP(cmd, 2,  16,	8,	char,		type[10]);\
+	MC_CMD_PARAM_OP(cmd, 2,  24,	8,	char,		type[11]);\
+	MC_CMD_PARAM_OP(cmd, 2,  32,	8,	char,		type[12]);\
+	MC_CMD_PARAM_OP(cmd, 2,  40,	8,	char,		type[13]);\
+	MC_CMD_PARAM_OP(cmd, 2,  48,	8,	char,		type[14]);\
+	MC_CMD_PARAM_OP(cmd, 2,  56,	8,	char,		type[15]);\
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_RSP_GET_RES_COUNT(cmd, res_count) \
+	MC_RSP_PARAM_OP(cmd, 0,	0,	32,	int,		res_count)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_GET_RES_IDS(cmd, type, range_desc) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 0,	42,	7,	enum dprc_iter_status,	\
+						      range_desc->iter_status);\
+	MC_CMD_PARAM_OP(cmd, 1,	0,	32,	int,  range_desc->base_id); \
+	MC_CMD_PARAM_OP(cmd, 1,	32,	32,	int,  range_desc->last_id);\
+	MC_CMD_PARAM_OP(cmd, 2,  0,	8,	char, type[0]);\
+	MC_CMD_PARAM_OP(cmd, 2,  8,	8,	char, type[1]);\
+	MC_CMD_PARAM_OP(cmd, 2,  16,	8,	char, type[2]);\
+	MC_CMD_PARAM_OP(cmd, 2,  24,	8,	char, type[3]);\
+	MC_CMD_PARAM_OP(cmd, 2,  32,	8,	char, type[4]);\
+	MC_CMD_PARAM_OP(cmd, 2,  40,	8,	char, type[5]);\
+	MC_CMD_PARAM_OP(cmd, 2,  48,	8,	char, type[6]);\
+	MC_CMD_PARAM_OP(cmd, 2,  56,	8,	char, type[7]);\
+	MC_CMD_PARAM_OP(cmd, 3,  0,	8,	char, type[8]);\
+	MC_CMD_PARAM_OP(cmd, 3,  8,	8,	char, type[9]);\
+	MC_CMD_PARAM_OP(cmd, 3,  16,	8,	char, type[10]);\
+	MC_CMD_PARAM_OP(cmd, 3,  24,	8,	char, type[11]);\
+	MC_CMD_PARAM_OP(cmd, 3,  32,	8,	char, type[12]);\
+	MC_CMD_PARAM_OP(cmd, 3,  40,	8,	char, type[13]);\
+	MC_CMD_PARAM_OP(cmd, 3,  48,	8,	char, type[14]);\
+	MC_CMD_PARAM_OP(cmd, 3,  56,	8,	char, type[15]);\
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_RSP_GET_RES_IDS(cmd, range_desc) \
+do { \
+	MC_RSP_PARAM_OP(cmd, 0,	42,	7,	enum dprc_iter_status,	\
+						     range_desc->iter_status);\
+	MC_RSP_PARAM_OP(cmd, 1,	0,	32,	int, range_desc->base_id); \
+	MC_RSP_PARAM_OP(cmd, 1,	32,	32,	int, range_desc->last_id);\
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_RSP_GET_ATTRIBUTES(cmd, attr) \
+do { \
+	MC_RSP_PARAM_OP(cmd, 0,	0,	32,	int,	  attr->container_id); \
+	MC_RSP_PARAM_OP(cmd, 0,	32,	16,	uint16_t, attr->icid); \
+	MC_RSP_PARAM_OP(cmd, 0,	48,	16,	uint16_t, attr->portal_id); \
+	MC_RSP_PARAM_OP(cmd, 1,	0,	32,	uint32_t, attr->options);\
+	MC_RSP_PARAM_OP(cmd, 2,  0,	32,	uint32_t, attr->version.major);\
+	MC_RSP_PARAM_OP(cmd, 2,  32,	32,	uint32_t, attr->version.minor);\
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_GET_OBJ_REGION(cmd, obj_id, region_index, obj_type) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	32,	int,		obj_id); \
+	MC_CMD_PARAM_OP(cmd, 0,	48,	8,	uint8_t,	region_index);\
+	MC_CMD_PARAM_OP(cmd, 3,  0,	8,	char,		obj_type[0]);\
+	MC_CMD_PARAM_OP(cmd, 3,  8,	8,	char,		obj_type[1]);\
+	MC_CMD_PARAM_OP(cmd, 3,  16,	8,	char,		obj_type[2]);\
+	MC_CMD_PARAM_OP(cmd, 3,  24,	8,	char,		obj_type[3]);\
+	MC_CMD_PARAM_OP(cmd, 3,  32,	8,	char,		obj_type[4]);\
+	MC_CMD_PARAM_OP(cmd, 3,  40,	8,	char,		obj_type[5]);\
+	MC_CMD_PARAM_OP(cmd, 3,  48,	8,	char,		obj_type[6]);\
+	MC_CMD_PARAM_OP(cmd, 3,  56,	8,	char,		obj_type[7]);\
+	MC_CMD_PARAM_OP(cmd, 4,  0,	8,	char,		obj_type[8]);\
+	MC_CMD_PARAM_OP(cmd, 4,  8,	8,	char,		obj_type[9]);\
+	MC_CMD_PARAM_OP(cmd, 4,  16,	8,	char,		obj_type[10]);\
+	MC_CMD_PARAM_OP(cmd, 4,  24,	8,	char,		obj_type[11]);\
+	MC_CMD_PARAM_OP(cmd, 4,  32,	8,	char,		obj_type[12]);\
+	MC_CMD_PARAM_OP(cmd, 4,  40,	8,	char,		obj_type[13]);\
+	MC_CMD_PARAM_OP(cmd, 4,  48,	8,	char,		obj_type[14]);\
+	MC_CMD_PARAM_OP(cmd, 4,  56,	8,	char,		obj_type[15]);\
+} while (0)
+
+/*	param, offset, width,	type,		arg_name */
+#define DPRC_RSP_GET_OBJ_REGION(cmd, region_desc) \
+do { \
+	MC_RSP_PARAM_OP(cmd, 1,	0,	64, uint64_t, region_desc->base_paddr);\
+	MC_RSP_PARAM_OP(cmd, 2,	0,	32, uint32_t, region_desc->size); \
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_SET_IRQ(cmd, irq_index, irq_val, irq_paddr, user_irq_id) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 0,	32,	8,	uint8_t,    irq_index); \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	32,	uint32_t,   irq_val); \
+	MC_CMD_PARAM_OP(cmd, 1,	0,	64,	uint64_t,   irq_paddr);\
+	MC_CMD_PARAM_OP(cmd, 2,  0,	32,	int,	    user_irq_id); \
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_GET_IRQ(cmd, irq_index) \
+	MC_CMD_PARAM_OP(cmd, 0,	32,	8,	uint8_t,	irq_index)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_RSP_GET_IRQ(cmd, irq_val, irq_paddr, user_irq_id, type) \
+do { \
+	MC_RSP_PARAM_OP(cmd, 0,	0,	32,	uint32_t,   irq_val); \
+	MC_RSP_PARAM_OP(cmd, 1,	0,	64,	uint64_t,   irq_paddr);\
+	MC_RSP_PARAM_OP(cmd, 2,  0,	32,	int,	    user_irq_id); \
+	MC_RSP_PARAM_OP(cmd, 2,	32,	32,	int,	    type); \
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_SET_IRQ_ENABLE(cmd, enable_state, irq_index) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	8,	uint8_t,    enable_state); \
+	MC_CMD_PARAM_OP(cmd, 0,	32,	8,	uint8_t,    irq_index);\
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_GET_IRQ_ENABLE(cmd, irq_index) \
+	MC_CMD_PARAM_OP(cmd, 0,	32,	8,	uint8_t,	irq_index)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_RSP_GET_IRQ_ENABLE(cmd, enable_state) \
+	MC_RSP_PARAM_OP(cmd, 0,	0,	8,	uint8_t,	enable_state)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_SET_IRQ_MASK(cmd, mask, irq_index) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	32,	uint32_t,	mask); \
+	MC_CMD_PARAM_OP(cmd, 0,	32,	8,	uint8_t,	irq_index);\
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_GET_IRQ_MASK(cmd, irq_index) \
+	MC_CMD_PARAM_OP(cmd, 0,	32,	8,	uint8_t,	irq_index)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_RSP_GET_IRQ_MASK(cmd, mask) \
+	MC_RSP_PARAM_OP(cmd, 0,	0,	32,	uint32_t,	mask)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_GET_IRQ_STATUS(cmd, irq_index) \
+	MC_CMD_PARAM_OP(cmd, 0,	32,	8,	uint8_t,	irq_index)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_RSP_GET_IRQ_STATUS(cmd, status) \
+	MC_RSP_PARAM_OP(cmd, 0,	0,	32,	uint32_t,	status)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_CLEAR_IRQ_STATUS(cmd, status, irq_index) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	32,	uint32_t,	status); \
+	MC_CMD_PARAM_OP(cmd, 0,	32,	8,	uint8_t,	irq_index);\
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_CONNECT(cmd, endpoint1, endpoint2) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	32, int,    endpoint1->id); \
+	MC_CMD_PARAM_OP(cmd, 0,	32,	32, int,    endpoint1->interface_id); \
+	MC_CMD_PARAM_OP(cmd, 1,	0,	32, int,    endpoint2->id); \
+	MC_CMD_PARAM_OP(cmd, 1,	32,	32, int,    endpoint2->interface_id); \
+	MC_CMD_PARAM_OP(cmd, 2,	0,	8,  char,   endpoint1->type[0]); \
+	MC_CMD_PARAM_OP(cmd, 2,	8,	8,  char,   endpoint1->type[1]); \
+	MC_CMD_PARAM_OP(cmd, 2,	16,	8,  char,   endpoint1->type[2]); \
+	MC_CMD_PARAM_OP(cmd, 2,	24,	8,  char,   endpoint1->type[3]); \
+	MC_CMD_PARAM_OP(cmd, 2,	32,	8,  char,   endpoint1->type[4]); \
+	MC_CMD_PARAM_OP(cmd, 2,	40,	8,  char,   endpoint1->type[5]); \
+	MC_CMD_PARAM_OP(cmd, 2,	48,	8,  char,   endpoint1->type[6]); \
+	MC_CMD_PARAM_OP(cmd, 2,	56,	8,  char,   endpoint1->type[7]); \
+	MC_CMD_PARAM_OP(cmd, 3,	0,	8,  char,   endpoint1->type[8]); \
+	MC_CMD_PARAM_OP(cmd, 3,	8,	8,  char,   endpoint1->type[9]); \
+	MC_CMD_PARAM_OP(cmd, 3,	16,	8,  char,   endpoint1->type[10]); \
+	MC_CMD_PARAM_OP(cmd, 3,	24,	8,  char,   endpoint1->type[11]); \
+	MC_CMD_PARAM_OP(cmd, 3,	32,	8,  char,   endpoint1->type[12]); \
+	MC_CMD_PARAM_OP(cmd, 3,	40,	8,  char,   endpoint1->type[13]); \
+	MC_CMD_PARAM_OP(cmd, 3,	48,	8,  char,   endpoint1->type[14]); \
+	MC_CMD_PARAM_OP(cmd, 3,	56,	8,  char,   endpoint1->type[15]); \
+	MC_CMD_PARAM_OP(cmd, 5,	0,	8,  char,   endpoint2->type[0]); \
+	MC_CMD_PARAM_OP(cmd, 5,	8,	8,  char,   endpoint2->type[1]); \
+	MC_CMD_PARAM_OP(cmd, 5,	16,	8,  char,   endpoint2->type[2]); \
+	MC_CMD_PARAM_OP(cmd, 5,	24,	8,  char,   endpoint2->type[3]); \
+	MC_CMD_PARAM_OP(cmd, 5,	32,	8,  char,   endpoint2->type[4]); \
+	MC_CMD_PARAM_OP(cmd, 5,	40,	8,  char,   endpoint2->type[5]); \
+	MC_CMD_PARAM_OP(cmd, 5,	48,	8,  char,   endpoint2->type[6]); \
+	MC_CMD_PARAM_OP(cmd, 5,	56,	8,  char,   endpoint2->type[7]); \
+	MC_CMD_PARAM_OP(cmd, 6,	0,	8,  char,   endpoint2->type[8]); \
+	MC_CMD_PARAM_OP(cmd, 6,	8,	8,  char,   endpoint2->type[9]); \
+	MC_CMD_PARAM_OP(cmd, 6,	16,	8,  char,   endpoint2->type[10]); \
+	MC_CMD_PARAM_OP(cmd, 6,	24,	8,  char,   endpoint2->type[11]); \
+	MC_CMD_PARAM_OP(cmd, 6,	32,	8,  char,   endpoint2->type[12]); \
+	MC_CMD_PARAM_OP(cmd, 6,	40,	8,  char,   endpoint2->type[13]); \
+	MC_CMD_PARAM_OP(cmd, 6,	48,	8,  char,   endpoint2->type[14]); \
+	MC_CMD_PARAM_OP(cmd, 6,	56,	8,  char,   endpoint2->type[15]); \
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_DISCONNECT(cmd, endpoint) \
+do { \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	32,	int,  endpoint->id); \
+	MC_CMD_PARAM_OP(cmd, 0,	32,	32,	int,  endpoint->interface_id); \
+	MC_CMD_PARAM_OP(cmd, 1,	0,	8,	char, endpoint->type[0]); \
+	MC_CMD_PARAM_OP(cmd, 1,	8,	8,	char, endpoint->type[1]); \
+	MC_CMD_PARAM_OP(cmd, 1,	16,	8,	char, endpoint->type[2]); \
+	MC_CMD_PARAM_OP(cmd, 1,	24,	8,	char, endpoint->type[3]); \
+	MC_CMD_PARAM_OP(cmd, 1,	32,	8,	char, endpoint->type[4]); \
+	MC_CMD_PARAM_OP(cmd, 1,	40,	8,	char, endpoint->type[5]); \
+	MC_CMD_PARAM_OP(cmd, 1,	48,	8,	char, endpoint->type[6]); \
+	MC_CMD_PARAM_OP(cmd, 1,	56,	8,	char, endpoint->type[7]); \
+	MC_CMD_PARAM_OP(cmd, 2,	0,	8,	char, endpoint->type[8]); \
+	MC_CMD_PARAM_OP(cmd, 2,	8,	8,	char, endpoint->type[9]); \
+	MC_CMD_PARAM_OP(cmd, 2,	16,	8,	char, endpoint->type[10]); \
+	MC_CMD_PARAM_OP(cmd, 2,	24,	8,	char, endpoint->type[11]); \
+	MC_CMD_PARAM_OP(cmd, 2,	32,	8,	char, endpoint->type[12]); \
+	MC_CMD_PARAM_OP(cmd, 2,	40,	8,	char, endpoint->type[13]); \
+	MC_CMD_PARAM_OP(cmd, 2,	48,	8,	char, endpoint->type[14]); \
+	MC_CMD_PARAM_OP(cmd, 2,	56,	8,	char, endpoint->type[15]); \
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_GET_POOL(cmd, pool_index) \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	32,	int,	pool_index)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_RSP_GET_POOL(cmd, type) \
+do { \
+	MC_RSP_PARAM_OP(cmd, 1,	0,	8,	char,		type[0]);\
+	MC_RSP_PARAM_OP(cmd, 1,	8,	8,	char,		type[1]);\
+	MC_RSP_PARAM_OP(cmd, 1,	16,	8,	char,		type[2]);\
+	MC_RSP_PARAM_OP(cmd, 1,	24,	8,	char,		type[3]);\
+	MC_RSP_PARAM_OP(cmd, 1,	32,	8,	char,		type[4]);\
+	MC_RSP_PARAM_OP(cmd, 1,	40,	8,	char,		type[5]);\
+	MC_RSP_PARAM_OP(cmd, 1,	48,	8,	char,		type[6]);\
+	MC_RSP_PARAM_OP(cmd, 1,	56,	8,	char,		type[7]);\
+	MC_RSP_PARAM_OP(cmd, 2,	0,	8,	char,		type[8]);\
+	MC_RSP_PARAM_OP(cmd, 2,	8,	8,	char,		type[9]);\
+	MC_RSP_PARAM_OP(cmd, 2,	16,	8,	char,		type[10]);\
+	MC_RSP_PARAM_OP(cmd, 2,	24,	8,	char,		type[11]);\
+	MC_RSP_PARAM_OP(cmd, 2,	32,	8,	char,		type[12]);\
+	MC_RSP_PARAM_OP(cmd, 2,	40,	8,	char,		type[13]);\
+	MC_RSP_PARAM_OP(cmd, 2,	48,	8,	char,		type[14]);\
+	MC_RSP_PARAM_OP(cmd, 2,	56,	8,	char,		type[15]);\
+} while (0)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_RSP_GET_POOL_COUNT(cmd, pool_count) \
+	MC_RSP_PARAM_OP(cmd, 0,	0,	32,	int,		pool_count)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_CMD_GET_PORTAL_PADDR(cmd, portal_id) \
+	MC_CMD_PARAM_OP(cmd, 0,	0,	32,	int,	portal_id)
+
+/*	param, offset, width,	type,			arg_name */
+#define DPRC_RSP_GET_PORTAL_PADDR(cmd, portal_addr) \
+	MC_RSP_PARAM_OP(cmd, 1,	0,	64,	uint64_t,	portal_addr)
+
+#endif /* _FSL_DPRC_CMD_H */
diff --git a/drivers/bus/fsl-mc/fsl_mc_sys.c b/drivers/bus/fsl-mc/fsl_mc_sys.c
new file mode 100644
index 0000000..5d252c0
--- /dev/null
+++ b/drivers/bus/fsl-mc/fsl_mc_sys.c
@@ -0,0 +1,195 @@
+/* Copyright 2014 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *     * Neither the name of Freescale Semiconductor nor the
+ *       names of its contributors may be used to endorse or promote products
+ *       derived from this software without specific prior written permission.
+ *
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#include <linux/fsl_mc_sys.h>
+#include <linux/fsl_mc_cmd.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/ioport.h>
+#include <linux/device.h>
+
+/**
+ * Timeout in jiffies to wait for the completion of an MC command
+ */
+#define MC_CMD_COMPLETION_TIMEOUT_JIFFIES   (HZ / 2)	/* 500 ms */
+
+/**
+ * Delay in microseconds between polling iterations while
+ * waiting for MC command completion
+ */
+#define MC_CMD_COMPLETION_POLLING_INTERVAL_USECS    500	/* 0.5 ms */
+
+int __must_check fsl_create_mc_io(struct device *dev,
+				  phys_addr_t mc_portal_phys_addr,
+				  uint32_t mc_portal_size,
+				  uint32_t flags, struct fsl_mc_io **new_mc_io)
+{
+	struct fsl_mc_io *mc_io;
+	void __iomem *mc_portal_virt_addr;
+	struct resource *res;
+
+	mc_io = devm_kzalloc(dev, sizeof(*mc_io), GFP_KERNEL);
+	if (mc_io == NULL)
+		return -ENOMEM;
+
+	mc_io->dev = dev;
+	mc_io->flags = flags;
+	mc_io->portal_phys_addr = mc_portal_phys_addr;
+	mc_io->portal_size = mc_portal_size;
+	if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS)
+		spin_lock_init(&mc_io->spinlock);
+	else if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS)
+		mutex_init(&mc_io->mutex);
+
+	res = devm_request_mem_region(dev,
+				      mc_portal_phys_addr,
+				      mc_portal_size,
+				      "mc_portal");
+	if (res == NULL) {
+		dev_err(dev,
+			"devm_request_mem_region failed for MC portal %#llx\n",
+			mc_portal_phys_addr);
+		return -EBUSY;
+	}
+
+	mc_portal_virt_addr = devm_ioremap_nocache(dev,
+						   mc_portal_phys_addr,
+						   mc_portal_size);
+	if (mc_portal_virt_addr == NULL) {
+		dev_err(dev,
+			"devm_ioremap_nocache failed for MC portal %#llx\n",
+			mc_portal_phys_addr);
+		return -ENXIO;
+	}
+
+	mc_io->portal_virt_addr = mc_portal_virt_addr;
+	*new_mc_io = mc_io;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fsl_create_mc_io);
+
+void fsl_destroy_mc_io(struct fsl_mc_io *mc_io)
+{
+	if (WARN_ON(mc_io->portal_virt_addr == NULL))
+		return;
+
+	devm_iounmap(mc_io->dev, mc_io->portal_virt_addr);
+	devm_release_mem_region(mc_io->dev,
+				mc_io->portal_phys_addr,
+				mc_io->portal_size);
+
+	mc_io->portal_virt_addr = NULL;
+	devm_kfree(mc_io->dev, mc_io);
+}
+EXPORT_SYMBOL_GPL(fsl_destroy_mc_io);
+
+static int mc_status_to_error(enum mc_cmd_status status)
+{
+	static const int mc_status_to_error_map[] = {
+		[MC_CMD_STATUS_OK] = 0,
+		[MC_CMD_STATUS_AUTH_ERR] = -EACCES,
+		[MC_CMD_STATUS_NO_PRIVILEGE] = -EPERM,
+		[MC_CMD_STATUS_DMA_ERR] = -EIO,
+		[MC_CMD_STATUS_CONFIG_ERR] = -ENXIO,
+		[MC_CMD_STATUS_TIMEOUT] = -ETIMEDOUT,
+		[MC_CMD_STATUS_NO_RESOURCE] = -ENAVAIL,
+		[MC_CMD_STATUS_NO_MEMORY] = -ENOMEM,
+		[MC_CMD_STATUS_BUSY] = -EBUSY,
+		[MC_CMD_STATUS_UNSUPPORTED_OP] = -ENOTSUPP,
+		[MC_CMD_STATUS_INVALID_STATE] = -ENODEV,
+	};
+
+	if (WARN_ON(status >= ARRAY_SIZE(mc_status_to_error_map)))
+		return -EINVAL;
+
+	return mc_status_to_error_map[status];
+}
+
+int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
+{
+	enum mc_cmd_status status;
+	int error;
+	unsigned long irqsave_flags = 0;
+	unsigned long jiffies_until_timeout =
+	    jiffies + MC_CMD_COMPLETION_TIMEOUT_JIFFIES;
+
+	/*
+	 * Acquire lock depending on mc_io flags:
+	 */
+	if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS) {
+		if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS)
+			spin_lock_irqsave(&mc_io->spinlock, irqsave_flags);
+		else
+			spin_lock(&mc_io->spinlock);
+	} else if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS) {
+		mutex_lock(&mc_io->mutex);
+	}
+
+	/*
+	 * Send command to the MC hardware:
+	 */
+	mc_write_command(mc_io->portal_virt_addr, 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 udelay()
+		 */
+		udelay(MC_CMD_COMPLETION_POLLING_INTERVAL_USECS);
+		if (time_after_eq(jiffies, jiffies_until_timeout)) {
+			error = -ETIMEDOUT;
+			goto out;
+		}
+	}
+
+	error = mc_status_to_error(status);
+out:
+	/*
+	 * Release lock depending on mc_io flags:
+	 */
+	if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS) {
+		if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS)
+			spin_unlock_irqrestore(&mc_io->spinlock, irqsave_flags);
+		else
+			spin_unlock(&mc_io->spinlock);
+	} else if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS) {
+		mutex_unlock(&mc_io->mutex);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(mc_send_command);
diff --git a/include/linux/fsl_dpmng.h b/include/linux/fsl_dpmng.h
new file mode 100644
index 0000000..68af9de
--- /dev/null
+++ b/include/linux/fsl_dpmng.h
@@ -0,0 +1,120 @@
+/* Copyright 2013-2014 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *     * Neither the name of Freescale Semiconductor nor the
+ *       names of its contributors may be used to endorse or promote products
+ *       derived from this software without specific prior written permission.
+ *
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/*!
+ *  @file    fsl_dpmng.h
+ *  @brief   Management Complex General API
+ */
+
+#ifndef __FSL_DPMNG_H
+#define __FSL_DPMNG_H
+
+/*!
+ * @Group grp_dpmng	Management Complex General API
+ *
+ * @brief	Contains general API for the Management Complex firmware
+ * @{
+ */
+
+struct fsl_mc_io;
+
+/**
+ * @brief	Management Complex firmware version information
+ */
+#define MC_VER_MAJOR 2
+#define MC_VER_MINOR 0
+
+struct mc_version {
+	uint32_t major;
+	/*!<
+	 * Major version number: incremented on API compatibility changes
+	 */
+	uint32_t minor;
+	/*!<
+	 * Minor version number: incremented on API additions (backward
+	 * compatible); reset when major version is incremented.
+	 */
+	uint32_t revision;
+	/*!<
+	 * Internal revision number: incremented on implementation changes
+	 * and/or bug fixes that have no impact on API
+	 */
+};
+
+/**
+ * @brief	Retrieves the Management Complex firmware version information
+ *
+ * @param[in]   mc_io - Pointer to opaque I/O object
+ * @param[out]	mc_ver_info - Pointer to version information structure
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int mc_get_version(struct fsl_mc_io *mc_io, struct mc_version *mc_ver_info);
+
+/**
+ * @brief	Resets an AIOP tile
+ *
+ * @param[in]   mc_io - Pointer to opaque I/O object
+ * @param[in]	aiop_tile_id - AIOP tile ID to reset
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dpmng_reset_aiop(struct fsl_mc_io *mc_io, int aiop_tile_id);
+
+/**
+ * @brief	Loads an image to AIOP tile
+ *
+ * @param[in]   mc_io - Pointer to opaque I/O object
+ * @param[in]	aiop_tile_id - AIOP tile ID
+ * @param[in]	img_addr - Pointer to AIOP ELF image in memory
+ * @param[in]	img_size - Size in bytes of AIOP ELF image in memory
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dpmng_load_aiop(struct fsl_mc_io *mc_io,
+		    int aiop_tile_id, uint8_t *img_addr, int img_size);
+
+/**
+ * @brief	Starts AIOP tile execution
+ *
+ * @param[in]   mc_io - Pointer to opaque I/O object
+ * @param[in]	aiop_tile_id - AIOP tile ID to run
+ * @param[in]	cores_mask - Mask of AIOP cores to run (core 0 in msb)
+ * @param[in]	options - Execution options (currently none defined)
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dpmng_run_aiop(struct fsl_mc_io *mc_io,
+		   int aiop_tile_id, uint32_t cores_mask, uint64_t options);
+
+/** @} */
+
+#endif /* __FSL_DPMNG_H */
diff --git a/include/linux/fsl_dprc.h b/include/linux/fsl_dprc.h
new file mode 100644
index 0000000..8bd753e
--- /dev/null
+++ b/include/linux/fsl_dprc.h
@@ -0,0 +1,790 @@
+/* Copyright 2013-2014 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *     * Neither the name of Freescale Semiconductor nor the
+ *       names of its contributors may be used to endorse or promote products
+ *       derived from this software without specific prior written permission.
+ *
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/*!
+ *  @file    fsl_dprc.h
+ *  @brief   Data Path Resource Container API
+ */
+
+#ifndef _FSL_DPRC_H
+#define _FSL_DPRC_H
+
+/*!
+ * @Group grp_dprc	Data Path Resource Container API
+ *
+ * @brief	Contains DPRC API for managing and querying LDPAA resources
+ * @{
+ */
+
+struct fsl_mc_io;
+
+/*!
+ * Set this value as the icid value in dprc_cfg structure when creating a
+ * container, in case the ICID is not selected by the user and should be
+ * allocated by the DPRC from the pool of ICIDs.
+ */
+#define DPRC_GET_ICID_FROM_POOL			(uint16_t)(~(0))
+/*!
+ * Set this value as the portal_id value in dprc_cfg structure when creating a
+ * container, in case the portal id is not specifically selected by the
+ * user and should be allocated by the DPRC from the pool of portal ids.
+ */
+#define DPRC_GET_PORTAL_ID_FROM_POOL	(int)(~(0))
+
+/*!
+ * @name Resource request options
+ */
+#define DPRC_RES_REQ_OPT_EXPLICIT		0x00000001
+/*!< Explicit resource id request - The requested objects/resources
+ * are explicit and sequential (in case of resources).
+ * The base ID is given at res_req at base_align field */
+#define DPRC_RES_REQ_OPT_ALIGNED		0x00000002
+/*!< Aligned resources request - Relevant only for resources
+ * request (and not objects). Indicates that resources base id should be
+ * sequential and aligned to the value given at dprc_res_req base_align field*/
+#define DPRC_RES_REQ_OPT_PLUGGED		0x00000004
+/*!< Plugged Flag - Relevant only for object assignment request.
+ * Indicates that after all objects assigned. An interrupt will be invoked at
+ * the relevant GPP. The assigned object will be marked as plugged.
+ * plugged objects can't be assigned from their container */
+/* @} */
+
+/*!
+ * @name Container general options
+ *
+ * These options may be selected at container creation by the container creator
+ * and can be retrieved using dprc_get_attributes()
+ */
+#define DPRC_CFG_OPT_SPAWN_ALLOWED		0x00000001
+/*!< Spawn Policy Option allowed - Indicates that the new container is allowed
+ * to spawn and have its own child containers. */
+#define DPRC_CFG_OPT_ALLOC_ALLOWED		0x00000002
+/*!< General Container allocation policy - Indicates that the new container is
+ * allowed to allocate requested resources from its parent container; if not
+ * set, the container is only allowed to use resources in its own pools; Note
+ * that this is a container's global policy, but the parent container may
+ * override it and set specific quota per resource type. */
+#define DPRC_CFG_OPT_OBJ_CREATE_ALLOWED	0x00000004
+/*!< Object initialization allowed - software context associated with this
+ * container is allowed to invoke object initialization operations. */
+#define DPRC_CFG_OPT_TOPOLOGY_CHANGES_ALLOWED	0x00000008
+/*!< Topology change allowed - software context associated with this
+ * container is allowed to invoke topology operations, such as attach/detach
+ * of network objects. */
+#define DPRC_CFG_OPT_IOMMU_BYPASS		0x00000010
+/*!<IOMMU bypass - indicates whether objects of this container are permitted
+ * to bypass the IOMMU.  */
+#define DPRC_CFG_OPT_AIOP			0x00000020
+/*!<AIOP -Indicates that container belongs to aiop.  */
+/* @} */
+
+/*!
+ * @name Objects Attributes Flags
+ */
+#define DPRC_OBJ_STATE_OPEN		0x00000001
+/*!< Opened state - Indicates that an object is open by at least one owner */
+#define DPRC_OBJ_STATE_PLUGGED		0x00000002
+/*!< Plugged state - Indicates that the object is plugged */
+/* @} */
+
+/*!
+ * @name Irq
+ */
+#define DPRC_NUM_OF_IRQS		1
+/*!< Number of dprc's IRQs */
+/* @} */
+
+/*!
+ * @name Object irq events
+ */
+#define DPRC_IRQ_EVENT_OBJ_ASSIGNED			0x00000001
+/*!< Irq event - Indicates that a new object assigned to the container */
+#define DPRC_IRQ_EVENT_OBJ_UNASSIGNED		0x00000002
+/*!< Irq event - Indicates that an object was unassigned from the container */
+#define DPRC_IRQ_EVENT_RES_ASSIGNED			0x00000004
+/*!< Irq event - Indicates that resources assigned to the container */
+#define DPRC_IRQ_EVENT_RES_UNASSIGNED		0x00000008
+/*!< Irq event - Indicates that resources unassigned from the container */
+#define DPRC_IRQ_EVENT_CONTAINER_DESTROYED	0x00000010
+/*!< Irq event - Indicates that one of the descendant containers that opened by
+ * this container is destroyed */
+#define DPRC_IRQ_EVENT_OBJ_DESTROYED		0x00000011
+/*!< Irq event - Indicates that on one of the container's opened object is
+ destroyed */
+/* @} */
+
+/**
+ * @brief	Resource request descriptor, to be used in assignment or
+ *		un-assignment of resources and objects.
+ */
+struct dprc_res_req {
+	char type[16];
+	/*!<
+	 * Resource/object type: Represent as a NULL terminated string.
+	 * This string may received by using dprc_get_pool() to get resource
+	 * type and dprc_get_obj() to get object type.
+	 * Note: it is not possible to assign/unassign DP_OBJ_DPRC objects
+	 */
+	uint32_t num;
+	/*!< Number of resources */
+	uint32_t options;
+	/*!< Request options: combination of DPRC_RES_REQ_OPT_ options */
+	int id_base_align;
+	/*!<
+	 * In case of explicit assignment (DPRC_RES_REQ_OPT_EXPLICIT is set at
+	 * option), this field represents the required base ID for resource
+	 * allocation;
+	 * In case of aligned assignment (DPRC_RES_REQ_OPT_ALIGNED is set at
+	 * option), this field indicates the required alignment for the
+	 * resource ID(s) - use 0 if there is no alignment or explicit id
+	 * requirements.
+	 */
+};
+
+/**
+ * @brief	Object descriptor, returned from dprc_get_obj()
+ */
+struct dprc_obj_desc {
+	uint16_t vendor;
+	/*!< Object vendor identifier */
+	char type[16];
+	/*!< Type of object: NULL terminated string */
+	int id;
+	/*!< ID of logical object resource */
+	uint32_t ver_major;
+	/*!< Major version number */
+	uint32_t ver_minor;
+	/*!< Minor version number */
+	uint8_t irq_count;
+	/*!< Number of interrupts supported by the object */
+	uint8_t region_count;
+	/*!< Number of mappable regions supported by the object */
+	uint32_t state;
+	/*!< Object state: combination of DPRC_OBJ_STATE_ states */
+};
+
+/**
+ * @brief	Mappable region descriptor, returned by dprc_get_obj_region()
+ */
+struct dprc_region_desc {
+	uint64_t base_paddr;
+	/*!< Region base physical address */
+	uint32_t size;
+	/*!< Region size (in bytes) */
+};
+
+/**
+ * @brief	Iteration status, for use with dprc_get_res_ids() function
+ *
+ */
+enum dprc_iter_status {
+	DPRC_ITER_STATUS_FIRST = 0,
+	/*!< Perform first iteration */
+	DPRC_ITER_STATUS_MORE = 1,
+	/*!< Indicates more/next iteration is needed */
+	DPRC_ITER_STATUS_LAST = 2
+	    /*!< Indicates last iteration */
+};
+
+/**
+ * @brief	Resource Id range descriptor, Used at dprc_get_res_ids() and
+ *		contains one range details.
+ */
+struct dprc_res_ids_range_desc {
+	int base_id;
+	/*!< Base resource ID of this range */
+	int last_id;
+	/*!< Last resource ID of this range */
+	enum dprc_iter_status iter_status;
+	/*!<
+	 * Iteration status - should be set to DPRC_ITER_STATUS_FIRST at
+	 * first iteration; while the returned marker is DPRC_ITER_STATUS_MORE,
+	 * additional iterations are needed, until the returned marker is
+	 * DPRC_ITER_STATUS_LAST
+	 */
+};
+
+/**
+ * @brief	Container attributes, returned by dprc_get_attributes()
+ */
+struct dprc_attributes {
+	int container_id;
+	/*!< Container's ID */
+	uint16_t icid;
+	/*!< Container's ICID */
+	int portal_id;
+	/*!< Container's portal ID */
+	uint64_t options;
+	/*!< Container's options as set at container's creation */
+	struct {
+		uint32_t major;	/*!< DPRC major version */
+		uint32_t minor;	/*!< DPRC minor version */
+	} version;
+	/*!< DPRC version */
+};
+
+/**
+ * @brief	Container configuration options, used in dprc_create_container()
+ */
+struct dprc_cfg {
+	uint16_t icid;
+	/*!< Container's ICID; if set to DPRC_GET_ICID_FROM_POOL, a free ICID
+	 * will be allocated by the DPRC */
+	int portal_id;
+	/*!< portal id; if set to DPRC_GET_PORTAL_ID_FROM_POOL, a free portal id
+	 * will be allocated by the DPRC */
+	uint64_t options;
+	/*!< Combination of DPRC_CFG_OPT_ options */
+};
+
+/**
+ * @brief	Endpoint description for link connect/disconnect operations
+ */
+struct dprc_endpoint {
+	char type[16];
+	/*!< Endpoint object type: NULL terminated string */
+	int id;
+	/*!< Endpoint object id */
+	int interface_id;
+	/*!<
+	 * Interface id; should be set for endpoints with multiple interfaces
+	 * (DP_OBJ_DPSW, DP_OBJ_DPDMUX); for others, always set to 0.
+	 */
+};
+
+/**
+ * @brief	Obtains the container id associated with a given portal.
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[out]	container_id	Requested container ID
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_get_container_id(struct fsl_mc_io *mc_io, int *container_id);
+
+/**
+ * @brief	Opens a DPRC object for use
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	container_id	Container ID to open
+ * @param[out]	dprc_handle	Handle to the DPRC object
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ *
+ * @warning	Required before any operation on the object.
+ */
+int dprc_open(struct fsl_mc_io *mc_io, int container_id,
+	      uint16_t *dprc_handle);
+
+/**
+ * @brief	Closes the DPRC object handle
+ *
+ * No further operations on the object are allowed after this call without
+ * re-opening the object.
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_close(struct fsl_mc_io *mc_io, uint16_t dprc_handle);
+
+/**
+ * @brief	Creates a child container
+ *
+ * @param[in]   mc_io			Pointer to opaque I/O object
+ * @param[in]	dprc_handle		Handle to the DPRC object
+ * @param[in]	cfg			Child container configuration
+ * @param[out]	child_container_id	Child container ID
+ * @param[out]	child_portal_paddr	Base physical address of the
+ *				child portal
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_create_container(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			  struct dprc_cfg *cfg,
+			  int *child_container_id,
+			  uint64_t *child_portal_paddr);
+
+/**
+ * @brief	Destroys a child container.
+ *
+ * This function terminates the child container, so following this call the
+ * child container ID becomes invalid.
+ *
+ * Notes:
+ * - All resources and objects of the destroyed container are returned to the
+ * parent container or destroyed if were created be the destroyed container.
+ * - This function destroy all the child containers of the specified
+ *   container prior to destroying the container itself.
+ *
+ * @param[in]   mc_io			Pointer to opaque I/O object
+ * @param[in]	dprc_handle		Handle to the DPRC object
+ * @param[in]	child_container_id	ID of the container to destroy
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ *
+ * @warning	Only the parent container is allowed to destroy a child policy
+ *		Container 0 can't be destroyed
+ */
+int dprc_destroy_container(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			   int child_container_id);
+
+/**
+ * @brief	Sets allocation policy for a specific resource/object type in a
+ * child container
+ *
+ * Allocation policy determines whether or not a container may allocate
+ * resources from its parent. Each container has a 'global' allocation policy
+ * that is set when the container is created.
+ *
+ * This function sets allocation policy for a specific resource type.
+ * The default policy for all resource types matches the container's 'global'
+ * allocation policy.
+ *
+ * @param[in]   mc_io		    Pointer to opaque I/O object
+ * @param[in]	dprc_handle	    Handle to the DPRC object
+ * @param[in]	child_container_id  ID of the child container
+ * @param[in]	type		    resource/object type
+ * @param[in]	quota		    Sets the maximum number of resources of
+ *				    the selected type that the child
+ *				    container is allowed to allocate
+ *				    from its parent;
+ *				    when quota is set to -1, the policy is
+ *				    the same as container's general policy.
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ *
+ * @warning	Only the parent container is allowed to change a child policy.
+ */
+int dprc_set_res_quota(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		       int child_container_id, char *type, uint16_t quota);
+
+/**
+ * @brief	Gets the allocation policy of a specific resource/object type
+ *		in a child container
+ *
+ * @param[in]   mc_io			Pointer to opaque I/O object
+ * @param[in]	dprc_handle		Handle to the DPRC object
+ * @param[in]	child_container_id	ID of the child container
+ * @param[in]	type			resource/object type
+ * @param[out]	quota			Holds the maximum number of resources of
+ *					the selected type that the child
+ *					container is allowed to allocate from
+ *					the parent;
+ *					when quota is set to -1, the policy is
+ *					the same as container's general policy.
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_get_res_quota(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		       int child_container_id, char *type, uint16_t *quota);
+
+/**
+ * @brief	Resets a child container.
+ *
+ * In case a software context crashes or becomes non-responsive, the parent
+ * may wish to reset its resources container before the software context is
+ * restarted.
+ *
+ * This routine informs all objects assigned to the child container that the
+ * container is being reset, so they may perform any cleanup operations that are
+ * needed. All objects handles that were owned by the child container shall be
+ * closed.
+ *
+ * Note that such request may be submitted even if the child software context
+ * has not crashed, but the resulting object cleanup operations will not be
+ * aware of that.
+ *
+ * @param[in]   mc_io			Pointer to opaque I/O object
+ * @param[in]	dprc_handle		Handle to the DPRC object
+ * @param[in]	child_container_id	ID of the container to reset
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_reset_container(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			 int child_container_id);
+
+/**
+ * @brief	Assigns objects or resource to a child container.
+ *
+ * Assignment is usually done by a parent (this DPRC) to one of its child
+ * containers.
+ *
+ * According to the DPRC allocation policy, the assigned resources may be taken
+ * (allocated) from the container's ancestors, if not enough resources are
+ * available in the container itself.
+ *
+ * The type of assignment depends on the dprc_res_req options, as follows:
+ * - DPRC_RES_REQ_OPT_EXPLICIT: indicates that assigned resources should have
+ *   the explicit base ID specified at the id_base_align field of res_req.
+ * - DPRC_RES_REQ_OPT_ALIGNED: indicates that the assigned resources should be
+ *   aligned to the value given at id_base_align field of res_req.
+ * - DPRC_RES_REQ_OPT_PLUGGED: Relevant only for object assignment,
+ *   and indicates that the object must be set to the plugged state.
+ *
+ * A container may use this function with its own ID in order to change a
+ * object state to plugged or unplugged.
+ *
+ * If IRQ information has been set in the child DPRC, it will signal an
+ * interrupt following every change in its object assignment.
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]	container_id	ID of the child container
+ * @param[in]	res_req		Describes the type and amount of resources to
+ *				assign to the given container.
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_assign(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		int container_id, struct dprc_res_req *res_req);
+
+/**
+ * @brief	Un-assigns objects or resources from a child container
+ *		and moves them into this (parent) DPRC.
+ *
+ * Un-assignment of objects can succeed only if the object is not in the
+ * plugged or opened state.
+ *
+ * @param[in]   mc_io			Pointer to opaque I/O object
+ * @param[in]	dprc_handle		Handle to the DPRC object
+ * @param[in]	child_container_id	ID of the child container
+ * @param[in]	res_req			Describes the type and amount of
+ *					resources to un-assign from the child
+ *					container
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_unassign(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		  int child_container_id, struct dprc_res_req *res_req);
+
+/**
+ * @brief	Get the number of dprc's pools
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[out]  pool_count	Number of resource pools in the dprc.
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ * */
+int dprc_get_pool_count(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			int *pool_count);
+
+/**
+ * @brief	Get the type (string) of a certain dprc's pool
+ *
+ * The pool types retrieved one by one by incrementing
+ * pool_index up to (not including) the value of pool_count returned
+ * from dprc_get_pool_count(). dprc_get_pool_count() must
+ * be called prior to dprc_get_pool().
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]   pool_index	Index of the pool to be queried (< pool_count)
+ * @param[out]  type		The type of the pool.
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ * */
+int dprc_get_pool(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		  int pool_index, char *type);
+
+/**
+ * @brief	Obtains the number of objects in the DPRC
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[out]	obj_count	Number of objects assigned to the DPRC
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_get_obj_count(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		       int *obj_count);
+
+/**
+ * @brief	Obtains general information on an object
+ *
+ * The object descriptors are retrieved one by one by incrementing
+ * obj_index up to (not including) the value of obj_count returned
+ * from dprc_get_obj_count(). dprc_get_obj_count() must
+ * be called prior to dprc_get_obj().
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]	obj_index	Index of the object to be queried (< obj_count)
+ * @param[out]	obj_desc	Returns the requested object descriptor
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_get_obj(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		 int obj_index, struct dprc_obj_desc *obj_desc);
+
+/**
+ * @brief	Obtains the number of free resources that are assigned
+ *		to this container, by pool type
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]	type		pool type
+ * @param[out]	res_count	Number of free resources of the given
+ *				resource type that are assigned to this DPRC
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_get_res_count(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		       char *type, int *res_count);
+
+/**
+ * @brief	Obtains IDs of free resources in the container
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]	type		pool type
+ * @param[in,out] range_desc	range descriptor
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_get_res_ids(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		     char *type, struct dprc_res_ids_range_desc *range_desc);
+
+/**
+ * @brief	Obtains the physical address of MC portals
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]	portal_id	MC portal id
+ * @param[out]  portal_addr	The physical address of the MC portal id
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_get_portal_paddr(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			  int portal_id, uint64_t *portal_addr);
+
+/**
+ * @brief	Obtains container attributes
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[out]	attributes	Container attributes
+ *
+ * @returns     '0' on Success; Error code otherwise.
+ */
+int dprc_get_attributes(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			struct dprc_attributes *attributes);
+
+/**
+ * @brief	Returns region information for a specified object.
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]	obj_type	Object type as returned in dprc_get_obj()
+ * @param[in]	obj_id		Unique object instance as returned in
+ *				dprc_get_obj()
+ * @param[in]	region_index	The specific region to query
+ * @param[out]	region_desc	Returns the requested region descriptor
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_get_obj_region(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			char *obj_type,
+			int obj_id,
+			uint8_t region_index,
+			struct dprc_region_desc *region_desc);
+
+/**
+ * @brief	Sets IRQ information for the DPRC to trigger an interrupt.
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]	irq_index	Identifies the interrupt index to configure
+ *				DPRC supports only irq_index 0 - this interrupt
+ *				will be signaled on every change to
+ *				resource/object assignment in this DPRC.
+ * @param[in]	irq_paddr	Physical IRQ address that must be written to
+ *				signal a message-based interrupt
+ * @param[in]	irq_val		Value to write into irq_paddr address
+ * @param[in]	user_irq_id	A user defined number associated with this IRQ;
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_set_irq(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		 uint8_t irq_index,
+		 uint64_t irq_paddr, uint32_t irq_val, int user_irq_id);
+
+/**
+ * @brief	Gets IRQ information from the DPRC.
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]   irq_index	The interrupt index to configure;
+ *				DPRC supports only irq_index 0 - this interrupt
+ *				will be signaled on every change to
+ *				resource/object assignment in this DPRC.
+ * @param[out]  type		Interrupt type: 0 represents message interrupt
+ *				type (both irq_paddr and irq_val are valid);
+ * @param[out]	irq_paddr	Physical address that must be written in order
+ *				to signal the message-based interrupt
+ * @param[out]	irq_val		Value to write in order to signal the
+ *				message-based interrupt
+ * @param[out]	user_irq_id	A user defined number associated with this IRQ;
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_get_irq(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		 uint8_t irq_index,
+		 int *type,
+		 uint64_t *irq_paddr, uint32_t *irq_val, int *user_irq_id);
+
+/**
+ * @brief	Sets overall interrupt state.
+ *
+ * Allows GPP software to control when interrupts are generated.
+ * Each interrupt can have up to 32 causes.  The enable/disable control's the
+ * overall interrupt state. if the interrupt is disabled no causes will cause an
+ * interrupt.
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]   irq_index	The interrupt index to configure
+ * @param[in]	enable_state	Interrupt state - enable = 1, disable = 0.
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_set_irq_enable(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			uint8_t irq_index, uint8_t enable_state);
+
+/**
+ * @brief	Gets overall interrupt state
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]   irq_index	The interrupt index to configure
+ * @param[out]	enable_state	Interrupt state - enable = 1, disable = 0.
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_get_irq_enable(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			uint8_t irq_index, uint8_t *enable_state);
+
+/**
+ * @brief	Sets interrupt mask.
+ *
+ * Every interrupt can have up to 32 causes and the interrupt model supports
+ * masking/unmasking each cause independently
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]   irq_index	The interrupt index to configure
+ * @param[in]	mask		Event mask to trigger interrupt.
+ *				each bit:
+ *					0 = ignore event
+ *					1 = consider event for asserting irq
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_set_irq_mask(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		      uint8_t irq_index, uint32_t mask);
+
+/**
+ * @brief	Gets interrupt mask.
+ *
+ * Every interrupt can have up to 32 causes and the interrupt model supports
+ * masking/unmasking each cause independently
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]   irq_index	The interrupt index to configure
+ * @param[out]	mask		Event mask to trigger interrupt
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ */
+int dprc_get_irq_mask(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		      uint8_t irq_index, uint32_t *mask);
+
+/**
+ * @brief	Gets the current status of any pending interrupts.
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]   irq_index	The interrupt index to configure
+ * @param[out]	 status		Interrupts status - one bit per cause
+ *					0 = no interrupt pending
+ *					1 = interrupt pending
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ * */
+int dprc_get_irq_status(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			uint8_t irq_index, uint32_t *status);
+
+/**
+ * @brief	Clears a pending interrupt's status
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]   irq_index	The interrupt index to configure
+ * @param[out]	status		Bits to clear (W1C) - one bit per cause
+ *					0 = don't change
+ *					1 = clear status bit
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ * */
+int dprc_clear_irq_status(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+			  uint8_t irq_index, uint32_t status);
+
+/**
+ * @brief	Connects two endpoints to create a network link between them
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]   endpoint1	Endpoint 1 configuration parameters.
+ * @param[in]	endpoint2	Endpoint 2 configuration parameters.
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ * */
+int dprc_connect(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		 struct dprc_endpoint *endpoint1,
+		 struct dprc_endpoint *endpoint2);
+
+/**
+ * @brief	Disconnects one endpoint to remove its network link
+ *
+ * @param[in]   mc_io		Pointer to opaque I/O object
+ * @param[in]	dprc_handle	Handle to the DPRC object
+ * @param[in]   endpoint	Endpoint configuration parameters.
+ *
+ * @returns	'0' on Success; Error code otherwise.
+ * */
+int dprc_disconnect(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
+		    struct dprc_endpoint *endpoint);
+
+/*! @} */
+
+#endif /* _FSL_DPRC_H */
diff --git a/include/linux/fsl_mc_cmd.h b/include/linux/fsl_mc_cmd.h
new file mode 100644
index 0000000..bf190e9
--- /dev/null
+++ b/include/linux/fsl_mc_cmd.h
@@ -0,0 +1,168 @@
+/* Copyright 2013-2014 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *     * Neither the name of Freescale Semiconductor nor the
+ *       names of its contributors may be used to endorse or promote products
+ *       derived from this software without specific prior written permission.
+ *
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef __FSL_MC_CMD_H
+#define __FSL_MC_CMD_H
+
+#include <linux/fsl_mc_sys.h>
+
+/*
+ * MC clients (GPP side) encode MC command parameters and decode MC response
+ * parameters
+ */
+
+#define MC_CMD_PARAM_OP(_cmd, _param, _offset, _width, _type, _arg) \
+	((_cmd).params[_param] |= u64_enc((_offset), (_width), (_type)(_arg)))
+
+#define MC_RSP_PARAM_OP(_cmd, _param, _offset, _width, _type, _arg) \
+	((_arg) = (_type)u64_dec((_cmd).params[_param], (_offset), (_width)))
+
+#define MAKE_UMASK64(_width) \
+	((uint64_t)((_width) < 64 ? ((uint64_t)1 << (_width)) - 1 : -1))
+
+static inline uint64_t u64_enc(int lsoffset, int width, uint64_t val)
+{
+	return (uint64_t)(((uint64_t)val & MAKE_UMASK64(width)) << lsoffset);
+}
+
+static inline uint64_t u64_dec(uint64_t val, int lsoffset, int width)
+{
+	return (uint64_t)((val >> lsoffset) & MAKE_UMASK64(width));
+}
+
+#define MC_CMD_NUM_OF_PARAMS	7
+
+struct mc_command {
+	uint64_t header;
+	uint64_t params[MC_CMD_NUM_OF_PARAMS];
+};
+
+enum mc_cmd_status {
+	MC_CMD_STATUS_OK = 0x0,	/*!< Completed successfully */
+	MC_CMD_STATUS_READY = 0x1,	/*!< Ready to be processed */
+	MC_CMD_STATUS_AUTH_ERR = 0x3,	/*!< Authentication error */
+	MC_CMD_STATUS_NO_PRIVILEGE = 0x4,	/*!< No privilege */
+	MC_CMD_STATUS_DMA_ERR = 0x5,	/*!< DMA or I/O error */
+	MC_CMD_STATUS_CONFIG_ERR = 0x6,	/*!< Configuration error */
+	MC_CMD_STATUS_TIMEOUT = 0x7,	/*!< Operation timed out */
+	MC_CMD_STATUS_NO_RESOURCE = 0x8,	/*!< No resources */
+	MC_CMD_STATUS_NO_MEMORY = 0x9,	/*!< No memory available */
+	MC_CMD_STATUS_BUSY = 0xA,	/*!< Device is busy */
+	MC_CMD_STATUS_UNSUPPORTED_OP = 0xB,	/*!< Unsupported operation */
+	MC_CMD_STATUS_INVALID_STATE = 0xC	/*!< Invalid state */
+};
+
+#define MC_CMD_HDR_CMDID_O	52	/* Command ID field offset */
+#define MC_CMD_HDR_CMDID_S	12	/* Command ID field size */
+#define MC_CMD_HDR_AUTHID_O	38	/* Authentication ID field offset */
+#define MC_CMD_HDR_AUTHID_S	10	/* Authentication ID field size */
+#define MC_CMD_HDR_SIZE_O	31	/* Size field offset */
+#define MC_CMD_HDR_SIZE_S	6	/* Size field size */
+#define MC_CMD_HDR_STATUS_O	16	/* Status field offset */
+#define MC_CMD_HDR_STATUS_S	8	/* Status field size */
+#define MC_CMD_HDR_PRI_O	15	/* Priority field offset */
+#define MC_CMD_HDR_PRI_S	1	/* Priority field size */
+
+#define MC_CMD_HDR_READ_STATUS(_hdr) \
+	((enum mc_cmd_status)u64_dec((_hdr), \
+		MC_CMD_HDR_STATUS_O, MC_CMD_HDR_STATUS_S))
+
+#define MC_CMD_HDR_READ_AUTHID(_hdr) \
+	((uint16_t)u64_dec((_hdr), MC_CMD_HDR_AUTHID_O, MC_CMD_HDR_AUTHID_S))
+
+#define MC_CMD_PRI_LOW		0	/*!< Low Priority command indication */
+#define MC_CMD_PRI_HIGH		1	/*!< High Priority command indication */
+
+static inline uint64_t mc_encode_cmd_header(uint16_t cmd_id,
+					    uint8_t cmd_size,
+					    uint8_t priority, uint16_t auth_id)
+{
+	uint64_t hdr;
+
+	hdr = u64_enc(MC_CMD_HDR_CMDID_O, MC_CMD_HDR_CMDID_S, cmd_id);
+	hdr |= u64_enc(MC_CMD_HDR_AUTHID_O, MC_CMD_HDR_AUTHID_S, auth_id);
+	hdr |= u64_enc(MC_CMD_HDR_SIZE_O, MC_CMD_HDR_SIZE_S, cmd_size);
+	hdr |= u64_enc(MC_CMD_HDR_PRI_O, MC_CMD_HDR_PRI_S, priority);
+	hdr |= u64_enc(MC_CMD_HDR_STATUS_O, MC_CMD_HDR_STATUS_S,
+		       MC_CMD_STATUS_READY);
+
+	return hdr;
+}
+
+/**
+ * mc_write_command - writes a command to a Management Complex (MC) portal
+ *
+ * @portal: pointer to an MC portal
+ * @cmd: pointer to a filled command
+ */
+static inline void mc_write_command(struct mc_command __iomem *portal,
+				    struct mc_command *cmd)
+{
+	int i;
+
+	/* copy command parameters into the portal */
+	for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++)
+		writeq(cmd->params[i], &portal->params[i]);
+
+	/* submit the command by writing the header */
+	writeq(cmd->header, &portal->header);
+}
+
+/**
+ * mc_read_response - reads the response for the last MC command from a
+ * Management Complex (MC) portal
+ *
+ * @portal: pointer to an MC portal
+ * @resp: pointer to command response buffer
+ *
+ * Returns MC_CMD_STATUS_OK on Success; Error code otherwise.
+ */
+static inline enum mc_cmd_status mc_read_response(struct mc_command __iomem *
+						  portal,
+						  struct mc_command *resp)
+{
+	int i;
+	enum mc_cmd_status status;
+
+	/* Copy command response header from MC portal: */
+	resp->header = readq(&portal->header);
+	status = MC_CMD_HDR_READ_STATUS(resp->header);
+	if (status != MC_CMD_STATUS_OK)
+		return status;
+
+	/* Copy command response data from MC portal: */
+	for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++)
+		resp->params[i] = readq(&portal->params[i]);
+
+	return status;
+}
+
+#endif /* __FSL_MC_CMD_H */
diff --git a/include/linux/fsl_mc_sys.h b/include/linux/fsl_mc_sys.h
new file mode 100644
index 0000000..f7a9748
--- /dev/null
+++ b/include/linux/fsl_mc_sys.h
@@ -0,0 +1,89 @@
+/* Copyright 2013-2014 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *     * Neither the name of Freescale Semiconductor nor the
+ *       names of its contributors may be used to endorse or promote products
+ *       derived from this software without specific prior written permission.
+ *
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _FSL_MC_SYS_H
+#define _FSL_MC_SYS_H
+
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/spinlock_types.h>
+#include <linux/mutex.h>
+
+/**
+ * Bit masks for a MC I/O object (struct fsl_mc_io) flags
+ */
+#define FSL_MC_PORTAL_SHARED_BY_THREADS		0x0001
+#define FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS	0x0002
+
+struct mc_command;
+
+/**
+ * struct fsl_mc_io - MC I/O object to be passed-in to mc_send_command()
+ * @dev: device associated with this Mc I/O object
+ * @flags: flags for mc_send_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
+ * @spinlock: spin lock to serialize access to the MC command portal when
+ * the FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS flag is set.
+ * @mutex: mutex to serialize access to the MC command portal when
+ * the FSL_MC_PORTAL_SHARED_BY_THREADS flag is set, but the
+ * FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS is not set.
+ *
+ * NOTE: If both FSL_MC_PORTAL_SHARED_BY_THREADS and
+ * FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS are set, the spinlock is used
+ * and interrupts are disabled, to serialize access to the MC command
+ * portal from threads and interrupt handles, potentially running on
+ * separate processors.
+ */
+struct fsl_mc_io {
+	struct device *dev;
+	uint32_t flags;
+	uint32_t portal_size;
+	phys_addr_t portal_phys_addr;
+	void __iomem *portal_virt_addr;
+	union {
+		spinlock_t spinlock; /* for interrupt handler serialization */
+		struct mutex mutex;  /* for thread serialization */
+	};
+};
+
+int __must_check fsl_create_mc_io(struct device *dev,
+				  phys_addr_t mc_portal_phys_addr,
+				  uint32_t mc_portal_size,
+				  uint32_t flags, struct fsl_mc_io **new_mc_io);
+
+void fsl_destroy_mc_io(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 */
--
1.7.9.7


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

* [PATCH 2/3 v2] drivers/bus: Freescale Management Complex (fsl-mc) bus driver
  2014-09-19 22:49 [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series J. German Rivera
  2014-09-19 22:49 ` [PATCH 1/3 v2] drivers/bus: Added Freescale Management Complex APIs J. German Rivera
@ 2014-09-19 22:49 ` J. German Rivera
  2014-09-19 22:49 ` [PATCH 3/3 v2] drivers/bus: Device driver for FSL-MC DPRC devices J. German Rivera
  2014-09-22 16:53 ` [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series Kim Phillips
  3 siblings, 0 replies; 20+ messages in thread
From: J. German Rivera @ 2014-09-19 22:49 UTC (permalink / raw)
  To: gregkh, arnd, linux-kernel
  Cc: stuart.yoder, Kim.Phillips, scottwood, agraf, linuxppc-release,
	J. German Rivera

From: "J. German Rivera" <German.Rivera@freescale.com>

Platform device driver that sets up the basic bus infrastructure
for the fsl-mc bus type, including support for adding/removing
fsl-mc devices, register/unregister of fsl-mc drivers, and bus
match support to bind devices to drivers.

Signed-off-by: J. German Rivera <German.Rivera@freescale.com>
Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
---
Changes in v2:
- Addressed comment from Joe Perches:
  * Changed pr_debug to dev_dbg in fsl_mc_bus_match

- Addressed comments from Kim Phillips and Alex Graf:
  * Changed version check to allow the driver to run with MC
    firmware that has major version number greater than or equal
    to the driver's major version number.
  * Removed minor version check

- Removed unused variable parent_dev in fsl_mc_device_remove

 drivers/bus/Kconfig             |    3 +
 drivers/bus/Makefile            |    3 +
 drivers/bus/fsl-mc/Kconfig      |   13 +
 drivers/bus/fsl-mc/Makefile     |   14 +
 drivers/bus/fsl-mc/fsl_mc_bus.c |  595 +++++++++++++++++++++++++++++++++++++++
 include/linux/fsl_mc.h          |  137 +++++++++
 include/linux/fsl_mc_private.h  |   33 +++
 7 files changed, 798 insertions(+)
 create mode 100644 drivers/bus/fsl-mc/Kconfig
 create mode 100644 drivers/bus/fsl-mc/Makefile
 create mode 100644 drivers/bus/fsl-mc/fsl_mc_bus.c
 create mode 100644 include/linux/fsl_mc.h
 create mode 100644 include/linux/fsl_mc_private.h

diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 603eb1b..2fbb1fd 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -67,4 +67,7 @@ config VEXPRESS_CONFIG
 	help
 	  Platform configuration infrastructure for the ARM Ltd.
 	  Versatile Express.
+
+source "drivers/bus/fsl-mc/Kconfig"
+
 endmenu
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index 2973c18..6abcab1 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -15,3 +15,6 @@ obj-$(CONFIG_ARM_CCI)		+= arm-cci.o
 obj-$(CONFIG_ARM_CCN)		+= arm-ccn.o

 obj-$(CONFIG_VEXPRESS_CONFIG)	+= vexpress-config.o
+
+# Freescale Management Complex (MC) bus drivers
+obj-$(CONFIG_FSL_MC_BUS)	+= fsl-mc/
diff --git a/drivers/bus/fsl-mc/Kconfig b/drivers/bus/fsl-mc/Kconfig
new file mode 100644
index 0000000..e3226f9
--- /dev/null
+++ b/drivers/bus/fsl-mc/Kconfig
@@ -0,0 +1,13 @@
+#
+# Freescale Management Complex (MC) bus drivers
+#
+# Copyright (C) 2014 Freescale Semiconductor, Inc.
+#
+# This file is released under the GPLv2
+#
+
+config FSL_MC_BUS
+	tristate "Freescale Management Complex (MC) bus driver"
+	help
+	  Driver to enable the bus infrastructure for the Freescale
+          QorIQ Management Complex.
diff --git a/drivers/bus/fsl-mc/Makefile b/drivers/bus/fsl-mc/Makefile
new file mode 100644
index 0000000..2981d70
--- /dev/null
+++ b/drivers/bus/fsl-mc/Makefile
@@ -0,0 +1,14 @@
+#
+# Freescale Management Complex (MC) bus drivers
+#
+# Copyright (C) 2014 Freescale Semiconductor, Inc.
+#
+# This file is released under the GPLv2
+#
+obj-$(CONFIG_FSL_MC_BUS) += fsl_mc_bus_driver.o
+
+fsl_mc_bus_driver-objs := fsl_mc_bus.o \
+			      fsl_mc_sys.o \
+			      dprc.o \
+			      dpmng.o
+
diff --git a/drivers/bus/fsl-mc/fsl_mc_bus.c b/drivers/bus/fsl-mc/fsl_mc_bus.c
new file mode 100644
index 0000000..e3a994f
--- /dev/null
+++ b/drivers/bus/fsl-mc/fsl_mc_bus.c
@@ -0,0 +1,595 @@
+/*
+ * Freescale Management Complex (MC) bus driver
+ *
+ * Copyright (C) 2014 Freescale Semiconductor, Inc.
+ * Author: German Rivera <German.Rivera@freescale.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/fsl_mc_private.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/ioport.h>
+#include <linux/slab.h>
+#include <linux/limits.h>
+#include <linux/fsl_dpmng.h>
+#include <linux/fsl_mc_sys.h>
+#include "fsl_dprc_cmd.h"
+
+static struct kmem_cache *mc_dev_cache;
+
+/**
+ * fsl_mc_bus_match - device to driver matching callback
+ * @dev: the MC object device structure to match against
+ * @drv: the device driver to search for matching MC object device id
+ * structures
+ *
+ * Returns 1 on success, 0 otherwise.
+ */
+static int fsl_mc_bus_match(struct device *dev, struct device_driver *drv)
+{
+	const struct fsl_mc_device_match_id *id;
+	struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+	struct fsl_mc_driver *mc_drv = to_fsl_mc_driver(drv);
+	bool found = false;
+
+	if (mc_drv->match_id_table == NULL)
+		goto out;
+
+	/*
+	 * If the object is not 'plugged' don't match.
+	 * Only exception is the root DPRC, which is a special case.
+	 */
+	if ((mc_dev->obj_desc.state & DPRC_OBJ_STATE_PLUGGED) == 0 &&
+	    &mc_dev->dev != fsl_mc_bus_type.dev_root)
+		goto out;
+
+	/*
+	 * Traverse the match_id table of the given driver, trying to find
+	 * a matching for the given MC object device.
+	 */
+	for (id = mc_drv->match_id_table; id->vendor != 0x0; id++) {
+		if (id->vendor == mc_dev->obj_desc.vendor &&
+		    strcmp(id->obj_type, mc_dev->obj_desc.type) == 0 &&
+		    id->ver_major == mc_dev->obj_desc.ver_major &&
+		    id->ver_minor == mc_dev->obj_desc.ver_minor) {
+			found = true;
+			break;
+		}
+	}
+
+out:
+	dev_dbg(dev, "%smatched\n", found ? "" : "not ");
+	return found;
+}
+
+/**
+ * fsl_mc_bus_uevent - callback invoked when a device is added
+ */
+static int fsl_mc_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+	pr_debug("%s invoked\n", __func__);
+	return 0;
+}
+
+struct bus_type fsl_mc_bus_type = {
+	.name = "fsl-mc",
+	.match = fsl_mc_bus_match,
+	.uevent = fsl_mc_bus_uevent,
+};
+EXPORT_SYMBOL_GPL(fsl_mc_bus_type);
+
+static int fsl_mc_driver_probe(struct device *dev)
+{
+	struct fsl_mc_driver *mc_drv;
+	struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+	int error = -EINVAL;
+
+	if (WARN_ON(dev->driver == NULL))
+		goto error;
+
+	mc_drv = to_fsl_mc_driver(dev->driver);
+	if (WARN_ON(mc_drv->probe == NULL))
+		goto error;
+
+	error = mc_drv->probe(mc_dev);
+	if (error < 0) {
+		dev_err(dev, "MC object device probe callback failed: %d\n",
+			error);
+		goto error;
+	}
+
+	return 0;
+error:
+	return error;
+}
+
+static int fsl_mc_driver_remove(struct device *dev)
+{
+	struct fsl_mc_driver *mc_drv = to_fsl_mc_driver(dev->driver);
+	struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+	int error = -EINVAL;
+
+	if (WARN_ON(dev->driver == NULL))
+		goto error;
+
+	error = mc_drv->remove(mc_dev);
+	if (error < 0) {
+		dev_err(dev,
+			"MC object device remove callback failed: %d\n",
+			error);
+		goto error;
+	}
+
+	return 0;
+error:
+	return error;
+}
+
+static void fsl_mc_driver_shutdown(struct device *dev)
+{
+	struct fsl_mc_driver *mc_drv = to_fsl_mc_driver(dev->driver);
+	struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+
+	mc_drv->shutdown(mc_dev);
+}
+
+/**
+ * __fsl_mc_driver_register - registers a child device driver with the
+ * MC bus
+ *
+ * This function is implicitly invoked from the registration function of
+ * fsl_mc device drivers, which is generated by the
+ * module_fsl_mc_driver() macro.
+ */
+int __fsl_mc_driver_register(struct fsl_mc_driver *mc_driver,
+			     struct module *owner)
+{
+	int error = -EINVAL;
+
+	mc_driver->driver.owner = owner;
+	mc_driver->driver.bus = &fsl_mc_bus_type;
+
+	/*
+	 * Set default driver callbacks, if not set by the child driver:
+	 */
+	if (mc_driver->probe != NULL)
+		mc_driver->driver.probe = fsl_mc_driver_probe;
+
+	if (mc_driver->remove != NULL)
+		mc_driver->driver.remove = fsl_mc_driver_remove;
+
+	if (mc_driver->shutdown != NULL)
+		mc_driver->driver.shutdown = fsl_mc_driver_shutdown;
+
+	error = driver_register(&mc_driver->driver);
+	if (error < 0) {
+		pr_err("driver_register() failed for %s: %d\n",
+		       mc_driver->driver.name, error);
+		goto error;
+	}
+
+	pr_info("MC object device driver %s registered\n",
+		mc_driver->driver.name);
+	return 0;
+error:
+	return error;
+}
+EXPORT_SYMBOL_GPL(__fsl_mc_driver_register);
+
+/**
+ * fsl_mc_driver_unregister - unregisters a device driver from the
+ * MC bus
+ */
+void fsl_mc_driver_unregister(struct fsl_mc_driver *mc_driver)
+{
+	driver_unregister(&mc_driver->driver);
+}
+EXPORT_SYMBOL_GPL(fsl_mc_driver_unregister);
+
+static int get_dprc_icid(struct fsl_mc_io *mc_io,
+			 int container_id, uint16_t *icid)
+{
+	uint16_t dprc_handle;
+	struct dprc_attributes attr;
+	bool dprc_opened = false;
+	int error = -EINVAL;
+
+	error = dprc_open(mc_io, container_id, &dprc_handle);
+	if (error < 0) {
+		pr_err("dprc_open() failed: %d\n", error);
+		goto out;
+	}
+
+	dprc_opened = true;
+	memset(&attr, 0, sizeof(attr));
+	error = dprc_get_attributes(mc_io, dprc_handle, &attr);
+	if (error < 0) {
+		pr_err("dprc_get_attributes() failed: %d\n", error);
+		goto out;
+	}
+
+	*icid = attr.icid;
+	error = 0;
+out:
+	if (dprc_opened)
+		(void)dprc_close(mc_io, dprc_handle);
+
+	return error;
+}
+
+static int fsl_mc_device_get_mmio_regions(struct fsl_mc_device *mc_dev,
+					  struct fsl_mc_device *mc_bus_dev)
+{
+	int i;
+	int error;
+	struct resource *regions;
+	struct dprc_obj_desc *obj_desc = &mc_dev->obj_desc;
+	struct device *parent_dev = mc_dev->dev.parent;
+
+	regions = kmalloc_array(obj_desc->region_count,
+				sizeof(regions[0]), GFP_KERNEL);
+	if (regions == NULL) {
+		error = -ENOMEM;
+		dev_err(parent_dev, "No memory to allocate regions[]\n");
+		goto error;
+	}
+
+	for (i = 0; i < obj_desc->region_count; i++) {
+		struct dprc_region_desc region_desc;
+
+		error = dprc_get_obj_region(mc_bus_dev->mc_io,
+					    mc_bus_dev->mc_handle,
+					    obj_desc->type,
+					    obj_desc->id, i, &region_desc);
+		if (error < 0) {
+			dev_err(parent_dev,
+				"dprc_get_obj_region() failed: %d\n", error);
+			goto error;
+		}
+
+		WARN_ON(region_desc.base_paddr == 0x0);
+		WARN_ON(region_desc.size == 0);
+		regions[i].start = region_desc.base_paddr;
+		regions[i].end = region_desc.base_paddr + region_desc.size - 1;
+		regions[i].name = "fsl-mc object MMIO region";
+		regions[i].flags = IORESOURCE_IO;
+	}
+
+	mc_dev->regions = regions;
+	return 0;
+error:
+	kfree(regions);
+	return error;
+}
+
+/**
+ * Add a newly discovered MC object device to be visible in Linux
+ */
+int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
+		      struct fsl_mc_io *mc_io,
+		      struct device *parent_dev,
+		      struct fsl_mc_device **new_mc_dev)
+{
+	int error = -EINVAL;
+	struct fsl_mc_device *mc_dev = NULL;
+	bool device_added = false;
+	struct fsl_mc_device *parent_mc_dev;
+
+	if (parent_dev->bus == &fsl_mc_bus_type)
+		parent_mc_dev = to_fsl_mc_device(parent_dev);
+	else
+		parent_mc_dev = NULL;
+
+	/*
+	 * DPRC devices must have a dedicated MC portal
+	 */
+	if (WARN_ON(strcmp(obj_desc->type, "dprc") == 0 && mc_io == NULL))
+		goto error;
+
+	mc_dev = kmem_cache_zalloc(mc_dev_cache, GFP_KERNEL);
+	if (mc_dev == NULL) {
+		error = -ENOMEM;
+		dev_err(parent_dev,
+			"No memory to allocate fsl_mc_device\n");
+		goto error;
+	}
+
+	mc_dev->obj_desc = *obj_desc;
+	mc_dev->mc_io = mc_io;
+	device_initialize(&mc_dev->dev);
+	mc_dev->dev.parent = parent_dev;
+	mc_dev->dev.bus = &fsl_mc_bus_type;
+	dev_set_name(&mc_dev->dev, "%s.%x", obj_desc->type, obj_desc->id);
+
+	if (strcmp(obj_desc->type, "dprc") == 0) {
+		struct fsl_mc_io *mc_io2;
+
+		mc_dev->flags |= FSL_MC_IS_DPRC;
+
+		/*
+		 * To get the DPRC's ICID, we need to open the DPRC
+		 * in get_dprc_icid(). For child DPRCs, we do so using the
+		 * parent DPRC's MC portal instead of the child DPRC's MC
+		 * portal, in case the child DPRC is already opened with
+		 * its own portal (e.g., the DPRC used by AIOP).
+		 *
+		 * NOTE: There cannot be more than one active open for a
+		 * given MC object, using the same MC portal.
+		 */
+		if (parent_mc_dev != NULL) {
+			/*
+			 * device being added is a child DPRC device
+			 */
+			mc_io2 = parent_mc_dev->mc_io;
+		} else {
+			/*
+			 * device being added is the root DPRC device
+			 */
+			mc_io2 = mc_io;
+		}
+
+		error = get_dprc_icid(mc_io2, obj_desc->id, &mc_dev->icid);
+		if (error < 0)
+			goto error;
+	} else {
+		/*
+		 * A non-DPRC MC object device has to be a child of another
+		 * MC object (specifically a DPRC object)
+		 */
+		mc_dev->icid = parent_mc_dev->icid;
+		mc_dev->dma_mask = FSL_MC_DEFAULT_DMA_MASK;
+		mc_dev->dev.dma_mask = &mc_dev->dma_mask;
+		if (obj_desc->region_count != 0) {
+			error = fsl_mc_device_get_mmio_regions(mc_dev,
+							       parent_mc_dev);
+			if (error < 0)
+				goto error;
+		}
+	}
+
+	/*
+	 * The device-specific probe callback will get invoked by device_add()
+	 */
+	error = device_add(&mc_dev->dev);
+	if (error < 0) {
+		dev_err(parent_dev,
+			"device_add() failed for device %s: %d\n",
+			dev_name(&mc_dev->dev), error);
+		goto error;
+	}
+
+	get_device(&mc_dev->dev);
+	device_added = true;
+	dev_dbg(parent_dev, "Added MC object device %s\n",
+		dev_name(&mc_dev->dev));
+
+	*new_mc_dev = mc_dev;
+	return 0;
+error:
+	if (device_added) {
+		device_del(&mc_dev->dev);
+		put_device(&mc_dev->dev);
+	}
+
+	if (mc_dev != NULL) {
+		if (mc_dev->regions != NULL)
+			kfree(mc_dev->regions);
+
+		kmem_cache_free(mc_dev_cache, mc_dev);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(fsl_mc_device_add);
+
+/**
+ * fsl_mc_device_remove - Remove a MC object device from being visible to
+ * Linux
+ *
+ * @mc_dev: Pointer to a MC object device object
+ */
+void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
+{
+	if (mc_dev->regions != NULL)
+		kfree(mc_dev->regions);
+
+	/*
+	 * The device-specific remove callback will get invoked by device_del()
+	 */
+	device_del(&mc_dev->dev);
+	put_device(&mc_dev->dev);
+
+	if (strcmp(mc_dev->obj_desc.type, "dprc") == 0)
+		fsl_destroy_mc_io(mc_dev->mc_io);
+
+	mc_dev->mc_io = NULL;
+	kmem_cache_free(mc_dev_cache, mc_dev);
+}
+EXPORT_SYMBOL_GPL(fsl_mc_device_remove);
+
+/**
+ * fsl_mc_bus_probe - callback invoked when the root MC bus is being
+ * added
+ */
+static int fsl_mc_bus_probe(struct platform_device *pdev)
+{
+	struct dprc_obj_desc obj_desc;
+	int error = -EINVAL;
+	struct fsl_mc_device *mc_bus_dev = NULL;
+	struct fsl_mc_io *mc_io = NULL;
+	int container_id;
+	phys_addr_t mc_portal_phys_addr;
+	uint32_t mc_portal_size;
+	struct mc_version mc_version;
+	struct resource res;
+
+	dev_info(&pdev->dev, "Root MC bus device probed");
+	error = of_address_to_resource(pdev->dev.of_node, 0, &res);
+	if (error < 0) {
+		dev_err(&pdev->dev,
+			"of_address_to_resource() failed for %s\n",
+			pdev->dev.of_node->full_name);
+		goto error;
+	}
+
+	mc_portal_phys_addr = res.start;
+	mc_portal_size = resource_size(&res);
+	error = fsl_create_mc_io(&pdev->dev, mc_portal_phys_addr,
+				 mc_portal_size, 0, &mc_io);
+	if (error < 0)
+		goto error;
+
+	error = mc_get_version(mc_io, &mc_version);
+	if (error != 0) {
+		dev_err(&pdev->dev,
+			"mc_get_version() failed with error %d\n", error);
+		goto error;
+	}
+
+	dev_info(&pdev->dev,
+		 "Freescale Management Complex Firmware version: %u.%u.%u\n",
+		 mc_version.major, mc_version.minor, mc_version.revision);
+
+	if (mc_version.major < MC_VER_MAJOR) {
+		dev_err(&pdev->dev,
+			"ERROR: MC firmware version not supported by driver (driver version: %u.%u)\n",
+			MC_VER_MAJOR, MC_VER_MINOR);
+		error = -ENOTSUPP;
+		goto error;
+	}
+
+	if (mc_version.major > MC_VER_MAJOR) {
+		dev_warn(&pdev->dev,
+			 "WARNING: driver may not support newer MC firmware features (driver version: %u.%u)\n",
+			 MC_VER_MAJOR, MC_VER_MINOR);
+	}
+
+	error = dprc_get_container_id(mc_io, &container_id);
+	if (error < 0) {
+		dev_err(&pdev->dev,
+			"dprc_get_container_id() failed: %d\n", error);
+		goto error;
+	}
+
+	obj_desc.vendor = FSL_MC_VENDOR_FREESCALE;
+	strcpy(obj_desc.type, "dprc");
+	obj_desc.id = container_id;
+	obj_desc.ver_major = DPRC_VER_MAJOR;
+	obj_desc.ver_minor = DPRC_VER_MINOR;
+	obj_desc.region_count = 0;
+
+	error = fsl_mc_device_add(&obj_desc, mc_io, &pdev->dev, &mc_bus_dev);
+	if (error < 0)
+		goto error;
+
+	platform_set_drvdata(pdev, mc_bus_dev);
+	fsl_mc_bus_type.dev_root = &mc_bus_dev->dev;
+	return 0;
+error:
+	if (mc_io != NULL)
+		fsl_destroy_mc_io(mc_io);
+
+	return error;
+}
+
+/**
+ * fsl_mc_bus_remove - callback invoked when the root MC bus is being
+ * removed
+ */
+static int fsl_mc_bus_remove(struct platform_device *pdev)
+{
+	int error = -EINVAL;
+	struct fsl_mc_device *mc_bus_dev = platform_get_drvdata(pdev);
+
+	if (WARN_ON(&mc_bus_dev->dev != fsl_mc_bus_type.dev_root))
+		goto error;
+
+	fsl_mc_device_remove(mc_bus_dev);
+	fsl_mc_bus_type.dev_root = NULL;
+	dev_info(&pdev->dev, "Root MC bus device removed");
+	return 0;
+error:
+	return error;
+}
+
+static const struct of_device_id fsl_mc_bus_match_table[] = {
+	{.compatible = "fsl,qoriq-mc",},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, fsl_mc_bus_match_table);
+
+static struct platform_driver fsl_mc_bus_driver = {
+	.driver = {
+		   .name = "fsl_mc_bus",
+		   .owner = THIS_MODULE,
+		   .pm = NULL,
+		   .of_match_table = fsl_mc_bus_match_table,
+		   },
+	.probe = fsl_mc_bus_probe,
+	.remove = fsl_mc_bus_remove,
+};
+
+static int __init fsl_mc_bus_driver_init(void)
+{
+	int error = -EINVAL;
+	bool bus_registered = false;
+
+	mc_dev_cache = kmem_cache_create("fsl_mc_device",
+					 sizeof(struct fsl_mc_device), 0, 0,
+					 NULL);
+	if (mc_dev_cache == NULL) {
+		error = -ENOMEM;
+		pr_err("Could not create fsl_mc_device cache\n");
+		goto error;
+	}
+
+	error = bus_register(&fsl_mc_bus_type);
+	if (error < 0) {
+		pr_err("fsl-mc bus type registration failed: %d\n", error);
+		goto error;
+	}
+
+	bus_registered = true;
+	pr_info("fsl-mc bus type registered\n");
+
+	error = platform_driver_register(&fsl_mc_bus_driver);
+	if (error < 0) {
+		pr_err("platform_driver_register() failed: %d\n", error);
+		goto error;
+	}
+
+	return 0;
+error:
+	if (bus_registered)
+		bus_unregister(&fsl_mc_bus_type);
+
+	if (mc_dev_cache != NULL)
+		kmem_cache_destroy(mc_dev_cache);
+
+	return error;
+}
+
+postcore_initcall(fsl_mc_bus_driver_init);
+
+static void __exit fsl_mc_bus_driver_exit(void)
+{
+	if (WARN_ON(mc_dev_cache == NULL))
+		return;
+
+	platform_driver_unregister(&fsl_mc_bus_driver);
+	bus_unregister(&fsl_mc_bus_type);
+	kmem_cache_destroy(mc_dev_cache);
+	pr_info("MC bus unregistered\n");
+}
+
+module_exit(fsl_mc_bus_driver_exit);
+
+MODULE_AUTHOR("Freescale Semiconductor Inc.");
+MODULE_DESCRIPTION("Freescale Management Complex (MC) bus driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/fsl_mc.h b/include/linux/fsl_mc.h
new file mode 100644
index 0000000..b291eba
--- /dev/null
+++ b/include/linux/fsl_mc.h
@@ -0,0 +1,137 @@
+/*
+ * Freescale Management Complex (MC) bus public interface
+ *
+ * Copyright (C) 2014 Freescale Semiconductor, Inc.
+ * Author: German Rivera <German.Rivera@freescale.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+#ifndef _FSL_MC_H_
+#define _FSL_MC_H_
+
+#include <linux/device.h>
+#include <linux/mod_devicetable.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
+#include <linux/fsl_dprc.h>
+
+#define FSL_MC_VENDOR_FREESCALE	0x1957
+
+struct fsl_mc_device;
+struct fsl_mc_io;
+
+/**
+ * struct fsl_mc_driver - MC object device driver object
+ * @driver: Generic device driver
+ * @match_id_table: table of supported device matching Ids
+ * @probe: Function called when a device is added
+ * @remove: Function called when a device is removed
+ * @shutdown: Function called at shutdown time to quiesce the device
+ * @suspend: Function called when a device is stopped
+ * @resume: Function called when a device is resumed
+ *
+ * Generic DPAA device driver object for device drivers that are registered
+ * with a DPRC bus. This structure is to be embedded in each device-specific
+ * driver structure.
+ */
+struct fsl_mc_driver {
+	struct device_driver driver;
+	const struct fsl_mc_device_match_id *match_id_table;
+	int (*probe)(struct fsl_mc_device *dev);
+	int (*remove)(struct fsl_mc_device *dev);
+	void (*shutdown)(struct fsl_mc_device *dev);
+	int (*suspend)(struct fsl_mc_device *dev, pm_message_t state);
+	int (*resume)(struct fsl_mc_device *dev);
+};
+
+#define to_fsl_mc_driver(_drv) \
+	container_of(_drv, struct fsl_mc_driver, driver)
+
+/**
+ * struct fsl_mc_device_match_id - MC object device Id entry for driver matching
+ * @vendor: vendor ID
+ * @obj_type: MC object type
+ * @ver_major: MC object version major number
+ * @ver_minor: MC object version minor number
+ *
+ * Type of entries in the "device Id" table for MC object devices supported by
+ * a MC object device driver. The last entry of the table has vendor set to 0x0
+ */
+struct fsl_mc_device_match_id {
+	uint16_t vendor;
+	const char obj_type[16];
+	uint32_t ver_major;
+	uint32_t ver_minor;
+};
+
+/**
+ * Bit masks for a MC object device (struct fsl_mc_device) flags
+ */
+#define FSL_MC_IS_DPRC	0x0001
+
+/**
+ * Default DMA mask for devices on a fsl-mc bus
+ */
+#define FSL_MC_DEFAULT_DMA_MASK	(~0ULL)
+
+/**
+ * struct fsl_mc_device - MC object device object
+ * @dev: Linux driver model device object
+ * @dma_mask: Default DMA mask
+ * @flags: MC object device flags
+ * @icid: Isolation context ID for the device
+ * @mc_handle: MC handle for the corresponding MC object opened
+ * @mc_io: Pointer to MC IO object assigned to this device or
+ * NULL if none.
+ * @obj_desc: MC description of the DPAA device
+ * @regions: pointer to array of MMIO region entries
+ *
+ * Generic device object for MC object devices that are "attached" to a
+ * MC bus.
+ *
+ * NOTES:
+ * - For a non-DPRC object its icid is the same as its parent DPRC's icid.
+ * - The SMMU notifier callback gets invoked after device_add() has been
+ *   called for an MC object device, but before the device-specific probe
+ *   callback gets called.
+ */
+struct fsl_mc_device {
+	struct device dev;
+	uint64_t dma_mask;
+	uint16_t flags;
+	uint16_t icid;
+	uint16_t mc_handle;
+	struct fsl_mc_io *mc_io;
+	struct dprc_obj_desc obj_desc;
+	struct resource *regions;
+};
+
+#define to_fsl_mc_device(_dev) \
+	container_of(_dev, struct fsl_mc_device, dev)
+
+/*
+ * module_fsl_mc_driver() - Helper macro for drivers that don't do
+ * anything special in module init/exit.  This eliminates a lot of
+ * boilerplate.  Each module may only use this macro once, and
+ * calling it replaces module_init() and module_exit()
+ */
+#define module_fsl_mc_driver(__fsl_mc_driver) \
+	module_driver(__fsl_mc_driver, fsl_mc_driver_register, \
+		      fsl_mc_driver_unregister)
+
+/*
+ * Macro to avoid include chaining to get THIS_MODULE
+ */
+#define fsl_mc_driver_register(drv) \
+	__fsl_mc_driver_register(drv, THIS_MODULE)
+
+int __must_check __fsl_mc_driver_register(struct fsl_mc_driver *fsl_mc_driver,
+					  struct module *owner);
+
+void fsl_mc_driver_unregister(struct fsl_mc_driver *driver);
+
+extern struct bus_type fsl_mc_bus_type;
+
+#endif /* _FSL_MC_H_ */
diff --git a/include/linux/fsl_mc_private.h b/include/linux/fsl_mc_private.h
new file mode 100644
index 0000000..3ff3f2b
--- /dev/null
+++ b/include/linux/fsl_mc_private.h
@@ -0,0 +1,33 @@
+/*
+ * Freescale Management Complex (MC) bus private declarations
+ *
+ * Copyright (C) 2014 Freescale Semiconductor, Inc.
+ * Author: German Rivera <German.Rivera@freescale.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+#ifndef _FSL_MC_PRIVATE_H_
+#define _FSL_MC_PRIVATE_H_
+
+#include <linux/fsl_mc.h>
+#include <linux/mutex.h>
+#include <linux/stringify.h>
+
+#define FSL_MC_DEVICE_MATCH(_mc_dev, _obj_desc) \
+	(strcmp((_mc_dev)->obj_desc.type, (_obj_desc)->type) == 0 && \
+	 (_mc_dev)->obj_desc.id == (_obj_desc)->id)
+
+#define FSL_MC_IS_ALLOCATABLE(_obj_type) \
+	(strcmp(_obj_type, "dpbp") == 0 || \
+	 strcmp(_obj_type, "dpcon") == 0)
+
+int __must_check fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
+				   struct fsl_mc_io *mc_io,
+				   struct device *parent_dev,
+				   struct fsl_mc_device **new_mc_dev);
+
+void fsl_mc_device_remove(struct fsl_mc_device *mc_dev);
+
+#endif /* _FSL_MC_PRIVATE_H_ */
--
1.7.9.7


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

* [PATCH 3/3 v2] drivers/bus: Device driver for FSL-MC DPRC devices
  2014-09-19 22:49 [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series J. German Rivera
  2014-09-19 22:49 ` [PATCH 1/3 v2] drivers/bus: Added Freescale Management Complex APIs J. German Rivera
  2014-09-19 22:49 ` [PATCH 2/3 v2] drivers/bus: Freescale Management Complex (fsl-mc) bus driver J. German Rivera
@ 2014-09-19 22:49 ` J. German Rivera
  2014-10-01  2:19   ` Timur Tabi
  2014-09-22 16:53 ` [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series Kim Phillips
  3 siblings, 1 reply; 20+ messages in thread
From: J. German Rivera @ 2014-09-19 22:49 UTC (permalink / raw)
  To: gregkh, arnd, linux-kernel
  Cc: stuart.yoder, Kim.Phillips, scottwood, agraf, linuxppc-release,
	J. German Rivera

From: "J. German Rivera" <German.Rivera@freescale.com>

A DPRC (Data Path Resource Container) is an isolation device
that contains a set of DPAA networking devices to be
assigned to an isolation domain (e.g., a virtual machine).

Signed-off-by: J. German Rivera <German.Rivera@freescale.com>
Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
---
Changes in v2:
- Addressed comments from Kim Phillips:
  * Fix warning in drivers/bus/fsl-mc/fsl_mc_dprc.c:173
  * Fixed linker errors when MC bus driver built as module

 drivers/bus/fsl-mc/Makefile      |   11 +-
 drivers/bus/fsl-mc/fsl_mc_bus.c  |   10 +
 drivers/bus/fsl-mc/fsl_mc_dprc.c |  402 ++++++++++++++++++++++++++++++++++++++
 include/linux/fsl_mc_private.h   |   10 +
 4 files changed, 428 insertions(+), 5 deletions(-)
 create mode 100644 drivers/bus/fsl-mc/fsl_mc_dprc.c

diff --git a/drivers/bus/fsl-mc/Makefile b/drivers/bus/fsl-mc/Makefile
index 2981d70..8ee3284 100644
--- a/drivers/bus/fsl-mc/Makefile
+++ b/drivers/bus/fsl-mc/Makefile
@@ -5,10 +5,11 @@
 #
 # This file is released under the GPLv2
 #
-obj-$(CONFIG_FSL_MC_BUS) += fsl_mc_bus_driver.o
+obj-$(CONFIG_FSL_MC_BUS) += fsl_mc_bus_driver.o \

-fsl_mc_bus_driver-objs := fsl_mc_bus.o \
-			      fsl_mc_sys.o \
-			      dprc.o \
-			      dpmng.o
+fsl_mc_bus_driver-objs :=	fsl_mc_bus.o \
+				fsl_mc_sys.o \
+				dprc.o \
+				dpmng.o \
+				fsl_mc_dprc.o

diff --git a/drivers/bus/fsl-mc/fsl_mc_bus.c b/drivers/bus/fsl-mc/fsl_mc_bus.c
index e3a994f..afe16cd 100644
--- a/drivers/bus/fsl-mc/fsl_mc_bus.c
+++ b/drivers/bus/fsl-mc/fsl_mc_bus.c
@@ -539,6 +539,7 @@ static int __init fsl_mc_bus_driver_init(void)
 {
 	int error = -EINVAL;
 	bool bus_registered = false;
+	bool platform_driver_registered = false;

 	mc_dev_cache = kmem_cache_create("fsl_mc_device",
 					 sizeof(struct fsl_mc_device), 0, 0,
@@ -564,8 +565,16 @@ static int __init fsl_mc_bus_driver_init(void)
 		goto error;
 	}

+	platform_driver_registered = true;
+	error = dprc_driver_init();
+	if (error < 0)
+		goto error;
+
 	return 0;
 error:
+	if (platform_driver_registered)
+		platform_driver_unregister(&fsl_mc_bus_driver);
+
 	if (bus_registered)
 		bus_unregister(&fsl_mc_bus_type);

@@ -582,6 +591,7 @@ static void __exit fsl_mc_bus_driver_exit(void)
 	if (WARN_ON(mc_dev_cache == NULL))
 		return;

+	dprc_driver_exit();
 	platform_driver_unregister(&fsl_mc_bus_driver);
 	bus_unregister(&fsl_mc_bus_type);
 	kmem_cache_destroy(mc_dev_cache);
diff --git a/drivers/bus/fsl-mc/fsl_mc_dprc.c b/drivers/bus/fsl-mc/fsl_mc_dprc.c
new file mode 100644
index 0000000..bf96de0
--- /dev/null
+++ b/drivers/bus/fsl-mc/fsl_mc_dprc.c
@@ -0,0 +1,402 @@
+/*
+ * Freescale daata path resource container (DPRC) driver
+ *
+ * Copyright (C) 2014 Freescale Semiconductor, Inc.
+ * Author: German Rivera <German.Rivera@freescale.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/fsl_mc_private.h>
+#include <linux/fsl_mc_sys.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include "fsl_dprc_cmd.h"
+
+struct dprc_child_objs {
+	int child_count;
+	struct dprc_obj_desc *child_array;
+};
+
+static int __fsl_mc_device_remove_if_not_in_mc(struct device *dev, void *data)
+{
+	int i;
+	struct dprc_child_objs *objs;
+	struct fsl_mc_device *mc_dev;
+
+	WARN_ON(dev == NULL);
+	WARN_ON(data == NULL);
+	mc_dev = to_fsl_mc_device(dev);
+	objs = data;
+
+	for (i = 0; i < objs->child_count; i++) {
+		struct dprc_obj_desc *obj_desc = &objs->child_array[i];
+
+		if (strlen(obj_desc->type) != 0 &&
+		    FSL_MC_DEVICE_MATCH(mc_dev, obj_desc))
+			break;
+	}
+
+	if (i == objs->child_count)
+		fsl_mc_device_remove(mc_dev);
+
+	return 0;
+}
+
+static int __fsl_mc_device_remove(struct device *dev, void *data)
+{
+	WARN_ON(dev == NULL);
+	WARN_ON(data != NULL);
+	fsl_mc_device_remove(to_fsl_mc_device(dev));
+	return 0;
+}
+
+/**
+ * dprc_remove_devices - Removes devices for objects removed from a DPRC
+ *
+ * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
+ * @obj_desc_array: array of object descriptors for child objects currently
+ * present in the DPRC in the MC.
+ * @num_child_objects_in_mc: number of entries in obj_desc_array
+ *
+ * Synchronizes the state of the Linux bus driver with the actual state of
+ * the MC by removing devices that represent MC objects that have
+ * been dynamically removed in the physical DPRC.
+ */
+static void dprc_remove_devices(struct fsl_mc_device *mc_bus_dev,
+				struct dprc_obj_desc *obj_desc_array,
+				int num_child_objects_in_mc)
+{
+	if (num_child_objects_in_mc != 0) {
+		/*
+		 * Remove child objects that are in the DPRC in Linux,
+		 * but not in the MC:
+		 */
+		struct dprc_child_objs objs;
+
+		objs.child_count = num_child_objects_in_mc;
+		objs.child_array = obj_desc_array;
+		device_for_each_child(&mc_bus_dev->dev, &objs,
+				      __fsl_mc_device_remove_if_not_in_mc);
+	} else {
+		/*
+		 * There are no child objects for this DPRC in the MC.
+		 * So, remove all the child devices from Linux:
+		 */
+		device_for_each_child(&mc_bus_dev->dev, NULL,
+				      __fsl_mc_device_remove);
+	}
+}
+
+static int __fsl_mc_device_match(struct device *dev, void *data)
+{
+	struct dprc_obj_desc *obj_desc = data;
+	struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+
+	return FSL_MC_DEVICE_MATCH(mc_dev, obj_desc);
+}
+
+static struct fsl_mc_device *fsl_mc_device_lookup(struct dprc_obj_desc
+								*obj_desc,
+						  struct fsl_mc_device
+								*mc_bus_dev)
+{
+	struct device *dev;
+
+	dev = device_find_child(&mc_bus_dev->dev, obj_desc,
+				__fsl_mc_device_match);
+
+	return (dev != NULL) ? to_fsl_mc_device(dev) : NULL;
+}
+
+/**
+ * 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
+ * @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
+ *
+ * Synchronizes the state of the Linux bus driver with the actual
+ * state of the MC by adding objects that have been newly discovered
+ * in the physical DPRC.
+ */
+static void dprc_add_new_devices(struct fsl_mc_device *mc_bus_dev,
+				 struct dprc_obj_desc *obj_desc_array,
+				 int num_child_objects_in_mc)
+{
+	int error;
+	int i;
+
+	for (i = 0; i < num_child_objects_in_mc; i++) {
+		struct dprc_region_desc region_desc;
+		struct fsl_mc_device *child_dev;
+		struct fsl_mc_io *mc_io = NULL;
+		struct dprc_obj_desc *obj_desc = &obj_desc_array[i];
+
+		if (strlen(obj_desc->type) == 0)
+			continue;
+		/*
+		 * Check if device is already known to Linux:
+		 */
+		child_dev = fsl_mc_device_lookup(obj_desc, mc_bus_dev);
+		if (child_dev != NULL)
+			continue;
+
+		if (strcmp(obj_desc->type, "dprc") == 0) {
+			/*
+			 * Get the MC portal physical address for the device
+			 * (specified in the device's first MMIO region):
+			 */
+			if (WARN_ON(obj_desc->region_count == 0))
+				continue;
+
+			error = dprc_get_obj_region(mc_bus_dev->mc_io,
+						    mc_bus_dev->mc_handle,
+						    obj_desc->type,
+						    obj_desc->id,
+						    0, &region_desc);
+			if (error < 0) {
+				dev_err(&mc_bus_dev->dev,
+					"dprc_get_obj_region() failed: %d\n",
+					error);
+				continue;
+			}
+
+			if (region_desc.size !=
+					mc_bus_dev->mc_io->portal_size) {
+				error = -EINVAL;
+				dev_err(&mc_bus_dev->dev,
+					"Invalid MC portal size: %u\n",
+					region_desc.size);
+				continue;
+			}
+
+			error = fsl_create_mc_io(&mc_bus_dev->dev,
+						 region_desc.base_paddr,
+						 region_desc.size, 0, &mc_io);
+			if (error < 0)
+				continue;
+		}
+
+		error = fsl_mc_device_add(obj_desc, mc_io, &mc_bus_dev->dev,
+					  &child_dev);
+		if (error < 0) {
+			if (mc_io != NULL)
+				fsl_destroy_mc_io(mc_io);
+
+			continue;
+		}
+	}
+}
+
+/**
+ * dprc_scan_objects - Discover objects in a DPRC
+ *
+ * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
+ *
+ * Detects objects added and removed from a DPRC and synchronizes the
+ * state of the Linux bus driver, MC by adding and removing
+ * devices accordingly.
+ */
+int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev)
+{
+	int num_child_objects;
+	int dprc_get_obj_failures;
+	int error = -EINVAL;
+	struct dprc_obj_desc *child_obj_desc_array = NULL;
+
+	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);
+		goto out;
+	}
+
+	if (num_child_objects != 0) {
+		int i;
+
+		child_obj_desc_array =
+		    devm_kmalloc_array(&mc_bus_dev->dev, num_child_objects,
+				       sizeof(*child_obj_desc_array),
+				       GFP_KERNEL);
+		if (child_obj_desc_array == NULL) {
+			error = -ENOMEM;
+			dev_err(&mc_bus_dev->dev,
+				"No memory to allocate obj_desc array\n");
+			goto out;
+		}
+
+		/*
+		 * Discover objects currently present in the physical DPRC:
+		 */
+		dprc_get_obj_failures = 0;
+		for (i = 0; i < num_child_objects; i++) {
+			struct dprc_obj_desc *obj_desc =
+			    &child_obj_desc_array[i];
+
+			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);
+				/*
+				 * Mark the obj entry as "invalid", by using the
+				 * empty string as obj type:
+				 */
+				obj_desc->type[0] = '\0';
+				obj_desc->id = error;
+				dprc_get_obj_failures++;
+				continue;
+			}
+
+			dev_info(&mc_bus_dev->dev,
+				 "Discovered object: type %s, id %d\n",
+				 obj_desc->type, obj_desc->id);
+		}
+
+		if (dprc_get_obj_failures != 0) {
+			dev_err(&mc_bus_dev->dev,
+				"%d out of %d devices could not be retrieved\n",
+				dprc_get_obj_failures, num_child_objects);
+		}
+	}
+
+	dprc_remove_devices(mc_bus_dev, child_obj_desc_array,
+			    num_child_objects);
+
+	dprc_add_new_devices(mc_bus_dev, child_obj_desc_array,
+			     num_child_objects);
+
+	error = 0;
+out:
+	if (child_obj_desc_array != NULL)
+		devm_kfree(&mc_bus_dev->dev, child_obj_desc_array);
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(dprc_scan_objects);
+
+/**
+ * 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
+ *
+ * Scans the physical DPRC and synchronizes the state of the Linux
+ * 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)
+{
+	int error = -EINVAL;
+
+	error = dprc_scan_objects(mc_bus_dev);
+	if (error < 0)
+		goto error;
+
+	return 0;
+error:
+	return error;
+}
+EXPORT_SYMBOL_GPL(dprc_scan_container);
+
+/**
+ * dprc_probe - callback invoked when a DPRC is being bound to this driver
+ *
+ * @mc_dev: Pointer to fsl-mc device representing a DPRC
+ *
+ * It opens the physical DPRC in the MC.
+ * It scans the DPRC to discover the MC objects contained in it.
+ */
+static int dprc_probe(struct fsl_mc_device *mc_dev)
+{
+	int error = -EINVAL;
+	bool dprc_opened = false;
+
+	if (WARN_ON(strcmp(mc_dev->obj_desc.type, "dprc") != 0))
+		goto error;
+
+	error = dprc_open(mc_dev->mc_io, mc_dev->obj_desc.id,
+			  &mc_dev->mc_handle);
+	if (error < 0) {
+		dev_err(&mc_dev->dev, "dprc_open() failed: %d\n", error);
+		goto error;
+	}
+
+	dprc_opened = true;
+	error = dprc_scan_container(mc_dev);
+	if (error < 0)
+		goto error;
+
+	dev_info(&mc_dev->dev, "DPRC device bound to driver");
+	return 0;
+error:
+	if (dprc_opened)
+		(void)dprc_close(mc_dev->mc_io, mc_dev->mc_handle);
+
+	return error;
+}
+
+/**
+ * dprc_remove - callback invoked when a DPRC is being unbound from this driver
+ *
+ * @mc_dev: Pointer to fsl-mc device representing the DPRC
+ *
+ * It removes the DPRC's child objects from Linux (not from the MC) and
+ * closes the DPRC device in the MC.
+ */
+static int dprc_remove(struct fsl_mc_device *mc_dev)
+{
+	int error = -EINVAL;
+
+	if (WARN_ON(strcmp(mc_dev->obj_desc.type, "dprc") != 0))
+		goto error;
+	if (WARN_ON(mc_dev->mc_io == NULL))
+		goto error;
+
+	device_for_each_child(&mc_dev->dev, NULL, __fsl_mc_device_remove);
+	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);
+
+	dev_info(&mc_dev->dev, "DPRC device unbound from driver");
+	return 0;
+error:
+	return error;
+}
+
+static const struct fsl_mc_device_match_id match_id_table[] = {
+	{
+	 .vendor = FSL_MC_VENDOR_FREESCALE,
+	 .obj_type = "dprc",
+	 .ver_major = DPRC_VER_MAJOR,
+	 .ver_minor = DPRC_VER_MINOR},
+	{.vendor = 0x0},
+};
+
+static struct fsl_mc_driver dprc_driver = {
+	.driver = {
+		   .name = FSL_MC_DPRC_DRIVER_NAME,
+		   .owner = THIS_MODULE,
+		   .pm = NULL,
+		   },
+	.match_id_table = match_id_table,
+	.probe = dprc_probe,
+	.remove = dprc_remove,
+};
+
+int __init dprc_driver_init(void)
+{
+	return fsl_mc_driver_register(&dprc_driver);
+}
+
+void __exit dprc_driver_exit(void)
+{
+	fsl_mc_driver_unregister(&dprc_driver);
+}
diff --git a/include/linux/fsl_mc_private.h b/include/linux/fsl_mc_private.h
index 3ff3f2b..52721f7 100644
--- a/include/linux/fsl_mc_private.h
+++ b/include/linux/fsl_mc_private.h
@@ -15,6 +15,8 @@
 #include <linux/mutex.h>
 #include <linux/stringify.h>

+#define FSL_MC_DPRC_DRIVER_NAME    "fsl_mc_dprc"
+
 #define FSL_MC_DEVICE_MATCH(_mc_dev, _obj_desc) \
 	(strcmp((_mc_dev)->obj_desc.type, (_obj_desc)->type) == 0 && \
 	 (_mc_dev)->obj_desc.id == (_obj_desc)->id)
@@ -30,4 +32,12 @@ 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 __must_check dprc_scan_container(struct fsl_mc_device *mc_bus_dev);
+
+int __must_check dprc_scan_objects(struct fsl_mc_device *mc_bus_dev);
+
+int __init dprc_driver_init(void);
+
+void __exit dprc_driver_exit(void);
+
 #endif /* _FSL_MC_PRIVATE_H_ */
--
1.7.9.7


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

* Re: [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series
  2014-09-19 22:49 [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series J. German Rivera
                   ` (2 preceding siblings ...)
  2014-09-19 22:49 ` [PATCH 3/3 v2] drivers/bus: Device driver for FSL-MC DPRC devices J. German Rivera
@ 2014-09-22 16:53 ` Kim Phillips
  2014-09-22 17:59   ` Stuart Yoder
  3 siblings, 1 reply; 20+ messages in thread
From: Kim Phillips @ 2014-09-22 16:53 UTC (permalink / raw)
  To: J. German Rivera
  Cc: gregkh, arnd, linux-kernel, stuart.yoder, scottwood, agraf,
	linuxppc-release

On Fri, 19 Sep 2014 17:49:38 -0500
"J. German Rivera" <German.Rivera@freescale.com> wrote:

> CHANGE HISTORY
> 
> Issues pending resolution not addressed by v2:
> - What to do with Doxygen comments in patch 1

It's clear they should be removed.

> - Whether to move or not FSL-specific header files added in include/linux,
>   by this patch series, to another location

there wasn't a valid objection against moving them under fsl/ and
changing them to use dashes instead of underscores, was there?

> - Whether to change or not the wording of the licenses used in some
>   source files

I agree with Alex to use the existing kernel versions, to be on
the safe side, but IANAL - did you want to go to the source of the
change (presumably a lawyer?) and find out why the contributors
clause was omitted and how they think that's appropriate in an
upstream submission?  If not, please resubmit after converting all
files to use the existing common kernel license text.

Thanks,

Kim

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

* RE: [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series
  2014-09-22 16:53 ` [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series Kim Phillips
@ 2014-09-22 17:59   ` Stuart Yoder
  2014-09-22 22:03     ` Kim Phillips
  2014-09-23 14:52     ` German Rivera
  0 siblings, 2 replies; 20+ messages in thread
From: Stuart Yoder @ 2014-09-22 17:59 UTC (permalink / raw)
  To: Kim Phillips, Jose Rivera
  Cc: gregkh, arnd, linux-kernel, Scott Wood, agraf, linuxppc-release



> -----Original Message-----
> From: Kim Phillips [mailto:kim.phillips@freescale.com]
> Sent: Monday, September 22, 2014 11:53 AM
> To: Rivera Jose-B46482
> Cc: gregkh@linuxfoundation.org; arnd@arndb.de; linux-kernel@vger.kernel.org; Yoder Stuart-B08248; Wood Scott-
> B07421; agraf@suse.de; linuxppc-release@linux.freescale.net
> Subject: Re: [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series
> 
> On Fri, 19 Sep 2014 17:49:38 -0500
> "J. German Rivera" <German.Rivera@freescale.com> wrote:
> 
> > CHANGE HISTORY
> >
> > Issues pending resolution not addressed by v2:
> > - What to do with Doxygen comments in patch 1
> 
> It's clear they should be removed.
> 
> > - Whether to move or not FSL-specific header files added in include/linux,
> >   by this patch series, to another location
> 
> there wasn't a valid objection against moving them under fsl/ and
> changing them to use dashes instead of underscores, was there?

There was no objection, but here is the observation.  The current
convention seems to be that under include/linux are 'subsystem'
types--
   include/linux/mmc
   include/linux/spi
   include/linux/raid
    etc

There is no other "company" that has an include/linux/[company-name] that I can
see.  Freescale seems to be the only one.   And there is only a single driver
in there.  So it looks like a complete anomaly.

Why is that?

I guess we could try moving our stuff to incluce/linux/fsl and see if there is
any negative feedback on it.

Stuart

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

* Re: [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series
  2014-09-22 17:59   ` Stuart Yoder
@ 2014-09-22 22:03     ` Kim Phillips
  2014-09-23 14:52     ` German Rivera
  1 sibling, 0 replies; 20+ messages in thread
From: Kim Phillips @ 2014-09-22 22:03 UTC (permalink / raw)
  To: Yoder Stuart-B08248
  Cc: Rivera Jose-B46482, gregkh, arnd, linux-kernel,
	Wood Scott-B07421, agraf, linuxppc-release

On Mon, 22 Sep 2014 12:59:21 -0500
Yoder Stuart-B08248 <stuart.yoder@freescale.com> wrote:

> > From: Kim Phillips [mailto:kim.phillips@freescale.com]
> > Sent: Monday, September 22, 2014 11:53 AM
> > 
> > On Fri, 19 Sep 2014 17:49:38 -0500
> > "J. German Rivera" <German.Rivera@freescale.com> wrote:
> > 
> > > CHANGE HISTORY
> > >
> > > Issues pending resolution not addressed by v2:
> > > - What to do with Doxygen comments in patch 1
> > 
> > It's clear they should be removed.
> > 
> > > - Whether to move or not FSL-specific header files added in include/linux,
> > >   by this patch series, to another location
> > 
> > there wasn't a valid objection against moving them under fsl/ and
> > changing them to use dashes instead of underscores, was there?
> 
> There was no objection, but here is the observation.  The current
> convention seems to be that under include/linux are 'subsystem'
> types--
>    include/linux/mmc
>    include/linux/spi
>    include/linux/raid
>     etc
> 
> There is no other "company" that has an include/linux/[company-name] that I can
> see.  Freescale seems to be the only one.   And there is only a single driver
> in there.  So it looks like a complete anomaly.
> 
> Why is that?
> 
> I guess we could try moving our stuff to incluce/linux/fsl and see if there is
> any negative feedback on it.

these two commits:

commit 9a32299 "powerpc, dma: move bestcomm driver from
arch/powerpc/sysdev to drivers/dma"

commit 3946860 "mxs-dma : move the mxs dma.h to a more common place"

create/update files in include/linux/fsl for the same reason as this
patchseries wants, i.e., in order to support multiple drivers
including the files, and in an arch- (and mach-) independent manner.

hth,

Kim

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

* Re: [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series
  2014-09-22 17:59   ` Stuart Yoder
  2014-09-22 22:03     ` Kim Phillips
@ 2014-09-23 14:52     ` German Rivera
  1 sibling, 0 replies; 20+ messages in thread
From: German Rivera @ 2014-09-23 14:52 UTC (permalink / raw)
  To: Yoder Stuart-B08248, Phillips Kim-R1AAHA
  Cc: gregkh, arnd, linux-kernel, Wood Scott-B07421, agraf,
	linuxppc-release, Erez Nir-RM30794



On 09/22/2014 12:59 PM, Yoder Stuart-B08248 wrote:
>
>
>> -----Original Message-----
>> From: Kim Phillips [mailto:kim.phillips@freescale.com]
>> Sent: Monday, September 22, 2014 11:53 AM
>> To: Rivera Jose-B46482
>> Cc: gregkh@linuxfoundation.org; arnd@arndb.de; linux-kernel@vger.kernel.org; Yoder Stuart-B08248; Wood Scott-
>> B07421; agraf@suse.de; linuxppc-release@linux.freescale.net
>> Subject: Re: [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series
>>
>> On Fri, 19 Sep 2014 17:49:38 -0500
>> "J. German Rivera" <German.Rivera@freescale.com> wrote:
>>
>>> CHANGE HISTORY
>>>
>>> Issues pending resolution not addressed by v2:
>>> - What to do with Doxygen comments in patch 1
>>
>> It's clear they should be removed.
>>
>>> - Whether to move or not FSL-specific header files added in include/linux,
>>>    by this patch series, to another location
>>
>> there wasn't a valid objection against moving them under fsl/ and
>> changing them to use dashes instead of underscores, was there?
>
> There was no objection, but here is the observation.  The current
> convention seems to be that under include/linux are 'subsystem'
> types--
>     include/linux/mmc
>     include/linux/spi
>     include/linux/raid
>      etc
>
> There is no other "company" that has an include/linux/[company-name] that I can
> see.  Freescale seems to be the only one.   And there is only a single driver
> in there.  So it looks like a complete anomaly.
>
> Why is that?
>
> I guess we could try moving our stuff to incluce/linux/fsl and see if there is
> any negative feedback on it.
>

Ok, unless there is any objection, we will do the following file renamings:

Patch 1/3
drivers/bus/fsl-mc/fsl_dpmng_cmd.h -> drivers/bus/fsl-mc/dpmng-cmd.h
drivers/bus/fsl-mc/fsl_dprc_cmd.h -> drivers/bus/fsl-mc/dprc-cmd.h
drivers/bus/fsl-mc/fsl_mc_sys.c -> drivers/bus/fsl-mc/mc-sys.c
include/linux/fsl_dpmng.h -> include/linux/fsl/dpmng.h
include/linux/fsl_dprc.h -> include/linux/fsl/dprc.h
include/linux/fsl_mc_cmd.h -> include/linux/fsl/mc-cmd.h
include/linux/fsl_mc_sys.h -> include/linux/fsl/mc-sys.h

Patch 2/3
drivers/bus/fsl-mc/fsl_mc_bus.c -> drivers/bus/fsl-mc/mc-bus.c
include/linux/fsl_mc.h -> include/linux/fsl/mc.h
include/linux/fsl_mc_private.h -> include/linux/fsl/mc-private.h

Patch 3/3
drivers/bus/fsl-mc/fsl_mc_dprc.c -> drivers/bus/fsl-mc/dprc-driver.c
(there is a file already named drivers/bus/fsl-mc/dprc.c)

> Stuart
>

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

* Re: [PATCH 1/3 v2] drivers/bus: Added Freescale Management Complex APIs
  2014-09-19 22:49 ` [PATCH 1/3 v2] drivers/bus: Added Freescale Management Complex APIs J. German Rivera
@ 2014-09-24  0:49   ` Kim Phillips
  2014-09-25  2:23     ` German Rivera
  0 siblings, 1 reply; 20+ messages in thread
From: Kim Phillips @ 2014-09-24  0:49 UTC (permalink / raw)
  To: J. German Rivera
  Cc: gregkh, arnd, linux-kernel, stuart.yoder, Kim.Phillips,
	scottwood, agraf, linuxppc-release

On Fri, 19 Sep 2014 17:49:39 -0500
"J. German Rivera" <German.Rivera@freescale.com> wrote:

> +int mc_get_version(struct fsl_mc_io *mc_io, struct mc_version *mc_ver_info)
...
> +	err = mc_send_command(mc_io, &cmd);
> +	if (err)
> +		return err;
> +
> +		DPMNG_RSP_GET_VERSION(cmd, mc_ver_info);

alignment

> +int dpmng_load_aiop(struct fsl_mc_io *mc_io,
> +		    int aiop_tile_id, uint8_t *img_addr, int img_size)
> +{
> +	struct mc_command cmd = { 0 };
> +	uint64_t img_paddr = virt_to_phys(img_addr);

Direct use of virt_to_phys by drivers is deprecated; this code needs
to map the i/o space via the DMA API.  This is in order to handle
situations where e.g., the device sitting behind an IOMMU.  See
Documentation/DMA-API* for more info.

> +/**
> + * Delay in microseconds between polling iterations while
> + * waiting for MC command completion
> + */
> +#define MC_CMD_COMPLETION_POLLING_INTERVAL_USECS    500	/* 0.5 ms */
> +
> +int __must_check fsl_create_mc_io(struct device *dev,
> +				  phys_addr_t mc_portal_phys_addr,
> +				  uint32_t mc_portal_size,
> +				  uint32_t flags, struct fsl_mc_io **new_mc_io)
> +{
> +	struct fsl_mc_io *mc_io;
> +	void __iomem *mc_portal_virt_addr;
> +	struct resource *res;
> +
> +	mc_io = devm_kzalloc(dev, sizeof(*mc_io), GFP_KERNEL);
> +	if (mc_io == NULL)
> +		return -ENOMEM;
> +
> +	mc_io->dev = dev;
> +	mc_io->flags = flags;
> +	mc_io->portal_phys_addr = mc_portal_phys_addr;
> +	mc_io->portal_size = mc_portal_size;
> +	if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS)
> +		spin_lock_init(&mc_io->spinlock);

I'm confused - this patseries doesn't register an interrupt handler,
so this can't be true (or it's premature if it will, in which case
it should be left out for now).

However, if somehow users of this code register an IRQ handler for
the portal (I don't see any users to tell how they get the IRQ line
either?), then it's up to them to establish mutual exclusion rules
for access, among themselves.  Unless you think they will be calling
mc_send_command from h/w IRQ context, in which case I'd reconsider
that assumption because send_command looks like it'd take too long
to get an answer from the h/w - IRQ handlers should just ack the h/w
IRQ, and notify the scheduler that the driver has work to do (in s/w
IRQ context perhaps).

> +	else if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS)
> +		mutex_init(&mc_io->mutex);

I'd assume SHARED_BY_THREADS to always be true in linux.

> +	res = devm_request_mem_region(dev,
> +				      mc_portal_phys_addr,
> +				      mc_portal_size,
> +				      "mc_portal");
> +	if (res == NULL) {
> +		dev_err(dev,
> +			"devm_request_mem_region failed for MC portal %#llx\n",
> +			mc_portal_phys_addr);
> +		return -EBUSY;
> +	}
> +
> +	mc_portal_virt_addr = devm_ioremap_nocache(dev,
> +						   mc_portal_phys_addr,
> +						   mc_portal_size);
> +	if (mc_portal_virt_addr == NULL) {
> +		dev_err(dev,
> +			"devm_ioremap_nocache failed for MC portal %#llx\n",
> +			mc_portal_phys_addr);
> +		return -ENXIO;
> +	}
> +
> +	mc_io->portal_virt_addr = mc_portal_virt_addr;
> +	*new_mc_io = mc_io;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(fsl_create_mc_io);
> +
> +void fsl_destroy_mc_io(struct fsl_mc_io *mc_io)
> +{
> +	if (WARN_ON(mc_io->portal_virt_addr == NULL))
> +		return;

this is unnecessary - you'll get the stack trace anyway, and users
calling destroy on a not successfully created mc_io object should
not get the luxury of maybe being able to continue after the stack
trace, after possibly leaking memory.

> +	mc_io->portal_virt_addr = NULL;
> +	devm_kfree(mc_io->dev, mc_io);

like I said before, there's really no point in clearing something
out right before it's freed.

> +}
> +EXPORT_SYMBOL_GPL(fsl_destroy_mc_io);
> +
> +static int mc_status_to_error(enum mc_cmd_status status)
> +{
> +	static const int mc_status_to_error_map[] = {
> +		[MC_CMD_STATUS_OK] = 0,
> +		[MC_CMD_STATUS_AUTH_ERR] = -EACCES,
> +		[MC_CMD_STATUS_NO_PRIVILEGE] = -EPERM,
> +		[MC_CMD_STATUS_DMA_ERR] = -EIO,
> +		[MC_CMD_STATUS_CONFIG_ERR] = -ENXIO,
> +		[MC_CMD_STATUS_TIMEOUT] = -ETIMEDOUT,
> +		[MC_CMD_STATUS_NO_RESOURCE] = -ENAVAIL,
> +		[MC_CMD_STATUS_NO_MEMORY] = -ENOMEM,
> +		[MC_CMD_STATUS_BUSY] = -EBUSY,
> +		[MC_CMD_STATUS_UNSUPPORTED_OP] = -ENOTSUPP,
> +		[MC_CMD_STATUS_INVALID_STATE] = -ENODEV,
> +	};
> +
> +	if (WARN_ON(status >= ARRAY_SIZE(mc_status_to_error_map)))
> +		return -EINVAL;
> +	return mc_status_to_error_map[status];
> +}

great - CONFIG_ERR is now -ENXIO instead of -EINVAL, so at least
it's now mutually exclusive, but it doesn't handle
MC_CMD_STATUS_READY - should it?

> +int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
> +{
> +	enum mc_cmd_status status;
> +	int error;
> +	unsigned long irqsave_flags = 0;
> +	unsigned long jiffies_until_timeout =
> +	    jiffies + MC_CMD_COMPLETION_TIMEOUT_JIFFIES;
> +
> +	/*
> +	 * Acquire lock depending on mc_io flags:
> +	 */
> +	if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS) {
> +		if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS)
> +			spin_lock_irqsave(&mc_io->spinlock, irqsave_flags);
> +		else
> +			spin_lock(&mc_io->spinlock);
> +	} else if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS) {
> +		mutex_lock(&mc_io->mutex);
> +	}

again, I think we need to drop the coming from h/w IRQ context here
(SHARED_BY_INT_HANDLERS); there's no IRQ handlers in this
patchseries, and calling this function from an IRQ handler would be
prohibitively wasteful, guessing by the udelay and timeout values
below.

Can we just mutex_lock for now, and unconditionally (no
SHARED_BY_THREADS check), to protect from nesting?

> +	/*
> +	 * 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 udelay()
> +		 */
> +		udelay(MC_CMD_COMPLETION_POLLING_INTERVAL_USECS);

this pauses any caller for 0.5ms on every successful command
write.  Can the next submission of the patchseries wait until
completion IRQs are indeed supported, since both that and the above
locking needs to be resolved?

Kim

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

* Re: [PATCH 1/3 v2] drivers/bus: Added Freescale Management Complex APIs
  2014-09-24  0:49   ` Kim Phillips
@ 2014-09-25  2:23     ` German Rivera
  2014-09-25  3:40       ` Kim Phillips
  0 siblings, 1 reply; 20+ messages in thread
From: German Rivera @ 2014-09-25  2:23 UTC (permalink / raw)
  To: Kim Phillips
  Cc: gregkh, arnd, linux-kernel, stuart.yoder, scottwood, agraf,
	linuxppc-release, Erez Nir-RM30794



On 09/23/2014 07:49 PM, Kim Phillips wrote:
> On Fri, 19 Sep 2014 17:49:39 -0500
> "J. German Rivera" <German.Rivera@freescale.com> wrote:
>
>> +int mc_get_version(struct fsl_mc_io *mc_io, struct mc_version *mc_ver_info)
> ...
>> +	err = mc_send_command(mc_io, &cmd);
>> +	if (err)
>> +		return err;
>> +
>> +		DPMNG_RSP_GET_VERSION(cmd, mc_ver_info);
>
> alignment
>
>> +int dpmng_load_aiop(struct fsl_mc_io *mc_io,
>> +		    int aiop_tile_id, uint8_t *img_addr, int img_size)
>> +{
>> +	struct mc_command cmd = { 0 };
>> +	uint64_t img_paddr = virt_to_phys(img_addr);
>
> Direct use of virt_to_phys by drivers is deprecated; this code needs
> to map the i/o space via the DMA API.  This is in order to handle
> situations where e.g., the device sitting behind an IOMMU.  See
> Documentation/DMA-API* for more info.
>
Ok, we will make this change in the v3 respin.

>> +/**
>> + * Delay in microseconds between polling iterations while
>> + * waiting for MC command completion
>> + */
>> +#define MC_CMD_COMPLETION_POLLING_INTERVAL_USECS    500	/* 0.5 ms */
>> +
>> +int __must_check fsl_create_mc_io(struct device *dev,
>> +				  phys_addr_t mc_portal_phys_addr,
>> +				  uint32_t mc_portal_size,
>> +				  uint32_t flags, struct fsl_mc_io **new_mc_io)
>> +{
>> +	struct fsl_mc_io *mc_io;
>> +	void __iomem *mc_portal_virt_addr;
>> +	struct resource *res;
>> +
>> +	mc_io = devm_kzalloc(dev, sizeof(*mc_io), GFP_KERNEL);
>> +	if (mc_io == NULL)
>> +		return -ENOMEM;
>> +
>> +	mc_io->dev = dev;
>> +	mc_io->flags = flags;
>> +	mc_io->portal_phys_addr = mc_portal_phys_addr;
>> +	mc_io->portal_size = mc_portal_size;
>> +	if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS)
>> +		spin_lock_init(&mc_io->spinlock);
>
> I'm confused - this patseries doesn't register an interrupt handler,
> so this can't be true (or it's premature if it will, in which case
> it should be left out for now).
>
> However, if somehow users of this code register an IRQ handler for
> the portal (I don't see any users to tell how they get the IRQ line
> either?), then it's up to them to establish mutual exclusion rules
> for access, among themselves.  Unless you think they will be calling
> mc_send_command from h/w IRQ context, in which case I'd reconsider
> that assumption because send_command looks like it'd take too long
> to get an answer from the h/w - IRQ handlers should just ack the h/w
> IRQ, and notify the scheduler that the driver has work to do (in s/w
> IRQ context perhaps).
>
Although not included in this patch series, there are cases in
subsequent patch series, in which mc_send_command() will need to be
called from interrupt context.

For example, the dprc_get_irq_status() needs to be called from the DPRC
ISR to determine the actual cause of the interrupt. Also, to clear
the interrupt the dprc_clear_irq_status() needs to be called from the
ISR. Both od these functions call mc_send_command():

int dprc_get_irq_status(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
			uint8_t irq_index, uint32_t *status)
{
	struct mc_command cmd = { 0 };
	int err;

	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_IRQ_STATUS,
					  DPRC_CMDSZ_GET_IRQ_STATUS,
					  MC_CMD_PRI_LOW, dprc_handle);

	DPRC_CMD_GET_IRQ_STATUS(cmd, irq_index);
	err = mc_send_command(mc_io, &cmd);
	if (!err)
		DPRC_RSP_GET_IRQ_STATUS(cmd, *status);

	return err;
}

int dprc_clear_irq_status(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
			  uint8_t irq_index, uint32_t status)
{
	struct mc_command cmd = { 0 };

	cmd.header = mc_encode_cmd_header(DPRC_CMDID_CLEAR_IRQ_STATUS,
					  DPRC_CMDSZ_CLEAR_IRQ_STATUS,
					  MC_CMD_PRI_LOW, dprc_handle);

	DPRC_CMD_CLEAR_IRQ_STATUS(cmd, status, irq_index);
	return mc_send_command(mc_io, &cmd);
}

>> +	else if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS)
>> +		mutex_init(&mc_io->mutex);
>
> I'd assume SHARED_BY_THREADS to always be true in linux.
>
Not, if mc_send_command() is called from interrupt context, as
explained above. However, since this patch series does not include
any interrupt handlers, we can remove the
FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS flag and the associated spinlock,
from this patch series.

>> +	res = devm_request_mem_region(dev,
>> +				      mc_portal_phys_addr,
>> +				      mc_portal_size,
>> +				      "mc_portal");
>> +	if (res == NULL) {
>> +		dev_err(dev,
>> +			"devm_request_mem_region failed for MC portal %#llx\n",
>> +			mc_portal_phys_addr);
>> +		return -EBUSY;
>> +	}
>> +
>> +	mc_portal_virt_addr = devm_ioremap_nocache(dev,
>> +						   mc_portal_phys_addr,
>> +						   mc_portal_size);
>> +	if (mc_portal_virt_addr == NULL) {
>> +		dev_err(dev,
>> +			"devm_ioremap_nocache failed for MC portal %#llx\n",
>> +			mc_portal_phys_addr);
>> +		return -ENXIO;
>> +	}
>> +
>> +	mc_io->portal_virt_addr = mc_portal_virt_addr;
>> +	*new_mc_io = mc_io;
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(fsl_create_mc_io);
>> +
>> +void fsl_destroy_mc_io(struct fsl_mc_io *mc_io)
>> +{
>> +	if (WARN_ON(mc_io->portal_virt_addr == NULL))
>> +		return;
>
> this is unnecessary - you'll get the stack trace anyway, and users
> calling destroy on a not successfully created mc_io object should
> not get the luxury of maybe being able to continue after the stack
> trace, after possibly leaking memory.
Ok, I'ĺl remove this WARN_ON in the v3 respin.

>
>> +	mc_io->portal_virt_addr = NULL;
>> +	devm_kfree(mc_io->dev, mc_io);
>
> like I said before, there's really no point in clearing something
> out right before it's freed.
>
I disagree. This can help detect cases of double-freeing.

>> +}
>> +EXPORT_SYMBOL_GPL(fsl_destroy_mc_io);
>> +
>> +static int mc_status_to_error(enum mc_cmd_status status)
>> +{
>> +	static const int mc_status_to_error_map[] = {
>> +		[MC_CMD_STATUS_OK] = 0,
>> +		[MC_CMD_STATUS_AUTH_ERR] = -EACCES,
>> +		[MC_CMD_STATUS_NO_PRIVILEGE] = -EPERM,
>> +		[MC_CMD_STATUS_DMA_ERR] = -EIO,
>> +		[MC_CMD_STATUS_CONFIG_ERR] = -ENXIO,
>> +		[MC_CMD_STATUS_TIMEOUT] = -ETIMEDOUT,
>> +		[MC_CMD_STATUS_NO_RESOURCE] = -ENAVAIL,
>> +		[MC_CMD_STATUS_NO_MEMORY] = -ENOMEM,
>> +		[MC_CMD_STATUS_BUSY] = -EBUSY,
>> +		[MC_CMD_STATUS_UNSUPPORTED_OP] = -ENOTSUPP,
>> +		[MC_CMD_STATUS_INVALID_STATE] = -ENODEV,
>> +	};
>> +
>> +	if (WARN_ON(status >= ARRAY_SIZE(mc_status_to_error_map)))
>> +		return -EINVAL;
>> +	return mc_status_to_error_map[status];
>> +}
>
> great - CONFIG_ERR is now -ENXIO instead of -EINVAL, so at least
> it's now mutually exclusive, but it doesn't handle
> MC_CMD_STATUS_READY - should it?
>
No. MC_CMD_STATUS_READY does not need to be seen by mc_send_command()
callers. It is only used to know when to stop polling the MC for
command completion.

>> +int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
>> +{
>> +	enum mc_cmd_status status;
>> +	int error;
>> +	unsigned long irqsave_flags = 0;
>> +	unsigned long jiffies_until_timeout =
>> +	    jiffies + MC_CMD_COMPLETION_TIMEOUT_JIFFIES;
>> +
>> +	/*
>> +	 * Acquire lock depending on mc_io flags:
>> +	 */
>> +	if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS) {
>> +		if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS)
>> +			spin_lock_irqsave(&mc_io->spinlock, irqsave_flags);
>> +		else
>> +			spin_lock(&mc_io->spinlock);
>> +	} else if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS) {
>> +		mutex_lock(&mc_io->mutex);
>> +	}
>
> again, I think we need to drop the coming from h/w IRQ context here
> (SHARED_BY_INT_HANDLERS); there's no IRQ handlers in this
> patchseries, and calling this function from an IRQ handler would be
> prohibitively wasteful, guessing by the udelay and timeout values
> below.
>
> Can we just mutex_lock for now, and unconditionally (no
> SHARED_BY_THREADS check), to protect from nesting?
>
I would still prefer to keep the SHARED_BY_THREADS flag, to give option 
of not doing any locking, in cases where the portal used in 
mc_send_command() is not shared among concurrent callers

What do you mean by nesting in this case?

>> +	/*
>> +	 * 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 udelay()
>> +		 */
>> +		udelay(MC_CMD_COMPLETION_POLLING_INTERVAL_USECS);
>
> this pauses any caller for 0.5ms on every successful command
> write.  Can the next submission of the patchseries wait until
> completion IRQs are indeed supported, since both that and the above
> locking needs to be resolved?
>
No. Interrupt handlers will come in a later patch series as they are
not needed for using the basic MC functionality.

> Kim
>

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

* Re: [PATCH 1/3 v2] drivers/bus: Added Freescale Management Complex APIs
  2014-09-25  2:23     ` German Rivera
@ 2014-09-25  3:40       ` Kim Phillips
  2014-09-25 15:44         ` German Rivera
  0 siblings, 1 reply; 20+ messages in thread
From: Kim Phillips @ 2014-09-25  3:40 UTC (permalink / raw)
  To: German Rivera
  Cc: gregkh, arnd, linux-kernel, stuart.yoder, scottwood, agraf,
	linuxppc-release, Erez Nir-RM30794

On Wed, 24 Sep 2014 21:23:59 -0500
German Rivera <German.Rivera@freescale.com> wrote:

> On 09/23/2014 07:49 PM, Kim Phillips wrote:
> > On Fri, 19 Sep 2014 17:49:39 -0500
> > "J. German Rivera" <German.Rivera@freescale.com> wrote:
> >
> >> + * Delay in microseconds between polling iterations while
> >> + * waiting for MC command completion
> >> + */
> >> +#define MC_CMD_COMPLETION_POLLING_INTERVAL_USECS    500	/* 0.5 ms */
> >> +
> >> +int __must_check fsl_create_mc_io(struct device *dev,
> >> +				  phys_addr_t mc_portal_phys_addr,
> >> +				  uint32_t mc_portal_size,
> >> +				  uint32_t flags, struct fsl_mc_io **new_mc_io)
> >> +{
> >> +	struct fsl_mc_io *mc_io;
> >> +	void __iomem *mc_portal_virt_addr;
> >> +	struct resource *res;
> >> +
> >> +	mc_io = devm_kzalloc(dev, sizeof(*mc_io), GFP_KERNEL);
> >> +	if (mc_io == NULL)
> >> +		return -ENOMEM;
> >> +
> >> +	mc_io->dev = dev;
> >> +	mc_io->flags = flags;
> >> +	mc_io->portal_phys_addr = mc_portal_phys_addr;
> >> +	mc_io->portal_size = mc_portal_size;
> >> +	if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS)
> >> +		spin_lock_init(&mc_io->spinlock);
> >
> > I'm confused - this patseries doesn't register an interrupt handler,
> > so this can't be true (or it's premature if it will, in which case
> > it should be left out for now).
> >
> > However, if somehow users of this code register an IRQ handler for
> > the portal (I don't see any users to tell how they get the IRQ line
> > either?), then it's up to them to establish mutual exclusion rules
> > for access, among themselves.  Unless you think they will be calling
> > mc_send_command from h/w IRQ context, in which case I'd reconsider
> > that assumption because send_command looks like it'd take too long
> > to get an answer from the h/w - IRQ handlers should just ack the h/w
> > IRQ, and notify the scheduler that the driver has work to do (in s/w
> > IRQ context perhaps).
> >
> Although not included in this patch series, there are cases in
> subsequent patch series, in which mc_send_command() will need to be
> called from interrupt context.
> 
> For example, the dprc_get_irq_status() needs to be called from the DPRC
> ISR to determine the actual cause of the interrupt. Also, to clear
> the interrupt the dprc_clear_irq_status() needs to be called from the
> ISR. Both od these functions call mc_send_command():

yet mc_send_command itself has the capability to trigger a h/w IRQ,
no?  So how is that expected to work, given h/w IRQ handlers run
with IRQs turned off?

> >> +	else if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS)
> >> +		mutex_init(&mc_io->mutex);
> >
> > I'd assume SHARED_BY_THREADS to always be true in linux.
> >
> Not, if mc_send_command() is called from interrupt context, as
> explained above. However, since this patch series does not include
> any interrupt handlers, we can remove the
> FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS flag and the associated spinlock,
> from this patch series.
> 
> >> +	res = devm_request_mem_region(dev,
> >> +				      mc_portal_phys_addr,
> >> +				      mc_portal_size,
> >> +				      "mc_portal");
> >> +	if (res == NULL) {
> >> +		dev_err(dev,
> >> +			"devm_request_mem_region failed for MC portal %#llx\n",
> >> +			mc_portal_phys_addr);
> >> +		return -EBUSY;
> >> +	}
> >> +
> >> +	mc_portal_virt_addr = devm_ioremap_nocache(dev,
> >> +						   mc_portal_phys_addr,
> >> +						   mc_portal_size);
> >> +	if (mc_portal_virt_addr == NULL) {
> >> +		dev_err(dev,
> >> +			"devm_ioremap_nocache failed for MC portal %#llx\n",
> >> +			mc_portal_phys_addr);
> >> +		return -ENXIO;
> >> +	}
> >> +
> >> +	mc_io->portal_virt_addr = mc_portal_virt_addr;
> >> +	*new_mc_io = mc_io;
> >> +	return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(fsl_create_mc_io);
> >> +
> >> +void fsl_destroy_mc_io(struct fsl_mc_io *mc_io)
> >> +{
> >> +	if (WARN_ON(mc_io->portal_virt_addr == NULL))
> >> +		return;
> >
> > this is unnecessary - you'll get the stack trace anyway, and users
> > calling destroy on a not successfully created mc_io object should
> > not get the luxury of maybe being able to continue after the stack
> > trace, after possibly leaking memory.
> Ok, I'ĺl remove this WARN_ON in the v3 respin.

subsequent code will cause a stack trace when dereferencing
portal_virt_addr anyway, so the if statement isn't needed at all.

> >> +	mc_io->portal_virt_addr = NULL;
> >> +	devm_kfree(mc_io->dev, mc_io);
> >
> > like I said before, there's really no point in clearing something
> > out right before it's freed.
> >
> I disagree. This can help detect cases of double-freeing.

?  freeing NULL does nothing - it just returns - which doesn't help
detect anything.  What's more, the kernel has a memory debugging
infrastructure that detects double freeing of the same object.

> >> +int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
> >> +{
> >> +	enum mc_cmd_status status;
> >> +	int error;
> >> +	unsigned long irqsave_flags = 0;
> >> +	unsigned long jiffies_until_timeout =
> >> +	    jiffies + MC_CMD_COMPLETION_TIMEOUT_JIFFIES;
> >> +
> >> +	/*
> >> +	 * Acquire lock depending on mc_io flags:
> >> +	 */
> >> +	if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS) {
> >> +		if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS)
> >> +			spin_lock_irqsave(&mc_io->spinlock, irqsave_flags);
> >> +		else
> >> +			spin_lock(&mc_io->spinlock);
> >> +	} else if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS) {
> >> +		mutex_lock(&mc_io->mutex);
> >> +	}
> >
> > again, I think we need to drop the coming from h/w IRQ context here
> > (SHARED_BY_INT_HANDLERS); there's no IRQ handlers in this
> > patchseries, and calling this function from an IRQ handler would be
> > prohibitively wasteful, guessing by the udelay and timeout values
> > below.
> >
> > Can we just mutex_lock for now, and unconditionally (no
> > SHARED_BY_THREADS check), to protect from nesting?
> >
> I would still prefer to keep the SHARED_BY_THREADS flag, to give option 
> of not doing any locking, in cases where the portal used in 
> mc_send_command() is not shared among concurrent callers

how can you guarantee there won't be concurrent callers?  The linux
kernel is multithreaded.

> What do you mean by nesting in this case?

when a thread gets interrupted by another thread, and/or another
IRQ: this would cause an unrecoverable race condition.

> >> +	/*
> >> +	 * 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 udelay()
> >> +		 */
> >> +		udelay(MC_CMD_COMPLETION_POLLING_INTERVAL_USECS);
> >
> > this pauses any caller for 0.5ms on every successful command
> > write.  Can the next submission of the patchseries wait until
> > completion IRQs are indeed supported, since both that and the above
> > locking needs to be resolved?
> >
> No. Interrupt handlers will come in a later patch series as they are
> not needed for using the basic MC functionality.

meanwhile unnecessarily udelaying kernel threads for .5ms upsets
basic kernel functionality :)  Would using the kernel's
wait_for_completion API be a good compromise here?  See
include/linux/completion.h.

Kim

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

* Re: [PATCH 1/3 v2] drivers/bus: Added Freescale Management Complex APIs
  2014-09-25  3:40       ` Kim Phillips
@ 2014-09-25 15:44         ` German Rivera
  2014-09-25 16:16           ` Scott Wood
  0 siblings, 1 reply; 20+ messages in thread
From: German Rivera @ 2014-09-25 15:44 UTC (permalink / raw)
  To: Kim Phillips
  Cc: gregkh, arnd, linux-kernel, stuart.yoder, scottwood, agraf,
	linuxppc-release, Erez Nir-RM30794



On 09/24/2014 10:40 PM, Kim Phillips wrote:
> On Wed, 24 Sep 2014 21:23:59 -0500
> German Rivera <German.Rivera@freescale.com> wrote:
>
>> On 09/23/2014 07:49 PM, Kim Phillips wrote:
>>> On Fri, 19 Sep 2014 17:49:39 -0500
>>> "J. German Rivera" <German.Rivera@freescale.com> wrote:
>>>
>>>> + * Delay in microseconds between polling iterations while
>>>> + * waiting for MC command completion
>>>> + */
>>>> +#define MC_CMD_COMPLETION_POLLING_INTERVAL_USECS    500	/* 0.5 ms */
>>>> +
>>>> +int __must_check fsl_create_mc_io(struct device *dev,
>>>> +				  phys_addr_t mc_portal_phys_addr,
>>>> +				  uint32_t mc_portal_size,
>>>> +				  uint32_t flags, struct fsl_mc_io **new_mc_io)
>>>> +{
>>>> +	struct fsl_mc_io *mc_io;
>>>> +	void __iomem *mc_portal_virt_addr;
>>>> +	struct resource *res;
>>>> +
>>>> +	mc_io = devm_kzalloc(dev, sizeof(*mc_io), GFP_KERNEL);
>>>> +	if (mc_io == NULL)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	mc_io->dev = dev;
>>>> +	mc_io->flags = flags;
>>>> +	mc_io->portal_phys_addr = mc_portal_phys_addr;
>>>> +	mc_io->portal_size = mc_portal_size;
>>>> +	if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS)
>>>> +		spin_lock_init(&mc_io->spinlock);
>>>
>>> I'm confused - this patseries doesn't register an interrupt handler,
>>> so this can't be true (or it's premature if it will, in which case
>>> it should be left out for now).
>>>
>>> However, if somehow users of this code register an IRQ handler for
>>> the portal (I don't see any users to tell how they get the IRQ line
>>> either?), then it's up to them to establish mutual exclusion rules
>>> for access, among themselves.  Unless you think they will be calling
>>> mc_send_command from h/w IRQ context, in which case I'd reconsider
>>> that assumption because send_command looks like it'd take too long
>>> to get an answer from the h/w - IRQ handlers should just ack the h/w
>>> IRQ, and notify the scheduler that the driver has work to do (in s/w
>>> IRQ context perhaps).
>>>
>> Although not included in this patch series, there are cases in
>> subsequent patch series, in which mc_send_command() will need to be
>> called from interrupt context.
>>
>> For example, the dprc_get_irq_status() needs to be called from the DPRC
>> ISR to determine the actual cause of the interrupt. Also, to clear
>> the interrupt the dprc_clear_irq_status() needs to be called from the
>> ISR. Both od these functions call mc_send_command():
>
> yet mc_send_command itself has the capability to trigger a h/w IRQ,
> no?  So how is that expected to work, given h/w IRQ handlers run
> with IRQs turned off?
>
"MC command completion" interrupts has not been implemented in the MC 
firmware, so sending a command to the MC does not currently trigger an 
interrupt by itself. I imagine that when "MC completion interrupts" get 
implemented, a flag will be available in the MC command header to 
indicate whether or not a "completion interrupt" is wanted. Different
MC commands have different latencies. So, enabling generation
of completion interrupts, is something that should be done only
for long-latency MC commands. For low-latency MC commands commands,
(including those that can be called in interrupt context) such as 
inspecting interrupt status or clearing interrupts, polling for 
completion would be a more efficient choice.

>>>> +	else if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS)
>>>> +		mutex_init(&mc_io->mutex);
>>>
>>> I'd assume SHARED_BY_THREADS to always be true in linux.
>>>
>> Not, if mc_send_command() is called from interrupt context, as
>> explained above. However, since this patch series does not include
>> any interrupt handlers, we can remove the
>> FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS flag and the associated spinlock,
>> from this patch series.
>>
>>>> +	res = devm_request_mem_region(dev,
>>>> +				      mc_portal_phys_addr,
>>>> +				      mc_portal_size,
>>>> +				      "mc_portal");
>>>> +	if (res == NULL) {
>>>> +		dev_err(dev,
>>>> +			"devm_request_mem_region failed for MC portal %#llx\n",
>>>> +			mc_portal_phys_addr);
>>>> +		return -EBUSY;
>>>> +	}
>>>> +
>>>> +	mc_portal_virt_addr = devm_ioremap_nocache(dev,
>>>> +						   mc_portal_phys_addr,
>>>> +						   mc_portal_size);
>>>> +	if (mc_portal_virt_addr == NULL) {
>>>> +		dev_err(dev,
>>>> +			"devm_ioremap_nocache failed for MC portal %#llx\n",
>>>> +			mc_portal_phys_addr);
>>>> +		return -ENXIO;
>>>> +	}
>>>> +
>>>> +	mc_io->portal_virt_addr = mc_portal_virt_addr;
>>>> +	*new_mc_io = mc_io;
>>>> +	return 0;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(fsl_create_mc_io);
>>>> +
>>>> +void fsl_destroy_mc_io(struct fsl_mc_io *mc_io)
>>>> +{
>>>> +	if (WARN_ON(mc_io->portal_virt_addr == NULL))
>>>> +		return;
>>>
>>> this is unnecessary - you'll get the stack trace anyway, and users
>>> calling destroy on a not successfully created mc_io object should
>>> not get the luxury of maybe being able to continue after the stack
>>> trace, after possibly leaking memory.
>> Ok, I'ĺl remove this WARN_ON in the v3 respin.
>
> subsequent code will cause a stack trace when dereferencing
> portal_virt_addr anyway, so the if statement isn't needed at all.
>
I meant to remove both the 'if' and the WARN_ON, as obviously keeping 
the 'if' without the WARN_ON is bad as it would silently hide a bug.

>>>> +	mc_io->portal_virt_addr = NULL;
>>>> +	devm_kfree(mc_io->dev, mc_io);
>>>
>>> like I said before, there's really no point in clearing something
>>> out right before it's freed.
>>>
>> I disagree. This can help detect cases of double-freeing.
>
> ?  freeing NULL does nothing - it just returns - which doesn't help
> detect anything.  What's more, the kernel has a memory debugging
> infrastructure that detects double freeing of the same object.
I know that, but silently doing nothing when freeing NULL is a bad 
practice in general, because it hides a bug. Is the memory debugging 
infrastructure enabled by default? If so, then I would agree with you. 
If not, we would need to be able to reproduce the bug while having
memory debugging enabled. This assumes that the bug is easy to
reproduce. What if it is not easy to reproduce?
Having "first-failure data capture" checks is useful for helping
diagnose bugs that are not easy to reproduce.

In this case, if due to some bug, fsl_destroy_mc_io() is
called twice for the same mc_io object, the combination of doing
"mc_io->portal_virt_addr = NULL" and the original "if (WARN_ON",
that you want removed, would have helped catch the bug on
"first failure".
Even removing the "if (WARN_ON)" but keeping the
"mc_io->portal_virt_addr = NULL" would still help catch the bug
on "first failure", assuming that the system crashes when calling
"devm_iounmap(mc_io->dev, NULL);" or at least prints a stack trace.

> >>>
>>>> + * Delay in microseconds between polling iterations while
>>>> + * waiting for MC command completion
>>>> + */
>>>> +#define MC_CMD_COMPLETION_POLLING_INTERVAL_USECS    500	/* 0.5 ms */
>>>> +
>>>> +int __must_check fsl_create_mc_io(struct device *dev,
>>>> +				  phys_addr_t mc_portal_phys_addr,
>>>> +				  uint32_t mc_portal_size,
>>>> +				  uint32_t flags, struct fsl_mc_io **new_mc_io)
>>>> +{
>>>> +	struct fsl_mc_io *mc_io;
>>>> +	void __iomem *mc_portal_virt_addr;
>>>> +	struct resource *res;
>>>> +
>>>> +	mc_io = devm_kzalloc(dev, sizeof(*mc_io), GFP_KERNEL);
>>>> +	if (mc_io == NULL)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	mc_io->dev = dev;
>>>> +	mc_io->flags = flags;
>>>> +	mc_io->portal_phys_addr = mc_portal_phys_addr;
>>>> +	mc_io->portal_size = mc_portal_size;
>>>> +	if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS)
>>>> +		spin_lock_init(&mc_io->spinlock);
>>>
>>> I'm confused - this patseries doesn't register an interrupt handler,
>>> so this can't be true (or it's premature if it will, in which case
>>> it should be left out for now).
>>>
>>> However, if somehow users of this code register an IRQ handler for
>>> the portal (I don't see any users to tell how they get the IRQ line
>>> either?), then it's up to them to establish mutual exclusion rules
>>> for access, among themselves.  Unless you think they will be calling
>>> mc_send_command from h/w IRQ context, in which case I'd reconsider
>>> that assumption because send_command looks like it'd take too long
>>> to get an answer from the h/w - IRQ handlers should just ack the h/w
>>> IRQ, and notify the scheduler that the driver has work to do (in s/w
>>> IRQ context perhaps).
>>>
>> Although not included in this patch series, there are cases in
>> subsequent patch series, in which mc_send_command() will need to be
>> called from interrupt context.
>>
>> For example, the dprc_get_irq_status() needs to be called from the DPRC
>> ISR to determine the actual cause of the interrupt. Also, to clear
>> the interrupt the dprc_clear_irq_status() needs to be called from the
>> ISR. Both od these functions call mc_send_command():
>
>>>> +int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
>>>> +{
>>>> +	enum mc_cmd_status status;
>>>> +	int error;
>>>> +	unsigned long irqsave_flags = 0;
>>>> +	unsigned long jiffies_until_timeout =
>>>> +	    jiffies + MC_CMD_COMPLETION_TIMEOUT_JIFFIES;
>>>> +
>>>> +	/*
>>>> +	 * Acquire lock depending on mc_io flags:
>>>> +	 */
>>>> +	if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS) {
>>>> +		if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS)
>>>> +			spin_lock_irqsave(&mc_io->spinlock, irqsave_flags);
>>>> +		else
>>>> +			spin_lock(&mc_io->spinlock);
>>>> +	} else if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS) {
>>>> +		mutex_lock(&mc_io->mutex);
>>>> +	}
>>>
>>> again, I think we need to drop the coming from h/w IRQ context here
>>> (SHARED_BY_INT_HANDLERS); there's no IRQ handlers in this
>>> patchseries, and calling this function from an IRQ handler would be
>>> prohibitively wasteful, guessing by the udelay and timeout values
>>> below.
>>>
>>> Can we just mutex_lock for now, and unconditionally (no
>>> SHARED_BY_THREADS check), to protect from nesting?
>>>
>> I would still prefer to keep the SHARED_BY_THREADS flag, to give option
>> of not doing any locking, in cases where the portal used in
>> mc_send_command() is not shared among concurrent callers
>
> how can you guarantee there won't be concurrent callers?  The linux
> kernel is multithreaded.
>
The owner of the portal should know if his/her code can be invoked using 
the same portal, from multiple threads or not.

>> What do you mean by nesting in this case?
>
> when a thread gets interrupted by another thread, and/or another
> IRQ: this would cause an unrecoverable race condition.
>
>>>> +	/*
>>>> +	 * 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 udelay()
>>>> +		 */
>>>> +		udelay(MC_CMD_COMPLETION_POLLING_INTERVAL_USECS);
>>>
>>> this pauses any caller for 0.5ms on every successful command
>>> write.  Can the next submission of the patchseries wait until
>>> completion IRQs are indeed supported, since both that and the above
>>> locking needs to be resolved?
>>>
>> No. Interrupt handlers will come in a later patch series as they are
>> not needed for using the basic MC functionality.
>
> meanwhile unnecessarily udelaying kernel threads for .5ms upsets
> basic kernel functionality :)  Would using the kernel's
> wait_for_completion API be a good compromise here?  See
> include/linux/completion.h.
>
The intent of doing the udelay() in the middle of the polling loop was 
to throttle down the frequency of I/Os done while polling for the
completion of the command. Can you elaborate on why ".5ms udelay upsets
basic kernel functionality"?

Would it be better to just use "plain delay loop", instead of the udelay 
call, such as the following?

for (i = 0; i < 1000; i++)
    ;

I can see that in the cases where we use "completion interrupts", the 
ISR can signal the completion, and the polling loop can be replaced by 
waiting on the completion. However, I don't see how using a completion 
can help make a polling loop more efficient, if you don't have a 
"completion interrupt" to signal the completion.

Thanks,

German
> Kim
>

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

* Re: [PATCH 1/3 v2] drivers/bus: Added Freescale Management Complex APIs
  2014-09-25 15:44         ` German Rivera
@ 2014-09-25 16:16           ` Scott Wood
  2014-09-25 16:44             ` German Rivera
  0 siblings, 1 reply; 20+ messages in thread
From: Scott Wood @ 2014-09-25 16:16 UTC (permalink / raw)
  To: German Rivera
  Cc: Kim Phillips, gregkh, arnd, linux-kernel, stuart.yoder, agraf,
	linuxppc-release, Erez Nir-RM30794

On Thu, 2014-09-25 at 10:44 -0500, German Rivera wrote:
> 
> On 09/24/2014 10:40 PM, Kim Phillips wrote:
> > On Wed, 24 Sep 2014 21:23:59 -0500
> > German Rivera <German.Rivera@freescale.com> wrote:
> >
> >> On 09/23/2014 07:49 PM, Kim Phillips wrote:
> >>> On Fri, 19 Sep 2014 17:49:39 -0500
> >>> "J. German Rivera" <German.Rivera@freescale.com> wrote:
> >>>> +	mc_io->portal_virt_addr = NULL;
> >>>> +	devm_kfree(mc_io->dev, mc_io);
> >>>
> >>> like I said before, there's really no point in clearing something
> >>> out right before it's freed.
> >>>
> >> I disagree. This can help detect cases of double-freeing.
> >
> > ?  freeing NULL does nothing - it just returns - which doesn't help
> > detect anything.  What's more, the kernel has a memory debugging
> > infrastructure that detects double freeing of the same object.
> I know that, but silently doing nothing when freeing NULL is a bad 
> practice in general, because it hides a bug.

It doesn't hide a bug "in general".  I'm not sure what the relevance is
here, though -- this seems to be about reusing portal_virt_addr as a
debug flag rather than anything to do with actually calling free() on
portal_virt_addr.

> Is the memory debugging infrastructure enabled by default? If so, then
> I would agree with you. If not, we would need to be able to reproduce
> the bug while having memory debugging enabled. This assumes that the
> bug is easy to reproduce. What if it is not easy to reproduce? Having
> "first-failure data capture" checks is useful for helping diagnose bugs
> that are not easy to reproduce.
> 
> In this case, if due to some bug, fsl_destroy_mc_io() is
> called twice for the same mc_io object, the combination of doing
> "mc_io->portal_virt_addr = NULL" and the original "if (WARN_ON",
> that you want removed, would have helped catch the bug on
> "first failure".
> Even removing the "if (WARN_ON)" but keeping the
> "mc_io->portal_virt_addr = NULL" would still help catch the bug
> on "first failure", assuming that the system crashes when calling
> "devm_iounmap(mc_io->dev, NULL);" or at least prints a stack trace.

iounmap of NULL will not crash or print a stack trace.  It is a no-op.

> >>>> +int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
> >>>> +{
> >>>> +	enum mc_cmd_status status;
> >>>> +	int error;
> >>>> +	unsigned long irqsave_flags = 0;
> >>>> +	unsigned long jiffies_until_timeout =
> >>>> +	    jiffies + MC_CMD_COMPLETION_TIMEOUT_JIFFIES;
> >>>> +
> >>>> +	/*
> >>>> +	 * Acquire lock depending on mc_io flags:
> >>>> +	 */
> >>>> +	if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS) {
> >>>> +		if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS)
> >>>> +			spin_lock_irqsave(&mc_io->spinlock, irqsave_flags);
> >>>> +		else
> >>>> +			spin_lock(&mc_io->spinlock);
> >>>> +	} else if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS) {
> >>>> +		mutex_lock(&mc_io->mutex);
> >>>> +	}
> >>>
> >>> again, I think we need to drop the coming from h/w IRQ context here
> >>> (SHARED_BY_INT_HANDLERS); there's no IRQ handlers in this
> >>> patchseries, and calling this function from an IRQ handler would be
> >>> prohibitively wasteful, guessing by the udelay and timeout values
> >>> below.
> >>>
> >>> Can we just mutex_lock for now, and unconditionally (no
> >>> SHARED_BY_THREADS check), to protect from nesting?
> >>>
> >> I would still prefer to keep the SHARED_BY_THREADS flag, to give option
> >> of not doing any locking, in cases where the portal used in
> >> mc_send_command() is not shared among concurrent callers
> >
> > how can you guarantee there won't be concurrent callers?  The linux
> > kernel is multithreaded.
> >
> The owner of the portal should know if his/her code can be invoked using 
> the same portal, from multiple threads or not.

Or more generally, whether the caller is responsible for
synchronization.

Would it make sense to simplify by saying the caller is always
responsible for synchronization?

Then again, the management complex is not supposed to be on the
performance critical path, so why not simplify by just always do the
locking here?

> >>>> +	/*
> >>>> +	 * 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 udelay()
> >>>> +		 */
> >>>> +		udelay(MC_CMD_COMPLETION_POLLING_INTERVAL_USECS);
> >>>
> >>> this pauses any caller for 0.5ms on every successful command
> >>> write.  Can the next submission of the patchseries wait until
> >>> completion IRQs are indeed supported, since both that and the above
> >>> locking needs to be resolved?
> >>>
> >> No. Interrupt handlers will come in a later patch series as they are
> >> not needed for using the basic MC functionality.
> >
> > meanwhile unnecessarily udelaying kernel threads for .5ms upsets
> > basic kernel functionality :)  Would using the kernel's
> > wait_for_completion API be a good compromise here?  See
> > include/linux/completion.h.
> >
> The intent of doing the udelay() in the middle of the polling loop was 
> to throttle down the frequency of I/Os done while polling for the
> completion of the command. Can you elaborate on why ".5ms udelay upsets
> basic kernel functionality"?

It introduces latency, especially since it's possible for it to happen
with interrupts disabled.  And you're actually potentially blocking for
more than that, since 500us is just one iteration of the loop.

The jiffies test for exiting the loop is 500ms, which is *way* too long
to spend in a critical section.

> Would it be better to just use "plain delay loop", instead of the udelay 
> call, such as the following?
> 
> for (i = 0; i < 1000; i++)
>     ;

No, never do that.  You have no idea how long it will actually take.
GCC might even optimize it out entirely.

> I can see that in the cases where we use "completion interrupts", the 
> ISR can signal the completion, and the polling loop can be replaced by 
> waiting on the completion. However, I don't see how using a completion 
> can help make a polling loop more efficient, if you don't have a 
> "completion interrupt" to signal the completion.

It's not about making it more efficient.  It's about not causing
problems for other things going on in the system.

-Scott



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

* Re: [PATCH 1/3 v2] drivers/bus: Added Freescale Management Complex APIs
  2014-09-25 16:16           ` Scott Wood
@ 2014-09-25 16:44             ` German Rivera
  2014-09-25 20:05               ` Scott Wood
  0 siblings, 1 reply; 20+ messages in thread
From: German Rivera @ 2014-09-25 16:44 UTC (permalink / raw)
  To: Scott Wood
  Cc: Kim Phillips, gregkh, arnd, linux-kernel, stuart.yoder, agraf,
	linuxppc-release, Erez Nir-RM30794


On 09/25/2014 11:16 AM, Scott Wood wrote:
> On Thu, 2014-09-25 at 10:44 -0500, German Rivera wrote:
>>
>> On 09/24/2014 10:40 PM, Kim Phillips wrote:
>>> On Wed, 24 Sep 2014 21:23:59 -0500
>>> German Rivera <German.Rivera@freescale.com> wrote:
>>>
>>>> On 09/23/2014 07:49 PM, Kim Phillips wrote:
>>>>> On Fri, 19 Sep 2014 17:49:39 -0500
>>>>> "J. German Rivera" <German.Rivera@freescale.com> wrote:
>>>>>> +	mc_io->portal_virt_addr = NULL;
>>>>>> +	devm_kfree(mc_io->dev, mc_io);
>>>>>
>>>>> like I said before, there's really no point in clearing something
>>>>> out right before it's freed.
>>>>>
>>>> I disagree. This can help detect cases of double-freeing.
>>>
>>> ?  freeing NULL does nothing - it just returns - which doesn't help
>>> detect anything.  What's more, the kernel has a memory debugging
>>> infrastructure that detects double freeing of the same object.
>> I know that, but silently doing nothing when freeing NULL is a bad
>> practice in general, because it hides a bug.
>
> It doesn't hide a bug "in general".  I'm not sure what the relevance is
> here, though -- this seems to be about reusing portal_virt_addr as a
> debug flag rather than anything to do with actually calling free() on
> portal_virt_addr.
>
>> Is the memory debugging infrastructure enabled by default? If so, then
>> I would agree with you. If not, we would need to be able to reproduce
>> the bug while having memory debugging enabled. This assumes that the
>> bug is easy to reproduce. What if it is not easy to reproduce? Having
>> "first-failure data capture" checks is useful for helping diagnose bugs
>> that are not easy to reproduce.
>>
>> In this case, if due to some bug, fsl_destroy_mc_io() is
>> called twice for the same mc_io object, the combination of doing
>> "mc_io->portal_virt_addr = NULL" and the original "if (WARN_ON",
>> that you want removed, would have helped catch the bug on
>> "first failure".
>> Even removing the "if (WARN_ON)" but keeping the
>> "mc_io->portal_virt_addr = NULL" would still help catch the bug
>> on "first failure", assuming that the system crashes when calling
>> "devm_iounmap(mc_io->dev, NULL);" or at least prints a stack trace.
>
> iounmap of NULL will not crash or print a stack trace.  It is a no-op.
>
Ok, so if iounmap silently does nothing for NULL, and free silently does
nothing for NULL, that means we may not be be able to easily catch 
"destroy double-call" bugs on first-failure, in this case.
If that is not an issue, then I'll remove the "mc_io->portal_virt_addr = 
NULL" as well.

>>>>>> +int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
>>>>>> +{
>>>>>> +	enum mc_cmd_status status;
>>>>>> +	int error;
>>>>>> +	unsigned long irqsave_flags = 0;
>>>>>> +	unsigned long jiffies_until_timeout =
>>>>>> +	    jiffies + MC_CMD_COMPLETION_TIMEOUT_JIFFIES;
>>>>>> +
>>>>>> +	/*
>>>>>> +	 * Acquire lock depending on mc_io flags:
>>>>>> +	 */
>>>>>> +	if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_INT_HANDLERS) {
>>>>>> +		if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS)
>>>>>> +			spin_lock_irqsave(&mc_io->spinlock, irqsave_flags);
>>>>>> +		else
>>>>>> +			spin_lock(&mc_io->spinlock);
>>>>>> +	} else if (mc_io->flags & FSL_MC_PORTAL_SHARED_BY_THREADS) {
>>>>>> +		mutex_lock(&mc_io->mutex);
>>>>>> +	}
>>>>>
>>>>> again, I think we need to drop the coming from h/w IRQ context here
>>>>> (SHARED_BY_INT_HANDLERS); there's no IRQ handlers in this
>>>>> patchseries, and calling this function from an IRQ handler would be
>>>>> prohibitively wasteful, guessing by the udelay and timeout values
>>>>> below.
>>>>>
>>>>> Can we just mutex_lock for now, and unconditionally (no
>>>>> SHARED_BY_THREADS check), to protect from nesting?
>>>>>
>>>> I would still prefer to keep the SHARED_BY_THREADS flag, to give option
>>>> of not doing any locking, in cases where the portal used in
>>>> mc_send_command() is not shared among concurrent callers
>>>
>>> how can you guarantee there won't be concurrent callers?  The linux
>>> kernel is multithreaded.
>>>
>> The owner of the portal should know if his/her code can be invoked using
>> the same portal, from multiple threads or not.
>
> Or more generally, whether the caller is responsible for
> synchronization.
>
> Would it make sense to simplify by saying the caller is always
> responsible for synchronization?
>
Certainly this will simplify mc_send_command() but it will put the 
additional burden on all the callers of MC commands. If there is no 
objection, we can remove locking entirely from mc_send_command() and
the the callers of MC commands deal with that.

> Then again, the management complex is not supposed to be on the
> performance critical path, so why not simplify by just always do the
> locking here?
>
But about the few MC commands that need to run on interrupt context
(such as to inspect or clear MC interrupts)?

>>>>>> +	/*
>>>>>> +	 * 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 udelay()
>>>>>> +		 */
>>>>>> +		udelay(MC_CMD_COMPLETION_POLLING_INTERVAL_USECS);
>>>>>
>>>>> this pauses any caller for 0.5ms on every successful command
>>>>> write.  Can the next submission of the patchseries wait until
>>>>> completion IRQs are indeed supported, since both that and the above
>>>>> locking needs to be resolved?
>>>>>
>>>> No. Interrupt handlers will come in a later patch series as they are
>>>> not needed for using the basic MC functionality.
>>>
>>> meanwhile unnecessarily udelaying kernel threads for .5ms upsets
>>> basic kernel functionality :)  Would using the kernel's
>>> wait_for_completion API be a good compromise here?  See
>>> include/linux/completion.h.
>>>
>> The intent of doing the udelay() in the middle of the polling loop was
>> to throttle down the frequency of I/Os done while polling for the
>> completion of the command. Can you elaborate on why ".5ms udelay upsets
>> basic kernel functionality"?
>
> It introduces latency, especially since it's possible for it to happen
> with interrupts disabled.  And you're actually potentially blocking for
> more than that, since 500us is just one iteration of the loop.
>
> The jiffies test for exiting the loop is 500ms, which is *way* too long
> to spend in a critical section.
>
But that would be a worst case, since that is a timeout check.
What timeout value do you think would be more appropriate in this case?

>> Would it be better to just use "plain delay loop", instead of the udelay
>> call, such as the following?
>>
>> for (i = 0; i < 1000; i++)
>>      ;
>
> No, never do that.  You have no idea how long it will actually take.
> GCC might even optimize it out entirely.
>
Ok, so udelay is not good here, a plain vanilla delay loop is not good 
either. Are there other alternatives or we just don't worry about
throttling down the frequency of I/Os done in the polling loop?
(Given the fact that MC commands are not expected to be executed that 
frequently, to frequently cause a lot of I/O traffic with this polling loop)

>> I can see that in the cases where we use "completion interrupts", the
>> ISR can signal the completion, and the polling loop can be replaced by
>> waiting on the completion. However, I don't see how using a completion
>> can help make a polling loop more efficient, if you don't have a
>> "completion interrupt" to signal the completion.
>
> It's not about making it more efficient.  It's about not causing
> problems for other things going on in the system.
>
But still I don't see how a completion can help here, unless
you can signal the completion from an ISR.

-German


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

* Re: [PATCH 1/3 v2] drivers/bus: Added Freescale Management Complex APIs
  2014-09-25 16:44             ` German Rivera
@ 2014-09-25 20:05               ` Scott Wood
  0 siblings, 0 replies; 20+ messages in thread
From: Scott Wood @ 2014-09-25 20:05 UTC (permalink / raw)
  To: German Rivera
  Cc: Kim Phillips, gregkh, arnd, linux-kernel, stuart.yoder, agraf,
	linuxppc-release, Erez Nir-RM30794

On Thu, 2014-09-25 at 11:44 -0500, German Rivera wrote:
> On 09/25/2014 11:16 AM, Scott Wood wrote:
> > Then again, the management complex is not supposed to be on the
> > performance critical path, so why not simplify by just always do the
> > locking here?
> >
> But about the few MC commands that need to run on interrupt context
> (such as to inspect or clear MC interrupts)?

I think that should be disallowed.  Unless you can guarantee that the MC
firmware will respond in a very short time, have those interrupts move
their work into a workqueue, with the MC interrupt masked until
completion.  We do similar things for interrupts from i2c devices, etc.

> >> The intent of doing the udelay() in the middle of the polling loop was
> >> to throttle down the frequency of I/Os done while polling for the
> >> completion of the command. Can you elaborate on why ".5ms udelay upsets
> >> basic kernel functionality"?
> >
> > It introduces latency, especially since it's possible for it to happen
> > with interrupts disabled.  And you're actually potentially blocking for
> > more than that, since 500us is just one iteration of the loop.
> >
> > The jiffies test for exiting the loop is 500ms, which is *way* too long
> > to spend in a critical section.
> >
> But that would be a worst case, since that is a timeout check.
> What timeout value do you think would be more appropriate in this case?

Worst case latency matters.  Some workloads would be bothered by an
occasional 500us.  Even normal interactive use would notice 500ms.

> >> Would it be better to just use "plain delay loop", instead of the udelay
> >> call, such as the following?
> >>
> >> for (i = 0; i < 1000; i++)
> >>      ;
> >
> > No, never do that.  You have no idea how long it will actually take.
> > GCC might even optimize it out entirely.
> >
> Ok, so udelay is not good here, a plain vanilla delay loop is not good 
> either. Are there other alternatives or we just don't worry about
> throttling down the frequency of I/Os done in the polling loop?

udelay is OK for this purpose, but not for so long, and find some way to
make the kernel preemptible between loop iterations.

> (Given the fact that MC commands are not expected to be executed that 
> frequently, to frequently cause a lot of I/O traffic with this polling loop)
> 
> >> I can see that in the cases where we use "completion interrupts", the
> >> ISR can signal the completion, and the polling loop can be replaced by
> >> waiting on the completion. However, I don't see how using a completion
> >> can help make a polling loop more efficient, if you don't have a
> >> "completion interrupt" to signal the completion.
> >
> > It's not about making it more efficient.  It's about not causing
> > problems for other things going on in the system.
> >
> But still I don't see how a completion can help here, unless
> you can signal the completion from an ISR.

Right, but you can still sleep on a timer instead of busy waiting if you
need such a long timeout.

-Scott



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

* Re: [PATCH 3/3 v2] drivers/bus: Device driver for FSL-MC DPRC devices
  2014-09-19 22:49 ` [PATCH 3/3 v2] drivers/bus: Device driver for FSL-MC DPRC devices J. German Rivera
@ 2014-10-01  2:19   ` Timur Tabi
  2014-10-01  2:27     ` Scott Wood
  2014-10-02 16:36     ` German Rivera
  0 siblings, 2 replies; 20+ messages in thread
From: Timur Tabi @ 2014-10-01  2:19 UTC (permalink / raw)
  To: J. German Rivera
  Cc: Greg Kroah-Hartman, Arnd Bergmann, lkml, stuart.yoder,
	Kim Phillips, Scott Wood, Alexander Graf, linuxppc-release

On Fri, Sep 19, 2014 at 5:49 PM, J. German Rivera
<German.Rivera@freescale.com> wrote:

> +static int __fsl_mc_device_remove(struct device *dev, void *data)
> +{
> +       WARN_ON(dev == NULL);
> +       WARN_ON(data != NULL);

I see a lot of direct comparisons with NULL and 0.  You don't need to
be so explicit:

WARN_ON(!dev);
WARN_ON(!data);

"!= 0" and "!= NULL" is almost always unnecessary, and you can delete them.


> +       fsl_mc_device_remove(to_fsl_mc_device(dev));
> +       return 0;
> +}
> +
> +/**
> + * dprc_remove_devices - Removes devices for objects removed from a DPRC
> + *
> + * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
> + * @obj_desc_array: array of object descriptors for child objects currently
> + * present in the DPRC in the MC.
> + * @num_child_objects_in_mc: number of entries in obj_desc_array
> + *
> + * Synchronizes the state of the Linux bus driver with the actual state of
> + * the MC by removing devices that represent MC objects that have
> + * been dynamically removed in the physical DPRC.
> + */
> +static void dprc_remove_devices(struct fsl_mc_device *mc_bus_dev,
> +                               struct dprc_obj_desc *obj_desc_array,
> +                               int num_child_objects_in_mc)
> +{
> +       if (num_child_objects_in_mc != 0) {

Like here.  Just do "if (num_child_objects_in_mc) {"

> +               /*
> +                * Remove child objects that are in the DPRC in Linux,
> +                * but not in the MC:
> +                */
> +               struct dprc_child_objs objs;
> +
> +               objs.child_count = num_child_objects_in_mc;
> +               objs.child_array = obj_desc_array;
> +               device_for_each_child(&mc_bus_dev->dev, &objs,
> +                                     __fsl_mc_device_remove_if_not_in_mc);
> +       } else {
> +               /*
> +                * There are no child objects for this DPRC in the MC.
> +                * So, remove all the child devices from Linux:
> +                */
> +               device_for_each_child(&mc_bus_dev->dev, NULL,
> +                                     __fsl_mc_device_remove);
> +       }
> +}
> +
> +static int __fsl_mc_device_match(struct device *dev, void *data)
> +{
> +       struct dprc_obj_desc *obj_desc = data;
> +       struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> +
> +       return FSL_MC_DEVICE_MATCH(mc_dev, obj_desc);
> +}
> +
> +static struct fsl_mc_device *fsl_mc_device_lookup(struct dprc_obj_desc
> +                                                               *obj_desc,
> +                                                 struct fsl_mc_device
> +                                                               *mc_bus_dev)
> +{
> +       struct device *dev;
> +
> +       dev = device_find_child(&mc_bus_dev->dev, obj_desc,
> +                               __fsl_mc_device_match);
> +
> +       return (dev != NULL) ? to_fsl_mc_device(dev) : NULL;

return dev ? to_fsl_mc_device(dev) : NULL;

> +}
> +
> +/**
> + * 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
> + * @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
> + *
> + * Synchronizes the state of the Linux bus driver with the actual
> + * state of the MC by adding objects that have been newly discovered
> + * in the physical DPRC.
> + */
> +static void dprc_add_new_devices(struct fsl_mc_device *mc_bus_dev,
> +                                struct dprc_obj_desc *obj_desc_array,
> +                                int num_child_objects_in_mc)
> +{
> +       int error;
> +       int i;
> +
> +       for (i = 0; i < num_child_objects_in_mc; i++) {
> +               struct dprc_region_desc region_desc;
> +               struct fsl_mc_device *child_dev;
> +               struct fsl_mc_io *mc_io = NULL;
> +               struct dprc_obj_desc *obj_desc = &obj_desc_array[i];
> +
> +               if (strlen(obj_desc->type) == 0)
> +                       continue;
> +               /*
> +                * Check if device is already known to Linux:
> +                */
> +               child_dev = fsl_mc_device_lookup(obj_desc, mc_bus_dev);
> +               if (child_dev != NULL)
> +                       continue;
> +
> +               if (strcmp(obj_desc->type, "dprc") == 0) {

As for the !=0 thing, I make an exception for strcmp(), however, since
its meaning is not intuitive.

> +                       /*
> +                        * Get the MC portal physical address for the device
> +                        * (specified in the device's first MMIO region):
> +                        */
> +                       if (WARN_ON(obj_desc->region_count == 0))
> +                               continue;
> +
> +                       error = dprc_get_obj_region(mc_bus_dev->mc_io,
> +                                                   mc_bus_dev->mc_handle,
> +                                                   obj_desc->type,
> +                                                   obj_desc->id,
> +                                                   0, &region_desc);
> +                       if (error < 0) {
> +                               dev_err(&mc_bus_dev->dev,
> +                                       "dprc_get_obj_region() failed: %d\n",
> +                                       error);
> +                               continue;
> +                       }
> +
> +                       if (region_desc.size !=
> +                                       mc_bus_dev->mc_io->portal_size) {
> +                               error = -EINVAL;
> +                               dev_err(&mc_bus_dev->dev,
> +                                       "Invalid MC portal size: %u\n",
> +                                       region_desc.size);
> +                               continue;
> +                       }
> +
> +                       error = fsl_create_mc_io(&mc_bus_dev->dev,
> +                                                region_desc.base_paddr,
> +                                                region_desc.size, 0, &mc_io);
> +                       if (error < 0)
> +                               continue;
> +               }
> +
> +               error = fsl_mc_device_add(obj_desc, mc_io, &mc_bus_dev->dev,
> +                                         &child_dev);
> +               if (error < 0) {
> +                       if (mc_io != NULL)
> +                               fsl_destroy_mc_io(mc_io);
> +
> +                       continue;
> +               }
> +       }
> +}
> +
> +/**
> + * dprc_scan_objects - Discover objects in a DPRC
> + *
> + * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
> + *
> + * Detects objects added and removed from a DPRC and synchronizes the
> + * state of the Linux bus driver, MC by adding and removing
> + * devices accordingly.
> + */
> +int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev)
> +{
> +       int num_child_objects;
> +       int dprc_get_obj_failures;
> +       int error = -EINVAL;
> +       struct dprc_obj_desc *child_obj_desc_array = NULL;
> +
> +       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);
> +               goto out;
> +       }
> +
> +       if (num_child_objects != 0) {
> +               int i;
> +
> +               child_obj_desc_array =
> +                   devm_kmalloc_array(&mc_bus_dev->dev, num_child_objects,
> +                                      sizeof(*child_obj_desc_array),
> +                                      GFP_KERNEL);
> +               if (child_obj_desc_array == NULL) {
> +                       error = -ENOMEM;
> +                       dev_err(&mc_bus_dev->dev,
> +                               "No memory to allocate obj_desc array\n");
> +                       goto out;
> +               }
> +
> +               /*
> +                * Discover objects currently present in the physical DPRC:
> +                */
> +               dprc_get_obj_failures = 0;
> +               for (i = 0; i < num_child_objects; i++) {
> +                       struct dprc_obj_desc *obj_desc =
> +                           &child_obj_desc_array[i];
> +
> +                       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);
> +                               /*
> +                                * Mark the obj entry as "invalid", by using the
> +                                * empty string as obj type:
> +                                */
> +                               obj_desc->type[0] = '\0';
> +                               obj_desc->id = error;
> +                               dprc_get_obj_failures++;
> +                               continue;
> +                       }
> +
> +                       dev_info(&mc_bus_dev->dev,
> +                                "Discovered object: type %s, id %d\n",
> +                                obj_desc->type, obj_desc->id);
> +               }
> +
> +               if (dprc_get_obj_failures != 0) {
> +                       dev_err(&mc_bus_dev->dev,
> +                               "%d out of %d devices could not be retrieved\n",
> +                               dprc_get_obj_failures, num_child_objects);
> +               }
> +       }
> +
> +       dprc_remove_devices(mc_bus_dev, child_obj_desc_array,
> +                           num_child_objects);
> +
> +       dprc_add_new_devices(mc_bus_dev, child_obj_desc_array,
> +                            num_child_objects);
> +
> +       error = 0;
> +out:
> +       if (child_obj_desc_array != NULL)
> +               devm_kfree(&mc_bus_dev->dev, child_obj_desc_array);

The whole point behind the devm_ functions is that you shoudn't need
to manually release the resources.

> +
> +       return error;
> +}
> +EXPORT_SYMBOL_GPL(dprc_scan_objects);
> +
> +/**
> + * 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
> + *
> + * Scans the physical DPRC and synchronizes the state of the Linux
> + * 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)
> +{
> +       int error = -EINVAL;
> +
> +       error = dprc_scan_objects(mc_bus_dev);
> +       if (error < 0)
> +               goto error;
> +
> +       return 0;
> +error:
> +       return error;
> +}

This is silly.

int dprc_scan_container(struct fsl_mc_device *mc_bus_dev)
{
      return dprc_scan_objects(mc_bus_dev);
}


> +EXPORT_SYMBOL_GPL(dprc_scan_container);
> +
> +/**
> + * dprc_probe - callback invoked when a DPRC is being bound to this driver
> + *
> + * @mc_dev: Pointer to fsl-mc device representing a DPRC
> + *
> + * It opens the physical DPRC in the MC.
> + * It scans the DPRC to discover the MC objects contained in it.
> + */
> +static int dprc_probe(struct fsl_mc_device *mc_dev)
> +{
> +       int error = -EINVAL;
> +       bool dprc_opened = false;
> +
> +       if (WARN_ON(strcmp(mc_dev->obj_desc.type, "dprc") != 0))
> +               goto error;

Just 'return' here.   And if you're creative, I think you can get rid
of dprc_opened.  You can have multiple exit labels for gotos.  Try to
avoid using booleans to tell you at the end of the function whether
you succeeded.  That's not how kernel code is normally written.

> +
> +       error = dprc_open(mc_dev->mc_io, mc_dev->obj_desc.id,
> +                         &mc_dev->mc_handle);
> +       if (error < 0) {
> +               dev_err(&mc_dev->dev, "dprc_open() failed: %d\n", error);
> +               goto error;
> +       }
> +
> +       dprc_opened = true;
> +       error = dprc_scan_container(mc_dev);
> +       if (error < 0)
> +               goto error;
> +
> +       dev_info(&mc_dev->dev, "DPRC device bound to driver");
> +       return 0;
> +error:
> +       if (dprc_opened)
> +               (void)dprc_close(mc_dev->mc_io, mc_dev->mc_handle);
> +
> +       return error;
> +}
> +
> +/**
> + * dprc_remove - callback invoked when a DPRC is being unbound from this driver
> + *
> + * @mc_dev: Pointer to fsl-mc device representing the DPRC
> + *
> + * It removes the DPRC's child objects from Linux (not from the MC) and
> + * closes the DPRC device in the MC.
> + */
> +static int dprc_remove(struct fsl_mc_device *mc_dev)
> +{
> +       int error = -EINVAL;
> +
> +       if (WARN_ON(strcmp(mc_dev->obj_desc.type, "dprc") != 0))
> +               goto error;
> +       if (WARN_ON(mc_dev->mc_io == NULL))
> +               goto error;
> +
> +       device_for_each_child(&mc_dev->dev, NULL, __fsl_mc_device_remove);
> +       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);
> +
> +       dev_info(&mc_dev->dev, "DPRC device unbound from driver");
> +       return 0;
> +error:
> +       return error;
> +}

Try this:

static int dprc_remove(struct fsl_mc_device *mc_dev)
{
       int error;

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

       if (WARN_ON(mc_dev->mc_io == NULL))
               return -EINVAL;

       device_for_each_child(&mc_dev->dev, NULL, __fsl_mc_device_remove);

       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);
               return error;
       }

       dev_dbg(&mc_dev->dev, "DPRC device unbound from driver");
       return 0;
}


> +
> +static const struct fsl_mc_device_match_id match_id_table[] = {
> +       {
> +        .vendor = FSL_MC_VENDOR_FREESCALE,
> +        .obj_type = "dprc",
> +        .ver_major = DPRC_VER_MAJOR,
> +        .ver_minor = DPRC_VER_MINOR},
> +       {.vendor = 0x0},
> +};
> +
> +static struct fsl_mc_driver dprc_driver = {
> +       .driver = {
> +                  .name = FSL_MC_DPRC_DRIVER_NAME,
> +                  .owner = THIS_MODULE,
> +                  .pm = NULL,
> +                  },
> +       .match_id_table = match_id_table,
> +       .probe = dprc_probe,
> +       .remove = dprc_remove,
> +};
> +
> +int __init dprc_driver_init(void)
> +{
> +       return fsl_mc_driver_register(&dprc_driver);
> +}
> +
> +void __exit dprc_driver_exit(void)
> +{
> +       fsl_mc_driver_unregister(&dprc_driver);
> +}
> diff --git a/include/linux/fsl_mc_private.h b/include/linux/fsl_mc_private.h
> index 3ff3f2b..52721f7 100644
> --- a/include/linux/fsl_mc_private.h
> +++ b/include/linux/fsl_mc_private.h
> @@ -15,6 +15,8 @@
>  #include <linux/mutex.h>
>  #include <linux/stringify.h>
>
> +#define FSL_MC_DPRC_DRIVER_NAME    "fsl_mc_dprc"
> +
>  #define FSL_MC_DEVICE_MATCH(_mc_dev, _obj_desc) \
>         (strcmp((_mc_dev)->obj_desc.type, (_obj_desc)->type) == 0 && \
>          (_mc_dev)->obj_desc.id == (_obj_desc)->id)
> @@ -30,4 +32,12 @@ 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 __must_check dprc_scan_container(struct fsl_mc_device *mc_bus_dev);
> +
> +int __must_check dprc_scan_objects(struct fsl_mc_device *mc_bus_dev);

__must_check?  Really?

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

* Re: [PATCH 3/3 v2] drivers/bus: Device driver for FSL-MC DPRC devices
  2014-10-01  2:19   ` Timur Tabi
@ 2014-10-01  2:27     ` Scott Wood
  2014-10-01  2:35       ` Timur Tabi
  2014-10-02 16:36     ` German Rivera
  1 sibling, 1 reply; 20+ messages in thread
From: Scott Wood @ 2014-10-01  2:27 UTC (permalink / raw)
  To: Timur Tabi
  Cc: J. German Rivera, Greg Kroah-Hartman, Arnd Bergmann, lkml,
	stuart.yoder, Kim Phillips, Alexander Graf, linuxppc-release

On Tue, 2014-09-30 at 21:19 -0500, Timur Tabi wrote:
> On Fri, Sep 19, 2014 at 5:49 PM, J. German Rivera
> <German.Rivera@freescale.com> wrote:
> > +/**
> > + * dprc_remove_devices - Removes devices for objects removed from a DPRC
> > + *
> > + * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
> > + * @obj_desc_array: array of object descriptors for child objects currently
> > + * present in the DPRC in the MC.
> > + * @num_child_objects_in_mc: number of entries in obj_desc_array
> > + *
> > + * Synchronizes the state of the Linux bus driver with the actual state of
> > + * the MC by removing devices that represent MC objects that have
> > + * been dynamically removed in the physical DPRC.
> > + */
> > +static void dprc_remove_devices(struct fsl_mc_device *mc_bus_dev,
> > +                               struct dprc_obj_desc *obj_desc_array,
> > +                               int num_child_objects_in_mc)
> > +{
> > +       if (num_child_objects_in_mc != 0) {
> 
> Like here.  Just do "if (num_child_objects_in_mc) {"

This seems to be a place that is testing for zero as a value rather than
as a stand-in for NULL, so I'd argue it's better style to leave it as
is.

-Scott



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

* Re: [PATCH 3/3 v2] drivers/bus: Device driver for FSL-MC DPRC devices
  2014-10-01  2:27     ` Scott Wood
@ 2014-10-01  2:35       ` Timur Tabi
  0 siblings, 0 replies; 20+ messages in thread
From: Timur Tabi @ 2014-10-01  2:35 UTC (permalink / raw)
  To: Scott Wood
  Cc: J. German Rivera, Greg Kroah-Hartman, Arnd Bergmann, lkml,
	stuart.yoder, Kim Phillips, Alexander Graf

Scott Wood wrote:
>>> > >+       if (num_child_objects_in_mc != 0) {
>> >
>> >Like here.  Just do "if (num_child_objects_in_mc) {"

> This seems to be a place that is testing for zero as a value rather than
> as a stand-in for NULL, so I'd argue it's better style to leave it as
> is.

But in this case, zero means "none left", so this is a perfectly valid 
use-case for omitting the !=.

strcmp() is the only time I can think of to put an explicit != 0, 
because this:

	if (!strcmp(s1, s2))

is counter-intuitive.

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

* Re: [PATCH 3/3 v2] drivers/bus: Device driver for FSL-MC DPRC devices
  2014-10-01  2:19   ` Timur Tabi
  2014-10-01  2:27     ` Scott Wood
@ 2014-10-02 16:36     ` German Rivera
  2014-10-02 17:19       ` Timur Tabi
  1 sibling, 1 reply; 20+ messages in thread
From: German Rivera @ 2014-10-02 16:36 UTC (permalink / raw)
  To: Timur Tabi
  Cc: Greg Kroah-Hartman, Arnd Bergmann, lkml, stuart.yoder,
	Kim Phillips, Scott Wood, Alexander Graf, linuxppc-release


On 09/30/2014 09:19 PM, Timur Tabi wrote:
> On Fri, Sep 19, 2014 at 5:49 PM, J. German Rivera
> <German.Rivera@freescale.com> wrote:
>
>> +static int __fsl_mc_device_remove(struct device *dev, void *data)
>> +{
>> +       WARN_ON(dev == NULL);
>> +       WARN_ON(data != NULL);
>
> I see a lot of direct comparisons with NULL and 0.  You don't need to
> be so explicit:
>
> WARN_ON(!dev);
> WARN_ON(!data);
>
> "!= 0" and "!= NULL" is almost always unnecessary, and you can delete them.
>
I know that this is not necessary from the point of view of C boolean 
semantics, but doing explicit comparison improves readability IMHO. 
Anyway, this is subjective and largely a matter of preference.
Besides, "Documentation/CodingStyle" does not seem to advocate one way 
or the other.
Also, I there is evidence that explicit comparisons are allowed in
the kernel source tree:

$  git grep '== NULL' | wc -l
22173
$  git grep '!= NULL' | wc -l
9987
$  git grep '!= 0' | wc -l
22993
$  git grep '== 0' | wc -l
46060

So, unless Greg Kroah-Hartman or Arnd Bergmann feel strongly that I need 
to make this change, I will not make it.

>
>> +       fsl_mc_device_remove(to_fsl_mc_device(dev));
>> +       return 0;
>> +}
>> +
>> +/**
>> + * dprc_remove_devices - Removes devices for objects removed from a DPRC
>> + *
>> + * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
>> + * @obj_desc_array: array of object descriptors for child objects currently
>> + * present in the DPRC in the MC.
>> + * @num_child_objects_in_mc: number of entries in obj_desc_array
>> + *
>> + * Synchronizes the state of the Linux bus driver with the actual state of
>> + * the MC by removing devices that represent MC objects that have
>> + * been dynamically removed in the physical DPRC.
>> + */
>> +static void dprc_remove_devices(struct fsl_mc_device *mc_bus_dev,
>> +                               struct dprc_obj_desc *obj_desc_array,
>> +                               int num_child_objects_in_mc)
>> +{
>> +       if (num_child_objects_in_mc != 0) {
>
> Like here.  Just do "if (num_child_objects_in_mc) {"
>
>> +               /*
>> +                * Remove child objects that are in the DPRC in Linux,
>> +                * but not in the MC:
>> +                */
>> +               struct dprc_child_objs objs;
>> +
>> +               objs.child_count = num_child_objects_in_mc;
>> +               objs.child_array = obj_desc_array;
>> +               device_for_each_child(&mc_bus_dev->dev, &objs,
>> +                                     __fsl_mc_device_remove_if_not_in_mc);
>> +       } else {
>> +               /*
>> +                * There are no child objects for this DPRC in the MC.
>> +                * So, remove all the child devices from Linux:
>> +                */
>> +               device_for_each_child(&mc_bus_dev->dev, NULL,
>> +                                     __fsl_mc_device_remove);
>> +       }
>> +}
>> +
>> +static int __fsl_mc_device_match(struct device *dev, void *data)
>> +{
>> +       struct dprc_obj_desc *obj_desc = data;
>> +       struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
>> +
>> +       return FSL_MC_DEVICE_MATCH(mc_dev, obj_desc);
>> +}
>> +
>> +static struct fsl_mc_device *fsl_mc_device_lookup(struct dprc_obj_desc
>> +                                                               *obj_desc,
>> +                                                 struct fsl_mc_device
>> +                                                               *mc_bus_dev)
>> +{
>> +       struct device *dev;
>> +
>> +       dev = device_find_child(&mc_bus_dev->dev, obj_desc,
>> +                               __fsl_mc_device_match);
>> +
>> +       return (dev != NULL) ? to_fsl_mc_device(dev) : NULL;
>
> return dev ? to_fsl_mc_device(dev) : NULL;
>
>> +}
>> +
>> +/**
>> + * 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
>> + * @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
>> + *
>> + * Synchronizes the state of the Linux bus driver with the actual
>> + * state of the MC by adding objects that have been newly discovered
>> + * in the physical DPRC.
>> + */
>> +static void dprc_add_new_devices(struct fsl_mc_device *mc_bus_dev,
>> +                                struct dprc_obj_desc *obj_desc_array,
>> +                                int num_child_objects_in_mc)
>> +{
>> +       int error;
>> +       int i;
>> +
>> +       for (i = 0; i < num_child_objects_in_mc; i++) {
>> +               struct dprc_region_desc region_desc;
>> +               struct fsl_mc_device *child_dev;
>> +               struct fsl_mc_io *mc_io = NULL;
>> +               struct dprc_obj_desc *obj_desc = &obj_desc_array[i];
>> +
>> +               if (strlen(obj_desc->type) == 0)
>> +                       continue;
>> +               /*
>> +                * Check if device is already known to Linux:
>> +                */
>> +               child_dev = fsl_mc_device_lookup(obj_desc, mc_bus_dev);
>> +               if (child_dev != NULL)
>> +                       continue;
>> +
>> +               if (strcmp(obj_desc->type, "dprc") == 0) {
>
> As for the !=0 thing, I make an exception for strcmp(), however, since
> its meaning is not intuitive.
>
>> +                       /*
>> +                        * Get the MC portal physical address for the device
>> +                        * (specified in the device's first MMIO region):
>> +                        */
>> +                       if (WARN_ON(obj_desc->region_count == 0))
>> +                               continue;
>> +
>> +                       error = dprc_get_obj_region(mc_bus_dev->mc_io,
>> +                                                   mc_bus_dev->mc_handle,
>> +                                                   obj_desc->type,
>> +                                                   obj_desc->id,
>> +                                                   0, &region_desc);
>> +                       if (error < 0) {
>> +                               dev_err(&mc_bus_dev->dev,
>> +                                       "dprc_get_obj_region() failed: %d\n",
>> +                                       error);
>> +                               continue;
>> +                       }
>> +
>> +                       if (region_desc.size !=
>> +                                       mc_bus_dev->mc_io->portal_size) {
>> +                               error = -EINVAL;
>> +                               dev_err(&mc_bus_dev->dev,
>> +                                       "Invalid MC portal size: %u\n",
>> +                                       region_desc.size);
>> +                               continue;
>> +                       }
>> +
>> +                       error = fsl_create_mc_io(&mc_bus_dev->dev,
>> +                                                region_desc.base_paddr,
>> +                                                region_desc.size, 0, &mc_io);
>> +                       if (error < 0)
>> +                               continue;
>> +               }
>> +
>> +               error = fsl_mc_device_add(obj_desc, mc_io, &mc_bus_dev->dev,
>> +                                         &child_dev);
>> +               if (error < 0) {
>> +                       if (mc_io != NULL)
>> +                               fsl_destroy_mc_io(mc_io);
>> +
>> +                       continue;
>> +               }
>> +       }
>> +}
>> +
>> +/**
>> + * dprc_scan_objects - Discover objects in a DPRC
>> + *
>> + * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
>> + *
>> + * Detects objects added and removed from a DPRC and synchronizes the
>> + * state of the Linux bus driver, MC by adding and removing
>> + * devices accordingly.
>> + */
>> +int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev)
>> +{
>> +       int num_child_objects;
>> +       int dprc_get_obj_failures;
>> +       int error = -EINVAL;
>> +       struct dprc_obj_desc *child_obj_desc_array = NULL;
>> +
>> +       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);
>> +               goto out;
>> +       }
>> +
>> +       if (num_child_objects != 0) {
>> +               int i;
>> +
>> +               child_obj_desc_array =
>> +                   devm_kmalloc_array(&mc_bus_dev->dev, num_child_objects,
>> +                                      sizeof(*child_obj_desc_array),
>> +                                      GFP_KERNEL);
>> +               if (child_obj_desc_array == NULL) {
>> +                       error = -ENOMEM;
>> +                       dev_err(&mc_bus_dev->dev,
>> +                               "No memory to allocate obj_desc array\n");
>> +                       goto out;
>> +               }
>> +
>> +               /*
>> +                * Discover objects currently present in the physical DPRC:
>> +                */
>> +               dprc_get_obj_failures = 0;
>> +               for (i = 0; i < num_child_objects; i++) {
>> +                       struct dprc_obj_desc *obj_desc =
>> +                           &child_obj_desc_array[i];
>> +
>> +                       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);
>> +                               /*
>> +                                * Mark the obj entry as "invalid", by using the
>> +                                * empty string as obj type:
>> +                                */
>> +                               obj_desc->type[0] = '\0';
>> +                               obj_desc->id = error;
>> +                               dprc_get_obj_failures++;
>> +                               continue;
>> +                       }
>> +
>> +                       dev_info(&mc_bus_dev->dev,
>> +                                "Discovered object: type %s, id %d\n",
>> +                                obj_desc->type, obj_desc->id);
>> +               }
>> +
>> +               if (dprc_get_obj_failures != 0) {
>> +                       dev_err(&mc_bus_dev->dev,
>> +                               "%d out of %d devices could not be retrieved\n",
>> +                               dprc_get_obj_failures, num_child_objects);
>> +               }
>> +       }
>> +
>> +       dprc_remove_devices(mc_bus_dev, child_obj_desc_array,
>> +                           num_child_objects);
>> +
>> +       dprc_add_new_devices(mc_bus_dev, child_obj_desc_array,
>> +                            num_child_objects);
>> +
>> +       error = 0;
>> +out:
>> +       if (child_obj_desc_array != NULL)
>> +               devm_kfree(&mc_bus_dev->dev, child_obj_desc_array);
>
> The whole point behind the devm_ functions is that you shoudn't need
> to manually release the resources.
>
Yes, but wouldn't the implicit/automatic freeing only happens when the 
device itself is removed?

The function above may get invoked many times before the device gets 
removed. So, we would just keep accumulating wasted memory in the meantime.

>> +
>> +       return error;
>> +}
>> +EXPORT_SYMBOL_GPL(dprc_scan_objects);
>> +
>> +/**
>> + * 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
>> + *
>> + * Scans the physical DPRC and synchronizes the state of the Linux
>> + * 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)
>> +{
>> +       int error = -EINVAL;
>> +
>> +       error = dprc_scan_objects(mc_bus_dev);
>> +       if (error < 0)
>> +               goto error;
>> +
>> +       return 0;
>> +error:
>> +       return error;
>> +}
>
> This is silly.
>
> int dprc_scan_container(struct fsl_mc_device *mc_bus_dev)
> {
>        return dprc_scan_objects(mc_bus_dev);
> }
>
Agreed. I'll simplify this.
>
>> +EXPORT_SYMBOL_GPL(dprc_scan_container);
>> +
>> +/**
>> + * dprc_probe - callback invoked when a DPRC is being bound to this driver
>> + *
>> + * @mc_dev: Pointer to fsl-mc device representing a DPRC
>> + *
>> + * It opens the physical DPRC in the MC.
>> + * It scans the DPRC to discover the MC objects contained in it.
>> + */
>> +static int dprc_probe(struct fsl_mc_device *mc_dev)
>> +{
>> +       int error = -EINVAL;
>> +       bool dprc_opened = false;
>> +
>> +       if (WARN_ON(strcmp(mc_dev->obj_desc.type, "dprc") != 0))
>> +               goto error;
>
> Just 'return' here.   And if you're creative, I think you can get rid
> of dprc_opened.  You can have multiple exit labels for gotos.  Try to
> avoid using booleans to tell you at the end of the function whether
> you succeeded.  That's not how kernel code is normally written.
>
OK, I will make this change.
>> +
>> +       error = dprc_open(mc_dev->mc_io, mc_dev->obj_desc.id,
>> +                         &mc_dev->mc_handle);
>> +       if (error < 0) {
>> +               dev_err(&mc_dev->dev, "dprc_open() failed: %d\n", error);
>> +               goto error;
>> +       }
>> +
>> +       dprc_opened = true;
>> +       error = dprc_scan_container(mc_dev);
>> +       if (error < 0)
>> +               goto error;
>> +
>> +       dev_info(&mc_dev->dev, "DPRC device bound to driver");
>> +       return 0;
>> +error:
>> +       if (dprc_opened)
>> +               (void)dprc_close(mc_dev->mc_io, mc_dev->mc_handle);
>> +
>> +       return error;
>> +}
>> +
>> +/**
>> + * dprc_remove - callback invoked when a DPRC is being unbound from this driver
>> + *
>> + * @mc_dev: Pointer to fsl-mc device representing the DPRC
>> + *
>> + * It removes the DPRC's child objects from Linux (not from the MC) and
>> + * closes the DPRC device in the MC.
>> + */
>> +static int dprc_remove(struct fsl_mc_device *mc_dev)
>> +{
>> +       int error = -EINVAL;
>> +
>> +       if (WARN_ON(strcmp(mc_dev->obj_desc.type, "dprc") != 0))
>> +               goto error;
>> +       if (WARN_ON(mc_dev->mc_io == NULL))
>> +               goto error;
>> +
>> +       device_for_each_child(&mc_dev->dev, NULL, __fsl_mc_device_remove);
>> +       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);
>> +
>> +       dev_info(&mc_dev->dev, "DPRC device unbound from driver");
>> +       return 0;
>> +error:
>> +       return error;
>> +}
>
> Try this:
>
> static int dprc_remove(struct fsl_mc_device *mc_dev)
> {
>         int error;
>
>         if (WARN_ON(strcmp(mc_dev->obj_desc.type, "dprc") != 0))
>                 return -EINVAL;
>
>         if (WARN_ON(mc_dev->mc_io == NULL))
>                 return -EINVAL;
>
>         device_for_each_child(&mc_dev->dev, NULL, __fsl_mc_device_remove);
>
>         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);
>                 return error;
>         }
>
>         dev_dbg(&mc_dev->dev, "DPRC device unbound from driver");
>         return 0;
> }
>
OK, I will make this change, as there is no common cleanup to do in this 
case.
>
>> +
>> +static const struct fsl_mc_device_match_id match_id_table[] = {
>> +       {
>> +        .vendor = FSL_MC_VENDOR_FREESCALE,
>> +        .obj_type = "dprc",
>> +        .ver_major = DPRC_VER_MAJOR,
>> +        .ver_minor = DPRC_VER_MINOR},
>> +       {.vendor = 0x0},
>> +};
>> +
>> +static struct fsl_mc_driver dprc_driver = {
>> +       .driver = {
>> +                  .name = FSL_MC_DPRC_DRIVER_NAME,
>> +                  .owner = THIS_MODULE,
>> +                  .pm = NULL,
>> +                  },
>> +       .match_id_table = match_id_table,
>> +       .probe = dprc_probe,
>> +       .remove = dprc_remove,
>> +};
>> +
>> +int __init dprc_driver_init(void)
>> +{
>> +       return fsl_mc_driver_register(&dprc_driver);
>> +}
>> +
>> +void __exit dprc_driver_exit(void)
>> +{
>> +       fsl_mc_driver_unregister(&dprc_driver);
>> +}
>> diff --git a/include/linux/fsl_mc_private.h b/include/linux/fsl_mc_private.h
>> index 3ff3f2b..52721f7 100644
>> --- a/include/linux/fsl_mc_private.h
>> +++ b/include/linux/fsl_mc_private.h
>> @@ -15,6 +15,8 @@
>>   #include <linux/mutex.h>
>>   #include <linux/stringify.h>
>>
>> +#define FSL_MC_DPRC_DRIVER_NAME    "fsl_mc_dprc"
>> +
>>   #define FSL_MC_DEVICE_MATCH(_mc_dev, _obj_desc) \
>>          (strcmp((_mc_dev)->obj_desc.type, (_obj_desc)->type) == 0 && \
>>           (_mc_dev)->obj_desc.id == (_obj_desc)->id)
>> @@ -30,4 +32,12 @@ 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 __must_check dprc_scan_container(struct fsl_mc_device *mc_bus_dev);
>> +
>> +int __must_check dprc_scan_objects(struct fsl_mc_device *mc_bus_dev);
>
> __must_check?  Really?
Yes, to ensure that callers that are not checking the return code from 
dprc_scan_objects() are caught at compile-time (CHECK time).

Thanks,

German
>

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

* Re: [PATCH 3/3 v2] drivers/bus: Device driver for FSL-MC DPRC devices
  2014-10-02 16:36     ` German Rivera
@ 2014-10-02 17:19       ` Timur Tabi
  0 siblings, 0 replies; 20+ messages in thread
From: Timur Tabi @ 2014-10-02 17:19 UTC (permalink / raw)
  To: German Rivera
  Cc: Greg Kroah-Hartman, Arnd Bergmann, lkml, stuart.yoder,
	Kim Phillips, Scott Wood, Alexander Graf

German Rivera wrote:
>>
> I know that this is not necessary from the point of view of C boolean
> semantics, but doing explicit comparison improves readability IMHO.

I guess we'll have to agree to disagree.  I think it makes the code less 
readable.

> Anyway, this is subjective and largely a matter of preference.
> Besides, "Documentation/CodingStyle" does not seem to advocate one way
> or the other.

CodingStyle is a starting point, not the final word.  If I had a dime 
whenever someone insisted a code snippet is okay because it's not 
covered by CodingStyle, I could retire.

> Also, I there is evidence that explicit comparisons are allowed in
> the kernel source tree:

Allowed != preferred.  Besides, there are tons of examples of almost 
every style mistake in the kernel today.  Some code is really old, or it 
was accepted by lazy maintainers and never fixed.  You can't really use 
that as a basis for a decision.


>>> +int __must_check dprc_scan_container(struct fsl_mc_device *mc_bus_dev);
>>> +
>>> +int __must_check dprc_scan_objects(struct fsl_mc_device *mc_bus_dev);
>>
>> __must_check?  Really?

> Yes, to ensure that callers that are not checking the return code from
> dprc_scan_objects() are caught at compile-time (CHECK time).

I know what __must_check is for.  I'm just saying that you kinda need to 
justify using it.  It's like using likely() on non-time-critical code. 
Overuse is worse than not using it at all, and I don't see what's so 
special about these functions that they need __must_check.

(on a side note, you're supposed to bcc: 
linuxppc-release@linux.freescale.net, not cc: it.)

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

end of thread, other threads:[~2014-10-02 17:19 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-19 22:49 [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series J. German Rivera
2014-09-19 22:49 ` [PATCH 1/3 v2] drivers/bus: Added Freescale Management Complex APIs J. German Rivera
2014-09-24  0:49   ` Kim Phillips
2014-09-25  2:23     ` German Rivera
2014-09-25  3:40       ` Kim Phillips
2014-09-25 15:44         ` German Rivera
2014-09-25 16:16           ` Scott Wood
2014-09-25 16:44             ` German Rivera
2014-09-25 20:05               ` Scott Wood
2014-09-19 22:49 ` [PATCH 2/3 v2] drivers/bus: Freescale Management Complex (fsl-mc) bus driver J. German Rivera
2014-09-19 22:49 ` [PATCH 3/3 v2] drivers/bus: Device driver for FSL-MC DPRC devices J. German Rivera
2014-10-01  2:19   ` Timur Tabi
2014-10-01  2:27     ` Scott Wood
2014-10-01  2:35       ` Timur Tabi
2014-10-02 16:36     ` German Rivera
2014-10-02 17:19       ` Timur Tabi
2014-09-22 16:53 ` [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series Kim Phillips
2014-09-22 17:59   ` Stuart Yoder
2014-09-22 22:03     ` Kim Phillips
2014-09-23 14:52     ` German Rivera

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