Re: [FIRST-PATCH] staging : greybus : gb-beagleplay.c : fixing the checks as first-patch

[Julia Lawall]

On Wed, 16 Apr 2025, Ayush Singh wrote:

> On 4/16/25 17:47, rujra wrote:
>
> > added comments on spinlocks for producer-consumer model, rearranged the
> > lines on function calls where it should not end with "(" this bracket,
> > also removed white-spaces and aligned the arguments of function calls.
>
> Are these manual adjustments, or using clang-format?
>
> I do not care about formatting being "readable". As long as it can be done by
> a tool like clang-format, that's fine with me.
>
> Of course if you are fixing some checkpatch error, that is okay, but if now,
> please avoid formatting changes.
>
> The comments are fine. Although you probably want to add a space between `//`
> and the sentence start.

I don't think the kernel commonly uses // for comments.

julia

>
>
> >
> > Signed-off-by: Rujra Bhatt <braker.noob.kernel@xxxxxxxxx>
> >
> > > 8------------------------------------------------------8<
> >   drivers/greybus/gb-beagleplay.c | 16 ++++++++--------
> >   1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/greybus/gb-beagleplay.c
> > b/drivers/greybus/gb-beagleplay.c
> > index 473ac3f2d382..fa1c3a40dd0b 100644
> > --- a/drivers/greybus/gb-beagleplay.c
> > +++ b/drivers/greybus/gb-beagleplay.c
> > @@ -73,7 +73,9 @@ struct gb_beagleplay {
> >          struct gb_host_device *gb_hd;
> >
> >          struct work_struct tx_work;
> > +       //used to ensure that only one producer can access the shared
> > resource at a time.
> >          spinlock_t tx_producer_lock;
> > +       //used to ensure that only one consumer can access the shared
> > resource at a time.
> >          spinlock_t tx_consumer_lock;
> >          struct circ_buf tx_circ_buf;
> >          u16 tx_crc;
> > @@ -642,8 +644,8 @@ static int cc1352_bootloader_wait_for_ack(struct
> > gb_beagleplay *bg)
> >   {
> >          int ret;
> >
> > -       ret = wait_for_completion_timeout(
> > -               &bg->fwl_ack_com,
> > msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT));
> > +       ret = wait_for_completion_timeout(&bg->fwl_ack_com,
> > +
> > msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT));
> >          if (ret < 0)
> >                  return dev_err_probe(&bg->sd->dev, ret,
> >                                       "Failed to acquire ack semaphore");
> > @@ -680,9 +682,8 @@ static int cc1352_bootloader_get_status(struct
> > gb_beagleplay *bg)
> >          if (ret < 0)
> >                  return ret;
> >
> > -       ret = wait_for_completion_timeout(
> > -               &bg->fwl_cmd_response_com,
> > -               msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT));
> > +       ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com,
> > +
> > msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT));
> >          if (ret < 0)
> >                  return dev_err_probe(&bg->sd->dev, ret,
> >                                       "Failed to acquire last status
> > semaphore");
> > @@ -765,9 +766,8 @@ static int cc1352_bootloader_crc32(struct
> > gb_beagleplay *bg, u32 *crc32)
> >          if (ret < 0)
> >                  return ret;
> >
> > -       ret = wait_for_completion_timeout(
> > -               &bg->fwl_cmd_response_com,
> > -               msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT));
> > +       ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com,
> > +
> > msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT));
> >          if (ret < 0)
> >                  return dev_err_probe(&bg->sd->dev, ret,
> >                                       "Failed to acquire last status
> > semaphore");
> > --
> > 2.43.0
>
>
> Best Regards,
>
> Ayush Singh
>
>
>