[PATCH] staging: greybus: Multiple cleanups and refactors

gpittala posted 1 patch 8 months, 3 weeks ago
.../greybus/Documentation/firmware/firmware.c |  32 ++--
drivers/staging/greybus/arche-apb-ctrl.c      |  11 +-
drivers/staging/greybus/arche-platform.c      |  11 +-
drivers/staging/greybus/audio_gb.c            |  37 +++-
.../staging/greybus/audio_manager_module.c    |  13 +-
drivers/staging/greybus/gbphy.c               |   3 +-
drivers/staging/greybus/light.c               |   5 +-
drivers/staging/greybus/loopback.c            | 170 ++++++++++--------
8 files changed, 159 insertions(+), 123 deletions(-)
[PATCH] staging: greybus: Multiple cleanups and refactors
Posted by gpittala 8 months, 3 weeks ago
This patch includes multiple meaningful cleanups for the Greybus staging driver:

1. firmware.c: Replaced deprecated 'strncpy()' with 'strscpy()'
2. sysfs show functions: Replaced 'sprintf()' with 'sysfs_emit()'
3. loopback.c: Refactored a large function (gp_loopback_fn) to improve readability
4. audio_gb.c: Split logic in get_topology() into separate calls as per TODO

All changes are tested and pass checkpatch.pl

Signed-off-by: gpittala <ganeshkpittala@gmail.com>
---
 .../greybus/Documentation/firmware/firmware.c |  32 ++--
 drivers/staging/greybus/arche-apb-ctrl.c      |  11 +-
 drivers/staging/greybus/arche-platform.c      |  11 +-
 drivers/staging/greybus/audio_gb.c            |  37 +++-
 .../staging/greybus/audio_manager_module.c    |  13 +-
 drivers/staging/greybus/gbphy.c               |   3 +-
 drivers/staging/greybus/light.c               |   5 +-
 drivers/staging/greybus/loopback.c            | 170 ++++++++++--------
 8 files changed, 159 insertions(+), 123 deletions(-)

diff --git a/drivers/staging/greybus/Documentation/firmware/firmware.c b/drivers/staging/greybus/Documentation/firmware/firmware.c
index 765d69faa9cc..8e375c88c881 100644
--- a/drivers/staging/greybus/Documentation/firmware/firmware.c
+++ b/drivers/staging/greybus/Documentation/firmware/firmware.c
@@ -47,12 +47,12 @@ static int update_intf_firmware(int fd)
 	ret = ioctl(fd, FW_MGMT_IOC_GET_INTF_FW, &intf_fw_info);
 	if (ret < 0) {
 		printf("Failed to get interface firmware version: %s (%d)\n",
-			fwdev, ret);
+		       fwdev, ret);
 		return -1;
 	}
 
 	printf("Interface Firmware tag (%s), major (%d), minor (%d)\n",
-		intf_fw_info.firmware_tag, intf_fw_info.major,
+	       intf_fw_info.firmware_tag, intf_fw_info.major,
 		intf_fw_info.minor);
 
 	/* Try Interface Firmware load over Unipro */
@@ -63,25 +63,25 @@ static int update_intf_firmware(int fd)
 	intf_load.major = 0;
 	intf_load.minor = 0;
 
-	strncpy((char *)&intf_load.firmware_tag, firmware_tag,
+	strscpy((char *)&intf_load.firmware_tag, firmware_tag,
 		GB_FIRMWARE_U_TAG_MAX_SIZE);
 
 	ret = ioctl(fd, FW_MGMT_IOC_INTF_LOAD_AND_VALIDATE, &intf_load);
 	if (ret < 0) {
 		printf("Failed to load interface firmware: %s (%d)\n", fwdev,
-			ret);
+		       ret);
 		return -1;
 	}
 
 	if (intf_load.status != GB_FW_U_LOAD_STATUS_VALIDATED &&
 	    intf_load.status != GB_FW_U_LOAD_STATUS_UNVALIDATED) {
 		printf("Load status says loading failed: %d\n",
-			intf_load.status);
+		       intf_load.status);
 		return -1;
 	}
 
 	printf("Interface Firmware (%s) Load done: major: %d, minor: %d, status: %d\n",
-		firmware_tag, intf_load.major, intf_load.minor,
+	       firmware_tag, intf_load.major, intf_load.minor,
 		intf_load.status);
 
 	/* Initiate Mode-switch to the newly loaded firmware */
@@ -101,35 +101,35 @@ static int update_backend_firmware(int fd)
 	/* Get Backend Firmware Version */
 	printf("Getting Backend Firmware Version\n");
 
-	strncpy((char *)&backend_fw_info.firmware_tag, firmware_tag,
+	strscpy((char *)&backend_fw_info.firmware_tag, firmware_tag,
 		GB_FIRMWARE_U_TAG_MAX_SIZE);
 
 retry_fw_version:
 	ret = ioctl(fd, FW_MGMT_IOC_GET_BACKEND_FW, &backend_fw_info);
 	if (ret < 0) {
 		printf("Failed to get backend firmware version: %s (%d)\n",
-			fwdev, ret);
+		       fwdev, ret);
 		return -1;
 	}
 
 	printf("Backend Firmware tag (%s), major (%d), minor (%d), status (%d)\n",
-		backend_fw_info.firmware_tag, backend_fw_info.major,
+	       backend_fw_info.firmware_tag, backend_fw_info.major,
 		backend_fw_info.minor, backend_fw_info.status);
 
 	if (backend_fw_info.status == GB_FW_U_BACKEND_VERSION_STATUS_RETRY)
 		goto retry_fw_version;
 
-	if ((backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_SUCCESS)
-	    && (backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_NOT_AVAILABLE)) {
+	if ((backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_SUCCESS) &&
+	    (backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_NOT_AVAILABLE)) {
 		printf("Failed to get backend firmware version: %s (%d)\n",
-			fwdev, backend_fw_info.status);
+		       fwdev, backend_fw_info.status);
 		return -1;
 	}
 
 	/* Try Backend Firmware Update over Unipro */
 	printf("Updating Backend Firmware\n");
 
-	strncpy((char *)&backend_update.firmware_tag, firmware_tag,
+	strscpy((char *)&backend_update.firmware_tag, firmware_tag,
 		GB_FIRMWARE_U_TAG_MAX_SIZE);
 
 retry_fw_update:
@@ -148,10 +148,10 @@ static int update_backend_firmware(int fd)
 
 	if (backend_update.status != GB_FW_U_BACKEND_FW_STATUS_SUCCESS) {
 		printf("Load status says loading failed: %d\n",
-			backend_update.status);
+		       backend_update.status);
 	} else {
 		printf("Backend Firmware (%s) Load done: status: %d\n",
-				firmware_tag, backend_update.status);
+		       firmware_tag, backend_update.status);
 	}
 
 	return 0;
@@ -185,7 +185,7 @@ int main(int argc, char *argv[])
 		fw_timeout = strtoul(argv[4], &endptr, 10);
 
 	printf("Trying Firmware update: fwdev: %s, type: %s, tag: %s, timeout: %u\n",
-		fwdev, fw_update_type == 0 ? "interface" : "backend",
+	       fwdev, fw_update_type == 0 ? "interface" : "backend",
 		firmware_tag, fw_timeout);
 
 	printf("Opening %s firmware management device\n", fwdev);
diff --git a/drivers/staging/greybus/arche-apb-ctrl.c b/drivers/staging/greybus/arche-apb-ctrl.c
index 90ab32638d3f..9862188e8367 100644
--- a/drivers/staging/greybus/arche-apb-ctrl.c
+++ b/drivers/staging/greybus/arche-apb-ctrl.c
@@ -17,6 +17,7 @@
 #include <linux/pm.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spinlock.h>
+#include <linux/sysfs.h>
 #include "arche_platform.h"
 
 static void apb_bootret_deassert(struct device *dev);
@@ -299,16 +300,16 @@ static ssize_t state_show(struct device *dev,
 
 	switch (apb->state) {
 	case ARCHE_PLATFORM_STATE_OFF:
-		return sprintf(buf, "off%s\n",
+		return sysfs_emit(buf, "off%s\n",
 				apb->init_disabled ? ",disabled" : "");
 	case ARCHE_PLATFORM_STATE_ACTIVE:
-		return sprintf(buf, "active\n");
+		return sysfs_emit(buf, "active\n");
 	case ARCHE_PLATFORM_STATE_STANDBY:
-		return sprintf(buf, "standby\n");
+		return sysfs_emit(buf, "standby\n");
 	case ARCHE_PLATFORM_STATE_FW_FLASHING:
-		return sprintf(buf, "fw_flashing\n");
+		return sysfs_emit(buf, "fw_flashing\n");
 	default:
-		return sprintf(buf, "unknown state\n");
+		return sysfs_emit(buf, "unknown state\n");
 	}
 }
 
diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c
index d48464390f58..2e706c1169d5 100644
--- a/drivers/staging/greybus/arche-platform.c
+++ b/drivers/staging/greybus/arche-platform.c
@@ -21,6 +21,7 @@
 #include <linux/time.h>
 #include <linux/greybus.h>
 #include <linux/of.h>
+#include <linux/sysfs.h>
 #include "arche_platform.h"
 
 #if IS_ENABLED(CONFIG_USB_HSIC_USB3613)
@@ -374,15 +375,15 @@ static ssize_t state_show(struct device *dev,
 
 	switch (arche_pdata->state) {
 	case ARCHE_PLATFORM_STATE_OFF:
-		return sprintf(buf, "off\n");
+		return sysfs_emit(buf, "off\n");
 	case ARCHE_PLATFORM_STATE_ACTIVE:
-		return sprintf(buf, "active\n");
+		return sysfs_emit(buf, "active\n");
 	case ARCHE_PLATFORM_STATE_STANDBY:
-		return sprintf(buf, "standby\n");
+		return sysfs_emit(buf, "standby\n");
 	case ARCHE_PLATFORM_STATE_FW_FLASHING:
-		return sprintf(buf, "fw_flashing\n");
+		return sysfs_emit(buf, "fw_flashing\n");
 	default:
-		return sprintf(buf, "unknown state\n");
+		return sysfs_emit(buf, "unknown state\n");
 	}
 }
 
diff --git a/drivers/staging/greybus/audio_gb.c b/drivers/staging/greybus/audio_gb.c
index 9d8994fdb41a..c7f8df7b4cbe 100644
--- a/drivers/staging/greybus/audio_gb.c
+++ b/drivers/staging/greybus/audio_gb.c
@@ -8,21 +8,28 @@
 #include <linux/greybus.h>
 #include "audio_codec.h"
 
-/* TODO: Split into separate calls */
-int gb_audio_gb_get_topology(struct gb_connection *connection,
-			     struct gb_audio_topology **topology)
+static int gb_audio_gb_get_topology_size(struct gb_connection *connection,
+					 u16 *size)
 {
-	struct gb_audio_get_topology_size_response size_resp;
-	struct gb_audio_topology *topo;
-	u16 size;
+	struct gb_audio_get_topology_size_response resp;
 	int ret;
 
 	ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY_SIZE,
-				NULL, 0, &size_resp, sizeof(size_resp));
+				NULL, 0, &resp, sizeof(resp));
 	if (ret)
 		return ret;
 
-	size = le16_to_cpu(size_resp.size);
+	*size = le16_to_cpu(resp.size);
+	return 0;
+}
+
+static int gb_audio_gb_read_topology(struct gb_connection *connection,
+				     struct gb_audio_topology **topology,
+				     u16 size)
+{
+	struct gb_audio_topology *topo;
+	int ret;
+
 	if (size < sizeof(*topo))
 		return -ENODATA;
 
@@ -38,9 +45,21 @@ int gb_audio_gb_get_topology(struct gb_connection *connection,
 	}
 
 	*topology = topo;
-
 	return 0;
 }
+
+int gb_audio_gb_get_topology(struct gb_connection *connection,
+			     struct gb_audio_topology **topology)
+{
+	u16 size;
+	int ret;
+
+	ret = gb_audio_gb_get_topology_size(connection, &size);
+	if (ret)
+		return ret;
+
+	return gb_audio_gb_read_topology(connection, topology, size);
+}
 EXPORT_SYMBOL_GPL(gb_audio_gb_get_topology);
 
 int gb_audio_gb_get_control(struct gb_connection *connection,
diff --git a/drivers/staging/greybus/audio_manager_module.c b/drivers/staging/greybus/audio_manager_module.c
index 4a4dfb42f50f..781144be4eec 100644
--- a/drivers/staging/greybus/audio_manager_module.c
+++ b/drivers/staging/greybus/audio_manager_module.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/sysfs.h>
 
 #include "audio_manager.h"
 #include "audio_manager_private.h"
@@ -76,7 +77,7 @@ static void gb_audio_module_release(struct kobject *kobj)
 static ssize_t gb_audio_module_name_show(struct gb_audio_manager_module *module,
 					 struct gb_audio_manager_module_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%s", module->desc.name);
+	return sysfs_emit(buf, "%s\n", module->desc.name);
 }
 
 static struct gb_audio_manager_module_attribute gb_audio_module_name_attribute =
@@ -85,7 +86,7 @@ static struct gb_audio_manager_module_attribute gb_audio_module_name_attribute =
 static ssize_t gb_audio_module_vid_show(struct gb_audio_manager_module *module,
 					struct gb_audio_manager_module_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%d", module->desc.vid);
+	return sysfs_emit(buf, "%d\n", module->desc.vid);
 }
 
 static struct gb_audio_manager_module_attribute gb_audio_module_vid_attribute =
@@ -94,7 +95,7 @@ static struct gb_audio_manager_module_attribute gb_audio_module_vid_attribute =
 static ssize_t gb_audio_module_pid_show(struct gb_audio_manager_module *module,
 					struct gb_audio_manager_module_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%d", module->desc.pid);
+	return sysfs_emit(buf, "%d\n", module->desc.pid);
 }
 
 static struct gb_audio_manager_module_attribute gb_audio_module_pid_attribute =
@@ -104,7 +105,7 @@ static ssize_t gb_audio_module_intf_id_show(struct gb_audio_manager_module *modu
 					    struct gb_audio_manager_module_attribute *attr,
 					    char *buf)
 {
-	return sprintf(buf, "%d", module->desc.intf_id);
+	return sysfs_emit(buf, "%d\n", module->desc.intf_id);
 }
 
 static struct gb_audio_manager_module_attribute
@@ -115,7 +116,7 @@ static ssize_t gb_audio_module_ip_devices_show(struct gb_audio_manager_module *m
 					       struct gb_audio_manager_module_attribute *attr,
 					       char *buf)
 {
-	return sprintf(buf, "0x%X", module->desc.ip_devices);
+	return sysfs_emit(buf, "0x%X\n", module->desc.ip_devices);
 }
 
 static struct gb_audio_manager_module_attribute
@@ -126,7 +127,7 @@ static ssize_t gb_audio_module_op_devices_show(struct gb_audio_manager_module *m
 					       struct gb_audio_manager_module_attribute *attr,
 					       char *buf)
 {
-	return sprintf(buf, "0x%X", module->desc.op_devices);
+	return sysfs_emit(buf, "0x%X\n", module->desc.op_devices);
 }
 
 static struct gb_audio_manager_module_attribute
diff --git a/drivers/staging/greybus/gbphy.c b/drivers/staging/greybus/gbphy.c
index 6adcad286633..72d72aa7cb0f 100644
--- a/drivers/staging/greybus/gbphy.c
+++ b/drivers/staging/greybus/gbphy.c
@@ -14,6 +14,7 @@
 #include <linux/slab.h>
 #include <linux/device.h>
 #include <linux/greybus.h>
+#include <linux/sysfs.h>
 
 #include "gbphy.h"
 
@@ -31,7 +32,7 @@ static ssize_t protocol_id_show(struct device *dev,
 {
 	struct gbphy_device *gbphy_dev = to_gbphy_dev(dev);
 
-	return sprintf(buf, "0x%02x\n", gbphy_dev->cport_desc->protocol_id);
+	return sysfs_emit(buf, "0x%02x\n", gbphy_dev->cport_desc->protocol_id);
 }
 static DEVICE_ATTR_RO(protocol_id);
 
diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
index e509fdc715db..db0e98faec08 100644
--- a/drivers/staging/greybus/light.c
+++ b/drivers/staging/greybus/light.c
@@ -12,6 +12,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/greybus.h>
+#include <linux/sysfs.h>
 #include <media/v4l2-flash-led-class.h>
 
 #define NAMES_MAX	32
@@ -173,7 +174,7 @@ static ssize_t fade_##__dir##_show(struct device *dev,			\
 	struct led_classdev *cdev = dev_get_drvdata(dev);		\
 	struct gb_channel *channel = get_channel_from_cdev(cdev);	\
 									\
-	return sprintf(buf, "%u\n", channel->fade_##__dir);		\
+	return sysfs_emit(buf, "%u\n", channel->fade_##__dir);		\
 }									\
 									\
 static ssize_t fade_##__dir##_store(struct device *dev,			\
@@ -220,7 +221,7 @@ static ssize_t color_show(struct device *dev, struct device_attribute *attr,
 	struct led_classdev *cdev = dev_get_drvdata(dev);
 	struct gb_channel *channel = get_channel_from_cdev(cdev);
 
-	return sprintf(buf, "0x%08x\n", channel->color);
+	return sysfs_emit(buf, "0x%08x\n", channel->color);
 }
 
 static ssize_t color_store(struct device *dev, struct device_attribute *attr,
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index 1f19323b0e1a..0c1b45aa8b7b 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -26,6 +26,7 @@
 #include <linux/atomic.h>
 #include <linux/pm_runtime.h>
 #include <linux/greybus.h>
+#include <linux/sysfs.h>
 #include <asm/div64.h>
 
 #define NSEC_PER_DAY 86400000000000ULL
@@ -125,7 +126,7 @@ static ssize_t field##_show(struct device *dev,			\
 			    char *buf)					\
 {									\
 	struct gb_loopback *gb = dev_get_drvdata(dev);			\
-	return sprintf(buf, "%u\n", gb->field);			\
+	return sysfs_emit(buf, "%u\n", gb->field);			\
 }									\
 static DEVICE_ATTR_RO(field)
 
@@ -137,8 +138,8 @@ static ssize_t name##_##field##_show(struct device *dev,	\
 	struct gb_loopback *gb = dev_get_drvdata(dev);			\
 	/* Report 0 for min and max if no transfer succeeded */		\
 	if (!gb->requests_completed)					\
-		return sprintf(buf, "0\n");				\
-	return sprintf(buf, "%" #type "\n", gb->name.field);		\
+		return sysfs_emit(buf, "0\n");				\
+	return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);		\
 }									\
 static DEVICE_ATTR_RO(name##_##field)
 
@@ -158,7 +159,7 @@ static ssize_t name##_avg_show(struct device *dev,		\
 	rem = do_div(avg, count);					\
 	rem *= 1000000;							\
 	do_div(rem, count);						\
-	return sprintf(buf, "%llu.%06u\n", avg, (u32)rem);		\
+	return sysfs_emit(buf, "%llu.%06u\n", avg, (u32)rem);		\
 }									\
 static DEVICE_ATTR_RO(name##_avg)
 
@@ -173,7 +174,7 @@ static ssize_t field##_show(struct device *dev,				\
 			    char *buf)					\
 {									\
 	struct gb_loopback *gb = dev_get_drvdata(dev);			\
-	return sprintf(buf, "%" #type "\n", gb->field);			\
+	return sysfs_emit(buf, "%\n" #type "\n", gb->field);			\
 }									\
 static ssize_t field##_store(struct device *dev,			\
 			    struct device_attribute *attr,		\
@@ -199,7 +200,7 @@ static ssize_t field##_show(struct device *dev,		\
 			    char *buf)					\
 {									\
 	struct gb_loopback *gb = dev_get_drvdata(dev);			\
-	return sprintf(buf, "%u\n", gb->field);				\
+	return sysfs_emit(buf, "%u\n", gb->field);				\
 }									\
 static DEVICE_ATTR_RO(field)
 
@@ -209,7 +210,7 @@ static ssize_t field##_show(struct device *dev,				\
 			    char *buf)					\
 {									\
 	struct gb_loopback *gb = dev_get_drvdata(dev);			\
-	return sprintf(buf, "%" #type "\n", gb->field);			\
+	return sysfs_emit(buf, "%\n" #type "\n", gb->field);			\
 }									\
 static ssize_t field##_store(struct device *dev,			\
 			    struct device_attribute *attr,		\
@@ -679,7 +680,7 @@ static int gb_loopback_request_handler(struct gb_operation *operation)
 		}
 
 		if (!gb_operation_response_alloc(operation,
-				len + sizeof(*response), GFP_KERNEL)) {
+						 len + sizeof(*response), GFP_KERNEL)) {
 			dev_err(dev, "error allocating response\n");
 			return -ENOMEM;
 		}
@@ -831,109 +832,120 @@ static void gb_loopback_async_wait_to_send(struct gb_loopback *gb)
 				  kthread_should_stop());
 }
 
-static int gb_loopback_fn(void *data)
+static bool gb_loopback_should_stop(struct gb_loopback *gb,
+				    struct gb_bundle *bundle)
+{
+	if (!gb->type) {
+		gb_pm_runtime_put_autosuspend(bundle);
+		wait_event_interruptible(gb->wq,
+					 gb->type || kthread_should_stop());
+		if (kthread_should_stop())
+			return true;
+		gb_pm_runtime_get_sync(bundle);
+	}
+	return kthread_should_stop();
+}
+
+static void gb_loopback_handle_completion(struct gb_loopback *gb,
+					  struct gb_bundle *bundle)
+{
+	gb_loopback_async_wait_all(gb);
+
+	mutex_lock(&gb->mutex);
+	if (gb->iteration_count == gb->iteration_max) {
+		gb->type = 0;
+		gb->send_count = 0;
+		sysfs_notify(&gb->dev->kobj, NULL, "iteration_count");
+		dev_dbg(&bundle->dev, "load test complete\n");
+	} else {
+		dev_dbg(&bundle->dev, "continuing on with new test set\n");
+	}
+	mutex_unlock(&gb->mutex);
+}
+
+static void gb_loopback_dispatch_operation(struct gb_loopback *gb, int type,
+					   u32 size)
 {
 	int error = 0;
-	int us_wait = 0;
-	int type;
-	int ret;
-	u32 size;
 
+	if (gb->async) {
+		if (type == GB_LOOPBACK_TYPE_PING)
+			error = gb_loopback_async_ping(gb);
+		else if (type == GB_LOOPBACK_TYPE_TRANSFER)
+			error = gb_loopback_async_transfer(gb, size);
+		else if (type == GB_LOOPBACK_TYPE_SINK)
+			error = gb_loopback_async_sink(gb, size);
+
+		if (error) {
+			gb->error++;
+			gb->iteration_count++;
+		}
+	} else {
+		if (type == GB_LOOPBACK_TYPE_PING)
+			error = gb_loopback_sync_ping(gb);
+		else if (type == GB_LOOPBACK_TYPE_TRANSFER)
+			error = gb_loopback_sync_transfer(gb, size);
+		else if (type == GB_LOOPBACK_TYPE_SINK)
+			error = gb_loopback_sync_sink(gb, size);
+
+		if (error)
+			gb->error++;
+		gb->iteration_count++;
+		gb_loopback_calculate_stats(gb, !!error);
+	}
+}
+
+static void gb_loopback_delay_if_needed(int us_wait)
+{
+	if (us_wait) {
+		if (us_wait < 20000)
+			usleep_range(us_wait, us_wait + 100);
+		else
+			msleep(us_wait / 1000);
+	}
+}
+
+static int gb_loopback_fn(void *data)
+{
+	int us_wait = 0, type;
+	u32 size;
 	struct gb_loopback *gb = data;
 	struct gb_bundle *bundle = gb->connection->bundle;
 
-	ret = gb_pm_runtime_get_sync(bundle);
-	if (ret)
-		return ret;
+	if (gb_pm_runtime_get_sync(bundle))
+		return -EIO;
 
 	while (1) {
-		if (!gb->type) {
-			gb_pm_runtime_put_autosuspend(bundle);
-			wait_event_interruptible(gb->wq, gb->type ||
-						 kthread_should_stop());
-			ret = gb_pm_runtime_get_sync(bundle);
-			if (ret)
-				return ret;
-		}
-
-		if (kthread_should_stop())
+		if (gb_loopback_should_stop(gb, bundle))
 			break;
 
-		/* Limit the maximum number of in-flight async operations */
 		gb_loopback_async_wait_to_send(gb);
 		if (kthread_should_stop())
 			break;
 
 		mutex_lock(&gb->mutex);
 
-		/* Optionally terminate */
 		if (gb->send_count == gb->iteration_max) {
 			mutex_unlock(&gb->mutex);
-
-			/* Wait for synchronous and asynchronous completion */
-			gb_loopback_async_wait_all(gb);
-
-			/* Mark complete unless user-space has poked us */
-			mutex_lock(&gb->mutex);
-			if (gb->iteration_count == gb->iteration_max) {
-				gb->type = 0;
-				gb->send_count = 0;
-				sysfs_notify(&gb->dev->kobj,  NULL,
-					     "iteration_count");
-				dev_dbg(&bundle->dev, "load test complete\n");
-			} else {
-				dev_dbg(&bundle->dev,
-					"continuing on with new test set\n");
-			}
-			mutex_unlock(&gb->mutex);
+			gb_loopback_handle_completion(gb, bundle);
 			continue;
 		}
+
 		size = gb->size;
 		us_wait = gb->us_wait;
 		type = gb->type;
 		if (ktime_to_ns(gb->ts) == 0)
 			gb->ts = ktime_get();
 
-		/* Else operations to perform */
-		if (gb->async) {
-			if (type == GB_LOOPBACK_TYPE_PING)
-				error = gb_loopback_async_ping(gb);
-			else if (type == GB_LOOPBACK_TYPE_TRANSFER)
-				error = gb_loopback_async_transfer(gb, size);
-			else if (type == GB_LOOPBACK_TYPE_SINK)
-				error = gb_loopback_async_sink(gb, size);
-
-			if (error) {
-				gb->error++;
-				gb->iteration_count++;
-			}
-		} else {
-			/* We are effectively single threaded here */
-			if (type == GB_LOOPBACK_TYPE_PING)
-				error = gb_loopback_sync_ping(gb);
-			else if (type == GB_LOOPBACK_TYPE_TRANSFER)
-				error = gb_loopback_sync_transfer(gb, size);
-			else if (type == GB_LOOPBACK_TYPE_SINK)
-				error = gb_loopback_sync_sink(gb, size);
-
-			if (error)
-				gb->error++;
-			gb->iteration_count++;
-			gb_loopback_calculate_stats(gb, !!error);
-		}
+		gb_loopback_dispatch_operation(gb, type, size);
+
 		gb->send_count++;
 		mutex_unlock(&gb->mutex);
 
-		if (us_wait) {
-			if (us_wait < 20000)
-				usleep_range(us_wait, us_wait + 100);
-			else
-				msleep(us_wait / 1000);
-		}
+		gb_loopback_delay_if_needed(us_wait);
 	}
 
 	gb_pm_runtime_put_autosuspend(bundle);
-
 	return 0;
 }
 
-- 
2.43.0
Re: [PATCH] staging: greybus: Multiple cleanups and refactors
Posted by kernel test robot 8 months, 3 weeks ago
Hi gpittala,

kernel test robot noticed the following build warnings:

[auto build test WARNING on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/gpittala/staging-greybus-Multiple-cleanups-and-refactors/20250401-053719
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20250331213337.6171-1-ganeshkpittala%40gmail.com
patch subject: [PATCH] staging: greybus: Multiple cleanups and refactors
config: sparc-randconfig-002-20250401 (https://download.01.org/0day-ci/archive/20250401/202504011217.iRb2Bbls-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250401/202504011217.iRb2Bbls-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504011217.iRb2Bbls-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/staging/greybus/loopback.c: In function 'latency_min_show':
>> drivers/staging/greybus/loopback.c:142:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:167:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, min, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:272:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(latency);
    ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/staging/greybus/loopback.c:142:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:167:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, min, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:272:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(latency);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'latency_max_show':
>> drivers/staging/greybus/loopback.c:142:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:168:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, max, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:272:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(latency);
    ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/staging/greybus/loopback.c:142:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:168:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, max, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:272:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(latency);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'requests_per_second_min_show':
>> drivers/staging/greybus/loopback.c:142:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:167:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, min, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:274:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(requests_per_second);
    ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/staging/greybus/loopback.c:142:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:167:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, min, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:274:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(requests_per_second);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'requests_per_second_max_show':
>> drivers/staging/greybus/loopback.c:142:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:168:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, max, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:274:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(requests_per_second);
    ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/staging/greybus/loopback.c:142:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:168:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, max, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:274:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(requests_per_second);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'throughput_min_show':
>> drivers/staging/greybus/loopback.c:142:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:167:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, min, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:276:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(throughput);
    ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/staging/greybus/loopback.c:142:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:167:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, min, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:276:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(throughput);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'throughput_max_show':
>> drivers/staging/greybus/loopback.c:142:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:168:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, max, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:276:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(throughput);
    ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/staging/greybus/loopback.c:142:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:168:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, max, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:276:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(throughput);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'apbridge_unipro_latency_min_show':
>> drivers/staging/greybus/loopback.c:142:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:167:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, min, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:278:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(apbridge_unipro_latency);
    ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/staging/greybus/loopback.c:142:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:167:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, min, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:278:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(apbridge_unipro_latency);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'apbridge_unipro_latency_max_show':
>> drivers/staging/greybus/loopback.c:142:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:168:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, max, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:278:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(apbridge_unipro_latency);
    ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/staging/greybus/loopback.c:142:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:168:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, max, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:278:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(apbridge_unipro_latency);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'gbphy_firmware_latency_min_show':
>> drivers/staging/greybus/loopback.c:142:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:167:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, min, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:280:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(gbphy_firmware_latency);
    ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/staging/greybus/loopback.c:142:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:167:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, min, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:280:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(gbphy_firmware_latency);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'gbphy_firmware_latency_max_show':
>> drivers/staging/greybus/loopback.c:142:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:168:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, max, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:280:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(gbphy_firmware_latency);
    ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/staging/greybus/loopback.c:142:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);  \
                            ^
   drivers/staging/greybus/loopback.c:168:2: note: in expansion of macro 'gb_loopback_ro_stats_attr'
     gb_loopback_ro_stats_attr(field, max, u);  \
     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:280:1: note: in expansion of macro 'gb_loopback_stats_attrs'
    gb_loopback_stats_attrs(gbphy_firmware_latency);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'type_show':
   drivers/staging/greybus/loopback.c:213:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->field);   \
                            ^
   drivers/staging/greybus/loopback.c:301:1: note: in expansion of macro 'gb_dev_loopback_rw_attr'
    gb_dev_loopback_rw_attr(type, d);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:213:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->field);   \
                            ^
   drivers/staging/greybus/loopback.c:301:1: note: in expansion of macro 'gb_dev_loopback_rw_attr'
    gb_dev_loopback_rw_attr(type, d);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'size_show':
   drivers/staging/greybus/loopback.c:213:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->field);   \
                            ^
   drivers/staging/greybus/loopback.c:303:1: note: in expansion of macro 'gb_dev_loopback_rw_attr'
    gb_dev_loopback_rw_attr(size, u);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:213:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->field);   \
                            ^
   drivers/staging/greybus/loopback.c:303:1: note: in expansion of macro 'gb_dev_loopback_rw_attr'
    gb_dev_loopback_rw_attr(size, u);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'us_wait_show':
   drivers/staging/greybus/loopback.c:213:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->field);   \
                            ^
   drivers/staging/greybus/loopback.c:305:1: note: in expansion of macro 'gb_dev_loopback_rw_attr'
    gb_dev_loopback_rw_attr(us_wait, d);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:213:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->field);   \
                            ^
   drivers/staging/greybus/loopback.c:305:1: note: in expansion of macro 'gb_dev_loopback_rw_attr'
    gb_dev_loopback_rw_attr(us_wait, d);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'iteration_max_show':
   drivers/staging/greybus/loopback.c:213:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->field);   \
                            ^
   drivers/staging/greybus/loopback.c:307:1: note: in expansion of macro 'gb_dev_loopback_rw_attr'
    gb_dev_loopback_rw_attr(iteration_max, u);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:213:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->field);   \
                            ^
   drivers/staging/greybus/loopback.c:307:1: note: in expansion of macro 'gb_dev_loopback_rw_attr'
    gb_dev_loopback_rw_attr(iteration_max, u);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'async_show':
   drivers/staging/greybus/loopback.c:213:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->field);   \
                            ^
   drivers/staging/greybus/loopback.c:311:1: note: in expansion of macro 'gb_dev_loopback_rw_attr'
    gb_dev_loopback_rw_attr(async, u);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:213:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->field);   \
                            ^
   drivers/staging/greybus/loopback.c:311:1: note: in expansion of macro 'gb_dev_loopback_rw_attr'
    gb_dev_loopback_rw_attr(async, u);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'timeout_show':
   drivers/staging/greybus/loopback.c:213:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->field);   \
                            ^
   drivers/staging/greybus/loopback.c:313:1: note: in expansion of macro 'gb_dev_loopback_rw_attr'
    gb_dev_loopback_rw_attr(timeout, u);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:213:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->field);   \
                            ^
   drivers/staging/greybus/loopback.c:313:1: note: in expansion of macro 'gb_dev_loopback_rw_attr'
    gb_dev_loopback_rw_attr(timeout, u);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c: In function 'outstanding_operations_max_show':
   drivers/staging/greybus/loopback.c:213:25: warning: unknown conversion type character '\x0a' in format [-Wformat=]
     return sysfs_emit(buf, "%\n" #type "\n", gb->field);   \
                            ^
   drivers/staging/greybus/loopback.c:315:1: note: in expansion of macro 'gb_dev_loopback_rw_attr'
    gb_dev_loopback_rw_attr(outstanding_operations_max, u);
    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:213:25: warning: too many arguments for format [-Wformat-extra-args]
     return sysfs_emit(buf, "%\n" #type "\n", gb->field);   \
                            ^
   drivers/staging/greybus/loopback.c:315:1: note: in expansion of macro 'gb_dev_loopback_rw_attr'
    gb_dev_loopback_rw_attr(outstanding_operations_max, u);
    ^~~~~~~~~~~~~~~~~~~~~~~


vim +/x0a +142 drivers/staging/greybus/loopback.c

   132	
   133	#define gb_loopback_ro_stats_attr(name, field, type)		\
   134	static ssize_t name##_##field##_show(struct device *dev,	\
   135				    struct device_attribute *attr,		\
   136				    char *buf)					\
   137	{									\
   138		struct gb_loopback *gb = dev_get_drvdata(dev);			\
   139		/* Report 0 for min and max if no transfer succeeded */		\
   140		if (!gb->requests_completed)					\
   141			return sysfs_emit(buf, "0\n");				\
 > 142		return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);		\
   143	}									\
   144	static DEVICE_ATTR_RO(name##_##field)
   145	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] staging: greybus: Multiple cleanups and refactors
Posted by kernel test robot 8 months, 3 weeks ago
Hi gpittala,

kernel test robot noticed the following build warnings:

[auto build test WARNING on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/gpittala/staging-greybus-Multiple-cleanups-and-refactors/20250401-053719
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20250331213337.6171-1-ganeshkpittala%40gmail.com
patch subject: [PATCH] staging: greybus: Multiple cleanups and refactors
config: i386-buildonly-randconfig-004-20250401 (https://download.01.org/0day-ci/archive/20250401/202504010829.vIzweYue-lkp@intel.com/config)
compiler: clang version 20.1.1 (https://github.com/llvm/llvm-project 424c2d9b7e4de40d0804dd374721e6411c27d1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250401/202504010829.vIzweYue-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504010829.vIzweYue-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/staging/greybus/loopback.c:272:1: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]
     272 | gb_loopback_stats_attrs(latency);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:167:2: note: expanded from macro 'gb_loopback_stats_attrs'
     167 |         gb_loopback_ro_stats_attr(field, min, u);               \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:142:27: note: expanded from macro 'gb_loopback_ro_stats_attr'
     142 |         return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);               \
         |                                 ~^
>> drivers/staging/greybus/loopback.c:272:1: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]
     272 | gb_loopback_stats_attrs(latency);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:168:2: note: expanded from macro 'gb_loopback_stats_attrs'
     168 |         gb_loopback_ro_stats_attr(field, max, u);               \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:142:27: note: expanded from macro 'gb_loopback_ro_stats_attr'
     142 |         return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);               \
         |                                 ~^
   drivers/staging/greybus/loopback.c:274:1: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]
     274 | gb_loopback_stats_attrs(requests_per_second);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:167:2: note: expanded from macro 'gb_loopback_stats_attrs'
     167 |         gb_loopback_ro_stats_attr(field, min, u);               \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:142:27: note: expanded from macro 'gb_loopback_ro_stats_attr'
     142 |         return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);               \
         |                                 ~^
   drivers/staging/greybus/loopback.c:274:1: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]
     274 | gb_loopback_stats_attrs(requests_per_second);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:168:2: note: expanded from macro 'gb_loopback_stats_attrs'
     168 |         gb_loopback_ro_stats_attr(field, max, u);               \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:142:27: note: expanded from macro 'gb_loopback_ro_stats_attr'
     142 |         return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);               \
         |                                 ~^
   drivers/staging/greybus/loopback.c:276:1: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]
     276 | gb_loopback_stats_attrs(throughput);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:167:2: note: expanded from macro 'gb_loopback_stats_attrs'
     167 |         gb_loopback_ro_stats_attr(field, min, u);               \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:142:27: note: expanded from macro 'gb_loopback_ro_stats_attr'
     142 |         return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);               \
         |                                 ~^
   drivers/staging/greybus/loopback.c:276:1: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]
     276 | gb_loopback_stats_attrs(throughput);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:168:2: note: expanded from macro 'gb_loopback_stats_attrs'
     168 |         gb_loopback_ro_stats_attr(field, max, u);               \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:142:27: note: expanded from macro 'gb_loopback_ro_stats_attr'
     142 |         return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);               \
         |                                 ~^
   drivers/staging/greybus/loopback.c:278:1: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]
     278 | gb_loopback_stats_attrs(apbridge_unipro_latency);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:167:2: note: expanded from macro 'gb_loopback_stats_attrs'
     167 |         gb_loopback_ro_stats_attr(field, min, u);               \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:142:27: note: expanded from macro 'gb_loopback_ro_stats_attr'
     142 |         return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);               \
         |                                 ~^
   drivers/staging/greybus/loopback.c:278:1: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]
     278 | gb_loopback_stats_attrs(apbridge_unipro_latency);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:168:2: note: expanded from macro 'gb_loopback_stats_attrs'
     168 |         gb_loopback_ro_stats_attr(field, max, u);               \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:142:27: note: expanded from macro 'gb_loopback_ro_stats_attr'
     142 |         return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);               \
         |                                 ~^
   drivers/staging/greybus/loopback.c:280:1: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]
     280 | gb_loopback_stats_attrs(gbphy_firmware_latency);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:167:2: note: expanded from macro 'gb_loopback_stats_attrs'
     167 |         gb_loopback_ro_stats_attr(field, min, u);               \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:142:27: note: expanded from macro 'gb_loopback_ro_stats_attr'
     142 |         return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);               \
         |                                 ~^
   drivers/staging/greybus/loopback.c:280:1: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]
     280 | gb_loopback_stats_attrs(gbphy_firmware_latency);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:168:2: note: expanded from macro 'gb_loopback_stats_attrs'
     168 |         gb_loopback_ro_stats_attr(field, max, u);               \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:142:27: note: expanded from macro 'gb_loopback_ro_stats_attr'
     142 |         return sysfs_emit(buf, "%\n" #type "\n", gb->name.field);               \
         |                                 ~^
   drivers/staging/greybus/loopback.c:301:1: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]
     301 | gb_dev_loopback_rw_attr(type, d);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:213:27: note: expanded from macro 'gb_dev_loopback_rw_attr'
     213 |         return sysfs_emit(buf, "%\n" #type "\n", gb->field);                    \
         |                                 ~^
   drivers/staging/greybus/loopback.c:303:1: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]
     303 | gb_dev_loopback_rw_attr(size, u);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:213:27: note: expanded from macro 'gb_dev_loopback_rw_attr'
     213 |         return sysfs_emit(buf, "%\n" #type "\n", gb->field);                    \
         |                                 ~^
   drivers/staging/greybus/loopback.c:305:1: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]
     305 | gb_dev_loopback_rw_attr(us_wait, d);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/greybus/loopback.c:213:27: note: expanded from macro 'gb_dev_loopback_rw_attr'
     213 |         return sysfs_emit(buf, "%\n" #type "\n", gb->field);                    \
         |                                 ~^
   drivers/staging/greybus/loopback.c:307:1: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]
     307 | gb_dev_loopback_rw_attr(iteration_max, u);


vim +/x0a +272 drivers/staging/greybus/loopback.c

355a7058153e04b Alexandre Bailon 2015-03-31  270  
355a7058153e04b Alexandre Bailon 2015-03-31  271  /* Time to send and receive one message */
8e1d6c336d74977 Bryan O'Donoghue 2015-12-03 @272  gb_loopback_stats_attrs(latency);
583cbf50e0a4c89 Bryan O'Donoghue 2015-07-21  273  /* Number of requests sent per second on this cport */
8e1d6c336d74977 Bryan O'Donoghue 2015-12-03  274  gb_loopback_stats_attrs(requests_per_second);
355a7058153e04b Alexandre Bailon 2015-03-31  275  /* Quantity of data sent and received on this cport */
8e1d6c336d74977 Bryan O'Donoghue 2015-12-03  276  gb_loopback_stats_attrs(throughput);
1ec5843ee988991 Bryan O'Donoghue 2015-10-15  277  /* Latency across the UniPro link from APBridge's perspective */
8e1d6c336d74977 Bryan O'Donoghue 2015-12-03  278  gb_loopback_stats_attrs(apbridge_unipro_latency);
1ec5843ee988991 Bryan O'Donoghue 2015-10-15  279  /* Firmware induced overhead in the GPBridge */
e54b106dd1be503 Sandeep Patil    2016-05-19  280  gb_loopback_stats_attrs(gbphy_firmware_latency);
8e1d6c336d74977 Bryan O'Donoghue 2015-12-03  281  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [greybus-dev] [PATCH] staging: greybus: Multiple cleanups and refactors
Posted by Jeff Johnson 8 months, 3 weeks ago
On 3/31/2025 2:33 PM, gpittala wrote:
> This patch includes multiple meaningful cleanups for the Greybus staging driver:
> 
> 1. firmware.c: Replaced deprecated 'strncpy()' with 'strscpy()'
> 2. sysfs show functions: Replaced 'sprintf()' with 'sysfs_emit()'
> 3. loopback.c: Refactored a large function (gp_loopback_fn) to improve readability
> 4. audio_gb.c: Split logic in get_topology() into separate calls as per TODO
> 
> All changes are tested and pass checkpatch.pl
> 
> Signed-off-by: gpittala <ganeshkpittala@gmail.com>

Please refer to:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

So it is generally expected that "gpittala" be replaced with "a known
identity" which is normally your actual name.
Re: [PATCH] staging: greybus: Multiple cleanups and refactors
Posted by Alex Elder 8 months, 3 weeks ago
On 3/31/25 4:33 PM, gpittala wrote:
> This patch includes multiple meaningful cleanups for the Greybus staging driver:
> 
> 1. firmware.c: Replaced deprecated 'strncpy()' with 'strscpy()'

This is a good type of change to make.

> 2. sysfs show functions: Replaced 'sprintf()' with 'sysfs_emit()'

This is also an improvement.

> 3. loopback.c: Refactored a large function (gp_loopback_fn) to improve readability

I have only glanced at this, but refactoring something can
sometimes be clearer if you do it in several small patches.

> 4. audio_gb.c: Split logic in get_topology() into separate calls as per TODO

I'll comment more below, but you should almost always have
only one change per patch.  So each of the four items listed
above deserves its own patch.  You could send them separately
(because they're unrelated), or as a series of cleanups.

Note that "one change per patch" is a logical (not literal)
statement.  For example, you could do a single patch that
replaces *all* calls to strncpy() with strcspy().

> All changes are tested and pass checkpatch.pl
> 
> Signed-off-by: gpittala <ganeshkpittala@gmail.com>
> ---
>   .../greybus/Documentation/firmware/firmware.c |  32 ++--
>   drivers/staging/greybus/arche-apb-ctrl.c      |  11 +-
>   drivers/staging/greybus/arche-platform.c      |  11 +-
>   drivers/staging/greybus/audio_gb.c            |  37 +++-
>   .../staging/greybus/audio_manager_module.c    |  13 +-
>   drivers/staging/greybus/gbphy.c               |   3 +-
>   drivers/staging/greybus/light.c               |   5 +-
>   drivers/staging/greybus/loopback.c            | 170 ++++++++++--------
>   8 files changed, 159 insertions(+), 123 deletions(-)
> 
> diff --git a/drivers/staging/greybus/Documentation/firmware/firmware.c b/drivers/staging/greybus/Documentation/firmware/firmware.c
> index 765d69faa9cc..8e375c88c881 100644
> --- a/drivers/staging/greybus/Documentation/firmware/firmware.c
> +++ b/drivers/staging/greybus/Documentation/firmware/firmware.c
> @@ -47,12 +47,12 @@ static int update_intf_firmware(int fd)
>   	ret = ioctl(fd, FW_MGMT_IOC_GET_INTF_FW, &intf_fw_info);
>   	if (ret < 0) {
>   		printf("Failed to get interface firmware version: %s (%d)\n",
> -			fwdev, ret);
> +		       fwdev, ret);

The two changes in this hunk are not mentioned in the
description above.  Please remove these changes.  If
you want to do reformatting like this, do it in a
separate patch.

While it might be reasonable to include a little white
space change like this occasionally, you should avoid
doing it.  It is unrelated, and complicates your patch
unnecessarily.

This comment applies to several other changes you've
made below.  It also applies to removal (or addition) of
blank lines, or really, any other white space changes.

					-Alex

>   		return -1;
>   	}
>   
>   	printf("Interface Firmware tag (%s), major (%d), minor (%d)\n",
> -		intf_fw_info.firmware_tag, intf_fw_info.major,
> +	       intf_fw_info.firmware_tag, intf_fw_info.major,
>   		intf_fw_info.minor);
>   
>   	/* Try Interface Firmware load over Unipro */
. . .