On Thu, May 24, 2018 at 02:16:04PM +0200, Uwe Kleine-König wrote:
> Hello Pavel,
>
> On Thu, May 24, 2018 at 01:54:26PM +0200, Pavel Machek wrote:
> > > Given that activating a trigger can fail, let the callback return an
> > > indication. This prevents to have a trigger active according to the
> > > "trigger" sysfs attribure but not functional.
> > >
> > > This is done using a new function callback .new_activate to not break
> > > existing drivers and to allow to convert one after the other to the new
> > > mechanism. Once all users are converted, .activate can be dropped and
> > > .new_activate renamed.
> >
> > No, this is not reasonable.
> >
> > Instead of big rename, replace return value in ->activate with int. It
> > will currently return 0 for all drivers.
>
> No, currently the .activate callbacks "return" void, which e.g. on ARM
> means that the r0 register (which holds the return value when the return
> type is int) contains an unspecified value which most of the time is not
> zero.
To demonstrate the problem:
uwe@omnia:~/tmp$ cat test.c
#include <stdio.h>
static void voidfunc(void)
{
printf("lala\n");
}
int main(int argc, char *argv)
{
int (*funcptr)(void) = voidfunc;
int ret;
ret = funcptr();
printf("ret = %d\n", ret);
return 0;
}
uwe@omnia:~/tmp$ make test
cc test.c -o test
test.c: In function ‘main’:
test.c:10:25: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types]
int (*funcptr)(void) = voidfunc;
^~~~~~~~
uwe@omnia:~/tmp$ ./test
lala
ret = 5
So the return value happens to be what is still in r0 from voidfunc's
printf call. The same happens on my amd64 machine. So just changing
.activate without fixing all drivers in one go isn't an option. (Even if
I'd be willing to accept being spammed for introducing compiler
warnings.)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
