Re: [PATCH v2 12/14] platform/x86/intel/ifs: Add current_batch sysfs entry

From: Sohil Mehta
Date: Wed Nov 09 2022 - 18:46:23 EST


On 11/7/2022 2:53 PM, Jithu Joseph wrote:
Reviewed-by: Tony Luck <tony.luck@xxxxxxxxx>
Signed-off-by: Jithu Joseph <jithu.joseph@xxxxxxxxx>
---
drivers/platform/x86/intel/ifs/ifs.h | 23 ++++++++++----
drivers/platform/x86/intel/ifs/core.c | 1 +
drivers/platform/x86/intel/ifs/load.c | 18 +++++++----
drivers/platform/x86/intel/ifs/runtest.c | 10 ++++---
drivers/platform/x86/intel/ifs/sysfs.c | 38 ++++++++++++++++++++++++
5 files changed, 74 insertions(+), 16 deletions(-)


Reviewed-by: Sohil Mehta <sohil.mehta@xxxxxxxxx>

A Nit below.

+static ssize_t current_batch_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ifs_data *ifsd = ifs_get_data(dev);
+ int cur_batch;
+ int rc;
+
+ rc = kstrtouint(buf, 0, &cur_batch);

Should this be kstrtoint() since cur_batch is defined as int? Or maybe it might be better to define cur_batch as unsigned int to avoid the negative input?

+ if (rc < 0 || cur_batch > 0xff)
+ return -EINVAL;
+
+ if (down_interruptible(&ifs_sem))
+ return -EINTR;
+
+ ifsd->cur_batch = cur_batch;
+


Sohil