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 <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: James Clark <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Anshuman Khandual <[email protected]>
---
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)
+ 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;
+ 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:
--
2.25.1
On 23/01/2024 05:45, Anshuman Khandual wrote:
> 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 <[email protected]>
> Cc: Mike Leach <[email protected]>
> Cc: James Clark <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Anshuman Khandual <[email protected]>
> ---
> 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)
> + 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
> + 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:
On 2/15/24 16:25, Suzuki K Poulose wrote:
> On 23/01/2024 05:45, Anshuman Khandual wrote:
>> 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 <[email protected]>
>> Cc: Mike Leach <[email protected]>
>> Cc: James Clark <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Anshuman Khandual <[email protected]>
>> ---
>> 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)
Sure, will this change as suggested.
>
>> + 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).
Understood, will change the helper as follows
static inline void *coresight_get_uci_data_from_amba(const struct amba_id *table, u32 pid)
{
while (table->mask) {
if ((pid & table->mask) == table->id)
return coresight_get_uci_data(table);
table++;
};
return NULL;
}
and also drop both the custom masks STM_AMBA_MASK and TMC_AMBA_MASK.