* [PATCH] staging: greybus: move topology allocation to codec probe
@ 2026-02-23 19:59 Jose A. Perez de Azpillaga
2026-02-24 0:14 ` kernel test robot
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jose A. Perez de Azpillaga @ 2026-02-23 19:59 UTC (permalink / raw)
To: vaibhav.sr, mgreer
Cc: johan, elder, gregkh, greybus-dev, linux-staging, linux-kernel
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.
Signed-off-by: Jose A. Perez de Azpillaga <azpijr@gmail.com>
---
drivers/staging/greybus/audio_gb.c | 33 +++-----------------------
drivers/staging/greybus/audio_module.c | 29 ++++++++++++++++++----
2 files changed, 27 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/greybus/audio_gb.c b/drivers/staging/greybus/audio_gb.c
index 9d8994fdb41a..4037b3bf1e9f 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)
{
- 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, le16_to_cpu(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..8efb720c3f30 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,30 @@ 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;
+ }
+
+ topology->size = cpu_to_le16(size);
+
+ ret = gb_audio_gb_get_topology(gbmodule->mgmt_connection, topology);
if (ret) {
dev_err(dev, "%d:Error while fetching topology\n", ret);
+ kfree(topology);
goto disable_connection;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: greybus: move topology allocation to codec probe
2026-02-23 19:59 [PATCH] staging: greybus: move topology allocation to codec probe Jose A. Perez de Azpillaga
@ 2026-02-24 0:14 ` kernel test robot
2026-02-24 0:25 ` kernel test robot
2026-02-24 8:44 ` [PATCH v2] " Jose A. Perez de Azpillaga
2 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2026-02-24 0:14 UTC (permalink / raw)
To: Jose A. Perez de Azpillaga, vaibhav.sr, mgreer
Cc: oe-kbuild-all, johan, elder, gregkh, greybus-dev, linux-staging,
linux-kernel
Hi Jose,
kernel test robot noticed the following build errors:
[auto build test ERROR on staging/staging-testing]
url: https://github.com/intel-lab-lkp/linux/commits/Jose-A-Perez-de-Azpillaga/staging-greybus-move-topology-allocation-to-codec-probe/20260224-040440
base: staging/staging-testing
patch link: https://lore.kernel.org/r/20260223195939.71151-1-azpijr%40gmail.com
patch subject: [PATCH] staging: greybus: move topology allocation to codec probe
config: parisc-randconfig-001-20260224 (https://download.01.org/0day-ci/archive/20260224/202602240844.4eT24iVh-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 10.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260224/202602240844.4eT24iVh-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602240844.4eT24iVh-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
drivers/staging/greybus/audio_module.c: In function 'gb_audio_probe':
>> drivers/staging/greybus/audio_module.c:327:10: error: 'struct gb_audio_topology' has no member named 'size'
327 | topology->size = cpu_to_le16(size);
| ^~
>> drivers/staging/greybus/audio_module.c:329:60: error: passing argument 2 of 'gb_audio_gb_get_topology' from incompatible pointer type [-Werror=incompatible-pointer-types]
329 | ret = gb_audio_gb_get_topology(gbmodule->mgmt_connection, topology);
| ^~~~~~~~
| |
| struct gb_audio_topology *
In file included from drivers/staging/greybus/audio_module.c:12:
drivers/staging/greybus/audio_codec.h:182:36: note: expected 'struct gb_audio_topology **' but argument is of type 'struct gb_audio_topology *'
182 | struct gb_audio_topology **topology);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~
cc1: some warnings being treated as errors
--
>> drivers/staging/greybus/audio_gb.c:11:5: error: conflicting types for 'gb_audio_gb_get_topology'
11 | int gb_audio_gb_get_topology(struct gb_connection *connection,
| ^~~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/staging/greybus/audio_gb.c:9:
drivers/staging/greybus/audio_codec.h:181:5: note: previous declaration of 'gb_audio_gb_get_topology' was here
181 | int gb_audio_gb_get_topology(struct gb_connection *connection,
| ^~~~~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/swab.h:5,
from include/uapi/linux/byteorder/big_endian.h:14,
from include/linux/byteorder/big_endian.h:5,
from arch/parisc/include/uapi/asm/byteorder.h:5,
from arch/parisc/include/asm/bitops.h:11,
from include/linux/bitops.h:67,
from include/linux/kernel.h:23,
from include/linux/greybus.h:14,
from drivers/staging/greybus/audio_gb.c:8:
drivers/staging/greybus/audio_gb.c: In function 'gb_audio_gb_get_topology':
>> drivers/staging/greybus/audio_gb.c:15:43: error: 'struct gb_audio_topology' has no member named 'size'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~
include/uapi/linux/swab.h:105:31: note: in definition of macro '__swab16'
105 | (__u16)(__builtin_constant_p(x) ? \
| ^
include/linux/byteorder/generic.h:91:21: note: in expansion of macro '__le16_to_cpu'
91 | #define le16_to_cpu __le16_to_cpu
| ^~~~~~~~~~~~~
drivers/staging/greybus/audio_gb.c:15:23: note: in expansion of macro 'le16_to_cpu'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~~~~~~~~~~
>> drivers/staging/greybus/audio_gb.c:15:43: error: 'struct gb_audio_topology' has no member named 'size'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~
include/uapi/linux/swab.h:106:2: note: in expansion of macro '___constant_swab16'
106 | ___constant_swab16(x) : \
| ^~~~~~~~~~~~~~~~~~
include/uapi/linux/byteorder/big_endian.h:37:26: note: in expansion of macro '__swab16'
37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
| ^~~~~~~~
include/linux/byteorder/generic.h:91:21: note: in expansion of macro '__le16_to_cpu'
91 | #define le16_to_cpu __le16_to_cpu
| ^~~~~~~~~~~~~
drivers/staging/greybus/audio_gb.c:15:23: note: in expansion of macro 'le16_to_cpu'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~~~~~~~~~~
>> drivers/staging/greybus/audio_gb.c:15:43: error: 'struct gb_audio_topology' has no member named 'size'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~
include/uapi/linux/swab.h:16:12: note: in definition of macro '___constant_swab16'
16 | (((__u16)(x) & (__u16)0xff00U) >> 8)))
| ^
include/uapi/linux/byteorder/big_endian.h:37:26: note: in expansion of macro '__swab16'
37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
| ^~~~~~~~
include/linux/byteorder/generic.h:91:21: note: in expansion of macro '__le16_to_cpu'
91 | #define le16_to_cpu __le16_to_cpu
| ^~~~~~~~~~~~~
drivers/staging/greybus/audio_gb.c:15:23: note: in expansion of macro 'le16_to_cpu'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~~~~~~~~~~
>> drivers/staging/greybus/audio_gb.c:15:43: error: 'struct gb_audio_topology' has no member named 'size'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~
include/uapi/linux/swab.h:107:12: note: in definition of macro '__swab16'
107 | __fswab16(x))
| ^
include/linux/byteorder/generic.h:91:21: note: in expansion of macro '__le16_to_cpu'
91 | #define le16_to_cpu __le16_to_cpu
| ^~~~~~~~~~~~~
drivers/staging/greybus/audio_gb.c:15:23: note: in expansion of macro 'le16_to_cpu'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~~~~~~~~~~
In file included from include/linux/linkage.h:7,
from include/linux/kernel.h:18,
from include/linux/greybus.h:14,
from drivers/staging/greybus/audio_gb.c:8:
drivers/staging/greybus/audio_gb.c: At top level:
drivers/staging/greybus/audio_gb.c:17:19: error: conflicting types for 'gb_audio_gb_get_topology'
17 | EXPORT_SYMBOL_GPL(gb_audio_gb_get_topology);
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/export.h:76:21: note: in definition of macro '__EXPORT_SYMBOL'
76 | extern typeof(sym) sym; \
| ^~~
include/linux/export.h:90:33: note: in expansion of macro '_EXPORT_SYMBOL'
90 | #define EXPORT_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "GPL")
| ^~~~~~~~~~~~~~
drivers/staging/greybus/audio_gb.c:17:1: note: in expansion of macro 'EXPORT_SYMBOL_GPL'
17 | EXPORT_SYMBOL_GPL(gb_audio_gb_get_topology);
| ^~~~~~~~~~~~~~~~~
In file included from drivers/staging/greybus/audio_gb.c:9:
drivers/staging/greybus/audio_codec.h:181:5: note: previous declaration of 'gb_audio_gb_get_topology' was here
181 | int gb_audio_gb_get_topology(struct gb_connection *connection,
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/staging/greybus/audio_gb.c: In function 'gb_audio_gb_get_topology':
>> drivers/staging/greybus/audio_gb.c:16:1: warning: control reaches end of non-void function [-Wreturn-type]
16 | }
| ^
vim +327 drivers/staging/greybus/audio_module.c
228
229 /*
230 * This is the basic hook get things initialized and registered w/ gb
231 */
232
233 static int gb_audio_probe(struct gb_bundle *bundle,
234 const struct greybus_bundle_id *id)
235 {
236 struct device *dev = &bundle->dev;
237 struct gbaudio_module_info *gbmodule;
238 struct greybus_descriptor_cport *cport_desc;
239 struct gb_audio_manager_module_descriptor desc;
240 struct gbaudio_data_connection *dai, *_dai;
241 int ret, i;
242 struct gb_audio_topology *topology;
243 struct gb_audio_get_topology_size_response size_resp;
244 u16 size;
245
246 /* There should be at least one Management and one Data cport */
247 if (bundle->num_cports < 2)
248 return -ENODEV;
249
250 /*
251 * There can be only one Management connection and any number of data
252 * connections.
253 */
254 gbmodule = devm_kzalloc(dev, sizeof(*gbmodule), GFP_KERNEL);
255 if (!gbmodule)
256 return -ENOMEM;
257
258 gbmodule->num_data_connections = bundle->num_cports - 1;
259 INIT_LIST_HEAD(&gbmodule->data_list);
260 INIT_LIST_HEAD(&gbmodule->widget_list);
261 INIT_LIST_HEAD(&gbmodule->ctl_list);
262 INIT_LIST_HEAD(&gbmodule->widget_ctl_list);
263 INIT_LIST_HEAD(&gbmodule->jack_list);
264 gbmodule->dev = dev;
265 snprintf(gbmodule->name, sizeof(gbmodule->name), "%s.%s", dev->driver->name,
266 dev_name(dev));
267 greybus_set_drvdata(bundle, gbmodule);
268
269 /* Create all connections */
270 for (i = 0; i < bundle->num_cports; i++) {
271 cport_desc = &bundle->cport_desc[i];
272
273 switch (cport_desc->protocol_id) {
274 case GREYBUS_PROTOCOL_AUDIO_MGMT:
275 ret = gb_audio_add_mgmt_connection(gbmodule, cport_desc,
276 bundle);
277 if (ret)
278 goto destroy_connections;
279 break;
280 case GREYBUS_PROTOCOL_AUDIO_DATA:
281 ret = gb_audio_add_data_connection(gbmodule, cport_desc,
282 bundle);
283 if (ret)
284 goto destroy_connections;
285 break;
286 default:
287 dev_err(dev, "Unsupported protocol: 0x%02x\n",
288 cport_desc->protocol_id);
289 ret = -ENODEV;
290 goto destroy_connections;
291 }
292 }
293
294 /* There must be a management cport */
295 if (!gbmodule->mgmt_connection) {
296 ret = -EINVAL;
297 dev_err(dev, "Missing management connection\n");
298 goto destroy_connections;
299 }
300
301 /* Initialize management connection */
302 ret = gb_connection_enable(gbmodule->mgmt_connection);
303 if (ret) {
304 dev_err(dev, "%d: Error while enabling mgmt connection\n", ret);
305 goto destroy_connections;
306 }
307 gbmodule->dev_id = gbmodule->mgmt_connection->intf->interface_id;
308
309 ret = gb_operation_sync(gbmodule->mgmt_connection,
310 GB_AUDIO_TYPE_GET_TOPOLOGY_SIZE, NULL, 0,
311 &size_resp, sizeof(size_resp));
312 if (ret)
313 goto disable_connection;
314
315 size = le16_to_cpu(size_resp.size);
316 if (size < sizeof(*topology)) {
317 ret = -ENODATA;
318 goto disable_connection;
319 }
320
321 topology = kzalloc(size, GFP_KERNEL);
322 if (!topology) {
323 ret = -ENOMEM;
324 goto disable_connection;
325 }
326
> 327 topology->size = cpu_to_le16(size);
328
> 329 ret = gb_audio_gb_get_topology(gbmodule->mgmt_connection, topology);
330 if (ret) {
331 dev_err(dev, "%d:Error while fetching topology\n", ret);
332 kfree(topology);
333 goto disable_connection;
334 }
335
336 /* process topology data */
337 ret = gbaudio_tplg_parse_data(gbmodule, topology);
338 if (ret) {
339 dev_err(dev, "%d:Error while parsing topology data\n",
340 ret);
341 goto free_topology;
342 }
343 gbmodule->topology = topology;
344
345 /* Initialize data connections */
346 list_for_each_entry(dai, &gbmodule->data_list, list) {
347 ret = gb_connection_enable(dai->connection);
348 if (ret) {
349 dev_err(dev,
350 "%d:Error while enabling %d:data connection\n",
351 ret, le16_to_cpu(dai->data_cport));
352 goto disable_data_connection;
353 }
354 }
355
356 /* register module with gbcodec */
357 ret = gbaudio_register_module(gbmodule);
358 if (ret)
359 goto disable_data_connection;
360
361 /* inform above layer for uevent */
362 dev_dbg(dev, "Inform set_event:%d to above layer\n", 1);
363 /* prepare for the audio manager */
364 strscpy(desc.name, gbmodule->name, sizeof(desc.name));
365 desc.vid = 2; /* todo */
366 desc.pid = 3; /* todo */
367 desc.intf_id = gbmodule->dev_id;
368 desc.op_devices = gbmodule->op_devices;
369 desc.ip_devices = gbmodule->ip_devices;
370 gbmodule->manager_id = gb_audio_manager_add(&desc);
371
372 dev_dbg(dev, "Add GB Audio device:%s\n", gbmodule->name);
373
374 gb_pm_runtime_put_autosuspend(bundle);
375
376 return 0;
377
378 disable_data_connection:
379 list_for_each_entry_safe(dai, _dai, &gbmodule->data_list, list)
380 gb_connection_disable(dai->connection);
381 gbaudio_tplg_release(gbmodule);
382 gbmodule->topology = NULL;
383
384 free_topology:
385 kfree(topology);
386
387 disable_connection:
388 gb_connection_disable(gbmodule->mgmt_connection);
389
390 destroy_connections:
391 list_for_each_entry_safe(dai, _dai, &gbmodule->data_list, list) {
392 gb_connection_destroy(dai->connection);
393 list_del(&dai->list);
394 devm_kfree(dev, dai);
395 }
396
397 if (gbmodule->mgmt_connection)
398 gb_connection_destroy(gbmodule->mgmt_connection);
399
400 devm_kfree(dev, gbmodule);
401
402 return ret;
403 }
404
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: greybus: move topology allocation to codec probe
2026-02-23 19:59 [PATCH] staging: greybus: move topology allocation to codec probe Jose A. Perez de Azpillaga
2026-02-24 0:14 ` kernel test robot
@ 2026-02-24 0:25 ` kernel test robot
2026-02-24 8:44 ` [PATCH v2] " Jose A. Perez de Azpillaga
2 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2026-02-24 0:25 UTC (permalink / raw)
To: Jose A. Perez de Azpillaga, vaibhav.sr, mgreer
Cc: oe-kbuild-all, johan, elder, gregkh, greybus-dev, linux-staging,
linux-kernel
Hi Jose,
kernel test robot noticed the following build errors:
[auto build test ERROR on staging/staging-testing]
url: https://github.com/intel-lab-lkp/linux/commits/Jose-A-Perez-de-Azpillaga/staging-greybus-move-topology-allocation-to-codec-probe/20260224-040440
base: staging/staging-testing
patch link: https://lore.kernel.org/r/20260223195939.71151-1-azpijr%40gmail.com
patch subject: [PATCH] staging: greybus: move topology allocation to codec probe
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20260224/202602240818.4lyiDRiD-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260224/202602240818.4lyiDRiD-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602240818.4lyiDRiD-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/staging/greybus/audio_gb.c:11:5: error: conflicting types for 'gb_audio_gb_get_topology'; have 'int(struct gb_connection *, struct gb_audio_topology *)'
11 | int gb_audio_gb_get_topology(struct gb_connection *connection,
| ^~~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/staging/greybus/audio_gb.c:9:
drivers/staging/greybus/audio_codec.h:181:5: note: previous declaration of 'gb_audio_gb_get_topology' with type 'int(struct gb_connection *, struct gb_audio_topology **)'
181 | int gb_audio_gb_get_topology(struct gb_connection *connection,
| ^~~~~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/swab.h:5,
from include/uapi/linux/byteorder/big_endian.h:14,
from include/linux/byteorder/big_endian.h:5,
from arch/m68k/include/uapi/asm/byteorder.h:5,
from include/asm-generic/bitops/le.h:6,
from arch/m68k/include/asm/bitops.h:569,
from include/linux/bitops.h:67,
from include/linux/kernel.h:23,
from include/linux/greybus.h:14,
from drivers/staging/greybus/audio_gb.c:8:
drivers/staging/greybus/audio_gb.c: In function 'gb_audio_gb_get_topology':
drivers/staging/greybus/audio_gb.c:15:64: error: 'struct gb_audio_topology' has no member named 'size'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~
include/uapi/linux/swab.h:105:38: note: in definition of macro '__swab16'
105 | (__u16)(__builtin_constant_p(x) ? \
| ^
include/linux/byteorder/generic.h:91:21: note: in expansion of macro '__le16_to_cpu'
91 | #define le16_to_cpu __le16_to_cpu
| ^~~~~~~~~~~~~
drivers/staging/greybus/audio_gb.c:15:44: note: in expansion of macro 'le16_to_cpu'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~~~~~~~~~~
drivers/staging/greybus/audio_gb.c:15:64: error: 'struct gb_audio_topology' has no member named 'size'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~
include/uapi/linux/swab.h:106:9: note: in expansion of macro '___constant_swab16'
106 | ___constant_swab16(x) : \
| ^~~~~~~~~~~~~~~~~~
include/uapi/linux/byteorder/big_endian.h:37:26: note: in expansion of macro '__swab16'
37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
| ^~~~~~~~
include/linux/byteorder/generic.h:91:21: note: in expansion of macro '__le16_to_cpu'
91 | #define le16_to_cpu __le16_to_cpu
| ^~~~~~~~~~~~~
drivers/staging/greybus/audio_gb.c:15:44: note: in expansion of macro 'le16_to_cpu'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~~~~~~~~~~
drivers/staging/greybus/audio_gb.c:15:64: error: 'struct gb_audio_topology' has no member named 'size'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~
include/uapi/linux/swab.h:16:19: note: in definition of macro '___constant_swab16'
16 | (((__u16)(x) & (__u16)0xff00U) >> 8)))
| ^
include/uapi/linux/byteorder/big_endian.h:37:26: note: in expansion of macro '__swab16'
37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
| ^~~~~~~~
include/linux/byteorder/generic.h:91:21: note: in expansion of macro '__le16_to_cpu'
91 | #define le16_to_cpu __le16_to_cpu
| ^~~~~~~~~~~~~
drivers/staging/greybus/audio_gb.c:15:44: note: in expansion of macro 'le16_to_cpu'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~~~~~~~~~~
drivers/staging/greybus/audio_gb.c:15:64: error: 'struct gb_audio_topology' has no member named 'size'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~
include/uapi/linux/swab.h:107:19: note: in definition of macro '__swab16'
107 | __fswab16(x))
| ^
include/linux/byteorder/generic.h:91:21: note: in expansion of macro '__le16_to_cpu'
91 | #define le16_to_cpu __le16_to_cpu
| ^~~~~~~~~~~~~
drivers/staging/greybus/audio_gb.c:15:44: note: in expansion of macro 'le16_to_cpu'
15 | NULL, 0, topology, le16_to_cpu(topology->size));
| ^~~~~~~~~~~
In file included from include/linux/linkage.h:7,
from include/linux/kernel.h:18:
drivers/staging/greybus/audio_gb.c: At top level:
drivers/staging/greybus/audio_gb.c:17:19: error: conflicting types for 'gb_audio_gb_get_topology'; have 'int(struct gb_connection *, struct gb_audio_topology *)'
17 | EXPORT_SYMBOL_GPL(gb_audio_gb_get_topology);
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/export.h:76:28: note: in definition of macro '__EXPORT_SYMBOL'
76 | extern typeof(sym) sym; \
| ^~~
include/linux/export.h:90:41: note: in expansion of macro '_EXPORT_SYMBOL'
90 | #define EXPORT_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "GPL")
| ^~~~~~~~~~~~~~
drivers/staging/greybus/audio_gb.c:17:1: note: in expansion of macro 'EXPORT_SYMBOL_GPL'
17 | EXPORT_SYMBOL_GPL(gb_audio_gb_get_topology);
| ^~~~~~~~~~~~~~~~~
drivers/staging/greybus/audio_codec.h:181:5: note: previous declaration of 'gb_audio_gb_get_topology' with type 'int(struct gb_connection *, struct gb_audio_topology **)'
181 | int gb_audio_gb_get_topology(struct gb_connection *connection,
| ^~~~~~~~~~~~~~~~~~~~~~~~
vim +11 drivers/staging/greybus/audio_gb.c
184992e305f1de Mark Greer 2016-01-13 10
184992e305f1de Mark Greer 2016-01-13 @11 int gb_audio_gb_get_topology(struct gb_connection *connection,
b554255ff0c003 Jose A. Perez de Azpillaga 2026-02-23 12 struct gb_audio_topology *topology)
184992e305f1de Mark Greer 2016-01-13 13 {
b554255ff0c003 Jose A. Perez de Azpillaga 2026-02-23 14 return gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY,
b554255ff0c003 Jose A. Perez de Azpillaga 2026-02-23 15 NULL, 0, topology, le16_to_cpu(topology->size));
184992e305f1de Mark Greer 2016-01-13 16 }
184992e305f1de Mark Greer 2016-01-13 17 EXPORT_SYMBOL_GPL(gb_audio_gb_get_topology);
184992e305f1de Mark Greer 2016-01-13 18
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] staging: greybus: move topology allocation to codec probe
2026-02-23 19:59 [PATCH] staging: greybus: move topology allocation to codec probe Jose A. Perez de Azpillaga
2026-02-24 0:14 ` kernel test robot
2026-02-24 0:25 ` kernel test robot
@ 2026-02-24 8:44 ` Jose A. Perez de Azpillaga
2026-02-24 17:58 ` Greg KH
2 siblings, 1 reply; 6+ messages in thread
From: Jose A. Perez de Azpillaga @ 2026-02-24 8:44 UTC (permalink / raw)
To: vaibhav.sr, mgreer
Cc: johan, elder, gregkh, greybus-dev, linux-staging, linux-kernel,
kernel test robot
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] staging: greybus: move topology allocation to codec probe
2026-02-24 8:44 ` [PATCH v2] " Jose A. Perez de Azpillaga
@ 2026-02-24 17:58 ` Greg KH
2026-02-25 10:16 ` Jose A. Perez de Azpillaga
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2026-02-24 17:58 UTC (permalink / raw)
To: Jose A. Perez de Azpillaga
Cc: vaibhav.sr, mgreer, johan, elder, greybus-dev, linux-staging,
linux-kernel, kernel test robot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] staging: greybus: move topology allocation to codec probe
2026-02-24 17:58 ` Greg KH
@ 2026-02-25 10:16 ` Jose A. Perez de Azpillaga
0 siblings, 0 replies; 6+ messages in thread
From: Jose A. Perez de Azpillaga @ 2026-02-25 10:16 UTC (permalink / raw)
To: gregkh; +Cc: azpijr, greybus-dev, linux-staging
Hi Greg,
Thank you for the feedback and the guidance.
You're absolutely right. I am new to kernel development, and in my
attempt to satisfy an old FIXME, I created a clunkier API and added
unnecessary complexity. The original implementation is much cleaner and
more efficient than the change I proposed.
I am withdrawing this patch. I'm sorry for the noise, and I appreciate
you taking the time to lead me in the right direction. I'll focus on
finding more meaningful improvements where the logic is sound.
Best regards, Jose
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-02-25 10:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-23 19:59 [PATCH] staging: greybus: move topology allocation to codec probe Jose A. Perez de Azpillaga
2026-02-24 0:14 ` kernel test robot
2026-02-24 0:25 ` kernel test robot
2026-02-24 8:44 ` [PATCH v2] " Jose A. Perez de Azpillaga
2026-02-24 17:58 ` Greg KH
2026-02-25 10:16 ` Jose A. Perez de Azpillaga
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox