[PATCH v6 20/25] coresight: cti: don't disable ect device if it's not enabled
Tingwei Zhang
tingweiz at codeaurora.org
Mon Aug 3 20:21:48 EDT 2020
Hi Mike,
On Tue, Aug 04, 2020 at 01:13:06AM +0800, Mike Leach wrote:
> Hi Tingwei,
>
> On Fri, 31 Jul 2020 at 07:42, Tingwei Zhang <tingwei at codeaurora.org> wrote:
> >
> > If associated ect device is not enabled at first place, disable
> > routine should not be called. Add ect_enabled flag to check whether
> > ect device is enabled. Fix the issue in below case. Ect device is
> > not available when associated coresight device enabled and the
> > association is established after coresight device is enabled.
> >
> > Signed-off-by: Mike Leach <mike.leach at linaro.org>
> > Signed-off-by: Tingwei Zhang <tingwei at codeaurora.org>
> > ---
> > drivers/hwtracing/coresight/coresight.c | 11 ++++++++---
> > include/linux/coresight.h | 1 +
> > 2 files changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight.c
> > b/drivers/hwtracing/coresight/coresight.c
> > index 8b55383cfcf1..83a46cf37968 100644
> > --- a/drivers/hwtracing/coresight/coresight.c
> > +++ b/drivers/hwtracing/coresight/coresight.c
> > @@ -245,13 +245,18 @@ coresight_control_assoc_ectdev(struct
> > coresight_device *csdev, bool enable)
> >
> > if (!ect_csdev)
> > return 0;
> > + if ((!ect_ops(ect_csdev)->enable) ||
> > (!ect_ops(ect_csdev)->disable))
> > + return 0;
> >
> > if (enable) {
> > - if (ect_ops(ect_csdev)->enable)
> > - ect_ret = ect_ops(ect_csdev)->enable(ect_csdev);
> > + ect_ret = ect_ops(ect_csdev)->enable(ect_csdev);
> > + if (ect_ret)
>
> Should this be:-
> if (!ect_ret)
>
> However, this problem is corrected in the next patch - so the set as
> tested overall works.
Thanks a lot for testing this series, Mike.
Sorry for this stupid mistake. I'll address it in v7.
>
> This needs fixing or it might be better to combine this patch and the
> next patch into a single patch?
> Both are required because the CTI can be loaded as a module. Both
> adjust the same block of functionality.
I separate them into two since this patch adds ect_enabled flag and
next patch add module and device reference. Kind of two things in
my opinion. I'm open for discusion and merge them into one path.
Thanks,
Tingwei
>
> Mathieu may have a view on the best solution option here.
>
> Mike
>
>
>
> > + csdev->ect_enabled = true;
> > } else {
> > - if (ect_ops(ect_csdev)->disable)
> > + if (csdev->ect_enabled) {
> > ect_ret = ect_ops(ect_csdev)->disable(ect_csdev);
> > + csdev->ect_enabled = false;
> > + }
> > }
> >
> > /* output warning if ECT enable is preventing trace operation */
> > diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> > index 3bb738f9a326..7d3c87e5b97c 100644
> > --- a/include/linux/coresight.h
> > +++ b/include/linux/coresight.h
> > @@ -208,6 +208,7 @@ struct coresight_device {
> > /* sysfs links between components */
> > int nr_links;
> > bool has_conns_grp;
> > + bool ect_enabled; /* true only if associated ect device is enabled
> > */
> > };
> >
> > /*
> > --
> > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> > a Linux Foundation Collaborative Project
> >
>
>
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK
More information about the linux-arm-kernel
mailing list