[PATCH] staging: greybus: Refactor gb_audio_gb_get_topology() into separate calls

[PATCH] staging: greybus: Refactor gb_audio_gb_get_topology() into separate calls

[Madhumitha Prabakaran]

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,
-- 
2.25.1

Re: [PATCH] staging: greybus: Refactor gb_audio_gb_get_topology() into separate calls

[kernel test robot]

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-greybus-Refactor-gb_audio_gb_get_topology-into-separate-calls/20230805-043242
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@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230805/202308050511.y5Yb9otW-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/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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] staging: greybus: Refactor gb_audio_gb_get_topology() into separate calls

[Dan Carpenter]

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