From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3AC74C47076 for ; Fri, 21 May 2021 17:58:16 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AEDCE61004 for ; Fri, 21 May 2021 17:58:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AEDCE61004 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=tM7X4ZiT8OztymZt/C0NQSZtvLAk8XBcMi4WGuyLZZs=; b=mcQeiHuJgMqRkD+J9bE+6q4njZ cW+xUNgF05t4CJLtN3oa2oqBQhgpi+4fP0elv7mF2u6O7Oyma8I2VI8qQBd33sFdyl2s67Xdk/JRP kpKmsLFCq9FSw+gzGMOJcG0fL/yFxUGQkULsdPdNJDe+M9PxdSZ3utZ8k9KQ5914ERcBmOBver8VW U4SitQPhIpq1A7ECPZbP3c2dNUE2R7mJzRzYdHE4jBcCtXA2xs0aIHrN6mMUUpoMqwW9Xateecmmi EgON/gs9yce1rJkkul8fpPuEWldCHz6hDeKaK6oNj9XKmDtS6YnmNVxjI/tMzvAoqDtWBopm8+G4b EkaN2+tA==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lk9OF-000dgw-Mn; Fri, 21 May 2021 17:56:52 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lk9OA-000dfu-NC for linux-arm-kernel@desiato.infradead.org; Fri, 21 May 2021 17:56:47 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=wKVm0yH2stD7aAlIFxf23fbIeu5Nd9DF1pDEe/btboY=; b=nsRqBbpVbHZKpaoBlevd6pTN4s Oc2YAUmN4WJdSyswbzs2LCG8cgjhZDOFqllkRQpKmx11CPaEHSr065DLR8jxnU38i0Xnl99CRZN4V vZTW3N0EiFhEUknfpHK79+TE4xnRFaOQjGZ/vNW7thlkVHaQDCYlQtEbJs9XmeYUipdO4DFaVMuGv DrjjZI7QZnvOnUW9FbW3/URDV7n3KJulSRtwE1dBuGgv4j5x0YWEWTqtIj7hGx6azGEbEOoxBQfzt NZxVl/fz/sGIzFz0Rb/DrOsJ91ujfOp31Z2lShIYTNQwGwTfAIVAJ9K2/Xiec4tg5DbjgaFcb4Vhh cDMHC1SA==; Received: from mail-pj1-x102c.google.com ([2607:f8b0:4864:20::102c]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lk9O6-00HKvp-Pd for linux-arm-kernel@lists.infradead.org; Fri, 21 May 2021 17:56:45 +0000 Received: by mail-pj1-x102c.google.com with SMTP id k5so11368434pjj.1 for ; Fri, 21 May 2021 10:56:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=wKVm0yH2stD7aAlIFxf23fbIeu5Nd9DF1pDEe/btboY=; b=eohY9gHyeYUdgAJPFo4P0fM5dnPVeVum5uwdQM4PSsEgcIMEtCOgJCEhNbVOCExWGR T+Y/OGy4khP5x0RMOiZRWZo2ySx9SWO1Be5c0vn1f2+gBkVxLb4+sYd59GRhJx5cbyvJ UMbjObhMeAgd2HX/unoCOejU7QxA830DHqwOnr8PEZQCJt/6ERJYTIDKwe0D+IWfPz0n DB6nC2Rvthy8nMfCA/hQDrAeTvE2N79uOJ7kZYPhp17pT9aTesOFcH+KprLDnvxVJZ6O sL22xZAeiG5HWDbZa7MRJo795VITMGYRq63kCo8pFzimJ+EUeNdZ109F2ARn8txNlA0i 5+KQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=wKVm0yH2stD7aAlIFxf23fbIeu5Nd9DF1pDEe/btboY=; b=QMwCuIw2J24jSlAAXJ/niYeoVUHAQwNJTmAWPvnNLkQRwDUlnN45A7FjBbjilXsv5U ezX+KI03IBe5fg6ksn66urKevA1+31wqxZljWG9ruFZHXjJkB7mfu7WXsxopuII1r9wD INocI55+KVKWr5u5dT8zMnFLLcgJjRcvFzli2oLLuhz1QzxgIfMDZYQYT+PA5auD/nyb YQhNkku16VIklyJWPOuTnACNAXZTFLpzKXJHsyLs8YKiDiRFWb7ZBUWHHwQK6LkVgHpD 82MVSRiXFA/1R9sYfzrdt/4CHMnsDmH7MqoPoXccMr3aFEhT8TJ4rL9axdb6Y93qq5pk 411A== X-Gm-Message-State: AOAM530dbTlNc9N0rHpz53PbYTk1p7RjXzXIoCLHId1nIaqekeBoCe1x PoCFq1DPNaoiqWeyZ1HM8CdvSA== X-Google-Smtp-Source: ABdhPJybNLFFLrNckIIubWC1EaDeN3/qwad3r/xRC5M1peZmZygfWTlG9SXM5vJZJMYKXgDkcVz5Gg== X-Received: by 2002:a17:90a:460d:: with SMTP id w13mr12006961pjg.35.1621619801346; Fri, 21 May 2021 10:56:41 -0700 (PDT) Received: from xps15 (S0106889e681aac74.cg.shawcable.net. [68.147.0.187]) by smtp.gmail.com with ESMTPSA id k38sm4832318pgi.73.2021.05.21.10.56.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 May 2021 10:56:40 -0700 (PDT) Date: Fri, 21 May 2021 11:56:38 -0600 From: Mathieu Poirier To: Mike Leach Cc: coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, suzuki.poulose@arm.com, leo.yan@linaro.org Subject: Re: [RFC PATCH 5/8] coresight: syscfg: Add API to check and validate device resources. Message-ID: <20210521175638.GB905529@xps15> References: <20210512211752.4103-1-mike.leach@linaro.org> <20210512211752.4103-6-mike.leach@linaro.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210512211752.4103-6-mike.leach@linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210521_105642_935749_739325BC X-CRM114-Status: GOOD ( 52.39 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Mike, On Wed, May 12, 2021 at 10:17:49PM +0100, Mike Leach wrote: > Adds in API to allow features which define device resources, to check > resource availability on load, and allocate on enable. > > 1) Check on load ensures that the resources required by the feature do > not exceed the maximum avialable on the device. > > 2) Allocate on enable ensures that sufficient unused resources are > available to satifsfy the feature on enable. Allocate can also be > used to resolve any resource selection requirements - i.e. select > an available resource from a pool of resources. > > Signed-off-by: Mike Leach > --- > .../hwtracing/coresight/coresight-config.c | 71 +++++++++++++++--- > .../hwtracing/coresight/coresight-config.h | 36 +++++++++- > .../hwtracing/coresight/coresight-syscfg.c | 72 +++++++++++++++++-- > 3 files changed, 163 insertions(+), 16 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-config.c b/drivers/hwtracing/coresight/coresight-config.c > index 3c501e027bc0..fdfda1975188 100644 > --- a/drivers/hwtracing/coresight/coresight-config.c > +++ b/drivers/hwtracing/coresight/coresight-config.c > @@ -23,6 +23,10 @@ static void cscfg_set_reg(struct cscfg_regval_csdev *reg_csdev) > u32 *p_val32 = (u32 *)reg_csdev->driver_regval; > u32 tmp32 = reg_csdev->reg_desc.val32; > > + /* resource mapped registers may have custom handling in the device */ > + if (!reg_csdev->driver_regval) > + return; > + I am curious as to why it is needed now and wasn't in the patchset that introduced the complex configuration feature. > if (reg_csdev->reg_desc.type & CS_CFG_REG_TYPE_VAL_64BIT) { > *((u64 *)reg_csdev->driver_regval) = reg_csdev->reg_desc.val64; > return; > @@ -43,6 +47,8 @@ static void cscfg_save_reg(struct cscfg_regval_csdev *reg_csdev) > { > if (!(reg_csdev->reg_desc.type & CS_CFG_REG_TYPE_VAL_SAVE)) > return; > + if (!reg_csdev->driver_regval) > + return; > if (reg_csdev->reg_desc.type & CS_CFG_REG_TYPE_VAL_64BIT) > reg_csdev->reg_desc.val64 = *(u64 *)(reg_csdev->driver_regval); > else > @@ -73,15 +79,20 @@ static void cscfg_init_reg_param(struct cscfg_feature_csdev *feat_csdev, > /* set values into the driver locations referenced in cscfg_reg_csdev */ > static int cscfg_set_on_enable(struct cscfg_feature_csdev *feat_csdev) > { > - int i; > + int i, err = 0; > > spin_lock(feat_csdev->drv_spinlock); > for (i = 0; i < feat_csdev->nr_regs; i++) > cscfg_set_reg(&feat_csdev->regs_csdev[i]); > spin_unlock(feat_csdev->drv_spinlock); > + > + if (feat_csdev->feat_ops->set_on_enable) > + err = feat_csdev->feat_ops->set_on_enable(feat_csdev); > + > dev_dbg(&feat_csdev->csdev->dev, "Feature %s: %s", > - feat_csdev->feat_desc->name, "set on enable"); > - return 0; > + feat_csdev->feat_desc->name, > + err ? "error on enable" : "set on enable"); > + return err; > } > > /* copy back values from the driver locations referenced in cscfg_reg_csdev */ > @@ -89,6 +100,9 @@ static void cscfg_save_on_disable(struct cscfg_feature_csdev *feat_csdev) > { > int i; > > + if (feat_csdev->feat_ops->clear_on_disable) > + feat_csdev->feat_ops->clear_on_disable(feat_csdev); > + > spin_lock(feat_csdev->drv_spinlock); > for (i = 0; i < feat_csdev->nr_regs; i++) > cscfg_save_reg(&feat_csdev->regs_csdev[i]); > @@ -217,6 +231,37 @@ static int cscfg_update_curr_params(struct cscfg_config_csdev *config_csdev) > return 0; > } > > +static void cscfg_clear_res_config(struct cscfg_config_csdev *config_csdev) > +{ > + int i; > + struct cscfg_feature_csdev *feat_csdev; > + > + for (i = 0; i < config_csdev->nr_feat; i++) { > + feat_csdev = config_csdev->feats_csdev[i]; > + if (feat_csdev->feat_ops->clear_feat_res) > + feat_csdev->feat_ops->clear_feat_res(feat_csdev); > + } > +} > + > +#define FEAT_ALLOC_RES_FMT "coresight-syscfg: Insufficient resource to enable feature %s\n" > +static int cscfg_alloc_res_config(struct cscfg_config_csdev *config_csdev) > +{ > + int err = 0, i; > + struct cscfg_feature_csdev *feat_csdev; > + > + for (i = 0; i < config_csdev->nr_feat; i++) { > + feat_csdev = config_csdev->feats_csdev[i]; > + if (feat_csdev->feat_ops->alloc_feat_res) > + err = feat_csdev->feat_ops->alloc_feat_res(feat_csdev); > + if (err) { > + pr_warn(FEAT_ALLOC_RES_FMT, feat_csdev->feat_desc->name); > + cscfg_clear_res_config(config_csdev); > + break; > + } > + } > + return err; > +} > + > /* > * Configuration values will be programmed into the driver locations if enabling, or read > * from relevant locations on disable. > @@ -227,19 +272,29 @@ static int cscfg_prog_config(struct cscfg_config_csdev *config_csdev, bool enabl > struct cscfg_feature_csdev *feat_csdev; > struct coresight_device *csdev; > > + /* check we have resources to enable this */ > + if (enable) { > + err = cscfg_alloc_res_config(config_csdev); > + if (err) > + return err; > + } else > + cscfg_clear_res_config(config_csdev); > + > for (i = 0; i < config_csdev->nr_feat; i++) { > feat_csdev = config_csdev->feats_csdev[i]; > csdev = feat_csdev->csdev; > dev_dbg(&csdev->dev, "cfg %s; %s feature:%s", config_csdev->config_desc->name, > enable ? "enable" : "disable", feat_csdev->feat_desc->name); > > - if (enable) > + if (enable) { > err = cscfg_set_on_enable(feat_csdev); > - else > + if (err) { > + /* failed to enable - release any resources */ > + cscfg_clear_res_config(config_csdev); > + break; > + } > + } else > cscfg_save_on_disable(feat_csdev); > - > - if (err) > - break; Is all of the above related to what this patchset is providing or an enhancement of the original feature? > } > return err; > } > diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h > index 9bd44b940add..0e46832df993 100644 > --- a/drivers/hwtracing/coresight/coresight-config.h > +++ b/drivers/hwtracing/coresight/coresight-config.h > @@ -199,6 +199,8 @@ struct cscfg_parameter_csdev { > * @params_csdev: current parameter values on this device > * @nr_regs: number of registers to be programmed. > * @regs_csdev: Programming details for the registers > + * @res_used: Device specific context for resources used on enable. > + * @feat_ops: Feature operations to support alloc/release of resources. > */ > struct cscfg_feature_csdev { > const struct cscfg_feature_desc *feat_desc; > @@ -209,6 +211,8 @@ struct cscfg_feature_csdev { > struct cscfg_parameter_csdev *params_csdev; > int nr_regs; > struct cscfg_regval_csdev *regs_csdev; > + void *res_used; This isn't used in this patch. Please move it to the patch that will do so. > + struct cscfg_csdev_feat_ops *feat_ops; > }; > > /** > @@ -238,14 +242,44 @@ struct cscfg_config_csdev { > * Coresight device operations. > * > * Registered coresight devices provide these operations to manage feature > - * instances compatible with the device hardware and drivers > + * instances compatible with the device hardware and drivers. > + * > + * The @check_feat_res function can be used at load time to see if the device has > + * sufficient resources to support the feature. This takes into account all resources > + * on the device. e.g. if the feature requires three counters, and the device has a s/device./device, > + * total of two implemented counters, this will return a -ENODEV error. > + * > + * The @alloc_feat_res function checks if there are sufficient unallocated resources > + * left to enable the feature. This allows for multiple features being loaded that > + * may compete for resources and also selects and allocates resources at enable time. This is the second time so I'm wondering if this is an accepted way of writing things in english. > + * e.g. if a feature requires two comparators, and there is only one left unallocated, > + * then this will return a -ENOSPC error. > * > * @load_feat: Pass a feature descriptor into the device and create the > * loaded feature instance (struct cscfg_feature_csdev). > + * @check_feat_res: Check that the coresight device has sufficient resources to > + * load the feature. Optional function. Return -ENODEV if this device > + * does not have enough resources. Called before @load_feat for feature > + * to ensure all feature can be installed on device. > + * @alloc_feat_res: Allocate release feature resources when enabling a feature on the device > + * . Optional function. Returns -ENOSPC if the device has not enough available The '.' at the beginning of the line looks wierd. > + * resources to enable the feature. Called before registers programmed. > + * @clear_feat_res: Release allocated resources. Must be implemented if @alloc_feat_res is > + * implemented. > + * @set_on_enable : Optional programming specific to device. Called immediately after > + * generic register programming operation. > + * @clear_on_disable: Optional programming specific to device. Called before generic register > + * save operation. > */ > struct cscfg_csdev_feat_ops { > int (*load_feat)(struct coresight_device *csdev, > struct cscfg_feature_csdev *feat_csdev); > + int (*check_feat_res)(struct coresight_device *csdev, > + struct cscfg_feature_desc *feat_desc); > + int (*alloc_feat_res)(struct cscfg_feature_csdev *feat_csdev); > + void (*clear_feat_res)(struct cscfg_feature_csdev *feat_csdev); > + int (*set_on_enable)(struct cscfg_feature_csdev *feat_csdev); > + void (*clear_on_disable)(struct cscfg_feature_csdev *feat_csdev); > }; > > /* coresight config helper functions*/ > diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c > index ab74e33b892b..984459c8f168 100644 > --- a/drivers/hwtracing/coresight/coresight-syscfg.c > +++ b/drivers/hwtracing/coresight/coresight-syscfg.c > @@ -121,7 +121,8 @@ static int cscfg_add_cfg_to_csdevs(struct cscfg_config_desc *config_desc) > * memory allocated using the csdev->dev object using devm managed allocator. > */ > static struct cscfg_feature_csdev * > -cscfg_alloc_csdev_feat(struct coresight_device *csdev, struct cscfg_feature_desc *feat_desc) > +cscfg_alloc_csdev_feat(struct coresight_device *csdev, struct cscfg_feature_desc *feat_desc, > + struct cscfg_csdev_feat_ops *ops) > { > struct cscfg_feature_csdev *feat_csdev = NULL; > struct device *dev = csdev->dev.parent; > @@ -165,9 +166,16 @@ cscfg_alloc_csdev_feat(struct coresight_device *csdev, struct cscfg_feature_desc > if (!feat_csdev->regs_csdev) > return NULL; > > + /* copy the static register info */ > + for (i = 0; i < feat_desc->nr_regs; i++) { > + memcpy(&feat_csdev->regs_csdev[i].reg_desc, > + &feat_desc->regs_desc[i], sizeof(struct cscfg_regval_desc)); > + } > + I really spend a lot of time on this snippet, especially since it duplicates some of the work done in cscfg_reset_feat() and that we now have two places where things get initialised. > /* load the feature default values */ > feat_csdev->feat_desc = feat_desc; > feat_csdev->csdev = csdev; > + feat_csdev->feat_ops = ops; > > return feat_csdev; > } > @@ -180,10 +188,7 @@ static int cscfg_load_feat_csdev(struct coresight_device *csdev, > struct cscfg_feature_csdev *feat_csdev; > int err; > > - if (!ops->load_feat) > - return -EINVAL; > - > - feat_csdev = cscfg_alloc_csdev_feat(csdev, feat_desc); > + feat_csdev = cscfg_alloc_csdev_feat(csdev, feat_desc, ops); > if (!feat_csdev) > return -ENOMEM; > > @@ -201,6 +206,57 @@ static int cscfg_load_feat_csdev(struct coresight_device *csdev, > return 0; > } > > +#define WARN_RES_FMT "coresight-syscfg: Insufficient resources to load feature %s in device %s\n" > +#define PARAM_ERR_FMT "coresight-syscfg: Cannot load feature %s. Invalid parameter index\n" > + > +/* > + * Check if device supports resource check function, and check for sufficient resources > + * to load feature onto device. Validate parameter references. > + * > + * Load feature if sufficient resources & valid parameter references. > + */ > +static int cscfg_check_feat_res_load(struct coresight_device *csdev, > + struct cscfg_feature_desc *feat_desc, > + struct cscfg_csdev_feat_ops *ops) > +{ > + int err = 0, i; > + > + /* if we cannot load - fail early */ > + if (!ops->load_feat) > + return -EINVAL; > + > + /* ensure matched resource ops */ > + if ((ops->alloc_feat_res && !ops->clear_feat_res) || > + (!ops->alloc_feat_res && ops->clear_feat_res)) > + return -EINVAL; > + > + /* ensure that the coresight device can support the feature */ > + if (ops->check_feat_res) { > + err = ops->check_feat_res(csdev, feat_desc); > + if (err == -ENODEV) { > + /* treat as mismatch, warn and continue */ > + pr_warn(WARN_RES_FMT, feat_desc->name, > + dev_name(&csdev->dev)); > + return 0; > + } > + } > + > + /* check parameter indexes are valid */ > + for (i = 0; i < feat_desc->nr_regs; i++) { > + if (feat_desc->regs_desc[i].type & CS_CFG_REG_TYPE_VAL_PARAM) { > + if (feat_desc->regs_desc[i].param_idx >= feat_desc->nr_params) { > + pr_err(PARAM_ERR_FMT, feat_desc->name); > + return -EINVAL; > + } > + } > + } > + > + /* sufficient resources - load if no other error */ > + if (!err) > + err = cscfg_load_feat_csdev(csdev, feat_desc, ops); > + return err; > +} > + > /* > * Add feature to any matching devices - call with mutex locked. > * Iterates through devices - any device that matches the feature will be > @@ -213,7 +269,9 @@ static int cscfg_add_feat_to_csdevs(struct cscfg_feature_desc *feat_desc) > > list_for_each_entry(csdev_item, &cscfg_mgr->csdev_desc_list, item) { > if (csdev_item->match_flags & feat_desc->match_flags) { > - err = cscfg_load_feat_csdev(csdev_item->csdev, feat_desc, &csdev_item->ops); > + /* check sufficient resources and load feature */ > + err = cscfg_check_feat_res_load(csdev_item->csdev, feat_desc, > + &csdev_item->ops); Checking the resources and loading them are two different operations and as such we should keep them separate. It will also make it easier to understand and review this set. I will come back to all the operations introduced by struct cscfg_csdev_feat_ops once I have a better understanding of what they do. More comments to come next week. Mathieu > if (err) > return err; > } > @@ -616,7 +674,7 @@ static int cscfg_add_feats_csdev(struct coresight_device *csdev, > > list_for_each_entry(feat_desc, &cscfg_mgr->feat_desc_list, item) { > if (feat_desc->match_flags & match_flags) { > - err = cscfg_load_feat_csdev(csdev, feat_desc, ops); > + err = cscfg_check_feat_res_load(csdev, feat_desc, ops); > if (err) > break; > } > -- > 2.17.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel