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; }
On 3/31/25 4:33 PM, gpittala wrote:
This patch includes multiple meaningful cleanups for the Greybus staging driver:
- firmware.c: Replaced deprecated 'strncpy()' with 'strscpy()'
This is a good type of change to make.
- sysfs show functions: Replaced 'sprintf()' with 'sysfs_emit()'
This is also an improvement.
- 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.
- 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,/* Try Interface Firmware load over Unipro */
. . .
Hi Alex,
Thank you for the detailed feedback and guidance. I appreciate your time reviewing the patch.
I’ll split the changes into separate patches as suggested and send a v2 shortly.
Best regards, Ganesh Pittala
On Mon, Mar 31, 2025 at 7:49 PM Alex Elder elder@riscstar.com wrote:
On 3/31/25 4:33 PM, gpittala wrote:
This patch includes multiple meaningful cleanups for the Greybus staging
driver:
- firmware.c: Replaced deprecated 'strncpy()' with 'strscpy()'
This is a good type of change to make.
- sysfs show functions: Replaced 'sprintf()' with 'sysfs_emit()'
This is also an improvement.
- 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.
- 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.
-Alexreturn -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 */. . .
On 3/31/2025 5:03 PM, Ganesh Kumar Pittala wrote:
Hi Alex,
Thank you for the detailed feedback and guidance. I appreciate your time reviewing the patch.
I’ll split the changes into separate patches as suggested and send a v2 shortly.
Best regards, Ganesh Pittala
Some more feedback:
Don't "top post" https://www.kernel.org/doc/html/latest/process/submitting-patches.html#use-t...
Also don't use HTML e-mail https://www.kernel.org/doc/html/latest/process/submitting-patches.html#no-mi...
Yes, there are a lot of hoops you have to jump through to get your code in the kernel. It becomes easier over time.
/jeff
On 3/31/2025 2:33 PM, gpittala wrote:
This patch includes multiple meaningful cleanups for the Greybus staging driver:
- firmware.c: Replaced deprecated 'strncpy()' with 'strscpy()'
- sysfs show functions: Replaced 'sprintf()' with 'sysfs_emit()'
- loopback.c: Refactored a large function (gp_loopback_fn) to improve readability
- 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-...
So it is generally expected that "gpittala" be replaced with "a known identity" which is normally your actual name.
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-Mult... 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@i...) 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@i...)
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
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-Mult... 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@i...) 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@i...)
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
-
Alex Elder -
Ganesh Kumar Pittala -
gpittala
-
Jeff Johnson -
kernel test robot