Messages in this thread Patch in this message | | | Date | Fri, 10 Dec 2021 10:22:20 -0700 | From | Mathieu Poirier <> | Subject | Re: [PATCH 2/3] coresight: Fail to open with return stacks if they are unavailable |
| |
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 <james.clark@arm.com> > >> --- > >> 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.
Thanks, Mathieu
> > > Otherwise, looks good to me > > > > Suzuki
| |