Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7E6C5C4332F for ; Mon, 13 Dec 2021 14:23:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238894AbhLMOXz (ORCPT ); Mon, 13 Dec 2021 09:23:55 -0500 Received: from foss.arm.com ([217.140.110.172]:56514 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238184AbhLMOXx (ORCPT ); Mon, 13 Dec 2021 09:23:53 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D4DFA1FB; Mon, 13 Dec 2021 06:23:51 -0800 (PST) Received: from [10.57.34.21] (unknown [10.57.34.21]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 00BE83F73B; Mon, 13 Dec 2021 06:23:49 -0800 (PST) Subject: Re: [PATCH 2/3] coresight: Fail to open with return stacks if they are unavailable To: Mathieu Poirier Cc: Suzuki K Poulose , coresight@lists.linaro.org, Mike Leach , Leo Yan , John Garry , Will Deacon , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org References: <20211208160907.749482-1-james.clark@arm.com> <20211208160907.749482-2-james.clark@arm.com> <20211210172220.GA1238770@p14s> From: James Clark Message-ID: <36c3469c-4fc4-e156-048a-47f9d001f47f@arm.com> Date: Mon, 13 Dec 2021 14:23:48 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <20211210172220.GA1238770@p14s> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/12/2021 17:22, Mathieu Poirier wrote: > Hi James, > > On Thu, Dec 09, 2021 at 11:13:55AM +0000, James Clark wrote: >> >> >> On 09/12/2021 11:00, Suzuki K Poulose wrote: >>> On 08/12/2021 16:09, James Clark wrote: >>>> Maintain consistency with the other options by failing to open when they >>>> aren't supported. For example ETM_OPT_TS, ETM_OPT_CTXTID2 and the newly >>>> added ETM_OPT_BRANCH_BROADCAST all return with -EINVAL if they are >>>> requested but not supported by hardware. >>>> >>>> The consequence of not doing this is that the user may not be >>>> aware that they are not enabling the feature as it is silently disabled. >>>> >>>> Signed-off-by: James Clark >>>> --- >>>> � drivers/hwtracing/coresight/coresight-etm4x-core.c | 13 +++++++++---- >>>> � 1 file changed, 9 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c >>>> index d2bafb50c66a..0a9bb943a5e5 100644 >>>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c >>>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c >>>> @@ -674,10 +674,15 @@ static int etm4_parse_event_config(struct coresight_device *csdev, >>>> ����� } >>>> � ����� /* return stack - enable if selected and supported */ >>>> -��� if ((attr->config & BIT(ETM_OPT_RETSTK)) && drvdata->retstack) >>>> -������� /* bit[12], Return stack enable bit */ >>>> -������� config->cfg |= BIT(12); >>>> - >>>> +��� if (attr->config & BIT(ETM_OPT_RETSTK)) { >>>> +������� if (!drvdata->retstack) { >>>> +����������� ret = -EINVAL; >>>> +����������� goto out; >>>> +������� } else { >>>> +����������� /* bit[12], Return stack enable bit */ >>>> +����������� config->cfg |= BIT(12); >>>> +������� } >>> >>> nit: While at this, please could you change the hard coded value >>> to ETM4_CFG_BIT_RETSTK ? >>> >> I started changing them all because I had trouble searching for bits by name but then >> I thought it would snowball into a bigger change so I undid it. >> >> I think I'll just go and do it now if it's an issue here. > > I can apply this set right away and you send another patch to fix all hard coded > bitfields or you can send another revision with all 4 patches included in it > (bitfields fix plus these 3). Just let me know what you want to do. And next > time please add a cover letter. I think I would like to hold off for a bit based on Mike's feedback. Seems like I may need to make a change about return stacks. Thanks James > > Thanks, > Mathieu > >> >>> Otherwise, looks good to me >>> >>> Suzuki