Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver

[Shenwei Wang]

> -----Original Message-----
> From: Andrew Lunn <andrew@xxxxxxx>
> Sent: Thursday, February 19, 2026 9:25 AM
> To: Shenwei Wang <shenwei.wang@xxxxxxx>
> Cc: Arnaud POULIQUEN <arnaud.pouliquen@xxxxxxxxxxx>; Linus Walleij
> <linusw@xxxxxxxxxx>; Bartosz Golaszewski <brgl@xxxxxxxxxx>; Jonathan Corbet
> <corbet@xxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski
> <krzk+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>; Bjorn Andersson
> <andersson@xxxxxxxxxx>; Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>; Frank Li
> <frank.li@xxxxxxx>; Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>; Shuah Khan
> <skhan@xxxxxxxxxxxxxxxxxxx>; linux-gpio@xxxxxxxxxxxxxxx; linux-
> doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Pengutronix Kernel Team
> <kernel@xxxxxxxxxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>; Peng Fan
> <peng.fan@xxxxxxx>; devicetree@xxxxxxxxxxxxxxx; linux-
> remoteproc@xxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; Bartosz
> Golaszewski <brgl@xxxxxxxx>
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> On Thu, Feb 19, 2026 at 02:17:26PM +0000, Shenwei Wang wrote:
> >
> >
> > > -----Original Message-----
> > > From: Andrew Lunn <andrew@xxxxxxx>
> > > Sent: Thursday, February 19, 2026 7:27 AM
> > > To: Arnaud POULIQUEN <arnaud.pouliquen@xxxxxxxxxxx>
> > > Cc: Shenwei Wang <shenwei.wang@xxxxxxx>; Linus Walleij
> > > <linusw@xxxxxxxxxx>; Bartosz Golaszewski <brgl@xxxxxxxxxx>; Jonathan
> > > Corbet <corbet@xxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Krzysztof
> > > Kozlowski <krzk+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>;
> > > Bjorn Andersson <andersson@xxxxxxxxxx>; Mathieu Poirier
> > > <mathieu.poirier@xxxxxxxxxx>; Frank Li <frank.li@xxxxxxx>; Sascha
> > > Hauer <s.hauer@xxxxxxxxxxxxxx>; Shuah Khan
> > > <skhan@xxxxxxxxxxxxxxxxxxx>; linux-gpio@xxxxxxxxxxxxxxx; linux-
> > > doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Pengutronix
> > > Kernel Team <kernel@xxxxxxxxxxxxxx>; Fabio Estevam
> > > <festevam@xxxxxxxxx>; Peng Fan <peng.fan@xxxxxxx>;
> > > devicetree@xxxxxxxxxxxxxxx; linux- remoteproc@xxxxxxxxxxxxxxx;
> > > imx@xxxxxxxxxxxxxxx; linux-arm- kernel@xxxxxxxxxxxxxxxxxxx;
> > > dl-linux-imx <linux-imx@xxxxxxx>; Bartosz Golaszewski
> > > <brgl@xxxxxxxx>
> > > Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg
> > > GPIO driver
> > >
> > > > > +   if (sync) {
> > > > > +           err = wait_for_completion_timeout(&info->cmd_complete,
> > > > > +                                             msecs_to_jiffies(RPMSG_TIMEOUT));
> > > > > +           if (err == 0) {
> > > > > +                   dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
> > > > > +                   return -ETIMEDOUT;
> > > >
> > > > strange condition you return an error if err == 0, for redability use 'ret'
> > > > variable or simply:
> > > >
> > > >               if(!wait_for_completion_timeout(&info->cmd_complete,
> > > >                                 msecs_to_jiffies(RPMSG_TIMEOUT)) {
> > > >                       dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
> > > >                       return -ETIMEDOUT;
> > > >               }
> > >
> > > This will be from a comment i made. It appears that
> > > do_wait_for_common() can return -ERESTARTSYS. I assume that should
> > > be returned to user space?
> > >
> >
> > It looks like there might be a bit of confusion around what
> > wait_for_completion_timeout() actually returns. That function never
> > returns -ERESTARTSYS. Instead, its behavior is pretty
> > simple:
> >
> > - 0 means the wait timed out
> > - A positive value means the completion happened (the value is just
> > the remaining jiffies)
> >
> > So the driver returns the timeout error, and the upper application can
> > decide how it wants to handle that situation, for example restart or ignore.
> 
> wait_for_completion_timeout():
>         return wait_for_common(x, timeout, TASK_UNINTERRUPTIBLE);
> 
> wait_for_common():
>         return __wait_for_common(x, io_schedule_timeout, timeout, state);
> 
> __wait_for_common():
>         timeout = do_wait_for_common(x, action, timeout, state);
>         ...
>         return timeout;
> 
> do_wait_for_common():
> 
>                 do {
>                         if (signal_pending_state(state, current)) {
>                                 timeout = -ERESTARTSYS;

Once wait_for_common() is called with state = TASK_UNINTERRUPTIBLE, signal_pending_state() will 
always return false. As a result, -ERESTARTSYS is not a possible return value in this scenario.

Thanks,
Shenwei 

>                                 break;
>                         }
> ...
>                 } while (!x->done && timeout); ...
>                 if (!x->done)
>                         return timeout;
> ...
>         return timeout ?: 1;
>         Andrew