[PATCH v2] staging: greybus: Align function arguments

[PATCH v2] staging: greybus: Align function arguments

[pavankumaryalagada]

From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>

Align the function arguments to follow the coding style and
improve readability, addressing the alignment warning reported by
checkpatch.pl

Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
---
v2:
 - Rebased onto staging-next.
v1: https://lore.kernel.org/outreachy/C560A15E-6F28-4FC8-8BE2-277B503340AD@inria.fr/T/#t

 drivers/staging/greybus/loopback.c | 3 ++-
 drivers/staging/greybus/spilib.c   | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index fe48a687377a..efc69c10cffe 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -652,7 +652,8 @@ static int gb_loopback_request_handler(struct gb_operation *operation)
 		}
 
 		if (!gb_operation_response_alloc(operation,
-						 len + sizeof(*response), GFP_KERNEL)) {
+						 len + sizeof(*response),
+						 GFP_KERNEL)) {
 			dev_err(dev, "error allocating response\n");
 			return -ENOMEM;
 		}
diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c
index e4d1ae8308aa..c1982f3fb931 100644
--- a/drivers/staging/greybus/spilib.c
+++ b/drivers/staging/greybus/spilib.c
@@ -160,7 +160,8 @@ static struct spi_transfer *get_next_xfer(struct spi_transfer *xfer,
 
 /* Routines to transfer data */
 static struct gb_operation *gb_spi_operation_create(struct gb_spilib *spi,
-		struct gb_connection *connection, struct spi_message *msg)
+						    struct gb_connection *connection,
+						    struct spi_message *msg)
 {
 	struct gb_spi_transfer_request *request;
 	struct spi_device *dev = msg->spi;
-- 
2.43.0

Re: [PATCH v2] staging: greybus: Align function arguments

[Alex Elder]

On 10/17/26 3:44 AM, pavankumaryalagada@gmail.com wrote:
> From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> 
> Align the function arguments to follow the coding style and
> improve readability, addressing the alignment warning reported by
> checkpatch.pl
> 
> Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>

This patch is OK, and it's a good kind of thing to do for
getting your feet wet with sending out patches for review.

But I encourage you to try to really *read* and understand
the code, and see if you can find anything you think is wrong,
or some bit of code or comments that you think could be more
clear.

I just scanned through "loopback.c" and one minor change (to
*code*) you could try is using fsleep() in one place that
uses other functions to delay.

Another thing you could look for is printf-style format
specifiers.  I see more than one place where an unsigned
value uses %d (which, for large values, produce misleading
results).

These suggestions aren't especially complex, but my hope
is they lead you further into understanding the code and
not relying heavily on checkpatch.pl to find things.

Anyway, this change looks fine to me.

Reviewed-by: Alex Elder <elder@riscstar.com>
> ---
> v2:
>   - Rebased onto staging-next.
> v1: https://lore.kernel.org/outreachy/C560A15E-6F28-4FC8-8BE2-277B503340AD@inria.fr/T/#t
> 
>   drivers/staging/greybus/loopback.c | 3 ++-
>   drivers/staging/greybus/spilib.c   | 3 ++-
>   2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> index fe48a687377a..efc69c10cffe 100644
> --- a/drivers/staging/greybus/loopback.c
> +++ b/drivers/staging/greybus/loopback.c
> @@ -652,7 +652,8 @@ static int gb_loopback_request_handler(struct gb_operation *operation)
>   		}
>   
>   		if (!gb_operation_response_alloc(operation,
> -						 len + sizeof(*response), GFP_KERNEL)) {
> +						 len + sizeof(*response),
> +						 GFP_KERNEL)) {
>   			dev_err(dev, "error allocating response\n");
>   			return -ENOMEM;
>   		}
> diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c
> index e4d1ae8308aa..c1982f3fb931 100644
> --- a/drivers/staging/greybus/spilib.c
> +++ b/drivers/staging/greybus/spilib.c
> @@ -160,7 +160,8 @@ static struct spi_transfer *get_next_xfer(struct spi_transfer *xfer,
>   
>   /* Routines to transfer data */
>   static struct gb_operation *gb_spi_operation_create(struct gb_spilib *spi,
> -		struct gb_connection *connection, struct spi_message *msg)
> +						    struct gb_connection *connection,
> +						    struct spi_message *msg)
>   {
>   	struct gb_spi_transfer_request *request;
>   	struct spi_device *dev = msg->spi;

Re: [PATCH v2] staging: greybus: Align function arguments

[Yalagada Pavan Kumar]

On Tue, Oct 20, 2026 at 09:00:03AM -0500, Alex Elder wrote:
> On 10/17/26 3:44 AM, pavankumaryalagada@gmail.com wrote:
> > From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> > 
> > Align the function arguments to follow the coding style and
> > improve readability, addressing the alignment warning reported by
> > checkpatch.pl
> > 
> > Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> 
> This patch is OK, and it's a good kind of thing to do for
> getting your feet wet with sending out patches for review.

Thank you for reviewing my patch.

> 
> But I encourage you to try to really *read* and understand
> the code, and see if you can find anything you think is wrong,
> or some bit of code or comments that you think could be more
> clear.
> 

I will try to understand the code better and look for ways to
improve it.

> I just scanned through "loopback.c" and one minor change (to
> *code*) you could try is using fsleep() in one place that
> uses other functions to delay.

I will look into this and work on it.

> 
> Another thing you could look for is printf-style format
> specifiers.  I see more than one place where an unsigned
> value uses %d (which, for large values, produce misleading
> results).

I will also check the format specifiers and make sure they
match the variable types.

> 
> These suggestions aren't especially complex, but my hope
> is they lead you further into understanding the code and
> not relying heavily on checkpatch.pl to find things.

I will keep these suggestions in mind when working on future
patches.

Thanks,
Pavan

> 
> Anyway, this change looks fine to me.
> 
> Reviewed-by: Alex Elder <elder@riscstar.com>
> > ---
> > v2:
> >   - Rebased onto staging-next.
> > v1: https://lore.kernel.org/outreachy/C560A15E-6F28-4FC8-8BE2-277B503340AD@inria.fr/T/#t
> > 
> >   drivers/staging/greybus/loopback.c | 3 ++-
> >   drivers/staging/greybus/spilib.c   | 3 ++-
> >   2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> > index fe48a687377a..efc69c10cffe 100644
> > --- a/drivers/staging/greybus/loopback.c
> > +++ b/drivers/staging/greybus/loopback.c
> > @@ -652,7 +652,8 @@ static int gb_loopback_request_handler(struct gb_operation *operation)
> >   		}
> >   		if (!gb_operation_response_alloc(operation,
> > -						 len + sizeof(*response), GFP_KERNEL)) {
> > +						 len + sizeof(*response),
> > +						 GFP_KERNEL)) {
> >   			dev_err(dev, "error allocating response\n");
> >   			return -ENOMEM;
> >   		}
> > diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c
> > index e4d1ae8308aa..c1982f3fb931 100644
> > --- a/drivers/staging/greybus/spilib.c
> > +++ b/drivers/staging/greybus/spilib.c
> > @@ -160,7 +160,8 @@ static struct spi_transfer *get_next_xfer(struct spi_transfer *xfer,
> >   /* Routines to transfer data */
> >   static struct gb_operation *gb_spi_operation_create(struct gb_spilib *spi,
> > -		struct gb_connection *connection, struct spi_message *msg)
> > +						    struct gb_connection *connection,
> > +						    struct spi_message *msg)
> >   {
> >   	struct gb_spi_transfer_request *request;
> >   	struct spi_device *dev = msg->spi;
>