Re: [PATCH v4 3/5] hwtracing: hisi_ptt: Export available filters through sysfs

Jonathan Cameron
2023-06-06T16:46:43Z

On Tue, 6 Jun 2023 22:22:42 +0800
Yicong Yang <yangyicong@huawei.com> wrote:

> From: Yicong Yang <yangyicong@hisilicon.com>
>
> The PTT can only filter the traced TLP headers by the Root Ports or the
> Requester ID of the Endpoint, which are located on the same PCIe core of
> the PTT device. The filter value used is derived from the BDF number of
> the supported Root Port or the Endpoint. It's not friendly enough for the
> users since it requires the user to be familiar enough with the platform
> and calculate the filter value manually.
>
> This patch export the available filters through sysfs. Each available
> filters is presented as an individual file with the name of the BDF
> number of the related PCIe device. The files are created under
> $(PTT PMU dir)/available_root_port_filters and
> $(PTT PMU dir)/available_requester_filters respectively. The filter
> value can be known by reading the related file.
>
> Then the users can easily know the available filters for trace and get
> the filter values without calculating.
>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>

Two minor tweaks suggested inline. With those tidied up
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
> .../ABI/testing/sysfs-devices-hisi_ptt | 50 +++++
> Documentation/trace/hisi-ptt.rst | 6 +
> drivers/hwtracing/ptt/hisi_ptt.c | 205 ++++++++++++++++++
> drivers/hwtracing/ptt/hisi_ptt.h | 14 ++
> 4 files changed, 275 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-hisi_ptt b/Documentation/ABI/testing/sysfs-devices-hisi_ptt
> index 82de6d710266..10df63568df9 100644
> --- a/Documentation/ABI/testing/sysfs-devices-hisi_ptt
> +++ b/Documentation/ABI/testing/sysfs-devices-hisi_ptt
> @@ -59,3 +59,53 @@ Description: (RW) Control the allocated buffer watermark of outbound packets.
> The available tune data is [0, 1, 2]. Writing a negative value
> will return an error, and out of range values will be converted
> to 2. The value indicates a probable level of the event.
> +
> +What: /sys/devices/hisi_ptt<sicl_id>_<core_id>/root_port_filters
> +Date: May 2023
> +KernelVersion: 6.5
> +Contact: Yicong Yang <yangyicong@hisilicon.com>
> +Description: This directory contains the files providing the PCIe Root Port filters
> + information used for PTT trace. Each file is named after the supported
> + Root Port device name <domain>:<bus>:<device>.<function>.
> +
> + See the description of the "filter" in Documentation/trace/hisi-ptt.rst
> + for more information.
> +
> +What: /sys/devices/hisi_ptt<sicl_id>_<core_id>/root_port_filters/multiselect
> +Date: May 2023
> +KernelVersion: 6.5
> +Contact: Yicong Yang <yangyicong@hisilicon.com>
> +Description: (Read) Indicates if this kind of filter can be selected at the same
> + time as others filters, or must be used on it's own.

Call out meaning of 1 and 0 for these two cases.

> +
> +static int hisi_ptt_init_filter_attributes(struct hisi_ptt *hisi_ptt)
> +{
> + struct hisi_ptt_filter_desc *filter;
> + int ret;
> +
> + mutex_lock(&hisi_ptt->filter_lock);
> +
> + /*
> + * Register the reset callback in the first stage. In reset we traverse
> + * the filters list to remove the sysfs attributes so the callback alone

the callback can be
(alone got left and shouldn't have been. I wasn't clear in previous feedback)

> + * can be called safely even without below filter attributes creation.
> + */
> + ret = devm_add_action(&hisi_ptt->pdev->dev,
> + hisi_ptt_remove_all_filter_attributes,
> + hisi_ptt);
> + if (ret)
> + goto out;
> +
> + list_for_each_entry(filter, &hisi_ptt->port_filters, list) {
> + ret = hisi_ptt_create_filter_attr(hisi_ptt, filter);
> + if (ret)
> + goto out;
> + }
> +
> + list_for_each_entry(filter, &hisi_ptt->req_filters, list) {
> + ret = hisi_ptt_create_filter_attr(hisi_ptt, filter);
> + if (ret)
> + goto out;
> + }
> +
> + hisi_ptt->sysfs_inited = true;
> +out:
> + mutex_unlock(&hisi_ptt->filter_lock);
> + return ret;
> +}