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

[Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>]

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;
>