On 01/04/2017 12:20 AM, Bryan O'Donoghue wrote: > > > On 03/01/17 17:57, Alexandre Bailon wrote: >> Exiting the thread let loopback in incorrect state: >> nothing will work because the thread stopped but sysfs / debugfs files >> are still there. > > actually... reading your email again > > why does the thread need to exit because gb_pm_runtime_get_sync() failed ? > > Can't we just print some sort of warning message and continue ? > > diff --git a/drivers/staging/greybus/loopback.c > b/drivers/staging/greybus/loopback.c > index 3184dd3..dfdcea3 100644 > --- a/drivers/staging/greybus/loopback.c > +++ b/drivers/staging/greybus/loopback.c > @@ -993,7 +993,7 @@ static int gb_loopback_fn(void *data) > kthread_should_stop()); > ret = gb_pm_runtime_get_sync(bundle); > if (ret) > - return ret; > + dev_warn(&bundle->dev, "broken - oh > well\n"); > } > > if (kthread_should_stop()) > >> I'm volunteer to fix the issue but I'm not sure how to do it, >> and may be there is other better solutions I might have missed. >> So, all suggestion are welcome. > > You made a valid point about sysfs entries - these should only be > cleaned up in the module exit path and a do_exit() as I suggested in my > previous email would (I think anyway) fix the error you reported on > rmmod but would still leave /sysfs entries dangling around between the > time of the do_exit() and the rmmod (if it ever happened) which would be > wrong. > > So - can't we just ignore gb_pm_runtime_get_sync() if it returns an error ? I think it's a good solution. In addition of avoiding the kthread issue, it will also let the thread try to do a transfer which will not work and rise an error to the user space application. > > Trying to exit the thread and cleanup the associated /sysfs entries > would be a dogs dinner of a solution IMO. > > --- > bod _______________________________________________ greybus-dev mailing list greybus-dev@xxxxxxxxxxxxxxxx https://lists.linaro.org/mailman/listinfo/greybus-dev
|