Re: [PATCH v3 7/8] hwtracing: hisi_ptt: Add pattern PMU config for trace format selection

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The `pattern` field in `struct hisi_ptt_trace_ctrl` is defined as a 1-bit bitfield, despite being documented and parsed as a 4-bit field.
- [Low] Typo in the newly added documentation: 'message-type-basedfield' is missing a space.
--

commit 089e32a211137f88a4d8b5c44e954565dc797fe5
Author: Sizhe Liu <liusizhe5@huawei.com>

hwtracing: hisi_ptt: Add pattern PMU config for trace format selection

This commit introduces a pattern PMU config field for controlling the TLP
header format traced by the hardware. It allows selecting between a legacy
format and a new format using a 4-bit configuration value.

> diff --git a/Documentation/trace/hisi-ptt.rst b/Documentation/trace/hisi-ptt.rst
> index f6a2655f99e57..c95eaa12f4d35 100644
> --- a/Documentation/trace/hisi-ptt.rst
> +++ b/Documentation/trace/hisi-ptt.rst
[ ... ]
> @@ -255,7 +255,28 @@ directly from the TLP header.
>       DW2 [                     Header DW2                            ]
>       DW3 [                     Header DW3                            ]
>  
> -5. Memory Management
> +5. Trace pattern
> +-----------------
> +
> +You can control whether the trace is taken with the new version of the
> +TLP header format by specifying the `pattern` parameter. The default
> +value is 0, which means the legacy format is used for backward
> +compatibility. The parameter value is 4 bit and bit[3:1] are currently
> +reserved for extension. Current supported values are shown below:
> +
> +- 4'b0000: legacy trace format
> +    DW2 and DW3 are printed with generic field names only, and
> +    no message-type-basedfield decoding.

[Severity: Low]
This isn't a bug, but there is a typo here: "message-type-basedfield" is
missing a space.

[ ... ]
> diff --git a/drivers/hwtracing/ptt/hisi_ptt.h b/drivers/hwtracing/ptt/hisi_ptt.h
> index 46030aa880811..584f65ba1e320 100644
> --- a/drivers/hwtracing/ptt/hisi_ptt.h
> +++ b/drivers/hwtracing/ptt/hisi_ptt.h
[ ... ]
> @@ -139,6 +142,7 @@ struct hisi_ptt_trace_ctrl {
>  	u32 filter:16;
>  	u32 format:1;
>  	u32 type:4;
> +	u32 pattern:1;
>  };

[Severity: Low]
Could this 1-bit bitfield inadvertently truncate the 4-bit pattern value?

The documentation and hisi_ptt_pmu_init_configs() parse the pattern as a
4-bit field, but assigning it to this 1-bit bitfield will truncate it.

If future extensions utilize the reserved bits (e.g., a value of 2 or 3),
the truncation would silently drop those bits, which could cause the
driver to mistakenly fall back to the legacy format.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903075241.1445338-1-liusizhe5@huawei.com?part=7