Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752686AbbEHHW6 (ORCPT ); Fri, 8 May 2015 03:22:58 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:20958 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752663AbbEHHWy (ORCPT ); Fri, 8 May 2015 03:22:54 -0400 Date: Fri, 8 May 2015 10:22:30 +0300 From: Dan Carpenter To: "J. German Rivera" Cc: gregkh@linuxfoundation.org, arnd@arndb.de, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, stuart.yoder@freescale.com, bhupesh.sharma@freescale.com, agraf@suse.de, bhamciu1@freescale.com, nir.erez@freescale.com, itai.katz@freescale.com, scottwood@freescale.com, R89243@freescale.com, richard.schmitt@freescale.com Subject: Re: [PATCH v2 4/7] staging: fsl-mc: Upgraded MC bus driver to match MC fw 7.0.0 Message-ID: <20150508072229.GL14154@mwanda> References: <1430947708-10521-1-git-send-email-German.Rivera@freescale.com> <1430947708-10521-5-git-send-email-German.Rivera@freescale.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1430947708-10521-5-git-send-email-German.Rivera@freescale.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 24006 Lines: 701 Sorry, I'm reviewing this patchset slowly. On Wed, May 06, 2015 at 04:28:25PM -0500, J. German Rivera wrote: > - Migrated MC bus driver to use DPRC API 0.6. > - Changed IRQ setup infrastructure to be able to program MSIs > for MC objects in an object-independent way. > > Signed-off-by: J. German Rivera > --- > Changes in v2: > - Addressed comments from Dan Carpenter: > * Added #ifdef GIC_ITS_MC_SUPPORT's that had been removed > by mistake. > * Removed EXPORTs that are not used by other MC object drivers > yet. > * Removed unused function dprc_lookup_object() > > drivers/staging/fsl-mc/bus/dpmcp-cmd.h | 79 -------------- > drivers/staging/fsl-mc/bus/dprc-cmd.h | 6 +- > drivers/staging/fsl-mc/bus/dprc-driver.c | 37 +++++-- > drivers/staging/fsl-mc/bus/dprc.c | 155 ++++++++++++++++++++++------ > drivers/staging/fsl-mc/bus/mc-allocator.c | 26 +++-- > drivers/staging/fsl-mc/bus/mc-bus.c | 77 +++++++++----- > drivers/staging/fsl-mc/bus/mc-sys.c | 6 +- > drivers/staging/fsl-mc/include/dpmng.h | 4 +- > drivers/staging/fsl-mc/include/dprc.h | 114 +++++++++++++++----- > drivers/staging/fsl-mc/include/mc-private.h | 29 +++++- > drivers/staging/fsl-mc/include/mc.h | 4 + > 11 files changed, 345 insertions(+), 192 deletions(-) > > diff --git a/drivers/staging/fsl-mc/bus/dpmcp-cmd.h b/drivers/staging/fsl-mc/bus/dpmcp-cmd.h > index 57f326b..62bdc18 100644 > --- a/drivers/staging/fsl-mc/bus/dpmcp-cmd.h > +++ b/drivers/staging/fsl-mc/bus/dpmcp-cmd.h > @@ -54,83 +54,4 @@ > #define DPMCP_CMDID_GET_IRQ_STATUS 0x016 > #define DPMCP_CMDID_CLEAR_IRQ_STATUS 0x017 > > -/* cmd, param, offset, width, type, arg_name */ > -#define DPMCP_CMD_CREATE(cmd, cfg) \ > - MC_CMD_OP(cmd, 0, 0, 32, int, cfg->portal_id) > - > -/* cmd, param, offset, width, type, arg_name */ > -#define DPMCP_CMD_SET_IRQ(cmd, irq_index, irq_addr, irq_val, user_irq_id) \ > -do { \ > - MC_CMD_OP(cmd, 0, 0, 8, uint8_t, irq_index);\ > - MC_CMD_OP(cmd, 0, 32, 32, uint32_t, irq_val);\ > - MC_CMD_OP(cmd, 1, 0, 64, uint64_t, irq_addr); \ > - MC_CMD_OP(cmd, 2, 0, 32, int, user_irq_id); \ > -} while (0) > - > -/* cmd, param, offset, width, type, arg_name */ > -#define DPMCP_CMD_GET_IRQ(cmd, irq_index) \ > - MC_CMD_OP(cmd, 0, 32, 8, uint8_t, irq_index) > - > -/* cmd, param, offset, width, type, arg_name */ > -#define DPMCP_RSP_GET_IRQ(cmd, type, irq_addr, irq_val, user_irq_id) \ > -do { \ > - MC_RSP_OP(cmd, 0, 0, 32, uint32_t, irq_val); \ > - MC_RSP_OP(cmd, 1, 0, 64, uint64_t, irq_addr); \ > - MC_RSP_OP(cmd, 2, 0, 32, int, user_irq_id); \ > - MC_RSP_OP(cmd, 2, 32, 32, int, type); \ > -} while (0) > - > -/* cmd, param, offset, width, type, arg_name */ > -#define DPMCP_CMD_SET_IRQ_ENABLE(cmd, irq_index, en) \ > -do { \ > - MC_CMD_OP(cmd, 0, 0, 8, uint8_t, en); \ > - MC_CMD_OP(cmd, 0, 32, 8, uint8_t, irq_index);\ > -} while (0) > - > -/* cmd, param, offset, width, type, arg_name */ > -#define DPMCP_CMD_GET_IRQ_ENABLE(cmd, irq_index) \ > - MC_CMD_OP(cmd, 0, 32, 8, uint8_t, irq_index) > - > -/* cmd, param, offset, width, type, arg_name */ > -#define DPMCP_RSP_GET_IRQ_ENABLE(cmd, en) \ > - MC_RSP_OP(cmd, 0, 0, 8, uint8_t, en) > - > -/* cmd, param, offset, width, type, arg_name */ > -#define DPMCP_CMD_SET_IRQ_MASK(cmd, irq_index, mask) \ > -do { \ > - MC_CMD_OP(cmd, 0, 0, 32, uint32_t, mask);\ > - MC_CMD_OP(cmd, 0, 32, 8, uint8_t, irq_index);\ > -} while (0) > - > -/* cmd, param, offset, width, type, arg_name */ > -#define DPMCP_CMD_GET_IRQ_MASK(cmd, irq_index) \ > - MC_CMD_OP(cmd, 0, 32, 8, uint8_t, irq_index) > - > -/* cmd, param, offset, width, type, arg_name */ > -#define DPMCP_RSP_GET_IRQ_MASK(cmd, mask) \ > - MC_RSP_OP(cmd, 0, 0, 32, uint32_t, mask) > - > -/* cmd, param, offset, width, type, arg_name */ > -#define DPMCP_CMD_GET_IRQ_STATUS(cmd, irq_index) \ > - MC_CMD_OP(cmd, 0, 32, 8, uint8_t, irq_index) > - > -/* cmd, param, offset, width, type, arg_name */ > -#define DPMCP_RSP_GET_IRQ_STATUS(cmd, status) \ > - MC_RSP_OP(cmd, 0, 0, 32, uint32_t, status) > - > -/* cmd, param, offset, width, type, arg_name */ > -#define DPMCP_CMD_CLEAR_IRQ_STATUS(cmd, irq_index, status) \ > -do { \ > - MC_CMD_OP(cmd, 0, 0, 32, uint32_t, status); \ > - MC_CMD_OP(cmd, 0, 32, 8, uint8_t, irq_index);\ > -} while (0) > - > -/* cmd, param, offset, width, type, arg_name */ > -#define DPMCP_RSP_GET_ATTRIBUTES(cmd, attr) \ > -do { \ > - MC_RSP_OP(cmd, 0, 32, 32, int, attr->id);\ > - MC_RSP_OP(cmd, 1, 0, 16, uint16_t, attr->version.major);\ > - MC_RSP_OP(cmd, 1, 16, 16, uint16_t, attr->version.minor);\ > -} while (0) > - > #endif /* _FSL_DPMCP_CMD_H */ These changes are not related to the rest. They should be sent by themselves as: [patch] remove unused defines > diff --git a/drivers/staging/fsl-mc/bus/dprc-cmd.h b/drivers/staging/fsl-mc/bus/dprc-cmd.h > index 0920248..df5ad5f 100644 > --- a/drivers/staging/fsl-mc/bus/dprc-cmd.h > +++ b/drivers/staging/fsl-mc/bus/dprc-cmd.h > @@ -41,7 +41,7 @@ > #define _FSL_DPRC_CMD_H > > /* DPRC Version */ > -#define DPRC_VER_MAJOR 3 > +#define DPRC_VER_MAJOR 4 > #define DPRC_VER_MINOR 0 > > /* Command IDs */ > @@ -72,12 +72,14 @@ > #define DPRC_CMDID_GET_RES_COUNT 0x15B > #define DPRC_CMDID_GET_RES_IDS 0x15C > #define DPRC_CMDID_GET_OBJ_REG 0x15E > +#define DPRC_CMDID_OBJ_SET_IRQ 0x15F > +#define DPRC_CMDID_OBJ_GET_IRQ 0x160 > +#define DPRC_CMDID_SET_OBJ_LABEL 0x161 > > #define DPRC_CMDID_CONNECT 0x167 > #define DPRC_CMDID_DISCONNECT 0x168 > #define DPRC_CMDID_GET_POOL 0x169 > #define DPRC_CMDID_GET_POOL_COUNT 0x16A > -#define DPRC_CMDID_GET_PORTAL_PADDR 0x16B > > #define DPRC_CMDID_GET_CONNECTION 0x16C > > diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c > index 85e293b..338fd7d 100644 > --- a/drivers/staging/fsl-mc/bus/dprc-driver.c > +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c > @@ -500,6 +500,21 @@ static int register_dprc_irq_handlers(struct fsl_mc_device *mc_dev) > > for (i = 0; i < ARRAY_SIZE(irq_handlers); i++) { > irq = mc_dev->irqs[i]; > + > + if (WARN_ON(irq->dev_irq_index != i)) { > + error = -EINVAL; > + goto error_unregister_irq_handlers; We added ->dev_irq_index and ->mc_dev to the irq pointer but this is the only place where we use it. This WARN_ON() can never trigger unless we have memory corruption. Anyway, since it's not connected to anything else in the patch we can do it as a separate patch. > + } > + > + /* > + * NOTE: Normally, devm_request_threaded_irq() programs the MSI > + * physically in the device (by invoking a device-specific > + * callback). However, for MC IRQs, we have to program the MSI > + * outside of this callback in an object-specific way, because > + * the object-independent way of programming MSI is not reliable > + * yet. For now, the MC callback just sets the msi_paddr and > + * msi_value fields of the irq structure. > + */ > error = devm_request_threaded_irq(&mc_dev->dev, > irq->irq_number, > irq_handlers[i].irq_handler, > @@ -518,18 +533,17 @@ static int register_dprc_irq_handlers(struct fsl_mc_device *mc_dev) > > /* > * Program the MSI (paddr, value) pair in the device: > - * > - * TODO: This needs to be moved to mc_bus_msi_domain_write_msg() > - * when the MC object-independent dprc_set_irq() flib API > - * becomes available > */ > - error = dprc_set_irq(mc_dev->mc_io, mc_dev->mc_handle, > - i, irq->msi_paddr, > + error = dprc_set_irq(mc_dev->mc_io, > + mc_dev->mc_handle, > + i, > + irq->msi_paddr, > irq->msi_value, > irq->irq_number); > if (error < 0) { > dev_err(&mc_dev->dev, > - "mc_set_irq() failed: %d\n", error); > + "dprc_set_irq() failed for IRQ %u: %d\n", > + i, error); > return error; > } > } This chunk is basically white space cleanups. It would be better to fold it into patch 1/7 where the function was first introduced. > @@ -663,15 +677,16 @@ static int dprc_probe(struct fsl_mc_device *mc_dev) > */ > error = dprc_setup_irqs(mc_dev); > if (error < 0) > - goto error_cleanup_open; > + goto error_cleanup_dprc_scan; > > dev_info(&mc_dev->dev, "DPRC device bound to driver"); > return 0; > > -error_cleanup_open: > - if (mc_bus->irq_resources) > - fsl_mc_cleanup_irq_pool(mc_bus); > +error_cleanup_dprc_scan: > + device_for_each_child(&mc_dev->dev, NULL, __fsl_mc_device_remove); I assume this is a bug fix? It would be better to fold this into patch 1/7 where the bug was introduced? > + fsl_mc_cleanup_irq_pool(mc_bus); > > +error_cleanup_open: > (void)dprc_close(mc_dev->mc_io, mc_dev->mc_handle); > > error_cleanup_mc_io: > diff --git a/drivers/staging/fsl-mc/bus/dprc.c b/drivers/staging/fsl-mc/bus/dprc.c > index 19b26e6..62087cc 100644 > --- a/drivers/staging/fsl-mc/bus/dprc.c > +++ b/drivers/staging/fsl-mc/bus/dprc.c > @@ -73,7 +73,7 @@ int dprc_create_container(struct fsl_mc_io *mc_io, > uint16_t token, > struct dprc_cfg *cfg, > int *child_container_id, > - uint64_t *child_portal_paddr) > + uint64_t *child_portal_offset) > { > struct mc_command cmd = { 0 }; > int err; The renaming of "paddr" to "offset" is just a white space change. It's really easy to review on its own but it makes reviewing difficult to mix everything together. [patch 2] rename paddr to offset > @@ -82,6 +82,22 @@ int dprc_create_container(struct fsl_mc_io *mc_io, > cmd.params[0] |= mc_enc(32, 16, cfg->icid); > cmd.params[0] |= mc_enc(0, 32, cfg->options); > cmd.params[1] |= mc_enc(32, 32, cfg->portal_id); > + cmd.params[2] |= mc_enc(0, 8, cfg->label[0]); > + cmd.params[2] |= mc_enc(8, 8, cfg->label[1]); > + cmd.params[2] |= mc_enc(16, 8, cfg->label[2]); > + cmd.params[2] |= mc_enc(24, 8, cfg->label[3]); > + cmd.params[2] |= mc_enc(32, 8, cfg->label[4]); > + cmd.params[2] |= mc_enc(40, 8, cfg->label[5]); > + cmd.params[2] |= mc_enc(48, 8, cfg->label[6]); > + cmd.params[2] |= mc_enc(56, 8, cfg->label[7]); > + cmd.params[3] |= mc_enc(0, 8, cfg->label[8]); > + cmd.params[3] |= mc_enc(8, 8, cfg->label[9]); > + cmd.params[3] |= mc_enc(16, 8, cfg->label[10]); > + cmd.params[3] |= mc_enc(24, 8, cfg->label[11]); > + cmd.params[3] |= mc_enc(32, 8, cfg->label[12]); > + cmd.params[3] |= mc_enc(40, 8, cfg->label[13]); > + cmd.params[3] |= mc_enc(48, 8, cfg->label[14]); > + cmd.params[3] |= mc_enc(56, 8, cfg->label[15]); > > cmd.header = mc_encode_cmd_header(DPRC_CMDID_CREATE_CONT, > MC_CMD_PRI_LOW, token); > @@ -93,7 +109,7 @@ int dprc_create_container(struct fsl_mc_io *mc_io, > > /* retrieve response parameters */ > *child_container_id = mc_dec(cmd.params[1], 0, 32); > - *child_portal_paddr = mc_dec(cmd.params[2], 0, 64); > + *child_portal_offset = mc_dec(cmd.params[2], 0, 64); > > return 0; > } > @@ -159,6 +175,39 @@ int dprc_get_irq(struct fsl_mc_io *mc_io, > return 0; > } > > +int dprc_obj_get_irq(struct fsl_mc_io *mc_io, > + uint16_t token, > + int obj_index, > + uint8_t irq_index, > + int *type, > + uint64_t *irq_addr, > + uint32_t *irq_val, > + int *user_irq_id) > +{ > + struct mc_command cmd = { 0 }; > + int err; > + > + /* prepare command */ > + cmd.header = mc_encode_cmd_header(DPRC_CMDID_OBJ_GET_IRQ, > + MC_CMD_PRI_LOW, > + token); > + > + cmd.params[0] |= mc_enc(0, 32, obj_index); > + cmd.params[0] |= mc_enc(32, 8, irq_index); > + > + /* send command to mc*/ > + err = mc_send_command(mc_io, &cmd); > + if (err) > + return err; > + > + /* retrieve response parameters */ > + *irq_val = mc_dec(cmd.params[0], 0, 32); > + *irq_addr = mc_dec(cmd.params[1], 0, 64); > + *user_irq_id = mc_dec(cmd.params[2], 0, 32); > + *type = mc_dec(cmd.params[2], 32, 32); > + return 0; > +} > + This function is never called. I checked the later functions and it's not called in those either. Drop it until we have a user. > int dprc_set_irq(struct fsl_mc_io *mc_io, > uint16_t token, > uint8_t irq_index, > @@ -181,6 +230,31 @@ int dprc_set_irq(struct fsl_mc_io *mc_io, > return mc_send_command(mc_io, &cmd); > } > > +int dprc_obj_set_irq(struct fsl_mc_io *mc_io, > + uint16_t token, > + int obj_index, > + uint8_t irq_index, > + uint64_t irq_addr, > + uint32_t irq_val, > + int user_irq_id) > +{ > + struct mc_command cmd = { 0 }; > + > + /* prepare command */ > + cmd.header = mc_encode_cmd_header(DPRC_CMDID_OBJ_SET_IRQ, > + MC_CMD_PRI_LOW, > + token); > + > + cmd.params[0] |= mc_enc(32, 8, irq_index); > + cmd.params[0] |= mc_enc(0, 32, irq_val); > + cmd.params[1] |= mc_enc(0, 64, irq_addr); > + cmd.params[2] |= mc_enc(0, 32, user_irq_id); > + cmd.params[2] |= mc_enc(32, 32, obj_index); > + > + /* send command to mc*/ > + return mc_send_command(mc_io, &cmd); > +} > + > int dprc_get_irq_enable(struct fsl_mc_io *mc_io, > uint16_t token, > uint8_t irq_index, > @@ -604,7 +678,22 @@ int dprc_get_obj(struct fsl_mc_io *mc_io, > obj_desc->type[13] = mc_dec(cmd.params[4], 40, 8); > obj_desc->type[14] = mc_dec(cmd.params[4], 48, 8); > obj_desc->type[15] = '\0'; > - > + obj_desc->label[0] = mc_dec(cmd.params[5], 0, 8); > + obj_desc->label[1] = mc_dec(cmd.params[5], 8, 8); > + obj_desc->label[2] = mc_dec(cmd.params[5], 16, 8); > + obj_desc->label[3] = mc_dec(cmd.params[5], 24, 8); > + obj_desc->label[4] = mc_dec(cmd.params[5], 32, 8); > + obj_desc->label[5] = mc_dec(cmd.params[5], 40, 8); > + obj_desc->label[6] = mc_dec(cmd.params[5], 48, 8); > + obj_desc->label[7] = mc_dec(cmd.params[5], 56, 8); > + obj_desc->label[8] = mc_dec(cmd.params[6], 0, 8); > + obj_desc->label[9] = mc_dec(cmd.params[6], 8, 8); > + obj_desc->label[10] = mc_dec(cmd.params[6], 16, 8); > + obj_desc->label[11] = mc_dec(cmd.params[6], 24, 8); > + obj_desc->label[12] = mc_dec(cmd.params[6], 32, 8); > + obj_desc->label[13] = mc_dec(cmd.params[6], 40, 8); > + obj_desc->label[14] = mc_dec(cmd.params[6], 48, 8); > + obj_desc->label[15] = '\0'; > return 0; > } > EXPORT_SYMBOL(dprc_get_obj); > @@ -696,31 +785,6 @@ int dprc_get_res_ids(struct fsl_mc_io *mc_io, > } > EXPORT_SYMBOL(dprc_get_res_ids); > > -int dprc_get_portal_paddr(struct fsl_mc_io *mc_io, > - uint16_t token, > - int portal_id, > - uint64_t *portal_addr) > -{ > - struct mc_command cmd = { 0 }; > - int err; > - > - /* prepare command */ > - cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_PORTAL_PADDR, > - MC_CMD_PRI_LOW, token); > - cmd.params[0] |= mc_enc(0, 32, portal_id); > - > - /* send command to mc*/ > - err = mc_send_command(mc_io, &cmd); > - if (err) > - return err; > - > - /* retrieve response parameters */ > - *portal_addr = mc_dec(cmd.params[1], 0, 64); > - > - return 0; > -} > -EXPORT_SYMBOL(dprc_get_portal_paddr); > - This should be a separate patch. [patch 3] remove an unused function > int dprc_get_obj_region(struct fsl_mc_io *mc_io, > uint16_t token, > char *obj_type, > @@ -759,13 +823,46 @@ int dprc_get_obj_region(struct fsl_mc_io *mc_io, > return err; > > /* retrieve response parameters */ > - region_desc->base_paddr = mc_dec(cmd.params[1], 0, 64); > + region_desc->base_offset = mc_dec(cmd.params[1], 0, 64); > region_desc->size = mc_dec(cmd.params[2], 0, 32); > > return 0; > } This hunk was part of patch 2. > EXPORT_SYMBOL(dprc_get_obj_region); > > +int dprc_set_obj_label(struct fsl_mc_io *mc_io, > + uint16_t token, > + int obj_index, > + char *label) > +{ > + struct mc_command cmd = { 0 }; > + > + /* prepare command */ > + cmd.header = mc_encode_cmd_header(DPRC_CMDID_SET_OBJ_LABEL, > + MC_CMD_PRI_LOW, token); > + > + cmd.params[0] |= mc_enc(0, 32, obj_index); > + cmd.params[1] |= mc_enc(0, 8, label[0]); > + cmd.params[1] |= mc_enc(8, 8, label[1]); > + cmd.params[1] |= mc_enc(16, 8, label[2]); > + cmd.params[1] |= mc_enc(24, 8, label[3]); > + cmd.params[1] |= mc_enc(32, 8, label[4]); > + cmd.params[1] |= mc_enc(40, 8, label[5]); > + cmd.params[1] |= mc_enc(48, 8, label[6]); > + cmd.params[1] |= mc_enc(56, 8, label[7]); > + cmd.params[2] |= mc_enc(0, 8, label[8]); > + cmd.params[2] |= mc_enc(8, 8, label[9]); > + cmd.params[2] |= mc_enc(16, 8, label[10]); > + cmd.params[2] |= mc_enc(24, 8, label[11]); > + cmd.params[2] |= mc_enc(32, 8, label[12]); > + cmd.params[2] |= mc_enc(40, 8, label[13]); > + cmd.params[2] |= mc_enc(48, 8, label[14]); > + cmd.params[2] |= mc_enc(56, 8, label[15]); > + > + /* send command to mc*/ > + return mc_send_command(mc_io, &cmd); > +} > + > int dprc_connect(struct fsl_mc_io *mc_io, > uint16_t token, > const struct dprc_endpoint *endpoint1, > diff --git a/drivers/staging/fsl-mc/bus/mc-allocator.c b/drivers/staging/fsl-mc/bus/mc-allocator.c > index aa8280a..e445f79 100644 > --- a/drivers/staging/fsl-mc/bus/mc-allocator.c > +++ b/drivers/staging/fsl-mc/bus/mc-allocator.c > @@ -523,14 +523,20 @@ int __must_check fsl_mc_allocate_irqs(struct fsl_mc_device *mc_dev) > > irqs[i] = to_fsl_mc_irq(resource); > res_allocated_count++; > + > + WARN_ON(irqs[i]->mc_dev); > + irqs[i]->mc_dev = mc_dev; > + irqs[i]->dev_irq_index = i; > } > > mc_dev->irqs = irqs; > return 0; > > error_resource_alloc: > - for (i = 0; i < res_allocated_count; i++) > + for (i = 0; i < res_allocated_count; i++) { > + irqs[i]->mc_dev = NULL; > fsl_mc_resource_free(&irqs[i]->resource); > + } > > return error; > } > @@ -545,8 +551,9 @@ void fsl_mc_free_irqs(struct fsl_mc_device *mc_dev) > int i; > int irq_count; > struct fsl_mc_bus *mc_bus; > + struct fsl_mc_device_irq **irqs = mc_dev->irqs; > > - if (WARN_ON(!mc_dev->irqs)) > + if (WARN_ON(!irqs)) > return; > > irq_count = mc_dev->obj_desc.irq_count; > @@ -559,8 +566,11 @@ void fsl_mc_free_irqs(struct fsl_mc_device *mc_dev) > if (WARN_ON(!mc_bus->irq_resources)) > return; > > - for (i = 0; i < irq_count; i++) > - fsl_mc_resource_free(&mc_dev->irqs[i]->resource); > + for (i = 0; i < irq_count; i++) { > + WARN_ON(!irqs[i]->mc_dev); > + irqs[i]->mc_dev = NULL; > + fsl_mc_resource_free(&irqs[i]->resource); > + } > > mc_dev->irqs = NULL; > } > @@ -593,8 +603,8 @@ static int fsl_mc_allocator_probe(struct fsl_mc_device *mc_dev) > if (error < 0) > goto error; > > - dev_info(&mc_dev->dev, > - "Allocatable MC object device bound to fsl_mc_allocator driver"); > + dev_dbg(&mc_dev->dev, > + "Allocatable MC object device bound to fsl_mc_allocator driver"); > return 0; > error: > > @@ -616,8 +626,8 @@ static int fsl_mc_allocator_remove(struct fsl_mc_device *mc_dev) > if (error < 0) > goto out; > > - dev_info(&mc_dev->dev, > - "Allocatable MC object device unbound from fsl_mc_allocator driver"); > + dev_dbg(&mc_dev->dev, > + "Allocatable MC object device unbound from fsl_mc_allocator driver"); > error = 0; > out: > return error; These two can separated out: [patch 4] make dmesg not so spammy > diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c > index 83eb906..b82fd7b 100644 > --- a/drivers/staging/fsl-mc/bus/mc-bus.c > +++ b/drivers/staging/fsl-mc/bus/mc-bus.c > @@ -315,7 +315,8 @@ common_cleanup: > return error; > } > > -static int translate_mc_addr(uint64_t mc_addr, phys_addr_t *phys_addr) > +static int translate_mc_addr(enum fsl_mc_region_types mc_region_type, > + uint64_t mc_offset, phys_addr_t *phys_addr) > { > int i; > struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent); > @@ -324,7 +325,7 @@ static int translate_mc_addr(uint64_t mc_addr, phys_addr_t *phys_addr) > /* > * Do identity mapping: > */ > - *phys_addr = mc_addr; > + *phys_addr = mc_offset; > return 0; > } > > @@ -332,10 +333,11 @@ static int translate_mc_addr(uint64_t mc_addr, phys_addr_t *phys_addr) > struct fsl_mc_addr_translation_range *range = > &mc->translation_ranges[i]; > > - if (mc_addr >= range->start_mc_addr && > - mc_addr < range->end_mc_addr) { > + if (mc_region_type == range->mc_region_type && > + mc_offset >= range->start_mc_offset && > + mc_offset < range->end_mc_offset) { > *phys_addr = range->start_phys_addr + > - (mc_addr - range->start_mc_addr); > + (mc_offset - range->start_mc_offset); > return 0; > } > } > @@ -351,6 +353,22 @@ static int fsl_mc_device_get_mmio_regions(struct fsl_mc_device *mc_dev, > struct resource *regions; > struct dprc_obj_desc *obj_desc = &mc_dev->obj_desc; > struct device *parent_dev = mc_dev->dev.parent; > + enum fsl_mc_region_types mc_region_type; > + > + if (strcmp(obj_desc->type, "dprc") == 0 || > + strcmp(obj_desc->type, "dpmcp") == 0) { > + mc_region_type = FSL_MC_PORTAL; > + } else if (strcmp(obj_desc->type, "dpio") == 0) { > + mc_region_type = FSL_QBMAN_PORTAL; > + } else { > + /* > + * This function should not have been called for this MC object > + * type, as this object type is not supposed to have MMIO > + * regions > + */ > + WARN_ON(true); > + return -EINVAL; > + } > > regions = kmalloc_array(obj_desc->region_count, > sizeof(regions[0]), GFP_KERNEL); > @@ -370,14 +388,14 @@ static int fsl_mc_device_get_mmio_regions(struct fsl_mc_device *mc_dev, > goto error_cleanup_regions; > } > > - WARN_ON(region_desc.base_paddr == 0x0); > WARN_ON(region_desc.size == 0); > - error = translate_mc_addr(region_desc.base_paddr, > + error = translate_mc_addr(mc_region_type, > + region_desc.base_offset, > ®ions[i].start); > if (error < 0) { > dev_err(parent_dev, > - "Invalid MC address: %#llx (for %s.%d\'s region %d)\n", > - region_desc.base_paddr, > + "Invalid MC offset: %#llx (for %s.%d\'s region %d)\n", > + region_desc.base_offset, > obj_desc->type, obj_desc->id, i); > goto error_cleanup_regions; > } > @@ -641,6 +659,10 @@ static void mc_bus_unmask_msi_irq(struct irq_data *d) > irq_chip_unmask_parent(d); > } > > +/* > + * This function is invoked from devm_request_irq(), > + * devm_request_threaded_irq(), dev_free_irq() > + */ > static void mc_bus_msi_domain_write_msg(struct irq_data *irq_data, > struct msi_msg *msg) > { > @@ -657,6 +679,13 @@ static void mc_bus_msi_domain_write_msg(struct irq_data *irq_data, > irq_res->msi_paddr = > ((u64)msg->address_hi << 32) | msg->address_lo; > irq_res->msi_value = msg->data; > + > + /* > + * NOTE: We cannot do the actual programming of the MSI > + * in the MC, in this function, as the object-independent > + * way of programming MSIs for MC objects is not reliable > + * if objects are being added/removed dynamically. > + */ > } > } > > @@ -706,10 +735,6 @@ static int create_mc_irq_domain(struct platform_device *mc_pdev, > goto cleanup_its_of_node; > } > > - /* > - * FIXME: Enable this code when the GIC-ITS MC support patch is merged > - */ > -#ifdef GIC_ITS_MC_SUPPORT > irq_domain = msi_create_irq_domain(mc_of_node, &mc_bus_msi_domain_info, > its_domain->parent); > if (!irq_domain) { > @@ -719,9 +744,6 @@ static int create_mc_irq_domain(struct platform_device *mc_pdev, > } > > dev_dbg(&mc_pdev->dev, "Allocated MSI domain\n"); > -#else > - irq_domain = NULL; > -#endif > *new_irq_domain = irq_domain; > return 0; > We still seem to be removing GIC_ITS_MC_SUPPORT. In v1 this was a mistake. Is this intentional now? regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/