Re: [PATCH v1 1/2] dt-bindings: input: atmel,maxtouch: add poweroff-in-suspend property

[Linus Walleij]

On Fri, Dec 8, 2023 at 2:11 PM Stefan Eichenberger <eichest@xxxxxxxxx> wrote:

> > I can't help but wonder: shouldn't that pretty much be the default behaviour
> > if wakeup-source is *not* specified?
> >
> > I.e. the property kind of describes !wakeup-source.
>
> The maxtouch controller has a deep sleep mode which is currently used
> without powering down vdd and vdda. However, because we have a shared
> regulator which powers other peripherals that we want to shut down in
> suspend we need a way to power down vdd and vdda. However, I agree this
> is not really a feature of the device. The feature would basically be
> the internal deep sleep mode.

While it is of no concern to the device tree bindings, Linux regulators
are counting meaning that you need to make all peripherals disable
their regulators and it will come down.

> I didn't want to change the default
> behaviour of the driver, so I added this property but maybe I could
> change it to:
>
> atmel,deep-sleep:
>   description: |
>      Use the maxtouch deep-sleep mode instead of powering down vdd and
>      vdda.
>
> Or to not change the default behaviour:
> atmel,no-deep-sleep:
>   description: |
>      Do not use the maxtouch deep-sleep mode but power down vdd and vdda
>      in suspend.
>
> As I understand the datasheet even if the maxtouch is using its deep
> sleep mode it does not act as a wakeup source.

Do you mean it can still work as a wakeup source in deep sleep mode?
(there is a "not" too much above ...)

> It is just faster in
> waking up because it can keep the configuration in memory.

That sounds like a good reason to have the property, because that
means that if you can control the wakeup latency and specify in the binding
how much in absolute time units it is affected.

I would define it in positive terms instead of reverse "no-deep-sleep"
though such as "atmel,fast-wakeup".

And: If you disable the regulators it will probably *not* be able to wake the
system up, right? And that is just a few lines of code in the driver such as:

go_to_sleep():
  if (!wakeup_source):
     disable_regulators()

Yours,
Linus Walleij