On Mon, 14 Jan 2019 11:51:29 +0100 Johan Hovold <jo...@kernel.org> wrote:
> On Thu, Jan 10, 2019 at 11:02:23PM +0100, Andreas Kemnade wrote: > > Hi Johan, > > > > On Thu, 10 Jan 2019 13:10:38 +0100 > > Johan Hovold <jo...@kernel.org> wrote: > > > > On Sun, Dec 09, 2018 at 08:51:47PM +0100, Andreas Kemnade wrote: > > > > > Additionally it checks for the initial state of the device during > > > > probing to ensure it is off. > > > > > > Is this really needed? If so, it should probably go in a separate patch. > > > > > Well, it is the main motivation for the new try of upstreaming this again. > > You know the loooong history... > > It has several times messed up my power consumption statistics. As I try > > to test patches on top of mainline, this has often led to false alarms > > regarding pm bugs in other drivers. > > > > We could also come from another kernel here via kexec having the > > device in another state. > > > > And why we do not want to check for uncommon things here? We e.g. do > > multiple tries for changing power state. > > You still need to argue why it is needed (e.g. the kexec case) and that > needs to go in the commit message of a separate patch adding something > like that as it is orthogonal to supporting configurations without > wakeup. > yes, totally agreed, there is already a separate patch with an extensive commit message. > This may also be better handled by a shutdown() callback which is where > such kexec concerns are supposed to be handled, and that would also take > care of the reboot case. This way, not everyone has to pay a penalty on > every boot for the arguable rare use case of kexec. > there are more possibilities than kexec. > > GPS chips will have usually some boot messages. So it is not the > > "send nmea data set every X seconds" for the wakeup case, it is > > another situation deserving IMHO another name. > > Ok, but unless all supported (sirf-star-based) chips have boot messages, > we'd still need to wait that long for wakeup. > I have never seen one without. But that could also be attached to the dtb compatible name or a separate property. > Are these messages you refer to output also on wake from hibernate, and > not just on boot? > also wake from hibernate. I mean $PSRF150,1*3E > > > In pseudo code we have: > > > > > > activate: > > > - toggle on-off > > > - wait(data-received, ACTIVATE_TIMEOUT + REPORT_CYCLE) > > > - reception: success > > > > Note: we can also get the goodbye/shutdown message from the chip here > > so there are chances of a false success, but since we initially power down, > > we will rule out wrong state here. > > Good point. Unless we know the current state, we'd need to sleep for > HIBERNATE_TIMEOUT before waiting for data reception. > > > > - timeout: failure > > > > > > hibernate: > > > - toggle on-off > > > - sleep(HIBERNATE_TIMEOUT) > > we could also additionally check here for > > if (last_bytes_received == GOODBYE_MSG) > > Caching and parsing the stream for this could get messy. And is the > expected message clearly defined somewhere, or would it be device (and > firmware) dependent? > I think so but I must check. $PSRF150.0 But as said, these ideas are be for a possibly later patchset. Regards, Andreas
pgpBPNdSSfKRW.pgp
Description: OpenPGP digital signature
