Thread (25 messages) 25 messages, 3 authors, 2024-02-22

Re: [PATCH V4 02/11] coresight: stm: Extract device name from AMBA pid based table lookup

From: Suzuki K Poulose <suzuki.poulose@arm.com>
Date: 2024-02-15 10:55:35
Also in: linux-acpi, lkml

On 23/01/2024 05:45, Anshuman Khandual wrote:
quoted hunk ↗ jump to hunk
Instead of using AMBA private data field, extract the device name from AMBA
pid based table lookup using new coresight_get_uci_data_from_amba() helper.

Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mike Leach <redacted>
Cc: James Clark <redacted>
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
Signed-off-by: Anshuman Khandual <redacted>
---
  drivers/hwtracing/coresight/coresight-priv.h | 10 ++++++++++
  drivers/hwtracing/coresight/coresight-stm.c  | 14 +++++++++++++-
  2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 767076e07970..68cbb036cec8 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -221,6 +221,16 @@ static inline void *coresight_get_uci_data(const struct amba_id *id)
  	return uci_id->data;
  }
  
+static inline void *coresight_get_uci_data_from_amba(const struct amba_id *table, u32 pid)
+{
+	while (table->mask) {
+		if ((table->id & table->mask) == pid)
Why are we masking table->id ? table->id is a static value that the
driver wants to check for "variants" of a given device. The table->mask
is there to filter out the "irrelevant" bits of the PID that we read
from the device. So this should instead be:

		if ((table->mask & pid) == table->id)
quoted hunk ↗ jump to hunk
+			return coresight_get_uci_data(table);
+		table++;
+	};
+	return NULL;
+}
+
  void coresight_release_platform_data(struct coresight_device *csdev,
  				     struct device *dev,
  				     struct coresight_platform_data *pdata);
diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
index a1c27c901ad1..9cdca4f86cab 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -804,6 +804,18 @@ static void stm_init_generic_data(struct stm_drvdata *drvdata,
  	drvdata->stm.set_options = stm_generic_set_options;
  }
  
+#define STM_AMBA_MASK 0xfffff
+
+static const struct amba_id stm_ids[];
+
+static char *stm_csdev_name(struct coresight_device *csdev)
+{
+	u32 stm_pid = coresight_get_pid(&csdev->access) & STM_AMBA_MASK;
Similar to above:

Why do we apply a "custom" mask to the PID and later check the PID with
that of the table->pid.

The way it is supposed work is :

	(table->mask & dev_pid) == table->pid

the table->mask is there for a reason: i.e., to get the relevant bits 
from the device_pid and compare it against "the" expected value 
(table->pid).


Suzuki
quoted hunk ↗ jump to hunk
+	void *uci_data = coresight_get_uci_data_from_amba(stm_ids, stm_pid);
+
+	return uci_data ? (char *)uci_data : "STM";
+}
+
  static int stm_probe(struct amba_device *adev, const struct amba_id *id)
  {
  	int ret, trace_id;
@@ -900,7 +912,7 @@ static int stm_probe(struct amba_device *adev, const struct amba_id *id)
  	pm_runtime_put(&adev->dev);
  
  	dev_info(&drvdata->csdev->dev, "%s initialized\n",
-		 (char *)coresight_get_uci_data(id));
+		 stm_csdev_name(drvdata->csdev));
  	return 0;
  
  cs_unregister:

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help