Re: [PATCH] soc: qcom: icc-bwmon: Add tracepoints in bwmon_intr_thread

[Konrad Dybcio <konrad.dybcio@linaro.org>]

On 6/19/24 15:51, Shivnandan Kumar wrote:
> Add tracepoint for tracing the measured traffic in kbps,
> up_kbps and down_kbps in bwmon. This information is valuable
> for understanding what bwmon hw measures at the system cache
> level and at the DDR level which is helpful in debugging
> bwmon behavior.
> 
> Signed-off-by: Shivnandan Kumar <quic_kshivnan@quicinc.com>
> ---
>   MAINTAINERS                        |  1 +
>   drivers/soc/qcom/icc-bwmon.c       |  7 +++--
>   drivers/soc/qcom/trace_icc-bwmon.h | 49 ++++++++++++++++++++++++++++++
>   3 files changed, 55 insertions(+), 2 deletions(-)
>   create mode 100644 drivers/soc/qcom/trace_icc-bwmon.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 242fc612fbc5..1b410c0183bb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18573,6 +18573,7 @@ M:	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>   L:	linux-arm-msm@vger.kernel.org
>   S:	Maintained
>   F:	Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml
> +F:	drivers/soc/qcom/trace_icc-bwmon.h
>   F:	drivers/soc/qcom/icc-bwmon.c
> 
>   QUALCOMM IOMMU
> diff --git a/drivers/soc/qcom/icc-bwmon.c b/drivers/soc/qcom/icc-bwmon.c
> index fb323b3364db..9b5ac1e62673 100644
> --- a/drivers/soc/qcom/icc-bwmon.c
> +++ b/drivers/soc/qcom/icc-bwmon.c
> @@ -17,6 +17,8 @@
>   #include <linux/pm_opp.h>
>   #include <linux/regmap.h>
>   #include <linux/sizes.h>
> +#define CREATE_TRACE_POINTS
> +#include "trace_icc-bwmon.h"
> 
>   /*
>    * The BWMON samples data throughput within 'sample_ms' time. With three
> @@ -645,9 +647,9 @@ static irqreturn_t bwmon_intr_thread(int irq, void *dev_id)
>   	struct icc_bwmon *bwmon = dev_id;
>   	unsigned int irq_enable = 0;
>   	struct dev_pm_opp *opp, *target_opp;
> -	unsigned int bw_kbps, up_kbps, down_kbps;
> +	unsigned int bw_kbps, up_kbps, down_kbps, meas_kbps;
> 
> -	bw_kbps = bwmon->target_kbps;
> +	meas_kbps = bw_kbps = bwmon->target_kbps;
> 
>   	target_opp = dev_pm_opp_find_bw_ceil(bwmon->dev, &bw_kbps, 0);

This breaks bwmon, as dev_pm_opp_find_bw_ceil is now fed a random
(uninitialized variable) value

Konrad