lkml.org 
[lkml]   [2024]   [Feb]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    SubjectRe: [PATCH V4 02/11] coresight: stm: Extract device name from AMBA pid based table lookup
    From
    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:


    \
     
     \ /
      Last update: 2024-05-27 15:04    [W:9.506 / U:0.164 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site