lkml.org 
[lkml]   [2019]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 2/5] gnss: sirf: power on logic for devices without wakeup signal
On Mon, 14 Jan 2019 11:51:29 +0100
Johan Hovold <johan@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 <johan@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
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2019-01-15 17:00    [W:0.109 / U:7.108 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site