From: Yalagada Pavan Kumar pavankumaryalagada@gmail.com
Unsigned variables are printed using %d. Replace them with %u to match their types.
Signed-off-by: Yalagada Pavan Kumar pavankumaryalagada@gmail.com --- drivers/staging/greybus/audio_codec.c | 20 ++++++++-------- drivers/staging/greybus/audio_topology.c | 30 ++++++++++++------------ drivers/staging/greybus/bootrom.c | 2 +- drivers/staging/greybus/fw-download.c | 2 +- drivers/staging/greybus/fw-management.c | 2 +- drivers/staging/greybus/gpio.c | 2 +- drivers/staging/greybus/light.c | 2 +- drivers/staging/greybus/log.c | 4 ++-- drivers/staging/greybus/loopback.c | 4 ++-- drivers/staging/greybus/power_supply.c | 2 +- drivers/staging/greybus/raw.c | 2 +- drivers/staging/greybus/sdio.c | 4 ++-- drivers/staging/greybus/spilib.c | 2 +- drivers/staging/greybus/uart.c | 4 ++-- 14 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c index 6daa4e706792..a66c2ee9c808 100644 --- a/drivers/staging/greybus/audio_codec.c +++ b/drivers/staging/greybus/audio_codec.c @@ -78,7 +78,7 @@ static int gbaudio_module_enable_tx(struct gbaudio_codec_info *codec, return ret; } data->state[SNDRV_PCM_STREAM_PLAYBACK] = GBAUDIO_CODEC_STARTUP; - dev_dbg(module->dev, "Dynamic Register %d DAI\n", cportid); + dev_dbg(module->dev, "Dynamic Register %u DAI\n", cportid); }
/* hw_params */ @@ -95,7 +95,7 @@ static int gbaudio_module_enable_tx(struct gbaudio_codec_info *codec, return ret; } data->state[SNDRV_PCM_STREAM_PLAYBACK] = GBAUDIO_CODEC_HWPARAMS; - dev_dbg(module->dev, "Dynamic hw_params %d DAI\n", data_cport); + dev_dbg(module->dev, "Dynamic hw_params %u DAI\n", data_cport); }
/* prepare */ @@ -116,7 +116,7 @@ static int gbaudio_module_enable_tx(struct gbaudio_codec_info *codec, return ret; } data->state[SNDRV_PCM_STREAM_PLAYBACK] = GBAUDIO_CODEC_PREPARE; - dev_dbg(module->dev, "Dynamic prepare %d DAI\n", data_cport); + dev_dbg(module->dev, "Dynamic prepare %u DAI\n", data_cport); }
return 0; @@ -146,7 +146,7 @@ static int gbaudio_module_disable_tx(struct gbaudio_module_info *module, int id) "deactivate_tx failed:%d\n", ret); return ret; } - dev_dbg(module->dev, "Dynamic deactivate %d DAI\n", data_cport); + dev_dbg(module->dev, "Dynamic deactivate %u DAI\n", data_cport); data->state[SNDRV_PCM_STREAM_PLAYBACK] = GBAUDIO_CODEC_HWPARAMS; }
@@ -161,7 +161,7 @@ static int gbaudio_module_disable_tx(struct gbaudio_module_info *module, int id) "unregister_cport failed:%d\n", ret); return ret; } - dev_dbg(module->dev, "Dynamic Unregister %d DAI\n", cportid); + dev_dbg(module->dev, "Dynamic Unregister %u DAI\n", cportid); data->state[SNDRV_PCM_STREAM_PLAYBACK] = GBAUDIO_CODEC_SHUTDOWN; }
@@ -204,7 +204,7 @@ static int gbaudio_module_enable_rx(struct gbaudio_codec_info *codec, return ret; } data->state[SNDRV_PCM_STREAM_CAPTURE] = GBAUDIO_CODEC_STARTUP; - dev_dbg(module->dev, "Dynamic Register %d DAI\n", cportid); + dev_dbg(module->dev, "Dynamic Register %u DAI\n", cportid); }
/* hw_params */ @@ -221,7 +221,7 @@ static int gbaudio_module_enable_rx(struct gbaudio_codec_info *codec, return ret; } data->state[SNDRV_PCM_STREAM_CAPTURE] = GBAUDIO_CODEC_HWPARAMS; - dev_dbg(module->dev, "Dynamic hw_params %d DAI\n", data_cport); + dev_dbg(module->dev, "Dynamic hw_params %u DAI\n", data_cport); }
/* prepare */ @@ -243,7 +243,7 @@ static int gbaudio_module_enable_rx(struct gbaudio_codec_info *codec, return ret; } data->state[SNDRV_PCM_STREAM_CAPTURE] = GBAUDIO_CODEC_PREPARE; - dev_dbg(module->dev, "Dynamic prepare %d DAI\n", data_cport); + dev_dbg(module->dev, "Dynamic prepare %u DAI\n", data_cport); }
return 0; @@ -273,7 +273,7 @@ static int gbaudio_module_disable_rx(struct gbaudio_module_info *module, int id) "deactivate_rx failed:%d\n", ret); return ret; } - dev_dbg(module->dev, "Dynamic deactivate %d DAI\n", data_cport); + dev_dbg(module->dev, "Dynamic deactivate %u DAI\n", data_cport); data->state[SNDRV_PCM_STREAM_CAPTURE] = GBAUDIO_CODEC_HWPARAMS; }
@@ -288,7 +288,7 @@ static int gbaudio_module_disable_rx(struct gbaudio_module_info *module, int id) "unregister_cport failed:%d\n", ret); return ret; } - dev_dbg(module->dev, "Dynamic Unregister %d DAI\n", cportid); + dev_dbg(module->dev, "Dynamic Unregister %u DAI\n", cportid); data->state[SNDRV_PCM_STREAM_CAPTURE] = GBAUDIO_CODEC_SHUTDOWN; }
diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c index 76146f91cddc..a5edf2ff009b 100644 --- a/drivers/staging/greybus/audio_topology.c +++ b/drivers/staging/greybus/audio_topology.c @@ -199,7 +199,7 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol *kcontrol, strscpy(uinfo->value.enumerated.name, name, sizeof(uinfo->value.enumerated.name)); break; default: - dev_err(comp->dev, "Invalid type: %d for %s:kcontrol\n", + dev_err(comp->dev, "Invalid type: %u for %s:kcontrol\n", info->type, kcontrol->id.name); break; } @@ -260,7 +260,7 @@ static int gbcodec_mixer_ctl_get(struct snd_kcontrol *kcontrol, le32_to_cpu(gbvalue.value.enumerated_item[1]); break; default: - dev_err(comp->dev, "Invalid type: %d for %s:kcontrol\n", + dev_err(comp->dev, "Invalid type: %u for %s:kcontrol\n", info->type, kcontrol->id.name); ret = -EINVAL; break; @@ -307,7 +307,7 @@ static int gbcodec_mixer_ctl_put(struct snd_kcontrol *kcontrol, cpu_to_le32(ucontrol->value.enumerated.item[1]); break; default: - dev_err(comp->dev, "Invalid type: %d for %s:kcontrol\n", + dev_err(comp->dev, "Invalid type: %u for %s:kcontrol\n", info->type, kcontrol->id.name); ret = -EINVAL; break; @@ -655,7 +655,7 @@ static int gbaudio_tplg_create_enum_kctl(struct gbaudio_module_info *gb, return -ENOMEM;
/* debug enum info */ - dev_dbg(gb->dev, "Max:%d, name_length:%d\n", gbe->items, + dev_dbg(gb->dev, "Max:%u, name_length:%u\n", gbe->items, le16_to_cpu(gb_enum->names_length)); for (i = 0; i < gbe->items; i++) dev_dbg(gb->dev, "src[%d]: %s\n", i, gbe->texts[i]); @@ -700,7 +700,7 @@ static int gbaudio_tplg_create_kcontrol(struct gbaudio_module_info *gb, return -EINVAL; }
- dev_dbg(gb->dev, "%s:%d control created\n", ctl->name, ctl->id); + dev_dbg(gb->dev, "%s:%u control created\n", ctl->name, ctl->id); return ret; }
@@ -861,7 +861,7 @@ static int gbaudio_tplg_create_enum_ctl(struct gbaudio_module_info *gb, return -ENOMEM;
/* debug enum info */ - dev_dbg(gb->dev, "Max:%d, name_length:%d\n", gbe->items, + dev_dbg(gb->dev, "Max:%u, name_length:%u\n", gbe->items, le16_to_cpu(gb_enum->names_length)); for (i = 0; i < gbe->items; i++) dev_dbg(gb->dev, "src[%d]: %s\n", i, gbe->texts[i]); @@ -914,7 +914,7 @@ static int gbaudio_tplg_create_wcontrol(struct gbaudio_module_info *gb, return -EINVAL; }
- dev_dbg(gb->dev, "%s:%d DAPM control created, ret:%d\n", ctl->name, + dev_dbg(gb->dev, "%s:%u DAPM control created, ret:%d\n", ctl->name, ctl->id, ret); return ret; } @@ -1022,7 +1022,7 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
ret = gbaudio_validate_kcontrol_count(w); if (ret) { - dev_err(module->dev, "Invalid kcontrol count=%d for %s\n", + dev_err(module->dev, "Invalid kcontrol count=%u for %s\n", w->ncontrols, w->name); return ret; } @@ -1044,7 +1044,7 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module, curr); if (ret) { dev_err(module->dev, - "%s:%d type widget_ctl not supported\n", + "%s:%u type widget_ctl not supported\n", curr->name, curr->iface); goto error; } @@ -1155,7 +1155,7 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module, ret = gbaudio_tplg_create_kcontrol(module, &dapm_kctls[i], curr); if (ret) { - dev_err(module->dev, "%s:%d type not supported\n", + dev_err(module->dev, "%s:%u type not supported\n", curr->name, curr->iface); goto error; } @@ -1192,7 +1192,7 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module, }
list_add(&control->list, &module->ctl_list); - dev_dbg(module->dev, "%d:%s created of type %d\n", curr->id, + dev_dbg(module->dev, "%u:%s created of type %u\n", curr->id, curr->name, curr->info.type); curr = (void *)curr + csize; } @@ -1228,7 +1228,7 @@ static int gbaudio_tplg_process_widgets(struct gbaudio_module_info *module, ret = gbaudio_tplg_create_widget(module, &dapm_widgets[i], curr, &w_size); if (ret) { - dev_err(module->dev, "%s:%d type not supported\n", + dev_err(module->dev, "%s:%u type not supported\n", curr->name, curr->type); goto error; } @@ -1278,7 +1278,7 @@ static int gbaudio_tplg_process_routes(struct gbaudio_module_info *module, dapm_routes->sink = gbaudio_map_widgetid(module, curr->destination_id); if (!dapm_routes->sink) { - dev_err(module->dev, "%d:%d:%d:%d - Invalid sink\n", + dev_err(module->dev, "%u:%u:%u:%u - Invalid sink\n", curr->source_id, curr->destination_id, curr->control_id, curr->index); ret = -EINVAL; @@ -1287,7 +1287,7 @@ static int gbaudio_tplg_process_routes(struct gbaudio_module_info *module, dapm_routes->source = gbaudio_map_widgetid(module, curr->source_id); if (!dapm_routes->source) { - dev_err(module->dev, "%d:%d:%d:%d - Invalid source\n", + dev_err(module->dev, "%u:%u:%u:%u - Invalid source\n", curr->source_id, curr->destination_id, curr->control_id, curr->index); ret = -EINVAL; @@ -1299,7 +1299,7 @@ static int gbaudio_tplg_process_routes(struct gbaudio_module_info *module, curr->index); if ((curr->control_id != GBAUDIO_INVALID_ID) && !dapm_routes->control) { - dev_err(module->dev, "%d:%d:%d:%d - Invalid control\n", + dev_err(module->dev, "%u:%u:%u:%u - Invalid control\n", curr->source_id, curr->destination_id, curr->control_id, curr->index); ret = -EINVAL; diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index 83921d90c322..fb1fbd3f7b99 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -222,7 +222,7 @@ static int gb_bootrom_firmware_size_request(struct gb_operation *op) size_response = op->response->payload; size_response->size = cpu_to_le32(bootrom->fw->size);
- dev_dbg(dev, "%s: firmware size %d bytes\n", + dev_dbg(dev, "%s: firmware size %u bytes\n", __func__, size_response->size);
unlock: diff --git a/drivers/staging/greybus/fw-download.c b/drivers/staging/greybus/fw-download.c index a0a683447b3d..9f7f68411418 100644 --- a/drivers/staging/greybus/fw-download.c +++ b/drivers/staging/greybus/fw-download.c @@ -148,7 +148,7 @@ static int exceeds_release_timeout(struct fw_request *fw_req) return 0;
dev_err(fw_download->parent, - "Firmware download didn't finish in time, abort: %d\n", + "Firmware download didn't finish in time, abort: %u\n", fw_req->firmware_id);
fw_req->timedout = true; diff --git a/drivers/staging/greybus/fw-management.c b/drivers/staging/greybus/fw-management.c index 5d01da6b6eb9..ee86ed9686c4 100644 --- a/drivers/staging/greybus/fw-management.c +++ b/drivers/staging/greybus/fw-management.c @@ -141,7 +141,7 @@ static int fw_mgmt_load_and_validate_operation(struct fw_mgmt *fw_mgmt, if (load_method != GB_FW_LOAD_METHOD_UNIPRO && load_method != GB_FW_LOAD_METHOD_INTERNAL) { dev_err(fw_mgmt->parent, - "invalid load-method (%d)\n", load_method); + "invalid load-method (%u)\n", load_method); return -EINVAL; }
diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c index 12185f7a982c..7616bcc23c6c 100644 --- a/drivers/staging/greybus/gpio.c +++ b/drivers/staging/greybus/gpio.c @@ -379,7 +379,7 @@ static int gb_gpio_request_handler(struct gb_operation *op)
event = request->payload; if (event->which > ggc->line_max) { - dev_err(dev, "invalid hw irq: %d\n", event->which); + dev_err(dev, "invalid hw irq: %u\n", event->which); return -EINVAL; }
diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c index cab02b5da867..1fbd80523d4e 100644 --- a/drivers/staging/greybus/light.c +++ b/drivers/staging/greybus/light.c @@ -1221,7 +1221,7 @@ static int gb_lights_request_handler(struct gb_operation *op)
if (light_id >= glights->lights_count || !glights->lights[light_id].ready) { - dev_err(dev, "Event received for unconfigured light id: %d\n", + dev_err(dev, "Event received for unconfigured light id: %u\n", light_id); return -EINVAL; } diff --git a/drivers/staging/greybus/log.c b/drivers/staging/greybus/log.c index 815d20ba9b6b..24f5d5cf21be 100644 --- a/drivers/staging/greybus/log.c +++ b/drivers/staging/greybus/log.c @@ -36,7 +36,7 @@ static int gb_log_request_handler(struct gb_operation *op) receive = op->request->payload; len = le16_to_cpu(receive->len); if (len != (op->request->payload_size - sizeof(*receive))) { - dev_err(dev, "log request wrong size %d vs %zu\n", len, + dev_err(dev, "log request wrong size %u vs %zu\n", len, (op->request->payload_size - sizeof(*receive))); return -EINVAL; } @@ -46,7 +46,7 @@ static int gb_log_request_handler(struct gb_operation *op) }
if (len > GB_LOG_MAX_LEN) { - dev_err(dev, "log request too big: %d\n", len); + dev_err(dev, "log request too big: %u\n", len); return -EINVAL; }
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index efc69c10cffe..d06402dd3f60 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -424,7 +424,7 @@ static void gb_loopback_async_operation_callback(struct gb_operation *operation)
mutex_unlock(&gb->mutex);
- dev_dbg(&gb->connection->bundle->dev, "complete operation %d\n", + dev_dbg(&gb->connection->bundle->dev, "complete operation %u\n", operation->id);
/* Wake up waiters */ @@ -579,7 +579,7 @@ static int gb_loopback_async_transfer_complete(struct gb_loopback_async_operatio
if (memcmp(request->data, response->data, len)) { dev_err(&gb->connection->bundle->dev, - "Loopback Data doesn't match operation id %d\n", + "Loopback Data doesn't match operation id %u\n", operation->id); retval = -EREMOTEIO; } else { diff --git a/drivers/staging/greybus/power_supply.c b/drivers/staging/greybus/power_supply.c index beefcbaf3681..1eeccf452eaf 100644 --- a/drivers/staging/greybus/power_supply.c +++ b/drivers/staging/greybus/power_supply.c @@ -1011,7 +1011,7 @@ static int gb_supplies_request_handler(struct gb_operation *op) if (psy_id >= supplies->supplies_count || !supplies->supply[psy_id].registered) { dev_err(&connection->bundle->dev, - "Event received for unconfigured power_supply id: %d\n", + "Event received for unconfigured power_supply id: %u\n", psy_id); ret = -EINVAL; goto out_unlock; diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c index 4f1b3f4db404..2314496a858a 100644 --- a/drivers/staging/greybus/raw.c +++ b/drivers/staging/greybus/raw.c @@ -112,7 +112,7 @@ static int gb_raw_request_handler(struct gb_operation *op) receive = op->request->payload; len = le32_to_cpu(receive->len); if (len != (int)(op->request->payload_size - sizeof(__le32))) { - dev_err(dev, "raw receive request wrong size %d vs %d\n", len, + dev_err(dev, "raw receive request wrong size %u vs %d\n", len, (int)(op->request->payload_size - sizeof(__le32))); return -EINVAL; } diff --git a/drivers/staging/greybus/sdio.c b/drivers/staging/greybus/sdio.c index 3952f3d225db..773ec1f72407 100644 --- a/drivers/staging/greybus/sdio.c +++ b/drivers/staging/greybus/sdio.c @@ -294,7 +294,7 @@ static int _gb_sdio_send(struct gb_sdio_host *host, struct mmc_data *data, send_blksz = le16_to_cpu(response->data_blksz);
if (len != send_blksz * send_blocks) { - dev_err(mmc_dev(host->mmc), "send: size received: %zu != %d\n", + dev_err(mmc_dev(host->mmc), "send: size received: %zu != %u\n", len, send_blksz * send_blocks); ret = -EINVAL; } @@ -340,7 +340,7 @@ static int _gb_sdio_recv(struct gb_sdio_host *host, struct mmc_data *data, recv_blksz = le16_to_cpu(response->data_blksz);
if (len != recv_blksz * recv_blocks) { - dev_err(mmc_dev(host->mmc), "recv: size received: %d != %zu\n", + dev_err(mmc_dev(host->mmc), "recv: size received: %u != %zu\n", recv_blksz * recv_blocks, len); ret = -EINVAL; goto err_put_operation; diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c index c1982f3fb931..6e34757d9051 100644 --- a/drivers/staging/greybus/spilib.c +++ b/drivers/staging/greybus/spilib.c @@ -540,7 +540,7 @@ int gb_spilib_master_init(struct gb_connection *connection, struct device *dev, for (i = 0; i < spi->num_chipselect; i++) { ret = gb_spi_setup_device(spi, i); if (ret < 0) { - dev_err(dev, "failed to allocate spi device %d: %d\n", + dev_err(dev, "failed to allocate spi device %u: %d\n", i, ret); goto exit_spi_unregister; } diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c index 2448fc9098d6..442aad685d33 100644 --- a/drivers/staging/greybus/uart.c +++ b/drivers/staging/greybus/uart.c @@ -176,7 +176,7 @@ static int gb_uart_receive_credits_handler(struct gb_operation *op)
if (ret) { dev_err(&gb_tty->gbphy_dev->dev, - "invalid number of incoming credits: %d\n", + "invalid number of incoming credits: %u\n", incoming_credits); return ret; } @@ -300,7 +300,7 @@ static int send_break(struct gb_tty *gb_tty, u8 state)
if ((state != 0) && (state != 1)) { dev_err(&gb_tty->gbphy_dev->dev, - "invalid break state of %d\n", state); + "invalid break state of %u\n", state); return -EINVAL; }
On 10/21/26 11:34 AM, pavankumaryalagada@gmail.com wrote:
From: Yalagada Pavan Kumar pavankumaryalagada@gmail.com
Unsigned variables are printed using %d. Replace them with %u to match their types.
Signed-off-by: Yalagada Pavan Kumar pavankumaryalagada@gmail.com
I have reviewed every one of these and they look good. Quite a few of them--because the size of the value being formatted is less than 32 bits--would print correctly no matter what their value. But I think it at least does no harm to convert the format specifier to unsigned in all of these cases.
I have highlighted below an existing bug that should be fixed. It gives an example of something to look for when you do this sort of activity. Please arrange to get that bug fixed.
I also pointed out one other observation for you to think about.
Thanks for sending this. I didn't check to find any other instances you might have missed, but you seem to have found quite a few.
Reviewed-by: Alex Elder elder@riscstar.com
drivers/staging/greybus/audio_codec.c | 20 ++++++++-------- drivers/staging/greybus/audio_topology.c | 30 ++++++++++++------------ drivers/staging/greybus/bootrom.c | 2 +- drivers/staging/greybus/fw-download.c | 2 +- drivers/staging/greybus/fw-management.c | 2 +- drivers/staging/greybus/gpio.c | 2 +- drivers/staging/greybus/light.c | 2 +- drivers/staging/greybus/log.c | 4 ++-- drivers/staging/greybus/loopback.c | 4 ++-- drivers/staging/greybus/power_supply.c | 2 +- drivers/staging/greybus/raw.c | 2 +- drivers/staging/greybus/sdio.c | 4 ++-- drivers/staging/greybus/spilib.c | 2 +- drivers/staging/greybus/uart.c | 4 ++-- 14 files changed, 41 insertions(+), 41 deletions(-)
. . .
diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index 83921d90c322..fb1fbd3f7b99 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -222,7 +222,7 @@ static int gb_bootrom_firmware_size_request(struct gb_operation *op) size_response = op->response->payload; size_response->size = cpu_to_le32(bootrom->fw->size);
- dev_dbg(dev, "%s: firmware size %d bytes\n",
- dev_dbg(dev, "%s: firmware size %u bytes\n", __func__, size_response->size);
This line actually contains an error (and it was there before).
The type of gb_bootrom_firmware_size_response->size is __le32. So the value being formatted needs to be converted into CPU byte order before being used. That could be done this way: le32_to_cpu(size_response->size) or just use this, which is known from a few lines above to have the proper value and byte order: bootrom->fw->size
Please add this to your list of things to fix, and submit it as a separate patch. This would be a bug fix, which would also give you a little bit of a new experience.
unlock:
. . .
diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c index 4f1b3f4db404..2314496a858a 100644 --- a/drivers/staging/greybus/raw.c +++ b/drivers/staging/greybus/raw.c @@ -112,7 +112,7 @@ static int gb_raw_request_handler(struct gb_operation *op) receive = op->request->payload; len = le32_to_cpu(receive->len); if (len != (int)(op->request->payload_size - sizeof(__le32))) {
dev_err(dev, "raw receive request wrong size %d vs %d\n", len,
dev_err(dev, "raw receive request wrong size %u vs %d\n", len, (int)(op->request->payload_size - sizeof(__le32)));
The above size calculation was done somewhere else without the (int) cast, and the format specifier was %zu in that case. I don't think this needs to be changed necessarily, but it's something inconsistent across the Greybus code. Consistency helps readability and maintainability.
return -EINVAL;}
. . .
On Thu, 22 Oct 2026, Alex Elder wrote:
On 10/21/26 11:34 AM, pavankumaryalagada@gmail.com wrote:
From: Yalagada Pavan Kumar pavankumaryalagada@gmail.com
Unsigned variables are printed using %d. Replace them with %u to match their types.
Signed-off-by: Yalagada Pavan Kumar pavankumaryalagada@gmail.com
I have reviewed every one of these and they look good. Quite a few of them--because the size of the value being formatted is less than 32 bits--would print correctly no matter what their value. But I think it at least does no harm to convert the format specifier to unsigned in all of these cases.
I was suprised the compiler doesn't complain about this. It seems to complain a lot about type-related format specifiers. Yalagada maybe it could be worth adding something clearly invalid in one of the argument lists to see if the code is actually getting compiled, or if you need to set some config option to make that happen.
julia
I have highlighted below an existing bug that should be fixed. It gives an example of something to look for when you do this sort of activity. Please arrange to get that bug fixed.
I also pointed out one other observation for you to think about.
Thanks for sending this. I didn't check to find any other instances you might have missed, but you seem to have found quite a few.
Reviewed-by: Alex Elder elder@riscstar.com
drivers/staging/greybus/audio_codec.c | 20 ++++++++-------- drivers/staging/greybus/audio_topology.c | 30 ++++++++++++------------ drivers/staging/greybus/bootrom.c | 2 +- drivers/staging/greybus/fw-download.c | 2 +- drivers/staging/greybus/fw-management.c | 2 +- drivers/staging/greybus/gpio.c | 2 +- drivers/staging/greybus/light.c | 2 +- drivers/staging/greybus/log.c | 4 ++-- drivers/staging/greybus/loopback.c | 4 ++-- drivers/staging/greybus/power_supply.c | 2 +- drivers/staging/greybus/raw.c | 2 +- drivers/staging/greybus/sdio.c | 4 ++-- drivers/staging/greybus/spilib.c | 2 +- drivers/staging/greybus/uart.c | 4 ++-- 14 files changed, 41 insertions(+), 41 deletions(-)
. . .
diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index 83921d90c322..fb1fbd3f7b99 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -222,7 +222,7 @@ static int gb_bootrom_firmware_size_request(struct gb_operation *op) size_response = op->response->payload; size_response->size = cpu_to_le32(bootrom->fw->size);
- dev_dbg(dev, "%s: firmware size %d bytes\n",
- dev_dbg(dev, "%s: firmware size %u bytes\n", __func__, size_response->size);
This line actually contains an error (and it was there before).
The type of gb_bootrom_firmware_size_response->size is __le32. So the value being formatted needs to be converted into CPU byte order before being used. That could be done this way: le32_to_cpu(size_response->size) or just use this, which is known from a few lines above to have the proper value and byte order: bootrom->fw->size
Please add this to your list of things to fix, and submit it as a separate patch. This would be a bug fix, which would also give you a little bit of a new experience.
unlock:. . .
diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c index 4f1b3f4db404..2314496a858a 100644 --- a/drivers/staging/greybus/raw.c +++ b/drivers/staging/greybus/raw.c @@ -112,7 +112,7 @@ static int gb_raw_request_handler(struct gb_operation *op) receive = op->request->payload; len = le32_to_cpu(receive->len); if (len != (int)(op->request->payload_size - sizeof(__le32))) {
dev_err(dev, "raw receive request wrong size %d vs %d\n", len,
dev_err(dev, "raw receive request wrong size %u vs %d\n", len, (int)(op->request->payload_size - sizeof(__le32)));The above size calculation was done somewhere else without the (int) cast, and the format specifier was %zu in that case. I don't think this needs to be changed necessarily, but it's something inconsistent across the Greybus code. Consistency helps readability and maintainability.
return -EINVAL;}
. . .
On Thu, Oct 22, 2026 at 09:30:26AM -0400, Julia Lawall wrote:
On Thu, 22 Oct 2026, Alex Elder wrote:
On 10/21/26 11:34 AM, pavankumaryalagada@gmail.com wrote:
From: Yalagada Pavan Kumar pavankumaryalagada@gmail.com
Unsigned variables are printed using %d. Replace them with %u to match their types.
Signed-off-by: Yalagada Pavan Kumar pavankumaryalagada@gmail.com
I have reviewed every one of these and they look good. Quite a few of them--because the size of the value being formatted is less than 32 bits--would print correctly no matter what their value. But I think it at least does no harm to convert the format specifier to unsigned in all of these cases.
I was suprised the compiler doesn't complain about this. It seems to complain a lot about type-related format specifiers. Yalagada maybe it could be worth adding something clearly invalid in one of the argument lists to see if the code is actually getting compiled, or if you need to set some config option to make that happen.
The files compiled sucessfully with the normal build settings. I also enabled -Wformat-signedness to check for format mismatchse, as this check is not enabled by default. It revealed a few more issues, which i'll look into as well.
Pavan
julia
I have highlighted below an existing bug that should be fixed. It gives an example of something to look for when you do this sort of activity. Please arrange to get that bug fixed.
I also pointed out one other observation for you to think about.
Thanks for sending this. I didn't check to find any other instances you might have missed, but you seem to have found quite a few.
Reviewed-by: Alex Elder elder@riscstar.com
drivers/staging/greybus/audio_codec.c | 20 ++++++++-------- drivers/staging/greybus/audio_topology.c | 30 ++++++++++++------------ drivers/staging/greybus/bootrom.c | 2 +- drivers/staging/greybus/fw-download.c | 2 +- drivers/staging/greybus/fw-management.c | 2 +- drivers/staging/greybus/gpio.c | 2 +- drivers/staging/greybus/light.c | 2 +- drivers/staging/greybus/log.c | 4 ++-- drivers/staging/greybus/loopback.c | 4 ++-- drivers/staging/greybus/power_supply.c | 2 +- drivers/staging/greybus/raw.c | 2 +- drivers/staging/greybus/sdio.c | 4 ++-- drivers/staging/greybus/spilib.c | 2 +- drivers/staging/greybus/uart.c | 4 ++-- 14 files changed, 41 insertions(+), 41 deletions(-)
. . .
diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index 83921d90c322..fb1fbd3f7b99 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -222,7 +222,7 @@ static int gb_bootrom_firmware_size_request(struct gb_operation *op) size_response = op->response->payload; size_response->size = cpu_to_le32(bootrom->fw->size);
- dev_dbg(dev, "%s: firmware size %d bytes\n",
- dev_dbg(dev, "%s: firmware size %u bytes\n", __func__, size_response->size);
This line actually contains an error (and it was there before).
The type of gb_bootrom_firmware_size_response->size is __le32. So the value being formatted needs to be converted into CPU byte order before being used. That could be done this way: le32_to_cpu(size_response->size) or just use this, which is known from a few lines above to have the proper value and byte order: bootrom->fw->size
Please add this to your list of things to fix, and submit it as a separate patch. This would be a bug fix, which would also give you a little bit of a new experience.
unlock:. . .
diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c index 4f1b3f4db404..2314496a858a 100644 --- a/drivers/staging/greybus/raw.c +++ b/drivers/staging/greybus/raw.c @@ -112,7 +112,7 @@ static int gb_raw_request_handler(struct gb_operation *op) receive = op->request->payload; len = le32_to_cpu(receive->len); if (len != (int)(op->request->payload_size - sizeof(__le32))) {
dev_err(dev, "raw receive request wrong size %d vs %d\n", len,
dev_err(dev, "raw receive request wrong size %u vs %d\n", len, (int)(op->request->payload_size - sizeof(__le32)));The above size calculation was done somewhere else without the (int) cast, and the format specifier was %zu in that case. I don't think this needs to be changed necessarily, but it's something inconsistent across the Greybus code. Consistency helps readability and maintainability.
return -EINVAL;}
. . .
On 10/23/26 12:24 PM, Yalagada Pavan Kumar wrote:
On Thu, Oct 22, 2026 at 09:30:26AM -0400, Julia Lawall wrote:
On Thu, 22 Oct 2026, Alex Elder wrote:
On 10/21/26 11:34 AM,pavankumaryalagada@gmail.com wrote:
From: Yalagada Pavan Kumarpavankumaryalagada@gmail.com
Unsigned variables are printed using %d. Replace them with %u to match their types.
Signed-off-by: Yalagada Pavan Kumarpavankumaryalagada@gmail.com
I have reviewed every one of these and they look good. Quite a few of them--because the size of the value being formatted is less than 32 bits--would print correctly no matter what their value. But I think it at least does no harm to convert the format specifier to unsigned in all of these cases.
I was suprised the compiler doesn't complain about this. It seems to complain a lot about type-related format specifiers. Yalagada maybe it could be worth adding something clearly invalid in one of the argument lists to see if the code is actually getting compiled, or if you need to set some config option to make that happen.
The files compiled sucessfully with the normal build settings. I also enabled -Wformat-signedness to check for format mismatchse, as this check is not enabled by default. It revealed a few more issues, which i'll look into as well.
I think you might find that "sparse" complains. Run your build with "C=1" to run (by default) sparse, and see if it says something.
-Alex
Pavan
julia
Does anyone on the greybus list know anything about this person:
Bryan O'Donoghue pure.logic@nexus-software.ie
The email address is not valid, which is somewhat tiresome...
julia
On Thu, 22 Oct 2026, Alex Elder wrote:
On 10/21/26 11:34 AM, pavankumaryalagada@gmail.com wrote:
From: Yalagada Pavan Kumar pavankumaryalagada@gmail.com
Unsigned variables are printed using %d. Replace them with %u to match their types.
Signed-off-by: Yalagada Pavan Kumar pavankumaryalagada@gmail.com
I have reviewed every one of these and they look good. Quite a few of them--because the size of the value being formatted is less than 32 bits--would print correctly no matter what their value. But I think it at least does no harm to convert the format specifier to unsigned in all of these cases.
I have highlighted below an existing bug that should be fixed. It gives an example of something to look for when you do this sort of activity. Please arrange to get that bug fixed.
I also pointed out one other observation for you to think about.
Thanks for sending this. I didn't check to find any other instances you might have missed, but you seem to have found quite a few.
Reviewed-by: Alex Elder elder@riscstar.com
drivers/staging/greybus/audio_codec.c | 20 ++++++++-------- drivers/staging/greybus/audio_topology.c | 30 ++++++++++++------------ drivers/staging/greybus/bootrom.c | 2 +- drivers/staging/greybus/fw-download.c | 2 +- drivers/staging/greybus/fw-management.c | 2 +- drivers/staging/greybus/gpio.c | 2 +- drivers/staging/greybus/light.c | 2 +- drivers/staging/greybus/log.c | 4 ++-- drivers/staging/greybus/loopback.c | 4 ++-- drivers/staging/greybus/power_supply.c | 2 +- drivers/staging/greybus/raw.c | 2 +- drivers/staging/greybus/sdio.c | 4 ++-- drivers/staging/greybus/spilib.c | 2 +- drivers/staging/greybus/uart.c | 4 ++-- 14 files changed, 41 insertions(+), 41 deletions(-)
. . .
diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index 83921d90c322..fb1fbd3f7b99 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -222,7 +222,7 @@ static int gb_bootrom_firmware_size_request(struct gb_operation *op) size_response = op->response->payload; size_response->size = cpu_to_le32(bootrom->fw->size);
- dev_dbg(dev, "%s: firmware size %d bytes\n",
- dev_dbg(dev, "%s: firmware size %u bytes\n", __func__, size_response->size);
This line actually contains an error (and it was there before).
The type of gb_bootrom_firmware_size_response->size is __le32. So the value being formatted needs to be converted into CPU byte order before being used. That could be done this way: le32_to_cpu(size_response->size) or just use this, which is known from a few lines above to have the proper value and byte order: bootrom->fw->size
Please add this to your list of things to fix, and submit it as a separate patch. This would be a bug fix, which would also give you a little bit of a new experience.
unlock:. . .
diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c index 4f1b3f4db404..2314496a858a 100644 --- a/drivers/staging/greybus/raw.c +++ b/drivers/staging/greybus/raw.c @@ -112,7 +112,7 @@ static int gb_raw_request_handler(struct gb_operation *op) receive = op->request->payload; len = le32_to_cpu(receive->len); if (len != (int)(op->request->payload_size - sizeof(__le32))) {
dev_err(dev, "raw receive request wrong size %d vs %d\n", len,
dev_err(dev, "raw receive request wrong size %u vs %d\n", len, (int)(op->request->payload_size - sizeof(__le32)));The above size calculation was done somewhere else without the (int) cast, and the format specifier was %zu in that case. I don't think this needs to be changed necessarily, but it's something inconsistent across the Greybus code. Consistency helps readability and maintainability.
return -EINVAL;}
. . .
On 10/22/26 8:35 AM, Julia Lawall wrote:
Does anyone on the greybus list know anything about this person:
Bryan O'Donoghue pure.logic@nexus-software.ie
The email address is not valid, which is somewhat tiresome...
Bryan was on the Greybus kernel team. Remove that address from the list. I'll let him know that e-mail is bouncing so he can add himself back.
-Alex
julia
On Thu, 22 Oct 2026, Alex Elder wrote:
On 10/21/26 11:34 AM, pavankumaryalagada@gmail.com wrote:
From: Yalagada Pavan Kumar pavankumaryalagada@gmail.com
Unsigned variables are printed using %d. Replace them with %u to match their types.
Signed-off-by: Yalagada Pavan Kumar pavankumaryalagada@gmail.com
I have reviewed every one of these and they look good. Quite a few of them--because the size of the value being formatted is less than 32 bits--would print correctly no matter what their value. But I think it at least does no harm to convert the format specifier to unsigned in all of these cases.
I have highlighted below an existing bug that should be fixed. It gives an example of something to look for when you do this sort of activity. Please arrange to get that bug fixed.
I also pointed out one other observation for you to think about.
Thanks for sending this. I didn't check to find any other instances you might have missed, but you seem to have found quite a few.
Reviewed-by: Alex Elder elder@riscstar.com
drivers/staging/greybus/audio_codec.c | 20 ++++++++-------- drivers/staging/greybus/audio_topology.c | 30 ++++++++++++------------ drivers/staging/greybus/bootrom.c | 2 +- drivers/staging/greybus/fw-download.c | 2 +- drivers/staging/greybus/fw-management.c | 2 +- drivers/staging/greybus/gpio.c | 2 +- drivers/staging/greybus/light.c | 2 +- drivers/staging/greybus/log.c | 4 ++-- drivers/staging/greybus/loopback.c | 4 ++-- drivers/staging/greybus/power_supply.c | 2 +- drivers/staging/greybus/raw.c | 2 +- drivers/staging/greybus/sdio.c | 4 ++-- drivers/staging/greybus/spilib.c | 2 +- drivers/staging/greybus/uart.c | 4 ++-- 14 files changed, 41 insertions(+), 41 deletions(-)
. . .
diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index 83921d90c322..fb1fbd3f7b99 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -222,7 +222,7 @@ static int gb_bootrom_firmware_size_request(struct gb_operation *op) size_response = op->response->payload; size_response->size = cpu_to_le32(bootrom->fw->size);
- dev_dbg(dev, "%s: firmware size %d bytes\n",
- dev_dbg(dev, "%s: firmware size %u bytes\n", __func__, size_response->size);
This line actually contains an error (and it was there before).
The type of gb_bootrom_firmware_size_response->size is __le32. So the value being formatted needs to be converted into CPU byte order before being used. That could be done this way: le32_to_cpu(size_response->size) or just use this, which is known from a few lines above to have the proper value and byte order: bootrom->fw->size
Please add this to your list of things to fix, and submit it as a separate patch. This would be a bug fix, which would also give you a little bit of a new experience.
unlock:. . .
diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c index 4f1b3f4db404..2314496a858a 100644 --- a/drivers/staging/greybus/raw.c +++ b/drivers/staging/greybus/raw.c @@ -112,7 +112,7 @@ static int gb_raw_request_handler(struct gb_operation *op) receive = op->request->payload; len = le32_to_cpu(receive->len); if (len != (int)(op->request->payload_size - sizeof(__le32))) {
dev_err(dev, "raw receive request wrong size %d vs %d\n", len,
dev_err(dev, "raw receive request wrong size %u vs %d\n", len, (int)(op->request->payload_size - sizeof(__le32)));The above size calculation was done somewhere else without the (int) cast, and the format specifier was %zu in that case. I don't think this needs to be changed necessarily, but it's something inconsistent across the Greybus code. Consistency helps readability and maintainability.
return -EINVAL; }. . .
On Thu, 22 Oct 2026, Alex Elder wrote:
On 10/22/26 8:35 AM, Julia Lawall wrote:
Does anyone on the greybus list know anything about this person:
Bryan O'Donoghue pure.logic@nexus-software.ie
The email address is not valid, which is somewhat tiresome...
Bryan was on the Greybus kernel team. Remove that address from the list. I'll let him know that e-mail is bouncing so he can add himself back.
Actually, the email comes from the MAINTAINERS file.
julia
-Alexjulia
On Thu, 22 Oct 2026, Alex Elder wrote:
On 10/21/26 11:34 AM, pavankumaryalagada@gmail.com wrote:
From: Yalagada Pavan Kumar pavankumaryalagada@gmail.com
Unsigned variables are printed using %d. Replace them with %u to match their types.
Signed-off-by: Yalagada Pavan Kumar pavankumaryalagada@gmail.com
I have reviewed every one of these and they look good. Quite a few of them--because the size of the value being formatted is less than 32 bits--would print correctly no matter what their value. But I think it at least does no harm to convert the format specifier to unsigned in all of these cases.
I have highlighted below an existing bug that should be fixed. It gives an example of something to look for when you do this sort of activity. Please arrange to get that bug fixed.
I also pointed out one other observation for you to think about.
Thanks for sending this. I didn't check to find any other instances you might have missed, but you seem to have found quite a few.
Reviewed-by: Alex Elder elder@riscstar.com
drivers/staging/greybus/audio_codec.c | 20 ++++++++-------- drivers/staging/greybus/audio_topology.c | 30 ++++++++++++------------ drivers/staging/greybus/bootrom.c | 2 +- drivers/staging/greybus/fw-download.c | 2 +- drivers/staging/greybus/fw-management.c | 2 +- drivers/staging/greybus/gpio.c | 2 +- drivers/staging/greybus/light.c | 2 +- drivers/staging/greybus/log.c | 4 ++-- drivers/staging/greybus/loopback.c | 4 ++-- drivers/staging/greybus/power_supply.c | 2 +- drivers/staging/greybus/raw.c | 2 +- drivers/staging/greybus/sdio.c | 4 ++-- drivers/staging/greybus/spilib.c | 2 +- drivers/staging/greybus/uart.c | 4 ++-- 14 files changed, 41 insertions(+), 41 deletions(-)
. . .
diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index 83921d90c322..fb1fbd3f7b99 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -222,7 +222,7 @@ static int gb_bootrom_firmware_size_request(struct gb_operation *op) size_response = op->response->payload; size_response->size = cpu_to_le32(bootrom->fw->size);
- dev_dbg(dev, "%s: firmware size %d bytes\n",
- dev_dbg(dev, "%s: firmware size %u bytes\n", __func__, size_response->size);
This line actually contains an error (and it was there before).
The type of gb_bootrom_firmware_size_response->size is __le32. So the value being formatted needs to be converted into CPU byte order before being used. That could be done this way: le32_to_cpu(size_response->size) or just use this, which is known from a few lines above to have the proper value and byte order: bootrom->fw->size
Please add this to your list of things to fix, and submit it as a separate patch. This would be a bug fix, which would also give you a little bit of a new experience.
unlock:. . .
diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c index 4f1b3f4db404..2314496a858a 100644 --- a/drivers/staging/greybus/raw.c +++ b/drivers/staging/greybus/raw.c @@ -112,7 +112,7 @@ static int gb_raw_request_handler(struct gb_operation *op) receive = op->request->payload; len = le32_to_cpu(receive->len); if (len != (int)(op->request->payload_size - sizeof(__le32))) {
dev_err(dev, "raw receive request wrong size %d vs %d\n", len,
dev_err(dev, "raw receive request wrong size %u vs %d\n", len, (int)(op->request->payload_size -sizeof(__le32)));
The above size calculation was done somewhere else without the (int) cast, and the format specifier was %zu in that case. I don't think this needs to be changed necessarily, but it's something inconsistent across the Greybus code. Consistency helps readability and maintainability.
return -EINVAL; }. . .
On Thu, Oct 22, 2026 at 09:45:51AM -0400, Julia Lawall wrote:
On Thu, 22 Oct 2026, Alex Elder wrote:
On 10/22/26 8:35 AM, Julia Lawall wrote:
Does anyone on the greybus list know anything about this person:
Bryan O'Donoghue pure.logic@nexus-software.ie
The email address is not valid, which is somewhat tiresome...
Bryan was on the Greybus kernel team. Remove that address from the list. I'll let him know that e-mail is bouncing so he can add himself back.
Actually, the email comes from the MAINTAINERS file.
Hmm. I haven't seen any such bounces (and I CC:ed that address a couple of hours ago).
Bryan, is your nexus-software address still valid?
Johan
On 22/10/2026 14:52, Johan Hovold wrote:
On Thu, Oct 22, 2026 at 09:45:51AM -0400, Julia Lawall wrote:
On Thu, 22 Oct 2026, Alex Elder wrote:
On 10/22/26 8:35 AM, Julia Lawall wrote:
Does anyone on the greybus list know anything about this person:
Bryan O'Donoghue pure.logic@nexus-software.ie
The email address is not valid, which is somewhat tiresome...
Bryan was on the Greybus kernel team. Remove that address from the list. I'll let him know that e-mail is bouncing so he can add himself back.
Actually, the email comes from the MAINTAINERS file.
Hmm. I haven't seen any such bounces (and I CC:ed that address a couple of hours ago).
Bryan, is your nexus-software address still valid?
Johan
Not at all.
20261023022913.3763777-1-bod@kernel.org
--- bod
-
Alex Elder -
Bryan O'Donoghue -
Johan Hovold -
Julia Lawall -
pavankumaryalagadaï¼ gmail.com -
Yalagada Pavan Kumar