The FIXME in gb_audio_probe noted that memory allocation for the
topology should happen within the codec driver rather than the
greybus helper.

Move the size-check and kzalloc from audio_gb.c to audio_module.c
and update the function signature of gb_audio_gb_get_topology to
accept the pointer. This clarifies ownership of the allocated memory.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202602240844.4eT24iVh-lkp@intel.com/
Signed-off-by: Jose A. Perez de Azpillaga <azpijr@gmail.com>
---
v2:
 - Fixed build error by updating function prototype in audio_codec.h.
 - Fixed 'struct gb_audio_topology has no member named size' by passing
   size as an explicit argument to gb_audio_gb_get_topology().
---
 drivers/staging/greybus/audio_codec.h  |  2 +-
 drivers/staging/greybus/audio_gb.c     | 33 +++-----------------------
 drivers/staging/greybus/audio_module.c | 27 +++++++++++++++++----
 3 files changed, 26 insertions(+), 36 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.h b/drivers/staging/greybus/audio_codec.h
index f3f7a7ec6be4..2d7c3f928b1d 100644
--- a/drivers/staging/greybus/audio_codec.h
+++ b/drivers/staging/greybus/audio_codec.h
@@ -179,7 +179,7 @@ void gbaudio_unregister_module(struct gbaudio_module_info *module);

 /* protocol related */
 int gb_audio_gb_get_topology(struct gb_connection *connection,
-			     struct gb_audio_topology **topology);
+			     struct gb_audio_topology *topology, u16 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 9d8994fdb41a..8459a6d6f289 100644
--- a/drivers/staging/greybus/audio_gb.c
+++ b/drivers/staging/greybus/audio_gb.c
@@ -8,38 +8,11 @@
 #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)
+			     struct gb_audio_topology *topology, u16 size)
 {
-	struct gb_audio_get_topology_size_response size_resp;
-	struct gb_audio_topology *topo;
-	u16 size;
-	int ret;
-
-	ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY_SIZE,
-				NULL, 0, &size_resp, sizeof(size_resp));
-	if (ret)
-		return ret;
-
-	size = le16_to_cpu(size_resp.size);
-	if (size < sizeof(*topo))
-		return -ENODATA;
-
-	topo = kzalloc(size, GFP_KERNEL);
-	if (!topo)
-		return -ENOMEM;
-
-	ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY, NULL, 0,
-				topo, size);
-	if (ret) {
-		kfree(topo);
-		return ret;
-	}
-
-	*topology = topo;
-
-	return 0;
+	return gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY,
+			NULL, 0, topology, size);
 }
 EXPORT_SYMBOL_GPL(gb_audio_gb_get_topology);

diff --git a/drivers/staging/greybus/audio_module.c b/drivers/staging/greybus/audio_module.c
index 12c376c477b3..84c591b59b4e 100644
--- a/drivers/staging/greybus/audio_module.c
+++ b/drivers/staging/greybus/audio_module.c
@@ -240,6 +240,8 @@ static int gb_audio_probe(struct gb_bundle *bundle,
 	struct gbaudio_data_connection *dai, *_dai;
 	int ret, i;
 	struct gb_audio_topology *topology;
+	struct gb_audio_get_topology_size_response size_resp;
+	u16 size;

 	/* There should be at least one Management and one Data cport */
 	if (bundle->num_cports < 2)
@@ -304,13 +306,28 @@ static int gb_audio_probe(struct gb_bundle *bundle,
 	}
 	gbmodule->dev_id = gbmodule->mgmt_connection->intf->interface_id;

-	/*
-	 * FIXME: malloc for topology happens via audio_gb driver
-	 * should be done within codec driver itself
-	 */
-	ret = gb_audio_gb_get_topology(gbmodule->mgmt_connection, &topology);
+	ret = gb_operation_sync(gbmodule->mgmt_connection,
+				GB_AUDIO_TYPE_GET_TOPOLOGY_SIZE, NULL, 0,
+			&size_resp, sizeof(size_resp));
+	if (ret)
+		goto disable_connection;
+
+	size = le16_to_cpu(size_resp.size);
+	if (size < sizeof(*topology)) {
+		ret = -ENODATA;
+		goto disable_connection;
+	}
+
+	topology = kzalloc(size, GFP_KERNEL);
+	if (!topology) {
+		ret = -ENOMEM;
+		goto disable_connection;
+	}
+
+	ret = gb_audio_gb_get_topology(gbmodule->mgmt_connection, topology, size);
 	if (ret) {
 		dev_err(dev, "%d:Error while fetching topology\n", ret);
+		kfree(topology);
 		goto disable_connection;
 	}

--
2.53.0

On Tue, Feb 24, 2026 at 09:44:23AM +0100, Jose A. Perez de Azpillaga wrote:
> The FIXME in gb_audio_probe noted that memory allocation for the
> topology should happen within the codec driver rather than the
> greybus helper.
> 
> Move the size-check and kzalloc from audio_gb.c to audio_module.c
> and update the function signature of gb_audio_gb_get_topology to
> accept the pointer. This clarifies ownership of the allocated memory.
> 
> Reported-by: kernel test robot <lkp@intel.com>

The kernel test robot did not ask for this patch, it reported a problem
with your v1 patch.

> Closes: https://lore.kernel.org/oe-kbuild-all/202602240844.4eT24iVh-lkp@intel.com/

Nor does this change fix anything.


> Signed-off-by: Jose A. Perez de Azpillaga <azpijr@gmail.com>
> ---
> v2:
>  - Fixed build error by updating function prototype in audio_codec.h.
>  - Fixed 'struct gb_audio_topology has no member named size' by passing
>    size as an explicit argument to gb_audio_gb_get_topology().

Did you test this version as well?  But step back, why is this change
needed at all?

> ---
>  drivers/staging/greybus/audio_codec.h  |  2 +-
>  drivers/staging/greybus/audio_gb.c     | 33 +++-----------------------
>  drivers/staging/greybus/audio_module.c | 27 +++++++++++++++++----
>  3 files changed, 26 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/staging/greybus/audio_codec.h b/drivers/staging/greybus/audio_codec.h
> index f3f7a7ec6be4..2d7c3f928b1d 100644
> --- a/drivers/staging/greybus/audio_codec.h
> +++ b/drivers/staging/greybus/audio_codec.h
> @@ -179,7 +179,7 @@ void gbaudio_unregister_module(struct gbaudio_module_info *module);
> 
>  /* protocol related */
>  int gb_audio_gb_get_topology(struct gb_connection *connection,
> -			     struct gb_audio_topology **topology);
> +			     struct gb_audio_topology *topology, u16 size);

Adding a random new field to a function means that we need to look up
what that value is to try to figure out what is going on.  That makes
things much harder overall.  So did this make the code harder to
understand?

Why can't the size be determined automatically by this function?

thanks,

greg k-h