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

[Andrew Lunn]

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;
				break;
			}
...		
		} while (!x->done && timeout);
...
		if (!x->done)
			return timeout;
...
	return timeout ?: 1;

This last line is interesting, and i had to go look at the
documentation:

https://gcc.gnu.org/onlinedocs/gcc/Conditionals.html

So this is equivalent to

        return timeout ? timeout : 1;

Hence, it does appear wait_for_completion_timeout() can return
-ERESTARTSYS

There is however a comment in include/linux/errno.h

/*
 * These should never be seen by user programs.  To return one of ERESTART*
 * codes, signal_pending() MUST be set.  Note that ptrace can observe these
 * at syscall exit tracing, but they will never be left for the debugged user
 * process to see.
 */
#define ERESTARTSYS	512

So we do seem to be talking about a corner case, allowing gdb(1) to
know about it, but not user space.

	Andrew