NAK: [SRU][F:linux-bluefield][PATCH v1 0/1] UBUNTU: SAUCE: mlxbf-pmc: Fix error when reading unprogrammed events

Tim Gardner tim.gardner at canonical.com
Mon Sep 19 13:20:27 UTC 2022


On 9/19/22 06:21, Shravan Ramani wrote:
>>> SRU Justification:
>>>
>>> [Impact]
>>> When the counters are not programmed to monitor any events, reading
>>> the "event<n>" file results in the following error:
>>> cat: /sys/class/hwmon/hwmon0/smmu0/event0: Invalid argument This is
>>> misleading and instead needs to indicate that the counter is currently
>>> not enabled.
>>>
>>> [Fix]
>>> * First, skip the check for whether the counter is enabled when
>>> reading the event info. If the counter is not enabled, it should
>>> return 0, which is the default value when coming out of reset.
>>> * 0 is not a valid event in most blocks as per the respective event
>>> lists, so add a "Disable" event to all the event lists.
>>>
>>> [Test Case]
>>> Without the fix, reading any "event" file right after reset when the
>>> counters are not programmed should result in the following error:
>>> cat: /sys/class/hwmon/hwmon0/smmu0/event0: Invalid argument
>>>
>>> With the fix, it should instead print:
>>> 0x0: DISABLE
>>>
>>> [Regression Potential]
>>> Can be considered minimal
>>>
>>> Shravan Kumar Ramani (1):
>>>     UBUNTU: SAUCE: mlxbf-pmc: Fix error when reading unprogrammed
>>> events
>>>
>>>    drivers/platform/mellanox/mlxbf-pmc.c | 24 ++----------------------
>>>    drivers/platform/mellanox/mlxbf-pmc.h |  6 ++++++
>>>    2 files changed, 8 insertions(+), 22 deletions(-)
>>>
>>
>> You have an unrelated version change in this patch.
> 
> I'd like to update the driver version along with this fix.
> Would you prefer a separate commit or can I just add it in the same commit message?
> 

I would prefer a separate commit. As it is these version updates are a 
bit of a mystery. Perhaps you can add some description of what the 
version number represents and why an observer should care ? Is there a 
way to determine this version from user space ?


-- 
-----------
Tim Gardner
Canonical, Inc



More information about the kernel-team mailing list