[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <148b684e-b32f-4159-8163-5e360bc36ed3@quicinc.com>
Date: Thu, 10 Apr 2025 13:44:14 +0800
From: songchai <quic_songchai@...cinc.com>
To: Mike Leach <mike.leach@...aro.org>
CC: Suzuki K Poulose <suzuki.poulose@....com>,
James Clark
<james.clark@....com>,
Alexander Shishkin
<alexander.shishkin@...ux.intel.com>,
Andy Gross <agross@...nel.org>,
Bjorn
Andersson <andersson@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof
Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, <linux-kernel@...r.kernel.org>,
<coresight@...ts.linaro.org>, <linux-arm-kernel@...ts.infradead.org>,
<linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>
Subject: Re: [PATCH v3 3/7] coresight-tgu: Add signal priority support
On 3/7/2025 8:17 PM, Mike Leach wrote:
> Hi
>
> On Thu, 27 Feb 2025 at 09:27, songchai <quic_songchai@...cinc.com> wrote:
>> From: Songwei Chai <quic_songchai@...cinc.com>
>>
>> Like circuit of a Logic analyzer, in TGU, the requirement could be
>> configured in each step and the trigger will be created once the
>> requirements are met. Add priority functionality here to sort the
>> signals into different priorities. The signal which is wanted could
>> be configured in each step's priority node, the larger number means
>> the higher priority and the signal with higher priority will be sensed
>> more preferentially.
>>
>> Signed-off-by: Songwei Chai <quic_songchai@...cinc.com>
>> Signed-off-by: songchai <quic_songchai@...cinc.com>
>> ---
>> .../testing/sysfs-bus-coresight-devices-tgu | 7 +
>> drivers/hwtracing/coresight/coresight-tgu.c | 139 ++++++++++++++++++
>> drivers/hwtracing/coresight/coresight-tgu.h | 110 ++++++++++++++
>> 3 files changed, 256 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu
>> index 741bc9fd9df5..af7332153833 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu
>> @@ -7,3 +7,10 @@ Description:
>> Accepts only one of the 2 values - 0 or 1.
>> 0 : disable TGU.
>> 1 : enable TGU.
>> +
>> +What: /sys/bus/coresight/devices/<tgu-name>/step[0:7]_priority[0:3]/reg[0:17]
>> +Date: February 2025
>> +KernelVersion 6.15
>> +Contact: Jinlong Mao (QUIC) <quic_jinlmao@...cinc.com>, Sam Chai (QUIC) <quic_songchai@...cinc.com>
>> +Description:
>> + (RW) Set/Get the sensed siganal with specific step and priority for TGU.
> sp/siganal/signal
Done.
>
>> diff --git a/drivers/hwtracing/coresight/coresight-tgu.c b/drivers/hwtracing/coresight/coresight-tgu.c
>> index da4c04ac1097..f28761619ebe 100644
>> --- a/drivers/hwtracing/coresight/coresight-tgu.c
>> +++ b/drivers/hwtracing/coresight/coresight-tgu.c
>> @@ -17,9 +17,92 @@
>>
>> DEFINE_CORESIGHT_DEVLIST(tgu_devs, "tgu");
>>
>> +static int calculate_array_location(struct tgu_drvdata *drvdata, int step_index,
>> + int operation_index, int reg_index)
>> +{
>> + int ret = -EINVAL;
>> +
>> + ret = operation_index * (drvdata->max_step) *
>> + (drvdata->max_reg) + step_index * (drvdata->max_reg)
>> + + reg_index;
>> +
>> + return ret;
>> +}
>> +
>> +static ssize_t tgu_dataset_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> + struct tgu_attribute *tgu_attr =
>> + container_of(attr, struct tgu_attribute, attr);
>> +
>> + return sysfs_emit(buf, "0x%x\n",
>> + drvdata->value_table->priority[calculate_array_location(
>> + drvdata, tgu_attr->step_index,
>> + tgu_attr->operation_index, tgu_attr->reg_num)]);
>> +
>> +}
>> +
>> +static ssize_t tgu_dataset_store(struct device *dev,
>> + struct device_attribute *attr, const char *buf,
>> + size_t size)
>> +{
>> + unsigned long val;
>> + ssize_t ret = -EINVAL;
>> +
>> + struct tgu_drvdata *tgu_drvdata = dev_get_drvdata(dev->parent);
>> + struct tgu_attribute *tgu_attr =
>> + container_of(attr, struct tgu_attribute, attr);
>> +
>> + if (kstrtoul(buf, 0, &val))
>> + return ret;
>> +
>> + guard(spinlock)(&tgu_drvdata->spinlock);
>> + tgu_drvdata->value_table->priority[calculate_array_location(
>> + tgu_drvdata, tgu_attr->step_index, tgu_attr->operation_index,
>> + tgu_attr->reg_num)] = val;
>> + ret = size;
>> +
>> + return ret;
> ret is unneeded - directly return either size or -EINVAL.
Done.
>
>> +}
>> +
>> +static umode_t tgu_node_visible(struct kobject *kobject, struct attribute *attr,
>> + int n)
>> +{
>> + struct device *dev = kobj_to_dev(kobject);
>> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> + int ret = 0;
>> +
>> + struct device_attribute *dev_attr =
>> + container_of(attr, struct device_attribute, attr);
>> + struct tgu_attribute *tgu_attr =
>> + container_of(dev_attr, struct tgu_attribute, attr);
>> +
>> + if (tgu_attr->step_index < drvdata->max_step) {
>> + ret = (tgu_attr->reg_num < drvdata->max_reg) ?
>> + attr->mode : 0;
>> + return ret;
>> + }
>> + return SYSFS_GROUP_INVISIBLE;
>> +}
> default ret as SYSFS_GROUP_INVISIBLE, and returnret at end
Done.
>
>> +
>> static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata)
>> {
>> + int i, j, k;
>> +
>> CS_UNLOCK(drvdata->base);
>> +
>> + for (i = 0; i < drvdata->max_step; i++) {
>> + for (j = 0; j < MAX_PRIORITY; j++) {
>> + for (k = 0; k < drvdata->max_reg; k++) {
>> + tgu_writel(drvdata,
>> + drvdata->value_table->priority
>> + [calculate_array_location(drvdata, i, j, k)],
>> + PRIORITY_REG_STEP(i, j, k));
>> + }
>> + }
>> + }
>> +
>> /* Enable TGU to program the triggers */
>> tgu_writel(drvdata, 1, TGU_CONTROL);
>> CS_LOCK(drvdata->base);
>> @@ -130,6 +213,38 @@ static const struct attribute_group tgu_common_grp = {
>>
>> static const struct attribute_group *tgu_attr_groups[] = {
>> &tgu_common_grp,
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 0),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 1),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 2),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 3),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 0),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 1),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 2),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 3),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 0),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 1),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 2),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 3),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 0),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 1),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 2),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 3),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 0),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 1),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 2),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 3),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 0),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 1),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 2),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 3),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 0),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 1),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 2),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 3),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 0),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 1),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 2),
>> + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 3),
>> NULL,
>> };
>>
>> @@ -164,6 +279,30 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
>>
>> spin_lock_init(&drvdata->spinlock);
>>
>> + ret = of_property_read_u32(adev->dev.of_node, "tgu-regs",
>> + &drvdata->max_reg);
>> + if (ret)
>> + return -EINVAL;
>> +
>> + ret = of_property_read_u32(adev->dev.of_node, "tgu-steps",
>> + &drvdata->max_step);
>> + if (ret)
>> + return -EINVAL;
>> +
>> + drvdata->value_table =
>> + devm_kzalloc(dev, sizeof(*drvdata->value_table), GFP_KERNEL);
>> + if (!drvdata->value_table)
>> + return -ENOMEM;
>> +
>> + drvdata->value_table->priority = devm_kzalloc(
>> + dev,
>> + MAX_PRIORITY * drvdata->max_reg * drvdata->max_step *
>> + sizeof(*(drvdata->value_table->priority)),
>> + GFP_KERNEL);
>> +
>> + if (!drvdata->value_table->priority)
>> + return -ENOMEM;
>> +
>> drvdata->enable = false;
>> desc.type = CORESIGHT_DEV_TYPE_HELPER;
>> desc.pdata = adev->dev.platform_data;
>> diff --git a/drivers/hwtracing/coresight/coresight-tgu.h b/drivers/hwtracing/coresight/coresight-tgu.h
>> index 380686f94130..6e5d465117df 100644
>> --- a/drivers/hwtracing/coresight/coresight-tgu.h
>> +++ b/drivers/hwtracing/coresight/coresight-tgu.h
>> @@ -13,6 +13,110 @@
>> #define tgu_writel(drvdata, val, off) __raw_writel((val), drvdata->base + off)
>> #define tgu_readl(drvdata, off) __raw_readl(drvdata->base + off)
>>
>> +/*
>> + * TGU configuration space Step configuration
>> + * offset table space layout
>> + * x-------------------------x$ x-------------x$
>> + * | |$ | |$
>> + * | | | reserve |$
>> + * | | | |$
>> + * |coresight management | |-------------|base+n*0x1D8+0x1F4$
>> + * | registe | |---> |prioroty[3] |$
>> + * | | | |-------------|base+n*0x1D8+0x194$
>> + * | | | |prioroty[2] |$
>> + * |-------------------------| | |-------------|base+n*0x1D8+0x134$
>> + * | | | |prioroty[1] |$
>> + * | step[7] | | |-------------|base+n*0x1D8+0xD4$
>> + * |-------------------------|->base+0x40+7*0x1D8 | |prioroty[0] |$
>> + * | | | |-------------|base+n*0x1D8+0x74$
>> + * | ... | | | condition |$
>> + * | | | | select |$
>> + * |-------------------------|->base+0x40+1*0x1D8 | |-------------|base+n*0x1D8+0x60$
>> + * | | | | condition |$
>> + * | step[0] |--------------------> | decode |$
>> + * |-------------------------|-> base+0x40 |-------------|base+n*0x1D8+0x50$
>> + * | | | |$
>> + * | Control and status space| |Timer/Counter|$
>> + * | space | | |$
>> + * x-------------------------x->base x-------------x base+n*0x1D8+0x40$
>> + *
>> + */
>> +
>> +/* Calculate compare step addresses */
>> +#define PRIORITY_REG_STEP(step, priority, reg)\
>> + (0x0074 + 0x60 * priority + 0x4 * reg + 0x1D8 * step)
>> +
> use #defines + explanation instead of arbitrary magic numbers
Done.
>
>> +#define tgu_dataset_ro(name, step_index, type, reg_num) \
>> + (&((struct tgu_attribute[]){ { \
>> + __ATTR(name, 0444, tgu_dataset_show, NULL), \
>> + step_index, \
>> + type, \
>> + reg_num, \
>> + } })[0].attr.attr)
>> +
>
> This define unused in this patch, Drop till it is used.
Done.
>
>> +#define tgu_dataset_rw(name, step_index, type, reg_num) \
>> + (&((struct tgu_attribute[]){ { \
>> + __ATTR(name, 0644, tgu_dataset_show, tgu_dataset_store), \
>> + step_index, \
>> + type, \
>> + reg_num, \
>> + } })[0].attr.attr)
>> +
>> +#define STEP_PRIORITY(step_index, reg_num, priority) \
>> + tgu_dataset_rw(reg##reg_num, step_index, TGU_PRIORITY##priority, \
>> + reg_num)
>> +
>> +#define STEP_PRIORITY_LIST(step_index, priority) \
>> + {STEP_PRIORITY(step_index, 0, priority), \
>> + STEP_PRIORITY(step_index, 1, priority), \
>> + STEP_PRIORITY(step_index, 2, priority), \
>> + STEP_PRIORITY(step_index, 3, priority), \
>> + STEP_PRIORITY(step_index, 4, priority), \
>> + STEP_PRIORITY(step_index, 5, priority), \
>> + STEP_PRIORITY(step_index, 6, priority), \
>> + STEP_PRIORITY(step_index, 7, priority), \
>> + STEP_PRIORITY(step_index, 8, priority), \
>> + STEP_PRIORITY(step_index, 9, priority), \
>> + STEP_PRIORITY(step_index, 10, priority), \
>> + STEP_PRIORITY(step_index, 11, priority), \
>> + STEP_PRIORITY(step_index, 12, priority), \
>> + STEP_PRIORITY(step_index, 13, priority), \
>> + STEP_PRIORITY(step_index, 14, priority), \
>> + STEP_PRIORITY(step_index, 15, priority), \
>> + STEP_PRIORITY(step_index, 16, priority), \
>> + STEP_PRIORITY(step_index, 17, priority), \
>> + NULL \
>> + }
>> +
>> +#define PRIORITY_ATTRIBUTE_GROUP_INIT(step, priority)\
>> + (&(const struct attribute_group){\
>> + .attrs = (struct attribute*[])STEP_PRIORITY_LIST(step, priority),\
>> + .is_visible = tgu_node_visible,\
>> + .name = "step" #step "_priority" #priority \
>> + })
>> +
>> +enum operation_index {
>> + TGU_PRIORITY0,
>> + TGU_PRIORITY1,
>> + TGU_PRIORITY2,
>> + TGU_PRIORITY3
>> +
>> +};
>> +
>> +/* Maximum priority that TGU supports */
>> +#define MAX_PRIORITY 4
>> +
>> +struct tgu_attribute {
>> + struct device_attribute attr;
>> + u32 step_index;
>> + enum operation_index operation_index;
>> + u32 reg_num;
>> +};
>> +
>> +struct value_table {
>> + unsigned int *priority;
>> +};
>> +
>> /**
>> * struct tgu_drvdata - Data structure for a TGU (Trigger Generator Unit) device
>> * @base: Memory-mapped base address of the TGU device
>> @@ -20,6 +124,9 @@
>> * @csdev: Pointer to the associated coresight device
>> * @spinlock: Spinlock for handling concurrent access
>> * @enable: Flag indicating whether the TGU device is enabled
>> + * @value_table: Store given value based on relevant parameters.
>> + * @max_reg: Maximum number of registers
>> + * @max_step: Maximum step size
>> *
>> * This structure defines the data associated with a TGU device, including its base
>> * address, device pointers, clock, spinlock for synchronization, trigger data pointers,
>> @@ -31,6 +138,9 @@ struct tgu_drvdata {
>> struct coresight_device *csdev;
>> spinlock_t spinlock;
>> bool enable;
>> + struct value_table *value_table;
>> + int max_reg;
>> + int max_step;
>> };
>>
>> #endif
> Regards
>
> Mike
>
Powered by blists - more mailing lists