[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e84c3ab5-4064-425f-8c04-1d49f0425234@linaro.org>
Date: Thu, 20 Jun 2024 12:48:04 +0200
From: Konrad Dybcio <konrad.dybcio@...aro.org>
To: Shivnandan Kumar <quic_kshivnan@...cinc.com>,
Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Bjorn Andersson <andersson@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
quic_rgottimu@...cinc.com
Subject: Re: [PATCH] soc: qcom: icc-bwmon: Add tracepoints in
bwmon_intr_thread
On 6/20/24 10:55, Shivnandan Kumar wrote:
>
>
> On 6/20/2024 12:46 AM, Konrad Dybcio wrote:
>>
>>
>> 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@...cinc.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@...aro.org>
>>> L: linux-arm-msm@...r.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
>>
>
> Thank you for reviewing the patch.
> I didn't get it, still the variable "bw_kbps" is being initialized along with "meas_kbps". Which variable are you referring to as being fed to dev_pm_opp_find_bw_ceil with an uninitialized value?
Oh this one's on me, I skipped over the middle assignment.. Sorry!
Konrad
Powered by blists - more mailing lists