| Message ID | 20240123054608.1790189-3-anshuman.khandual@arm.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | coresight: Move remaining AMBA ACPI devices into platform driver | expand |
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 <suzuki.poulose@arm.com> > Cc: Mike Leach <mike.leach@linaro.org> > Cc: James Clark <james.clark@arm.com> > 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 <anshuman.khandual@arm.com> > --- > 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 <suzuki.poulose@arm.com> >> Cc: Mike Leach <mike.leach@linaro.org> >> Cc: James Clark <james.clark@arm.com> >> 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 <anshuman.khandual@arm.com> >> --- >> 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.
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:
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 <mike.leach@linaro.org> Cc: James Clark <james.clark@arm.com> 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 <anshuman.khandual@arm.com> --- drivers/hwtracing/coresight/coresight-priv.h | 10 ++++++++++ drivers/hwtracing/coresight/coresight-stm.c | 14 +++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-)