Linux Input/HID development
 help / color / mirror / Atom feed
From: Conor Dooley <conor@kernel.org>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Stefan Eichenberger <eichest@gmail.com>,
	nick@shmanahar.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	nicolas.ferre@microchip.com, alexandre.belloni@bootlin.com,
	claudiu.beznea@tuxon.dev, linux-input@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Stefan Eichenberger <stefan.eichenberger@toradex.com>
Subject: Re: [PATCH v1 1/2] dt-bindings: input: atmel,maxtouch: add poweroff-in-suspend property
Date: Sat, 9 Dec 2023 10:57:01 +0000	[thread overview]
Message-ID: <20231209-sizzle-monthly-6e4f3c966b0f@spud> (raw)
In-Reply-To: <ZXOoy8mFdhUQsZAu@google.com>

[-- Attachment #1: Type: text/plain, Size: 2005 bytes --]

On Fri, Dec 08, 2023 at 11:37:47PM +0000, Dmitry Torokhov wrote:
> Hi Linus, Krzysztof,
> 
> On Fri, Dec 08, 2023 at 01:54:21PM +0100, Linus Walleij wrote:
> > On Thu, Dec 7, 2023 at 12:13 PM Stefan Eichenberger <eichest@gmail.com> wrote:
> > 
> > > From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> > >
> > > Add a new property to indicate that the device should be powered off in
> > > suspend mode.
> > >
> > > Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> > (...)
> > > +  atmel,poweroff-in-suspend:
> > > +    description: |
> > > +      When this property is set, all supplies are turned off when the system is
> > > +      going to suspend.
> > > +    type: boolean
> >    wakeup-source:
> >      type: boolean
> > 
> > As Krzysztof says it seems you are describing an operating system feature.
> 
> It appears to be an OS feature, but I would argue that it is also a
> property of a board. It is tempting to say that if DTS defines supplies
> for the controller we should use them to power off the controller in
> suspend, otherwise we should use the deep sleep functionality of the
> controller. But a mere presence of regulators does not indicate if they
> can actually be powered off in suspend (i.e. if controllers shares power
> rails with another device that can be a wakeup source), so we need to
> have additional hints on how OS should behave on a given device.
> 
> On top of that we have regulator framework supplying dummy regulators...

Simply rephrasing the property might be sufficient? The current one
sounds making policy decisions with the "should be". Reframing the
commit message and property description etc in terms of what aspect of
the hardware the ability to turn off all supplies in suspend comes from
would make it more acceptable. Pretty much answering the question "why
can't we try and turn off all supplies on all systems with this device"
should get things rolling.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  reply	other threads:[~2023-12-09 10:57 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-07 11:12 [PATCH v1 0/2] Add a property to turn off the max touch controller in suspend mode Stefan Eichenberger
2023-12-07 11:12 ` [PATCH v1 1/2] dt-bindings: input: atmel,maxtouch: add poweroff-in-suspend property Stefan Eichenberger
2023-12-07 16:59   ` Krzysztof Kozlowski
2023-12-08 12:37     ` Stefan Eichenberger
2023-12-08 12:47       ` Krzysztof Kozlowski
2023-12-08 12:54   ` Linus Walleij
2023-12-08 13:11     ` Stefan Eichenberger
2023-12-08 14:23       ` Linus Walleij
2023-12-08 15:05         ` Stefan Eichenberger
2023-12-08 23:37     ` Dmitry Torokhov
2023-12-09 10:57       ` Conor Dooley [this message]
2023-12-07 11:13 ` [PATCH v1 2/2] Input: atmel_mxt_ts - support poweroff in suspend Stefan Eichenberger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231209-sizzle-monthly-6e4f3c966b0f@spud \
    --to=conor@kernel.org \
    --cc=alexandre.belloni@bootlin.com \
    --cc=claudiu.beznea@tuxon.dev \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=eichest@gmail.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nick@shmanahar.org \
    --cc=nicolas.ferre@microchip.com \
    --cc=robh+dt@kernel.org \
    --cc=stefan.eichenberger@toradex.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox