On Tue, Jun 30, 2026 at 10:16:02PM +0530, adi25charis@gmail.com wrote:
From: Aditya Chari S adi25charis@gmail.com
Split gb_audio_gb_get_topology() into two functions: gb_audio_gb_get_topology_size() to fetch the topology size, and gb_audio_gb_get_topology() to fetch the topology data into a caller-provided buffer. This moves buffer allocation out of the audio_gb protocol helper and into gb_audio_probe(), where it belongs, addressing a long-standing FIXME.
Signed-off-by: Aditya Chari S adi25charis@gmail.com
v2:
- Store size as size_t instead of u16, per Dan Carpenter.
- Move the size validation (size < sizeof(*topology)) out of gb_audio_gb_get_topology() and into gb_audio_probe(), before the kzalloc(), per Dan Carpenter.
- Fix dev_err() format strings so %d is no longer printed first; put it at the end of the message instead, per Dan Carpenter.
Compile-tested with `make M=drivers/staging/greybus modules`. All modified files (audio_codec.h, audio_gb.c, audio_module.c) compile without errors or warnings. checkpatch.pl --no-tree passes clean on all three files.
This v2 patch is built on top of the v1 patch which we're not going to apply. It needs to be folded into the v1 patch instead.
drivers/staging/greybus/audio_codec.h | 4 ++-- drivers/staging/greybus/audio_gb.c | 7 ++----- drivers/staging/greybus/audio_module.c | 12 +++++++++--- 3 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/greybus/audio_codec.h b/drivers/staging/greybus/audio_codec.h index be5a2a86b..b45cd257d 100644 --- a/drivers/staging/greybus/audio_codec.h +++ b/drivers/staging/greybus/audio_codec.h @@ -179,9 +179,9 @@ void gbaudio_unregister_module(struct gbaudio_module_info *module); /* protocol related */ int gb_audio_gb_get_topology_size(struct gb_connection *connection,
u16 *size);
size_t *size);int gb_audio_gb_get_topology(struct gb_connection *connection,
struct gb_audio_topology *topology, u16 size);
struct gb_audio_topology *topology, size_t size);int gb_audio_gb_get_control(struct gb_connection *connection, u8 control_id, u8 index, struct gb_audio_ctl_elem_value *value); diff --git a/drivers/staging/greybus/audio_gb.c b/drivers/staging/greybus/audio_gb.c index e6356643d..2e6f155d8 100644 --- a/drivers/staging/greybus/audio_gb.c +++ b/drivers/staging/greybus/audio_gb.c @@ -9,7 +9,7 @@ #include "audio_codec.h" int gb_audio_gb_get_topology_size(struct gb_connection *connection,
u16 *size)
size_t *size){ struct gb_audio_get_topology_size_response size_resp; int ret; @@ -26,11 +26,8 @@ int gb_audio_gb_get_topology_size(struct gb_connection *connection, EXPORT_SYMBOL_GPL(gb_audio_gb_get_topology_size); int gb_audio_gb_get_topology(struct gb_connection *connection,
struct gb_audio_topology *topology, u16 size)
struct gb_audio_topology *topology, size_t size){
- if (size < sizeof(*topology))
return -ENODATA;- return gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY, NULL, 0, topology, size);
} diff --git a/drivers/staging/greybus/audio_module.c b/drivers/staging/greybus/audio_module.c index 1163cf093..806533f03 100644 --- a/drivers/staging/greybus/audio_module.c +++ b/drivers/staging/greybus/audio_module.c @@ -239,7 +239,7 @@ static int gb_audio_probe(struct gb_bundle *bundle, struct gb_audio_manager_module_descriptor desc; struct gbaudio_data_connection *dai, *_dai; int ret, i;
- u16 size;
- size_t size; struct gb_audio_topology *topology;
/* There should be at least one Management and one Data cport */ @@ -307,7 +307,13 @@ static int gb_audio_probe(struct gb_bundle *bundle, ret = gb_audio_gb_get_topology_size(gbmodule->mgmt_connection, &size); if (ret) {
dev_err(dev, "%d:Error while fetching topology size\n", ret);
dev_err(dev, "Error while fetching topology size: %d\n", ret);
Just leave this one as-is. It's from the original code. Although you added the word "size" so I guess that kind of makes changing it acceptable as well. The rule is you're allowed to make minor white space changes to line you're touching. So I guess either way is fine.
goto disable_connection;- }
- if (size < sizeof(*topology)) {
dev_err(dev, "Invalid topology size: %zu\n", size);ret = -ENODATA;
I would probably have said -EINVAL is more appropriate.
goto disable_connection;} @@ -319,7 +325,7 @@ static int gb_audio_probe(struct gb_bundle *bundle, ret = gb_audio_gb_get_topology(gbmodule->mgmt_connection, topology, size); if (ret) {
dev_err(dev, "%d:Error while fetching topology\n", ret);
dev_err(dev, "Error while fetching topology: %d\n", ret);
This change is entirely unrelated so it's not allowed.
regards, dan carpenter
goto free_topology;} -- 2.53.0