All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Stone <ahs3@redhat.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Mark Brown <broonie@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Furquan Shaikh <furquan@chromium.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Tony Lindgren <tony@atomide.com>, Len Brown <lenb@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Hanjun Guo <hanjun.guo@linaro.org>,
	Will Deacon <will.deacon@arm.com>, Rob Herring <robh@kernel.org>,
	Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Adam Thomson <Adam.Thomson.Opensource@diasemi.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	linux-gpio@vger.kernel.org, ACPI Devel Maling List <linux-acp>
Subject: Re: [PATCH 0/7] Implement generic regulator constraints parsing for ACPI and OF
Date: Wed, 25 Jan 2017 14:44:10 -0700	[thread overview]
Message-ID: <572bcaff-7b17-0614-0cd2-0887ae33eb65@redhat.com> (raw)
In-Reply-To: <20170125192711.GB27255@dtor-ws>

On 01/25/2017 12:27 PM, Dmitry Torokhov wrote:
> On Wed, Jan 25, 2017 at 10:44:32AM -0800, Dmitry Torokhov wrote:
>> On Wed, Jan 25, 2017 at 06:29:55PM +0000, Mark Brown wrote:
>>> On Wed, Jan 25, 2017 at 06:23:20PM +0000, Mark Rutland wrote:
>>>> On Wed, Jan 25, 2017 at 08:56:42AM -0800, Furquan Shaikh wrote:
>>>
>>>>> That is the reason why the recent change to add ACPI support to fixed
>>>>> regulators was done
>>>>> (https://github.com/torvalds/linux/blob/master/drivers/regulator/fixed.c#L100).
>>>
>>>> To be honest, I'm surprised this got merged.
>>>
>>> My understanding was that it was instantiated from another device as an
>>> implementation detail of that device, letting it say "this GPIO should
>>> be handled as a regulator".
>>>
>>>> Mark, this was added in this cycle; can we please rip that out for now?
>>>
>>> If it's instantiated directly we probably should.
>>>
>>>> We can certainly come up with something that allows drivers to support
>>>> both, but trying to do this without updating drivers opens a huge set of
>>>> problems.
>>>
>>> I think there's a reasonable chance that any ACPI specs could be written
>>> in such a way as to allow transparent support in Linux, the main thing
>>> I'd worry about is naming issues.
>>
>> So if I am reading this correctly, currently ACPI does not expose power
>> supplies directly, but rather ties them to the device power state (D0,
>> D3cold, etc). Linux drivers do not usually follow that state model and
>> expect to have all their power supplies be given to them and then
>> figures out what to do with them itself. Given that, what do we do? Do
>> we map only entries from _PR3 so they are available to drivers via
>> regulator_get()? Or we ask the standard to add method enumerating all
>> supplies?
> 
> For the record, the main issue for the drivers, which is being solved by
> exposing power supplies to the driver, is the following:
> 
> 1. We suspend the device. Since there is no regulators the driver
> assumes that it will retain it's state upon resume
> 2. System goes into some sleep state
> 3. System wakes up
> 4. Device goes through resume, normally disabling wakeup interrupt and
> enabling normal processing
> 5. We end up with non functioning device because the firmware actually
> cut the power off without the driver knowing anything about it.
> 
> I would really hate to go through _every_ driver and add the following
> code to the resume path:
> 
> #if IS_ENABLED(CONFIG_ACPI)
> 	if (acpi_device_was_powered_off_between_suspend_and_now(dev)) {
> 		completely_reinitialize_device(dev);
> 	}
> #endif
> 
> Thanks.
> 

Please see Sections 3.2-3.5 (3.6, too, for a broader picture) in the ACPI spec
[0] for an overview of ACPI power management.  Section 7 of the spec [0] adds
the details on how the firmware and OS are to cooperate in managing power.

In those sections, ACPI defines a PowerResource object, aka a power supply of
some flavor.  ACPI devices are then connected to that power resource.  So, the
spec may already have what you need defined.  Further, the code in drivers/acpi
/device_pm.c and drivers/acpi/power.c may handle the situations described -- I
believe that's their intent, at any rate.   If the ACPI ASL is written
properly, I don't think the device driver would have to add the code shown
above; it would just be handled via the acpi driver and ASL.  The ACPI model is
specifically designed so that drivers don't have to know these sorts of things,
so that the hardware and firmware underneath the OS can change over time
without having to change the OS -- the OS just needs to know how to talk to
ACPI.

But, to the point of some of the other discussion on the thread, this ACPI sort
of power management is a very, very different model than DT so that intertwining
the two models is highly unlikely to work, IMHO.



[0] http://www.uefi.org/specifications -- version 6.1, specifically.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@redhat.com
-----------------------------------

WARNING: multiple messages have this Message-ID (diff)
From: Al Stone <ahs3@redhat.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Mark Brown <broonie@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Furquan Shaikh <furquan@chromium.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Tony Lindgren <tony@atomide.com>, Len Brown <lenb@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Hanjun Guo <hanjun.guo@linaro.org>,
	Will Deacon <will.deacon@arm.com>, Rob Herring <robh@kernel.org>,
	Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Adam Thomson <Adam.Thomson.Opensource@diasemi.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	linux-gpio@vger.kernel.org,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux OMAP Mailing List <linux-omap@vger.kernel.org>,
	Aaron Durbin <adurbin@chromium.org>,
	dlaurie@chromium.org
Subject: Re: [PATCH 0/7] Implement generic regulator constraints parsing for ACPI and OF
Date: Wed, 25 Jan 2017 14:44:10 -0700	[thread overview]
Message-ID: <572bcaff-7b17-0614-0cd2-0887ae33eb65@redhat.com> (raw)
In-Reply-To: <20170125192711.GB27255@dtor-ws>

On 01/25/2017 12:27 PM, Dmitry Torokhov wrote:
> On Wed, Jan 25, 2017 at 10:44:32AM -0800, Dmitry Torokhov wrote:
>> On Wed, Jan 25, 2017 at 06:29:55PM +0000, Mark Brown wrote:
>>> On Wed, Jan 25, 2017 at 06:23:20PM +0000, Mark Rutland wrote:
>>>> On Wed, Jan 25, 2017 at 08:56:42AM -0800, Furquan Shaikh wrote:
>>>
>>>>> That is the reason why the recent change to add ACPI support to fixed
>>>>> regulators was done
>>>>> (https://github.com/torvalds/linux/blob/master/drivers/regulator/fixed.c#L100).
>>>
>>>> To be honest, I'm surprised this got merged.
>>>
>>> My understanding was that it was instantiated from another device as an
>>> implementation detail of that device, letting it say "this GPIO should
>>> be handled as a regulator".
>>>
>>>> Mark, this was added in this cycle; can we please rip that out for now?
>>>
>>> If it's instantiated directly we probably should.
>>>
>>>> We can certainly come up with something that allows drivers to support
>>>> both, but trying to do this without updating drivers opens a huge set of
>>>> problems.
>>>
>>> I think there's a reasonable chance that any ACPI specs could be written
>>> in such a way as to allow transparent support in Linux, the main thing
>>> I'd worry about is naming issues.
>>
>> So if I am reading this correctly, currently ACPI does not expose power
>> supplies directly, but rather ties them to the device power state (D0,
>> D3cold, etc). Linux drivers do not usually follow that state model and
>> expect to have all their power supplies be given to them and then
>> figures out what to do with them itself. Given that, what do we do? Do
>> we map only entries from _PR3 so they are available to drivers via
>> regulator_get()? Or we ask the standard to add method enumerating all
>> supplies?
> 
> For the record, the main issue for the drivers, which is being solved by
> exposing power supplies to the driver, is the following:
> 
> 1. We suspend the device. Since there is no regulators the driver
> assumes that it will retain it's state upon resume
> 2. System goes into some sleep state
> 3. System wakes up
> 4. Device goes through resume, normally disabling wakeup interrupt and
> enabling normal processing
> 5. We end up with non functioning device because the firmware actually
> cut the power off without the driver knowing anything about it.
> 
> I would really hate to go through _every_ driver and add the following
> code to the resume path:
> 
> #if IS_ENABLED(CONFIG_ACPI)
> 	if (acpi_device_was_powered_off_between_suspend_and_now(dev)) {
> 		completely_reinitialize_device(dev);
> 	}
> #endif
> 
> Thanks.
> 

Please see Sections 3.2-3.5 (3.6, too, for a broader picture) in the ACPI spec
[0] for an overview of ACPI power management.  Section 7 of the spec [0] adds
the details on how the firmware and OS are to cooperate in managing power.

In those sections, ACPI defines a PowerResource object, aka a power supply of
some flavor.  ACPI devices are then connected to that power resource.  So, the
spec may already have what you need defined.  Further, the code in drivers/acpi
/device_pm.c and drivers/acpi/power.c may handle the situations described -- I
believe that's their intent, at any rate.   If the ACPI ASL is written
properly, I don't think the device driver would have to add the code shown
above; it would just be handled via the acpi driver and ASL.  The ACPI model is
specifically designed so that drivers don't have to know these sorts of things,
so that the hardware and firmware underneath the OS can change over time
without having to change the OS -- the OS just needs to know how to talk to
ACPI.

But, to the point of some of the other discussion on the thread, this ACPI sort
of power management is a very, very different model than DT so that intertwining
the two models is highly unlikely to work, IMHO.



[0] http://www.uefi.org/specifications -- version 6.1, specifically.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@redhat.com
-----------------------------------

  parent reply	other threads:[~2017-01-25 21:44 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-25  0:06 [PATCH 0/7] Implement generic regulator constraints parsing for ACPI and OF Furquan Shaikh
2017-01-25  0:06 ` [PATCH 1/7] drivers/regulator: Rename of_map_mode to map_mode in regulator desc Furquan Shaikh
2017-01-25  0:06 ` [PATCH 2/7] ACPI / property: have acpi_get_next_subnode take fwnode_handle Furquan Shaikh
2017-01-25  0:06   ` Furquan Shaikh
2017-01-25 11:00   ` kbuild test robot
2017-01-25 11:00     ` kbuild test robot
2017-01-25  0:06 ` [PATCH 3/7] device property: introduce fwnode_for_each_child() Furquan Shaikh
2017-01-25  0:06   ` Furquan Shaikh
2017-01-25  0:06 ` [PATCH 4/7] device property: introduce fwnode_get_named_child_node() Furquan Shaikh
2017-01-25  0:06   ` Furquan Shaikh
2017-01-25  0:06 ` [PATCH 5/7] device property: Export dev_fwnode Furquan Shaikh
2017-01-25  0:06 ` [PATCH 6/7] drivers/gpio: Add and export gpiod_lookup[_index] Furquan Shaikh
2017-01-25  0:06   ` Furquan Shaikh
2017-01-25 11:18   ` kbuild test robot
2017-01-25 11:18     ` kbuild test robot
2017-01-26 15:24   ` Linus Walleij
2017-01-26 15:24     ` Linus Walleij
2017-01-25  0:06 ` [PATCH 7/7] drivers/regulator: Initialize regulator init data for ACPI regulators Furquan Shaikh
2017-01-25 12:29 ` [PATCH 0/7] Implement generic regulator constraints parsing for ACPI and OF Lorenzo Pieralisi
2017-01-25 12:49 ` Mark Brown
2017-01-25 12:49   ` Mark Brown
2017-01-25 12:55   ` Rafael J. Wysocki
2017-01-25 12:55     ` Rafael J. Wysocki
2017-01-25 16:56     ` Furquan Shaikh
2017-01-25 16:56       ` Furquan Shaikh
2017-01-25 18:23       ` Mark Rutland
2017-01-25 18:23         ` Mark Rutland
2017-01-25 18:29         ` Mark Brown
2017-01-25 18:29           ` Mark Brown
2017-01-25 18:34           ` Mark Rutland
2017-01-25 18:34             ` Mark Rutland
2017-01-25 18:49             ` Mark Brown
2017-01-25 18:49               ` Mark Brown
2017-01-25 19:39               ` Mark Rutland
2017-01-25 19:39                 ` Mark Rutland
2017-01-25 18:44           ` Dmitry Torokhov
2017-01-25 18:44             ` Dmitry Torokhov
2017-01-25 19:27             ` Dmitry Torokhov
2017-01-25 19:27               ` Dmitry Torokhov
2017-01-25 20:39               ` Mark Brown
2017-01-25 20:39                 ` Mark Brown
2017-01-25 21:17                 ` Dmitry Torokhov
2017-01-25 21:17                   ` Dmitry Torokhov
2017-01-25 21:30                   ` Mark Brown
2017-01-25 21:30                     ` Mark Brown
2017-01-25 22:05                     ` Dmitry Torokhov
2017-01-25 22:05                       ` Dmitry Torokhov
2017-01-25 22:25                       ` Mark Brown
2017-01-25 22:25                         ` Mark Brown
2017-01-25 21:44               ` Al Stone [this message]
2017-01-25 21:44                 ` Al Stone
2017-01-25 23:27                 ` Dmitry Torokhov
2017-01-25 23:27                   ` Dmitry Torokhov
2017-01-26  0:15                   ` Al Stone
2017-01-26  0:15                     ` Al Stone
2017-01-26  0:33                     ` Dmitry Torokhov
2017-01-26  0:33                       ` Dmitry Torokhov
2017-01-26 10:35                       ` Rafael J. Wysocki
2017-01-26 10:35                         ` Rafael J. Wysocki
2017-02-04 16:08                         ` Mark Brown
2017-02-04 16:08                           ` Mark Brown
2017-01-25 19:21           ` Lorenzo Pieralisi
2017-01-25 19:21             ` Lorenzo Pieralisi
2017-01-25 20:40             ` Mark Brown
2017-01-25 20:40               ` Mark Brown
2017-01-25 18:25       ` Mark Brown
2017-01-25 18:25         ` Mark Brown

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=572bcaff-7b17-0614-0cd2-0887ae33eb65@redhat.com \
    --to=ahs3@redhat.com \
    --cc=Adam.Thomson.Opensource@diasemi.com \
    --cc=broonie@kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=furquan@chromium.org \
    --cc=gnurou@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hanjun.guo@linaro.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=lenb@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=mark.rutland@arm.com \
    --cc=rafael@kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=robh@kernel.org \
    --cc=sathyanarayana.nujella@intel.com \
    --cc=tony@atomide.com \
    --cc=will.deacon@arm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.