Re: [PATCH v7 02/10] Coresight: Add coresight TPDM source driver

From: Jinlong Mao
Date: Wed Jun 01 2022 - 05:21:36 EST


Hi Suzuki,

On 5/24/2022 3:00 PM, Jinlong Mao wrote:
Hi Suzuki,

Thank you for the review.

On 5/23/2022 4:57 PM, Suzuki K Poulose wrote:
Hi

On 09/05/2022 14:39, Mao Jinlong wrote:
Add driver to support Coresight device TPDM (Trace, Profiling and
Diagnostics Monitor). TPDM is a monitor to collect data from
different datasets. This change is to add probe/enable/disable
functions for tpdm source.

Signed-off-by: Tao Zhang <quic_taozha@xxxxxxxxxxx>
Signed-off-by: Mao Jinlong <quic_jinlmao@xxxxxxxxxxx>
---
  drivers/hwtracing/coresight/Kconfig          |  13 ++
  drivers/hwtracing/coresight/Makefile         |   1 +
  drivers/hwtracing/coresight/coresight-core.c |   5 +-
  drivers/hwtracing/coresight/coresight-tpdm.c | 146 +++++++++++++++++++
  drivers/hwtracing/coresight/coresight-tpdm.h |  26 ++++
  include/linux/coresight.h                    |   1 +
  6 files changed, 191 insertions(+), 1 deletion(-)
  create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.c
  create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.h


+/**
+ * struct tpdm_drvdata - specifics associated to an TPDM component
+ * @base:       memory mapped base address for this component.
+ * @dev:        The device entity associated to this component.
+ * @csdev:      component vitals needed by the framework.
+ * @lock:       lock for the enable value.
+ * @enable:     enable status of the component.
+ */
+
+struct tpdm_drvdata {
+    void __iomem        *base;
+    struct device        *dev;
+    struct coresight_device    *csdev;
+    struct mutex        lock;

Why mutex lock ? Couldn't this be a spinlock ?
1. There is no irq for TPDM
2. As there are 7 dataset types, there will be some FOR loop to configure
tpdm registers which may cause some time.

I think we can use mutex lock here. Do you have any more comments for this ?

Thanks
Jinlong Mao

+    bool            enable;
+};
+
+#endif  /* _CORESIGHT_CORESIGHT_TPDM_H */
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 247147c11231..a9efac55029d 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -61,6 +61,7 @@ enum coresight_dev_subtype_source {
      CORESIGHT_DEV_SUBTYPE_SOURCE_PROC,
      CORESIGHT_DEV_SUBTYPE_SOURCE_BUS,
      CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE,
+    CORESIGHT_DEV_SUBTYPE_SOURCE_DATA_ONLY,

super minor nit: I find the choice of name a bit odd.
We could simply make it something like :

    CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS:

Suzuki
I will check and update.

  };
    enum coresight_dev_subtype_helper {

_______________________________________________
CoreSight mailing list -- coresight@xxxxxxxxxxxxxxxx
To unsubscribe send an email to coresight-leave@xxxxxxxxxxxxxxxx
_______________________________________________
CoreSight mailing list -- coresight@xxxxxxxxxxxxxxxx
To unsubscribe send an email to coresight-leave@xxxxxxxxxxxxxxxx