Refactor gb_audio_gb_get_topology() into separate calls for better modularity.
Signed-off-by: Madhumitha Prabakaran madhumithabiw@gmail.com --- drivers/staging/greybus/audio_gb.c | 67 +++++++++++++++++++----------- 1 file changed, 42 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/greybus/audio_gb.c b/drivers/staging/greybus/audio_gb.c index 9d8994fdb41a..a48ddadd6f1e 100644 --- a/drivers/staging/greybus/audio_gb.c +++ b/drivers/staging/greybus/audio_gb.c @@ -8,39 +8,56 @@ #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) +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; - int ret; + struct gb_audio_get_topology_size_response size_resp; + int ret;
- ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY_SIZE, - NULL, 0, &size_resp, sizeof(size_resp)); - if (ret) - return 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; + *size = le16_to_cpu(size_resp.size); + return 0; +}
- topo = kzalloc(size, GFP_KERNEL); - if (!topo) - return -ENOMEM; +struct gb_audio_topology *gb_audio_gb_alloc_topology(u16 size) +{ + struct gb_audio_topology *topo;
- ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY, NULL, 0, - topo, size); - if (ret) { - kfree(topo); - return ret; - } + if (size < sizeof(struct gb_audio_topology)) + return NULL;
- *topology = topo; + topo = kzalloc(size, GFP_KERNEL); + return 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; + + *topology = gb_audio_gb_alloc_topology(size); + if (!*topology) + return -ENOMEM; + + ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY, + NULL, 0, *topology, size); + if (ret) { + kfree(*topology); + *topology = NULL; + return ret; + } + + return 0; } + EXPORT_SYMBOL_GPL(gb_audio_gb_get_topology);
int gb_audio_gb_get_control(struct gb_connection *connection,
Hi Madhumitha,
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/Madhumitha-Prabakaran/staging... base: staging/staging-testing patch link: https://lore.kernel.org/r/20230804203134.GA618419%40madhu-kernel patch subject: [PATCH] staging: greybus: Refactor gb_audio_gb_get_topology() into separate calls config: loongarch-allyesconfig (https://download.01.org/0day-ci/archive/20230805/202308050511.y5Yb9otW-lkp@i...) compiler: loongarch64-linux-gcc (GCC) 12.3.0 reproduce: (https://download.01.org/0day-ci/archive/20230805/202308050511.y5Yb9otW-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/202308050511.y5Yb9otW-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/staging/greybus/audio_gb.c:11:5: warning: no previous prototype for 'gb_audio_gb_get_topology_size' [-Wmissing-prototypes]
11 | int gb_audio_gb_get_topology_size(struct gb_connection *connection, u16 *size) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/staging/greybus/audio_gb.c:25:27: warning: no previous prototype for 'gb_audio_gb_alloc_topology' [-Wmissing-prototypes]
25 | struct gb_audio_topology *gb_audio_gb_alloc_topology(u16 size) | ^~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/gb_audio_gb_get_topology_size +11 drivers/staging/greybus/audio_gb.c
10
11 int gb_audio_gb_get_topology_size(struct gb_connection *connection, u16 *size)
12 { 13 struct gb_audio_get_topology_size_response size_resp; 14 int ret; 15 16 ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY_SIZE, 17 NULL, 0, &size_resp, sizeof(size_resp)); 18 if (ret) 19 return ret; 20 21 *size = le16_to_cpu(size_resp.size); 22 return 0; 23 } 24
25 struct gb_audio_topology *gb_audio_gb_alloc_topology(u16 size)
26 { 27 struct gb_audio_topology *topo; 28 29 if (size < sizeof(struct gb_audio_topology)) 30 return NULL; 31 32 topo = kzalloc(size, GFP_KERNEL); 33 return topo; 34 } 35
On Fri, Aug 04, 2023 at 03:31:34PM -0500, Madhumitha Prabakaran wrote:
Refactor gb_audio_gb_get_topology() into separate calls for better modularity.
This is too vague. Just say "There is a comment which says 'Split into separate calls' so I have done it." But actually, please just delete the comment instead. This code is already an endless series of wrappers around wrappers.
Also, please run your patch through scripts/checkpatch.pl.
Btw, I just want to emphasize again that I was 100% serious when I asked you to delete the comment.
regards, dan carpenter